Migrate firebase-appcheck to use standard executors provided by Firebase Common. (#4431)

* Migrate to use standard executors provided by Firebase Common.

* Update changelog and fix unit test.

* Address review comments.

* Fix unit test.

* Address review comments.

Author: rosalyntan

appcheck/firebase-appcheck/CHANGELOG.md

@@ -1,4 +1,5 @@
 # Unreleased
+* [changed] Migrated the `firebase-appcheck` library to use standard Firebase executors. (#4431)
 
 # 16.1.0
 * [unchanged] Updated to accommodate the release of the updated

appcheck/firebase-appcheck/firebase-appcheck.gradle

@@ -41,6 +41,7 @@ android {
 }
 
 dependencies {
+    implementation project(':firebase-annotations')
     implementation project(':firebase-common')
     implementation project(':firebase-components')
     implementation project(":appcheck:firebase-appcheck-interop")
@@ -49,6 +50,7 @@ dependencies {
 
     javadocClasspath 'com.google.auto.value:auto-value-annotations:1.6.6'
 
+    testImplementation project(':integ-testing')
     testImplementation 'junit:junit:4.13-beta-2'
     testImplementation 'org.mockito:mockito-core:2.25.0'
     testImplementation 'org.mockito:mockito-inline:2.25.0'

appcheck/firebase-appcheck/src/main/java/com/google/firebase/appcheck/FirebaseAppCheckRegistrar.java

@@ -16,16 +16,21 @@
 
 import com.google.android.gms.common.annotation.KeepForSdk;
 import com.google.firebase.FirebaseApp;
+import com.google.firebase.annotations.concurrent.Background;
+import com.google.firebase.annotations.concurrent.Blocking;
 import com.google.firebase.appcheck.internal.DefaultFirebaseAppCheck;
 import com.google.firebase.appcheck.interop.InternalAppCheckTokenProvider;
 import com.google.firebase.components.Component;
 import com.google.firebase.components.ComponentRegistrar;
 import com.google.firebase.components.Dependency;
+import com.google.firebase.components.Qualified;
 import com.google.firebase.heartbeatinfo.HeartBeatConsumerComponent;
 import com.google.firebase.heartbeatinfo.HeartBeatController;
 import com.google.firebase.platforminfo.LibraryVersionComponent;
 import java.util.Arrays;
 import java.util.List;
+import java.util.concurrent.Executor;
+import java.util.concurrent.ScheduledExecutorService;
 
 /**
  * {@link ComponentRegistrar} for setting up FirebaseAppCheck's dependency injections in Firebase
@@ -39,16 +44,24 @@ public class FirebaseAppCheckRegistrar implements ComponentRegistrar {
 
   @Override
   public List<Component<?>> getComponents() {
+    Qualified<Executor> backgroundExecutor = Qualified.qualified(Background.class, Executor.class);
+    Qualified<ScheduledExecutorService> blockingScheduledExecutorService =
+        Qualified.qualified(Blocking.class, ScheduledExecutorService.class);
+
     return Arrays.asList(
         Component.builder(FirebaseAppCheck.class, (InternalAppCheckTokenProvider.class))
             .name(LIBRARY_NAME)
             .add(Dependency.required(FirebaseApp.class))
+            .add(Dependency.required(backgroundExecutor))
+            .add(Dependency.required(blockingScheduledExecutorService))
             .add(Dependency.optionalProvider(HeartBeatController.class))
             .factory(
                 (container) ->
                     new DefaultFirebaseAppCheck(
                         container.get(FirebaseApp.class),
-                        container.getProvider(HeartBeatController.class)))
+                        container.getProvider(HeartBeatController.class),
+                        container.get(backgroundExecutor),
+                        container.get(blockingScheduledExecutorService)))
             .alwaysEager()
             .build(),
         HeartBeatConsumerComponent.create(),

appcheck/firebase-appcheck/src/main/java/com/google/firebase/appcheck/internal/DefaultFirebaseAppCheck.java

@@ -25,6 +25,8 @@
 import com.google.android.gms.tasks.Tasks;
 import com.google.firebase.FirebaseApp;
 import com.google.firebase.FirebaseException;
+import com.google.firebase.annotations.concurrent.Background;
+import com.google.firebase.annotations.concurrent.Blocking;
 import com.google.firebase.appcheck.AppCheckProvider;
 import com.google.firebase.appcheck.AppCheckProviderFactory;
 import com.google.firebase.appcheck.AppCheckToken;
@@ -36,8 +38,8 @@
 import com.google.firebase.inject.Provider;
 import java.util.ArrayList;
 import java.util.List;
-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Executors;
+import java.util.concurrent.Executor;
+import java.util.concurrent.ScheduledExecutorService;
 
 public class DefaultFirebaseAppCheck extends FirebaseAppCheck {
 
@@ -49,30 +51,19 @@ public class DefaultFirebaseAppCheck extends FirebaseAppCheck {
   private final List<AppCheckListener> appCheckListenerList;
   private final StorageHelper storageHelper;
   private final TokenRefreshManager tokenRefreshManager;
-  private final ExecutorService backgroundExecutor;
+  private final Executor backgroundExecutor;
   private final Task<Void> retrieveStoredTokenTask;
   private final Clock clock;
 
   private AppCheckProviderFactory appCheckProviderFactory;
   private AppCheckProvider appCheckProvider;
   private AppCheckToken cachedToken;
 
-  // TODO(b/258273630): Migrate to go/firebase-android-executors
-  @SuppressLint("ThreadPoolCreation")
   public DefaultFirebaseAppCheck(
-      @NonNull FirebaseApp firebaseApp,
-      @NonNull Provider<HeartBeatController> heartBeatController) {
-    this(
-        checkNotNull(firebaseApp),
-        checkNotNull(heartBeatController),
-        Executors.newCachedThreadPool());
-  }
-
-  @VisibleForTesting
-  DefaultFirebaseAppCheck(
       @NonNull FirebaseApp firebaseApp,
       @NonNull Provider<HeartBeatController> heartBeatController,
-      @NonNull ExecutorService backgroundExecutor) {
+      @Background Executor backgroundExecutor,
+      @Blocking ScheduledExecutorService scheduledExecutorService) {
     checkNotNull(firebaseApp);
     checkNotNull(heartBeatController);
     this.firebaseApp = firebaseApp;
@@ -82,13 +73,16 @@ public DefaultFirebaseAppCheck(
     this.storageHelper =
         new StorageHelper(firebaseApp.getApplicationContext(), firebaseApp.getPersistenceKey());
     this.tokenRefreshManager =
-        new TokenRefreshManager(firebaseApp.getApplicationContext(), /* firebaseAppCheck= */ this);
+        new TokenRefreshManager(
+            firebaseApp.getApplicationContext(),
+            /* firebaseAppCheck= */ this,
+            scheduledExecutorService);
     this.backgroundExecutor = backgroundExecutor;
     this.retrieveStoredTokenTask = retrieveStoredAppCheckTokenInBackground(backgroundExecutor);
     this.clock = new Clock.DefaultClock();
   }
 
-  private Task<Void> retrieveStoredAppCheckTokenInBackground(@NonNull ExecutorService executor) {
+  private Task<Void> retrieveStoredAppCheckTokenInBackground(@NonNull Executor executor) {
     TaskCompletionSource<Void> taskCompletionSource = new TaskCompletionSource<>();
     executor.execute(
         () -> {

appcheck/firebase-appcheck/src/main/java/com/google/firebase/appcheck/internal/DefaultTokenRefresher.java

@@ -21,7 +21,7 @@
 import android.annotation.SuppressLint;
 import androidx.annotation.NonNull;
 import androidx.annotation.VisibleForTesting;
-import java.util.concurrent.Executors;
+import com.google.firebase.annotations.concurrent.Blocking;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.ScheduledFuture;
 
@@ -41,16 +41,10 @@ public class DefaultTokenRefresher {
   private volatile ScheduledFuture<?> refreshFuture;
   private volatile long delayAfterFailureSeconds;
 
-  // TODO(b/258273630): Migrate to go/firebase-android-executors
-  @SuppressLint("ThreadPoolCreation")
-  DefaultTokenRefresher(@NonNull DefaultFirebaseAppCheck firebaseAppCheck) {
-    this(checkNotNull(firebaseAppCheck), Executors.newScheduledThreadPool(/* corePoolSize= */ 1));
-  }
-
-  @VisibleForTesting
   DefaultTokenRefresher(
-      DefaultFirebaseAppCheck firebaseAppCheck, ScheduledExecutorService scheduledExecutorService) {
-    this.firebaseAppCheck = firebaseAppCheck;
+      @NonNull DefaultFirebaseAppCheck firebaseAppCheck,
+      @Blocking ScheduledExecutorService scheduledExecutorService) {
+    this.firebaseAppCheck = checkNotNull(firebaseAppCheck);
     this.scheduledExecutorService = scheduledExecutorService;
     this.delayAfterFailureSeconds = UNSET_DELAY;
   }

appcheck/firebase-appcheck/src/main/java/com/google/firebase/appcheck/internal/TokenRefreshManager.java

@@ -22,8 +22,10 @@
 import androidx.annotation.VisibleForTesting;
 import com.google.android.gms.common.api.internal.BackgroundDetector;
 import com.google.android.gms.common.api.internal.BackgroundDetector.BackgroundStateChangeListener;
+import com.google.firebase.annotations.concurrent.Blocking;
 import com.google.firebase.appcheck.AppCheckToken;
 import com.google.firebase.appcheck.internal.util.Clock;
+import java.util.concurrent.ScheduledExecutorService;
 
 /** Class to manage whether or not to schedule an {@link AppCheckToken} refresh attempt. */
 public final class TokenRefreshManager {
@@ -41,10 +43,13 @@ public final class TokenRefreshManager {
   private volatile long nextRefreshTimeMillis;
   private volatile boolean isAutoRefreshEnabled;
 
-  TokenRefreshManager(@NonNull Context context, @NonNull DefaultFirebaseAppCheck firebaseAppCheck) {
+  TokenRefreshManager(
+      @NonNull Context context,
+      @NonNull DefaultFirebaseAppCheck firebaseAppCheck,
+      @Blocking ScheduledExecutorService scheduledExecutorService) {
     this(
         checkNotNull(context),
-        new DefaultTokenRefresher(checkNotNull(firebaseAppCheck)),
+        new DefaultTokenRefresher(checkNotNull(firebaseAppCheck), scheduledExecutorService),
         new Clock.DefaultClock());
   }
 

appcheck/firebase-appcheck/src/test/java/com/google/firebase/appcheck/FirebaseAppCheckRegistrarTest.java

@@ -17,10 +17,15 @@
 import static com.google.common.truth.Truth.assertThat;
 
 import com.google.firebase.FirebaseApp;
+import com.google.firebase.annotations.concurrent.Background;
+import com.google.firebase.annotations.concurrent.Blocking;
 import com.google.firebase.components.Component;
 import com.google.firebase.components.Dependency;
+import com.google.firebase.components.Qualified;
 import com.google.firebase.heartbeatinfo.HeartBeatController;
 import java.util.List;
+import java.util.concurrent.Executor;
+import java.util.concurrent.ScheduledExecutorService;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.robolectric.RobolectricTestRunner;
@@ -39,6 +44,9 @@ public void testGetComponents() {
     assertThat(firebaseAppCheckComponent.getDependencies())
         .containsExactly(
             Dependency.required(FirebaseApp.class),
+            Dependency.required(Qualified.qualified(Background.class, Executor.class)),
+            Dependency.required(
+                Qualified.qualified(Blocking.class, ScheduledExecutorService.class)),
             Dependency.optionalProvider(HeartBeatController.class));
     assertThat(firebaseAppCheckComponent.isAlwaysEager()).isTrue();
   }

appcheck/firebase-appcheck/src/test/java/com/google/firebase/appcheck/internal/DefaultFirebaseAppCheckTest.java

@@ -32,6 +32,7 @@
 import com.google.firebase.appcheck.AppCheckTokenResult;
 import com.google.firebase.appcheck.FirebaseAppCheck.AppCheckListener;
 import com.google.firebase.appcheck.interop.AppCheckTokenListener;
+import com.google.firebase.concurrent.TestOnlyExecutors;
 import com.google.firebase.heartbeatinfo.HeartBeatController;
 import org.junit.Before;
 import org.junit.Test;
@@ -76,19 +77,26 @@ public void setup() {
     when(mockAppCheckProviderFactory.create(any())).thenReturn(mockAppCheckProvider);
     when(mockAppCheckProvider.getToken()).thenReturn(Tasks.forResult(validDefaultAppCheckToken));
 
+    // TODO(b/258273630): Use TestOnlyExecutors.background() instead of
+    // MoreExecutors.directExecutor().
     defaultFirebaseAppCheck =
         new DefaultFirebaseAppCheck(
             mockFirebaseApp,
             () -> mockHeartBeatController,
-            MoreExecutors.newDirectExecutorService());
+            MoreExecutors.directExecutor(),
+            TestOnlyExecutors.blocking());
   }
 
   @Test
   public void testConstructor_nullFirebaseApp_expectThrows() {
     assertThrows(
         NullPointerException.class,
         () -> {
-          new DefaultFirebaseAppCheck(null, () -> mockHeartBeatController);
+          new DefaultFirebaseAppCheck(
+              null,
+              () -> mockHeartBeatController,
+              TestOnlyExecutors.background(),
+              TestOnlyExecutors.blocking());
         });
   }
 
@@ -97,7 +105,8 @@ public void testConstructor_nullHeartBeatControllerProvider_expectThrows() {
     assertThrows(
         NullPointerException.class,
         () -> {
-          new DefaultFirebaseAppCheck(mockFirebaseApp, null);
+          new DefaultFirebaseAppCheck(
+              mockFirebaseApp, null, TestOnlyExecutors.background(), TestOnlyExecutors.blocking());
         });
   }