Repository: googleapis/java-storage
Pull request #1834 (2023-01-09 19:42:04)

feat: update GrpcBlobReadChannel to allow seek/limit after read
### Refactoring
Extract the majority of BlobReadChannelV2 to a new abstract base class BaseStorageReadChannel which effectively adapts a LazyReadChannel to a StorageReadChannel.

BaseStorageReadChannel now defines an abstract method newLazyReadChannel which each implementation is responsible for implementing to integrate into the lifecycle.




Patch:
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
new file mode 100644
index 00000000..c11c3845
--- /dev/null
+++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/BaseStorageReadChannel.java
@@ -0,0 +1,149 @@
+/*
+ * Copyright 2023 Google LLC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *       http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.google.cloud.storage;
+
+import static com.google.cloud.storage.ByteSizeConstants._2MiB;
+import static java.util.Objects.requireNonNull;
+
+import com.google.cloud.storage.BufferedReadableByteChannelSession.BufferedReadableByteChannel;
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+abstract class BaseStorageReadChannel<T> implements StorageReadChannel {
+
+  private ByteRangeSpec byteRangeSpec;
+  private int chunkSize = _2MiB;
+  private BufferHandle bufferHandle;
+  private LazyReadChannel<T> lazyReadChannel;
+
+  @Nullable private T resolvedObject;
+
+  protected BaseStorageReadChannel() {
+    this.byteRangeSpec = ByteRangeSpec.nullRange();
+  }
+
+  @Override
+  public final synchronized void setChunkSize(int chunkSize) {
+    StorageException.wrapIOException(() -> maybeResetChannel(true));
+    this.chunkSize = chunkSize;
+  }
+
+  @Override
+  public final synchronized boolean isOpen() {
+    if (lazyReadChannel == null) {
+      return true;
+    } else {
+      LazyReadChannel<T> tmp = internalGetLazyChannel();
+      return tmp.isOpen();
+    }
+  }
+
+  @Override
+  public final synchronized void close() {
+    if (internalGetLazyChannel().isOpen()) {
+      StorageException.wrapIOException(internalGetLazyChannel().getChannel()::close);
+    }
+  }
+
+  @Override
+  public final synchronized StorageReadChannel setByteRangeSpec(ByteRangeSpec byteRangeSpec) {
+    requireNonNull(byteRangeSpec, "byteRangeSpec must be non null");
+    StorageException.wrapIOException(() -> maybeResetChannel(false));
+    this.byteRangeSpec = byteRangeSpec;
+    return this;
+  }
+
+  @Override
+  public final ByteRangeSpec getByteRangeSpec() {
+    return byteRangeSpec;
+  }
+
+  @Override
+  public final synchronized int read(ByteBuffer dst) throws IOException {
+    long diff = byteRangeSpec.length();
+    if (diff <= 0) {
+      close();
+      return -1;
+    }
+    try {
+      int read = internalGetLazyChannel().getChannel().read(dst);
+      if (read != -1) {
+        byteRangeSpec = byteRangeSpec.withShiftBeginOffset(read);
+      } else {
+        close();
+      }
+      return read;
+    } catch (StorageException e) {
+      if (e.getCode() == 416) {
+        // HttpStorageRpc turns 416 into a null etag with an empty byte array, leading
+        // BlobReadChannel to believe it read 0 bytes, returning -1 and leaving the channel open.
+        // Emulate that same behavior here to preserve behavior compatibility, though this should
+        // be removed in the next major version.
+        return -1;
+      } else {
+        throw new IOException(e);
+      }
+    } catch (IOException e) {
+      throw e;
+    } catch (Exception e) {
+      throw new IOException(StorageException.coalesce(e));
+    }
+  }
+
+  protected final BufferHandle getBufferHandle() {
+    if (bufferHandle == null) {
+      bufferHandle = BufferHandle.allocate(chunkSize);
+    }
+    return bufferHandle;
+  }
+
+  protected final int getChunkSize() {
+    return chunkSize;
+  }
+
+  @Nullable
+  protected T getResolvedObject() {
+    return resolvedObject;
+  }
+
+  protected void setResolvedObject(@Nullable T resolvedObject) {
+    this.resolvedObject = resolvedObject;
+  }
+
+  protected abstract LazyReadChannel<T> newLazyReadChannel();
+
+  private void maybeResetChannel(boolean freeBuffer) throws IOException {
+    if (lazyReadChannel != null && lazyReadChannel.isOpen()) {
+      try (BufferedReadableByteChannel ignore = lazyReadChannel.getChannel()) {
+        if (bufferHandle != null && !freeBuffer) {
+          bufferHandle.get().clear();
+        } else if (freeBuffer) {
+          bufferHandle = null;
+        }
+        lazyReadChannel = null;
+      }
+    }
+  }
+
+  private LazyReadChannel<T> internalGetLazyChannel() {
+    if (lazyReadChannel == null) {
+      lazyReadChannel = newLazyReadChannel();
+    }
+    return lazyReadChannel;
+  }
+}
diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/BlobReadChannelV2.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/BlobReadChannelV2.java
index e768b04e..05b3513a 100644
--- a/google-cloud-storage/src/main/java/com/google/cloud/storage/BlobReadChannelV2.java
+++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/BlobReadChannelV2.java
@@ -16,36 +16,23 @@
 
 package com.google.cloud.storage;
 
-import static com.google.cloud.storage.ByteSizeConstants._2MiB;
-import static java.util.Objects.requireNonNull;
-
 import com.google.api.services.storage.Storage;
 import com.google.api.services.storage.model.StorageObject;
 import com.google.cloud.ReadChannel;
 import com.google.cloud.RestorableState;
 import com.google.cloud.storage.ApiaryUnbufferedReadableByteChannel.ApiaryReadRequest;
-import com.google.cloud.storage.BufferedReadableByteChannelSession.BufferedReadableByteChannel;
 import com.google.cloud.storage.spi.v1.StorageRpc;
 import com.google.common.base.MoreObjects;
-import java.io.IOException;
 import java.io.Serializable;
-import java.nio.ByteBuffer;
 import java.util.Map;
 import java.util.Objects;
 
-final class BlobReadChannelV2 implements StorageReadChannel {
+final class BlobReadChannelV2 extends BaseStorageReadChannel<StorageObject> {
 
   private final StorageObject storageObject;
   private final Map<StorageRpc.Option, ?> opts;
   private final BlobReadChannelContext blobReadChannelContext;
 
-  private LazyReadChannel<StorageObject> lazyReadChannel;
-  private StorageObject resolvedObject;
-  private ByteRangeSpec byteRangeSpec;
-
-  private int chunkSize = _2MiB;
-  private BufferHandle bufferHandle;
-
   BlobReadChannelV2(
       StorageObject storageObject,
       Map<StorageRpc.Option, ?> opts,
@@ -53,127 +40,30 @@ final class BlobReadChannelV2 implements StorageReadChannel {
     this.storageObject = storageObject;
     this.opts = opts;
     this.blobReadChannelContext = blobReadChannelContext;
-    this.byteRangeSpec = ByteRangeSpec.nullRange();
-  }
-
-  @Override
-  public synchronized void setChunkSize(int chunkSize) {
-    StorageException.wrapIOException(() -> maybeResetChannel(true));
-    this.chunkSize = chunkSize;
-  }
-
-  @Override
-  public synchronized boolean isOpen() {
-    if (lazyReadChannel == null) {
-      return true;
-    } else {
-      LazyReadChannel<StorageObject> tmp = internalGetLazyChannel();
-      return tmp.isOpen();
-    }
-  }
-
-  @Override
-  public synchronized void close() {
-    if (internalGetLazyChannel().isOpen()) {
-      StorageException.wrapIOException(internalGetLazyChannel().getChannel()::close);
-    }
-  }
-
-  @Override
-  public synchronized StorageReadChannel setByteRangeSpec(ByteRangeSpec byteRangeSpec) {
-    requireNonNull(byteRangeSpec, "byteRangeSpec must be non null");
-    StorageException.wrapIOException(() -> maybeResetChannel(false));
-    this.byteRangeSpec = byteRangeSpec;
-    return this;
-  }
-
-  @Override
-  public ByteRangeSpec getByteRangeSpec() {
-    return byteRangeSpec;
-  }
-
-  @Override
-  public synchronized int read(ByteBuffer dst) throws IOException {
-    long diff = byteRangeSpec.length();
-    if (diff <= 0) {
-      close();
-      return -1;
-    }
-    try {
-      int read = internalGetLazyChannel().getChannel().read(dst);
-      if (read != -1) {
-        byteRangeSpec = byteRangeSpec.withShiftBeginOffset(read);
-      } else {
-        close();
-      }
-      return read;
-    } catch (StorageException e) {
-      if (e.getCode() == 416) {
-        // HttpStorageRpc turns 416 into a null etag with an empty byte array, leading
-        // BlobReadChannel to believe it read 0 bytes, returning -1 and leaving the channel open.
-        // Emulate that same behavior here to preserve behavior compatibility, though this should
-        // be removed in the next major version.
-        return -1;
-      } else {
-        throw new IOException(e);
-      }
-    } catch (IOException e) {
-      throw e;
-    } catch (Exception e) {
-      throw new IOException(StorageException.coalesce(e));
-    }
   }
 
   @Override
   public RestorableState<ReadChannel> capture() {
     ApiaryReadRequest apiaryReadRequest = getApiaryReadRequest();
     return new BlobReadChannelV2State(
-        apiaryReadRequest, blobReadChannelContext.getStorageOptions(), chunkSize);
+        apiaryReadRequest, blobReadChannelContext.getStorageOptions(), getChunkSize());
   }
 
-  private void maybeResetChannel(boolean umallocBuffer) throws IOException {
-    if (lazyReadChannel != null && lazyReadChannel.isOpen()) {
-      try (BufferedReadableByteChannel ignore = lazyReadChannel.getChannel()) {
-        if (bufferHandle != null && !umallocBuffer) {
-          bufferHandle.get().clear();
-        } else if (umallocBuffer) {
-          bufferHandle = null;
-        }
-        lazyReadChannel = null;
-      }
-    }
-  }
-
-  private LazyReadChannel<StorageObject> internalGetLazyChannel() {
-    if (lazyReadChannel == null) {
-      lazyReadChannel = newLazyReadChannel();
-    }
-    return lazyReadChannel;
-  }
-
-  private LazyReadChannel<StorageObject> newLazyReadChannel() {
+  protected LazyReadChannel<StorageObject> newLazyReadChannel() {
     return new LazyReadChannel<>(
-        () -> {
-          if (bufferHandle == null) {
-            bufferHandle = BufferHandle.allocate(chunkSize);
-          }
-          return ResumableMedia.http()
-              .read()
-              .byteChannel(blobReadChannelContext)
-              .setCallback(this::setResolvedObject)
-              .buffered(bufferHandle)
-              .setApiaryReadRequest(getApiaryReadRequest())
-              .build();
-        });
-  }
-
-  private void setResolvedObject(StorageObject resolvedObject) {
-    this.resolvedObject = resolvedObject;
+        () ->
+            ResumableMedia.http()
+                .read()
+                .byteChannel(blobReadChannelContext)
+                .setCallback(this::setResolvedObject)
+                .buffered(getBufferHandle())
+                .setApiaryReadRequest(getApiaryReadRequest())
+                .build());
   }
 
   private ApiaryReadRequest getApiaryReadRequest() {
-    StorageObject object = resolvedObject != null ? resolvedObject : storageObject;
-    return new ApiaryReadRequest(object, opts, byteRangeSpec);
+    StorageObject object = getResolvedObject() != null ? getResolvedObject() : storageObject;
+    return new ApiaryReadRequest(object, opts, getByteRangeSpec());
   }
 
   static class BlobReadChannelV2State implements RestorableState<ReadChannel>, Serializable {
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
index 737e1a82..3b501eee 100644
--- 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
@@ -19,6 +19,7 @@ package com.google.cloud.storage;
 import com.google.api.core.InternalApi;
 import com.google.common.base.MoreObjects;
 import com.google.common.base.MoreObjects.ToStringHelper;
+import com.google.storage.v2.ReadObjectRequest;
 import java.io.Serializable;
 import java.util.Objects;
 import java.util.function.BiFunction;
@@ -80,6 +81,8 @@ abstract class ByteRangeSpec implements Serializable {
     return httpRangeHeader;
   }
 
+  abstract ReadObjectRequest.Builder seekReadObjectRequest(ReadObjectRequest.Builder b);
+
   @Nullable
   protected abstract String fmtAsHttpRangeHeader() throws ArithmeticException;
 
@@ -212,6 +215,11 @@ abstract class ByteRangeSpec implements Serializable {
       }
     }
 
+    @Override
+    public ReadObjectRequest.Builder seekReadObjectRequest(ReadObjectRequest.Builder b) {
+      return b.setReadOffset(beginOffset()).setReadLimit(length());
+    }
+
     @Override
     protected String fmtAsHttpRangeHeader() throws ArithmeticException {
       return String.format("bytes=%d-%d", beginOffset, endOffsetInclusive());
@@ -292,6 +300,11 @@ abstract class ByteRangeSpec implements Serializable {
       return new RelativeByteRangeSpec(beginOffset, relativeLength);
     }
 
+    @Override
+    public ReadObjectRequest.Builder seekReadObjectRequest(ReadObjectRequest.Builder b) {
+      return b.setReadOffset(beginOffset()).setReadLimit(length());
+    }
+
     @Override
     protected String fmtAsHttpRangeHeader() throws ArithmeticException {
       return String.format("bytes=%d-%d", beginOffset, endOffsetInclusive());
@@ -372,6 +385,11 @@ abstract class ByteRangeSpec implements Serializable {
       return new RelativeByteRangeSpec(beginOffset, relativeLength);
     }
 
+    @Override
+    public ReadObjectRequest.Builder seekReadObjectRequest(ReadObjectRequest.Builder b) {
+      return b.setReadOffset(beginOffset()).setReadLimit(length());
+    }
+
     @Override
     protected String fmtAsHttpRangeHeader() throws ArithmeticException {
       return String.format("bytes=%d-%d", beginOffset, endOffsetInclusive);
@@ -445,6 +463,11 @@ abstract class ByteRangeSpec implements Serializable {
       return new RelativeByteRangeSpec(beginOffset, relativeLength);
     }
 
+    @Override
+    public ReadObjectRequest.Builder seekReadObjectRequest(ReadObjectRequest.Builder b) {
+      return b.setReadOffset(beginOffset());
+    }
+
     @Override
     protected String fmtAsHttpRangeHeader() throws ArithmeticException {
       if (beginOffset > 0) {
@@ -530,6 +553,11 @@ abstract class ByteRangeSpec implements Serializable {
       }
     }
 
+    @Override
+    public ReadObjectRequest.Builder seekReadObjectRequest(ReadObjectRequest.Builder b) {
+      return b;
+    }
+
     @Override
     protected String fmtAsHttpRangeHeader() throws ArithmeticException {
       return null;
diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcBlobReadChannel.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcBlobReadChannel.java
index ba9b5b50..cd183118 100644
--- a/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcBlobReadChannel.java
+++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcBlobReadChannel.java
@@ -16,78 +16,27 @@
 
 package com.google.cloud.storage;
 
-import static com.google.cloud.storage.ByteSizeConstants._16MiB;
-import static com.google.cloud.storage.Maths.sub;
-import static com.google.cloud.storage.StorageV2ProtoUtils.seekReadObjectRequest;
-import static com.google.common.base.Preconditions.checkArgument;
-import static com.google.common.base.Preconditions.checkState;
-
-import com.google.api.core.ApiFuture;
 import com.google.api.gax.rpc.ServerStreamingCallable;
 import com.google.cloud.ReadChannel;
 import com.google.cloud.RestorableState;
 import com.google.storage.v2.Object;
 import com.google.storage.v2.ReadObjectRequest;
 import com.google.storage.v2.ReadObjectResponse;
-import java.io.IOException;
-import java.nio.ByteBuffer;
-
-final class GrpcBlobReadChannel implements ReadChannel {
+import org.checkerframework.checker.nullness.qual.NonNull;
 
-  private final LazyReadChannel<Object> lazyReadChannel;
+final class GrpcBlobReadChannel extends BaseStorageReadChannel<Object> {
 
-  private Long position;
-  private Long limit;
-  private int chunkSize = _16MiB;
+  private final ServerStreamingCallable<ReadObjectRequest, ReadObjectResponse> read;
+  private final ReadObjectRequest request;
+  private final boolean autoGzipDecompression;
 
   GrpcBlobReadChannel(
       ServerStreamingCallable<ReadObjectRequest, ReadObjectResponse> read,
       ReadObjectRequest request,
       boolean autoGzipDecompression) {
-    this.lazyReadChannel =
-        new LazyReadChannel<>(
-            () -> {
-              ReadObjectRequest req =
-                  seekReadObjectRequest(request, position, sub(limit, position));
-              return ResumableMedia.gapic()
-                  .read()
-                  .byteChannel(read)
-                  .setHasher(Hasher.noop())
-                  .setAutoGzipDecompression(autoGzipDecompression)
-                  .buffered(BufferHandle.allocate(chunkSize))
-                  .setReadObjectRequest(req)
-                  .build();
-            });
-  }
-
-  @Override
-  public void setChunkSize(int chunkSize) {
-    checkState(!isOpen(), "Unable to change chunkSize after read");
-    this.chunkSize = chunkSize;
-  }
-
-  @Override
-  public boolean isOpen() {
-    return lazyReadChannel.isOpen() && lazyReadChannel.getChannel().isOpen();
-  }
-
-  @Override
-  public void close() {
-    if (lazyReadChannel.isOpen()) {
-      try {
-        lazyReadChannel.getChannel().close();
-      } catch (IOException e) {
-        // why does ReadChannel remove IOException?!
-        throw StorageException.coalesce(e);
-      }
-    }
-  }
-
-  @Override
-  public void seek(long position) throws IOException {
-    checkArgument(position >= 0, "position must be >= 0");
-    checkState(!isOpen(), "Unable to change position after read");
-    this.position = position;
+    this.read = read;
+    this.request = request;
+    this.autoGzipDecompression = autoGzipDecompression;
   }
 
   @Override
@@ -96,35 +45,29 @@ final class GrpcBlobReadChannel implements ReadChannel {
   }
 
   @Override
-  public ReadChannel limit(long limit) {
-    checkArgument(limit >= 0, "limit must be >= 0");
-    checkState(!isOpen(), "Unable to change limit after read");
-    this.limit = limit;
-    return this;
-  }
-
-  @Override
-  public long limit() {
-    return limit != null ? limit : Long.MAX_VALUE;
+  protected LazyReadChannel<Object> newLazyReadChannel() {
+    return new LazyReadChannel<>(
+        () ->
+            ResumableMedia.gapic()
+                .read()
+                .byteChannel(read)
+                .setHasher(Hasher.noop())
+                .setAutoGzipDecompression(autoGzipDecompression)
+                .buffered(getBufferHandle())
+                .setReadObjectRequest(getReadObjectRequest())
+                .build());
   }
 
-  @Override
-  public int read(ByteBuffer dst) throws IOException {
-    Long diff = sub(limit, position);
-    if (diff != null && diff <= 0) {
-      close();
-      return -1;
-    }
-    try {
-      return lazyReadChannel.getChannel().read(dst);
-    } catch (IOException e) {
-      throw e;
-    } catch (Exception e) {
-      throw new IOException(StorageException.coalesce(e));
+  @NonNull
+  private ReadObjectRequest getReadObjectRequest() {
+    ByteRangeSpec rangeSpec = getByteRangeSpec();
+    ReadObjectRequest.Builder b = request.toBuilder();
+    if (request.getGeneration() == 0) {
+      Object resolvedObject = getResolvedObject();
+      if (resolvedObject != null) {
+        b.setGeneration(resolvedObject.getGeneration());
+      }
     }
-  }
-
-  ApiFuture<Object> getResults() {
-    return lazyReadChannel.getSession().get().getResult();
+    return rangeSpec.seekReadObjectRequest(b).build();
   }
 }
diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageImpl.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageImpl.java
index efbbd998..cd6816e2 100644
--- a/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageImpl.java
+++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageImpl.java
@@ -34,7 +34,6 @@ import com.google.cloud.BatchResult;
 import com.google.cloud.PageImpl;
 import com.google.cloud.PageImpl.NextPageFetcher;
 import com.google.cloud.Policy;
-import com.google.cloud.ReadChannel;
 import com.google.cloud.WriteChannel;
 import com.google.cloud.storage.Acl.Entity;
 import com.google.cloud.storage.BlobReadChannelV2.BlobReadChannelContext;
@@ -581,12 +580,12 @@ final class StorageImpl extends BaseService<StorageOptions> implements Storage {
   }
 
   @Override
-  public ReadChannel reader(String bucket, String blob, BlobSourceOption... options) {
+  public StorageReadChannel reader(String bucket, String blob, BlobSourceOption... options) {
     return reader(BlobId.of(bucket, blob), options);
   }
 
   @Override
-  public ReadChannel reader(BlobId blob, BlobSourceOption... options) {
+  public StorageReadChannel reader(BlobId blob, BlobSourceOption... options) {
     Opts<ObjectSourceOpt> opts = Opts.unwrap(options).resolveFrom(blob);
     StorageObject storageObject = Conversions.apiary().blobId().encode(blob);
     ImmutableMap<StorageRpc.Option, ?> optionsMap = opts.getRpcOptions();
diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageV2ProtoUtils.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageV2ProtoUtils.java
index cbf49ef6..0bc8e8e1 100644
--- a/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageV2ProtoUtils.java
+++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageV2ProtoUtils.java
@@ -16,8 +16,6 @@
 
 package com.google.cloud.storage;
 
-import static com.google.common.base.Preconditions.checkArgument;
-
 import com.google.protobuf.InvalidProtocolBufferException;
 import com.google.protobuf.MessageOrBuilder;
 import com.google.protobuf.util.JsonFormat;
@@ -27,7 +25,6 @@ import com.google.storage.v2.ObjectAccessControl;
 import com.google.storage.v2.ReadObjectRequest;
 import java.util.function.Predicate;
 import org.checkerframework.checker.nullness.qual.NonNull;
-import org.checkerframework.checker.nullness.qual.Nullable;
 
 final class StorageV2ProtoUtils {
 
@@ -39,44 +36,23 @@ final class StorageV2ProtoUtils {
 
   private StorageV2ProtoUtils() {}
 
+  // TODO: can we eliminate this method all together?
   @NonNull
   static ReadObjectRequest seekReadObjectRequest(
-      @NonNull ReadObjectRequest request, @Nullable Long offset, @Nullable Long limit) {
-    validate(offset, limit);
+      @NonNull ReadObjectRequest request, @NonNull ByteRangeSpec byteRangeSpec) {
+
+    long offset = byteRangeSpec.beginOffset();
+    long limit = byteRangeSpec.length();
     ReadObjectRequest req = request;
 
-    boolean setOffset = offset != null && (offset > 0 || offset != req.getReadOffset());
-    boolean setLimit = limit != null && (limit < Long.MAX_VALUE || limit != req.getReadLimit());
+    boolean setOffset = (offset > 0 && offset != req.getReadOffset());
+    boolean setLimit = (limit < ByteRangeSpec.EFFECTIVE_INFINITY && limit != req.getReadLimit());
     if (setOffset || setLimit) {
-      ReadObjectRequest.Builder b = req.toBuilder();
-      if (setOffset) {
-        b.setReadOffset(offset);
-      }
-      if (setLimit) {
-        b.setReadLimit(limit);
-      }
-      req = b.build();
+      req = byteRangeSpec.seekReadObjectRequest(req.toBuilder()).build();
     }
     return req;
   }
 
-  private static void validate(@Nullable Long offset, @Nullable Long limit) {
-    boolean offsetNull = offset == null;
-    boolean limitNull = limit == null;
-
-    if (offsetNull && limitNull) {
-      return;
-    }
-
-    if (!offsetNull) {
-      checkArgument(0 <= offset, VALIDATION_TEMPLATE, offset, limit);
-    }
-
-    if (!limitNull) {
-      checkArgument(0 <= limit, VALIDATION_TEMPLATE, offset, limit);
-    }
-  }
-
   @NonNull
   static String fmtProto(@NonNull final MessageOrBuilder msg) {
     try {
diff --git a/samples/native-image-sample/pom.xml b/samples/native-image-sample/pom.xml
index cf93b127..d5d25b6a 100644
--- a/samples/native-image-sample/pom.xml
+++ b/samples/native-image-sample/pom.xml
@@ -86,7 +86,7 @@
       <plugin>
         <groupId>org.apache.maven.plugins</groupId>
         <artifactId>maven-dependency-plugin</artifactId>
-        <version>3.4.0</version>
+        <version>3.5.0</version>
         <executions>
           <execution>
             <id>copy-dependencies</id>
@@ -124,7 +124,7 @@
           <plugin>
             <groupId>org.apache.maven.plugins</groupId>
             <artifactId>maven-surefire-plugin</artifactId>
-            <version>3.0.0-M7</version>
+            <version>3.0.0-M8</version>
             <configuration>
               <includes>
                 <include>**/*IT</include>


