Skip to content

Commit 0b85357

Browse files
sugmanuesugmanue
authored
Fix a bug that prevents a retry strategy override to get used (#5300)
* Scenarios that we need to support when using RetryMode * Use SdkClientOption and lazily set the strategies at build time * Update client tests for the new updateRetryStrategyClientConfiguration method * Create a new change entry for the PR * Fix checkstyle and japicmp issues * Add dependency to retries-spi for all services * Change to client builder properly set retry strategy from plugins * Remove added files by mistake * Add missing dependency to retries-spi module * Add test cases for plugins * Move the retries-spi dependencies to services pom.xml * Update javadocs in an attempt to make them more clear * Update the javadocs * Remove a comment that might be confusing * Put back the original link for the deprecated tag * Address PR comments * Address PR comments
1 parent 4711910 commit 0b85357

File tree

52 files changed

+5293
-3707
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

52 files changed

+5293
-3707
lines changed

‎.changes/next-release/bugfix-AWSSDKforJavav2-5a6324d.json

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"category": "AWS SDK for Java v2",
3+
"contributor": "",
4+
"type": "bugfix",
5+
"description": "Fix a bug that prevented users from overriding retry strategies"
6+
}

‎codegen/pom.xml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,11 @@
9797
<artifactId>http-auth-aws</artifactId>
9898
<version>${awsjavasdk.version}</version>
9999
</dependency>
100+
<dependency>
101+
<groupId>software.amazon.awssdk</groupId>
102+
<artifactId>retries-spi</artifactId>
103+
<version>${awsjavasdk.version}</version>
104+
</dependency>
100105
<dependency>
101106
<groupId>software.amazon.awssdk</groupId>
102107
<artifactId>arns</artifactId>

‎codegen/src/main/java/software/amazon/awssdk/codegen/poet/builder/BaseClientBuilderClass.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@
5252
import software.amazon.awssdk.codegen.poet.PoetUtils;
5353
import software.amazon.awssdk.codegen.poet.auth.scheme.AuthSchemeSpecUtils;
5454
import software.amazon.awssdk.codegen.poet.auth.scheme.ModelAuthSchemeClassesKnowledgeIndex;
55+
import software.amazon.awssdk.codegen.poet.client.ClientClassUtils;
5556
import software.amazon.awssdk.codegen.poet.model.ServiceClientConfigurationUtils;
5657
import software.amazon.awssdk.codegen.poet.rules.EndpointParamsKnowledgeIndex;
5758
import software.amazon.awssdk.codegen.poet.rules.EndpointRulesSpecUtils;
@@ -180,6 +181,7 @@ public TypeSpec poetSpec() {
180181
}
181182
addServiceHttpConfigIfNeeded(builder, model);
182183
builder.addMethod(invokePluginsMethod());
184+
builder.addMethod(ClientClassUtils.updateRetryStrategyClientConfigurationMethod());
183185
builder.addMethod(internalPluginsMethod());
184186

185187
endpointParamsKnowledgeIndex.resolveAccountIdEndpointModeMethod().ifPresent(builder::addMethod);
@@ -783,6 +785,7 @@ private MethodSpec invokePluginsMethod() {
783785
.beginControlFlow("for ($T plugin : plugins)", SdkPlugin.class)
784786
.addStatement("plugin.configureClient(serviceConfigBuilder)")
785787
.endControlFlow()
788+
.addStatement("updateRetryStrategyClientConfiguration(configuration)")
786789
.addStatement("return configuration.build()");
787790
return builder.build();
788791
}

‎codegen/src/main/java/software/amazon/awssdk/codegen/poet/client/AsyncClientClass.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,7 @@ protected void addAdditionalMethods(TypeSpec.Builder type) {
173173
type.addMethod(isSignerOverriddenOnClientMethod());
174174
}
175175
}
176+
type.addMethod(ClientClassUtils.updateRetryStrategyClientConfigurationMethod());
176177
type.addMethod(updateSdkClientConfigurationMethod(configurationUtils.serviceClientConfigurationBuilderClassName()));
177178
protocolSpec.createErrorResponseHandler().ifPresent(type::addMethod);
178179
}
@@ -313,8 +314,9 @@ protected static MethodSpec updateSdkClientConfigurationMethod(
313314
.addStatement("$1T serviceConfigBuilder = new $1T(configuration)", serviceClientConfigurationBuilderClassName)
314315
.beginControlFlow("for ($T plugin : plugins)", SdkPlugin.class)
315316
.addStatement("plugin.configureClient(serviceConfigBuilder)")
316-
.endControlFlow()
317-
.addStatement("return configuration.build()");
317+
.endControlFlow();
318+
builder.addStatement("updateRetryStrategyClientConfiguration(configuration)");
319+
builder.addStatement("return configuration.build()");
318320

319321
return builder.build();
320322
}

