From d10c70e03a18b54b4307ff9279dbc733024a751d Mon Sep 17 00:00:00 2001 From: Jonathan Edey Date: Mon, 20 Jan 2025 15:52:09 -0500 Subject: [PATCH 1/4] fix: `ApiClientUtils.getDefaultTransport()` should return cached instance to an `ApacheHttp2Transport` --- .../google/firebase/internal/ApiClientUtils.java | 6 +++++- .../firebase/internal/ApacheHttp2TransportIT.java | 15 +++++++++++++++ .../firebase/internal/ApiClientUtilsTest.java | 9 +++++++++ 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/google/firebase/internal/ApiClientUtils.java b/src/main/java/com/google/firebase/internal/ApiClientUtils.java index 339726105..9d501fae3 100644 --- a/src/main/java/com/google/firebase/internal/ApiClientUtils.java +++ b/src/main/java/com/google/firebase/internal/ApiClientUtils.java @@ -88,6 +88,10 @@ public static JsonFactory getDefaultJsonFactory() { } public static HttpTransport getDefaultTransport() { - return new ApacheHttp2Transport(); + return TransportInstanceHolder.INSTANCE; + } + + private static class TransportInstanceHolder { + static final HttpTransport INSTANCE = new ApacheHttp2Transport(); } } diff --git a/src/test/java/com/google/firebase/internal/ApacheHttp2TransportIT.java b/src/test/java/com/google/firebase/internal/ApacheHttp2TransportIT.java index 047e1de0b..f175401a8 100644 --- a/src/test/java/com/google/firebase/internal/ApacheHttp2TransportIT.java +++ b/src/test/java/com/google/firebase/internal/ApacheHttp2TransportIT.java @@ -292,6 +292,21 @@ public void testVerifyProxyIsRespected() { } } + @Test + public void testVerifyDefaultTransportReused() { + FirebaseOptions o1 = FirebaseOptions.builder() + .setCredentials(MOCK_CREDENTIALS) + .build(); + + FirebaseOptions o2 = FirebaseOptions.builder() + .setCredentials(MOCK_CREDENTIALS) + .build(); + + HttpTransport t1 = o1.getHttpTransport(); + HttpTransport t2 = o2.getHttpTransport(); + assertEquals(t1, t2); + } + private static ErrorHandlingHttpClient getHttpClient(boolean authorized, FirebaseApp app) { HttpRequestFactory requestFactory; diff --git a/src/test/java/com/google/firebase/internal/ApiClientUtilsTest.java b/src/test/java/com/google/firebase/internal/ApiClientUtilsTest.java index f23c7a34d..e0e619f3c 100644 --- a/src/test/java/com/google/firebase/internal/ApiClientUtilsTest.java +++ b/src/test/java/com/google/firebase/internal/ApiClientUtilsTest.java @@ -25,6 +25,7 @@ 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.HttpTransport; import com.google.api.client.http.HttpUnsuccessfulResponseHandler; import com.google.api.client.testing.http.MockHttpTransport; import com.google.api.client.testing.http.MockLowLevelHttpResponse; @@ -128,4 +129,12 @@ public void disconnect() throws IOException { assertTrue(lowLevelResponse.isDisconnected()); } + + @Test + public void testVerifyDefaultTransportReused() { + HttpTransport t1 = ApiClientUtils.getDefaultTransport(); + HttpTransport t2 = ApiClientUtils.getDefaultTransport(); + assertEquals(t1, t2); + } + } From 2014b6fa950286068e2ad6ba6d7bd4edbfcaf1b7 Mon Sep 17 00:00:00 2001 From: Jonathan Edey Date: Mon, 20 Jan 2025 16:10:18 -0500 Subject: [PATCH 2/4] fix: lint --- .../google/firebase/internal/ApacheHttp2TransportIT.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/test/java/com/google/firebase/internal/ApacheHttp2TransportIT.java b/src/test/java/com/google/firebase/internal/ApacheHttp2TransportIT.java index f175401a8..58fd23da3 100644 --- a/src/test/java/com/google/firebase/internal/ApacheHttp2TransportIT.java +++ b/src/test/java/com/google/firebase/internal/ApacheHttp2TransportIT.java @@ -295,12 +295,12 @@ public void testVerifyProxyIsRespected() { @Test public void testVerifyDefaultTransportReused() { FirebaseOptions o1 = FirebaseOptions.builder() - .setCredentials(MOCK_CREDENTIALS) - .build(); + .setCredentials(MOCK_CREDENTIALS) + .build(); FirebaseOptions o2 = FirebaseOptions.builder() - .setCredentials(MOCK_CREDENTIALS) - .build(); + .setCredentials(MOCK_CREDENTIALS) + .build(); HttpTransport t1 = o1.getHttpTransport(); HttpTransport t2 = o2.getHttpTransport(); From d00b7ac493e8e2882bb9469f94a12cf8aea66906 Mon Sep 17 00:00:00 2001 From: Jonathan Edey Date: Mon, 20 Jan 2025 16:38:31 -0500 Subject: [PATCH 3/4] reduce timeout time --- .../com/google/firebase/internal/ApacheHttp2TransportIT.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/java/com/google/firebase/internal/ApacheHttp2TransportIT.java b/src/test/java/com/google/firebase/internal/ApacheHttp2TransportIT.java index 58fd23da3..d4401c2ba 100644 --- a/src/test/java/com/google/firebase/internal/ApacheHttp2TransportIT.java +++ b/src/test/java/com/google/firebase/internal/ApacheHttp2TransportIT.java @@ -200,7 +200,7 @@ public void testReadTimeoutAuthorizedPost() throws FirebaseException { public void testWriteTimeoutAuthorizedGet() throws FirebaseException { app = FirebaseApp.initializeApp(FirebaseOptions.builder() .setCredentials(MOCK_CREDENTIALS) - .setWriteTimeout(100) + .setWriteTimeout(1) .build(), "test-app"); ErrorHandlingHttpClient httpClient = getHttpClient(true, app); HttpRequestInfo request = HttpRequestInfo.buildGetRequest(GET_URL); @@ -219,7 +219,7 @@ public void testWriteTimeoutAuthorizedGet() throws FirebaseException { public void testWriteTimeoutAuthorizedPost() throws FirebaseException { app = FirebaseApp.initializeApp(FirebaseOptions.builder() .setCredentials(MOCK_CREDENTIALS) - .setWriteTimeout(100) + .setWriteTimeout(1) .build(), "test-app"); ErrorHandlingHttpClient httpClient = getHttpClient(true, app); HttpRequestInfo request = HttpRequestInfo.buildJsonPostRequest(POST_URL, payload); From fa2418679390d5c01e711a54192a48683b617834 Mon Sep 17 00:00:00 2001 From: Jonathan Edey Date: Thu, 23 Jan 2025 10:21:20 -0500 Subject: [PATCH 4/4] fix: use new transport to trigger writeTimeout --- .../firebase/internal/ApacheHttp2TransportIT.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/test/java/com/google/firebase/internal/ApacheHttp2TransportIT.java b/src/test/java/com/google/firebase/internal/ApacheHttp2TransportIT.java index d4401c2ba..0beb1b3b4 100644 --- a/src/test/java/com/google/firebase/internal/ApacheHttp2TransportIT.java +++ b/src/test/java/com/google/firebase/internal/ApacheHttp2TransportIT.java @@ -198,9 +198,12 @@ public void testReadTimeoutAuthorizedPost() throws FirebaseException { @Test(timeout = 10_000L) public void testWriteTimeoutAuthorizedGet() throws FirebaseException { + // Use a fresh transport so that writeTimeout triggers while waiting for the transport to + // be ready to receive data. app = FirebaseApp.initializeApp(FirebaseOptions.builder() .setCredentials(MOCK_CREDENTIALS) - .setWriteTimeout(1) + .setWriteTimeout(100) + .setHttpTransport(new ApacheHttp2Transport()) .build(), "test-app"); ErrorHandlingHttpClient httpClient = getHttpClient(true, app); HttpRequestInfo request = HttpRequestInfo.buildGetRequest(GET_URL); @@ -217,9 +220,12 @@ public void testWriteTimeoutAuthorizedGet() throws FirebaseException { @Test(timeout = 10_000L) public void testWriteTimeoutAuthorizedPost() throws FirebaseException { + // Use a fresh transport so that writeTimeout triggers while waiting for the transport to + // be ready to receive data. app = FirebaseApp.initializeApp(FirebaseOptions.builder() .setCredentials(MOCK_CREDENTIALS) - .setWriteTimeout(1) + .setWriteTimeout(100) + .setHttpTransport(new ApacheHttp2Transport()) .build(), "test-app"); ErrorHandlingHttpClient httpClient = getHttpClient(true, app); HttpRequestInfo request = HttpRequestInfo.buildJsonPostRequest(POST_URL, payload);