Test patch:
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
index 234a24b4..dce7190d 100644
--- 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
@@ -24,6 +24,7 @@ import static java.util.Objects.requireNonNull;
 import com.google.common.base.MoreObjects;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Streams;
+import com.google.storage.v2.ReadObjectRequest;
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
@@ -31,6 +32,7 @@ import java.io.ObjectInputStream;
 import java.io.ObjectOutputStream;
 import java.util.Arrays;
 import java.util.stream.Stream;
+import org.checkerframework.checker.nullness.qual.NonNull;
 import org.checkerframework.checker.nullness.qual.Nullable;
 import org.junit.Test;
 import org.junit.experimental.runners.Enclosed;
@@ -457,6 +459,13 @@ public final class ByteRangeSpecTest {
       assertThat(rs.getSpec().getHttpRangeHeader()).isEqualTo(expect.getHttpRange());
     }
 
+    @Test
+    public void seekReadObjectRequest() {
+      ByteRangeSpec spec = rs.getSpec();
+      ReadObjectRequest actual = spec.seekReadObjectRequest(ReadObjectRequest.newBuilder()).build();
+      assertThat(actual).isEqualTo(expect.getReadObjectRequest());
+    }
+
     @Test
     public void beginOffset() {
       assertThat(rs.getSpec().beginOffset()).isEqualTo(expect.getBeginOffset());
@@ -487,6 +496,7 @@ public final class ByteRangeSpecTest {
               .endOffsetInclusive(EFFECTIVE_INFINITY)
               .length(EFFECTIVE_INFINITY)
               .httpRange(null)
+              .readObjectRequest(reqId())
               .isApplicableTo(
                   ByteRangeSpec.relativeLength(null, null),
                   ByteRangeSpec.explicit(null, null),
@@ -509,6 +519,7 @@ public final class ByteRangeSpecTest {
               .endOffsetInclusive(EFFECTIVE_INFINITY)
               .length(EFFECTIVE_INFINITY)
               .httpRange(headerRangeOpen(3))
+              .readObjectRequest(reqOpen(3))
               .isApplicableTo(
                   ByteRangeSpec.relativeLength(3L, null),
                   ByteRangeSpec.explicit(3L, null),
@@ -571,6 +582,7 @@ public final class ByteRangeSpecTest {
                   .endOffsetInclusive(EFFECTIVE_INFINITY)
                   .length(EFFECTIVE_INFINITY)
                   .httpRange(headerRangeOpen(1L))
+                  .readObjectRequest(reqOpen(1L))
                   .isApplicableTo(
                       ByteRangeSpec.relativeLength(1L, null),
                       ByteRangeSpec.explicit(1L, null),
@@ -582,6 +594,7 @@ public final class ByteRangeSpecTest {
                   .endOffsetInclusive(EFFECTIVE_INFINITY)
                   .length(EFFECTIVE_INFINITY)
                   .httpRange(headerRangeOpen(EFFECTIVE_INFINITY))
+                  .readObjectRequest(reqOpen(EFFECTIVE_INFINITY))
                   .isApplicableTo(
                       ByteRangeSpec.relativeLength(EFFECTIVE_INFINITY, null),
                       ByteRangeSpec.explicit(EFFECTIVE_INFINITY, null),
@@ -624,6 +637,18 @@ public final class ByteRangeSpecTest {
     return String.format("bytes=%d-%d", min, max);
   }
 
+  private static ReadObjectRequest reqOpen(long offset) {
+    return ReadObjectRequest.newBuilder().setReadOffset(offset).build();
+  }
+
+  private static ReadObjectRequest reqBounded(long offset, long length) {
+    return ReadObjectRequest.newBuilder().setReadOffset(offset).setReadLimit(length).build();
+  }
+
+  private static ReadObjectRequest reqId() {
+    return ReadObjectRequest.getDefaultInstance();
+  }
+
   private static final class RangeScenario {
     private final ByteRangeSpec spec;
     private final Expectations expectations;
@@ -657,18 +682,21 @@ public final class ByteRangeSpecTest {
       private final long endOffsetInclusive;
       private final long length;
       @Nullable private final String httpRange;
+      @NonNull private final ReadObjectRequest readObjectRequest;
 
       private Expectations(
           long beginOffset,
           long endOffset,
           long endOffsetInclusive,
           long length,
-          @Nullable String httpRange) {
+          @Nullable String httpRange,
+          @NonNull ReadObjectRequest readObjectRequest) {
         this.beginOffset = beginOffset;
         this.endOffset = endOffset;
         this.endOffsetInclusive = endOffsetInclusive;
         this.length = length;
         this.httpRange = httpRange;
+        this.readObjectRequest = readObjectRequest;
       }
 
       public long getBeginOffset() {
@@ -691,6 +719,10 @@ public final class ByteRangeSpecTest {
         return httpRange;
       }
 
+      public @NonNull ReadObjectRequest getReadObjectRequest() {
+        return readObjectRequest;
+      }
+
       String testNameFormat() {
         return MoreObjects.toStringHelper("")
             .add("bo", fmt(beginOffset))
@@ -708,6 +740,7 @@ public final class ByteRangeSpecTest {
             .add("endOffsetInclusive", fmt(endOffsetInclusive))
             .add("length", fmt(length))
             .add("httpRange", httpRange)
+            .add("readObjectRequest", StorageV2ProtoUtils.fmtProto(readObjectRequest))
             .toString();
       }
 
@@ -736,6 +769,7 @@ public final class ByteRangeSpecTest {
       private Long endOffsetInclusive;
       private Long length;
       @Nullable private String httpRange;
+      @NonNull private ReadObjectRequest readObjectRequest;
 
       public ExpectationsBuilder beginOffset(Long beginOffset) {
         this.beginOffset = beginOffset;
@@ -762,8 +796,14 @@ public final class ByteRangeSpecTest {
         return this;
       }
 
+      public ExpectationsBuilder readObjectRequest(ReadObjectRequest readObjectRequest) {
+        this.readObjectRequest = readObjectRequest;
+        return this;
+      }
+
       public ExpectationsBuilder bounded() {
-        return this.httpRange(headerRangeClosed(beginOffset, endOffsetInclusive));
+        return this.httpRange(headerRangeClosed(beginOffset, endOffsetInclusive))
+            .readObjectRequest(reqBounded(beginOffset, length));
       }
 
       public Stream<RangeScenario> isApplicableTo(ByteRangeSpec... brss) {
@@ -777,7 +817,8 @@ public final class ByteRangeSpecTest {
             requireNonNull(endOffset, "endOffset must be non null"),
             requireNonNull(endOffsetInclusive, "endOffsetInclusive must be non null"),
             requireNonNull(length, "length must be non null"),
-            httpRange);
+            httpRange,
+            requireNonNull(readObjectRequest, "readObjectRequest must be non null"));
       }
     }
   }
diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageV2ProtoUtilsTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageV2ProtoUtilsTest.java
index a3b45a06..a6ccc8ff 100644
--- a/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageV2ProtoUtilsTest.java
+++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageV2ProtoUtilsTest.java
@@ -16,10 +16,10 @@
 
 package com.google.cloud.storage;
 
+import static com.google.cloud.storage.ByteRangeSpec.relativeLength;
 import static com.google.cloud.storage.StorageV2ProtoUtils.fmtProto;
 import static com.google.cloud.storage.StorageV2ProtoUtils.seekReadObjectRequest;
 import static com.google.common.truth.Truth.assertThat;
-import static org.junit.Assert.assertThrows;
 
 import com.google.cloud.storage.jqwik.StorageArbitraries;
 import com.google.storage.v2.ObjectAccessControl;
@@ -35,39 +35,6 @@ import net.jqwik.api.Provide;
 
 public final class StorageV2ProtoUtilsTest {
 
-  @Example
-  void validation_nullOffset_effectiveMin() {
-    seekReadObjectRequest(ReadObjectRequest.getDefaultInstance(), null, 0L);
-    seekReadObjectRequest(ReadObjectRequest.getDefaultInstance(), null, null);
-  }
-
-  @Example
-  void validation_nullLimit_effectiveMax() {
-    seekReadObjectRequest(ReadObjectRequest.getDefaultInstance(), 0L, null);
-    seekReadObjectRequest(ReadObjectRequest.getDefaultInstance(), null, null);
-  }
-
-  @Example
-  void validation_offset_lteq_limit() {
-    seekReadObjectRequest(ReadObjectRequest.getDefaultInstance(), 3L, 2L);
-    seekReadObjectRequest(ReadObjectRequest.getDefaultInstance(), 0L, 0L);
-    seekReadObjectRequest(ReadObjectRequest.getDefaultInstance(), 1L, 1L);
-  }
-
-  @Example
-  void validation_offset_gteq_0() {
-    assertThrows(
-        IllegalArgumentException.class,
-        () -> seekReadObjectRequest(ReadObjectRequest.getDefaultInstance(), -1L, null));
-  }
-
-  @Example
-  void validation_limit_gteq_0() {
-    assertThrows(
-        IllegalArgumentException.class,
-        () -> seekReadObjectRequest(ReadObjectRequest.getDefaultInstance(), null, -1L));
-  }
-
   @Example
   void objectAclEntityIdOrAltEq() {
     String entity = "project-viewer-123123";
@@ -86,21 +53,19 @@ public final class StorageV2ProtoUtilsTest {
   void seek(@ForAll("seekCases") SeekCase srr) {
     Long offset = srr.offset;
     Long limit = srr.limit;
+    ReadObjectRequest seek = seekReadObjectRequest(srr.req, relativeLength(offset, limit));
 
-    // I miss pattern matching...
+    // If both offset and limit are null, avoid allocating a new instance as we don't have any
+    // meaningful change to apply
     if (offset == null && limit == null) {
-      ReadObjectRequest seek = seekReadObjectRequest(srr.req, offset, limit);
       assertThat(seek).isSameInstanceAs(srr.req);
-    } else if (offset != null && limit == null) {
-      ReadObjectRequest seek = seekReadObjectRequest(srr.req, offset, limit);
-      assertThat(seek.getReadOffset()).isEqualTo(offset);
-    } else if (offset == null && limit != null) {
-      ReadObjectRequest seek = seekReadObjectRequest(srr.req, offset, limit);
-      assertThat(seek.getReadLimit()).isEqualTo(limit);
     } else {
-      ReadObjectRequest seek = seekReadObjectRequest(srr.req, offset, limit);
-      assertThat(seek.getReadOffset()).isEqualTo(offset);
-      assertThat(seek.getReadLimit()).isEqualTo(limit);
+      if (offset != null && offset != 0) {
+        assertThat(seek.getReadOffset()).isEqualTo(offset);
+      }
+      if (limit != null && limit != Long.MAX_VALUE) {
+        assertThat(seek.getReadLimit()).isEqualTo(limit);
+      }
     }
   }
 
@@ -128,14 +93,7 @@ public final class StorageV2ProtoUtilsTest {
 
     @Override
     public String toString() {
-      return "SeekReadRequest{"
-          + "req="
-          + fmtProto(req)
-          + ", offset="
-          + offset
-          + ", limit="
-          + limit
-          + '}';
+      return "SeekCase{" + "req=" + fmtProto(req) + ", offset=" + offset + ", limit=" + limit + '}';
     }
 
     private static SeekCase of(
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
index a4657db7..751b0975 100644
--- 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
@@ -30,7 +30,6 @@ import com.google.cloud.storage.Blob;
 import com.google.cloud.storage.BlobId;
 import com.google.cloud.storage.BlobInfo;
 import com.google.cloud.storage.BucketInfo;
-import com.google.cloud.storage.DataGeneration;
 import com.google.cloud.storage.DataGenerator;
 import com.google.cloud.storage.Storage;
 import com.google.cloud.storage.Storage.BlobSourceOption;
@@ -56,10 +55,10 @@ import java.nio.channels.Channels;
 import java.nio.channels.FileChannel;
 import java.nio.channels.WritableByteChannel;
 import java.nio.file.Files;
+import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.nio.file.StandardOpenOption;
 import java.util.Arrays;
-import java.util.Random;
 import java.util.zip.GZIPInputStream;
 import org.checkerframework.checker.nullness.qual.Nullable;
 import org.junit.Rule;
@@ -80,8 +79,6 @@ public final class ITBlobReadChannelTest {
       BaseEncoding.base64()
           .decode("H4sIAAAAAAAAAPNIzcnJV3DPz0/PSVVwzskvTVEILskvSkxPVQQA/LySchsAAAA=");
 
-  @Rule public final DataGeneration dataGeneration = new DataGeneration(new Random(872364872));
-
   @Rule public final TemporaryFolder tmp = new TemporaryFolder();
 
   @Inject public Storage storage;
@@ -139,7 +136,7 @@ public final class ITBlobReadChannelTest {
   public void testLimit_downloadToFile() throws IOException {
     String blobName = String.format("%s/src", generator.randomObjectName());
     BlobId blobId = BlobId.of(bucket.getName(), blobName);
-    ByteBuffer content = dataGeneration.randByteBuffer(108);
+    ByteBuffer content = DataGenerator.base64Characters().genByteBuffer(108);
     try (WriteChannel writer = storage.writer(BlobInfo.newBuilder(blobId).build())) {
       writer.write(content);
     }
@@ -151,18 +148,21 @@ public final class ITBlobReadChannelTest {
     duplicate.position(14);
     duplicate.limit(37);
     duplicate.get(expectedBytes);
+    String xxdExpected = xxd(expectedBytes);
 
     try {
+      Path path = Paths.get(destFileName);
       try (ReadChannel from = storage.reader(blobId);
-          FileChannel to = FileChannel.open(Paths.get(destFileName), StandardOpenOption.WRITE)) {
+          FileChannel to = FileChannel.open(path, StandardOpenOption.WRITE)) {
         from.seek(14);
         from.limit(37);
 
         ByteStreams.copy(from, to);
       }
 
-      byte[] readBytes = Files.readAllBytes(Paths.get(destFileName));
-      assertThat(readBytes).isEqualTo(expectedBytes);
+      byte[] readBytes = Files.readAllBytes(path);
+      String xxdActual = xxd(readBytes);
+      assertThat(xxdActual).isEqualTo(xxdExpected);
     } finally {
       file.delete();
     }
@@ -344,7 +344,6 @@ public final class ITBlobReadChannelTest {
   }
 
   @Test
-  @Exclude(transports = Transport.GRPC)
   public void seekAfterReadWorks() throws IOException {
     ObjectAndContent obj512KiB = objectsFixture.getObj512KiB();
     BlobInfo gen1 = obj512KiB.getInfo();
@@ -374,7 +373,6 @@ public final class ITBlobReadChannelTest {
   }
 
   @Test
-  @Exclude(transports = Transport.GRPC)
   public void limitAfterReadWorks() throws IOException {
     ObjectAndContent obj512KiB = objectsFixture.getObj512KiB();
     BlobInfo gen1 = obj512KiB.getInfo();
@@ -404,7 +402,6 @@ public final class ITBlobReadChannelTest {
   }
 
   @Test
-  @Exclude(transports = Transport.GRPC)
   public void readingLastByteReturnsOneByte_seekOnly() throws IOException {
     int length = 10;
     byte[] bytes = DataGenerator.base64Characters().genBytes(length);
@@ -426,7 +423,6 @@ public final class ITBlobReadChannelTest {
   }
 
   @Test
-  @Exclude(transports = Transport.GRPC)
   public void readingLastByteReturnsOneByte_seekAndLimit() throws IOException {
     int length = 10;
     byte[] bytes = DataGenerator.base64Characters().genBytes(length);
@@ -521,7 +517,7 @@ public final class ITBlobReadChannelTest {
       throws IOException {
     String blobName = String.format("%s/src", generator.randomObjectName());
     BlobInfo src = BlobInfo.newBuilder(bucket, blobName).build();
-    ByteBuffer content = dataGeneration.randByteBuffer(srcContentSize);
+    ByteBuffer content = DataGenerator.base64Characters().genByteBuffer(srcContentSize);
     ByteBuffer dup = content.duplicate();
     dup.position(rangeBegin);
     int newLimit = Math.min(dup.capacity(), rangeEnd);