Repository: googleapis/java-storage (java)

Issue:
Blob.reload() does not work as intuitively expected
Documentation for Blob.writer(BlobWriterOption...) recommends the following way to upload information:

```
 byte[] content = "Hello, World!".getBytes(UTF_8);
 try (WriteChannel writer = blob.writer()) {
   try {
     writer.write(ByteBuffer.wrap(content, 0, content.length));
   } catch (Exception ex) {
     // handle exception
   }
 }
```

But doc is silent on how to renew the blob object, which is a bit tricky because neither `blob.reload()` nor `storage.getBlob(blob.getBlobId())` does not help. Behavior varies depending on versioning setting for the bucket.

If versioning is enabled:

Attempt to reload as recommended:
`blob.reload(Blob.BlobSourceOption.generationNotMatch())` causes `StorageException '304 Not Modified'`

`blob.reload()`  returns the same blob.
`storage.get(blob.getBlobId())` returns the same blob (that seems very confusing).
And only `storage.get(blobId)` returns the updated blob.

If versioning is Suspended:
before upload: `blob.getContent()` returns an empty array
After upload: `blob.getContent()` throws `StorageException: 404 Not Found , No such object`
`storage.get(blob.getBlobId())`, `blob.reload()` and `blob.reload(Blob.BlobSourceOption.generationNotMatch())` return null.

And again only storage.get(blobId) returns the updated blob.


Pull request:
fix: blob.reload() does not work as intuitively expected
Fixes #149 


Patch:
diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS
new file mode 100644
index 00000000..36446533
--- /dev/null
+++ b/.github/CODEOWNERS
@@ -0,0 +1,10 @@
+# Code owners file.
+# This file controls who is tagged for review for any given pull request.
+#
+# For syntax help see:
+# https://help.github.com/en/github/creating-cloning-and-archiving-repositories/about-code-owners#codeowners-syntax
+
+
+# The storage-dpe team is the default owner for anything not
+# explicitly taken by someone else.
+*                               @googleapis/storage-dpe
diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml
new file mode 100644
index 00000000..445b4bf8
--- /dev/null
+++ b/.github/workflows/ci.yaml
@@ -0,0 +1,76 @@
+on:
+  push:
+    branches:
+    - master
+  pull_request:
+name: ci
+jobs:
+  units:
+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        java: [7, 8, 11]
+    steps:
+    - uses: actions/checkout@v2
+    - uses: actions/setup-java@v1
+      with:
+        java-version: ${{matrix.java}}
+    - run: java -version
+    - run: .kokoro/build.sh
+      env:
+        JOB_TYPE: test
+    - name: coverage
+      uses: codecov/codecov-action@v1
+      with:
+        name: actions ${{matrix.java}}
+  windows:
+    runs-on: windows-latest
+    steps:
+    - uses: actions/checkout@v2
+    - uses: actions/setup-java@v1
+      with:
+        java-version: 8
+    - run: java -version
+    - run: .kokoro/build.bat
+      env:
+        JOB_TYPE: test
+  dependencies:
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/checkout@v2
+    - uses: actions/setup-java@v1
+      with:
+        java-version: 8
+    - run: java -version
+    - run: .kokoro/dependencies.sh
+  linkage-monitor:
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/checkout@v2
+    - uses: actions/setup-java@v1
+      with:
+        java-version: 8
+    - run: java -version
+    - run: .kokoro/linkage-monitor.sh
+  lint:
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/checkout@v2
+    - uses: actions/setup-java@v1
+      with:
+        java-version: 8
+    - run: java -version
+    - run: .kokoro/build.sh
+      env:
+        JOB_TYPE: lint
+  clirr:
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/checkout@v2
+    - uses: actions/setup-java@v1
+      with:
+        java-version: 8
+    - run: java -version
+    - run: .kokoro/build.sh
+      env:
+        JOB_TYPE: clirr
\ No newline at end of file
diff --git a/.kokoro/build.bat b/.kokoro/build.bat
index 1a8a4502..05826ad9 100644
--- a/.kokoro/build.bat
+++ b/.kokoro/build.bat
@@ -1,3 +1,3 @@
 :: See documentation in type-shell-output.bat
 
