Skip to content

Commit

Permalink
Fix: Panic in bid adjustments (#3547)
Browse files Browse the repository at this point in the history 
* #3543 fix panic in bids adjustment

* add a test for empty ext.prebid and account with enabled bid adjustments

* add more tests for bid adjustments

* remove extra space

---------

Co-authored-by: oaleksieiev <oleksandr@assertive.ai>
  • Loading branch information
@linux019
linux019 and oaleksieiev committed Mar 18, 2024
1 parent 6cc298c commit 82ba585
Show file tree
Hide file tree
Showing 2 changed files with 164 additions and 98 deletions.
8 changes: 3 additions & 5 deletions bidadjustment/build_rules.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,11 @@ func merge(req *openrtb_ext.RequestWrapper, acct *openrtb_ext.ExtRequestPrebidBi
}
extPrebid := reqExt.GetPrebid()

if extPrebid == nil && acct == nil {
return nil, nil
}
if extPrebid == nil && acct != nil {
if extPrebid == nil || extPrebid.BidAdjustments == nil {
return acct, nil
}
if extPrebid != nil && acct == nil {

if acct == nil {
return extPrebid.BidAdjustments, nil
}

Expand Down
254 changes: 161 additions & 93 deletions bidadjustment/build_rules_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -220,34 +220,26 @@ func TestMerge(t *testing.T) {
testCases := []struct {
name string
givenRequestWrapper *openrtb_ext.RequestWrapper
givenAccount *config.Account
acctBidAdjustments *openrtb_ext.ExtRequestPrebidBidAdjustments
expectedBidAdjustments *openrtb_ext.ExtRequestPrebidBidAdjustments
}{
{
name: "DiffBidderNames",
givenRequestWrapper: &openrtb_ext.RequestWrapper{
BidRequest: &openrtb2.BidRequest{Ext: []byte(`{"prebid":{"bidadjustments":{"mediatype":{"banner":{"bidderA":{"dealId":[{ "adjtype": "multiplier", "value": 1.1}]}}}}}}`)},
},
givenAccount: &config.Account{
BidAdjustments: &openrtb_ext.ExtRequestPrebidBidAdjustments{
MediaType: openrtb_ext.MediaType{
Banner: map[openrtb_ext.BidderName]openrtb_ext.AdjustmentsByDealID{
"bidderB": {
"dealId": []openrtb_ext.Adjustment{{Type: AdjustmentTypeMultiplier, Value: 1.5}},
},
},
acctBidAdjustments: &openrtb_ext.ExtRequestPrebidBidAdjustments{
MediaType: openrtb_ext.MediaType{
Banner: map[openrtb_ext.BidderName]openrtb_ext.AdjustmentsByDealID{
"bidderB": {"dealId": []openrtb_ext.Adjustment{{Type: AdjustmentTypeMultiplier, Value: 1.5}}},
},
},
},
expectedBidAdjustments: &openrtb_ext.ExtRequestPrebidBidAdjustments{
MediaType: openrtb_ext.MediaType{
Banner: map[openrtb_ext.BidderName]openrtb_ext.AdjustmentsByDealID{
"bidderA": {
"dealId": []openrtb_ext.Adjustment{{Type: AdjustmentTypeMultiplier, Value: 1.1}},
},
"bidderB": {
"dealId": []openrtb_ext.Adjustment{{Type: AdjustmentTypeMultiplier, Value: 1.5}},
},
"bidderA": {"dealId": []openrtb_ext.Adjustment{{Type: AdjustmentTypeMultiplier, Value: 1.1}}},
"bidderB": {"dealId": []openrtb_ext.Adjustment{{Type: AdjustmentTypeMultiplier, Value: 1.5}}},
},
},
},
Expand All @@ -257,23 +249,17 @@ func TestMerge(t *testing.T) {
givenRequestWrapper: &openrtb_ext.RequestWrapper{
BidRequest: &openrtb2.BidRequest{Ext: []byte(`{"prebid":{"bidadjustments":{"mediatype":{"audio":{"bidderA":{"dealId":[{ "adjtype": "multiplier", "value": 1.1}]}}}}}}`)},
},
givenAccount: &config.Account{
BidAdjustments: &openrtb_ext.ExtRequestPrebidBidAdjustments{
MediaType: openrtb_ext.MediaType{
Audio: map[openrtb_ext.BidderName]openrtb_ext.AdjustmentsByDealID{
"bidderA": {
"dealId": []openrtb_ext.Adjustment{{Type: AdjustmentTypeMultiplier, Value: 1.5}},
},
},
acctBidAdjustments: &openrtb_ext.ExtRequestPrebidBidAdjustments{
MediaType: openrtb_ext.MediaType{
Audio: map[openrtb_ext.BidderName]openrtb_ext.AdjustmentsByDealID{
"bidderA": {"dealId": []openrtb_ext.Adjustment{{Type: AdjustmentTypeMultiplier, Value: 1.5}}},
},
},
},
expectedBidAdjustments: &openrtb_ext.ExtRequestPrebidBidAdjustments{
MediaType: openrtb_ext.MediaType{
Audio: map[openrtb_ext.BidderName]openrtb_ext.AdjustmentsByDealID{
"bidderA": {
"dealId": []openrtb_ext.Adjustment{{Type: AdjustmentTypeMultiplier, Value: 1.1}},
},
"bidderA": {"dealId": []openrtb_ext.Adjustment{{Type: AdjustmentTypeMultiplier, Value: 1.1}}},
},
},
},
Expand All @@ -283,14 +269,10 @@ func TestMerge(t *testing.T) {
givenRequestWrapper: &openrtb_ext.RequestWrapper{
BidRequest: &openrtb2.BidRequest{Ext: []byte(`{"prebid":{"bidadjustments":{"mediatype":{"video-instream":{"bidderA":{"dealId":[{ "adjtype": "static", "value": 3.00, "currency": "USD"}]}}}}}}`)},
},
givenAccount: &config.Account{
BidAdjustments: &openrtb_ext.ExtRequestPrebidBidAdjustments{
MediaType: openrtb_ext.MediaType{
VideoInstream: map[openrtb_ext.BidderName]openrtb_ext.AdjustmentsByDealID{
"bidderA": {
"diffDealId": []openrtb_ext.Adjustment{{Type: AdjustmentTypeMultiplier, Value: 1.5}},
},
},
acctBidAdjustments: &openrtb_ext.ExtRequestPrebidBidAdjustments{
MediaType: openrtb_ext.MediaType{
VideoInstream: map[openrtb_ext.BidderName]openrtb_ext.AdjustmentsByDealID{
"bidderA": {"diffDealId": []openrtb_ext.Adjustment{{Type: AdjustmentTypeMultiplier, Value: 1.5}}},
},
},
},
Expand All @@ -310,26 +292,18 @@ func TestMerge(t *testing.T) {
givenRequestWrapper: &openrtb_ext.RequestWrapper{
BidRequest: &openrtb2.BidRequest{Ext: []byte(`{"prebid":{"bidadjustments":{"mediatype":{"native":{"bidderA":{"dealId":[{"adjtype": "cpm", "value": 0.18, "currency": "USD"}]}}}}}}`)},
},
givenAccount: &config.Account{
BidAdjustments: &openrtb_ext.ExtRequestPrebidBidAdjustments{
MediaType: openrtb_ext.MediaType{
Native: map[openrtb_ext.BidderName]openrtb_ext.AdjustmentsByDealID{
"bidderB": {
"dealId": []openrtb_ext.Adjustment{{Type: AdjustmentTypeMultiplier, Value: 1.5}},
},
},
acctBidAdjustments: &openrtb_ext.ExtRequestPrebidBidAdjustments{
MediaType: openrtb_ext.MediaType{
Native: map[openrtb_ext.BidderName]openrtb_ext.AdjustmentsByDealID{
"bidderB": {"dealId": []openrtb_ext.Adjustment{{Type: AdjustmentTypeMultiplier, Value: 1.5}}},
},
},
},
expectedBidAdjustments: &openrtb_ext.ExtRequestPrebidBidAdjustments{
MediaType: openrtb_ext.MediaType{
Native: map[openrtb_ext.BidderName]openrtb_ext.AdjustmentsByDealID{
"bidderA": {
"dealId": []openrtb_ext.Adjustment{{Type: AdjustmentTypeCPM, Value: 0.18, Currency: "USD"}},
},
"bidderB": {
"dealId": []openrtb_ext.Adjustment{{Type: AdjustmentTypeMultiplier, Value: 1.5}},
},
"bidderA": {"dealId": []openrtb_ext.Adjustment{{Type: AdjustmentTypeCPM, Value: 0.18, Currency: "USD"}}},
"bidderB": {"dealId": []openrtb_ext.Adjustment{{Type: AdjustmentTypeMultiplier, Value: 1.5}}},
},
},
},
Expand All @@ -339,28 +313,20 @@ func TestMerge(t *testing.T) {
givenRequestWrapper: &openrtb_ext.RequestWrapper{
BidRequest: &openrtb2.BidRequest{Ext: []byte(`{"prebid":{"bidadjustments":{"mediatype":{"video-outstream":{"bidderA":{"dealId":[{ "adjtype": "multiplier", "value": 1.1}]}}}}}}`)},
},
givenAccount: &config.Account{
BidAdjustments: &openrtb_ext.ExtRequestPrebidBidAdjustments{
MediaType: openrtb_ext.MediaType{
Banner: map[openrtb_ext.BidderName]openrtb_ext.AdjustmentsByDealID{
"bidderB": {
"dealId": []openrtb_ext.Adjustment{{Type: AdjustmentTypeMultiplier, Value: 1.5}},
},
},
acctBidAdjustments: &openrtb_ext.ExtRequestPrebidBidAdjustments{
MediaType: openrtb_ext.MediaType{
Banner: map[openrtb_ext.BidderName]openrtb_ext.AdjustmentsByDealID{
"bidderB": {"dealId": []openrtb_ext.Adjustment{{Type: AdjustmentTypeMultiplier, Value: 1.5}}},
},
},
},
expectedBidAdjustments: &openrtb_ext.ExtRequestPrebidBidAdjustments{
MediaType: openrtb_ext.MediaType{
Banner: map[openrtb_ext.BidderName]openrtb_ext.AdjustmentsByDealID{
"bidderB": {
"dealId": []openrtb_ext.Adjustment{{Type: AdjustmentTypeMultiplier, Value: 1.5}},
},
"bidderB": {"dealId": []openrtb_ext.Adjustment{{Type: AdjustmentTypeMultiplier, Value: 1.5}}},
},
VideoOutstream: map[openrtb_ext.BidderName]openrtb_ext.AdjustmentsByDealID{
"bidderA": {
"dealId": []openrtb_ext.Adjustment{{Type: AdjustmentTypeMultiplier, Value: 1.1}},
},
"bidderA": {"dealId": []openrtb_ext.Adjustment{{Type: AdjustmentTypeMultiplier, Value: 1.1}}},
},
},
},
Expand All @@ -370,23 +336,17 @@ func TestMerge(t *testing.T) {
givenRequestWrapper: &openrtb_ext.RequestWrapper{
BidRequest: &openrtb2.BidRequest{Ext: []byte(`{"ext":{"bidder": {}}}`)},
},
givenAccount: &config.Account{
BidAdjustments: &openrtb_ext.ExtRequestPrebidBidAdjustments{
MediaType: openrtb_ext.MediaType{
Banner: map[openrtb_ext.BidderName]openrtb_ext.AdjustmentsByDealID{
"bidderB": {
"dealId": []openrtb_ext.Adjustment{{Type: AdjustmentTypeMultiplier, Value: 1.5}},
},
},
acctBidAdjustments: &openrtb_ext.ExtRequestPrebidBidAdjustments{
MediaType: openrtb_ext.MediaType{
Banner: map[openrtb_ext.BidderName]openrtb_ext.AdjustmentsByDealID{
"bidderB": {"dealId": []openrtb_ext.Adjustment{{Type: AdjustmentTypeMultiplier, Value: 1.5}}},
},
},
},
expectedBidAdjustments: &openrtb_ext.ExtRequestPrebidBidAdjustments{
MediaType: openrtb_ext.MediaType{
Banner: map[openrtb_ext.BidderName]openrtb_ext.AdjustmentsByDealID{
"bidderB": {
"dealId": []openrtb_ext.Adjustment{{Type: AdjustmentTypeMultiplier, Value: 1.5}},
},
"bidderB": {"dealId": []openrtb_ext.Adjustment{{Type: AdjustmentTypeMultiplier, Value: 1.5}}},
},
},
},
Expand All @@ -396,28 +356,20 @@ func TestMerge(t *testing.T) {
givenRequestWrapper: &openrtb_ext.RequestWrapper{
BidRequest: &openrtb2.BidRequest{Ext: []byte(`{"prebid":{"bidadjustments":{"mediatype":{"video-instream":{"bidderA":{"dealId":[{ "adjtype": "multiplier", "value": 1.1}]}}}}}}`)},
},
givenAccount: &config.Account{
BidAdjustments: &openrtb_ext.ExtRequestPrebidBidAdjustments{
MediaType: openrtb_ext.MediaType{
WildCard: map[openrtb_ext.BidderName]openrtb_ext.AdjustmentsByDealID{
"bidderB": {
"dealId": []openrtb_ext.Adjustment{{Type: AdjustmentTypeMultiplier, Value: 1.5}},
},
},
acctBidAdjustments: &openrtb_ext.ExtRequestPrebidBidAdjustments{
MediaType: openrtb_ext.MediaType{
WildCard: map[openrtb_ext.BidderName]openrtb_ext.AdjustmentsByDealID{
"bidderB": {"dealId": []openrtb_ext.Adjustment{{Type: AdjustmentTypeMultiplier, Value: 1.5}}},
},
},
},
expectedBidAdjustments: &openrtb_ext.ExtRequestPrebidBidAdjustments{
MediaType: openrtb_ext.MediaType{
WildCard: map[openrtb_ext.BidderName]openrtb_ext.AdjustmentsByDealID{
"bidderB": {
"dealId": []openrtb_ext.Adjustment{{Type: AdjustmentTypeMultiplier, Value: 1.5}},
},
"bidderB": {"dealId": []openrtb_ext.Adjustment{{Type: AdjustmentTypeMultiplier, Value: 1.5}}},
},
VideoInstream: map[openrtb_ext.BidderName]openrtb_ext.AdjustmentsByDealID{
"bidderA": {
"dealId": []openrtb_ext.Adjustment{{Type: AdjustmentTypeMultiplier, Value: 1.1}},
},
"bidderA": {"dealId": []openrtb_ext.Adjustment{{Type: AdjustmentTypeMultiplier, Value: 1.1}}},
},
},
},
Expand All @@ -427,21 +379,137 @@ func TestMerge(t *testing.T) {
givenRequestWrapper: &openrtb_ext.RequestWrapper{
BidRequest: &openrtb2.BidRequest{Ext: []byte(`{"ext":{"bidder": {}}}`)},
},
givenAccount: &config.Account{},
acctBidAdjustments: nil,
expectedBidAdjustments: nil,
},
{
name: "NilAcctBidAdj",
givenRequestWrapper: &openrtb_ext.RequestWrapper{
BidRequest: &openrtb2.BidRequest{Ext: []byte(`{"prebid":{"bidadjustments":{"mediatype":{"banner":{"bidderA":{"dealId":[{ "adjtype": "multiplier", "value": 1.1}]}}}}}}`)},
},
givenAccount: &config.Account{},
acctBidAdjustments: nil,
expectedBidAdjustments: &openrtb_ext.ExtRequestPrebidBidAdjustments{
MediaType: openrtb_ext.MediaType{
Banner: map[openrtb_ext.BidderName]openrtb_ext.AdjustmentsByDealID{
"bidderA": {
"dealId": []openrtb_ext.Adjustment{{Type: AdjustmentTypeMultiplier, Value: 1.1}},
},
"bidderA": {"dealId": []openrtb_ext.Adjustment{{Type: AdjustmentTypeMultiplier, Value: 1.1}}},
},
},
},
},

{
name: "NilExtPrebid-NilExtPrebidBidAdj_NilAcct",
givenRequestWrapper: &openrtb_ext.RequestWrapper{
BidRequest: &openrtb2.BidRequest{},
},
acctBidAdjustments: nil,
expectedBidAdjustments: nil,
},
{
name: "NilExtPrebid-NilExtPrebidBidAdj-Acct",
givenRequestWrapper: &openrtb_ext.RequestWrapper{
BidRequest: &openrtb2.BidRequest{},
},
acctBidAdjustments: &openrtb_ext.ExtRequestPrebidBidAdjustments{
MediaType: openrtb_ext.MediaType{
Banner: map[openrtb_ext.BidderName]openrtb_ext.AdjustmentsByDealID{
"bidderA": {"dealId": []openrtb_ext.Adjustment{{Type: AdjustmentTypeMultiplier, Value: 1.1}}},
},
},
},
expectedBidAdjustments: &openrtb_ext.ExtRequestPrebidBidAdjustments{
MediaType: openrtb_ext.MediaType{
Banner: map[openrtb_ext.BidderName]openrtb_ext.AdjustmentsByDealID{
"bidderA": {"dealId": []openrtb_ext.Adjustment{{Type: AdjustmentTypeMultiplier, Value: 1.1}}},
},
},
},
},
{
name: "NotNilExtPrebid-NilExtBidAdj-NilAcct",
givenRequestWrapper: &openrtb_ext.RequestWrapper{
BidRequest: &openrtb2.BidRequest{Ext: []byte(`{"prebid":{}}`)},
},
acctBidAdjustments: nil,
expectedBidAdjustments: nil,
},
{
name: "NotNilExtPrebid_NilExtBidAdj_NotNilAcct",
givenRequestWrapper: &openrtb_ext.RequestWrapper{
BidRequest: &openrtb2.BidRequest{Ext: []byte(`{"prebid":{}}`)},
},
acctBidAdjustments: &openrtb_ext.ExtRequestPrebidBidAdjustments{
MediaType: openrtb_ext.MediaType{
Banner: map[openrtb_ext.BidderName]openrtb_ext.AdjustmentsByDealID{
"bidderA": {"dealId": []openrtb_ext.Adjustment{{Type: AdjustmentTypeMultiplier, Value: 1.1}}},
},
},
},
expectedBidAdjustments: &openrtb_ext.ExtRequestPrebidBidAdjustments{
MediaType: openrtb_ext.MediaType{
Banner: map[openrtb_ext.BidderName]openrtb_ext.AdjustmentsByDealID{
"bidderA": {"dealId": []openrtb_ext.Adjustment{{Type: AdjustmentTypeMultiplier, Value: 1.1}}},
},
},
},
},
{
name: "NotNilExtPrebid-NotNilExtBidAdj-NilAcct",
givenRequestWrapper: &openrtb_ext.RequestWrapper{
BidRequest: &openrtb2.BidRequest{Ext: []byte(`{"prebid":{"bidadjustments":{"mediatype":{"banner":{"bidderA":{"dealId":[{ "adjtype": "multiplier", "value": 1.1}]}}}}}}`)},
},
acctBidAdjustments: nil,
expectedBidAdjustments: &openrtb_ext.ExtRequestPrebidBidAdjustments{
MediaType: openrtb_ext.MediaType{
Banner: map[openrtb_ext.BidderName]openrtb_ext.AdjustmentsByDealID{
"bidderA": {"dealId": []openrtb_ext.Adjustment{{Type: AdjustmentTypeMultiplier, Value: 1.1}}},
},
},
},
},
{
name: "NotNilExtPrebid-NotNilExtBidAdj-NotNilAcct",
givenRequestWrapper: &openrtb_ext.RequestWrapper{
BidRequest: &openrtb2.BidRequest{Ext: []byte(`{"prebid":{"bidadjustments":{"mediatype":{"banner":{"bidderA":{"dealId":[{ "adjtype": "multiplier", "value": 1.1}]}}}}}}`)},
},
acctBidAdjustments: &openrtb_ext.ExtRequestPrebidBidAdjustments{
MediaType: openrtb_ext.MediaType{
VideoInstream: map[openrtb_ext.BidderName]openrtb_ext.AdjustmentsByDealID{
"bidderB": {"dealId": []openrtb_ext.Adjustment{{Type: AdjustmentTypeCPM, Value: 3}}},
},
VideoOutstream: map[openrtb_ext.BidderName]openrtb_ext.AdjustmentsByDealID{
"bidderC": {"dealId": []openrtb_ext.Adjustment{{Type: AdjustmentTypeCPM, Value: 3}}},
},
Audio: map[openrtb_ext.BidderName]openrtb_ext.AdjustmentsByDealID{
"bidderD": {"dealId": []openrtb_ext.Adjustment{{Type: AdjustmentTypeCPM, Value: 3}}},
},
Native: map[openrtb_ext.BidderName]openrtb_ext.AdjustmentsByDealID{
"bidderE": {"dealId": []openrtb_ext.Adjustment{{Type: AdjustmentTypeCPM, Value: 3}}},
},
WildCard: map[openrtb_ext.BidderName]openrtb_ext.AdjustmentsByDealID{
"bidderF": {"dealId": []openrtb_ext.Adjustment{{Type: AdjustmentTypeCPM, Value: 3}}},
},
},
},
expectedBidAdjustments: &openrtb_ext.ExtRequestPrebidBidAdjustments{
MediaType: openrtb_ext.MediaType{
Banner: map[openrtb_ext.BidderName]openrtb_ext.AdjustmentsByDealID{
"bidderA": {"dealId": []openrtb_ext.Adjustment{{Type: AdjustmentTypeMultiplier, Value: 1.1}}},
},
VideoInstream: map[openrtb_ext.BidderName]openrtb_ext.AdjustmentsByDealID{
"bidderB": {"dealId": []openrtb_ext.Adjustment{{Type: AdjustmentTypeCPM, Value: 3}}},
},
VideoOutstream: map[openrtb_ext.BidderName]openrtb_ext.AdjustmentsByDealID{
"bidderC": {"dealId": []openrtb_ext.Adjustment{{Type: AdjustmentTypeCPM, Value: 3}}},
},
Audio: map[openrtb_ext.BidderName]openrtb_ext.AdjustmentsByDealID{
"bidderD": {"dealId": []openrtb_ext.Adjustment{{Type: AdjustmentTypeCPM, Value: 3}}},
},
Native: map[openrtb_ext.BidderName]openrtb_ext.AdjustmentsByDealID{
"bidderE": {"dealId": []openrtb_ext.Adjustment{{Type: AdjustmentTypeCPM, Value: 3}}},
},
WildCard: map[openrtb_ext.BidderName]openrtb_ext.AdjustmentsByDealID{
"bidderF": {"dealId": []openrtb_ext.Adjustment{{Type: AdjustmentTypeCPM, Value: 3}}},
},
},
},
Expand All @@ -450,7 +518,7 @@ func TestMerge(t *testing.T) {

for _, test := range testCases {
t.Run(test.name, func(t *testing.T) {
mergedBidAdj, err := merge(test.givenRequestWrapper, test.givenAccount.BidAdjustments)
mergedBidAdj, err := merge(test.givenRequestWrapper, test.acctBidAdjustments)
assert.NoError(t, err)
assert.Equal(t, test.expectedBidAdjustments, mergedBidAdj)
})
Expand Down

0 comments on commit 82ba585

Please sign in to comment.