Inject ScheduledExecutorService so it can be tested.

Author: danasilver

firebase-config/src/main/java/com/google/firebase/remoteconfig/RemoteConfigComponent.java

@@ -44,6 +44,7 @@
 import java.util.concurrent.Executor;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.atomic.AtomicReference;
 
 /**
@@ -85,6 +86,7 @@ public class RemoteConfigComponent {
 
   private final Context context;
   private final ExecutorService executorService;
+  private final ScheduledExecutorService scheduledExecutorService;
   private final FirebaseApp firebaseApp;
   private final FirebaseInstallationsApi firebaseInstallations;
   private final FirebaseABTesting firebaseAbt;
@@ -105,6 +107,7 @@ public class RemoteConfigComponent {
     this(
         context,
         Executors.newCachedThreadPool(),
+        Executors.newSingleThreadScheduledExecutor(),
         firebaseApp,
         firebaseInstallations,
         firebaseAbt,
@@ -117,13 +120,15 @@ public class RemoteConfigComponent {
   protected RemoteConfigComponent(
       Context context,
       ExecutorService executorService,
+      ScheduledExecutorService scheduledExecutorService,
       FirebaseApp firebaseApp,
       FirebaseInstallationsApi firebaseInstallations,
       FirebaseABTesting firebaseAbt,
       Provider<AnalyticsConnector> analyticsConnector,
       boolean loadGetDefault) {
     this.context = context;
     this.executorService = executorService;
+    this.scheduledExecutorService = scheduledExecutorService;
     this.firebaseApp = firebaseApp;
     this.firebaseInstallations = firebaseInstallations;
     this.firebaseAbt = firebaseAbt;
@@ -280,7 +285,8 @@ synchronized ConfigRealtimeHandler getRealtime(
         activatedCacheClient,
         context,
         namespace,
-        executorService);
+            scheduledExecutorService
+        );
   }
 
   private ConfigGetParameterHandler getGetHandler(

firebase-config/src/main/java/com/google/firebase/remoteconfig/internal/ConfigAutoFetch.java

@@ -33,7 +33,7 @@
 import java.util.HashSet;
 import java.util.Random;
 import java.util.Set;
-import java.util.concurrent.Executors;
+import java.util.concurrent.ExecutorService;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.TimeUnit;
 import org.json.JSONException;
@@ -53,21 +53,22 @@ public class ConfigAutoFetch {
   private final ConfigFetchHandler configFetchHandler;
   private final ConfigCacheClient activatedCache;
   private final ConfigUpdateListener retryCallback;
-  private final ScheduledExecutorService scheduledExecutorService;
+  private final ScheduledExecutorService executorService;
   private final Random random;
 
   public ConfigAutoFetch(
       HttpURLConnection httpURLConnection,
       ConfigFetchHandler configFetchHandler,
       ConfigCacheClient activatedCache,
       Set<ConfigUpdateListener> eventListeners,
-      ConfigUpdateListener retryCallback) {
+      ConfigUpdateListener retryCallback,
+      ScheduledExecutorService executorService) {
     this.httpURLConnection = httpURLConnection;
     this.configFetchHandler = configFetchHandler;
     this.activatedCache = activatedCache;
     this.eventListeners = eventListeners;
     this.retryCallback = retryCallback;
-    this.scheduledExecutorService = Executors.newSingleThreadScheduledExecutor();
+    this.executorService = executorService;
     this.random = new Random();
   }
 
@@ -117,9 +118,9 @@ public void listenForNotifications() {
 
     // TODO: Factor ConfigUpdateListener out of internal retry logic.
     retryCallback.onUpdate(new HashSet<>());
-    scheduledExecutorService.shutdownNow();
+    executorService.shutdownNow();
     try {
-      scheduledExecutorService.awaitTermination(3L, TimeUnit.SECONDS);
+      executorService.awaitTermination(3L, TimeUnit.SECONDS);
     } catch (InterruptedException ex) {
       Log.d(TAG, "Thread Interrupted.");
     }
@@ -188,7 +189,7 @@ private void autoFetch(int remainingAttempts, long targetVersion) {
 
     // Needs fetch to occur between 0 - 4 seconds. Randomize to not cause ddos alerts in backend
     int timeTillFetch = random.nextInt(4);
-    scheduledExecutorService.schedule(
+    executorService.schedule(
         new Runnable() {
           @Override
           public void run() {
@@ -206,7 +207,7 @@ public synchronized Task<Void> fetchLatestConfig(int remainingAttempts, long tar
 
     return Tasks.whenAllComplete(fetchTask, activatedConfigsTask)
         .continueWithTask(
-            scheduledExecutorService,
+                executorService,
             (listOfUnusedCompletedTasks) -> {
               if (!fetchTask.isSuccessful()) {
                 return Tasks.forException(

firebase-config/src/main/java/com/google/firebase/remoteconfig/internal/ConfigRealtimeHandler.java

@@ -24,9 +24,9 @@
 import com.google.firebase.remoteconfig.FirebaseRemoteConfigException;
 import java.util.LinkedHashSet;
 import java.util.Set;
-import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Future;
 import java.util.concurrent.FutureTask;
+import java.util.concurrent.ScheduledExecutorService;
 
 public class ConfigRealtimeHandler {
 
@@ -42,7 +42,7 @@ public class ConfigRealtimeHandler {
   private final ConfigCacheClient activatedCacheClient;
   private final Context context;
   private final String namespace;
-  private final ExecutorService executorService;
+  private final ScheduledExecutorService scheduledExecutorService;
 
   public ConfigRealtimeHandler(
       FirebaseApp firebaseApp,
@@ -51,7 +51,7 @@ public ConfigRealtimeHandler(
       ConfigCacheClient activatedCacheClient,
       Context context,
       String namespace,
-      ExecutorService executorService) {
+      ScheduledExecutorService scheduledExecutorService) {
 
     this.listeners = new LinkedHashSet<>();
     this.realtimeHttpClientTask = null;
@@ -62,7 +62,7 @@ public ConfigRealtimeHandler(
     this.activatedCacheClient = activatedCacheClient;
     this.context = context;
     this.namespace = namespace;
-    this.executorService = executorService;
+    this.scheduledExecutorService = scheduledExecutorService;
   }
 
   private synchronized boolean canCreateRealtimeHttpClientTask() {
@@ -96,9 +96,10 @@ private synchronized void beginRealtime() {
               activatedCacheClient,
               context,
               namespace,
-              listeners);
+              listeners,
+                  scheduledExecutorService);
       this.realtimeHttpClientTask =
-          this.executorService.submit(
+          this.scheduledExecutorService.submit(
               new RealtimeHttpClientFutureTask(
                   createRealtimeHttpClientTask(realtimeHttpClient), realtimeHttpClient));
     }

firebase-config/src/main/java/com/google/firebase/remoteconfig/internal/ConfigRealtimeHttpClient.java

@@ -46,7 +46,6 @@
 import java.util.Map;
 import java.util.Random;
 import java.util.Set;
-import java.util.concurrent.Executors;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.TimeUnit;
 import java.util.regex.Matcher;
@@ -95,11 +94,12 @@ public ConfigRealtimeHttpClient(
       ConfigCacheClient activatedCache,
       Context context,
       String namespace,
-      Set<ConfigUpdateListener> listeners) {
+      Set<ConfigUpdateListener> listeners,
+      ScheduledExecutorService scheduledExecutorService) {
 
     this.listeners = listeners;
     this.httpURLConnection = null;
-    this.scheduledExecutorService = Executors.newSingleThreadScheduledExecutor();
+    this.scheduledExecutorService = scheduledExecutorService;
 
     // Retry parameters
     this.random = new Random();
@@ -307,7 +307,7 @@ public void onError(@NonNull FirebaseRemoteConfigException error) {
         };
 
     return new ConfigAutoFetch(
-        httpURLConnection, configFetchHandler, activatedCache, listeners, retryCallback);
+        httpURLConnection, configFetchHandler, activatedCache, listeners, retryCallback, scheduledExecutorService);
   }
 
   // HTTP status code that the Realtime client should retry on.

firebase-config/src/test/java/com/google/firebase/remoteconfig/FirebaseRemoteConfigTest.java

@@ -14,6 +14,7 @@
 
 package com.google.firebase.remoteconfig;
 
+import static android.os.AsyncTask.THREAD_POOL_EXECUTOR;
 import static androidx.test.ext.truth.os.BundleSubject.assertThat;
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.truth.Truth.assertWithMessage;
@@ -26,6 +27,7 @@
 import static com.google.firebase.remoteconfig.FirebaseRemoteConfig.toExperimentInfoMaps;
 import static com.google.firebase.remoteconfig.internal.Personalization.EXTERNAL_ARM_VALUE_PARAM;
 import static com.google.firebase.remoteconfig.internal.Personalization.EXTERNAL_PERSONALIZATION_ID_PARAM;
+import static com.google.firebase.remoteconfig.testutil.Assert.assertTrue;
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Matchers.any;
@@ -42,6 +44,7 @@
 
 import android.content.Context;
 import android.content.res.Resources;
+import android.os.AsyncTask;
 import android.os.Bundle;
 import androidx.annotation.NonNull;
 import androidx.test.core.app.ApplicationProvider;
@@ -83,7 +86,14 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.Callable;
+import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.Executor;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
+
 import org.json.JSONArray;
 import org.json.JSONException;
 import org.json.JSONObject;
@@ -93,7 +103,9 @@
 import org.mockito.ArgumentCaptor;
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
+import org.robolectric.Robolectric;
 import org.robolectric.RobolectricTestRunner;
+import org.robolectric.android.util.concurrent.InlineExecutorService;
 import org.robolectric.annotation.Config;
 import org.robolectric.annotation.LooperMode;
 import org.skyscreamer.jsonassert.JSONAssert;
@@ -179,9 +191,10 @@ public final class FirebaseRemoteConfigTest {
   private ConfigContainer realtimeFetchedContainer;
   private ConfigAutoFetch configAutoFetch;
   private ConfigRealtimeHttpClient configRealtimeHttpClient;
-
   private FetchResponse firstFetchedContainerResponse;
 
+  private final ScheduledExecutorService executorService = Executors.newSingleThreadScheduledExecutor();
+
   @Before
   public void setUp() throws Exception {
     DEFAULTS_MAP.put("first_default_key", "first_default_value");
@@ -324,7 +337,8 @@ public void onError(@NonNull FirebaseRemoteConfigException error) {
             mockFetchHandler,
             mockActivatedCache,
             listeners,
-            mockRetryListener);
+            mockRetryListener,
+                executorService);
     configRealtimeHttpClient =
         new ConfigRealtimeHttpClient(
             firebaseApp,
@@ -333,7 +347,8 @@ public void onError(@NonNull FirebaseRemoteConfigException error) {
             mockActivatedCache,
             context,
             "firebase",
-            listeners);
+            listeners,
+                executorService);
   }
 
   @Test
@@ -1300,51 +1315,42 @@ public void realtime_stream_listen_get_inputstream_fail() throws Exception {
 
   @Test
   public void realtime_stream_autofetch_success() throws Exception {
+    // Setup activated configs with keys "string_param", "long_param"
+    loadCacheWithConfig(mockActivatedCache, firstFetchedContainer);
     when(mockFetchHandler.getTemplateVersionNumber()).thenReturn(1L);
     when(mockFetchHandler.fetch(0L)).thenReturn(Tasks.forResult(realtimeFetchedContainerResponse));
 
-    // Setup activated configs with keys "string_param", "long_param"
-    loadCacheWithConfig(mockActivatedCache, firstFetchedContainer);
-    Set<String> updatedParams = Sets.newHashSet("realtime_param");
+    configAutoFetch.fetchLatestConfig(1, 1);
+    flushTasks();
 
-    configAutoFetch
-        .fetchLatestConfig(1, 1)
-        .addOnCompleteListener(
-            unused -> {
-              verify(mockOnUpdateListener).onUpdate(updatedParams);
-            });
+    Set<String> updatedParams = Sets.newHashSet("realtime_param");
+    verify(mockOnUpdateListener).onUpdate(updatedParams);
   }
 
   @Test
-  public void realtime_autofetchBeforeActivate_callsOnUpdateWithAllFetchedParams() {
+  public void realtime_autofetchBeforeActivate_callsOnUpdateWithAllFetchedParams() throws Exception {
+    // The first call to get() returns null while the cache is loading.
+    loadCacheWithConfig(mockActivatedCache, null);
     when(mockFetchHandler.getTemplateVersionNumber()).thenReturn(1L);
     when(mockFetchHandler.fetch(0)).thenReturn(Tasks.forResult(realtimeFetchedContainerResponse));
 
-    // The first call to get() returns null while the cache is loading.
-    loadCacheWithConfig(mockActivatedCache, null);
+    configAutoFetch.fetchLatestConfig(1, 1);
+    flushTasks();
 
     Set<String> updatedParams = Sets.newHashSet("string_param", "long_param", "realtime_param");
-
-    configAutoFetch
-        .fetchLatestConfig(1, 1)
-        .addOnCompleteListener(
-            unused -> {
-              verify(mockOnUpdateListener).onUpdate(updatedParams);
-            });
+    verify(mockOnUpdateListener).onUpdate(updatedParams);
   }
 
   @Test
-  public void realtime_stream_autofetch_failure() {
+  public void realtime_stream_autofetch_failure() throws Exception {
+    loadCacheWithConfig(mockActivatedCache, firstFetchedContainer);
     when(mockFetchHandler.getTemplateVersionNumber()).thenReturn(1L);
     when(mockFetchHandler.fetch(0)).thenReturn(Tasks.forResult(realtimeFetchedContainerResponse));
 
-    configAutoFetch
-        .fetchLatestConfig(1, 1000)
-        .addOnCompleteListener(
-            unused -> {
-              verify(mockNotFetchedEventListener)
-                  .onError(any(FirebaseRemoteConfigServerException.class));
-            });
+    configAutoFetch.fetchLatestConfig(1, 1000);
+    flushTasks();
+
+    verify(mockNotFetchedEventListener).onError(any(FirebaseRemoteConfigServerException.class));
   }
 
   private static void loadCacheWithConfig(
@@ -1424,6 +1430,17 @@ private static FirebaseApp initializeFirebaseApp(Context context) {
             .build());
   }
 
+  /**
+   * Flush tasks on the {@code executorService}'s thread.
+   *
+   * @throws InterruptedException
+   */
+  private void flushTasks() throws InterruptedException {
+    CountDownLatch latch = new CountDownLatch(1);
+    executorService.execute(latch::countDown);
+    assertTrue("Task didn't finish", latch.await(10, TimeUnit.MILLISECONDS));
+  }
+
   private ConfigUpdateListener generateEmptyRealtimeListener() {
     return new ConfigUpdateListener() {
       @Override