prepare 5.3.0 release

ldfiledata/file_data_source_builder.go

@@ -9,6 +9,19 @@ import (
 // detecting when data files should be reloaded. Its standard implementation is in the ldfilewatch package.
 type ReloaderFactory func(paths []string, loggers ldlog.Loggers, reload func(), closeCh <-chan struct{}) error
 
+// DuplicateKeysHandling is a parameter type used with DataSourceBuilder.DuplicateKeysHandling.
+type DuplicateKeysHandling string
+
+const (
+	// DuplicateKeysFail is an option for DataSourceBuilder.DuplicateKeysHandling, meaning that data loading
+	// should fail if keys are duplicated across files. This is the default behavior.
+	DuplicateKeysFail DuplicateKeysHandling = "fail"
+
+	// DuplicateKeysIgnoreAllButFirst is an option for DataSourceBuilder.DuplicateKeysHandling, meaning that
+	// if keys are duplicated across files the first occurrence will be used.
+	DuplicateKeysIgnoreAllButFirst DuplicateKeysHandling = "ignore"
+)
+
 // DataSourceBuilder is a builder for configuring the file-based data source.
 //
 // Obtain an instance of this type by calling DataSource(). After calling its methods to specify any
@@ -21,13 +34,22 @@ type ReloaderFactory func(paths []string, loggers ldlog.Loggers, reload func(),
 // You do not need to call the builder's CreatePersistentDataSource() method yourself; that will be
 // done by the SDK.
 type DataSourceBuilder struct {
-	filePaths       []string
-	reloaderFactory ReloaderFactory
+	filePaths             []string
+	duplicateKeysHandling DuplicateKeysHandling
+	reloaderFactory       ReloaderFactory
 }
 
 // DataSource returns a configurable builder for a file-based data source.
 func DataSource() *DataSourceBuilder {
-	return &DataSourceBuilder{}
+	return &DataSourceBuilder{duplicateKeysHandling: DuplicateKeysFail}
+}
+
+// DuplicateKeysHandling specifies how to handle keys that are duplicated across files.
+//
+// If this is not specified, or if you set it to an unrecognized value, the default is DuplicateKeysFail.
+func (b *DataSourceBuilder) DuplicateKeysHandling(duplicateKeysHandling DuplicateKeysHandling) *DataSourceBuilder {
+	b.duplicateKeysHandling = duplicateKeysHandling
+	return b
 }
 
 // FilePaths specifies the input data files. The paths may be any number of absolute or relative file paths.
@@ -55,5 +77,6 @@ func (b *DataSourceBuilder) CreateDataSource(
 	context interfaces.ClientContext,
 	dataSourceUpdates interfaces.DataSourceUpdates,
 ) (interfaces.DataSource, error) {
-	return newFileDataSourceImpl(context, dataSourceUpdates, b.filePaths, b.reloaderFactory)
+	return newFileDataSourceImpl(context, dataSourceUpdates, b.filePaths,
+		b.duplicateKeysHandling, b.reloaderFactory)
 }

ldfiledata/file_data_source_impl.go

@@ -22,21 +22,23 @@ import (
 )
 
 type fileDataSource struct {
-	dataSourceUpdates interfaces.DataSourceUpdates
-	absFilePaths      []string
-	reloaderFactory   ReloaderFactory
-	loggers           ldlog.Loggers
-	isInitialized     bool
-	readyCh           chan<- struct{}
-	readyOnce         sync.Once
-	closeOnce         sync.Once
-	closeReloaderCh   chan struct{}
+	dataSourceUpdates     interfaces.DataSourceUpdates
+	absFilePaths          []string
+	duplicateKeysHandling DuplicateKeysHandling
+	reloaderFactory       ReloaderFactory
+	loggers               ldlog.Loggers
+	isInitialized         bool
+	readyCh               chan<- struct{}
+	readyOnce             sync.Once
+	closeOnce             sync.Once
+	closeReloaderCh       chan struct{}
 }
 
 func newFileDataSourceImpl(
 	context interfaces.ClientContext,
 	dataSourceUpdates interfaces.DataSourceUpdates,
 	filePaths []string,
+	duplicateKeysHandling DuplicateKeysHandling,
 	reloaderFactory ReloaderFactory,
 ) (interfaces.DataSource, error) {
 	abs, err := absFilePaths(filePaths)
@@ -46,10 +48,11 @@ func newFileDataSourceImpl(
 	}
 
 	fs := &fileDataSource{
-		dataSourceUpdates: dataSourceUpdates,
-		absFilePaths:      abs,
-		reloaderFactory:   reloaderFactory,
-		loggers:           context.GetLogging().GetLoggers(),
+		dataSourceUpdates:     dataSourceUpdates,
+		absFilePaths:          abs,
+		duplicateKeysHandling: duplicateKeysHandling,
+		reloaderFactory:       reloaderFactory,
+		loggers:               context.GetLogging().GetLoggers(),
 	}
 	fs.loggers.SetPrefix("FileDataSource:")
 	return fs, nil
@@ -102,7 +105,7 @@ func (fs *fileDataSource) reload() {
 			return
 		}
 	}
-	storeData, err := mergeFileData(filesData...)
+	storeData, err := mergeFileData(fs.duplicateKeysHandling, filesData...)
 	if err == nil {
 		if fs.dataSourceUpdates.Init(storeData) {
 			fs.signalStartComplete(true)
@@ -154,9 +157,15 @@ func insertData(
 	kind ldstoretypes.DataKind,
 	key string,
 	data ldstoretypes.ItemDescriptor,
+	duplicateKeysHandling DuplicateKeysHandling,
 ) error {
 	if _, exists := all[kind][key]; exists {
-		return fmt.Errorf("%s '%s' is specified by multiple files", kind, key)
+		switch duplicateKeysHandling {
+		case DuplicateKeysIgnoreAllButFirst:
+			return nil
+		default:
+			return fmt.Errorf("%s '%s' is specified by multiple files", kind, key)
+		}
 	}
 	all[kind][key] = data
 	return nil
@@ -185,7 +194,10 @@ func detectJSON(rawData []byte) bool {
 	return strings.HasPrefix(strings.TrimLeftFunc(string(rawData), unicode.IsSpace), "{")
 }
 
-func mergeFileData(allFileData ...fileData) ([]ldstoretypes.Collection, error) {
+func mergeFileData(
+	duplicateKeysHandling DuplicateKeysHandling,
+	allFileData ...fileData,
+) ([]ldstoretypes.Collection, error) {
 	all := map[ldstoretypes.DataKind]map[string]ldstoretypes.ItemDescriptor{
 		datakinds.Features: {},
 		datakinds.Segments: {},
@@ -195,7 +207,7 @@ func mergeFileData(allFileData ...fileData) ([]ldstoretypes.Collection, error) {
 			for key, f := range *d.Flags {
 				ff := f
 				data := ldstoretypes.ItemDescriptor{Version: f.Version, Item: &ff}
-				if err := insertData(all, datakinds.Features, key, data); err != nil {
+				if err := insertData(all, datakinds.Features, key, data, duplicateKeysHandling); err != nil {
 					return nil, err
 				}
 			}
@@ -204,7 +216,7 @@ func mergeFileData(allFileData ...fileData) ([]ldstoretypes.Collection, error) {
 			for key, value := range *d.FlagValues {
 				flag := makeFlagWithValue(key, value)
 				data := ldstoretypes.ItemDescriptor{Version: flag.Version, Item: flag}
-				if err := insertData(all, datakinds.Features, key, data); err != nil {
+				if err := insertData(all, datakinds.Features, key, data, duplicateKeysHandling); err != nil {
 					return nil, err
 				}
 			}
@@ -213,7 +225,7 @@ func mergeFileData(allFileData ...fileData) ([]ldstoretypes.Collection, error) {
 			for key, s := range *d.Segments {
 				ss := s
 				data := ldstoretypes.ItemDescriptor{Version: s.Version, Item: &ss}
-				if err := insertData(all, datakinds.Segments, key, data); err != nil {
+				if err := insertData(all, datakinds.Segments, key, data, duplicateKeysHandling); err != nil {
 					return nil, err
 				}
 			}

ldfiledata/file_data_source_test.go

@@ -5,6 +5,7 @@ import (
 	"os"
 	"testing"
 
+	"gopkg.in/launchdarkly/go-server-sdk.v5/interfaces"
 	"gopkg.in/launchdarkly/go-server-sdk.v5/internal/datakinds"
 	"gopkg.in/launchdarkly/go-server-sdk.v5/internal/sharedtest"
 	"gopkg.in/launchdarkly/go-server-sdk.v5/ldcomponents"
@@ -13,7 +14,6 @@ import (
 	"gopkg.in/launchdarkly/go-sdk-common.v2/ldlogtest"
 	"gopkg.in/launchdarkly/go-sdk-common.v2/ldvalue"
 	"gopkg.in/launchdarkly/go-server-sdk-evaluation.v1/ldmodel"
-	"gopkg.in/launchdarkly/go-server-sdk.v5/interfaces"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -74,15 +74,11 @@ segments:
 			p.waitForStart()
 			require.True(t, p.dataSource.IsInitialized())
 
-			flagItem, err := p.updates.DataStore.Get(datakinds.Features, "my-flag")
-			require.NoError(t, err)
-			require.NotNil(t, flagItem.Item)
-			assert.True(t, flagItem.Item.(*ldmodel.FeatureFlag).On)
+			flag := requireFlag(t, p.updates.DataStore, "my-flag")
+			assert.True(t, flag.On)
 
-			segmentItem, err := p.updates.DataStore.Get(datakinds.Segments, "my-segment")
-			require.NoError(t, err)
-			require.NotNil(t, segmentItem.Item)
-			assert.Empty(t, segmentItem.Item.(*ldmodel.Segment).Rules)
+			segment := requireSegment(t, p.updates.DataStore, "my-segment")
+			assert.Empty(t, segment.Rules)
 		})
 	})
 }
@@ -94,10 +90,8 @@ func TestNewFileDataSourceJson(t *testing.T) {
 			p.waitForStart()
 			require.True(t, p.dataSource.IsInitialized())
 
-			flagItem, err := p.updates.DataStore.Get(datakinds.Features, "my-flag")
-			require.NoError(t, err)
-			require.NotNil(t, flagItem.Item)
-			assert.True(t, flagItem.Item.(*ldmodel.FeatureFlag).On)
+			flag := requireFlag(t, p.updates.DataStore, "my-flag")
+			assert.True(t, flag.On)
 		})
 	})
 }
@@ -122,15 +116,11 @@ func TestNewFileDataSourceJsonWithTwoFiles(t *testing.T) {
 				p.waitForStart()
 				require.True(t, p.dataSource.IsInitialized())
 
-				flagItem1, err := p.updates.DataStore.Get(datakinds.Features, "my-flag1")
-				require.NoError(t, err)
-				require.NotNil(t, flagItem1.Item)
-				assert.True(t, flagItem1.Item.(*ldmodel.FeatureFlag).On)
+				flag1 := requireFlag(t, p.updates.DataStore, "my-flag1")
+				assert.True(t, flag1.On)
 
-				flagItem2, err := p.updates.DataStore.Get(datakinds.Features, "my-flag2")
-				require.NoError(t, err)
-				require.NotNil(t, flagItem2.Item)
-				assert.True(t, flagItem2.Item.(*ldmodel.FeatureFlag).On)
+				flag2 := requireFlag(t, p.updates.DataStore, "my-flag2")
+				assert.True(t, flag2.On)
 			})
 		})
 	})
@@ -149,12 +139,35 @@ func TestNewFileDataSourceJsonWithTwoConflictingFiles(t *testing.T) {
 				withFileDataSourceTestParams(factory, func(p fileDataSourceTestParams) {
 					p.waitForStart()
 					require.False(t, p.dataSource.IsInitialized())
+
+					p.mockLog.AssertMessageMatch(t, true, ldlog.Error, "specified by multiple files")
 				})
 			})
 		}
 	})
 }
 
