From ffa388930735cbd46eb4416bfe9364d9f976fced Mon Sep 17 00:00:00 2001 From: Gus Carreon Date: Tue, 10 Aug 2021 13:15:48 -0400 Subject: [PATCH] Remove time.Now() as the first parameter of NewRates() --- currency/aggregate_conversions_test.go | 5 +- currency/rate_converter_test.go | 3 - currency/rates.go | 26 +------- currency/rates_test.go | 87 ++++++++------------------ endpoints/openrtb2/auction_test.go | 8 +-- exchange/bidder_test.go | 14 ----- exchange/exchange.go | 4 +- 7 files changed, 34 insertions(+), 113 deletions(-) diff --git a/currency/aggregate_conversions_test.go b/currency/aggregate_conversions_test.go index 773a596c28c..d2ac961a8a7 100644 --- a/currency/aggregate_conversions_test.go +++ b/currency/aggregate_conversions_test.go @@ -3,7 +3,6 @@ package currency import ( "errors" "testing" - "time" "github.com/stretchr/testify/assert" ) @@ -11,14 +10,14 @@ import ( func TestGroupedGetRate(t *testing.T) { // Setup: - customRates := NewRates(time.Now(), map[string]map[string]float64{ + customRates := NewRates(map[string]map[string]float64{ "USD": { "GBP": 3.00, "EUR": 2.00, }, }) - pbsRates := NewRates(time.Now(), map[string]map[string]float64{ + pbsRates := NewRates(map[string]map[string]float64{ "USD": { "GBP": 4.00, "MXN": 10.00, diff --git a/currency/rate_converter_test.go b/currency/rate_converter_test.go index e1d6741dff2..261b954807e 100644 --- a/currency/rate_converter_test.go +++ b/currency/rate_converter_test.go @@ -143,7 +143,6 @@ func TestReadWriteRates(t *testing.T) { } else { rates := currencyConverter.Rates().(*Rates) assert.Equal(t, tt.wantConversions, (*rates).Conversions, tt.description) - assert.Equal(t, tt.wantDataAsOf, (*rates).DataAsOf, tt.description) } lastUpdated := currencyConverter.LastUpdated() @@ -169,7 +168,6 @@ func TestRateStaleness(t *testing.T) { defer mockedHttpServer.Close() expectedRates := &Rates{ - DataAsOf: time.Date(2018, time.September, 12, 0, 0, 0, 0, time.UTC), Conversions: map[string]map[string]float64{ "USD": { "GBP": 0.77208, @@ -257,7 +255,6 @@ func TestRatesAreNeverConsideredStale(t *testing.T) { defer mockedHttpServer.Close() expectedRates := &Rates{ - DataAsOf: time.Date(2018, time.September, 12, 0, 0, 0, 0, time.UTC), Conversions: map[string]map[string]float64{ "USD": { "GBP": 0.77208, diff --git a/currency/rates.go b/currency/rates.go index b9cb0201b38..f1cc19fcccb 100644 --- a/currency/rates.go +++ b/currency/rates.go @@ -1,9 +1,7 @@ package currency import ( - "encoding/json" "errors" - "time" "golang.org/x/text/currency" ) @@ -12,38 +10,16 @@ import ( // note that `DataAsOfRaw` field is needed when parsing remote JSON as the date format if not standard and requires // custom parsing to be properly set as Golang time.Time type Rates struct { - DataAsOf time.Time `json:"dataAsOf"` Conversions map[string]map[string]float64 `json:"conversions"` } // NewRates creates a new Rates object holding currencies rates -func NewRates(dataAsOf time.Time, conversions map[string]map[string]float64) *Rates { +func NewRates(conversions map[string]map[string]float64) *Rates { return &Rates{ - DataAsOf: dataAsOf, Conversions: conversions, } } -// UnmarshalJSON unmarshal raw JSON bytes to Rates object -func (r *Rates) UnmarshalJSON(b []byte) error { - c := &struct { - DataAsOf string `json:"dataAsOf"` - Conversions map[string]map[string]float64 `json:"conversions"` - }{} - if err := json.Unmarshal(b, &c); err != nil { - return err - } - - r.Conversions = c.Conversions - - layout := "2006-01-02" - if date, err := time.Parse(layout, c.DataAsOf); err == nil { - r.DataAsOf = date - } - - return nil -} - // GetRate returns the conversion rate between two currencies or: // - An error if one of the currency strings is not well-formed // - An error if any of the currency strings is not a recognized currency code. diff --git a/currency/rates_test.go b/currency/rates_test.go index bca332e0858..23226dce8fb 100644 --- a/currency/rates_test.go +++ b/currency/rates_test.go @@ -2,48 +2,31 @@ package currency import ( "encoding/json" + "errors" "testing" - "time" "github.com/stretchr/testify/assert" ) func TestUnMarshallRates(t *testing.T) { - // Setup: testCases := []struct { + desc string ratesJSON string expectedRates Rates expectsError bool + expectedError error }{ { - ratesJSON: `{ - "dataAsOf":"2018-09-12", - "conversions":{ - "USD":{ - "GBP":0.7662523901 - }, - "GBP":{ - "USD":1.3050530256 - } - } - }`, - expectedRates: Rates{ - DataAsOf: time.Date(2018, time.September, 12, 0, 0, 0, 0, time.UTC), - Conversions: map[string]map[string]float64{ - "USD": { - "GBP": 0.7662523901, - }, - "GBP": { - "USD": 1.3050530256, - }, - }, - }, - expectsError: false, + desc: "malformed JSON object, return error", + ratesJSON: `malformed`, + expectedRates: Rates{}, + expectsError: true, + expectedError: errors.New("invalid character 'm' looking for beginning of value"), }, { + desc: "Valid JSON field defining valid conversion object. Expect no error", ratesJSON: `{ - "dataAsOf":"", "conversions":{ "USD":{ "GBP":0.7662523901 @@ -54,7 +37,6 @@ func TestUnMarshallRates(t *testing.T) { } }`, expectedRates: Rates{ - DataAsOf: time.Time{}, Conversions: map[string]map[string]float64{ "USD": { "GBP": 0.7662523901, @@ -64,47 +46,25 @@ func TestUnMarshallRates(t *testing.T) { }, }, }, - expectsError: false, + expectsError: false, + expectedError: nil, }, { + desc: "Valid JSON field defines a conversions map with repeated entries, expect error", ratesJSON: `{ - "dataAsOf":"blabla", "conversions":{ "USD":{ - "GBP":0.7662523901 - }, - "GBP":{ - "USD":1.3050530256 - } - } - }`, - expectedRates: Rates{ - DataAsOf: time.Time{}, - Conversions: map[string]map[string]float64{ - "USD": { - "GBP": 0.7662523901, - }, - "GBP": { - "USD": 1.3050530256, + "GBP":0.7662523901, + "MXN":20.00 }, - }, - }, - expectsError: false, - }, - { - ratesJSON: `{ - "dataAsOf":"blabla", - "conversions":{ "USD":{ - "GBP":0.7662523901, + "GBP":0.7662523901 }, - "GBP":{ - "USD":1.3050530256, - } } }`, expectedRates: Rates{}, expectsError: true, + expectedError: errors.New("invalid character '}' looking for beginning of object key string"), }, } @@ -114,15 +74,18 @@ func TestUnMarshallRates(t *testing.T) { err := json.Unmarshal([]byte(tc.ratesJSON), &updatedRates) // Verify: - assert.Equal(t, err != nil, tc.expectsError) - assert.Equal(t, tc.expectedRates, updatedRates, "Rates weren't the expected ones") + assert.Equal(t, err != nil, tc.expectsError, tc.desc) + if tc.expectsError { + assert.Equal(t, err.Error(), tc.expectedError.Error(), tc.desc) + } + assert.Equal(t, tc.expectedRates, updatedRates, tc.desc) } } func TestGetRate(t *testing.T) { // Setup: - rates := NewRates(time.Now(), map[string]map[string]float64{ + rates := NewRates(map[string]map[string]float64{ "USD": { "GBP": 0.77208, }, @@ -165,7 +128,7 @@ func TestGetRate(t *testing.T) { func TestGetRate_ReverseConversion(t *testing.T) { // Setup: - rates := NewRates(time.Now(), map[string]map[string]float64{ + rates := NewRates(map[string]map[string]float64{ "USD": { "GBP": 0.77208, }, @@ -219,7 +182,7 @@ func TestGetRate_ReverseConversion(t *testing.T) { func TestGetRate_EmptyRates(t *testing.T) { // Setup: - rates := NewRates(time.Time{}, nil) + rates := NewRates(nil) // Execute: rate, err := rates.GetRate("USD", "EUR") @@ -232,7 +195,7 @@ func TestGetRate_EmptyRates(t *testing.T) { func TestGetRate_NotValidISOCurrency(t *testing.T) { // Setup: - rates := NewRates(time.Time{}, nil) + rates := NewRates(nil) testCases := []struct { from string diff --git a/endpoints/openrtb2/auction_test.go b/endpoints/openrtb2/auction_test.go index ec373094864..91910560476 100644 --- a/endpoints/openrtb2/auction_test.go +++ b/endpoints/openrtb2/auction_test.go @@ -2774,7 +2774,7 @@ func (e *mockBidExchange) getAuctionCurrencyRates(customRates *openrtb_ext.ExtRe if customRates == nil { // The timestamp is required for the function signature, but is not used and its // value has no significance in the tests - return currency.NewRates(time.Now(), e.pbsRates) + return currency.NewRates(e.pbsRates) } usePbsRates := true @@ -2785,18 +2785,18 @@ func (e *mockBidExchange) getAuctionCurrencyRates(customRates *openrtb_ext.ExtRe if !usePbsRates { // The timestamp is required for the function signature, but is not used and its // value has no significance in the tests - return currency.NewRates(time.Now(), customRates.ConversionRates) + return currency.NewRates(customRates.ConversionRates) } // Both PBS and custom rates can be used, check if ConversionRates is not empty if len(customRates.ConversionRates) == 0 { // Custom rates map is empty, use PBS rates only - return currency.NewRates(time.Now(), e.pbsRates) + return currency.NewRates(e.pbsRates) } // Return an AggregateConversions object that includes both custom and PBS currency rates but will // prioritize custom rates over PBS rates whenever a currency rate is found in both - return currency.NewAggregateConversions(currency.NewRates(time.Time{}, customRates.ConversionRates), currency.NewRates(time.Now(), e.pbsRates)) + return currency.NewAggregateConversions(currency.NewRates(customRates.ConversionRates), currency.NewRates(e.pbsRates)) } // mockBidExchange is a well-behaved exchange that lists the bidders found in every bidRequest.Imp[i].Ext diff --git a/exchange/bidder_test.go b/exchange/bidder_test.go index 213af9a0f05..a31a195c512 100644 --- a/exchange/bidder_test.go +++ b/exchange/bidder_test.go @@ -424,7 +424,6 @@ func TestMultiCurrencies(t *testing.T) { {currency: "USD", price: 1.3}, }, rates: currency.Rates{ - DataAsOf: time.Now(), Conversions: map[string]map[string]float64{ "GBP": { "USD": 1.3050530256, @@ -449,7 +448,6 @@ func TestMultiCurrencies(t *testing.T) { {currency: "", price: 1.3}, }, rates: currency.Rates{ - DataAsOf: time.Now(), Conversions: map[string]map[string]float64{ "GBP": { "USD": 1.3050530256, @@ -474,7 +472,6 @@ func TestMultiCurrencies(t *testing.T) { {currency: "EUR", price: 1.3}, }, rates: currency.Rates{ - DataAsOf: time.Now(), Conversions: map[string]map[string]float64{ "GBP": { "USD": 1.3050530256, @@ -499,7 +496,6 @@ func TestMultiCurrencies(t *testing.T) { {currency: "GBP", price: 1.3}, }, rates: currency.Rates{ - DataAsOf: time.Now(), Conversions: map[string]map[string]float64{ "GBP": { "USD": 1.3050530256, @@ -524,7 +520,6 @@ func TestMultiCurrencies(t *testing.T) { {currency: "GBP", price: 1.3}, }, rates: currency.Rates{ - DataAsOf: time.Now(), Conversions: map[string]map[string]float64{ "GBP": { "USD": 1.3050530256, @@ -549,7 +544,6 @@ func TestMultiCurrencies(t *testing.T) { {currency: "GBP", price: 1.3}, }, rates: currency.Rates{ - DataAsOf: time.Now(), Conversions: map[string]map[string]float64{ "GBP": { "USD": 1.3050530256, @@ -575,7 +569,6 @@ func TestMultiCurrencies(t *testing.T) { {currency: "DKK", price: 1.3}, }, rates: currency.Rates{ - DataAsOf: time.Now(), Conversions: map[string]map[string]float64{ "GBP": { "USD": 1.3050530256, @@ -600,7 +593,6 @@ func TestMultiCurrencies(t *testing.T) { {currency: "CCC", price: 1.3}, }, rates: currency.Rates{ - DataAsOf: time.Now(), Conversions: map[string]map[string]float64{ "GBP": { "USD": 1.3050530256, @@ -858,7 +850,6 @@ func TestMultiCurrencies_RequestCurrencyPick(t *testing.T) { expectedPickedCurrency: "EUR", expectedError: false, rates: currency.Rates{ - DataAsOf: time.Now(), Conversions: map[string]map[string]float64{ "JPY": { "USD": 0.0089, @@ -879,7 +870,6 @@ func TestMultiCurrencies_RequestCurrencyPick(t *testing.T) { expectedPickedCurrency: "JPY", expectedError: false, rates: currency.Rates{ - DataAsOf: time.Now(), Conversions: map[string]map[string]float64{ "JPY": { "USD": 0.0089, @@ -894,7 +884,6 @@ func TestMultiCurrencies_RequestCurrencyPick(t *testing.T) { expectedPickedCurrency: "USD", expectedError: false, rates: currency.Rates{ - DataAsOf: time.Now(), Conversions: map[string]map[string]float64{ "JPY": { "USD": 0.0089, @@ -915,7 +904,6 @@ func TestMultiCurrencies_RequestCurrencyPick(t *testing.T) { expectedPickedCurrency: "", expectedError: true, rates: currency.Rates{ - DataAsOf: time.Now(), Conversions: map[string]map[string]float64{}, }, description: "Case 4 - None allowed currencies in bid request are known, an error is returned", @@ -926,7 +914,6 @@ func TestMultiCurrencies_RequestCurrencyPick(t *testing.T) { expectedPickedCurrency: "USD", expectedError: false, rates: currency.Rates{ - DataAsOf: time.Now(), Conversions: map[string]map[string]float64{}, }, description: "Case 5 - None allowed currencies in bid request are known but the default one (`USD`), no rates are set but default currency will be picked", @@ -937,7 +924,6 @@ func TestMultiCurrencies_RequestCurrencyPick(t *testing.T) { expectedPickedCurrency: "USD", expectedError: false, rates: currency.Rates{ - DataAsOf: time.Now(), Conversions: map[string]map[string]float64{}, }, description: "Case 6 - No allowed currencies specified in bid request, default one is picked: `USD`", diff --git a/exchange/exchange.go b/exchange/exchange.go index 1cedb5d9f43..87b28782e59 100644 --- a/exchange/exchange.go +++ b/exchange/exchange.go @@ -1015,7 +1015,7 @@ func (e *exchange) getAuctionCurrencyRates(requestRates *openrtb_ext.ExtRequestC // At this point, we can safely assume the ConversionRates map is not empty because // validateCustomRates(bidReqCurrencyRates *openrtb_ext.ExtRequestCurrency) would have // thrown an error under such conditions. - return currency.NewRates(time.Time{}, requestRates.ConversionRates) + return currency.NewRates(requestRates.ConversionRates) } // Both PBS and custom rates can be used, check if ConversionRates is not empty @@ -1026,7 +1026,7 @@ func (e *exchange) getAuctionCurrencyRates(requestRates *openrtb_ext.ExtRequestC // Return an AggregateConversions object that includes both custom and PBS currency rates but will // prioritize custom rates over PBS rates whenever a currency rate is found in both - return currency.NewAggregateConversions(currency.NewRates(time.Time{}, requestRates.ConversionRates), e.currencyConverter.Rates()) + return currency.NewAggregateConversions(currency.NewRates(requestRates.ConversionRates), e.currencyConverter.Rates()) } func findCacheID(bid *pbsOrtbBid, auction *auction) (string, bool) {