Skip to content

Commit f6608c2

Browse files
lfkellogglfkellogg
committed
Simplify sign in state management
1 parent 973f40e commit f6608c2

File tree

3 files changed

+79
-34
lines changed

3 files changed

+79
-34
lines changed

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

Lines changed: 29 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -24,48 +24,58 @@
2424

2525
/** Class that handles storage for App Distribution SignIn persistence. */
2626
class SignInStorage {
27-
2827
private static final String SIGNIN_PREFERENCES_NAME = "FirebaseAppDistributionSignInStorage";
2928
private static final String SIGNIN_TAG = "firebase_app_distribution_signin";
3029

3130
private final Context applicationContext;
3231
@Background private final Executor backgroundExecutor;
32+
private SharedPreferences sharedPreferences;
33+
34+
private interface SharedPreferencesFunction<T> {
35+
T apply(SharedPreferences sharedPreferences);
36+
}
3337

3438
SignInStorage(Context applicationContext, @Background Executor backgroundExecutor) {
3539
this.applicationContext = applicationContext;
3640
this.backgroundExecutor = backgroundExecutor;
3741
}
3842

3943
Task<Void> setSignInStatus(boolean testerSignedIn) {
40-
return getSharedPreferences()
41-
.onSuccessTask(
42-
backgroundExecutor,
43-
sharedPreferences -> {
44-
sharedPreferences.edit().putBoolean(SIGNIN_TAG, testerSignedIn).apply();
45-
return null;
46-
});
44+
return applyToSharedPreferences(
45+
sharedPreferences -> {
46+
sharedPreferences.edit().putBoolean(SIGNIN_TAG, testerSignedIn).apply();
47+
return null;
48+
});
4749
}
4850

4951
Task<Boolean> getSignInStatus() {
50-
return getSharedPreferences()
51-
.onSuccessTask(
52-
backgroundExecutor,
53-
sharedPreferences -> Tasks.forResult(sharedPreferences.getBoolean(SIGNIN_TAG, false)));
52+
return applyToSharedPreferences(
53+
sharedPreferences -> sharedPreferences.getBoolean(SIGNIN_TAG, false));
5454
}
5555

5656
boolean getSignInStatusBlocking() {
5757
return getSharedPreferencesBlocking().getBoolean(SIGNIN_TAG, false);
5858
}
5959

60-
private Task<SharedPreferences> getSharedPreferences() {
61-
TaskCompletionSource<SharedPreferences> taskCompletionSource = new TaskCompletionSource<>();
62-
backgroundExecutor.execute(
63-
() -> taskCompletionSource.setResult(getSharedPreferencesBlocking()));
64-
return taskCompletionSource.getTask();
65-
}
66-
6760
private SharedPreferences getSharedPreferencesBlocking() {
6861
// This may construct a new SharedPreferences object, which requires storage I/O
6962
return applicationContext.getSharedPreferences(SIGNIN_PREFERENCES_NAME, Context.MODE_PRIVATE);
7063
}
64+
65+
private <T> Task<T> applyToSharedPreferences(SharedPreferencesFunction<T> func) {
66+
// Check nullness of sharedPreferences directly even though multiple threads could be calling
67+
// this function at once. This isn't a problem because: 1) once it is set it will never be set,
68+
// back to null, and 2) even if it is initialized twice on different threads the second call
69+
// will get the exact same instance.
70+
if (sharedPreferences != null) {
71+
return Tasks.forResult(func.apply(sharedPreferences));
72+
}
73+
TaskCompletionSource<T> taskCompletionSource = new TaskCompletionSource<>();
74+
backgroundExecutor.execute(
75+
() -> {
76+
sharedPreferences = getSharedPreferencesBlocking();
77+
taskCompletionSource.setResult(func.apply(sharedPreferences));
78+
});
79+
return taskCompletionSource.getTask();
80+
}
7181
}

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

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
package com.google.firebase.appdistribution.impl;
1616

