fix: Enabled automatic HTTP retries for FirebaseProjectManagement (#356)

hiranya911 · web-flow · commit a039e051c943 · 2020-02-10T11:16:50.000-08:00
* Enabled automatic HTTP retries for FirebaseProjectManagement

* Added some test cases

* Added helper function for disabling exponential backoff during tests

* Added util class for simplifying retry tests

* Simplified test cases

* Removed unused method
diff --git a/src/main/java/com/google/firebase/internal/ApiClientUtils.java b/src/main/java/com/google/firebase/internal/ApiClientUtils.java
@@ -29,7 +29,7 @@
  */
 public class ApiClientUtils {
 
-  private static final RetryConfig DEFAULT_RETRY_CONFIG = RetryConfig.builder()
+  static final RetryConfig DEFAULT_RETRY_CONFIG = RetryConfig.builder()
       .setMaxRetries(4)
       .setRetryStatusCodes(ImmutableList.of(500, 503))
       .setMaxIntervalMillis(60 * 1000)
@@ -43,9 +43,21 @@ public class ApiClientUtils {
    * @return A new {@code HttpRequestFactory} instance.
    */
   public static HttpRequestFactory newAuthorizedRequestFactory(FirebaseApp app) {
+    return newAuthorizedRequestFactory(app, DEFAULT_RETRY_CONFIG);
+  }
+
+  /**
+   * Creates a new {@code HttpRequestFactory} which provides authorization (OAuth2), timeouts and
+   * automatic retries.
+   *
+   * @param app {@link FirebaseApp} from which to obtain authorization credentials.
+   * @param retryConfig {@link RetryConfig} instance or null to disable retries.
+   * @return A new {@code HttpRequestFactory} instance.
+   */
+  public static HttpRequestFactory newAuthorizedRequestFactory(
+      FirebaseApp app, @Nullable RetryConfig retryConfig) {
     HttpTransport transport = app.getOptions().getHttpTransport();
-    return transport.createRequestFactory(
-        new FirebaseRequestInitializer(app, DEFAULT_RETRY_CONFIG));
+    return transport.createRequestFactory(new FirebaseRequestInitializer(app, retryConfig));
   }
 
   public static HttpRequestFactory newUnauthorizedRequestFactory(FirebaseApp app) {
diff --git a/src/main/java/com/google/firebase/projectmanagement/FirebaseProjectManagementServiceImpl.java b/src/main/java/com/google/firebase/projectmanagement/FirebaseProjectManagementServiceImpl.java
@@ -18,6 +18,7 @@
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
 
+import com.google.api.client.http.HttpRequestFactory;
 import com.google.api.client.http.HttpResponseInterceptor;
 import com.google.api.client.util.Base64;
 import com.google.api.client.util.Key;
@@ -32,8 +33,8 @@
 import com.google.common.collect.ImmutableMap;
 import com.google.firebase.FirebaseApp;
 import com.google.firebase.ImplFirebaseTrampolines;
+import com.google.firebase.internal.ApiClientUtils;
 import com.google.firebase.internal.CallableOperation;
-import com.google.firebase.internal.FirebaseRequestInitializer;
 import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.List;
@@ -55,6 +56,7 @@ 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 =
@@ -63,17 +65,26 @@ class FirebaseProjectManagementServiceImpl implements AndroidAppService, IosAppS
       new CreateIosAppFromAppIdFunction();
 
   FirebaseProjectManagementServiceImpl(FirebaseApp app) {
-    this(app, Sleeper.DEFAULT, new FirebaseAppScheduler(app));
+    this(
+        app,
+        Sleeper.DEFAULT,
+        new FirebaseAppScheduler(app),
+        ApiClientUtils.newAuthorizedRequestFactory(app));
   }
 
-  FirebaseProjectManagementServiceImpl(FirebaseApp app, Sleeper sleeper, Scheduler scheduler) {
+  @VisibleForTesting
+  FirebaseProjectManagementServiceImpl(
+      FirebaseApp app, Sleeper sleeper, Scheduler scheduler, HttpRequestFactory requestFactory) {
     this.app = checkNotNull(app);
     this.sleeper = checkNotNull(sleeper);
     this.scheduler = checkNotNull(scheduler);
-    this.httpHelper = new HttpHelper(
-        app.getOptions().getJsonFactory(),
-        app.getOptions().getHttpTransport().createRequestFactory(
-            new FirebaseRequestInitializer(app)));
+    this.requestFactory = checkNotNull(requestFactory);
+    this.httpHelper = new HttpHelper(app.getOptions().getJsonFactory(), requestFactory);
+  }
+
+  @VisibleForTesting
+  HttpRequestFactory getRequestFactory() {
+    return requestFactory;
   }
 
   @VisibleForTesting
diff --git a/src/test/java/com/google/firebase/internal/ApiClientUtilsTest.java b/src/test/java/com/google/firebase/internal/ApiClientUtilsTest.java
@@ -69,6 +69,19 @@ public void testAuthorizedHttpClient() throws IOException {
     assertEquals(retryConfig.getRetryStatusCodes(), ImmutableList.of(500, 503));
   }
 
+  @Test
+  public void testAuthorizedHttpClientWithoutRetry() throws IOException {
+    FirebaseApp app = FirebaseApp.initializeApp(TEST_OPTIONS);
+
+    HttpRequestFactory requestFactory = ApiClientUtils.newAuthorizedRequestFactory(app, null);
+
+    assertTrue(requestFactory.getInitializer() instanceof FirebaseRequestInitializer);
+    HttpRequest request = requestFactory.buildGetRequest(TEST_URL);
+    assertEquals("Bearer test-token", request.getHeaders().getAuthorization());
+    HttpUnsuccessfulResponseHandler retryHandler = request.getUnsuccessfulResponseHandler();
+    assertFalse(retryHandler instanceof RetryHandlerDecorator);
+  }
+
   @Test
   public void testUnauthorizedHttpClient() throws IOException {
     FirebaseApp app = FirebaseApp.initializeApp(TEST_OPTIONS);
diff --git a/src/test/java/com/google/firebase/internal/TestApiClientUtils.java b/src/test/java/com/google/firebase/internal/TestApiClientUtils.java
@@ -0,0 +1,95 @@
+/*
+ * Copyright 2020 Google Inc.
+ *
+ * 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.internal;
+
+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 {
+
+  private static final RetryConfig TEST_RETRY_CONFIG = RetryConfig.builder()
+      .setMaxRetries(DEFAULT_RETRY_CONFIG.getMaxRetries())
+      .setRetryStatusCodes(DEFAULT_RETRY_CONFIG.getRetryStatusCodes())
+      .setMaxIntervalMillis(DEFAULT_RETRY_CONFIG.getMaxIntervalMillis())
+      .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
+   * execution during tests.
+   *
+   * @param app {@link FirebaseApp} from which to obtain authorization credentials.
+   * @return A new {@code HttpRequestFactory} instance.
+   */
+  public static HttpRequestFactory delayBypassedRequestFactory(FirebaseApp app) {
+    return ApiClientUtils.newAuthorizedRequestFactory(app, TEST_RETRY_CONFIG);
+  }
+
+  /**
+   * Creates a new {@code HttpRequestFactory} which provides authorization (OAuth2), timeouts but
+   * no retries.
+   *
+   * @param app {@link FirebaseApp} from which to obtain authorization credentials.
+   * @return A new {@code HttpRequestFactory} instance.
+   */
+  public static HttpRequestFactory retryDisabledRequestFactory(FirebaseApp app) {
+    return ApiClientUtils.newAuthorizedRequestFactory(app, null);
+  }
+
+  /**
+   * Checks whther the given HttpRequestFactory has been configured for authorization and
+   * automatic retries.
+   *
+   * @param requestFactory The HttpRequestFactory 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);
+    }
+
+    // Verify authorization
+    assertTrue(request.getHeaders().getAuthorization().startsWith("Bearer "));
+
+    // Verify retry support
+    HttpUnsuccessfulResponseHandler retryHandler = request.getUnsuccessfulResponseHandler();
+    assertTrue(retryHandler instanceof RetryHandlerDecorator);
+    RetryConfig retryConfig = ((RetryHandlerDecorator) retryHandler).getRetryHandler()
+        .getRetryConfig();
+    assertEquals(DEFAULT_RETRY_CONFIG.getMaxRetries(), retryConfig.getMaxRetries());
+    assertEquals(DEFAULT_RETRY_CONFIG.getMaxIntervalMillis(), retryConfig.getMaxIntervalMillis());
+    assertFalse(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
@@ -29,6 +29,7 @@
 
 import com.google.api.client.googleapis.util.Utils;
 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.HttpResponseInterceptor;
 import com.google.api.client.json.JsonParser;
@@ -43,6 +44,7 @@
 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 java.io.ByteArrayOutputStream;
 import java.io.IOException;
@@ -370,7 +372,7 @@ public void listIosAppsAsyncMultiplePages() throws Exception {
     MockLowLevelHttpResponse secondRpcResponse = new MockLowLevelHttpResponse();
     secondRpcResponse.setContent(LIST_IOS_APPS_PAGE_2_RESPONSE);
     serviceImpl = initServiceImpl(
-        ImmutableList.<MockLowLevelHttpResponse>of(firstRpcResponse, secondRpcResponse),
+        ImmutableList.of(firstRpcResponse, secondRpcResponse),
         interceptor);
 
     List<IosApp> iosAppList = serviceImpl.listIosAppsAsync(PROJECT_ID).get();
@@ -400,7 +402,7 @@ public void createIosApp() throws Exception {
     MockLowLevelHttpResponse thirdRpcResponse = new MockLowLevelHttpResponse();
     thirdRpcResponse.setContent(CREATE_IOS_GET_OPERATION_ATTEMPT_2_RESPONSE);
     serviceImpl = initServiceImpl(
-        ImmutableList.<MockLowLevelHttpResponse>of(
+        ImmutableList.of(
             firstRpcResponse, secondRpcResponse, thirdRpcResponse),
         interceptor);
 
@@ -624,7 +626,7 @@ public void listAndroidAppsMultiplePages() throws Exception {
     MockLowLevelHttpResponse secondRpcResponse = new MockLowLevelHttpResponse();
     secondRpcResponse.setContent(LIST_ANDROID_APPS_PAGE_2_RESPONSE);
     serviceImpl = initServiceImpl(
-        ImmutableList.<MockLowLevelHttpResponse>of(firstRpcResponse, secondRpcResponse),
+        ImmutableList.of(firstRpcResponse, secondRpcResponse),
         interceptor);
 
     List<AndroidApp> androidAppList = serviceImpl.listAndroidApps(PROJECT_ID);
@@ -652,7 +654,7 @@ public void listAndroidAppsAsyncMultiplePages() throws Exception {
     MockLowLevelHttpResponse secondRpcResponse = new MockLowLevelHttpResponse();
     secondRpcResponse.setContent(LIST_ANDROID_APPS_PAGE_2_RESPONSE);
     serviceImpl = initServiceImpl(
-        ImmutableList.<MockLowLevelHttpResponse>of(firstRpcResponse, secondRpcResponse),
+        ImmutableList.of(firstRpcResponse, secondRpcResponse),
         interceptor);
 
     List<AndroidApp> androidAppList = serviceImpl.listAndroidAppsAsync(PROJECT_ID).get();
@@ -682,7 +684,7 @@ public void createAndroidApp() throws Exception {
     MockLowLevelHttpResponse thirdRpcResponse = new MockLowLevelHttpResponse();
     thirdRpcResponse.setContent(CREATE_ANDROID_GET_OPERATION_ATTEMPT_2_RESPONSE);
     serviceImpl = initServiceImpl(
-        ImmutableList.<MockLowLevelHttpResponse>of(
+        ImmutableList.of(
             firstRpcResponse, secondRpcResponse, thirdRpcResponse),
         interceptor);
 
@@ -714,7 +716,7 @@ public void createAndroidAppAsync() throws Exception {
     MockLowLevelHttpResponse thirdRpcResponse = new MockLowLevelHttpResponse();
     thirdRpcResponse.setContent(CREATE_ANDROID_GET_OPERATION_ATTEMPT_2_RESPONSE);
     serviceImpl = initServiceImpl(
-        ImmutableList.<MockLowLevelHttpResponse>of(
+        ImmutableList.of(
             firstRpcResponse, secondRpcResponse, thirdRpcResponse),
         interceptor);
 
@@ -915,10 +917,48 @@ public void deleteShaCertificateAsync() throws Exception {
     checkRequestHeader(expectedUrl, HttpMethod.DELETE);
   }
 
+  @Test
+  public void testAuthAndRetriesSupport() {
+    FirebaseOptions options = new FirebaseOptions.Builder()
+        .setCredentials(new MockGoogleCredentials("test-token"))
+        .setProjectId(PROJECT_ID)
+        .build();
+    FirebaseApp app = FirebaseApp.initializeApp(options);
+
+    FirebaseProjectManagementServiceImpl serviceImpl =
+        new FirebaseProjectManagementServiceImpl(app);
+
+    TestApiClientUtils.assertAuthAndRetrySupport(serviceImpl.getRequestFactory());
+  }
+
+  @Test
+  public void testHttpRetries() throws Exception {
+    List<MockLowLevelHttpResponse> mockResponses = ImmutableList.of(
+        firstRpcResponse.setStatusCode(503).setContent("{}"),
+        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);
+    HttpRequestFactory requestFactory = TestApiClientUtils.delayBypassedRequestFactory(app);
+    FirebaseProjectManagementServiceImpl serviceImpl = new FirebaseProjectManagementServiceImpl(
+        app, new MockSleeper(), new MockScheduler(), requestFactory);
+    serviceImpl.setInterceptor(interceptor);
+
+    serviceImpl.deleteShaCertificate(SHA1_RESOURCE_NAME);
+
+    String expectedUrl = String.format(
+        "%s/v1beta1/%s", FIREBASE_PROJECT_MANAGEMENT_URL, SHA1_RESOURCE_NAME);
+    checkRequestHeader(expectedUrl, HttpMethod.DELETE);
+  }
+
   private static FirebaseProjectManagementServiceImpl initServiceImpl(
       MockLowLevelHttpResponse mockResponse,
       MultiRequestTestResponseInterceptor interceptor) {
-    return initServiceImpl(ImmutableList.<MockLowLevelHttpResponse>of(mockResponse), interceptor);
+    return initServiceImpl(ImmutableList.of(mockResponse), interceptor);
   }
 
   private static FirebaseProjectManagementServiceImpl initServiceImpl(
@@ -931,8 +971,9 @@ private static FirebaseProjectManagementServiceImpl initServiceImpl(
         .setHttpTransport(transport)
         .build();
     FirebaseApp app = FirebaseApp.initializeApp(options);
-    FirebaseProjectManagementServiceImpl serviceImpl =
-        new FirebaseProjectManagementServiceImpl(app, new MockSleeper(), new MockScheduler());
+    HttpRequestFactory requestFactory = TestApiClientUtils.retryDisabledRequestFactory(app);
+    FirebaseProjectManagementServiceImpl serviceImpl = new FirebaseProjectManagementServiceImpl(
+        app, new MockSleeper(), new MockScheduler(), requestFactory);
     serviceImpl.setInterceptor(interceptor);
     return serviceImpl;
   }
