-
Notifications
You must be signed in to change notification settings - Fork 19
feat: Add support for waiting for non-cached values. #160
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.
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
kinyoklion
merged 6 commits into
main
from
rlamb/sc-251068/identify-optionally-non-cached
Jul 31, 2024
Merged
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
f57f29b
feat: Add support for waiting for non-cached values.
kinyoklion a120edf
Undo formatting in client.
kinyoklion f1a094d
Remove comment.
kinyoklion 1844322
Format files.
kinyoklion 08bd136
Change name and add some more comments.
kinyoklion cc25480
Remove duplicate line.
kinyoklion File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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,15 @@ 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() { | ||
/// | ||
/// The [waitForNonCachedValues] 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. Generally this | ||
/// option should NOT be used and instead flag changes should be listened to. | ||
/// If [waitForNonCachedValues] is true, and an error is encountered, then | ||
/// false may be returned even if cached values were loaded. | ||
Future<bool> start({bool waitForNonCachedValues = false}) { | ||
if (_startFuture != null) { | ||
return _startFuture!.then(_mapIdentifyStart); | ||
} | ||
|
@@ -191,7 +235,8 @@ final class LDCommonClient { | |
// having been set resulting in a crash. | ||
_identifyQueue.execute(() async { | ||
await _startInternal(); | ||
await _identifyInternal(_initialUndecoratedContext); | ||
await _identifyInternal(_initialUndecoratedContext, | ||
waitForNonCachedValues: waitForNonCachedValues); | ||
}).then((res) { | ||
_startCompleter!.complete(_mapIdentifyResult(res)); | ||
}); | ||
|
@@ -259,32 +304,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 +371,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 waitForNonCachedValues = false}) async { | ||
if (_startFuture == null) { | ||
const message = | ||
'Identify called before SDK has been started. Start the SDK before ' | ||
|
@@ -359,7 +381,8 @@ final class LDCommonClient { | |
return IdentifyError(Exception(message)); | ||
} | ||
final res = await _identifyQueue.execute(() async { | ||
await _identifyInternal(context); | ||
await _identifyInternal(context, | ||
waitForNonCachedValues: waitForNonCachedValues); | ||
}); | ||
return _mapIdentifyResult(res); | ||
} | ||
|
@@ -375,19 +398,19 @@ final class LDCommonClient { | |
} | ||
} | ||
|
||
Future<void> _identifyInternal(LDContext context) async { | ||
Future<void> _identifyInternal(LDContext context, | ||
{bool waitForNonCachedValues = 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 && !waitForNonCachedValues) { | ||
|
||
return; | ||
} | ||
return completer.future; | ||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, if you want to use cached values on app start, but not on identify, then this supports that.
If you want to identify multiple contexts sequentially, and you want to ensure your cache gets updated, you can do that. You can also choose to use the cached values for the last context, being as that one would be updated in the background.
If you want to ensure you have "fresh" values when navigating to specific sections of your app, you could do that.
What this does not solve is the situation where someone wants to identify multiple contexts sequentially, wants the cache to be updated, but doesn't want the performance penalty. (One request was basically to allow the identify to continue and update the cache in the background, but concurrently start identifying a new context.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you want flag values to "not change", then you can evaluate all relevant flags after the identify and use those values.
Alternatively, you could set the SDK to offline after completing the identify, and then only go online again when you want to specifically update values.