Skip to content

Commit b1391cc

Browse files
zoewanggzoewangg
committed
Aggregate all unresolved member shape targets into a single validation error
1 parent 3b6b861 commit b1391cc

2 files changed

Lines changed: 132 additions & 17 deletions

File tree

‎codegen/src/main/java/software/amazon/awssdk/codegen/validation/MemberShapeTargetValidator.java‎

Lines changed: 29 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,10 @@
1515

1616
package software.amazon.awssdk.codegen.validation;
1717

18+
import java.util.ArrayList;
19+
import java.util.HashSet;
20+
import java.util.List;
21+
import java.util.Set;
1822
import software.amazon.awssdk.codegen.model.intermediate.IntermediateModel;
1923
import software.amazon.awssdk.codegen.model.intermediate.MapModel;
2024
import software.amazon.awssdk.codegen.model.intermediate.MemberModel;
@@ -35,62 +39,73 @@ private MemberShapeTargetValidator() {
3539
}
3640

3741
/**
38-
* Fails fast on the first member whose target shape cannot be resolved.
42+
* Collects every member whose target shape cannot be resolved and, if any are found, throws a single
43+
* {@link ModelInvalidException} carrying one entry per distinct offending member.
3944
*
40-
* @throws ModelInvalidException if a member references a shape that does not exist in the model.
45+
* @throws ModelInvalidException if any member references a shape that does not exist in the model.
4146
*/
4247
public static void validate(IntermediateModel model) {
48+
List<ValidationEntry> entries = new ArrayList<>();
49+
Set<String> reportedKeys = new HashSet<>();
4350
for (ShapeModel shape : model.getShapes().values()) {
4451
if (shape.getMembers() == null) {
4552
continue;
4653
}
4754
for (MemberModel member : shape.getMembers()) {
48-
validateMember(model, shape, member, member);
55+
validateMember(model, shape, member, member, entries, reportedKeys);
4956
}
5057
}
58+
if (!entries.isEmpty()) {
59+
throw ModelInvalidException.builder().validationEntries(entries).build();
60+
}
5161
}
5262

5363
/**
5464
* @param topLevelMember the member declared directly on the shape; for list/map element members this stays pointed at the
5565
* container member so the error message identifies a member the service team can locate.
5666
*/
5767
private static void validateMember(IntermediateModel model, ShapeModel shape, MemberModel topLevelMember,
58-
MemberModel member) {
68+
MemberModel member, List<ValidationEntry> entries, Set<String> reportedKeys) {
5969
if (member == null) {
6070
return;
6171
}
6272

6373
if (member.getEnumType() != null) {
64-
requireResolvable(model, shape, topLevelMember, member, member.getEnumType());
74+
requireResolvable(model, shape, topLevelMember, member, member.getEnumType(), entries, reportedKeys);
6575
} else if (member.isList()) {
66-
validateMember(model, shape, topLevelMember, member.getListModel().getListMemberModel());
76+
validateMember(model, shape, topLevelMember, member.getListModel().getListMemberModel(), entries, reportedKeys);
6777
} else if (member.isMap()) {
6878
MapModel mapModel = member.getMapModel();
69-
validateMember(model, shape, topLevelMember, mapModel.getKeyModel());
70-
validateMember(model, shape, topLevelMember, mapModel.getValueModel());
79+
validateMember(model, shape, topLevelMember, mapModel.getKeyModel(), entries, reportedKeys);
80+
validateMember(model, shape, topLevelMember, mapModel.getValueModel(), entries, reportedKeys);
7181
} else if (!member.isSimple()) {
72-
requireResolvable(model, shape, topLevelMember, member, member.getVariable().getSimpleType());
82+
requireResolvable(model, shape, topLevelMember, member, member.getVariable().getSimpleType(), entries, reportedKeys);
7383
}
7484
}
7585

7686
private static void requireResolvable(IntermediateModel model, ShapeModel shape, MemberModel topLevelMember,
77-
MemberModel member, String targetName) {
87+
MemberModel member, String targetName,
88+
List<ValidationEntry> entries, Set<String> reportedKeys) {
7889
// linkMembersToShapes only links members declared directly on a shape, so a list/map element member always carries a
7990
// null shape; resolve its target by name (matching shape retention) instead of reading the unset getShape().
8091
boolean resolved = member == topLevelMember ? member.getShape() != null
8192
: model.getShapes().containsKey(targetName);
8293
if (!resolved) {
83-
throw dangling(shape, topLevelMember, targetName);
94+
recordDangling(shape, topLevelMember, targetName, entries, reportedKeys);
8495
}
8596
}
8697

