From 291635a3d1e0ad0e4047a25ee39eee89deedb17f Mon Sep 17 00:00:00 2001 From: Ankita Jhawar Date: Wed, 8 Jul 2020 14:00:25 -0700 Subject: [PATCH 1/2] Revert "Completing getId call with the disk reads on the caller thread." This reverts commit f0ff2995 --- .../installations/FirebaseInstallations.java | 54 +++++++++---------- 1 file changed, 25 insertions(+), 29 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 da460dae2d0..9309b840648 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,10 +66,6 @@ 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<>(); @@ -225,7 +221,9 @@ String getName() { @Override public Task getId() { preConditionChecks(); - return Tasks.forResult(doGetId()); + Task task = addGetIdListener(); + backgroundExecutor.execute(this::doGetId); + return task; } /** @@ -241,7 +239,11 @@ public Task getId() { public Task getToken(boolean forceRefresh) { preConditionChecks(); Task task = addGetAuthTokenListener(); - backgroundExecutor.execute(() -> doGetAuthToken(forceRefresh)); + if (forceRefresh) { + backgroundExecutor.execute(this::doGetAuthTokenForceRefresh); + } else { + backgroundExecutor.execute(this::doGetAuthTokenWithoutForceRefresh); + } return task; } @@ -256,6 +258,15 @@ public Task delete() { return Tasks.call(backgroundExecutor, this::deleteFirebaseInstallationId); } + private Task addGetIdListener() { + TaskCompletionSource taskCompletionSource = new TaskCompletionSource<>(); + StateListener l = new GetIdListener(taskCompletionSource); + synchronized (lock) { + listeners.add(l); + } + return taskCompletionSource.getTask(); + } + private Task addGetAuthTokenListener() { TaskCompletionSource taskCompletionSource = new TaskCompletionSource<>(); @@ -292,25 +303,16 @@ private void triggerOnException(PersistedInstallationEntry prefs, Exception exce } } - private synchronized void updateCacheFid(String cachedFid) { - this.cachedFid = cachedFid; + private final void doGetId() { + doRegistrationInternal(false); } - private synchronized String getCacheFid() { - return cachedFid; + private final void doGetAuthTokenWithoutForceRefresh() { + doRegistrationInternal(false); } - private String doGetId() { - - String fid = getCacheFid(); - if (fid != null) { - return fid; - } - PersistedInstallationEntry prefs = getPrefsWithGeneratedIdMultiProcessSafe(); - // Execute network calls (CreateInstallations) to the FIS Servers on a separate executor - // i.e networkExecutor - networkExecutor.execute(() -> doNetworkCallIfNecessary(false)); - return prefs.getFirebaseInstallationId(); + private final void doGetAuthTokenForceRefresh() { + doRegistrationInternal(true); } /** @@ -322,7 +324,7 @@ private String doGetId() { * @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 void doGetAuthToken(boolean forceRefresh) { + private final void doRegistrationInternal(boolean forceRefresh) { PersistedInstallationEntry prefs = getPrefsWithGeneratedIdMultiProcessSafe(); // Since the caller wants to force an authtoken refresh remove the authtoken from the @@ -359,11 +361,6 @@ 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)); @@ -523,7 +520,6 @@ 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( @@ -537,7 +533,6 @@ 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. @@ -547,6 +542,7 @@ private Void deleteFirebaseInstallationId() throws FirebaseInstallationsExceptio /*projectID= */ getProjectIdentifier(), /*refreshToken= */ entry.getRefreshToken()); } + insertOrUpdatePrefs(entry.withNoGeneratedFid()); return null; } From e4e8571c6847af9f367502397128076aa14e4692 Mon Sep 17 00:00:00 2001 From: Ankita Jhawar Date: Wed, 8 Jul 2020 14:01:10 -0700 Subject: [PATCH 2/2] Revert "Addressing Fred's comments" This reverts commit bc6ec5ce --- .../firebase/installations/GetIdListener.java | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 firebase-installations/src/main/java/com/google/firebase/installations/GetIdListener.java 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 new file mode 100644 index 00000000000..dd79c2c9319 --- /dev/null +++ b/firebase-installations/src/main/java/com/google/firebase/installations/GetIdListener.java @@ -0,0 +1,43 @@ +// Copyright 2019 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.firebase.installations; + +import com.google.android.gms.tasks.TaskCompletionSource; +import com.google.firebase.installations.local.PersistedInstallationEntry; + +class GetIdListener implements StateListener { + final TaskCompletionSource taskCompletionSource; + + public GetIdListener(TaskCompletionSource taskCompletionSource) { + this.taskCompletionSource = taskCompletionSource; + } + + @Override + public boolean onStateReached(PersistedInstallationEntry persistedInstallationEntry) { + if (persistedInstallationEntry.isUnregistered() + || persistedInstallationEntry.isRegistered() + || persistedInstallationEntry.isErrored()) { + taskCompletionSource.trySetResult(persistedInstallationEntry.getFirebaseInstallationId()); + return true; + } + return false; + } + + @Override + public boolean onException( + PersistedInstallationEntry persistedInstallationEntry, Exception exception) { + return false; + } +}