From 25d7eb7de9a89a1c416a02b859f4e0df3b832873 Mon Sep 17 00:00:00 2001 From: sandeeppal Date: Mon, 2 Nov 2026 10:42:45 -0800 Subject: [PATCH] fix(storage): do not apply retention or holds to parallel composite upload parts Parts and intermediate compose objects were built from the destination BlobInfo, so they inherited its retention, temporaryHold and eventBasedHold. GCS then rejected the part cleanup deletes, causing the upload to report failure after a successful final compose and leaving undeletable part objects behind. Fixes #14892 --- ...lelCompositeUploadWritableByteChannel.java | 5 ++ ...ompositeUploadWritableByteChannelTest.java | 86 +++++++++++++++++++ 2 files changed, 91 insertions(+) diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/ParallelCompositeUploadWritableByteChannel.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/ParallelCompositeUploadWritableByteChannel.java index 464c6b2372d4..3b4467cc80b9 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/ParallelCompositeUploadWritableByteChannel.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/ParallelCompositeUploadWritableByteChannel.java @@ -441,6 +441,11 @@ private BlobInfo definePart(BlobInfo ultimateObject, PartRange partRange, long o // however, gcs will not accept that version resource identifier when creating a new object // strip it out, so it can be included as a query string parameter instead b.setKmsKeyName(null); + // parts are transient and must remain deletable. Retention and holds only belong on the + // ultimate object, which the final compose applies from ultimateObject. + b.setRetention(null); + b.setTemporaryHold(null); + b.setEventBasedHold(null); b = partMetadataFieldDecorator.apply(b); return b.build(); } diff --git a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/ParallelCompositeUploadWritableByteChannelTest.java b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/ParallelCompositeUploadWritableByteChannelTest.java index 2990aae5dd69..2750e9ee80d3 100644 --- a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/ParallelCompositeUploadWritableByteChannelTest.java +++ b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/ParallelCompositeUploadWritableByteChannelTest.java @@ -25,6 +25,7 @@ import com.google.api.gax.grpc.GrpcStatusCode; import com.google.api.gax.rpc.ApiExceptionFactory; import com.google.api.gax.rpc.ApiExceptions; +import com.google.cloud.storage.BlobInfo.Retention; import com.google.cloud.storage.BufferHandlePool.PooledBuffer; import com.google.cloud.storage.Crc32cValue.Crc32cLengthKnown; import com.google.cloud.storage.MetadataField.PartRange; @@ -846,6 +847,91 @@ public BlobInfo compose(ComposeRequest composeRequest) { pcu.close(); } + @Test + public void partsDoNotInheritRetentionOrHolds() throws Exception { + Retention retention = + Retention.newBuilder() + .setMode(Retention.Mode.LOCKED) + .setRetainUntilTime(OffsetDateTime.parse("2100-01-01T00:00:00Z")) + .build(); + BlobInfo ultimate = + info.toBuilder() + .setRetention(retention) + .setTemporaryHold(true) + .setEventBasedHold(true) + .build(); + + List partTargets = Collections.synchronizedList(new ArrayList<>()); + AtomicReference finalTarget = new AtomicReference<>(); + FakeStorageInternal storageInternal = + new FakeStorageInternal() { + @Override + public BlobInfo internalDirectUpload( + BlobInfo info, Opts opts, ByteBuffer buf) { + partTargets.add(info); + return super.internalDirectUpload(info, opts, buf); + } + + @Override + public BlobInfo compose(ComposeRequest composeRequest) { + BlobInfo target = composeRequest.getTarget(); + if (target.getBlobId().getName().startsWith(partNamingStrategy.prefix)) { + partTargets.add(target); + } else { + finalTarget.set(target); + } + return super.compose(composeRequest); + } + + @Override + public Void internalObjectDelete(BlobId id, Opts opts) { + // mimic GCS: retained or held objects cannot be deleted + Data d = addedObjects.get(id); + if (d != null) { + BlobInfo i = d.getInfo(); + if (i.getRetention() != null + || Boolean.TRUE.equals(i.getTemporaryHold()) + || Boolean.TRUE.equals(i.getEventBasedHold())) { + deleteRequests.add(id); + throw ApiExceptionFactory.createException( + null, GrpcStatusCode.of(Code.PERMISSION_DENIED), false); + } + } + return super.internalObjectDelete(id, opts); + } + }; + ParallelCompositeUploadWritableByteChannel pcu = + new ParallelCompositeUploadWritableByteChannel( + bufferHandlePool, + MoreExecutors.directExecutor(), + partNamingStrategy, + PartCleanupStrategy.always(), + 2, + partMetadataFieldDecorator, + finalObject, + storageInternal, + ultimate, + opts); + + byte[] bytes = DataGenerator.base64Characters().genBytes(bufferCapacity * 3 - 1); + pcu.write(ByteBuffer.wrap(bytes)); + pcu.close(); + + BlobInfo result = ApiFutureUtils.await(finalObject); + + assertAll( + () -> assertThat(result.getName()).isEqualTo(info.getName()), + // 3 direct uploads + 2 intermediate composes + () -> assertThat(partTargets).hasSize(5), + () -> assertThat(partTargets.stream().allMatch(p -> p.getRetention() == null)).isTrue(), + () -> assertThat(partTargets.stream().allMatch(p -> p.getTemporaryHold() == null)).isTrue(), + () -> + assertThat(partTargets.stream().allMatch(p -> p.getEventBasedHold() == null)).isTrue(), + () -> assertThat(finalTarget.get().getRetention()).isEqualTo(retention), + () -> assertThat(finalTarget.get().getTemporaryHold()).isTrue(), + () -> assertThat(finalTarget.get().getEventBasedHold()).isTrue()); + } + @NonNull private ParallelCompositeUploadWritableByteChannel defaultPcu(int maxElementsPerCompact) { return new ParallelCompositeUploadWritableByteChannel(