Skip to content
This repository has been archived by the owner on Dec 22, 2022. It is now read-only.

Commit

Permalink
Currency support prebid#280 - step 1 (prebid#634)
Browse files Browse the repository at this point in the history
This CL allows prebid server to reject any bids having a currency mismatch between allowed
currencies in the bid request and the currency declared in bids.

For the time being, if allowed currencies are not declared in bid request or in bid
the currency is implicitly set to `USD` to prevent from any breaking changes.

This CL includes:
- Prebid server to reject bids having currency mistmatch

Tnis CL doesn't include:
- Changing the default currency (which is USD)
  • Loading branch information
benjaminch authored and dbemiller committed Aug 10, 2018
1 parent e51fae1 commit 153f587
Show file tree
Hide file tree
Showing 4 changed files with 375 additions and 19 deletions.
47 changes: 39 additions & 8 deletions exchange/bidder.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ type pbsOrtbBid struct {
type pbsOrtbSeatBid struct {
// bids is the list of bids which this adaptedBidder wishes to make.
bids []*pbsOrtbBid
// currency is the currency in which the bids are made.
// Should be a valid curreny ISO code.
currency string
// httpCalls is the list of debugging info. It should only be populated if the request.test == 1.
// This will become response.ext.debug.httpcalls.{bidder} on the final Response.
httpCalls []*openrtb_ext.ExtHttpCall
Expand Down Expand Up @@ -107,9 +110,12 @@ func (bidder *bidderAdapter) requestBid(ctx context.Context, request *openrtb.Bi

seatBid := &pbsOrtbSeatBid{
bids: make([]*pbsOrtbBid, 0, len(reqData)),
currency: "USD",
httpCalls: make([]*openrtb_ext.ExtHttpCall, 0, len(reqData)),
}

firstHTTPCallCurrency := ""

// If the bidder made multiple requests, we still want them to enter as many bids as possible...
// even if the timeout occurs sometime halfway through.
for i := 0; i < len(reqData); i++ {
Expand All @@ -120,18 +126,43 @@ func (bidder *bidderAdapter) requestBid(ctx context.Context, request *openrtb.Bi
}

if httpInfo.err == nil {

bidResponse, moreErrs := bidder.Bidder.MakeBids(request, httpInfo.request, httpInfo.response)
errs = append(errs, moreErrs...)

if bidResponse != nil {
for i := 0; i < len(bidResponse.Bids); i++ {
if bidResponse.Bids[i].Bid != nil {
// TODO #280: Convert the bid price
bidResponse.Bids[i].Bid.Price = bidResponse.Bids[i].Bid.Price * bidAdjustment

if bidResponse.Currency == "" {
bidResponse.Currency = "USD"
}

// Related to #281 - currency support
// Prebid can't make sure that each HTTP call returns bids with the same currency as the others.
// If a Bidder makes two HTTP calls, and their servers respond with different currencies,
// we will consider the first call currency as standard currency and then reject others which contradict it.
if firstHTTPCallCurrency == "" { // First HTTP call
firstHTTPCallCurrency = bidResponse.Currency
}

// TODO: #281 - Once currencies rate conversion is out, this shouldn't be an issue anymore, we will only
// need to convert the bid price based on the currency.
if firstHTTPCallCurrency == bidResponse.Currency {
for i := 0; i < len(bidResponse.Bids); i++ {
if bidResponse.Bids[i].Bid != nil {
// TODO #280: Convert the bid price
bidResponse.Bids[i].Bid.Price = bidResponse.Bids[i].Bid.Price * bidAdjustment
}
seatBid.bids = append(seatBid.bids, &pbsOrtbBid{
bid: bidResponse.Bids[i].Bid,
bidType: bidResponse.Bids[i].BidType,
})
}
seatBid.bids = append(seatBid.bids, &pbsOrtbBid{
bid: bidResponse.Bids[i].Bid,
bidType: bidResponse.Bids[i].BidType,
})
} else {
errs = append(errs, fmt.Errorf(
"Bid currencies mistmatch found. Expected all bids to have the same currencies. Expected '%s', was: '%s'",
firstHTTPCallCurrency,
bidResponse.Currency,
))
}
}
} else {
Expand Down
154 changes: 154 additions & 0 deletions exchange/bidder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,134 @@ func TestConnectionClose(t *testing.T) {
}
}

// TestMultiCurrencies makes sure that bidderAdapter.requestBid returns errors if the bidder pass several currencies in case of multi HTTP calls.
func TestMultiCurrencies(t *testing.T) {
// Setup:
respStatus := 200
getRespBody := "{\"wasPost\":false}"
postRespBody := "{\"wasPost\":true}"

testCases := []struct {
bidRequestCurrencies []string
bidCurrency []string
expectedBidsCount uint
expectedBadCurrencyErrorsCount uint
}{
// Bidder respond with the same currency (default one) on all HTTP responses
{
bidCurrency: []string{"USD", "USD", "USD"},
expectedBidsCount: 3,
expectedBadCurrencyErrorsCount: 0,
},
// Bidder respond with the same currency (not default one) on all HTTP responses
{
bidCurrency: []string{"EUR", "EUR", "EUR"},
expectedBidsCount: 3,
expectedBadCurrencyErrorsCount: 0,
},
// Bidder responds with currency not set on all HTTP responses
{
bidCurrency: []string{"", "", ""},
expectedBidsCount: 3,
expectedBadCurrencyErrorsCount: 0,
},
// Bidder responds with a mix of not set and default currency in HTTP responses
{
bidCurrency: []string{"", "USD", ""},
expectedBidsCount: 3,
expectedBadCurrencyErrorsCount: 0,
},
// Bidder responds with a mix of not set and default currency in HTTP responses
{
bidCurrency: []string{"USD", "USD", ""},
expectedBidsCount: 3,
expectedBadCurrencyErrorsCount: 0,
},
// Bidder responds with a mix of not set and default currency in HTTP responses
{
bidCurrency: []string{"", "", "USD"},
expectedBidsCount: 3,
expectedBadCurrencyErrorsCount: 0,
},
// Bidder responds with a mix of not set, non default currency and default currency in HTTP responses
{
bidCurrency: []string{"EUR", "", "USD"},
expectedBidsCount: 1,
expectedBadCurrencyErrorsCount: 2,
},
// Bidder responds with a mix of not set, non default currency and default currency in HTTP responses
{
bidCurrency: []string{"GDB", "", "USD"},
expectedBidsCount: 1,
expectedBadCurrencyErrorsCount: 2,
},
// Bidder responds with a mix of not set and default currency in HTTP responses
{
bidCurrency: []string{"GDB", "", ""},
expectedBidsCount: 1,
expectedBadCurrencyErrorsCount: 2,
},
// Bidder responds with a mix of not set and default currency in HTTP responses
{
bidCurrency: []string{"GDB", "GDB", "GDB"},
expectedBidsCount: 3,
expectedBadCurrencyErrorsCount: 0,
},
}

server := httptest.NewServer(mockHandler(respStatus, getRespBody, postRespBody))
defer server.Close()

for _, tc := range testCases {
mockBidderResponses := make([]*adapters.BidderResponse, len(tc.bidCurrency))
bidderImpl := &goodMultiHTTPCallsBidder{
bidResponses: mockBidderResponses,
}
bidderImpl.httpRequest = make([]*adapters.RequestData, len(tc.bidCurrency))

for i, cur := range tc.bidCurrency {

mockBidderResponses[i] = &adapters.BidderResponse{
Bids: []*adapters.TypedBid{
{
Bid: &openrtb.Bid{},
BidType: openrtb_ext.BidTypeBanner,
},
},
Currency: cur,
}

bidderImpl.httpRequest[i] = &adapters.RequestData{
Method: "POST",
Uri: server.URL,
Body: []byte("{\"key\":\"val\"}"),
Headers: http.Header{},
}
}

// Execute:
bidder := adaptBidder(bidderImpl, server.Client())
seatBid, errs := bidder.requestBid(
context.Background(),
&openrtb.BidRequest{},
"test",
1,
)

// Verify:
if seatBid == nil && tc.expectedBidsCount != 0 {
t.Errorf("Seatbid is nil but expected not to be nil")
}
if tc.expectedBidsCount != uint(len(seatBid.bids)) {
t.Errorf("Expected to have %d bids count but got %d", tc.expectedBidsCount, len(seatBid.bids))
}
if tc.expectedBadCurrencyErrorsCount != uint(len(errs)) {
t.Errorf("Expected to have %d errors count but got %d", tc.expectedBadCurrencyErrorsCount, len(errs))
}

}
}

// TestBadResponseLogging makes sure that openrtb_ext works properly on malformed HTTP requests.
func TestBadRequestLogging(t *testing.T) {
info := &httpCallInfo{
Expand Down Expand Up @@ -372,6 +500,32 @@ func (bidder *goodSingleBidder) MakeBids(internalRequest *openrtb.BidRequest, ex
return bidder.bidResponse, nil
}

type goodMultiHTTPCallsBidder struct {
bidRequest *openrtb.BidRequest
httpRequest []*adapters.RequestData
httpResponses []*adapters.ResponseData
bidResponses []*adapters.BidderResponse
bidResponseNumber int
}

func (bidder *goodMultiHTTPCallsBidder) MakeRequests(request *openrtb.BidRequest) ([]*adapters.RequestData, []error) {
bidder.bidRequest = request
response := make([]*adapters.RequestData, len(bidder.httpRequest))

for i, r := range bidder.httpRequest {
response[i] = r
}
return response, nil
}

func (bidder *goodMultiHTTPCallsBidder) MakeBids(internalRequest *openrtb.BidRequest, externalRequest *adapters.RequestData, response *adapters.ResponseData) (*adapters.BidderResponse, []error) {
br := bidder.bidResponses[bidder.bidResponseNumber]
bidder.bidResponseNumber++
bidder.httpResponses = append(bidder.httpResponses, response)

return br, nil
}

type mixedMultiBidder struct {
bidRequest *openrtb.BidRequest
httpRequests []*adapters.RequestData
Expand Down
55 changes: 50 additions & 5 deletions exchange/exchange.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@ import (
"fmt"
"net/http"
"runtime/debug"
"strings"
"time"

"github.com/golang/glog"
"golang.org/x/text/currency"

"github.com/mxmCherry/openrtb"

Expand Down Expand Up @@ -176,7 +178,7 @@ func (e *exchange) getAllBids(ctx context.Context, cleanRequests map[openrtb_ext
elapsed := time.Since(start)
brw.adapterBids = bids
// validate bids ASAP, so we don't waste time on invalid bids.
err2 := brw.validateBids()
err2 := brw.validateBids(request)
if len(err2) > 0 {
err = append(err, err2...)
}
Expand Down Expand Up @@ -384,15 +386,21 @@ func (e *exchange) makeBid(Bids []*pbsOrtbBid, adapter openrtb_ext.BidderName) (
}

// validateBids will run some validation checks on the returned bids and excise any invalid bids
func (brw *bidResponseWrapper) validateBids() (err []error) {
func (brw *bidResponseWrapper) validateBids(request *openrtb.BidRequest) (err []error) {
// Exit early if there is nothing to do.
if brw.adapterBids == nil || len(brw.adapterBids.bids) == 0 {
return
}
// TODO #280: Exit if there is a currency mismatch between currencies passed in bid request
// and the currency received in the bid.
// Check also if the currency received exists.

err = make([]error, 0, len(brw.adapterBids.bids))

// By design, default currency is USD.
if cerr := validateCurrency(request.Cur, brw.adapterBids.currency); cerr != nil {
brw.adapterBids.bids = nil
err = append(err, cerr)
return
}

validBids := make([]*pbsOrtbBid, 0, len(brw.adapterBids.bids))
for _, bid := range brw.adapterBids.bids {
if ok, berr := validateBid(bid); ok {
Expand All @@ -408,6 +416,42 @@ func (brw *bidResponseWrapper) validateBids() (err []error) {
return err
}

// validateCurrency will run currency validation checks and return true if it passes, false otherwise.
func validateCurrency(requestAllowedCurrencies []string, bidCurrency string) error {
// Default currency is `USD` by design.
defaultCurrency := "USD"
// Make sure bid currency is a valid ISO currency code
if bidCurrency == "" {
// If bid currency is not set, then consider it's default currency.
bidCurrency = defaultCurrency
}
currencyUnit, cerr := currency.ParseISO(bidCurrency)
if cerr != nil {
return cerr
}
// Make sure the bid currency is allowed from bid request via `cur` field.
// If `cur` field array from bid request is empty, then consider it accepts the default currency.
currencyAllowed := false
if len(requestAllowedCurrencies) == 0 {
requestAllowedCurrencies = []string{defaultCurrency}
}
for _, allowedCurrency := range requestAllowedCurrencies {
if strings.ToUpper(allowedCurrency) == currencyUnit.String() {
currencyAllowed = true
break
}
}
if currencyAllowed == false {
return fmt.Errorf(
"Bid currency is not allowed. Was '%s', wants: ['%s']",
currencyUnit.String(),
strings.Join(requestAllowedCurrencies, "', '"),
)
}

return nil
}

// validateBid will run the supplied bid through validation checks and return true if it passes, false otherwise.
func validateBid(bid *pbsOrtbBid) (bool, error) {
if bid.bid == nil {
Expand All @@ -426,5 +470,6 @@ func validateBid(bid *pbsOrtbBid) (bool, error) {
if bid.bid.CrID == "" {
return false, fmt.Errorf("Bid \"%s\" missing creative ID", bid.bid.ID)
}

return true, nil
}
Loading

0 comments on commit 153f587

Please sign in to comment.