Repository: firebase/firebase-admin-java (java)

Issue:
Support Configurable Timeouts
Admin SDK uses Google Java HTTP client to make outbound HTTP connections. But we currently do not set the connection and read timeout for outbound requests. These need to be configurable via `FirebaseOptions` or a similar API.

Pull request description:
Support configuring timeout for outbound requests
Resolves #76

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index e080075..2c665ec 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,8 @@
 # Unreleased
 
+- [added] Connection timeout and read timeout for HTTP/REST connections
+  can now be configured via `FirebaseOptions.Builder` at app
+  initialization.
 - [added] Added new `setMutableContent()`, `putCustomData()` and
   `putAllCustomData()` methods to the `Aps.Builder` API.
 - [fixed] Improved error handling in FCM by mapping more server-side
diff --git a/src/main/java/com/google/firebase/FirebaseOptions.java b/src/main/java/com/google/firebase/FirebaseOptions.java
index fc4df26..28c9e9e 100644
--- a/src/main/java/com/google/firebase/FirebaseOptions.java
+++ b/src/main/java/com/google/firebase/FirebaseOptions.java
@@ -47,6 +47,8 @@ public final class FirebaseOptions {
   private final Map<String, Object> databaseAuthVariableOverride;
   private final String projectId;
   private final HttpTransport httpTransport;
+  private final int connectTimeout;
+  private final int readTimeout;
   private final JsonFactory jsonFactory;
   private final ThreadManager threadManager;
 
@@ -68,6 +70,10 @@ public final class FirebaseOptions {
         "FirebaseOptions must be initialized with a non-null JsonFactory.");
     this.threadManager = checkNotNull(builder.threadManager,
         "FirebaseOptions must be initialized with a non-null ThreadManager.");
+    checkArgument(builder.connectTimeout >= 0);
+    this.connectTimeout = builder.connectTimeout;
+    checkArgument(builder.readTimeout >= 0);
+    this.readTimeout = builder.readTimeout;
   }
 
   /**
@@ -132,6 +138,26 @@ public final class FirebaseOptions {
     return jsonFactory;
   }
 
+  /**
+   * Returns the connect timeout in milliseconds, which is applied to outgoing REST calls
+   * made by the SDK.
+   *
+   * @return Connect timeout in milliseconds. 0 indicates an infinite timeout.
+   */
+  public int getConnectTimeout() {
+    return connectTimeout;
+  }
+
+  /**
+   * Returns the read timeout in milliseconds, which is applied to outgoing REST calls
+   * made by the SDK.
+   *
+   * @return Read timeout in milliseconds. 0 indicates an infinite timeout.
+   */
+  public int getReadTimeout() {
+    return readTimeout;
+  }
+
   @NonNull
   ThreadManager getThreadManager() {
     return threadManager;
@@ -157,6 +183,8 @@ public final class FirebaseOptions {
     private HttpTransport httpTransport = Utils.getDefaultTransport();
     private JsonFactory jsonFactory = Utils.getDefaultJsonFactory();
     private ThreadManager threadManager = FirebaseThreadManagers.DEFAULT_THREAD_MANAGER;
+    private int connectTimeout;
+    private int readTimeout;
 
     /** Constructs an empty builder. */
     public Builder() {}
@@ -176,6 +204,8 @@ public final class FirebaseOptions {
       httpTransport = options.httpTransport;
       jsonFactory = options.jsonFactory;
       threadManager = options.threadManager;
+      connectTimeout = options.connectTimeout;
+      readTimeout = options.readTimeout;
     }
 
     /**
@@ -321,6 +351,33 @@ public final class FirebaseOptions {
       return this;
     }
 
+    /**
+     * Sets the connect timeout for outgoing HTTP (REST) connections made by the SDK. This is used
+     * when opening a communication link to a remote HTTP endpoint. This setting does not
+     * affect the {@link com.google.firebase.database.FirebaseDatabase} and
+     * {@link com.google.firebase.cloud.FirestoreClient} APIs.
+     *
+     * @param connectTimeout Connect timeout in milliseconds. Must not be negative.
+     * @return This <code>Builder</code> instance is returned so subsequent calls can be chained.
+     */
+    public Builder setConnectTimeout(int connectTimeout) {
+      this.connectTimeout = connectTimeout;
+      return this;
+    }
+
+    /**
+     * Sets the read timeout for outgoing HTTP (REST) calls made by the SDK. This does not affect
+     * the {@link com.google.firebase.database.FirebaseDatabase} and
+     * {@link com.google.firebase.cloud.FirestoreClient} APIs.
+     *
+     * @param readTimeout Read timeout in milliseconds. Must not be negative.
+     * @return This <code>Builder</code> instance is returned so subsequent calls can be chained.
+     */
+    public Builder setReadTimeout(int readTimeout) {
+      this.readTimeout = readTimeout;
+      return this;
+    }
+
     /**
      * Builds the {@link FirebaseOptions} instance from the previously set options.
      *
diff --git a/src/main/java/com/google/firebase/auth/FirebaseAuth.java b/src/main/java/com/google/firebase/auth/FirebaseAuth.java
index bbb9feb..5f295c1 100644
--- a/src/main/java/com/google/firebase/auth/FirebaseAuth.java
+++ b/src/main/java/com/google/firebase/auth/FirebaseAuth.java
@@ -85,8 +85,7 @@ public class FirebaseAuth {
     this.credentials = ImplFirebaseTrampolines.getCredentials(firebaseApp);
     this.projectId = ImplFirebaseTrampolines.getProjectId(firebaseApp);
     this.jsonFactory = firebaseApp.getOptions().getJsonFactory();
-    this.userManager = new FirebaseUserManager(jsonFactory,
-        firebaseApp.getOptions().getHttpTransport(), this.credentials);
+    this.userManager = new FirebaseUserManager(firebaseApp);
     this.destroyed = new AtomicBoolean(false);
     this.lock = new Object();
   }
diff --git a/src/main/java/com/google/firebase/auth/FirebaseUserManager.java b/src/main/java/com/google/firebase/auth/FirebaseUserManager.java
index 5044764..79090ed 100644
--- a/src/main/java/com/google/firebase/auth/FirebaseUserManager.java
+++ b/src/main/java/com/google/firebase/auth/FirebaseUserManager.java
@@ -30,18 +30,19 @@ import com.google.api.client.http.json.JsonHttpContent;
 import com.google.api.client.json.GenericJson;
 import com.google.api.client.json.JsonFactory;
 import com.google.api.client.json.JsonObjectParser;
-import com.google.auth.http.HttpCredentialsAdapter;
-import com.google.auth.oauth2.GoogleCredentials;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.firebase.FirebaseApp;
 import com.google.firebase.auth.UserRecord.CreateRequest;
 import com.google.firebase.auth.UserRecord.UpdateRequest;
 import com.google.firebase.auth.internal.DownloadAccountResponse;
 import com.google.firebase.auth.internal.GetAccountInfoResponse;
 
 import com.google.firebase.auth.internal.HttpErrorResponse;
+import com.google.firebase.internal.FirebaseRequestInitializer;
+import com.google.firebase.internal.NonNull;
 import com.google.firebase.internal.SdkUtils;
 import java.io.IOException;
 import java.util.List;
@@ -98,13 +99,13 @@ class FirebaseUserManager {
   /**
    * Creates a new FirebaseUserManager instance.
    *
-   * @param jsonFactory JsonFactory instance used to transform Java objects into JSON and back.
-   * @param transport HttpTransport used to make REST API calls.
+   * @param app A non-null {@link FirebaseApp}.
    */
-  FirebaseUserManager(JsonFactory jsonFactory, HttpTransport transport,
-      GoogleCredentials credentials) {
-    this.jsonFactory = checkNotNull(jsonFactory, "jsonFactory must not be null");
-    this.requestFactory = transport.createRequestFactory(new HttpCredentialsAdapter(credentials));
+  FirebaseUserManager(@NonNull FirebaseApp app) {
+    checkNotNull(app, "FirebaseApp must not be null");
+    this.jsonFactory = app.getOptions().getJsonFactory();
+    HttpTransport transport = app.getOptions().getHttpTransport();
+    this.requestFactory = transport.createRequestFactory(new FirebaseRequestInitializer(app));
   }
 
   @VisibleForTesting
diff --git a/src/main/java/com/google/firebase/iid/FirebaseInstanceId.java b/src/main/java/com/google/firebase/iid/FirebaseInstanceId.java
index 6993b8f..9057905 100644
--- a/src/main/java/com/google/firebase/iid/FirebaseInstanceId.java
+++ b/src/main/java/com/google/firebase/iid/FirebaseInstanceId.java
@@ -28,14 +28,13 @@ import com.google.api.client.http.HttpTransport;
 import com.google.api.client.json.JsonFactory;
 import com.google.api.client.json.JsonObjectParser;
 import com.google.api.core.ApiFuture;
-import com.google.auth.http.HttpCredentialsAdapter;
-import com.google.auth.oauth2.GoogleCredentials;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.io.ByteStreams;
 import com.google.firebase.FirebaseApp;
 import com.google.firebase.ImplFirebaseTrampolines;
+import com.google.firebase.internal.FirebaseRequestInitializer;
 import com.google.firebase.internal.FirebaseService;
 import com.google.firebase.internal.NonNull;
 import com.google.firebase.internal.TaskToApiFuture;
@@ -74,10 +73,8 @@ public class FirebaseInstanceId {
 
   private FirebaseInstanceId(FirebaseApp app) {
     HttpTransport httpTransport = app.getOptions().getHttpTransport();
-    GoogleCredentials credentials = ImplFirebaseTrampolines.getCredentials(app);
     this.app = app;
-    this.requestFactory = httpTransport.createRequestFactory(
-        new HttpCredentialsAdapter(credentials));
+    this.requestFactory = httpTransport.createRequestFactory(new FirebaseRequestInitializer(app));
     this.jsonFactory = app.getOptions().getJsonFactory();
     this.projectId = ImplFirebaseTrampolines.getProjectId(app);
     checkArgument(!Strings.isNullOrEmpty(projectId),
diff --git a/src/main/java/com/google/firebase/internal/FirebaseRequestInitializer.java b/src/main/java/com/google/firebase/internal/FirebaseRequestInitializer.java
new file mode 100644
index 0000000..0cd4e93
--- /dev/null
+++ b/src/main/java/com/google/firebase/internal/FirebaseRequestInitializer.java
@@ -0,0 +1,50 @@
+/*
+ * Copyright 2018 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 com.google.api.client.http.HttpRequest;
+import com.google.api.client.http.HttpRequestInitializer;
+import com.google.auth.http.HttpCredentialsAdapter;
+import com.google.auth.oauth2.GoogleCredentials;
+import com.google.firebase.FirebaseApp;
+import com.google.firebase.ImplFirebaseTrampolines;
+import java.io.IOException;
+
+/**
+ * {@code HttpRequestInitializer} for configuring outgoing REST calls. Handles OAuth2 authorization
+ * and setting timeout values.
+ */
+public class FirebaseRequestInitializer implements HttpRequestInitializer {
+
+  private final HttpCredentialsAdapter credentialsAdapter;
+  private final int connectTimeout;
+  private final int readTimeout;
+
+  public FirebaseRequestInitializer(FirebaseApp app) {
+    GoogleCredentials credentials = ImplFirebaseTrampolines.getCredentials(app);
+    this.credentialsAdapter = new HttpCredentialsAdapter(credentials);
+    this.connectTimeout = app.getOptions().getConnectTimeout();
+    this.readTimeout = app.getOptions().getReadTimeout();
+  }
+
+  @Override
+  public void initialize(HttpRequest httpRequest) throws IOException {
+    credentialsAdapter.initialize(httpRequest);
+    httpRequest.setConnectTimeout(connectTimeout);
+    httpRequest.setReadTimeout(readTimeout);
+  }
+}
diff --git a/src/main/java/com/google/firebase/messaging/FirebaseMessaging.java b/src/main/java/com/google/firebase/messaging/FirebaseMessaging.java
index 21ab389..2daec84 100644
--- a/src/main/java/com/google/firebase/messaging/FirebaseMessaging.java
+++ b/src/main/java/com/google/firebase/messaging/FirebaseMessaging.java
@@ -33,13 +33,12 @@ import com.google.api.client.json.JsonObjectParser;
 import com.google.api.client.json.JsonParser;
 import com.google.api.client.util.Key;
 import com.google.api.core.ApiFuture;
-import com.google.auth.http.HttpCredentialsAdapter;
-import com.google.auth.oauth2.GoogleCredentials;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableMap;
 import com.google.firebase.FirebaseApp;
 import com.google.firebase.ImplFirebaseTrampolines;
+import com.google.firebase.internal.FirebaseRequestInitializer;
 import com.google.firebase.internal.FirebaseService;
 import com.google.firebase.internal.NonNull;
 import com.google.firebase.internal.TaskToApiFuture;
@@ -102,10 +101,8 @@ public class FirebaseMessaging {
 
   private FirebaseMessaging(FirebaseApp app) {
     HttpTransport httpTransport = app.getOptions().getHttpTransport();
-    GoogleCredentials credentials = ImplFirebaseTrampolines.getCredentials(app);
     this.app = app;
-    this.requestFactory = httpTransport.createRequestFactory(
-        new HttpCredentialsAdapter(credentials));
+    this.requestFactory = httpTransport.createRequestFactory(new FirebaseRequestInitializer(app));
     this.jsonFactory = app.getOptions().getJsonFactory();
     String projectId = ImplFirebaseTrampolines.getProjectId(app);
     checkArgument(!Strings.isNullOrEmpty(projectId),


Test patch:
diff --git a/src/test/java/com/google/firebase/FirebaseOptionsTest.java b/src/test/java/com/google/firebase/FirebaseOptionsTest.java
index b02fbbd..33e07ae 100644
--- a/src/test/java/com/google/firebase/FirebaseOptionsTest.java
+++ b/src/test/java/com/google/firebase/FirebaseOptionsTest.java
@@ -87,6 +87,8 @@ public class FirebaseOptionsTest {
             .setJsonFactory(jsonFactory)
             .setHttpTransport(httpTransport)
             .setThreadManager(MOCK_THREAD_MANAGER)
+            .setConnectTimeout(30000)
+            .setReadTimeout(60000)
             .build();
     assertEquals(FIREBASE_DB_URL, firebaseOptions.getDatabaseUrl());
     assertEquals(FIREBASE_STORAGE_BUCKET, firebaseOptions.getStorageBucket());
@@ -94,6 +96,8 @@ public class FirebaseOptionsTest {
     assertSame(jsonFactory, firebaseOptions.getJsonFactory());
     assertSame(httpTransport, firebaseOptions.getHttpTransport());
     assertSame(MOCK_THREAD_MANAGER, firebaseOptions.getThreadManager());
+    assertEquals(30000, firebaseOptions.getConnectTimeout());
+    assertEquals(60000, firebaseOptions.getReadTimeout());
 
     GoogleCredentials credentials = firebaseOptions.getCredentials();
     assertNotNull(credentials);
@@ -114,6 +118,8 @@ public class FirebaseOptionsTest {
     assertNotNull(firebaseOptions.getThreadManager());
     assertNull(firebaseOptions.getDatabaseUrl());
     assertNull(firebaseOptions.getStorageBucket());
+    assertEquals(0, firebaseOptions.getConnectTimeout());
+    assertEquals(0, firebaseOptions.getReadTimeout());
 
     GoogleCredentials credentials = firebaseOptions.getCredentials();
     assertNotNull(credentials);
@@ -200,6 +206,24 @@ public class FirebaseOptionsTest {
     assertEquals(ALL_VALUES_OPTIONS.getJsonFactory(), allValuesOptionsCopy.getJsonFactory());
     assertEquals(ALL_VALUES_OPTIONS.getHttpTransport(), allValuesOptionsCopy.getHttpTransport());
     assertEquals(ALL_VALUES_OPTIONS.getThreadManager(), allValuesOptionsCopy.getThreadManager());
+    assertEquals(ALL_VALUES_OPTIONS.getConnectTimeout(), allValuesOptionsCopy.getConnectTimeout());
+    assertEquals(ALL_VALUES_OPTIONS.getReadTimeout(), allValuesOptionsCopy.getReadTimeout());
+  }
+
+  @Test(expected = IllegalArgumentException.class)
+  public void createOptionsWithInvalidConnectTimeout() {
+    new FirebaseOptions.Builder()
+        .setCredentials(TestUtils.getCertCredential(ServiceAccount.EDITOR.asStream()))
+        .setConnectTimeout(-1)
+        .build();
+  }
+
+  @Test(expected = IllegalArgumentException.class)
+  public void createOptionsWithInvalidReadTimeout() {
+    new FirebaseOptions.Builder()
+        .setCredentials(TestUtils.getCertCredential(ServiceAccount.EDITOR.asStream()))
+        .setReadTimeout(-1)
+        .build();
   }
 
   @Test
diff --git a/src/test/java/com/google/firebase/auth/FirebaseUserManagerTest.java b/src/test/java/com/google/firebase/auth/FirebaseUserManagerTest.java
index de6862b..3f29b06 100644
--- a/src/test/java/com/google/firebase/auth/FirebaseUserManagerTest.java
+++ b/src/test/java/com/google/firebase/auth/FirebaseUserManagerTest.java
@@ -25,6 +25,7 @@ import static org.junit.Assert.fail;
 
 import com.google.api.client.googleapis.util.Utils;
 import com.google.api.client.http.HttpHeaders;
+import com.google.api.client.http.HttpRequest;
 import com.google.api.client.http.HttpResponseException;
 import com.google.api.client.json.GenericJson;
 import com.google.api.client.json.JsonFactory;
@@ -382,6 +383,27 @@ public class FirebaseUserManagerTest {
     }
   }
 
+  @Test
+  public void testTimeout() throws Exception {
+    MockHttpTransport transport = new MultiRequestMockHttpTransport(ImmutableList.of(
+        new MockLowLevelHttpResponse().setContent(TestUtils.loadResource("getUser.json"))));
+    FirebaseApp.initializeApp(new FirebaseOptions.Builder()
+        .setCredentials(credentials)
+        .setHttpTransport(transport)
+        .setConnectTimeout(30000)
+        .setReadTimeout(60000)
+        .build());
+    FirebaseAuth auth = FirebaseAuth.getInstance();
+    FirebaseUserManager userManager = auth.getUserManager();
+    TestResponseInterceptor interceptor = new TestResponseInterceptor();
+    userManager.setInterceptor(interceptor);
+
+    FirebaseAuth.getInstance().getUserAsync("testuser").get();
+    HttpRequest request = interceptor.getResponse().getRequest();
+    assertEquals(30000, request.getConnectTimeout());
+    assertEquals(60000, request.getReadTimeout());
+  }
+
   @Test
   public void testUserBuilder() {
     Map<String, Object> map = new CreateRequest().getProperties();
diff --git a/src/test/java/com/google/firebase/internal/FirebaseRequestInitializerTest.java b/src/test/java/com/google/firebase/internal/FirebaseRequestInitializerTest.java
new file mode 100644
index 0000000..c7919eb
--- /dev/null
+++ b/src/test/java/com/google/firebase/internal/FirebaseRequestInitializerTest.java
@@ -0,0 +1,71 @@
+/*
+ * Copyright 2018 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 org.junit.Assert.assertEquals;
+
+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.HttpTransport;
+import com.google.api.client.testing.http.MockHttpTransport;
+import com.google.firebase.FirebaseApp;
+import com.google.firebase.FirebaseOptions;
+import com.google.firebase.TestOnlyImplFirebaseTrampolines;
+import com.google.firebase.auth.MockGoogleCredentials;
+import org.junit.After;
+import org.junit.Test;
+
+public class FirebaseRequestInitializerTest {
+
+  @After
+  public void tearDown() {
+    TestOnlyImplFirebaseTrampolines.clearInstancesForTest();
+  }
+
+  @Test
+  public void testDefaultTimeouts() throws Exception {
+    FirebaseApp app = FirebaseApp.initializeApp(new FirebaseOptions.Builder()
+        .setCredentials(new MockGoogleCredentials("token"))
+        .build());
+    HttpTransport transport = new MockHttpTransport();
+    HttpRequestFactory factory = transport.createRequestFactory(
+        new FirebaseRequestInitializer(app));
+    HttpRequest request = factory.buildGetRequest(
+        new GenericUrl("https://firebase.google.com"));
+    assertEquals(0, request.getConnectTimeout());
+    assertEquals(0, request.getReadTimeout());
+    assertEquals("Bearer token", request.getHeaders().getAuthorization());
+  }
+
+  @Test
+  public void testExplicitTimeouts() throws Exception {
+    FirebaseApp app = FirebaseApp.initializeApp(new FirebaseOptions.Builder()
+        .setCredentials(new MockGoogleCredentials("token"))
+        .setConnectTimeout(30000)
+        .setReadTimeout(60000)
+        .build());
+    HttpTransport transport = new MockHttpTransport();
+    HttpRequestFactory factory = transport.createRequestFactory(
+        new FirebaseRequestInitializer(app));
+    HttpRequest request = factory.buildGetRequest(
+        new GenericUrl("https://firebase.google.com"));
+    assertEquals(30000, request.getConnectTimeout());
+    assertEquals(60000, request.getReadTimeout());
+    assertEquals("Bearer token", request.getHeaders().getAuthorization());
+  }
+}