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: ext.prebid.multibid #3217

Merged
merged 8 commits into from
Oct 18, 2023
Merged

Adapter Name Case Insensitive: ext.prebid.multibid #3217

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
e37ba8d
First commit
guscarreon Oct 13, 2023
b4963fd
Rolled back test changes
guscarreon Oct 13, 2023
61b73e0
buildRequestExtMultiBid normalized
guscarreon Oct 17, 2023
1a26df8
Corrected uppercase test
guscarreon Oct 17, 2023
c2f396b
jsonlinted exchange/exchangetest/multi-bids-mixed-case.json
guscarreon Oct 17, 2023
e152d85
rollback the use of openrtb_ext.NormalizeBidderName() in buildRequest…
guscarreon Oct 17, 2023
3036d8b
ifs
guscarreon Oct 17, 2023
134d8ed
Corrected ifs
guscarreon Oct 17, 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
12 changes: 10 additions & 2 deletions exchange/exchange.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -514,10 +514,18 @@ func buildMultiBidMap(prebid *openrtb_ext.ExtRequestPrebid) map[string]openrtb_e
multiBidMap := make(map[string]openrtb_ext.ExtMultiBid)
for _, multiBid := range prebid.MultiBid {
if multiBid.Bidder != "" {
multiBidMap[multiBid.Bidder] = *multiBid
bidderNormalized, bidderFound := openrtb_ext.NormalizeBidderName(multiBid.Bidder)
if !bidderFound {
continue
}
multiBidMap[string(bidderNormalized)] = *multiBid
} else {
for _, bidder := range multiBid.Bidders {
multiBidMap[bidder] = *multiBid
bidderNormalized, bidderFound := openrtb_ext.NormalizeBidderName(bidder)
if !bidderFound {
continue
}
multiBidMap[string(bidderNormalized)] = *multiBid
}
}
}
Expand Down
179 changes: 179 additions & 0 deletions exchange/exchange_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5750,3 +5750,182 @@ func TestSetSeatNonBid(t *testing.T) {
})
}
}

