From 3b1df1d00a459b134103bc8738f0294188502a37 Mon Sep 17 00:00:00 2001 From: BenWhitehead Date: Tue, 1 Jun 2021 16:24:57 -0400 Subject: [PATCH] fix: update BucketInfo translation code to properly handle lifecycle rules (#852) Fixes #850 --- .../com/google/cloud/storage/BucketInfo.java | 70 +++++++++++------- .../google/cloud/storage/BucketInfoTest.java | 71 +++++++++++++++++++ 2 files changed, 116 insertions(+), 25 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 5b69814ec..c13395238 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 @@ -43,6 +43,7 @@ import java.io.ObjectInputStream; import java.io.ObjectOutputStream; import java.io.Serializable; +import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -1838,33 +1839,52 @@ public ObjectAccessControl apply(Acl acl) { website.setNotFoundPage(notFoundPage); bucketPb.setWebsite(website); } - Set rules = new HashSet<>(); - if (deleteRules != null) { - rules.addAll( - transform( - deleteRules, - new Function() { - @Override - public Rule apply(DeleteRule deleteRule) { - return deleteRule.toPb(); - } - })); - } - if (lifecycleRules != null) { - rules.addAll( - transform( - lifecycleRules, - new Function() { - @Override - public Rule apply(LifecycleRule lifecycleRule) { - return lifecycleRule.toPb(); - } - })); - } - if (rules != null) { + if (deleteRules != null || lifecycleRules != null) { Lifecycle lifecycle = new Lifecycle(); - lifecycle.setRule(ImmutableList.copyOf(rules)); + + // Here we determine if we need to "clear" any defined Lifecycle rules by explicitly setting + // the Rule list of lifecycle to the empty list. + // In order for us to clear the rules, one of the three following must be true: + // 1. deleteRules is null while lifecycleRules is non-null and empty + // 2. lifecycleRules is null while deleteRules is non-null and empty + // 3. lifecycleRules is non-null and empty while deleteRules is non-null and empty + // If none of the above three is true, we will interpret as the Lifecycle rules being + // updated to the defined set of DeleteRule and LifecycleRule. + if ((deleteRules == null && lifecycleRules.isEmpty()) + || (lifecycleRules == null && deleteRules.isEmpty()) + || (deleteRules != null && deleteRules.isEmpty() && lifecycleRules.isEmpty())) { + lifecycle.setRule(Collections.emptyList()); + } else { + Set rules = new HashSet<>(); + if (deleteRules != null) { + rules.addAll( + transform( + deleteRules, + new Function() { + @Override + public Rule apply(DeleteRule deleteRule) { + return deleteRule.toPb(); + } + })); + } + if (lifecycleRules != null) { + rules.addAll( + transform( + lifecycleRules, + new Function() { + @Override + public Rule apply(LifecycleRule lifecycleRule) { + return lifecycleRule.toPb(); + } + })); + } + + if (!rules.isEmpty()) { + lifecycle.setRule(ImmutableList.copyOf(rules)); + } + } + bucketPb.setLifecycle(lifecycle); } diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/BucketInfoTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/BucketInfoTest.java index 8def7c306..d72901c17 100644 --- a/google-cloud-storage/src/test/java/com/google/cloud/storage/BucketInfoTest.java +++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/BucketInfoTest.java @@ -19,10 +19,12 @@ import static com.google.cloud.storage.Acl.Project.ProjectRole.VIEWERS; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import com.google.api.client.util.DateTime; import com.google.api.services.storage.model.Bucket; +import com.google.api.services.storage.model.Bucket.Lifecycle; import com.google.api.services.storage.model.Bucket.Lifecycle.Rule; import com.google.cloud.storage.Acl.Project; import com.google.cloud.storage.Acl.Role; @@ -39,6 +41,7 @@ import com.google.cloud.storage.BucketInfo.RawDeleteRule; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.List; @@ -172,6 +175,8 @@ public class BucketInfoTest { .setLogging(LOGGING) .build(); + private static final Lifecycle EMPTY_LIFECYCLE = lifecycle(Collections.emptyList()); + @Test public void testToBuilder() { compareBuckets(BUCKET_INFO, BUCKET_INFO.toBuilder().build()); @@ -376,4 +381,70 @@ public void testLogging() { assertEquals("test-bucket", logging.getLogBucket()); assertEquals("test-", logging.getLogObjectPrefix()); } + + @Test + public void testRuleMappingIsCorrect_noMutations() { + Bucket bucket = bi().build().toPb(); + assertNull(bucket.getLifecycle()); + } + + @Test + public void testRuleMappingIsCorrect_deleteLifecycleRules() { + Bucket bucket = bi().deleteLifecycleRules().build().toPb(); + assertEquals(EMPTY_LIFECYCLE, bucket.getLifecycle()); + } + + @Test + @SuppressWarnings({"deprecation"}) + public void testRuleMappingIsCorrect_setDeleteRules_null() { + Bucket bucket = bi().setDeleteRules(null).build().toPb(); + assertNull(bucket.getLifecycle()); + } + + @Test + @SuppressWarnings({"deprecation"}) + public void testRuleMappingIsCorrect_setDeleteRules_empty() { + Bucket bucket = bi().setDeleteRules(Collections.emptyList()).build().toPb(); + assertEquals(EMPTY_LIFECYCLE, bucket.getLifecycle()); + } + + @Test + public void testRuleMappingIsCorrect_setLifecycleRules_empty() { + Bucket bucket = bi().setLifecycleRules(Collections.emptyList()).build().toPb(); + assertEquals(EMPTY_LIFECYCLE, bucket.getLifecycle()); + } + + @Test + public void testRuleMappingIsCorrect_setLifeCycleRules_nonEmpty() { + LifecycleRule lifecycleRule = + new LifecycleRule( + LifecycleAction.newDeleteAction(), LifecycleCondition.newBuilder().setAge(10).build()); + Rule lifecycleDeleteAfter10 = lifecycleRule.toPb(); + Bucket bucket = bi().setLifecycleRules(ImmutableList.of(lifecycleRule)).build().toPb(); + assertEquals(lifecycle(lifecycleDeleteAfter10), bucket.getLifecycle()); + } + + @Test + @SuppressWarnings({"deprecation"}) + public void testRuleMappingIsCorrect_setDeleteRules_nonEmpty() { + DeleteRule deleteRule = DELETE_RULES.get(0); + Rule deleteRuleAge5 = deleteRule.toPb(); + Bucket bucket = bi().setDeleteRules(ImmutableList.of(deleteRule)).build().toPb(); + assertEquals(lifecycle(deleteRuleAge5), bucket.getLifecycle()); + } + + private static Lifecycle lifecycle(Rule... rules) { + return lifecycle(Arrays.asList(rules)); + } + + private static Lifecycle lifecycle(List rules) { + Lifecycle emptyLifecycle = new Lifecycle(); + emptyLifecycle.setRule(rules); + return emptyLifecycle; + } + + private static BucketInfo.Builder bi() { + String bucketId = "bucketId"; + return BucketInfo.newBuilder(bucketId); + } }