Skip to content

Commit f669ab6

Browse files
alextwoodsalextwoods
committed
Validate paginators before generating
1 parent f9de18c commit f669ab6

10 files changed

Lines changed: 585 additions & 27 deletions

File tree

‎.changes/next-release/bugfix-AWSSDKforJavav2Codegen-c916e7f.json‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"type": "bugfix",
3+
"category": "AWS SDK for Java v2 Codegen\"",
4+
"contributor": "",
5+
"description": "Validate paginators before generating"
6+
}

‎codegen/src/main/java/software/amazon/awssdk/codegen/AddOperations.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,6 @@ private Integer getHttpStatusCode(ErrorTrait errorTrait) {
278278
}
279279

280280
private boolean isPaginated(Operation op) {
281-
return paginators.containsKey(op.getName()) && paginators.get(op.getName()).isValid();
281+
return paginators.containsKey(op.getName()) && paginators.get(op.getName()).hasAllRequiredFields();
282282
}
283283
}

‎codegen/src/main/java/software/amazon/awssdk/codegen/CodeGenerator.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import software.amazon.awssdk.codegen.validation.ModelValidationContext;
3535
import software.amazon.awssdk.codegen.validation.ModelValidationReport;
3636
import software.amazon.awssdk.codegen.validation.ModelValidator;
37+
import software.amazon.awssdk.codegen.validation.PaginatorDefinitionValidator;
3738
import software.amazon.awssdk.codegen.validation.SharedModelsValidator;
3839
import software.amazon.awssdk.codegen.validation.ValidationEntry;
3940
import software.amazon.awssdk.utils.Logger;
@@ -44,7 +45,8 @@ public class CodeGenerator {
4445

4546
private static final List<ModelValidator> DEFAULT_MODEL_VALIDATORS = Arrays.asList(
4647
new SharedModelsValidator(),
47-
new CustomizationConfigValidator()
48+
new CustomizationConfigValidator(),
49+
new PaginatorDefinitionValidator()
4850
);
4951

5052
private final C2jModels c2jModels;

‎codegen/src/main/java/software/amazon/awssdk/codegen/emitters/tasks/PaginatorsGeneratorTasks.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ protected boolean hasTasks() {
5353
@Override
5454
protected List<GeneratorTask> createTasks() throws Exception {
5555
return model.getPaginators().entrySet().stream()
56-
.filter(entry -> entry.getValue().isValid())
56+
.filter(entry -> entry.getValue().hasAllRequiredFields())
5757
.flatMap(safeFunction(this::createSyncAndAsyncTasks))
5858
.collect(Collectors.toList());
5959
}

‎codegen/src/main/java/software/amazon/awssdk/codegen/model/service/PaginatorDefinition.java‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -109,12 +109,14 @@ public void setLimitKey(String limitKey) {
109109
}
110110

111111
/**
112-
* Returns a boolean value indicating if the information present in this object
113-
* is sufficient to generate the paginated APIs.
112+
* Returns a boolean value indicating if the required fields are present in this paginator definition.
113+
* This does not fully validate the paginator against the service model (e.g. it does not check that referenced
114+
* members actually exist in the operation's request/response shapes). It only checks that the minimum required
115+
* fields (inputToken, outputToken) are present and well-formed.
114116
*
115-
* @return True if all necessary information to generate paginator APIs is present. Otherwise false.
117+
* @return True if the minimum required fields to generate paginator APIs are present. Otherwise false.
116118
*/
117-
public boolean isValid() {
119+
public boolean hasAllRequiredFields() {
118120
Pattern p = Pattern.compile(VALID_REGEX);
119121

120122
return !CollectionUtils.isNullOrEmpty(inputToken) &&

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

Lines changed: 204 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,204 @@
1+
/*
2+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License").
5+
* You may not use this file except in compliance with the License.
6+
* A copy of the License is located at
7+
*
8+
* http://aws.amazon.com/apache2.0
9+
*
10+
* or in the "license" file accompanying this file. This file is distributed
11+
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
12+
* express or implied. See the License for the specific language governing
13+
* permissions and limitations under the License.
14+
*/
15+
16+
package software.amazon.awssdk.codegen.validation;
17+
18+
import java.util.ArrayList;
19+
import java.util.List;
20+
import java.util.Map;
21+
import software.amazon.awssdk.codegen.model.intermediate.IntermediateModel;
22+
import software.amazon.awssdk.codegen.model.intermediate.MemberModel;
23+
import software.amazon.awssdk.codegen.model.intermediate.OperationModel;
24+
import software.amazon.awssdk.codegen.model.intermediate.ShapeModel;
25+
import software.amazon.awssdk.codegen.model.service.PaginatorDefinition;
26+
27+
/**
28+
* Validates paginator definitions against the service model. Checks that:
29+
* <ul>
30+
* <li>Each paginator references an operation that exists in the model</li>
31+
* <li>Input tokens reference valid members in the request shape</li>
32+
* <li>Output tokens reference valid members in the response shape</li>
33+
* <li>Result keys reference valid members in the response shape</li>
34+
* <li>The more_results field, if specified, references a valid member in the response shape</li>
35+
* <li>The limit_key field, if specified, references a valid member in the request shape</li>
36+
* </ul>
37+
*/
38+
public final class PaginatorDefinitionValidator implements ModelValidator {
39+
40+
@Override
41+
public List<ValidationEntry> validateModels(ModelValidationContext context) {
42+
List<ValidationEntry> entries = new ArrayList<>();
43+
IntermediateModel model = context.intermediateModel();
44+
Map<String, PaginatorDefinition> paginators = model.getPaginators();
45+
46+
if (paginators == null || paginators.isEmpty()) {
47+
return entries;
48+
}
49+
50+
for (Map.Entry<String, PaginatorDefinition> entry : paginators.entrySet()) {
51+
String operationName = entry.getKey();
52+
PaginatorDefinition definition = entry.getValue();
53+
54+
// Skip paginators that don't have the minimum required fields (inputToken, outputToken).
55+
// This is legacy behavior: codegen has historically silently skipped these incomplete definitions
56+
// rather than generating code for them. We mirror that filter here so we only validate paginators
57+
// that will actually be used during code generation.
58+
if (!definition.hasAllRequiredFields()) {
59+
continue;
60+
}
61+
62+
OperationModel operationModel = model.getOperation(operationName);
63+
if (operationModel == null) {
64+
entries.add(ValidationEntry.create(
65+
ValidationErrorId.UNKNOWN_OPERATION,
66+
ValidationErrorSeverity.DANGER,
67+
String.format("Invalid paginator definition - The service model does not contain the "
68+
+ "referenced operation '%s'", operationName)
69+
));
70+
continue;
71+
}
72+
73+
ShapeModel inputShape = operationModel.getInputShape();
74+
ShapeModel outputShape = operationModel.getOutputShape();
75+
76+
validateInputTokens(entries, operationName, definition, inputShape);
77+
validateOutputTokens(entries, operationName, definition, outputShape);
78+
validateResultKeys(entries, operationName, definition, outputShape);
79+
validateMoreResults(entries, operationName, definition, outputShape);
80+
validateLimitKey(entries, operationName, definition, inputShape);
81+
}
82+
83+
return entries;
84+
}
85+
86+
private void validateInputTokens(List<ValidationEntry> entries, String operationName,
87+
PaginatorDefinition definition, ShapeModel inputShape) {
88+
if (definition.getInputToken() == null || inputShape == null) {
89+
return;
90+
}
91+
92+
for (String inputToken : definition.getInputToken()) {
93+
if (!isValidMemberPath(inputShape, inputToken)) {
94+
entries.add(ValidationEntry.create(
95+
ValidationErrorId.INVALID_PAGINATOR_DEFINITION,
96+
ValidationErrorSeverity.DANGER,
97+
String.format("Paginator for operation '%s' references input_token '%s' which does not "
98+
+ "exist in the request shape '%s'", operationName, inputToken,
99+
inputShape.getShapeName())
100+
));
101+
}
102+
}
103+
}
104+
105+
private void validateOutputTokens(List<ValidationEntry> entries, String operationName,
106+
PaginatorDefinition definition, ShapeModel outputShape) {
107+
if (definition.getOutputToken() == null || outputShape == null) {
108+
return;
109+
}
110+
111+
for (String outputToken : definition.getOutputToken()) {
112+
if (!isValidMemberPath(outputShape, outputToken)) {
113+
entries.add(ValidationEntry.create(
114+
ValidationErrorId.INVALID_PAGINATOR_DEFINITION,
115+
ValidationErrorSeverity.DANGER,
116+
String.format("Paginator for operation '%s' references output_token '%s' which does not "
117+
+ "exist in the response shape '%s'", operationName, outputToken,
118+
outputShape.getShapeName())
119+
));
120+
}
121+
}
122+
}
123+
124+
private void validateResultKeys(List<ValidationEntry> entries, String operationName,
125+
PaginatorDefinition definition, ShapeModel outputShape) {
126+
if (definition.getResultKey() == null || outputShape == null) {
127+
return;
128+
}
129+
130+
for (String resultKey : definition.getResultKey()) {
131+
if (!isValidMemberPath(outputShape, resultKey)) {
132+
entries.add(ValidationEntry.create(
133+
ValidationErrorId.INVALID_PAGINATOR_DEFINITION,
134+
ValidationErrorSeverity.DANGER,
135+
String.format("Paginator for operation '%s' references result_key '%s' which does not "
136+
+ "exist in the response shape '%s'", operationName, resultKey,
137+
outputShape.getShapeName())
138+
));
139+
}
140+
}
141+
}
142+
143+
private void validateMoreResults(List<ValidationEntry> entries, String operationName,
144+
PaginatorDefinition definition, ShapeModel outputShape) {
145+
if (definition.getMoreResults() == null || outputShape == null) {
146+
return;
147+
}
148+
149+
if (!isValidMemberPath(outputShape, definition.getMoreResults())) {
150+
entries.add(ValidationEntry.create(
151+
ValidationErrorId.INVALID_PAGINATOR_DEFINITION,
152+
ValidationErrorSeverity.DANGER,
153+
String.format("Paginator for operation '%s' references more_results '%s' which does not "
154+
+ "exist in the response shape '%s'", operationName, definition.getMoreResults(),
155+
outputShape.getShapeName())
156+
));
157+
}
158+
}
159+
160+
private void validateLimitKey(List<ValidationEntry> entries, String operationName,
161+
PaginatorDefinition definition, ShapeModel inputShape) {
162+
if (definition.getLimitKey() == null || inputShape == null) {
163+
return;
164+
}
165+
166+
if (!isValidMemberPath(inputShape, definition.getLimitKey())) {
167+
entries.add(ValidationEntry.create(
168+
ValidationErrorId.INVALID_PAGINATOR_DEFINITION,
169+
ValidationErrorSeverity.DANGER,
170+
String.format("Paginator for operation '%s' references limit_key '%s' which does not "
171+
+ "exist in the request shape '%s'", operationName, definition.getLimitKey(),
172+
inputShape.getShapeName())
173+
));
174+
}
175+
}
176+
177+
/**
178+
* Validates that a dotted member path (e.g. "StreamDescription.Shards") resolves to a valid member
179+
* in the given shape hierarchy.
180+
*/
181+
private boolean isValidMemberPath(ShapeModel shape, String memberPath) {
182+
if (shape == null || memberPath == null || memberPath.isEmpty()) {
183+
return false;
184+
}
185+
186+
String[] segments = memberPath.split("\\.");
187+
ShapeModel currentShape = shape;
188+
189+
for (int i = 0; i < segments.length; i++) {
190+
MemberModel member = currentShape.getMemberByC2jName(segments[i]);
191+
if (member == null) {
192+
return false;
193+
}
194+
if (i < segments.length - 1) {
195+
currentShape = member.getShape();
196+
if (currentShape == null) {
197+
return false;
198+
}
199+
}
200+
}
201+
202+
return true;
203+
}
204+
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ public enum ValidationErrorId {
2626
INVALID_CODEGEN_CUSTOMIZATION("A customization is enabled for this service that cannot be applied for the given service "
2727
+ "model."),
2828
UNKNOWN_OPERATION("The model references an unknown operation."),
29+
INVALID_PAGINATOR_DEFINITION("The paginator definition references members that do not exist in the model."),
2930
INVALID_IDENTIFIER_NAME("The model contains an invalid or non-idiomatic name or identifier."),
3031
SHARED_EVENTSTREAM_EVENT("Event shape shared between multiple EventStreams. "
3132
+ "Missing duplicateAndRenameSharedEvents customization.");

‎codegen/src/test/java/software/amazon/awssdk/codegen/model/service/PaginatorDefinitionTest.java‎

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -26,78 +26,78 @@
2626
public class PaginatorDefinitionTest {
2727

2828
@Test
29-
public void isValid_ReturnsFalse_WhenInputToken_IsNullOrEmpty() {
29+
public void hasAllRequiredFields_ReturnsFalse_WhenInputToken_IsNullOrEmpty() {
3030
PaginatorDefinition paginatorDefinition = new PaginatorDefinition();
3131

3232
assertNull(paginatorDefinition.getInputToken());
33-
assertFalse(paginatorDefinition.isValid());
33+
assertFalse(paginatorDefinition.hasAllRequiredFields());
3434

3535
paginatorDefinition.setInputToken(Collections.emptyList());
36-
assertFalse(paginatorDefinition.isValid());
36+
assertFalse(paginatorDefinition.hasAllRequiredFields());
3737
}
3838

3939
@Test
40-
public void isValid_ReturnsFalse_WhenOutputToken_IsNullOrEmpty() {
40+
public void hasAllRequiredFields_ReturnsFalse_WhenOutputToken_IsNullOrEmpty() {
4141
PaginatorDefinition paginatorDefinition = new PaginatorDefinition();
4242

4343
assertNull(paginatorDefinition.getOutputToken());
44-
assertFalse(paginatorDefinition.isValid());
44+
assertFalse(paginatorDefinition.hasAllRequiredFields());
4545

4646
paginatorDefinition.setOutputToken(Collections.emptyList());
47-
assertFalse(paginatorDefinition.isValid());
47+
assertFalse(paginatorDefinition.hasAllRequiredFields());
4848
}
4949

5050
@Test
51-
public void isValid_ReturnsFalse_WhenInputTokenIsPresent_AndOutputTokenIsMissing() {
51+
public void hasAllRequiredFields_ReturnsFalse_WhenInputTokenIsPresent_AndOutputTokenIsMissing() {
5252
PaginatorDefinition paginatorDefinition = new PaginatorDefinition();
5353
paginatorDefinition.setInputToken(Arrays.asList("inputToken"));
5454

55-
assertFalse(paginatorDefinition.isValid());
55+
assertFalse(paginatorDefinition.hasAllRequiredFields());
5656
}
5757

5858
@Test
59-
public void isValid_ReturnsTrue_WhenBothInputTokenAndOutputToken_ArePresent() {
59+
public void hasAllRequiredFields_ReturnsTrue_WhenBothInputTokenAndOutputToken_ArePresent() {
6060
PaginatorDefinition paginatorDefinition = new PaginatorDefinition();
6161
paginatorDefinition.setInputToken(Arrays.asList("inputToken"));
6262
paginatorDefinition.setOutputToken(Arrays.asList("foo", "bar"));
6363

64-
assertTrue(paginatorDefinition.isValid());
64+
assertTrue(paginatorDefinition.hasAllRequiredFields());
6565
}
6666

6767
@Test
68-
public void isValid_ReturnsTrue_WhenResultKey_IsNull() {
68+
public void hasAllRequiredFields_ReturnsTrue_WhenResultKey_IsNull() {
6969
PaginatorDefinition paginatorDefinition = new PaginatorDefinition();
7070
paginatorDefinition.setInputToken(Arrays.asList("inputToken"));
7171
paginatorDefinition.setOutputToken(Arrays.asList("foo", "bar"));
7272

7373
assertNull(paginatorDefinition.getResultKey());
74-
assertTrue(paginatorDefinition.isValid());
74+
assertTrue(paginatorDefinition.hasAllRequiredFields());
7575
}
7676

7777
@Test
78-
public void isValid_ReturnsTrue_WhenOutputTokenList_ContainsValidString() {
78+
public void hasAllRequiredFields_ReturnsTrue_WhenOutputTokenList_ContainsValidString() {
7979
PaginatorDefinition paginatorDefinition = new PaginatorDefinition();
8080
paginatorDefinition.setOutputToken(Arrays.asList("Foo", "Foo.Bar"));
8181
paginatorDefinition.setInputToken(Arrays.asList("token"));
8282

83-
assertTrue(paginatorDefinition.isValid());
83+
assertTrue(paginatorDefinition.hasAllRequiredFields());
8484
}
8585

8686
@Test
87-
public void isValid_ReturnsFalse_WhenOutputTokenList_ContainsAListMember() {
87+
public void hasAllRequiredFields_ReturnsFalse_WhenOutputTokenList_ContainsAListMember() {
8888
PaginatorDefinition paginatorDefinition = new PaginatorDefinition();
8989
paginatorDefinition.setOutputToken(Arrays.asList("Contents[-1]"));
9090
paginatorDefinition.setInputToken(Arrays.asList("token"));
9191

92-
assertFalse(paginatorDefinition.isValid());
92+
assertFalse(paginatorDefinition.hasAllRequiredFields());
9393
}
9494

9595
@Test
96-
public void isValid_ReturnsTrue_WhenOutputTokenList_ContainsOrCharacter() {
96+
public void hasAllRequiredFields_ReturnsTrue_WhenOutputTokenList_ContainsOrCharacter() {
9797
PaginatorDefinition paginatorDefinition = new PaginatorDefinition();
9898
paginatorDefinition.setOutputToken(Arrays.asList("NextMarker || Contents"));
9999
paginatorDefinition.setInputToken(Arrays.asList("token"));
100100

101-
assertFalse(paginatorDefinition.isValid());
101+
assertFalse(paginatorDefinition.hasAllRequiredFields());
102102
}
103103
}

‎codegen/src/test/java/software/amazon/awssdk/codegen/poet/paginators/PaginatedResponseClassSpecTest.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ private static Paginators getPaginatorsModel(File file) {
7171
public void testGeneratedResponseForSyncOperations() {
7272
paginators.getPagination().entrySet()
7373
.stream()
74-
.filter(entry -> entry.getValue().isValid())
74+
.filter(entry -> entry.getValue().hasAllRequiredFields())
7575
.forEach(entry ->
7676
{
7777
ClassSpec classSpec = getSyncResponseSpec(entry);
@@ -83,7 +83,7 @@ public void testGeneratedResponseForSyncOperations() {
8383
public void testGeneratedResponseForAsyncOperations() {
8484
paginators.getPagination().entrySet()
8585
.stream()
86-
.filter(entry -> entry.getValue().isValid())
86+
.filter(entry -> entry.getValue().hasAllRequiredFields())
8787
.forEach(entry ->
8888
{
8989
ClassSpec classSpec = getAsyncResponseSpec(entry);

0 commit comments

Comments
 (0)