Skip to content

Commit

Permalink
Bidder-specific imp FPD (prebid#3704)
Browse files Browse the repository at this point in the history
  • Loading branch information
@bsardo @mefjush
bsardo authored and mefjush committed Jul 19, 2024
1 parent 5708201 commit 74f7ed5
Show file tree
Hide file tree
Showing 16 changed files with 952 additions and 33 deletions.
4 changes: 2 additions & 2 deletions endpoints/openrtb2/amp_auction.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -236,8 +236,8 @@ func (deps *endpointDeps) AmpAuction(w http.ResponseWriter, r *http.Request, _ h
errL = append(errL, errs...)
}

hasStoredResponses := len(storedAuctionResponses) > 0
errs := deps.validateRequest(account, r, reqWrapper, true, hasStoredResponses, storedBidResponses, false)
hasStoredAuctionResponses := len(storedAuctionResponses) > 0
errs := deps.validateRequest(account, r, reqWrapper, true, hasStoredAuctionResponses, storedBidResponses, false)
errL = append(errL, errs...)
ao.Errors = append(ao.Errors, errs...)
if errortypes.ContainsFatalError(errs) {
Expand Down
8 changes: 4 additions & 4 deletions endpoints/openrtb2/auction.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -562,8 +562,8 @@ func (deps *endpointDeps) parseRequest(httpRequest *http.Request, labels *metric
return nil, nil, nil, nil, nil, nil, errs
}

hasStoredResponses := len(storedAuctionResponses) > 0
errL = deps.validateRequest(account, httpRequest, req, false, hasStoredResponses, storedBidResponses, hasStoredBidRequest)
hasStoredAuctionResponses := len(storedAuctionResponses) > 0
errL = deps.validateRequest(account, httpRequest, req, false, hasStoredAuctionResponses, storedBidResponses, hasStoredBidRequest)
if len(errL) > 0 {
errs = append(errs, errL...)
}
Expand Down Expand Up @@ -757,7 +757,7 @@ func mergeBidderParamsImpExtPrebid(impExt *openrtb_ext.ImpExt, reqExtParams map[
return nil
}

func (deps *endpointDeps) validateRequest(account *config.Account, httpReq *http.Request, req *openrtb_ext.RequestWrapper, isAmp bool, hasStoredResponses bool, storedBidResp stored_responses.ImpBidderStoredResp, hasStoredBidRequest bool) []error {
func (deps *endpointDeps) validateRequest(account *config.Account, httpReq *http.Request, req *openrtb_ext.RequestWrapper, isAmp bool, hasStoredAuctionResponses bool, storedBidResp stored_responses.ImpBidderStoredResp, hasStoredBidRequest bool) []error {
errL := []error{}
if req.ID == "" {
return []error{errors.New("request missing required field: \"id\"")}
Expand Down Expand Up @@ -918,7 +918,7 @@ func (deps *endpointDeps) validateRequest(account *config.Account, httpReq *http
}
impIDs[imp.ID] = i

errs := deps.requestValidator.ValidateImp(imp, i, requestAliases, hasStoredResponses, storedBidResp)
errs := deps.requestValidator.ValidateImp(imp, i, requestAliases, hasStoredAuctionResponses, storedBidResp)
if len(errs) > 0 {
errL = append(errL, errs...)
}
Expand Down
4 changes: 3 additions & 1 deletion endpoints/openrtb2/auction_benchmark_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ func BenchmarkOpenrtbEndpoint(b *testing.B) {
if err != nil {
return
}
requestValidator := ortb.NewRequestValidator(nil, map[string]string{}, paramValidator)

nilMetrics := &metricsConfig.NilMetricsEngine{}

Expand All @@ -88,6 +89,7 @@ func BenchmarkOpenrtbEndpoint(b *testing.B) {
adapters,
nil,
&config.Configuration{},
requestValidator,
map[string]usersync.Syncer{},
nilMetrics,
infos,
Expand All @@ -102,7 +104,7 @@ func BenchmarkOpenrtbEndpoint(b *testing.B) {
endpoint, _ := NewEndpoint(
fakeUUIDGenerator{},
exchange,
ortb.NewRequestValidator(nil, map[string]string{}, paramValidator),
requestValidator,
empty_fetcher.EmptyFetcher{},
empty_fetcher.EmptyFetcher{},
&config.Configuration{MaxRequestSize: maxSize},
Expand Down
5 changes: 3 additions & 2 deletions endpoints/openrtb2/test_utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1197,7 +1197,7 @@ func (te *exchangeTestWrapper) HoldAuction(ctx context.Context, r *exchange.Auct
}

// buildTestExchange returns an exchange with mock bidder servers and mock currency conversion server
func buildTestExchange(testCfg *testConfigValues, adapterMap map[openrtb_ext.BidderName]exchange.AdaptedBidder, mockBidServersArray []*httptest.Server, mockCurrencyRatesServer *httptest.Server, bidderInfos config.BidderInfos, cfg *config.Configuration, met metrics.MetricsEngine, mockFetcher stored_requests.CategoryFetcher) (exchange.Exchange, []*httptest.Server) {
func buildTestExchange(testCfg *testConfigValues, adapterMap map[openrtb_ext.BidderName]exchange.AdaptedBidder, mockBidServersArray []*httptest.Server, mockCurrencyRatesServer *httptest.Server, bidderInfos config.BidderInfos, cfg *config.Configuration, met metrics.MetricsEngine, mockFetcher stored_requests.CategoryFetcher, requestValidator ortb.RequestValidator) (exchange.Exchange, []*httptest.Server) {
if len(testCfg.MockBidders) == 0 {
testCfg.MockBidders = append(testCfg.MockBidders, mockBidderHandler{BidderName: "appnexus", Currency: "USD", Price: 0.00})
}
Expand All @@ -1220,6 +1220,7 @@ func buildTestExchange(testCfg *testConfigValues, adapterMap map[openrtb_ext.Bid
testExchange := exchange.NewExchange(adapterMap,
&wellBehavedCache{},
cfg,
requestValidator,
nil,
met,
bidderInfos,
Expand Down Expand Up @@ -1276,7 +1277,7 @@ func buildTestEndpoint(test testCase, cfg *config.Configuration) (httprouter.Han
}
mockCurrencyRatesServer := httptest.NewServer(http.HandlerFunc(mockCurrencyConversionService.handle))

testExchange, mockBidServersArray := buildTestExchange(test.Config, adapterMap, mockBidServersArray, mockCurrencyRatesServer, bidderInfos, cfg, met, mockFetcher)
testExchange, mockBidServersArray := buildTestExchange(test.Config, adapterMap, mockBidServersArray, mockCurrencyRatesServer, bidderInfos, cfg, met, mockFetcher, requestValidator)

var storedRequestFetcher stored_requests.Fetcher
if len(test.StoredRequest) > 0 {
Expand Down
1 change: 1 addition & 0 deletions errortypes/code.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ const (
TmaxTimeoutErrorCode
FailedToMarshalErrorCode
FailedToUnmarshalErrorCode
InvalidImpFirstPartyDataErrorCode
)

// Defines numeric codes for well-known warnings.
Expand Down
18 changes: 18 additions & 0 deletions errortypes/errortypes.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -274,3 +274,21 @@ func (err *DebugWarning) Severity() Severity {
func (err *DebugWarning) Scope() Scope {
return ScopeDebug
}

// InvalidImpFirstPartyData should be used when the retrieved account config cannot be unmarshaled
// These errors will be written to http.ResponseWriter before canceling execution
type InvalidImpFirstPartyData struct {
Message string
}

func (err *InvalidImpFirstPartyData) Error() string {
return err.Message
}

func (err *InvalidImpFirstPartyData) Code() int {
return InvalidImpFirstPartyDataErrorCode
}

func (err *InvalidImpFirstPartyData) Severity() Severity {
return SeverityFatal
}
9 changes: 8 additions & 1 deletion exchange/exchange.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"strings"
"time"

"github.com/prebid/prebid-server/v2/ortb"
"github.com/prebid/prebid-server/v2/privacy"

"github.com/prebid/prebid-server/v2/adapters"
Expand Down Expand Up @@ -132,7 +133,7 @@ func (randomDeduplicateBidBooleanGenerator) Generate() bool {
return rand.Intn(100) < 50
}

func NewExchange(adapters map[openrtb_ext.BidderName]AdaptedBidder, cache prebid_cache_client.Client, cfg *config.Configuration, syncersByBidder map[string]usersync.Syncer, metricsEngine metrics.MetricsEngine, infos config.BidderInfos, gdprPermsBuilder gdpr.PermissionsBuilder, currencyConverter *currency.RateConverter, categoriesFetcher stored_requests.CategoryFetcher, adsCertSigner adscert.Signer, macroReplacer macros.Replacer, priceFloorFetcher floors.FloorFetcher) Exchange {
func NewExchange(adapters map[openrtb_ext.BidderName]AdaptedBidder, cache prebid_cache_client.Client, cfg *config.Configuration, requestValidator ortb.RequestValidator, syncersByBidder map[string]usersync.Syncer, metricsEngine metrics.MetricsEngine, infos config.BidderInfos, gdprPermsBuilder gdpr.PermissionsBuilder, currencyConverter *currency.RateConverter, categoriesFetcher stored_requests.CategoryFetcher, adsCertSigner adscert.Signer, macroReplacer macros.Replacer, priceFloorFetcher floors.FloorFetcher) Exchange {
bidderToSyncerKey := map[string]string{}
for bidder, syncer := range syncersByBidder {
bidderToSyncerKey[bidder] = syncer.Key()
Expand All @@ -155,6 +156,7 @@ func NewExchange(adapters map[openrtb_ext.BidderName]AdaptedBidder, cache prebid
gdprPermsBuilder: gdprPermsBuilder,
hostSChainNode: cfg.HostSChainNode,
bidderInfo: infos,
requestValidator: requestValidator,
}

return &exchange{
Expand Down Expand Up @@ -338,6 +340,11 @@ func (e *exchange) HoldAuction(ctx context.Context, r *AuctionRequest, debugLog
SChain: requestExt.GetSChain(),
}
bidderRequests, privacyLabels, errs := e.requestSplitter.cleanOpenRTBRequests(ctx, *r, requestExtLegacy, gdprSignal, gdprEnforced, bidAdjustmentFactors)
for _, err := range errs {
if errortypes.ReadCode(err) == errortypes.InvalidImpFirstPartyDataErrorCode {
return nil, err
}
}
errs = append(errs, floorErrs...)

mergedBidAdj, err := bidadjustment.Merge(r.BidRequestWrapper, r.Account.BidAdjustments)
Expand Down
40 changes: 30 additions & 10 deletions exchange/exchange_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,12 @@ import (
metricsConf "github.com/prebid/prebid-server/v2/metrics/config"
metricsConfig "github.com/prebid/prebid-server/v2/metrics/config"
"github.com/prebid/prebid-server/v2/openrtb_ext"
"github.com/prebid/prebid-server/v2/ortb"
pbc "github.com/prebid/prebid-server/v2/prebid_cache_client"
"github.com/prebid/prebid-server/v2/privacy"
"github.com/prebid/prebid-server/v2/stored_requests"
"github.com/prebid/prebid-server/v2/stored_requests/backends/file_fetcher"
"github.com/prebid/prebid-server/v2/stored_responses"
"github.com/prebid/prebid-server/v2/usersync"
"github.com/prebid/prebid-server/v2/util/jsonutil"
"github.com/prebid/prebid-server/v2/util/ptrutil"
Expand Down Expand Up @@ -82,7 +84,7 @@ func TestNewExchange(t *testing.T) {
},
}.Builder

e := NewExchange(adapters, nil, cfg, map[string]usersync.Syncer{}, &metricsConf.NilMetricsEngine{}, biddersInfo, gdprPermsBuilder, currencyConverter, nilCategoryFetcher{}, &adscert.NilSigner{}, macros.NewStringIndexBasedReplacer(), nil).(*exchange)
e := NewExchange(adapters, nil, cfg, &mockRequestValidator{}, map[string]usersync.Syncer{}, &metricsConf.NilMetricsEngine{}, biddersInfo, gdprPermsBuilder, currencyConverter, nilCategoryFetcher{}, &adscert.NilSigner{}, macros.NewStringIndexBasedReplacer(), nil).(*exchange)
for _, bidderName := range knownAdapters {
if _, ok := e.adapterMap[bidderName]; !ok {
if biddersInfo[string(bidderName)].IsEnabled() {
Expand Down Expand Up @@ -132,7 +134,7 @@ func TestCharacterEscape(t *testing.T) {
},
}.Builder

e := NewExchange(adapters, nil, cfg, map[string]usersync.Syncer{}, &metricsConf.NilMetricsEngine{}, biddersInfo, gdprPermsBuilder, currencyConverter, nilCategoryFetcher{}, &adscert.NilSigner{}, macros.NewStringIndexBasedReplacer(), nil).(*exchange)
e := NewExchange(adapters, nil, cfg, &mockRequestValidator{}, map[string]usersync.Syncer{}, &metricsConf.NilMetricsEngine{}, biddersInfo, gdprPermsBuilder, currencyConverter, nilCategoryFetcher{}, &adscert.NilSigner{}, macros.NewStringIndexBasedReplacer(), nil).(*exchange)

// 3) Build all the parameters e.buildBidResponse(ctx.Background(), liveA... ) needs
//liveAdapters []openrtb_ext.BidderName,
Expand Down Expand Up @@ -1235,7 +1237,7 @@ func TestGetBidCacheInfoEndToEnd(t *testing.T) {
},
}.Builder

e := NewExchange(adapters, pbc, cfg, map[string]usersync.Syncer{}, &metricsConf.NilMetricsEngine{}, biddersInfo, gdprPermsBuilder, currencyConverter, nilCategoryFetcher{}, &adscert.NilSigner{}, macros.NewStringIndexBasedReplacer(), nil).(*exchange)
e := NewExchange(adapters, pbc, cfg, &mockRequestValidator{}, map[string]usersync.Syncer{}, &metricsConf.NilMetricsEngine{}, biddersInfo, gdprPermsBuilder, currencyConverter, nilCategoryFetcher{}, &adscert.NilSigner{}, macros.NewStringIndexBasedReplacer(), nil).(*exchange)
// 3) Build all the parameters e.buildBidResponse(ctx.Background(), liveA... ) needs
liveAdapters := []openrtb_ext.BidderName{bidderName}

Expand Down Expand Up @@ -1594,7 +1596,7 @@ func TestBidResponseCurrency(t *testing.T) {
},
}.Builder

e := NewExchange(adapters, nil, cfg, map[string]usersync.Syncer{}, &metricsConf.NilMetricsEngine{}, biddersInfo, gdprPermsBuilder, currencyConverter, nilCategoryFetcher{}, &adscert.NilSigner{}, macros.NewStringIndexBasedReplacer(), nil).(*exchange)
e := NewExchange(adapters, nil, cfg, &mockRequestValidator{}, map[string]usersync.Syncer{}, &metricsConf.NilMetricsEngine{}, biddersInfo, gdprPermsBuilder, currencyConverter, nilCategoryFetcher{}, &adscert.NilSigner{}, macros.NewStringIndexBasedReplacer(), nil).(*exchange)

liveAdapters := make([]openrtb_ext.BidderName, 1)
liveAdapters[0] = "appnexus"
Expand Down Expand Up @@ -1742,7 +1744,7 @@ func TestBidResponseImpExtInfo(t *testing.T) {
t.Fatalf("Error intializing adapters: %v", adaptersErr)
}

e := NewExchange(adapters, nil, cfg, map[string]usersync.Syncer{}, &metricsConf.NilMetricsEngine{}, nil, gdprPermsBuilder, nil, nilCategoryFetcher{}, &adscert.NilSigner{}, macros.NewStringIndexBasedReplacer(), nil).(*exchange)
e := NewExchange(adapters, nil, cfg, &mockRequestValidator{}, map[string]usersync.Syncer{}, &metricsConf.NilMetricsEngine{}, nil, gdprPermsBuilder, nil, nilCategoryFetcher{}, &adscert.NilSigner{}, macros.NewStringIndexBasedReplacer(), nil).(*exchange)

liveAdapters := make([]openrtb_ext.BidderName, 1)
liveAdapters[0] = "appnexus"
Expand Down Expand Up @@ -1836,7 +1838,7 @@ func TestRaceIntegration(t *testing.T) {
},
}.Builder

ex := NewExchange(adapters, &wellBehavedCache{}, cfg, map[string]usersync.Syncer{}, &metricsConf.NilMetricsEngine{}, biddersInfo, gdprPermsBuilder, currencyConverter, &nilCategoryFetcher{}, &adscert.NilSigner{}, macros.NewStringIndexBasedReplacer(), nil).(*exchange)
ex := NewExchange(adapters, &wellBehavedCache{}, cfg, &mockRequestValidator{}, map[string]usersync.Syncer{}, &metricsConf.NilMetricsEngine{}, biddersInfo, gdprPermsBuilder, currencyConverter, &nilCategoryFetcher{}, &adscert.NilSigner{}, macros.NewStringIndexBasedReplacer(), nil).(*exchange)
_, err = ex.HoldAuction(context.Background(), auctionRequest, &debugLog)
if err != nil {
t.Errorf("HoldAuction returned unexpected error: %v", err)
Expand Down Expand Up @@ -1934,7 +1936,7 @@ func TestPanicRecovery(t *testing.T) {
},
}.Builder

e := NewExchange(adapters, nil, cfg, map[string]usersync.Syncer{}, &metricsConf.NilMetricsEngine{}, biddersInfo, gdprPermsBuilder, currencyConverter, nilCategoryFetcher{}, &adscert.NilSigner{}, macros.NewStringIndexBasedReplacer(), nil).(*exchange)
e := NewExchange(adapters, nil, cfg, &mockRequestValidator{}, map[string]usersync.Syncer{}, &metricsConf.NilMetricsEngine{}, biddersInfo, gdprPermsBuilder, currencyConverter, nilCategoryFetcher{}, &adscert.NilSigner{}, macros.NewStringIndexBasedReplacer(), nil).(*exchange)

chBids := make(chan *bidResponseWrapper, 1)
panicker := func(bidderRequest BidderRequest, conversions currency.Conversions) {
Expand Down Expand Up @@ -2004,7 +2006,7 @@ func TestPanicRecoveryHighLevel(t *testing.T) {
allowAllBidders: true,
},
}.Builder
e := NewExchange(adapters, &mockCache{}, cfg, map[string]usersync.Syncer{}, &metricsConf.NilMetricsEngine{}, biddersInfo, gdprPermsBuilder, currencyConverter, categoriesFetcher, &adscert.NilSigner{}, macros.NewStringIndexBasedReplacer(), nil).(*exchange)
e := NewExchange(adapters, &mockCache{}, cfg, &mockRequestValidator{}, map[string]usersync.Syncer{}, &metricsConf.NilMetricsEngine{}, biddersInfo, gdprPermsBuilder, currencyConverter, categoriesFetcher, &adscert.NilSigner{}, macros.NewStringIndexBasedReplacer(), nil).(*exchange)

e.adapterMap[openrtb_ext.BidderBeachfront] = panicingAdapter{}
e.adapterMap[openrtb_ext.BidderAppnexus] = panicingAdapter{}
Expand Down Expand Up @@ -2335,7 +2337,8 @@ func runSpec(t *testing.T, filename string, spec *exchangeSpec) {
}

func findBiddersInAuction(t *testing.T, context string, req *openrtb2.BidRequest) []string {
if splitImps, err := splitImps(req.Imp); err != nil {

if splitImps, err := splitImps(req.Imp, &mockRequestValidator{}, nil, false, nil); err != nil {
t.Errorf("%s: Failed to parse Bidders from request: %v", context, err)
return nil
} else {
Expand Down Expand Up @@ -2384,6 +2387,8 @@ func newExchangeForTests(t *testing.T, filename string, expectations map[string]
mockResponses: map[string]bidderResponse{string(bidderName): spec.MockResponse},
}
bidderInfos[string(bidderName)] = config.BidderInfo{ModifyingVastXmlAllowed: spec.ModifyingVastXmlAllowed}
} else {
bidderInfos[string(bidderName)] = config.BidderInfo{}
}
}

Expand Down Expand Up @@ -2433,13 +2438,20 @@ func newExchangeForTests(t *testing.T, filename string, expectations map[string]
}

metricsEngine := metricsConf.NewMetricsEngine(&config.Configuration{}, openrtb_ext.CoreBidderNames(), nil, nil)
paramValidator, err := openrtb_ext.NewBidderParamsValidator("../static/bidder-params")
if err != nil {
t.Fatalf("Failed to create params validator: %v", error)
}
bidderMap := GetActiveBidders(bidderInfos)
requestValidator := ortb.NewRequestValidator(bidderMap, map[string]string{}, paramValidator)
requestSplitter := requestSplitter{
bidderToSyncerKey: bidderToSyncerKey,
me: metricsEngine,
privacyConfig: privacyConfig,
gdprPermsBuilder: gdprPermsBuilder,
hostSChainNode: hostSChainNode,
bidderInfo: bidderInfos,
requestValidator: requestValidator,
}

return &exchange{
Expand Down Expand Up @@ -4565,7 +4577,7 @@ func TestPassExperimentConfigsToHoldAuction(t *testing.T) {
},
}.Builder

e := NewExchange(adapters, nil, cfg, map[string]usersync.Syncer{}, &metricsConf.NilMetricsEngine{}, biddersInfo, gdprPermsBuilder, currencyConverter, nilCategoryFetcher{}, &signer, macros.NewStringIndexBasedReplacer(), nil).(*exchange)
e := NewExchange(adapters, nil, cfg, &mockRequestValidator{}, map[string]usersync.Syncer{}, &metricsConf.NilMetricsEngine{}, biddersInfo, gdprPermsBuilder, currencyConverter, nilCategoryFetcher{}, &signer, macros.NewStringIndexBasedReplacer(), nil).(*exchange)

// Define mock incoming bid requeset
mockBidRequest := &openrtb2.BidRequest{
Expand Down Expand Up @@ -6332,3 +6344,11 @@ func TestBidsToUpdate(t *testing.T) {
})
}
}

type mockRequestValidator struct {
errors []error
}

func (mrv *mockRequestValidator) ValidateImp(imp *openrtb_ext.ImpWrapper, index int, aliases map[string]string, hasStoredResponses bool, storedBidResponses stored_responses.ImpBidderStoredResp) []error {
return mrv.errors
}
55 changes: 55 additions & 0 deletions exchange/exchangetest/imp-fpd-invalid.json
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
{
"requestType": "openrtb2-web",
"incomingRequest": {
"ortbRequest": {
"id": "some-request-id",
"site": {
"page": "test.somepage.com"
},
"imp": [
{
"id": "imp-id-1",
"banner": {
"format": [
{
"w": 300,
"h": 600
}
]
},
"ext": {
"prebid": {
"bidder": {
"appnexus": {
"placementId": 1
},
"audienceNetwork": {
"placementId": "some-placement"
}
},
"imp": {
"appnexus": {
"id": "imp-id-1-appnexus"
},
"audienceNetwork": {
"banner": {
"format": [
{
"w": -1,
"h": -1
}
]
}
}
}
}
}
}
]
}
},
"outgoingRequests": {},
"response": {
"error": "merging bidder imp first party data for imp imp-id-1 results in an invalid imp: [request.imp[0].banner.format[0].w must be a positive number]"
}
}
Loading

0 comments on commit 74f7ed5

Please sign in to comment.