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.

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

fix: blob.reload() does not work as intuitively expected #308

Merged
merged 6 commits into from May 13, 2020
Merged

fix: blob.reload() does not work as intuitively expected #308

merged 6 commits into from May 13, 2020

Conversation

dmitry-fa
Copy link
Contributor

Fixes #149

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label May 12, 2020
@codecov
Copy link

codecov bot commented May 12, 2020 •
edited

Codecov Report

Merging #308 into master will increase coverage by 0.13%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #308      +/-   ##
============================================
+ Coverage     62.49%   62.63%   +0.13%     
- Complexity      552      553       +1     
============================================
  Files            31       31              
  Lines          5007     5023      +16     
  Branches        449      480      +31     
============================================
+ Hits           3129     3146      +17     
- Misses         1709     1711       +2     
+ Partials        169      166       -3
Impacted Files Coverage Δ Complexity Δ
...e/src/main/java/com/google/cloud/storage/Blob.java 82.18% <100.00%> (+0.20%) 30.00 <1.00> (+1.00)
...om/google/cloud/storage/spi/v1/HttpStorageRpc.java 1.52% <0.00%> (-0.01%) 1.00% <0.00%> (ø%)
...ain/java/com/google/cloud/storage/StorageImpl.java 80.27% <0.00%> (+0.12%) 131.00% <0.00%> (ø%)
...main/java/com/google/cloud/storage/BucketInfo.java 81.04% <0.00%> (+0.19%) 84.00% <0.00%> (ø%)
...gle/cloud/storage/testing/RemoteStorageHelper.java 64.51% <0.00%> (+0.58%) 9.00% <0.00%> (ø%)
...in/java/com/google/cloud/storage/StorageBatch.java 92.00% <0.00%> (+4.00%) 13.00% <0.00%> (ø%)

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 5566daf...da4d96c. Read the comment docs.

