Skip to content

Wraps s3 client in cross regional client when enabled #4080

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

Merged
Show file tree
Hide file tree
Changes from 1 commit
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 @@ -212,6 +212,14 @@ public class CustomizationConfig {
*/
private boolean delegateSyncClientClass;

/**
* Fully qualified name of a class that given the default client instance can return the final client instance,
* for instance by decorating the client with specific-purpose implementations of the client interface.
* The class should expose one async and one sync public static method that takes an instance of the client class
* and the client configuration and returns the same type for the client. See S3 customization.config for an example.
*/
private String clientComposerFactory;

/**
* Whether to skip generating endpoint tests from endpoint-tests.json
*/
Expand Down Expand Up @@ -561,6 +569,14 @@ public void setDelegateAsyncClientClass(boolean delegateAsyncClientClass) {
this.delegateAsyncClientClass = delegateAsyncClientClass;
}

public String getClientComposerFactory() {
return clientComposerFactory;
}

public void setClientComposerFactory(String clientComposerFactory) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add some validator here using regex to match the expected fqcn pattern and error out if it fails to match the supported package patters ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It doesn't seem like we have any checks for similar parameters (fqcn). What do you suggest would be allowed namespace patterns?

Copy link
Contributor

@joviegas joviegas Jun 13, 2023

Choose a reason for hiding this comment

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

We can do it in future something like this

        String pattern = "^(?:[a-zA-Z_$][a-zA-Z\\d_$]*\\.)+[a-zA-Z_$][a-zA-Z\\d_$]*$";

        Validate.true(Pattern.matches(pattern, fqcnSuppliedUser) , "Class name is not fqcn");

this.clientComposerFactory = clientComposerFactory;
}

public boolean isDelegateSyncClientClass() {
return delegateSyncClientClass;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,13 @@ public String getSdkResponseBaseClassName() {
}
}

public Optional<String> clientComposerClassName() {
if (customizationConfig.getClientComposerFactory() != null) {
return Optional.of(customizationConfig.getClientComposerFactory());
}
return Optional.empty();
}

public String getFileHeader() {
return FILE_HEADER;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,10 @@ public TypeSpec poetSpec() {
builder.addMethod(bearerTokenProviderMethod());
}

return builder.addMethod(buildClientMethod()).build();
builder.addMethod(buildClientMethod());
builder.addMethod(initializeServiceClientConfigMethod());

return builder.build();
}