‎codegen/src/main/java/software/amazon/awssdk/codegen/poet/client/ClientClassUtils.java

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import software.amazon.awssdk.arns.Arn;
3333
import software.amazon.awssdk.auth.signer.EventStreamAws4Signer;
3434
import software.amazon.awssdk.awscore.AwsRequestOverrideConfiguration;
35+
import software.amazon.awssdk.awscore.retry.AwsRetryStrategy;
3536
import software.amazon.awssdk.codegen.model.config.customization.S3ArnableFieldConfig;
3637
import software.amazon.awssdk.codegen.model.intermediate.IntermediateModel;
3738
import software.amazon.awssdk.codegen.model.intermediate.MemberModel;
@@ -40,12 +41,17 @@
4041
import software.amazon.awssdk.codegen.model.service.HostPrefixProcessor;
4142
import software.amazon.awssdk.codegen.poet.PoetExtension;
4243
import software.amazon.awssdk.codegen.poet.PoetUtils;
44+
import software.amazon.awssdk.core.client.config.ClientOverrideConfiguration;
45+
import software.amazon.awssdk.core.client.config.SdkClientConfiguration;
46+
import software.amazon.awssdk.core.client.config.SdkClientOption;
47+
import software.amazon.awssdk.core.retry.RetryMode;
4348
import software.amazon.awssdk.core.signer.Signer;
49+
import software.amazon.awssdk.retries.api.RetryStrategy;
4450
import software.amazon.awssdk.utils.HostnameValidator;
4551
import software.amazon.awssdk.utils.StringUtils;
4652
import software.amazon.awssdk.utils.Validate;
4753

48-
final class ClientClassUtils {
54+
public final class ClientClassUtils {
4955

5056
private ClientClassUtils() {
5157
}
@@ -243,4 +249,32 @@ private static String inputShapeMemberGetter(OperationModel opModel, String c2jN
243249
return opModel.getInput().getVariableName() + "." +
244250
opModel.getInputShape().getMemberByC2jName(c2jName).getFluentGetterMethodName() + "()";
245251
}
252+
253+
public static MethodSpec updateRetryStrategyClientConfigurationMethod() {
254+
MethodSpec.Builder builder = MethodSpec.methodBuilder("updateRetryStrategyClientConfiguration")
255+
.addModifiers(Modifier.PRIVATE)
256+
.addParameter(SdkClientConfiguration.Builder.class, "configuration");
257+
builder.addStatement("$T builder = configuration.asOverrideConfigurationBuilder()",
258+
ClientOverrideConfiguration.Builder.class);
259+
builder.addStatement("$T retryMode = builder.retryMode()", RetryMode.class);
260+
builder.beginControlFlow("if (retryMode != null)")
261+
.addStatement("configuration.option($T.RETRY_STRATEGY, $T.forRetryMode(retryMode))", SdkClientOption.class,
262+
AwsRetryStrategy.class)
263+
.addStatement("return")
264+
.endControlFlow();
265+
builder.addStatement("$T<$T<?, ?>> configurator = builder.retryStrategyConfigurator()", Consumer.class,
266+
RetryStrategy.Builder.class);
267+
builder.beginControlFlow("if (configurator != null)")
268+
.addStatement("$T<?, ?> defaultBuilder = $T.defaultRetryStrategy().toBuilder()", RetryStrategy.Builder.class,
269+
AwsRetryStrategy.class)
270+
.addStatement("configurator.accept(defaultBuilder)")
271+
.addStatement("configuration.option($T.RETRY_STRATEGY, defaultBuilder.build())", SdkClientOption.class)
272+
.addStatement("return")
273+
.endControlFlow();
274+
builder.addStatement("$T retryStrategy = builder.retryStrategy()", RetryStrategy.class);
275+
builder.beginControlFlow("if (retryStrategy != null)")
276+
.addStatement("configuration.option($T.RETRY_STRATEGY, retryStrategy)", SdkClientOption.class)
277+
.endControlFlow();
278+
return builder.build();
279+
}
246280
}

‎codegen/src/main/java/software/amazon/awssdk/codegen/poet/client/SyncClientClass.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,7 @@ protected void addAdditionalMethods(TypeSpec.Builder type) {
152152
.addMethod(resolveMetricPublishersMethod());
153153

154154
protocolSpec.createErrorResponseHandler().ifPresent(type::addMethod);
155+
type.addMethod(ClientClassUtils.updateRetryStrategyClientConfigurationMethod());
155156
type.addMethod(updateSdkClientConfigurationMethod(configurationUtils.serviceClientConfigurationBuilderClassName()));
156157
type.addMethod(protocolSpec.initProtocolFactory(model));
157158
}
@@ -478,6 +479,7 @@ protected MethodSpec updateSdkClientConfigurationMethod(
478479

479480
if (model.getCustomizationConfig() == null ||
480481
CollectionUtils.isNullOrEmpty(model.getCustomizationConfig().getCustomClientContextParams())) {
482+
builder.addStatement("updateRetryStrategyClientConfiguration(configuration)");
481483
builder.addStatement("return configuration.build()");
482484
return builder.build();
483485
}
@@ -499,7 +501,7 @@ protected MethodSpec updateSdkClientConfigurationMethod(
499501
Validate.class, Objects.class, endpointRulesSpecUtils.clientContextParamsName(), keyName,
500502
keyName + " cannot be modified by request level plugins");
501503
});
502-
504+
builder.addStatement("updateRetryStrategyClientConfiguration(configuration)");
503505
builder.addStatement("return configuration.build()");
504506
return builder.build();
505507
}

