Skip to content

Commit

Permalink
No Longer Move bid.ext To bid.ext.bidder (prebid#1742)
Browse files Browse the repository at this point in the history 
* No Longer Move bid.ext To bid.ext.bidder

* Remove Similar Behavior From seatbid.ext

* Avoid Second Bid Copy

* Removed Unused seatbid.ext
  • Loading branch information
@SyntaxNode
SyntaxNode authored and Dan Barnett committed May 11, 2021
1 parent 69b3060 commit d11dde5
Show file tree
Hide file tree
Showing 9 changed files with 236 additions and 78 deletions.
6 changes: 3 additions & 3 deletions adapters/rhythmone/rhythmone.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,9 +132,9 @@ func (a *RhythmoneAdapter) preProcess(req *openrtb.BidRequest, errors []error) (
errors = append(errors, err)
return nil, "", errors
}
bidderExtCopy := openrtb_ext.ExtBid{
Bidder: rhythmoneExtCopy,
}
bidderExtCopy := struct {
Bidder json.RawMessage `json:"bidder,omitempty"`
}{rhythmoneExtCopy}
impExtCopy, err := json.Marshal(&bidderExtCopy)
if err != nil {
errors = append(errors, err)
Expand Down
4 changes: 0 additions & 4 deletions exchange/bidder.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,6 @@ type pbsOrtbSeatBid struct {
// httpCalls is the list of debugging info. It should only be populated if the request.test == 1.
// This will become response.ext.debug.httpcalls.{bidder} on the final Response.
httpCalls []*openrtb_ext.ExtHttpCall
// ext contains the extension for this seatbid.
// if len(bids) > 0, this will become response.seatbid[i].ext.{bidder} on the final OpenRTB response.
// if len(bids) == 0, this will be ignored because the OpenRTB spec doesn't allow a SeatBid with 0 Bids.
ext json.RawMessage
}

// adaptBidder converts an adapters.Bidder into an exchange.adaptedBidder.
Expand Down
4 changes: 0 additions & 4 deletions exchange/bidder_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,10 +135,6 @@ func TestSingleBidder(t *testing.T) {
if len(seatBid.httpCalls) != test.httpCallsLen {
t.Errorf("The bidder shouldn't log HttpCalls when request.test == 0. Found %d", len(seatBid.httpCalls))
}

if len(seatBid.ext) != 0 {
t.Errorf("The bidder shouldn't define any seatBid.ext. Got %s", string(seatBid.ext))
}
}
}

Expand Down
85 changes: 42 additions & 43 deletions exchange/exchange.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -788,25 +788,9 @@ func (e *exchange) makeExtBidResponse(adapterBids map[openrtb_ext.BidderName]*pb
// Return an openrtb seatBid for a bidder
// BuildBidResponse is responsible for ensuring nil bid seatbids are not included
func (e *exchange) makeSeatBid(adapterBid *pbsOrtbSeatBid, adapter openrtb_ext.BidderName, adapterExtra map[openrtb_ext.BidderName]*seatResponseExtra, auc *auction, returnCreative bool) *openrtb.SeatBid {
seatBid := new(openrtb.SeatBid)
seatBid.Seat = adapter.String()
// Prebid cannot support roadblocking
seatBid.Group = 0

if len(adapterBid.ext) > 0 {
sbExt := ExtSeatBid{
Bidder: adapterBid.ext,
}

ext, err := json.Marshal(sbExt)
if err != nil {
extError := openrtb_ext.ExtBidderError{
Code: errortypes.ReadCode(err),
Message: fmt.Sprintf("Error writing SeatBid.Ext: %s", err.Error()),
}
adapterExtra[adapter].Errors = append(adapterExtra[adapter].Errors, extError)
}
seatBid.Ext = ext
seatBid := &openrtb.SeatBid{
Seat: adapter.String(),
Group: 0, // Prebid cannot support roadblocking
}

var errList []error
Expand All @@ -818,39 +802,54 @@ func (e *exchange) makeSeatBid(adapterBid *pbsOrtbSeatBid, adapter openrtb_ext.B
return seatBid
}

// Create the Bid array inside of SeatBid
func (e *exchange) makeBid(Bids []*pbsOrtbBid, auc *auction, returnCreative bool) ([]openrtb.Bid, []error) {
bids := make([]openrtb.Bid, 0, len(Bids))
errList := make([]error, 0, 1)
for _, thisBid := range Bids {
bidExt := &openrtb_ext.ExtBid{
Bidder: thisBid.bid.Ext,
Prebid: &openrtb_ext.ExtBidPrebid{
Targeting: thisBid.bidTargets,
Type: thisBid.bidType,
Video: thisBid.bidVideo,
Events: thisBid.bidEvents,
DealPriority: thisBid.dealPriority,
DealTierSatisfied: thisBid.dealTierSatisfied,
},
func (e *exchange) makeBid(bids []*pbsOrtbBid, auc *auction, returnCreative bool) ([]openrtb.Bid, []error) {
result := make([]openrtb.Bid, 0, len(bids))
errs := make([]error, 0, 1)

for _, bid := range bids {
bidExtPrebid := &openrtb_ext.ExtBidPrebid{
DealPriority: bid.dealPriority,
DealTierSatisfied: bid.dealTierSatisfied,
Events: bid.bidEvents,
Targeting: bid.bidTargets,
Type: bid.bidType,
Video: bid.bidVideo,
}
if cacheInfo, found := e.getBidCacheInfo(thisBid, auc); found {
bidExt.Prebid.Cache = &openrtb_ext.ExtBidPrebidCache{

if cacheInfo, found := e.getBidCacheInfo(bid, auc); found {
bidExtPrebid.Cache = &openrtb_ext.ExtBidPrebidCache{
Bids: &cacheInfo,
}
}
ext, err := json.Marshal(bidExt)
if err != nil {
errList = append(errList, err)

if bidExtJSON, err := makeBidExtJSON(bid.bid.Ext, bidExtPrebid); err != nil {
errs = append(errs, err)
} else {
bids = append(bids, *thisBid.bid)
bids[len(bids)-1].Ext = ext
result = append(result, *bid.bid)
resultBid := &result[len(result)-1]
resultBid.Ext = bidExtJSON
if !returnCreative {
bids[len(bids)-1].AdM = ""
resultBid.AdM = ""
}
}
}
return bids, errList
return result, errs
}

func makeBidExtJSON(ext json.RawMessage, prebid *openrtb_ext.ExtBidPrebid) (json.RawMessage, error) {
// no existing bid.ext. generate a bid.ext with just our prebid section populated.
if len(ext) == 0 {
bidExt := &openrtb_ext.ExtBid{Prebid: prebid}
return json.Marshal(bidExt)
}

// update existing bid.ext with our prebid section. if bid.ext.prebid already exists, it will be overwritten.
var extMap map[string]interface{}
if err := json.Unmarshal(ext, &extMap); err != nil {
return nil, err
}
extMap[openrtb_ext.PrebidExtKey] = prebid
return json.Marshal(extMap)
}

// If bid got cached inside `(a *auction) doCache(ctx context.Context, cache prebid_cache_client.Client, targData *targetData, bidRequest *openrtb.BidRequest, ttlBuffer int64, defaultTTLs *config.DefaultTTLs, bidCategory map[string]string)`,
Expand Down
26 changes: 13 additions & 13 deletions exchange/exchange_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1823,7 +1823,7 @@ func TestCategoryMapping(t *testing.T) {
&bid1_4,
}

seatBid := pbsOrtbSeatBid{innerBids, "USD", nil, nil}
seatBid := pbsOrtbSeatBid{bids: innerBids, currency: "USD"}
bidderName1 := openrtb_ext.BidderName("appnexus")

adapterBids[bidderName1] = &seatBid
Expand Down Expand Up @@ -1878,7 +1878,7 @@ func TestCategoryMappingNoIncludeBrandCategory(t *testing.T) {
&bid1_4,
}

seatBid := pbsOrtbSeatBid{innerBids, "USD", nil, nil}
seatBid := pbsOrtbSeatBid{bids: innerBids, currency: "USD"}
bidderName1 := openrtb_ext.BidderName("appnexus")

adapterBids[bidderName1] = &seatBid
Expand Down Expand Up @@ -1930,7 +1930,7 @@ func TestCategoryMappingTranslateCategoriesNil(t *testing.T) {
&bid1_3,
}

seatBid := pbsOrtbSeatBid{innerBids, "USD", nil, nil}
seatBid := pbsOrtbSeatBid{bids: innerBids, currency: "USD"}
bidderName1 := openrtb_ext.BidderName("appnexus")

adapterBids[bidderName1] = &seatBid
Expand Down Expand Up @@ -2012,7 +2012,7 @@ func TestCategoryMappingTranslateCategoriesFalse(t *testing.T) {
&bid1_3,
}

seatBid := pbsOrtbSeatBid{innerBids, "USD", nil, nil}
seatBid := pbsOrtbSeatBid{bids: innerBids, currency: "USD"}
bidderName1 := openrtb_ext.BidderName("appnexus")

adapterBids[bidderName1] = &seatBid
Expand Down Expand Up @@ -2082,7 +2082,7 @@ func TestCategoryDedupe(t *testing.T) {
&bid1_5,
}

seatBid := pbsOrtbSeatBid{innerBids, "USD", nil, nil}
seatBid := pbsOrtbSeatBid{bids: innerBids, currency: "USD"}
bidderName1 := openrtb_ext.BidderName("appnexus")

adapterBids[bidderName1] = &seatBid
Expand Down Expand Up @@ -2162,7 +2162,7 @@ func TestNoCategoryDedupe(t *testing.T) {
&bid1_5,
}

seatBid := pbsOrtbSeatBid{innerBids, "USD", nil, nil}
seatBid := pbsOrtbSeatBid{bids: innerBids, currency: "USD"}
bidderName1 := openrtb_ext.BidderName("appnexus")

adapterBids[bidderName1] = &seatBid
Expand Down Expand Up @@ -2223,10 +2223,10 @@ func TestCategoryMappingBidderName(t *testing.T) {
&bid1_2,
}

seatBid1 := pbsOrtbSeatBid{innerBids1, "USD", nil, nil}
seatBid1 := pbsOrtbSeatBid{bids: innerBids1, currency: "USD"}
bidderName1 := openrtb_ext.BidderName("bidder1")

seatBid2 := pbsOrtbSeatBid{innerBids2, "USD", nil, nil}
seatBid2 := pbsOrtbSeatBid{bids: innerBids2, currency: "USD"}
bidderName2 := openrtb_ext.BidderName("bidder2")

adapterBids[bidderName1] = &seatBid1
Expand Down Expand Up @@ -2277,10 +2277,10 @@ func TestCategoryMappingBidderNameNoCategories(t *testing.T) {
&bid1_2,
}

seatBid1 := pbsOrtbSeatBid{innerBids1, "USD", nil, nil}
seatBid1 := pbsOrtbSeatBid{bids: innerBids1, currency: "USD"}
bidderName1 := openrtb_ext.BidderName("bidder1")

seatBid2 := pbsOrtbSeatBid{innerBids2, "USD", nil, nil}
seatBid2 := pbsOrtbSeatBid{bids: innerBids2, currency: "USD"}
bidderName2 := openrtb_ext.BidderName("bidder2")

adapterBids[bidderName1] = &seatBid1
Expand Down Expand Up @@ -2384,7 +2384,7 @@ func TestBidRejectionErrors(t *testing.T) {
innerBids = append(innerBids, &currentBid)
}

seatBid := pbsOrtbSeatBid{innerBids, "USD", nil, nil}
seatBid := pbsOrtbSeatBid{bids: innerBids, currency: "USD"}

adapterBids[bidderName] = &seatBid

Expand Down Expand Up @@ -2442,10 +2442,10 @@ func TestCategoryMappingTwoBiddersOneBidEachNoCategorySamePrice(t *testing.T) {
for i := 1; i < 10; i++ {
adapterBids := make(map[openrtb_ext.BidderName]*pbsOrtbSeatBid)

seatBidApn1 := pbsOrtbSeatBid{innerBidsApn1, "USD", nil, nil}
seatBidApn1 := pbsOrtbSeatBid{bids: innerBidsApn1, currency: "USD"}
bidderNameApn1 := openrtb_ext.BidderName("appnexus1")

seatBidApn2 := pbsOrtbSeatBid{innerBidsApn2, "USD", nil, nil}
seatBidApn2 := pbsOrtbSeatBid{bids: innerBidsApn2, currency: "USD"}
bidderNameApn2 := openrtb_ext.BidderName("appnexus2")

adapterBids[bidderNameApn1] = &seatBidApn1
Expand Down
90 changes: 90 additions & 0 deletions exchange/exchangetest/bid-ext-prebid-collision.json
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
{
"description": "Verifies bid.ext values are left alone from the adapter, except for overwriting bid.ext.prebid if the adapter ext includes a collision.",

"incomingRequest": {
"ortbRequest": {
"id": "some-request-id",
"site": {
"page": "test.somepage.com"
},
"imp": [{
"id": "my-imp-id",
"video": {
"mimes": ["video/mp4"]
},
"ext": {
"appnexus": {
"placementId": 1
}
}
}]
}
},
"outgoingRequests": {
"appnexus": {
"expectRequest": {
"ortbRequest": {
"id": "some-request-id",
"site": {
"page": "test.somepage.com"
},
"imp": [{
"id": "my-imp-id",
"video": {
"mimes": ["video/mp4"]
},
"ext": {
"bidder": {
"placementId": 1
}
}
}]
},
"bidAdjustment": 1.0
},
"mockResponse": {
"pbsSeatBid": {
"pbsBids": [{
"ortbBid": {
"id": "apn-bid",
"impid": "my-imp-id",
"price": 0.3,
"w": 200,
"h": 250,
"crid": "creative-1",
"ext": {
"someField": "someValue",
"prebid": {
"willBeOverwritten": "by core logic"
}
}
},
"bidType": "video"
}]
}
}
}
},
"response": {
"bids": {
"id": "some-request-id",
"seatbid": [{
"seat": "appnexus",
"bid": [{
"id": "apn-bid",
"impid": "my-imp-id",
"price": 0.3,
"w": 200,
"h": 250,
"crid": "creative-1",
"ext": {
"someField": "someValue",
"prebid": {
"type": "video"
}
}
}]
}]
}
}
}
Loading

0 comments on commit d11dde5

Please sign in to comment.