+func TestDuplicateKeysHandlingCanSuppressErrors(t *testing.T) {
+	file1Data := `{"flags": {"flag1": {"on": true}, "flag2": {"on": false}}, "segments": {"segment1": {}}}`
+	file2Data := `{"flags": {"flag2": {"on": true}}}`
+
+	sharedtest.WithTempFileContaining([]byte(file1Data), func(filename1 string) {
+		sharedtest.WithTempFileContaining([]byte(file2Data), func(filename2 string) {
+			factory := DataSource().FilePaths(filename1, filename2).
+				DuplicateKeysHandling(DuplicateKeysIgnoreAllButFirst)
+			withFileDataSourceTestParams(factory, func(p fileDataSourceTestParams) {
+				p.waitForStart()
+				require.True(t, p.dataSource.IsInitialized())
+
+				flag2 := requireFlag(t, p.updates.DataStore, "flag2")
+				assert.False(t, flag2.On)
+
+				p.mockLog.AssertMessageMatch(t, false, ldlog.Error, "specified by multiple files")
+			})
+		})
+	})
+}
+
 func TestNewFileDataSourceBadData(t *testing.T) {
 	sharedtest.WithTempFileContaining([]byte(`bad data`), func(filename string) {
 		factory := DataSource().FilePaths(filename)
@@ -200,10 +213,9 @@ flagValues:
 		withFileDataSourceTestParams(factory, func(p fileDataSourceTestParams) {
 			p.waitForStart()
 			require.True(t, p.dataSource.IsInitialized())
-			flagItem, err := p.updates.DataStore.Get(datakinds.Features, "my-flag")
-			require.NoError(t, err)
-			require.NotNil(t, flagItem.Item)
-			assert.Equal(t, []ldvalue.Value{ldvalue.Bool(true)}, flagItem.Item.(*ldmodel.FeatureFlag).Variations)
+
+			flag := requireFlag(t, p.updates.DataStore, "my-flag")
+			assert.Equal(t, []ldvalue.Value{ldvalue.Bool(true)}, flag.Variations)
 		})
 	})
 }
