From 853ab4ba1bd81420f7b236c2c8f40c4a253a482e Mon Sep 17 00:00:00 2001 From: BenWhitehead Date: Tue, 17 Dec 2019 11:46:05 -0500 Subject: [PATCH] fix: update HttpRequest#getVersion to use stable logic (#919) The original implementation of `getVersion` used the implementation version of the package of the declaring class, however there is differing behavior between Android and openjdk based jvms. In order to be consistent across platforms we will now always resolve the value from the `google-http-client.properties` file that is generated during the build. This method should be stable as it based on loading a classpath resource which has cross jvm support. Fixes #892 --- .../google/api/client/http/HttpRequest.java | 22 +++++++++---------- .../api/client/http/HttpRequestTest.java | 17 +++++++++----- 2 files changed, 22 insertions(+), 17 deletions(-) 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 a2c01d425..23ec3e168 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 @@ -1224,19 +1224,17 @@ private static void addSpanAttribute(Span span, String key, String value) { } private static String getVersion() { - String version = HttpRequest.class.getPackage().getImplementationVersion(); - // in a non-packaged environment (local), there's no implementation version to read - if (version == null) { - // fall back to reading from a properties file - note this value is expected to be cached - try (InputStream inputStream = HttpRequest.class.getResourceAsStream("/google-http-client.properties")) { - if (inputStream != null) { - Properties properties = new Properties(); - properties.load(inputStream); - version = properties.getProperty("google-http-client.version"); - } - } catch (IOException e) { - // ignore + // attempt to read the library's version from a properties file generated during the build + // this value should be read and cached for later use + String version = "unknown-version"; + try (InputStream inputStream = HttpRequest.class.getResourceAsStream("/google-http-client.properties")) { + if (inputStream != null) { + final Properties properties = new Properties(); + properties.load(inputStream); + version = properties.getProperty("google-http-client.version"); } + } catch (IOException e) { + // ignore } return version; } diff --git a/google-http-client/src/test/java/com/google/api/client/http/HttpRequestTest.java b/google-http-client/src/test/java/com/google/api/client/http/HttpRequestTest.java index a76b63850..154f3af16 100644 --- a/google-http-client/src/test/java/com/google/api/client/http/HttpRequestTest.java +++ b/google-http-client/src/test/java/com/google/api/client/http/HttpRequestTest.java @@ -31,10 +31,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Lists; - -import java.util.regex.Pattern; -import junit.framework.TestCase; - import java.io.ByteArrayInputStream; import java.io.IOException; import java.util.Arrays; @@ -45,7 +41,8 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; import java.util.logging.Level; - +import java.util.regex.Pattern; +import junit.framework.TestCase; import org.junit.Assert; /** @@ -1267,4 +1264,14 @@ public void testExecute_curlLoggerWithContentEncoding() throws Exception { } assertTrue(found); } + + public void testVersion_matchesAcceptablePatterns() throws Exception { + String acceptableVersionPattern = + "unknown-version|(?:\\d+\\.\\d+\\.\\d+(?:-.*?)?(?:-SNAPSHOT)?)"; + String version = HttpRequest.VERSION; + assertTrue( + String.format("the loaded version '%s' did not match the acceptable pattern", version), + version.matches(acceptableVersionPattern) + ); + } }