Repository: prebid/prebid-server
Pull request #1250 (2020-04-07 21:19:41)

Moved hb_pb_cat_dur modification to be before caching
This change is to fix a bug related to caching bids that contained deals. Originally, the caching occurred before the `hb_pb_cat_dur` was updated, so the UUID in the cache was still using the price instead of the deal prefix and priority. This change moves the modification so that if the key is modified, then the cache key matches the new `hb_pb_cat_dur`.


Patch:
diff --git a/exchange/exchange.go b/exchange/exchange.go
index 3cab1880..e625e5ca 100644
--- a/exchange/exchange.go
+++ b/exchange/exchange.go
@@ -179,6 +179,12 @@ func (e *exchange) HoldAuction(ctx context.Context, bidRequest *openrtb.BidReque
 					errs = append(errs, errors.New("Unable to marshal response ext for debugging"))
 				}
 			}
+
+			if requestExt.Prebid.SupportDeals {
+				dealErrs := applyDealSupport(bidRequest, auc, bidCategory)
+				errs = append(errs, dealErrs...)
+			}
+
 			cacheErrs := auc.doCache(ctx, e.cache, targData, bidRequest, 60, &e.defaultTTLs, bidCategory, debugLog)
 			if len(cacheErrs) > 0 {
 				errs = append(errs, cacheErrs...)
@@ -186,10 +192,6 @@ func (e *exchange) HoldAuction(ctx context.Context, bidRequest *openrtb.BidReque
 			targData.setTargeting(auc, bidRequest.App != nil, bidCategory)
 		}
 
-		if requestExt.Prebid.SupportDeals {
-			dealErrs := applyDealSupport(bidRequest, auc)
-			errs = append(errs, dealErrs...)
-		}
 	}
 
 	// Build the response
@@ -210,7 +212,7 @@ type BidderDealTier struct {
 }
 
 // applyDealSupport updates targeting keys with deal prefixes if minimum deal tier exceeded
-func applyDealSupport(bidRequest *openrtb.BidRequest, auc *auction) []error {
+func applyDealSupport(bidRequest *openrtb.BidRequest, auc *auction, bidCategory map[string]string) []error {
 	errs := []error{}
 	impDealMap := getDealTiers(bidRequest)
 
@@ -221,7 +223,7 @@ func applyDealSupport(bidRequest *openrtb.BidRequest, auc *auction) []error {
 
 			if topBidPerBidder.dealPriority > 0 {
 				if validateAndNormalizeDealTier(impDeal[bidderString]) {
-					updateHbPbCatDur(topBidPerBidder, impDeal[bidderString].Info)
+					updateHbPbCatDur(topBidPerBidder, impDeal[bidderString].Info, bidCategory)
 				} else {
 					errs = append(errs, fmt.Errorf("dealTier configuration invalid for bidder '%s', imp ID '%s'", bidderString, impID))
 				}
@@ -258,16 +260,16 @@ func validateAndNormalizeDealTier(impDeal *DealTier) bool {
 	return len(impDeal.Info.Prefix) > 0 && impDeal.Info.MinDealTier > 0
 }
 
-func updateHbPbCatDur(bid *pbsOrtbBid, dealTierInfo *DealTierInfo) {
+func updateHbPbCatDur(bid *pbsOrtbBid, dealTierInfo *DealTierInfo, bidCategory map[string]string) {
 	if bid.dealPriority >= dealTierInfo.MinDealTier {
 		prefixTier := fmt.Sprintf("%s%d_", dealTierInfo.Prefix, bid.dealPriority)
 
-		if oldCatDur, ok := bid.bidTargets["hb_pb_cat_dur"]; ok {
+		if oldCatDur, ok := bidCategory[bid.bid.ID]; ok {
 			oldCatDurSplit := strings.SplitAfterN(oldCatDur, "_", 2)
 			oldCatDurSplit[0] = prefixTier
 
 			newCatDur := strings.Join(oldCatDurSplit, "")
-			bid.bidTargets["hb_pb_cat_dur"] = newCatDur
+			bidCategory[bid.bid.ID] = newCatDur
 		}
 	}
 }


Test patch:
diff --git a/exchange/exchange_test.go b/exchange/exchange_test.go
index 2f115ca4..f263eea8 100644
--- a/exchange/exchange_test.go
+++ b/exchange/exchange_test.go
@@ -1403,7 +1403,10 @@ func TestApplyDealSupport(t *testing.T) {
 			},
 		}
 
-		bid := pbsOrtbBid{&openrtb.Bid{}, "video", test.targ, &openrtb_ext.ExtBidPrebidVideo{}, test.dealPriority}
+		bid := pbsOrtbBid{&openrtb.Bid{ID: "123456"}, "video", map[string]string{}, &openrtb_ext.ExtBidPrebidVideo{}, test.dealPriority}
+		bidCategory := map[string]string{
+			bid.bid.ID: test.targ["hb_pb_cat_dur"],
+		}
 
 		auc := &auction{
 			winningBidsByBidder: map[string]map[openrtb_ext.BidderName]*pbsOrtbBid{
@@ -1413,9 +1416,9 @@ func TestApplyDealSupport(t *testing.T) {
 			},
 		}
 
-		dealErrs := applyDealSupport(bidRequest, auc)
+		dealErrs := applyDealSupport(bidRequest, auc, bidCategory)
 
-		assert.Equal(t, test.expectedHbPbCatDur, auc.winningBidsByBidder["imp_id1"][bidderName].bidTargets["hb_pb_cat_dur"], test.description)
+		assert.Equal(t, test.expectedHbPbCatDur, bidCategory[auc.winningBidsByBidder["imp_id1"][bidderName].bid.ID], test.description)
 		if len(test.expectedDealErr) > 0 {
 			assert.Containsf(t, dealErrs, errors.New(test.expectedDealErr), "Expected error message not found in deal errors")
 		}
@@ -1590,11 +1593,14 @@ func TestUpdateHbPbCatDur(t *testing.T) {
 	}
 
 	for _, test := range testCases {
-		bid := pbsOrtbBid{&openrtb.Bid{}, "video", test.targ, &openrtb_ext.ExtBidPrebidVideo{}, test.dealPriority}
+		bid := pbsOrtbBid{&openrtb.Bid{ID: "123456"}, "video", map[string]string{}, &openrtb_ext.ExtBidPrebidVideo{}, test.dealPriority}
+		bidCategory := map[string]string{
+			bid.bid.ID: test.targ["hb_pb_cat_dur"],
+		}
 
-		updateHbPbCatDur(&bid, test.dealTier)
+		updateHbPbCatDur(&bid, test.dealTier, bidCategory)
 
-		assert.Equal(t, test.expectedHbPbCatDur, bid.bidTargets["hb_pb_cat_dur"], test.description)
+		assert.Equal(t, test.expectedHbPbCatDur, bidCategory[bid.bid.ID], test.description)
 	}
 }