Simplify sign in state management

Author: lfkellogg

firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/SignInStorage.java

@@ -24,48 +24,56 @@
 
 /** Class that handles storage for App Distribution SignIn persistence. */
 class SignInStorage {
-
   private static final String SIGNIN_PREFERENCES_NAME = "FirebaseAppDistributionSignInStorage";
   private static final String SIGNIN_TAG = "firebase_app_distribution_signin";
 
   private final Context applicationContext;
   @Background private final Executor backgroundExecutor;
+  private SharedPreferences sharedPreferences;
+
+  private interface SharedPreferencesFunction<T> {
+    T apply(SharedPreferences sharedPreferences);
+  }
 
   SignInStorage(Context applicationContext, @Background Executor backgroundExecutor) {
     this.applicationContext = applicationContext;
     this.backgroundExecutor = backgroundExecutor;
   }
 
   Task<Void> setSignInStatus(boolean testerSignedIn) {
-    return getSharedPreferences()
-        .onSuccessTask(
-            backgroundExecutor,
-            sharedPreferences -> {
-              sharedPreferences.edit().putBoolean(SIGNIN_TAG, testerSignedIn).apply();
-              return null;
-            });
+    return applyToSharedPreferences(
+        sharedPreferences -> {
+          sharedPreferences.edit().putBoolean(SIGNIN_TAG, testerSignedIn).apply();
+          return null;
+        });
   }
 
   Task<Boolean> getSignInStatus() {
-    return getSharedPreferences()
-        .onSuccessTask(
-            backgroundExecutor,
-            sharedPreferences -> Tasks.forResult(sharedPreferences.getBoolean(SIGNIN_TAG, false)));
+    return applyToSharedPreferences(
+        sharedPreferences -> sharedPreferences.getBoolean(SIGNIN_TAG, false));
   }
 
   boolean getSignInStatusBlocking() {
     return getSharedPreferencesBlocking().getBoolean(SIGNIN_TAG, false);
   }
 
-  private Task<SharedPreferences> getSharedPreferences() {
-    TaskCompletionSource<SharedPreferences> taskCompletionSource = new TaskCompletionSource<>();
-    backgroundExecutor.execute(
-        () -> taskCompletionSource.setResult(getSharedPreferencesBlocking()));
-    return taskCompletionSource.getTask();
-  }
-
   private SharedPreferences getSharedPreferencesBlocking() {
     // This may construct a new SharedPreferences object, which requires storage I/O
     return applicationContext.getSharedPreferences(SIGNIN_PREFERENCES_NAME, Context.MODE_PRIVATE);
   }
+
+  private <T> Task<T> applyToSharedPreferences(SharedPreferencesFunction<T> func) {
+    // Check nullness of sharedPreferences directly because even if it is initialized twice on
+    // different threads it will be to an equivalent SharedPreferences reference.
+    if (sharedPreferences != null) {
+      return Tasks.forResult(func.apply(sharedPreferences));
+    }
+    TaskCompletionSource<T> taskCompletionSource = new TaskCompletionSource<>();
+    backgroundExecutor.execute(
+        () -> {
+          sharedPreferences = getSharedPreferencesBlocking();
+          taskCompletionSource.setResult(func.apply(sharedPreferences));
+        });
+    return taskCompletionSource.getTask();
+  }
 }

firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/TesterSignInManager.java

@@ -15,6 +15,8 @@
 package com.google.firebase.appdistribution.impl;
 
 import static com.google.firebase.appdistribution.FirebaseAppDistributionException.Status.AUTHENTICATION_CANCELED;
+import static com.google.firebase.appdistribution.FirebaseAppDistributionException.Status.AUTHENTICATION_FAILURE;
+import static com.google.firebase.appdistribution.FirebaseAppDistributionException.Status.UNKNOWN;
 import static com.google.firebase.appdistribution.impl.TaskUtils.safeSetTaskException;
 import static com.google.firebase.appdistribution.impl.TaskUtils.safeSetTaskResult;
 
