From cc3677d238f2f566e17e4c4e3467ab80e63bad75 Mon Sep 17 00:00:00 2001 From: Zoe Wang <33073555+zoewangg@users.noreply.github.com> Date: Thu, 18 Jun 2026 10:57:20 -0700 Subject: [PATCH 1/3] Throw clear model validation error for unresolved member shape targets in codegen When a shape member's target references a shape that does not exist in the intermediate model (missing from the service model, removed by a customization, or misspelled), codegen previously failed with a cryptic NullPointerException deep in POJO generation. Add an eager check in IntermediateModelBuilder after member-to-shape linking that throws ModelInvalidException (UNKNOWN_SHAPE_MEMBER) naming the owning shape, member, and unresolved target. --- .../codegen/IntermediateModelBuilder.java | 2 + .../MemberShapeTargetValidator.java | 96 +++++++++ .../MemberShapeTargetValidationBuildTest.java | 60 ++++++ .../MemberShapeTargetValidatorTest.java | 195 ++++++++++++++++++ .../customization.config | 3 + .../dangling-shape-validator/service-2.json | 47 +++++ 6 files changed, 403 insertions(+) create mode 100644 codegen/src/main/java/software/amazon/awssdk/codegen/validation/MemberShapeTargetValidator.java create mode 100644 codegen/src/test/java/software/amazon/awssdk/codegen/MemberShapeTargetValidationBuildTest.java create mode 100644 codegen/src/test/java/software/amazon/awssdk/codegen/validation/MemberShapeTargetValidatorTest.java create mode 100644 codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/c2j/dangling-shape-validator/customization.config create mode 100644 codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/c2j/dangling-shape-validator/service-2.json diff --git a/codegen/src/main/java/software/amazon/awssdk/codegen/IntermediateModelBuilder.java b/codegen/src/main/java/software/amazon/awssdk/codegen/IntermediateModelBuilder.java index 33290241d534..8848973412d9 100644 --- a/codegen/src/main/java/software/amazon/awssdk/codegen/IntermediateModelBuilder.java +++ b/codegen/src/main/java/software/amazon/awssdk/codegen/IntermediateModelBuilder.java @@ -46,6 +46,7 @@ import software.amazon.awssdk.codegen.model.service.Waiters; import software.amazon.awssdk.codegen.naming.DefaultNamingStrategy; import software.amazon.awssdk.codegen.naming.NamingStrategy; +import software.amazon.awssdk.codegen.validation.MemberShapeTargetValidator; import software.amazon.awssdk.utils.CollectionUtils; /** @@ -162,6 +163,7 @@ public IntermediateModel build() { service.getClientContextParams()); linkMembersToShapes(trimmedModel); + MemberShapeTargetValidator.validate(trimmedModel); linkOperationsToInputOutputShapes(trimmedModel); linkCustomAuthorizationToRequestShapes(trimmedModel); diff --git a/codegen/src/main/java/software/amazon/awssdk/codegen/validation/MemberShapeTargetValidator.java b/codegen/src/main/java/software/amazon/awssdk/codegen/validation/MemberShapeTargetValidator.java new file mode 100644 index 000000000000..868a41df99c8 --- /dev/null +++ b/codegen/src/main/java/software/amazon/awssdk/codegen/validation/MemberShapeTargetValidator.java @@ -0,0 +1,96 @@ +/* + * 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.validation; + +import software.amazon.awssdk.codegen.model.intermediate.IntermediateModel; +import software.amazon.awssdk.codegen.model.intermediate.MapModel; +import software.amazon.awssdk.codegen.model.intermediate.MemberModel; +import software.amazon.awssdk.codegen.model.intermediate.ShapeModel; + +/** + * Validates that every member which is expected to reference a model shape actually resolves to one after member-to-shape + * linking. A member can be left referencing a target shape that no longer exists in the intermediate model (missing from the + * service model, removed by a customization, or misspelled), which otherwise surfaces as a cryptic {@link NullPointerException} + * deep inside code emission. + * + *

