-
Notifications
You must be signed in to change notification settings - Fork 737
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: ResetDigital #3766
base: master
Are you sure you want to change the base?
Changes from 34 commits
70de190
dd48470
32551fc
125124b
5c56e12
fe4ff39
664614f
0b44384
0775c72
20fa856
33f4603
e468217
b8ca566
2de7781
eb067f7
86206d7
3fc5455
3762676
8500d56
817afeb
fe55575
9f2caf9
3295c41
2a8483b
07f3ee2
057e25d
09a3dd4
8784615
80a90dd
a907ecb
670e1d4
989c565
99431ad
39905ed
4482bc9
d426e8a
ac2fc01
52a6794
a1b4451
5fefeaa
9626103
0a3271d
da658e7
0f5a1fe
f087eea
115f773
d1596c8
536821f
dea6cf7
382f793
83bce5a
55094fe
466ff83
ed3e4a1
843a81c
7c92e10
8b13ebc
c50e264
c3d8379
efc1e8b
e8e2848
804334a
dabc386
cba6221
211f13a
ce331a7
4f177ca
2a19924
4d64623
0e9b234
2606e75
5cfe0ae
e4bd6d3
6be7244
e8509e6
d161712
a556e2d
59a5b07
54f8759
bd85ba4
84a8162
8d7117d
ece8152
baa553c
4ea0e33
2e2b49f
e825553
f7caea5
8237f7f
ec6a45d
ecc90bb
6a011ed
b920cca
e0a21d0
6cbedf0
7613ff5
c02ee8c
ffdd75f
3e24be7
640b97c
3c4527e
905b3a5
93368cc
11b6546
4462fce
8b1b96e
c42fe53
53f51a6
6c154e0
0d54a8d
f27bcef
8689e0d
d54c3ed
64584f6
87d4412
9bb9b3d
c37951a
5fcbbbf
cbe9876
b56923c
8134328
451bc44
bcf6491
18f6798
03a4abd
660dba7
a23380f
ddf897c
1419761
db2a872
df58baf
3907f1a
5b11f59
5f70f11
6e150f3
613317a
a083c03
a788661
bc7caaf
4569e97
0bc2aeb
d212d91
3403544
cff2442
44bee69
96fde76
6e08b5d
ec4005d
fd3ec0d
8bca6ad
278be3f
56a77a0
f0e2574
0e365ce
f2d3afc
d6e24d0
eceef87
e86d017
baa974f
73c3fe0
246010f
cff5817
5ab5517
d3df8f2
c1795b8
1665bc9
0f6f5b1
27863db
f0da5d7
6c745fb
56b03f1
21eecdd
dcd9dad
2584c6b
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,302 @@ | ||
package resetdigital | ||
|
||
import ( | ||
"encoding/json" | ||
"fmt" | ||
"net/http" | ||
"strconv" | ||
"text/template" | ||
|
||
"github.com/prebid/openrtb/v20/openrtb2" | ||
"github.com/prebid/prebid-server/v2/adapters" | ||
"github.com/prebid/prebid-server/v2/config" | ||
"github.com/prebid/prebid-server/v2/openrtb_ext" | ||
) | ||
|
||
type adapter struct { | ||
endpoint *template.Template | ||
endpointUri string | ||
} | ||
|
||
type resetDigitalRequest struct { | ||
Site resetDigitalSite `json:"site"` | ||
Imps []resetDigitalImp `json:"imps"` | ||
} | ||
type resetDigitalSite struct { | ||
Domain string `json:"domain"` | ||
Referrer string `json:"referrer"` | ||
} | ||
type resetDigitalImp struct { | ||
ZoneID resetDigitalImpZone `json:"zone_id"` | ||
BidID string `json:"bid_id"` | ||
ImpID string `json:"imp_id"` | ||
Ext resetDigitalImpExt `json:"ext"` | ||
MediaTypes resetDigitalMediaTypes `json:"media_types"` | ||
} | ||
type resetDigitalImpZone struct { | ||
PlacementID string `json:"placementId"` | ||
} | ||
type resetDigitalImpExt struct { | ||
Gpid string `json:"gpid"` | ||
} | ||
type resetDigitalMediaTypes struct { | ||
Banner resetDigitalMediaType `json:"banner"` | ||
Video resetDigitalMediaType `json:"video"` | ||
} | ||
type resetDigitalMediaType struct { | ||
Sizes [][]int64 `json:"sizes"` | ||
} | ||
|
||
type resetDigitalBidResponse struct { | ||
Bids []resetDigitalBid `json:"bids"` | ||
} | ||
type resetDigitalBid struct { | ||
BidID string `json:"bid_id"` | ||
ImpID string `json:"imp_id"` | ||
CPM float64 `json:"cpm"` | ||
CID string `json:"cid,omitempty"` | ||
CrID string `json:"crid,omitempty"` | ||
AdID string `json:"adid"` | ||
W string `json:"w,omitempty"` | ||
H string `json:"h,omitempty"` | ||
Seat string `json:"seat"` | ||
HTML string `json:"html"` | ||
} | ||
|
||
func Builder(bidderName openrtb_ext.BidderName, config config.Adapter, server config.Server) (adapters.Bidder, error) { | ||
template, err := template.New("endpointTemplate").Parse(config.Endpoint) | ||
if err != nil { | ||
return nil, fmt.Errorf("unable to parse endpoint url template: %v", err) | ||
} | ||
bidder := &adapter{ | ||
endpoint: template, | ||
} | ||
return bidder, nil | ||
} | ||
|
||
func getHeaders(request *openrtb2.BidRequest) http.Header { | ||
headers := http.Header{} | ||
|
||
if request != nil && request.Device != nil && request.Site != nil { // what about request.App? Do we need to do something different with Referrer in the app case assuming we care about app? | ||
addNonEmptyHeaders(&headers, map[string]string{ | ||
"Referer": request.Site.Page, | ||
"Accept-Language": request.Device.Language, | ||
"User-Agent": request.Device.UA, | ||
"X-Forwarded-For": request.Device.IP, | ||
"X-Real-Ip": request.Device.IP, | ||
"Content-Type": "application/json;charset=utf-8", | ||
"Accept": "application/json", | ||
}) | ||
} | ||
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 only adds the headers if both site and device are not 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. Discussed offline. For now two conditionals will suffice to protect against accessing fields on nil pointers.
|
||
return headers | ||
} | ||
|
||
func addNonEmptyHeaders(headers *http.Header, headerValues map[string]string) { | ||
for key, value := range headerValues { | ||
if len(value) > 0 { | ||
headers.Add(key, value) | ||
} | ||
} | ||
} | ||
|
||
func (a *adapter) MakeRequests(requestData *openrtb2.BidRequest, requestInfo *adapters.ExtraRequestInfo) ([]*adapters.RequestData, []error) { | ||
var ( | ||
requests []*adapters.RequestData | ||
errors []error | ||
) | ||
|
||
for _, imp := range requestData.Imp { | ||
bidType, err := getBidType(imp) | ||
if err != nil { | ||
errors = append(errors, err) | ||
continue | ||
Comment on lines
+120
to
+121
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. code path is not covered. should add json tests to improve coverage |
||
} | ||
|
||
splittedRequestData, err := processDataFromRequest(requestData, imp, bidType) | ||
if err != nil { | ||
errors = append(errors, err) | ||
continue | ||
Comment on lines
+126
to
+127
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. code path is not covered. should add json tests to improve coverage |
||
} | ||
|
||
requestBody, err := json.Marshal(splittedRequestData) | ||
if err != nil { | ||
errors = append(errors, err) | ||
continue | ||
} | ||
|
||
requests = append(requests, &adapters.RequestData{ | ||
Method: "POST", | ||
Uri: a.endpointUri, | ||
Body: requestBody, | ||
Headers: getHeaders(requestData), | ||
ImpIDs: []string{imp.ID}, | ||
}) | ||
} | ||
|
||
return requests, errors | ||
} | ||
|
||
func processDataFromRequest(requestData *openrtb2.BidRequest, imp openrtb2.Imp, bidType openrtb_ext.BidType) (resetDigitalRequest, error) { | ||
var reqData resetDigitalRequest | ||
|
||
if requestData.Site != 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. Do you need to handle App data? 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. Discussed offline. We do not need to handle app data. |
||
reqData.Site.Domain = requestData.Site.Domain | ||
reqData.Site.Referrer = requestData.Site.Page | ||
} | ||
|
||
reqData.Imps = append(reqData.Imps, resetDigitalImp{ | ||
BidID: requestData.ID, | ||
ImpID: imp.ID, | ||
}) | ||
|
||
if bidType == openrtb_ext.BidTypeBanner && imp.Banner != nil { | ||
var tempH, tempW int64 | ||
if imp.Banner.H != nil { | ||
tempH = *imp.Banner.H | ||
} | ||
if imp.Banner.W != nil { | ||
tempW = *imp.Banner.W | ||
} | ||
if tempH > 0 && tempW > 0 { | ||
reqData.Imps[0].MediaTypes.Banner.Sizes = append( | ||
reqData.Imps[0].MediaTypes.Banner.Sizes, | ||
[]int64{tempW, tempH}, | ||
) | ||
} | ||
} | ||
if bidType == openrtb_ext.BidTypeVideo && imp.Video != nil { | ||
var tempH, tempW int64 | ||
if imp.Video.H != nil { | ||
tempH = *imp.Video.H | ||
} | ||
if imp.Video.W != nil { | ||
tempW = *imp.Video.W | ||
} | ||
if tempH > 0 && tempW > 0 { | ||
reqData.Imps[0].MediaTypes.Video.Sizes = append( | ||
reqData.Imps[0].MediaTypes.Video.Sizes, | ||
[]int64{tempW, tempH}, | ||
) | ||
} | ||
} | ||
|
||
var bidderExt adapters.ExtImpBidder | ||
var resetDigitalExt openrtb_ext.ImpExtResetDigital | ||
|
||
if err := json.Unmarshal(imp.Ext, &bidderExt); err != nil { | ||
return resetDigitalRequest{}, err | ||
} | ||
if err := json.Unmarshal(bidderExt.Bidder, &resetDigitalExt); err != nil { | ||
return resetDigitalRequest{}, err | ||
} | ||
reqData.Imps[0].ZoneID.PlacementID = resetDigitalExt.PlacementID | ||
|
||
return reqData, 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 resetDigitalBidResponse | ||
if err := json.Unmarshal(responseData.Body, &response); err != nil { | ||
return nil, []error{err} | ||
} | ||
|
||
bidResponse := adapters.NewBidderResponseWithBidsCapacity(len(request.Imp)) | ||
|
||
var errs []error | ||
requestImps := make(map[string]openrtb2.Imp) | ||
for _, imp := range request.Imp { | ||
requestImps[imp.ID] = imp | ||
} | ||
|
||
for i := range response.Bids { | ||
resetDigitalBid := &response.Bids[i] | ||
|
||
bid, err := getBidFromResponse(resetDigitalBid) | ||
// handle the error | ||
if bid == nil { | ||
// it would be better to return an error here | ||
errs = append(errs, err) | ||
continue | ||
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. code path is not covered. should add json tests to improve coverage |
||
} | ||
|
||
bidType, err := GetMediaTypeForImp(requestImps, bid.ImpID) | ||
if err != nil { | ||
errs = append(errs, err) | ||
continue | ||
} | ||
|
||
b := &adapters.TypedBid{ | ||
Bid: bid, | ||
BidType: bidType, | ||
Seat: openrtb_ext.BidderName(resetDigitalBid.Seat), | ||
} | ||
bidResponse.Bids = append(bidResponse.Bids, b) | ||
} | ||
|
||
if len(request.Cur) == 0 { | ||
bidResponse.Currency = "USD" | ||
} | ||
|
||
return bidResponse, errs | ||
} | ||
|
||
func getBidFromResponse(bidResponse *resetDigitalBid) (*openrtb2.Bid, error) { | ||
if bidResponse.CPM == 0 { | ||
// brian to check how to report this | ||
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. I will look into this and get back to you. 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. You don't need this check. Core will discard a zero dollar bid as long as it isn't associated with a deal. |
||
return nil, nil | ||
} | ||
|
||
bid := &openrtb2.Bid{ | ||
ID: bidResponse.BidID, | ||
Price: bidResponse.CPM, | ||
ImpID: bidResponse.ImpID, | ||
CID: bidResponse.CID, | ||
CrID: bidResponse.CrID, | ||
AdM: bidResponse.HTML, | ||
} | ||
|
||
w, err := strconv.ParseInt(bidResponse.W, 10, 64) | ||
if err != nil { | ||
return nil, err | ||
} | ||
bid.W = w | ||
|
||
h, err := strconv.ParseInt(bidResponse.H, 10, 64) | ||
if err != nil { | ||
return nil, err | ||
} | ||
bid.H = h | ||
return bid, nil | ||
} | ||
|
||
func getBidType(imp openrtb2.Imp) (openrtb_ext.BidType, error) { | ||
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. 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. Implemented as suggested 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.
could you point out or link where 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. Sorry, it was addressed on the point that we support only single format bids, so we could assume the anti pattern. Anyway, it would be more advisable to change to the normal pattern? 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.
Prebid team recommends using MType field. But if it's not doable then current change suffices single format bid. @bruno-siira should mention in Bidder docs that adapter expects only single format bids in the incoming request 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. When we're talking about the Bidder Docs what is this file exacly @onkarvhanumante 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 openrtb_ext.BidTypeBanner, nil | ||
} else if imp.Video != nil { | ||
bruno-siira marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return openrtb_ext.BidTypeVideo, nil | ||
} else if imp.Audio != nil { | ||
bruno-siira marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return openrtb_ext.BidTypeAudio, nil | ||
} | ||
|
||
return "", fmt.Errorf("failed to find matching imp for bid %s", imp.ID) | ||
Comment on lines
+302
to
+307
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. code path is not covered. should add json tests to improve coverage |
||
} | ||
|
||
func GetMediaTypeForImp(reqImps map[string]openrtb2.Imp, bidImpID string) (openrtb_ext.BidType, error) { | ||
mediaType := openrtb_ext.BidTypeBanner | ||
|
||
if reqImp, ok := reqImps[bidImpID]; ok { | ||
if reqImp.Banner == nil && reqImp.Video != nil { | ||
mediaType = openrtb_ext.BidTypeVideo | ||
} | ||
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. You're also declaring support for audio in your yaml file. Shouldn't you also handle media type audio here? 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. Discussed offline. Audio will be added. |
||
return mediaType, nil | ||
} | ||
return "", fmt.Errorf("unknown media type for bid imp ID %s", bidImpID) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
package resetdigital | ||
|
||
import ( | ||
"testing" | ||
|
||
"github.com/prebid/prebid-server/v2/adapters/adapterstest" | ||
"github.com/prebid/prebid-server/v2/config" | ||
"github.com/prebid/prebid-server/v2/openrtb_ext" | ||
) | ||
|
||
func TestJsonSamples(t *testing.T) { | ||
|
||
bidder, buildErr := Builder(openrtb_ext.BidderResetDigital, config.Adapter{ | ||
Endpoint: "https://test.com"}, config.Server{ExternalUrl: "http://hosturl.com", GvlID: 1, DataCenter: "2"}) | ||
|
||
if buildErr != nil { | ||
t.Fatalf("Builder returned unexpected error %v", buildErr) | ||
} | ||
|
||
adapterstest.RunJSONBidderTest(t, "resetdigitaltest", bidder) | ||
} |
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.
As specified in the comment, how are you handling app requests? Do you support app? If so, please update your yaml file declaring what formats you support for app. If you don't support it, let us know and delete this comment.
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.
Discussed offline. Not supporting app at this time.