From 157e6937e6770ec0ea83200e398283a73df2f8de Mon Sep 17 00:00:00 2001 From: hiranya911 Date: Thu, 13 Feb 2020 15:27:14 -0800 Subject: [PATCH 1/3] Error handling revamp for the project management API --- .../internal/ErrorHandlingHttpClient.java | 2 +- .../FirebaseProjectManagementException.java | 21 ++- .../FirebaseProjectManagementServiceImpl.java | 59 ++++--- .../projectmanagement/HttpHelper.java | 153 ++++++------------ ...ebaseProjectManagementServiceImplTest.java | 133 ++++++++++++++- .../FirebaseProjectManagementTest.java | 3 +- .../projectmanagement/IosAppTest.java | 3 +- 7 files changed, 236 insertions(+), 138 deletions(-) diff --git a/src/main/java/com/google/firebase/internal/ErrorHandlingHttpClient.java b/src/main/java/com/google/firebase/internal/ErrorHandlingHttpClient.java index e7d9e7373..f533f1315 100644 --- a/src/main/java/com/google/firebase/internal/ErrorHandlingHttpClient.java +++ b/src/main/java/com/google/firebase/internal/ErrorHandlingHttpClient.java @@ -116,7 +116,7 @@ public V parse(IncomingHttpResponse response, Class responseType) throws } } - private void parse(IncomingHttpResponse response, Object destination) throws T { + public void parse(IncomingHttpResponse response, Object destination) throws T { try { JsonParser parser = jsonFactory.createJsonParser(response.getContent()); parser.parse(destination); diff --git a/src/main/java/com/google/firebase/projectmanagement/FirebaseProjectManagementException.java b/src/main/java/com/google/firebase/projectmanagement/FirebaseProjectManagementException.java index fa939b0c4..361ed2700 100644 --- a/src/main/java/com/google/firebase/projectmanagement/FirebaseProjectManagementException.java +++ b/src/main/java/com/google/firebase/projectmanagement/FirebaseProjectManagementException.java @@ -15,18 +15,27 @@ package com.google.firebase.projectmanagement; +import com.google.firebase.ErrorCode; import com.google.firebase.FirebaseException; -import com.google.firebase.internal.Nullable; +import com.google.firebase.IncomingHttpResponse; +import com.google.firebase.database.annotations.Nullable; +import com.google.firebase.internal.NonNull; /** * An exception encountered while interacting with the Firebase Project Management Service. */ -public class FirebaseProjectManagementException extends FirebaseException { - FirebaseProjectManagementException(String detailMessage) { - this(detailMessage, null); +public final class FirebaseProjectManagementException extends FirebaseException { + + FirebaseProjectManagementException(@NonNull FirebaseException base) { + this(base, base.getMessage()); + } + + FirebaseProjectManagementException(@NonNull FirebaseException base, @NonNull String message) { + super(base.getErrorCodeNew(), message, base.getCause(), base.getHttpResponse()); } - FirebaseProjectManagementException(String detailMessage, @Nullable Throwable cause) { - super(detailMessage, cause); + FirebaseProjectManagementException( + @NonNull ErrorCode code, @NonNull String message, @Nullable IncomingHttpResponse response) { + super(code, message, null, response); } } diff --git a/src/main/java/com/google/firebase/projectmanagement/FirebaseProjectManagementServiceImpl.java b/src/main/java/com/google/firebase/projectmanagement/FirebaseProjectManagementServiceImpl.java index 8abced696..52b639f23 100644 --- a/src/main/java/com/google/firebase/projectmanagement/FirebaseProjectManagementServiceImpl.java +++ b/src/main/java/com/google/firebase/projectmanagement/FirebaseProjectManagementServiceImpl.java @@ -31,9 +31,11 @@ import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.firebase.ErrorCode; import com.google.firebase.FirebaseApp; import com.google.firebase.ImplFirebaseTrampolines; import com.google.firebase.internal.ApiClientUtils; +import com.google.firebase.IncomingHttpResponse; import com.google.firebase.internal.CallableOperation; import java.nio.charset.StandardCharsets; import java.util.ArrayList; @@ -318,20 +320,24 @@ protected String execute() throws FirebaseProjectManagementException { payloadBuilder.put("display_name", displayName); } OperationResponse operationResponseInstance = new OperationResponse(); - httpHelper.makePostRequest( + IncomingHttpResponse response = httpHelper.makePostRequest( url, payloadBuilder.build(), operationResponseInstance, projectId, "Project ID"); if (Strings.isNullOrEmpty(operationResponseInstance.name)) { - throw HttpHelper.createFirebaseProjectManagementException( + String message = buildMessage( namespace, "Bundle ID", - "Unable to create App: server returned null operation name.", - /* cause= */ null); + "Unable to create App: server returned null operation name."); + throw new FirebaseProjectManagementException(ErrorCode.INTERNAL, message, response); } return operationResponseInstance.name; } }; } + private String buildMessage(String resourceId, String resourceIdName, String description) { + return String.format("%s \"%s\": %s", resourceIdName, resourceId, description); + } + private String pollOperation(String projectId, String operationName) throws FirebaseProjectManagementException { String url = String.format("%s/v1/%s", FIREBASE_PROJECT_MANAGEMENT_URL, operationName); @@ -341,7 +347,8 @@ private String pollOperation(String projectId, String operationName) * Math.pow(POLL_EXPONENTIAL_BACKOFF_FACTOR, currentAttempt)); sleepOrThrow(projectId, delayMillis); OperationResponse operationResponseInstance = new OperationResponse(); - httpHelper.makeGetRequest(url, operationResponseInstance, projectId, "Project ID"); + IncomingHttpResponse response = httpHelper.makeGetRequest( + url, operationResponseInstance, projectId, "Project ID"); if (!operationResponseInstance.done) { continue; } @@ -349,19 +356,20 @@ private String pollOperation(String projectId, String operationName) // or 'error' is set. if (operationResponseInstance.response == null || Strings.isNullOrEmpty(operationResponseInstance.response.appId)) { - throw HttpHelper.createFirebaseProjectManagementException( + String message = buildMessage( projectId, "Project ID", - "Unable to create App: internal server error.", - /* cause= */ null); + "Unable to create App: internal server error."); + throw new FirebaseProjectManagementException(ErrorCode.INTERNAL, message, response); } return operationResponseInstance.response.appId; } - throw HttpHelper.createFirebaseProjectManagementException( + + String message = buildMessage( projectId, "Project ID", - "Unable to create App: deadline exceeded.", - /* cause= */ null); + "Unable to create App: deadline exceeded."); + throw new FirebaseProjectManagementException(ErrorCode.DEADLINE_EXCEEDED, message, null); } /** @@ -420,19 +428,22 @@ private WaitOperationRunnable( public void run() { String url = String.format("%s/v1/%s", FIREBASE_PROJECT_MANAGEMENT_URL, operationName); OperationResponse operationResponseInstance = new OperationResponse(); + IncomingHttpResponse httpResponse; try { - httpHelper.makeGetRequest(url, operationResponseInstance, projectId, "Project ID"); + httpResponse = httpHelper.makeGetRequest( + url, operationResponseInstance, projectId, "Project ID"); } catch (FirebaseProjectManagementException e) { settableFuture.setException(e); return; } if (!operationResponseInstance.done) { if (numberOfPreviousPolls + 1 >= MAXIMUM_POLLING_ATTEMPTS) { - settableFuture.setException(HttpHelper.createFirebaseProjectManagementException( - projectId, + String message = buildMessage(projectId, "Project ID", - "Unable to create App: deadline exceeded.", - /* cause= */ null)); + "Unable to create App: deadline exceeded."); + FirebaseProjectManagementException exception = new FirebaseProjectManagementException( + ErrorCode.DEADLINE_EXCEEDED, message, httpResponse); + settableFuture.setException(exception); } else { long delayMillis = (long) ( POLL_BASE_WAIT_TIME_MILLIS @@ -451,11 +462,12 @@ public void run() { // or 'error' is set. if (operationResponseInstance.response == null || Strings.isNullOrEmpty(operationResponseInstance.response.appId)) { - settableFuture.setException(HttpHelper.createFirebaseProjectManagementException( - projectId, + String message = buildMessage(projectId, "Project ID", - "Unable to create App: internal server error.", - /* cause= */ null)); + "Unable to create App: internal server error."); + FirebaseProjectManagementException exception = new FirebaseProjectManagementException( + ErrorCode.INTERNAL, message, httpResponse); + settableFuture.setException(exception); } else { settableFuture.set(operationResponseInstance.response.appId); } @@ -765,11 +777,10 @@ private void sleepOrThrow(String projectId, long delayMillis) try { sleeper.sleep(delayMillis); } catch (InterruptedException e) { - throw HttpHelper.createFirebaseProjectManagementException( - projectId, + String message = buildMessage(projectId, "Project ID", - "Unable to create App: exponential backoff interrupted.", - /* cause= */ null); + "Unable to create App: exponential backoff interrupted."); + throw new FirebaseProjectManagementException(ErrorCode.ABORTED, message, null); } } diff --git a/src/main/java/com/google/firebase/projectmanagement/HttpHelper.java b/src/main/java/com/google/firebase/projectmanagement/HttpHelper.java index 2143c1453..a9282ea26 100644 --- a/src/main/java/com/google/firebase/projectmanagement/HttpHelper.java +++ b/src/main/java/com/google/firebase/projectmanagement/HttpHelper.java @@ -16,84 +16,63 @@ package com.google.firebase.projectmanagement; -import com.google.api.client.http.GenericUrl; -import com.google.api.client.http.HttpRequest; import com.google.api.client.http.HttpRequestFactory; -import com.google.api.client.http.HttpResponse; -import com.google.api.client.http.HttpResponseException; import com.google.api.client.http.HttpResponseInterceptor; import com.google.api.client.http.json.JsonHttpContent; import com.google.api.client.json.JsonFactory; -import com.google.api.client.json.JsonObjectParser; import com.google.common.annotations.VisibleForTesting; -import com.google.common.base.Charsets; -import com.google.common.collect.ImmutableMap; -import com.google.firebase.internal.Nullable; +import com.google.firebase.FirebaseException; +import com.google.firebase.IncomingHttpResponse; +import com.google.firebase.internal.AbstractPlatformErrorHandler; +import com.google.firebase.internal.ErrorHandlingHttpClient; +import com.google.firebase.internal.HttpRequestInfo; import com.google.firebase.internal.SdkUtils; -import java.io.IOException; -class HttpHelper { +final class HttpHelper { @VisibleForTesting static final String PATCH_OVERRIDE_KEY = "X-HTTP-Method-Override"; @VisibleForTesting static final String PATCH_OVERRIDE_VALUE = "PATCH"; - private static final ImmutableMap ERROR_CODES = - ImmutableMap.builder() - .put(401, "Request not authorized.") - .put(403, "Client does not have sufficient privileges.") - .put(404, "Failed to find the resource.") - .put(409, "The resource already exists.") - .put(429, "Request throttled by the backend server.") - .put(500, "Internal server error.") - .put(503, "Backend servers are over capacity. Try again later.") - .build(); private static final String CLIENT_VERSION_HEADER = "X-Client-Version"; private final String clientVersion = "Java/Admin/" + SdkUtils.getVersion(); private final JsonFactory jsonFactory; - private final HttpRequestFactory requestFactory; + private final ErrorHandlingHttpClient httpClient; + private HttpResponseInterceptor interceptor; HttpHelper(JsonFactory jsonFactory, HttpRequestFactory requestFactory) { this.jsonFactory = jsonFactory; - this.requestFactory = requestFactory; + ProjectManagementErrorHandler errorHandler = new ProjectManagementErrorHandler(jsonFactory); + this.httpClient = new ErrorHandlingHttpClient<>(requestFactory, jsonFactory, errorHandler); } void setInterceptor(HttpResponseInterceptor interceptor) { this.interceptor = interceptor; } - void makeGetRequest( + IncomingHttpResponse makeGetRequest( String url, T parsedResponseInstance, String requestIdentifier, String requestIdentifierDescription) throws FirebaseProjectManagementException { - try { - makeRequest( - requestFactory.buildGetRequest(new GenericUrl(url)), - parsedResponseInstance, - requestIdentifier, - requestIdentifierDescription); - } catch (IOException e) { - handleError(requestIdentifier, requestIdentifierDescription, e); - } + return makeRequest( + HttpRequestInfo.buildGetRequest(url), + parsedResponseInstance, + requestIdentifier, + requestIdentifierDescription); } - void makePostRequest( + IncomingHttpResponse makePostRequest( String url, Object payload, T parsedResponseInstance, String requestIdentifier, String requestIdentifierDescription) throws FirebaseProjectManagementException { - try { - makeRequest( - requestFactory.buildPostRequest( - new GenericUrl(url), new JsonHttpContent(jsonFactory, payload)), - parsedResponseInstance, - requestIdentifier, - requestIdentifierDescription); - } catch (IOException e) { - handleError(requestIdentifier, requestIdentifierDescription, e); - } + return makeRequest( + HttpRequestInfo.buildPostRequest(url, new JsonHttpContent(jsonFactory, payload)), + parsedResponseInstance, + requestIdentifier, + requestIdentifierDescription); } void makePatchRequest( @@ -102,15 +81,11 @@ void makePatchRequest( T parsedResponseInstance, String requestIdentifier, String requestIdentifierDescription) throws FirebaseProjectManagementException { - try { - HttpRequest baseRequest = requestFactory.buildPostRequest( - new GenericUrl(url), new JsonHttpContent(jsonFactory, payload)); - baseRequest.getHeaders().set(PATCH_OVERRIDE_KEY, PATCH_OVERRIDE_VALUE); - makeRequest( - baseRequest, parsedResponseInstance, requestIdentifier, requestIdentifierDescription); - } catch (IOException e) { - handleError(requestIdentifier, requestIdentifierDescription, e); - } + HttpRequestInfo baseRequest = HttpRequestInfo.buildPostRequest( + url, new JsonHttpContent(jsonFactory, payload)); + baseRequest.addHeader(PATCH_OVERRIDE_KEY, PATCH_OVERRIDE_VALUE); + makeRequest( + baseRequest, parsedResponseInstance, requestIdentifier, requestIdentifierDescription); } void makeDeleteRequest( @@ -118,69 +93,41 @@ void makeDeleteRequest( T parsedResponseInstance, String requestIdentifier, String requestIdentifierDescription) throws FirebaseProjectManagementException { - try { - makeRequest( - requestFactory.buildDeleteRequest(new GenericUrl(url)), - parsedResponseInstance, - requestIdentifier, - requestIdentifierDescription); - } catch (IOException e) { - handleError(requestIdentifier, requestIdentifierDescription, e); - } + makeRequest( + HttpRequestInfo.buildDeleteRequest(url), + parsedResponseInstance, + requestIdentifier, + requestIdentifierDescription); } - void makeRequest( - HttpRequest baseRequest, + IncomingHttpResponse makeRequest( + HttpRequestInfo baseRequest, T parsedResponseInstance, String requestIdentifier, String requestIdentifierDescription) throws FirebaseProjectManagementException { - HttpResponse response = null; try { - baseRequest.getHeaders().set(CLIENT_VERSION_HEADER, clientVersion); - baseRequest.setParser(new JsonObjectParser(jsonFactory)); + baseRequest.addHeader(CLIENT_VERSION_HEADER, clientVersion); baseRequest.setResponseInterceptor(interceptor); - response = baseRequest.execute(); - jsonFactory.createJsonParser(response.getContent(), Charsets.UTF_8) - .parseAndClose(parsedResponseInstance); - } catch (Exception e) { - handleError(requestIdentifier, requestIdentifierDescription, e); - } finally { - disconnectQuietly(response); + IncomingHttpResponse response = httpClient.send(baseRequest); + httpClient.parse(response, parsedResponseInstance); + return response; + } catch (FirebaseProjectManagementException e) { + String message = String.format( + "%s \"%s\": %s", requestIdentifierDescription, requestIdentifier, e.getMessage()); + throw new FirebaseProjectManagementException(e, message); } } - private static void disconnectQuietly(HttpResponse response) { - if (response != null) { - try { - response.disconnect(); - } catch (IOException ignored) { - // Ignored. - } - } - } + private static class ProjectManagementErrorHandler + extends AbstractPlatformErrorHandler { - private static void handleError( - String requestIdentifier, String requestIdentifierDescription, Exception e) - throws FirebaseProjectManagementException { - String messageBody = "Error while invoking Firebase Project Management service."; - if (e instanceof HttpResponseException) { - int statusCode = ((HttpResponseException) e).getStatusCode(); - if (ERROR_CODES.containsKey(statusCode)) { - messageBody = ERROR_CODES.get(statusCode); - } + ProjectManagementErrorHandler(JsonFactory jsonFactory) { + super(jsonFactory); } - throw createFirebaseProjectManagementException( - requestIdentifier, requestIdentifierDescription, messageBody, e); - } - static FirebaseProjectManagementException createFirebaseProjectManagementException( - String requestIdentifier, - String requestIdentifierDescription, - String messageBody, - @Nullable Exception cause) { - return new FirebaseProjectManagementException( - String.format( - "%s \"%s\": %s", requestIdentifierDescription, requestIdentifier, messageBody), - cause); + @Override + protected FirebaseProjectManagementException createException(FirebaseException base) { + return new FirebaseProjectManagementException(base); + } } } diff --git a/src/test/java/com/google/firebase/projectmanagement/FirebaseProjectManagementServiceImplTest.java b/src/test/java/com/google/firebase/projectmanagement/FirebaseProjectManagementServiceImplTest.java index 87afe609c..bdac57493 100644 --- a/src/test/java/com/google/firebase/projectmanagement/FirebaseProjectManagementServiceImplTest.java +++ b/src/test/java/com/google/firebase/projectmanagement/FirebaseProjectManagementServiceImplTest.java @@ -24,6 +24,7 @@ import static com.google.firebase.projectmanagement.ShaCertificateType.SHA_256; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -40,12 +41,14 @@ import com.google.common.base.Charsets; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.firebase.ErrorCode; import com.google.firebase.FirebaseApp; import com.google.firebase.FirebaseOptions; import com.google.firebase.TestOnlyImplFirebaseTrampolines; import com.google.firebase.auth.MockGoogleCredentials; import com.google.firebase.internal.TestApiClientUtils; import com.google.firebase.testing.MultiRequestMockHttpTransport; +import com.google.firebase.testing.TestUtils; import java.io.ByteArrayOutputStream; import java.io.IOException; import java.util.ArrayList; @@ -272,7 +275,65 @@ public void getIosAppHttpError() { serviceImpl.getIosApp(IOS_APP_ID); fail("No exception thrown for HTTP error"); } catch (FirebaseProjectManagementException e) { - assertEquals("App ID \"test-ios-app-id\": Internal server error.", e.getMessage()); + assertEquals(ErrorCode.INTERNAL, e.getErrorCodeNew()); + assertEquals( + "App ID \"test-ios-app-id\": Unexpected HTTP response with status: 500\n{}", + e.getMessage()); + assertNotNull(e.getCause()); + assertNotNull(e.getHttpResponse()); + } + } + + @Test + public void getIosAppHttpErrorWithCode() { + firstRpcResponse.setStatusCode(500); + firstRpcResponse.setContent( + "{\"error\": {\"status\":\"NOT_FOUND\", \"message\":\"Test error\"}}"); + serviceImpl = initServiceImpl(firstRpcResponse, interceptor); + + try { + serviceImpl.getIosApp(IOS_APP_ID); + fail("No exception thrown for HTTP error"); + } catch (FirebaseProjectManagementException e) { + assertEquals(ErrorCode.NOT_FOUND, e.getErrorCodeNew()); + assertEquals("App ID \"test-ios-app-id\": Test error", e.getMessage()); + assertNotNull(e.getCause()); + assertNotNull(e.getHttpResponse()); + } + } + + @Test + public void getIosAppParseError() { + firstRpcResponse.setContent("not json"); + serviceImpl = initServiceImpl(firstRpcResponse, interceptor); + + try { + serviceImpl.getIosApp(IOS_APP_ID); + fail("No exception thrown for HTTP error"); + } catch (FirebaseProjectManagementException e) { + assertEquals(ErrorCode.UNKNOWN, e.getErrorCodeNew()); + assertTrue(e.getMessage().startsWith( + "App ID \"test-ios-app-id\": Error while parsing HTTP response")); + assertNotNull(e.getCause()); + assertNotNull(e.getHttpResponse()); + } + } + + @Test + public void getIosAppTransportError() { + FirebaseProjectManagementServiceImpl serviceImpl = initServiceImplWithFaultyTransport(); + + try { + serviceImpl.getIosApp(IOS_APP_ID); + fail("No exception thrown for HTTP error"); + } catch (FirebaseProjectManagementException e) { + assertEquals(ErrorCode.UNKNOWN, e.getErrorCodeNew()); + assertEquals( + "App ID \"test-ios-app-id\": Unknown error while making a remote service call: " + + "transport error", + e.getMessage()); + assertNotNull(e.getCause()); + assertNull(e.getHttpResponse()); } } @@ -554,7 +615,65 @@ public void getAndroidAppHttpError() { serviceImpl.getAndroidApp(ANDROID_APP_ID); fail("No exception thrown for HTTP error"); } catch (FirebaseProjectManagementException e) { - assertEquals("App ID \"test-android-app-id\": Internal server error.", e.getMessage()); + assertEquals(ErrorCode.INTERNAL, e.getErrorCodeNew()); + assertEquals( + "App ID \"test-android-app-id\": Unexpected HTTP response with status: 500\n{}", + e.getMessage()); + assertNotNull(e.getCause()); + assertNotNull(e.getHttpResponse()); + } + } + + @Test + public void getAndroidAppHttpErrorWithCode() { + firstRpcResponse.setStatusCode(500); + firstRpcResponse.setContent( + "{\"error\": {\"status\":\"NOT_FOUND\", \"message\":\"Test error\"}}"); + serviceImpl = initServiceImpl(firstRpcResponse, interceptor); + + try { + serviceImpl.getAndroidApp(ANDROID_APP_ID); + fail("No exception thrown for HTTP error"); + } catch (FirebaseProjectManagementException e) { + assertEquals(ErrorCode.NOT_FOUND, e.getErrorCodeNew()); + assertEquals("App ID \"test-android-app-id\": Test error", e.getMessage()); + assertNotNull(e.getCause()); + assertNotNull(e.getHttpResponse()); + } + } + + @Test + public void getAndroidAppParseError() { + firstRpcResponse.setContent("not json"); + serviceImpl = initServiceImpl(firstRpcResponse, interceptor); + + try { + serviceImpl.getAndroidApp(ANDROID_APP_ID); + fail("No exception thrown for HTTP error"); + } catch (FirebaseProjectManagementException e) { + assertEquals(ErrorCode.UNKNOWN, e.getErrorCodeNew()); + assertTrue(e.getMessage().startsWith( + "App ID \"test-android-app-id\": Error while parsing HTTP response")); + assertNotNull(e.getCause()); + assertNotNull(e.getHttpResponse()); + } + } + + @Test + public void getAndroidAppTransportError() { + FirebaseProjectManagementServiceImpl serviceImpl = initServiceImplWithFaultyTransport(); + + try { + serviceImpl.getAndroidApp(ANDROID_APP_ID); + fail("No exception thrown for HTTP error"); + } catch (FirebaseProjectManagementException e) { + assertEquals(ErrorCode.UNKNOWN, e.getErrorCodeNew()); + assertEquals( + "App ID \"test-android-app-id\": Unknown error while making a remote service call: " + + "transport error", + e.getMessage()); + assertNotNull(e.getCause()); + assertNull(e.getHttpResponse()); } } @@ -978,6 +1097,16 @@ private static FirebaseProjectManagementServiceImpl initServiceImpl( return serviceImpl; } + private static FirebaseProjectManagementServiceImpl initServiceImplWithFaultyTransport() { + FirebaseOptions options = new FirebaseOptions.Builder() + .setCredentials(new MockGoogleCredentials("test-token")) + .setProjectId(PROJECT_ID) + .setHttpTransport(TestUtils.createFaultyHttpTransport()) + .build(); + FirebaseApp app = FirebaseApp.initializeApp(options); + return new FirebaseProjectManagementServiceImpl(app, new MockSleeper(), new MockScheduler()); + } + private void checkRequestHeader(String expectedUrl, HttpMethod httpMethod) { assertEquals( "The number of HttpResponses is not equal to 1.", 1, interceptor.getNumberOfResponses()); diff --git a/src/test/java/com/google/firebase/projectmanagement/FirebaseProjectManagementTest.java b/src/test/java/com/google/firebase/projectmanagement/FirebaseProjectManagementTest.java index 7569b0bc2..bb83ce190 100644 --- a/src/test/java/com/google/firebase/projectmanagement/FirebaseProjectManagementTest.java +++ b/src/test/java/com/google/firebase/projectmanagement/FirebaseProjectManagementTest.java @@ -26,6 +26,7 @@ import com.google.api.core.ApiFutures; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; +import com.google.firebase.ErrorCode; import com.google.firebase.FirebaseApp; import com.google.firebase.FirebaseOptions; import com.google.firebase.TestOnlyImplFirebaseTrampolines; @@ -52,7 +53,7 @@ public class FirebaseProjectManagementTest { private static final String TEST_PROJECT_ID = "hello-world"; private static final String TEST_APP_BUNDLE_ID = "com.hello.world"; private static final FirebaseProjectManagementException FIREBASE_PROJECT_MANAGEMENT_EXCEPTION = - new FirebaseProjectManagementException("Error!", null); + new FirebaseProjectManagementException(ErrorCode.UNKNOWN, "Error!", null); @Rule public final MockitoRule mockitoRule = MockitoJUnit.rule(); diff --git a/src/test/java/com/google/firebase/projectmanagement/IosAppTest.java b/src/test/java/com/google/firebase/projectmanagement/IosAppTest.java index 269d3dae0..02d50279b 100644 --- a/src/test/java/com/google/firebase/projectmanagement/IosAppTest.java +++ b/src/test/java/com/google/firebase/projectmanagement/IosAppTest.java @@ -26,6 +26,7 @@ import com.google.api.core.ApiFuture; import com.google.api.core.ApiFutures; +import com.google.firebase.ErrorCode; import com.google.firebase.TestOnlyImplFirebaseTrampolines; import java.util.concurrent.ExecutionException; import org.junit.After; @@ -53,7 +54,7 @@ public class IosAppTest { + "SOME_OTHER_KEYsome-other-value" + ""; private static final FirebaseProjectManagementException FIREBASE_PROJECT_MANAGEMENT_EXCEPTION = - new FirebaseProjectManagementException("Error!", null); + new FirebaseProjectManagementException(ErrorCode.UNKNOWN, "Error!", null); private static final IosAppMetadata TEST_IOS_APP_METADATA = new IosAppMetadata( TEST_APP_NAME, From 540d4b2244b5d3d3427b067bb5f2101a56a25a8a Mon Sep 17 00:00:00 2001 From: hiranya911 Date: Thu, 13 Feb 2020 15:33:15 -0800 Subject: [PATCH 2/3] Minor code and test cleanup --- .../FirebaseProjectManagementServiceImpl.java | 8 ++++---- .../FirebaseProjectManagementServiceImplTest.java | 5 ++++- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/main/java/com/google/firebase/projectmanagement/FirebaseProjectManagementServiceImpl.java b/src/main/java/com/google/firebase/projectmanagement/FirebaseProjectManagementServiceImpl.java index 52b639f23..8389c307b 100644 --- a/src/main/java/com/google/firebase/projectmanagement/FirebaseProjectManagementServiceImpl.java +++ b/src/main/java/com/google/firebase/projectmanagement/FirebaseProjectManagementServiceImpl.java @@ -334,10 +334,6 @@ protected String execute() throws FirebaseProjectManagementException { }; } - private String buildMessage(String resourceId, String resourceIdName, String description) { - return String.format("%s \"%s\": %s", resourceIdName, resourceId, description); - } - private String pollOperation(String projectId, String operationName) throws FirebaseProjectManagementException { String url = String.format("%s/v1/%s", FIREBASE_PROJECT_MANAGEMENT_URL, operationName); @@ -784,6 +780,10 @@ private void sleepOrThrow(String projectId, long delayMillis) } } + private String buildMessage(String resourceId, String resourceIdName, String description) { + return String.format("%s \"%s\": %s", resourceIdName, resourceId, description); + } + /* Helper types. */ private interface CreateAppFromAppIdFunction extends ApiFunction {} diff --git a/src/test/java/com/google/firebase/projectmanagement/FirebaseProjectManagementServiceImplTest.java b/src/test/java/com/google/firebase/projectmanagement/FirebaseProjectManagementServiceImplTest.java index bdac57493..59ed0c50f 100644 --- a/src/test/java/com/google/firebase/projectmanagement/FirebaseProjectManagementServiceImplTest.java +++ b/src/test/java/com/google/firebase/projectmanagement/FirebaseProjectManagementServiceImplTest.java @@ -47,6 +47,7 @@ import com.google.firebase.TestOnlyImplFirebaseTrampolines; import com.google.firebase.auth.MockGoogleCredentials; import com.google.firebase.internal.TestApiClientUtils; +import com.google.firebase.internal.SdkUtils; import com.google.firebase.testing.MultiRequestMockHttpTransport; import com.google.firebase.testing.TestUtils; import java.io.ByteArrayOutputStream; @@ -72,6 +73,7 @@ public class FirebaseProjectManagementServiceImplTest { private static final String IOS_APP_ID = "test-ios-app-id"; private static final String IOS_APP_RESOURCE_NAME = "ios/11111"; private static final String ANDROID_APP_RESOURCE_NAME = "android/11111"; + private static final String CLIENT_VERSION = "Java/Admin/" + SdkUtils.getVersion(); private static final IosAppMetadata IOS_APP_METADATA = new IosAppMetadata(IOS_APP_RESOURCE_NAME, IOS_APP_ID, DISPLAY_NAME, PROJECT_ID, BUNDLE_ID); private static final AndroidAppMetadata ANDROID_APP_METADATA = @@ -1104,7 +1106,7 @@ private static FirebaseProjectManagementServiceImpl initServiceImplWithFaultyTra .setHttpTransport(TestUtils.createFaultyHttpTransport()) .build(); FirebaseApp app = FirebaseApp.initializeApp(options); - return new FirebaseProjectManagementServiceImpl(app, new MockSleeper(), new MockScheduler()); + return new FirebaseProjectManagementServiceImpl(app); } private void checkRequestHeader(String expectedUrl, HttpMethod httpMethod) { @@ -1119,6 +1121,7 @@ private void checkRequestHeader(int index, String expectedUrl, HttpMethod httpMe assertEquals(httpMethod.name(), request.getRequestMethod()); assertEquals(expectedUrl, request.getUrl().toString()); assertEquals("Bearer test-token", request.getHeaders().getAuthorization()); + assertEquals(CLIENT_VERSION, request.getHeaders().get("X-Client-Version")); } private void checkRequestPayload(Map expected) throws IOException { From 6cfbd2e16de922b1c914796542314c4e4f01e628 Mon Sep 17 00:00:00 2001 From: hiranya911 Date: Thu, 13 Feb 2020 16:04:31 -0800 Subject: [PATCH 3/3] Fixed some lint errors; Removed requestFactory reference from project mgt service impl --- .../FirebaseProjectManagementServiceImpl.java | 9 +------- .../firebase/internal/TestApiClientUtils.java | 21 ++++--------------- ...ebaseProjectManagementServiceImplTest.java | 14 ++++++++++--- 3 files changed, 16 insertions(+), 28 deletions(-) diff --git a/src/main/java/com/google/firebase/projectmanagement/FirebaseProjectManagementServiceImpl.java b/src/main/java/com/google/firebase/projectmanagement/FirebaseProjectManagementServiceImpl.java index 8389c307b..0f9eb3b55 100644 --- a/src/main/java/com/google/firebase/projectmanagement/FirebaseProjectManagementServiceImpl.java +++ b/src/main/java/com/google/firebase/projectmanagement/FirebaseProjectManagementServiceImpl.java @@ -34,8 +34,8 @@ import com.google.firebase.ErrorCode; import com.google.firebase.FirebaseApp; import com.google.firebase.ImplFirebaseTrampolines; -import com.google.firebase.internal.ApiClientUtils; import com.google.firebase.IncomingHttpResponse; +import com.google.firebase.internal.ApiClientUtils; import com.google.firebase.internal.CallableOperation; import java.nio.charset.StandardCharsets; import java.util.ArrayList; @@ -58,7 +58,6 @@ class FirebaseProjectManagementServiceImpl implements AndroidAppService, IosAppS private final FirebaseApp app; private final Sleeper sleeper; private final Scheduler scheduler; - private final HttpRequestFactory requestFactory; private final HttpHelper httpHelper; private final CreateAndroidAppFromAppIdFunction createAndroidAppFromAppIdFunction = @@ -80,15 +79,9 @@ class FirebaseProjectManagementServiceImpl implements AndroidAppService, IosAppS this.app = checkNotNull(app); this.sleeper = checkNotNull(sleeper); this.scheduler = checkNotNull(scheduler); - this.requestFactory = checkNotNull(requestFactory); this.httpHelper = new HttpHelper(app.getOptions().getJsonFactory(), requestFactory); } - @VisibleForTesting - HttpRequestFactory getRequestFactory() { - return requestFactory; - } - @VisibleForTesting void setInterceptor(HttpResponseInterceptor interceptor) { httpHelper.setInterceptor(interceptor); diff --git a/src/test/java/com/google/firebase/internal/TestApiClientUtils.java b/src/test/java/com/google/firebase/internal/TestApiClientUtils.java index 9f4bc2d09..510e08cd5 100644 --- a/src/test/java/com/google/firebase/internal/TestApiClientUtils.java +++ b/src/test/java/com/google/firebase/internal/TestApiClientUtils.java @@ -18,17 +18,14 @@ import static com.google.firebase.internal.ApiClientUtils.DEFAULT_RETRY_CONFIG; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; -import com.google.api.client.http.GenericUrl; import com.google.api.client.http.HttpRequest; import com.google.api.client.http.HttpRequestFactory; import com.google.api.client.http.HttpUnsuccessfulResponseHandler; import com.google.api.client.testing.util.MockSleeper; import com.google.firebase.FirebaseApp; import com.google.firebase.internal.RetryInitializer.RetryHandlerDecorator; -import java.io.IOException; public class TestApiClientUtils { @@ -39,8 +36,6 @@ public class TestApiClientUtils { .setSleeper(new MockSleeper()) .build(); - private static final GenericUrl TEST_URL = new GenericUrl("https://firebase.google.com"); - /** * Creates a new {@code HttpRequestFactory} which provides authorization (OAuth2), timeouts and * automatic retries. Bypasses exponential backoff between consecutive retries for faster @@ -65,20 +60,12 @@ public static HttpRequestFactory retryDisabledRequestFactory(FirebaseApp app) { } /** - * Checks whther the given HttpRequestFactory has been configured for authorization and + * Checks whether the given HttpRequest has been configured for authorization and * automatic retries. * - * @param requestFactory The HttpRequestFactory to check. + * @param request The HttpRequest to check. */ - public static void assertAuthAndRetrySupport(HttpRequestFactory requestFactory) { - assertTrue(requestFactory.getInitializer() instanceof FirebaseRequestInitializer); - HttpRequest request; - try { - request = requestFactory.buildGetRequest(TEST_URL); - } catch (IOException e) { - throw new RuntimeException("Failed to initialize request", e); - } - + public static void assertAuthAndRetrySupport(HttpRequest request) { // Verify authorization assertTrue(request.getHeaders().getAuthorization().startsWith("Bearer ")); @@ -89,7 +76,7 @@ public static void assertAuthAndRetrySupport(HttpRequestFactory requestFactory) .getRetryConfig(); assertEquals(DEFAULT_RETRY_CONFIG.getMaxRetries(), retryConfig.getMaxRetries()); assertEquals(DEFAULT_RETRY_CONFIG.getMaxIntervalMillis(), retryConfig.getMaxIntervalMillis()); - assertFalse(retryConfig.isRetryOnIOExceptions()); + assertEquals(DEFAULT_RETRY_CONFIG.isRetryOnIOExceptions(), retryConfig.isRetryOnIOExceptions()); assertEquals(DEFAULT_RETRY_CONFIG.getRetryStatusCodes(), retryConfig.getRetryStatusCodes()); } } diff --git a/src/test/java/com/google/firebase/projectmanagement/FirebaseProjectManagementServiceImplTest.java b/src/test/java/com/google/firebase/projectmanagement/FirebaseProjectManagementServiceImplTest.java index 59ed0c50f..89f511918 100644 --- a/src/test/java/com/google/firebase/projectmanagement/FirebaseProjectManagementServiceImplTest.java +++ b/src/test/java/com/google/firebase/projectmanagement/FirebaseProjectManagementServiceImplTest.java @@ -46,8 +46,8 @@ import com.google.firebase.FirebaseOptions; import com.google.firebase.TestOnlyImplFirebaseTrampolines; import com.google.firebase.auth.MockGoogleCredentials; -import com.google.firebase.internal.TestApiClientUtils; import com.google.firebase.internal.SdkUtils; +import com.google.firebase.internal.TestApiClientUtils; import com.google.firebase.testing.MultiRequestMockHttpTransport; import com.google.firebase.testing.TestUtils; import java.io.ByteArrayOutputStream; @@ -1039,17 +1039,25 @@ public void deleteShaCertificateAsync() throws Exception { } @Test - public void testAuthAndRetriesSupport() { + public void testAuthAndRetriesSupport() throws Exception { + List mockResponses = ImmutableList.of( + new MockLowLevelHttpResponse().setContent("{}")); + MockHttpTransport transport = new MultiRequestMockHttpTransport(mockResponses); FirebaseOptions options = new FirebaseOptions.Builder() .setCredentials(new MockGoogleCredentials("test-token")) .setProjectId(PROJECT_ID) + .setHttpTransport(transport) .build(); FirebaseApp app = FirebaseApp.initializeApp(options); FirebaseProjectManagementServiceImpl serviceImpl = new FirebaseProjectManagementServiceImpl(app); + serviceImpl.setInterceptor(interceptor); + + serviceImpl.deleteShaCertificate(SHA1_RESOURCE_NAME); - TestApiClientUtils.assertAuthAndRetrySupport(serviceImpl.getRequestFactory()); + assertEquals(1, interceptor.getNumberOfResponses()); + TestApiClientUtils.assertAuthAndRetrySupport(interceptor.getResponse(0).getRequest()); } @Test