From b20cee507794fa90997eec93a4c2f845a038a243 Mon Sep 17 00:00:00 2001 From: BenWhitehead Date: Thu, 7 Aug 2025 18:51:22 -0400 Subject: [PATCH] chore: update some retry/takeover tests and initialization after reviewing semantics --- .../google/cloud/storage/GrpcStorageImpl.java | 19 ++----- .../storage/ITAppendableUploadFakeTest.java | 51 ------------------- 2 files changed, 3 insertions(+), 67 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 index 7d17a0f87a..ce3e6c62d2 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 @@ -52,7 +52,6 @@ import com.google.cloud.storage.BlobWriteSessionConfig.WriterFactory; import com.google.cloud.storage.BufferedWritableByteChannelSession.BufferedWritableByteChannel; import com.google.cloud.storage.Conversions.Decoder; -import com.google.cloud.storage.Crc32cValue.Crc32cLengthKnown; import com.google.cloud.storage.GrpcUtils.ZeroCopyServerStreamingCallable; import com.google.cloud.storage.HmacKey.HmacKeyMetadata; import com.google.cloud.storage.HmacKey.HmacKeyState; @@ -1502,27 +1501,16 @@ public AppendableUploadState getAppendableState( BidiWriteObjectRequest req = takeOver ? getBidiWriteObjectRequestForTakeover(info, opts) - : getBidiWriteObjectRequest(info, opts, true); + : getBidiWriteObjectRequest(info, opts, /* appendable= */ true); AppendableUploadState state; if (takeOver) { - Crc32cLengthKnown initialCrc32c = null; - if (initialCrc32c != null) { - // TODO: doesn't accessing checksum require the dencryption key? - Opts filter = - opts.transformTo(ObjectSourceOpt.class) - .prepend( - Opts.from( - UnifiedOpts.fields(ImmutableSet.of(BlobField.SIZE, BlobField.CRC32C)))); - BlobInfo blob = internalObjectGet(info.getBlobId(), filter); - initialCrc32c = Crc32cValue.of(Utils.crc32cCodec.decode(blob.getCrc32c()), blob.getSize()); - } state = BidiUploadState.appendableTakeover( req, Retrying::newCallContext, maxPendingBytes, SettableApiFuture.create(), - initialCrc32c); + /* initialCrc32c= */ null); } else { state = BidiUploadState.appendableNew( @@ -1789,9 +1777,8 @@ BidiWriteObjectRequest getBidiWriteObjectRequest( Object object = codecs.blobInfo().encode(info); Object.Builder objectBuilder = object.toBuilder() - // required if the data is changing + // clear out the checksums, if a crc32cMatch is specified it'll come back via opts .clearChecksums() - // trimmed to shave payload size .clearGeneration() .clearMetageneration() .clearSize() diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/ITAppendableUploadFakeTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/ITAppendableUploadFakeTest.java index 76ac29aa1f..ddab9da924 100644 --- a/google-cloud-storage/src/test/java/com/google/cloud/storage/ITAppendableUploadFakeTest.java +++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/ITAppendableUploadFakeTest.java @@ -809,57 +809,6 @@ public void testFlushMultipleSegments_200ResponsePartialFlushHalfway() throws Ex } } - /** - * We get a retryable error in our first flush. We don't have a generation so we do a metadata - * lookup, but we get an ObjectNotFound, which means that GCS never received the WriteObjectSpec - * and never created the object. Thus, we just send the WriteObjectSpec again - */ - @Test - @Ignore("TODO: does this need to be implemented?") - public void retryableError_ObjectNotFound() throws Exception { - - ChecksummedTestContent abcde = content.slice(0, 5); - ChecksummedTestContent fghij = content.slice(5, 5); - - BidiWriteObjectRequest req1 = - REQ_OPEN.toBuilder() - .setFlush(true) - .setStateLookup(true) - .setChecksummedData(abcde.asChecksummedData()) - .build(); - - BidiWriteObjectRequest req2 = - BidiWriteObjectRequest.newBuilder() - .setWriteOffset(5) - .setChecksummedData(fghij.asChecksummedData()) - .setFinishWrite(true) - .build(); - BidiWriteObjectResponse res = BidiUploadTest.resourceFor(content); - - Map map = new ConcurrentHashMap<>(); - FakeStorage fake = - FakeStorage.of( - ImmutableMap.of( - req1, retryableErrorOnce(req1, res, map, 2), - req2, maxRetries(req2, res, map, 1)), - ImmutableMap.of(get_generation_mask, Object.getDefaultInstance())); - - try (FakeServer fakeServer = FakeServer.of(fake); - Storage storage = fakeServer.getGrpcStorageOptions().getService()) { - - BlobId id = BlobId.of("b", "o"); - BlobAppendableUpload b = - storage.blobAppendableUpload(BlobInfo.newBuilder(id).build(), UPLOAD_CONFIG); - try (AppendableUploadWriteableByteChannel channel = b.open()) { - channel.write(ByteBuffer.wrap(content.getBytes())); - } - BlobInfo bi = b.getResult().get(5, TimeUnit.SECONDS); - assertThat(bi.getSize()).isEqualTo(5); - - assertThat(map).isEqualTo(ImmutableMap.of(req1, 2, req2, 1)); - } - } - @Test public void crc32cWorks() throws Exception { byte[] b = new byte[25];