-
Notifications
You must be signed in to change notification settings - Fork 23
chore: FDv2 Cache Initializer #342
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
Changes from 22 commits
e044023
596bc97
563ec22
dbe5c65
70bd195
f856058
24c962f
e1617fe
7b5ed27
6aeea41
b8a50de
3f0066b
8f5353a
4e9746d
b9b1851
4a82402
e8caa9f
41ac382
9e51057
3050faf
2bdaf5a
5e9fa17
9fbd135
fc931a8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,75 @@ | ||
| package com.launchdarkly.sdk.android; | ||
|
|
||
| import androidx.annotation.NonNull; | ||
| import androidx.annotation.Nullable; | ||
|
|
||
| import com.launchdarkly.logging.LDLogger; | ||
| import com.launchdarkly.sdk.LDContext; | ||
| import com.launchdarkly.sdk.android.interfaces.ServiceEndpoints; | ||
| import com.launchdarkly.sdk.android.subsystems.DataSourceBuildInputs; | ||
| import com.launchdarkly.sdk.android.subsystems.HttpConfiguration; | ||
|
|
||
| import java.io.File; | ||
| import java.util.concurrent.ScheduledExecutorService; | ||
|
|
||
| /** | ||
| * Package-private subclass of {@link DataSourceBuildInputs} that carries additional | ||
| * internal-only dependencies not exposed in the public API. | ||
| * <p> | ||
| * This follows the same pattern as {@link ClientContextImpl} extending | ||
| * {@link com.launchdarkly.sdk.android.subsystems.ClientContext}: the public base class | ||
| * defines the stable contract for customer-implemented components, while this subclass | ||
| * adds SDK-internal properties that our built-in components can access via | ||
| * {@link #get(DataSourceBuildInputs)}. | ||
| * <p> | ||
| * This class is for internal SDK use only. It is not subject to any backwards | ||
| * compatibility guarantees. | ||
| */ | ||
| final class DataSourceBuildInputsInternal extends DataSourceBuildInputs { | ||
|
|
||
| @Nullable | ||
| private final PersistentDataStoreWrapper.ReadOnlyPerEnvironmentData perEnvironmentData; | ||
|
|
||
| DataSourceBuildInputsInternal( | ||
| LDContext evaluationContext, | ||
| ServiceEndpoints serviceEndpoints, | ||
| HttpConfiguration http, | ||
| boolean evaluationReasons, | ||
| SelectorSource selectorSource, | ||
| ScheduledExecutorService sharedExecutor, | ||
| @NonNull File cacheDir, | ||
| LDLogger baseLogger, | ||
| @Nullable PersistentDataStoreWrapper.ReadOnlyPerEnvironmentData perEnvironmentData | ||
| ) { | ||
| super(evaluationContext, serviceEndpoints, http, evaluationReasons, | ||
| selectorSource, sharedExecutor, cacheDir, baseLogger); | ||
| this.perEnvironmentData = perEnvironmentData; | ||
| } | ||
|
|
||
| /** | ||
| * Unwraps a {@link DataSourceBuildInputs} to obtain the internal subclass. | ||
| * If the instance is already a {@code DataSourceBuildInputsInternal}, it is | ||
| * returned directly. Otherwise a wrapper is created with null internal fields. | ||
| */ | ||
| static DataSourceBuildInputsInternal get(DataSourceBuildInputs inputs) { | ||
| if (inputs instanceof DataSourceBuildInputsInternal) { | ||
| return (DataSourceBuildInputsInternal) inputs; | ||
| } | ||
| return new DataSourceBuildInputsInternal( | ||
| inputs.getEvaluationContext(), | ||
| inputs.getServiceEndpoints(), | ||
| inputs.getHttp(), | ||
| inputs.isEvaluationReasons(), | ||
| inputs.getSelectorSource(), | ||
| inputs.getSharedExecutor(), | ||
| inputs.getCacheDir(), | ||
| inputs.getBaseLogger(), | ||
| null | ||
| ); | ||
| } | ||
|
|
||
| @Nullable | ||
| PersistentDataStoreWrapper.ReadOnlyPerEnvironmentData getPerEnvironmentDataIfAvailable() { | ||
| return perEnvironmentData; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,106 @@ | ||||||||||||||||||||
| package com.launchdarkly.sdk.android; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| import androidx.annotation.NonNull; | ||||||||||||||||||||
| import androidx.annotation.Nullable; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| import com.launchdarkly.logging.LDLogger; | ||||||||||||||||||||
| import com.launchdarkly.sdk.LDContext; | ||||||||||||||||||||
| import com.launchdarkly.sdk.android.DataModel.Flag; | ||||||||||||||||||||
| import com.launchdarkly.sdk.android.subsystems.FDv2SourceResult; | ||||||||||||||||||||
| import com.launchdarkly.sdk.android.subsystems.Initializer; | ||||||||||||||||||||
| import com.launchdarkly.sdk.fdv2.ChangeSet; | ||||||||||||||||||||
| import com.launchdarkly.sdk.fdv2.ChangeSetType; | ||||||||||||||||||||
| import com.launchdarkly.sdk.fdv2.Selector; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| import java.util.Collections; | ||||||||||||||||||||
| import java.util.Map; | ||||||||||||||||||||
| import java.util.concurrent.Future; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| /** | ||||||||||||||||||||
| * FDv2 cache initializer: loads persisted flag data from the local cache. | ||||||||||||||||||||
| * <p> | ||||||||||||||||||||
| * Per CONNMODE 4.1.2, a cache hit returns data with {@code persist=false} and | ||||||||||||||||||||
| * {@link Selector#EMPTY} (no selector). | ||||||||||||||||||||
| * <p> | ||||||||||||||||||||
| * All non-hit outcomes — cache miss, missing persistent store, and exceptions during | ||||||||||||||||||||
| * cache read — are returned as a {@link ChangeSetType#None} changeset, signaling | ||||||||||||||||||||
| * "no data available" rather than an error. A corrupt or unreadable cache is | ||||||||||||||||||||
| * semantically equivalent to an empty cache: neither provides usable data. | ||||||||||||||||||||
| * <p> | ||||||||||||||||||||
| * The cache read runs synchronously on the caller's thread because the underlying | ||||||||||||||||||||
| * {@code SharedPreferences} access is fast enough that executor dispatch overhead | ||||||||||||||||||||
| * would dominate the total time. | ||||||||||||||||||||
| */ | ||||||||||||||||||||
| final class FDv2CacheInitializer implements Initializer { | ||||||||||||||||||||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here's a summary of how the spec requirements and the js-core implementation were used when developing this class:
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| @Nullable | ||||||||||||||||||||
| private final PersistentDataStoreWrapper.ReadOnlyPerEnvironmentData envData; | ||||||||||||||||||||
| private final LDContext context; | ||||||||||||||||||||
| private final LDLogger logger; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| FDv2CacheInitializer( | ||||||||||||||||||||
| @Nullable PersistentDataStoreWrapper.ReadOnlyPerEnvironmentData envData, | ||||||||||||||||||||
| @NonNull LDContext context, | ||||||||||||||||||||
| @NonNull LDLogger logger | ||||||||||||||||||||
| ) { | ||||||||||||||||||||
| this.envData = envData; | ||||||||||||||||||||
| this.context = context; | ||||||||||||||||||||
| this.logger = logger; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| @Override | ||||||||||||||||||||
| @NonNull | ||||||||||||||||||||
| public Future<FDv2SourceResult> run() { | ||||||||||||||||||||
| FDv2SourceResult result; | ||||||||||||||||||||
| try { | ||||||||||||||||||||
| if (envData == null) { | ||||||||||||||||||||
| logger.debug("No persistent store configured; skipping cache"); | ||||||||||||||||||||
| result = FDv2SourceResult.changeSet(new ChangeSet<>( | ||||||||||||||||||||
| ChangeSetType.None, | ||||||||||||||||||||
| Selector.EMPTY, | ||||||||||||||||||||
| Collections.emptyMap(), | ||||||||||||||||||||
| null, | ||||||||||||||||||||
| false), false); | ||||||||||||||||||||
| } else { | ||||||||||||||||||||
| String hashedContextId = LDUtil.urlSafeBase64HashedContextId(context); | ||||||||||||||||||||
| EnvironmentData stored = envData.getContextData(hashedContextId); | ||||||||||||||||||||
| if (stored == null) { | ||||||||||||||||||||
| logger.debug("Cache miss for context"); | ||||||||||||||||||||
| result = FDv2SourceResult.changeSet(new ChangeSet<>( | ||||||||||||||||||||
| ChangeSetType.None, | ||||||||||||||||||||
| Selector.EMPTY, | ||||||||||||||||||||
| Collections.emptyMap(), | ||||||||||||||||||||
| null, | ||||||||||||||||||||
| false), false); | ||||||||||||||||||||
| } else { | ||||||||||||||||||||
| Map<String, Flag> flags = stored.getAll(); | ||||||||||||||||||||
| ChangeSet<Map<String, Flag>> changeSet = new ChangeSet<>( | ||||||||||||||||||||
| ChangeSetType.Full, | ||||||||||||||||||||
| Selector.EMPTY, | ||||||||||||||||||||
| flags, | ||||||||||||||||||||
| null, | ||||||||||||||||||||
| false); | ||||||||||||||||||||
| logger.debug("Cache hit: loaded {} flags for context", flags.size()); | ||||||||||||||||||||
| result = FDv2SourceResult.changeSet(changeSet, false); | ||||||||||||||||||||
| } | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
cursor[bot] marked this conversation as resolved.
|
||||||||||||||||||||
| } catch (Exception e) { | ||||||||||||||||||||
| logger.warn("Cache initializer failed: {}", e.toString()); | ||||||||||||||||||||
| result = FDv2SourceResult.changeSet(new ChangeSet<>( | ||||||||||||||||||||
| ChangeSetType.None, | ||||||||||||||||||||
| Selector.EMPTY, | ||||||||||||||||||||
| Collections.emptyMap(), | ||||||||||||||||||||
| null, | ||||||||||||||||||||
| false), false); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| LDAwaitFuture<FDv2SourceResult> future = new LDAwaitFuture<>(); | ||||||||||||||||||||
| future.set(result); | ||||||||||||||||||||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's a built-in future type other than LDAwaitFuture that's already complete. Composable futures were added in Android 24.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is still open.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Question about this. Our build.gradle files define the min SDK value at 21. Here's an example:
Since the CompletableFuture class was introduced in SDK version 24, I think the SDK will throw errors when it's run on API versions 21-23. Is this a big problem? What do you think about this? THose versions are pretty old and I wonder if we can update our min SDK value. |
||||||||||||||||||||
| return future; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| @Override | ||||||||||||||||||||
| public void close() { | ||||||||||||||||||||
| // No-op: the cache read runs synchronously in run(), so there is nothing to cancel. | ||||||||||||||||||||
| } | ||||||||||||||||||||
| } | ||||||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.