From b58c1e795b0582d720003c9c5563936d90e56d47 Mon Sep 17 00:00:00 2001 From: hdavidh Date: Mon, 7 Jul 2025 22:10:15 -0700 Subject: [PATCH 1/4] URI cache for DynamoDB account id based endpoint - PR #6087 --- .../feature-AWSSDKforJavav2-960b6f3.json | 6 - .../feature-AWSSDKforJavav2-b405876.json | 6 + .../amazon/awssdk/spotbugs-suppressions.xml | 6 +- .../customization/CustomizationConfig.java | 13 + .../poet/rules2/CodeGeneratorVisitor.java | 10 +- .../poet/rules2/EndpointProviderSpec2.java | 2 + .../awssdk/codegen/poet/ClientTestModels.java | 46 ++- ...intProviderCompiledRulesClassSpecTest.java | 7 + .../c2j/query/customization-uri-cache.config | 37 +++ .../endpoint-provider-uri-cache-class.java | 155 +++++++++ .../internal/CrtHttpRequestConverter.java | 3 +- .../endpoint/AwsClientEndpointProvider.java | 5 +- .../util/CrtHttpRequestConverter.java | 3 +- .../internal/util/ServiceMetadataUtils.java | 4 +- .../core/internal/util/MetricUtils.java | 4 +- .../amazon/awssdk/http/SdkHttpRequest.java | 3 +- .../impl/ApacheHttpRequestFactory.java | 3 +- .../awssdk/http/crt/AwsCrtHttpClientBase.java | 5 +- .../nio/netty/NettyNioAsyncHttpClient.java | 5 +- .../dynamodb/customization.config | 3 +- utils/pom.xml | 5 + .../amazon/awssdk/utils/uri/SdkUri.java | 294 +++++++++++++++++ .../uri/internal/UriConstructorArgs.java | 38 +++ .../amazon/awssdk/utils/SdkUriTest.java | 305 ++++++++++++++++++ 24 files changed, 935 insertions(+), 33 deletions(-) delete mode 100644 .changes/next-release/feature-AWSSDKforJavav2-960b6f3.json create mode 100644 .changes/next-release/feature-AWSSDKforJavav2-b405876.json create mode 100644 codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/c2j/query/customization-uri-cache.config create mode 100644 codegen/src/test/resources/software/amazon/awssdk/codegen/poet/rules2/endpoint-provider-uri-cache-class.java create mode 100644 utils/src/main/java/software/amazon/awssdk/utils/uri/SdkUri.java create mode 100644 utils/src/main/java/software/amazon/awssdk/utils/uri/internal/UriConstructorArgs.java create mode 100644 utils/src/test/java/software/amazon/awssdk/utils/SdkUriTest.java diff --git a/.changes/next-release/feature-AWSSDKforJavav2-960b6f3.json b/.changes/next-release/feature-AWSSDKforJavav2-960b6f3.json deleted file mode 100644 index aaa74d616f3c..000000000000 --- a/.changes/next-release/feature-AWSSDKforJavav2-960b6f3.json +++ /dev/null @@ -1,6 +0,0 @@ -{ - "type": "feature", - "category": "AWS SDK for Java v2", - "contributor": "", - "description": "Add validation for invalid usages of the `enableEnvironmentBearerToken` codegen customization." -} diff --git a/.changes/next-release/feature-AWSSDKforJavav2-b405876.json b/.changes/next-release/feature-AWSSDKforJavav2-b405876.json new file mode 100644 index 000000000000..bb9d5276a1ac --- /dev/null +++ b/.changes/next-release/feature-AWSSDKforJavav2-b405876.json @@ -0,0 +1,6 @@ +{ + "type": "feature", + "category": "Amazon DyanmoDB", + "contributor": "", + "description": "Enable caching calls to URI constructors for account-id based endpoints" +} diff --git a/build-tools/src/main/resources/software/amazon/awssdk/spotbugs-suppressions.xml b/build-tools/src/main/resources/software/amazon/awssdk/spotbugs-suppressions.xml index e123f641d076..f822308b6ced 100644 --- a/build-tools/src/main/resources/software/amazon/awssdk/spotbugs-suppressions.xml +++ b/build-tools/src/main/resources/software/amazon/awssdk/spotbugs-suppressions.xml @@ -353,10 +353,14 @@ - + + + + + diff --git a/codegen/src/main/java/software/amazon/awssdk/codegen/model/config/customization/CustomizationConfig.java b/codegen/src/main/java/software/amazon/awssdk/codegen/model/config/customization/CustomizationConfig.java index 069c6ca6d5a2..cfa719d4c738 100644 --- a/codegen/src/main/java/software/amazon/awssdk/codegen/model/config/customization/CustomizationConfig.java +++ b/codegen/src/main/java/software/amazon/awssdk/codegen/model/config/customization/CustomizationConfig.java @@ -357,6 +357,11 @@ public class CustomizationConfig { */ private boolean enableEnvironmentBearerToken = false; + /** + * A boolean flag to indicate if the code-generated endpoint providers class should cache the calls to URI constructors. + */ + private boolean enableEndpointProviderUriCaching; + private CustomizationConfig() { } @@ -939,4 +944,12 @@ public boolean isEnableEnvironmentBearerToken() { public void setEnableEnvironmentBearerToken(boolean enableEnvironmentBearerToken) { this.enableEnvironmentBearerToken = enableEnvironmentBearerToken; } + + public boolean getEnableEndpointProviderUriCaching() { + return enableEndpointProviderUriCaching; + } + + public void setEnableEndpointProviderUriCaching(boolean enableEndpointProviderUriCaching) { + this.enableEndpointProviderUriCaching = enableEndpointProviderUriCaching; + } } diff --git a/codegen/src/main/java/software/amazon/awssdk/codegen/poet/rules2/CodeGeneratorVisitor.java b/codegen/src/main/java/software/amazon/awssdk/codegen/poet/rules2/CodeGeneratorVisitor.java index 609e2a5f5663..72f8b28a1fa2 100644 --- a/codegen/src/main/java/software/amazon/awssdk/codegen/poet/rules2/CodeGeneratorVisitor.java +++ b/codegen/src/main/java/software/amazon/awssdk/codegen/poet/rules2/CodeGeneratorVisitor.java @@ -29,6 +29,7 @@ import software.amazon.awssdk.awscore.endpoints.authscheme.SigV4aAuthScheme; import software.amazon.awssdk.codegen.model.config.customization.KeyTypePair; import software.amazon.awssdk.endpoints.Endpoint; +import software.amazon.awssdk.utils.uri.SdkUri; public class CodeGeneratorVisitor extends WalkRuleExpressionVisitor { private static final Logger log = LoggerFactory.getLogger(CodeGeneratorVisitor.class); @@ -38,17 +39,20 @@ public class CodeGeneratorVisitor extends WalkRuleExpressionVisitor { private final SymbolTable symbolTable; private final Map knownEndpointAttributes; private final Map ruleIdToScope; + private final boolean endpointCaching; public CodeGeneratorVisitor(RuleRuntimeTypeMirror typeMirror, SymbolTable symbolTable, Map knownEndpointAttributes, Map ruleIdToScope, + boolean endpointCaching, CodeBlock.Builder builder) { this.builder = builder; this.symbolTable = symbolTable; this.knownEndpointAttributes = knownEndpointAttributes; this.ruleIdToScope = ruleIdToScope; this.typeMirror = typeMirror; + this.endpointCaching = endpointCaching; } @Override @@ -329,7 +333,11 @@ private String callParams(String ruleId) { @Override public Void visitEndpointExpression(EndpointExpression e) { builder.add("return $T.endpoint(", typeMirror.rulesResult().type()); - builder.add("$T.builder().url($T.create(", Endpoint.class, URI.class); + if (endpointCaching) { + builder.add("$T.builder().url($T.getInstance().create(", Endpoint.class, SdkUri.class); + } else { + builder.add("$T.builder().url($T.create(", Endpoint.class, URI.class); + } e.url().accept(this); builder.add("))"); e.headers().accept(this); diff --git a/codegen/src/main/java/software/amazon/awssdk/codegen/poet/rules2/EndpointProviderSpec2.java b/codegen/src/main/java/software/amazon/awssdk/codegen/poet/rules2/EndpointProviderSpec2.java index 831b8d88af83..55dbf5358842 100644 --- a/codegen/src/main/java/software/amazon/awssdk/codegen/poet/rules2/EndpointProviderSpec2.java +++ b/codegen/src/main/java/software/amazon/awssdk/codegen/poet/rules2/EndpointProviderSpec2.java @@ -229,10 +229,12 @@ private MethodSpec.Builder methodBuilderForRule(RuleSetExpression expr) { } private void codegenExpr(RuleSetExpression expr, CodeBlock.Builder builder) { + boolean useEndpointCaching = intermediateModel.getCustomizationConfig().getEnableEndpointProviderUriCaching(); CodeGeneratorVisitor visitor = new CodeGeneratorVisitor(typeMirror, utils.symbolTable(), knownEndpointAttributes, utils.scopesByName(), + useEndpointCaching, builder); visitor.visitRuleSetExpression(expr); } diff --git a/codegen/src/test/java/software/amazon/awssdk/codegen/poet/ClientTestModels.java b/codegen/src/test/java/software/amazon/awssdk/codegen/poet/ClientTestModels.java index 36360b4b3b37..5bb2a8baa495 100644 --- a/codegen/src/test/java/software/amazon/awssdk/codegen/poet/ClientTestModels.java +++ b/codegen/src/test/java/software/amazon/awssdk/codegen/poet/ClientTestModels.java @@ -39,10 +39,10 @@ public static IntermediateModel awsJsonServiceModels() { File customizationModel = new File(ClientTestModels.class.getResource("client/c2j/json/customization.config").getFile()); File paginatorsModel = new File(ClientTestModels.class.getResource("client/c2j/json/paginators.json").getFile()); C2jModels models = C2jModels.builder() - .serviceModel(getServiceModel(serviceModel)) - .customizationConfig(getCustomizationConfig(customizationModel)) - .paginatorsModel(getPaginatorsModel(paginatorsModel)) - .build(); + .serviceModel(getServiceModel(serviceModel)) + .customizationConfig(getCustomizationConfig(customizationModel)) + .paginatorsModel(getPaginatorsModel(paginatorsModel)) + .build(); return new IntermediateModelBuilder(models).build(); } @@ -136,13 +136,13 @@ public static IntermediateModel queryServiceModels() { new File(ClientTestModels.class.getResource("client/c2j/query/endpoint-tests.json").getFile()); C2jModels models = C2jModels - .builder() - .serviceModel(getServiceModel(serviceModel)) - .customizationConfig(getCustomizationConfig(customizationModel)) - .waitersModel(getWaiters(waitersModel)) + .builder() + .serviceModel(getServiceModel(serviceModel)) + .customizationConfig(getCustomizationConfig(customizationModel)) + .waitersModel(getWaiters(waitersModel)) .endpointRuleSetModel(getEndpointRuleSet(endpointRuleSetModel)) .endpointTestSuiteModel(getEndpointTestSuite(endpointTestsModel)) - .build(); + .build(); return new IntermediateModelBuilder(models).build(); } @@ -206,6 +206,28 @@ public static IntermediateModel queryServiceModelsWithUnknownEndpointProperties( return new IntermediateModelBuilder(models).build(); } + public static IntermediateModel queryServiceModelsWithUriCache() { + File serviceModel = new File(ClientTestModels.class.getResource("client/c2j/query/service-2.json").getFile()); + File customizationModel = + new File(ClientTestModels.class.getResource("client/c2j/query/customization-uri-cache.config").getFile()); + File waitersModel = new File(ClientTestModels.class.getResource("client/c2j/query/waiters-2.json").getFile()); + File endpointRuleSetModel = + new File(ClientTestModels.class.getResource("client/c2j/query/endpoint-rule-set.json").getFile()); + File endpointTestsModel = + new File(ClientTestModels.class.getResource("client/c2j/query/endpoint-tests.json").getFile()); + + C2jModels models = C2jModels + .builder() + .serviceModel(getServiceModel(serviceModel)) + .customizationConfig(getCustomizationConfig(customizationModel)) + .waitersModel(getWaiters(waitersModel)) + .endpointRuleSetModel(getEndpointRuleSet(endpointRuleSetModel)) + .endpointTestSuiteModel(getEndpointTestSuite(endpointTestsModel)) + .build(); + + return new IntermediateModelBuilder(models).build(); + } + public static IntermediateModel queryServiceModelsEndpointAuthParamsWithAllowList() { File serviceModel = new File(ClientTestModels.class.getResource("client/c2j/query/service-2.json").getFile()); File customizationModel = @@ -458,9 +480,9 @@ public static IntermediateModel customContentTypeModels() { File customizationModel = new File(ClientTestModels.class.getResource("client/c2j/customservicemetadata/customization.config").getFile()); C2jModels models = C2jModels.builder() - .serviceModel(getServiceModel(serviceModel)) - .customizationConfig(getCustomizationConfig(customizationModel)) - .build(); + .serviceModel(getServiceModel(serviceModel)) + .customizationConfig(getCustomizationConfig(customizationModel)) + .build(); return new IntermediateModelBuilder(models).build(); } diff --git a/codegen/src/test/java/software/amazon/awssdk/codegen/poet/rules/EndpointProviderCompiledRulesClassSpecTest.java b/codegen/src/test/java/software/amazon/awssdk/codegen/poet/rules/EndpointProviderCompiledRulesClassSpecTest.java index 1b0961bc8b79..5bc9a1f74d75 100644 --- a/codegen/src/test/java/software/amazon/awssdk/codegen/poet/rules/EndpointProviderCompiledRulesClassSpecTest.java +++ b/codegen/src/test/java/software/amazon/awssdk/codegen/poet/rules/EndpointProviderCompiledRulesClassSpecTest.java @@ -44,4 +44,11 @@ void unknownEndpointProperties() { new EndpointProviderSpec2(ClientTestModels.queryServiceModelsWithUnknownEndpointProperties()); assertThat(endpointProviderSpec, generatesTo("endpoint-provider-unknown-property-class.java")); } + + @Test + void endpointProviderClassWithUriCache() { + ClassSpec endpointProviderSpec = + new EndpointProviderSpec2(ClientTestModels.queryServiceModelsWithUriCache()); + assertThat(endpointProviderSpec, generatesTo("endpoint-provider-uri-cache-class.java")); + } } diff --git a/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/c2j/query/customization-uri-cache.config b/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/c2j/query/customization-uri-cache.config new file mode 100644 index 000000000000..75393cba13cf --- /dev/null +++ b/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/c2j/query/customization-uri-cache.config @@ -0,0 +1,37 @@ +{ + "authPolicyActions" : { + "skip" : true + }, + "skipEndpointTests": { + "test case 4": "Does not work" + }, + "endpointParameters": { + "CustomEndpointArray": { + "required": false, + "documentation": "Parameter from the customization config", + "type": "StringArray" + }, + "ArnList": { + "required": false, + "documentation": "Parameter from the customization config", + "type": "StringArray" + } + }, + "customOperationContextParams": [ + { + "operationName": "OperationWithCustomizedOperationContextParam", + "operationContextParamsMap": { + "customEndpointArray": { + "path": "ListMember.StringList[*].LeafString" + } + } + } + ], + "preClientExecutionRequestCustomizer": { + "OperationWithCustomMember": { + "methodName": "dummyRequestModifier", + "className": "software.amazon.awssdk.codegen.internal.UtilsTest" + } + }, + "enableEndpointProviderUriCaching": true +} diff --git a/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/rules2/endpoint-provider-uri-cache-class.java b/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/rules2/endpoint-provider-uri-cache-class.java new file mode 100644 index 000000000000..53eb66b3c147 --- /dev/null +++ b/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/rules2/endpoint-provider-uri-cache-class.java @@ -0,0 +1,155 @@ +package software.amazon.awssdk.services.query.endpoints.internal; + +import java.util.Arrays; +import java.util.concurrent.CompletableFuture; +import software.amazon.awssdk.annotations.Generated; +import software.amazon.awssdk.annotations.SdkInternalApi; +import software.amazon.awssdk.awscore.endpoints.AwsEndpointAttribute; +import software.amazon.awssdk.awscore.endpoints.authscheme.SigV4AuthScheme; +import software.amazon.awssdk.awscore.endpoints.authscheme.SigV4aAuthScheme; +import software.amazon.awssdk.core.exception.SdkClientException; +import software.amazon.awssdk.endpoints.Endpoint; +import software.amazon.awssdk.regions.Region; +import software.amazon.awssdk.services.query.endpoints.QueryEndpointParams; +import software.amazon.awssdk.services.query.endpoints.QueryEndpointProvider; +import software.amazon.awssdk.utils.CompletableFutureUtils; +import software.amazon.awssdk.utils.Validate; +import software.amazon.awssdk.utils.uri.SdkUri; + +@Generated("software.amazon.awssdk:codegen") +@SdkInternalApi +public final class DefaultQueryEndpointProvider implements QueryEndpointProvider { + @Override + public CompletableFuture resolveEndpoint(QueryEndpointParams params) { + Validate.notNull(params.region(), "Parameter 'region' must not be null"); + try { + Region region = params.region(); + String regionId = region == null ? null : region.id(); + RuleResult result = endpointRule0(params, regionId); + if (result.canContinue()) { + throw SdkClientException.create("Rule engine did not reach an error or endpoint result"); + } + if (result.isError()) { + String errorMsg = result.error(); + if (errorMsg.contains("Invalid ARN") && errorMsg.contains(":s3:::")) { + errorMsg += ". Use the bucket name instead of simple bucket ARNs in GetBucketLocationRequest."; + } + throw SdkClientException.create(errorMsg); + } + return CompletableFuture.completedFuture(result.endpoint()); + } catch (Exception error) { + return CompletableFutureUtils.failedFuture(error); + } + } + + private static RuleResult endpointRule0(QueryEndpointParams params, String region) { + return endpointRule1(params, region); + } + + private static RuleResult endpointRule1(QueryEndpointParams params, String region) { + RulePartition partitionResult = RulesFunctions.awsPartition(region); + if (partitionResult != null) { + RuleResult result = endpointRule2(params, partitionResult); + if (result.isResolved()) { + return result; + } + result = endpointRule6(params, region, partitionResult); + if (result.isResolved()) { + return result; + } + return RuleResult.error(region + " is not a valid HTTP host-label"); + if (params.useFipsEndpoint() == null && params.useDualStackEndpoint() != null && params.useDualStackEndpoint() + && params.arnList() != null) { + String firstArn = RulesFunctions.listAccess(params.arnList(), 0); + if (firstArn != null) { + RuleArn parsedArn = RulesFunctions.awsParseArn(firstArn); + if (parsedArn != null) { + return RuleResult.endpoint(Endpoint + .builder() + .url(SdkUri.getInstance().create( + "https://" + params.endpointId() + ".query." + partitionResult.dualStackDnsSuffix())) + .putAttribute( + AwsEndpointAttribute.AUTH_SCHEMES, + Arrays.asList(SigV4aAuthScheme.builder().signingName("query") + .signingRegionSet(Arrays.asList("*")).build())).build()); + } + } + } + } + return RuleResult.carryOn(); + } + + private static RuleResult endpointRule2(QueryEndpointParams params, RulePartition partitionResult) { + if (params.endpointId() != null) { + if (params.useFipsEndpoint() != null && params.useFipsEndpoint()) { + return RuleResult.error("FIPS endpoints not supported with multi-region endpoints"); + } + if (params.useFipsEndpoint() == null && params.useDualStackEndpoint() != null && params.useDualStackEndpoint()) { + return RuleResult.endpoint(Endpoint + .builder() + .url(SdkUri.getInstance().create( + "https://" + params.endpointId() + ".query." + partitionResult.dualStackDnsSuffix())) + .putAttribute( + AwsEndpointAttribute.AUTH_SCHEMES, + Arrays.asList(SigV4aAuthScheme.builder().signingName("query") + .signingRegionSet(Arrays.asList("*")).build())).build()); + } + return RuleResult.endpoint(Endpoint + .builder() + .url(SdkUri.getInstance().create("https://" + params.endpointId() + ".query." + partitionResult.dnsSuffix())) + .putAttribute( + AwsEndpointAttribute.AUTH_SCHEMES, + Arrays.asList(SigV4aAuthScheme.builder().signingName("query").signingRegionSet(Arrays.asList("*")) + .build())).build()); + } + return RuleResult.carryOn(); + } + + private static RuleResult endpointRule6(QueryEndpointParams params, String region, RulePartition partitionResult) { + if (RulesFunctions.isValidHostLabel(region, false)) { + if (params.useFipsEndpoint() != null && params.useFipsEndpoint() && params.useDualStackEndpoint() == null) { + return RuleResult.endpoint(Endpoint + .builder() + .url(SdkUri.getInstance().create("https://query-fips." + region + "." + partitionResult.dnsSuffix())) + .putAttribute( + AwsEndpointAttribute.AUTH_SCHEMES, + Arrays.asList(SigV4aAuthScheme.builder().signingName("query") + .signingRegionSet(Arrays.asList("*")).build())).build()); + } + if (params.useDualStackEndpoint() != null && params.useDualStackEndpoint() && params.useFipsEndpoint() == null) { + return RuleResult.endpoint(Endpoint + .builder() + .url(SdkUri.getInstance().create("https://query." + region + "." + partitionResult.dualStackDnsSuffix())) + .putAttribute( + AwsEndpointAttribute.AUTH_SCHEMES, + Arrays.asList(SigV4aAuthScheme.builder().signingName("query") + .signingRegionSet(Arrays.asList("*")).build(), + SigV4AuthScheme.builder().signingName("query").signingRegion(region).build())).build()); + } + if (params.useDualStackEndpoint() != null && params.useFipsEndpoint() != null && params.useDualStackEndpoint() + && params.useFipsEndpoint()) { + return RuleResult.endpoint(Endpoint + .builder() + .url(SdkUri.getInstance().create( + "https://query-fips." + region + "." + partitionResult.dualStackDnsSuffix())) + .putAttribute( + AwsEndpointAttribute.AUTH_SCHEMES, + Arrays.asList(SigV4aAuthScheme.builder().signingName("query") + .signingRegionSet(Arrays.asList("*")).build())).build()); + } + return RuleResult.endpoint(Endpoint.builder() + .url(SdkUri.getInstance().create("https://query." + region + "." + partitionResult.dnsSuffix())).build()); + } + return RuleResult.carryOn(); + } + + @Override + public boolean equals(Object rhs) { + return rhs != null && getClass().equals(rhs.getClass()); + } + + @Override + public int hashCode() { + return getClass().hashCode(); + } +} diff --git a/core/auth-crt/src/main/java/software/amazon/awssdk/authcrt/signer/internal/CrtHttpRequestConverter.java b/core/auth-crt/src/main/java/software/amazon/awssdk/authcrt/signer/internal/CrtHttpRequestConverter.java index 89f3541348cd..2047e88f5bad 100644 --- a/core/auth-crt/src/main/java/software/amazon/awssdk/authcrt/signer/internal/CrtHttpRequestConverter.java +++ b/core/auth-crt/src/main/java/software/amazon/awssdk/authcrt/signer/internal/CrtHttpRequestConverter.java @@ -36,6 +36,7 @@ import software.amazon.awssdk.http.SdkHttpFullRequest; import software.amazon.awssdk.utils.StringUtils; import software.amazon.awssdk.utils.http.SdkHttpUtils; +import software.amazon.awssdk.utils.uri.SdkUri; @SdkInternalApi public final class CrtHttpRequestConverter { @@ -77,7 +78,7 @@ public SdkHttpFullRequest crtRequestToHttp(SdkHttpFullRequest inputRequest, Http String portString = SdkHttpUtils.isUsingStandardPort(builder.protocol(), builder.port()) ? "" : ":" + builder.port(); String encodedPath = encodedPathFromCrtFormat(inputRequest.encodedPath(), signedCrtRequest.getEncodedPath()); String fullUriString = builder.protocol() + "://" + builder.host() + portString + encodedPath; - fullUri = new URI(fullUriString); + fullUri = SdkUri.getInstance().newUri(fullUriString); } catch (URISyntaxException e) { return null; } diff --git a/core/aws-core/src/main/java/software/amazon/awssdk/awscore/endpoint/AwsClientEndpointProvider.java b/core/aws-core/src/main/java/software/amazon/awssdk/awscore/endpoint/AwsClientEndpointProvider.java index fbee6e0ec47f..0bb5119b369e 100644 --- a/core/aws-core/src/main/java/software/amazon/awssdk/awscore/endpoint/AwsClientEndpointProvider.java +++ b/core/aws-core/src/main/java/software/amazon/awssdk/awscore/endpoint/AwsClientEndpointProvider.java @@ -41,6 +41,7 @@ import software.amazon.awssdk.utils.ToString; import software.amazon.awssdk.utils.Validate; import software.amazon.awssdk.utils.internal.SystemSettingUtils; +import software.amazon.awssdk.utils.uri.SdkUri; /** * An implementation of {@link ClientEndpointProvider} that loads the default client endpoint from: @@ -238,7 +239,7 @@ private Optional clientEndpointFromServiceMetadata(Builder build .region(builder.region) .tags(endpointTags) .build()); - URI endpoint = URI.create(builder.protocol + "://" + endpointWithoutProtocol); + URI endpoint = SdkUri.getInstance().create(builder.protocol + "://" + endpointWithoutProtocol); if (endpoint.getHost() == null) { String error = "Configured region (" + builder.region + ") and tags (" + endpointTags + ") resulted in " + "an invalid URI: " + endpoint + ". This is usually caused by an invalid region " @@ -260,7 +261,7 @@ private Optional clientEndpointFromServiceMetadata(Builder build private Optional createUri(String source, Optional uri) { return uri.map(u -> { try { - URI parsedUri = new URI(uri.get()); + URI parsedUri = SdkUri.getInstance().newUri(uri.get()); log.trace(() -> "Client endpoint was loaded from the " + source + ": " + parsedUri); return parsedUri; } catch (URISyntaxException e) { diff --git a/core/http-auth-aws/src/main/java/software/amazon/awssdk/http/auth/aws/crt/internal/util/CrtHttpRequestConverter.java b/core/http-auth-aws/src/main/java/software/amazon/awssdk/http/auth/aws/crt/internal/util/CrtHttpRequestConverter.java index 0485cf128887..fd0bb010fd8f 100644 --- a/core/http-auth-aws/src/main/java/software/amazon/awssdk/http/auth/aws/crt/internal/util/CrtHttpRequestConverter.java +++ b/core/http-auth-aws/src/main/java/software/amazon/awssdk/http/auth/aws/crt/internal/util/CrtHttpRequestConverter.java @@ -31,6 +31,7 @@ import software.amazon.awssdk.http.auth.aws.crt.internal.io.CrtInputStream; import software.amazon.awssdk.utils.StringUtils; import software.amazon.awssdk.utils.http.SdkHttpUtils; +import software.amazon.awssdk.utils.uri.SdkUri; @SdkInternalApi public final class CrtHttpRequestConverter { @@ -73,7 +74,7 @@ public static SdkHttpRequest toRequest(SdkHttpRequest request, HttpRequest crtRe String portString = SdkHttpUtils.isUsingStandardPort(builder.protocol(), builder.port()) ? "" : ":" + builder.port(); String encodedPath = encodedPathFromCrtFormat(request.encodedPath(), crtRequest.getEncodedPath()); String fullUriString = builder.protocol() + "://" + builder.host() + portString + encodedPath; - fullUri = new URI(fullUriString); + fullUri = SdkUri.getInstance().newUri(fullUriString); } catch (URISyntaxException e) { throw new RuntimeException("Full URI could not be formed.", e); } diff --git a/core/regions/src/main/java/software/amazon/awssdk/regions/internal/util/ServiceMetadataUtils.java b/core/regions/src/main/java/software/amazon/awssdk/regions/internal/util/ServiceMetadataUtils.java index 87e1d9f89f37..19810aedfd43 100644 --- a/core/regions/src/main/java/software/amazon/awssdk/regions/internal/util/ServiceMetadataUtils.java +++ b/core/regions/src/main/java/software/amazon/awssdk/regions/internal/util/ServiceMetadataUtils.java @@ -25,6 +25,7 @@ import software.amazon.awssdk.utils.Pair; import software.amazon.awssdk.utils.StringUtils; import software.amazon.awssdk.utils.Validate; +import software.amazon.awssdk.utils.uri.SdkUri; @SdkInternalApi public class ServiceMetadataUtils { @@ -38,7 +39,8 @@ public static URI endpointFor(String hostname, String endpointPrefix, String region, String dnsSuffix) { - return URI.create(StringUtils.replaceEach(hostname, SEARCH_LIST, new String[] { endpointPrefix, region, dnsSuffix })); + return SdkUri.getInstance().create( + StringUtils.replaceEach(hostname, SEARCH_LIST, new String[] {endpointPrefix, region, dnsSuffix })); } public static Region signingRegion(ServiceEndpointKey key, diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/util/MetricUtils.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/util/MetricUtils.java index ed2c85f86943..d30e66692368 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/util/MetricUtils.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/util/MetricUtils.java @@ -38,6 +38,7 @@ import software.amazon.awssdk.metrics.NoOpMetricCollector; import software.amazon.awssdk.metrics.SdkMetric; import software.amazon.awssdk.utils.Pair; +import software.amazon.awssdk.utils.uri.SdkUri; /** * Utility methods for working with metrics. @@ -112,7 +113,8 @@ public static void collectServiceEndpointMetrics(MetricCollector metricCollector // Only interested in the service endpoint so don't include any path, query, or fragment component URI requestUri = httpRequest.getUri(); try { - URI serviceEndpoint = new URI(requestUri.getScheme(), requestUri.getAuthority(), null, null, null); + URI serviceEndpoint = SdkUri.getInstance().newUri( + requestUri.getScheme(), requestUri.getAuthority(), null, null, null); metricCollector.reportMetric(CoreMetric.SERVICE_ENDPOINT, serviceEndpoint); } catch (URISyntaxException e) { // This should not happen since getUri() should return a valid URI diff --git a/http-client-spi/src/main/java/software/amazon/awssdk/http/SdkHttpRequest.java b/http-client-spi/src/main/java/software/amazon/awssdk/http/SdkHttpRequest.java index bb34909b5f36..72bcea0299e6 100644 --- a/http-client-spi/src/main/java/software/amazon/awssdk/http/SdkHttpRequest.java +++ b/http-client-spi/src/main/java/software/amazon/awssdk/http/SdkHttpRequest.java @@ -29,6 +29,7 @@ import software.amazon.awssdk.utils.builder.CopyableBuilder; import software.amazon.awssdk.utils.builder.ToCopyableBuilder; import software.amazon.awssdk.utils.http.SdkHttpUtils; +import software.amazon.awssdk.utils.uri.SdkUri; /** * An immutable HTTP request without access to the request body. {@link SdkHttpFullRequest} should be used when access to a @@ -154,7 +155,7 @@ default URI getUri() { // Do not include the port in the URI when using the default port for the protocol. String portString = SdkHttpUtils.isUsingStandardPort(protocol(), port()) ? "" : ":" + port(); - return URI.create(protocol() + "://" + host() + portString + encodedPath() + encodedQueryString); + return SdkUri.getInstance().create(protocol() + "://" + host() + portString + encodedPath() + encodedQueryString); } /** diff --git a/http-clients/apache-client/src/main/java/software/amazon/awssdk/http/apache/internal/impl/ApacheHttpRequestFactory.java b/http-clients/apache-client/src/main/java/software/amazon/awssdk/http/apache/internal/impl/ApacheHttpRequestFactory.java index cfb22343ba3f..1c77e8738f3e 100644 --- a/http-clients/apache-client/src/main/java/software/amazon/awssdk/http/apache/internal/impl/ApacheHttpRequestFactory.java +++ b/http-clients/apache-client/src/main/java/software/amazon/awssdk/http/apache/internal/impl/ApacheHttpRequestFactory.java @@ -42,6 +42,7 @@ import software.amazon.awssdk.http.apache.internal.utils.ApacheUtils; import software.amazon.awssdk.utils.StringUtils; import software.amazon.awssdk.utils.http.SdkHttpUtils; +import software.amazon.awssdk.utils.uri.SdkUri; /** * Responsible for creating Apache HttpClient 4 request objects. @@ -80,7 +81,7 @@ private URI sanitizeUri(SdkHttpRequest request) { String portString = SdkHttpUtils.isUsingStandardPort(protocol, port) ? "" : ":" + port; - return URI.create(protocol + "://" + request.host() + portString + newPath + encodedQueryString); + return SdkUri.getInstance().create(protocol + "://" + request.host() + portString + newPath + encodedQueryString); } return request.getUri(); diff --git a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/AwsCrtHttpClientBase.java b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/AwsCrtHttpClientBase.java index 76af9bc6d8f2..2df865f0fa0b 100644 --- a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/AwsCrtHttpClientBase.java +++ b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/AwsCrtHttpClientBase.java @@ -44,6 +44,7 @@ import software.amazon.awssdk.utils.IoUtils; import software.amazon.awssdk.utils.Logger; import software.amazon.awssdk.utils.SdkAutoCloseable; +import software.amazon.awssdk.utils.uri.SdkUri; /** * Common functionality and configuration for the CRT Http clients. @@ -162,8 +163,8 @@ HttpClientConnectionManager getOrCreateConnectionPool(URI uri) { } URI poolKey(SdkHttpRequest sdkRequest) { - return invokeSafely(() -> new URI(sdkRequest.protocol(), null, sdkRequest.host(), - sdkRequest.port(), null, null, null)); + return invokeSafely(() -> SdkUri.getInstance().newUri(sdkRequest.protocol(), null, sdkRequest.host(), + sdkRequest.port(), null, null, null)); } @Override diff --git a/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/NettyNioAsyncHttpClient.java b/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/NettyNioAsyncHttpClient.java index b28b32da1df7..f8a5b99809ca 100644 --- a/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/NettyNioAsyncHttpClient.java +++ b/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/NettyNioAsyncHttpClient.java @@ -60,6 +60,7 @@ import software.amazon.awssdk.utils.AttributeMap; import software.amazon.awssdk.utils.Either; import software.amazon.awssdk.utils.Validate; +import software.amazon.awssdk.utils.uri.SdkUri; /** * An implementation of {@link SdkAsyncHttpClient} that uses a Netty non-blocking HTTP client to communicate with the service. @@ -169,8 +170,8 @@ private SdkEventLoopGroup eventLoopGroup(DefaultBuilder builder) { } private static URI poolKey(SdkHttpRequest sdkRequest) { - return invokeSafely(() -> new URI(sdkRequest.protocol(), null, sdkRequest.host(), - sdkRequest.port(), null, null, null)); + return invokeSafely(() -> SdkUri.getInstance().newUri(sdkRequest.protocol(), null, sdkRequest.host(), + sdkRequest.port(), null, null, null)); } private SslProvider resolveSslProvider(DefaultBuilder builder) { diff --git a/services/dynamodb/src/main/resources/codegen-resources/dynamodb/customization.config b/services/dynamodb/src/main/resources/codegen-resources/dynamodb/customization.config index a7b525068c07..b777570861ea 100644 --- a/services/dynamodb/src/main/resources/codegen-resources/dynamodb/customization.config +++ b/services/dynamodb/src/main/resources/codegen-resources/dynamodb/customization.config @@ -36,5 +36,6 @@ "customRetryStrategy" : "software.amazon.awssdk.services.dynamodb.DynamoDbRetryPolicy", "enableEndpointDiscoveryMethodRequired": true, "enableGenerateCompiledEndpointRules": true, - "enableFastUnmarshaller": false + "enableFastUnmarshaller": false, + "enableEndpointProviderUriCaching": true } diff --git a/utils/pom.xml b/utils/pom.xml index 3a27cef3997b..1fb7f6750f38 100644 --- a/utils/pom.xml +++ b/utils/pom.xml @@ -130,6 +130,11 @@ rxjava test + + nl.jqno.equalsverifier + equalsverifier + test + diff --git a/utils/src/main/java/software/amazon/awssdk/utils/uri/SdkUri.java b/utils/src/main/java/software/amazon/awssdk/utils/uri/SdkUri.java new file mode 100644 index 000000000000..4fe04b951ee9 --- /dev/null +++ b/utils/src/main/java/software/amazon/awssdk/utils/uri/SdkUri.java @@ -0,0 +1,294 @@ +/* + * 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.utils.uri; + +import java.net.URI; +import java.net.URISyntaxException; +import java.util.Objects; +import software.amazon.awssdk.annotations.SdkProtectedApi; +import software.amazon.awssdk.utils.Lazy; +import software.amazon.awssdk.utils.Logger; +import software.amazon.awssdk.utils.cache.lru.LruCache; +import software.amazon.awssdk.utils.uri.internal.UriConstructorArgs; + +/** + * Global cache for account-id based URI. Prevent calling new URI constructor for the same string, which can cause performance + * issues with some uri pattern. Do not directly depend on this class, it will be removed in the future. + */ +@SdkProtectedApi +public final class SdkUri { + private static final Logger log = Logger.loggerFor(SdkUri.class); + + private static final String HTTPS_PREFIX = "https://"; + private static final String HTTP_PREFIX = "http://"; + private static final int MAX_INT_DIGITS_BASE_10 = 10; + + /* + * The default LRUCache size is 100, but for a single service call we cache at least 3 different URIs so the cache size is + * increased a bit to account for the different URIs. + */ + private static final int CACHE_SIZE = 150; + + private static final Lazy INSTANCE = new Lazy<>(SdkUri::new); + + private final LruCache cache; + + private SdkUri() { + this.cache = LruCache.builder(UriConstructorArgs::newInstance) + .maxSize(CACHE_SIZE) + .build(); + } + + public static SdkUri getInstance() { + return INSTANCE.getValue(); + } + + public URI create(String s) { + if (!isAccountIdUri(s)) { + log.trace(() -> "skipping cache for uri " + s); + return URI.create(s); + } + StringConstructorArgs key = new StringConstructorArgs(s); + boolean containsK = cache.containsKey(key); + URI uri = cache.get(key); + logCacheUsage(containsK, uri); + return uri; + } + + public URI newUri(String s) throws URISyntaxException { + if (!isAccountIdUri(s)) { + log.trace(() -> "skipping cache for uri " + s); + return new URI(s); + } + try { + StringConstructorArgs key = new StringConstructorArgs(s); + boolean containsK = cache.containsKey(key); + URI uri = cache.get(key); + logCacheUsage(containsK, uri); + return uri; + } catch (IllegalArgumentException e) { + // URI.create() wraps the URISyntaxException thrown by new URI in a IllegalArgumentException, we need to unwrap it + if (e.getCause() instanceof URISyntaxException) { + throw (URISyntaxException) e.getCause(); + } + throw e; + } + } + + public URI newUri(String scheme, + String userInfo, String host, int port, + String path, String query, String fragment) throws URISyntaxException { + if (!isAccountIdUri(host)) { + log.trace(() -> "skipping cache for host " + host); + return new URI(scheme, userInfo, host, port, path, query, fragment); + } + try { + HostConstructorArgs key = new HostConstructorArgs(scheme, userInfo, host, port, path, query, fragment); + boolean containsK = cache.containsKey(key); + URI uri = cache.get(key); + logCacheUsage(containsK, uri); + return uri; + } catch (IllegalArgumentException e) { + if (e.getCause() instanceof URISyntaxException) { + throw (URISyntaxException) e.getCause(); + } + throw e; + } + } + + public URI newUri(String scheme, + String authority, + String path, String query, String fragment) throws URISyntaxException { + if (!isAccountIdUri(authority)) { + log.trace(() -> "skipping cache for authority " + authority); + return new URI(scheme, authority, path, query, fragment); + } + try { + AuthorityConstructorArgs key = new AuthorityConstructorArgs(scheme, authority, path, query, fragment); + boolean containsK = cache.containsKey(key); + URI uri = cache.get(key); + logCacheUsage(containsK, uri); + return uri; + } catch (IllegalArgumentException e) { + if (e.getCause() instanceof URISyntaxException) { + throw (URISyntaxException) e.getCause(); + } + throw e; + } + } + + /* + * Best-effort check for uri string being account-id based. + * + * The troublesome uris are of the form 'https://123456789012.ddb.us-east-1.amazonaws.com' The heuristic chosen to detect such + * candidate URI is to check the first char after the scheme, and then the char 10 places further down the string. If both + * are digits, there is a potential for that string to represent a number that would exceed the value of Integer.MAX_VALUE, + * which would cause the performance degradation observed with such URIs. + */ + private boolean isAccountIdUri(String s) { + int firstCharAfterScheme = 0; + if (s.startsWith(HTTPS_PREFIX)) { + firstCharAfterScheme = HTTPS_PREFIX.length(); + } else if (s.startsWith(HTTP_PREFIX)) { + firstCharAfterScheme = HTTP_PREFIX.length(); + } + + if (s.length() > firstCharAfterScheme + MAX_INT_DIGITS_BASE_10) { + return Character.isDigit(s.charAt(firstCharAfterScheme)) + && Character.isDigit(s.charAt(firstCharAfterScheme + MAX_INT_DIGITS_BASE_10)); + } + return false; + } + + private void logCacheUsage(boolean containsKey, URI uri) { + log.trace(() -> "URI cache size: " + cache.size()); + if (containsKey) { + log.trace(() -> "Using cached uri for " + uri.toString()); + } else { + log.trace(() -> "Cache empty for " + uri.toString()); + } + } + + private static final class StringConstructorArgs implements UriConstructorArgs { + private final String str; + + private StringConstructorArgs(String str) { + this.str = str; + } + + @Override + public URI newInstance() { + return URI.create(str); + } + + @Override + public boolean equals(Object o) { + if (o == null || getClass() != o.getClass()) { + return false; + } + + StringConstructorArgs that = (StringConstructorArgs) o; + return Objects.equals(str, that.str); + } + + @Override + public int hashCode() { + return Objects.hashCode(str); + } + } + + private static final class HostConstructorArgs implements UriConstructorArgs { + private final String scheme; + private final String userInfo; + private final String host; + private final int port; + private final String path; + private final String query; + private final String fragment; + + private HostConstructorArgs(String scheme, + String userInfo, String host, int port, + String path, String query, String fragment) { + this.scheme = scheme; + this.userInfo = userInfo; + this.host = host; + this.port = port; + this.path = path; + this.query = query; + this.fragment = fragment; + } + + @Override + public URI newInstance() { + try { + return new URI(scheme, userInfo, host, port, path, query, fragment); + } catch (URISyntaxException x) { + throw new IllegalArgumentException(x.getMessage(), x); + } + } + + @Override + public boolean equals(Object o) { + if (o == null || getClass() != o.getClass()) { + return false; + } + + HostConstructorArgs that = (HostConstructorArgs) o; + return port == that.port && Objects.equals(scheme, that.scheme) && Objects.equals(userInfo, that.userInfo) + && Objects.equals(host, that.host) && Objects.equals(path, that.path) && Objects.equals(query, that.query) + && Objects.equals(fragment, that.fragment); + } + + @Override + public int hashCode() { + int result = Objects.hashCode(scheme); + result = 31 * result + Objects.hashCode(userInfo); + result = 31 * result + Objects.hashCode(host); + result = 31 * result + port; + result = 31 * result + Objects.hashCode(path); + result = 31 * result + Objects.hashCode(query); + result = 31 * result + Objects.hashCode(fragment); + return result; + } + } + + private static final class AuthorityConstructorArgs implements UriConstructorArgs { + private final String scheme; + private final String authority; + private final String path; + private final String query; + private final String fragment; + + private AuthorityConstructorArgs(String scheme, String authority, String path, String query, String fragment) { + this.scheme = scheme; + this.authority = authority; + this.path = path; + this.query = query; + this.fragment = fragment; + } + + @Override + public URI newInstance() { + try { + return new URI(scheme, authority, path, query, fragment); + } catch (URISyntaxException x) { + throw new IllegalArgumentException(x.getMessage(), x); + } + } + + @Override + public boolean equals(Object o) { + if (o == null || getClass() != o.getClass()) { + return false; + } + + AuthorityConstructorArgs that = (AuthorityConstructorArgs) o; + return Objects.equals(scheme, that.scheme) && Objects.equals(authority, that.authority) + && Objects.equals(path, that.path) && Objects.equals(query, that.query) + && Objects.equals(fragment, that.fragment); + } + + @Override + public int hashCode() { + int result = Objects.hashCode(scheme); + result = 31 * result + Objects.hashCode(authority); + result = 31 * result + Objects.hashCode(path); + result = 31 * result + Objects.hashCode(query); + result = 31 * result + Objects.hashCode(fragment); + return result; + } + } +} \ No newline at end of file diff --git a/utils/src/main/java/software/amazon/awssdk/utils/uri/internal/UriConstructorArgs.java b/utils/src/main/java/software/amazon/awssdk/utils/uri/internal/UriConstructorArgs.java new file mode 100644 index 000000000000..86251e30d42d --- /dev/null +++ b/utils/src/main/java/software/amazon/awssdk/utils/uri/internal/UriConstructorArgs.java @@ -0,0 +1,38 @@ +/* + * 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.utils.uri.internal; + +import java.net.URI; +import software.amazon.awssdk.annotations.SdkInternalApi; + +/** + * Represent the different constructor to the URI class used by the SDK. Implementation of this interface are able to create new + * URIs based on the different arguments passed to classes to them. + * + * @see URI#create(String) + * @see URI#URI(String, String, String, String, String) + * @see URI#URI(String, String, String, int, String, String, String) + */ +@SdkInternalApi +public interface UriConstructorArgs { + + /** + * Creates a new instance of the URI. Can return a new instance everytime it is called. + * + * @return a new URI instance + */ + URI newInstance(); +} diff --git a/utils/src/test/java/software/amazon/awssdk/utils/SdkUriTest.java b/utils/src/test/java/software/amazon/awssdk/utils/SdkUriTest.java new file mode 100644 index 000000000000..c838041cf558 --- /dev/null +++ b/utils/src/test/java/software/amazon/awssdk/utils/SdkUriTest.java @@ -0,0 +1,305 @@ +/* + * 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.utils; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.junit.jupiter.api.Assertions.*; + +import java.lang.reflect.Field; +import java.net.URI; +import java.net.URISyntaxException; +import nl.jqno.equalsverifier.EqualsVerifier; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import org.junit.platform.commons.util.ReflectionUtils; +import org.opentest4j.AssertionFailedError; +import software.amazon.awssdk.utils.cache.lru.LruCache; +import software.amazon.awssdk.utils.uri.SdkUri; +import software.amazon.awssdk.utils.uri.internal.UriConstructorArgs; + +class SdkUriTest { + + @AfterEach + void resetCache() throws IllegalAccessException { + Field cacheField = getCacheField(); + cacheField.setAccessible(true); + cacheField.set(SdkUri.getInstance(), LruCache.builder(UriConstructorArgs::newInstance) + .maxSize(100) + .build()); + } + + @ParameterizedTest + @ValueSource(strings = {"https://123456789012.ddb.us-east-1.amazonaws.com", + "http://123456789012.ddb.us-east-1.amazonaws.com"}) + void multipleCreate_simpleURI_SameStringConstructor_ShouldCacheOnlyOnce(String strURI) { + URI uri = SdkUri.getInstance().create(strURI); + String scheme = strURI.startsWith("https") ? "https" : "http"; + assertThat(uri).hasHost("123456789012.ddb.us-east-1.amazonaws.com") + .hasScheme(scheme) + .hasNoParameters() + .hasNoPort() + .hasNoQuery(); + assertThat(getCache().size()).isEqualTo(1); + URI uri2 = SdkUri.getInstance().create(strURI); + assertThat(getCache().size()).isEqualTo(1); + assertThat(uri).isSameAs(uri2); + } + + @ParameterizedTest + @ValueSource(strings = {"http", "https"}) + void multipleCreate_FullUri_SameConstructor_ShouldCacheOnlyOne(String scheme) { + String strURI = scheme + "://123456789012.ddb.us-east-1.amazonaws.com:322/some/path?foo=bar#test"; + URI uri = SdkUri.getInstance().create(strURI); + assertThat(uri).hasHost("123456789012.ddb.us-east-1.amazonaws.com") + .hasScheme(scheme) + .hasNoUserInfo() + .hasPort(322) + .hasPath("/some/path") + .hasQuery("foo=bar") + .hasFragment("test"); + + assertThat(getCache().size()).isEqualTo(1); + URI uri2 = SdkUri.getInstance().create(strURI); + assertThat(getCache().size()).isEqualTo(1); + assertThat(uri).isSameAs(uri2); + + } + + @Test + void multipleCreate_withDifferentStringConstructor_shouldCacheOnlyOnce() { + String[] strURIs = { + "https://123456789012.ddb.us-east-1.amazonaws.com", + "https://123456789013.ddb.us-east-1.amazonaws.com", + "https://123456789014.ddb.us-east-1.amazonaws.com", + "https://123456789015.ddb.us-east-1.amazonaws.com", + "https://123456789016.ddb.us-east-1.amazonaws.com", + "https://123456789017.ddb.us-east-1.amazonaws.com", + "https://123456789018.ddb.us-east-1.amazonaws.com", + "https://123456789019.ddb.us-east-1.amazonaws.com", + }; + for (String uri : strURIs) { + URI u = SdkUri.getInstance().create(uri); + } + assertThat(getCache().size()).isEqualTo(8); + } + + @ParameterizedTest + @ValueSource(strings = {"http", "https"}) + void multipleNewUriWithNulls_SameAuthorityConstructor_ShouldCacheOnlyOnce(String scheme) throws URISyntaxException { + String strURI = "123456789012.ddb.us-east-1.amazonaws.com"; + URI uri = SdkUri.getInstance().newUri(scheme, strURI, null, null, null); + assertThat(uri).hasHost("123456789012.ddb.us-east-1.amazonaws.com") + .hasScheme(scheme) + .hasNoParameters() + .hasNoPort() + .hasNoQuery(); + assertThat(getCache().size()).isEqualTo(1); + URI uri2 = SdkUri.getInstance().newUri(scheme, strURI, null, null, null); + assertThat(getCache().size()).isEqualTo(1); + assertThat(uri).isSameAs(uri2); + } + + @ParameterizedTest + @ValueSource(strings = {"http", "https"}) + void multipleNewUri_SameAuthorityConstructor_ShouldCacheOnlyOnce(String scheme) throws URISyntaxException { + String strURI = "123456789012.ddb.us-east-1.amazonaws.com"; + URI uri = SdkUri.getInstance().newUri(scheme, strURI, "/somePath/to/resource", "foo=bar", "test"); + assertThat(uri).hasHost(strURI) + .hasPath("/somePath/to/resource") + .hasQuery("foo=bar") + .hasFragment("test") + .hasScheme(scheme); + assertThat(getCache().size()).isEqualTo(1); + URI uri2 = SdkUri.getInstance().newUri(scheme, strURI, "/somePath/to/resource", "foo=bar", "test"); + assertThat(getCache().size()).isEqualTo(1); + assertThat(uri).isSameAs(uri2); + } + + @ParameterizedTest + @ValueSource(strings = {"http", "https"}) + void multipleNewUri_DifferentAuthorityConstructor_ShouldCacheAll(String scheme) throws URISyntaxException { + String strURI = "123456789012.ddb.us-east-1.amazonaws.com"; + URI uri = SdkUri.getInstance().newUri(scheme, strURI, "/somePath/to/resource", "foo=bar", "test"); + assertThat(uri).hasHost(strURI) + .hasPath("/somePath/to/resource") + .hasQuery("foo=bar") + .hasFragment("test") + .hasScheme(scheme); + assertThat(getCache().size()).isEqualTo(1); + URI uri2 = SdkUri.getInstance().newUri(scheme, strURI, "/some/otherPath/to/resource", null, "test2"); + assertThat(getCache().size()).isEqualTo(2); + assertThat(uri).isNotSameAs(uri2); + } + + @ParameterizedTest + @ValueSource(strings = {"http", "https"}) + void multipleNewUriWithNulls_SameHostConstructor_ShouldCacheOnlyOnce(String scheme) throws URISyntaxException { + String strURI = "123456789012.ddb.us-east-1.amazonaws.com"; + URI uri = SdkUri.getInstance().newUri(scheme, null, strURI, 322, null, null, null); + assertThat(uri).hasHost("123456789012.ddb.us-east-1.amazonaws.com") + .hasNoParameters() + .hasPort(322) + .hasNoQuery(); + assertThat(getCache().size()).isEqualTo(1); + URI uri2 = SdkUri.getInstance().newUri(scheme, null, strURI, 322, null, null, null); + assertThat(getCache().size()).isEqualTo(1); + assertThat(uri).isSameAs(uri2); + } + + @ParameterizedTest + @ValueSource(strings = {"http", "https"}) + void multipleNewUri_SameHostConstructor_ShouldCacheOnlyOnce(String scheme) throws URISyntaxException { + String strURI = "123456789012.ddb.us-east-1.amazonaws.com"; + URI uri = SdkUri.getInstance().newUri(scheme, "user1", strURI, 322, "/some/path", "foo=bar", "test"); + assertThat(uri).hasHost("123456789012.ddb.us-east-1.amazonaws.com") + .hasScheme(scheme) + .hasUserInfo("user1") + .hasPort(322) + .hasPath("/some/path") + .hasQuery("foo=bar") + .hasFragment("test"); + assertThat(getCache().size()).isEqualTo(1); + URI uri2 = SdkUri.getInstance().newUri(scheme, "user1", strURI, 322, "/some/path", "foo=bar", "test"); + assertThat(getCache().size()).isEqualTo(1); + assertThat(uri).isSameAs(uri2); + } + + @ParameterizedTest + @ValueSource(strings = {"http", "https"}) + void multipleNewUri_DifferentHostConstructor_ShouldCacheOnlyOnce(String scheme) throws URISyntaxException { + String strURI = "123456789012.ddb.us-east-1.amazonaws.com"; + URI uri = SdkUri.getInstance().newUri(scheme, "user1", strURI, 322, "/some/path", "foo=bar", "test"); + assertThat(uri).hasHost("123456789012.ddb.us-east-1.amazonaws.com") + .hasScheme(scheme) + .hasUserInfo("user1") + .hasPort(322) + .hasPath("/some/path") + .hasQuery("foo=bar") + .hasFragment("test"); + assertThat(getCache().size()).isEqualTo(1); + URI uri2 = SdkUri.getInstance().newUri(scheme, "user1", strURI, 322, "/some/other/path", "foo=bar", "test2"); + assertThat(getCache().size()).isEqualTo(2); + assertThat(uri).isNotSameAs(uri2); + } + + @Test + void notCached_shouldCreateNewInstance() { + String strURI = "https://ddb.us-east-1.amazonaws.com"; + URI uri = SdkUri.getInstance().create(strURI); + assertThat(uri).hasHost("ddb.us-east-1.amazonaws.com") + .hasNoParameters() + .hasNoPort() + .hasNoQuery(); + assertThat(getCache().size()).isEqualTo(0); + URI uri2 = SdkUri.getInstance().create(strURI); + assertThat(getCache().size()).isEqualTo(0); + assertThat(uri).isNotSameAs(uri2); + } + + @ParameterizedTest + @ValueSource(strings = {"potatoes tomatoes", "123412341234 potatoes tomatoes"}) + void malformedURI_shouldThrowsSameExceptionAsUriClass(String malformedUri) { + + assertThatThrownBy(() -> SdkUri.getInstance().create(malformedUri)) + .as("Malformed uri should throw IllegalArgumentException using the create method") + .isInstanceOf(IllegalArgumentException.class); + assertThat(getCache().size()).as("Cache should be empty if create URI fails") + .isEqualTo(0); + + assertThatThrownBy(() -> SdkUri.getInstance().newUri(malformedUri)) + .as("Malformed uri should throw URISyntaxException using the newURI method") + .isInstanceOf(URISyntaxException.class); + assertThat(getCache().size()).as("Cache should be empty if create URI fails") + .isEqualTo(0); + + assertThatThrownBy(() -> SdkUri.getInstance().newUri("scheme", malformedUri, "path", "query", "fragment")) + .as("Malformed uri should throw URISyntaxException using the newURI with authority method") + .isInstanceOf(URISyntaxException.class); + assertThat(getCache().size()).as("Cache should be empty if create URI fails") + .isEqualTo(0); + + assertThatThrownBy(() -> new URI("scheme", malformedUri, "path", "query", "fragment")) + .as("CONSTRUCTOR") + .isInstanceOf(URISyntaxException.class); + assertThat(getCache().size()).as("Cache should be empty if create URI fails") + .isEqualTo(0); + + + assertThatThrownBy(() -> SdkUri.getInstance().newUri("scheme", "userInfo", malformedUri, + 444, "path", "query", "fragment")) + .as("Malformed uri should throw URISyntaxException using the newURI with host method") + .isInstanceOf(URISyntaxException.class); + assertThat(getCache().size()).as("Cache should be empty if create URI fails") + .isEqualTo(0); + } + + @ParameterizedTest + @ValueSource(strings = { + "http://123456789.ddb.com", + "https://123456789.ddb.com", + "123456789.ddb.com", + "http://123.ddb.com", + "https://123.ddb.com", + "123.ddb.com", + "http://123z.ddb.com", + "https://123z.ddb.com", + "123z.ddb.com", + "http://1", + "https://1", + "1", + "http://z", + "https://z", + "z" + }) + void shouldNotCache_whenLeadingDigitsDoNotExceedIntegerMaxValue(String strURI) { + URI uri = SdkUri.getInstance().create(strURI); + assertThat(getCache().size()).isEqualTo(0); + URI uri2 = SdkUri.getInstance().create(strURI); + assertThat(getCache().size()).isEqualTo(0); + assertThat(uri).isNotSameAs(uri2); + } + + + private LruCache getCache() { + Field field = getCacheField(); + field.setAccessible(true); + try { + return (LruCache) field.get(SdkUri.getInstance()); + } catch (IllegalAccessException e) { + fail(e); + return null; + } + } + + private Field getCacheField() { + return ReflectionUtils.streamFields(SdkUri.class, + f -> "cache".equals(f.getName()), + ReflectionUtils.HierarchyTraversalMode.TOP_DOWN) + .findFirst() + .orElseThrow(() -> new AssertionFailedError("Unexpected error - Could not find field " + + "'cache' in " + SdkUri.class.getName())); + } + + @Test + void equals_hashCode() { + EqualsVerifier.forPackage("software.amazon.awssdk.utils.uri") + .except(SdkUri.class) + .verify(); + } +} \ No newline at end of file From ba1b63fa2e0952006b9e6aede1a04e3fda547c62 Mon Sep 17 00:00:00 2001 From: hdavidh Date: Mon, 7 Jul 2025 22:14:58 -0700 Subject: [PATCH 2/4] Update SdkUri to use BoundedCache --- .../utils/cache/bounded/BoundedCache.java | 126 ++++++++++++++++++ .../amazon/awssdk/utils/uri/SdkUri.java | 12 +- .../amazon/awssdk/utils/SdkUriTest.java | 12 +- 3 files changed, 138 insertions(+), 12 deletions(-) create mode 100644 utils/src/main/java/software/amazon/awssdk/utils/cache/bounded/BoundedCache.java diff --git a/utils/src/main/java/software/amazon/awssdk/utils/cache/bounded/BoundedCache.java b/utils/src/main/java/software/amazon/awssdk/utils/cache/bounded/BoundedCache.java new file mode 100644 index 000000000000..37b22594c4df --- /dev/null +++ b/utils/src/main/java/software/amazon/awssdk/utils/cache/bounded/BoundedCache.java @@ -0,0 +1,126 @@ +/* + * 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.utils.cache.bounded; + +import java.util.Iterator; +import java.util.concurrent.ConcurrentHashMap; +import java.util.function.Function; +import software.amazon.awssdk.annotations.SdkProtectedApi; +import software.amazon.awssdk.annotations.ThreadSafe; +import software.amazon.awssdk.utils.Logger; +import software.amazon.awssdk.utils.Validate; + +/** + * A thread-safe cache implementation that returns the value for a specified key, + * retrieving it by either getting the stored value from the cache or using a supplied function to calculate that value + * and add it to the cache. + *

