Repository: googleapis/java-storage
Pull request #2246 (2023-10-04 15:23:10)

test: ensure Storage instances are closed in ITStorageLifecycleTest
Looking at CI logs I noticed grpc was logging a leaked channel pool. The storage instance wasn't being closed in these tests.




Patch:
diff --git a/.kokoro/continuous/propose_release.sh b/.kokoro/continuous/propose_release.sh
deleted file mode 100755
index be0ae485..00000000
--- a/.kokoro/continuous/propose_release.sh
+++ /dev/null
@@ -1,29 +0,0 @@
-#!/bin/bash
-
-# Copyright 2019 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
-#
-#     https://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.
-
-set -eo pipefail
-
-export NPM_CONFIG_PREFIX=/home/node/.npm-global
-
-if [ -f ${KOKORO_KEYSTORE_DIR}/73713_github-magic-proxy-url-release-please ]; then
-  # Groom the release PR as new commits are merged.
-  npx release-please release-pr --token=${KOKORO_KEYSTORE_DIR}/73713_github-magic-proxy-token-release-please \
-    --repo-url=googleapis/java-storage \
-    --package-name="storage" \
-    --api-url=${KOKORO_KEYSTORE_DIR}/73713_github-magic-proxy-url-release-please \
-    --proxy-key=${KOKORO_KEYSTORE_DIR}/73713_github-magic-proxy-key-release-please \
-    --release-type=java-yoshi
-fi
diff --git a/.kokoro/readme.sh b/.kokoro/readme.sh
deleted file mode 100755
index 013ce452..00000000
--- a/.kokoro/readme.sh
+++ /dev/null
@@ -1,45 +0,0 @@
-#!/bin/bash
-# Copyright 2020 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.
-
-set -eo pipefail
-
-cd ${KOKORO_ARTIFACTS_DIR}/github/java-storage
-
-# Disable buffering, so that the logs stream through.
-export PYTHONUNBUFFERED=1
-
-# Kokoro exposes this as a file, but the scripts expect just a plain variable.
-export GITHUB_TOKEN=$(cat ${KOKORO_KEYSTORE_DIR}/73713_yoshi-automation-github-key)
-
-# Setup git credentials
-echo "https://${GITHUB_TOKEN}:@github.com" >> ~/.git-credentials
-git config --global credential.helper 'store --file ~/.git-credentials'
-
-python3.6 -m pip install git+https://github.com/googleapis/synthtool.git#egg=gcp-synthtool
-
-set +e
-python3.6 -m autosynth.synth \
-    --repository=googleapis/java-storage \
-    --synth-file-name=.github/readme/synth.py \
-    --metadata-path=.github/readme/synth.metadata \
-    --pr-title="chore: regenerate README" \
-    --branch-suffix="readme"
-
-# autosynth returns 28 to signal there are no changes
-RETURN_CODE=$?
-if [[ ${RETURN_CODE} -ne 0 && ${RETURN_CODE} -ne 28 ]]
-then
-    exit ${RETURN_CODE}
-fi
diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/ParallelCompositeUploadBlobWriteSessionConfig.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/ParallelCompositeUploadBlobWriteSessionConfig.java
index 6dcf6a4c..9a964e7a 100644
--- a/google-cloud-storage/src/main/java/com/google/cloud/storage/ParallelCompositeUploadBlobWriteSessionConfig.java
+++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/ParallelCompositeUploadBlobWriteSessionConfig.java
@@ -121,19 +121,19 @@ public final class ParallelCompositeUploadBlobWriteSessionConfig extends BlobWri
   private static final int MAX_PARTS_PER_COMPOSE = 32;
   private final int maxPartsPerCompose;
   private final ExecutorSupplier executorSupplier;
-  private final BufferStrategy bufferStrategy;
+  private final BufferAllocationStrategy bufferAllocationStrategy;
   private final PartNamingStrategy partNamingStrategy;
   private final PartCleanupStrategy partCleanupStrategy;
 
   private ParallelCompositeUploadBlobWriteSessionConfig(
       int maxPartsPerCompose,
       ExecutorSupplier executorSupplier,
-      BufferStrategy bufferStrategy,
+      BufferAllocationStrategy bufferAllocationStrategy,
       PartNamingStrategy partNamingStrategy,
       PartCleanupStrategy partCleanupStrategy) {
     this.maxPartsPerCompose = maxPartsPerCompose;
     this.executorSupplier = executorSupplier;
-    this.bufferStrategy = bufferStrategy;
+    this.bufferAllocationStrategy = bufferAllocationStrategy;
     this.partNamingStrategy = partNamingStrategy;
     this.partCleanupStrategy = partCleanupStrategy;
   }
