feat: honor polling interval across process restarts #282
Conversation
| @@ -244,30 +244,45 @@ static final class PollingDataSourceBuilderImpl extends PollingDataSourceBuilder | |||
| implements DiagnosticDescription, DataSourceRequiresFeatureFetcher { | |||
| @Override | |||
| public DataSource build(ClientContext clientContext) { | |||
There was a problem hiding this comment.
For reviewers: Updated from using ClientContext to ClientContextImpl type here to have access to an internal only object, namely the PerEnvironmentData.
| @@ -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)); | |||
There was a problem hiding this comment.
For reviewers: Just a rename
There was a problem hiding this comment.
For reviewers: This class was a glorified static method, so I got rid of it and moved it to LDUtil.
| public static String hashedContextId(final LDContext context) { | ||
| return HASHER.hash(context.getFullyQualifiedKey()); | ||
| } | ||
|
|
There was a problem hiding this comment.
For Reviewers: Moved to LDUtil
There was a problem hiding this comment.
For reviewers: Associated class was removed.
| @@ -65,6 +65,11 @@ public void setUp() { | |||
| }); | |||
| } | |||
|
|
|||
| @Before | |||
| public void before() { | |||
| store = new InMemoryPersistentDataStore(); | |||
There was a problem hiding this comment.
For reviewers: Tests were affecting each other's stores. Now they don't.
| // 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); |
There was a problem hiding this comment.
For reviewers: These code changes are the meat and potatoes of the PR.
| pollIntervalMillis; | ||
| int initialDelayMillis; | ||
| if (clientContext.isInBackground() && Boolean.FALSE.equals(clientContext.getPreviouslyInBackground())) { |
There was a problem hiding this comment.
For reviewers: This background/foregrounding distinction is no more! The only impact foreground/background has is on the polling interval. Now they are treated the same after that which results in some improvements in behavioral consistency.
| @@ -39,8 +39,6 @@ | |||
| * deferred listener calls are done via the {@link TaskExecutor} abstraction. | |||
| */ | |||
| final class ContextDataManager { | |||
| static final ContextHasher HASHER = new ContextHasher(); | |||
There was a problem hiding this comment.
For reviewers: ContextHasher was eliminated in favor of a static method.
| } | ||
|
|
||
| if (logger.isEnabled(LDLogLevel.DEBUG)) { | ||
| logger.debug("Stored context index is now: {}", newIndex.toJson()); |
There was a problem hiding this comment.
For reviewers: Android was updating the index timestamp when switching context. This is not consistent with JS, Flutter, and the iOS SDK. Updated this logic so that the index is only updated when new flag data needs to be written to persistence.
| String fingerprint = LDUtil.urlSafeBase64Hash(context); | ||
| environmentStore.setContextData(hashedContextId, fingerprint, updatedFlags); | ||
| index = index.updateTimestamp(hashedContextId, System.currentTimeMillis()); | ||
| environmentStore.setIndex(index); |
There was a problem hiding this comment.
For reviewers: This code path is hit when a flag patch comes in.
...ndroid-client-sdk/src/main/java/com/launchdarkly/sdk/android/PersistentDataStoreWrapper.java
Show resolved
Hide resolved
🤖 I have created a release *beep* *boop* --- ## [5.5.0](5.4.0...5.5.0) (2024-11-22) ### Features * honor polling interval across process restarts ([#282](#282)) ([9b3ca70](9b3ca70)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Requirements
I have added test coverage for new or changed functionality
I have followed the repository's pull request submission guidelines
I have validated my changes against all supported platform versions
Related issues
SDK-881
Describe the solution you've provided
Polling data source builder now uses context index timestamp to calculate if an initial polling delay.