Skip to content

Commit 0ce007a

Browse files
authored
Revert caching fid & disk read on caller thread. (#1767)
* Revert "Completing getId call with the disk reads on the caller thread." This reverts commit f0ff299 * Revert "Addressing Fred's comments" This reverts commit bc6ec5c
1 parent 04a9e0c commit 0ce007a

File tree

2 files changed

+68
-29
lines changed

2 files changed

+68
-29
lines changed

firebase-installations/src/main/java/com/google/firebase/installations/FirebaseInstallations.java

Lines changed: 25 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -66,10 +66,6 @@ public class FirebaseInstallations implements FirebaseInstallationsApi {
6666
private final Object lock = new Object();
6767
private final ExecutorService backgroundExecutor;
6868
private final ExecutorService networkExecutor;
69-
/* FID of this Firebase Installations instance. Cached after successfully registering and
70-
persisting the FID locally. NOTE: cachedFid resets if FID is deleted.*/
71-
@GuardedBy("this")
72-
private String cachedFid;
7369

7470
@GuardedBy("lock")
7571
private final List<StateListener> listeners = new ArrayList<>();
@@ -225,7 +221,9 @@ String getName() {
225221
@Override
226222
public Task<String> getId() {
227223
preConditionChecks();
228-
return Tasks.forResult(doGetId());
224+
Task<String> task = addGetIdListener();
225+
backgroundExecutor.execute(this::doGetId);
226+
return task;
229227
}
230228

231229
/**
@@ -241,7 +239,11 @@ public Task<String> getId() {
241239
public Task<InstallationTokenResult> getToken(boolean forceRefresh) {
242240
preConditionChecks();
243241
Task<InstallationTokenResult> task = addGetAuthTokenListener();
244-
backgroundExecutor.execute(() -> doGetAuthToken(forceRefresh));
242+
if (forceRefresh) {
243+
backgroundExecutor.execute(this::doGetAuthTokenForceRefresh);
244+
} else {
245+
backgroundExecutor.execute(this::doGetAuthTokenWithoutForceRefresh);
246+
}
245247
return task;
246248
}
247249

@@ -256,6 +258,15 @@ public Task<Void> delete() {
256258
return Tasks.call(backgroundExecutor, this::deleteFirebaseInstallationId);
257259
}
258260

261+
private Task<String> addGetIdListener() {
262+
TaskCompletionSource<String> taskCompletionSource = new TaskCompletionSource<>();
263+
StateListener l = new GetIdListener(taskCompletionSource);
264+
synchronized (lock) {
265+
listeners.add(l);
266+
}
267+
return taskCompletionSource.getTask();
268+
}
269+
259270
private Task<InstallationTokenResult> addGetAuthTokenListener() {
260271
TaskCompletionSource<InstallationTokenResult> taskCompletionSource =
261272
new TaskCompletionSource<>();
@@ -292,25 +303,16 @@ private void triggerOnException(PersistedInstallationEntry prefs, Exception exce
292303
}
293304
}
294305

295-
private synchronized void updateCacheFid(String cachedFid) {
296-
this.cachedFid = cachedFid;
306+
private final void doGetId() {
307+
doRegistrationInternal(false);
297308
}
298309

299-
private synchronized String getCacheFid() {
300-
return cachedFid;
310+
private final void doGetAuthTokenWithoutForceRefresh() {
311+
doRegistrationInternal(false);
301312
}
302313

303-
private String doGetId() {
304-
305-
String fid = getCacheFid();
306-
if (fid != null) {
307-
return fid;
308-
}
309-
PersistedInstallationEntry prefs = getPrefsWithGeneratedIdMultiProcessSafe();
310-
// Execute network calls (CreateInstallations) to the FIS Servers on a separate executor
311-
// i.e networkExecutor
312-
networkExecutor.execute(() -> doNetworkCallIfNecessary(false));
313-
return prefs.getFirebaseInstallationId();
314+
private final void doGetAuthTokenForceRefresh() {
315+
doRegistrationInternal(true);
314316
}
315317

316318
/**
@@ -322,7 +324,7 @@ private String doGetId() {
322324
* @param forceRefresh true if this is for a getAuthToken call and if the caller wants to fetch a
323325
* new auth token from the server even if an unexpired auth token exists on the client.
324326
*/
325-
private void doGetAuthToken(boolean forceRefresh) {
327+
private final void doRegistrationInternal(boolean forceRefresh) {
326328
PersistedInstallationEntry prefs = getPrefsWithGeneratedIdMultiProcessSafe();
327329

328330
// Since the caller wants to force an authtoken refresh remove the authtoken from the
@@ -359,11 +361,6 @@ private void doNetworkCallIfNecessary(boolean forceRefresh) {
359361
// Store the prefs to persist the result of the previous step.
360362
insertOrUpdatePrefs(prefs);
361363

362-
// Update cachedFID, if FID is successfully REGISTERED and persisted.
363-
if (prefs.isRegistered()) {
364-
updateCacheFid(prefs.getFirebaseInstallationId());
365-
}
366-
367364
// Let the caller know about the result.
368365
if (prefs.isErrored()) {
369366
triggerOnException(prefs, new FirebaseInstallationsException(Status.BAD_CONFIG));
@@ -523,7 +520,6 @@ private PersistedInstallationEntry fetchAuthTokenFromServer(
523520
case AUTH_ERROR:
524521
// The the server refused to generate a new auth token due to bad credentials, clear the
525522
// FID to force the generation of a new one.
526-
updateCacheFid(null);
527523
return prefs.withNoGeneratedFid();
528524
default:
529525
throw new FirebaseInstallationsException(
@@ -537,7 +533,6 @@ private PersistedInstallationEntry fetchAuthTokenFromServer(
537533
* storage.
538534
*/
539535
private Void deleteFirebaseInstallationId() throws FirebaseInstallationsException {
540-
updateCacheFid(null);
541536
PersistedInstallationEntry entry = getMultiProcessSafePrefs();
542537
if (entry.isRegistered()) {
543538
// Call the FIS servers to delete this Firebase Installation Id.
@@ -547,6 +542,7 @@ private Void deleteFirebaseInstallationId() throws FirebaseInstallationsExceptio
547542
/*projectID= */ getProjectIdentifier(),
548543
/*refreshToken= */ entry.getRefreshToken());
549544
}
545+
550546
insertOrUpdatePrefs(entry.withNoGeneratedFid());
551547
return null;
552548
}
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
// Copyright 2019 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package com.google.firebase.installations;
16+
17+
import com.google.android.gms.tasks.TaskCompletionSource;
18+
import com.google.firebase.installations.local.PersistedInstallationEntry;
19+
20+
class GetIdListener implements StateListener {
21+
final TaskCompletionSource<String> taskCompletionSource;
22+
23+
public GetIdListener(TaskCompletionSource<String> taskCompletionSource) {
24+
this.taskCompletionSource = taskCompletionSource;
25+
}
26+
27+
@Override
28+
public boolean onStateReached(PersistedInstallationEntry persistedInstallationEntry) {
29+
if (persistedInstallationEntry.isUnregistered()
30+
|| persistedInstallationEntry.isRegistered()
31+
|| persistedInstallationEntry.isErrored()) {
32+
taskCompletionSource.trySetResult(persistedInstallationEntry.getFirebaseInstallationId());
33+
return true;
34+
}
35+
return false;
36+
}
37+
38+
@Override
39+
public boolean onException(
40+
PersistedInstallationEntry persistedInstallationEntry, Exception exception) {
41+
return false;
42+
}
43+
}

0 commit comments

Comments
 (0)