@@ -101,8 +103,14 @@ void onActivityCreated(Activity activity) {
     // result of the signIn Task in the onActivityCreated callback
     if (activity instanceof SignInResultActivity) {
       LogWrapper.v(TAG, "Sign in completed");
-      this.setSuccessfulSignInResult();
-      this.signInStorage.setSignInStatus(true);
+      this.signInStorage
+          .setSignInStatus(true)
+          .addOnSuccessListener(unused -> this.setSuccessfulSignInResult())
+          .addOnFailureListener(
+              e ->
+                  this.setSignInTaskCompletionError(
+                      new FirebaseAppDistributionException(
+                          "Error storing tester sign in state", UNKNOWN, e)));
     }
   }
 
@@ -160,14 +168,14 @@ private Task<Void> doSignInTester() {
           .getId()
           .addOnFailureListener(
               blockingExecutor,
-              handleTaskFailure(ErrorMessages.AUTHENTICATION_ERROR, Status.AUTHENTICATION_FAILURE))
+              handleTaskFailure(ErrorMessages.AUTHENTICATION_ERROR, AUTHENTICATION_FAILURE))
           .addOnSuccessListener(
               blockingExecutor,
               fid ->
                   getForegroundActivityAndOpenSignInFlow(fid)
                       .addOnFailureListener(
                           blockingExecutor,
-                          handleTaskFailure(ErrorMessages.UNKNOWN_ERROR, Status.UNKNOWN)));
+                          handleTaskFailure(ErrorMessages.UNKNOWN_ERROR, UNKNOWN)));
 
       return signInTaskCompletionSource.getTask();
     }

firebase-appdistribution/src/test/java/com/google/firebase/appdistribution/impl/TesterSignInManagerTest.java

@@ -16,12 +16,15 @@
 
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.firebase.appdistribution.FirebaseAppDistributionException.Status.AUTHENTICATION_CANCELED;
+import static com.google.firebase.appdistribution.FirebaseAppDistributionException.Status.UNKNOWN;
 import static com.google.firebase.appdistribution.impl.TestUtils.assertTaskFailure;
 import static com.google.firebase.appdistribution.impl.TestUtils.awaitAsyncOperations;
 import static com.google.firebase.appdistribution.impl.TestUtils.awaitTask;
 import static com.google.firebase.appdistribution.impl.TestUtils.awaitTaskFailure;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyBoolean;
+import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoInteractions;
@@ -41,6 +44,8 @@
 import com.google.android.gms.tasks.Tasks;
 import com.google.firebase.FirebaseApp;
 import com.google.firebase.FirebaseOptions;
+import com.google.firebase.annotations.concurrent.Background;
+import com.google.firebase.annotations.concurrent.Blocking;
 import com.google.firebase.appdistribution.FirebaseAppDistributionException;
 import com.google.firebase.appdistribution.FirebaseAppDistributionException.Status;
 import com.google.firebase.appdistribution.impl.FirebaseAppDistributionServiceImplTest.TestActivity;