@@ -220,3 +232,17 @@ func TestReloaderFailureDoesNotPreventStarting(t *testing.T) {
 		assert.Len(t, p.mockLog.GetOutput(ldlog.Error), 1)
 	})
 }
+
+func requireFlag(t *testing.T, store interfaces.DataStore, key string) *ldmodel.FeatureFlag {
+	item, err := store.Get(datakinds.Features, key)
+	require.NoError(t, err)
+	require.NotNil(t, item.Item)
+	return item.Item.(*ldmodel.FeatureFlag)
+}
+
+func requireSegment(t *testing.T, store interfaces.DataStore, key string) *ldmodel.Segment {
+	item, err := store.Get(datakinds.Segments, key)
+	require.NoError(t, err)
+	require.NotNil(t, item.Item)
+	return item.Item.(*ldmodel.Segment)
+}

ldfiledata/package_info.go

@@ -72,7 +72,8 @@
 //
 // It is also possible to specify both "flags" and "flagValues", if you want some flags to have simple
 // values and others to have complex behavior. However, it is an error to use the same flag key or
-// segment key more than once, either in a single file or across multiple files.
+// segment key more than once, either in a single file or across multiple files, unless you specify
+// otherwise with the DuplicateKeysHandling method.
 //
 // If the data source encounters any error in any file-- malformed content, a missing file, or a
 // duplicate key-- it will not load flags from any of the files.