diff --git a/src/java/org/apache/cassandra/config/DataRateSpec.java b/src/java/org/apache/cassandra/config/DataRateSpec.java index d496cfa671..7440ac64f6 100644 --- a/src/java/org/apache/cassandra/config/DataRateSpec.java +++ b/src/java/org/apache/cassandra/config/DataRateSpec.java @@ -37,28 +37,36 @@ public abstract class DataRateSpec /** * The Regexp used to parse the rate provided as String in cassandra.yaml. */ - private static final Pattern UNITS_PATTERN = Pattern.compile("^(\\d+)(MiB/s|KiB/s|B/s)$"); + private static final Pattern UNITS_PATTERN = Pattern.compile("^(\\d+)(MiB/s|KiB/s|B/s)?$"); private final long quantity; private final DataRateUnit unit; - private DataRateSpec(String value) + private DataRateSpec(String value, DataRateUnit minUnit) { //parse the string field value Matcher matcher = UNITS_PATTERN.matcher(value); + IllegalArgumentException ex = new IllegalArgumentException("Invalid data rate: " + value + " Accepted units: MiB/s, KiB/s, B/s where " + + "case matters and only non-negative values are valid"); if (!matcher.find()) - throw new IllegalArgumentException("Invalid data rate: " + value + " Accepted units: MiB/s, KiB/s, B/s where " + - "case matters and " + "only non-negative values are valid"); + throw ex; quantity = Long.parseLong(matcher.group(1)); - unit = DataRateUnit.fromSymbol(matcher.group(2)); + + String symbol = matcher.group(2); + if (symbol != null) + unit = DataRateUnit.fromSymbol(symbol); + else if (quantity == 0L) // accept 0 if it's without a unit + unit = minUnit; + else + throw ex; } private DataRateSpec(String value, DataRateUnit minUnit, long max) { - this(value); + this(value, minUnit); validateQuantity(value, quantity(), unit(), minUnit, max); } diff --git a/src/java/org/apache/cassandra/config/DataStorageSpec.java b/src/java/org/apache/cassandra/config/DataStorageSpec.java index b5d4374088..7d6fefc351 100644 --- a/src/java/org/apache/cassandra/config/DataStorageSpec.java +++ b/src/java/org/apache/cassandra/config/DataStorageSpec.java @@ -39,7 +39,7 @@ public abstract class DataStorageSpec /** * The Regexp used to parse the storage provided as String. */ - private static final Pattern UNITS_PATTERN = Pattern.compile("^(\\d+)(GiB|MiB|KiB|B)$"); + private static final Pattern UNITS_PATTERN = Pattern.compile("^(\\d+)(GiB|MiB|KiB|B)?$"); private final long quantity; @@ -58,18 +58,26 @@ public abstract class DataStorageSpec { //parse the string field value Matcher matcher = UNITS_PATTERN.matcher(value); + IllegalArgumentException ex = new IllegalArgumentException("Invalid data storage: " + value + " Accepted units:" + acceptedUnits(minUnit) + + " where case matters and only non-negative values are accepted"); if (matcher.find()) { quantity = Long.parseLong(matcher.group(1)); - unit = DataStorageUnit.fromSymbol(matcher.group(2)); + + String symbol = matcher.group(2); + if (symbol != null) + unit = DataStorageUnit.fromSymbol(symbol); + else if (quantity == 0L) // accept 0 if it's without a unit + unit = minUnit; + else + throw ex; // this constructor is used only by extended classes for min unit; upper bound and min unit are guarded there accordingly } else { - throw new IllegalArgumentException("Invalid data storage: " + value + " Accepted units:" + acceptedUnits(minUnit) + - " where case matters and only non-negative values are accepted"); + throw ex; } } diff --git a/src/java/org/apache/cassandra/config/DurationSpec.java b/src/java/org/apache/cassandra/config/DurationSpec.java index 2522d86124..bee8df10bb 100644 --- a/src/java/org/apache/cassandra/config/DurationSpec.java +++ b/src/java/org/apache/cassandra/config/DurationSpec.java @@ -44,7 +44,7 @@ public abstract class DurationSpec /** * The Regexp used to parse the duration provided as String. */ - private static final Pattern UNITS_PATTERN = Pattern.compile(("^(\\d+)(d|h|s|ms|us|µs|ns|m)$")); + private static final Pattern UNITS_PATTERN = Pattern.compile(("^(\\d+)(d|h|s|ms|us|µs|ns|m)?$")); private final long quantity; @@ -67,18 +67,26 @@ public abstract class DurationSpec private DurationSpec(String value, TimeUnit minUnit) { Matcher matcher = UNITS_PATTERN.matcher(value); + IllegalArgumentException ex = new IllegalArgumentException("Invalid duration: " + value + " Accepted units:" + acceptedUnits(minUnit) + + " where case matters and only non-negative values."); if (matcher.find()) { quantity = Long.parseLong(matcher.group(1)); - unit = fromSymbol(matcher.group(2)); + + String symbol = matcher.group(2); + if (symbol != null) + unit = fromSymbol(symbol); + else if (quantity == 0L) // accept 0 if it's without a unit + unit = minUnit; + else + throw ex; // this constructor is used only by extended classes for min unit; upper bound and min unit are guarded there accordingly } else { - throw new IllegalArgumentException("Invalid duration: " + value + " Accepted units:" + acceptedUnits(minUnit) + - " where case matters and only non-negative values."); + throw ex; } } diff --git a/test/unit/org/apache/cassandra/config/DataRateSpecTest.java b/test/unit/org/apache/cassandra/config/DataRateSpecTest.java index be0973e0fa..0cbce474dc 100644 --- a/test/unit/org/apache/cassandra/config/DataRateSpecTest.java +++ b/test/unit/org/apache/cassandra/config/DataRateSpecTest.java @@ -31,6 +31,7 @@ public class DataRateSpecTest @Test public void testConversions() { + assertEquals(0, new DataRateSpec.LongBytesPerSecondBound("0").toBytesPerSecond(), 0); assertEquals(10, new DataRateSpec.LongBytesPerSecondBound("10B/s").toBytesPerSecond(), 0); assertEquals(10240, new DataRateSpec.LongBytesPerSecondBound("10KiB/s").toBytesPerSecond(), 0); assertEquals(0, new DataRateSpec.LongBytesPerSecondBound("10KiB/s").toMebibytesPerSecond(), 0.1); @@ -103,6 +104,8 @@ public class DataRateSpecTest @Test public void testInvalidInputs() { + assertThatThrownBy(() -> new DataRateSpec.LongBytesPerSecondBound("-0")).isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Invalid data rate: -0"); assertThatThrownBy(() -> new DataRateSpec.LongBytesPerSecondBound("10")).isInstanceOf(IllegalArgumentException.class) .hasMessageContaining("Invalid data rate: 10"); assertThatThrownBy(() -> new DataRateSpec.LongBytesPerSecondBound("-10b/s")).isInstanceOf(IllegalArgumentException.class) @@ -153,6 +156,7 @@ public class DataRateSpecTest @Test public void testEquals() { + assertEquals(new DataRateSpec.LongBytesPerSecondBound("0"), new DataRateSpec.LongBytesPerSecondBound("0B/s")); assertEquals(new DataRateSpec.LongBytesPerSecondBound("10B/s"), new DataRateSpec.LongBytesPerSecondBound("10B/s")); assertEquals(new DataRateSpec.LongBytesPerSecondBound("10KiB/s"), new DataRateSpec.LongBytesPerSecondBound("10240B/s")); assertEquals(new DataRateSpec.LongBytesPerSecondBound("10240B/s"), new DataRateSpec.LongBytesPerSecondBound("10KiB/s")); diff --git a/test/unit/org/apache/cassandra/config/DataStorageSpecTest.java b/test/unit/org/apache/cassandra/config/DataStorageSpecTest.java index 334e33f259..922228d5b8 100644 --- a/test/unit/org/apache/cassandra/config/DataStorageSpecTest.java +++ b/test/unit/org/apache/cassandra/config/DataStorageSpecTest.java @@ -38,11 +38,13 @@ public class DataStorageSpecTest @Test public void testConversions() { + assertEquals(0, new DataStorageSpec.LongBytesBound("0").toBytes()); assertEquals(10, new DataStorageSpec.LongBytesBound("10B").toBytes()); assertEquals(10240, new DataStorageSpec.LongBytesBound("10KiB").toBytes()); assertEquals(10485760, new DataStorageSpec.LongBytesBound("10MiB").toBytes()); assertEquals(1073741824, new DataStorageSpec.LongBytesBound("1GiB").toBytes()); + assertEquals(0, new DataStorageSpec.LongMebibytesBound("0").toMebibytes()); assertEquals(1024, new DataStorageSpec.LongMebibytesBound("1GiB").toMebibytes()); assertEquals(10485760, new DataStorageSpec.LongMebibytesBound("10MiB").toBytes()); assertEquals(10240, new DataStorageSpec.LongMebibytesBound("10MiB").toKibibytes()); @@ -167,6 +169,8 @@ public class DataStorageSpecTest @Test public void testInvalidInputs() { + assertThatThrownBy(() -> new DataStorageSpec.LongBytesBound("-0")).isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Invalid data storage: -0"); assertThatThrownBy(() -> new DataStorageSpec.LongBytesBound("10")).isInstanceOf(IllegalArgumentException.class) .hasMessageContaining("Invalid data storage: 10"); assertThatThrownBy(() -> new DataStorageSpec.LongBytesBound("-10bps")).isInstanceOf(IllegalArgumentException.class) @@ -221,6 +225,12 @@ public class DataStorageSpecTest @Test public void testEquals() { + assertEquals(new DataStorageSpec.LongBytesBound("0"), new DataStorageSpec.LongBytesBound("0B")); + assertEquals(new DataStorageSpec.IntBytesBound("0"), new DataStorageSpec.IntBytesBound("0B")); + assertEquals(new DataStorageSpec.IntMebibytesBound("0"), new DataStorageSpec.IntMebibytesBound("0MiB")); + assertEquals(new DataStorageSpec.LongMebibytesBound("0"), new DataStorageSpec.LongMebibytesBound("0MiB")); + assertEquals(new DataStorageSpec.IntKibibytesBound("0"), new DataStorageSpec.IntKibibytesBound("0KiB")); + assertEquals(new DataStorageSpec.LongBytesBound("10B"), new DataStorageSpec.LongBytesBound("10B")); assertEquals(new DataStorageSpec.LongBytesBound.LongBytesBound("10KiB"), new DataStorageSpec.LongBytesBound.LongBytesBound("10240B")); diff --git a/test/unit/org/apache/cassandra/config/DurationSpecTest.java b/test/unit/org/apache/cassandra/config/DurationSpecTest.java index b0c73dedf2..78d4f43e45 100644 --- a/test/unit/org/apache/cassandra/config/DurationSpecTest.java +++ b/test/unit/org/apache/cassandra/config/DurationSpecTest.java @@ -40,10 +40,11 @@ public class DurationSpecTest @Test public void testConversions() { - assertEquals(10000000000L, new DurationSpec.LongNanosecondsBound ("10s").toNanoseconds()); + assertEquals(0, new DurationSpec.LongNanosecondsBound("0").toNanoseconds()); + assertEquals(10000000000L, new DurationSpec.LongNanosecondsBound("10s").toNanoseconds()); assertEquals(MAX_INT_CONFIG_VALUE, new DurationSpec.IntSecondsBound(MAX_INT_CONFIG_VALUE + "s").toSeconds()); assertEquals(MAX_INT_CONFIG_VALUE, new DurationSpec.LongMillisecondsBound(MAX_INT_CONFIG_VALUE + "ms").toMilliseconds()); - assertEquals(600000000000L, new DurationSpec.LongNanosecondsBound ("10m").toNanoseconds()); + assertEquals(600000000000L, new DurationSpec.LongNanosecondsBound("10m").toNanoseconds()); assertEquals(MAX_INT_CONFIG_VALUE, new DurationSpec.IntMinutesBound(MAX_INT_CONFIG_VALUE + "m").toMinutes()); assertEquals(MAX_INT_CONFIG_VALUE, new DurationSpec.IntSecondsBound(MAX_INT_CONFIG_VALUE + "s").toSeconds()); assertEquals(new DurationSpec.IntMillisecondsBound(0.7, TimeUnit.MILLISECONDS), new DurationSpec.LongNanosecondsBound("1ms")); @@ -80,6 +81,8 @@ public class DurationSpecTest @Test public void testInvalidInputs() { + assertThatThrownBy(() -> new DurationSpec.LongNanosecondsBound("-0")).isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Invalid duration: -0"); assertThatThrownBy(() -> new DurationSpec.LongNanosecondsBound("10")).isInstanceOf(IllegalArgumentException.class) .hasMessageContaining("Invalid duration: 10"); assertThatThrownBy(() -> new DurationSpec.LongNanosecondsBound("-10s")).isInstanceOf(IllegalArgumentException.class) @@ -182,15 +185,23 @@ public class DurationSpecTest @Test public void testEquals() { - assertEquals(new DurationSpec.LongNanosecondsBound ("10s"), new DurationSpec.LongNanosecondsBound ("10s")); - assertEquals(new DurationSpec.LongNanosecondsBound ("10s"), new DurationSpec.LongNanosecondsBound ("10000ms")); - assertEquals(new DurationSpec.LongNanosecondsBound ("10000ms"), new DurationSpec.LongNanosecondsBound ("10s")); - assertEquals(new DurationSpec.LongNanosecondsBound ("4h"), new DurationSpec.LongNanosecondsBound ("14400s")); - assertEquals(DurationSpec.LongNanosecondsBound .IntSecondsBound.inSecondsString("14400"), new DurationSpec.LongNanosecondsBound ("14400s")); - assertEquals(DurationSpec.LongNanosecondsBound .IntSecondsBound.inSecondsString("4h"), new DurationSpec.LongNanosecondsBound ("14400s")); - assertEquals(DurationSpec.LongNanosecondsBound .IntSecondsBound.inSecondsString("14400s"), new DurationSpec.LongNanosecondsBound ("14400s")); - assertNotEquals(new DurationSpec.LongNanosecondsBound ("0m"), new DurationSpec.LongNanosecondsBound ("10ms")); - assertEquals(Long.MAX_VALUE-1, new DurationSpec.LongNanosecondsBound ("9223372036854775806ns").toNanoseconds()); + assertEquals(new DurationSpec.IntSecondsBound("0"), new DurationSpec.IntSecondsBound("0s")); + assertEquals(new DurationSpec.LongSecondsBound("0"), new DurationSpec.LongSecondsBound("0s")); + assertEquals(new DurationSpec.IntMillisecondsBound("0"), new DurationSpec.IntMillisecondsBound("0ms")); + assertEquals(new DurationSpec.LongMillisecondsBound("0"), new DurationSpec.LongMillisecondsBound("0ms")); + assertEquals(new DurationSpec.IntMinutesBound("0"), new DurationSpec.IntMinutesBound("0m")); + assertEquals(new DurationSpec.LongMicrosecondsBound("0"), new DurationSpec.LongMicrosecondsBound("0us")); + assertEquals(new DurationSpec.LongNanosecondsBound("0"), new DurationSpec.LongNanosecondsBound("0ns")); + + assertEquals(new DurationSpec.LongNanosecondsBound("10s"), new DurationSpec.LongNanosecondsBound("10s")); + assertEquals(new DurationSpec.LongNanosecondsBound("10s"), new DurationSpec.LongNanosecondsBound("10000ms")); + assertEquals(new DurationSpec.LongNanosecondsBound("10000ms"), new DurationSpec.LongNanosecondsBound("10s")); + assertEquals(new DurationSpec.LongNanosecondsBound("4h"), new DurationSpec.LongNanosecondsBound("14400s")); + assertEquals(DurationSpec.LongNanosecondsBound.IntSecondsBound.inSecondsString("14400"), new DurationSpec.LongNanosecondsBound("14400s")); + assertEquals(DurationSpec.LongNanosecondsBound.IntSecondsBound.inSecondsString("4h"), new DurationSpec.LongNanosecondsBound("14400s")); + assertEquals(DurationSpec.LongNanosecondsBound.IntSecondsBound.inSecondsString("14400s"), new DurationSpec.LongNanosecondsBound("14400s")); + assertNotEquals(new DurationSpec.LongNanosecondsBound("0m"), new DurationSpec.LongNanosecondsBound("10ms")); + assertEquals(Long.MAX_VALUE-1, new DurationSpec.LongNanosecondsBound("9223372036854775806ns").toNanoseconds()); } @Test