Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adapter Name Case Insensitive: dealTier #3218

Merged
merged 8 commits into from
Nov 20, 2023
Merged

Adapter Name Case Insensitive: dealTier #3218

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
3060d15
Notes
guscarreon Oct 12, 2023
b9ddd6e
First commit
guscarreon Oct 13, 2023
b1783e5
Merge branch 'master' into caseInsensitiveDealTier
guscarreon Oct 25, 2023
bd353a0
update
guscarreon Oct 30, 2023
fc673db
Merge branch 'master' into caseInsensitiveDealTier
guscarreon Nov 1, 2023
027ae48
Normalize bidder in applyDealSupport()
guscarreon Nov 1, 2023
ed3ee69
Clean up code
guscarreon Nov 2, 2023
c5564e1
Alex's review
guscarreon Nov 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions endpoints/openrtb2/sample-requests/valid-whole/exemplary/all-ext-case-insensitive.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
]
},
"ext": {
"appnexus": {
"APPnexus": {
"placementId": 12883451
},
"districtm": {
Expand Down Expand Up @@ -109,7 +109,7 @@
"price": 1.01
}
],
"seat": "appnexus"
"seat": "APPnexus"
},
{
"bid": [
Expand Down Expand Up @@ -137,4 +137,4 @@
"nbr": 0
},
"expectedReturnCode": 200
}
}
11 changes: 7 additions & 4 deletions exchange/exchange.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -575,15 +575,18 @@ func applyDealSupport(bidRequest *openrtb2.BidRequest, auc *auction, bidCategory
for impID, topBidsPerImp := range auc.winningBidsByBidder {
impDeal := impDealMap[impID]
for bidder, topBidsPerBidder := range topBidsPerImp {
maxBid := bidsToUpdate(multiBid, bidder.String())

bidderNormalized, bidderFound := openrtb_ext.NormalizeBidderName(bidder.String())
if !bidderFound {
continue
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line isn't covered by tests, do you think you can add a test where the bidder is not found from the NormalizeBidderName call?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Also refactored TestApplyDealSupport a little bit.

}
maxBid := bidsToUpdate(multiBid, bidderNormalized.String())
for i, topBid := range topBidsPerBidder {
if i == maxBid {
break
}
if topBid.DealPriority > 0 {
if validateDealTier(impDeal[bidder]) {
updateHbPbCatDur(topBid, impDeal[bidder], bidCategory)
if validateDealTier(impDeal[bidderNormalized]) {
updateHbPbCatDur(topBid, impDeal[bidderNormalized], bidCategory)
} else {
errs = append(errs, fmt.Errorf("dealTier configuration invalid for bidder '%s', imp ID '%s'", string(bidder), impID))
}
Expand Down
234 changes: 186 additions & 48 deletions exchange/exchange_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3585,92 +3585,168 @@ func TestUpdateRejections(t *testing.T) {
}

func TestApplyDealSupport(t *testing.T) {
type testInput struct {
dealPriority int
impExt json.RawMessage
targ map[string]string
bidderName openrtb_ext.BidderName
}

type testOutput struct {
hbPbCatDur string
dealErr string
dealTierSatisfied bool
}

testCases := []struct {
description string
dealPriority int
impExt json.RawMessage
targ map[string]string
expectedHbPbCatDur string
expectedDealErr string
expectedDealTierSatisfied bool
description string
in testInput
expected testOutput
}{
{
description: "hb_pb_cat_dur should be modified",
dealPriority: 5,
impExt: json.RawMessage(`{"prebid": {"bidder": {"appnexus": {"dealTier": {"minDealTier": 5, "prefix": "tier"}, "placementId": 10433394}}}}`),
targ: map[string]string{
"hb_pb_cat_dur": "12.00_movies_30s",
description: "hb_pb_cat_dur should be modified",
in: testInput{
dealPriority: 5,
impExt: json.RawMessage(`{"prebid": {"bidder": {"appnexus": {"dealTier": {"minDealTier": 5, "prefix": "tier"}, "placementId": 10433394}}}}`),
targ: map[string]string{
"hb_pb_cat_dur": "12.00_movies_30s",
},
bidderName: openrtb_ext.BidderName("appnexus"),
},
expected: testOutput{
hbPbCatDur: "tier5_movies_30s",
dealErr: "",
dealTierSatisfied: true,
},
expectedHbPbCatDur: "tier5_movies_30s",
expectedDealErr: "",
expectedDealTierSatisfied: true,
},
{
description: "hb_pb_cat_dur should not be modified due to priority not exceeding min",
dealPriority: 9,
impExt: json.RawMessage(`{"prebid": {"bidder": {"appnexus": {"dealTier": {"minDealTier": 10, "prefix": "tier"}, "placementId": 10433394}}}}`),
targ: map[string]string{
"hb_pb_cat_dur": "12.00_medicine_30s",
description: "hb_pb_cat_dur should be modified even with a mixed case bidder in the impExt",
in: testInput{
dealPriority: 5,
impExt: json.RawMessage(`{"prebid": {"bidder": {"APPnexus": {"dealTier": {"minDealTier": 5, "prefix": "tier"}, "placementId": 10433394}}}}`),
targ: map[string]string{
"hb_pb_cat_dur": "12.00_movies_30s",
},
bidderName: openrtb_ext.BidderName("appnexus"),
},
expected: testOutput{
hbPbCatDur: "tier5_movies_30s",
dealErr: "",
dealTierSatisfied: true,
},
expectedHbPbCatDur: "12.00_medicine_30s",
expectedDealErr: "",
expectedDealTierSatisfied: false,
},
{
description: "hb_pb_cat_dur should not be modified due to invalid config",
dealPriority: 5,
impExt: json.RawMessage(`{"prebid": {"bidder": {"appnexus": {"dealTier": {"minDealTier": 5, "prefix": ""}, "placementId": 10433394}}}}`),
targ: map[string]string{
"hb_pb_cat_dur": "12.00_games_30s",
description: "hb_pb_cat_dur should be modified even with a mixed case bidder in the winningBidsByBidder map",
in: testInput{
dealPriority: 5,
impExt: json.RawMessage(`{"prebid": {"bidder": {"appnexus": {"dealTier": {"minDealTier": 5, "prefix": "tier"}, "placementId": 10433394}}}}`),
targ: map[string]string{
"hb_pb_cat_dur": "12.00_movies_30s",
},
bidderName: openrtb_ext.BidderName("APPnexus"),
},
expected: testOutput{
hbPbCatDur: "tier5_movies_30s",
dealErr: "",
dealTierSatisfied: true,
},
expectedHbPbCatDur: "12.00_games_30s",
expectedDealErr: "dealTier configuration invalid for bidder 'appnexus', imp ID 'imp_id1'",
expectedDealTierSatisfied: false,
},
{
description: "hb_pb_cat_dur should not be modified due to deal priority of 0",
dealPriority: 0,
impExt: json.RawMessage(`{"prebid": {"bidder": {"appnexus": {"dealTier": {"minDealTier": 5, "prefix": "tier"}, "placementId": 10433394}}}}`),
targ: map[string]string{
"hb_pb_cat_dur": "12.00_auto_30s",
description: "hb_pb_cat_dur should not be modified due to unknown bidder in the winningBidsByBidder map",
in: testInput{
dealPriority: 9,
impExt: json.RawMessage(`{"prebid": {"bidder": {"appnexus": {"dealTier": {"minDealTier": 10, "prefix": "tier"}, "placementId": 10433394}}}}`),
targ: map[string]string{
"hb_pb_cat_dur": "12.00_medicine_30s",
},
bidderName: openrtb_ext.BidderName("unknown"),
},
expected: testOutput{
hbPbCatDur: "12.00_medicine_30s",
dealErr: "",
dealTierSatisfied: false,
},
},
{
description: "hb_pb_cat_dur should not be modified due to priority not exceeding min",
in: testInput{
dealPriority: 9,
impExt: json.RawMessage(`{"prebid": {"bidder": {"appnexus": {"dealTier": {"minDealTier": 10, "prefix": "tier"}, "placementId": 10433394}}}}`),
targ: map[string]string{
"hb_pb_cat_dur": "12.00_medicine_30s",
},
bidderName: openrtb_ext.BidderName("appnexus"),
},
expected: testOutput{
hbPbCatDur: "12.00_medicine_30s",
dealErr: "",
dealTierSatisfied: false,
},
},
{
description: "hb_pb_cat_dur should not be modified due to invalid config",
in: testInput{
dealPriority: 5,
impExt: json.RawMessage(`{"prebid": {"bidder": {"appnexus": {"dealTier": {"minDealTier": 5, "prefix": ""}, "placementId": 10433394}}}}`),
targ: map[string]string{
"hb_pb_cat_dur": "12.00_games_30s",
},
bidderName: openrtb_ext.BidderName("appnexus"),
},
expected: testOutput{
hbPbCatDur: "12.00_games_30s",
dealErr: "dealTier configuration invalid for bidder 'appnexus', imp ID 'imp_id1'",
dealTierSatisfied: false,
},
},
{
description: "hb_pb_cat_dur should not be modified due to deal priority of 0",
in: testInput{
dealPriority: 0,
impExt: json.RawMessage(`{"prebid": {"bidder": {"appnexus": {"dealTier": {"minDealTier": 5, "prefix": "tier"}, "placementId": 10433394}}}}`),
targ: map[string]string{
"hb_pb_cat_dur": "12.00_auto_30s",
},
bidderName: openrtb_ext.BidderName("appnexus"),
},
expected: testOutput{
hbPbCatDur: "12.00_auto_30s",
dealErr: "",
dealTierSatisfied: false,
},
expectedHbPbCatDur: "12.00_auto_30s",
expectedDealErr: "",
expectedDealTierSatisfied: false,
},
}

bidderName := openrtb_ext.BidderName("appnexus")
for _, test := range testCases {
bidRequest := &openrtb2.BidRequest{
ID: "some-request-id",
Imp: []openrtb2.Imp{
{
ID: "imp_id1",
Ext: test.impExt,
Ext: test.in.impExt,
},
},
}

bid := entities.PbsOrtbBid{&openrtb2.Bid{ID: "123456"}, nil, "video", map[string]string{}, &openrtb_ext.ExtBidPrebidVideo{}, nil, nil, test.dealPriority, false, "", 0, "USD", ""}
bid := entities.PbsOrtbBid{&openrtb2.Bid{ID: "123456"}, nil, "video", map[string]string{}, &openrtb_ext.ExtBidPrebidVideo{}, nil, nil, test.in.dealPriority, false, "", 0, "USD", ""}
bidCategory := map[string]string{
bid.Bid.ID: test.targ["hb_pb_cat_dur"],
bid.Bid.ID: test.in.targ["hb_pb_cat_dur"],
}

auc := &auction{
winningBidsByBidder: map[string]map[openrtb_ext.BidderName][]*entities.PbsOrtbBid{
"imp_id1": {
bidderName: {&bid},
test.in.bidderName: {&bid},
},
},
}

dealErrs := applyDealSupport(bidRequest, auc, bidCategory, nil)

assert.Equal(t, test.expectedHbPbCatDur, bidCategory[auc.winningBidsByBidder["imp_id1"][bidderName][0].Bid.ID], test.description)
assert.Equal(t, test.expectedDealTierSatisfied, auc.winningBidsByBidder["imp_id1"][bidderName][0].DealTierSatisfied, "expectedDealTierSatisfied=%v when %v", test.expectedDealTierSatisfied, test.description)
if len(test.expectedDealErr) > 0 {
assert.Containsf(t, dealErrs, errors.New(test.expectedDealErr), "Expected error message not found in deal errors")
assert.Equal(t, test.expected.hbPbCatDur, bidCategory[auc.winningBidsByBidder["imp_id1"][test.in.bidderName][0].Bid.ID], test.description)
assert.Equal(t, test.expected.dealTierSatisfied, auc.winningBidsByBidder["imp_id1"][test.in.bidderName][0].DealTierSatisfied, "expected.dealTierSatisfied=%v when %v", test.expected.dealTierSatisfied, test.description)
if len(test.expected.dealErr) > 0 {
assert.Containsf(t, dealErrs, errors.New(test.expected.dealErr), "Expected error message not found in deal errors")
}
}
}
Expand Down Expand Up @@ -5940,3 +6016,65 @@ func TestBuildMultiBidMap(t *testing.T) {
}
}
}

func TestBidsToUpdate(t *testing.T) {
type testInput struct {
multiBid map[string]openrtb_ext.ExtMultiBid
bidder string
}
testCases := []struct {
desc string
in testInput
expected int
}{
{
desc: "Empty multibid map. Expect openrtb_ext.DefaultBidLimit",
in: testInput{},
expected: openrtb_ext.DefaultBidLimit,
},
{
desc: "Empty bidder. Expect openrtb_ext.DefaultBidLimit",
in: testInput{
multiBid: map[string]openrtb_ext.ExtMultiBid{
"appnexus": {
Bidder: "appnexus",
MaxBids: ptrutil.ToPtr(2),
},
},
},
expected: openrtb_ext.DefaultBidLimit,
},
{
desc: "bidder finds a match in multibid map but TargetBidderCodePrefix is empty. Expect openrtb_ext.DefaultBidLimit",
in: testInput{
multiBid: map[string]openrtb_ext.ExtMultiBid{
"appnexus": {
Bidder: "appnexus",
MaxBids: ptrutil.ToPtr(2),
},
},
bidder: "appnexus",
},
expected: openrtb_ext.DefaultBidLimit,
},
{
desc: "multibid element with non-empty TargetBidderCodePrefix matches bidder. Expect MaxBids value",
in: testInput{
multiBid: map[string]openrtb_ext.ExtMultiBid{
"appnexus": {
Bidder: "appnexus",
MaxBids: ptrutil.ToPtr(2),
TargetBidderCodePrefix: "aPrefix",
},
},
bidder: "appnexus",
},
expected: 2,
},
}
for _, tc := range testCases {
t.Run(tc.desc, func(t *testing.T) {
assert.Equal(t, tc.expected, bidsToUpdate(tc.in.multiBid, tc.in.bidder), tc.desc)
})
}
}
4 changes: 3 additions & 1 deletion openrtb_ext/deal_tier.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@ func ReadDealTiersFromImp(imp openrtb2.Imp) (DealTierBidderMap, error) {
}
for bidder, param := range impPrebidExt.Prebid.Bidders {
if param.DealTier != nil {
dealTiers[BidderName(bidder)] = *param.DealTier
if bidderNormalized, bidderFound := NormalizeBidderName(bidder); bidderFound {
dealTiers[bidderNormalized] = *param.DealTier
}
Comment on lines +41 to +43
Copy link
Collaborator

@bsardo bsardo Oct 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The deal tier map being created here from the original request has keys that are the normalized bidder names. This map is then used in applyDealSupport but that function does not appear to alter the bidder name in winningBidsByBidder topBidsPerImp prior to checking if the bidder has any deal tiers in the deal tier map. Is a change needed in applyDealSupport as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. Modified.

}
}

Expand Down
15 changes: 15 additions & 0 deletions openrtb_ext/deal_tier_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,16 @@ func TestReadDealTiersFromImp(t *testing.T) {
impExt: json.RawMessage(`{"prebid": {"bidder": {"appnexus": {"dealTier": {"minDealTier": 5, "prefix": "anyPrefix"}, "placementId": 12345}}}}`),
expectedResult: DealTierBidderMap{BidderAppnexus: {Prefix: "anyPrefix", MinDealTier: 5}},
},
{
description: "imp.ext.prebid.bidder - one but it's not found in the Adapter Bidder list",
impExt: json.RawMessage(`{"prebid": {"bidder": {"unknown": {"dealTier": {"minDealTier": 5, "prefix": "anyPrefix"}, "placementId": 12345}}}}`),
expectedResult: DealTierBidderMap{},
},
{
description: "imp.ext.prebid.bidder - one but case is different from the Adapter Bidder list",
impExt: json.RawMessage(`{"prebid": {"bidder": {"APpNExUS": {"dealTier": {"minDealTier": 5, "prefix": "anyPrefix"}, "placementId": 12345}}}}`),
expectedResult: DealTierBidderMap{BidderAppnexus: {Prefix: "anyPrefix", MinDealTier: 5}},
},
{
description: "imp.ext.prebid.bidder - one with other params",
impExt: json.RawMessage(`{"prebid": {"bidder": {"appnexus": {"dealTier": {"minDealTier": 5, "prefix": "anyPrefix"}, "placementId": 12345}}, "supportdeals": true}, "tid": "1234"}`),
Expand All @@ -66,6 +76,11 @@ func TestReadDealTiersFromImp(t *testing.T) {
impExt: json.RawMessage(`{"prebid": {"bidder": {"appnexus": {"dealTier": {"minDealTier": 5, "prefix": "appnexusPrefix"}, "placementId": 12345}, "rubicon": {"dealTier": {"minDealTier": 8, "prefix": "rubiconPrefix"}, "placementId": 12345}}}}`),
expectedResult: DealTierBidderMap{BidderAppnexus: {Prefix: "appnexusPrefix", MinDealTier: 5}, BidderRubicon: {Prefix: "rubiconPrefix", MinDealTier: 8}},
},
{
description: "imp.ext.prebid.bidder - same bidder listed twice but with different case the last one prevails",
impExt: json.RawMessage(`{"prebid": {"bidder": {"appnexus": {"dealTier": {"minDealTier": 100, "prefix": "appnexusPrefix"}, "placementId": 12345},"APpNExUS": {"dealTier": {"minDealTier": 5, "prefix": "APpNExUSPrefix"}, "placementId": 12345}}}}`),
expectedResult: DealTierBidderMap{BidderAppnexus: {Prefix: "APpNExUSPrefix", MinDealTier: 5}},
},
{
description: "imp.ext.prebid.bidder - one without deal tier",
impExt: json.RawMessage(`{"prebid": {"bidder": {"appnexus": {"placementId": 12345}}}}`),
Expand Down
Loading