Cache validation fix (prebid#911)

* Cache validation fix

if no bids returned - don't throw cache error, just return empty result

* Cache validation fix

- optimization: do not run auction logic if no bids returned

* Cache validation fix: minor refactoring

* Cache validation fix: minor refactoring

* Cache validation fix: added unit test for no bids returned

* Cache validation fix: minor refactoring

Author: VeronikaSolovei9

endpoints/openrtb2/video_auction.go

@@ -359,8 +359,10 @@ func minMax(array []int) (int, int) {
 func buildVideoResponse(bidresponse *openrtb.BidResponse, podErrors []PodError) (*openrtb_ext.BidResponseVideo, error) {
 
 	adPods := make([]*openrtb_ext.AdPod, 0)
+	anyBidsReturned := false
 	for _, seatBid := range bidresponse.SeatBid {
 		for _, bid := range seatBid.Bid {
+			anyBidsReturned = true
 
 			var tempRespBidExt openrtb_ext.ExtBid
 			if err := json.Unmarshal(bid.Ext, &tempRespBidExt); err != nil {
@@ -393,14 +395,14 @@ func buildVideoResponse(bidresponse *openrtb.BidResponse, podErrors []PodError)
 		}
 	}
 
-	if len(adPods) == 0 {
+	//check if there are any bids in response.
+	//if there are no bids - empty response should be returned, no cache errors
+	if len(adPods) == 0 && anyBidsReturned {
 		//means there is a global cache error, we need to reject all bids
 		err := errors.New("caching failed for all bids")
 		return nil, err
 	}
 
-	videoResponse := openrtb_ext.BidResponseVideo{}
-
 	// If there were incorrect pods, we put them back to response with error message
 	if len(podErrors) > 0 {
 		for _, podEr := range podErrors {
@@ -412,9 +414,7 @@ func buildVideoResponse(bidresponse *openrtb.BidResponse, podErrors []PodError)
 		}
 	}
 
-	videoResponse.AdPods = adPods
-
-	return &videoResponse, nil
+	return &openrtb_ext.BidResponseVideo{AdPods: adPods}, nil
 }
 
 func findAdPod(podInd int64, pods []*openrtb_ext.AdPod) *openrtb_ext.AdPod {

endpoints/openrtb2/video_auction_test.go

@@ -478,6 +478,15 @@ func TestVideoBuildVideoResponsePodErrors(t *testing.T) {
 	assert.Equal(t, int64(333), bidRespVideo.AdPods[2].PodId, "AdPods should contain error element at index 2")
 }
 
+func TestVideoBuildVideoResponseNoBids(t *testing.T) {
+	openRtbBidResp := openrtb.BidResponse{}
+	podErrors := make([]PodError, 0, 0)
+	openRtbBidResp.SeatBid = make([]openrtb.SeatBid, 0)
+	bidRespVideo, err := buildVideoResponse(&openRtbBidResp, podErrors)
+	assert.NoError(t, err, "Error should be nil")
+	assert.Len(t, bidRespVideo.AdPods, 0, "AdPods length should be 0")
+}
+
 func mockDeps(t *testing.T, ex *mockExchangeVideo) *endpointDeps {
 	theMetrics := pbsmetrics.NewMetrics(metrics.NewRegistry(), openrtb_ext.BidderList())
 	edep := &endpointDeps{

exchange/exchange.go

@@ -138,21 +138,26 @@ func (e *exchange) HoldAuction(ctx context.Context, bidRequest *openrtb.BidReque
 	// Get currency rates conversions for the auction
 	conversions := e.currencyConverter.Rates()
 
-	adapterBids, adapterExtra := e.getAllBids(auctionCtx, cleanRequests, aliases, bidAdjustmentFactors, blabels, conversions)
-	bidCategory, adapterBids, err := applyCategoryMapping(ctx, requestExt, adapterBids, *categoriesFetcher, targData)
-	auc := newAuction(adapterBids, len(bidRequest.Imp))
-	if err != nil {
-		return nil, fmt.Errorf("Error in category mapping : %s", err.Error())
-	}
+	adapterBids, adapterExtra, anyBidsReturned := e.getAllBids(auctionCtx, cleanRequests, aliases, bidAdjustmentFactors, blabels, conversions)
+
+	if anyBidsReturned {
+		bidCategory, adapterBids, err := applyCategoryMapping(ctx, requestExt, adapterBids, *categoriesFetcher, targData)
+		if err != nil {
+			return nil, fmt.Errorf("Error in category mapping : %s", err.Error())
+		}
 
-	if targData != nil && adapterBids != nil {
-		auc.setRoundedPrices(targData.priceGranularity)
-		cacheErrs := auc.doCache(ctx, e.cache, targData, bidRequest, 60, &e.defaultTTLs, bidCategory)
-		if len(cacheErrs) > 0 {
-			errs = append(errs, cacheErrs...)
+		auc := newAuction(adapterBids, len(bidRequest.Imp))
+
+		if targData != nil {
+			auc.setRoundedPrices(targData.priceGranularity)
+			cacheErrs := auc.doCache(ctx, e.cache, targData, bidRequest, 60, &e.defaultTTLs, bidCategory)
+			if len(cacheErrs) > 0 {
+				errs = append(errs, cacheErrs...)
+			}
+			targData.setTargeting(auc, bidRequest.App != nil, bidCategory)
 		}
-		targData.setTargeting(auc, bidRequest.App != nil, bidCategory)
 	}
+
 	// Build the response
 	return e.buildBidResponse(ctx, liveAdapters, adapterBids, bidRequest, resolvedRequest, adapterExtra, errs)
 }
@@ -169,11 +174,12 @@ func (e *exchange) makeAuctionContext(ctx context.Context, needsCache bool) (auc
 }
 
 // This piece sends all the requests to the bidder adapters and gathers the results.
-func (e *exchange) getAllBids(ctx context.Context, cleanRequests map[openrtb_ext.BidderName]*openrtb.BidRequest, aliases map[string]string, bidAdjustments map[string]float64, blabels map[openrtb_ext.BidderName]*pbsmetrics.AdapterLabels, conversions currencies.Conversions) (map[openrtb_ext.BidderName]*pbsOrtbSeatBid, map[openrtb_ext.BidderName]*seatResponseExtra) {
+func (e *exchange) getAllBids(ctx context.Context, cleanRequests map[openrtb_ext.BidderName]*openrtb.BidRequest, aliases map[string]string, bidAdjustments map[string]float64, blabels map[openrtb_ext.BidderName]*pbsmetrics.AdapterLabels, conversions currencies.Conversions) (map[openrtb_ext.BidderName]*pbsOrtbSeatBid, map[openrtb_ext.BidderName]*seatResponseExtra, bool) {
 	// Set up pointers to the bid results
 	adapterBids := make(map[openrtb_ext.BidderName]*pbsOrtbSeatBid, len(cleanRequests))
 	adapterExtra := make(map[openrtb_ext.BidderName]*seatResponseExtra, len(cleanRequests))
 	chBids := make(chan *bidResponseWrapper, len(cleanRequests))
+	bidsFound := false
 
 	for bidderName, req := range cleanRequests {
 		// Here we actually call the adapters and collect the bids.
@@ -228,9 +234,13 @@ func (e *exchange) getAllBids(ctx context.Context, cleanRequests map[openrtb_ext
 		brw := <-chBids
 		adapterBids[brw.bidder] = brw.adapterBids
 		adapterExtra[brw.bidder] = brw.adapterExtra
+
+		if !bidsFound && adapterBids[brw.bidder] != nil && len(adapterBids[brw.bidder].bids) > 0 {
+			bidsFound = true
+		}
 	}
 
-	return adapterBids, adapterExtra
+	return adapterBids, adapterExtra, bidsFound
 }
 
 func (e *exchange) recoverSafely(inner func(openrtb_ext.BidderName, openrtb_ext.BidderName, *openrtb.BidRequest, *pbsmetrics.AdapterLabels, currencies.Conversions), chBids chan *bidResponseWrapper) func(openrtb_ext.BidderName, openrtb_ext.BidderName, *openrtb.BidRequest, *pbsmetrics.AdapterLabels, currencies.Conversions) {