Repository: googleapis/google-http-java-client (java)

Issue:
Mandatory dependence to javax.annotation in osgi environment
#### Environment details
OS:Linux
Java version:1.8
google-http-java-client version: 1.39.1

#### Any additional information below
osgi manifest requires javax.annotation, which makes jsr305 bundle mandatory at runtime (in osgi)

Import-Package: com.google.common.base;version="[30.1,31)",com.google.
 common.collect;version="[30.1,31)",com.google.common.io;version="[30.
 1,31)",com.google.common.util.concurrent;version="[30.1,31)",io.openc
 ensus.common,io.opencensus.contrib.http.util,io.opencensus.trace,io.o
 pencensus.trace.export,io.opencensus.trace.propagation,**javax.annotati
 on;version="[3.0,4)"**,javax.net.ssl,org.apache.http,org.apache.http.cl
 ient,org.apache.http.client.methods,org.apache.http.conn,org.apache.h
 ttp.conn.params,org.apache.http.conn.routing,org.apache.http.conn.sch
 eme,org.apache.http.conn.ssl,org.apache.http.entity,org.apache.http.i
 mpl.client,org.apache.http.impl.conn,org.apache.http.impl.conn.tsccm,
 org.apache.http.message,org.apache.http.params,org.apache.http.protoc
 ol

javax.annotation should not be mandatory at runtime 

