Allow pagination tokens to be maps

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

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<ShapeType> ITEM_SHAPES = SetUtils.of(ShapeType.LIST, ShapeType.MAP);
     private static final Set<ShapeType> PAGE_SHAPES = SetUtils.of(ShapeType.INTEGER);
-    private static final Set<ShapeType> STRING_SET = SetUtils.of(ShapeType.STRING);
+    private static final Set<ShapeType> TOKEN_SHAPES = SetUtils.of(ShapeType.STRING, ShapeType.MAP);
+    private static final Set<ShapeType> DANGER_TOKEN_SHAPES = SetUtils.of(ShapeType.MAP);
     private static final Pattern PATH_PATTERN = Pattern.compile("\\.");
 
     @Override
@@ -160,12 +162,23 @@ private List<ValidationEvent> 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<ShapeType> 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<ShapeType> validTargets();
 
+        Set<ShapeType> dangerTargets() {
+            return Collections.emptySet();
+        }
+
         abstract Optional<String> getMemberPath(OperationIndex opIndex, OperationShape operation, PaginatedTrait trait);
 
         abstract Optional<MemberShape> getMember(
@@ -229,7 +246,11 @@ String propertyName() {
         }
 
         Set<ShapeType> validTargets() {
-            return STRING_SET;
+            return TOKEN_SHAPES;
+        }
+
+        Set<ShapeType> dangerTargets() {
+            return DANGER_TOKEN_SHAPES;
         }
 
         Optional<String> getMemberPath(OperationIndex opIndex, OperationShape operation, PaginatedTrait trait) {
@@ -258,7 +279,11 @@ String propertyName() {
         }
 
         Set<ShapeType> validTargets() {
-            return STRING_SET;
+            return TOKEN_SHAPES;
+        }
+
+        Set<ShapeType> dangerTargets() {
+            return DANGER_TOKEN_SHAPES;
         }
 
         Optional<String> getMemberPath(OperationIndex opIndex, OperationShape operation, PaginatedTrait trait) {

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

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"
+            }
         }
     }
 }