Repository: googleapis/java-storage
Pull request #1422 (2022-05-27 19:15:28)

feat: make Storage extend AutoClosable
* Add StorageFixture to manage the test scoped lifecycle of a storage instance
* Update ITStorageTest to use new fixture
* Update ITBlobReadChannelTest to use new fixture


Patch:
diff --git a/google-cloud-storage/clirr-ignored-differences.xml b/google-cloud-storage/clirr-ignored-differences.xml
index 674def1e..2ef4ca3e 100644
--- a/google-cloud-storage/clirr-ignored-differences.xml
+++ b/google-cloud-storage/clirr-ignored-differences.xml
@@ -46,4 +46,15 @@
     <method>* setTransportOptions(*)</method>
   </difference>
 
+  <!--
+  Allow com.google.cloud.storage.Storage to extends AutoClosable
+  // Storage defines a default implementation in the interface, but clirr
+  // is ignorant to java 8 constructs
+  -->
+  <difference>
+    <differenceType>7012</differenceType>
+    <className>com/google/cloud/storage/Storage</className>
+    <method>void close()</method>
+  </difference>
+
 </differences>
diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java
index 05b07b26..3cf34c65 100644
--- a/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java
+++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java
@@ -58,6 +58,13 @@ final class GrpcStorageImpl extends BaseService<StorageOptions> implements Stora
     this.retryAlgorithmManager = options.getRetryAlgorithmManager();
   }
 
