Add support for multiple root schain nodes #1374
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,6 +28,29 @@ type cleanMetrics struct { | |
| gdprTcfVersion int | ||
| } | ||
|
|
||
| func BidderToPrebidSChains(req *openrtb_ext.ExtRequest) (map[string]*openrtb_ext.ExtRequestPrebidSChainSChain, error) { | ||
| bidderToSChains := make(map[string]*openrtb_ext.ExtRequestPrebidSChainSChain) | ||
|
|
||
| if len(req.Prebid.SChains) == 0 { | ||
| return bidderToSChains, nil | ||
| } | ||
|
|
||
| for _, schainWrapper := range req.Prebid.SChains { | ||
| if schainWrapper != nil && len(schainWrapper.Bidders) > 0 { | ||
| for _, bidder := range schainWrapper.Bidders { | ||
| if _, present := bidderToSChains[bidder]; present { | ||
|
There was a problem hiding this comment. This seems strange. If a bidder is in the list, and that bidder also has an entry in There was a problem hiding this comment. Good catch. I don't see any info in the spec or in the applicable github issues/PRs about how to handle the case where multiple non-wildcard schains are defined for a bidder. The java version appears to take the same approach of ignoring all schains for a bidder if multiple are defined for a bidder, which is where I got the idea from. I'm not a java expert but it looks like the java version has the same bug. I'll confirm that this is the expected behavior and update the logic accordingly. There was a problem hiding this comment. The team decided that this is a data error that should be detected during request validation preventing further processing. |
||
| return nil, fmt.Errorf("request.ext.prebid.schains contains multiple schains for bidder %s; "+ | ||
| "it must contain no more than one per bidder.", bidder) | ||
| } else { | ||
| bidderToSChains[bidder] = &schainWrapper.SChain | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return bidderToSChains, nil | ||
| } | ||
|
|
||
| // cleanOpenRTBRequests splits the input request into requests which are sanitized for each bidder. Intended behavior is: | ||
| // | ||
| // 1. BidRequest.Imp[].Ext will only contain the "prebid" field and a "bidder" field which has the params for the intended Bidder. | ||
|
|
@@ -103,12 +126,35 @@ func cleanOpenRTBRequests(ctx context.Context, | |
| return | ||
| } | ||
|
|
||
| func splitBidRequest(req *openrtb.BidRequest, | ||
| impsByBidder map[string][]openrtb.Imp, | ||
| aliases map[string]string, | ||
| usersyncs IdFetcher, | ||
| blabels map[openrtb_ext.BidderName]*pbsmetrics.AdapterLabels, | ||
| labels pbsmetrics.Labels) (map[openrtb_ext.BidderName]*openrtb.BidRequest, []error) { | ||
|
|
||
| requestsByBidder := make(map[openrtb_ext.BidderName]*openrtb.BidRequest, len(impsByBidder)) | ||
| explicitBuyerUIDs, err := extractBuyerUIDs(req.User) | ||
| if err != nil { | ||
| return nil, []error{err} | ||
| } | ||
|
|
||
| var requestExt openrtb_ext.ExtRequest | ||
| var sChainsByBidder map[string]*openrtb_ext.ExtRequestPrebidSChainSChain | ||
| if len(req.Ext) > 0 { | ||
| err := json.Unmarshal(req.Ext, &requestExt) | ||
|
There was a problem hiding this comment. This data is also unmarshaled here: https://github.com/prebid/prebid-server/blob/master/exchange/exchange.go#L118-L122 I'd rather only unmarshal once but things might get a little convoluted without a refactor so I figured I would punt on that for now. |
||
| if err != nil { | ||
| return nil, []error{err} | ||
| } | ||
|
|
||
| sChainsByBidder, err = BidderToPrebidSChains(&requestExt) | ||
| if err != nil { | ||
| return nil, []error{err} | ||
| } | ||
| } else { | ||
| sChainsByBidder = make(map[string]*openrtb_ext.ExtRequestPrebidSChainSChain) | ||
| } | ||
|
|
||
| for bidder, imps := range impsByBidder { | ||
| reqCopy := *req | ||
| coreBidder := resolveBidder(bidder, aliases) | ||
|
|
@@ -128,11 +174,57 @@ func splitBidRequest(req *openrtb.BidRequest, impsByBidder map[string][]openrtb. | |
| blabels[coreBidder].CookieFlag = pbsmetrics.CookieFlagYes | ||
| } | ||
| reqCopy.Imp = imps | ||
|
|
||
| prepareSource(&reqCopy, bidder, sChainsByBidder) | ||
| prepareExt(&reqCopy, &requestExt) | ||
|
|
||
| requestsByBidder[openrtb_ext.BidderName(bidder)] = &reqCopy | ||
| } | ||
| return requestsByBidder, nil | ||
| } | ||
|
|
||
| func prepareExt(req *openrtb.BidRequest, unpackedExt *openrtb_ext.ExtRequest) { | ||
| if len(req.Ext) == 0 { | ||
| return | ||
| } | ||
| extCopy := *unpackedExt | ||
| extCopy.Prebid.SChains = nil | ||
| reqExt, err := json.Marshal(extCopy) | ||
| if err == nil { | ||
| req.Ext = reqExt | ||
| } | ||
| } | ||
|
|
||
| func prepareSource(req *openrtb.BidRequest, bidder string, sChainsByBidder map[string]*openrtb_ext.ExtRequestPrebidSChainSChain) { | ||
| const sChainWildCard = "*" | ||
| var selectedSChain *openrtb_ext.ExtRequestPrebidSChainSChain | ||
|
|
||
| wildCardSChain := sChainsByBidder[sChainWildCard] | ||
| bidderSChain := sChainsByBidder[bidder] | ||
|
|
||
| // source should not be modified | ||
| if bidderSChain == nil && wildCardSChain == nil { | ||
| return | ||
| } | ||
|
|
||
| if bidderSChain != nil { | ||
| selectedSChain = bidderSChain | ||
| } else { | ||
| selectedSChain = wildCardSChain | ||
| } | ||
|
|
||
| // set source | ||
| var source openrtb.Source | ||
| schain := openrtb_ext.ExtRequestPrebidSChain{ | ||
| SChain: *selectedSChain, | ||
| } | ||
| sourceExt, err := json.Marshal(schain) | ||
|
There was a problem hiding this comment. There's a performance optimization opportunity here that I punted on. We could cache each marshalled schain and copy the byte array to There was a problem hiding this comment. Application wide we are guilty of re-marshaling a ton. I agree this is fine for now and part of a much larger performance fix in the (near?) future. |
||
| if err == nil { | ||
| source.Ext = sourceExt | ||
| req.Source = &source | ||
| } | ||
| } | ||
|
|
||
| // extractBuyerUIDs parses the values from user.ext.prebid.buyeruids, and then deletes those values from the ext. | ||
| // This prevents a Bidder from using these values to figure out who else is involved in the Auction. | ||
| func extractBuyerUIDs(user *openrtb.User) (map[string]string, error) { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
*openrtb_ext.ExtRequestalso gets unmarshaled and validated in filesendpoints/openrtb2/amp_auction.goandendpoints/openrtb2/video_auction.gowe could probably alsovalidateSChains(bidExt)from there in order to prevent a defective "ext"JSON field from getting all the way tosplitBidRequestsThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
openrtb2/amp_auction.goandopenrtb2/video_auction.goendpoints both call thevalidateRequestmethod inopenrtb2/auction.gowhich callsvalidateSChainsso we're covered. Those endpoints forgo processing if the method returns an endpoint as well.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ohh I see