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

Adding type to native markup for Prebid mobile #1081

Merged
merged 4 commits into from
Oct 29, 2019
Merged
Show file tree
Hide file tree
Changes from all 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
82 changes: 82 additions & 0 deletions exchange/bidder.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,14 @@ import (
"bytes"
"context"
"encoding/json"
"errors"
"fmt"
"io/ioutil"
"net/http"

"github.com/mxmCherry/openrtb"
nativeRequests "github.com/mxmCherry/openrtb/native/request"
nativeResponse "github.com/mxmCherry/openrtb/native/response"
"github.com/prebid/prebid-server/adapters"
"github.com/prebid/prebid-server/currencies"
"github.com/prebid/prebid-server/errortypes"
Expand Down Expand Up @@ -153,6 +156,25 @@ func (bidder *bidderAdapter) requestBid(ctx context.Context, request *openrtb.Bi
}
}

// Only do this for request from mobile app
if request.App != nil {
for i := 0; i < len(bidResponse.Bids); i++ {
SyntaxNode marked this conversation as resolved.
Show resolved Hide resolved
if bidResponse.Bids[i].BidType == openrtb_ext.BidTypeNative {
nativeMarkup, moreErrs := addNativeTypes(bidResponse.Bids[i].Bid, request)
errs = append(errs, moreErrs...)

if nativeMarkup != nil {
markup, err := json.Marshal(*nativeMarkup)
if err != nil {
errs = append(errs, err)
} else {
bidResponse.Bids[i].Bid.AdM = string(markup)
}
}
}
}
}

