Skip to content

Commit e2f5537

Browse files
BenWhiteheadBenWhitehead
authored
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
1 parent a173c80 commit e2f5537

File tree

6 files changed

+391
-94
lines changed

6 files changed

+391
-94
lines changed

‎google-cloud-storage/src/main/java/com/google/cloud/storage/JsonConversions.java

+7-1
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@
8383
import java.util.List;
8484
import java.util.Map;
8585
import java.util.Map.Entry;
86+
import java.util.Objects;
8687
import java.util.function.Function;
8788
import java.util.stream.Collectors;
8889

@@ -1082,6 +1083,11 @@ private static <T1, T2> Function<List<T1>, List<T2>> toListOf(Function<T1, T2> f
10821083
// various data level methods in the apiary model are hostile to ImmutableList, as it does not
10831084
// provide a public default no args constructor. Instead, apiary uses ArrayList for all internal
10841085
// representations of JSON Arrays.
1085-
return l -> l.stream().map(f).collect(Collectors.toList());
1086+
return l -> {
1087+
if (l == null) {
1088+
return ImmutableList.of();
1089+
}
1090+
return l.stream().filter(Objects::nonNull).map(f).collect(Collectors.toList());
1091+
};
10861092
}
10871093
}

‎google-cloud-storage/src/main/java/com/google/cloud/storage/Storage.java

+150-71
Original file line numberDiff line numberDiff line change
@@ -45,11 +45,13 @@
4545
import com.google.cloud.storage.UnifiedOpts.HmacKeySourceOpt;
4646
import com.google.cloud.storage.UnifiedOpts.HmacKeyTargetOpt;
4747
import com.google.cloud.storage.UnifiedOpts.NamedField;
48+
import com.google.cloud.storage.UnifiedOpts.NestedNamedField;
4849
import com.google.cloud.storage.UnifiedOpts.ObjectListOpt;
4950
import com.google.cloud.storage.UnifiedOpts.ObjectSourceOpt;
5051
import com.google.cloud.storage.UnifiedOpts.ObjectTargetOpt;
5152
import com.google.cloud.storage.UnifiedOpts.Opts;
5253
import com.google.common.collect.ImmutableList;
54+
import com.google.common.collect.ImmutableMap;
5355
import com.google.common.collect.ImmutableSet;
5456
import com.google.common.collect.Iterables;
5557
import com.google.common.collect.Streams;
@@ -62,9 +64,11 @@
6264
import java.net.URLConnection;
6365
import java.nio.file.Path;
6466
import java.security.Key;
67+
import java.util.ArrayList;
6568
import java.util.Arrays;
6669
import java.util.Collection;
6770
import java.util.Collections;
71+
import java.util.HashMap;
6872
import java.util.LinkedHashSet;
6973
import java.util.LinkedList;
7074
import java.util.List;
@@ -73,6 +77,7 @@
7377
import java.util.concurrent.TimeUnit;
7478
import java.util.stream.Stream;
7579
import org.checkerframework.checker.nullness.qual.NonNull;
80+
import org.checkerframework.checker.nullness.qual.Nullable;
7681

