fix: update grpc upload logic to follow hashing behavior of json (#2107)

googleapis · Jul 11, 2023 · ed05232 · ed05232
1 parent 8b17574
commit ed05232
Show file tree

Hide file tree

Showing 2 changed files with 6 additions and 24 deletions.
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
@@ -103,7 +103,6 @@
 import com.google.storage.v2.NotificationConfigName;
 import com.google.storage.v2.Object;
 import com.google.storage.v2.ObjectAccessControl;
-import com.google.storage.v2.ObjectChecksums;
 import com.google.storage.v2.ReadObjectRequest;
 import com.google.storage.v2.RewriteObjectRequest;
 import com.google.storage.v2.RewriteResponse;
@@ -233,7 +232,7 @@ public Blob create(
     GrpcCallContext grpcCallContext =
         opts.grpcMetadataMapper().apply(GrpcCallContext.createDefault());
     WriteObjectRequest req = getWriteObjectRequest(blobInfo, opts);
-    Hasher hasher = getHasherForRequest(req, Hasher.enabled());
+    Hasher hasher = Hasher.enabled();
     GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext());
     return Retrying.run(
         getOptions(),
@@ -286,8 +285,6 @@ public Blob createFrom(BlobInfo blobInfo, Path path, int bufferSize, BlobWriteOp
         opts.grpcMetadataMapper().apply(GrpcCallContext.createDefault());
     WriteObjectRequest req = getWriteObjectRequest(blobInfo, opts);
 
-    Hasher hasher = getHasherForRequest(req, Hasher.enabled());
-
     long size = Files.size(path);
     if (size < bufferSize) {
       // ignore the bufferSize argument if the file is smaller than it
@@ -300,7 +297,7 @@ public Blob createFrom(BlobInfo blobInfo, Path path, int bufferSize, BlobWriteOp
                 ResumableMedia.gapic()
                     .write()
                     .byteChannel(storageClient.writeObjectCallable().withDefaultCallContext(merge))
-                    .setHasher(hasher)
+                    .setHasher(Hasher.enabled())
                     .setByteStringStrategy(ByteStringStrategy.noCopy())
                     .direct()
                     .buffered(Buffers.allocate(size))
@@ -323,7 +320,7 @@ public Blob createFrom(BlobInfo blobInfo, Path path, int bufferSize, BlobWriteOp
               .write()
               .byteChannel(
                   storageClient.writeObjectCallable().withDefaultCallContext(grpcCallContext))
-              .setHasher(hasher)
+              .setHasher(Hasher.noop())
               .setByteStringStrategy(ByteStringStrategy.noCopy())
               .resumable()
               .withRetryConfig(getOptions(), retryAlgorithmManager.idempotent())
@@ -359,13 +356,12 @@ public Blob createFrom(
 
     ApiFuture<ResumableWrite> start = startResumableWrite(grpcCallContext, req);
 
-    Hasher hasher = getHasherForRequest(req, Hasher.enabled());
     BufferedWritableByteChannelSession<WriteObjectResponse> session =
         ResumableMedia.gapic()
             .write()
             .byteChannel(
                 storageClient.writeObjectCallable().withDefaultCallContext(grpcCallContext))
-            .setHasher(hasher)
+            .setHasher(Hasher.noop())
             .setByteStringStrategy(ByteStringStrategy.noCopy())
             .resumable()
             .withRetryConfig(getOptions(), retryAlgorithmManager.idempotent())
@@ -739,7 +735,7 @@ public GrpcBlobWriteChannel writer(BlobInfo blobInfo, BlobWriteOption... options
     GrpcCallContext grpcCallContext =
         opts.grpcMetadataMapper().apply(GrpcCallContext.createDefault());
     WriteObjectRequest req = getWriteObjectRequest(blobInfo, opts);
-    Hasher hasher = getHasherForRequest(req, Hasher.enabled());
+    Hasher hasher = Hasher.noop();
     return new GrpcBlobWriteChannel(
         storageClient.writeObjectCallable(),
         getOptions(),
@@ -1967,19 +1963,6 @@ private Object updateObject(UpdateObjectRequest req) {
         Decoder.identity());
   }
 
-  private static Hasher getHasherForRequest(WriteObjectRequest req, Hasher defaultHasher) {
-    if (!req.hasObjectChecksums()) {
-      return defaultHasher;
-    } else {
-      ObjectChecksums checksums = req.getObjectChecksums();
-      if (!checksums.hasCrc32C() && checksums.getMd5Hash().isEmpty()) {
-        return defaultHasher;
-      } else {
-        return Hasher.noop();
-      }
-    }
-  }
-
   @Nullable
   private Blob internalBlobGet(BlobId blob, Opts<ObjectSourceOpt> unwrap) {
     Opts<ObjectSourceOpt> opts = unwrap.resolveFrom(blob).prepend(defaultOpts);

diff --git a/...loud-storage/src/test/java/com/google/cloud/storage/ITGrpcStorageImplUploadRetryTest.java b/...loud-storage/src/test/java/com/google/cloud/storage/ITGrpcStorageImplUploadRetryTest.java
@@ -241,12 +241,11 @@ private static final class Resumable {
         StartResumableWriteResponse.newBuilder().setUploadId(uploadId).build();
 
     private static final ChecksummedData checksummedData =
-        TestUtils.getChecksummedData(ByteString.copyFrom(bytes), Hasher.enabled());
+        TestUtils.getChecksummedData(ByteString.copyFrom(bytes), Hasher.noop());
     private static final WriteObjectRequest req1 =
         WriteObjectRequest.newBuilder()
             .setUploadId(uploadId)
             .setChecksummedData(checksummedData)
-            .setObjectChecksums(ObjectChecksums.newBuilder().setCrc32C(checksummedData.getCrc32C()))
             .setFinishWrite(true)
             .build();
     private static final WriteObjectResponse resp1 =