-
Notifications
You must be signed in to change notification settings - Fork 721
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
Yieldlab: Add Digital Service Act (DSA) handling #3473
Changes from all commits
b294d92
bd3004d
3061bba
7851634
01da46a
a0f2dde
915ab4e
3245491
e97787f
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 |
---|---|---|
|
@@ -257,6 +257,98 @@ func Test_makeSupplyChain(t *testing.T) { | |
} | ||
} | ||
|
||
func Test_makeDSATransparencyUrlParam(t *testing.T) { | ||
tests := []struct { | ||
name string | ||
transparencies []dsaTransparency | ||
expected string | ||
}{ | ||
{ | ||
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. You might want to consider adding two more test cases for when |
||
name: "No transparency objects", | ||
transparencies: []dsaTransparency{}, | ||
expected: "", | ||
}, | ||
{ | ||
name: "Nil transparency", | ||
transparencies: nil, | ||
expected: "", | ||
}, | ||
{ | ||
name: "Params without a domain", | ||
transparencies: []dsaTransparency{ | ||
{ | ||
Params: []int{1, 2}, | ||
}, | ||
}, | ||
expected: "", | ||
}, | ||
{ | ||
name: "Params without a params", | ||
transparencies: []dsaTransparency{ | ||
{ | ||
Domain: "domain.com", | ||
}, | ||
}, | ||
expected: "domain.com", | ||
}, | ||
{ | ||
name: "One object; No Params", | ||
transparencies: []dsaTransparency{ | ||
{ | ||
Domain: "domain.com", | ||
Params: []int{}, | ||
}, | ||
}, | ||
expected: "domain.com", | ||
}, | ||
{ | ||
name: "One object; One Param", | ||
transparencies: []dsaTransparency{ | ||
{ | ||
Domain: "domain.com", | ||
Params: []int{1}, | ||
}, | ||
}, | ||
expected: "domain.com~1", | ||
}, | ||
{ | ||
name: "Three domain objects", | ||
transparencies: []dsaTransparency{ | ||
{ | ||
Domain: "domain1.com", | ||
Params: []int{1, 2}, | ||
}, | ||
{ | ||
Domain: "domain2.com", | ||
Params: []int{3, 4}, | ||
}, | ||
{ | ||
Domain: "domain3.com", | ||
Params: []int{5, 6}, | ||
}, | ||
}, | ||
expected: "domain1.com~1_2~~domain2.com~3_4~~domain3.com~5_6", | ||
}, | ||
} | ||
for _, test := range tests { | ||
t.Run(test.name, func(t *testing.T) { | ||
actual := makeDSATransparencyURLParam(test.transparencies) | ||
assert.Equal(t, test.expected, actual) | ||
}) | ||
} | ||
} | ||
|
||
func Test_getDSA_invalidRequestExt(t *testing.T) { | ||
req := &openrtb2.BidRequest{ | ||
Regs: &openrtb2.Regs{Ext: json.RawMessage(`{"DSA":"wrongValueType"}`)}, | ||
} | ||
|
||
dsa, err := getDSA(req) | ||
|
||
assert.NotNil(t, err) | ||
assert.Nil(t, dsa) | ||
} | ||
Comment on lines
+341
to
+350
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. Error cases like this should be covered using the JSON test framework wherever possible. To do so, you can create a new directory
In your JSON test you declare the expected
|
||
|
||
func TestYieldlabAdapter_makeEndpointURL_invalidEndpoint(t *testing.T) { | ||
bidder, buildErr := Builder(openrtb_ext.BidderYieldlab, config.Adapter{ | ||
Endpoint: "test$:/something§"}, config.Server{ExternalUrl: "http://hosturl.com", GvlID: 1, DataCenter: "2"}) | ||
|
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.
It is unclear to me as to what this parameter is supposed to be. I'm guessing what you have here is correct but I'm trying to confirm. In Object Specification for OpenRTB 2.X it is
datatopub
and in URL-based support it isdsadatapubs
.I would have expected the two to be the same with maybe the URL-based parameter having a
dsa
prefix that the object field does not.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.
This is indeed correct. The have prefixed all our DSA URL parameters, but saw no reason why this parameter should be plural, even though this might be against the current spec.