From 8a81efb1a4d162dfd0d0529700273751c2c30e44 Mon Sep 17 00:00:00 2001 From: John Viegas Date: Sun, 12 Jul 2026 16:27:01 +0000 Subject: [PATCH 1/2] Update to give more preference to operation with outputs --- .../poet/crac/WarmUpOperationSelector.java | 124 ++++++++ .../crac/WarmUpOperationSelectorTest.java | 292 ++++++++++++++++++ 2 files changed, 416 insertions(+) create mode 100644 codegen/src/main/java/software/amazon/awssdk/codegen/poet/crac/WarmUpOperationSelector.java create mode 100644 codegen/src/test/java/software/amazon/awssdk/codegen/poet/crac/WarmUpOperationSelectorTest.java diff --git a/codegen/src/main/java/software/amazon/awssdk/codegen/poet/crac/WarmUpOperationSelector.java b/codegen/src/main/java/software/amazon/awssdk/codegen/poet/crac/WarmUpOperationSelector.java new file mode 100644 index 000000000000..5d946143f1d4 --- /dev/null +++ b/codegen/src/main/java/software/amazon/awssdk/codegen/poet/crac/WarmUpOperationSelector.java @@ -0,0 +1,124 @@ +/* + * 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.crac; + +import java.util.Arrays; +import java.util.Collections; +import java.util.Comparator; +import java.util.List; +import java.util.Optional; +import software.amazon.awssdk.codegen.model.intermediate.IntermediateModel; +import software.amazon.awssdk.codegen.model.intermediate.MemberModel; +import software.amazon.awssdk.codegen.model.intermediate.OperationModel; +import software.amazon.awssdk.codegen.model.intermediate.ShapeModel; +import software.amazon.awssdk.utils.CollectionUtils; +import software.amazon.awssdk.utils.NumericUtils; + +/** + * Selects the operation used for the CRaC warm-up call: filters out streaming/event-stream and deprecated + * operations, then ranks the rest (see {@link #warmUpPreference}). + */ +public final class WarmUpOperationSelector { + + private static final List PREFERRED_VERBS = Arrays.asList("List", "Describe", "Get"); + + private static final Comparator BY_EMPTY_REQUEST_FIRST = + Comparator.comparing(op -> !acceptsEmptyRequest(op)); + + private static final Comparator BY_FEWEST_REQUIRED_INPUTS = + Comparator.comparingInt(WarmUpOperationSelector::requiredInputMemberCount); + + private static final Comparator BY_HAS_OUTPUT_FIRST = + Comparator.comparing(op -> !hasOutput(op)); + + private static final Comparator BY_PREFERRED_VERB = + Comparator.comparingInt(WarmUpOperationSelector::verbRank); + + private static final Comparator BY_NAME_ALPHABETICAL = + Comparator.comparing(OperationModel::getOperationName); + + private WarmUpOperationSelector() { + } + + /** + * Selects the warm-up operation for the given service, or {@link Optional#empty()} if no operation is safe to + * call as a warm-up. + */ + public static Optional selectWarmUpOperation(IntermediateModel model) { + List verifiedSimpleMethods = model.getCustomizationConfig().getVerifiedSimpleMethods(); + Comparator preference = warmUpPreference(verifiedSimpleMethods); + + return model.getOperations().values().stream() + .filter(WarmUpOperationSelector::passesHardGates) + .min(preference); + } + + /** + * Preference order: returns output (so the unmarshaller is primed too), verified simple method, accepts an + * empty request, fewest required input members, read-only verb, then operation name as the deterministic + * tie-break. + */ + private static Comparator warmUpPreference(List verifiedSimpleMethods) { + Comparator byVerifiedSimpleFirst = + Comparator.comparing(op -> !verifiedSimpleMethods.contains(op.getMethodName())); + + return BY_HAS_OUTPUT_FIRST + .thenComparing(byVerifiedSimpleFirst) + .thenComparing(BY_EMPTY_REQUEST_FIRST) + .thenComparing(BY_FEWEST_REQUIRED_INPUTS) + .thenComparing(BY_PREFERRED_VERB) + .thenComparing(BY_NAME_ALPHABETICAL); + } + + private static boolean passesHardGates(OperationModel operation) { + return !isStreamingOrEventStream(operation) + && !operation.isDeprecated(); + } + + private static boolean isStreamingOrEventStream(OperationModel operation) { + return operation.isStreaming() || operation.hasEventStreamInput() || operation.hasEventStreamOutput(); + } + + private static boolean acceptsEmptyRequest(OperationModel operation) { + return requiredInputMemberCount(operation) == 0; + } + + private static boolean hasOutput(OperationModel operation) { + return operation.getOutputShape() != null; + } + + private static int requiredInputMemberCount(OperationModel operation) { + return NumericUtils.saturatedCast(inputMembers(operation).stream().filter(MemberModel::isRequired).count()); + } + + private static List inputMembers(OperationModel operation) { + ShapeModel inputShape = operation.getInputShape(); + if (inputShape == null || CollectionUtils.isNullOrEmpty(inputShape.getMembers())) { + return Collections.emptyList(); + } + return inputShape.getMembers(); + } + + private static int verbRank(OperationModel operation) { + String operationName = operation.getOperationName(); + for (int i = 0; i < PREFERRED_VERBS.size(); i++) { + if (operationName.startsWith(PREFERRED_VERBS.get(i))) { + return i; + } + } + return Integer.MAX_VALUE; + } +} diff --git a/codegen/src/test/java/software/amazon/awssdk/codegen/poet/crac/WarmUpOperationSelectorTest.java b/codegen/src/test/java/software/amazon/awssdk/codegen/poet/crac/WarmUpOperationSelectorTest.java new file mode 100644 index 000000000000..e528a81d9154 --- /dev/null +++ b/codegen/src/test/java/software/amazon/awssdk/codegen/poet/crac/WarmUpOperationSelectorTest.java @@ -0,0 +1,292 @@ +/* + * 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.crac; + +import static org.assertj.core.api.Assertions.assertThat; + +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; +import java.util.stream.Stream; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; +import software.amazon.awssdk.codegen.model.config.customization.CustomizationConfig; +import software.amazon.awssdk.codegen.model.intermediate.IntermediateModel; +import software.amazon.awssdk.codegen.model.intermediate.MemberModel; +import software.amazon.awssdk.codegen.model.intermediate.Metadata; +import software.amazon.awssdk.codegen.model.intermediate.OperationModel; +import software.amazon.awssdk.codegen.model.intermediate.ShapeModel; + +public class WarmUpOperationSelectorTest { + + @ParameterizedTest(name = "{0}") + @MethodSource("selectionScenarios") + public void selectsExpectedOperation(Scenario scenario) { + Optional selected = WarmUpOperationSelector.selectWarmUpOperation(scenario.model()); + + if (scenario.expected == null) { + assertThat(selected).isEmpty(); + } else { + assertThat(selected).map(OperationModel::getOperationName).contains(scenario.expected); + } + } + + private static Stream selectionScenarios() { + return Stream.of( + // Filter: streaming, event-stream and deprecated operations. + scenario("streamingInputOperationOnly_isNotSelected") + .operation(op("PutObject").withStreamingInput()) + .expectNothing(), + scenario("streamingOutputOperationOnly_isNotSelected") + .operation(op("GetObject").withStreamingOutput()) + .expectNothing(), + scenario("eventStreamInputOperationOnly_isNotSelected") + .operation(op("StartConversation").withEventStreamInput()) + .expectNothing(), + scenario("eventStreamOutputOperationOnly_isNotSelected") + .operation(op("SubscribeToShard").withEventStreamOutput()) + .expectNothing(), + scenario("deprecatedOperationOnly_isNotSelected") + .operation(op("OldListThings").deprecated()) + .expectNothing(), + scenario("serviceWithNoOperations_selectsNothing") + .expectNothing(), + + // Preference 1: returns output, so the unmarshaller is primed too. An operation with no output shape + // (e.g. S3's DeleteBucket) can win every other tier and still lose here. + scenario("hasOutput_beatsVoidOutput_whenOtherwiseEqual") + .operation(op("ListThings").withOutput()) + .operation(op("ListOthers")) + .expect("ListThings"), + scenario("hasOutput_outranksVerifiedSimpleMethod") + .verifiedSimpleMethods("listThings") + .operation(op("ListThings")) + .operation(op("ListOthers").withOutput()) + .expect("ListOthers"), + scenario("hasOutput_outranksEmptyRequest") + .operation(op("ListThings")) + .operation(op("ListOthers").withOutput().withRequiredMembers(1)) + .expect("ListOthers"), + scenario("hasOutput_outranksFewestRequiredMembers") + .operation(op("ListThings").withRequiredMembers(1)) + .operation(op("ListOthers").withOutput().withRequiredMembers(2)) + .expect("ListOthers"), + scenario("hasOutput_outranksPreferredVerb") + .operation(op("ListThings")) + .operation(op("DescribeThings").withOutput()) + .expect("DescribeThings"), + scenario("hasOutput_outranksEveryOtherPreferenceCombined") + .verifiedSimpleMethods("deleteThings") + .operation(op("DeleteThings")) + .operation(op("ListOthers").withOutput().withRequiredMembers(1)) + .expect("ListOthers"), + + // Preference 2: verified simple method. Both operations have output so tier 1 ties. + scenario("verifiedSimpleMethod_beatsNonVerified_whenOtherwiseEqual") + .verifiedSimpleMethods("listThings") + .operation(op("ListThings").withOutput()) + .operation(op("ListOthers").withOutput()) + .expect("ListThings"), + scenario("verifiedSimpleMethod_outranksEmptyRequest") + .verifiedSimpleMethods("listThings") + .operation(op("ListThings").withOutput().withRequiredMembers(1)) + .operation(op("ListOthers").withOutput()) + .expect("ListThings"), + + // Preference 3: accepts an empty request. Both operations have output and neither is verified simple. + scenario("emptyRequest_beatsRequiredMembers") + .operation(op("ListThings").withOutput()) + .operation(op("ListOthers").withOutput().withRequiredMembers(1)) + .expect("ListThings"), + + // Preference 4: fewest required input members. Both operations have output and require input, so + // tiers 1-3 tie. + scenario("fewerRequiredMembers_beatsMore_whenBothRequireInput") + .operation(op("ListThings").withOutput().withRequiredMembers(1)) + .operation(op("ListOthers").withOutput().withRequiredMembers(2)) + .expect("ListThings"), + + // Preference 5: read-only verb, List > Describe > Get. Fixtures are chosen so the alphabetical + // tie-break would pick the loser. + scenario("verb_listBeatsDescribe") + .operation(op("DescribeThings").withOutput()) + .operation(op("ListThings").withOutput()) + .expect("ListThings"), + scenario("verb_describeBeatsNonPreferred") + .operation(op("BatchThings").withOutput()) + .operation(op("DescribeThings").withOutput()) + .expect("DescribeThings"), + scenario("verb_getBeatsNonPreferred") + .operation(op("CountThings").withOutput()) + .operation(op("GetThings").withOutput()) + .expect("GetThings"), + scenario("verb_describeBeatsGet") + .operation(op("DescribeThings").withOutput()) + .operation(op("GetThings").withOutput()) + .expect("DescribeThings"), + + // Preference 6: alphabetical tie-break. + scenario("fullyTiedOperations_areBrokenAlphabetically") + .operation(op("BravoOperation").withOutput()) + .operation(op("AlphaOperation").withOutput()) + .expect("AlphaOperation"), + + // All four operations return output. + scenario("dynamoDbWorkedExample_selectsListTables") + .verifiedSimpleMethods("listTables", "describeLimits") + .operation(op("ListTables").withOutput()) + .operation(op("DescribeLimits").withOutput()) + .operation(op("GetItem").withOutput().withRequiredMembers(2)) + .operation(op("PutItem").withRequiredMembers(2)) + .expect("ListTables") + ); + } + + private static Scenario scenario(String name) { + return new Scenario(name); + } + + private static OperationBuilder op(String operationName) { + return new OperationBuilder(operationName); + } + + private static final class Scenario { + private final String name; + private final List operations = new ArrayList<>(); + private List verifiedSimpleMethods = Collections.emptyList(); + private String expected; + + private Scenario(String name) { + this.name = name; + } + + private Scenario verifiedSimpleMethods(String... methodNames) { + this.verifiedSimpleMethods = Arrays.asList(methodNames); + return this; + } + + private Scenario operation(OperationBuilder operation) { + this.operations.add(operation.build()); + return this; + } + + private Scenario expect(String operationName) { + this.expected = operationName; + return this; + } + + private Scenario expectNothing() { + this.expected = null; + return this; + } + + private IntermediateModel model() { + Map operationsByName = new HashMap<>(); + for (OperationModel operation : operations) { + operationsByName.put(operation.getOperationName(), operation); + } + + CustomizationConfig config = CustomizationConfig.create(); + config.setVerifiedSimpleMethods(verifiedSimpleMethods); + + Metadata metadata = new Metadata().withServiceName("TestService"); + return new IntermediateModel(metadata, operationsByName, Collections.emptyMap(), config); + } + + @Override + public String toString() { + return name; + } + } + + private static final class OperationBuilder { + private final OperationModel operation = new OperationModel(); + + private OperationBuilder(String operationName) { + operation.setOperationName(operationName); + } + + private OperationBuilder withOutput() { + operation.setOutputShape(new ShapeModel()); + return this; + } + + private OperationBuilder withRequiredMembers(int requiredCount) { + ShapeModel shape = new ShapeModel(); + List members = new ArrayList<>(); + for (int i = 0; i < requiredCount; i++) { + MemberModel member = new MemberModel(); + member.setName("Member" + i); + member.setRequired(true); + members.add(member); + } + shape.setMembers(members); + operation.setInputShape(shape); + return this; + } + + private OperationBuilder withStreamingInput() { + operation.setInputShape(streamingShape()); + return this; + } + + private OperationBuilder withStreamingOutput() { + operation.setOutputShape(streamingShape()); + return this; + } + + private OperationBuilder withEventStreamInput() { + operation.setInputShape(eventStreamShape()); + return this; + } + + private OperationBuilder withEventStreamOutput() { + operation.setOutputShape(eventStreamShape()); + return this; + } + + private OperationBuilder deprecated() { + operation.setDeprecated(true); + return this; + } + + private OperationModel build() { + return operation; + } + + private static ShapeModel streamingShape() { + ShapeModel shape = new ShapeModel(); + shape.setHasStreamingMember(true); + return shape; + } + + private static ShapeModel eventStreamShape() { + ShapeModel eventStreamShape = new ShapeModel(); + eventStreamShape.withIsEventStream(true); + + MemberModel member = new MemberModel(); + member.setShape(eventStreamShape); + + ShapeModel shape = new ShapeModel(); + shape.setMembers(Collections.singletonList(member)); + return shape; + } + } +} From 194c60e1ec4bba8502760ce80898558d43ea6b09 Mon Sep 17 00:00:00 2001 From: John Viegas Date: Tue, 14 Jul 2026 22:34:39 +0000 Subject: [PATCH 2/2] Handle review comments --- .../poet/crac/WarmUpOperationSelector.java | 31 ++++++++++++------- .../crac/WarmUpOperationSelectorTest.java | 31 ++++++++++++++----- 2 files changed, 44 insertions(+), 18 deletions(-) diff --git a/codegen/src/main/java/software/amazon/awssdk/codegen/poet/crac/WarmUpOperationSelector.java b/codegen/src/main/java/software/amazon/awssdk/codegen/poet/crac/WarmUpOperationSelector.java index 5d946143f1d4..a84a872a7a4c 100644 --- a/codegen/src/main/java/software/amazon/awssdk/codegen/poet/crac/WarmUpOperationSelector.java +++ b/codegen/src/main/java/software/amazon/awssdk/codegen/poet/crac/WarmUpOperationSelector.java @@ -35,15 +35,18 @@ public final class WarmUpOperationSelector { private static final List PREFERRED_VERBS = Arrays.asList("List", "Describe", "Get"); + private static final Comparator BY_HAS_OUTPUT_FIRST = + Comparator.comparing(op -> hasOutput(op) ? 0 : 1); + + private static final Comparator BY_AUTHENTICATED_FIRST = + Comparator.comparing(op -> op.isAuthenticated() ? 0 : 1); + private static final Comparator BY_EMPTY_REQUEST_FIRST = - Comparator.comparing(op -> !acceptsEmptyRequest(op)); + Comparator.comparing(op -> acceptsEmptyRequest(op) ? 0 : 1); private static final Comparator BY_FEWEST_REQUIRED_INPUTS = Comparator.comparingInt(WarmUpOperationSelector::requiredInputMemberCount); - private static final Comparator BY_HAS_OUTPUT_FIRST = - Comparator.comparing(op -> !hasOutput(op)); - private static final Comparator BY_PREFERRED_VERB = Comparator.comparingInt(WarmUpOperationSelector::verbRank); @@ -67,22 +70,28 @@ public static Optional selectWarmUpOperation(IntermediateModel m } /** - * Preference order: returns output (so the unmarshaller is primed too), verified simple method, accepts an - * empty request, fewest required input members, read-only verb, then operation name as the deterministic - * tie-break. + * Preference order: returns output (so the unmarshaller is primed too), is authenticated (so signing is primed + * too; {@code noAuth} operations skip signing entirely), verified simple method, accepts an empty request, + * fewest required input members, read-only verb, then operation name as the deterministic tie-break. */ private static Comparator warmUpPreference(List verifiedSimpleMethods) { - Comparator byVerifiedSimpleFirst = - Comparator.comparing(op -> !verifiedSimpleMethods.contains(op.getMethodName())); - return BY_HAS_OUTPUT_FIRST - .thenComparing(byVerifiedSimpleFirst) + .thenComparing(BY_AUTHENTICATED_FIRST) + .thenComparing(byVerifiedSimpleFirst(verifiedSimpleMethods)) .thenComparing(BY_EMPTY_REQUEST_FIRST) .thenComparing(BY_FEWEST_REQUIRED_INPUTS) .thenComparing(BY_PREFERRED_VERB) .thenComparing(BY_NAME_ALPHABETICAL); } + /** + * Unlike the other tiers, this one depends on the service's customization config, so it cannot be a static + * comparator constant. + */ + private static Comparator byVerifiedSimpleFirst(List verifiedSimpleMethods) { + return Comparator.comparing(op -> verifiedSimpleMethods.contains(op.getMethodName()) ? 0 : 1); + } + private static boolean passesHardGates(OperationModel operation) { return !isStreamingOrEventStream(operation) && !operation.isDeprecated(); diff --git a/codegen/src/test/java/software/amazon/awssdk/codegen/poet/crac/WarmUpOperationSelectorTest.java b/codegen/src/test/java/software/amazon/awssdk/codegen/poet/crac/WarmUpOperationSelectorTest.java index e528a81d9154..73c9eff943fb 100644 --- a/codegen/src/test/java/software/amazon/awssdk/codegen/poet/crac/WarmUpOperationSelectorTest.java +++ b/codegen/src/test/java/software/amazon/awssdk/codegen/poet/crac/WarmUpOperationSelectorTest.java @@ -98,7 +98,19 @@ private static Stream selectionScenarios() { .operation(op("ListOthers").withOutput().withRequiredMembers(1)) .expect("ListOthers"), - // Preference 2: verified simple method. Both operations have output so tier 1 ties. + // Preference 2: is authenticated, so signing is primed too. Both operations have output so tier 1 ties. + scenario("authenticated_beatsNoAuth_whenOtherwiseEqual") + .operation(op("ListThings").withOutput()) + .operation(op("ListOthers").withOutput().withNoAuth()) + .expect("ListThings"), + scenario("authenticated_outranksVerifiedSimpleMethod") + .verifiedSimpleMethods("listOthers") + .operation(op("ListThings").withOutput()) + .operation(op("ListOthers").withOutput().withNoAuth()) + .expect("ListThings"), + + // Preference 3: verified simple method. Both operations have output and are authenticated, so tiers + // 1-2 tie. scenario("verifiedSimpleMethod_beatsNonVerified_whenOtherwiseEqual") .verifiedSimpleMethods("listThings") .operation(op("ListThings").withOutput()) @@ -110,20 +122,21 @@ private static Stream selectionScenarios() { .operation(op("ListOthers").withOutput()) .expect("ListThings"), - // Preference 3: accepts an empty request. Both operations have output and neither is verified simple. + // Preference 4: accepts an empty request. Both operations have output and are authenticated, and + // neither is verified simple, so tiers 1-3 tie. scenario("emptyRequest_beatsRequiredMembers") .operation(op("ListThings").withOutput()) .operation(op("ListOthers").withOutput().withRequiredMembers(1)) .expect("ListThings"), - // Preference 4: fewest required input members. Both operations have output and require input, so - // tiers 1-3 tie. + // Preference 5: fewest required input members. Both operations have output and require input, so + // tiers 1-4 tie. scenario("fewerRequiredMembers_beatsMore_whenBothRequireInput") .operation(op("ListThings").withOutput().withRequiredMembers(1)) .operation(op("ListOthers").withOutput().withRequiredMembers(2)) .expect("ListThings"), - // Preference 5: read-only verb, List > Describe > Get. Fixtures are chosen so the alphabetical + // Preference 6: read-only verb, List > Describe > Get. Fixtures are chosen so the alphabetical // tie-break would pick the loser. scenario("verb_listBeatsDescribe") .operation(op("DescribeThings").withOutput()) @@ -142,13 +155,12 @@ private static Stream selectionScenarios() { .operation(op("GetThings").withOutput()) .expect("DescribeThings"), - // Preference 6: alphabetical tie-break. + // Preference 7: alphabetical tie-break. scenario("fullyTiedOperations_areBrokenAlphabetically") .operation(op("BravoOperation").withOutput()) .operation(op("AlphaOperation").withOutput()) .expect("AlphaOperation"), - // All four operations return output. scenario("dynamoDbWorkedExample_selectsListTables") .verifiedSimpleMethods("listTables", "describeLimits") .operation(op("ListTables").withOutput()) @@ -267,6 +279,11 @@ private OperationBuilder deprecated() { return this; } + private OperationBuilder withNoAuth() { + operation.setIsAuthenticated(false); + return this; + } + private OperationModel build() { return operation; }