From 08dc14c346978d0033f44825b48989670d6d0b56 Mon Sep 17 00:00:00 2001 From: Todd Anderson Date: Tue, 19 Nov 2024 11:31:38 -0600 Subject: [PATCH 1/7] feat: honor polling interval across process restarts --- .../sdk/android/ClientContextImpl.java | 23 ++++++++--- .../sdk/android/ComponentsImpl.java | 37 ++++++++++++------ .../sdk/android/ContextDataManager.java | 6 +++ .../launchdarkly/sdk/android/LDClient.java | 5 ++- .../sdk/android/PollingDataSource.java | 8 ++-- .../sdk/android/ConnectivityManagerTest.java | 3 +- .../android/ContextDataManagerTestBase.java | 3 +- .../sdk/android/DiagnosticConfigTest.java | 2 +- .../sdk/android/LDConfigTest.java | 4 +- .../sdk/android/PollingDataSourceTest.java | 38 +++++++++++++++++-- .../sdk/android/StreamingDataSourceTest.java | 15 +++++--- 11 files changed, 107 insertions(+), 37 deletions(-) diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ClientContextImpl.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ClientContextImpl.java index 134b04dc..ac0cb66f 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ClientContextImpl.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ClientContextImpl.java @@ -32,21 +32,25 @@ final class ClientContextImpl extends ClientContext { private final FeatureFetcher fetcher; private final PlatformState platformState; private final TaskExecutor taskExecutor; + private final PersistentDataStoreWrapper.PerEnvironmentData perEnvironmentData; ClientContextImpl( ClientContext base, DiagnosticStore diagnosticStore, FeatureFetcher fetcher, PlatformState platformState, - TaskExecutor taskExecutor + TaskExecutor taskExecutor, + PersistentDataStoreWrapper.PerEnvironmentData perEnvironmentData ) { super(base); this.diagnosticStore = diagnosticStore; this.fetcher = fetcher; this.platformState = platformState; this.taskExecutor = taskExecutor; + this.perEnvironmentData = perEnvironmentData; } + // TODO: consider re-ordering so perEnvironmentData is earlier in list of params static ClientContextImpl fromConfig( LDConfig config, String mobileKey, @@ -56,7 +60,8 @@ static ClientContextImpl fromConfig( LDLogger logger, PlatformState platformState, IEnvironmentReporter environmentReporter, - TaskExecutor taskExecutor + TaskExecutor taskExecutor, + PersistentDataStoreWrapper.PerEnvironmentData perEnvironmentData ) { boolean initiallyInBackground = platformState != null && !platformState.isForeground(); ClientContext minimalContext = new ClientContext(mobileKey, environmentReporter, logger, config, @@ -82,14 +87,14 @@ static ClientContextImpl fromConfig( if (!config.getDiagnosticOptOut()) { diagnosticStore = new DiagnosticStore(EventUtil.makeDiagnosticParams(baseClientContext)); } - return new ClientContextImpl(baseClientContext, diagnosticStore, fetcher, platformState, taskExecutor); + return new ClientContextImpl(baseClientContext, diagnosticStore, fetcher, platformState, taskExecutor, perEnvironmentData); } public static ClientContextImpl get(ClientContext context) { if (context instanceof ClientContextImpl) { return (ClientContextImpl)context; } - return new ClientContextImpl(context, null, null, null, null); + return new ClientContextImpl(context, null, null, null, null, null); } public static ClientContextImpl forDataSource( @@ -119,7 +124,8 @@ public static ClientContextImpl forDataSource( baseContextImpl.getDiagnosticStore(), baseContextImpl.getFetcher(), baseContextImpl.getPlatformState(), - baseContextImpl.getTaskExecutor() + baseContextImpl.getTaskExecutor(), + baseContextImpl.getPerEnvironmentData() ); } @@ -134,7 +140,8 @@ public ClientContextImpl setEvaluationContext(LDContext context) { this.diagnosticStore, this.fetcher, this.platformState, - this.taskExecutor + this.taskExecutor, + this.perEnvironmentData ); } @@ -154,6 +161,10 @@ public TaskExecutor getTaskExecutor() { return throwExceptionIfNull(taskExecutor); } + public PersistentDataStoreWrapper.PerEnvironmentData getPerEnvironmentData() { + return throwExceptionIfNull(perEnvironmentData); + } + private static T throwExceptionIfNull(T o) { if (o == null) { throw new IllegalStateException( diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ComponentsImpl.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ComponentsImpl.java index 605d98c3..489b2214 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ComponentsImpl.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ComponentsImpl.java @@ -244,30 +244,45 @@ static final class PollingDataSourceBuilderImpl extends PollingDataSourceBuilder implements DiagnosticDescription, DataSourceRequiresFeatureFetcher { @Override public DataSource build(ClientContext clientContext) { - clientContext.getDataSourceUpdateSink().setStatus( - clientContext.isInBackground() ? ConnectionInformation.ConnectionMode.BACKGROUND_POLLING : + ClientContextImpl clientContextImpl = ClientContextImpl.get(clientContext); + clientContextImpl.getDataSourceUpdateSink().setStatus( + clientContextImpl.isInBackground() ? ConnectionInformation.ConnectionMode.BACKGROUND_POLLING : ConnectionInformation.ConnectionMode.POLLING, null ); - int actualPollIntervalMillis = clientContext.isInBackground() ? backgroundPollIntervalMillis : + + int pollInterval = clientContextImpl.isInBackground() ? backgroundPollIntervalMillis : pollIntervalMillis; - int initialDelayMillis; + + long initialDelayMillis; if (clientContext.isInBackground() && Boolean.FALSE.equals(clientContext.getPreviouslyInBackground())) { // If we're transitioning from foreground to background, then we don't want to do a // poll right away because we already have recent flag data. Start polling *after* // the first background poll interval. initialDelayMillis = backgroundPollIntervalMillis; } else { - // If we're in the foreground-- or, if we're in the background but we started out - // that way rather than transitioning-- then we should do the first poll right away. - initialDelayMillis = 0; + // TODO: refactor context hashing calls to use a common util + String hashed = new ContextHasher().hash(clientContextImpl.getEvaluationContext().getFullyQualifiedKey()); + + long lastUpdated = 0; // assume last updated is beginning of time + for (ContextIndex.IndexEntry entry : clientContextImpl.getPerEnvironmentData().getIndex().data) { + if (entry.contextId.equals(hashed)) { + lastUpdated = entry.timestamp; + } + } + + // To avoid unnecessarily frequent polling requests due to process or application lifecycle, we have added + // this initial delay logic. Calculate how much time has passed since the last update, if that is less than + // the polling interval, delay by the difference, otherwise 0 delay. + long elapsedSinceUpdate = System.currentTimeMillis() - lastUpdated; + initialDelayMillis = Math.max(pollInterval - elapsedSinceUpdate, 0); } - ClientContextImpl clientContextImpl = ClientContextImpl.get(clientContext); + return new PollingDataSource( - clientContext.getEvaluationContext(), - clientContext.getDataSourceUpdateSink(), + clientContextImpl.getEvaluationContext(), + clientContextImpl.getDataSourceUpdateSink(), initialDelayMillis, - actualPollIntervalMillis, + pollInterval, clientContextImpl.getFetcher(), clientContextImpl.getPlatformState(), clientContextImpl.getTaskExecutor(), diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ContextDataManager.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ContextDataManager.java index e997f033..c7695319 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ContextDataManager.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ContextDataManager.java @@ -243,6 +243,12 @@ public boolean upsert(@NonNull LDContext context, @NonNull Flag flag) { String contextId = hashedContextId(context); environmentStore.setContextData(contextId, updatedFlags); + + if (index == null) { + index = environmentStore.getIndex(); + } + index = index.updateTimestamp(contextId, System.currentTimeMillis()); + environmentStore.setIndex(index); } Collection updatedFlag = Collections.singletonList(flag.getKey()); diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClient.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClient.java index 0129032e..c9a38e17 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClient.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClient.java @@ -330,7 +330,7 @@ protected LDClient( FeatureFetcher fetcher = null; if (config.dataSource instanceof ComponentsImpl.DataSourceRequiresFeatureFetcher) { ClientContextImpl minimalContext = ClientContextImpl.fromConfig(config, mobileKey, - environmentName, null, initialContext, logger, platformState, environmentReporter, taskExecutor + environmentName, null, initialContext, logger, platformState, environmentReporter, taskExecutor, environmentStore ); fetcher = new HttpFeatureFlagFetcher(minimalContext); } @@ -344,7 +344,8 @@ protected LDClient( logger, platformState, environmentReporter, - taskExecutor + taskExecutor, + environmentStore ); this.contextDataManager = new ContextDataManager( diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/PollingDataSource.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/PollingDataSource.java index 371d31f0..c50a527d 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/PollingDataSource.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/PollingDataSource.java @@ -20,8 +20,8 @@ final class PollingDataSource implements DataSource { private final LDContext currentContext; private final DataSourceUpdateSink dataSourceUpdateSink; - final int initialDelayMillis; // visible for testing - final int pollIntervalMillis; // visible for testing + final long initialDelayMillis; // visible for testing + final long pollIntervalMillis; // visible for testing private final FeatureFetcher fetcher; private final PlatformState platformState; private final TaskExecutor taskExecutor; @@ -32,8 +32,8 @@ final class PollingDataSource implements DataSource { PollingDataSource( LDContext currentContext, DataSourceUpdateSink dataSourceUpdateSink, - int initialDelayMillis, - int pollIntervalMillis, + long initialDelayMillis, + long pollIntervalMillis, FeatureFetcher fetcher, PlatformState platformState, TaskExecutor taskExecutor, diff --git a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/ConnectivityManagerTest.java b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/ConnectivityManagerTest.java index 290a90d1..cbfadae5 100644 --- a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/ConnectivityManagerTest.java +++ b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/ConnectivityManagerTest.java @@ -116,7 +116,8 @@ private void createTestManager( logging.logger, mockPlatformState, environmentReporter, - taskExecutor + taskExecutor, + environmentStore ); contextDataManager = new ContextDataManager( diff --git a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/ContextDataManagerTestBase.java b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/ContextDataManagerTestBase.java index d403a2ac..f08895fd 100644 --- a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/ContextDataManagerTestBase.java +++ b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/ContextDataManagerTestBase.java @@ -55,7 +55,8 @@ protected ContextDataManager createDataManager(int maxCachedContexts) { logging.logger, null, environmentReporter, - taskExecutor + taskExecutor, + environmentStore ); return new ContextDataManager( clientContext, diff --git a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/DiagnosticConfigTest.java b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/DiagnosticConfigTest.java index ed9507e4..0c7f49d8 100644 --- a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/DiagnosticConfigTest.java +++ b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/DiagnosticConfigTest.java @@ -142,7 +142,7 @@ public void customDiagnosticConfigurationHttp() throws Exception { private static LDValue makeDiagnosticJson(LDConfig config) throws Exception { ClientContext clientContext = ClientContextImpl.fromConfig(config, "", "", - null, null, LDLogger.none(), null, new EnvironmentReporterBuilder().build(), null); + null, null, LDLogger.none(), null, new EnvironmentReporterBuilder().build(), null, null); DiagnosticStore.SdkDiagnosticParams params = EventUtil.makeDiagnosticParams(clientContext); DiagnosticStore diagnosticStore = new DiagnosticStore(params); MockDiagnosticEventSender mockSender = new MockDiagnosticEventSender(); diff --git a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/LDConfigTest.java b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/LDConfigTest.java index f5328cd3..a890bb37 100644 --- a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/LDConfigTest.java +++ b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/LDConfigTest.java @@ -75,7 +75,7 @@ Map headersToMap(Headers headers) { public void headersForEnvironment() { LDConfig config = new LDConfig.Builder(AutoEnvAttributes.Disabled).mobileKey("test-key").build(); ClientContext clientContext = ClientContextImpl.fromConfig(config, "test-key", "", - null, null, null, null, new EnvironmentReporterBuilder().build(), null); + null, null, null, null, new EnvironmentReporterBuilder().build(), null, null); Map headers = headersToMap( LDUtil.makeHttpProperties(clientContext).toHeadersBuilder().build() ); @@ -96,7 +96,7 @@ public void headersForEnvironmentWithTransform() { })) .build(); ClientContext clientContext = ClientContextImpl.fromConfig(config, "test-key", "", - null, null, null, null, new EnvironmentReporterBuilder().build(), null); + null, null, null, null, new EnvironmentReporterBuilder().build(), null, null); expected.put("User-Agent", LDUtil.USER_AGENT_HEADER_VALUE); expected.put("Authorization", "api_key test-key"); diff --git a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/PollingDataSourceTest.java b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/PollingDataSourceTest.java index c022a8c3..aa720507 100644 --- a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/PollingDataSourceTest.java +++ b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/PollingDataSourceTest.java @@ -3,6 +3,7 @@ import static com.launchdarkly.sdk.android.AssertHelpers.requireNoMoreValues; import static com.launchdarkly.sdk.android.AssertHelpers.requireValue; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; import com.launchdarkly.sdk.LDContext; import com.launchdarkly.sdk.LDValue; @@ -15,6 +16,7 @@ import com.launchdarkly.sdk.android.subsystems.DataSource; import org.easymock.EasyMockSupport; +import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -24,8 +26,9 @@ import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.TimeUnit; -public class PollingDataSourceTest extends EasyMockSupport { +public class PollingDataSourceTest { private static final LDContext CONTEXT = LDContext.create("context-key"); + private static final String MOBILE_KEY = "test-mobile-key"; private static final LDConfig EMPTY_CONFIG = new LDConfig.Builder(AutoEnvAttributes.Disabled).build(); private final MockComponents.MockDataSourceUpdateSink dataSourceUpdateSink = new MockComponents.MockDataSourceUpdateSink(); @@ -34,14 +37,20 @@ public class PollingDataSourceTest extends EasyMockSupport { private final IEnvironmentReporter environmentReporter = new EnvironmentReporterBuilder().build(); private final SimpleTestTaskExecutor taskExecutor = new SimpleTestTaskExecutor(); + private PersistentDataStoreWrapper.PerEnvironmentData perEnvironmentData; @Rule public LogCaptureRule logging = new LogCaptureRule(); - private ClientContext makeClientContext(boolean inBackground, Boolean previouslyInBackground) { - ClientContext baseClientContext = ClientContextImpl.fromConfig( + @Before + public void before() { + perEnvironmentData = TestUtil.makeSimplePersistentDataStoreWrapper().perEnvironmentData(MOBILE_KEY); + } + + private ClientContextImpl makeClientContext(boolean inBackground, Boolean previouslyInBackground) { + ClientContextImpl baseClientContext = ClientContextImpl.fromConfig( EMPTY_CONFIG, "", "", fetcher, CONTEXT, - logging.logger, platformState, environmentReporter, taskExecutor); + logging.logger, platformState, environmentReporter, taskExecutor, perEnvironmentData); return ClientContextImpl.forDataSource( baseClientContext, dataSourceUpdateSink, @@ -115,6 +124,27 @@ public void firstPollIsImmediateWhenStartingInBackground() throws Exception { } } + @Test + public void pollingIntervalHonoredAcrossMultipleBuildCalls() throws Exception { + ClientContextImpl clientContext = makeClientContext(true, null); + PollingDataSourceBuilder builder = Components.pollingDataSource() + .pollIntervalMillis(100000) + .backgroundPollIntervalMillis(100000); + + // first build should have no delay + PollingDataSource ds1 = (PollingDataSource) builder.build(clientContext); + assertEquals(0, ds1.initialDelayMillis); + + // simulate successful update of context index timestamp + String hash = new ContextHasher().hash(CONTEXT.getFullyQualifiedKey()); + ContextIndex newIndex = clientContext.getPerEnvironmentData().getIndex().updateTimestamp(hash, System.currentTimeMillis()); + clientContext.getPerEnvironmentData().setIndex(newIndex); + + // second build should have a non-zero delay due to simulated response storing a recent timestamp + PollingDataSource ds2 = (PollingDataSource) builder.build(clientContext); + assertNotEquals(0, ds2.initialDelayMillis); + } + @Test public void firstPollHappensAfterBackgroundPollingIntervalWhenTransitioningToBackground() throws Exception { ClientContext clientContext = makeClientContext(true, false); diff --git a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/StreamingDataSourceTest.java b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/StreamingDataSourceTest.java index 8346dee5..e9ac2795 100644 --- a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/StreamingDataSourceTest.java +++ b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/StreamingDataSourceTest.java @@ -11,6 +11,7 @@ import com.launchdarkly.sdk.android.subsystems.ClientContext; import com.launchdarkly.sdk.android.subsystems.DataSource; +import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -23,20 +24,24 @@ public class StreamingDataSourceTest { // the tests here cover other aspects of how the streaming data source component behaves. private static final LDContext CONTEXT = LDContext.create("context-key"); - + private static final String MOBILE_KEY = "test-mobile-key"; @Rule public LogCaptureRule logging = new LogCaptureRule(); - private final MockComponents.MockDataSourceUpdateSink dataSourceUpdateSink = new MockComponents.MockDataSourceUpdateSink(); private final MockPlatformState platformState = new MockPlatformState(); - private final IEnvironmentReporter environmentReporter = new EnvironmentReporterBuilder().build(); private final SimpleTestTaskExecutor taskExecutor = new SimpleTestTaskExecutor(); + private PersistentDataStoreWrapper.PerEnvironmentData perEnvironmentData; + + @Before + public void before() { + perEnvironmentData = TestUtil.makeSimplePersistentDataStoreWrapper().perEnvironmentData(MOBILE_KEY); + } private ClientContext makeClientContext(boolean inBackground, Boolean previouslyInBackground) { ClientContext baseClientContext = ClientContextImpl.fromConfig( new LDConfig.Builder(AutoEnvAttributes.Disabled).build(), "", "", null, CONTEXT, - logging.logger, platformState, environmentReporter, taskExecutor); + logging.logger, platformState, environmentReporter, taskExecutor, perEnvironmentData); return ClientContextImpl.forDataSource( baseClientContext, dataSourceUpdateSink, @@ -53,7 +58,7 @@ private ClientContext makeClientContext(boolean inBackground, Boolean previously private ClientContext makeClientContextWithFetcher() { ClientContext baseClientContext = ClientContextImpl.fromConfig( new LDConfig.Builder(AutoEnvAttributes.Disabled).build(), "", "", makeFeatureFetcher(), CONTEXT, - logging.logger, platformState, environmentReporter, taskExecutor); + logging.logger, platformState, environmentReporter, taskExecutor, perEnvironmentData); return ClientContextImpl.forDataSource( baseClientContext, dataSourceUpdateSink, From 99c99a26214ad9793256234dcbf8c1143f29ead1 Mon Sep 17 00:00:00 2001 From: Todd Anderson Date: Wed, 20 Nov 2024 16:16:22 -0600 Subject: [PATCH 2/7] adding fingerprinting --- .../sdk/android/ComponentsImpl.java | 15 ++++--- .../sdk/android/ConnectivityManager.java | 2 +- .../sdk/android/ContextDataManager.java | 30 +++++--------- .../sdk/android/ContextHasher.java | 30 -------------- .../sdk/android/HttpFeatureFlagFetcher.java | 2 +- .../com/launchdarkly/sdk/android/LDUtil.java | 10 ++++- .../android/PersistentDataStoreWrapper.java | 41 +++++++++++++++++-- .../sdk/android/StreamingDataSource.java | 2 +- .../android/ContextDataManagerTestBase.java | 4 +- .../PersistentDataStoreWrapperTest.java | 5 ++- .../sdk/android/PollingDataSourceTest.java | 10 +++-- .../launchdarkly/sdk/android/TestUtil.java | 4 +- 12 files changed, 79 insertions(+), 76 deletions(-) delete mode 100644 launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ContextHasher.java diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ComponentsImpl.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ComponentsImpl.java index 489b2214..3561c3e3 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ComponentsImpl.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ComponentsImpl.java @@ -261,14 +261,13 @@ public DataSource build(ClientContext clientContext) { // the first background poll interval. initialDelayMillis = backgroundPollIntervalMillis; } else { - // TODO: refactor context hashing calls to use a common util - String hashed = new ContextHasher().hash(clientContextImpl.getEvaluationContext().getFullyQualifiedKey()); - - long lastUpdated = 0; // assume last updated is beginning of time - for (ContextIndex.IndexEntry entry : clientContextImpl.getPerEnvironmentData().getIndex().data) { - if (entry.contextId.equals(hashed)) { - lastUpdated = entry.timestamp; - } + // get the last updated timestamp for this context + PersistentDataStoreWrapper.PerEnvironmentData perEnvironmentData = clientContextImpl.getPerEnvironmentData(); + String hashedContextId = LDUtil.urlSafeBase64HashedContextId(clientContextImpl.getEvaluationContext()); + String fingerprint = LDUtil.urlSafeBase64Hash(clientContextImpl.getEvaluationContext()); + Long lastUpdated = perEnvironmentData.getLastUpdated(hashedContextId, fingerprint); + if (lastUpdated == null) { + lastUpdated = 0L; // default to beginning of time } // To avoid unnecessarily frequent polling requests due to process or application lifecycle, we have added diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ConnectivityManager.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ConnectivityManager.java index f99cb042..b6dc6915 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ConnectivityManager.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ConnectivityManager.java @@ -447,7 +447,7 @@ public void onSuccess(String flagsJson) { @Override public void onError(Throwable e) { logger.error("Error when attempting to get flag data: [{}] [{}]: {}", - LDUtil.base64Url(contextToFetch), + LDUtil.urlSafeBase64(contextToFetch), contextToFetch, LogValues.exceptionSummary(e)); resultCallback.onError(e); diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ContextDataManager.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ContextDataManager.java index c7695319..586dc92a 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ContextDataManager.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ContextDataManager.java @@ -39,8 +39,6 @@ * deferred listener calls are done via the {@link TaskExecutor} abstraction. */ final class ContextDataManager { - static final ContextHasher HASHER = new ContextHasher(); - private final PersistentDataStoreWrapper.PerEnvironmentData environmentStore; private final int maxCachedContexts; private final TaskExecutor taskExecutor; @@ -57,7 +55,7 @@ final class ContextDataManager { @NonNull private volatile LDContext currentContext; @NonNull private volatile EnvironmentData flags = new EnvironmentData(); - @NonNull private volatile ContextIndex index = null; + @NonNull private volatile ContextIndex index; ContextDataManager( @NonNull ClientContext clientContext, @@ -65,6 +63,7 @@ final class ContextDataManager { int maxCachedContexts ) { this.environmentStore = environmentStore; + this.index = environmentStore.getIndex(); this.maxCachedContexts = maxCachedContexts; this.taskExecutor = ClientContextImpl.get(clientContext).getTaskExecutor(); this.logger = clientContext.getBaseLogger(); @@ -121,7 +120,8 @@ private void initDataInternal( boolean writeFlagsToPersistentStore ) { List removedContextIds = new ArrayList<>(); - String contextId = hashedContextId(context); + String contextId = LDUtil.urlSafeBase64HashedContextId(context); + String fingerprint = LDUtil.urlSafeBase64Hash(context); EnvironmentData oldData; ContextIndex newIndex; @@ -133,9 +133,6 @@ private void initDataInternal( oldData = flags; flags = newData; - if (index == null) { - index = environmentStore.getIndex(); - } newIndex = index.updateTimestamp(contextId, System.currentTimeMillis()) .prune(maxCachedContexts, removedContextIds); index = newIndex; @@ -145,7 +142,7 @@ private void initDataInternal( logger.debug("Removed flag data for context {} from persistent store", removedContextId); } if (writeFlagsToPersistentStore && maxCachedContexts != 0) { - environmentStore.setContextData(contextId, newData); + environmentStore.setContextData(contextId, fingerprint, newData); logger.debug("Updated flag data for context {} in persistent store", contextId); } environmentStore.setIndex(newIndex); @@ -241,13 +238,10 @@ public boolean upsert(@NonNull LDContext context, @NonNull Flag flag) { updatedFlags = flags.withFlagUpdatedOrAdded(flag); flags = updatedFlags; - String contextId = hashedContextId(context); - environmentStore.setContextData(contextId, updatedFlags); - - if (index == null) { - index = environmentStore.getIndex(); - } - index = index.updateTimestamp(contextId, System.currentTimeMillis()); + String hashedContextId = LDUtil.urlSafeBase64HashedContextId(context); + String fingerprint = LDUtil.urlSafeBase64Hash(context); + environmentStore.setContextData(hashedContextId, fingerprint, updatedFlags); + index = index.updateTimestamp(hashedContextId, System.currentTimeMillis()); environmentStore.setIndex(index); } @@ -298,10 +292,6 @@ public Collection getListenersByKey(String key) { return res == null ? new HashSet<>() : res; } - public static String hashedContextId(final LDContext context) { - return HASHER.hash(context.getFullyQualifiedKey()); - } - /** * Attempts to retrieve data for the specified context, if any, from the persistent store. This * does not affect the current context/flags state. @@ -311,7 +301,7 @@ public static String hashedContextId(final LDContext context) { */ @VisibleForTesting public @Nullable EnvironmentData getStoredData(LDContext context) { - return environmentStore.getContextData(hashedContextId(context)); + return environmentStore.getContextData(LDUtil.urlSafeBase64HashedContextId(context)); } private void notifyFlagListeners(Collection updatedFlagKeys) { diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ContextHasher.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ContextHasher.java deleted file mode 100644 index 9b3ac2a8..00000000 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ContextHasher.java +++ /dev/null @@ -1,30 +0,0 @@ -package com.launchdarkly.sdk.android; - -import android.util.Base64; - -import java.nio.charset.Charset; -import java.security.MessageDigest; -import java.security.NoSuchAlgorithmException; - -/** - * Provides a single hash method that takes a String and returns a unique filename-safe hash of it. - * It exists as a separate class so we can unit test it and assert that different instances - * produce the same output given the same input. - */ -class ContextHasher { - - String hash(String toHash) { - try { - MessageDigest messageDigest = MessageDigest.getInstance("SHA-256"); - messageDigest.reset(); - // All instances of the JVM are required to support UTF-8 charset - byte[] hash = messageDigest.digest(toHash.getBytes(Charset.forName("UTF-8"))); - return Base64.encodeToString(hash, Base64.URL_SAFE + Base64.NO_WRAP); - } catch (NoSuchAlgorithmException ignored) { - // SHA-256 should be supported on all devices. This exception case is because Java - // can't statically verify that the string "SHA-256" is always a valid MessageDigest. - // We return a string of the correct length in case anything depends on it. - return "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA="; - } - } -} diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/HttpFeatureFlagFetcher.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/HttpFeatureFlagFetcher.java index eca1c9c3..c6129e48 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/HttpFeatureFlagFetcher.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/HttpFeatureFlagFetcher.java @@ -138,7 +138,7 @@ private Request getDefaultRequest(LDContext ldContext) throws IOException { // and methods like Uri.withAppendedPath, simply to minimize the amount of code that relies on // Android-specific APIs so our components are more easily unit-testable. URI uri = HttpHelpers.concatenateUriPath(pollUri, StandardEndpoints.POLLING_REQUEST_GET_BASE_PATH); - uri = HttpHelpers.concatenateUriPath(uri, LDUtil.base64Url(ldContext)); + uri = HttpHelpers.concatenateUriPath(uri, LDUtil.urlSafeBase64(ldContext)); if (evaluationReasons) { uri = URI.create(uri.toString() + "?withReasons=true"); } diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDUtil.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDUtil.java index a5301e94..163ea5bb 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDUtil.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDUtil.java @@ -145,7 +145,15 @@ static String urlSafeBase64Hash(String input) { } } - static String base64Url(final LDContext context) { + public static String urlSafeBase64HashedContextId(LDContext context) { + return urlSafeBase64Hash(context.getFullyQualifiedKey()); + } + + static String urlSafeBase64Hash(LDContext context) { + return urlSafeBase64Hash(JsonSerialization.serialize(context)); + } + + static String urlSafeBase64(LDContext context) { return Base64.encodeToString(JsonSerialization.serialize(context).getBytes(), Base64.URL_SAFE + Base64.NO_WRAP); } diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/PersistentDataStoreWrapper.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/PersistentDataStoreWrapper.java index d12574e0..17234030 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/PersistentDataStoreWrapper.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/PersistentDataStoreWrapper.java @@ -3,6 +3,7 @@ import static com.launchdarkly.sdk.internal.GsonHelpers.gsonInstance; import androidx.annotation.NonNull; +import androidx.annotation.Nullable; import com.launchdarkly.logging.LDLogger; import com.launchdarkly.sdk.ContextKind; @@ -11,6 +12,7 @@ import java.util.HashMap; import java.util.Map; +import java.util.Objects; import java.util.UUID; import java.util.concurrent.atomic.AtomicBoolean; @@ -53,6 +55,7 @@ static class SavedConnectionInfo { private static final String ANON_CONTEXT_KEY_PREFIX = "anonKey_"; private static final String ENVIRONMENT_METADATA_KEY = "index"; private static final String ENVIRONMENT_CONTEXT_DATA_KEY_PREFIX = "flags_"; + private static final String ENVIRONMENT_CONTEXT_FINGERPRINT_KEY_PREFIX = "contextFingerprint_"; private static final String ENVIRONMENT_LAST_SUCCESS_TIME_KEY = "lastSuccessfulConnection"; private static final String ENVIRONMENT_LAST_FAILURE_TIME_KEY = "lastFailedConnection"; private static final String ENVIRONMENT_LAST_FAILURE_KEY = "lastFailure"; @@ -155,7 +158,7 @@ final class PerEnvironmentData { /** * Returns the stored flag data, if any, for a specific context. * - * @param hashedContextId the hashed key of the context + * @param hashedContextId the hashed canonical key of the context * @return the {@link EnvironmentData}, or null if not found */ public EnvironmentData getContextData(String hashedContextId) { @@ -171,20 +174,23 @@ public EnvironmentData getContextData(String hashedContextId) { /** * Stores flag data for a specific context, overwriting any previous data for that context. * - * @param hashedContextId the hashed key of the context + * @param hashedContextId the hashed canonical key of the context + * @param fingerprint that is unique for the given context and considers all attributes as part of its calculation * @param allData the flag data */ - public void setContextData(String hashedContextId, EnvironmentData allData) { + public void setContextData(String hashedContextId, String fingerprint, EnvironmentData allData) { trySetValue(environmentNamespace, keyForContextId(hashedContextId), allData.toJson()); + trySetValue(environmentNamespace, keyForContextFingerprint(hashedContextId), fingerprint); } /** * Removes the stored flag data, if any, for a specific context. * - * @param hashedContextId the hashed key of the context + * @param hashedContextId the hashed canonical key of the context */ public void removeContextData(String hashedContextId) { trySetValue(environmentNamespace, keyForContextId(hashedContextId), null); + trySetValue(environmentNamespace, keyForContextFingerprint(hashedContextId), null); } /** @@ -203,6 +209,29 @@ public ContextIndex getIndex() { } } + /** + * @param hashedContextId the hashed canonical key of the context + * @param fingerprint that is unique for the given context and considers all attributes as part of its calculation + * @return the timestamp in millis that the context data was last updated, null if no data is stored for the fingerprint + */ + @Nullable + public Long getLastUpdated(String hashedContextId, String fingerprint) { + String storedFingerprint = tryGetValue(environmentNamespace, keyForContextFingerprint(hashedContextId)); + if (!Objects.equals(storedFingerprint, fingerprint)) { + // we don't a timestamp stored for this fingerprint + return null; + } + + for (ContextIndex.IndexEntry entry : getIndex().data) { + if (entry.contextId.equals(hashedContextId)) { + return entry.timestamp; + } + } + + // no match found + return null; + } + /** * Updates the list of contexts that have stored flag data for this environment. * @@ -252,6 +281,10 @@ private String keyForContextId(String hashedContextId) { return ENVIRONMENT_CONTEXT_DATA_KEY_PREFIX + hashedContextId; } + private String keyForContextFingerprint(String hashedContextId) { + return ENVIRONMENT_CONTEXT_FINGERPRINT_KEY_PREFIX + hashedContextId; + } + private String tryGetValue(String namespace, String key) { try { synchronized (storeLock) { diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/StreamingDataSource.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/StreamingDataSource.java index fd19005f..1f74b601 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/StreamingDataSource.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/StreamingDataSource.java @@ -190,7 +190,7 @@ private URI getUri(@Nullable LDContext context) { StandardEndpoints.STREAMING_REQUEST_BASE_PATH); if (!useReport && context != null) { - uri = HttpHelpers.concatenateUriPath(uri, LDUtil.base64Url(context)); + uri = HttpHelpers.concatenateUriPath(uri, LDUtil.urlSafeBase64(context)); } if (evaluationReasons) { diff --git a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/ContextDataManagerTestBase.java b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/ContextDataManagerTestBase.java index f08895fd..ae13c641 100644 --- a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/ContextDataManagerTestBase.java +++ b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/ContextDataManagerTestBase.java @@ -70,7 +70,7 @@ protected ContextDataManager createDataManager() { } protected void assertContextIsCached(LDContext context, EnvironmentData expectedData) { - String contextHash = ContextDataManager.hashedContextId(context); + String contextHash = LDUtil.urlSafeBase64HashedContextId(context); EnvironmentData data = environmentStore.getContextData(contextHash); assertNotNull("flag data for context " + contextHash + " not found in store", data); assertDataSetsEqual(expectedData, data); @@ -86,7 +86,7 @@ protected void assertContextIsCached(LDContext context, EnvironmentData expected } protected void assertContextIsNotCached(LDContext context) { - String contextHash = ContextDataManager.hashedContextId(context); + String contextHash = LDUtil.urlSafeBase64HashedContextId(context); assertNull("flag data for " + context.getKey() + " should not have been in store", environmentStore.getContextData(contextHash)); diff --git a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/PersistentDataStoreWrapperTest.java b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/PersistentDataStoreWrapperTest.java index 1706bd0c..10a5fc35 100644 --- a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/PersistentDataStoreWrapperTest.java +++ b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/PersistentDataStoreWrapperTest.java @@ -32,6 +32,7 @@ public class PersistentDataStoreWrapperTest extends EasyMockSupport { private static final String EXPECTED_ENVIRONMENT_NAMESPACE = "LaunchDarkly_" + MOBILE_KEY_HASH; private static final String CONTEXT_KEY = "context-key"; private static final String CONTEXT_KEY_HASH = LDUtil.urlSafeBase64Hash(CONTEXT_KEY); + private static final String CONTEXT_FINGERPRINT = "mock-context-fingerprint"; private static final String EXPECTED_CONTEXT_FLAGS_KEY = "flags_" + CONTEXT_KEY_HASH; private static final String EXPECTED_INDEX_KEY = "index"; private static final String EXPECTED_GENERATED_CONTEXT_KEY_PREFIX = "anonKey_"; @@ -103,7 +104,7 @@ public void setContextData() { expectLastCall(); replayAll(); - envWrapper.setContextData(CONTEXT_KEY_HASH, data); + envWrapper.setContextData(CONTEXT_KEY_HASH, CONTEXT_FINGERPRINT, data); verifyAll(); logging.assertNothingLogged(); } @@ -116,7 +117,7 @@ public void setContextDataWhenStoreThrowsException() { expectLastCall().andThrow(makeException()); replayAll(); - envWrapper.setContextData(CONTEXT_KEY_HASH, data); + envWrapper.setContextData(CONTEXT_KEY_HASH, CONTEXT_FINGERPRINT, data); verifyAll(); assertStoreErrorWasLogged(); } diff --git a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/PollingDataSourceTest.java b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/PollingDataSourceTest.java index aa720507..36dd2e12 100644 --- a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/PollingDataSourceTest.java +++ b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/PollingDataSourceTest.java @@ -15,7 +15,6 @@ import com.launchdarkly.sdk.android.subsystems.ClientContext; import com.launchdarkly.sdk.android.subsystems.DataSource; -import org.easymock.EasyMockSupport; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -136,9 +135,12 @@ public void pollingIntervalHonoredAcrossMultipleBuildCalls() throws Exception { assertEquals(0, ds1.initialDelayMillis); // simulate successful update of context index timestamp - String hash = new ContextHasher().hash(CONTEXT.getFullyQualifiedKey()); - ContextIndex newIndex = clientContext.getPerEnvironmentData().getIndex().updateTimestamp(hash, System.currentTimeMillis()); - clientContext.getPerEnvironmentData().setIndex(newIndex); + String hashedContextId = LDUtil.urlSafeBase64HashedContextId(CONTEXT); + String fingerPrint = LDUtil.urlSafeBase64Hash(CONTEXT); + PersistentDataStoreWrapper.PerEnvironmentData perEnvironmentData = clientContext.getPerEnvironmentData(); + perEnvironmentData.setContextData(hashedContextId, fingerPrint, new EnvironmentData()); + ContextIndex newIndex = perEnvironmentData.getIndex().updateTimestamp(hashedContextId, System.currentTimeMillis()); + perEnvironmentData.setIndex(newIndex); // second build should have a non-zero delay due to simulated response storing a recent timestamp PollingDataSource ds2 = (PollingDataSource) builder.build(clientContext); diff --git a/shared-test-code/src/main/java/com/launchdarkly/sdk/android/TestUtil.java b/shared-test-code/src/main/java/com/launchdarkly/sdk/android/TestUtil.java index 9ed8acbf..5e828f60 100644 --- a/shared-test-code/src/main/java/com/launchdarkly/sdk/android/TestUtil.java +++ b/shared-test-code/src/main/java/com/launchdarkly/sdk/android/TestUtil.java @@ -48,9 +48,9 @@ public static void writeFlagUpdateToStore( ) { PersistentDataStoreWrapper.PerEnvironmentData environmentStore = new PersistentDataStoreWrapper(store, LDLogger.none()).perEnvironmentData(mobileKey); - EnvironmentData data = environmentStore.getContextData(ContextDataManager.hashedContextId(context)); + EnvironmentData data = environmentStore.getContextData(LDUtil.urlSafeBase64HashedContextId(context)); EnvironmentData newData = (data == null ? new EnvironmentData() : data).withFlagUpdatedOrAdded(flag); - environmentStore.setContextData(ContextDataManager.hashedContextId(context), newData); + environmentStore.setContextData(LDUtil.urlSafeBase64HashedContextId(context), LDUtil.urlSafeBase64Hash(context), newData); } public static void doSynchronouslyOnNewThread(Runnable action) { From a0154a922c71d9979813966cfa75a28f2d059e67 Mon Sep 17 00:00:00 2001 From: Todd Anderson Date: Thu, 21 Nov 2024 09:16:16 -0600 Subject: [PATCH 3/7] fixing some tests --- .../sdk/android/PersistentDataStoreWrapperTest.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/PersistentDataStoreWrapperTest.java b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/PersistentDataStoreWrapperTest.java index 10a5fc35..22b163d7 100644 --- a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/PersistentDataStoreWrapperTest.java +++ b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/PersistentDataStoreWrapperTest.java @@ -34,6 +34,7 @@ public class PersistentDataStoreWrapperTest extends EasyMockSupport { private static final String CONTEXT_KEY_HASH = LDUtil.urlSafeBase64Hash(CONTEXT_KEY); private static final String CONTEXT_FINGERPRINT = "mock-context-fingerprint"; private static final String EXPECTED_CONTEXT_FLAGS_KEY = "flags_" + CONTEXT_KEY_HASH; + private static final String EXPECTED_CONTEXT_FINGERPRINT_KEY = "contextFingerprint_" + CONTEXT_KEY_HASH; private static final String EXPECTED_INDEX_KEY = "index"; private static final String EXPECTED_GENERATED_CONTEXT_KEY_PREFIX = "anonKey_"; private static final Flag FLAG = new Flag("flagkey", LDValue.of(true), 1, @@ -101,6 +102,8 @@ public void setContextData() { EnvironmentData data = new DataSetBuilder().add(FLAG).build(); mockPersistentStore.setValue(EXPECTED_ENVIRONMENT_NAMESPACE, EXPECTED_CONTEXT_FLAGS_KEY, data.toJson()); + mockPersistentStore.setValue(EXPECTED_ENVIRONMENT_NAMESPACE, + EXPECTED_CONTEXT_FINGERPRINT_KEY, CONTEXT_FINGERPRINT); expectLastCall(); replayAll(); @@ -114,6 +117,8 @@ public void setContextDataWhenStoreThrowsException() { EnvironmentData data = new DataSetBuilder().add(FLAG).build(); mockPersistentStore.setValue(EXPECTED_ENVIRONMENT_NAMESPACE, EXPECTED_CONTEXT_FLAGS_KEY, data.toJson()); + mockPersistentStore.setValue(EXPECTED_ENVIRONMENT_NAMESPACE, + EXPECTED_CONTEXT_FINGERPRINT_KEY, CONTEXT_FINGERPRINT); expectLastCall().andThrow(makeException()); replayAll(); @@ -125,6 +130,7 @@ public void setContextDataWhenStoreThrowsException() { @Test public void removeContextData() { mockPersistentStore.setValue(EXPECTED_ENVIRONMENT_NAMESPACE, EXPECTED_CONTEXT_FLAGS_KEY, null); + mockPersistentStore.setValue(EXPECTED_ENVIRONMENT_NAMESPACE, EXPECTED_CONTEXT_FINGERPRINT_KEY, null); expectLastCall(); replayAll(); @@ -136,6 +142,7 @@ public void removeContextData() { @Test public void removeContextDataWhenStoreThrowsException() { mockPersistentStore.setValue(EXPECTED_ENVIRONMENT_NAMESPACE, EXPECTED_CONTEXT_FLAGS_KEY, null); + mockPersistentStore.setValue(EXPECTED_ENVIRONMENT_NAMESPACE, EXPECTED_CONTEXT_FINGERPRINT_KEY, null); expectLastCall().andThrow(makeException()); replayAll(); From 117a0cc13b340a3db1a76379e5af615da8002c33 Mon Sep 17 00:00:00 2001 From: Todd Anderson Date: Thu, 21 Nov 2024 10:53:22 -0600 Subject: [PATCH 4/7] adding more tests --- .../sdk/android/ClientContextImpl.java | 6 +-- .../launchdarkly/sdk/android/LDClient.java | 7 ++-- .../android/PersistentDataStoreWrapper.java | 18 ++++----- .../sdk/android/ConnectivityManagerTest.java | 6 +-- .../ContextDataManagerFlagDataTest.java | 18 +++++++++ .../android/ContextDataManagerTestBase.java | 5 +-- .../sdk/android/DiagnosticConfigTest.java | 2 +- .../sdk/android/LDConfigTest.java | 4 +- .../PersistentDataStoreWrapperTest.java | 37 ++++++++++++++++++- .../sdk/android/PollingDataSourceTest.java | 4 +- .../sdk/android/StreamingDataSourceTest.java | 8 ++-- 11 files changed, 81 insertions(+), 34 deletions(-) diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ClientContextImpl.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ClientContextImpl.java index ac0cb66f..85066fbe 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ClientContextImpl.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ClientContextImpl.java @@ -50,18 +50,16 @@ final class ClientContextImpl extends ClientContext { this.perEnvironmentData = perEnvironmentData; } - // TODO: consider re-ordering so perEnvironmentData is earlier in list of params static ClientContextImpl fromConfig( LDConfig config, String mobileKey, String environmentName, - FeatureFetcher fetcher, + PersistentDataStoreWrapper.PerEnvironmentData perEnvironmentData, FeatureFetcher fetcher, LDContext initialContext, LDLogger logger, PlatformState platformState, IEnvironmentReporter environmentReporter, - TaskExecutor taskExecutor, - PersistentDataStoreWrapper.PerEnvironmentData perEnvironmentData + TaskExecutor taskExecutor ) { boolean initiallyInBackground = platformState != null && !platformState.isForeground(); ClientContext minimalContext = new ClientContext(mobileKey, environmentReporter, logger, config, diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClient.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClient.java index c9a38e17..60eb7028 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClient.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClient.java @@ -330,7 +330,7 @@ protected LDClient( FeatureFetcher fetcher = null; if (config.dataSource instanceof ComponentsImpl.DataSourceRequiresFeatureFetcher) { ClientContextImpl minimalContext = ClientContextImpl.fromConfig(config, mobileKey, - environmentName, null, initialContext, logger, platformState, environmentReporter, taskExecutor, environmentStore + environmentName, environmentStore, null, initialContext, logger, platformState, environmentReporter, taskExecutor ); fetcher = new HttpFeatureFlagFetcher(minimalContext); } @@ -339,13 +339,12 @@ protected LDClient( config, mobileKey, environmentName, - fetcher, + environmentStore, fetcher, initialContext, logger, platformState, environmentReporter, - taskExecutor, - environmentStore + taskExecutor ); this.contextDataManager = new ContextDataManager( diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/PersistentDataStoreWrapper.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/PersistentDataStoreWrapper.java index 17234030..8289ab5c 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/PersistentDataStoreWrapper.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/PersistentDataStoreWrapper.java @@ -209,6 +209,15 @@ public ContextIndex getIndex() { } } + /** + * Updates the list of contexts that have stored flag data for this environment. + * + * @param contextIndex the context index + */ + public void setIndex(@NonNull ContextIndex contextIndex) { + trySetValue(environmentNamespace, ENVIRONMENT_METADATA_KEY, contextIndex.toJson()); + } + /** * @param hashedContextId the hashed canonical key of the context * @param fingerprint that is unique for the given context and considers all attributes as part of its calculation @@ -232,15 +241,6 @@ public Long getLastUpdated(String hashedContextId, String fingerprint) { return null; } - /** - * Updates the list of contexts that have stored flag data for this environment. - * - * @param contextIndex the context index - */ - public void setIndex(@NonNull ContextIndex contextIndex) { - trySetValue(environmentNamespace, ENVIRONMENT_METADATA_KEY, contextIndex.toJson()); - } - /** * Retrieves stored connection status properties. * diff --git a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/ConnectivityManagerTest.java b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/ConnectivityManagerTest.java index cbfadae5..26523e5e 100644 --- a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/ConnectivityManagerTest.java +++ b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/ConnectivityManagerTest.java @@ -4,7 +4,6 @@ import static com.launchdarkly.sdk.android.TestUtil.requireValue; import static org.easymock.EasyMock.anyObject; import static org.easymock.EasyMock.expectLastCall; -import static org.easymock.EasyMock.replay; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotEquals; @@ -111,13 +110,12 @@ private void createTestManager( config, MOBILE_KEY, "", - null, + environmentStore, null, CONTEXT, logging.logger, mockPlatformState, environmentReporter, - taskExecutor, - environmentStore + taskExecutor ); contextDataManager = new ContextDataManager( diff --git a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/ContextDataManagerFlagDataTest.java b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/ContextDataManagerFlagDataTest.java index b2606e25..c31aa1b6 100644 --- a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/ContextDataManagerFlagDataTest.java +++ b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/ContextDataManagerFlagDataTest.java @@ -6,6 +6,7 @@ import static com.launchdarkly.sdk.android.AssertHelpers.assertFlagsEqual; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; @@ -163,6 +164,23 @@ public void upsertUpdatesFlag() { assertDataSetsEqual(expectedData, manager.getStoredData(CONTEXT)); } + @Test + public void upsertUpdatesIndexTimestamp() throws Exception { + Flag flag1a = new FlagBuilder("flag1").version(1).value(true).build(), + flag1b = new FlagBuilder(flag1a.getKey()).version(2).value(false).build(); + EnvironmentData initialData = new DataSetBuilder().add(flag1a).build(); + ContextDataManager manager = createDataManager(); + manager.switchToContext(CONTEXT); + manager.initData(CONTEXT, initialData); + long firstTimestamp = environmentStore.getLastUpdated(LDUtil.urlSafeBase64HashedContextId(CONTEXT), LDUtil.urlSafeBase64Hash(CONTEXT)); + + Thread.sleep(2); // sleep for an amount that is greater than precision of System.currentTimeMillis so the change can be detected + + manager.upsert(CONTEXT, flag1b); + long secondTimestamp = environmentStore.getLastUpdated(LDUtil.urlSafeBase64HashedContextId(CONTEXT), LDUtil.urlSafeBase64Hash(CONTEXT)); + assertNotEquals(firstTimestamp, secondTimestamp); + } + @Test public void upsertDoesNotUpdateFlagWithSameVersion() { upsertDoesNotUpdateFlag( diff --git a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/ContextDataManagerTestBase.java b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/ContextDataManagerTestBase.java index ae13c641..545e97c4 100644 --- a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/ContextDataManagerTestBase.java +++ b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/ContextDataManagerTestBase.java @@ -50,13 +50,12 @@ protected ContextDataManager createDataManager(int maxCachedContexts) { new LDConfig.Builder(AutoEnvAttributes.Disabled).build(), "mobile-key", "", - null, + environmentStore, null, INITIAL_CONTEXT, logging.logger, null, environmentReporter, - taskExecutor, - environmentStore + taskExecutor ); return new ContextDataManager( clientContext, diff --git a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/DiagnosticConfigTest.java b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/DiagnosticConfigTest.java index 0c7f49d8..8243052b 100644 --- a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/DiagnosticConfigTest.java +++ b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/DiagnosticConfigTest.java @@ -142,7 +142,7 @@ public void customDiagnosticConfigurationHttp() throws Exception { private static LDValue makeDiagnosticJson(LDConfig config) throws Exception { ClientContext clientContext = ClientContextImpl.fromConfig(config, "", "", - null, null, LDLogger.none(), null, new EnvironmentReporterBuilder().build(), null, null); + null, null, null, LDLogger.none(), null, new EnvironmentReporterBuilder().build(), null); DiagnosticStore.SdkDiagnosticParams params = EventUtil.makeDiagnosticParams(clientContext); DiagnosticStore diagnosticStore = new DiagnosticStore(params); MockDiagnosticEventSender mockSender = new MockDiagnosticEventSender(); diff --git a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/LDConfigTest.java b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/LDConfigTest.java index a890bb37..c0684882 100644 --- a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/LDConfigTest.java +++ b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/LDConfigTest.java @@ -75,7 +75,7 @@ Map headersToMap(Headers headers) { public void headersForEnvironment() { LDConfig config = new LDConfig.Builder(AutoEnvAttributes.Disabled).mobileKey("test-key").build(); ClientContext clientContext = ClientContextImpl.fromConfig(config, "test-key", "", - null, null, null, null, new EnvironmentReporterBuilder().build(), null, null); + null, null, null, null, null, new EnvironmentReporterBuilder().build(), null); Map headers = headersToMap( LDUtil.makeHttpProperties(clientContext).toHeadersBuilder().build() ); @@ -96,7 +96,7 @@ public void headersForEnvironmentWithTransform() { })) .build(); ClientContext clientContext = ClientContextImpl.fromConfig(config, "test-key", "", - null, null, null, null, new EnvironmentReporterBuilder().build(), null, null); + null, null, null, null, null, new EnvironmentReporterBuilder().build(), null); expected.put("User-Agent", LDUtil.USER_AGENT_HEADER_VALUE); expected.put("Authorization", "api_key test-key"); diff --git a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/PersistentDataStoreWrapperTest.java b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/PersistentDataStoreWrapperTest.java index 22b163d7..cdca5e0a 100644 --- a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/PersistentDataStoreWrapperTest.java +++ b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/PersistentDataStoreWrapperTest.java @@ -17,10 +17,10 @@ import static org.easymock.EasyMock.eq; import static org.easymock.EasyMock.expect; import static org.easymock.EasyMock.expectLastCall; -import static org.easymock.EasyMock.verify; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; public class PersistentDataStoreWrapperTest extends EasyMockSupport { // This verifies non-platform-dependent behavior, such as what keys we store particular @@ -215,6 +215,41 @@ public void setIndexWhenStoreThrowsException() { assertStoreErrorWasLogged(); } + @Test + public void getLastUpdated() { + ContextIndex expectedIndex = new ContextIndex().updateTimestamp(CONTEXT_KEY_HASH, 1000); + expect(mockPersistentStore.getValue(EXPECTED_ENVIRONMENT_NAMESPACE, EXPECTED_CONTEXT_FINGERPRINT_KEY)).andReturn(CONTEXT_FINGERPRINT); + expect(mockPersistentStore.getValue(EXPECTED_ENVIRONMENT_NAMESPACE, EXPECTED_INDEX_KEY)) + .andReturn(expectedIndex.toJson()); + replayAll(); + + long lastUpdated = envWrapper.getLastUpdated(CONTEXT_KEY_HASH, CONTEXT_FINGERPRINT); + verifyAll(); + assertEquals(lastUpdated, 1000); + } + + @Test + public void getLastUpdatedNoMatchingHashedContextId() { + ContextIndex expectedIndex = new ContextIndex().updateTimestamp("ImABogusContextHash", 1000); + expect(mockPersistentStore.getValue(EXPECTED_ENVIRONMENT_NAMESPACE, EXPECTED_CONTEXT_FINGERPRINT_KEY)).andReturn(CONTEXT_FINGERPRINT); + expect(mockPersistentStore.getValue(EXPECTED_ENVIRONMENT_NAMESPACE, EXPECTED_INDEX_KEY)) + .andReturn(expectedIndex.toJson()); + replayAll(); + + assertNull(envWrapper.getLastUpdated(CONTEXT_KEY_HASH, CONTEXT_FINGERPRINT)); + } + + @Test + public void getLastUpdatedNoMatchingFingerprint() { + ContextIndex expectedIndex = new ContextIndex().updateTimestamp(CONTEXT_KEY_HASH, 1000); + expect(mockPersistentStore.getValue(EXPECTED_ENVIRONMENT_NAMESPACE, EXPECTED_CONTEXT_FINGERPRINT_KEY)).andReturn(null); + expect(mockPersistentStore.getValue(EXPECTED_ENVIRONMENT_NAMESPACE, EXPECTED_INDEX_KEY)) + .andReturn(expectedIndex.toJson()); + replayAll(); + + assertNull(envWrapper.getLastUpdated(CONTEXT_KEY_HASH, CONTEXT_FINGERPRINT)); + } + @Test public void getOrGenerateContextKeyExisting() { expect(mockPersistentStore.getValue(EXPECTED_GLOBAL_NAMESPACE, diff --git a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/PollingDataSourceTest.java b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/PollingDataSourceTest.java index 36dd2e12..608a0fc6 100644 --- a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/PollingDataSourceTest.java +++ b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/PollingDataSourceTest.java @@ -48,8 +48,8 @@ public void before() { private ClientContextImpl makeClientContext(boolean inBackground, Boolean previouslyInBackground) { ClientContextImpl baseClientContext = ClientContextImpl.fromConfig( - EMPTY_CONFIG, "", "", fetcher, CONTEXT, - logging.logger, platformState, environmentReporter, taskExecutor, perEnvironmentData); + EMPTY_CONFIG, "", "", perEnvironmentData, fetcher, CONTEXT, + logging.logger, platformState, environmentReporter, taskExecutor); return ClientContextImpl.forDataSource( baseClientContext, dataSourceUpdateSink, diff --git a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/StreamingDataSourceTest.java b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/StreamingDataSourceTest.java index e9ac2795..b4952df3 100644 --- a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/StreamingDataSourceTest.java +++ b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/StreamingDataSourceTest.java @@ -40,8 +40,8 @@ public void before() { private ClientContext makeClientContext(boolean inBackground, Boolean previouslyInBackground) { ClientContext baseClientContext = ClientContextImpl.fromConfig( - new LDConfig.Builder(AutoEnvAttributes.Disabled).build(), "", "", null, CONTEXT, - logging.logger, platformState, environmentReporter, taskExecutor, perEnvironmentData); + new LDConfig.Builder(AutoEnvAttributes.Disabled).build(), "", "", perEnvironmentData, null, CONTEXT, + logging.logger, platformState, environmentReporter, taskExecutor); return ClientContextImpl.forDataSource( baseClientContext, dataSourceUpdateSink, @@ -57,8 +57,8 @@ private ClientContext makeClientContext(boolean inBackground, Boolean previously // that has a fetcher private ClientContext makeClientContextWithFetcher() { ClientContext baseClientContext = ClientContextImpl.fromConfig( - new LDConfig.Builder(AutoEnvAttributes.Disabled).build(), "", "", makeFeatureFetcher(), CONTEXT, - logging.logger, platformState, environmentReporter, taskExecutor, perEnvironmentData); + new LDConfig.Builder(AutoEnvAttributes.Disabled).build(), "", "", perEnvironmentData, makeFeatureFetcher(), CONTEXT, + logging.logger, platformState, environmentReporter, taskExecutor); return ClientContextImpl.forDataSource( baseClientContext, dataSourceUpdateSink, From 70faaa6f6e41e4aa5d9391f5990a0e04cdcc7b63 Mon Sep 17 00:00:00 2001 From: Todd Anderson Date: Thu, 21 Nov 2024 11:04:35 -0600 Subject: [PATCH 5/7] removing tests for deleted class --- .../sdk/android/ContextHasherTest.java | 39 ------------------- 1 file changed, 39 deletions(-) delete mode 100644 launchdarkly-android-client-sdk/src/androidTest/java/com/launchdarkly/sdk/android/ContextHasherTest.java diff --git a/launchdarkly-android-client-sdk/src/androidTest/java/com/launchdarkly/sdk/android/ContextHasherTest.java b/launchdarkly-android-client-sdk/src/androidTest/java/com/launchdarkly/sdk/android/ContextHasherTest.java deleted file mode 100644 index db2922fd..00000000 --- a/launchdarkly-android-client-sdk/src/androidTest/java/com/launchdarkly/sdk/android/ContextHasherTest.java +++ /dev/null @@ -1,39 +0,0 @@ -package com.launchdarkly.sdk.android; - -import androidx.test.ext.junit.runners.AndroidJUnit4; - -import org.junit.Test; -import org.junit.runner.RunWith; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotEquals; - -@RunWith(AndroidJUnit4.class) -public class ContextHasherTest { - - @Test - public void testContextHasherReturnsUniqueResults(){ - ContextHasher contextHasher1 = new ContextHasher(); - - String input1 = "{'key':'userKey1'}"; - String input2 = "{'key':'userKey2'}"; - - assertNotEquals("Expected different results! instead got the same.", contextHasher1.hash(input1), contextHasher1.hash(input2)); - } - - @Test - public void testDifferentContextHashersReturnSameResults(){ - ContextHasher contextHasher1 = new ContextHasher(); - ContextHasher contextHasher2 = new ContextHasher(); - ContextHasher contextHasher3 = new ContextHasher(); - - String input1 = "{'key':'userKey1','email':'fake@example.com'}"; - String output1 = contextHasher1.hash(input1); - String output2 = contextHasher2.hash(input1); - String output3 = contextHasher3.hash(input1); - - assertEquals("Expected the same, but got different!", output1, output2); - assertEquals("Expected the same, but got different!", output1, output3); - assertEquals("Expected the same, but got different!", output2, output3); - } -} \ No newline at end of file From 85a993a64357cf1a6ea527fffc3e49275e7e18cc Mon Sep 17 00:00:00 2001 From: Todd Anderson Date: Thu, 21 Nov 2024 16:03:24 -0600 Subject: [PATCH 6/7] making initial polling delay not block init --- .../sdk/android/ComponentsImpl.java | 34 +++++++---------- .../sdk/android/PollingDataSource.java | 37 +++++++++++++------ .../sdk/android/PollingDataSourceTest.java | 23 ------------ .../sdk/android/StreamingDataSourceTest.java | 13 ------- 4 files changed, 38 insertions(+), 69 deletions(-) diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ComponentsImpl.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ComponentsImpl.java index 3561c3e3..1a59f007 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ComponentsImpl.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ComponentsImpl.java @@ -254,29 +254,21 @@ public DataSource build(ClientContext clientContext) { int pollInterval = clientContextImpl.isInBackground() ? backgroundPollIntervalMillis : pollIntervalMillis; - long initialDelayMillis; - if (clientContext.isInBackground() && Boolean.FALSE.equals(clientContext.getPreviouslyInBackground())) { - // If we're transitioning from foreground to background, then we don't want to do a - // poll right away because we already have recent flag data. Start polling *after* - // the first background poll interval. - initialDelayMillis = backgroundPollIntervalMillis; - } else { - // get the last updated timestamp for this context - PersistentDataStoreWrapper.PerEnvironmentData perEnvironmentData = clientContextImpl.getPerEnvironmentData(); - String hashedContextId = LDUtil.urlSafeBase64HashedContextId(clientContextImpl.getEvaluationContext()); - String fingerprint = LDUtil.urlSafeBase64Hash(clientContextImpl.getEvaluationContext()); - Long lastUpdated = perEnvironmentData.getLastUpdated(hashedContextId, fingerprint); - if (lastUpdated == null) { - lastUpdated = 0L; // default to beginning of time - } - - // To avoid unnecessarily frequent polling requests due to process or application lifecycle, we have added - // this initial delay logic. Calculate how much time has passed since the last update, if that is less than - // the polling interval, delay by the difference, otherwise 0 delay. - long elapsedSinceUpdate = System.currentTimeMillis() - lastUpdated; - initialDelayMillis = Math.max(pollInterval - elapsedSinceUpdate, 0); + // get the last updated timestamp for this context + PersistentDataStoreWrapper.PerEnvironmentData perEnvironmentData = clientContextImpl.getPerEnvironmentData(); + String hashedContextId = LDUtil.urlSafeBase64HashedContextId(clientContextImpl.getEvaluationContext()); + String fingerprint = LDUtil.urlSafeBase64Hash(clientContextImpl.getEvaluationContext()); + Long lastUpdated = perEnvironmentData.getLastUpdated(hashedContextId, fingerprint); + if (lastUpdated == null) { + lastUpdated = 0L; // default to beginning of time } + // To avoid unnecessarily frequent polling requests due to process or application lifecycle, we have added + // this initial delay logic. Calculate how much time has passed since the last update, if that is less than + // the polling interval, delay by the difference, otherwise 0 delay. + long elapsedSinceUpdate = System.currentTimeMillis() - lastUpdated; + long initialDelayMillis = Math.max(pollInterval - elapsedSinceUpdate, 0); + return new PollingDataSource( clientContextImpl.getEvaluationContext(), clientContextImpl.getDataSourceUpdateSink(), diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/PollingDataSource.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/PollingDataSource.java index c50a527d..bee3789e 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/PollingDataSource.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/PollingDataSource.java @@ -18,7 +18,7 @@ * ComponentsImpl.PollingDataSourceBuilderImpl and ComponentsImpl.StreamingDataSourceBuilderImpl. */ final class PollingDataSource implements DataSource { - private final LDContext currentContext; + private final LDContext context; private final DataSourceUpdateSink dataSourceUpdateSink; final long initialDelayMillis; // visible for testing final long pollIntervalMillis; // visible for testing @@ -29,8 +29,20 @@ final class PollingDataSource implements DataSource { private final AtomicReference> currentPollTask = new AtomicReference<>(); + /** + * @param context that this data source will fetch data for + * @param dataSourceUpdateSink to send data to + * @param initialDelayMillis delays when the data source begins polling. If this is greater than 0, the polling data + * source will report success immediately as it is now running even if data has not been + * fetched. + * @param pollIntervalMillis interval in millis between each polling request + * @param fetcher that will be used for each fetch + * @param platformState used for making decisions based on platform state + * @param taskExecutor that will be used to schedule the polling tasks + * @param logger for logging + */ PollingDataSource( - LDContext currentContext, + LDContext context, DataSourceUpdateSink dataSourceUpdateSink, long initialDelayMillis, long pollIntervalMillis, @@ -39,7 +51,7 @@ final class PollingDataSource implements DataSource { TaskExecutor taskExecutor, LDLogger logger ) { - this.currentContext = currentContext; + this.context = context; this.dataSourceUpdateSink = dataSourceUpdateSink; this.initialDelayMillis = initialDelayMillis; this.pollIntervalMillis = pollIntervalMillis; @@ -51,15 +63,16 @@ final class PollingDataSource implements DataSource { @Override public void start(final Callback resultCallback) { - Runnable trigger = new Runnable() { - @Override - public void run() { - triggerPoll(resultCallback); - } - }; + + if (initialDelayMillis > 0) { + // if there is an initial delay, we will immediately report the successful start of the data source + resultCallback.onSuccess(true); + } + + Runnable pollRunnable = () -> poll(resultCallback); logger.debug("Scheduling polling task with interval of {}ms, starting after {}ms", pollIntervalMillis, initialDelayMillis); - ScheduledFuture task = taskExecutor.startRepeatingTask(trigger, + ScheduledFuture task = taskExecutor.startRepeatingTask(pollRunnable, initialDelayMillis, pollIntervalMillis); currentPollTask.set(task); } @@ -73,8 +86,8 @@ public void stop(Callback completionCallback) { completionCallback.onSuccess(null); } - private void triggerPoll(Callback resultCallback) { - ConnectivityManager.fetchAndSetData(fetcher, currentContext, dataSourceUpdateSink, + private void poll(Callback resultCallback) { + ConnectivityManager.fetchAndSetData(fetcher, context, dataSourceUpdateSink, resultCallback, logger); } } diff --git a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/PollingDataSourceTest.java b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/PollingDataSourceTest.java index 608a0fc6..35f739b6 100644 --- a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/PollingDataSourceTest.java +++ b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/PollingDataSourceTest.java @@ -147,29 +147,6 @@ public void pollingIntervalHonoredAcrossMultipleBuildCalls() throws Exception { assertNotEquals(0, ds2.initialDelayMillis); } - @Test - public void firstPollHappensAfterBackgroundPollingIntervalWhenTransitioningToBackground() throws Exception { - ClientContext clientContext = makeClientContext(true, false); - PollingDataSourceBuilder builder = Components.pollingDataSource() - .pollIntervalMillis(100000); - ((ComponentsImpl.PollingDataSourceBuilderImpl)builder).backgroundPollIntervalMillisNoMinimum(200); - DataSource ds = builder.build(clientContext); - fetcher.setupSuccessResponse("{}"); - - try { - ds.start(LDUtil.noOpCallback()); - - requireNoMoreValues(fetcher.receivedContexts, 10, TimeUnit.MILLISECONDS); - - Thread.sleep(300); - - LDContext context = requireValue(fetcher.receivedContexts, 100, TimeUnit.MILLISECONDS); - assertEquals(CONTEXT, context); - } finally { - ds.stop(LDUtil.noOpCallback()); - } - } - @Test public void pollsAreRepeatedAtBackgroundPollIntervalInBackground() throws Exception { ClientContext clientContext = makeClientContext(true, null); diff --git a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/StreamingDataSourceTest.java b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/StreamingDataSourceTest.java index b4952df3..ee83015d 100644 --- a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/StreamingDataSourceTest.java +++ b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/StreamingDataSourceTest.java @@ -111,19 +111,6 @@ public void builderCreatesPollingDataSourceWhenStartingInBackground() { assertEquals(0L, ((PollingDataSource)ds).initialDelayMillis); } - @Test - public void pollingDataSourceHasInitialDelayWhenTransitioningToBackground() { - ClientContext clientContext = makeClientContext(true, false); - DataSource ds = Components.streamingDataSource() - .backgroundPollIntervalMillis(999999) - .build(clientContext); - - assertEquals(PollingDataSource.class, ds.getClass()); - - assertEquals(999999L, ((PollingDataSource)ds).pollIntervalMillis); - assertEquals(999999L, ((PollingDataSource)ds).initialDelayMillis); - } - @Test public void builderCreatesStreamingDataSourceWhenStartingInBackgroundWithOverride() { ClientContext clientContext = makeClientContext(true, null); From 1aacaf1db8b2fea0e584febf6d4416577a135099 Mon Sep 17 00:00:00 2001 From: Todd Anderson Date: Fri, 22 Nov 2024 10:55:36 -0600 Subject: [PATCH 7/7] fix: switching context no longer affects context caching timestamp --- .../sdk/android/LDClientEndToEndTest.java | 7 +++- .../sdk/android/ContextDataManager.java | 32 +++++++++++-------- .../ContextDataManagerFlagDataTest.java | 17 ++++++++++ 3 files changed, 41 insertions(+), 15 deletions(-) diff --git a/launchdarkly-android-client-sdk/src/androidTest/java/com/launchdarkly/sdk/android/LDClientEndToEndTest.java b/launchdarkly-android-client-sdk/src/androidTest/java/com/launchdarkly/sdk/android/LDClientEndToEndTest.java index 0440be88..2126360c 100644 --- a/launchdarkly-android-client-sdk/src/androidTest/java/com/launchdarkly/sdk/android/LDClientEndToEndTest.java +++ b/launchdarkly-android-client-sdk/src/androidTest/java/com/launchdarkly/sdk/android/LDClientEndToEndTest.java @@ -38,7 +38,7 @@ public class LDClientEndToEndTest { private Application application; private MockWebServer mockPollingServer; private URI mockPollingServerUri; - private final PersistentDataStore store = new InMemoryPersistentDataStore(); + private PersistentDataStore store; @Rule public final ActivityScenarioRule testScenario = @@ -65,6 +65,11 @@ public void setUp() { }); } + @Before + public void before() { + store = new InMemoryPersistentDataStore(); + } + @After public void after() throws IOException { mockPollingServer.close(); diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ContextDataManager.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ContextDataManager.java index 586dc92a..f466b38d 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ContextDataManager.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ContextDataManager.java @@ -119,7 +119,7 @@ private void initDataInternal( @NonNull EnvironmentData newData, boolean writeFlagsToPersistentStore ) { - List removedContextIds = new ArrayList<>(); + String contextId = LDUtil.urlSafeBase64HashedContextId(context); String fingerprint = LDUtil.urlSafeBase64Hash(context); EnvironmentData oldData; @@ -133,23 +133,27 @@ private void initDataInternal( oldData = flags; flags = newData; - newIndex = index.updateTimestamp(contextId, System.currentTimeMillis()) - .prune(maxCachedContexts, removedContextIds); - index = newIndex; - for (String removedContextId: removedContextIds) { - environmentStore.removeContextData(removedContextId); - logger.debug("Removed flag data for context {} from persistent store", removedContextId); - } - if (writeFlagsToPersistentStore && maxCachedContexts != 0) { + if (writeFlagsToPersistentStore) { + List removedContextIds = new ArrayList<>(); + newIndex = index.updateTimestamp(contextId, System.currentTimeMillis()) + .prune(maxCachedContexts, removedContextIds); + index = newIndex; + + for (String removedContextId: removedContextIds) { + environmentStore.removeContextData(removedContextId); + logger.debug("Removed flag data for context {} from persistent store", removedContextId); + } + environmentStore.setContextData(contextId, fingerprint, newData); + environmentStore.setIndex(newIndex); + + if (logger.isEnabled(LDLogLevel.DEBUG)) { + logger.debug("Stored context index is now: {}", newIndex.toJson()); + } + logger.debug("Updated flag data for context {} in persistent store", contextId); } - environmentStore.setIndex(newIndex); - } - - if (logger.isEnabled(LDLogLevel.DEBUG)) { - logger.debug("Stored context index is now: {}", newIndex.toJson()); } // Determine which flags were updated and notify listeners, if any diff --git a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/ContextDataManagerFlagDataTest.java b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/ContextDataManagerFlagDataTest.java index c31aa1b6..665154ba 100644 --- a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/ContextDataManagerFlagDataTest.java +++ b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/ContextDataManagerFlagDataTest.java @@ -164,6 +164,23 @@ public void upsertUpdatesFlag() { assertDataSetsEqual(expectedData, manager.getStoredData(CONTEXT)); } + @Test + public void switchDoesNotUpdateIndexTimestamp() throws Exception { + EnvironmentData data = new DataSetBuilder().add(new FlagBuilder("flag1").build()).build(); + ContextDataManager manager1 = createDataManager(); + manager1.switchToContext(INITIAL_CONTEXT); + manager1.initData(INITIAL_CONTEXT, data); + Long firstTimestamp = environmentStore.getLastUpdated(LDUtil.urlSafeBase64HashedContextId(INITIAL_CONTEXT), LDUtil.urlSafeBase64Hash(INITIAL_CONTEXT)); + + Thread.sleep(2); // sleep for an amount that is greater than precision of System.currentTimeMillis so the change can be detected + + manager1.switchToContext(CONTEXT); + manager1.switchToContext(INITIAL_CONTEXT); + Long secondTimestamp = environmentStore.getLastUpdated(LDUtil.urlSafeBase64HashedContextId(INITIAL_CONTEXT), LDUtil.urlSafeBase64Hash(INITIAL_CONTEXT)); + + assertEquals(firstTimestamp, secondTimestamp); + } + @Test public void upsertUpdatesIndexTimestamp() throws Exception { Flag flag1a = new FlagBuilder("flag1").version(1).value(true).build(),