Remove time.Now() as the first parameter of NewRates() (#1953)

prebid · Aug 18, 2021 · 744bf42 · 744bf42
1 parent 7a9bc31
commit 744bf42
Show file tree

Hide file tree

Showing 7 changed files with 34 additions and 116 deletions.
diff --git a/currency/aggregate_conversions_test.go b/currency/aggregate_conversions_test.go
@@ -3,22 +3,21 @@ package currency
 import (
 	"errors"
 	"testing"
-	"time"
 
 	"github.com/stretchr/testify/assert"
 )
 
 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
@@ -54,7 +54,6 @@ func TestReadWriteRates(t *testing.T) {
 		wantUpdateErr     bool
 		wantConstantRates bool
 		wantLastUpdated   time.Time
-		wantDataAsOf      time.Time
 		wantConversions   map[string]map[string]float64
 	}{
 		{
@@ -63,7 +62,6 @@ func TestReadWriteRates(t *testing.T) {
 			giveMockResponse: getMockRates(),
 			giveMockStatus:   200,
 			wantLastUpdated:  time.Date(2018, time.September, 12, 30, 0, 0, 0, time.UTC),
-			wantDataAsOf:     time.Date(2018, time.September, 12, 0, 0, 0, 0, time.UTC),
 			wantConversions:  map[string]map[string]float64{"USD": {"GBP": 0.77208}, "GBP": {"USD": 1.2952}},
 		},
 		{
@@ -72,7 +70,6 @@ func TestReadWriteRates(t *testing.T) {
 			giveMockResponse: []byte("{}"),
 			giveMockStatus:   200,
 			wantLastUpdated:  time.Date(2018, time.September, 12, 30, 0, 0, 0, time.UTC),
-			wantDataAsOf:     time.Time{},
 			wantConversions:  nil,
 		},
 		{
@@ -143,7 +140,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 +165,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 +252,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
@@ -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
@@ -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
@@ -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