diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/GapicCopyWriter.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/GapicCopyWriter.java index 038ff4667..9d6a1f870 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/GapicCopyWriter.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/GapicCopyWriter.java @@ -31,6 +31,7 @@ final class GapicCopyWriter extends CopyWriter { private final GrpcStorageOptions options; private final UnaryCallable callable; private final ResultRetryAlgorithm alg; + private final RewriteObjectRequest originalRequest; private final RewriteResponse initialResponse; private RewriteResponse mostRecentResponse; @@ -39,6 +40,7 @@ final class GapicCopyWriter extends CopyWriter { GrpcStorageImpl storage, UnaryCallable callable, ResultRetryAlgorithm alg, + RewriteObjectRequest originalRequest, RewriteResponse initialResponse) { this.storage = storage; this.options = storage.getOptions(); @@ -46,6 +48,7 @@ final class GapicCopyWriter extends CopyWriter { this.alg = alg; this.initialResponse = initialResponse; this.mostRecentResponse = initialResponse; + this.originalRequest = originalRequest; } @Override @@ -76,9 +79,7 @@ public long getTotalBytesCopied() { public void copyChunk() { if (!isDone()) { RewriteObjectRequest req = - RewriteObjectRequest.newBuilder() - .setRewriteToken(mostRecentResponse.getRewriteToken()) - .build(); + originalRequest.toBuilder().setRewriteToken(mostRecentResponse.getRewriteToken()).build(); GrpcCallContext retryContext = Retrying.newCallContext(); mostRecentResponse = Retrying.run(options, alg, () -> callable.call(req, retryContext), Decoder.identity()); diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java index d7472f5ff..428f85bc7 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java @@ -656,17 +656,24 @@ public CopyWriter copy(CopyRequest copyRequest) { RewriteObjectRequest.newBuilder() .setDestinationName(dstProto.getName()) .setDestinationBucket(dstProto.getBucket()) - // destination_kms_key comes from dstOpts - // according to the docs in the protos, it is illegal to populate the following fields, - // clear them out if they are set - // destination_predefined_acl comes from dstOpts - // if_*_match come from srcOpts and dstOpts - // copy_source_encryption_* come from srcOpts - // common_object_request_params come from dstOpts - .setDestination(dstProto.toBuilder().clearName().clearBucket().clearKmsKey().build()) .setSourceBucket(srcProto.getBucket()) .setSourceObject(srcProto.getName()); + // according to the docs in the protos, it is illegal to populate the following fields, + // clear them out if they are set + // * destination_kms_key comes from dstOpts + // * destination_predefined_acl comes from dstOpts + // * if_*_match come from srcOpts and dstOpts + // * copy_source_encryption_* come from srcOpts + // * common_object_request_params come from dstOpts + Object cleanedDst = dstProto.toBuilder().clearName().clearBucket().clearKmsKey().build(); + // only set the destination if it is not equal to the default instance + // otherwise we will clobber default values populated in the gcs server side for the object + // metadata + if (!cleanedDst.equals(Object.getDefaultInstance())) { + b.setDestination(cleanedDst); + } + if (src.getGeneration() != null) { b.setSourceGeneration(src.getGeneration()); } @@ -685,7 +692,8 @@ public CopyWriter copy(CopyRequest copyRequest) { getOptions(), retryAlgorithmManager.getFor(req), () -> callable.call(req, retryContext), - (resp) -> new GapicCopyWriter(this, callable, retryAlgorithmManager.idempotent(), resp)); + (resp) -> + new GapicCopyWriter(this, callable, retryAlgorithmManager.idempotent(), req, resp)); } @Override diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITObjectTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITObjectTest.java index 7a88da5e4..ed6811329 100644 --- a/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITObjectTest.java +++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITObjectTest.java @@ -39,6 +39,7 @@ import com.google.cloud.storage.Bucket; import com.google.cloud.storage.BucketInfo; import com.google.cloud.storage.CopyWriter; +import com.google.cloud.storage.DataGenerator; import com.google.cloud.storage.PackagePrivateMethodWorkarounds; import com.google.cloud.storage.Storage; import com.google.cloud.storage.Storage.BlobField; @@ -562,7 +563,6 @@ public void testListBlobsCurrentDirectoryIncludesBothObjectsAndSyntheticDirector } @Test - // When gRPC support is added for matchGlob, enable this test for gRPC. public void testListBlobsWithMatchGlob() throws Exception { BucketInfo bucketInfo = BucketInfo.newBuilder(generator.randomBucketName()).build(); try (TemporaryBucket tempBucket = @@ -848,8 +848,6 @@ public void testComposeBlobFail() { } @Test - // Bucket attribute extration on allowlist bug b/246634709 - @Exclude(transports = Transport.GRPC) public void testCopyBlob() { String sourceBlobName = generator.randomObjectName() + "-source"; @@ -872,8 +870,35 @@ public void testCopyBlob() { } @Test - // Bucket attribute extration on allowlist bug b/246634709 - @Exclude(transports = Transport.GRPC) + public void copyBlob_classChange_multipleChunks() { + + String sourceBlobName = generator.randomObjectName() + "-source"; + BlobId source = BlobId.of(bucket.getName(), sourceBlobName); + ImmutableMap metadata = ImmutableMap.of("k", "v"); + BlobInfo blob = BlobInfo.newBuilder(source).setMetadata(metadata).build(); + int _5MiB = 5 * 1024 * 1024; + byte[] bytes = DataGenerator.base64Characters().genBytes(_5MiB); + Blob remoteBlob = storage.create(blob, bytes); + assertThat(remoteBlob).isNotNull(); + String targetBlobName = generator.randomObjectName() + "-target"; + CopyRequest req = + CopyRequest.newBuilder() + .setSource(source) + .setTarget( + BlobInfo.newBuilder(bucket, targetBlobName) + // change the storage class to force GCS to copy bytes + .setStorageClass(StorageClass.NEARLINE) + .build(), + BlobTargetOption.doesNotExist()) + .setMegabytesCopiedPerChunk(2L) + .build(); + CopyWriter copyWriter = storage.copy(req); + BlobInfo remoteBlob2 = copyWriter.getResult(); + assertThat(copyWriter.isDone()).isTrue(); + assertThat(remoteBlob2).isNotNull(); + } + + @Test public void testCopyBlobWithPredefinedAcl() { String sourceBlobName = generator.randomObjectName() + "-source"; @@ -903,8 +928,6 @@ public void testCopyBlobWithPredefinedAcl() { } @Test - // Bucket attribute extration on allowlist bug b/246634709 - @Exclude(transports = Transport.GRPC) public void testCopyBlobWithEncryptionKeys() { String sourceBlobName = generator.randomObjectName() + "-source"; @@ -955,8 +978,6 @@ public void testCopyBlobWithEncryptionKeys() { } @Test - // Bucket attribute extration on allowlist bug b/246634709 - @Exclude(transports = Transport.GRPC) public void testCopyBlobUpdateMetadata() { String sourceBlobName = generator.randomObjectName() + "-source"; @@ -981,9 +1002,7 @@ public void testCopyBlobUpdateMetadata() { assertTrue(storage.delete(bucket.getName(), targetBlobName)); } - // Re-enable this test when it stops failing - // @Test - @Exclude(transports = Transport.GRPC) + @Test public void testCopyBlobUpdateStorageClass() { String sourceBlobName = generator.randomObjectName() + "-source"; BlobId source = BlobId.of(bucket.getName(), sourceBlobName); @@ -1007,8 +1026,6 @@ public void testCopyBlobUpdateStorageClass() { } @Test - // Bucket attribute extration on allowlist bug b/246634709 - @Exclude(transports = Transport.GRPC) public void testCopyBlobNoContentType() { String sourceBlobName = generator.randomObjectName() + "-source"; @@ -1022,7 +1039,9 @@ public void testCopyBlobNoContentType() { CopyWriter copyWriter = storage.copy(req); assertEquals(bucket.getName(), copyWriter.getResult().getBucket()); assertEquals(targetBlobName, copyWriter.getResult().getName()); - assertNull(copyWriter.getResult().getContentType()); + assertTrue( + copyWriter.getResult().getContentType() == null + || copyWriter.getResult().getContentType().isEmpty()); assertEquals(metadata, copyWriter.getResult().getMetadata()); assertTrue(copyWriter.isDone()); assertTrue(remoteSourceBlob.delete()); @@ -1030,9 +1049,6 @@ public void testCopyBlobNoContentType() { } @Test - // Verified against testbench - // Bucket attribute extration on allowlist bug b/246634709 - @Exclude(transports = Transport.GRPC) public void testCopyBlobFail() { String sourceBlobName = "test-copy-blob-source-fail";