-
Notifications
You must be signed in to change notification settings - Fork 765
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
New Adapter: Smoot #4148
New Adapter: Smoot #4148
Changes from 2 commits
7c0f3cf
4fefda1
e61e169
6ac536d
7423f36
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 |
---|---|---|
@@ -0,0 +1,47 @@ | ||
package smoot | ||
|
||
import ( | ||
"encoding/json" | ||
"testing" | ||
|
||
"github.com/prebid/prebid-server/v3/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 schema. %v", err) | ||
} | ||
|
||
for _, p := range validParams { | ||
if err := validator.Validate(openrtb_ext.BidderSmoot, json.RawMessage(p)); err != nil { | ||
t.Errorf("Schema rejected valid params: %s", p) | ||
} | ||
} | ||
} | ||
|
||
func TestInvalidParams(t *testing.T) { | ||
validator, err := openrtb_ext.NewBidderParamsValidator("../../static/bidder-params") | ||
if err != nil { | ||
t.Fatalf("Failed to fetch the json schema. %v", err) | ||
} | ||
|
||
for _, p := range invalidParams { | ||
if err := validator.Validate(openrtb_ext.BidderSmoot, json.RawMessage(p)); err == nil { | ||
t.Errorf("Schema allowed invalid params: %s", p) | ||
} | ||
} | ||
} | ||
|
||
var validParams = []string{ | ||
`{"placementId": "test"}`, | ||
`{"placementId": "1"}`, | ||
`{"endpointId": "test"}`, | ||
`{"endpointId": "1"}`, | ||
} | ||
|
||
var invalidParams = []string{ | ||
`{"placementId": 42}`, | ||
`{"endpointId": 42}`, | ||
`{"placementId": "1", "endpointId": "1"}`, | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,153 @@ | ||
package smoot | ||
|
||
import ( | ||
"encoding/json" | ||
"fmt" | ||
"net/http" | ||
|
||
"github.com/prebid/openrtb/v20/openrtb2" | ||
"github.com/prebid/prebid-server/v3/adapters" | ||
"github.com/prebid/prebid-server/v3/config" | ||
"github.com/prebid/prebid-server/v3/openrtb_ext" | ||
"github.com/prebid/prebid-server/v3/util/jsonutil" | ||
) | ||
|
||
type adapter struct { | ||
endpoint string | ||
} | ||
|
||
type reqBodyExt struct { | ||
SmootBidderExt reqBodyExtBidder `json:"bidder"` | ||
} | ||
|
||
type reqBodyExtBidder struct { | ||
Type string `json:"type"` | ||
PlacementID string `json:"placementId,omitempty"` | ||
EndpointID string `json:"endpointId,omitempty"` | ||
} | ||
|
||
func Builder(bidderName openrtb_ext.BidderName, config config.Adapter, server config.Server) (adapters.Bidder, error) { | ||
bidder := &adapter{ | ||
endpoint: config.Endpoint, | ||
} | ||
return bidder, nil | ||
} | ||
|
||
func (a *adapter) MakeRequests(request *openrtb2.BidRequest, reqInfo *adapters.ExtraRequestInfo) ([]*adapters.RequestData, []error) { | ||
var errs []error | ||
var adapterRequests []*adapters.RequestData | ||
|
||
reqCopy := *request | ||
for _, imp := range request.Imp { | ||
reqCopy.Imp = []openrtb2.Imp{imp} | ||
|
||
var bidderExt adapters.ExtImpBidder | ||
var smootExt openrtb_ext.ImpExtSmoot | ||
|
||
if err := jsonutil.Unmarshal(imp.Ext, &bidderExt); err != 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. Can you add test coverage on these unmarshal errors 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. Yes, sorry for that. Done ✅ |
||
errs = append(errs, err) | ||
continue | ||
} | ||
if err := jsonutil.Unmarshal(bidderExt.Bidder, &smootExt); err != nil { | ||
errs = append(errs, err) | ||
continue | ||
} | ||
|
||
impExt := reqBodyExt{SmootBidderExt: reqBodyExtBidder{}} | ||
|
||
if smootExt.PlacementID != "" { | ||
impExt.SmootBidderExt.PlacementID = smootExt.PlacementID | ||
impExt.SmootBidderExt.Type = "publisher" | ||
} else if smootExt.EndpointID != "" { | ||
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 conditional can be deleted with 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. Done ✅ |
||
impExt.SmootBidderExt.EndpointID = smootExt.EndpointID | ||
impExt.SmootBidderExt.Type = "network" | ||
} | ||
|
||
finalyImpExt, err := json.Marshal(impExt) | ||
if err != nil { | ||
errs = append(errs, err) | ||
continue | ||
} | ||
|
||
reqCopy.Imp[0].Ext = finalyImpExt | ||
|
||
adapterReq, err := a.makeRequest(&reqCopy) | ||
if err != nil { | ||
errs = append(errs, err) | ||
continue | ||
} | ||
|
||
adapterRequests = append(adapterRequests, adapterReq) | ||
} | ||
|
||
return adapterRequests, errs | ||
} | ||
|
||
func (a *adapter) makeRequest(request *openrtb2.BidRequest) (*adapters.RequestData, error) { | ||
reqJSON, err := json.Marshal(request) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
headers := http.Header{} | ||
headers.Add("Content-Type", "application/json;charset=utf-8") | ||
headers.Add("Accept", "application/json") | ||
return &adapters.RequestData{ | ||
Method: http.MethodPost, | ||
Uri: a.endpoint, | ||
Body: reqJSON, | ||
Headers: headers, | ||
ImpIDs: openrtb_ext.GetImpIDs(request.Imp), | ||
}, nil | ||
} | ||
|
||
func (a *adapter) MakeBids(request *openrtb2.BidRequest, requestData *adapters.RequestData, responseData *adapters.ResponseData) (*adapters.BidderResponse, []error) { | ||
if adapters.IsResponseStatusCodeNoContent(responseData) { | ||
return nil, nil | ||
} | ||
|
||
if err := adapters.CheckResponseStatusCodeForErrors(responseData); err != nil { | ||
return nil, []error{err} | ||
} | ||
|
||
var response openrtb2.BidResponse | ||
if err := jsonutil.Unmarshal(responseData.Body, &response); err != nil { | ||
return nil, []error{err} | ||
} | ||
|
||
bidResponse := adapters.NewBidderResponseWithBidsCapacity(len(request.Imp)) | ||
if len(response.Cur) != 0 { | ||
bidResponse.Currency = response.Cur | ||
} | ||
|
||
for _, seatBid := range response.SeatBid { | ||
for i := range seatBid.Bid { | ||
bid := seatBid.Bid[i] | ||
bidType, err := getBidType(bid) | ||
leamarty marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if err != nil { | ||
return nil, []error{err} | ||
leamarty marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
b := &adapters.TypedBid{ | ||
Bid: &seatBid.Bid[i], | ||
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. What you have works. I'll leave it up to you but you can optimize this with a couple of simple tweaks so you don't ever copy the bid (which you're doing twice) but rather copy the bid address and pass that reference around to read the mtype and to include in the typed bid:
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. Decided not to do it |
||
BidType: bidType, | ||
} | ||
bidResponse.Bids = append(bidResponse.Bids, b) | ||
} | ||
} | ||
return bidResponse, nil | ||
} | ||
|
||
func getBidType(bid openrtb2.Bid) (openrtb_ext.BidType, error) { | ||
leamarty marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// determinate media type by bid response field mtype | ||
switch bid.MType { | ||
case openrtb2.MarkupBanner: | ||
return openrtb_ext.BidTypeBanner, nil | ||
case openrtb2.MarkupVideo: | ||
return openrtb_ext.BidTypeVideo, nil | ||
case openrtb2.MarkupNative: | ||
return openrtb_ext.BidTypeNative, nil | ||
} | ||
|
||
return "", fmt.Errorf("could not define media type for impression: %s", bid.ImpID) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
package smoot | ||
|
||
import ( | ||
"testing" | ||
|
||
"github.com/prebid/prebid-server/v3/adapters/adapterstest" | ||
"github.com/prebid/prebid-server/v3/config" | ||
"github.com/prebid/prebid-server/v3/openrtb_ext" | ||
) | ||
|
||
func TestJsonSamples(t *testing.T) { | ||
bidder, buildErr := Builder(openrtb_ext.BidderSmoot, config.Adapter{ | ||
Endpoint: "https://fake.test.io/pserver"}, config.Server{ExternalUrl: "http://hosturl.com", GvlID: 1, DataCenter: "2"}) | ||
|
||
if buildErr != nil { | ||
t.Fatalf("Builder returned unexpected error %v", buildErr) | ||
} | ||
|
||
adapterstest.RunJSONBidderTest(t, "smoottest", bidder) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,136 @@ | ||
{ | ||
"mockBidRequest": { | ||
"id": "test-request-id", | ||
"device": { | ||
"ip": "123.123.123.123", | ||
"ua": "iPad" | ||
}, | ||
"app": { | ||
"id": "1", | ||
"bundle": "com.wls.testwlsapplication" | ||
}, | ||
"imp": [ | ||
{ | ||
"id": "test-imp-id", | ||
"tagid": "test", | ||
"banner": { | ||
"format": [ | ||
{ | ||
"w": 300, | ||
"h": 250 | ||
}, | ||
{ | ||
"w": 300, | ||
"h": 600 | ||
} | ||
] | ||
}, | ||
"ext": { | ||
"bidder": { | ||
"endpointId": "test" | ||
} | ||
} | ||
} | ||
] | ||
}, | ||
"httpCalls": [ | ||
{ | ||
"expectedRequest": { | ||
"uri": "https://fake.test.io/pserver", | ||
"body": { | ||
"id": "test-request-id", | ||
"imp": [ | ||
{ | ||
"id": "test-imp-id", | ||
"tagid": "test", | ||
"banner": { | ||
"format": [ | ||
{ | ||
"w": 300, | ||
"h": 250 | ||
}, | ||
{ | ||
"w": 300, | ||
"h": 600 | ||
} | ||
] | ||
}, | ||
"ext": { | ||
"bidder": { | ||
"endpointId": "test", | ||
"type": "network" | ||
} | ||
} | ||
} | ||
], | ||
"app": { | ||
"id": "1", | ||
"bundle": "com.wls.testwlsapplication" | ||
}, | ||
"device": { | ||
"ip": "123.123.123.123", | ||
"ua": "iPad" | ||
} | ||
}, | ||
"impIDs": ["test-imp-id"] | ||
}, | ||
"mockResponse": { | ||
"status": 200, | ||
"body": { | ||
"id": "test-request-id", | ||
"seatbid": [ | ||
{ | ||
"bid": [ | ||
{ | ||
"id": "test_bid_id", | ||
"impid": "test-imp-id", | ||
"price": 0.27543, | ||
"adm": "<iframe id=\"adm-banner-16\" width=\"300\" height=\"250\" frameborder=\"0\" marginheight=\"0\" marginwidth=\"0\" style=\"{overflow:hidden}\" src=\"https://fake.test.io/pserver&k=882b2510ed6d6c94fa69c99aa522a708\"></iframe>", | ||
"cid": "test_cid", | ||
"crid": "test_crid", | ||
"dealid": "test_dealid", | ||
"mtype": 1, | ||
"w": 300, | ||
"h": 250, | ||
"ext": { | ||
"prebid": { | ||
"type": "banner" | ||
} | ||
} | ||
} | ||
], | ||
"seat": "smoot" | ||
} | ||
], | ||
"cur": "USD" | ||
} | ||
} | ||
} | ||
], | ||
"expectedBidResponses": [ | ||
{ | ||
"bids": [ | ||
{ | ||
"bid": { | ||
"id": "test_bid_id", | ||
"impid": "test-imp-id", | ||
"price": 0.27543, | ||
"adm": "<iframe id=\"adm-banner-16\" width=\"300\" height=\"250\" frameborder=\"0\" marginheight=\"0\" marginwidth=\"0\" style=\"{overflow:hidden}\" src=\"https://fake.test.io/pserver&k=882b2510ed6d6c94fa69c99aa522a708\"></iframe>", | ||
"cid": "test_cid", | ||
"crid": "test_crid", | ||
"dealid": "test_dealid", | ||
"mtype": 1, | ||
"w": 300, | ||
"h": 250, | ||
"ext": { | ||
"prebid": { | ||
"type": "banner" | ||
} | ||
} | ||
}, | ||
"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.
Can you add the following test cases to test the minimum length:
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.
Done ✅
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.
Adding these two separate test cases instead of combining them into one test case like you have (
{"placementId": "", "endpointId": ""}
ensures that there is a min length constraint on each field instead of on at least one of the fields. I suggest making this change.