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 all 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
8 changes: 6 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,14 @@ 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
if bidderNormalized, bidderFound := openrtb_ext.NormalizeBidderName(multiBid.Bidder); bidderFound {
multiBidMap[string(bidderNormalized)] = *multiBid
}
} else {
for _, bidder := range multiBid.Bidders {
multiBidMap[bidder] = *multiBid
if bidderNormalized, bidderFound := openrtb_ext.NormalizeBidderName(bidder); bidderFound {
multiBidMap[string(bidderNormalized)] = *multiBid
}
}
}
}
Expand Down
197 changes: 197 additions & 0 deletions exchange/exchange_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2259,8 +2259,13 @@ func TestTimeoutComputation(t *testing.T) {
func TestExchangeJSON(t *testing.T) {
if specFiles, err := os.ReadDir("./exchangetest"); err == nil {
for _, specFile := range specFiles {
if !strings.HasSuffix(specFile.Name(), ".json") {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added so vim's .swp files wouldn't error. Can remove if you want

continue
}

fileName := "./exchangetest/" + specFile.Name()
fileDisplayName := "exchange/exchangetest/" + specFile.Name()

t.Run(fileDisplayName, func(t *testing.T) {
specData, err := loadFile(fileName)
if assert.NoError(t, err, "Failed to load contents of file %s: %v", fileDisplayName, err) {
Expand Down Expand Up @@ -5750,3 +5755,195 @@ 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: "Uppercase 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"},
},
},
{
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: "prebid.MultiBid.Bidder 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: "prebid.MultiBid.Bidder 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 _, group := range testGroups {
for _, tc := range group.tests {
t.Run(group.groupDesc+tc.desc, func(t *testing.T) {
multiBidMap := buildMultiBidMap(tc.inPrebid)
assert.Equal(t, tc.expected, multiBidMap, tc.desc)
})
}
}
}
Loading
Loading