From 96c55bdc6219800a09c589f411e7801014aef7e2 Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Fri, 5 Mar 2021 09:36:34 -0800 Subject: [PATCH 1/4] fix: npe in create-from --- .../src/main/java/com/google/cloud/storage/StorageImpl.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageImpl.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageImpl.java index c3778a8175..32b3311673 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageImpl.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageImpl.java @@ -264,6 +264,9 @@ public Blob createFrom( uploadHelper(Channels.newChannel(content), writer, bufferSize); } StorageObject objectProto = blobWriteChannel.getStorageObject(); + if (objectProto == null) { + return get(blobInfo.getBlobId(), null); + } return Blob.fromPb(this, objectProto); } From 2914587b6756d5a9e6793af468bd629cc9d8618a Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Mon, 8 Mar 2021 10:55:08 -0800 Subject: [PATCH 2/4] address NPE issue --- .../cloud/storage/BlobWriteChannel.java | 4 +- .../com/google/cloud/storage/StorageImpl.java | 3 -- .../cloud/storage/it/ITStorageTest.java | 39 ++++++++++++------- 3 files changed, 28 insertions(+), 18 deletions(-) diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/BlobWriteChannel.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/BlobWriteChannel.java index aa5c3a8118..1ce6f8e384 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/BlobWriteChannel.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/BlobWriteChannel.java @@ -25,6 +25,8 @@ import com.google.cloud.RetryHelper; import com.google.cloud.WriteChannel; import com.google.cloud.storage.spi.v1.StorageRpc; +import com.google.common.collect.Maps; + import java.net.URL; import java.util.Map; import java.util.concurrent.Callable; @@ -77,7 +79,7 @@ private long getRemotePosition() { } private StorageObject getRemoteStorageObject() { - return getOptions().getStorageRpcV1().get(getEntity().toPb(), null); + return getOptions().getStorageRpcV1().get(getEntity().toPb(), Maps.newEnumMap(StorageRpc.Option.class)); } private StorageException unrecoverableState( diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageImpl.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageImpl.java index 32b3311673..c3778a8175 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageImpl.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageImpl.java @@ -264,9 +264,6 @@ public Blob createFrom( uploadHelper(Channels.newChannel(content), writer, bufferSize); } StorageObject objectProto = blobWriteChannel.getStorageObject(); - if (objectProto == null) { - return get(blobInfo.getBlobId(), null); - } return Blob.fromPb(this, objectProto); } 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 80685a2775..a512a6b191 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 @@ -3404,21 +3404,32 @@ public void testDeleteLifecycleRules() throws ExecutionException, InterruptedExc @Test public void testUploadFromDownloadTo() throws Exception { - String blobName = "test-uploadFrom-downloadTo-blob"; - BlobId blobId = BlobId.of(BUCKET, blobName); - BlobInfo blobInfo = BlobInfo.newBuilder(blobId).build(); - Path tempFileFrom = Files.createTempFile("ITStorageTest_", ".tmp"); - Files.write(tempFileFrom, BLOB_BYTE_CONTENT); - Blob blob = storage.createFrom(blobInfo, tempFileFrom); - assertEquals(BUCKET, blob.getBucket()); - assertEquals(blobName, blob.getName()); - assertEquals(BLOB_BYTE_CONTENT.length, (long) blob.getSize()); - - Path tempFileTo = Files.createTempFile("ITStorageTest_", ".tmp"); - storage.get(blobId).downloadTo(tempFileTo); - byte[] readBytes = Files.readAllBytes(tempFileTo); - assertArrayEquals(BLOB_BYTE_CONTENT, readBytes); + Random rnd = new Random(); + byte[] bytes = new byte[1024*1024*30]; // 30MiB + rnd.nextBytes(bytes); + Files.write(tempFileFrom, bytes); + final int iterations = 10; + final int totalBytes = bytes.length * iterations; + long startTime = System.nanoTime(); + for (int i = 0; i < iterations; i++) { + String blobName = "test-uploadFrom-downloadTo-blob"; + BlobId blobId = BlobId.of(BUCKET, blobName); + BlobInfo blobInfo = BlobInfo.newBuilder(blobId).build(); + + Blob blob = storage.createFrom(blobInfo, tempFileFrom); + assertEquals(BUCKET, blob.getBucket()); + assertEquals(blobName, blob.getName()); + assertEquals(bytes.length, (long) blob.getSize()); + +// Path tempFileTo = Files.createTempFile("ITStorageTest_", ".tmp"); +// storage.get(blobId).downloadTo(tempFileTo); +// byte[] readBytes = Files.readAllBytes(tempFileTo); +// assertArrayEquals(BLOB_BYTE_CONTENT, readBytes); + } + long endTime = System.nanoTime(); + double deltaInSeconds = (endTime - startTime) / 1_000_000_000.0; + System.out.println((8*totalBytes/deltaInSeconds)/(1024*1024)); } @Test From d0e998244dbb09f658e603b4d90589eb6c5bba83 Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Mon, 8 Mar 2021 10:58:52 -0800 Subject: [PATCH 3/4] revert test it --- .../cloud/storage/it/ITStorageTest.java | 39 +++++++------------ 1 file changed, 14 insertions(+), 25 deletions(-) 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 a512a6b191..80685a2775 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 @@ -3404,32 +3404,21 @@ public void testDeleteLifecycleRules() throws ExecutionException, InterruptedExc @Test public void testUploadFromDownloadTo() throws Exception { - Path tempFileFrom = Files.createTempFile("ITStorageTest_", ".tmp"); - Random rnd = new Random(); - byte[] bytes = new byte[1024*1024*30]; // 30MiB - rnd.nextBytes(bytes); - Files.write(tempFileFrom, bytes); - final int iterations = 10; - final int totalBytes = bytes.length * iterations; - long startTime = System.nanoTime(); - for (int i = 0; i < iterations; i++) { - String blobName = "test-uploadFrom-downloadTo-blob"; - BlobId blobId = BlobId.of(BUCKET, blobName); - BlobInfo blobInfo = BlobInfo.newBuilder(blobId).build(); - - Blob blob = storage.createFrom(blobInfo, tempFileFrom); - assertEquals(BUCKET, blob.getBucket()); - assertEquals(blobName, blob.getName()); - assertEquals(bytes.length, (long) blob.getSize()); + String blobName = "test-uploadFrom-downloadTo-blob"; + BlobId blobId = BlobId.of(BUCKET, blobName); + BlobInfo blobInfo = BlobInfo.newBuilder(blobId).build(); -// Path tempFileTo = Files.createTempFile("ITStorageTest_", ".tmp"); -// storage.get(blobId).downloadTo(tempFileTo); -// byte[] readBytes = Files.readAllBytes(tempFileTo); -// assertArrayEquals(BLOB_BYTE_CONTENT, readBytes); - } - long endTime = System.nanoTime(); - double deltaInSeconds = (endTime - startTime) / 1_000_000_000.0; - System.out.println((8*totalBytes/deltaInSeconds)/(1024*1024)); + Path tempFileFrom = Files.createTempFile("ITStorageTest_", ".tmp"); + Files.write(tempFileFrom, BLOB_BYTE_CONTENT); + Blob blob = storage.createFrom(blobInfo, tempFileFrom); + assertEquals(BUCKET, blob.getBucket()); + assertEquals(blobName, blob.getName()); + assertEquals(BLOB_BYTE_CONTENT.length, (long) blob.getSize()); + + Path tempFileTo = Files.createTempFile("ITStorageTest_", ".tmp"); + storage.get(blobId).downloadTo(tempFileTo); + byte[] readBytes = Files.readAllBytes(tempFileTo); + assertArrayEquals(BLOB_BYTE_CONTENT, readBytes); } @Test From 411b636dd0a3d86e6099b0cf2394718ca7f7414d Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Mon, 8 Mar 2021 11:34:34 -0800 Subject: [PATCH 4/4] address unit tests --- .../java/com/google/cloud/storage/BlobWriteChannel.java | 5 +++-- .../com/google/cloud/storage/BlobWriteChannelTest.java | 8 +++++--- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/BlobWriteChannel.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/BlobWriteChannel.java index 1ce6f8e384..09bc17081a 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/BlobWriteChannel.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/BlobWriteChannel.java @@ -26,7 +26,6 @@ import com.google.cloud.WriteChannel; import com.google.cloud.storage.spi.v1.StorageRpc; import com.google.common.collect.Maps; - import java.net.URL; import java.util.Map; import java.util.concurrent.Callable; @@ -79,7 +78,9 @@ private long getRemotePosition() { } private StorageObject getRemoteStorageObject() { - return getOptions().getStorageRpcV1().get(getEntity().toPb(), Maps.newEnumMap(StorageRpc.Option.class)); + return getOptions() + .getStorageRpcV1() + .get(getEntity().toPb(), Maps.newEnumMap(StorageRpc.Option.class)); } private StorageException unrecoverableState( diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/BlobWriteChannelTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/BlobWriteChannelTest.java index ba60a3c4d6..d74e87d640 100644 --- a/google-cloud-storage/src/test/java/com/google/cloud/storage/BlobWriteChannelTest.java +++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/BlobWriteChannelTest.java @@ -40,6 +40,7 @@ import com.google.cloud.storage.spi.StorageRpcFactory; import com.google.cloud.storage.spi.v1.StorageRpc; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Maps; import java.io.IOException; import java.math.BigInteger; import java.net.MalformedURLException; @@ -333,9 +334,10 @@ public void testWriteWithRetryAndObjectMetadata() throws IOException { .andThrow(socketClosedException); expect(storageRpcMock.getCurrentUploadOffset(eq(UPLOAD_ID))).andReturn(-1L); expect(storageRpcMock.getCurrentUploadOffset(eq(UPLOAD_ID))).andReturn(-1L); - expect(storageRpcMock.get(BLOB_INFO.toPb(), null)).andThrow(socketClosedException); + expect(storageRpcMock.get(BLOB_INFO.toPb(), Maps.newEnumMap(StorageRpc.Option.class))) + .andThrow(socketClosedException); expect(storageRpcMock.getCurrentUploadOffset(eq(UPLOAD_ID))).andReturn(-1L); - expect(storageRpcMock.get(BLOB_INFO.toPb(), null)) + expect(storageRpcMock.get(BLOB_INFO.toPb(), Maps.newEnumMap(StorageRpc.Option.class))) .andReturn(BLOB_INFO.toPb().setSize(BigInteger.valueOf(MIN_CHUNK_SIZE))); replay(storageRpcMock); writer = new BlobWriteChannel(options, BLOB_INFO, EMPTY_RPC_OPTIONS); @@ -485,7 +487,7 @@ public void testWriteWithLastFlushRetryChunkButCompleted() throws IOException { eq(true))) .andThrow(socketClosedException); expect(storageRpcMock.getCurrentUploadOffset(eq(UPLOAD_ID))).andReturn(-1L); - expect(storageRpcMock.get(BLOB_INFO.toPb(), null)) + expect(storageRpcMock.get(BLOB_INFO.toPb(), Maps.newEnumMap(StorageRpc.Option.class))) .andReturn(BLOB_INFO.toPb().setSize(BigInteger.valueOf(MIN_CHUNK_SIZE))); replay(storageRpcMock); writer = new BlobWriteChannel(options, BLOB_INFO, EMPTY_RPC_OPTIONS);