The classification of which members reference a shape mirrors {@code RemoveUnusedShapes.resolveMemberShapes} so that + * validation and shape retention never disagree. + */ +public final class MemberShapeTargetValidator { + + private MemberShapeTargetValidator() { + } + + /** + * Fails fast on the first member whose target shape cannot be resolved. + * + * @throws ModelInvalidException if a member references a shape that does not exist in the model. + */ + public static void validate(IntermediateModel model) { + for (ShapeModel shape : model.getShapes().values()) { + if (shape.getMembers() == null) { + continue; + } + for (MemberModel member : shape.getMembers()) { + validateMember(model, shape, member, member); + } + } + } + + /** + * @param topLevelMember the member declared directly on the shape; for list/map element members this stays pointed at the + * container member so the error message identifies a member the service team can locate. + */ + private static void validateMember(IntermediateModel model, ShapeModel shape, MemberModel topLevelMember, + MemberModel member) { + if (member == null) { + return; + } + + if (member.getEnumType() != null) { + requireResolvable(model, shape, topLevelMember, member, member.getEnumType()); + } else if (member.isList()) { + validateMember(model, shape, topLevelMember, member.getListModel().getListMemberModel()); + } else if (member.isMap()) { + MapModel mapModel = member.getMapModel(); + validateMember(model, shape, topLevelMember, mapModel.getKeyModel()); + validateMember(model, shape, topLevelMember, mapModel.getValueModel()); + } else if (!member.isSimple()) { + requireResolvable(model, shape, topLevelMember, member, member.getVariable().getSimpleType()); + } + } + + private static void requireResolvable(IntermediateModel model, ShapeModel shape, MemberModel topLevelMember, + MemberModel member, String targetName) { + // linkMembersToShapes only links members declared directly on a shape, so a list/map element member always carries a + // null shape; resolve its target by name (matching shape retention) instead of reading the unset getShape(). + boolean resolved = member == topLevelMember ? member.getShape() != null + : model.getShapes().containsKey(targetName); + if (!resolved) { + throw dangling(shape, topLevelMember, targetName); + } + } + + private static ModelInvalidException dangling(ShapeModel shape, MemberModel member, String targetName) { + String detail = String.format( + "Member '%s' of shape '%s' targets shape '%s' which does not exist in the intermediate model. The target shape " + + "may be missing from the service model, removed by a customization, or misspelled.", + member.getC2jName(), shape.getShapeName(), targetName); + + return ModelInvalidException.fromEntry( + ValidationEntry.create(ValidationErrorId.UNKNOWN_SHAPE_MEMBER, ValidationErrorSeverity.DANGER, detail)); + } +} diff --git a/codegen/src/test/java/software/amazon/awssdk/codegen/MemberShapeTargetValidationBuildTest.java b/codegen/src/test/java/software/amazon/awssdk/codegen/MemberShapeTargetValidationBuildTest.java new file mode 100644 index 000000000000..24b59035fe3d --- /dev/null +++ b/codegen/src/test/java/software/amazon/awssdk/codegen/MemberShapeTargetValidationBuildTest.java @@ -0,0 +1,60 @@ +/* + * 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; + +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import java.io.File; +import org.junit.jupiter.api.Test; +import software.amazon.awssdk.codegen.model.config.customization.CustomizationConfig; +import software.amazon.awssdk.codegen.model.service.ServiceModel; +import software.amazon.awssdk.codegen.utils.ModelLoaderUtils; +import software.amazon.awssdk.codegen.validation.ModelInvalidException; +import software.amazon.awssdk.codegen.validation.ValidationEntry; +import software.amazon.awssdk.codegen.validation.ValidationErrorId; +import software.amazon.awssdk.codegen.validation.ValidationErrorSeverity; + +/** + * Full-build coverage proving that member-to-shape linking actually leaves a null shape when a referenced target is removed by + * a customization, and that {@link IntermediateModelBuilder#build()} fails fast instead of producing a downstream + * {@link NullPointerException}. Reproduces P456014803. + */ +public class MemberShapeTargetValidationBuildTest { + + @Test + public void build_enumTargetRemovedByDeprecatedShapesCustomization_throwsModelInvalid() { + File serviceModelFile = new File(MemberShapeTargetValidationBuildTest.class + .getResource("poet/client/c2j/dangling-shape-validator/service-2.json").getFile()); + File customizationFile = new File(MemberShapeTargetValidationBuildTest.class + .getResource("poet/client/c2j/dangling-shape-validator/customization.config").getFile()); + + C2jModels models = C2jModels.builder() + .serviceModel(ModelLoaderUtils.loadModel(ServiceModel.class, serviceModelFile)) + .customizationConfig(ModelLoaderUtils.loadModel(CustomizationConfig.class, customizationFile)) + .build(); + + assertThatThrownBy(() -> new IntermediateModelBuilder(models).build()) + .isInstanceOf(ModelInvalidException.class) + .hasMessageContaining("GetRequestAuthorizationDetailsResponse") + .hasMessageContaining("AuthorizationDetails") + .hasMessageContaining("AuthDetailType") + .matches(e -> { + ValidationEntry entry = ((ModelInvalidException) e).validationEntries().get(0); + return entry.getErrorId() == ValidationErrorId.UNKNOWN_SHAPE_MEMBER + && entry.getSeverity() == ValidationErrorSeverity.DANGER; + }, "validation entry is UNKNOWN_SHAPE_MEMBER / DANGER"); + } +} diff --git a/codegen/src/test/java/software/amazon/awssdk/codegen/validation/MemberShapeTargetValidatorTest.java b/codegen/src/test/java/software/amazon/awssdk/codegen/validation/MemberShapeTargetValidatorTest.java new file mode 100644 index 000000000000..90809af8bef4 --- /dev/null +++ b/codegen/src/test/java/software/amazon/awssdk/codegen/validation/MemberShapeTargetValidatorTest.java @@ -0,0 +1,195 @@ +/* + * 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.validation; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import org.junit.jupiter.api.Test; +import software.amazon.awssdk.codegen.model.intermediate.IntermediateModel; +import software.amazon.awssdk.codegen.model.intermediate.ListModel; +import software.amazon.awssdk.codegen.model.intermediate.MapModel; +import software.amazon.awssdk.codegen.model.intermediate.MemberModel; +import software.amazon.awssdk.codegen.model.intermediate.ShapeModel; +import software.amazon.awssdk.codegen.model.intermediate.VariableModel; +import software.amazon.awssdk.codegen.poet.ClientTestModels; + +public class MemberShapeTargetValidatorTest { + + @Test + void validate_enumMemberWithDanglingTarget_throwsWithShapeMemberAndTarget() { + String owningShape = "GetRequestAuthorizationDetailsResponse"; + MemberModel enumMember = new MemberModel() + .withC2jName("AuthorizationDetails") + .withC2jShape("AuthDetailType") + .withVariable(new VariableModel("authorizationDetails", "String")) + .withEnumType("AuthDetailType"); + IntermediateModel model = modelWithShape(owningShape, enumMember); + + assertThatThrownBy(() -> MemberShapeTargetValidator.validate(model)) + .isInstanceOf(ModelInvalidException.class) + .matches(MemberShapeTargetValidatorTest::isUnknownShapeMemberDanger) + .hasMessageContaining(owningShape) + .hasMessageContaining("AuthorizationDetails") + .hasMessageContaining("AuthDetailType"); + } + + @Test + void validate_structureMemberWithDanglingTarget_throwsWithShapeMemberAndTarget() { + String owningShape = "ContainerShape"; + MemberModel structureMember = new MemberModel() + .withC2jName("Nested") + .withC2jShape("MissingStruct") + .withVariable(new VariableModel("nested", "MissingStruct")); + IntermediateModel model = modelWithShape(owningShape, structureMember); + + assertThatThrownBy(() -> MemberShapeTargetValidator.validate(model)) + .isInstanceOf(ModelInvalidException.class) + .matches(MemberShapeTargetValidatorTest::isUnknownShapeMemberDanger) + .hasMessageContaining(owningShape) + .hasMessageContaining("Nested") + .hasMessageContaining("MissingStruct"); + } + + @Test + void validate_listElementWithDanglingTarget_throwsIdentifyingContainerMember() { + String owningShape = "ListContainerShape"; + MemberModel elementMember = new MemberModel() + .withC2jName("member") + .withC2jShape("MissingElement") + .withVariable(new VariableModel("member", "MissingElement")); + MemberModel listMember = new MemberModel() + .withC2jName("Items") + .withC2jShape("ItemList") + .withVariable(new VariableModel("items", "java.util.List")) + .withListModel(new ListModel("MissingElement", null, "java.util.ArrayList", "java.util.List", elementMember)); + IntermediateModel model = modelWithShape(owningShape, listMember); + + assertThatThrownBy(() -> MemberShapeTargetValidator.validate(model)) + .isInstanceOf(ModelInvalidException.class) + .matches(MemberShapeTargetValidatorTest::isUnknownShapeMemberDanger) + .hasMessageContaining(owningShape) + .hasMessageContaining("Items") + .hasMessageContaining("MissingElement"); + } + + @Test + void validate_mapValueWithDanglingTarget_throwsIdentifyingContainerMember() { + String owningShape = "MapContainerShape"; + MemberModel keyMember = new MemberModel() + .withC2jName("key") + .withC2jShape("String") + .withVariable(new VariableModel("key", "String")); + MemberModel valueMember = new MemberModel() + .withC2jName("value") + .withC2jShape("MissingValue") + .withVariable(new VariableModel("value", "MissingValue")); + MemberModel mapMember = new MemberModel() + .withC2jName("Attributes") + .withC2jShape("AttributeMap") + .withVariable(new VariableModel("attributes", "java.util.Map")) + .withMapModel(new MapModel("java.util.HashMap", "java.util.Map", "key", keyMember, "value", valueMember)); + IntermediateModel model = modelWithShape(owningShape, mapMember); + + assertThatThrownBy(() -> MemberShapeTargetValidator.validate(model)) + .isInstanceOf(ModelInvalidException.class) + .matches(MemberShapeTargetValidatorTest::isUnknownShapeMemberDanger) + .hasMessageContaining(owningShape) + .hasMessageContaining("Attributes") + .hasMessageContaining("MissingValue"); + } + + @Test + void validate_scalarMembersWithNullShape_doesNotThrow() { + MemberModel scalarMember = new MemberModel() + .withC2jName("Name") + .withC2jShape("String") + .withVariable(new VariableModel("name", "String")); + IntermediateModel model = modelWithShape("ScalarShape", scalarMember); + + assertThatCode(() -> MemberShapeTargetValidator.validate(model)).doesNotThrowAnyException(); + } + + @Test + void validate_linkedEnumAndStructureMembers_doesNotThrow() { + ShapeModel enumShape = shape("AuthDetailType"); + ShapeModel structShape = shape("NestedStruct"); + + MemberModel enumMember = new MemberModel() + .withC2jName("AuthorizationDetails") + .withC2jShape("AuthDetailType") + .withVariable(new VariableModel("authorizationDetails", "String")) + .withEnumType("AuthDetailType"); + enumMember.setShape(enumShape); + + MemberModel structureMember = new MemberModel() + .withC2jName("Nested") + .withC2jShape("NestedStruct") + .withVariable(new VariableModel("nested", "NestedStruct")); + structureMember.setShape(structShape); + + ShapeModel owning = shape("OwningShape"); + owning.setMembers(Arrays.asList(enumMember, structureMember)); + + Map shapes = new HashMap<>(); + shapes.put("OwningShape", owning); + shapes.put("AuthDetailType", enumShape); + shapes.put("NestedStruct", structShape); + + IntermediateModel model = new IntermediateModel(); + model.setShapes(shapes); + + assertThatCode(() -> MemberShapeTargetValidator.validate(model)).doesNotThrowAnyException(); + } + + @Test + void validate_wellFormedAwsJsonModel_doesNotThrow() { + assertThatCode(() -> MemberShapeTargetValidator.validate(ClientTestModels.awsJsonServiceModels())) + .doesNotThrowAnyException(); + } + + private static IntermediateModel modelWithShape(String shapeName, MemberModel... members) { + ShapeModel shape = shape(shapeName); + shape.setMembers(Arrays.asList(members)); + + Map shapes = new HashMap<>(); + shapes.put(shapeName, shape); + + IntermediateModel model = new IntermediateModel(); + model.setShapes(shapes); + return model; + } + + private static ShapeModel shape(String name) { + ShapeModel shape = new ShapeModel(name); + shape.setShapeName(name); + shape.setMembers(Collections.emptyList()); + return shape; + } + + private static boolean isUnknownShapeMemberDanger(Throwable t) { + List entries = ((ModelInvalidException) t).validationEntries(); + return entries.size() == 1 + && entries.get(0).getErrorId() == ValidationErrorId.UNKNOWN_SHAPE_MEMBER + && entries.get(0).getSeverity() == ValidationErrorSeverity.DANGER; + } +} diff --git a/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/c2j/dangling-shape-validator/customization.config b/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/c2j/dangling-shape-validator/customization.config new file mode 100644 index 000000000000..4bbf53b55844 --- /dev/null +++ b/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/c2j/dangling-shape-validator/customization.config @@ -0,0 +1,3 @@ +{ + "deprecatedShapes": ["AuthDetailType"] +} diff --git a/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/c2j/dangling-shape-validator/service-2.json b/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/c2j/dangling-shape-validator/service-2.json new file mode 100644 index 000000000000..5da6de933135 --- /dev/null +++ b/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/c2j/dangling-shape-validator/service-2.json @@ -0,0 +1,47 @@ +{ + "version": "2.0", + "metadata": { + "apiVersion": "2024-01-01", + "endpointPrefix": "dangling", + "jsonVersion": "1.1", + "protocol": "json", + "serviceAbbreviation": "Dangling", + "serviceFullName": "Dangling Shape Validation Test Service", + "serviceId": "Dangling", + "signatureVersion": "v4", + "targetPrefix": "Dangling", + "uid": "dangling-2024-01-01" + }, + "operations": { + "GetRequestAuthorizationDetails": { + "name": "GetRequestAuthorizationDetails", + "http": { + "method": "POST", + "requestUri": "/" + }, + "input": {"shape": "GetRequestAuthorizationDetailsRequest"}, + "output": {"shape": "GetRequestAuthorizationDetailsResponse"} + } + }, + "shapes": { + "GetRequestAuthorizationDetailsRequest": { + "type": "structure", + "members": { + "Name": {"shape": "String"} + } + }, + "GetRequestAuthorizationDetailsResponse": { + "type": "structure", + "members": { + "AuthorizationDetails": {"shape": "AuthDetailType"} + } + }, + "AuthDetailType": { + "type": "string", + "enum": ["FULL", "PARTIAL"] + }, + "String": { + "type": "string" + } + } +} From 3b6b86155d03bc33ea262d5de1a3a0b599e3950e Mon Sep 17 00:00:00 2001 From: Zoe Wang <33073555+zoewangg@users.noreply.github.com> Date: Thu, 18 Jun 2026 11:01:44 -0700 Subject: [PATCH 2/3] Add changelog entry --- .changes/next-release/bugfix-AWSSDKforJavav2-8aed2cb.json | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changes/next-release/bugfix-AWSSDKforJavav2-8aed2cb.json diff --git a/.changes/next-release/bugfix-AWSSDKforJavav2-8aed2cb.json b/.changes/next-release/bugfix-AWSSDKforJavav2-8aed2cb.json new file mode 100644 index 000000000000..d8c9fb3935c5 --- /dev/null +++ b/.changes/next-release/bugfix-AWSSDKforJavav2-8aed2cb.json @@ -0,0 +1,6 @@ +{ + "type": "bugfix", + "category": "AWS SDK for Java v2", + "contributor": "", + "description": "Codegen now fails with a clear model validation error identifying the offending shape, member, and unresolved target when a shape member references a shape that does not exist in the model, instead of a NullPointerException during code generation." +} From b1391cc90a5fadd0b9df4de462adef44fb188a1b Mon Sep 17 00:00:00 2001 From: Zoe Wang <33073555+zoewangg@users.noreply.github.com> Date: Fri, 19 Jun 2026 16:40:24 -0700 Subject: [PATCH 3/3] Aggregate all unresolved member shape targets into a single validation error --- .../MemberShapeTargetValidator.java | 43 ++++--- .../MemberShapeTargetValidatorTest.java | 106 +++++++++++++++++- 2 files changed, 132 insertions(+), 17 deletions(-) diff --git a/codegen/src/main/java/software/amazon/awssdk/codegen/validation/MemberShapeTargetValidator.java b/codegen/src/main/java/software/amazon/awssdk/codegen/validation/MemberShapeTargetValidator.java index 868a41df99c8..0e1f88e699a0 100644 --- a/codegen/src/main/java/software/amazon/awssdk/codegen/validation/MemberShapeTargetValidator.java +++ b/codegen/src/main/java/software/amazon/awssdk/codegen/validation/MemberShapeTargetValidator.java @@ -15,6 +15,10 @@ package software.amazon.awssdk.codegen.validation; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Set; import software.amazon.awssdk.codegen.model.intermediate.IntermediateModel; import software.amazon.awssdk.codegen.model.intermediate.MapModel; import software.amazon.awssdk.codegen.model.intermediate.MemberModel; @@ -35,19 +39,25 @@ private MemberShapeTargetValidator() { } /** - * Fails fast on the first member whose target shape cannot be resolved. + * Collects every member whose target shape cannot be resolved and, if any are found, throws a single + * {@link ModelInvalidException} carrying one entry per distinct offending member. * - * @throws ModelInvalidException if a member references a shape that does not exist in the model. + * @throws ModelInvalidException if any member references a shape that does not exist in the model. */ public static void validate(IntermediateModel model) { + List entries = new ArrayList<>(); + Set reportedKeys = new HashSet<>(); for (ShapeModel shape : model.getShapes().values()) { if (shape.getMembers() == null) { continue; } for (MemberModel member : shape.getMembers()) { - validateMember(model, shape, member, member); + validateMember(model, shape, member, member, entries, reportedKeys); } } + if (!entries.isEmpty()) { + throw ModelInvalidException.builder().validationEntries(entries).build(); + } } /** @@ -55,42 +65,47 @@ public static void validate(IntermediateModel model) { * container member so the error message identifies a member the service team can locate. */ private static void validateMember(IntermediateModel model, ShapeModel shape, MemberModel topLevelMember, - MemberModel member) { + MemberModel member, List entries, Set reportedKeys) { if (member == null) { return; } if (member.getEnumType() != null) { - requireResolvable(model, shape, topLevelMember, member, member.getEnumType()); + requireResolvable(model, shape, topLevelMember, member, member.getEnumType(), entries, reportedKeys); } else if (member.isList()) { - validateMember(model, shape, topLevelMember, member.getListModel().getListMemberModel()); + validateMember(model, shape, topLevelMember, member.getListModel().getListMemberModel(), entries, reportedKeys); } else if (member.isMap()) { MapModel mapModel = member.getMapModel(); - validateMember(model, shape, topLevelMember, mapModel.getKeyModel()); - validateMember(model, shape, topLevelMember, mapModel.getValueModel()); + validateMember(model, shape, topLevelMember, mapModel.getKeyModel(), entries, reportedKeys); + validateMember(model, shape, topLevelMember, mapModel.getValueModel(), entries, reportedKeys); } else if (!member.isSimple()) { - requireResolvable(model, shape, topLevelMember, member, member.getVariable().getSimpleType()); + requireResolvable(model, shape, topLevelMember, member, member.getVariable().getSimpleType(), entries, reportedKeys); } } private static void requireResolvable(IntermediateModel model, ShapeModel shape, MemberModel topLevelMember, - MemberModel member, String targetName) { + MemberModel member, String targetName, + List entries, Set reportedKeys) { // linkMembersToShapes only links members declared directly on a shape, so a list/map element member always carries a // null shape; resolve its target by name (matching shape retention) instead of reading the unset getShape(). boolean resolved = member == topLevelMember ? member.getShape() != null : model.getShapes().containsKey(targetName); if (!resolved) { - throw dangling(shape, topLevelMember, targetName); + recordDangling(shape, topLevelMember, targetName, entries, reportedKeys); } } - private static ModelInvalidException dangling(ShapeModel shape, MemberModel member, String targetName) { + private static void recordDangling(ShapeModel shape, MemberModel member, String targetName, + List entries, Set reportedKeys) { + String dedupKey = shape.getShapeName() + '|' + member.getC2jName() + '|' + targetName; + if (!reportedKeys.add(dedupKey)) { + return; + } String detail = String.format( "Member '%s' of shape '%s' targets shape '%s' which does not exist in the intermediate model. The target shape " + "may be missing from the service model, removed by a customization, or misspelled.", member.getC2jName(), shape.getShapeName(), targetName); - return ModelInvalidException.fromEntry( - ValidationEntry.create(ValidationErrorId.UNKNOWN_SHAPE_MEMBER, ValidationErrorSeverity.DANGER, detail)); + entries.add(ValidationEntry.create(ValidationErrorId.UNKNOWN_SHAPE_MEMBER, ValidationErrorSeverity.DANGER, detail)); } } diff --git a/codegen/src/test/java/software/amazon/awssdk/codegen/validation/MemberShapeTargetValidatorTest.java b/codegen/src/test/java/software/amazon/awssdk/codegen/validation/MemberShapeTargetValidatorTest.java index 90809af8bef4..188b993d4f44 100644 --- a/codegen/src/test/java/software/amazon/awssdk/codegen/validation/MemberShapeTargetValidatorTest.java +++ b/codegen/src/test/java/software/amazon/awssdk/codegen/validation/MemberShapeTargetValidatorTest.java @@ -24,6 +24,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.stream.Collectors; import org.junit.jupiter.api.Test; import software.amazon.awssdk.codegen.model.intermediate.IntermediateModel; import software.amazon.awssdk.codegen.model.intermediate.ListModel; @@ -118,6 +119,95 @@ void validate_mapValueWithDanglingTarget_throwsIdentifyingContainerMember() { .hasMessageContaining("MissingValue"); } + @Test + void validate_multipleDanglingMembers_aggregatesAllIntoSingleException() { + MemberModel enumMember = new MemberModel() + .withC2jName("AuthorizationDetails") + .withC2jShape("AuthDetailType") + .withVariable(new VariableModel("authorizationDetails", "String")) + .withEnumType("AuthDetailType"); + ShapeModel enumOwner = shape("ResponseShape"); + enumOwner.setMembers(Arrays.asList(enumMember)); + + MemberModel structureMember = new MemberModel() + .withC2jName("Nested") + .withC2jShape("MissingStruct") + .withVariable(new VariableModel("nested", "MissingStruct")); + ShapeModel structOwner = shape("RequestShape"); + structOwner.setMembers(Arrays.asList(structureMember)); + + Map shapes = new HashMap<>(); + shapes.put("ResponseShape", enumOwner); + shapes.put("RequestShape", structOwner); + IntermediateModel model = new IntermediateModel(); + model.setShapes(shapes); + + assertThatThrownBy(() -> MemberShapeTargetValidator.validate(model)) + .isInstanceOf(ModelInvalidException.class) + .matches(t -> allUnknownShapeMemberDanger(t, 2), "two UNKNOWN_SHAPE_MEMBER / DANGER entries") + .matches(t -> entryMessages(t).stream().anyMatch(m -> m.contains("ResponseShape") + && m.contains("AuthorizationDetails") + && m.contains("AuthDetailType")), + "entry for the enum member names its own shape/member/target") + .matches(t -> entryMessages(t).stream().anyMatch(m -> m.contains("RequestShape") + && m.contains("Nested") + && m.contains("MissingStruct")), + "entry for the structure member names its own shape/member/target"); + } + + @Test + void validate_mapKeyAndValueBothDangling_reportsDistinctEntryPerTarget() { + MemberModel keyMember = new MemberModel() + .withC2jName("key") + .withC2jShape("MissingKey") + .withVariable(new VariableModel("key", "MissingKey")); + MemberModel valueMember = new MemberModel() + .withC2jName("value") + .withC2jShape("MissingValue") + .withVariable(new VariableModel("value", "MissingValue")); + MemberModel mapMember = new MemberModel() + .withC2jName("Attributes") + .withC2jShape("AttributeMap") + .withVariable(new VariableModel("attributes", "java.util.Map")) + .withMapModel(new MapModel("java.util.HashMap", "java.util.Map", "key", keyMember, "value", valueMember)); + IntermediateModel model = modelWithShape("MapContainerShape", mapMember); + + assertThatThrownBy(() -> MemberShapeTargetValidator.validate(model)) + .isInstanceOf(ModelInvalidException.class) + .matches(t -> allUnknownShapeMemberDanger(t, 2), "two UNKNOWN_SHAPE_MEMBER / DANGER entries") + .matches(t -> entryMessages(t).stream().anyMatch(m -> m.contains("MissingKey")), "key target reported") + .matches(t -> entryMessages(t).stream().anyMatch(m -> m.contains("MissingValue")), "value target reported"); + } + + @Test + void validate_sameTargetReachableTwice_reportedOnce() { + MemberModel innerKey = new MemberModel() + .withC2jName("key") + .withC2jShape("MissingShared") + .withVariable(new VariableModel("key", "MissingShared")); + MemberModel innerValue = new MemberModel() + .withC2jName("value") + .withC2jShape("MissingShared") + .withVariable(new VariableModel("value", "MissingShared")); + MemberModel mapElement = new MemberModel() + .withC2jName("member") + .withC2jShape("SharedMap") + .withVariable(new VariableModel("member", "java.util.Map")) + .withMapModel(new MapModel("java.util.HashMap", "java.util.Map", "key", innerKey, "value", innerValue)); + MemberModel listMember = new MemberModel() + .withC2jName("Items") + .withC2jShape("ItemList") + .withVariable(new VariableModel("items", "java.util.List")) + .withListModel(new ListModel("java.util.Map", null, "java.util.ArrayList", "java.util.List", mapElement)); + IntermediateModel model = modelWithShape("ListOfMapShape", listMember); + + assertThatThrownBy(() -> MemberShapeTargetValidator.validate(model)) + .isInstanceOf(ModelInvalidException.class) + .matches(t -> allUnknownShapeMemberDanger(t, 1), "single deduped UNKNOWN_SHAPE_MEMBER / DANGER entry") + .matches(t -> entryMessages(t).get(0).contains("Items") && entryMessages(t).get(0).contains("MissingShared"), + "the single entry names the container member and shared target"); + } + @Test void validate_scalarMembersWithNullShape_doesNotThrow() { MemberModel scalarMember = new MemberModel() @@ -187,9 +277,19 @@ private static ShapeModel shape(String name) { } private static boolean isUnknownShapeMemberDanger(Throwable t) { + return allUnknownShapeMemberDanger(t, 1); + } + + private static boolean allUnknownShapeMemberDanger(Throwable t, int expectedCount) { List entries = ((ModelInvalidException) t).validationEntries(); - return entries.size() == 1 - && entries.get(0).getErrorId() == ValidationErrorId.UNKNOWN_SHAPE_MEMBER - && entries.get(0).getSeverity() == ValidationErrorSeverity.DANGER; + return entries.size() == expectedCount + && entries.stream().allMatch(e -> e.getErrorId() == ValidationErrorId.UNKNOWN_SHAPE_MEMBER + && e.getSeverity() == ValidationErrorSeverity.DANGER); + } + + private static List entryMessages(Throwable t) { + return ((ModelInvalidException) t).validationEntries().stream() + .map(ValidationEntry::getDetailMessage) + .collect(Collectors.toList()); } }