diff --git a/firebase-installations/api.txt b/firebase-installations/api.txt index 4e676ce4849..e344cd2e484 100644 --- a/firebase-installations/api.txt +++ b/firebase-installations/api.txt @@ -45,6 +45,7 @@ package com.google.firebase.installations.local { method @Nullable public abstract String getAuthToken(); method public abstract long getExpiresInSecs(); method @Nullable public abstract String getFirebaseInstallationId(); + method @Nullable public abstract String getFisError(); method @Nullable public abstract String getRefreshToken(); method @NonNull public abstract com.google.firebase.installations.local.PersistedInstallation.RegistrationStatus getRegistrationStatus(); method public abstract long getTokenCreationEpochInSecs(); @@ -61,6 +62,7 @@ package com.google.firebase.installations.local { method @NonNull public abstract com.google.firebase.installations.local.PersistedInstallationEntry.Builder setAuthToken(@Nullable String); method @NonNull public abstract com.google.firebase.installations.local.PersistedInstallationEntry.Builder setExpiresInSecs(long); method @NonNull public abstract com.google.firebase.installations.local.PersistedInstallationEntry.Builder setFirebaseInstallationId(@NonNull String); + method @NonNull public abstract com.google.firebase.installations.local.PersistedInstallationEntry.Builder setFisError(@Nullable String); method @NonNull public abstract com.google.firebase.installations.local.PersistedInstallationEntry.Builder setRefreshToken(@Nullable String); method @NonNull public abstract com.google.firebase.installations.local.PersistedInstallationEntry.Builder setRegistrationStatus(@NonNull com.google.firebase.installations.local.PersistedInstallation.RegistrationStatus); method @NonNull public abstract com.google.firebase.installations.local.PersistedInstallationEntry.Builder setTokenCreationEpochInSecs(long); @@ -80,10 +82,11 @@ package com.google.firebase.installations.remote { public abstract class InstallationResponse { ctor public InstallationResponse(); method @NonNull public static com.google.firebase.installations.remote.InstallationResponse.Builder builder(); - method @NonNull public abstract InstallationTokenResult getAuthToken(); - method @NonNull public abstract String getFid(); - method @NonNull public abstract String getRefreshToken(); - method @NonNull public abstract String getUri(); + method @Nullable public abstract InstallationTokenResult getAuthToken(); + method @Nullable public abstract String getFid(); + method @Nullable public abstract String getRefreshToken(); + method @Nullable public abstract com.google.firebase.installations.remote.InstallationResponse.ResponseCode getResponseCode(); + method @Nullable public abstract String getUri(); method @NonNull public abstract com.google.firebase.installations.remote.InstallationResponse.Builder toBuilder(); } @@ -93,8 +96,14 @@ package com.google.firebase.installations.remote { method @NonNull public abstract com.google.firebase.installations.remote.InstallationResponse.Builder setAuthToken(@NonNull InstallationTokenResult); method @NonNull public abstract com.google.firebase.installations.remote.InstallationResponse.Builder setFid(@NonNull String); method @NonNull public abstract com.google.firebase.installations.remote.InstallationResponse.Builder setRefreshToken(@NonNull String); + method @NonNull public abstract com.google.firebase.installations.remote.InstallationResponse.Builder setResponseCode(@NonNull com.google.firebase.installations.remote.InstallationResponse.ResponseCode); method @NonNull public abstract com.google.firebase.installations.remote.InstallationResponse.Builder setUri(@NonNull String); } + public enum InstallationResponse.ResponseCode { + enum_constant public static final com.google.firebase.installations.remote.InstallationResponse.ResponseCode OK; + enum_constant public static final com.google.firebase.installations.remote.InstallationResponse.ResponseCode SERVER_ERROR; + } + } 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 d64d34ffc72..caa4eea58ff 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 @@ -17,6 +17,7 @@ import static com.google.common.truth.Truth.assertWithMessage; import static com.google.firebase.installations.FisAndroidTestConstants.DEFAULT_PERSISTED_INSTALLATION_ENTRY; import static com.google.firebase.installations.FisAndroidTestConstants.INVALID_TEST_FID; +import static com.google.firebase.installations.FisAndroidTestConstants.SERVER_ERROR_INSTALLATION_RESPONSE; 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; @@ -291,6 +292,32 @@ public void testGetId_PersistedInstallationOk_BackendError() throws Exception { assertThat(updatedInstallationEntry).hasRegistrationStatus(RegistrationStatus.REGISTER_ERROR); } + @Test + public void testGetId_ServerError_UnregisteredFID() throws Exception { + // Mocking server error on FIS createFirebaseInstallation, returns empty InstallationResponse + when(backendClientReturnsOk.createFirebaseInstallation( + anyString(), anyString(), anyString(), anyString())) + .thenReturn(SERVER_ERROR_INSTALLATION_RESPONSE); + + FirebaseInstallations firebaseInstallations = + new FirebaseInstallations( + executor, firebaseApp, backendClientReturnsOk, persistedInstallation, mockUtils); + + Tasks.await(firebaseInstallations.getId()); + + PersistedInstallationEntry entryValue = + persistedInstallation.readPersistedInstallationEntryValue(); + assertThat(entryValue).hasFid(TEST_FID_1); + + // Waiting for Task that registers FID on the FIS Servers + executor.awaitTermination(500, TimeUnit.MILLISECONDS); + + PersistedInstallationEntry updatedInstallationEntry = + persistedInstallation.readPersistedInstallationEntryValue(); + assertThat(updatedInstallationEntry).hasFid(TEST_FID_1); + assertThat(updatedInstallationEntry).hasRegistrationStatus(RegistrationStatus.UNREGISTERED); + } + @Test public void testGetId_PersistedInstallationError_BackendOk() throws InterruptedException { FirebaseInstallations firebaseInstallations = 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 d94cdcc214c..1f17b79aee9 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 @@ -16,6 +16,7 @@ import com.google.firebase.installations.local.PersistedInstallationEntry; import com.google.firebase.installations.remote.InstallationResponse; +import com.google.firebase.installations.remote.InstallationResponse.ResponseCode; public final class FisAndroidTestConstants { public static final String TEST_FID_1 = "cccccccccccccccccccccc"; @@ -55,6 +56,7 @@ public final class FisAndroidTestConstants { .setTokenExpirationTimestamp(TEST_TOKEN_EXPIRATION_TIMESTAMP) .setTokenCreationTimestamp(TEST_CREATION_TIMESTAMP_1) .build()) + .setResponseCode(ResponseCode.OK) .build(); public static final InstallationTokenResult TEST_INSTALLATION_TOKEN_RESULT = @@ -63,4 +65,7 @@ public final class FisAndroidTestConstants { .setTokenExpirationTimestamp(TEST_TOKEN_EXPIRATION_TIMESTAMP) .setTokenCreationTimestamp(TEST_CREATION_TIMESTAMP_1) .build(); + + public static final InstallationResponse SERVER_ERROR_INSTALLATION_RESPONSE = + InstallationResponse.builder().setResponseCode(ResponseCode.SERVER_ERROR).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 709b78637a8..a29fa0e7d9e 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 @@ -29,6 +29,7 @@ import com.google.firebase.installations.local.PersistedInstallationEntry; import com.google.firebase.installations.remote.FirebaseInstallationServiceClient; import com.google.firebase.installations.remote.InstallationResponse; +import com.google.firebase.installations.remote.InstallationResponse.ResponseCode; import java.util.ArrayList; import java.util.Iterator; import java.util.List; @@ -218,12 +219,18 @@ private final void doRegistration() { persistedInstallation.readPersistedInstallationEntryValue(); // New FID needs to be created - if (persistedInstallationEntry.isErrored() || persistedInstallationEntry.isNotGenerated()) { + if (persistedInstallationEntry.isNotGenerated()) { String fid = utils.createRandomFid(); persistFid(fid); persistedInstallationEntry = persistedInstallation.readPersistedInstallationEntryValue(); } + if (persistedInstallationEntry.isErrored()) { + throw new FirebaseInstallationsException( + persistedInstallationEntry.getFisError(), + FirebaseInstallationsException.Status.SDK_INTERNAL_ERROR); + } + triggerOnStateReached(persistedInstallationEntry); // FID needs to be registered @@ -262,6 +269,7 @@ private final void doRegistration() { PersistedInstallationEntry errorInstallationEntry = persistedInstallationEntry .toBuilder() + .setFisError(e.getMessage()) .setRegistrationStatus(RegistrationStatus.REGISTER_ERROR) .build(); persistedInstallation.insertOrUpdatePersistedInstallationEntry(errorInstallationEntry); @@ -296,15 +304,17 @@ private Void registerAndSaveFid(PersistedInstallationEntry persistedInstallation /*fid= */ persistedInstallationEntry.getFirebaseInstallationId(), /*projectID= */ firebaseApp.getOptions().getProjectId(), /*appId= */ getApplicationId()); - persistedInstallation.insertOrUpdatePersistedInstallationEntry( - PersistedInstallationEntry.builder() - .setFirebaseInstallationId(installationResponse.getFid()) - .setRegistrationStatus(RegistrationStatus.REGISTERED) - .setAuthToken(installationResponse.getAuthToken().getToken()) - .setRefreshToken(installationResponse.getRefreshToken()) - .setExpiresInSecs(installationResponse.getAuthToken().getTokenExpirationTimestamp()) - .setTokenCreationEpochInSecs(creationTime) - .build()); + if (installationResponse.getResponseCode() == ResponseCode.OK) { + persistedInstallation.insertOrUpdatePersistedInstallationEntry( + PersistedInstallationEntry.builder() + .setFirebaseInstallationId(installationResponse.getFid()) + .setRegistrationStatus(RegistrationStatus.REGISTERED) + .setAuthToken(installationResponse.getAuthToken().getToken()) + .setRefreshToken(installationResponse.getRefreshToken()) + .setExpiresInSecs(installationResponse.getAuthToken().getTokenExpirationTimestamp()) + .setTokenCreationEpochInSecs(creationTime) + .build()); + } } catch (FirebaseException exception) { throw new FirebaseInstallationsException( 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 03fd022befd..d367ed6e230 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 @@ -53,9 +53,10 @@ class Utils { * #AUTH_TOKEN_EXPIRATION_BUFFER_IN_SECS}. */ public boolean isAuthTokenExpired(PersistedInstallationEntry persistedInstallationEntry) { - return persistedInstallationEntry.getTokenCreationEpochInSecs() - + persistedInstallationEntry.getExpiresInSecs() - > currentTimeInSecs() + AUTH_TOKEN_EXPIRATION_BUFFER_IN_SECS; + return persistedInstallationEntry.isRegistered() + && persistedInstallationEntry.getTokenCreationEpochInSecs() + + persistedInstallationEntry.getExpiresInSecs() + > currentTimeInSecs() + AUTH_TOKEN_EXPIRATION_BUFFER_IN_SECS; } /** Returns current time in seconds. */ diff --git a/firebase-installations/src/main/java/com/google/firebase/installations/local/PersistedInstallationEntry.java b/firebase-installations/src/main/java/com/google/firebase/installations/local/PersistedInstallationEntry.java index e7d66ae914f..aa2ffee2e58 100644 --- a/firebase-installations/src/main/java/com/google/firebase/installations/local/PersistedInstallationEntry.java +++ b/firebase-installations/src/main/java/com/google/firebase/installations/local/PersistedInstallationEntry.java @@ -41,6 +41,9 @@ public abstract class PersistedInstallationEntry { public abstract long getTokenCreationEpochInSecs(); + @Nullable + public abstract String getFisError(); + public boolean isRegistered() { return getRegistrationStatus() == PersistedInstallation.RegistrationStatus.REGISTERED; } @@ -90,6 +93,9 @@ public abstract Builder setRegistrationStatus( @NonNull public abstract Builder setTokenCreationEpochInSecs(long value); + @NonNull + public abstract Builder setFisError(@Nullable String value); + @NonNull public abstract PersistedInstallationEntry build(); } 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 6a1bafba6f5..65a26534623 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 @@ -27,6 +27,7 @@ import com.google.android.gms.common.util.VisibleForTesting; import com.google.firebase.FirebaseException; import com.google.firebase.installations.InstallationTokenResult; +import com.google.firebase.installations.remote.InstallationResponse.ResponseCode; import java.io.BufferedReader; import java.io.IOException; import java.io.InputStreamReader; @@ -118,17 +119,21 @@ public InstallationResponse createFirebaseInstallation( } int httpResponseCode = httpsURLConnection.getResponseCode(); - switch (httpResponseCode) { - case 200: - return readCreateResponse(httpsURLConnection); - case 500: - retryCount++; - break; - default: - throw new FirebaseException(readErrorResponse(httpsURLConnection)); + + if (httpResponseCode == 200) { + return readCreateResponse(httpsURLConnection); + } + // Usually the FIS server recovers from errors: retry one time before giving up. + if (httpResponseCode >= 500 && httpResponseCode < 600) { + retryCount++; + continue; } + + // Unrecoverable server response or unknown error + throw new FirebaseException(readErrorResponse(httpsURLConnection)); } - throw new FirebaseException(INTERNAL_SERVER_ERROR_MESSAGE); + // Return empty installation response with SERVER_ERROR response code after max retries + return InstallationResponse.builder().setResponseCode(ResponseCode.SERVER_ERROR).build(); } catch (IOException e) { throw new FirebaseException(String.format(NETWORK_ERROR_MESSAGE, e.getMessage())); } @@ -219,15 +224,18 @@ public InstallationTokenResult generateAuthToken( httpsURLConnection.addRequestProperty("Authorization", "FIS_v2 " + refreshToken); int httpResponseCode = httpsURLConnection.getResponseCode(); - switch (httpResponseCode) { - case 200: - return readGenerateAuthTokenResponse(httpsURLConnection); - case 500: - retryCount++; - break; - default: - throw new FirebaseException(readErrorResponse(httpsURLConnection)); + + if (httpResponseCode == 200) { + return readGenerateAuthTokenResponse(httpsURLConnection); + } + // Usually the FIS server recovers from errors: retry one time before giving up. + if (httpResponseCode >= 500 && httpResponseCode < 600) { + retryCount++; + continue; } + + // Unrecoverable server response or unknown error + throw new FirebaseException(readErrorResponse(httpsURLConnection)); } throw new FirebaseException(INTERNAL_SERVER_ERROR_MESSAGE); } catch (IOException e) { @@ -283,7 +291,7 @@ private InstallationResponse readCreateResponse(HttpsURLConnection conn) throws } reader.endObject(); - return builder.build(); + return builder.setResponseCode(ResponseCode.OK).build(); } // Read the response from the generateAuthToken FirebaseInstallation API. diff --git a/firebase-installations/src/main/java/com/google/firebase/installations/remote/InstallationResponse.java b/firebase-installations/src/main/java/com/google/firebase/installations/remote/InstallationResponse.java index c0c04a33be9..8bea66ff2e4 100644 --- a/firebase-installations/src/main/java/com/google/firebase/installations/remote/InstallationResponse.java +++ b/firebase-installations/src/main/java/com/google/firebase/installations/remote/InstallationResponse.java @@ -15,24 +15,35 @@ package com.google.firebase.installations.remote; import androidx.annotation.NonNull; +import androidx.annotation.Nullable; import com.google.auto.value.AutoValue; import com.google.firebase.installations.InstallationTokenResult; @AutoValue public abstract class InstallationResponse { - @NonNull + public enum ResponseCode { + // Returned on success + OK, + // An error occurred on the server while processing this request(temporary) + SERVER_ERROR + } + + @Nullable public abstract String getUri(); - @NonNull + @Nullable public abstract String getFid(); - @NonNull + @Nullable public abstract String getRefreshToken(); - @NonNull + @Nullable public abstract InstallationTokenResult getAuthToken(); + @Nullable + public abstract ResponseCode getResponseCode(); + @NonNull public abstract Builder toBuilder(); @@ -56,6 +67,9 @@ public abstract static class Builder { @NonNull public abstract Builder setAuthToken(@NonNull InstallationTokenResult value); + @NonNull + public abstract Builder setResponseCode(@NonNull ResponseCode value); + @NonNull public abstract InstallationResponse build(); }