From 49708d26ffce81f6c2eb8ddeaaf590cd30da009f Mon Sep 17 00:00:00 2001 From: Ankita Jhawar Date: Wed, 25 Sep 2019 13:18:29 -0700 Subject: [PATCH 01/15] Simplifying FirebaseInstallations class by adding listeners. --- firebase-installations/api.txt | 10 +- ...FirebaseInstallationsInstrumentedTest.java | 283 ++++++++++------- .../FisAndroidTestConstants.java | 24 +- .../installations/local/PersistedFidTest.java | 6 +- .../installations/FirebaseInstallations.java | 292 ++++++------------ .../firebase/installations/StateListener.java | 79 +++++ .../google/firebase/installations/Utils.java | 24 ++ .../installations/local/PersistedFid.java | 71 +++-- .../local/PersistedFidEntry.java | 19 +- 9 files changed, 462 insertions(+), 346 deletions(-) create mode 100644 firebase-installations/src/main/java/com/google/firebase/installations/StateListener.java diff --git a/firebase-installations/api.txt b/firebase-installations/api.txt index 1b73247dcfa..1514498e651 100644 --- a/firebase-installations/api.txt +++ b/firebase-installations/api.txt @@ -29,11 +29,11 @@ package com.google.firebase.installations.local { ctor public PersistedFid(@NonNull FirebaseApp); method @NonNull public boolean clear(); method @NonNull public boolean insertOrUpdatePersistedFidEntry(@NonNull com.google.firebase.installations.local.PersistedFidEntry); - method @Nullable public com.google.firebase.installations.local.PersistedFidEntry readPersistedFidEntryValue(); + method @NonNull public com.google.firebase.installations.local.PersistedFidEntry readPersistedFidEntryValue(); } public enum PersistedFid.RegistrationStatus { - enum_constant public static final com.google.firebase.installations.local.PersistedFid.RegistrationStatus PENDING; + enum_constant public static final com.google.firebase.installations.local.PersistedFid.RegistrationStatus NOT_GENERATED; enum_constant public static final com.google.firebase.installations.local.PersistedFid.RegistrationStatus REGISTERED; enum_constant public static final com.google.firebase.installations.local.PersistedFid.RegistrationStatus REGISTER_ERROR; enum_constant public static final com.google.firebase.installations.local.PersistedFid.RegistrationStatus UNREGISTERED; @@ -44,10 +44,14 @@ package com.google.firebase.installations.local { method @NonNull public static com.google.firebase.installations.local.PersistedFidEntry.Builder builder(); method @Nullable public abstract String getAuthToken(); method public abstract long getExpiresInSecs(); - method @NonNull public abstract String getFirebaseInstallationId(); + method @Nullable public abstract String getFirebaseInstallationId(); method @Nullable public abstract String getRefreshToken(); method @NonNull public abstract com.google.firebase.installations.local.PersistedFid.RegistrationStatus getRegistrationStatus(); method public abstract long getTokenCreationEpochInSecs(); + method public boolean isErrored(); + method public boolean isNotGenerated(); + method public boolean isRegistered(); + method public boolean isUnregistered(); method @NonNull public abstract com.google.firebase.installations.local.PersistedFidEntry.Builder toBuilder(); } diff --git a/firebase-installations/src/androidTest/java/com/google/firebase/installations/FirebaseInstallationsInstrumentedTest.java b/firebase-installations/src/androidTest/java/com/google/firebase/installations/FirebaseInstallationsInstrumentedTest.java index f28b63517a4..8b6b4b39187 100644 --- a/firebase-installations/src/androidTest/java/com/google/firebase/installations/FirebaseInstallationsInstrumentedTest.java +++ b/firebase-installations/src/androidTest/java/com/google/firebase/installations/FirebaseInstallationsInstrumentedTest.java @@ -15,6 +15,7 @@ package com.google.firebase.installations; import static com.google.common.truth.Truth.assertWithMessage; +import static com.google.firebase.installations.FisAndroidTestConstants.DEFAULT_PERSISTED_FID_ENTRY; import static com.google.firebase.installations.FisAndroidTestConstants.TEST_API_KEY; import static com.google.firebase.installations.FisAndroidTestConstants.TEST_APP_ID_1; import static com.google.firebase.installations.FisAndroidTestConstants.TEST_AUTH_TOKEN; @@ -24,10 +25,13 @@ import static com.google.firebase.installations.FisAndroidTestConstants.TEST_CREATION_TIMESTAMP_1; import static com.google.firebase.installations.FisAndroidTestConstants.TEST_CREATION_TIMESTAMP_2; import static com.google.firebase.installations.FisAndroidTestConstants.TEST_FID_1; +import static com.google.firebase.installations.FisAndroidTestConstants.TEST_INSTALLATION_RESPONSE; +import static com.google.firebase.installations.FisAndroidTestConstants.TEST_INSTALLATION_TOKEN_RESULT; import static com.google.firebase.installations.FisAndroidTestConstants.TEST_PROJECT_ID; import static com.google.firebase.installations.FisAndroidTestConstants.TEST_REFRESH_TOKEN; import static com.google.firebase.installations.FisAndroidTestConstants.TEST_TOKEN_EXPIRATION_TIMESTAMP; import static com.google.firebase.installations.FisAndroidTestConstants.TEST_TOKEN_EXPIRATION_TIMESTAMP_2; +import static com.google.firebase.installations.local.PersistedFidEntrySubject.assertThat; import static org.junit.Assert.fail; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; @@ -41,16 +45,15 @@ import androidx.test.core.app.ApplicationProvider; import androidx.test.runner.AndroidJUnit4; -import com.google.android.gms.common.util.Clock; import com.google.android.gms.tasks.Task; import com.google.android.gms.tasks.Tasks; import com.google.firebase.FirebaseApp; import com.google.firebase.FirebaseOptions; import com.google.firebase.installations.local.PersistedFid; +import com.google.firebase.installations.local.PersistedFid.RegistrationStatus; import com.google.firebase.installations.local.PersistedFidEntry; import com.google.firebase.installations.remote.FirebaseInstallationServiceClient; import com.google.firebase.installations.remote.FirebaseInstallationServiceException; -import com.google.firebase.installations.remote.InstallationResponse; import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.LinkedBlockingQueue; @@ -81,15 +84,15 @@ public class FirebaseInstallationsInstrumentedTest { @Mock private FirebaseInstallationServiceClient backendClientReturnsError; @Mock private PersistedFid persistedFidReturnsError; @Mock private Utils mockUtils; - @Mock private Clock mockClock; @Mock private PersistedFid mockPersistedFid; + @Mock private FirebaseInstallationServiceClient mockClient; private static final PersistedFidEntry REGISTERED_FID_ENTRY = PersistedFidEntry.builder() .setFirebaseInstallationId(TEST_FID_1) .setAuthToken(TEST_AUTH_TOKEN) .setRefreshToken(TEST_REFRESH_TOKEN) - .setTokenCreationEpochInSecs(TEST_CREATION_TIMESTAMP_2) + .setTokenCreationEpochInSecs(TEST_CREATION_TIMESTAMP_1) .setExpiresInSecs(TEST_TOKEN_EXPIRATION_TIMESTAMP) .setRegistrationStatus(PersistedFid.RegistrationStatus.REGISTERED) .build(); @@ -99,7 +102,7 @@ public class FirebaseInstallationsInstrumentedTest { .setFirebaseInstallationId(TEST_FID_1) .setAuthToken(TEST_AUTH_TOKEN) .setRefreshToken(TEST_REFRESH_TOKEN) - .setTokenCreationEpochInSecs(TEST_CREATION_TIMESTAMP_2) + .setTokenCreationEpochInSecs(TEST_CREATION_TIMESTAMP_1) .setExpiresInSecs(TEST_TOKEN_EXPIRATION_TIMESTAMP_2) .setRegistrationStatus(PersistedFid.RegistrationStatus.REGISTERED) .build(); @@ -109,7 +112,7 @@ public class FirebaseInstallationsInstrumentedTest { .setFirebaseInstallationId(TEST_FID_1) .setAuthToken("") .setRefreshToken("") - .setTokenCreationEpochInSecs(TEST_CREATION_TIMESTAMP_2) + .setTokenCreationEpochInSecs(TEST_CREATION_TIMESTAMP_1) .setExpiresInSecs(0) .setRegistrationStatus(PersistedFid.RegistrationStatus.UNREGISTERED) .build(); @@ -138,34 +141,28 @@ public void setUp() throws FirebaseInstallationServiceException { .setApiKey(TEST_API_KEY) .build()); persistedFid = new PersistedFid(firebaseApp); + when(backendClientReturnsOk.createFirebaseInstallation( anyString(), anyString(), anyString(), anyString())) - .thenReturn( - InstallationResponse.builder() - .setName("/projects/" + TEST_PROJECT_ID + "/installations/" + TEST_FID_1) - .setRefreshToken(TEST_REFRESH_TOKEN) - .setAuthToken( - InstallationTokenResult.builder() - .setToken(TEST_AUTH_TOKEN) - .setTokenExpirationInSecs(TEST_TOKEN_EXPIRATION_TIMESTAMP) - .build()) - .build()); + .thenReturn(TEST_INSTALLATION_RESPONSE); when(backendClientReturnsOk.generateAuthToken( anyString(), anyString(), anyString(), anyString())) - .thenReturn( - InstallationTokenResult.builder() - .setToken(TEST_AUTH_TOKEN_2) - .setTokenExpirationInSecs(TEST_TOKEN_EXPIRATION_TIMESTAMP) - .build()); + .thenReturn(TEST_INSTALLATION_TOKEN_RESULT); + when(backendClientReturnsError.createFirebaseInstallation( anyString(), anyString(), anyString(), anyString())) .thenThrow( new FirebaseInstallationServiceException( "SDK Error", FirebaseInstallationServiceException.Status.SERVER_ERROR)); + when(persistedFidReturnsError.insertOrUpdatePersistedFidEntry(any())).thenReturn(false); - when(persistedFidReturnsError.readPersistedFidEntryValue()).thenReturn(null); + when(persistedFidReturnsError.readPersistedFidEntryValue()) + .thenReturn(DEFAULT_PERSISTED_FID_ENTRY); + when(mockUtils.createRandomFid()).thenReturn(TEST_FID_1); - when(mockClock.currentTimeMillis()).thenReturn(TEST_CREATION_TIMESTAMP_1); + when(mockUtils.currentTimeInSecs()).thenReturn(TEST_CREATION_TIMESTAMP_2); + when(mockUtils.isAuthTokenExpired(any())).thenReturn(false); + // Mocks success on FIS deletion doNothing() .when(backendClientReturnsOk) @@ -187,93 +184,73 @@ public void cleanUp() throws Exception { public void testGetId_PersistedFidOk_BackendOk() throws Exception { FirebaseInstallations firebaseInstallations = new FirebaseInstallations( - mockClock, executor, firebaseApp, backendClientReturnsOk, persistedFid, mockUtils); + executor, firebaseApp, backendClientReturnsOk, persistedFid, mockUtils); // No exception, means success. - assertWithMessage("getId Task fails.") + assertWithMessage("getId Task failed.") .that(Tasks.await(firebaseInstallations.getId())) .isNotEmpty(); PersistedFidEntry entryValue = persistedFid.readPersistedFidEntryValue(); - assertWithMessage("Persisted Fid doesn't match") - .that(entryValue.getFirebaseInstallationId()) - .isEqualTo(TEST_FID_1); + assertThat(entryValue).hasFid(TEST_FID_1); // Waiting for Task that registers FID on the FIS Servers executor.awaitTermination(500, TimeUnit.MILLISECONDS); PersistedFidEntry updatedFidEntry = persistedFid.readPersistedFidEntryValue(); - assertWithMessage("Persisted Fid doesn't match") - .that(updatedFidEntry.getFirebaseInstallationId()) - .isEqualTo(TEST_FID_1); - assertWithMessage("Registration status doesn't match") - .that(updatedFidEntry.getRegistrationStatus()) - .isEqualTo(PersistedFid.RegistrationStatus.REGISTERED); + assertThat(updatedFidEntry).hasFid(TEST_FID_1); + assertThat(updatedFidEntry).hasRegistrationStatus(RegistrationStatus.REGISTERED); } @Test public void testGetId_multipleCalls_sameFIDReturned() throws Exception { FirebaseInstallations firebaseInstallations = new FirebaseInstallations( - mockClock, executor, firebaseApp, backendClientReturnsOk, persistedFid, mockUtils); - - // No exception, means success. - assertWithMessage("getId Task fails.") - .that(Tasks.await(firebaseInstallations.getId())) - .isNotEmpty(); - PersistedFidEntry entryValue = persistedFid.readPersistedFidEntryValue(); - assertWithMessage("Persisted Fid doesn't match") - .that(entryValue.getFirebaseInstallationId()) - .isEqualTo(TEST_FID_1); - - Tasks.await(firebaseInstallations.getId()); + executor, firebaseApp, backendClientReturnsOk, persistedFid, mockUtils); + // Call getId multiple times + Task task1 = firebaseInstallations.getId(); + Task task2 = firebaseInstallations.getId(); + Tasks.await(Tasks.whenAllComplete(task1, task2)); // Waiting for Task that registers FID on the FIS Servers executor.awaitTermination(500, TimeUnit.MILLISECONDS); - PersistedFidEntry updatedFidEntry = persistedFid.readPersistedFidEntryValue(); - assertWithMessage("Persisted Fid doesn't match") - .that(updatedFidEntry.getFirebaseInstallationId()) + assertWithMessage("Persisted Fid of Task1 doesn't match.") + .that(task1.getResult()) .isEqualTo(TEST_FID_1); - assertWithMessage("Registration status doesn't match") - .that(updatedFidEntry.getRegistrationStatus()) - .isEqualTo(PersistedFid.RegistrationStatus.REGISTERED); + assertWithMessage("Persisted Fid of Task2 doesn't match.") + .that(task2.getResult()) + .isEqualTo(TEST_FID_1); + verify(backendClientReturnsOk, times(1)) + .createFirebaseInstallation(TEST_API_KEY, TEST_FID_1, TEST_PROJECT_ID, TEST_APP_ID_1); + PersistedFidEntry updatedFidEntry = persistedFid.readPersistedFidEntryValue(); + assertThat(updatedFidEntry).hasFid(TEST_FID_1); + assertThat(updatedFidEntry).hasRegistrationStatus(RegistrationStatus.REGISTERED); } @Test public void testGetId_PersistedFidOk_BackendError() throws Exception { FirebaseInstallations firebaseInstallations = new FirebaseInstallations( - mockClock, executor, firebaseApp, backendClientReturnsError, persistedFid, mockUtils); + executor, firebaseApp, backendClientReturnsError, persistedFid, mockUtils); Tasks.await(firebaseInstallations.getId()); PersistedFidEntry entryValue = persistedFid.readPersistedFidEntryValue(); - assertWithMessage("Persisted Fid doesn't match") - .that(entryValue.getFirebaseInstallationId()) - .isEqualTo(TEST_FID_1); + assertThat(entryValue).hasFid(TEST_FID_1); // Waiting for Task that registers FID on the FIS Servers executor.awaitTermination(500, TimeUnit.MILLISECONDS); PersistedFidEntry updatedFidEntry = persistedFid.readPersistedFidEntryValue(); - assertWithMessage("Persisted Fid doesn't match") - .that(updatedFidEntry.getFirebaseInstallationId()) - .isEqualTo(TEST_FID_1); - assertWithMessage("Registration Fid doesn't match") - .that(updatedFidEntry.getRegistrationStatus()) - .isEqualTo(PersistedFid.RegistrationStatus.REGISTER_ERROR); + assertThat(updatedFidEntry).hasFid(TEST_FID_1); + assertThat(updatedFidEntry).hasRegistrationStatus(RegistrationStatus.REGISTER_ERROR); } @Test public void testGetId_PersistedFidError_BackendOk() throws InterruptedException { FirebaseInstallations firebaseInstallations = new FirebaseInstallations( - mockClock, - executor, - firebaseApp, - backendClientReturnsOk, - persistedFidReturnsError, - mockUtils); + executor, firebaseApp, backendClientReturnsOk, persistedFidReturnsError, mockUtils); // Expect exception try { @@ -290,30 +267,106 @@ public void testGetId_PersistedFidError_BackendOk() throws InterruptedException } } + @Test + public void testGetId_fidRegistrationUncheckedException_statusUpdated() throws Exception { + // Mocking unchecked exception on FIS createFirebaseInstallation + when(mockClient.createFirebaseInstallation(anyString(), anyString(), anyString(), anyString())) + .thenAnswer( + invocation -> { + throw new InterruptedException(); + }); + + FirebaseInstallations firebaseInstallations = + new FirebaseInstallations(executor, firebaseApp, mockClient, persistedFid, mockUtils); + + Tasks.await(firebaseInstallations.getId()); + + PersistedFidEntry entryValue = persistedFid.readPersistedFidEntryValue(); + assertThat(entryValue).hasFid(TEST_FID_1); + + // Waiting for Task that registers FID on the FIS Servers + executor.awaitTermination(500, TimeUnit.MILLISECONDS); + + // Validate that registration status is REGISTER_ERROR + PersistedFidEntry updatedFidEntry = persistedFid.readPersistedFidEntryValue(); + assertThat(updatedFidEntry).hasFid(TEST_FID_1); + assertThat(updatedFidEntry).hasRegistrationStatus(RegistrationStatus.REGISTER_ERROR); + } + + @Test + public void testGetId_expiredAuthTokenUncheckedException_statusUpdated() throws Exception { + // Update local storage with fid entry that has auth token expired. + persistedFid.insertOrUpdatePersistedFidEntry(EXPIRED_AUTH_TOKEN_ENTRY); + // Mocking unchecked exception on FIS generateAuthToken + when(mockClient.generateAuthToken(anyString(), anyString(), anyString(), anyString())) + .thenAnswer( + invocation -> { + throw new InterruptedException(); + }); + when(mockUtils.isAuthTokenExpired(any())).thenReturn(true); + + FirebaseInstallations firebaseInstallations = + new FirebaseInstallations(executor, firebaseApp, mockClient, persistedFid, mockUtils); + + assertWithMessage("getId Task failed") + .that(Tasks.await(firebaseInstallations.getId())) + .isNotEmpty(); + PersistedFidEntry entryValue = persistedFid.readPersistedFidEntryValue(); + assertThat(entryValue).hasFid(TEST_FID_1); + + // Waiting for Task that generates auth token with the FIS Servers + executor.awaitTermination(500, TimeUnit.MILLISECONDS); + + // Validate that registration status is REGISTER_ERROR + PersistedFidEntry updatedFidEntry = persistedFid.readPersistedFidEntryValue(); + assertThat(updatedFidEntry).hasFid(TEST_FID_1); + assertThat(updatedFidEntry).hasRegistrationStatus(RegistrationStatus.REGISTER_ERROR); + } + + @Test + public void testGetId_expiredAuthToken_refreshesAuthToken() throws Exception { + when(mockUtils.isAuthTokenExpired(any())).thenReturn(true); + // Update local storage with fid entry that has auth token expired. + persistedFid.insertOrUpdatePersistedFidEntry(EXPIRED_AUTH_TOKEN_ENTRY); + FirebaseInstallations firebaseInstallations = + new FirebaseInstallations( + executor, firebaseApp, backendClientReturnsOk, persistedFid, mockUtils); + + assertWithMessage("getId Task failed") + .that(Tasks.await(firebaseInstallations.getId())) + .isNotEmpty(); + PersistedFidEntry entryValue = persistedFid.readPersistedFidEntryValue(); + assertThat(entryValue).hasFid(TEST_FID_1); + + // Waiting for Task that registers FID on the FIS Servers + executor.awaitTermination(500, TimeUnit.MILLISECONDS); + + // Validate that registration is complete with updated auth token + PersistedFidEntry updatedFidEntry = persistedFid.readPersistedFidEntryValue(); + assertThat(updatedFidEntry).isEqualTo(UPDATED_AUTH_TOKEN_FID_ENTRY); + verify(backendClientReturnsOk, never()) + .createFirebaseInstallation(TEST_API_KEY, TEST_FID_1, TEST_PROJECT_ID, TEST_APP_ID_1); + verify(backendClientReturnsOk, times(1)) + .generateAuthToken(TEST_API_KEY, TEST_FID_1, TEST_PROJECT_ID, TEST_REFRESH_TOKEN); + } + @Test public void testGetAuthToken_fidDoesNotExist_successful() throws Exception { FirebaseInstallations firebaseInstallations = new FirebaseInstallations( - mockClock, executor, firebaseApp, backendClientReturnsOk, persistedFid, mockUtils); + executor, firebaseApp, backendClientReturnsOk, persistedFid, mockUtils); Tasks.await(firebaseInstallations.getAuthToken(FirebaseInstallationsApi.DO_NOT_FORCE_REFRESH)); PersistedFidEntry entryValue = persistedFid.readPersistedFidEntryValue(); - assertWithMessage("Persisted Auth Token doesn't match") - .that(entryValue.getAuthToken()) - .isEqualTo(TEST_AUTH_TOKEN); + assertThat(entryValue).hasAuthToken(TEST_AUTH_TOKEN); } @Test public void testGetAuthToken_PersistedFidError_failure() throws Exception { FirebaseInstallations firebaseInstallations = new FirebaseInstallations( - mockClock, - executor, - firebaseApp, - backendClientReturnsOk, - persistedFidReturnsError, - mockUtils); + executor, firebaseApp, backendClientReturnsOk, persistedFidReturnsError, mockUtils); // Expect exception try { @@ -336,7 +389,7 @@ public void testGetAuthToken_fidExists_successful() throws Exception { when(mockPersistedFid.readPersistedFidEntryValue()).thenReturn(REGISTERED_FID_ENTRY); FirebaseInstallations firebaseInstallations = new FirebaseInstallations( - mockClock, executor, firebaseApp, backendClientReturnsOk, mockPersistedFid, mockUtils); + executor, firebaseApp, backendClientReturnsOk, mockPersistedFid, mockUtils); InstallationTokenResult installationTokenResult = Tasks.await( @@ -345,14 +398,17 @@ public void testGetAuthToken_fidExists_successful() throws Exception { assertWithMessage("Persisted Auth Token doesn't match") .that(installationTokenResult.getToken()) .isEqualTo(TEST_AUTH_TOKEN); + verify(backendClientReturnsOk, never()) + .generateAuthToken(TEST_API_KEY, TEST_FID_1, TEST_PROJECT_ID, TEST_REFRESH_TOKEN); } @Test public void testGetAuthToken_expiredAuthToken_fetchedNewTokenFromFIS() throws Exception { - when(mockPersistedFid.readPersistedFidEntryValue()).thenReturn(EXPIRED_AUTH_TOKEN_ENTRY); + when(mockUtils.isAuthTokenExpired(any())).thenReturn(true, true, false); + persistedFid.insertOrUpdatePersistedFidEntry(EXPIRED_AUTH_TOKEN_ENTRY); FirebaseInstallations firebaseInstallations = new FirebaseInstallations( - mockClock, executor, firebaseApp, backendClientReturnsOk, mockPersistedFid, mockUtils); + executor, firebaseApp, backendClientReturnsOk, persistedFid, mockUtils); InstallationTokenResult installationTokenResult = Tasks.await( @@ -361,18 +417,18 @@ public void testGetAuthToken_expiredAuthToken_fetchedNewTokenFromFIS() throws Ex assertWithMessage("Persisted Auth Token doesn't match") .that(installationTokenResult.getToken()) .isEqualTo(TEST_AUTH_TOKEN_2); + verify(backendClientReturnsOk, times(1)) + .generateAuthToken(TEST_API_KEY, TEST_FID_1, TEST_PROJECT_ID, TEST_REFRESH_TOKEN); } @Test public void testGetAuthToken_unregisteredFid_fetchedNewTokenFromFIS() throws Exception { - // Using mockPersistedFid to ensure the order of returning persistedFidEntry. This test - // validates that getAuthToken calls getId to ensure FID registration and returns a valid auth - // token. - when(mockPersistedFid.readPersistedFidEntryValue()) - .thenReturn(UNREGISTERED_FID_ENTRY, REGISTERED_FID_ENTRY); + // Update local storage with a unregistered fid entry to validate that getAuthToken calls getId + // to ensure FID registration and returns a valid auth token. + persistedFid.insertOrUpdatePersistedFidEntry(UNREGISTERED_FID_ENTRY); FirebaseInstallations firebaseInstallations = new FirebaseInstallations( - mockClock, executor, firebaseApp, backendClientReturnsOk, mockPersistedFid, mockUtils); + executor, firebaseApp, backendClientReturnsOk, persistedFid, mockUtils); InstallationTokenResult installationTokenResult = Tasks.await( @@ -381,6 +437,8 @@ public void testGetAuthToken_unregisteredFid_fetchedNewTokenFromFIS() throws Exc assertWithMessage("Persisted Auth Token doesn't match") .that(installationTokenResult.getToken()) .isEqualTo(TEST_AUTH_TOKEN); + verify(backendClientReturnsOk, times(1)) + .createFirebaseInstallation(TEST_API_KEY, TEST_FID_1, TEST_PROJECT_ID, TEST_APP_ID_1); } @Test @@ -393,12 +451,7 @@ public void testGetAuthToken_serverError_failure() throws Exception { "Server Error", FirebaseInstallationServiceException.Status.SERVER_ERROR)); FirebaseInstallations firebaseInstallations = new FirebaseInstallations( - mockClock, - executor, - firebaseApp, - backendClientReturnsError, - mockPersistedFid, - mockUtils); + executor, firebaseApp, backendClientReturnsError, mockPersistedFid, mockUtils); // Expect exception try { @@ -418,18 +471,14 @@ public void testGetAuthToken_serverError_failure() throws Exception { @Test public void testGetAuthToken_multipleCallsDoNotForceRefresh_fetchedNewTokenOnce() throws Exception { - // Using mockPersistedFid to ensure the order of returning persistedFidEntry to 2 tasks - // triggered simultaneously. Task2 waits for Task1 to complete. On Task1 completion, task2 reads - // the UPDATED_AUTH_TOKEN_FID_ENTRY by Task1 on execution. - when(mockPersistedFid.readPersistedFidEntryValue()) - .thenReturn( - EXPIRED_AUTH_TOKEN_ENTRY, - EXPIRED_AUTH_TOKEN_ENTRY, - EXPIRED_AUTH_TOKEN_ENTRY, - UPDATED_AUTH_TOKEN_FID_ENTRY); + // Update local storage with a EXPIRED_AUTH_TOKEN_ENTRY to validate the flow of multiple tasks + // triggered simultaneously. Task2 waits for Task1 to complete. On task1 completion, task2 reads + // the UPDATED_AUTH_TOKEN_FID_ENTRY generated by Task1. + persistedFid.insertOrUpdatePersistedFidEntry(EXPIRED_AUTH_TOKEN_ENTRY); + when(mockUtils.isAuthTokenExpired(any())).thenReturn(true, true, true, false); FirebaseInstallations firebaseInstallations = new FirebaseInstallations( - mockClock, executor, firebaseApp, backendClientReturnsOk, mockPersistedFid, mockUtils); + executor, firebaseApp, backendClientReturnsOk, persistedFid, mockUtils); // Call getAuthToken multiple times with DO_NOT_FORCE_REFRESH option Task task1 = @@ -451,14 +500,14 @@ public void testGetAuthToken_multipleCallsDoNotForceRefresh_fetchedNewTokenOnce( @Test public void testGetAuthToken_multipleCallsForceRefresh_fetchedNewTokenTwice() throws Exception { - when(mockPersistedFid.readPersistedFidEntryValue()).thenReturn(REGISTERED_FID_ENTRY); - // Use a mock ServiceClient for network calls with delay(1000ms) to ensure first task is not + persistedFid.insertOrUpdatePersistedFidEntry(REGISTERED_FID_ENTRY); + // Use a mock ServiceClient for network calls with delay(500ms) to ensure first task is not // completed before the second task starts. Hence, we can test multiple calls to getAuthToken() // and verify one task waits for another task to complete. doAnswer( AdditionalAnswers.answersWithDelay( - 1000, + 500, (unused) -> InstallationTokenResult.builder() .setToken(TEST_AUTH_TOKEN_3) @@ -466,7 +515,7 @@ public void testGetAuthToken_multipleCallsForceRefresh_fetchedNewTokenTwice() th .build())) .doAnswer( AdditionalAnswers.answersWithDelay( - 1000, + 500, (unused) -> InstallationTokenResult.builder() .setToken(TEST_AUTH_TOKEN_4) @@ -477,7 +526,7 @@ public void testGetAuthToken_multipleCallsForceRefresh_fetchedNewTokenTwice() th FirebaseInstallations firebaseInstallations = new FirebaseInstallations( - mockClock, executor, firebaseApp, backendClientReturnsOk, mockPersistedFid, mockUtils); + executor, firebaseApp, backendClientReturnsOk, persistedFid, mockUtils); // Call getAuthToken multiple times with FORCE_REFRESH option. Task task1 = @@ -503,12 +552,12 @@ public void testDelete_registeredFID_successful() throws Exception { persistedFid.insertOrUpdatePersistedFidEntry(REGISTERED_FID_ENTRY); FirebaseInstallations firebaseInstallations = new FirebaseInstallations( - mockClock, executor, firebaseApp, backendClientReturnsOk, persistedFid, mockUtils); + executor, firebaseApp, backendClientReturnsOk, persistedFid, mockUtils); Tasks.await(firebaseInstallations.delete()); PersistedFidEntry entryValue = persistedFid.readPersistedFidEntryValue(); - assertWithMessage("Persisted Fid Entry is not null.").that(entryValue).isNull(); + assertThat(entryValue).isEqualTo(DEFAULT_PERSISTED_FID_ENTRY); verify(backendClientReturnsOk, times(1)) .deleteFirebaseInstallation(TEST_API_KEY, TEST_FID_1, TEST_PROJECT_ID, TEST_REFRESH_TOKEN); } @@ -519,12 +568,12 @@ public void testDelete_unregisteredFID_successful() throws Exception { persistedFid.insertOrUpdatePersistedFidEntry(UNREGISTERED_FID_ENTRY); FirebaseInstallations firebaseInstallations = new FirebaseInstallations( - mockClock, executor, firebaseApp, backendClientReturnsOk, persistedFid, mockUtils); + executor, firebaseApp, backendClientReturnsOk, persistedFid, mockUtils); Tasks.await(firebaseInstallations.delete()); PersistedFidEntry entryValue = persistedFid.readPersistedFidEntryValue(); - assertWithMessage("Persisted Fid Entry is not null.").that(entryValue).isNull(); + assertThat(entryValue).isEqualTo(DEFAULT_PERSISTED_FID_ENTRY); verify(backendClientReturnsOk, never()) .deleteFirebaseInstallation(TEST_API_KEY, TEST_FID_1, TEST_PROJECT_ID, TEST_REFRESH_TOKEN); } @@ -533,12 +582,12 @@ public void testDelete_unregisteredFID_successful() throws Exception { public void testDelete_emptyPersistedFidEntry_successful() throws Exception { FirebaseInstallations firebaseInstallations = new FirebaseInstallations( - mockClock, executor, firebaseApp, backendClientReturnsOk, persistedFid, mockUtils); + executor, firebaseApp, backendClientReturnsOk, persistedFid, mockUtils); Tasks.await(firebaseInstallations.delete()); PersistedFidEntry entryValue = persistedFid.readPersistedFidEntryValue(); - assertWithMessage("Persisted Fid Entry is not null.").that(entryValue).isNull(); + assertThat(entryValue).isEqualTo(DEFAULT_PERSISTED_FID_ENTRY); verify(backendClientReturnsOk, never()) .deleteFirebaseInstallation(TEST_API_KEY, TEST_FID_1, TEST_PROJECT_ID, TEST_REFRESH_TOKEN); } @@ -549,7 +598,7 @@ public void testDelete_serverError_failure() throws Exception { persistedFid.insertOrUpdatePersistedFidEntry(REGISTERED_FID_ENTRY); FirebaseInstallations firebaseInstallations = new FirebaseInstallations( - mockClock, executor, firebaseApp, backendClientReturnsError, persistedFid, mockUtils); + executor, firebaseApp, backendClientReturnsError, persistedFid, mockUtils); // Expect exception try { @@ -564,9 +613,7 @@ public void testDelete_serverError_failure() throws Exception { .that(((FirebaseInstallationsException) expected.getCause()).getStatus()) .isEqualTo(FirebaseInstallationsException.Status.SDK_INTERNAL_ERROR); PersistedFidEntry entryValue = persistedFid.readPersistedFidEntryValue(); - assertWithMessage("Persisted Fid Entry doesn't match") - .that(entryValue) - .isEqualTo(REGISTERED_FID_ENTRY); + assertThat(entryValue).isEqualTo(REGISTERED_FID_ENTRY); } } } diff --git a/firebase-installations/src/androidTest/java/com/google/firebase/installations/FisAndroidTestConstants.java b/firebase-installations/src/androidTest/java/com/google/firebase/installations/FisAndroidTestConstants.java index e77a2b1155b..5645e3a5c56 100644 --- a/firebase-installations/src/androidTest/java/com/google/firebase/installations/FisAndroidTestConstants.java +++ b/firebase-installations/src/androidTest/java/com/google/firebase/installations/FisAndroidTestConstants.java @@ -14,6 +14,9 @@ package com.google.firebase.installations; +import com.google.firebase.installations.local.PersistedFidEntry; +import com.google.firebase.installations.remote.InstallationResponse; + public final class FisAndroidTestConstants { public static final String TEST_FID_1 = "cccccccccccccccccccccc"; @@ -35,5 +38,24 @@ public final class FisAndroidTestConstants { public static final long TEST_TOKEN_EXPIRATION_TIMESTAMP_2 = 2000L; public static final long TEST_CREATION_TIMESTAMP_1 = 2000L; - public static final long TEST_CREATION_TIMESTAMP_2 = 2000L; + public static final long TEST_CREATION_TIMESTAMP_2 = 2L; + + public static final PersistedFidEntry DEFAULT_PERSISTED_FID_ENTRY = + PersistedFidEntry.builder().build(); + public static final InstallationResponse TEST_INSTALLATION_RESPONSE = + InstallationResponse.builder() + .setName("/projects/" + TEST_PROJECT_ID + "/installations/" + TEST_FID_1) + .setRefreshToken(TEST_REFRESH_TOKEN) + .setAuthToken( + InstallationTokenResult.builder() + .setToken(TEST_AUTH_TOKEN) + .setTokenExpirationInSecs(TEST_TOKEN_EXPIRATION_TIMESTAMP) + .build()) + .build(); + + public static final InstallationTokenResult TEST_INSTALLATION_TOKEN_RESULT = + InstallationTokenResult.builder() + .setToken(TEST_AUTH_TOKEN_2) + .setTokenExpirationInSecs(TEST_TOKEN_EXPIRATION_TIMESTAMP) + .build(); } diff --git a/firebase-installations/src/androidTest/java/com/google/firebase/installations/local/PersistedFidTest.java b/firebase-installations/src/androidTest/java/com/google/firebase/installations/local/PersistedFidTest.java index f6aa77f45de..f00b9447bdb 100644 --- a/firebase-installations/src/androidTest/java/com/google/firebase/installations/local/PersistedFidTest.java +++ b/firebase-installations/src/androidTest/java/com/google/firebase/installations/local/PersistedFidTest.java @@ -14,6 +14,7 @@ package com.google.firebase.installations.local; +import static com.google.firebase.installations.FisAndroidTestConstants.DEFAULT_PERSISTED_FID_ENTRY; import static com.google.firebase.installations.FisAndroidTestConstants.TEST_APP_ID_1; import static com.google.firebase.installations.FisAndroidTestConstants.TEST_APP_ID_2; import static com.google.firebase.installations.FisAndroidTestConstants.TEST_AUTH_TOKEN; @@ -23,7 +24,6 @@ import static com.google.firebase.installations.FisAndroidTestConstants.TEST_REFRESH_TOKEN; import static com.google.firebase.installations.FisAndroidTestConstants.TEST_TOKEN_EXPIRATION_TIMESTAMP; import static com.google.firebase.installations.local.PersistedFidEntrySubject.assertThat; -import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import androidx.test.core.app.ApplicationProvider; @@ -69,8 +69,8 @@ public void cleanUp() throws Exception { @Test public void testReadPersistedFidEntry_Null() { - assertNull(persistedFid0.readPersistedFidEntryValue()); - assertNull(persistedFid1.readPersistedFidEntryValue()); + assertThat(persistedFid0.readPersistedFidEntryValue()).isEqualTo(DEFAULT_PERSISTED_FID_ENTRY); + assertThat(persistedFid1.readPersistedFidEntryValue()).isEqualTo(DEFAULT_PERSISTED_FID_ENTRY); } @Test 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 b2390ec8f06..33116195da5 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 @@ -14,13 +14,13 @@ package com.google.firebase.installations; +import androidx.annotation.GuardedBy; import androidx.annotation.NonNull; import androidx.annotation.VisibleForTesting; import com.google.android.gms.common.internal.Preconditions; -import com.google.android.gms.common.util.Clock; import com.google.android.gms.common.util.DefaultClock; -import com.google.android.gms.tasks.Continuation; import com.google.android.gms.tasks.Task; +import com.google.android.gms.tasks.TaskCompletionSource; import com.google.android.gms.tasks.Tasks; import com.google.firebase.FirebaseApp; import com.google.firebase.installations.local.PersistedFid; @@ -29,6 +29,9 @@ import com.google.firebase.installations.remote.FirebaseInstallationServiceClient; import com.google.firebase.installations.remote.FirebaseInstallationServiceException; import com.google.firebase.installations.remote.InstallationResponse; +import java.util.ArrayList; +import java.util.Iterator; +import java.util.List; import java.util.concurrent.ExecutorService; import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.ThreadPoolExecutor; @@ -51,31 +54,28 @@ public class FirebaseInstallations implements FirebaseInstallationsApi { private final FirebaseInstallationServiceClient serviceClient; private final PersistedFid persistedFid; private final ExecutorService executor; - private final Clock clock; private final Utils utils; + private final Object lock = new Object(); - private static final long AUTH_TOKEN_EXPIRATION_BUFFER_IN_SECS = 3600L; // 1 hour - private static final long AWAIT_TIMEOUT_IN_SECS = 10L; + @GuardedBy("lock") + private final List listeners = new ArrayList<>(); /** package private constructor. */ FirebaseInstallations(FirebaseApp firebaseApp) { this( - DefaultClock.getInstance(), new ThreadPoolExecutor(0, 1, 30L, TimeUnit.SECONDS, new LinkedBlockingQueue<>()), firebaseApp, new FirebaseInstallationServiceClient(firebaseApp.getApplicationContext()), new PersistedFid(firebaseApp), - new Utils()); + new Utils(DefaultClock.getInstance())); } FirebaseInstallations( - Clock clock, ExecutorService executor, FirebaseApp firebaseApp, FirebaseInstallationServiceClient serviceClient, PersistedFid persistedFid, Utils utils) { - this.clock = clock; this.firebaseApp = firebaseApp; this.serviceClient = serviceClient; this.executor = executor; @@ -106,6 +106,18 @@ public static FirebaseInstallations getInstance(@NonNull FirebaseApp app) { return (FirebaseInstallations) app.get(FirebaseInstallationsApi.class); } + /** Returns the application id of the {@link FirebaseApp} of this {@link FirebaseInstallations} */ + @VisibleForTesting + String getApplicationId() { + return firebaseApp.getOptions().getApplicationId(); + } + + /** Returns the nick name of the {@link FirebaseApp} of this {@link FirebaseInstallations} */ + @VisibleForTesting + String getName() { + return firebaseApp.getName(); + } + /** * Returns a globally unique identifier of this Firebase app installation. This is a url-safe * base64 string of a 128-bit integer. @@ -113,18 +125,9 @@ public static FirebaseInstallations getInstance(@NonNull FirebaseApp app) { @NonNull @Override public Task getId() { - return getId(null); - } - - /** - * Returns a globally unique identifier of this Firebase app installation.Also, updates the {@link - * AwaitListener} when the FID registration is complete. - */ - private Task getId(AwaitListener awaitListener) { - return Tasks.call(executor, this::getPersistedFid) - .continueWith(orElse(this::createAndPersistNewFid)) - .onSuccessTask( - persistedFidEntry -> registerFidIfNecessary(persistedFidEntry, awaitListener)); + Task task = addGetIdListener(); + executor.execute(doRegistration(DO_NOT_FORCE_REFRESH)); + return task; } /** @@ -140,12 +143,9 @@ private Task getId(AwaitListener awaitListener) { @Override public synchronized Task getAuthToken( @AuthTokenOption int authTokenOption) { - AwaitListener awaitListener = new AwaitListener(); - return getId(awaitListener) - .continueWith( - executor, - awaitFidRegistration( - () -> refreshAuthTokenIfNecessary(authTokenOption), awaitListener)); + Task task = addGetAuthTokenListener(); + executor.execute(doRegistration(authTokenOption)); + return task; } /** @@ -159,66 +159,86 @@ public Task delete() { return Tasks.call(executor, this::deleteFirebaseInstallationId); } - /** Returns the application id of the {@link FirebaseApp} of this {@link FirebaseInstallations} */ - @VisibleForTesting - String getApplicationId() { - return firebaseApp.getOptions().getApplicationId(); - } - - /** Returns the nick name of the {@link FirebaseApp} of this {@link FirebaseInstallations} */ - @VisibleForTesting - String getName() { - return firebaseApp.getName(); - } - - /** - * Returns the {@link PersistedFidEntry} from shared prefs. - * - * @throws {@link FirebaseInstallationsException} when shared pref is empty or {@link - * PersistedFidEntry} is in error state. - */ - private PersistedFidEntry getPersistedFid() throws FirebaseInstallationsException { - PersistedFidEntry persistedFidEntry = persistedFid.readPersistedFidEntryValue(); - if (persistedFidMissingOrInErrorState(persistedFidEntry)) { - throw new FirebaseInstallationsException( - "Failed to get existing fid.", FirebaseInstallationsException.Status.CLIENT_ERROR); + private Task addGetIdListener() { + TaskCompletionSource taskCompletionSource = new TaskCompletionSource<>(); + StateListener l = new GetIdListener(taskCompletionSource); + synchronized (lock) { + listeners.add(l); } - return persistedFidEntry; + return taskCompletionSource.getTask(); } - private static boolean persistedFidMissingOrInErrorState(PersistedFidEntry persistedFidEntry) { - return persistedFidEntry == null - || persistedFidEntry.getRegistrationStatus() == RegistrationStatus.REGISTER_ERROR; + private Task addGetAuthTokenListener() { + TaskCompletionSource taskCompletionSource = + new TaskCompletionSource<>(); + StateListener l = new GetAuthTokenListener(utils, taskCompletionSource); + synchronized (lock) { + listeners.add(l); + } + return taskCompletionSource.getTask(); } - @NonNull - private static Continuation orElse(@NonNull Supplier supplier) { - return t -> { - if (t.isSuccessful()) { - return (T) t.getResult(); + private void triggerOnStateReached(PersistedFidEntry persistedFidEntry) { + synchronized (lock) { + Iterator it = listeners.iterator(); + while (it.hasNext()) { + StateListener l = it.next(); + boolean doneListening = l.onStateReached(persistedFidEntry); + if (doneListening) { + it.remove(); + break; + } } - return supplier.get(); - }; + } } - @NonNull - private static Continuation awaitFidRegistration( - @NonNull Supplier supplier, AwaitListener listener) { - return t -> { - // Waiting for Task that registers FID on the FIS Servers - listener.await(AWAIT_TIMEOUT_IN_SECS, TimeUnit.SECONDS); - return supplier.get(); + private final Runnable doRegistration(int authTokenOption) { + return () -> { + try { + PersistedFidEntry persistedFidEntry = persistedFid.readPersistedFidEntryValue(); + + // New FID needs to be created + if (persistedFidEntry.isErrored() || persistedFidEntry.isNotGenerated()) { + String fid = utils.createRandomFid(); + persistFid(fid); + persistedFidEntry = persistedFid.readPersistedFidEntryValue(); + } + + // GetIdListener will be notified as authTokenOption is always DO_NOT_FORCE_REFRESH. + // GetAuthTokenListener should only be notified if FORCE_REFRESH is not required. + if (authTokenOption != FORCE_REFRESH) { + triggerOnStateReached(persistedFidEntry); + } + + // FID needs to be registered + if (persistedFidEntry.isUnregistered()) { + registerAndSaveFid(persistedFidEntry); + persistedFidEntry = persistedFid.readPersistedFidEntryValue(); + } + + // Don't notify the listeners at this point; we might as well make ure the auth token is up + // to date before letting them know. + + // Refresh Auth token if needed + if (authTokenOption == FORCE_REFRESH || utils.isAuthTokenExpired(persistedFidEntry)) { + fetchAuthTokenFromServer(persistedFidEntry); + persistedFidEntry = persistedFid.readPersistedFidEntryValue(); + } + + triggerOnStateReached(persistedFidEntry); + } catch (Exception e) { + PersistedFidEntry persistedFidEntry = persistedFid.readPersistedFidEntryValue(); + PersistedFidEntry errorFidEntry = + persistedFidEntry + .toBuilder() + .setRegistrationStatus(RegistrationStatus.REGISTER_ERROR) + .build(); + persistedFid.insertOrUpdatePersistedFidEntry(errorFidEntry); + triggerOnStateReached(errorFidEntry); + } }; } - /** Creates a random FID and persists it in the shared prefs with UNREGISTERED status. */ - private PersistedFidEntry createAndPersistNewFid() throws FirebaseInstallationsException { - String fid = utils.createRandomFid(); - persistFid(fid); - PersistedFidEntry persistedFidEntry = persistedFid.readPersistedFidEntryValue(); - return persistedFidEntry; - } - private void persistFid(String fid) throws FirebaseInstallationsException { boolean firstUpdateCacheResult = persistedFid.insertOrUpdatePersistedFidEntry( @@ -234,58 +254,11 @@ private void persistFid(String fid) throws FirebaseInstallationsException { } } - /** - * Registers the FID with FIS servers if FID is in UNREGISTERED state. - * - *

Updates FID registration status to PENDING to avoid multiple network calls to FIS Servers. - */ - private Task registerFidIfNecessary( - PersistedFidEntry persistedFidEntry, AwaitListener listener) { - String fid = persistedFidEntry.getFirebaseInstallationId(); - - // Check if the fid is unregistered - if (persistedFidEntry.getRegistrationStatus() == RegistrationStatus.UNREGISTERED) { - updatePersistedFidWithPendingStatus(fid); - executeFidRegistration(persistedFidEntry, listener); - } else { - updateAwaitListenerIfRegisteredFid(persistedFidEntry, listener); - } - - return Tasks.forResult(fid); - } - - private void updateAwaitListenerIfRegisteredFid( - PersistedFidEntry persistedFidEntry, AwaitListener listener) { - if (listener != null - && persistedFidEntry.getRegistrationStatus() == RegistrationStatus.REGISTERED) { - listener.onSuccess(); - } - } - - /** - * Registers the FID with FIS servers in a background thread and updates the listener on - * completion. - */ - private void executeFidRegistration(PersistedFidEntry persistedFidEntry, AwaitListener listener) { - Task task = Tasks.call(executor, () -> registerAndSaveFid(persistedFidEntry)); - if (listener != null) { - task.addOnCompleteListener(listener); - } - } - - private void updatePersistedFidWithPendingStatus(String fid) { - persistedFid.insertOrUpdatePersistedFidEntry( - PersistedFidEntry.builder() - .setFirebaseInstallationId(fid) - .setRegistrationStatus(RegistrationStatus.PENDING) - .build()); - } - /** Registers the created Fid with FIS servers and update the shared prefs. */ private Void registerAndSaveFid(PersistedFidEntry persistedFidEntry) throws FirebaseInstallationsException { try { - long creationTime = currentTimeInSecs(); + long creationTime = utils.currentTimeInSecs(); InstallationResponse installationResponse = serviceClient.createFirebaseInstallation( @@ -304,65 +277,17 @@ private Void registerAndSaveFid(PersistedFidEntry persistedFidEntry) .build()); } catch (FirebaseInstallationServiceException exception) { - persistedFid.insertOrUpdatePersistedFidEntry( - PersistedFidEntry.builder() - .setFirebaseInstallationId(persistedFidEntry.getFirebaseInstallationId()) - .setRegistrationStatus(RegistrationStatus.REGISTER_ERROR) - .build()); throw new FirebaseInstallationsException( exception.getMessage(), FirebaseInstallationsException.Status.SDK_INTERNAL_ERROR); } return null; } - private InstallationTokenResult refreshAuthTokenIfNecessary(int authTokenOption) - throws FirebaseInstallationsException { - - PersistedFidEntry persistedFidEntry = persistedFid.readPersistedFidEntryValue(); - - if (!isPersistedFidRegistered(persistedFidEntry)) { - throw new FirebaseInstallationsException( - "Firebase Installation is not registered.", - FirebaseInstallationsException.Status.SDK_INTERNAL_ERROR); - } - - switch (authTokenOption) { - case FORCE_REFRESH: - return fetchAuthTokenFromServer(persistedFidEntry); - case DO_NOT_FORCE_REFRESH: - return getValidAuthToken(persistedFidEntry); - default: - throw new FirebaseInstallationsException( - "Incorrect refreshAuthTokenOption.", - FirebaseInstallationsException.Status.SDK_INTERNAL_ERROR); - } - } - - /** - * Returns a {@link InstallationTokenResult} created from the {@link PersistedFidEntry} if the - * auth token is valid else generates a new auth token by calling the FIS servers. - */ - private InstallationTokenResult getValidAuthToken(PersistedFidEntry persistedFidEntry) - throws FirebaseInstallationsException { - - return isAuthTokenExpired(persistedFidEntry) - ? fetchAuthTokenFromServer(persistedFidEntry) - : InstallationTokenResult.builder() - .setToken(persistedFidEntry.getAuthToken()) - .setTokenExpirationInSecs(persistedFidEntry.getExpiresInSecs()) - .build(); - } - - private boolean isPersistedFidRegistered(PersistedFidEntry persistedFidEntry) { - return persistedFidEntry != null - && persistedFidEntry.getRegistrationStatus() == RegistrationStatus.REGISTERED; - } - /** Calls the FIS servers to generate an auth token for this Firebase installation. */ private InstallationTokenResult fetchAuthTokenFromServer(PersistedFidEntry persistedFidEntry) throws FirebaseInstallationsException { try { - long creationTime = currentTimeInSecs(); + long creationTime = utils.currentTimeInSecs(); InstallationTokenResult tokenResult = serviceClient.generateAuthToken( /*apiKey= */ firebaseApp.getOptions().getApiKey(), @@ -388,19 +313,6 @@ private InstallationTokenResult fetchAuthTokenFromServer(PersistedFidEntry persi } } - /** - * Checks if the FIS Auth token is expired or going to expire in next 1 hour - * (AUTH_TOKEN_EXPIRATION_BUFFER_IN_SECS). - */ - private boolean isAuthTokenExpired(PersistedFidEntry persistedFidEntry) { - return (persistedFidEntry.getTokenCreationEpochInSecs() + persistedFidEntry.getExpiresInSecs() - > currentTimeInSecs() + AUTH_TOKEN_EXPIRATION_BUFFER_IN_SECS); - } - - private long currentTimeInSecs() { - return TimeUnit.MILLISECONDS.toSeconds(clock.currentTimeMillis()); - } - /** * Deletes the firebase installation id of the {@link FirebaseApp} from FIS servers and local * storage. @@ -409,8 +321,8 @@ private Void deleteFirebaseInstallationId() throws FirebaseInstallationsExceptio PersistedFidEntry persistedFidEntry = persistedFid.readPersistedFidEntryValue(); - if (isPersistedFidRegistered(persistedFidEntry)) { - // Call the FIS servers to delete this firebase installation id. + if (persistedFidEntry.isRegistered()) { + // Call the FIS servers to delete this Firebase Installation Id. try { serviceClient.deleteFirebaseInstallation( firebaseApp.getOptions().getApiKey(), @@ -429,7 +341,3 @@ private Void deleteFirebaseInstallationId() throws FirebaseInstallationsExceptio return null; } } - -interface Supplier { - T get() throws Exception; -} diff --git a/firebase-installations/src/main/java/com/google/firebase/installations/StateListener.java b/firebase-installations/src/main/java/com/google/firebase/installations/StateListener.java new file mode 100644 index 00000000000..98364c1495d --- /dev/null +++ b/firebase-installations/src/main/java/com/google/firebase/installations/StateListener.java @@ -0,0 +1,79 @@ +// 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.PersistedFidEntry; + +interface StateListener { + boolean onStateReached(PersistedFidEntry persistedFidEntry); +} + +class GetIdListener implements StateListener { + final TaskCompletionSource taskCompletionSource; + + public GetIdListener(TaskCompletionSource taskCompletionSource) { + this.taskCompletionSource = taskCompletionSource; + } + + @Override + public boolean onStateReached(PersistedFidEntry persistedFidEntry) { + if (persistedFidEntry.isUnregistered() || persistedFidEntry.isRegistered()) { + taskCompletionSource.setResult(persistedFidEntry.getFirebaseInstallationId()); + return true; + } + + if (persistedFidEntry.isErrored()) { + taskCompletionSource.setException( + new FirebaseInstallationsException( + "Failed to update client side cache.", + FirebaseInstallationsException.Status.CLIENT_ERROR)); + return true; + } + return false; + } +} + +class GetAuthTokenListener implements StateListener { + private final Utils utils; + private final TaskCompletionSource resultTaskCompletionSource; + + public GetAuthTokenListener( + Utils utils, TaskCompletionSource resultTaskCompletionSource) { + this.utils = utils; + this.resultTaskCompletionSource = resultTaskCompletionSource; + } + + @Override + public boolean onStateReached(PersistedFidEntry persistedFidEntry) { + if (persistedFidEntry.isRegistered() && !utils.isAuthTokenExpired(persistedFidEntry)) { + resultTaskCompletionSource.setResult( + InstallationTokenResult.builder() + .setToken(persistedFidEntry.getAuthToken()) + .setTokenExpirationInSecs(persistedFidEntry.getExpiresInSecs()) + .build()); + return true; + } + + if (persistedFidEntry.isErrored()) { + resultTaskCompletionSource.setException( + new FirebaseInstallationsException( + "Firebase Installation is not registered.", + FirebaseInstallationsException.Status.SDK_INTERNAL_ERROR)); + return true; + } + return false; + } +} diff --git a/firebase-installations/src/main/java/com/google/firebase/installations/Utils.java b/firebase-installations/src/main/java/com/google/firebase/installations/Utils.java index 1ad7efab2f2..0c2b4da6a14 100644 --- a/firebase-installations/src/main/java/com/google/firebase/installations/Utils.java +++ b/firebase-installations/src/main/java/com/google/firebase/installations/Utils.java @@ -15,13 +15,17 @@ package com.google.firebase.installations; import androidx.annotation.NonNull; +import com.google.android.gms.common.util.Clock; +import com.google.firebase.installations.local.PersistedFidEntry; import java.nio.ByteBuffer; import java.nio.charset.Charset; import java.util.UUID; +import java.util.concurrent.TimeUnit; /** Util methods used for {@link FirebaseInstallations} */ class Utils { + private final Clock clock; /** * 1 Byte with the first 4 header-bits set to the identifying FID prefix 0111 (0x7). Use this * constant to create FIDs or check the first byte of FIDs. This prefix is also used in legacy @@ -38,6 +42,26 @@ class Utils { /** Length of new-format FIDs as introduced in 2019. */ public static final int FID_LENGTH = 22; + private static final long AUTH_TOKEN_EXPIRATION_BUFFER_IN_SECS = 3600L; // 1 hour + + Utils(Clock clock) { + this.clock = clock; + } + + /** + * Checks if the FIS Auth token is expired or going to expire in next 1 hour + * (AUTH_TOKEN_EXPIRATION_BUFFER_IN_SECS). + */ + public boolean isAuthTokenExpired(PersistedFidEntry persistedFidEntry) { + return (persistedFidEntry.getTokenCreationEpochInSecs() + persistedFidEntry.getExpiresInSecs() + > currentTimeInSecs() + AUTH_TOKEN_EXPIRATION_BUFFER_IN_SECS); + } + + /** Returns current time in seconds. */ + public long currentTimeInSecs() { + return TimeUnit.MILLISECONDS.toSeconds(clock.currentTimeMillis()); + } + /** * Creates a random FID of valid format without checking if the FID is already in use by any * Firebase Installation. diff --git a/firebase-installations/src/main/java/com/google/firebase/installations/local/PersistedFid.java b/firebase-installations/src/main/java/com/google/firebase/installations/local/PersistedFid.java index 1da104fa350..808b277c467 100644 --- a/firebase-installations/src/main/java/com/google/firebase/installations/local/PersistedFid.java +++ b/firebase-installations/src/main/java/com/google/firebase/installations/local/PersistedFid.java @@ -16,8 +16,8 @@ import android.content.Context; import android.content.SharedPreferences; +import androidx.annotation.GuardedBy; import androidx.annotation.NonNull; -import androidx.annotation.Nullable; import com.google.firebase.FirebaseApp; import java.util.Arrays; import java.util.List; @@ -31,14 +31,28 @@ public class PersistedFid { // NOTE: never change the ordinal of the enum values because the enum values are stored in shared // prefs as their ordinal numbers. public enum RegistrationStatus { - /** {@link PersistedFidEntry} is synced to FIS servers */ - REGISTERED, - /** {@link PersistedFidEntry} is not synced with FIS server */ + /** + * {@link PersistedFidEntry} default registration status. Next state: UNREGISTERED - A new FID + * is created and persisted locally before registering with FIS servers. + */ + NOT_GENERATED, + /** + * {@link PersistedFidEntry} is not synced with FIS servers. Next state: REGISTERED - If FID + * registration is successful. REGISTER_ERROR - If FID registration or refresh auth token + * failed. + */ UNREGISTERED, - /** {@link PersistedFidEntry} is in error state when syncing with FIS server */ + /** + * {@link PersistedFidEntry} is synced to FIS servers. Next state: REGISTER_ERROR - If FID + * registration or refresh auth token failed. + */ + REGISTERED, + /** + * {@link PersistedFidEntry} is in error state when an exception is thrown while syncing with + * FIS server. Next state: UNREGISTERED - A new FID is created and persisted locally before + * registering with FIS servers. + */ REGISTER_ERROR, - /** {@link PersistedFidEntry} is in pending state when waiting for FIS server response */ - PENDING } private static final String SHARED_PREFS_NAME = "PersistedFid"; @@ -59,7 +73,9 @@ public enum RegistrationStatus { EXPIRES_IN_SECONDS_KEY, PERSISTED_STATUS_KEY); + @GuardedBy("prefs") private final SharedPreferences prefs; + private final String persistenceKey; public PersistedFid(@NonNull FirebaseApp firebaseApp) { @@ -72,30 +88,29 @@ public PersistedFid(@NonNull FirebaseApp firebaseApp) { persistenceKey = firebaseApp.getPersistenceKey(); } - @Nullable + @NonNull public PersistedFidEntry readPersistedFidEntryValue() { - String fid = prefs.getString(getSharedPreferencesKey(FIREBASE_INSTALLATION_ID_KEY), null); - int status = prefs.getInt(getSharedPreferencesKey(PERSISTED_STATUS_KEY), -1); - String authToken = prefs.getString(getSharedPreferencesKey(AUTH_TOKEN_KEY), null); - String refreshToken = prefs.getString(getSharedPreferencesKey(REFRESH_TOKEN_KEY), null); - long tokenCreationTime = - prefs.getLong(getSharedPreferencesKey(TOKEN_CREATION_TIME_IN_SECONDS_KEY), 0); - long expiresIn = prefs.getLong(getSharedPreferencesKey(EXPIRES_IN_SECONDS_KEY), 0); + synchronized (prefs) { + String fid = prefs.getString(getSharedPreferencesKey(FIREBASE_INSTALLATION_ID_KEY), null); + int status = prefs.getInt(getSharedPreferencesKey(PERSISTED_STATUS_KEY), -1); + String authToken = prefs.getString(getSharedPreferencesKey(AUTH_TOKEN_KEY), null); + String refreshToken = prefs.getString(getSharedPreferencesKey(REFRESH_TOKEN_KEY), null); + long tokenCreationTime = + prefs.getLong(getSharedPreferencesKey(TOKEN_CREATION_TIME_IN_SECONDS_KEY), 0); + long expiresIn = prefs.getLong(getSharedPreferencesKey(EXPIRES_IN_SECONDS_KEY), 0); - if (fid == null - || status == -1 - || !(status >= 0 && status < RegistrationStatus.values().length)) { - return null; + if (fid == null || !(status >= 0 && status < RegistrationStatus.values().length)) { + return PersistedFidEntry.builder().build(); + } + return PersistedFidEntry.builder() + .setFirebaseInstallationId(fid) + .setRegistrationStatus(RegistrationStatus.values()[status]) + .setAuthToken(authToken) + .setRefreshToken(refreshToken) + .setTokenCreationEpochInSecs(tokenCreationTime) + .setExpiresInSecs(expiresIn) + .build(); } - - return PersistedFidEntry.builder() - .setFirebaseInstallationId(fid) - .setRegistrationStatus(RegistrationStatus.values()[status]) - .setAuthToken(authToken) - .setRefreshToken(refreshToken) - .setTokenCreationEpochInSecs(tokenCreationTime) - .setExpiresInSecs(expiresIn) - .build(); } @NonNull diff --git a/firebase-installations/src/main/java/com/google/firebase/installations/local/PersistedFidEntry.java b/firebase-installations/src/main/java/com/google/firebase/installations/local/PersistedFidEntry.java index 4c8a9a56ec4..4019967cdb6 100644 --- a/firebase-installations/src/main/java/com/google/firebase/installations/local/PersistedFidEntry.java +++ b/firebase-installations/src/main/java/com/google/firebase/installations/local/PersistedFidEntry.java @@ -25,7 +25,7 @@ @AutoValue public abstract class PersistedFidEntry { - @NonNull + @Nullable public abstract String getFirebaseInstallationId(); @NonNull @@ -41,6 +41,22 @@ public abstract class PersistedFidEntry { public abstract long getTokenCreationEpochInSecs(); + public boolean isRegistered() { + return getRegistrationStatus() == PersistedFid.RegistrationStatus.REGISTERED; + } + + public boolean isErrored() { + return getRegistrationStatus() == PersistedFid.RegistrationStatus.REGISTER_ERROR; + } + + public boolean isUnregistered() { + return getRegistrationStatus() == PersistedFid.RegistrationStatus.UNREGISTERED; + } + + public boolean isNotGenerated() { + return getRegistrationStatus() == PersistedFid.RegistrationStatus.NOT_GENERATED; + } + @NonNull public abstract Builder toBuilder(); @@ -49,6 +65,7 @@ public abstract class PersistedFidEntry { public static PersistedFidEntry.Builder builder() { return new AutoValue_PersistedFidEntry.Builder() .setTokenCreationEpochInSecs(0) + .setRegistrationStatus(PersistedFid.RegistrationStatus.NOT_GENERATED) .setExpiresInSecs(0); } From a987647c020f3ded60b86105d37604e6c84b7ee1 Mon Sep 17 00:00:00 2001 From: Ankita Jhawar Date: Wed, 25 Sep 2019 14:06:15 -0700 Subject: [PATCH 02/15] Splitting multiple classes/interfaces into separate files. --- .../installations/GetAuthTokenListener.java | 50 ++++++++++++++++ .../firebase/installations/GetIdListener.java | 43 ++++++++++++++ .../firebase/installations/StateListener.java | 58 ------------------- 3 files changed, 93 insertions(+), 58 deletions(-) create mode 100644 firebase-installations/src/main/java/com/google/firebase/installations/GetAuthTokenListener.java 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/GetAuthTokenListener.java b/firebase-installations/src/main/java/com/google/firebase/installations/GetAuthTokenListener.java new file mode 100644 index 00000000000..2e16e8cbb42 --- /dev/null +++ b/firebase-installations/src/main/java/com/google/firebase/installations/GetAuthTokenListener.java @@ -0,0 +1,50 @@ +// 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.PersistedFidEntry; + +class GetAuthTokenListener implements StateListener { + private final Utils utils; + private final TaskCompletionSource resultTaskCompletionSource; + + public GetAuthTokenListener( + Utils utils, TaskCompletionSource resultTaskCompletionSource) { + this.utils = utils; + this.resultTaskCompletionSource = resultTaskCompletionSource; + } + + @Override + public boolean onStateReached(PersistedFidEntry persistedFidEntry) { + if (persistedFidEntry.isRegistered() && !utils.isAuthTokenExpired(persistedFidEntry)) { + resultTaskCompletionSource.setResult( + InstallationTokenResult.builder() + .setToken(persistedFidEntry.getAuthToken()) + .setTokenExpirationInSecs(persistedFidEntry.getExpiresInSecs()) + .build()); + return true; + } + + if (persistedFidEntry.isErrored()) { + resultTaskCompletionSource.setException( + new FirebaseInstallationsException( + "Firebase Installation is not registered.", + FirebaseInstallationsException.Status.SDK_INTERNAL_ERROR)); + return true; + } + return false; + } +} 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..cee158348e3 --- /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.PersistedFidEntry; + +class GetIdListener implements StateListener { + final TaskCompletionSource taskCompletionSource; + + public GetIdListener(TaskCompletionSource taskCompletionSource) { + this.taskCompletionSource = taskCompletionSource; + } + + @Override + public boolean onStateReached(PersistedFidEntry persistedFidEntry) { + if (persistedFidEntry.isUnregistered() || persistedFidEntry.isRegistered()) { + taskCompletionSource.setResult(persistedFidEntry.getFirebaseInstallationId()); + return true; + } + + if (persistedFidEntry.isErrored()) { + taskCompletionSource.setException( + new FirebaseInstallationsException( + "Failed to update client side cache.", + FirebaseInstallationsException.Status.CLIENT_ERROR)); + return true; + } + return false; + } +} diff --git a/firebase-installations/src/main/java/com/google/firebase/installations/StateListener.java b/firebase-installations/src/main/java/com/google/firebase/installations/StateListener.java index 98364c1495d..45d1e5e219a 100644 --- a/firebase-installations/src/main/java/com/google/firebase/installations/StateListener.java +++ b/firebase-installations/src/main/java/com/google/firebase/installations/StateListener.java @@ -14,66 +14,8 @@ package com.google.firebase.installations; -import com.google.android.gms.tasks.TaskCompletionSource; import com.google.firebase.installations.local.PersistedFidEntry; interface StateListener { boolean onStateReached(PersistedFidEntry persistedFidEntry); } - -class GetIdListener implements StateListener { - final TaskCompletionSource taskCompletionSource; - - public GetIdListener(TaskCompletionSource taskCompletionSource) { - this.taskCompletionSource = taskCompletionSource; - } - - @Override - public boolean onStateReached(PersistedFidEntry persistedFidEntry) { - if (persistedFidEntry.isUnregistered() || persistedFidEntry.isRegistered()) { - taskCompletionSource.setResult(persistedFidEntry.getFirebaseInstallationId()); - return true; - } - - if (persistedFidEntry.isErrored()) { - taskCompletionSource.setException( - new FirebaseInstallationsException( - "Failed to update client side cache.", - FirebaseInstallationsException.Status.CLIENT_ERROR)); - return true; - } - return false; - } -} - -class GetAuthTokenListener implements StateListener { - private final Utils utils; - private final TaskCompletionSource resultTaskCompletionSource; - - public GetAuthTokenListener( - Utils utils, TaskCompletionSource resultTaskCompletionSource) { - this.utils = utils; - this.resultTaskCompletionSource = resultTaskCompletionSource; - } - - @Override - public boolean onStateReached(PersistedFidEntry persistedFidEntry) { - if (persistedFidEntry.isRegistered() && !utils.isAuthTokenExpired(persistedFidEntry)) { - resultTaskCompletionSource.setResult( - InstallationTokenResult.builder() - .setToken(persistedFidEntry.getAuthToken()) - .setTokenExpirationInSecs(persistedFidEntry.getExpiresInSecs()) - .build()); - return true; - } - - if (persistedFidEntry.isErrored()) { - resultTaskCompletionSource.setException( - new FirebaseInstallationsException( - "Firebase Installation is not registered.", - FirebaseInstallationsException.Status.SDK_INTERNAL_ERROR)); - return true; - } - return false; - } -} From 1d9b1ca2957f86b10c8627589fbebcc3f0aaa4e1 Mon Sep 17 00:00:00 2001 From: Ankita Jhawar Date: Thu, 26 Sep 2019 14:24:55 -0700 Subject: [PATCH 03/15] Addressing ciaran's comments to return same token if multiple getAuthToken() calls are triggered simultaneously. --- ...FirebaseInstallationsInstrumentedTest.java | 14 +++++---- .../installations/FirebaseInstallations.java | 31 +++++++++++-------- .../installations/GetAuthTokenListener.java | 20 ++++++++++-- 3 files changed, 44 insertions(+), 21 deletions(-) diff --git a/firebase-installations/src/androidTest/java/com/google/firebase/installations/FirebaseInstallationsInstrumentedTest.java b/firebase-installations/src/androidTest/java/com/google/firebase/installations/FirebaseInstallationsInstrumentedTest.java index 8b6b4b39187..f5750314a10 100644 --- a/firebase-installations/src/androidTest/java/com/google/firebase/installations/FirebaseInstallationsInstrumentedTest.java +++ b/firebase-installations/src/androidTest/java/com/google/firebase/installations/FirebaseInstallationsInstrumentedTest.java @@ -149,16 +149,16 @@ public void setUp() throws FirebaseInstallationServiceException { anyString(), anyString(), anyString(), anyString())) .thenReturn(TEST_INSTALLATION_TOKEN_RESULT); + when(persistedFidReturnsError.insertOrUpdatePersistedFidEntry(any())).thenReturn(false); + when(persistedFidReturnsError.readPersistedFidEntryValue()) + .thenReturn(DEFAULT_PERSISTED_FID_ENTRY); + when(backendClientReturnsError.createFirebaseInstallation( anyString(), anyString(), anyString(), anyString())) .thenThrow( new FirebaseInstallationServiceException( "SDK Error", FirebaseInstallationServiceException.Status.SERVER_ERROR)); - when(persistedFidReturnsError.insertOrUpdatePersistedFidEntry(any())).thenReturn(false); - when(persistedFidReturnsError.readPersistedFidEntryValue()) - .thenReturn(DEFAULT_PERSISTED_FID_ENTRY); - when(mockUtils.createRandomFid()).thenReturn(TEST_FID_1); when(mockUtils.currentTimeInSecs()).thenReturn(TEST_CREATION_TIMESTAMP_2); when(mockUtils.isAuthTokenExpired(any())).thenReturn(false); @@ -541,9 +541,11 @@ public void testGetAuthToken_multipleCallsForceRefresh_fetchedNewTokenTwice() th .isEqualTo(TEST_AUTH_TOKEN_3); assertWithMessage("Persisted Auth Token doesn't match") .that(task2.getResult().getToken()) - .isEqualTo(TEST_AUTH_TOKEN_4); - verify(backendClientReturnsOk, times(2)) + .isEqualTo(TEST_AUTH_TOKEN_3); + verify(backendClientReturnsOk, times(1)) .generateAuthToken(TEST_API_KEY, TEST_FID_1, TEST_PROJECT_ID, TEST_REFRESH_TOKEN); + PersistedFidEntry updatedFidEntry = persistedFid.readPersistedFidEntryValue(); + assertThat(updatedFidEntry).hasAuthToken(TEST_AUTH_TOKEN_3); } @Test 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 33116195da5..7e96b3a24b8 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 @@ -143,7 +143,7 @@ public Task getId() { @Override public synchronized Task getAuthToken( @AuthTokenOption int authTokenOption) { - Task task = addGetAuthTokenListener(); + Task task = addGetAuthTokenListener(authTokenOption); executor.execute(doRegistration(authTokenOption)); return task; } @@ -168,10 +168,15 @@ private Task addGetIdListener() { return taskCompletionSource.getTask(); } - private Task addGetAuthTokenListener() { + private Task addGetAuthTokenListener(int authTokenOption) { TaskCompletionSource taskCompletionSource = new TaskCompletionSource<>(); - StateListener l = new GetAuthTokenListener(utils, taskCompletionSource); + String oldAuthToken = + persistedFid.readPersistedFidEntryValue().getAuthToken() == null + ? "" + : persistedFid.readPersistedFidEntryValue().getAuthToken(); + StateListener l = + new GetAuthTokenListener(utils, taskCompletionSource, oldAuthToken, authTokenOption); synchronized (lock) { listeners.add(l); } @@ -186,7 +191,6 @@ private void triggerOnStateReached(PersistedFidEntry persistedFidEntry) { boolean doneListening = l.onStateReached(persistedFidEntry); if (doneListening) { it.remove(); - break; } } } @@ -204,11 +208,7 @@ private final Runnable doRegistration(int authTokenOption) { persistedFidEntry = persistedFid.readPersistedFidEntryValue(); } - // GetIdListener will be notified as authTokenOption is always DO_NOT_FORCE_REFRESH. - // GetAuthTokenListener should only be notified if FORCE_REFRESH is not required. - if (authTokenOption != FORCE_REFRESH) { - triggerOnStateReached(persistedFidEntry); - } + triggerOnStateReached(persistedFidEntry); // FID needs to be registered if (persistedFidEntry.isUnregistered()) { @@ -219,10 +219,15 @@ private final Runnable doRegistration(int authTokenOption) { // Don't notify the listeners at this point; we might as well make ure the auth token is up // to date before letting them know. - // Refresh Auth token if needed - if (authTokenOption == FORCE_REFRESH || utils.isAuthTokenExpired(persistedFidEntry)) { - fetchAuthTokenFromServer(persistedFidEntry); - persistedFidEntry = persistedFid.readPersistedFidEntryValue(); + // Refresh Auth token if: + // 1. Invalid auth token + // 2. If FORCE_REFRESH with a pending listener + synchronized (lock) { + if ((authTokenOption == FORCE_REFRESH && !listeners.isEmpty()) + || utils.isAuthTokenExpired(persistedFidEntry)) { + fetchAuthTokenFromServer(persistedFidEntry); + persistedFidEntry = persistedFid.readPersistedFidEntryValue(); + } } triggerOnStateReached(persistedFidEntry); diff --git a/firebase-installations/src/main/java/com/google/firebase/installations/GetAuthTokenListener.java b/firebase-installations/src/main/java/com/google/firebase/installations/GetAuthTokenListener.java index 2e16e8cbb42..1038ce02ac2 100644 --- a/firebase-installations/src/main/java/com/google/firebase/installations/GetAuthTokenListener.java +++ b/firebase-installations/src/main/java/com/google/firebase/installations/GetAuthTokenListener.java @@ -15,21 +15,37 @@ package com.google.firebase.installations; import com.google.android.gms.tasks.TaskCompletionSource; +import com.google.firebase.installations.FirebaseInstallationsApi.AuthTokenOption; import com.google.firebase.installations.local.PersistedFidEntry; class GetAuthTokenListener implements StateListener { private final Utils utils; private final TaskCompletionSource resultTaskCompletionSource; + private final String oldAuthToken; + @AuthTokenOption private final int authTokenOption; public GetAuthTokenListener( - Utils utils, TaskCompletionSource resultTaskCompletionSource) { + Utils utils, + TaskCompletionSource resultTaskCompletionSource, + String oldAuthToken, + @AuthTokenOption int authTokenOption) { this.utils = utils; this.resultTaskCompletionSource = resultTaskCompletionSource; + this.oldAuthToken = oldAuthToken; + this.authTokenOption = authTokenOption; } @Override public boolean onStateReached(PersistedFidEntry persistedFidEntry) { - if (persistedFidEntry.isRegistered() && !utils.isAuthTokenExpired(persistedFidEntry)) { + // AuthTokenListener state is reached when: + // 1. If AuthTokenOption is DO_NOT_FORCE_REFRESH : FID is registered and has a valid auth token + // 2. If AuthTokenOption is FORCE_REFRESH : FID is registered, has a valid new auth token which + // is not same as the old auth token + + if (persistedFidEntry.isRegistered() + && !utils.isAuthTokenExpired(persistedFidEntry) + && (authTokenOption == FirebaseInstallationsApi.DO_NOT_FORCE_REFRESH + || !oldAuthToken.equals(persistedFidEntry.getAuthToken()))) { resultTaskCompletionSource.setResult( InstallationTokenResult.builder() .setToken(persistedFidEntry.getAuthToken()) From 28d0d85e67669e90bb848aadbb4a384f71d29b8c Mon Sep 17 00:00:00 2001 From: Ankita Jhawar Date: Fri, 27 Sep 2019 14:38:33 -0700 Subject: [PATCH 04/15] Addressing ciaran's comments to use a boolean to refresh auth token --- ...FirebaseInstallationsInstrumentedTest.java | 4 +- .../installations/FirebaseInstallations.java | 41 +++++++++---------- .../installations/GetAuthTokenListener.java | 21 ++-------- 3 files changed, 25 insertions(+), 41 deletions(-) diff --git a/firebase-installations/src/androidTest/java/com/google/firebase/installations/FirebaseInstallationsInstrumentedTest.java b/firebase-installations/src/androidTest/java/com/google/firebase/installations/FirebaseInstallationsInstrumentedTest.java index f5750314a10..95b3359316a 100644 --- a/firebase-installations/src/androidTest/java/com/google/firebase/installations/FirebaseInstallationsInstrumentedTest.java +++ b/firebase-installations/src/androidTest/java/com/google/firebase/installations/FirebaseInstallationsInstrumentedTest.java @@ -341,9 +341,9 @@ public void testGetId_expiredAuthToken_refreshesAuthToken() throws Exception { // Waiting for Task that registers FID on the FIS Servers executor.awaitTermination(500, TimeUnit.MILLISECONDS); - // Validate that registration is complete with updated auth token + // Validate that registration is complete with a refreshed auth token PersistedFidEntry updatedFidEntry = persistedFid.readPersistedFidEntryValue(); - assertThat(updatedFidEntry).isEqualTo(UPDATED_AUTH_TOKEN_FID_ENTRY); + assertThat(updatedFidEntry).hasAuthToken(TEST_AUTH_TOKEN_2); verify(backendClientReturnsOk, never()) .createFirebaseInstallation(TEST_API_KEY, TEST_FID_1, TEST_PROJECT_ID, TEST_APP_ID_1); verify(backendClientReturnsOk, times(1)) 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 7e96b3a24b8..5109523815b 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 @@ -57,6 +57,8 @@ public class FirebaseInstallations implements FirebaseInstallationsApi { private final Utils utils; private final Object lock = new Object(); + private boolean shouldRefreshAuthToken; + @GuardedBy("lock") private final List listeners = new ArrayList<>(); @@ -126,7 +128,7 @@ String getName() { @Override public Task getId() { Task task = addGetIdListener(); - executor.execute(doRegistration(DO_NOT_FORCE_REFRESH)); + executor.execute(doRegistration()); return task; } @@ -143,8 +145,11 @@ public Task getId() { @Override public synchronized Task getAuthToken( @AuthTokenOption int authTokenOption) { - Task task = addGetAuthTokenListener(authTokenOption); - executor.execute(doRegistration(authTokenOption)); + if (authTokenOption == FORCE_REFRESH) { + shouldRefreshAuthToken = true; + } + Task task = addGetAuthTokenListener(); + executor.execute(doRegistration()); return task; } @@ -168,15 +173,10 @@ private Task addGetIdListener() { return taskCompletionSource.getTask(); } - private Task addGetAuthTokenListener(int authTokenOption) { + private Task addGetAuthTokenListener() { TaskCompletionSource taskCompletionSource = new TaskCompletionSource<>(); - String oldAuthToken = - persistedFid.readPersistedFidEntryValue().getAuthToken() == null - ? "" - : persistedFid.readPersistedFidEntryValue().getAuthToken(); - StateListener l = - new GetAuthTokenListener(utils, taskCompletionSource, oldAuthToken, authTokenOption); + StateListener l = new GetAuthTokenListener(utils, taskCompletionSource); synchronized (lock) { listeners.add(l); } @@ -196,7 +196,7 @@ private void triggerOnStateReached(PersistedFidEntry persistedFidEntry) { } } - private final Runnable doRegistration(int authTokenOption) { + private final Runnable doRegistration() { return () -> { try { PersistedFidEntry persistedFidEntry = persistedFid.readPersistedFidEntryValue(); @@ -208,7 +208,10 @@ private final Runnable doRegistration(int authTokenOption) { persistedFidEntry = persistedFid.readPersistedFidEntryValue(); } - triggerOnStateReached(persistedFidEntry); + // If Auth Token needs refresh, notify the listeners only after force refresh + if (!shouldRefreshAuthToken) { + triggerOnStateReached(persistedFidEntry); + } // FID needs to be registered if (persistedFidEntry.isUnregistered()) { @@ -219,15 +222,11 @@ private final Runnable doRegistration(int authTokenOption) { // Don't notify the listeners at this point; we might as well make ure the auth token is up // to date before letting them know. - // Refresh Auth token if: - // 1. Invalid auth token - // 2. If FORCE_REFRESH with a pending listener - synchronized (lock) { - if ((authTokenOption == FORCE_REFRESH && !listeners.isEmpty()) - || utils.isAuthTokenExpired(persistedFidEntry)) { - fetchAuthTokenFromServer(persistedFidEntry); - persistedFidEntry = persistedFid.readPersistedFidEntryValue(); - } + // Refresh Auth token if needed + if (shouldRefreshAuthToken || utils.isAuthTokenExpired(persistedFidEntry)) { + shouldRefreshAuthToken = false; + fetchAuthTokenFromServer(persistedFidEntry); + persistedFidEntry = persistedFid.readPersistedFidEntryValue(); } triggerOnStateReached(persistedFidEntry); diff --git a/firebase-installations/src/main/java/com/google/firebase/installations/GetAuthTokenListener.java b/firebase-installations/src/main/java/com/google/firebase/installations/GetAuthTokenListener.java index 1038ce02ac2..57760e9adb3 100644 --- a/firebase-installations/src/main/java/com/google/firebase/installations/GetAuthTokenListener.java +++ b/firebase-installations/src/main/java/com/google/firebase/installations/GetAuthTokenListener.java @@ -15,37 +15,22 @@ package com.google.firebase.installations; import com.google.android.gms.tasks.TaskCompletionSource; -import com.google.firebase.installations.FirebaseInstallationsApi.AuthTokenOption; import com.google.firebase.installations.local.PersistedFidEntry; class GetAuthTokenListener implements StateListener { private final Utils utils; private final TaskCompletionSource resultTaskCompletionSource; - private final String oldAuthToken; - @AuthTokenOption private final int authTokenOption; public GetAuthTokenListener( - Utils utils, - TaskCompletionSource resultTaskCompletionSource, - String oldAuthToken, - @AuthTokenOption int authTokenOption) { + Utils utils, TaskCompletionSource resultTaskCompletionSource) { this.utils = utils; this.resultTaskCompletionSource = resultTaskCompletionSource; - this.oldAuthToken = oldAuthToken; - this.authTokenOption = authTokenOption; } @Override public boolean onStateReached(PersistedFidEntry persistedFidEntry) { - // AuthTokenListener state is reached when: - // 1. If AuthTokenOption is DO_NOT_FORCE_REFRESH : FID is registered and has a valid auth token - // 2. If AuthTokenOption is FORCE_REFRESH : FID is registered, has a valid new auth token which - // is not same as the old auth token - - if (persistedFidEntry.isRegistered() - && !utils.isAuthTokenExpired(persistedFidEntry) - && (authTokenOption == FirebaseInstallationsApi.DO_NOT_FORCE_REFRESH - || !oldAuthToken.equals(persistedFidEntry.getAuthToken()))) { + // AuthTokenListener state is reached when FID is registered and has a valid auth token + if (persistedFidEntry.isRegistered() && !utils.isAuthTokenExpired(persistedFidEntry)) { resultTaskCompletionSource.setResult( InstallationTokenResult.builder() .setToken(persistedFidEntry.getAuthToken()) From 126e3725c3408da452ea3910780bb47a3c3157be Mon Sep 17 00:00:00 2001 From: Ankita Jhawar Date: Fri, 27 Sep 2019 14:52:32 -0700 Subject: [PATCH 05/15] Cleaning up instrumented test file. --- .../FirebaseInstallationsInstrumentedTest.java | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/firebase-installations/src/androidTest/java/com/google/firebase/installations/FirebaseInstallationsInstrumentedTest.java b/firebase-installations/src/androidTest/java/com/google/firebase/installations/FirebaseInstallationsInstrumentedTest.java index 95b3359316a..fd031703f3b 100644 --- a/firebase-installations/src/androidTest/java/com/google/firebase/installations/FirebaseInstallationsInstrumentedTest.java +++ b/firebase-installations/src/androidTest/java/com/google/firebase/installations/FirebaseInstallationsInstrumentedTest.java @@ -117,16 +117,6 @@ public class FirebaseInstallationsInstrumentedTest { .setRegistrationStatus(PersistedFid.RegistrationStatus.UNREGISTERED) .build(); - private static final PersistedFidEntry UPDATED_AUTH_TOKEN_FID_ENTRY = - PersistedFidEntry.builder() - .setFirebaseInstallationId(TEST_FID_1) - .setAuthToken(TEST_AUTH_TOKEN_2) - .setRefreshToken(TEST_REFRESH_TOKEN) - .setTokenCreationEpochInSecs(TEST_CREATION_TIMESTAMP_2) - .setExpiresInSecs(TEST_TOKEN_EXPIRATION_TIMESTAMP) - .setRegistrationStatus(PersistedFid.RegistrationStatus.REGISTERED) - .build(); - @Before public void setUp() throws FirebaseInstallationServiceException { MockitoAnnotations.initMocks(this); @@ -145,6 +135,7 @@ public void setUp() throws FirebaseInstallationServiceException { when(backendClientReturnsOk.createFirebaseInstallation( anyString(), anyString(), anyString(), anyString())) .thenReturn(TEST_INSTALLATION_RESPONSE); + // Mocks successful auth token generation when(backendClientReturnsOk.generateAuthToken( anyString(), anyString(), anyString(), anyString())) .thenReturn(TEST_INSTALLATION_TOKEN_RESULT); @@ -341,7 +332,7 @@ public void testGetId_expiredAuthToken_refreshesAuthToken() throws Exception { // Waiting for Task that registers FID on the FIS Servers executor.awaitTermination(500, TimeUnit.MILLISECONDS); - // Validate that registration is complete with a refreshed auth token + // Validate that Persisted FID has a refreshed auth token now PersistedFidEntry updatedFidEntry = persistedFid.readPersistedFidEntryValue(); assertThat(updatedFidEntry).hasAuthToken(TEST_AUTH_TOKEN_2); verify(backendClientReturnsOk, never()) From c39cdb8e01aaf60759f68dac89874b696abbff93 Mon Sep 17 00:00:00 2001 From: Ankita Jhawar Date: Sun, 29 Sep 2019 00:10:22 -0700 Subject: [PATCH 06/15] Ciaran's comments on synchronization and runnable. --- .../installations/FirebaseInstallations.java | 40 +++++++++++++------ 1 file changed, 28 insertions(+), 12 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 5109523815b..ea5afa17337 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 @@ -56,7 +56,9 @@ public class FirebaseInstallations implements FirebaseInstallationsApi { private final ExecutorService executor; private final Utils utils; private final Object lock = new Object(); + private final Runnable runnable; + @GuardedBy("lock") private boolean shouldRefreshAuthToken; @GuardedBy("lock") @@ -83,6 +85,7 @@ public class FirebaseInstallations implements FirebaseInstallationsApi { this.executor = executor; this.persistedFid = persistedFid; this.utils = utils; + this.runnable = doRegistration(); } /** @@ -128,7 +131,7 @@ String getName() { @Override public Task getId() { Task task = addGetIdListener(); - executor.execute(doRegistration()); + executor.execute(runnable); return task; } @@ -145,11 +148,8 @@ public Task getId() { @Override public synchronized Task getAuthToken( @AuthTokenOption int authTokenOption) { - if (authTokenOption == FORCE_REFRESH) { - shouldRefreshAuthToken = true; - } - Task task = addGetAuthTokenListener(); - executor.execute(doRegistration()); + Task task = addGetAuthTokenListener(authTokenOption); + executor.execute(runnable); return task; } @@ -173,11 +173,15 @@ private Task addGetIdListener() { return taskCompletionSource.getTask(); } - private Task addGetAuthTokenListener() { + private Task addGetAuthTokenListener( + @AuthTokenOption int authTokenOption) { TaskCompletionSource taskCompletionSource = new TaskCompletionSource<>(); StateListener l = new GetAuthTokenListener(utils, taskCompletionSource); synchronized (lock) { + if (authTokenOption == FORCE_REFRESH) { + shouldRefreshAuthToken = true; + } listeners.add(l); } return taskCompletionSource.getTask(); @@ -208,9 +212,12 @@ private final Runnable doRegistration() { persistedFidEntry = persistedFid.readPersistedFidEntryValue(); } - // If Auth Token needs refresh, notify the listeners only after force refresh - if (!shouldRefreshAuthToken) { - triggerOnStateReached(persistedFidEntry); + // Notify the listeners only after force refreshing auth token if shouldRefreshAuthToken is + // true + synchronized (lock) { + if (!shouldRefreshAuthToken) { + triggerOnStateReached(persistedFidEntry); + } } // FID needs to be registered @@ -222,9 +229,18 @@ private final Runnable doRegistration() { // Don't notify the listeners at this point; we might as well make ure the auth token is up // to date before letting them know. + boolean needRefresh = utils.isAuthTokenExpired(persistedFidEntry); + if (!needRefresh) { + synchronized (lock) { + needRefresh = shouldRefreshAuthToken; + } + } + // Refresh Auth token if needed - if (shouldRefreshAuthToken || utils.isAuthTokenExpired(persistedFidEntry)) { - shouldRefreshAuthToken = false; + if (needRefresh) { + synchronized (lock) { + shouldRefreshAuthToken = false; + } fetchAuthTokenFromServer(persistedFidEntry); persistedFidEntry = persistedFid.readPersistedFidEntryValue(); } From c21a59ef30b5952998cd03013e316dce93a7eca9 Mon Sep 17 00:00:00 2001 From: Ankita Jhawar Date: Sun, 29 Sep 2019 00:10:22 -0700 Subject: [PATCH 07/15] Ciaran's comments on synchronization and runnable. --- .../installations/FirebaseInstallations.java | 42 +++++++++++++------ 1 file changed, 29 insertions(+), 13 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 5109523815b..015e296c563 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 @@ -56,7 +56,9 @@ public class FirebaseInstallations implements FirebaseInstallationsApi { private final ExecutorService executor; private final Utils utils; private final Object lock = new Object(); + private final Runnable runnable; + @GuardedBy("lock") private boolean shouldRefreshAuthToken; @GuardedBy("lock") @@ -83,6 +85,7 @@ public class FirebaseInstallations implements FirebaseInstallationsApi { this.executor = executor; this.persistedFid = persistedFid; this.utils = utils; + this.runnable = doRegistration(); } /** @@ -128,7 +131,7 @@ String getName() { @Override public Task getId() { Task task = addGetIdListener(); - executor.execute(doRegistration()); + executor.execute(runnable); return task; } @@ -143,13 +146,10 @@ public Task getId() { */ @NonNull @Override - public synchronized Task getAuthToken( + public Task getAuthToken( @AuthTokenOption int authTokenOption) { - if (authTokenOption == FORCE_REFRESH) { - shouldRefreshAuthToken = true; - } - Task task = addGetAuthTokenListener(); - executor.execute(doRegistration()); + Task task = addGetAuthTokenListener(authTokenOption); + executor.execute(runnable); return task; } @@ -173,11 +173,15 @@ private Task addGetIdListener() { return taskCompletionSource.getTask(); } - private Task addGetAuthTokenListener() { + private Task addGetAuthTokenListener( + @AuthTokenOption int authTokenOption) { TaskCompletionSource taskCompletionSource = new TaskCompletionSource<>(); StateListener l = new GetAuthTokenListener(utils, taskCompletionSource); synchronized (lock) { + if (authTokenOption == FORCE_REFRESH) { + shouldRefreshAuthToken = true; + } listeners.add(l); } return taskCompletionSource.getTask(); @@ -208,9 +212,12 @@ private final Runnable doRegistration() { persistedFidEntry = persistedFid.readPersistedFidEntryValue(); } - // If Auth Token needs refresh, notify the listeners only after force refresh - if (!shouldRefreshAuthToken) { - triggerOnStateReached(persistedFidEntry); + // Notify the listeners only after force refreshing auth token if shouldRefreshAuthToken is + // true + synchronized (lock) { + if (!shouldRefreshAuthToken) { + triggerOnStateReached(persistedFidEntry); + } } // FID needs to be registered @@ -222,9 +229,18 @@ private final Runnable doRegistration() { // Don't notify the listeners at this point; we might as well make ure the auth token is up // to date before letting them know. + boolean needRefresh = utils.isAuthTokenExpired(persistedFidEntry); + if (!needRefresh) { + synchronized (lock) { + needRefresh = shouldRefreshAuthToken; + } + } + // Refresh Auth token if needed - if (shouldRefreshAuthToken || utils.isAuthTokenExpired(persistedFidEntry)) { - shouldRefreshAuthToken = false; + if (needRefresh) { + synchronized (lock) { + shouldRefreshAuthToken = false; + } fetchAuthTokenFromServer(persistedFidEntry); persistedFidEntry = persistedFid.readPersistedFidEntryValue(); } From 314b3bb51eaffe73769236d644a45a29a41e4b76 Mon Sep 17 00:00:00 2001 From: Ankita Jhawar Date: Mon, 30 Sep 2019 11:13:56 -0700 Subject: [PATCH 08/15] Fixing precheck failures. --- .../google/firebase/installations/FirebaseInstallations.java | 3 +-- 1 file changed, 1 insertion(+), 2 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 015e296c563..c3278a1ee88 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 @@ -146,8 +146,7 @@ public Task getId() { */ @NonNull @Override - public Task getAuthToken( - @AuthTokenOption int authTokenOption) { + public Task getAuthToken(@AuthTokenOption int authTokenOption) { Task task = addGetAuthTokenListener(authTokenOption); executor.execute(runnable); return task; From 715d427e8f4c9b2e2d1cf21c7636915246ae5361 Mon Sep 17 00:00:00 2001 From: Ankita Jhawar Date: Mon, 30 Sep 2019 14:35:49 -0700 Subject: [PATCH 09/15] Addressing Ciaran's comment on mock utils. --- ...FirebaseInstallationsInstrumentedTest.java | 38 ++++++++++++++++--- 1 file changed, 32 insertions(+), 6 deletions(-) diff --git a/firebase-installations/src/androidTest/java/com/google/firebase/installations/FirebaseInstallationsInstrumentedTest.java b/firebase-installations/src/androidTest/java/com/google/firebase/installations/FirebaseInstallationsInstrumentedTest.java index fd031703f3b..ca6e34b761d 100644 --- a/firebase-installations/src/androidTest/java/com/google/firebase/installations/FirebaseInstallationsInstrumentedTest.java +++ b/firebase-installations/src/androidTest/java/com/google/firebase/installations/FirebaseInstallationsInstrumentedTest.java @@ -92,7 +92,7 @@ public class FirebaseInstallationsInstrumentedTest { .setFirebaseInstallationId(TEST_FID_1) .setAuthToken(TEST_AUTH_TOKEN) .setRefreshToken(TEST_REFRESH_TOKEN) - .setTokenCreationEpochInSecs(TEST_CREATION_TIMESTAMP_1) + .setTokenCreationEpochInSecs(TEST_CREATION_TIMESTAMP_2) .setExpiresInSecs(TEST_TOKEN_EXPIRATION_TIMESTAMP) .setRegistrationStatus(PersistedFid.RegistrationStatus.REGISTERED) .build(); @@ -117,6 +117,16 @@ public class FirebaseInstallationsInstrumentedTest { .setRegistrationStatus(PersistedFid.RegistrationStatus.UNREGISTERED) .build(); + private static final PersistedFidEntry UPDATED_AUTH_TOKEN_ENTRY = + PersistedFidEntry.builder() + .setFirebaseInstallationId(TEST_FID_1) + .setAuthToken(TEST_AUTH_TOKEN_2) + .setRefreshToken(TEST_REFRESH_TOKEN) + .setTokenCreationEpochInSecs(TEST_CREATION_TIMESTAMP_2) + .setExpiresInSecs(TEST_TOKEN_EXPIRATION_TIMESTAMP) + .setRegistrationStatus(PersistedFid.RegistrationStatus.REGISTERED) + .build(); + @Before public void setUp() throws FirebaseInstallationServiceException { MockitoAnnotations.initMocks(this); @@ -152,7 +162,6 @@ public void setUp() throws FirebaseInstallationServiceException { when(mockUtils.createRandomFid()).thenReturn(TEST_FID_1); when(mockUtils.currentTimeInSecs()).thenReturn(TEST_CREATION_TIMESTAMP_2); - when(mockUtils.isAuthTokenExpired(any())).thenReturn(false); // Mocks success on FIS deletion doNothing() @@ -173,6 +182,7 @@ public void cleanUp() throws Exception { @Test public void testGetId_PersistedFidOk_BackendOk() throws Exception { + when(mockUtils.isAuthTokenExpired(REGISTERED_FID_ENTRY)).thenReturn(false); FirebaseInstallations firebaseInstallations = new FirebaseInstallations( executor, firebaseApp, backendClientReturnsOk, persistedFid, mockUtils); @@ -194,6 +204,7 @@ public void testGetId_PersistedFidOk_BackendOk() throws Exception { @Test public void testGetId_multipleCalls_sameFIDReturned() throws Exception { + when(mockUtils.isAuthTokenExpired(REGISTERED_FID_ENTRY)).thenReturn(false); FirebaseInstallations firebaseInstallations = new FirebaseInstallations( executor, firebaseApp, backendClientReturnsOk, persistedFid, mockUtils); @@ -266,6 +277,7 @@ public void testGetId_fidRegistrationUncheckedException_statusUpdated() throws E invocation -> { throw new InterruptedException(); }); + when(mockUtils.isAuthTokenExpired(REGISTERED_FID_ENTRY)).thenReturn(false); FirebaseInstallations firebaseInstallations = new FirebaseInstallations(executor, firebaseApp, mockClient, persistedFid, mockUtils); @@ -294,7 +306,7 @@ public void testGetId_expiredAuthTokenUncheckedException_statusUpdated() throws invocation -> { throw new InterruptedException(); }); - when(mockUtils.isAuthTokenExpired(any())).thenReturn(true); + when(mockUtils.isAuthTokenExpired(EXPIRED_AUTH_TOKEN_ENTRY)).thenReturn(true); FirebaseInstallations firebaseInstallations = new FirebaseInstallations(executor, firebaseApp, mockClient, persistedFid, mockUtils); @@ -316,9 +328,10 @@ public void testGetId_expiredAuthTokenUncheckedException_statusUpdated() throws @Test public void testGetId_expiredAuthToken_refreshesAuthToken() throws Exception { - when(mockUtils.isAuthTokenExpired(any())).thenReturn(true); // Update local storage with fid entry that has auth token expired. persistedFid.insertOrUpdatePersistedFidEntry(EXPIRED_AUTH_TOKEN_ENTRY); + when(mockUtils.isAuthTokenExpired(EXPIRED_AUTH_TOKEN_ENTRY)).thenReturn(true); + FirebaseInstallations firebaseInstallations = new FirebaseInstallations( executor, firebaseApp, backendClientReturnsOk, persistedFid, mockUtils); @@ -343,6 +356,7 @@ public void testGetId_expiredAuthToken_refreshesAuthToken() throws Exception { @Test public void testGetAuthToken_fidDoesNotExist_successful() throws Exception { + when(mockUtils.isAuthTokenExpired(REGISTERED_FID_ENTRY)).thenReturn(false); FirebaseInstallations firebaseInstallations = new FirebaseInstallations( executor, firebaseApp, backendClientReturnsOk, persistedFid, mockUtils); @@ -355,6 +369,7 @@ public void testGetAuthToken_fidDoesNotExist_successful() throws Exception { @Test public void testGetAuthToken_PersistedFidError_failure() throws Exception { + when(mockUtils.isAuthTokenExpired(REGISTERED_FID_ENTRY)).thenReturn(false); FirebaseInstallations firebaseInstallations = new FirebaseInstallations( executor, firebaseApp, backendClientReturnsOk, persistedFidReturnsError, mockUtils); @@ -378,6 +393,8 @@ public void testGetAuthToken_PersistedFidError_failure() throws Exception { @Test public void testGetAuthToken_fidExists_successful() throws Exception { when(mockPersistedFid.readPersistedFidEntryValue()).thenReturn(REGISTERED_FID_ENTRY); + when(mockUtils.isAuthTokenExpired(REGISTERED_FID_ENTRY)).thenReturn(false); + FirebaseInstallations firebaseInstallations = new FirebaseInstallations( executor, firebaseApp, backendClientReturnsOk, mockPersistedFid, mockUtils); @@ -395,8 +412,10 @@ public void testGetAuthToken_fidExists_successful() throws Exception { @Test public void testGetAuthToken_expiredAuthToken_fetchedNewTokenFromFIS() throws Exception { - when(mockUtils.isAuthTokenExpired(any())).thenReturn(true, true, false); persistedFid.insertOrUpdatePersistedFidEntry(EXPIRED_AUTH_TOKEN_ENTRY); + when(mockUtils.isAuthTokenExpired(EXPIRED_AUTH_TOKEN_ENTRY)).thenReturn(true); + when(mockUtils.isAuthTokenExpired(UPDATED_AUTH_TOKEN_ENTRY)).thenReturn(false); + FirebaseInstallations firebaseInstallations = new FirebaseInstallations( executor, firebaseApp, backendClientReturnsOk, persistedFid, mockUtils); @@ -417,6 +436,8 @@ public void testGetAuthToken_unregisteredFid_fetchedNewTokenFromFIS() throws Exc // Update local storage with a unregistered fid entry to validate that getAuthToken calls getId // to ensure FID registration and returns a valid auth token. persistedFid.insertOrUpdatePersistedFidEntry(UNREGISTERED_FID_ENTRY); + when(mockUtils.isAuthTokenExpired(REGISTERED_FID_ENTRY)).thenReturn(false); + FirebaseInstallations firebaseInstallations = new FirebaseInstallations( executor, firebaseApp, backendClientReturnsOk, persistedFid, mockUtils); @@ -440,6 +461,8 @@ public void testGetAuthToken_serverError_failure() throws Exception { .thenThrow( new FirebaseInstallationServiceException( "Server Error", FirebaseInstallationServiceException.Status.SERVER_ERROR)); + when(mockUtils.isAuthTokenExpired(REGISTERED_FID_ENTRY)).thenReturn(false); + FirebaseInstallations firebaseInstallations = new FirebaseInstallations( executor, firebaseApp, backendClientReturnsError, mockPersistedFid, mockUtils); @@ -466,7 +489,9 @@ public void testGetAuthToken_multipleCallsDoNotForceRefresh_fetchedNewTokenOnce( // triggered simultaneously. Task2 waits for Task1 to complete. On task1 completion, task2 reads // the UPDATED_AUTH_TOKEN_FID_ENTRY generated by Task1. persistedFid.insertOrUpdatePersistedFidEntry(EXPIRED_AUTH_TOKEN_ENTRY); - when(mockUtils.isAuthTokenExpired(any())).thenReturn(true, true, true, false); + when(mockUtils.isAuthTokenExpired(EXPIRED_AUTH_TOKEN_ENTRY)).thenReturn(true); + when(mockUtils.isAuthTokenExpired(UPDATED_AUTH_TOKEN_ENTRY)).thenReturn(false); + FirebaseInstallations firebaseInstallations = new FirebaseInstallations( executor, firebaseApp, backendClientReturnsOk, persistedFid, mockUtils); @@ -514,6 +539,7 @@ public void testGetAuthToken_multipleCallsForceRefresh_fetchedNewTokenTwice() th .build())) .when(backendClientReturnsOk) .generateAuthToken(anyString(), anyString(), anyString(), anyString()); + when(mockUtils.isAuthTokenExpired(any())).thenReturn(false); FirebaseInstallations firebaseInstallations = new FirebaseInstallations( From bb322d07e4de4a08efb41825ba15933f6f9fe71c Mon Sep 17 00:00:00 2001 From: Ankita Jhawar Date: Mon, 30 Sep 2019 17:59:25 -0700 Subject: [PATCH 10/15] Cleaning doRegistration method. --- .../installations/FirebaseInstallations.java | 98 +++++++++---------- 1 file changed, 47 insertions(+), 51 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 c3278a1ee88..a6de5b93e31 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 @@ -56,7 +56,6 @@ public class FirebaseInstallations implements FirebaseInstallationsApi { private final ExecutorService executor; private final Utils utils; private final Object lock = new Object(); - private final Runnable runnable; @GuardedBy("lock") private boolean shouldRefreshAuthToken; @@ -85,7 +84,6 @@ public class FirebaseInstallations implements FirebaseInstallationsApi { this.executor = executor; this.persistedFid = persistedFid; this.utils = utils; - this.runnable = doRegistration(); } /** @@ -131,7 +129,7 @@ String getName() { @Override public Task getId() { Task task = addGetIdListener(); - executor.execute(runnable); + executor.execute(this::doRegistration); return task; } @@ -148,7 +146,7 @@ public Task getId() { @Override public Task getAuthToken(@AuthTokenOption int authTokenOption) { Task task = addGetAuthTokenListener(authTokenOption); - executor.execute(runnable); + executor.execute(this::doRegistration); return task; } @@ -199,63 +197,61 @@ private void triggerOnStateReached(PersistedFidEntry persistedFidEntry) { } } - private final Runnable doRegistration() { - return () -> { - try { - PersistedFidEntry persistedFidEntry = persistedFid.readPersistedFidEntryValue(); + private final void doRegistration() { + try { + PersistedFidEntry persistedFidEntry = persistedFid.readPersistedFidEntryValue(); - // New FID needs to be created - if (persistedFidEntry.isErrored() || persistedFidEntry.isNotGenerated()) { - String fid = utils.createRandomFid(); - persistFid(fid); - persistedFidEntry = persistedFid.readPersistedFidEntryValue(); - } + // New FID needs to be created + if (persistedFidEntry.isErrored() || persistedFidEntry.isNotGenerated()) { + String fid = utils.createRandomFid(); + persistFid(fid); + persistedFidEntry = persistedFid.readPersistedFidEntryValue(); + } - // Notify the listeners only after force refreshing auth token if shouldRefreshAuthToken is - // true - synchronized (lock) { - if (!shouldRefreshAuthToken) { - triggerOnStateReached(persistedFidEntry); - } + // Always notify the GetIdListeners. For GetAuthTokenListeners, only notify if force + // refreshing auth token is not required. + synchronized (lock) { + if (!shouldRefreshAuthToken) { + triggerOnStateReached(persistedFidEntry); } + } - // FID needs to be registered - if (persistedFidEntry.isUnregistered()) { - registerAndSaveFid(persistedFidEntry); - persistedFidEntry = persistedFid.readPersistedFidEntryValue(); - } + // FID needs to be registered + if (persistedFidEntry.isUnregistered()) { + registerAndSaveFid(persistedFidEntry); + persistedFidEntry = persistedFid.readPersistedFidEntryValue(); + } - // Don't notify the listeners at this point; we might as well make ure the auth token is up - // to date before letting them know. + // Don't notify the listeners at this point; we might as well make ure the auth token is up + // to date before letting them know. - boolean needRefresh = utils.isAuthTokenExpired(persistedFidEntry); - if (!needRefresh) { - synchronized (lock) { - needRefresh = shouldRefreshAuthToken; - } + boolean needRefresh = utils.isAuthTokenExpired(persistedFidEntry); + if (!needRefresh) { + synchronized (lock) { + needRefresh = shouldRefreshAuthToken; } + } - // Refresh Auth token if needed - if (needRefresh) { - synchronized (lock) { - shouldRefreshAuthToken = false; - } - fetchAuthTokenFromServer(persistedFidEntry); - persistedFidEntry = persistedFid.readPersistedFidEntryValue(); + // Refresh Auth token if needed + if (needRefresh) { + fetchAuthTokenFromServer(persistedFidEntry); + synchronized (lock) { + shouldRefreshAuthToken = false; } - - triggerOnStateReached(persistedFidEntry); - } catch (Exception e) { - PersistedFidEntry persistedFidEntry = persistedFid.readPersistedFidEntryValue(); - PersistedFidEntry errorFidEntry = - persistedFidEntry - .toBuilder() - .setRegistrationStatus(RegistrationStatus.REGISTER_ERROR) - .build(); - persistedFid.insertOrUpdatePersistedFidEntry(errorFidEntry); - triggerOnStateReached(errorFidEntry); + persistedFidEntry = persistedFid.readPersistedFidEntryValue(); } - }; + + triggerOnStateReached(persistedFidEntry); + } catch (Exception e) { + PersistedFidEntry persistedFidEntry = persistedFid.readPersistedFidEntryValue(); + PersistedFidEntry errorFidEntry = + persistedFidEntry + .toBuilder() + .setRegistrationStatus(RegistrationStatus.REGISTER_ERROR) + .build(); + persistedFid.insertOrUpdatePersistedFidEntry(errorFidEntry); + triggerOnStateReached(errorFidEntry); + } } private void persistFid(String fid) throws FirebaseInstallationsException { From 1fd193b4d2c8e58c1f510f8ae934c42b86aaf685 Mon Sep 17 00:00:00 2001 From: Ankita Jhawar Date: Tue, 8 Oct 2019 11:57:41 -0700 Subject: [PATCH 11/15] Fixing nit comments. --- .../java/com/google/firebase/installations/Utils.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/firebase-installations/src/main/java/com/google/firebase/installations/Utils.java b/firebase-installations/src/main/java/com/google/firebase/installations/Utils.java index 0c2b4da6a14..1a3477e9894 100644 --- a/firebase-installations/src/main/java/com/google/firebase/installations/Utils.java +++ b/firebase-installations/src/main/java/com/google/firebase/installations/Utils.java @@ -42,19 +42,19 @@ class Utils { /** Length of new-format FIDs as introduced in 2019. */ public static final int FID_LENGTH = 22; - private static final long AUTH_TOKEN_EXPIRATION_BUFFER_IN_SECS = 3600L; // 1 hour + private static final long AUTH_TOKEN_EXPIRATION_BUFFER_IN_SECS = TimeUnit.HOURS.toSeconds(1); Utils(Clock clock) { this.clock = clock; } /** - * Checks if the FIS Auth token is expired or going to expire in next 1 hour - * (AUTH_TOKEN_EXPIRATION_BUFFER_IN_SECS). + * Checks if the FIS Auth token is expired or going to expire in next 1 hour {@link + * #AUTH_TOKEN_EXPIRATION_BUFFER_IN_SECS}. */ public boolean isAuthTokenExpired(PersistedFidEntry persistedFidEntry) { - return (persistedFidEntry.getTokenCreationEpochInSecs() + persistedFidEntry.getExpiresInSecs() - > currentTimeInSecs() + AUTH_TOKEN_EXPIRATION_BUFFER_IN_SECS); + return persistedFidEntry.getTokenCreationEpochInSecs() + persistedFidEntry.getExpiresInSecs() + > currentTimeInSecs() + AUTH_TOKEN_EXPIRATION_BUFFER_IN_SECS; } /** Returns current time in seconds. */ From e2d363190376319fabcc9b6d4256a1296727337c Mon Sep 17 00:00:00 2001 From: Ankita Jhawar Date: Tue, 8 Oct 2019 16:56:32 -0700 Subject: [PATCH 12/15] Addressing ciaran's comment on satisfying immediate getId response. --- .../installations/FirebaseInstallations.java | 16 +++++++--------- .../installations/GetAuthTokenListener.java | 7 +++++-- .../firebase/installations/GetIdListener.java | 2 +- .../firebase/installations/StateListener.java | 2 +- 4 files changed, 14 insertions(+), 13 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 a6de5b93e31..76961076092 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 @@ -189,7 +189,7 @@ private void triggerOnStateReached(PersistedFidEntry persistedFidEntry) { Iterator it = listeners.iterator(); while (it.hasNext()) { StateListener l = it.next(); - boolean doneListening = l.onStateReached(persistedFidEntry); + boolean doneListening = l.onStateReached(persistedFidEntry, shouldRefreshAuthToken); if (doneListening) { it.remove(); } @@ -208,18 +208,16 @@ private final void doRegistration() { persistedFidEntry = persistedFid.readPersistedFidEntryValue(); } - // Always notify the GetIdListeners. For GetAuthTokenListeners, only notify if force - // refreshing auth token is not required. - synchronized (lock) { - if (!shouldRefreshAuthToken) { - triggerOnStateReached(persistedFidEntry); - } - } + triggerOnStateReached(persistedFidEntry); // FID needs to be registered if (persistedFidEntry.isUnregistered()) { registerAndSaveFid(persistedFidEntry); persistedFidEntry = persistedFid.readPersistedFidEntryValue(); + // Newly registered Fid will have valid auth token. No refresh required. + synchronized (lock) { + shouldRefreshAuthToken = false; + } } // Don't notify the listeners at this point; we might as well make ure the auth token is up @@ -235,10 +233,10 @@ private final void doRegistration() { // Refresh Auth token if needed if (needRefresh) { fetchAuthTokenFromServer(persistedFidEntry); + persistedFidEntry = persistedFid.readPersistedFidEntryValue(); synchronized (lock) { shouldRefreshAuthToken = false; } - persistedFidEntry = persistedFid.readPersistedFidEntryValue(); } triggerOnStateReached(persistedFidEntry); diff --git a/firebase-installations/src/main/java/com/google/firebase/installations/GetAuthTokenListener.java b/firebase-installations/src/main/java/com/google/firebase/installations/GetAuthTokenListener.java index 57760e9adb3..bc9348a344b 100644 --- a/firebase-installations/src/main/java/com/google/firebase/installations/GetAuthTokenListener.java +++ b/firebase-installations/src/main/java/com/google/firebase/installations/GetAuthTokenListener.java @@ -28,9 +28,12 @@ public GetAuthTokenListener( } @Override - public boolean onStateReached(PersistedFidEntry persistedFidEntry) { + public boolean onStateReached( + PersistedFidEntry persistedFidEntry, boolean shouldRefreshAuthToken) { // AuthTokenListener state is reached when FID is registered and has a valid auth token - if (persistedFidEntry.isRegistered() && !utils.isAuthTokenExpired(persistedFidEntry)) { + if (persistedFidEntry.isRegistered() + && !utils.isAuthTokenExpired(persistedFidEntry) + && !shouldRefreshAuthToken) { resultTaskCompletionSource.setResult( InstallationTokenResult.builder() .setToken(persistedFidEntry.getAuthToken()) 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 cee158348e3..b1b9036f0e0 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 @@ -25,7 +25,7 @@ public GetIdListener(TaskCompletionSource taskCompletionSource) { } @Override - public boolean onStateReached(PersistedFidEntry persistedFidEntry) { + public boolean onStateReached(PersistedFidEntry persistedFidEntry, boolean unused) { if (persistedFidEntry.isUnregistered() || persistedFidEntry.isRegistered()) { taskCompletionSource.setResult(persistedFidEntry.getFirebaseInstallationId()); return true; diff --git a/firebase-installations/src/main/java/com/google/firebase/installations/StateListener.java b/firebase-installations/src/main/java/com/google/firebase/installations/StateListener.java index 45d1e5e219a..c839a107e87 100644 --- a/firebase-installations/src/main/java/com/google/firebase/installations/StateListener.java +++ b/firebase-installations/src/main/java/com/google/firebase/installations/StateListener.java @@ -17,5 +17,5 @@ import com.google.firebase.installations.local.PersistedFidEntry; interface StateListener { - boolean onStateReached(PersistedFidEntry persistedFidEntry); + boolean onStateReached(PersistedFidEntry persistedFidEntry, boolean shouldRefreshAuthToken); } From e68e5991129a0280f0729ee91888cc829d761eac Mon Sep 17 00:00:00 2001 From: Ankita Date: Wed, 9 Oct 2019 10:23:10 -0700 Subject: [PATCH 13/15] Fixing FISClient to correctly parse expiration timestamp. (#848) * Fixing FISClient to correctly parse expiration timestamp. --- .../firebase-installations.gradle | 2 + .../FirebaseInstallationServiceClient.java | 28 +++++++++- ...FirebaseInstallationServiceClientTest.java | 55 +++++++++++++++++++ 3 files changed, 82 insertions(+), 3 deletions(-) create mode 100644 firebase-installations/src/test/java/com/google/firebase/installations/remote/FirebaseInstallationServiceClientTest.java diff --git a/firebase-installations/firebase-installations.gradle b/firebase-installations/firebase-installations.gradle index 39b289c0834..0ef070e8b6d 100644 --- a/firebase-installations/firebase-installations.gradle +++ b/firebase-installations/firebase-installations.gradle @@ -52,6 +52,8 @@ dependencies { testImplementation 'androidx.test:core:1.2.0' testImplementation 'junit:junit:4.12' testImplementation "org.robolectric:robolectric:$robolectricVersion" + testImplementation "com.google.truth:truth:$googleTruthVersion" + androidTestImplementation 'androidx.test.ext:junit:1.1.1' androidTestImplementation 'androidx.test:runner:1.2.0' diff --git a/firebase-installations/src/main/java/com/google/firebase/installations/remote/FirebaseInstallationServiceClient.java b/firebase-installations/src/main/java/com/google/firebase/installations/remote/FirebaseInstallationServiceClient.java index 51cc1869fae..e1a6facd6f5 100644 --- a/firebase-installations/src/main/java/com/google/firebase/installations/remote/FirebaseInstallationServiceClient.java +++ b/firebase-installations/src/main/java/com/google/firebase/installations/remote/FirebaseInstallationServiceClient.java @@ -15,6 +15,7 @@ package com.google.firebase.installations.remote; import static android.content.ContentValues.TAG; +import static com.google.android.gms.common.internal.Preconditions.checkArgument; import android.content.Context; import android.content.pm.PackageManager; @@ -23,12 +24,13 @@ import androidx.annotation.NonNull; import com.google.android.gms.common.util.AndroidUtilsLight; import com.google.android.gms.common.util.Hex; +import com.google.android.gms.common.util.VisibleForTesting; import com.google.firebase.installations.InstallationTokenResult; import java.io.IOException; import java.io.InputStreamReader; import java.net.URL; import java.nio.charset.Charset; -import java.util.concurrent.TimeUnit; +import java.util.regex.Pattern; import java.util.zip.GZIPOutputStream; import javax.net.ssl.HttpsURLConnection; import org.json.JSONException; @@ -61,6 +63,11 @@ public class FirebaseInstallationServiceClient { private static final int NETWORK_TIMEOUT_MILLIS = 10000; + private static final Pattern EXPIRATION_TIMESTAMP_PATTERN = Pattern.compile("[0-9]+s"); + + @VisibleForTesting + static final String PARSING_EXPIRATION_TIME_ERROR_MESSAGE = "Invalid Expiration Timestamp."; + private final Context context; public FirebaseInstallationServiceClient(@NonNull Context context) { @@ -273,7 +280,7 @@ private InstallationResponse readCreateResponse(HttpsURLConnection conn) throws installationTokenResult.setToken(reader.nextString()); } else if (key.equals("expiresIn")) { installationTokenResult.setTokenExpirationInSecs( - TimeUnit.MILLISECONDS.toSeconds(reader.nextLong())); + parseTokenExpirationTimestamp(reader.nextString())); } else { reader.skipValue(); } @@ -301,7 +308,7 @@ private InstallationTokenResult readGenerateAuthTokenResponse(HttpsURLConnection if (name.equals("token")) { builder.setToken(reader.nextString()); } else if (name.equals("expiresIn")) { - builder.setTokenExpirationInSecs(TimeUnit.MILLISECONDS.toSeconds(reader.nextLong())); + builder.setTokenExpirationInSecs(parseTokenExpirationTimestamp(reader.nextString())); } else { reader.skipValue(); } @@ -329,4 +336,19 @@ private String getFingerprintHashForPackage() { return null; } } + + /** + * Returns parsed token expiration timestamp in seconds. + * + * @param expiresIn is expiration timestamp in String format: 604800s + */ + @VisibleForTesting + static long parseTokenExpirationTimestamp(String expiresIn) { + checkArgument( + EXPIRATION_TIMESTAMP_PATTERN.matcher(expiresIn).matches(), + PARSING_EXPIRATION_TIME_ERROR_MESSAGE); + return (expiresIn == null || expiresIn.length() == 0) + ? 0L + : Long.parseLong(expiresIn.substring(0, expiresIn.length() - 1)); + } } diff --git a/firebase-installations/src/test/java/com/google/firebase/installations/remote/FirebaseInstallationServiceClientTest.java b/firebase-installations/src/test/java/com/google/firebase/installations/remote/FirebaseInstallationServiceClientTest.java new file mode 100644 index 00000000000..eb30312c0e4 --- /dev/null +++ b/firebase-installations/src/test/java/com/google/firebase/installations/remote/FirebaseInstallationServiceClientTest.java @@ -0,0 +1,55 @@ +// 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.remote; + +import static com.google.common.truth.Truth.assertThat; +import static com.google.common.truth.Truth.assertWithMessage; +import static org.junit.Assert.fail; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.robolectric.RobolectricTestRunner; + +/** Tests for {@link FirebaseInstallationServiceClient}. */ +@RunWith(RobolectricTestRunner.class) +public class FirebaseInstallationServiceClientTest { + + private final String TEST_EXPIRATION_TIMESTAMP = "604800s"; + private final long TEST_EXPIRATION_IN_SECS = 604800; + private final String INCORRECT_EXPIRATION_TIMESTAMP = "2345"; + + @Test + public void parseTokenExpirationTimestamp_successful() { + long actual = + FirebaseInstallationServiceClient.parseTokenExpirationTimestamp(TEST_EXPIRATION_TIMESTAMP); + + assertWithMessage("Exception status doesn't match") + .that(actual) + .isEqualTo(TEST_EXPIRATION_IN_SECS); + } + + @Test + public void parseTokenExpirationTimestamp_failed() { + try { + FirebaseInstallationServiceClient.parseTokenExpirationTimestamp( + INCORRECT_EXPIRATION_TIMESTAMP); + fail("Parsing token expiration timestamp failed."); + } catch (IllegalArgumentException expected) { + assertThat(expected) + .hasMessageThat() + .isEqualTo(FirebaseInstallationServiceClient.PARSING_EXPIRATION_TIME_ERROR_MESSAGE); + } + } +} From 58c8efdd9d8a8fec3490c803f7e386c14951e6eb Mon Sep 17 00:00:00 2001 From: Ankita Date: Wed, 9 Oct 2019 11:23:53 -0700 Subject: [PATCH 14/15] Updating getAuthToken to return creation timestamp (#884) * Updating getAuthToken to return creation timestamp along with auth token and expiration timestamp. --- firebase-installations-interop/api.txt | 6 ++++-- .../installations/InstallationTokenResult.java | 13 +++++++++++-- .../FirebaseInstallationsInstrumentedTest.java | 6 ++++-- .../installations/FisAndroidTestConstants.java | 6 ++++-- .../installations/FirebaseInstallations.java | 4 ++-- .../installations/GetAuthTokenListener.java | 3 ++- .../remote/FirebaseInstallationServiceClient.java | 4 ++-- 7 files changed, 29 insertions(+), 13 deletions(-) diff --git a/firebase-installations-interop/api.txt b/firebase-installations-interop/api.txt index b2a54de1051..99b8e8affa4 100644 --- a/firebase-installations-interop/api.txt +++ b/firebase-installations-interop/api.txt @@ -5,7 +5,8 @@ package com.google.firebase.installations { ctor public InstallationTokenResult(); method @NonNull public static com.google.firebase.installations.InstallationTokenResult.Builder builder(); method @NonNull public abstract String getToken(); - method @NonNull public abstract long getTokenExpirationInSecs(); + method @NonNull public abstract long getTokenCreationTimestamp(); + method @NonNull public abstract long getTokenExpirationTimestamp(); method @NonNull public abstract com.google.firebase.installations.InstallationTokenResult.Builder toBuilder(); } @@ -13,7 +14,8 @@ package com.google.firebase.installations { ctor public InstallationTokenResult.Builder(); method @NonNull public abstract com.google.firebase.installations.InstallationTokenResult build(); method @NonNull public abstract com.google.firebase.installations.InstallationTokenResult.Builder setToken(@NonNull String); - method @NonNull public abstract com.google.firebase.installations.InstallationTokenResult.Builder setTokenExpirationInSecs(long); + method @NonNull public abstract com.google.firebase.installations.InstallationTokenResult.Builder setTokenCreationTimestamp(long); + method @NonNull public abstract com.google.firebase.installations.InstallationTokenResult.Builder setTokenExpirationTimestamp(long); } } diff --git a/firebase-installations-interop/src/main/java/com/google/firebase/installations/InstallationTokenResult.java b/firebase-installations-interop/src/main/java/com/google/firebase/installations/InstallationTokenResult.java index 94266bcc9f2..3dbc02c3e3e 100644 --- a/firebase-installations-interop/src/main/java/com/google/firebase/installations/InstallationTokenResult.java +++ b/firebase-installations-interop/src/main/java/com/google/firebase/installations/InstallationTokenResult.java @@ -28,7 +28,13 @@ public abstract class InstallationTokenResult { * The amount of time, in seconds, before the auth-token expires for this Firebase Installation. */ @NonNull - public abstract long getTokenExpirationInSecs(); + public abstract long getTokenExpirationTimestamp(); + + /** + * The amount of time, in seconds, when the auth-token was created for this Firebase Installation. + */ + @NonNull + public abstract long getTokenCreationTimestamp(); @NonNull public abstract Builder toBuilder(); @@ -45,7 +51,10 @@ public abstract static class Builder { public abstract Builder setToken(@NonNull String value); @NonNull - public abstract Builder setTokenExpirationInSecs(long value); + public abstract Builder setTokenExpirationTimestamp(long value); + + @NonNull + public abstract Builder setTokenCreationTimestamp(long value); @NonNull public abstract InstallationTokenResult build(); diff --git a/firebase-installations/src/androidTest/java/com/google/firebase/installations/FirebaseInstallationsInstrumentedTest.java b/firebase-installations/src/androidTest/java/com/google/firebase/installations/FirebaseInstallationsInstrumentedTest.java index ca6e34b761d..a886f3c24bb 100644 --- a/firebase-installations/src/androidTest/java/com/google/firebase/installations/FirebaseInstallationsInstrumentedTest.java +++ b/firebase-installations/src/androidTest/java/com/google/firebase/installations/FirebaseInstallationsInstrumentedTest.java @@ -527,7 +527,8 @@ public void testGetAuthToken_multipleCallsForceRefresh_fetchedNewTokenTwice() th (unused) -> InstallationTokenResult.builder() .setToken(TEST_AUTH_TOKEN_3) - .setTokenExpirationInSecs(TEST_TOKEN_EXPIRATION_TIMESTAMP) + .setTokenExpirationTimestamp(TEST_TOKEN_EXPIRATION_TIMESTAMP) + .setTokenCreationTimestamp(TEST_CREATION_TIMESTAMP_1) .build())) .doAnswer( AdditionalAnswers.answersWithDelay( @@ -535,7 +536,8 @@ public void testGetAuthToken_multipleCallsForceRefresh_fetchedNewTokenTwice() th (unused) -> InstallationTokenResult.builder() .setToken(TEST_AUTH_TOKEN_4) - .setTokenExpirationInSecs(TEST_TOKEN_EXPIRATION_TIMESTAMP) + .setTokenExpirationTimestamp(TEST_TOKEN_EXPIRATION_TIMESTAMP) + .setTokenCreationTimestamp(TEST_CREATION_TIMESTAMP_1) .build())) .when(backendClientReturnsOk) .generateAuthToken(anyString(), anyString(), anyString(), anyString()); diff --git a/firebase-installations/src/androidTest/java/com/google/firebase/installations/FisAndroidTestConstants.java b/firebase-installations/src/androidTest/java/com/google/firebase/installations/FisAndroidTestConstants.java index 5645e3a5c56..1728cb56554 100644 --- a/firebase-installations/src/androidTest/java/com/google/firebase/installations/FisAndroidTestConstants.java +++ b/firebase-installations/src/androidTest/java/com/google/firebase/installations/FisAndroidTestConstants.java @@ -49,13 +49,15 @@ public final class FisAndroidTestConstants { .setAuthToken( InstallationTokenResult.builder() .setToken(TEST_AUTH_TOKEN) - .setTokenExpirationInSecs(TEST_TOKEN_EXPIRATION_TIMESTAMP) + .setTokenExpirationTimestamp(TEST_TOKEN_EXPIRATION_TIMESTAMP) + .setTokenCreationTimestamp(TEST_CREATION_TIMESTAMP_1) .build()) .build(); public static final InstallationTokenResult TEST_INSTALLATION_TOKEN_RESULT = InstallationTokenResult.builder() .setToken(TEST_AUTH_TOKEN_2) - .setTokenExpirationInSecs(TEST_TOKEN_EXPIRATION_TIMESTAMP) + .setTokenExpirationTimestamp(TEST_TOKEN_EXPIRATION_TIMESTAMP) + .setTokenCreationTimestamp(TEST_CREATION_TIMESTAMP_1) .build(); } 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 76961076092..ce1f7de3ac3 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 @@ -285,7 +285,7 @@ private Void registerAndSaveFid(PersistedFidEntry persistedFidEntry) .setRegistrationStatus(RegistrationStatus.REGISTERED) .setAuthToken(installationResponse.getAuthToken().getToken()) .setRefreshToken(installationResponse.getRefreshToken()) - .setExpiresInSecs(installationResponse.getAuthToken().getTokenExpirationInSecs()) + .setExpiresInSecs(installationResponse.getAuthToken().getTokenExpirationTimestamp()) .setTokenCreationEpochInSecs(creationTime) .build()); @@ -314,7 +314,7 @@ private InstallationTokenResult fetchAuthTokenFromServer(PersistedFidEntry persi .setRegistrationStatus(RegistrationStatus.REGISTERED) .setAuthToken(tokenResult.getToken()) .setRefreshToken(persistedFidEntry.getRefreshToken()) - .setExpiresInSecs(tokenResult.getTokenExpirationInSecs()) + .setExpiresInSecs(tokenResult.getTokenExpirationTimestamp()) .setTokenCreationEpochInSecs(creationTime) .build()); diff --git a/firebase-installations/src/main/java/com/google/firebase/installations/GetAuthTokenListener.java b/firebase-installations/src/main/java/com/google/firebase/installations/GetAuthTokenListener.java index bc9348a344b..583710ccddb 100644 --- a/firebase-installations/src/main/java/com/google/firebase/installations/GetAuthTokenListener.java +++ b/firebase-installations/src/main/java/com/google/firebase/installations/GetAuthTokenListener.java @@ -37,7 +37,8 @@ public boolean onStateReached( resultTaskCompletionSource.setResult( InstallationTokenResult.builder() .setToken(persistedFidEntry.getAuthToken()) - .setTokenExpirationInSecs(persistedFidEntry.getExpiresInSecs()) + .setTokenExpirationTimestamp(persistedFidEntry.getExpiresInSecs()) + .setTokenCreationTimestamp(persistedFidEntry.getTokenCreationEpochInSecs()) .build()); return true; } diff --git a/firebase-installations/src/main/java/com/google/firebase/installations/remote/FirebaseInstallationServiceClient.java b/firebase-installations/src/main/java/com/google/firebase/installations/remote/FirebaseInstallationServiceClient.java index e1a6facd6f5..ddb43292471 100644 --- a/firebase-installations/src/main/java/com/google/firebase/installations/remote/FirebaseInstallationServiceClient.java +++ b/firebase-installations/src/main/java/com/google/firebase/installations/remote/FirebaseInstallationServiceClient.java @@ -279,7 +279,7 @@ private InstallationResponse readCreateResponse(HttpsURLConnection conn) throws if (key.equals("token")) { installationTokenResult.setToken(reader.nextString()); } else if (key.equals("expiresIn")) { - installationTokenResult.setTokenExpirationInSecs( + installationTokenResult.setTokenExpirationTimestamp( parseTokenExpirationTimestamp(reader.nextString())); } else { reader.skipValue(); @@ -308,7 +308,7 @@ private InstallationTokenResult readGenerateAuthTokenResponse(HttpsURLConnection if (name.equals("token")) { builder.setToken(reader.nextString()); } else if (name.equals("expiresIn")) { - builder.setTokenExpirationInSecs(parseTokenExpirationTimestamp(reader.nextString())); + builder.setTokenExpirationTimestamp(parseTokenExpirationTimestamp(reader.nextString())); } else { reader.skipValue(); } From 56d1eab24d597c1c3355c2073cda41492c9aa65a Mon Sep 17 00:00:00 2001 From: Ankita Date: Thu, 10 Oct 2019 13:11:38 -0700 Subject: [PATCH 15/15] Propagating the exceptions to the clients. (#856) * Propagating the exceptions to the clients. * Adding javadocs for StateListener class. --- .../FirebaseInstallationsInstrumentedTest.java | 2 +- .../installations/FirebaseInstallations.java | 15 ++++++++++++++- .../installations/GetAuthTokenListener.java | 9 +++++---- .../firebase/installations/GetIdListener.java | 11 ++++++----- .../firebase/installations/StateListener.java | 10 ++++++++++ 5 files changed, 36 insertions(+), 11 deletions(-) diff --git a/firebase-installations/src/androidTest/java/com/google/firebase/installations/FirebaseInstallationsInstrumentedTest.java b/firebase-installations/src/androidTest/java/com/google/firebase/installations/FirebaseInstallationsInstrumentedTest.java index a886f3c24bb..bd6ef7eb292 100644 --- a/firebase-installations/src/androidTest/java/com/google/firebase/installations/FirebaseInstallationsInstrumentedTest.java +++ b/firebase-installations/src/androidTest/java/com/google/firebase/installations/FirebaseInstallationsInstrumentedTest.java @@ -386,7 +386,7 @@ public void testGetAuthToken_PersistedFidError_failure() throws Exception { .isInstanceOf(FirebaseInstallationsException.class); assertWithMessage("Exception status doesn't match") .that(((FirebaseInstallationsException) expected.getCause()).getStatus()) - .isEqualTo(FirebaseInstallationsException.Status.SDK_INTERNAL_ERROR); + .isEqualTo(FirebaseInstallationsException.Status.CLIENT_ERROR); } } 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 ce1f7de3ac3..7eebd24933e 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 @@ -197,6 +197,19 @@ private void triggerOnStateReached(PersistedFidEntry persistedFidEntry) { } } + private void triggerOnException(PersistedFidEntry persistedFidEntry, Exception exception) { + synchronized (lock) { + Iterator it = listeners.iterator(); + while (it.hasNext()) { + StateListener l = it.next(); + boolean doneListening = l.onException(persistedFidEntry, exception); + if (doneListening) { + it.remove(); + } + } + } + } + private final void doRegistration() { try { PersistedFidEntry persistedFidEntry = persistedFid.readPersistedFidEntryValue(); @@ -248,7 +261,7 @@ private final void doRegistration() { .setRegistrationStatus(RegistrationStatus.REGISTER_ERROR) .build(); persistedFid.insertOrUpdatePersistedFidEntry(errorFidEntry); - triggerOnStateReached(errorFidEntry); + triggerOnException(errorFidEntry, e); } } diff --git a/firebase-installations/src/main/java/com/google/firebase/installations/GetAuthTokenListener.java b/firebase-installations/src/main/java/com/google/firebase/installations/GetAuthTokenListener.java index 583710ccddb..008daadbe65 100644 --- a/firebase-installations/src/main/java/com/google/firebase/installations/GetAuthTokenListener.java +++ b/firebase-installations/src/main/java/com/google/firebase/installations/GetAuthTokenListener.java @@ -42,12 +42,13 @@ public boolean onStateReached( .build()); return true; } + return false; + } + @Override + public boolean onException(PersistedFidEntry persistedFidEntry, Exception exception) { if (persistedFidEntry.isErrored()) { - resultTaskCompletionSource.setException( - new FirebaseInstallationsException( - "Firebase Installation is not registered.", - FirebaseInstallationsException.Status.SDK_INTERNAL_ERROR)); + resultTaskCompletionSource.trySetException(exception); return true; } return false; 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 b1b9036f0e0..2e24b97cc07 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 @@ -27,15 +27,16 @@ public GetIdListener(TaskCompletionSource taskCompletionSource) { @Override public boolean onStateReached(PersistedFidEntry persistedFidEntry, boolean unused) { if (persistedFidEntry.isUnregistered() || persistedFidEntry.isRegistered()) { - taskCompletionSource.setResult(persistedFidEntry.getFirebaseInstallationId()); + taskCompletionSource.trySetResult(persistedFidEntry.getFirebaseInstallationId()); return true; } + return false; + } + @Override + public boolean onException(PersistedFidEntry persistedFidEntry, Exception exception) { if (persistedFidEntry.isErrored()) { - taskCompletionSource.setException( - new FirebaseInstallationsException( - "Failed to update client side cache.", - FirebaseInstallationsException.Status.CLIENT_ERROR)); + taskCompletionSource.trySetException(exception); return true; } return false; diff --git a/firebase-installations/src/main/java/com/google/firebase/installations/StateListener.java b/firebase-installations/src/main/java/com/google/firebase/installations/StateListener.java index c839a107e87..a9691f63d18 100644 --- a/firebase-installations/src/main/java/com/google/firebase/installations/StateListener.java +++ b/firebase-installations/src/main/java/com/google/firebase/installations/StateListener.java @@ -17,5 +17,15 @@ import com.google.firebase.installations.local.PersistedFidEntry; interface StateListener { + /** + * Returns {@code true} if the defined {@link PersistedFidEntry} state is reached, {@code false} + * otherwise. + */ boolean onStateReached(PersistedFidEntry persistedFidEntry, boolean shouldRefreshAuthToken); + + /** + * Returns {@code true} if an exception is thrown while registering a Firebase Installation, + * {@code false} otherwise. + */ + boolean onException(PersistedFidEntry persistedFidEntry, Exception exception); }