Skip to content

Add PresignedUrlManager support to sync S3 client codegen #6249

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ public static Metadata constructMetadata(ServiceModel serviceModel,
.withDocumentation(serviceModel.getDocumentation())
.withServiceAbbreviation(serviceMetadata.getServiceAbbreviation())
.withBatchmanagerPackageName(namingStrategy.getBatchManagerPackageName(serviceName))
.withPresignedUrlManagerPackageName(namingStrategy.getPresignedUrlManagerPackageName(serviceName))
.withServiceFullName(serviceMetadata.getServiceFullName())
.withServiceName(serviceName)
.withSyncClient(String.format(Constant.SYNC_CLIENT_CLASS_NAME_PATTERN, serviceName))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ public final class Constant {

public static final String PACKAGE_NAME_BATCHMANAGER_PATTERN = "%s.batchmanager";

public static final String PACKAGE_NAME_PRESIGNED_URL_MANAGER_PATTERN = "%s.presignedurl";

public static final String PACKAGE_NAME_CUSTOM_AUTH_PATTERN = "%s.auth";

public static final String AUTH_POLICY_ENUM_CLASS_DIR = "software/amazon/awssdk/auth/policy/actions";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,11 @@ public class CustomizationConfig {
*/
private boolean batchManagerSupported;

/**
* A boolean flag to indicate if Presigned URL Manager is supported.
*/
private boolean presignedUrlManagerSupported;

/**
* A boolean flag to indicate if the fast unmarshaller code path is enabled.
*/
Expand Down Expand Up @@ -924,6 +929,14 @@ public void setBatchManagerSupported(boolean batchManagerSupported) {
this.batchManagerSupported = batchManagerSupported;
}

public boolean getPresignedUrlManagerSupported() {
return presignedUrlManagerSupported;
}

public void setPresignedUrlManagerSupported(boolean presignedUrlManagerSupported) {
this.presignedUrlManagerSupported = presignedUrlManagerSupported;
}

public boolean getEnableFastUnmarshaller() {
return enableFastUnmarshaller;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ public class Metadata {

private String batchManagerPackageName;

private String presignedUrlManagerPackageName;

private String endpointRulesPackageName;

private String authSchemePackageName;
Expand Down Expand Up @@ -815,4 +817,21 @@ public String getFullBatchManagerPackageName() {
return joinPackageNames(rootPackageName, getBatchManagerPackageName());
}

public String getPresignedUrlManagerPackageName() {
return presignedUrlManagerPackageName;
}

public void setPresignedUrlManagerPackageName(String presignedUrlManagerPackageName) {
this.presignedUrlManagerPackageName = presignedUrlManagerPackageName;
}

public Metadata withPresignedUrlManagerPackageName(String presignedUrlManagerPackageName) {
setPresignedUrlManagerPackageName(presignedUrlManagerPackageName);
return this;
}

public String getFullPresignedUrlPackageName() {
return joinPackageNames(rootPackageName, getPresignedUrlManagerPackageName());
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,12 @@ public String getBatchManagerPackageName(String serviceName) {
return getCustomizedPackageName(concatServiceNameIfShareModel(serviceName), Constant.PACKAGE_NAME_BATCHMANAGER_PATTERN);
}

@Override
public String getPresignedUrlManagerPackageName(String serviceName) {
return getCustomizedPackageName(concatServiceNameIfShareModel(serviceName),
Constant.PACKAGE_NAME_PRESIGNED_URL_MANAGER_PATTERN);
}

@Override
public String getSmokeTestPackageName(String serviceName) {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,11 @@ public interface NamingStrategy {
*/
String getBatchManagerPackageName(String serviceName);

/**
* Retrieve the presignedUrlManager package name that should be used based on the service name.
*/
String getPresignedUrlManagerPackageName(String serviceName);

/**
* Retrieve the smote test package name that should be used based on the service name.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -209,4 +209,14 @@ public ClassName getBatchManagerAsyncInterface() {
return ClassName.get(model.getMetadata().getFullBatchManagerPackageName(),
model.getMetadata().getServiceName() + "AsyncBatchManager");
}

public ClassName getPresignedUrlManagerInterface() {
return ClassName.get(model.getMetadata().getFullPresignedUrlPackageName(),
"PresignedUrlManager");
}

public ClassName getDefaultPresignedUrlManagerClass() {
return ClassName.get(model.getMetadata().getFullClientInternalPackageName() + ".presignedurl",
"DefaultPresignedUrlManager");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -232,4 +232,20 @@ protected MethodSpec serviceClientConfigMethod() {
.addStatement("return delegate.serviceClientConfiguration()")
.build();
}

@Override
protected void addPresignedUrlManagerMethod(TypeSpec.Builder type) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought we are only going to generate this in async client, is that not the case?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The delegating client needs the presignedUrlManager implementation because DelegatingS3Client implements the S3Client interface - without it, we get a compilation error "CLASS DelegatingS3Client.presignedUrlManager cannot be default" since interface methods can't be default methods in this case, so we need to maintain the delegation pattern by forwarding the call to the delegate.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, we are implementing presignedUrlManager support for both sync and async S3 clients. While the initial discussion might have focused on async clients, the final design has both sync and async implementations.

I thought we are only going to generate this in async client, is that not the case?

Copy link
Contributor

@joviegas joviegas Jul 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Presigned URLs in S3 are not exclusively limited to multipart upload scenarios . They serve a broader range of use cases that extend beyond just multipart operations.

Presigned URLs can be generated for various S3 operations, including single-part file uploads and downloads. This flexibility is particularly valuable because many file operations don't require the complexity of multipart handling, especially for smaller files or straightforward download scenarios.

The decision to make this functionality available in the synchronous client addresses a practical developer need. By providing presigned URL generation in the sync client, developers who are already using synchronous operations can access this feature without introducing additional dependencies. This approach eliminates the need to add Netty dependencies or maintain a separate async client solely for presigned URL operations, which would add unnecessary complexity to their application architecture.

This implementation strategy provides developers with consistent access to presigned URL functionality regardless of their chosen client type, making the feature more accessible and reducing the overhead of managing multiple client configurations.

if (model.getCustomizationConfig().getPresignedUrlManagerSupported()) {
ClassName returnType = poetExtensions.getPresignedUrlManagerInterface();
MethodSpec method = MethodSpec.methodBuilder("presignedUrlManager")
.addAnnotation(Override.class)
.addModifiers(PUBLIC)
.returns(returnType)
.addJavadoc("Creates an instance of {@link $T} object with the "
+ "configuration set on this client.", returnType)
.addStatement("return delegate.presignedUrlManager()")
.build();
type.addMethod(method);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -513,4 +513,12 @@ protected MethodSpec updateSdkClientConfigurationMethod(
builder.addStatement("return configuration.build()");
return builder.build();
}

@Override
protected MethodSpec.Builder presignedUrlManagerOperationBody(MethodSpec.Builder builder) {
ClassName returnType = poetExtensions.getDefaultPresignedUrlManagerClass();
return builder.addAnnotation(Override.class)
.addStatement("return new $T(clientHandler, $N, clientConfiguration, protocolMetadata)",
returnType, protocolSpec.protocolFactory(model).name);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,9 @@ public final TypeSpec poetSpec() {
if (model.hasWaiters()) {
result.addMethod(waiterMethod());
}
if (model.getCustomizationConfig().getPresignedUrlManagerSupported()) {
addPresignedUrlManagerMethod(result);
}
addAdditionalMethods(result);
result.addMethod(serviceClientConfigMethod());
addCloseMethod(result);
Expand Down Expand Up @@ -145,6 +148,16 @@ protected void addAdditionalMethods(TypeSpec.Builder type) {

}

protected void addPresignedUrlManagerMethod(TypeSpec.Builder type) {
ClassName returnType = poetExtensions.getPresignedUrlManagerInterface();
MethodSpec.Builder builder = MethodSpec.methodBuilder("presignedUrlManager")
.addModifiers(PUBLIC)
.returns(returnType)
.addJavadoc("Creates an instance of {@link $T} object with the "
+ "configuration set on this client.", returnType);
type.addMethod(presignedUrlManagerOperationBody(builder).build());
}

@Override
public ClassName className() {
return className;
Expand Down Expand Up @@ -559,4 +572,9 @@ protected MethodSpec.Builder waiterOperationBody(MethodSpec.Builder builder) {
return builder.addModifiers(DEFAULT, PUBLIC)
.addStatement("throw new $T()", UnsupportedOperationException.class);
}

protected MethodSpec.Builder presignedUrlManagerOperationBody(MethodSpec.Builder builder) {
return builder.addModifiers(DEFAULT)
.addStatement("throw new $T()", UnsupportedOperationException.class);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -526,6 +526,18 @@ public static IntermediateModel batchManagerModels() {
return new IntermediateModelBuilder(models).build();
}

public static IntermediateModel presignedUrlModels() {
File serviceModel = new File(ClientTestModels.class.getResource("client/c2j/query/service-2.json").getFile());
File customizationModel = new File(ClientTestModels.class.getResource("client/c2j/presignedurl/customization.config").getFile());

C2jModels models = C2jModels.builder()
.serviceModel(getServiceModel(serviceModel))
.customizationConfig(getCustomizationConfig(customizationModel))
.build();

return new IntermediateModelBuilder(models).build();
}

private static ServiceModel getServiceModel(File file) {
return ModelLoaderUtils.loadModel(ServiceModel.class, file);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import static org.hamcrest.MatcherAssert.assertThat;
import static software.amazon.awssdk.codegen.poet.ClientTestModels.restJsonServiceModels;
import static software.amazon.awssdk.codegen.poet.ClientTestModels.presignedUrlModels;
import static software.amazon.awssdk.codegen.poet.PoetMatchers.generatesTo;

import org.junit.Test;
Expand All @@ -28,4 +29,11 @@ public void delegatingSyncClientClass() {
new DelegatingSyncClientClass(restJsonServiceModels());
assertThat(syncClientDecoratorAbstractClass, generatesTo("test-abstract-sync-client-class.java"));
}

@Test
public void delegatingSyncClientClassWithPresignedUrlManager() {
DelegatingSyncClientClass syncClientDecoratorAbstractClass =
new DelegatingSyncClientClass(presignedUrlModels());
assertThat(syncClientDecoratorAbstractClass, generatesTo("test-abstract-sync-client-class-presignedurl.java"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import static software.amazon.awssdk.codegen.poet.ClientTestModels.restJsonServiceModels;
import static software.amazon.awssdk.codegen.poet.ClientTestModels.rpcv2ServiceModels;
import static software.amazon.awssdk.codegen.poet.ClientTestModels.xmlServiceModels;
import static software.amazon.awssdk.codegen.poet.ClientTestModels.presignedUrlModels;
import static software.amazon.awssdk.codegen.poet.PoetMatchers.generatesTo;

import org.junit.Test;
Expand Down Expand Up @@ -112,4 +113,10 @@ public void syncClientClassWithUnsignedPayload() {
assertThat(syncClientClass, generatesTo("test-unsigned-payload-trait-sync-client-class.java"));
}

@Test
public void syncClientPresignedUrlManager() {
ClassSpec syncClientPresignedUrl = createSyncClientClass(presignedUrlModels());
assertThat(syncClientPresignedUrl, generatesTo("test-presignedurl-sync.java"));
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import static org.hamcrest.MatcherAssert.assertThat;
import static software.amazon.awssdk.codegen.poet.ClientTestModels.restJsonServiceModels;
import static software.amazon.awssdk.codegen.poet.ClientTestModels.presignedUrlModels;
import static software.amazon.awssdk.codegen.poet.PoetMatchers.generatesTo;

import org.junit.Test;
Expand All @@ -28,4 +29,10 @@ public void syncClientInterface() {
ClassSpec syncClientInterface = new SyncClientInterface(restJsonServiceModels());
assertThat(syncClientInterface, generatesTo("test-json-client-interface.java"));
}

@Test
public void syncClientInterfaceWithPresignedUrlManager() {
ClassSpec syncClientInterface = new SyncClientInterface(presignedUrlModels());
assertThat(syncClientInterface, generatesTo("test-json-sync-client-interface-presignedurl.java"));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"presignedUrlManagerSupported": true
}
Loading