FS-12403: add bcat/badv support for banner + fix video merge#65
Open
ym-aaron wants to merge 4 commits into
Open
FS-12403: add bcat/badv support for banner + fix video merge#65ym-aaron wants to merge 4 commits into
ym-aaron wants to merge 4 commits into
Conversation
- Add mergeBlocklist() helper: union ortb2.<field> + params.<field>, dedupe, filter invalid/non-string entries with logWarn, never drop the bid. - Banner (non-video) path now sends bcat/badv as comma-delimited params (per the AS-5349 ad-server endpoint); omitted when empty; never subject to URL-length trimming. - Video path reads ortb2.bcat/ortb2.badv (was the dead bidderRequest.bcat path) and merges with params instead of || precedence. - Remove bcat/badv checks from validateVideoParams (now normalized in mergeBlocklist; no longer bid-dropping, applies to all media types). - Add unit tests and yieldmoBidAdapter.md docs for both params. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ym-aaron
force-pushed
the
FS-12403-bcat-badv
branch
from
95f25a8 to
cf627a1
Compare
Per review discussion (Aaron + Niko): keep brand-safety strict for malformed input while not penalizing absence. - Add validateBlocklistParams(), run for ALL bids (banner + video) via isBidRequestValid: a missing bcat/badv passes; a value that is present but not an array fails validation (drops the bid). - mergeBlocklist still merges + dedupes, and defensively filters the ortb2 source (not bid-validated) and non-string/empty array elements, with logWarn. - Update spec (missing-allowed / malformed-dropped cases) and the .md docs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Move isDefined / paramRequired / paramInvalid and the field validator out of validateVideoParams to module scope (createParamValidator binds them to a bid), so validateVideoParams and validateBlocklistParams share one validation style. validateBlocklistParams now uses the same validate(...) + try/catch pattern instead of its own bespoke check. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Rename mergeBlocklist -> getBlocklist and have it take (bidderRequest, bid, field), doing the ortb2.<field> / params.<field> deepAccess lookups itself. Call sites no longer repeat deepAccess or the path strings; the field name appears once per call. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
What
Adds
bcat/badvsupport to the banner (non-video) path and fixes the video path's blocklist handling in the Yieldmo adapter. Resolves FS-12403.Why
Per FS-12403 (and Abhineet's comments), the adapter had several blocklist gaps:
bcatread the wrong path —deepAccess(bidderRequest, 'bcat')is alwaysundefined; the value lives atortb2.bcat. The ORTB source was effectively dead.badvhad no ortb2 support — onlyparams.badvwas read.||meant a publisher setting bothortb2andparamssilently lost one.bcat/badv.yieldmoBidAdapter.md.The ad-server side already accepts these on the prebid-js endpoint (AS-5349), so the wire format is fixed: comma-delimited params on the banner GET.
Changes
mergeBlocklist()helper — unionsortb2.<field>+params.<field>, de-dupes (Set), and filters non-array sources / non-string / empty entries with alogWarninstead of dropping the bid (mirrors the in-treepubmaticBidAdapternorm).bcat/badvas comma-delimited GET params; omitted when empty; intentionally kept out ofBANNER_REQUEST_PROPERTIES_TO_REDUCEso URL-length trimming can never corrupt a blocklist.ortb2.bcat/ortb2.badvand merges withparams.*(OpenRTB arrays, unchanged shape).validateVideoParams— removed thebcat/badvarray checks (they were video-only and dropped the bid); normalization now lives inmergeBlocklist, so malformed values no longer drop a bid.Behavior change
bcat/badvis allowed — unchanged.isBidRequestValid, for both banner and video (previously this reject only ran for video). Keeps brand-safety strict on bad input without penalizing absence.ortb2value (which isn't bid-validated), are filtered out with alogWarnrather than failing the bid.See the decision trail on FS-12403.
Tests
13 new spec cases (banner comma format, union, dedupe+order, ortb2-only, params-only, omit-when-empty; video arrays, correct
ortb2.bcatpath, empty defaults; malformed filter+warn, trim, bid-not-dropped). FullyieldmoBidAdapter_spec.jssuite passes (82 tests).Prior art
Surveyed every adapter that handles
bcat/badv. On rejecting malformed input,outbrainBidAdapterdrops the bid (requires an array of strings); most others ignore/filter (discovery:Array.isArray(x) ? x : [];pubmaticfilters elements with alogWarn;taboola/vidoomy/etc. pass through). This PR lands in the middle: strict like outbrain on a malformed param (drop the bid), but lenient on element-level issues andortb2(filter +logWarn), and it never penalizes a missing value.🤖 Generated with Claude Code