Port Floxis: New Adapter - #4529
Conversation
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
8763dcd to
86a34b6
Compare
…sistency seat lacked the explicit @JsonProperty that region and partner carry; add it so all three params are annotated uniformly (no behavioural change — seat already maps to the 'seat' key).
…tner are subdomain labels) Per the dev-guide, a bidder endpoint domain must not be fully variable. Move the fixed .floxis.tech suffix into the bidder-config endpoint and have resolveBidHost return just the validated region/partner subdomain label. Resolved URLs are unchanged (e.g. https://us-e.floxis.tech/pbs); matches the fixed-suffix pattern of merged region-routed adapters (rubicon, clydo, mediago, algorix).
Drop redundant in-adapter validation covered elsewhere: the empty-imp guard (framework guarantees at least one imp), the region/partner DNS-label regex (already enforced by floxis.json), and the custom isBlank helper (use StringUtils.isBlank). Restructure makeHttpRequests to parse the first imp ext then validate the rest in a loop; extract extractBids and countFormats helpers; URL-encode the dynamic host parts; drop redundant @JsonProperty on ExtImpFloxis and one-line @propertysource; remove explanatory comments. Tests updated to match.
|
Thanks for the thorough review @CTMBNara — all of it is addressed in 21d1900: dropped the redundant empty-imp check, the in-code DNS regex (floxis.json already covers it) and the custom isBlank helper; reworked makeHttpRequests into the parse-first/validate-the-rest shape; extracted extractBids and countFormats; url-encoded the host parts; and cleared out the comments. Also synced the PR description with the final code. Ready for another look whenever you have a moment. |
|
Hi @CTMBNara — thanks for the thorough review. I've applied all of the feedback in the latest commit:
I've replied inline on each thread. Whenever you have a moment, could you take another pass? Happy to adjust anything further — thanks again for your time. |
|
Documentation PR (per the add-new-bidder checklist): prebid/prebid.github.io#6596 — already approved by @muuki88; it sets @CTMBNara re-review is requested since 21d1900 addressed all of your feedback — and one note: the CI workflows on that commit (Java CI / functional tests / CodeQL / Trivy) are awaiting first-time-contributor approval, so they haven't run yet. Could you approve the runs when you pick this back up? |
|
Bumping this — @CTMBNara's review feedback was fully addressed in The blocker right now is CI, not code. Workflows have never run on this PR at all, due to first-time-contributor gating — they need a maintainer to approve the run. The PR therefore shows no green checks, which likely makes it look unready in the queue when it isn't. @CTMBNara — could you approve the workflow run, and re-review when you have bandwidth? @Lightwood13 @And1sS — if @CTMBNara is stretched thin, could either of you pick this one up? For context: the Go counterpart (prebid/prebid-server#4811) already carries an approval from @przemkaczmarek, and the docs PR (prebid/prebid.github.io#6596) is approved by @muuki88. This has been open since June 3. If anyone can give a rough sense of timing, it would genuinely help us plan around it. |
| .build(); | ||
| } | ||
|
|
||
| private static com.fasterxml.jackson.databind.node.ObjectNode givenImpExt(String seat, String region) { |
Merge master and switch the endpoint from manual {{Macro}} substitution to org.prebid.server.util.Uri, dropping our HttpUtil.encodeUrl calls so dynamic parts are encoded once by the template instead of twice. Update the usersync redirect URL to the single-brace macro form.
Compare imp exts with equals (Lombok @value supplies it) rather than field by field. Move ObjectNode to imports. Drop the seat query param from the integration-test endpoint and its wiremock stub; seat routing is covered by unit tests, plus a new one pinning that a macro-less endpoint resolves unchanged.
Master removed UsersyncerCreator (prebid#4464); usersync is now assembled from bidder config, so drop the wiring.
|
Merged master and applied all five.
Two things the merge surfaced beyond the review:
Unit tests are 24 green and checkstyle is clean. I can't run the integration tests locally, so those are on CI — which is the remaining problem: no workflow has ever run on this PR. They have been sitting in first-time-contributor gating since June, which is also why the PR shows no checks. Could you approve the run when you next pick this up? |
…ixture The expected outgoing request predates ext.prebid.server.http_method, so the wiremock stub never matched, the exchange returned 404 and the auction came back with no bids. Every other fixture carrying that block already declares it.
|
Fixed. The fixture was missing @And1sS the new commit needs another workflow approval when you get a chance — sorry for the extra click. |
…imp exts The Go port compares resolved hosts, so imps differing only by an omitted region or partner still share a single request. The Java port compared raw ext values and rejected them. Apply the defaults when parsing so equals() sees resolved values, and cover the case the Go suite already pins.
|
One more on top, so this replaces the sha from my last comment — sorry. Reviewing against the Go port I found the two disagree on multi-imp routing: Go compares resolved hosts, so imps differing only by an omitted |
| return Result.withValue(HttpRequest.<BidRequest>builder() | ||
| .method(HttpMethod.POST) | ||
| .uri(resolveUrl(firstImpExt)) | ||
| .headers(HttpUtil.headers()) | ||
| .impIds(BidderUtil.impIds(request)) | ||
| .payload(request) | ||
| .body(mapper.encodeToBytes(request)) | ||
| .build()); |
There was a problem hiding this comment.
Use BidderUtil.defaultRequest instead
| cookie-family-name: floxis | ||
| redirect: | ||
| url: "https://px-us-e.floxis.tech/sync?gdpr={gdpr}&gdpr_consent={gdpr_consent}&gpp={gpp}&gpp_sid={gpp_sid}&us_privacy={us_privacy}&dest={redirect_url}" | ||
| support-cors: false |
additionalProperties:false made any param outside seat/region/partner fail schema validation, and ImpValidator turns that into a ValidationException for the whole request - so a publisher setting the documented bidFloor param lost the entire auction, not just the floxis imp. 261 of 263 sibling schemas leave the root open.
… key Also switches extractBids to the corpus stream shape, removes the comment re-added in 8640651, and aligns the makeBids parse-failure test name with the CouldNotBeParsed convention (220 of 254 siblings).
Imps targeting different seats or hosts were rejected outright, dropping the whole auction; they are now grouped into one request each. A single unparseable imp.ext no longer aborts the request either - that imp is dropped and the error appended, matching the corpus.
|
Both review items are done — Checking this port against the Go one also turned up a params-schema defect, fixed in both:
@And1sS sorry — the push reset the workflow approvals again. |
The bidder-params schema was the only enforcement point, which is what let the Go port ship a host-injection path. A missing seat also escaped makeHttpRequests as an uncaught NoSuchElementException from Uri.expand rather than a BidderError. Region and partner are matched with Matcher.matches(), not the schema pattern - Java's $ also matches before a trailing newline, so "eu\n" passed here while Go rejected it. Both are lowercased before becoming routing state.
2 participants
🔧 Type of changes
✨ Whats the context?
This PR ports the Floxis bid adapter from PBS-Go to PBS-Java. Floxis is an ad exchange; the adapter sends standard OpenRTB 2.x server-to-server bid requests to the Floxis exchange so PBS publishers can access Floxis demand. It is the Java counterpart of the in-review Go adapter prebid/prebid-server#4811.
Scope:
appandsite).modifying-vast-xml-allowed: false.🧠 Rationale behind the change
Bidder params (
imp[].ext.prebid.bidder.floxis):seat(string, required, minLength 1) — the Floxis seat the publisher buys through; appended url-escaped to the endpoint as?seat=.region(string, optional, defaultus-e) — the regional RTB host, used as a subdomain label. Validated as a DNS label by the params JSON schema.partner(string, optional, defaultfloxis) — white-label partner, used as an additional subdomain label. Validated as a DNS label by the params JSON schema.Endpoint routing: the fixed parent domain (
.floxis.tech) is pinned in the adapter's endpoint config; only the{{Host}}subdomain is filled from the request. The subdomain isregion(e.g.us-e.floxis.tech), orpartner-regionfor a named white-label partner (e.g.acme-us-e.floxis.tech); the defaultfloxispartner adds no prefix.region/partnerare constrained to DNS labels by the params schema and url-encoded before interpolation, so the host can never be derived from a request-supplied hostname — satisfying the "no fully dynamic hostnames" requirement.seat/region/partnerare read from the first imp; a multi-imp request that mixes seats/regions/partners is rejected as bad input (one request routes to one host). The request body is forwarded unchanged (no caller-owned object is mutated).Bid type resolution treats
bid.mtype(OpenRTB 2.6) as authoritative (1/2/3/4 → banner/video/audio/native). Whenmtypeis absent, a single-format imp resolves to that media type; an imp without exactly one format cannot be disambiguated and surfaces abadServerResponse, while other bids in the response continue to be processed.gvlVendorIDis set to1609— Floxis (legal entity Ad Tech Company OÜ) is registered as an IAB TCF vendor with GVL ID 1609, so EU/TCF consent is enforced against that vendor ID.🔎 New Bid Adapter Checklist
🧪 Test plan
FloxisBidderTest— unit tests covering region/partner host resolution (default, explicit region, partner-prefixed host), url-encoding of the seat and the host parts, request-forwarded-unchanged, the per-imp consistency check, and the fullgetMediaTypeForBidmatrix (mtype-authoritative for all four types, unsupported mtype, single-format fallback for all four, the non-single-format error, imp-not-found), plus per-bid error accumulation. JaCoCo reports ~100% line coverage onFloxisBidder.FloxisTestintegration test — exercises the full auction path against WireMock and validates the adapter configuration is reachable.mvn test(unit + IT) andmvn checkstyle:checkpass locally.🏎 Quality check
Companion Go adapter: prebid/prebid-server#4811. The corresponding
prebid.github.iodocumentation PR: prebid/prebid.github.io#6596 (approved).Please add the
do not portlabel per the porting guide (I do not have label permissions on this repo).