Skip to content

Commit 952241f

Browse files
pawels-optimizelypawels-optimizely
authored
feat: add valid datafile version check (#234)
* feat: add valid datafile version check
1 parent a9fd8b8 commit 952241f

File tree

5 files changed

+60
-40
lines changed

5 files changed

+60
-40
lines changed

‎pkg/client/factory_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ func TestClientWithCustomCtx(t *testing.T) {
136136
}
137137

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

‎pkg/config/datafileprojectconfig/config.go‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
package datafileprojectconfig
1919

2020
import (
21+
"errors"
2122
"fmt"
2223

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

2829
var logger = logging.GetLogger("DatafileProjectConfig")
2930

31+
var datafileVersions = map[string]struct{}{
32+
"4": {},
33+
}
34+
3035
// DatafileProjectConfig is a project config backed by a datafile
3136
type DatafileProjectConfig struct {
3237
accountID string
@@ -173,7 +178,13 @@ func (c DatafileProjectConfig) GetGroupByID(groupID string) (entities.Group, err
173178
func NewDatafileProjectConfig(jsonDatafile []byte) (*DatafileProjectConfig, error) {
174179
datafile, err := Parse(jsonDatafile)
175180
if err != nil {
176-
logger.Error("Error parsing datafile.", err)
181+
logger.Error("Error parsing datafile", err)
182+
return nil, err
183+
}
184+
185+
if _, ok := datafileVersions[datafile.Version]; !ok {
186+
err = errors.New("unsupported datafile version")
187+
logger.Error(fmt.Sprintf("Version %s of datafile not supported", datafile.Version), err)
177188
return nil, err
178189
}
179190

‎pkg/config/datafileprojectconfig/config_test.go‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,18 @@ import (
2828

2929
func TestNewDatafileProjectConfigNil(t *testing.T) {
3030
projectConfig, err := NewDatafileProjectConfig(nil)
31-
assert.NotNil(t, err)
31+
assert.Error(t, err)
32+
assert.Nil(t, projectConfig)
33+
34+
jsonDatafileStr := `{"accountID": "123", "revision": "1", "projectId": "12345", "version": "2"}`
35+
projectConfig, err = NewDatafileProjectConfig([]byte(jsonDatafileStr))
36+
assert.Error(t, err)
3237
assert.Nil(t, projectConfig)
3338
}
3439

3540
func TestNewDatafileProjectConfigNotNil(t *testing.T) {
3641
dpc := DatafileProjectConfig{accountID: "123", revision: "1", projectID: "12345"}
37-
jsonDatafileStr := `{"accountID": "123", "revision": "1", "projectId": "12345"}`
42+
jsonDatafileStr := `{"accountID": "123", "revision": "1", "projectId": "12345", "version": "4"}`
3843
jsonDatafile := []byte(jsonDatafileStr)
3944
projectConfig, err := NewDatafileProjectConfig(jsonDatafile)
4045
assert.Nil(t, err)

‎pkg/config/polling_manager_test.go‎

Lines changed: 29 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ func assertPeriodically(t *testing.T, evaluationMethod func() bool) {
5555
func TestNewPollingProjectConfigManagerWithOptions(t *testing.T) {
5656

5757
invalidDatafile := []byte(`INVALID`)
58-
mockDatafile := []byte(`{"revision":"42"}`)
58+
mockDatafile := []byte(`{"revision":"42","version": "4"}`)
5959

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

8181
func TestNewAsyncPollingProjectConfigManagerWithOptions(t *testing.T) {
8282

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

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

103103
func TestSyncConfigFetchesDatafileUsingRequester(t *testing.T) {
104104

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

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

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

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

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

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

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

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

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

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

380380
projectConfig1, _ := datafileprojectconfig.NewDatafileProjectConfig(mockDatafile1)
381381
projectConfig2, _ := datafileprojectconfig.NewDatafileProjectConfig(mockDatafile2)
@@ -408,8 +408,8 @@ func TestNewAsyncPollingProjectConfigManagerWithErrorHandling(t *testing.T) {
408408
}
409409

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

@@ -441,7 +441,7 @@ func TestNewPollingProjectConfigManagerOnDecision(t *testing.T) {
441441
}
442442

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

470470
func TestGetOptimizelyConfigForNewPollingProjectConfigManager(t *testing.T) {
471471

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

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

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

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

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

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

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

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

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

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

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

572572
mockRequester := new(MockRequester)
@@ -614,7 +614,7 @@ func TestDatafileTemplate(t *testing.T) {
614614
func TestWithRequester(t *testing.T) {
615615

616616
sdkKey := "test_sdk_key"
617-
mockDatafile := []byte(`{"revision":"42"}`)
617+
mockDatafile := []byte(`{"revision":"42","version": "4"}`)
618618
mockRequester := new(MockRequester)
619619
mockRequester.On("Get", []utils.Header(nil)).Return(mockDatafile, http.Header{}, http.StatusOK, nil)
620620
configManager := NewPollingProjectConfigManager(sdkKey, WithRequester(mockRequester))

‎pkg/config/static_manager_test.go‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,13 @@ func TestNewStaticProjectConfigManagerFromPayload(t *testing.T) {
3737

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

42-
mockDatafile = []byte(`{"accountId":"42","projectId":"123"}`)
42+
mockDatafile = []byte(`{"accountId":"42","projectId":"123",}`)
43+
configManager, err = NewStaticProjectConfigManagerFromPayload(mockDatafile)
44+
assert.Error(t, err)
45+
46+
mockDatafile = []byte(`{"accountId":"42","projectId":"123","version":"4"}`)
4347
configManager, err = NewStaticProjectConfigManagerFromPayload(mockDatafile)
4448
assert.Nil(t, err)
4549

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

5256
func TestStaticGetOptimizelyConfig(t *testing.T) {
5357

54-
mockDatafile := []byte(`{"accountId":"42","projectId":"123"}`)
58+
mockDatafile := []byte(`{"accountId":"42","projectId":"123","version":"4"}`)
5559
configManager, err := NewStaticProjectConfigManagerFromPayload(mockDatafile)
5660
assert.Nil(t, err)
5761

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

6771
configManager, err := NewStaticProjectConfigManagerFromURL("no_key_exists")
68-
assert.NotNil(t, err)
72+
assert.Error(t, err)
6973
assert.Nil(t, configManager)
7074
}
7175

7276
func TestNewStaticProjectConfigManagerOnDecision(t *testing.T) {
73-
mockDatafile := []byte(`{"accountId":"42","projectId":"123"}`)
77+
mockDatafile := []byte(`{"accountId":"42","projectId":"123","version":"4"}`)
7478
configManager, err := NewStaticProjectConfigManagerFromPayload(mockDatafile)
7579
assert.Nil(t, err)
7680

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

82-
assert.NotNil(t, err)
86+
assert.Error(t, err)
8387
assert.Equal(t, err, errors.New("method OnProjectConfigUpdate does not have any effect on StaticProjectConfigManager"))
8488
assert.Equal(t, id, 0)
8589

8690
err = configManager.RemoveOnProjectConfigUpdate(id)
87-
assert.NotNil(t, err)
91+
assert.Error(t, err)
8892
assert.Equal(t, err, errors.New("method RemoveOnProjectConfigUpdate does not have any effect on StaticProjectConfigManager"))
8993

9094
}

0 commit comments

Comments
 (0)