From 8f93953793727cb6cbabada2eddc82c5956269d5 Mon Sep 17 00:00:00 2001 From: Pawel Szczodruch Date: Mon, 25 Nov 2019 15:14:16 -0800 Subject: [PATCH 1/4] refac: put URI as a parameter for a requester method (not the requester itself) --- pkg/client/factory.go | 2 +- pkg/client/factory_test.go | 13 +----- pkg/config/polling_manager.go | 28 +++++------- pkg/config/polling_manager_test.go | 42 +++++++++-------- pkg/config/static_manager.go | 6 ++- pkg/event/dispatcher.go | 4 +- pkg/event/factory.go | 5 ++- pkg/utils/requester.go | 61 ++++++++++++++----------- pkg/utils/requester_test.go | 72 +++++++++++++++++------------- 9 files changed, 118 insertions(+), 115 deletions(-) diff --git a/pkg/client/factory.go b/pkg/client/factory.go index 205864673..26764655f 100644 --- a/pkg/client/factory.go +++ b/pkg/client/factory.go @@ -102,7 +102,7 @@ func (f OptimizelyFactory) Client(clientOptions ...OptionFunc) (*OptimizelyClien // Initialize the default services with the execution context if pollingConfigManager, ok := appClient.ConfigManager.(*config.PollingProjectConfigManager); ok { - pollingConfigManager.Start(appClient.executionCtx) + pollingConfigManager.Start(f.SDKKey, appClient.executionCtx) } if batchProcessor, ok := appClient.EventProcessor.(*event.BatchEventProcessor); ok { diff --git a/pkg/client/factory_test.go b/pkg/client/factory_test.go index b23cc0dcc..9cacf966b 100644 --- a/pkg/client/factory_test.go +++ b/pkg/client/factory_test.go @@ -18,10 +18,6 @@ package client import ( "errors" - "fmt" - "log" - "net/http" - "net/http/httptest" "testing" "time" @@ -75,15 +71,8 @@ func TestClientWithPollingConfigManager(t *testing.T) { func TestClientWithPollingConfigManagerRequester(t *testing.T) { - ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - log.Print(">request: ", r) - if r.URL.String() == "/good" { - fmt.Fprintln(w, "Hello, client") - } - })) - factory := OptimizelyFactory{} - requester := utils.NewHTTPRequester(ts.URL + "/good") + requester := utils.NewHTTPRequester() optimizelyClient, err := factory.Client(WithPollingConfigManagerRequester(requester, time.Minute, nil)) assert.NoError(t, err) assert.NotNil(t, optimizelyClient.ConfigManager) diff --git a/pkg/config/polling_manager.go b/pkg/config/polling_manager.go index 85bc3f54b..37a649548 100644 --- a/pkg/config/polling_manager.go +++ b/pkg/config/polling_manager.go @@ -34,9 +34,6 @@ import ( // DefaultPollingInterval sets default interval for polling manager const DefaultPollingInterval = 5 * time.Minute // default to 5 minutes for polling -// DatafileURLTemplate is used to construct the endpoint for retrieving the datafile from the CDN -const DatafileURLTemplate = "https://cdn.optimizely.com/datafiles/%s.json" - // ModifiedSince header key for request const ModifiedSince = "If-Modified-Since" @@ -61,13 +58,11 @@ type PollingProjectConfigManager struct { // OptionFunc is a type to a proper func type OptionFunc func(*PollingProjectConfigManager) -// DefaultRequester is an optional function, sets default requester based on a key. -func DefaultRequester(sdkKey string) OptionFunc { +// DefaultRequester is an optional function, sets default requester +func DefaultRequester() OptionFunc { return func(p *PollingProjectConfigManager) { - url := fmt.Sprintf(DatafileURLTemplate, sdkKey) - requester := utils.NewHTTPRequester(url) - + requester := utils.NewHTTPRequester() p.requester = requester } } @@ -94,7 +89,7 @@ func InitialDatafile(datafile []byte) OptionFunc { } // SyncConfig gets current datafile and updates projectConfig -func (cm *PollingProjectConfigManager) SyncConfig(datafile []byte) { +func (cm *PollingProjectConfigManager) SyncConfig(sdkKey string, datafile []byte) { var e error var code int var respHeaders http.Header @@ -103,13 +98,13 @@ func (cm *PollingProjectConfigManager) SyncConfig(datafile []byte) { cm.err = e cm.configLock.Unlock() } - + uri := "/" + sdkKey + ".json" if len(datafile) == 0 { if cm.lastModified != "" { lastModifiedHeader := utils.Header{Name: ModifiedSince, Value: cm.lastModified} - datafile, respHeaders, code, e = cm.requester.Get(lastModifiedHeader) + datafile, respHeaders, code, e = cm.requester.Get(uri, lastModifiedHeader) } else { - datafile, respHeaders, code, e = cm.requester.Get() + datafile, respHeaders, code, e = cm.requester.Get(uri) } if e != nil { @@ -168,14 +163,14 @@ func (cm *PollingProjectConfigManager) SyncConfig(datafile []byte) { } // Start starts the polling -func (cm *PollingProjectConfigManager) Start(exeCtx utils.ExecutionCtx) { +func (cm *PollingProjectConfigManager) Start(sdkKey string, exeCtx utils.ExecutionCtx) { go func() { cmLogger.Debug("Polling Config Manager Initiated") t := time.NewTicker(cm.pollingInterval) for { select { case <-t.C: - cm.SyncConfig([]byte{}) + cm.SyncConfig(sdkKey, []byte{}) case <-exeCtx.GetContext().Done(): cmLogger.Debug("Polling Config Manager Stopped") return @@ -186,12 +181,11 @@ func (cm *PollingProjectConfigManager) Start(exeCtx utils.ExecutionCtx) { // NewPollingProjectConfigManager returns an instance of the polling config manager with the customized configuration func NewPollingProjectConfigManager(sdkKey string, pollingMangerOptions ...OptionFunc) *PollingProjectConfigManager { - url := fmt.Sprintf(DatafileURLTemplate, sdkKey) pollingProjectConfigManager := PollingProjectConfigManager{ notificationCenter: registry.GetNotificationCenter(sdkKey), pollingInterval: DefaultPollingInterval, - requester: utils.NewHTTPRequester(url), + requester: utils.NewHTTPRequester(), } for _, opt := range pollingMangerOptions { @@ -199,7 +193,7 @@ func NewPollingProjectConfigManager(sdkKey string, pollingMangerOptions ...Optio } initDatafile := pollingProjectConfigManager.initDatafile - pollingProjectConfigManager.SyncConfig(initDatafile) // initial poll + pollingProjectConfigManager.SyncConfig(sdkKey, initDatafile) // initial poll return &pollingProjectConfigManager } diff --git a/pkg/config/polling_manager_test.go b/pkg/config/polling_manager_test.go index 3512db60c..2646dce83 100644 --- a/pkg/config/polling_manager_test.go +++ b/pkg/config/polling_manager_test.go @@ -34,7 +34,7 @@ type MockRequester struct { mock.Mock } -func (m *MockRequester) Get(headers ...utils.Header) (response []byte, responseHeaders http.Header, code int, err error) { +func (m *MockRequester) Get(uri string, headers ...utils.Header) (response []byte, responseHeaders http.Header, code int, err error) { args := m.Called(headers) return args.Get(0).([]byte), args.Get(1).(http.Header), args.Int(2), args.Error(3) } @@ -51,7 +51,7 @@ func TestNewPollingProjectConfigManagerWithOptions(t *testing.T) { exeCtx := utils.NewCancelableExecutionCtx() configManager := NewPollingProjectConfigManager(sdkKey, Requester(mockRequester)) - configManager.Start(exeCtx) + configManager.Start(sdkKey, exeCtx) mockRequester.AssertExpectations(t) actual, err := configManager.GetConfig() @@ -72,7 +72,7 @@ func TestNewPollingProjectConfigManagerWithNull(t *testing.T) { exeCtx := utils.NewCancelableExecutionCtx() configManager := NewPollingProjectConfigManager(sdkKey, Requester(mockRequester)) - configManager.Start(exeCtx) + configManager.Start(sdkKey, exeCtx) mockRequester.AssertExpectations(t) _, err := configManager.GetConfig() @@ -90,7 +90,7 @@ func TestNewPollingProjectConfigManagerWithSimilarDatafileRevisions(t *testing.T exeCtx := utils.NewCancelableExecutionCtx() configManager := NewPollingProjectConfigManager(sdkKey, Requester(mockRequester)) - configManager.Start(exeCtx) + configManager.Start(sdkKey, exeCtx) mockRequester.AssertExpectations(t) actual, err := configManager.GetConfig() @@ -98,7 +98,7 @@ func TestNewPollingProjectConfigManagerWithSimilarDatafileRevisions(t *testing.T assert.NotNil(t, actual) assert.Equal(t, projectConfig1, actual) - configManager.SyncConfig(mockDatafile2) + configManager.SyncConfig(sdkKey, mockDatafile2) actual, err = configManager.GetConfig() assert.Equal(t, projectConfig1, actual) } @@ -118,7 +118,7 @@ func TestNewPollingProjectConfigManagerWithLastModifiedDates(t *testing.T) { exeCtx := utils.NewCancelableExecutionCtx() configManager := NewPollingProjectConfigManager(sdkKey, Requester(mockRequester)) - configManager.Start(exeCtx) + configManager.Start(sdkKey, exeCtx) // Fetch valid config actual, err := configManager.GetConfig() @@ -127,7 +127,7 @@ func TestNewPollingProjectConfigManagerWithLastModifiedDates(t *testing.T) { assert.Equal(t, projectConfig1, actual) // Sync and check no changes were made to the previous config because of 304 error code - configManager.SyncConfig([]byte{}) + configManager.SyncConfig(sdkKey, []byte{}) actual, err = configManager.GetConfig() assert.Nil(t, err) assert.NotNil(t, actual) @@ -148,7 +148,7 @@ func TestNewPollingProjectConfigManagerWithDifferentDatafileRevisions(t *testing exeCtx := utils.NewCancelableExecutionCtx() configManager := NewPollingProjectConfigManager(sdkKey, Requester(mockRequester)) - configManager.Start(exeCtx) + configManager.Start(sdkKey, exeCtx) mockRequester.AssertExpectations(t) actual, err := configManager.GetConfig() @@ -156,7 +156,7 @@ func TestNewPollingProjectConfigManagerWithDifferentDatafileRevisions(t *testing assert.NotNil(t, actual) assert.Equal(t, projectConfig1, actual) - configManager.SyncConfig(mockDatafile2) + configManager.SyncConfig(sdkKey, mockDatafile2) actual, err = configManager.GetConfig() assert.Equal(t, projectConfig2, actual) } @@ -175,7 +175,7 @@ func TestNewPollingProjectConfigManagerWithErrorHandling(t *testing.T) { exeCtx := utils.NewCancelableExecutionCtx() configManager := NewPollingProjectConfigManager(sdkKey, Requester(mockRequester)) - configManager.Start(exeCtx) + configManager.Start(sdkKey, exeCtx) mockRequester.AssertExpectations(t) actual, err := configManager.GetConfig() // polling for bad file @@ -183,12 +183,12 @@ func TestNewPollingProjectConfigManagerWithErrorHandling(t *testing.T) { assert.Nil(t, actual) assert.Nil(t, projectConfig1) - configManager.SyncConfig(mockDatafile2) // polling for good file + configManager.SyncConfig(sdkKey, mockDatafile2) // polling for good file actual, err = configManager.GetConfig() assert.Nil(t, err) assert.Equal(t, projectConfig2, actual) - configManager.SyncConfig(mockDatafile1) // polling for bad file, error not null but good project + configManager.SyncConfig(sdkKey, mockDatafile1) // polling for bad file, error not null but good project actual, err = configManager.GetConfig() assert.Nil(t, err) assert.Equal(t, projectConfig2, actual) @@ -206,7 +206,7 @@ func TestNewPollingProjectConfigManagerOnDecision(t *testing.T) { exeCtx := utils.NewCancelableExecutionCtx() configManager := NewPollingProjectConfigManager(sdkKey, Requester(mockRequester)) - configManager.Start(exeCtx) + configManager.Start(sdkKey, exeCtx) var numberOfCalls = 0 callback := func(notification notification.ProjectConfigUpdateNotification) { @@ -219,7 +219,7 @@ func TestNewPollingProjectConfigManagerOnDecision(t *testing.T) { assert.Nil(t, err) assert.NotNil(t, actual) - configManager.SyncConfig(mockDatafile2) + configManager.SyncConfig(sdkKey, mockDatafile2) actual, err = configManager.GetConfig() assert.Nil(t, err) assert.NotNil(t, actual) @@ -237,23 +237,22 @@ func TestNewPollingProjectConfigManagerOnDecision(t *testing.T) { func TestDefaultRequester(t *testing.T) { sdkKey := "test_sdk_key" - DefaultRequester(sdkKey) exeCtx := utils.NewCancelableExecutionCtx() - configManager := NewPollingProjectConfigManager(sdkKey, DefaultRequester(sdkKey)) - configManager.Start(exeCtx) + configManager := NewPollingProjectConfigManager(sdkKey, DefaultRequester()) + configManager.Start(sdkKey, exeCtx) requester := configManager.requester assert.NotNil(t, requester) - assert.Equal(t, requester.String(), "{url: https://cdn.optimizely.com/datafiles/test_sdk_key.json, timeout: 5s, retries: 1}") + assert.Equal(t, requester.String(), "{api: https://cdn.optimizely.com/datafiles, timeout: 5s, retries: 1}") } func TestPollingInterval(t *testing.T) { sdkKey := "test_sdk_key" - DefaultRequester(sdkKey) + exeCtx := utils.NewCancelableExecutionCtx() configManager := NewPollingProjectConfigManager(sdkKey, PollingInterval(5*time.Second)) - configManager.Start(exeCtx) + configManager.Start(sdkKey, exeCtx) assert.Equal(t, configManager.pollingInterval, 5*time.Second) } @@ -261,10 +260,9 @@ func TestPollingInterval(t *testing.T) { func TestInitialDatafile(t *testing.T) { sdkKey := "test_sdk_key" - DefaultRequester(sdkKey) exeCtx := utils.NewCancelableExecutionCtx() configManager := NewPollingProjectConfigManager(sdkKey, InitialDatafile([]byte("test"))) - configManager.Start(exeCtx) + configManager.Start(sdkKey, exeCtx) assert.Equal(t, configManager.initDatafile, []byte("test")) } diff --git a/pkg/config/static_manager.go b/pkg/config/static_manager.go index 3b7bddfdf..6ed1a2228 100644 --- a/pkg/config/static_manager.go +++ b/pkg/config/static_manager.go @@ -37,8 +37,10 @@ type StaticProjectConfigManager struct { // NewStaticProjectConfigManagerFromURL returns new instance of StaticProjectConfigManager for URL func NewStaticProjectConfigManagerFromURL(sdkKey string) (*StaticProjectConfigManager, error) { - requester := utils.NewHTTPRequester(fmt.Sprintf(DatafileURLTemplate, sdkKey)) - datafile, _, code, e := requester.Get() + requester := utils.NewHTTPRequester() + + uri := "/" + sdkKey + ".json" + datafile, _, code, e := requester.Get(uri) if e != nil { cmLogger.Error(fmt.Sprintf("request returned with http code=%d", code), e) return nil, e diff --git a/pkg/event/dispatcher.go b/pkg/event/dispatcher.go index 2a4da9b14..01fcea055 100644 --- a/pkg/event/dispatcher.go +++ b/pkg/event/dispatcher.go @@ -46,8 +46,8 @@ type HTTPEventDispatcher struct { // DispatchEvent dispatches event with callback func (*HTTPEventDispatcher) DispatchEvent(event LogEvent) (bool, error) { - requester := utils.NewHTTPRequester(event.EndPoint) - _, _, code, err := requester.Post(event.Event) + requester := utils.NewHTTPRequester(utils.API(eventAPI)) + _, _, code, err := requester.Post(eventURI, event.Event) // also check response codes // resp.StatusCode == 400 is an error diff --git a/pkg/event/factory.go b/pkg/event/factory.go index 5add1700d..ba476b827 100644 --- a/pkg/event/factory.go +++ b/pkg/event/factory.go @@ -39,12 +39,13 @@ const clientVersion string = pkg.Version const attributeType = "custom" const specialPrefix = "$opt_" const botFilteringKey = "$opt_bot_filtering" -const eventEndPoint = "https://logx.optimizely.com/v1/events" +const eventAPI = "https://logx.optimizely.com/v1" +const eventURI = "/events" const revenueKey = "revenue" const valueKey = "value" func createLogEvent(event Batch) LogEvent { - return LogEvent{EndPoint: eventEndPoint, Event: event} + return LogEvent{EndPoint: eventAPI + eventURI, Event: event} } func makeTimestamp() int64 { diff --git a/pkg/utils/requester.go b/pkg/utils/requester.go index c4220c3c6..eefecba1e 100644 --- a/pkg/utils/requester.go +++ b/pkg/utils/requester.go @@ -33,16 +33,19 @@ import ( const defaultTTL = 5 * time.Second +// DatafileAPI is used as a default API for retrieving the datafile from the CDN +const DatafileAPI = "https://cdn.optimizely.com/datafiles" + var requesterLogger = logging.GetLogger("Requester") var json = jsoniter.ConfigCompatibleWithStandardLibrary // Requester is used to make outbound requests with type Requester interface { - Get(...Header) (response []byte, responseHeaders http.Header, code int, err error) - GetObj(result interface{}, headers ...Header) error + Get(uri string, headers ...Header) (response []byte, responseHeaders http.Header, code int, err error) + GetObj(uri string, result interface{}, headers ...Header) error - Post(body interface{}, headers ...Header) (response []byte, responseHeaders http.Header, code int, err error) - PostObj(body interface{}, result interface{}, headers ...Header) error + Post(uri string, body interface{}, headers ...Header) (response []byte, responseHeaders http.Header, code int, err error) + PostObj(uri string, body interface{}, result interface{}, headers ...Header) error String() string } @@ -59,6 +62,13 @@ func Timeout(timeout time.Duration) func(r *HTTPRequester) { } } +// API sets api portion of url +func API(api string) func(r *HTTPRequester) { + return func(r *HTTPRequester) { + r.api = api + } +} + // Retries sets max number of retries for failed calls func Retries(retries int) func(r *HTTPRequester) { return func(r *HTTPRequester) { @@ -76,18 +86,18 @@ func Headers(headers ...Header) func(r *HTTPRequester) { // HTTPRequester contains main info type HTTPRequester struct { - url string + api string client http.Client retries int headers []Header } // NewHTTPRequester makes Requester with api and parameters. Sets defaults -// url has a complete url of the request like https://cdn.optimizely.com/datafiles/24234.json -func NewHTTPRequester(url string, params ...func(*HTTPRequester)) *HTTPRequester { +// api has the base part of request's url, like http://localhost/api/v1 +func NewHTTPRequester(params ...func(*HTTPRequester)) *HTTPRequester { res := HTTPRequester{ - url: url, + api: DatafileAPI, retries: 1, headers: []Header{{"Content-Type", "application/json"}, {"Accept", "application/json"}}, client: http.Client{Timeout: defaultTTL}, @@ -100,14 +110,14 @@ func NewHTTPRequester(url string, params ...func(*HTTPRequester)) *HTTPRequester } // Get executes HTTP GET with url and optional extra headers, returns body in []bytes -// url created as url+sdkKey.json -func (r HTTPRequester) Get(headers ...Header) (response []byte, responseHeaders http.Header, code int, err error) { - return r.Do("GET", nil, headers) +// url created as api+sdkKey.json +func (r HTTPRequester) Get(uri string, headers ...Header) (response []byte, responseHeaders http.Header, code int, err error) { + return r.Do(uri, "GET", nil, headers) } // GetObj executes HTTP GET with url and optional extra headers, returns filled object -func (r HTTPRequester) GetObj(result interface{}, headers ...Header) error { - b, _, _, err := r.Do("GET", nil, headers) +func (r HTTPRequester) GetObj(uri string, result interface{}, headers ...Header) error { + b, _, _, err := r.Do(uri, "GET", nil, headers) if err != nil { return err } @@ -115,17 +125,17 @@ func (r HTTPRequester) GetObj(result interface{}, headers ...Header) error { } // Post executes HTTP POST with url, body and optional extra headers -func (r HTTPRequester) Post(body interface{}, headers ...Header) (response []byte, responseHeaders http.Header, code int, err error) { +func (r HTTPRequester) Post(uri string, body interface{}, headers ...Header) (response []byte, responseHeaders http.Header, code int, err error) { b, err := json.Marshal(body) if err != nil { return nil, nil, http.StatusBadRequest, err } - return r.Do("POST", bytes.NewBuffer(b), headers) + return r.Do(uri, "POST", bytes.NewBuffer(b), headers) } // PostObj executes HTTP POST with uri, body and optional extra headers. Returns filled object -func (r HTTPRequester) PostObj(body, result interface{}, headers ...Header) error { - b, _, _, err := r.Post(body, headers...) +func (r HTTPRequester) PostObj(uri string, body, result interface{}, headers ...Header) error { + b, _, _, err := r.Post(uri, body, headers...) if err != nil { return err } @@ -133,7 +143,7 @@ func (r HTTPRequester) PostObj(body, result interface{}, headers ...Header) erro } // Do executes request and returns response body for requested uri (sdkKey.json). -func (r HTTPRequester) Do(method string, body io.Reader, headers []Header) (response []byte, responseHeaders http.Header, code int, err error) { +func (r HTTPRequester) Do(uri string, method string, body io.Reader, headers []Header) (response []byte, responseHeaders http.Header, code int, err error) { single := func(request *http.Request) (response []byte, responseHeaders http.Header, code int, e error) { resp, doErr := r.client.Do(request) @@ -159,11 +169,11 @@ func (r HTTPRequester) Do(method string, body io.Reader, headers []Header) (resp return response, resp.Header, resp.StatusCode, nil } - - requesterLogger.Debug(fmt.Sprintf("request %s", r.url)) - req, err := http.NewRequest(method, r.url, body) + reqURL := fmt.Sprintf("%s%s", r.api, uri) + requesterLogger.Debug(fmt.Sprintf("request %s", reqURL)) + req, err := http.NewRequest(method, reqURL, body) if err != nil { - requesterLogger.Error(fmt.Sprintf("failed to make request %s", r.url), err) + requesterLogger.Error(fmt.Sprintf("failed to make request %s", reqURL), err) return nil, nil, 0, err } @@ -176,10 +186,10 @@ func (r HTTPRequester) Do(method string, body io.Reader, headers []Header) (resp if i > 0 { triedMsg = fmt.Sprintf(", tried %d time(s)", i+1) } - requesterLogger.Debug(fmt.Sprintf("completed %s%s", r.url, triedMsg)) + requesterLogger.Debug(fmt.Sprintf("completed %s%s", reqURL, triedMsg)) return response, responseHeaders, code, err } - requesterLogger.Debug(fmt.Sprintf("failed %s with %v", r.url, err)) + requesterLogger.Debug(fmt.Sprintf("failed %s with %v", reqURL, err)) if i != r.retries { delay := time.Duration(500) * time.Millisecond @@ -201,6 +211,5 @@ func (r HTTPRequester) addHeaders(req *http.Request, headers []Header) *http.Req } func (r HTTPRequester) String() string { - return fmt.Sprintf("{url: %s, timeout: %v, retries: %d}", - r.url, r.client.Timeout, r.retries) + return fmt.Sprintf("{api: %s, timeout: %v, retries: %d}", r.api, r.client.Timeout, r.retries) } diff --git a/pkg/utils/requester_test.go b/pkg/utils/requester_test.go index d62d1c5c0..8dcd6e25c 100644 --- a/pkg/utils/requester_test.go +++ b/pkg/utils/requester_test.go @@ -62,14 +62,14 @@ func TestGet(t *testing.T) { defer ts.Close() var httpreq Requester - httpreq = NewHTTPRequester(ts.URL + "/good") - resp, headers, code, err := httpreq.Get() + httpreq = NewHTTPRequester(API(ts.URL)) + resp, headers, code, err := httpreq.Get("/good") assert.NotEqual(t, headers.Get("Content-Type"), "") assert.Nil(t, err) assert.Equal(t, "Hello, client\n", string(resp)) - httpreq = NewHTTPRequester(ts.URL + "/bad") - _, headers, code, err = httpreq.Get() + httpreq = NewHTTPRequester(API(ts.URL)) + _, headers, code, err = httpreq.Get("/bad") assert.Equal(t, errors.New("400 Bad Request"), err) assert.Equal(t, code, http.StatusBadRequest) } @@ -94,14 +94,14 @@ func TestGetObj(t *testing.T) { defer ts.Close() var httpreq Requester - httpreq = NewHTTPRequester(ts.URL + "/good") + httpreq = NewHTTPRequester(API(ts.URL)) r := resp{} - err := httpreq.GetObj(&r) + err := httpreq.GetObj("/good", &r) assert.Nil(t, err) assert.Equal(t, resp{Fld1: "Hello, client", Fld2: 123}, r) - httpreq = NewHTTPRequester(ts.URL + "/bad") - err = httpreq.GetObj(&r) + httpreq = NewHTTPRequester(API(ts.URL)) + err = httpreq.GetObj("/bad", &r) assert.NotNil(t, err) } @@ -124,16 +124,16 @@ func TestPost(t *testing.T) { defer ts.Close() b := body{"one", 1} var httpreq Requester - httpreq = NewHTTPRequester(ts.URL + "/good") - resp, headers, code, err := httpreq.Post(b) + httpreq = NewHTTPRequester(API(ts.URL)) + resp, headers, code, err := httpreq.Post("/good", b) assert.Nil(t, err) assert.NotEqual(t, headers.Get("Content-Type"), "") assert.Equal(t, "Hello, client\n", string(resp)) assert.Equal(t, code, http.StatusOK) - httpreq = NewHTTPRequester(ts.URL + "/bad") - _, _, code, err = httpreq.Post(nil) + httpreq = NewHTTPRequester(API(ts.URL)) + _, _, code, err = httpreq.Post("/bad", nil) assert.Equal(t, errors.New("400 Bad Request"), err) assert.Equal(t, code, http.StatusBadRequest) } @@ -158,21 +158,30 @@ func TestPostObj(t *testing.T) { defer ts.Close() var httpreq Requester - httpreq = NewHTTPRequester(ts.URL + "/good") + httpreq = NewHTTPRequester(API(ts.URL)) b := body{"one", 1} r := body{} - err := httpreq.PostObj(b, &r) + err := httpreq.PostObj("/good", b, &r) assert.Nil(t, err) assert.Equal(t, body{Fld1: "Hello, client", Fld2: 123}, r) - httpreq = NewHTTPRequester(ts.URL + "/bad") - err = httpreq.PostObj(b, &r) + httpreq = NewHTTPRequester(API(ts.URL)) + err = httpreq.PostObj("/bad", b, &r) assert.NotNil(t, err) } func TestGetBad(t *testing.T) { - httpreq := NewHTTPRequester("blah12345/good") - _, _, _, err := httpreq.Get() + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + log.Print(">request: ", r) + if r.URL.String() == "/good" { + fmt.Fprintln(w, "Hello, client") + } + if r.URL.String() == "/bad" { + w.WriteHeader(http.StatusBadRequest) + } + })) + httpreq := NewHTTPRequester(API(ts.URL)) + _, _, _, err := httpreq.Get("blah12345/good") _, ok := err.(*url.Error) assert.True(t, ok, "url error") } @@ -191,8 +200,8 @@ func TestGetBadWithResponse(t *testing.T) { })) defer ts.Close() - httpreq := NewHTTPRequester(ts.URL+"/bad", Retries(1)) - data, _, _, err := httpreq.Get() + httpreq := NewHTTPRequester(API(ts.URL), Retries(1)) + data, _, _, err := httpreq.Get("/bad") assert.Equal(t, "400 Bad Request", err.Error()) assert.Equal(t, "bad bad response\n", string(data)) } @@ -220,10 +229,10 @@ func TestGetRetry(t *testing.T) { })) defer ts.Close() - httpreq := NewHTTPRequester(ts.URL+"/test", Retries(10)) + httpreq := NewHTTPRequester(API(ts.URL), Retries(10)) st := time.Now() - resp, _, _, err := httpreq.Get() + resp, _, _, err := httpreq.Get("/test") assert.Nil(t, err) assert.Equal(t, "Hello, client\n", string(resp)) assert.Equal(t, 5, called, "called 5 retries") @@ -231,27 +240,28 @@ func TestGetRetry(t *testing.T) { assert.True(t, elapsed >= 400*5*time.Millisecond && elapsed <= 510*5*time.Second, "took %s", elapsed) - httpreq = NewHTTPRequester(ts.URL+"/test", Retries(3)) + httpreq = NewHTTPRequester(API(ts.URL), Retries(3)) called = 0 - _, _, _, err = httpreq.Get() + _, _, _, err = httpreq.Get("/test") assert.Equal(t, errors.New("400 Bad Request"), err) assert.Equal(t, 3, called, "called 3 retries") - httpreq = NewHTTPRequester(ts.URL+"/test", Retries(1)) + httpreq = NewHTTPRequester(API(ts.URL), Retries(1)) called = 0 - _, _, _, err = httpreq.Get() + _, _, _, err = httpreq.Get("/test") assert.Equal(t, errors.New("400 Bad Request"), err) assert.Equal(t, 1, called, "called 1 retries") - httpreq = NewHTTPRequester(ts.URL + "/test") + httpreq = NewHTTPRequester(API(ts.URL)) called = 0 - _, _, _, err = httpreq.Get() + _, _, _, err = httpreq.Get("/test") assert.Equal(t, errors.New("400 Bad Request"), err) assert.Equal(t, 1, called, "called 1 retries") } func TestString(t *testing.T) { - assert.Equal(t, "{url: 127.0.0.1/blah, timeout: 5s, retries: 1}", NewHTTPRequester("127.0.0.1/blah").String()) - assert.Equal(t, "{url: 127.0.0.1/blah, timeout: 19s, retries: 10}", - NewHTTPRequester("127.0.0.1/blah", Retries(10), Timeout(time.Duration(19)*time.Second)).String()) + assert.Equal(t, "{api: https://cdn.optimizely.com/datafiles, timeout: 5s, retries: 1}", NewHTTPRequester().String()) + assert.Equal(t, "{api: 127.0.0.1/blah, timeout: 19s, retries: 10}", + NewHTTPRequester(API("127.0.0.1/blah"), Retries(10), Timeout(time.Duration(19)*time.Second)).String()) + } From 31bb55f6ae818c45a0e6d057aac143290e37aaf4 Mon Sep 17 00:00:00 2001 From: Pawel Szczodruch Date: Mon, 25 Nov 2019 15:18:43 -0800 Subject: [PATCH 2/4] addressing a small linter complaint --- pkg/utils/requester.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/utils/requester.go b/pkg/utils/requester.go index eefecba1e..05d9897f9 100644 --- a/pkg/utils/requester.go +++ b/pkg/utils/requester.go @@ -143,7 +143,7 @@ func (r HTTPRequester) PostObj(uri string, body, result interface{}, headers ... } // Do executes request and returns response body for requested uri (sdkKey.json). -func (r HTTPRequester) Do(uri string, method string, body io.Reader, headers []Header) (response []byte, responseHeaders http.Header, code int, err error) { +func (r HTTPRequester) Do(uri, method string, body io.Reader, headers []Header) (response []byte, responseHeaders http.Header, code int, err error) { single := func(request *http.Request) (response []byte, responseHeaders http.Header, code int, e error) { resp, doErr := r.client.Do(request) From 293bf829d5db625416c454f9bcd753125cdb4519 Mon Sep 17 00:00:00 2001 From: Pawel Szczodruch Date: Tue, 26 Nov 2019 09:53:37 -0800 Subject: [PATCH 3/4] addressing after PR review --- pkg/config/polling_manager.go | 35 ++++++++++----- pkg/config/polling_manager_test.go | 11 ++++- pkg/config/static_manager.go | 4 +- pkg/event/dispatcher.go | 4 +- pkg/event/factory.go | 5 +-- pkg/utils/requester.go | 56 ++++++++++-------------- pkg/utils/requester_test.go | 70 +++++++++++++----------------- 7 files changed, 93 insertions(+), 92 deletions(-) diff --git a/pkg/config/polling_manager.go b/pkg/config/polling_manager.go index 37a649548..2568a4555 100644 --- a/pkg/config/polling_manager.go +++ b/pkg/config/polling_manager.go @@ -40,15 +40,19 @@ const ModifiedSince = "If-Modified-Since" // LastModified header key for response const LastModified = "Last-Modified" +// DatafileURLTemplate is used to construct the endpoint for retrieving the datafile from the CDN +const DatafileURLTemplate = "https://cdn.optimizely.com/datafiles/%s.json" + var cmLogger = logging.GetLogger("PollingConfigManager") // PollingProjectConfigManager maintains a dynamic copy of the project config type PollingProjectConfigManager struct { - requester utils.Requester - pollingInterval time.Duration - notificationCenter notification.Center - initDatafile []byte - lastModified string + requester utils.Requester + pollingInterval time.Duration + notificationCenter notification.Center + initDatafile []byte + lastModified string + datafileURLTemplate string configLock sync.RWMutex err error @@ -74,6 +78,13 @@ func Requester(requester utils.Requester) OptionFunc { } } +// DatafileTemplate is an optional function, sets a passed datafile URL template +func DatafileTemplate(datafileTemplate string) OptionFunc { + return func(p *PollingProjectConfigManager) { + p.datafileURLTemplate = datafileTemplate + } +} + // PollingInterval is an optional function, sets a passed polling interval func PollingInterval(interval time.Duration) OptionFunc { return func(p *PollingProjectConfigManager) { @@ -98,13 +109,14 @@ func (cm *PollingProjectConfigManager) SyncConfig(sdkKey string, datafile []byte cm.err = e cm.configLock.Unlock() } - uri := "/" + sdkKey + ".json" + + url := fmt.Sprintf(cm.datafileURLTemplate, sdkKey) if len(datafile) == 0 { if cm.lastModified != "" { lastModifiedHeader := utils.Header{Name: ModifiedSince, Value: cm.lastModified} - datafile, respHeaders, code, e = cm.requester.Get(uri, lastModifiedHeader) + datafile, respHeaders, code, e = cm.requester.Get(url, lastModifiedHeader) } else { - datafile, respHeaders, code, e = cm.requester.Get(uri) + datafile, respHeaders, code, e = cm.requester.Get(url) } if e != nil { @@ -183,9 +195,10 @@ func (cm *PollingProjectConfigManager) Start(sdkKey string, exeCtx utils.Executi func NewPollingProjectConfigManager(sdkKey string, pollingMangerOptions ...OptionFunc) *PollingProjectConfigManager { pollingProjectConfigManager := PollingProjectConfigManager{ - notificationCenter: registry.GetNotificationCenter(sdkKey), - pollingInterval: DefaultPollingInterval, - requester: utils.NewHTTPRequester(), + notificationCenter: registry.GetNotificationCenter(sdkKey), + pollingInterval: DefaultPollingInterval, + requester: utils.NewHTTPRequester(), + datafileURLTemplate: DatafileURLTemplate, } for _, opt := range pollingMangerOptions { diff --git a/pkg/config/polling_manager_test.go b/pkg/config/polling_manager_test.go index 2646dce83..3f2f450d8 100644 --- a/pkg/config/polling_manager_test.go +++ b/pkg/config/polling_manager_test.go @@ -243,7 +243,7 @@ func TestDefaultRequester(t *testing.T) { requester := configManager.requester assert.NotNil(t, requester) - assert.Equal(t, requester.String(), "{api: https://cdn.optimizely.com/datafiles, timeout: 5s, retries: 1}") + assert.Equal(t, requester.String(), "{timeout: 5s, retries: 1}") } func TestPollingInterval(t *testing.T) { @@ -266,3 +266,12 @@ func TestInitialDatafile(t *testing.T) { assert.Equal(t, configManager.initDatafile, []byte("test")) } + +func TestDatafileTemplate(t *testing.T) { + + sdkKey := "test_sdk_key" + datafileTemplate := "https://localhost/v1/%s.json" + configManager := NewPollingProjectConfigManager(sdkKey, DatafileTemplate(datafileTemplate)) + + assert.Equal(t, datafileTemplate, configManager.datafileURLTemplate) +} diff --git a/pkg/config/static_manager.go b/pkg/config/static_manager.go index 6ed1a2228..08c975250 100644 --- a/pkg/config/static_manager.go +++ b/pkg/config/static_manager.go @@ -39,8 +39,8 @@ func NewStaticProjectConfigManagerFromURL(sdkKey string) (*StaticProjectConfigMa requester := utils.NewHTTPRequester() - uri := "/" + sdkKey + ".json" - datafile, _, code, e := requester.Get(uri) + url := fmt.Sprintf(DatafileURLTemplate, sdkKey) + datafile, _, code, e := requester.Get(url) if e != nil { cmLogger.Error(fmt.Sprintf("request returned with http code=%d", code), e) return nil, e diff --git a/pkg/event/dispatcher.go b/pkg/event/dispatcher.go index 01fcea055..dea882bb7 100644 --- a/pkg/event/dispatcher.go +++ b/pkg/event/dispatcher.go @@ -46,8 +46,8 @@ type HTTPEventDispatcher struct { // DispatchEvent dispatches event with callback func (*HTTPEventDispatcher) DispatchEvent(event LogEvent) (bool, error) { - requester := utils.NewHTTPRequester(utils.API(eventAPI)) - _, _, code, err := requester.Post(eventURI, event.Event) + requester := utils.NewHTTPRequester() + _, _, code, err := requester.Post(event.EndPoint, event.Event) // also check response codes // resp.StatusCode == 400 is an error diff --git a/pkg/event/factory.go b/pkg/event/factory.go index ba476b827..5add1700d 100644 --- a/pkg/event/factory.go +++ b/pkg/event/factory.go @@ -39,13 +39,12 @@ const clientVersion string = pkg.Version const attributeType = "custom" const specialPrefix = "$opt_" const botFilteringKey = "$opt_bot_filtering" -const eventAPI = "https://logx.optimizely.com/v1" -const eventURI = "/events" +const eventEndPoint = "https://logx.optimizely.com/v1/events" const revenueKey = "revenue" const valueKey = "value" func createLogEvent(event Batch) LogEvent { - return LogEvent{EndPoint: eventAPI + eventURI, Event: event} + return LogEvent{EndPoint: eventEndPoint, Event: event} } func makeTimestamp() int64 { diff --git a/pkg/utils/requester.go b/pkg/utils/requester.go index 05d9897f9..bc2fe85ce 100644 --- a/pkg/utils/requester.go +++ b/pkg/utils/requester.go @@ -33,19 +33,16 @@ import ( const defaultTTL = 5 * time.Second -// DatafileAPI is used as a default API for retrieving the datafile from the CDN -const DatafileAPI = "https://cdn.optimizely.com/datafiles" - var requesterLogger = logging.GetLogger("Requester") var json = jsoniter.ConfigCompatibleWithStandardLibrary // Requester is used to make outbound requests with type Requester interface { - Get(uri string, headers ...Header) (response []byte, responseHeaders http.Header, code int, err error) - GetObj(uri string, result interface{}, headers ...Header) error + Get(url string, headers ...Header) (response []byte, responseHeaders http.Header, code int, err error) + GetObj(url string, result interface{}, headers ...Header) error - Post(uri string, body interface{}, headers ...Header) (response []byte, responseHeaders http.Header, code int, err error) - PostObj(uri string, body interface{}, result interface{}, headers ...Header) error + Post(url string, body interface{}, headers ...Header) (response []byte, responseHeaders http.Header, code int, err error) + PostObj(url string, body interface{}, result interface{}, headers ...Header) error String() string } @@ -62,13 +59,6 @@ func Timeout(timeout time.Duration) func(r *HTTPRequester) { } } -// API sets api portion of url -func API(api string) func(r *HTTPRequester) { - return func(r *HTTPRequester) { - r.api = api - } -} - // Retries sets max number of retries for failed calls func Retries(retries int) func(r *HTTPRequester) { return func(r *HTTPRequester) { @@ -86,7 +76,6 @@ func Headers(headers ...Header) func(r *HTTPRequester) { // HTTPRequester contains main info type HTTPRequester struct { - api string client http.Client retries int headers []Header @@ -97,7 +86,6 @@ type HTTPRequester struct { func NewHTTPRequester(params ...func(*HTTPRequester)) *HTTPRequester { res := HTTPRequester{ - api: DatafileAPI, retries: 1, headers: []Header{{"Content-Type", "application/json"}, {"Accept", "application/json"}}, client: http.Client{Timeout: defaultTTL}, @@ -111,13 +99,13 @@ func NewHTTPRequester(params ...func(*HTTPRequester)) *HTTPRequester { // Get executes HTTP GET with url and optional extra headers, returns body in []bytes // url created as api+sdkKey.json -func (r HTTPRequester) Get(uri string, headers ...Header) (response []byte, responseHeaders http.Header, code int, err error) { - return r.Do(uri, "GET", nil, headers) +func (r HTTPRequester) Get(url string, headers ...Header) (response []byte, responseHeaders http.Header, code int, err error) { + return r.Do(url, "GET", nil, headers) } // GetObj executes HTTP GET with url and optional extra headers, returns filled object -func (r HTTPRequester) GetObj(uri string, result interface{}, headers ...Header) error { - b, _, _, err := r.Do(uri, "GET", nil, headers) +func (r HTTPRequester) GetObj(url string, result interface{}, headers ...Header) error { + b, _, _, err := r.Do(url, "GET", nil, headers) if err != nil { return err } @@ -125,25 +113,25 @@ func (r HTTPRequester) GetObj(uri string, result interface{}, headers ...Header) } // Post executes HTTP POST with url, body and optional extra headers -func (r HTTPRequester) Post(uri string, body interface{}, headers ...Header) (response []byte, responseHeaders http.Header, code int, err error) { +func (r HTTPRequester) Post(url string, body interface{}, headers ...Header) (response []byte, responseHeaders http.Header, code int, err error) { b, err := json.Marshal(body) if err != nil { return nil, nil, http.StatusBadRequest, err } - return r.Do(uri, "POST", bytes.NewBuffer(b), headers) + return r.Do(url, "POST", bytes.NewBuffer(b), headers) } -// PostObj executes HTTP POST with uri, body and optional extra headers. Returns filled object -func (r HTTPRequester) PostObj(uri string, body, result interface{}, headers ...Header) error { - b, _, _, err := r.Post(uri, body, headers...) +// PostObj executes HTTP POST with url, body and optional extra headers. Returns filled object +func (r HTTPRequester) PostObj(url string, body, result interface{}, headers ...Header) error { + b, _, _, err := r.Post(url, body, headers...) if err != nil { return err } return json.Unmarshal(b, result) } -// Do executes request and returns response body for requested uri (sdkKey.json). -func (r HTTPRequester) Do(uri, method string, body io.Reader, headers []Header) (response []byte, responseHeaders http.Header, code int, err error) { +// Do executes request and returns response body for requested url +func (r HTTPRequester) Do(url, method string, body io.Reader, headers []Header) (response []byte, responseHeaders http.Header, code int, err error) { single := func(request *http.Request) (response []byte, responseHeaders http.Header, code int, e error) { resp, doErr := r.client.Do(request) @@ -169,11 +157,11 @@ func (r HTTPRequester) Do(uri, method string, body io.Reader, headers []Header) return response, resp.Header, resp.StatusCode, nil } - reqURL := fmt.Sprintf("%s%s", r.api, uri) - requesterLogger.Debug(fmt.Sprintf("request %s", reqURL)) - req, err := http.NewRequest(method, reqURL, body) + + requesterLogger.Debug(fmt.Sprintf("request %s", url)) + req, err := http.NewRequest(method, url, body) if err != nil { - requesterLogger.Error(fmt.Sprintf("failed to make request %s", reqURL), err) + requesterLogger.Error(fmt.Sprintf("failed to make request %s", url), err) return nil, nil, 0, err } @@ -186,10 +174,10 @@ func (r HTTPRequester) Do(uri, method string, body io.Reader, headers []Header) if i > 0 { triedMsg = fmt.Sprintf(", tried %d time(s)", i+1) } - requesterLogger.Debug(fmt.Sprintf("completed %s%s", reqURL, triedMsg)) + requesterLogger.Debug(fmt.Sprintf("completed %s%s", url, triedMsg)) return response, responseHeaders, code, err } - requesterLogger.Debug(fmt.Sprintf("failed %s with %v", reqURL, err)) + requesterLogger.Debug(fmt.Sprintf("failed %s with %v", url, err)) if i != r.retries { delay := time.Duration(500) * time.Millisecond @@ -211,5 +199,5 @@ func (r HTTPRequester) addHeaders(req *http.Request, headers []Header) *http.Req } func (r HTTPRequester) String() string { - return fmt.Sprintf("{api: %s, timeout: %v, retries: %d}", r.api, r.client.Timeout, r.retries) + return fmt.Sprintf("{timeout: %v, retries: %d}", r.client.Timeout, r.retries) } diff --git a/pkg/utils/requester_test.go b/pkg/utils/requester_test.go index 8dcd6e25c..35838b9da 100644 --- a/pkg/utils/requester_test.go +++ b/pkg/utils/requester_test.go @@ -62,14 +62,14 @@ func TestGet(t *testing.T) { defer ts.Close() var httpreq Requester - httpreq = NewHTTPRequester(API(ts.URL)) - resp, headers, code, err := httpreq.Get("/good") + httpreq = NewHTTPRequester() + resp, headers, code, err := httpreq.Get(ts.URL + "/good") assert.NotEqual(t, headers.Get("Content-Type"), "") assert.Nil(t, err) assert.Equal(t, "Hello, client\n", string(resp)) - httpreq = NewHTTPRequester(API(ts.URL)) - _, headers, code, err = httpreq.Get("/bad") + httpreq = NewHTTPRequester() + _, headers, code, err = httpreq.Get(ts.URL + "/bad") assert.Equal(t, errors.New("400 Bad Request"), err) assert.Equal(t, code, http.StatusBadRequest) } @@ -94,14 +94,14 @@ func TestGetObj(t *testing.T) { defer ts.Close() var httpreq Requester - httpreq = NewHTTPRequester(API(ts.URL)) + httpreq = NewHTTPRequester() r := resp{} - err := httpreq.GetObj("/good", &r) + err := httpreq.GetObj(ts.URL+"/good", &r) assert.Nil(t, err) assert.Equal(t, resp{Fld1: "Hello, client", Fld2: 123}, r) - httpreq = NewHTTPRequester(API(ts.URL)) - err = httpreq.GetObj("/bad", &r) + httpreq = NewHTTPRequester() + err = httpreq.GetObj(ts.URL+"/bad", &r) assert.NotNil(t, err) } @@ -124,16 +124,16 @@ func TestPost(t *testing.T) { defer ts.Close() b := body{"one", 1} var httpreq Requester - httpreq = NewHTTPRequester(API(ts.URL)) - resp, headers, code, err := httpreq.Post("/good", b) + httpreq = NewHTTPRequester() + resp, headers, code, err := httpreq.Post(ts.URL+"/good", b) assert.Nil(t, err) assert.NotEqual(t, headers.Get("Content-Type"), "") assert.Equal(t, "Hello, client\n", string(resp)) assert.Equal(t, code, http.StatusOK) - httpreq = NewHTTPRequester(API(ts.URL)) - _, _, code, err = httpreq.Post("/bad", nil) + httpreq = NewHTTPRequester() + _, _, code, err = httpreq.Post(ts.URL+"/bad", nil) assert.Equal(t, errors.New("400 Bad Request"), err) assert.Equal(t, code, http.StatusBadRequest) } @@ -158,29 +158,21 @@ func TestPostObj(t *testing.T) { defer ts.Close() var httpreq Requester - httpreq = NewHTTPRequester(API(ts.URL)) + httpreq = NewHTTPRequester() b := body{"one", 1} r := body{} - err := httpreq.PostObj("/good", b, &r) + err := httpreq.PostObj(ts.URL+"/good", b, &r) assert.Nil(t, err) assert.Equal(t, body{Fld1: "Hello, client", Fld2: 123}, r) - httpreq = NewHTTPRequester(API(ts.URL)) - err = httpreq.PostObj("/bad", b, &r) + httpreq = NewHTTPRequester() + err = httpreq.PostObj(ts.URL+"/bad", b, &r) assert.NotNil(t, err) } func TestGetBad(t *testing.T) { - ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - log.Print(">request: ", r) - if r.URL.String() == "/good" { - fmt.Fprintln(w, "Hello, client") - } - if r.URL.String() == "/bad" { - w.WriteHeader(http.StatusBadRequest) - } - })) - httpreq := NewHTTPRequester(API(ts.URL)) + + httpreq := NewHTTPRequester() _, _, _, err := httpreq.Get("blah12345/good") _, ok := err.(*url.Error) assert.True(t, ok, "url error") @@ -200,8 +192,8 @@ func TestGetBadWithResponse(t *testing.T) { })) defer ts.Close() - httpreq := NewHTTPRequester(API(ts.URL), Retries(1)) - data, _, _, err := httpreq.Get("/bad") + httpreq := NewHTTPRequester(Retries(1)) + data, _, _, err := httpreq.Get(ts.URL + "/bad") assert.Equal(t, "400 Bad Request", err.Error()) assert.Equal(t, "bad bad response\n", string(data)) } @@ -229,10 +221,10 @@ func TestGetRetry(t *testing.T) { })) defer ts.Close() - httpreq := NewHTTPRequester(API(ts.URL), Retries(10)) + httpreq := NewHTTPRequester(Retries(10)) st := time.Now() - resp, _, _, err := httpreq.Get("/test") + resp, _, _, err := httpreq.Get(ts.URL + "/test") assert.Nil(t, err) assert.Equal(t, "Hello, client\n", string(resp)) assert.Equal(t, 5, called, "called 5 retries") @@ -240,28 +232,28 @@ func TestGetRetry(t *testing.T) { assert.True(t, elapsed >= 400*5*time.Millisecond && elapsed <= 510*5*time.Second, "took %s", elapsed) - httpreq = NewHTTPRequester(API(ts.URL), Retries(3)) + httpreq = NewHTTPRequester(Retries(3)) called = 0 - _, _, _, err = httpreq.Get("/test") + _, _, _, err = httpreq.Get(ts.URL + "/test") assert.Equal(t, errors.New("400 Bad Request"), err) assert.Equal(t, 3, called, "called 3 retries") - httpreq = NewHTTPRequester(API(ts.URL), Retries(1)) + httpreq = NewHTTPRequester(Retries(1)) called = 0 - _, _, _, err = httpreq.Get("/test") + _, _, _, err = httpreq.Get(ts.URL + "/test") assert.Equal(t, errors.New("400 Bad Request"), err) assert.Equal(t, 1, called, "called 1 retries") - httpreq = NewHTTPRequester(API(ts.URL)) + httpreq = NewHTTPRequester() called = 0 - _, _, _, err = httpreq.Get("/test") + _, _, _, err = httpreq.Get(ts.URL + "/test") assert.Equal(t, errors.New("400 Bad Request"), err) assert.Equal(t, 1, called, "called 1 retries") } func TestString(t *testing.T) { - assert.Equal(t, "{api: https://cdn.optimizely.com/datafiles, timeout: 5s, retries: 1}", NewHTTPRequester().String()) - assert.Equal(t, "{api: 127.0.0.1/blah, timeout: 19s, retries: 10}", - NewHTTPRequester(API("127.0.0.1/blah"), Retries(10), Timeout(time.Duration(19)*time.Second)).String()) + assert.Equal(t, "{timeout: 5s, retries: 1}", NewHTTPRequester().String()) + assert.Equal(t, "{timeout: 19s, retries: 10}", + NewHTTPRequester(Retries(10), Timeout(time.Duration(19)*time.Second)).String()) } From 5bed9ebac44b7d5e2da4b79433a82d46ef67371f Mon Sep 17 00:00:00 2001 From: Pawel Szczodruch Date: Tue, 26 Nov 2019 11:30:23 -0800 Subject: [PATCH 4/4] address PR comments --- pkg/client/factory_test.go | 18 ++++++++++++++++-- pkg/config/polling_manager.go | 9 --------- pkg/config/polling_manager_test.go | 12 ------------ pkg/utils/requester.go | 1 - 4 files changed, 16 insertions(+), 24 deletions(-) diff --git a/pkg/client/factory_test.go b/pkg/client/factory_test.go index 9cacf966b..21e72775d 100644 --- a/pkg/client/factory_test.go +++ b/pkg/client/factory_test.go @@ -18,6 +18,7 @@ package client import ( "errors" + "net/http" "testing" "time" @@ -28,8 +29,19 @@ import ( "github.com/optimizely/go-sdk/pkg/utils" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" ) +type MockRequester struct { + utils.Requester + mock.Mock +} + +func (m *MockRequester) Get(uri string, headers ...utils.Header) (response []byte, responseHeaders http.Header, code int, err error) { + args := m.Called(headers) + return args.Get(0).([]byte), args.Get(1).(http.Header), args.Int(2), args.Error(3) +} + type MockDispatcher struct { Events []event.LogEvent } @@ -72,8 +84,10 @@ func TestClientWithPollingConfigManager(t *testing.T) { func TestClientWithPollingConfigManagerRequester(t *testing.T) { factory := OptimizelyFactory{} - requester := utils.NewHTTPRequester() - optimizelyClient, err := factory.Client(WithPollingConfigManagerRequester(requester, time.Minute, nil)) + mockRequester := new(MockRequester) + mockRequester.On("Get", []utils.Header(nil)).Return([]byte(`{"revision":"42"}`), http.Header{}, http.StatusOK, nil) + + optimizelyClient, err := factory.Client(WithPollingConfigManagerRequester(mockRequester, time.Minute, nil)) assert.NoError(t, err) assert.NotNil(t, optimizelyClient.ConfigManager) assert.NotNil(t, optimizelyClient.DecisionService) diff --git a/pkg/config/polling_manager.go b/pkg/config/polling_manager.go index 2568a4555..968a7303a 100644 --- a/pkg/config/polling_manager.go +++ b/pkg/config/polling_manager.go @@ -62,15 +62,6 @@ type PollingProjectConfigManager struct { // OptionFunc is a type to a proper func type OptionFunc func(*PollingProjectConfigManager) -// DefaultRequester is an optional function, sets default requester -func DefaultRequester() OptionFunc { - return func(p *PollingProjectConfigManager) { - - requester := utils.NewHTTPRequester() - p.requester = requester - } -} - // Requester is an optional function, sets a passed requester func Requester(requester utils.Requester) OptionFunc { return func(p *PollingProjectConfigManager) { diff --git a/pkg/config/polling_manager_test.go b/pkg/config/polling_manager_test.go index 3f2f450d8..cdd0f0765 100644 --- a/pkg/config/polling_manager_test.go +++ b/pkg/config/polling_manager_test.go @@ -234,18 +234,6 @@ func TestNewPollingProjectConfigManagerOnDecision(t *testing.T) { assert.Nil(t, err) } -func TestDefaultRequester(t *testing.T) { - - sdkKey := "test_sdk_key" - exeCtx := utils.NewCancelableExecutionCtx() - configManager := NewPollingProjectConfigManager(sdkKey, DefaultRequester()) - configManager.Start(sdkKey, exeCtx) - - requester := configManager.requester - assert.NotNil(t, requester) - assert.Equal(t, requester.String(), "{timeout: 5s, retries: 1}") -} - func TestPollingInterval(t *testing.T) { sdkKey := "test_sdk_key" diff --git a/pkg/utils/requester.go b/pkg/utils/requester.go index bc2fe85ce..036aac818 100644 --- a/pkg/utils/requester.go +++ b/pkg/utils/requester.go @@ -98,7 +98,6 @@ func NewHTTPRequester(params ...func(*HTTPRequester)) *HTTPRequester { } // Get executes HTTP GET with url and optional extra headers, returns body in []bytes -// url created as api+sdkKey.json func (r HTTPRequester) Get(url string, headers ...Header) (response []byte, responseHeaders http.Header, code int, err error) { return r.Do(url, "GET", nil, headers) }