@@ -147,7 +147,7 @@ public final class ParallelCompositeUploadBlobWriteSessionConfig extends BlobWri
     return new ParallelCompositeUploadBlobWriteSessionConfig(
         maxPartsPerCompose,
         executorSupplier,
-        bufferStrategy,
+        bufferAllocationStrategy,
         partNamingStrategy,
         partCleanupStrategy);
   }
@@ -167,7 +167,7 @@ public final class ParallelCompositeUploadBlobWriteSessionConfig extends BlobWri
     return new ParallelCompositeUploadBlobWriteSessionConfig(
         maxPartsPerCompose,
         executorSupplier,
-        bufferStrategy,
+        bufferAllocationStrategy,
         partNamingStrategy,
         partCleanupStrategy);
   }
@@ -176,18 +176,19 @@ public final class ParallelCompositeUploadBlobWriteSessionConfig extends BlobWri
    * Specify a specific buffering strategy which will dictate how buffers are allocated and used
    * when performing a parallel composite upload.
    *
-   * <p><i>Default: </i> {@link BufferStrategy#simple(int) BufferStrategy#simple(16MiB)}
+   * <p><i>Default: </i> {@link BufferAllocationStrategy#simple(int)
+   * BufferAllocationStrategy#simple(16MiB)}
    *
    * @since 2.28.0 This new api is in preview and is subject to breaking changes.
    */
   @BetaApi
-  public ParallelCompositeUploadBlobWriteSessionConfig withBufferStrategy(
-      BufferStrategy bufferStrategy) {
-    checkNotNull(bufferStrategy, "bufferStrategy must be non null");
+  public ParallelCompositeUploadBlobWriteSessionConfig withBufferAllocationStrategy(
+      BufferAllocationStrategy bufferAllocationStrategy) {
+    checkNotNull(bufferAllocationStrategy, "bufferAllocationStrategy must be non null");
     return new ParallelCompositeUploadBlobWriteSessionConfig(
         maxPartsPerCompose,
         executorSupplier,
-        bufferStrategy,
+        bufferAllocationStrategy,
         partNamingStrategy,
         partCleanupStrategy);
   }
@@ -207,7 +208,7 @@ public final class ParallelCompositeUploadBlobWriteSessionConfig extends BlobWri
     return new ParallelCompositeUploadBlobWriteSessionConfig(
         maxPartsPerCompose,
         executorSupplier,
-        bufferStrategy,
+        bufferAllocationStrategy,
         partNamingStrategy,
         partCleanupStrategy);
   }
@@ -227,7 +228,7 @@ public final class ParallelCompositeUploadBlobWriteSessionConfig extends BlobWri
     return new ParallelCompositeUploadBlobWriteSessionConfig(
         maxPartsPerCompose,
         executorSupplier,
-        bufferStrategy,
+        bufferAllocationStrategy,
         partNamingStrategy,
         partCleanupStrategy);
   }
@@ -237,7 +238,7 @@ public final class ParallelCompositeUploadBlobWriteSessionConfig extends BlobWri
     return new ParallelCompositeUploadBlobWriteSessionConfig(
         MAX_PARTS_PER_COMPOSE,
         ExecutorSupplier.cachedPool(),
-        BufferStrategy.simple(ByteSizeConstants._16MiB),
+        BufferAllocationStrategy.simple(ByteSizeConstants._16MiB),
         PartNamingStrategy.noPrefix(),
         PartCleanupStrategy.always());
   }
@@ -246,7 +247,7 @@ public final class ParallelCompositeUploadBlobWriteSessionConfig extends BlobWri
   @Override
   WriterFactory createFactory(Clock clock) throws IOException {
     Executor executor = executorSupplier.get();
-    BufferHandlePool bufferHandlePool = bufferStrategy.get();
+    BufferHandlePool bufferHandlePool = bufferAllocationStrategy.get();
     return new ParallelCompositeUploadWriterFactory(clock, executor, bufferHandlePool);
   }
 
@@ -255,27 +256,27 @@ public final class ParallelCompositeUploadBlobWriteSessionConfig extends BlobWri
    * will apply to all instances of {@link BlobWriteSession} created from a single instance of
    * {@link Storage}.
    *
-   * @see #withBufferStrategy(BufferStrategy)
+   * @see #withBufferAllocationStrategy(BufferAllocationStrategy)
    * @since 2.28.0 This new api is in preview and is subject to breaking changes.
    */
   @BetaApi
   @Immutable
