Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Dec 10, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions appcheck/firebase-appcheck/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
2 changes: 2 additions & 0 deletions appcheck/firebase-appcheck/firebase-appcheck.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ android {
}

dependencies {
implementation project(':firebase-annotations')
implementation project(':firebase-common')
implementation project(':firebase-components')
implementation project(":appcheck:firebase-appcheck-interop")
Expand All @@ -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'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 {

Expand All @@ -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;
Expand All @@ -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(
() -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
});
}

Expand All @@ -97,7 +105,8 @@ public void testConstructor_nullHeartBeatControllerProvider_expectThrows() {
assertThrows(
NullPointerException.class,
() -> {
new DefaultFirebaseAppCheck(mockFirebaseApp, null);
new DefaultFirebaseAppCheck(
mockFirebaseApp, null, TestOnlyExecutors.background(), TestOnlyExecutors.blocking());
});
}

Expand Down