diff --git a/README.md b/README.md index f2099fc4..33f60035 100644 --- a/README.md +++ b/README.md @@ -18,7 +18,7 @@ Refer to the [SDK documentation](https://docs.launchdarkly.com/sdk/client-side/a ## Learn more -Check out our [documentation](https://docs.launchdarkly.com) for in-depth instructions on configuring and using LaunchDarkly. You can also head straight to the [complete reference guide for this SDK](https://docs.launchdarkly.com/docs/android-sdk-reference) or our [Javadocs](http://launchdarkly.github.io/android-client-sdk/). +Check out our [documentation](https://docs.launchdarkly.com) for in-depth instructions on configuring and using LaunchDarkly. You can also head straight to the [complete reference guide for this SDK](https://docs.launchdarkly.com/sdk/client-side/android) or our [Javadocs](http://launchdarkly.github.io/android-client-sdk/). ## Testing 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 a1f97805..d004d85c 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 @@ -182,11 +182,18 @@ private void initDataInternal( Set updatedFlagKeys = new HashSet<>(); for (Flag newFlag: newData.values()) { Flag oldFlag = oldData.getFlag(newFlag.getKey()); - if (oldFlag == null || oldFlag.getVersion() != newFlag.getVersion()) { + if (oldFlag == null || !oldFlag.getValue().equals(newFlag.getValue())) { + // if the flag is new or the value has changed, notify. This logic can be run if + // the context changes, which can result in an evaluation change even if the version + // of the flag stays the same. You will notice this logic slightly differs from + // upsert. Upsert should only be calling to listeners if the value has changed, + // but we left upsert alone out of fear of that we'd uncover bugs in customer code + // if we added conditionals in upsert updatedFlagKeys.add(newFlag.getKey()); } } for (Flag oldFlag: oldData.values()) { + // if old flag is no longer appearing, notify if (newData.getFlag(oldFlag.getKey()) == null) { updatedFlagKeys.add(oldFlag.getKey()); } @@ -283,6 +290,10 @@ public boolean upsert(@NonNull Flag flag) { environmentStore.setContextData(contextId, updatedFlags); Collection updatedFlag = Collections.singletonList(flag.getKey()); + + // We really should only be calling to listeners if the value has changed, but we left this + // unconditional out of fear that we'd uncover bugs in customer code as a result of + // conditionally notifying listeners notifyAllFlagsListeners(updatedFlag); notifyFlagListeners(updatedFlag); @@ -343,6 +354,7 @@ private void notifyFlagListeners(Collection updatedFlagKeys) { if (listenersToCall.isEmpty()) { return; } + // We make sure to call listener callbacks on the main thread, as we consistently did so in // the past by virtue of using SharedPreferences to implement the callbacks. taskExecutor.executeOnMainThread(() -> { 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 a1d3ce8e..96b7dcf2 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 @@ -2,6 +2,7 @@ import androidx.annotation.NonNull; import androidx.annotation.Nullable; +import androidx.annotation.VisibleForTesting; import com.launchdarkly.eventsource.EventHandler; import com.launchdarkly.eventsource.EventSource; @@ -202,7 +203,8 @@ private URI getUri(@Nullable LDContext context) { return uri; } - private void handle(final String name, final String eventData, + @VisibleForTesting + void handle(final String name, final String eventData, @NonNull final Callback resultCallback) { switch (name.toLowerCase()) { case PUT: @@ -226,7 +228,7 @@ private void handle(final String name, final String eventData, break; case PING: ConnectivityManager.fetchAndSetData(fetcher, currentContext, dataSourceUpdateSink, - LDUtil.noOpCallback(), logger); + resultCallback, logger); break; default: logger.debug("Found an unknown stream protocol: {}", name); diff --git a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/ContextDataManagerListenersTest.java b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/ContextDataManagerListenersTest.java index 33f3373e..e6550290 100644 --- a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/ContextDataManagerListenersTest.java +++ b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/ContextDataManagerListenersTest.java @@ -4,6 +4,8 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import com.launchdarkly.sdk.LDContext; +import com.launchdarkly.sdk.LDValue; import com.launchdarkly.sdk.android.DataModel.Flag; import org.junit.Test; @@ -86,6 +88,44 @@ public void listenerIsNotCalledAfterUnregistering() throws InterruptedException verifyAll(); } + @Test + public void listenerIsCalledAfterInitData() { + + LDContext context = LDContext.create("user"); + final String FLAG_KEY = "key"; + Flag flagState1 = new FlagBuilder(FLAG_KEY).value(LDValue.of(1)).build(); + + final ContextDataManager manager = createDataManager(); + + // register some listeners + AwaitableFlagListener specific1 = new AwaitableFlagListener(); + AwaitableFlagListener all1 = new AwaitableFlagListener(); + manager.registerListener(FLAG_KEY, specific1); + manager.registerAllFlagsListener(all1); + + // change the data + manager.upsert(flagState1); + + // verify callbacks + assertEquals(FLAG_KEY, specific1.expectUpdate(5, TimeUnit.SECONDS)); + assertEquals(FLAG_KEY, all1.expectUpdate(5, TimeUnit.SECONDS)); + + // register some more listeners + AwaitableFlagListener specific2 = new AwaitableFlagListener(); + AwaitableFlagListener all2 = new AwaitableFlagListener(); + manager.registerListener(FLAG_KEY, specific2); + manager.registerAllFlagsListener(all2); + + // simulate switching context + Flag flagState2 = new FlagBuilder(FLAG_KEY).value(LDValue.of(2)).build(); + EnvironmentData envData = new EnvironmentData().withFlagUpdatedOrAdded(flagState2); + manager.initData(context, envData); + + // verify callbacks + assertEquals(FLAG_KEY, specific2.expectUpdate(5, TimeUnit.SECONDS)); + assertEquals(FLAG_KEY, all2.expectUpdate(5, TimeUnit.SECONDS)); + } + @Test public void listenerIsCalledOnMainThread() throws InterruptedException { Flag flag = Flag.deletedItemPlaceholder("flag", 1); 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 90339a4e..2c6a086c 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 @@ -1,14 +1,18 @@ package com.launchdarkly.sdk.android; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; import com.launchdarkly.sdk.LDContext; +import com.launchdarkly.sdk.android.subsystems.Callback; import com.launchdarkly.sdk.android.subsystems.ClientContext; import com.launchdarkly.sdk.android.subsystems.DataSource; import org.junit.Rule; import org.junit.Test; +import java.io.IOException; + public class StreamingDataSourceTest { // This class doesn't currently include any tests done with real HTTP, because we don't yet have // a good solution for setting up an embedded HTTP server with chunked streaming data. When we @@ -37,6 +41,39 @@ private ClientContext makeClientContext(boolean inBackground, Boolean previously ); } + // When react to a PING message from the stream, + // the StreamingDataSource will try to use the fetcher + // to get flag data, so we need to create a ClientContext (Android runtime state) + // that has a fetcher + private ClientContext makeClientContextWithFetcher() { + ClientContext baseClientContext = ClientContextImpl.fromConfig( + new LDConfig.Builder().build(), "", "", makeFeatureFetcher(), CONTEXT, + logging.logger, platformState, taskExecutor); + return ClientContextImpl.forDataSource( + baseClientContext, + dataSourceUpdateSink, + CONTEXT, + false, //Not in background + false //Not Previously in background + ); + } + private FeatureFetcher makeFeatureFetcher() { + return new FeatureFetcher() { + @Override + public void close() throws IOException { + // Do nothing + } + + @Override + public void fetch(LDContext context, Callback callback) { + String json = "{" + + "\"flag1\":{\"key\":\"flag1\",\"version\":200,\"value\":false}" + + "}"; + callback.onSuccess(json); + } + }; + } + @Test public void builderCreatesStreamingDataSourceWhenStartingInForeground() { ClientContext clientContext = makeClientContext(false, null); @@ -86,4 +123,26 @@ public void builderCreatesStreamingDataSourceWhenStartingInBackgroundWithOverrid assertEquals(StreamingDataSource.class, ds.getClass()); } + @Test + public void handlePingMessageBehavior() { + ClientContext clientContext = makeClientContextWithFetcher(); + StreamingDataSource sds = (StreamingDataSource) Components.streamingDataSource() + .streamEvenInBackground(true) + .build(clientContext); + + final Boolean[] callbackInvoked = {false}; + Callback callback = new Callback() { + @Override + public void onSuccess(Boolean result) { + callbackInvoked[0] = true; + } + + @Override + public void onError(Throwable error) { + // We are testing the callback is getting call, not which callback is called + } + }; + sds.handle("ping", null, callback); + assertTrue(callbackInvoked[0]); + } }