-
Notifications
You must be signed in to change notification settings - Fork 749
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
Vrtcal: Added Video(VAST) Support #1965
Changes from all commits
48da8c9
fc72d74
dda79e4
40a71fe
25a44da
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 |
---|---|---|
|
@@ -68,15 +68,21 @@ func (a *VrtcalAdapter) MakeBids(internalRequest *openrtb2.BidRequest, externalR | |
|
||
bidResponse := adapters.NewBidderResponseWithBidsCapacity(1) | ||
|
||
var errs []error | ||
for _, sb := range bidResp.SeatBid { | ||
for i := range sb.Bid { | ||
bidResponse.Bids = append(bidResponse.Bids, &adapters.TypedBid{ | ||
Bid: &sb.Bid[i], | ||
BidType: "banner", | ||
}) | ||
bidType, err := getReturnTypeForImp(sb.Bid[i].ImpID, internalRequest.Imp) | ||
if err == nil { | ||
bidResponse.Bids = append(bidResponse.Bids, &adapters.TypedBid{ | ||
Bid: &sb.Bid[i], | ||
BidType: bidType, | ||
}) | ||
} else { | ||
errs = append(errs, err) | ||
} | ||
} | ||
} | ||
return bidResponse, nil | ||
return bidResponse, errs | ||
|
||
} | ||
|
||
|
@@ -87,3 +93,26 @@ func Builder(bidderName openrtb_ext.BidderName, config config.Adapter) (adapters | |
} | ||
return bidder, nil | ||
} | ||
|
||
func getReturnTypeForImp(impID string, imps []openrtb2.Imp) (openrtb_ext.BidType, error) { | ||
for _, imp := range imps { | ||
if imp.ID == impID { | ||
if imp.Banner != nil { | ||
vrtcal-dev marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return openrtb_ext.BidTypeBanner, nil | ||
} | ||
|
||
if imp.Video != nil { | ||
return openrtb_ext.BidTypeVideo, nil | ||
} | ||
|
||
return "", &errortypes.BadServerResponse{ | ||
Message: fmt.Sprintf("Unsupported return type for ID: \"%s\"", impID), | ||
} | ||
} | ||
} | ||
|
||
//Failsafe default in case impression ID is not found | ||
return "", &errortypes.BadServerResponse{ | ||
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. Can you please add test coverage for this case 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.
I've added supplemental test coverage for the failsafe unmatched impression ID. Thanks. |
||
Message: fmt.Sprintf("Failed to find impression for ID: \"%s\"", impID), | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,96 @@ | ||
{ | ||
"mockBidRequest": { | ||
"id": "test-request-id", | ||
"imp": [ | ||
{ | ||
"id": "test-imp-id", | ||
"video": { | ||
"mimes": ["video/mp4"], | ||
"protocols": [2, 5], | ||
"w": 300, | ||
"h": 250 | ||
}, | ||
"ext": { | ||
"bidder": { | ||
"Just_an_unused_vrtcal_param": "unused_data_for_this" | ||
} | ||
} | ||
} | ||
], | ||
"app": { | ||
"id": "fake-app-id" | ||
} | ||
}, | ||
"httpCalls": [ | ||
{ | ||
"expectedRequest": { | ||
"uri": "http://rtb.vrtcal.com/bidder_prebid.vap?ssp=1804", | ||
"body": { | ||
"id": "test-request-id", | ||
"imp": [ | ||
{ | ||
"id": "test-imp-id", | ||
"video": { | ||
"mimes": ["video/mp4"], | ||
"protocols": [2, 5], | ||
"w": 300, | ||
"h": 250 | ||
}, | ||
|
||
"ext": { | ||
"bidder": { | ||
"Just_an_unused_vrtcal_param": "unused_data_for_this" | ||
} | ||
} | ||
} | ||
], | ||
"app": { | ||
"id": "fake-app-id" | ||
} | ||
} | ||
}, | ||
"mockResponse": { | ||
"status": 200, | ||
"body": { | ||
"id": "test-request-id", | ||
"seatbid": [ | ||
{ | ||
"seat": "vrtcal", | ||
"bid": [ | ||
{ | ||
"id": "8ee514f1-b2b8-4abb-89fd-084437d1e800", | ||
"impid": "test-imp-id", | ||
"price": 0.500000, | ||
"adm": "some-test-video-ad", | ||
"crid": "crid_10", | ||
"h": 250, | ||
"w": 300 | ||
} | ||
] | ||
} | ||
], | ||
"cur": "USD" | ||
} | ||
} | ||
} | ||
], | ||
"expectedBidResponses": [ | ||
{ | ||
"currency": "USD", | ||
"bids": [ | ||
{ | ||
"bid": { | ||
"id": "8ee514f1-b2b8-4abb-89fd-084437d1e800", | ||
"impid": "test-imp-id", | ||
"price": 0.5, | ||
"adm": "some-test-video-ad", | ||
"crid": "crid_10", | ||
"w": 300, | ||
"h": 250 | ||
}, | ||
"type": "video" | ||
} | ||
] | ||
} | ||
] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,87 @@ | ||
{ | ||
"mockBidRequest": { | ||
"id": "test-request-id", | ||
"imp": [ | ||
{ | ||
"id": "test-imp-id", | ||
"banner": { | ||
"format": [ | ||
{ | ||
"w": 300, | ||
"h": 250 | ||
} | ||
] | ||
}, | ||
"ext": { | ||
"bidder": { | ||
"Just_an_unused_vrtcal_param": "unused_data_for_this" | ||
} | ||
} | ||
} | ||
], | ||
"app": { | ||
"id": "fake-app-id" | ||
} | ||
}, | ||
"httpCalls": [ | ||
{ | ||
"expectedRequest": { | ||
"uri": "http://rtb.vrtcal.com/bidder_prebid.vap?ssp=1804", | ||
"body": { | ||
"id": "test-request-id", | ||
"imp": [ | ||
{ | ||
"id": "test-imp-id", | ||
"banner": { | ||
"format": [ | ||
{ | ||
"w": 300, | ||
"h": 250 | ||
} | ||
] | ||
}, | ||
|
||
"ext": { | ||
"bidder": { | ||
"Just_an_unused_vrtcal_param": "unused_data_for_this" | ||
} | ||
} | ||
} | ||
], | ||
"app": { | ||
"id": "fake-app-id" | ||
} | ||
} | ||
}, | ||
"mockResponse": { | ||
"status": 200, | ||
"body": { | ||
"id": "test-request-id", | ||
"seatbid": [ | ||
{ | ||
"seat": "vrtcal", | ||
"bid": [ | ||
{ | ||
"id": "8ee514f1-b2b8-4abb-89fd-084437d1e800", | ||
"impid": "WRONG-IMPRESSION_ID", | ||
"price": 0.500000, | ||
"adm": "some-test-ad", | ||
"crid": "crid_10", | ||
"h": 250, | ||
"w": 300 | ||
} | ||
] | ||
} | ||
], | ||
"cur": "USD" | ||
} | ||
} | ||
} | ||
], | ||
"expectedMakeBidsErrors": [ | ||
{ | ||
"value": "Failed to find impression for ID: \"WRONG-IMPRESSION_ID\"", | ||
"comparison": "literal" | ||
} | ||
] | ||
} |
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.
@vrtcal-dev This check would just ignore the error. I'd recommend changing this to the following so that the errors are properly communicated to the user.
You'll just. have to instantiate and
errs
slice either at the start of this function or before the for loopThere 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.
Thanks mansinahar, I've udpated to properly report errored bids while continuing to proces further bids.