From 053a6f624018e3a2486b109d8f15fc03287f9ecb Mon Sep 17 00:00:00 2001 From: athakor Date: Mon, 20 Jul 2020 18:30:24 +0530 Subject: [PATCH 1/2] feat: add support of null to remove bucket CORS configuration --- .../com/google/cloud/storage/BucketInfo.java | 4 +-- .../com/google/cloud/storage/BucketTest.java | 32 +++++++++++++++++++ .../cloud/storage/it/ITStorageTest.java | 31 ++++++++++++++++++ 3 files changed, 65 insertions(+), 2 deletions(-) diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/BucketInfo.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/BucketInfo.java index 9a716d702..f71d6b355 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/BucketInfo.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/BucketInfo.java @@ -1240,7 +1240,7 @@ Builder setMetageneration(Long metageneration) { @Override public Builder setCors(Iterable cors) { - this.cors = cors != null ? ImmutableList.copyOf(cors) : null; + this.cors = cors != null ? ImmutableList.copyOf(cors) : ImmutableList.of(); return this; } @@ -1509,7 +1509,7 @@ public StorageClass getStorageClass() { * (CORS) */ public List getCors() { - return cors; + return cors != null ? cors : ImmutableList.of(); } /** diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/BucketTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/BucketTest.java index 94050ffef..4e871ada3 100644 --- a/google-cloud-storage/src/test/java/com/google/cloud/storage/BucketTest.java +++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/BucketTest.java @@ -886,4 +886,36 @@ public void testUpdateBucketLogging() { assertThat(actualUpdatedBucket.getLogging().getLogBucket()).isNull(); assertThat(actualUpdatedBucket.getLogging().getLogObjectPrefix()).isNull(); } + + @Test + public void testRemoveBucketCORS() { + initializeExpectedBucket(6); + List origins = ImmutableList.of(Cors.Origin.of("http://cloud.google.com")); + List httpMethods = ImmutableList.of(HttpMethod.GET); + List responseHeaders = ImmutableList.of("Content-Type"); + Cors cors = + Cors.newBuilder() + .setOrigins(origins) + .setMethods(httpMethods) + .setResponseHeaders(responseHeaders) + .setMaxAgeSeconds(100) + .build(); + BucketInfo bucketInfo = BucketInfo.newBuilder("b").setCors(ImmutableList.of(cors)).build(); + Bucket bucket = new Bucket(serviceMockReturnsOptions, new BucketInfo.BuilderImpl(bucketInfo)); + assertThat(bucket.getCors().size()).isEqualTo(1); + assertThat(bucket.getCors().get(0).getMaxAgeSeconds()).isEqualTo(100); + assertThat(bucket.getCors().get(0).getMethods()).isEqualTo(httpMethods); + assertThat(bucket.getCors().get(0).getOrigins()).isEqualTo(origins); + assertThat(bucket.getCors().get(0).getResponseHeaders()).isEqualTo(responseHeaders); + + // Remove bucket CORS configuration. + Bucket expectedUpdatedBucket = bucket.toBuilder().setCors(null).build(); + expect(storage.getOptions()).andReturn(mockOptions).times(2); + expect(storage.update(expectedUpdatedBucket)).andReturn(expectedUpdatedBucket); + replay(storage); + initializeBucket(); + Bucket updatedBucket = new Bucket(storage, new BucketInfo.BuilderImpl(expectedUpdatedBucket)); + Bucket actualUpdatedBucket = updatedBucket.update(); + assertThat(actualUpdatedBucket.getCors().size()).isEqualTo(0); + } } 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 0f509271d..9a49bb3c2 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 @@ -66,6 +66,7 @@ import com.google.cloud.storage.BucketInfo.LifecycleRule.LifecycleAction; import com.google.cloud.storage.BucketInfo.LifecycleRule.LifecycleCondition; import com.google.cloud.storage.CopyWriter; +import com.google.cloud.storage.Cors; import com.google.cloud.storage.HmacKey; import com.google.cloud.storage.HttpMethod; import com.google.cloud.storage.PostPolicyV4; @@ -3474,4 +3475,34 @@ public void testAutoContentTypeCreateFrom() throws IOException { public void testAutoContentTypeWriter() throws IOException { testAutoContentType("writer"); } + + @Test + public void testRemoveBucketCORS() throws ExecutionException, InterruptedException { + String bucketName = RemoteStorageHelper.generateBucketName(); + List origins = ImmutableList.of(Cors.Origin.of("http://cloud.google.com")); + List httpMethods = ImmutableList.of(HttpMethod.GET); + List responseHeaders = ImmutableList.of("Content-Type"); + try { + Cors cors = + Cors.newBuilder() + .setOrigins(origins) + .setMethods(httpMethods) + .setResponseHeaders(responseHeaders) + .setMaxAgeSeconds(100) + .build(); + Bucket bucket = + storage.create(BucketInfo.newBuilder(bucketName).setCors(ImmutableList.of(cors)).build()); + assertThat(bucket.getCors().size()).isEqualTo(1); + assertThat(bucket.getCors().get(0).getMaxAgeSeconds()).isEqualTo(100); + assertThat(bucket.getCors().get(0).getMethods()).isEqualTo(httpMethods); + assertThat(bucket.getCors().get(0).getOrigins()).isEqualTo(origins); + assertThat(bucket.getCors().get(0).getResponseHeaders()).isEqualTo(responseHeaders); + + // Remove bucket CORS configuration. + Bucket updatedBucket = bucket.toBuilder().setCors(ImmutableList.of()).build().update(); + assertThat(updatedBucket.getCors().size()).isEqualTo(0); + } finally { + RemoteStorageHelper.forceDelete(storage, bucketName, 5, TimeUnit.SECONDS); + } + } } From 47f366d1ac6e6c4b1653e9b8509e2352b6f7cd1c Mon Sep 17 00:00:00 2001 From: athakor Date: Tue, 21 Jul 2020 12:13:59 +0530 Subject: [PATCH 2/2] feat: addresse review changes --- .../com/google/cloud/storage/BucketInfo.java | 2 +- .../com/google/cloud/storage/BucketTest.java | 4 +- .../cloud/storage/it/ITStorageTest.java | 38 +++++++++++++------ 3 files changed, 30 insertions(+), 14 deletions(-) diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/BucketInfo.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/BucketInfo.java index f71d6b355..361363638 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/BucketInfo.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/BucketInfo.java @@ -1509,7 +1509,7 @@ public StorageClass getStorageClass() { * (CORS) */ public List getCors() { - return cors != null ? cors : ImmutableList.of(); + return cors; } /** diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/BucketTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/BucketTest.java index 4e871ada3..862fbb358 100644 --- a/google-cloud-storage/src/test/java/com/google/cloud/storage/BucketTest.java +++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/BucketTest.java @@ -902,7 +902,7 @@ public void testRemoveBucketCORS() { .build(); BucketInfo bucketInfo = BucketInfo.newBuilder("b").setCors(ImmutableList.of(cors)).build(); Bucket bucket = new Bucket(serviceMockReturnsOptions, new BucketInfo.BuilderImpl(bucketInfo)); - assertThat(bucket.getCors().size()).isEqualTo(1); + assertThat(bucket.getCors()).isNotNull(); assertThat(bucket.getCors().get(0).getMaxAgeSeconds()).isEqualTo(100); assertThat(bucket.getCors().get(0).getMethods()).isEqualTo(httpMethods); assertThat(bucket.getCors().get(0).getOrigins()).isEqualTo(origins); @@ -916,6 +916,6 @@ public void testRemoveBucketCORS() { initializeBucket(); Bucket updatedBucket = new Bucket(storage, new BucketInfo.BuilderImpl(expectedUpdatedBucket)); Bucket actualUpdatedBucket = updatedBucket.update(); - assertThat(actualUpdatedBucket.getCors().size()).isEqualTo(0); + assertThat(actualUpdatedBucket.getCors()).isEmpty(); } } 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 9a49bb3c2..b2bbe54ed 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 @@ -3490,17 +3490,33 @@ public void testRemoveBucketCORS() throws ExecutionException, InterruptedExcepti .setResponseHeaders(responseHeaders) .setMaxAgeSeconds(100) .build(); - Bucket bucket = - storage.create(BucketInfo.newBuilder(bucketName).setCors(ImmutableList.of(cors)).build()); - assertThat(bucket.getCors().size()).isEqualTo(1); - assertThat(bucket.getCors().get(0).getMaxAgeSeconds()).isEqualTo(100); - assertThat(bucket.getCors().get(0).getMethods()).isEqualTo(httpMethods); - assertThat(bucket.getCors().get(0).getOrigins()).isEqualTo(origins); - assertThat(bucket.getCors().get(0).getResponseHeaders()).isEqualTo(responseHeaders); - - // Remove bucket CORS configuration. - Bucket updatedBucket = bucket.toBuilder().setCors(ImmutableList.of()).build().update(); - assertThat(updatedBucket.getCors().size()).isEqualTo(0); + storage.create(BucketInfo.newBuilder(bucketName).setCors(ImmutableList.of(cors)).build()); + + // case-1 : Cors are set and field selector is selected then returns not-null. + Bucket remoteBucket = + storage.get(bucketName, Storage.BucketGetOption.fields(BucketField.CORS)); + assertThat(remoteBucket.getCors()).isNotNull(); + assertThat(remoteBucket.getCors().get(0).getMaxAgeSeconds()).isEqualTo(100); + assertThat(remoteBucket.getCors().get(0).getMethods()).isEqualTo(httpMethods); + assertThat(remoteBucket.getCors().get(0).getOrigins()).isEqualTo(origins); + assertThat(remoteBucket.getCors().get(0).getResponseHeaders()).isEqualTo(responseHeaders); + + // case-2 : Cors are set but field selector isn't selected then returns not-null. + remoteBucket = storage.get(bucketName); + assertThat(remoteBucket.getCors()).isNotNull(); + + // Remove CORS configuration from the bucket. + Bucket updatedBucket = remoteBucket.toBuilder().setCors(null).build().update(); + assertThat(updatedBucket.getCors()).isNull(); + + // case-3 : Cors are not set and field selector is selected then returns null. + updatedBucket = storage.get(bucketName, Storage.BucketGetOption.fields(BucketField.CORS)); + assertThat(updatedBucket.getCors()).isNull(); + + // case-4 : Cors are not set and field selector isn't selected then returns null. + updatedBucket = storage.get(bucketName); + assertThat(updatedBucket.getCors()).isNull(); + } finally { RemoteStorageHelper.forceDelete(storage, bucketName, 5, TimeUnit.SECONDS); }