-
Notifications
You must be signed in to change notification settings - Fork 771
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
Idsync removal #1644
Idsync removal #1644
Conversation
Merging old code
Prebid-server latest status
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.
Files adapters/dmx/dmxtest/params/race/banner.json
and adapters/dmx/dmxtest/params/race/video.json
don't include the "memberid"
field that is the only required parameter according to static/bidder-params/dmx.json
. Should we add a "memberid"
field to these two files? On the other hand, should we make "tagid"
and "publisher_id"
required params?
Many other adapters run test cases on their params in a adapters/foo/params_test.go
file. Given the complexity of your adapter parameters, can we please include adapters/dmx/params_test.go
in order to double-check the expected parameters of your adapter?
@stevealliance Could you please update DMX's email in |
@guscarreon and @hhhjort, can we have this PR merge before schedule release? Thanks 👍 |
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.
Can you please add a adapters/dmx/params_test.go
file in order to test parameter validation? Something like:
package dmx
import (
"encoding/json"
"testing"
"github.com/prebid/prebid-server/openrtb_ext"
)
func TestValidParams(t *testing.T) {
validator, err := openrtb_ext.NewBidderParamsValidator("../../static/bidder-params")
if err != nil {
t.Fatalf("Failed to fetch the json-schemas. %v", err)
}
for _, validParam := range validParams {
if err := validator.Validate(openrtb_ext.BidderDmx, json.RawMessage(validParam)); err != nil {
t.Errorf("Schema rejected dmx params: %s", validParam)
}
}
}
func TestInvalidParams(t *testing.T) {
validator, err := openrtb_ext.NewBidderParamsValidator("../../static/bidder-params")
if err != nil {
t.Fatalf("Failed to fetch the json-schemas. %v", err)
}
for _, invalidParam := range invalidParams {
if err := validator.Validate(openrtb_ext.BidderDmx, json.RawMessage(invalidParam)); err == nil {
t.Errorf("Schema allowed unexpected params: %s", invalidParam)
}
}
}
var validParams = []string{
`{"memberid":"anyMemberId", "placement_id":"anyPlacementId", "seller_id":"anySellerId", "dmxid":"anyDmxid", "tagid": "25251", "bidfloor": 0.01}`,
`{"memberid": "anyMemberId", "seller_id": "anySellerId", "dmxid": "anyDmxid", "tagid": "25251", "bidfloor": 0.01}`,
`{"memberid": "anyMemberId", "placement_id": "anyPlacementId", "dmxid": "anyDmxid", "tagid": "25251", "bidfloor": 0.01}`,
`{"memberid": "anyMemberId", "placement_id": "anyPlacementId", "seller_id": "anySellerId", "tagid": "25251", "bidfloor": 0.01}`,
`{"memberid": "anyMemberId", "placement_id": "anyPlacementId", "seller_id": "anySellerId", "dmxid": "anyDmxid", "bidfloor": 0.01}`,
`{"memberid": "anyMemberId", "placement_id": "anyPlacementId", "seller_id": "anySellerId", "dmxid": "anyDmxid", "tagid": "25251"}`,
`{"memberid": "anyMemberId"}`,
}
var invalidParams = []string{
`null`,
`nil`,
``,
`[]`,
`true`,
`{}`,
// Invalid because missing "memberid"
`{"placement_id":"anyPlacementId", "seller_id":"anySellerId", "dmxid":"anyDmxid", "tagid": "25251", "bidfloor": 0.01}`,
// Invalid because "memberid" not a string
`{"memberid": 5}`,
//
// ...more invalid param scenarios
//
}
adapters/dmx/params_test.go
…idfloor is optional
Removing id sync since we need a server setup on our side to provide all client.
Another update to support Ext for publisher that are using the puslisher.id field in site & app