From 46e4196d68137eabc18e677d0849770baa520a1b Mon Sep 17 00:00:00 2001 From: Rosalyn Tan Date: Wed, 7 Dec 2022 16:23:25 -0800 Subject: [PATCH 1/5] Migrate to use standard executors provided by Firebase Common. --- .../firebase-appcheck.gradle | 1 + .../appcheck/FirebaseAppCheckRegistrar.java | 16 +++++++++++- .../internal/DefaultFirebaseAppCheck.java | 26 +++++++++---------- .../internal/DefaultTokenRefresher.java | 5 +--- .../internal/DefaultFirebaseAppCheckTest.java | 17 +++++++++--- 5 files changed, 43 insertions(+), 22 deletions(-) diff --git a/appcheck/firebase-appcheck/firebase-appcheck.gradle b/appcheck/firebase-appcheck/firebase-appcheck.gradle index 6ed2202dd30..214f5795b1c 100644 --- a/appcheck/firebase-appcheck/firebase-appcheck.gradle +++ b/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") diff --git a/appcheck/firebase-appcheck/src/main/java/com/google/firebase/appcheck/FirebaseAppCheckRegistrar.java b/appcheck/firebase-appcheck/src/main/java/com/google/firebase/appcheck/FirebaseAppCheckRegistrar.java index 8a52acdf20a..f5830da88a9 100644 --- a/appcheck/firebase-appcheck/src/main/java/com/google/firebase/appcheck/FirebaseAppCheckRegistrar.java +++ b/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.ExecutorService; +import java.util.concurrent.ScheduledExecutorService; /** * {@link ComponentRegistrar} for setting up FirebaseAppCheck's dependency injections in Firebase @@ -39,16 +44,25 @@ public class FirebaseAppCheckRegistrar implements ComponentRegistrar { @Override public List> getComponents() { + Qualified backgroundExecutorService = + Qualified.qualified(Background.class, ExecutorService.class); + Qualified 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(backgroundExecutorService)) + .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(backgroundExecutorService), + container.get(blockingScheduledExecutorService))) .alwaysEager() .build(), HeartBeatConsumerComponent.create(), diff --git a/appcheck/firebase-appcheck/src/main/java/com/google/firebase/appcheck/internal/DefaultFirebaseAppCheck.java b/appcheck/firebase-appcheck/src/main/java/com/google/firebase/appcheck/internal/DefaultFirebaseAppCheck.java index 11815fdcb28..894ba70e70a 100644 --- a/appcheck/firebase-appcheck/src/main/java/com/google/firebase/appcheck/internal/DefaultFirebaseAppCheck.java +++ b/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; @@ -37,7 +39,7 @@ import java.util.ArrayList; import java.util.List; import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; public class DefaultFirebaseAppCheck extends FirebaseAppCheck { @@ -50,6 +52,7 @@ public class DefaultFirebaseAppCheck extends FirebaseAppCheck { private final StorageHelper storageHelper; private final TokenRefreshManager tokenRefreshManager; private final ExecutorService backgroundExecutor; + private final ScheduledExecutorService scheduledExecutorService; private final Task retrieveStoredTokenTask; private final Clock clock; @@ -57,22 +60,11 @@ public class DefaultFirebaseAppCheck extends FirebaseAppCheck { 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) { - this( - checkNotNull(firebaseApp), - checkNotNull(heartBeatController), - Executors.newCachedThreadPool()); - } - - @VisibleForTesting - DefaultFirebaseAppCheck( @NonNull FirebaseApp firebaseApp, @NonNull Provider heartBeatController, - @NonNull ExecutorService backgroundExecutor) { + @Background ExecutorService backgroundExecutor, + @Blocking ScheduledExecutorService scheduledExecutorService) { checkNotNull(firebaseApp); checkNotNull(heartBeatController); this.firebaseApp = firebaseApp; @@ -84,6 +76,7 @@ public DefaultFirebaseAppCheck( this.tokenRefreshManager = new TokenRefreshManager(firebaseApp.getApplicationContext(), /* firebaseAppCheck= */ this); this.backgroundExecutor = backgroundExecutor; + this.scheduledExecutorService = scheduledExecutorService; this.retrieveStoredTokenTask = retrieveStoredAppCheckTokenInBackground(backgroundExecutor); this.clock = new Clock.DefaultClock(); } @@ -257,6 +250,11 @@ Provider getHeartbeatControllerProvider() { return heartbeatControllerProvider; } + @NonNull + ScheduledExecutorService getScheduledExecutorService() { + return scheduledExecutorService; + } + /** Sets the in-memory cached {@link AppCheckToken}. */ @VisibleForTesting void setCachedToken(@NonNull AppCheckToken token) { diff --git a/appcheck/firebase-appcheck/src/main/java/com/google/firebase/appcheck/internal/DefaultTokenRefresher.java b/appcheck/firebase-appcheck/src/main/java/com/google/firebase/appcheck/internal/DefaultTokenRefresher.java index d2b5e764fa0..d11fa0711cb 100644 --- a/appcheck/firebase-appcheck/src/main/java/com/google/firebase/appcheck/internal/DefaultTokenRefresher.java +++ b/appcheck/firebase-appcheck/src/main/java/com/google/firebase/appcheck/internal/DefaultTokenRefresher.java @@ -21,7 +21,6 @@ import android.annotation.SuppressLint; import androidx.annotation.NonNull; import androidx.annotation.VisibleForTesting; -import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledFuture; @@ -41,10 +40,8 @@ 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)); + this(checkNotNull(firebaseAppCheck), firebaseAppCheck.getScheduledExecutorService()); } @VisibleForTesting diff --git a/appcheck/firebase-appcheck/src/test/java/com/google/firebase/appcheck/internal/DefaultFirebaseAppCheckTest.java b/appcheck/firebase-appcheck/src/test/java/com/google/firebase/appcheck/internal/DefaultFirebaseAppCheckTest.java index 7b065e4a827..3c04e7d5f48 100644 --- a/appcheck/firebase-appcheck/src/test/java/com/google/firebase/appcheck/internal/DefaultFirebaseAppCheckTest.java +++ b/appcheck/firebase-appcheck/src/test/java/com/google/firebase/appcheck/internal/DefaultFirebaseAppCheckTest.java @@ -17,6 +17,7 @@ import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertThrows; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -33,6 +34,7 @@ import com.google.firebase.appcheck.FirebaseAppCheck.AppCheckListener; import com.google.firebase.appcheck.interop.AppCheckTokenListener; import com.google.firebase.heartbeatinfo.HeartBeatController; +import java.util.concurrent.ScheduledExecutorService; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -80,7 +82,8 @@ public void setup() { new DefaultFirebaseAppCheck( mockFirebaseApp, () -> mockHeartBeatController, - MoreExecutors.newDirectExecutorService()); + MoreExecutors.newDirectExecutorService(), + mock(ScheduledExecutorService.class)); } @Test @@ -88,7 +91,11 @@ public void testConstructor_nullFirebaseApp_expectThrows() { assertThrows( NullPointerException.class, () -> { - new DefaultFirebaseAppCheck(null, () -> mockHeartBeatController); + new DefaultFirebaseAppCheck( + null, + () -> mockHeartBeatController, + MoreExecutors.newDirectExecutorService(), + mock(ScheduledExecutorService.class)); }); } @@ -97,7 +104,11 @@ public void testConstructor_nullHeartBeatControllerProvider_expectThrows() { assertThrows( NullPointerException.class, () -> { - new DefaultFirebaseAppCheck(mockFirebaseApp, null); + new DefaultFirebaseAppCheck( + mockFirebaseApp, + null, + MoreExecutors.newDirectExecutorService(), + mock(ScheduledExecutorService.class)); }); } From ccd7ae31be3244eabe7d46cf3ffc9faa734d7456 Mon Sep 17 00:00:00 2001 From: Rosalyn Tan Date: Wed, 7 Dec 2022 16:39:44 -0800 Subject: [PATCH 2/5] Update changelog and fix unit test. --- appcheck/firebase-appcheck/CHANGELOG.md | 1 + .../firebase/appcheck/FirebaseAppCheckRegistrarTest.java | 8 ++++++++ 2 files changed, 9 insertions(+) diff --git a/appcheck/firebase-appcheck/CHANGELOG.md b/appcheck/firebase-appcheck/CHANGELOG.md index e66859e9abe..34188c9770c 100644 --- a/appcheck/firebase-appcheck/CHANGELOG.md +++ b/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 diff --git a/appcheck/firebase-appcheck/src/test/java/com/google/firebase/appcheck/FirebaseAppCheckRegistrarTest.java b/appcheck/firebase-appcheck/src/test/java/com/google/firebase/appcheck/FirebaseAppCheckRegistrarTest.java index c6a39d3a45e..ae04d6f1f3d 100644 --- a/appcheck/firebase-appcheck/src/test/java/com/google/firebase/appcheck/FirebaseAppCheckRegistrarTest.java +++ b/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.ExecutorService; +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, ExecutorService.class)), + Dependency.required( + Qualified.qualified(Blocking.class, ScheduledExecutorService.class)), Dependency.optionalProvider(HeartBeatController.class)); assertThat(firebaseAppCheckComponent.isAlwaysEager()).isTrue(); } From 2032a922c221280f8a236f5acc5041630e114732 Mon Sep 17 00:00:00 2001 From: Rosalyn Tan Date: Fri, 9 Dec 2022 13:28:18 -0800 Subject: [PATCH 3/5] Address review comments. --- .../firebase-appcheck.gradle | 1 + .../appcheck/FirebaseAppCheckRegistrar.java | 9 +++---- .../internal/DefaultFirebaseAppCheck.java | 8 +++--- .../FirebaseAppCheckRegistrarTest.java | 4 +-- .../internal/DefaultFirebaseAppCheckTest.java | 25 ++++++++++--------- 5 files changed, 24 insertions(+), 23 deletions(-) diff --git a/appcheck/firebase-appcheck/firebase-appcheck.gradle b/appcheck/firebase-appcheck/firebase-appcheck.gradle index 214f5795b1c..6529ec89b38 100644 --- a/appcheck/firebase-appcheck/firebase-appcheck.gradle +++ b/appcheck/firebase-appcheck/firebase-appcheck.gradle @@ -50,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' diff --git a/appcheck/firebase-appcheck/src/main/java/com/google/firebase/appcheck/FirebaseAppCheckRegistrar.java b/appcheck/firebase-appcheck/src/main/java/com/google/firebase/appcheck/FirebaseAppCheckRegistrar.java index f5830da88a9..3464c0f2206 100644 --- a/appcheck/firebase-appcheck/src/main/java/com/google/firebase/appcheck/FirebaseAppCheckRegistrar.java +++ b/appcheck/firebase-appcheck/src/main/java/com/google/firebase/appcheck/FirebaseAppCheckRegistrar.java @@ -29,7 +29,7 @@ import com.google.firebase.platforminfo.LibraryVersionComponent; import java.util.Arrays; import java.util.List; -import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executor; import java.util.concurrent.ScheduledExecutorService; /** @@ -44,8 +44,7 @@ public class FirebaseAppCheckRegistrar implements ComponentRegistrar { @Override public List> getComponents() { - Qualified backgroundExecutorService = - Qualified.qualified(Background.class, ExecutorService.class); + Qualified backgroundExecutor = Qualified.qualified(Background.class, Executor.class); Qualified blockingScheduledExecutorService = Qualified.qualified(Blocking.class, ScheduledExecutorService.class); @@ -53,7 +52,7 @@ public List> getComponents() { Component.builder(FirebaseAppCheck.class, (InternalAppCheckTokenProvider.class)) .name(LIBRARY_NAME) .add(Dependency.required(FirebaseApp.class)) - .add(Dependency.required(backgroundExecutorService)) + .add(Dependency.required(backgroundExecutor)) .add(Dependency.required(blockingScheduledExecutorService)) .add(Dependency.optionalProvider(HeartBeatController.class)) .factory( @@ -61,7 +60,7 @@ public List> getComponents() { new DefaultFirebaseAppCheck( container.get(FirebaseApp.class), container.getProvider(HeartBeatController.class), - container.get(backgroundExecutorService), + container.get(backgroundExecutor), container.get(blockingScheduledExecutorService))) .alwaysEager() .build(), diff --git a/appcheck/firebase-appcheck/src/main/java/com/google/firebase/appcheck/internal/DefaultFirebaseAppCheck.java b/appcheck/firebase-appcheck/src/main/java/com/google/firebase/appcheck/internal/DefaultFirebaseAppCheck.java index 894ba70e70a..e735ee87ec5 100644 --- a/appcheck/firebase-appcheck/src/main/java/com/google/firebase/appcheck/internal/DefaultFirebaseAppCheck.java +++ b/appcheck/firebase-appcheck/src/main/java/com/google/firebase/appcheck/internal/DefaultFirebaseAppCheck.java @@ -38,7 +38,7 @@ import com.google.firebase.inject.Provider; import java.util.ArrayList; import java.util.List; -import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executor; import java.util.concurrent.ScheduledExecutorService; public class DefaultFirebaseAppCheck extends FirebaseAppCheck { @@ -51,7 +51,7 @@ public class DefaultFirebaseAppCheck extends FirebaseAppCheck { private final List appCheckListenerList; private final StorageHelper storageHelper; private final TokenRefreshManager tokenRefreshManager; - private final ExecutorService backgroundExecutor; + private final Executor backgroundExecutor; private final ScheduledExecutorService scheduledExecutorService; private final Task retrieveStoredTokenTask; private final Clock clock; @@ -63,7 +63,7 @@ public class DefaultFirebaseAppCheck extends FirebaseAppCheck { public DefaultFirebaseAppCheck( @NonNull FirebaseApp firebaseApp, @NonNull Provider heartBeatController, - @Background ExecutorService backgroundExecutor, + @Background Executor backgroundExecutor, @Blocking ScheduledExecutorService scheduledExecutorService) { checkNotNull(firebaseApp); checkNotNull(heartBeatController); @@ -81,7 +81,7 @@ public DefaultFirebaseAppCheck( this.clock = new Clock.DefaultClock(); } - private Task retrieveStoredAppCheckTokenInBackground(@NonNull ExecutorService executor) { + private Task retrieveStoredAppCheckTokenInBackground(@NonNull Executor executor) { TaskCompletionSource taskCompletionSource = new TaskCompletionSource<>(); executor.execute( () -> { diff --git a/appcheck/firebase-appcheck/src/test/java/com/google/firebase/appcheck/FirebaseAppCheckRegistrarTest.java b/appcheck/firebase-appcheck/src/test/java/com/google/firebase/appcheck/FirebaseAppCheckRegistrarTest.java index ae04d6f1f3d..58149419bd5 100644 --- a/appcheck/firebase-appcheck/src/test/java/com/google/firebase/appcheck/FirebaseAppCheckRegistrarTest.java +++ b/appcheck/firebase-appcheck/src/test/java/com/google/firebase/appcheck/FirebaseAppCheckRegistrarTest.java @@ -24,7 +24,7 @@ import com.google.firebase.components.Qualified; import com.google.firebase.heartbeatinfo.HeartBeatController; import java.util.List; -import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executor; import java.util.concurrent.ScheduledExecutorService; import org.junit.Test; import org.junit.runner.RunWith; @@ -44,7 +44,7 @@ public void testGetComponents() { assertThat(firebaseAppCheckComponent.getDependencies()) .containsExactly( Dependency.required(FirebaseApp.class), - Dependency.required(Qualified.qualified(Background.class, ExecutorService.class)), + Dependency.required(Qualified.qualified(Background.class, Executor.class)), Dependency.required( Qualified.qualified(Blocking.class, ScheduledExecutorService.class)), Dependency.optionalProvider(HeartBeatController.class)); diff --git a/appcheck/firebase-appcheck/src/test/java/com/google/firebase/appcheck/internal/DefaultFirebaseAppCheckTest.java b/appcheck/firebase-appcheck/src/test/java/com/google/firebase/appcheck/internal/DefaultFirebaseAppCheckTest.java index 3c04e7d5f48..0c07556c483 100644 --- a/appcheck/firebase-appcheck/src/test/java/com/google/firebase/appcheck/internal/DefaultFirebaseAppCheckTest.java +++ b/appcheck/firebase-appcheck/src/test/java/com/google/firebase/appcheck/internal/DefaultFirebaseAppCheckTest.java @@ -17,15 +17,15 @@ import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertThrows; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static org.robolectric.Shadows.shadowOf; +import android.os.Looper; import androidx.test.core.app.ApplicationProvider; import com.google.android.gms.tasks.Task; import com.google.android.gms.tasks.Tasks; -import com.google.common.util.concurrent.MoreExecutors; import com.google.firebase.FirebaseApp; import com.google.firebase.appcheck.AppCheckProvider; import com.google.firebase.appcheck.AppCheckProviderFactory; @@ -33,8 +33,8 @@ 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 java.util.concurrent.ScheduledExecutorService; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -44,11 +44,12 @@ import org.robolectric.RobolectricTestRunner; import org.robolectric.annotation.Config; import org.robolectric.annotation.LooperMode; +import org.robolectric.annotation.LooperMode.Mode; /** Tests for {@link DefaultFirebaseAppCheck}. */ @RunWith(RobolectricTestRunner.class) @Config(manifest = Config.NONE) -@LooperMode(LooperMode.Mode.LEGACY) +@LooperMode(Mode.PAUSED) public class DefaultFirebaseAppCheckTest { private static final String EXCEPTION_TEXT = "exceptionText"; @@ -82,8 +83,10 @@ public void setup() { new DefaultFirebaseAppCheck( mockFirebaseApp, () -> mockHeartBeatController, - MoreExecutors.newDirectExecutorService(), - mock(ScheduledExecutorService.class)); + TestOnlyExecutors.background(), + TestOnlyExecutors.blocking()); + + shadowOf(Looper.getMainLooper()).idle(); } @Test @@ -94,8 +97,8 @@ public void testConstructor_nullFirebaseApp_expectThrows() { new DefaultFirebaseAppCheck( null, () -> mockHeartBeatController, - MoreExecutors.newDirectExecutorService(), - mock(ScheduledExecutorService.class)); + TestOnlyExecutors.background(), + TestOnlyExecutors.blocking()); }); } @@ -105,10 +108,7 @@ public void testConstructor_nullHeartBeatControllerProvider_expectThrows() { NullPointerException.class, () -> { new DefaultFirebaseAppCheck( - mockFirebaseApp, - null, - MoreExecutors.newDirectExecutorService(), - mock(ScheduledExecutorService.class)); + mockFirebaseApp, null, TestOnlyExecutors.background(), TestOnlyExecutors.blocking()); }); } @@ -343,6 +343,7 @@ public void testGetToken_existingValidToken_forceRefresh_requestsNewToken() { defaultFirebaseAppCheck.installAppCheckProviderFactory(mockAppCheckProviderFactory); defaultFirebaseAppCheck.getToken(/* forceRefresh= */ true); + shadowOf(Looper.getMainLooper()).idle(); verify(mockAppCheckProvider).getToken(); } From 92e508ea71ce8118054b01fc1152b19e39aaee1e Mon Sep 17 00:00:00 2001 From: Rosalyn Tan Date: Fri, 9 Dec 2022 14:41:02 -0800 Subject: [PATCH 4/5] Fix unit test. --- .../internal/DefaultFirebaseAppCheckTest.java | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/appcheck/firebase-appcheck/src/test/java/com/google/firebase/appcheck/internal/DefaultFirebaseAppCheckTest.java b/appcheck/firebase-appcheck/src/test/java/com/google/firebase/appcheck/internal/DefaultFirebaseAppCheckTest.java index 0c07556c483..822ee3f136c 100644 --- a/appcheck/firebase-appcheck/src/test/java/com/google/firebase/appcheck/internal/DefaultFirebaseAppCheckTest.java +++ b/appcheck/firebase-appcheck/src/test/java/com/google/firebase/appcheck/internal/DefaultFirebaseAppCheckTest.java @@ -20,12 +20,11 @@ import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -import static org.robolectric.Shadows.shadowOf; -import android.os.Looper; import androidx.test.core.app.ApplicationProvider; import com.google.android.gms.tasks.Task; import com.google.android.gms.tasks.Tasks; +import com.google.common.util.concurrent.MoreExecutors; import com.google.firebase.FirebaseApp; import com.google.firebase.appcheck.AppCheckProvider; import com.google.firebase.appcheck.AppCheckProviderFactory; @@ -49,7 +48,7 @@ /** Tests for {@link DefaultFirebaseAppCheck}. */ @RunWith(RobolectricTestRunner.class) @Config(manifest = Config.NONE) -@LooperMode(Mode.PAUSED) +@LooperMode(Mode.LEGACY) public class DefaultFirebaseAppCheckTest { private static final String EXCEPTION_TEXT = "exceptionText"; @@ -79,14 +78,14 @@ 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, - TestOnlyExecutors.background(), + MoreExecutors.directExecutor(), TestOnlyExecutors.blocking()); - - shadowOf(Looper.getMainLooper()).idle(); } @Test @@ -343,7 +342,6 @@ public void testGetToken_existingValidToken_forceRefresh_requestsNewToken() { defaultFirebaseAppCheck.installAppCheckProviderFactory(mockAppCheckProviderFactory); defaultFirebaseAppCheck.getToken(/* forceRefresh= */ true); - shadowOf(Looper.getMainLooper()).idle(); verify(mockAppCheckProvider).getToken(); } From 51dfb74055be87824a34e3bace055b25c50123bd Mon Sep 17 00:00:00 2001 From: Rosalyn Tan Date: Fri, 9 Dec 2022 14:51:17 -0800 Subject: [PATCH 5/5] Address review comments. --- .../appcheck/internal/DefaultFirebaseAppCheck.java | 12 ++++-------- .../appcheck/internal/DefaultTokenRefresher.java | 11 ++++------- .../appcheck/internal/TokenRefreshManager.java | 9 +++++++-- .../internal/DefaultFirebaseAppCheckTest.java | 3 +-- 4 files changed, 16 insertions(+), 19 deletions(-) diff --git a/appcheck/firebase-appcheck/src/main/java/com/google/firebase/appcheck/internal/DefaultFirebaseAppCheck.java b/appcheck/firebase-appcheck/src/main/java/com/google/firebase/appcheck/internal/DefaultFirebaseAppCheck.java index e735ee87ec5..af3a7056635 100644 --- a/appcheck/firebase-appcheck/src/main/java/com/google/firebase/appcheck/internal/DefaultFirebaseAppCheck.java +++ b/appcheck/firebase-appcheck/src/main/java/com/google/firebase/appcheck/internal/DefaultFirebaseAppCheck.java @@ -52,7 +52,6 @@ public class DefaultFirebaseAppCheck extends FirebaseAppCheck { private final StorageHelper storageHelper; private final TokenRefreshManager tokenRefreshManager; private final Executor backgroundExecutor; - private final ScheduledExecutorService scheduledExecutorService; private final Task retrieveStoredTokenTask; private final Clock clock; @@ -74,9 +73,11 @@ 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.scheduledExecutorService = scheduledExecutorService; this.retrieveStoredTokenTask = retrieveStoredAppCheckTokenInBackground(backgroundExecutor); this.clock = new Clock.DefaultClock(); } @@ -250,11 +251,6 @@ Provider getHeartbeatControllerProvider() { return heartbeatControllerProvider; } - @NonNull - ScheduledExecutorService getScheduledExecutorService() { - return scheduledExecutorService; - } - /** Sets the in-memory cached {@link AppCheckToken}. */ @VisibleForTesting void setCachedToken(@NonNull AppCheckToken token) { diff --git a/appcheck/firebase-appcheck/src/main/java/com/google/firebase/appcheck/internal/DefaultTokenRefresher.java b/appcheck/firebase-appcheck/src/main/java/com/google/firebase/appcheck/internal/DefaultTokenRefresher.java index d11fa0711cb..5e265966c0a 100644 --- a/appcheck/firebase-appcheck/src/main/java/com/google/firebase/appcheck/internal/DefaultTokenRefresher.java +++ b/appcheck/firebase-appcheck/src/main/java/com/google/firebase/appcheck/internal/DefaultTokenRefresher.java @@ -21,6 +21,7 @@ import android.annotation.SuppressLint; import androidx.annotation.NonNull; import androidx.annotation.VisibleForTesting; +import com.google.firebase.annotations.concurrent.Blocking; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledFuture; @@ -40,14 +41,10 @@ public class DefaultTokenRefresher { private volatile ScheduledFuture refreshFuture; private volatile long delayAfterFailureSeconds; - DefaultTokenRefresher(@NonNull DefaultFirebaseAppCheck firebaseAppCheck) { - this(checkNotNull(firebaseAppCheck), firebaseAppCheck.getScheduledExecutorService()); - } - - @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; } diff --git a/appcheck/firebase-appcheck/src/main/java/com/google/firebase/appcheck/internal/TokenRefreshManager.java b/appcheck/firebase-appcheck/src/main/java/com/google/firebase/appcheck/internal/TokenRefreshManager.java index ac116dd0ca7..8652dd4edfd 100644 --- a/appcheck/firebase-appcheck/src/main/java/com/google/firebase/appcheck/internal/TokenRefreshManager.java +++ b/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()); } diff --git a/appcheck/firebase-appcheck/src/test/java/com/google/firebase/appcheck/internal/DefaultFirebaseAppCheckTest.java b/appcheck/firebase-appcheck/src/test/java/com/google/firebase/appcheck/internal/DefaultFirebaseAppCheckTest.java index 822ee3f136c..0247a9ffe82 100644 --- a/appcheck/firebase-appcheck/src/test/java/com/google/firebase/appcheck/internal/DefaultFirebaseAppCheckTest.java +++ b/appcheck/firebase-appcheck/src/test/java/com/google/firebase/appcheck/internal/DefaultFirebaseAppCheckTest.java @@ -43,12 +43,11 @@ import org.robolectric.RobolectricTestRunner; import org.robolectric.annotation.Config; import org.robolectric.annotation.LooperMode; -import org.robolectric.annotation.LooperMode.Mode; /** Tests for {@link DefaultFirebaseAppCheck}. */ @RunWith(RobolectricTestRunner.class) @Config(manifest = Config.NONE) -@LooperMode(Mode.LEGACY) +@LooperMode(LooperMode.Mode.LEGACY) public class DefaultFirebaseAppCheckTest { private static final String EXCEPTION_TEXT = "exceptionText";