-
Notifications
You must be signed in to change notification settings - Fork 748
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 all commits
70de190
dd48470
32551fc
125124b
5c56e12
fe4ff39
664614f
0b44384
0775c72
20fa856
33f4603
e468217
b8ca566
2de7781
eb067f7
86206d7
3fc5455
3762676
989c565
39905ed
4482bc9
d426e8a
ac2fc01
52a6794
9626103
dea6cf7
5cfe0ae
ece8152
baa553c
ecc90bb
0d54a8d
660dba7
5f70f11
613317a
a083c03
bc7caaf
4569e97
0bc2aeb
d212d91
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
123cb13
4950da1
cc06d05
330d84f
17cf6c8
5b44280
ee3546c
2ecc9a8
0a4d9bc
b93dca3
ef65f7b
6bac6cd
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,2 @@ | ||
# Ignore test coverage output | ||
cover.out |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,307 @@ | ||
package resetdigital | ||
|
||
import ( | ||
"encoding/json" | ||
"fmt" | ||
"net/http" | ||
"strconv" | ||
"text/template" | ||
|
||
"github.com/prebid/openrtb/v20/openrtb2" | ||
"github.com/prebid/prebid-server/v3/adapters" | ||
"github.com/prebid/prebid-server/v3/config" | ||
"github.com/prebid/prebid-server/v3/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,omitempty"` | ||
Video resetDigitalMediaType `json:"video,omitempty"` | ||
Audio resetDigitalMediaType `json:"audio,omitempty"` | ||
} | ||
type resetDigitalMediaType struct { | ||
Sizes [][]int64 `json:"sizes,omitempty"` | ||
Mimes []string `json:"mimes,omitempty"` | ||
} | ||
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{} | ||
|
||
addNonEmptyHeaders(&headers, map[string]string{ | ||
"Content-Type": "application/json;charset=utf-8", | ||
"Accept": "application/json", | ||
}) | ||
|
||
if request != nil && request.Device != nil { | ||
addNonEmptyHeaders(&headers, map[string]string{ | ||
"Accept-Language": request.Device.Language, | ||
"User-Agent": request.Device.UA, | ||
"X-Forwarded-For": request.Device.IP, | ||
"X-Real-Ip": request.Device.IP, | ||
}) | ||
} | ||
if request != nil && request.Site != nil { | ||
addNonEmptyHeaders(&headers, map[string]string{ | ||
"Referer": request.Site.Page, | ||
}) | ||
} | ||
|
||
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 | ||
} | ||
|
||
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) { | ||
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. Here's a suggestion to make this function a bit easier to read:
|
||
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 | ||
} | ||
|
||
rdImp := 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 { | ||
rdImp.MediaTypes.Banner.Sizes = append(rdImp.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 { | ||
rdImp.MediaTypes.Video.Sizes = append(rdImp.MediaTypes.Video.Sizes, []int64{tempW, tempH}) | ||
} | ||
if imp.Video.MIMEs != nil { | ||
rdImp.MediaTypes.Video.Mimes = append(rdImp.MediaTypes.Video.Mimes, imp.Video.MIMEs...) | ||
} | ||
} | ||
if bidType == openrtb_ext.BidTypeAudio && imp.Audio != nil && imp.Audio.MIMEs != nil { | ||
rdImp.MediaTypes.Audio.Mimes = append(rdImp.MediaTypes.Audio.Mimes, imp.Audio.MIMEs...) | ||
} | ||
|
||
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 | ||
} | ||
rdImp.ZoneID.PlacementID = resetDigitalExt.PlacementID | ||
|
||
reqData.Imps = append(reqData.Imps, rdImp) | ||
|
||
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) | ||
if bid == nil { | ||
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) { | ||
|
||
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 | ||
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 test coverage for this line by adding a supplemental JSON test |
||
} | ||
bid.W = w | ||
|
||
h, err := strconv.ParseInt(bidResponse.H, 10, 64) | ||
if err != nil { | ||
return nil, err | ||
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 test coverage for this line by adding a supplemental JSON test |
||
} | ||
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
+290
to
+295
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(reqImp openrtb2.Imp) (openrtb_ext.BidType, error) { | ||
|
||
if reqImp.Video != 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.BidTypeVideo, 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.BidTypeVideo, nil | ||
} | ||
if reqImp.Audio != 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.BidTypeAudio, 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.BidTypeAudio, nil | ||
} | ||
return openrtb_ext.BidTypeBanner, nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
package resetdigital | ||
|
||
import ( | ||
"testing" | ||
|
||
"github.com/prebid/prebid-server/v3/adapters/adapterstest" | ||
"github.com/prebid/prebid-server/v3/config" | ||
"github.com/prebid/prebid-server/v3/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.
code path is not covered. should add json tests to improve coverage