From bee68c8673251bdfb688e40748bc8d5007f6cbae Mon Sep 17 00:00:00 2001 From: "mike.hoyt" Date: Tue, 20 Jan 2026 19:02:01 -0700 Subject: [PATCH] Update caching/returnCreative logic to work the same as prebid-server-java --- exchange/exchange.go | 25 +++-- exchange/exchange_test.go | 208 +++++++++++++++++++++++++++----------- exchange/utils.go | 19 ++-- exchange/utils_test.go | 181 +++++++++++++++++++++------------ 4 files changed, 287 insertions(+), 146 deletions(-) diff --git a/exchange/exchange.go b/exchange/exchange.go index 8c5d69827..20bac090e 100644 --- a/exchange/exchange.go +++ b/exchange/exchange.go @@ -47,7 +47,9 @@ import ( ) type extCacheInstructions struct { - cacheBids, cacheVAST, returnCreative bool + cacheBids, cacheVAST bool + returnCreativeBids bool + returnCreativeVast bool } // Exchange runs Auctions. Implementations must be threadsafe, and will be shared across many goroutines. @@ -551,7 +553,7 @@ func (e *exchange) HoldAuction(ctx context.Context, r *AuctionRequest, debugLog e.bidValidationEnforcement.SetBannerCreativeMaxSize(r.Account.Validations) // Build the response - bidResponse := e.buildBidResponse(ctx, liveAdapters, adapterBids, r.BidRequestWrapper, adapterExtra, auc, bidResponseExt, cacheInstructions.returnCreative, r.ImpExtInfoMap, r.PubID, errs, &seatNonBidBuilder) + bidResponse := e.buildBidResponse(ctx, liveAdapters, adapterBids, r.BidRequestWrapper, adapterExtra, auc, bidResponseExt, cacheInstructions, r.ImpExtInfoMap, r.PubID, errs, &seatNonBidBuilder) bidResponse = adservertargeting.Apply(r.BidRequestWrapper, r.ResolvedBidRequest, bidResponse, r.QueryParams, bidResponseExt, r.Account.TruncateTargetAttribute) bidResponse.Ext, err = encodeBidResponseExt(bidResponseExt) @@ -993,7 +995,7 @@ func errsToBidderWarnings(errs []error) []openrtb_ext.ExtBidderMessage { } // This piece takes all the bids supplied by the adapters and crafts an openRTB response to send back to the requester -func (e *exchange) buildBidResponse(ctx context.Context, liveAdapters []openrtb_ext.BidderName, adapterSeatBids map[openrtb_ext.BidderName]*entities.PbsOrtbSeatBid, bidRequest *openrtb_ext.RequestWrapper, adapterExtra map[openrtb_ext.BidderName]*seatResponseExtra, auc *auction, bidResponseExt *openrtb_ext.ExtBidResponse, returnCreative bool, impExtInfoMap map[string]ImpExtInfo, pubID string, errList []error, seatNonBidBuilder *SeatNonBidBuilder) *openrtb2.BidResponse { +func (e *exchange) buildBidResponse(ctx context.Context, liveAdapters []openrtb_ext.BidderName, adapterSeatBids map[openrtb_ext.BidderName]*entities.PbsOrtbSeatBid, bidRequest *openrtb_ext.RequestWrapper, adapterExtra map[openrtb_ext.BidderName]*seatResponseExtra, auc *auction, bidResponseExt *openrtb_ext.ExtBidResponse, cacheInstructions extCacheInstructions, impExtInfoMap map[string]ImpExtInfo, pubID string, errList []error, seatNonBidBuilder *SeatNonBidBuilder) *openrtb2.BidResponse { bidResponse := new(openrtb2.BidResponse) bidResponse.ID = bidRequest.ID @@ -1008,7 +1010,7 @@ func (e *exchange) buildBidResponse(ctx context.Context, liveAdapters []openrtb_ for a, adapterSeatBids := range adapterSeatBids { //while processing every single bib, do we need to handle categories here? if adapterSeatBids != nil && len(adapterSeatBids.Bids) > 0 { - sb := e.makeSeatBid(adapterSeatBids, a, adapterExtra, auc, returnCreative, impExtInfoMap, bidRequest, bidResponseExt, pubID, seatNonBidBuilder) + sb := e.makeSeatBid(adapterSeatBids, a, adapterExtra, auc, cacheInstructions, impExtInfoMap, bidRequest, bidResponseExt, pubID, seatNonBidBuilder) seatBids = append(seatBids, *sb) bidResponse.Cur = adapterSeatBids.Currency } @@ -1327,14 +1329,14 @@ func (e *exchange) makeExtBidResponse(adapterBids map[openrtb_ext.BidderName]*en // Return an openrtb seatBid for a bidder // buildBidResponse is responsible for ensuring nil bid seatbids are not included -func (e *exchange) makeSeatBid(adapterBid *entities.PbsOrtbSeatBid, adapter openrtb_ext.BidderName, adapterExtra map[openrtb_ext.BidderName]*seatResponseExtra, auc *auction, returnCreative bool, impExtInfoMap map[string]ImpExtInfo, bidRequest *openrtb_ext.RequestWrapper, bidResponseExt *openrtb_ext.ExtBidResponse, pubID string, seatNonBidBuilder *SeatNonBidBuilder) *openrtb2.SeatBid { +func (e *exchange) makeSeatBid(adapterBid *entities.PbsOrtbSeatBid, adapter openrtb_ext.BidderName, adapterExtra map[openrtb_ext.BidderName]*seatResponseExtra, auc *auction, cacheInstructions extCacheInstructions, impExtInfoMap map[string]ImpExtInfo, bidRequest *openrtb_ext.RequestWrapper, bidResponseExt *openrtb_ext.ExtBidResponse, pubID string, seatNonBidBuilder *SeatNonBidBuilder) *openrtb2.SeatBid { seatBid := &openrtb2.SeatBid{ Seat: adapter.String(), Group: 0, // Prebid cannot support roadblocking } var errList []error - seatBid.Bid, errList = e.makeBid(adapterBid.Bids, auc, returnCreative, impExtInfoMap, bidRequest, bidResponseExt, adapter, pubID, seatNonBidBuilder) + seatBid.Bid, errList = e.makeBid(adapterBid.Bids, auc, cacheInstructions, impExtInfoMap, bidRequest, bidResponseExt, adapter, pubID, seatNonBidBuilder) if len(errList) > 0 { adapterExtra[adapter].Errors = append(adapterExtra[adapter].Errors, errsToBidderErrors(errList)...) } @@ -1342,7 +1344,7 @@ func (e *exchange) makeSeatBid(adapterBid *entities.PbsOrtbSeatBid, adapter open return seatBid } -func (e *exchange) makeBid(bids []*entities.PbsOrtbBid, auc *auction, returnCreative bool, impExtInfoMap map[string]ImpExtInfo, bidRequest *openrtb_ext.RequestWrapper, bidResponseExt *openrtb_ext.ExtBidResponse, adapter openrtb_ext.BidderName, pubID string, seatNonBidBuilder *SeatNonBidBuilder) ([]openrtb2.Bid, []error) { +func (e *exchange) makeBid(bids []*entities.PbsOrtbBid, auc *auction, cacheInstructions extCacheInstructions, impExtInfoMap map[string]ImpExtInfo, bidRequest *openrtb_ext.RequestWrapper, bidResponseExt *openrtb_ext.ExtBidResponse, adapter openrtb_ext.BidderName, pubID string, seatNonBidBuilder *SeatNonBidBuilder) ([]openrtb2.Bid, []error) { result := make([]openrtb2.Bid, 0, len(bids)) errs := make([]error, 0, 1) @@ -1401,8 +1403,13 @@ func (e *exchange) makeBid(bids []*entities.PbsOrtbBid, auc *auction, returnCrea result = append(result, *bid.Bid) resultBid := &result[len(result)-1] resultBid.Ext = bidExtJSON - if !returnCreative { - resultBid.AdM = "" + if auc != nil { + _, hasCacheId := auc.cacheIds[bid.Bid] + _, hasVastCacheId := auc.vastCacheIds[bid.Bid] + if (hasVastCacheId && !cacheInstructions.returnCreativeVast) || + (hasCacheId && !cacheInstructions.returnCreativeBids) { + resultBid.AdM = "" + } } } } diff --git a/exchange/exchange_test.go b/exchange/exchange_test.go index ebba882c5..6c27b64c6 100644 --- a/exchange/exchange_test.go +++ b/exchange/exchange_test.go @@ -172,7 +172,7 @@ func TestCharacterEscape(t *testing.T) { var errList []error // 4) Build bid response - bidResp := e.buildBidResponse(context.Background(), liveAdapters, adapterBids, bidRequest, adapterExtra, nil, nil, true, nil, "", errList, &SeatNonBidBuilder{}) + bidResp := e.buildBidResponse(context.Background(), liveAdapters, adapterBids, bidRequest, adapterExtra, nil, nil, extCacheInstructions{returnCreativeBids: true, returnCreativeVast: true}, nil, "", errList, &SeatNonBidBuilder{}) // 5) Assert we have no errors and one '&' character as we are supposed to if len(errList) > 0 { @@ -1009,6 +1009,8 @@ func TestReturnCreativeEndToEnd(t *testing.T) { inExt json.RawMessage outAdM string } + // Tests require targeting to be enabled so caching actually happens + targetingBase := `"targeting":{"pricegranularity":"med","includewinners":true}` testGroups := []struct { groupDesc string testCases []aTest @@ -1029,12 +1031,12 @@ func TestReturnCreativeEndToEnd(t *testing.T) { }, { "bids doesn't come with returnCreative value", - json.RawMessage(`{"prebid":{"cache":{"bids":{}}}}`), + json.RawMessage(`{"prebid":{` + targetingBase + `,"cache":{"bids":{}}}}`), sampleAd, }, { "vast doesn't come with returnCreative value", - json.RawMessage(`{"prebid":{"cache":{"vastXml":{}}}}`), + json.RawMessage(`{"prebid":{` + targetingBase + `,"cache":{"vastXml":{}}}}`), sampleAd, }, }, @@ -1044,52 +1046,52 @@ func TestReturnCreativeEndToEnd(t *testing.T) { testCases: []aTest{ { "Bids returnCreative set to true, return ad markup in response", - json.RawMessage(`{"prebid":{"cache":{"bids":{"returnCreative":true}}}}`), + json.RawMessage(`{"prebid":{` + targetingBase + `,"cache":{"bids":{"returnCreative":true}}}}`), sampleAd, }, { "Bids returnCreative set to false, don't return ad markup in response", - json.RawMessage(`{"prebid":{"cache":{"bids":{"returnCreative":false}}}}`), + json.RawMessage(`{"prebid":{` + targetingBase + `,"cache":{"bids":{"returnCreative":false}}}}`), "", }, }, }, { - groupDesc: "Vast field comes with returnCreative value", + groupDesc: "Vast field comes with returnCreative value (banner bid uses returnCreativeBids, not returnCreativeVast)", testCases: []aTest{ { - "Vast returnCreative set to true, return ad markup in response", - json.RawMessage(`{"prebid":{"cache":{"vastXml":{"returnCreative":true}}}}`), + "Vast returnCreative set to true, banner bid still returns AdM (uses default returnCreativeBids=true)", + json.RawMessage(`{"prebid":{` + targetingBase + `,"cache":{"vastXml":{"returnCreative":true}}}}`), sampleAd, }, { - "Vast returnCreative set to false, don't return ad markup in response", - json.RawMessage(`{"prebid":{"cache":{"vastXml":{"returnCreative":false}}}}`), - "", + "Vast returnCreative set to false, banner bid still returns AdM (uses default returnCreativeBids=true)", + json.RawMessage(`{"prebid":{` + targetingBase + `,"cache":{"vastXml":{"returnCreative":false}}}}`), + sampleAd, }, }, }, { - groupDesc: "Both Bids and Vast come with their own returnCreative value", + groupDesc: "Both Bids and Vast come with their own returnCreative value (banner bid uses returnCreativeBids)", testCases: []aTest{ { - "Both false, expect empty AdM", - json.RawMessage(`{"prebid":{"cache":{"bids":{"returnCreative":false},"vastXml":{"returnCreative":false}}}}`), + "Both false, banner bid has no AdM (returnCreativeBids=false)", + json.RawMessage(`{"prebid":{` + targetingBase + `,"cache":{"bids":{"returnCreative":false},"vastXml":{"returnCreative":false}}}}`), "", }, { - "Bids returnCreative is true, expect valid AdM", - json.RawMessage(`{"prebid":{"cache":{"bids":{"returnCreative":true},"vastXml":{"returnCreative":false}}}}`), + "Bids returnCreative is true, banner bid has AdM (returnCreativeBids=true)", + json.RawMessage(`{"prebid":{` + targetingBase + `,"cache":{"bids":{"returnCreative":true},"vastXml":{"returnCreative":false}}}}`), sampleAd, }, { - "Vast returnCreative is true, expect valid AdM", - json.RawMessage(`{"prebid":{"cache":{"bids":{"returnCreative":false},"vastXml":{"returnCreative":true}}}}`), - sampleAd, + "Vast returnCreative is true but Bids is false, banner bid has no AdM (returnCreativeBids=false)", + json.RawMessage(`{"prebid":{` + targetingBase + `,"cache":{"bids":{"returnCreative":false},"vastXml":{"returnCreative":true}}}}`), + "", }, { - "Both field's returnCreative set to true, expect valid AdM", - json.RawMessage(`{"prebid":{"cache":{"bids":{"returnCreative":true},"vastXml":{"returnCreative":true}}}}`), + "Both field's returnCreative set to true, banner bid has AdM", + json.RawMessage(`{"prebid":{` + targetingBase + `,"cache":{"bids":{"returnCreative":true},"vastXml":{"returnCreative":true}}}}`), sampleAd, }, }, @@ -1346,7 +1348,7 @@ func TestGetBidCacheInfoEndToEnd(t *testing.T) { var errList []error // 4) Build bid response - bid_resp := e.buildBidResponse(context.Background(), liveAdapters, adapterBids, bidRequest, adapterExtra, auc, nil, true, nil, "", errList, &SeatNonBidBuilder{}) + bid_resp := e.buildBidResponse(context.Background(), liveAdapters, adapterBids, bidRequest, adapterExtra, auc, nil, extCacheInstructions{returnCreativeBids: true, returnCreativeVast: true}, nil, "", errList, &SeatNonBidBuilder{}) expectedBidResponse := &openrtb2.BidResponse{ SeatBid: []openrtb2.SeatBid{ @@ -1381,37 +1383,74 @@ func TestGetBidCacheInfoEndToEnd(t *testing.T) { func TestBidReturnsCreative(t *testing.T) { sampleAd := "" - sampleOpenrtbBid := &openrtb2.Bid{ID: "some-bid-id", AdM: sampleAd} - // Define test cases - testCases := []struct { - description string - inReturnCreative bool - expectedCreativeMarkup string + type aTest struct { + desc string + ci extCacheInstructions + expectBannerAdM bool + expectVideoAdM bool + } + testGroups := []struct { + groupDesc string + bidTypes []openrtb_ext.BidType + testCases []aTest }{ { - "returnCreative set to true, expect a full creative markup string in returned bid", - true, - sampleAd, + groupDesc: "Banner bid only", + bidTypes: []openrtb_ext.BidType{openrtb_ext.BidTypeBanner}, + testCases: []aTest{ + // No caching - always keep + {"no cache", extCacheInstructions{}, true, false}, + // cacheBids only + {"cacheBids=T rcBids=T", extCacheInstructions{cacheBids: true, returnCreativeBids: true}, true, false}, + {"cacheBids=T rcBids=F", extCacheInstructions{cacheBids: true}, false, false}, + // cacheVAST only (doesn't affect banner) + {"cacheVAST=T rcVast=F", extCacheInstructions{cacheVAST: true}, true, false}, + // Both caches (banner only checks cacheBids) + {"both cache, rcBids=T rcVast=T", extCacheInstructions{cacheBids: true, cacheVAST: true, returnCreativeBids: true, returnCreativeVast: true}, true, false}, + {"both cache, rcBids=F rcVast=T", extCacheInstructions{cacheBids: true, cacheVAST: true, returnCreativeVast: true}, false, false}, + }, }, { - "returnCreative set to false, expect empty creative markup string in returned bid", - false, - "", + groupDesc: "Video bid only", + bidTypes: []openrtb_ext.BidType{openrtb_ext.BidTypeVideo}, + testCases: []aTest{ + // No caching - always keep + {"no cache", extCacheInstructions{}, false, true}, + // cacheVAST only + {"cacheVAST=T rcVast=T", extCacheInstructions{cacheVAST: true, returnCreativeVast: true}, false, true}, + {"cacheVAST=T rcVast=F", extCacheInstructions{cacheVAST: true}, false, false}, + // cacheBids only (affects video too in PBS-Java) + {"cacheBids=T rcBids=T", extCacheInstructions{cacheBids: true, returnCreativeBids: true}, false, true}, + {"cacheBids=T rcBids=F", extCacheInstructions{cacheBids: true}, false, false}, + // Both caches - AND logic for video + {"both cache, rcBids=T rcVast=T", extCacheInstructions{cacheBids: true, cacheVAST: true, returnCreativeBids: true, returnCreativeVast: true}, false, true}, + {"both cache, rcBids=T rcVast=F (strip)", extCacheInstructions{cacheBids: true, cacheVAST: true, returnCreativeBids: true}, false, false}, + {"both cache, rcBids=F rcVast=T (strip)", extCacheInstructions{cacheBids: true, cacheVAST: true, returnCreativeVast: true}, false, false}, + {"both cache, both rc=F (strip)", extCacheInstructions{cacheBids: true, cacheVAST: true}, false, false}, + }, + }, + { + groupDesc: "Banner and Video bids together", + bidTypes: []openrtb_ext.BidType{openrtb_ext.BidTypeBanner, openrtb_ext.BidTypeVideo}, + testCases: []aTest{ + // No caching - always keep + {"no cache", extCacheInstructions{}, true, true}, + // cacheBids only + {"cacheBids=T rcBids=T", extCacheInstructions{cacheBids: true, returnCreativeBids: true}, true, true}, + {"cacheBids=T rcBids=F (both strip)", extCacheInstructions{cacheBids: true}, false, false}, + // cacheVAST only + {"cacheVAST=T rcVast=T", extCacheInstructions{cacheVAST: true, returnCreativeVast: true}, true, true}, + {"cacheVAST=T rcVast=F (video strip)", extCacheInstructions{cacheVAST: true}, true, false}, + // Both caches + {"both cache, both rc=T", extCacheInstructions{cacheBids: true, cacheVAST: true, returnCreativeBids: true, returnCreativeVast: true}, true, true}, + {"both cache, rcBids=T rcVast=F (video strip)", extCacheInstructions{cacheBids: true, cacheVAST: true, returnCreativeBids: true}, true, false}, + {"both cache, rcBids=F rcVast=T (banner strip, video strip)", extCacheInstructions{cacheBids: true, cacheVAST: true, returnCreativeVast: true}, false, false}, + {"both cache, both rc=F (both strip)", extCacheInstructions{cacheBids: true, cacheVAST: true}, false, false}, + }, }, } - // Test set up - sampleBids := []*entities.PbsOrtbBid{ - { - Bid: sampleOpenrtbBid, - BidType: openrtb_ext.BidTypeBanner, - BidTargets: map[string]string{}, - GeneratedBidID: "randomId", - }, - } - sampleAuction := &auction{cacheIds: map[*openrtb2.Bid]string{sampleOpenrtbBid: "CACHE_UUID_1234"}} - noBidHandler := func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(204) } server := httptest.NewServer(http.HandlerFunc(noBidHandler)) defer server.Close() @@ -1431,20 +1470,73 @@ func TestBidReturnsCreative(t *testing.T) { } e.cache = &wellBehavedCache{} e.me = &metricsConf.NilMetricsEngine{} - e.currencyConverter = currency.NewRateConverter(&http.Client{}, time.Duration(1), "", time.Duration(0)) - //Run tests - for _, test := range testCases { - resultingBids, resultingErrs := e.makeBid(sampleBids, sampleAuction, test.inReturnCreative, nil, &openrtb_ext.RequestWrapper{}, nil, "", "", &SeatNonBidBuilder{}) + for _, group := range testGroups { + for _, test := range group.testCases { + testName := group.groupDesc + "/" + test.desc + t.Run(testName, func(t *testing.T) { + // Build bids based on group's bidTypes + var sampleBids []*entities.PbsOrtbBid + cacheIds := make(map[*openrtb2.Bid]string) + vastCacheIds := make(map[*openrtb2.Bid]string) + + var bannerBid, videoBid *openrtb2.Bid + for _, bidType := range group.bidTypes { + bid := &openrtb2.Bid{ID: string(bidType) + "-bid", AdM: sampleAd} + if bidType == openrtb_ext.BidTypeBanner && test.ci.cacheBids { + cacheIds[bid] = "CACHE_" + string(bidType) + } + if bidType == openrtb_ext.BidTypeVideo { + if test.ci.cacheBids { + cacheIds[bid] = "CACHE_" + string(bidType) + } + if test.ci.cacheVAST { + vastCacheIds[bid] = "VAST_CACHE_" + string(bidType) + } + } + sampleBids = append(sampleBids, &entities.PbsOrtbBid{ + Bid: bid, + BidType: bidType, + BidTargets: map[string]string{}, + GeneratedBidID: string(bidType) + "-id", + }) + if bidType == openrtb_ext.BidTypeBanner { + bannerBid = bid + } else if bidType == openrtb_ext.BidTypeVideo { + videoBid = bid + } + } + sampleAuction := &auction{cacheIds: cacheIds, vastCacheIds: vastCacheIds} + + resultingBids, resultingErrs := e.makeBid(sampleBids, sampleAuction, test.ci, nil, &openrtb_ext.RequestWrapper{}, nil, "", "", &SeatNonBidBuilder{}) + + assert.Empty(t, resultingErrs, "Unexpected errors") - assert.Equal(t, 0, len(resultingErrs), "%s. Test should not return errors \n", test.description) - assert.Equal(t, test.expectedCreativeMarkup, resultingBids[0].AdM, "%s. Ad markup string doesn't match expected \n", test.description) + for _, resultBid := range resultingBids { + if bannerBid != nil && resultBid.ID == bannerBid.ID { + if test.expectBannerAdM { + assert.Equal(t, sampleAd, resultBid.AdM, "Banner should have AdM") + } else { + assert.Empty(t, resultBid.AdM, "Banner should have AdM stripped") + } + } + if videoBid != nil && resultBid.ID == videoBid.ID { + if test.expectVideoAdM { + assert.Equal(t, sampleAd, resultBid.AdM, "Video should have AdM") + } else { + assert.Empty(t, resultBid.AdM, "Video should have AdM stripped") + } + } - var bidExt openrtb_ext.ExtBid - jsonutil.UnmarshalValid(resultingBids[0].Ext, &bidExt) - assert.Equal(t, 0, bidExt.Prebid.DealPriority, "%s. Test should have DealPriority set to 0", test.description) - assert.Equal(t, false, bidExt.Prebid.DealTierSatisfied, "%s. Test should have DealTierSatisfied set to false", test.description) + // Verify default deal values + var bidExt openrtb_ext.ExtBid + jsonutil.UnmarshalValid(resultBid.Ext, &bidExt) + assert.Equal(t, 0, bidExt.Prebid.DealPriority, "DealPriority should be 0") + assert.Equal(t, false, bidExt.Prebid.DealTierSatisfied, "DealTierSatisfied should be false") + } + }) + } } } @@ -1721,7 +1813,7 @@ func TestBidResponseCurrency(t *testing.T) { } // Run tests for i := range testCases { - actualBidResp := e.buildBidResponse(context.Background(), liveAdapters, testCases[i].adapterBids, bidRequest, adapterExtra, nil, bidResponseExt, true, nil, "", errList, &SeatNonBidBuilder{}) + actualBidResp := e.buildBidResponse(context.Background(), liveAdapters, testCases[i].adapterBids, bidRequest, adapterExtra, nil, bidResponseExt, extCacheInstructions{returnCreativeBids: true, returnCreativeVast: true}, nil, "", errList, &SeatNonBidBuilder{}) assert.Equalf(t, testCases[i].expectedBidResponse, actualBidResp, fmt.Sprintf("[TEST_FAILED] Objects must be equal for test: %s \n Expected: >>%s<< \n Actual: >>%s<< ", testCases[i].description, testCases[i].expectedBidResponse.Ext, actualBidResp.Ext)) } } @@ -1789,7 +1881,7 @@ func TestBidResponseImpExtInfo(t *testing.T) { expectedBidResponseExt := `{"origbidcpm":0,"prebid":{"meta":{"adaptercode":"appnexus"},"type":"video","passthrough":{"imp_passthrough_val":1}},"storedrequestattributes":{"h":480,"mimes":["video/mp4"]}}` - actualBidResp := e.buildBidResponse(context.Background(), liveAdapters, adapterBids, bidRequest, nil, nil, nil, true, impExtInfo, "", errList, &SeatNonBidBuilder{}) + actualBidResp := e.buildBidResponse(context.Background(), liveAdapters, adapterBids, bidRequest, nil, nil, nil, extCacheInstructions{returnCreativeBids: true, returnCreativeVast: true}, impExtInfo, "", errList, &SeatNonBidBuilder{}) resBidExt := string(actualBidResp.SeatBid[0].Bid[0].Ext) assert.Equalf(t, expectedBidResponseExt, resBidExt, "Expected bid response extension is incorrect") @@ -4935,7 +5027,7 @@ func TestMakeBidWithValidation(t *testing.T) { e.bidValidationEnforcement = test.givenValidations sampleBids := test.givenBids nonBids := &SeatNonBidBuilder{} - resultingBids, resultingErrs := e.makeBid(sampleBids, sampleAuction, true, ImpExtInfoMap, bidRequest, bidExtResponse, test.givenSeat, "", nonBids) + resultingBids, resultingErrs := e.makeBid(sampleBids, sampleAuction, extCacheInstructions{returnCreativeBids: true, returnCreativeVast: true}, ImpExtInfoMap, bidRequest, bidExtResponse, test.givenSeat, "", nonBids) assert.Equal(t, 0, len(resultingErrs)) assert.Equal(t, test.expectedNumOfBids, len(resultingBids)) diff --git a/exchange/utils.go b/exchange/utils.go index 08ea0186d..01045ace9 100644 --- a/exchange/utils.go +++ b/exchange/utils.go @@ -895,33 +895,28 @@ func randomizeList(list []openrtb_ext.BidderName) { } func getExtCacheInstructions(requestExtPrebid *openrtb_ext.ExtRequestPrebid) extCacheInstructions { - //returnCreative defaults to true - cacheInstructions := extCacheInstructions{returnCreative: true} - foundBidsRC := false - foundVastRC := false + // returnCreative default to true for both bids and vast + cacheInstructions := extCacheInstructions{ + returnCreativeBids: true, + returnCreativeVast: true, + } if requestExtPrebid != nil && requestExtPrebid.Cache != nil { if requestExtPrebid.Cache.Bids != nil { cacheInstructions.cacheBids = true if requestExtPrebid.Cache.Bids.ReturnCreative != nil { - cacheInstructions.returnCreative = *requestExtPrebid.Cache.Bids.ReturnCreative - foundBidsRC = true + cacheInstructions.returnCreativeBids = *requestExtPrebid.Cache.Bids.ReturnCreative } } if requestExtPrebid.Cache.VastXML != nil { cacheInstructions.cacheVAST = true if requestExtPrebid.Cache.VastXML.ReturnCreative != nil { - cacheInstructions.returnCreative = *requestExtPrebid.Cache.VastXML.ReturnCreative - foundVastRC = true + cacheInstructions.returnCreativeVast = *requestExtPrebid.Cache.VastXML.ReturnCreative } } } - if foundBidsRC && foundVastRC { - cacheInstructions.returnCreative = *requestExtPrebid.Cache.Bids.ReturnCreative || *requestExtPrebid.Cache.VastXML.ReturnCreative - } - return cacheInstructions } diff --git a/exchange/utils_test.go b/exchange/utils_test.go index 1b08ee1a3..5c594208c 100644 --- a/exchange/utils_test.go +++ b/exchange/utils_test.go @@ -1750,27 +1750,29 @@ func TestGetExtCacheInstructions(t *testing.T) { outCacheInstructions extCacheInstructions }{ { - desc: "Nil request ext, all cache flags false except for returnCreative that defaults to true", + desc: "Nil request ext, all cache flags false, returnCreative flags default to true", requestExtPrebid: nil, outCacheInstructions: extCacheInstructions{ - cacheBids: false, - cacheVAST: false, - returnCreative: true, + cacheBids: false, + cacheVAST: false, + returnCreativeBids: true, + returnCreativeVast: true, }, }, { - desc: "Non-nil request ext, nil Cache field, all cache flags false except for returnCreative that defaults to true", + desc: "Non-nil request ext, nil Cache field, all cache flags false, returnCreative flags default to true", requestExtPrebid: &openrtb_ext.ExtRequestPrebid{ Cache: nil, }, outCacheInstructions: extCacheInstructions{ - cacheBids: false, - cacheVAST: false, - returnCreative: true, + cacheBids: false, + cacheVAST: false, + returnCreativeBids: true, + returnCreativeVast: true, }, }, { - desc: "Non-nil Cache field, both ExtRequestPrebidCacheBids and ExtRequestPrebidCacheVAST nil returnCreative that defaults to true", + desc: "Non-nil Cache field, both Bids and VastXML nil, returnCreative flags default to true", requestExtPrebid: &openrtb_ext.ExtRequestPrebid{ Cache: &openrtb_ext.ExtRequestPrebidCache{ Bids: nil, @@ -1778,13 +1780,14 @@ func TestGetExtCacheInstructions(t *testing.T) { }, }, outCacheInstructions: extCacheInstructions{ - cacheBids: false, - cacheVAST: false, - returnCreative: true, + cacheBids: false, + cacheVAST: false, + returnCreativeBids: true, + returnCreativeVast: true, }, }, { - desc: "Non-nil ExtRequest.Cache.ExtRequestPrebidCacheVAST with unspecified ReturnCreative field, cacheVAST = true and returnCreative defaults to true", + desc: "VastXML only with unspecified ReturnCreative, cacheVAST=true, both returnCreative flags default to true", requestExtPrebid: &openrtb_ext.ExtRequestPrebid{ Cache: &openrtb_ext.ExtRequestPrebidCache{ Bids: nil, @@ -1792,13 +1795,14 @@ func TestGetExtCacheInstructions(t *testing.T) { }, }, outCacheInstructions: extCacheInstructions{ - cacheBids: false, - cacheVAST: true, - returnCreative: true, // default value + cacheBids: false, + cacheVAST: true, + returnCreativeBids: true, + returnCreativeVast: true, }, }, { - desc: "Non-nil ExtRequest.Cache.ExtRequestPrebidCacheVAST where ReturnCreative is set to false, cacheVAST = true and returnCreative = false", + desc: "VastXML ReturnCreative=false, only returnCreativeVast=false, returnCreativeBids stays true", requestExtPrebid: &openrtb_ext.ExtRequestPrebid{ Cache: &openrtb_ext.ExtRequestPrebidCache{ Bids: nil, @@ -1806,13 +1810,14 @@ func TestGetExtCacheInstructions(t *testing.T) { }, }, outCacheInstructions: extCacheInstructions{ - cacheBids: false, - cacheVAST: true, - returnCreative: false, + cacheBids: false, + cacheVAST: true, + returnCreativeBids: true, + returnCreativeVast: false, }, }, { - desc: "Non-nil ExtRequest.Cache.ExtRequestPrebidCacheVAST where ReturnCreative is set to true, cacheVAST = true and returnCreative = true", + desc: "VastXML ReturnCreative=true, both returnCreative flags true", requestExtPrebid: &openrtb_ext.ExtRequestPrebid{ Cache: &openrtb_ext.ExtRequestPrebidCache{ Bids: nil, @@ -1820,13 +1825,14 @@ func TestGetExtCacheInstructions(t *testing.T) { }, }, outCacheInstructions: extCacheInstructions{ - cacheBids: false, - cacheVAST: true, - returnCreative: true, + cacheBids: false, + cacheVAST: true, + returnCreativeBids: true, + returnCreativeVast: true, }, }, { - desc: "Non-nil ExtRequest.Cache.ExtRequestPrebidCacheBids with unspecified ReturnCreative field, cacheBids = true and returnCreative defaults to true", + desc: "Bids only with unspecified ReturnCreative, cacheBids=true, both returnCreative flags default to true", requestExtPrebid: &openrtb_ext.ExtRequestPrebid{ Cache: &openrtb_ext.ExtRequestPrebidCache{ Bids: &openrtb_ext.ExtRequestPrebidCacheBids{}, @@ -1834,13 +1840,14 @@ func TestGetExtCacheInstructions(t *testing.T) { }, }, outCacheInstructions: extCacheInstructions{ - cacheBids: true, - cacheVAST: false, - returnCreative: true, // default value + cacheBids: true, + cacheVAST: false, + returnCreativeBids: true, + returnCreativeVast: true, }, }, { - desc: "Non-nil ExtRequest.Cache.ExtRequestPrebidCacheBids where ReturnCreative is set to false, cacheBids = true and returnCreative = false", + desc: "Bids ReturnCreative=false, only returnCreativeBids=false, returnCreativeVast stays true", requestExtPrebid: &openrtb_ext.ExtRequestPrebid{ Cache: &openrtb_ext.ExtRequestPrebidCache{ Bids: &openrtb_ext.ExtRequestPrebidCacheBids{ReturnCreative: boolFalse}, @@ -1848,13 +1855,14 @@ func TestGetExtCacheInstructions(t *testing.T) { }, }, outCacheInstructions: extCacheInstructions{ - cacheBids: true, - cacheVAST: false, - returnCreative: false, + cacheBids: true, + cacheVAST: false, + returnCreativeBids: false, + returnCreativeVast: true, }, }, { - desc: "Non-nil ExtRequest.Cache.ExtRequestPrebidCacheBids where ReturnCreative is set to true, cacheBids = true and returnCreative = true", + desc: "Bids ReturnCreative=true, both returnCreative flags true", requestExtPrebid: &openrtb_ext.ExtRequestPrebid{ Cache: &openrtb_ext.ExtRequestPrebidCache{ Bids: &openrtb_ext.ExtRequestPrebidCacheBids{ReturnCreative: boolTrue}, @@ -1862,13 +1870,14 @@ func TestGetExtCacheInstructions(t *testing.T) { }, }, outCacheInstructions: extCacheInstructions{ - cacheBids: true, - cacheVAST: false, - returnCreative: true, + cacheBids: true, + cacheVAST: false, + returnCreativeBids: true, + returnCreativeVast: true, }, }, { - desc: "Non-nil ExtRequest.Cache.ExtRequestPrebidCacheBids and ExtRequest.Cache.ExtRequestPrebidCacheVAST, neither specify a ReturnCreative field value, all extCacheInstructions fields set to true", + desc: "Both Bids and VastXML with unspecified ReturnCreative, all flags true", requestExtPrebid: &openrtb_ext.ExtRequestPrebid{ Cache: &openrtb_ext.ExtRequestPrebidCache{ Bids: &openrtb_ext.ExtRequestPrebidCacheBids{}, @@ -1876,13 +1885,14 @@ func TestGetExtCacheInstructions(t *testing.T) { }, }, outCacheInstructions: extCacheInstructions{ - cacheBids: true, - cacheVAST: true, - returnCreative: true, + cacheBids: true, + cacheVAST: true, + returnCreativeBids: true, + returnCreativeVast: true, }, }, { - desc: "Non-nil ExtRequest.Cache.ExtRequestPrebidCacheBids and ExtRequest.Cache.ExtRequestPrebidCacheVAST sets ReturnCreative to true, all extCacheInstructions fields set to true", + desc: "Both present, VastXML ReturnCreative=true, both returnCreative flags true", requestExtPrebid: &openrtb_ext.ExtRequestPrebid{ Cache: &openrtb_ext.ExtRequestPrebidCache{ Bids: &openrtb_ext.ExtRequestPrebidCacheBids{}, @@ -1890,13 +1900,14 @@ func TestGetExtCacheInstructions(t *testing.T) { }, }, outCacheInstructions: extCacheInstructions{ - cacheBids: true, - cacheVAST: true, - returnCreative: true, + cacheBids: true, + cacheVAST: true, + returnCreativeBids: true, + returnCreativeVast: true, }, }, { - desc: "Non-nil ExtRequest.Cache.ExtRequestPrebidCacheBids and ExtRequest.Cache.ExtRequestPrebidCacheVAST sets ReturnCreative to false, returnCreative = false", + desc: "Both present, VastXML ReturnCreative=false, returnCreativeVast=false, returnCreativeBids=true", requestExtPrebid: &openrtb_ext.ExtRequestPrebid{ Cache: &openrtb_ext.ExtRequestPrebidCache{ Bids: &openrtb_ext.ExtRequestPrebidCacheBids{}, @@ -1904,13 +1915,14 @@ func TestGetExtCacheInstructions(t *testing.T) { }, }, outCacheInstructions: extCacheInstructions{ - cacheBids: true, - cacheVAST: true, - returnCreative: false, + cacheBids: true, + cacheVAST: true, + returnCreativeBids: true, + returnCreativeVast: false, }, }, { - desc: "Non-nil ExtRequest.Cache.ExtRequestPrebidCacheVAST and ExtRequest.Cache.ExtRequestPrebidCacheBids sets ReturnCreative to true, all extCacheInstructions fields set to true", + desc: "Both present, Bids ReturnCreative=true, both returnCreative flags true", requestExtPrebid: &openrtb_ext.ExtRequestPrebid{ Cache: &openrtb_ext.ExtRequestPrebidCache{ Bids: &openrtb_ext.ExtRequestPrebidCacheBids{ReturnCreative: boolTrue}, @@ -1918,13 +1930,14 @@ func TestGetExtCacheInstructions(t *testing.T) { }, }, outCacheInstructions: extCacheInstructions{ - cacheBids: true, - cacheVAST: true, - returnCreative: true, + cacheBids: true, + cacheVAST: true, + returnCreativeBids: true, + returnCreativeVast: true, }, }, { - desc: "Non-nil ExtRequest.Cache.ExtRequestPrebidCacheVAST and ExtRequest.Cache.ExtRequestPrebidCacheBids sets ReturnCreative to false, returnCreative = false", + desc: "Both present, Bids ReturnCreative=false, returnCreativeBids=false, returnCreativeVast=true", requestExtPrebid: &openrtb_ext.ExtRequestPrebid{ Cache: &openrtb_ext.ExtRequestPrebidCache{ Bids: &openrtb_ext.ExtRequestPrebidCacheBids{ReturnCreative: boolFalse}, @@ -1932,13 +1945,14 @@ func TestGetExtCacheInstructions(t *testing.T) { }, }, outCacheInstructions: extCacheInstructions{ - cacheBids: true, - cacheVAST: true, - returnCreative: false, + cacheBids: true, + cacheVAST: true, + returnCreativeBids: false, + returnCreativeVast: true, }, }, { - desc: "Non-nil ExtRequest.Cache.ExtRequestPrebidCacheVAST and ExtRequest.Cache.ExtRequestPrebidCacheBids set different ReturnCreative values, returnCreative = true because one of them is true", + desc: "Both present, Bids=false VastXML=true, per-type flags are independent", requestExtPrebid: &openrtb_ext.ExtRequestPrebid{ Cache: &openrtb_ext.ExtRequestPrebidCache{ Bids: &openrtb_ext.ExtRequestPrebidCacheBids{ReturnCreative: boolFalse}, @@ -1946,13 +1960,14 @@ func TestGetExtCacheInstructions(t *testing.T) { }, }, outCacheInstructions: extCacheInstructions{ - cacheBids: true, - cacheVAST: true, - returnCreative: true, + cacheBids: true, + cacheVAST: true, + returnCreativeBids: false, + returnCreativeVast: true, }, }, { - desc: "Non-nil ExtRequest.Cache.ExtRequestPrebidCacheVAST and ExtRequest.Cache.ExtRequestPrebidCacheBids set different ReturnCreative values, returnCreative = true because one of them is true", + desc: "Both present, Bids=true VastXML=false, per-type flags are independent", requestExtPrebid: &openrtb_ext.ExtRequestPrebid{ Cache: &openrtb_ext.ExtRequestPrebidCache{ Bids: &openrtb_ext.ExtRequestPrebidCacheBids{ReturnCreative: boolTrue}, @@ -1960,9 +1975,40 @@ func TestGetExtCacheInstructions(t *testing.T) { }, }, outCacheInstructions: extCacheInstructions{ - cacheBids: true, - cacheVAST: true, - returnCreative: true, + cacheBids: true, + cacheVAST: true, + returnCreativeBids: true, + returnCreativeVast: false, + }, + }, + { + desc: "Both present, both ReturnCreative=false", + requestExtPrebid: &openrtb_ext.ExtRequestPrebid{ + Cache: &openrtb_ext.ExtRequestPrebidCache{ + Bids: &openrtb_ext.ExtRequestPrebidCacheBids{ReturnCreative: boolFalse}, + VastXML: &openrtb_ext.ExtRequestPrebidCacheVAST{ReturnCreative: boolFalse}, + }, + }, + outCacheInstructions: extCacheInstructions{ + cacheBids: true, + cacheVAST: true, + returnCreativeBids: false, + returnCreativeVast: false, + }, + }, + { + desc: "Both present, both ReturnCreative=true", + requestExtPrebid: &openrtb_ext.ExtRequestPrebid{ + Cache: &openrtb_ext.ExtRequestPrebidCache{ + Bids: &openrtb_ext.ExtRequestPrebidCacheBids{ReturnCreative: boolTrue}, + VastXML: &openrtb_ext.ExtRequestPrebidCacheVAST{ReturnCreative: boolTrue}, + }, + }, + outCacheInstructions: extCacheInstructions{ + cacheBids: true, + cacheVAST: true, + returnCreativeBids: true, + returnCreativeVast: true, }, }, } @@ -1970,9 +2016,10 @@ func TestGetExtCacheInstructions(t *testing.T) { for _, test := range testCases { cacheInstructions := getExtCacheInstructions(test.requestExtPrebid) - assert.Equal(t, test.outCacheInstructions.cacheBids, cacheInstructions.cacheBids, "%s. Unexpected shouldCacheBids value. \n", test.desc) - assert.Equal(t, test.outCacheInstructions.cacheVAST, cacheInstructions.cacheVAST, "%s. Unexpected shouldCacheVAST value. \n", test.desc) - assert.Equal(t, test.outCacheInstructions.returnCreative, cacheInstructions.returnCreative, "%s. Unexpected returnCreative value. \n", test.desc) + assert.Equal(t, test.outCacheInstructions.cacheBids, cacheInstructions.cacheBids, "%s. Unexpected cacheBids value. \n", test.desc) + assert.Equal(t, test.outCacheInstructions.cacheVAST, cacheInstructions.cacheVAST, "%s. Unexpected cacheVAST value. \n", test.desc) + assert.Equal(t, test.outCacheInstructions.returnCreativeBids, cacheInstructions.returnCreativeBids, "%s. Unexpected returnCreativeBids value. \n", test.desc) + assert.Equal(t, test.outCacheInstructions.returnCreativeVast, cacheInstructions.returnCreativeVast, "%s. Unexpected returnCreativeVast value. \n", test.desc) } }