Skip to content
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

Initial RhythmOne Prebid Server Adapter #704

Merged
merged 8 commits into from
Oct 15, 2018
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 52 additions & 0 deletions adapters/rhythmone/params_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
package rhythmone

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.BidderRhythmone, json.RawMessage(validParam)); err != nil {
t.Errorf("Schema rejected rhythmone 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.BidderRhythmone, json.RawMessage(invalidParam)); err == nil {
t.Errorf("Schema allowed unexpected params: %s", invalidParam)
}
}
}

var validParams = []string{
`{"placementId":"123", "zone":"12345", "path":"34567"}`,
}

var invalidParams = []string{
dineshkumarjayaram marked this conversation as resolved.
Show resolved Hide resolved
`{"appId":"123", "bidfloor":0.01}`,
`{"publisherName": 100}`,
`{"placementId": 1234}`,
`{"zone": true}`,
``,
`null`,
`nil`,
`true`,
`9`,
`[]`,
`{}`,
}
218 changes: 218 additions & 0 deletions adapters/rhythmone/rhythmone.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,218 @@
package rhythmone

import (
"encoding/json"
"fmt"

"github.com/mxmCherry/openrtb"
"github.com/prebid/prebid-server/adapters"
"github.com/prebid/prebid-server/errortypes"
"github.com/prebid/prebid-server/openrtb_ext"
"net/http"
)

type RhythmoneAdapter struct {
http *adapters.HTTPAdapter
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This field is unused... it can be deleted.

endPoint string
}

// used for cookies and such
func (a *RhythmoneAdapter) Name() string {
return "rhythmone"
}

func (a *RhythmoneAdapter) SkipNoCookies() bool {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Name() and SkipNoCookies() are legacy things... you don't need them.

return false
}

func (a *RhythmoneAdapter) MakeRequests(request *openrtb.BidRequest) ([]*adapters.RequestData, []error) {
errs := make([]error, 0, len(request.Imp))
if len(request.Imp) == 0 {
err := &errortypes.BadInput{
Message: "No imp data provided in the bid request.",
}
errs = append(errs, err)
return nil, errs
}

validImpDataExists := false

for _, imp := range request.Imp {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can delete this block. The adapters.EnforceBidderInfo() call in adapter_map makes sure your Imps have valid formats, according to your static/bidder-info/rhythmone.yaml definitions.

if imp.Banner != nil {
validImpDataExists = true
} else if imp.Video != nil {
validImpDataExists = true
} else {
err := &errortypes.BadInput{
Message: fmt.Sprintf("RhythmOne only supports banner and video imps. Ignoring imp id=%s", imp.ID),
}
errs = append(errs, err)
}
}

if !validImpDataExists {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here. This is guaranteed by the EnforceBidderInfo() stuff.

err := &errortypes.BadInput{
Message: fmt.Sprintf("No valid imp data in the bid request"),
}
errs = append(errs, err)
return nil, errs
}

var uri string
request, uri, errs = a.preProcess(request, errs)
if request != nil {
reqJSON, err := json.Marshal(request)
if err != nil {
errs = append(errs, err)
return nil, errs
}
if uri != "" {
headers := http.Header{}
headers.Add("Content-Type", "application/json;charset=utf-8")
headers.Add("Accept", "application/json")
return []*adapters.RequestData{{
Method: "POST",
Uri: uri,
Body: reqJSON,
Headers: headers,
}}, errs
}
}
return nil, errs
}

func (a *RhythmoneAdapter) MakeBids(internalRequest *openrtb.BidRequest, externalRequest *adapters.RequestData, response *adapters.ResponseData) (*adapters.BidderResponse, []error) {
if response.StatusCode == http.StatusNoContent {
return nil, nil
}

if response.StatusCode == http.StatusBadRequest {
return nil, []error{&errortypes.BadInput{
Message: fmt.Sprintf("unexpected status code: %d. Run with request.debug = 1 for more info", response.StatusCode),
}}
}

if response.StatusCode != http.StatusOK {
return nil, []error{&errortypes.BadServerResponse{
Message: fmt.Sprintf("unexpected status code: %d. Run with request.debug = 1 for more info", response.StatusCode),
}}
}
var bidResp openrtb.BidResponse
if err := json.Unmarshal(response.Body, &bidResp); err != nil {
return nil, []error{&errortypes.BadServerResponse{
Message: fmt.Sprintf("bad server response: %d. ", err),
}}
}

var errs []error
bidResponse := adapters.NewBidderResponseWithBidsCapacity(5)

for _, sb := range bidResp.SeatBid {
for i := range sb.Bid {
bidResponse.Bids = append(bidResponse.Bids, &adapters.TypedBid{
Bid: &sb.Bid[i],
BidType: getMediaTypeForImp(sb.Bid[i].ImpID, internalRequest.Imp),
})
}
}
return bidResponse, errs
}

