diff --git a/codegen/src/main/java/software/amazon/awssdk/codegen/poet/rules/EndpointParamsKnowledgeIndex.java b/codegen/src/main/java/software/amazon/awssdk/codegen/poet/rules/EndpointParamsKnowledgeIndex.java index 7d200aeae0b5..4f1914000833 100644 --- a/codegen/src/main/java/software/amazon/awssdk/codegen/poet/rules/EndpointParamsKnowledgeIndex.java +++ b/codegen/src/main/java/software/amazon/awssdk/codegen/poet/rules/EndpointParamsKnowledgeIndex.java @@ -191,7 +191,11 @@ public MethodSpec recordAccountIdEndpointModeMethod() { + ".ifPresent(m -> executionAttributes.getAttribute($T.BUSINESS_METRICS).addMetric(m))", BusinessMetricsUtils.class, SdkInternalExecutionAttribute.class); - builder.addStatement("return mode.name().toLowerCase()"); + // Use endpointModeValue() rather than name().toLowerCase() so that the returned String is an interned + // compile-time literal. That keeps the reference stable across calls and removes a per-request allocation. + // It also avoids name().toLowerCase()'s dependence on the default locale, which mangles the value under a + // Turkish locale. + builder.addStatement("return mode.endpointModeValue()"); return builder.build(); } diff --git a/codegen/src/main/java/software/amazon/awssdk/codegen/poet/rules/EndpointResolverUtilsSpec.java b/codegen/src/main/java/software/amazon/awssdk/codegen/poet/rules/EndpointResolverUtilsSpec.java index 84677fcae0cd..cd49ec926a5e 100644 --- a/codegen/src/main/java/software/amazon/awssdk/codegen/poet/rules/EndpointResolverUtilsSpec.java +++ b/codegen/src/main/java/software/amazon/awssdk/codegen/poet/rules/EndpointResolverUtilsSpec.java @@ -22,11 +22,13 @@ import com.fasterxml.jackson.jr.stree.JrsString; import com.squareup.javapoet.ClassName; import com.squareup.javapoet.CodeBlock; +import com.squareup.javapoet.FieldSpec; import com.squareup.javapoet.MethodSpec; import com.squareup.javapoet.ParameterizedTypeName; import com.squareup.javapoet.TypeName; import com.squareup.javapoet.TypeSpec; import com.squareup.javapoet.TypeVariableName; +import java.util.Collections; import java.util.Iterator; import java.util.List; import java.util.Locale; @@ -34,6 +36,8 @@ import java.util.Objects; import java.util.Optional; import java.util.Set; +import java.util.stream.Collectors; +import java.util.stream.Stream; import javax.lang.model.element.Modifier; import software.amazon.awssdk.annotations.SdkInternalApi; import software.amazon.awssdk.awscore.AwsExecutionAttribute; @@ -119,6 +123,7 @@ public TypeSpec poetSpec() { b.addMethod(setStaticContextParamsMethod()); addStaticContextParamMethods(b); + addStaticListFields(b); b.addMethod(authSchemeWithEndpointSignerPropertiesMethod()); @@ -342,9 +347,8 @@ private MethodSpec addStaticContextParamsMethod(OperationModel opModel) { b.addStatement("params.$N($L)", setterName, ((JrsBoolean) value).booleanValue()); break; case START_ARRAY: - JrsArray arrayValue = (JrsArray) value; - CodeBlock arrayCode = endpointRulesSpecUtils.treeNodeToLiteral(arrayValue); - b.addStatement("params.$N($L)", setterName, arrayCode); + String fieldName = staticListFieldName(opModel, n); + b.addStatement("params.$N($N)", setterName, fieldName); break; default: throw new RuntimeException("Don't know how to set parameter of type " + value.asToken()); @@ -358,6 +362,57 @@ private String staticContextParamsMethodName(OperationModel opModel) { return opModel.getMethodName() + "StaticContextParams"; } + /** + * Generates the name of the {@code static final List} field holding the static array value of + * {@code paramName} for {@code opModel}. + * + *

