Refactor CopyRequest: remove targetId, add boolean overrideInfo

gcloud-java-storage/src/main/java/com/google/gcloud/storage/CopyWriter.java

@@ -120,11 +120,9 @@ public RestorableState<CopyWriter> 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<CopyWriter>, Serializable {
     private final StorageOptions serviceOptions;
     private final BlobId source;
     private final Map<StorageRpc.Option, ?> sourceOptions;
-    private final BlobId targetId;
-    private final BlobInfo targetInfo;
+    private final boolean overrideInfo;
+    private final BlobInfo target;
     private final Map<StorageRpc.Option, ?> targetOptions;
     private final BlobInfo result;
     private final long blobSize;
@@ -155,8 +153,8 @@ static class StateImpl implements RestorableState<CopyWriter>, 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<StorageRpc.Option, ?> sourceOptions;
-      private final BlobId targetId;
+      private final boolean overrideInfo;
+      private BlobInfo target;
       private final Map<StorageRpc.Option, ?> 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<StorageRpc.Option, ?> sourceOptions,
-          BlobId targetId, Map<StorageRpc.Option, ?> targetOptions) {
+          Map<StorageRpc.Option, ?> sourceOptions, boolean overrideInfo, BlobInfo target,
+          Map<StorageRpc.Option, ?> 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<CopyWriter> build() {
     }
 
     static Builder builder(StorageOptions options, BlobId source,
-        Map<StorageRpc.Option, ?> sourceOptions, BlobId targetId,
+        Map<StorageRpc.Option, ?> sourceOptions, boolean overrideInfo, BlobInfo target,
         Map<StorageRpc.Option, ?> 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)

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<BlobSourceOption> sourceOptions;
-    private final BlobId targetId;
-    private final BlobInfo targetInfo;
+    private final boolean overrideInfo;
+    private final BlobInfo target;
     private final List<BlobTargetOption> targetOptions;
     private final Long megabytesCopiedPerChunk;
 
@@ -972,8 +972,8 @@ public static class Builder {
       private final Set<BlobSourceOption> sourceOptions = new LinkedHashSet<>();
       private final Set<BlobTargetOption> targetOptions = new LinkedHashSet<>();
       private BlobId source;
-      private BlobId targetId;
-      private BlobInfo targetInfo;
+      private boolean overrideInfo;
+      private BlobInfo target;
       private Long megabytesCopiedPerChunk;
 
       /**
@@ -1022,21 +1022,22 @@ public Builder sourceOptions(Iterable<BlobSourceOption> options) {
        * @return the builder
        */
       public Builder target(BlobId targetId) {
-        this.targetId = targetId;
+        this.overrideInfo = false;
+        this.target = BlobInfo.builder(targetId).build();
         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}). 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<BlobTargetOption> options) {
-        this.targetId = targetInfo.blobId();
-        this.targetInfo = targetInfo;
+      public Builder target(BlobInfo target, Iterable<BlobTargetOption> 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<BlobSourceOption> 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;
     }
 
     /**

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<StorageRpc.Option, ?> 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<StorageRpc.Option, ?> 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<StorageRpc.Option, ?> targetOptions = optionMap(copyRequest.target().generation(),
+        copyRequest.target().metageneration(), copyRequest.targetOptions());
     try {
       RewriteResponse rewriteResponse = runWithRetries(new Callable<RewriteResponse>() {
         @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);

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)

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<StorageRpc.Option, ?> sourceOptions;
-    public final String targetBucket;
-    public final String targetName;
-    public final StorageObject targetObject;
+    public final boolean overrideInfo;
+    public final StorageObject target;
     public final Map<StorageRpc.Option, ?> targetOptions;
     public final Long megabytesRewrittenPerCall;
 
     public RewriteRequest(StorageObject source, Map<StorageRpc.Option, ?> sourceOptions,
-        String targetBucket, String targetName, StorageObject targetObject,
-        Map<StorageRpc.Option, ?> targetOptions, Long megabytesRewrittenPerCall) {
+        boolean overrideInfo, StorageObject target, Map<StorageRpc.Option, ?> 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);
     }
   }
 

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<CopyRequest> capturedCopyRequest = Capture.newInstance();
     expect(storage.options()).andReturn(mockOptions);
@@ -231,15 +232,16 @@ 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());
   }
 
   @Test
   public void testCopyTo() throws Exception {
     initializeExpectedBlob(2);
+    BlobInfo target = BlobInfo.builder(BlobId.of("bt", "nt")).build();
     CopyWriter copyWriter = createMock(CopyWriter.class);
     Capture<CopyRequest> capturedCopyRequest = Capture.newInstance();
     expect(storage.options()).andReturn(mockOptions);
@@ -249,15 +251,16 @@ 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());
   }
 
   @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<CopyRequest> 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());
   }