Skip to content

Commit de5f678

Browse files
lfkelloggkaibolay
lfkellogg
authored and
kaibolay
committed
Make isTesterSignedIn more reliable (#4493)
1 parent bdef386 commit de5f678

File tree

3 files changed

+74
-40
lines changed

3 files changed

+74
-40
lines changed

‎firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionImpl.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -244,8 +244,9 @@ public Task<Void> signInTester() {
244244

245245
@Override
246246
public void signOutTester() {
247-
cachedNewRelease.set(null);
247+
// Set sign in status first so isTesterSigned returns the correct state as soon as possible
248248
signInStorage.setSignInStatus(false);
249+
cachedNewRelease.set(null);
249250
}
250251

251252
@Override

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

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
import android.content.Context;
1818
import android.content.SharedPreferences;
19+
import androidx.annotation.VisibleForTesting;
1920
import com.google.android.gms.tasks.Task;
2021
import com.google.android.gms.tasks.TaskCompletionSource;
2122
import com.google.android.gms.tasks.Tasks;
@@ -24,8 +25,10 @@
2425

2526
/** Class that handles storage for App Distribution SignIn persistence. */
2627
class SignInStorage {
27-
private static final String SIGNIN_PREFERENCES_NAME = "FirebaseAppDistributionSignInStorage";
28-
private static final String SIGNIN_TAG = "firebase_app_distribution_signin";
28+
@VisibleForTesting
29+
static final String SIGNIN_PREFERENCES_NAME = "FirebaseAppDistributionSignInStorage";
30+
31+
@VisibleForTesting static final String SIGNIN_TAG = "firebase_app_distribution_signin";
2932

3033
private final Context applicationContext;
3134
@Background private final Executor backgroundExecutor;
@@ -54,12 +57,14 @@ Task<Boolean> getSignInStatus() {
5457
}
5558

5659
boolean getSignInStatusBlocking() {
57-
return getSharedPreferencesBlocking().getBoolean(SIGNIN_TAG, false);
60+
return getAndCacheSharedPreferences().getBoolean(SIGNIN_TAG, false);
5861
}
5962

60-
private SharedPreferences getSharedPreferencesBlocking() {
63+
private SharedPreferences getAndCacheSharedPreferences() {
6164
// This may construct a new SharedPreferences object, which requires storage I/O
62-
return applicationContext.getSharedPreferences(SIGNIN_PREFERENCES_NAME, Context.MODE_PRIVATE);
65+
sharedPreferences =
66+
applicationContext.getSharedPreferences(SIGNIN_PREFERENCES_NAME, Context.MODE_PRIVATE);
67+
return sharedPreferences;
6368
}
6469

6570
private <T> Task<T> applyToSharedPreferences(SharedPreferencesFunction<T> func) {
@@ -72,10 +77,7 @@ private <T> Task<T> applyToSharedPreferences(SharedPreferencesFunction<T> func)
7277
}
7378
TaskCompletionSource<T> taskCompletionSource = new TaskCompletionSource<>();
7479
backgroundExecutor.execute(
75-
() -> {
76-
sharedPreferences = getSharedPreferencesBlocking();
77-
taskCompletionSource.setResult(func.apply(sharedPreferences));
78-
});
80+
() -> taskCompletionSource.setResult(func.apply(getAndCacheSharedPreferences())));
7981
return taskCompletionSource.getTask();
8082
}
8183
}

‎firebase-appdistribution/src/test/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionServiceImplTest.java

Lines changed: 61 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
package com.google.firebase.appdistribution.impl;
1616

17+
import static android.content.Context.MODE_PRIVATE;
1718
import static android.os.Looper.getMainLooper;
1819
import static com.google.common.truth.Truth.assertThat;
1920
import static com.google.firebase.appdistribution.FirebaseAppDistributionException.Status.AUTHENTICATION_CANCELED;
@@ -43,7 +44,6 @@
4344
import static org.junit.Assert.assertNull;
4445
import static org.junit.Assert.assertTrue;
4546
import static org.mockito.ArgumentMatchers.any;
46-
import static org.mockito.ArgumentMatchers.anyBoolean;
4747
import static org.mockito.Mockito.doReturn;
4848
import static org.mockito.Mockito.never;
4949
import static org.mockito.Mockito.spy;
@@ -58,6 +58,7 @@
5858
import android.app.Dialog;
5959
import android.content.DialogInterface;
6060
import android.content.Intent;
61+
import android.content.SharedPreferences;
6162
import android.content.pm.ApplicationInfo;
6263
import android.content.pm.PackageInfo;
6364
import android.net.Uri;
@@ -70,6 +71,7 @@
7071
import com.google.android.gms.tasks.Tasks;
7172
import com.google.firebase.FirebaseApp;
7273
import com.google.firebase.FirebaseOptions;
74+
import com.google.firebase.annotations.concurrent.Background;
7375
import com.google.firebase.annotations.concurrent.Blocking;
7476
import com.google.firebase.annotations.concurrent.Lightweight;
7577
import com.google.firebase.appdistribution.AppDistributionRelease;
@@ -143,18 +145,18 @@ public class FirebaseAppDistributionServiceImplTest {
143145

144146
@Lightweight private final ExecutorService lightweightExecutor = TestOnlyExecutors.lite();
145147
@Blocking private final ExecutorService blockingExecutor = TestOnlyExecutors.blocking();
148+
@Background private final ExecutorService backgroundExecutor = TestOnlyExecutors.background();
146149

147150
private FirebaseAppDistributionImpl firebaseAppDistribution;
148151
private ActivityController<TestActivity> activityController;
149152
private TestActivity activity;
150-
private FirebaseApp firebaseApp;
151153

152154
@Mock private InstallationTokenResult mockInstallationTokenResult;
153155
@Mock private TesterSignInManager mockTesterSignInManager;
154156
@Mock private NewReleaseFetcher mockNewReleaseFetcher;
155157
@Mock private ApkUpdater mockApkUpdater;
156158
@Mock private AabUpdater mockAabUpdater;
157-
@Mock private SignInStorage mockSignInStorage;
159+
@Mock private SignInStorage signInStorage;
158160
@Mock private FirebaseAppDistributionLifecycleNotifier mockLifecycleNotifier;
159161
@Mock private ReleaseIdentifier mockReleaseIdentifier;
160162
@Mock private ScreenshotTaker mockScreenshotTaker;
@@ -168,7 +170,10 @@ public void setup() throws FirebaseAppDistributionException {
168170

169171
FirebaseApp.clearInstancesForTest();
170172

171-
firebaseApp =
173+
signInStorage =
174+
spy(new SignInStorage(ApplicationProvider.getApplicationContext(), backgroundExecutor));
175+
176+
FirebaseApp firebaseApp =
172177
FirebaseApp.initializeApp(
173178
ApplicationProvider.getApplicationContext(),
174179
new FirebaseOptions.Builder()
@@ -185,16 +190,15 @@ public void setup() throws FirebaseAppDistributionException {
185190
mockNewReleaseFetcher,
186191
mockApkUpdater,
187192
mockAabUpdater,
188-
mockSignInStorage,
193+
signInStorage,
189194
mockLifecycleNotifier,
190195
mockReleaseIdentifier,
191196
mockScreenshotTaker,
192197
lightweightExecutor,
193198
blockingExecutor));
194199

195200
when(mockTesterSignInManager.signInTester()).thenReturn(Tasks.forResult(null));
196-
when(mockSignInStorage.getSignInStatus()).thenReturn(Tasks.forResult(true));
197-
when(mockSignInStorage.setSignInStatus(anyBoolean())).thenReturn(Tasks.forResult(null));
201+
setSignInStatusSharedPreference(true);
198202

199203
when(mockInstallationTokenResult.getToken()).thenReturn(TEST_AUTH_TOKEN);
200204

@@ -244,7 +248,7 @@ public void checkForNewRelease_whenCheckForNewReleaseFails_throwsError() {
244248

245249
@Test
246250
public void checkForNewRelease_testerIsNotSignedIn_taskFails() {
247-
when(mockSignInStorage.getSignInStatus()).thenReturn(Tasks.forResult(false));
251+
setSignInStatusSharedPreference(false);
248252

249253
Task<AppDistributionRelease> task = firebaseAppDistribution.checkForNewRelease();
250254

@@ -277,12 +281,12 @@ public void checkForNewRelease_authenticationFailure_signOutTester() throws Inte
277281

278282
awaitTaskFailure(task, AUTHENTICATION_FAILURE, "Test");
279283
awaitTermination(lightweightExecutor);
280-
verify(mockSignInStorage, times(1)).setSignInStatus(false);
284+
verify(signInStorage, times(1)).setSignInStatus(false);
281285
}
282286

283287
@Test
284288
public void updateApp_whenNotSignedIn_throwsError() {
285-
when(mockSignInStorage.getSignInStatus()).thenReturn(Tasks.forResult(false));
289+
setSignInStatusSharedPreference(false);
286290

287291
UpdateTask updateTask = firebaseAppDistribution.updateApp();
288292

@@ -372,7 +376,7 @@ public void updateIfNewReleaseAvailable_whenActivityBackgrounded_updateDialogNot
372376
@Test
373377
public void updateIfNewReleaseAvailable_whenSignInCancelled_checkForUpdateNotCalled()
374378
throws InterruptedException {
375-
when(mockSignInStorage.getSignInStatus()).thenReturn(Tasks.forResult(false));
379+
setSignInStatusSharedPreference(false);
376380
when(mockTesterSignInManager.signInTester())
377381
.thenReturn(
378382
Tasks.forException(
@@ -393,7 +397,7 @@ public void updateIfNewReleaseAvailable_whenSignInCancelled_checkForUpdateNotCal
393397
@Test
394398
public void updateIfNewReleaseAvailable_whenSignInFailed_checkForUpdateNotCalled()
395399
throws InterruptedException {
396-
when(mockSignInStorage.getSignInStatus()).thenReturn(Tasks.forResult(false));
400+
setSignInStatusSharedPreference(false);
397401
when(mockTesterSignInManager.signInTester())
398402
.thenReturn(
399403
Tasks.forException(
@@ -469,7 +473,6 @@ public void updateIfNewReleaseAvailable_whenCheckForUpdateFails_updateAppNotCall
469473
public void updateIfNewReleaseAvailable_whenTesterIsSignedIn_doesNotOpenDialog()
470474
throws InterruptedException, FirebaseAppDistributionException, ExecutionException {
471475
when(mockNewReleaseFetcher.checkForNewRelease()).thenReturn(Tasks.forResult(null));
472-
when(mockSignInStorage.getSignInStatus()).thenReturn(Tasks.forResult(true));
473476

474477
UpdateTask task = firebaseAppDistribution.updateIfNewReleaseAvailable();
475478
awaitTask(task);
@@ -479,8 +482,8 @@ public void updateIfNewReleaseAvailable_whenTesterIsSignedIn_doesNotOpenDialog()
479482

480483
@Test
481484
public void signInTester_whenDialogDismissed_taskFails() throws InterruptedException {
482-
when(mockSignInStorage.getSignInStatus()).thenReturn(Tasks.forResult(false));
483-
Task updateTask = firebaseAppDistribution.updateIfNewReleaseAvailable();
485+
setSignInStatusSharedPreference(false);
486+
UpdateTask updateTask = firebaseAppDistribution.updateIfNewReleaseAvailable();
484487
awaitAsyncOperations(lightweightExecutor);
485488

486489
AlertDialog dialog = assertAlertDialogShown();
@@ -492,8 +495,8 @@ public void signInTester_whenDialogDismissed_taskFails() throws InterruptedExcep
492495
@Test
493496
public void updateIfNewReleaseAvailable_whenSignInDialogCanceled_taskFails()
494497
throws InterruptedException {
495-
when(mockSignInStorage.getSignInStatus()).thenReturn(Tasks.forResult(false));
496-
Task signInTask = firebaseAppDistribution.updateIfNewReleaseAvailable();
498+
setSignInStatusSharedPreference(false);
499+
UpdateTask signInTask = firebaseAppDistribution.updateIfNewReleaseAvailable();
497500
awaitAsyncOperations(lightweightExecutor);
498501

499502
AlertDialog dialog = assertAlertDialogShown();
@@ -514,7 +517,7 @@ private AlertDialog assertAlertDialogShown() {
514517
public void signOutTester_setsSignInStatusFalse() {
515518
firebaseAppDistribution.signOutTester();
516519

517-
verify(mockSignInStorage).setSignInStatus(false);
520+
assertThat(signInStorage.getSignInStatusBlocking()).isFalse();
518521
}
519522

520523
@Test
@@ -532,6 +535,30 @@ public void signOutTester_unsetsCachedNewRelease()
532535
assertThat(cachedNewReleaseTask.getResult()).isNull();
533536
}
534537

538+
@Test
539+
public void isTesterSignedIn_returnsTrueWhenSharePreferenceIsTrue() {
540+
setSignInStatusSharedPreference(true);
541+
assertThat(firebaseAppDistribution.isTesterSignedIn()).isTrue();
542+
}
543+
544+
@Test
545+
public void isTesterSignedIn_returnsFalseWhenSharePreferenceIsFalse() {
546+
setSignInStatusSharedPreference(false);
547+
assertThat(firebaseAppDistribution.isTesterSignedIn()).isFalse();
548+
}
549+
550+
@Test
551+
public void isTesterSignedIn_whenStorageWarmedUp_returnsCorrectStatusImmediately() {
552+
// First check the sign in status, which has the side effect of warming up the SharedPreferences
553+
// instance. This simulates a typical flow, where a developer would check the status before
554+
// trying to sign out the tester.
555+
firebaseAppDistribution.isTesterSignedIn();
556+
557+
firebaseAppDistribution.signOutTester();
558+
559+
assertThat(firebaseAppDistribution.isTesterSignedIn()).isFalse();
560+
}
561+
535562
@Test
536563
public void updateIfNewReleaseAvailable_receiveProgressUpdateFromUpdateApp()
537564
throws InterruptedException {
@@ -565,21 +592,21 @@ public void updateIfNewReleaseAvailable_receiveProgressUpdateFromUpdateApp()
565592
@Test
566593
public void updateIfNewReleaseAvailable_fromABackgroundThread_showsSignInDialog()
567594
throws InterruptedException, ExecutionException {
568-
when(mockSignInStorage.getSignInStatus()).thenReturn(Tasks.forResult(false));
595+
setSignInStatusSharedPreference(false);
569596

570597
ExecutorService executorService = Executors.newSingleThreadExecutor();
571598
Future<UpdateTask> future =
572599
executorService.submit(() -> firebaseAppDistribution.updateIfNewReleaseAvailable());
573600
awaitAsyncOperations(executorService);
574601

575602
assertAlertDialogShown();
576-
assertFalse(((UpdateTask) future.get()).isComplete());
603+
assertFalse((future.get()).isComplete());
577604
}
578605

579606
@Test
580607
public void updateIfNewReleaseAvailable_whenScreenRotates_signInConfirmationDialogReappears()
581608
throws InterruptedException {
582-
when(mockSignInStorage.getSignInStatus()).thenReturn(Tasks.forResult(false));
609+
setSignInStatusSharedPreference(false);
583610
when(activity.isChangingConfigurations()).thenReturn(true);
584611

585612
UpdateTask updateTask = firebaseAppDistribution.updateIfNewReleaseAvailable();
@@ -597,8 +624,8 @@ public void updateIfNewReleaseAvailable_whenScreenRotates_signInConfirmationDial
597624
@Test
598625
public void updateIfNewReleaseAvailable_whenScreenRotates_updateDialogReappears()
599626
throws InterruptedException {
600-
AppDistributionReleaseInternal newRelease = TEST_RELEASE_NEWER_AAB_INTERNAL;
601-
when(mockNewReleaseFetcher.checkForNewRelease()).thenReturn(Tasks.forResult(newRelease));
627+
when(mockNewReleaseFetcher.checkForNewRelease())
628+
.thenReturn(Tasks.forResult(TEST_RELEASE_NEWER_AAB_INTERNAL));
602629
when(activity.isChangingConfigurations()).thenReturn(true);
603630

604631
UpdateTask updateTask = firebaseAppDistribution.updateIfNewReleaseAvailable();
@@ -617,7 +644,7 @@ public void updateIfNewReleaseAvailable_whenScreenRotates_updateDialogReappears(
617644
updateIfNewReleaseAvailable_whenSignInDialogShowingAndNewActivityStarts_signInTaskCancelled()
618645
throws InterruptedException {
619646
TestActivity testActivity2 = new TestActivity();
620-
when(mockSignInStorage.getSignInStatus()).thenReturn(Tasks.forResult(false));
647+
setSignInStatusSharedPreference(false);
621648

622649
UpdateTask updateTask = firebaseAppDistribution.updateIfNewReleaseAvailable();
623650
awaitAsyncOperations(lightweightExecutor);
@@ -635,8 +662,8 @@ public void updateIfNewReleaseAvailable_whenScreenRotates_updateDialogReappears(
635662
updateIfNewReleaseAvailable_whenUpdateDialogShowingAndNewActivityStarts_updateTaskCancelled()
636663
throws InterruptedException {
637664
TestActivity testActivity2 = new TestActivity();
638-
AppDistributionReleaseInternal newRelease = TEST_RELEASE_NEWER_AAB_INTERNAL;
639-
when(mockNewReleaseFetcher.checkForNewRelease()).thenReturn(Tasks.forResult(newRelease));
665+
when(mockNewReleaseFetcher.checkForNewRelease())
666+
.thenReturn(Tasks.forResult(TEST_RELEASE_NEWER_AAB_INTERNAL));
640667

641668
UpdateTask updateTask = firebaseAppDistribution.updateIfNewReleaseAvailable();
642669
awaitAsyncOperations(lightweightExecutor);
@@ -652,7 +679,6 @@ public void updateIfNewReleaseAvailable_whenScreenRotates_updateDialogReappears(
652679
@Test
653680
public void updateAppTask_whenNoReleaseAvailable_throwsError() {
654681
firebaseAppDistribution.getCachedNewRelease().set(null);
655-
when(mockSignInStorage.getSignInStatus()).thenReturn(Tasks.forResult(true));
656682

657683
UpdateTask updateTask = firebaseAppDistribution.updateApp();
658684

@@ -666,7 +692,6 @@ public void updateApp_withAabReleaseAvailable_returnsSameAabTask()
666692
firebaseAppDistribution.getCachedNewRelease().set(release);
667693
UpdateTaskImpl updateTaskToReturn = new UpdateTaskImpl();
668694
doReturn(updateTaskToReturn).when(mockAabUpdater).updateAab(release);
669-
when(mockSignInStorage.getSignInStatus()).thenReturn(Tasks.forResult(true));
670695

671696
UpdateTask updateTask = firebaseAppDistribution.updateApp();
672697
assertFalse(updateTask.isComplete());
@@ -682,7 +707,6 @@ public void updateApp_withAabReleaseAvailable_returnsSameAabTask()
682707
@Test
683708
public void updateApp_withApkReleaseAvailable_returnsSameApkTask()
684709
throws InterruptedException, FirebaseAppDistributionException, ExecutionException {
685-
when(mockSignInStorage.getSignInStatus()).thenReturn(Tasks.forResult(true));
686710
AppDistributionReleaseInternal release = TEST_RELEASE_NEWER_APK_INTERNAL;
687711
firebaseAppDistribution.getCachedNewRelease().set(release);
688712
UpdateTaskImpl updateTaskToReturn = new UpdateTaskImpl();
@@ -829,9 +853,16 @@ private static void assertLoggedError(String partialMessage, Throwable e) {
829853
log ->
830854
log.type == Log.ERROR
831855
&& log.msg.contains(partialMessage)
832-
&& (e != null ? log.throwable == e : true);
856+
&& (e == null || log.throwable == e);
833857
List<ShadowLog.LogItem> matchingLogs =
834858
ShadowLog.getLogs().stream().filter(predicate).collect(toList());
835859
assertThat(matchingLogs).hasSize(1);
836860
}
861+
862+
private void setSignInStatusSharedPreference(boolean testerSignedIn) {
863+
SharedPreferences sharedPreferences =
864+
ApplicationProvider.getApplicationContext()
865+
.getSharedPreferences(SignInStorage.SIGNIN_PREFERENCES_NAME, MODE_PRIVATE);
866+
sharedPreferences.edit().putBoolean(SignInStorage.SIGNIN_TAG, testerSignedIn).commit();
867+
}
837868
}

0 commit comments

Comments
 (0)