From 0158cb3cf024c4e9f67c2f6f1f5a8150ef0394e0 Mon Sep 17 00:00:00 2001 From: Serhii Nahornyi Date: Wed, 14 Jul 2021 20:32:05 +0300 Subject: [PATCH 1/3] Rubicon: Use currency conversion function --- adapters/bidder.go | 6 +- adapters/rubicon/rubicon.go | 21 +-- adapters/rubicon/rubicon_test.go | 120 +++++++++++++----- .../rubicontest/exemplary/simple-video.json | 3 - .../supplemental/no-site-content-data.json | 3 - .../supplemental/no-site-content.json | 3 - 6 files changed, 104 insertions(+), 52 deletions(-) diff --git a/adapters/bidder.go b/adapters/bidder.go index b7bde4bc55d..dbe56f8eb91 100644 --- a/adapters/bidder.go +++ b/adapters/bidder.go @@ -139,12 +139,12 @@ func (r *RequestData) SetBasicAuth(username string, password string) { type ExtraRequestInfo struct { PbsEntryPoint metrics.RequestType GlobalPrivacyControlHeader string - currencyConversions currency.Conversions + CurrencyConversions currency.Conversions } func NewExtraRequestInfo(c currency.Conversions) ExtraRequestInfo { return ExtraRequestInfo{ - currencyConversions: c, + CurrencyConversions: c, } } @@ -153,7 +153,7 @@ func NewExtraRequestInfo(c currency.Conversions) ExtraRequestInfo { // - ConversionNotFoundError if the conversion mapping is unknown to Prebid Server // and not provided in the bid request. func (r ExtraRequestInfo) ConvertCurrency(value float64, from, to string) (float64, error) { - if rate, err := r.currencyConversions.GetRate(from, to); err == nil { + if rate, err := r.CurrencyConversions.GetRate(from, to); err == nil { return value * rate, nil } else { return 0, err diff --git a/adapters/rubicon/rubicon.go b/adapters/rubicon/rubicon.go index 84200431992..219d8c54680 100644 --- a/adapters/rubicon/rubicon.go +++ b/adapters/rubicon/rubicon.go @@ -676,7 +676,6 @@ func (a *RubiconAdapter) MakeRequests(request *openrtb2.BidRequest, reqInfo *ada numRequests := len(request.Imp) errs := make([]error, 0, len(request.Imp)) var err error - requestData := make([]*adapters.RequestData, 0, numRequests) headers := http.Header{} headers.Add("Content-Type", "application/json;charset=utf-8") @@ -750,8 +749,15 @@ func (a *RubiconAdapter) MakeRequests(request *openrtb2.BidRequest, reqInfo *ada continue } - resolvedBidFloor, resolvedBidFloorCur := resolveBidFloorAttributes(thisImp.BidFloor, thisImp.BidFloorCur) - thisImp.BidFloorCur = resolvedBidFloorCur + resolvedBidFloor, err := resolveBidFloor(thisImp.BidFloor, thisImp.BidFloorCur, reqInfo) + if err != nil { + errs = append(errs, &errortypes.BadInput{ + Message: fmt.Sprintf("Unable to convert provided bid floor currency from %s to USD", + thisImp.BidFloorCur), + }) + continue + } + thisImp.BidFloorCur = "USD" thisImp.BidFloor = resolvedBidFloor if request.User != nil { @@ -908,15 +914,14 @@ func (a *RubiconAdapter) MakeRequests(request *openrtb2.BidRequest, reqInfo *ada return requestData, errs } -// Will be replaced after https://github.com/prebid/prebid-server/issues/1482 resolution -func resolveBidFloorAttributes(bidFloor float64, bidFloorCur string) (float64, string) { +func resolveBidFloor(bidFloor float64, bidFloorCur string, reqInfo *adapters.ExtraRequestInfo) (float64, error) { if bidFloor > 0 { - if strings.ToUpper(bidFloorCur) == "EUR" { - return bidFloor * 1.2, "USD" + if strings.ToUpper(bidFloorCur) != "USD" { + return reqInfo.ConvertCurrency(bidFloor, bidFloorCur, "USD") } } - return bidFloor, bidFloorCur + return bidFloor, nil } func updateExtWithIabAttribute(target json.RawMessage, data []openrtb2.Data, segTaxes []int) (json.RawMessage, error) { diff --git a/adapters/rubicon/rubicon_test.go b/adapters/rubicon/rubicon_test.go index 76904a42137..aa0191ab8b1 100644 --- a/adapters/rubicon/rubicon_test.go +++ b/adapters/rubicon/rubicon_test.go @@ -4,6 +4,8 @@ import ( "bytes" "context" "encoding/json" + "errors" + "github.com/stretchr/testify/mock" "io/ioutil" "net/http" "net/http/httptest" @@ -572,52 +574,106 @@ func TestResolveVideoSizeId(t *testing.T) { } } -func TestResolveBidFloorAttributes(t *testing.T) { +func TestOpenRTBRequestWithDifferentBidFloorAttributes(t *testing.T) { testScenarios := []struct { - bidFloor float64 - bidFloorCur string - expectedBidFloor float64 - expectedBidFloorCur string + bidFloor float64 + bidFloorCur string + setMock func(m *mock.Mock) + expectedBidFloor float64 + expectedErrors []error }{ { - bidFloor: 1, - bidFloorCur: "EUR", - expectedBidFloor: 1.2, - expectedBidFloorCur: "USD", - }, - { - bidFloor: 1, - bidFloorCur: "Eur", - expectedBidFloor: 1.2, - expectedBidFloorCur: "USD", - }, - { - bidFloor: 0, - bidFloorCur: "EUR", - expectedBidFloor: 0, - expectedBidFloorCur: "EUR", + bidFloor: 1, + bidFloorCur: "WRONG", + setMock: func(m *mock.Mock) { m.On("GetRate", "WRONG", "USD").Return(2.5, errors.New("some error")) }, + expectedBidFloor: 0, + expectedErrors: []error{ + &errortypes.BadInput{Message: "Unable to convert provided bid floor currency from WRONG to USD"}, + }, }, { - bidFloor: -1, - bidFloorCur: "EUR", - expectedBidFloor: -1, - expectedBidFloorCur: "EUR", + bidFloor: 1, + bidFloorCur: "USD", + setMock: func(m *mock.Mock) {}, + expectedBidFloor: 1, + expectedErrors: nil, }, { - bidFloor: 1, - bidFloorCur: "USD", - expectedBidFloor: 1, - expectedBidFloorCur: "USD", + bidFloor: 1, + bidFloorCur: "EUR", + setMock: func(m *mock.Mock) { m.On("GetRate", "EUR", "USD").Return(1.2, nil) }, + expectedBidFloor: 1.2, + expectedErrors: nil, }, } for _, scenario := range testScenarios { - bidFloor, bidFloorCur := resolveBidFloorAttributes(scenario.bidFloor, scenario.bidFloorCur) - assert.Equal(t, scenario.expectedBidFloor, bidFloor) - assert.Equal(t, scenario.expectedBidFloorCur, bidFloorCur) + mockConversions := &mockCurrencyConversion{} + scenario.setMock(&mockConversions.Mock) + + extraRequestInfo := adapters.ExtraRequestInfo{ + CurrencyConversions: mockConversions, + } + + SIZE_ID := getTestSizes() + bidder := new(RubiconAdapter) + + request := &openrtb2.BidRequest{ + ID: "test-request-id", + Imp: []openrtb2.Imp{{ + ID: "test-imp-id", + BidFloorCur: scenario.bidFloorCur, + BidFloor: scenario.bidFloor, + Banner: &openrtb2.Banner{ + Format: []openrtb2.Format{ + SIZE_ID[15], + SIZE_ID[10], + }, + }, + Ext: json.RawMessage(`{"bidder": { + "zoneId": 8394, + "siteId": 283282, + "accountId": 7891 + }}`), + }}, + App: &openrtb2.App{ + ID: "com.test", + Name: "testApp", + }, + } + + reqs, errs := bidder.MakeRequests(request, &extraRequestInfo) + + mockConversions.AssertExpectations(t) + rubiconReq := &openrtb2.BidRequest{} + if scenario.expectedErrors == nil { + if err := json.Unmarshal(reqs[0].Body, rubiconReq); err != nil { + t.Fatalf("Unexpected error while decoding request: %s", err) + } + assert.NotNil(t, rubiconReq.Imp[0].BidFloorCur, "User.Ext object should not be nil.") + assert.NotNil(t, rubiconReq.Imp[0].BidFloor, "User.Ext object should not be nil.") + assert.Equal(t, scenario.expectedBidFloor, rubiconReq.Imp[0].BidFloor) + assert.Equal(t, "USD", rubiconReq.Imp[0].BidFloorCur) + } else { + assert.Equal(t, scenario.expectedErrors, errs) + } } } +type mockCurrencyConversion struct { + mock.Mock +} + +func (m mockCurrencyConversion) GetRate(from string, to string) (float64, error) { + args := m.Called(from, to) + return args.Get(0).(float64), args.Error(1) +} + +func (m mockCurrencyConversion) GetRates() *map[string]map[string]float64 { + args := m.Called() + return args.Get(0).(*map[string]map[string]float64) +} + func TestNoContentResponse(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusNoContent) diff --git a/adapters/rubicon/rubicontest/exemplary/simple-video.json b/adapters/rubicon/rubicontest/exemplary/simple-video.json index 11afdd50d2b..e7e215f0c24 100644 --- a/adapters/rubicon/rubicontest/exemplary/simple-video.json +++ b/adapters/rubicon/rubicontest/exemplary/simple-video.json @@ -120,8 +120,6 @@ "w": 1024, "h": 576 }, - "bidfloor": 1, - "bidfloorcur": "EuR", "ext": { "bidder": { "video": { @@ -298,7 +296,6 @@ "w": 1024, "h": 576 }, - "bidfloor": 1.2, "bidfloorcur": "USD", "ext": { "rp": { diff --git a/adapters/rubicon/rubicontest/supplemental/no-site-content-data.json b/adapters/rubicon/rubicontest/supplemental/no-site-content-data.json index f67788a3154..75a35c40c14 100644 --- a/adapters/rubicon/rubicontest/supplemental/no-site-content-data.json +++ b/adapters/rubicon/rubicontest/supplemental/no-site-content-data.json @@ -85,8 +85,6 @@ "w": 1024, "h": 576 }, - "bidfloor": 1, - "bidfloorcur": "EuR", "ext": { "bidder": { "video": { @@ -221,7 +219,6 @@ "w": 1024, "h": 576 }, - "bidfloor": 1.2, "bidfloorcur": "USD", "ext": { "rp": { diff --git a/adapters/rubicon/rubicontest/supplemental/no-site-content.json b/adapters/rubicon/rubicontest/supplemental/no-site-content.json index d3b8f8b7454..1cedfbd42de 100644 --- a/adapters/rubicon/rubicontest/supplemental/no-site-content.json +++ b/adapters/rubicon/rubicontest/supplemental/no-site-content.json @@ -83,8 +83,6 @@ "w": 1024, "h": 576 }, - "bidfloor": 1, - "bidfloorcur": "EuR", "ext": { "bidder": { "video": { @@ -217,7 +215,6 @@ "w": 1024, "h": 576 }, - "bidfloor": 1.2, "bidfloorcur": "USD", "ext": { "rp": { From 22bf6a65c4a0565a6afd813cabdd588916bb9ddd Mon Sep 17 00:00:00 2001 From: Serhii Nahornyi Date: Wed, 14 Jul 2021 20:35:39 +0300 Subject: [PATCH 2/3] Style fix --- adapters/rubicon/rubicon_test.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/adapters/rubicon/rubicon_test.go b/adapters/rubicon/rubicon_test.go index aa0191ab8b1..550d755f5d5 100644 --- a/adapters/rubicon/rubicon_test.go +++ b/adapters/rubicon/rubicon_test.go @@ -645,13 +645,12 @@ func TestOpenRTBRequestWithDifferentBidFloorAttributes(t *testing.T) { reqs, errs := bidder.MakeRequests(request, &extraRequestInfo) mockConversions.AssertExpectations(t) - rubiconReq := &openrtb2.BidRequest{} + if scenario.expectedErrors == nil { + rubiconReq := &openrtb2.BidRequest{} if err := json.Unmarshal(reqs[0].Body, rubiconReq); err != nil { t.Fatalf("Unexpected error while decoding request: %s", err) } - assert.NotNil(t, rubiconReq.Imp[0].BidFloorCur, "User.Ext object should not be nil.") - assert.NotNil(t, rubiconReq.Imp[0].BidFloor, "User.Ext object should not be nil.") assert.Equal(t, scenario.expectedBidFloor, rubiconReq.Imp[0].BidFloor) assert.Equal(t, "USD", rubiconReq.Imp[0].BidFloorCur) } else { From 8cafb7c3cf1fb7380f2b6e32660a42f359317715 Mon Sep 17 00:00:00 2001 From: Serhii Nahornyi Date: Fri, 16 Jul 2021 11:13:31 +0300 Subject: [PATCH 3/3] Update to work with only positive bidFloor --- adapters/rubicon/rubicon.go | 13 ++++++----- adapters/rubicon/rubicon_test.go | 22 ++++++++++++++++++- .../rubicontest/exemplary/simple-video.json | 1 - .../supplemental/no-site-content-data.json | 1 - .../supplemental/no-site-content.json | 1 - 5 files changed, 28 insertions(+), 10 deletions(-) diff --git a/adapters/rubicon/rubicon.go b/adapters/rubicon/rubicon.go index 219d8c54680..e9627916cc6 100644 --- a/adapters/rubicon/rubicon.go +++ b/adapters/rubicon/rubicon.go @@ -757,8 +757,11 @@ func (a *RubiconAdapter) MakeRequests(request *openrtb2.BidRequest, reqInfo *ada }) continue } - thisImp.BidFloorCur = "USD" - thisImp.BidFloor = resolvedBidFloor + + if resolvedBidFloor > 0 { + thisImp.BidFloorCur = "USD" + thisImp.BidFloor = resolvedBidFloor + } if request.User != nil { userCopy := *request.User @@ -915,10 +918,8 @@ func (a *RubiconAdapter) MakeRequests(request *openrtb2.BidRequest, reqInfo *ada } func resolveBidFloor(bidFloor float64, bidFloorCur string, reqInfo *adapters.ExtraRequestInfo) (float64, error) { - if bidFloor > 0 { - if strings.ToUpper(bidFloorCur) != "USD" { - return reqInfo.ConvertCurrency(bidFloor, bidFloorCur, "USD") - } + if bidFloor > 0 && bidFloorCur != "" && strings.ToUpper(bidFloorCur) != "USD" { + return reqInfo.ConvertCurrency(bidFloor, bidFloorCur, "USD") } return bidFloor, nil diff --git a/adapters/rubicon/rubicon_test.go b/adapters/rubicon/rubicon_test.go index 550d755f5d5..28ddebbf5e3 100644 --- a/adapters/rubicon/rubicon_test.go +++ b/adapters/rubicon/rubicon_test.go @@ -580,6 +580,7 @@ func TestOpenRTBRequestWithDifferentBidFloorAttributes(t *testing.T) { bidFloorCur string setMock func(m *mock.Mock) expectedBidFloor float64 + expectedBidCur string expectedErrors []error }{ { @@ -587,6 +588,7 @@ func TestOpenRTBRequestWithDifferentBidFloorAttributes(t *testing.T) { bidFloorCur: "WRONG", setMock: func(m *mock.Mock) { m.On("GetRate", "WRONG", "USD").Return(2.5, errors.New("some error")) }, expectedBidFloor: 0, + expectedBidCur: "", expectedErrors: []error{ &errortypes.BadInput{Message: "Unable to convert provided bid floor currency from WRONG to USD"}, }, @@ -596,6 +598,7 @@ func TestOpenRTBRequestWithDifferentBidFloorAttributes(t *testing.T) { bidFloorCur: "USD", setMock: func(m *mock.Mock) {}, expectedBidFloor: 1, + expectedBidCur: "USD", expectedErrors: nil, }, { @@ -603,6 +606,23 @@ func TestOpenRTBRequestWithDifferentBidFloorAttributes(t *testing.T) { bidFloorCur: "EUR", setMock: func(m *mock.Mock) { m.On("GetRate", "EUR", "USD").Return(1.2, nil) }, expectedBidFloor: 1.2, + expectedBidCur: "USD", + expectedErrors: nil, + }, + { + bidFloor: 0, + bidFloorCur: "", + setMock: func(m *mock.Mock) {}, + expectedBidFloor: 0, + expectedBidCur: "", + expectedErrors: nil, + }, + { + bidFloor: -1, + bidFloorCur: "CZK", + setMock: func(m *mock.Mock) {}, + expectedBidFloor: -1, + expectedBidCur: "CZK", expectedErrors: nil, }, } @@ -652,7 +672,7 @@ func TestOpenRTBRequestWithDifferentBidFloorAttributes(t *testing.T) { t.Fatalf("Unexpected error while decoding request: %s", err) } assert.Equal(t, scenario.expectedBidFloor, rubiconReq.Imp[0].BidFloor) - assert.Equal(t, "USD", rubiconReq.Imp[0].BidFloorCur) + assert.Equal(t, scenario.expectedBidCur, rubiconReq.Imp[0].BidFloorCur) } else { assert.Equal(t, scenario.expectedErrors, errs) } diff --git a/adapters/rubicon/rubicontest/exemplary/simple-video.json b/adapters/rubicon/rubicontest/exemplary/simple-video.json index e7e215f0c24..1daffe9b386 100644 --- a/adapters/rubicon/rubicontest/exemplary/simple-video.json +++ b/adapters/rubicon/rubicontest/exemplary/simple-video.json @@ -296,7 +296,6 @@ "w": 1024, "h": 576 }, - "bidfloorcur": "USD", "ext": { "rp": { "track": { diff --git a/adapters/rubicon/rubicontest/supplemental/no-site-content-data.json b/adapters/rubicon/rubicontest/supplemental/no-site-content-data.json index 75a35c40c14..0be214da4bc 100644 --- a/adapters/rubicon/rubicontest/supplemental/no-site-content-data.json +++ b/adapters/rubicon/rubicontest/supplemental/no-site-content-data.json @@ -219,7 +219,6 @@ "w": 1024, "h": 576 }, - "bidfloorcur": "USD", "ext": { "rp": { "track": { diff --git a/adapters/rubicon/rubicontest/supplemental/no-site-content.json b/adapters/rubicon/rubicontest/supplemental/no-site-content.json index 1cedfbd42de..2e830a2dd00 100644 --- a/adapters/rubicon/rubicontest/supplemental/no-site-content.json +++ b/adapters/rubicon/rubicontest/supplemental/no-site-content.json @@ -215,7 +215,6 @@ "w": 1024, "h": 576 }, - "bidfloorcur": "USD", "ext": { "rp": { "track": {