if err == nil {
// Conversion rate found, using it for conversion
for i := 0; i < len(bidResponse.Bids); i++ {
Expand All @@ -178,6 +200,66 @@ func (bidder *bidderAdapter) requestBid(ctx context.Context, request *openrtb.Bi
return seatBid, errs
}

func addNativeTypes(bid *openrtb.Bid, request *openrtb.BidRequest) (*nativeResponse.Response, []error) {
var errs []error
var nativeMarkup *nativeResponse.Response
if err := json.Unmarshal(json.RawMessage(bid.AdM), &nativeMarkup); err != nil || len(nativeMarkup.Assets) == 0 {
// Some bidders are returning non-IAB complaiant native markup. In this case Prebid server will not be able to add types. E.g Facebook
return nil, errs
}

nativeImp, err := getNativeImpByImpID(bid.ImpID, request)
if err != nil {
errs = append(errs, err)
return nil, errs
}

var nativePayload nativeRequests.Request
if err := json.Unmarshal(json.RawMessage((*nativeImp).Request), &nativePayload); err != nil {
Copy link
Collaborator

Choose a reason for hiding this comment

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

We have a potential panic here. If getNativeImpByImpID() fails to find the imp, then nativeImp is nil. Trying to access nil.Request will then panic, as you cannot dereference a nil. You should probably abort above if nativeImp is nil. I don't think we need to abort here on Unmarshal errors as the worst that should happen is we fail asset lookups and don't do anything.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

errs = append(errs, err)
}

for _, asset := range nativeMarkup.Assets {
setAssetTypes(asset, nativePayload)
}

return nativeMarkup, errs
}

func setAssetTypes(asset nativeResponse.Asset, nativePayload nativeRequests.Request) {
if asset.Img != nil {
tempAsset := getAssetByID(asset.ID, nativePayload.Assets)
if tempAsset.Img.Type != 0 {
asset.Img.Type = tempAsset.Img.Type
}
}

if asset.Data != nil {
tempAsset := getAssetByID(asset.ID, nativePayload.Assets)
if tempAsset.Data.Type != 0 {
asset.Data.Type = tempAsset.Data.Type
}
}
}

func getNativeImpByImpID(impID string, request *openrtb.BidRequest) (*openrtb.Native, error) {
for _, impInRequest := range request.Imp {
if impInRequest.ID == impID && impInRequest.Native != nil {
return impInRequest.Native, nil
}
}
return nil, errors.New("Could not find native imp")
}

func getAssetByID(id int64, assets []nativeRequests.Asset) nativeRequests.Asset {
for _, asset := range assets {
if id == asset.ID {
return asset
}
}
return nativeRequests.Asset{}
}

// makeExt transforms information about the HTTP call into the contract class for the PBS response.
func makeExt(httpInfo *httpCallInfo) *openrtb_ext.ExtHttpCall {
if httpInfo.err == nil {
Expand Down
101 changes: 101 additions & 0 deletions exchange/bidder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -962,6 +962,107 @@ func TestServerCallDebugging(t *testing.T) {
}
}

func TestMobileNativeTypes(t *testing.T) {
respBody := "{\"bid\":false}"
respStatus := 200
server := httptest.NewServer(mockHandler(respStatus, "getBody", respBody))
defer server.Close()

reqBody := "{\"key\":\"val\"}"
reqURL := server.URL

testCases := []struct {
mockBidderRequest *openrtb.BidRequest
mockBidderResponse *adapters.BidderResponse
expectedValue string
description string
}{
{
mockBidderRequest: &openrtb.BidRequest{
Imp: []openrtb.Imp{
{
ID: "some-imp-id",
Native: &openrtb.Native{
Request: "{\"ver\":\"1.1\",\"context\":1,\"contextsubtype\":11,\"plcmttype\":4,\"plcmtcnt\":1,\"assets\":[{\"id\":1,\"required\":1,\"title\":{\"len\":500}},{\"id\":2,\"required\":1,\"img\":{\"type\":3,\"wmin\":1,\"hmin\":1}},{\"id\":3,\"required\":0,\"data\":{\"type\":1,\"len\":200}},{\"id\":4,\"required\":0,\"data\":{\"type\":2,\"len\":15000}},{\"id\":5,\"required\":0,\"data\":{\"type\":6,\"len\":40}}]}",
},
},
},
App: &openrtb.App{},
},
mockBidderResponse: &adapters.BidderResponse{
Bids: []*adapters.TypedBid{
{
Bid: &openrtb.Bid{
ImpID: "some-imp-id",
AdM: "{\"assets\":[{\"id\":2,\"img\":{\"url\":\"http://vcdn.adnxs.com/p/creative-image/f8/7f/0f/13/f87f0f13-230c-4f05-8087-db9216e393de.jpg\",\"w\":989,\"h\":742,\"ext\":{\"appnexus\":{\"prevent_crop\":0}}}},{\"id\":1,\"title\":{\"text\":\"This is a Prebid Native Creative\"}},{\"id\":3,\"data\":{\"value\":\"Prebid.org\"}},{\"id\":4,\"data\":{\"value\":\"This is a Prebid Native Creative. There are many like it, but this one is mine.\"}}],\"link\":{\"url\":\"http://some-url.com\"},\"imptrackers\":[\"http://someimptracker.com\"],\"jstracker\":\"some-js-tracker\"}",
Price: 10,
},
BidType: openrtb_ext.BidTypeNative,
},
},
},
expectedValue: "{\"assets\":[{\"id\":2,\"img\":{\"type\":3,\"url\":\"http://vcdn.adnxs.com/p/creative-image/f8/7f/0f/13/f87f0f13-230c-4f05-8087-db9216e393de.jpg\",\"w\":989,\"h\":742,\"ext\":{\"appnexus\":{\"prevent_crop\":0}}}},{\"id\":1,\"title\":{\"text\":\"This is a Prebid Native Creative\"}},{\"id\":3,\"data\":{\"type\":1,\"value\":\"Prebid.org\"}},{\"id\":4,\"data\":{\"type\":2,\"value\":\"This is a Prebid Native Creative. There are many like it, but this one is mine.\"}}],\"link\":{\"url\":\"http://some-url.com\"},\"imptrackers\":[\"http://someimptracker.com\"],\"jstracker\":\"some-js-tracker\"}",
description: "Checks types in response",
},
{
mockBidderRequest: &openrtb.BidRequest{
Imp: []openrtb.Imp{
{
ID: "some-imp-id",
Native: &openrtb.Native{
Request: "{\"ver\":\"1.1\",\"context\":1,\"contextsubtype\":11,\"plcmttype\":4,\"plcmtcnt\":1,\"assets\":[{\"id\":1,\"required\":1,\"title\":{\"len\":500}},{\"id\":2,\"required\":1,\"img\":{\"type\":3,\"wmin\":1,\"hmin\":1}},{\"id\":3,\"required\":0,\"data\":{\"type\":1,\"len\":200}},{\"id\":4,\"required\":0,\"data\":{\"type\":2,\"len\":15000}},{\"id\":5,\"required\":0,\"data\":{\"type\":6,\"len\":40}}]}",
},
},
},
App: &openrtb.App{},
},
mockBidderResponse: &adapters.BidderResponse{
Bids: []*adapters.TypedBid{
{
Bid: &openrtb.Bid{
ImpID: "some-imp-id",
AdM: "{\"some-diff-markup\":\"creative\"}",
Price: 10,
},
BidType: openrtb_ext.BidTypeNative,
},
},
},
expectedValue: "{\"some-diff-markup\":\"creative\"}",
description: "Non IAB compliant markup",
},
}

for _, tc := range testCases {
bidderImpl := &goodSingleBidder{
httpRequest: &adapters.RequestData{
Method: "POST",
Uri: reqURL,
Body: []byte(reqBody),
Headers: http.Header{},
},
bidResponse: tc.mockBidderResponse,
}
bidder := adaptBidder(bidderImpl, server.Client())
currencyConverter := currencies.NewRateConverterDefault()

seatBids, _ := bidder.requestBid(
context.Background(),
tc.mockBidderRequest,
"test",
1.0,
currencyConverter.Rates(),
&adapters.ExtraRequestInfo{},
)

var actualValue string
for _, bid := range seatBids.bids {
actualValue = bid.bid.AdM
diffJson(t, tc.description, []byte(actualValue), []byte(tc.expectedValue))
}
}
}

func TestErrorReporting(t *testing.T) {
bidder := adaptBidder(&bidRejector{}, nil)
currencyConverter := currencies.NewRateConverterDefault()
Expand Down