diff --git a/docs/source/1.0/spec/core/json-ast.rst b/docs/source/1.0/spec/core/json-ast.rst index 7d40734f465..b72ce4bf4fa 100644 --- a/docs/source/1.0/spec/core/json-ast.rst +++ b/docs/source/1.0/spec/core/json-ast.rst @@ -399,6 +399,12 @@ shapes defined in JSON support the same properties as the Smithy IDL. - [:ref:`AST shape reference `] - Binds a list of resources to the service. Each reference MUST target a resource. + * - errors + - [:ref:`AST shape reference `] + - Defines a list of common errors that every operation bound within the + closure of the service can return. Each provided shape ID MUST target + a :ref:`structure ` shape that is marked with the + :ref:`error-trait`. * - traits - map of :ref:`shape ID ` to trait values - Traits to apply to the service @@ -425,6 +431,11 @@ shapes defined in JSON support the same properties as the Smithy IDL. "target": "smithy.example#SomeResource" } ], + "errors": [ + { + "target": "smithy.example#SomeError" + } + ], "traits": { "smithy.api#documentation": "Documentation for the service" }, diff --git a/docs/source/1.0/spec/core/model.rst b/docs/source/1.0/spec/core/model.rst index 4fff0f4c494..1f4068e0b58 100644 --- a/docs/source/1.0/spec/core/model.rst +++ b/docs/source/1.0/spec/core/model.rst @@ -1014,6 +1014,12 @@ The service shape supports the following properties: - Binds a set of ``resource`` shapes to the service. Each element in the given list MUST be a valid :ref:`shape ID ` that targets a :ref:`resource ` shape. + * - errors + - [``string``] + - Defines a list of common errors that every operation bound within the + closure of the service can return. Each provided shape ID MUST target + a :ref:`structure ` shape that is marked with the + :ref:`error-trait`. * - rename - map of :ref:`shape ID ` to ``string`` - Disambiguates shape name conflicts in the @@ -1060,6 +1066,47 @@ The following example defines a service with no operations or resources. } } +The following example defines a service shape that defines a set of errors +that are common to every operation in the service: + +.. tabs:: + + .. code-tab:: smithy + + namespace smithy.example + + service MyService { + version: "2017-02-11", + errors: [SomeError] + } + + @error("client") + structure SomeError {} + + .. code-tab:: json + + { + "smithy": "1.0", + "shapes": { + "smithy.example#MyService": { + "type": "service", + "version": "2017-02-11", + "errors": [ + { + "target": "smithy.example#SomeError" + } + ] + }, + "smithy.example#SomeError": { + "type": "structure", + "traits": { + "smithy.api#error": "client" + } + } + } + } + + .. _service-operations: diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/knowledge/OperationIndex.java b/smithy-model/src/main/java/software/amazon/smithy/model/knowledge/OperationIndex.java index 98ec4a0b82a..ce0fc5267f6 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/knowledge/OperationIndex.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/knowledge/OperationIndex.java @@ -21,9 +21,12 @@ import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.Set; +import java.util.TreeSet; import software.amazon.smithy.model.Model; import software.amazon.smithy.model.shapes.MemberShape; import software.amazon.smithy.model.shapes.OperationShape; +import software.amazon.smithy.model.shapes.ServiceShape; import software.amazon.smithy.model.shapes.Shape; import software.amazon.smithy.model.shapes.ShapeId; import software.amazon.smithy.model.shapes.StructureShape; @@ -50,13 +53,20 @@ public OperationIndex(Model model) { operation.getOutput() .flatMap(id -> getStructure(model, id)) .ifPresent(shape -> outputs.put(operation.getId(), shape)); + addErrorsFromShape(model, operation.getId(), operation.getErrors()); + } - List errorShapes = new ArrayList<>(operation.getErrors().size()); - for (ShapeId target : operation.getErrors()) { - model.getShape(target).flatMap(Shape::asStructureShape).ifPresent(errorShapes::add); - } - errors.put(operation.getId(), errorShapes); + for (ServiceShape service : model.getServiceShapes()) { + addErrorsFromShape(model, service.getId(), service.getErrors()); + } + } + + private void addErrorsFromShape(Model model, ShapeId source, List errorShapeIds) { + List errorShapes = new ArrayList<>(errorShapeIds.size()); + for (ShapeId target : errorShapeIds) { + model.getShape(target).flatMap(Shape::asStructureShape).ifPresent(errorShapes::add); } + errors.put(source, errorShapes); } public static OperationIndex of(Model model) { @@ -161,11 +171,29 @@ public boolean isOutputStructure(ToShapeId structureId) { * * @param operation Operation to get the errors of. * @return Returns the list of error structures, or an empty list. + * @see #getErrors(ToShapeId, ToShapeId) to get errors that inherit from a service. */ public List getErrors(ToShapeId operation) { return errors.getOrDefault(operation.toShapeId(), ListUtils.of()); } + /** + * Gets the list of error structures defined on an operation, + * including any common errors inherited from a service shape. + * + *

An empty list is returned if the operation is not found or + * has no errors. + * + * @param service Service shape to inherit common errors from. + * @param operation Operation to get the errors of. + * @return Returns the list of error structures, or an empty list. + */ + public List getErrors(ToShapeId service, ToShapeId operation) { + Set result = new TreeSet<>(getErrors(service)); + result.addAll(getErrors(operation)); + return new ArrayList<>(result); + } + private Optional getStructure(Model model, ToShapeId id) { return model.getShape(id.toShapeId()).flatMap(Shape::asStructureShape); } diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/loader/AstModelLoader.java b/smithy-model/src/main/java/software/amazon/smithy/model/loader/AstModelLoader.java index b13595d1f46..9c83cbd808a 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/loader/AstModelLoader.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/loader/AstModelLoader.java @@ -68,6 +68,7 @@ enum AstModelLoader { private static final String TRAITS = "traits"; private static final String TYPE = "type"; private static final String TARGET = "target"; + private static final String ERRORS = "errors"; private static final List TOP_LEVEL_PROPERTIES = ListUtils.of("smithy", SHAPES, METADATA); private static final List APPLY_PROPERTIES = ListUtils.of(TYPE, TRAITS); @@ -78,12 +79,12 @@ enum AstModelLoader { private static final Set MEMBER_PROPERTIES = SetUtils.of(TARGET, TRAITS); private static final Set REFERENCE_PROPERTIES = SetUtils.of(TARGET); private static final Set OPERATION_PROPERTY_NAMES = SetUtils.of( - TYPE, "input", "output", "errors", TRAITS); + TYPE, "input", "output", ERRORS, TRAITS); private static final Set RESOURCE_PROPERTIES = SetUtils.of( TYPE, "create", "read", "update", "delete", "list", "put", "identifiers", "resources", "operations", "collectionOperations", TRAITS); private static final Set SERVICE_PROPERTIES = SetUtils.of( - TYPE, "version", "operations", "resources", "rename", TRAITS); + TYPE, "version", "operations", "resources", "rename", ERRORS, TRAITS); ModelFile load(TraitFactory traitFactory, ObjectNode model) { FullyResolvedModelFile modelFile = new FullyResolvedModelFile(traitFactory); @@ -244,7 +245,7 @@ private void loadOperation(ShapeId id, ObjectNode node, FullyResolvedModelFile m OperationShape.Builder builder = OperationShape.builder() .id(id) .source(node.getSourceLocation()) - .addErrors(loadOptionalTargetList(modelFile, id, node, "errors")); + .addErrors(loadOptionalTargetList(modelFile, id, node, ERRORS)); loadOptionalTarget(modelFile, id, node, "input").ifPresent(builder::input); loadOptionalTarget(modelFile, id, node, "output").ifPresent(builder::output); @@ -285,6 +286,7 @@ private void loadService(ShapeId id, ObjectNode node, FullyResolvedModelFile mod builder.operations(loadOptionalTargetList(modelFile, id, node, "operations")); builder.resources(loadOptionalTargetList(modelFile, id, node, "resources")); loadServiceRenameIntoBuilder(builder, node); + builder.addErrors(loadOptionalTargetList(modelFile, id, node, ERRORS)); modelFile.onShape(builder); } diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/loader/IdlModelParser.java b/smithy-model/src/main/java/software/amazon/smithy/model/loader/IdlModelParser.java index c4078524b51..b9f9fd98a03 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/loader/IdlModelParser.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/loader/IdlModelParser.java @@ -89,6 +89,7 @@ final class IdlModelParser extends SimpleParser { private static final String IDENTIFIERS_KEY = "identifiers"; private static final String VERSION_KEY = "version"; private static final String TYPE_KEY = "type"; + private static final String ERRORS_KEYS = "errors"; static final Collection RESOURCE_PROPERTY_NAMES = ListUtils.of( TYPE_KEY, CREATE_KEY, READ_KEY, UPDATE_KEY, DELETE_KEY, LIST_KEY, @@ -550,7 +551,7 @@ private void parseOperationStatement(ShapeId id, SourceLocation location) { modelFile.onShape(builder); optionalId(node, "input", builder::input); optionalId(node, "output", builder::output); - optionalIdList(node, "errors", builder::addError); + optionalIdList(node, ERRORS_KEYS, builder::addError); } private void parseServiceStatement(ShapeId id, SourceLocation location) { @@ -562,6 +563,7 @@ private void parseServiceStatement(ShapeId id, SourceLocation location) { modelFile.onShape(builder); optionalIdList(shapeNode, OPERATIONS_KEY, builder::addOperation); optionalIdList(shapeNode, RESOURCES_KEY, builder::addResource); + optionalIdList(shapeNode, ERRORS_KEYS, builder::addError); AstModelLoader.loadServiceRenameIntoBuilder(builder, shapeNode); } diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/neighbor/NeighborVisitor.java b/smithy-model/src/main/java/software/amazon/smithy/model/neighbor/NeighborVisitor.java index 756da6c36d7..4000d5d94b8 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/neighbor/NeighborVisitor.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/neighbor/NeighborVisitor.java @@ -71,6 +71,10 @@ public List serviceShape(ServiceShape shape) { for (ShapeId resource : shape.getResources()) { addBinding(result, shape, resource, RelationshipType.RESOURCE); } + // Add ERROR relationships from service -> errors. + for (ShapeId errorId : shape.getErrors()) { + result.add(relationship(shape, RelationshipType.ERROR, errorId)); + } return result; } diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/shapes/ModelSerializer.java b/smithy-model/src/main/java/software/amazon/smithy/model/shapes/ModelSerializer.java index e72470469f5..eac5684e94c 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/shapes/ModelSerializer.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/shapes/ModelSerializer.java @@ -232,6 +232,7 @@ public Node serviceShape(ServiceShape shape) { serviceBuilder.withOptionalMember("operations", createOptionalIdList(shape.getOperations())); serviceBuilder.withOptionalMember("resources", createOptionalIdList(shape.getResources())); + serviceBuilder.withOptionalMember("errors", createOptionalIdList(shape.getErrors())); if (!shape.getRename().isEmpty()) { ObjectNode.Builder renameBuilder = Node.objectNodeBuilder(); diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/shapes/OperationShape.java b/smithy-model/src/main/java/software/amazon/smithy/model/shapes/OperationShape.java index 87fe76d5c93..28a1158e49a 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/shapes/OperationShape.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/shapes/OperationShape.java @@ -180,7 +180,7 @@ public Builder addError(String errorShapeId) { * @param errorShapeIds Error shape IDs to add. * @return Returns the builder. */ - public Builder addErrors(List errorShapeIds) { + public Builder addErrors(Collection errorShapeIds) { errors.addAll(Objects.requireNonNull(errorShapeIds)); return this; } diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/shapes/ServiceShape.java b/smithy-model/src/main/java/software/amazon/smithy/model/shapes/ServiceShape.java index 8eca1ced36d..d0e5adde67e 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/shapes/ServiceShape.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/shapes/ServiceShape.java @@ -15,10 +15,14 @@ package software.amazon.smithy.model.shapes; +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Optional; import java.util.TreeMap; +import software.amazon.smithy.utils.ListUtils; import software.amazon.smithy.utils.MapUtils; import software.amazon.smithy.utils.ToSmithyBuilder; @@ -29,11 +33,13 @@ public final class ServiceShape extends EntityShape implements ToSmithyBuilder rename; + private final List errors; private ServiceShape(Builder builder) { super(builder); version = builder.version; rename = MapUtils.orderedCopyOf(builder.rename); + errors = ListUtils.copyOf(builder.errors); } public static Builder builder() { @@ -42,11 +48,13 @@ public static Builder builder() { @Override public Builder toBuilder() { - Builder builder = builder().from(this).version(version); - getOperations().forEach(builder::addOperation); - getResources().forEach(builder::addResource); - builder.rename(rename); - return builder; + return builder() + .from(this) + .version(version) + .errors(errors) + .rename(rename) + .operations(getOperations()) + .resources(getResources()); } @Override @@ -66,7 +74,9 @@ public boolean equals(Object other) { } ServiceShape o = (ServiceShape) other; - return version.equals(o.version) && rename.equals(o.rename); + return version.equals(o.version) + && rename.equals(o.rename) + && errors.equals(o.errors); } /** @@ -86,6 +96,20 @@ public Map getRename() { return rename; } + /** + *

Gets a list of the common errors that can be encountered by + * every operation in the service.

+ * + *

Each returned {@link ShapeId} must resolve to a + * {@link StructureShape} that is targeted by an error trait; however, + * this is only guaranteed after a model is validated.

+ * + * @return Returns the errors. + */ + public List getErrors() { + return errors; + } + /** * Gets the contextual name of a shape within the closure. * @@ -111,6 +135,7 @@ public String getContextualName(ToShapeId shape) { public static final class Builder extends EntityShape.Builder { private String version = ""; private final Map rename = new TreeMap<>(); + private final List errors = new ArrayList<>(); @Override public ServiceShape build() { @@ -147,5 +172,74 @@ public Builder removeRename(ToShapeId from) { rename.remove(from.toShapeId()); return this; } + + /** + * Sets and replaces the errors of the service. Each error is implicitly + * bound to every operation within the closure of the service. + * + * @param errorShapeIds Error shape IDs to set. + * @return Returns the builder. + */ + public Builder errors(Collection errorShapeIds) { + errors.clear(); + errorShapeIds.forEach(this::addError); + return this; + } + + /** + * Adds an error to the service that is implicitly bound to every operation + * within the closure of the service. + * + * @param errorShapeId Error shape ID to add. + * @return Returns the builder. + */ + public Builder addError(ToShapeId errorShapeId) { + errors.add(errorShapeId.toShapeId()); + return this; + } + + /** + * Adds an error to the service that is implicitly bound to every + * operation within the closure of the service. + * + * @param errorShapeId Error shape ID to add. + * @return Returns the builder. + * @throws ShapeIdSyntaxException if the shape ID is invalid. + */ + public Builder addError(String errorShapeId) { + return addError(ShapeId.from(errorShapeId)); + } + + /** + * Adds errors to the service that are implicitly bound to every operation + * within the closure of the service. + * + * @param errorShapeIds Error shape IDs to add. + * @return Returns the builder. + */ + public Builder addErrors(List errorShapeIds) { + errors.addAll(Objects.requireNonNull(errorShapeIds)); + return this; + } + + /** + * Removes an error by Shape ID. + * + * @param errorShapeId Error shape ID to remove. + * @return Returns the builder. + */ + public Builder removeError(ToShapeId errorShapeId) { + errors.remove(errorShapeId.toShapeId()); + return this; + } + + /** + * Removes all errors. + * @return Returns the builder. + */ + public Builder clearErrors() { + errors.clear(); + return this; + } } } diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/shapes/SmithyIdlModelSerializer.java b/smithy-model/src/main/java/software/amazon/smithy/model/shapes/SmithyIdlModelSerializer.java index a02f8294b8e..502c8d53fd6 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/shapes/SmithyIdlModelSerializer.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/shapes/SmithyIdlModelSerializer.java @@ -432,6 +432,7 @@ public Void serviceShape(ServiceShape shape) { codeWriter.writeOptionalIdList("operations", shape.getOperations()); codeWriter.writeOptionalIdList("resources", shape.getResources()); + codeWriter.writeOptionalIdList("errors", shape.getErrors()); if (!shape.getRename().isEmpty()) { codeWriter.openBlock("rename: {", "}", () -> { for (Map.Entry entry : shape.getRename().entrySet()) { diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/transform/CopyServiceErrorsToOperationsTransform.java b/smithy-model/src/main/java/software/amazon/smithy/model/transform/CopyServiceErrorsToOperationsTransform.java new file mode 100644 index 00000000000..7cca3013fbb --- /dev/null +++ b/smithy-model/src/main/java/software/amazon/smithy/model/transform/CopyServiceErrorsToOperationsTransform.java @@ -0,0 +1,54 @@ +/* + * Copyright 2021 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package software.amazon.smithy.model.transform; + +import java.util.HashSet; +import java.util.LinkedHashSet; +import java.util.Set; +import software.amazon.smithy.model.Model; +import software.amazon.smithy.model.knowledge.TopDownIndex; +import software.amazon.smithy.model.shapes.OperationShape; +import software.amazon.smithy.model.shapes.ServiceShape; +import software.amazon.smithy.model.shapes.Shape; +import software.amazon.smithy.model.shapes.ShapeId; + +/** + * Copies errors from a service onto each operation bound to the service. + */ +final class CopyServiceErrorsToOperationsTransform { + + private final ServiceShape forService; + + CopyServiceErrorsToOperationsTransform(ServiceShape forService) { + this.forService = forService; + } + + Model transform(ModelTransformer transformer, Model model) { + if (forService.getErrors().isEmpty()) { + return model; + } + + Set toReplace = new HashSet<>(); + TopDownIndex topDownIndex = TopDownIndex.of(model); + for (OperationShape operation : topDownIndex.getContainedOperations(forService)) { + Set errors = new LinkedHashSet<>(operation.getErrors()); + errors.addAll(forService.getErrors()); + toReplace.add(operation.toBuilder().errors(errors).build()); + } + + return transformer.replaceShapes(model, toReplace); + } +} diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/transform/ModelTransformer.java b/smithy-model/src/main/java/software/amazon/smithy/model/transform/ModelTransformer.java index 734b1cc5e7b..29860eec10e 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/transform/ModelTransformer.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/transform/ModelTransformer.java @@ -33,6 +33,7 @@ import software.amazon.smithy.model.neighbor.UnreferencedTraitDefinitions; import software.amazon.smithy.model.node.Node; import software.amazon.smithy.model.shapes.MemberShape; +import software.amazon.smithy.model.shapes.ServiceShape; import software.amazon.smithy.model.shapes.Shape; import software.amazon.smithy.model.shapes.ShapeId; import software.amazon.smithy.model.shapes.ShapeType; @@ -478,4 +479,16 @@ public Model sortMembers(Model model, Comparator comparator) { public Model changeShapeType(Model model, Map shapeToType) { return new ChangeShapeType(shapeToType).transform(this, model); } + + /** + * Copies the errors defined on the given service onto each operation bound to the + * service, effectively flattening service error inheritance. + * + * @param model Model to modify. + * @param forService Service shape to use as the basis for copying errors to operations. + * @return Returns the transformed model. + */ + public Model copyServiceErrorsToOperations(Model model, ServiceShape forService) { + return new CopyServiceErrorsToOperationsTransform(forService).transform(this, model); + } } diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/TargetValidator.java b/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/TargetValidator.java index 62863a62ee7..694d64ab664 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/TargetValidator.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/TargetValidator.java @@ -223,7 +223,7 @@ private ValidationEvent inputOutputWithErrorTrait(Shape shape, Shape target, Rel private ValidationEvent errorNoTrait(Shape shape, ShapeId target) { return error(shape, format( - "Operation error targets an invalid structure, `%s`, that is not marked with the `error` trait.", + "`%s` cannot be bound as an error because it is not marked with the `error` trait.", target)); } } diff --git a/smithy-model/src/test/java/software/amazon/smithy/model/knowledge/OperationIndexTest.java b/smithy-model/src/test/java/software/amazon/smithy/model/knowledge/OperationIndexTest.java index e65110541d4..fa2963044b5 100644 --- a/smithy-model/src/test/java/software/amazon/smithy/model/knowledge/OperationIndexTest.java +++ b/smithy-model/src/test/java/software/amazon/smithy/model/knowledge/OperationIndexTest.java @@ -27,6 +27,7 @@ import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import software.amazon.smithy.model.Model; +import software.amazon.smithy.model.shapes.ServiceShape; import software.amazon.smithy.model.shapes.Shape; import software.amazon.smithy.model.shapes.ShapeId; import software.amazon.smithy.model.shapes.StructureShape; @@ -62,12 +63,9 @@ public void indexesOperations() { OperationIndex opIndex = OperationIndex.of(model); Shape input = model.getShape(ShapeId.from("ns.foo#Input")).get(); Shape output = model.getShape(ShapeId.from("ns.foo#Output")).get(); - Shape error1 = model.getShape(ShapeId.from("ns.foo#Error1")).get(); - Shape error2 = model.getShape(ShapeId.from("ns.foo#Error2")).get(); assertThat(opIndex.getInput(ShapeId.from("ns.foo#B")), is(Optional.of(input))); assertThat(opIndex.getOutput(ShapeId.from("ns.foo#B")), is(Optional.of(output))); - assertThat(opIndex.getErrors(ShapeId.from("ns.foo#B")), containsInAnyOrder(error1, error2)); } @Test @@ -107,4 +105,22 @@ public void determinesIfShapeIsUsedAsOutput() { assertThat(opIndex.isOutputStructure(output), is(true)); assertThat(opIndex.isOutputStructure(input), is(false)); } + + @Test + public void getsOperationErrorsAndInheritedErrors() { + OperationIndex opIndex = OperationIndex.of(model); + ShapeId a = ShapeId.from("ns.foo#A"); + ShapeId b = ShapeId.from("ns.foo#B"); + ServiceShape service = model.expectShape(ShapeId.from("ns.foo#MyService"), ServiceShape.class); + StructureShape error1 = model.expectShape(ShapeId.from("ns.foo#Error1"), StructureShape.class); + StructureShape error2 = model.expectShape(ShapeId.from("ns.foo#Error2"), StructureShape.class); + StructureShape common1 = model.expectShape(ShapeId.from("ns.foo#CommonError1"), StructureShape.class); + StructureShape common2 = model.expectShape(ShapeId.from("ns.foo#CommonError2"), StructureShape.class); + + assertThat(opIndex.getErrors(service), containsInAnyOrder(common1, common2)); + assertThat(opIndex.getErrors(a), empty()); + assertThat(opIndex.getErrors(service, a), containsInAnyOrder(common1, common2)); + assertThat(opIndex.getErrors(b), containsInAnyOrder(error1, error2)); + assertThat(opIndex.getErrors(service, b), containsInAnyOrder(error1, error2, common1, common2)); + } } diff --git a/smithy-model/src/test/java/software/amazon/smithy/model/neighbor/NeighborVisitorTest.java b/smithy-model/src/test/java/software/amazon/smithy/model/neighbor/NeighborVisitorTest.java index 2865d2c04cd..8222d37bbe5 100644 --- a/smithy-model/src/test/java/software/amazon/smithy/model/neighbor/NeighborVisitorTest.java +++ b/smithy-model/src/test/java/software/amazon/smithy/model/neighbor/NeighborVisitorTest.java @@ -37,6 +37,7 @@ import software.amazon.smithy.model.shapes.StructureShape; import software.amazon.smithy.model.shapes.TimestampShape; import software.amazon.smithy.model.shapes.UnionShape; +import software.amazon.smithy.model.traits.ErrorTrait; import software.amazon.smithy.model.traits.IdempotentTrait; import software.amazon.smithy.model.traits.ReadonlyTrait; @@ -210,6 +211,25 @@ public void serviceShape() { Relationship.create(operationShape, RelationshipType.BOUND, service))); } + @Test + public void serviceErrors() { + ServiceShape service = ServiceShape.builder() + .id("ns.foo#Svc") + .version("2017-01-17") + .addError("ns.foo#Common1") + .build(); + StructureShape errorShape = StructureShape.builder() + .id("ns.foo#Common1") + .addTrait(new ErrorTrait("client")) + .build(); + Model model = Model.builder().addShapes(service, errorShape).build(); + NeighborVisitor neighborVisitor = new NeighborVisitor(model); + List relationships = service.accept(neighborVisitor); + + assertThat(relationships, contains( + Relationship.create(service, RelationshipType.ERROR, errorShape))); + } + @Test public void resourceShape() { ServiceShape parent = ServiceShape.builder() diff --git a/smithy-model/src/test/java/software/amazon/smithy/model/selector/SelectorTest.java b/smithy-model/src/test/java/software/amazon/smithy/model/selector/SelectorTest.java index 7c721174e2e..c0e2f6d0530 100644 --- a/smithy-model/src/test/java/software/amazon/smithy/model/selector/SelectorTest.java +++ b/smithy-model/src/test/java/software/amazon/smithy/model/selector/SelectorTest.java @@ -42,10 +42,13 @@ import software.amazon.smithy.model.node.NodeMapper; import software.amazon.smithy.model.shapes.ListShape; import software.amazon.smithy.model.shapes.MemberShape; +import software.amazon.smithy.model.shapes.ServiceShape; import software.amazon.smithy.model.shapes.SetShape; import software.amazon.smithy.model.shapes.Shape; import software.amazon.smithy.model.shapes.ShapeId; +import software.amazon.smithy.model.shapes.StructureShape; import software.amazon.smithy.model.traits.DynamicTrait; +import software.amazon.smithy.model.traits.ErrorTrait; import software.amazon.smithy.model.traits.RequiredTrait; import software.amazon.smithy.model.traits.Trait; import software.amazon.smithy.utils.ListUtils; @@ -1026,4 +1029,23 @@ public void returnsStreamOfMatches() { assertThat(matches, equalTo(matches2)); } + + @Test + public void canPathToErrorsOfStructure() { + ServiceShape service = ServiceShape.builder() + .id("ns.foo#Svc") + .version("2017-01-17") + .addError("ns.foo#Common1") + .build(); + StructureShape errorShape = StructureShape.builder() + .id("ns.foo#Common1") + .addTrait(new ErrorTrait("client")) + .build(); + Model model = Model.builder().addShapes(service, errorShape).build(); + + Selector selector = Selector.parse("service -[error]-> structure"); + Set result = selector.select(model); + + assertThat(result, containsInAnyOrder(errorShape)); + } } diff --git a/smithy-model/src/test/java/software/amazon/smithy/model/shapes/ServiceShapeTest.java b/smithy-model/src/test/java/software/amazon/smithy/model/shapes/ServiceShapeTest.java index c38c3270194..d64b95c5b0d 100644 --- a/smithy-model/src/test/java/software/amazon/smithy/model/shapes/ServiceShapeTest.java +++ b/smithy-model/src/test/java/software/amazon/smithy/model/shapes/ServiceShapeTest.java @@ -19,6 +19,7 @@ import static org.hamcrest.Matchers.equalTo; import static org.junit.jupiter.api.Assertions.assertEquals; +import java.util.Arrays; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import software.amazon.smithy.model.SourceException; @@ -67,4 +68,23 @@ public void versionDefaultsToEmptyString() { assertThat(shape.getVersion(), equalTo("")); } + + @Test + public void hasErrors() { + ServiceShape shape = ServiceShape.builder() + .id("com.foo#Example") + .version("x") + .addError("com.foo#Common1") + .addError(ShapeId.from("com.foo#Common2")) + .build(); + + assertThat(shape, equalTo(shape)); + assertThat(shape, equalTo(shape.toBuilder().build())); + + ServiceShape shape2 = shape.toBuilder() + .errors(Arrays.asList(ShapeId.from("com.foo#Common1"), ShapeId.from("com.foo#Common2"))) + .build(); + + assertThat(shape, equalTo(shape2)); + } } diff --git a/smithy-model/src/test/java/software/amazon/smithy/model/transform/CopyServiceErrorsToOperationsTransformTest.java b/smithy-model/src/test/java/software/amazon/smithy/model/transform/CopyServiceErrorsToOperationsTransformTest.java new file mode 100644 index 00000000000..56dd6f140a4 --- /dev/null +++ b/smithy-model/src/test/java/software/amazon/smithy/model/transform/CopyServiceErrorsToOperationsTransformTest.java @@ -0,0 +1,64 @@ +/* + * Copyright 2021 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package software.amazon.smithy.model.transform; + +import static org.hamcrest.MatcherAssert.assertThat; + +import org.hamcrest.Matchers; +import org.junit.jupiter.api.Test; +import software.amazon.smithy.model.Model; +import software.amazon.smithy.model.shapes.OperationShape; +import software.amazon.smithy.model.shapes.ServiceShape; +import software.amazon.smithy.model.shapes.StructureShape; +import software.amazon.smithy.model.traits.ErrorTrait; + +public class CopyServiceErrorsToOperationsTransformTest { + @Test + public void copiesErrors() { + StructureShape errorShape1 = StructureShape.builder() + .id("ns.foo#Error1") + .addTrait(new ErrorTrait("client")) + .build(); + StructureShape errorShape2 = StructureShape.builder() + .id("ns.foo#Error2") + .addTrait(new ErrorTrait("client")) + .build(); + OperationShape operation1 = OperationShape.builder() + .id("smithy.example#Operation1") + .addError(errorShape1) + .build(); + OperationShape operation2 = OperationShape.builder() + .id("smithy.example#Operation2") + .build(); + ServiceShape service = ServiceShape.builder() + .id("ns.foo#Svc") + .version("2017-01-17") + .addError(errorShape2) + .addOperation(operation1) + .build(); + Model model = Model.builder() + .addShapes(service, errorShape1, errorShape2, operation1, operation2) + .build(); + Model result = ModelTransformer.create().copyServiceErrorsToOperations(model, service); + + // operation2 is not in the service closure so leave it alone. + assertThat(operation2, Matchers.equalTo(result.expectShape(operation2.getId()))); + + // Make sure service errors were copied to the operation bound within it. + assertThat(result.expectShape(operation1.getId(), OperationShape.class).getErrors(), + Matchers.containsInAnyOrder(errorShape1.getId(), errorShape2.getId())); + } +} diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/target-validator-service-errors.errors b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/target-validator-service-errors.errors new file mode 100644 index 00000000000..ff9ffdcd767 --- /dev/null +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/target-validator-service-errors.errors @@ -0,0 +1,3 @@ +[ERROR] ns.foo#InvalidService1: service shape has an `error` relationship to an unresolved shape `ns.foo#NotFound` | Target +[ERROR] ns.foo#InvalidService2: service shape `error` relationships must target a structure shape, but found (string: `smithy.api#String`) | Target +[ERROR] ns.foo#InvalidService3: `ns.foo#NotError` cannot be bound as an error because it is not marked with the `error` trait. | Target diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/target-validator-service-errors.json b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/target-validator-service-errors.json new file mode 100644 index 00000000000..97610bc1cf6 --- /dev/null +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/target-validator-service-errors.json @@ -0,0 +1,35 @@ +{ + "smithy": "1.0", + "shapes": { + "ns.foo#InvalidService1": { + "type": "service", + "version": "x", + "errors": [ + { + "target": "ns.foo#NotFound" + } + ] + }, + "ns.foo#InvalidService2": { + "type": "service", + "version": "x", + "errors": [ + { + "target": "smithy.api#String" + } + ] + }, + "ns.foo#InvalidService3": { + "type": "service", + "version": "x", + "errors": [ + { + "target": "ns.foo#NotError" + } + ] + }, + "ns.foo#NotError": { + "type": "structure" + } + } +} diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/target-validator.errors b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/target-validator.errors index 0b448b86ca8..71aaedc4e21 100644 --- a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/target-validator.errors +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/target-validator.errors @@ -9,7 +9,7 @@ [ERROR] ns.foo#InvalidListMemberResource$member: Members cannot target resource shapes, but found (resource: `ns.foo#MyResource`) | Target [ERROR] ns.foo#InvalidListMemberService$member: Members cannot target service shapes, but found (service: `ns.foo#MyService`) | Target [ERROR] ns.foo#InvalidMapType: Map key member targets (structure: `ns.foo#ValidInput`), but is expected to target a string | Target -[ERROR] ns.foo#InvalidOperationBadErrorTraits: Operation error targets an invalid structure, `ns.foo#ValidInput`, that is not marked with the `error` trait. | Target +[ERROR] ns.foo#InvalidOperationBadErrorTraits: `ns.foo#ValidInput` cannot be bound as an error because it is not marked with the `error` trait. | Target [ERROR] ns.foo#InvalidOperationBadErrorTraits: Operation input targets an invalid structure `ns.foo#ValidError` that is marked with the `error` trait. | Target [ERROR] ns.foo#InvalidOperationBadErrorTraits: Operation output targets an invalid structure `ns.foo#ValidError` that is marked with the `error` trait. | Target [ERROR] ns.foo#InvalidResourceBindingReference: resource shape has a `resource` relationship to an unresolved shape `ns.foo#NotFound` | Target diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/knowledge/operation-index-test.json b/smithy-model/src/test/resources/software/amazon/smithy/model/knowledge/operation-index-test.json index 330135ea481..8bfc614c2ea 100644 --- a/smithy-model/src/test/resources/software/amazon/smithy/model/knowledge/operation-index-test.json +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/knowledge/operation-index-test.json @@ -11,6 +11,14 @@ { "target": "ns.foo#B" } + ], + "errors": [ + { + "target": "ns.foo#CommonError1" + }, + { + "target": "ns.foo#CommonError2" + } ] }, "ns.foo#A": { @@ -56,6 +64,18 @@ "traits": { "smithy.api#error": "server" } + }, + "ns.foo#CommonError1": { + "type": "structure", + "traits": { + "smithy.api#error": "server" + } + }, + "ns.foo#CommonError2": { + "type": "structure", + "traits": { + "smithy.api#error": "server" + } } } } diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/loader/valid/service-with-errors.json b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/valid/service-with-errors.json new file mode 100644 index 00000000000..50fe446f7cf --- /dev/null +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/valid/service-with-errors.json @@ -0,0 +1,31 @@ +{ + "smithy": "1.0", + "shapes": { + "ns.foo#Common1": { + "type": "structure", + "members": {}, + "traits": { + "smithy.api#error": "client" + } + }, + "ns.foo#Common2": { + "type": "structure", + "members": {}, + "traits": { + "smithy.api#error": "server" + } + }, + "ns.foo#EmptyService": { + "type": "service", + "version": "2020-02-18", + "errors": [ + { + "target": "ns.foo#Common1" + }, + { + "target": "ns.foo#Common2" + } + ] + } + } +} diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/loader/valid/service-with-errors.smithy b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/valid/service-with-errors.smithy new file mode 100644 index 00000000000..43611b7bb32 --- /dev/null +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/valid/service-with-errors.smithy @@ -0,0 +1,17 @@ +$version: "1.0" + +namespace ns.foo + +service EmptyService { + version: "2020-02-18", + errors: [ + Common1, + Common2, + ], +} + +@error("client") +structure Common1 {} + +@error("server") +structure Common2 {} diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/shapes/idl-serialization/cases/service-errors.smithy b/smithy-model/src/test/resources/software/amazon/smithy/model/shapes/idl-serialization/cases/service-errors.smithy new file mode 100644 index 00000000000..43611b7bb32 --- /dev/null +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/shapes/idl-serialization/cases/service-errors.smithy @@ -0,0 +1,17 @@ +$version: "1.0" + +namespace ns.foo + +service EmptyService { + version: "2020-02-18", + errors: [ + Common1, + Common2, + ], +} + +@error("client") +structure Common1 {} + +@error("server") +structure Common2 {} diff --git a/smithy-openapi/src/main/java/software/amazon/smithy/openapi/fromsmithy/OpenApiConverter.java b/smithy-openapi/src/main/java/software/amazon/smithy/openapi/fromsmithy/OpenApiConverter.java index 36055458770..1dd4c7e1905 100644 --- a/smithy-openapi/src/main/java/software/amazon/smithy/openapi/fromsmithy/OpenApiConverter.java +++ b/smithy-openapi/src/main/java/software/amazon/smithy/openapi/fromsmithy/OpenApiConverter.java @@ -47,6 +47,7 @@ import software.amazon.smithy.model.traits.DocumentationTrait; import software.amazon.smithy.model.traits.TitleTrait; import software.amazon.smithy.model.traits.Trait; +import software.amazon.smithy.model.transform.ModelTransformer; import software.amazon.smithy.model.validation.ValidationUtils; import software.amazon.smithy.openapi.OpenApiConfig; import software.amazon.smithy.openapi.OpenApiException; @@ -173,6 +174,18 @@ private ConversionEnvironment createConversionEnvironment(Model throw new OpenApiException("openapi is missing required property, `service`"); } + // Find the service shape. + ServiceShape service = model.getShape(serviceShapeId) + .orElseThrow(() -> new IllegalArgumentException(String.format( + "Shape `%s` not found in model", serviceShapeId))) + .asServiceShape() + .orElseThrow(() -> new IllegalArgumentException(String.format( + "Shape `%s` is not a service shape", serviceShapeId))); + + // Copy service errors onto each operation to ensure that common errors are + // generated for each operation. + model = ModelTransformer.create().copyServiceErrorsToOperations(model, service); + JsonSchemaConverter.Builder jsonSchemaConverterBuilder = JsonSchemaConverter.builder(); jsonSchemaConverterBuilder.model(model); @@ -187,14 +200,6 @@ private ConversionEnvironment createConversionEnvironment(Model } } - // Find the service shape. - ServiceShape service = model.getShape(serviceShapeId) - .orElseThrow(() -> new IllegalArgumentException(String.format( - "Shape `%s` not found in model", serviceShapeId))) - .asServiceShape() - .orElseThrow(() -> new IllegalArgumentException(String.format( - "Shape `%s` is not a service shape", serviceShapeId))); - Trait protocolTrait = loadOrDeriveProtocolTrait(model, service); OpenApiProtocol openApiProtocol = loadOpenApiProtocol(service, protocolTrait, extensions); diff --git a/smithy-openapi/src/test/java/software/amazon/smithy/openapi/fromsmithy/OpenApiConverterTest.java b/smithy-openapi/src/test/java/software/amazon/smithy/openapi/fromsmithy/OpenApiConverterTest.java index bee1e95d603..2fec7bbc3a8 100644 --- a/smithy-openapi/src/test/java/software/amazon/smithy/openapi/fromsmithy/OpenApiConverterTest.java +++ b/smithy-openapi/src/test/java/software/amazon/smithy/openapi/fromsmithy/OpenApiConverterTest.java @@ -498,4 +498,20 @@ public void properlyDealsWithServiceRenames() { Node.assertEquals(result, expectedNode); } + + @Test + public void generatesOpenApiForSharedErrors() { + Model model = Model.assembler() + .addImport(getClass().getResource("service-with-common-errors.json")) + .discoverModels() + .assemble() + .unwrap(); + OpenApiConfig config = new OpenApiConfig(); + config.setService(ShapeId.from("smithy.example#MyService")); + Node result = OpenApiConverter.create().config(config).convertToNode(model); + Node expectedNode = Node.parse(IoUtils.toUtf8String( + getClass().getResourceAsStream("service-with-common-errors.openapi.json"))); + + Node.assertEquals(result, expectedNode); + } } diff --git a/smithy-openapi/src/test/resources/software/amazon/smithy/openapi/fromsmithy/service-with-common-errors.json b/smithy-openapi/src/test/resources/software/amazon/smithy/openapi/fromsmithy/service-with-common-errors.json new file mode 100644 index 00000000000..4601982399f --- /dev/null +++ b/smithy-openapi/src/test/resources/software/amazon/smithy/openapi/fromsmithy/service-with-common-errors.json @@ -0,0 +1,43 @@ +{ + "smithy": "1.0", + "shapes": { + "smithy.example#MyService": { + "type": "service", + "version": "2017-02-11", + "operations": [ + { + "target": "smithy.example#GetSomething" + } + ], + "errors": [ + { + "target": "smithy.example#MyError" + } + ], + "traits": { + "aws.protocols#restJson1": {} + } + }, + "smithy.example#GetSomething": { + "type": "operation", + "output": { + "target": "smithy.example#GetSomethingOutput" + }, + "traits": { + "smithy.api#http": { + "method": "GET", + "uri": "/" + } + } + }, + "smithy.example#GetSomethingOutput": { + "type": "structure" + }, + "smithy.example#MyError": { + "type": "structure", + "traits": { + "smithy.api#error": "client" + } + } + } +} diff --git a/smithy-openapi/src/test/resources/software/amazon/smithy/openapi/fromsmithy/service-with-common-errors.openapi.json b/smithy-openapi/src/test/resources/software/amazon/smithy/openapi/fromsmithy/service-with-common-errors.openapi.json new file mode 100644 index 00000000000..bcad7ced124 --- /dev/null +++ b/smithy-openapi/src/test/resources/software/amazon/smithy/openapi/fromsmithy/service-with-common-errors.openapi.json @@ -0,0 +1,22 @@ +{ + "openapi": "3.0.2", + "info": { + "title": "MyService", + "version": "2017-02-11" + }, + "paths": { + "/": { + "get": { + "operationId": "GetSomething", + "responses": { + "200": { + "description": "GetSomething 200 response" + }, + "400": { + "description": "MyError 400 response" + } + } + } + } + } +}