From d02a5c3c8bbf3fe05b262125358677847b303688 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Wed, 5 Aug 2015 17:03:31 -0700 Subject: [PATCH] 474361 - Handle JVM version extensions like -internal + Refreshed Version.java to handle parsing alternate syntaxes + Introduced Version.toShortString() for the non-suffixed versions + Using updated Version obj in StartArgs.setProperty("java.version") to allow both normal System property access and arbitrary property setting on command line to work in a consistent fashion. --- .../java/org/eclipse/jetty/start/Modules.java | 6 +- .../org/eclipse/jetty/start/StartArgs.java | 26 +- .../java/org/eclipse/jetty/start/Version.java | 238 +++++++++++++----- .../jetty/start/ConfigurationAssert.java | 3 +- .../org/eclipse/jetty/start/ModulesTest.java | 15 +- .../eclipse/jetty/start/TestBadUseCases.java | 4 +- .../org/eclipse/jetty/start/VersionTest.java | 35 +++ .../test/resources/assert-home-with-http2.txt | 2 +- .../usecases/agent-properties.assert.txt | 2 +- .../test/resources/usecases/http2.assert.txt | 2 +- 10 files changed, 241 insertions(+), 92 deletions(-) diff --git a/jetty-start/src/main/java/org/eclipse/jetty/start/Modules.java b/jetty-start/src/main/java/org/eclipse/jetty/start/Modules.java index 8a078a7cb05..68db15e627d 100644 --- a/jetty-start/src/main/java/org/eclipse/jetty/start/Modules.java +++ b/jetty-start/src/main/java/org/eclipse/jetty/start/Modules.java @@ -47,11 +47,7 @@ public class Modules extends Graph String java_version = System.getProperty("java.version"); if (java_version!=null) { - String[] parts = java_version.split("\\."); - if (parts!=null && parts.length>0) - System.setProperty("java.version.major",parts[0]); - if (parts!=null && parts.length>1) - System.setProperty("java.version.minor",parts[1]); + args.setProperty("java.version",java_version,"",false); } } diff --git a/jetty-start/src/main/java/org/eclipse/jetty/start/StartArgs.java b/jetty-start/src/main/java/org/eclipse/jetty/start/StartArgs.java index 88e9688ff33..f810a80c7ea 100644 --- a/jetty-start/src/main/java/org/eclipse/jetty/start/StartArgs.java +++ b/jetty-start/src/main/java/org/eclipse/jetty/start/StartArgs.java @@ -665,7 +665,7 @@ public class StartArgs { return properties; } - + public Set getSkipFileValidationModules() { return skipFileValidationModules; @@ -1125,8 +1125,8 @@ public class StartArgs { this.allModules = allModules; } - - private void setProperty(String key, String value, String source, boolean replaceProp) + + public void setProperty(String key, String value, String source, boolean replaceProp) { // Special / Prevent override from start.ini's if (key.equals("jetty.home")) @@ -1141,19 +1141,19 @@ public class StartArgs properties.setProperty("jetty.base",System.getProperty("jetty.base"),source); return; } - - // Normal - if (replaceProp) + + if (replaceProp || (!properties.containsKey(key))) { - // always override properties.setProperty(key,value,source); - } - else - { - // only set if unset - if (!properties.containsKey(key)) + if(key.equals("java.version")) { - properties.setProperty(key,value,source); + Version ver = new Version(value); + + properties.setProperty("java.version",ver.toShortString(),source); + properties.setProperty("java.version.major",Integer.toString(ver.getLegacyMajor()),source); + properties.setProperty("java.version.minor",Integer.toString(ver.getMajor()),source); + properties.setProperty("java.version.revision",Integer.toString(ver.getRevision()),source); + properties.setProperty("java.version.update",Integer.toString(ver.getUpdate()),source); } } } diff --git a/jetty-start/src/main/java/org/eclipse/jetty/start/Version.java b/jetty-start/src/main/java/org/eclipse/jetty/start/Version.java index e47e7da7285..1cf5fd9ce3c 100644 --- a/jetty-start/src/main/java/org/eclipse/jetty/start/Version.java +++ b/jetty-start/src/main/java/org/eclipse/jetty/start/Version.java @@ -19,20 +19,51 @@ package org.eclipse.jetty.start; /** - * Utility class for parsing and comparing version strings. JDK 1.1 compatible. + * Utility class for parsing and comparing version strings. + *

+ * http://www.oracle.com/technetwork/java/javase/namechange-140185.html */ public class Version implements Comparable { - private int _version = 0; - private int _revision = -1; - private int _subrevision = -1; - private String _suffix = ""; + /** + * The major version for java is always "1" (per + * legacy versioning history) + */ + private int legacyMajor = 0; + /** + * The true major version is the second value ("1.5" == "Java 5", "1.8" = "Java 8", etc..) + */ + private int major = -1; + /** + * The revision of the version. + *

+ * This value is always "0" (also per legacy versioning history) + */ + private int revision = -1; + /** + * The update (where bug fixes are placed) + */ + private int update = -1; + /** + * Extra versioning information present on the version string, but not relevant for version comparison reason. + * (eg: with "1.8.0_45-internal", the suffix would be "-internal") + */ + private String suffix = ""; - public Version(String version_string) + private static enum ParseState { - parse(version_string); + LEGACY, + MAJOR, + REVISION, + UPDATE; } - + + public Version(String versionString) + { + parse(versionString); + } + @Override /** * Compares with other version. Does not take extension into account, as there is no reliable way to order them. @@ -46,49 +77,82 @@ public class Version implements Comparable { throw new NullPointerException("other version is null"); } - if (this._version < other._version) + if (this.legacyMajor < other.legacyMajor) { return -1; } - if (this._version > other._version) + if (this.legacyMajor > other.legacyMajor) { return 1; } - if (this._revision < other._revision) + if (this.major < other.major) { return -1; } - if (this._revision > other._revision) + if (this.major > other.major) { return 1; } - if (this._subrevision < other._subrevision) + if (this.revision < other.revision) { return -1; } - if (this._subrevision > other._subrevision) + if (this.revision > other.revision) + { + return 1; + } + if (this.update < other.update) + { + return -1; + } + if (this.update > other.update) { return 1; } return 0; } - + + public int getLegacyMajor() + { + return legacyMajor; + } + + public int getMajor() + { + return major; + } + + public int getRevision() + { + return revision; + } + + public int getUpdate() + { + return update; + } + + public String getSuffix() + { + return suffix; + } + public boolean isNewerThan(Version other) { return compareTo(other) == 1; } - + public boolean isNewerThanOrEqualTo(Version other) { int comp = compareTo(other); return (comp == 0) || (comp == 1); } - + public boolean isOlderThan(Version other) { return compareTo(other) == -1; } - + public boolean isOlderThanOrEqualTo(Version other) { int comp = compareTo(other); @@ -98,8 +162,10 @@ public class Version implements Comparable /** * Check whether this version is in range of versions specified * - * @param low the low part of the range - * @param high the high part of the range + * @param low + * the low part of the range + * @param high + * the high part of the range * @return true if this version is within the provided range */ public boolean isInRange(Version low, Version high) @@ -108,47 +174,71 @@ public class Version implements Comparable } /** - * parses version string in the form version[.revision[.subrevision[extension]]] into this instance. - * @param version_string the version string + * parses version string in the form legacy[.major[.revision[_update[-suffix]]]] into this instance. + * + * @param versionStr + * the version string */ - public void parse(String version_string) + private void parse(String versionStr) { - _version = 0; - _revision = -1; - _subrevision = -1; - _suffix = ""; - int pos = 0; - int startpos = 0; - int endpos = version_string.length(); - while ((pos < endpos) && Character.isDigit(version_string.charAt(pos))) + legacyMajor = 0; + major = -1; + revision = -1; + update = -1; + suffix = ""; + + ParseState state = ParseState.LEGACY; + int offset = 0; + int len = versionStr.length(); + int val = 0; + while (offset < len) { - pos++; - } - _version = Integer.parseInt(version_string.substring(startpos,pos)); - if ((pos < endpos) && (version_string.charAt(pos) == '.')) - { - startpos = ++pos; - while ((pos < endpos) && Character.isDigit(version_string.charAt(pos))) + char c = versionStr.charAt(offset); + boolean isSeparator = !Character.isLetterOrDigit(c); + if (isSeparator) { - pos++; + val = 0; } - _revision = Integer.parseInt(version_string.substring(startpos,pos)); - } - if ((pos < endpos) && (version_string.charAt(pos) == '.')) - { - startpos = ++pos; - while ((pos < endpos) && Character.isDigit(version_string.charAt(pos))) + else if (Character.isDigit(c)) { - pos++; + val = (val * 10) + (c - '0'); } - _subrevision = Integer.parseInt(version_string.substring(startpos,pos)); - } - if (pos < endpos) - { - _suffix = version_string.substring(pos); + else if (Character.isLetter(c)) + { + suffix = versionStr.substring(offset); + return; + } + + switch (state) + { + case LEGACY: + if (isSeparator) + state = ParseState.MAJOR; + else + legacyMajor = val; + break; + case MAJOR: + if (isSeparator) + state = ParseState.REVISION; + else + major = val; + break; + case REVISION: + if (isSeparator) + state = ParseState.UPDATE; + else + revision = val; + break; + case UPDATE: + if (!isSeparator) + update = val; + break; + } + + offset++; } } - + /** * @return string representation of this version */ @@ -156,16 +246,44 @@ public class Version implements Comparable public String toString() { StringBuffer sb = new StringBuffer(10); - sb.append(_version); - if (_revision >= 0) + sb.append(legacyMajor); + if (major >= 0) { - sb.append('.'); - sb.append(_revision); - if (_subrevision >= 0) + sb.append('.').append(major); + if (revision >= 0) { - sb.append('.'); - sb.append(_subrevision); - sb.append(_suffix); + sb.append('.').append(revision); + if (update >= 0) + { + sb.append('_').append(update); + } + } + } + if (Utils.isNotBlank(suffix)) + { + sb.append('-').append(suffix); + } + return sb.toString(); + } + + /** + * Return short string form (without suffix) + * @return string the short version string form + */ + public String toShortString() + { + StringBuffer sb = new StringBuffer(10); + sb.append(legacyMajor); + if (major >= 0) + { + sb.append('.').append(major); + if (revision >= 0) + { + sb.append('.').append(revision); + if (update >= 0) + { + sb.append('_').append(update); + } } } return sb.toString(); diff --git a/jetty-start/src/test/java/org/eclipse/jetty/start/ConfigurationAssert.java b/jetty-start/src/test/java/org/eclipse/jetty/start/ConfigurationAssert.java index ef365b94c14..e651030ff94 100644 --- a/jetty-start/src/test/java/org/eclipse/jetty/start/ConfigurationAssert.java +++ b/jetty-start/src/test/java/org/eclipse/jetty/start/ConfigurationAssert.java @@ -103,7 +103,8 @@ public class ConfigurationAssert { String name = prop.key; if ("jetty.home".equals(name) || "jetty.base".equals(name) || - "user.dir".equals(name) || prop.origin.equals(Props.ORIGIN_SYSPROP)) + "user.dir".equals(name) || prop.origin.equals(Props.ORIGIN_SYSPROP) || + name.startsWith("java.")) { // strip these out from assertion, to make assertions easier. continue; diff --git a/jetty-start/src/test/java/org/eclipse/jetty/start/ModulesTest.java b/jetty-start/src/test/java/org/eclipse/jetty/start/ModulesTest.java index 3efc1c10cc8..ceea461e00f 100644 --- a/jetty-start/src/test/java/org/eclipse/jetty/start/ModulesTest.java +++ b/jetty-start/src/test/java/org/eclipse/jetty/start/ModulesTest.java @@ -53,7 +53,7 @@ public class ModulesTest { // Test Env File homeDir = MavenTestingUtils.getTestResourceDir("dist-home"); - File baseDir = testdir.getEmptyDir(); + File baseDir = testdir.getEmptyPathDir().toFile(); String cmdLine[] = new String[] { "jetty.version=TEST" }; // Configuration @@ -74,9 +74,8 @@ public class ModulesTest modules.registerAll(); // Check versions - assertThat(System.getProperty("java.version.major"),equalTo("1")); - assertThat(System.getProperty("java.version.minor"),anyOf(equalTo("7"),Matchers.equalTo("8"),Matchers.equalTo("9"))); - + assertThat("java.version.major", args.getProperties().getString("java.version.major"),equalTo("1")); + assertThat("java.version.minor", args.getProperties().getString("java.version.minor"),anyOf(equalTo("7"),Matchers.equalTo("8"),Matchers.equalTo("9"))); List moduleNames = new ArrayList<>(); for (Module mod : modules) @@ -197,7 +196,7 @@ public class ModulesTest { // Test Env File homeDir = MavenTestingUtils.getTestResourceDir("dist-home"); - File baseDir = testdir.getEmptyDir(); + File baseDir = testdir.getEmptyPathDir().toFile(); String cmdLine[] = new String[] { "jetty.version=TEST", "java.version=1.8.0_31" }; // Configuration @@ -263,7 +262,7 @@ public class ModulesTest { // Test Env File homeDir = MavenTestingUtils.getTestResourceDir("dist-home"); - File baseDir = testdir.getEmptyDir(); + File baseDir = testdir.getEmptyPathDir().toFile(); String cmdLine[] = new String[] { "jetty.version=TEST" }; // Configuration @@ -332,7 +331,7 @@ public class ModulesTest { // Test Env File homeDir = MavenTestingUtils.getTestResourceDir("dist-home"); - File baseDir = testdir.getEmptyDir(); + File baseDir = testdir.getEmptyPathDir().toFile(); String cmdLine[] = new String[] { "jetty.version=TEST" }; // Configuration @@ -420,7 +419,7 @@ public class ModulesTest { // Test Env File homeDir = MavenTestingUtils.getTestResourceDir("dist-home"); - File baseDir = testdir.getEmptyDir(); + File baseDir = testdir.getEmptyPathDir().toFile(); String cmdLine[] = new String[] { "jetty.version=TEST" }; // Configuration diff --git a/jetty-start/src/test/java/org/eclipse/jetty/start/TestBadUseCases.java b/jetty-start/src/test/java/org/eclipse/jetty/start/TestBadUseCases.java index 1f4a6f3ec4f..4b915834b1f 100644 --- a/jetty-start/src/test/java/org/eclipse/jetty/start/TestBadUseCases.java +++ b/jetty-start/src/test/java/org/eclipse/jetty/start/TestBadUseCases.java @@ -46,8 +46,8 @@ public class TestBadUseCases List ret = new ArrayList<>(); ret.add(new Object[]{ "http2", - "Missing referenced dependency: alpn-impl/alpn-0.0.0_00", - new String[]{"java.version=0.0.0_00"}}); + "Missing referenced dependency: alpn-impl/alpn-0.0.0_0", + new String[]{"java.version=0.0.0_0"}}); ret.add(new Object[]{ "versioned-modules-too-new", "Module [http3] specifies jetty version [10.0] which is newer than this version of jetty [" + RebuildTestResources.JETTY_VERSION + "]", diff --git a/jetty-start/src/test/java/org/eclipse/jetty/start/VersionTest.java b/jetty-start/src/test/java/org/eclipse/jetty/start/VersionTest.java index 81727e23549..f7df07b8f1f 100644 --- a/jetty-start/src/test/java/org/eclipse/jetty/start/VersionTest.java +++ b/jetty-start/src/test/java/org/eclipse/jetty/start/VersionTest.java @@ -25,6 +25,41 @@ import org.junit.Test; public class VersionTest { + @Test + public void testParse() + { + assertParse("1.8.0_45",1,8,0,45); + assertParse("1.8.0_45-internal",1,8,0,45); + assertParse("1.8.0-debug",1,8,0,-1); + } + + private void assertParse(String verStr, int legacyMajor, int major, int revision, int update) + { + Version ver = new Version(verStr); + assertThat("Version [" + verStr + "].legacyMajor", ver.getLegacyMajor(), is(legacyMajor)); + assertThat("Version [" + verStr + "].major", ver.getMajor(), is(major)); + assertThat("Version [" + verStr + "].revision", ver.getRevision(), is(revision)); + assertThat("Version [" + verStr + "].update", ver.getUpdate(), is(update)); + + assertThat("Version [" + verStr + "].toString", ver.toString(), is(verStr)); + } + + @Test + public void testToShortString() + { + assertToShortString("1.8","1.8"); + assertToShortString("1.8.0","1.8.0"); + assertToShortString("1.8.0_45","1.8.0_45"); + assertToShortString("1.8.0_45-internal","1.8.0_45"); + assertToShortString("1.8.0-debug","1.8.0"); + } + + private void assertToShortString(String verStr, String expectedShortString) + { + Version ver = new Version(verStr); + assertThat("Version [" + verStr + "].toShortString", ver.toShortString(), is(expectedShortString)); + } + @Test public void testNewerVersion() { assertIsNewer("0.0.0", "0.0.1"); diff --git a/jetty-start/src/test/resources/assert-home-with-http2.txt b/jetty-start/src/test/resources/assert-home-with-http2.txt index 3d8973b2520..f02d1649c79 100644 --- a/jetty-start/src/test/resources/assert-home-with-http2.txt +++ b/jetty-start/src/test/resources/assert-home-with-http2.txt @@ -53,7 +53,7 @@ LIB|${jetty.base}/lib/jetty-jndi-9.3.jar # The Properties we expect (order is irrelevant) # (these are the properties we actually set in the configuration) -PROP|java.version=1.8.0_31 +# PROP|java.version=1.8.0_31 PROP|jetty.http.port=8080 PROP|jetty.httpConfig.delayDispatchUntilContent=false PROP|jetty.server.dumpAfterStart=false diff --git a/jetty-start/src/test/resources/usecases/agent-properties.assert.txt b/jetty-start/src/test/resources/usecases/agent-properties.assert.txt index 5707b4ce405..664fec83815 100644 --- a/jetty-start/src/test/resources/usecases/agent-properties.assert.txt +++ b/jetty-start/src/test/resources/usecases/agent-properties.assert.txt @@ -17,7 +17,7 @@ LIB|${jetty.base}/lib/agent-jdk-1.6.jar # The Properties we expect (order is irrelevant) # (this is the property we actually set in jetty.base) PROP|jetty.http.port=9090 -PROP|java.vm.specification.version=1.6 +# PROP|java.vm.specification.version=1.6 # (these are the ones set by default from jetty.home modules) # PROP|jetty.http.idleTimeout=30000 # PROP|jetty.httpConfig.delayDispatchUntilContent=false diff --git a/jetty-start/src/test/resources/usecases/http2.assert.txt b/jetty-start/src/test/resources/usecases/http2.assert.txt index da7e6208cc3..d5baccccbb9 100644 --- a/jetty-start/src/test/resources/usecases/http2.assert.txt +++ b/jetty-start/src/test/resources/usecases/http2.assert.txt @@ -25,7 +25,7 @@ LIB|${jetty.home}/lib/http2/http2-server-9.3.jar # The Properties we expect (order is irrelevant) # (this is the property we actually set in jetty.base) PROP|jetty.http.port=9090 -PROP|java.version=1.8.0_31 +#PROP|java.version=1.8.0_31 PROP|jetty.sslContext.keyStorePath=etc/keystore PROP|jetty.sslContext.keyStorePassword=friendly PROP|jetty.sslContext.keyManagerPassword=icecream