Pull request:
fix: make dependency on javax.annotation optional (#1323)
This PR changes the configuration of the `maven-bundle-plugin` so that the generated `MANIFEST.MF` declares the runtime depencency on `javax.annotation` as optional.

This way, OSGI environments try not to resolve `javax.annotation` from findbugs that could collide with `javax.annotation` from other bundles.

- [X] Make sure to open an issue as a [bug/issue](https://github.com/googleapis/google-http-java-client/issues/new/choose) before writing your code!  That way we can discuss the change, evaluate designs, and agree on the general idea
- [X] Ensure the tests and linter pass
- [X] Code coverage does not decrease (if any source code was changed)
> No source Code was changed
- [X] Appropriate docs were updated (if necessary)
> Not necessary

Fixes #1323  ☕️


Patch:
diff --git a/.kokoro/coerce_logs.sh b/.kokoro/coerce_logs.sh
index 5cf7ba49..46edbf7f 100755
--- a/.kokoro/coerce_logs.sh
+++ b/.kokoro/coerce_logs.sh
@@ -28,7 +28,6 @@ job=$(basename ${KOKORO_JOB_NAME})
 echo "coercing sponge logs..."
 for xml in `find . -name *-sponge_log.xml`
 do
-  echo "processing ${xml}"
   class=$(basename ${xml} | cut -d- -f2)
   dir=$(dirname ${xml})/${job}/${class}
   text=$(dirname ${xml})/${class}-sponge_log.txt
diff --git a/.kokoro/dependencies.sh b/.kokoro/dependencies.sh
index 59d2aafc..9030ba8f 100755
--- a/.kokoro/dependencies.sh
+++ b/.kokoro/dependencies.sh
@@ -14,6 +14,7 @@
 # limitations under the License.
 
 set -eo pipefail
+shopt -s nullglob
 
 ## Get the directory of the build script
 scriptDir=$(realpath $(dirname "${BASH_SOURCE[0]}"))
@@ -46,7 +47,7 @@ function completenessCheck() {
   # This is stripped from the output as it is not present in the flattened pom.
   # Only dependencies with 'compile' or 'runtime' scope are included from original dependency list.
   msg "Generating dependency list using original pom..."
-  mvn dependency:list -f pom.xml -DincludeScope=runtime -Dsort=true | grep '\[INFO]    .*:.*:.*:.*:.*' | sed -e s/\\s--\\smodule.*// >.org-list.txt
+  mvn dependency:list -f pom.xml -DincludeScope=runtime -Dsort=true | grep '\[INFO]    .*:.*:.*:.*:.*' | sed -e 's/ --.*//' >.org-list.txt
 
   # Output dep list generated using the flattened pom (only 'compile' and 'runtime' scopes)
   msg "Generating dependency list using flattened pom..."
@@ -70,7 +71,7 @@ function completenessCheck() {
 set +e
 
 error_count=0
-for path in $(find -name ".flattened-pom.xml")
+for path in **/.flattened-pom.xml
 do
   # Check flattened pom in each dir that contains it for completeness
   dir=$(dirname "$path")
diff --git a/google-http-client/pom.xml b/google-http-client/pom.xml
index 05680357..5ff0a0d2 100644
--- a/google-http-client/pom.xml
+++ b/google-http-client/pom.xml
@@ -107,6 +107,11 @@
             </goals>   
           </execution>
         </executions>
+        <configuration>
+          <instructions>
+            <Import-Package>javax.annotation;resolution:=optional</Import-Package>
+          </instructions>
+        </configuration>
       </plugin>
     </plugins>
 
diff --git a/google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java b/google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java
index 77497a2a..2ffa4652 100644
--- a/google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java
+++ b/google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java
@@ -18,6 +18,7 @@ import com.google.api.client.util.IOUtils;
 import com.google.api.client.util.LoggingInputStream;
 import com.google.api.client.util.Preconditions;
 import com.google.api.client.util.StringUtils;
+import java.io.BufferedInputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.EOFException;
 import java.io.IOException;
@@ -371,7 +372,13 @@ public final class HttpResponse {
                 new LoggingInputStream(
                     lowLevelResponseContent, logger, Level.CONFIG, contentLoggingLimit);
           }
-          content = lowLevelResponseContent;
+          if (returnRawInputStream) {
+            content = lowLevelResponseContent;
+          } else {
+            // wrap the content with BufferedInputStream to support
+            // mark()/reset() while error checking in error handlers
+            content = new BufferedInputStream(lowLevelResponseContent);
+          }
           contentProcessed = true;
         } catch (EOFException e) {
           // this may happen for example on a HEAD request since there no actual response data read
diff --git a/pom.xml b/pom.xml
index 65310081..dd18c989 100644
--- a/pom.xml
+++ b/pom.xml
@@ -119,7 +119,7 @@
       <dependency>
         <groupId>com.google.truth</groupId>
         <artifactId>truth</artifactId>
-        <version>1.1.2</version>
+        <version>1.1.3</version>
         <scope>test</scope>
       </dependency>
       <dependency>
diff --git a/renovate.json b/renovate.json
index c37889d4..87e183eb 100644
--- a/renovate.json
+++ b/renovate.json
@@ -69,5 +69,8 @@
     }
   ],
   "semanticCommits": true,
-  "masterIssue": true
+  "dependencyDashboard": true,
+  "dependencyDashboardLabels": [
+    "type: process"
+  ]
 }
diff --git a/samples/install-without-bom/pom.xml b/samples/install-without-bom/pom.xml
index 866168cd..7980adb7 100644
--- a/samples/install-without-bom/pom.xml
+++ b/samples/install-without-bom/pom.xml
@@ -42,7 +42,7 @@
     <dependency>
       <groupId>com.google.truth</groupId>
       <artifactId>truth</artifactId>
-      <version>1.1.2</version>
+      <version>1.1.3</version>
       <scope>test</scope>
     </dependency>
   </dependencies>
diff --git a/samples/snapshot/pom.xml b/samples/snapshot/pom.xml
index 08e83355..0cc5be9c 100644
--- a/samples/snapshot/pom.xml
+++ b/samples/snapshot/pom.xml
@@ -40,7 +40,7 @@
     <dependency>
       <groupId>com.google.truth</groupId>
       <artifactId>truth</artifactId>
-      <version>1.1.2</version>
+      <version>1.1.3</version>
       <scope>test</scope>
     </dependency>
   </dependencies>
diff --git a/samples/snippets/pom.xml b/samples/snippets/pom.xml
index f43fa985..79c22405 100644
--- a/samples/snippets/pom.xml
+++ b/samples/snippets/pom.xml
@@ -58,7 +58,7 @@
     <dependency>
       <groupId>com.google.truth</groupId>
       <artifactId>truth</artifactId>
-      <version>1.1.2</version>
+      <version>1.1.3</version>
       <scope>test</scope>
     </dependency>
     <!-- END_EXCLUDE -->
diff --git a/synth.metadata b/synth.metadata
index be82d601..70951b3e 100644
--- a/synth.metadata
+++ b/synth.metadata
@@ -4,14 +4,14 @@
       "git": {
         "name": ".",
         "remote": "https://github.com/googleapis/google-http-java-client.git",
-        "sha": "e69275ecaa4d85372ebc253dd415a02ba63075be"
+        "sha": "933b0bd386f413bd960f81c706edae81d9dc030a"
       }
     },
     {
       "git": {
         "name": "synthtool",
         "remote": "https://github.com/googleapis/synthtool.git",
-        "sha": "8eae0234a16b26c2ff616d305dbd9786c8b10a47"
+        "sha": "5d3c52f5db664eee9026db76b5bcb466f3a09ca9"
       }
     }
   ],
@@ -81,6 +81,7 @@
     "CODE_OF_CONDUCT.md",
     "CONTRIBUTING.md",
     "LICENSE",
+    "SECURITY.md",
     "codecov.yaml",
     "renovate.json",
     "samples/install-without-bom/pom.xml",


