Refactor range and add toString

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)

src/main/java/org/apache/cassandra/sidecar/models/Range.java

@@ -31,132 +31,130 @@
  * 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 boolean equals(Object o)
         }
         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 int hashCode()
         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);
     }
 }

src/main/java/org/apache/cassandra/sidecar/utils/FileStreamer.java

@@ -198,7 +198,7 @@ private boolean checkRetriesExhausted(Instant startTime)
      */
     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);