-"C:\Program Files\Git\bin\bash.exe" github/java-storage/.kokoro/build.sh
+"C:\Program Files\Git\bin\bash.exe" %~dp0build.sh
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index 085021dd..f2dbdee0 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -99,7 +99,16 @@ mvn -Penable-samples clean verify
     ```
 
 2. [Activate](#profile-activation) the profile.
-3. Define your samples in a normal Maven project in the `samples/` directory
+3. Define your samples in a normal Maven project in the `samples/` directory.
+
+### Code Formatting
+
+Code in this repo is formatted with
+[google-java-format](https://github.com/google/google-java-format).
+To run formatting on your project, you can run:
+```
+mvn com.coveo:fmt-maven-plugin:format
+```
 
 ### Profile Activation
 
diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/Blob.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/Blob.java
index 743f3e7b..6646926a 100644
--- a/google-cloud-storage/src/main/java/com/google/cloud/storage/Blob.java
+++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/Blob.java
@@ -525,24 +525,41 @@ public class Blob extends BlobInfo {
   }
 
   /**
-   * Fetches current blob's latest information. Returns {@code null} if the blob does not exist.
+   * Fetches the latest blob properties. Returns {@code null} if the blob no longer exists.
    *
-   * <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, the user might want to
+   * get the blob properties only if the content has not been updated externally. {@code
+   * StorageException} with the code {@code 412} is thrown if preconditions fail.
+   *
+   * <p>Example of retrieving the blob's latest information only if the content is not updated
+   * externally:
    *
    * <pre>{@code
-   * Blob latestBlob = blob.reload(BlobSourceOption.generationNotMatch());
-   * if (latestBlob == null) {
-   *   // the blob was not found
+   * Blob blob = storage.get(BlobId.of(bucketName, blobName));
+   *
+   * doSomething();
+   *
+   * try {
+   *   blob = blob.reload(Blob.BlobSourceOption.generationMatch());
+   * } catch (StorageException e) {
+   *   if (e.getCode() == 412) {
+   *     // the content was updated externally
+   *   } else {
+   *     throw e;
+   *   }
    * }
    * }</pre>
    *
-   * @param options blob read options
-   * @return a {@code Blob} object with latest information or {@code null} if not found
+   * @param options preconditions to use on reload, see <a
+   *     href="https://cloud.google.com/storage/docs/json_api/v1/objects/get">https://cloud.google.com/storage/docs/json_api/v1/objects/get</a>
+   *     for more information.
+   * @return a {@code Blob} object with latest information or {@code null} if no longer exists.
    * @throws StorageException upon failure
    */
   public Blob reload(BlobSourceOption... options) {
-    return storage.get(getBlobId(), toGetOptions(this, options));
+    // BlobId with generation unset is needed to retrieve the latest version of the Blob
+    BlobId idWithoutGeneration = BlobId.of(getBucket(), getName());
+    return storage.get(idWithoutGeneration, toGetOptions(this, options));
   }
 
   /**
@@ -726,12 +743,11 @@ public class Blob extends BlobInfo {
    * <pre>{@code
    * byte[] content = "Hello, World!".getBytes(UTF_8);
    * try (WriteChannel writer = blob.writer()) {
-   *   try {
    *     writer.write(ByteBuffer.wrap(content, 0, content.length));
-   *   } catch (Exception ex) {
-   *     // handle exception
-   *   }
+   * } catch (IOException ex) {
+   *   // handle exception
    * }
+   * blob = blob.reload();
    * }</pre>
    *
    * @param options target blob options
diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/Storage.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/Storage.java
index 38794cd4..e28a9410 100644
--- a/google-cloud-storage/src/main/java/com/google/cloud/storage/Storage.java
+++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/Storage.java
@@ -2503,11 +2503,9 @@ public interface Storage extends Service<StorageOptions> {
    * byte[] content = "Hello, World!".getBytes(UTF_8);
    * BlobInfo blobInfo = BlobInfo.newBuilder(blobId).setContentType("text/plain").build();
    * try (WriteChannel writer = storage.writer(blobInfo)) {
-   *   try {
    *     writer.write(ByteBuffer.wrap(content, 0, content.length));
-   *   } catch (Exception ex) {
-   *     // handle exception
-   *   }
+   * } catch (IOException ex) {
+   *   // handle exception
    * }
    * }</pre>
    *
diff --git a/synth.metadata b/synth.metadata
index b80af4cf..d9f41df1 100644
--- a/synth.metadata
+++ b/synth.metadata
@@ -4,14 +4,14 @@
       "git": {
         "name": ".",
         "remote": "https://github.com/googleapis/java-storage.git",
-        "sha": "8bbaea6b616167b8249b153736c271048ee2844a"
+        "sha": "16487f197ddb047c84d80674b8a60efe13feeceb"
       }
     },
     {
       "git": {
         "name": "synthtool",
         "remote": "https://github.com/googleapis/synthtool.git",
-        "sha": "f8a9933e5e98202b04ef427f28d1d79221190fa4"
+        "sha": "5b48b0716a36ca069db3038da7e205c87a22ed19"
       }
     }
   ]


Test patch:
diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/BlobTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/BlobTest.java
index 51d606c2..7ff384fb 100644
--- a/google-cloud-storage/src/test/java/com/google/cloud/storage/BlobTest.java
+++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/BlobTest.java
@@ -129,7 +129,11 @@ public class BlobTest {
           .setRetentionExpirationTime(RETENTION_EXPIRATION_TIME)
           .build();
   private static final BlobInfo BLOB_INFO =
-      BlobInfo.newBuilder("b", "n").setMetageneration(42L).build();
+      BlobInfo.newBuilder("b", "n", 12345678L).setMetageneration(42L).build();
+  private static final BlobInfo BLOB_INFO_NO_GENERATION =
+      BlobInfo.newBuilder(BLOB_INFO.getBucket(), BLOB_INFO.getName())
+          .setMetageneration(42L)
+          .build();
   private static final BlobInfo DIRECTORY_INFO =
       BlobInfo.newBuilder("b", "n/").setSize(0L).setIsDirectory(true).build();
   private static final String BASE64_KEY = "JVzfVl8NLD9FjedFuStegjRfES5ll5zc59CIXw572OA=";
@@ -233,7 +237,7 @@ public class BlobTest {
     initializeExpectedBlob(2);
     Blob expectedReloadedBlob = expectedBlob.toBuilder().setCacheControl("c").build();
     expect(storage.getOptions()).andReturn(mockOptions);
-    expect(storage.get(BLOB_INFO.getBlobId(), new Storage.BlobGetOption[0]))
+    expect(storage.get(BLOB_INFO_NO_GENERATION.getBlobId(), new Storage.BlobGetOption[0]))
         .andReturn(expectedReloadedBlob);
     replay(storage);
     initializeBlob();
@@ -245,7 +249,8 @@ public class BlobTest {
   public void testReloadNull() throws Exception {
     initializeExpectedBlob(1);
     expect(storage.getOptions()).andReturn(mockOptions);
-    expect(storage.get(BLOB_INFO.getBlobId(), new Storage.BlobGetOption[0])).andReturn(null);
+    expect(storage.get(BLOB_INFO_NO_GENERATION.getBlobId(), new Storage.BlobGetOption[0]))
+        .andReturn(null);
     replay(storage);
     initializeBlob();
     Blob reloadedBlob = blob.reload();
@@ -258,7 +263,8 @@ public class BlobTest {
     Blob expectedReloadedBlob = expectedBlob.toBuilder().setCacheControl("c").build();
     Storage.BlobGetOption[] options = {Storage.BlobGetOption.metagenerationMatch(42L)};
     expect(storage.getOptions()).andReturn(mockOptions);
-    expect(storage.get(BLOB_INFO.getBlobId(), options)).andReturn(expectedReloadedBlob);
+    expect(storage.get(BLOB_INFO_NO_GENERATION.getBlobId(), options))
+        .andReturn(expectedReloadedBlob);
     replay(storage);
     initializeBlob();
     Blob updatedBlob = blob.reload(BlobSourceOption.metagenerationMatch());
@@ -301,8 +307,8 @@ public class BlobTest {
     initializeBlob();
     CopyWriter returnedCopyWriter = blob.copyTo("bt");
     assertEquals(copyWriter, returnedCopyWriter);
-    assertEquals(capturedCopyRequest.getValue().getSource(), blob.getBlobId());
-    assertEquals(capturedCopyRequest.getValue().getTarget(), target);
+    assertEquals(BLOB_INFO_NO_GENERATION.getBlobId(), capturedCopyRequest.getValue().getSource());
+    assertEquals(target, capturedCopyRequest.getValue().getTarget());
     assertFalse(capturedCopyRequest.getValue().overrideInfo());
     assertTrue(capturedCopyRequest.getValue().getSourceOptions().isEmpty());
     assertTrue(capturedCopyRequest.getValue().getTargetOptions().isEmpty());
@@ -320,8 +326,8 @@ public class BlobTest {
     initializeBlob();
     CopyWriter returnedCopyWriter = blob.copyTo("bt", "nt");
     assertEquals(copyWriter, returnedCopyWriter);
-    assertEquals(capturedCopyRequest.getValue().getSource(), blob.getBlobId());
-    assertEquals(capturedCopyRequest.getValue().getTarget(), target);
+    assertEquals(BLOB_INFO_NO_GENERATION.getBlobId(), capturedCopyRequest.getValue().getSource());
+    assertEquals(target, capturedCopyRequest.getValue().getTarget());
     assertFalse(capturedCopyRequest.getValue().overrideInfo());
     assertTrue(capturedCopyRequest.getValue().getSourceOptions().isEmpty());
     assertTrue(capturedCopyRequest.getValue().getTargetOptions().isEmpty());
@@ -340,8 +346,8 @@ public class BlobTest {
     initializeBlob();
     CopyWriter returnedCopyWriter = blob.copyTo(targetId);
     assertEquals(copyWriter, returnedCopyWriter);
-    assertEquals(capturedCopyRequest.getValue().getSource(), blob.getBlobId());
-    assertEquals(capturedCopyRequest.getValue().getTarget(), target);
+    assertEquals(BLOB_INFO_NO_GENERATION.getBlobId(), capturedCopyRequest.getValue().getSource());
+    assertEquals(target, capturedCopyRequest.getValue().getTarget());
     assertFalse(capturedCopyRequest.getValue().overrideInfo());
     assertTrue(capturedCopyRequest.getValue().getSourceOptions().isEmpty());
     assertTrue(capturedCopyRequest.getValue().getTargetOptions().isEmpty());
diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITStorageTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITStorageTest.java
index e1fb5260..2bbb20ce 100644
--- a/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITStorageTest.java
+++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITStorageTest.java
@@ -21,6 +21,7 @@ import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
@@ -3243,4 +3244,33 @@ public class ITStorageTest {
 
     assertEquals("hello world", new String(storage.get(BUCKET, "my-object").getContent()));
   }
+
+  @Test
+  public void testBlobReload() throws Exception {
+    String blobName = "test-blob-reload";
+    BlobId blobId = BlobId.of(BUCKET, blobName);
+    BlobInfo blobInfo = BlobInfo.newBuilder(blobId).build();
+    Blob blob = storage.create(blobInfo, new byte[] {0, 1, 2});
+
+    Blob blobUnchanged = blob.reload();
+    assertEquals(blob, blobUnchanged);
+
+    blob.writer().close();
+    try {
+      blob.reload(Blob.BlobSourceOption.generationMatch());
+      fail("StorageException was expected");
+    } catch (StorageException e) {
+      assertEquals(412, e.getCode());
+      assertEquals("Precondition Failed", e.getMessage());
+    }
+
+    Blob updated = blob.reload();
+    assertEquals(blob.getBucket(), updated.getBucket());
+    assertEquals(blob.getName(), updated.getName());
+    assertNotEquals(blob.getGeneration(), updated.getGeneration());
+    assertEquals(new Long(0), updated.getSize());
+
+    updated.delete();
+    assertNull(updated.reload());
+  }
 }