Fix: Triplelift Native add site.publisher nil check #3824
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| package triplelift_native | ||
|
|
||
| import ( | ||
| "encoding/json" | ||
| "testing" | ||
|
|
||
| "github.com/prebid/prebid-server/v2/openrtb_ext" | ||
| ) | ||
|
|
||
| func TestValidParams(t *testing.T) { | ||
| validator, err := openrtb_ext.NewBidderParamsValidator("../../static/bidder-params") | ||
| if err != nil { | ||
| t.Fatalf("Failed to fetch the json-schemas. %v", err) | ||
| } | ||
|
|
||
| for _, validParam := range validParams { | ||
| if err := validator.Validate(openrtb_ext.BidderTripleliftNative, json.RawMessage(validParam)); err != nil { | ||
| t.Errorf("Schema rejected triplelift native params: %s", validParam) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| func TestInvalidParams(t *testing.T) { | ||
| validator, err := openrtb_ext.NewBidderParamsValidator("../../static/bidder-params") | ||
| if err != nil { | ||
| t.Fatalf("Failed to fetch the json-schemas. %v", err) | ||
| } | ||
|
|
||
| for _, invalidParam := range invalidParams { | ||
| if err := validator.Validate(openrtb_ext.BidderTripleliftNative, json.RawMessage(invalidParam)); err == nil { | ||
| t.Errorf("Schema allowed unexpected params: %s", invalidParam) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| var validParams = []string{ | ||
| `{"inventoryCode":"1"}`, | ||
| `{"inventoryCode":"test"}`, | ||
| `{"inventoryCode":"test", "floor":10}`, | ||
| } | ||
|
|
||
| var invalidParams = []string{ | ||
| ``, | ||
| `null`, | ||
| `true`, | ||
| `5`, | ||
| `4.2`, | ||
| `[]`, | ||
| `{}`, | ||
| `{"inventoryCode":1}`, | ||
| `{"inventoryCode":""}`, | ||
| `{"inventoryCode":"1", "floor": "10"}`, | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -50,12 +50,6 @@ func processImp(imp *openrtb2.Imp, request *openrtb2.BidRequest) error { | |
| // get the triplelift extension | ||
| var ext ExtImp | ||
| var tlext openrtb_ext.ExtImpTriplelift | ||
|
|
||
| if err := json.Unmarshal(imp.Ext, &ext); err != nil { | ||
| return err | ||
|
|
@@ -66,32 +60,32 @@ func processImp(imp *openrtb2.Imp, request *openrtb2.BidRequest) error { | |
| if imp.Native == nil { | ||
| return fmt.Errorf("no native object specified") | ||
| } | ||
SyntaxNode marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
|
Comment on lines
-73
to
-82
There was a problem hiding this comment. This logic has been simplified as pointed out by @guscarreon. |
||
| if ext.Data != nil && len(ext.Data.TagCode) > 0 && (msnInSite(request) || msnInApp(request)) { | ||
| imp.TagID = ext.Data.TagCode | ||
| } else { | ||
| imp.TagID = tlext.InvCode | ||
| } | ||
|
|
||
| // floor is optional | ||
| if tlext.Floor == nil { | ||
| return nil | ||
| } | ||
| imp.BidFloor = *tlext.Floor | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| // msnInApp returns whether msn.com is in request.app.publisher.domain | ||
| func msnInApp(request *openrtb2.BidRequest) bool { | ||
| return request.App != nil && request.App.Publisher != nil && request.App.Publisher.Domain == "msn.com" | ||
| } | ||
|
Comment on lines
+80
to
+82
There was a problem hiding this comment. Adding support for app.publisher as discussed. |
||
|
|
||
| // msnInSite returns whether msn.com is in request.site.publisher.domain | ||
| func msnInSite(request *openrtb2.BidRequest) bool { | ||
| return request.Site != nil && request.Site.Publisher != nil && request.Site.Publisher.Domain == "msn.com" | ||
|
There was a problem hiding this comment. Nil checks added to address panics. |
||
| } | ||
|
|
||
| // Returns the effective publisher ID | ||
| func effectivePubID(pub *openrtb2.Publisher) string { | ||
| if pub != nil { | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,112 @@ | ||
| { | ||
| "mockBidRequest": { | ||
| "id": "test-request-id", | ||
| "app": { | ||
| "publisher": { | ||
| "id": "foo", | ||
| "name": "foo", | ||
| "domain": "msn.com" | ||
| } | ||
| }, | ||
| "imp": [{ | ||
| "native": { | ||
| "request": "{\"plcmtcnt\":1,\"plcmttype\":2,\"privacy\":1,\"context\":1,\"contextsubtype\":12,\"eventtrackers\":[{\"event\":1,\"methods\":[1,2]},{\"event\":2,\"methods\":[1]}],\"assets\":[{\"data\":{\"type\":12},\"required\":1},{\"title\":{\"len\":50},\"required\":1},{\"img\":{\"w\":80,\"h\":80,\"type\":1},\"required\":1},{\"img\":{\"w\":1200,\"h\":627,\"type\":3},\"required\":1},{\"data\":{\"type\":3},\"required\":0},{\"data\":{\"len\":100,\"type\":2},\"required\":1},{\"video\":{\"mimes\":[\"video/mpeg\",\"video/mp4\"],\"minduration\":2,\"protocols\":[2,5],\"maxduration\":2,\"ext\":{\"playbackmethod\":[1,2]}},\"required\":1}],\"ver\":\"1.2\"}" | ||
| }, | ||
| "id": "test-imp-id", | ||
| "ext": { | ||
| "bidder": { | ||
| "inventoryCode": "invcode", | ||
| "floor": 20 | ||
| }, | ||
| "data": { | ||
| "tag_code": "" | ||
| } | ||
| } | ||
| }] | ||
| }, | ||
| "httpCalls": [{ | ||
| "expectedRequest": { | ||
| "uri": "http://tlx.3lift.net/s2sn/auction?supplier_id=20", | ||
| "body": { | ||
| "id": "test-request-id", | ||
| "imp": [{ | ||
| "id": "test-imp-id", | ||
| "native": { | ||
| "request": "{\"plcmtcnt\":1,\"plcmttype\":2,\"privacy\":1,\"context\":1,\"contextsubtype\":12,\"eventtrackers\":[{\"event\":1,\"methods\":[1,2]},{\"event\":2,\"methods\":[1]}],\"assets\":[{\"data\":{\"type\":12},\"required\":1},{\"title\":{\"len\":50},\"required\":1},{\"img\":{\"w\":80,\"h\":80,\"type\":1},\"required\":1},{\"img\":{\"w\":1200,\"h\":627,\"type\":3},\"required\":1},{\"data\":{\"type\":3},\"required\":0},{\"data\":{\"len\":100,\"type\":2},\"required\":1},{\"video\":{\"mimes\":[\"video/mpeg\",\"video/mp4\"],\"minduration\":2,\"protocols\":[2,5],\"maxduration\":2,\"ext\":{\"playbackmethod\":[1,2]}},\"required\":1}],\"ver\":\"1.2\"}" | ||
| }, | ||
| "tagid": "invcode", | ||
| "bidfloor": 20, | ||
| "ext": { | ||
| "bidder": { | ||
| "inventoryCode": "invcode", | ||
| "floor": 20 | ||
| }, | ||
| "data": { | ||
| "tag_code": "" | ||
| } | ||
| } | ||
| }], | ||
| "app": { | ||
| "publisher": { | ||
| "id": "foo", | ||
| "name": "foo", | ||
| "domain": "msn.com" | ||
| } | ||
| } | ||
| }, | ||
| "impIDs": ["test-imp-id"] | ||
| }, | ||
| "mockResponse": { | ||
| "status": 200, | ||
| "body": { | ||
| "id": "test-request-id", | ||
| "seatbid": [{ | ||
| "seat": "958", | ||
| "bid": [{ | ||
| "id": "7706636740145184841", | ||
| "impid": "test-imp-id", | ||
| "price": 0.5, | ||
| "adid": "29681110", | ||
| "adm": "some-test-ad", | ||
| "adomain": [ | ||
| "triplelift.com" | ||
| ], | ||
| "iurl": "http://example.com", | ||
| "cid": "958", | ||
| "crid": "29681110", | ||
| "h": 250, | ||
| "w": 300, | ||
| "ext": "aaa" | ||
| }] | ||
| }], | ||
| "bidid": "5778926625248726496", | ||
| "cur": "USD" | ||
| } | ||
| } | ||
| }], | ||
| "expectedBidResponses": [{ | ||
| "currency": "USD", | ||
| "bids": [{ | ||
| "id": "test-request-id", | ||
| "type": "native", | ||
| "bid": { | ||
| "id": "7706636740145184841", | ||
| "impid": "test-imp-id", | ||
| "price": 0.5, | ||
| "adid": "29681110", | ||
| "adm": "some-test-ad", | ||
| "adomain": [ | ||
| "triplelift.com" | ||
| ], | ||
| "iurl": "http://example.com", | ||
| "cid": "958", | ||
| "crid": "29681110", | ||
| "h": 250, | ||
| "w": 300, | ||
| "ext": "aaa" | ||
| }, | ||
| "bidid": "5778926625248726496", | ||
| "cur": "USD" | ||
| }] | ||
| }] | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,112 @@ | ||
| { | ||
| "mockBidRequest": { | ||
| "id": "test-request-id", | ||
| "app": { | ||
| "publisher": { | ||
| "id": "foo", | ||
| "name": "foo", | ||
| "domain": "msn.com" | ||
| } | ||
| }, | ||
| "imp": [{ | ||
| "native": { | ||
| "request": "{\"plcmtcnt\":1,\"plcmttype\":2,\"privacy\":1,\"context\":1,\"contextsubtype\":12,\"eventtrackers\":[{\"event\":1,\"methods\":[1,2]},{\"event\":2,\"methods\":[1]}],\"assets\":[{\"data\":{\"type\":12},\"required\":1},{\"title\":{\"len\":50},\"required\":1},{\"img\":{\"w\":80,\"h\":80,\"type\":1},\"required\":1},{\"img\":{\"w\":1200,\"h\":627,\"type\":3},\"required\":1},{\"data\":{\"type\":3},\"required\":0},{\"data\":{\"len\":100,\"type\":2},\"required\":1},{\"video\":{\"mimes\":[\"video/mpeg\",\"video/mp4\"],\"minduration\":2,\"protocols\":[2,5],\"maxduration\":2,\"ext\":{\"playbackmethod\":[1,2]}},\"required\":1}],\"ver\":\"1.2\"}" | ||
| }, | ||
| "id": "test-imp-id", | ||
| "ext": { | ||
| "bidder": { | ||
| "inventoryCode": "invcode", | ||
| "floor": 20 | ||
| }, | ||
| "data": { | ||
| "tag_code": "bar" | ||
| } | ||
| } | ||
| }] | ||
| }, | ||
| "httpCalls": [{ | ||
| "expectedRequest": { | ||
| "uri": "http://tlx.3lift.net/s2sn/auction?supplier_id=20", | ||
| "body": { | ||
| "id": "test-request-id", | ||
| "imp": [{ | ||
| "id": "test-imp-id", | ||
| "native": { | ||
| "request": "{\"plcmtcnt\":1,\"plcmttype\":2,\"privacy\":1,\"context\":1,\"contextsubtype\":12,\"eventtrackers\":[{\"event\":1,\"methods\":[1,2]},{\"event\":2,\"methods\":[1]}],\"assets\":[{\"data\":{\"type\":12},\"required\":1},{\"title\":{\"len\":50},\"required\":1},{\"img\":{\"w\":80,\"h\":80,\"type\":1},\"required\":1},{\"img\":{\"w\":1200,\"h\":627,\"type\":3},\"required\":1},{\"data\":{\"type\":3},\"required\":0},{\"data\":{\"len\":100,\"type\":2},\"required\":1},{\"video\":{\"mimes\":[\"video/mpeg\",\"video/mp4\"],\"minduration\":2,\"protocols\":[2,5],\"maxduration\":2,\"ext\":{\"playbackmethod\":[1,2]}},\"required\":1}],\"ver\":\"1.2\"}" | ||
| }, | ||
| "tagid": "bar", | ||
| "bidfloor": 20, | ||
| "ext": { | ||
| "bidder": { | ||
| "inventoryCode": "invcode", | ||
| "floor": 20 | ||
| }, | ||
| "data": { | ||
| "tag_code": "bar" | ||
| } | ||
| } | ||
| }], | ||
| "app": { | ||
| "publisher": { | ||
| "id": "foo", | ||
| "name": "foo", | ||
| "domain": "msn.com" | ||
| } | ||
| } | ||
| }, | ||
| "impIDs": ["test-imp-id"] | ||
| }, | ||
| "mockResponse": { | ||
| "status": 200, | ||
| "body": { | ||
| "id": "test-request-id", | ||
| "seatbid": [{ | ||
| "seat": "958", | ||
| "bid": [{ | ||
| "id": "7706636740145184841", | ||
| "impid": "test-imp-id", | ||
| "price": 0.5, | ||
| "adid": "29681110", | ||
| "adm": "some-test-ad", | ||
| "adomain": [ | ||
| "triplelift.com" | ||
| ], | ||
| "iurl": "http://example.com", | ||
| "cid": "958", | ||
| "crid": "29681110", | ||
| "h": 250, | ||
| "w": 300, | ||
| "ext": "aaa" | ||
| }] | ||
| }], | ||
| "bidid": "5778926625248726496", | ||
| "cur": "USD" | ||
| } | ||
| } | ||
| }], | ||
| "expectedBidResponses": [{ | ||
| "currency": "USD", | ||
| "bids": [{ | ||
| "id": "test-request-id", | ||
| "type": "native", | ||
| "bid": { | ||
| "id": "7706636740145184841", | ||
| "impid": "test-imp-id", | ||
| "price": 0.5, | ||
| "adid": "29681110", | ||
| "adm": "some-test-ad", | ||
| "adomain": [ | ||
| "triplelift.com" | ||
| ], | ||
| "iurl": "http://example.com", | ||
| "cid": "958", | ||
| "crid": "29681110", | ||
| "h": 250, | ||
| "w": 300, | ||
| "ext": "aaa" | ||
| }, | ||
| "bidid": "5778926625248726496", | ||
| "cur": "USD" | ||
| }] | ||
| }] | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As pointed out by @guscarreon, these two variables are only ever read; they aren't written so we can get rid of them and just read the data from the original source.