+  @Override
+  public void close() throws Exception {
+    try (GrpcStorageStub s = grpcStorageStub) {
+      s.shutdownNow();
+    }
+  }
+
   @Override
   public Bucket create(BucketInfo bucketInfo, BucketTargetOption... options) {
     return todo();
diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/Storage.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/Storage.java
index d27de901..0c6960f8 100644
--- a/google-cloud-storage/src/main/java/com/google/cloud/storage/Storage.java
+++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/Storage.java
@@ -31,7 +31,6 @@ import com.google.cloud.Service;
 import com.google.cloud.Tuple;
 import com.google.cloud.WriteChannel;
 import com.google.cloud.storage.Acl.Entity;
-import com.google.cloud.storage.Blob.BlobSourceOption;
 import com.google.cloud.storage.HmacKey.HmacKeyMetadata;
 import com.google.cloud.storage.PostPolicyV4.PostConditionsV4;
 import com.google.cloud.storage.PostPolicyV4.PostFieldsV4;
@@ -63,7 +62,7 @@ import java.util.concurrent.TimeUnit;
  * @see <a href="https://cloud.google.com/storage/docs">Google Cloud Storage</a>
  */
 @InternalExtensionOnly
-public interface Storage extends Service<StorageOptions> {
+public interface Storage extends Service<StorageOptions>, AutoCloseable {
 
   enum PredefinedAcl {
     AUTHENTICATED_READ("authenticatedRead"),
@@ -3746,4 +3745,7 @@ public interface Storage extends Service<StorageOptions> {
    * @throws StorageException upon failure
    */
   boolean deleteNotification(String bucket, String notificationId);
+
+  @Override
+  default void close() throws Exception {}
 }


Test patch:
diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageFixture.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageFixture.java
new file mode 100644
index 00000000..26991b1b
--- /dev/null
+++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageFixture.java
@@ -0,0 +1,74 @@
+/*
+ * Copyright 2022 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.common.base.Preconditions.checkState;
+import static java.util.Objects.requireNonNull;
+
+import java.util.function.Supplier;
+import org.checkerframework.checker.nullness.qual.NonNull;
+import org.junit.rules.TestRule;
+import org.junit.runner.Description;
+import org.junit.runners.model.Statement;
+
+/** A JUnit 4 {@link TestRule} which creates a JUnit lifecycle bound instance of {@link Storage}. */
+public final class StorageFixture implements TestRule {
+
+  private final Supplier<@NonNull Storage> factory;
+
+  private Storage instance;
+
+  private StorageFixture(Supplier<@NonNull Storage> factory) {
+    this.factory = factory;
+  }
+
+  public Storage getInstance() {
+    checkState(instance != null, "not active. Should only be called within junit managed scope");
+    return instance;
+  }
+
+  @Override
+  public Statement apply(Statement base, Description description) {
+    return new Statement() {
+      @Override
+      public void evaluate() throws Throwable {
+        instance = requireNonNull(factory.get(), "factory.get() must not provide a null value");
+        try (Storage s = instance) {
+          base.evaluate();
+        } finally {
+          instance = null;
+        }
+      }
+    };
+  }
+
+  public static StorageFixture defaultHttp() {
+    return from(() -> StorageOptions.http().build());
+  }
+
+  public static StorageFixture defaultGrpc() {
+    return from(() -> StorageOptions.grpc().build());
+  }
+
+  public static StorageFixture from(Supplier<StorageOptions> opts) {
+    return of(() -> opts.get().getService());
+  }
+
+  public static StorageFixture of(Supplier<@NonNull Storage> s) {
+    return new StorageFixture(s);
+  }
+}
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 30fc0d1d..438622a7 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
@@ -25,7 +25,7 @@ import com.google.cloud.storage.BlobInfo;
 import com.google.cloud.storage.BucketInfo;
 import com.google.cloud.storage.DataGeneration;
 import com.google.cloud.storage.Storage;
-import com.google.cloud.storage.StorageOptions;
+import com.google.cloud.storage.StorageFixture;
 import com.google.cloud.storage.testing.RemoteStorageHelper;
 import com.google.common.io.ByteStreams;
 import java.io.File;
@@ -54,13 +54,15 @@ public final class ITBlobReadChannelTest {
 
   @Rule public final TemporaryFolder tmp = new TemporaryFolder();
 
+  @Rule public final StorageFixture storageFixture = StorageFixture.defaultHttp();
+
   private Storage storage;
   private String bucketName;
   private String blobName;
 
   @Before
   public void setUp() throws Exception {
-    storage = StorageOptions.http().build().getService();
+    storage = storageFixture.getInstance();
 
     bucketName = RemoteStorageHelper.generateBucketName();
     storage.create(BucketInfo.of(bucketName));
diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITStorageTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITStorageTest.java
index 009dcbc2..c9d764b6 100644
--- a/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITStorageTest.java
+++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITStorageTest.java
@@ -88,6 +88,7 @@ import com.google.cloud.storage.StorageBatch;
 import com.google.cloud.storage.StorageBatchResult;
 import com.google.cloud.storage.StorageClass;
 import com.google.cloud.storage.StorageException;
+import com.google.cloud.storage.StorageFixture;
 import com.google.cloud.storage.StorageOptions;
 import com.google.cloud.storage.StorageRoles;
 import com.google.cloud.storage.spi.StorageRpcFactory;
@@ -157,6 +158,7 @@ import org.apache.http.impl.client.HttpClients;
 import org.apache.http.impl.conn.PoolingHttpClientConnectionManager;
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
+import org.junit.ClassRule;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TestName;
@@ -169,7 +171,6 @@ import org.threeten.bp.format.DateTimeFormatter;
 
 public class ITStorageTest {
 
-  private static RemoteStorageHelper remoteStorageHelper;
   private static Storage storage;
   private static TopicAdminClient topicAdminClient;
   private static String kmsKeyOneResourcePath;
@@ -227,6 +228,10 @@ public class ITStorageTest {
   private static final ImmutableList<LifecycleRule> LIFECYCLE_RULES =
       ImmutableList.of(LIFECYCLE_RULE_1, LIFECYCLE_RULE_2);
 
+  @ClassRule
+  public static final StorageFixture storageFixture =
+      StorageFixture.of(() -> RemoteStorageHelper.create().getOptions().getService());
+
   @Rule public final TestName testName = new TestName();
   @Rule public final DataGeneration dataGeneration = new DataGeneration(new Random(1234567890));
   private static final String PROJECT = ServiceOptions.getDefaultProjectId();
@@ -241,8 +246,7 @@ public class ITStorageTest {
 
   @BeforeClass
   public static void beforeClass() throws IOException {
-    remoteStorageHelper = RemoteStorageHelper.create();
-    storage = remoteStorageHelper.getOptions().getService();
+    storage = storageFixture.getInstance();
 
     storage.create(
         BucketInfo.newBuilder(BUCKET)
@@ -329,7 +333,7 @@ public class ITStorageTest {
 
   private static void prepareKmsKeys() throws IOException {
     // https://cloud.google.com/storage/docs/encryption/using-customer-managed-keys
-    String projectId = remoteStorageHelper.getOptions().getProjectId();
+    String projectId = storage.getOptions().getProjectId();
     GoogleCredentials credentials = GoogleCredentials.getApplicationDefault();
     kmsChannel = ManagedChannelBuilder.forTarget("cloudkms.googleapis.com:443").build();
     KeyManagementServiceBlockingStub kmsStub =
@@ -1067,7 +1071,7 @@ public class ITStorageTest {
       assertTrue(e.getMessage().contains("User project specified in the request is invalid"));
     }
 
-    String projectId = remoteStorageHelper.getOptions().getProjectId();
+    String projectId = storage.getOptions().getProjectId();
     while (true) {
       Page<Blob> page =
           storage.list(
@@ -2380,7 +2384,7 @@ public class ITStorageTest {
       assertTrue(updatedBucket.requesterPays());
     }
 
-    String projectId = remoteStorageHelper.getOptions().getProjectId();
+    String projectId = storage.getOptions().getProjectId();
 
     Storage.BucketSourceOption[] bucketOptions =
         requesterPays
@@ -2502,7 +2506,7 @@ public class ITStorageTest {
     assertNotNull(accessId);
     assertNotNull(metadata.getEtag());
     assertNotNull(metadata.getId());
-    assertEquals(remoteStorageHelper.getOptions().getProjectId(), metadata.getProjectId());
+    assertEquals(storage.getOptions().getProjectId(), metadata.getProjectId());
     assertEquals(serviceAccount.getEmail(), metadata.getServiceAccount().getEmail());
     assertEquals(HmacKey.HmacKeyState.ACTIVE, metadata.getState());
     assertNotNull(metadata.getCreateTime());
@@ -2606,7 +2610,7 @@ public class ITStorageTest {
     Bucket bucketTrue = storage.update(bucketDefault.toBuilder().setRequesterPays(true).build());
     assertTrue(bucketTrue.requesterPays());
 
-    String projectId = remoteStorageHelper.getOptions().getProjectId();
+    String projectId = storage.getOptions().getProjectId();
 
     Storage.BucketSourceOption[] bucketOptions =
         new Storage.BucketSourceOption[] {Storage.BucketSourceOption.userProject(projectId)};
@@ -2669,7 +2673,7 @@ public class ITStorageTest {
 
   @Test
   public void testBucketPolicyV1() {
-    String projectId = remoteStorageHelper.getOptions().getProjectId();
+    String projectId = storage.getOptions().getProjectId();
 
     Storage.BucketSourceOption[] bucketOptions = new Storage.BucketSourceOption[] {};
     Identity projectOwner = Identity.projectOwner(projectId);
@@ -2734,7 +2738,7 @@ public class ITStorageTest {
                     .setIsUniformBucketLevelAccessEnabled(true)
                     .build())
             .build());
-    String projectId = remoteStorageHelper.getOptions().getProjectId();
+    String projectId = storage.getOptions().getProjectId();
 
     Storage.BucketSourceOption[] bucketOptions =
         new Storage.BucketSourceOption[] {Storage.BucketSourceOption.requestedPolicyVersion(3)};
@@ -2904,7 +2908,7 @@ public class ITStorageTest {
     Bucket updatedBucket = storage.update(remoteBucket);
     assertTrue(updatedBucket.requesterPays());
 
-    String projectId = remoteStorageHelper.getOptions().getProjectId();
+    String projectId = storage.getOptions().getProjectId();
     Bucket.BlobTargetOption option = Bucket.BlobTargetOption.userProject(projectId);
     String blobName = "test-create-empty-blob-requester-pays";
     Blob remoteBlob = updatedBucket.create(blobName, BLOB_BYTE_CONTENT, option);
@@ -2920,7 +2924,7 @@ public class ITStorageTest {
 
   @Test
   public void testListBucketRequesterPaysFails() throws InterruptedException {
-    String projectId = remoteStorageHelper.getOptions().getProjectId();
+    String projectId = storage.getOptions().getProjectId();
     Iterator<Bucket> bucketIterator =
         storage
             .list(
@@ -3023,7 +3027,7 @@ public class ITStorageTest {
 
   private void retentionPolicyLockRequesterPays(boolean requesterPays)
       throws ExecutionException, InterruptedException {
-    String projectId = remoteStorageHelper.getOptions().getProjectId();
+    String projectId = storage.getOptions().getProjectId();
     String bucketName = RemoteStorageHelper.generateBucketName();
     BucketInfo bucketInfo;
     if (requesterPays) {
@@ -3163,7 +3167,7 @@ public class ITStorageTest {
 
   @Test
   public void testGetServiceAccount() {
-    String projectId = remoteStorageHelper.getOptions().getProjectId();
+    String projectId = storage.getOptions().getProjectId();
     ServiceAccount serviceAccount = storage.getServiceAccount(projectId);
     assertNotNull(serviceAccount);
     assertTrue(serviceAccount.getEmail().endsWith(SERVICE_ACCOUNT_EMAIL_SUFFIX));