diff --git a/.changes/next-release/bugfix-AWSSDKforJavav2-ac30dda.json b/.changes/next-release/bugfix-AWSSDKforJavav2-ac30dda.json new file mode 100644 index 000000000000..68b8ee75804c --- /dev/null +++ b/.changes/next-release/bugfix-AWSSDKforJavav2-ac30dda.json @@ -0,0 +1,6 @@ +{ + "type": "bugfix", + "category": "AWS SDK for Java v2", + "contributor": "", + "description": "Add default retry predicates on top of user-provided ones" +} diff --git a/core/aws-core/pom.xml b/core/aws-core/pom.xml index 2cc0d8f411ef..88667727a876 100644 --- a/core/aws-core/pom.xml +++ b/core/aws-core/pom.xml @@ -165,6 +165,11 @@ mockito-core test + + org.mockito + mockito-junit-jupiter + test + nl.jqno.equalsverifier equalsverifier diff --git a/core/aws-core/src/main/java/software/amazon/awssdk/awscore/client/builder/AwsDefaultClientBuilder.java b/core/aws-core/src/main/java/software/amazon/awssdk/awscore/client/builder/AwsDefaultClientBuilder.java index a00d144dc147..0b4f28050210 100644 --- a/core/aws-core/src/main/java/software/amazon/awssdk/awscore/client/builder/AwsDefaultClientBuilder.java +++ b/core/aws-core/src/main/java/software/amazon/awssdk/awscore/client/builder/AwsDefaultClientBuilder.java @@ -65,7 +65,7 @@ import software.amazon.awssdk.regions.ServiceMetadataAdvancedOption; import software.amazon.awssdk.regions.providers.DefaultAwsRegionProviderChain; import software.amazon.awssdk.retries.api.RetryStrategy; -import software.amazon.awssdk.retries.internal.BaseRetryStrategy; +import software.amazon.awssdk.retries.internal.DefaultAwareRetryStrategy; import software.amazon.awssdk.utils.AttributeMap; import software.amazon.awssdk.utils.AttributeMap.LazyValueSource; import software.amazon.awssdk.utils.CollectionUtils; @@ -76,12 +76,10 @@ /** * An SDK-internal implementation of the methods in {@link AwsClientBuilder}, {@link AwsAsyncClientBuilder} and * {@link AwsSyncClientBuilder}. This implements all methods required by those interfaces, allowing service-specific builders to - * just - * implement the configuration they wish to add. + * just implement the configuration they wish to add. * *

By implementing both the sync and async interface's methods, service-specific builders can share code between their sync - * and - * async variants without needing one to extend the other. Note: This only defines the methods in the sync and async builder + * and async variants without needing one to extend the other. Note: This only defines the methods in the sync and async builder * interfaces. It does not implement the interfaces themselves. This is because the sync and async client builder interfaces both * require a type-constrained parameter for use in fluent chaining, and a generic type parameter conflict is introduced into the * class hierarchy by this interface extending the builder interfaces themselves.

@@ -89,7 +87,7 @@ *

Like all {@link AwsClientBuilder}s, this class is not thread safe.

* * @param The type of builder, for chaining. - * @param The type of client generated by this builder. + * @param The type of client generated by this builder. */ @SdkProtectedApi public abstract class AwsDefaultClientBuilder, ClientT> @@ -228,8 +226,8 @@ protected final SdkClientConfiguration setOverrides(SdkClientConfiguration confi } /** - * Check {@link SdkInternalTestAdvancedClientOption#ENDPOINT_OVERRIDDEN_OVERRIDE} to see if we should override the - * value returned by {@link SdkClientOption#CLIENT_ENDPOINT_PROVIDER}'s isEndpointOverridden. + * Check {@link SdkInternalTestAdvancedClientOption#ENDPOINT_OVERRIDDEN_OVERRIDE} to see if we should override the value + * returned by {@link SdkClientOption#CLIENT_ENDPOINT_PROVIDER}'s isEndpointOverridden. */ private void checkEndpointOverriddenOverride(SdkClientConfiguration configuration, SdkClientConfiguration.Builder builder) { Optional endpointOverriddenOverride = @@ -339,16 +337,16 @@ private ClientEndpointProvider resolveClientEndpointProvider(LazyValueSource con } /** - * Resolve the client endpoint. This code is only needed by old SDK client versions. Newer SDK client versions - * resolve this information from the client endpoint provider. + * Resolve the client endpoint. This code is only needed by old SDK client versions. Newer SDK client versions resolve this + * information from the client endpoint provider. */ private URI resolveEndpoint(LazyValueSource config) { return config.get(SdkClientOption.CLIENT_ENDPOINT_PROVIDER).clientEndpoint(); } /** - * Resolve whether the endpoint was overridden by the customer. This code is only needed by old SDK client - * versions. Newer SDK client versions resolve this information from the client endpoint provider. + * Resolve whether the endpoint was overridden by the customer. This code is only needed by old SDK client versions. Newer SDK + * client versions resolve this information from the client endpoint provider. */ private boolean resolveEndpointOverridden(LazyValueSource config) { return config.get(SdkClientOption.CLIENT_ENDPOINT_PROVIDER).isEndpointOverridden(); @@ -423,28 +421,25 @@ private void configureRetryPolicy(SdkClientConfiguration.Builder config) { private void configureRetryStrategy(SdkClientConfiguration.Builder config) { RetryStrategy strategy = config.option(SdkClientOption.RETRY_STRATEGY); - if (strategy != null) { - // TODO(10/09/24) This is a temporal workaround and not a long term solution. It will fail to add the SDK and AWS - // defaults if the users add one or more if their own retry predicates. A long term fix is needed that can - // "remember" which defaults have been already applied, e.g., - // if (strategy.shouldAddDefaults("aws")) { - // strategy = strategy.toBuilder() - // .applyMutation(AwsRetryStrategy::applyDefaults) - // .markDefaultsAdded("aws") - // .build(); - // } - if (strategy.maxAttempts() > 1 - && (strategy instanceof BaseRetryStrategy) - && !((BaseRetryStrategy) strategy).hasRetryPredicates() - ) { - RetryStrategy.Builder builder = strategy.toBuilder(); - SdkDefaultRetryStrategy.configureStrategy(builder); - AwsRetryStrategy.configureStrategy(builder); - config.option(SdkClientOption.RETRY_STRATEGY, builder.build()); - } + if (strategy == null) { + config.lazyOption(SdkClientOption.RETRY_STRATEGY, this::resolveAwsRetryStrategy); return; } - config.lazyOption(SdkClientOption.RETRY_STRATEGY, this::resolveAwsRetryStrategy); + + if (!strategy.useClientDefaults()) { + return; + } + + if (!(strategy instanceof DefaultAwareRetryStrategy)) { + return; + } + DefaultAwareRetryStrategy defaultAwareRetryStrategy = (DefaultAwareRetryStrategy) strategy; + defaultAwareRetryStrategy = defaultAwareRetryStrategy.addDefaults(AwsRetryStrategy.retryStrategyDefaults()); + defaultAwareRetryStrategy = defaultAwareRetryStrategy.addDefaults(SdkDefaultRetryStrategy.retryStrategyDefaults()); + + RetryStrategy.Builder strategyBuilder = defaultAwareRetryStrategy.toBuilder(); + config.option(SdkClientOption.RETRY_STRATEGY, strategyBuilder.build()); + } private RetryStrategy resolveAwsRetryStrategy(LazyValueSource config) { @@ -536,8 +531,8 @@ public final void setDefaultsMode(DefaultsMode defaultsMode) { /** * If the region is a FIPS pseudo region (contains "fips"), this method returns a pair of values, the left side being the - * region with the "fips" string removed, and the right being {@code true}. Otherwise, the region is returned - * unchanged, and the right will be empty. + * region with the "fips" string removed, and the right being {@code true}. Otherwise, the region is returned unchanged, and + * the right will be empty. */ private static Pair> transformFipsPseudoRegionIfNecessary(Region region) { String id = region.id(); diff --git a/core/aws-core/src/main/java/software/amazon/awssdk/awscore/retry/AwsRetryStrategy.java b/core/aws-core/src/main/java/software/amazon/awssdk/awscore/retry/AwsRetryStrategy.java index bef57e64f301..17cd00b58e08 100644 --- a/core/aws-core/src/main/java/software/amazon/awssdk/awscore/retry/AwsRetryStrategy.java +++ b/core/aws-core/src/main/java/software/amazon/awssdk/awscore/retry/AwsRetryStrategy.java @@ -26,6 +26,8 @@ import software.amazon.awssdk.retries.LegacyRetryStrategy; import software.amazon.awssdk.retries.StandardRetryStrategy; import software.amazon.awssdk.retries.api.RetryStrategy; +import software.amazon.awssdk.retries.internal.DefaultAwareRetryStrategy; +import software.amazon.awssdk.retries.internal.RetryStrategyDefaults; /** * Retry strategies used by clients when communicating with AWS services. @@ -33,6 +35,21 @@ @SdkPublicApi public final class AwsRetryStrategy { + private static final String DEFAULTS_NAME = "aws"; + + private static final RetryStrategyDefaults DEFAULTS_PREDICATES = new RetryStrategyDefaults() { + @Override + public String name() { + return DEFAULTS_NAME; + } + + @Override + public void applyDefaults(RetryStrategy.Builder retryStrategyBuilder) { + configureStrategy(retryStrategyBuilder); + markDefaultsAdded(retryStrategyBuilder); + } + }; + private AwsRetryStrategy() { } @@ -127,7 +144,9 @@ public static AdaptiveRetryStrategy adaptiveRetryStrategy() { * @return The given builder */ public static > T configure(T builder) { - return builder.retryOnException(AwsRetryStrategy::retryOnAwsRetryableErrors); + builder.retryOnException(AwsRetryStrategy::retryOnAwsRetryableErrors); + markDefaultsAdded(builder); + return builder; } /** @@ -162,4 +181,15 @@ private static RetryStrategy legacyAdaptiveRetryStrategy() { .build(); } + public static RetryStrategyDefaults retryStrategyDefaults() { + return DEFAULTS_PREDICATES; + } + + private static void markDefaultsAdded(RetryStrategy.Builder builder) { + if (builder instanceof DefaultAwareRetryStrategy.Builder) { + DefaultAwareRetryStrategy.Builder b = (DefaultAwareRetryStrategy.Builder) builder; + b.markDefaultAdded(DEFAULTS_NAME); + } + } + } diff --git a/core/aws-core/src/test/java/software/amazon/awssdk/awscore/client/builder/DefaultAwsClientBuilderTest.java b/core/aws-core/src/test/java/software/amazon/awssdk/awscore/client/builder/DefaultAwsClientBuilderTest.java index ca3061cca171..74d263c68f43 100644 --- a/core/aws-core/src/test/java/software/amazon/awssdk/awscore/client/builder/DefaultAwsClientBuilderTest.java +++ b/core/aws-core/src/test/java/software/amazon/awssdk/awscore/client/builder/DefaultAwsClientBuilderTest.java @@ -35,29 +35,43 @@ import java.time.Duration; import java.util.Arrays; import java.util.Optional; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; +import java.util.function.Predicate; +import java.util.stream.Stream; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import org.mockito.Mock; -import org.mockito.junit.MockitoJUnitRunner; import software.amazon.awssdk.auth.credentials.AnonymousCredentialsProvider; import software.amazon.awssdk.auth.signer.Aws4Signer; import software.amazon.awssdk.awscore.client.config.AwsClientOption; import software.amazon.awssdk.awscore.internal.defaultsmode.AutoDefaultsModeDiscovery; +import software.amazon.awssdk.awscore.retry.AwsRetryStrategy; import software.amazon.awssdk.core.client.config.ClientOverrideConfiguration; import software.amazon.awssdk.core.client.config.SdkClientConfiguration; import software.amazon.awssdk.core.client.config.SdkClientOption; +import software.amazon.awssdk.core.internal.retry.SdkDefaultRetryStrategy; import software.amazon.awssdk.core.signer.Signer; import software.amazon.awssdk.http.SdkHttpClient; import software.amazon.awssdk.http.SdkHttpConfigurationOption; import software.amazon.awssdk.http.async.SdkAsyncHttpClient; import software.amazon.awssdk.regions.Region; +import software.amazon.awssdk.retries.AdaptiveRetryStrategy; +import software.amazon.awssdk.retries.DefaultRetryStrategy; +import software.amazon.awssdk.retries.LegacyRetryStrategy; +import software.amazon.awssdk.retries.StandardRetryStrategy; +import software.amazon.awssdk.retries.api.BackoffStrategy; +import software.amazon.awssdk.retries.api.RetryStrategy; +import software.amazon.awssdk.retries.internal.BaseRetryStrategy; import software.amazon.awssdk.utils.AttributeMap; +import org.mockito.junit.jupiter.MockitoExtension; /** * Validate the functionality of the {@link AwsDefaultClientBuilder}. */ -@RunWith(MockitoJUnitRunner.class) +@ExtendWith(MockitoExtension.class) public class DefaultAwsClientBuilderTest { private static final AttributeMap MOCK_DEFAULTS = AttributeMap @@ -71,16 +85,16 @@ public class DefaultAwsClientBuilderTest { private static final Signer TEST_SIGNER = Aws4Signer.create(); private static final URI ENDPOINT = URI.create("https://example.com"); - @Mock + @Mock(lenient = true) private SdkHttpClient.Builder defaultHttpClientBuilder; - @Mock + @Mock(lenient = true) private SdkAsyncHttpClient.Builder defaultAsyncHttpClientFactory; @Mock private AutoDefaultsModeDiscovery autoModeDiscovery; - @Before + @BeforeEach public void setup() { when(defaultHttpClientBuilder.buildWithDefaults(any())).thenReturn(mock(SdkHttpClient.class)); when(defaultAsyncHttpClientFactory.buildWithDefaults(any())).thenReturn(mock(SdkAsyncHttpClient.class)); @@ -251,6 +265,102 @@ public void clientBuilderFieldsHaveBeanEquivalents() throws Exception { }); } + @ParameterizedTest(name = "{0} - expectedPredicateAmount: {2}") + @MethodSource("retryArguments") + void retryStrategyConfiguration_shouldAddDefaultPredicatesWhenRequired( + String testDescription, RetryStrategy retryStrategy, int expectedPredicateAmount) { + TestClientBuilder builder = testClientBuilder() + .region(Region.US_EAST_1) + .overrideConfiguration(c -> c.retryStrategy(retryStrategy)); + TestClient client = builder.build(); + + ClientOverrideConfiguration conf = client.clientConfiguration.asOverrideConfiguration(); + assertThat(conf.retryStrategy()).isPresent(); + + RetryStrategy configuredRetryStrategy = conf.retryStrategy().get(); + BaseRetryStrategy baseRetryStrategy = (BaseRetryStrategy) configuredRetryStrategy; + assertThat(baseRetryStrategy.shouldAddDefaults(AwsRetryStrategy.retryStrategyDefaults().name())).isFalse(); + assertThat(baseRetryStrategy.shouldAddDefaults(SdkDefaultRetryStrategy.retryStrategyDefaults().name())).isFalse(); + assertThat(baseRetryStrategy.retryPredicates()).hasSize(expectedPredicateAmount); + } + + private static Stream retryArguments() { + return Stream.of( + Arguments.of("Standard - static method creation", + SdkDefaultRetryStrategy.standardRetryStrategy(), 9), + Arguments.of("Standard - static builder method", + DefaultRetryStrategy.standardStrategyBuilder().build(), 9), + Arguments.of("Standard - builder with retryOnException", + StandardRetryStrategy.builder() + .retryOnException(TestException.class) + .backoffStrategy(BackoffStrategy.retryImmediately()) + .throttlingBackoffStrategy(BackoffStrategy.retryImmediately()) + .build(), 10), + Arguments.of("Standard - useClientDefaults=false", + StandardRetryStrategy.builder() + .retryOnException(TestException.class) + .backoffStrategy(BackoffStrategy.retryImmediately()) + .throttlingBackoffStrategy(BackoffStrategy.retryImmediately()) + .useClientDefaults(false) + .build(), 1), + Arguments.of("Adaptive - static method creation", + SdkDefaultRetryStrategy.adaptiveRetryStrategy(), 9), + Arguments.of("Adaptive - builder with retryOnException", + AdaptiveRetryStrategy.builder() + .retryOnException(TestException.class) + .backoffStrategy(BackoffStrategy.retryImmediately()) + .throttlingBackoffStrategy(BackoffStrategy.retryImmediately()) + .build(), 10), + Arguments.of("Adaptive - useClientDefaults=false", + AdaptiveRetryStrategy.builder() + .retryOnException(TestException.class) + .backoffStrategy(BackoffStrategy.retryImmediately()) + .throttlingBackoffStrategy(BackoffStrategy.retryImmediately()) + .useClientDefaults(false) + .build(), 1), + Arguments.of("Legacy - static method creation", + SdkDefaultRetryStrategy.legacyRetryStrategy(), 9), + Arguments.of("Legacy - builder with retryOnException", + LegacyRetryStrategy.builder() + .retryOnException(TestException.class) + .backoffStrategy(BackoffStrategy.retryImmediately()) + .throttlingBackoffStrategy(BackoffStrategy.retryImmediately()) + .build(), 10), + Arguments.of("Legacy - useClientDefaults=false", + LegacyRetryStrategy.builder() + .retryOnException(TestException.class) + .backoffStrategy(BackoffStrategy.retryImmediately()) + .throttlingBackoffStrategy(BackoffStrategy.retryImmediately()) + .useClientDefaults(false) + .build(), 1) + ); + } + + @ParameterizedTest(name = "{0} - expectedPredicateAmount: ({2}+2)") + @MethodSource("retryArguments") + void retryBuilderRoundTrip_ShouldKeepDefaults_ShouldAddNewPredicates( + String testDescription, RetryStrategy retryStrategy, int expectedPredicateAmount) { + RetryStrategy.Builder retryBuilder = retryStrategy.toBuilder(); + Predicate newDummyPredicate1 = t -> t instanceof RuntimeException; + Predicate newDummyPredicate2 = t -> t instanceof IllegalArgumentException; + retryBuilder.retryOnException(newDummyPredicate1); + retryBuilder.retryOnException(newDummyPredicate2); + + TestClientBuilder clientBuilder = testClientBuilder() + .region(Region.US_EAST_1) + .overrideConfiguration(c -> c.retryStrategy(retryBuilder.build())); + TestClient client = clientBuilder.build(); + + ClientOverrideConfiguration conf = client.clientConfiguration.asOverrideConfiguration(); + assertThat(conf.retryStrategy()).isPresent(); + + RetryStrategy configuredRetryStrategy = conf.retryStrategy().get(); + BaseRetryStrategy baseRetryStrategy = (BaseRetryStrategy) configuredRetryStrategy; + assertThat(baseRetryStrategy.shouldAddDefaults(AwsRetryStrategy.retryStrategyDefaults().name())).isFalse(); + assertThat(baseRetryStrategy.shouldAddDefaults(SdkDefaultRetryStrategy.retryStrategyDefaults().name())).isFalse(); + assertThat(baseRetryStrategy.retryPredicates()).hasSize(expectedPredicateAmount + 2); + } + private AwsClientBuilder testClientBuilder() { ClientOverrideConfiguration overrideConfig = ClientOverrideConfiguration.builder() @@ -354,4 +464,7 @@ protected AttributeMap serviceHttpConfig() { return MOCK_DEFAULTS; } } + + private static class TestException extends Throwable { + } } diff --git a/core/retries-spi/src/main/java/software/amazon/awssdk/retries/api/RetryStrategy.java b/core/retries-spi/src/main/java/software/amazon/awssdk/retries/api/RetryStrategy.java index b7d8a66e93ec..491bab172e65 100644 --- a/core/retries-spi/src/main/java/software/amazon/awssdk/retries/api/RetryStrategy.java +++ b/core/retries-spi/src/main/java/software/amazon/awssdk/retries/api/RetryStrategy.java @@ -18,6 +18,7 @@ import java.util.function.Predicate; import software.amazon.awssdk.annotations.SdkPublicApi; import software.amazon.awssdk.annotations.ThreadSafe; +import software.amazon.awssdk.utils.builder.SdkBuilder; /** * A strategy used by an SDK to determine when something should be retried. @@ -85,6 +86,15 @@ public interface RetryStrategy { */ int maxAttempts(); + /** + * Returns whether the retry strategy uses default retry predicates. + * + * @return true if this retry strategy should use the default retry predicates, false otherwise. + */ + default boolean useClientDefaults() { + return true; + } + /** * Create a new {@link Builder} with the current configuration. * @@ -97,7 +107,7 @@ public interface RetryStrategy { */ interface Builder< B extends Builder, - T extends RetryStrategy> { + T extends RetryStrategy> extends SdkBuilder { /** * Configure the strategy to retry when the provided predicate returns true, given a failure exception. */ @@ -218,6 +228,16 @@ default B retryOnRootCauseInstanceOf(Class throwable) { */ B treatAsThrottling(Predicate treatAsThrottling); + + /** + * Configure whether the default predicates should be used, or not. When set to false, only user-provided retry + * predicates will be considered to determine what should be retried or not. Setting this to false and providing + * no predicate will result in an empty retry strategy. + */ + default B useClientDefaults(boolean useClientDefaults) { + return (B) this; + } + /** * Build a new {@link RetryStrategy} with the current configuration on this builder. */ diff --git a/core/retries/src/main/java/software/amazon/awssdk/retries/internal/BaseRetryStrategy.java b/core/retries/src/main/java/software/amazon/awssdk/retries/internal/BaseRetryStrategy.java index 4d950dcae06a..2a74eef5a3a2 100644 --- a/core/retries/src/main/java/software/amazon/awssdk/retries/internal/BaseRetryStrategy.java +++ b/core/retries/src/main/java/software/amazon/awssdk/retries/internal/BaseRetryStrategy.java @@ -18,7 +18,9 @@ import java.time.Duration; import java.util.ArrayList; import java.util.Collections; +import java.util.HashSet; import java.util.List; +import java.util.Set; import java.util.function.Predicate; import software.amazon.awssdk.annotations.SdkInternalApi; import software.amazon.awssdk.retries.api.AcquireInitialTokenRequest; @@ -37,6 +39,7 @@ import software.amazon.awssdk.retries.internal.circuitbreaker.TokenBucket; import software.amazon.awssdk.retries.internal.circuitbreaker.TokenBucketStore; import software.amazon.awssdk.utils.Logger; +import software.amazon.awssdk.utils.ToString; import software.amazon.awssdk.utils.Validate; /** @@ -44,7 +47,7 @@ * tailor the behavior to its needs. */ @SdkInternalApi -public abstract class BaseRetryStrategy implements RetryStrategy { +public abstract class BaseRetryStrategy implements DefaultAwareRetryStrategy { protected final Logger log; protected final List> retryPredicates; @@ -55,6 +58,8 @@ public abstract class BaseRetryStrategy implements RetryStrategy { protected final Predicate treatAsThrottling; protected final int exceptionCost; protected final TokenBucketStore tokenBucketStore; + protected final Set defaultsAdded; + protected final Boolean useClientDefaults; BaseRetryStrategy(Logger log, Builder builder) { this.log = log; @@ -66,6 +71,8 @@ public abstract class BaseRetryStrategy implements RetryStrategy { this.treatAsThrottling = Validate.paramNotNull(builder.treatAsThrottling, "treatAsThrottling"); this.exceptionCost = Validate.paramNotNull(builder.exceptionCost, "exceptionCost"); this.tokenBucketStore = Validate.paramNotNull(builder.tokenBucketStore, "tokenBucketStore"); + this.defaultsAdded = Validate.paramNotNull(builder.defaultsAdded, "defaultsAdded"); + this.useClientDefaults = builder.useClientDefaults == null || builder.useClientDefaults; } /** @@ -139,6 +146,10 @@ public int maxAttempts() { return maxAttempts; } + @Override + public boolean useClientDefaults() { + return useClientDefaults; + } /** * Computes the backoff before the first attempt, by default {@link Duration#ZERO}. Extending classes can override this method @@ -196,6 +207,10 @@ public final boolean hasRetryPredicates() { return !retryPredicates.isEmpty(); } + public List> retryPredicates() { + return retryPredicates; + } + private DefaultRetryToken refreshToken(RefreshRetryTokenRequest request, AcquireResponse acquireResponse) { DefaultRetryToken token = asDefaultRetryToken(request.token()); return token.toBuilder() @@ -352,10 +367,44 @@ static DefaultRetryToken asDefaultRetryToken(RetryToken token) { token.getClass().getName()); } - static class Builder { + + public boolean shouldAddDefaults(String defaultPredicateName) { + return useClientDefaults && !defaultsAdded.contains(defaultPredicateName); + } + + @Override + public DefaultAwareRetryStrategy addDefaults(RetryStrategyDefaults retryStrategyDefaults) { + if (!shouldAddDefaults(retryStrategyDefaults.name())) { + return this; + } + RetryStrategy.Builder builder = this.toBuilder(); + retryStrategyDefaults.applyDefaults(builder); + return (DefaultAwareRetryStrategy) builder.build(); + } + + @Override + public String toString() { + return ToString.builder("BaseRetryStrategy") + .add("retryPredicates", retryPredicates) + .add("maxAttempts", maxAttempts) + .add("circuitBreakerEnabled", circuitBreakerEnabled) + .add("backoffStrategy", backoffStrategy) + .add("throttlingBackoffStrategy", throttlingBackoffStrategy) + .add("treatAsThrottling", treatAsThrottling) + .add("exceptionCost", exceptionCost) + .add("tokenBucketStore", tokenBucketStore) + .add("defaultsAdded", defaultsAdded) + .add("useClientDefaults", useClientDefaults) + .build(); + } + + + public abstract static class Builder implements DefaultAwareRetryStrategy.Builder { private List> retryPredicates; + private Set defaultsAdded; private int maxAttempts; private Boolean circuitBreakerEnabled; + private Boolean useClientDefaults; private Integer exceptionCost; private BackoffStrategy backoffStrategy; private BackoffStrategy throttlingBackoffStrategy; @@ -364,6 +413,7 @@ static class Builder { Builder() { retryPredicates = new ArrayList<>(); + defaultsAdded = new HashSet<>(); } Builder(BaseRetryStrategy strategy) { @@ -375,6 +425,8 @@ static class Builder { this.throttlingBackoffStrategy = strategy.throttlingBackoffStrategy; this.treatAsThrottling = strategy.treatAsThrottling; this.tokenBucketStore = strategy.tokenBucketStore; + this.defaultsAdded = strategy.defaultsAdded; + this.useClientDefaults = strategy.useClientDefaults; } void setRetryOnException(Predicate shouldRetry) { @@ -408,5 +460,14 @@ void setTreatAsThrottling(Predicate treatAsThrottling) { void setTokenBucketExceptionCost(int exceptionCost) { this.exceptionCost = exceptionCost; } + + void setUseClientDefaults(Boolean useClientDefaults) { + this.useClientDefaults = useClientDefaults; + } + + @Override + public void markDefaultAdded(String d) { + defaultsAdded.add(d); + } } } diff --git a/core/retries/src/main/java/software/amazon/awssdk/retries/internal/DefaultAdaptiveRetryStrategy.java b/core/retries/src/main/java/software/amazon/awssdk/retries/internal/DefaultAdaptiveRetryStrategy.java index bd526b057e4e..e5e56b602b39 100644 --- a/core/retries/src/main/java/software/amazon/awssdk/retries/internal/DefaultAdaptiveRetryStrategy.java +++ b/core/retries/src/main/java/software/amazon/awssdk/retries/internal/DefaultAdaptiveRetryStrategy.java @@ -139,6 +139,12 @@ public Builder tokenBucketStore(TokenBucketStore tokenBucketStore) { return this; } + @Override + public Builder useClientDefaults(boolean useClientDefaults) { + setUseClientDefaults(useClientDefaults); + return this; + } + @Override public AdaptiveRetryStrategy build() { return new DefaultAdaptiveRetryStrategy(this); diff --git a/core/retries/src/main/java/software/amazon/awssdk/retries/internal/DefaultAwareRetryStrategy.java b/core/retries/src/main/java/software/amazon/awssdk/retries/internal/DefaultAwareRetryStrategy.java new file mode 100644 index 000000000000..a36bf9b7ad03 --- /dev/null +++ b/core/retries/src/main/java/software/amazon/awssdk/retries/internal/DefaultAwareRetryStrategy.java @@ -0,0 +1,42 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package software.amazon.awssdk.retries.internal; + +import software.amazon.awssdk.annotations.SdkProtectedApi; +import software.amazon.awssdk.retries.api.RetryStrategy; + +/** + * Identify a {@link RetryStrategy} that has the capacity to work with sets of default retry predicates. + */ +@SdkProtectedApi +public interface DefaultAwareRetryStrategy extends RetryStrategy { + + /** + * Add the specified defaults to this retry strategy + * @param retryStrategyDefaults the defaults to add to this strategy + * @return a new retry strategy containing the specified defaults. + */ + DefaultAwareRetryStrategy addDefaults(RetryStrategyDefaults retryStrategyDefaults); + + interface Builder { + + /** + * Identify the Builder as having the specified defaults to it. + * @param defaultPredicatesName the name the defaults to mark as added + */ + void markDefaultAdded(String defaultPredicatesName); + } +} diff --git a/core/retries/src/main/java/software/amazon/awssdk/retries/internal/DefaultLegacyRetryStrategy.java b/core/retries/src/main/java/software/amazon/awssdk/retries/internal/DefaultLegacyRetryStrategy.java index 9ad358e4c8f5..23520a68c234 100644 --- a/core/retries/src/main/java/software/amazon/awssdk/retries/internal/DefaultLegacyRetryStrategy.java +++ b/core/retries/src/main/java/software/amazon/awssdk/retries/internal/DefaultLegacyRetryStrategy.java @@ -117,6 +117,12 @@ public Builder tokenBucketStore(TokenBucketStore tokenBucketStore) { return this; } + @Override + public Builder useClientDefaults(boolean useClientDefaults) { + setUseClientDefaults(useClientDefaults); + return this; + } + @Override public LegacyRetryStrategy build() { return new DefaultLegacyRetryStrategy(this); diff --git a/core/retries/src/main/java/software/amazon/awssdk/retries/internal/DefaultStandardRetryStrategy.java b/core/retries/src/main/java/software/amazon/awssdk/retries/internal/DefaultStandardRetryStrategy.java index ada7ba1efe3c..3a6a120fa398 100644 --- a/core/retries/src/main/java/software/amazon/awssdk/retries/internal/DefaultStandardRetryStrategy.java +++ b/core/retries/src/main/java/software/amazon/awssdk/retries/internal/DefaultStandardRetryStrategy.java @@ -95,6 +95,12 @@ public Builder tokenBucketStore(TokenBucketStore tokenBucketStore) { return this; } + @Override + public Builder useClientDefaults(boolean useClientDefaults) { + setUseClientDefaults(useClientDefaults); + return this; + } + @Override public StandardRetryStrategy build() { return new DefaultStandardRetryStrategy(this); diff --git a/core/retries/src/main/java/software/amazon/awssdk/retries/internal/RetryStrategyDefaults.java b/core/retries/src/main/java/software/amazon/awssdk/retries/internal/RetryStrategyDefaults.java new file mode 100644 index 000000000000..3cf2658bedec --- /dev/null +++ b/core/retries/src/main/java/software/amazon/awssdk/retries/internal/RetryStrategyDefaults.java @@ -0,0 +1,37 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package software.amazon.awssdk.retries.internal; + +import software.amazon.awssdk.annotations.SdkProtectedApi; +import software.amazon.awssdk.retries.api.RetryStrategy; + +/** + * The set of retry predicates that are by default added to a retry strategy. + */ +@SdkProtectedApi +public interface RetryStrategyDefaults { + + /** + * @return The unique name that identifies this set of predicates + */ + String name(); + + /** + * Apply this set of defaults to the provided retry strategy builder. + * @param retryStrategyBuilder the retry strategy to apply the defaults to + */ + void applyDefaults(RetryStrategy.Builder retryStrategyBuilder); +} diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/retry/SdkDefaultRetryStrategy.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/retry/SdkDefaultRetryStrategy.java index d5f1320f1af2..bc7685b6126c 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/retry/SdkDefaultRetryStrategy.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/retry/SdkDefaultRetryStrategy.java @@ -27,6 +27,8 @@ import software.amazon.awssdk.retries.LegacyRetryStrategy; import software.amazon.awssdk.retries.StandardRetryStrategy; import software.amazon.awssdk.retries.api.RetryStrategy; +import software.amazon.awssdk.retries.internal.DefaultAwareRetryStrategy; +import software.amazon.awssdk.retries.internal.RetryStrategyDefaults; /** * Retry strategies used by any SDK client. @@ -34,6 +36,21 @@ @SdkPublicApi public final class SdkDefaultRetryStrategy { + private static final String DEFAULTS_NAME = "sdk"; + + private static final RetryStrategyDefaults DEFAULTS_PREDICATES = new RetryStrategyDefaults() { + @Override + public String name() { + return DEFAULTS_NAME; + } + + @Override + public void applyDefaults(RetryStrategy.Builder builder) { + configureStrategy(builder); + markDefaultsAdded(builder); + } + }; + private SdkDefaultRetryStrategy() { } @@ -153,7 +170,6 @@ public static AdaptiveRetryStrategy.Builder adaptiveRetryStrategyBuilder() { * @param The type of the builder extending {@link RetryStrategy.Builder} * @return The given builder */ - public static > T configure(T builder) { builder.retryOnException(SdkDefaultRetryStrategy::retryOnRetryableException) .retryOnException(SdkDefaultRetryStrategy::retryOnStatusCodes) @@ -165,6 +181,10 @@ public static AdaptiveRetryStrategy.Builder adaptiveRetryStrategyBuilder() { if (maxAttempts != null) { builder.maxAttempts(maxAttempts); } + if (builder instanceof DefaultAwareRetryStrategy.Builder) { + DefaultAwareRetryStrategy.Builder b = (DefaultAwareRetryStrategy.Builder) builder; + b.markDefaultAdded(DEFAULTS_NAME); + } return builder; } @@ -230,5 +250,17 @@ private static RetryStrategy legacyAdaptiveRetryStrategy() { .retryPolicy(RetryPolicy.forRetryMode(RetryMode.ADAPTIVE)) .build(); } + + public static RetryStrategyDefaults retryStrategyDefaults() { + return DEFAULTS_PREDICATES; + } + + private static void markDefaultsAdded(RetryStrategy.Builder builder) { + if (builder instanceof DefaultAwareRetryStrategy.Builder) { + DefaultAwareRetryStrategy.Builder b = (DefaultAwareRetryStrategy.Builder) builder; + b.markDefaultAdded(DEFAULTS_NAME); + } + } + }