Skip to content

feat: add valid datafile version check #234

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jan 23, 2020
Merged

feat: add valid datafile version check #234

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a321de6
feat: add valid datafile version check
pawels-optimizely Jan 23, 2020
fd5f4fc
some nits
pawels-optimizely Jan 23, 2020
0b1b5c5
addressing PR comments
pawels-optimizely Jan 23, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pkg/client/factory_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ func TestClientWithCustomCtx(t *testing.T) {
}

func TestStaticClient(t *testing.T) {
factory := OptimizelyFactory{Datafile: []byte(`{"revision": "42"}`)}
factory := OptimizelyFactory{Datafile: []byte(`{"revision": "42", "version": "4"}`)}
optlyClient, err := factory.StaticClient()
assert.NoError(t, err)

Expand Down
13 changes: 12 additions & 1 deletion pkg/config/datafileprojectconfig/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
package datafileprojectconfig

import (
"errors"
"fmt"

"github.com/optimizely/go-sdk/pkg/config/datafileprojectconfig/mappers"
Expand All @@ -27,6 +28,10 @@ import (

var logger = logging.GetLogger("DatafileProjectConfig")

var datafileVersions = map[string]struct{}{
"4": {},
}

// DatafileProjectConfig is a project config backed by a datafile
type DatafileProjectConfig struct {
accountID string
Expand Down Expand Up @@ -173,7 +178,13 @@ func (c DatafileProjectConfig) GetGroupByID(groupID string) (entities.Group, err
func NewDatafileProjectConfig(jsonDatafile []byte) (*DatafileProjectConfig, error) {
datafile, err := Parse(jsonDatafile)
if err != nil {
logger.Error("Error parsing datafile.", err)
logger.Error("Error parsing datafile", err)
return nil, err
}

if _, ok := datafileVersions[datafile.Version]; !ok {
err = errors.New("unsupported datafile version")
logger.Error(fmt.Sprintf("Version %s of datafile not supported", datafile.Version), err)
Copy link
Contributor

@aliabbasrizvi aliabbasrizvi Jan 23, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will be useful to say unable to initialize/update or something along those lines.

return nil, err
}

Expand Down
9 changes: 7 additions & 2 deletions pkg/config/datafileprojectconfig/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,18 @@ import (

func TestNewDatafileProjectConfigNil(t *testing.T) {
projectConfig, err := NewDatafileProjectConfig(nil)
assert.NotNil(t, err)
assert.Error(t, err)
assert.Nil(t, projectConfig)

jsonDatafileStr := `{"accountID": "123", "revision": "1", "projectId": "12345", "version": "2"}`
projectConfig, err = NewDatafileProjectConfig([]byte(jsonDatafileStr))
assert.Error(t, err)
assert.Nil(t, projectConfig)
}

func TestNewDatafileProjectConfigNotNil(t *testing.T) {
dpc := DatafileProjectConfig{accountID: "123", revision: "1", projectID: "12345"}
jsonDatafileStr := `{"accountID": "123", "revision": "1", "projectId": "12345"}`
jsonDatafileStr := `{"accountID": "123", "revision": "1", "projectId": "12345", "version": "4"}`
jsonDatafile := []byte(jsonDatafileStr)
projectConfig, err := NewDatafileProjectConfig(jsonDatafile)
assert.Nil(t, err)
Expand Down
58 changes: 29 additions & 29 deletions pkg/config/polling_manager_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func assertPeriodically(t *testing.T, evaluationMethod func() bool) {
func TestNewPollingProjectConfigManagerWithOptions(t *testing.T) {

invalidDatafile := []byte(`INVALID`)
mockDatafile := []byte(`{"revision":"42"}`)
mockDatafile := []byte(`{"revision":"42","version": "4"}`)

mockRequester := new(MockRequester)
mockRequester.On("Get", []utils.Header(nil)).Return(invalidDatafile, http.Header{}, http.StatusOK, nil).Times(1)
Expand All @@ -80,7 +80,7 @@ func TestNewPollingProjectConfigManagerWithOptions(t *testing.T) {

func TestNewAsyncPollingProjectConfigManagerWithOptions(t *testing.T) {

mockDatafile := []byte(`{"revision":"42"}`)
mockDatafile := []byte(`{"revision":"42", "version": "4"}`)
mockRequester := new(MockRequester)
mockRequester.On("Get", []utils.Header(nil)).Return(mockDatafile, http.Header{}, http.StatusOK, nil)

Expand All @@ -102,7 +102,7 @@ func TestNewAsyncPollingProjectConfigManagerWithOptions(t *testing.T) {

func TestSyncConfigFetchesDatafileUsingRequester(t *testing.T) {

mockDatafile := []byte(`{"revision":"42"}`)
mockDatafile := []byte(`{"revision":"42","version": "4"}`)
projectConfig, _ := datafileprojectconfig.NewDatafileProjectConfig(mockDatafile)
mockRequester := new(MockRequester)
mockRequester.On("Get", []utils.Header(nil)).Return(mockDatafile, http.Header{}, http.StatusOK, nil)
Expand Down Expand Up @@ -148,8 +148,8 @@ func TestNewAsyncPollingProjectConfigManagerWithNullDatafile(t *testing.T) {

func TestNewPollingProjectConfigManagerWithSimilarDatafileRevisions(t *testing.T) {
// Test newer datafile should not replace the older one if revisions are the same
mockDatafile1 := []byte(`{"revision":"42","botFiltering":true}`)
mockDatafile2 := []byte(`{"revision":"42","botFiltering":false}`)
mockDatafile1 := []byte(`{"revision":"42","botFiltering":true,"version": "4"}`)
mockDatafile2 := []byte(`{"revision":"42","botFiltering":false,"version": "4"}`)
projectConfig1, _ := datafileprojectconfig.NewDatafileProjectConfig(mockDatafile1)
mockRequester := new(MockRequester)
mockRequester.On("Get", []utils.Header(nil)).Return(mockDatafile2, http.Header{}, http.StatusOK, nil)
Expand Down Expand Up @@ -183,8 +183,8 @@ func TestNewPollingProjectConfigManagerWithSimilarDatafileRevisions(t *testing.T

func TestNewAsyncPollingProjectConfigManagerWithSimilarDatafileRevisions(t *testing.T) {
// Test newer datafile should not replace the older one if revisions are the same
mockDatafile1 := []byte(`{"revision":"42","botFiltering":true}`)
mockDatafile2 := []byte(`{"revision":"42","botFiltering":false}`)
mockDatafile1 := []byte(`{"revision":"42","botFiltering":true,"version": "4"}`)
mockDatafile2 := []byte(`{"revision":"42","botFiltering":false,"version": "4"}`)
projectConfig1, _ := datafileprojectconfig.NewDatafileProjectConfig(mockDatafile1)
mockRequester := new(MockRequester)
mockRequester.On("Get", []utils.Header(nil)).Return(mockDatafile2, http.Header{}, http.StatusOK, nil)
Expand Down Expand Up @@ -217,7 +217,7 @@ func TestNewAsyncPollingProjectConfigManagerWithSimilarDatafileRevisions(t *test
}

func TestNewPollingProjectConfigManagerWithLastModifiedDates(t *testing.T) {
mockDatafile1 := []byte(`{"revision":"42","botFiltering":true}`)
mockDatafile1 := []byte(`{"revision":"42","botFiltering":true,"version": "4"}`)
projectConfig1, _ := datafileprojectconfig.NewDatafileProjectConfig(mockDatafile1)
mockRequester := new(MockRequester)
modifiedDate := "Wed, 16 Oct 2019 20:16:45 GMT"
Expand All @@ -244,7 +244,7 @@ func TestNewPollingProjectConfigManagerWithLastModifiedDates(t *testing.T) {
}

func TestNewAsyncPollingProjectConfigManagerWithLastModifiedDates(t *testing.T) {
mockDatafile1 := []byte(`{"revision":"42","botFiltering":true}`)
mockDatafile1 := []byte(`{"revision":"42","botFiltering":true,"version": "4"}`)
mockRequester := new(MockRequester)
modifiedDate := "Wed, 16 Oct 2019 20:16:45 GMT"
responseHeaders := http.Header{}
Expand All @@ -270,8 +270,8 @@ func TestNewAsyncPollingProjectConfigManagerWithLastModifiedDates(t *testing.T)

func TestNewPollingProjectConfigManagerWithDifferentDatafileRevisions(t *testing.T) {
// Test newer datafile should replace the older one if revisions are different
mockDatafile1 := []byte(`{"revision":"42","botFiltering":true}`)
mockDatafile2 := []byte(`{"revision":"43","botFiltering":false}`)
mockDatafile1 := []byte(`{"revision":"42","botFiltering":true,"version": "4"}`)
mockDatafile2 := []byte(`{"revision":"43","botFiltering":false,"version": "4"}`)
projectConfig1, _ := datafileprojectconfig.NewDatafileProjectConfig(mockDatafile1)
projectConfig2, _ := datafileprojectconfig.NewDatafileProjectConfig(mockDatafile2)
mockRequester := new(MockRequester)
Expand Down Expand Up @@ -306,8 +306,8 @@ func TestNewPollingProjectConfigManagerWithDifferentDatafileRevisions(t *testing

func TestNewAsyncPollingProjectConfigManagerWithDifferentDatafileRevisions(t *testing.T) {
// Test newer datafile should replace the older one if revisions are different
mockDatafile1 := []byte(`{"revision":"42","botFiltering":true}`)
mockDatafile2 := []byte(`{"revision":"43","botFiltering":false}`)
mockDatafile1 := []byte(`{"revision":"42","botFiltering":true,"version": "4"}`)
mockDatafile2 := []byte(`{"revision":"43","botFiltering":false,"version": "4"}`)
projectConfig1, _ := datafileprojectconfig.NewDatafileProjectConfig(mockDatafile1)
projectConfig2, _ := datafileprojectconfig.NewDatafileProjectConfig(mockDatafile2)
mockRequester := new(MockRequester)
Expand Down Expand Up @@ -342,7 +342,7 @@ func TestNewAsyncPollingProjectConfigManagerWithDifferentDatafileRevisions(t *te

func TestNewPollingProjectConfigManagerWithErrorHandling(t *testing.T) {
mockDatafile1 := []byte("NOT-VALID")
mockDatafile2 := []byte(`{"revision":"43","botFiltering":false}`)
mockDatafile2 := []byte(`{"revision":"43","botFiltering":false,"version": "4"}`)

projectConfig1, _ := datafileprojectconfig.NewDatafileProjectConfig(mockDatafile1)
projectConfig2, _ := datafileprojectconfig.NewDatafileProjectConfig(mockDatafile2)
Expand Down Expand Up @@ -375,7 +375,7 @@ func TestNewPollingProjectConfigManagerWithErrorHandling(t *testing.T) {

func TestNewAsyncPollingProjectConfigManagerWithErrorHandling(t *testing.T) {
mockDatafile1 := []byte("NOT-VALID")
mockDatafile2 := []byte(`{"revision":"43","botFiltering":false}`)
mockDatafile2 := []byte(`{"revision":"43","botFiltering":false,"version": "4"}`)

projectConfig1, _ := datafileprojectconfig.NewDatafileProjectConfig(mockDatafile1)
projectConfig2, _ := datafileprojectconfig.NewDatafileProjectConfig(mockDatafile2)
Expand Down Expand Up @@ -408,8 +408,8 @@ func TestNewAsyncPollingProjectConfigManagerWithErrorHandling(t *testing.T) {
}

func TestNewPollingProjectConfigManagerOnDecision(t *testing.T) {
mockDatafile1 := []byte(`{"revision":"42","botFiltering":true}`)
mockDatafile2 := []byte(`{"revision":"43","botFiltering":false}`)
mockDatafile1 := []byte(`{"revision":"42","botFiltering":true,"version": "4"}`)
mockDatafile2 := []byte(`{"revision":"43","botFiltering":false,"version": "4"}`)
mockRequester := new(MockRequester)
mockRequester.On("Get", []utils.Header(nil)).Return(mockDatafile2, http.Header{}, http.StatusOK, nil)

Expand Down Expand Up @@ -441,7 +441,7 @@ func TestNewPollingProjectConfigManagerOnDecision(t *testing.T) {
}

func TestNewAsyncPollingProjectConfigManagerOnDecision(t *testing.T) {
mockDatafile1 := []byte(`{"revision":"42","botFiltering":true}`)
mockDatafile1 := []byte(`{"revision":"42","botFiltering":true,"version": "4"}`)
projectConfig1, _ := datafileprojectconfig.NewDatafileProjectConfig(mockDatafile1)
mockRequester := new(MockRequester)
mockRequester.On("Get", []utils.Header(nil)).Return(mockDatafile1, http.Header{}, http.StatusOK, nil)
Expand Down Expand Up @@ -469,8 +469,8 @@ func TestNewAsyncPollingProjectConfigManagerOnDecision(t *testing.T) {

func TestGetOptimizelyConfigForNewPollingProjectConfigManager(t *testing.T) {

mockDatafile1 := []byte(`{"revision":"42","botFiltering":true}`)
mockDatafile2 := []byte(`{"revision":"43","botFiltering":false}`)
mockDatafile1 := []byte(`{"revision":"42","botFiltering":true,"version": "4"}`)
mockDatafile2 := []byte(`{"revision":"43","botFiltering":false,"version": "4"}`)

mockRequester := new(MockRequester)
mockRequester.On("Get", []utils.Header(nil)).Return(mockDatafile2, http.Header{}, http.StatusOK, nil)
Expand All @@ -493,8 +493,8 @@ func TestGetOptimizelyConfigForNewPollingProjectConfigManager(t *testing.T) {
}

func TestGetOptimizelyConfigForNewAsyncPollingProjectConfigManager(t *testing.T) {
mockDatafile1 := []byte(`{"revision":"42","botFiltering":true}`)
mockDatafile2 := []byte(`{"revision":"43","botFiltering":false}`)
mockDatafile1 := []byte(`{"revision":"42","botFiltering":true,"version": "4"}`)
mockDatafile2 := []byte(`{"revision":"43","botFiltering":false,"version": "4"}`)
mockRequester := new(MockRequester)
mockRequester.On("Get", []utils.Header(nil)).Return(mockDatafile2, http.Header{}, http.StatusOK, nil)

Expand All @@ -516,8 +516,8 @@ func TestGetOptimizelyConfigForNewAsyncPollingProjectConfigManager(t *testing.T)
}

func TestNewPollingProjectConfigManagerHardcodedDatafile(t *testing.T) {
mockDatafile1 := []byte(`{"revision":"42"}`)
mockDatafile2 := []byte(`{"revision":"43"}`)
mockDatafile1 := []byte(`{"revision":"42","version": "4"}`)
mockDatafile2 := []byte(`{"revision":"43","version": "4"}`)
sdkKey := "test_sdk_key"

mockRequester := new(MockRequester)
Expand All @@ -533,8 +533,8 @@ func TestNewPollingProjectConfigManagerHardcodedDatafile(t *testing.T) {
}

func TestNewAsyncPollingProjectConfigManagerHardcodedDatafile(t *testing.T) {
mockDatafile1 := []byte(`{"revision":"42"}`)
mockDatafile2 := []byte(`{"revision":"43"}`)
mockDatafile1 := []byte(`{"revision":"42","version": "4"}`)
mockDatafile2 := []byte(`{"revision":"43","version": "4"}`)
sdkKey := "test_sdk_key"

mockRequester := new(MockRequester)
Expand All @@ -550,7 +550,7 @@ func TestNewAsyncPollingProjectConfigManagerHardcodedDatafile(t *testing.T) {
}

func TestNewPollingProjectConfigManagerPullsImmediately(t *testing.T) {
mockDatafile1 := []byte(`{"revision":"42"}`)
mockDatafile1 := []byte(`{"revision":"42","version": "4"}`)
sdkKey := "test_sdk_key"

mockRequester := new(MockRequester)
Expand All @@ -566,7 +566,7 @@ func TestNewPollingProjectConfigManagerPullsImmediately(t *testing.T) {
}

func TestNewAsyncPollingProjectConfigManagerDoesNotPullImmediately(t *testing.T) {
mockDatafile1 := []byte(`{"revision":"42"}`)
mockDatafile1 := []byte(`{"revision":"42","version": "4"}`)
sdkKey := "test_sdk_key"

mockRequester := new(MockRequester)
Expand Down Expand Up @@ -614,7 +614,7 @@ func TestDatafileTemplate(t *testing.T) {
func TestWithRequester(t *testing.T) {

sdkKey := "test_sdk_key"
mockDatafile := []byte(`{"revision":"42"}`)
mockDatafile := []byte(`{"revision":"42","version": "4"}`)
mockRequester := new(MockRequester)
mockRequester.On("Get", []utils.Header(nil)).Return(mockDatafile, http.Header{}, http.StatusOK, nil)
configManager := NewPollingProjectConfigManager(sdkKey, WithRequester(mockRequester))
Expand Down
18 changes: 11 additions & 7 deletions pkg/config/static_manager_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,13 @@ func TestNewStaticProjectConfigManagerFromPayload(t *testing.T) {

mockDatafile := []byte(`{"accountId":"42","projectId":"123""}`)
configManager, err := NewStaticProjectConfigManagerFromPayload(mockDatafile)
assert.NotNil(t, err)
assert.Error(t, err)

mockDatafile = []byte(`{"accountId":"42","projectId":"123"}`)
mockDatafile = []byte(`{"accountId":"42","projectId":"123",}`)
configManager, err = NewStaticProjectConfigManagerFromPayload(mockDatafile)
assert.Error(t, err)

mockDatafile = []byte(`{"accountId":"42","projectId":"123","version":"4"}`)
configManager, err = NewStaticProjectConfigManagerFromPayload(mockDatafile)
assert.Nil(t, err)

Expand All @@ -51,7 +55,7 @@ func TestNewStaticProjectConfigManagerFromPayload(t *testing.T) {

func TestStaticGetOptimizelyConfig(t *testing.T) {

mockDatafile := []byte(`{"accountId":"42","projectId":"123"}`)
mockDatafile := []byte(`{"accountId":"42","projectId":"123","version":"4"}`)
configManager, err := NewStaticProjectConfigManagerFromPayload(mockDatafile)
assert.Nil(t, err)

Expand All @@ -65,12 +69,12 @@ func TestStaticGetOptimizelyConfig(t *testing.T) {
func TestNewStaticProjectConfigManagerFromURL(t *testing.T) {

configManager, err := NewStaticProjectConfigManagerFromURL("no_key_exists")
assert.NotNil(t, err)
assert.Error(t, err)
assert.Nil(t, configManager)
}

func TestNewStaticProjectConfigManagerOnDecision(t *testing.T) {
mockDatafile := []byte(`{"accountId":"42","projectId":"123"}`)
mockDatafile := []byte(`{"accountId":"42","projectId":"123","version":"4"}`)
configManager, err := NewStaticProjectConfigManagerFromPayload(mockDatafile)
assert.Nil(t, err)

Expand All @@ -79,12 +83,12 @@ func TestNewStaticProjectConfigManagerOnDecision(t *testing.T) {
}
id, err := configManager.OnProjectConfigUpdate(callback)

assert.NotNil(t, err)
assert.Error(t, err)
assert.Equal(t, err, errors.New("method OnProjectConfigUpdate does not have any effect on StaticProjectConfigManager"))
assert.Equal(t, id, 0)

err = configManager.RemoveOnProjectConfigUpdate(id)
assert.NotNil(t, err)
assert.Error(t, err)
assert.Equal(t, err, errors.New("method RemoveOnProjectConfigUpdate does not have any effect on StaticProjectConfigManager"))

}