This is an automated email from the ASF dual-hosted git repository.
brandonwilliams pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/cassandra.git
The following commit(s) were added to refs/heads/trunk by this push:
new 57a2a86 Fix version parsing logic when upgrading from 3.0
57a2a86 is described below
commit 57a2a8613d2595b8650c24ef1cf3bb0055202409
Author: David Capwell <[email protected]>
AuthorDate: Thu Jul 23 10:31:33 2020 -0700
Fix version parsing logic when upgrading from 3.0
Patch by David Capwell, reviewed by Jon Meredith and brandonwilliams for
CASSANDRA-15973
---
CHANGES.txt | 1 +
.../apache/cassandra/utils/CassandraVersion.java | 111 +++++++-------
.../cassandra/utils/CassandraVersionTest.java | 164 +++++++++++++++------
.../org/apache/cassandra/utils/Generators.java | 61 ++++++++
4 files changed, 241 insertions(+), 96 deletions(-)
diff --git a/CHANGES.txt b/CHANGES.txt
index 0ab4548..a36f379 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
4.0-beta2
+ * Fix version parsing logic when upgrading from 3.0 (CASSANDRA-15973)
* Optimize NoSpamLogger use in hot paths (CASSANDRA-15766)
* Verify sstable components on startup (CASSANDRA-15945)
Merged from 3.11:
diff --git a/src/java/org/apache/cassandra/utils/CassandraVersion.java
b/src/java/org/apache/cassandra/utils/CassandraVersion.java
index aed0fe7..b3dca96 100644
--- a/src/java/org/apache/cassandra/utils/CassandraVersion.java
+++ b/src/java/org/apache/cassandra/utils/CassandraVersion.java
@@ -18,10 +18,13 @@
package org.apache.cassandra.utils;
import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import java.util.Objects;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
-import com.google.common.base.Objects;
+import com.google.common.annotations.VisibleForTesting;
import org.apache.commons.lang3.StringUtils;
/**
@@ -33,22 +36,35 @@ import org.apache.commons.lang3.StringUtils;
public class CassandraVersion implements Comparable<CassandraVersion>
{
/**
- * note: 3rd group matches to words but only allows number and checked
after regexp test.
+ * note: 3rd/4th groups matches to words but only allows number and
checked after regexp test.
* this is because 3rd and the last can be identical.
**/
- private static final String VERSION_REGEXP =
"(\\d+)\\.(\\d+)(?:\\.(\\w+))?(\\-[.\\w]+)?([.+][.\\w]+)?";
- private static final Pattern PATTERN_WHITESPACE = Pattern.compile("\\w+");
+ private static final String VERSION_REGEXP =
"(\\d+)\\.(\\d+)(?:\\.(\\w+))?(?:\\.(\\w+))?(\\-[-.\\w]+)?([.+][.\\w]+)?";
+ private static final Pattern PATTERN_WORDS = Pattern.compile("\\w+");
+ @VisibleForTesting
+ static final int NO_HOTFIX = -1;
- private static final Pattern pattern = Pattern.compile(VERSION_REGEXP);
- private static final Pattern SNAPSHOT = Pattern.compile("-SNAPSHOT");
+ private static final Pattern PATTERN = Pattern.compile(VERSION_REGEXP);
public final int major;
public final int minor;
public final int patch;
+ public final int hotfix;
private final String[] preRelease;
private final String[] build;
+ @VisibleForTesting
+ CassandraVersion(int major, int minor, int patch, int hotfix, String[]
preRelease, String[] build)
+ {
+ this.major = major;
+ this.minor = minor;
+ this.patch = patch;
+ this.hotfix = hotfix;
+ this.preRelease = preRelease;
+ this.build = build;
+ }
+
/**
* Parse a version from a string.
*
@@ -58,8 +74,7 @@ public class CassandraVersion implements
Comparable<CassandraVersion>
*/
public CassandraVersion(String version)
{
- String stripped = SNAPSHOT.matcher(version).replaceFirst("");
- Matcher matcher = pattern.matcher(stripped);
+ Matcher matcher = PATTERN.matcher(version);
if (!matcher.matches())
throw new IllegalArgumentException("Invalid version value: " +
version);
@@ -68,12 +83,13 @@ public class CassandraVersion implements
Comparable<CassandraVersion>
this.major = Integer.parseInt(matcher.group(1));
this.minor = Integer.parseInt(matcher.group(2));
this.patch = matcher.group(3) != null ?
Integer.parseInt(matcher.group(3)) : 0;
+ this.hotfix = matcher.group(4) != null ?
Integer.parseInt(matcher.group(4)) : NO_HOTFIX;
- String pr = matcher.group(4);
- String bld = matcher.group(5);
+ String pr = matcher.group(5);
+ String bld = matcher.group(6);
- this.preRelease = pr == null || pr.isEmpty() ? null :
parseIdentifiers(stripped, pr);
- this.build = bld == null || bld.isEmpty() ? null :
parseIdentifiers(stripped, bld);
+ this.preRelease = pr == null || pr.isEmpty() ? null :
parseIdentifiers(version, pr);
+ this.build = bld == null || bld.isEmpty() ? null :
parseIdentifiers(version, bld);
}
catch (NumberFormatException e)
{
@@ -85,15 +101,25 @@ public class CassandraVersion implements
Comparable<CassandraVersion>
{
// Drop initial - or +
str = str.substring(1);
- String[] parts = StringUtils.split(str, '.');
+ String[] parts = StringUtils.split(str, ".-");
for (String part : parts)
{
- if (!PATTERN_WHITESPACE.matcher(part).matches())
- throw new IllegalArgumentException("Invalid version value: " +
version);
+ if (!PATTERN_WORDS.matcher(part).matches())
+ throw new IllegalArgumentException("Invalid version value: " +
version + "; " + part + " not a valid identifier");
}
return parts;
}
+ public List<String> getPreRelease()
+ {
+ return preRelease != null ? Arrays.asList(preRelease) :
Collections.emptyList();
+ }
+
+ public List<String> getBuild()
+ {
+ return build != null ? Arrays.asList(build) : Collections.emptyList();
+ }
+
public int compareTo(CassandraVersion other)
{
if (major < other.major)
@@ -111,38 +137,15 @@ public class CassandraVersion implements
Comparable<CassandraVersion>
if (patch > other.patch)
return 1;
- int c = compareIdentifiers(preRelease, other.preRelease, 1);
+ int c = Integer.compare(hotfix, other.hotfix);
if (c != 0)
return c;
- return compareIdentifiers(build, other.build, -1);
- }
-
- /**
- * Returns a version that is backward compatible with this version amongst
a list
- * of provided version, or null if none can be found.
- * <p>
- * For instance:
- * "2.0.0".findSupportingVersion("2.0.0", "3.0.0") == "2.0.0"
- * "2.0.0".findSupportingVersion("2.1.3", "3.0.0") == "2.1.3"
- * "2.0.0".findSupportingVersion("3.0.0") == null
- * "2.0.3".findSupportingVersion("2.0.0") == "2.0.0"
- * "2.1.0".findSupportingVersion("2.0.0") == null
- * </p>
- */
- public CassandraVersion findSupportingVersion(CassandraVersion... versions)
- {
- for (CassandraVersion version : versions)
- {
- if (isSupportedBy(version))
- return version;
- }
- return null;
- }
+ c = compareIdentifiers(preRelease, other.preRelease, 1);
+ if (c != 0)
+ return c;
- public boolean isSupportedBy(CassandraVersion version)
- {
- return version != null && major == version.major &&
this.compareTo(version) <= 0;
+ return compareIdentifiers(build, other.build, -1);
}
private static int compareIdentifiers(String[] ids1, String[] ids2, int
defaultPred)
@@ -200,20 +203,24 @@ public class CassandraVersion implements
Comparable<CassandraVersion>
@Override
public boolean equals(Object o)
{
- if (!(o instanceof CassandraVersion))
- return false;
+ if (this == o) return true;
+ if (o == null || getClass() != o.getClass()) return false;
CassandraVersion that = (CassandraVersion) o;
- return major == that.major
- && minor == that.minor
- && patch == that.patch
- && Arrays.equals(preRelease, that.preRelease)
- && Arrays.equals(build, that.build);
+ return major == that.major &&
+ minor == that.minor &&
+ patch == that.patch &&
+ hotfix == that.hotfix &&
+ Arrays.equals(preRelease, that.preRelease) &&
+ Arrays.equals(build, that.build);
}
@Override
public int hashCode()
{
- return Objects.hashCode(major, minor, patch, preRelease, build);
+ int result = Objects.hash(major, minor, patch, hotfix);
+ result = 31 * result + Arrays.hashCode(preRelease);
+ result = 31 * result + Arrays.hashCode(build);
+ return result;
}
@Override
@@ -221,6 +228,8 @@ public class CassandraVersion implements
Comparable<CassandraVersion>
{
StringBuilder sb = new StringBuilder();
sb.append(major).append('.').append(minor).append('.').append(patch);
+ if (hotfix != NO_HOTFIX)
+ sb.append('.').append(hotfix);
if (preRelease != null)
sb.append('-').append(StringUtils.join(preRelease, "."));
if (build != null)
diff --git a/test/unit/org/apache/cassandra/utils/CassandraVersionTest.java
b/test/unit/org/apache/cassandra/utils/CassandraVersionTest.java
index 683d9e4..b83990e 100644
--- a/test/unit/org/apache/cassandra/utils/CassandraVersionTest.java
+++ b/test/unit/org/apache/cassandra/utils/CassandraVersionTest.java
@@ -20,17 +20,115 @@ package org.apache.cassandra.utils;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+
+import com.google.common.base.Splitter;
+import org.apache.commons.lang.CharRange;
+import org.junit.Assert;
+import org.junit.Ignore;
import org.junit.Test;
+import com.datastax.driver.core.VersionNumber;
+import org.apache.cassandra.io.sstable.format.Version;
+import org.assertj.core.api.Assertions;
+import org.quicktheories.core.Gen;
+import org.quicktheories.generators.Generate;
+import org.quicktheories.generators.SourceDSL;
+import org.quicktheories.impl.Constraint;
+
import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.junit.matchers.JUnitMatchers.containsString;
+import static org.quicktheories.QuickTheory.qt;
public class CassandraVersionTest
{
@Test
+ public void toStringParses()
+ {
+ qt().forAll(versionGen()).checkAssert(version -> {
+ Assertions.assertThat(new CassandraVersion(version.toString()))
+ .isEqualTo(version)
+ .hasSameHashCodeAs(version)
+ .isEqualByComparingTo(version);
+
+ });
+ }
+
+ @Test
+ public void clientCanParse()
+ {
+ qt().forAll(versionGen()).checkAssert(version -> {
+
Assertions.assertThat(VersionNumber.parse(version.toString())).isNotNull();
+ });
+ }
+
+ private static Gen<CassandraVersion> versionGen()
+ {
+ Gen<Integer> positive = SourceDSL.integers().allPositive();
+ Gen<Integer> hotfixGen =
positive.mix(Generate.constant(CassandraVersion.NO_HOTFIX));
+ Gen<Integer> smallSizes = SourceDSL.integers().between(0, 5);
+ Gen<String> word =
Generators.regexWord(SourceDSL.integers().between(1, 100)); // empty isn't
allowed while parsing since \w+ is used, so must be at least 1
+ return td -> {
+ int major = positive.generate(td);
+ int minor = positive.generate(td);
+ int patch = positive.generate(td);
+
+ int hotfix = hotfixGen.generate(td);
+
+ int numPreRelease = smallSizes.generate(td);
+ String[] preRelease = numPreRelease == 0 ? null : new
String[numPreRelease];
+ for (int i = 0; i < numPreRelease; i++)
+ preRelease[i] = word.generate(td);
+
+ int numBuild = smallSizes.generate(td);
+ String[] build = numBuild == 0 ? null : new String[numBuild];
+ for (int i = 0; i < numBuild; i++)
+ build[i] = word.generate(td);
+ return new CassandraVersion(major, minor, patch, hotfix,
preRelease, build);
+ };
+ }
+
+ @Test
+ public void multiplePreRelease()
+ {
+ for (String version : Arrays.asList("4.0-alpha1-SNAPSHOT",
+ "4.0.1-alpha1-SNAPSHOT",
+ "4.0.1.1-alpha1-SNAPSHOT",
+ "4.0.0.0-a-b-c-d-e-f-g"))
+ {
+ CassandraVersion cassandra = new CassandraVersion(version);
+ VersionNumber client = VersionNumber.parse(version);
+ Assert.assertEquals(cassandra.major, client.getMajor());
+ Assert.assertEquals(cassandra.minor, client.getMinor());
+ Assert.assertEquals(cassandra.patch, client.getPatch());
+ Assert.assertEquals(cassandra.hotfix, client.getDSEPatch());
+ Assert.assertEquals(cassandra.getPreRelease(),
client.getPreReleaseLabels());
+ }
+ }
+
+ @Test
+ public void multipleBuild()
+ {
+ for (String version : Arrays.asList("4.0+alpha1.SNAPSHOT",
+ "4.0.1+alpha1.SNAPSHOT",
+ "4.0.1.1+alpha1.SNAPSHOT",
+ "4.0.0.0+a.b.c.d.e.f.g"))
+ {
+ CassandraVersion cassandra = new CassandraVersion(version);
+ VersionNumber client = VersionNumber.parse(version);
+ Assert.assertEquals(cassandra.major, client.getMajor());
+ Assert.assertEquals(cassandra.minor, client.getMinor());
+ Assert.assertEquals(cassandra.patch, client.getPatch());
+ Assert.assertEquals(cassandra.hotfix, client.getDSEPatch());
+ Assert.assertEquals(cassandra.getBuild(),
Splitter.on(".").splitToList(client.getBuildLabel()));
+ }
+ }
+
+ @Test
public void testParsing()
{
CassandraVersion version;
@@ -86,51 +184,6 @@ public class CassandraVersionTest
}
@Test
- public void testIsSupportedBy()
- {
- CassandraVersion v1, v2;
-
- v1 = new CassandraVersion("3.0.2");
- assertTrue(v1.isSupportedBy(v1));
-
- v1 = new CassandraVersion("1.2.3");
- v2 = new CassandraVersion("1.2.4");
- assertTrue(v1.isSupportedBy(v2));
- assertTrue(!v2.isSupportedBy(v1));
-
- v1 = new CassandraVersion("1.2.3");
- v2 = new CassandraVersion("1.3.3");
- assertTrue(v1.isSupportedBy(v2));
- assertTrue(!v2.isSupportedBy(v1));
-
- v1 = new CassandraVersion("2.2.3");
- v2 = new CassandraVersion("1.3.3");
- assertTrue(!v1.isSupportedBy(v2));
- assertTrue(!v2.isSupportedBy(v1));
-
- v1 = new CassandraVersion("3.1.0");
- v2 = new CassandraVersion("3.0.1");
- assertTrue(!v1.isSupportedBy(v2));
- assertTrue(v2.isSupportedBy(v1));
-
- v1 = new CassandraVersion("3.7");
- v2 = new CassandraVersion("3.8");
- assertTrue(v1.isSupportedBy(v2));
- assertTrue(!v2.isSupportedBy(v1));
-
- v1 = new CassandraVersion("3.0.8");
- v2 = new CassandraVersion("3.8");
- assertTrue(v1.isSupportedBy(v2));
- assertTrue(!v2.isSupportedBy(v1));
- assertTrue(v2.isSupportedBy(v2));
-
- v1 = new CassandraVersion("3.8");
- v2 = new CassandraVersion("3.8-SNAPSHOT");
- assertTrue(v1.isSupportedBy(v2));
- assertTrue(v2.isSupportedBy(v1));
- }
-
- @Test
public void testInvalid()
{
assertThrows("1.0.0a");
@@ -208,6 +261,27 @@ public class CassandraVersionTest
assertThat(e.getMessage(), containsString(version));
}
}
+
+ @Test
+ public void testExtraOrdering()
+ {
+ List<CassandraVersion> versions = Arrays.asList(version("4.0.0"),
+
version("4.0.0-SNAPSHOT"),
+ version("4.0.0.0"),
+
version("4.0.0.0-SNAPSHOT"));
+ List<CassandraVersion> expected =
Arrays.asList(version("4.0.0-SNAPSHOT"),
+ version("4.0.0"),
+
version("4.0.0.0-SNAPSHOT"),
+ version("4.0.0.0"));
+ Collections.sort(versions);
+ Assertions.assertThat(versions).isEqualTo(expected);
+ }
+
+ private static CassandraVersion version(String str)
+ {
+ return new CassandraVersion(str);
+ }
+
private static String[] parseIdentifiers(String version, String str)
throws Throwable
{
String name = "parseIdentifiers";
diff --git a/test/unit/org/apache/cassandra/utils/Generators.java
b/test/unit/org/apache/cassandra/utils/Generators.java
new file mode 100644
index 0000000..18df830
--- /dev/null
+++ b/test/unit/org/apache/cassandra/utils/Generators.java
@@ -0,0 +1,61 @@
+package org.apache.cassandra.utils;
+
+import org.quicktheories.core.Gen;
+import org.quicktheories.impl.Constraint;
+
+public final class Generators
+{
+ private static final char[] REGEX_WORD_DOMAIN = createRegexWordDomain();
+
+ private Generators()
+ {
+
+ }
+
+ public static Gen<String> regexWord(Gen<Integer> sizes)
+ {
+ return string(sizes, REGEX_WORD_DOMAIN);
+ }
+
+ public static Gen<String> string(Gen<Integer> sizes, char[] domain)
+ {
+ // note, map is overloaded so String::new is ambugious to javac, so
need a lambda here
+ return charArray(sizes, domain).map(c -> new String(c));
+ }
+
+ public static Gen<char[]> charArray(Gen<Integer> sizes, char[] domain)
+ {
+ Constraint constraints = Constraint.between(0, domain.length -
1).withNoShrinkPoint();
+ Gen<char[]> gen = td -> {
+ int size = sizes.generate(td);
+ char[] is = new char[size];
+ for (int i = 0; i != size; i++) {
+ int idx = (int) td.next(constraints);
+ is[i] = domain[idx];
+ }
+ return is;
+ };
+ gen.describedAs(String::new);
+ return gen;
+ }
+
+ private static char[] createRegexWordDomain()
+ {
+ // \w == [a-zA-Z_0-9]
+ char[] domain = new char[26 * 2 + 10 + 1];
+
+ int offset = 0;
+ // _
+ domain[offset++] = (char) 95;
+ // 0-9
+ for (int c = 48; c < 58; c++)
+ domain[offset++] = (char) c;
+ // A-Z
+ for (int c = 65; c < 91; c++)
+ domain[offset++] = (char) c;
+ // a-z
+ for (int c = 97; c < 123; c++)
+ domain[offset++] = (char) c;
+ return domain;
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
