Skip to content

Commit

Permalink
Allow common errors to be bound to a service
Browse files Browse the repository at this point in the history
This commit allows a common list of errors to be bound to a service
shape so that every operation within the closure of the service
implicitly can return the common errors. This is a very common pattern,
and this change makes it easier to add common errors without needing
things like validation to enforce them being added to every operation.

Existing code generation tooling will either need to be updated to
perform a model transform that copies common errors onto each operation
bound within a service (see `copyServiceErrorsToOperations`), or they
need to use the introduced `getErrors` method on `OperationIndex` that
also takes in a service shape ID.

Closes #894
  • Loading branch information
mtdowling committed Sep 24, 2021
1 parent da88fe8 commit 3065ea3
Show file tree
Hide file tree
Showing 29 changed files with 637 additions and 29 deletions.
11 changes: 11 additions & 0 deletions docs/source/1.0/spec/core/json-ast.rst
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,12 @@ shapes defined in JSON support the same properties as the Smithy IDL.
- [:ref:`AST shape reference <ast-shape-reference>`]
- Binds a list of resources to the service. Each reference MUST target
a resource.
* - errors
- [:ref:`AST shape reference <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 <structure>` shape that is marked with the
:ref:`error-trait`.
* - traits
- map of :ref:`shape ID <shape-id>` to trait values
- Traits to apply to the service
Expand All @@ -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"
},
Expand Down
47 changes: 47 additions & 0 deletions docs/source/1.0/spec/core/model.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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 <shape-id>` that targets
a :ref:`resource <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 <structure>` shape that is marked with the
:ref:`error-trait`.
* - rename
- map of :ref:`shape ID <shape-id>` to ``string``
- Disambiguates shape name conflicts in the
Expand Down Expand Up @@ -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:

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<StructureShape> 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<ShapeId> errorShapeIds) {
List<StructureShape> 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) {
Expand Down Expand Up @@ -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<StructureShape> 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.
*
* <p>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<StructureShape> getErrors(ToShapeId service, ToShapeId operation) {
Set<StructureShape> result = new TreeSet<>(getErrors(service));
result.addAll(getErrors(operation));
return new ArrayList<>(result);
}

private Optional<StructureShape> getStructure(Model model, ToShapeId id) {
return model.getShape(id.toShapeId()).flatMap(Shape::asStructureShape);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> TOP_LEVEL_PROPERTIES = ListUtils.of("smithy", SHAPES, METADATA);
private static final List<String> APPLY_PROPERTIES = ListUtils.of(TYPE, TRAITS);
Expand All @@ -78,12 +79,12 @@ enum AstModelLoader {
private static final Set<String> MEMBER_PROPERTIES = SetUtils.of(TARGET, TRAITS);
private static final Set<String> REFERENCE_PROPERTIES = SetUtils.of(TARGET);
private static final Set<String> OPERATION_PROPERTY_NAMES = SetUtils.of(
TYPE, "input", "output", "errors", TRAITS);
TYPE, "input", "output", ERRORS, TRAITS);
private static final Set<String> RESOURCE_PROPERTIES = SetUtils.of(
TYPE, "create", "read", "update", "delete", "list", "put",
"identifiers", "resources", "operations", "collectionOperations", TRAITS);
private static final Set<String> 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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> RESOURCE_PROPERTY_NAMES = ListUtils.of(
TYPE_KEY, CREATE_KEY, READ_KEY, UPDATE_KEY, DELETE_KEY, LIST_KEY,
Expand Down Expand Up @@ -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) {
Expand All @@ -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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,10 @@ public List<Relationship> 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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ public Builder addError(String errorShapeId) {
* @param errorShapeIds Error shape IDs to add.
* @return Returns the builder.
*/
public Builder addErrors(List<ShapeId> errorShapeIds) {
public Builder addErrors(Collection<ShapeId> errorShapeIds) {
errors.addAll(Objects.requireNonNull(errorShapeIds));
return this;
}
Expand Down
Loading

0 comments on commit 3065ea3

Please sign in to comment.