87-
private static ModelInvalidException dangling(ShapeModel shape, MemberModel member, String targetName) {
98+
private static void recordDangling(ShapeModel shape, MemberModel member, String targetName,
99+
List<ValidationEntry> entries, Set<String> reportedKeys) {
100+
String dedupKey = shape.getShapeName() + '|' + member.getC2jName() + '|' + targetName;
101+
if (!reportedKeys.add(dedupKey)) {
102+
return;
103+
}
88104
String detail = String.format(
89105
"Member '%s' of shape '%s' targets shape '%s' which does not exist in the intermediate model. The target shape "
90106
+ "may be missing from the service model, removed by a customization, or misspelled.",
91107
member.getC2jName(), shape.getShapeName(), targetName);
92108

93-
return ModelInvalidException.fromEntry(
94-
ValidationEntry.create(ValidationErrorId.UNKNOWN_SHAPE_MEMBER, ValidationErrorSeverity.DANGER, detail));
109+
entries.add(ValidationEntry.create(ValidationErrorId.UNKNOWN_SHAPE_MEMBER, ValidationErrorSeverity.DANGER, detail));
95110
}
96111
}

‎codegen/src/test/java/software/amazon/awssdk/codegen/validation/MemberShapeTargetValidatorTest.java‎

Lines changed: 103 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import java.util.HashMap;
2525
import java.util.List;
2626
import java.util.Map;
27+
import java.util.stream.Collectors;
2728
import org.junit.jupiter.api.Test;
2829
import software.amazon.awssdk.codegen.model.intermediate.IntermediateModel;
2930
import software.amazon.awssdk.codegen.model.intermediate.ListModel;
@@ -118,6 +119,95 @@ void validate_mapValueWithDanglingTarget_throwsIdentifyingContainerMember() {
118119
.hasMessageContaining("MissingValue");
119120
}
120121

