feat: Add support for waiting for non-cached values. (#160)

kinyoklion · web-flow · commit 28f7efa6128b · 2024-07-31T13:32:03.000-07:00
1. Allows for the common client to have customized data sources. This
was used for testing here, but could be used for more specialized
use-cases in the future.
2. Adds the ability for `start` and `identify` to wait for values from
LaunchDarkly instead of resolving with cached values when available.

With the previous wrapper SDK this behavior was controlled by the
underlying SDK and those would operate similar to the new, optional,
behavior.

For most uses cases it is both safer and higher performance to used
cached values when they are available instead of only using those cached
values as a fallback.

Ideally we would add new return values to start and identify. Next major
version we should consider:
`Cached`, `Timeout`. While retaining the current value for completing
via data from LD.
diff --git a/packages/common_client/lib/src/data_sources/data_source_manager.dart b/packages/common_client/lib/src/data_sources/data_source_manager.dart
@@ -41,7 +41,7 @@ final class DataSourceManager {
         _dataSourceEventHandler = dataSourceEventHandler;
 
   /// Set the available data source factories. These factories will not apply
-  /// until the next identify fall. Currently factories will be set once during
+  /// until the next identify call. Currently factories will be set once during
   /// startup and before the first identify.
   void setFactories(Map<ConnectionMode, DataSourceFactory> factories) {
     _dataSourceFactories.clear();
diff --git a/packages/common_client/lib/src/ld_common_client.dart b/packages/common_client/lib/src/ld_common_client.dart
@@ -45,6 +45,38 @@ final class IdentifyError implements IdentifyResult {
   IdentifyError(this.error);
 }
 
+typedef DataSourceFactoriesFn = Map<ConnectionMode, DataSourceFactory> Function(
+    LDCommonConfig config, LDLogger logger, HttpProperties httpProperties);
+
+Map<ConnectionMode, DataSourceFactory> _defaultFactories(
+    LDCommonConfig config, LDLogger logger, HttpProperties httpProperties) {
+  return {
+    ConnectionMode.streaming: (LDContext context) {
+      return StreamingDataSource(
+          credential: config.sdkCredential,
+          context: context,
+          endpoints: config.serviceEndpoints,
+          logger: logger,
+          dataSourceConfig: StreamingDataSourceConfig(
+              useReport: config.dataSourceConfig.useReport,
+              withReasons: config.dataSourceConfig.evaluationReasons),
+          httpProperties: httpProperties);
+    },
+    ConnectionMode.polling: (LDContext context) {
+      return PollingDataSource(
+          credential: config.sdkCredential,
+          context: context,
+          endpoints: config.serviceEndpoints,
+          logger: logger,
+          dataSourceConfig: PollingDataSourceConfig(
+              useReport: config.dataSourceConfig.useReport,
+              withReasons: config.dataSourceConfig.evaluationReasons,
+              pollingInterval: config.dataSourceConfig.polling.pollingInterval),
+          httpProperties: httpProperties);
+    },
+  };
+}
+
 final class LDCommonClient {
   final LDCommonConfig _config;
   final Persistence _persistence;
@@ -58,6 +90,7 @@ final class LDCommonClient {
   late final DataSourceManager _dataSourceManager;
   late final EnvironmentReport _envReport;
   late final AsyncSingleQueue<void> _identifyQueue = AsyncSingleQueue();
+  late final DataSourceFactoriesFn _dataSourceFactories;
 
   // Modifications will happen in the order they are specified in this list.
   // If there are cross-dependent modifiers, then this must be considered.
@@ -93,7 +126,8 @@ final class LDCommonClient {
   }
 
   LDCommonClient(LDCommonConfig commonConfig, CommonPlatform platform,
-      LDContext context, DiagnosticSdkData sdkData)
+      LDContext context, DiagnosticSdkData sdkData,
+      {DataSourceFactoriesFn? dataSourceFactories})
       : _config = commonConfig,
         _platform = platform,
         _persistence = ValidatingPersistence(
@@ -107,6 +141,8 @@ final class LDCommonClient {
             persistence: platform.persistence),
         _dataSourceStatusManager = DataSourceStatusManager(),
         _initialUndecoratedContext = context,
+        // Data source factories is primarily a mechanism for testing.
+        _dataSourceFactories = dataSourceFactories ?? _defaultFactories,
         _sdkData = sdkData {
     final dataSourceEventHandler = DataSourceEventHandler(
         flagManager: _flagManager,
@@ -179,7 +215,7 @@ final class LDCommonClient {
   ///
   /// If the return value is true, then the SDK has initialized, if false
   /// then the SDK has encountered an unrecoverable error.
-  Future<bool> start() {
+  Future<bool> start({bool waitForNetworkResults = false}) {
     if (_startFuture != null) {
       return _startFuture!.then(_mapIdentifyStart);
     }
@@ -191,7 +227,8 @@ final class LDCommonClient {
     // having been set resulting in a crash.
     _identifyQueue.execute(() async {
       await _startInternal();
-      await _identifyInternal(_initialUndecoratedContext);
+      await _identifyInternal(_initialUndecoratedContext,
+          waitForNetworkResults: waitForNetworkResults);
     }).then((res) {
       _startCompleter!.complete(_mapIdentifyResult(res));
     });
@@ -259,32 +296,8 @@ final class LDCommonClient {
     _updateEventSendingState();
 
     if (!_config.offline) {
-      _dataSourceManager.setFactories({
-        ConnectionMode.streaming: (LDContext context) {
-          return StreamingDataSource(
-              credential: _config.sdkCredential,
-              context: context,
-              endpoints: _config.serviceEndpoints,
-              logger: _logger,
-              dataSourceConfig: StreamingDataSourceConfig(
-                  useReport: _config.dataSourceConfig.useReport,
-                  withReasons: _config.dataSourceConfig.evaluationReasons),
-              httpProperties: httpProperties);
-        },
-        ConnectionMode.polling: (LDContext context) {
-          return PollingDataSource(
-              credential: _config.sdkCredential,
-              context: context,
-              endpoints: _config.serviceEndpoints,
-              logger: _logger,
-              dataSourceConfig: PollingDataSourceConfig(
-                  useReport: _config.dataSourceConfig.useReport,
-                  withReasons: _config.dataSourceConfig.evaluationReasons,
-                  pollingInterval:
-                      _config.dataSourceConfig.polling.pollingInterval),
-              httpProperties: httpProperties);
-        },
-      });
+      _dataSourceManager
+          .setFactories(_dataSourceFactories(_config, _logger, httpProperties));
     } else {
       _dataSourceManager.setFactories({
         ConnectionMode.streaming: (LDContext context) {
@@ -350,7 +363,8 @@ final class LDCommonClient {
   /// When the context is changed, the SDK will load flag values for the context from a local cache if available, while
   /// initiating a connection to retrieve the most current flag values. An event will be queued to be sent to the service
   /// containing the public [LDContext] fields for indexing on the dashboard.
-  Future<IdentifyResult> identify(LDContext context) async {
+  Future<IdentifyResult> identify(LDContext context,
+      {bool waitForNetworkResults = false}) async {
     if (_startFuture == null) {
       const message =
           'Identify called before SDK has been started. Start the SDK before '
@@ -359,7 +373,8 @@ final class LDCommonClient {
       return IdentifyError(Exception(message));
     }
     final res = await _identifyQueue.execute(() async {
-      await _identifyInternal(context);
+      await _identifyInternal(context,
+          waitForNetworkResults: waitForNetworkResults);
     });
     return _mapIdentifyResult(res);
   }
@@ -375,19 +390,19 @@ final class LDCommonClient {
     }
   }
 
-  Future<void> _identifyInternal(LDContext context) async {
+  Future<void> _identifyInternal(LDContext context,
+      {bool waitForNetworkResults = false}) async {
     await _setAndDecorateContext(context);
     final completer = Completer<void>();
     _eventProcessor?.processIdentifyEvent(IdentifyEvent(context: _context));
     final loadedFromCache = await _flagManager.loadCached(_context);
 
     if (_config.offline) {
-      // TODO: Do we need to do anything different here?
       return;
     }
     _dataSourceManager.identify(_context, completer);
 
-    if (loadedFromCache) {
+    if (loadedFromCache && !waitForNetworkResults) {
       return;
     }
     return completer.future;
diff --git a/packages/common_client/test/ld_dart_client_test.dart b/packages/common_client/test/ld_dart_client_test.dart
@@ -1,6 +1,13 @@
+import 'dart:async';
+import 'dart:convert';
+
+import 'package:crypto/crypto.dart';
 import 'package:launchdarkly_common_client/launchdarkly_common_client.dart';
+import 'package:launchdarkly_common_client/src/data_sources/data_source.dart';
 import 'package:test/test.dart';
 
+import 'mock_persistence.dart';
+
 final class TestConfig extends LDCommonConfig {
   TestConfig(super.sdkCredential, super.autoEnvAttributes,
       {super.applicationInfo,
@@ -13,6 +20,35 @@ final class TestConfig extends LDCommonConfig {
       super.dataSourceConfig});
 }
 
+final class TestDataSource implements DataSource {
+  final StreamController<DataSourceEvent> _eventController = StreamController();
+
+  @override
+  Stream<DataSourceEvent> get events => _eventController.stream;
+
+  @override
+  void restart() {}
+
+  @override
+  void start() {
+    Timer(Duration(milliseconds: 10), () {
+      _eventController.sink.add(DataEvent(
+          'put',
+          '{"flagA":{'
+              '"version":1,'
+              '"value":"datasource",'
+              '"variation":0,'
+              '"reason":{"kind":"OFF"}'
+              '}}'));
+    });
+  }
+
+  @override
+  void stop() {
+    _eventController.close();
+  }
+}
+
 void main() {
   group('given an offline client', () {
     late LDCommonClient client;
@@ -146,4 +182,105 @@ void main() {
       await client.flush();
     });
   });
+
+  group('given a mock data source', () {
+    late LDCommonClient client;
+    late MockPersistence mockPersistence;
+    final sdkKey = 'the-sdk-key';
+    final sdkKeyPersistence =
+        'LaunchDarkly_${sha256.convert(utf8.encode(sdkKey))}';
+
+    setUp(() {
+      mockPersistence = MockPersistence();
+      client = LDCommonClient(
+          TestConfig(sdkKey, AutoEnvAttributes.disabled),
+          CommonPlatform(persistence: mockPersistence),
+          LDContextBuilder().kind('user', 'bob').build(),
+          DiagnosticSdkData(name: '', version: ''), dataSourceFactories:
+              (LDCommonConfig config, LDLogger logger,
+                  HttpProperties properties) {
+        return {
+          ConnectionMode.streaming: (LDContext context) {
+            return TestDataSource();
+          },
+          ConnectionMode.polling: (LDContext context) {
+            return TestDataSource();
+          },
+        };
+      });
+    });
+
+    test('identify can resolve cached values', () async {
+      final contextPersistenceKey =
+          sha256.convert(utf8.encode('joe')).toString();
+      mockPersistence.storage[sdkKeyPersistence] = {
+        contextPersistenceKey: '{"flagA":{'
+            '"version":1,'
+            '"value":"storage",'
+            '"variation":0,'
+            '"reason":{"kind":"OFF"}'
+            '}}'
+      };
+      // We are going to ignore the items for the first context.
+      await client.start();
+
+      await client.identify(LDContextBuilder().kind('user', 'joe').build());
+      final res = client.stringVariation('flagA', 'default');
+      expect(res, 'storage');
+    });
+
+    test('identify can resolve non-cached values', () async {
+      final contextPersistenceKey =
+          sha256.convert(utf8.encode('joe')).toString();
+      mockPersistence.storage[sdkKeyPersistence] = {
+        contextPersistenceKey: '{"flagA":{'
+            '"version":1,'
+            '"value":"storage",'
+            '"variation":0,'
+            '"reason":{"kind":"OFF"}'
+            '}}'
+      };
+      // We are going to ignore the items for the first context.
+      await client.start();
+
+      await client.identify(LDContextBuilder().kind('user', 'joe').build(),
+          waitForNetworkResults: true);
+      final res = client.stringVariation('flagA', 'default');
+      expect(res, 'datasource');
+    });
+
+    test('start can resolve cached values', () async {
+      final contextPersistenceKey =
+          sha256.convert(utf8.encode('bob')).toString();
+      mockPersistence.storage[sdkKeyPersistence] = {
+        contextPersistenceKey: '{"flagA":{'
+            '"version":1,'
+            '"value":"storage",'
+            '"variation":0,'
+            '"reason":{"kind":"OFF"}'
+            '}}'
+      };
+
+      await client.start();
+      final res = client.stringVariation('flagA', 'default');
+      expect(res, 'storage');
+    });
+
+    test('start can resolve non-cached values', () async {
+      final contextPersistenceKey =
+          sha256.convert(utf8.encode('bob')).toString();
+      mockPersistence.storage[sdkKeyPersistence] = {
+        contextPersistenceKey: '{"flagA":{'
+            '"version":1,'
+            '"value":"storage",'
+            '"variation":0,'
+            '"reason":{"kind":"OFF"}'
+            '}}'
+      };
+
+      await client.start(waitForNetworkResults: true);
+      final res = client.stringVariation('flagA', 'default');
+      expect(res, 'datasource');
+    });
+  });
 }
diff --git a/packages/flutter_client_sdk/lib/src/ld_client.dart b/packages/flutter_client_sdk/lib/src/ld_client.dart
@@ -107,8 +107,18 @@ interface class LDClient {
   /// ```dart
   /// await client.start().timeout(const Duration(seconds: 30));
   /// ```
-  Future<bool> start() async {
-    return _client.start();
+  /// The [waitForNetworkResults] parameters, when true, indicates that the SDK
+  /// will attempt to wait for values from LaunchDarkly instead of depending
+  /// on cached values. The cached values will still be loaded, but the future
+  /// returned by this function will not resolve as a result of those cached
+  /// values being loaded. Generally this option should NOT be used and instead
+  /// flag changes should be listened to. It the client is set to offline mode,
+  /// then this option is ignored.
+  ///
+  /// If [waitForNetworkResults] is true, and an error is encountered, then
+  /// false may be returned even if cached values were loaded.
+  Future<bool> start({bool waitForNetworkResults = false}) async {
+    return _client.start(waitForNetworkResults: waitForNetworkResults);
   }
 
   /// Changes the active context.
@@ -119,10 +129,24 @@ interface class LDClient {
   /// service containing the public [LDContext] fields for indexing on the
   /// dashboard.
   ///
+  /// A context with the same kinds and same keys will use the same cached
+  /// context.
+  ///
   /// This returned future can be awaited to wait for the identify process to
   /// be complete. As with [start] this can take an extended period if there
   /// is not network availability, so a timeout is recommended.
   ///
+  /// The [waitForNetworkResults] parameters, when true, indicates that the SDK
+  /// will attempt to wait for values from LaunchDarkly instead of depending
+  /// on cached values. The cached values will still be loaded, but the future
+  /// returned by this function will not resolve as a result of those cached
+  /// values being loaded. Generally this option should NOT be used and instead
+  /// flag changes should be listened to. It the client is set to offline mode,
+  /// then this option is ignored.
+  ///
+  /// If [waitForNetworkResults] is true, and an error is encountered, then
+  /// [IdentifyError] may be returned even if cached values were loaded.
+  ///
   /// The identify will complete with 1 of three possible values:
   /// [IdentifyComplete], [IdentifySuperseded], or [IdentifyError].
   ///
@@ -139,8 +163,10 @@ interface class LDClient {
   ///
   /// [IdentifyError] this means that the identify has permanently failed. For
   /// instance the SDK key is no longer valid.
-  Future<IdentifyResult> identify(LDContext context) async {
-    return _client.identify(context);
+  Future<IdentifyResult> identify(LDContext context,
+      {bool waitForNetworkResults = false}) async {
+    return _client.identify(context,
+        waitForNetworkResults: waitForNetworkResults);
   }
 
   /// Track custom events associated with the current context for data export or
