From 0a113cd7431722d0c10d849a2760a41eb2ce4d87 Mon Sep 17 00:00:00 2001 From: Marco Ziccardi Date: Fri, 18 Mar 2016 14:25:25 +0100 Subject: [PATCH 1/6] Remove default contentType from Storage's compose --- .../java/com/google/gcloud/storage/spi/DefaultStorageRpc.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/spi/DefaultStorageRpc.java b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/spi/DefaultStorageRpc.java index aa6085e161ed..3f465e0ab20e 100644 --- a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/spi/DefaultStorageRpc.java +++ b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/spi/DefaultStorageRpc.java @@ -319,9 +319,6 @@ private Storage.Objects.Delete deleteRequest(StorageObject blob, Map public StorageObject compose(Iterable sources, StorageObject target, Map targetOptions) { ComposeRequest request = new ComposeRequest(); - if (target.getContentType() == null) { - target.setContentType("application/octet-stream"); - } request.setDestination(target); List sourceObjects = new ArrayList<>(); for (StorageObject source : sources) { From 8840b33a9cfbb2ab780c846e12131716fda8e0ab Mon Sep 17 00:00:00 2001 From: Marco Ziccardi Date: Fri, 18 Mar 2016 14:56:46 +0100 Subject: [PATCH 2/6] Remove default contentType from Bucket's create --- .../com/google/gcloud/storage/Bucket.java | 50 +++++++++++++++---- .../com/google/gcloud/storage/Storage.java | 2 - .../com/google/gcloud/storage/BucketTest.java | 12 ++--- 3 files changed, 47 insertions(+), 17 deletions(-) diff --git a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/Bucket.java b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/Bucket.java index 5df305ff371c..e44bd60d785c 100644 --- a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/Bucket.java +++ b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/Bucket.java @@ -22,7 +22,6 @@ import static com.google.gcloud.storage.Bucket.BucketSourceOption.toSourceOptions; import com.google.common.base.Function; -import com.google.common.base.MoreObjects; import com.google.common.collect.Lists; import com.google.common.collect.Sets; import com.google.gcloud.Page; @@ -633,15 +632,13 @@ public List get(String blobName1, String blobName2, String... blobNames) { * * @param blob a blob name * @param content the blob content - * @param contentType the blob content type. If {@code null} then - * {@value com.google.gcloud.storage.Storage#DEFAULT_CONTENT_TYPE} is used. + * @param contentType the blob content type * @param options options for blob creation * @return a complete blob information * @throws StorageException upon failure */ public Blob create(String blob, byte[] content, String contentType, BlobTargetOption... options) { - BlobInfo blobInfo = BlobInfo.builder(BlobId.of(name(), blob)) - .contentType(MoreObjects.firstNonNull(contentType, Storage.DEFAULT_CONTENT_TYPE)).build(); + BlobInfo blobInfo = BlobInfo.builder(BlobId.of(name(), blob)).contentType(contentType).build(); StorageRpc.Tuple target = BlobTargetOption.toTargetOptions(blobInfo, options); return storage.create(target.x(), content, target.y()); @@ -654,16 +651,51 @@ public Blob create(String blob, byte[] content, String contentType, BlobTargetOp * * @param blob a blob name * @param content the blob content as a stream - * @param contentType the blob content type. If {@code null} then - * {@value com.google.gcloud.storage.Storage#DEFAULT_CONTENT_TYPE} is used. + * @param contentType the blob content type * @param options options for blob creation * @return a complete blob information * @throws StorageException upon failure */ public Blob create(String blob, InputStream content, String contentType, BlobWriteOption... options) { - BlobInfo blobInfo = BlobInfo.builder(BlobId.of(name(), blob)) - .contentType(MoreObjects.firstNonNull(contentType, Storage.DEFAULT_CONTENT_TYPE)).build(); + BlobInfo blobInfo = BlobInfo.builder(BlobId.of(name(), blob)).contentType(contentType).build(); + StorageRpc.Tuple write = + BlobWriteOption.toWriteOptions(blobInfo, options); + return storage.create(write.x(), content, write.y()); + } + + /** + * Creates a new blob in this bucket. Direct upload is used to upload {@code content}. + * For large content, {@link Blob#writer(com.google.gcloud.storage.Storage.BlobWriteOption...)} + * is recommended as it uses resumable upload. MD5 and CRC32C hashes of {@code content} are + * computed and used for validating transferred data. + * + * @param blob a blob name + * @param content the blob content + * @param options options for blob creation + * @return a complete blob information + * @throws StorageException upon failure + */ + public Blob create(String blob, byte[] content, BlobTargetOption... options) { + BlobInfo blobInfo = BlobInfo.builder(BlobId.of(name(), blob)).build(); + StorageRpc.Tuple target = + BlobTargetOption.toTargetOptions(blobInfo, options); + return storage.create(target.x(), content, target.y()); + } + + /** + * Creates a new blob in this bucket. Direct upload is used to upload {@code content}. + * For large content, {@link Blob#writer(com.google.gcloud.storage.Storage.BlobWriteOption...)} + * is recommended as it uses resumable upload. + * + * @param blob a blob name + * @param content the blob content as a stream + * @param options options for blob creation + * @return a complete blob information + * @throws StorageException upon failure + */ + public Blob create(String blob, InputStream content, BlobWriteOption... options) { + BlobInfo blobInfo = BlobInfo.builder(BlobId.of(name(), blob)).build(); StorageRpc.Tuple write = BlobWriteOption.toWriteOptions(blobInfo, options); return storage.create(write.x(), content, write.y()); diff --git a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/Storage.java b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/Storage.java index c30111e50835..35fc6117cbc2 100644 --- a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/Storage.java +++ b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/Storage.java @@ -53,8 +53,6 @@ */ public interface Storage extends Service { - String DEFAULT_CONTENT_TYPE = "application/octet-stream"; - enum PredefinedAcl { AUTHENTICATED_READ("authenticatedRead"), ALL_AUTHENTICATED_USERS("allAuthenticatedUsers"), diff --git a/gcloud-java-storage/src/test/java/com/google/gcloud/storage/BucketTest.java b/gcloud-java-storage/src/test/java/com/google/gcloud/storage/BucketTest.java index 236411e0c2d8..53056c39c0dc 100644 --- a/gcloud-java-storage/src/test/java/com/google/gcloud/storage/BucketTest.java +++ b/gcloud-java-storage/src/test/java/com/google/gcloud/storage/BucketTest.java @@ -293,16 +293,16 @@ public void testCreate() throws Exception { } @Test - public void testCreateNullContentType() throws Exception { + public void testCreateNoContentType() throws Exception { initializeExpectedBucket(5); - BlobInfo info = BlobInfo.builder("b", "n").contentType(Storage.DEFAULT_CONTENT_TYPE).build(); + BlobInfo info = BlobInfo.builder("b", "n").build(); Blob expectedBlob = new Blob(serviceMockReturnsOptions, new BlobInfo.BuilderImpl(info)); byte[] content = {0xD, 0xE, 0xA, 0xD}; expect(storage.options()).andReturn(mockOptions); expect(storage.create(info, content)).andReturn(expectedBlob); replay(storage); initializeBucket(); - Blob blob = bucket.create("n", content, null); + Blob blob = bucket.create("n", content); assertEquals(expectedBlob, blob); } @@ -388,9 +388,9 @@ public void testCreateFromStream() throws Exception { } @Test - public void testCreateFromStreamNullContentType() throws Exception { + public void testCreateFromStreamNoContentType() throws Exception { initializeExpectedBucket(5); - BlobInfo info = BlobInfo.builder("b", "n").contentType(Storage.DEFAULT_CONTENT_TYPE).build(); + BlobInfo info = BlobInfo.builder("b", "n").build(); Blob expectedBlob = new Blob(serviceMockReturnsOptions, new BlobInfo.BuilderImpl(info)); byte[] content = {0xD, 0xE, 0xA, 0xD}; InputStream streamContent = new ByteArrayInputStream(content); @@ -398,7 +398,7 @@ public void testCreateFromStreamNullContentType() throws Exception { expect(storage.create(info, streamContent)).andReturn(expectedBlob); replay(storage); initializeBucket(); - Blob blob = bucket.create("n", streamContent, null); + Blob blob = bucket.create("n", streamContent); assertEquals(expectedBlob, blob); } From 22636e276005b809e7bef8cc869a38cba786b162 Mon Sep 17 00:00:00 2001 From: Marco Ziccardi Date: Fri, 18 Mar 2016 17:00:47 +0100 Subject: [PATCH 3/6] Remove checks for contentType in CopyRequest - update CopyRequest to hold both targetId and targetInfo - avoid sending storage object in StorageRpc.rewrite when only targetId is set - refactor CopyWriter and state - add integration tests --- .../com/google/gcloud/storage/CopyWriter.java | 57 +++++--- .../com/google/gcloud/storage/Storage.java | 80 ++++++------ .../google/gcloud/storage/StorageImpl.java | 15 ++- .../gcloud/storage/spi/DefaultStorageRpc.java | 4 +- .../google/gcloud/storage/spi/StorageRpc.java | 19 ++- .../com/google/gcloud/storage/BlobTest.java | 12 +- .../gcloud/storage/CopyRequestTest.java | 53 +++----- .../google/gcloud/storage/CopyWriterTest.java | 122 +++++++++++++++--- .../gcloud/storage/StorageImplTest.java | 14 +- .../gcloud/storage/it/ITStorageTest.java | 51 ++++++++ 10 files changed, 288 insertions(+), 139 deletions(-) diff --git a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/CopyWriter.java b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/CopyWriter.java index 62b39e005369..38d9c0d5e952 100644 --- a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/CopyWriter.java +++ b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/CopyWriter.java @@ -65,11 +65,11 @@ public class CopyWriter implements Restorable { * * @throws StorageException upon failure */ - public BlobInfo result() { + public Blob result() { while (!isDone()) { copyChunk(); } - return BlobInfo.fromPb(rewriteResponse.result); + return Blob.fromPb(serviceOptions.service(), rewriteResponse.result); } /** @@ -120,8 +120,12 @@ public RestorableState capture() { serviceOptions, BlobId.fromPb(rewriteResponse.rewriteRequest.source), rewriteResponse.rewriteRequest.sourceOptions, - BlobInfo.fromPb(rewriteResponse.rewriteRequest.target), + BlobId.of(rewriteResponse.rewriteRequest.targetBucket, + rewriteResponse.rewriteRequest.targetName), rewriteResponse.rewriteRequest.targetOptions) + .targetInfo(rewriteResponse.rewriteRequest.targetObject != null + ? BlobInfo.fromPb(rewriteResponse.rewriteRequest.targetObject) : null) + .result(rewriteResponse.result != null ? BlobInfo.fromPb(rewriteResponse.result) : null) .blobSize(blobSize()) .isDone(isDone()) .megabytesCopiedPerChunk(rewriteResponse.rewriteRequest.megabytesRewrittenPerCall) @@ -132,12 +136,13 @@ public RestorableState capture() { static class StateImpl implements RestorableState, Serializable { - private static final long serialVersionUID = 8279287678903181701L; + private static final long serialVersionUID = 1693964441435822700L; private final StorageOptions serviceOptions; private final BlobId source; private final Map sourceOptions; - private final BlobInfo target; + private final BlobId targetId; + private final BlobInfo targetInfo; private final Map targetOptions; private final BlobInfo result; private final long blobSize; @@ -150,7 +155,8 @@ static class StateImpl implements RestorableState, Serializable { this.serviceOptions = builder.serviceOptions; this.source = builder.source; this.sourceOptions = builder.sourceOptions; - this.target = builder.target; + this.targetId = builder.targetId; + this.targetInfo = builder.targetInfo; this.targetOptions = builder.targetOptions; this.result = builder.result; this.blobSize = builder.blobSize; @@ -165,8 +171,9 @@ static class Builder { private final StorageOptions serviceOptions; private final BlobId source; private final Map sourceOptions; - private final BlobInfo target; + private final BlobId targetId; private final Map targetOptions; + private BlobInfo targetInfo; private BlobInfo result; private long blobSize; private boolean isDone; @@ -176,14 +183,19 @@ static class Builder { private Builder(StorageOptions options, BlobId source, Map sourceOptions, - BlobInfo target, Map targetOptions) { + BlobId targetId, Map targetOptions) { this.serviceOptions = options; this.source = source; this.sourceOptions = sourceOptions; - this.target = target; + this.targetId = targetId; this.targetOptions = targetOptions; } + Builder targetInfo(BlobInfo targetInfo) { + this.targetInfo = targetInfo; + return this; + } + Builder result(BlobInfo result) { this.result = result; return this; @@ -220,15 +232,16 @@ RestorableState build() { } static Builder builder(StorageOptions options, BlobId source, - Map sourceOptions, BlobInfo target, + Map sourceOptions, BlobId targetId, Map targetOptions) { - return new Builder(options, source, sourceOptions, target, targetOptions); + return new Builder(options, source, sourceOptions, targetId, targetOptions); } @Override public CopyWriter restore() { - RewriteRequest rewriteRequest = new RewriteRequest( - source.toPb(), sourceOptions, target.toPb(), targetOptions, megabytesCopiedPerChunk); + RewriteRequest rewriteRequest = new RewriteRequest(source.toPb(), sourceOptions, + targetId.bucket(), targetId.name(), targetInfo != null ? targetInfo.toPb() : null, + targetOptions, megabytesCopiedPerChunk); RewriteResponse rewriteResponse = new RewriteResponse(rewriteRequest, result != null ? result.toPb() : null, blobSize, isDone, rewriteToken, totalBytesCopied); @@ -237,8 +250,9 @@ public CopyWriter restore() { @Override public int hashCode() { - return Objects.hash(serviceOptions, source, sourceOptions, target, targetOptions, result, - blobSize, isDone, megabytesCopiedPerChunk, rewriteToken, totalBytesCopied); + return Objects.hash(serviceOptions, source, sourceOptions, targetId, targetInfo, + targetOptions, result, blobSize, isDone, megabytesCopiedPerChunk, rewriteToken, + totalBytesCopied); } @Override @@ -253,7 +267,8 @@ public boolean equals(Object obj) { return Objects.equals(this.serviceOptions, other.serviceOptions) && Objects.equals(this.source, other.source) && Objects.equals(this.sourceOptions, other.sourceOptions) - && Objects.equals(this.target, other.target) + && Objects.equals(this.targetId, other.targetId) + && Objects.equals(this.targetInfo, other.targetInfo) && Objects.equals(this.targetOptions, other.targetOptions) && Objects.equals(this.result, other.result) && Objects.equals(this.rewriteToken, other.rewriteToken) @@ -267,10 +282,14 @@ public boolean equals(Object obj) { public String toString() { return MoreObjects.toStringHelper(this) .add("source", source) - .add("target", target) - .add("isDone", isDone) - .add("totalBytesRewritten", totalBytesCopied) + .add("targetId", targetId) + .add("targetInfo", targetInfo) + .add("result", result) .add("blobSize", blobSize) + .add("isDone", isDone) + .add("rewriteToken", rewriteToken) + .add("totalBytesCopied", totalBytesCopied) + .add("megabytesCopiedPerChunk", megabytesCopiedPerChunk) .toString(); } } diff --git a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/Storage.java b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/Storage.java index 35fc6117cbc2..8204f0105e7e 100644 --- a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/Storage.java +++ b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/Storage.java @@ -962,7 +962,8 @@ class CopyRequest implements Serializable { private final BlobId source; private final List sourceOptions; - private final BlobInfo target; + private final BlobId targetId; + private final BlobInfo targetInfo; private final List targetOptions; private final Long megabytesCopiedPerChunk; @@ -971,7 +972,8 @@ public static class Builder { private final Set sourceOptions = new LinkedHashSet<>(); private final Set targetOptions = new LinkedHashSet<>(); private BlobId source; - private BlobInfo target; + private BlobId targetId; + private BlobInfo targetInfo; private Long megabytesCopiedPerChunk; /** @@ -1019,39 +1021,37 @@ public Builder sourceOptions(Iterable options) { * * @return the builder */ - public Builder target(BlobId target) { - this.target = BlobInfo.builder(target).build(); + public Builder target(BlobId targetId) { + this.targetId = targetId; return this; } /** * Sets the copy target and target options. {@code target} parameter is used to override - * source blob information (e.g. {@code contentType}, {@code contentLanguage}). {@code - * target.contentType} is a required field. + * source blob information (e.g. {@code contentType}, {@code contentLanguage}). Target blob + * information is set exactly to {@code target}, no information is inherited from the source + * blob. If not set, target blob information is inherited from the source blob. * * @return the builder - * @throws IllegalArgumentException if {@code target.contentType} is {@code null} */ - public Builder target(BlobInfo target, BlobTargetOption... options) - throws IllegalArgumentException { - checkContentType(target); - this.target = target; + public Builder target(BlobInfo targetInfo, BlobTargetOption... options) { + this.targetId = targetInfo.blobId(); + this.targetInfo = targetInfo; Collections.addAll(targetOptions, options); return this; } /** * Sets the copy target and target options. {@code target} parameter is used to override - * source blob information (e.g. {@code contentType}, {@code contentLanguage}). {@code - * target.contentType} is a required field. + * source blob information (e.g. {@code contentType}, {@code contentLanguage}). Target blob + * information is set exactly to {@code target}, no information is inherited from the source + * blob. If not set, target blob information is inherited from the source blob. * * @return the builder - * @throws IllegalArgumentException if {@code target.contentType} is {@code null} */ - public Builder target(BlobInfo target, Iterable options) - throws IllegalArgumentException { - checkContentType(target); - this.target = target; + public Builder target(BlobInfo targetInfo, Iterable options) { + this.targetId = targetInfo.blobId(); + this.targetInfo = targetInfo; Iterables.addAll(targetOptions, options); return this; } @@ -1072,8 +1072,6 @@ public Builder megabytesCopiedPerChunk(Long megabytesCopiedPerChunk) { * Creates a {@code CopyRequest} object. */ public CopyRequest build() { - checkNotNull(source); - checkNotNull(target); return new CopyRequest(this); } } @@ -1081,7 +1079,8 @@ public CopyRequest build() { private CopyRequest(Builder builder) { source = checkNotNull(builder.source); sourceOptions = ImmutableList.copyOf(builder.sourceOptions); - target = checkNotNull(builder.target); + targetId = checkNotNull(builder.targetId); + targetInfo = builder.targetInfo; targetOptions = ImmutableList.copyOf(builder.targetOptions); megabytesCopiedPerChunk = builder.megabytesCopiedPerChunk; } @@ -1101,10 +1100,20 @@ public List sourceOptions() { } /** - * Returns the {@link BlobInfo} for the target blob. + * Returns the {@link BlobId} for the target blob. */ - public BlobInfo target() { - return target; + public BlobId targetId() { + return targetId; + } + + /** + * Returns the {@link BlobInfo} for the target blob. If set, this value is used to replace + * source blob information (e.g. {@code contentType}, {@code contentLanguage}). Target blob + * information is set exactly to this value, no information is inherited from the source blob. + * If not set, target blob information is inherited from the source blob. + */ + public BlobInfo targetInfo() { + return targetInfo; } /** @@ -1125,34 +1134,27 @@ public Long megabytesCopiedPerChunk() { /** * Creates a copy request. {@code target} parameter is used to override source blob information - * (e.g. {@code contentType}, {@code contentLanguage}). {@code target.contentType} is a required - * field. + * (e.g. {@code contentType}, {@code contentLanguage}). * * @param sourceBucket name of the bucket containing the source blob * @param sourceBlob name of the source blob * @param target a {@code BlobInfo} object for the target blob * @return a copy request - * @throws IllegalArgumentException if {@code target.contentType} is {@code null} */ - public static CopyRequest of(String sourceBucket, String sourceBlob, BlobInfo target) - throws IllegalArgumentException { - checkContentType(target); + public static CopyRequest of(String sourceBucket, String sourceBlob, BlobInfo target) { return builder().source(sourceBucket, sourceBlob).target(target).build(); } /** - * Creates a copy request. {@code target} parameter is used to override source blob information - * (e.g. {@code contentType}, {@code contentLanguage}). {@code target.contentType} is a required - * field. + * Creates a copy request. {@code target} parameter is used to replace source blob information + * (e.g. {@code contentType}, {@code contentLanguage}). Target blob information is set exactly + * to {@code target}, no information is inherited from the source blob. * * @param sourceBlobId a {@code BlobId} object for the source blob * @param target a {@code BlobInfo} object for the target blob * @return a copy request - * @throws IllegalArgumentException if {@code target.contentType} is {@code null} */ - public static CopyRequest of(BlobId sourceBlobId, BlobInfo target) - throws IllegalArgumentException { - checkContentType(target); + public static CopyRequest of(BlobId sourceBlobId, BlobInfo target) { return builder().source(sourceBlobId).target(target).build(); } @@ -1214,10 +1216,6 @@ public static CopyRequest of(BlobId sourceBlobId, BlobId targetBlobId) { public static Builder builder() { return new Builder(); } - - private static void checkContentType(BlobInfo blobInfo) throws IllegalArgumentException { - checkArgument(blobInfo.contentType() != null, "Blob content type can not be null"); - } } /** diff --git a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/StorageImpl.java b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/StorageImpl.java index d58c9e43aea9..30c0046b802c 100644 --- a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/StorageImpl.java +++ b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/StorageImpl.java @@ -413,15 +413,20 @@ public CopyWriter copy(final CopyRequest copyRequest) { final StorageObject source = copyRequest.source().toPb(); final Map sourceOptions = optionMap(copyRequest.source().generation(), null, copyRequest.sourceOptions(), true); - final StorageObject target = copyRequest.target().toPb(); - final Map targetOptions = optionMap(copyRequest.target().generation(), - copyRequest.target().metageneration(), copyRequest.targetOptions()); + final BlobId targetId = copyRequest.targetId(); + final StorageObject targetObject = + copyRequest.targetInfo() != null ? copyRequest.targetInfo().toPb() : null; + final Map targetOptions = optionMap( + copyRequest.targetInfo() != null ? copyRequest.targetInfo().generation() : null, + copyRequest.targetInfo() != null ? copyRequest.targetInfo().metageneration() : null, + copyRequest.targetOptions()); try { RewriteResponse rewriteResponse = runWithRetries(new Callable() { @Override public RewriteResponse call() { - return storageRpc.openRewrite(new StorageRpc.RewriteRequest(source, sourceOptions, target, - targetOptions, copyRequest.megabytesCopiedPerChunk())); + return storageRpc.openRewrite(new StorageRpc.RewriteRequest(source, sourceOptions, + targetId.bucket(), targetId.name(), targetObject, targetOptions, + copyRequest.megabytesCopiedPerChunk())); } }, options().retryParams(), EXCEPTION_HANDLER); return new CopyWriter(options(), rewriteResponse); diff --git a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/spi/DefaultStorageRpc.java b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/spi/DefaultStorageRpc.java index 3f465e0ab20e..7f13212556aa 100644 --- a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/spi/DefaultStorageRpc.java +++ b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/spi/DefaultStorageRpc.java @@ -580,8 +580,8 @@ private RewriteResponse rewrite(RewriteRequest req, String token) { Long maxBytesRewrittenPerCall = req.megabytesRewrittenPerCall != null ? req.megabytesRewrittenPerCall * MEGABYTE : null; com.google.api.services.storage.model.RewriteResponse rewriteResponse = storage.objects() - .rewrite(req.source.getBucket(), req.source.getName(), req.target.getBucket(), - req.target.getName(), req.target.getContentType() != null ? req.target : null) + .rewrite(req.source.getBucket(), req.source.getName(), req.targetBucket, + req.targetName, req.targetObject) .setSourceGeneration(req.source.getGeneration()) .setRewriteToken(token) .setMaxBytesRewrittenPerCall(maxBytesRewrittenPerCall) diff --git a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/spi/StorageRpc.java b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/spi/StorageRpc.java index d239a475a6dd..7256368e189f 100644 --- a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/spi/StorageRpc.java +++ b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/spi/StorageRpc.java @@ -138,16 +138,20 @@ class RewriteRequest { public final StorageObject source; public final Map sourceOptions; - public final StorageObject target; + public final String targetBucket; + public final String targetName; + public final StorageObject targetObject; public final Map targetOptions; public final Long megabytesRewrittenPerCall; public RewriteRequest(StorageObject source, Map sourceOptions, - StorageObject target, Map targetOptions, - Long megabytesRewrittenPerCall) { + String targetBucket, String targetName, StorageObject targetObject, + Map targetOptions, Long megabytesRewrittenPerCall) { this.source = source; this.sourceOptions = sourceOptions; - this.target = target; + this.targetBucket = targetBucket; + this.targetName = targetName; + this.targetObject = targetObject; this.targetOptions = targetOptions; this.megabytesRewrittenPerCall = megabytesRewrittenPerCall; } @@ -163,14 +167,17 @@ public boolean equals(Object obj) { final RewriteRequest other = (RewriteRequest) obj; return Objects.equals(this.source, other.source) && Objects.equals(this.sourceOptions, other.sourceOptions) - && Objects.equals(this.target, other.target) + && Objects.equals(this.targetBucket, other.targetBucket) + && Objects.equals(this.targetName, other.targetName) + && Objects.equals(this.targetObject, other.targetObject) && Objects.equals(this.targetOptions, other.targetOptions) && Objects.equals(this.megabytesRewrittenPerCall, other.megabytesRewrittenPerCall); } @Override public int hashCode() { - return Objects.hash(source, sourceOptions, target, targetOptions, megabytesRewrittenPerCall); + return Objects.hash(source, sourceOptions, targetBucket, targetName, targetObject, + targetOptions, megabytesRewrittenPerCall); } } diff --git a/gcloud-java-storage/src/test/java/com/google/gcloud/storage/BlobTest.java b/gcloud-java-storage/src/test/java/com/google/gcloud/storage/BlobTest.java index 5a6173c08199..2d4920816c38 100644 --- a/gcloud-java-storage/src/test/java/com/google/gcloud/storage/BlobTest.java +++ b/gcloud-java-storage/src/test/java/com/google/gcloud/storage/BlobTest.java @@ -222,7 +222,6 @@ public void testDelete() throws Exception { @Test public void testCopyToBucket() throws Exception { initializeExpectedBlob(2); - BlobInfo target = BlobInfo.builder(BlobId.of("bt", "n")).build(); CopyWriter copyWriter = createMock(CopyWriter.class); Capture capturedCopyRequest = Capture.newInstance(); expect(storage.options()).andReturn(mockOptions); @@ -232,7 +231,8 @@ public void testCopyToBucket() throws Exception { CopyWriter returnedCopyWriter = blob.copyTo("bt"); assertEquals(copyWriter, returnedCopyWriter); assertEquals(capturedCopyRequest.getValue().source(), blob.blobId()); - assertEquals(capturedCopyRequest.getValue().target(), target); + assertEquals(capturedCopyRequest.getValue().targetId(), BlobId.of("bt", "n")); + assertNull(capturedCopyRequest.getValue().targetInfo()); assertTrue(capturedCopyRequest.getValue().sourceOptions().isEmpty()); assertTrue(capturedCopyRequest.getValue().targetOptions().isEmpty()); } @@ -240,7 +240,6 @@ public void testCopyToBucket() throws Exception { @Test public void testCopyTo() throws Exception { initializeExpectedBlob(2); - BlobInfo target = BlobInfo.builder(BlobId.of("bt", "nt")).build(); CopyWriter copyWriter = createMock(CopyWriter.class); Capture capturedCopyRequest = Capture.newInstance(); expect(storage.options()).andReturn(mockOptions); @@ -250,7 +249,8 @@ public void testCopyTo() throws Exception { CopyWriter returnedCopyWriter = blob.copyTo("bt", "nt"); assertEquals(copyWriter, returnedCopyWriter); assertEquals(capturedCopyRequest.getValue().source(), blob.blobId()); - assertEquals(capturedCopyRequest.getValue().target(), target); + assertEquals(capturedCopyRequest.getValue().targetId(), BlobId.of("bt", "nt")); + assertNull(capturedCopyRequest.getValue().targetInfo()); assertTrue(capturedCopyRequest.getValue().sourceOptions().isEmpty()); assertTrue(capturedCopyRequest.getValue().targetOptions().isEmpty()); } @@ -260,7 +260,6 @@ public void testCopyToBlobId() throws Exception { initializeExpectedBlob(2); BlobId targetId = BlobId.of("bt", "nt"); CopyWriter copyWriter = createMock(CopyWriter.class); - BlobInfo target = BlobInfo.builder(targetId).build(); Capture capturedCopyRequest = Capture.newInstance(); expect(storage.options()).andReturn(mockOptions); expect(storage.copy(capture(capturedCopyRequest))).andReturn(copyWriter); @@ -269,7 +268,8 @@ public void testCopyToBlobId() throws Exception { CopyWriter returnedCopyWriter = blob.copyTo(targetId); assertEquals(copyWriter, returnedCopyWriter); assertEquals(capturedCopyRequest.getValue().source(), blob.blobId()); - assertEquals(capturedCopyRequest.getValue().target(), target); + assertEquals(capturedCopyRequest.getValue().targetId(), targetId); + assertNull(capturedCopyRequest.getValue().targetInfo()); assertTrue(capturedCopyRequest.getValue().sourceOptions().isEmpty()); assertTrue(capturedCopyRequest.getValue().targetOptions().isEmpty()); } diff --git a/gcloud-java-storage/src/test/java/com/google/gcloud/storage/CopyRequestTest.java b/gcloud-java-storage/src/test/java/com/google/gcloud/storage/CopyRequestTest.java index b7e8d14e53a1..5700ea804cd6 100644 --- a/gcloud-java-storage/src/test/java/com/google/gcloud/storage/CopyRequestTest.java +++ b/gcloud-java-storage/src/test/java/com/google/gcloud/storage/CopyRequestTest.java @@ -18,6 +18,7 @@ import static com.google.gcloud.storage.Storage.PredefinedAcl.PUBLIC_READ; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; import com.google.common.collect.ImmutableList; import com.google.gcloud.storage.Storage.BlobSourceOption; @@ -52,7 +53,8 @@ public void testCopyRequest() { assertEquals(SOURCE_BLOB_ID, copyRequest1.source()); assertEquals(1, copyRequest1.sourceOptions().size()); assertEquals(BlobSourceOption.generationMatch(1), copyRequest1.sourceOptions().get(0)); - assertEquals(TARGET_BLOB_INFO, copyRequest1.target()); + assertEquals(TARGET_BLOB_INFO.blobId(), copyRequest1.targetId()); + assertEquals(TARGET_BLOB_INFO, copyRequest1.targetInfo()); assertEquals(1, copyRequest1.targetOptions().size()); assertEquals(BlobTargetOption.predefinedAcl(PUBLIC_READ), copyRequest1.targetOptions().get(0)); @@ -61,14 +63,16 @@ public void testCopyRequest() { .target(TARGET_BLOB_ID) .build(); assertEquals(SOURCE_BLOB_ID, copyRequest2.source()); - assertEquals(BlobInfo.builder(TARGET_BLOB_ID).build(), copyRequest2.target()); + assertEquals(TARGET_BLOB_ID, copyRequest2.targetId()); + assertNull(copyRequest2.targetInfo()); Storage.CopyRequest copyRequest3 = Storage.CopyRequest.builder() .source(SOURCE_BLOB_ID) .target(TARGET_BLOB_INFO, ImmutableList.of(BlobTargetOption.predefinedAcl(PUBLIC_READ))) .build(); assertEquals(SOURCE_BLOB_ID, copyRequest3.source()); - assertEquals(TARGET_BLOB_INFO, copyRequest3.target()); + assertEquals(TARGET_BLOB_INFO.blobId(), copyRequest3.targetId()); + assertEquals(TARGET_BLOB_INFO, copyRequest3.targetInfo()); assertEquals(ImmutableList.of(BlobTargetOption.predefinedAcl(PUBLIC_READ)), copyRequest3.targetOptions()); } @@ -77,53 +81,34 @@ public void testCopyRequest() { public void testCopyRequestOf() { Storage.CopyRequest copyRequest1 = Storage.CopyRequest.of(SOURCE_BLOB_ID, TARGET_BLOB_INFO); assertEquals(SOURCE_BLOB_ID, copyRequest1.source()); - assertEquals(TARGET_BLOB_INFO, copyRequest1.target()); + assertEquals(TARGET_BLOB_INFO.blobId(), copyRequest1.targetId()); + assertEquals(TARGET_BLOB_INFO, copyRequest1.targetInfo()); Storage.CopyRequest copyRequest2 = Storage.CopyRequest.of(SOURCE_BLOB_ID, TARGET_BLOB_NAME); assertEquals(SOURCE_BLOB_ID, copyRequest2.source()); - assertEquals(BlobInfo.builder(SOURCE_BUCKET_NAME, TARGET_BLOB_NAME).build(), - copyRequest2.target()); + assertEquals(BlobId.of(SOURCE_BUCKET_NAME, TARGET_BLOB_NAME), copyRequest2.targetId()); + assertNull(copyRequest2.targetInfo()); Storage.CopyRequest copyRequest3 = Storage.CopyRequest.of(SOURCE_BUCKET_NAME, SOURCE_BLOB_NAME, TARGET_BLOB_INFO); assertEquals(SOURCE_BLOB_ID, copyRequest3.source()); - assertEquals(TARGET_BLOB_INFO, copyRequest3.target()); + assertEquals(TARGET_BLOB_INFO.blobId(), copyRequest3.targetId()); + assertEquals(TARGET_BLOB_INFO, copyRequest3.targetInfo()); Storage.CopyRequest copyRequest4 = Storage.CopyRequest.of(SOURCE_BUCKET_NAME, SOURCE_BLOB_NAME, TARGET_BLOB_NAME); assertEquals(SOURCE_BLOB_ID, copyRequest4.source()); - assertEquals(BlobInfo.builder(SOURCE_BUCKET_NAME, TARGET_BLOB_NAME).build(), - copyRequest4.target()); + assertEquals(BlobId.of(SOURCE_BUCKET_NAME, TARGET_BLOB_NAME), copyRequest4.targetId()); + assertNull(copyRequest4.targetInfo()); Storage.CopyRequest copyRequest5 = Storage.CopyRequest.of(SOURCE_BLOB_ID, TARGET_BLOB_ID); assertEquals(SOURCE_BLOB_ID, copyRequest5.source()); - assertEquals(BlobInfo.builder(TARGET_BLOB_ID).build(), copyRequest5.target()); + assertEquals(TARGET_BLOB_ID, copyRequest5.targetId()); + assertNull(copyRequest5.targetInfo()); Storage.CopyRequest copyRequest6 = Storage.CopyRequest.of(SOURCE_BUCKET_NAME, SOURCE_BLOB_NAME, TARGET_BLOB_ID); assertEquals(SOURCE_BLOB_ID, copyRequest6.source()); - assertEquals(BlobInfo.builder(TARGET_BLOB_ID).build(), copyRequest6.target()); - } - - @Test - public void testCopyRequestFail() { - thrown.expect(IllegalArgumentException.class); - Storage.CopyRequest.builder() - .source(SOURCE_BLOB_ID) - .target(BlobInfo.builder(TARGET_BLOB_ID).build()) - .build(); - } - - @Test - public void testCopyRequestOfBlobInfoFail() { - thrown.expect(IllegalArgumentException.class); - Storage.CopyRequest.of(SOURCE_BLOB_ID, BlobInfo.builder(TARGET_BLOB_ID).build()); - } - - @Test - public void testCopyRequestOfStringFail() { - thrown.expect(IllegalArgumentException.class); - Storage.CopyRequest.of( - SOURCE_BUCKET_NAME, SOURCE_BLOB_NAME, BlobInfo.builder(TARGET_BLOB_ID).build()); - } + assertEquals(TARGET_BLOB_ID, copyRequest6.targetId()); + assertNull(copyRequest6.targetInfo()); } } diff --git a/gcloud-java-storage/src/test/java/com/google/gcloud/storage/CopyWriterTest.java b/gcloud-java-storage/src/test/java/com/google/gcloud/storage/CopyWriterTest.java index ad4a04c34127..75b863075770 100644 --- a/gcloud-java-storage/src/test/java/com/google/gcloud/storage/CopyWriterTest.java +++ b/gcloud-java-storage/src/test/java/com/google/gcloud/storage/CopyWriterTest.java @@ -48,20 +48,29 @@ public class CopyWriterTest { private static final BlobId BLOB_ID = BlobId.of(SOURCE_BUCKET_NAME, SOURCE_BLOB_NAME); private static final BlobInfo BLOB_INFO = BlobInfo.builder(DESTINATION_BUCKET_NAME, DESTINATION_BLOB_NAME).build(); - private static final BlobInfo RESULT = + private static final BlobInfo RESULT_INFO = BlobInfo.builder(DESTINATION_BUCKET_NAME, DESTINATION_BLOB_NAME).contentType("type").build(); private static final Map EMPTY_OPTIONS = ImmutableMap.of(); - private static final RewriteRequest REQUEST = new StorageRpc.RewriteRequest(BLOB_ID.toPb(), - EMPTY_OPTIONS, BLOB_INFO.toPb(), EMPTY_OPTIONS, null); - private static final RewriteResponse RESPONSE = new StorageRpc.RewriteResponse(REQUEST, - null, 42L, false, "token", 21L); - private static final RewriteResponse RESPONSE_DONE = new StorageRpc.RewriteResponse(REQUEST, - RESULT.toPb(), 42L, true, "token", 42L); + private static final RewriteRequest REQUEST_WITH_OBJECT = + new StorageRpc.RewriteRequest(BLOB_ID.toPb(), EMPTY_OPTIONS, DESTINATION_BUCKET_NAME, + DESTINATION_BLOB_NAME, BLOB_INFO.toPb(), EMPTY_OPTIONS, null); + private static final RewriteRequest REQUEST_WITHOUT_OBJECT = + new StorageRpc.RewriteRequest(BLOB_ID.toPb(), EMPTY_OPTIONS, DESTINATION_BUCKET_NAME, + DESTINATION_BLOB_NAME, null, EMPTY_OPTIONS, null); + private static final RewriteResponse RESPONSE_WITH_OBJECT = new RewriteResponse( + REQUEST_WITH_OBJECT, null, 42L, false, "token", 21L); + private static final RewriteResponse RESPONSE_WITHOUT_OBJECT = new RewriteResponse( + REQUEST_WITHOUT_OBJECT, null, 42L, false, "token", 21L); + private static final RewriteResponse RESPONSE_WITH_OBJECT_DONE = + new RewriteResponse(REQUEST_WITH_OBJECT, RESULT_INFO.toPb(), 42L, true, "token", 42L); + private static final RewriteResponse RESPONSE_WITHOUT_OBJECT_DONE = + new RewriteResponse(REQUEST_WITHOUT_OBJECT, RESULT_INFO.toPb(), 42L, true, "token", 42L); private StorageOptions options; private StorageRpcFactory rpcFactoryMock; private StorageRpc storageRpcMock; private CopyWriter copyWriter; + private Blob result; @Before public void setUp() { @@ -75,6 +84,7 @@ public void setUp() { .serviceRpcFactory(rpcFactoryMock) .retryParams(RetryParams.noRetries()) .build(); + result = new Blob(options.service(), new BlobInfo.BuilderImpl(RESULT_INFO)); } @After @@ -83,41 +93,111 @@ public void tearDown() throws Exception { } @Test - public void testRewrite() { - EasyMock.expect(storageRpcMock.continueRewrite(RESPONSE)).andReturn(RESPONSE_DONE); + public void testRewriteWithObject() { + EasyMock.expect(storageRpcMock.continueRewrite(RESPONSE_WITH_OBJECT)) + .andReturn(RESPONSE_WITH_OBJECT_DONE); EasyMock.replay(storageRpcMock); - copyWriter = new CopyWriter(options, RESPONSE); - assertEquals(RESULT, copyWriter.result()); + copyWriter = new CopyWriter(options, RESPONSE_WITH_OBJECT); + assertEquals(result, copyWriter.result()); assertTrue(copyWriter.isDone()); assertEquals(42L, copyWriter.totalBytesCopied()); assertEquals(42L, copyWriter.blobSize()); } @Test - public void testRewriteMultipleRequests() { - EasyMock.expect(storageRpcMock.continueRewrite(RESPONSE)).andReturn(RESPONSE); - EasyMock.expect(storageRpcMock.continueRewrite(RESPONSE)).andReturn(RESPONSE_DONE); + public void testRewriteWithoutObject() { + EasyMock.expect(storageRpcMock.continueRewrite(RESPONSE_WITHOUT_OBJECT)) + .andReturn(RESPONSE_WITHOUT_OBJECT_DONE); EasyMock.replay(storageRpcMock); - copyWriter = new CopyWriter(options, RESPONSE); - assertEquals(RESULT, copyWriter.result()); + copyWriter = new CopyWriter(options, RESPONSE_WITHOUT_OBJECT); + assertEquals(result, copyWriter.result()); assertTrue(copyWriter.isDone()); assertEquals(42L, copyWriter.totalBytesCopied()); assertEquals(42L, copyWriter.blobSize()); } @Test - public void testSaveAndRestore() { - EasyMock.expect(storageRpcMock.continueRewrite(RESPONSE)).andReturn(RESPONSE); - EasyMock.expect(storageRpcMock.continueRewrite(RESPONSE)).andReturn(RESPONSE_DONE); + public void testRewriteWithObjectMultipleRequests() { + EasyMock.expect(storageRpcMock.continueRewrite(RESPONSE_WITH_OBJECT)) + .andReturn(RESPONSE_WITH_OBJECT); + EasyMock.expect(storageRpcMock.continueRewrite(RESPONSE_WITH_OBJECT)) + .andReturn(RESPONSE_WITH_OBJECT_DONE); EasyMock.replay(storageRpcMock); - copyWriter = new CopyWriter(options, RESPONSE); + copyWriter = new CopyWriter(options, RESPONSE_WITH_OBJECT); + assertEquals(result, copyWriter.result()); + assertTrue(copyWriter.isDone()); + assertEquals(42L, copyWriter.totalBytesCopied()); + assertEquals(42L, copyWriter.blobSize()); + } + + @Test + public void testRewriteWithoutObjectMultipleRequests() { + EasyMock.expect(storageRpcMock.continueRewrite(RESPONSE_WITHOUT_OBJECT)) + .andReturn(RESPONSE_WITHOUT_OBJECT); + EasyMock.expect(storageRpcMock.continueRewrite(RESPONSE_WITHOUT_OBJECT)) + .andReturn(RESPONSE_WITHOUT_OBJECT_DONE); + EasyMock.replay(storageRpcMock); + copyWriter = new CopyWriter(options, RESPONSE_WITHOUT_OBJECT); + assertEquals(result, copyWriter.result()); + assertTrue(copyWriter.isDone()); + assertEquals(42L, copyWriter.totalBytesCopied()); + assertEquals(42L, copyWriter.blobSize()); + } + + @Test + public void testSaveAndRestoreWithObject() { + EasyMock.expect(storageRpcMock.continueRewrite(RESPONSE_WITH_OBJECT)) + .andReturn(RESPONSE_WITH_OBJECT); + EasyMock.expect(storageRpcMock.continueRewrite(RESPONSE_WITH_OBJECT)) + .andReturn(RESPONSE_WITH_OBJECT_DONE); + EasyMock.replay(storageRpcMock); + copyWriter = new CopyWriter(options, RESPONSE_WITH_OBJECT); + copyWriter.copyChunk(); + assertTrue(!copyWriter.isDone()); + assertEquals(21L, copyWriter.totalBytesCopied()); + assertEquals(42L, copyWriter.blobSize()); + RestorableState rewriterState = copyWriter.capture(); + CopyWriter restoredRewriter = rewriterState.restore(); + assertEquals(result, restoredRewriter.result()); + assertTrue(restoredRewriter.isDone()); + assertEquals(42L, restoredRewriter.totalBytesCopied()); + assertEquals(42L, restoredRewriter.blobSize()); + } + + @Test + public void testSaveAndRestoreWithoutObject() { + EasyMock.expect(storageRpcMock.continueRewrite(RESPONSE_WITHOUT_OBJECT)) + .andReturn(RESPONSE_WITHOUT_OBJECT); + EasyMock.expect(storageRpcMock.continueRewrite(RESPONSE_WITHOUT_OBJECT)) + .andReturn(RESPONSE_WITHOUT_OBJECT_DONE); + EasyMock.replay(storageRpcMock); + copyWriter = new CopyWriter(options, RESPONSE_WITHOUT_OBJECT); copyWriter.copyChunk(); assertTrue(!copyWriter.isDone()); assertEquals(21L, copyWriter.totalBytesCopied()); assertEquals(42L, copyWriter.blobSize()); RestorableState rewriterState = copyWriter.capture(); CopyWriter restoredRewriter = rewriterState.restore(); - assertEquals(RESULT, restoredRewriter.result()); + assertEquals(result, restoredRewriter.result()); + assertTrue(restoredRewriter.isDone()); + assertEquals(42L, restoredRewriter.totalBytesCopied()); + assertEquals(42L, restoredRewriter.blobSize()); + } + + @Test + public void testSaveAndRestoreWithResult() { + EasyMock.expect(storageRpcMock.continueRewrite(RESPONSE_WITH_OBJECT)) + .andReturn(RESPONSE_WITH_OBJECT_DONE); + EasyMock.replay(storageRpcMock); + copyWriter = new CopyWriter(options, RESPONSE_WITH_OBJECT); + copyWriter.copyChunk(); + assertEquals(result, copyWriter.result()); + assertTrue(copyWriter.isDone()); + assertEquals(42L, copyWriter.totalBytesCopied()); + assertEquals(42L, copyWriter.blobSize()); + RestorableState rewriterState = copyWriter.capture(); + CopyWriter restoredRewriter = rewriterState.restore(); + assertEquals(result, restoredRewriter.result()); assertTrue(restoredRewriter.isDone()); assertEquals(42L, restoredRewriter.totalBytesCopied()); assertEquals(42L, restoredRewriter.blobSize()); diff --git a/gcloud-java-storage/src/test/java/com/google/gcloud/storage/StorageImplTest.java b/gcloud-java-storage/src/test/java/com/google/gcloud/storage/StorageImplTest.java index 38b4bb58e77f..db1166598237 100644 --- a/gcloud-java-storage/src/test/java/com/google/gcloud/storage/StorageImplTest.java +++ b/gcloud-java-storage/src/test/java/com/google/gcloud/storage/StorageImplTest.java @@ -866,7 +866,8 @@ public void testComposeWithOptions() { public void testCopy() { CopyRequest request = Storage.CopyRequest.of(BLOB_INFO1.blobId(), BLOB_INFO2.blobId()); StorageRpc.RewriteRequest rpcRequest = new StorageRpc.RewriteRequest(request.source().toPb(), - EMPTY_RPC_OPTIONS, request.target().toPb(), EMPTY_RPC_OPTIONS, null); + EMPTY_RPC_OPTIONS, BLOB_INFO2.blobId().bucket(), BLOB_INFO2.blobId().name(), null, + EMPTY_RPC_OPTIONS, null); StorageRpc.RewriteResponse rpcResponse = new StorageRpc.RewriteResponse(rpcRequest, null, 42L, false, "token", 21L); EasyMock.expect(storageRpcMock.openRewrite(rpcRequest)).andReturn(rpcResponse); @@ -886,7 +887,8 @@ public void testCopyWithOptions() { .target(BLOB_INFO1, BLOB_TARGET_GENERATION, BLOB_TARGET_METAGENERATION) .build(); StorageRpc.RewriteRequest rpcRequest = new StorageRpc.RewriteRequest(request.source().toPb(), - BLOB_SOURCE_OPTIONS_COPY, request.target().toPb(), BLOB_TARGET_OPTIONS_COMPOSE, null); + BLOB_SOURCE_OPTIONS_COPY, BLOB_INFO1.blobId().bucket(), BLOB_INFO1.blobId().name(), + request.targetInfo().toPb(), BLOB_TARGET_OPTIONS_COMPOSE, null); StorageRpc.RewriteResponse rpcResponse = new StorageRpc.RewriteResponse(rpcRequest, null, 42L, false, "token", 21L); EasyMock.expect(storageRpcMock.openRewrite(rpcRequest)).andReturn(rpcResponse); @@ -906,7 +908,8 @@ public void testCopyWithOptionsFromBlobId() { .target(BLOB_INFO1, BLOB_TARGET_GENERATION, BLOB_TARGET_METAGENERATION) .build(); StorageRpc.RewriteRequest rpcRequest = new StorageRpc.RewriteRequest(request.source().toPb(), - BLOB_SOURCE_OPTIONS_COPY, request.target().toPb(), BLOB_TARGET_OPTIONS_COMPOSE, null); + BLOB_SOURCE_OPTIONS_COPY, BLOB_INFO1.blobId().bucket(), BLOB_INFO1.blobId().name(), + request.targetInfo().toPb(), BLOB_TARGET_OPTIONS_COMPOSE, null); StorageRpc.RewriteResponse rpcResponse = new StorageRpc.RewriteResponse(rpcRequest, null, 42L, false, "token", 21L); EasyMock.expect(storageRpcMock.openRewrite(rpcRequest)).andReturn(rpcResponse); @@ -922,7 +925,8 @@ public void testCopyWithOptionsFromBlobId() { public void testCopyMultipleRequests() { CopyRequest request = Storage.CopyRequest.of(BLOB_INFO1.blobId(), BLOB_INFO2.blobId()); StorageRpc.RewriteRequest rpcRequest = new StorageRpc.RewriteRequest(request.source().toPb(), - EMPTY_RPC_OPTIONS, request.target().toPb(), EMPTY_RPC_OPTIONS, null); + EMPTY_RPC_OPTIONS, BLOB_INFO2.blobId().bucket(), BLOB_INFO2.blobId().name(), null, + EMPTY_RPC_OPTIONS, null); StorageRpc.RewriteResponse rpcResponse1 = new StorageRpc.RewriteResponse(rpcRequest, null, 42L, false, "token", 21L); StorageRpc.RewriteResponse rpcResponse2 = new StorageRpc.RewriteResponse(rpcRequest, @@ -935,7 +939,7 @@ public void testCopyMultipleRequests() { assertEquals(42L, writer.blobSize()); assertEquals(21L, writer.totalBytesCopied()); assertTrue(!writer.isDone()); - assertEquals(BLOB_INFO1, writer.result()); + assertEquals(expectedBlob1, writer.result()); assertTrue(writer.isDone()); assertEquals(42L, writer.totalBytesCopied()); assertEquals(42L, writer.blobSize()); diff --git a/gcloud-java-storage/src/test/java/com/google/gcloud/storage/it/ITStorageTest.java b/gcloud-java-storage/src/test/java/com/google/gcloud/storage/it/ITStorageTest.java index 563a621c48fb..13d768442c34 100644 --- a/gcloud-java-storage/src/test/java/com/google/gcloud/storage/it/ITStorageTest.java +++ b/gcloud-java-storage/src/test/java/com/google/gcloud/storage/it/ITStorageTest.java @@ -599,6 +599,37 @@ public void testComposeBlob() { assertNotNull(remoteTargetBlob); assertEquals(targetBlob.name(), remoteTargetBlob.name()); assertEquals(targetBlob.bucket(), remoteTargetBlob.bucket()); + assertNull(remoteTargetBlob.contentType()); + byte[] readBytes = storage.readAllBytes(BUCKET, targetBlobName); + byte[] composedBytes = Arrays.copyOf(BLOB_BYTE_CONTENT, BLOB_BYTE_CONTENT.length * 2); + System.arraycopy(BLOB_BYTE_CONTENT, 0, composedBytes, BLOB_BYTE_CONTENT.length, + BLOB_BYTE_CONTENT.length); + assertArrayEquals(composedBytes, readBytes); + assertTrue(remoteSourceBlob1.delete()); + assertTrue(remoteSourceBlob2.delete()); + assertTrue(remoteTargetBlob.delete()); + } + + @Test + public void testComposeBlobWithContentType() { + String sourceBlobName1 = "test-compose-blob-with-content-type-source-1"; + String sourceBlobName2 = "test-compose-blob-with-content-type-source-2"; + BlobInfo sourceBlob1 = BlobInfo.builder(BUCKET, sourceBlobName1).build(); + BlobInfo sourceBlob2 = BlobInfo.builder(BUCKET, sourceBlobName2).build(); + Blob remoteSourceBlob1 = storage.create(sourceBlob1, BLOB_BYTE_CONTENT); + Blob remoteSourceBlob2 = storage.create(sourceBlob2, BLOB_BYTE_CONTENT); + assertNotNull(remoteSourceBlob1); + assertNotNull(remoteSourceBlob2); + String targetBlobName = "test-compose-blob-with-content-type-target"; + BlobInfo targetBlob = + BlobInfo.builder(BUCKET, targetBlobName).contentType(CONTENT_TYPE).build(); + Storage.ComposeRequest req = + Storage.ComposeRequest.of(ImmutableList.of(sourceBlobName1, sourceBlobName2), targetBlob); + Blob remoteTargetBlob = storage.compose(req); + assertNotNull(remoteTargetBlob); + assertEquals(targetBlob.name(), remoteTargetBlob.name()); + assertEquals(targetBlob.bucket(), remoteTargetBlob.bucket()); + assertEquals(CONTENT_TYPE, remoteTargetBlob.contentType()); byte[] readBytes = storage.readAllBytes(BUCKET, targetBlobName); byte[] composedBytes = Arrays.copyOf(BLOB_BYTE_CONTENT, BLOB_BYTE_CONTENT.length * 2); System.arraycopy(BLOB_BYTE_CONTENT, 0, composedBytes, BLOB_BYTE_CONTENT.length, @@ -682,6 +713,26 @@ public void testCopyBlobUpdateMetadata() { assertTrue(storage.delete(BUCKET, targetBlobName)); } + @Test + public void testCopyBlobNoContentType() { + String sourceBlobName = "test-copy-blob-no-content-type-source"; + BlobId source = BlobId.of(BUCKET, sourceBlobName); + Blob remoteSourceBlob = storage.create(BlobInfo.builder(source).build(), BLOB_BYTE_CONTENT); + assertNotNull(remoteSourceBlob); + String targetBlobName = "test-copy-blob-no-content-type-target"; + ImmutableMap metadata = ImmutableMap.of("k", "v"); + BlobInfo target = BlobInfo.builder(BUCKET, targetBlobName).metadata(metadata).build(); + Storage.CopyRequest req = Storage.CopyRequest.of(source, target); + CopyWriter copyWriter = storage.copy(req); + assertEquals(BUCKET, copyWriter.result().bucket()); + assertEquals(targetBlobName, copyWriter.result().name()); + assertNull(copyWriter.result().contentType()); + assertEquals(metadata, copyWriter.result().metadata()); + assertTrue(copyWriter.isDone()); + assertTrue(remoteSourceBlob.delete()); + assertTrue(storage.delete(BUCKET, targetBlobName)); + } + @Test public void testCopyBlobFail() { String sourceBlobName = "test-copy-blob-source-fail"; From 3e5db7a87013d677d179d6d484448fcae3cbdbc6 Mon Sep 17 00:00:00 2001 From: Marco Ziccardi Date: Sun, 20 Mar 2016 21:05:16 +0100 Subject: [PATCH 4/6] Refactor CopyRequest: remove targetId, add boolean overrideInfo --- .../com/google/gcloud/storage/CopyWriter.java | 47 ++++++++--------- .../com/google/gcloud/storage/Storage.java | 50 ++++++++++--------- .../google/gcloud/storage/StorageImpl.java | 12 ++--- .../gcloud/storage/spi/DefaultStorageRpc.java | 4 +- .../google/gcloud/storage/spi/StorageRpc.java | 23 ++++----- .../com/google/gcloud/storage/BlobTest.java | 15 +++--- .../gcloud/storage/CopyRequestTest.java | 42 +++++++++------- .../google/gcloud/storage/CopyWriterTest.java | 8 +-- .../gcloud/storage/StorageImplTest.java | 12 ++--- 9 files changed, 102 insertions(+), 111 deletions(-) diff --git a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/CopyWriter.java b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/CopyWriter.java index 38d9c0d5e952..26c728942a28 100644 --- a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/CopyWriter.java +++ b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/CopyWriter.java @@ -120,11 +120,9 @@ public RestorableState capture() { serviceOptions, BlobId.fromPb(rewriteResponse.rewriteRequest.source), rewriteResponse.rewriteRequest.sourceOptions, - BlobId.of(rewriteResponse.rewriteRequest.targetBucket, - rewriteResponse.rewriteRequest.targetName), + rewriteResponse.rewriteRequest.overrideInfo, + BlobInfo.fromPb(rewriteResponse.rewriteRequest.target), rewriteResponse.rewriteRequest.targetOptions) - .targetInfo(rewriteResponse.rewriteRequest.targetObject != null - ? BlobInfo.fromPb(rewriteResponse.rewriteRequest.targetObject) : null) .result(rewriteResponse.result != null ? BlobInfo.fromPb(rewriteResponse.result) : null) .blobSize(blobSize()) .isDone(isDone()) @@ -141,8 +139,8 @@ static class StateImpl implements RestorableState, Serializable { private final StorageOptions serviceOptions; private final BlobId source; private final Map sourceOptions; - private final BlobId targetId; - private final BlobInfo targetInfo; + private final boolean overrideInfo; + private final BlobInfo target; private final Map targetOptions; private final BlobInfo result; private final long blobSize; @@ -155,8 +153,8 @@ static class StateImpl implements RestorableState, Serializable { this.serviceOptions = builder.serviceOptions; this.source = builder.source; this.sourceOptions = builder.sourceOptions; - this.targetId = builder.targetId; - this.targetInfo = builder.targetInfo; + this.overrideInfo = builder.overrideInfo; + this.target = builder.target; this.targetOptions = builder.targetOptions; this.result = builder.result; this.blobSize = builder.blobSize; @@ -171,9 +169,9 @@ static class Builder { private final StorageOptions serviceOptions; private final BlobId source; private final Map sourceOptions; - private final BlobId targetId; + private final boolean overrideInfo; + private BlobInfo target; private final Map targetOptions; - private BlobInfo targetInfo; private BlobInfo result; private long blobSize; private boolean isDone; @@ -182,20 +180,16 @@ static class Builder { private Long megabytesCopiedPerChunk; private Builder(StorageOptions options, BlobId source, - Map sourceOptions, - BlobId targetId, Map targetOptions) { + Map sourceOptions, boolean overrideInfo, BlobInfo target, + Map targetOptions) { this.serviceOptions = options; this.source = source; this.sourceOptions = sourceOptions; - this.targetId = targetId; + this.overrideInfo = overrideInfo; + this.target = target; this.targetOptions = targetOptions; } - Builder targetInfo(BlobInfo targetInfo) { - this.targetInfo = targetInfo; - return this; - } - Builder result(BlobInfo result) { this.result = result; return this; @@ -232,16 +226,15 @@ RestorableState build() { } static Builder builder(StorageOptions options, BlobId source, - Map sourceOptions, BlobId targetId, + Map sourceOptions, boolean overrideInfo, BlobInfo target, Map targetOptions) { - return new Builder(options, source, sourceOptions, targetId, targetOptions); + return new Builder(options, source, sourceOptions, overrideInfo, target, targetOptions); } @Override public CopyWriter restore() { RewriteRequest rewriteRequest = new RewriteRequest(source.toPb(), sourceOptions, - targetId.bucket(), targetId.name(), targetInfo != null ? targetInfo.toPb() : null, - targetOptions, megabytesCopiedPerChunk); + overrideInfo, target.toPb(), targetOptions, megabytesCopiedPerChunk); RewriteResponse rewriteResponse = new RewriteResponse(rewriteRequest, result != null ? result.toPb() : null, blobSize, isDone, rewriteToken, totalBytesCopied); @@ -250,7 +243,7 @@ public CopyWriter restore() { @Override public int hashCode() { - return Objects.hash(serviceOptions, source, sourceOptions, targetId, targetInfo, + return Objects.hash(serviceOptions, source, sourceOptions, overrideInfo, target, targetOptions, result, blobSize, isDone, megabytesCopiedPerChunk, rewriteToken, totalBytesCopied); } @@ -267,8 +260,8 @@ public boolean equals(Object obj) { return Objects.equals(this.serviceOptions, other.serviceOptions) && Objects.equals(this.source, other.source) && Objects.equals(this.sourceOptions, other.sourceOptions) - && Objects.equals(this.targetId, other.targetId) - && Objects.equals(this.targetInfo, other.targetInfo) + && Objects.equals(this.overrideInfo, other.overrideInfo) + && Objects.equals(this.target, other.target) && Objects.equals(this.targetOptions, other.targetOptions) && Objects.equals(this.result, other.result) && Objects.equals(this.rewriteToken, other.rewriteToken) @@ -282,8 +275,8 @@ public boolean equals(Object obj) { public String toString() { return MoreObjects.toStringHelper(this) .add("source", source) - .add("targetId", targetId) - .add("targetInfo", targetInfo) + .add("overrideInfo", overrideInfo) + .add("target", target) .add("result", result) .add("blobSize", blobSize) .add("isDone", isDone) diff --git a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/Storage.java b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/Storage.java index 8204f0105e7e..b30d46431de0 100644 --- a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/Storage.java +++ b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/Storage.java @@ -962,8 +962,8 @@ class CopyRequest implements Serializable { private final BlobId source; private final List sourceOptions; - private final BlobId targetId; - private final BlobInfo targetInfo; + private final boolean overrideInfo; + private final BlobInfo target; private final List targetOptions; private final Long megabytesCopiedPerChunk; @@ -972,8 +972,8 @@ public static class Builder { private final Set sourceOptions = new LinkedHashSet<>(); private final Set targetOptions = new LinkedHashSet<>(); private BlobId source; - private BlobId targetId; - private BlobInfo targetInfo; + private boolean overrideInfo; + private BlobInfo target; private Long megabytesCopiedPerChunk; /** @@ -1022,7 +1022,8 @@ public Builder sourceOptions(Iterable options) { * @return the builder */ public Builder target(BlobId targetId) { - this.targetId = targetId; + this.overrideInfo = false; + this.target = BlobInfo.builder(targetId).build(); return this; } @@ -1030,13 +1031,13 @@ public Builder target(BlobId targetId) { * Sets the copy target and target options. {@code target} parameter is used to override * source blob information (e.g. {@code contentType}, {@code contentLanguage}). Target blob * information is set exactly to {@code target}, no information is inherited from the source - * blob. If not set, target blob information is inherited from the source blob. + * blob. * * @return the builder */ - public Builder target(BlobInfo targetInfo, BlobTargetOption... options) { - this.targetId = targetInfo.blobId(); - this.targetInfo = targetInfo; + public Builder target(BlobInfo target, BlobTargetOption... options) { + this.overrideInfo = true; + this.target = checkNotNull(target); Collections.addAll(targetOptions, options); return this; } @@ -1045,13 +1046,13 @@ public Builder target(BlobInfo targetInfo, BlobTargetOption... options) { * Sets the copy target and target options. {@code target} parameter is used to override * source blob information (e.g. {@code contentType}, {@code contentLanguage}). Target blob * information is set exactly to {@code target}, no information is inherited from the source - * blob. If not set, target blob information is inherited from the source blob. + * blob. * * @return the builder */ - public Builder target(BlobInfo targetInfo, Iterable options) { - this.targetId = targetInfo.blobId(); - this.targetInfo = targetInfo; + public Builder target(BlobInfo target, Iterable options) { + this.overrideInfo = true; + this.target = checkNotNull(target); Iterables.addAll(targetOptions, options); return this; } @@ -1079,8 +1080,8 @@ public CopyRequest build() { private CopyRequest(Builder builder) { source = checkNotNull(builder.source); sourceOptions = ImmutableList.copyOf(builder.sourceOptions); - targetId = checkNotNull(builder.targetId); - targetInfo = builder.targetInfo; + overrideInfo = builder.overrideInfo; + target = checkNotNull(builder.target); targetOptions = ImmutableList.copyOf(builder.targetOptions); megabytesCopiedPerChunk = builder.megabytesCopiedPerChunk; } @@ -1100,20 +1101,21 @@ public List sourceOptions() { } /** - * Returns the {@link BlobId} for the target blob. + * Returns the {@link BlobInfo} for the target blob. */ - public BlobId targetId() { - return targetId; + public BlobInfo target() { + return target; } /** - * Returns the {@link BlobInfo} for the target blob. If set, this value is used to replace - * source blob information (e.g. {@code contentType}, {@code contentLanguage}). Target blob - * information is set exactly to this value, no information is inherited from the source blob. - * If not set, target blob information is inherited from the source blob. + * Returns whether to override the target blob information with {@link #target()}. + * If {@code true}, the value of {@link #target()} is used to replace source blob information + * (e.g. {@code contentType}, {@code contentLanguage}). Target blob information is set exactly + * to this value, no information is inherited from the source blob. If {@code false}, target + * blob information is inherited from the source blob. */ - public BlobInfo targetInfo() { - return targetInfo; + public boolean overrideInfo() { + return overrideInfo; } /** diff --git a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/StorageImpl.java b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/StorageImpl.java index 30c0046b802c..cf709ba5e293 100644 --- a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/StorageImpl.java +++ b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/StorageImpl.java @@ -413,19 +413,15 @@ public CopyWriter copy(final CopyRequest copyRequest) { final StorageObject source = copyRequest.source().toPb(); final Map sourceOptions = optionMap(copyRequest.source().generation(), null, copyRequest.sourceOptions(), true); - final BlobId targetId = copyRequest.targetId(); - final StorageObject targetObject = - copyRequest.targetInfo() != null ? copyRequest.targetInfo().toPb() : null; - final Map targetOptions = optionMap( - copyRequest.targetInfo() != null ? copyRequest.targetInfo().generation() : null, - copyRequest.targetInfo() != null ? copyRequest.targetInfo().metageneration() : null, - copyRequest.targetOptions()); + final StorageObject targetObject = copyRequest.target().toPb(); + final Map targetOptions = optionMap(copyRequest.target().generation(), + copyRequest.target().metageneration(), copyRequest.targetOptions()); try { RewriteResponse rewriteResponse = runWithRetries(new Callable() { @Override public RewriteResponse call() { return storageRpc.openRewrite(new StorageRpc.RewriteRequest(source, sourceOptions, - targetId.bucket(), targetId.name(), targetObject, targetOptions, + copyRequest.overrideInfo(), targetObject, targetOptions, copyRequest.megabytesCopiedPerChunk())); } }, options().retryParams(), EXCEPTION_HANDLER); diff --git a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/spi/DefaultStorageRpc.java b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/spi/DefaultStorageRpc.java index 7f13212556aa..8d06832534e2 100644 --- a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/spi/DefaultStorageRpc.java +++ b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/spi/DefaultStorageRpc.java @@ -580,8 +580,8 @@ private RewriteResponse rewrite(RewriteRequest req, String token) { Long maxBytesRewrittenPerCall = req.megabytesRewrittenPerCall != null ? req.megabytesRewrittenPerCall * MEGABYTE : null; com.google.api.services.storage.model.RewriteResponse rewriteResponse = storage.objects() - .rewrite(req.source.getBucket(), req.source.getName(), req.targetBucket, - req.targetName, req.targetObject) + .rewrite(req.source.getBucket(), req.source.getName(), req.target.getBucket(), + req.target.getName(), req.overrideInfo ? req.target : null) .setSourceGeneration(req.source.getGeneration()) .setRewriteToken(token) .setMaxBytesRewrittenPerCall(maxBytesRewrittenPerCall) diff --git a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/spi/StorageRpc.java b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/spi/StorageRpc.java index 7256368e189f..74f8171de87f 100644 --- a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/spi/StorageRpc.java +++ b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/spi/StorageRpc.java @@ -138,20 +138,18 @@ class RewriteRequest { public final StorageObject source; public final Map sourceOptions; - public final String targetBucket; - public final String targetName; - public final StorageObject targetObject; + public final boolean overrideInfo; + public final StorageObject target; public final Map targetOptions; public final Long megabytesRewrittenPerCall; public RewriteRequest(StorageObject source, Map sourceOptions, - String targetBucket, String targetName, StorageObject targetObject, - Map targetOptions, Long megabytesRewrittenPerCall) { + boolean overrideInfo, StorageObject target, Map targetOptions, + Long megabytesRewrittenPerCall) { this.source = source; this.sourceOptions = sourceOptions; - this.targetBucket = targetBucket; - this.targetName = targetName; - this.targetObject = targetObject; + this.overrideInfo = overrideInfo; + this.target = target; this.targetOptions = targetOptions; this.megabytesRewrittenPerCall = megabytesRewrittenPerCall; } @@ -167,17 +165,16 @@ public boolean equals(Object obj) { final RewriteRequest other = (RewriteRequest) obj; return Objects.equals(this.source, other.source) && Objects.equals(this.sourceOptions, other.sourceOptions) - && Objects.equals(this.targetBucket, other.targetBucket) - && Objects.equals(this.targetName, other.targetName) - && Objects.equals(this.targetObject, other.targetObject) + && Objects.equals(this.overrideInfo, other.overrideInfo) + && Objects.equals(this.target, other.target) && Objects.equals(this.targetOptions, other.targetOptions) && Objects.equals(this.megabytesRewrittenPerCall, other.megabytesRewrittenPerCall); } @Override public int hashCode() { - return Objects.hash(source, sourceOptions, targetBucket, targetName, targetObject, - targetOptions, megabytesRewrittenPerCall); + return Objects.hash(source, sourceOptions, overrideInfo, target, targetOptions, + megabytesRewrittenPerCall); } } diff --git a/gcloud-java-storage/src/test/java/com/google/gcloud/storage/BlobTest.java b/gcloud-java-storage/src/test/java/com/google/gcloud/storage/BlobTest.java index 2d4920816c38..d6c97ca9ca03 100644 --- a/gcloud-java-storage/src/test/java/com/google/gcloud/storage/BlobTest.java +++ b/gcloud-java-storage/src/test/java/com/google/gcloud/storage/BlobTest.java @@ -222,6 +222,7 @@ public void testDelete() throws Exception { @Test public void testCopyToBucket() throws Exception { initializeExpectedBlob(2); + BlobInfo target = BlobInfo.builder(BlobId.of("bt", "n")).build(); CopyWriter copyWriter = createMock(CopyWriter.class); Capture capturedCopyRequest = Capture.newInstance(); expect(storage.options()).andReturn(mockOptions); @@ -231,8 +232,8 @@ public void testCopyToBucket() throws Exception { CopyWriter returnedCopyWriter = blob.copyTo("bt"); assertEquals(copyWriter, returnedCopyWriter); assertEquals(capturedCopyRequest.getValue().source(), blob.blobId()); - assertEquals(capturedCopyRequest.getValue().targetId(), BlobId.of("bt", "n")); - assertNull(capturedCopyRequest.getValue().targetInfo()); + assertEquals(capturedCopyRequest.getValue().target(), target); + assertFalse(capturedCopyRequest.getValue().overrideInfo()); assertTrue(capturedCopyRequest.getValue().sourceOptions().isEmpty()); assertTrue(capturedCopyRequest.getValue().targetOptions().isEmpty()); } @@ -240,6 +241,7 @@ public void testCopyToBucket() throws Exception { @Test public void testCopyTo() throws Exception { initializeExpectedBlob(2); + BlobInfo target = BlobInfo.builder(BlobId.of("bt", "nt")).build(); CopyWriter copyWriter = createMock(CopyWriter.class); Capture capturedCopyRequest = Capture.newInstance(); expect(storage.options()).andReturn(mockOptions); @@ -249,8 +251,8 @@ public void testCopyTo() throws Exception { CopyWriter returnedCopyWriter = blob.copyTo("bt", "nt"); assertEquals(copyWriter, returnedCopyWriter); assertEquals(capturedCopyRequest.getValue().source(), blob.blobId()); - assertEquals(capturedCopyRequest.getValue().targetId(), BlobId.of("bt", "nt")); - assertNull(capturedCopyRequest.getValue().targetInfo()); + assertEquals(capturedCopyRequest.getValue().target(), target); + assertFalse(capturedCopyRequest.getValue().overrideInfo()); assertTrue(capturedCopyRequest.getValue().sourceOptions().isEmpty()); assertTrue(capturedCopyRequest.getValue().targetOptions().isEmpty()); } @@ -258,6 +260,7 @@ public void testCopyTo() throws Exception { @Test public void testCopyToBlobId() throws Exception { initializeExpectedBlob(2); + BlobInfo target = BlobInfo.builder(BlobId.of("bt", "nt")).build(); BlobId targetId = BlobId.of("bt", "nt"); CopyWriter copyWriter = createMock(CopyWriter.class); Capture capturedCopyRequest = Capture.newInstance(); @@ -268,8 +271,8 @@ public void testCopyToBlobId() throws Exception { CopyWriter returnedCopyWriter = blob.copyTo(targetId); assertEquals(copyWriter, returnedCopyWriter); assertEquals(capturedCopyRequest.getValue().source(), blob.blobId()); - assertEquals(capturedCopyRequest.getValue().targetId(), targetId); - assertNull(capturedCopyRequest.getValue().targetInfo()); + assertEquals(capturedCopyRequest.getValue().target(), target); + assertFalse(capturedCopyRequest.getValue().overrideInfo()); assertTrue(capturedCopyRequest.getValue().sourceOptions().isEmpty()); assertTrue(capturedCopyRequest.getValue().targetOptions().isEmpty()); } diff --git a/gcloud-java-storage/src/test/java/com/google/gcloud/storage/CopyRequestTest.java b/gcloud-java-storage/src/test/java/com/google/gcloud/storage/CopyRequestTest.java index 5700ea804cd6..9f8edfb84162 100644 --- a/gcloud-java-storage/src/test/java/com/google/gcloud/storage/CopyRequestTest.java +++ b/gcloud-java-storage/src/test/java/com/google/gcloud/storage/CopyRequestTest.java @@ -18,7 +18,8 @@ import static com.google.gcloud.storage.Storage.PredefinedAcl.PUBLIC_READ; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; import com.google.common.collect.ImmutableList; import com.google.gcloud.storage.Storage.BlobSourceOption; @@ -53,8 +54,8 @@ public void testCopyRequest() { assertEquals(SOURCE_BLOB_ID, copyRequest1.source()); assertEquals(1, copyRequest1.sourceOptions().size()); assertEquals(BlobSourceOption.generationMatch(1), copyRequest1.sourceOptions().get(0)); - assertEquals(TARGET_BLOB_INFO.blobId(), copyRequest1.targetId()); - assertEquals(TARGET_BLOB_INFO, copyRequest1.targetInfo()); + assertEquals(TARGET_BLOB_INFO, copyRequest1.target()); + assertTrue(copyRequest1.overrideInfo()); assertEquals(1, copyRequest1.targetOptions().size()); assertEquals(BlobTargetOption.predefinedAcl(PUBLIC_READ), copyRequest1.targetOptions().get(0)); @@ -63,16 +64,16 @@ public void testCopyRequest() { .target(TARGET_BLOB_ID) .build(); assertEquals(SOURCE_BLOB_ID, copyRequest2.source()); - assertEquals(TARGET_BLOB_ID, copyRequest2.targetId()); - assertNull(copyRequest2.targetInfo()); + assertEquals(BlobInfo.builder(TARGET_BLOB_ID).build(), copyRequest2.target()); + assertFalse(copyRequest2.overrideInfo()); Storage.CopyRequest copyRequest3 = Storage.CopyRequest.builder() .source(SOURCE_BLOB_ID) .target(TARGET_BLOB_INFO, ImmutableList.of(BlobTargetOption.predefinedAcl(PUBLIC_READ))) .build(); assertEquals(SOURCE_BLOB_ID, copyRequest3.source()); - assertEquals(TARGET_BLOB_INFO.blobId(), copyRequest3.targetId()); - assertEquals(TARGET_BLOB_INFO, copyRequest3.targetInfo()); + assertEquals(TARGET_BLOB_INFO, copyRequest3.target()); + assertTrue(copyRequest3.overrideInfo()); assertEquals(ImmutableList.of(BlobTargetOption.predefinedAcl(PUBLIC_READ)), copyRequest3.targetOptions()); } @@ -81,34 +82,37 @@ public void testCopyRequest() { public void testCopyRequestOf() { Storage.CopyRequest copyRequest1 = Storage.CopyRequest.of(SOURCE_BLOB_ID, TARGET_BLOB_INFO); assertEquals(SOURCE_BLOB_ID, copyRequest1.source()); - assertEquals(TARGET_BLOB_INFO.blobId(), copyRequest1.targetId()); - assertEquals(TARGET_BLOB_INFO, copyRequest1.targetInfo()); + assertEquals(TARGET_BLOB_INFO, copyRequest1.target()); + assertTrue(copyRequest1.overrideInfo()); Storage.CopyRequest copyRequest2 = Storage.CopyRequest.of(SOURCE_BLOB_ID, TARGET_BLOB_NAME); assertEquals(SOURCE_BLOB_ID, copyRequest2.source()); - assertEquals(BlobId.of(SOURCE_BUCKET_NAME, TARGET_BLOB_NAME), copyRequest2.targetId()); - assertNull(copyRequest2.targetInfo()); + assertEquals(BlobInfo.builder(BlobId.of(SOURCE_BUCKET_NAME, TARGET_BLOB_NAME)).build(), + copyRequest2.target()); + assertFalse(copyRequest2.overrideInfo()); Storage.CopyRequest copyRequest3 = Storage.CopyRequest.of(SOURCE_BUCKET_NAME, SOURCE_BLOB_NAME, TARGET_BLOB_INFO); assertEquals(SOURCE_BLOB_ID, copyRequest3.source()); - assertEquals(TARGET_BLOB_INFO.blobId(), copyRequest3.targetId()); - assertEquals(TARGET_BLOB_INFO, copyRequest3.targetInfo()); + assertEquals(TARGET_BLOB_INFO, copyRequest3.target()); + assertTrue(copyRequest3.overrideInfo()); Storage.CopyRequest copyRequest4 = Storage.CopyRequest.of(SOURCE_BUCKET_NAME, SOURCE_BLOB_NAME, TARGET_BLOB_NAME); assertEquals(SOURCE_BLOB_ID, copyRequest4.source()); - assertEquals(BlobId.of(SOURCE_BUCKET_NAME, TARGET_BLOB_NAME), copyRequest4.targetId()); - assertNull(copyRequest4.targetInfo()); + assertEquals(BlobInfo.builder(BlobId.of(SOURCE_BUCKET_NAME, TARGET_BLOB_NAME)).build(), + copyRequest4.target()); + assertFalse(copyRequest4.overrideInfo()); Storage.CopyRequest copyRequest5 = Storage.CopyRequest.of(SOURCE_BLOB_ID, TARGET_BLOB_ID); assertEquals(SOURCE_BLOB_ID, copyRequest5.source()); - assertEquals(TARGET_BLOB_ID, copyRequest5.targetId()); - assertNull(copyRequest5.targetInfo()); + assertEquals(BlobInfo.builder(TARGET_BLOB_ID).build(), copyRequest5.target()); + assertFalse(copyRequest5.overrideInfo()); Storage.CopyRequest copyRequest6 = Storage.CopyRequest.of(SOURCE_BUCKET_NAME, SOURCE_BLOB_NAME, TARGET_BLOB_ID); assertEquals(SOURCE_BLOB_ID, copyRequest6.source()); - assertEquals(TARGET_BLOB_ID, copyRequest6.targetId()); - assertNull(copyRequest6.targetInfo()); } + assertEquals(BlobInfo.builder(TARGET_BLOB_ID).build(), copyRequest6.target()); + assertFalse(copyRequest6.overrideInfo()); + } } diff --git a/gcloud-java-storage/src/test/java/com/google/gcloud/storage/CopyWriterTest.java b/gcloud-java-storage/src/test/java/com/google/gcloud/storage/CopyWriterTest.java index 75b863075770..8ccb81688b65 100644 --- a/gcloud-java-storage/src/test/java/com/google/gcloud/storage/CopyWriterTest.java +++ b/gcloud-java-storage/src/test/java/com/google/gcloud/storage/CopyWriterTest.java @@ -52,11 +52,11 @@ public class CopyWriterTest { BlobInfo.builder(DESTINATION_BUCKET_NAME, DESTINATION_BLOB_NAME).contentType("type").build(); private static final Map EMPTY_OPTIONS = ImmutableMap.of(); private static final RewriteRequest REQUEST_WITH_OBJECT = - new StorageRpc.RewriteRequest(BLOB_ID.toPb(), EMPTY_OPTIONS, DESTINATION_BUCKET_NAME, - DESTINATION_BLOB_NAME, BLOB_INFO.toPb(), EMPTY_OPTIONS, null); + new StorageRpc.RewriteRequest(BLOB_ID.toPb(), EMPTY_OPTIONS, true, BLOB_INFO.toPb(), + EMPTY_OPTIONS, null); private static final RewriteRequest REQUEST_WITHOUT_OBJECT = - new StorageRpc.RewriteRequest(BLOB_ID.toPb(), EMPTY_OPTIONS, DESTINATION_BUCKET_NAME, - DESTINATION_BLOB_NAME, null, EMPTY_OPTIONS, null); + new StorageRpc.RewriteRequest(BLOB_ID.toPb(), EMPTY_OPTIONS, false, BLOB_INFO.toPb(), + EMPTY_OPTIONS, null); private static final RewriteResponse RESPONSE_WITH_OBJECT = new RewriteResponse( REQUEST_WITH_OBJECT, null, 42L, false, "token", 21L); private static final RewriteResponse RESPONSE_WITHOUT_OBJECT = new RewriteResponse( diff --git a/gcloud-java-storage/src/test/java/com/google/gcloud/storage/StorageImplTest.java b/gcloud-java-storage/src/test/java/com/google/gcloud/storage/StorageImplTest.java index db1166598237..3cc99e3bf884 100644 --- a/gcloud-java-storage/src/test/java/com/google/gcloud/storage/StorageImplTest.java +++ b/gcloud-java-storage/src/test/java/com/google/gcloud/storage/StorageImplTest.java @@ -866,8 +866,7 @@ public void testComposeWithOptions() { public void testCopy() { CopyRequest request = Storage.CopyRequest.of(BLOB_INFO1.blobId(), BLOB_INFO2.blobId()); StorageRpc.RewriteRequest rpcRequest = new StorageRpc.RewriteRequest(request.source().toPb(), - EMPTY_RPC_OPTIONS, BLOB_INFO2.blobId().bucket(), BLOB_INFO2.blobId().name(), null, - EMPTY_RPC_OPTIONS, null); + EMPTY_RPC_OPTIONS, false, BLOB_INFO2.toPb(), EMPTY_RPC_OPTIONS, null); StorageRpc.RewriteResponse rpcResponse = new StorageRpc.RewriteResponse(rpcRequest, null, 42L, false, "token", 21L); EasyMock.expect(storageRpcMock.openRewrite(rpcRequest)).andReturn(rpcResponse); @@ -887,8 +886,7 @@ public void testCopyWithOptions() { .target(BLOB_INFO1, BLOB_TARGET_GENERATION, BLOB_TARGET_METAGENERATION) .build(); StorageRpc.RewriteRequest rpcRequest = new StorageRpc.RewriteRequest(request.source().toPb(), - BLOB_SOURCE_OPTIONS_COPY, BLOB_INFO1.blobId().bucket(), BLOB_INFO1.blobId().name(), - request.targetInfo().toPb(), BLOB_TARGET_OPTIONS_COMPOSE, null); + BLOB_SOURCE_OPTIONS_COPY, true, request.target().toPb(), BLOB_TARGET_OPTIONS_COMPOSE, null); StorageRpc.RewriteResponse rpcResponse = new StorageRpc.RewriteResponse(rpcRequest, null, 42L, false, "token", 21L); EasyMock.expect(storageRpcMock.openRewrite(rpcRequest)).andReturn(rpcResponse); @@ -908,8 +906,7 @@ public void testCopyWithOptionsFromBlobId() { .target(BLOB_INFO1, BLOB_TARGET_GENERATION, BLOB_TARGET_METAGENERATION) .build(); StorageRpc.RewriteRequest rpcRequest = new StorageRpc.RewriteRequest(request.source().toPb(), - BLOB_SOURCE_OPTIONS_COPY, BLOB_INFO1.blobId().bucket(), BLOB_INFO1.blobId().name(), - request.targetInfo().toPb(), BLOB_TARGET_OPTIONS_COMPOSE, null); + BLOB_SOURCE_OPTIONS_COPY, true, request.target().toPb(), BLOB_TARGET_OPTIONS_COMPOSE, null); StorageRpc.RewriteResponse rpcResponse = new StorageRpc.RewriteResponse(rpcRequest, null, 42L, false, "token", 21L); EasyMock.expect(storageRpcMock.openRewrite(rpcRequest)).andReturn(rpcResponse); @@ -925,8 +922,7 @@ public void testCopyWithOptionsFromBlobId() { public void testCopyMultipleRequests() { CopyRequest request = Storage.CopyRequest.of(BLOB_INFO1.blobId(), BLOB_INFO2.blobId()); StorageRpc.RewriteRequest rpcRequest = new StorageRpc.RewriteRequest(request.source().toPb(), - EMPTY_RPC_OPTIONS, BLOB_INFO2.blobId().bucket(), BLOB_INFO2.blobId().name(), null, - EMPTY_RPC_OPTIONS, null); + EMPTY_RPC_OPTIONS, false, BLOB_INFO2.toPb(), EMPTY_RPC_OPTIONS, null); StorageRpc.RewriteResponse rpcResponse1 = new StorageRpc.RewriteResponse(rpcRequest, null, 42L, false, "token", 21L); StorageRpc.RewriteResponse rpcResponse2 = new StorageRpc.RewriteResponse(rpcRequest, From 1855ae1a86370e90bc5fae34dff58bf39cb27671 Mon Sep 17 00:00:00 2001 From: Marco Ziccardi Date: Sun, 20 Mar 2016 21:05:49 +0100 Subject: [PATCH 5/6] Add better javadoc to Storage.copy and CopyWriter --- .../com/google/gcloud/storage/CopyWriter.java | 8 +++++++- .../java/com/google/gcloud/storage/Storage.java | 17 +++++++++++------ 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/CopyWriter.java b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/CopyWriter.java index 26c728942a28..2f3850d5dd03 100644 --- a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/CopyWriter.java +++ b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/CopyWriter.java @@ -32,7 +32,13 @@ import java.util.concurrent.Callable; /** - * Google Storage blob copy writer. This class holds the result of a copy request. If source and + * Google Storage blob copy writer. A {@code CopyWriter} object allows to copy both blob's data and + * information. To override source blob's information call {@link Storage#copy(Storage.CopyRequest)} + * with a {@code CopyRequest} object where the copy target is set via + * {@link Storage.CopyRequest.Builder#target(BlobInfo, Storage.BlobTargetOption...)} or + * {@link Storage.CopyRequest.Builder#target(BlobInfo, Iterable)}. + * + *

This class holds the result of a copy request. If source and * destination blobs share the same location and storage class the copy is completed in one RPC call * otherwise one or more {@link #copyChunk} calls are necessary to complete the copy. In addition, * {@link CopyWriter#result()} can be used to automatically complete the copy and return information diff --git a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/Storage.java b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/Storage.java index b30d46431de0..33c1d7e7e143 100644 --- a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/Storage.java +++ b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/Storage.java @@ -1385,12 +1385,17 @@ public static Builder builder() { Blob compose(ComposeRequest composeRequest); /** - * Sends a copy request. Returns a {@link CopyWriter} object for the provided - * {@code CopyRequest}. If source and destination objects share the same location and storage - * class the source blob is copied with one request and {@link CopyWriter#result()} immediately - * returns, regardless of the {@link CopyRequest#megabytesCopiedPerChunk} parameter. - * If source and destination have different location or storage class {@link CopyWriter#result()} - * might issue multiple RPC calls depending on blob's size. + * Sends a copy request. This method copies both blob's data and information. To override source + * blob's information set the copy target via + * {@link CopyRequest.Builder#target(BlobInfo, BlobTargetOption...)} or + * {@link CopyRequest.Builder#target(BlobInfo, Iterable)}. + * + *

This method returns a {@link CopyWriter} object for the provided {@code CopyRequest}. If + * source and destination objects share the same location and storage class the source blob is + * copied with one request and {@link CopyWriter#result()} immediately returns, regardless of the + * {@link CopyRequest#megabytesCopiedPerChunk} parameter. If source and destination have different + * location or storage class {@link CopyWriter#result()} might issue multiple RPC calls depending + * on blob's size. * *

Example usage of copy: * 

 {@code BlobInfo blob = service.copy(copyRequest).result();}

From 5dd0292085c00ac79aed381d4192bc33b09e2b97 Mon Sep 17 00:00:00 2001
From: Marco Ziccardi 
Date: Sun, 20 Mar 2016 22:39:16 +0100
Subject: [PATCH 6/6] Rephrase RewriteChannel and Storage.copy javadoc. Add
 final to CopyWriter's StateImpl.target

---
 .../main/java/com/google/gcloud/storage/CopyWriter.java    | 6 +++---
 .../src/main/java/com/google/gcloud/storage/Storage.java   | 7 ++++---
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/CopyWriter.java b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/CopyWriter.java
index 2f3850d5dd03..743630b6c4c2 100644
--- a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/CopyWriter.java
+++ b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/CopyWriter.java
@@ -33,8 +33,8 @@
 
 /**
  * Google Storage blob copy writer. A {@code CopyWriter} object allows to copy both blob's data and
- * information. To override source blob's information call {@link Storage#copy(Storage.CopyRequest)}
- * with a {@code CopyRequest} object where the copy target is set via
+ * information. To override source blob's information supply a {@code BlobInfo} to the
+ * {@code CopyRequest} using either
  * {@link Storage.CopyRequest.Builder#target(BlobInfo, Storage.BlobTargetOption...)} or
  * {@link Storage.CopyRequest.Builder#target(BlobInfo, Iterable)}.
  *
@@ -176,7 +176,7 @@ static class Builder {
       private final BlobId source;
       private final Map sourceOptions;
       private final boolean overrideInfo;
-      private BlobInfo target;
+      private final BlobInfo target;
       private final Map targetOptions;
       private BlobInfo result;
       private long blobSize;
diff --git a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/Storage.java b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/Storage.java
index 33c1d7e7e143..b4fbe45244b0 100644
--- a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/Storage.java
+++ b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/Storage.java
@@ -1386,9 +1386,10 @@ public static Builder builder() {
 
   /**
    * Sends a copy request. This method copies both blob's data and information. To override source
-   * blob's information set the copy target via
-   * {@link CopyRequest.Builder#target(BlobInfo, BlobTargetOption...)} or
-   * {@link CopyRequest.Builder#target(BlobInfo, Iterable)}.
+   * blob's information supply a {@code BlobInfo} to the
+   * {@code CopyRequest} using either
+   * {@link Storage.CopyRequest.Builder#target(BlobInfo, Storage.BlobTargetOption...)} or
+   * {@link Storage.CopyRequest.Builder#target(BlobInfo, Iterable)}.
    *
    * 
This method returns a {@link CopyWriter} object for the provided {@code CopyRequest}. If
    * source and destination objects share the same location and storage class the source blob is