Test patch:
diff --git a/google-http-client/src/test/java/com/google/api/client/http/HttpResponseTest.java b/google-http-client/src/test/java/com/google/api/client/http/HttpResponseTest.java
index bfc09b6d..bed019f4 100644
--- a/google-http-client/src/test/java/com/google/api/client/http/HttpResponseTest.java
+++ b/google-http-client/src/test/java/com/google/api/client/http/HttpResponseTest.java
@@ -22,9 +22,13 @@ import com.google.api.client.testing.http.MockLowLevelHttpResponse;
 import com.google.api.client.testing.util.LogRecordingHandler;
 import com.google.api.client.testing.util.TestableByteArrayInputStream;
 import com.google.api.client.util.Key;
+import com.google.common.io.ByteStreams;
+import java.io.BufferedInputStream;
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
+import java.io.FilterInputStream;
 import java.io.IOException;
+import java.io.InputStream;
 import java.lang.reflect.Type;
 import java.nio.charset.StandardCharsets;
 import java.text.NumberFormat;
@@ -59,6 +63,8 @@ public class HttpResponseTest extends TestCase {
   private static final String SAMPLE = "123\u05D9\u05e0\u05D9\u05D1";
   private static final String SAMPLE2 = "123abc";
   private static final String JSON_SAMPLE = "{\"foo\": \"ßar\"}";
+  private static final String ERROR_SAMPLE =
+      "{domain:'global',reason:'domainPolicy',message:'msg'}";
   private static final String VALID_CONTENT_TYPE = "text/plain";
   private static final String VALID_CONTENT_TYPE_WITH_PARAMS =
       "application/vnd.com.google.datastore.entity+json; charset=utf-8; version=v1; q=0.9";
@@ -543,7 +549,8 @@ public class HttpResponseTest extends TestCase {
         };
     HttpRequest request =
         transport.createRequestFactory().buildHeadRequest(HttpTesting.SIMPLE_GENERIC_URL);
-    request.execute().getContent();
+    InputStream noContent = request.execute().getContent();
+    assertNull(noContent);
   }
 
   public void testGetContent_gzipEncoding_ReturnRawStream() throws IOException {
@@ -570,6 +577,9 @@ public class HttpResponseTest extends TestCase {
     assertFalse(
         "it should not decompress stream",
         request.execute().getContent() instanceof GZIPInputStream);
+    assertFalse(
+        "it should not buffer stream",
+        request.execute().getContent() instanceof BufferedInputStream);
   }
 
   public void testGetContent_gzipEncoding_finishReading() throws IOException {
@@ -665,4 +675,71 @@ public class HttpResponseTest extends TestCase {
     HttpResponse response = request.execute();
     assertEquals("abcd", response.parseAsString());
   }
+
+  public void testGetContent_bufferedContent() throws IOException {
+    HttpTransport transport =
+        new MockHttpTransport() {
+          @Override
+          public LowLevelHttpRequest buildRequest(String method, String url) throws IOException {
+            return new MockLowLevelHttpRequest() {
+              @Override
+              public LowLevelHttpResponse execute() throws IOException {
+                // have to use gzip here because MockLowLevelHttpResponse.setContent()
+                // returns BufferedStream by itself, so test always success
+                byte[] dataToCompress = ERROR_SAMPLE.getBytes(StandardCharsets.UTF_8);
+                ByteArrayOutputStream content = new ByteArrayOutputStream(dataToCompress.length);
+                try (GZIPOutputStream zipStream = new GZIPOutputStream((content))) {
+                  zipStream.write(dataToCompress);
+                }
+
+                MockLowLevelHttpResponse result = new MockLowLevelHttpResponse();
+                result.setStatusCode(403);
+                result.setContentType(JSON_CONTENT_TYPE);
+                result.setContentEncoding("gzip");
+                result.setContent(content.toByteArray());
+
+                return result;
+              }
+            };
+          }
+        };
+    HttpRequest request =
+        transport
+            .createRequestFactory()
+            .buildGetRequest(HttpTesting.SIMPLE_GENERIC_URL)
+            .setThrowExceptionOnExecuteError(false);
+
+    HttpResponse response = request.execute();
+    InputStream content = response.getContent();
+    assertTrue(content.markSupported());
+
+    // inspect content like in HttpUnsuccessfulResponseHandler
+    try (RollbackInputStream is = new RollbackInputStream(content)) {
+      byte[] bytes = ByteStreams.toByteArray(is);
+      String text = new String(bytes, response.getContentCharset());
+      assertEquals(ERROR_SAMPLE, text);
+    }
+
+    // original response still parsable by HttpResponseException
+    HttpResponseException exception = new HttpResponseException(response);
+    assertEquals(exception.getStatusCode(), 403);
+    assertEquals(exception.getContent(), ERROR_SAMPLE);
+  }
+
+  static class RollbackInputStream extends FilterInputStream {
+    private boolean closed;
+
+    RollbackInputStream(InputStream in) {
+      super(in);
+      in.mark(8192); // big enough to keep most error messages
+    }
+
+    @Override
+    public void close() throws IOException {
+      if (!closed) {
+        closed = true;
+        in.reset();
+      }
+    }
+  }
 }