chore: cleanup Crc32cValue and associated Hasher operations

google-cloud-storage/src/main/java/com/google/cloud/storage/Crc32cValue.java

@@ -16,6 +16,7 @@
 
 package com.google.cloud.storage;
 
+import java.nio.ByteBuffer;
 import java.util.Locale;
 import java.util.Objects;
 
@@ -56,6 +57,10 @@ public boolean eqValue(Crc32cValue<?> other) {
     return this.getValue() == other.getValue();
   }
 
+  static Crc32cLengthKnown zero() {
+    return Crc32cLengthKnown.ZERO;
+  }
+
   static Crc32cLengthUnknown of(int value) {
     return new Crc32cLengthUnknown(value);
   }
@@ -81,6 +86,9 @@ public int getValue() {
 
     @Override
     public Crc32cLengthUnknown concat(Crc32cLengthKnown other) {
+      if (other == Crc32cLengthKnown.ZERO) {
+        return this;
+      }
       int combined = Crc32cUtility.concatCrc32c(value, other.value, other.length);
       return new Crc32cLengthUnknown(combined);
     }
@@ -118,6 +126,7 @@ public int hashCode() {
   }
 
   static final class Crc32cLengthKnown extends Crc32cValue<Crc32cLengthKnown> {
+    private static final Crc32cLengthKnown ZERO = Hasher.enabled().hash(ByteBuffer.allocate(0));
     private final int value;
     private final long length;
 
@@ -137,6 +146,11 @@ public long getLength() {
 
     @Override
     public Crc32cLengthKnown concat(Crc32cLengthKnown other) {
+      if (other == ZERO) {
+        return this;
+      } else if (this == ZERO) {
+        return other;
+      }
       int combined = Crc32cUtility.concatCrc32c(value, other.value, other.length);
       return new Crc32cLengthKnown(combined, length + other.length);
     }

google-cloud-storage/src/main/java/com/google/cloud/storage/Hasher.java

@@ -27,10 +27,13 @@
 import java.util.List;
 import java.util.Locale;
 import java.util.function.Supplier;
+import javax.annotation.ParametersAreNonnullByDefault;
 import javax.annotation.concurrent.Immutable;
 import org.checkerframework.checker.nullness.qual.NonNull;
 import org.checkerframework.checker.nullness.qual.Nullable;
 
+@SuppressWarnings("ClassEscapesDefinedScope")
+@ParametersAreNonnullByDefault
 interface Hasher {
 
   @Nullable
@@ -49,13 +52,14 @@ default Crc32cLengthKnown hash(Supplier<ByteBuffer> b) {
   void validateUnchecked(Crc32cValue<?> expected, ByteString byteString)
       throws UncheckedChecksumMismatchException;
 
-  @Nullable Crc32cLengthKnown nullSafeConcat(Crc32cLengthKnown r1, Crc32cLengthKnown r2);
+  @Nullable Crc32cLengthKnown nullSafeConcat(
+      @Nullable Crc32cLengthKnown r1, @NonNull Crc32cLengthKnown r2);
 
-  static Hasher noop() {
+  static NoOpHasher noop() {
     return NoOpHasher.INSTANCE;
   }
 
-  static Hasher enabled() {
+  static GuavaHasher enabled() {
     return GuavaHasher.INSTANCE;
   }
 
@@ -85,7 +89,8 @@ public void validate(Crc32cValue<?> expected, ByteString b) {}
     public void validateUnchecked(Crc32cValue<?> expected, ByteString byteString) {}
 
     @Override
-    public @Nullable Crc32cLengthKnown nullSafeConcat(Crc32cLengthKnown r1, Crc32cLengthKnown r2) {
+    public @Nullable Crc32cLengthKnown nullSafeConcat(
+        @Nullable Crc32cLengthKnown r1, @NonNull Crc32cLengthKnown r2) {
       return null;
     }
   }
@@ -107,7 +112,7 @@ private GuavaHasher() {}
       return Crc32cValue.of(Hashing.crc32c().hashBytes(b).asInt(), remaining);
     }
 
-    @SuppressWarnings({"ConstantConditions", "UnstableApiUsage"})
+    @SuppressWarnings({"UnstableApiUsage"})
     @Override
     public @NonNull Crc32cLengthKnown hash(ByteString byteString) {
       List<ByteBuffer> buffers = byteString.asReadOnlyByteBufferList();
@@ -118,7 +123,6 @@ private GuavaHasher() {}
       return Crc32cValue.of(crc32c.hash().asInt(), byteString.size());
     }
 
-    @SuppressWarnings({"ConstantConditions"})
     @Override
     public void validate(Crc32cValue<?> expected, ByteString byteString)
         throws ChecksumMismatchException {
@@ -137,7 +141,6 @@ public void validate(Crc32cValue<?> expected, Supplier<ByteBuffer> b)
       }
     }
 
-    @SuppressWarnings({"ConstantConditions"})
     @Override
     public void validateUnchecked(Crc32cValue<?> expected, ByteString byteString)
         throws UncheckedChecksumMismatchException {
@@ -149,9 +152,10 @@ public void validateUnchecked(Crc32cValue<?> expected, ByteString byteString)
 
     @Override
     @Nullable
-    public Crc32cLengthKnown nullSafeConcat(Crc32cLengthKnown r1, Crc32cLengthKnown r2) {
+    public Crc32cLengthKnown nullSafeConcat(
+        @Nullable Crc32cLengthKnown r1, @NonNull Crc32cLengthKnown r2) {
       if (r1 == null) {
-        return r2;
+        return null;
       } else {
         return r1.concat(r2);
       }

google-cloud-storage/src/main/java/com/google/cloud/storage/WriteCtx.java

@@ -31,13 +31,22 @@ final class WriteCtx<RequestFactoryT extends WriteObjectRequestBuilderFactory> {
 
   private final AtomicLong totalSentBytes;
   private final AtomicLong confirmedBytes;
-  private final AtomicReference<Crc32cLengthKnown> cumulativeCrc32c;
+  private final AtomicReference<@Nullable Crc32cLengthKnown> cumulativeCrc32c;
 
   WriteCtx(RequestFactoryT requestFactory) {
+    this(requestFactory, null);
+  }
+
+  /**
+   * TODO: Remove initialValue and replace with Crc32cValue.zero() once all uploads have been
+   * updated to do e2e checksumming by default.
+   */
+  @Deprecated
+  WriteCtx(RequestFactoryT requestFactory, @Nullable Crc32cLengthKnown initialValue) {
     this.requestFactory = requestFactory;
     this.totalSentBytes = new AtomicLong(0);
     this.confirmedBytes = new AtomicLong(0);
-    this.cumulativeCrc32c = new AtomicReference<>();
+    this.cumulativeCrc32c = new AtomicReference<>(initialValue);
   }
 
   public RequestFactoryT getRequestFactory() {
@@ -56,7 +65,7 @@ public AtomicLong getConfirmedBytes() {
     return confirmedBytes;
   }
 
-  public AtomicReference<Crc32cLengthKnown> getCumulativeCrc32c() {
+  public AtomicReference<@Nullable Crc32cLengthKnown> getCumulativeCrc32c() {
     return cumulativeCrc32c;
   }
 

google-cloud-storage/src/test/java/com/google/cloud/storage/Crc32cValueTest.java

@@ -20,9 +20,11 @@
 
 import com.google.cloud.storage.Crc32cValue.Crc32cLengthKnown;
 import com.google.cloud.storage.Crc32cValue.Crc32cLengthUnknown;
+import com.google.cloud.storage.it.ChecksummedTestContent;
 import com.google.common.hash.HashFunction;
 import com.google.common.hash.Hashing;
 import net.jqwik.api.Example;
+import org.checkerframework.checker.nullness.qual.NonNull;
 
 final class Crc32cValueTest {
 
@@ -67,4 +69,30 @@ public void ensureConcatSatisfiesTheLeftDistributedProperty() {
     assertThat(nesting.getValue()).isEqualTo(expected);
     assertThat(mixed.getValue()).isEqualTo(expected);
   }
+
+  @Example
+  void zeroDoesNotTransform() {
+    Crc32cLengthKnown base =
+        Hasher.enabled().hash(DataGenerator.base64Characters().genByteBuffer(64));
+
+    assertThat(base.concat(Crc32cValue.zero())).isSameInstanceAs(base);
+    assertThat(Crc32cValue.zero().concat(base)).isSameInstanceAs(base);
+  }
+
+  @Example
+  void nullSafeConcat_isAlwaysNull() {
+    ChecksummedTestContent testContent =
+        ChecksummedTestContent.of(DataGenerator.base64Characters().genBytes(2 * 1024 * 1024));
+
+    Crc32cLengthKnown actual =
+        testContent.chunkup(373).stream()
+            .map(Crc32cValueTest::toCrc32cValue)
+            .reduce(null, Hasher.enabled()::nullSafeConcat);
+
+    assertThat(actual).isNull();
+  }
+
+  private static @NonNull Crc32cLengthKnown toCrc32cValue(ChecksummedTestContent testContent) {
+    return Crc32cValue.of(testContent.getCrc32c(), testContent.getBytes().length);
+  }
 }

google-cloud-storage/src/test/java/com/google/cloud/storage/ITGapicUnbufferedWritableByteChannelTest.java

@@ -159,7 +159,10 @@ public void directUpload() throws IOException, InterruptedException, ExecutionEx
       SettableApiFuture<WriteObjectResponse> result = SettableApiFuture.create();
       try (GapicUnbufferedDirectWritableByteChannel c =
           new GapicUnbufferedDirectWritableByteChannel(
-              result, segmenter, sc.writeObjectCallable(), new WriteCtx<>(reqFactory))) {
+              result,
+              segmenter,
+              sc.writeObjectCallable(),
+              new WriteCtx<>(reqFactory, Crc32cValue.zero()))) {
         c.write(ByteBuffer.wrap(bytes));
       }
       assertThat(result.get()).isEqualTo(resp);
@@ -185,7 +188,7 @@ public void resumableUpload() throws IOException, InterruptedException, Executio
               result,
               segmenter,
               sc.writeObjectCallable(),
-              new WriteCtx<>(reqFactory),
+              new WriteCtx<>(reqFactory, Crc32cValue.zero()),
               RetrierWithAlg.attemptOnce(),
               Retrying::newCallContext);
       ArrayList<String> debugMessages = new ArrayList<>();
@@ -267,7 +270,7 @@ public void resumableUpload_chunkAutomaticRetry()
               result,
               segmenter,
               sc.writeObjectCallable(),
-              new WriteCtx<>(reqFactory),
+              new WriteCtx<>(reqFactory, Crc32cValue.zero()),
               TestUtils.retrierFromStorageOptions(fake.getGrpcStorageOptions())
                   .withAlg(Retrying.alwaysRetry()),
               Retrying::newCallContext)) {
@@ -319,7 +322,7 @@ public void resumableUpload_finalizeWhenWriteAndCloseCalledEvenWhenQuantumAligne
               result,
               segmenter,
               sc.writeObjectCallable(),
-              new WriteCtx<>(reqFactory),
+              new WriteCtx<>(reqFactory, Crc32cValue.zero()),
               RetrierWithAlg.attemptOnce(),
               Retrying::newCallContext);
       try {

google-cloud-storage/src/test/java/com/google/cloud/storage/ParallelCompositeUploadWritableByteChannelTest.java

@@ -892,7 +892,8 @@ public BlobInfo compose(ComposeRequest composeRequest) {
               .map(Data::getCrc32c)
               .collect(ImmutableList.toImmutableList());
 
-      Crc32cLengthKnown reduce = crc32cs.stream().reduce(null, HASHER::nullSafeConcat);
+      Crc32cLengthKnown reduce =
+          crc32cs.stream().reduce(Crc32cValue.zero(), Crc32cLengthKnown::concat);
       Preconditions.checkState(reduce != null, "unable to compute crc32c for compose request");
       b.setCrc32c(Utils.crc32cCodec.encode(reduce.getValue()));
       BlobInfo gen1 = b.build();

google-cloud-storage/src/test/java/com/google/cloud/storage/it/ChecksummedTestContent.java

@@ -20,6 +20,7 @@
 import static com.google.common.base.Preconditions.checkPositionIndexes;
 
 import com.google.common.base.MoreObjects;
+import com.google.common.collect.ImmutableList;
 import com.google.common.hash.Hashing;
 import com.google.common.io.BaseEncoding;
 import com.google.common.primitives.Ints;
@@ -28,8 +29,10 @@
 import com.google.storage.v2.ChecksummedData;
 import java.io.ByteArrayInputStream;
 import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Base64;
+import java.util.List;
 
 public final class ChecksummedTestContent {
 
@@ -100,6 +103,14 @@ public ChecksummedTestContent slice(int begin, int length) {
     return of(bytes, begin, Math.min(length, bytes.length - begin));
   }
 
+  public List<ChecksummedTestContent> chunkup(int chunkSize) {
+    List<ChecksummedTestContent> elements = new ArrayList<>();
+    for (int i = 0; i < bytes.length; i += chunkSize) {
+      elements.add(slice(i, chunkSize));
+    }
+    return ImmutableList.copyOf(elements);
+  }
+
   @Override
   public String toString() {
     return MoreObjects.toStringHelper(this)