feat(PollingConfigManager): Implemented caching headers in PollingConfigManager. (#189)

yasirfolio3 · Michael Ng · commit cd323d45bda0 · 2019-11-19T11:49:08.000-08:00
diff --git a/pkg/config/polling_manager.go b/pkg/config/polling_manager.go
@@ -19,6 +19,7 @@ package config
 
 import (
 	"fmt"
+	"net/http"
 	"sync"
 	"time"
 
@@ -36,6 +37,12 @@ const DefaultPollingInterval = 5 * time.Minute // default to 5 minutes for polli
 // 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"
+
+// LastModified header key for response
+const LastModified = "Last-Modified"
+
 var cmLogger = logging.GetLogger("PollingConfigManager")
 
 // PollingProjectConfigManager maintains a dynamic copy of the project config
@@ -44,6 +51,7 @@ type PollingProjectConfigManager struct {
 	pollingInterval    time.Duration
 	notificationCenter notification.Center
 	initDatafile       []byte
+	lastModified       string
 
 	configLock    sync.RWMutex
 	err           error
@@ -89,21 +97,40 @@ func InitialDatafile(datafile []byte) OptionFunc {
 func (cm *PollingProjectConfigManager) SyncConfig(datafile []byte) {
 	var e error
 	var code int
+	var respHeaders http.Header
 
 	closeMutex := func(e error) {
 		cm.err = e
 		cm.configLock.Unlock()
 	}
 
 	if len(datafile) == 0 {
-		datafile, code, e = cm.requester.Get()
+		if cm.lastModified != "" {
+			lastModifiedHeader := utils.Header{Name: ModifiedSince, Value: cm.lastModified}
+			datafile, respHeaders, code, e = cm.requester.Get(lastModifiedHeader)
+		} else {
+			datafile, respHeaders, code, e = cm.requester.Get()
+		}
 
 		if e != nil {
 			cmLogger.Error(fmt.Sprintf("request returned with http code=%d", code), e)
 			cm.configLock.Lock()
 			closeMutex(e)
 			return
 		}
+
+		if code == http.StatusNotModified {
+			cmLogger.Debug("The datafile was not modified and won't be downloaded again")
+			return
+		}
+
+		// Save last-modified date from response header
+		lastModified := respHeaders.Get(LastModified)
+		if lastModified != "" {
+			cm.configLock.Lock()
+			cm.lastModified = lastModified
+			cm.configLock.Unlock()
+		}
 	}
 
 	projectConfig, err := datafileprojectconfig.NewDatafileProjectConfig(datafile)
diff --git a/pkg/config/polling_manager_test.go b/pkg/config/polling_manager_test.go
@@ -17,6 +17,7 @@
 package config
 
 import (
+	"net/http"
 	"testing"
 	"time"
 
@@ -33,17 +34,17 @@ type MockRequester struct {
 	mock.Mock
 }
 
-func (m *MockRequester) Get(headers ...utils.Header) (response []byte, code int, err error) {
+func (m *MockRequester) Get(headers ...utils.Header) (response []byte, responseHeaders http.Header, code int, err error) {
 	args := m.Called(headers)
-	return args.Get(0).([]byte), args.Int(1), args.Error(2)
+	return args.Get(0).([]byte), args.Get(1).(http.Header), args.Int(2), args.Error(3)
 }
 
 func TestNewPollingProjectConfigManagerWithOptions(t *testing.T) {
 
 	mockDatafile := []byte(`{"revision":"42"}`)
 	projectConfig, _ := datafileprojectconfig.NewDatafileProjectConfig(mockDatafile)
 	mockRequester := new(MockRequester)
-	mockRequester.On("Get", []utils.Header(nil)).Return(mockDatafile, 200, nil)
+	mockRequester.On("Get", []utils.Header(nil)).Return(mockDatafile, http.Header{}, http.StatusOK, nil)
 
 	// Test we fetch using requester
 	sdkKey := "test_sdk_key"
@@ -57,13 +58,14 @@ func TestNewPollingProjectConfigManagerWithOptions(t *testing.T) {
 	assert.Nil(t, err)
 	assert.NotNil(t, actual)
 	assert.Equal(t, projectConfig, actual)
+
 	exeCtx.TerminateAndWait() // just sending signal and improving coverage
 }
 
 func TestNewPollingProjectConfigManagerWithNull(t *testing.T) {
 	mockDatafile := []byte("NOT-VALID")
 	mockRequester := new(MockRequester)
-	mockRequester.On("Get", []utils.Header(nil)).Return(mockDatafile, 200, nil)
+	mockRequester.On("Get", []utils.Header(nil)).Return(mockDatafile, http.Header{}, http.StatusOK, nil)
 
 	// Test we fetch using requester
 	sdkKey := "test_sdk_key"
@@ -82,7 +84,7 @@ func TestNewPollingProjectConfigManagerWithSimilarDatafileRevisions(t *testing.T
 	mockDatafile2 := []byte(`{"revision":"42","botFiltering":false}`)
 	projectConfig1, _ := datafileprojectconfig.NewDatafileProjectConfig(mockDatafile1)
 	mockRequester := new(MockRequester)
-	mockRequester.On("Get", []utils.Header(nil)).Return(mockDatafile1, 200, nil)
+	mockRequester.On("Get", []utils.Header(nil)).Return(mockDatafile1, http.Header{}, http.StatusOK, nil)
 
 	sdkKey := "test_sdk_key"
 
@@ -101,13 +103,45 @@ func TestNewPollingProjectConfigManagerWithSimilarDatafileRevisions(t *testing.T
 	assert.Equal(t, projectConfig1, actual)
 }
 
+func TestNewPollingProjectConfigManagerWithLastModifiedDates(t *testing.T) {
+	mockDatafile1 := []byte(`{"revision":"42","botFiltering":true}`)
+	projectConfig1, _ := datafileprojectconfig.NewDatafileProjectConfig(mockDatafile1)
+	mockRequester := new(MockRequester)
+	modifiedDate := "Wed, 16 Oct 2019 20:16:45 GMT"
+	responseHeaders := http.Header{}
+	responseHeaders.Set(LastModified, modifiedDate)
+
+	mockRequester.On("Get", []utils.Header(nil)).Return(mockDatafile1, responseHeaders, http.StatusOK, nil)
+	mockRequester.On("Get", []utils.Header{utils.Header{Name: ModifiedSince, Value: modifiedDate}}).Return([]byte{}, responseHeaders, http.StatusNotModified, nil)
+
+	sdkKey := "test_sdk_key"
+
+	exeCtx := utils.NewCancelableExecutionCtx()
+	configManager := NewPollingProjectConfigManager(sdkKey, Requester(mockRequester))
+	configManager.Start(exeCtx)
+
+	// Fetch valid config
+	actual, err := configManager.GetConfig()
+	assert.Nil(t, err)
+	assert.NotNil(t, actual)
+	assert.Equal(t, projectConfig1, actual)
+
+	// Sync and check no changes were made to the previous config because of 304 error code
+	configManager.SyncConfig([]byte{})
+	actual, err = configManager.GetConfig()
+	assert.Nil(t, err)
+	assert.NotNil(t, actual)
+	assert.Equal(t, projectConfig1, actual)
+	mockRequester.AssertExpectations(t)
+}
+
 func TestNewPollingProjectConfigManagerWithDifferentDatafileRevisions(t *testing.T) {
 	mockDatafile1 := []byte(`{"revision":"42","botFiltering":true}`)
 	mockDatafile2 := []byte(`{"revision":"43","botFiltering":false}`)
 	projectConfig1, _ := datafileprojectconfig.NewDatafileProjectConfig(mockDatafile1)
 	projectConfig2, _ := datafileprojectconfig.NewDatafileProjectConfig(mockDatafile2)
 	mockRequester := new(MockRequester)
-	mockRequester.On("Get", []utils.Header(nil)).Return(mockDatafile1, 200, nil)
+	mockRequester.On("Get", []utils.Header(nil)).Return(mockDatafile1, http.Header{}, http.StatusOK, nil)
 
 	// Test we fetch using requester
 	sdkKey := "test_sdk_key"
@@ -134,7 +168,7 @@ func TestNewPollingProjectConfigManagerWithErrorHandling(t *testing.T) {
 	projectConfig1, _ := datafileprojectconfig.NewDatafileProjectConfig(mockDatafile1)
 	projectConfig2, _ := datafileprojectconfig.NewDatafileProjectConfig(mockDatafile2)
 	mockRequester := new(MockRequester)
-	mockRequester.On("Get", []utils.Header(nil)).Return(mockDatafile1, 200, nil)
+	mockRequester.On("Get", []utils.Header(nil)).Return(mockDatafile1, http.Header{}, http.StatusOK, nil)
 
 	// Test we fetch using requester
 	sdkKey := "test_sdk_key"
@@ -165,7 +199,7 @@ func TestNewPollingProjectConfigManagerOnDecision(t *testing.T) {
 	mockDatafile2 := []byte(`{"revision":"43","botFiltering":false}`)
 
 	mockRequester := new(MockRequester)
-	mockRequester.On("Get", []utils.Header(nil)).Return(mockDatafile1, 200, nil)
+	mockRequester.On("Get", []utils.Header(nil)).Return(mockDatafile1, http.Header{}, http.StatusOK, nil)
 
 	// Test we fetch using requester
 	sdkKey := "test_sdk_key"
diff --git a/pkg/config/static_manager.go b/pkg/config/static_manager.go
@@ -38,7 +38,7 @@ type StaticProjectConfigManager struct {
 func NewStaticProjectConfigManagerFromURL(sdkKey string) (*StaticProjectConfigManager, error) {
 
 	requester := utils.NewHTTPRequester(fmt.Sprintf(DatafileURLTemplate, sdkKey))
-	datafile, code, e := requester.Get()
+	datafile, _, code, e := requester.Get()
 	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
@@ -20,6 +20,7 @@ package event
 import (
 	"context"
 	"fmt"
+	"net/http"
 	"sync"
 	"time"
 
@@ -46,7 +47,7 @@ type HTTPEventDispatcher struct {
 func (*HTTPEventDispatcher) DispatchEvent(event LogEvent) (bool, error) {
 
 	requester := utils.NewHTTPRequester(event.EndPoint)
-	_, code, err := requester.Post(event.Event)
+	_, _, code, err := requester.Post(event.Event)
 
 	// also check response codes
 	// resp.StatusCode == 400 is an error
@@ -55,7 +56,7 @@ func (*HTTPEventDispatcher) DispatchEvent(event LogEvent) (bool, error) {
 		dispatcherLogger.Error("http.Post failed:", err)
 		success = false
 	} else {
-		if code == 204 {
+		if code == http.StatusNoContent {
 			success = true
 		} else {
 			dispatcherLogger.Error(fmt.Sprintf("http.Post invalid response %d", code), nil)
diff --git a/pkg/utils/requester.go b/pkg/utils/requester.go
@@ -38,10 +38,10 @@ var json = jsoniter.ConfigCompatibleWithStandardLibrary
 
 // Requester is used to make outbound requests with
 type Requester interface {
-	Get(...Header) (response []byte, code int, err error)
+	Get(...Header) (response []byte, responseHeaders http.Header, code int, err error)
 	GetObj(result interface{}, headers ...Header) error
 
-	Post(body interface{}, headers ...Header) (response []byte, code int, err error)
+	Post(body interface{}, headers ...Header) (response []byte, responseHeaders http.Header, code int, err error)
 	PostObj(body interface{}, result interface{}, headers ...Header) error
 
 	String() string
@@ -101,45 +101,45 @@ 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, code int, err error) {
+func (r HTTPRequester) Get(headers ...Header) (response []byte, responseHeaders http.Header, code int, err error) {
 	return r.Do("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)
+	b, _, _, err := r.Do("GET", nil, headers)
 	if err != nil {
 		return err
 	}
 	return json.Unmarshal(b, result)
 }
 
 // Post executes HTTP POST with url, body and optional extra headers
-func (r HTTPRequester) Post(body interface{}, headers ...Header) (response []byte, code int, err error) {
+func (r HTTPRequester) Post(body interface{}, headers ...Header) (response []byte, responseHeaders http.Header, code int, err error) {
 	b, err := json.Marshal(body)
 	if err != nil {
-		return nil, 400, err
+		return nil, nil, http.StatusBadRequest, err
 	}
 	return r.Do("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...)
+	b, _, _, err := r.Post(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(method string, body io.Reader, headers []Header) (response []byte, code int, err error) {
+func (r HTTPRequester) Do(method string, body io.Reader, headers []Header) (response []byte, responseHeaders http.Header, code int, err error) {
 
-	single := func(request *http.Request) (response []byte, code int, e error) {
+	single := func(request *http.Request) (response []byte, responseHeaders http.Header, code int, e error) {
 		resp, doErr := r.client.Do(request)
 		if doErr != nil {
 			requesterLogger.Error(fmt.Sprintf("failed to send request %v", request), e)
-			return nil, 0, doErr
+			return nil, http.Header{}, 0, doErr
 		}
 		defer func() {
 			if e := resp.Body.Close(); e != nil {
@@ -149,35 +149,35 @@ func (r HTTPRequester) Do(method string, body io.Reader, headers []Header) (resp
 
 		if response, err = ioutil.ReadAll(resp.Body); err != nil {
 			requesterLogger.Error("failed to read body", err)
-			return nil, resp.StatusCode, err
+			return nil, resp.Header, resp.StatusCode, err
 		}
 
-		if resp.StatusCode >= 400 {
+		if resp.StatusCode >= http.StatusBadRequest {
 			requesterLogger.Warning(fmt.Sprintf("error status code=%d", resp.StatusCode))
-			return response, resp.StatusCode, errors.New(resp.Status)
+			return response, resp.Header, resp.StatusCode, errors.New(resp.Status)
 		}
 
-		return response, resp.StatusCode, nil
+		return response, resp.Header, resp.StatusCode, nil
 	}
 
 	requesterLogger.Debug(fmt.Sprintf("request %s", r.url))
 	req, err := http.NewRequest(method, r.url, body)
 	if err != nil {
 		requesterLogger.Error(fmt.Sprintf("failed to make request %s", r.url), err)
-		return nil, 0, err
+		return nil, nil, 0, err
 	}
 
 	r.addHeaders(req, headers)
 
 	for i := 0; i < r.retries; i++ {
 
-		if response, code, err = single(req); err == nil {
+		if response, responseHeaders, code, err = single(req); err == nil {
 			triedMsg := ""
 			if i > 0 {
 				triedMsg = fmt.Sprintf(", tried %d time(s)", i+1)
 			}
 			requesterLogger.Debug(fmt.Sprintf("completed %s%s", r.url, triedMsg))
-			return response, code, err
+			return response, responseHeaders, code, err
 		}
 		requesterLogger.Debug(fmt.Sprintf("failed %s with %v", r.url, err))
 
@@ -187,7 +187,7 @@ func (r HTTPRequester) Do(method string, body io.Reader, headers []Header) (resp
 		}
 	}
 
-	return response, code, err
+	return response, responseHeaders, code, err
 }
 
 func (r HTTPRequester) addHeaders(req *http.Request, headers []Header) *http.Request {
diff --git a/pkg/utils/requester_test.go b/pkg/utils/requester_test.go