func TestBuildMultiBidMap(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you also a JSON test in /exchangetest similar to multi-bids.json, where we can test the case insensitivity there 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! That extra test revealed another spot where case insensitivity was needed inside function buildRequestExtMultiBid(). Added exchange/exchangetest/multi-bids-mixed-case.json.

type testCase struct {
desc string
inPrebid *openrtb_ext.ExtRequestPrebid
expected map[string]openrtb_ext.ExtMultiBid
}
testGroups := []struct {
groupDesc string
tests []testCase
}{
{
groupDesc: "Nil or empty tests",
tests: []testCase{
{
desc: "prebid nil, expect nil map",
inPrebid: nil,
expected: nil,
},
{
desc: "prebid.MultiBid nil, expect nil map",
inPrebid: &openrtb_ext.ExtRequestPrebid{},
expected: nil,
},
{
desc: "not-nil prebid.MultiBid is empty, expect empty map",
inPrebid: &openrtb_ext.ExtRequestPrebid{
MultiBid: []*openrtb_ext.ExtMultiBid{},
},
expected: map[string]openrtb_ext.ExtMultiBid{},
},
},
},
{
groupDesc: "prebid.MultiBid.Bidder tests",
tests: []testCase{
{
desc: "Lowercase prebid.MultiBid.Bidder is found in the BidderName list, entry is mapped",
inPrebid: &openrtb_ext.ExtRequestPrebid{
MultiBid: []*openrtb_ext.ExtMultiBid{
{Bidder: "appnexus"},
},
},
expected: map[string]openrtb_ext.ExtMultiBid{
"appnexus": {Bidder: "appnexus"},
},
Comment on lines +5795 to +5803
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add another test case just like this, except the Bidder is capitalized: (i.e APPNEXUS)

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

},
{
desc: "Lowercase prebid.MultiBid.Bidder is not found in the BidderName list, expect empty map",
inPrebid: &openrtb_ext.ExtRequestPrebid{
MultiBid: []*openrtb_ext.ExtMultiBid{
{Bidder: "unknown"},
},
},
expected: map[string]openrtb_ext.ExtMultiBid{},
},
{
desc: "Mixed-case prebid.MultiBid.Bidder is not found in the BidderName list, expect empty map",
inPrebid: &openrtb_ext.ExtRequestPrebid{
MultiBid: []*openrtb_ext.ExtMultiBid{
{Bidder: "UnknownBidder"},
},
},
expected: map[string]openrtb_ext.ExtMultiBid{},
},
{
desc: "Different-cased prebid.MultiBid.Bidder entries that refer to the same adapter are found in the BidderName list are mapped once",
inPrebid: &openrtb_ext.ExtRequestPrebid{
MultiBid: []*openrtb_ext.ExtMultiBid{
{Bidder: "AppNexus"},
{Bidder: "appnexus"},
},
},
expected: map[string]openrtb_ext.ExtMultiBid{
"appnexus": {Bidder: "appnexus"},
},
},
},
},
{
groupDesc: "prebid.MultiBid.Bidders tests",
tests: []testCase{
{
desc: "Lowercase prebid.MultiBid.Bidder is found in the BidderName list, entry is mapped",
inPrebid: &openrtb_ext.ExtRequestPrebid{
MultiBid: []*openrtb_ext.ExtMultiBid{
{Bidders: []string{"appnexus"}},
},
},
expected: map[string]openrtb_ext.ExtMultiBid{
"appnexus": {
Bidders: []string{"appnexus"},
},
},
},
{
desc: "Lowercase prebid.MultiBid.Bidder is not found in the BidderName list, expect empty map",
inPrebid: &openrtb_ext.ExtRequestPrebid{
MultiBid: []*openrtb_ext.ExtMultiBid{
{Bidders: []string{"unknown"}},
},
},
expected: map[string]openrtb_ext.ExtMultiBid{},
},
{
desc: "Mixed-case prebid.MultiBid.Bidder is not found in the BidderName list, expect empty map",
inPrebid: &openrtb_ext.ExtRequestPrebid{
MultiBid: []*openrtb_ext.ExtMultiBid{
{Bidders: []string{"UnknownBidder"}},
},
},
expected: map[string]openrtb_ext.ExtMultiBid{},
},
{
desc: "Different-cased prebid.MultiBid.Bidder entries that refer to the same adapter are found in the BidderName list are mapped once",
inPrebid: &openrtb_ext.ExtRequestPrebid{
MultiBid: []*openrtb_ext.ExtMultiBid{
{Bidders: []string{"AppNexus", "appnexus", "UnknownBidder"}},
},
},
expected: map[string]openrtb_ext.ExtMultiBid{
"appnexus": {
Bidders: []string{"AppNexus", "appnexus", "UnknownBidder"},
},
},
},
},
},
{
groupDesc: "prebid.MultiBid.Bidder and prebid.MultiBid.Bidders entries in tests",
tests: []testCase{
{
desc: "If prebid.MultiBid.Bidder is found ignore entries in prebid.MultiBid.Bidders, even if its unknown",
inPrebid: &openrtb_ext.ExtRequestPrebid{
MultiBid: []*openrtb_ext.ExtMultiBid{
{
Bidder: "UnknownBidder",
Bidders: []string{"appnexus", "rubicon", "pubmatic"},
},
},
},
expected: map[string]openrtb_ext.ExtMultiBid{},
},
{
desc: "If prebid.MultiBid.Bidder is found in one entry, prebid.MultiBid.Bidders in another. Add all to map",
inPrebid: &openrtb_ext.ExtRequestPrebid{
MultiBid: []*openrtb_ext.ExtMultiBid{
{
Bidder: "pubmatic",
Bidders: []string{"appnexus", "rubicon", "UnknownBidder"},
},
{
Bidders: []string{"UnknownBidder", "appnexus", "rubicon"},
},
},
},
expected: map[string]openrtb_ext.ExtMultiBid{
"pubmatic": {
Bidder: "pubmatic",
Bidders: []string{"appnexus", "rubicon", "UnknownBidder"},
},
"appnexus": {
Bidders: []string{"UnknownBidder", "appnexus", "rubicon"},
},
"rubicon": {
Bidders: []string{"UnknownBidder", "appnexus", "rubicon"},
},
},
},
},
},
}
for _, groups := range testGroups {
for _, tc := range groups.tests {
multiBidMap := buildMultiBidMap(tc.inPrebid)
assert.Equal(t, tc.expected, multiBidMap, tc.desc)
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add t.Run

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

}
}
}
Loading