elharo
elharo reviewed May 12, 2020
View reviewed changes
google-cloud-storage/src/main/java/com/google/cloud/storage/Blob.java Outdated
*
* <p>Example of getting the blob's latest information, if its generation does not match the
* {@link Blob#getGeneration()} value, otherwise a {@link StorageException} is thrown.
* <p>{@code options} parameter can contain the preconditions. E.g. user may need to get the blob
Copy link
Contributor

Choose a reason for hiding this comment

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

E.g. --> For example,

Google style prohibits Latin abbreviations

Copy link
Contributor Author

Choose a reason for hiding this comment

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

E.g. is widely used, perhaps we need to collect such things and do some code clean up.

google-cloud-storage/src/main/java/com/google/cloud/storage/Blob.java Outdated
* @throws StorageException upon failure
*/
public Blob reload(BlobSourceOption... options) {
return storage.get(getBlobId(), toGetOptions(this, options));
BlobId id = getBlobId();
return storage.get(BlobId.of(id.getBucket(), id.getName()), toGetOptions(this, options));
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you make a copy of the ID here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

BlobId includes three components: bucket, name and generation. I form BlobId without generation to request the latest version of the blob.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, please add a comment explaining this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

elharo
elharo reviewed May 12, 2020
View reviewed changes
google-cloud-storage/src/main/java/com/google/cloud/storage/Blob.java Outdated
*
* <p>Example of getting the blob's latest information, if its generation does not match the
* {@link Blob#getGeneration()} value, otherwise a {@link StorageException} is thrown.
* <p>{@code options} parameter can contain the preconditions. For example, user may need to get
Copy link
Contributor

Choose a reason for hiding this comment

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

user may need --> the user might want

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

frankyn
frankyn requested changes May 13, 2020
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.

Adding a nit, overall LGTM.

google-cloud-storage/src/main/java/com/google/cloud/storage/Blob.java Outdated
* }
* }</pre>
*
* @param options blob read options
* @return a {@code Blob} object with latest information or {@code null} if not found
* @param options preconditions to fetch
Copy link
Member

Choose a reason for hiding this comment

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

preconditions to use, see https://cloud.google.com/storage/docs/json_api/v1/objects/get for more information.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

reload-jdoc

Copy link
Member

@frankyn frankyn May 13, 2020 •
edited

Choose a reason for hiding this comment

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

Thanks @dmitry-fa,

use:

preconditions to use on reload, see https://cloud.google.com/storage/docs/json_api/v1/objects/get for more information

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@frankyn frankyn merged commit a2bab58 into googleapis:master May 13, 2020
gcf-merge-on-green bot pushed a commit that referenced this pull request Jun 11, 2020
@release-please
chore: release 1.109.0 (#326)
1e31ea0 
🤖 I have created a release \*beep\* \*boop\* 
---
## [1.109.0](https://www.github.com/googleapis/java-storage/compare/v1.108.0...v1.109.0) (2020-06-11)


### Features

* adopt flatten-maven-plugin and java-shared-dependencies ([#325](https://www.github.com/googleapis/java-storage/issues/325)) ([209cae3](https://www.github.com/googleapis/java-storage/commit/209cae322932a4f87729fe4c5176a4f11962cfae))
* stub implementation of StorageRpc for the sake of testing ([#351](https://www.github.com/googleapis/java-storage/issues/351)) ([dd58025](https://www.github.com/googleapis/java-storage/commit/dd5802555eb0351a5afa2f2f197cb93ca6d3b66e))


### Bug Fixes

* blob.reload() does not work as intuitively expected ([#308](https://www.github.com/googleapis/java-storage/issues/308)) ([a2bab58](https://www.github.com/googleapis/java-storage/commit/a2bab58ccd89f48e8d4a8ee2dd776b201598420d))


### Documentation

* **fix:** update client documentation link ([#324](https://www.github.com/googleapis/java-storage/issues/324)) ([eb8940c](https://www.github.com/googleapis/java-storage/commit/eb8940cc6a88b5e2b3dea8d0ab2ffc1e350ab924))
* Add doc for equals method in blob ([#311](https://www.github.com/googleapis/java-storage/issues/311)) ([91fc36a](https://www.github.com/googleapis/java-storage/commit/91fc36a6673e30d1cfa8c4da51b874e1fd0b0535))
* catch actual exception in java doc comment ([#312](https://www.github.com/googleapis/java-storage/issues/312)) ([9201de5](https://www.github.com/googleapis/java-storage/commit/9201de559fe4218abd2e4fac47beac62454547cf)), closes [#309](https://www.github.com/googleapis/java-storage/issues/309)
* update CONTRIBUTING.md to include code formatting ([#534](https://www.github.com/googleapis/java-storage/issues/534)) ([#315](https://www.github.com/googleapis/java-storage/issues/315)) ([466d08f](https://www.github.com/googleapis/java-storage/commit/466d08f9835a0f1dd00b5c9b3a08551be68d03ad))
* update readme to point client libarary documentation ([#317](https://www.github.com/googleapis/java-storage/issues/317)) ([8650f80](https://www.github.com/googleapis/java-storage/commit/8650f806736beec7bf7ab09a337b333bbf144f7b))


### Dependencies

* update dependency com.google.api.grpc:proto-google-common-protos to v1.18.0 ([#301](https://www.github.com/googleapis/java-storage/issues/301)) ([ff2dee2](https://www.github.com/googleapis/java-storage/commit/ff2dee2ce41d37787f0866ae740d3cd7f3b2bbd6))
* update dependency com.google.apis:google-api-services-storage to v1-rev20200410-1.30.9 ([#296](https://www.github.com/googleapis/java-storage/issues/296)) ([2e55aa2](https://www.github.com/googleapis/java-storage/commit/2e55aa2c8b9c78df9eebfe748fe72dcaae63ff81))
* update dependency com.google.apis:google-api-services-storage to v1-rev20200430-1.30.9 ([#319](https://www.github.com/googleapis/java-storage/issues/319)) ([3d03fa3](https://www.github.com/googleapis/java-storage/commit/3d03fa3381cfbb76d1501ec3d2ad14742a8a58dd))
* update dependency com.google.cloud:google-cloud-conformance-tests to v0.0.11 ([#320](https://www.github.com/googleapis/java-storage/issues/320)) ([6c18c88](https://www.github.com/googleapis/java-storage/commit/6c18c882cfe0c35b310a518e6044847e6fbeab94))
---


This PR was generated with [Release Please](https://github.com/googleapis/release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@elharo elharo elharo left review comments

@frankyn frankyn frankyn approved these changes

@JesseLovelace JesseLovelace Awaiting requested review from JesseLovelace

Assignees
No one assigned
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Blob.reload() does not work as intuitively expected
4 participants
@dmitry-fa @elharo @frankyn @googlebot