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

New Adapter: UNICORN #1719

Merged
merged 25 commits into from
Mar 4, 2021
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
b699f42
add bidder-info, bidder-params for UNICORN
faithnh Jan 28, 2021
956aa91
Add adapter
faithnh Feb 1, 2021
9bdb44b
Fix Bidder unicorn parameter name
faithnh Feb 2, 2021
0515c28
remove modifing request.app.ext, append modifing request.ext
faithnh Feb 3, 2021
36ae31f
add test code and fix adapter logic
faithnh Feb 4, 2021
ec49639
fix stype, add message that COPPA and GDPR and CCPA is not supported,…
faithnh Feb 5, 2021
725321d
change logic for placement_id not set. change stype. add test case
faithnh Feb 8, 2021
003e27d
indicate that user sync is not supported, add supplement test case
faithnh Feb 8, 2021
95844a4
banner only, native later support
faithnh Feb 8, 2021
ca3026a
fix description placementid
faithnh Feb 8, 2021
285c5d7
fix syncer_test.go
faithnh Feb 8, 2021
f1c2427
apply gofmt
faithnh Feb 9, 2021
42ae15a
Remove currency lock JPY
faithnh Feb 15, 2021
019800d
fix comment, fix test code 500.json
faithnh Feb 18, 2021
7be6aa9
Remove set AT
faithnh Feb 18, 2021
8b9dcd1
change maintener email address
faithnh Feb 19, 2021
f067fac
support fpd
faithnh Feb 22, 2021
8cb4906
apply fmt
faithnh Feb 22, 2021
74b363a
fix fpd format on testcode
faithnh Feb 22, 2021
3b1f698
fix PR problem ( Except for #discussion_r580592627)
faithnh Feb 24, 2021
1080b56
Remove comment on adapter
faithnh Feb 24, 2021
435bf6e
fix PR problem
faithnh Feb 25, 2021
c27da8b
fix PR problem again
faithnh Feb 26, 2021
fb58a7e
Fixed copying Source problem
faithnh Mar 1, 2021
deaf6c0
Fix review problem
faithnh Mar 3, 2021
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
61 changes: 61 additions & 0 deletions adapters/unicorn/params_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
package unicorn

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 schema. %v", err)
}

for _, p := range validParams {
if err := validator.Validate(openrtb_ext.BidderUnicorn, 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.BidderUnicorn, json.RawMessage(p)); err == nil {
t.Errorf("Schema allowed invalid params: %s", p)
}
}
}

var validParams = []string{
`{
"accountId": 199578,
"publisherId": 123456,
"mediaId": "test_media",
"placementId": "test_placement"
}`,
`{
"accountId": 199578,
"mediaId": "test_media"
}`,
}

var invalidParams = []string{
`{}`,
`{
"accountId": "199578",
"publisherId": "123456",
"mediaId": 12345,
"placementId": 12345
}`,
`{
"publisherId": 123456,
"placementId": "test_placement"
}`,
}
244 changes: 244 additions & 0 deletions adapters/unicorn/unicorn.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,244 @@
package unicorn

import (
"encoding/json"
"fmt"
"net/http"

"github.com/buger/jsonparser"
"github.com/mxmCherry/openrtb"
"github.com/prebid/prebid-server/adapters"
"github.com/prebid/prebid-server/config"
"github.com/prebid/prebid-server/errortypes"
"github.com/prebid/prebid-server/openrtb_ext"
)

type adapter struct {
endpoint string
}

// unicornImpExt is imp ext for UNICORN
type unicornImpExt struct {
Context *unicornImpExtContext `json:"context,omitempty"`
Bidder openrtb_ext.ExtImpUnicorn `json:"bidder"`
}

type unicornImpExtContext struct {
Data interface{} `json:"data,omitempty"`
}

// unicornExt is ext for UNICORN
type unicornExt struct {
Prebid *openrtb_ext.ExtImpPrebid `json:"prebid,omitempty"`
AccountID int64 `json:"accountId,omitempty"`
}

