Skip to content

Commit

Permalink
fix: update modified field handling for blob and bucket with json tra…
Browse files Browse the repository at this point in the history 
…nsport to properly clear fields

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
  • Loading branch information
@BenWhitehead
BenWhitehead committed Aug 13, 2024
1 parent 48e7a4c commit bae9bb9
Show file tree
Hide file tree
Showing 6 changed files with 391 additions and 94 deletions.
8 changes: 7 additions & 1 deletion google-cloud-storage/src/main/java/com/google/cloud/storage/JsonConversions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -1082,6 +1083,11 @@ private static <T1, T2> Function<List<T1>, List<T2>> toListOf(Function<T1, T2> 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());
};
}
}
221 changes: 150 additions & 71 deletions google-cloud-storage/src/main/java/com/google/cloud/storage/Storage.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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.
Expand Down Expand Up @@ -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<BucketField> REQUIRED_FIELDS = ImmutableList.of(NAME);
private static final Map<String, BucketField> JSON_FIELD_NAME_INDEX;

static {
ImmutableMap.Builder<String, BucketField> 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
Expand All @@ -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<NamedField> REQUIRED_FIELDS = ImmutableList.of(BUCKET, NAME);
private static final Map<String, BlobField> JSON_FIELD_NAME_INDEX;

static {
ImmutableMap.Builder<String, BlobField> 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
Expand All @@ -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 {
Expand Down
Loading

0 comments on commit bae9bb9

Please sign in to comment.