From 987fdd519c4c0150226ee72e876f47919fbcd949 Mon Sep 17 00:00:00 2001 From: David Ho Date: Thu, 17 Sep 2026 10:54:31 -0700 Subject: [PATCH 1/5] Lower endpoint operationContextParams JMESPath expressions to typed getters at codegen time --- .../feature-AWSSDKforJavav2-0f38bd9.json | 6 + .../poet/rules/EndpointResolverUtilsSpec.java | 71 +- .../rules/JmesPathTypedGetterGenerator.java | 694 ++++++++++++++++++ .../rules/EndpointResolverUtilsSpecTest.java | 83 +++ .../JmesPathTypedGetterGeneratorTest.java | 209 ++++++ .../client/c2j/stringarray/service-2.json | 89 ++- ...point-resolver-utils-with-stringarray.java | 65 +- ...onContextParamsBindingEquivalenceTest.java | 248 +++++++ ...onContextParamsBindingEquivalenceTest.java | 133 ++++ 9 files changed, 1577 insertions(+), 21 deletions(-) create mode 100644 .changes/next-release/feature-AWSSDKforJavav2-0f38bd9.json create mode 100644 codegen/src/main/java/software/amazon/awssdk/codegen/poet/rules/JmesPathTypedGetterGenerator.java create mode 100644 codegen/src/test/java/software/amazon/awssdk/codegen/poet/rules/JmesPathTypedGetterGeneratorTest.java create mode 100644 services/dynamodb/src/test/java/software/amazon/awssdk/services/dynamodb/endpoints/internal/OperationContextParamsBindingEquivalenceTest.java create mode 100644 services/s3/src/test/java/software/amazon/awssdk/services/s3/endpoints/internal/OperationContextParamsBindingEquivalenceTest.java diff --git a/.changes/next-release/feature-AWSSDKforJavav2-0f38bd9.json b/.changes/next-release/feature-AWSSDKforJavav2-0f38bd9.json new file mode 100644 index 000000000000..483adfd78329 --- /dev/null +++ b/.changes/next-release/feature-AWSSDKforJavav2-0f38bd9.json @@ -0,0 +1,6 @@ +{ + "type": "perf-improvement", + "category": "AWS SDK for Java v2", + "contributor": "", + "description": "Endpoint parameters derived from JMESPath expressions (operationContextParams) are now bound using typed getters generated at build time instead of a reflective runtime, reducing per-request allocation and latency during endpoint resolution for operations that use them (e.g. DynamoDB batch/transaction and Amazon S3 DeleteObjects). Behavior is unchanged." +} 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 cd49ec926a5e..7d8d6a264d7a 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 @@ -24,10 +24,12 @@ import com.squareup.javapoet.CodeBlock; import com.squareup.javapoet.FieldSpec; import com.squareup.javapoet.MethodSpec; +import com.squareup.javapoet.NameAllocator; import com.squareup.javapoet.ParameterizedTypeName; import com.squareup.javapoet.TypeName; import com.squareup.javapoet.TypeSpec; import com.squareup.javapoet.TypeVariableName; +import java.util.ArrayList; import java.util.Collections; import java.util.Iterator; import java.util.List; @@ -53,6 +55,7 @@ import software.amazon.awssdk.codegen.model.service.ContextParam; import software.amazon.awssdk.codegen.model.service.EndpointTrait; import software.amazon.awssdk.codegen.model.service.HostPrefixProcessor; +import software.amazon.awssdk.codegen.model.service.OperationContextParam; import software.amazon.awssdk.codegen.model.service.StaticContextParam; import software.amazon.awssdk.codegen.poet.ClassSpec; import software.amazon.awssdk.codegen.poet.PoetExtension; @@ -90,6 +93,7 @@ public class EndpointResolverUtilsSpec implements ClassSpec { private final EndpointParamsKnowledgeIndex endpointParamsKnowledgeIndex; private final PoetExtension poetExtension; private final JmesPathAcceptorGenerator jmesPathGenerator; + private final JmesPathTypedGetterGenerator jmesPathTypedGetterGenerator; private final boolean dependsOnHttpAuthAws; private final boolean multiAuthSigv4a; private final boolean legacyAuthFromEndpointRulesService; @@ -100,6 +104,7 @@ public EndpointResolverUtilsSpec(IntermediateModel model) { this.endpointParamsKnowledgeIndex = EndpointParamsKnowledgeIndex.of(model); this.poetExtension = new PoetExtension(model); this.jmesPathGenerator = new JmesPathAcceptorGenerator(poetExtension.jmesPathRuntimeClass()); + this.jmesPathTypedGetterGenerator = new JmesPathTypedGetterGenerator(model); Set> supportedAuthSchemes = ModelAuthSchemeClassesKnowledgeIndex.of(model).serviceConcreteAuthSchemeClasses(); @@ -467,28 +472,64 @@ private MethodSpec setOperationContextParamsMethod(OperationModel opModel) { .addParameter(requestClass, "request") .returns(void.class); - b.addStatement("$1T input = new $1T(request)", poetExtension.jmesPathRuntimeClass().nestedClass("Value")); + Map operationContextParams = opModel.getOperationContextParams(); - opModel.getOperationContextParams().forEach((key, value) -> { - if (Objects.requireNonNull(value.getPath().asToken()) == JsonToken.VALUE_STRING) { - String setterName = endpointRulesSpecUtils.paramMethodName(key); - String jmesPathString = ((JrsString) value.getPath()).getValue(); - CodeBlock addParam = CodeBlock.builder() - .add("params.$N(", setterName) - .add(jmesPathGenerator.interpret(jmesPathString, "input")) - .add(matchToParameterType(key)) - .add(")") - .build(); - b.addStatement(addParam); - } else { + // Validated up front rather than while lowering, so a malformed path is reported even when an earlier binding + // sends the operation down the reflective path. + for (OperationContextParam value : operationContextParams.values()) { + JsonToken token = Objects.requireNonNull(value.getPath().asToken()); + if (token != JsonToken.VALUE_STRING) { throw new RuntimeException("Invalid operation context parameter path for " + opModel.getOperationName() + - ". Expected VALUE_STRING, but got " + value.getPath().asToken()); + ". Expected VALUE_STRING, but got " + token); } - }); + } + List loweredBindings = new ArrayList<>(); + NameAllocator names = jmesPathTypedGetterGenerator.newNameAllocator(); + boolean allBindingsLowered = true; + for (Map.Entry entry : operationContextParams.entrySet()) { + String key = entry.getKey(); + OperationContextParam value = entry.getValue(); + String jmesPathString = ((JrsString) value.getPath()).getValue(); + String setterName = endpointRulesSpecUtils.paramMethodName(key); + try { + loweredBindings.add(jmesPathTypedGetterGenerator.lower(opModel.getInputShape(), jmesPathString, + operationContextParamType(key), setterName, names)); + } catch (JmesPathTypedGetterGenerator.UnsupportedLoweringException e) { + allBindingsLowered = false; + break; + } + } + + if (allBindingsLowered) { + loweredBindings.forEach(b::addCode); + return b.build(); + } + + b.addStatement("$1T input = new $1T(request)", poetExtension.jmesPathRuntimeClass().nestedClass("Value")); + operationContextParams.forEach((key, value) -> { + String setterName = endpointRulesSpecUtils.paramMethodName(key); + String jmesPathString = ((JrsString) value.getPath()).getValue(); + CodeBlock addParam = CodeBlock.builder() + .add("params.$N(", setterName) + .add(jmesPathGenerator.interpret(jmesPathString, "input")) + .add(matchToParameterType(key)) + .add(")") + .build(); + b.addStatement(addParam); + }); return b.build(); } + private String operationContextParamType(String paramName) { + Map parameters = model.getEndpointRuleSetModel().getParameters(); + return parameters.entrySet().stream() + .filter(e -> e.getKey().toLowerCase(Locale.US).equals(paramName.toLowerCase(Locale.US))) + .map(e -> e.getValue().getType()) + .findFirst() + .orElse(null); + } + private boolean hasOperationContextParams(OperationModel opModel) { return CollectionUtils.isNotEmpty(opModel.getOperationContextParams()); } diff --git a/codegen/src/main/java/software/amazon/awssdk/codegen/poet/rules/JmesPathTypedGetterGenerator.java b/codegen/src/main/java/software/amazon/awssdk/codegen/poet/rules/JmesPathTypedGetterGenerator.java new file mode 100644 index 000000000000..0d2d140208c9 --- /dev/null +++ b/codegen/src/main/java/software/amazon/awssdk/codegen/poet/rules/JmesPathTypedGetterGenerator.java @@ -0,0 +1,694 @@ +/* + * 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.rules; + +import com.squareup.javapoet.ClassName; +import com.squareup.javapoet.CodeBlock; +import com.squareup.javapoet.NameAllocator; +import com.squareup.javapoet.ParameterizedTypeName; +import com.squareup.javapoet.TypeName; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Locale; +import java.util.Map; +import software.amazon.awssdk.codegen.internal.Utils; +import software.amazon.awssdk.codegen.jmespath.component.AndExpression; +import software.amazon.awssdk.codegen.jmespath.component.BracketSpecifier; +import software.amazon.awssdk.codegen.jmespath.component.BracketSpecifierWithContents; +import software.amazon.awssdk.codegen.jmespath.component.BracketSpecifierWithQuestionMark; +import software.amazon.awssdk.codegen.jmespath.component.BracketSpecifierWithoutContents; +import software.amazon.awssdk.codegen.jmespath.component.ComparatorExpression; +import software.amazon.awssdk.codegen.jmespath.component.CurrentNode; +import software.amazon.awssdk.codegen.jmespath.component.Expression; +import software.amazon.awssdk.codegen.jmespath.component.ExpressionType; +import software.amazon.awssdk.codegen.jmespath.component.FunctionArg; +import software.amazon.awssdk.codegen.jmespath.component.FunctionExpression; +import software.amazon.awssdk.codegen.jmespath.component.IndexExpression; +import software.amazon.awssdk.codegen.jmespath.component.Literal; +import software.amazon.awssdk.codegen.jmespath.component.MultiSelectHash; +import software.amazon.awssdk.codegen.jmespath.component.MultiSelectList; +import software.amazon.awssdk.codegen.jmespath.component.NotExpression; +import software.amazon.awssdk.codegen.jmespath.component.OrExpression; +import software.amazon.awssdk.codegen.jmespath.component.ParenExpression; +import software.amazon.awssdk.codegen.jmespath.component.PipeExpression; +import software.amazon.awssdk.codegen.jmespath.component.SliceExpression; +import software.amazon.awssdk.codegen.jmespath.component.SubExpression; +import software.amazon.awssdk.codegen.jmespath.component.SubExpressionRight; +import software.amazon.awssdk.codegen.jmespath.component.WildcardExpression; +import software.amazon.awssdk.codegen.jmespath.parser.JmesPathParser; +import software.amazon.awssdk.codegen.jmespath.parser.JmesPathVisitor; +import software.amazon.awssdk.codegen.model.intermediate.IntermediateModel; +import software.amazon.awssdk.codegen.model.intermediate.MemberModel; +import software.amazon.awssdk.codegen.model.intermediate.ShapeModel; +import software.amazon.awssdk.codegen.poet.model.TypeProvider; + +/** + * Generates direct getter bindings for a validated subset of endpoint operation-context JMESPath expressions. + * Expressions outside the subset throw {@link UnsupportedLoweringException} before code is emitted. + */ +final class JmesPathTypedGetterGenerator { + + /** + * Signals that an expression cannot be lowered and the caller should fall back to the reflective runtime. A + * dedicated type so that the fallback catch cannot mask an unexpected exception from a genuine codegen bug. + */ + static final class UnsupportedLoweringException extends RuntimeException { + UnsupportedLoweringException(String message) { + super(message); + } + } + + private static final TypeName STRING_TYPE = ClassName.get(String.class); + private static final TypeName BOOLEAN_TYPE = ClassName.get(Boolean.class); + + private final IntermediateModel model; + private final TypeProvider typeProvider; + + JmesPathTypedGetterGenerator(IntermediateModel model) { + this.model = model; + this.typeProvider = new TypeProvider(model); + } + + NameAllocator newNameAllocator() { + NameAllocator names = new NameAllocator(); + names.newName("params"); + names.newName("request"); + names.newName("input"); + return names; + } + + CodeBlock lower(ShapeModel inputShape, String jmesPath, String terminalType, String setterName) { + return lower(inputShape, jmesPath, terminalType, setterName, newNameAllocator()); + } + + CodeBlock lower(ShapeModel inputShape, String jmesPath, String terminalType, String setterName, NameAllocator names) { + String normalizedTerminalType = terminalType == null ? "" : terminalType.toLowerCase(Locale.US); + if (!"string".equals(normalizedTerminalType) + && !"boolean".equals(normalizedTerminalType) + && !"stringarray".equals(normalizedTerminalType)) { + throw new UnsupportedLoweringException("Unsupported endpoint parameter type for " + setterName); + } + + List steps = new StepCollector().collect(JmesPathParser.parse(jmesPath)); + boolean listTerminal = "stringarray".equals(normalizedTerminalType); + + if (steps.size() == 1 && steps.get(0) instanceof KeysStep) { + if (!listTerminal) { + throw new UnsupportedLoweringException("keys() result can only bind a stringarray parameter"); + } + List argument = ((KeysStep) steps.get(0)).arg; + validateKeys(inputShape, argument); + return emitKeys(inputShape, argument, setterName, names); + } + if (steps.stream().anyMatch(s -> s instanceof KeysStep)) { + throw new UnsupportedLoweringException("keys() is only supported as the top-level expression"); + } + + int projectionIndex = indexOfProjection(steps); + if (projectionIndex < 0) { + if (listTerminal) { + throw new UnsupportedLoweringException("List parameter requires a projection or keys()"); + } + List fields = fieldNames(steps); + MemberModel leaf = resolvePath(inputShape, fields); + validateScalarTerminal(leaf, normalizedTerminalType); + return emitScalarPath(inputShape, fields, setterName, names); + } + + if (!listTerminal) { + throw new UnsupportedLoweringException("Projection result can only bind a stringarray parameter"); + } + ProjectionParts parts = projectionParts(steps, projectionIndex); + validateProjection(inputShape, parts); + return emitProjection(inputShape, parts, setterName, names); + } + + private ShapeModel targetShape(MemberModel member) { + if (member.getShape() != null) { + return member.getShape(); + } + return model.getShapes().get(member.getC2jShape()); + } + + private CodeBlock emitKeys(ShapeModel inputShape, List argSteps, String setterName, NameAllocator names) { + List fields = fieldNames(argSteps); + CodeBlock.Builder b = CodeBlock.builder(); + + TypeName resultType = ParameterizedTypeName.get(ClassName.get(List.class), ClassName.get(String.class)); + String resultVar = names.newName(setterName); + // If the prefix is guarded, the loop may never run: seed the immutable empty list that the reflective + // runtime's stringValues() returns for a null prefix. + boolean prefixGuarded = fields.size() > 1; + if (prefixGuarded) { + b.addStatement("$T $N = $T.emptyList()", resultType, resultVar, ClassName.get(Collections.class)); + } else { + b.addStatement("$T $N = new $T<>()", resultType, resultVar, ClassName.get(ArrayList.class)); + } + + Walk walk = walkToLast(b, names, inputShape, fields); + if (prefixGuarded) { + b.addStatement("$N = new $T<>()", resultVar, ClassName.get(ArrayList.class)); + } + String keyVar = names.newName("key"); + b.beginControlFlow("for ($T $N : new $T<>($L).keySet())", String.class, keyVar, HashMap.class, + accessLast(walk, walk.lastMember)); + b.beginControlFlow("if ($N != null)", keyVar); + b.addStatement("$N.add($N)", resultVar, keyVar); + b.endControlFlow(); + b.endControlFlow(); + for (int i = 0; i < walk.openedGuards; i++) { + b.endControlFlow(); + } + b.addStatement("params.$N($N)", setterName, resultVar); + return b.build(); + } + + private CodeBlock emitScalarPath(ShapeModel inputShape, List fields, String setterName, + NameAllocator names) { + CodeBlock.Builder b = CodeBlock.builder(); + CodeBlock value = scalarChain(b, names, inputShape, fields); + b.addStatement("params.$N($L)", setterName, value); + return b.build(); + } + + private CodeBlock emitProjection(ShapeModel inputShape, ProjectionParts parts, + String setterName, NameAllocator names) { + CodeBlock.Builder b = CodeBlock.builder(); + TypeName listElementType = ParameterizedTypeName.get(ClassName.get(List.class), ClassName.get(String.class)); + String resultVar = names.newName(setterName); + // walkToLast guards every prefix field but the last, so a single-field prefix always reaches the loop. If the + // prefix is guarded, seed the immutable empty list that the reflective runtime's stringValues() returns for a + // null prefix. + boolean prefixGuarded = parts.prefix.size() > 1; + if (prefixGuarded) { + b.addStatement("$T $N = $T.emptyList()", listElementType, resultVar, ClassName.get(Collections.class)); + } else { + b.addStatement("$T $N = new $T<>()", listElementType, resultVar, ClassName.get(ArrayList.class)); + } + + Walk walk = walkToLast(b, names, inputShape, parts.prefix); + if (prefixGuarded) { + b.addStatement("$N = new $T<>()", resultVar, ClassName.get(ArrayList.class)); + } + MemberModel listMember = walk.lastMember; + MemberModel elementMember = listMember.getListModel().getListMemberModel(); + ShapeModel elementShape = targetShape(elementMember); + TypeName elementType = typeProvider.returnType(elementMember); + String elementVar = names.newName(elementBaseName(elementMember)); + + b.beginControlFlow("for ($T $N : $L)", elementType, elementVar, accessLast(walk, listMember)); + b.beginControlFlow("if ($N != null)", elementVar); + if (parts.isMultiSelect()) { + for (List branch : parts.multiSelect().branches) { + emitLeafAppend(b, names, elementVar, elementShape, fieldNames(branch), resultVar); + } + } else { + emitLeafAppend(b, names, elementVar, elementShape, fieldNames(parts.rest), resultVar); + } + b.endControlFlow(); + b.endControlFlow(); + for (int i = 0; i < walk.openedGuards; i++) { + b.endControlFlow(); + } + b.addStatement("params.$N($N)", setterName, resultVar); + return b.build(); + } + + /** + * Emit a null-guarded walk of {@code fields} starting from {@code startVar} (of {@code startShape}), appending the non-null + * scalar leaf to {@code resultVar}. Opens and closes its own guard blocks (balanced). + */ + private void emitLeafAppend(CodeBlock.Builder b, NameAllocator names, String startVar, ShapeModel startShape, + List fields, String resultVar) { + ShapeModel current = startShape; + String parentVar = startVar; + int opened = 0; + for (int i = 0; i < fields.size() - 1; i++) { + MemberModel m = resolve(current, fields.get(i)); + if (targetShape(m) == null) { + throw new UnsupportedLoweringException("Field path traverses a non-structure member: " + fields.get(i)); + } + String var = names.newName(baseName(m)); + b.addStatement("$T $N = $N.$N()", typeProvider.returnType(m), var, parentVar, m.getFluentGetterMethodName()); + b.beginControlFlow("if ($N != null)", var); + opened++; + parentVar = var; + current = targetShape(m); + } + MemberModel leaf = resolve(current, fields.get(fields.size() - 1)); + String leafVar = names.newName(baseName(leaf)); + b.addStatement("$T $N = $N.$N()", typeProvider.returnType(leaf), leafVar, parentVar, leaf.getFluentGetterMethodName()); + b.beginControlFlow("if ($N != null)", leafVar); + b.addStatement("$N.add($N)", resultVar, leafVar); + b.endControlFlow(); + for (int i = 0; i < opened; i++) { + b.endControlFlow(); + } + } + + private void validateKeys(ShapeModel inputShape, List argument) { + MemberModel mapMember = resolvePath(inputShape, fieldNames(argument)); + if (!mapMember.isMap()) { + throw new UnsupportedLoweringException("keys() argument must resolve to a map"); + } + requireType(mapMember.getMapModel().getKeyModel(), STRING_TYPE, "keys() map key"); + } + + private void validateProjection(ShapeModel inputShape, ProjectionParts parts) { + MemberModel listMember = resolvePath(inputShape, parts.prefix); + if (!listMember.isList()) { + throw new UnsupportedLoweringException("Projection [*] must apply to a list member"); + } + if (parts.rest.isEmpty()) { + throw new UnsupportedLoweringException("Projection must select a scalar field"); + } + + MemberModel elementMember = listMember.getListModel().getListMemberModel(); + ShapeModel elementShape = targetShape(elementMember); + if (parts.isMultiSelect()) { + if (!parts.flatten) { + throw new UnsupportedLoweringException("multi-select-list in a projection must be followed by []"); + } + for (List branch : parts.multiSelect().branches) { + MemberModel leaf = resolvePath(elementShape, fieldNames(branch)); + requireType(leaf, STRING_TYPE, "multi-select-list leaf"); + } + return; + } + if (parts.flatten) { + throw new UnsupportedLoweringException("Flatten is only supported after a multi-select-list"); + } + MemberModel leaf = resolvePath(elementShape, fieldNames(parts.rest)); + requireType(leaf, STRING_TYPE, "projection leaf"); + } + + private void validateScalarTerminal(MemberModel leaf, String terminalType) { + if ("string".equals(terminalType)) { + requireType(leaf, STRING_TYPE, "string terminal"); + } else if ("boolean".equals(terminalType)) { + requireType(leaf, BOOLEAN_TYPE, "boolean terminal"); + } else { + throw new UnsupportedLoweringException("Unsupported scalar endpoint parameter type: " + terminalType); + } + } + + private void requireType(MemberModel member, TypeName expected, String description) { + if (!expected.equals(typeProvider.returnType(member))) { + throw new UnsupportedLoweringException(description + " has incompatible type"); + } + } + + private MemberModel resolvePath(ShapeModel startShape, List fields) { + if (fields.isEmpty()) { + throw new UnsupportedLoweringException("Field path must not be empty"); + } + ShapeModel current = startShape; + MemberModel member = null; + for (int i = 0; i < fields.size(); i++) { + member = resolve(current, fields.get(i)); + if (hasRuntimeDefault(member)) { + throw new UnsupportedLoweringException("Field uses an SdkField runtime default: " + fields.get(i)); + } + if (i < fields.size() - 1) { + current = targetShape(member); + if (current == null) { + throw new UnsupportedLoweringException("Field path traverses a non-structure member: " + fields.get(i)); + } + } + } + return member; + } + + private boolean hasRuntimeDefault(MemberModel member) { + if (member.isIdempotencyToken()) { + return true; + } + if (model.getCustomizationConfig() == null) { + return false; + } + Map defaults = model.getCustomizationConfig().getModelMarshallerDefaultValueSupplier(); + return defaults != null && defaults.containsKey(member.getC2jName()); + } + + /** + * Walk a scalar field path, emitting a null-guarded local per intermediate hop as a ternary (never an {@code if} block, so + * the setter is still invoked with {@code null} when the path breaks, matching the reflective runtime). Returns a null-safe + * expression yielding the final member's value. + */ + private CodeBlock scalarChain(CodeBlock.Builder b, NameAllocator names, ShapeModel inputShape, List fields) { + ShapeModel current = inputShape; + String prevVar = null; + for (int i = 0; i < fields.size() - 1; i++) { + MemberModel m = resolve(current, fields.get(i)); + if (targetShape(m) == null) { + throw new UnsupportedLoweringException("Field path traverses a non-structure member: " + fields.get(i)); + } + String var = names.newName(baseName(m)); + if (prevVar == null) { + b.addStatement("$T $N = request.$N()", typeProvider.returnType(m), var, m.getFluentGetterMethodName()); + } else { + b.addStatement("$T $N = $N == null ? null : $N.$N()", typeProvider.returnType(m), var, prevVar, prevVar, + m.getFluentGetterMethodName()); + } + prevVar = var; + current = targetShape(m); + } + MemberModel last = resolve(current, fields.get(fields.size() - 1)); + if (prevVar == null) { + return CodeBlock.of("request.$N()", last.getFluentGetterMethodName()); + } + return CodeBlock.of("$N == null ? null : $N.$N()", prevVar, prevVar, last.getFluentGetterMethodName()); + } + + /** + * Result of walking a field path: the resolved terminal member, the code expression for its parent object, and how many + * guard control-flow blocks were opened (which the caller must close). + */ + private static final class Walk { + private final MemberModel lastMember; + private final CodeBlock parentExpr; + private final int openedGuards; + + private Walk(MemberModel lastMember, CodeBlock parentExpr, int openedGuards) { + this.lastMember = lastMember; + this.parentExpr = parentExpr; + this.openedGuards = openedGuards; + } + } + + /** + * Walk all-but-last fields as null-guarded locals rooted at {@code request}, returning the parent expression from which the + * last field is accessed. Guards are opened with {@code beginControlFlow} and must be closed by the caller. + */ + private Walk walkToLast(CodeBlock.Builder b, NameAllocator names, ShapeModel inputShape, List fields) { + ShapeModel current = inputShape; + CodeBlock parentExpr = CodeBlock.of("request"); + int opened = 0; + for (int i = 0; i < fields.size() - 1; i++) { + MemberModel m = resolve(current, fields.get(i)); + if (targetShape(m) == null) { + throw new UnsupportedLoweringException("Field path traverses a non-structure member: " + fields.get(i)); + } + String var = names.newName(baseName(m)); + b.addStatement("$T $N = $L.$N()", typeProvider.returnType(m), var, parentExpr, m.getFluentGetterMethodName()); + b.beginControlFlow("if ($N != null)", var); + opened++; + parentExpr = CodeBlock.of("$N", var); + current = targetShape(m); + } + MemberModel last = resolve(current, fields.get(fields.size() - 1)); + return new Walk(last, parentExpr, opened); + } + + /** + * Expression reading the walked path's final member from its parent. Safe to consume without a null guard only + * for collection members: generated list and map getters return auto-construct empties, never null. + */ + private CodeBlock accessLast(Walk walk, MemberModel lastMember) { + return CodeBlock.of("$L.$N()", walk.parentExpr, lastMember.getFluentGetterMethodName()); + } + + private MemberModel resolve(ShapeModel shape, String c2jName) { + if (shape == null) { + throw new UnsupportedLoweringException("Cannot resolve '" + c2jName + "' against a non-structure"); + } + MemberModel member = shape.getMemberByC2jName(c2jName); + if (member == null) { + throw new UnsupportedLoweringException("No member '" + c2jName + "' on shape " + shape.getShapeName()); + } + return member; + } + + private String baseName(MemberModel m) { + return m.getVariable().getVariableName(); + } + + private String elementBaseName(MemberModel elementMember) { + ShapeModel shape = targetShape(elementMember); + if (shape != null && shape.getShapeName() != null) { + return Utils.unCapitalize(shape.getShapeName()); + } + return "item"; + } + + private static int indexOfProjection(List steps) { + for (int i = 0; i < steps.size(); i++) { + if (steps.get(i) instanceof ProjectionStep) { + return i; + } + } + return -1; + } + + private static List fieldNames(List steps) { + List names = new ArrayList<>(); + for (Step s : steps) { + if (!(s instanceof FieldStep)) { + throw new UnsupportedLoweringException("Expected a plain field path"); + } + names.add(((FieldStep) s).c2jName); + } + return names; + } + + /** + * The structural decomposition of a projection expression, computed once so that validation and emission cannot + * disagree about the shape they are processing. + */ + private static final class ProjectionParts { + /** Field path before the projection. */ + private final List prefix; + /** Steps applied to each projected element, with a trailing flatten already removed. */ + private final List rest; + private final boolean flatten; + + private ProjectionParts(List prefix, List rest, boolean flatten) { + this.prefix = prefix; + this.rest = rest; + this.flatten = flatten; + } + + private boolean isMultiSelect() { + return rest.size() == 1 && rest.get(0) instanceof MultiSelectStep; + } + + private MultiSelectStep multiSelect() { + return (MultiSelectStep) rest.get(0); + } + } + + private static ProjectionParts projectionParts(List steps, int projectionIndex) { + List prefix = fieldNames(steps.subList(0, projectionIndex)); + List rest = new ArrayList<>(steps.subList(projectionIndex + 1, steps.size())); + boolean flatten = !rest.isEmpty() && rest.get(rest.size() - 1) instanceof FlattenStep; + if (flatten) { + rest.remove(rest.size() - 1); + } + return new ProjectionParts(prefix, rest, flatten); + } + + private interface Step { + } + + private static final class FieldStep implements Step { + private final String c2jName; + + private FieldStep(String c2jName) { + this.c2jName = c2jName; + } + } + + private static final class ProjectionStep implements Step { + } + + private static final class FlattenStep implements Step { + } + + private static final class MultiSelectStep implements Step { + private final List> branches; + + private MultiSelectStep(List> branches) { + this.branches = branches; + } + } + + private static final class KeysStep implements Step { + private final List arg; + + private KeysStep(List arg) { + this.arg = arg; + } + } + + /** + * Flattens the parsed JMESPath AST into a linear {@link Step} list. Any node type outside the supported subset + * throws {@link UnsupportedLoweringException}. + */ + private static final class StepCollector implements JmesPathVisitor { + private final List steps = new ArrayList<>(); + + private List collect(Expression expression) { + expression.visit(this); + return steps; + } + + @Override + public void visitExpression(Expression input) { + input.visit(this); + } + + @Override + public void visitSubExpression(SubExpression input) { + input.leftExpression().visit(this); + visitSubExpressionRight(input.rightSubExpression()); + } + + @Override + public void visitSubExpressionRight(SubExpressionRight input) { + input.visit(this); + } + + @Override + public void visitIndexExpression(IndexExpression input) { + input.expression().ifPresent(e -> e.visit(this)); + input.bracketSpecifier().visit(this); + } + + @Override + public void visitBracketSpecifier(BracketSpecifier input) { + input.visit(this); + } + + @Override + public void visitBracketSpecifierWithContents(BracketSpecifierWithContents input) { + if (input.isWildcardExpression()) { + steps.add(new ProjectionStep()); + } else if (input.isMultiSelectList()) { + visitMultiSelectList(input.asMultiSelectList()); + } else { + throw new UnsupportedLoweringException("Unsupported bracket specifier"); + } + } + + @Override + public void visitBracketSpecifierWithoutContents(BracketSpecifierWithoutContents input) { + steps.add(new FlattenStep()); + } + + @Override + public void visitBracketSpecifierWithQuestionMark(BracketSpecifierWithQuestionMark input) { + throw new UnsupportedLoweringException("Filter expressions are not supported"); + } + + @Override + public void visitWildcardExpression(WildcardExpression input) { + steps.add(new ProjectionStep()); + } + + @Override + public void visitMultiSelectList(MultiSelectList input) { + List> branches = new ArrayList<>(); + for (Expression expression : input.expressions()) { + branches.add(new StepCollector().collect(expression)); + } + steps.add(new MultiSelectStep(branches)); + } + + @Override + public void visitFunctionExpression(FunctionExpression input) { + if (!"keys".equals(input.function())) { + throw new UnsupportedLoweringException("Unsupported function: " + input.function()); + } + List args = input.functionArgs(); + if (args.size() != 1 || !args.get(0).isExpression()) { + throw new UnsupportedLoweringException("keys() requires a single expression argument"); + } + steps.add(new KeysStep(new StepCollector().collect(args.get(0).asExpression()))); + } + + @Override + public void visitIdentifier(String input) { + steps.add(new FieldStep(input)); + } + + @Override + public void visitSliceExpression(SliceExpression input) { + throw new UnsupportedLoweringException("slice expression is not supported"); + } + + @Override + public void visitComparatorExpression(ComparatorExpression input) { + throw new UnsupportedLoweringException("comparator expression is not supported"); + } + + @Override + public void visitOrExpression(OrExpression input) { + throw new UnsupportedLoweringException("or expression is not supported"); + } + + @Override + public void visitAndExpression(AndExpression input) { + throw new UnsupportedLoweringException("and expression is not supported"); + } + + @Override + public void visitNotExpression(NotExpression input) { + throw new UnsupportedLoweringException("not expression is not supported"); + } + + @Override + public void visitParenExpression(ParenExpression input) { + throw new UnsupportedLoweringException("paren expression is not supported"); + } + + @Override + public void visitMultiSelectHash(MultiSelectHash input) { + throw new UnsupportedLoweringException("multi select hash is not supported"); + } + + @Override + public void visitExpressionType(ExpressionType input) { + throw new UnsupportedLoweringException("expression type is not supported"); + } + + @Override + public void visitPipeExpression(PipeExpression input) { + throw new UnsupportedLoweringException("pipe expression is not supported"); + } + + @Override + public void visitCurrentNode(CurrentNode input) { + throw new UnsupportedLoweringException("current node is not supported"); + } + + @Override + public void visitRawString(String input) { + throw new UnsupportedLoweringException("raw string is not supported"); + } + + @Override + public void visitLiteral(Literal input) { + throw new UnsupportedLoweringException("literal is not supported"); + } + + @Override + public void visitNumber(int input) { + throw new UnsupportedLoweringException("number is not supported"); + } + } +} diff --git a/codegen/src/test/java/software/amazon/awssdk/codegen/poet/rules/EndpointResolverUtilsSpecTest.java b/codegen/src/test/java/software/amazon/awssdk/codegen/poet/rules/EndpointResolverUtilsSpecTest.java index ba70047a40db..b1e0ba624253 100644 --- a/codegen/src/test/java/software/amazon/awssdk/codegen/poet/rules/EndpointResolverUtilsSpecTest.java +++ b/codegen/src/test/java/software/amazon/awssdk/codegen/poet/rules/EndpointResolverUtilsSpecTest.java @@ -16,11 +16,24 @@ package software.amazon.awssdk.codegen.poet.rules; import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import static software.amazon.awssdk.codegen.poet.PoetMatchers.generatesTo; +import com.fasterxml.jackson.core.TreeNode; +import com.fasterxml.jackson.jr.stree.JrsNumber; +import com.fasterxml.jackson.jr.stree.JrsString; +import java.util.LinkedHashMap; +import java.util.Map; import org.junit.jupiter.api.Test; import software.amazon.awssdk.codegen.poet.ClassSpec; +import software.amazon.awssdk.codegen.model.intermediate.IntermediateModel; +import software.amazon.awssdk.codegen.model.intermediate.OperationModel; +import software.amazon.awssdk.codegen.model.rules.endpoints.ParameterModel; +import software.amazon.awssdk.codegen.model.service.OperationContextParam; import software.amazon.awssdk.codegen.poet.ClientTestModels; +import software.amazon.awssdk.codegen.poet.PoetUtils; public class EndpointResolverUtilsSpecTest { @@ -48,4 +61,74 @@ void endpointResolverUtilsClassWithStringArray() { ClassSpec spec = new EndpointResolverUtilsSpec(ClientTestModels.stringArrayServiceModels()); assertThat(spec, generatesTo("endpoint-resolver-utils-with-stringarray.java")); } + + @Test + void multipleLoweredBindingsUseUniqueNames() { + IntermediateModel model = ClientTestModels.stringArrayServiceModels(); + addStringArrayBinding(model, "ListOfObjectsOperation", "stringArrayParam2", + "nested.listOfObjects[*].key"); + + String generated = PoetUtils.buildJavaFile(new EndpointResolverUtilsSpec(model)).toString(); + String method = operationBindingMethod(generated, "ListOfObjectsOperationRequest request"); + + assertTrue(method.contains("Nested nested = request.nested()")); + assertTrue(method.contains("Nested nested_ = request.nested()")); + } + + @Test + void unsupportedBindingFallsBackForWholeOperation() { + IntermediateModel model = ClientTestModels.stringArrayServiceModels(); + addStringArrayBinding(model, "ListOfObjectsOperation", "stringArrayParam2", "nested.listOfObjects"); + + String generated = PoetUtils.buildJavaFile(new EndpointResolverUtilsSpec(model)).toString(); + String method = operationBindingMethod(generated, "ListOfObjectsOperationRequest request"); + + assertTrue(method.contains("JmesPathRuntime.Value input = new JmesPathRuntime.Value(request)")); + assertTrue(method.contains("params.stringArrayParam(input.field(\"nested\")")); + assertTrue(method.contains("params.stringArrayParam2(input.field(\"nested\")")); + assertFalse(method.contains("List stringArrayParam = new ArrayList<>()")); + } + + @Test + void malformedPathOrderedAfterUnsupportedBindingStillReportsTheOperation() { + IntermediateModel model = ClientTestModels.stringArrayServiceModels(); + addStringArrayBinding(model, "ListOfObjectsOperation", "stringArrayParam2", "nested.listOfObjects"); + addBinding(model, "ListOfObjectsOperation", "stringArrayParam3", new JrsNumber(1)); + + RuntimeException e = assertThrows(RuntimeException.class, + () -> PoetUtils.buildJavaFile(new EndpointResolverUtilsSpec(model))); + + assertTrue(e.getMessage() != null && e.getMessage().contains("ListOfObjectsOperation"), + "expected the descriptive model error naming the operation, but got: " + e); + assertTrue(e.getMessage().contains("VALUE_NUMBER_INT")); + } + + private static void addStringArrayBinding(IntermediateModel model, String operationName, String parameterName, + String path) { + addBinding(model, operationName, parameterName, new JrsString(path)); + } + + private static void addBinding(IntermediateModel model, String operationName, String parameterName, + TreeNode path) { + ParameterModel parameter = model.getEndpointRuleSetModel().getParameters().get("stringArrayParam"); + model.getEndpointRuleSetModel().getParameters().put(parameterName, parameter); + + OperationContextParam operationContextParam = new OperationContextParam(); + operationContextParam.setPath(path); + OperationModel operation = model.getOperation(operationName); + Map params = new LinkedHashMap<>(operation.getOperationContextParams()); + params.put(parameterName, operationContextParam); + operation.setOperationContextParams(params); + } + + private static String operationBindingMethod(String generated, String requestParameter) { + int request = generated.indexOf(requestParameter); + int start = generated.lastIndexOf("private static void setOperationContextParams", request); + int end = generated.indexOf("private static void", request + requestParameter.length()); + if (end < 0) { + end = generated.length(); + } + return generated.substring(start, end); + } + } diff --git a/codegen/src/test/java/software/amazon/awssdk/codegen/poet/rules/JmesPathTypedGetterGeneratorTest.java b/codegen/src/test/java/software/amazon/awssdk/codegen/poet/rules/JmesPathTypedGetterGeneratorTest.java new file mode 100644 index 000000000000..9edbde48def0 --- /dev/null +++ b/codegen/src/test/java/software/amazon/awssdk/codegen/poet/rules/JmesPathTypedGetterGeneratorTest.java @@ -0,0 +1,209 @@ +/* + * 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.rules; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import com.squareup.javapoet.CodeBlock; +import com.squareup.javapoet.NameAllocator; +import java.util.Collections; +import org.junit.jupiter.api.Test; +import software.amazon.awssdk.codegen.model.intermediate.IntermediateModel; +import software.amazon.awssdk.codegen.model.intermediate.ShapeModel; +import software.amazon.awssdk.codegen.poet.ClientTestModels; + +class JmesPathTypedGetterGeneratorTest { + + @Test + void sharedAllocatorAvoidsMethodParametersAndPreviousBindings() { + IntermediateModel model = ClientTestModels.stringArrayServiceModels(); + JmesPathTypedGetterGenerator generator = new JmesPathTypedGetterGenerator(model); + ShapeModel input = model.getOperation("ListOfObjectsOperation").getInputShape(); + NameAllocator names = generator.newNameAllocator(); + + CodeBlock first = generator.lower(input, "Request.Value", "string", "firstParam", names); + CodeBlock second = generator.lower(input, "Request.Value", "string", "secondParam", names); + String generated = first.toString() + second.toString(); + + assertTrue(generated.contains("request_ = request.request()")); + assertTrue(generated.contains("request__ = request.request()")); + } + + @Test + void bareProjectionFallsBack() { + IntermediateModel model = ClientTestModels.stringArrayServiceModels(); + JmesPathTypedGetterGenerator generator = new JmesPathTypedGetterGenerator(model); + ShapeModel input = model.getOperation("TransactionOperation").getInputShape(); + + assertThrows(JmesPathTypedGetterGenerator.UnsupportedLoweringException.class, + () -> generator.lower(input, "TransactItems[*]", "stringarray", "stringArrayParam")); + } + + @Test + void collectionFlattenFallsBack() { + IntermediateModel model = ClientTestModels.stringArrayServiceModels(); + JmesPathTypedGetterGenerator generator = new JmesPathTypedGetterGenerator(model); + ShapeModel input = model.getOperation("ListOfObjectsOperation").getInputShape(); + + assertThrows(JmesPathTypedGetterGenerator.UnsupportedLoweringException.class, + () -> generator.lower(input, "nested.listOfObjects[*].aliases[]", "stringarray", + "stringArrayParam")); + } + + @Test + void incompatibleTerminalConversionFallsBack() { + IntermediateModel model = ClientTestModels.stringArrayServiceModels(); + JmesPathTypedGetterGenerator generator = new JmesPathTypedGetterGenerator(model); + ShapeModel input = model.getOperation("ListOfObjectsOperation").getInputShape(); + + assertThrows(JmesPathTypedGetterGenerator.UnsupportedLoweringException.class, + () -> generator.lower(input, "Request.Value", "boolean", "booleanParam")); + } + + @Test + void idempotencyTokenFallsBack() { + IntermediateModel model = ClientTestModels.stringArrayServiceModels(); + JmesPathTypedGetterGenerator generator = new JmesPathTypedGetterGenerator(model); + ShapeModel input = model.getOperation("ListOfObjectsOperation").getInputShape(); + + assertThrows(JmesPathTypedGetterGenerator.UnsupportedLoweringException.class, + () -> generator.lower(input, "Request.Token", "string", "stringParam")); + } + + /** + * A boolean leaf three hops deep exercises the boolean terminal check and the mid-chain ternary guard. + */ + @Test + void booleanLeafThroughDeepScalarChainLowers() { + IntermediateModel model = ClientTestModels.stringArrayServiceModels(); + JmesPathTypedGetterGenerator generator = new JmesPathTypedGetterGenerator(model); + ShapeModel input = model.getOperation("ListOfObjectsOperation").getInputShape(); + + CodeBlock generated = generator.lower(input, "Request.Inner.Flag", "boolean", "booleanParam"); + + String code = generated.toString(); + assertTrue(code.contains("inner = request_ == null ? null : request_.inner()"), + "expected a ternary-guarded mid-chain hop, but got: " + code); + assertTrue(code.contains("params.booleanParam(inner == null ? null : inner.flag())"), + "expected the setter to be invoked with a null-safe boolean leaf, but got: " + code); + } + + @Test + void customizedRuntimeDefaultFallsBack() { + IntermediateModel model = ClientTestModels.stringArrayServiceModels(); + model.getCustomizationConfig().setModelMarshallerDefaultValueSupplier( + Collections.singletonMap("Value", "example.DefaultSupplier")); + JmesPathTypedGetterGenerator generator = new JmesPathTypedGetterGenerator(model); + ShapeModel input = model.getOperation("ListOfObjectsOperation").getInputShape(); + + assertThrows(JmesPathTypedGetterGenerator.UnsupportedLoweringException.class, + () -> generator.lower(input, "Request.Value", "string", "stringParam")); + } + + @Test + void keysFiltersNullEntries() { + IntermediateModel model = ClientTestModels.stringArrayServiceModels(); + JmesPathTypedGetterGenerator generator = new JmesPathTypedGetterGenerator(model); + ShapeModel input = model.getOperation("MapKeysOperation").getInputShape(); + + CodeBlock generated = generator.lower(input, "keys(RequestItems)", "stringarray", "stringArrayParam"); + + assertTrue(generated.toString().contains("if (key != null)")); + } + + @Test + void keysLoopVariableDoesNotShadowResultLocal() { + IntermediateModel model = ClientTestModels.stringArrayServiceModels(); + JmesPathTypedGetterGenerator generator = new JmesPathTypedGetterGenerator(model); + ShapeModel input = model.getOperation("MapKeysOperation").getInputShape(); + + CodeBlock generated = generator.lower(input, "keys(RequestItems)", "stringarray", "key"); + + assertFalse(generated.toString().contains("for (java.lang.String key :")); + assertFalse(generated.toString().contains("key.add(key)")); + } + + @Test + void keysLoopVariableParticipatesInSharedNamespace() { + IntermediateModel model = ClientTestModels.stringArrayServiceModels(); + JmesPathTypedGetterGenerator generator = new JmesPathTypedGetterGenerator(model); + ShapeModel input = model.getOperation("MapKeysOperation").getInputShape(); + NameAllocator names = generator.newNameAllocator(); + // Stand in for an earlier binding in the same method that already took "key". + names.newName("key"); + + CodeBlock generated = generator.lower(input, "keys(RequestItems)", "stringarray", "stringArrayParam", names); + + assertFalse(generated.toString().contains("for (java.lang.String key :")); + } + + /** + * {@code keys()} behind a nullable struct must seed the immutable empty list that the reflective runtime returns + * for a null prefix. + */ + @Test + void keysWithNullablePrefixSeedsAnImmutableEmptyList() { + IntermediateModel model = ClientTestModels.stringArrayServiceModels(); + JmesPathTypedGetterGenerator generator = new JmesPathTypedGetterGenerator(model); + ShapeModel input = model.getOperation("ListOfObjectsOperation").getInputShape(); + + CodeBlock generated = generator.lower(input, "keys(Request.ItemMap)", "stringarray", "stringArrayParam"); + + String code = generated.toString(); + assertTrue(code.contains("java.util.Collections.emptyList()"), + "expected the result to be seeded with an immutable empty list, but got: " + code); + assertTrue(code.contains("if (request_ != null)"), + "expected the struct prefix to be null-guarded, but got: " + code); + assertTrue(code.contains("stringArrayParam = new java.util.ArrayList<>()"), + "expected the guarded branch to switch to a mutable list, but got: " + code); + assertTrue(code.contains("new java.util.HashMap<>(request_.itemMap()).keySet()"), + "expected the key loop to copy the map through HashMap, but got: " + code); + } + + /** + * {@code stringValues()} returns an immutable list when the projected prefix is null, so a projection behind a + * nullable prefix must not hand the endpoint params an always-mutable list. + */ + @Test + void nullableProjectionPrefixSeedsAnImmutableEmptyList() { + IntermediateModel model = ClientTestModels.stringArrayServiceModels(); + JmesPathTypedGetterGenerator generator = new JmesPathTypedGetterGenerator(model); + ShapeModel input = model.getOperation("ListOfObjectsOperation").getInputShape(); + + CodeBlock generated = generator.lower(input, "nested.listOfObjects[*].key", "stringarray", "stringArrayParam"); + + assertTrue(generated.toString().contains("java.util.Collections.emptyList()"), + "expected the result to be seeded with an immutable empty list, but got: " + generated); + } + + /** + * A projection with no nullable prefix always reaches its loop, so the result list is allocated directly. + */ + @Test + void projectionWithoutNullablePrefixAllocatesDirectly() { + IntermediateModel model = ClientTestModels.stringArrayServiceModels(); + JmesPathTypedGetterGenerator generator = new JmesPathTypedGetterGenerator(model); + ShapeModel input = model.getOperation("TransactionOperation").getInputShape(); + + CodeBlock generated = generator.lower(input, "TransactItems[*].Put.TableName", "stringarray", + "stringArrayParam"); + + assertFalse(generated.toString().contains("java.util.Collections.emptyList()"), + "unguarded projection should allocate directly, but got: " + generated); + } +} diff --git a/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/c2j/stringarray/service-2.json b/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/c2j/stringarray/service-2.json index 771df7b3b933..217c98d7bb47 100644 --- a/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/c2j/stringarray/service-2.json +++ b/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/c2j/stringarray/service-2.json @@ -50,6 +50,30 @@ "method": "POST", "requestUri": "/" } + }, + "MapKeysOperation": { + "input": { "shape": "MapKeysOperationRequest" }, + "operationContextParams": { + "stringArrayParam": { + "path": "keys(RequestItems)" + } + }, + "http": { + "method": "POST", + "requestUri": "/" + } + }, + "TransactionOperation": { + "input": { "shape": "TransactionOperationRequest" }, + "operationContextParams": { + "stringArrayParam": { + "path": "TransactItems[*].[Put.TableName, Delete.TableName][]" + } + }, + "http": { + "method": "POST", + "requestUri": "/" + } } }, "shapes": { @@ -60,15 +84,29 @@ "ListOfObjectsOperationRequest": { "type": "structure", "members": { - "nested":{"shape":"Nested"} + "nested":{"shape":"Nested"}, + "Request":{"shape":"Nested"} } }, "Nested": { "type": "structure", "members": { - "listOfObjects":{"shape":"ListOfObjects"} + "listOfObjects":{"shape":"ListOfObjects"}, + "Value":{"shape":"String"}, + "Token":{"shape":"String", "idempotencyToken":true}, + "Inner":{"shape":"Inner"}, + "ItemMap":{"shape":"StringMap"} } }, + "Inner": { + "type": "structure", + "members": { + "Flag":{"shape":"Boolean"} + } + }, + "Boolean": { + "type": "boolean" + }, "ListOfObjects": { "type": "list", "member":{"shape":"ObjectMember"} @@ -76,7 +114,52 @@ "ObjectMember": { "type": "structure", "members": { - "key":{"shape":"String"} + "key":{"shape":"String"}, + "aliases":{"shape":"StringList"} + } + }, + "StringList": { + "type": "list", + "member":{"shape":"String"} + }, + "MapKeysOperationRequest": { + "type": "structure", + "members": { + "RequestItems":{"shape":"StringMap"} + } + }, + "StringMap": { + "type": "map", + "key":{"shape":"String"}, + "value":{"shape":"String"} + }, + "TransactionOperationRequest": { + "type": "structure", + "members": { + "TransactItems":{"shape":"TransactItemList"} + } + }, + "TransactItemList": { + "type": "list", + "member":{"shape":"TransactItem"} + }, + "TransactItem": { + "type": "structure", + "members": { + "Put":{"shape":"Put"}, + "Delete":{"shape":"Delete"} + } + }, + "Put": { + "type": "structure", + "members": { + "TableName":{"shape":"String"} + } + }, + "Delete": { + "type": "structure", + "members": { + "TableName":{"shape":"String"} } }, "String":{ 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 a3ff124832a3..3d2b0f929eb8 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,7 +1,9 @@ package software.amazon.awssdk.services.samplesvc.endpoints.internal; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.Optional; import software.amazon.awssdk.annotations.Generated; @@ -22,8 +24,14 @@ import software.amazon.awssdk.http.auth.spi.scheme.AuthSchemeOption; import software.amazon.awssdk.identity.spi.Identity; import software.amazon.awssdk.services.samplesvc.endpoints.SampleSvcEndpointParams; -import software.amazon.awssdk.services.samplesvc.jmespath.internal.JmesPathRuntime; +import software.amazon.awssdk.services.samplesvc.model.Delete; import software.amazon.awssdk.services.samplesvc.model.ListOfObjectsOperationRequest; +import software.amazon.awssdk.services.samplesvc.model.MapKeysOperationRequest; +import software.amazon.awssdk.services.samplesvc.model.Nested; +import software.amazon.awssdk.services.samplesvc.model.ObjectMember; +import software.amazon.awssdk.services.samplesvc.model.Put; +import software.amazon.awssdk.services.samplesvc.model.TransactItem; +import software.amazon.awssdk.services.samplesvc.model.TransactionOperationRequest; import software.amazon.awssdk.utils.CollectionUtils; @Generated("software.amazon.awssdk:codegen") @@ -114,14 +122,65 @@ private static void setOperationContextParams(SampleSvcEndpointParams.Builder pa switch (operationName) { case "ListOfObjectsOperation":setOperationContextParams(params, (ListOfObjectsOperationRequest) request); break; + case "MapKeysOperation":setOperationContextParams(params, (MapKeysOperationRequest) request); + break; + case "TransactionOperation":setOperationContextParams(params, (TransactionOperationRequest) request); + break; default:break; } } private static void setOperationContextParams(SampleSvcEndpointParams.Builder params, ListOfObjectsOperationRequest request) { - JmesPathRuntime.Value input = new JmesPathRuntime.Value(request); - params.stringArrayParam(input.field("nested").field("listOfObjects").wildcard().field("key").stringValues()); + List stringArrayParam = Collections.emptyList(); + Nested nested = request.nested(); + if (nested != null) { + stringArrayParam = new ArrayList<>(); + for (ObjectMember objectMember : nested.listOfObjects()) { + if (objectMember != null) { + String key = objectMember.key(); + if (key != null) { + stringArrayParam.add(key); + } + } + } + } + params.stringArrayParam(stringArrayParam); + } + + private static void setOperationContextParams(SampleSvcEndpointParams.Builder params, + MapKeysOperationRequest request) { + List stringArrayParam = new ArrayList<>(); + for (String key : new HashMap<>(request.requestItems()).keySet()) { + if (key != null) { + stringArrayParam.add(key); + } + } + params.stringArrayParam(stringArrayParam); + } + + private static void setOperationContextParams(SampleSvcEndpointParams.Builder params, + TransactionOperationRequest request) { + List stringArrayParam = new ArrayList<>(); + for (TransactItem transactItem : request.transactItems()) { + if (transactItem != null) { + Put put = transactItem.put(); + if (put != null) { + String tableName = put.tableName(); + if (tableName != null) { + stringArrayParam.add(tableName); + } + } + Delete delete = transactItem.delete(); + if (delete != null) { + String tableName_ = delete.tableName(); + if (tableName_ != null) { + stringArrayParam.add(tableName_); + } + } + } + } + params.stringArrayParam(stringArrayParam); } public static Optional hostPrefix(String operationName, SdkRequest request) { diff --git a/services/dynamodb/src/test/java/software/amazon/awssdk/services/dynamodb/endpoints/internal/OperationContextParamsBindingEquivalenceTest.java b/services/dynamodb/src/test/java/software/amazon/awssdk/services/dynamodb/endpoints/internal/OperationContextParamsBindingEquivalenceTest.java new file mode 100644 index 000000000000..621f3ecd568e --- /dev/null +++ b/services/dynamodb/src/test/java/software/amazon/awssdk/services/dynamodb/endpoints/internal/OperationContextParamsBindingEquivalenceTest.java @@ -0,0 +1,248 @@ +/* + * 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.dynamodb.endpoints.internal; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.lang.reflect.Method; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.function.Function; +import org.junit.jupiter.api.Test; +import software.amazon.awssdk.services.dynamodb.endpoints.DynamoDbEndpointParams; +import software.amazon.awssdk.services.dynamodb.jmespath.internal.JmesPathRuntime.Value; +import software.amazon.awssdk.services.dynamodb.model.BatchGetItemRequest; +import software.amazon.awssdk.services.dynamodb.model.BatchWriteItemRequest; +import software.amazon.awssdk.services.dynamodb.model.ConditionCheck; +import software.amazon.awssdk.services.dynamodb.model.Delete; +import software.amazon.awssdk.services.dynamodb.model.Get; +import software.amazon.awssdk.services.dynamodb.model.ImportTableRequest; +import software.amazon.awssdk.services.dynamodb.model.KeysAndAttributes; +import software.amazon.awssdk.services.dynamodb.model.Put; +import software.amazon.awssdk.services.dynamodb.model.TableCreationParameters; +import software.amazon.awssdk.services.dynamodb.model.TransactGetItem; +import software.amazon.awssdk.services.dynamodb.model.TransactGetItemsRequest; +import software.amazon.awssdk.services.dynamodb.model.TransactWriteItem; +import software.amazon.awssdk.services.dynamodb.model.TransactWriteItemsRequest; +import software.amazon.awssdk.services.dynamodb.model.Update; +import software.amazon.awssdk.services.dynamodb.model.WriteRequest; + +/** + * Verifies that the codegen-lowered {@code operationContextParams} bindings for DynamoDB produce the same + * values as the reflective {@code JmesPathRuntime} evaluation, which remains the fallback for unsupported + * expressions and is used here as the equivalence oracle. Covers {@code keys()}, a scalar field chain, a + * projection, and a multiselect-list + flatten (TransactWriteItems). + * + *

Each lowered binding is invoked directly via its generated, private + * {@code setOperationContextParams(builder, request)} overload, so the check is isolated to the binding itself. + */ +public class OperationContextParamsBindingEquivalenceTest { + + private static List loweredList(Object request) { + return invokeBinding(request).resourceArnList(); + } + + private static String loweredScalar(Object request) { + return invokeBinding(request).resourceArn(); + } + + private static DynamoDbEndpointParams invokeBinding(Object request) { + try { + Method binding = DynamoDbEndpointResolverUtils.class.getDeclaredMethod( + "setOperationContextParams", DynamoDbEndpointParams.Builder.class, request.getClass()); + binding.setAccessible(true); + DynamoDbEndpointParams.Builder builder = DynamoDbEndpointParams.builder(); + binding.invoke(null, builder, request); + return builder.build(); + } catch (ReflectiveOperationException e) { + throw new RuntimeException(e); + } + } + + private static void assertKeysEquivalent(List reflectiveResult, List loweredResult, + List expectedContent) { + assertEquals(reflectiveResult, loweredResult, "lowered binding must equal reflective evaluation"); + // Key order is asserted positionally against the reflective oracle above; the expected content is + // written in insertion order, so compare it as a set. + assertEquals(new HashSet<>(expectedContent), new HashSet<>(loweredResult), "content must match"); + } + + @Test + public void batchGetItemKeys() { + BatchGetItemRequest empty = BatchGetItemRequest.builder().build(); + assertKeysEquivalent(new Value(empty).field("RequestItems").keys().stringValues(), + loweredList(empty), Collections.emptyList()); + + // Keys whose insertion order differs from their HashMap iteration order: the endpoint ruleset reads + // ResourceArnList positionally (getAttr(..., "[0]")) and the reflective runtime wraps maps as + // new HashMap<>(map), so the lowered binding must reproduce that hash ordering. + String[] keys = {"zebra", "mango", "apple", "delta", "foxtrot", "bravo", "yankee", "tango", "kilo", "echo"}; + Map items = new LinkedHashMap<>(); + for (String key : keys) { + items.put(key, KeysAndAttributes.builder().build()); + } + BatchGetItemRequest req = BatchGetItemRequest.builder().requestItems(items).build(); + assertKeysEquivalent(new Value(req).field("RequestItems").keys().stringValues(), + loweredList(req), Arrays.asList(keys)); + } + + @Test + public void batchGetItemNullKeyDropped() { + Map items = new LinkedHashMap<>(); + items.put(null, KeysAndAttributes.builder().build()); + items.put("table", KeysAndAttributes.builder().build()); + BatchGetItemRequest req = BatchGetItemRequest.builder().requestItems(items).build(); + assertKeysEquivalent(new Value(req).field("RequestItems").keys().stringValues(), + loweredList(req), Collections.singletonList("table")); + } + + @Test + public void batchWriteItemKeys() { + Map> items = new LinkedHashMap<>(); + items.put("t1", Collections.emptyList()); + items.put("t2", Collections.singletonList(WriteRequest.builder().build())); + BatchWriteItemRequest req = BatchWriteItemRequest.builder().requestItems(items).build(); + assertKeysEquivalent(new Value(req).field("RequestItems").keys().stringValues(), + loweredList(req), Arrays.asList("t1", "t2")); + } + + private static void assertImportTableEquivalent(ImportTableRequest req, String expected) { + String ref = new Value(req).field("TableCreationParameters").field("TableName").stringValue(); + String low = loweredScalar(req); + assertEquals(ref, low, "lowered binding must equal reflective evaluation"); + assertEquals(expected, low, "lowered binding must equal the hand-computed expectation"); + } + + @Test + public void importTableContainerNull() { + assertImportTableEquivalent(ImportTableRequest.builder().build(), null); + } + + @Test + public void importTableScalarPresent() { + assertImportTableEquivalent( + ImportTableRequest.builder() + .tableCreationParameters(TableCreationParameters.builder().tableName("my-table").build()) + .build(), + "my-table"); + } + + @Test + public void importTableScalarLeafNull() { + assertImportTableEquivalent( + ImportTableRequest.builder() + .tableCreationParameters(TableCreationParameters.builder().build()) + .build(), + null); + } + + private static void assertGetEquivalent(TransactGetItemsRequest req, List expected) { + List ref = new Value(req).field("TransactItems").wildcard().field("Get").field("TableName").stringValues(); + List low = loweredList(req); + assertEquals(ref, low, "lowered binding must equal reflective evaluation"); + assertEquals(expected, low, "lowered binding must equal the hand-computed expectation"); + } + + @Test + public void transactGetEmpty() { + assertGetEquivalent(TransactGetItemsRequest.builder().build(), Collections.emptyList()); + } + + @Test + public void transactGetMultiple() { + TransactGetItem g1 = TransactGetItem.builder().get(Get.builder().tableName("ga").build()).build(); + TransactGetItem g2 = TransactGetItem.builder().get(Get.builder().tableName("gb").build()).build(); + assertGetEquivalent(TransactGetItemsRequest.builder().transactItems(g1, g2).build(), + Arrays.asList("ga", "gb")); + } + + @Test + public void transactGetNullInnerAndLeaf() { + TransactGetItem hasName = TransactGetItem.builder().get(Get.builder().tableName("ga").build()).build(); + TransactGetItem getNull = TransactGetItem.builder().build(); + TransactGetItem leafNull = TransactGetItem.builder().get(Get.builder().build()).build(); + assertGetEquivalent(TransactGetItemsRequest.builder().transactItems(hasName, getNull, leafNull).build(), + Collections.singletonList("ga")); + } + + private static List reflectiveWrite(TransactWriteItemsRequest req) { + Function cc = v -> v.field("ConditionCheck").field("TableName"); + Function put = v -> v.field("Put").field("TableName"); + Function del = v -> v.field("Delete").field("TableName"); + Function upd = v -> v.field("Update").field("TableName"); + return new Value(req).field("TransactItems").wildcard() + .multiSelectList(cc, put, del, upd).flatten().stringValues(); + } + + private static void assertWriteEquivalent(TransactWriteItemsRequest req, List expected) { + List low = loweredList(req); + assertEquals(reflectiveWrite(req), low, "lowered binding must equal reflective evaluation"); + assertEquals(expected, low, "lowered binding must equal the hand-computed expectation"); + } + + @Test + public void transactWriteEmpty() { + assertWriteEquivalent(TransactWriteItemsRequest.builder().build(), Collections.emptyList()); + } + + @Test + public void transactWriteSingleBranch() { + TransactWriteItem putOnly = TransactWriteItem.builder().put(Put.builder().tableName("p1").build()).build(); + assertWriteEquivalent(TransactWriteItemsRequest.builder().transactItems(putOnly).build(), + Collections.singletonList("p1")); + } + + @Test + public void transactWriteAllBranchesOrdered() { + // Within one item the multiselect order is [ConditionCheck, Put, Delete, Update]. + TransactWriteItem all = TransactWriteItem.builder() + .conditionCheck(ConditionCheck.builder().tableName("cc").build()) + .put(Put.builder().tableName("p").build()) + .delete(Delete.builder().tableName("d").build()) + .update(Update.builder().tableName("u").build()) + .build(); + assertWriteEquivalent(TransactWriteItemsRequest.builder().transactItems(all).build(), + Arrays.asList("cc", "p", "d", "u")); + } + + @Test + public void transactWriteMixedItemsAndNullLeaf() { + TransactWriteItem item1 = TransactWriteItem.builder().put(Put.builder().tableName("p1").build()).build(); + TransactWriteItem item2 = TransactWriteItem.builder() + .conditionCheck(ConditionCheck.builder().tableName("cc2").build()) + .delete(Delete.builder().tableName("d2").build()) + .build(); + // Put present but its TableName null -> dropped. + TransactWriteItem item3 = TransactWriteItem.builder().put(Put.builder().build()).build(); + assertWriteEquivalent( + TransactWriteItemsRequest.builder().transactItems(item1, item2, item3).build(), + Arrays.asList("p1", "cc2", "d2")); + } + + @Test + public void transactWriteNullItemGuarded() { + List items = new ArrayList<>(); + items.add(TransactWriteItem.builder().put(Put.builder().tableName("p1").build()).build()); + items.add(null); + assertWriteEquivalent(TransactWriteItemsRequest.builder().transactItems(items).build(), + Collections.singletonList("p1")); + } +} diff --git a/services/s3/src/test/java/software/amazon/awssdk/services/s3/endpoints/internal/OperationContextParamsBindingEquivalenceTest.java b/services/s3/src/test/java/software/amazon/awssdk/services/s3/endpoints/internal/OperationContextParamsBindingEquivalenceTest.java new file mode 100644 index 000000000000..0a8af5a2cbae --- /dev/null +++ b/services/s3/src/test/java/software/amazon/awssdk/services/s3/endpoints/internal/OperationContextParamsBindingEquivalenceTest.java @@ -0,0 +1,133 @@ +/* + * 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.s3.endpoints.internal; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.lang.reflect.Method; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import org.junit.jupiter.api.Test; +import software.amazon.awssdk.services.s3.endpoints.S3EndpointParams; +import software.amazon.awssdk.services.s3.jmespath.internal.JmesPathRuntime; +import software.amazon.awssdk.services.s3.model.Delete; +import software.amazon.awssdk.services.s3.model.DeleteObjectsRequest; +import software.amazon.awssdk.services.s3.model.ObjectIdentifier; + +/** + * Verifies that the codegen-lowered {@code operationContextParams} binding for S3 DeleteObjects + * ("Delete.Objects[*].Key") produces the same value as the reflective {@link JmesPathRuntime} evaluation, + * which remains the fallback for unsupported expressions and is used here as the equivalence oracle. + * + *

The lowered binding is invoked directly via its generated, private + * {@code setOperationContextParams(builder, request)} overload, so the check is isolated to the binding + * itself, independent of the rest of endpoint parameter resolution. + */ +public class OperationContextParamsBindingEquivalenceTest { + + private static List reflective(DeleteObjectsRequest request) { + JmesPathRuntime.Value input = new JmesPathRuntime.Value(request); + return input.field("Delete").field("Objects").wildcard().field("Key").stringValues(); + } + + private static List lowered(DeleteObjectsRequest request) { + try { + Method binding = S3EndpointResolverUtils.class.getDeclaredMethod( + "setOperationContextParams", S3EndpointParams.Builder.class, DeleteObjectsRequest.class); + binding.setAccessible(true); + S3EndpointParams.Builder builder = S3EndpointParams.builder(); + binding.invoke(null, builder, request); + return builder.build().deleteObjectKeys(); + } catch (ReflectiveOperationException e) { + throw new RuntimeException(e); + } + } + + private static void assertEquivalent(DeleteObjectsRequest request, List expected) { + List low = lowered(request); + assertEquals(reflective(request), low, "lowered binding must equal reflective evaluation"); + assertEquals(expected, low, "lowered binding must equal the hand-computed expectation"); + } + + private static ObjectIdentifier oid(String key) { + return ObjectIdentifier.builder().key(key).build(); + } + + private static DeleteObjectsRequest req(ObjectIdentifier... objects) { + return DeleteObjectsRequest.builder() + .bucket("bucket") + .delete(Delete.builder().objects(Arrays.asList(objects)).build()) + .build(); + } + + @Test + public void deleteContainerNull() { + assertEquivalent(DeleteObjectsRequest.builder().bucket("bucket").build(), Collections.emptyList()); + } + + /** + * A null projection prefix yields {@code Collections.emptyList()} from {@code stringValues()}, so the lowered + * binding must not substitute a mutable list: the endpoint params class stores and returns the reference as-is. + */ + @Test + public void deleteContainerNullYieldsSameMutabilityAsReflective() { + DeleteObjectsRequest request = DeleteObjectsRequest.builder().bucket("bucket").build(); + assertThrows(UnsupportedOperationException.class, () -> reflective(request).add("x"), + "oracle assumption: reflective evaluation returns an immutable list for a null prefix"); + assertThrows(UnsupportedOperationException.class, () -> lowered(request).add("x"), + "lowered binding must match the reflective list's mutability for a null prefix"); + } + + @Test + public void objectsEmpty() { + assertEquivalent(DeleteObjectsRequest.builder() + .bucket("bucket") + .delete(Delete.builder().objects(Collections.emptyList()).build()) + .build(), + Collections.emptyList()); + } + + @Test + public void singleKey() { + assertEquivalent(req(oid("k1")), Collections.singletonList("k1")); + } + + @Test + public void multipleKeys() { + assertEquivalent(req(oid("k1"), oid("k2"), oid("k3")), Arrays.asList("k1", "k2", "k3")); + } + + @Test + public void nullLeafKeyDropped() { + assertEquivalent(req(oid("k1"), oid(null), oid("k3")), Arrays.asList("k1", "k3")); + } + + @Test + public void nullListElementDropped() { + List objects = new ArrayList<>(); + objects.add(oid("k1")); + objects.add(null); + objects.add(oid("k2")); + DeleteObjectsRequest request = DeleteObjectsRequest.builder() + .bucket("bucket") + .delete(Delete.builder().objects(objects).build()) + .build(); + assertEquivalent(request, Arrays.asList("k1", "k2")); + } +} From b8e98948afb4fca40f8305a1f73e10295bfe9d49 Mon Sep 17 00:00:00 2001 From: David Ho Date: Thu, 17 Sep 2026 18:10:11 -0700 Subject: [PATCH 2/5] address comments --- .../poet/rules/EndpointResolverUtilsSpec.java | 2 +- .../rules/JmesPathTypedGetterGenerator.java | 26 ++- .../rules/EndpointResolverUtilsSpecTest.java | 5 +- .../JmesPathTypedGetterGeneratorTest.java | 6 +- ...point-resolver-utils-with-stringarray.java | 16 +- ...onContextParamsBindingEquivalenceTest.java | 185 ++++++++++++++++++ 6 files changed, 218 insertions(+), 22 deletions(-) create mode 100644 test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/stringarray/endpoints/internal/OperationContextParamsBindingEquivalenceTest.java 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 3677352fd608..4649419ea56c 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 @@ -522,7 +522,7 @@ private MethodSpec setOperationContextParamsMethod(OperationModel opModel) { } private String operationContextParamType(String paramName) { - Map parameters = model.getEndpointRuleSetModel().getParameters(); + Map parameters = model.getEndpointParameters(); return parameters.entrySet().stream() .filter(e -> e.getKey().toLowerCase(Locale.US).equals(paramName.toLowerCase(Locale.US))) .map(e -> e.getValue().getType()) diff --git a/codegen/src/main/java/software/amazon/awssdk/codegen/poet/rules/JmesPathTypedGetterGenerator.java b/codegen/src/main/java/software/amazon/awssdk/codegen/poet/rules/JmesPathTypedGetterGenerator.java index 0d2d140208c9..e4c00fa1c49e 100644 --- a/codegen/src/main/java/software/amazon/awssdk/codegen/poet/rules/JmesPathTypedGetterGenerator.java +++ b/codegen/src/main/java/software/amazon/awssdk/codegen/poet/rules/JmesPathTypedGetterGenerator.java @@ -156,17 +156,19 @@ private CodeBlock emitKeys(ShapeModel inputShape, List argSteps, String se boolean prefixGuarded = fields.size() > 1; if (prefixGuarded) { b.addStatement("$T $N = $T.emptyList()", resultType, resultVar, ClassName.get(Collections.class)); - } else { - b.addStatement("$T $N = new $T<>()", resultType, resultVar, ClassName.get(ArrayList.class)); } Walk walk = walkToLast(b, names, inputShape, fields); + String mapVar = names.newName(baseName(walk.lastMember)); + b.addStatement("$T $N = $L", typeProvider.returnType(walk.lastMember), mapVar, + accessLast(walk, walk.lastMember)); if (prefixGuarded) { - b.addStatement("$N = new $T<>()", resultVar, ClassName.get(ArrayList.class)); + b.addStatement("$N = new $T<>($N.size())", resultVar, ClassName.get(ArrayList.class), mapVar); + } else { + b.addStatement("$T $N = new $T<>($N.size())", resultType, resultVar, ClassName.get(ArrayList.class), mapVar); } String keyVar = names.newName("key"); - b.beginControlFlow("for ($T $N : new $T<>($L).keySet())", String.class, keyVar, HashMap.class, - accessLast(walk, walk.lastMember)); + b.beginControlFlow("for ($T $N : new $T<>($N).keySet())", String.class, keyVar, HashMap.class, mapVar); b.beginControlFlow("if ($N != null)", keyVar); b.addStatement("$N.add($N)", resultVar, keyVar); b.endControlFlow(); @@ -197,21 +199,25 @@ private CodeBlock emitProjection(ShapeModel inputShape, ProjectionParts parts, boolean prefixGuarded = parts.prefix.size() > 1; if (prefixGuarded) { b.addStatement("$T $N = $T.emptyList()", listElementType, resultVar, ClassName.get(Collections.class)); - } else { - b.addStatement("$T $N = new $T<>()", listElementType, resultVar, ClassName.get(ArrayList.class)); } Walk walk = walkToLast(b, names, inputShape, parts.prefix); + MemberModel listMember = walk.lastMember; + String listVar = names.newName(baseName(listMember)); + b.addStatement("$T $N = $L", typeProvider.returnType(listMember), listVar, accessLast(walk, listMember)); + // The source size is exact for a single-leaf projection and a lower bound for a multiselect. if (prefixGuarded) { - b.addStatement("$N = new $T<>()", resultVar, ClassName.get(ArrayList.class)); + b.addStatement("$N = new $T<>($N.size())", resultVar, ClassName.get(ArrayList.class), listVar); + } else { + b.addStatement("$T $N = new $T<>($N.size())", listElementType, resultVar, ClassName.get(ArrayList.class), + listVar); } - MemberModel listMember = walk.lastMember; MemberModel elementMember = listMember.getListModel().getListMemberModel(); ShapeModel elementShape = targetShape(elementMember); TypeName elementType = typeProvider.returnType(elementMember); String elementVar = names.newName(elementBaseName(elementMember)); - b.beginControlFlow("for ($T $N : $L)", elementType, elementVar, accessLast(walk, listMember)); + b.beginControlFlow("for ($T $N : $N)", elementType, elementVar, listVar); b.beginControlFlow("if ($N != null)", elementVar); if (parts.isMultiSelect()) { for (List branch : parts.multiSelect().branches) { diff --git a/codegen/src/test/java/software/amazon/awssdk/codegen/poet/rules/EndpointResolverUtilsSpecTest.java b/codegen/src/test/java/software/amazon/awssdk/codegen/poet/rules/EndpointResolverUtilsSpecTest.java index b1e0ba624253..5781dd8e3247 100644 --- a/codegen/src/test/java/software/amazon/awssdk/codegen/poet/rules/EndpointResolverUtilsSpecTest.java +++ b/codegen/src/test/java/software/amazon/awssdk/codegen/poet/rules/EndpointResolverUtilsSpecTest.java @@ -110,8 +110,9 @@ private static void addStringArrayBinding(IntermediateModel model, String operat private static void addBinding(IntermediateModel model, String operationName, String parameterName, TreeNode path) { - ParameterModel parameter = model.getEndpointRuleSetModel().getParameters().get("stringArrayParam"); - model.getEndpointRuleSetModel().getParameters().put(parameterName, parameter); + Map parameters = new LinkedHashMap<>(model.getEndpointParameters()); + parameters.put(parameterName, parameters.get("stringArrayParam")); + model.setEndpointParameters(parameters); OperationContextParam operationContextParam = new OperationContextParam(); operationContextParam.setPath(path); diff --git a/codegen/src/test/java/software/amazon/awssdk/codegen/poet/rules/JmesPathTypedGetterGeneratorTest.java b/codegen/src/test/java/software/amazon/awssdk/codegen/poet/rules/JmesPathTypedGetterGeneratorTest.java index 9edbde48def0..6ce397d46b6c 100644 --- a/codegen/src/test/java/software/amazon/awssdk/codegen/poet/rules/JmesPathTypedGetterGeneratorTest.java +++ b/codegen/src/test/java/software/amazon/awssdk/codegen/poet/rules/JmesPathTypedGetterGeneratorTest.java @@ -169,9 +169,9 @@ void keysWithNullablePrefixSeedsAnImmutableEmptyList() { "expected the result to be seeded with an immutable empty list, but got: " + code); assertTrue(code.contains("if (request_ != null)"), "expected the struct prefix to be null-guarded, but got: " + code); - assertTrue(code.contains("stringArrayParam = new java.util.ArrayList<>()"), - "expected the guarded branch to switch to a mutable list, but got: " + code); - assertTrue(code.contains("new java.util.HashMap<>(request_.itemMap()).keySet()"), + assertTrue(code.contains("stringArrayParam = new java.util.ArrayList<>(itemMap.size())"), + "expected the guarded branch to switch to a presized mutable list, but got: " + code); + assertTrue(code.contains("new java.util.HashMap<>(itemMap).keySet()"), "expected the key loop to copy the map through HashMap, but got: " + code); } 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 3d2b0f929eb8..091a05ed4704 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 @@ -5,6 +5,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Optional; import software.amazon.awssdk.annotations.Generated; import software.amazon.awssdk.annotations.SdkInternalApi; @@ -135,8 +136,9 @@ private static void setOperationContextParams(SampleSvcEndpointParams.Builder pa List stringArrayParam = Collections.emptyList(); Nested nested = request.nested(); if (nested != null) { - stringArrayParam = new ArrayList<>(); - for (ObjectMember objectMember : nested.listOfObjects()) { + List listOfObjects = nested.listOfObjects(); + stringArrayParam = new ArrayList<>(listOfObjects.size()); + for (ObjectMember objectMember : listOfObjects) { if (objectMember != null) { String key = objectMember.key(); if (key != null) { @@ -150,8 +152,9 @@ private static void setOperationContextParams(SampleSvcEndpointParams.Builder pa private static void setOperationContextParams(SampleSvcEndpointParams.Builder params, MapKeysOperationRequest request) { - List stringArrayParam = new ArrayList<>(); - for (String key : new HashMap<>(request.requestItems()).keySet()) { + Map requestItems = request.requestItems(); + List stringArrayParam = new ArrayList<>(requestItems.size()); + for (String key : new HashMap<>(requestItems).keySet()) { if (key != null) { stringArrayParam.add(key); } @@ -161,8 +164,9 @@ private static void setOperationContextParams(SampleSvcEndpointParams.Builder pa private static void setOperationContextParams(SampleSvcEndpointParams.Builder params, TransactionOperationRequest request) { - List stringArrayParam = new ArrayList<>(); - for (TransactItem transactItem : request.transactItems()) { + List transactItems = request.transactItems(); + List stringArrayParam = new ArrayList<>(transactItems.size()); + for (TransactItem transactItem : transactItems) { if (transactItem != null) { Put put = transactItem.put(); if (put != null) { diff --git a/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/stringarray/endpoints/internal/OperationContextParamsBindingEquivalenceTest.java b/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/stringarray/endpoints/internal/OperationContextParamsBindingEquivalenceTest.java new file mode 100644 index 000000000000..51458dc26ed8 --- /dev/null +++ b/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/stringarray/endpoints/internal/OperationContextParamsBindingEquivalenceTest.java @@ -0,0 +1,185 @@ +/* + * 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.stringarray.endpoints.internal; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.lang.reflect.Method; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.function.Function; +import org.junit.jupiter.api.Test; +import software.amazon.awssdk.services.stringarray.endpoints.StringArrayEndpointParams; +import software.amazon.awssdk.services.stringarray.jmespath.internal.JmesPathRuntime.Value; +import software.amazon.awssdk.services.stringarray.model.ListOfObjectsOperationRequest; +import software.amazon.awssdk.services.stringarray.model.ListOfUnionsOperationRequest; +import software.amazon.awssdk.services.stringarray.model.MapOperationRequest; +import software.amazon.awssdk.services.stringarray.model.ObjectMember; +import software.amazon.awssdk.services.stringarray.model.UnionMember; + +/** + * Verifies that the codegen-lowered {@code operationContextParams} bindings for the synthetic string array service + * produce the same values as the reflective {@code JmesPathRuntime} evaluation, which remains the fallback for + * unsupported expressions and is used here as the equivalence oracle. Covers a projection behind a nullable struct + * ("nested.listOfObjects[*].key"), a multiselect-list + flatten over union members + * ("listOfUnions[*][string, object.key][]"), and {@code keys()} over a map ("keys(map)"). + * + *

Each lowered binding is invoked directly via its generated, private + * {@code setOperationContextParams(builder, request)} overload, so the check is isolated to the binding itself. + */ +public class OperationContextParamsBindingEquivalenceTest { + + private static List lowered(Object request) { + try { + Method binding = StringArrayEndpointResolverUtils.class.getDeclaredMethod( + "setOperationContextParams", StringArrayEndpointParams.Builder.class, request.getClass()); + binding.setAccessible(true); + StringArrayEndpointParams.Builder builder = StringArrayEndpointParams.builder(); + binding.invoke(null, builder, request); + return builder.build().stringArrayParam(); + } catch (ReflectiveOperationException e) { + throw new RuntimeException(e); + } + } + + private static List reflectiveObjects(ListOfObjectsOperationRequest request) { + return new Value(request).field("nested").field("listOfObjects").wildcard().field("key").stringValues(); + } + + private static List reflectiveUnions(ListOfUnionsOperationRequest request) { + Function string = v -> v.field("string"); + Function objectKey = v -> v.field("object").field("key"); + return new Value(request).field("listOfUnions").wildcard() + .multiSelectList(string, objectKey).flatten().stringValues(); + } + + private static List reflectiveMap(MapOperationRequest request) { + return new Value(request).field("map").keys().stringValues(); + } + + private static void assertObjectsEquivalent(ListOfObjectsOperationRequest request, List expected) { + List low = lowered(request); + assertEquals(reflectiveObjects(request), low, "lowered binding must equal reflective evaluation"); + assertEquals(expected, low, "lowered binding must equal the hand-computed expectation"); + } + + private static void assertUnionsEquivalent(ListOfUnionsOperationRequest request, List expected) { + List low = lowered(request); + assertEquals(reflectiveUnions(request), low, "lowered binding must equal reflective evaluation"); + assertEquals(expected, low, "lowered binding must equal the hand-computed expectation"); + } + + private static ObjectMember object(String key) { + return ObjectMember.builder().key(key).build(); + } + + @Test + public void objectsNestedNull() { + assertObjectsEquivalent(ListOfObjectsOperationRequest.builder().build(), Collections.emptyList()); + } + + /** + * A null projection prefix yields {@code Collections.emptyList()} from {@code stringValues()}, so the lowered + * binding must not substitute a mutable list. + */ + @Test + public void objectsNestedNullYieldsSameMutabilityAsReflective() { + ListOfObjectsOperationRequest request = ListOfObjectsOperationRequest.builder().build(); + assertThrows(UnsupportedOperationException.class, () -> reflectiveObjects(request).add("x"), + "oracle assumption: reflective evaluation returns an immutable list for a null prefix"); + assertThrows(UnsupportedOperationException.class, () -> lowered(request).add("x"), + "lowered binding must match the reflective list's mutability for a null prefix"); + } + + @Test + public void objectsMultipleKeysOrdered() { + ListOfObjectsOperationRequest request = ListOfObjectsOperationRequest.builder() + .nested(n -> n.listOfObjects(object("k1"), object("k2"), object("k3"))) + .build(); + assertObjectsEquivalent(request, Arrays.asList("k1", "k2", "k3")); + } + + @Test + public void objectsNullKeyAndNullElementDropped() { + List objects = new ArrayList<>(); + objects.add(object("k1")); + objects.add(null); + objects.add(object(null)); + objects.add(object("k2")); + ListOfObjectsOperationRequest request = ListOfObjectsOperationRequest.builder() + .nested(n -> n.listOfObjects(objects)) + .build(); + assertObjectsEquivalent(request, Arrays.asList("k1", "k2")); + } + + @Test + public void unionsEmptyRequest() { + assertUnionsEquivalent(ListOfUnionsOperationRequest.builder().build(), Collections.emptyList()); + } + + @Test + public void unionsStringAndObjectBranchesOrdered() { + // Within one union the multiselect order is [string, object.key]. + ListOfUnionsOperationRequest request = ListOfUnionsOperationRequest.builder() + .listOfUnions(UnionMember.builder().string("s1").object(object("o1")).build(), + UnionMember.builder().object(object("o2")).build(), + UnionMember.builder().string("s3").build()) + .build(); + assertUnionsEquivalent(request, Arrays.asList("s1", "o1", "o2", "s3")); + } + + @Test + public void unionsEmptyMemberAndNullLeafDropped() { + List unions = new ArrayList<>(); + unions.add(UnionMember.builder().string("s1").build()); + unions.add(UnionMember.builder().build()); + unions.add(UnionMember.builder().object(object(null)).build()); + unions.add(null); + ListOfUnionsOperationRequest request = ListOfUnionsOperationRequest.builder() + .listOfUnions(unions) + .build(); + assertUnionsEquivalent(request, Collections.singletonList("s1")); + } + + @Test + public void mapEmptyRequest() { + MapOperationRequest request = MapOperationRequest.builder().build(); + List low = lowered(request); + assertEquals(reflectiveMap(request), low, "lowered binding must equal reflective evaluation"); + assertEquals(Collections.emptyList(), low); + } + + @Test + public void mapKeysMatchReflectiveHashOrdering() { + // Keys whose insertion order differs from their HashMap iteration order: the reflective runtime wraps maps + // as new HashMap<>(map), so the lowered binding must reproduce that hash ordering positionally. + String[] keys = {"zebra", "mango", "apple", "delta", "foxtrot", "bravo", "yankee", "tango", "kilo", "echo"}; + Map map = new LinkedHashMap<>(); + for (String key : keys) { + map.put(key, "value"); + } + MapOperationRequest request = MapOperationRequest.builder().map(map).build(); + List low = lowered(request); + assertEquals(reflectiveMap(request), low, "lowered binding must equal reflective evaluation"); + assertEquals(new HashSet<>(Arrays.asList(keys)), new HashSet<>(low), "content must match"); + } +} From 2a1525b606476471ba0534bccaab862ea84fe51a Mon Sep 17 00:00:00 2001 From: David Ho Date: Fri, 18 Sep 2026 09:54:32 -0700 Subject: [PATCH 3/5] Fix guarded keys() empty-list mutability, add generated-code regression assertions --- .../rules/JmesPathTypedGetterGenerator.java | 9 +-- .../JmesPathTypedGetterGeneratorTest.java | 13 ++-- ...onContextParamsBindingEquivalenceTest.java | 32 +++++++++ ...onContextParamsBindingEquivalenceTest.java | 32 +++++++++ .../stringarray/service-2.json | 24 +++++++ ...onContextParamsBindingEquivalenceTest.java | 71 ++++++++++++++++++- 6 files changed, 169 insertions(+), 12 deletions(-) diff --git a/codegen/src/main/java/software/amazon/awssdk/codegen/poet/rules/JmesPathTypedGetterGenerator.java b/codegen/src/main/java/software/amazon/awssdk/codegen/poet/rules/JmesPathTypedGetterGenerator.java index e4c00fa1c49e..7c433ab091a3 100644 --- a/codegen/src/main/java/software/amazon/awssdk/codegen/poet/rules/JmesPathTypedGetterGenerator.java +++ b/codegen/src/main/java/software/amazon/awssdk/codegen/poet/rules/JmesPathTypedGetterGenerator.java @@ -151,11 +151,11 @@ private CodeBlock emitKeys(ShapeModel inputShape, List argSteps, String se TypeName resultType = ParameterizedTypeName.get(ClassName.get(List.class), ClassName.get(String.class)); String resultVar = names.newName(setterName); - // If the prefix is guarded, the loop may never run: seed the immutable empty list that the reflective - // runtime's stringValues() returns for a null prefix. + // If the prefix is guarded, the loop may never run: seed the result the reflective runtime returns for a + // null prefix, which for keys() is a mutable empty list (a null projection prefix yields an immutable one). boolean prefixGuarded = fields.size() > 1; if (prefixGuarded) { - b.addStatement("$T $N = $T.emptyList()", resultType, resultVar, ClassName.get(Collections.class)); + b.addStatement("$T $N = new $T<>()", resultType, resultVar, ClassName.get(ArrayList.class)); } Walk walk = walkToLast(b, names, inputShape, fields); @@ -205,7 +205,8 @@ private CodeBlock emitProjection(ShapeModel inputShape, ProjectionParts parts, MemberModel listMember = walk.lastMember; String listVar = names.newName(baseName(listMember)); b.addStatement("$T $N = $L", typeProvider.returnType(listMember), listVar, accessLast(walk, listMember)); - // The source size is exact for a single-leaf projection and a lower bound for a multiselect. + // Presizing to the source list size avoids growth in the common one-result-per-element case and reduces + // growth for multiselects; null filtering can leave the result smaller. if (prefixGuarded) { b.addStatement("$N = new $T<>($N.size())", resultVar, ClassName.get(ArrayList.class), listVar); } else { diff --git a/codegen/src/test/java/software/amazon/awssdk/codegen/poet/rules/JmesPathTypedGetterGeneratorTest.java b/codegen/src/test/java/software/amazon/awssdk/codegen/poet/rules/JmesPathTypedGetterGeneratorTest.java index 6ce397d46b6c..14865197c10b 100644 --- a/codegen/src/test/java/software/amazon/awssdk/codegen/poet/rules/JmesPathTypedGetterGeneratorTest.java +++ b/codegen/src/test/java/software/amazon/awssdk/codegen/poet/rules/JmesPathTypedGetterGeneratorTest.java @@ -153,11 +153,12 @@ void keysLoopVariableParticipatesInSharedNamespace() { } /** - * {@code keys()} behind a nullable struct must seed the immutable empty list that the reflective runtime returns - * for a null prefix. + * {@code keys()} behind a nullable struct must seed the mutable empty list that the reflective runtime returns + * for a null prefix: keys() normalizes null to an empty list value whose {@code stringValues()} is mutable, + * unlike a null projection prefix. */ @Test - void keysWithNullablePrefixSeedsAnImmutableEmptyList() { + void keysWithNullablePrefixSeedsAMutableEmptyList() { IntermediateModel model = ClientTestModels.stringArrayServiceModels(); JmesPathTypedGetterGenerator generator = new JmesPathTypedGetterGenerator(model); ShapeModel input = model.getOperation("ListOfObjectsOperation").getInputShape(); @@ -165,12 +166,12 @@ void keysWithNullablePrefixSeedsAnImmutableEmptyList() { CodeBlock generated = generator.lower(input, "keys(Request.ItemMap)", "stringarray", "stringArrayParam"); String code = generated.toString(); - assertTrue(code.contains("java.util.Collections.emptyList()"), - "expected the result to be seeded with an immutable empty list, but got: " + code); + assertTrue(code.contains("stringArrayParam = new java.util.ArrayList<>();"), + "expected the result to be seeded with a mutable empty list, but got: " + code); assertTrue(code.contains("if (request_ != null)"), "expected the struct prefix to be null-guarded, but got: " + code); assertTrue(code.contains("stringArrayParam = new java.util.ArrayList<>(itemMap.size())"), - "expected the guarded branch to switch to a presized mutable list, but got: " + code); + "expected the guarded branch to switch to a presized list, but got: " + code); assertTrue(code.contains("new java.util.HashMap<>(itemMap).keySet()"), "expected the key loop to copy the map through HashMap, but got: " + code); } diff --git a/services/dynamodb/src/test/java/software/amazon/awssdk/services/dynamodb/endpoints/internal/OperationContextParamsBindingEquivalenceTest.java b/services/dynamodb/src/test/java/software/amazon/awssdk/services/dynamodb/endpoints/internal/OperationContextParamsBindingEquivalenceTest.java index 621f3ecd568e..896a507598f9 100644 --- a/services/dynamodb/src/test/java/software/amazon/awssdk/services/dynamodb/endpoints/internal/OperationContextParamsBindingEquivalenceTest.java +++ b/services/dynamodb/src/test/java/software/amazon/awssdk/services/dynamodb/endpoints/internal/OperationContextParamsBindingEquivalenceTest.java @@ -16,8 +16,13 @@ package software.amazon.awssdk.services.dynamodb.endpoints.internal; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.InputStream; import java.lang.reflect.Method; +import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -245,4 +250,31 @@ public void transactWriteNullItemGuarded() { assertWriteEquivalent(TransactWriteItemsRequest.builder().transactItems(items).build(), Collections.singletonList("p1")); } + + /** + * The equivalence assertions above are only meaningful if the generated methods are actually lowered: a codegen + * regression that sends these operations back to the reflective path would make every comparison + * oracle-against-itself. The resolver's class file must not reference the reflective runtime. + */ + @Test + public void operationContextParamBindingsDoNotUseReflectiveFallback() throws IOException { + byte[] classBytes = readClassBytes(DynamoDbEndpointResolverUtils.class); + assertFalse(new String(classBytes, StandardCharsets.ISO_8859_1).contains("JmesPathRuntime"), + "The generated resolver references JmesPathRuntime, so at least one operation's bindings use the " + + "reflective fallback. If a codegen change caused this, fix the regression. If a model change " + + "added an expression outside the lowering subset, the fallback is working as designed but is a " + + "performance regression for that operation: extend the lowerer or consciously update this test."); + } + + private static byte[] readClassBytes(Class clazz) throws IOException { + try (InputStream in = clazz.getResourceAsStream(clazz.getSimpleName() + ".class"); + ByteArrayOutputStream out = new ByteArrayOutputStream()) { + byte[] buffer = new byte[8192]; + int read; + while ((read = in.read(buffer)) > 0) { + out.write(buffer, 0, read); + } + return out.toByteArray(); + } + } } diff --git a/services/s3/src/test/java/software/amazon/awssdk/services/s3/endpoints/internal/OperationContextParamsBindingEquivalenceTest.java b/services/s3/src/test/java/software/amazon/awssdk/services/s3/endpoints/internal/OperationContextParamsBindingEquivalenceTest.java index 0a8af5a2cbae..92bb9c5fdc05 100644 --- a/services/s3/src/test/java/software/amazon/awssdk/services/s3/endpoints/internal/OperationContextParamsBindingEquivalenceTest.java +++ b/services/s3/src/test/java/software/amazon/awssdk/services/s3/endpoints/internal/OperationContextParamsBindingEquivalenceTest.java @@ -16,9 +16,14 @@ package software.amazon.awssdk.services.s3.endpoints.internal; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.InputStream; import java.lang.reflect.Method; +import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -130,4 +135,31 @@ public void nullListElementDropped() { .build(); assertEquivalent(request, Arrays.asList("k1", "k2")); } + + /** + * The equivalence assertions above are only meaningful if the generated method is actually lowered: a codegen + * regression that sends this operation back to the reflective path would make every comparison + * oracle-against-itself. The resolver's class file must not reference the reflective runtime. + */ + @Test + public void operationContextParamBindingsDoNotUseReflectiveFallback() throws IOException { + byte[] classBytes = readClassBytes(S3EndpointResolverUtils.class); + assertFalse(new String(classBytes, StandardCharsets.ISO_8859_1).contains("JmesPathRuntime"), + "The generated resolver references JmesPathRuntime, so at least one operation's bindings use the " + + "reflective fallback. If a codegen change caused this, fix the regression. If a model change " + + "added an expression outside the lowering subset, the fallback is working as designed but is a " + + "performance regression for that operation: extend the lowerer or consciously update this test."); + } + + private static byte[] readClassBytes(Class clazz) throws IOException { + try (InputStream in = clazz.getResourceAsStream(clazz.getSimpleName() + ".class"); + ByteArrayOutputStream out = new ByteArrayOutputStream()) { + byte[] buffer = new byte[8192]; + int read; + while ((read = in.read(buffer)) > 0) { + out.write(buffer, 0, read); + } + return out.toByteArray(); + } + } } diff --git a/test/codegen-generated-classes-test/src/main/resources/codegen-resources/stringarray/service-2.json b/test/codegen-generated-classes-test/src/main/resources/codegen-resources/stringarray/service-2.json index 7f630bcea8d3..9ac538399676 100644 --- a/test/codegen-generated-classes-test/src/main/resources/codegen-resources/stringarray/service-2.json +++ b/test/codegen-generated-classes-test/src/main/resources/codegen-resources/stringarray/service-2.json @@ -73,6 +73,18 @@ "method": "POST", "requestUri": "/" } + }, + "NestedMapOperation": { + "input": { "shape": "NestedMapOperationRequest" }, + "operationContextParams": { + "stringArrayParam": { + "path": "keys(nestedMap.map)" + } + }, + "http": { + "method": "POST", + "requestUri": "/" + } } }, "shapes": { @@ -125,6 +137,18 @@ "map":{"shape":"Map"} } }, + "NestedMapOperationRequest": { + "type": "structure", + "members": { + "nestedMap":{"shape":"NestedMap"} + } + }, + "NestedMap": { + "type": "structure", + "members": { + "map":{"shape":"Map"} + } + }, "Map":{ "type":"map", "key":{"shape":"String"}, diff --git a/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/stringarray/endpoints/internal/OperationContextParamsBindingEquivalenceTest.java b/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/stringarray/endpoints/internal/OperationContextParamsBindingEquivalenceTest.java index 51458dc26ed8..fe0a52d38401 100644 --- a/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/stringarray/endpoints/internal/OperationContextParamsBindingEquivalenceTest.java +++ b/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/stringarray/endpoints/internal/OperationContextParamsBindingEquivalenceTest.java @@ -15,10 +15,16 @@ package software.amazon.awssdk.services.stringarray.endpoints.internal; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.InputStream; import java.lang.reflect.Method; +import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -33,6 +39,7 @@ import software.amazon.awssdk.services.stringarray.model.ListOfObjectsOperationRequest; import software.amazon.awssdk.services.stringarray.model.ListOfUnionsOperationRequest; import software.amazon.awssdk.services.stringarray.model.MapOperationRequest; +import software.amazon.awssdk.services.stringarray.model.NestedMapOperationRequest; import software.amazon.awssdk.services.stringarray.model.ObjectMember; import software.amazon.awssdk.services.stringarray.model.UnionMember; @@ -40,8 +47,8 @@ * Verifies that the codegen-lowered {@code operationContextParams} bindings for the synthetic string array service * produce the same values as the reflective {@code JmesPathRuntime} evaluation, which remains the fallback for * unsupported expressions and is used here as the equivalence oracle. Covers a projection behind a nullable struct - * ("nested.listOfObjects[*].key"), a multiselect-list + flatten over union members - * ("listOfUnions[*][string, object.key][]"), and {@code keys()} over a map ("keys(map)"). + * ("nested.listOfObjects[*].key"), a multiselect-list + flatten ("listOfUnions[*][string, object.key][]"), + * {@code keys()} over a map ("keys(map)"), and {@code keys()} behind a nullable struct ("keys(nestedMap.map)"). * *

Each lowered binding is invoked directly via its generated, private * {@code setOperationContextParams(builder, request)} overload, so the check is isolated to the binding itself. @@ -182,4 +189,64 @@ public void mapKeysMatchReflectiveHashOrdering() { assertEquals(reflectiveMap(request), low, "lowered binding must equal reflective evaluation"); assertEquals(new HashSet<>(Arrays.asList(keys)), new HashSet<>(low), "content must match"); } + + private static List reflectiveNestedMap(NestedMapOperationRequest request) { + return new Value(request).field("nestedMap").field("map").keys().stringValues(); + } + + private static void assertNestedMapEquivalent(NestedMapOperationRequest request, List expectedContent) { + List low = lowered(request); + assertEquals(reflectiveNestedMap(request), low, "lowered binding must equal reflective evaluation"); + assertEquals(new HashSet<>(expectedContent), new HashSet<>(low), "content must match"); + } + + @Test + public void nestedMapNullPrefix() { + assertNestedMapEquivalent(NestedMapOperationRequest.builder().build(), Collections.emptyList()); + } + + /** + * {@code keys()} normalizes a null prefix to an empty list value, whose {@code stringValues()} is mutable, so the + * lowered binding must hand the endpoint params a mutable list here, unlike a null projection prefix. + */ + @Test + public void nestedMapNullPrefixYieldsSameMutabilityAsReflective() { + NestedMapOperationRequest request = NestedMapOperationRequest.builder().build(); + assertDoesNotThrow(() -> reflectiveNestedMap(request).add("x"), + "oracle assumption: reflective keys() returns a mutable list for a null prefix"); + assertDoesNotThrow(() -> lowered(request).add("x"), + "lowered binding must match the reflective list's mutability for a null prefix"); + } + + @Test + public void nestedMapKeys() { + NestedMapOperationRequest request = NestedMapOperationRequest.builder() + .nestedMap(n -> n.map(Collections.singletonMap("table", "value"))) + .build(); + assertNestedMapEquivalent(request, Collections.singletonList("table")); + } + + /** + * The equivalence assertions above are only meaningful if the generated methods are actually lowered: a codegen + * regression that sends these operations back to the reflective path would make every comparison + * oracle-against-itself. The resolver's class file must not reference the reflective runtime. + */ + @Test + public void operationContextParamBindingsDoNotUseReflectiveFallback() throws IOException { + byte[] classBytes = readClassBytes(StringArrayEndpointResolverUtils.class); + assertFalse(new String(classBytes, StandardCharsets.ISO_8859_1).contains("JmesPathRuntime"), + "generated resolver references JmesPathRuntime; bindings fell back to the reflective path"); + } + + private static byte[] readClassBytes(Class clazz) throws IOException { + try (InputStream in = clazz.getResourceAsStream(clazz.getSimpleName() + ".class"); + ByteArrayOutputStream out = new ByteArrayOutputStream()) { + byte[] buffer = new byte[8192]; + int read; + while ((read = in.read(buffer)) > 0) { + out.write(buffer, 0, read); + } + return out.toByteArray(); + } + } } From 41f3503308c0779ea0af3f7d6a4c7a623a86c327 Mon Sep 17 00:00:00 2001 From: David Ho Date: Tue, 22 Sep 2026 11:10:05 -0700 Subject: [PATCH 4/5] Remove HashMap copy from generated keys() bindings --- .../feature-AWSSDKforJavav2-0f38bd9.json | 2 +- .../poet/rules/JmesPathTypedGetterGenerator.java | 3 +-- .../rules/JmesPathTypedGetterGeneratorTest.java | 4 ++-- ...endpoint-resolver-utils-with-stringarray.java | 3 +-- ...ationContextParamsBindingEquivalenceTest.java | 10 ++++------ ...ationContextParamsBindingEquivalenceTest.java | 16 +++++++++++----- 6 files changed, 20 insertions(+), 18 deletions(-) diff --git a/.changes/next-release/feature-AWSSDKforJavav2-0f38bd9.json b/.changes/next-release/feature-AWSSDKforJavav2-0f38bd9.json index 483adfd78329..aceb05b46d6e 100644 --- a/.changes/next-release/feature-AWSSDKforJavav2-0f38bd9.json +++ b/.changes/next-release/feature-AWSSDKforJavav2-0f38bd9.json @@ -2,5 +2,5 @@ "type": "perf-improvement", "category": "AWS SDK for Java v2", "contributor": "", - "description": "Endpoint parameters derived from JMESPath expressions (operationContextParams) are now bound using typed getters generated at build time instead of a reflective runtime, reducing per-request allocation and latency during endpoint resolution for operations that use them (e.g. DynamoDB batch/transaction and Amazon S3 DeleteObjects). Behavior is unchanged." + "description": "Endpoint parameters derived from JMESPath expressions (operationContextParams) are now bound using typed getters generated at build time instead of a reflective runtime, reducing per-request allocation and latency during endpoint resolution. Generated keys() bindings now iterate request maps directly instead of copying them through HashMap. This changes the unspecified order of the derived key list, but current DynamoDB endpoint routing behavior is unchanged." } diff --git a/codegen/src/main/java/software/amazon/awssdk/codegen/poet/rules/JmesPathTypedGetterGenerator.java b/codegen/src/main/java/software/amazon/awssdk/codegen/poet/rules/JmesPathTypedGetterGenerator.java index 7c433ab091a3..6f9f0f64366b 100644 --- a/codegen/src/main/java/software/amazon/awssdk/codegen/poet/rules/JmesPathTypedGetterGenerator.java +++ b/codegen/src/main/java/software/amazon/awssdk/codegen/poet/rules/JmesPathTypedGetterGenerator.java @@ -22,7 +22,6 @@ import com.squareup.javapoet.TypeName; import java.util.ArrayList; import java.util.Collections; -import java.util.HashMap; import java.util.List; import java.util.Locale; import java.util.Map; @@ -168,7 +167,7 @@ private CodeBlock emitKeys(ShapeModel inputShape, List argSteps, String se b.addStatement("$T $N = new $T<>($N.size())", resultType, resultVar, ClassName.get(ArrayList.class), mapVar); } String keyVar = names.newName("key"); - b.beginControlFlow("for ($T $N : new $T<>($N).keySet())", String.class, keyVar, HashMap.class, mapVar); + b.beginControlFlow("for ($T $N : $N.keySet())", String.class, keyVar, mapVar); b.beginControlFlow("if ($N != null)", keyVar); b.addStatement("$N.add($N)", resultVar, keyVar); b.endControlFlow(); diff --git a/codegen/src/test/java/software/amazon/awssdk/codegen/poet/rules/JmesPathTypedGetterGeneratorTest.java b/codegen/src/test/java/software/amazon/awssdk/codegen/poet/rules/JmesPathTypedGetterGeneratorTest.java index 14865197c10b..de29cd72c9d8 100644 --- a/codegen/src/test/java/software/amazon/awssdk/codegen/poet/rules/JmesPathTypedGetterGeneratorTest.java +++ b/codegen/src/test/java/software/amazon/awssdk/codegen/poet/rules/JmesPathTypedGetterGeneratorTest.java @@ -172,8 +172,8 @@ void keysWithNullablePrefixSeedsAMutableEmptyList() { "expected the struct prefix to be null-guarded, but got: " + code); assertTrue(code.contains("stringArrayParam = new java.util.ArrayList<>(itemMap.size())"), "expected the guarded branch to switch to a presized list, but got: " + code); - assertTrue(code.contains("new java.util.HashMap<>(itemMap).keySet()"), - "expected the key loop to copy the map through HashMap, but got: " + code); + assertTrue(code.contains("for (java.lang.String key : itemMap.keySet())"), + "expected the key loop to iterate the map directly, but got: " + code); } /** 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 091a05ed4704..9ffb9eced65c 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 @@ -3,7 +3,6 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; -import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Optional; @@ -154,7 +153,7 @@ private static void setOperationContextParams(SampleSvcEndpointParams.Builder pa MapKeysOperationRequest request) { Map requestItems = request.requestItems(); List stringArrayParam = new ArrayList<>(requestItems.size()); - for (String key : new HashMap<>(requestItems).keySet()) { + for (String key : requestItems.keySet()) { if (key != null) { stringArrayParam.add(key); } diff --git a/services/dynamodb/src/test/java/software/amazon/awssdk/services/dynamodb/endpoints/internal/OperationContextParamsBindingEquivalenceTest.java b/services/dynamodb/src/test/java/software/amazon/awssdk/services/dynamodb/endpoints/internal/OperationContextParamsBindingEquivalenceTest.java index 896a507598f9..489d8309c377 100644 --- a/services/dynamodb/src/test/java/software/amazon/awssdk/services/dynamodb/endpoints/internal/OperationContextParamsBindingEquivalenceTest.java +++ b/services/dynamodb/src/test/java/software/amazon/awssdk/services/dynamodb/endpoints/internal/OperationContextParamsBindingEquivalenceTest.java @@ -84,9 +84,10 @@ private static DynamoDbEndpointParams invokeBinding(Object request) { private static void assertKeysEquivalent(List reflectiveResult, List loweredResult, List expectedContent) { - assertEquals(reflectiveResult, loweredResult, "lowered binding must equal reflective evaluation"); - // Key order is asserted positionally against the reflective oracle above; the expected content is - // written in insertion order, so compare it as a set. + assertEquals(reflectiveResult.size(), loweredResult.size(), + "lowered result must contain the same number of keys as reflective evaluation"); + assertEquals(new HashSet<>(reflectiveResult), new HashSet<>(loweredResult), + "lowered key content must equal reflective evaluation"); assertEquals(new HashSet<>(expectedContent), new HashSet<>(loweredResult), "content must match"); } @@ -96,9 +97,6 @@ public void batchGetItemKeys() { assertKeysEquivalent(new Value(empty).field("RequestItems").keys().stringValues(), loweredList(empty), Collections.emptyList()); - // Keys whose insertion order differs from their HashMap iteration order: the endpoint ruleset reads - // ResourceArnList positionally (getAttr(..., "[0]")) and the reflective runtime wraps maps as - // new HashMap<>(map), so the lowered binding must reproduce that hash ordering. String[] keys = {"zebra", "mango", "apple", "delta", "foxtrot", "bravo", "yankee", "tango", "kilo", "echo"}; Map items = new LinkedHashMap<>(); for (String key : keys) { diff --git a/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/stringarray/endpoints/internal/OperationContextParamsBindingEquivalenceTest.java b/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/stringarray/endpoints/internal/OperationContextParamsBindingEquivalenceTest.java index fe0a52d38401..39b9da93cf81 100644 --- a/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/stringarray/endpoints/internal/OperationContextParamsBindingEquivalenceTest.java +++ b/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/stringarray/endpoints/internal/OperationContextParamsBindingEquivalenceTest.java @@ -176,17 +176,19 @@ public void mapEmptyRequest() { } @Test - public void mapKeysMatchReflectiveHashOrdering() { - // Keys whose insertion order differs from their HashMap iteration order: the reflective runtime wraps maps - // as new HashMap<>(map), so the lowered binding must reproduce that hash ordering positionally. + public void mapKeysMatchReflectiveContent() { String[] keys = {"zebra", "mango", "apple", "delta", "foxtrot", "bravo", "yankee", "tango", "kilo", "echo"}; Map map = new LinkedHashMap<>(); for (String key : keys) { map.put(key, "value"); } MapOperationRequest request = MapOperationRequest.builder().map(map).build(); + List reflective = reflectiveMap(request); List low = lowered(request); - assertEquals(reflectiveMap(request), low, "lowered binding must equal reflective evaluation"); + assertEquals(reflective.size(), low.size(), + "lowered result must contain the same number of keys as reflective evaluation"); + assertEquals(new HashSet<>(reflective), new HashSet<>(low), + "lowered key content must equal reflective evaluation"); assertEquals(new HashSet<>(Arrays.asList(keys)), new HashSet<>(low), "content must match"); } @@ -195,8 +197,12 @@ private static List reflectiveNestedMap(NestedMapOperationRequest reques } private static void assertNestedMapEquivalent(NestedMapOperationRequest request, List expectedContent) { + List reflective = reflectiveNestedMap(request); List low = lowered(request); - assertEquals(reflectiveNestedMap(request), low, "lowered binding must equal reflective evaluation"); + assertEquals(reflective.size(), low.size(), + "lowered result must contain the same number of keys as reflective evaluation"); + assertEquals(new HashSet<>(reflective), new HashSet<>(low), + "lowered key content must equal reflective evaluation"); assertEquals(new HashSet<>(expectedContent), new HashSet<>(low), "content must match"); } From 7a3621febcd8adb1a0e6872cb87a640ea3c2cced Mon Sep 17 00:00:00 2001 From: David Ho Date: Tue, 22 Sep 2026 11:28:20 -0700 Subject: [PATCH 5/5] Update changelog --- .changes/next-release/feature-AWSSDKforJavav2-0f38bd9.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changes/next-release/feature-AWSSDKforJavav2-0f38bd9.json b/.changes/next-release/feature-AWSSDKforJavav2-0f38bd9.json index aceb05b46d6e..e19dc9e718a1 100644 --- a/.changes/next-release/feature-AWSSDKforJavav2-0f38bd9.json +++ b/.changes/next-release/feature-AWSSDKforJavav2-0f38bd9.json @@ -2,5 +2,5 @@ "type": "perf-improvement", "category": "AWS SDK for Java v2", "contributor": "", - "description": "Endpoint parameters derived from JMESPath expressions (operationContextParams) are now bound using typed getters generated at build time instead of a reflective runtime, reducing per-request allocation and latency during endpoint resolution. Generated keys() bindings now iterate request maps directly instead of copying them through HashMap. This changes the unspecified order of the derived key list, but current DynamoDB endpoint routing behavior is unchanged." + "description": "Fix unnecessary allocation and request latency during endpoint resolution for operations that derive endpoint parameters from request data, including DynamoDB batch and transaction operations and Amazon S3 DeleteObjects. These bindings now use generated typed getters and iterate request maps directly instead of using reflection and copying maps. The order of derived key lists may change, but current DynamoDB endpoint routing behavior is unchanged." }