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

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

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

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

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

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

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

Author: dmitry-fa

google-cloud-storage/src/main/java/com/google/cloud/storage/Blob.java

@@ -525,24 +525,41 @@ public byte[] getContent(BlobSourceOption... options) {
   }
 
   /**
-   * 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));
   }
 
   /**
@@ -730,6 +747,7 @@ public ReadChannel reader(BlobSourceOption... options) {
    * } catch (IOException ex) {
    *   // handle exception
    * }
+   * blob = blob.reload();
    * }</pre>
    *
    * @param options target blob options

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 void testReload() throws Exception {
     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 void testReload() throws Exception {
   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 void testReloadWithOptions() throws Exception {
     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 void testCopyToBucket() throws Exception {
     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 void testCopyTo() throws Exception {
     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 void testCopyToBlobId() throws Exception {
     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());

google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITStorageTest.java

@@ -21,6 +21,7 @@
 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 void testSignedPostPolicyV4() throws Exception {
 
     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());
+  }
 }