-
Notifications
You must be signed in to change notification settings - Fork 917
Expose client-level context params on service client configuration #4834
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
Changes from 6 commits
3531dfd
f05ec6b
3285269
b64688b
e581fb3
1ffb8af
c0ebb64
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
{ | ||
"type": "feature", | ||
"category": "AWS SDK for Java v2", | ||
"contributor": "anirudh9391", | ||
"description": "Enabled consumption of custom client context params defined in customization.config and made them accessible and modifiable via service client configuration instances" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,6 +34,8 @@ | |
import java.util.ArrayList; | ||
import java.util.Collections; | ||
import java.util.List; | ||
import java.util.Map; | ||
import java.util.Objects; | ||
import java.util.concurrent.CompletableFuture; | ||
import java.util.stream.Collectors; | ||
import java.util.stream.Stream; | ||
|
@@ -47,6 +49,7 @@ | |
import software.amazon.awssdk.codegen.model.intermediate.IntermediateModel; | ||
import software.amazon.awssdk.codegen.model.intermediate.OperationModel; | ||
import software.amazon.awssdk.codegen.model.intermediate.Protocol; | ||
import software.amazon.awssdk.codegen.model.service.ClientContextParam; | ||
import software.amazon.awssdk.codegen.poet.PoetExtension; | ||
import software.amazon.awssdk.codegen.poet.PoetUtils; | ||
import software.amazon.awssdk.codegen.poet.auth.scheme.AuthSchemeSpecUtils; | ||
|
@@ -56,6 +59,7 @@ | |
import software.amazon.awssdk.codegen.poet.client.specs.QueryProtocolSpec; | ||
import software.amazon.awssdk.codegen.poet.client.specs.XmlProtocolSpec; | ||
import software.amazon.awssdk.codegen.poet.model.ServiceClientConfigurationUtils; | ||
import software.amazon.awssdk.codegen.poet.rules.EndpointRulesSpecUtils; | ||
import software.amazon.awssdk.core.RequestOverrideConfiguration; | ||
import software.amazon.awssdk.core.SdkPlugin; | ||
import software.amazon.awssdk.core.SdkRequest; | ||
|
@@ -69,8 +73,11 @@ | |
import software.amazon.awssdk.metrics.MetricCollector; | ||
import software.amazon.awssdk.metrics.MetricPublisher; | ||
import software.amazon.awssdk.metrics.NoOpMetricCollector; | ||
import software.amazon.awssdk.utils.AttributeMap; | ||
import software.amazon.awssdk.utils.CollectionUtils; | ||
import software.amazon.awssdk.utils.CompletableFutureUtils; | ||
import software.amazon.awssdk.utils.Logger; | ||
import software.amazon.awssdk.utils.Validate; | ||
|
||
public class SyncClientClass extends SyncClientInterface { | ||
|
||
|
@@ -418,7 +425,7 @@ protected MethodSpec.Builder waiterOperationBody(MethodSpec.Builder builder) { | |
poetExtensions.getSyncWaiterInterface()); | ||
} | ||
|
||
protected static MethodSpec updateSdkClientConfigurationMethod( | ||
protected MethodSpec updateSdkClientConfigurationMethod( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is to enable the method to access the model instance. I am unsure as to why a code gen method is static in the first place. |
||
TypeName serviceClientConfigurationBuilderClassName, | ||
boolean shouldAddClientReference) { | ||
MethodSpec.Builder builder = MethodSpec.methodBuilder("updateSdkClientConfiguration") | ||
|
@@ -442,9 +449,34 @@ protected static MethodSpec updateSdkClientConfigurationMethod( | |
.addStatement("$1T serviceConfigBuilder = new $1T(configuration)", serviceClientConfigurationBuilderClassName) | ||
.beginControlFlow("for ($T plugin : plugins)", SdkPlugin.class) | ||
.addStatement("plugin.configureClient(serviceConfigBuilder)") | ||
.endControlFlow() | ||
.addStatement("return configuration.build()"); | ||
.endControlFlow(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I only modified the Sync class and did not modify the Async class as it would suffice to incorporate the "request-level" modification check into the DefaultServiceClient Client |
||
EndpointRulesSpecUtils endpointRulesSpecUtils = new EndpointRulesSpecUtils(this.model); | ||
|
||
if (model.getCustomizationConfig() == null || | ||
CollectionUtils.isNullOrEmpty(model.getCustomizationConfig().getCustomClientContextParams())) { | ||
builder.addStatement("return configuration.build()"); | ||
return builder.build(); | ||
} | ||
|
||
Map<String, ClientContextParam> customClientConfigParams = model.getCustomizationConfig().getCustomClientContextParams(); | ||
|
||
builder.addCode("$1T newContextParams = configuration.option($2T.CLIENT_CONTEXT_PARAMS);\n" | ||
+ "$1T originalContextParams = clientConfiguration.option($2T.CLIENT_CONTEXT_PARAMS);", | ||
AttributeMap.class, SdkClientOption.class); | ||
|
||
builder.addCode("newContextParams = (newContextParams != null) ? newContextParams : $1T.empty();\n" | ||
+ "originalContextParams = originalContextParams != null ? originalContextParams : $1T.empty();", | ||
AttributeMap.class); | ||
|
||
customClientConfigParams.forEach((n, m) -> { | ||
String keyName = model.getNamingStrategy().getEnumValueName(n); | ||
builder.addStatement("$1T.validState($2T.equals(originalContextParams.get($3T.$4N), newContextParams.get($3T.$4N))," | ||
+ " $5S)", | ||
Validate.class, Objects.class, endpointRulesSpecUtils.clientContextParamsName(), keyName, | ||
keyName + " cannot be modified by request level plugins"); | ||
}); | ||
|
||
builder.addStatement("return configuration.build()"); | ||
return builder.build(); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: it's better for changelog entries, where possible, to talk in terms of functionality the customer understands. So, "allow SDK plugins to read and modify S3's crossRegionEnabled and SQS's whateverThatThingIsCalled" would be more useful for customers reading the changelog.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay this makes sense