From 6f6006e5b507327eeaa3c39d5f9bbd1932ef14c2 Mon Sep 17 00:00:00 2001 From: Ankita Date: Wed, 8 Jul 2020 17:37:05 -0700 Subject: [PATCH] Patch caching fid in memory on successful registration. (#1764) * Revert "Completing getId call with the disk reads on the caller thread. (#1570)" This reverts commit 1a36dd76 * Updating caching FID implementation due to the revert PR. * Addressing Rayo's comments. * Minor spacing. * Addressing Rayo's comments. --- .../installations/FirebaseInstallations.java | 52 ++++++++++++------- .../firebase/installations/GetIdListener.java | 6 +++ 2 files changed, 38 insertions(+), 20 deletions(-) diff --git a/firebase-installations/src/main/java/com/google/firebase/installations/FirebaseInstallations.java b/firebase-installations/src/main/java/com/google/firebase/installations/FirebaseInstallations.java index 9309b840648..57ae1df04d3 100644 --- a/firebase-installations/src/main/java/com/google/firebase/installations/FirebaseInstallations.java +++ b/firebase-installations/src/main/java/com/google/firebase/installations/FirebaseInstallations.java @@ -66,6 +66,10 @@ public class FirebaseInstallations implements FirebaseInstallationsApi { private final Object lock = new Object(); private final ExecutorService backgroundExecutor; private final ExecutorService networkExecutor; + /* FID of this Firebase Installations instance. Cached after successfully registering and + persisting the FID locally. NOTE: cachedFid resets if FID is deleted.*/ + @GuardedBy("this") + private String cachedFid; @GuardedBy("lock") private final List listeners = new ArrayList<>(); @@ -221,8 +225,15 @@ String getName() { @Override public Task getId() { preConditionChecks(); + + // Return cached fid if available. + String fid = getCacheFid(); + if (fid != null) { + return Tasks.forResult(fid); + } + Task task = addGetIdListener(); - backgroundExecutor.execute(this::doGetId); + backgroundExecutor.execute(() -> doRegistrationOrRefresh(false)); return task; } @@ -239,11 +250,7 @@ public Task getId() { public Task getToken(boolean forceRefresh) { preConditionChecks(); Task task = addGetAuthTokenListener(); - if (forceRefresh) { - backgroundExecutor.execute(this::doGetAuthTokenForceRefresh); - } else { - backgroundExecutor.execute(this::doGetAuthTokenWithoutForceRefresh); - } + backgroundExecutor.execute(() -> doRegistrationOrRefresh(forceRefresh)); return task; } @@ -261,9 +268,7 @@ public Task delete() { private Task addGetIdListener() { TaskCompletionSource taskCompletionSource = new TaskCompletionSource<>(); StateListener l = new GetIdListener(taskCompletionSource); - synchronized (lock) { - listeners.add(l); - } + addStateListeners(l); return taskCompletionSource.getTask(); } @@ -271,10 +276,14 @@ private Task addGetAuthTokenListener() { TaskCompletionSource taskCompletionSource = new TaskCompletionSource<>(); StateListener l = new GetAuthTokenListener(utils, taskCompletionSource); + addStateListeners(l); + return taskCompletionSource.getTask(); + } + + private void addStateListeners(StateListener l) { synchronized (lock) { listeners.add(l); } - return taskCompletionSource.getTask(); } private void triggerOnStateReached(PersistedInstallationEntry persistedInstallationEntry) { @@ -303,16 +312,12 @@ private void triggerOnException(PersistedInstallationEntry prefs, Exception exce } } - private final void doGetId() { - doRegistrationInternal(false); - } - - private final void doGetAuthTokenWithoutForceRefresh() { - doRegistrationInternal(false); + private synchronized void updateCacheFid(String cachedFid) { + this.cachedFid = cachedFid; } - private final void doGetAuthTokenForceRefresh() { - doRegistrationInternal(true); + private synchronized String getCacheFid() { + return cachedFid; } /** @@ -324,7 +329,8 @@ private final void doGetAuthTokenForceRefresh() { * @param forceRefresh true if this is for a getAuthToken call and if the caller wants to fetch a * new auth token from the server even if an unexpired auth token exists on the client. */ - private final void doRegistrationInternal(boolean forceRefresh) { + private final void doRegistrationOrRefresh(boolean forceRefresh) { + PersistedInstallationEntry prefs = getPrefsWithGeneratedIdMultiProcessSafe(); // Since the caller wants to force an authtoken refresh remove the authtoken from the @@ -361,6 +367,11 @@ private void doNetworkCallIfNecessary(boolean forceRefresh) { // Store the prefs to persist the result of the previous step. insertOrUpdatePrefs(prefs); + // Update cachedFID, if FID is successfully REGISTERED and persisted. + if (prefs.isRegistered()) { + updateCacheFid(prefs.getFirebaseInstallationId()); + } + // Let the caller know about the result. if (prefs.isErrored()) { triggerOnException(prefs, new FirebaseInstallationsException(Status.BAD_CONFIG)); @@ -520,6 +531,7 @@ private PersistedInstallationEntry fetchAuthTokenFromServer( case AUTH_ERROR: // The the server refused to generate a new auth token due to bad credentials, clear the // FID to force the generation of a new one. + updateCacheFid(null); return prefs.withNoGeneratedFid(); default: throw new FirebaseInstallationsException( @@ -533,6 +545,7 @@ private PersistedInstallationEntry fetchAuthTokenFromServer( * storage. */ private Void deleteFirebaseInstallationId() throws FirebaseInstallationsException { + updateCacheFid(null); PersistedInstallationEntry entry = getMultiProcessSafePrefs(); if (entry.isRegistered()) { // Call the FIS servers to delete this Firebase Installation Id. @@ -542,7 +555,6 @@ private Void deleteFirebaseInstallationId() throws FirebaseInstallationsExceptio /*projectID= */ getProjectIdentifier(), /*refreshToken= */ entry.getRefreshToken()); } - insertOrUpdatePrefs(entry.withNoGeneratedFid()); return null; } diff --git a/firebase-installations/src/main/java/com/google/firebase/installations/GetIdListener.java b/firebase-installations/src/main/java/com/google/firebase/installations/GetIdListener.java index dd79c2c9319..e1ac46cc7e5 100644 --- a/firebase-installations/src/main/java/com/google/firebase/installations/GetIdListener.java +++ b/firebase-installations/src/main/java/com/google/firebase/installations/GetIdListener.java @@ -17,6 +17,10 @@ import com.google.android.gms.tasks.TaskCompletionSource; import com.google.firebase.installations.local.PersistedInstallationEntry; +/** + * This class manages {@link PersistedInstallationEntry} state transitions valid for getId() and + * updates the caller once the id is generated. + */ class GetIdListener implements StateListener { final TaskCompletionSource taskCompletionSource; @@ -32,6 +36,8 @@ public boolean onStateReached(PersistedInstallationEntry persistedInstallationEn taskCompletionSource.trySetResult(persistedInstallationEntry.getFirebaseInstallationId()); return true; } + // Don't update the caller if the PersistedInstallationEntry registration status is + // ATTEMPT_MIGRATION or NOT_GENERATED. return false; }