@@ -83,12 +88,13 @@ public class TesterSignInManagerTest {
   private TestActivity activity;
   private ShadowActivity shadowActivity;
   private ShadowPackageManager shadowPackageManager;
-  private ExecutorService blockingExecutor;
+  @Blocking private ExecutorService blockingExecutor = TestOnlyExecutors.blocking();
+  @Background private ExecutorService backgroundExecutor = TestOnlyExecutors.background();
+  private SignInStorage signInStorage;
 
   @Mock private Provider<FirebaseInstallationsApi> mockFirebaseInstallationsProvider;
   @Mock private FirebaseInstallationsApi mockFirebaseInstallations;
   @Mock private InstallationTokenResult mockInstallationTokenResult;
-  @Mock private SignInStorage mockSignInStorage;
   @Mock private FirebaseAppDistributionLifecycleNotifier mockLifecycleNotifier;
   @Mock private SignInResultActivity mockSignInResultActivity;
 
@@ -114,7 +120,8 @@ public void setUp() {
 
     when(mockInstallationTokenResult.getToken()).thenReturn(TEST_AUTH_TOKEN);
 
-    when(mockSignInStorage.getSignInStatus()).thenReturn(Tasks.forResult(false));
+    signInStorage =
+        spy(new SignInStorage(ApplicationProvider.getApplicationContext(), backgroundExecutor));
 
     shadowPackageManager =
         shadowOf(ApplicationProvider.getApplicationContext().getPackageManager());
@@ -137,21 +144,19 @@ public void setUp() {
     shadowActivity = shadowOf(activity);
     TestUtils.mockForegroundActivity(mockLifecycleNotifier, activity);
 
-    blockingExecutor = TestOnlyExecutors.blocking();
-
     testerSignInManager =
         new TesterSignInManager(
             firebaseApp,
             mockFirebaseInstallationsProvider,
-            mockSignInStorage,
+            signInStorage,
             mockLifecycleNotifier,
             blockingExecutor);
   }
 
   @Test
   public void signInTester_alreadySignedIn_doesNothing()
       throws FirebaseAppDistributionException, ExecutionException, InterruptedException {
-    when(mockSignInStorage.getSignInStatus()).thenReturn(Tasks.forResult(true));
+    TestUtils.awaitTask(signInStorage.setSignInStatus(true));
 
     Task signInTask = testerSignInManager.signInTester();
     awaitTask(signInTask);
@@ -181,7 +186,7 @@ public void signInTester_whenUnexpectedFailureInTask_failsWithUnknownError() {
 
     Task signInTask = testerSignInManager.signInTester();
 
-    awaitTaskFailure(signInTask, Status.UNKNOWN, "Unknown", unexpectedException);
+    awaitTaskFailure(signInTask, UNKNOWN, "Unknown", unexpectedException);
   }
 
   @Test
@@ -193,6 +198,7 @@ public void signInTester_whenChromeAvailable_opensCustomTab() throws Interrupted
     shadowPackageManager.addResolveInfoForIntent(customTabIntent, resolveInfo);
 
     testerSignInManager.signInTester();
+    awaitAsyncOperations(backgroundExecutor);
     awaitAsyncOperations(blockingExecutor);
 
     verify(mockFirebaseInstallations, times(1)).getId();
@@ -207,6 +213,7 @@ public void signInTester_whenChromeNotAvailable_opensBrowserIntent() throws Inte
     shadowPackageManager.addResolveInfoForIntent(browserIntent, resolveInfo);
 
     testerSignInManager.signInTester();
+    awaitAsyncOperations(backgroundExecutor);
     awaitAsyncOperations(blockingExecutor);
 
     verify(mockFirebaseInstallations, times(1)).getId();
@@ -219,6 +226,7 @@ public void signInTester_whenSignInCalledMultipleTimes_secondCallHasNoEffect()
     testerSignInManager.signInTester();
     testerSignInManager.signInTester();
 
+    awaitAsyncOperations(backgroundExecutor);
     awaitAsyncOperations(blockingExecutor);
 
     verify(mockFirebaseInstallationsProvider, times(1)).get();
@@ -228,18 +236,37 @@ public void signInTester_whenSignInCalledMultipleTimes_secondCallHasNoEffect()
   public void signInTester_whenReturnFromSignIn_taskSucceeds()
       throws InterruptedException, FirebaseAppDistributionException, ExecutionException {
     Task signInTask = testerSignInManager.signInTester();
+    awaitAsyncOperations(backgroundExecutor);
+    awaitAsyncOperations(blockingExecutor);
 
     // Simulate re-entering app after successful sign in, via SignInResultActivity
     testerSignInManager.onActivityCreated(mockSignInResultActivity);
     awaitTask(signInTask);
 
     assertTrue(signInTask.isSuccessful());
-    verify(mockSignInStorage).setSignInStatus(true);
+    assertThat(signInStorage.getSignInStatusBlocking()).isTrue();
+  }
+
+  @Test
+  public void signInTester_whenStorageFailsToRecordSignInStatus_taskFails()
+      throws InterruptedException {
+    Exception expectedException = new RuntimeException("Error");
+    when(signInStorage.setSignInStatus(anyBoolean()))
+        .thenReturn(Tasks.forException(expectedException));
+    Task signInTask = testerSignInManager.signInTester();
+    awaitAsyncOperations(backgroundExecutor);
+    awaitAsyncOperations(blockingExecutor);
+
+    // Simulate re-entering app after successful sign in, via SignInResultActivity
+    testerSignInManager.onActivityCreated(mockSignInResultActivity);
+
+    awaitTaskFailure(signInTask, UNKNOWN, "Error storing tester sign in state", expectedException);
   }
 
   @Test
   public void signInTester_whenAppReenteredDuringSignIn_taskFails() throws InterruptedException {
     Task signInTask = testerSignInManager.signInTester();
+    awaitAsyncOperations(backgroundExecutor);
     awaitAsyncOperations(blockingExecutor);
 
     // Simulate re-entering app before completing sign in