From a59b3dd37a762a178f3a1a94299b76579a64a4b3 Mon Sep 17 00:00:00 2001 From: Jency Mary Joseph Date: Wed, 23 Jul 2025 09:20:14 -0700 Subject: [PATCH] update design after surface API Review --- .../core/presignedURL-Get/DecisionLog.md | 23 ++++- docs/design/core/presignedURL-Get/Design.md | 88 +++++-------------- 2 files changed, 44 insertions(+), 67 deletions(-) diff --git a/docs/design/core/presignedURL-Get/DecisionLog.md b/docs/design/core/presignedURL-Get/DecisionLog.md index 84be9cb17bf4..31af819c4b45 100644 --- a/docs/design/core/presignedURL-Get/DecisionLog.md +++ b/docs/design/core/presignedURL-Get/DecisionLog.md @@ -2,7 +2,7 @@ # S3 Pre-signed URL GET - Decision Log ## Review Meeting: 06/17/2025 -**Attendees**: Alban, John, Zoe, Dongie, Bole, Ran, Saranya +**Attendees**: Alban Gicquel, John Viegas, Zoe Wang, Dongie Agnir, Bole Yi, Ran Vaknin, Saranya Somepalli ### Closed Decisions @@ -21,7 +21,7 @@ 3. What should we name the Helper API? Options include PresignedURLManager or PresignedUrlExtension. Will be addressed in the Surface API Review. ## Review Meeting: 06/23/2025 -**Attendees**: John, Zoe, Dongie, Bole, Ran, Saranya, Alex, David +**Attendees**: John Viegas, Zoe Wang, Dongie Agnir, Bole Yi, Ran Vaknin, Saranya Somepalli, David Ho ### Decisions Addressed @@ -32,7 +32,24 @@ 3. Use separate rangeStart/rangeEnd fields vs single range string parameter. Decided to use separate rangeStart and rangeEnd Long fields for better user experience, as start/end is more intuitive than string parsing. ## Decision Poll Meeting: 06/30/2025 -**Attendees**: John, Zoe, Dongie, Bole, Ran, Saranya, Alex +**Attendees**: John Viegas, Zoe Wang, Dongie Agnir, Bole Yi, Ran Vaknin, Saranya Somepalli, David Ho, Alex Woods ### Decision Addressed Decided to use String range field for Request object to support all RFC 7233 formats including suffix ranges (bytes=-100) and future multi-range support, since S3 currently doesn't support multiple ranges but may in the future without requiring SDK changes. + +## Post-standup Meeting: 07/14/2025 +**Attendees**: Alban Gicquel, John Viegas, Zoe Wang, Dongie Agnir, Bole Yi, Ran Vaknin, Saranya Somepalli, David Ho, Alex Woods + +### Decision Addressed +The team has decided to implement functionality only for S3 async client and defer S3 sync client implementation for now. This decision was made because implementing S3 sync client would require supporting multipart download capabilities. + +## API Surface Area Review: 07/21/2025 +**Attendees**: John Viegas, Zoe Wang, Dongie Agnir, Bole Yi, Ran Vaknin, Saranya Somepalli, David Ho, Alex Woods + +### Decisions Addressed + +1. Decided on the naming for the surface APIs - AsyncPresignedUrlExtension for the new core API, presignedUrlExtension() for the method to call from the AsyncS3Client and PresignedUrlDownloadRequest for the Get Object Request. Also decided to have PresignedUrlDownload as the Operation Name for metric collection. + +2. Remove the consumer builder pattern from Get Request Model. + +3. throw UnsupportedOperationException for Multipart S3 Client and S3 CRT Client for now. diff --git a/docs/design/core/presignedURL-Get/Design.md b/docs/design/core/presignedURL-Get/Design.md index 79d61b1f5147..183530100f75 100644 --- a/docs/design/core/presignedURL-Get/Design.md +++ b/docs/design/core/presignedURL-Get/Design.md @@ -10,61 +10,52 @@ This document proposes how this functionality should be implemented in the Java Look at decision log here: [Decision Log Section](DecisionLog.md) -The Java SDK team has decided to implement a separate `PresignedUrlManager`. The team chose the helper API pattern over direct `S3Client` integration to maintain clean separation of concerns while preserving SDK functionality. +The Java SDK team has decided to implement a separate `AsyncPresignedUrlExtension`. The team chose the helper API pattern over direct `S3AsyncClient` integration to maintain clean separation of concerns while preserving SDK functionality. ## Overview -The design introduces new helper APIs `AsyncPresignedUrlManager` and `PresignedUrlManager` which can be instantiated via the existing `S3AsyncClient` and `S3Client` respectively. These managers provide a clean abstraction layer that preserves SDK benefits while handling the unique requirements of pre-signed URL requests. +The design introduces a new helper API `AsyncPresignedUrlExtension` which can be instantiated via the existing `S3AsyncClient`. This extension provides a clean abstraction layer that preserves SDK benefits while handling the unique requirements of pre-signed URL requests. -This design will implement only the GET /download function for presigned URLs. +This design will implement only the GET /download function for presigned URLs for the S3AsyncClient. The synchronous S3Client implementation is deferred to future work. ## Proposed APIs -The v2 SDK will support a presigned URL manager for both sync and async clients that can leverage pre-signed URL downloads. +The v2 SDK will support a presigned URL extension for the async client that can leverage pre-signed URL downloads. ### Instantiation Instantiating from existing client: ```java -// Async Presigned URL Manager +// Async Presigned URL Extension S3AsyncClient s3Client = S3AsyncClient.create(); -AsyncPresignedUrlManager presignManager = s3Client.presignedManager(); - -// Sync Presigned URL Manager -S3Client s3Client = S3Client.create(); -PresignedUrlManager presignManager = s3Client.presignedManager(); +AsyncPresignedUrlExtension presignExtension = s3Client.presignedUrlExtension(); ``` ### General Usage Examples ```java // Create presigned URL request -PresignedUrlGetObjectRequest request = PresignedUrlGetObjectRequest.builder() +PresignedUrlDownloadRequest request = PresignedUrlDownloadRequest.builder() .presignedUrl(presignedUrl) .range("range=0-1024") .build(); // Async usage S3AsyncClient s3Client = S3AsyncClient.create(); -AsyncPresignedUrlManager presignManager = s3Client.presignedManager(); -CompletableFuture response = presignManager.getObject(request); - -// Sync usage -S3Client s3Client = S3Client.create(); -PresignedUrlManager presignManager = s3Client.presignedManager(); -GetObjectResponse response = presignManager.getObject(request); +AsyncPresignedUrlExtension presignedUrlExtension = s3Client.presignedUrlExtension(); +CompletableFuture response = presignedUrlExtension.getObject(request, AsyncResponseTransformer.toBytes()); ``` -### AsyncPresignedUrlManager Interface +### AsyncPresignedUrlExtension Interface ```java /** * Interface for presigned URL operations used by Async clients. */ @SdkPublicApi -public interface AsyncPresignedUrlManager { +public interface AsyncPresignedUrlExtension { /** * Downloads S3 objects using pre-signed URLs with custom response transformation. @@ -73,7 +64,7 @@ public interface AsyncPresignedUrlManager { * @param responseTransformer custom transformer for processing the response. * @return a CompletableFuture of the transformed response. */ - CompletableFuture getObject(PresignedUrlGetObjectRequest request, + CompletableFuture getObject(PresignedUrlDownloadRequest request, AsyncResponseTransformer responseTransformer); // Additional getObject() overloads for file downloads, byte arrays, etc. @@ -81,52 +72,18 @@ public interface AsyncPresignedUrlManager { } ``` -### PresignedUrlManager Interface - -```java -/** - * Interface for presigned URL operations used by Sync clients. - */ -@SdkPublicApi -public interface PresignedUrlManager { - - /** - * Downloads S3 objects using pre-signed URLs. Bypasses normal authentication - * and endpoint resolution while maintaining SDK benefits like retries and metrics. - * - * @param request the presigned URL request containing URL and optional range parameters. - * @return the GetObjectResponse. - */ - GetObjectResponse getObject(PresignedUrlGetObjectRequest request); - - /** - * Downloads S3 objects using pre-signed URLs with custom response transformation. - * - * @param request the presigned URL request. - * @param responseTransformer custom transformer for processing the response. - * @return the transformed response. - */ - T getObject(PresignedUrlGetObjectRequest request, - ResponseTransformer responseTransformer); - - // Additional getObject() overloads for file downloads, byte arrays, etc. - // Standard Builder interface with client() and overrideConfiguration() methods -} -``` - -### PresignedUrlGetObjectRequest +### PresignedUrlDownloadRequest ```java /** * Request object for presigned URL GET operations. */ @SdkPublicApi -@Immutable @ThreadSafe -public final class PresignedUrlGetObjectRequest - implements ToCopyableBuilder { +public final class PresignedUrlDownloadRequest + implements ToCopyableBuilder { - private final String presignedUrl; + private final URL presignedUrl; private final String range; // Standard getters: presignedUrl(), range() @@ -137,11 +94,11 @@ public final class PresignedUrlGetObjectRequest ## FAQ -### Why don't we implement presigned URL download/GET feature directly on the S3Client? +### Why don't we implement presigned URL download/GET feature directly on the S3AsyncClient? Three approaches were considered: -1. **Dedicated PresignedUrlManager (CHOSEN)**: Separate manager accessed via `s3Client.presignedManager()` +1. **Dedicated AsyncPresignedUrlExtension (CHOSEN)**: Separate extension accessed via `s3AsyncClient.presignedUrlExtension()` - **Pros**: Clean separation, preserves SDK features, follows v2 patterns - **Cons**: New API surface for users to learn @@ -153,12 +110,15 @@ Three approaches were considered: - **Pros**: Logical extension of presigner concept - **Cons**: Breaks current stateless presigner patterns -**Decision**: Option 1 provides clean separation while preserving SDK benefits and following established v2 utility patterns.cutePresignedGetObject(presignedRequest); +**Decision**: Option 1 provides clean separation while preserving SDK benefits and following established v2 utility patterns. -### Why doesn't PresignedUrlGetObjectRequest extend S3Request? +### What about synchronous S3Client and S3 CRT Client support? -While extending S3Request would provide access to RequestOverrideConfiguration, many of these configurations (like credentials provider, signers) are not supported with presigned URL execution. Instead, we use a standalone request with only essential parameters (presignedUrl, range). Internally, this gets wrapped in an encapsulated class that extends S3Request for use with ClientHandler. +The synchronous S3Client implementation has been deferred to future work due to complexities around multipart download requirements. Support for S3CrtAsyncClient will also be added in future work once the AWS CRT team addresses current limitations with pre-signed URL handling. + +### Why doesn't PresignedUrlDownloadRequest extend S3Request? +While extending S3Request would provide access to RequestOverrideConfiguration, many of these configurations (like credentials provider, signers) are not supported with presigned URL execution. Instead, we use a standalone request with only essential parameters (presignedUrl, range). Internally, this gets wrapped in an encapsulated class that extends S3Request for use with ClientHandler. ## References