refac(config-manager): simplifies the creation of polling config manager with options (#61)

Author: Michael Ng

optimizely/client/factory.go

@@ -28,8 +28,6 @@ import (
 	"github.com/optimizely/go-sdk/optimizely/decision"
 )
 
-const datafileURLTemplate = "https://cdn.optimizely.com/datafiles/%s.json"
-
 // Options are used to create an instance of the OptimizelyClient with custom configuration
 type Options struct {
 	Context              context.Context
@@ -47,7 +45,7 @@ func (f OptimizelyFactory) StaticClient() (*OptimizelyClient, error) {
 	var configManager optimizely.ProjectConfigManager
 
 	if f.SDKKey != "" {
-		url := fmt.Sprintf(datafileURLTemplate, f.SDKKey)
+		url := fmt.Sprintf(config.DatafileURLTemplate, f.SDKKey)
 		staticConfigManager, err := config.NewStaticProjectConfigManagerFromURL(url)
 
 		if err != nil {
@@ -95,9 +93,10 @@ func (f OptimizelyFactory) ClientWithOptions(clientOptions Options) (*Optimizely
 	if clientOptions.ProjectConfigManager != nil {
 		client.configManager = clientOptions.ProjectConfigManager
 	} else if f.SDKKey != "" {
-		url := fmt.Sprintf(datafileURLTemplate, f.SDKKey)
-		request := config.NewRequester(url)
-		client.configManager = config.NewPollingProjectConfigManager(ctx, request, f.Datafile, 0)
+		options := config.PollingProjectConfigManagerOptions{
+			Datafile: f.Datafile,
+		}
+		client.configManager = config.NewPollingProjectConfigManagerWithOptions(ctx, f.SDKKey, options)
 	} else if f.Datafile != nil {
 		staticConfigManager, _ := config.NewStaticProjectConfigManagerFromPayload(f.Datafile)
 		client.configManager = staticConfigManager

optimizely/config/polling_manager.go

@@ -27,16 +27,26 @@ import (
 	"github.com/optimizely/go-sdk/optimizely/logging"
 )
 
-const defaultPollingWait = time.Duration(5 * time.Minute) // default 5 minutes for polling wait
+const defaultPollingInterval = time.Duration(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"
 
 var cmLogger = logging.GetLogger("PollingConfigManager")
 
+// PollingProjectConfigManagerOptions used to create an instance with custom configuration
+type PollingProjectConfigManagerOptions struct {
+	Datafile        []byte
+	PollingInterval time.Duration
+	Requester       Requester
+}
+
 // PollingProjectConfigManager maintains a dynamic copy of the project config
 type PollingProjectConfigManager struct {
-	requester     *Requester
-	pollingWait   time.Duration
-	projectConfig optimizely.ProjectConfig
-	configLock    sync.RWMutex
+	requester       Requester
+	pollingInterval time.Duration
+	projectConfig   optimizely.ProjectConfig
+	configLock      sync.RWMutex
 
 	ctx context.Context // context used for cancellation
 }
@@ -71,7 +81,7 @@ func (cm *PollingProjectConfigManager) activate(initialPayload []byte, init bool
 		update()
 		return
 	}
-	t := time.NewTicker(cm.pollingWait)
+	t := time.NewTicker(cm.pollingInterval)
 	for {
 		select {
 		case <-t.C:
@@ -83,22 +93,38 @@ func (cm *PollingProjectConfigManager) activate(initialPayload []byte, init bool
 	}
 }
 
-// NewPollingProjectConfigManager returns new instance of PollingProjectConfigManager
-func NewPollingProjectConfigManager(ctx context.Context, requester *Requester, initialPayload []byte, pollingWait time.Duration) *PollingProjectConfigManager {
+// NewPollingProjectConfigManagerWithOptions returns new instance of PollingProjectConfigManager with the given options
+func NewPollingProjectConfigManagerWithOptions(ctx context.Context, sdkKey string, options PollingProjectConfigManagerOptions) *PollingProjectConfigManager {
 
-	if pollingWait == 0 {
-		pollingWait = defaultPollingWait
+	var requester Requester
+	if options.Requester != nil {
+		requester = options.Requester
+	} else {
+		url := fmt.Sprintf(DatafileURLTemplate, sdkKey)
+		requester = NewHTTPRequester(url)
 	}
 
-	pollingProjectConfigManager := PollingProjectConfigManager{requester: requester, pollingWait: pollingWait, ctx: ctx}
+	pollingInterval := options.PollingInterval
+	if pollingInterval == 0 {
+		pollingInterval = defaultPollingInterval
+	}
 
-	pollingProjectConfigManager.activate(initialPayload, true) // initial poll
+	pollingProjectConfigManager := PollingProjectConfigManager{requester: requester, pollingInterval: pollingInterval, ctx: ctx}
+
+	pollingProjectConfigManager.activate(options.Datafile, true) // initial poll
 
 	cmLogger.Debug("Polling Config Manager Initiated")
 	go pollingProjectConfigManager.activate([]byte{}, false)
 	return &pollingProjectConfigManager
 }
 
+// NewPollingProjectConfigManager returns an instance of the polling config manager with the default configuration
+func NewPollingProjectConfigManager(ctx context.Context, sdkKey string) *PollingProjectConfigManager {
+	options := PollingProjectConfigManagerOptions{}
+	configManager := NewPollingProjectConfigManagerWithOptions(ctx, sdkKey, options)
+	return configManager
+}
+
 // GetConfig returns the project config
 func (cm *PollingProjectConfigManager) GetConfig() optimizely.ProjectConfig {
 	cm.configLock.RLock()

optimizely/config/polling_manager_test.go

@@ -20,25 +20,34 @@ import (
 	"context"
 	"testing"
 
-	"github.com/optimizely/go-sdk/optimizely/config/datafileprojectconfig"
 	"github.com/stretchr/testify/assert"
+
+	"github.com/optimizely/go-sdk/optimizely/config/datafileprojectconfig"
+	"github.com/stretchr/testify/mock"
 )
 
-func TestNewPollingProjectConfigManager(t *testing.T) {
-	URL := "https://cdn.optimizely.com/datafiles/4SLpaJA1r1pgE6T2CoMs9q_bad.json"
-	projectConfig, _ := datafileprojectconfig.NewDatafileProjectConfig([]byte{})
-	request := NewRequester(URL)
+type MockRequester struct {
+	Requester
+	mock.Mock
+}
 
-	// Bad SDK Key test
-	configManager := NewPollingProjectConfigManager(context.Background(), request, []byte{}, 0)
-	assert.Equal(t, projectConfig, configManager.GetConfig())
+func (m *MockRequester) Get(headers ...Header) (response []byte, code int, err error) {
+	args := m.Called(headers)
+	return args.Get(0).([]byte), args.Int(1), args.Error(2)
+}
 
-	// Good SDK Key test
-	URL = "https://cdn.optimizely.com/datafiles/4SLpaJA1r1pgE6T2CoMs9q.json"
-	request = NewRequester(URL)
-	configManager = NewPollingProjectConfigManager(context.Background(), request, []byte{}, 0)
-	newConfig := configManager.GetConfig()
+func TestNewPollingProjectConfigManagerWithOptions(t *testing.T) {
+	mockDatafile := []byte("{ revision: \"42\" }")
+	projectConfig, _ := datafileprojectconfig.NewDatafileProjectConfig(mockDatafile)
+	mockRequester := new(MockRequester)
+	mockRequester.On("Get", []Header(nil)).Return(mockDatafile, 200, nil)
 
-	assert.Equal(t, "", newConfig.GetAccountID())
-	assert.Equal(t, 4, len(newConfig.GetAudienceMap()))
+	// Test we fetch using requester
+	sdkKey := "test_sdk_key"
+	options := PollingProjectConfigManagerOptions{
+		Requester: mockRequester,
+	}
+	configManager := NewPollingProjectConfigManagerWithOptions(context.Background(), sdkKey, options)
+	mockRequester.AssertExpectations(t)
+	assert.Equal(t, projectConfig, configManager.GetConfig())
 }

optimizely/config/requester.go

@@ -32,47 +32,53 @@ const defaultTTL = 5 * time.Second
 
 var requesterLogger = logging.GetLogger("Requester")
 
+// Requester is used to make outbound requests with
+type Requester interface {
+	Get(...Header) (response []byte, code int, err error)
+	GetObj(result interface{}, headers ...Header) error
+}
+
 // Header element to be sent
 type Header struct {
 	Name, Value string
 }
 
 // Timeout sets http client timeout
-func Timeout(timeout time.Duration) func(r *Requester) {
-	return func(r *Requester) {
+func Timeout(timeout time.Duration) func(r *HTTPRequester) {
+	return func(r *HTTPRequester) {
 		r.client = http.Client{Timeout: timeout}
 	}
 }
 
 // Retries sets max number of retries for failed calls
-func Retries(retries int) func(r *Requester) {
-	return func(r *Requester) {
+func Retries(retries int) func(r *HTTPRequester) {
+	return func(r *HTTPRequester) {
 		r.retries = retries
 	}
 }
 
 // Headers sets request headers
-func Headers(headers ...Header) func(r *Requester) {
-	return func(r *Requester) {
+func Headers(headers ...Header) func(r *HTTPRequester) {
+	return func(r *HTTPRequester) {
 		r.headers = []Header{}
 		r.headers = append(r.headers, headers...)
 	}
 }
 
-// Requester contains main info
-type Requester struct {
+// HTTPRequester contains main info
+type HTTPRequester struct {
 	url     string
 	client  http.Client
 	retries int
 	headers []Header
 	ttl     time.Duration // time-to-live
 }
 
-// NewRequester makes Requester with api and parameters. Sets defaults
+// 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 NewRequester(url string, params ...func(*Requester)) *Requester {
+func NewHTTPRequester(url string, params ...func(*HTTPRequester)) *HTTPRequester {
 
-	res := Requester{
+	res := HTTPRequester{
 		url:     url,
 		retries: 1,
 		headers: []Header{{"Content-Type", "application/json"}, {"Accept", "application/json"}},
@@ -87,12 +93,12 @@ func NewRequester(url string, params ...func(*Requester)) *Requester {
 
 // Get executes HTTP GET with uri and optional extra headers, returns body in []bytes
 // url created as url+sdkKey.json
-func (r Requester) Get(headers ...Header) (response []byte, code int, err error) {
+func (r HTTPRequester) Get(headers ...Header) (response []byte, code int, err error) {
 	return r.Do("GET", headers)
 }
 
 // GetObj executes HTTP GET with uri and optional extra headers, returns filled object
-func (r Requester) GetObj(result interface{}, headers ...Header) error {
+func (r HTTPRequester) GetObj(result interface{}, headers ...Header) error {
 	b, _, err := r.Do("GET", headers)
 	if err != nil {
 		return err
@@ -101,7 +107,7 @@ func (r Requester) GetObj(result interface{}, headers ...Header) error {
 }
 
 // Do executes request and returns response body for requested uri (sdkKey.json).
-func (r Requester) Do(method string, headers []Header) (response []byte, code int, err error) {
+func (r HTTPRequester) Do(method string, headers []Header) (response []byte, code int, err error) {
 
 	single := func(request *http.Request) (response []byte, code int, e error) {
 		resp, doErr := r.client.Do(request)
@@ -159,7 +165,7 @@ func (r Requester) Do(method string, headers []Header) (response []byte, code in
 	return response, code, err
 }
 
-func (r Requester) addHeaders(req *http.Request, headers []Header) *http.Request {
+func (r HTTPRequester) addHeaders(req *http.Request, headers []Header) *http.Request {
 	for _, h := range r.headers {
 		req.Header.Add(h.Name, h.Value)
 	}
@@ -169,7 +175,7 @@ func (r Requester) addHeaders(req *http.Request, headers []Header) *http.Request
 	return req
 }
 
-func (r Requester) String() string {
+func (r HTTPRequester) String() string {
 	return fmt.Sprintf("{url: %s, timeout: %v, retries: %d}",
 		r.url, r.client.Timeout, r.retries)
 }

optimizely/config/requester_test.go

@@ -42,13 +42,13 @@ func TestGet(t *testing.T) {
 	}))
 	defer ts.Close()
 
-	var httpreq *Requester
-	httpreq = NewRequester(ts.URL + "/good")
+	var httpreq Requester
+	httpreq = NewHTTPRequester(ts.URL + "/good")
 	resp, code, err := httpreq.Get()
 	assert.Nil(t, err)
 	assert.Equal(t, "Hello, client\n", string(resp))
 
-	httpreq = NewRequester(ts.URL + "/bad")
+	httpreq = NewHTTPRequester(ts.URL + "/bad")
 	_, code, err = httpreq.Get()
 	assert.Equal(t, errors.New("400 Bad Request"), err)
 	assert.Equal(t, code, 400)
@@ -72,16 +72,16 @@ func TestGetObj(t *testing.T) {
 	}))
 	defer ts.Close()
 
-	var httpreq *Requester
-	httpreq = NewRequester(ts.URL + "/good")
+	var httpreq Requester
+	httpreq = NewHTTPRequester(ts.URL + "/good")
 	r := resp{}
 	err := httpreq.GetObj(&r)
 	assert.Nil(t, err)
 	assert.Equal(t, resp{Fld1: "Hello, client", Fld2: 123}, r)
 }
 
 func TestGetBad(t *testing.T) {
-	httpreq := NewRequester("blah12345/good")
+	httpreq := NewHTTPRequester("blah12345/good")
 	_, _, err := httpreq.Get()
 	_, ok := err.(*url.Error)
 	assert.True(t, ok, "url error")
@@ -101,7 +101,7 @@ func TestGetBadWithResponse(t *testing.T) {
 	}))
 	defer ts.Close()
 
-	httpreq := NewRequester(ts.URL+"/bad", Retries(1))
+	httpreq := NewHTTPRequester(ts.URL+"/bad", Retries(1))
 	data, _, err := httpreq.Get()
 	assert.Equal(t, "400 Bad Request", err.Error())
 	assert.Equal(t, "bad bad response\n", string(data))
@@ -130,7 +130,7 @@ func TestGetRetry(t *testing.T) {
 	}))
 	defer ts.Close()
 
-	httpreq := NewRequester(ts.URL+"/test", Retries(10))
+	httpreq := NewHTTPRequester(ts.URL+"/test", Retries(10))
 
 	st := time.Now()
 	resp, _, err := httpreq.Get()
@@ -141,27 +141,27 @@ func TestGetRetry(t *testing.T) {
 
 	assert.True(t, elapsed >= 400*5*time.Millisecond && elapsed <= 510*5*time.Second, "took %s", elapsed)
 
-	httpreq = NewRequester(ts.URL+"/test", Retries(3))
+	httpreq = NewHTTPRequester(ts.URL+"/test", Retries(3))
 	called = 0
 	_, _, err = httpreq.Get()
 	assert.Equal(t, errors.New("400 Bad Request"), err)
 	assert.Equal(t, 3, called, "called 3 retries")
 
-	httpreq = NewRequester(ts.URL+"/test", Retries(1))
+	httpreq = NewHTTPRequester(ts.URL+"/test", Retries(1))
 	called = 0
 	_, _, err = httpreq.Get()
 	assert.Equal(t, errors.New("400 Bad Request"), err)
 	assert.Equal(t, 1, called, "called 1 retries")
 
-	httpreq = NewRequester(ts.URL + "/test")
+	httpreq = NewHTTPRequester(ts.URL + "/test")
 	called = 0
 	_, _, err = httpreq.Get()
 	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}", NewRequester("127.0.0.1/blah").String())
+	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}",
-		NewRequester("127.0.0.1/blah", Retries(10), Timeout(time.Duration(19)*time.Second)).String())
+		NewHTTPRequester("127.0.0.1/blah", Retries(10), Timeout(time.Duration(19)*time.Second)).String())
 }