From 942ab179a4f280a49cc1ccf7b13c815d089924dc Mon Sep 17 00:00:00 2001 From: mck Date: Sat, 15 May 2021 21:37:23 +0200 Subject: [PATCH 1/2] SQUASH add VersionsTest - fixes pattern - handles non-existant build directory - reduces some method visibilities - adds dummy jar files for testing against --- .gitignore | 1 + pom.xml | 22 ----- .../distributed/shared/Versions.java | 25 ++--- .../distributed/shared/VersionsTest.java | 97 +++++++++++++++++++ 4 files changed, 112 insertions(+), 33 deletions(-) create mode 100644 src/test/java/org/apache/cassandra/distributed/shared/VersionsTest.java diff --git a/.gitignore b/.gitignore index 41a9edb..e016fc5 100644 --- a/.gitignore +++ b/.gitignore @@ -79,3 +79,4 @@ doc/source/tools/nodetool # Python virtual environment venv/ +/nbproject/ diff --git a/pom.xml b/pom.xml index 4e206b4..88cb573 100644 --- a/pom.xml +++ b/pom.xml @@ -90,10 +90,6 @@ - - maven-surefire-plugin - 2.22.2 - @@ -122,7 +118,6 @@ org.apache.rat apache-rat-plugin - 0.13 true @@ -151,22 +146,6 @@ - - org.apache.maven.plugins - maven-javadoc-plugin - 3.0.1 - - - attach-javadocs - - jar - - - none - - - - @@ -174,7 +153,6 @@ scm:git:https://gitbox.apache.org/repos/asf/cassandra-in-jvm-dtest-api.git scm:git:https://gitbox.apache.org/repos/asf/cassandra-in-jvm-dtest-api.git https://gitbox.apache.org/repos/asf/cassandra-in-jvm-dtest-api.git - 0.0.6 diff --git a/src/main/java/org/apache/cassandra/distributed/shared/Versions.java b/src/main/java/org/apache/cassandra/distributed/shared/Versions.java index 62a731b..bc47625 100644 --- a/src/main/java/org/apache/cassandra/distributed/shared/Versions.java +++ b/src/main/java/org/apache/cassandra/distributed/shared/Versions.java @@ -35,7 +35,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -public class Versions +public final class Versions { private static final Logger logger = LoggerFactory.getLogger(Versions.class); @@ -153,7 +153,7 @@ public Version(Major major, String version, URL[] classpath) private final Map> versions; - public Versions(Map> versions) + private Versions(Map> versions) { this.versions = versions; } @@ -177,19 +177,22 @@ public static Versions find() final String dtestJarDirectory = System.getProperty(PROPERTY_PREFIX + "test.dtest_jar_path", "build"); final File sourceDirectory = new File(dtestJarDirectory); logger.info("Looking for dtest jars in " + sourceDirectory.getAbsolutePath()); - final Pattern pattern = Pattern.compile("dtest-(?(\\d+)\\.(\\d+)(\\.\\d+)?(\\.\\d+)?)([~\\-]\\w[.\\w]*(?:\\-\\w[.\\w]*)*)?(\\+[.\\w]+)?\\.jar"); + final Pattern pattern = Pattern.compile("dtest-(?(\\d+)\\.(\\d+)(\\.|-alpha|-beta|-rc)([0-9]+)?(\\.\\d+)?)([~\\-]\\w[.\\w]*(?:\\-\\w[.\\w]*)*)?(\\+[.\\w]+)?\\.jar"); final Map> versions = new HashMap<>(); for (Major major : Major.values()) versions.put(major, new ArrayList<>()); - for (File file : sourceDirectory.listFiles()) + if (sourceDirectory.exists()) { - Matcher m = pattern.matcher(file.getName()); - if (!m.matches()) - continue; - String version = m.group(1); - Major major = Major.fromFull(version); - versions.get(major).add(new Version(major, version, new URL[]{ toURL(file) })); + for (File file : sourceDirectory.listFiles()) + { + Matcher m = pattern.matcher(file.getName()); + if (!m.matches()) + continue; + String version = m.group(1); + Major major = Major.fromFull(version); + versions.get(major).add(new Version(major, version, new URL[]{ toURL(file) })); + } } for (Map.Entry> e : versions.entrySet()) @@ -203,7 +206,7 @@ public static Versions find() return new Versions(versions); } - public static URL toURL(File file) + private static URL toURL(File file) { try { diff --git a/src/test/java/org/apache/cassandra/distributed/shared/VersionsTest.java b/src/test/java/org/apache/cassandra/distributed/shared/VersionsTest.java new file mode 100644 index 0000000..8ac7071 --- /dev/null +++ b/src/test/java/org/apache/cassandra/distributed/shared/VersionsTest.java @@ -0,0 +1,97 @@ +/* + * 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 + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.cassandra.distributed.shared; + +import org.junit.jupiter.api.Assertions; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; + +import com.vdurmont.semver4j.Semver; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +public class VersionsTest +{ + public static final String[] VERSIONS = new String[] + { + "2.2.12", + "2.2.2", + "3.0.0", + "3.0.10", + "3.11.10", + "3.11.9", + "4.0-alpha1", + "4.0-beta1", + "4.0-rc1", + "4.0.0", + "4.1.0" + }; + + @BeforeAll + public static void beforeAll() throws IOException + { + Path root = Files.createTempDirectory("versions"); + System.setProperty(Versions.PROPERTY_PREFIX + "test.dtest_jar_path", root.toAbsolutePath().toString()); + + for (String version : VERSIONS) + Files.createFile(Paths.get(root.toAbsolutePath().toString(), "dtest-" + version + ".jar")); + } + + @AfterAll + public static void afterAll() throws IOException + { + System.clearProperty(Versions.PROPERTY_PREFIX + "test.dtest_jar_path"); + } + + @Test + public void testVersions() throws IOException + { + Versions versions = Versions.find(); + for (String version : VERSIONS) + assertThat(versions.get(version)).isNotNull(); + } + + @Test + public void testGet() + { + assertThat(Versions.find().get("2.2.2")).isNotNull(); + } + + @Test + public void testGetLatest() + { + Versions.find().getLatest(Versions.Major.v22); + } + + @Test + public void testFind() + { + Versions versions = Versions.find(); + assertThat(versions).isNotNull(); + + } + + @Test + public void testToURL() + { + assertThat(Versions.getClassPath()).isNotEmpty(); + } + +} From 2057dbe30dc5a8fee8b2efad988c4d56c531140b Mon Sep 17 00:00:00 2001 From: mck Date: Sat, 15 May 2021 23:53:38 +0200 Subject: [PATCH 2/2] Replace version parsing and handling with SemVer4j library patch by Mick Semb Wever; reviewed by XXX for XXX --- pom.xml | 5 + .../distributed/shared/Versions.java | 91 ++++++++----------- 2 files changed, 43 insertions(+), 53 deletions(-) diff --git a/pom.xml b/pom.xml index 88cb573..bfc2d8c 100644 --- a/pom.xml +++ b/pom.xml @@ -50,6 +50,11 @@ slf4j-api 1.7.25 + + com.vdurmont + semver4j + 3.1.0 + org.junit.jupiter diff --git a/src/main/java/org/apache/cassandra/distributed/shared/Versions.java b/src/main/java/org/apache/cassandra/distributed/shared/Versions.java index bc47625..6270aff 100644 --- a/src/main/java/org/apache/cassandra/distributed/shared/Versions.java +++ b/src/main/java/org/apache/cassandra/distributed/shared/Versions.java @@ -24,7 +24,6 @@ import java.nio.file.Paths; import java.util.ArrayList; import java.util.Collections; -import java.util.Comparator; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -32,6 +31,8 @@ import java.util.regex.Pattern; import java.util.stream.Collectors; +import com.vdurmont.semver4j.Semver; +import com.vdurmont.semver4j.Semver.SemverType; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -73,82 +74,61 @@ public enum Major v4X("4\\.([1-9][0-9]*)(\\.([0-9]+))?"); final Pattern pattern; - Major(String verify) + private Major(String verify) { this.pattern = Pattern.compile(verify); } - static Major fromFull(String version) + static Major of(Semver version) { - if (version.isEmpty()) - throw new IllegalArgumentException(version); - switch (version.charAt(0)) + switch (version.getMajor()) { - case '2': - if (version.startsWith("2.2")) + case 2: + if (2 == version.getMinor()) return v22; - throw new IllegalArgumentException(version); - case '3': - if (version.startsWith("3.0")) + throw new IllegalArgumentException(version.getOriginalValue()); + case 3: + if (0 == version.getMinor()) return v30; return v3X; - case '4': - if (version.startsWith("4.0")) + case 4: + if (0 == version.getMinor()) return v40; return v4X; default: - throw new IllegalArgumentException(version); + throw new IllegalArgumentException(version.getOriginalValue()); } } // verify that the version string is valid for this major version - boolean verify(String version) + boolean verify(Semver version) { - return pattern.matcher(version).matches(); - } - - // sort two strings of the same major version as this enum instance - int compare(String a, String b) - { - Matcher ma = pattern.matcher(a); - Matcher mb = pattern.matcher(b); - if (!ma.matches()) throw new IllegalArgumentException(a); - if (!mb.matches()) throw new IllegalArgumentException(b); - int result = Integer.compare(Integer.parseInt(ma.group(1)), Integer.parseInt(mb.group(1))); - if (result == 0 && this == v3X && (ma.group(3) != null || mb.group(3) != null)) - { - if (ma.group(3) != null && mb.group(3) != null) - { - // XXX likely wrong for alpha|beta|rc versions - result = Integer.compare(Integer.parseInt(ma.group(3)), Integer.parseInt(mb.group(3))); - } - else - { - result = ma.group(3) != null ? 1 : -1; - } - } - // sort descending - return -result; + return pattern.matcher(version.getOriginalValue()).matches(); } } - public static class Version + public static final class Version implements Comparable { public final Major major; - public final String version; + public final Semver version; public final URL[] classpath; public Version(String version, URL[] classpath) { - this(Major.fromFull(version), version, classpath); + this(new Semver(version, SemverType.LOOSE), classpath); } - public Version(Major major, String version, URL[] classpath) + public Version(Semver version, URL[] classpath) { - this.major = major; + this.major = Major.of(version); this.version = version; this.classpath = classpath; } + + @Override + public int compareTo(Version o) { + return version.compareTo(o.version); + } } private final Map> versions; @@ -160,11 +140,16 @@ private Versions(Map> versions) public Version get(String full) { - return versions.get(Major.fromFull(full)) + return get(new Semver(full, SemverType.LOOSE)); + } + + public Version get(Semver version) + { + return versions.get(Major.of(version)) .stream() - .filter(v -> full.equals(v.version)) + .filter(v -> version.equals(v.version)) .findFirst() - .orElseThrow(() -> new RuntimeException("No version " + full + " found")); + .orElseThrow(() -> new RuntimeException("No version " + version.getOriginalValue() + " found")); } public Version getLatest(Major major) @@ -189,9 +174,9 @@ public static Versions find() Matcher m = pattern.matcher(file.getName()); if (!m.matches()) continue; - String version = m.group(1); - Major major = Major.fromFull(version); - versions.get(major).add(new Version(major, version, new URL[]{ toURL(file) })); + Semver sem = new Semver(m.group(1), SemverType.LOOSE); + Major major = Major.of(sem); + versions.get(major).add(new Version(sem, new URL[]{ toURL(file) })); } } @@ -199,8 +184,8 @@ public static Versions find() { if (e.getValue().isEmpty()) continue; - Collections.sort(e.getValue(), Comparator.comparing(v -> v.version, e.getKey()::compare)); - System.out.println("Found " + e.getValue().stream().map(v -> v.version).collect(Collectors.joining(", "))); + Collections.sort(e.getValue(), Collections.reverseOrder()); + System.out.println("Found " + e.getValue().stream().map(v -> v.version.getOriginalValue()).collect(Collectors.joining(", "))); } return new Versions(versions);