From e16c5064ba22862d8b685b541939c9d08ceb601e Mon Sep 17 00:00:00 2001 From: Mahitha Adapa Date: Thu, 9 Jul 2026 19:41:34 -0400 Subject: [PATCH] fix(storage): handle map clears without current state --- .../com/google/cloud/storage/BlobInfo.java | 24 ++++--- .../com/google/cloud/storage/BucketInfo.java | 12 +++- .../storage/it/ITNestedUpdateMaskTest.java | 72 +++++++++++++++++++ 3 files changed, 98 insertions(+), 10 deletions(-) diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/BlobInfo.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/BlobInfo.java index a083338e745c..d94f49fc1d30 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/BlobInfo.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/BlobInfo.java @@ -814,13 +814,16 @@ static final class BuilderImpl extends Builder { private OffsetDateTime softDeleteTime; private OffsetDateTime hardDeleteTime; private ObjectContexts contexts; + private final boolean hasOriginalState; private final ImmutableSet.Builder modifiedFields = ImmutableSet.builder(); BuilderImpl(BlobId blobId) { this.blobId = blobId; + this.hasOriginalState = false; } BuilderImpl(BlobInfo blobInfo) { + this.hasOriginalState = true; blobId = blobInfo.blobId; generatedId = blobInfo.generatedId; cacheControl = blobInfo.cacheControl; @@ -1080,8 +1083,13 @@ Builder setMediaLink(String mediaLink) { public Builder setMetadata(@Nullable Map<@NonNull String, @Nullable String> metadata) { Map left = this.metadata; Map right = metadata; - if (!Objects.equals(left, right)) { - diffMaps(BlobField.METADATA, left, right, modifiedFields::add); + boolean clearWithoutBase = !hasOriginalState && (right == null || right.isEmpty()); + if (!Objects.equals(left, right) || clearWithoutBase) { + if (clearWithoutBase) { + modifiedFields.add(BlobField.METADATA); + } else { + diffMaps(BlobField.METADATA, left, right, modifiedFields::add); + } if (right != null) { this.metadata = new HashMap<>(right); } else { @@ -1275,18 +1283,18 @@ public Builder setContexts(ObjectContexts contexts) { this.contexts == null ? null : this.contexts.getCustom(); Map right = contexts == null ? null : contexts.getCustom(); - if (!Objects.equals(left, right)) { - if (right != null) { + boolean clearWithoutBase = !hasOriginalState && (right == null || right.isEmpty()); + if (!Objects.equals(left, right) || clearWithoutBase) { + if (clearWithoutBase || right == null) { + modifiedFields.add(BlobField.OBJECT_CONTEXTS); + } else { diffMaps( NamedField.nested(BlobField.OBJECT_CONTEXTS, NamedField.literal("custom")), left, right, modifiedFields::add); - this.contexts = contexts; - } else { - modifiedFields.add(BlobField.OBJECT_CONTEXTS); - this.contexts = null; } + this.contexts = contexts; } return this; } diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/BucketInfo.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/BucketInfo.java index e6ec27cca423..9ee37555c969 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/BucketInfo.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/BucketInfo.java @@ -2757,13 +2757,16 @@ static final class BuilderImpl extends Builder { private CustomerManagedEncryptionEnforcementConfig customerManagedEncryptionEnforcementConfig; private CustomerSuppliedEncryptionEnforcementConfig customerSuppliedEncryptionEnforcementConfig; private Boolean isUnreachable; + private final boolean hasOriginalState; private final ImmutableSet.Builder modifiedFields = ImmutableSet.builder(); BuilderImpl(String name) { this.name = name; + this.hasOriginalState = false; } BuilderImpl(BucketInfo bucketInfo) { + this.hasOriginalState = true; generatedId = bucketInfo.generatedId; project = bucketInfo.project; name = bucketInfo.name; @@ -3061,8 +3064,13 @@ public Builder setDefaultAcl(Iterable acl) { public Builder setLabels(@Nullable Map<@NonNull String, @Nullable String> labels) { Map left = this.labels; Map right = labels; - if (!Objects.equals(left, right)) { - diffMaps(BucketField.LABELS, left, right, modifiedFields::add); + boolean clearWithoutBase = !hasOriginalState && (right == null || right.isEmpty()); + if (!Objects.equals(left, right) || clearWithoutBase) { + if (clearWithoutBase) { + modifiedFields.add(BucketField.LABELS); + } else { + diffMaps(BucketField.LABELS, left, right, modifiedFields::add); + } if (right != null) { this.labels = new HashMap<>(right); } else { diff --git a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITNestedUpdateMaskTest.java b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITNestedUpdateMaskTest.java index b40eb1d8e258..08f0e0f690de 100644 --- a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITNestedUpdateMaskTest.java +++ b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITNestedUpdateMaskTest.java @@ -130,6 +130,18 @@ public void testBucketLabels() throws Exception { } } + @Test + public void testBucketLabels_updateBaseOnNewInfoInsteadOfResolvedInfo() throws Exception { + BucketInfo bucket = newBucketInfo(param.initial); + try (TemporaryBucket tempB = + TemporaryBucket.newBuilder().setBucketInfo(bucket).setStorage(storage).build()) { + BucketInfo gen1 = tempB.getBucket(); + BucketInfo modified = BucketInfo.newBuilder(gen1.getName()).setLabels(param.update).build(); + Bucket gen2 = storage.update(modified); + assertThat(gen2.getLabels()).isEqualTo(param.expected); + } + } + @Test public void testBlobMetadata() { BlobInfo blob = newBlobInfo(param.initial); @@ -139,6 +151,17 @@ public void testBlobMetadata() { assertThat(gen2.getMetadata()).isEqualTo(param.expected); } + @Test + public void testBlobMetadata_updateBaseOnNewInfoInsteadOfResolvedInfo() { + BlobInfo blob = newBlobInfo(param.initial); + Blob gen1 = storage.create(blob, BlobTargetOption.doesNotExist()); + BlobInfo modified = + BlobInfo.newBuilder(bucket, gen1.getName()).setMetadata(param.update).build(); + Blob gen2 = + storage.update(modified, BlobTargetOption.metagenerationMatch(gen1.getMetageneration())); + assertThat(gen2.getMetadata()).isEqualTo(param.expected); + } + @Test public void testBlobContexts() { ObjectContexts initial = contextsFromMap(param.initial); @@ -158,6 +181,26 @@ public void testBlobContexts() { assertContextsWithEqualValues(gen2.getContexts(), expected); } + @Test + public void testBlobContexts_updateBaseOnNewInfoInsteadOfResolvedInfo() { + ObjectContexts initial = contextsFromMap(param.initial); + ObjectContexts update = contextsFromMap(param.update); + ObjectContexts expected = contextsFromMap(param.expected); + + String blobName = generator.randomObjectName(); + BlobInfo.Builder builder = BlobInfo.newBuilder(bucket, blobName); + if (initial != null) { + builder.setContexts(initial); + } + BlobInfo info = builder.build(); + Blob gen1 = storage.create(info, BlobTargetOption.doesNotExist()); + + BlobInfo modified = BlobInfo.newBuilder(bucket, gen1.getName()).setContexts(update).build(); + Blob gen2 = + storage.update(modified, BlobTargetOption.metagenerationMatch(gen1.getMetageneration())); + assertContextsWithEqualValues(gen2.getContexts(), expected); + } + @Test public void testBlob_metadataAndContext() { ObjectContexts initial = contextsFromMap(param.initial); @@ -182,6 +225,35 @@ public void testBlob_metadataAndContext() { assertThat(gen2.getMetadata()).isEqualTo(param.expected); } + @Test + public void testBlob_metadataAndContext_updateBaseOnNewInfoInsteadOfResolvedInfo() { + ObjectContexts initial = contextsFromMap(param.initial); + ObjectContexts update = contextsFromMap(param.update); + ObjectContexts expected = contextsFromMap(param.expected); + + String blobName = generator.randomObjectName(); + BlobInfo.Builder builder = BlobInfo.newBuilder(bucket, blobName); + if (initial != null) { + builder.setContexts(initial); + } + if (param.initial != null) { + builder.setMetadata(param.initial); + } + + BlobInfo info = builder.build(); + Blob gen1 = storage.create(info, BlobTargetOption.doesNotExist()); + + BlobInfo modified = + BlobInfo.newBuilder(bucket, gen1.getName()) + .setContexts(update) + .setMetadata(param.update) + .build(); + Blob gen2 = + storage.update(modified, BlobTargetOption.metagenerationMatch(gen1.getMetageneration())); + assertContextsWithEqualValues(gen2.getContexts(), expected); + assertThat(gen2.getMetadata()).isEqualTo(param.expected); + } + private static void assertContextsWithEqualValues( @Nullable ObjectContexts actual, @Nullable ObjectContexts expected) { if (expected != null && !expected.getCustom().isEmpty() && actual != null) {