Skip to content

Implementing retry once for FIS Client. #895

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Oct 14, 2019
Merged

Implementing retry once for FIS Client. #895

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
c8367dc
Implemneting retry for FIS Client.
ankitaj224 Oct 10, 2019
72e335d
Merge branch 'listeners' of github.com:firebase/firebase-android-sdk …
ankitaj224 Oct 10, 2019
12ae11d
Simplifying FirebaseInstallations class by adding listeners. (#847)
ankitaj224 Oct 10, 2019
560fba1
Addressing ciaran's comments.
ankitaj224 Oct 10, 2019
3ac3da2
Merge branch 'fis_sdk' of github.com:firebase/firebase-android-sdk in…
ankitaj224 Oct 10, 2019
d9e5c22
Merge conflicts.
ankitaj224 Oct 10, 2019
f57137c
Nit fixes
ankitaj224 Oct 10, 2019
0c0b9c2
Merge branch 'listeners' of github.com:firebase/firebase-android-sdk …
ankitaj224 Oct 14, 2019
0407614
Fixing verify format issue.
ankitaj224 Oct 14, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 3 additions & 16 deletions firebase-installations/api.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,22 +72,9 @@ package com.google.firebase.installations.remote {

public class FirebaseInstallationServiceClient {
ctor public FirebaseInstallationServiceClient(@NonNull Context);
method @NonNull public com.google.firebase.installations.remote.InstallationResponse createFirebaseInstallation(@NonNull String, @NonNull String, @NonNull String, @NonNull String) throws com.google.firebase.installations.remote.FirebaseInstallationServiceException;
method @NonNull public void deleteFirebaseInstallation(@NonNull String, @NonNull String, @NonNull String, @NonNull String) throws com.google.firebase.installations.remote.FirebaseInstallationServiceException;
method @NonNull public InstallationTokenResult generateAuthToken(@NonNull String, @NonNull String, @NonNull String, @NonNull String) throws com.google.firebase.installations.remote.FirebaseInstallationServiceException;
}

public class FirebaseInstallationServiceException {
ctor public FirebaseInstallationServiceException(@NonNull com.google.firebase.installations.remote.FirebaseInstallationServiceException.Status);
ctor public FirebaseInstallationServiceException(@NonNull String, @NonNull com.google.firebase.installations.remote.FirebaseInstallationServiceException.Status);
ctor public FirebaseInstallationServiceException(@NonNull String, @NonNull com.google.firebase.installations.remote.FirebaseInstallationServiceException.Status, @NonNull Throwable);
method @NonNull public com.google.firebase.installations.remote.FirebaseInstallationServiceException.Status getStatus();
}

public enum FirebaseInstallationServiceException.Status {
enum_constant public static final com.google.firebase.installations.remote.FirebaseInstallationServiceException.Status NETWORK_ERROR;
enum_constant public static final com.google.firebase.installations.remote.FirebaseInstallationServiceException.Status SERVER_ERROR;
enum_constant public static final com.google.firebase.installations.remote.FirebaseInstallationServiceException.Status UNAUTHORIZED;
method @NonNull public com.google.firebase.installations.remote.InstallationResponse createFirebaseInstallation(@NonNull String, @NonNull String, @NonNull String, @NonNull String);
method @NonNull public void deleteFirebaseInstallation(@NonNull String, @NonNull String, @NonNull String, @NonNull String);
method @NonNull public InstallationTokenResult generateAuthToken(@NonNull String, @NonNull String, @NonNull String, @NonNull String);
}

public abstract class InstallationResponse {
Expand Down
16 changes: 5 additions & 11 deletions ...oidTest/java/com/google/firebase/installations/FirebaseInstallationsInstrumentedTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,12 @@
import com.google.android.gms.tasks.Task;
import com.google.android.gms.tasks.Tasks;
import com.google.firebase.FirebaseApp;
import com.google.firebase.FirebaseException;
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 java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.LinkedBlockingQueue;
Expand Down Expand Up @@ -128,7 +128,7 @@ public class FirebaseInstallationsInstrumentedTest {
.build();

@Before
public void setUp() throws FirebaseInstallationServiceException {
public void setUp() throws FirebaseException {
MockitoAnnotations.initMocks(this);
FirebaseApp.clearInstancesForTest();
executor = new ThreadPoolExecutor(0, 1, 30L, TimeUnit.SECONDS, new LinkedBlockingQueue<>());
Expand Down Expand Up @@ -156,9 +156,7 @@ public void setUp() throws FirebaseInstallationServiceException {

when(backendClientReturnsError.createFirebaseInstallation(
anyString(), anyString(), anyString(), anyString()))
.thenThrow(
new FirebaseInstallationServiceException(
"SDK Error", FirebaseInstallationServiceException.Status.SERVER_ERROR));
.thenThrow(new FirebaseException("SDK Error"));

when(mockUtils.createRandomFid()).thenReturn(TEST_FID_1);
when(mockUtils.currentTimeInSecs()).thenReturn(TEST_CREATION_TIMESTAMP_2);
Expand All @@ -168,9 +166,7 @@ public void setUp() throws FirebaseInstallationServiceException {
.when(backendClientReturnsOk)
.deleteFirebaseInstallation(anyString(), anyString(), anyString(), anyString());
// Mocks server error on FIS deletion
doThrow(
new FirebaseInstallationServiceException(
"Server Error", FirebaseInstallationServiceException.Status.SERVER_ERROR))
doThrow(new FirebaseException("Server Error"))
.when(backendClientReturnsError)
.deleteFirebaseInstallation(anyString(), anyString(), anyString(), anyString());
}
Expand Down Expand Up @@ -458,9 +454,7 @@ public void testGetAuthToken_serverError_failure() throws Exception {
when(mockPersistedFid.readPersistedFidEntryValue()).thenReturn(REGISTERED_FID_ENTRY);
when(backendClientReturnsError.generateAuthToken(
anyString(), anyString(), anyString(), anyString()))
.thenThrow(
new FirebaseInstallationServiceException(
"Server Error", FirebaseInstallationServiceException.Status.SERVER_ERROR));
.thenThrow(new FirebaseException("Server Error"));
when(mockUtils.isAuthTokenExpired(REGISTERED_FID_ENTRY)).thenReturn(false);

FirebaseInstallations firebaseInstallations =
Expand Down
8 changes: 4 additions & 4 deletions ...-installations/src/main/java/com/google/firebase/installations/FirebaseInstallations.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,11 @@
import com.google.android.gms.tasks.TaskCompletionSource;
import com.google.android.gms.tasks.Tasks;
import com.google.firebase.FirebaseApp;
import com.google.firebase.FirebaseException;
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.ArrayList;
import java.util.Iterator;
Expand Down Expand Up @@ -302,7 +302,7 @@ private Void registerAndSaveFid(PersistedFidEntry persistedFidEntry)
.setTokenCreationEpochInSecs(creationTime)
.build());

} catch (FirebaseInstallationServiceException exception) {
} catch (FirebaseException exception) {
throw new FirebaseInstallationsException(
exception.getMessage(), FirebaseInstallationsException.Status.SDK_INTERNAL_ERROR);
}
Expand Down Expand Up @@ -332,7 +332,7 @@ private InstallationTokenResult fetchAuthTokenFromServer(PersistedFidEntry persi
.build());

return tokenResult;
} catch (FirebaseInstallationServiceException exception) {
} catch (FirebaseException exception) {
throw new FirebaseInstallationsException(
"Failed to generate auth token for a Firebase Installation.",
FirebaseInstallationsException.Status.SDK_INTERNAL_ERROR);
Expand All @@ -356,7 +356,7 @@ private Void deleteFirebaseInstallationId() throws FirebaseInstallationsExceptio
firebaseApp.getOptions().getProjectId(),
persistedFidEntry.getRefreshToken());

} catch (FirebaseInstallationServiceException exception) {
} catch (FirebaseException exception) {
throw new FirebaseInstallationsException(
"Failed to delete a Firebase Installation.",
FirebaseInstallationsException.Status.SDK_INTERNAL_ERROR);
Expand Down
187 changes: 98 additions & 89 deletions ...main/java/com/google/firebase/installations/remote/FirebaseInstallationServiceClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@
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.FirebaseException;
import com.google.firebase.installations.InstallationTokenResult;
import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;
import java.net.URL;
Expand Down Expand Up @@ -53,8 +55,6 @@ public class FirebaseInstallationServiceClient {
private static final String CONTENT_ENCODING_HEADER_KEY = "Content-Encoding";
private static final String GZIP_CONTENT_ENCODING = "gzip";

private static final String UNAUTHORIZED_ERROR_MESSAGE =
"The request did not have the required credentials.";
private static final String INTERNAL_SERVER_ERROR_MESSAGE = "There was an internal server error.";
private static final String NETWORK_ERROR_MESSAGE = "The server returned an unexpected error:";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing a space between the colon and the error when you concat using +. It would be slightly less error prone to use either a format string here or have a dedicated method for generating these errors (or both)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated. PTAL.


Expand All @@ -65,6 +65,8 @@ public class FirebaseInstallationServiceClient {

private static final Pattern EXPIRATION_TIMESTAMP_PATTERN = Pattern.compile("[0-9]+s");

private static final int MAX_RETRIES = 1;

@VisibleForTesting
static final String PARSING_EXPIRATION_TIME_ERROR_MESSAGE = "Invalid Expiration Timestamp.";

Expand All @@ -86,57 +88,58 @@ public FirebaseInstallationServiceClient(@NonNull Context context) {
@NonNull
public InstallationResponse createFirebaseInstallation(
@NonNull String apiKey, @NonNull String fid, @NonNull String projectID, @NonNull String appId)
throws FirebaseInstallationServiceException {
throws FirebaseException {
String resourceName = String.format(CREATE_REQUEST_RESOURCE_NAME_FORMAT, projectID);
try {
URL url =
new URL(
String.format(
"https://%s/%s/%s?key=%s",
FIREBASE_INSTALLATIONS_API_DOMAIN,
FIREBASE_INSTALLATIONS_API_VERSION,
resourceName,
apiKey));
int retryCount = 0;
while (retryCount <= MAX_RETRIES) {
URL url =
new URL(
String.format(
"https://%s/%s/%s?key=%s",
FIREBASE_INSTALLATIONS_API_DOMAIN,
FIREBASE_INSTALLATIONS_API_VERSION,
resourceName,
apiKey));

HttpsURLConnection httpsURLConnection = (HttpsURLConnection) url.openConnection();
httpsURLConnection.setConnectTimeout(NETWORK_TIMEOUT_MILLIS);
httpsURLConnection.setReadTimeout(NETWORK_TIMEOUT_MILLIS);
httpsURLConnection.setDoOutput(true);
httpsURLConnection.setRequestMethod("POST");
httpsURLConnection.addRequestProperty(CONTENT_TYPE_HEADER_KEY, JSON_CONTENT_TYPE);
httpsURLConnection.addRequestProperty(ACCEPT_HEADER_KEY, JSON_CONTENT_TYPE);
httpsURLConnection.addRequestProperty(CONTENT_ENCODING_HEADER_KEY, GZIP_CONTENT_ENCODING);
httpsURLConnection.addRequestProperty(X_ANDROID_PACKAGE_HEADER_KEY, context.getPackageName());
httpsURLConnection.addRequestProperty(
X_ANDROID_CERT_HEADER_KEY, getFingerprintHashForPackage());

GZIPOutputStream gzipOutputStream =
new GZIPOutputStream(httpsURLConnection.getOutputStream());
try {
gzipOutputStream.write(
buildCreateFirebaseInstallationRequestBody(fid, appId).toString().getBytes("UTF-8"));
} catch (JSONException e) {
throw new IllegalStateException(e);
} finally {
gzipOutputStream.close();
}
HttpsURLConnection httpsURLConnection = (HttpsURLConnection) url.openConnection();
httpsURLConnection.setConnectTimeout(NETWORK_TIMEOUT_MILLIS);
httpsURLConnection.setReadTimeout(NETWORK_TIMEOUT_MILLIS);
httpsURLConnection.setDoOutput(true);
httpsURLConnection.setRequestMethod("POST");
httpsURLConnection.addRequestProperty(CONTENT_TYPE_HEADER_KEY, JSON_CONTENT_TYPE);
httpsURLConnection.addRequestProperty(ACCEPT_HEADER_KEY, JSON_CONTENT_TYPE);
httpsURLConnection.addRequestProperty(CONTENT_ENCODING_HEADER_KEY, GZIP_CONTENT_ENCODING);
httpsURLConnection.addRequestProperty(
X_ANDROID_PACKAGE_HEADER_KEY, context.getPackageName());
httpsURLConnection.addRequestProperty(
X_ANDROID_CERT_HEADER_KEY, getFingerprintHashForPackage());

int httpResponseCode = httpsURLConnection.getResponseCode();
switch (httpResponseCode) {
case 200:
return readCreateResponse(httpsURLConnection);
case 401:
throw new FirebaseInstallationServiceException(
UNAUTHORIZED_ERROR_MESSAGE, FirebaseInstallationServiceException.Status.UNAUTHORIZED);
default:
throw new FirebaseInstallationServiceException(
INTERNAL_SERVER_ERROR_MESSAGE,
FirebaseInstallationServiceException.Status.SERVER_ERROR);
GZIPOutputStream gzipOutputStream =
new GZIPOutputStream(httpsURLConnection.getOutputStream());
try {
gzipOutputStream.write(
buildCreateFirebaseInstallationRequestBody(fid, appId).toString().getBytes("UTF-8"));
} catch (JSONException e) {
throw new IllegalStateException(e);
} finally {
gzipOutputStream.close();
}

int httpResponseCode = httpsURLConnection.getResponseCode();
switch (httpResponseCode) {
case 200:
return readCreateResponse(httpsURLConnection);
case 500:
retryCount++;
break;
default:
throw new FirebaseException(readErrorResponse(httpsURLConnection));
}
}
throw new FirebaseException(INTERNAL_SERVER_ERROR_MESSAGE);
} catch (IOException e) {
throw new FirebaseInstallationServiceException(
NETWORK_ERROR_MESSAGE + e.getMessage(),
FirebaseInstallationServiceException.Status.NETWORK_ERROR);
throw new FirebaseException(NETWORK_ERROR_MESSAGE + e.getMessage());
}
}

Expand All @@ -163,7 +166,7 @@ public void deleteFirebaseInstallation(
@NonNull String fid,
@NonNull String projectID,
@NonNull String refreshToken)
throws FirebaseInstallationServiceException {
throws FirebaseException {
String resourceName = String.format(DELETE_REQUEST_RESOURCE_NAME_FORMAT, projectID, fid);
try {
URL url =
Expand All @@ -188,18 +191,11 @@ public void deleteFirebaseInstallation(
switch (httpResponseCode) {
case 200:
return;
case 401:
throw new FirebaseInstallationServiceException(
UNAUTHORIZED_ERROR_MESSAGE, FirebaseInstallationServiceException.Status.UNAUTHORIZED);
default:
throw new FirebaseInstallationServiceException(
INTERNAL_SERVER_ERROR_MESSAGE,
FirebaseInstallationServiceException.Status.SERVER_ERROR);
throw new FirebaseException(readErrorResponse(httpsURLConnection));
}
} catch (IOException e) {
throw new FirebaseInstallationServiceException(
NETWORK_ERROR_MESSAGE + e.getMessage(),
FirebaseInstallationServiceException.Status.NETWORK_ERROR);
throw new FirebaseException(NETWORK_ERROR_MESSAGE + e.getMessage());
}
}

Expand All @@ -218,45 +214,45 @@ public InstallationTokenResult generateAuthToken(
@NonNull String fid,
@NonNull String projectID,
@NonNull String refreshToken)
throws FirebaseInstallationServiceException {
throws FirebaseException {
String resourceName =
String.format(GENERATE_AUTH_TOKEN_REQUEST_RESOURCE_NAME_FORMAT, projectID, fid);
try {
URL url =
new URL(
String.format(
"https://%s/%s/%s?key=%s",
FIREBASE_INSTALLATIONS_API_DOMAIN,
FIREBASE_INSTALLATIONS_API_VERSION,
resourceName,
apiKey));
int retryCount = 0;
while (retryCount <= MAX_RETRIES) {
URL url =
new URL(
String.format(
"https://%s/%s/%s?key=%s",
FIREBASE_INSTALLATIONS_API_DOMAIN,
FIREBASE_INSTALLATIONS_API_VERSION,
resourceName,
apiKey));

HttpsURLConnection httpsURLConnection = (HttpsURLConnection) url.openConnection();
httpsURLConnection.setConnectTimeout(NETWORK_TIMEOUT_MILLIS);
httpsURLConnection.setReadTimeout(NETWORK_TIMEOUT_MILLIS);
httpsURLConnection.setDoOutput(true);
httpsURLConnection.setRequestMethod("POST");
httpsURLConnection.addRequestProperty("Authorization", "FIS_v2 " + refreshToken);
httpsURLConnection.addRequestProperty(CONTENT_TYPE_HEADER_KEY, JSON_CONTENT_TYPE);
httpsURLConnection.addRequestProperty(ACCEPT_HEADER_KEY, JSON_CONTENT_TYPE);
httpsURLConnection.addRequestProperty(CONTENT_ENCODING_HEADER_KEY, GZIP_CONTENT_ENCODING);
HttpsURLConnection httpsURLConnection = (HttpsURLConnection) url.openConnection();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there's a lot of overlap between this method and the other one that sets up a POST request. Can we maybe consolidate some of the shared logic?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

httpsURLConnection.setConnectTimeout(NETWORK_TIMEOUT_MILLIS);
httpsURLConnection.setReadTimeout(NETWORK_TIMEOUT_MILLIS);
httpsURLConnection.setDoOutput(true);
httpsURLConnection.setRequestMethod("POST");
httpsURLConnection.addRequestProperty("Authorization", "FIS_v2 " + refreshToken);
httpsURLConnection.addRequestProperty(CONTENT_TYPE_HEADER_KEY, JSON_CONTENT_TYPE);
httpsURLConnection.addRequestProperty(ACCEPT_HEADER_KEY, JSON_CONTENT_TYPE);
httpsURLConnection.addRequestProperty(CONTENT_ENCODING_HEADER_KEY, GZIP_CONTENT_ENCODING);

int httpResponseCode = httpsURLConnection.getResponseCode();
switch (httpResponseCode) {
case 200:
return readGenerateAuthTokenResponse(httpsURLConnection);
case 401:
throw new FirebaseInstallationServiceException(
UNAUTHORIZED_ERROR_MESSAGE, FirebaseInstallationServiceException.Status.UNAUTHORIZED);
default:
throw new FirebaseInstallationServiceException(
INTERNAL_SERVER_ERROR_MESSAGE,
FirebaseInstallationServiceException.Status.SERVER_ERROR);
int httpResponseCode = httpsURLConnection.getResponseCode();
switch (httpResponseCode) {
case 200:
return readGenerateAuthTokenResponse(httpsURLConnection);
case 500:
retryCount++;
break;
default:
throw new FirebaseException(readErrorResponse(httpsURLConnection));
}
}
throw new FirebaseException(INTERNAL_SERVER_ERROR_MESSAGE);
} catch (IOException e) {
throw new FirebaseInstallationServiceException(
NETWORK_ERROR_MESSAGE + e.getMessage(),
FirebaseInstallationServiceException.Status.NETWORK_ERROR);
throw new FirebaseException(NETWORK_ERROR_MESSAGE + e.getMessage());
}
}
// Read the response from the createFirebaseInstallation API.
Expand Down Expand Up @@ -318,6 +314,19 @@ private InstallationTokenResult readGenerateAuthTokenResponse(HttpsURLConnection
return builder.build();
}

// Read the error message from the response.
private String readErrorResponse(HttpsURLConnection conn) throws IOException {
BufferedReader reader =
new BufferedReader(new InputStreamReader(conn.getErrorStream(), Charset.defaultCharset()));
StringBuilder response = new StringBuilder();
for (String input = reader.readLine(); input != null; input = reader.readLine()) {
response.append(input);
}
return String.format(
"The server responded with an error. HTTP response:[%d %s %s]",
conn.getResponseCode(), conn.getResponseMessage(), response);
}

/** Gets the Android package's SHA-1 fingerprint. */
private String getFingerprintHashForPackage() {
byte[] hash;
Expand Down
Loading