This is an automated email from the ASF dual-hosted git repository.
ycai pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/cassandra-sidecar.git
The following commit(s) were added to refs/heads/trunk by this push:
new c5669b5 Refactor range and add toString
c5669b5 is described below
commit c5669b5e84e33109c407944563e8c466b5a14717
Author: Yifan Cai <yifan_...@apple.com>
AuthorDate: Fri Aug 5 14:54:32 2022 -0700
Refactor range and add toString
---
CHANGES.txt | 6 +
.../org/apache/cassandra/sidecar/models/Range.java | 186 ++++++++++-----------
.../cassandra/sidecar/utils/FileStreamer.java | 2 +-
.../org/apache/cassandra/sidecar/RangeTest.java | 93 +++++++++--
.../routes/StreamSSTableComponentHandlerTest.java | 6 +-
5 files changed, 176 insertions(+), 117 deletions(-)
diff --git a/CHANGES.txt b/CHANGES.txt
index 27ce525..1c92ca5 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,5 +1,11 @@
1.0.0
-----
+ * Refactor range and add toString (CASSANDRASC-41)
+ * Fix search in list snapshot endpoint (CASSANDRASC-40)
+ * Allow Cassandra input validation to be configurable (CASSANDRAC-39)
+ * Add endpoint to list snapshot files (CASSANDRASC-38)
+ * Optimize file path builder and have separate handler for streaming file
(CASSANDRASC-37)
+ * Support for ErrorHandler in Sidecar (CASSANDRASC-36)
* Allow injecting a LoggerHandler to vertxRouter (CASSANDRASC-34)
* Optionally support multiple cassandra instances in Sidecar (CASSANDRASC-33)
* Call the start method of CassandraAdaptorDelegate to start periodic health
check (CASSANDRASC-32)
diff --git a/src/main/java/org/apache/cassandra/sidecar/models/Range.java
b/src/main/java/org/apache/cassandra/sidecar/models/Range.java
index b005064..4f8a006 100644
--- a/src/main/java/org/apache/cassandra/sidecar/models/Range.java
+++ b/src/main/java/org/apache/cassandra/sidecar/models/Range.java
@@ -31,132 +31,130 @@ import
org.apache.cassandra.sidecar.exceptions.RangeException;
* Accepted Range formats are start-end, start-, -suffix_length
* start-end (start = start index of the range, end = end index of the range,
both inclusive)
* start- (start = start index of the range, end = end of file)
- * -suffix-length (Requested length from end of file)
+ * -suffix-length (Requested length from end of file. The length should be
positive)
*/
public class Range
{
- private static final Pattern START_END =
Pattern.compile("^(\\d+)-(\\d+)$");
- private static final Pattern PARTIAL =
Pattern.compile("^((\\d+)-)$|^(-(\\d+))$");
private static final String RANGE_UNIT = "bytes";
+ // matches a. bytes=1-2, b. bytes=1-, c. bytes=-2, d. bytes=-. Need to do
another valid for case d.
+ private static final Pattern RANGE_HEADER = Pattern.compile("^" +
RANGE_UNIT + "=(\\d*)-(\\d*)$");
private final long start;
private final long end;
private final long length;
+ private static final long BOUND_ABSENT = -1L;
- private Range(final long start, final long end)
+ /**
+ * Accepted RangeHeader formats are bytes=start-end, bytes=start-,
bytes=-suffix_length
+ */
+ public static Range parseHeader(final String header, final long fileSize)
{
- this.start = start;
- this.end = end;
- this.length = length(start, end);
+ if (header == null)
+ {
+ return new Range(0, fileSize - 1);
+ }
+ return Range.parse(header, fileSize);
}
- public Range(final long start, final long end, final long length)
+ public static Range of(final long start, final long end)
{
- this.start = start;
- this.end = end;
- this.length = length;
+ return new Range(start, end);
}
- private long length(final long start, final long end)
+ /**
+ * Accepted string formats "bytes=1453-3563", "bytes=-22344", "bytes=5346-"
+ * Sample invalid string formats "bytes=8-3", "bytes=-", "bytes=-0",
"bytes=a-b"
+ *
+ * @param fileSize - passed in to convert partial range into absolute range
+ */
+ private static Range parse(@NotNull String rangeHeader, final long
fileSize)
{
- if (start == 0 && end == Long.MAX_VALUE) // avoid overflow (extra byte)
+ Matcher m = RANGE_HEADER.matcher(rangeHeader);
+ if (!m.matches())
{
- return Long.MAX_VALUE;
+ throw invalidRangeHeaderException(rangeHeader);
}
- return end - start + 1;
- }
- public long start()
- {
- return this.start;
+ long left = parseLong(m.group(1), rangeHeader);
+ long right = parseLong(m.group(2), rangeHeader);
+
+ if (left == BOUND_ABSENT && right == BOUND_ABSENT) // matching
"bytes=-"
+ {
+ throw invalidRangeHeaderException(rangeHeader);
+ }
+ else if (left == BOUND_ABSENT) // matching "bytes=-1"
+ {
+ long len = Math.min(right, fileSize); // correct the range if it
exceeds file size.
+ return new Range(fileSize - len, fileSize - 1);
+ }
+ else if (right == BOUND_ABSENT) // matching "bytes=1-"
+ {
+ return new Range(left, fileSize - 1);
+ }
+ else
+ {
+ return new Range(left, right);
+ }
}
- public long end()
+ // return -1 for empty string; return long value otherwise.
+ // throws IllegalArgumentException for invalid value string
+ private static long parseLong(String valStr, String rangeHeader)
{
- return this.end;
+ if (valStr == null || valStr.isEmpty())
+ return BOUND_ABSENT;
+
+ try
+ {
+ return Long.parseLong(valStr);
+ }
+ catch (NumberFormatException e)
+ {
+ throw invalidRangeHeaderException(rangeHeader);
+ }
}
- public long length()
+ private static IllegalArgumentException invalidRangeHeaderException(String
rangeHeader)
{
- return this.length;
+ return new IllegalArgumentException("Invalid range header: " +
rangeHeader + ". " +
+ "Supported Range formats are
bytes=<start>-<end>, bytes=<start>-, bytes=-<suffix-length>");
}
- public boolean isValidHttpRange()
+ // An initialized range is always valid; invalid params fail range
initialization.
+ private Range(final long start, final long end)
{
- return start >= 0 && end >= start && length > 0;
+ Preconditions.checkArgument(start >= 0, "Range start can not be
negative");
+ Preconditions.checkArgument(end >= start, "Range does not satisfy
boundary requirements");
+ this.start = start;
+ this.end = end;
+ long len = end - start + 1; // Assign long max if overflows
+ this.length = len < 0 ? Long.MAX_VALUE : len;
}
- public Range intersect(@NotNull final Range range)
+ public long start()
{
- if (!(start >= range.start() && start <= range.end()) && !(end >=
range.start() && end <= range.end()) &&
- !(range.start() >= start && range.start() <= end) && !(range.end()
>= start && range.end() <= end))
- {
- throw new RangeException("Range does not overlap");
- }
-
- return new Range(Math.max(start, range.start()), Math.min(end,
range.end()));
+ return this.start;
}
- /**
- * Accepted string formats "start-end", both ends of the range required to
be parsed
- * Sample accepted formats "1-2", "232-2355"
- */
- private static Range parseAbsolute(@NotNull String range)
+ public long end()
{
- Matcher m = START_END.matcher(range);
-
- if (!m.matches())
- {
- throw new IllegalArgumentException("Supported Range formats are
<start>-<end>, <start>-, -<suffix-length>");
- }
-
- final long start = Long.parseLong(m.group(1));
- Preconditions.checkArgument(start >= 0, "Range start can not be
negative");
-
- final long end = Long.parseLong(m.group(2));
- if (end < start)
- {
- throw new RangeException("Range does not satisfy boundary
requirements");
- }
- return new Range(start, end);
+ return this.end;
}
- public static Range parse(@NotNull String range)
+ public long length()
{
- // since file size is not passed, we set it to 0
- return parse(range, 0);
+ return this.length;
}
- /**
- * Accepted string formats "1453-3563", "-22344", "5346-"
- * Sample invalid string formats "8-3", "-", "-0", "a-b"
- *
- * @param fileSize - passed in to convert partial range into absolute range
- */
- public static Range parse(@NotNull String range, final long fileSize)
+ public Range intersect(@NotNull final Range range)
{
- Matcher m = PARTIAL.matcher(range);
- if (!m.matches())
+ long newStart = Math.max(start, range.start());
+ long newEnd = Math.min(end, range.end());
+ if (newStart > newEnd)
{
- return parseAbsolute(range);
- }
-
- Preconditions.checkArgument(fileSize > 0, "Reference file size
invalid");
- if (range.startsWith("-"))
- {
- final long length = Long.parseLong(m.group(4));
- if (length <= 0)
- {
- throw new IllegalArgumentException("Suffix length in " + range
+ " cannot be less than or equal to 0");
- }
- Preconditions.checkArgument(length <= fileSize, "Suffix length
exceeds");
- return new Range(fileSize - length, fileSize - 1, length);
+ throw new RangeException("Range does not overlap");
}
- final long start = Long.parseLong(m.group(2));
- Preconditions.checkArgument(start >= 0, "Range start can not be
negative");
- Preconditions.checkArgument(start < fileSize, "Range exceeds");
-
- return new Range(start, fileSize - 1);
+ return new Range(newStart, newEnd);
}
@Override
@@ -172,8 +170,8 @@ public class Range
}
Range range = (Range) o;
return start == range.start &&
- end == range.end &&
- length == range.length;
+ end == range.end &&
+ length == range.length;
}
@Override
@@ -182,19 +180,9 @@ public class Range
return Objects.hash(start, end, length);
}
- /**
- * Accepted RangeHeader formats are bytes=start-end, bytes=start-,
bytes=-suffix_length
- */
- public static Range parseHeader(final String header, final long fileSize)
+ @Override
+ public String toString()
{
- if (header == null)
- {
- return new Range(0, fileSize - 1, fileSize);
- }
- if (!header.startsWith(RANGE_UNIT + "="))
- {
- throw new UnsupportedOperationException("Unsupported range unit
only bytes are allowed");
- }
- return Range.parse(header.substring(header.indexOf("=") + 1),
fileSize);
+ return String.format("%s=%d-%d", RANGE_UNIT, start, end);
}
}
diff --git a/src/main/java/org/apache/cassandra/sidecar/utils/FileStreamer.java
b/src/main/java/org/apache/cassandra/sidecar/utils/FileStreamer.java
index c327d34..53de99f 100644
--- a/src/main/java/org/apache/cassandra/sidecar/utils/FileStreamer.java
+++ b/src/main/java/org/apache/cassandra/sidecar/utils/FileStreamer.java
@@ -198,7 +198,7 @@ public class FileStreamer
*/
private Future<Range> parseRangeHeader(String rangeHeader, long fileLength)
{
- Range fr = new Range(0, fileLength - 1, fileLength);
+ Range fr = Range.of(0, fileLength - 1);
if (rangeHeader == null)
return Future.succeededFuture(fr);
diff --git a/src/test/java/org/apache/cassandra/sidecar/RangeTest.java
b/src/test/java/org/apache/cassandra/sidecar/RangeTest.java
index e98392d..2d301ff 100644
--- a/src/test/java/org/apache/cassandra/sidecar/RangeTest.java
+++ b/src/test/java/org/apache/cassandra/sidecar/RangeTest.java
@@ -25,7 +25,6 @@ import org.apache.cassandra.sidecar.models.Range;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
-import static org.junit.jupiter.api.Assertions.assertTrue;
/**
* RangeTest
@@ -35,12 +34,23 @@ public class RangeTest
@Test
public void testValidPartialRange()
{
- final String rangeHeaderVal = "bytes=2-";
- final Range range = Range.parseHeader(rangeHeaderVal, 5);
+ String rangeHeaderVal = "bytes=2-";
+ Range range = Range.parseHeader(rangeHeaderVal, 5);
assertEquals(3, range.length());
assertEquals(2, range.start());
assertEquals(4, range.end());
- assertTrue(range.isValidHttpRange());
+
+ rangeHeaderVal = "bytes=-100";
+ range = Range.parseHeader(rangeHeaderVal, 5);
+ assertEquals(5, range.length());
+ assertEquals(0, range.start());
+ assertEquals(4, range.end());
+
+ rangeHeaderVal = "bytes=-100";
+ range = Range.parseHeader(rangeHeaderVal, 200);
+ assertEquals(100, range.length());
+ assertEquals(100, range.start());
+ assertEquals(199, range.end());
}
@Test
@@ -51,40 +61,39 @@ public class RangeTest
assertEquals(101, range.length());
assertEquals(0, range.start());
assertEquals(100, range.end());
- assertTrue(range.isValidHttpRange());
}
@Test
public void testInvalidRangeFormat()
{
- final String rangeVal = "2344--3432";
+ final String rangeHeader = "bytes=2344--3432";
IllegalArgumentException thrownException =
assertThrows(IllegalArgumentException.class, () ->
{
- Range.parse(rangeVal);
+ Range.parseHeader(rangeHeader, Long.MAX_VALUE);
});
- String msg = "Supported Range formats are <start>-<end>, <start>-,
-<suffix-length>";
+ String msg = "Invalid range header: bytes=2344--3432. Supported Range
formats are bytes=<start>-<end>, bytes=<start>-, bytes=-<suffix-length>";
assertEquals(msg, thrownException.getMessage());
}
@Test
public void testInvalidSuffixLength()
{
- final String rangeVal = "-0";
+ final String rangeHeader = "bytes=-0";
IllegalArgumentException thrownException =
assertThrows(IllegalArgumentException.class, () ->
{
- Range.parse(rangeVal, Long.MAX_VALUE);
+ Range.parseHeader(rangeHeader, Long.MAX_VALUE);
});
- String msg = "Suffix length in -0 cannot be less than or equal to 0";
+ String msg = "Range does not satisfy boundary requirements";
assertEquals(msg, thrownException.getMessage());
}
@Test
public void testInvalidRangeBoundary()
{
- final String rangeVal = "9-2";
- RangeException thrownException = assertThrows(RangeException.class, ()
->
+ final String rangeHeader = "bytes=9-2";
+ IllegalArgumentException thrownException =
assertThrows(IllegalArgumentException.class, () ->
{
- Range.parse(rangeVal);
+ Range.parseHeader(rangeHeader, Long.MAX_VALUE);
});
String msg = "Range does not satisfy boundary requirements";
assertEquals(msg, thrownException.getMessage());
@@ -94,11 +103,63 @@ public class RangeTest
public void testWrongRangeUnitUsed()
{
final String rangeVal = "bits=0-";
- UnsupportedOperationException thrownException =
assertThrows(UnsupportedOperationException.class, () ->
+ IllegalArgumentException thrownException =
assertThrows(IllegalArgumentException.class, () ->
{
Range.parseHeader(rangeVal, 5);
});
- String msg = "Unsupported range unit only bytes are allowed";
+ String msg = "Invalid range header: bits=0-. Supported Range formats
are bytes=<start>-<end>, bytes=<start>-, bytes=-<suffix-length>";
+ assertEquals(msg, thrownException.getMessage());
+ }
+
+ @Test
+ public void testToString()
+ {
+ final String rangeHeaderVal = "bytes=0-100";
+ final Range range = Range.parseHeader(rangeHeaderVal, Long.MAX_VALUE);
+ assertEquals("bytes=0-100", range.toString());
+ }
+
+ @Test
+ public void testInvalidRangeBoundValueInHeader()
+ {
+ // the right end of range is larger than long
+ final String rangeHeader = "bytes=0-1" + Long.MAX_VALUE;
+ IllegalArgumentException thrownException =
assertThrows(IllegalArgumentException.class, () ->
+ {
+ Range.parseHeader(rangeHeader, Long.MAX_VALUE);
+ });
+ String msg = "Invalid range header: bytes=0-19223372036854775807.
Supported Range formats are bytes=<start>-<end>, bytes=<start>-,
bytes=-<suffix-length>";
assertEquals(msg, thrownException.getMessage());
}
+
+ @Test
+ public void testIntersect() {
+ Range range1, range2, expected;
+ range1 = Range.of(5, 10);
+ range2 = Range.of(9, 15);
+ expected = Range.of(9, 10);
+ assertEquals(expected, range1.intersect(range2));
+ assertEquals(expected, range2.intersect(range1));
+
+ range1 = Range.of(1, 5);
+ range2 = Range.of(5, 15);
+ expected = Range.of(5, 5);
+ assertEquals(expected, range1.intersect(range2));
+ assertEquals(expected, range2.intersect(range1));
+
+ range1 = Range.of(1, 15);
+ range2 = Range.of(5, 10);
+ expected = Range.of(5, 10);
+ assertEquals(expected, range1.intersect(range2));
+ assertEquals(expected, range2.intersect(range1));
+
+ }
+
+ @Test
+ public void testRangesDoNotIntersect() {
+ Range range1 = Range.of(1, 5);
+ Range range2 = Range.of(9, 15);
+
+ assertThrows(RangeException.class, () -> range1.intersect(range2));
+ }
}
diff --git
a/src/test/java/org/apache/cassandra/sidecar/routes/StreamSSTableComponentHandlerTest.java
b/src/test/java/org/apache/cassandra/sidecar/routes/StreamSSTableComponentHandlerTest.java
index 91916ed..2e788c6 100644
---
a/src/test/java/org/apache/cassandra/sidecar/routes/StreamSSTableComponentHandlerTest.java
+++
b/src/test/java/org/apache/cassandra/sidecar/routes/StreamSSTableComponentHandlerTest.java
@@ -31,6 +31,7 @@ import org.slf4j.LoggerFactory;
import com.google.inject.Guice;
import com.google.inject.Injector;
import com.google.inject.util.Modules;
+import io.netty.handler.codec.http.HttpHeaderNames;
import io.vertx.core.Vertx;
import io.vertx.core.http.HttpServer;
import io.vertx.ext.web.client.WebClient;
@@ -313,7 +314,10 @@ public class StreamSSTableComponentHandlerTest
.putHeader("Range", "bytes=-5")
.send(context.succeeding(response -> context.verify(() ->
{
-
assertThat(response.statusCode()).isEqualTo(REQUESTED_RANGE_NOT_SATISFIABLE.code());
+ assertThat(response.statusCode()).isEqualTo(OK.code());
+
assertThat(response.getHeader(HttpHeaderNames.CONTENT_LENGTH.toString()))
+ .isEqualTo("4")
+ .describedAs("Server should shrink the range to the
file length");
context.completeNow();
})));
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
