Hoist GVL ID To Bidder Info #1721
Conversation
| @@ -246,7 +245,8 @@ func New(cfg *config.Configuration, rateConvertor *currency.RateConverter) (r *R | |||
| } | |||
|
|
|||
| syncers := usersyncers.NewSyncerMap(cfg) | |||
| gdprPerms := gdpr.NewPermissions(context.Background(), cfg.GDPR, adapters.GDPRAwareSyncerIDs(syncers), generalHttpClient) | |||
| gvlVendorIDs := bidderInfos.ToGVLVendorIDMap() | |||
There was a problem hiding this comment.
Not that it really matters, but is there a reason that you took bidderInfos.ToGVLVendorIDMap() out into its own line?
There was a problem hiding this comment.
To make the gdprPerms line a bit shorter and to make it 100% clear that we're passing through the vendor ids.
| @@ -132,31 +132,3 @@ func TestNewSyncerMap(t *testing.T) { | |||
| } | |||
| } | |||
| } | |||
|
|
|||
| // Bidders may have an ID on the IAB-maintained global vendor list. | |||
There was a problem hiding this comment.
Do we want to move this uniqueness test to config/bidderinfos_test.go or just drop it entirely? We can run it against the default configs in the static/ directory.
There was a problem hiding this comment.
I removed the test because it's broken and the intent of the test is flawed.
Broken
The test uses an empty configuration object which results in empty sync urls which in turn causes the NewSyncerMap method to return an empty map. This has been broken for at least 2 years.
Flawed
The test looks to ensure each bidder has it's own unique vendor id. However, we have adapters like adkernel/adkernelAdn, brightroll/verizonmedia, and triplelift/triplelift_native which legitimately need to have the same vendor id.
I suppose the original intent was to eliminate copy / paste errors, but I don't see how we can continue to automate a check like this without allowing for the legitimate cases.
We have also become more thorough in vetting vendor ids during the review process.
Good point. To ensure I didn't make a mistake, I serialized the map from the master branch and this PR and used a tool to compare. I can confirm there are no mistakes. The new mapping now includes (intentionally) vendor ids for bidders which don't have a default value. Specifically: adocean, dmx, invibes, ix, rtbhouse, rubicon, verizonmedia. |
|
docs PR prebid/prebid.github.io#2750 |
This is the first step in transforming the user syncer from code into pure config, while also adding in the choice between both iframe and redirect.