diff --git a/docs/source/1.0/spec/core/behavior-traits.rst b/docs/source/1.0/spec/core/behavior-traits.rst index ea95a74f1b7..ace4645ade0 100644 --- a/docs/source/1.0/spec/core/behavior-traits.rst +++ b/docs/source/1.0/spec/core/behavior-traits.rst @@ -194,7 +194,8 @@ The ``paginated`` trait is a structure that contains the following members: - The name of the operation input member that contains a continuation token. When this value is provided as input, the service returns results from where the previous response left off. This input member - MUST NOT be marked as ``required`` and MUST target a string shape. + MUST NOT be marked as ``required`` and SHOULD target a string shape. + It can, but SHOULD NOT target a map shape. When contained within a service, a paginated operation MUST either configure ``inputToken`` on the operation itself or inherit it from @@ -206,8 +207,8 @@ The ``paginated`` trait is a structure that contains the following members: it indicates that there are more results to retrieve. To get the next page of results, the client passes the received output continuation token to the input continuation token of the next request. This - output member MUST NOT be marked as ``required`` and MUST target a - string shape. + output member MUST NOT be marked as ``required`` and SHOULD target a + string shape. It can, but SHOULD NOT target a map shape. When contained within a service, a paginated operation MUST either configure ``outputToken`` on the operation itself or inherit it from diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/PaginatedTraitValidator.java b/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/PaginatedTraitValidator.java index 508d3cc40c1..fcfda2eb152 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/PaginatedTraitValidator.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/PaginatedTraitValidator.java @@ -20,6 +20,7 @@ import java.util.List; import java.util.Optional; import java.util.Set; +import java.util.TreeSet; import java.util.regex.Pattern; import software.amazon.smithy.model.Model; import software.amazon.smithy.model.knowledge.OperationIndex; @@ -57,7 +58,8 @@ public final class PaginatedTraitValidator extends AbstractValidator { private static final Set ITEM_SHAPES = SetUtils.of(ShapeType.LIST, ShapeType.MAP); private static final Set PAGE_SHAPES = SetUtils.of(ShapeType.INTEGER); - private static final Set STRING_SET = SetUtils.of(ShapeType.STRING); + private static final Set TOKEN_SHAPES = SetUtils.of(ShapeType.STRING, ShapeType.MAP); + private static final Set DANGER_TOKEN_SHAPES = SetUtils.of(ShapeType.MAP); private static final Pattern PATH_PATTERN = Pattern.compile("\\."); @Override @@ -160,12 +162,23 @@ private List validateMember( } Shape target = model.getShape(member.getTarget()).orElse(null); - if (target != null && !validator.validTargets().contains(target.getType())) { - events.add(error(operation, trait, String.format( - "%spaginated trait `%s` member `%s` targets a %s shape, but must target one of " - + "the following: [%s]", - prefix, validator.propertyName(), member.getId().getName(), target.getType(), - ValidationUtils.tickedList(validator.validTargets())))); + if (target != null) { + if (!validator.validTargets().contains(target.getType())) { + events.add(error(operation, trait, String.format( + "%spaginated trait `%s` member `%s` targets a %s shape, but must target one of " + + "the following: [%s]", + prefix, validator.propertyName(), member.getId().getName(), target.getType(), + ValidationUtils.tickedList(validator.validTargets())))); + } + if (validator.dangerTargets().contains(target.getType())) { + Set preferredTargets = new TreeSet<>(validator.validTargets()); + preferredTargets.removeAll(validator.dangerTargets()); + events.add(danger(operation, trait, String.format( + "%spaginated trait `%s` member `%s` targets a %s shape, but this is not recommended. " + + "One of [%s] SHOULD be targeted.", + prefix, validator.propertyName(), member.getId().getName(), target.getType(), + ValidationUtils.tickedList(preferredTargets)))); + } } if (validator.pathsAllowed() && PATH_PATTERN.split(memberPath).length > 2) { @@ -188,6 +201,10 @@ private abstract static class PropertyValidator { abstract Set validTargets(); + Set dangerTargets() { + return Collections.emptySet(); + } + abstract Optional getMemberPath(OperationIndex opIndex, OperationShape operation, PaginatedTrait trait); abstract Optional getMember( @@ -229,7 +246,11 @@ String propertyName() { } Set validTargets() { - return STRING_SET; + return TOKEN_SHAPES; + } + + Set dangerTargets() { + return DANGER_TOKEN_SHAPES; } Optional getMemberPath(OperationIndex opIndex, OperationShape operation, PaginatedTrait trait) { @@ -258,7 +279,11 @@ String propertyName() { } Set validTargets() { - return STRING_SET; + return TOKEN_SHAPES; + } + + Set dangerTargets() { + return DANGER_TOKEN_SHAPES; } Optional getMemberPath(OperationIndex opIndex, OperationShape operation, PaginatedTrait trait) { diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/paginated-trait-test.errors b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/paginated-trait-test.errors index 8708efd4a4e..8154ebaf558 100644 --- a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/paginated-trait-test.errors +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/paginated-trait-test.errors @@ -1,9 +1,9 @@ [ERROR] ns.foo#Invalid1: paginated operations require an input | PaginatedTrait [ERROR] ns.foo#Invalid2: paginated trait `inputToken` targets a member `nextToken` that does not exist | PaginatedTrait [ERROR] ns.foo#Invalid2: paginated trait `outputToken` targets a member `nextToken` that does not exist | PaginatedTrait -[ERROR] ns.foo#Invalid3: paginated trait `inputToken` member `InputNotString` targets a integer shape, but must target one of the following: [`string`] | PaginatedTrait +[ERROR] ns.foo#Invalid3: paginated trait `inputToken` member `InputNotString` targets a integer shape, but must target one of the following: [`map`, `string`] | PaginatedTrait [ERROR] ns.foo#Invalid3: paginated trait `items` targets a member `items` that does not exist | PaginatedTrait -[ERROR] ns.foo#Invalid3: paginated trait `outputToken` member `OutputNotString` targets a integer shape, but must target one of the following: [`string`] | PaginatedTrait +[ERROR] ns.foo#Invalid3: paginated trait `outputToken` member `OutputNotString` targets a integer shape, but must target one of the following: [`map`, `string`] | PaginatedTrait [ERROR] ns.foo#Invalid4: paginated trait `inputToken` member `nextToken` must not be required | PaginatedTrait [ERROR] ns.foo#Invalid4: paginated trait `outputToken` member `nextToken` must not be required | PaginatedTrait [WARNING] ns.foo#Invalid4: paginated trait `pageSize` member `pageSize` should not be required | PaginatedTrait @@ -22,3 +22,5 @@ [WARNING] ns.foo#DeeplyNestedOutputOperation: paginated trait `outputToken` contains a path with more than two parts, which can make your API cumbersome to use | PaginatedTrait [ERROR] ns.foo#InvalidNestedInput: paginated trait `inputToken` does not allow path values | PaginatedTrait [ERROR] ns.foo#InvalidNestedInput: paginated trait `pageSize` does not allow path values | PaginatedTrait +[DANGER] ns.foo#MapTokens: paginated trait `inputToken` member `MapTokenInputOutput` targets a map shape, but this is not recommended. One of [`string`] SHOULD be targeted. | PaginatedTrait +[DANGER] ns.foo#MapTokens: paginated trait `outputToken` member `MapTokenInputOutput` targets a map shape, but this is not recommended. One of [`string`] SHOULD be targeted. | PaginatedTrait diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/paginated-trait-test.json b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/paginated-trait-test.json index ac0c2b8e20c..ae65d1e8c2b 100644 --- a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/paginated-trait-test.json +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/paginated-trait-test.json @@ -52,6 +52,9 @@ }, { "target": "ns.foo#InvalidNestedInput" + }, + { + "target": "ns.foo#MapTokens" } ] }, @@ -589,6 +592,39 @@ "target": "ns.foo#StringList" } } + }, + "ns.foo#MapTokens": { + "type": "operation", + "input": { + "target": "ns.foo#MapTokenInputOutput" + }, + "output": { + "target": "ns.foo#MapTokenInputOutput" + }, + "traits": { + "smithy.api#readonly": {}, + "smithy.api#paginated": { + "inputToken": "nextToken", + "outputToken": "nextToken" + } + } + }, + "ns.foo#MapTokenInputOutput": { + "type": "structure", + "members": { + "nextToken": { + "target": "ns.foo#StringMap" + } + } + }, + "ns.foo#StringMap": { + "type": "map", + "key": { + "target": "smithy.api#String" + }, + "value": { + "target": "smithy.api#String" + } } } }