fix: make GrpcBlobWriteChannel open upon construction (#2022)

BenWhitehead · web-flow · commit 65497365072a · 2023-05-17T21:42:14.000Z
Add new integration tests for both json and grpc to verify openness
diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcBlobWriteChannel.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcBlobWriteChannel.java
@@ -91,7 +91,11 @@ public int write(ByteBuffer src) throws IOException {
 
   @Override
   public boolean isOpen() {
-    return lazyWriteChannel.isOpened() && lazyWriteChannel.getChannel().isOpen();
+    if (!writeCalledAtLeastOnce) {
+      return true;
+    } else {
+      return lazyWriteChannel.isOpened() && lazyWriteChannel.getChannel().isOpen();
+    }
   }
 
   @Override
diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/PackagePrivateMethodWorkarounds.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/PackagePrivateMethodWorkarounds.java
@@ -18,12 +18,13 @@
 
 import com.google.api.core.ApiFuture;
 import com.google.api.core.ApiFutures;
-import com.google.api.services.storage.model.StorageObject;
 import com.google.cloud.ReadChannel;
 import com.google.cloud.WriteChannel;
 import com.google.cloud.storage.BucketInfo.BuilderImpl;
 import com.google.common.collect.ImmutableList;
+import com.google.storage.v2.WriteObjectResponse;
 import java.util.Optional;
+import java.util.concurrent.ExecutionException;
 import java.util.function.Consumer;
 import java.util.function.Function;
 import javax.annotation.Nullable;
@@ -59,11 +60,26 @@ public static Blob blobCopyWithStorage(Blob b, Storage s) {
     return new Blob(s, builder);
   }
 
-  public static Function<WriteChannel, Optional<StorageObject>> maybeGetStorageObjectFunction() {
+  public static Function<WriteChannel, Optional<BlobInfo>> maybeGetBlobInfoFunction() {
     return (w) -> {
+      BlobWriteChannel blobWriteChannel;
       if (w instanceof BlobWriteChannel) {
-        BlobWriteChannel blobWriteChannel = (BlobWriteChannel) w;
-        return Optional.of(blobWriteChannel.getStorageObject());
+        blobWriteChannel = (BlobWriteChannel) w;
+        return Optional.of(blobWriteChannel.getStorageObject())
+            .map(Conversions.apiary().blobInfo()::decode);
+      } else if (w instanceof GrpcBlobWriteChannel) {
+        GrpcBlobWriteChannel grpcBlobWriteChannel = (GrpcBlobWriteChannel) w;
+        return Optional.of(grpcBlobWriteChannel.getResults())
+            .map(
+                f -> {
+                  try {
+                    return f.get();
+                  } catch (InterruptedException | ExecutionException e) {
+                    throw new RuntimeException(e);
+                  }
+                })
+            .map(WriteObjectResponse::getResource)
+            .map(Conversions.grpc().blobInfo()::decode);
       } else {
         return Optional.empty();
       }
diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITBlobWriteChannelTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITBlobWriteChannelTest.java
@@ -25,7 +25,6 @@
 
 import com.google.api.client.json.JsonParser;
 import com.google.api.gax.rpc.FixedHeaderProvider;
-import com.google.api.services.storage.model.StorageObject;
 import com.google.cloud.NoCredentials;
 import com.google.cloud.WriteChannel;
 import com.google.cloud.conformance.storage.v1.InstructionList;
@@ -232,15 +231,15 @@ private void doJsonUnexpectedEOFTest(int contentSize, int cappedByteCount) throw
     RetryTestResource postRunState = testBench.getRetryTest(retryTest);
     assertTrue(postRunState.completed);
 
-    Optional<StorageObject> optionalStorageObject =
-        PackagePrivateMethodWorkarounds.maybeGetStorageObjectFunction().apply(w);
+    Optional<BlobInfo> optionalStorageObject =
+        PackagePrivateMethodWorkarounds.maybeGetBlobInfoFunction().apply(w);
 
     assertTrue(optionalStorageObject.isPresent());
-    StorageObject storageObject = optionalStorageObject.get();
-    assertThat(storageObject.getName()).isEqualTo(blobInfoGen0.getName());
+    BlobInfo internalInfo = optionalStorageObject.get();
+    assertThat(internalInfo.getName()).isEqualTo(blobInfoGen0.getName());
 
     // construct a new blob id, without a generation, so we get the latest when we perform a get
-    BlobId blobIdGen1 = BlobId.of(storageObject.getBucket(), storageObject.getName());
+    BlobId blobIdGen1 = BlobId.of(internalInfo.getBucket(), internalInfo.getName());
     Blob blobGen2 = testStorage.get(blobIdGen1);
     assertEquals(contentSize, (long) blobGen2.getSize());
     assertNotEquals(blobInfoGen0.getGeneration(), blobGen2.getGeneration());
diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITWriteChannelTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITWriteChannelTest.java
@@ -0,0 +1,91 @@
+/*
+ * 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.it;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.cloud.WriteChannel;
+import com.google.cloud.storage.Blob;
+import com.google.cloud.storage.BlobInfo;
+import com.google.cloud.storage.BucketInfo;
+import com.google.cloud.storage.DataGenerator;
+import com.google.cloud.storage.PackagePrivateMethodWorkarounds;
+import com.google.cloud.storage.Storage;
+import com.google.cloud.storage.TransportCompatibility.Transport;
+import com.google.cloud.storage.it.runner.StorageITRunner;
+import com.google.cloud.storage.it.runner.annotations.Backend;
+import com.google.cloud.storage.it.runner.annotations.CrossRun;
+import com.google.cloud.storage.it.runner.annotations.Inject;
+import com.google.cloud.storage.it.runner.registry.Generator;
+import java.io.IOException;
+import java.util.Optional;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+@RunWith(StorageITRunner.class)
+@CrossRun(
+    backends = {Backend.PROD},
+    transports = {Transport.HTTP, Transport.GRPC})
+public final class ITWriteChannelTest {
+
+  @Inject public Storage storage;
+  @Inject public BucketInfo bucket;
+  @Inject public Generator generator;
+
+  @Test
+  public void writeChannel_isOpen_onConstruction() throws IOException {
+    BlobInfo info = BlobInfo.newBuilder(bucket, generator.randomObjectName()).build();
+    try (WriteChannel writer = storage.writer(info)) {
+      assertThat(writer.isOpen()).isTrue();
+    }
+  }
+
+  @Test
+  public void writeChannel_createsObjectEvenIfWriteNeverCalled() throws IOException {
+    BlobInfo info = BlobInfo.newBuilder(bucket, generator.randomObjectName()).build();
+    WriteChannel w;
+    try (WriteChannel writer = storage.writer(info)) {
+      w = writer;
+      assertThat(writer.isOpen()).isTrue();
+    }
+
+    Optional<BlobInfo> internalInfo =
+        PackagePrivateMethodWorkarounds.maybeGetBlobInfoFunction().apply(w);
+
+    assertThat(internalInfo.isPresent()).isTrue();
+
+    Blob blob = storage.get(info.getBlobId());
+    assertThat(blob).isNotNull();
+  }
+
+  @Test
+  public void writeChannel_openAfterWriteSmallerThanBlockSize() throws IOException {
+    BlobInfo info = BlobInfo.newBuilder(bucket, generator.randomObjectName()).build();
+    WriteChannel w;
+    try (WriteChannel writer = storage.writer(info)) {
+      w = writer;
+      assertThat(writer.isOpen()).isTrue();
+
+      int write = writer.write(DataGenerator.base64Characters().genByteBuffer(10));
+      assertThat(write).isEqualTo(10);
+
+      assertThat(writer.isOpen()).isTrue();
+    }
+
+    assertThat(w.isOpen()).isFalse();
+  }
+}
