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

Jump to bottom

Support Bucket/Object lock operations #320

Merged
merged 12 commits into from
Sep 10, 2018
Merged

Support Bucket/Object lock operations #320

merged 12 commits into from
Sep 10, 2018

Conversation

stephenplusplus
Copy link
Contributor

@stephenplusplus stephenplusplus commented Aug 2, 2018 •
edited
Loading

To Dos

  • Tests
    • System
    • Unit

References

This introduces new behavior:

Buckets

  • bucket#lock()
    Lock a previously-defined retention policy. This will prevent changes to the policy.

  • bucket#removeRetentionPeriod()
    Remove an already-existing retention policy from this bucket.

  • bucket#setRetentionPeriod(durationInSeconds)
    Lock all objects contained in the bucket, based on their creation time.

Files

  • file#getExpirationDate()
    Get a Date object representing the earliest time this file will expire.

Additionally, bucket#getMetadata(), bucket#setMetadata(), file#getMetadata(), and file#setMetadata() are available if users wish to interact with the raw resource schemas as defined by the API.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Aug 2, 2018
@ghost ghost assigned stephenplusplus Aug 2, 2018
@stephenplusplus stephenplusplus added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Aug 2, 2018
frankyn
frankyn requested changes Aug 17, 2018
View reviewed changes
Copy link
Member

@frankyn frankyn left a comment

Choose a reason for hiding this comment

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

Thanks @stephenplusplus, getting back on track for bucket lock.

src/file.js Outdated
* retention policy will expire.
*/
/**
* Get a Date object representing the earliest time this file will expire.

This comment was marked as spam.

Sign in to view

src/bucket.js Outdated
@@ -1799,6 +1836,34 @@ class Bucket extends common.ServiceObject {
return new Notification(this, id);
}

/**
* Remove an already-existing retention policy from this bucket.

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

src/bucket.js Outdated
* {@link File#removeRetentionPeriod} and {@link File#setRetentionPeriod}. A
* locked retention policy cannot be removed or shortened in duration for the
* lifetime of the bucket. Attempting to remove or decrease period of a locked
* retention policy will result in a `PERMISSION_DENIED` error.

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

src/file.js Outdated
* released.
*
* @param {object} [options] Configuration object.
* @param {boolean} [options.temporary=false] - Set a temporary hold.

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

src/file.js Outdated
* @param {SetFileMetadataCallback} [callback] Callback function.
* @returns {Promise<SetFileMetadataResponse>}
*/
hold(options, callback) {

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@codecov
Copy link

codecov bot commented Aug 20, 2018 •
edited
Loading

Codecov Report

❗ No coverage uploaded for pull request base (master@74beabc). Click here to learn what that means.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #320   +/-   ##
=========================================
  Coverage          ?   96.98%           
=========================================
  Files             ?        7           
  Lines             ?     1063           
  Branches          ?        0           
=========================================
  Hits              ?     1031           
  Misses            ?       32           
  Partials          ?        0
Impacted Files Coverage Δ
src/index.js 100% <ø> (ø)
src/file.js 94.52% <0%> (ø)
src/bucket.js 98.07% <0%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 74beabc...873b024. Read the comment docs.

frankyn
frankyn reviewed Sep 4, 2018
View reviewed changes
src/bucket.js Outdated
* const storage = require('@google-cloud/storage')();
* const bucket = storage.bucket('albums');
*
* bucket.lock(function(err, apiResponse) {});

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

src/bucket.js Outdated
@@ -1799,6 +1836,34 @@ class Bucket extends common.ServiceObject {
return new Notification(this, id);
}

/**
* Remove an already-existing retention policy from this bucket.

This comment was marked as spam.

Sign in to view

src/file.js Outdated
* @param {SetFileMetadataCallback} [callback] Callback function.
* @returns {Promise<SetFileMetadataResponse>}
*/
hold(options, callback) {

This comment was marked as spam.

Sign in to view

@stephenplusplus
Copy link
Contributor Author

@frankyn I've added the system tests to verify the hold behavior will block overwrite and delete operations on File objects. I've also removed file.hold() and file.release() and updated the docs for file.setMetadata().

@stephenplusplus stephenplusplus removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Sep 6, 2018
frankyn
frankyn requested changes Sep 6, 2018
View reviewed changes
Copy link
Member

@frankyn frankyn left a comment

Choose a reason for hiding this comment

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

Thanks @stephenplusplus, rest of the surface LGTM. Only requested change is to include a paramater for metageneration in lockRetentionPolicy() instead of performing a getMetadata() beforehand.

frankyn
frankyn reviewed Sep 7, 2018
View reviewed changes
src/bucket.ts Outdated
* //-
* // If the callback is omitted, we'll return a Promise.
* //-
* bucket.lock().then(function(data) {

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@ghost ghost assigned JustinBeckwith Sep 7, 2018
frankyn
frankyn approved these changes Sep 10, 2018
View reviewed changes
Copy link
Member

@frankyn frankyn left a comment

Choose a reason for hiding this comment

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

Thanks @stephenplusplus! Lgtm

@frankyn frankyn added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Sep 10, 2018
@stephenplusplus stephenplusplus merged commit 2b1ffaf into googleapis:master Sep 10, 2018
stephenplusplus added a commit that referenced this pull request Sep 10, 2018
@stephenplusplus
Revert "Support Bucket/Object lock operations (#320)"
36c4ca2 
This reverts commit 2b1ffaf.
stephenplusplus added a commit that referenced this pull request Sep 10, 2018
@stephenplusplus
Revert "Support Bucket/Object lock operations (#320)" (#373)
702d7bd 
This reverts commit 2b1ffaf.
stephenplusplus added a commit that referenced this pull request Sep 10, 2018
@stephenplusplus
Revert "Revert "Support Bucket/Object lock operations (#320)" (#373)"
6feffd5 
This reverts commit 702d7bd.
@stephenplusplus stephenplusplus mentioned this pull request Sep 10, 2018
Merged
2 tasks
frankyn pushed a commit to frankyn/nodejs-storage that referenced this pull request Oct 3, 2018
@stephenplusplus @frankyn
Revert "Revert "Support Bucket/Object lock operations (googleapis#320)…
afa7d5e 
…" (googleapis#373)"

This reverts commit 702d7bd.
stephenplusplus added a commit that referenced this pull request Oct 3, 2018
@stephenplusplus
Revert "Revert "Support Bucket/Object lock operations (#320)" (#373)"
bbac7a6 
This reverts commit 702d7bd.
JustinBeckwith pushed a commit that referenced this pull request Oct 9, 2018
@stephenplusplus @JustinBeckwith
Revert "Revert "Support Bucket/Object lock operations (#320)" (#373)"
b6268e5 
This reverts commit 702d7bd.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@frankyn frankyn frankyn approved these changes

@callmehiphop callmehiphop Awaiting requested review from callmehiphop

Assignees

@JustinBeckwith JustinBeckwith

@stephenplusplus stephenplusplus

Labels
cla: yes This human has signed the Contributor License Agreement. do not merge Indicates a pull request not ready for merge, due to either quality or timing.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@stephenplusplus @frankyn @JustinBeckwith @googlebot