Skip to content

Commit

Permalink
fix: update HttpRequest#getVersion to use stable logic (#919)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
BenWhitehead authored and chingor13 committed Dec 17, 2019
1 parent 7ea53eb commit 853ab4b
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 17 deletions.
Expand Up @@ -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;
}
Expand Down
Expand Up @@ -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;
Expand All @@ -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;

/**
Expand Down Expand Up @@ -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)
);
}
}

0 comments on commit 853ab4b

Please sign in to comment.