private MethodSpec endpointDiscoveryEnabled() {
Expand Down Expand Up @@ -120,30 +123,26 @@ private MethodSpec endpointProviderMethod() {
}

private MethodSpec buildClientMethod() {
return MethodSpec.methodBuilder("buildClient")
.addAnnotation(Override.class)
.addModifiers(Modifier.PROTECTED, Modifier.FINAL)
.returns(clientInterfaceName)
.addStatement("$T clientConfiguration = super.asyncClientConfiguration()", SdkClientConfiguration.class)
.addStatement("this.validateClientOptions(clientConfiguration)")
.addStatement("$T endpointOverride = null", URI.class)
.addStatement("$T endpointProvider = clientConfiguration.option($T.ENDPOINT_PROVIDER)",
EndpointProvider.class,
SdkClientOption.class)
.addCode("if (clientConfiguration.option($T.ENDPOINT_OVERRIDDEN) != null"
+ "&& $T.TRUE.equals(clientConfiguration.option($T.ENDPOINT_OVERRIDDEN))) {"
+ "endpointOverride = clientConfiguration.option($T.ENDPOINT);"
+ "}",
SdkClientOption.class, Boolean.class, SdkClientOption.class, SdkClientOption.class)
.addStatement("$T serviceClientConfiguration = $T.builder()"
+ ".overrideConfiguration(overrideConfiguration())"
+ ".region(clientConfiguration.option($T.AWS_REGION))"
+ ".endpointOverride(endpointOverride)"
+ ".endpointProvider(endpointProvider)"
+ ".build()",
serviceConfigClassName, serviceConfigClassName, AwsClientOption.class)
.addStatement("return new $T(serviceClientConfiguration, clientConfiguration)", clientClassName)
.build();
MethodSpec.Builder builder = MethodSpec.methodBuilder("buildClient")
.addAnnotation(Override.class)
.addModifiers(Modifier.PROTECTED, Modifier.FINAL)
.returns(clientInterfaceName)
.addStatement("$T clientConfiguration = super.asyncClientConfiguration()",
SdkClientConfiguration.class)
.addStatement("this.validateClientOptions(clientConfiguration)")
.addStatement("$T serviceClientConfiguration = initializeServiceClientConfig"
+ "(clientConfiguration)",
serviceConfigClassName);

builder.addStatement("$1T client = new $2T(serviceClientConfiguration, clientConfiguration)",
clientInterfaceName, clientClassName);
if (model.clientComposerClassName().isPresent()) {
builder.addStatement("return $T.composeAsync(client, clientConfiguration)",
PoetUtils.classNameFromFqcn(model.clientComposerClassName().get()));
} else {
builder.addStatement("return client");
}
return builder.build();
}

private MethodSpec bearerTokenProviderMethod() {
Expand All @@ -157,6 +156,29 @@ private MethodSpec bearerTokenProviderMethod() {
.build();
}

private MethodSpec initializeServiceClientConfigMethod() {
return MethodSpec.methodBuilder("initializeServiceClientConfig").addModifiers(Modifier.PRIVATE)
.addParameter(SdkClientConfiguration.class, "clientConfig")
.returns(serviceConfigClassName)
.addStatement("$T endpointOverride = null", URI.class)
.addStatement("$T endpointProvider = clientConfig.option($T.ENDPOINT_PROVIDER)",
EndpointProvider.class,
SdkClientOption.class)
.addCode("if (clientConfig.option($T.ENDPOINT_OVERRIDDEN) != null"
+ "&& $T.TRUE.equals(clientConfig.option($T.ENDPOINT_OVERRIDDEN))) {"
+ "endpointOverride = clientConfig.option($T.ENDPOINT);"
+ "}",
SdkClientOption.class, Boolean.class, SdkClientOption.class, SdkClientOption.class)
.addStatement("return $T.builder()"
+ ".overrideConfiguration(overrideConfiguration())"
+ ".region(clientConfig.option($T.AWS_REGION))"
+ ".endpointOverride(endpointOverride)"
+ ".endpointProvider(endpointProvider)"
+ ".build()",
serviceConfigClassName, AwsClientOption.class)
.build();
}

@Override
public ClassName className() {
return builderClassName;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,10 @@ public TypeSpec poetSpec() {
builder.addMethod(tokenProviderMethodImpl());
}

return builder.addMethod(buildClientMethod()).build();
builder.addMethod(buildClientMethod());
builder.addMethod(initializeServiceClientConfigMethod());

return builder.build();
}

private MethodSpec endpointDiscoveryEnabled() {
Expand Down Expand Up @@ -120,29 +123,26 @@ private MethodSpec endpointProviderMethod() {


private MethodSpec buildClientMethod() {
return MethodSpec.methodBuilder("buildClient")
.addAnnotation(Override.class)
.addModifiers(Modifier.PROTECTED, Modifier.FINAL)
.returns(clientInterfaceName)
.addStatement("$T clientConfiguration = super.syncClientConfiguration()", SdkClientConfiguration.class)
.addStatement("this.validateClientOptions(clientConfiguration)")
.addStatement("$T endpointOverride = null", URI.class)
.addStatement("$T endpointProvider = clientConfiguration.option($T.ENDPOINT_PROVIDER)",
EndpointProvider.class, SdkClientOption.class)
.addCode("if (clientConfiguration.option($T.ENDPOINT_OVERRIDDEN) != null"
+ "&& $T.TRUE.equals(clientConfiguration.option($T.ENDPOINT_OVERRIDDEN))) {"
+ "endpointOverride = clientConfiguration.option($T.ENDPOINT);"
+ "}",
SdkClientOption.class, Boolean.class, SdkClientOption.class, SdkClientOption.class)
.addStatement("$T serviceClientConfiguration = $T.builder()"
+ ".overrideConfiguration(overrideConfiguration())"
+ ".region(clientConfiguration.option($T.AWS_REGION))"
+ ".endpointOverride(endpointOverride)"
+ ".endpointProvider(endpointProvider)"
+ ".build()",
serviceConfigClassName, serviceConfigClassName, AwsClientOption.class)
.addStatement("return new $T(serviceClientConfiguration, clientConfiguration)", clientClassName)
.build();
MethodSpec.Builder builder = MethodSpec.methodBuilder("buildClient")
.addAnnotation(Override.class)
.addModifiers(Modifier.PROTECTED, Modifier.FINAL)
.returns(clientInterfaceName)
.addStatement("$T clientConfiguration = super.syncClientConfiguration()",
SdkClientConfiguration.class)
.addStatement("this.validateClientOptions(clientConfiguration)")
.addStatement("$T serviceClientConfiguration = initializeServiceClientConfig"
+ "(clientConfiguration)",
serviceConfigClassName);

builder.addStatement("$1T client = new $2T(serviceClientConfiguration, clientConfiguration)",
clientInterfaceName, clientClassName);
if (model.clientComposerClassName().isPresent()) {
builder.addStatement("return $T.composeSync(client, clientConfiguration)",
PoetUtils.classNameFromFqcn(model.clientComposerClassName().get()));
} else {
builder.addStatement("return client");
}
return builder.build();
}

private MethodSpec tokenProviderMethodImpl() {
Expand All @@ -156,6 +156,29 @@ private MethodSpec tokenProviderMethodImpl() {
.build();
}

private MethodSpec initializeServiceClientConfigMethod() {
return MethodSpec.methodBuilder("initializeServiceClientConfig").addModifiers(Modifier.PRIVATE)
.addParameter(SdkClientConfiguration.class, "clientConfig")
.returns(serviceConfigClassName)
.addStatement("$T endpointOverride = null", URI.class)
.addStatement("$T endpointProvider = clientConfig.option($T.ENDPOINT_PROVIDER)",
EndpointProvider.class,
SdkClientOption.class)
.addCode("if (clientConfig.option($T.ENDPOINT_OVERRIDDEN) != null"
+ "&& $T.TRUE.equals(clientConfig.option($T.ENDPOINT_OVERRIDDEN))) {"
+ "endpointOverride = clientConfig.option($T.ENDPOINT);"
+ "}",
SdkClientOption.class, Boolean.class, SdkClientOption.class, SdkClientOption.class)
.addStatement("return $T.builder()"
+ ".overrideConfiguration(overrideConfiguration())"
+ ".region(clientConfig.option($T.AWS_REGION))"
+ ".endpointOverride(endpointOverride)"
+ ".endpointProvider(endpointProvider)"
+ ".build()",
serviceConfigClassName, AwsClientOption.class)
.build();
}

@Override
public ClassName className() {
return builderClassName;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ protected MethodSpec.Builder operationBody(MethodSpec.Builder builder, Operation
builder.addModifiers(PUBLIC)
.addAnnotation(Override.class);

if (builder.parameters.size() < 1) {
if (builder.parameters.isEmpty()) {
throw new IllegalStateException("All client methods must have an argument");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ protected MethodSpec.Builder operationBody(MethodSpec.Builder builder, Operation
builder.addModifiers(PUBLIC)
.addAnnotation(Override.class);

if (builder.parameters.size() < 1) {
if (builder.parameters.isEmpty()) {
throw new IllegalStateException("All client methods must have an argument");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,19 @@ public static IntermediateModel customContentTypeModels() {
return new IntermediateModelBuilder(models).build();
}

public static IntermediateModel composedClientJsonServiceModels() {
File serviceModel = new File(ClientTestModels.class.getResource("client/c2j/rest-json/service-2.json").getFile());
File customizationModel =
new File(ClientTestModels.class.getResource("client/c2j/composedclient/customization.config").getFile());
CustomizationConfig customizationConfig = getCustomizationConfig(customizationModel);
C2jModels models = C2jModels.builder()
.serviceModel(getServiceModel(serviceModel))
.customizationConfig(customizationConfig)
.build();

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

public static IntermediateModel internalConfigModels() {
File serviceModel = new File(ClientTestModels.class.getResource("client/c2j/internalconfig/service-2.json").getFile());
File customizationModel = new File(ClientTestModels.class.getResource("client/c2j/internalconfig/customization.config").getFile());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,11 @@ public void syncClientBuilderClass() throws Exception {
validateGeneration(SyncClientBuilderClass::new, "test-sync-client-builder-class.java");
}

@Test
public void syncComposedClientBuilderClass() throws Exception {
validateComposedClientGeneration(SyncClientBuilderClass::new, "test-composed-sync-client-builder-class.java");
}

@Test
public void asyncClientBuilderInterface() throws Exception {
validateGeneration(AsyncClientBuilderInterface::new, "test-async-client-builder-interface.java");
Expand All @@ -78,10 +83,20 @@ public void asyncClientBuilderClass() throws Exception {
validateGeneration(AsyncClientBuilderClass::new, "test-async-client-builder-class.java");
}

@Test
public void asyncComposedClientBuilderClass() throws Exception {
validateComposedClientGeneration(AsyncClientBuilderClass::new, "test-composed-async-client-builder-class.java");
}

private void validateGeneration(Function<IntermediateModel, ClassSpec> generatorConstructor, String expectedClassName) {
assertThat(generatorConstructor.apply(ClientTestModels.restJsonServiceModels()), generatesTo(expectedClassName));
}

private void validateComposedClientGeneration(Function<IntermediateModel, ClassSpec> generatorConstructor,
String expectedClassName) {
assertThat(generatorConstructor.apply(ClientTestModels.composedClientJsonServiceModels()), generatesTo(expectedClassName));
}

private void validateBearerAuthGeneration(Function<IntermediateModel, ClassSpec> generatorConstructor,
String expectedClassName) {
assertThat(generatorConstructor.apply(ClientTestModels.bearerAuthServiceModels()), generatesTo(expectedClassName));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
public class UserAgentClassSpecTest {

@Test
public void testGeneratedResponseForSyncOperations() {
void testUserAgentClass() {
ClassSpec useragentspec = new UserAgentUtilsSpec(ClientTestModels.restJsonServiceModels());
assertThat(useragentspec, generatesTo("test-user-agent-class.java"));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,20 @@ public DefaultJsonAsyncClientBuilder tokenProvider(SdkTokenProvider tokenProvide
protected final JsonAsyncClient buildClient() {
SdkClientConfiguration clientConfiguration = super.asyncClientConfiguration();
this.validateClientOptions(clientConfiguration);
JsonServiceClientConfiguration serviceClientConfiguration = initializeServiceClientConfig(clientConfiguration);
JsonAsyncClient client = new DefaultJsonAsyncClient(serviceClientConfiguration, clientConfiguration);
return client;
}

private JsonServiceClientConfiguration initializeServiceClientConfig(SdkClientConfiguration clientConfig) {
URI endpointOverride = null;
EndpointProvider endpointProvider = clientConfiguration.option(SdkClientOption.ENDPOINT_PROVIDER);
if (clientConfiguration.option(SdkClientOption.ENDPOINT_OVERRIDDEN) != null
&& Boolean.TRUE.equals(clientConfiguration.option(SdkClientOption.ENDPOINT_OVERRIDDEN))) {
endpointOverride = clientConfiguration.option(SdkClientOption.ENDPOINT);
EndpointProvider endpointProvider = clientConfig.option(SdkClientOption.ENDPOINT_PROVIDER);
if (clientConfig.option(SdkClientOption.ENDPOINT_OVERRIDDEN) != null
&& Boolean.TRUE.equals(clientConfig.option(SdkClientOption.ENDPOINT_OVERRIDDEN))) {
endpointOverride = clientConfig.option(SdkClientOption.ENDPOINT);
}
JsonServiceClientConfiguration serviceClientConfiguration = JsonServiceClientConfiguration.builder()
.overrideConfiguration(overrideConfiguration()).region(clientConfiguration.option(AwsClientOption.AWS_REGION))
.endpointOverride(endpointOverride).endpointProvider(endpointProvider).build();
return new DefaultJsonAsyncClient(serviceClientConfiguration, clientConfiguration);
return JsonServiceClientConfiguration.builder().overrideConfiguration(overrideConfiguration())
.region(clientConfig.option(AwsClientOption.AWS_REGION)).endpointOverride(endpointOverride)
.endpointProvider(endpointProvider).build();
}
}
Loading