Skip to content

Commit d17bc80

Browse files
committed
fix: update modified field handling for blob and bucket with json transport 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
1 parent 48e7a4c commit d17bc80

File tree

6 files changed

+335
-93
lines changed

6 files changed

+335
-93
lines changed

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

Lines changed: 7 additions & 1 deletion
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

Lines changed: 107 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -106,80 +106,98 @@ String getEntry() {
106106

107107
enum BucketField implements FieldSelector, NamedField {
108108
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
109-
ID("id", "bucket_id"),
109+
ID("id", "bucket_id", String.class),
110110
@TransportCompatibility(Transport.HTTP)
111-
SELF_LINK("selfLink"),
111+
SELF_LINK("selfLink", String.class),
112112
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
113-
NAME("name"),
113+
NAME("name", String.class),
114114
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
115-
TIME_CREATED("timeCreated", "create_time"),
115+
TIME_CREATED("timeCreated", "create_time", com.google.api.client.util.DateTime.class),
116116
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
117-
METAGENERATION("metageneration"),
117+
METAGENERATION("metageneration", Long.class),
118118
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
119-
ACL("acl"),
119+
ACL("acl", List.class),
120120
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
121-
DEFAULT_OBJECT_ACL("defaultObjectAcl", "default_object_acl"),
121+
DEFAULT_OBJECT_ACL("defaultObjectAcl", "default_object_acl", List.class),
122122
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
123-
OWNER("owner"),
123+
OWNER("owner", com.google.api.services.storage.model.Bucket.Owner.class),
124124
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
125-
LABELS("labels"),
125+
LABELS("labels", Map.class),
126126
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
127-
LOCATION("location"),
127+
LOCATION("location", String.class),
128128
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
129-
LOCATION_TYPE("locationType", "location_type"),
129+
LOCATION_TYPE("locationType", "location_type", String.class),
130130
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
131-
WEBSITE("website"),
131+
WEBSITE("website", com.google.api.services.storage.model.Bucket.Website.class),
132132
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
133-
VERSIONING("versioning"),
133+
VERSIONING("versioning", com.google.api.services.storage.model.Bucket.Versioning.class),
134134
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
135-
CORS("cors"),
135+
CORS("cors", List.class),
136136
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
137-
LIFECYCLE("lifecycle"),
137+
LIFECYCLE("lifecycle", com.google.api.services.storage.model.Bucket.Lifecycle.class),
138138
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
139-
STORAGE_CLASS("storageClass", "storage_class"),
139+
STORAGE_CLASS("storageClass", "storage_class", String.class),
140140
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
141-
ETAG("etag"),
141+
ETAG("etag", String.class),
142142
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
143-
ENCRYPTION("encryption"),
143+
ENCRYPTION("encryption", com.google.api.services.storage.model.Bucket.Encryption.class),
144144
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
145-
BILLING("billing"),
145+
BILLING("billing", com.google.api.services.storage.model.Bucket.Billing.class),
146146
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
147-
DEFAULT_EVENT_BASED_HOLD("defaultEventBasedHold", "default_event_based_hold"),
147+
DEFAULT_EVENT_BASED_HOLD("defaultEventBasedHold", "default_event_based_hold", Boolean.class),
148148
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
149-
RETENTION_POLICY("retentionPolicy", "retention_policy"),
149+
RETENTION_POLICY(
150+
"retentionPolicy",
151+
"retention_policy",
152+
com.google.api.services.storage.model.Bucket.RetentionPolicy.class),
150153
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
151-
IAMCONFIGURATION("iamConfiguration", "iam_config"),
154+
IAMCONFIGURATION(
155+
"iamConfiguration",
156+
"iam_config",
157+
com.google.api.services.storage.model.Bucket.IamConfiguration.class),
152158
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
153-
LOGGING("logging"),
159+
LOGGING("logging", com.google.api.services.storage.model.Bucket.Logging.class),
154160
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
155-
UPDATED("updated", "update_time"),
161+
UPDATED("updated", "update_time", com.google.api.client.util.DateTime.class),
156162
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
157-
RPO("rpo"),
163+
RPO("rpo", String.class),
158164
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
159-
CUSTOM_PLACEMENT_CONFIG("customPlacementConfig", "custom_placement_config"),
165+
CUSTOM_PLACEMENT_CONFIG(
166+
"customPlacementConfig",
167+
"custom_placement_config",
168+
com.google.api.services.storage.model.Bucket.CustomPlacementConfig.class),
160169
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
161-
AUTOCLASS("autoclass"),
170+
AUTOCLASS("autoclass", com.google.api.services.storage.model.Bucket.Autoclass.class),
162171

163172
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
164-
HIERARCHICAL_NAMESPACE("hierarchicalNamespace", "hierarchical_namespace"),
173+
HIERARCHICAL_NAMESPACE(
174+
"hierarchicalNamespace",
175+
"hierarchical_namespace",
176+
com.google.api.services.storage.model.Bucket.HierarchicalNamespace.class),
165177
@TransportCompatibility({Transport.HTTP})
166-
OBJECT_RETENTION("objectRetention"),
178+
OBJECT_RETENTION(
179+
"objectRetention", com.google.api.services.storage.model.Bucket.ObjectRetention.class),
167180

168181
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
169-
SOFT_DELETE_POLICY("softDeletePolicy", "soft_delete_policy");
182+
SOFT_DELETE_POLICY(
183+
"softDeletePolicy",
184+
"soft_delete_policy",
185+
com.google.api.services.storage.model.Bucket.SoftDeletePolicy.class);
170186

171187
static final List<BucketField> REQUIRED_FIELDS = ImmutableList.of(NAME);
172188

173189
private final String selector;
174190
private final String grpcFieldName;
191+
private final Class<?> jsonClass;
175192

176-
BucketField(String selector) {
177-
this(selector, selector);
193+
BucketField(String selector, Class<?> jsonClass) {
194+
this(selector, selector, jsonClass);
178195
}
179196

180-
BucketField(String selector, String grpcFieldName) {
197+
BucketField(String selector, String grpcFieldName, Class<?> jsonClass) {
181198
this.selector = selector;
182199
this.grpcFieldName = grpcFieldName;
200+
this.jsonClass = jsonClass;
183201
}
184202

185203
@Override
@@ -196,96 +214,110 @@ public String getApiaryName() {
196214
public String getGrpcName() {
197215
return grpcFieldName;
198216
}
217+
218+
Class<?> getJsonClass() {
219+
return jsonClass;
220+
}
199221
}
200222

201223
enum BlobField implements FieldSelector, NamedField {
202224
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
203-
ACL("acl"),
225+
ACL("acl", com.google.api.services.storage.model.ObjectAccessControl.class),
204226
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
205-
BUCKET("bucket"),
227+
BUCKET("bucket", String.class),
206228
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
207-
CACHE_CONTROL("cacheControl", "cache_control"),
229+
CACHE_CONTROL("cacheControl", "cache_control", String.class),
208230
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
209-
COMPONENT_COUNT("componentCount", "component_count"),
231+
COMPONENT_COUNT("componentCount", "component_count", Integer.class),
210232
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
211-
CONTENT_DISPOSITION("contentDisposition", "content_disposition"),
233+
CONTENT_DISPOSITION("contentDisposition", "content_disposition", String.class),
212234
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
213-
CONTENT_ENCODING("contentEncoding", "content_encoding"),
235+
CONTENT_ENCODING("contentEncoding", "content_encoding", String.class),
214236
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
215-
CONTENT_LANGUAGE("contentLanguage", "content_language"),
237+
CONTENT_LANGUAGE("contentLanguage", "content_language", String.class),
216238
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
217-
CONTENT_TYPE("contentType", "content_type"),
239+
CONTENT_TYPE("contentType", "content_type", String.class),
218240
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
219-
CRC32C("crc32c", "checksums.crc32c"),
241+
CRC32C("crc32c", "checksums.crc32c", String.class),
220242
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
221-
ETAG("etag"),
243+
ETAG("etag", String.class),
222244
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
223-
GENERATION("generation"),
245+
GENERATION("generation", Long.class),
224246
@TransportCompatibility(Transport.HTTP)
225-
ID("id"),
247+
ID("id", String.class),
226248
/** {@code kind} is not exposed in {@link BlobInfo} or {@link Blob} no need to select it */
227249
@Deprecated
228250
@TransportCompatibility(Transport.HTTP)
229-
KIND("kind"),
251+
KIND("kind", String.class),
230252
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
231-
MD5HASH("md5Hash", "checksums.md5_hash"),
253+
MD5HASH("md5Hash", "checksums.md5_hash", String.class),
232254
@TransportCompatibility(Transport.HTTP)
233-
MEDIA_LINK("mediaLink"),
255+
MEDIA_LINK("mediaLink", String.class),
234256
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
235-
METADATA("metadata"),
257+
METADATA("metadata", Map.class),
236258
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
237-
METAGENERATION("metageneration"),
259+
METAGENERATION("metageneration", Long.class),
238260
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
239-
NAME("name"),
261+
NAME("name", String.class),
240262
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
241-
OWNER("owner"),
263+
OWNER("owner", com.google.api.services.storage.model.StorageObject.Owner.class),
242264
@TransportCompatibility(Transport.HTTP)
243-
SELF_LINK("selfLink"),
265+
SELF_LINK("selfLink", String.class),
244266
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
245-
SIZE("size"),
267+
SIZE("size", java.math.BigInteger.class),
246268
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
247-
STORAGE_CLASS("storageClass", "storage_class"),
269+
STORAGE_CLASS("storageClass", "storage_class", String.class),
248270
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
249-
TIME_DELETED("timeDeleted", "delete_time"),
271+
TIME_DELETED("timeDeleted", "delete_time", com.google.api.client.util.DateTime.class),
250272
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
251-
TIME_CREATED("timeCreated", "create_time"),
273+
TIME_CREATED("timeCreated", "create_time", com.google.api.client.util.DateTime.class),
252274
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
253-
KMS_KEY_NAME("kmsKeyName", "kms_key"),
275+
KMS_KEY_NAME("kmsKeyName", "kms_key", String.class),
254276
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
255-
EVENT_BASED_HOLD("eventBasedHold", "event_based_hold"),
277+
EVENT_BASED_HOLD("eventBasedHold", "event_based_hold", String.class),
256278
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
257-
TEMPORARY_HOLD("temporaryHold", "temporary_hold"),
279+
TEMPORARY_HOLD("temporaryHold", "temporary_hold", String.class),
258280
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
259-
RETENTION_EXPIRATION_TIME("retentionExpirationTime", "retention_expire_time"),
281+
RETENTION_EXPIRATION_TIME(
282+
"retentionExpirationTime",
283+
"retention_expire_time",
284+
com.google.api.client.util.DateTime.class),
260285
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
261-
UPDATED("updated", "update_time"),
286+
UPDATED("updated", "update_time", com.google.api.client.util.DateTime.class),
262287
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
263-
CUSTOM_TIME("customTime", "custom_time"),
288+
CUSTOM_TIME("customTime", "custom_time", com.google.api.client.util.DateTime.class),
264289
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
265-
TIME_STORAGE_CLASS_UPDATED("timeStorageClassUpdated", "update_storage_class_time"),
290+
TIME_STORAGE_CLASS_UPDATED(
291+
"timeStorageClassUpdated",
292+
"update_storage_class_time",
293+
com.google.api.client.util.DateTime.class),
266294
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
267-
CUSTOMER_ENCRYPTION("customerEncryption", "customer_encryption"),
295+
CUSTOMER_ENCRYPTION("customerEncryption", "customer_encryption", String.class),
268296
@TransportCompatibility({Transport.HTTP})
269-
RETENTION("retention"),
297+
RETENTION("retention", com.google.api.services.storage.model.StorageObject.Retention.class),
270298

271299
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
272-
SOFT_DELETE_TIME("softDeleteTime", "soft_delete_time"),
300+
SOFT_DELETE_TIME(
301+
"softDeleteTime", "soft_delete_time", com.google.api.client.util.DateTime.class),
273302

274303
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
275-
HARD_DELETE_TIME("hardDeleteTime", "hard_delete_time");
304+
HARD_DELETE_TIME(
305+
"hardDeleteTime", "hard_delete_time", com.google.api.client.util.DateTime.class);
276306

277307
static final List<NamedField> REQUIRED_FIELDS = ImmutableList.of(BUCKET, NAME);
278308

279309
private final String selector;
280310
private final String grpcFieldName;
311+
private final Class<?> jsonClass;
281312

282-
BlobField(String selector) {
283-
this(selector, selector);
313+
BlobField(String selector, Class<?> jsonClass) {
314+
this(selector, selector, jsonClass);
284315
}
285316

286-
BlobField(String selector, String grpcFieldName) {
317+
BlobField(String selector, String grpcFieldName, Class<?> jsonClass) {
287318
this.selector = selector;
288319
this.grpcFieldName = grpcFieldName;
320+
this.jsonClass = jsonClass;
289321
}
290322

291323
@Override
@@ -302,6 +334,10 @@ public String getApiaryName() {
302334
public String getGrpcName() {
303335
return grpcFieldName;
304336
}
337+
338+
Class<?> getJsonClass() {
339+
return jsonClass;
340+
}
305341
}
306342

307343
enum UriScheme {

0 commit comments

Comments
 (0)