Modifies ExamplesTraitValidator to fail when both output and error ar…

…e defined (#1599) * Fix behavior of examples with output and error This commit changes the behavior of the ExamplesTraitValidator when both output and error are defined. This is achieved by treating the output member as optional, only validating it if it's present and failing when it's defined alongside the error member.
smithy-lang · Feb 23, 2023 · 43b13da · 43b13da
1 parent 57873f3
commit 43b13da
Show file tree

Hide file tree

Showing 7 changed files with 53 additions and 24 deletions.
diff --git a/docs/source-1.0/spec/core/documentation-traits.rst b/docs/source-1.0/spec/core/documentation-traits.rst
@@ -206,6 +206,9 @@ compatible with the shapes and constraints of the corresponding structure.
 These values use the same semantics and format as
 :ref:`custom trait values <trait-node-values>`.
 
+A value for ``output`` or ``error`` SHOULD be provided. However, both
+MUST NOT be defined for the same example.
+
 .. tabs::
 
     .. code-tab:: smithy

diff --git a/docs/source-2.0/spec/documentation-traits.rst b/docs/source-2.0/spec/documentation-traits.rst
@@ -144,6 +144,9 @@ compatible with the shapes and constraints of the corresponding structure.
 These values use the same semantics and format as
 :ref:`custom trait values <trait-node-values>`.
 
+A value for ``output`` or ``error`` SHOULD be provided. However, both
+MUST NOT be defined for the same example.
+
 .. code-block:: smithy
 
     @readonly
@@ -175,9 +178,7 @@ These values use the same semantics and format as
         {
             title: "Error example for MyOperation"
             input: {
-                foo: "!",
-            }
-                foo: 1
+                foo: "!"
             }
             error: {
                 shapeId: MyOperationError

diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/traits/ExamplesTrait.java b/smithy-model/src/main/java/software/amazon/smithy/model/traits/ExamplesTrait.java
@@ -133,8 +133,8 @@ public ObjectNode getInput() {
         /**
          * @return Gets the output object.
          */
-        public ObjectNode getOutput() {
-            return output;
+        public Optional<ObjectNode> getOutput() {
+            return Optional.ofNullable(output);
         }
 
         /**
@@ -154,7 +154,7 @@ public Node toNode() {
             if (!input.isEmpty()) {
                 builder.withMember("input", input);
             }
-            if (!output.isEmpty()) {
+            if (this.getOutput().isPresent()) {
                 builder.withMember("output", output);
             }
 
@@ -177,7 +177,7 @@ public static final class Builder implements SmithyBuilder<Example> {
             private String title;
             private String documentation;
             private ObjectNode input = Node.objectNode();
-            private ObjectNode output = Node.objectNode();
+            private ObjectNode output;
             private ErrorExample error;
 
             @Override

diff --git a/.../main/java/software/amazon/smithy/model/validation/validators/ExamplesTraitValidator.java b/.../main/java/software/amazon/smithy/model/validation/validators/ExamplesTraitValidator.java
@@ -53,13 +53,20 @@ private List<ValidationEvent> validateExamples(Model model, OperationShape shape
                 events.addAll(input.accept(validator));
             });
 
-            model.getShape(shape.getOutputShape()).ifPresent(output -> {
-                NodeValidationVisitor validator = createVisitor(
-                        "output", example.getOutput(), model, shape, example);
-                events.addAll(output.accept(validator));
-            });
+            boolean isOutputDefined = example.getOutput().isPresent();
+            boolean isErrorDefined = example.getError().isPresent();
 
-            if (example.getError().isPresent()) {
+            if (isOutputDefined && isErrorDefined) {
+                events.add(error(shape, trait, String.format(
+                        "Example: `%s` has both output and error defined, only one should be present.",
+                        example.getTitle())));
+            } else if (isOutputDefined) {
+                model.getShape(shape.getOutputShape()).ifPresent(output -> {
+                    NodeValidationVisitor validator = createVisitor(
+                            "output", example.getOutput().get(), model, shape, example);
+                    events.addAll(output.accept(validator));
+                });
+            } else if (isErrorDefined) {
                 ExamplesTrait.ErrorExample errorExample = example.getError().get();
                 Optional<Shape> errorShape = model.getShape(errorExample.getShapeId());
                 if (errorShape.isPresent() && shape.getErrors().contains(errorExample.getShapeId())) {
@@ -68,7 +75,7 @@ private List<ValidationEvent> validateExamples(Model model, OperationShape shape
                     events.addAll(errorShape.get().accept(validator));
                 } else {
                     events.add(error(shape, trait, String.format(
-                        "Error parameters provided for operation without the `%s` error: `%s`",
+                            "Error parameters provided for operation without the `%s` error: `%s`",
                             errorExample.getShapeId(), example.getTitle())));
                 }
             }

diff --git a/...ources/software/amazon/smithy/model/errorfiles/validators/examples-trait-validator.errors b/...ources/software/amazon/smithy/model/errorfiles/validators/examples-trait-validator.errors
@@ -1,6 +1,7 @@
-[WARNING] ns.foo#Operation2: Example input of `Testing 3`: Invalid structure member `foo` found for `smithy.api#Unit` | ExamplesTrait
-[WARNING] ns.foo#Operation2: Example output of `Testing 3`: Invalid structure member `bam` found for `smithy.api#Unit` | ExamplesTrait
-[ERROR] ns.foo#Operation2: Error parameters provided for operation without the `ns.foo#OperationError` error: `Testing 3` | ExamplesTrait
+[ERROR] ns.foo#Operation2: Error parameters provided for operation without the `ns.foo#OperationError` error: `Testing 5` | ExamplesTrait
+[WARNING] ns.foo#Operation2: Example input of `Testing 4`: Invalid structure member `foo` found for `smithy.api#Unit` | ExamplesTrait
+[WARNING] ns.foo#Operation2: Example output of `Testing 4`: Invalid structure member `bam` found for `smithy.api#Unit` | ExamplesTrait
+[ERROR] ns.foo#Operation: Example: `Testing 3` has both output and error defined, only one should be present. | ExamplesTrait
 [ERROR] ns.foo#Operation: Example input of `Testing 2`: Missing required structure member `foo` for `ns.foo#OperationInput` | ExamplesTrait
 [WARNING] ns.foo#Operation: Example output of `Testing 2`: Invalid structure member `additional` found for `ns.foo#OperationOutput` | ExamplesTrait
 [ERROR] ns.foo#Operation: Example output of `Testing 2`: Missing required structure member `bam` for `ns.foo#OperationOutput` | ExamplesTrait

diff --git a/...esources/software/amazon/smithy/model/errorfiles/validators/examples-trait-validator.json b/...esources/software/amazon/smithy/model/errorfiles/validators/examples-trait-validator.json
@@ -22,9 +22,6 @@
                         "input": {
                             "foo": "value1"
                         },
-                        "output": {
-                            "bam": "value2"
-                        },
                         "error": {
                             "shapeId": "ns.foo#OperationError",
                             "content": {
@@ -41,6 +38,21 @@
                         "output": {
                             "additional": "value"
                         }
+                    },
+                    {
+                        "title": "Testing 3",
+                        "input": {
+                            "foo": "value1"
+                        },
+                        "output": {
+                            "bam": "value2"
+                        },
+                        "error": {
+                            "shapeId": "ns.foo#OperationError",
+                            "content": {
+                                "bat": "baz"
+                            }
+                        }
                     }
                 ]
             }
@@ -93,13 +105,16 @@
                 "smithy.api#readonly": {},
                 "smithy.api#examples": [
                     {
-                        "title": "Testing 3",
+                        "title": "Testing 4",
                         "input": {
                             "foo": "baz"
                         },
                         "output": {
                             "bam": "baz"
-                        },
+                        }
+                    },
+                    {
+                        "title": "Testing 5",
                         "error": {
                             "shapeId": "ns.foo#OperationError",
                             "content": {

diff --git a/...c/main/java/software/amazon/smithy/openapi/fromsmithy/protocols/AbstractRestProtocol.java b/...c/main/java/software/amazon/smithy/openapi/fromsmithy/protocols/AbstractRestProtocol.java
@@ -246,7 +246,8 @@ private Map<String, Node> createExamplesForMembersWithHttpTraits(
             Optional<ExamplesTrait> examplesTrait = operationOrError.getTrait(ExamplesTrait.class);
             for (ExamplesTrait.Example example
                     : examplesTrait.map(ExamplesTrait::getExamples).orElse(Collections.emptyList())) {
-                ObjectNode inputOrOutput = type == MessageType.REQUEST ? example.getInput() : example.getOutput();
+                ObjectNode inputOrOutput = type == MessageType.REQUEST ? example.getInput()
+                        : example.getOutput().orElse(Node.objectNode());
                 String name = operationOrError.getId().getName() + "_example" + uniqueNum++;
 
                 // this if condition is needed to avoid errors when converting examples of response.
@@ -327,7 +328,8 @@ private Map<String, Node> createBodyExamples(
                     : examplesTrait.map(ExamplesTrait::getExamples).orElse(Collections.emptyList())) {
                 // get members included in bindings
                 ObjectNode values = getMembersWithHttpBindingTrait(bindings,
-                        type == MessageType.REQUEST ? example.getInput() : example.getOutput());
+                        type == MessageType.REQUEST ? example.getInput()
+                                : example.getOutput().orElse(Node.objectNode()));
                 String name = operationOrError.getId().getName() + "_example" + uniqueNum++;
                 // this if condition is needed to avoid errors when converting examples of response.
                 if (!example.getError().isPresent() || type == MessageType.REQUEST) {