+ * When the cache is full, a new value will push out an unspecified value. + *

+ * The user can configure the maximum size of the cache, which is set to a default of 100. + *

+ * Null values are not cached. + */ +@SdkProtectedApi +@ThreadSafe +public final class BoundedCache { + + private static final Logger log = Logger.loggerFor(BoundedCache.class); + + private static final int DEFAULT_SIZE = 100; + + private final ConcurrentHashMap cache; + private final Function valueSupplier; + private final int maxCacheSize; + private final Object cacheLock; + + private BoundedCache(Builder builder) { + this.valueSupplier = builder.supplier; + this.maxCacheSize = builder.maxSize != null ? + Validate.isPositive(builder.maxSize, "maxSize") + : DEFAULT_SIZE; + this.cache = new ConcurrentHashMap<>(); + this.cacheLock = new Object(); + } + + /** + * Get a value based on the key. If the value exists in the cache, it's returned. + * Otherwise, the value is calculated based on the supplied function {@link Builder#builder(Function)}. + */ + public V get(K key) { + V value = cache.get(key); + if (value != null) { + return value; + } + + V newValue = valueSupplier.apply(key); + if (newValue == null) { + return null; + } + + synchronized (cacheLock) { + value = cache.get(key); + if (value != null) { + return value; + } + + if (cache.size() >= maxCacheSize) { + cleanup(); + } + + cache.put(key, newValue); + return newValue; + } + } + + /** + * Clean up the cache by removing an unspecified entry + */ + private void cleanup() { + Iterator iterator = cache.keySet().iterator(); + if (iterator.hasNext()) { + K key = iterator.next(); + cache.remove(key); + } + } + + public int size() { + return cache.size(); + } + + public static BoundedCache.Builder builder(Function supplier) { + return new Builder<>(supplier); + } + + public static final class Builder { + + private final Function supplier; + private Integer maxSize; + + private Builder(Function supplier) { + this.supplier = supplier; + } + + public Builder maxSize(Integer maxSize) { + this.maxSize = maxSize; + return this; + } + + public BoundedCache build() { + return new BoundedCache<>(this); + } + } +} \ No newline at end of file diff --git a/utils/src/main/java/software/amazon/awssdk/utils/uri/SdkUri.java b/utils/src/main/java/software/amazon/awssdk/utils/uri/SdkUri.java index 4fe04b951ee9..0b8e8d3a496f 100644 --- a/utils/src/main/java/software/amazon/awssdk/utils/uri/SdkUri.java +++ b/utils/src/main/java/software/amazon/awssdk/utils/uri/SdkUri.java @@ -21,7 +21,7 @@ import software.amazon.awssdk.annotations.SdkProtectedApi; import software.amazon.awssdk.utils.Lazy; import software.amazon.awssdk.utils.Logger; -import software.amazon.awssdk.utils.cache.lru.LruCache; +import software.amazon.awssdk.utils.cache.bounded.BoundedCache; import software.amazon.awssdk.utils.uri.internal.UriConstructorArgs; /** @@ -37,19 +37,19 @@ public final class SdkUri { private static final int MAX_INT_DIGITS_BASE_10 = 10; /* - * The default LRUCache size is 100, but for a single service call we cache at least 3 different URIs so the cache size is + * The default BoundedCache size is 100, but for a single service call we cache at least 3 different URIs so the cache size is * increased a bit to account for the different URIs. */ private static final int CACHE_SIZE = 150; private static final Lazy INSTANCE = new Lazy<>(SdkUri::new); - private final LruCache cache; + private final BoundedCache cache; private SdkUri() { - this.cache = LruCache.builder(UriConstructorArgs::newInstance) - .maxSize(CACHE_SIZE) - .build(); + this.cache = BoundedCache.builder(UriConstructorArgs::newInstance) + .maxSize(CACHE_SIZE) + .build(); } public static SdkUri getInstance() { diff --git a/utils/src/test/java/software/amazon/awssdk/utils/SdkUriTest.java b/utils/src/test/java/software/amazon/awssdk/utils/SdkUriTest.java index c838041cf558..2364f8278bdd 100644 --- a/utils/src/test/java/software/amazon/awssdk/utils/SdkUriTest.java +++ b/utils/src/test/java/software/amazon/awssdk/utils/SdkUriTest.java @@ -29,7 +29,7 @@ import org.junit.jupiter.params.provider.ValueSource; import org.junit.platform.commons.util.ReflectionUtils; import org.opentest4j.AssertionFailedError; -import software.amazon.awssdk.utils.cache.lru.LruCache; +import software.amazon.awssdk.utils.cache.bounded.BoundedCache; import software.amazon.awssdk.utils.uri.SdkUri; import software.amazon.awssdk.utils.uri.internal.UriConstructorArgs; @@ -39,9 +39,9 @@ class SdkUriTest { void resetCache() throws IllegalAccessException { Field cacheField = getCacheField(); cacheField.setAccessible(true); - cacheField.set(SdkUri.getInstance(), LruCache.builder(UriConstructorArgs::newInstance) - .maxSize(100) - .build()); + cacheField.set(SdkUri.getInstance(), BoundedCache.builder(UriConstructorArgs::newInstance) + .maxSize(100) + .build()); } @ParameterizedTest @@ -276,11 +276,11 @@ void shouldNotCache_whenLeadingDigitsDoNotExceedIntegerMaxValue(String strURI) { } - private LruCache getCache() { + private BoundedCache getCache() { Field field = getCacheField(); field.setAccessible(true); try { - return (LruCache) field.get(SdkUri.getInstance()); + return (BoundedCache) field.get(SdkUri.getInstance()); } catch (IllegalAccessException e) { fail(e); return null; From d170e9fb48fd8817ed6911812f79c2c2232f0a34 Mon Sep 17 00:00:00 2001 From: hdavidh Date: Mon, 7 Jul 2025 22:17:53 -0700 Subject: [PATCH 3/4] Update BoundedCache --- .../utils/cache/bounded/BoundedCache.java | 82 ++++++++++++------- .../amazon/awssdk/utils/uri/SdkUri.java | 4 +- .../amazon/awssdk/utils/SdkUriTest.java | 1 + 3 files changed, 54 insertions(+), 33 deletions(-) diff --git a/utils/src/main/java/software/amazon/awssdk/utils/cache/bounded/BoundedCache.java b/utils/src/main/java/software/amazon/awssdk/utils/cache/bounded/BoundedCache.java index 37b22594c4df..c488ed298d2b 100644 --- a/utils/src/main/java/software/amazon/awssdk/utils/cache/bounded/BoundedCache.java +++ b/utils/src/main/java/software/amazon/awssdk/utils/cache/bounded/BoundedCache.java @@ -17,101 +17,116 @@ import java.util.Iterator; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Function; import software.amazon.awssdk.annotations.SdkProtectedApi; import software.amazon.awssdk.annotations.ThreadSafe; -import software.amazon.awssdk.utils.Logger; import software.amazon.awssdk.utils.Validate; /** - * A thread-safe cache implementation that returns the value for a specified key, - * retrieving it by either getting the stored value from the cache or using a supplied function to calculate that value - * and add it to the cache. + * A thread-safe cache implementation that returns the value for a specified key, retrieving it by either getting the stored + * value from the cache or using a supplied function to calculate that value and add it to the cache. *

- * When the cache is full, a new value will push out an unspecified value. + * When the cache is full, batch eviction of random values will be performed, with a default evictionBatchSize of 10. *

- * The user can configure the maximum size of the cache, which is set to a default of 100. + * The user can configure the maximum size of the cache, which is set to a default of 150. *

- * Null values are not cached. + * Keys must not be null, otherwise an error will be thrown. Null values are not cached. */ @SdkProtectedApi @ThreadSafe public final class BoundedCache { - - private static final Logger log = Logger.loggerFor(BoundedCache.class); - - private static final int DEFAULT_SIZE = 100; + private static final int DEFAULT_CACHE_SIZE = 150; + private static final int DEFAULT_EVICTION_BATCH_SIZE = 10; private final ConcurrentHashMap cache; - private final Function valueSupplier; + private final Function valueMappingFunction; private final int maxCacheSize; + private final int evictionBatchSize; private final Object cacheLock; - - private BoundedCache(Builder builder) { - this.valueSupplier = builder.supplier; - this.maxCacheSize = builder.maxSize != null ? - Validate.isPositive(builder.maxSize, "maxSize") - : DEFAULT_SIZE; + private final AtomicInteger cacheSize; + + private BoundedCache(Builder b) { + this.valueMappingFunction = b.mappingFunction; + this.maxCacheSize = b.maxSize != null ? Validate.isPositive(b.maxSize, "maxSize") : DEFAULT_CACHE_SIZE; + this.evictionBatchSize = b.evictionBatchSize != null ? + Validate.isPositive(b.evictionBatchSize, "evictionBatchSize") : + DEFAULT_EVICTION_BATCH_SIZE; this.cache = new ConcurrentHashMap<>(); this.cacheLock = new Object(); + this.cacheSize = new AtomicInteger(); } /** - * Get a value based on the key. If the value exists in the cache, it's returned. + * Get a value based on the key. The key must not be null, otherwise an error is thrown. + * If the value exists in the cache, it's returned. * Otherwise, the value is calculated based on the supplied function {@link Builder#builder(Function)}. */ public V get(K key) { + Validate.paramNotNull(key, "key"); V value = cache.get(key); if (value != null) { return value; } - V newValue = valueSupplier.apply(key); + V newValue = valueMappingFunction.apply(key); + + // If the value is null, just return it without caching if (newValue == null) { return null; } synchronized (cacheLock) { + // Check again inside the synchronized block in case another thread added the value value = cache.get(key); if (value != null) { return value; } - if (cache.size() >= maxCacheSize) { + if (cacheSize.get() >= maxCacheSize) { cleanup(); } cache.put(key, newValue); + cacheSize.incrementAndGet(); return newValue; } } /** - * Clean up the cache by removing an unspecified entry + * Clean up the cache by batch removing random entries of evictionBatchSize */ private void cleanup() { Iterator iterator = cache.keySet().iterator(); - if (iterator.hasNext()) { - K key = iterator.next(); - cache.remove(key); + int count = 0; + while (iterator.hasNext() && count < evictionBatchSize) { + iterator.next(); + iterator.remove(); + count++; + cacheSize.decrementAndGet(); } } public int size() { - return cache.size(); + return cacheSize.get(); } - public static BoundedCache.Builder builder(Function supplier) { - return new Builder<>(supplier); + public boolean containsKey(K key) { + return cache.containsKey(key); + } + + public static BoundedCache.Builder builder(Function mappingFunction) { + return new Builder<>(mappingFunction); } public static final class Builder { - private final Function supplier; + private final Function mappingFunction; private Integer maxSize; + private Integer evictionBatchSize; - private Builder(Function supplier) { - this.supplier = supplier; + private Builder(Function mappingFunction) { + this.mappingFunction = mappingFunction; } public Builder maxSize(Integer maxSize) { @@ -119,6 +134,11 @@ public Builder maxSize(Integer maxSize) { return this; } + public Builder evictionBatchSize(Integer evictionBatchSize) { + this.evictionBatchSize = evictionBatchSize; + return this; + } + public BoundedCache build() { return new BoundedCache<>(this); } diff --git a/utils/src/main/java/software/amazon/awssdk/utils/uri/SdkUri.java b/utils/src/main/java/software/amazon/awssdk/utils/uri/SdkUri.java index 0b8e8d3a496f..3402124ca798 100644 --- a/utils/src/main/java/software/amazon/awssdk/utils/uri/SdkUri.java +++ b/utils/src/main/java/software/amazon/awssdk/utils/uri/SdkUri.java @@ -37,8 +37,8 @@ public final class SdkUri { private static final int MAX_INT_DIGITS_BASE_10 = 10; /* - * The default BoundedCache size is 100, but for a single service call we cache at least 3 different URIs so the cache size is - * increased a bit to account for the different URIs. + * Same value as default BoundedCache size. This contrasts to the default LruCache size of 100, since for a single service + * call we cache at least 3 different URIs, so the cache size is increased a bit to account for the different URIs. */ private static final int CACHE_SIZE = 150; diff --git a/utils/src/test/java/software/amazon/awssdk/utils/SdkUriTest.java b/utils/src/test/java/software/amazon/awssdk/utils/SdkUriTest.java index 2364f8278bdd..700d34649e88 100644 --- a/utils/src/test/java/software/amazon/awssdk/utils/SdkUriTest.java +++ b/utils/src/test/java/software/amazon/awssdk/utils/SdkUriTest.java @@ -41,6 +41,7 @@ void resetCache() throws IllegalAccessException { cacheField.setAccessible(true); cacheField.set(SdkUri.getInstance(), BoundedCache.builder(UriConstructorArgs::newInstance) .maxSize(100) + .evictionBatchSize(5) .build()); } From ec1c5b679a1a832d5997c628c18102b7aa9b8abe Mon Sep 17 00:00:00 2001 From: hdavidh Date: Tue, 8 Jul 2025 17:15:54 -0700 Subject: [PATCH 4/4] Address comments --- .changes/next-release/feature-AWSSDKforJavav2-b405876.json | 4 ++-- .changes/next-release/feature-AWSSDKforJavav2-f6d5c46.json | 6 ++++++ .../amazon/awssdk/utils/cache/bounded/BoundedCache.java | 4 ++-- 3 files changed, 10 insertions(+), 4 deletions(-) create mode 100644 .changes/next-release/feature-AWSSDKforJavav2-f6d5c46.json diff --git a/.changes/next-release/feature-AWSSDKforJavav2-b405876.json b/.changes/next-release/feature-AWSSDKforJavav2-b405876.json index bb9d5276a1ac..50051af11976 100644 --- a/.changes/next-release/feature-AWSSDKforJavav2-b405876.json +++ b/.changes/next-release/feature-AWSSDKforJavav2-b405876.json @@ -1,6 +1,6 @@ { "type": "feature", - "category": "Amazon DyanmoDB", + "category": "Amazon DynamoDB", "contributor": "", - "description": "Enable caching calls to URI constructors for account-id based endpoints" + "description": "Enable caching results to URI constructors for account-id based endpoints" } diff --git a/.changes/next-release/feature-AWSSDKforJavav2-f6d5c46.json b/.changes/next-release/feature-AWSSDKforJavav2-f6d5c46.json new file mode 100644 index 000000000000..8202978b2df3 --- /dev/null +++ b/.changes/next-release/feature-AWSSDKforJavav2-f6d5c46.json @@ -0,0 +1,6 @@ +{ + "type": "feature", + "category": "AWS SDK for Java v2", + "contributor": "", + "description": "Add support for caching results to URI constructors for account-id based endpoints" +} diff --git a/utils/src/main/java/software/amazon/awssdk/utils/cache/bounded/BoundedCache.java b/utils/src/main/java/software/amazon/awssdk/utils/cache/bounded/BoundedCache.java index c488ed298d2b..88c9a7da4af2 100644 --- a/utils/src/main/java/software/amazon/awssdk/utils/cache/bounded/BoundedCache.java +++ b/utils/src/main/java/software/amazon/awssdk/utils/cache/bounded/BoundedCache.java @@ -27,7 +27,7 @@ * A thread-safe cache implementation that returns the value for a specified key, retrieving it by either getting the stored * value from the cache or using a supplied function to calculate that value and add it to the cache. *

- * When the cache is full, batch eviction of random values will be performed, with a default evictionBatchSize of 10. + * When the cache is full, batch eviction of unspecified values will be performed, with a default evictionBatchSize of 10. *

* The user can configure the maximum size of the cache, which is set to a default of 150. *

@@ -94,7 +94,7 @@ public V get(K key) { } /** - * Clean up the cache by batch removing random entries of evictionBatchSize + * Clean up the cache by batch removing unspecified entries of evictionBatchSize */ private void cleanup() { Iterator iterator = cache.keySet().iterator();