-
Notifications
You must be signed in to change notification settings - Fork 930
SuperEdge: add new adapter #4846
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
base: master
Are you sure you want to change the base?
Changes from 5 commits
ed5fee0
d3c7ce8
c4d5bd4
f568cee
7e467a2
02b2019
07e4523
c5aed73
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,50 @@ | ||
| package superedge | ||
|
|
||
| import ( | ||
| "encoding/json" | ||
| "testing" | ||
|
|
||
| "github.com/prebid/prebid-server/v4/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.BidderSuperEdge, json.RawMessage(validParam)); err != nil { | ||
| t.Errorf("Schema rejected superEdge params: %s with err: %v", validParam, err) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // TestInvalidParams makes sure that the superEdge schema rejects all the imp.ext fields we don't support. | ||
| 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.BidderSuperEdge, json.RawMessage(invalidParam)); err == nil { | ||
| t.Errorf("Schema allowed unexpected params: %s", invalidParam) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| var validParams = []string{ | ||
| `{"sk": "7f096f84f44f4adfa7602f037179c98b"}`, | ||
| `{"sk": "1e9ead5397ae44d78c6792bc7cddc050"}`, | ||
| `{"sk": "27bb74d57068406ebcbb29ab9bfeb9b9"}`, | ||
| `{"sk": "0c3356713c184ca186779eecdd5aff5d"}`, | ||
| } | ||
|
|
||
| var invalidParams = []string{ | ||
| `{}`, | ||
| `{"tn": "0c3356713c184ca186779eecdd5aff5d"}`, | ||
| `{"region": "APAC"}`, | ||
| `{"region": "US"}`, | ||
| `{"tn": "27bb74d57068406ebcbb29ab9bfeb9b9"}`, | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,175 @@ | ||
| package superedge | ||
|
|
||
| import ( | ||
| "errors" | ||
| "fmt" | ||
| "net/http" | ||
| "text/template" | ||
|
|
||
| "github.com/prebid/openrtb/v20/openrtb2" | ||
| "github.com/prebid/prebid-server/v4/adapters" | ||
| "github.com/prebid/prebid-server/v4/config" | ||
| "github.com/prebid/prebid-server/v4/errortypes" | ||
| "github.com/prebid/prebid-server/v4/macros" | ||
| "github.com/prebid/prebid-server/v4/openrtb_ext" | ||
| "github.com/prebid/prebid-server/v4/util/jsonutil" | ||
| ) | ||
|
|
||
| type adapter struct { | ||
| EndpointTemplate *template.Template | ||
| } | ||
|
|
||
| // Builder builds a new instance of the SuperEdge adapter for the given bidder with the given config. | ||
| func Builder(_ openrtb_ext.BidderName, config config.Adapter, _ config.Server) (adapters.Bidder, error) { | ||
| endpoint, err := template.New("").Parse(config.Endpoint) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("unable to parse endpoint url template: %v", err) | ||
| } | ||
| bidder := &adapter{EndpointTemplate: endpoint} | ||
| return bidder, nil | ||
| } | ||
|
|
||
| func (a *adapter) MakeRequests(request *openrtb2.BidRequest, _ *adapters.ExtraRequestInfo) ([]*adapters.RequestData, []error) { | ||
| var adapterRequests []*adapters.RequestData | ||
| var errs []error | ||
| adapterRequest, err := a.makeRequest(request) | ||
| if err == nil { | ||
| adapterRequests = append(adapterRequests, adapterRequest) | ||
| } else { | ||
| errs = append(errs, err) | ||
| } | ||
| return adapterRequests, errs | ||
| } | ||
|
|
||
| func (a *adapter) makeRequest(request *openrtb2.BidRequest) (*adapters.RequestData, error) { | ||
| superEdgeExt, err := getSuperEdgeExt(request) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| endPoint, err := a.getEndPoint(superEdgeExt) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| preProcess(request) | ||
| reqBody, err := jsonutil.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") | ||
| headers.Add("x-openrtb-version", "2.5") | ||
|
Contributor
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.
|
||
| return &adapters.RequestData{ | ||
| Method: "POST", | ||
| Uri: endPoint, | ||
| Body: reqBody, | ||
| Headers: headers, | ||
| ImpIDs: openrtb_ext.GetImpIDs(request.Imp), | ||
| }, nil | ||
| } | ||
|
|
||
| // getSuperEdgeExt extracts ExtSuperEdge from the first imp's ext.bidder or request.ext.prebid.bidderparams. | ||
| func getSuperEdgeExt(request *openrtb2.BidRequest) (*openrtb_ext.ExtSuperEdge, error) { | ||
| var extSuperEdge openrtb_ext.ExtSuperEdge | ||
|
|
||
| // Try to get sk from request.ext.prebid.bidderparams first | ||
| if request.Ext != nil { | ||
| reqExt := &openrtb_ext.ExtRequest{} | ||
| if err := jsonutil.Unmarshal(request.Ext, reqExt); err == nil { | ||
| if len(reqExt.Prebid.BidderParams) > 0 { | ||
| if err := jsonutil.Unmarshal(reqExt.Prebid.BidderParams, &extSuperEdge); err == nil && extSuperEdge.Sk != "" { | ||
| return &extSuperEdge, nil | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
Contributor
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.
|
||
|
|
||
| // Fallback to first imp's ext.bidder | ||
| if len(request.Imp) == 0 { | ||
| return nil, errors.New("superEdge sk not found") | ||
| } | ||
|
|
||
| var extBidder adapters.ExtImpBidder | ||
| if err := jsonutil.Unmarshal(request.Imp[0].Ext, &extBidder); err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| if err := jsonutil.Unmarshal(extBidder.Bidder, &extSuperEdge); err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| if extSuperEdge.Sk != "" { | ||
| return &extSuperEdge, nil | ||
| } | ||
| return nil, errors.New("superEdge sk not found") | ||
| } | ||
|
|
||
| func (a *adapter) getEndPoint(ext *openrtb_ext.ExtSuperEdge) (string, error) { | ||
| return macros.ResolveMacros(a.EndpointTemplate, map[string]string{"sk": ext.Sk}) | ||
| } | ||
|
Comment on lines
+98
to
+115
Contributor
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.
|
||
|
|
||
| func preProcess(request *openrtb2.BidRequest) { | ||
| for i := range request.Imp { | ||
| if request.Imp[i].Banner != nil { | ||
| banner := *request.Imp[i].Banner | ||
| if (banner.W == nil || banner.H == nil || *banner.W == 0 || *banner.H == 0) && len(banner.Format) > 0 { | ||
| firstFormat := banner.Format[0] | ||
| banner.W = &firstFormat.W | ||
| banner.H = &firstFormat.H | ||
| request.Imp[i].Banner = &banner | ||
| } | ||
|
Collaborator
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 line mutates the caller's original Fix: make a shallow copy of the request and its Imp slice before calling func (a *adapter) makeRequest(request *openrtb2.BidRequest) (*adapters.RequestData, error) {
...
requestCopy := *request
requestCopy.Imp = make([]openrtb2.Imp, len(request.Imp))
copy(requestCopy.Imp, request.Imp)
preProcess(&requestCopy)
reqBody, err := jsonutil.Marshal(&requestCopy)
...
} |
||
| } | ||
| } | ||
| } | ||
|
|
||
| func (a *adapter) MakeBids(internalRequest *openrtb2.BidRequest, _ *adapters.RequestData, response *adapters.ResponseData) (*adapters.BidderResponse, []error) { | ||
| if adapters.IsResponseStatusCodeNoContent(response) { | ||
| return nil, nil | ||
| } | ||
| if err := adapters.CheckResponseStatusCodeForErrors(response); err != nil { | ||
| return nil, []error{err} | ||
| } | ||
| var bidResp openrtb2.BidResponse | ||
| if err := jsonutil.Unmarshal(response.Body, &bidResp); err != nil { | ||
| return nil, []error{err} | ||
| } | ||
| bidResponse := adapters.NewBidderResponseWithBidsCapacity(1) | ||
|
Contributor
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. Response currency dropped — builds "Please avoid common mistakes, such as not specifying the bid currency and not properly detecting the media type from the bidding server response." — https://docs.prebid.org/prebid-server/developers/add-new-bidder-go.html |
||
| var errs []error | ||
| for _, seatBid := range bidResp.SeatBid { | ||
| for idx := range seatBid.Bid { | ||
| bidType, err := getBidType(seatBid.Bid[idx], internalRequest.Imp) | ||
| if err != nil { | ||
| errs = append(errs, err) | ||
| } else { | ||
| bidResponse.Bids = append(bidResponse.Bids, &adapters.TypedBid{ | ||
| Bid: &seatBid.Bid[idx], | ||
| BidType: bidType, | ||
| }) | ||
| } | ||
| } | ||
| } | ||
| return bidResponse, errs | ||
| } | ||
|
|
||
| func getBidType(bid openrtb2.Bid, imps []openrtb2.Imp) (openrtb_ext.BidType, error) { | ||
| switch bid.MType { | ||
| case openrtb2.MarkupBanner: | ||
| return openrtb_ext.BidTypeBanner, nil | ||
| case openrtb2.MarkupNative: | ||
| return openrtb_ext.BidTypeNative, nil | ||
| default: | ||
| for _, imp := range imps { | ||
| if imp.ID == bid.ImpID { | ||
| if imp.Banner != 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. Consider this as a suggestion. The current implementation follows an anti-pattern, assumes that if there is a multi-format request, the media type defaults to openrtb_ext.BidTypeBanner, nil. Prebid server expects the media type to be explicitly set in the adapter response. Therefore, we strongly recommend implementing a pattern where the adapter server sets the MType field in the response to accurately determine the media type for the impression. |
||
| return openrtb_ext.BidTypeBanner, nil | ||
| } | ||
| if imp.Native != 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. Consider this as a suggestion. The current implementation follows an anti-pattern, assumes that if there is a multi-format request, the media type defaults to openrtb_ext.BidTypeNative, nil. Prebid server expects the media type to be explicitly set in the adapter response. Therefore, we strongly recommend implementing a pattern where the adapter server sets the MType field in the response to accurately determine the media type for the impression. |
||
| return openrtb_ext.BidTypeNative, nil | ||
| } | ||
| } | ||
| } | ||
| return "", &errortypes.BadServerResponse{ | ||
| Message: fmt.Sprintf("Unsupported MType %d", bid.MType), | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| package superedge | ||
|
|
||
| import ( | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/assert" | ||
|
|
||
| "github.com/prebid/prebid-server/v4/adapters/adapterstest" | ||
| "github.com/prebid/prebid-server/v4/config" | ||
| "github.com/prebid/prebid-server/v4/openrtb_ext" | ||
| ) | ||
|
|
||
| func TestJsonSamples(t *testing.T) { | ||
| bidder, buildErr := Builder(openrtb_ext.BidderSuperEdge, config.Adapter{ | ||
| Endpoint: "https://rtb-us.superedge.co.jp/bid?sk={{.sk}}"}, config.Server{ExternalUrl: "http://hosturl.com"}) | ||
|
|
||
| if buildErr != nil { | ||
| t.Fatalf("Builder returned unexpected error %v", buildErr) | ||
| } | ||
|
|
||
| adapterstest.RunJSONBidderTest(t, "superedgetest", bidder) | ||
| } | ||
|
|
||
| func TestEndpointTemplateMalformed(t *testing.T) { | ||
| _, buildErr := Builder(openrtb_ext.BidderSuperEdge, config.Adapter{Endpoint: "{{Malformed}}"}, config.Server{ExternalUrl: "http://hosturl.com"}) | ||
|
|
||
| assert.Error(t, buildErr) | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,122 @@ | ||
| { | ||
|
Collaborator
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. please add multi imps example
Author
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. Thanks, I will handle the issues with glvid and multi-imp as soon as possible. |
||
| "mockBidRequest": { | ||
| "id": "test-request-id", | ||
| "imp": [ | ||
| { | ||
| "id": "test-imp-id", | ||
| "banner": { | ||
| "format": [ | ||
| { | ||
| "w": 320, | ||
| "h": 50 | ||
| } | ||
| ] | ||
| }, | ||
| "ext": { | ||
| "bidder": { | ||
| "sk": "f9f2b1ef23fe2759c2cad0953029a94b", | ||
| "placementId": "testPlacementId", | ||
| "region": "APAC" | ||
| } | ||
| } | ||
| } | ||
| ], | ||
| "site": { | ||
| "id": "test-site-id", | ||
| "page": "https://www.example.com/" | ||
| }, | ||
| "ext": { | ||
| "prebid": { | ||
| "bidderparams": { | ||
| "sk": "f9f2b1ef23fe2759c2cad0953029a94b", | ||
| "region": "APAC" | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "httpCalls": [ | ||
| { | ||
| "expectedRequest": { | ||
| "uri": "https://rtb-us.superedge.co.jp/bid?sk=f9f2b1ef23fe2759c2cad0953029a94b", | ||
| "body": { | ||
| "id": "test-request-id", | ||
| "imp": [ | ||
| { | ||
| "id": "test-imp-id", | ||
| "banner": { | ||
| "format": [ | ||
| { | ||
| "w": 320, | ||
| "h": 50 | ||
| } | ||
| ], | ||
| "w": 320, | ||
| "h": 50 | ||
| }, | ||
| "ext": { | ||
| "bidder": { | ||
| "sk": "f9f2b1ef23fe2759c2cad0953029a94b", | ||
| "region": "APAC", | ||
| "placementId": "testPlacementId" | ||
| } | ||
| } | ||
| } | ||
| ], | ||
| "site": { | ||
| "id": "test-site-id", | ||
| "page": "https://www.example.com/" | ||
| }, | ||
| "ext": { | ||
| "prebid": { | ||
| "bidderparams": { | ||
| "sk": "f9f2b1ef23fe2759c2cad0953029a94b", | ||
| "region": "APAC" | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "impIDs": [ | ||
| "test-imp-id" | ||
| ] | ||
| }, | ||
| "mockResponse": { | ||
| "status": 200, | ||
| "body": { | ||
| "id": "test-request-id", | ||
| "seatbid": [ | ||
| { | ||
| "seat": "superedge", | ||
| "bid": [ | ||
| { | ||
| "id": "test-imp-id", | ||
| "impid": "test-imp-id", | ||
| "price": 0.5, | ||
| "adm": "some-ads", | ||
| "crid": "crid_testid" | ||
| } | ||
| ] | ||
| } | ||
| ], | ||
| "cur": "USD" | ||
| } | ||
| } | ||
| } | ||
| ], | ||
| "expectedBidResponses": [ | ||
| { | ||
| "currency": "USD", | ||
| "bids": [ | ||
| { | ||
| "bid": { | ||
| "id": "test-imp-id", | ||
| "impid": "test-imp-id", | ||
| "price": 0.5, | ||
| "adm": "some-ads", | ||
| "crid": "crid_testid" | ||
| }, | ||
| "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.
params_test.gomissing type-mismatch /minLengthcases —invalidParamscovers missing-sk+ unknown-field, but not{"sk":123}(type) or{"sk":""}(minLength:1). Two-line add."Please include tests for required fields, optional fields, conditional fields such as
oneOf, regex filters, and data type mismatches." — https://docs.prebid.org/prebid-server/developers/add-new-bidder-go.html