7782
/**
7883
* An interface for Google Cloud Storage.
@@ -106,80 +111,107 @@ String getEntry() {
106111

107112
enum BucketField implements FieldSelector, NamedField {
108113
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
109-
ID("id", "bucket_id"),
114+
ID("id", "bucket_id", String.class),
110115
@TransportCompatibility(Transport.HTTP)
111-
SELF_LINK("selfLink"),
116+
SELF_LINK("selfLink", String.class),
112117
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
113-
NAME("name"),
118+
NAME("name", String.class),
114119
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
115-
TIME_CREATED("timeCreated", "create_time"),
120+
TIME_CREATED("timeCreated", "create_time", com.google.api.client.util.DateTime.class),
116121
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
117-
METAGENERATION("metageneration"),
122+
METAGENERATION("metageneration", Long.class),
118123
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
119-
ACL("acl"),
124+
ACL("acl", ArrayList.class),
120125
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
121-
DEFAULT_OBJECT_ACL("defaultObjectAcl", "default_object_acl"),
126+
DEFAULT_OBJECT_ACL("defaultObjectAcl", "default_object_acl", ArrayList.class),
122127
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
123-
OWNER("owner"),
128+
OWNER("owner", com.google.api.services.storage.model.Bucket.Owner.class),
124129
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
125-
LABELS("labels"),
130+
LABELS("labels", HashMap.class),
126131
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
127-
LOCATION("location"),
132+
LOCATION("location", String.class),
128133
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
129-
LOCATION_TYPE("locationType", "location_type"),
134+
LOCATION_TYPE("locationType", "location_type", String.class),
130135
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
131-
WEBSITE("website"),
136+
WEBSITE("website", com.google.api.services.storage.model.Bucket.Website.class),
132137
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
133-
VERSIONING("versioning"),
138+
VERSIONING("versioning", com.google.api.services.storage.model.Bucket.Versioning.class),
134139
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
135-
CORS("cors"),
140+
CORS("cors", ArrayList.class),
136141
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
137-
LIFECYCLE("lifecycle"),
142+
LIFECYCLE("lifecycle", com.google.api.services.storage.model.Bucket.Lifecycle.class),
138143
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
139-
STORAGE_CLASS("storageClass", "storage_class"),
144+
STORAGE_CLASS("storageClass", "storage_class", String.class),
140145
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
141-
ETAG("etag"),
146+
ETAG("etag", String.class),
142147
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
143-
ENCRYPTION("encryption"),
148+
ENCRYPTION("encryption", com.google.api.services.storage.model.Bucket.Encryption.class),
144149
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
145-
BILLING("billing"),
150+
BILLING("billing", com.google.api.services.storage.model.Bucket.Billing.class),
146151
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
147-
DEFAULT_EVENT_BASED_HOLD("defaultEventBasedHold", "default_event_based_hold"),
152+
DEFAULT_EVENT_BASED_HOLD("defaultEventBasedHold", "default_event_based_hold", Boolean.class),
148153
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
149-
RETENTION_POLICY("retentionPolicy", "retention_policy"),
154+
RETENTION_POLICY(
155+
"retentionPolicy",
156+
"retention_policy",
157+
com.google.api.services.storage.model.Bucket.RetentionPolicy.class),
150158
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
151-
IAMCONFIGURATION("iamConfiguration", "iam_config"),
159+
IAMCONFIGURATION(
160+
"iamConfiguration",
161+
"iam_config",
162+
com.google.api.services.storage.model.Bucket.IamConfiguration.class),
152163
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
153-
LOGGING("logging"),
164+
LOGGING("logging", com.google.api.services.storage.model.Bucket.Logging.class),
154165
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
155-
UPDATED("updated", "update_time"),
166+
UPDATED("updated", "update_time", com.google.api.client.util.DateTime.class),
156167
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
157-
RPO("rpo"),
168+
RPO("rpo", String.class),
158169
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
159-
CUSTOM_PLACEMENT_CONFIG("customPlacementConfig", "custom_placement_config"),
170+
CUSTOM_PLACEMENT_CONFIG(
171+
"customPlacementConfig",
172+
"custom_placement_config",
173+
com.google.api.services.storage.model.Bucket.CustomPlacementConfig.class),
160174
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
161-
AUTOCLASS("autoclass"),
175+
AUTOCLASS("autoclass", com.google.api.services.storage.model.Bucket.Autoclass.class),
162176

163177
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
164-
HIERARCHICAL_NAMESPACE("hierarchicalNamespace", "hierarchical_namespace"),
178+
HIERARCHICAL_NAMESPACE(
179+
"hierarchicalNamespace",
180+
"hierarchical_namespace",
181+
com.google.api.services.storage.model.Bucket.HierarchicalNamespace.class),
165182
@TransportCompatibility({Transport.HTTP})
166-
OBJECT_RETENTION("objectRetention"),
183+
OBJECT_RETENTION(
184+
"objectRetention", com.google.api.services.storage.model.Bucket.ObjectRetention.class),
167185

168186
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
169-
SOFT_DELETE_POLICY("softDeletePolicy", "soft_delete_policy");
187+
SOFT_DELETE_POLICY(
188+
"softDeletePolicy",
189+
"soft_delete_policy",
190+
com.google.api.services.storage.model.Bucket.SoftDeletePolicy.class);
170191

171192
static final List<BucketField> REQUIRED_FIELDS = ImmutableList.of(NAME);
193+
private static final Map<String, BucketField> JSON_FIELD_NAME_INDEX;
194+
195+
static {
196+
ImmutableMap.Builder<String, BucketField> tmp = ImmutableMap.builder();
197+
for (BucketField field : values()) {
198+
tmp.put(field.selector, field);
199+
}
200+
JSON_FIELD_NAME_INDEX = Utils.mapBuild(tmp);
201+
}
172202

173203
private final String selector;
174204
private final String grpcFieldName;
205+
private final Class<?> jsonClass;
175206

176-
BucketField(String selector) {
177-
this(selector, selector);
207+
BucketField(String selector, Class<?> jsonClass) {
208+
this(selector, selector, jsonClass);
178209
}
179210

180-
BucketField(String selector, String grpcFieldName) {
211+
BucketField(String selector, String grpcFieldName, Class<?> jsonClass) {
181212
this.selector = selector;
182213
this.grpcFieldName = grpcFieldName;
214+
this.jsonClass = jsonClass;
183215
}
184216

185217
@Override
@@ -196,96 +228,129 @@ public String getApiaryName() {
196228
public String getGrpcName() {
197229
return grpcFieldName;
198230
}
231+
232+
Class<?> getJsonClass() {
233+
return jsonClass;
234+
}
235+
236+
@Nullable
237+
static BucketField lookup(NamedField nf) {
238+
NamedField lookup = nf;
239+
if (nf instanceof NestedNamedField) {
240+
NestedNamedField nested = (NestedNamedField) nf;
241+
lookup = nested.getParent();
242+
}
243+
return JSON_FIELD_NAME_INDEX.get(lookup.getApiaryName());
244+
}
199245
}
200246

201247
enum BlobField implements FieldSelector, NamedField {
202248
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
203-
ACL("acl"),
249+
ACL("acl", com.google.api.services.storage.model.ObjectAccessControl.class),
204250
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
205-
BUCKET("bucket"),
251+
BUCKET("bucket", String.class),
206252
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
207-
CACHE_CONTROL("cacheControl", "cache_control"),
253+
CACHE_CONTROL("cacheControl", "cache_control", String.class),
208254
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
209-
COMPONENT_COUNT("componentCount", "component_count"),
255+
COMPONENT_COUNT("componentCount", "component_count", Integer.class),
210256
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
211-
CONTENT_DISPOSITION("contentDisposition", "content_disposition"),
257+
CONTENT_DISPOSITION("contentDisposition", "content_disposition", String.class),
212258
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
213-
CONTENT_ENCODING("contentEncoding", "content_encoding"),
259+
CONTENT_ENCODING("contentEncoding", "content_encoding", String.class),
214260
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
215-
CONTENT_LANGUAGE("contentLanguage", "content_language"),
261+
CONTENT_LANGUAGE("contentLanguage", "content_language", String.class),
216262
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
217-
CONTENT_TYPE("contentType", "content_type"),
263+
CONTENT_TYPE("contentType", "content_type", String.class),
218264
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
219-
CRC32C("crc32c", "checksums.crc32c"),
265+
CRC32C("crc32c", "checksums.crc32c", String.class),
220266
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
221-
ETAG("etag"),
267+
ETAG("etag", String.class),
222268
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
223-
GENERATION("generation"),
269+
GENERATION("generation", Long.class),
224270
@TransportCompatibility(Transport.HTTP)
225-
ID("id"),
271+
ID("id", String.class),
226272
/** {@code kind} is not exposed in {@link BlobInfo} or {@link Blob} no need to select it */
227273
@Deprecated
228274
@TransportCompatibility(Transport.HTTP)
229-
KIND("kind"),
275+
KIND("kind", String.class),
230276
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
231-
MD5HASH("md5Hash", "checksums.md5_hash"),
277+
MD5HASH("md5Hash", "checksums.md5_hash", String.class),
232278
@TransportCompatibility(Transport.HTTP)
233-
MEDIA_LINK("mediaLink"),
279+
MEDIA_LINK("mediaLink", String.class),
234280
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
235-
METADATA("metadata"),
281+
METADATA("metadata", HashMap.class),
236282
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
237-
METAGENERATION("metageneration"),
283+
METAGENERATION("metageneration", Long.class),
238284
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
239-
NAME("name"),
285+
NAME("name", String.class),
240286
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
241-
OWNER("owner"),
287+
OWNER("owner", com.google.api.services.storage.model.StorageObject.Owner.class),
242288
@TransportCompatibility(Transport.HTTP)
243-
SELF_LINK("selfLink"),
289+
SELF_LINK("selfLink", String.class),
244290
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
245-
SIZE("size"),
291+
SIZE("size", java.math.BigInteger.class),
246292
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
247-
STORAGE_CLASS("storageClass", "storage_class"),
293+
STORAGE_CLASS("storageClass", "storage_class", String.class),
248294
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
249-
TIME_DELETED("timeDeleted", "delete_time"),
295+
TIME_DELETED("timeDeleted", "delete_time", com.google.api.client.util.DateTime.class),
250296
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
251-
TIME_CREATED("timeCreated", "create_time"),
297+
TIME_CREATED("timeCreated", "create_time", com.google.api.client.util.DateTime.class),
252298
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
253-
KMS_KEY_NAME("kmsKeyName", "kms_key"),
299+
KMS_KEY_NAME("kmsKeyName", "kms_key", String.class),
254300
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
255-
EVENT_BASED_HOLD("eventBasedHold", "event_based_hold"),
301+
EVENT_BASED_HOLD("eventBasedHold", "event_based_hold", String.class),
256302
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
257-
TEMPORARY_HOLD("temporaryHold", "temporary_hold"),
303+
TEMPORARY_HOLD("temporaryHold", "temporary_hold", String.class),
258304
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
259-
RETENTION_EXPIRATION_TIME("retentionExpirationTime", "retention_expire_time"),
305+
RETENTION_EXPIRATION_TIME(
306+
"retentionExpirationTime",
307+
"retention_expire_time",
308+
com.google.api.client.util.DateTime.class),
260309
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
261-
UPDATED("updated", "update_time"),
310+
UPDATED("updated", "update_time", com.google.api.client.util.DateTime.class),
262311
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
263-
CUSTOM_TIME("customTime", "custom_time"),
312+
CUSTOM_TIME("customTime", "custom_time", com.google.api.client.util.DateTime.class),
264313
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
265-
TIME_STORAGE_CLASS_UPDATED("timeStorageClassUpdated", "update_storage_class_time"),
314+
TIME_STORAGE_CLASS_UPDATED(
315+
"timeStorageClassUpdated",
316+
"update_storage_class_time",
317+
com.google.api.client.util.DateTime.class),
266318
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
267-
CUSTOMER_ENCRYPTION("customerEncryption", "customer_encryption"),
319+
CUSTOMER_ENCRYPTION("customerEncryption", "customer_encryption", String.class),
268320
@TransportCompatibility({Transport.HTTP})
269-
RETENTION("retention"),
321+
RETENTION("retention", com.google.api.services.storage.model.StorageObject.Retention.class),
270322

