Repository: smithy-lang/smithy
Pull request #1866 (2023-07-19 18:10:07)

Slightly relax paginated pageSize types
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.



Patch:
diff --git a/docs/source-1.0/spec/core/behavior-traits.rst b/docs/source-1.0/spec/core/behavior-traits.rst
index 9aa6966d1..c1b0d1e45 100644
--- a/docs/source-1.0/spec/core/behavior-traits.rst
+++ b/docs/source-1.0/spec/core/behavior-traits.rst
@@ -214,7 +214,8 @@ The ``paginated`` trait is a structure that contains the following members:
       - ``string``
       - The name of an operation input member that limits the maximum number
         of results to include in the operation output. This input member
-        SHOULD NOT be required and MUST target an integer shape.
+        SHOULD NOT be required and SHOULD target an integer shape. It can, but
+        SHOULD NOT target a byte, short, or long shape.
 
         .. warning::
 
diff --git a/docs/source-2.0/spec/behavior-traits.rst b/docs/source-2.0/spec/behavior-traits.rst
index b885845b5..7ada59fea 100644
--- a/docs/source-2.0/spec/behavior-traits.rst
+++ b/docs/source-2.0/spec/behavior-traits.rst
@@ -225,7 +225,8 @@ The ``paginated`` trait is a structure that contains the following members:
       - ``string``
       - The name of an operation input member that limits the maximum number
         of results to include in the operation output. This input member
-        SHOULD NOT be required and MUST target an integer shape.
+        SHOULD NOT be required and SHOULD target an integer shape. It can, but
+        SHOULD NOT target a byte, short, or long shape.
 
         .. warning::
 
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 08dbdf56e..51f6c0f3c 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
@@ -45,19 +45,21 @@ import software.amazon.smithy.utils.SetUtils;
  *      operation's properties are optionally merged with a service's.</li>
  *     <li>The items property, if set, must reference a list or map
  *      output member.</li>
- *     <li>The pageSize property, if set, must reference an optional integer
- *      input member.</li>
- *     <li>The inputToken property must reference an optional string input
- *      member.</li>
- *     <li>The outputToken property must reference an optional string output
- *      member.</li>
- *     <li>The pageSize property, if set, must reference an optional input
- *      member that targets an integer.</li>
+ *     <li>The pageSize property, if set, should reference an optional integer
+ *      input member. It may, but should not reference an optional byte, short,
+ *      or long.</li>
+ *     <li>The inputToken property should reference an optional string input
+ *      member. It may, but should not reference an optional map.</li>
+ *     <li>The outputToken property should reference an optional string output
+ *      member. It may, but should not reference an optional map.</li>
  * </ul>
  */
 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> PAGE_SHAPES = SetUtils.of(ShapeType.BYTE, ShapeType.INTEGER,
+            ShapeType.LONG, ShapeType.SHORT);
+    private static final Set<ShapeType> DANGER_PAGE_SHAPES = SetUtils.of(ShapeType.BYTE, ShapeType.LONG,
+            ShapeType.SHORT);
     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("\\.");
@@ -167,14 +169,14 @@ public final class PaginatedTraitValidator extends AbstractValidator {
                 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(),
+                        prefix, validator.propertyName(), member.getId().getMember().get(), target.getType(),
                         ValidationUtils.tickedList(validator.validTargets()))));
             }
             if (validator.dangerTargets().contains(target.getType())) {
                 Set<ShapeType> preferredTargets = new TreeSet<>(validator.validTargets());
                 preferredTargets.removeAll(validator.dangerTargets());
                 String traitName = validator.propertyName();
-                String memberName = member.getId().getName();
+                String memberName = member.getId().getMember().get();
                 String targetType = target.getType().toString();
                 events.add(danger(operation, trait, String.format(
                             "%spaginated trait `%s` member `%s` targets a %s shape, but this is not recommended. "
@@ -319,6 +321,10 @@ public final class PaginatedTraitValidator extends AbstractValidator {
             return PAGE_SHAPES;
         }
 
+        Set<ShapeType> dangerTargets() {
+            return DANGER_PAGE_SHAPES;
+        }
+
         Optional<String> getMemberPath(OperationIndex opIndex, OperationShape operation, PaginatedTrait trait) {
             return trait.getPageSize();
         }


Test patch:
diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/paginated/paginated-invalid.errors b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/paginated/paginated-invalid.errors
index 90123a12a..48508ebf0 100644
--- a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/paginated/paginated-invalid.errors
+++ b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/paginated/paginated-invalid.errors
@@ -1,13 +1,13 @@
 [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: [`map`, `string`] | PaginatedTrait
+[ERROR] ns.foo#Invalid3: paginated trait `inputToken` member `nextToken` 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: [`map`, `string`] | PaginatedTrait
+[ERROR] ns.foo#Invalid3: paginated trait `outputToken` member `nextToken` 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.ShouldNotBeRequired.pageSize
-[ERROR] ns.foo#Invalid5: paginated trait `pageSize` member `Invalid5Input` targets a string shape, but must target one of the following: [`integer`] | PaginatedTrait
-[ERROR] ns.foo#Invalid6: paginated trait `items` member `Invalid6Output` targets a string shape, but must target one of the following: [`list`, `map`] | PaginatedTrait
+[ERROR] ns.foo#Invalid5: paginated trait `pageSize` member `pageSize` targets a string shape, but must target one of the following: [`byte`, `integer`, `long`, `short`] | PaginatedTrait
+[ERROR] ns.foo#Invalid6: paginated trait `items` member `items` targets a string shape, but must target one of the following: [`list`, `map`] | PaginatedTrait
 [ERROR] ns.foo#Invalid7: paginated trait `items` targets a member `items` that does not exist | PaginatedTrait
 [ERROR] ns.foo#Invalid7: paginated trait `outputToken` targets a member `missing` that does not exist | PaginatedTrait
 [ERROR] ns.foo#Invalid8: When bound within the `ns.foo#Service` service, paginated trait `inputToken` is not configured | PaginatedTrait
@@ -18,3 +18,6 @@
 [ERROR] ns.foo#Invalid1: paginated trait `pageSize` targets a member `pageSize` that does not exist | PaginatedTrait
 [ERROR] ns.foo#InvalidNoOutput: paginated trait `items` targets a member `items` that does not exist | PaginatedTrait
 [ERROR] ns.foo#InvalidNoOutput: paginated trait `outputToken` targets a member `nextToken` that does not exist | PaginatedTrait
+[DANGER] ns.foo#DangerSizeByte: paginated trait `pageSize` member `pageSize` targets a byte shape, but this is not recommended. One of [`integer`] SHOULD be targeted. | PaginatedTrait
+[DANGER] ns.foo#DangerSizeShort: paginated trait `pageSize` member `pageSize` targets a short shape, but this is not recommended. One of [`integer`] SHOULD be targeted. | PaginatedTrait
+[DANGER] ns.foo#DangerSizeLong: paginated trait `pageSize` member `pageSize` targets a long shape, but this is not recommended. One of [`integer`] SHOULD be targeted. | PaginatedTrait
diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/paginated/paginated-invalid.json b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/paginated/paginated-invalid.json
index 5080a86cf..79d3e73dd 100644
--- a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/paginated/paginated-invalid.json
+++ b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/paginated/paginated-invalid.json
@@ -34,6 +34,15 @@
                 },
                 {
                     "target": "ns.foo#InvalidNestedInput"
+                },
+                {
+                    "target": "ns.foo#DangerSizeByte"
+                },
+                {
+                    "target": "ns.foo#DangerSizeShort"
+                },
+                {
+                    "target": "ns.foo#DangerSizeLong"
                 }
             ]
         },
@@ -303,6 +312,93 @@
                 }
             }
         },
