Skip to content

fix(storage): handle map clears without current state #13725

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mahitha-ada wants to merge 1 commit into
base: main
Choose a base branch
from
+98 −10
Open

fix(storage): handle map clears without current state #13725

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 16 additions & 8 deletions java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/BlobInfo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<NamedField> 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;
Expand Down Expand Up @@ -1080,8 +1083,13 @@ Builder setMediaLink(String mediaLink) {
public Builder setMetadata(@Nullable Map<@NonNull String, @Nullable String> metadata) {
Map<String, String> left = this.metadata;
Map<String, String> 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 {
Expand Down Expand Up @@ -1275,18 +1283,18 @@ public Builder setContexts(ObjectContexts contexts) {
this.contexts == null ? null : this.contexts.getCustom();
Map<String, ObjectCustomContextPayload> 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;
}
Expand Down
12 changes: 10 additions & 2 deletions java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/BucketInfo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<NamedField> 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;
Expand Down Expand Up @@ -3061,8 +3064,13 @@ public Builder setDefaultAcl(Iterable<Acl> acl) {
public Builder setLabels(@Nullable Map<@NonNull String, @Nullable String> labels) {
Map<String, String> left = this.labels;
Map<String, String> 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 {
Expand Down
72 changes: 72 additions & 0 deletions ...oogle-cloud-storage/src/test/java/com/google/cloud/storage/it/ITNestedUpdateMaskTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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) {
Expand Down
Loading