122+
@Test
123+
void validate_multipleDanglingMembers_aggregatesAllIntoSingleException() {
124+
MemberModel enumMember = new MemberModel()
125+
.withC2jName("AuthorizationDetails")
126+
.withC2jShape("AuthDetailType")
127+
.withVariable(new VariableModel("authorizationDetails", "String"))
128+
.withEnumType("AuthDetailType");
129+
ShapeModel enumOwner = shape("ResponseShape");
130+
enumOwner.setMembers(Arrays.asList(enumMember));
131+
132+
MemberModel structureMember = new MemberModel()
133+
.withC2jName("Nested")
134+
.withC2jShape("MissingStruct")
135+
.withVariable(new VariableModel("nested", "MissingStruct"));
136+
ShapeModel structOwner = shape("RequestShape");
137+
structOwner.setMembers(Arrays.asList(structureMember));
138+
139+
Map<String, ShapeModel> shapes = new HashMap<>();
140+
shapes.put("ResponseShape", enumOwner);
141+
shapes.put("RequestShape", structOwner);
142+
IntermediateModel model = new IntermediateModel();
143+
model.setShapes(shapes);
144+
145+
assertThatThrownBy(() -> MemberShapeTargetValidator.validate(model))
146+
.isInstanceOf(ModelInvalidException.class)
147+
.matches(t -> allUnknownShapeMemberDanger(t, 2), "two UNKNOWN_SHAPE_MEMBER / DANGER entries")
148+
.matches(t -> entryMessages(t).stream().anyMatch(m -> m.contains("ResponseShape")
149+
&& m.contains("AuthorizationDetails")
150+
&& m.contains("AuthDetailType")),
151+
"entry for the enum member names its own shape/member/target")
152+
.matches(t -> entryMessages(t).stream().anyMatch(m -> m.contains("RequestShape")
153+
&& m.contains("Nested")
154+
&& m.contains("MissingStruct")),
155+
"entry for the structure member names its own shape/member/target");
156+
}
157+
158+
@Test
159+
void validate_mapKeyAndValueBothDangling_reportsDistinctEntryPerTarget() {
160+
MemberModel keyMember = new MemberModel()
161+
.withC2jName("key")
162+
.withC2jShape("MissingKey")
163+
.withVariable(new VariableModel("key", "MissingKey"));
164+
MemberModel valueMember = new MemberModel()
165+
.withC2jName("value")
166+
.withC2jShape("MissingValue")
167+
.withVariable(new VariableModel("value", "MissingValue"));
168+
MemberModel mapMember = new MemberModel()
169+
.withC2jName("Attributes")
170+
.withC2jShape("AttributeMap")
171+
.withVariable(new VariableModel("attributes", "java.util.Map"))
172+
.withMapModel(new MapModel("java.util.HashMap", "java.util.Map", "key", keyMember, "value", valueMember));
173+
IntermediateModel model = modelWithShape("MapContainerShape", mapMember);
174+
175+
assertThatThrownBy(() -> MemberShapeTargetValidator.validate(model))
176+
.isInstanceOf(ModelInvalidException.class)
177+
.matches(t -> allUnknownShapeMemberDanger(t, 2), "two UNKNOWN_SHAPE_MEMBER / DANGER entries")
178+
.matches(t -> entryMessages(t).stream().anyMatch(m -> m.contains("MissingKey")), "key target reported")
179+
.matches(t -> entryMessages(t).stream().anyMatch(m -> m.contains("MissingValue")), "value target reported");
180+
}
181+
182+
@Test
183+
void validate_sameTargetReachableTwice_reportedOnce() {
184+
MemberModel innerKey = new MemberModel()
185+
.withC2jName("key")
186+
.withC2jShape("MissingShared")
187+
.withVariable(new VariableModel("key", "MissingShared"));
188+
MemberModel innerValue = new MemberModel()
189+
.withC2jName("value")
190+
.withC2jShape("MissingShared")
191+
.withVariable(new VariableModel("value", "MissingShared"));
192+
MemberModel mapElement = new MemberModel()
193+
.withC2jName("member")
194+
.withC2jShape("SharedMap")
195+
.withVariable(new VariableModel("member", "java.util.Map"))
196+
.withMapModel(new MapModel("java.util.HashMap", "java.util.Map", "key", innerKey, "value", innerValue));
197+
MemberModel listMember = new MemberModel()
198+
.withC2jName("Items")
199+
.withC2jShape("ItemList")
200+
.withVariable(new VariableModel("items", "java.util.List"))
201+
.withListModel(new ListModel("java.util.Map", null, "java.util.ArrayList", "java.util.List", mapElement));
202+
IntermediateModel model = modelWithShape("ListOfMapShape", listMember);
203+
204+
assertThatThrownBy(() -> MemberShapeTargetValidator.validate(model))
205+
.isInstanceOf(ModelInvalidException.class)
206+
.matches(t -> allUnknownShapeMemberDanger(t, 1), "single deduped UNKNOWN_SHAPE_MEMBER / DANGER entry")
207+
.matches(t -> entryMessages(t).get(0).contains("Items") && entryMessages(t).get(0).contains("MissingShared"),
208+
"the single entry names the container member and shared target");
209+
}
210+
121211
@Test
122212
void validate_scalarMembersWithNullShape_doesNotThrow() {
123213
MemberModel scalarMember = new MemberModel()
@@ -187,9 +277,19 @@ private static ShapeModel shape(String name) {
187277
}
188278

189279
private static boolean isUnknownShapeMemberDanger(Throwable t) {
280+
return allUnknownShapeMemberDanger(t, 1);
281+
}
282+
283+
private static boolean allUnknownShapeMemberDanger(Throwable t, int expectedCount) {
190284
List<ValidationEntry> entries = ((ModelInvalidException) t).validationEntries();
191-
return entries.size() == 1
192-
&& entries.get(0).getErrorId() == ValidationErrorId.UNKNOWN_SHAPE_MEMBER
193-
&& entries.get(0).getSeverity() == ValidationErrorSeverity.DANGER;
285+
return entries.size() == expectedCount
286+
&& entries.stream().allMatch(e -> e.getErrorId() == ValidationErrorId.UNKNOWN_SHAPE_MEMBER
287+
&& e.getSeverity() == ValidationErrorSeverity.DANGER);
288+
}
289+
290+
private static List<String> entryMessages(Throwable t) {
291+
return ((ModelInvalidException) t).validationEntries().stream()
292+
.map(ValidationEntry::getDetailMessage)
293+
.collect(Collectors.toList());
194294
}
195295
}

0 commit comments

Comments
 (0)