-
Notifications
You must be signed in to change notification settings - Fork 748
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Pass Through First Party Context Data #1479
Changes from 5 commits
43bd697
df0fbc9
dd0c680
715bfa1
079426e
a8c66ed
1bc35a4
15f4a9c
7e1a7e6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -141,6 +141,16 @@ func TestGoodRequests(t *testing.T) { | |
supplementary.assert(t) | ||
} | ||
|
||
func TestFirstPartyDataRequests(t *testing.T) { | ||
validRequests := &getResponseFromDirectory{ | ||
dir: "sample-requests/first-party-data", | ||
payloadGetter: getRequestPayload, | ||
messageGetter: nilReturner, | ||
expectedCode: http.StatusOK, | ||
} | ||
validRequests.assert(t) | ||
} | ||
|
||
// TestGoodNativeRequests makes sure we return 200s on well-formed Native requests. | ||
func TestGoodNativeRequests(t *testing.T) { | ||
tests := &getResponseFromDirectory{ | ||
|
@@ -1126,31 +1136,104 @@ func TestDisabledBidder(t *testing.T) { | |
} | ||
} | ||
|
||
func TestValidateImpExtDisabledBidder(t *testing.T) { | ||
imp := &openrtb.Imp{ | ||
Ext: json.RawMessage(`{"appnexus":{"placement_id":555},"unknownbidder":{"foo":"bar"}}`), | ||
func TestValidateImpExt(t *testing.T) { | ||
testCases := []struct { | ||
description string | ||
impExt json.RawMessage | ||
expectedImpExt string | ||
expectedErrs []error | ||
}{ | ||
{ | ||
description: "Empty", | ||
impExt: nil, | ||
expectedImpExt: "", | ||
expectedErrs: []error{errors.New("request.imp[0].ext is required")}, | ||
}, | ||
{ | ||
description: "Valid Bidder", | ||
impExt: json.RawMessage(`{"appnexus":{"placement_id":555}}`), | ||
expectedImpExt: `{"appnexus":{"placement_id":555}}`, | ||
expectedErrs: []error{}, | ||
}, | ||
{ | ||
description: "Valid Bidder + Disabled Bidder", | ||
impExt: json.RawMessage(`{"appnexus":{"placement_id":555},"unknownbidder":{"foo":"bar"}}`), | ||
expectedImpExt: `{"appnexus":{"placement_id":555}}`, | ||
expectedErrs: []error{&errortypes.BidderTemporarilyDisabled{Message: "The bidder 'unknownbidder' has been disabled."}}, | ||
}, | ||
{ | ||
description: "Valid Bidder + Disabled Bidder + First Party Data Context", | ||
impExt: json.RawMessage(`{"appnexus":{"placement_id":555},"unknownbidder":{"foo":"bar"},"context":{"data":{"keywords":"prebid server example"}}}`), | ||
expectedImpExt: `{"appnexus":{"placement_id":555},"context":{"data":{"keywords":"prebid server example"}}}`, | ||
expectedErrs: []error{&errortypes.BidderTemporarilyDisabled{Message: "The bidder 'unknownbidder' has been disabled."}}, | ||
}, | ||
{ | ||
description: "Valid Bidder + First Party Data Context", | ||
impExt: json.RawMessage(`{"appnexus":{"placement_id":555},"context":{"data":{"keywords":"prebid server example"}}}`), | ||
expectedImpExt: `{"appnexus":{"placement_id":555},"context":{"data":{"keywords":"prebid server example"}}}`, | ||
expectedErrs: []error{}, | ||
}, | ||
{ | ||
description: "Valid Prebid Ext Bidder", | ||
impExt: json.RawMessage(`{"prebid":{"bidder":{"appnexus":{"placement_id":555}}}}`), | ||
expectedImpExt: `{"prebid":{"bidder":{"appnexus":{"placement_id":555}}}}`, | ||
expectedErrs: []error{}, | ||
// request.imp[x].ext.prebid.bidder.{biddername} is only promoted/copied to request.ext.{biddername} if there is at least one disabled bidder. | ||
}, | ||
{ | ||
description: "Valid Prebid Ext Bidder + First Party Data Context", | ||
impExt: json.RawMessage(`{"prebid":{"bidder":{"appnexus":{"placement_id":555}}} ,"context":{"data":{"keywords":"prebid server example"}}}`), | ||
expectedImpExt: `{"prebid":{"bidder":{"appnexus":{"placement_id":555}}},"context":{"data":{"keywords":"prebid server example"}}}`, | ||
expectedErrs: []error{}, | ||
// request.imp[x].ext.prebid.bidder.{biddername} is only promoted/copied to request.ext.{biddername} if there is at least one disabled bidder. | ||
}, | ||
{ | ||
description: "Valid Prebid Ext Bidder + Disabled Bidder", | ||
impExt: json.RawMessage(`{"prebid":{"bidder":{"appnexus":{"placement_id":555},"unknownbidder":{"foo":"bar"}}}}`), | ||
expectedImpExt: `{"prebid":{"bidder":{"appnexus":{"placement_id": 555},"unknownbidder":{"foo":"bar"}}},"appnexus":{"placement_id":555}}`, | ||
expectedErrs: []error{&errortypes.BidderTemporarilyDisabled{Message: "The bidder 'unknownbidder' has been disabled."}}, | ||
// request.imp[x].ext.prebid.bidder.{biddername} disabled bidders are not removed. if there is a disabled bidder, the valid ones are promoted/copied to request.ext.{biddername}. | ||
}, | ||
{ | ||
description: "Valid Prebid Ext Bidder + Disabled Bidder + First Party Data Context", | ||
impExt: json.RawMessage(`{"prebid":{"bidder":{"appnexus":{"placement_id":555},"unknownbidder":{"foo":"bar"}}},"context":{"data":{"keywords":"prebid server example"}}}`), | ||
expectedImpExt: `{"prebid":{"bidder":{"appnexus":{"placement_id": 555},"unknownbidder":{"foo":"bar"}}},"appnexus":{"placement_id":555},"context":{"data":{"keywords":"prebid server example"}}}`, | ||
expectedErrs: []error{&errortypes.BidderTemporarilyDisabled{Message: "The bidder 'unknownbidder' has been disabled."}}, | ||
// request.imp[x].ext.prebid.bidder.{biddername} disabled bidders are not removed. if there is a disabled bidder, the valid ones are promoted/copied to request.ext.{biddername}. | ||
}, | ||
} | ||
deps := &endpointDeps{ | ||
&nobidExchange{}, | ||
newParamsValidator(t), | ||
&mockStoredReqFetcher{}, | ||
empty_fetcher.EmptyFetcher{}, | ||
empty_fetcher.EmptyFetcher{}, | ||
empty_fetcher.EmptyFetcher{}, | ||
&config.Configuration{MaxRequestSize: int64(8096)}, | ||
pbsmetrics.NewMetrics(metrics.NewRegistry(), openrtb_ext.BidderList(), config.DisabledMetrics{}), | ||
analyticsConf.NewPBSAnalytics(&config.Analytics{}), | ||
map[string]string{"unknownbidder": "The bidder 'unknownbidder' has been disabled."}, | ||
false, | ||
[]byte{}, | ||
openrtb_ext.BidderMap, | ||
nil, | ||
nil, | ||
hardcodedResponseIPValidator{response: true}, | ||
|
||
for _, test := range testCases { | ||
deps := &endpointDeps{ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd suggest moving the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moved. |
||
&nobidExchange{}, | ||
newParamsValidator(t), | ||
&mockStoredReqFetcher{}, | ||
empty_fetcher.EmptyFetcher{}, | ||
empty_fetcher.EmptyFetcher{}, | ||
empty_fetcher.EmptyFetcher{}, | ||
&config.Configuration{MaxRequestSize: int64(8096)}, | ||
pbsmetrics.NewMetrics(metrics.NewRegistry(), openrtb_ext.BidderList(), config.DisabledMetrics{}), | ||
analyticsConf.NewPBSAnalytics(&config.Analytics{}), | ||
map[string]string{"unknownbidder": "The bidder 'unknownbidder' has been disabled."}, | ||
false, | ||
[]byte{}, | ||
openrtb_ext.BidderMap, | ||
nil, | ||
nil, | ||
hardcodedResponseIPValidator{response: true}, | ||
} | ||
|
||
imp := &openrtb.Imp{Ext: test.impExt} | ||
errs := deps.validateImpExt(imp, nil, 0) | ||
|
||
if len(test.expectedImpExt) > 0 { | ||
assert.JSONEq(t, test.expectedImpExt, string(imp.Ext)) | ||
} else { | ||
assert.Empty(t, imp.Ext) | ||
} | ||
|
||
assert.Equal(t, test.expectedErrs, errs) | ||
} | ||
errs := deps.validateImpExt(imp, nil, 0) | ||
assert.JSONEq(t, `{"appnexus":{"placement_id":555}}`, string(imp.Ext)) | ||
assert.Equal(t, []error{&errortypes.BidderTemporarilyDisabled{Message: "The bidder 'unknownbidder' has been disabled."}}, errs) | ||
} | ||
|
||
func validRequest(t *testing.T, filename string) string { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
{ | ||
"description": "xxx", | ||
|
||
"requestPayload": { | ||
"id": "some-request-id", | ||
"site": { | ||
"page": "test.somepage.com" | ||
}, | ||
"imp": [{ | ||
"id": "some-imp-id", | ||
"banner": { | ||
"format": [{ | ||
"w": 600, | ||
"h": 500 | ||
}, { | ||
"w": 300, | ||
"h": 600 | ||
}] | ||
}, | ||
"ext": { | ||
"appnexus": { | ||
"placementId": 12883451 | ||
}, | ||
"context": { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This throws a validation exception currently. |
||
"data": { | ||
"keywords": "prebid server example" | ||
} | ||
} | ||
} | ||
}] | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,173 @@ | ||
{ | ||
"incomingRequest": { | ||
"ortbRequest": { | ||
"id": "some-request-id", | ||
"site": { | ||
"page": "test.somepage.com" | ||
}, | ||
"imp": [{ | ||
"id": "some-imp-id", | ||
"banner": { | ||
"format": [{ | ||
"w": 600, | ||
"h": 500 | ||
}, { | ||
"w": 300, | ||
"h": 600 | ||
}] | ||
}, | ||
"ext": { | ||
"appnexus": { | ||
"placementId": 1 | ||
}, | ||
"rubicon": { | ||
"accountId": 1, | ||
"siteId": 2, | ||
"zoneId": 3 | ||
}, | ||
"context": { | ||
"data": { | ||
"keywords": "prebid server example" | ||
} | ||
} | ||
} | ||
}] | ||
} | ||
}, | ||
"outgoingRequests": { | ||
"appnexus": { | ||
"expectRequest": { | ||
"ortbRequest": { | ||
"id": "some-request-id", | ||
"site": { | ||
"page": "test.somepage.com" | ||
}, | ||
"imp": [{ | ||
"id": "some-imp-id", | ||
"banner": { | ||
"format": [{ | ||
"w": 600, | ||
"h": 500 | ||
}, { | ||
"w": 300, | ||
"h": 600 | ||
}] | ||
}, | ||
"ext": { | ||
"bidder": { | ||
"placementId": 1 | ||
}, | ||
"context": { | ||
"data": { | ||
"keywords": "prebid server example" | ||
} | ||
} | ||
} | ||
}] | ||
}, | ||
"bidAdjustment": 1.0 | ||
}, | ||
"mockResponse": { | ||
"pbsSeatBid": { | ||
"pbsBids": [{ | ||
"ortbBid": { | ||
"id": "apn-bid", | ||
"impid": "some-imp-id", | ||
"price": 0.3, | ||
"w": 200, | ||
"h": 500, | ||
"crid": "creative-1" | ||
}, | ||
"bidType": "banner" | ||
}] | ||
} | ||
} | ||
}, | ||
"rubicon": { | ||
"expectRequest": { | ||
"ortbRequest": { | ||
"id": "some-request-id", | ||
"site": { | ||
"page": "test.somepage.com" | ||
}, | ||
"imp": [{ | ||
"id": "some-imp-id", | ||
"banner": { | ||
"format": [{ | ||
"w": 600, | ||
"h": 500 | ||
}, { | ||
"w": 300, | ||
"h": 600 | ||
}] | ||
}, | ||
"ext": { | ||
"bidder": { | ||
"accountId": 1, | ||
"siteId": 2, | ||
"zoneId": 3 | ||
}, | ||
"context": { | ||
"data": { | ||
"keywords": "prebid server example" | ||
} | ||
} | ||
} | ||
}] | ||
}, | ||
"bidAdjustment": 1.0 | ||
}, | ||
"mockResponse": { | ||
"pbsSeatBid": { | ||
"pbsBids": [{ | ||
"ortbBid": { | ||
"id": "rubi-bid", | ||
"impid": "some-imp-id", | ||
"price": 0.4, | ||
"w": 200, | ||
"h": 500, | ||
"crid": "creative-2" | ||
}, | ||
"bidType": "banner" | ||
}] | ||
} | ||
} | ||
} | ||
}, | ||
"response": { | ||
"bids": { | ||
"id": "some-request-id", | ||
"seatbid": [{ | ||
"seat": "appnexus", | ||
"bid": [{ | ||
"id": "apn-bid", | ||
"impid": "some-imp-id", | ||
"price": 0.3, | ||
"w": 200, | ||
"h": 500, | ||
"crid": "creative-1", | ||
"ext": { | ||
"prebid": { | ||
"type": "banner" | ||
} | ||
} | ||
}] | ||
}, { | ||
"seat": "rubicon", | ||
"bid": [{ | ||
"id": "rubi-bid", | ||
"impid": "some-imp-id", | ||
"price": 0.4, | ||
"w": 200, | ||
"h": 500, | ||
"crid": "creative-2", | ||
"ext": { | ||
"prebid": { | ||
"type": "banner" | ||
} | ||
} | ||
}] | ||
}] | ||
} | ||
} | ||
} |
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.
Should it be
RequestFirstPartyDataContextExtKey
to match thePrebidExtKey
pattern?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.
I'll rename it to
FirstPartyDataContextExtKey
. Push incoming.