prepare 4.2.3 release

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
 

launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ContextDataManager.java

@@ -182,11 +182,18 @@ private void initDataInternal(
         Set<String> 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<String> 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<String> 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(() -> {

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<Boolean> 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);

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);

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<String> 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<Boolean> callback = new Callback<Boolean>() {
+            @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]);
+    }
 }