From e2f553788eb3f3685056728de75c358893887604 Mon Sep 17 00:00:00 2001 From: BenWhitehead Date: Thu, 15 Aug 2024 21:43:26 +0000 Subject: [PATCH] fix: update modified field handling for blob and bucket with json transport to properly clear fields (#2664) Update StorageImpl#update(BlobInfo) and StorageImpl#update(BucketInfo) to only send modified fields in the case of an actual update. Currently, it simply sends the json version of the current info, this can mean that if a field is cleared the request to gcs doesn't actually include the field to clear. This same issue does not impact grpc transport, because grpc transport has an explicit `update_mask` that is populated. Fixes #2662 --- .../google/cloud/storage/JsonConversions.java | 8 +- .../com/google/cloud/storage/Storage.java | 221 ++++++++++++------ .../com/google/cloud/storage/StorageImpl.java | 62 ++++- .../com/google/cloud/storage/UnifiedOpts.java | 27 +-- .../java/com/google/cloud/storage/Utils.java | 20 ++ .../cloud/storage/it/ITJsonPatchTest.java | 147 ++++++++++++ 6 files changed, 391 insertions(+), 94 deletions(-) create mode 100644 google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITJsonPatchTest.java diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/JsonConversions.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/JsonConversions.java index 7f84db77d0..274441436a 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/JsonConversions.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/JsonConversions.java @@ -83,6 +83,7 @@ import java.util.List; import java.util.Map; import java.util.Map.Entry; +import java.util.Objects; import java.util.function.Function; import java.util.stream.Collectors; @@ -1082,6 +1083,11 @@ private static Function, List> toListOf(Function f // various data level methods in the apiary model are hostile to ImmutableList, as it does not // provide a public default no args constructor. Instead, apiary uses ArrayList for all internal // representations of JSON Arrays. - return l -> l.stream().map(f).collect(Collectors.toList()); + return l -> { + if (l == null) { + return ImmutableList.of(); + } + return l.stream().filter(Objects::nonNull).map(f).collect(Collectors.toList()); + }; } } diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/Storage.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/Storage.java index fca1d1294d..89624d9b0a 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/Storage.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/Storage.java @@ -45,11 +45,13 @@ import com.google.cloud.storage.UnifiedOpts.HmacKeySourceOpt; import com.google.cloud.storage.UnifiedOpts.HmacKeyTargetOpt; import com.google.cloud.storage.UnifiedOpts.NamedField; +import com.google.cloud.storage.UnifiedOpts.NestedNamedField; import com.google.cloud.storage.UnifiedOpts.ObjectListOpt; import com.google.cloud.storage.UnifiedOpts.ObjectSourceOpt; import com.google.cloud.storage.UnifiedOpts.ObjectTargetOpt; import com.google.cloud.storage.UnifiedOpts.Opts; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.common.collect.Streams; @@ -62,9 +64,11 @@ import java.net.URLConnection; import java.nio.file.Path; import java.security.Key; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.HashMap; import java.util.LinkedHashSet; import java.util.LinkedList; import java.util.List; @@ -73,6 +77,7 @@ import java.util.concurrent.TimeUnit; import java.util.stream.Stream; import org.checkerframework.checker.nullness.qual.NonNull; +import org.checkerframework.checker.nullness.qual.Nullable; /** * An interface for Google Cloud Storage. @@ -106,80 +111,107 @@ String getEntry() { enum BucketField implements FieldSelector, NamedField { @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - ID("id", "bucket_id"), + ID("id", "bucket_id", String.class), @TransportCompatibility(Transport.HTTP) - SELF_LINK("selfLink"), + SELF_LINK("selfLink", String.class), @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - NAME("name"), + NAME("name", String.class), @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - TIME_CREATED("timeCreated", "create_time"), + TIME_CREATED("timeCreated", "create_time", com.google.api.client.util.DateTime.class), @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - METAGENERATION("metageneration"), + METAGENERATION("metageneration", Long.class), @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - ACL("acl"), + ACL("acl", ArrayList.class), @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - DEFAULT_OBJECT_ACL("defaultObjectAcl", "default_object_acl"), + DEFAULT_OBJECT_ACL("defaultObjectAcl", "default_object_acl", ArrayList.class), @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - OWNER("owner"), + OWNER("owner", com.google.api.services.storage.model.Bucket.Owner.class), @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - LABELS("labels"), + LABELS("labels", HashMap.class), @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - LOCATION("location"), + LOCATION("location", String.class), @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - LOCATION_TYPE("locationType", "location_type"), + LOCATION_TYPE("locationType", "location_type", String.class), @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - WEBSITE("website"), + WEBSITE("website", com.google.api.services.storage.model.Bucket.Website.class), @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - VERSIONING("versioning"), + VERSIONING("versioning", com.google.api.services.storage.model.Bucket.Versioning.class), @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - CORS("cors"), + CORS("cors", ArrayList.class), @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - LIFECYCLE("lifecycle"), + LIFECYCLE("lifecycle", com.google.api.services.storage.model.Bucket.Lifecycle.class), @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - STORAGE_CLASS("storageClass", "storage_class"), + STORAGE_CLASS("storageClass", "storage_class", String.class), @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - ETAG("etag"), + ETAG("etag", String.class), @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - ENCRYPTION("encryption"), + ENCRYPTION("encryption", com.google.api.services.storage.model.Bucket.Encryption.class), @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - BILLING("billing"), + BILLING("billing", com.google.api.services.storage.model.Bucket.Billing.class), @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - DEFAULT_EVENT_BASED_HOLD("defaultEventBasedHold", "default_event_based_hold"), + DEFAULT_EVENT_BASED_HOLD("defaultEventBasedHold", "default_event_based_hold", Boolean.class), @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - RETENTION_POLICY("retentionPolicy", "retention_policy"), + RETENTION_POLICY( + "retentionPolicy", + "retention_policy", + com.google.api.services.storage.model.Bucket.RetentionPolicy.class), @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - IAMCONFIGURATION("iamConfiguration", "iam_config"), + IAMCONFIGURATION( + "iamConfiguration", + "iam_config", + com.google.api.services.storage.model.Bucket.IamConfiguration.class), @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - LOGGING("logging"), + LOGGING("logging", com.google.api.services.storage.model.Bucket.Logging.class), @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - UPDATED("updated", "update_time"), + UPDATED("updated", "update_time", com.google.api.client.util.DateTime.class), @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - RPO("rpo"), + RPO("rpo", String.class), @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - CUSTOM_PLACEMENT_CONFIG("customPlacementConfig", "custom_placement_config"), + CUSTOM_PLACEMENT_CONFIG( + "customPlacementConfig", + "custom_placement_config", + com.google.api.services.storage.model.Bucket.CustomPlacementConfig.class), @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - AUTOCLASS("autoclass"), + AUTOCLASS("autoclass", com.google.api.services.storage.model.Bucket.Autoclass.class), @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - HIERARCHICAL_NAMESPACE("hierarchicalNamespace", "hierarchical_namespace"), + HIERARCHICAL_NAMESPACE( + "hierarchicalNamespace", + "hierarchical_namespace", + com.google.api.services.storage.model.Bucket.HierarchicalNamespace.class), @TransportCompatibility({Transport.HTTP}) - OBJECT_RETENTION("objectRetention"), + OBJECT_RETENTION( + "objectRetention", com.google.api.services.storage.model.Bucket.ObjectRetention.class), @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - SOFT_DELETE_POLICY("softDeletePolicy", "soft_delete_policy"); + SOFT_DELETE_POLICY( + "softDeletePolicy", + "soft_delete_policy", + com.google.api.services.storage.model.Bucket.SoftDeletePolicy.class); static final List REQUIRED_FIELDS = ImmutableList.of(NAME); + private static final Map JSON_FIELD_NAME_INDEX; + + static { + ImmutableMap.Builder tmp = ImmutableMap.builder(); + for (BucketField field : values()) { + tmp.put(field.selector, field); + } + JSON_FIELD_NAME_INDEX = Utils.mapBuild(tmp); + } private final String selector; private final String grpcFieldName; + private final Class jsonClass; - BucketField(String selector) { - this(selector, selector); + BucketField(String selector, Class jsonClass) { + this(selector, selector, jsonClass); } - BucketField(String selector, String grpcFieldName) { + BucketField(String selector, String grpcFieldName, Class jsonClass) { this.selector = selector; this.grpcFieldName = grpcFieldName; + this.jsonClass = jsonClass; } @Override @@ -196,96 +228,129 @@ public String getApiaryName() { public String getGrpcName() { return grpcFieldName; } + + Class getJsonClass() { + return jsonClass; + } + + @Nullable + static BucketField lookup(NamedField nf) { + NamedField lookup = nf; + if (nf instanceof NestedNamedField) { + NestedNamedField nested = (NestedNamedField) nf; + lookup = nested.getParent(); + } + return JSON_FIELD_NAME_INDEX.get(lookup.getApiaryName()); + } } enum BlobField implements FieldSelector, NamedField { @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - ACL("acl"), + ACL("acl", com.google.api.services.storage.model.ObjectAccessControl.class), @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - BUCKET("bucket"), + BUCKET("bucket", String.class), @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - CACHE_CONTROL("cacheControl", "cache_control"), + CACHE_CONTROL("cacheControl", "cache_control", String.class), @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - COMPONENT_COUNT("componentCount", "component_count"), + COMPONENT_COUNT("componentCount", "component_count", Integer.class), @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - CONTENT_DISPOSITION("contentDisposition", "content_disposition"), + CONTENT_DISPOSITION("contentDisposition", "content_disposition", String.class), @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - CONTENT_ENCODING("contentEncoding", "content_encoding"), + CONTENT_ENCODING("contentEncoding", "content_encoding", String.class), @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - CONTENT_LANGUAGE("contentLanguage", "content_language"), + CONTENT_LANGUAGE("contentLanguage", "content_language", String.class), @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - CONTENT_TYPE("contentType", "content_type"), + CONTENT_TYPE("contentType", "content_type", String.class), @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - CRC32C("crc32c", "checksums.crc32c"), + CRC32C("crc32c", "checksums.crc32c", String.class), @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - ETAG("etag"), + ETAG("etag", String.class), @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - GENERATION("generation"), + GENERATION("generation", Long.class), @TransportCompatibility(Transport.HTTP) - ID("id"), + ID("id", String.class), /** {@code kind} is not exposed in {@link BlobInfo} or {@link Blob} no need to select it */ @Deprecated @TransportCompatibility(Transport.HTTP) - KIND("kind"), + KIND("kind", String.class), @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - MD5HASH("md5Hash", "checksums.md5_hash"), + MD5HASH("md5Hash", "checksums.md5_hash", String.class), @TransportCompatibility(Transport.HTTP) - MEDIA_LINK("mediaLink"), + MEDIA_LINK("mediaLink", String.class), @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - METADATA("metadata"), + METADATA("metadata", HashMap.class), @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - METAGENERATION("metageneration"), + METAGENERATION("metageneration", Long.class), @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - NAME("name"), + NAME("name", String.class), @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - OWNER("owner"), + OWNER("owner", com.google.api.services.storage.model.StorageObject.Owner.class), @TransportCompatibility(Transport.HTTP) - SELF_LINK("selfLink"), + SELF_LINK("selfLink", String.class), @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - SIZE("size"), + SIZE("size", java.math.BigInteger.class), @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - STORAGE_CLASS("storageClass", "storage_class"), + STORAGE_CLASS("storageClass", "storage_class", String.class), @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - TIME_DELETED("timeDeleted", "delete_time"), + TIME_DELETED("timeDeleted", "delete_time", com.google.api.client.util.DateTime.class), @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - TIME_CREATED("timeCreated", "create_time"), + TIME_CREATED("timeCreated", "create_time", com.google.api.client.util.DateTime.class), @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - KMS_KEY_NAME("kmsKeyName", "kms_key"), + KMS_KEY_NAME("kmsKeyName", "kms_key", String.class), @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - EVENT_BASED_HOLD("eventBasedHold", "event_based_hold"), + EVENT_BASED_HOLD("eventBasedHold", "event_based_hold", String.class), @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - TEMPORARY_HOLD("temporaryHold", "temporary_hold"), + TEMPORARY_HOLD("temporaryHold", "temporary_hold", String.class), @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - RETENTION_EXPIRATION_TIME("retentionExpirationTime", "retention_expire_time"), + RETENTION_EXPIRATION_TIME( + "retentionExpirationTime", + "retention_expire_time", + com.google.api.client.util.DateTime.class), @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - UPDATED("updated", "update_time"), + UPDATED("updated", "update_time", com.google.api.client.util.DateTime.class), @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - CUSTOM_TIME("customTime", "custom_time"), + CUSTOM_TIME("customTime", "custom_time", com.google.api.client.util.DateTime.class), @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - TIME_STORAGE_CLASS_UPDATED("timeStorageClassUpdated", "update_storage_class_time"), + TIME_STORAGE_CLASS_UPDATED( + "timeStorageClassUpdated", + "update_storage_class_time", + com.google.api.client.util.DateTime.class), @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - CUSTOMER_ENCRYPTION("customerEncryption", "customer_encryption"), + CUSTOMER_ENCRYPTION("customerEncryption", "customer_encryption", String.class), @TransportCompatibility({Transport.HTTP}) - RETENTION("retention"), + RETENTION("retention", com.google.api.services.storage.model.StorageObject.Retention.class), @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - SOFT_DELETE_TIME("softDeleteTime", "soft_delete_time"), + SOFT_DELETE_TIME( + "softDeleteTime", "soft_delete_time", com.google.api.client.util.DateTime.class), @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - HARD_DELETE_TIME("hardDeleteTime", "hard_delete_time"); + HARD_DELETE_TIME( + "hardDeleteTime", "hard_delete_time", com.google.api.client.util.DateTime.class); static final List REQUIRED_FIELDS = ImmutableList.of(BUCKET, NAME); + private static final Map JSON_FIELD_NAME_INDEX; + + static { + ImmutableMap.Builder tmp = ImmutableMap.builder(); + for (BlobField field : values()) { + tmp.put(field.selector, field); + } + JSON_FIELD_NAME_INDEX = Utils.mapBuild(tmp); + } private final String selector; private final String grpcFieldName; + private final Class jsonClass; - BlobField(String selector) { - this(selector, selector); + BlobField(String selector, Class jsonClass) { + this(selector, selector, jsonClass); } - BlobField(String selector, String grpcFieldName) { + BlobField(String selector, String grpcFieldName, Class jsonClass) { this.selector = selector; this.grpcFieldName = grpcFieldName; + this.jsonClass = jsonClass; } @Override @@ -302,6 +367,20 @@ public String getApiaryName() { public String getGrpcName() { return grpcFieldName; } + + Class getJsonClass() { + return jsonClass; + } + + @Nullable + static BlobField lookup(NamedField nf) { + NamedField lookup = nf; + if (nf instanceof NestedNamedField) { + NestedNamedField nested = (NestedNamedField) nf; + lookup = nested.getParent(); + } + return JSON_FIELD_NAME_INDEX.get(lookup.getApiaryName()); + } } enum UriScheme { diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageImpl.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageImpl.java index 5884a51e52..c01742b860 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageImpl.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageImpl.java @@ -23,6 +23,7 @@ import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.concurrent.Executors.callable; +import com.google.api.client.util.Data; import com.google.api.core.ApiFuture; import com.google.api.gax.paging.Page; import com.google.api.gax.retrying.ResultRetryAlgorithm; @@ -44,6 +45,8 @@ import com.google.cloud.storage.PostPolicyV4.PostConditionsV4; import com.google.cloud.storage.PostPolicyV4.PostFieldsV4; import com.google.cloud.storage.PostPolicyV4.PostPolicyV4Document; +import com.google.cloud.storage.UnifiedOpts.NamedField; +import com.google.cloud.storage.UnifiedOpts.NestedNamedField; import com.google.cloud.storage.UnifiedOpts.ObjectSourceOpt; import com.google.cloud.storage.UnifiedOpts.ObjectTargetOpt; import com.google.cloud.storage.UnifiedOpts.Opts; @@ -91,6 +94,7 @@ import java.util.concurrent.TimeoutException; import java.util.function.Function; import java.util.function.Supplier; +import java.util.stream.Stream; import org.checkerframework.checker.nullness.qual.NonNull; import org.checkerframework.checker.nullness.qual.Nullable; @@ -497,11 +501,35 @@ private static Page listBlobs( public Bucket update(BucketInfo bucketInfo, BucketTargetOption... options) { Map optionsMap = Opts.unwrap(options).resolveFrom(bucketInfo).getRpcOptions(); - if (bucketInfo.getModifiedFields().isEmpty()) { + ImmutableSet modifiedFields = bucketInfo.getModifiedFields(); + if (modifiedFields.isEmpty()) { return internalBucketGet(bucketInfo.getName(), optionsMap); } else { + com.google.api.services.storage.model.Bucket tmp = codecs.bucketInfo().encode(bucketInfo); com.google.api.services.storage.model.Bucket bucketPb = - codecs.bucketInfo().encode(bucketInfo); + new com.google.api.services.storage.model.Bucket(); + Stream.concat(modifiedFields.stream(), BucketField.REQUIRED_FIELDS.stream()) + .map( + f -> { + if (f instanceof NestedNamedField) { + return ((NestedNamedField) f).getParent(); + } else { + return f; + } + }) + .forEach( + field -> { + String jsonName = field.getApiaryName(); + if (tmp.containsKey(jsonName)) { + bucketPb.put(jsonName, tmp.get(jsonName)); + } else { + BucketField lookup = BucketField.lookup(field); + if (lookup != null) { + bucketPb.put(jsonName, Data.nullOf(lookup.getJsonClass())); + } + } + }); + ResultRetryAlgorithm algorithm = retryAlgorithmManager.getForBucketsUpdate(bucketPb, optionsMap); return run( @@ -515,7 +543,8 @@ public Bucket update(BucketInfo bucketInfo, BucketTargetOption... options) { public Blob update(BlobInfo blobInfo, BlobTargetOption... options) { Opts opts = Opts.unwrap(options).resolveFrom(blobInfo); Map optionsMap = opts.getRpcOptions(); - boolean unmodifiedBeforeOpts = blobInfo.getModifiedFields().isEmpty(); + ImmutableSet modifiedFields = blobInfo.getModifiedFields(); + boolean unmodifiedBeforeOpts = modifiedFields.isEmpty(); BlobInfo.Builder builder = blobInfo.toBuilder(); // This is a workaround until everything is in prod for both json and grpc. @@ -523,7 +552,7 @@ public Blob update(BlobInfo blobInfo, BlobTargetOption... options) { // request if it was modified, so that we don't send a null object in a // grpc or json request. // todo: b/308194853 - if (blobInfo.getModifiedFields().contains(BlobField.RETENTION)) { + if (modifiedFields.contains(BlobField.RETENTION)) { builder.setRetention(blobInfo.getRetention()); } BlobInfo updated = opts.blobInfoMapper().apply(builder).build(); @@ -531,7 +560,30 @@ public Blob update(BlobInfo blobInfo, BlobTargetOption... options) { if (unmodifiedBeforeOpts && unmodifiedAfterOpts) { return internalGetBlob(blobInfo.getBlobId(), optionsMap); } else { - StorageObject pb = codecs.blobInfo().encode(updated); + StorageObject tmp = codecs.blobInfo().encode(updated); + StorageObject pb = new StorageObject(); + Stream.concat(modifiedFields.stream(), BlobField.REQUIRED_FIELDS.stream()) + .map( + f -> { + if (f instanceof NestedNamedField) { + return ((NestedNamedField) f).getParent(); + } else { + return f; + } + }) + .forEach( + field -> { + String jsonName = field.getApiaryName(); + if (tmp.containsKey(jsonName)) { + pb.put(jsonName, tmp.get(jsonName)); + } else { + BlobField lookup = BlobField.lookup(field); + if (lookup != null) { + pb.put(jsonName, Data.nullOf(lookup.getJsonClass())); + } + } + }); + ResultRetryAlgorithm algorithm = retryAlgorithmManager.getForObjectsUpdate(pb, optionsMap); return run( algorithm, diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/UnifiedOpts.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/UnifiedOpts.java index 16c76ef926..15476cfcf0 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/UnifiedOpts.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/UnifiedOpts.java @@ -2301,21 +2301,6 @@ public String toString() { */ @SuppressWarnings("unchecked") static final class Opts { - private static final Function, ImmutableMap> mapBuild; - - static { - Function, ImmutableMap> tmp; - // buildOrThrow was added in guava 31.0 - // if it fails, fallback to the older build() method instead. - // The behavior was the same, but the new name makes the behavior clear - try { - ImmutableMap.builder().buildOrThrow(); - tmp = ImmutableMap.Builder::buildOrThrow; - } catch (NoSuchMethodError e) { - tmp = ImmutableMap.Builder::build; - } - mapBuild = tmp; - } private final ImmutableList opts; @@ -2402,7 +2387,7 @@ Opts projectAsSource() { ImmutableMap getRpcOptions() { ImmutableMap.Builder builder = rpcOptionMapper().apply(ImmutableMap.builder()); - return (ImmutableMap) mapBuild.apply(builder); + return Utils.mapBuild(builder); } Mapper grpcMetadataMapper() { @@ -2798,7 +2783,7 @@ public String toString() { } } - private static final class NestedNamedField implements NamedField { + static final class NestedNamedField implements NamedField { private static long serialVersionUID = -7623005572810688221L; private final NamedField parent; private final NamedField child; @@ -2818,6 +2803,14 @@ public String getGrpcName() { return parent.getGrpcName() + "." + child.getGrpcName(); } + NamedField getParent() { + return parent; + } + + NamedField getChild() { + return child; + } + @Override public boolean equals(Object o) { if (this == o) { diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/Utils.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/Utils.java index c0374d3f06..6359ec5d13 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/Utils.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/Utils.java @@ -26,6 +26,7 @@ import com.google.cloud.storage.UnifiedOpts.NamedField; import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.MapDifference; import com.google.common.collect.Maps; import com.google.common.io.BaseEncoding; @@ -56,6 +57,21 @@ */ @InternalApi final class Utils { + private static final Function, ImmutableMap> mapBuild; + + static { + Function, ImmutableMap> tmp; + // buildOrThrow was added in guava 31.0 + // if it fails, fallback to the older build() method instead. + // The behavior was the same, but the new name makes the behavior clear + try { + ImmutableMap.builder().buildOrThrow(); + tmp = ImmutableMap.Builder::buildOrThrow; + } catch (NoSuchMethodError e) { + tmp = ImmutableMap.Builder::build; + } + mapBuild = tmp; + } static final DateTimeFormatter RFC_3339_DATE_TIME_FORMATTER = DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss.SSSXXX"); @@ -319,4 +335,8 @@ static ImmutableList nullSafeList(@Nullable T t) { return ImmutableList.of(t); } } + + static ImmutableMap mapBuild(ImmutableMap.Builder b) { + return (ImmutableMap) mapBuild.apply(b); + } } diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITJsonPatchTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITJsonPatchTest.java new file mode 100644 index 0000000000..bb2bdfe691 --- /dev/null +++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITJsonPatchTest.java @@ -0,0 +1,147 @@ +/* + * Copyright 2024 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.cloud.storage.it; + +import static com.google.cloud.storage.TestUtils.assertAll; +import static com.google.common.truth.Truth.assertThat; + +import com.google.cloud.storage.Blob; +import com.google.cloud.storage.BlobInfo; +import com.google.cloud.storage.Bucket; +import com.google.cloud.storage.BucketInfo; +import com.google.cloud.storage.BucketInfo.LifecycleRule; +import com.google.cloud.storage.BucketInfo.LifecycleRule.LifecycleAction; +import com.google.cloud.storage.BucketInfo.LifecycleRule.LifecycleCondition; +import com.google.cloud.storage.Cors; +import com.google.cloud.storage.Cors.Origin; +import com.google.cloud.storage.HttpMethod; +import com.google.cloud.storage.Storage; +import com.google.cloud.storage.Storage.BlobTargetOption; +import com.google.cloud.storage.Storage.BucketTargetOption; +import com.google.cloud.storage.TransportCompatibility.Transport; +import com.google.cloud.storage.it.runner.StorageITRunner; +import com.google.cloud.storage.it.runner.annotations.Backend; +import com.google.cloud.storage.it.runner.annotations.CrossRun; +import com.google.cloud.storage.it.runner.annotations.Inject; +import com.google.cloud.storage.it.runner.registry.Generator; +import com.google.common.collect.ImmutableList; +import org.junit.Test; +import org.junit.runner.RunWith; + +@RunWith(StorageITRunner.class) +@CrossRun( + backends = {Backend.PROD}, + transports = {Transport.HTTP, Transport.GRPC}) +public final class ITJsonPatchTest { + + @Inject public Storage storage; + @Inject public BucketInfo bucket; + @Inject public Generator generator; + + @Test + public void object() throws Exception { + String cacheControl = "max-age=60"; + String contentDisposition = "attachment"; + String contentEncoding = "identity"; + String contentLanguage = "en-US"; + String contentType = "text/plain"; + BlobInfo info = + BlobInfo.newBuilder(bucket, generator.randomObjectName()) + .setCacheControl(cacheControl) + .setContentDisposition(contentDisposition) + .setContentEncoding(contentEncoding) + .setContentLanguage(contentLanguage) + .setContentType(contentType) + .build(); + + Blob gen1 = storage.create(info); + assertAll( + () -> assertThat(gen1.getCacheControl()).isEqualTo(cacheControl), + () -> assertThat(gen1.getContentDisposition()).isEqualTo(contentDisposition), + () -> assertThat(gen1.getContentEncoding()).isEqualTo(contentEncoding), + () -> assertThat(gen1.getContentLanguage()).isEqualTo(contentLanguage), + () -> assertThat(gen1.getContentType()).isEqualTo(contentType)); + BlobInfo update = + gen1.toBuilder() + .setCacheControl(null) + .setContentDisposition(null) + .setContentEncoding(null) + .setContentLanguage(null) + .setContentType(null) + .build(); + Blob gen2 = + storage.update(update, BlobTargetOption.metagenerationMatch(gen1.getMetageneration())); + assertAll( + () -> assertThat(gen2.getCacheControl()).isAnyOf("", null), + () -> assertThat(gen2.getContentDisposition()).isAnyOf("", null), + () -> assertThat(gen2.getContentEncoding()).isAnyOf("", null), + () -> assertThat(gen2.getContentLanguage()).isAnyOf("", null), + () -> assertThat(gen2.getContentType()).isAnyOf("", null)); + } + + @Test + public void bucket() throws Exception { + ImmutableList lifecycleRules = + ImmutableList.of( + new LifecycleRule( + LifecycleAction.newDeleteAction(), + LifecycleCondition.newBuilder() + .setMatchesPrefix(ImmutableList.of("blahblahblah")) + .build())); + ImmutableList cors = + ImmutableList.of( + Cors.newBuilder() + .setMaxAgeSeconds(300) + .setMethods(ImmutableList.of(HttpMethod.GET)) + .setOrigins(ImmutableList.of(Origin.any())) + .setResponseHeaders(ImmutableList.of("blah2blah")) + .build()); + String indexPage = "index.html"; + String notFoundPage = "404.html"; + BucketInfo info = + BucketInfo.newBuilder(generator.randomBucketName()) + .setLifecycleRules(lifecycleRules) + .setCors(cors) + .setIndexPage(indexPage) + .setNotFoundPage(notFoundPage) + .build(); + + try (TemporaryBucket tmpBucket = + TemporaryBucket.newBuilder().setBucketInfo(info).setStorage(storage).build()) { + BucketInfo gen1 = tmpBucket.getBucket(); + + assertAll( + () -> assertThat(gen1.getLifecycleRules()).isEqualTo(lifecycleRules), + () -> assertThat(gen1.getCors()).isEqualTo(cors), + () -> assertThat(gen1.getIndexPage()).isEqualTo(indexPage), + () -> assertThat(gen1.getNotFoundPage()).isEqualTo(notFoundPage)); + BucketInfo update = + gen1.toBuilder() + .setLifecycleRules(ImmutableList.of()) + .setCors(ImmutableList.of()) + .setIndexPage(null) + .setNotFoundPage(null) + .build(); + Bucket gen2 = storage.update(update, BucketTargetOption.metagenerationMatch()); + assertAll( + () -> assertThat(gen2.getLifecycleRules()).isAnyOf(ImmutableList.of(), null), + () -> assertThat(gen2.getCors()).isAnyOf(ImmutableList.of(), null), + () -> assertThat(gen2.getIndexPage()).isAnyOf("", null), + () -> assertThat(gen2.getNotFoundPage()).isAnyOf("", null)); + } + } +}