feat: Bucket metadata grpc #1435
Conversation
| } | ||
|
|
||
| private Bucket.Lifecycle buildLifecyclePolicy(BucketInfo from) { | ||
| // Handle duplicate rules introduced by deleteRules using a backing ImmutableSet |
There was a problem hiding this comment.
Handling duplicates, what are your thoughts @BenWhitehead; I'm deduping proto Rule message types, but not sure if equals() will handle unknown fields as different. Looking into this next; but thought you may already know.
There was a problem hiding this comment.
Looks like this should be okay, tested an unknown field on Rule (thankfully action and condition are both messages as well): Let me know if I goofed.
// The following passes
@Test
public void assertNotEqualWithUnknownField() {
com.google.storage.v2.Bucket.Lifecycle.Rule rule =
com.google.storage.v2.Bucket.Lifecycle.Rule.getDefaultInstance();
assertNotEquals(
com.google.storage.v2.Bucket.Lifecycle.Rule.getDefaultInstance()
.toBuilder()
.setUnknownFields(
UnknownFieldSet.newBuilder()
.addField(100, UnknownFieldSet.Field.newBuilder().addFixed32(10).build())
.build()).build(), com.google.storage.v2.Bucket.Lifecycle.Rule.getDefaultInstance());
}There was a problem hiding this comment.
Hmm, duplicates is an interesting case that we haven't thus far accounted for.
In fact the existing BucketInfo.fromPb[1] is duplicating and not deduplicating later.
I think we're going to want to start another PR after merging this one, where we actually go through and remove all internal usage of DeleteRule and instead translate the respective delete rule operations into the correct operations for lifecycle rules. Then, we can remove this duplication from both the apiary and grpc converters.
Regarding handling of unknown fields, we only have to account for them when decoding to BlobInfo. Once the BlobInfo is decoded, the unknown fields will effectively be abandoned.
There was a problem hiding this comment.
We spoke offline, we're going to open a separate PR to remove the duplication in BucketInfo and instead shim the old DeleteRules over exclusively LifecycleRules
Add remaining fields for Bucket metadata (defaultObjectAcls, Owner, Logging, Cors, and IamConfiguration)
Fix OLM deleteRules encode/decode to only function when Rules are exactly what's expected otherwise return null.