UserID module: better initialization logic; new global function #8201
Conversation
This updates the userID module to avoid premature initialization (prebid#5447): - `pbjs.getUserIds` and `pbjs.getUserIdsAsEids` no longer force initialization of the ID system - ID submodules are now initialized after after GDPR consent has been resolved *and* `userSync` configuration has been set - both `pbjs.getUserIds` and `pbjs.getUserIdsAsEids` now accept a callback that, if provided, is scheduled run after all ID submodules have filled in their data. Depending on `userSync` configuration, this may neeed to wait for an AUCTION_END event - `pbjs.refreshUserIds` now forces a fetch regardless of `userSync` config and returns a promise that completes when all ID submodules have filled in their data. `pbjs.refreshUserIds({refresh: false})` will return the promise without reinitialization.
patmmccann
requested review from
SKOCHERI and
smenzer
and removed request for
SKOCHERI
patmmccann
added
the
needs 2nd review
|
@dgirardi please note the merge conflict |
|
@dgirardi Thanks for this, looks promising. |
|
What about adding |
|
Sounds good, that would leave the current implementations that are working as they are... and whoever is having issues would switch to the async version. |
There was a problem hiding this comment.
Overall this looks good. I'm not an engineer so I can't say that every bit of code is written in the best way, but the logic makes sense and I like that it doesn't break existing implementations.
I would like to see the examples page updated to show how to properly use the pbjs.getUserIds() method with the new callback function, as well as any related docs (I know the docs PR is still pending). Once all of that is done, I'm happy to approve.
We will need another engineer to review and approve since this is core.
|
Documentation PR: prebid/prebid.github.io#3698 |
| // TODO: this whole suite needs to be redesigned; it is passing by accident | ||
| // some tests do not pass if consent data is available | ||
| // (there are functions here with signature `getId(config, storedId)`, but storedId is actually consentData) | ||
| // also, this file is ginormous; do we really need to test *all* id systems as one? | ||
| resetConsentData(); |
There was a problem hiding this comment.
I agree completely. a lot of this was from the first iteration of the user id module, so a lot of copy/paste from newer modules and signatures weren't necessarily updated as we made changes.
I think we need to have a few tests that do test all id systems for basic processing, but the majority of these tests could probably just be mock id systems to test core functionality
smenzer
removed
needs review
needs 2nd review
patmmccann
changed the title
* UserID module: better initialization logic This updates the userID module to avoid premature initialization (prebid#5447): - `pbjs.getUserIds` and `pbjs.getUserIdsAsEids` no longer force initialization of the ID system - ID submodules are now initialized after after GDPR consent has been resolved *and* `userSync` configuration has been set - both `pbjs.getUserIds` and `pbjs.getUserIdsAsEids` now accept a callback that, if provided, is scheduled run after all ID submodules have filled in their data. Depending on `userSync` configuration, this may neeed to wait for an AUCTION_END event - `pbjs.refreshUserIds` now forces a fetch regardless of `userSync` config and returns a promise that completes when all ID submodules have filled in their data. `pbjs.refreshUserIds({refresh: false})` will return the promise without reinitialization. * Add `pbjs.getUserIdsAsync` instead of tacking on callbacks to every other method * Remove out of date typedef
… infinite loop In some situations userID submodules can throw exceptions (prebid#8360) which then, after prebid#8201, causes the ID system to get stuck in an infinite loop.
… infinite loop (prebid#8362) In some situations userID submodules can throw exceptions (prebid#8360) which then, after prebid#8201, causes the ID system to get stuck in an infinite loop.
Type of change
Description of change
This updates the userID module to avoid premature initialization (#5447):
pbjs.getUserIdsandpbjs.getUserIdsAsEidsno longer force initialization of the ID systemuserSyncconfiguration has been setpbjs.getUserIdsAsync(), which returns a promise to user IDs, completing only once all ID submodules have completed initpbjs.refreshUserIdsnow forces an immediate refresh regardless ofuserSyncconfig and returns a promise that completes when all ID submodules have filled in their data.pbjs.refreshUserIds({refresh: false})will just return the promise without refreshing.Note that until #8185 gets merged, GDPR data cannot be resolved without an auction, so this will prevent any ID from being resolved if the consentManagement module is installed and no auction is started. No additional action should be required to fix this besides the merge.
Other information
Fixes #5447
Documentation PR: prebid/prebid.github.io#3698