func getMediaTypeForImp(impId string, imps []openrtb.Imp) openrtb_ext.BidType {
mediaType := openrtb_ext.BidTypeBanner
for _, imp := range imps {
if imp.ID == impId {
if imp.Banner != nil {
mediaType = openrtb_ext.BidTypeBanner
} else if imp.Video != nil {
mediaType = openrtb_ext.BidTypeVideo
}
if imp.Banner != nil && imp.Video != nil {
mediaType = openrtb_ext.BidTypeBanner
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This if imp.Banner != nil && imp.Video != nil { block is a no-op, and can be deleted.

If both are non-nil, the if imp.Banner != nil { block will already have set this to BidTypeBanner.

}
return mediaType
}
}
return mediaType
}

func NewRhythmoneAdapter(config *adapters.HTTPAdapterConfig, endpoint string) *RhythmoneAdapter {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is unused, and can be deleted.

return NewRhythmoneBidder(adapters.NewHTTPAdapter(config).Client, endpoint)
}

func NewRhythmoneBidder(client *http.Client, endpoint string) *RhythmoneAdapter {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

client is unused... please remove it.

a := &adapters.HTTPAdapter{Client: client}
return &RhythmoneAdapter{
http: a,
endPoint: endpoint,
}
}

func (a *RhythmoneAdapter) preProcess(req *openrtb.BidRequest, errors []error) (*openrtb.BidRequest, string, []error) {
numRequests := len(req.Imp)

requestImpCopy := req.Imp
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since req.Imp is a slice, and a slice is essentially a pointer to an existing array structure, requestImpCopy := req.Imp will end up referencing the same underlying array as req.Imp So it is not a true copy.

You only seem to use reqestImpCopy for imp := requestImpCopy[i], which will make imp a copy shallow of the original imp. So there does not seem to be a reason to define reqestImpCopy.

var uri string = ""
for i := 0; i < numRequests; i++ {
imp := requestImpCopy[i]

if imp.Audio != nil {
continue
}
if imp.Native != nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two blocks can go away... audio/native aren't possible.

continue
}
if imp.Banner != nil || imp.Video != nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This if clause can go away too. At least one of Banner or Video are guaranteed to be non-nil by the EnforceBidderInfo() thing.


var bidderExt adapters.ExtImpBidder
err := json.Unmarshal(imp.Ext, &bidderExt)

if err != nil {
err = &errortypes.BadInput{
Message: fmt.Sprintf("ext data not provided in imp id=%s. Abort all Request", imp.ID),
}
errors = append(errors, err)
return nil, "", errors
}
var rhythmoneExt openrtb_ext.ExtImpRhythmone
err = json.Unmarshal(bidderExt.Bidder, &rhythmoneExt)
if err != nil {
err = &errortypes.BadInput{
Message: fmt.Sprintf("placementId | zone | path not provided in imp id=%s. Abort all Request", imp.ID),
}
errors = append(errors, err)
return nil, "", errors
}

if rhythmoneExt.PlacementId == "" || rhythmoneExt.Zone == "" || rhythmoneExt.Path == "" {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This constraint should go in your static/bidder-params/rhythmone.json schema as a minlength: https://json-schema.org/understanding-json-schema/reference/string.html#length

Then you can remove this logic here. If the params don't validate against that schema, your adapter won't be called.

err = &errortypes.BadInput{
Message: fmt.Sprintf("placementId | zone | path empty in imp id=%s. Abort all Request", imp.ID),
}
errors = append(errors, err)
return nil, "", errors
}
rhythmoneExt.S2S = true
rhythmoneExtCopy, err := json.Marshal(&rhythmoneExt)
if err != nil {
errors = append(errors, err)
return nil, "", errors
}
bidderExtCopy := openrtb_ext.ExtBid{
Bidder: rhythmoneExtCopy,
}

impExtCopy, err := json.Marshal(&bidderExtCopy)
if err != nil {
errors = append(errors, err)
return nil, "", errors
}
imp.Ext = impExtCopy
req.Imp[i] = imp
if uri == "" {
uri = fmt.Sprintf("%s/%s/0/%s?z=%s&s2s=%s", a.endPoint, rhythmoneExt.PlacementId, rhythmoneExt.Path, rhythmoneExt.Zone, "true")
}
}
}

return req, uri, errors
}
10 changes: 10 additions & 0 deletions adapters/rhythmone/rhythmone_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package rhythmone

import (
"github.com/prebid/prebid-server/adapters/adapterstest"
"testing"
)

func TestJsonSamples(t *testing.T) {
adapterstest.RunJSONBidderTest(t, "rhythmonetest", NewRhythmoneBidder(nil, "http://tag.1rx.io/rmp"))
}
Loading