-
Notifications
You must be signed in to change notification settings - Fork 77
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.
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
fix: Documentation for Blob.update() and Storage.update() methods is confusing/incorrect #261
Merged
Merged
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -546,44 +546,24 @@ public Blob reload(BlobSourceOption... options) { | |
} | ||
|
||
/** | ||
* Updates the blob's information. Bucket or blob's name cannot be changed by this method. If you | ||
* want to rename the blob or move it to a different bucket use the {@link #copyTo} and {@link | ||
* #delete} operations. A new {@code Blob} object is returned. By default no checks are made on | ||
* the metadata generation of the current blob. If you want to update the information only if the | ||
* current blob metadata are at their latest version use the {@code metagenerationMatch} option: | ||
* {@code newBlob.update(BlobTargetOption.metagenerationMatch())}. | ||
* Updates the blob properties. The {@code options} parameter contains the preconditions for | ||
* applying the update. To update the properties call {@link #toBuilder()}, set the properties you | ||
* want to change, build the new {@code Blob} instance, and then call {@link | ||
* #update(BlobTargetOption...)}. | ||
* | ||
* <p>Original metadata are merged with metadata in the provided {@code blobInfo}. If the original | ||
* metadata already contains a key specified in the provided {@code blobInfo's} metadata map, it | ||
* will be replaced by the new value. Removing metadata can be done by setting that metadata's | ||
* value to {@code null}. | ||
* <p>The property update details are described in {@link Storage#update(BlobInfo)}. {@link | ||
* Storage#update(BlobInfo, BlobTargetOption...)} describes how to specify preconditions. | ||
* | ||
* <p>Example of adding new metadata values or updating existing ones. | ||
* <p>Example of updating the content type: | ||
* | ||
* <pre>{@code | ||
* String bucketName = "my_unique_bucket"; | ||
* String blobName = "my_blob_name"; | ||
* Map<String, String> newMetadata = new HashMap<>(); | ||
* newMetadata.put("keyToAddOrUpdate", "value"); | ||
* Blob blob = storage.update(BlobInfo.newBuilder(bucketName, blobName) | ||
* .setMetadata(newMetadata) | ||
* .build()); | ||
* }</pre> | ||
* | ||
* <p>Example of removing metadata values. | ||
* | ||
* <pre>{@code | ||
* String bucketName = "my_unique_bucket"; | ||
* String blobName = "my_blob_name"; | ||
* Map<String, String> newMetadata = new HashMap<>(); | ||
* newMetadata.put("keyToRemove", null); | ||
* Blob blob = storage.update(BlobInfo.newBuilder(bucketName, blobName) | ||
* .setMetadata(newMetadata) | ||
* .build()); | ||
* BlobId blobId = BlobId.of(bucketName, blobName); | ||
* Blob blob = storage.get(blobId); | ||
* blob.toBuilder().setContentType("text/plain").build().update(); | ||
* }</pre> | ||
* | ||
* @param options update options | ||
* @return a {@code Blob} object with updated information | ||
* @param options preconditions to apply the update | ||
* @return the updated blob | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. okay |
||
* @throws StorageException upon failure | ||
*/ | ||
public Blob update(BlobTargetOption... options) { | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd state
{@code options}
isBlobTargetOption
and instead ofpreconditions
userequest options
and link to update API documentation: https://cloud.google.com/storage/docs/json_api/v1/objects/updateThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
I think this is clear from the parameter type
Personally for me it was hard to understand the purspose of this parameter. So, I tried to rephrase this statement to help users not very familiar with the Storage details to get the meaning. I came up to:
The options parameter contains the preconditions for applying the update
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, sgtm, I think having the link will help provide information.