Floxis Bid Adapter: first-party fallback id (user.ext.floxisId) - #15295
Floxis Bid Adapter: first-party fallback id (user.ext.floxisId)#15295floxis-admin wants to merge 6 commits into
Conversation
Barecheck - Code coverage reportTotal: 96.67%Your code coverage diff: 0.00% ▴ ✅ All code changes are covered |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c95a1c89c7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…Floors Import priceFloors in the spec (matches ortbConverter README guidance and the pattern used by openx, showheroies, adf, and other adapter specs) so the CI coverage environment is reproduced locally. Add a test that returns a floor without a currency field — this bypasses priceFloors' tryGetFloor() currency-null guard, leaving imp.bidfloor unset and forcing the adapter's own getFloor branch (L164-165) to execute. Closes the barecheck gap reported on prebid#15295.
4c24970 to
66f559b
Compare
…Floors Import priceFloors in the spec (matches ortbConverter README guidance and the pattern used by openx, showheroies, adf, and other adapter specs) so the CI coverage environment is reproduced locally. Add a test that returns a floor without a currency field — this bypasses priceFloors' tryGetFloor() currency-null guard, leaving imp.bidfloor unset and forcing the adapter's own getFloor branch (L164-165) to execute. Closes the barecheck gap reported on prebid#15295.
|
Rebased on latest master: the 11.23.0 release regenerated adapter metadata, which had put this PR into a conflicting state. Our published disclosure file already carries the @robertrmartinez @gwhigs — ready for review whenever you have a chance; happy to turn any feedback around quickly. Thanks! |
|
@patmmccann — would you be able to take a look, or point us to the right reviewer? This has been open since June 3 without a review yet. It adds a first-party fallback id (
CI is green and the branch is mergeable. Any guidance on next steps would be much appreciated — thanks! |
|
Bumping this — all checks are green, and have been since the July 10 rebase. (The 11.23.0 release regenerated adapter metadata and briefly put this PR into conflict; our published disclosure file already carried the This has been open since June 3 without a first review. @patmmccann @dgirardi — could one of you take a pass, or point us to whoever currently owns first-party-id / storage review? If there's anything on our end that would speed it up — a different shape for the storage disclosure, or something else — we're glad to make the change. A rough ETA would also help us plan: the two PBS adapters (prebid/prebid-server#4811, prebid/prebid-server-java#4529) and both docs PRs are waiting on this set to land. |
|
@floxis-admin hi sorry about the delay. We have been having discussions about this id in storage from bidders thing. And we are leaning to not allow bidders to do it any longer. This is still being hashed out on how / what it will look like but you can track here: #15247 However, because we have allowed bidders in past to merge similar changes (by accident or not) we will allow this for now. Just note that this feature may have to be removed in some future MAJOR prebid release (aiming for Prebid 12 perhaps) Hope that makes sense. I will add a review today for the code. |
| } | ||
| }); | ||
| if (!req.user?.ext?.floxisId) { | ||
| const floxisId = getOrCreateFloxisId(); |
There was a problem hiding this comment.
this is inside the converter's request() hook, and buildRequests calls toORTB once per seat|region|partner group down at groups.map(...).
so a pub running three seats gets three trips through this on one auction
ID is same for all 3 I think so we should just resolve the floxisId once per auction, right before groups.map ??
Than we can pass it in via context!
// resolve once per auction, not once per seat group
const floxisId = getOrCreateFloxisId();
...
return groups.map((groupedBidRequests) => {
...
data: CONVERTER.toORTB({ bidRequests: groupedBidRequests, bidderRequest, context: { floxisId } }),There was a problem hiding this comment.
Done in 64596fb — resolution moved to a memoized resolver created once in buildRequests and passed through the converter context, so a multi-seat auction now does one storage round-trip instead of one per group. Kept it lazy (context carries the resolver, not the resolved value) so an auction where FPD already supplies user.ext.floxisId still touches storage zero times; details in the PR comment.
| // Never returns an id that didn't persist (no store, or storageControl denying the key while the | ||
| // keyless enablement checks pass) — an unpersisted id would rotate every auction, worse than none. | ||
| function getOrCreateFloxisId() { | ||
| if (typeof window === 'undefined') return null; |
There was a problem hiding this comment.
Not sure this is needed this all runs in browser, and storageManager grabs docuement so if no window we have other problems. fine to keep but no other adapter does similar checks from what I can tell
There was a problem hiding this comment.
Removed in 64596fb. You were right that it was dead code — the surrounding try/catch already returns null if the storage accessors throw, so the guard never changed the outcome.
robertrmartinez
left a comment
There was a problem hiding this comment.
couple minor things
|
Thanks @robertrmartinez — both addressed in 1. Resolve the id once per auction. You're right, and the repetition was worse than it looks: it wasn't just N I went with your const resolveFloxisId = createFloxisIdResolver();
...
data: CONVERTER.toORTB({ bidRequests: groupedBidRequests, bidderRequest, context: { resolveFloxisId } }),The reason it's lazy rather than resolved eagerly above 2. The New test On #15247 — that makes sense, and thanks for the context. We'd rather be aligned with where Prebid is heading than grandfathered against it, so: understood and accepted that this may need to come out in a major release, and we won't treat it as a durable guarantee. If the shared-library approach lands, we're glad to migrate to it, and happy to be a test case if that's useful. Worth noting our situation is a bit narrower than the general pattern — this is a bidder-scoped fallback that only matters where the third-party |
|
This PR introduces changes that may not work on all browsers. According to Babel, the following polyfills may be needed, and they are not automatically included:
The best way to address this is to provide good test coverage, as normal PR checks run unit tests on older browsers. |
Mints a first-party UUID in the publisher's page context (localStorage + cookie, ~30d) as an identity-of-last-resort for cookieless browsers where the third-party __fxId cookie is blocked. Additive via mergeDeep; never overwrites an existing user.ext.floxisId; degrades to a safe no-op when storage access is disallowed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LroYxzKYQwfC21pPcUNhjn
…Floors Import priceFloors in the spec (matches ortbConverter README guidance and the pattern used by openx, showheroies, adf, and other adapter specs) so the CI coverage environment is reproduced locally. Add a test that returns a floor without a currency field — this bypasses priceFloors' tryGetFloor() currency-null guard, leaving imp.bidfloor unset and forcing the adapter's own getFloor branch (L164-165) to execute. Closes the barecheck gap reported on prebid#15295.
…e flx_uid storageControl in strict mode denies key-specific get/set while the keyless enablement checks still pass, so the adapter minted a fresh UUID every auction that never persisted. A minted id now has to survive a read-back or nothing is sent, and flx_uid (cookie + localStorage) is disclosed at the vendor disclosure URL / embedded metadata.
…per seat group buildRequests groups bids by seat|region|partner and calls toORTB per group, so the converter's request() hook ran getOrCreateFloxisId() once per group — a publisher on three seats did three storage round-trips per auction for an id that is identical across them. Resolution moves to a memoized resolver created once in buildRequests and passed through the converter context. The resolver is lazy rather than eagerly resolved before groups.map so that an auction whose groups all carry an FPD-supplied user.ext.floxisId still touches storage zero times; an eager hoist would mint and persist an id that is never sent. Also drops the typeof window guard in getOrCreateFloxisId: the surrounding try/catch already yields null if the storage accessors throw, so the guard was unreachable defensive code.
64596fb to
ba17ad9
Compare
|
Rebased, no code changes. The Safari E2E red was #15422 — it dropped the STP install in favour of bundled Safari, and this branch was still on a Jul 10 base, so it kept asking for a browser the runner no longer has. Rebase picks up the new config. Heads up in case other older PRs start failing the same way. |
|
Anything still outstanding here? Both review points went in on 4 August — the id resolves once per auction through the converter context, and the Happy to wait if this is parked pending the #15247 discussion. Just checking it hasn't slipped off the list. |
…dundant validity re-filter core already filters by isBidRequestValid before buildRequests - no other adapter re-runs it. Also drops the unreachable optional-call on the context resolver and trims the comments this PR introduced.
deepSetValue is dset, which keeps whatever it finds when typeof is object - and typeof null is object. A publisher ortb2.user of null then threw out of buildRequests and the adapter contributed no bids at all; an array dropped the id silently. mergeDeep repairs a non-object at each path segment.
Summary
Adds a first-party UUID fallback identity to the Floxis Bid Adapter, sent as
user.ext.floxisIdin the OpenRTB request.Why: Floxis's primary identity signal (
__fxId) is a third-party cookie on.floxis.tech. In Safari, Firefox, and other ITP/ETP environments it is blocked. This change mints a stable UUID in the publisher's own page context (first-party) so there is an identity-of-last-resort for cookieless browsers.What it does
generateUUID()utility on the first auction on a given publisher's site.storageManager(localStorage preferred, cookie fallback) scoped to the publisher's own origin — it is per-publisher, not cross-site.flx_uidkey). The id is refreshed on malformed or missing stored values.user.ext.floxisIdviamergeDeep— additive, never overwrites an existinguser.ext.floxisIdsupplied viaortb2FPD or the User ID module.__fxIdcookie (processedCookieUserId.orElse(clientFloxisId).orElse(user.id)), so existing Chrome/Edge behavior is unchanged.Storage & consent
storageManager, gated bydeviceAccessconfig and GDPR purpose-1 consent under Floxis's registeredgvlid(1609). No bespoke consent logic is added.bidderSettings:When storage is disallowed (or throws), no id is generated, no storage is written, and the auction is unaffected — a safe no-op.
Testing
All 118 adapter unit tests pass (
gulp test-only --file test/spec/modules/floxisBidAdapter_spec.js --nolinton Node 24.14.1). The 8 new tests cover: mint + persist new id, reuse stored id, cookie fallback when localStorage empty, regenerate on malformed stored value, no-op when storage disallowed, no-op on storage accessor throw, FPD field preservation (user.extfields fromortb2are not clobbered), and no-overwrite of a pre-setuser.ext.floxisId.Files changed
modules/floxisBidAdapter.js—getOrCreateFloxisId()helper +buildRequesthookmodules/floxisBidAdapter.md— First-Party Fallback Id section addedtest/spec/modules/floxisBidAdapter_spec.js— 8 new unit testsWhy not a userId (sharedId) module?
We considered the userId framework first. Three reasons the adapter-side fallback is the right shape here:
userId/sharedId— this adapter is deployed to long-tail publishers whose wrappers we don't control.user.ext.eidsfrom the userId module (including sharedId) is still forwarded via FPD passthrough and is preferred when present.__fxIdcookie on.floxis.tech; the backend applies__fxId → client floxisId → existing user.id, so this id only matters in ITP/ETP browsers where the third-party cookie is blocked. Placing it atuser.ext.floxisId(notuser.id/eids) keeps it from colliding with or masquerading as publisher-declared identity.storageManager(purpose-1 under GVL 1609), requires the explicitbidderSettings.floxis.storageAllowedopt-in, and is disclosed at https://floxis.tech/vendor-storage.json (embedded inmetadata/modules/floxisBidAdapter.json). An id that fails to persist is never sent.Docs PR
prebid/prebid.github.io#6650 — documents the
flx_uidstorage use +storageAllowedopt-in on the bidder page (stacks on the already-approved prebid/prebid.github.io#6596, which setsgvl_id: 1609andpbs: true).