ORTB 2.6: Test exchange json fixes and remove clear fields function calls #4010
Conversation
exchange/exchange_test.go
Outdated
| bidderInfos[string(bidderName)] = config.BidderInfo{ModifyingVastXmlAllowed: spec.ModifyingVastXmlAllowed} | ||
| ortbVersion, found := exSpec.ORTBVersion[string(bidderName)] | ||
| if !found { | ||
| ortbVersion = "2.5" |
There was a problem hiding this comment.
why not leave it blank if the string is not found? The vast majority of bidders today do not have a version value.
There was a problem hiding this comment.
Agree it would be preferable to leave it blank so it more accurately reflects actual configuration state.
There was a problem hiding this comment.
Just wanted to be more explicit. it will work both ways. Removed.
| "regs": { | ||
| "coppa": 1, | ||
| "ext": { | ||
| "gdpr": 1, | ||
| "us_privacy": "1YYY" | ||
| } | ||
| } |
There was a problem hiding this comment.
Since appnexus is declared as a 2.6 bidder, this should be:
"regs": {
"coppa": 1,
"gdpr": 1,
"us_privacy": "1YYY"
}
I expect this to cause the test to fail as we have identified a shared memory issue with respect to regs.
There was a problem hiding this comment.
Fixed input request and outgoing request for appnexus, good point!
There was a problem hiding this comment.
Just curious, was this test failing before? I don't see why it would have. I'm ok with the change you have here but if it was failing before I'd like to understand why.
There was a problem hiding this comment.
Discussed offline. 517ea0b addresses this.
VeronikaSolovei9
force-pushed
the
testExchangeJsonFix
branch
from
6accfef to
5f17008
Compare
| @@ -31,13 +31,6 @@ func ConvertDownTo25(r *RequestWrapper) error { | |||
| } | |||
| } | |||
|
|
|||
| // Remove fields introduced in OpenRTB 2.6+. The previous OpenRTB 2.5 spec did not specify that | |||
| // bidders must tolerate new or unexpected fields. | |||
| clear26Fields(r) | |||
There was a problem hiding this comment.
I thought we just want to drop the Imp fields?
There was a problem hiding this comment.
See #3613.
I think we want to delete these function calls, but not the functions themselves so adapters can use them, so that all 2.6 fields are passed through by default. Specific ext fields that are called out on that issue that need to be moved should be cleared when down converting (regs.ext.gdpr/us_privacy, source.ext.schain, user.ext.eids/consent). Note that imp.prebid.is_rewarded_inventoryshould be copied and not moved, in which case both it andimp.rwdd` should be passed through without clearing.
I suggest we merge this PR and then I can put up another PR that adds a function that only clears the moved ext fields during a down convert.
bsardo
changed the title
No description provided.