1717
import static com.google.firebase.appdistribution.FirebaseAppDistributionException.Status.AUTHENTICATION_CANCELED;
18+
import static com.google.firebase.appdistribution.FirebaseAppDistributionException.Status.AUTHENTICATION_FAILURE;
19+
import static com.google.firebase.appdistribution.FirebaseAppDistributionException.Status.UNKNOWN;
1820
import static com.google.firebase.appdistribution.impl.TaskUtils.safeSetTaskException;
1921
import static com.google.firebase.appdistribution.impl.TaskUtils.safeSetTaskResult;
2022

@@ -34,7 +36,6 @@
3436
import com.google.android.gms.tasks.Tasks;
3537
import com.google.firebase.FirebaseApp;
3638
import com.google.firebase.annotations.concurrent.Blocking;
37-
import com.google.firebase.annotations.concurrent.Lightweight;
3839
import com.google.firebase.appdistribution.FirebaseAppDistributionException;
3940
import com.google.firebase.appdistribution.FirebaseAppDistributionException.Status;
4041
import com.google.firebase.inject.Provider;
@@ -84,7 +85,7 @@ class TesterSignInManager {
8485
@NonNull Provider<FirebaseInstallationsApi> firebaseInstallationsApiProvider,
8586
@NonNull final SignInStorage signInStorage,
8687
@NonNull FirebaseAppDistributionLifecycleNotifier lifecycleNotifier,
87-
@Lightweight Executor blockingExecutor) {
88+
@Blocking Executor blockingExecutor) {
8889
this.firebaseApp = firebaseApp;
8990
this.firebaseInstallationsApiProvider = firebaseInstallationsApiProvider;
9091
this.signInStorage = signInStorage;
@@ -101,8 +102,15 @@ void onActivityCreated(Activity activity) {
101102
// result of the signIn Task in the onActivityCreated callback
102103
if (activity instanceof SignInResultActivity) {
103104
LogWrapper.v(TAG, "Sign in completed");
104-
this.setSuccessfulSignInResult();
105-
this.signInStorage.setSignInStatus(true);
105+
this.signInStorage
106+
.setSignInStatus(true)
107+
.addOnSuccessListener(blockingExecutor, unused -> this.setSuccessfulSignInResult())
108+
.addOnFailureListener(
109+
blockingExecutor,
110+
e ->
111+
this.setSignInTaskCompletionError(
112+
new FirebaseAppDistributionException(
113+
"Error storing tester sign in state", UNKNOWN, e)));
106114
}
107115
}
108116

@@ -160,14 +168,14 @@ private Task<Void> doSignInTester() {
160168
.getId()
161169
.addOnFailureListener(
162170
blockingExecutor,
163-
handleTaskFailure(ErrorMessages.AUTHENTICATION_ERROR, Status.AUTHENTICATION_FAILURE))
171+
handleTaskFailure(ErrorMessages.AUTHENTICATION_ERROR, AUTHENTICATION_FAILURE))
164172
.addOnSuccessListener(
165173
blockingExecutor,
166174
fid ->
167175
getForegroundActivityAndOpenSignInFlow(fid)
168176
.addOnFailureListener(
169177
blockingExecutor,
170-
handleTaskFailure(ErrorMessages.UNKNOWN_ERROR, Status.UNKNOWN)));
178+
handleTaskFailure(ErrorMessages.UNKNOWN_ERROR, UNKNOWN)));
171179

172180
return signInTaskCompletionSource.getTask();
173181
}

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

Lines changed: 36 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,15 @@
1616

