From 6438e7eae11849d9821388ba451696e391b2ba82 Mon Sep 17 00:00:00 2001 From: Joe Wang Date: Wed, 22 Nov 2023 09:53:41 -0500 Subject: [PATCH 1/8] feat: add isShutdown in HttpTransport --- .../extensions/appengine/http/UrlFetchTransport.java | 2 +- .../java/com/google/api/client/http/HttpRequest.java | 2 +- .../java/com/google/api/client/http/HttpTransport.java | 10 ++++++++-- .../api/client/http/javanet/NetHttpTransport.java | 7 ++++--- 4 files changed, 14 insertions(+), 7 deletions(-) diff --git a/google-http-client-appengine/src/main/java/com/google/api/client/extensions/appengine/http/UrlFetchTransport.java b/google-http-client-appengine/src/main/java/com/google/api/client/extensions/appengine/http/UrlFetchTransport.java index b6dfa8d3d..57496f19d 100644 --- a/google-http-client-appengine/src/main/java/com/google/api/client/extensions/appengine/http/UrlFetchTransport.java +++ b/google-http-client-appengine/src/main/java/com/google/api/client/extensions/appengine/http/UrlFetchTransport.java @@ -32,7 +32,7 @@ * *

URL Fetch is only available on Google App Engine (not on any other Java environment), and is * the underlying HTTP transport used for App Engine. Their implementation of {@link - * HttpURLConnection} is simply an abstraction layer on top of URL Fetch. By implementing a + * HttpURLConnection} is simply an abstraction layer on top of URL Fetch. By implementing * transport that directly uses URL Fetch, we can optimize the behavior slightly, and can * potentially take advantage of features in URL Fetch that are not available in {@link * HttpURLConnection}. Furthermore, there is currently a serious bug in how HTTP headers are diff --git a/google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java b/google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java index 78f15d868..3fdbf4ea1 100644 --- a/google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java +++ b/google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java @@ -1103,7 +1103,7 @@ public HttpResponse execute() throws IOException { span.end(OpenCensusUtils.getEndSpanOptions(response == null ? null : response.getStatusCode())); if (response == null) { - // Retries did not help resolve the execute exception, re-throw it. + // Retries did not help resolve the executed exception, re-throw it. throw executeException; } // response interceptor diff --git a/google-http-client/src/main/java/com/google/api/client/http/HttpTransport.java b/google-http-client/src/main/java/com/google/api/client/http/HttpTransport.java index d70d7fb5f..db144516c 100644 --- a/google-http-client/src/main/java/com/google/api/client/http/HttpTransport.java +++ b/google-http-client/src/main/java/com/google/api/client/http/HttpTransport.java @@ -21,7 +21,7 @@ /** * Thread-safe abstract HTTP transport. * - *

Implementation is thread-safe, and sub-classes must be thread-safe. For maximum efficiency, + *

Implementation is thread-safe, and subclasses must be thread-safe. For maximum efficiency, * applications should use a single globally-shared instance of the HTTP transport. * *

The recommended concrete implementation HTTP transport library to use depends on what @@ -157,5 +157,11 @@ public boolean isMtls() { * @throws IOException I/O exception * @since 1.4 */ - public void shutdown() throws IOException {} + public void shutdown() throws IOException { + + } + + public boolean isShutdown() { + return true; + } } diff --git a/google-http-client/src/main/java/com/google/api/client/http/javanet/NetHttpTransport.java b/google-http-client/src/main/java/com/google/api/client/http/javanet/NetHttpTransport.java index 2a0ae6c1f..45bb41e11 100644 --- a/google-http-client/src/main/java/com/google/api/client/http/javanet/NetHttpTransport.java +++ b/google-http-client/src/main/java/com/google/api/client/http/javanet/NetHttpTransport.java @@ -38,10 +38,11 @@ /** * Thread-safe HTTP low-level transport based on the {@code java.net} package. * - *

Users should consider modifying the keep alive property on {@link NetHttpTransport} to control + *

Users should consider modifying the keep alive property on {@link NetHttpTransport} to + * control * whether the socket should be returned to a pool of connected sockets. More information is * available here. + * href="http://docs.oracle.com/javase/7/docs/technotes/guides/net/http-keepalive.html">here. * *

We honor the default global caching behavior. To change the default behavior use {@link * HttpURLConnection#setDefaultUseCaches(boolean)}. @@ -49,8 +50,8 @@ *

Implementation is thread-safe. For maximum efficiency, applications should use a single * globally-shared instance of the HTTP transport. * - * @since 1.0 * @author Yaniv Inbar + * @since 1.0 */ public final class NetHttpTransport extends HttpTransport { private static Proxy defaultProxy() { From 5bb3ac3748814596b780c12dae1dd7c76c99d49b Mon Sep 17 00:00:00 2001 From: Joe Wang Date: Wed, 22 Nov 2023 09:57:14 -0500 Subject: [PATCH 2/8] fix lint --- .../appengine/http/UrlFetchTransport.java | 13 ++++++------- .../com/google/api/client/http/HttpTransport.java | 4 +--- .../api/client/http/javanet/NetHttpTransport.java | 3 +-- 3 files changed, 8 insertions(+), 12 deletions(-) diff --git a/google-http-client-appengine/src/main/java/com/google/api/client/extensions/appengine/http/UrlFetchTransport.java b/google-http-client-appengine/src/main/java/com/google/api/client/extensions/appengine/http/UrlFetchTransport.java index 57496f19d..fdc810de6 100644 --- a/google-http-client-appengine/src/main/java/com/google/api/client/extensions/appengine/http/UrlFetchTransport.java +++ b/google-http-client-appengine/src/main/java/com/google/api/client/extensions/appengine/http/UrlFetchTransport.java @@ -32,13 +32,12 @@ * *

URL Fetch is only available on Google App Engine (not on any other Java environment), and is * the underlying HTTP transport used for App Engine. Their implementation of {@link - * HttpURLConnection} is simply an abstraction layer on top of URL Fetch. By implementing - * transport that directly uses URL Fetch, we can optimize the behavior slightly, and can - * potentially take advantage of features in URL Fetch that are not available in {@link - * HttpURLConnection}. Furthermore, there is currently a serious bug in how HTTP headers are - * processed in the App Engine implementation of {@link HttpURLConnection}, which we are able to - * avoid using this implementation. Therefore, this is the recommended transport to use on App - * Engine. + * HttpURLConnection} is simply an abstraction layer on top of URL Fetch. By implementing transport + * that directly uses URL Fetch, we can optimize the behavior slightly, and can potentially take + * advantage of features in URL Fetch that are not available in {@link HttpURLConnection}. + * Furthermore, there is currently a serious bug in how HTTP headers are processed in the App Engine + * implementation of {@link HttpURLConnection}, which we are able to avoid using this + * implementation. Therefore, this is the recommended transport to use on App Engine. * * @since 1.10 * @author Yaniv Inbar diff --git a/google-http-client/src/main/java/com/google/api/client/http/HttpTransport.java b/google-http-client/src/main/java/com/google/api/client/http/HttpTransport.java index db144516c..a3d23c30b 100644 --- a/google-http-client/src/main/java/com/google/api/client/http/HttpTransport.java +++ b/google-http-client/src/main/java/com/google/api/client/http/HttpTransport.java @@ -157,9 +157,7 @@ public boolean isMtls() { * @throws IOException I/O exception * @since 1.4 */ - public void shutdown() throws IOException { - - } + public void shutdown() throws IOException {} public boolean isShutdown() { return true; diff --git a/google-http-client/src/main/java/com/google/api/client/http/javanet/NetHttpTransport.java b/google-http-client/src/main/java/com/google/api/client/http/javanet/NetHttpTransport.java index 45bb41e11..064d4dea3 100644 --- a/google-http-client/src/main/java/com/google/api/client/http/javanet/NetHttpTransport.java +++ b/google-http-client/src/main/java/com/google/api/client/http/javanet/NetHttpTransport.java @@ -38,8 +38,7 @@ /** * Thread-safe HTTP low-level transport based on the {@code java.net} package. * - *

Users should consider modifying the keep alive property on {@link NetHttpTransport} to - * control + *

Users should consider modifying the keep alive property on {@link NetHttpTransport} to control * whether the socket should be returned to a pool of connected sockets. More information is * available here. From cfaeb9364236072423526b5ac51be77f8aa36438 Mon Sep 17 00:00:00 2001 From: Joe Wang Date: Wed, 22 Nov 2023 10:08:49 -0500 Subject: [PATCH 3/8] add comment --- .../main/java/com/google/api/client/http/HttpTransport.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/google-http-client/src/main/java/com/google/api/client/http/HttpTransport.java b/google-http-client/src/main/java/com/google/api/client/http/HttpTransport.java index a3d23c30b..a7cafe1c0 100644 --- a/google-http-client/src/main/java/com/google/api/client/http/HttpTransport.java +++ b/google-http-client/src/main/java/com/google/api/client/http/HttpTransport.java @@ -159,6 +159,10 @@ public boolean isMtls() { */ public void shutdown() throws IOException {} + /** + * Default implementation of whether the http transport is shutdown or not. + * @since 1.4 + */ public boolean isShutdown() { return true; } From 6f39d67714416fce44beb47c21b61960c73a684d Mon Sep 17 00:00:00 2001 From: Owl Bot Date: Wed, 22 Nov 2023 15:11:14 +0000 Subject: [PATCH 4/8] =?UTF-8?q?=F0=9F=A6=89=20Updates=20from=20OwlBot=20po?= =?UTF-8?q?st-processor?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --- .../src/main/java/com/google/api/client/http/HttpTransport.java | 1 + 1 file changed, 1 insertion(+) diff --git a/google-http-client/src/main/java/com/google/api/client/http/HttpTransport.java b/google-http-client/src/main/java/com/google/api/client/http/HttpTransport.java index a7cafe1c0..de34f7c2f 100644 --- a/google-http-client/src/main/java/com/google/api/client/http/HttpTransport.java +++ b/google-http-client/src/main/java/com/google/api/client/http/HttpTransport.java @@ -161,6 +161,7 @@ public void shutdown() throws IOException {} /** * Default implementation of whether the http transport is shutdown or not. + * * @since 1.4 */ public boolean isShutdown() { From b71c6bc34f94525a223b460e587e707c1290a088 Mon Sep 17 00:00:00 2001 From: Joe Wang Date: Wed, 22 Nov 2023 16:52:48 -0500 Subject: [PATCH 5/8] refactor according to code review --- .../java/com/google/api/client/http/HttpTransport.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/google-http-client/src/main/java/com/google/api/client/http/HttpTransport.java b/google-http-client/src/main/java/com/google/api/client/http/HttpTransport.java index de34f7c2f..d847e941f 100644 --- a/google-http-client/src/main/java/com/google/api/client/http/HttpTransport.java +++ b/google-http-client/src/main/java/com/google/api/client/http/HttpTransport.java @@ -160,11 +160,12 @@ public boolean isMtls() { public void shutdown() throws IOException {} /** - * Default implementation of whether the http transport is shutdown or not. + * Returns whether the transport is shutdown or not. * - * @since 1.4 + * @return true if the transport is shutdown. + * @since 1.44.0 */ public boolean isShutdown() { - return true; + return false; } } From 11731507c681de6649044b4816e293af71db8ea7 Mon Sep 17 00:00:00 2001 From: Joe Wang Date: Thu, 23 Nov 2023 09:53:28 -0500 Subject: [PATCH 6/8] restore comment changes --- .../appengine/http/UrlFetchTransport.java | 13 +++++++------ .../com/google/api/client/http/HttpRequest.java | 2 +- .../api/client/http/javanet/NetHttpTransport.java | 4 ++-- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/google-http-client-appengine/src/main/java/com/google/api/client/extensions/appengine/http/UrlFetchTransport.java b/google-http-client-appengine/src/main/java/com/google/api/client/extensions/appengine/http/UrlFetchTransport.java index fdc810de6..b6dfa8d3d 100644 --- a/google-http-client-appengine/src/main/java/com/google/api/client/extensions/appengine/http/UrlFetchTransport.java +++ b/google-http-client-appengine/src/main/java/com/google/api/client/extensions/appengine/http/UrlFetchTransport.java @@ -32,12 +32,13 @@ * *

URL Fetch is only available on Google App Engine (not on any other Java environment), and is * the underlying HTTP transport used for App Engine. Their implementation of {@link - * HttpURLConnection} is simply an abstraction layer on top of URL Fetch. By implementing transport - * that directly uses URL Fetch, we can optimize the behavior slightly, and can potentially take - * advantage of features in URL Fetch that are not available in {@link HttpURLConnection}. - * Furthermore, there is currently a serious bug in how HTTP headers are processed in the App Engine - * implementation of {@link HttpURLConnection}, which we are able to avoid using this - * implementation. Therefore, this is the recommended transport to use on App Engine. + * HttpURLConnection} is simply an abstraction layer on top of URL Fetch. By implementing a + * transport that directly uses URL Fetch, we can optimize the behavior slightly, and can + * potentially take advantage of features in URL Fetch that are not available in {@link + * HttpURLConnection}. Furthermore, there is currently a serious bug in how HTTP headers are + * processed in the App Engine implementation of {@link HttpURLConnection}, which we are able to + * avoid using this implementation. Therefore, this is the recommended transport to use on App + * Engine. * * @since 1.10 * @author Yaniv Inbar diff --git a/google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java b/google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java index 3fdbf4ea1..78f15d868 100644 --- a/google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java +++ b/google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java @@ -1103,7 +1103,7 @@ public HttpResponse execute() throws IOException { span.end(OpenCensusUtils.getEndSpanOptions(response == null ? null : response.getStatusCode())); if (response == null) { - // Retries did not help resolve the executed exception, re-throw it. + // Retries did not help resolve the execute exception, re-throw it. throw executeException; } // response interceptor diff --git a/google-http-client/src/main/java/com/google/api/client/http/javanet/NetHttpTransport.java b/google-http-client/src/main/java/com/google/api/client/http/javanet/NetHttpTransport.java index 064d4dea3..2a0ae6c1f 100644 --- a/google-http-client/src/main/java/com/google/api/client/http/javanet/NetHttpTransport.java +++ b/google-http-client/src/main/java/com/google/api/client/http/javanet/NetHttpTransport.java @@ -41,7 +41,7 @@ *

Users should consider modifying the keep alive property on {@link NetHttpTransport} to control * whether the socket should be returned to a pool of connected sockets. More information is * available here. + * href='http://docs.oracle.com/javase/7/docs/technotes/guides/net/http-keepalive.html'>here. * *

We honor the default global caching behavior. To change the default behavior use {@link * HttpURLConnection#setDefaultUseCaches(boolean)}. @@ -49,8 +49,8 @@ *

Implementation is thread-safe. For maximum efficiency, applications should use a single * globally-shared instance of the HTTP transport. * - * @author Yaniv Inbar * @since 1.0 + * @author Yaniv Inbar */ public final class NetHttpTransport extends HttpTransport { private static Proxy defaultProxy() { From 5adbcf8dd128a76b17489c56d115fd0c713b13b6 Mon Sep 17 00:00:00 2001 From: Joe Wang Date: Thu, 23 Nov 2023 09:55:10 -0500 Subject: [PATCH 7/8] change year --- .../src/main/java/com/google/api/client/http/HttpTransport.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google-http-client/src/main/java/com/google/api/client/http/HttpTransport.java b/google-http-client/src/main/java/com/google/api/client/http/HttpTransport.java index d847e941f..6fde73217 100644 --- a/google-http-client/src/main/java/com/google/api/client/http/HttpTransport.java +++ b/google-http-client/src/main/java/com/google/api/client/http/HttpTransport.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2010 Google Inc. + * Copyright (c) 2023 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 From 0f445694e94833a779b76edd155bbdd75bd90d1d Mon Sep 17 00:00:00 2001 From: Joe Wang Date: Mon, 27 Nov 2023 11:23:01 -0500 Subject: [PATCH 8/8] change default implementation --- .../main/java/com/google/api/client/http/HttpTransport.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/google-http-client/src/main/java/com/google/api/client/http/HttpTransport.java b/google-http-client/src/main/java/com/google/api/client/http/HttpTransport.java index 6fde73217..9a2d04220 100644 --- a/google-http-client/src/main/java/com/google/api/client/http/HttpTransport.java +++ b/google-http-client/src/main/java/com/google/api/client/http/HttpTransport.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2023 Google Inc. + * Copyright (c) 2010 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 @@ -166,6 +166,6 @@ public void shutdown() throws IOException {} * @since 1.44.0 */ public boolean isShutdown() { - return false; + return true; } }