Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add object retention feature #2277

Merged
merged 16 commits into from
Nov 30, 2023
+548 −21
Merged

feat: add object retention feature #2277

Changes from 1 commit
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
dcc0ca4
feat: add object retention feature
JesseLovelace Oct 24, 2023
7b4a2a7
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] Oct 24, 2023
876c293
Merge branch 'main' into objlock
JesseLovelace Oct 25, 2023
b620c67
address review comments
JesseLovelace Oct 25, 2023
862a564
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] Oct 25, 2023
dc6d226
Add a workaround to avoid a testbench failure
JesseLovelace Oct 25, 2023
7ca2178
Merge branch 'objlock' of github.com:googleapis/java-storage into obj…
JesseLovelace Oct 25, 2023
40ae1f8
chore: some fixes from pairing with jesse
BenWhitehead Oct 25, 2023
5fd0937
chore: put a `!` :don't program while sleepy:
BenWhitehead Oct 26, 2023
9159db3
chore: mvn-fmt
BenWhitehead Oct 26, 2023
70c9c5e
add a way to avoid using the objectRetentionField in bucket tests
JesseLovelace Oct 26, 2023
52eecf7
add retention to blobfield tests
JesseLovelace Oct 27, 2023
e16abae
clirr and fmt
JesseLovelace Oct 27, 2023
3bc56f6
add todos
JesseLovelace Oct 27, 2023
abd314c
fmt
JesseLovelace Oct 27, 2023
2f9f38b
Merge branch 'main' into objlock
JesseLovelace Nov 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
chore: mvn-fmt
@BenWhitehead
BenWhitehead committed Oct 26, 2023
commit 9159db3fb3eae2a7b10b2658e8145cb5f29d61b5
10 changes: 6 additions & 4 deletions google-cloud-storage/src/main/java/com/google/cloud/storage/ApiaryConversions.java
View file
Original file line number Diff line number Diff line change
@@ -248,12 +248,14 @@ private StorageObject blobInfoEncode(BlobInfo from) {
to::setRetentionExpirationTime);

// todo: clean this up once retention is enabled in grpc
// This is a workaround so that explicitly null retention objects are only included when the user
// set an existing policy to null, to avoid sending any retention objects to the test bench.
// This is a workaround so that explicitly null retention objects are only included when the
// user set an existing policy to null, to avoid sending any retention objects to the test
// bench.
// We should clean this up once the test bench can handle the retention field.
// See also the comment in StorageImpl.update(BlobInfo blobInfo, BlobTargetOption... options)
if (from.getModifiedFields().contains(Storage.BlobField.RETENTION) && from.getRetention() == null) {
to.setRetention(Data.nullOf(StorageObject.Retention.class));
if (from.getModifiedFields().contains(Storage.BlobField.RETENTION)
&& from.getRetention() == null) {
to.setRetention(Data.nullOf(StorageObject.Retention.class));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What was the conclusion on detecting if a user set retention to null vs. it not being modified?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't work, due to the modified fields not being passed along to the final patch call. I talked to ben and we agreed to just do it this way, there's precedent for it in the logging encoding

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for clarifying, is this value mutable once set?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only certain cases work (i.e. going from Unlocked to Locked, reducing the retainUntilTime on an unlocked policy, and setting to null), and you have to pass in overrideUnlockedRetention for any of those cases to work, but yes

}
ifNonNull(from.getRetention(), this::retentionEncode, to::setRetention);
to.setKmsKeyName(from.getKmsKeyName());