From c20527d7ffc2f14cdbd1d5cd607d8f523e573ad8 Mon Sep 17 00:00:00 2001 From: Tom Desair Date: Sun, 25 Oct 2026 13:06:30 +0100 Subject: [PATCH] Optimize S3 server-side compose capability check using encoded part sizes - Add S3Utils.getPartSize to resolve part sizes in O(1) time from encoded part key name suffixes with statObject fallback - Add canCompose and canComposePartialUploads to S3ServerSideComposeHelper to eliminate N+1 statObject HTTP calls - Manage compose capability flag in S3ServerSideComposeHelper with automatic fallback disablement in S3ConcatenationService - Streamline prepareStreamWithExistingIncompletePart in S3StorageService to inspect parts in a single pass --- .../upload/s3/S3ConcatenationService.java | 29 +--- .../upload/s3/S3ServerSideComposeHelper.java | 103 ++++++++++++- .../server/upload/s3/S3StorageService.java | 133 +++++++---------- .../desair/tus/server/upload/s3/S3Utils.java | 31 ++++ .../upload/s3/S3ConcatenationServiceTest.java | 4 + .../s3/S3ServerSideComposeHelperTest.java | 135 ++++++++++++++++++ .../upload/s3/S3StorageServiceTest.java | 21 ++- .../tus/server/upload/s3/S3UtilsTest.java | 42 ++++++ 8 files changed, 386 insertions(+), 112 deletions(-) diff --git a/src/main/java/me/desair/tus/server/upload/s3/S3ConcatenationService.java b/src/main/java/me/desair/tus/server/upload/s3/S3ConcatenationService.java index e22aec60..2ce22f64 100644 --- a/src/main/java/me/desair/tus/server/upload/s3/S3ConcatenationService.java +++ b/src/main/java/me/desair/tus/server/upload/s3/S3ConcatenationService.java @@ -195,29 +195,8 @@ public void merge(UploadInfo uploadInfo) throws IOException, UploadNotFoundExcep boolean completed = checkAllCompleted(expirationPeriod, partialUploads); if (totalLength != null && totalLength > 0 && completed) { - // AWS S3 composeObject / multipart upload constraints: - // 1. Single part: must be >= minPartSize (5MB) and have valid storageUploadId to compose. - // 2. Multiple parts (>= 2): all parts EXCEPT the final part must be >= minPartSize (5MB). - // The final part is allowed to be < 5MB while still using zero-copy server-side - // composition. - boolean canUseServerSideCopy = !partialUploads.isEmpty(); - if (partialUploads.size() == 1) { - UploadInfo p = partialUploads.get(0); - canUseServerSideCopy = - p.getLength() != null && p.getLength() >= minPartSize && p.getStorageUploadId() != null; - } else { - for (int i = 0; i < partialUploads.size(); i++) { - UploadInfo p = partialUploads.get(i); - if (p.getLength() == null || p.getStorageUploadId() == null) { - canUseServerSideCopy = false; - break; - } - if (i < partialUploads.size() - 1 && p.getLength() < minPartSize) { - canUseServerSideCopy = false; - break; - } - } - } + boolean canUseServerSideCopy = + s3ComposeHelper.canComposePartialUploads(partialUploads, minPartSize); String targetObjectKey = buildObjectKey(uploadInfo.getId()); @@ -288,8 +267,10 @@ private void mergeUsingServerSideCopy( s3ComposeHelper.compose(bucket, targetKey, partKeys); } catch (Exception e) { // If server-side composition fails for any reason (e.g. S3 permissions, regional policy, - // or unhandled multipart copy restriction), fall back to streaming re-upload as a safety net. + // or unhandled multipart copy restriction), mark compose as unsupported on this helper + // and fall back to streaming re-upload as a safety net. // This ensures concatenated uploads always succeed even if server-side copy is denied. + s3ComposeHelper.setComposeSupported(false); log.warn( "Server-side S3 composition failed for target key {}; falling back to streaming re-upload: {}", targetKey, diff --git a/src/main/java/me/desair/tus/server/upload/s3/S3ServerSideComposeHelper.java b/src/main/java/me/desair/tus/server/upload/s3/S3ServerSideComposeHelper.java index b4c90204..9b0adf04 100644 --- a/src/main/java/me/desair/tus/server/upload/s3/S3ServerSideComposeHelper.java +++ b/src/main/java/me/desair/tus/server/upload/s3/S3ServerSideComposeHelper.java @@ -22,6 +22,9 @@ import java.time.ZonedDateTime; import java.util.ArrayList; import java.util.List; +import java.util.concurrent.locks.ReadWriteLock; +import java.util.concurrent.locks.ReentrantReadWriteLock; +import me.desair.tus.server.upload.UploadInfo; import okhttp3.HttpUrl; import okhttp3.OkHttpClient; import okhttp3.Request; @@ -62,6 +65,8 @@ public class S3ServerSideComposeHelper implements Closeable { private final Provider provider; private final OkHttpClient httpClient; private final String explicitRegion; + private final ReadWriteLock thisObjectLock = new ReentrantReadWriteLock(); + private boolean composeSupported = true; /** * Constructs an instance with explicit connection parameters without requiring a {@link @@ -89,15 +94,15 @@ public S3ServerSideComposeHelper( this(minioClient, null, null, null, null); } - private S3ServerSideComposeHelper( + S3ServerSideComposeHelper( MinioClient minioClient, String endpoint, String region, String accessKey, String secretKey) { - this.minioClient = minioClient; this.explicitRegion = (region != null && !region.isEmpty()) ? region : "local"; Http.BaseUrl base = null; Provider prov = null; OkHttpClient client = null; MinioAsyncClient async = null; + MinioClient syncClient = minioClient; if (endpoint != null && !endpoint.isEmpty()) { try { @@ -114,6 +119,9 @@ private S3ServerSideComposeHelper( asyncBuilder.region(region); } async = asyncBuilder.build(); + if (syncClient == null) { + syncClient = S3Utils.buildMinioClient(endpoint, region, accessKey, secretKey); + } } catch (Exception e) { log.warn( "Failed to initialize native S3 connection from endpoint {}: {}", @@ -122,6 +130,7 @@ private S3ServerSideComposeHelper( } } + this.minioClient = syncClient; this.baseUrl = base; this.provider = prov; this.httpClient = client; @@ -137,6 +146,96 @@ public boolean isAvailable() { return asyncClient != null && baseUrl != null && httpClient != null; } + /** + * Returns whether S3 server-side composition is supported by the storage endpoint. + * + * @return {@code true} if server-side compose is supported; {@code false} if disabled or failed + */ + public boolean isComposeSupported() { + thisObjectLock.readLock().lock(); + try { + return composeSupported; + } finally { + thisObjectLock.readLock().unlock(); + } + } + + /** + * Configures whether S3 server-side composition is supported by the storage endpoint. + * + * @param composeSupported {@code true} to enable; {@code false} to disable + */ + public void setComposeSupported(boolean composeSupported) { + thisObjectLock.writeLock().lock(); + try { + this.composeSupported = composeSupported; + } finally { + thisObjectLock.writeLock().unlock(); + } + } + + /** + * Checks whether the given part keys meet S3's minimum part size requirement for server-side + * composition (all parts except the final part must be at least {@code minPartSize} bytes). + * + *

Part sizes are resolved in O(1) time from the {@code _} suffix encoded in each part + * key name, falling back to {@code statObject} only for legacy keys without a size suffix. + * + * @param bucket The S3 bucket name + * @param partKeys The ordered list of part keys to compose + * @param minPartSize Minimum required byte size for non-final parts (typically 5 MiB) + * @return {@code true} if server-side composition can be used; {@code false} otherwise + */ + public boolean canCompose(String bucket, List partKeys, long minPartSize) { + if (!isComposeSupported() || partKeys == null || partKeys.isEmpty()) { + return false; + } + // AWS S3 requires all parts except the last to be >= minPartSize (5 MB) + for (int i = 0; i < partKeys.size() - 1; i++) { + if (S3Utils.getPartSize(minioClient, bucket, partKeys.get(i)) < minPartSize) { + return false; + } + } + return true; + } + + /** + * Checks whether the given partial uploads meet S3's constraints for server-side composition. + * + *

AWS S3 composeObject / multipart upload constraints: + * + *

+ * + * @param partialUploads The partial uploads to check + * @param minPartSize Minimum required byte size for non-final parts (typically 5 MiB) + * @return {@code true} if server-side composition can be used; {@code false} otherwise + */ + public boolean canComposePartialUploads(List partialUploads, long minPartSize) { + if (!isComposeSupported() || partialUploads == null || partialUploads.isEmpty()) { + return false; + } + if (partialUploads.size() == 1) { + UploadInfo p = partialUploads.get(0); + return p.getLength() != null + && p.getLength() >= minPartSize + && p.getStorageUploadId() != null; + } + for (int i = 0; i < partialUploads.size(); i++) { + UploadInfo p = partialUploads.get(i); + if (p.getLength() == null || p.getStorageUploadId() == null) { + return false; + } + if (i < partialUploads.size() - 1 && p.getLength() < minPartSize) { + return false; + } + } + return true; + } + /** * Composes the given part keys into the target object key entirely server-side on S3. * diff --git a/src/main/java/me/desair/tus/server/upload/s3/S3StorageService.java b/src/main/java/me/desair/tus/server/upload/s3/S3StorageService.java index 0203a0b6..39e1f337 100644 --- a/src/main/java/me/desair/tus/server/upload/s3/S3StorageService.java +++ b/src/main/java/me/desair/tus/server/upload/s3/S3StorageService.java @@ -26,8 +26,6 @@ import java.util.NoSuchElementException; import java.util.Objects; import java.util.UUID; -import java.util.concurrent.locks.ReadWriteLock; -import java.util.concurrent.locks.ReentrantReadWriteLock; import java.util.stream.Collectors; import java.util.stream.StreamSupport; import me.desair.tus.server.checksum.ChecksumKey; @@ -110,8 +108,6 @@ public class S3StorageService extends AbstractUploadStorageService { private final S3ChecksumIndex checksumIndex; private DeduplicationService deduplicationService; - private final ReadWriteLock thisObjectLock = new ReentrantReadWriteLock(); - private boolean s3ComposeObjectSupported = true; private S3ServerSideComposeHelper s3ComposeHelper; /** @@ -751,25 +747,21 @@ public boolean isS3ConditionalWritesSupported() { } boolean isS3ComposeObjectSupported() { - thisObjectLock.readLock().lock(); - try { - return s3ComposeObjectSupported; - } finally { - thisObjectLock.readLock().unlock(); - } + return s3ComposeHelper != null && s3ComposeHelper.isComposeSupported(); } void setS3ComposeObjectSupported(boolean s3ComposeObjectSupported) { - thisObjectLock.writeLock().lock(); - try { - this.s3ComposeObjectSupported = s3ComposeObjectSupported; - } finally { - thisObjectLock.writeLock().unlock(); + if (this.s3ComposeHelper != null) { + this.s3ComposeHelper.setComposeSupported(s3ComposeObjectSupported); } } void setS3ServerSideComposeHelper(S3ServerSideComposeHelper s3ComposeHelper) { this.s3ComposeHelper = s3ComposeHelper; + if (this.concatenationService instanceof S3ConcatenationService) { + ((S3ConcatenationService) this.concatenationService) + .setS3ServerSideComposeHelper(s3ComposeHelper); + } } S3ServerSideComposeHelper getS3ServerSideComposeHelper() { @@ -874,49 +866,51 @@ private PreparedStream prepareStreamWithExistingIncompletePart( ? new ArrayList<>(info.getUploadPartKeys()) : new ArrayList<>(); - long existingPartsTotalSize = calculateTotalPartsSize(existingPartKeys); + long existingPartsTotalSize = 0L; + long lastPartSize = 0L; + for (int i = 0; i < existingPartKeys.size(); i++) { + long partSize = getPartSize(existingPartKeys.get(i)); + existingPartsTotalSize += partSize; + if (i == existingPartKeys.size() - 1) { + lastPartSize = partSize; + } + } // 2. Roll back any existing sub-5MB numbered parts into the stream buffer so they can grow - if (!existingPartKeys.isEmpty()) { + if (!existingPartKeys.isEmpty() && lastPartSize > 0 && lastPartSize < minPartSize) { String lastPartKey = existingPartKeys.get(existingPartKeys.size() - 1); - long lastPartSize = getPartSize(lastPartKey); - if (lastPartSize > 0 && lastPartSize < minPartSize) { - log.info( - "Rolling back sub-5MB part {} (size: {}) into stream buffer", - lastPartKey, - lastPartSize); - // Stream into a secure temp file instead of buffering up to 5 MB directly in JVM heap - File tempPrepFile = - Files.createTempFile(temporaryDirectory, "tus-s3-prep-", ".tmp").toFile(); - InputStream prependedStream = null; - try { - try (InputStream lastPartStream = - S3Utils.getObjectInputStream(minioClient, bucket, lastPartKey); - FileOutputStream fos = new FileOutputStream(tempPrepFile)) { - IOUtils.copyLarge(lastPartStream, fos); - } - long tempPrepLength = tempPrepFile.length(); - prependedBytes += tempPrepLength; - prependedStream = - new FileInputStream(tempPrepFile) { - @Override - public void close() throws IOException { - try { - super.close(); - } finally { - FileUtils.deleteQuietly(tempPrepFile); - } - } - }; - combinedStream = new SequenceInputStream(prependedStream, combinedStream); - staleTailKeys.add(lastPartKey); - existingPartsTotalSize -= tempPrepLength; - existingPartKeys.remove(existingPartKeys.size() - 1); - } catch (Exception e) { - IOUtils.closeQuietly(prependedStream); - FileUtils.deleteQuietly(tempPrepFile); - log.debug("Error inspecting last part key {}: {}", lastPartKey, e.getMessage()); + log.info( + "Rolling back sub-5MB part {} (size: {}) into stream buffer", lastPartKey, lastPartSize); + // Stream into a secure temp file instead of buffering up to 5 MB directly in JVM heap + File tempPrepFile = Files.createTempFile(temporaryDirectory, "tus-s3-prep-", ".tmp").toFile(); + InputStream prependedStream = null; + try { + try (InputStream lastPartStream = + S3Utils.getObjectInputStream(minioClient, bucket, lastPartKey); + FileOutputStream fos = new FileOutputStream(tempPrepFile)) { + IOUtils.copyLarge(lastPartStream, fos); } + long tempPrepLength = tempPrepFile.length(); + prependedBytes += tempPrepLength; + prependedStream = + new FileInputStream(tempPrepFile) { + @Override + public void close() throws IOException { + try { + super.close(); + } finally { + FileUtils.deleteQuietly(tempPrepFile); + } + } + }; + combinedStream = new SequenceInputStream(prependedStream, combinedStream); + staleTailKeys.add(lastPartKey); + existingPartsTotalSize -= tempPrepLength; + existingPartKeys.remove(existingPartKeys.size() - 1); + } catch (Exception e) { + IOUtils.closeQuietly(prependedStream); + FileUtils.deleteQuietly(tempPrepFile); + log.debug("Error inspecting last part key {}: {}", lastPartKey, e.getMessage()); } } @@ -946,19 +940,7 @@ private long calculateTotalPartsSize(List partKeys) { * extracted in O(1) time without any network calls; otherwise falls back to statObject. */ private long getPartSize(String partKey) { - if (partKey == null) { - return 0L; - } - int underscoreIdx = partKey.lastIndexOf('_'); - if (underscoreIdx >= 0 && underscoreIdx < partKey.length() - 1) { - try { - return Long.parseLong(partKey.substring(underscoreIdx + 1)); - } catch (NumberFormatException ignored) { - // Fall back to statObject if suffix is not numeric - } - } - long size = S3Utils.getObjectSize(minioClient, bucket, partKey); - return size >= 0 ? size : 0L; + return S3Utils.getPartSize(minioClient, bucket, partKey); } /** @@ -1238,20 +1220,9 @@ private void finalizeCompletedUploadIfFinished( } if (!partKeys.isEmpty()) { - boolean canUseServerSideCompose = true; - if (partKeys.size() > 1) { - // AWS S3 requires all parts except the last to be >= 5 MB (5,242,880 bytes) for - // composeObject. We check sizes using getPartSize() in O(1) time without statObject. - for (int i = 0; i < partKeys.size() - 1; i++) { - long size = getPartSize(partKeys.get(i)); - if (size < minPartSize) { - canUseServerSideCompose = false; - break; - } - } - } + boolean canUseServerSideCompose = s3ComposeHelper.canCompose(bucket, partKeys, minPartSize); - if (canUseServerSideCompose && isS3ComposeObjectSupported()) { + if (canUseServerSideCompose) { try { // Perform S3 server-side object composition via native S3 multipart copy s3ComposeHelper.compose(bucket, objectKey, partKeys); @@ -1272,7 +1243,7 @@ private void finalizeCompletedUploadIfFinished( mergeUsingStreamingReupload(objectKey, partKeys, newOffset); } } else { - if (!canUseServerSideCompose) { + if (isS3ComposeObjectSupported()) { log.info( "Detected sub-5MB non-final parts for ID {}. Using streaming part composition" + " fallback.", diff --git a/src/main/java/me/desair/tus/server/upload/s3/S3Utils.java b/src/main/java/me/desair/tus/server/upload/s3/S3Utils.java index 896923e4..8ebbd8e2 100644 --- a/src/main/java/me/desair/tus/server/upload/s3/S3Utils.java +++ b/src/main/java/me/desair/tus/server/upload/s3/S3Utils.java @@ -192,6 +192,37 @@ public static long getObjectSize(MinioClient minioClient, String bucket, String } } + /** + * Resolves the byte size of an S3 part key. + * + *

If the key encodes its size suffix ({@code _}), the byte count is extracted in O(1) + * time without any remote network calls. If the key lacks a numeric suffix (e.g. legacy part keys + * or mock test keys), this method falls back to {@link #getObjectSize} via {@code statObject}. + * + * @param minioClient The {@link MinioClient} + * @param bucket The target bucket name + * @param partKey The S3 part object key + * @return The part size in bytes, or 0L if the key is null or an error occurs + */ + public static long getPartSize(MinioClient minioClient, String bucket, String partKey) { + if (partKey == null) { + return 0L; + } + int underscoreIdx = partKey.lastIndexOf('_'); + if (underscoreIdx >= 0 && underscoreIdx < partKey.length() - 1) { + try { + long parsed = Long.parseLong(partKey.substring(underscoreIdx + 1)); + if (parsed >= 0) { + return parsed; + } + } catch (NumberFormatException ignored) { + // Fall back to statObject if suffix is not numeric + } + } + long size = getObjectSize(minioClient, bucket, partKey); + return size >= 0 ? size : 0L; + } + /** * Checks whether an S3 object exists in the specified bucket. * diff --git a/src/test/java/me/desair/tus/server/upload/s3/S3ConcatenationServiceTest.java b/src/test/java/me/desair/tus/server/upload/s3/S3ConcatenationServiceTest.java index f712e526..df062518 100644 --- a/src/test/java/me/desair/tus/server/upload/s3/S3ConcatenationServiceTest.java +++ b/src/test/java/me/desair/tus/server/upload/s3/S3ConcatenationServiceTest.java @@ -558,6 +558,8 @@ public void testSetS3ServerSideComposeHelper() { @Test public void testMergeResolvesObjectKeysWhenStorageServiceNotS3StorageService() throws Exception { S3ServerSideComposeHelper mockHelper = Mockito.mock(S3ServerSideComposeHelper.class); + Mockito.when(mockHelper.canComposePartialUploads(Mockito.any(), Mockito.anyLong())) + .thenReturn(true); concatenationService.setS3ServerSideComposeHelper(mockHelper); UploadInfo childDup = new UploadInfo(); @@ -605,6 +607,8 @@ public void testMergeResolvesObjectKeysWhenStorageServiceIsS3StorageService() th null); S3ServerSideComposeHelper mockHelper = Mockito.mock(S3ServerSideComposeHelper.class); + Mockito.when(mockHelper.canComposePartialUploads(Mockito.any(), Mockito.anyLong())) + .thenReturn(true); service.setS3ServerSideComposeHelper(mockHelper); UploadInfo child1 = new UploadInfo(); diff --git a/src/test/java/me/desair/tus/server/upload/s3/S3ServerSideComposeHelperTest.java b/src/test/java/me/desair/tus/server/upload/s3/S3ServerSideComposeHelperTest.java index 552c7b0f..bfc50b5e 100644 --- a/src/test/java/me/desair/tus/server/upload/s3/S3ServerSideComposeHelperTest.java +++ b/src/test/java/me/desair/tus/server/upload/s3/S3ServerSideComposeHelperTest.java @@ -28,6 +28,7 @@ import java.util.Collections; import java.util.List; import java.util.concurrent.CompletableFuture; +import me.desair.tus.server.upload.UploadInfo; import okhttp3.Call; import okhttp3.HttpUrl; import okhttp3.MediaType; @@ -268,6 +269,11 @@ public void testExplicitConnectionParametersConstructor() { new S3ServerSideComposeHelper("https://s3.amazonaws.com", null, "testKey", "testSecret"); assertTrue(helperWithNullRegion.isAvailable()); + S3ServerSideComposeHelper helperWithBoth = + new S3ServerSideComposeHelper( + realClient, "https://s3.amazonaws.com", "us-east-1", "testKey", "testSecret"); + assertTrue(helperWithBoth.isAvailable()); + S3ServerSideComposeHelper minioOnlyHelper = new S3ServerSideComposeHelper(realClient); assertFalse(minioOnlyHelper.isAvailable()); } @@ -421,6 +427,135 @@ public void testNativeComposeRegionFallbackToLocal() throws Exception { verify(asyncClient).completeMultipartUpload(any(CompleteMultipartUploadArgs.class)); } + @Test + public void testComposeSupportedToggle() { + S3ServerSideComposeHelper helper = new S3ServerSideComposeHelper(null); + assertTrue(helper.isComposeSupported()); + + helper.setComposeSupported(false); + assertFalse(helper.isComposeSupported()); + + helper.setComposeSupported(true); + assertTrue(helper.isComposeSupported()); + } + + @Test + public void testCanComposeWithEncodedPartSizes() throws Exception { + MinioClient mockClient = mock(MinioClient.class); + S3ServerSideComposeHelper helper = new S3ServerSideComposeHelper(mockClient); + + long minPartSize = 5L * 1024 * 1024; // 5 MB + + // Null or empty part list + assertFalse(helper.canCompose("test-bucket", null, minPartSize)); + assertFalse(helper.canCompose("test-bucket", Collections.emptyList(), minPartSize)); + + // When compose is disabled + helper.setComposeSupported(false); + assertFalse( + helper.canCompose( + "test-bucket", Collections.singletonList("upload.part.00001_5242880"), minPartSize)); + helper.setComposeSupported(true); + + // Single part + assertTrue( + helper.canCompose( + "test-bucket", Collections.singletonList("upload.part.00001_100"), minPartSize)); + + // Multiple parts where all except final are >= 5 MB (final part can be < 5 MB) + List validParts = + Arrays.asList( + "upload.part.00001_5242880", + "upload.part.00002_10485760", + "upload.part.00003_100"); // final part < 5MB is allowed + assertTrue(helper.canCompose("test-bucket", validParts, minPartSize)); + + // Multiple parts where a non-final part is < 5 MB + List invalidParts = + Arrays.asList( + "upload.part.00001_5242880", + "upload.part.00002_100", // non-final sub-5MB part rejected + "upload.part.00003_5242880"); + assertFalse(helper.canCompose("test-bucket", invalidParts, minPartSize)); + + // Verify no remote statObject calls were issued during encoded key checks + verify(mockClient, org.mockito.Mockito.never()).statObject(any(io.minio.StatObjectArgs.class)); + } + + @Test + public void testCanComposePartialUploads() { + S3ServerSideComposeHelper helper = new S3ServerSideComposeHelper(null); + long minPartSize = 5L * 1024 * 1024; // 5 MB + + // Null or empty + assertFalse(helper.canComposePartialUploads(null, minPartSize)); + assertFalse(helper.canComposePartialUploads(Collections.emptyList(), minPartSize)); + + // When compose is disabled + helper.setComposeSupported(false); + UploadInfo pValid = new UploadInfo(); + pValid.setLength(minPartSize); + pValid.setStorageUploadId("uploads/p1"); + assertFalse(helper.canComposePartialUploads(Collections.singletonList(pValid), minPartSize)); + helper.setComposeSupported(true); + + // Single part: valid + assertTrue(helper.canComposePartialUploads(Collections.singletonList(pValid), minPartSize)); + + // Single part: missing storageUploadId + UploadInfo pNoStorageId = new UploadInfo(); + pNoStorageId.setLength(minPartSize); + pNoStorageId.setStorageUploadId(null); + assertFalse( + helper.canComposePartialUploads(Collections.singletonList(pNoStorageId), minPartSize)); + + // Single part: length < minPartSize + UploadInfo pSmall = new UploadInfo(); + pSmall.setLength(minPartSize - 1); + pSmall.setStorageUploadId("uploads/p-small"); + assertFalse(helper.canComposePartialUploads(Collections.singletonList(pSmall), minPartSize)); + + // Single part: null length + UploadInfo pNullLen = new UploadInfo(); + pNullLen.setLength(null); + pNullLen.setStorageUploadId("uploads/p-null"); + assertFalse(helper.canComposePartialUploads(Collections.singletonList(pNullLen), minPartSize)); + + // Multiple parts: valid (non-final >= 5MB, final < 5MB allowed) + UploadInfo p1 = new UploadInfo(); + p1.setLength(10L * 1024 * 1024); + p1.setStorageUploadId("uploads/p1"); + + UploadInfo p2 = new UploadInfo(); + p2.setLength(5L * 1024 * 1024); + p2.setStorageUploadId("uploads/p2"); + + UploadInfo p3Final = new UploadInfo(); + p3Final.setLength(100L); // < 5MB allowed for final part + p3Final.setStorageUploadId("uploads/p3"); + + assertTrue(helper.canComposePartialUploads(Arrays.asList(p1, p2, p3Final), minPartSize)); + + // Multiple parts: non-final part < 5MB + UploadInfo p2Small = new UploadInfo(); + p2Small.setLength(100L); + p2Small.setStorageUploadId("uploads/p2-small"); + assertFalse(helper.canComposePartialUploads(Arrays.asList(p1, p2Small, p3Final), minPartSize)); + + // Multiple parts: missing storageUploadId on one part + UploadInfo p2NoId = new UploadInfo(); + p2NoId.setLength(10L * 1024 * 1024); + p2NoId.setStorageUploadId(null); + assertFalse(helper.canComposePartialUploads(Arrays.asList(p1, p2NoId, p3Final), minPartSize)); + + // Multiple parts: null length on one part + UploadInfo p2NullLen = new UploadInfo(); + p2NullLen.setLength(null); + p2NullLen.setStorageUploadId("uploads/p2"); + assertFalse( + helper.canComposePartialUploads(Arrays.asList(p1, p2NullLen, p3Final), minPartSize)); + } + private static void setField(Object target, String fieldName, Object value) throws Exception { Field field = S3ServerSideComposeHelper.class.getDeclaredField(fieldName); field.setAccessible(true); diff --git a/src/test/java/me/desair/tus/server/upload/s3/S3StorageServiceTest.java b/src/test/java/me/desair/tus/server/upload/s3/S3StorageServiceTest.java index 7a7e7459..be126532 100644 --- a/src/test/java/me/desair/tus/server/upload/s3/S3StorageServiceTest.java +++ b/src/test/java/me/desair/tus/server/upload/s3/S3StorageServiceTest.java @@ -3127,9 +3127,20 @@ public void testIsJsonSerializationEnabled() { @Test public void testSetAndGetS3ServerSideComposeHelper() { + // When helper is null + storageService.setS3ServerSideComposeHelper(null); + assertNull(storageService.getS3ServerSideComposeHelper()); + assertFalse(storageService.isS3ComposeObjectSupported()); + storageService.setS3ComposeObjectSupported(true); // null-safe + + // When helper is present S3ServerSideComposeHelper mockHelper = mock(S3ServerSideComposeHelper.class); + when(mockHelper.isComposeSupported()).thenReturn(true); storageService.setS3ServerSideComposeHelper(mockHelper); assertEquals(mockHelper, storageService.getS3ServerSideComposeHelper()); + assertTrue(storageService.isS3ComposeObjectSupported()); + storageService.setS3ComposeObjectSupported(false); + verify(mockHelper).setComposeSupported(false); } @Test @@ -3152,7 +3163,7 @@ public void testPrepareStreamWithExistingIncompletePartInspectionExceptionHandle info.setId(uploadId); info.setLength(1000L); info.setOffset(50L); - String partKey = "metadata/part-inspect-err-id.part.00001"; + String partKey = "metadata/part-inspect-err-id.part.00001_50"; info.setUploadPartKeys(new ArrayList<>(Collections.singletonList(partKey))); when(minioClient.getObject( @@ -3161,10 +3172,10 @@ public void testPrepareStreamWithExistingIncompletePartInspectionExceptionHandle a != null && "metadata/part-inspect-err-id.info".equals(a.object())))) .thenReturn(mockGetObjectResponse(UploadInfoJsonSerializer.serialize(info).getBytes())); - // StatObject on last part throws Exception to test lines 1033-1034 catch block - when(minioClient.statObject( - argThat((StatObjectArgs a) -> a != null && partKey.equals(a.object())))) - .thenThrow(new RuntimeException("Part inspection failed")); + // GetObject on sub-5MB part throws Exception to test lines 911-913 catch block + when(minioClient.getObject( + argThat((GetObjectArgs a) -> a != null && partKey.equals(a.object())))) + .thenThrow(new RuntimeException("Part download failed")); UploadInfo result = storageService.append(info, new ByteArrayInputStream(new byte[0])); assertNotNull(result); diff --git a/src/test/java/me/desair/tus/server/upload/s3/S3UtilsTest.java b/src/test/java/me/desair/tus/server/upload/s3/S3UtilsTest.java index d2a5d91e..47c03089 100644 --- a/src/test/java/me/desair/tus/server/upload/s3/S3UtilsTest.java +++ b/src/test/java/me/desair/tus/server/upload/s3/S3UtilsTest.java @@ -216,6 +216,48 @@ public void testGetObjectSize() throws Exception { assertEquals(-1L, S3Utils.getObjectSize(client, "bucket", "key")); } + @Test + public void testGetPartSize() throws Exception { + MinioClient client = mock(MinioClient.class); + + // Null key returns 0L without statObject calls + assertEquals(0L, S3Utils.getPartSize(client, "bucket", null)); + verify(client, times(0)).statObject(any(StatObjectArgs.class)); + + // Valid encoded numeric suffix returns parsed size in O(1) without statObject calls + assertEquals( + 5242880L, + S3Utils.getPartSize(client, "bucket", "metadata/upload.part.00001-abc12345_5242880")); + assertEquals( + 0L, S3Utils.getPartSize(client, "bucket", "metadata/upload.part.00001-abc12345_0")); + assertEquals(123456789L, S3Utils.getPartSize(client, "bucket", "part_123456789")); + verify(client, times(0)).statObject(any(StatObjectArgs.class)); + + // Non-numeric suffix falls back to statObject + StatObjectResponse statResponse = mock(StatObjectResponse.class); + when(statResponse.size()).thenReturn(1048576L); + when(client.statObject(any(StatObjectArgs.class))).thenReturn(statResponse); + + assertEquals(1048576L, S3Utils.getPartSize(client, "bucket", "metadata/part_nonnumeric")); + verify(client, times(1)).statObject(any(StatObjectArgs.class)); + + // Trailing underscore falls back to statObject + assertEquals(1048576L, S3Utils.getPartSize(client, "bucket", "metadata/part_")); + verify(client, times(2)).statObject(any(StatObjectArgs.class)); + + // Negative suffix falls back to statObject + assertEquals(1048576L, S3Utils.getPartSize(client, "bucket", "metadata/part_-50")); + verify(client, times(3)).statObject(any(StatObjectArgs.class)); + + // Missing underscore entirely falls back to statObject + assertEquals(1048576L, S3Utils.getPartSize(client, "bucket", "metadata/part-no-underscore")); + verify(client, times(4)).statObject(any(StatObjectArgs.class)); + + // S3 statObject error falls back to 0L + when(client.statObject(any(StatObjectArgs.class))).thenThrow(new RuntimeException("NoSuchKey")); + assertEquals(0L, S3Utils.getPartSize(client, "bucket", "missing_notanumber")); + } + @Test public void testObjectExists() throws Exception { MinioClient client = mock(MinioClient.class);