From a995a4e0fae91552e563e4235970f933292d7be8 Mon Sep 17 00:00:00 2001 From: Todd Anderson <127344469+tanderson-ld@users.noreply.github.com> Date: Wed, 8 Jan 2025 11:20:38 -0600 Subject: [PATCH] feat: polling data source now supports one shot configuration (#285) BEGIN_COMMIT_OVERRIDE feat: polling data source now supports one shot configuration fix: polling data source no longer reports initialized=true incorrectly when rate limiting delays first poll END_COMMIT_OVERRIDE **Requirements** - [x] I have added test coverage for new or changed functionality - [x] I have followed the repository's [pull request submission guidelines](../blob/main/CONTRIBUTING.md#submitting-pull-requests) - [x] I have validated my changes against all supported platform versions **Describe the solution you've provided** Adds polling data source one shot mode. Updated data source to support providing the max number of polls to make. Updated builder to calculate number of polls to make based on one shot configuration. --- .github/workflows/ci.yml | 2 +- .../sdk/android/ComponentsImpl.java | 15 +++- .../sdk/android/LDClientInterface.java | 15 +++- .../sdk/android/PollingDataSource.java | 32 +++++--- .../PollingDataSourceBuilder.java | 18 +++++ .../sdk/android/PollingDataSourceTest.java | 80 ++++++++++++++++++- 6 files changed, 143 insertions(+), 19 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 04cc1ed6..7eef03d7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -19,7 +19,7 @@ jobs: # java_version: ['11', '17'] android_api_level: ['25'] java_version: ['17'] - runs-on: ubuntu-latest + runs-on: ubuntu-22.04 steps: # This enables hardware acceleration on large linux runners 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 1a59f007..f5ee8026 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 @@ -264,16 +264,27 @@ public DataSource build(ClientContext clientContext) { } // 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. + // this rate limiting logic. Calculate how much time has passed since the last update, if that is less than + // the polling interval, delay to when the next poll would have occurred, otherwise 0 delay. long elapsedSinceUpdate = System.currentTimeMillis() - lastUpdated; long initialDelayMillis = Math.max(pollInterval - elapsedSinceUpdate, 0); + long maxNumPolls = Long.MAX_VALUE; // effectively unlimited number of polls + if (oneShot) { + if (initialDelayMillis > 0) { + clientContext.getBaseLogger().info("One shot polling attempt will be blocked by rate limiting."); + maxNumPolls = 0; // one shot was blocked by rate limiting logic, so never poll + } else { + maxNumPolls = 1; // one shot was not blocked by rate limiting logic + } + } + return new PollingDataSource( clientContextImpl.getEvaluationContext(), clientContextImpl.getDataSourceUpdateSink(), initialDelayMillis, pollInterval, + maxNumPolls, clientContextImpl.getFetcher(), clientContextImpl.getPlatformState(), clientContextImpl.getTaskExecutor(), diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClientInterface.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClientInterface.java index 9acd178d..f2051934 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClientInterface.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDClientInterface.java @@ -17,11 +17,18 @@ */ public interface LDClientInterface extends Closeable { /** - * Checks whether the client is ready to return feature flag values. This is true if either - * the client has successfully connected to LaunchDarkly and received feature flags, or the - * client has been put into offline mode (in which case it will return only default flag values). + * Returns true if the client has successfully connected to LaunchDarkly and received feature flags after + * {@link LDClient#init(Application, LDConfig, LDContext, int)} was called. * - * @return true if the client is initialized or offline + * Also returns true if the SDK knows it will never be able to fetch flag data (such as when the client is set + * to offline mode or if in one shot configuration, the one shot fails). + * + * Otherwise this returns false until the client is able to retrieve latest feature flag data from + * LaunchDarkly services. This includes not connecting to LaunchDarkly within the start wait time provided to + * {@link LDClient#init(Application, LDConfig, LDContext, int)} even if the SDK has cached feature flags. + * + * @return true if the client is able to retrieve flag data from LaunchDarkly or offline, false if the client has been + * unable to up to this point. */ boolean isInitialized(); 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 bee3789e..a68697d2 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 @@ -22,12 +22,12 @@ final class PollingDataSource implements DataSource { private final DataSourceUpdateSink dataSourceUpdateSink; final long initialDelayMillis; // visible for testing final long pollIntervalMillis; // visible for testing + long numberOfPollsRemaining; // visible for testing private final FeatureFetcher fetcher; private final PlatformState platformState; private final TaskExecutor taskExecutor; private final LDLogger logger; - private final AtomicReference> currentPollTask = - new AtomicReference<>(); + final AtomicReference> currentPollTask = new AtomicReference<>(); // visible for testing /** * @param context that this data source will fetch data for @@ -36,6 +36,7 @@ final class PollingDataSource implements DataSource { * 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 maxNumberOfPolls the maximum number of polling attempts, use Long.MAX for effectively unlimited. * @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 @@ -46,6 +47,7 @@ final class PollingDataSource implements DataSource { DataSourceUpdateSink dataSourceUpdateSink, long initialDelayMillis, long pollIntervalMillis, + long maxNumberOfPolls, FeatureFetcher fetcher, PlatformState platformState, TaskExecutor taskExecutor, @@ -55,6 +57,7 @@ final class PollingDataSource implements DataSource { this.dataSourceUpdateSink = dataSourceUpdateSink; this.initialDelayMillis = initialDelayMillis; this.pollIntervalMillis = pollIntervalMillis; + this.numberOfPollsRemaining = maxNumberOfPolls; this.fetcher = fetcher; this.platformState = platformState; this.taskExecutor = taskExecutor; @@ -63,15 +66,16 @@ final class PollingDataSource implements DataSource { @Override public void start(final Callback resultCallback) { - - if (initialDelayMillis > 0) { - // if there is an initial delay, we will immediately report the successful start of the data source + if (numberOfPollsRemaining <= 0) { + // If there are no polls to be made, we will immediately report the successful start of the data source. This + // may seem strange, but one can think of this data source as behaving like a no-op in this configuration. resultCallback.onSuccess(true); + return; } Runnable pollRunnable = () -> poll(resultCallback); - logger.debug("Scheduling polling task with interval of {}ms, starting after {}ms", - pollIntervalMillis, initialDelayMillis); + logger.debug("Scheduling polling task with interval of {}ms, starting after {}ms, with number of polls {}", + pollIntervalMillis, initialDelayMillis, numberOfPollsRemaining); ScheduledFuture task = taskExecutor.startRepeatingTask(pollRunnable, initialDelayMillis, pollIntervalMillis); currentPollTask.set(task); @@ -87,7 +91,17 @@ public void stop(Callback completionCallback) { } private void poll(Callback resultCallback) { - ConnectivityManager.fetchAndSetData(fetcher, context, dataSourceUpdateSink, - resultCallback, logger); + // poll if there are polls remaining + if (numberOfPollsRemaining > 0) { + numberOfPollsRemaining--; + ConnectivityManager.fetchAndSetData(fetcher, context, dataSourceUpdateSink, + resultCallback, logger); + } else { + // terminate if we have no polls remaining + ScheduledFuture task = currentPollTask.getAndSet(null); + if (task != null) { + task.cancel(true); + } + } } } diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/integrations/PollingDataSourceBuilder.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/integrations/PollingDataSourceBuilder.java index 38e48d68..9ae96cc9 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/integrations/PollingDataSourceBuilder.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/integrations/PollingDataSourceBuilder.java @@ -46,6 +46,13 @@ public abstract class PollingDataSourceBuilder implements ComponentConfigurer @@ -80,4 +87,15 @@ public PollingDataSourceBuilder pollIntervalMillis(int pollIntervalMillis) { DEFAULT_POLL_INTERVAL_MILLIS : pollIntervalMillis; return this; } + + /** + * Sets the data source to make one and only one attempt to get feature flag updates. The one shot + * poll may be blocked by rate limiting logic and will not be retried if that occurs. + * + * @return the builder + */ + public PollingDataSourceBuilder oneShot() { + this.oneShot = true; + return this; + } } 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 35f739b6..fdc12f8a 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,7 +3,9 @@ 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.assertFalse; import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertTrue; import com.launchdarkly.sdk.LDContext; import com.launchdarkly.sdk.LDValue; @@ -23,6 +25,7 @@ import java.util.Map; import java.util.concurrent.BlockingQueue; import java.util.concurrent.LinkedBlockingQueue; +import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; public class PollingDataSourceTest { @@ -82,7 +85,7 @@ public void pollsAreRepeatedAtRegularPollIntervalInForeground() throws Exception ClientContext clientContext = makeClientContext(false, null); PollingDataSourceBuilder builder = Components.pollingDataSource() .backgroundPollIntervalMillis(100000); - ((ComponentsImpl.PollingDataSourceBuilderImpl)builder).pollIntervalMillisNoMinimum(200); + ((ComponentsImpl.PollingDataSourceBuilderImpl) builder).pollIntervalMillisNoMinimum(200); DataSource ds = builder.build(clientContext); fetcher.setupSuccessResponse("{}"); @@ -147,12 +150,46 @@ public void pollingIntervalHonoredAcrossMultipleBuildCalls() throws Exception { assertNotEquals(0, ds2.initialDelayMillis); } + @Test + public void oneShotPollingSetsMaxNumberOfPollsTo1() throws Exception { + ClientContextImpl clientContext = makeClientContext(true, null); + PollingDataSourceBuilder builder = Components.pollingDataSource().oneShot(); + + PollingDataSource ds = (PollingDataSource) builder.build(clientContext); + assertEquals(1, ds.numberOfPollsRemaining); + } + + @Test + public void oneShotIsPreventByRateLimiting() throws Exception { + ClientContextImpl clientContext = makeClientContext(true, null); + PollingDataSourceBuilder builder = Components.pollingDataSource() + .pollIntervalMillis(100000).oneShot(); + + // first build should have no delay + PollingDataSource ds1 = (PollingDataSource) builder.build(clientContext); + assertEquals(1, ds1.numberOfPollsRemaining); + assertEquals(0, ds1.initialDelayMillis); + + // simulate successful update of context index timestamp + 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 and so one shot is prevented by max number of polls being 0. + PollingDataSource ds2 = (PollingDataSource) builder.build(clientContext); + assertEquals(0, ds2.numberOfPollsRemaining); + assertNotEquals(0, ds2.initialDelayMillis); + } + @Test public void pollsAreRepeatedAtBackgroundPollIntervalInBackground() throws Exception { ClientContext clientContext = makeClientContext(true, null); PollingDataSourceBuilder builder = Components.pollingDataSource() .pollIntervalMillis(100000); - ((ComponentsImpl.PollingDataSourceBuilderImpl)builder).backgroundPollIntervalMillisNoMinimum(200); + ((ComponentsImpl.PollingDataSourceBuilderImpl) builder).backgroundPollIntervalMillisNoMinimum(200); DataSource ds = builder.build(clientContext); fetcher.setupSuccessResponse("{}"); @@ -180,7 +217,7 @@ public void dataIsUpdatedAfterEachPoll() throws Exception { ClientContext clientContext = makeClientContext(false, null); PollingDataSourceBuilder builder = Components.pollingDataSource() .backgroundPollIntervalMillis(100000); - ((ComponentsImpl.PollingDataSourceBuilderImpl)builder).pollIntervalMillisNoMinimum(200); + ((ComponentsImpl.PollingDataSourceBuilderImpl) builder).pollIntervalMillisNoMinimum(200); DataSource ds = builder.build(clientContext); EnvironmentData data1 = new DataSetBuilder() @@ -213,6 +250,43 @@ public void dataIsUpdatedAfterEachPoll() throws Exception { } } + @Test + public void terminatesAfterMaxNumberOfPolls() throws Exception { + ClientContextImpl clientContext = makeClientContext(false, null); + PollingDataSource ds = new PollingDataSource( + clientContext.getEvaluationContext(), + clientContext.getDataSourceUpdateSink(), + 0, + 50, + 2, // maximum number of requests is 2 + clientContext.getFetcher(), + clientContext.getPlatformState(), + clientContext.getTaskExecutor(), + clientContext.getBaseLogger() + ); + + fetcher.setupSuccessResponse("{}"); + fetcher.setupSuccessResponse("{}"); + fetcher.setupSuccessResponse("{}"); // need a third response to detect if the third request is sent which is a failure + + try { + ds.start(LDUtil.noOpCallback()); + ScheduledFuture pollTask = ds.currentPollTask.get(); + assertFalse(pollTask.isCancelled()); + + LDContext context1 = requireValue(fetcher.receivedContexts, 5, TimeUnit.MILLISECONDS); + Thread.sleep(50); + + LDContext context2 = requireValue(fetcher.receivedContexts, 5, TimeUnit.MILLISECONDS); + + // if a third request is sent, this will fail here + requireNoMoreValues(fetcher.receivedContexts, 100, TimeUnit.MILLISECONDS); + assertTrue(pollTask.isCancelled()); + } finally { + ds.stop(LDUtil.noOpCallback()); + } + } + private class MockFetcher implements FeatureFetcher { BlockingQueue receivedContexts = new LinkedBlockingQueue<>(); BlockingQueue responses = new LinkedBlockingQueue<>();