From 7280a1dd7936478cf8acaa56891d2b463281fd77 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Martin Date: Fri, 2 Nov 2018 13:57:51 -0700 Subject: [PATCH 1/2] Stop leaving leftover buckets in ITGcsNIO This integration test now deletes all the buckets it creates. The issue was that requester-pays buckets couldn't be deleted by the existing code. This PR updates RemoteStorageHelper so it can be given a userProject for requester-pays buckets. --- .../storage/contrib/nio/it/ITGcsNio.java | 10 ++-- .../storage/testing/RemoteStorageHelper.java | 48 ++++++++++++++++--- 2 files changed, 49 insertions(+), 9 deletions(-) diff --git a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/it/ITGcsNio.java b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/it/ITGcsNio.java index 08e75c1137fe..9b85a2963a16 100644 --- a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/it/ITGcsNio.java +++ b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/it/ITGcsNio.java @@ -125,9 +125,13 @@ public static void beforeClass() throws IOException { @AfterClass public static void afterClass() throws ExecutionException, InterruptedException { - if (storage != null && !RemoteStorageHelper.forceDelete(storage, BUCKET, 5, TimeUnit.SECONDS) && - log.isLoggable(Level.WARNING)) { - log.log(Level.WARNING, "Deletion of bucket {0} timed out, bucket is not empty", BUCKET); + if (storage != null) { + for (String bucket : new String[]{BUCKET, REQUESTER_PAYS_BUCKET}) { + if (!RemoteStorageHelper.forceDelete(storage, bucket, 5, TimeUnit.SECONDS, project) && + log.isLoggable(Level.WARNING)) { + log.log(Level.WARNING, "Deletion of bucket {0} timed out, bucket is not empty", bucket); + } + } } } diff --git a/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/testing/RemoteStorageHelper.java b/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/testing/RemoteStorageHelper.java index 666b9ed6449b..2cd61bfdcc4a 100644 --- a/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/testing/RemoteStorageHelper.java +++ b/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/testing/RemoteStorageHelper.java @@ -31,6 +31,7 @@ import java.io.IOException; import java.io.InputStream; import java.util.ArrayList; +import java.util.List; import java.util.UUID; import java.util.concurrent.Callable; import java.util.concurrent.ExecutionException; @@ -121,8 +122,29 @@ public void run() { */ public static Boolean forceDelete(Storage storage, String bucket, long timeout, TimeUnit unit) throws InterruptedException, ExecutionException { + return forceDelete(storage, bucket, timeout, unit, ""); + } + + /** + * Deletes a bucket, even if non-empty. Objects in the bucket are listed and deleted until bucket + * deletion succeeds or {@code timeout} expires. To allow for the timeout, this method uses a + * separate thread to send the delete requests. Use + * {@link #forceDelete(Storage storage, String bucket)} if spawning an additional thread is + * undesirable, such as in the App Engine production runtime. + * + * @param storage the storage service to be used to issue requests + * @param bucket the bucket to be deleted + * @param timeout the maximum time to wait + * @param unit the time unit of the timeout argument + * @param userProject the project to bill for requester-pays buckets (or "") + * @return true if deletion succeeded, false if timeout expired + * @throws InterruptedException if the thread deleting the bucket is interrupted while waiting + * @throws ExecutionException if an exception was thrown while deleting bucket or bucket objects + */ + public static Boolean forceDelete(Storage storage, String bucket, long timeout, TimeUnit unit, String userProject) + throws InterruptedException, ExecutionException { ExecutorService executor = Executors.newSingleThreadExecutor(); - Future future = executor.submit(new DeleteBucketTask(storage, bucket)); + Future future = executor.submit(new DeleteBucketTask(storage, bucket, userProject)); try { return future.get(timeout, unit); } catch (TimeoutException ex) { @@ -210,26 +232,40 @@ private static RetrySettings retrySettings() { private static class DeleteBucketTask implements Callable { - private Storage storage; - private String bucket; + private final Storage storage; + private final String bucket; + private final String userProject; public DeleteBucketTask(Storage storage, String bucket) { this.storage = storage; this.bucket = bucket; + this.userProject = ""; + } + + public DeleteBucketTask(Storage storage, String bucket, String userProject) { + this.storage = storage; + this.bucket = bucket; + this.userProject = userProject; } @Override public Boolean call() { while (true) { ArrayList ids = new ArrayList<>(); - for (BlobInfo info : storage.list(bucket, BlobListOption.versions(true)).getValues()) { + for (BlobInfo info : storage.list(bucket, BlobListOption.versions(true), BlobListOption.userProject(userProject)).getValues()) { ids.add(info.getBlobId()); } if (!ids.isEmpty()) { - storage.delete(ids); + List results = storage.delete(ids); + for (int i=0; i Date: Mon, 5 Nov 2018 12:19:03 -0800 Subject: [PATCH 2/2] Don't set userProject if empty, add test RemoteStorageHelper now doesn't set the userProject option if it's empty. Also, added testForceDeleteRetriesWithUserProject unit test. --- .../storage/testing/RemoteStorageHelper.java | 26 ++++++++++++++----- .../testing/RemoteStorageHelperTest.java | 26 ++++++++++++++++++- 2 files changed, 45 insertions(+), 7 deletions(-) diff --git a/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/testing/RemoteStorageHelper.java b/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/testing/RemoteStorageHelper.java index 2cd61bfdcc4a..57045875fed6 100644 --- a/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/testing/RemoteStorageHelper.java +++ b/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/testing/RemoteStorageHelper.java @@ -42,6 +42,8 @@ import java.util.concurrent.TimeoutException; import java.util.logging.Level; import java.util.logging.Logger; + +import com.google.common.base.Strings; import org.threeten.bp.Duration; /** @@ -252,20 +254,32 @@ public DeleteBucketTask(Storage storage, String bucket, String userProject) { public Boolean call() { while (true) { ArrayList ids = new ArrayList<>(); - for (BlobInfo info : storage.list(bucket, BlobListOption.versions(true), BlobListOption.userProject(userProject)).getValues()) { + Page listedBlobs; + if (Strings.isNullOrEmpty(userProject)) { + listedBlobs = storage.list(bucket, BlobListOption.versions(true)); + } else { + listedBlobs = storage.list(bucket, BlobListOption.versions(true), BlobListOption.userProject(userProject)); + } + for (BlobInfo info : listedBlobs.getValues()) { ids.add(info.getBlobId()); } if (!ids.isEmpty()) { List results = storage.delete(ids); - for (int i=0; i ids = new ArrayList<>(); + ids.add(BLOB_ID1); + ids.add(BLOB_ID2); + EasyMock.expect(storageMock.delete(ids)).andReturn(ImmutableList.of(Boolean.TRUE, Boolean.FALSE)).anyTimes(); + EasyMock.expect(storageMock.delete(BUCKET_NAME, BLOB_NAME2, Storage.BlobSourceOption.userProject(USER_PROJECT))).andReturn(true).anyTimes(); + EasyMock.expect(storageMock.list(BUCKET_NAME, BlobListOption.versions(true), BlobListOption.userProject(USER_PROJECT))) + .andReturn(blobPage); + EasyMock.expect(storageMock.delete(BUCKET_NAME, Storage.BucketSourceOption.userProject(USER_PROJECT))).andReturn(true); + EasyMock.replay(storageMock, blob1, blob2); + try { + RemoteStorageHelper.forceDelete(storageMock, BUCKET_NAME, 5, TimeUnit.SECONDS, USER_PROJECT); + } finally { + EasyMock.verify(storageMock); + } + } + @Test public void testCreateFromStream() { RemoteStorageHelper helper = RemoteStorageHelper.create(PROJECT_ID, JSON_KEY_STREAM);