// Builder builds a new instance of the UNICORN adapter for the given bidder with the given config.
func Builder(bidderName openrtb_ext.BidderName, config config.Adapter) (adapters.Bidder, error) {
bidder := &adapter{
endpoint: config.Endpoint,
}
return bidder, nil
}

// MakeRequests makes the HTTP requests which should be made to fetch bids.
func (a *adapter) MakeRequests(request *openrtb.BidRequest, requestInfo *adapters.ExtraRequestInfo) ([]*adapters.RequestData, []error) {
var extRegs openrtb_ext.ExtRegs
if request.Regs != nil {
if request.Regs.COPPA == 1 {
return nil, []error{&errortypes.BadInput{
Message: "COPPA is not supported",
}}
}
if err := json.Unmarshal(request.Regs.Ext, &extRegs); err == nil {
if extRegs.GDPR != nil && (*extRegs.GDPR == 1) {
return nil, []error{&errortypes.BadInput{
Message: "GDPR is not supported",
}}
}
if extRegs.USPrivacy != "" {
return nil, []error{&errortypes.BadInput{
Message: "CCPA is not supported",
}}
}
}
}

err := modifyImps(request, requestInfo)
if err != nil {
return nil, []error{err}
}

request.Source.Ext = setSourceExt()
Copy link
Contributor

Choose a reason for hiding this comment

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

Prebid server core passes a shallow copy of the openrtb.BidRequest object to the MakeRequest() function which makes pointer fields like Source to be red flags because they point to shared memory.

 14 type BidRequest struct {
 15
          .
          .
185
186     // Attribute:
187     //   source
188     // Type:
189     //   object
190     // Description:
191     //   A Sorce object (Section 3.2.2) that provides data about the
192     //   inventory source and which entity makes the final decision.
193     Source *Source `json:"source,omitempty"`
          .
          .
211 }
~/go/pkg/mod/github.com/mxm!cherry/openrtb@v11.0.0+incompatible/bid_request.go [RO]

Please create a new Source object copy locally and assign to the bidRequest. Notice that, depending on its initial configuration, prebid server core may or may not initialize bidRequest.Source field if it comes with a nil value (see endpoints/openrtb2/auction.go's line 311). In summary, we could do something like:

67     err := modifyImps(request, requestInfo)
68     if err != nil {
69         return nil, []error{err}
70     }
71
72 -   request.Source.Ext = setSourceExt()
   +   if request.Source != nil {
   +       modifiableSource = *request.Source
   +   } else {
   +       modifiableSource = &openrtb.Source{}
   +   }
   +   modifiableSource.Ext = setSourceExt()
   +   request.Source = modifiableSource
73
74     request.Ext, err = setExt(request)
75     if err != nil {
76         return nil, []error{err}
77     }
adapters/unicorn/unicorn.go

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed it. Thank you so much!

c27da8b

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed again because there is have copying source problem.

fb58a7e


request.Ext, err = setExt(request)
if err != nil {
return nil, []error{err}
}

requestJSON, err := json.Marshal(request)
if err != nil {
return nil, []error{err}
}

requestData := &adapters.RequestData{
Method: "POST",
Uri: a.endpoint,
Body: requestJSON,
Headers: getHeaders(request),
}

return []*adapters.RequestData{requestData}, nil
}

func getHeaders(request *openrtb.BidRequest) http.Header {
headers := http.Header{}
headers.Add("Content-Type", "application/json;charset=utf-8")
headers.Add("Accept", "application/json")
headers.Add("X-Openrtb-Version", "2.5")

if request.Device != nil {
if len(request.Device.UA) > 0 {
headers.Add("User-Agent", request.Device.UA)
}

if len(request.Device.IPv6) > 0 {
headers.Add("X-Forwarded-For", request.Device.IPv6)
}

if len(request.Device.IP) > 0 {
headers.Add("X-Forwarded-For", request.Device.IP)
}
}
SyntaxNode marked this conversation as resolved.
Show resolved Hide resolved

return headers
}

func modifyImps(request *openrtb.BidRequest, requestInfo *adapters.ExtraRequestInfo) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

The second parameter requestInfo *adapters.ExtraRequestInfo seems to not be used by this function at all. Do we need it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Certainly, requestInfo *adapters.ExtraRequestInfo is not used. Fixed it. Thank you!

c27da8b

for i := 0; i < len(request.Imp); i++ {
imp := &request.Imp[i]

var ext unicornImpExt
err := json.Unmarshal(imp.Ext, &ext)

if err != nil {
return &errortypes.BadInput{
Message: fmt.Sprintf("Error while decoding imp[%d].ext: %s", i, err),
}
}

var placementID string
if ext.Bidder.PlacementID != "" {
placementID = ext.Bidder.PlacementID
} else {
placementID, err = getStoredRequestImpID(imp)
if err != nil {
return &errortypes.BadInput{
Message: fmt.Sprintf("Error get StoredRequestImpID from imp[%d]: %s", i, err),
}
}
}

ext.Bidder.PlacementID = placementID
Copy link
Contributor

Choose a reason for hiding this comment

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

The fact that there's a scenario where ext.Bidder.PlacementID does not get modified at all makes for an opportunity to maybe refactor the if-else statement in line 138 to maybe something like:

128           var ext unicornImpExt
129           err := json.Unmarshal(imp.Ext, &ext)
130
131           if err != nil {
132               return &errortypes.BadInput{
133                   Message: fmt.Sprintf("Error while decoding imp[%d].ext: %s", i, err),
134               }
135           }
136
137 -         var placementID string
138 -         if ext.Bidder.PlacementID != "" {
139 -             placementID = ext.Bidder.PlacementID
140 -         } else {
    +         if ext.Bidder.PlacementID == "" {
141 -             placementID, err = getStoredRequestImpID(imp)
    +             ext.Bidder.PlacementID, err = getStoredRequestImpID(imp)
142               if err != nil {
143                   return &errortypes.BadInput{
144                       Message: fmt.Sprintf("Error get StoredRequestImpID from imp[%d]: %s", i, err),
145                   }
146               }
147           }
148
149 -         ext.Bidder.PlacementID = placementID
150
151           imp.Ext, err = json.Marshal(ext)
152           if err != nil {
153               return &errortypes.BadInput{
154                   Message: fmt.Sprintf("Error while encoding imp[%d].ext: %s", i, err),
155               }
156           }
157
158           secure := int8(1)
159           imp.Secure = &secure
160 -         imp.TagID = placementID
    +         imp.TagID = ext.Bidder.PlacementID
adapters/unicorn/unicorn.go

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Certainly, fixed it. Thanks you.

deaf6c0


imp.Ext, err = json.Marshal(ext)
if err != nil {
return &errortypes.BadInput{
Message: fmt.Sprintf("Error while encoding imp[%d].ext: %s", i, err),
}
}

secure := int8(1)
imp.Secure = &secure
imp.TagID = placementID
Copy link
Contributor

Choose a reason for hiding this comment

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

We seem to not be doing anything with imp which is a modified copy of the ith element of []Imp. Did we mean to add it to, say a modifiedImps array?

124 - func modifyImps(request *openrtb.BidRequest) error {
    + func modifyImps(request *openrtb.BidRequest) ([]openrtb.Imp, error) {
125       for i := 0; i < len(request.Imp); i++ {
126           imp := &request.Imp[i]
127  
128   *-- 29 lines: var ext unicornImpExt-----------------
157  
158           secure := int8(1)
159           imp.Secure = &secure
160           imp.TagID = placementID
    +         modifiedImps = append(modifiedImps, imp)
161       }
162 -     return nil
    +     return modifiedImps, nil
163   }
adapters/unicorn/unicorn.go

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, modifyImps is rewriting imp to include the required data.
For example, it is implemented to put placementId or storedRequestImpId for Imp[i].TagID.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ohh you are right. Misread line 126 as a local copy of the imp (imp := *request.Imp[i]) instead of imp := &request.Imp[i]

}
return nil
}

func getStoredRequestImpID(imp *openrtb.Imp) (string, error) {
SyntaxNode marked this conversation as resolved.
Show resolved Hide resolved
v, err := jsonparser.GetString(imp.Ext, "prebid", "storedrequest", "id")

if err != nil {
return "", fmt.Errorf("stored request id not found: %s", err)
}

return v, nil
}

func setSourceExt() json.RawMessage {
return json.RawMessage(`{"stype": "prebid_server_uncn", "bidder": "unicorn"}`)
}

func setExt(request *openrtb.BidRequest) (json.RawMessage, error) {
accountID, err := jsonparser.GetInt(request.Imp[0].Ext, "bidder", "accountId")
if err != nil {
accountID = 0
}
var decodedExt *unicornExt
err = json.Unmarshal(request.Ext, &decodedExt)
if err != nil {
decodedExt = &unicornExt{
Prebid: nil,
}
}
decodedExt.AccountID = accountID

ext, err := json.Marshal(decodedExt)
if err != nil {
return nil, &errortypes.BadInput{
Message: fmt.Sprintf("Error while encoding ext, err: %s", err),
}
}
return ext, nil
}

// MakeBids unpacks the server's response into Bids.
func (a *adapter) MakeBids(request *openrtb.BidRequest, requestData *adapters.RequestData, responseData *adapters.ResponseData) (*adapters.BidderResponse, []error) {

if responseData.StatusCode == http.StatusNoContent {
return nil, nil
}

if responseData.StatusCode == http.StatusBadRequest {
SyntaxNode marked this conversation as resolved.
Show resolved Hide resolved
err := &errortypes.BadInput{
Message: "Unexpected http status code: 400",
}
return nil, []error{err}
}

if responseData.StatusCode != http.StatusOK {
err := &errortypes.BadServerResponse{
Message: fmt.Sprintf("Unexpected http status code: %d", responseData.StatusCode),
}
return nil, []error{err}
}

var response openrtb.BidResponse
if err := json.Unmarshal(responseData.Body, &response); err != nil {
return nil, []error{err}
}

bidResponse := adapters.NewBidderResponseWithBidsCapacity(len(request.Imp))
bidResponse.Currency = response.Cur
for _, seatBid := range response.SeatBid {
for _, bid := range seatBid.Bid {
bid := bid
var bidType openrtb_ext.BidType
SyntaxNode marked this conversation as resolved.
Show resolved Hide resolved
for _, imp := range request.Imp {
if imp.ID == bid.ImpID {
if imp.Banner != nil {
SyntaxNode marked this conversation as resolved.
Show resolved Hide resolved
bidType = openrtb_ext.BidTypeBanner
} else if imp.Native != nil {
bidType = openrtb_ext.BidTypeNative
Copy link
Contributor

Choose a reason for hiding this comment

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

The true branch of this if statement is not covered in the tests cases. Could we add a native JSON test that covers it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Native will support it in the future, so I left it, but since it is not currently supported, I will remove this branch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed it

deaf6c0

}
}
}
b := &adapters.TypedBid{
Bid: &bid,
BidType: bidType,
}
bidResponse.Bids = append(bidResponse.Bids, b)
}
}
return bidResponse, nil
}
20 changes: 20 additions & 0 deletions adapters/unicorn/unicorn_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package unicorn

import (
"testing"

"github.com/prebid/prebid-server/adapters/adapterstest"
"github.com/prebid/prebid-server/config"
"github.com/prebid/prebid-server/openrtb_ext"
)

func TestJsonSamples(t *testing.T) {
bidder, buildErr := Builder(openrtb_ext.BidderUnicorn, config.Adapter{
Endpoint: "https://ds.uncn.jp"})

if buildErr != nil {
t.Fatalf("Builder returned unexpected error %v", buildErr)
}

adapterstest.RunJSONBidderTest(t, "unicorntest", bidder)
}
Loading