-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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.
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
Prebid Core: switch native assets to ortb2 format #8086
Merged
+1,652
−396
Merged
Changes from all commits
Commits
Show all changes
46 commits
Select commit
Hold shift + click to select a range
8a79cdd
switch native assets to ortb2 format
FilipStamenkovic 3def5dc
put rendererUrl, adTemplate back in message
FilipStamenkovic 48f06e6
rename ortb2 to ortb
FilipStamenkovic d8e08c2
typo fix
FilipStamenkovic 639a648
changes to support ortb for native media type
FilipStamenkovic 29eef9c
handle bid won case
FilipStamenkovic 96046ed
mark bid as won on any post message for native
FilipStamenkovic bc7423d
fix tests for bid_won event for native
FilipStamenkovic 6cab304
add missing imports
FilipStamenkovic 2b66a39
native converting functions
d2d79dc
convert new native ortb media type object to proprietary format for n…
c9c0203
fix LGTM.com issue
4666c4c
added comments; minor fixes
526db5e
added test for convertOrtbRequestToProprietaryNative + fixes
d487ed0
add nativeParams to conversion
f8e6e49
support nativeParams
baa6cf6
check that when native.ortb is present, it's the only property
1096207
remove commented code
18fa58b
removed unnecessary tests
1ae5be1
added test that checks that BID_WON is not fired more than once for t…
a2c5866
validation is now performed on ortb data only
685d298
fix for prebidServer_native_example.html
20cb53c
PrebidServerBidAdapter also responds in ortb format
948daef
fix aspect_ratios as an array in tests
5ca7b8d
LGTM fix - remove unused variables
194fc95
Better name for native constants
eceeead
use WeakSet instead of Set
bc1bbf1
when native request is openRTB, the whole native.ortb is passed over
5957c98
retain some defaults dor native PBS request
44cc459
fix for empty ortbRequest in nativeBidIsValid
5c227f9
add non-asset properties to media type object
55b0ae0
final fixes after rebasing
0903aed
handle tracking of soon-deprecated ortb imptrackers and jstracker
musikele 9ee2ec8
let native ad unit take all horizontal space
69d80df
pass "mapping" from legacy native to ortb to prebid universal creative
366e572
Convert ortb assets to legacy for backward compatibility with legacy …
e273140
add ortb conversion for more bid adapters
6991d2b
wrap conversion function in FEATURES.NATIVE
39ed9d3
remove expired bids from native wrapper
ef807e7
instead of modifying bidresponse, use nativeReq
6ef7e1d
wrap test in FEATURES.NATIVE
ca8747f
fix for native mapping in prebidServerBidAdapter
3479bf1
use copy-on-write to convert ortb requests to legacy
1155c10
fix comment on function
ce6b8f3
Merge branch 'master' into native_assets_ortb2
patmmccann 74338ad
Update criteoBidAdapter.js
patmmccann File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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.
This looks like it is overwriting the incoming bidRequests. Should it be just creating a copy instead here? Not sure is this affects downstream prebid core if these input bidRequests object are changed?
For example, if the input has rendererUrl / sendTargetingKeys: false
Then this function overwrites them and they are removed, does the core still handle the bidResponse correctly when it maps back to ortb2 etc?
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.
Looks like the function changes the incoming object anyways, so prob don't need to set it equal to it if it changes it anyways?
Maybe a copy should be returned so the original
bidRequests
object is not changed?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.
Hi @robertrmartinez
You are right, it is overwriting the incoming BidRequests. I tried to create a copy and pass it around, and this would be more correct (from a functional standpoint) and may lead to less bugs, however it is unpractical.
Some bidders in their tests rely on the fact that the
bidRequest
object they sent is exactly the one that will come back; some check the reference, other check some properties that they are attaching to the object; in the end, creating a copy ofbidRequests
instead of modifying it in place would mean to fix all their tests.Regarding the fact that rendererUrl/sendTargetingKeys may disappear: I'll do a change in the code so this will not happen.
To summarize, my idea is that we should create a copy, but this is impossible now, because it will break all the tests that assume that the bidRequest they're testing is the same they sent in input.
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.
@robertrmartinez check the code now!
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.
That all makes sense to me!
Hopefully this is a short term solution and ORTB gets fully adopted and legacy native mediatype is deprecated!
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.
@musikele would a compromise of "copy-on-write" work? since tests should not be setting up native ortb2 right now, if you return the original object when no conversion is necessary, but clone it when it is, I think it might work?
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.
@dgirardi I've implemented copy-on-write and tests are not failing.