From bf11479688b6cad4b7212ca6991b06512f2d5d22 Mon Sep 17 00:00:00 2001 From: andrey-qlogic Date: Mon, 14 Jan 2019 17:27:15 +0000 Subject: [PATCH 01/11] 3929: Added downloadToPathWithMediaHttpDownloader method to Blob class --- .../java/com/google/cloud/storage/Blob.java | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/Blob.java b/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/Blob.java index 7cdf742ef31e..18785a4fa120 100644 --- a/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/Blob.java +++ b/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/Blob.java @@ -16,16 +16,23 @@ package com.google.cloud.storage; +import static com.google.cloud.RetryHelper.runWithRetries; import static com.google.cloud.storage.Blob.BlobSourceOption.toGetOptions; import static com.google.cloud.storage.Blob.BlobSourceOption.toSourceOptions; import static com.google.common.base.Preconditions.checkNotNull; +import com.google.api.client.http.HttpRequestInitializer; +import com.google.api.client.http.HttpTransport; +import com.google.api.client.json.jackson2.JacksonFactory; import com.google.api.services.storage.model.StorageObject; import com.google.auth.ServiceAccountSigner; import com.google.auth.ServiceAccountSigner.SigningException; +import com.google.cloud.BaseService; import com.google.cloud.ReadChannel; +import com.google.cloud.RetryHelper; import com.google.cloud.Tuple; import com.google.cloud.WriteChannel; +import com.google.cloud.http.HttpTransportOptions; import com.google.cloud.storage.Acl.Entity; import com.google.cloud.storage.Storage.BlobTargetOption; import com.google.cloud.storage.Storage.BlobWriteOption; @@ -34,6 +41,7 @@ import com.google.cloud.storage.spi.v1.StorageRpc; import com.google.common.base.Function; import com.google.common.io.BaseEncoding; +import com.google.common.io.CountingOutputStream; import java.io.IOException; import java.io.ObjectInputStream; import java.io.OutputStream; @@ -48,6 +56,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.concurrent.Callable; import java.util.concurrent.TimeUnit; /** @@ -225,6 +234,62 @@ public void downloadTo(Path path, BlobSourceOption... options) { } } + /** + * Builds com.google.api.services.storage.Storage using storage options + * + * @param options storage options + * @return Storage + */ + private com.google.api.services.storage.Storage buildStorage(StorageOptions options) { + HttpTransportOptions transportOptions = (HttpTransportOptions) options.getTransportOptions(); + HttpTransport transport = transportOptions.getHttpTransportFactory().create(); + HttpRequestInitializer initializer = transportOptions.getHttpRequestInitializer(options); + + return new com.google.api.services.storage.Storage.Builder( + transport, JacksonFactory.getDefaultInstance(), initializer) + .setRootUrl(options.getHost()) + .setApplicationName(options.getApplicationName()) + .build(); + } + + /** + * Downloads this blob to the given file path using specified storage options. + * + * @param path destination + * @param options storage options + * @throws RetryHelper.RetryHelperException upon failure + * @throws StorageException upon failure + */ + public void downloadToPathWithMediaHttpDownloader(Path path, StorageOptions options) { + com.google.api.services.storage.Storage storage = buildStorage(options); + try { + final com.google.api.services.storage.Storage.Objects.Get getOperation = + storage.objects().get(this.getBucket(), this.getName()); + getOperation.getMediaHttpDownloader().setDirectDownloadEnabled(true); + final CountingOutputStream out = new CountingOutputStream(Files.newOutputStream(path)); + runWithRetries( + new Callable() { + @Override + public Void call() throws Exception { + try { + getOperation.getMediaHttpDownloader().setBytesDownloaded(out.getCount()); + getOperation.executeMediaAndDownloadTo(out); + return null; + } catch (IOException e) { + throw new StorageException(e); + } + }; + }, + options.getRetrySettings(), + BaseService.EXCEPTION_HANDLER, + options.getClock()); + } catch (RetryHelper.RetryHelperException e) { + throw StorageException.translateAndThrow(e); + } catch (IOException e) { + throw new StorageException(e); + } + } + /** * Downloads this blob to the given file path. * From dc468d4a3f7ea419299b765c064a25452631be96 Mon Sep 17 00:00:00 2001 From: andrey-qlogic Date: Mon, 14 Jan 2019 18:30:18 +0000 Subject: [PATCH 02/11] 3929: Added period at the end of the sentence --- .../src/main/java/com/google/cloud/storage/Blob.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/Blob.java b/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/Blob.java index 18785a4fa120..bf6a84826a82 100644 --- a/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/Blob.java +++ b/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/Blob.java @@ -235,7 +235,7 @@ public void downloadTo(Path path, BlobSourceOption... options) { } /** - * Builds com.google.api.services.storage.Storage using storage options + * Builds com.google.api.services.storage.Storage using storage options. * * @param options storage options * @return Storage From aba57f9113858bf7dd97d48d0fa13705d429adb9 Mon Sep 17 00:00:00 2001 From: andrey-qlogic Date: Thu, 24 Jan 2019 19:15:23 +0000 Subject: [PATCH 03/11] 3929: Added option USE_DIRECT_DOWNLOAD. Prepared tests. --- .../java/com/google/cloud/storage/Blob.java | 80 ++++--------------- .../com/google/cloud/storage/BlobInfo.java | 14 +++- .../com/google/cloud/storage/Storage.java | 8 ++ .../cloud/storage/spi/v1/HttpStorageRpc.java | 2 + .../cloud/storage/spi/v1/StorageRpc.java | 1 + .../com/google/cloud/storage/BlobTest.java | 37 +++++++++ .../cloud/storage/it/ITStorageTest.java | 32 ++++++++ 7 files changed, 108 insertions(+), 66 deletions(-) diff --git a/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/Blob.java b/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/Blob.java index bf6a84826a82..c8dffef2ab8b 100644 --- a/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/Blob.java +++ b/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/Blob.java @@ -16,23 +16,16 @@ package com.google.cloud.storage; -import static com.google.cloud.RetryHelper.runWithRetries; import static com.google.cloud.storage.Blob.BlobSourceOption.toGetOptions; import static com.google.cloud.storage.Blob.BlobSourceOption.toSourceOptions; import static com.google.common.base.Preconditions.checkNotNull; -import com.google.api.client.http.HttpRequestInitializer; -import com.google.api.client.http.HttpTransport; -import com.google.api.client.json.jackson2.JacksonFactory; import com.google.api.services.storage.model.StorageObject; import com.google.auth.ServiceAccountSigner; import com.google.auth.ServiceAccountSigner.SigningException; -import com.google.cloud.BaseService; import com.google.cloud.ReadChannel; -import com.google.cloud.RetryHelper; import com.google.cloud.Tuple; import com.google.cloud.WriteChannel; -import com.google.cloud.http.HttpTransportOptions; import com.google.cloud.storage.Acl.Entity; import com.google.cloud.storage.Storage.BlobTargetOption; import com.google.cloud.storage.Storage.BlobWriteOption; @@ -41,7 +34,6 @@ import com.google.cloud.storage.spi.v1.StorageRpc; import com.google.common.base.Function; import com.google.common.io.BaseEncoding; -import com.google.common.io.CountingOutputStream; import java.io.IOException; import java.io.ObjectInputStream; import java.io.OutputStream; @@ -56,7 +48,6 @@ import java.util.List; import java.util.Map; import java.util.Objects; -import java.util.concurrent.Callable; import java.util.concurrent.TimeUnit; /** @@ -109,6 +100,8 @@ private Storage.BlobSourceOption toSourceOptions(BlobInfo blobInfo) { return Storage.BlobSourceOption.metagenerationNotMatch(blobInfo.getMetageneration()); case CUSTOMER_SUPPLIED_KEY: return Storage.BlobSourceOption.decryptionKey((String) getValue()); + case USE_DIRECT_DOWNLOAD: + return Storage.BlobSourceOption.useDirectDownload((Boolean) getValue()); case USER_PROJECT: return Storage.BlobSourceOption.userProject((String) getValue()); default: @@ -184,6 +177,13 @@ public static BlobSourceOption decryptionKey(String key) { return new BlobSourceOption(StorageRpc.Option.CUSTOMER_SUPPLIED_KEY, key); } + /** + * Returns an option to download blobs directly when you use the MediaHttpDownloader + */ + public static BlobSourceOption useDirectDownload(boolean useDirectDownload) { + return new BlobSourceOption(StorageRpc.Option.USE_DIRECT_DOWNLOAD, useDirectDownload); + } + /** * Returns an option for blob's billing user project. This option is used only if the blob's * bucket has requester_pays flag enabled. @@ -234,62 +234,6 @@ public void downloadTo(Path path, BlobSourceOption... options) { } } - /** - * Builds com.google.api.services.storage.Storage using storage options. - * - * @param options storage options - * @return Storage - */ - private com.google.api.services.storage.Storage buildStorage(StorageOptions options) { - HttpTransportOptions transportOptions = (HttpTransportOptions) options.getTransportOptions(); - HttpTransport transport = transportOptions.getHttpTransportFactory().create(); - HttpRequestInitializer initializer = transportOptions.getHttpRequestInitializer(options); - - return new com.google.api.services.storage.Storage.Builder( - transport, JacksonFactory.getDefaultInstance(), initializer) - .setRootUrl(options.getHost()) - .setApplicationName(options.getApplicationName()) - .build(); - } - - /** - * Downloads this blob to the given file path using specified storage options. - * - * @param path destination - * @param options storage options - * @throws RetryHelper.RetryHelperException upon failure - * @throws StorageException upon failure - */ - public void downloadToPathWithMediaHttpDownloader(Path path, StorageOptions options) { - com.google.api.services.storage.Storage storage = buildStorage(options); - try { - final com.google.api.services.storage.Storage.Objects.Get getOperation = - storage.objects().get(this.getBucket(), this.getName()); - getOperation.getMediaHttpDownloader().setDirectDownloadEnabled(true); - final CountingOutputStream out = new CountingOutputStream(Files.newOutputStream(path)); - runWithRetries( - new Callable() { - @Override - public Void call() throws Exception { - try { - getOperation.getMediaHttpDownloader().setBytesDownloaded(out.getCount()); - getOperation.executeMediaAndDownloadTo(out); - return null; - } catch (IOException e) { - throw new StorageException(e); - } - }; - }, - options.getRetrySettings(), - BaseService.EXCEPTION_HANDLER, - options.getClock()); - } catch (RetryHelper.RetryHelperException e) { - throw StorageException.translateAndThrow(e); - } catch (IOException e) { - throw new StorageException(e); - } - } - /** * Downloads this blob to the given file path. * @@ -470,6 +414,12 @@ public Builder setEventBasedHold(Boolean eventBasedHold) { return this; } + @Override + public Builder setUseDirectDownload(Boolean useDirectDownload) { + infoBuilder.setUseDirectDownload(useDirectDownload); + return this; + } + @Override public Builder setTemporaryHold(Boolean temporaryHold) { infoBuilder.setTemporaryHold(temporaryHold); diff --git a/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/BlobInfo.java b/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/BlobInfo.java index 2ace22d6f9f1..dad1da8ad544 100644 --- a/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/BlobInfo.java +++ b/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/BlobInfo.java @@ -25,7 +25,6 @@ import com.google.api.services.storage.model.ObjectAccessControl; import com.google.api.services.storage.model.StorageObject; import com.google.api.services.storage.model.StorageObject.Owner; -import com.google.cloud.storage.Blob.Builder; import com.google.common.base.Function; import com.google.common.base.MoreObjects; import com.google.common.collect.ImmutableList; @@ -86,6 +85,7 @@ public StorageObject apply(BlobInfo blobInfo) { private final String kmsKeyName; private final Boolean eventBasedHold; private final Boolean temporaryHold; + private final Boolean useDirectDownload; private final Long retentionExpirationTime; /** This class is meant for internal use only. Users are discouraged from using this class. */ @@ -266,6 +266,9 @@ public abstract static class Builder { @BetaApi public abstract Builder setTemporaryHold(Boolean temporaryHold); + @BetaApi + public abstract Builder setUseDirectDownload(Boolean useDirectDownload); + @BetaApi abstract Builder setRetentionExpirationTime(Long retentionExpirationTime); @@ -302,6 +305,7 @@ static final class BuilderImpl extends Builder { private String kmsKeyName; private Boolean eventBasedHold; private Boolean temporaryHold; + private Boolean useDirectDownload; private Long retentionExpirationTime; BuilderImpl(BlobId blobId) { @@ -336,6 +340,7 @@ static final class BuilderImpl extends Builder { kmsKeyName = blobInfo.kmsKeyName; eventBasedHold = blobInfo.eventBasedHold; temporaryHold = blobInfo.temporaryHold; + useDirectDownload = blobInfo.useDirectDownload; retentionExpirationTime = blobInfo.retentionExpirationTime; } @@ -504,6 +509,12 @@ public Builder setTemporaryHold(Boolean temporaryHold) { return this; } + @Override + public Builder setUseDirectDownload(Boolean useDirectDownload) { + this.useDirectDownload = useDirectDownload; + return this; + } + @Override Builder setRetentionExpirationTime(Long retentionExpirationTime) { this.retentionExpirationTime = retentionExpirationTime; @@ -545,6 +556,7 @@ public BlobInfo build() { kmsKeyName = builder.kmsKeyName; eventBasedHold = builder.eventBasedHold; temporaryHold = builder.temporaryHold; + useDirectDownload = builder.useDirectDownload; retentionExpirationTime = builder.retentionExpirationTime; } diff --git a/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/Storage.java b/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/Storage.java index 6ce9ca8c6e2a..9140e80100e8 100644 --- a/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/Storage.java +++ b/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/Storage.java @@ -598,6 +598,14 @@ public static BlobSourceOption generationNotMatch() { return new BlobSourceOption(StorageRpc.Option.IF_GENERATION_NOT_MATCH, null); } + /** + * Returns an option for blob's downloading. If this option is used the request will use + * directDownload option for MediaMediaDownloader. + */ + public static BlobSourceOption useDirectDownload(boolean useDirectDownload) { + return new BlobSourceOption(StorageRpc.Option.USE_DIRECT_DOWNLOAD, useDirectDownload); + } + /** * Returns an option for blob's data generation mismatch. If this option is used the request * will fail if blob's generation matches the provided value. diff --git a/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/spi/v1/HttpStorageRpc.java b/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/spi/v1/HttpStorageRpc.java index a10b4ef8a973..2abb24b0e0d5 100644 --- a/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/spi/v1/HttpStorageRpc.java +++ b/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/spi/v1/HttpStorageRpc.java @@ -643,6 +643,8 @@ public Tuple read( .setIfGenerationMatch(Option.IF_GENERATION_MATCH.getLong(options)) .setIfGenerationNotMatch(Option.IF_GENERATION_NOT_MATCH.getLong(options)) .setUserProject(Option.USER_PROJECT.getString(options)); + req.getMediaHttpDownloader() + .setDirectDownloadEnabled(Option.USE_DIRECT_DOWNLOAD.getBoolean(options)); checkArgument(position >= 0, "Position should be non-negative, is %d", position); StringBuilder range = new StringBuilder(); range.append("bytes=").append(position).append("-").append(position + bytes - 1); diff --git a/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/spi/v1/StorageRpc.java b/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/spi/v1/StorageRpc.java index 447643283d94..553920da0b9a 100644 --- a/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/spi/v1/StorageRpc.java +++ b/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/spi/v1/StorageRpc.java @@ -56,6 +56,7 @@ enum Option { VERSIONS("versions"), FIELDS("fields"), CUSTOMER_SUPPLIED_KEY("customerSuppliedKey"), + USE_DIRECT_DOWNLOAD("useDirectDownload"), USER_PROJECT("userProject"), KMS_KEY_NAME("kmsKeyName"); diff --git a/google-cloud-clients/google-cloud-storage/src/test/java/com/google/cloud/storage/BlobTest.java b/google-cloud-clients/google-cloud-storage/src/test/java/com/google/cloud/storage/BlobTest.java index ff517d360dd9..710440721620 100644 --- a/google-cloud-clients/google-cloud-storage/src/test/java/com/google/cloud/storage/BlobTest.java +++ b/google-cloud-clients/google-cloud-storage/src/test/java/com/google/cloud/storage/BlobTest.java @@ -123,6 +123,8 @@ public class BlobTest { .build(); private static final BlobInfo BLOB_INFO = BlobInfo.newBuilder("b", "n").setMetageneration(42L).build(); + private static final BlobInfo BLOB_INFO_USE_DIRECT_DOWNLOAD = + BlobInfo.newBuilder("b", "n").setMetageneration(42L).setUseDirectDownload(true).build(); private static final BlobInfo DIRECTORY_INFO = BlobInfo.newBuilder("b", "n/").setSize(0L).setIsDirectory(true).build(); private static final String BASE64_KEY = "JVzfVl8NLD9FjedFuStegjRfES5ll5zc59CIXw572OA="; @@ -587,4 +589,39 @@ public Integer answer() throws Throwable { byte actual[] = Files.readAllBytes(file.toPath()); assertArrayEquals(expected, actual); } + + @Test + public void testDownloadWithUseDirectDownload() throws Exception { + final byte[] expected = {1, 2}; + + initializeExpectedBlob(2); + ReadChannel channel = createNiceMock(ReadChannel.class); + expect(storage.getOptions()).andReturn(mockOptions); + expect( + storage.reader( + BLOB_INFO_USE_DIRECT_DOWNLOAD.getBlobId(), + Storage.BlobSourceOption.useDirectDownload(true))) + .andReturn(channel); + replay(storage); + // First read should return 2 bytes. + expect(channel.read(anyObject(ByteBuffer.class))) + .andAnswer( + new IAnswer() { + @Override + public Integer answer() throws Throwable { + // Modify the argument to match the expected behavior of `read`. + ((ByteBuffer) getCurrentArguments()[0]).put(expected); + return 2; + } + }); + // Second read should return 0 bytes. + expect(channel.read(anyObject(ByteBuffer.class))).andReturn(0); + replay(channel); + initializeBlob(); + + File file = File.createTempFile("blob", ".tmp"); + blob.downloadTo(file.toPath(), BlobSourceOption.useDirectDownload(true)); + byte actual[] = Files.readAllBytes(file.toPath()); + assertArrayEquals(expected, actual); + } } diff --git a/google-cloud-clients/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITStorageTest.java b/google-cloud-clients/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITStorageTest.java index 3a97dab2800d..244493297bb1 100644 --- a/google-cloud-clients/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITStorageTest.java +++ b/google-cloud-clients/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITStorageTest.java @@ -1699,6 +1699,38 @@ public void testReadChannelFail() throws IOException { } } + @Test + public void testReadChannelWithUseDirectDownload() throws IOException { + String blobName = "test-read-channel-blob-direct-download"; + BlobInfo blob = BlobInfo.newBuilder(BUCKET, blobName).build(); + Blob remoteBlob = storage.create(blob); + assertNotNull(remoteBlob); + try (ReadChannel reader = + storage.reader(blob.getBlobId(), Storage.BlobSourceOption.useDirectDownload(true))) { + reader.read(ByteBuffer.allocate(42)); + } catch (StorageException ex) { + // expected + } + assertEquals(blob.getBucket(), remoteBlob.getBucket()); + assertEquals(blob.getName(), remoteBlob.getName()); + } + + @Test + public void testReadChannelWithNoUseDirectDownload() throws IOException { + String blobName = "test-read-channel-blob-direct-download"; + BlobInfo blob = BlobInfo.newBuilder(BUCKET, blobName).build(); + Blob remoteBlob = storage.create(blob); + assertNotNull(remoteBlob); + try (ReadChannel reader = + storage.reader(blob.getBlobId(), Storage.BlobSourceOption.useDirectDownload(false))) { + reader.read(ByteBuffer.allocate(42)); + } catch (StorageException ex) { + // expected + } + assertEquals(blob.getBucket(), remoteBlob.getBucket()); + assertEquals(blob.getName(), remoteBlob.getName()); + } + @Test public void testReadChannelFailUpdatedGeneration() throws IOException { String blobName = "test-read-blob-fail-updated-generation"; From 110be9ad2ab059e2c2247a1cf16b770477dab71b Mon Sep 17 00:00:00 2001 From: andrey-qlogic Date: Thu, 24 Jan 2019 19:25:18 +0000 Subject: [PATCH 04/11] 3929: Fixed typo. --- .../src/main/java/com/google/cloud/storage/Storage.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/Storage.java b/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/Storage.java index 9140e80100e8..c4caf365cdad 100644 --- a/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/Storage.java +++ b/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/Storage.java @@ -600,7 +600,7 @@ public static BlobSourceOption generationNotMatch() { /** * Returns an option for blob's downloading. If this option is used the request will use - * directDownload option for MediaMediaDownloader. + * directDownload option for MediaHttpDownloader. */ public static BlobSourceOption useDirectDownload(boolean useDirectDownload) { return new BlobSourceOption(StorageRpc.Option.USE_DIRECT_DOWNLOAD, useDirectDownload); From 6d09faedf89376783f905d2689aad2b181df5774 Mon Sep 17 00:00:00 2001 From: andrey-qlogic Date: Thu, 24 Jan 2019 20:47:33 +0000 Subject: [PATCH 05/11] 3929: Changes after review. --- .../src/main/java/com/google/cloud/storage/Blob.java | 5 ++++- .../src/test/java/com/google/cloud/storage/BlobTest.java | 4 +--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/Blob.java b/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/Blob.java index c8dffef2ab8b..df7c89e6fbfc 100644 --- a/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/Blob.java +++ b/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/Blob.java @@ -178,7 +178,10 @@ public static BlobSourceOption decryptionKey(String key) { } /** - * Returns an option to download blobs directly when you use the MediaHttpDownloader + * Returns the option to enable or disable direct download. If the value is set to {@code true}, + * then a direct download will be performed when all the blob content is loaded in one request. + * If {@code false} is specified, the download uses a resumed download protocol to load data + * into chunks. */ public static BlobSourceOption useDirectDownload(boolean useDirectDownload) { return new BlobSourceOption(StorageRpc.Option.USE_DIRECT_DOWNLOAD, useDirectDownload); diff --git a/google-cloud-clients/google-cloud-storage/src/test/java/com/google/cloud/storage/BlobTest.java b/google-cloud-clients/google-cloud-storage/src/test/java/com/google/cloud/storage/BlobTest.java index 710440721620..8fa6be150c95 100644 --- a/google-cloud-clients/google-cloud-storage/src/test/java/com/google/cloud/storage/BlobTest.java +++ b/google-cloud-clients/google-cloud-storage/src/test/java/com/google/cloud/storage/BlobTest.java @@ -123,8 +123,6 @@ public class BlobTest { .build(); private static final BlobInfo BLOB_INFO = BlobInfo.newBuilder("b", "n").setMetageneration(42L).build(); - private static final BlobInfo BLOB_INFO_USE_DIRECT_DOWNLOAD = - BlobInfo.newBuilder("b", "n").setMetageneration(42L).setUseDirectDownload(true).build(); private static final BlobInfo DIRECTORY_INFO = BlobInfo.newBuilder("b", "n/").setSize(0L).setIsDirectory(true).build(); private static final String BASE64_KEY = "JVzfVl8NLD9FjedFuStegjRfES5ll5zc59CIXw572OA="; @@ -599,7 +597,7 @@ public void testDownloadWithUseDirectDownload() throws Exception { expect(storage.getOptions()).andReturn(mockOptions); expect( storage.reader( - BLOB_INFO_USE_DIRECT_DOWNLOAD.getBlobId(), + BLOB_INFO.getBlobId(), Storage.BlobSourceOption.useDirectDownload(true))) .andReturn(channel); replay(storage); From 7b6f476e31bcdde82f23a941e0b19a5e41dd3d45 Mon Sep 17 00:00:00 2001 From: andrey-qlogic Date: Tue, 5 Feb 2019 18:21:17 +0000 Subject: [PATCH 06/11] 3929: WIP --- .../java/com/google/cloud/storage/Blob.java | 49 +++++++++++++++++++ .../cloud/storage/spi/v1/HttpStorageRpc.java | 29 +++++++++++ .../cloud/storage/spi/v1/StorageRpc.java | 10 ++++ .../com/google/cloud/storage/BlobTest.java | 34 ------------- 4 files changed, 88 insertions(+), 34 deletions(-) diff --git a/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/Blob.java b/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/Blob.java index df7c89e6fbfc..2c681056663f 100644 --- a/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/Blob.java +++ b/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/Blob.java @@ -16,14 +16,17 @@ package com.google.cloud.storage; +import static com.google.cloud.RetryHelper.runWithRetries; import static com.google.cloud.storage.Blob.BlobSourceOption.toGetOptions; import static com.google.cloud.storage.Blob.BlobSourceOption.toSourceOptions; import static com.google.common.base.Preconditions.checkNotNull; +import static java.util.concurrent.Executors.callable; import com.google.api.services.storage.model.StorageObject; import com.google.auth.ServiceAccountSigner; import com.google.auth.ServiceAccountSigner.SigningException; import com.google.cloud.ReadChannel; +import com.google.cloud.RetryHelper; import com.google.cloud.Tuple; import com.google.cloud.WriteChannel; import com.google.cloud.storage.Acl.Entity; @@ -34,6 +37,7 @@ import com.google.cloud.storage.spi.v1.StorageRpc; import com.google.common.base.Function; import com.google.common.io.BaseEncoding; +import com.google.common.io.CountingOutputStream; import java.io.IOException; import java.io.ObjectInputStream; import java.io.OutputStream; @@ -237,6 +241,41 @@ public void downloadTo(Path path, BlobSourceOption... options) { } } + /** + * Downloads this blob to the given output stream path using specified blob read options. + * + * @param outputStream destination + * @param options blob read options + * @throws StorageException upon failure + */ + public void downloadTo(OutputStream outputStream, final BlobSourceOption... options) { + try (CountingOutputStream countingOutputStream = new CountingOutputStream(outputStream)) { + final StorageObject storageObject = getBlobId().toPb(); + final StorageOptions storageOptions = this.options; + final StorageRpc storageRpc = storageOptions.getStorageRpcV1(); + final Map requestOptions = StorageImpl.optionMap(getBlobId(), options); + runWithRetries( + callable( + new Runnable() { + @Override + public void run() { + storageRpc.readToOutputStream( + storageObject, + countingOutputStream.getCount(), + countingOutputStream, + requestOptions); + } + }), + storageOptions.getRetrySettings(), + StorageImpl.EXCEPTION_HANDLER, + storageOptions.getClock()); + } catch (RetryHelper.RetryHelperException e) { + throw StorageException.translateAndThrow(e); + } catch (IOException e) { + throw new StorageException(e); + } + } + /** * Downloads this blob to the given file path. * @@ -250,6 +289,16 @@ public void downloadTo(Path path) { downloadTo(path, new BlobSourceOption[0]); } + /** + * Downloads this blob to the given outputStream. + * + * @param outputStream destination + * @throws StorageException upon failure + */ + public void downloadTo(OutputStream outputStream) { + downloadTo(outputStream, new BlobSourceOption[0]); + } + /** Builder for {@code Blob}. */ public static class Builder extends BlobInfo.Builder { diff --git a/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/spi/v1/HttpStorageRpc.java b/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/spi/v1/HttpStorageRpc.java index 2abb24b0e0d5..e8ebe1009703 100644 --- a/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/spi/v1/HttpStorageRpc.java +++ b/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/spi/v1/HttpStorageRpc.java @@ -66,6 +66,7 @@ import com.google.common.hash.HashFunction; import com.google.common.hash.Hashing; import com.google.common.io.BaseEncoding; +import com.google.common.io.CountingOutputStream; import io.opencensus.common.Scope; import io.opencensus.trace.AttributeValue; import io.opencensus.trace.Span; @@ -627,6 +628,34 @@ public RpcBatch createBatch() { return new DefaultRpcBatch(storage); } + @Override + public void readToOutputStream( + StorageObject from, long position, CountingOutputStream to, Map options) { + Span span = startSpan(HttpStorageRpcSpans.SPAN_NAME_READ); + Scope scope = tracer.withSpan(span); + try { + Get req = + storage + .objects() + .get(from.getBucket(), from.getName()) + .setGeneration(from.getGeneration()) + .setIfMetagenerationMatch(Option.IF_METAGENERATION_MATCH.getLong(options)) + .setIfMetagenerationNotMatch(Option.IF_METAGENERATION_NOT_MATCH.getLong(options)) + .setIfGenerationMatch(Option.IF_GENERATION_MATCH.getLong(options)) + .setIfGenerationNotMatch(Option.IF_GENERATION_NOT_MATCH.getLong(options)) + .setUserProject(Option.USER_PROJECT.getString(options)); + req.getMediaHttpDownloader().setDirectDownloadEnabled(true).setBytesDownloaded(position); + req.executeMediaAndDownloadTo(to); + } catch (IOException ex) { + span.setStatus(Status.UNKNOWN.withDescription(ex.getMessage())); + StorageException serviceException = translate(ex); + throw serviceException; + } finally { + scope.close(); + span.end(); + } + } + @Override public Tuple read( StorageObject from, Map options, long position, int bytes) { diff --git a/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/spi/v1/StorageRpc.java b/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/spi/v1/StorageRpc.java index 553920da0b9a..2de4da0756d6 100644 --- a/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/spi/v1/StorageRpc.java +++ b/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/spi/v1/StorageRpc.java @@ -28,6 +28,7 @@ import com.google.cloud.ServiceRpc; import com.google.cloud.Tuple; import com.google.cloud.storage.StorageException; +import com.google.common.io.CountingOutputStream; import java.io.InputStream; import java.util.List; import java.util.Map; @@ -57,6 +58,7 @@ enum Option { FIELDS("fields"), CUSTOMER_SUPPLIED_KEY("customerSuppliedKey"), USE_DIRECT_DOWNLOAD("useDirectDownload"), + READ_CHANNEL_CHUNK_SIZE_MULTIPLIER("useDirectDownload"), USER_PROJECT("userProject"), KMS_KEY_NAME("kmsKeyName"); @@ -282,6 +284,14 @@ StorageObject compose( */ Tuple read(StorageObject from, Map options, long position, int bytes); + /** + * Reads from a storage object at the given position directly to outputstream + * + * @throws StorageException upon failure + */ + void readToOutputStream( + StorageObject from, long position, CountingOutputStream to, Map options); + /** * Opens a resumable upload channel for a given storage object. * diff --git a/google-cloud-clients/google-cloud-storage/src/test/java/com/google/cloud/storage/BlobTest.java b/google-cloud-clients/google-cloud-storage/src/test/java/com/google/cloud/storage/BlobTest.java index 8fa6be150c95..c01455f2b2ba 100644 --- a/google-cloud-clients/google-cloud-storage/src/test/java/com/google/cloud/storage/BlobTest.java +++ b/google-cloud-clients/google-cloud-storage/src/test/java/com/google/cloud/storage/BlobTest.java @@ -588,38 +588,4 @@ public Integer answer() throws Throwable { assertArrayEquals(expected, actual); } - @Test - public void testDownloadWithUseDirectDownload() throws Exception { - final byte[] expected = {1, 2}; - - initializeExpectedBlob(2); - ReadChannel channel = createNiceMock(ReadChannel.class); - expect(storage.getOptions()).andReturn(mockOptions); - expect( - storage.reader( - BLOB_INFO.getBlobId(), - Storage.BlobSourceOption.useDirectDownload(true))) - .andReturn(channel); - replay(storage); - // First read should return 2 bytes. - expect(channel.read(anyObject(ByteBuffer.class))) - .andAnswer( - new IAnswer() { - @Override - public Integer answer() throws Throwable { - // Modify the argument to match the expected behavior of `read`. - ((ByteBuffer) getCurrentArguments()[0]).put(expected); - return 2; - } - }); - // Second read should return 0 bytes. - expect(channel.read(anyObject(ByteBuffer.class))).andReturn(0); - replay(channel); - initializeBlob(); - - File file = File.createTempFile("blob", ".tmp"); - blob.downloadTo(file.toPath(), BlobSourceOption.useDirectDownload(true)); - byte actual[] = Files.readAllBytes(file.toPath()); - assertArrayEquals(expected, actual); - } } From c057fa957ed0580607981fb9f2087274ec3af7b8 Mon Sep 17 00:00:00 2001 From: andrey-qlogic Date: Thu, 7 Feb 2019 17:22:03 +0000 Subject: [PATCH 07/11] 3929: WIP after reviwew. Removed useDirectDownload lines and added a unitTest. --- .../google-cloud-storage/pom.xml | 5 ++ .../java/com/google/cloud/storage/Blob.java | 33 ++------- .../com/google/cloud/storage/BlobInfo.java | 13 ---- .../com/google/cloud/storage/Storage.java | 8 --- .../cloud/storage/spi/v1/HttpStorageRpc.java | 13 ++-- .../cloud/storage/spi/v1/StorageRpc.java | 7 +- .../com/google/cloud/storage/BlobTest.java | 69 +++++++++++++++++++ .../cloud/storage/it/ITStorageTest.java | 32 --------- google-cloud-clients/pom.xml | 2 +- 9 files changed, 90 insertions(+), 92 deletions(-) diff --git a/google-cloud-clients/google-cloud-storage/pom.xml b/google-cloud-clients/google-cloud-storage/pom.xml index ff2c19459c20..4e1125bf5e07 100644 --- a/google-cloud-clients/google-cloud-storage/pom.xml +++ b/google-cloud-clients/google-cloud-storage/pom.xml @@ -31,6 +31,11 @@ google-api-services-storage compile + + com.google.api-client + google-api-client + 1.27.1-SNAPSHOT + diff --git a/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/Blob.java b/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/Blob.java index 2c681056663f..c27598ef11e0 100644 --- a/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/Blob.java +++ b/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/Blob.java @@ -104,8 +104,6 @@ private Storage.BlobSourceOption toSourceOptions(BlobInfo blobInfo) { return Storage.BlobSourceOption.metagenerationNotMatch(blobInfo.getMetageneration()); case CUSTOMER_SUPPLIED_KEY: return Storage.BlobSourceOption.decryptionKey((String) getValue()); - case USE_DIRECT_DOWNLOAD: - return Storage.BlobSourceOption.useDirectDownload((Boolean) getValue()); case USER_PROJECT: return Storage.BlobSourceOption.userProject((String) getValue()); default: @@ -181,16 +179,6 @@ public static BlobSourceOption decryptionKey(String key) { return new BlobSourceOption(StorageRpc.Option.CUSTOMER_SUPPLIED_KEY, key); } - /** - * Returns the option to enable or disable direct download. If the value is set to {@code true}, - * then a direct download will be performed when all the blob content is loaded in one request. - * If {@code false} is specified, the download uses a resumed download protocol to load data - * into chunks. - */ - public static BlobSourceOption useDirectDownload(boolean useDirectDownload) { - return new BlobSourceOption(StorageRpc.Option.USE_DIRECT_DOWNLOAD, useDirectDownload); - } - /** * Returns an option for blob's billing user project. This option is used only if the blob's * bucket has requester_pays flag enabled. @@ -250,9 +238,7 @@ public void downloadTo(Path path, BlobSourceOption... options) { */ public void downloadTo(OutputStream outputStream, final BlobSourceOption... options) { try (CountingOutputStream countingOutputStream = new CountingOutputStream(outputStream)) { - final StorageObject storageObject = getBlobId().toPb(); - final StorageOptions storageOptions = this.options; - final StorageRpc storageRpc = storageOptions.getStorageRpcV1(); + final StorageRpc storageRpc = this.options.getStorageRpcV1(); final Map requestOptions = StorageImpl.optionMap(getBlobId(), options); runWithRetries( callable( @@ -260,15 +246,12 @@ public void downloadTo(OutputStream outputStream, final BlobSourceOption... opti @Override public void run() { storageRpc.readToOutputStream( - storageObject, - countingOutputStream.getCount(), - countingOutputStream, - requestOptions); + getBlobId().toPb(), countingOutputStream, requestOptions); } }), - storageOptions.getRetrySettings(), + this.options.getRetrySettings(), StorageImpl.EXCEPTION_HANDLER, - storageOptions.getClock()); + this.options.getClock()); } catch (RetryHelper.RetryHelperException e) { throw StorageException.translateAndThrow(e); } catch (IOException e) { @@ -290,7 +273,7 @@ public void downloadTo(Path path) { } /** - * Downloads this blob to the given outputStream. + * Downloads this blob to the given output stream. * * @param outputStream destination * @throws StorageException upon failure @@ -466,12 +449,6 @@ public Builder setEventBasedHold(Boolean eventBasedHold) { return this; } - @Override - public Builder setUseDirectDownload(Boolean useDirectDownload) { - infoBuilder.setUseDirectDownload(useDirectDownload); - return this; - } - @Override public Builder setTemporaryHold(Boolean temporaryHold) { infoBuilder.setTemporaryHold(temporaryHold); diff --git a/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/BlobInfo.java b/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/BlobInfo.java index dad1da8ad544..d510953692b4 100644 --- a/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/BlobInfo.java +++ b/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/BlobInfo.java @@ -85,7 +85,6 @@ public StorageObject apply(BlobInfo blobInfo) { private final String kmsKeyName; private final Boolean eventBasedHold; private final Boolean temporaryHold; - private final Boolean useDirectDownload; private final Long retentionExpirationTime; /** This class is meant for internal use only. Users are discouraged from using this class. */ @@ -266,9 +265,6 @@ public abstract static class Builder { @BetaApi public abstract Builder setTemporaryHold(Boolean temporaryHold); - @BetaApi - public abstract Builder setUseDirectDownload(Boolean useDirectDownload); - @BetaApi abstract Builder setRetentionExpirationTime(Long retentionExpirationTime); @@ -305,7 +301,6 @@ static final class BuilderImpl extends Builder { private String kmsKeyName; private Boolean eventBasedHold; private Boolean temporaryHold; - private Boolean useDirectDownload; private Long retentionExpirationTime; BuilderImpl(BlobId blobId) { @@ -340,7 +335,6 @@ static final class BuilderImpl extends Builder { kmsKeyName = blobInfo.kmsKeyName; eventBasedHold = blobInfo.eventBasedHold; temporaryHold = blobInfo.temporaryHold; - useDirectDownload = blobInfo.useDirectDownload; retentionExpirationTime = blobInfo.retentionExpirationTime; } @@ -509,12 +503,6 @@ public Builder setTemporaryHold(Boolean temporaryHold) { return this; } - @Override - public Builder setUseDirectDownload(Boolean useDirectDownload) { - this.useDirectDownload = useDirectDownload; - return this; - } - @Override Builder setRetentionExpirationTime(Long retentionExpirationTime) { this.retentionExpirationTime = retentionExpirationTime; @@ -556,7 +544,6 @@ public BlobInfo build() { kmsKeyName = builder.kmsKeyName; eventBasedHold = builder.eventBasedHold; temporaryHold = builder.temporaryHold; - useDirectDownload = builder.useDirectDownload; retentionExpirationTime = builder.retentionExpirationTime; } diff --git a/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/Storage.java b/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/Storage.java index c4caf365cdad..6ce9ca8c6e2a 100644 --- a/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/Storage.java +++ b/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/Storage.java @@ -598,14 +598,6 @@ public static BlobSourceOption generationNotMatch() { return new BlobSourceOption(StorageRpc.Option.IF_GENERATION_NOT_MATCH, null); } - /** - * Returns an option for blob's downloading. If this option is used the request will use - * directDownload option for MediaHttpDownloader. - */ - public static BlobSourceOption useDirectDownload(boolean useDirectDownload) { - return new BlobSourceOption(StorageRpc.Option.USE_DIRECT_DOWNLOAD, useDirectDownload); - } - /** * Returns an option for blob's data generation mismatch. If this option is used the request * will fail if blob's generation matches the provided value. diff --git a/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/spi/v1/HttpStorageRpc.java b/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/spi/v1/HttpStorageRpc.java index e8ebe1009703..e4848e8b29d6 100644 --- a/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/spi/v1/HttpStorageRpc.java +++ b/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/spi/v1/HttpStorageRpc.java @@ -629,8 +629,8 @@ public RpcBatch createBatch() { } @Override - public void readToOutputStream( - StorageObject from, long position, CountingOutputStream to, Map options) { + public boolean readToOutputStream( + StorageObject from, CountingOutputStream to, Map options) { Span span = startSpan(HttpStorageRpcSpans.SPAN_NAME_READ); Scope scope = tracer.withSpan(span); try { @@ -644,8 +644,13 @@ public void readToOutputStream( .setIfGenerationMatch(Option.IF_GENERATION_MATCH.getLong(options)) .setIfGenerationNotMatch(Option.IF_GENERATION_NOT_MATCH.getLong(options)) .setUserProject(Option.USER_PROJECT.getString(options)); - req.getMediaHttpDownloader().setDirectDownloadEnabled(true).setBytesDownloaded(position); + req.getMediaHttpDownloader().setDirectDownloadEnabled(true); + long bytesDownloaded = to.getCount(); + if (bytesDownloaded > 0) { + req.getMediaHttpDownloader().setBytesDownloaded(bytesDownloaded); + } req.executeMediaAndDownloadTo(to); + return true; } catch (IOException ex) { span.setStatus(Status.UNKNOWN.withDescription(ex.getMessage())); StorageException serviceException = translate(ex); @@ -672,8 +677,6 @@ public Tuple read( .setIfGenerationMatch(Option.IF_GENERATION_MATCH.getLong(options)) .setIfGenerationNotMatch(Option.IF_GENERATION_NOT_MATCH.getLong(options)) .setUserProject(Option.USER_PROJECT.getString(options)); - req.getMediaHttpDownloader() - .setDirectDownloadEnabled(Option.USE_DIRECT_DOWNLOAD.getBoolean(options)); checkArgument(position >= 0, "Position should be non-negative, is %d", position); StringBuilder range = new StringBuilder(); range.append("bytes=").append(position).append("-").append(position + bytes - 1); diff --git a/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/spi/v1/StorageRpc.java b/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/spi/v1/StorageRpc.java index 2de4da0756d6..8d95ac23835f 100644 --- a/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/spi/v1/StorageRpc.java +++ b/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/spi/v1/StorageRpc.java @@ -57,8 +57,6 @@ enum Option { VERSIONS("versions"), FIELDS("fields"), CUSTOMER_SUPPLIED_KEY("customerSuppliedKey"), - USE_DIRECT_DOWNLOAD("useDirectDownload"), - READ_CHANNEL_CHUNK_SIZE_MULTIPLIER("useDirectDownload"), USER_PROJECT("userProject"), KMS_KEY_NAME("kmsKeyName"); @@ -285,12 +283,11 @@ StorageObject compose( Tuple read(StorageObject from, Map options, long position, int bytes); /** - * Reads from a storage object at the given position directly to outputstream + * Reads from a storage object at the given position directly to output stream. * * @throws StorageException upon failure */ - void readToOutputStream( - StorageObject from, long position, CountingOutputStream to, Map options); + boolean readToOutputStream(StorageObject from, CountingOutputStream to, Map options); /** * Opens a resumable upload channel for a given storage object. diff --git a/google-cloud-clients/google-cloud-storage/src/test/java/com/google/cloud/storage/BlobTest.java b/google-cloud-clients/google-cloud-storage/src/test/java/com/google/cloud/storage/BlobTest.java index c01455f2b2ba..70edb570d122 100644 --- a/google-cloud-clients/google-cloud-storage/src/test/java/com/google/cloud/storage/BlobTest.java +++ b/google-cloud-clients/google-cloud-storage/src/test/java/com/google/cloud/storage/BlobTest.java @@ -33,6 +33,9 @@ import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; +import com.google.api.core.ApiClock; +import com.google.api.gax.retrying.RetrySettings; +import com.google.api.services.storage.model.StorageObject; import com.google.cloud.ReadChannel; import com.google.cloud.storage.Acl.Project; import com.google.cloud.storage.Acl.Project.ProjectRole; @@ -41,10 +44,13 @@ import com.google.cloud.storage.Blob.BlobSourceOption; import com.google.cloud.storage.Storage.BlobWriteOption; import com.google.cloud.storage.Storage.CopyRequest; +import com.google.cloud.storage.spi.v1.StorageRpc; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.io.BaseEncoding; +import com.google.common.io.CountingOutputStream; import java.io.File; +import java.io.OutputStream; import java.net.URL; import java.nio.ByteBuffer; import java.nio.file.Files; @@ -58,6 +64,7 @@ import org.junit.After; import org.junit.Before; import org.junit.Test; +import org.threeten.bp.Duration; public class BlobTest { @@ -128,6 +135,28 @@ public class BlobTest { private static final String BASE64_KEY = "JVzfVl8NLD9FjedFuStegjRfES5ll5zc59CIXw572OA="; private static final Key KEY = new SecretKeySpec(BaseEncoding.base64().decode(BASE64_KEY), "AES256"); + private static final RetrySettings RETRY_SETTINGS = + RetrySettings.newBuilder() + .setInitialRetryDelay(Duration.ofMillis(100L)) + .setRetryDelayMultiplier(1.3) + .setMaxRetryDelay(Duration.ofMillis(60000L)) + .setInitialRpcTimeout(Duration.ofMillis(20000L)) + .setRpcTimeoutMultiplier(1.0) + .setMaxRpcTimeout(Duration.ofMillis(20000L)) + .setTotalTimeout(Duration.ofMillis(600000L)) + .build(); + private static final ApiClock API_CLOCK = + new ApiClock() { + @Override + public long nanoTime() { + return 42_000_000_000L; + } + + @Override + public long millisTime() { + return 42_000L; + } + }; private Storage storage; private Blob blob; @@ -588,4 +617,44 @@ public Integer answer() throws Throwable { assertArrayEquals(expected, actual); } + @Test + public void testDownloadToOutputStream() throws Exception { + final byte[] expected = {1, 2}; + + File file = File.createTempFile("blob", ".tmp"); + StorageRpc mockStorageRpc = createNiceMock(StorageRpc.class); + + expect(storage.getOptions()).andReturn(mockOptions).times(2); + replay(storage); + + expect(mockOptions.getStorageRpcV1()).andReturn(mockStorageRpc); + + expect(mockOptions.getRetrySettings()).andReturn(RETRY_SETTINGS); + expect(mockOptions.getClock()).andReturn(API_CLOCK); + + replay(mockOptions); + + storage.getOptions(); + blob = new Blob(storage, new BlobInfo.BuilderImpl(BLOB_INFO)); + + expect( + mockStorageRpc.readToOutputStream( + anyObject(StorageObject.class), + anyObject(CountingOutputStream.class), + anyObject(Map.class))) + .andAnswer( + new IAnswer() { + @Override + public Boolean answer() throws Throwable { + ((CountingOutputStream) getCurrentArguments()[1]).write(expected); + return true; + } + }); + replay(mockStorageRpc); + + OutputStream outputStream = Files.newOutputStream(file.toPath()); + blob.downloadTo(outputStream); + byte actual[] = Files.readAllBytes(file.toPath()); + assertArrayEquals(expected, actual); + } } diff --git a/google-cloud-clients/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITStorageTest.java b/google-cloud-clients/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITStorageTest.java index 244493297bb1..3a97dab2800d 100644 --- a/google-cloud-clients/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITStorageTest.java +++ b/google-cloud-clients/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITStorageTest.java @@ -1699,38 +1699,6 @@ public void testReadChannelFail() throws IOException { } } - @Test - public void testReadChannelWithUseDirectDownload() throws IOException { - String blobName = "test-read-channel-blob-direct-download"; - BlobInfo blob = BlobInfo.newBuilder(BUCKET, blobName).build(); - Blob remoteBlob = storage.create(blob); - assertNotNull(remoteBlob); - try (ReadChannel reader = - storage.reader(blob.getBlobId(), Storage.BlobSourceOption.useDirectDownload(true))) { - reader.read(ByteBuffer.allocate(42)); - } catch (StorageException ex) { - // expected - } - assertEquals(blob.getBucket(), remoteBlob.getBucket()); - assertEquals(blob.getName(), remoteBlob.getName()); - } - - @Test - public void testReadChannelWithNoUseDirectDownload() throws IOException { - String blobName = "test-read-channel-blob-direct-download"; - BlobInfo blob = BlobInfo.newBuilder(BUCKET, blobName).build(); - Blob remoteBlob = storage.create(blob); - assertNotNull(remoteBlob); - try (ReadChannel reader = - storage.reader(blob.getBlobId(), Storage.BlobSourceOption.useDirectDownload(false))) { - reader.read(ByteBuffer.allocate(42)); - } catch (StorageException ex) { - // expected - } - assertEquals(blob.getBucket(), remoteBlob.getBucket()); - assertEquals(blob.getName(), remoteBlob.getName()); - } - @Test public void testReadChannelFailUpdatedGeneration() throws IOException { String blobName = "test-read-blob-fail-updated-generation"; diff --git a/google-cloud-clients/pom.xml b/google-cloud-clients/pom.xml index 595bac9eafcb..25306d0ef5ac 100644 --- a/google-cloud-clients/pom.xml +++ b/google-cloud-clients/pom.xml @@ -153,7 +153,7 @@ github google-cloud-clients 0.79.1-alpha-SNAPSHOT - 1.27.0 + 1.27.1-SNAPSHOT 1.37.0 1.7.0 From ac984c276c4a52bd01e87441b577fb864b1ba798 Mon Sep 17 00:00:00 2001 From: andrey-qlogic Date: Thu, 7 Feb 2019 17:30:01 +0000 Subject: [PATCH 08/11] 3929: Reverted changes in local pom files used for testing. --- google-cloud-clients/google-cloud-storage/pom.xml | 5 ----- google-cloud-clients/pom.xml | 2 +- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/google-cloud-clients/google-cloud-storage/pom.xml b/google-cloud-clients/google-cloud-storage/pom.xml index 4e1125bf5e07..ff2c19459c20 100644 --- a/google-cloud-clients/google-cloud-storage/pom.xml +++ b/google-cloud-clients/google-cloud-storage/pom.xml @@ -31,11 +31,6 @@ google-api-services-storage compile - - com.google.api-client - google-api-client - 1.27.1-SNAPSHOT - diff --git a/google-cloud-clients/pom.xml b/google-cloud-clients/pom.xml index 25306d0ef5ac..595bac9eafcb 100644 --- a/google-cloud-clients/pom.xml +++ b/google-cloud-clients/pom.xml @@ -153,7 +153,7 @@ github google-cloud-clients 0.79.1-alpha-SNAPSHOT - 1.27.1-SNAPSHOT + 1.27.0 1.37.0 1.7.0 From a8c5c134c317c5588b62920cbfbc59547275c808 Mon Sep 17 00:00:00 2001 From: andrey-qlogic Date: Thu, 7 Feb 2019 18:39:14 +0000 Subject: [PATCH 09/11] 3929: Added implementation to FakeStorageRpc with UnsupportedOperationException. --- .../cloud/storage/contrib/nio/testing/FakeStorageRpc.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/testing/FakeStorageRpc.java b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/testing/FakeStorageRpc.java index 07cb169511a4..dff6b6ca8395 100644 --- a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/testing/FakeStorageRpc.java +++ b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/testing/FakeStorageRpc.java @@ -29,6 +29,7 @@ import com.google.cloud.storage.StorageException; import com.google.cloud.storage.spi.v1.RpcBatch; import com.google.cloud.storage.spi.v1.StorageRpc; +import com.google.common.io.CountingOutputStream; import java.io.IOException; import java.io.InputStream; import java.math.BigInteger; @@ -295,6 +296,12 @@ public Tuple read( return Tuple.of("etag-goes-here", ret); } + @Override + public boolean readToOutputStream( + StorageObject from, CountingOutputStream to, Map options) { + throw new UnsupportedOperationException(); + } + @Override public String open(StorageObject object, Map options) throws StorageException { String key = fullname(object); From 3873ea52cb0dda349e868c17ca350b43c615b410 Mon Sep 17 00:00:00 2001 From: andrey-qlogic Date: Thu, 21 Feb 2019 10:27:15 +0000 Subject: [PATCH 10/11] Added an unit test to StorageImplTest class --- .../google/cloud/storage/StorageImplTest.java | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/google-cloud-clients/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageImplTest.java b/google-cloud-clients/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageImplTest.java index 0d77547e7605..4f373dc186c6 100644 --- a/google-cloud-clients/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageImplTest.java +++ b/google-cloud-clients/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageImplTest.java @@ -18,6 +18,7 @@ import static com.google.cloud.storage.testing.ApiPolicyMatcher.eqApiPolicy; import static java.nio.charset.StandardCharsets.UTF_8; +import static org.easymock.EasyMock.getCurrentArguments; import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -55,9 +56,12 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; import com.google.common.io.BaseEncoding; +import com.google.common.io.CountingOutputStream; import com.google.common.net.UrlEscapers; import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.io.OutputStream; import java.io.UnsupportedEncodingException; import java.net.URL; import java.net.URLDecoder; @@ -81,6 +85,7 @@ import javax.crypto.spec.SecretKeySpec; import org.easymock.Capture; import org.easymock.EasyMock; +import org.easymock.IAnswer; import org.junit.After; import org.junit.Before; import org.junit.BeforeClass; @@ -1591,6 +1596,37 @@ public void testReaderWithOptionsFromBlobId() throws IOException { channel.read(ByteBuffer.allocate(42)); } + @Test + public void testReadToOutputStream() { + final byte[] expected = {0xD, 0xE, 0xA, 0xD}; + OutputStream outputStream = new ByteArrayOutputStream(); + CountingOutputStream countingOutputStream = new CountingOutputStream(outputStream); + EasyMock.expect( + storageRpcMock.readToOutputStream( + BlobId.of(BUCKET_NAME1, BLOB_NAME1).toPb(), + countingOutputStream, + EMPTY_RPC_OPTIONS)) + .andAnswer( + new IAnswer() { + @Override + public Boolean answer() throws Throwable { + ((CountingOutputStream) getCurrentArguments()[1]).write(expected); + return true; + } + }); + EasyMock.replay(storageRpcMock); + initializeService(); + boolean result = + options + .getStorageRpcV1() + .readToOutputStream( + BlobId.of(BUCKET_NAME1, BLOB_NAME1).toPb(), + countingOutputStream, + EMPTY_RPC_OPTIONS); + assertTrue(result); + assertArrayEquals(BLOB_CONTENT, ((ByteArrayOutputStream) outputStream).toByteArray()); + } + @Test public void testWriter() { BlobInfo.Builder infoBuilder = BLOB_INFO1.toBuilder(); From c7a89387b629c160880213bb00181eefc89ca304 Mon Sep 17 00:00:00 2001 From: andrey-qlogic Date: Thu, 21 Feb 2019 13:04:10 +0000 Subject: [PATCH 11/11] WIP: Work on unit tests and codecoverage --- .../java/com/google/cloud/storage/Blob.java | 8 ++- .../cloud/storage/spi/v1/HttpStorageRpc.java | 3 +- .../com/google/cloud/storage/BlobTest.java | 71 ++++++++++++++++--- .../google/cloud/storage/StorageImplTest.java | 24 +++++++ 4 files changed, 93 insertions(+), 13 deletions(-) diff --git a/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/Blob.java b/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/Blob.java index 544d0aa10420..003a52fc2379 100644 --- a/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/Blob.java +++ b/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/Blob.java @@ -236,17 +236,19 @@ public void downloadTo(Path path, BlobSourceOption... options) { * @param options blob read options * @throws StorageException upon failure */ - public void downloadTo(OutputStream outputStream, final BlobSourceOption... options) { + public void downloadTo(OutputStream outputStream, final BlobSourceOption... options) + throws StorageException { try (CountingOutputStream countingOutputStream = new CountingOutputStream(outputStream)) { final StorageRpc storageRpc = this.options.getStorageRpcV1(); - final Map requestOptions = StorageImpl.optionMap(getBlobId(), options); runWithRetries( callable( new Runnable() { @Override public void run() { storageRpc.readToOutputStream( - getBlobId().toPb(), countingOutputStream, requestOptions); + getBlobId().toPb(), + countingOutputStream, + StorageImpl.optionMap(getBlobId(), options)); } }), this.options.getRetrySettings(), diff --git a/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/spi/v1/HttpStorageRpc.java b/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/spi/v1/HttpStorageRpc.java index a89e749a539f..51790f102b56 100644 --- a/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/spi/v1/HttpStorageRpc.java +++ b/google-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/spi/v1/HttpStorageRpc.java @@ -668,8 +668,7 @@ public boolean readToOutputStream( return true; } catch (IOException ex) { span.setStatus(Status.UNKNOWN.withDescription(ex.getMessage())); - StorageException serviceException = translate(ex); - throw serviceException; + throw translate(ex); } finally { scope.close(); span.end(); diff --git a/google-cloud-clients/google-cloud-storage/src/test/java/com/google/cloud/storage/BlobTest.java b/google-cloud-clients/google-cloud-storage/src/test/java/com/google/cloud/storage/BlobTest.java index b69e9c6c445e..d7dd066d783b 100644 --- a/google-cloud-clients/google-cloud-storage/src/test/java/com/google/cloud/storage/BlobTest.java +++ b/google-cloud-clients/google-cloud-storage/src/test/java/com/google/cloud/storage/BlobTest.java @@ -32,6 +32,7 @@ import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import com.google.api.core.ApiClock; import com.google.api.gax.retrying.RetrySettings; @@ -50,6 +51,7 @@ import com.google.common.io.BaseEncoding; import com.google.common.io.CountingOutputStream; import java.io.File; +import java.io.IOException; import java.io.OutputStream; import java.net.URL; import java.nio.ByteBuffer; @@ -626,23 +628,16 @@ public Integer answer() throws Throwable { @Test public void testDownloadToOutputStream() throws Exception { final byte[] expected = {1, 2}; - File file = File.createTempFile("blob", ".tmp"); StorageRpc mockStorageRpc = createNiceMock(StorageRpc.class); - expect(storage.getOptions()).andReturn(mockOptions).times(2); replay(storage); - expect(mockOptions.getStorageRpcV1()).andReturn(mockStorageRpc); - expect(mockOptions.getRetrySettings()).andReturn(RETRY_SETTINGS); expect(mockOptions.getClock()).andReturn(API_CLOCK); - replay(mockOptions); - storage.getOptions(); blob = new Blob(storage, new BlobInfo.BuilderImpl(BLOB_INFO)); - expect( mockStorageRpc.readToOutputStream( anyObject(StorageObject.class), @@ -657,10 +652,70 @@ public Boolean answer() throws Throwable { } }); replay(mockStorageRpc); - OutputStream outputStream = Files.newOutputStream(file.toPath()); blob.downloadTo(outputStream); byte actual[] = Files.readAllBytes(file.toPath()); assertArrayEquals(expected, actual); } + + @Test + public void testDownloadToOutputStreamWithOptions() throws Exception { + final byte[] expected = {1, 2}; + File file = File.createTempFile("blob", ".tmp"); + StorageRpc mockStorageRpc = createNiceMock(StorageRpc.class); + expect(storage.getOptions()).andReturn(mockOptions).times(2); + replay(storage); + expect(mockOptions.getStorageRpcV1()).andReturn(mockStorageRpc); + expect(mockOptions.getRetrySettings()).andReturn(RETRY_SETTINGS); + expect(mockOptions.getClock()).andReturn(API_CLOCK); + replay(mockOptions); + storage.getOptions(); + blob = new Blob(storage, new BlobInfo.BuilderImpl(BLOB_INFO)); + expect( + mockStorageRpc.readToOutputStream( + anyObject(StorageObject.class), + anyObject(CountingOutputStream.class), + anyObject(Map.class))) + .andAnswer( + new IAnswer() { + @Override + public Boolean answer() throws Throwable { + ((CountingOutputStream) getCurrentArguments()[1]).write(expected); + return true; + } + }); + replay(mockStorageRpc); + OutputStream outputStream = Files.newOutputStream(file.toPath()); + blob.downloadTo(outputStream, new BlobSourceOption[0]); + byte actual[] = Files.readAllBytes(file.toPath()); + assertArrayEquals(expected, actual); + } + + @Test + public void testDownloadToOutputStreamStorageException() throws Exception { + File file = File.createTempFile("blob", ".tmp"); + StorageRpc mockStorageRpc = createNiceMock(StorageRpc.class); + expect(storage.getOptions()).andReturn(mockOptions).times(2); + replay(storage); + expect(mockOptions.getStorageRpcV1()).andReturn(mockStorageRpc); + expect(mockOptions.getRetrySettings()).andReturn(RETRY_SETTINGS); + expect(mockOptions.getClock()).andReturn(API_CLOCK); + replay(mockOptions); + storage.getOptions(); + blob = new Blob(storage, new BlobInfo.BuilderImpl(BLOB_INFO)); + expect( + mockStorageRpc.readToOutputStream( + anyObject(StorageObject.class), + anyObject(CountingOutputStream.class), + anyObject(Map.class))) + .andThrow(new StorageException(new IOException())); + replay(mockStorageRpc); + OutputStream outputStream = Files.newOutputStream(file.toPath()); + try { + blob.downloadTo(outputStream); + fail(); + } catch (StorageException e) { + // expected + } + } } diff --git a/google-cloud-clients/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageImplTest.java b/google-cloud-clients/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageImplTest.java index 4f373dc186c6..03a11e4ab1fc 100644 --- a/google-cloud-clients/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageImplTest.java +++ b/google-cloud-clients/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageImplTest.java @@ -26,6 +26,7 @@ import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import com.google.api.client.googleapis.json.GoogleJsonError; import com.google.api.core.ApiClock; @@ -1627,6 +1628,29 @@ public Boolean answer() throws Throwable { assertArrayEquals(BLOB_CONTENT, ((ByteArrayOutputStream) outputStream).toByteArray()); } + @Test + public void testReadToOutputStreamStorageException() { + CountingOutputStream countingOutputStream = + new CountingOutputStream(new ByteArrayOutputStream()); + EasyMock.expect( + storageRpcMock.readToOutputStream( + BlobId.of(BUCKET_NAME1, BLOB_NAME1).toPb(), + countingOutputStream, + EMPTY_RPC_OPTIONS)) + .andThrow(new StorageException(new IOException())); + EasyMock.replay(storageRpcMock); + initializeService(); + try { + options + .getStorageRpcV1() + .readToOutputStream( + BlobId.of(BUCKET_NAME1, BLOB_NAME1).toPb(), countingOutputStream, EMPTY_RPC_OPTIONS); + fail(); + } catch (StorageException e) { + // expected + } + } + @Test public void testWriter() { BlobInfo.Builder infoBuilder = BLOB_INFO1.toBuilder();