From 5a32688eaa9bf7e430c72ebc98f323b412577853 Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Mon, 16 Sep 2024 13:29:12 -0700 Subject: [PATCH 1/2] refactor: hide data source configuration behind dataSystem interface --- internal/datastore/data_store_eval_impl.go | 4 +- internal/datasystem/data_availability.go | 12 ++ internal/datasystem/fdv1_datasystem.go | 147 +++++++++++++++++++++ ldclient.go | 146 ++++++++------------ ldclient_evaluation_benchmark_test.go | 16 +-- ldclient_test.go | 22 ++- subsystems/data_store.go | 27 +--- subsystems/ldstoreimpl/data_store_eval.go | 2 +- subsystems/read_only_store.go | 30 +++++ 9 files changed, 278 insertions(+), 128 deletions(-) create mode 100644 internal/datasystem/data_availability.go create mode 100644 internal/datasystem/fdv1_datasystem.go create mode 100644 subsystems/read_only_store.go diff --git a/internal/datastore/data_store_eval_impl.go b/internal/datastore/data_store_eval_impl.go index da9ca3b8..869f84d8 100644 --- a/internal/datastore/data_store_eval_impl.go +++ b/internal/datastore/data_store_eval_impl.go @@ -9,13 +9,13 @@ import ( ) type dataStoreEvaluatorDataProviderImpl struct { - store subsystems.DataStore + store subsystems.ReadOnlyStore loggers ldlog.Loggers } // NewDataStoreEvaluatorDataProviderImpl creates the internal implementation of the adapter that connects // the Evaluator (from go-server-sdk-evaluation) with the data store. -func NewDataStoreEvaluatorDataProviderImpl(store subsystems.DataStore, loggers ldlog.Loggers) ldeval.DataProvider { +func NewDataStoreEvaluatorDataProviderImpl(store subsystems.ReadOnlyStore, loggers ldlog.Loggers) ldeval.DataProvider { return dataStoreEvaluatorDataProviderImpl{store, loggers} } diff --git a/internal/datasystem/data_availability.go b/internal/datasystem/data_availability.go new file mode 100644 index 00000000..630a10aa --- /dev/null +++ b/internal/datasystem/data_availability.go @@ -0,0 +1,12 @@ +package datasystem + +type DataAvailability string + +const ( + // Defaults means the SDK has no data and will evaluate flags using the application-provided default values. + Defaults = DataAvailability("defaults") + // Cached means the SDK has data, not necessarily the latest, which will be used to evaluate flags. + Cached = DataAvailability("cached") + // Refreshed means the SDK has obtained, at least once, the latest known data from LaunchDarkly. + Refreshed = DataAvailability("refreshed") +) diff --git a/internal/datasystem/fdv1_datasystem.go b/internal/datasystem/fdv1_datasystem.go new file mode 100644 index 00000000..8c7cedee --- /dev/null +++ b/internal/datasystem/fdv1_datasystem.go @@ -0,0 +1,147 @@ +package datasystem + +import ( + "github.com/launchdarkly/go-server-sdk/v7/interfaces" + "github.com/launchdarkly/go-server-sdk/v7/internal" + "github.com/launchdarkly/go-server-sdk/v7/internal/datasource" + "github.com/launchdarkly/go-server-sdk/v7/internal/datastore" + "github.com/launchdarkly/go-server-sdk/v7/ldcomponents" + "github.com/launchdarkly/go-server-sdk/v7/subsystems" +) + +type FDv1 struct { + dataSourceStatusBroadcaster *internal.Broadcaster[interfaces.DataSourceStatus] + dataSourceStatusProvider interfaces.DataSourceStatusProvider + dataStoreStatusBroadcaster *internal.Broadcaster[interfaces.DataStoreStatus] + dataStoreStatusProvider interfaces.DataStoreStatusProvider + flagChangeEventBroadcaster *internal.Broadcaster[interfaces.FlagChangeEvent] + dataStore subsystems.DataStore + dataSource subsystems.DataSource + offline bool +} + +func NewFDv1(offline bool, dataStoreFactory subsystems.ComponentConfigurer[subsystems.DataStore], dataSourceFactory subsystems.ComponentConfigurer[subsystems.DataSource], clientContext *internal.ClientContextImpl) (*FDv1, error) { + system := &FDv1{ + dataSourceStatusBroadcaster: internal.NewBroadcaster[interfaces.DataSourceStatus](), + dataStoreStatusBroadcaster: internal.NewBroadcaster[interfaces.DataStoreStatus](), + flagChangeEventBroadcaster: internal.NewBroadcaster[interfaces.FlagChangeEvent](), + offline: offline, + } + + dataStoreUpdateSink := datastore.NewDataStoreUpdateSinkImpl(system.dataStoreStatusBroadcaster) + storeFactory := dataStoreFactory + if storeFactory == nil { + storeFactory = ldcomponents.InMemoryDataStore() + } + clientContextWithDataStoreUpdateSink := clientContext + clientContextWithDataStoreUpdateSink.DataStoreUpdateSink = dataStoreUpdateSink + store, err := storeFactory.Build(clientContextWithDataStoreUpdateSink) + if err != nil { + return nil, err + } + system.dataStore = store + + system.dataStoreStatusProvider = datastore.NewDataStoreStatusProviderImpl(store, dataStoreUpdateSink) + + dataSourceUpdateSink := datasource.NewDataSourceUpdateSinkImpl( + store, + system.dataStoreStatusProvider, + system.dataSourceStatusBroadcaster, + system.flagChangeEventBroadcaster, + clientContext.GetLogging().LogDataSourceOutageAsErrorAfter, + clientContext.GetLogging().Loggers, + ) + + dataSource, err := createDataSource(clientContext, dataSourceFactory, dataSourceUpdateSink) + if err != nil { + return nil, err + } + system.dataSource = dataSource + system.dataSourceStatusProvider = datasource.NewDataSourceStatusProviderImpl( + system.dataSourceStatusBroadcaster, + dataSourceUpdateSink, + ) + + return system, nil + +} + +func createDataSource( + context *internal.ClientContextImpl, + dataSourceBuilder subsystems.ComponentConfigurer[subsystems.DataSource], + dataSourceUpdateSink subsystems.DataSourceUpdateSink, +) (subsystems.DataSource, error) { + if context.Offline { + context.GetLogging().Loggers.Info("Starting LaunchDarkly client in offline mode") + dataSourceUpdateSink.UpdateStatus(interfaces.DataSourceStateValid, interfaces.DataSourceErrorInfo{}) + return datasource.NewNullDataSource(), nil + } + factory := dataSourceBuilder + if factory == nil { + // COVERAGE: can't cause this condition in unit tests because it would try to connect to production LD + factory = ldcomponents.StreamingDataSource() + } + contextCopy := *context + contextCopy.BasicClientContext.DataSourceUpdateSink = dataSourceUpdateSink + return factory.Build(&contextCopy) +} + +func (f *FDv1) DataSourceStatusBroadcaster() *internal.Broadcaster[interfaces.DataSourceStatus] { + return f.dataSourceStatusBroadcaster +} + +func (f *FDv1) DataSourceStatusProvider() interfaces.DataSourceStatusProvider { + return f.dataSourceStatusProvider +} + +func (f *FDv1) DataStoreStatusBroadcaster() *internal.Broadcaster[interfaces.DataStoreStatus] { + return f.dataStoreStatusBroadcaster +} + +func (f *FDv1) DataStoreStatusProvider() interfaces.DataStoreStatusProvider { + return f.dataStoreStatusProvider +} + +func (f *FDv1) FlagChangeEventBroadcaster() *internal.Broadcaster[interfaces.FlagChangeEvent] { + return f.flagChangeEventBroadcaster +} + +func (f *FDv1) Start(closeWhenReady chan struct{}) { + f.dataSource.Start(closeWhenReady) +} + +func (f *FDv1) Stop() error { + if f.dataSource != nil { + _ = f.dataSource.Close() + } + if f.dataStore != nil { + _ = f.dataStore.Close() + } + if f.dataSourceStatusBroadcaster != nil { + f.dataSourceStatusBroadcaster.Close() + } + if f.dataStoreStatusBroadcaster != nil { + f.dataStoreStatusBroadcaster.Close() + } + if f.flagChangeEventBroadcaster != nil { + f.flagChangeEventBroadcaster.Close() + } + return nil +} + +func (f *FDv1) DataAvailability() DataAvailability { + if f.offline { + return Defaults + } + if f.dataSource.IsInitialized() { + return Refreshed + } + if f.dataStore.IsInitialized() { + return Cached + } + return Defaults +} + +func (f *FDv1) Store() subsystems.ReadOnlyStore { + return f.dataStore +} diff --git a/ldclient.go b/ldclient.go index 9dbd38ab..39d92800 100644 --- a/ldclient.go +++ b/ldclient.go @@ -10,6 +10,9 @@ import ( "reflect" "time" + "github.com/launchdarkly/go-server-sdk/v7/internal/datasystem" + "github.com/launchdarkly/go-server-sdk/v7/subsystems" + "github.com/launchdarkly/go-sdk-common/v3/ldcontext" "github.com/launchdarkly/go-sdk-common/v3/ldlog" "github.com/launchdarkly/go-sdk-common/v3/ldmigration" @@ -23,11 +26,8 @@ import ( "github.com/launchdarkly/go-server-sdk/v7/internal" "github.com/launchdarkly/go-server-sdk/v7/internal/bigsegments" "github.com/launchdarkly/go-server-sdk/v7/internal/datakinds" - "github.com/launchdarkly/go-server-sdk/v7/internal/datasource" - "github.com/launchdarkly/go-server-sdk/v7/internal/datastore" "github.com/launchdarkly/go-server-sdk/v7/internal/hooks" "github.com/launchdarkly/go-server-sdk/v7/ldcomponents" - "github.com/launchdarkly/go-server-sdk/v7/subsystems" "github.com/launchdarkly/go-server-sdk/v7/subsystems/ldstoreimpl" ) @@ -67,6 +67,30 @@ const ( migrationVarExFuncName = "LDClient.MigrationVariationCtx" ) +// The dataSystem interface represents the requirements for the client to retrieve data necessary +// for evaluations, as well as the related status updates related to the data. +type dataSystem interface { + DataSourceStatusBroadcaster() *internal.Broadcaster[interfaces.DataSourceStatus] + DataSourceStatusProvider() interfaces.DataSourceStatusProvider + DataStoreStatusBroadcaster() *internal.Broadcaster[interfaces.DataStoreStatus] + DataStoreStatusProvider() interfaces.DataStoreStatusProvider + FlagChangeEventBroadcaster() *internal.Broadcaster[interfaces.FlagChangeEvent] + + // Start starts the data system; the given channel will be closed when the system has reached an initial state + // (either permanently failed, e.g. due to bad auth, or succeeded, where Initialized() == true). + Start(closeWhenReady chan struct{}) + // Stop halts the data system. Should be called when the client is closed to stop any long running operations. + Stop() error + + Store() subsystems.ReadOnlyStore + + DataAvailability() datasystem.DataAvailability +} + +var ( + _ dataSystem = &datasystem.FDv1{} +) + // LDClient is the LaunchDarkly client. // // This object evaluates feature flags, generates analytics events, and communicates with @@ -86,14 +110,8 @@ type LDClient struct { sdkKey string loggers ldlog.Loggers eventProcessor ldevents.EventProcessor - dataSource subsystems.DataSource - store subsystems.DataStore evaluator ldeval.Evaluator - dataSourceStatusBroadcaster *internal.Broadcaster[interfaces.DataSourceStatus] - dataSourceStatusProvider interfaces.DataSourceStatusProvider - dataStoreStatusBroadcaster *internal.Broadcaster[interfaces.DataStoreStatus] - dataStoreStatusProvider interfaces.DataStoreStatusProvider - flagChangeEventBroadcaster *internal.Broadcaster[interfaces.FlagChangeEvent] + dataSystem dataSystem flagTracker interfaces.FlagTracker bigSegmentStoreStatusBroadcaster *internal.Broadcaster[interfaces.BigSegmentStoreStatus] bigSegmentStoreStatusProvider interfaces.BigSegmentStoreStatusProvider @@ -222,19 +240,11 @@ func MakeCustomClient(sdkKey string, config Config, waitFor time.Duration) (*LDC client.offline = config.Offline - client.dataStoreStatusBroadcaster = internal.NewBroadcaster[interfaces.DataStoreStatus]() - dataStoreUpdateSink := datastore.NewDataStoreUpdateSinkImpl(client.dataStoreStatusBroadcaster) - storeFactory := config.DataStore - if storeFactory == nil { - storeFactory = ldcomponents.InMemoryDataStore() - } - clientContextWithDataStoreUpdateSink := clientContext - clientContextWithDataStoreUpdateSink.DataStoreUpdateSink = dataStoreUpdateSink - store, err := storeFactory.Build(clientContextWithDataStoreUpdateSink) + system, err := datasystem.NewFDv1(config.Offline, config.DataStore, config.DataSource, clientContext) if err != nil { return nil, err } - client.store = store + client.dataSystem = system bigSegments := config.BigSegments if bigSegments == nil { @@ -269,7 +279,8 @@ func MakeCustomClient(sdkKey string, config Config, waitFor time.Duration) (*LDC ) } - dataProvider := ldstoreimpl.NewDataStoreEvaluatorDataProvider(store, loggers) + // TODO: We can't actually pass STore() here because it wont' swap between the active ones. + dataProvider := ldstoreimpl.NewDataStoreEvaluatorDataProvider(client.dataSystem.Store(), loggers) evalOptions := []ldeval.EvaluatorOption{ ldeval.EvaluatorOptionErrorLogger(client.loggers.ForLevel(ldlog.Error)), } @@ -278,19 +289,6 @@ func MakeCustomClient(sdkKey string, config Config, waitFor time.Duration) (*LDC } client.evaluator = ldeval.NewEvaluatorWithOptions(dataProvider, evalOptions...) - client.dataStoreStatusProvider = datastore.NewDataStoreStatusProviderImpl(store, dataStoreUpdateSink) - - client.dataSourceStatusBroadcaster = internal.NewBroadcaster[interfaces.DataSourceStatus]() - client.flagChangeEventBroadcaster = internal.NewBroadcaster[interfaces.FlagChangeEvent]() - dataSourceUpdateSink := datasource.NewDataSourceUpdateSinkImpl( - store, - client.dataStoreStatusProvider, - client.dataSourceStatusBroadcaster, - client.flagChangeEventBroadcaster, - clientContext.GetLogging().LogDataSourceOutageAsErrorAfter, - loggers, - ) - client.eventProcessor, err = eventProcessorFactory.Build(clientContext) if err != nil { return nil, err @@ -306,18 +304,8 @@ func MakeCustomClient(sdkKey string, config Config, waitFor time.Duration) (*LDC // frequently, it won't be causing an allocation each time. client.withEventsDisabled = newClientEventsDisabledDecorator(client) - dataSource, err := createDataSource(config, clientContext, dataSourceUpdateSink) - client.dataSource = dataSource - if err != nil { - return nil, err - } - client.dataSourceStatusProvider = datasource.NewDataSourceStatusProviderImpl( - client.dataSourceStatusBroadcaster, - dataSourceUpdateSink, - ) - client.flagTracker = internal.NewFlagTrackerImpl( - client.flagChangeEventBroadcaster, + client.dataSystem.FlagChangeEventBroadcaster(), func(flagKey string, context ldcontext.Context, defaultValue ldvalue.Value) ldvalue.Value { value, _ := client.JSONVariation(flagKey, context, defaultValue) return value @@ -327,8 +315,9 @@ func MakeCustomClient(sdkKey string, config Config, waitFor time.Duration) (*LDC client.hookRunner = hooks.NewRunner(loggers, config.Hooks) clientValid = true - client.dataSource.Start(closeWhenReady) - if waitFor > 0 && client.dataSource != datasource.NewNullDataSource() { + + client.dataSystem.Start(closeWhenReady) + if waitFor > 0 && !client.offline { loggers.Infof("Waiting up to %d milliseconds for LaunchDarkly client to start...", waitFor/time.Millisecond) @@ -343,7 +332,7 @@ func MakeCustomClient(sdkKey string, config Config, waitFor time.Duration) (*LDC for { select { case <-closeWhenReady: - if !client.dataSource.IsInitialized() { + if client.dataSystem.DataAvailability() != datasystem.Refreshed { loggers.Warn("LaunchDarkly client initialization failed") return client, ErrInitializationFailed } @@ -361,26 +350,6 @@ func MakeCustomClient(sdkKey string, config Config, waitFor time.Duration) (*LDC return client, nil } -func createDataSource( - config Config, - context *internal.ClientContextImpl, - dataSourceUpdateSink subsystems.DataSourceUpdateSink, -) (subsystems.DataSource, error) { - if config.Offline { - context.GetLogging().Loggers.Info("Starting LaunchDarkly client in offline mode") - dataSourceUpdateSink.UpdateStatus(interfaces.DataSourceStateValid, interfaces.DataSourceErrorInfo{}) - return datasource.NewNullDataSource(), nil - } - factory := config.DataSource - if factory == nil { - // COVERAGE: can't cause this condition in unit tests because it would try to connect to production LD - factory = ldcomponents.StreamingDataSource() - } - contextCopy := *context - contextCopy.BasicClientContext.DataSourceUpdateSink = dataSourceUpdateSink - return factory.Build(&contextCopy) -} - // MigrationVariation returns the migration stage of the migration feature flag for the given evaluation context. // // Returns defaultStage if there is an error or if the flag doesn't exist. @@ -583,13 +552,18 @@ func (client *LDClient) SecureModeHash(context ldcontext.Context) string { // this does not guarantee that the flags are up to date; if you need to know its status in more detail, use // [LDClient.GetDataSourceStatusProvider]. // +// Additionally, if the client was configured to be offline, this will always return true. +// // If this value is false, it means the client has not yet connected to LaunchDarkly, or has permanently // failed. See [MakeClient] for the reasons that this could happen. In this state, feature flag evaluations // will always return default values-- unless you are using a database integration and feature flags had // already been stored in the database by a successfully connected SDK in the past. You can use // [LDClient.GetDataSourceStatusProvider] to get information on errors, or to wait for a successful retry. func (client *LDClient) Initialized() bool { - return client.dataSource.IsInitialized() + if client.offline { + return true + } + return client.dataSystem.DataAvailability() != datasystem.Defaults } // Close shuts down the LaunchDarkly client. After calling this, the LaunchDarkly client @@ -604,21 +578,11 @@ func (client *LDClient) Close() error { if client.eventProcessor != nil { _ = client.eventProcessor.Close() } - if client.dataSource != nil { - _ = client.dataSource.Close() - } - if client.store != nil { - _ = client.store.Close() - } - if client.dataSourceStatusBroadcaster != nil { - client.dataSourceStatusBroadcaster.Close() - } - if client.dataStoreStatusBroadcaster != nil { - client.dataStoreStatusBroadcaster.Close() - } - if client.flagChangeEventBroadcaster != nil { - client.flagChangeEventBroadcaster.Close() + + if client.dataSystem != nil { + _ = client.dataSystem.Stop() } + if client.bigSegmentStoreStatusBroadcaster != nil { client.bigSegmentStoreStatusBroadcaster.Close() } @@ -683,8 +647,8 @@ func (client *LDClient) AllFlagsState(context ldcontext.Context, options ...flag if client.IsOffline() { client.loggers.Warn("Called AllFlagsState in offline mode. Returning empty state") valid = false - } else if !client.Initialized() { - if client.store.IsInitialized() { + } else if client.dataSystem.DataAvailability() != datasystem.Refreshed { + if client.dataSystem.DataAvailability() == datasystem.Cached { client.loggers.Warn("Called AllFlagsState before client initialization; using last known values from data store") } else { client.loggers.Warn("Called AllFlagsState before client initialization. Data store not available; returning empty state") //nolint:lll @@ -696,7 +660,7 @@ func (client *LDClient) AllFlagsState(context ldcontext.Context, options ...flag return flagstate.AllFlags{} } - items, err := client.store.GetAll(datakinds.Features) + items, err := client.dataSystem.Store().GetAll(datakinds.Features) if err != nil { client.loggers.Warn("Unable to fetch flags from data store. Returning empty state. Error: " + err.Error()) return flagstate.AllFlags{} @@ -1119,7 +1083,7 @@ func (client *LDClient) JSONVariationDetailCtx( // // See the DataSourceStatusProvider interface for more about this functionality. func (client *LDClient) GetDataSourceStatusProvider() interfaces.DataSourceStatusProvider { - return client.dataSourceStatusProvider + return client.dataSystem.DataSourceStatusProvider() } // GetDataStoreStatusProvider returns an interface for tracking the status of a persistent data store. @@ -1131,7 +1095,7 @@ func (client *LDClient) GetDataSourceStatusProvider() interfaces.DataSourceStatu // // See the DataStoreStatusProvider interface for more about this functionality. func (client *LDClient) GetDataStoreStatusProvider() interfaces.DataStoreStatusProvider { - return client.dataStoreStatusProvider + return client.dataSystem.DataStoreStatusProvider() } // GetFlagTracker returns an interface for tracking changes in feature flag configurations. @@ -1279,15 +1243,15 @@ func (client *LDClient) evaluateInternal( return ldeval.Result{Detail: detail}, flag, err } - if !client.Initialized() { - if client.store.IsInitialized() { + if client.dataSystem.DataAvailability() != datasystem.Refreshed { + if client.dataSystem.DataAvailability() == datasystem.Cached { client.loggers.Warn("Feature flag evaluation called before LaunchDarkly client initialization completed; using last known values from data store") //nolint:lll } else { return evalErrorResult(ldreason.EvalErrorClientNotReady, nil, ErrClientNotInitialized) } } - itemDesc, storeErr := client.store.Get(datakinds.Features, key) + itemDesc, storeErr := client.dataSystem.Store().Get(datakinds.Features, key) if storeErr != nil { client.loggers.Errorf("Encountered error fetching feature from store: %+v", storeErr) diff --git a/ldclient_evaluation_benchmark_test.go b/ldclient_evaluation_benchmark_test.go index 677ff056..a40b28d0 100644 --- a/ldclient_evaluation_benchmark_test.go +++ b/ldclient_evaluation_benchmark_test.go @@ -46,7 +46,7 @@ func newEvalBenchmarkEnv() *evalBenchmarkEnv { func (env *evalBenchmarkEnv) setUp(withEventGeneration bool, bc evalBenchmarkCase, variations []ldvalue.Value) { // Set up the client. - env.client = makeTestClientWithConfig(func(c *Config) { + env.client = makeTestClientWithConfigAndStore(func(c *Config) { if withEventGeneration { // In this mode, we use a stub EventProcessor implementation that immediately discards // every event, but the SDK will still generate the events before passing them to the stub, @@ -58,15 +58,15 @@ func (env *evalBenchmarkEnv) setUp(withEventGeneration bool, bc evalBenchmarkCas // Events is set to the specific factory returned by NoEvents(). c.Events = ldcomponents.NoEvents() } + }, func(store subsystems.DataStore) { + // Set up the feature flag store. Note that we're using a regular in-memory data store, so the + // benchmarks will include the overhead of calling Get on the store. + testFlags := makeEvalBenchmarkFlags(bc, variations) + for _, ff := range testFlags { + _, _ = store.Upsert(datakinds.Features, ff.Key, sharedtest.FlagDescriptor(*ff)) + } }) - // Set up the feature flag store. Note that we're using a regular in-memory data store, so the - // benchmarks will include the overhead of calling Get on the store. - testFlags := makeEvalBenchmarkFlags(bc, variations) - for _, ff := range testFlags { - env.client.store.Upsert(datakinds.Features, ff.Key, sharedtest.FlagDescriptor(*ff)) - } - env.evalUser = makeEvalBenchmarkUser(bc) // Target a feature key in the middle of the list in case a linear search is being used. diff --git a/ldclient_test.go b/ldclient_test.go index 5e596bd4..5904c9f6 100644 --- a/ldclient_test.go +++ b/ldclient_test.go @@ -5,6 +5,8 @@ import ( "testing" "time" + "github.com/launchdarkly/go-server-sdk/v7/internal/datastore" + "github.com/launchdarkly/go-server-sdk/v7/internal/sharedtest/mocks" "github.com/launchdarkly/go-sdk-common/v3/lduser" @@ -77,9 +79,16 @@ func makeTestClient() *LDClient { } func makeTestClientWithConfig(modConfig func(*Config)) *LDClient { + return makeTestClientWithConfigAndStore(modConfig, func(store subsystems.DataStore) {}) +} + +// makeTestClientWithConfigAndStore is a variant of makeTestClientWithConfig, which allows you to also pre-populate the +// client's in-memory data store with some test data. The second argument is a callback that provides you access to the +// store; then you can call Upsert/Init to inject data. +func makeTestClientWithConfigAndStore(modConfig func(*Config), populate func(store subsystems.DataStore)) *LDClient { config := Config{ Offline: false, - DataStore: ldcomponents.InMemoryDataStore(), + DataStore: populateStore(populate), DataSource: mocks.DataSourceThatIsAlwaysInitialized(), Events: mocks.SingleComponentConfigurer[ldevents.EventProcessor]{Instance: &mocks.CapturingEventProcessor{}}, Logging: ldcomponents.Logging().Loggers(sharedtest.NewTestLoggers()), @@ -90,3 +99,14 @@ func makeTestClientWithConfig(modConfig func(*Config)) *LDClient { client, _ := MakeCustomClient(testSdkKey, config, time.Duration(0)) return client } + +// populateStore (which is a function) is defined here a type so that we can implement the ComponentConfigurer interface +// on it. That way, when the SDK configures the data store, we can hook in additional logic to populate the store +// via the callback provided in makeTestClientWithConfigAndStore. +type populateStore func(store subsystems.DataStore) + +func (populate populateStore) Build(context subsystems.ClientContext) (subsystems.DataStore, error) { + inMemory := datastore.NewInMemoryDataStore(context.GetLogging().Loggers) + populate(inMemory) + return inMemory, nil +} diff --git a/subsystems/data_store.go b/subsystems/data_store.go index 5781dff3..20c540b7 100644 --- a/subsystems/data_store.go +++ b/subsystems/data_store.go @@ -15,6 +15,8 @@ import ( type DataStore interface { io.Closer + ReadOnlyStore + // Init overwrites the store's contents with a set of items for each collection. // // All previous data should be discarded, regardless of versioning. @@ -24,21 +26,6 @@ type DataStore interface { // data, and then delete any previously stored items that were not in the input data. Init(allData []ldstoretypes.Collection) error - // Get retrieves an item from the specified collection, if available. - // - // If the specified key does not exist in the collection, it should return an ItemDescriptor - // whose Version is -1. - // - // If the item has been deleted and the store contains a placeholder, it should return an - // ItemDescriptor whose Version is the version of the placeholder, and whose Item is nil. - Get(kind ldstoretypes.DataKind, key string) (ldstoretypes.ItemDescriptor, error) - - // GetAll retrieves all items from the specified collection. - // - // If the store contains placeholders for deleted items, it should include them in the results, - // not filter them out. - GetAll(kind ldstoretypes.DataKind) ([]ldstoretypes.KeyedItemDescriptor, error) - // Upsert updates or inserts an item in the specified collection. For updates, the object will only be // updated if the existing version is less than the new version. // @@ -50,16 +37,6 @@ type DataStore interface { // contains an equal or greater version. Upsert(kind ldstoretypes.DataKind, key string, item ldstoretypes.ItemDescriptor) (bool, error) - // IsInitialized returns true if the data store contains a data set, meaning that Init has been - // called at least once. - // - // In a shared data store, it should be able to detect this even if Init was called in a - // different process: that is, the test should be based on looking at what is in the data store. - // Once this has been determined to be true, it can continue to return true without having to - // check the store again; this method should be as fast as possible since it may be called during - // feature flag evaluations. - IsInitialized() bool - // IsStatusMonitoringEnabled returns true if this data store implementation supports status // monitoring. // diff --git a/subsystems/ldstoreimpl/data_store_eval.go b/subsystems/ldstoreimpl/data_store_eval.go index 5f37ae04..d7cef4b7 100644 --- a/subsystems/ldstoreimpl/data_store_eval.go +++ b/subsystems/ldstoreimpl/data_store_eval.go @@ -16,6 +16,6 @@ import ( // // Normal use of the SDK does not require this type. It is provided for use by other LaunchDarkly // components that use DataStore and Evaluator separately from the SDK. -func NewDataStoreEvaluatorDataProvider(store subsystems.DataStore, loggers ldlog.Loggers) ldeval.DataProvider { +func NewDataStoreEvaluatorDataProvider(store subsystems.ReadOnlyStore, loggers ldlog.Loggers) ldeval.DataProvider { return datastore.NewDataStoreEvaluatorDataProviderImpl(store, loggers) } diff --git a/subsystems/read_only_store.go b/subsystems/read_only_store.go new file mode 100644 index 00000000..c1eefcbf --- /dev/null +++ b/subsystems/read_only_store.go @@ -0,0 +1,30 @@ +package subsystems + +import "github.com/launchdarkly/go-server-sdk/v7/subsystems/ldstoretypes" + +type ReadOnlyStore interface { + // Get retrieves an item from the specified collection, if available. + // + // If the specified key does not exist in the collection, it should return an ItemDescriptor + // whose Version is -1. + // + // If the item has been deleted and the store contains a placeholder, it should return an + // ItemDescriptor whose Version is the version of the placeholder, and whose Item is nil. + Get(kind ldstoretypes.DataKind, key string) (ldstoretypes.ItemDescriptor, error) + + // GetAll retrieves all items from the specified collection. + // + // If the store contains placeholders for deleted items, it should include them in the results, + // not filter them out. + GetAll(kind ldstoretypes.DataKind) ([]ldstoretypes.KeyedItemDescriptor, error) + + // IsInitialized returns true if the data store contains a data set, meaning that Init has been + // called at least once. + // + // In a shared data store, it should be able to detect this even if Init was called in a + // different process: that is, the test should be based on looking at what is in the data store. + // Once this has been determined to be true, it can continue to return true without having to + // check the store again; this method should be as fast as possible since it may be called during + // feature flag evaluations. + IsInitialized() bool +} From d09e32de998b8c6e6b97d5f83d53818f7a830638 Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Mon, 16 Sep 2024 15:22:39 -0700 Subject: [PATCH 2/2] lints --- internal/datasystem/data_availability.go | 1 + internal/datasystem/fdv1_datasystem.go | 18 ++++++++++++++++-- internal/datasystem/package.go | 5 +++++ ldclient.go | 10 ++++++---- ldclient_test.go | 4 ++-- subsystems/read_only_store.go | 2 ++ 6 files changed, 32 insertions(+), 8 deletions(-) create mode 100644 internal/datasystem/package.go diff --git a/internal/datasystem/data_availability.go b/internal/datasystem/data_availability.go index 630a10aa..5e0fecbd 100644 --- a/internal/datasystem/data_availability.go +++ b/internal/datasystem/data_availability.go @@ -1,5 +1,6 @@ package datasystem +// DataAvailability represents the availability of data in the SDK. type DataAvailability string const ( diff --git a/internal/datasystem/fdv1_datasystem.go b/internal/datasystem/fdv1_datasystem.go index 8c7cedee..abc649e2 100644 --- a/internal/datasystem/fdv1_datasystem.go +++ b/internal/datasystem/fdv1_datasystem.go @@ -9,6 +9,8 @@ import ( "github.com/launchdarkly/go-server-sdk/v7/subsystems" ) +// FDv1 implements the configuration and interactions between the SDK's data store, data source, and +// other related components. type FDv1 struct { dataSourceStatusBroadcaster *internal.Broadcaster[interfaces.DataSourceStatus] dataSourceStatusProvider interfaces.DataSourceStatusProvider @@ -20,7 +22,11 @@ type FDv1 struct { offline bool } -func NewFDv1(offline bool, dataStoreFactory subsystems.ComponentConfigurer[subsystems.DataStore], dataSourceFactory subsystems.ComponentConfigurer[subsystems.DataSource], clientContext *internal.ClientContextImpl) (*FDv1, error) { +// NewFDv1 creates a new FDv1 instance from data store and data source configurers. Offline determines if the +// client is in offline mode. If configuration is invalid, an error will be returned. +func NewFDv1(offline bool, dataStoreFactory subsystems.ComponentConfigurer[subsystems.DataStore], + dataSourceFactory subsystems.ComponentConfigurer[subsystems.DataSource], + clientContext *internal.ClientContextImpl) (*FDv1, error) { system := &FDv1{ dataSourceStatusBroadcaster: internal.NewBroadcaster[interfaces.DataSourceStatus](), dataStoreStatusBroadcaster: internal.NewBroadcaster[interfaces.DataStoreStatus](), @@ -63,7 +69,6 @@ func NewFDv1(offline bool, dataStoreFactory subsystems.ComponentConfigurer[subsy ) return system, nil - } func createDataSource( @@ -86,30 +91,37 @@ func createDataSource( return factory.Build(&contextCopy) } +//nolint:revive // Data system implementation. func (f *FDv1) DataSourceStatusBroadcaster() *internal.Broadcaster[interfaces.DataSourceStatus] { return f.dataSourceStatusBroadcaster } +//nolint:revive // Data system implementation. func (f *FDv1) DataSourceStatusProvider() interfaces.DataSourceStatusProvider { return f.dataSourceStatusProvider } +//nolint:revive // Data system implementation. func (f *FDv1) DataStoreStatusBroadcaster() *internal.Broadcaster[interfaces.DataStoreStatus] { return f.dataStoreStatusBroadcaster } +//nolint:revive // Data system implementation. func (f *FDv1) DataStoreStatusProvider() interfaces.DataStoreStatusProvider { return f.dataStoreStatusProvider } +//nolint:revive // Data system implementation. func (f *FDv1) FlagChangeEventBroadcaster() *internal.Broadcaster[interfaces.FlagChangeEvent] { return f.flagChangeEventBroadcaster } +//nolint:revive // Data system implementation. func (f *FDv1) Start(closeWhenReady chan struct{}) { f.dataSource.Start(closeWhenReady) } +//nolint:revive // Data system implementation. func (f *FDv1) Stop() error { if f.dataSource != nil { _ = f.dataSource.Close() @@ -129,6 +141,7 @@ func (f *FDv1) Stop() error { return nil } +//nolint:revive // Data system implementation. func (f *FDv1) DataAvailability() DataAvailability { if f.offline { return Defaults @@ -142,6 +155,7 @@ func (f *FDv1) DataAvailability() DataAvailability { return Defaults } +//nolint:revive // Data system implementation. func (f *FDv1) Store() subsystems.ReadOnlyStore { return f.dataStore } diff --git a/internal/datasystem/package.go b/internal/datasystem/package.go new file mode 100644 index 00000000..1410623d --- /dev/null +++ b/internal/datasystem/package.go @@ -0,0 +1,5 @@ +// Package datasystem encapsulates the interactions between the SDK's data store, data source, and other related +// components. +// Currently, there is only one data system implementation, FDv1, which represents the functionality of the SDK +// before the FDv2 protocol was introduced. +package datasystem diff --git a/ldclient.go b/ldclient.go index 39d92800..2dabf92b 100644 --- a/ldclient.go +++ b/ldclient.go @@ -67,8 +67,8 @@ const ( migrationVarExFuncName = "LDClient.MigrationVariationCtx" ) -// The dataSystem interface represents the requirements for the client to retrieve data necessary -// for evaluations, as well as the related status updates related to the data. +// dataSystem represents the requirements the client has for storing/retrieving/detecting changes related +// to the SDK's data model. type dataSystem interface { DataSourceStatusBroadcaster() *internal.Broadcaster[interfaces.DataSourceStatus] DataSourceStatusProvider() interfaces.DataSourceStatusProvider @@ -79,11 +79,14 @@ type dataSystem interface { // Start starts the data system; the given channel will be closed when the system has reached an initial state // (either permanently failed, e.g. due to bad auth, or succeeded, where Initialized() == true). Start(closeWhenReady chan struct{}) - // Stop halts the data system. Should be called when the client is closed to stop any long running operations. + + // Stop halts the data system. Should be called when the client is closed to stop any long-running operations. Stop() error + // Store returns a read-only accessor for the data model. Store() subsystems.ReadOnlyStore + // DataAvailability indicates what form of data is available. DataAvailability() datasystem.DataAvailability } @@ -279,7 +282,6 @@ func MakeCustomClient(sdkKey string, config Config, waitFor time.Duration) (*LDC ) } - // TODO: We can't actually pass STore() here because it wont' swap between the active ones. dataProvider := ldstoreimpl.NewDataStoreEvaluatorDataProvider(client.dataSystem.Store(), loggers) evalOptions := []ldeval.EvaluatorOption{ ldeval.EvaluatorOptionErrorLogger(client.loggers.ForLevel(ldlog.Error)), diff --git a/ldclient_test.go b/ldclient_test.go index 5904c9f6..ef82e3e7 100644 --- a/ldclient_test.go +++ b/ldclient_test.go @@ -100,8 +100,8 @@ func makeTestClientWithConfigAndStore(modConfig func(*Config), populate func(sto return client } -// populateStore (which is a function) is defined here a type so that we can implement the ComponentConfigurer interface -// on it. That way, when the SDK configures the data store, we can hook in additional logic to populate the store +// The populateStore type exist so that we can implement the ComponentConfigurer interface +// on it. When the SDK configures the data store, we can hook in additional logic to populate the store // via the callback provided in makeTestClientWithConfigAndStore. type populateStore func(store subsystems.DataStore) diff --git a/subsystems/read_only_store.go b/subsystems/read_only_store.go index c1eefcbf..ad3b79d8 100644 --- a/subsystems/read_only_store.go +++ b/subsystems/read_only_store.go @@ -2,6 +2,8 @@ package subsystems import "github.com/launchdarkly/go-server-sdk/v7/subsystems/ldstoretypes" +// ReadOnlyStore represents a read-only data store that can be used to retrieve +// any of the SDK's supported DataKinds. type ReadOnlyStore interface { // Get retrieves an item from the specified collection, if available. //