-  public abstract static class BufferStrategy extends Factory<BufferHandlePool>
+  public abstract static class BufferAllocationStrategy extends Factory<BufferHandlePool>
       implements Serializable {
 
-    private BufferStrategy() {}
+    private BufferAllocationStrategy() {}
 
     /**
      * Create a buffer strategy which will rely upon standard garbage collection. Each buffer will
      * be used once and then garbage collected.
      *
      * @param capacity the number of bytes each buffer should be
-     * @see #withBufferStrategy(BufferStrategy)
+     * @see #withBufferAllocationStrategy(BufferAllocationStrategy)
      * @since 2.28.0 This new api is in preview and is subject to breaking changes.
      */
     @BetaApi
-    public static BufferStrategy simple(int capacity) {
-      return new SimpleBufferStrategy(capacity);
+    public static BufferAllocationStrategy simple(int capacity) {
+      return new SimpleBufferAllocationStrategy(capacity);
     }
 
     /**
@@ -284,20 +285,20 @@ public final class ParallelCompositeUploadBlobWriteSessionConfig extends BlobWri
      *
      * @param bufferCount the number of buffers the pool will be
      * @param bufferCapacity the number of bytes each buffer should be
-     * @see #withBufferStrategy(BufferStrategy)
+     * @see #withBufferAllocationStrategy(BufferAllocationStrategy)
      * @since 2.28.0 This new api is in preview and is subject to breaking changes.
      */
     @BetaApi
-    public static BufferStrategy fixedPool(int bufferCount, int bufferCapacity) {
-      return new FixedBufferStrategy(bufferCount, bufferCapacity);
+    public static BufferAllocationStrategy fixedPool(int bufferCount, int bufferCapacity) {
+      return new FixedPoolBufferAllocationStrategy(bufferCount, bufferCapacity);
     }
 
-    private static class SimpleBufferStrategy extends BufferStrategy {
+    private static class SimpleBufferAllocationStrategy extends BufferAllocationStrategy {
       private static final long serialVersionUID = 8884826090481043434L;
 
       private final int capacity;
 
-      private SimpleBufferStrategy(int capacity) {
+      private SimpleBufferAllocationStrategy(int capacity) {
         this.capacity = capacity;
       }
 
@@ -307,13 +308,13 @@ public final class ParallelCompositeUploadBlobWriteSessionConfig extends BlobWri
       }
     }
 
-    private static class FixedBufferStrategy extends BufferStrategy {
+    private static class FixedPoolBufferAllocationStrategy extends BufferAllocationStrategy {
       private static final long serialVersionUID = 3288902741819257066L;
 
       private final int bufferCount;
       private final int bufferCapacity;
 
-      private FixedBufferStrategy(int bufferCount, int bufferCapacity) {
+      private FixedPoolBufferAllocationStrategy(int bufferCount, int bufferCapacity) {
         this.bufferCount = bufferCount;
         this.bufferCapacity = bufferCapacity;
       }
diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/ParallelCompositeUploadException.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/ParallelCompositeUploadException.java
index b03348e5..2cc791e1 100644
--- a/google-cloud-storage/src/main/java/com/google/cloud/storage/ParallelCompositeUploadException.java
+++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/ParallelCompositeUploadException.java
@@ -21,28 +21,42 @@ import com.google.api.gax.grpc.GrpcStatusCode;
 import com.google.api.gax.rpc.ApiException;
 import com.google.api.gax.rpc.ErrorDetails;
 import com.google.api.gax.rpc.StatusCode;
-import com.google.common.collect.ImmutableList;
 import io.grpc.Status.Code;
+import java.util.List;
 
-final class ParallelCompositeUploadException extends ApiException {
+/**
+ * An exception which provides access to created objects during a Parallel Composite Upload that did
+ * not finish successfully.
+ *
+ * <p>This exception can occur when calling any method on the {@link
+ * java.nio.channels.WritableByteChannel} returned from {@link BlobWriteSession#open()}, in which
+ * case it will be the cause of a {@link StorageException}.
+ *
+ * <p>Similarly, this exception will be the cause of a {@link
+ * java.util.concurrent.CancellationException} thrown by the {@link BlobWriteSession#getResult()}.
+ */
+public final class ParallelCompositeUploadException extends ApiException {
 
-  private final ApiFuture<ImmutableList<BlobId>> createdObjects;
+  private final ApiFuture<List<BlobId>> createdObjects;
 
   private ParallelCompositeUploadException(
       Throwable cause,
       StatusCode statusCode,
       ErrorDetails errorDetails,
-      ApiFuture<ImmutableList<BlobId>> createdObjects) {
+      ApiFuture<List<BlobId>> createdObjects) {
     super(cause, statusCode, false, errorDetails);
     this.createdObjects = createdObjects;
   }
 
-  public ApiFuture<ImmutableList<BlobId>> getCreatedObjects() {
+  /**
+   * A future list of the {@link BlobId}s which were created during the Parallel Composite Upload
+   * but may not have successfully been cleaned up.
+   */
+  public ApiFuture<List<BlobId>> getCreatedObjects() {
     return createdObjects;
   }
 
-  static ParallelCompositeUploadException of(
-      Throwable t, ApiFuture<ImmutableList<BlobId>> createdObjects) {
+  static ParallelCompositeUploadException of(Throwable t, ApiFuture<List<BlobId>> createdObjects) {
     StatusCode statusCode;
     ErrorDetails errorDetails;
 
diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/ParallelCompositeUploadWritableByteChannel.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/ParallelCompositeUploadWritableByteChannel.java
index ed33f975..9ff1ebdb 100644
--- a/google-cloud-storage/src/main/java/com/google/cloud/storage/ParallelCompositeUploadWritableByteChannel.java
+++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/ParallelCompositeUploadWritableByteChannel.java
@@ -431,7 +431,7 @@ final class ParallelCompositeUploadWritableByteChannel implements BufferedWritab
   }
 
   private <R> ApiFuture<R> asyncCleanupAfterFailure(Throwable originalFailure) {
-    ApiFuture<ImmutableList<BlobId>> pendingAndSuccessfulBlobIds =
+    ApiFuture<List<BlobId>> pendingAndSuccessfulBlobIds =
         getPendingAndSuccessfulBlobIds(exec, pendingParts, successfulParts);
     return ApiFutures.transformAsync(
         pendingAndSuccessfulBlobIds,
@@ -558,21 +558,21 @@ final class ParallelCompositeUploadWritableByteChannel implements BufferedWritab
       Executor exec,
       List<ApiFuture<BlobInfo>> pendingParts,
       List<BlobId> successfulParts) {
-    ApiFuture<ImmutableList<BlobId>> fCreatedObjects =
+    ApiFuture<List<BlobId>> fCreatedObjects =
         getPendingAndSuccessfulBlobIds(exec, pendingParts, successfulParts);
 
     return ParallelCompositeUploadException.of(cause, fCreatedObjects);
   }
 
   @NonNull
-  private static ApiFuture<ImmutableList<BlobId>> getPendingAndSuccessfulBlobIds(
+  private static ApiFuture<List<BlobId>> getPendingAndSuccessfulBlobIds(
       Executor exec, List<ApiFuture<BlobInfo>> pendingParts, List<BlobId> successfulParts) {
     ApiFuture<List<BlobInfo>> successfulList = ApiFutures.successfulAsList(pendingParts);
     // suppress any failure that might happen when waiting for any pending futures to resolve
     ApiFuture<List<BlobInfo>> catching =
         ApiFutures.catching(successfulList, Throwable.class, t2 -> ImmutableList.of(), exec);
 
-    ApiFuture<ImmutableList<BlobId>> fCreatedObjects =
+    ApiFuture<List<BlobId>> fCreatedObjects =
         ApiFutures.transform(
             catching,
             l ->


Test patch:
diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/ITStorageLifecycleTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/ITStorageLifecycleTest.java
index ca50784e..0c8fc12b 100644
--- a/google-cloud-storage/src/test/java/com/google/cloud/storage/ITStorageLifecycleTest.java
+++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/ITStorageLifecycleTest.java
@@ -66,12 +66,12 @@ public final class ITStorageLifecycleTest {
     service1.close();
 
     // expect a new instance to be returned
-    Storage service3 = options.getService();
-
-    assertThat(service3).isNotSameInstanceAs(service1);
-    // make sure an RPC can be done
-    StreamSupport.stream(service3.list().iterateAll().spliterator(), false)
-        .collect(Collectors.toList());
+    try (Storage service3 = options.getService()) {
+      assertThat(service3).isNotSameInstanceAs(service1);
+      // make sure an RPC can be done
+      StreamSupport.stream(service3.list().iterateAll().spliterator(), false)
+          .collect(Collectors.toList());
+    }
   }
 
   @Test
@@ -95,11 +95,12 @@ public final class ITStorageLifecycleTest {
     service1.close(); // this should be a no-op for http
 
     // expect the original instance to still be returned
-    Storage service3 = options.getService();
+    try (Storage service3 = options.getService()) {
 
-    assertThat(service3).isSameInstanceAs(service1);
-    // make sure an RPC can be done
-    StreamSupport.stream(service3.list().iterateAll().spliterator(), false)
-        .collect(Collectors.toList());
+      assertThat(service3).isSameInstanceAs(service1);
+      // make sure an RPC can be done
+      StreamSupport.stream(service3.list().iterateAll().spliterator(), false)
+          .collect(Collectors.toList());
+    }
   }
 }
diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/SerializationTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/SerializationTest.java
index 15aa3516..48d3e4ba 100644
--- a/google-cloud-storage/src/test/java/com/google/cloud/storage/SerializationTest.java
+++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/SerializationTest.java
@@ -33,7 +33,7 @@ import com.google.cloud.storage.Acl.Project.ProjectRole;
 import com.google.cloud.storage.BlobReadChannelV2.BlobReadChannelContext;
 import com.google.cloud.storage.BlobReadChannelV2.BlobReadChannelV2State;
 import com.google.cloud.storage.BlobWriteChannelV2.BlobWriteChannelV2State;
-import com.google.cloud.storage.ParallelCompositeUploadBlobWriteSessionConfig.BufferStrategy;
+import com.google.cloud.storage.ParallelCompositeUploadBlobWriteSessionConfig.BufferAllocationStrategy;
 import com.google.cloud.storage.ParallelCompositeUploadBlobWriteSessionConfig.ExecutorSupplier;
 import com.google.cloud.storage.ParallelCompositeUploadBlobWriteSessionConfig.PartCleanupStrategy;
 import com.google.cloud.storage.ParallelCompositeUploadBlobWriteSessionConfig.PartNamingStrategy;
@@ -385,7 +385,7 @@ public class SerializationTest extends BaseSerializationTest {
 
     ParallelCompositeUploadBlobWriteSessionConfig pcu2 =
         BlobWriteSessionConfigs.parallelCompositeUpload()
-            .withBufferStrategy(BufferStrategy.fixedPool(1, 3))
+            .withBufferAllocationStrategy(BufferAllocationStrategy.fixedPool(1, 3))
             .withPartCleanupStrategy(PartCleanupStrategy.never())
             .withPartNamingStrategy(PartNamingStrategy.prefix("prefix"))
             .withExecutorSupplier(ExecutorSupplier.fixedPool(5));
diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITParallelCompositeUploadBlobWriteSessionConfigTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITParallelCompositeUploadBlobWriteSessionConfigTest.java
index 72a07dd5..6d3157c0 100644
--- a/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITParallelCompositeUploadBlobWriteSessionConfigTest.java
+++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITParallelCompositeUploadBlobWriteSessionConfigTest.java
@@ -29,7 +29,7 @@ import com.google.cloud.storage.BlobWriteSessionConfigs;
 import com.google.cloud.storage.BucketInfo;
 import com.google.cloud.storage.DataGenerator;
 import com.google.cloud.storage.GrpcStorageOptions;
-import com.google.cloud.storage.ParallelCompositeUploadBlobWriteSessionConfig.BufferStrategy;
+import com.google.cloud.storage.ParallelCompositeUploadBlobWriteSessionConfig.BufferAllocationStrategy;
 import com.google.cloud.storage.ParallelCompositeUploadBlobWriteSessionConfig.ExecutorSupplier;
 import com.google.cloud.storage.ParallelCompositeUploadBlobWriteSessionConfig.PartCleanupStrategy;
 import com.google.cloud.storage.ParallelCompositeUploadBlobWriteSessionConfig.PartNamingStrategy;
@@ -105,8 +105,8 @@ public final class ITParallelCompositeUploadBlobWriteSessionConfigTest {
             .setBlobWriteSessionConfig(
                 BlobWriteSessionConfigs.parallelCompositeUpload()
                     .withExecutorSupplier(ExecutorSupplier.useExecutor(exec))
-                    // deinfe a max part size that is fairly small to aid in test speed
-                    .withBufferStrategy(BufferStrategy.simple(_1MiB))
+                    // define a max part size that is fairly small to aid in test speed
+                    .withBufferAllocationStrategy(BufferAllocationStrategy.simple(_1MiB))
                     .withPartNamingStrategy(PartNamingStrategy.prefix("prefix-a"))
                     // let our fixtures take care of cleaning things up
                     .withPartCleanupStrategy(PartCleanupStrategy.never()))