+        "ns.foo#DangerSizeByte": {
+            "type": "operation",
+            "input": {
+                "target": "ns.foo#DangerSizeByteInput"
+            },
+            "output": {
+                "target": "ns.foo#ValidOutput"
+            },
+            "traits": {
+                "smithy.api#readonly": {},
+                "smithy.api#paginated": {
+                    "inputToken": "nextToken",
+                    "outputToken": "nextToken",
+                    "items": "items",
+                    "pageSize": "pageSize"
+                }
+            }
+        },
+        "ns.foo#DangerSizeByteInput": {
+            "type": "structure",
+            "members": {
+                "nextToken": {
+                    "target": "smithy.api#String"
+                },
+                "pageSize": {
+                    "target": "smithy.api#Byte"
+                }
+            }
+        },
+        "ns.foo#DangerSizeShort": {
+            "type": "operation",
+            "input": {
+                "target": "ns.foo#DangerSizeShortInput"
+            },
+            "output": {
+                "target": "ns.foo#ValidOutput"
+            },
+            "traits": {
+                "smithy.api#readonly": {},
+                "smithy.api#paginated": {
+                    "inputToken": "nextToken",
+                    "outputToken": "nextToken",
+                    "items": "items",
+                    "pageSize": "pageSize"
+                }
+            }
+        },
+        "ns.foo#DangerSizeShortInput": {
+            "type": "structure",
+            "members": {
+                "nextToken": {
+                    "target": "smithy.api#String"
+                },
+                "pageSize": {
+                    "target": "smithy.api#Short"
+                }
+            }
+        },
+        "ns.foo#DangerSizeLong": {
+            "type": "operation",
+            "input": {
+                "target": "ns.foo#DangerSizeLongInput"
+            },
+            "output": {
+                "target": "ns.foo#ValidOutput"
+            },
+            "traits": {
+                "smithy.api#readonly": {},
+                "smithy.api#paginated": {
+                    "inputToken": "nextToken",
+                    "outputToken": "nextToken",
+                    "items": "items",
+                    "pageSize": "pageSize"
+                }
+            }
+        },
+        "ns.foo#DangerSizeLongInput": {
+            "type": "structure",
+            "members": {
+                "nextToken": {
+                    "target": "smithy.api#String"
+                },
+                "pageSize": {
+                    "target": "smithy.api#Long"
+                }
+            }
+        },
         "ns.foo#NestedInput": {
             "type": "structure",
             "members": {
diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/paginated/paginated-map-tokens.errors b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/paginated/paginated-map-tokens.errors
index 9f6bc968c..30b2459a2 100644
--- a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/paginated/paginated-map-tokens.errors
+++ b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/paginated/paginated-map-tokens.errors
@@ -1,2 +1,2 @@
-[DANGER] ns.foo#MapTokens: paginated trait `inputToken` member `MapTokensInput` targets a map shape, but this is not recommended. One of [`string`] SHOULD be targeted. | PaginatedTrait.WrongShapeType.inputToken
-[DANGER] ns.foo#MapTokens: paginated trait `outputToken` member `MapTokensOutput` targets a map shape, but this is not recommended. One of [`string`] SHOULD be targeted. | PaginatedTrait.WrongShapeType.outputToken
+[DANGER] ns.foo#MapTokens: paginated trait `inputToken` member `nextToken` targets a map shape, but this is not recommended. One of [`string`] SHOULD be targeted. | PaginatedTrait.WrongShapeType.inputToken
+[DANGER] ns.foo#MapTokens: paginated trait `outputToken` member `nextToken` targets a map shape, but this is not recommended. One of [`string`] SHOULD be targeted. | PaginatedTrait.WrongShapeType.outputToken