Format: {@code STATIC_LIST_{OPERATION}_{PARAM}} + */ + private static String staticListFieldName(OperationModel opModel, String paramName) { + return "STATIC_LIST_" + screamCase(opModel.getOperationName()) + "_" + screamCase(paramName); + } + + private static String screamCase(String word) { + return Stream.of(CodegenNamingUtils.splitOnWordBoundaries(word)) + .map(s -> s.toUpperCase(Locale.US)) + .collect(Collectors.joining("_")); + } + + /** + * Generates a {@code private static final List} field for every {@code staticContextParams} entry whose + * value is an array, so that {@code setStaticContextParams} hands the same list reference to the endpoint params + * builder on every call rather than constructing a new list each time. + */ + private void addStaticListFields(TypeSpec.Builder classBuilder) { + ParameterizedTypeName listOfString = ParameterizedTypeName.get(List.class, String.class); + + model.getOperations().forEach((opName, opModel) -> { + Map statics = opModel.getStaticContextParams(); + if (CollectionUtils.isNullOrEmpty(statics)) { + return; + } + statics.forEach((paramName, scp) -> { + TreeNode value = scp.getValue(); + if (value.asToken() != JsonToken.START_ARRAY) { + return; + } + JrsArray arrayValue = (JrsArray) value; + CodeBlock initializer; + if (arrayValue.size() == 0) { + initializer = CodeBlock.of("$T.emptyList()", Collections.class); + } else { + initializer = CodeBlock.of("$T.unmodifiableList($L)", Collections.class, + endpointRulesSpecUtils.treeNodeToLiteral(arrayValue)); + } + FieldSpec field = FieldSpec.builder(listOfString, staticListFieldName(opModel, paramName), + Modifier.PRIVATE, Modifier.STATIC, Modifier.FINAL) + .initializer(initializer) + .build(); + classBuilder.addField(field); + }); + }); + } + private boolean hasStaticContextParams(OperationModel opModel) { Map staticContextParams = opModel.getStaticContextParams(); return staticContextParams != null && !staticContextParams.isEmpty(); diff --git a/codegen/src/main/java/software/amazon/awssdk/codegen/poet/rules2/bdd/BddEndpointProviderSpec.java b/codegen/src/main/java/software/amazon/awssdk/codegen/poet/rules2/bdd/BddEndpointProviderSpec.java index 3fb181eaa637..614498045d0d 100644 --- a/codegen/src/main/java/software/amazon/awssdk/codegen/poet/rules2/bdd/BddEndpointProviderSpec.java +++ b/codegen/src/main/java/software/amazon/awssdk/codegen/poet/rules2/bdd/BddEndpointProviderSpec.java @@ -30,6 +30,7 @@ import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.Objects; import java.util.concurrent.CompletableFuture; import javax.lang.model.element.Modifier; import software.amazon.awssdk.annotations.SdkInternalApi; @@ -41,6 +42,7 @@ import software.amazon.awssdk.codegen.model.rules.endpoints.ConditionModel; import software.amazon.awssdk.codegen.model.rules.endpoints.ParameterModel; import software.amazon.awssdk.codegen.model.rules.endpoints.RuleModel; +import software.amazon.awssdk.codegen.model.service.ClientContextParam; import software.amazon.awssdk.codegen.model.service.EndpointBddModel; import software.amazon.awssdk.codegen.poet.ClassSpec; import software.amazon.awssdk.codegen.poet.PoetUtils; @@ -80,6 +82,20 @@ public class BddEndpointProviderSpec implements ClassSpec { */ private static final int NO_MATCH_RESULT = 100_000_000; + /** + * A {@code stringArray} cache key parameter longer than this reports a miss without comparing elements, so the cost + * of a cache check cannot grow with the size of the caller's list. + * + *

The cap is a cost bound, not a coverage target. Above it the provider resolves, which is what it would have + * done anyway, so the value trades hit rate against a bounded worst case and can never affect correctness. Four is + * arbitrary but deliberately conservative. + * + *

No shipped service needs this path today. DynamoDB's {@code ResourceArnList} is the only {@code stringArray} + * any shipped rule set declares, and it is read only at index 0, so it is compared by + * {@link #cacheFirstElementsMatchMethod()} with no cap at all. + */ + private static final int MAX_LIST_COMPARISON_SIZE = 4; + private final IntermediateModel intermediateModel; private final EndpointBddModel endpointBddModel; private final EndpointRulesSpecUtils endpointRulesSpecUtils; @@ -87,8 +103,10 @@ public class BddEndpointProviderSpec implements ClassSpec { private final RuleRuntimeTypeMirror typeMirror; private final Map registerInfoMap; private final ClassName evaluatorType; + private final ClassName cacheEntryType; private final List bddNodes; private final List conditionTypes; + private final Map paramUsage; public BddEndpointProviderSpec(IntermediateModel intermediateModel) { this.intermediateModel = intermediateModel; @@ -99,8 +117,10 @@ public BddEndpointProviderSpec(IntermediateModel intermediateModel) { this.knownEndpointAttributes = knownEndpointAttributes(intermediateModel); this.registerInfoMap = buildRegisterInfoMap(); this.evaluatorType = className().nestedClass("Evaluator"); + this.cacheEntryType = className().nestedClass("CacheEntry"); this.bddNodes = endpointBddModel.getDecodedNodes(); this.conditionTypes = analyzeConditions(); + this.paramUsage = BddParameterReferences.analyze(endpointBddModel); } @Override @@ -110,12 +130,273 @@ public TypeSpec poetSpec() { .addSuperinterface(endpointRulesSpecUtils.providerInterfaceName()) .addAnnotation(SdkInternalApi.class); + builder.addField(cacheField()); builder.addType(evaluatorClass()); + builder.addType(cacheEntryClass()); builder.addMethod(resolveEndpointMethod()); + builder.addMethod(cacheParamsMatchMethod()); + if (needsFullListHelper()) { + builder.addMethod(cacheListsMatchMethod()); + } + if (needsFirstElementHelper()) { + builder.addMethod(cacheFirstElementsMatchMethod()); + } return builder.build(); } + // ---- Single-entry result cache ---- + + /** + * Generates {@code private volatile CacheEntry cache;}. + * + *

One entry, holding the most recent successfully resolved {@code (params, endpoint)} pair. A single entry is + * enough because the overwhelmingly common shape is a client resolving the same endpoint repeatedly: same region, + * same flags, and for most services no request-derived parameters at all. A service whose endpoint genuinely varies + * per request simply misses every time and pays only the key check. + * + *

{@code volatile} is the whole of the synchronisation. Racing threads compute equivalent entries for equal + * params, so a lost write costs one re-resolution and nothing more; {@code CacheEntry} is immutable with final + * fields, so a thread that reads the reference sees fully initialised contents. + */ + private FieldSpec cacheField() { + return FieldSpec.builder(cacheEntryType, "cache") + .addModifiers(Modifier.PRIVATE, Modifier.VOLATILE) + .build(); + } + + /** + * Generates the immutable {@code CacheEntry} holding one {@code (params, endpoint)} snapshot. + * + *

The entry keeps the caller's params object rather than copying it, so it relies on that object and any + * collections it holds being effectively immutable after {@code build()}. Generated params do not copy list values + * ({@code this.resourceArnList = builder.resourceArnList;}), so the assumption is load-bearing rather than + * enforced. + * + *

It holds on every SDK path, because request objects are immutable and the params for a request are built and + * discarded within the call. It is an assumption only for a caller that invokes + * {@code EndpointProvider#resolveEndpoint} directly, retains a list it passed in, and mutates it afterwards: the + * mutation reaches the stored key, and a later call carrying the post-mutation contents can then hit an entry that + * was resolved for the pre-mutation contents. Snapshotting list parameters here would close that off, at the cost + * of an allocation on every miss. + */ + private TypeSpec cacheEntryClass() { + ClassName paramsClass = endpointRulesSpecUtils.parametersClassName(); + return TypeSpec.classBuilder(cacheEntryType) + .addModifiers(Modifier.PRIVATE, Modifier.STATIC, Modifier.FINAL) + .addField(paramsClass, "params", Modifier.FINAL) + .addField(Endpoint.class, "endpoint", Modifier.FINAL) + .addMethod(MethodSpec.constructorBuilder() + .addParameter(paramsClass, "params") + .addParameter(Endpoint.class, "endpoint") + .addStatement("this.params = params") + .addStatement("this.endpoint = endpoint") + .build()) + .build(); + } + + /** + * Generates {@code cacheParamsMatch(a, b)}: true when the two parameter objects are interchangeable as far as + * endpoint resolution is concerned. + * + *

One uniform {@link Objects#equals} term per parameter, joined with {@code &&} so the chain short-circuits on + * the first mismatch. {@code Objects.equals} tries identity before {@code equals}, which is what makes a single + * emitter sufficient: a parameter whose reference the SDK keeps stable settles on the identity check, and one that + * arrives as a fresh reference falls through to {@code equals} and still matches. + * + *

Parameter order comes from {@link #cacheKeyParameterOrder()}. Order is the only thing that varies between + * parameters, and it only affects how quickly a mismatch is found. + * + *

List-valued parameters are special cased to avoid slow comparisons for large lists. + * See {@link #cacheListsMatchMethod()}. + */ + private MethodSpec cacheParamsMatchMethod() { + ClassName paramsClass = endpointRulesSpecUtils.parametersClassName(); + Map parameters = endpointBddModel.getParameters(); + + MethodSpec.Builder b = MethodSpec.methodBuilder("cacheParamsMatch") + .addModifiers(Modifier.PRIVATE, Modifier.STATIC) + .returns(boolean.class) + .addParameter(paramsClass, "a") + .addParameter(paramsClass, "b"); + + CodeBlock.Builder chain = CodeBlock.builder().add("return "); + boolean first = true; + for (String paramName : cacheKeyParameterOrder()) { + String getter = endpointRulesSpecUtils.paramMethodName(paramName) + "()"; + if (!first) { + chain.add("\n && "); + } + if (!isListParam(parameters.get(paramName))) { + chain.add("$T.equals(a.$L, b.$L)", Objects.class, getter, getter); + } else if (paramUsage.get(paramName) == BddParameterReferences.Usage.FIRST_ELEMENT_ONLY) { + chain.add("cacheFirstElementsMatch(a.$L, b.$L)", getter, getter); + } else { + chain.add("cacheListsMatch(a.$L, b.$L)", getter, getter); + } + first = false; + } + if (first) { + // Either the model declares no parameters, or it reads none of them. Both mean one endpoint for every + // request, so any two parameter objects are interchangeable. + chain.add("true"); + } + b.addStatement(chain.build()); + return b.build(); + } + + /** + * Returns the parameter names in the order the generated cache key compares them: booleans, then strings whose + * reference the SDK keeps stable across requests, then everything else. Each group keeps the model's declaration + * order, so the result is deterministic across builds. + * + *

Ordering exists only to reach a mismatch sooner. It cannot change the outcome, because the chain compares + * every parameter before returning true. Booleans come first because they can never fall through to a real + * {@code equals}; reference-stable strings come next because they normally settle on the identity check; and the + * request-derived values that may have to compare characters come last. + */ + private List cacheKeyParameterOrder() { + Map parameters = endpointBddModel.getParameters(); + Map clientContextParams = intermediateModel.getClientContextParams(); + + List booleans = new ArrayList<>(); + List stableStrings = new ArrayList<>(); + List rest = new ArrayList<>(); + + parameters.forEach((name, model) -> { + if (paramUsage.get(name) == BddParameterReferences.Usage.UNREFERENCED) { + // Nothing reads it, so it cannot change the endpoint and must not force a miss. + return; + } + if (isBooleanParam(model)) { + booleans.add(name); + } else if (isReferenceStable(name, model, clientContextParams)) { + stableStrings.add(name); + } else { + rest.add(name); + } + }); + + List order = new ArrayList<>(booleans.size() + stableStrings.size() + rest.size()); + order.addAll(booleans); + order.addAll(stableStrings); + order.addAll(rest); + return order; + } + + /** + * Returns true for a string parameter the SDK hands to every request as the same reference: {@code AWS::Region}, + * which {@code Region.of} interns, and {@code clientContextParams}, which are read from the client's + * {@code AttributeMap}. + * + *

Only used to order the comparison. If one of these ever stops being reference-stable, the + * {@code Objects.equals} term still compares it correctly; the check simply costs an extra call. + */ + private static boolean isReferenceStable(String paramName, + ParameterModel model, + Map clientContextParams) { + if (model.getBuiltInEnum() == BuiltInParameter.AWS_REGION) { + return true; + } + if (clientContextParams == null) { + return false; + } + // Endpoint parameter names are unique case-insensitively, so a case-insensitive match is the same parameter. + for (String key : clientContextParams.keySet()) { + if (key.equalsIgnoreCase(paramName)) { + return true; + } + } + return false; + } + + /** + * Generates the {@code cacheListsMatch} helper, emitted only when the model declares a {@code stringArray} + * parameter. + * + *

{@code Objects.equals} would be correct here, but {@code List.equals} is unbounded: a request carrying a large + * list would walk every element on every cache check, which can cost more than the resolution the cache exists to + * avoid. The comparison is therefore capped at {@value #MAX_LIST_COMPARISON_SIZE} elements. A service handling + * longer lists simply misses and pays resolution, which is what it would have paid anyway. + */ + private MethodSpec cacheListsMatchMethod() { + TypeName listOfString = RuleRuntimeTypeMirror.LIST_OF_STRING.type(); + return MethodSpec.methodBuilder("cacheListsMatch") + .addModifiers(Modifier.PRIVATE, Modifier.STATIC) + .returns(boolean.class) + .addParameter(listOfString, "a") + .addParameter(listOfString, "b") + .addStatement("if (a == b) return true") + .addStatement("if (a == null || b == null) return false") + .addStatement("int size = a.size()") + .addStatement("if (size != b.size()) return false") + .addComment("Bounded so that a long list cannot make the cache check cost more than " + + "resolving.") + .addStatement("if (size > $L) return false", MAX_LIST_COMPARISON_SIZE) + .beginControlFlow("for (int i = 0; i < size; i++)") + .addStatement("if (!$T.equals(a.get(i), b.get(i))) return false", Objects.class) + .endControlFlow() + .addStatement("return true") + .build(); + } + + /** + * Generates the {@code cacheFirstElementsMatch} helper, emitted only when a {@code stringArray} parameter is read + * exclusively as {@code param[0]}. + * + *

When the rules can only see whether the list is present and what its first element is, comparing the rest is + * work that cannot change the answer. + */ + private MethodSpec cacheFirstElementsMatchMethod() { + TypeName listOfString = RuleRuntimeTypeMirror.LIST_OF_STRING.type(); + return MethodSpec.methodBuilder("cacheFirstElementsMatch") + .addModifiers(Modifier.PRIVATE, Modifier.STATIC) + .returns(boolean.class) + .addParameter(listOfString, "a") + .addParameter(listOfString, "b") + .addStatement("if (a == b) return true") + .addComment("isSet can tell an absent list from an empty one, so presence is part of the key.") + .addStatement("if (a == null || b == null) return false") + .addComment("Nothing past element 0 can reach the endpoint.") + .addStatement("$T firstA = a.isEmpty() ? null : a.get(0)", String.class) + .addStatement("$T firstB = b.isEmpty() ? null : b.get(0)", String.class) + .addStatement("return $T.equals(firstA, firstB)", Objects.class) + .build(); + } + + /** + * True when some list parameter in the cache key needs a whole-list comparison. + */ + private boolean needsFullListHelper() { + return listParamsInKeyWithUsage(BddParameterReferences.Usage.FULL); + } + + /** + * True when some list parameter in the cache key is read only at index 0. + */ + private boolean needsFirstElementHelper() { + return listParamsInKeyWithUsage(BddParameterReferences.Usage.FIRST_ELEMENT_ONLY); + } + + private boolean listParamsInKeyWithUsage(BddParameterReferences.Usage usage) { + Map parameters = endpointBddModel.getParameters(); + return cacheKeyParameterOrder().stream() + .filter(name -> isListParam(parameters.get(name))) + .anyMatch(name -> paramUsage.get(name) == usage); + } + + private static boolean isBooleanParam(ParameterModel model) { + return "boolean".equalsIgnoreCase(model.getType()); + } + + /** + * Shared with {@link BddParameterReferences}, which must agree with this class on what a list is: the usage it + * derives selects which comparison helper gets emitted for the parameter. + */ + static boolean isListParam(ParameterModel model) { + return "stringarray".equalsIgnoreCase(model.getType()); + } + private TypeSpec evaluatorClass() { TypeSpec.Builder builder = TypeSpec.classBuilder(evaluatorType) .addModifiers(Modifier.PRIVATE, Modifier.STATIC, Modifier.FINAL); @@ -386,6 +667,15 @@ private MethodSpec resolveEndpointMethod() { builder.addCode(validateRequiredParams()); + // Cache check. This sits after required-param validation so that invalid params fail the same way on a hit as + // on a miss. One volatile read into a local, so the entry cannot be replaced between the null check and the + // comparison. + builder.addComment("Single-entry result cache: reuse the last endpoint when the params still match."); + builder.addStatement("$T cached = this.cache", cacheEntryType); + builder.beginControlFlow("if (cached != null && cacheParamsMatch(endpointParams, cached.params))"); + builder.addStatement("return $T.completedFuture(cached.endpoint)", CompletableFuture.class); + builder.endControlFlow(); + builder.beginControlFlow("try"); // Allocate evaluator per call — lightweight (just fields, no maps), immediately young-gen collected. @@ -409,6 +699,9 @@ private MethodSpec resolveEndpointMethod() { CompletableFutureUtils.class, SdkClientException.class, "Rule engine did not reach an error or endpoint result") .endControlFlow(); + // Populate on success only. A rule error and a no-match both leave the previous entry in place, so a transient + // bad-params call cannot poison the cache and an error is never replayed from it. + builder.addStatement("this.cache = new $T(endpointParams, result)", cacheEntryType); builder.addStatement("return $T.completedFuture(result)", CompletableFuture.class); // Catch errors thrown from result methods diff --git a/codegen/src/main/java/software/amazon/awssdk/codegen/poet/rules2/bdd/BddParameterReferences.java b/codegen/src/main/java/software/amazon/awssdk/codegen/poet/rules2/bdd/BddParameterReferences.java new file mode 100644 index 000000000000..04439e4a718b --- /dev/null +++ b/codegen/src/main/java/software/amazon/awssdk/codegen/poet/rules2/bdd/BddParameterReferences.java @@ -0,0 +1,180 @@ +/* + * 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.codegen.poet.rules2.bdd; + +import java.util.Collections; +import java.util.HashSet; +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.Set; +import software.amazon.awssdk.codegen.model.rules.endpoints.ConditionModel; +import software.amazon.awssdk.codegen.model.rules.endpoints.ParameterModel; +import software.amazon.awssdk.codegen.model.rules.endpoints.RuleModel; +import software.amazon.awssdk.codegen.model.service.EndpointBddModel; +import software.amazon.awssdk.codegen.poet.rules2.ExpressionParser; +import software.amazon.awssdk.codegen.poet.rules2.FunctionCallExpression; +import software.amazon.awssdk.codegen.poet.rules2.IndexedAccessExpression; +import software.amazon.awssdk.codegen.poet.rules2.MemberAccessExpression; +import software.amazon.awssdk.codegen.poet.rules2.RuleExpression; +import software.amazon.awssdk.codegen.poet.rules2.VariableReferenceExpression; +import software.amazon.awssdk.codegen.poet.rules2.WalkRuleExpressionVisitor; + +/** + * Works out how each endpoint parameter is used by a BDD, so that the generated result cache only compares what can + * actually change the resolved endpoint. + * + *

Analysis runs over the BDD's conditions and results using the same parser the generator uses, so the answer + * reflects what the generated provider reads rather than what the model happens to declare. + * + *

Two things fall out of it: + * + *

+ */ +final class BddParameterReferences { + + /** + * How much of a parameter's value can influence the resolved endpoint. + */ + enum Usage { + /** No condition and no result reads it. It cannot be part of the cache key. */ + UNREFERENCED, + + /** + * A {@code stringArray} the rules can only observe through {@code isSet(param)} and {@code param[0]}. Whether + * the list is present, plus its first element, is therefore the whole of what can reach the endpoint, and + * nothing past element 0 can change the answer. + */ + FIRST_ELEMENT_ONLY, + + /** Read in a way that can depend on the entire value. */ + FULL + } + + private BddParameterReferences() { + } + + /** + * Returns the usage of every parameter the model declares, in declaration order. + */ + static Map analyze(EndpointBddModel model) { + Collector collector = new Collector(); + + for (ConditionModel condition : model.getConditions()) { + RuleModel synthetic = new RuleModel(); + synthetic.setType("error"); + synthetic.setError("synthetic"); + synthetic.setConditions(Collections.singletonList(condition)); + ExpressionParser.parseRuleSetExpression(synthetic).accept(collector); + } + for (RuleModel result : model.getResults()) { + ExpressionParser.parseRuleSetExpression(result).accept(collector); + } + + Map usage = new LinkedHashMap<>(); + model.getParameters().forEach((name, parameter) -> usage.put(name, usageOf(name, parameter, collector))); + return Collections.unmodifiableMap(usage); + } + + private static Usage usageOf(String name, ParameterModel parameter, Collector collector) { + if (!collector.referenced.contains(name)) { + return Usage.UNREFERENCED; + } + if (collector.wholeValue.contains(name)) { + return Usage.FULL; + } + // Only lists benefit, and only lists can be read element-wise. Anything else that somehow reached here is + // compared in full rather than guessed at. + return BddEndpointProviderSpec.isListParam(parameter) ? Usage.FIRST_ELEMENT_ONLY : Usage.FULL; + } + + /** + * Collects, for every name the expressions reference, whether any reference needs more than the first element. + * + *

Erring towards {@link Usage#FULL} is the safe direction: it costs comparison work, whereas erring the other + * way would drop something from the cache key that can change the endpoint. + */ + private static final class Collector extends WalkRuleExpressionVisitor { + private final Set referenced = new HashSet<>(); + private final Set wholeValue = new HashSet<>(); + + @Override + public Void visitIndexedAccessExpression(IndexedAccessExpression e) { + String subject = firstElementSubject(e); + if (subject != null) { + referenced.add(subject); + // Deliberately not descending. Descending would reach the variable reference underneath and record it + // as a whole-value read, which is the thing this case exists to avoid. + return null; + } + return super.visitIndexedAccessExpression(e); + } + + /** + * {@code isSet(param)} observes only whether the parameter is present, so on its own it does not force a + * whole-value comparison. + */ + @Override + public Void visitFunctionCallExpression(FunctionCallExpression e) { + if ("isSet".equals(e.name()) && e.arguments().size() == 1) { + RuleExpression argument = e.arguments().get(0); + if (argument instanceof VariableReferenceExpression) { + referenced.add(((VariableReferenceExpression) argument).variableName()); + return null; + } + } + return super.visitFunctionCallExpression(e); + } + + @Override + public Void visitVariableReferenceExpression(VariableReferenceExpression e) { + referenced.add(e.variableName()); + wholeValue.add(e.variableName()); + return null; + } + + /** + * Returns the parameter name when this is an index-0 read of a plain parameter, otherwise null. + * + *

Both spellings reach here: {@code "list[0]"} inside a template parses to an indexed access straight over + * the variable, while {@code getAttr(list, "[0]")} wraps it in a direct-index member access first. + */ + private static String firstElementSubject(IndexedAccessExpression e) { + if (e.index() != 0) { + return null; + } + RuleExpression source = e.source(); + if (source instanceof MemberAccessExpression) { + MemberAccessExpression member = (MemberAccessExpression) source; + if (!member.directIndex()) { + return null; + } + source = member.source(); + } + if (source instanceof VariableReferenceExpression) { + return ((VariableReferenceExpression) source).variableName(); + } + return null; + } + } +} 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 8137ad2213f5..d59eb4cf701a 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 @@ -632,7 +632,7 @@ public static IntermediateModel batchManagerModels() { return new IntermediateModelBuilder(models).build(); } - + public static IntermediateModel presignedUrlExtensionModels() { File serviceModel = new File(ClientTestModels.class.getResource("client/c2j/presignedurl/service-2.json").getFile()); File customizationModel = new File(ClientTestModels.class.getResource("client/c2j/presignedurl/customization.config").getFile()); @@ -656,8 +656,11 @@ public static IntermediateModel queryServiceModelsWithBddEndpoints() { public static IntermediateModel queryServiceModelsWithBddEndpoints(boolean useS3ExpressSessionAuth) { File serviceModel = new File(ClientTestModels.class.getResource("client/c2j/query/service-2.json").getFile()); File waitersModel = new File(ClientTestModels.class.getResource("client/c2j/query/waiters-2.json").getFile()); + // The S3 rule set, not the default-regional one, because it declares the same 17 parameters as the S3 BDD. The + // generated params class comes from the rule set while the provider body comes from the BDD, so pairing the S3 + // BDD with a 4-parameter rule set would produce a provider referencing getters the params class does not have. File endpointRuleSetModel = - new File(ClientTestModels.class.getResource("client/c2j/query/endpoint-rule-set-default-regional.json").getFile()); + new File(ClientTestModels.class.getResource("client/c2j/s3-test/endpoint-rule-set.json").getFile()); File endpointTestsModel = new File(ClientTestModels.class.getResource("client/c2j/query/endpoint-tests.json").getFile()); File endpointBddModel = diff --git a/codegen/src/test/java/software/amazon/awssdk/codegen/poet/rules2/bdd/BddEndpointProviderSpecTest.java b/codegen/src/test/java/software/amazon/awssdk/codegen/poet/rules2/bdd/BddEndpointProviderSpecTest.java index 32e819a3f791..2ebf862b2461 100644 --- a/codegen/src/test/java/software/amazon/awssdk/codegen/poet/rules2/bdd/BddEndpointProviderSpecTest.java +++ b/codegen/src/test/java/software/amazon/awssdk/codegen/poet/rules2/bdd/BddEndpointProviderSpecTest.java @@ -19,12 +19,23 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy; import static software.amazon.awssdk.codegen.poet.PoetMatchers.generatesTo; +import java.util.ArrayList; +import java.util.List; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import org.hamcrest.MatcherAssert; import org.junit.jupiter.api.Test; import software.amazon.awssdk.codegen.poet.ClientTestModels; public class BddEndpointProviderSpecTest { + /** + * The trailing nodes of {@code endpoint-bdd-default-regional.json} were appended by hand with {@code high == low}, + * so that each parameter is read by a condition without altering any resolved endpoint. That is the one shape a + * reduced BDD can never contain, which is why it shows up in the golden file as degenerate {@code nodeP} methods + * whose branches are identical. A future peephole pass that collapses {@code high == low} nodes would silently make + * those parameters unreferenced and void the cache-key coverage the tests below rely on. + */ @Test void endpointProviderClass_simpleBdd_generatesExpectedCode() { BddEndpointProviderSpec spec = new BddEndpointProviderSpec( @@ -146,6 +157,165 @@ void complementEdge_generatesNodeNWithSwappedBranches() { assertThat(generated).contains("Endpoint nodeN1()"); } + /** + * The invariant the result cache depends on: every parameter the BDD reads must appear in the generated + * key. A parameter left out is not a slow cache, it is a cache that returns an endpoint resolved for a different + * value of that parameter, and nothing else in the test suite would catch it. + * + *

Asserted against the generated source rather than an intermediate model, so it holds regardless of how the + * comparison is built. + */ + @Test + void cacheKeyComparesEveryReferencedParameter() { + // The simple BDD declares 10 parameters and reads 9; unusedParam is the one it does not read. + assertThat(cacheKeyGetterOrder( + new BddEndpointProviderSpec(ClientTestModels.queryServiceModelsWithSimpleBddEndpoints()))) + .containsExactlyInAnyOrder("useDualStack", "useFips", "region", "stringContextParam", "endpoint", + "staticStringParam", "operationContextParam", "arnList", + "customEndpointArray"); + + // The S3 BDD declares 17 and reads 14; Key, Prefix and CopySource are vestigial declarations. + assertThat(cacheKeyGetterOrder( + new BddEndpointProviderSpec(ClientTestModels.queryServiceModelsWithBddEndpoints()))) + .containsExactlyInAnyOrder("useFips", "useDualStack", "forcePathStyle", "accelerate", "useGlobalEndpoint", + "useObjectLambdaEndpoint", "disableAccessPoints", + "disableMultiRegionAccessPoints", "useArnRegion", + "useS3ExpressControlEndpoint", "disableS3ExpressSessionAuth", "region", + "bucket", "endpoint"); + + // The complement BDD declares and reads exactly two. + assertThat(cacheKeyGetterOrder( + new BddEndpointProviderSpec(ClientTestModels.queryServiceModelsWithComplementBddEndpoints()))) + .containsExactlyInAnyOrder("region", "endpoint"); + } + + /** + * A parameter no condition and no result reads cannot change the resolved endpoint, so comparing it could only turn + * hits into misses that resolve to the endpoint already cached. + * + *

This is what makes the cache worth having for S3, whose rule set declares {@code Key}, {@code Prefix} and + * {@code CopySource} and reads none of them. {@code Key} changes on essentially every object request, so including + * it would mean the cache almost never hits. + */ + @Test + void cacheKeyOmitsParametersTheBddNeverReads() { + assertThat(cacheKeyGetterOrder( + new BddEndpointProviderSpec(ClientTestModels.queryServiceModelsWithSimpleBddEndpoints()))) + .as("a parameter nothing reads must not force a cache miss") + .doesNotContain("unusedParam"); + + assertThat(cacheKeyGetterOrder( + new BddEndpointProviderSpec(ClientTestModels.queryServiceModelsWithBddEndpoints()))) + .as("S3 declares Key, Prefix and CopySource but reads none of them") + .doesNotContain("key", "prefix", "copySource") + .contains("bucket"); + } + + /** + * The generated key compares booleans first, then the strings whose reference the SDK keeps stable, then everything + * else. Ordering cannot change the result - the chain compares every parameter before returning true - it only + * decides how quickly a mismatch is found, so this is a performance property rather than a correctness one. It is + * pinned because the ordering is the entire reason the grouping exists; if it silently degraded to declaration + * order the code would still be correct and the benefit would be gone. + */ + @Test + void cacheKeyOrdersBooleansThenStableStringsThenTheRest() { + List order = cacheKeyGetterOrder( + new BddEndpointProviderSpec(ClientTestModels.queryServiceModelsWithSimpleBddEndpoints())); + + assertThat(order).containsExactly("useDualStack", "useFips", // booleans + "region", "stringContextParam", // reference-stable strings + "endpoint", "staticStringParam", // everything else, declaration order + "operationContextParam", "arnList", "customEndpointArray"); + } + + /** + * A list read as a whole needs a bounded comparison, so it routes through the emitted helper rather than + * {@code Objects.equals}, whose {@code List.equals} would walk every element however long the list is. + */ + @Test + void listReadAsAWholeUsesTheBoundedHelper() { + String generated = new BddEndpointProviderSpec( + ClientTestModels.queryServiceModelsWithSimpleBddEndpoints()).poetSpec().toString(); + + assertThat(generated).contains("cacheListsMatch(a.customEndpointArray(), b.customEndpointArray())"); + assertThat(generated).contains("if (size > 4) return false"); + } + + /** + * When the rules can only see whether a list is present and what its first element is, the rest of the list cannot + * reach the endpoint, so the key compares presence and element 0. This is the DynamoDB shape: it reads + * {@code ResourceArnList} through {@code isSet} and {@code getAttr(ResourceArnList, "[0]")} and nothing else, and + * comparing the whole list costs more than half of a regional resolution. + * + *

The {@code isSet} guard is not incidental. The rules language requires a null check before an indexed access, + * so every real model that reads {@code list[0]} also reads {@code isSet(list)}; if the null check disqualified the + * parameter this case would never fire in production. + * + *

Also asserts the generated provider really does read only element 0, so the two halves cannot drift apart: were + * a read past the head to appear, this comparison would silently become wrong. + */ + @Test + void listReadOnlyAtIndexZeroComparesPresenceAndTheFirstElement() { + String generated = new BddEndpointProviderSpec( + ClientTestModels.queryServiceModelsWithSimpleBddEndpoints()).poetSpec().toString(); + + assertThat(generated).contains("cacheFirstElementsMatch(a.arnList(), b.arnList())"); + assertThat(generated).doesNotContain("cacheListsMatch(a.arnList()"); + assertThat(generated) + .as("the first-element comparison is only valid while the provider reads nothing past element 0") + .contains("RulesFunctions.listAccess(params.arnList(), 0)") + .doesNotContain("RulesFunctions.listAccess(params.arnList(), 1)"); + assertThat(generated) + .as("isSet distinguishes an absent list from an empty one, so presence stays part of the key") + .contains("if (a == null || b == null) return false"); + } + + /** + * A read past the head disqualifies the first-element comparison, because elements beyond the first can then reach + * the endpoint. Pinned because the difference between the two helpers is a correctness boundary, not a preference. + */ + @Test + void listReadPastTheHeadIsComparedInFull() { + String generated = new BddEndpointProviderSpec( + ClientTestModels.queryServiceModelsWithSimpleBddEndpoints()).poetSpec().toString(); + + assertThat(generated).contains("cacheListsMatch(a.customEndpointArray(), b.customEndpointArray())"); + assertThat(generated).doesNotContain("cacheFirstElementsMatch(a.customEndpointArray()"); + assertThat(generated).contains("RulesFunctions.listAccess(params.customEndpointArray(), 1)"); + } + + /** + * Neither helper is worth emitting when nothing needs it, and the S3 BDD keeps no list parameter in its key. + */ + @Test + void listHelpersAreOmittedWhenNoListParameterIsInTheKey() { + String generated = new BddEndpointProviderSpec( + ClientTestModels.queryServiceModelsWithBddEndpoints()).poetSpec().toString(); + + assertThat(generated).doesNotContain("cacheListsMatch"); + assertThat(generated).doesNotContain("cacheFirstElementsMatch"); + } + + /** + * Returns the parameter getters referenced by the generated {@code cacheParamsMatch}, in the order they are + * compared. + */ + private static List cacheKeyGetterOrder(BddEndpointProviderSpec spec) { + String generated = spec.poetSpec().toString(); + int start = generated.indexOf("boolean cacheParamsMatch("); + assertThat(start).as("generated provider must contain cacheParamsMatch").isNotNegative(); + int end = generated.indexOf(";", start); + String body = generated.substring(start, end); + + List getters = new ArrayList<>(); + Matcher matcher = Pattern.compile("\\ba\\.(\\w+)\\(\\)").matcher(body); + while (matcher.find()) { + getters.add(matcher.group(1)); + } + return getters; + } + /** * {@code DynamicEndpointAuthSchemeFactory} is S3-specific, so a dynamically resolved auth scheme name in any other * service must fail codegen rather than emitting code that cannot compile. diff --git a/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/c2j/query/endpoint-bdd-default-regional.json b/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/c2j/query/endpoint-bdd-default-regional.json index 9f7c8fc3a7e4..a05255ae456e 100644 --- a/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/c2j/query/endpoint-bdd-default-regional.json +++ b/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/c2j/query/endpoint-bdd-default-regional.json @@ -26,6 +26,36 @@ "required": false, "documentation": "Override the endpoint used to send this request", "type": "string" + }, + "stringContextParam": { + "required": false, + "documentation": "A client context parameter, so the cache key compares it early as reference-stable.", + "type": "string" + }, + "staticStringParam": { + "required": false, + "documentation": "Bound to a per-operation static literal.", + "type": "string" + }, + "operationContextParam": { + "required": false, + "documentation": "Bound to a request member, so it can change per request.", + "type": "string" + }, + "arnList": { + "required": false, + "documentation": "Read only as arnList[0], so the cache key compares just the first element.", + "type": "stringArray" + }, + "customEndpointArray": { + "required": false, + "documentation": "Read as a whole, so the cache key compares every element.", + "type": "stringArray" + }, + "unusedParam": { + "required": false, + "documentation": "Declared but read by no condition and no result, so the cache key must leave it out.", + "type": "string" } }, "conditions": [ @@ -116,6 +146,66 @@ ] } ] + }, + { + "fn": "isSet", + "argv": [ + { + "ref": "stringContextParam" + } + ] + }, + { + "fn": "isSet", + "argv": [ + { + "ref": "staticStringParam" + } + ] + }, + { + "fn": "isSet", + "argv": [ + { + "ref": "operationContextParam" + } + ] + }, + { + "fn": "isSet", + "argv": [ + { + "ref": "customEndpointArray" + } + ] + }, + { + "fn": "getAttr", + "argv": [ + { + "ref": "arnList" + }, + "[0]" + ], + "assign": "FirstArn" + }, + { + "fn": "isSet", + "argv": [ + { + "ref": "arnList" + } + ] + }, + { + "fn": "getAttr", + "argv": [ + { + "ref": "customEndpointArray" + }, + "[1]" + ], + "assign": "SecondEndpoint" } ], "results": [ @@ -206,7 +296,7 @@ "type": "error" } ], - "root": 15, - "nodeCount": 15, - "nodes": "/////wAAAAH/////AAAABAX14QIF9eEDAAAAAgX14QEAAAACAAAABgX14QQF9eEFAAAABQAAAAQF9eEFAAAABwX14QYF9eEHAAAABQAAAAYF9eEIAAAABAAAAAUAAAAHAAAAAwAAAAgF9eEMAAAABgX14QkF9eEKAAAABAAAAAoF9eELAAAAAwAAAAsF9eEMAAAAAgAAAAkAAAAMAAAAAQAAAA0F9eEMAAAAAAAAAAMAAAAO" -} \ No newline at end of file + "root": 22, + "nodeCount": 22, + "nodes": "/////wAAAAH/////AAAABAX14QIF9eEDAAAAAgX14QEAAAACAAAABgX14QQF9eEFAAAABQAAAAQF9eEFAAAABwX14QYF9eEHAAAABQAAAAYF9eEIAAAABAAAAAUAAAAHAAAAAwAAAAgF9eEMAAAABgX14QkF9eEKAAAABAAAAAoF9eELAAAAAwAAAAsF9eEMAAAAAgAAAAkAAAAMAAAAAQAAAA0F9eEMAAAAAAAAAAMAAAAOAAAACAAAABEAAAARAAAACQAAABIAAAASAAAACgAAABMAAAATAAAACwAAABQAAAAUAAAADAAAAA8AAAAPAAAADQAAABAAAAAQAAAADgAAABUAAAAV" +} diff --git a/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/c2j/query/endpoint-rule-set-default-regional.json b/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/c2j/query/endpoint-rule-set-default-regional.json index e0314767c0df..6c3b34e6e15f 100644 --- a/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/c2j/query/endpoint-rule-set-default-regional.json +++ b/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/c2j/query/endpoint-rule-set-default-regional.json @@ -26,6 +26,36 @@ "required": false, "documentation": "Override the endpoint used to send this request", "type": "string" + }, + "stringContextParam": { + "required": false, + "documentation": "A client context parameter, so the cache key compares it early as reference-stable.", + "type": "string" + }, + "staticStringParam": { + "required": false, + "documentation": "Bound to a per-operation static literal.", + "type": "string" + }, + "operationContextParam": { + "required": false, + "documentation": "Bound to a request member, so it can change per request.", + "type": "string" + }, + "arnList": { + "required": false, + "documentation": "Read only as arnList[0], so the cache key compares just the first element.", + "type": "stringArray" + }, + "customEndpointArray": { + "required": false, + "documentation": "Read as a whole, so the cache key compares every element.", + "type": "stringArray" + }, + "unusedParam": { + "required": false, + "documentation": "Declared but read by no condition and no result, so the cache key must leave it out.", + "type": "string" } }, "rules": [ @@ -336,4 +366,4 @@ "type": "error" } ] -} \ No newline at end of file +} diff --git a/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/rules/endpoint-resolver-utils-with-endpointsbasedauth.java b/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/rules/endpoint-resolver-utils-with-endpointsbasedauth.java index ad3b2e674750..a98fee735480 100644 --- a/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/rules/endpoint-resolver-utils-with-endpointsbasedauth.java +++ b/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/rules/endpoint-resolver-utils-with-endpointsbasedauth.java @@ -191,7 +191,7 @@ private static String recordAccountIdEndpointMode(ExecutionAttributes executionA AccountIdEndpointMode mode = executionAttributes.getAttribute(AwsExecutionAttribute.AWS_AUTH_ACCOUNT_ID_ENDPOINT_MODE); BusinessMetricsUtils.resolveAccountIdEndpointModeMetric(mode).ifPresent( m -> executionAttributes.getAttribute(SdkInternalExecutionAttribute.BUSINESS_METRICS).addMetric(m)); - return mode.name().toLowerCase(); + return mode.endpointModeValue(); } public static void setMetricValues(Endpoint endpoint, ExecutionAttributes executionAttributes) { diff --git a/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/rules/endpoint-resolver-utils-with-stringarray.java b/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/rules/endpoint-resolver-utils-with-stringarray.java index aaed43624739..a3ff124832a3 100644 --- a/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/rules/endpoint-resolver-utils-with-stringarray.java +++ b/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/rules/endpoint-resolver-utils-with-stringarray.java @@ -1,6 +1,7 @@ package software.amazon.awssdk.services.samplesvc.endpoints.internal; import java.util.Arrays; +import java.util.Collections; import java.util.List; import java.util.Optional; import software.amazon.awssdk.annotations.Generated; @@ -28,6 +29,10 @@ @Generated("software.amazon.awssdk:codegen") @SdkInternalApi public final class SampleSvcEndpointResolverUtils { + private static final List STATIC_LIST_EMPTY_STATIC_CONTEXT_OPERATION_STRING_ARRAY_PARAM = Collections.emptyList(); + + private static final List STATIC_LIST_STATIC_CONTEXT_OPERATION_STRING_ARRAY_PARAM = Collections.unmodifiableList(Arrays.asList("staticValue1")); + private SampleSvcEndpointResolverUtils() { } @@ -57,12 +62,12 @@ private static void setStaticContextParams(SampleSvcEndpointParams.Builder param private static void emptyStaticContextOperationStaticContextParams( SampleSvcEndpointParams.Builder params) { - params.stringArrayParam(Arrays.asList()); + params.stringArrayParam(STATIC_LIST_EMPTY_STATIC_CONTEXT_OPERATION_STRING_ARRAY_PARAM); } private static void staticContextOperationStaticContextParams( SampleSvcEndpointParams.Builder params) { - params.stringArrayParam(Arrays.asList("staticValue1")); + params.stringArrayParam(STATIC_LIST_STATIC_CONTEXT_OPERATION_STRING_ARRAY_PARAM); } public static SelectedAuthScheme authSchemeWithEndpointSignerProperties( diff --git a/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/rules/endpoint-resolver-utils.java b/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/rules/endpoint-resolver-utils.java index 01c7ba43831f..7c723c51914c 100644 --- a/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/rules/endpoint-resolver-utils.java +++ b/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/rules/endpoint-resolver-utils.java @@ -198,7 +198,7 @@ private static String recordAccountIdEndpointMode(ExecutionAttributes executionA AccountIdEndpointMode mode = executionAttributes.getAttribute(AwsExecutionAttribute.AWS_AUTH_ACCOUNT_ID_ENDPOINT_MODE); BusinessMetricsUtils.resolveAccountIdEndpointModeMetric(mode).ifPresent( m -> executionAttributes.getAttribute(SdkInternalExecutionAttribute.BUSINESS_METRICS).addMetric(m)); - return mode.name().toLowerCase(); + return mode.endpointModeValue(); } public static void setMetricValues(Endpoint endpoint, ExecutionAttributes executionAttributes) { diff --git a/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/rules2/bdd/endpoint-provider-bdd-class.java b/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/rules2/bdd/endpoint-provider-bdd-class.java index 39e5da5ba9ff..c0566ec44f4d 100644 --- a/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/rules2/bdd/endpoint-provider-bdd-class.java +++ b/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/rules2/bdd/endpoint-provider-bdd-class.java @@ -1,5 +1,7 @@ package software.amazon.awssdk.services.query.endpoints.internal; +import java.util.List; +import java.util.Objects; import java.util.concurrent.CompletableFuture; import software.amazon.awssdk.annotations.Generated; import software.amazon.awssdk.annotations.SdkInternalApi; @@ -13,16 +15,24 @@ @Generated("software.amazon.awssdk:codegen") @SdkInternalApi public final class DefaultQueryEndpointProvider implements QueryEndpointProvider { + private volatile CacheEntry cache; + @Override public CompletableFuture resolveEndpoint(QueryEndpointParams endpointParams) { + // Single-entry result cache: reuse the last endpoint when the params still match. + CacheEntry cached = this.cache; + if (cached != null && cacheParamsMatch(endpointParams, cached.params)) { + return CompletableFuture.completedFuture(cached.endpoint); + } try { Evaluator evaluator = new Evaluator(); evaluator.params = endpointParams; evaluator.region = endpointParams.region() == null ? null : endpointParams.region().id(); - Endpoint result = evaluator.nodeP14(); + Endpoint result = evaluator.nodeP21(); if (result == null) { return CompletableFutureUtils.failedFuture(SdkClientException.create("Rule engine did not reach an error or endpoint result")); } + this.cache = new CacheEntry(endpointParams, result); return CompletableFuture.completedFuture(result); } catch (SdkClientException e) { String errorMsg = e.getMessage(); @@ -35,6 +45,41 @@ public CompletableFuture resolveEndpoint(QueryEndpointParams endpointP } } + private static boolean cacheParamsMatch(QueryEndpointParams a, QueryEndpointParams b) { + return Objects.equals(a.useDualStack(), b.useDualStack()) + && Objects.equals(a.useFips(), b.useFips()) + && Objects.equals(a.region(), b.region()) + && Objects.equals(a.stringContextParam(), b.stringContextParam()) + && Objects.equals(a.endpoint(), b.endpoint()) + && Objects.equals(a.staticStringParam(), b.staticStringParam()) + && Objects.equals(a.operationContextParam(), b.operationContextParam()) + && cacheFirstElementsMatch(a.arnList(), b.arnList()) + && cacheListsMatch(a.customEndpointArray(), b.customEndpointArray()); + } + + private static boolean cacheListsMatch(List a, List b) { + if (a == b) return true; + if (a == null || b == null) return false; + int size = a.size(); + if (size != b.size()) return false; + // Bounded so that a long list cannot make the cache check cost more than resolving. + if (size > 4) return false; + for (int i = 0; i < size; i++) { + if (!Objects.equals(a.get(i), b.get(i))) return false; + } + return true; + } + + private static boolean cacheFirstElementsMatch(List a, List b) { + if (a == b) return true; + // isSet can tell an absent list from an empty one, so presence is part of the key. + if (a == null || b == null) return false; + // Nothing past element 0 can reach the endpoint. + String firstA = a.isEmpty() ? null : a.get(0); + String firstB = b.isEmpty() ? null : b.get(0); + return Objects.equals(firstA, firstB); + } + private static final class Evaluator { QueryEndpointParams params; @@ -42,6 +87,10 @@ private static final class Evaluator { RulePartition partitionResult; + String firstArn; + + String secondEndpoint; + private Endpoint nodeP0() { return null; } @@ -130,6 +179,48 @@ private Endpoint nodeP14() { : nodeP13(); } + private Endpoint nodeP15() { + return params.stringContextParam() != null + ? nodeP16() + : nodeP16(); + } + + private Endpoint nodeP16() { + return params.staticStringParam() != null + ? nodeP17() + : nodeP17(); + } + + private Endpoint nodeP17() { + return params.operationContextParam() != null + ? nodeP18() + : nodeP18(); + } + + private Endpoint nodeP18() { + return params.customEndpointArray() != null + ? nodeP19() + : nodeP19(); + } + + private Endpoint nodeP19() { + return cond12() + ? nodeP14() + : nodeP14(); + } + + private Endpoint nodeP20() { + return params.arnList() != null + ? nodeP15() + : nodeP15(); + } + + private Endpoint nodeP21() { + return cond14() + ? nodeP20() + : nodeP20(); + } + private boolean cond3() { partitionResult = RulesFunctions.awsPartition(region); return partitionResult != null; @@ -147,6 +238,16 @@ private boolean cond7() { return ("aws-us-gov".equals(partitionResult.name())); } + private boolean cond12() { + firstArn = RulesFunctions.listAccess(params.arnList(), 0); + return firstArn != null; + } + + private boolean cond14() { + secondEndpoint = RulesFunctions.listAccess(params.customEndpointArray(), 1); + return secondEndpoint != null; + } + private Endpoint result0() { throw SdkClientException.create("Invalid Configuration: FIPS and custom endpoint are not supported"); } @@ -195,4 +296,15 @@ private Endpoint result11() { throw SdkClientException.create("Invalid Configuration: Missing Region"); } } + + private static final class CacheEntry { + final QueryEndpointParams params; + + final Endpoint endpoint; + + CacheEntry(QueryEndpointParams params, Endpoint endpoint) { + this.params = params; + this.endpoint = endpoint; + } + } } diff --git a/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/rules2/bdd/endpoint-provider-bdd-s3-class.java b/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/rules2/bdd/endpoint-provider-bdd-s3-class.java index bb661bf8c9db..7b55c6c57497 100644 --- a/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/rules2/bdd/endpoint-provider-bdd-s3-class.java +++ b/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/rules2/bdd/endpoint-provider-bdd-s3-class.java @@ -1,6 +1,7 @@ package software.amazon.awssdk.services.query.endpoints.internal; import java.util.Arrays; +import java.util.Objects; import java.util.concurrent.CompletableFuture; import software.amazon.awssdk.annotations.Generated; import software.amazon.awssdk.annotations.SdkInternalApi; @@ -18,8 +19,15 @@ @Generated("software.amazon.awssdk:codegen") @SdkInternalApi public final class DefaultQueryEndpointProvider implements QueryEndpointProvider { + private volatile CacheEntry cache; + @Override public CompletableFuture resolveEndpoint(QueryEndpointParams endpointParams) { + // Single-entry result cache: reuse the last endpoint when the params still match. + CacheEntry cached = this.cache; + if (cached != null && cacheParamsMatch(endpointParams, cached.params)) { + return CompletableFuture.completedFuture(cached.endpoint); + } try { Evaluator evaluator = new Evaluator(); evaluator.params = endpointParams; @@ -28,6 +36,7 @@ public CompletableFuture resolveEndpoint(QueryEndpointParams endpointP if (result == null) { return CompletableFutureUtils.failedFuture(SdkClientException.create("Rule engine did not reach an error or endpoint result")); } + this.cache = new CacheEntry(endpointParams, result); return CompletableFuture.completedFuture(result); } catch (SdkClientException e) { String errorMsg = e.getMessage(); @@ -40,6 +49,23 @@ public CompletableFuture resolveEndpoint(QueryEndpointParams endpointP } } + private static boolean cacheParamsMatch(QueryEndpointParams a, QueryEndpointParams b) { + return Objects.equals(a.useFips(), b.useFips()) + && Objects.equals(a.useDualStack(), b.useDualStack()) + && Objects.equals(a.forcePathStyle(), b.forcePathStyle()) + && Objects.equals(a.accelerate(), b.accelerate()) + && Objects.equals(a.useGlobalEndpoint(), b.useGlobalEndpoint()) + && Objects.equals(a.useObjectLambdaEndpoint(), b.useObjectLambdaEndpoint()) + && Objects.equals(a.disableAccessPoints(), b.disableAccessPoints()) + && Objects.equals(a.disableMultiRegionAccessPoints(), b.disableMultiRegionAccessPoints()) + && Objects.equals(a.useArnRegion(), b.useArnRegion()) + && Objects.equals(a.useS3ExpressControlEndpoint(), b.useS3ExpressControlEndpoint()) + && Objects.equals(a.disableS3ExpressSessionAuth(), b.disableS3ExpressSessionAuth()) + && Objects.equals(a.region(), b.region()) + && Objects.equals(a.bucket(), b.bucket()) + && Objects.equals(a.endpoint(), b.endpoint()); + } + private static final class Evaluator { QueryEndpointParams params; @@ -4099,4 +4125,15 @@ private Endpoint result114() { throw SdkClientException.create("A region must be set when sending requests to S3."); } } + + private static final class CacheEntry { + final QueryEndpointParams params; + + final Endpoint endpoint; + + CacheEntry(QueryEndpointParams params, Endpoint endpoint) { + this.params = params; + this.endpoint = endpoint; + } + } } diff --git a/core/aws-core/src/main/java/software/amazon/awssdk/awscore/endpoints/AccountIdEndpointMode.java b/core/aws-core/src/main/java/software/amazon/awssdk/awscore/endpoints/AccountIdEndpointMode.java index 7e6f0050dc4e..eadbf7ea3172 100644 --- a/core/aws-core/src/main/java/software/amazon/awssdk/awscore/endpoints/AccountIdEndpointMode.java +++ b/core/aws-core/src/main/java/software/amazon/awssdk/awscore/endpoints/AccountIdEndpointMode.java @@ -26,19 +26,25 @@ public enum AccountIdEndpointMode { /** * Default value that indicates account ID values will be used in endpoint rules if available. */ - PREFERRED, + PREFERRED("preferred"), /** * When mode is disabled, any resolved account ID will not be used in endpoint construction and rules that * reference them will be bypassed. */ - DISABLED, + DISABLED("disabled"), /** * Required mode would be used in scenarios where endpoint resolution should return an error if no account ID is * available. */ - REQUIRED; + REQUIRED("required"); + + private final String endpointModeValue; + + AccountIdEndpointMode(String endpointModeValue) { + this.endpointModeValue = endpointModeValue; + } /** * Returns the appropriate AccountIdEndpointMode value after parsing the parameter. @@ -52,11 +58,25 @@ public static AccountIdEndpointMode fromValue(String s) { } for (AccountIdEndpointMode value : values()) { - if (value.name().equalsIgnoreCase(s)) { + // Matched against endpointModeValue rather than name() so that the wire form has a single definition. + // Behaviour is unchanged: the two differ only in case, and the comparison is case-insensitive. + if (value.endpointModeValue.equalsIgnoreCase(s)) { return value; } } throw new IllegalArgumentException("Unrecognized value for account id endpoint mode: " + s); } + + /** + * Returns the canonical lowercase string for this mode, as the endpoint rules engine expects to receive it in the + * {@code AWS::Auth::AccountIdEndpointMode} built-in. + *

+ * Unlike {@code name().toLowerCase()}, this returns the same interned {@link String} reference on every call instead + * of a fresh string per request, which removes an allocation from the request path. It is also independent of the + * default locale, which {@code name().toLowerCase()} is not. + */ + public String endpointModeValue() { + return endpointModeValue; + } } diff --git a/core/aws-core/src/main/java/software/amazon/awssdk/awscore/endpoints/AwsEndpointProviderUtils.java b/core/aws-core/src/main/java/software/amazon/awssdk/awscore/endpoints/AwsEndpointProviderUtils.java index b2ee07acbf8b..56ac41f02c69 100644 --- a/core/aws-core/src/main/java/software/amazon/awssdk/awscore/endpoints/AwsEndpointProviderUtils.java +++ b/core/aws-core/src/main/java/software/amazon/awssdk/awscore/endpoints/AwsEndpointProviderUtils.java @@ -15,8 +15,6 @@ package software.amazon.awssdk.awscore.endpoints; -import static software.amazon.awssdk.utils.FunctionalUtils.invokeSafely; - import java.net.URI; import software.amazon.awssdk.annotations.SdkProtectedApi; import software.amazon.awssdk.awscore.AwsExecutionAttribute; @@ -54,19 +52,19 @@ public static Boolean fipsEnabledBuiltIn(ExecutionAttributes executionAttributes } /** - * Returns the endpoint set on the client. Note that this strips off the query part of the URI because the endpoint - * rules library, e.g. {@code ParseURL} will return an exception if the URI it parses has query parameters. + * Returns the endpoint set on the client, sanitized for the rules engine. The rules engine (e.g. + * {@code ParseURL}) rejects URIs with query parameters, so we strip the query and user-info components. + *

+ * Delegates to {@link ClientEndpointProvider#sanitizedEndpointString()}, which returns a cached reference on + * {@link software.amazon.awssdk.core.internal.StaticClientEndpointProvider} reducing allocations and expensive + * URI.create calls. */ public static String endpointBuiltIn(ExecutionAttributes executionAttributes) { if (endpointIsOverridden(executionAttributes)) { executionAttributes.getOptionalAttribute(SdkInternalExecutionAttribute.BUSINESS_METRICS).ifPresent( metric -> metric.addMetric(BusinessMetricFeatureId.ENDPOINT_OVERRIDE.value())); - return invokeSafely(() -> { - URI endpointOverride = executionAttributes.getAttribute(SdkInternalExecutionAttribute.CLIENT_ENDPOINT_PROVIDER) - .clientEndpoint(); - return new URI(endpointOverride.getScheme(), null, endpointOverride.getHost(), endpointOverride.getPort(), - endpointOverride.getPath(), null, endpointOverride.getFragment()).toString(); - }); + return executionAttributes.getAttribute(SdkInternalExecutionAttribute.CLIENT_ENDPOINT_PROVIDER) + .sanitizedEndpointString(); } return null; } diff --git a/core/aws-core/src/test/java/software/amazon/awssdk/awscore/endpoints/AccountIdEndpointModeTest.java b/core/aws-core/src/test/java/software/amazon/awssdk/awscore/endpoints/AccountIdEndpointModeTest.java new file mode 100644 index 000000000000..a946feb57cd1 --- /dev/null +++ b/core/aws-core/src/test/java/software/amazon/awssdk/awscore/endpoints/AccountIdEndpointModeTest.java @@ -0,0 +1,99 @@ +/* + * 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.awscore.endpoints; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import java.util.Locale; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.EnumSource; + +/** + * {@link AccountIdEndpointMode#endpointModeValue()} is what generated endpoint resolvers pass to the rules engine as the + * {@code AWS::Auth::AccountIdEndpointMode} built-in, so these tests pin the wire form itself rather than any caller. + */ +class AccountIdEndpointModeTest { + private final Locale defaultLocale = Locale.getDefault(); + + @AfterEach + void restoreLocale() { + Locale.setDefault(defaultLocale); + } + + @Test + void endpointModeValue_returnsTheLowercaseWireForm() { + assertThat(AccountIdEndpointMode.PREFERRED.endpointModeValue()).isEqualTo("preferred"); + assertThat(AccountIdEndpointMode.DISABLED.endpointModeValue()).isEqualTo("disabled"); + assertThat(AccountIdEndpointMode.REQUIRED.endpointModeValue()).isEqualTo("required"); + } + + /** + * The value is a compile-time literal, so the same reference comes back every time. That is what removes the + * per-request allocation the generated resolver used to pay for {@code name().toLowerCase()}. + */ + @ParameterizedTest + @EnumSource(AccountIdEndpointMode.class) + void endpointModeValue_returnsTheSameReferenceEachCall(AccountIdEndpointMode mode) { + assertThat(mode.endpointModeValue()).isSameAs(mode.endpointModeValue()); + } + + /** + * {@code fromValue} and {@code endpointModeValue} must describe the same mapping in both directions, otherwise a + * value the SDK emits is one it cannot read back. + */ + @ParameterizedTest + @EnumSource(AccountIdEndpointMode.class) + void fromValue_roundTripsEndpointModeValue(AccountIdEndpointMode mode) { + assertThat(AccountIdEndpointMode.fromValue(mode.endpointModeValue())).isSameAs(mode); + } + + /** + * The predecessor of this method was {@code name().toLowerCase()}, which uses the default locale. Under a Turkish + * locale that produces a dotless i (U+0131), so {@code DISABLED} became {@code dısabled} and was handed to the rules + * engine in that form. + */ + @ParameterizedTest + @EnumSource(AccountIdEndpointMode.class) + void endpointModeValue_isIndependentOfTheDefaultLocale(AccountIdEndpointMode mode) { + String underDefaultLocale = mode.endpointModeValue(); + + Locale.setDefault(new Locale("tr", "TR")); + + assertThat(mode.endpointModeValue()).isEqualTo(underDefaultLocale); + assertThat(mode.endpointModeValue()).doesNotContain("\u0131"); + } + + @Test + void fromValue_isCaseInsensitive() { + assertThat(AccountIdEndpointMode.fromValue("PREFERRED")).isSameAs(AccountIdEndpointMode.PREFERRED); + assertThat(AccountIdEndpointMode.fromValue("Disabled")).isSameAs(AccountIdEndpointMode.DISABLED); + } + + @Test + void fromValue_nullReturnsNull() { + assertThat(AccountIdEndpointMode.fromValue(null)).isNull(); + } + + @Test + void fromValue_unrecognizedThrows() { + assertThatThrownBy(() -> AccountIdEndpointMode.fromValue("nonsense")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("nonsense"); + } +} diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/ClientEndpointProvider.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/ClientEndpointProvider.java index 500dd446af4d..572830b0c152 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/ClientEndpointProvider.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/ClientEndpointProvider.java @@ -19,6 +19,7 @@ import software.amazon.awssdk.annotations.SdkProtectedApi; import software.amazon.awssdk.core.internal.StaticClientEndpointProvider; import software.amazon.awssdk.endpoints.EndpointProvider; +import software.amazon.awssdk.utils.FunctionalUtils; /** * Client endpoint providers are responsible for resolving client-level endpoints. {@link EndpointProvider}s are @@ -48,6 +49,20 @@ static ClientEndpointProvider create(URI uri, boolean isEndpointOverridden) { */ URI clientEndpoint(); + /** + * Returns the client endpoint as a string with the query and user-info components stripped, as the rules engine's + * {@code ParseURL} rejects a URI carrying query parameters. Returns {@code null} if the endpoint is not overridden. + */ + default String sanitizedEndpointString() { + if (!isEndpointOverridden()) { + return null; + } + URI endpoint = clientEndpoint(); + return FunctionalUtils.invokeSafely( + () -> new URI(endpoint.getScheme(), null, endpoint.getHost(), endpoint.getPort(), + endpoint.getPath(), null, endpoint.getFragment()).toString()); + } + /** * Returns true if this endpoint was specified as an override by the customer, or false if it was determined * automatically by the SDK. diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/StaticClientEndpointProvider.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/StaticClientEndpointProvider.java index 40a9cd38f2c3..43be721c9110 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/StaticClientEndpointProvider.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/StaticClientEndpointProvider.java @@ -18,6 +18,7 @@ import java.net.URI; import software.amazon.awssdk.annotations.SdkInternalApi; import software.amazon.awssdk.core.ClientEndpointProvider; +import software.amazon.awssdk.utils.FunctionalUtils; import software.amazon.awssdk.utils.ToString; import software.amazon.awssdk.utils.Validate; @@ -31,10 +32,48 @@ public class StaticClientEndpointProvider implements ClientEndpointProvider { private final URI clientEndpoint; private final boolean isEndpointOverridden; + /** + * A sanitized form of {@link #clientEndpoint} with the query and user-info components stripped, formatted as a + * string. This is the value that endpoint rules receive via the {@code SDK::Endpoint} built-in. Computed once at + * construction so that every call to {@code endpointBuiltIn()} returns the same {@link String} reference. + *

+ * {@code null} when {@link #isEndpointOverridden} is {@code false}. + */ + private final String sanitizedEndpointString; + public StaticClientEndpointProvider(URI clientEndpoint, boolean isEndpointOverridden) { this.clientEndpoint = Validate.paramNotNull(clientEndpoint, "clientEndpoint"); this.isEndpointOverridden = isEndpointOverridden; Validate.paramNotNull(clientEndpoint.getScheme(), "The URI scheme of endpointOverride"); + this.sanitizedEndpointString = isEndpointOverridden ? sanitize(clientEndpoint) : null; + } + + /** + * {@inheritDoc} + *

+ * Returns the same {@link String} reference on every call, because the value is computed once at construction. That + * removes a URI construction and its string conversion from every request that resolves an endpoint against an + * overridden client endpoint. + *

+ * {@code final} so that the cached value cannot be shadowed by a subclass whose overridden accessors the constructor + * did not see. + */ + @Override + public final String sanitizedEndpointString() { + return sanitizedEndpointString; + } + + /** + * Repeats {@link ClientEndpointProvider#sanitizedEndpointString()}'s transformation over the constructor's argument, + * rather than calling that default from the constructor, so no virtual method runs before this class is fully + * initialised. The two must agree, and + * {@code ClientEndpointProviderTest.sanitizedEndpointString_cachedFormMatchesRecomputedForm} is what holds them + * together. + */ + private static String sanitize(URI endpoint) { + return FunctionalUtils.invokeSafely( + () -> new URI(endpoint.getScheme(), null, endpoint.getHost(), endpoint.getPort(), + endpoint.getPath(), null, endpoint.getFragment()).toString()); } @Override diff --git a/core/sdk-core/src/test/java/software/amazon/awssdk/core/ClientEndpointProviderTest.java b/core/sdk-core/src/test/java/software/amazon/awssdk/core/ClientEndpointProviderTest.java new file mode 100644 index 000000000000..43846d815357 --- /dev/null +++ b/core/sdk-core/src/test/java/software/amazon/awssdk/core/ClientEndpointProviderTest.java @@ -0,0 +1,107 @@ +/* + * 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.core; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.net.URI; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; + +/** + * Covers {@link ClientEndpointProvider#sanitizedEndpointString()}'s default implementation, which is the value generated + * endpoint resolvers receive as the {@code SDK::Endpoint} built-in. + * + *

Every provider the SDK itself installs is a {@code StaticClientEndpointProvider}, which overrides the method with a + * value computed once at construction. The default therefore runs only for an external implementor, and it exists to be + * the single definition of the transformation that the overriding implementation caches. Both halves are asserted here: + * what the transformation does, and that the two forms agree. + */ +class ClientEndpointProviderTest { + /** + * A provider that leaves {@code sanitizedEndpointString()} to the interface, which is what an external implementor + * gets and what no SDK code path produces. + */ + private static ClientEndpointProvider defaultImplementation(URI uri, boolean isEndpointOverridden) { + return new ClientEndpointProvider() { + @Override + public URI clientEndpoint() { + return uri; + } + + @Override + public boolean isEndpointOverridden() { + return isEndpointOverridden; + } + }; + } + + @ParameterizedTest + @CsvSource({ + // Query parameters are rejected by the rules engine's ParseURL, which is why they are stripped. + "https://example.com/path?foo=bar, https://example.com/path", + "https://example.com?foo=bar, https://example.com", + "https://example.com/path?foo=bar&baz=qux, https://example.com/path", + // User-info is stripped for the same reason: it is not part of what the rules engine resolves against. + "https://user:pass@example.com/path, https://example.com/path", + "https://user@example.com, https://example.com", + "https://user:pass@example.com/path?foo=bar,https://example.com/path", + // Everything else survives untouched. + "https://example.com:8443/path, https://example.com:8443/path", + "http://example.com/path, http://example.com/path", + "https://example.com/path#frag, https://example.com/path#frag", + "https://example.com/a/b/c, https://example.com/a/b/c" + }) + void sanitizedEndpointString_stripsQueryAndUserInfo(String input, String expected) { + assertThat(defaultImplementation(URI.create(input), true).sanitizedEndpointString()).isEqualTo(expected); + } + + @Test + void sanitizedEndpointString_returnsNullWhenNotOverridden() { + URI uri = URI.create("https://example.com/path?foo=bar"); + + assertThat(defaultImplementation(uri, false).sanitizedEndpointString()).isNull(); + assertThat(ClientEndpointProvider.create(uri, false).sanitizedEndpointString()).isNull(); + } + + /** + * The claim that makes it safe for {@code StaticClientEndpointProvider} to compute this once at construction: the + * cached value and the recomputed one cannot disagree, because there is one definition of the transformation. + */ + @ParameterizedTest + @CsvSource({ + "https://example.com/path?foo=bar", + "https://user:pass@example.com/path?foo=bar", + "https://example.com:8443/path", + "https://example.com/path#frag", + "http://example.com" + }) + void sanitizedEndpointString_cachedFormMatchesRecomputedForm(String input) { + URI uri = URI.create(input); + + assertThat(ClientEndpointProvider.create(uri, true).sanitizedEndpointString()) + .isEqualTo(defaultImplementation(uri, true).sanitizedEndpointString()); + } + + @Test + void sanitizedEndpointString_overridingImplementationReturnsAStableReference() { + ClientEndpointProvider provider = ClientEndpointProvider.forEndpointOverride( + URI.create("https://example.com/path?foo=bar")); + + assertThat(provider.sanitizedEndpointString()).isSameAs(provider.sanitizedEndpointString()); + } +} diff --git a/test/codegen-generated-classes-test/src/main/resources/codegen-resources/bddendpoints/endpoint-bdd-1.json b/test/codegen-generated-classes-test/src/main/resources/codegen-resources/bddendpoints/endpoint-bdd-1.json index 157f1b1237d4..66e220ed535f 100644 --- a/test/codegen-generated-classes-test/src/main/resources/codegen-resources/bddendpoints/endpoint-bdd-1.json +++ b/test/codegen-generated-classes-test/src/main/resources/codegen-resources/bddendpoints/endpoint-bdd-1.json @@ -26,6 +26,48 @@ "required": false, "documentation": "Override the endpoint used to send this request", "type": "string" + }, + "AccountId": { + "builtIn": "AWS::Auth::AccountId", + "required": false, + "documentation": "The AWS account ID, read off the resolved identity.", + "type": "string" + }, + "AccountIdEndpointMode": { + "builtIn": "AWS::Auth::AccountIdEndpointMode", + "required": false, + "documentation": "Whether the account ID may be used in the endpoint.", + "type": "string" + }, + "clientStringParam": { + "required": false, + "documentation": "A client context parameter, so the cache key compares it early as reference-stable.", + "type": "string" + }, + "staticStringParam": { + "required": false, + "documentation": "Bound to a per-operation static literal.", + "type": "string" + }, + "requestStringParam": { + "required": false, + "documentation": "Bound to a request member, so it can change per request.", + "type": "string" + }, + "resourceArnList": { + "required": false, + "documentation": "Read only as resourceArnList[0], so the cache key compares just the first element. This is the DynamoDB shape.", + "type": "stringArray" + }, + "unusedStringParam": { + "required": false, + "documentation": "Declared but read by no condition and no result, so the cache key must leave it out.", + "type": "string" + }, + "wholeArnList": { + "required": false, + "documentation": "Read as a whole, so the cache key compares every element.", + "type": "stringArray" } }, "conditions": [ @@ -116,6 +158,82 @@ }, "aws-us-gov" ] + }, + { + "fn": "isSet", + "argv": [ + { + "ref": "AccountId" + } + ] + }, + { + "fn": "isSet", + "argv": [ + { + "ref": "AccountIdEndpointMode" + } + ] + }, + { + "fn": "isSet", + "argv": [ + { + "ref": "clientStringParam" + } + ] + }, + { + "fn": "isSet", + "argv": [ + { + "ref": "staticStringParam" + } + ] + }, + { + "fn": "isSet", + "argv": [ + { + "ref": "requestStringParam" + } + ] + }, + { + "fn": "getAttr", + "argv": [ + { + "ref": "resourceArnList" + }, + "[0]" + ], + "assign": "FirstArn" + }, + { + "fn": "isSet", + "argv": [ + { + "ref": "wholeArnList" + } + ] + }, + { + "fn": "isSet", + "argv": [ + { + "ref": "resourceArnList" + } + ] + }, + { + "fn": "getAttr", + "argv": [ + { + "ref": "wholeArnList" + }, + "[1]" + ], + "assign": "SecondArn" } ], "results": [ @@ -206,7 +324,7 @@ "type": "error" } ], - "root": 2, - "nodeCount": 14, - "nodes": "/////wAAAAH/////AAAAAAAAAA0AAAADAAAAAQAAAAQF9eEMAAAAAgAAAAUF9eEMAAAAAwAAAAgAAAAGAAAABAAAAAcF9eELAAAABQX14QkF9eEKAAAABAAAAAsAAAAJAAAABgAAAAoF9eEIAAAABwX14QYF9eEHAAAABQAAAAwF9eEFAAAABgX14QQF9eEFAAAAAwX14QEAAAAOAAAABAX14QIF9eED" -} \ No newline at end of file + "root": 23, + "nodeCount": 23, + "nodes": "/////wAAAAH/////AAAAAAAAAA0AAAADAAAAAQAAAAQF9eEMAAAAAgAAAAUF9eEMAAAAAwAAAAgAAAAGAAAABAAAAAcF9eELAAAABQX14QkF9eEKAAAABAAAAAsAAAAJAAAABgAAAAoF9eEIAAAABwX14QYF9eEHAAAABQAAAAwF9eEFAAAABgX14QQF9eEFAAAAAwX14QEAAAAOAAAABAX14QIF9eEDAAAACAAAABAAAAAQAAAACQAAABEAAAARAAAACgAAABIAAAASAAAACwAAABMAAAATAAAADAAAABQAAAAUAAAADQAAAAIAAAACAAAADgAAAA8AAAAPAAAADwAAABUAAAAVAAAAEAAAABYAAAAW" +} diff --git a/test/codegen-generated-classes-test/src/main/resources/codegen-resources/bddendpoints/endpoint-rule-set.json b/test/codegen-generated-classes-test/src/main/resources/codegen-resources/bddendpoints/endpoint-rule-set.json index c87dbf336b6d..74d707c19c06 100644 --- a/test/codegen-generated-classes-test/src/main/resources/codegen-resources/bddendpoints/endpoint-rule-set.json +++ b/test/codegen-generated-classes-test/src/main/resources/codegen-resources/bddendpoints/endpoint-rule-set.json @@ -26,6 +26,48 @@ "required": false, "documentation": "Override the endpoint used to send this request", "type": "string" + }, + "AccountId": { + "builtIn": "AWS::Auth::AccountId", + "required": false, + "documentation": "The AWS account ID, read off the resolved identity.", + "type": "string" + }, + "AccountIdEndpointMode": { + "builtIn": "AWS::Auth::AccountIdEndpointMode", + "required": false, + "documentation": "Whether the account ID may be used in the endpoint.", + "type": "string" + }, + "clientStringParam": { + "required": false, + "documentation": "A client context parameter, so the cache key compares it early as reference-stable.", + "type": "string" + }, + "staticStringParam": { + "required": false, + "documentation": "Bound to a per-operation static literal.", + "type": "string" + }, + "requestStringParam": { + "required": false, + "documentation": "Bound to a request member, so it can change per request.", + "type": "string" + }, + "resourceArnList": { + "required": false, + "documentation": "Read only as resourceArnList[0], so the cache key compares just the first element. This is the DynamoDB shape.", + "type": "stringArray" + }, + "unusedStringParam": { + "required": false, + "documentation": "Declared but read by no condition and no result, so the cache key must leave it out.", + "type": "string" + }, + "wholeArnList": { + "required": false, + "documentation": "Read as a whole, so the cache key compares every element.", + "type": "stringArray" } }, "rules": [ @@ -336,4 +378,4 @@ "type": "error" } ] -} \ No newline at end of file +} diff --git a/test/codegen-generated-classes-test/src/main/resources/codegen-resources/bddendpoints/service-2.json b/test/codegen-generated-classes-test/src/main/resources/codegen-resources/bddendpoints/service-2.json index c8ef99738ea8..0b1408801fca 100644 --- a/test/codegen-generated-classes-test/src/main/resources/codegen-resources/bddendpoints/service-2.json +++ b/test/codegen-generated-classes-test/src/main/resources/codegen-resources/bddendpoints/service-2.json @@ -15,6 +15,12 @@ "aws.auth#sigv4" ] }, + "clientContextParams": { + "clientStringParam": { + "documentation": "A client-level string context parameter.", + "type": "string" + } + }, "operations": { "TestOperation": { "name": "TestOperation", @@ -28,6 +34,58 @@ "output": { "shape": "TestOperationResponse" } + }, + "OperationWithStaticParam": { + "name": "OperationWithStaticParam", + "http": { + "method": "POST", + "requestUri": "/" + }, + "staticContextParams": { + "staticStringParam": { + "value": "static-value" + } + }, + "input": { + "shape": "TestOperationRequest" + }, + "output": { + "shape": "TestOperationResponse" + } + }, + "OperationWithContextParam": { + "name": "OperationWithContextParam", + "http": { + "method": "POST", + "requestUri": "/" + }, + "input": { + "shape": "ContextParamInput" + }, + "output": { + "shape": "TestOperationResponse" + } + }, + "OperationWithListContextParam": { + "name": "OperationWithListContextParam", + "http": { + "method": "POST", + "requestUri": "/" + }, + "operationContextParams": { + "resourceArnList": { + "path": "Items[*].Arn" + }, + "wholeArnList": { + "path": "Items[*].Arn" + } + }, + "input": { + "shape": "ListContextParamInput" + }, + "output": { + "shape": "TestOperationResponse" + } } }, "shapes": { @@ -47,6 +105,39 @@ } } }, + "ContextParamInput": { + "type": "structure", + "members": { + "RequestMember": { + "shape": "String", + "contextParam": { + "name": "requestStringParam" + } + } + } + }, + "ListContextParamInput": { + "type": "structure", + "members": { + "Items": { + "shape": "ItemList" + } + } + }, + "ItemList": { + "type": "list", + "member": { + "shape": "Item" + } + }, + "Item": { + "type": "structure", + "members": { + "Arn": { + "shape": "String" + } + } + }, "String": { "type": "string" } diff --git a/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/bddendpoints/BddEndpointProviderCacheTest.java b/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/bddendpoints/BddEndpointProviderCacheTest.java new file mode 100644 index 000000000000..45887d41bc15 --- /dev/null +++ b/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/bddendpoints/BddEndpointProviderCacheTest.java @@ -0,0 +1,508 @@ +/* + * 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.services.bddendpoints; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.concurrent.Callable; +import java.util.concurrent.CompletionException; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.function.Consumer; +import java.util.stream.Collectors; +import java.util.stream.IntStream; +import org.junit.jupiter.api.Test; +import software.amazon.awssdk.endpoints.Endpoint; +import software.amazon.awssdk.regions.Region; +import software.amazon.awssdk.services.bddendpoints.endpoints.BddEndpointsEndpointParams; +import software.amazon.awssdk.services.bddendpoints.endpoints.BddEndpointsEndpointProvider; + +/** + * Behavioural tests for the single-entry result cache generated into the BDD endpoint provider. + * + *

The suite is organised around the one invariant that matters: a cache hit must be indistinguishable from a fresh + * resolution. That splits into two obligations. + * + *

    + *
  1. No stale hit. Changing any single parameter must not return the endpoint resolved for the previous value. + * Every parameter the model declares gets its own test, because a parameter accidentally left out of the + * generated key check is the one defect here that produces a wrong endpoint rather than a slow one.
    2. + *
  2. Hits where they are due. Equal params must actually reuse the cached instance, otherwise the change is + * cost without benefit. Asserted with {@code isSameAs}, which is the only externally visible evidence that the + * cache was consulted.
    3. + *
+ * + *

The model over-declares parameters on purpose, to cover every {@code BddParameterReferences.Usage} value and every + * parameter kind: boolean, string, {@code stringArray}, built-in, client context, static context and request context. + * Several of them cannot change the resolved URL, which makes them the more interesting cases to test: a stale hit is + * detectable only by instance identity, not by comparing hosts. + * + *

The trailing nodes of {@code bddendpoints/endpoint-bdd-1.json} were appended by hand with {@code high == low}, so + * that each parameter is read by a condition without altering any resolved endpoint. A future peephole pass that + * collapses {@code high == low} nodes would silently make those parameters unreferenced and void the coverage above. + */ +class BddEndpointProviderCacheTest { + private static final Region REGION = Region.US_EAST_1; + private static final Region OTHER_REGION = Region.US_WEST_2; + + /** + * Mirrors {@code BddEndpointProviderSpec.MAX_LIST_COMPARISON_SIZE}. Not importable from here, so the cap-related + * tests below derive their sizes from this one constant. + */ + private static final int LIST_SIZE_CAP = 4; + + /** + * Returns params that resolve successfully, with every optional parameter left unset. + */ + private static BddEndpointsEndpointParams.Builder baseBuilder() { + return BddEndpointsEndpointParams.builder() + .region(REGION) + .useDualStack(false) + .useFips(false); + } + + private static BddEndpointsEndpointParams params(Consumer customizer) { + BddEndpointsEndpointParams.Builder builder = baseBuilder(); + customizer.accept(builder); + return builder.build(); + } + + private static BddEndpointsEndpointProvider provider() { + return BddEndpointsEndpointProvider.defaultProvider(); + } + + private static Endpoint resolve(BddEndpointsEndpointProvider provider, BddEndpointsEndpointParams params) { + return provider.resolveEndpoint(params).join(); + } + + /** + * Resolves {@code first}, then {@code second}, and asserts the second call did not reuse the first result. + * + *

Instance identity rather than URL comparison, so this works for the parameters that do not influence the + * resolved URL. Those are exactly the parameters where a missing key check would go unnoticed. + * + *

Identity is a valid miss signal only while every resolution constructs a new {@link Endpoint}. If codegen ever + * hoists constant endpoint results to {@code static final}, this helper and {@link #cacheIsPerProviderInstance()} + * start failing for that reason rather than because the cache regressed. + */ + private static void assertInvalidates(BddEndpointsEndpointParams first, BddEndpointsEndpointParams second) { + BddEndpointsEndpointProvider provider = provider(); + Endpoint firstEndpoint = resolve(provider, first); + Endpoint secondEndpoint = resolve(provider, second); + assertThat(secondEndpoint).isNotSameAs(firstEndpoint); + } + + private static void assertHits(BddEndpointsEndpointParams first, BddEndpointsEndpointParams second) { + BddEndpointsEndpointProvider provider = provider(); + Endpoint firstEndpoint = resolve(provider, first); + Endpoint secondEndpoint = resolve(provider, second); + assertThat(secondEndpoint).isSameAs(firstEndpoint); + } + + // ---- hits ---- + + @Test + void sameParamsInstance_reusesCachedEndpoint() { + BddEndpointsEndpointParams p = params(b -> { + }); + assertHits(p, p); + } + + @Test + void distinctButEqualParams_reusesCachedEndpoint() { + assertHits(params(b -> { + }), params(b -> { + })); + } + + @Test + void cacheIsPerProviderInstance() { + BddEndpointsEndpointParams p = params(b -> { + }); + Endpoint fromFirstProvider = resolve(provider(), p); + Endpoint fromSecondProvider = resolve(provider(), p); + assertThat(fromSecondProvider).isNotSameAs(fromFirstProvider); + assertThat(fromSecondProvider.endpointUrl().host()).isEqualTo(fromFirstProvider.endpointUrl().host()); + } + + /** + * The cached endpoint must be the one the params call for, not merely some previously resolved endpoint. Alternating + * between two parameter sets in a loop would pass even if the cache returned the wrong entry, so each round asserts + * the host as well. + */ + @Test + void alternatingParams_eachResolutionMatchesItsOwnParams() { + BddEndpointsEndpointProvider provider = provider(); + BddEndpointsEndpointParams plain = params(b -> { + }); + BddEndpointsEndpointParams fips = params(b -> b.useFips(true)); + + String plainHost = resolve(provider, plain).endpointUrl().host(); + String fipsHost = resolve(provider, fips).endpointUrl().host(); + assertThat(plainHost).doesNotContain("fips"); + assertThat(fipsHost).contains("fips"); + + for (int i = 0; i < 4; i++) { + assertThat(resolve(provider, plain).endpointUrl().host()).isEqualTo(plainHost); + assertThat(resolve(provider, fips).endpointUrl().host()).isEqualTo(fipsHost); + } + } + + // ---- no stale hit, one test per parameter ---- + + @Test + void useFipsChange_invalidates() { + assertInvalidates(params(b -> { + }), params(b -> b.useFips(true))); + } + + @Test + void useDualStackChange_invalidates() { + assertInvalidates(params(b -> { + }), params(b -> b.useDualStack(true))); + } + + @Test + void regionChange_invalidates() { + assertInvalidates(params(b -> { + }), params(b -> b.region(OTHER_REGION))); + } + + @Test + void clientStringParamChange_invalidates() { + assertInvalidates(params(b -> b.clientStringParam("first")), + params(b -> b.clientStringParam("second"))); + } + + @Test + void staticStringParamChange_invalidates() { + assertInvalidates(params(b -> b.staticStringParam("first")), + params(b -> b.staticStringParam("second"))); + } + + @Test + void endpointOverrideChange_invalidates() { + assertInvalidates(params(b -> b.endpoint("https://first.example.com")), + params(b -> b.endpoint("https://second.example.com"))); + } + + @Test + void accountIdEndpointModeChange_invalidates() { + assertInvalidates(params(b -> b.accountIdEndpointMode("preferred")), + params(b -> b.accountIdEndpointMode("disabled"))); + } + + @Test + void accountIdChange_invalidates() { + assertInvalidates(params(b -> b.accountId("111111111111")), + params(b -> b.accountId("222222222222"))); + } + + @Test + void requestStringParamChange_invalidates() { + assertInvalidates(params(b -> b.requestStringParam("first")), + params(b -> b.requestStringParam("second"))); + } + + // ---- lists read as a whole: wholeArnList, reached via isSet, so every element is part of the key ---- + + @Test + void wholeList_elementChange_invalidates() { + assertInvalidates(params(b -> b.wholeArnList(Arrays.asList("a", "b"))), + params(b -> b.wholeArnList(Arrays.asList("a", "c")))); + } + + @Test + void wholeList_lengthChange_invalidates() { + assertInvalidates(params(b -> b.wholeArnList(Arrays.asList("a", "b"))), + params(b -> b.wholeArnList(Collections.singletonList("a")))); + } + + @Test + void wholeList_orderChange_invalidates() { + assertInvalidates(params(b -> b.wholeArnList(Arrays.asList("a", "b"))), + params(b -> b.wholeArnList(Arrays.asList("b", "a")))); + } + + // ---- lists read only at index 0: resourceArnList, reached via getAttr(list, "[0]") ---- + // + // The BDD's only read of this list is its first element, so nothing past element 0 can reach the endpoint. The key + // therefore compares element 0 alone, which turns changes the endpoint cannot see into hits rather than misses. + // This is the DynamoDB shape, where comparing the whole list costs more than half a regional resolution. + + @Test + void firstElementList_firstElementChange_invalidates() { + assertInvalidates(params(b -> b.resourceArnList(Arrays.asList("a", "b"))), + params(b -> b.resourceArnList(Arrays.asList("z", "b")))); + } + + @Test + void firstElementList_laterElementChange_isAHit() { + assertHits(params(b -> b.resourceArnList(Arrays.asList("a", "b"))), + params(b -> b.resourceArnList(Arrays.asList("a", "c")))); + } + + @Test + void firstElementList_lengthChangeKeepingFirstElement_isAHit() { + assertHits(params(b -> b.resourceArnList(Arrays.asList("a", "b", "c"))), + params(b -> b.resourceArnList(Collections.singletonList("a")))); + } + + @Test + void firstElementList_orderChangeMovingFirstElement_invalidates() { + assertInvalidates(params(b -> b.resourceArnList(Arrays.asList("a", "b"))), + params(b -> b.resourceArnList(Arrays.asList("b", "a")))); + } + + /** + * {@code isSet} can tell an absent list from an empty one, so presence stays part of the key even though + * {@code listAccess} yields null for both. + * + *

Collapsing the two would be sound only for a BDD whose branches for absent and empty converge. DynamoDB's do - + * both reach the same node - but that is a property of the graph rather than of the parameter, so the generated + * comparison does not assume it. The cost is one extra reference check and a miss in this case. + */ + @Test + void firstElementList_emptyAndUnset_areDistinguished() { + assertInvalidates(params(b -> b.resourceArnList(Collections.emptyList())), params(b -> { + })); + } + + /** + * No size cap applies when only the first element is compared, so a list far past the cap still hits. That is the + * point: the comparison is O(1) rather than bounded-but-linear. + */ + @Test + void firstElementList_farPastTheSizeCap_stillHits() { + List long1 = new ArrayList<>(listOfSize(500)); + List long2 = new ArrayList<>(listOfSize(500)); + long2.set(499, "different-tail"); + assertHits(params(b -> b.resourceArnList(long1)), params(b -> b.resourceArnList(long2))); + } + + // ---- parameters the BDD never reads are not part of the key ---- + + /** + * {@code unusedStringParam} is declared by the model and read by no condition and no result, so it cannot change the + * resolved endpoint and must not evict the cached one. + * + *

This is the behaviour that makes the cache worth having for S3, whose rule set declares {@code Key}, + * {@code Prefix} and {@code CopySource} and reads none of them. {@code Key} changes on essentially every object + * request, so treating it as part of the key would mean the cache almost never hits. + */ + @Test + void parameterTheBddNeverReads_doesNotInvalidate() { + assertHits(params(b -> b.unusedStringParam("first")), + params(b -> b.unusedStringParam("second"))); + } + + @Test + void parameterTheBddNeverReads_settingItDoesNotInvalidate() { + assertHits(params(b -> { + }), params(b -> b.unusedStringParam("now-set"))); + } + + // ---- transitions to and from unset ---- + + @Test + void settingAPreviouslyUnsetParam_invalidates() { + assertInvalidates(params(b -> { + }), params(b -> b.requestStringParam("now-set"))); + } + + @Test + void clearingAPreviouslySetParam_invalidates() { + assertInvalidates(params(b -> b.requestStringParam("was-set")), params(b -> { + })); + } + + @Test + void settingAPreviouslyUnsetList_invalidates() { + assertInvalidates(params(b -> { + }), params(b -> b.wholeArnList(Collections.singletonList("a")))); + } + + @Test + void clearingAPreviouslySetList_invalidates() { + assertInvalidates(params(b -> b.wholeArnList(Collections.singletonList("a"))), params(b -> { + })); + } + + @Test + void emptyListAndUnsetList_areDistinguished() { + assertInvalidates(params(b -> b.wholeArnList(Collections.emptyList())), params(b -> { + })); + } + + // ---- equals fallback ---- + + /** + * A parameter compared with {@code Objects.equals} must hit on an equal value arriving as a fresh reference. Without + * the {@code equals} fallback, a request-derived string would miss on every call and the cache would never pay off + * for the services that need it most. + */ + @Test + void equalValueDifferentReference_hits() { + String value = "shared-value"; + String copy = new String(value); + assertThat(value).isNotSameAs(copy); + + assertHits(params(b -> b.requestStringParam(value)), params(b -> b.requestStringParam(copy))); + assertHits(params(b -> b.accountId(value)), params(b -> b.accountId(copy))); + + String url = "https://override.example.com"; + assertHits(params(b -> b.endpoint(url)), params(b -> b.endpoint(new String(url)))); + } + + @Test + void requestList_equalContentsDifferentListInstance_hits() { + assertHits(params(b -> b.wholeArnList(new ArrayList<>(Arrays.asList("a", "b")))), + params(b -> b.wholeArnList(new ArrayList<>(Arrays.asList("a", "b"))))); + } + + /** + * Element comparison also falls back to {@code equals}, so equal strings held by different references still hit. + */ + @Test + void requestList_equalElementsDifferentReferences_hits() { + assertHits(params(b -> b.wholeArnList(Collections.singletonList("element"))), + params(b -> b.wholeArnList(Collections.singletonList(new String("element"))))); + } + + // ---- list size cap ---- + + /** + * At the cap the element walk still runs, so equal lists hit. + */ + @Test + void requestList_atSizeCap_stillHits() { + assertHits(params(b -> b.wholeArnList(listOfSize(LIST_SIZE_CAP))), + params(b -> b.wholeArnList(listOfSize(LIST_SIZE_CAP)))); + } + + /** + * Past the cap the check bails out and reports a miss without walking the elements, which keeps the key check + * bounded. Equal lists therefore stop hitting; that is a deliberate cost ceiling, not a defect, and it is pinned + * here so that changing the cap is a conscious decision. + */ + @Test + void requestList_pastSizeCap_alwaysMisses() { + assertInvalidates(params(b -> b.wholeArnList(listOfSize(LIST_SIZE_CAP + 1))), + params(b -> b.wholeArnList(listOfSize(LIST_SIZE_CAP + 1)))); + } + + /** + * Even a miss must still resolve correctly, so an oversized list is not a functional break. + */ + @Test + void requestList_pastSizeCap_stillResolvesCorrectly() { + Endpoint endpoint = resolve(provider(), params(b -> b.wholeArnList(listOfSize(50)))); + assertThat(endpoint.endpointUrl().host()).isEqualTo("connect.us-east-1.amazonaws.com"); + } + + private static List listOfSize(int size) { + return IntStream.range(0, size).mapToObj(i -> "element-" + i).collect(Collectors.toList()); + } + + // ---- failures are never cached ---- + + /** + * A rule error must not be stored, and must not evict a good entry. Replaying a cached failure would turn one bad + * call into a permanently broken client. + */ + @Test + void ruleError_isNotCached_andLeavesEarlierEntryIntact() { + BddEndpointsEndpointProvider provider = provider(); + BddEndpointsEndpointParams good = params(b -> { + }); + Endpoint first = resolve(provider, good); + + // FIPS combined with an endpoint override is an error in this rule set. + BddEndpointsEndpointParams bad = params(b -> b.useFips(true).endpoint("https://override.example.com")); + assertThatThrownBy(() -> resolve(provider, bad)).isInstanceOf(CompletionException.class); + + assertThat(resolve(provider, good)).isSameAs(first); + // Still an error the second time, rather than a replayed success. + assertThatThrownBy(() -> resolve(provider, bad)).isInstanceOf(CompletionException.class); + } + + @Test + void missingRegion_isNotCached() { + BddEndpointsEndpointProvider provider = provider(); + BddEndpointsEndpointParams noRegion = BddEndpointsEndpointParams.builder() + .useDualStack(false) + .useFips(false) + .build(); + assertThatThrownBy(() -> resolve(provider, noRegion)).isInstanceOf(CompletionException.class); + assertThatThrownBy(() -> resolve(provider, noRegion)).isInstanceOf(CompletionException.class); + + assertThat(resolve(provider, params(b -> { + })).endpointUrl().host()).isEqualTo("connect.us-east-1.amazonaws.com"); + } + + // ---- concurrency ---- + + /** + * Concurrent resolution of two distinct parameter sets against one provider. The cache field is written without any + * lock, so threads race to overwrite it; every thread must still receive the endpoint its own params call for. A + * torn or misattributed entry shows up here as a host mismatch. + */ + @Test + void concurrentResolution_neverReturnsAnotherThreadsEndpoint() throws Exception { + BddEndpointsEndpointProvider provider = provider(); + BddEndpointsEndpointParams plain = params(b -> { + }); + BddEndpointsEndpointParams dualStack = params(b -> b.useDualStack(true)); + String plainHost = resolve(provider(), plain).endpointUrl().host(); + String dualStackHost = resolve(provider(), dualStack).endpointUrl().host(); + assertThat(plainHost).isNotEqualTo(dualStackHost); + + int threads = 16; + int iterations = 500; + ExecutorService executor = Executors.newFixedThreadPool(threads); + try { + CountDownLatch start = new CountDownLatch(1); + List> tasks = new ArrayList<>(); + for (int t = 0; t < threads; t++) { + boolean useDualStack = t % 2 == 0; + BddEndpointsEndpointParams params = useDualStack ? dualStack : plain; + String expectedHost = useDualStack ? dualStackHost : plainHost; + tasks.add(() -> { + start.await(); + for (int i = 0; i < iterations; i++) { + assertThat(resolve(provider, params).endpointUrl().host()).isEqualTo(expectedHost); + } + return null; + }); + } + List> futures = tasks.stream().map(executor::submit).collect(Collectors.toList()); + start.countDown(); + for (Future future : futures) { + future.get(60, TimeUnit.SECONDS); + } + } finally { + executor.shutdownNow(); + } + } +}