fix: Make writeTimeout configurable (similar to readTimeout) (#900)

* add option to configure write timeout in FirebaseOptions

* improve tests and fix javadoc

---------

Co-authored-by: Felix Rittler <[email protected]>
Co-authored-by: Lahiru Maramba <[email protected]>

Author: 3 people

Diff for: src/main/java/com/google/firebase/FirebaseOptions.java

@@ -82,6 +82,7 @@ public GoogleCredentials get() {
   private final HttpTransport httpTransport;
   private final int connectTimeout;
   private final int readTimeout;
+  private final int writeTimeout;
   private final JsonFactory jsonFactory;
   private final ThreadManager threadManager;
   private final FirestoreOptions firestoreOptions;
@@ -112,6 +113,8 @@ private FirebaseOptions(@NonNull final FirebaseOptions.Builder builder) {
     this.connectTimeout = builder.connectTimeout;
     checkArgument(builder.readTimeout >= 0);
     this.readTimeout = builder.readTimeout;
+    checkArgument(builder.writeTimeout >= 0);
+    this.writeTimeout = builder.writeTimeout;
     this.firestoreOptions = builder.firestoreOptions;
   }
 
@@ -207,6 +210,16 @@ public int getReadTimeout() {
     return readTimeout;
   }
 
+  /**
+   * Returns the write timeout in milliseconds, which is applied to outgoing REST calls
+   * made by the SDK.
+   *
+   * @return Write timeout in milliseconds. 0 indicates an infinite timeout.
+   */
+  public int getWriteTimeout() {
+    return writeTimeout;
+  }
+
   @NonNull
   ThreadManager getThreadManager() {
     return threadManager;
@@ -260,6 +273,7 @@ public static final class Builder {
     private ThreadManager threadManager;
     private int connectTimeout;
     private int readTimeout;
+    private int writeTimeout;
 
     /**
      * Constructs an empty builder.
@@ -289,6 +303,7 @@ public Builder(FirebaseOptions options) {
       threadManager = options.threadManager;
       connectTimeout = options.connectTimeout;
       readTimeout = options.readTimeout;
+      writeTimeout = options.writeTimeout;
       firestoreOptions = options.firestoreOptions;
     }
 
@@ -495,6 +510,19 @@ public Builder setReadTimeout(int readTimeout) {
       return this;
     }
 
+    /**
+     * Sets the write 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 writeTimeout Write timeout in milliseconds. Must not be negative.
+     * @return This <code>Builder</code> instance is returned so subsequent calls can be chained.
+     */
+    public Builder setWriteTimeout(int writeTimeout) {
+      this.writeTimeout = writeTimeout;
+      return this;
+    }
+
     /**
      * Builds the {@link FirebaseOptions} instance from the previously set options.
      *

Diff for: src/main/java/com/google/firebase/internal/FirebaseRequestInitializer.java

@@ -60,16 +60,19 @@ private static class TimeoutInitializer implements HttpRequestInitializer {
 
     private final int connectTimeoutMillis;
     private final int readTimeoutMillis;
+    private final int writeTimeoutMillis;
 
     TimeoutInitializer(FirebaseOptions options) {
       this.connectTimeoutMillis = options.getConnectTimeout();
       this.readTimeoutMillis = options.getReadTimeout();
+      this.writeTimeoutMillis = options.getWriteTimeout();
     }
 
     @Override
     public void initialize(HttpRequest request) {
       request.setConnectTimeout(connectTimeoutMillis);
       request.setReadTimeout(readTimeoutMillis);
+      request.setWriteTimeout(writeTimeoutMillis);
     }
   }
 }

Diff for: src/test/java/com/google/firebase/FirebaseOptionsTest.java

@@ -38,7 +38,7 @@
 
 import org.junit.Test;
 
-/** 
+/**
  * Tests for {@link FirebaseOptions}.
  */
 public class FirebaseOptionsTest {
@@ -87,6 +87,7 @@ public void createOptionsWithAllValuesSet() throws IOException {
             .setThreadManager(MOCK_THREAD_MANAGER)
             .setConnectTimeout(30000)
             .setReadTimeout(60000)
+            .setWriteTimeout(90000)
             .setFirestoreOptions(firestoreOptions)
             .build();
     assertEquals(FIREBASE_DB_URL, firebaseOptions.getDatabaseUrl());
@@ -97,6 +98,7 @@ public void createOptionsWithAllValuesSet() throws IOException {
     assertSame(MOCK_THREAD_MANAGER, firebaseOptions.getThreadManager());
     assertEquals(30000, firebaseOptions.getConnectTimeout());
     assertEquals(60000, firebaseOptions.getReadTimeout());
+    assertEquals(90000, firebaseOptions.getWriteTimeout());
     assertSame(firestoreOptions, firebaseOptions.getFirestoreOptions());
 
     GoogleCredentials credentials = firebaseOptions.getCredentials();
@@ -209,6 +211,14 @@ public void createOptionsWithInvalidReadTimeout() {
         .build();
   }
 
+  @Test(expected = IllegalArgumentException.class)
+  public void createOptionsWithInvalidWriteTimeout() {
+    FirebaseOptions.builder()
+            .setCredentials(TestUtils.getCertCredential(ServiceAccount.EDITOR.asStream()))
+            .setWriteTimeout(-1)
+            .build();
+  }
+
   @Test
   public void testNotEquals() throws IOException {
     GoogleCredentials credentials = GoogleCredentials.fromStream(ServiceAccount.EDITOR.asStream());

Diff for: src/test/java/com/google/firebase/auth/FirebaseUserManagerTest.java

@@ -979,6 +979,7 @@ public void testTimeout() throws Exception {
         .setHttpTransport(transport)
         .setConnectTimeout(30000)
         .setReadTimeout(60000)
+        .setWriteTimeout(90000)
         .build());
     FirebaseAuth auth = FirebaseAuth.getInstance();
     FirebaseUserManager userManager = auth.getUserManager();
@@ -989,6 +990,7 @@ public void testTimeout() throws Exception {
     HttpRequest request = interceptor.getResponse().getRequest();
     assertEquals(30000, request.getConnectTimeout());
     assertEquals(60000, request.getReadTimeout());
+    assertEquals(90000, request.getWriteTimeout());
   }
 
   @Test

Diff for: src/test/java/com/google/firebase/internal/FirebaseRequestInitializerTest.java

@@ -38,6 +38,7 @@ public class FirebaseRequestInitializerTest {
   private static final int MAX_RETRIES = 5;
   private static final int CONNECT_TIMEOUT_MILLIS = 30000;
   private static final int READ_TIMEOUT_MILLIS = 60000;
+  private static final int WRITE_TIMEOUT_MILLIS = 90000;
 
   @After
   public void tearDown() {
@@ -69,6 +70,7 @@ public void testExplicitTimeouts() throws Exception {
         .setCredentials(new MockGoogleCredentials("token"))
         .setConnectTimeout(CONNECT_TIMEOUT_MILLIS)
         .setReadTimeout(READ_TIMEOUT_MILLIS)
+        .setWriteTimeout(WRITE_TIMEOUT_MILLIS)
         .build());
     HttpRequest request = TestUtils.createRequest();
 
@@ -77,6 +79,7 @@ public void testExplicitTimeouts() throws Exception {
 
     assertEquals(CONNECT_TIMEOUT_MILLIS, request.getConnectTimeout());
     assertEquals(READ_TIMEOUT_MILLIS, request.getReadTimeout());
+    assertEquals(WRITE_TIMEOUT_MILLIS, request.getWriteTimeout());
     assertEquals("Bearer token", request.getHeaders().getAuthorization());
     assertEquals(HttpRequest.DEFAULT_NUMBER_OF_RETRIES, request.getNumberOfRetries());
     assertNull(request.getIOExceptionHandler());