chore: ClientStuff -> BlobReadChannelContext

ClientStuff is no longer Serializable. Instead, BlobReadChannelV2State will store the instance of HttpStorageOptions and then reconstitute the BlobReadChannelContext at restore() time.

Author: BenWhitehead

google-cloud-storage/src/main/java/com/google/cloud/storage/BlobReadChannelV2.java

@@ -28,7 +28,6 @@
 import com.google.cloud.storage.spi.v1.StorageRpc;
 import com.google.common.base.MoreObjects;
 import java.io.IOException;
-import java.io.ObjectInputStream;
 import java.io.Serializable;
 import java.nio.ByteBuffer;
 import java.util.Map;
@@ -38,7 +37,7 @@ final class BlobReadChannelV2 implements StorageReadChannel {
 
   private final StorageObject storageObject;
   private final Map<StorageRpc.Option, ?> opts;
-  private final ClientStuff clientStuff;
+  private final BlobReadChannelContext blobReadChannelContext;
 
   private LazyReadChannel<StorageObject> lazyReadChannel;
   private StorageObject resolvedObject;
@@ -48,10 +47,12 @@ final class BlobReadChannelV2 implements StorageReadChannel {
   private BufferHandle bufferHandle;
 
   BlobReadChannelV2(
-      StorageObject storageObject, Map<StorageRpc.Option, ?> opts, ClientStuff clientStuff) {
+      StorageObject storageObject,
+      Map<StorageRpc.Option, ?> opts,
+      BlobReadChannelContext blobReadChannelContext) {
     this.storageObject = storageObject;
     this.opts = opts;
-    this.clientStuff = clientStuff;
+    this.blobReadChannelContext = blobReadChannelContext;
     this.byteRangeSpec = ByteRangeSpec.nullRange();
   }
 
@@ -116,7 +117,8 @@ public synchronized int read(ByteBuffer dst) throws IOException {
   @Override
   public RestorableState<ReadChannel> capture() {
     ApiaryReadRequest apiaryReadRequest = getApiaryReadRequest();
-    return new BlobReadChannelV2State(apiaryReadRequest, clientStuff, chunkSize);
+    return new BlobReadChannelV2State(
+        apiaryReadRequest, blobReadChannelContext.getStorageOptions(), chunkSize);
   }
 
   private void maybeResetChannel(boolean umallocBuffer) throws IOException {
@@ -147,7 +149,7 @@ private LazyReadChannel<StorageObject> newLazyReadChannel() {
           }
           return ResumableMedia.http()
               .read()
-              .byteChannel(clientStuff)
+              .byteChannel(blobReadChannelContext)
               .setCallback(this::setResolvedObject)
               .buffered(bufferHandle)
               .setApiaryReadRequest(getApiaryReadRequest())
@@ -170,21 +172,22 @@ static class BlobReadChannelV2State implements RestorableState<ReadChannel>, Ser
     private static final long serialVersionUID = -7595661593080505431L;
 
     private final ApiaryReadRequest request;
-    private final ClientStuff clientStuff;
+    private final HttpStorageOptions options;
 
     private final Integer chunkSize;
 
     private BlobReadChannelV2State(
-        ApiaryReadRequest request, ClientStuff clientStuff, Integer chunkSize) {
+        ApiaryReadRequest request, HttpStorageOptions options, Integer chunkSize) {
       this.request = request;
-      this.clientStuff = clientStuff;
+      this.options = options;
       this.chunkSize = chunkSize;
     }
 
     @Override
     public ReadChannel restore() {
       BlobReadChannelV2 channel =
-          new BlobReadChannelV2(request.getObject(), request.getOptions(), clientStuff);
+          new BlobReadChannelV2(
+              request.getObject(), request.getOptions(), BlobReadChannelContext.from(options));
       channel.setByteRangeSpec(request.getByteRangeSpec());
       if (chunkSize != null) {
         channel.setChunkSize(chunkSize);
@@ -202,32 +205,31 @@ public boolean equals(Object o) {
       }
       BlobReadChannelV2State that = (BlobReadChannelV2State) o;
       return Objects.equals(request, that.request)
-          && Objects.equals(clientStuff, that.clientStuff)
+          && Objects.equals(options, that.options)
           && Objects.equals(chunkSize, that.chunkSize);
     }
 
     @Override
     public int hashCode() {
-      return Objects.hash(request, clientStuff, chunkSize);
+      return Objects.hash(request, options, chunkSize);
     }
 
     @Override
     public String toString() {
       return MoreObjects.toStringHelper(this)
-          .add("\nrequest", request)
-          .add("\nclientStuff", clientStuff)
-          .add("\nchunkSize", chunkSize)
+          .add("request", request)
+          .add("options", options)
+          .add("chunkSize", chunkSize)
           .toString();
     }
   }
 
-  static final class ClientStuff implements Serializable {
-    private static final long serialVersionUID = 4244938428650333730L;
+  static final class BlobReadChannelContext {
     private final HttpStorageOptions storageOptions;
     private final HttpRetryAlgorithmManager retryAlgorithmManager;
-    private transient Storage apiaryClient;
+    private final Storage apiaryClient;
 
-    private ClientStuff(
+    private BlobReadChannelContext(
         HttpStorageOptions storageOptions,
         Storage apiaryClient,
         HttpRetryAlgorithmManager retryAlgorithmManager) {
@@ -248,20 +250,16 @@ public Storage getApiaryClient() {
       return apiaryClient;
     }
 
-    private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundException {
-      in.defaultReadObject();
-      this.apiaryClient = storageOptions.getStorageRpcV1().getStorage();
-    }
-
-    static ClientStuff from(HttpStorageOptions options) {
-      return new ClientStuff(
+    static BlobReadChannelContext from(HttpStorageOptions options) {
+      return new BlobReadChannelContext(
           options, options.getStorageRpcV1().getStorage(), options.getRetryAlgorithmManager());
     }
 
-    static ClientStuff from(com.google.cloud.storage.Storage s) {
-      if (s instanceof StorageImpl) {
-        StorageImpl impl = (StorageImpl) s;
-        return from(impl.getOptions());
+    static BlobReadChannelContext from(com.google.cloud.storage.Storage s) {
+      StorageOptions options = s.getOptions();
+      if (options instanceof HttpStorageOptions) {
+        HttpStorageOptions httpStorageOptions = (HttpStorageOptions) options;
+        return from(httpStorageOptions);
       }
       throw new IllegalArgumentException("Only HttpStorageOptions based instance supported");
     }
@@ -271,10 +269,10 @@ public boolean equals(Object o) {
       if (this == o) {
         return true;
       }
-      if (!(o instanceof ClientStuff)) {
+      if (!(o instanceof BlobReadChannelContext)) {
         return false;
       }
-      ClientStuff that = (ClientStuff) o;
+      BlobReadChannelContext that = (BlobReadChannelContext) o;
       return Objects.equals(storageOptions, that.storageOptions)
           && Objects.equals(retryAlgorithmManager, that.retryAlgorithmManager);
     }
@@ -286,10 +284,7 @@ public int hashCode() {
 
     @Override
     public String toString() {
-      return MoreObjects.toStringHelper(this)
-          .add("\nstorageOptions", storageOptions)
-          .add("\nretryAlgorithmManager", retryAlgorithmManager)
-          .toString();
+      return MoreObjects.toStringHelper(this).add("storageOptions", storageOptions).toString();
     }
   }
 }

google-cloud-storage/src/main/java/com/google/cloud/storage/HttpDownloadSessionBuilder.java

@@ -22,7 +22,7 @@
 import com.google.api.core.SettableApiFuture;
 import com.google.api.services.storage.model.StorageObject;
 import com.google.cloud.storage.ApiaryUnbufferedReadableByteChannel.ApiaryReadRequest;
-import com.google.cloud.storage.BlobReadChannelV2.ClientStuff;
+import com.google.cloud.storage.BlobReadChannelV2.BlobReadChannelContext;
 import com.google.cloud.storage.BufferedReadableByteChannelSession.BufferedReadableByteChannel;
 import com.google.cloud.storage.UnbufferedReadableByteChannelSession.UnbufferedReadableByteChannel;
 import java.nio.ByteBuffer;
@@ -42,20 +42,21 @@ public static HttpDownloadSessionBuilder create() {
     return INSTANCE;
   }
 
-  public ReadableByteChannelSessionBuilder byteChannel(ClientStuff clientStuff) {
-    // TODO: refactor ClientStuff to push retry to a lower individual config
+  public ReadableByteChannelSessionBuilder byteChannel(
+      BlobReadChannelContext blobReadChannelContext) {
+    // TODO: refactor BlobReadChannelContext to push retry to a lower individual config
     //   similar to GapicWritableByteChannelSessionBuilder.ResumableUploadBuilder.withRetryConfig
-    return new ReadableByteChannelSessionBuilder(clientStuff);
+    return new ReadableByteChannelSessionBuilder(blobReadChannelContext);
   }
 
   public static final class ReadableByteChannelSessionBuilder {
 
-    private final ClientStuff clientStuff;
+    private final BlobReadChannelContext blobReadChannelContext;
     // private Hasher hasher; // TODO: wire in Hasher
     private Consumer<StorageObject> callback;
 
-    private ReadableByteChannelSessionBuilder(ClientStuff clientStuff) {
-      this.clientStuff = clientStuff;
+    private ReadableByteChannelSessionBuilder(BlobReadChannelContext blobReadChannelContext) {
+      this.blobReadChannelContext = blobReadChannelContext;
     }
 
     public ReadableByteChannelSessionBuilder setCallback(Consumer<StorageObject> callback) {
@@ -86,10 +87,10 @@ public UnbufferedReadableByteChannelSessionBuilder unbuffered() {
       return (request, resultFuture) ->
           new ApiaryUnbufferedReadableByteChannel(
               request,
-              clientStuff.getApiaryClient(),
+              blobReadChannelContext.getApiaryClient(),
               resultFuture,
-              clientStuff.getStorageOptions(),
-              clientStuff.getRetryAlgorithmManager().idempotent(),
+              blobReadChannelContext.getStorageOptions(),
+              blobReadChannelContext.getRetryAlgorithmManager().idempotent(),
               callback);
     }
 

google-cloud-storage/src/main/java/com/google/cloud/storage/StorageImpl.java

@@ -37,7 +37,7 @@
 import com.google.cloud.ReadChannel;
 import com.google.cloud.WriteChannel;
 import com.google.cloud.storage.Acl.Entity;
-import com.google.cloud.storage.BlobReadChannelV2.ClientStuff;
+import com.google.cloud.storage.BlobReadChannelV2.BlobReadChannelContext;
 import com.google.cloud.storage.HmacKey.HmacKeyMetadata;
 import com.google.cloud.storage.PostPolicyV4.ConditionV4Type;
 import com.google.cloud.storage.PostPolicyV4.PostConditionsV4;
@@ -590,7 +590,7 @@ public ReadChannel 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();
-    return new BlobReadChannelV2(storageObject, optionsMap, ClientStuff.from(this));
+    return new BlobReadChannelV2(storageObject, optionsMap, BlobReadChannelContext.from(this));
   }
 
   @Override

google-cloud-storage/src/test/java/com/google/cloud/storage/SerializationTest.java

@@ -27,7 +27,7 @@
 import com.google.cloud.ReadChannel;
 import com.google.cloud.Restorable;
 import com.google.cloud.storage.Acl.Project.ProjectRole;
-import com.google.cloud.storage.BlobReadChannelV2.ClientStuff;
+import com.google.cloud.storage.BlobReadChannelV2.BlobReadChannelContext;
 import com.google.cloud.storage.Storage.BucketField;
 import com.google.cloud.storage.Storage.PredefinedAcl;
 import com.google.cloud.storage.UnifiedOpts.Opt;
@@ -192,6 +192,7 @@ protected Serializable[] serializableObjects() {
   }
 
   @Override
+  @SuppressWarnings("resource")
   protected Restorable<?>[] restorableObjects() {
     HttpStorageOptions options = HttpStorageOptions.newBuilder().setProjectId("p2").build();
     ResultRetryAlgorithm<?> algorithm =
@@ -201,9 +202,7 @@ protected Restorable<?>[] restorableObjects() {
         new BlobReadChannelV2(
             new StorageObject().setBucket("b").setName("n"),
             EMPTY_RPC_OPTIONS,
-            ClientStuff.from(options));
-    // avoid closing when you don't want partial writes to GCS upon failure
-    @SuppressWarnings("resource")
+            BlobReadChannelContext.from(options));
     BlobWriteChannel writer =
         new BlobWriteChannel(
             options, BlobInfo.newBuilder(BlobId.of("b", "n")).build(), "upload-id", algorithm);