271323
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
272-
SOFT_DELETE_TIME("softDeleteTime", "soft_delete_time"),
324+
SOFT_DELETE_TIME(
325+
"softDeleteTime", "soft_delete_time", com.google.api.client.util.DateTime.class),
273326

274327
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
275-
HARD_DELETE_TIME("hardDeleteTime", "hard_delete_time");
328+
HARD_DELETE_TIME(
329+
"hardDeleteTime", "hard_delete_time", com.google.api.client.util.DateTime.class);
276330

277331
static final List<NamedField> REQUIRED_FIELDS = ImmutableList.of(BUCKET, NAME);
332+
private static final Map<String, BlobField> JSON_FIELD_NAME_INDEX;
333+
334+
static {
335+
ImmutableMap.Builder<String, BlobField> tmp = ImmutableMap.builder();
336+
for (BlobField field : values()) {
337+
tmp.put(field.selector, field);
338+
}
339+
JSON_FIELD_NAME_INDEX = Utils.mapBuild(tmp);
340+
}
278341

279342
private final String selector;
280343
private final String grpcFieldName;
344+
private final Class<?> jsonClass;
281345

282-
BlobField(String selector) {
283-
this(selector, selector);
346+
BlobField(String selector, Class<?> jsonClass) {
347+
this(selector, selector, jsonClass);
284348
}
285349

286-
BlobField(String selector, String grpcFieldName) {
350+
BlobField(String selector, String grpcFieldName, Class<?> jsonClass) {
287351
this.selector = selector;
288352
this.grpcFieldName = grpcFieldName;
353+
this.jsonClass = jsonClass;
289354
}
290355

291356
@Override
@@ -302,6 +367,20 @@ public String getApiaryName() {
302367
public String getGrpcName() {
303368
return grpcFieldName;
304369
}
370+
371+
Class<?> getJsonClass() {
372+
return jsonClass;
373+
}
374+
375+
@Nullable
376+
static BlobField lookup(NamedField nf) {
377+
NamedField lookup = nf;
378+
if (nf instanceof NestedNamedField) {
379+
NestedNamedField nested = (NestedNamedField) nf;
380+
lookup = nested.getParent();
381+
}
382+
return JSON_FIELD_NAME_INDEX.get(lookup.getApiaryName());
383+
}
305384
}
306385

307386
enum UriScheme {

0 commit comments

Comments
 (0)