1717
import static com.google.common.truth.Truth.assertThat;
1818
import static com.google.firebase.appdistribution.FirebaseAppDistributionException.Status.AUTHENTICATION_CANCELED;
19+
import static com.google.firebase.appdistribution.FirebaseAppDistributionException.Status.UNKNOWN;
1920
import static com.google.firebase.appdistribution.impl.TestUtils.assertTaskFailure;
2021
import static com.google.firebase.appdistribution.impl.TestUtils.awaitAsyncOperations;
2122
import static com.google.firebase.appdistribution.impl.TestUtils.awaitTask;
2223
import static com.google.firebase.appdistribution.impl.TestUtils.awaitTaskFailure;
2324
import static org.junit.Assert.assertTrue;
2425
import static org.mockito.ArgumentMatchers.any;
26+
import static org.mockito.ArgumentMatchers.anyBoolean;
27+
import static org.mockito.Mockito.spy;
2528
import static org.mockito.Mockito.times;
2629
import static org.mockito.Mockito.verify;
2730
import static org.mockito.Mockito.verifyNoInteractions;
@@ -41,6 +44,8 @@
4144
import com.google.android.gms.tasks.Tasks;
4245
import com.google.firebase.FirebaseApp;
4346
import com.google.firebase.FirebaseOptions;
47+
import com.google.firebase.annotations.concurrent.Background;
48+
import com.google.firebase.annotations.concurrent.Blocking;
4449
import com.google.firebase.appdistribution.FirebaseAppDistributionException;
4550
import com.google.firebase.appdistribution.FirebaseAppDistributionException.Status;
4651
import com.google.firebase.appdistribution.impl.FirebaseAppDistributionServiceImplTest.TestActivity;
@@ -83,12 +88,13 @@ public class TesterSignInManagerTest {
8388
private TestActivity activity;
8489
private ShadowActivity shadowActivity;
8590
private ShadowPackageManager shadowPackageManager;
86-
private ExecutorService blockingExecutor;
91+
@Blocking private ExecutorService blockingExecutor = TestOnlyExecutors.blocking();
92+
@Background private ExecutorService backgroundExecutor = TestOnlyExecutors.background();
93+
private SignInStorage signInStorage;
8794

8895
@Mock private Provider<FirebaseInstallationsApi> mockFirebaseInstallationsProvider;
8996
@Mock private FirebaseInstallationsApi mockFirebaseInstallations;
9097
@Mock private InstallationTokenResult mockInstallationTokenResult;
91-
@Mock private SignInStorage mockSignInStorage;
9298
@Mock private FirebaseAppDistributionLifecycleNotifier mockLifecycleNotifier;
9399
@Mock private SignInResultActivity mockSignInResultActivity;
94100

@@ -114,7 +120,8 @@ public void setUp() {
114120

115121
when(mockInstallationTokenResult.getToken()).thenReturn(TEST_AUTH_TOKEN);
116122

117-
when(mockSignInStorage.getSignInStatus()).thenReturn(Tasks.forResult(false));
123+
signInStorage =
124+
spy(new SignInStorage(ApplicationProvider.getApplicationContext(), backgroundExecutor));
118125

119126
shadowPackageManager =
120127
shadowOf(ApplicationProvider.getApplicationContext().getPackageManager());
@@ -137,21 +144,19 @@ public void setUp() {
137144
shadowActivity = shadowOf(activity);
138145
TestUtils.mockForegroundActivity(mockLifecycleNotifier, activity);
139146

140-
blockingExecutor = TestOnlyExecutors.blocking();
141-
142147
testerSignInManager =
143148
new TesterSignInManager(
144149
firebaseApp,
145150
mockFirebaseInstallationsProvider,
146-
mockSignInStorage,
151+
signInStorage,
147152
mockLifecycleNotifier,
148153
blockingExecutor);
149154
}
150155

151156
@Test
152157
public void signInTester_alreadySignedIn_doesNothing()
153158
throws FirebaseAppDistributionException, ExecutionException, InterruptedException {
154-
when(mockSignInStorage.getSignInStatus()).thenReturn(Tasks.forResult(true));
159+
TestUtils.awaitTask(signInStorage.setSignInStatus(true));
155160

156161
Task signInTask = testerSignInManager.signInTester();
157162
awaitTask(signInTask);
@@ -181,7 +186,7 @@ public void signInTester_whenUnexpectedFailureInTask_failsWithUnknownError() {
181186

182187
Task signInTask = testerSignInManager.signInTester();
183188

184-
awaitTaskFailure(signInTask, Status.UNKNOWN, "Unknown", unexpectedException);
189+
awaitTaskFailure(signInTask, UNKNOWN, "Unknown", unexpectedException);
185190
}
186191

187192
@Test
@@ -193,6 +198,7 @@ public void signInTester_whenChromeAvailable_opensCustomTab() throws Interrupted
193198
shadowPackageManager.addResolveInfoForIntent(customTabIntent, resolveInfo);
194199

195200
testerSignInManager.signInTester();
201+
awaitAsyncOperations(backgroundExecutor);
196202
awaitAsyncOperations(blockingExecutor);
197203

198204
verify(mockFirebaseInstallations, times(1)).getId();
@@ -207,6 +213,7 @@ public void signInTester_whenChromeNotAvailable_opensBrowserIntent() throws Inte
207213
shadowPackageManager.addResolveInfoForIntent(browserIntent, resolveInfo);
208214

209215
testerSignInManager.signInTester();
216+
awaitAsyncOperations(backgroundExecutor);
210217
awaitAsyncOperations(blockingExecutor);
211218

212219
verify(mockFirebaseInstallations, times(1)).getId();
@@ -219,6 +226,7 @@ public void signInTester_whenSignInCalledMultipleTimes_secondCallHasNoEffect()
219226
testerSignInManager.signInTester();
220227
testerSignInManager.signInTester();
221228

229+
awaitAsyncOperations(backgroundExecutor);
222230
awaitAsyncOperations(blockingExecutor);
223231

224232
verify(mockFirebaseInstallationsProvider, times(1)).get();
@@ -228,18 +236,37 @@ public void signInTester_whenSignInCalledMultipleTimes_secondCallHasNoEffect()
228236
public void signInTester_whenReturnFromSignIn_taskSucceeds()
229237
throws InterruptedException, FirebaseAppDistributionException, ExecutionException {
230238
Task signInTask = testerSignInManager.signInTester();
239+
awaitAsyncOperations(backgroundExecutor);
240+
awaitAsyncOperations(blockingExecutor);
231241

232242
// Simulate re-entering app after successful sign in, via SignInResultActivity
233243
testerSignInManager.onActivityCreated(mockSignInResultActivity);
234244
awaitTask(signInTask);
235245

236246
assertTrue(signInTask.isSuccessful());
237-
verify(mockSignInStorage).setSignInStatus(true);
247+
assertThat(signInStorage.getSignInStatusBlocking()).isTrue();
248+
}
249+
250+
@Test
251+
public void signInTester_whenStorageFailsToRecordSignInStatus_taskFails()
252+
throws InterruptedException {
253+
Exception expectedException = new RuntimeException("Error");
254+
when(signInStorage.setSignInStatus(anyBoolean()))
255+
.thenReturn(Tasks.forException(expectedException));
256+
Task signInTask = testerSignInManager.signInTester();
257+
awaitAsyncOperations(backgroundExecutor);
258+
awaitAsyncOperations(blockingExecutor);
259+
260+
// Simulate re-entering app after successful sign in, via SignInResultActivity
261+
testerSignInManager.onActivityCreated(mockSignInResultActivity);
262+
263+
awaitTaskFailure(signInTask, UNKNOWN, "Error storing tester sign in state", expectedException);
238264
}
239265

240266
@Test
241267
public void signInTester_whenAppReenteredDuringSignIn_taskFails() throws InterruptedException {
242268
Task signInTask = testerSignInManager.signInTester();
269+
awaitAsyncOperations(backgroundExecutor);
243270
awaitAsyncOperations(blockingExecutor);
244271

245272
// Simulate re-entering app before completing sign in

0 commit comments

Comments
 (0)