-
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
CCPA Phase 1: AMP Endpoint #1125
Changes from all commits
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 |
---|---|---|
|
@@ -739,6 +739,102 @@ func TestWidthOnly(t *testing.T) { | |
}.execute(t) | ||
} | ||
|
||
func TestCCPAPresent(t *testing.T) { | ||
req, err := getTestBidRequest(false, false, "", "digitrustId") | ||
if err != nil { | ||
t.Fatalf("Failed to marshal the complete openrtb.BidRequest object %v", err) | ||
} | ||
|
||
reqStored := map[string]json.RawMessage{ | ||
"1": json.RawMessage(req), | ||
} | ||
|
||
theMetrics := pbsmetrics.NewMetrics(metrics.NewRegistry(), openrtb_ext.BidderList(), config.DisabledMetrics{}) | ||
|
||
exchange := &mockAmpExchange{} | ||
|
||
endpoint, _ := NewAmpEndpoint( | ||
exchange, | ||
newParamsValidator(t), | ||
&mockAmpStoredReqFetcher{reqStored}, | ||
empty_fetcher.EmptyFetcher{}, | ||
&config.Configuration{MaxRequestSize: maxSize}, | ||
theMetrics, | ||
analyticsConf.NewPBSAnalytics(&config.Analytics{}), | ||
map[string]string{}, | ||
[]byte{}, | ||
openrtb_ext.BidderMap, | ||
) | ||
|
||
usPrivacy := "1YYN" | ||
httpReq := httptest.NewRequest("GET", "/openrtb2/auction/amp?tag_id=1&us_privacy="+usPrivacy, nil) | ||
httpRecorder := httptest.NewRecorder() | ||
endpoint(httpRecorder, httpReq, nil) | ||
|
||
// Assert our bidRequest was valid | ||
if !assert.NotNil(t, exchange.lastRequest, "Endpoint responded with %d: %s", httpRecorder.Code, httpRecorder.Body.String()) { | ||
return | ||
} | ||
// Assert our bidRequest had a valid "Regs" field | ||
if !assert.NotNil(t, exchange.lastRequest.Regs) { | ||
return | ||
} | ||
// Assert our bidRequest had a valid "Regs.Ext" field | ||
if !assert.NotNil(t, exchange.lastRequest.Regs.Ext) { | ||
return | ||
} | ||
|
||
var regs openrtb_ext.ExtRegs | ||
err = json.Unmarshal(exchange.lastRequest.Regs.Ext, ®s) | ||
assert.NoError(t, err) | ||
assert.Equal(t, usPrivacy, regs.USPrivacy) | ||
} | ||
|
||
func TestCCPANotPresent(t *testing.T) { | ||
req, err := getTestBidRequest(false, false, "", "digitrustId") | ||
if err != nil { | ||
t.Fatalf("Failed to marshal the complete openrtb.BidRequest object %v", err) | ||
} | ||
|
||
reqStored := map[string]json.RawMessage{ | ||
"1": json.RawMessage(req), | ||
} | ||
|
||
theMetrics := pbsmetrics.NewMetrics(metrics.NewRegistry(), openrtb_ext.BidderList(), config.DisabledMetrics{}) | ||
|
||
exchange := &mockAmpExchange{} | ||
|
||
endpoint, _ := NewAmpEndpoint( | ||
exchange, | ||
newParamsValidator(t), | ||
&mockAmpStoredReqFetcher{reqStored}, | ||
empty_fetcher.EmptyFetcher{}, | ||
&config.Configuration{MaxRequestSize: maxSize}, | ||
theMetrics, | ||
analyticsConf.NewPBSAnalytics(&config.Analytics{}), | ||
map[string]string{}, | ||
[]byte{}, | ||
openrtb_ext.BidderMap, | ||
) | ||
|
||
httpReq := httptest.NewRequest("GET", "/openrtb2/auction/amp?tag_id=1", nil) | ||
httpRecorder := httptest.NewRecorder() | ||
endpoint(httpRecorder, httpReq, nil) | ||
|
||
// Assert our bidRequest was valid | ||
if !assert.NotNil(t, exchange.lastRequest, "Endpoint responded with %d: %s", httpRecorder.Code, httpRecorder.Body.String()) { | ||
return | ||
} | ||
|
||
// Assert CCPA Signal Not Found | ||
if exchange.lastRequest.Regs != nil && exchange.lastRequest.Regs.Ext != nil { | ||
var regs openrtb_ext.ExtRegs | ||
err = json.Unmarshal(exchange.lastRequest.Regs.Ext, ®s) | ||
assert.NoError(t, err) | ||
assert.Empty(t, regs.USPrivacy) | ||
} | ||
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. Should we log an error if we don't make it into the true branch of the if statement? 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 don't think so. I'm testing that the signal isn't present, so if there is no |
||
} | ||
|
||
type formatOverrideSpec struct { | ||
width uint64 | ||
height uint64 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
package ccpa | ||
|
||
import ( | ||
"encoding/json" | ||
|
||
"github.com/buger/jsonparser" | ||
"github.com/mxmCherry/openrtb" | ||
) | ||
|
||
// Policy represents the CCPA regulation for an OpenRTB bid request. | ||
type Policy struct { | ||
Value string | ||
} | ||
|
||
// Write mutates an OpenRTB bid request with the context of the CCPA policy. | ||
func (p Policy) Write(req *openrtb.BidRequest) error { | ||
if p.Value == "" { | ||
return nil | ||
} | ||
|
||
if req.Regs == nil { | ||
req.Regs = &openrtb.Regs{} | ||
} | ||
|
||
if req.Regs.Ext == nil { | ||
req.Regs.Ext = json.RawMessage(`{"us_privacy":"` + p.Value + `"}`) | ||
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. It's a bit strange how the CCPA regulation maps to 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. I believe we can call it the same. Maybe someone has a better take on it but I'm fine with it. 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 think the idea is that several states are enacting similar laws, and we will just filter based on the most strict version of the law as "us_privacy" signal. 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. Cool. Kept CCPA in code for now. |
||
return nil | ||
} | ||
|
||
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. Before we do lines 31 to 33, shouldn't we check the lenght of the string first? It occurs to me that if
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. Another option is probably to do the same as in the GDPR field and just:
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. Do we even need a string pointer, as not set and empty mean the same thing for the signal? 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've been going back and forth a lot on this. If we get an empty string in the AMP params then adding an empty string in the OpenRTB request makes sense. If we receive no query param at all in the AMP params then does it still make sense to add an empty string to the OpenRTB request? Or perhaps just treat nil and empty as the same and only add non-empty values in the OpenRTB request. In GDPR land, the signal is an int but we have it parsed as an int pointer because nil and empty (0) mean different things. 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. String pointer removed. |
||
var err error | ||
req.Regs.Ext, err = jsonparser.Set(req.Regs.Ext, []byte(`"`+p.Value+`"`), "us_privacy") | ||
return err | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,74 @@ | ||
package ccpa | ||
|
||
import ( | ||
"encoding/json" | ||
"testing" | ||
|
||
"github.com/mxmCherry/openrtb" | ||
"github.com/stretchr/testify/assert" | ||
) | ||
|
||
func TestWrite(t *testing.T) { | ||
testCases := []struct { | ||
description string | ||
policy Policy | ||
request *openrtb.BidRequest | ||
expected *openrtb.BidRequest | ||
expectedError bool | ||
}{ | ||
{ | ||
description: "Disabled", | ||
policy: Policy{Value: ""}, | ||
request: &openrtb.BidRequest{}, | ||
expected: &openrtb.BidRequest{}, | ||
}, | ||
{ | ||
description: "Enabled With Nil Request Regs Object", | ||
policy: Policy{Value: "anyValue"}, | ||
request: &openrtb.BidRequest{}, | ||
expected: &openrtb.BidRequest{Regs: &openrtb.Regs{ | ||
Ext: json.RawMessage(`{"us_privacy":"anyValue"}`)}}, | ||
}, | ||
{ | ||
description: "Enabled With Nil Request Regs Ext Object", | ||
policy: Policy{Value: "anyValue"}, | ||
request: &openrtb.BidRequest{Regs: &openrtb.Regs{}}, | ||
expected: &openrtb.BidRequest{Regs: &openrtb.Regs{ | ||
Ext: json.RawMessage(`{"us_privacy":"anyValue"}`)}}, | ||
}, | ||
{ | ||
description: "Enabled With Existing Request Regs Ext Object - Doesn't Overwrite", | ||
policy: Policy{Value: "anyValue"}, | ||
request: &openrtb.BidRequest{Regs: &openrtb.Regs{ | ||
Ext: json.RawMessage(`{"existing":"any"}`)}}, | ||
expected: &openrtb.BidRequest{Regs: &openrtb.Regs{ | ||
Ext: json.RawMessage(`{"existing":"any","us_privacy":"anyValue"}`)}}, | ||
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've been looking in the codebase for the recipient field of 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. It represents an existing json value to test the overwriting (or lack thereof) behavior. |
||
}, | ||
{ | ||
description: "Enabled With Existing Request Regs Ext Object - Overwrites", | ||
policy: Policy{Value: "anyValue"}, | ||
request: &openrtb.BidRequest{Regs: &openrtb.Regs{ | ||
Ext: json.RawMessage(`{"existing":"any","us_privacy":"toBeOverwritten"}`)}}, | ||
expected: &openrtb.BidRequest{Regs: &openrtb.Regs{ | ||
Ext: json.RawMessage(`{"existing":"any","us_privacy":"anyValue"}`)}}, | ||
}, | ||
{ | ||
description: "Enabled With Existing Malformed Request Regs Ext Object", | ||
policy: Policy{Value: "anyValue"}, | ||
request: &openrtb.BidRequest{Regs: &openrtb.Regs{ | ||
Ext: json.RawMessage(`malformed`)}}, | ||
expectedError: true, | ||
}, | ||
} | ||
|
||
for _, test := range testCases { | ||
err := test.policy.Write(test.request) | ||
|
||
if test.expectedError { | ||
assert.Error(t, err, test.description) | ||
} else { | ||
assert.NoError(t, err, test.description) | ||
assert.Equal(t, test.expected, test.request, test.description) | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
package gdpr | ||
|
||
import ( | ||
"encoding/json" | ||
|
||
"github.com/buger/jsonparser" | ||
"github.com/mxmCherry/openrtb" | ||
) | ||
|
||
// Policy represents the GDPR regulation of an OpenRTB bid request. | ||
type Policy struct { | ||
Consent string | ||
} | ||
|
||
// Write mutates an OpenRTB bid request with the context of the GDPR policy. | ||
func (p Policy) Write(req *openrtb.BidRequest) error { | ||
if p.Consent == "" { | ||
return nil | ||
} | ||
|
||
if req.User == nil { | ||
req.User = &openrtb.User{} | ||
} | ||
|
||
if req.User.Ext == nil { | ||
req.User.Ext = json.RawMessage(`{"consent":"` + p.Consent + `"}`) | ||
return nil | ||
} | ||
|
||
var err error | ||
req.User.Ext, err = jsonparser.Set(req.User.Ext, []byte(`"`+p.Consent+`"`), "consent") | ||
return err | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,74 @@ | ||
package gdpr | ||
|
||
import ( | ||
"encoding/json" | ||
"testing" | ||
|
||
"github.com/mxmCherry/openrtb" | ||
"github.com/stretchr/testify/assert" | ||
) | ||
|
||
func TestWrite(t *testing.T) { | ||
testCases := []struct { | ||
description string | ||
policy Policy | ||
request *openrtb.BidRequest | ||
expected *openrtb.BidRequest | ||
expectedError bool | ||
}{ | ||
{ | ||
description: "Disabled", | ||
policy: Policy{Consent: ""}, | ||
request: &openrtb.BidRequest{}, | ||
expected: &openrtb.BidRequest{}, | ||
}, | ||
{ | ||
description: "Enabled With Nil Request User Object", | ||
policy: Policy{Consent: "anyValue"}, | ||
request: &openrtb.BidRequest{}, | ||
expected: &openrtb.BidRequest{User: &openrtb.User{ | ||
Ext: json.RawMessage(`{"consent":"anyValue"}`)}}, | ||
}, | ||
{ | ||
description: "Enabled With Nil Request User Ext Object", | ||
policy: Policy{Consent: "anyValue"}, | ||
request: &openrtb.BidRequest{User: &openrtb.User{}}, | ||
expected: &openrtb.BidRequest{User: &openrtb.User{ | ||
Ext: json.RawMessage(`{"consent":"anyValue"}`)}}, | ||
}, | ||
{ | ||
description: "Enabled With Existing Request User Ext Object - Doesn't Overwrite", | ||
policy: Policy{Consent: "anyValue"}, | ||
request: &openrtb.BidRequest{User: &openrtb.User{ | ||
Ext: json.RawMessage(`{"existing":"any"}`)}}, | ||
expected: &openrtb.BidRequest{User: &openrtb.User{ | ||
Ext: json.RawMessage(`{"existing":"any","consent":"anyValue"}`)}}, | ||
}, | ||
{ | ||
description: "Enabled With Existing Request User Ext Object - Overwrites", | ||
policy: Policy{Consent: "anyValue"}, | ||
request: &openrtb.BidRequest{User: &openrtb.User{ | ||
Ext: json.RawMessage(`{"existing":"any","consent":"toBeOverwritten"}`)}}, | ||
expected: &openrtb.BidRequest{User: &openrtb.User{ | ||
Ext: json.RawMessage(`{"existing":"any","consent":"anyValue"}`)}}, | ||
}, | ||
{ | ||
description: "Enabled With Existing Malformed Request User Ext Object", | ||
policy: Policy{Consent: "anyValue"}, | ||
request: &openrtb.BidRequest{User: &openrtb.User{ | ||
Ext: json.RawMessage(`malformed`)}}, | ||
expectedError: true, | ||
}, | ||
} | ||
|
||
for _, test := range testCases { | ||
err := test.policy.Write(test.request) | ||
|
||
if test.expectedError { | ||
assert.Error(t, err, test.description) | ||
} else { | ||
assert.NoError(t, err, test.description) | ||
assert.Equal(t, test.expected, test.request, test.description) | ||
} | ||
} | ||
} |
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.
The GDPR implementation is scattered throughout the code. I'm beginning to consolidate the logic to a privacy package. I did a full refactor locally, but it was way too much change... so I'll be phasing it in.