feat: allow specifying a negative offset to ReadChannel (#1916)

GCS Supports negative offsets in ranged reads and is documented in https://cloud.google.com/storage/docs/json_api/v1/parameters#range Update the ReadChannel returned from Storage.reader to allow providing a negative value to `seek`. NOTE: `seek` to a negative offset will be ignored by GCS if `limit` is also specified
googleapis · Mar 1, 2023 · 6df5469 · 6df5469
1 parent b4cdb37
commit 6df5469
Show file tree

Hide file tree

Showing 6 changed files with 41 additions and 4 deletions.
diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/BaseStorageReadChannel.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/BaseStorageReadChannel.java
@@ -88,7 +88,9 @@ public final synchronized int read(ByteBuffer dst) throws IOException {
       throw new ClosedChannelException();
     }
     long diff = byteRangeSpec.length();
-    if (diff <= 0) {
+    // the check on beginOffset >= 0 used to be a precondition on seek(long)
+    // move it here to preserve existing behavior while allowing new negative offsets
+    if (diff <= 0 && byteRangeSpec.beginOffset() >= 0) {
       return -1;
     }
     try {

diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/ByteRangeSpec.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/ByteRangeSpec.java
@@ -472,6 +472,8 @@ public ReadObjectRequest.Builder seekReadObjectRequest(ReadObjectRequest.Builder
     protected String fmtAsHttpRangeHeader() throws ArithmeticException {
       if (beginOffset > 0) {
         return String.format("bytes=%d-", beginOffset);
+      } else if (beginOffset < 0) {
+        return String.format("bytes=%d", beginOffset);
       } else {
         return null;
       }
@@ -510,7 +512,7 @@ long length() throws ArithmeticException {
 
     @Override
     ByteRangeSpec withNewBeginOffset(long beginOffset) {
-      if (beginOffset > 0) {
+      if (beginOffset != 0) {
         return new LeftClosedByteRangeSpec(beginOffset);
       } else {
         return this;

diff --git a/...ud-storage/src/main/java/com/google/cloud/storage/GapicUnbufferedReadableByteChannel.java b/...ud-storage/src/main/java/com/google/cloud/storage/GapicUnbufferedReadableByteChannel.java
@@ -173,7 +173,6 @@ private static final class ReadCursor {
     private final long limit;
 
     private ReadCursor(long beginning, long limit) {
-      checkArgument(0 <= beginning && beginning <= limit, "0 <= %d <= %d", beginning, limit);
       this.limit = limit;
       this.beginning = beginning;
       this.offset = beginning;

diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageReadChannel.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageReadChannel.java
@@ -41,7 +41,6 @@ default ByteRangeSpec getByteRangeSpec() {
   @SuppressWarnings("resource")
   @Override
   default void seek(long position) throws IOException {
-    checkArgument(position >= 0, "position must be >= 0");
     try {
       setByteRangeSpec(getByteRangeSpec().withNewBeginOffset(position));
     } catch (StorageException e) {

diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/ByteRangeSpecTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/ByteRangeSpecTest.java
@@ -45,6 +45,20 @@ public final class ByteRangeSpecTest {
 
   public static final class Behavior {
 
+    @Test
+    public void negativeBeginOffset() throws Exception {
+      ByteRangeSpec rel = ByteRangeSpec.relativeLength(-5L, null);
+      ByteRangeSpec exO = ByteRangeSpec.explicit(-5L, null);
+      ByteRangeSpec exC = ByteRangeSpec.explicitClosed(-5L, null);
+      threeWayEqual(exO, exC, rel);
+    }
+
+    @Test
+    public void negativeBeginOffset_fromNull() {
+      ByteRangeSpec spec = ByteRangeSpec.nullRange().withNewBeginOffset(-5L);
+      assertThat(spec.getHttpRangeHeader()).isEqualTo("bytes=-5");
+    }
+
     @Test
     public void beginNonNullZero_endNonNullNonInfinity() throws Exception {
       ByteRangeSpec rel = ByteRangeSpec.relativeLength(0L, 52L);

diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITBlobReadChannelTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITBlobReadChannelTest.java
@@ -459,6 +459,27 @@ public void readingLastByteReturnsOneByte_seekOnly() throws IOException {
     }
   }
 
+  @Test
+  public void readingLastByteReturnsOneByte_seekOnly_negativeOffset() throws IOException {
+    int length = 10;
+    byte[] bytes = DataGenerator.base64Characters().genBytes(length);
+
+    BlobInfo info1 = BlobInfo.newBuilder(bucket, generator.randomObjectName()).build();
+    Blob gen1 = storage.create(info1, bytes, BlobTargetOption.doesNotExist());
+
+    byte[] expected1 = Arrays.copyOfRange(bytes, 9, 10);
+    String xxdExpected1 = xxd(expected1);
+    try (ReadChannel reader = storage.reader(gen1.getBlobId());
+        ByteArrayOutputStream baos = new ByteArrayOutputStream();
+        WritableByteChannel writer = Channels.newChannel(baos)) {
+      reader.seek(-1);
+      ByteStreams.copy(reader, writer);
+      byte[] bytes1 = baos.toByteArray();
+      String xxd1 = xxd(bytes1);
+      assertThat(xxd1).isEqualTo(xxdExpected1);
+    }
+  }
+
   @Test
   public void readingLastByteReturnsOneByte_seekAndLimit() throws IOException {
     int length = 10;