‎codegen/src/test/resources/software/amazon/awssdk/codegen/poet/builder/sra/test-bearer-auth-client-builder-class.java

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,23 +5,28 @@
55
import java.util.HashMap;
66
import java.util.List;
77
import java.util.Map;
8+
import java.util.function.Consumer;
89
import software.amazon.awssdk.annotations.Generated;
910
import software.amazon.awssdk.annotations.SdkInternalApi;
1011
import software.amazon.awssdk.auth.credentials.TokenUtils;
1112
import software.amazon.awssdk.auth.token.credentials.aws.DefaultAwsTokenProvider;
1213
import software.amazon.awssdk.awscore.client.builder.AwsDefaultClientBuilder;
1314
import software.amazon.awssdk.awscore.client.config.AwsClientOption;
15+
import software.amazon.awssdk.awscore.retry.AwsRetryStrategy;
1416
import software.amazon.awssdk.core.SdkPlugin;
17+
import software.amazon.awssdk.core.client.config.ClientOverrideConfiguration;
1518
import software.amazon.awssdk.core.client.config.SdkClientConfiguration;
1619
import software.amazon.awssdk.core.client.config.SdkClientOption;
1720
import software.amazon.awssdk.core.interceptor.ClasspathInterceptorChainFactory;
1821
import software.amazon.awssdk.core.interceptor.ExecutionInterceptor;
22+
import software.amazon.awssdk.core.retry.RetryMode;
1923
import software.amazon.awssdk.http.auth.scheme.BearerAuthScheme;
2024
import software.amazon.awssdk.http.auth.scheme.NoAuthAuthScheme;
2125
import software.amazon.awssdk.http.auth.spi.scheme.AuthScheme;
2226
import software.amazon.awssdk.identity.spi.IdentityProvider;
2327
import software.amazon.awssdk.identity.spi.IdentityProviders;
2428
import software.amazon.awssdk.identity.spi.TokenIdentity;
29+
import software.amazon.awssdk.retries.api.RetryStrategy;
2530
import software.amazon.awssdk.services.json.auth.scheme.JsonAuthSchemeProvider;
2631
import software.amazon.awssdk.services.json.auth.scheme.internal.JsonAuthSchemeInterceptor;
2732
import software.amazon.awssdk.services.json.endpoints.JsonEndpointProvider;
@@ -52,13 +57,13 @@ protected final String serviceName() {
5257
@Override
5358
protected final SdkClientConfiguration mergeServiceDefaults(SdkClientConfiguration config) {
5459
return config.merge(c -> c
55-
.option(SdkClientOption.ENDPOINT_PROVIDER, defaultEndpointProvider())
56-
.option(SdkClientOption.AUTH_SCHEME_PROVIDER, defaultAuthSchemeProvider())
57-
.option(SdkClientOption.AUTH_SCHEMES, authSchemes())
58-
.option(SdkClientOption.CRC32_FROM_COMPRESSED_DATA_ENABLED, false)
59-
.lazyOption(AwsClientOption.TOKEN_PROVIDER,
60+
.option(SdkClientOption.ENDPOINT_PROVIDER, defaultEndpointProvider())
61+
.option(SdkClientOption.AUTH_SCHEME_PROVIDER, defaultAuthSchemeProvider())
62+
.option(SdkClientOption.AUTH_SCHEMES, authSchemes())
63+
.option(SdkClientOption.CRC32_FROM_COMPRESSED_DATA_ENABLED, false)
64+
.lazyOption(AwsClientOption.TOKEN_PROVIDER,
6065
p -> TokenUtils.toSdkTokenProvider(p.get(AwsClientOption.TOKEN_IDENTITY_PROVIDER)))
61-
.option(AwsClientOption.TOKEN_IDENTITY_PROVIDER, defaultTokenProvider()));
66+
.option(AwsClientOption.TOKEN_IDENTITY_PROVIDER, defaultTokenProvider()));
6267
}
6368

6469
@Override
@@ -69,7 +74,7 @@ protected final SdkClientConfiguration finalizeServiceConfiguration(SdkClientCon
6974
endpointInterceptors.add(new JsonRequestSetEndpointInterceptor());
7075
ClasspathInterceptorChainFactory interceptorFactory = new ClasspathInterceptorChainFactory();
7176
List<ExecutionInterceptor> interceptors = interceptorFactory
72-
.getInterceptors("software/amazon/awssdk/services/json/execution.interceptors");
77+
.getInterceptors("software/amazon/awssdk/services/json/execution.interceptors");
7378
List<ExecutionInterceptor> additionalInterceptors = new ArrayList<>();
7479
interceptors = CollectionUtils.mergeLists(endpointInterceptors, interceptors);
7580
interceptors = CollectionUtils.mergeLists(interceptors, additionalInterceptors);
@@ -138,15 +143,36 @@ protected SdkClientConfiguration invokePlugins(SdkClientConfiguration config) {
138143
for (SdkPlugin plugin : plugins) {
139144
plugin.configureClient(serviceConfigBuilder);
140145
}
146+
updateRetryStrategyClientConfiguration(configuration);
141147
return configuration.build();
142148
}
143149

150+
private void updateRetryStrategyClientConfiguration(SdkClientConfiguration.Builder configuration) {
151+
ClientOverrideConfiguration.Builder builder = configuration.asOverrideConfigurationBuilder();
152+
RetryMode retryMode = builder.retryMode();
153+
if (retryMode != null) {
154+
configuration.option(SdkClientOption.RETRY_STRATEGY, AwsRetryStrategy.forRetryMode(retryMode));
155+
return;
156+
}
157+
Consumer<RetryStrategy.Builder<?, ?>> configurator = builder.retryStrategyConfigurator();
158+
if (configurator != null) {
159+
RetryStrategy.Builder<?, ?> defaultBuilder = AwsRetryStrategy.defaultRetryStrategy().toBuilder();
160+
configurator.accept(defaultBuilder);
161+
configuration.option(SdkClientOption.RETRY_STRATEGY, defaultBuilder.build());
162+
return;
163+
}
164+
RetryStrategy retryStrategy = builder.retryStrategy();
165+
if (retryStrategy != null) {
166+
configuration.option(SdkClientOption.RETRY_STRATEGY, retryStrategy);
167+
}
168+
}
169+
144170
private List<SdkPlugin> internalPlugins(SdkClientConfiguration config) {
145171
return Collections.emptyList();
146172
}
147173

148174
protected static void validateClientOptions(SdkClientConfiguration c) {
149175
Validate.notNull(c.option(AwsClientOption.TOKEN_IDENTITY_PROVIDER),
150-
"The 'tokenProvider' must be configured in the client builder.");
176+
"The 'tokenProvider' must be configured in the client builder.");
151177
}
152178
}

0 commit comments

Comments
 (0)