From c870c0b62c4d3429f27c8d2b3d10fc4a1986c117 Mon Sep 17 00:00:00 2001 From: VeronikaSolovei9 Date: Mon, 9 Oct 2023 17:13:30 -0700 Subject: [PATCH 1/8] Added support for stored bid responses with case-insensitive bidder name --- endpoints/openrtb2/auction.go | 13 +++---- endpoints/openrtb2/auction_test.go | 33 +++++++++++----- ...ed-bid-resp-case-matching-bidder-name.json | 39 +++++++++++++++++++ ...d-resp-case-not-matching-bidder-name.json} | 2 +- ...red-bid-resp-non-existing-bidder-name.json | 34 ++++++++++++++++ .../imp-with-stored-bid-resp.json | 6 +-- .../req-two-imps-stored-bid-responses.json | 10 ++--- ...with-and-without-stored-bid-responses.json | 4 +- exchange/utils.go | 2 +- exchange/utils_test.go | 23 +++++++++++ stored_responses/stored_responses.go | 11 ------ stored_responses/stored_responses_test.go | 21 ++-------- 12 files changed, 139 insertions(+), 59 deletions(-) create mode 100644 endpoints/openrtb2/sample-requests/valid-whole/supplementary/imp-with-stored-bid-resp-case-matching-bidder-name.json rename endpoints/openrtb2/sample-requests/valid-whole/supplementary/{imp-with-stored-bid-resp-insensitive-bidder-name.json => imp-with-stored-bid-resp-case-not-matching-bidder-name.json} (95%) create mode 100644 endpoints/openrtb2/sample-requests/valid-whole/supplementary/imp-with-stored-bid-resp-non-existing-bidder-name.json diff --git a/endpoints/openrtb2/auction.go b/endpoints/openrtb2/auction.go index 3a929c43abe..d97181560a3 100644 --- a/endpoints/openrtb2/auction.go +++ b/endpoints/openrtb2/auction.go @@ -1525,7 +1525,7 @@ func (deps *endpointDeps) validateImpExt(imp *openrtb_ext.ImpWrapper, aliases ma } if len(storedBidResp) > 0 { - if err := validateStoredBidRespAndImpExtBidders(prebid.Bidder, storedBidResp, imp.ID); err != nil { + if err := deps.validateStoredBidRespAndImpExtBidders(prebid.Bidder, storedBidResp, imp.ID); err != nil { return []error{err} } } @@ -2492,19 +2492,18 @@ func checkIfAppRequest(request []byte) (bool, error) { return false, nil } -func validateStoredBidRespAndImpExtBidders(bidderExts map[string]json.RawMessage, storedBidResp stored_responses.ImpBidderStoredResp, impId string) error { +func (deps *endpointDeps) validateStoredBidRespAndImpExtBidders(bidderExts map[string]json.RawMessage, storedBidResp stored_responses.ImpBidderStoredResp, impId string) error { if bidResponses, ok := storedBidResp[impId]; ok { if len(bidResponses) != len(bidderExts) { return generateStoredBidResponseValidationError(impId) } for bidderName := range bidResponses { - bidder := bidderName - normalizedCoreBidder, ok := openrtb_ext.NormalizeBidderName(bidder) - if ok { - bidder = normalizedCoreBidder.String() + _, bidderNameOk := deps.normalizeBidderName(bidderName) + if !bidderNameOk { + return fmt.Errorf(`unrecognized bidder "%v"`, bidderName) } - if _, present := bidderExts[bidder]; !present { + if _, present := bidderExts[bidderName]; !present { return generateStoredBidResponseValidationError(impId) } } diff --git a/endpoints/openrtb2/auction_test.go b/endpoints/openrtb2/auction_test.go index 8204a80e3db..fe04c336c64 100644 --- a/endpoints/openrtb2/auction_test.go +++ b/endpoints/openrtb2/auction_test.go @@ -4990,8 +4990,8 @@ func TestParseRequestStoredResponses(t *testing.T) { } func TestParseRequestStoredBidResponses(t *testing.T) { - bidRespId1 := json.RawMessage(`{"id": "resp_id1", "seatbid": [{"bid": [{"id": "bid_id1"}], "seat": "testBidder1"}], "bidid": "123", "cur": "USD"}`) - bidRespId2 := json.RawMessage(`{"id": "resp_id2", "seatbid": [{"bid": [{"id": "bid_id2"}], "seat": "testBidder2"}], "bidid": "124", "cur": "USD"}`) + bidRespId1 := json.RawMessage(`{"id": "resp_id1", "seatbid": [{"bid": [{"id": "bid_id1"}], "seat": "telaria"}], "bidid": "123", "cur": "USD"}`) + bidRespId2 := json.RawMessage(`{"id": "resp_id2", "seatbid": [{"bid": [{"id": "bid_id2"}], "seat": "amx"}], "bidid": "124", "cur": "USD"}`) bidRespId3 := json.RawMessage(`{"id": "resp_id3", "seatbid": [{"bid": [{"id": "bid_id3"}], "seat": "APPNEXUS"}], "bidid": "125", "cur": "USD"}`) mockStoredBidResponses := map[string]json.RawMessage{ "bidResponseId1": bidRespId1, @@ -5010,15 +5010,21 @@ func TestParseRequestStoredBidResponses(t *testing.T) { name: "req imp has valid stored bid response", givenRequestBody: validRequest(t, "imp-with-stored-bid-resp.json"), expectedStoredBidResponses: map[string]map[string]json.RawMessage{ - "imp-id1": {"testBidder1": bidRespId1}, + "imp-id1": {"telaria": bidRespId1}, }, expectedErrorCount: 0, }, { - name: "req imp has valid stored bid response with case insensitive bidder name", - givenRequestBody: validRequest(t, "imp-with-stored-bid-resp-insensitive-bidder-name.json"), + name: "req imp has valid stored bid response with case not-matching bidder name", + givenRequestBody: validRequest(t, "imp-with-stored-bid-resp-case-not-matching-bidder-name.json"), + expectedStoredBidResponses: nil, + expectedErrorCount: 1, + }, + { + name: "req imp has valid stored bid response with case matching bidder name", + givenRequestBody: validRequest(t, "imp-with-stored-bid-resp-case-matching-bidder-name.json"), expectedStoredBidResponses: map[string]map[string]json.RawMessage{ - "imp-id3": {"APPNEXUS": bidRespId3}, + "imp-id3": {"appnexus": bidRespId3}, }, expectedErrorCount: 0, }, @@ -5026,8 +5032,8 @@ func TestParseRequestStoredBidResponses(t *testing.T) { name: "req has two imps with valid stored bid responses", givenRequestBody: validRequest(t, "req-two-imps-stored-bid-responses.json"), expectedStoredBidResponses: map[string]map[string]json.RawMessage{ - "imp-id1": {"testBidder1": bidRespId1}, - "imp-id2": {"testBidder2": bidRespId2}, + "imp-id1": {"telaria": bidRespId1}, + "imp-id2": {"amx": bidRespId2}, }, expectedErrorCount: 0, }, @@ -5035,7 +5041,7 @@ func TestParseRequestStoredBidResponses(t *testing.T) { name: "req has two imps one with valid stored bid responses and another one without stored bid responses", givenRequestBody: validRequest(t, "req-two-imps-with-and-without-stored-bid-responses.json"), expectedStoredBidResponses: map[string]map[string]json.RawMessage{ - "imp-id2": {"testBidder2": bidRespId2}, + "imp-id2": {"amx": bidRespId2}, }, expectedErrorCount: 0, }, @@ -5045,6 +5051,12 @@ func TestParseRequestStoredBidResponses(t *testing.T) { expectedStoredBidResponses: nil, expectedErrorCount: 2, }, + { + name: "req imp has valid stored bid response with non existing bidder name", + givenRequestBody: validRequest(t, "imp-with-stored-bid-resp-non-existing-bidder-name.json"), + expectedStoredBidResponses: nil, + expectedErrorCount: 1, + }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { @@ -5062,7 +5074,7 @@ func TestParseRequestStoredBidResponses(t *testing.T) { map[string]string{}, false, []byte{}, - map[string]openrtb_ext.BidderName{"testBidder1": "testBidder1", "testBidder2": "testBidder2", "appnexus": "appnexus"}, + map[string]openrtb_ext.BidderName{"telaria": "telaria", "amx": "amx", "appnexus": "appnexus"}, nil, nil, hardcodedResponseIPValidator{response: true}, @@ -5077,6 +5089,7 @@ func TestParseRequestStoredBidResponses(t *testing.T) { req := httptest.NewRequest("POST", "/openrtb2/auction", strings.NewReader(test.givenRequestBody)) _, _, _, storedBidResponses, _, _, errL := deps.parseRequest(req, &metrics.Labels{}, hookExecutor) if test.expectedErrorCount == 0 { + assert.Empty(t, errL) assert.Equal(t, test.expectedStoredBidResponses, storedBidResponses, "stored responses should match") } else { assert.Contains(t, errL[0].Error(), test.expectedError, "error should match") diff --git a/endpoints/openrtb2/sample-requests/valid-whole/supplementary/imp-with-stored-bid-resp-case-matching-bidder-name.json b/endpoints/openrtb2/sample-requests/valid-whole/supplementary/imp-with-stored-bid-resp-case-matching-bidder-name.json new file mode 100644 index 00000000000..49576023b04 --- /dev/null +++ b/endpoints/openrtb2/sample-requests/valid-whole/supplementary/imp-with-stored-bid-resp-case-matching-bidder-name.json @@ -0,0 +1,39 @@ +{ + "description": "request with impression with stored bid response with case matching bidder name", + "mockBidRequest": { + "id": "request-with-stored-resp", + "site": { + "page": "test.somepage.com" + }, + "imp": [ + { + "id": "imp-id3", + "banner": { + "format": [ + { + "w": 300, + "h": 600 + } + ] + }, + "ext": { + "appnexus": { + "placementId": 12883451 + }, + "prebid": { + "storedbidresponse": [ + { + "bidder": "appnexus", + "id": "bidResponseId3" + } + ] + } + } + } + ], + "user": { + "yob": 1989 + } + }, + "expectedReturnCode": 200 +} \ No newline at end of file diff --git a/endpoints/openrtb2/sample-requests/valid-whole/supplementary/imp-with-stored-bid-resp-insensitive-bidder-name.json b/endpoints/openrtb2/sample-requests/valid-whole/supplementary/imp-with-stored-bid-resp-case-not-matching-bidder-name.json similarity index 95% rename from endpoints/openrtb2/sample-requests/valid-whole/supplementary/imp-with-stored-bid-resp-insensitive-bidder-name.json rename to endpoints/openrtb2/sample-requests/valid-whole/supplementary/imp-with-stored-bid-resp-case-not-matching-bidder-name.json index e7b688d4d83..96a5992b652 100644 --- a/endpoints/openrtb2/sample-requests/valid-whole/supplementary/imp-with-stored-bid-resp-insensitive-bidder-name.json +++ b/endpoints/openrtb2/sample-requests/valid-whole/supplementary/imp-with-stored-bid-resp-case-not-matching-bidder-name.json @@ -1,5 +1,5 @@ { - "description": "request with impression with stored bid response with sensitive bidder name", + "description": "request with impression with stored bid response with case matching bidder name", "mockBidRequest": { "id": "request-with-stored-resp", "site": { diff --git a/endpoints/openrtb2/sample-requests/valid-whole/supplementary/imp-with-stored-bid-resp-non-existing-bidder-name.json b/endpoints/openrtb2/sample-requests/valid-whole/supplementary/imp-with-stored-bid-resp-non-existing-bidder-name.json new file mode 100644 index 00000000000..d8685307e20 --- /dev/null +++ b/endpoints/openrtb2/sample-requests/valid-whole/supplementary/imp-with-stored-bid-resp-non-existing-bidder-name.json @@ -0,0 +1,34 @@ +{ + "description": "request with impression with stored bid response", + "mockBidRequest": { + "id": "request-with-stored-resp", + "site": { + "page": "test.somepage.com" + }, + "imp": [ + { + "id": "imp-id1", + "banner": { + "format": [ + { + "w": 300, + "h": 600 + } + ] + }, + "ext": { + "bidderABC": {}, + "prebid": { + "storedbidresponse": [ + {"bidder":"bidderABC", "id": "bidResponseId1"} + ] + } + } + } + ], + "user": { + "yob": 1989 + } + }, + "expectedReturnCode": 200 +} diff --git a/endpoints/openrtb2/sample-requests/valid-whole/supplementary/imp-with-stored-bid-resp.json b/endpoints/openrtb2/sample-requests/valid-whole/supplementary/imp-with-stored-bid-resp.json index e72cce49355..692a34c4f41 100644 --- a/endpoints/openrtb2/sample-requests/valid-whole/supplementary/imp-with-stored-bid-resp.json +++ b/endpoints/openrtb2/sample-requests/valid-whole/supplementary/imp-with-stored-bid-resp.json @@ -17,12 +17,10 @@ ] }, "ext": { - "appnexus": { - "placementId": 12883451 - }, + "telaria": {}, "prebid": { "storedbidresponse": [ - {"bidder":"testBidder1", "id": "bidResponseId1"} + {"bidder":"telaria", "id": "bidResponseId1"} ] } } diff --git a/endpoints/openrtb2/sample-requests/valid-whole/supplementary/req-two-imps-stored-bid-responses.json b/endpoints/openrtb2/sample-requests/valid-whole/supplementary/req-two-imps-stored-bid-responses.json index 5906eb9149c..09b4bc57746 100644 --- a/endpoints/openrtb2/sample-requests/valid-whole/supplementary/req-two-imps-stored-bid-responses.json +++ b/endpoints/openrtb2/sample-requests/valid-whole/supplementary/req-two-imps-stored-bid-responses.json @@ -17,12 +17,12 @@ ] }, "ext": { - "appnexus": { + "telaria": { "placementId": 12883451 }, "prebid": { "storedbidresponse": [ - {"bidder":"testBidder1", "id": "bidResponseId1"} + {"bidder":"telaria", "id": "bidResponseId1"} ] } } @@ -38,12 +38,10 @@ ] }, "ext": { - "appnexus": { - "placementId": 12883451 - }, + "amx": {}, "prebid": { "storedbidresponse": [ - {"bidder":"testBidder2", "id": "bidResponseId2"} + {"bidder":"amx", "id": "bidResponseId2"} ] } } diff --git a/endpoints/openrtb2/sample-requests/valid-whole/supplementary/req-two-imps-with-and-without-stored-bid-responses.json b/endpoints/openrtb2/sample-requests/valid-whole/supplementary/req-two-imps-with-and-without-stored-bid-responses.json index 6122e4df066..5387a9d61ae 100644 --- a/endpoints/openrtb2/sample-requests/valid-whole/supplementary/req-two-imps-with-and-without-stored-bid-responses.json +++ b/endpoints/openrtb2/sample-requests/valid-whole/supplementary/req-two-imps-with-and-without-stored-bid-responses.json @@ -33,12 +33,12 @@ ] }, "ext": { - "appnexus": { + "amx": { "placementId": 12883451 }, "prebid": { "storedbidresponse": [ - {"bidder":"testBidder2", "id": "bidResponseId2"} + {"bidder":"amx", "id": "bidResponseId2"} ] } } diff --git a/exchange/utils.go b/exchange/utils.go index 0d9c90cda99..e7d96a11f09 100644 --- a/exchange/utils.go +++ b/exchange/utils.go @@ -930,7 +930,7 @@ func buildBidResponseRequest(req *openrtb2.BidRequest, BidderCoreName: resolvedBidder, BidderName: bidderName, BidderStoredResponses: impResps, - ImpReplaceImpId: bidderImpReplaceImpID[string(resolvedBidder)], + ImpReplaceImpId: bidderImpReplaceImpID[string(bidderName)], BidderLabels: metrics.AdapterLabels{Adapter: resolvedBidder}, } } diff --git a/exchange/utils_test.go b/exchange/utils_test.go index 0dbf5b38e5c..b5e40a25851 100644 --- a/exchange/utils_test.go +++ b/exchange/utils_test.go @@ -5,6 +5,7 @@ import ( "encoding/json" "errors" "fmt" + "github.com/prebid/prebid-server/stored_responses" "sort" "testing" @@ -4679,3 +4680,25 @@ func TestApplyBidAdjustmentToFloor(t *testing.T) { }) } } + +func TestBuildBidResponseRequestBidderName(t *testing.T) { + bidderImpResponses := stored_responses.BidderImpsWithBidResponses{ + openrtb_ext.BidderName("appnexus"): {"impId1": json.RawMessage(`{}`), "impId2": json.RawMessage(`{}`)}, + openrtb_ext.BidderName("appneXUS"): {"impId3": json.RawMessage(`{}`), "impId4": json.RawMessage(`{}`)}, + } + + bidderImpReplaceImpID := stored_responses.BidderImpReplaceImpID{ + "appnexus": {"impId1": true, "impId2": false}, + "appneXUS": {"impId3": true, "impId4": false}, + } + result := buildBidResponseRequest(nil, bidderImpResponses, nil, bidderImpReplaceImpID) + + resultAppnexus := result["appnexus"] + assert.Equal(t, resultAppnexus.BidderName, openrtb_ext.BidderName("appnexus")) + assert.Equal(t, resultAppnexus.ImpReplaceImpId, map[string]bool{"impId1": true, "impId2": false}) + + resultAppneXUS := result["appneXUS"] + assert.Equal(t, resultAppneXUS.BidderName, openrtb_ext.BidderName("appneXUS")) + assert.Equal(t, resultAppneXUS.ImpReplaceImpId, map[string]bool{"impId3": true, "impId4": false}) + +} diff --git a/stored_responses/stored_responses.go b/stored_responses/stored_responses.go index 6a5da01deba..611ebd0fdfe 100644 --- a/stored_responses/stored_responses.go +++ b/stored_responses/stored_responses.go @@ -97,17 +97,6 @@ func extractStoredResponsesIds(impInfo []ImpExtPrebidData, if len(bidderResp.ID) == 0 || len(bidderResp.Bidder) == 0 { return nil, nil, nil, nil, fmt.Errorf("request.imp[%d] has ext.prebid.storedbidresponse specified, but \"id\" or/and \"bidder\" fields are missing ", index) } - bidderName := bidderResp.Bidder - normalizedCoreBidder, ok := openrtb_ext.NormalizeBidderName(bidderResp.Bidder) - if ok { - bidderName = normalizedCoreBidder.String() - } - //check if bidder is valid/exists - if _, isValid := bidderMap[bidderName]; !isValid { - return nil, nil, nil, nil, fmt.Errorf("request.imp[impId: %s].ext.prebid.bidder contains unknown bidder: %s. Did you forget an alias in request.ext.prebid.aliases?", impId, bidderResp.Bidder) - } - // bidder is unique per one bid stored response - // if more than one bidder specified the last defined bidder id will take precedence bidderStoredRespId[bidderResp.Bidder] = bidderResp.ID impBiddersWithBidResponseIDs[impId] = bidderStoredRespId diff --git a/stored_responses/stored_responses_test.go b/stored_responses/stored_responses_test.go index c4ddea278a7..c02877649f8 100644 --- a/stored_responses/stored_responses_test.go +++ b/stored_responses/stored_responses_test.go @@ -215,21 +215,6 @@ func TestProcessStoredAuctionAndBidResponsesErrors(t *testing.T) { ]}`), expectedErrorList: []error{errors.New("request.imp[0] has ext.prebid.storedbidresponse specified, but \"id\" or/and \"bidder\" fields are missing ")}, }, - { - description: "Invalid stored bid response: storedbidresponse.bidder not found", - requestJson: []byte(`{"imp": [ - { - "id": "imp-id1", - "ext": { - "prebid": { - "storedbidresponse": [ - { "bidder": "testBidder123", "id": "123abc"}] - } - } - } - ]}`), - expectedErrorList: []error{errors.New("request.imp[impId: imp-id1].ext.prebid.bidder contains unknown bidder: testBidder123. Did you forget an alias in request.ext.prebid.aliases?")}, - }, { description: "Invalid stored auction response format: empty stored Auction Response Id in second imp", requestJson: []byte(`{"imp": [ @@ -285,8 +270,10 @@ func TestProcessStoredAuctionAndBidResponsesErrors(t *testing.T) { } for _, test := range testCases { - _, _, _, errorList := ProcessStoredResponses(nil, test.requestJson, nil, bidderMap) - assert.Equalf(t, test.expectedErrorList, errorList, "Error doesn't match: %s\n", test.description) + t.Run(test.description, func(t *testing.T) { + _, _, _, errorList := ProcessStoredResponses(nil, test.requestJson, nil, bidderMap) + assert.Equalf(t, test.expectedErrorList, errorList, "Error doesn't match: %s\n", test.description) + }) } } From 421e54d4a84ee3ad6df3b2e7b8db5dba614d4299 Mon Sep 17 00:00:00 2001 From: VeronikaSolovei9 Date: Wed, 11 Oct 2023 15:12:24 -0700 Subject: [PATCH 2/8] Fix for stored bid responses --- endpoints/openrtb2/amp_auction.go | 2 +- endpoints/openrtb2/auction.go | 12 +- endpoints/openrtb2/auction_test.go | 12 +- stored_responses/stored_responses.go | 103 ++--- stored_responses/stored_responses_test.go | 515 ++++++++++++---------- 5 files changed, 346 insertions(+), 298 deletions(-) diff --git a/endpoints/openrtb2/amp_auction.go b/endpoints/openrtb2/amp_auction.go index 2fde59c25e4..6863b227cba 100644 --- a/endpoints/openrtb2/amp_auction.go +++ b/endpoints/openrtb2/amp_auction.go @@ -529,7 +529,7 @@ func (deps *endpointDeps) loadRequestJSONForAmp(httpRequest *http.Request) (req return } - storedAuctionResponses, storedBidResponses, bidderImpReplaceImp, errs = stored_responses.ProcessStoredResponses(ctx, requestJSON, deps.storedRespFetcher, deps.bidderMap) + storedAuctionResponses, storedBidResponses, bidderImpReplaceImp, errs = stored_responses.ProcessStoredResponses(ctx, &openrtb_ext.RequestWrapper{BidRequest: req}, deps.storedRespFetcher) if err != nil { errs = []error{err} return diff --git a/endpoints/openrtb2/auction.go b/endpoints/openrtb2/auction.go index d97181560a3..4132ec1ff9d 100644 --- a/endpoints/openrtb2/auction.go +++ b/endpoints/openrtb2/auction.go @@ -516,12 +516,6 @@ func (deps *endpointDeps) parseRequest(httpRequest *http.Request, labels *metric return } - //Stored auction responses should be processed after stored requests due to possible impression modification - storedAuctionResponses, storedBidResponses, bidderImpReplaceImpId, errs = stored_responses.ProcessStoredResponses(ctx, requestJson, deps.storedRespFetcher, deps.bidderMap) - if len(errs) > 0 { - return nil, nil, nil, nil, nil, nil, errs - } - if err := json.Unmarshal(requestJson, req.BidRequest); err != nil { errs = []error{err} return @@ -547,6 +541,12 @@ func (deps *endpointDeps) parseRequest(httpRequest *http.Request, labels *metric lmt.ModifyForIOS(req.BidRequest) + //Stored auction responses should be processed after stored requests due to possible impression modification + storedAuctionResponses, storedBidResponses, bidderImpReplaceImpId, errs = stored_responses.ProcessStoredResponses(ctx, req, deps.storedRespFetcher) + if len(errs) > 0 { + return nil, nil, nil, nil, nil, nil, errs + } + hasStoredResponses := len(storedAuctionResponses) > 0 errL := deps.validateRequest(req, false, hasStoredResponses, storedBidResponses, hasStoredBidRequest) if len(errL) > 0 { diff --git a/endpoints/openrtb2/auction_test.go b/endpoints/openrtb2/auction_test.go index fe04c336c64..7dc52d9be6b 100644 --- a/endpoints/openrtb2/auction_test.go +++ b/endpoints/openrtb2/auction_test.go @@ -5015,10 +5015,12 @@ func TestParseRequestStoredBidResponses(t *testing.T) { expectedErrorCount: 0, }, { - name: "req imp has valid stored bid response with case not-matching bidder name", - givenRequestBody: validRequest(t, "imp-with-stored-bid-resp-case-not-matching-bidder-name.json"), - expectedStoredBidResponses: nil, - expectedErrorCount: 1, + name: "req imp has valid stored bid response with case not-matching bidder name", + givenRequestBody: validRequest(t, "imp-with-stored-bid-resp-case-not-matching-bidder-name.json"), + expectedStoredBidResponses: map[string]map[string]json.RawMessage{ + "imp-id3": {"appnexus": bidRespId3}, + }, + expectedErrorCount: 0, }, { name: "req imp has valid stored bid response with case matching bidder name", @@ -5049,7 +5051,7 @@ func TestParseRequestStoredBidResponses(t *testing.T) { name: "req has two imps with missing stored bid responses", givenRequestBody: validRequest(t, "req-two-imps-missing-stored-bid-response.json"), expectedStoredBidResponses: nil, - expectedErrorCount: 2, + expectedErrorCount: 0, }, { name: "req imp has valid stored bid response with non existing bidder name", diff --git a/stored_responses/stored_responses.go b/stored_responses/stored_responses.go index 611ebd0fdfe..ab1357910d0 100644 --- a/stored_responses/stored_responses.go +++ b/stored_responses/stored_responses.go @@ -4,8 +4,8 @@ import ( "context" "encoding/json" "fmt" + "strings" - "github.com/buger/jsonparser" "github.com/prebid/openrtb/v19/openrtb2" "github.com/prebid/prebid-server/openrtb_ext" "github.com/prebid/prebid-server/stored_requests" @@ -56,8 +56,7 @@ func buildStoredResp(storedBidResponses ImpBidderStoredResp) BidderImpsWithBidRe return bidderToImpToResponses } -func extractStoredResponsesIds(impInfo []ImpExtPrebidData, - bidderMap map[string]openrtb_ext.BidderName) ( +func extractStoredResponsesIds(impInfo []*openrtb_ext.ImpWrapper) ( StoredResponseIDs, ImpBiddersWithBidResponseIDs, ImpsWithAuctionResponseIDs, @@ -75,42 +74,62 @@ func extractStoredResponsesIds(impInfo []ImpExtPrebidData, impBidderReplaceImp := ImpBidderReplaceImpID{} for index, impData := range impInfo { - impId, err := jsonparser.GetString(impData.Imp, "id") + impId := impData.ID + impExt, err := impData.GetImpExt() if err != nil { - return nil, nil, nil, nil, fmt.Errorf("request.imp[%d] missing required field: \"id\"", index) + return nil, nil, nil, nil, err + } + impExtPrebid := impExt.GetPrebid() + if impExtPrebid == nil { + continue } - if impData.ImpExtPrebid.StoredAuctionResponse != nil { - if len(impData.ImpExtPrebid.StoredAuctionResponse.ID) == 0 { + if impExtPrebid.StoredAuctionResponse != nil { + if len(impExtPrebid.StoredAuctionResponse.ID) == 0 { return nil, nil, nil, nil, fmt.Errorf("request.imp[%d] has ext.prebid.storedauctionresponse specified, but \"id\" field is missing ", index) } - allStoredResponseIDs = append(allStoredResponseIDs, impData.ImpExtPrebid.StoredAuctionResponse.ID) + allStoredResponseIDs = append(allStoredResponseIDs, impExtPrebid.StoredAuctionResponse.ID) - impAuctionResponseIDs[impId] = impData.ImpExtPrebid.StoredAuctionResponse.ID + impAuctionResponseIDs[impId] = impExtPrebid.StoredAuctionResponse.ID } - if len(impData.ImpExtPrebid.StoredBidResponse) > 0 { + if len(impExtPrebid.StoredBidResponse) > 0 { + + // bidders can be specified in imp.ext and in imp.ext.prebid.bidders + allBidderNames := make([]string, 0) + for bidderName := range impExtPrebid.Bidder { + allBidderNames = append(allBidderNames, bidderName) + } + for extData := range impExt.GetExt() { + // no bidders will not be processed + allBidderNames = append(allBidderNames, extData) + } bidderStoredRespId := make(map[string]string) bidderReplaceImpId := make(map[string]bool) - for _, bidderResp := range impData.ImpExtPrebid.StoredBidResponse { + for _, bidderResp := range impExtPrebid.StoredBidResponse { if len(bidderResp.ID) == 0 || len(bidderResp.Bidder) == 0 { return nil, nil, nil, nil, fmt.Errorf("request.imp[%d] has ext.prebid.storedbidresponse specified, but \"id\" or/and \"bidder\" fields are missing ", index) } - bidderStoredRespId[bidderResp.Bidder] = bidderResp.ID - impBiddersWithBidResponseIDs[impId] = bidderStoredRespId - - // stored response config can specify if imp id should be replaced with imp id from request - replaceImpId := true - if bidderResp.ReplaceImpId != nil { - // replaceimpid is true if not specified - replaceImpId = *bidderResp.ReplaceImpId - } - bidderReplaceImpId[bidderResp.Bidder] = replaceImpId - impBidderReplaceImp[impId] = bidderReplaceImpId - //storedAuctionResponseIds are not unique, but fetch will return single data for repeated ids - allStoredResponseIDs = append(allStoredResponseIDs, bidderResp.ID) + for _, bidderName := range allBidderNames { + if strings.EqualFold(bidderName, bidderResp.Bidder) { + bidderStoredRespId[bidderName] = bidderResp.ID + impBiddersWithBidResponseIDs[impId] = bidderStoredRespId + + // stored response config can specify if imp id should be replaced with imp id from request + replaceImpId := true + if bidderResp.ReplaceImpId != nil { + // replaceimpid is true if not specified + replaceImpId = *bidderResp.ReplaceImpId + } + bidderReplaceImpId[bidderName] = replaceImpId + impBidderReplaceImp[impId] = bidderReplaceImpId + + //storedAuctionResponseIds are not unique, but fetch will return single data for repeated ids + allStoredResponseIDs = append(allStoredResponseIDs, bidderResp.ID) + } + } } } } @@ -123,14 +142,11 @@ func extractStoredResponsesIds(impInfo []ImpExtPrebidData, // Note that processStoredResponses must be called after processStoredRequests // because stored imps and stored requests can contain stored auction responses and stored bid responses // so the stored requests/imps have to be merged into the incoming request prior to processing stored auction responses. -func ProcessStoredResponses(ctx context.Context, requestJson []byte, storedRespFetcher stored_requests.Fetcher, bidderMap map[string]openrtb_ext.BidderName) (ImpsWithBidResponses, ImpBidderStoredResp, BidderImpReplaceImpID, []error) { - impInfo, errs := parseImpInfo(requestJson) - if len(errs) > 0 { - return nil, nil, nil, errs - } - storedResponsesIds, impBidderToStoredBidResponseId, impIdToRespId, impBidderReplaceImp, err := extractStoredResponsesIds(impInfo, bidderMap) +func ProcessStoredResponses(ctx context.Context, requestWrapper *openrtb_ext.RequestWrapper, storedRespFetcher stored_requests.Fetcher) (ImpsWithBidResponses, ImpBidderStoredResp, BidderImpReplaceImpID, []error) { + + storedResponsesIds, impBidderToStoredBidResponseId, impIdToRespId, impBidderReplaceImp, err := extractStoredResponsesIds(requestWrapper.GetImp()) if err != nil { - return nil, nil, nil, append(errs, err) + return nil, nil, nil, []error{err} } if len(storedResponsesIds) > 0 { @@ -189,28 +205,3 @@ func buildStoredResponsesMaps(storedResponses StoredResponseIdToStoredResponse, } return impIdToStoredResp, impBidderToStoredBidResponse, errs } - -// parseImpInfo parses the request JSON and returns the impressions with their unmarshalled imp.ext.prebid -// copied from exchange to isolate stored responses code from auction dependencies -func parseImpInfo(requestJson []byte) (impData []ImpExtPrebidData, errs []error) { - - if impArray, dataType, _, err := jsonparser.Get(requestJson, "imp"); err == nil && dataType == jsonparser.Array { - _, err = jsonparser.ArrayEach(impArray, func(imp []byte, _ jsonparser.ValueType, _ int, err error) { - impExtData, _, _, err := jsonparser.Get(imp, "ext", "prebid") - var impExtPrebid openrtb_ext.ExtImpPrebid - if impExtData != nil { - if err := json.Unmarshal(impExtData, &impExtPrebid); err != nil { - errs = append(errs, err) - } - } - newImpData := ImpExtPrebidData{imp, impExtPrebid} - impData = append(impData, newImpData) - }) - } - return -} - -type ImpExtPrebidData struct { - Imp json.RawMessage - ImpExtPrebid openrtb_ext.ExtImpPrebid -} diff --git a/stored_responses/stored_responses_test.go b/stored_responses/stored_responses_test.go index c02877649f8..49938f73b88 100644 --- a/stored_responses/stored_responses_test.go +++ b/stored_responses/stored_responses_test.go @@ -163,115 +163,102 @@ func TestBuildStoredBidResponses(t *testing.T) { } func TestProcessStoredAuctionAndBidResponsesErrors(t *testing.T) { - bidderMap := map[string]openrtb_ext.BidderName{"testBidder": "testBidder"} - testCases := []struct { description string - requestJson []byte + request openrtb2.BidRequest expectedErrorList []error }{ { description: "Invalid stored auction response format: empty stored Auction Response Id", - requestJson: []byte(`{"imp": [ - { - "id": "imp-id1", - "ext": { + request: openrtb2.BidRequest{ + Imp: []openrtb2.Imp{ + {ID: "imp-id1", + Ext: json.RawMessage(`{ "prebid": { - "storedauctionresponse": { - } - } - } - } - ]}`), + "storedauctionresponse": {} + }}`)}, + }, + }, expectedErrorList: []error{errors.New("request.imp[0] has ext.prebid.storedauctionresponse specified, but \"id\" field is missing ")}, }, { description: "Invalid stored bid response format: empty storedbidresponse.bidder", - requestJson: []byte(`{"imp": [ - { - "id": "imp-id1", - "ext": { + request: openrtb2.BidRequest{ + Imp: []openrtb2.Imp{ + {ID: "imp-id1", + Ext: json.RawMessage(`{ "prebid": { "storedbidresponse": [ { "id": "123abc"}] - } - } - } - ]}`), + }}`)}, + }, + }, expectedErrorList: []error{errors.New("request.imp[0] has ext.prebid.storedbidresponse specified, but \"id\" or/and \"bidder\" fields are missing ")}, }, { description: "Invalid stored bid response format: empty storedbidresponse.id", - requestJson: []byte(`{"imp": [ - { - "id": "imp-id1", - "ext": { + request: openrtb2.BidRequest{ + Imp: []openrtb2.Imp{ + {ID: "imp-id1", + Ext: json.RawMessage(`{ "prebid": { "storedbidresponse": [ { "bidder": "testbidder"}] - } - } - } - ]}`), + }}`)}, + }, + }, expectedErrorList: []error{errors.New("request.imp[0] has ext.prebid.storedbidresponse specified, but \"id\" or/and \"bidder\" fields are missing ")}, }, { description: "Invalid stored auction response format: empty stored Auction Response Id in second imp", - requestJson: []byte(`{"imp": [ - { - "id": "imp-id1", - "ext": { + request: openrtb2.BidRequest{ + Imp: []openrtb2.Imp{ + {ID: "imp-id1", + Ext: json.RawMessage(`{ "prebid": { "storedauctionresponse": { "id":"123" } - } - } - }, - { - "id": "imp-id2", - "ext": { + }}`)}, + {ID: "imp-id2", + Ext: json.RawMessage(`{ "prebid": { - "storedauctionresponse": { + "storedauctionresponse": { "id":"" } - } - } - } - ]}`), + }}`)}, + }, + }, expectedErrorList: []error{errors.New("request.imp[1] has ext.prebid.storedauctionresponse specified, but \"id\" field is missing ")}, }, { description: "Invalid stored bid response format: empty stored bid Response Id in second imp", - requestJson: []byte(`{"imp": [ - { - "id": "imp-id1", - "ext": { + request: openrtb2.BidRequest{ + Imp: []openrtb2.Imp{ + {ID: "imp-id1", + Ext: json.RawMessage(`{ "prebid": { - "storedbidresponse": [ + "storedbidresponse": [ {"bidder":"testBidder", "id": "123abc"} ] - } - } - }, - { - "id": "imp-id2", - "ext": { + }}`)}, + {ID: "imp-id2", + Ext: json.RawMessage(`{ "prebid": { - "storedbidresponse": [ + "storedbidresponse": [ {"bidder":"testBidder", "id": ""} ] - } - } - } - ]}`), + }}`)}, + }, + }, expectedErrorList: []error{errors.New("request.imp[1] has ext.prebid.storedbidresponse specified, but \"id\" or/and \"bidder\" fields are missing ")}, }, } for _, test := range testCases { t.Run(test.description, func(t *testing.T) { - _, _, _, errorList := ProcessStoredResponses(nil, test.requestJson, nil, bidderMap) + rw := &openrtb_ext.RequestWrapper{BidRequest: &test.request} + _, _, _, errorList := ProcessStoredResponses(nil, rw, nil) assert.Equalf(t, test.expectedErrorList, errorList, "Error doesn't match: %s\n", test.description) }) } @@ -279,7 +266,6 @@ func TestProcessStoredAuctionAndBidResponsesErrors(t *testing.T) { } func TestProcessStoredAuctionAndBidResponses(t *testing.T) { - bidderMap := map[string]openrtb_ext.BidderName{"bidderA": "bidderA", "bidderB": "bidderB"} bidStoredResp1 := json.RawMessage(`[{"bid": [{"id": "bid_id1"],"seat": "bidderA"}]`) bidStoredResp2 := json.RawMessage(`[{"bid": [{"id": "bid_id2"],"seat": "bidderB"}]`) bidStoredResp3 := json.RawMessage(`[{"bid": [{"id": "bid_id3"],"seat": "bidderA"}]`) @@ -292,33 +278,31 @@ func TestProcessStoredAuctionAndBidResponses(t *testing.T) { testCases := []struct { description string - requestJson []byte + request openrtb2.BidRequest expectedStoredAuctionResponses ImpsWithBidResponses expectedStoredBidResponses ImpBidderStoredResp expectedBidderImpReplaceImpID BidderImpReplaceImpID }{ { description: "No stored responses", - requestJson: []byte(`{"imp": [ - { - "id": "imp-id1", - "ext": { - "prebid": { - - } - } - } - ]}`), + request: openrtb2.BidRequest{ + Imp: []openrtb2.Imp{ + {ID: "imp-id1", + Ext: json.RawMessage(`{ + "prebid": {} + }`)}, + }, + }, expectedStoredAuctionResponses: nil, expectedStoredBidResponses: nil, expectedBidderImpReplaceImpID: nil, }, { description: "Stored auction response one imp", - requestJson: []byte(`{"imp": [ - { - "id": "imp-id1", - "ext": { + request: openrtb2.BidRequest{ + Imp: []openrtb2.Imp{ + {ID: "imp-id1", + Ext: json.RawMessage(`{ "appnexus": { "placementId": 123 }, @@ -327,9 +311,9 @@ func TestProcessStoredAuctionAndBidResponses(t *testing.T) { "id": "1" } } - } - } - ]}`), + }`)}, + }, + }, expectedStoredAuctionResponses: ImpsWithBidResponses{ "imp-id1": bidStoredResp1, }, @@ -338,11 +322,11 @@ func TestProcessStoredAuctionAndBidResponses(t *testing.T) { }, { description: "Stored bid response one imp", - requestJson: []byte(`{"imp": [ - { - "id": "imp-id1", - "ext": { - "appnexus": { + request: openrtb2.BidRequest{ + Imp: []openrtb2.Imp{ + {ID: "imp-id1", + Ext: json.RawMessage(`{ + "bidderA": { "placementId": 123 }, "prebid": { @@ -350,9 +334,9 @@ func TestProcessStoredAuctionAndBidResponses(t *testing.T) { {"bidder":"bidderA", "id": "1"} ] } - } - } - ]}`), + }`)}, + }, + }, expectedStoredAuctionResponses: ImpsWithBidResponses{}, expectedStoredBidResponses: ImpBidderStoredResp{ "imp-id1": {"bidderA": bidStoredResp1}, @@ -363,11 +347,14 @@ func TestProcessStoredAuctionAndBidResponses(t *testing.T) { }, { description: "Stored bid responses two bidders one imp", - requestJson: []byte(`{"imp": [ - { - "id": "imp-id1", - "ext": { - "appnexus": { + request: openrtb2.BidRequest{ + Imp: []openrtb2.Imp{ + {ID: "imp-id1", + Ext: json.RawMessage(`{ + "bidderA": { + "placementId": 123 + }, + "bidderB": { "placementId": 123 }, "prebid": { @@ -376,9 +363,9 @@ func TestProcessStoredAuctionAndBidResponses(t *testing.T) { {"bidder":"bidderB", "id": "2", "replaceimpid": false} ] } - } - } - ]}`), + }`)}, + }, + }, expectedStoredAuctionResponses: ImpsWithBidResponses{}, expectedStoredBidResponses: ImpBidderStoredResp{ "imp-id1": {"bidderA": bidStoredResp1, "bidderB": bidStoredResp2}, @@ -388,14 +375,83 @@ func TestProcessStoredAuctionAndBidResponses(t *testing.T) { "bidderB": map[string]bool{"imp-id1": false}, }, }, + { + description: "Stored bid responses two same mixed case bidders one imp", + request: openrtb2.BidRequest{ + Imp: []openrtb2.Imp{ + {ID: "imp-id1", + Ext: json.RawMessage(`{ + "bidderA": { + "placementId": 123 + }, + "BIDDERa": { + "placementId": 123 + }, + "prebid": { + "storedbidresponse": [ + {"bidder":"bidderA", "id": "1", "replaceimpid": true}, + {"bidder":"bidderB", "id": "2", "replaceimpid": false} + ] + } + }`)}, + }, + }, + expectedStoredAuctionResponses: ImpsWithBidResponses{}, + expectedStoredBidResponses: ImpBidderStoredResp{ + "imp-id1": {"bidderA": bidStoredResp1, "BIDDERa": bidStoredResp1}, + }, + expectedBidderImpReplaceImpID: BidderImpReplaceImpID{ + "BIDDERa": map[string]bool{"imp-id1": true}, + "bidderA": map[string]bool{"imp-id1": true}, + }, + }, + { + description: "Stored bid responses 3 same mixed case bidders in imp.ext and imp.ext.prebid.bidders one imp", + request: openrtb2.BidRequest{ + Imp: []openrtb2.Imp{ + {ID: "imp-id1", + Ext: json.RawMessage(`{ + "bidderA": { + "placementId": 123 + }, + "BIDDERa": { + "placementId": 123 + }, + "prebid": { + "bidder": { + "BiddeRa": { + "placementId": 12883451 + } + }, + "storedbidresponse": [ + {"bidder":"bidderA", "id": "1", "replaceimpid": true}, + {"bidder":"bidderB", "id": "2", "replaceimpid": false} + ] + } + }`)}, + }, + }, + expectedStoredAuctionResponses: ImpsWithBidResponses{}, + expectedStoredBidResponses: ImpBidderStoredResp{ + "imp-id1": {"bidderA": bidStoredResp1, "BIDDERa": bidStoredResp1, "BiddeRa": bidStoredResp1}, + }, + expectedBidderImpReplaceImpID: BidderImpReplaceImpID{ + "BIDDERa": map[string]bool{"imp-id1": true}, + "bidderA": map[string]bool{"imp-id1": true}, + "BiddeRa": map[string]bool{"imp-id1": true}, + }, + }, { //This is not a valid scenario for real auction request, added for testing purposes description: "Stored auction and bid responses one imp", - requestJson: []byte(`{"imp": [ - { - "id": "imp-id1", - "ext": { - "appnexus": { + request: openrtb2.BidRequest{ + Imp: []openrtb2.Imp{ + {ID: "imp-id1", + Ext: json.RawMessage(`{ + "bidderA": { + "placementId": 123 + }, + "bidderB": { "placementId": 123 }, "prebid": { @@ -407,9 +463,9 @@ func TestProcessStoredAuctionAndBidResponses(t *testing.T) { {"bidder":"bidderB", "id": "2"} ] } - } - } - ]}`), + }`)}, + }, + }, expectedStoredAuctionResponses: ImpsWithBidResponses{ "imp-id1": bidStoredResp1, }, @@ -423,10 +479,10 @@ func TestProcessStoredAuctionAndBidResponses(t *testing.T) { }, { description: "Stored auction response three imps", - requestJson: []byte(`{"imp": [ - { - "id": "imp-id1", - "ext": { + request: openrtb2.BidRequest{ + Imp: []openrtb2.Imp{ + {ID: "imp-id1", + Ext: json.RawMessage(`{ "appnexus": { "placementId": 123 }, @@ -435,11 +491,9 @@ func TestProcessStoredAuctionAndBidResponses(t *testing.T) { "id": "1" } } - } - }, - { - "id": "imp-id2", - "ext": { + }`)}, + {ID: "imp-id2", + Ext: json.RawMessage(`{ "appnexus": { "placementId": 123 }, @@ -448,11 +502,10 @@ func TestProcessStoredAuctionAndBidResponses(t *testing.T) { "id": "2" } } - } - }, + }`)}, { - "id": "imp-id3", - "ext": { + ID: "imp-id3", + Ext: json.RawMessage(`{ "appnexus": { "placementId": 123 }, @@ -461,9 +514,10 @@ func TestProcessStoredAuctionAndBidResponses(t *testing.T) { "id": "3" } } - } - } - ]}`), + }`), + }, + }, + }, expectedStoredAuctionResponses: ImpsWithBidResponses{ "imp-id1": bidStoredResp1, "imp-id2": bidStoredResp2, @@ -474,10 +528,10 @@ func TestProcessStoredAuctionAndBidResponses(t *testing.T) { }, { description: "Stored auction response three imps duplicated stored auction response", - requestJson: []byte(`{"imp": [ - { - "id": "imp-id1", - "ext": { + request: openrtb2.BidRequest{ + Imp: []openrtb2.Imp{ + {ID: "imp-id1", + Ext: json.RawMessage(`{ "appnexus": { "placementId": 123 }, @@ -486,11 +540,9 @@ func TestProcessStoredAuctionAndBidResponses(t *testing.T) { "id": "1" } } - } - }, - { - "id": "imp-id2", - "ext": { + }`)}, + {ID: "imp-id2", + Ext: json.RawMessage(`{ "appnexus": { "placementId": 123 }, @@ -499,11 +551,10 @@ func TestProcessStoredAuctionAndBidResponses(t *testing.T) { "id": "2" } } - } - }, + }`)}, { - "id": "imp-id3", - "ext": { + ID: "imp-id3", + Ext: json.RawMessage(`{ "appnexus": { "placementId": 123 }, @@ -512,9 +563,10 @@ func TestProcessStoredAuctionAndBidResponses(t *testing.T) { "id": "2" } } - } - } - ]}`), + }`), + }, + }, + }, expectedStoredAuctionResponses: ImpsWithBidResponses{ "imp-id1": bidStoredResp1, "imp-id2": bidStoredResp2, @@ -525,11 +577,14 @@ func TestProcessStoredAuctionAndBidResponses(t *testing.T) { }, { description: "Stored bid responses two bidders two imp", - requestJson: []byte(`{"imp": [ - { - "id": "imp-id1", - "ext": { - "appnexus": { + request: openrtb2.BidRequest{ + Imp: []openrtb2.Imp{ + {ID: "imp-id1", + Ext: json.RawMessage(`{ + "bidderA": { + "placementId": 123 + }, + "bidderB": { "placementId": 123 }, "prebid": { @@ -538,12 +593,13 @@ func TestProcessStoredAuctionAndBidResponses(t *testing.T) { {"bidder":"bidderB", "id": "2"} ] } - } - }, - { - "id": "imp-id2", - "ext": { - "appnexus": { + }`)}, + {ID: "imp-id2", + Ext: json.RawMessage(`{ + "bidderA": { + "placementId": 123 + }, + "bidderB": { "placementId": 123 }, "prebid": { @@ -552,9 +608,9 @@ func TestProcessStoredAuctionAndBidResponses(t *testing.T) { {"bidder":"bidderB", "id": "2", "replaceimpid": false} ] } - } - } - ]}`), + }`)}, + }, + }, expectedStoredAuctionResponses: ImpsWithBidResponses{}, expectedStoredBidResponses: ImpBidderStoredResp{ "imp-id1": {"bidderA": bidStoredResp1, "bidderB": bidStoredResp2}, @@ -568,17 +624,19 @@ func TestProcessStoredAuctionAndBidResponses(t *testing.T) { } for _, test := range testCases { - storedAuctionResponses, storedBidResponses, bidderImpReplaceImpId, errorList := ProcessStoredResponses(nil, test.requestJson, fetcher, bidderMap) - assert.Equal(t, test.expectedStoredAuctionResponses, storedAuctionResponses, "storedAuctionResponses doesn't match: %s\n", test.description) - assert.Equalf(t, test.expectedStoredBidResponses, storedBidResponses, "storedBidResponses doesn't match: %s\n", test.description) - assert.Equal(t, test.expectedBidderImpReplaceImpID, bidderImpReplaceImpId, "bidderImpReplaceImpId doesn't match: %s\n", test.description) - assert.Nil(t, errorList, "Error should be nil") + t.Run(test.description, func(t *testing.T) { + rw := openrtb_ext.RequestWrapper{BidRequest: &test.request} + storedAuctionResponses, storedBidResponses, bidderImpReplaceImpId, errorList := ProcessStoredResponses(nil, &rw, fetcher) + assert.Equal(t, test.expectedStoredAuctionResponses, storedAuctionResponses, "storedAuctionResponses doesn't match: %s\n", test.description) + assert.Equalf(t, test.expectedStoredBidResponses, storedBidResponses, "storedBidResponses doesn't match: %s\n", test.description) + assert.Equal(t, test.expectedBidderImpReplaceImpID, bidderImpReplaceImpId, "bidderImpReplaceImpId doesn't match: %s\n", test.description) + assert.Nil(t, errorList, "Error should be nil") + }) } } func TestProcessStoredResponsesNotFoundResponse(t *testing.T) { - bidderMap := map[string]openrtb_ext.BidderName{"bidderA": "bidderA", "bidderB": "bidderB"} bidStoredResp1 := json.RawMessage(`[{"bid": [{"id": "bid_id1"],"seat": "bidderA"}]`) bidStoredResp2 := json.RawMessage(`[{"bid": [{"id": "bid_id2"],"seat": "bidderB"}]`) mockStoredResponses := map[string]json.RawMessage{ @@ -591,16 +649,16 @@ func TestProcessStoredResponsesNotFoundResponse(t *testing.T) { testCases := []struct { description string - requestJson []byte + request openrtb2.BidRequest expectedErrors []error }{ { description: "Stored bid response with nil data, one bidder one imp", - requestJson: []byte(`{"imp": [ - { - "id": "imp-id1", - "ext": { - "appnexus": { + request: openrtb2.BidRequest{ + Imp: []openrtb2.Imp{ + {ID: "imp-id1", + Ext: json.RawMessage(`{ + "bidderB": { "placementId": 123 }, "prebid": { @@ -608,20 +666,20 @@ func TestProcessStoredResponsesNotFoundResponse(t *testing.T) { {"bidder":"bidderB", "id": "3"} ] } - } - } - ]}`), + }`)}, + }, + }, expectedErrors: []error{ errors.New("failed to fetch stored bid response for impId = imp-id1, bidder = bidderB and storedBidResponse id = 3"), }, }, { description: "Stored bid response with nil data, one bidder, two imps, one with correct stored response", - requestJson: []byte(`{"imp": [ - { - "id": "imp-id1", - "ext": { - "appnexus": { + request: openrtb2.BidRequest{ + Imp: []openrtb2.Imp{ + {ID: "imp-id1", + Ext: json.RawMessage(`{ + "bidderB": { "placementId": 123 }, "prebid": { @@ -629,12 +687,10 @@ func TestProcessStoredResponsesNotFoundResponse(t *testing.T) { {"bidder":"bidderB", "id": "1"} ] } - } - }, - { - "id": "imp-id2", - "ext": { - "appnexus": { + }`)}, + {ID: "imp-id2", + Ext: json.RawMessage(`{ + "bidderB": { "placementId": 123 }, "prebid": { @@ -642,20 +698,20 @@ func TestProcessStoredResponsesNotFoundResponse(t *testing.T) { {"bidder":"bidderB", "id": "3"} ] } - } - } - ]}`), + }`)}, + }, + }, expectedErrors: []error{ errors.New("failed to fetch stored bid response for impId = imp-id2, bidder = bidderB and storedBidResponse id = 3"), }, }, { description: "Stored bid response with nil data, one bidder, two imps, both with correct stored response", - requestJson: []byte(`{"imp": [ - { - "id": "imp-id1", - "ext": { - "appnexus": { + request: openrtb2.BidRequest{ + Imp: []openrtb2.Imp{ + {ID: "imp-id1", + Ext: json.RawMessage(`{ + "bidderB": { "placementId": 123 }, "prebid": { @@ -663,12 +719,10 @@ func TestProcessStoredResponsesNotFoundResponse(t *testing.T) { {"bidder":"bidderB", "id": "4"} ] } - } - }, - { - "id": "imp-id2", - "ext": { - "appnexus": { + }`)}, + {ID: "imp-id2", + Ext: json.RawMessage(`{ + "bidderB": { "placementId": 123 }, "prebid": { @@ -676,9 +730,9 @@ func TestProcessStoredResponsesNotFoundResponse(t *testing.T) { {"bidder":"bidderB", "id": "3"} ] } - } - } - ]}`), + }`)}, + }, + }, expectedErrors: []error{ errors.New("failed to fetch stored bid response for impId = imp-id1, bidder = bidderB and storedBidResponse id = 4"), errors.New("failed to fetch stored bid response for impId = imp-id2, bidder = bidderB and storedBidResponse id = 3"), @@ -686,10 +740,10 @@ func TestProcessStoredResponsesNotFoundResponse(t *testing.T) { }, { description: "Stored auction response with nil data and one imp", - requestJson: []byte(`{"imp": [ - { - "id": "imp-id1", - "ext": { + request: openrtb2.BidRequest{ + Imp: []openrtb2.Imp{ + {ID: "imp-id1", + Ext: json.RawMessage(`{ "appnexus": { "placementId": 123 }, @@ -698,19 +752,19 @@ func TestProcessStoredResponsesNotFoundResponse(t *testing.T) { "id": "4" } } - } - } - ]}`), + }`)}, + }, + }, expectedErrors: []error{ errors.New("failed to fetch stored auction response for impId = imp-id1 and storedAuctionResponse id = 4"), }, }, { description: "Stored auction response with nil data, and two imps with nil responses", - requestJson: []byte(`{"imp": [ - { - "id": "imp-id1", - "ext": { + request: openrtb2.BidRequest{ + Imp: []openrtb2.Imp{ + {ID: "imp-id1", + Ext: json.RawMessage(`{ "appnexus": { "placementId": 123 }, @@ -719,11 +773,9 @@ func TestProcessStoredResponsesNotFoundResponse(t *testing.T) { "id": "4" } } - } - }, - { - "id": "imp-id2", - "ext": { + }`)}, + {ID: "imp-id2", + Ext: json.RawMessage(`{ "appnexus": { "placementId": 123 }, @@ -732,9 +784,9 @@ func TestProcessStoredResponsesNotFoundResponse(t *testing.T) { "id": "3" } } - } - } - ]}`), + }`)}, + }, + }, expectedErrors: []error{ errors.New("failed to fetch stored auction response for impId = imp-id1 and storedAuctionResponse id = 4"), errors.New("failed to fetch stored auction response for impId = imp-id2 and storedAuctionResponse id = 3"), @@ -742,10 +794,10 @@ func TestProcessStoredResponsesNotFoundResponse(t *testing.T) { }, { description: "Stored auction response with nil data, two imps, one with nil responses", - requestJson: []byte(`{"imp": [ - { - "id": "imp-id1", - "ext": { + request: openrtb2.BidRequest{ + Imp: []openrtb2.Imp{ + {ID: "imp-id1", + Ext: json.RawMessage(`{ "appnexus": { "placementId": 123 }, @@ -754,11 +806,9 @@ func TestProcessStoredResponsesNotFoundResponse(t *testing.T) { "id": "2" } } - } - }, - { - "id": "imp-id2", - "ext": { + }`)}, + {ID: "imp-id2", + Ext: json.RawMessage(`{ "appnexus": { "placementId": 123 }, @@ -767,9 +817,9 @@ func TestProcessStoredResponsesNotFoundResponse(t *testing.T) { "id": "3" } } - } - } - ]}`), + }`)}, + }, + }, expectedErrors: []error{ errors.New("failed to fetch stored auction response for impId = imp-id2 and storedAuctionResponse id = 3"), }, @@ -777,10 +827,13 @@ func TestProcessStoredResponsesNotFoundResponse(t *testing.T) { } for _, test := range testCases { - _, _, _, errorList := ProcessStoredResponses(nil, test.requestJson, fetcher, bidderMap) - for _, err := range test.expectedErrors { - assert.Contains(t, errorList, err, "incorrect errors returned: %s", test.description) - } + t.Run(test.description, func(t *testing.T) { + rw := openrtb_ext.RequestWrapper{BidRequest: &test.request} + _, _, _, errorList := ProcessStoredResponses(nil, &rw, fetcher) + for _, err := range test.expectedErrors { + assert.Contains(t, errorList, err, "incorrect errors returned: %s", test.description) + } + }) } } @@ -834,8 +887,10 @@ func TestFlipMap(t *testing.T) { } for _, test := range testCases { - actualResult := flipMap(test.inImpBidderReplaceImpID) - assert.Equal(t, test.outBidderImpReplaceImpID, actualResult, "Incorrect flipped map for test case %s\n", test.description) + t.Run(test.description, func(t *testing.T) { + actualResult := flipMap(test.inImpBidderReplaceImpID) + assert.Equal(t, test.outBidderImpReplaceImpID, actualResult, "Incorrect flipped map for test case %s\n", test.description) + }) } } From 3bc4b76b98b283d11acd47ab75a10fa32d62a92e Mon Sep 17 00:00:00 2001 From: VeronikaSolovei9 Date: Wed, 11 Oct 2023 15:42:58 -0700 Subject: [PATCH 3/8] Minor refactoring --- endpoints/openrtb2/auction.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/endpoints/openrtb2/auction.go b/endpoints/openrtb2/auction.go index 4132ec1ff9d..5759a8ab5c2 100644 --- a/endpoints/openrtb2/auction.go +++ b/endpoints/openrtb2/auction.go @@ -2499,8 +2499,7 @@ func (deps *endpointDeps) validateStoredBidRespAndImpExtBidders(bidderExts map[s } for bidderName := range bidResponses { - _, bidderNameOk := deps.normalizeBidderName(bidderName) - if !bidderNameOk { + if _, bidderNameOk := deps.normalizeBidderName(bidderName); !bidderNameOk { return fmt.Errorf(`unrecognized bidder "%v"`, bidderName) } if _, present := bidderExts[bidderName]; !present { From 3ecaeef764f407de1e933cee7e20cb06c632fa72 Mon Sep 17 00:00:00 2001 From: VeronikaSolovei9 Date: Tue, 17 Oct 2023 13:57:38 -0700 Subject: [PATCH 4/8] Added logic to ensure that the first stored response is used instead of the last if there are multiple stored responses for the same bidder --- stored_responses/stored_responses.go | 2 +- stored_responses/stored_responses_test.go | 37 +++++++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/stored_responses/stored_responses.go b/stored_responses/stored_responses.go index ab1357910d0..d6944a6f30f 100644 --- a/stored_responses/stored_responses.go +++ b/stored_responses/stored_responses.go @@ -113,7 +113,7 @@ func extractStoredResponsesIds(impInfo []*openrtb_ext.ImpWrapper) ( } for _, bidderName := range allBidderNames { - if strings.EqualFold(bidderName, bidderResp.Bidder) { + if _, found := bidderStoredRespId[bidderName]; !found && strings.EqualFold(bidderName, bidderResp.Bidder) { bidderStoredRespId[bidderName] = bidderResp.ID impBiddersWithBidResponseIDs[impId] = bidderStoredRespId diff --git a/stored_responses/stored_responses_test.go b/stored_responses/stored_responses_test.go index 49938f73b88..9578434d787 100644 --- a/stored_responses/stored_responses_test.go +++ b/stored_responses/stored_responses_test.go @@ -441,6 +441,43 @@ func TestProcessStoredAuctionAndBidResponses(t *testing.T) { "BiddeRa": map[string]bool{"imp-id1": true}, }, }, + { + description: "Stored bid responses 3 same mixed case bidders in imp.ext and imp.ext.prebid.bidders one imp, duplicated stored response", + request: openrtb2.BidRequest{ + Imp: []openrtb2.Imp{ + {ID: "imp-id1", + Ext: json.RawMessage(`{ + "bidderA": { + "placementId": 123 + }, + "BIDDERa": { + "placementId": 123 + }, + "prebid": { + "bidder": { + "BiddeRa": { + "placementId": 12883451 + } + }, + "storedbidresponse": [ + {"bidder":"bidderA", "id": "1", "replaceimpid": true}, + {"bidder":"bidderA", "id": "2", "replaceimpid": true}, + {"bidder":"bidderB", "id": "2", "replaceimpid": false} + ] + } + }`)}, + }, + }, + expectedStoredAuctionResponses: ImpsWithBidResponses{}, + expectedStoredBidResponses: ImpBidderStoredResp{ + "imp-id1": {"bidderA": bidStoredResp1, "BIDDERa": bidStoredResp1, "BiddeRa": bidStoredResp1}, + }, + expectedBidderImpReplaceImpID: BidderImpReplaceImpID{ + "BIDDERa": map[string]bool{"imp-id1": true}, + "bidderA": map[string]bool{"imp-id1": true}, + "BiddeRa": map[string]bool{"imp-id1": true}, + }, + }, { //This is not a valid scenario for real auction request, added for testing purposes description: "Stored auction and bid responses one imp", From 33187ccb49f4122302ff18a9cd54088628a2f379 Mon Sep 17 00:00:00 2001 From: VeronikaSolovei9 Date: Wed, 18 Oct 2023 13:02:46 -0700 Subject: [PATCH 5/8] Minor clean up --- ...-stored-bid-resp-case-not-matching-bidder-name.json | 2 +- stored_responses/stored_responses_test.go | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/endpoints/openrtb2/sample-requests/valid-whole/supplementary/imp-with-stored-bid-resp-case-not-matching-bidder-name.json b/endpoints/openrtb2/sample-requests/valid-whole/supplementary/imp-with-stored-bid-resp-case-not-matching-bidder-name.json index 96a5992b652..bcebe81472d 100644 --- a/endpoints/openrtb2/sample-requests/valid-whole/supplementary/imp-with-stored-bid-resp-case-not-matching-bidder-name.json +++ b/endpoints/openrtb2/sample-requests/valid-whole/supplementary/imp-with-stored-bid-resp-case-not-matching-bidder-name.json @@ -1,5 +1,5 @@ { - "description": "request with impression with stored bid response with case matching bidder name", + "description": "request with impression with stored bid response with case not matching bidder name", "mockBidRequest": { "id": "request-with-stored-resp", "site": { diff --git a/stored_responses/stored_responses_test.go b/stored_responses/stored_responses_test.go index 9578434d787..209b34a650b 100644 --- a/stored_responses/stored_responses_test.go +++ b/stored_responses/stored_responses_test.go @@ -664,9 +664,9 @@ func TestProcessStoredAuctionAndBidResponses(t *testing.T) { t.Run(test.description, func(t *testing.T) { rw := openrtb_ext.RequestWrapper{BidRequest: &test.request} storedAuctionResponses, storedBidResponses, bidderImpReplaceImpId, errorList := ProcessStoredResponses(nil, &rw, fetcher) - assert.Equal(t, test.expectedStoredAuctionResponses, storedAuctionResponses, "storedAuctionResponses doesn't match: %s\n", test.description) - assert.Equalf(t, test.expectedStoredBidResponses, storedBidResponses, "storedBidResponses doesn't match: %s\n", test.description) - assert.Equal(t, test.expectedBidderImpReplaceImpID, bidderImpReplaceImpId, "bidderImpReplaceImpId doesn't match: %s\n", test.description) + assert.Equal(t, test.expectedStoredAuctionResponses, storedAuctionResponses) + assert.Equal(t, test.expectedStoredBidResponses, storedBidResponses) + assert.Equal(t, test.expectedBidderImpReplaceImpID, bidderImpReplaceImpId) assert.Nil(t, errorList, "Error should be nil") }) } @@ -868,7 +868,7 @@ func TestProcessStoredResponsesNotFoundResponse(t *testing.T) { rw := openrtb_ext.RequestWrapper{BidRequest: &test.request} _, _, _, errorList := ProcessStoredResponses(nil, &rw, fetcher) for _, err := range test.expectedErrors { - assert.Contains(t, errorList, err, "incorrect errors returned: %s", test.description) + assert.Contains(t, errorList, err) } }) } @@ -926,7 +926,7 @@ func TestFlipMap(t *testing.T) { for _, test := range testCases { t.Run(test.description, func(t *testing.T) { actualResult := flipMap(test.inImpBidderReplaceImpID) - assert.Equal(t, test.outBidderImpReplaceImpID, actualResult, "Incorrect flipped map for test case %s\n", test.description) + assert.Equal(t, test.outBidderImpReplaceImpID, actualResult) }) } } From d4fb8e0f82bce877ba9b414a9301c3c0eeae1887 Mon Sep 17 00:00:00 2001 From: VeronikaSolovei9 Date: Wed, 18 Oct 2023 13:11:13 -0700 Subject: [PATCH 6/8] Merge fix --- exchange/utils.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exchange/utils.go b/exchange/utils.go index 70988fbf7b9..6a990be8ecf 100644 --- a/exchange/utils.go +++ b/exchange/utils.go @@ -957,7 +957,7 @@ func buildBidResponseRequest(req *openrtb2.BidRequest, BidderCoreName: resolvedBidder, BidderName: bidderName, BidderStoredResponses: impResps, - ImpReplaceImpId: bidderImpReplaceImpID[string(resolvedBidder)], + ImpReplaceImpId: bidderImpReplaceImpID[string(bidderName)], IsRequestAlias: isRequestAlias, BidderLabels: metrics.AdapterLabels{Adapter: resolvedBidder}, } From 1b53ce4d1aef4578b82e98930fb28fd5945b3f3c Mon Sep 17 00:00:00 2001 From: VeronikaSolovei9 Date: Wed, 18 Oct 2023 16:42:41 -0700 Subject: [PATCH 7/8] Validation fix for non-matching bidder names in imp.ext with imp.ext.prebid.storedbidresponse --- endpoints/openrtb2/auction.go | 9 ++++++++- endpoints/openrtb2/auction_test.go | 8 +++++--- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/endpoints/openrtb2/auction.go b/endpoints/openrtb2/auction.go index e54fee9d3b6..54e5c0135ac 100644 --- a/endpoints/openrtb2/auction.go +++ b/endpoints/openrtb2/auction.go @@ -1531,7 +1531,11 @@ func (deps *endpointDeps) validateImpExt(imp *openrtb_ext.ImpWrapper, aliases ma return []error{fmt.Errorf("request validation failed. The StoredAuctionResponse.ID field must be completely present with, or completely absent from, all impressions in request. No StoredAuctionResponse data found for request.imp[%d].ext.prebid \n", impIndex)} } - if len(storedBidResp) > 0 { + if len(prebid.StoredBidResponse) > 0 || len(storedBidResp) > 0 { + // prebid.StoredBidResponse - data from incoming imp.ext.prebid.storedbidresponse + // storedBidResp - imp id mapped to bidders from imp.ext (case insensitive) that have matched stored bid response + // in case only one of prebid.StoredBidResponse or storedBidResp exist means stored responses are specified + // in imp.ext.prebid.storedresponse, but don't match bidders in imp.ext (or imp.ext.prebid.bidders) if err := deps.validateStoredBidRespAndImpExtBidders(prebid.Bidder, storedBidResp, imp.ID); err != nil { return []error{err} } @@ -2502,6 +2506,9 @@ func checkIfAppRequest(request []byte) (bool, error) { } func (deps *endpointDeps) validateStoredBidRespAndImpExtBidders(bidderExts map[string]json.RawMessage, storedBidResp stored_responses.ImpBidderStoredResp, impId string) error { + if storedBidResp == nil { + return generateStoredBidResponseValidationError(impId) + } if bidResponses, ok := storedBidResp[impId]; ok { if len(bidResponses) != len(bidderExts) { return generateStoredBidResponseValidationError(impId) diff --git a/endpoints/openrtb2/auction_test.go b/endpoints/openrtb2/auction_test.go index 7dc52d9be6b..caf3b35ba4b 100644 --- a/endpoints/openrtb2/auction_test.go +++ b/endpoints/openrtb2/auction_test.go @@ -5051,7 +5051,7 @@ func TestParseRequestStoredBidResponses(t *testing.T) { name: "req has two imps with missing stored bid responses", givenRequestBody: validRequest(t, "req-two-imps-missing-stored-bid-response.json"), expectedStoredBidResponses: nil, - expectedErrorCount: 0, + expectedErrorCount: 1, }, { name: "req imp has valid stored bid response with non existing bidder name", @@ -5667,8 +5667,10 @@ func TestValidateStoredResp(t *testing.T) { } for _, test := range testCases { - errorList := deps.validateRequest(test.givenRequestWrapper, false, test.hasStoredAuctionResponses, test.storedBidResponses, false) - assert.Equalf(t, test.expectedErrorList, errorList, "Error doesn't match: %s\n", test.description) + t.Run(test.description, func(t *testing.T) { + errorList := deps.validateRequest(test.givenRequestWrapper, false, test.hasStoredAuctionResponses, test.storedBidResponses, false) + assert.Equalf(t, test.expectedErrorList, errorList, "Error doesn't match: %s\n", test.description) + }) } } From 0a7379876d703f649fbe28f442cf2536de0cab27 Mon Sep 17 00:00:00 2001 From: VeronikaSolovei9 Date: Thu, 19 Oct 2023 09:47:08 -0700 Subject: [PATCH 8/8] Code clean up --- endpoints/openrtb2/auction.go | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/endpoints/openrtb2/auction.go b/endpoints/openrtb2/auction.go index 54e5c0135ac..c704028d6e9 100644 --- a/endpoints/openrtb2/auction.go +++ b/endpoints/openrtb2/auction.go @@ -1531,14 +1531,8 @@ func (deps *endpointDeps) validateImpExt(imp *openrtb_ext.ImpWrapper, aliases ma return []error{fmt.Errorf("request validation failed. The StoredAuctionResponse.ID field must be completely present with, or completely absent from, all impressions in request. No StoredAuctionResponse data found for request.imp[%d].ext.prebid \n", impIndex)} } - if len(prebid.StoredBidResponse) > 0 || len(storedBidResp) > 0 { - // prebid.StoredBidResponse - data from incoming imp.ext.prebid.storedbidresponse - // storedBidResp - imp id mapped to bidders from imp.ext (case insensitive) that have matched stored bid response - // in case only one of prebid.StoredBidResponse or storedBidResp exist means stored responses are specified - // in imp.ext.prebid.storedresponse, but don't match bidders in imp.ext (or imp.ext.prebid.bidders) - if err := deps.validateStoredBidRespAndImpExtBidders(prebid.Bidder, storedBidResp, imp.ID); err != nil { - return []error{err} - } + if err := deps.validateStoredBidRespAndImpExtBidders(prebid, storedBidResp, imp.ID); err != nil { + return []error{err} } errL := []error{} @@ -2505,12 +2499,16 @@ func checkIfAppRequest(request []byte) (bool, error) { return false, nil } -func (deps *endpointDeps) validateStoredBidRespAndImpExtBidders(bidderExts map[string]json.RawMessage, storedBidResp stored_responses.ImpBidderStoredResp, impId string) error { +func (deps *endpointDeps) validateStoredBidRespAndImpExtBidders(prebid *openrtb_ext.ExtImpPrebid, storedBidResp stored_responses.ImpBidderStoredResp, impId string) error { + if storedBidResp == nil && len(prebid.StoredBidResponse) == 0 { + return nil + } + if storedBidResp == nil { return generateStoredBidResponseValidationError(impId) } if bidResponses, ok := storedBidResp[impId]; ok { - if len(bidResponses) != len(bidderExts) { + if len(bidResponses) != len(prebid.Bidder) { return generateStoredBidResponseValidationError(impId) } @@ -2518,7 +2516,7 @@ func (deps *endpointDeps) validateStoredBidRespAndImpExtBidders(bidderExts map[s if _, bidderNameOk := deps.normalizeBidderName(bidderName); !bidderNameOk { return fmt.Errorf(`unrecognized bidder "%v"`, bidderName) } - if _, present := bidderExts[bidderName]; !present { + if _, present := prebid.Bidder[bidderName]; !present { return generateStoredBidResponseValidationError(impId) } }