diff --git a/docs/source-1.0/spec/aws/aws-ec2-query-protocol.rst b/docs/source-1.0/spec/aws/aws-ec2-query-protocol.rst index 0263e2e0e38..460fe8cd993 100644 --- a/docs/source-1.0/spec/aws/aws-ec2-query-protocol.rst +++ b/docs/source-1.0/spec/aws/aws-ec2-query-protocol.rst @@ -394,3 +394,5 @@ the expected serialized HTTP requests and responses for each case. *TODO: Add specifications, protocol examples, etc.* + +.. include:: error-rename.rst.template diff --git a/docs/source-1.0/spec/aws/aws-json-1_0-protocol.rst b/docs/source-1.0/spec/aws/aws-json-1_0-protocol.rst index bbb3e38b22c..c4f48c29130 100644 --- a/docs/source-1.0/spec/aws/aws-json-1_0-protocol.rst +++ b/docs/source-1.0/spec/aws/aws-json-1_0-protocol.rst @@ -136,5 +136,6 @@ The following example defines a service that requires the use of .. |protocol error type contents| replace:: :ref:`shape-id` .. |protocol test link| replace:: https://github.com/awslabs/smithy/tree/main/smithy-aws-protocol-tests/model/awsJson1_0 .. include:: aws-json.rst.template +.. include:: error-rename-simple.rst.template .. _`Application-Layer Protocol Negotiation (ALPN) Protocol ID`: https://www.iana.org/assignments/tls-extensiontype-values/tls-extensiontype-values.xhtml#alpn-protocol-ids diff --git a/docs/source-1.0/spec/aws/aws-json-1_1-protocol.rst b/docs/source-1.0/spec/aws/aws-json-1_1-protocol.rst index ceff770e895..0d61d9174da 100644 --- a/docs/source-1.0/spec/aws/aws-json-1_1-protocol.rst +++ b/docs/source-1.0/spec/aws/aws-json-1_1-protocol.rst @@ -136,5 +136,6 @@ The following example defines a service that requires the use of .. |protocol error type contents| replace:: :token:`shape name ` .. |protocol test link| replace:: https://github.com/awslabs/smithy/tree/main/smithy-aws-protocol-tests/model/awsJson1_1 .. include:: aws-json.rst.template +.. include:: error-rename.rst.template .. _`Application-Layer Protocol Negotiation (ALPN) Protocol ID`: https://www.iana.org/assignments/tls-extensiontype-values/tls-extensiontype-values.xhtml#alpn-protocol-ids diff --git a/docs/source-1.0/spec/aws/aws-query-protocol.rst b/docs/source-1.0/spec/aws/aws-query-protocol.rst index b88bc6f7382..cdfd7b8d67b 100644 --- a/docs/source-1.0/spec/aws/aws-query-protocol.rst +++ b/docs/source-1.0/spec/aws/aws-query-protocol.rst @@ -635,3 +635,5 @@ reference: https://github.com/awslabs/smithy/tree/main/smithy-aws-protocol-tests These compliance tests define a model that is used to define test cases and the expected serialized HTTP requests and responses for each case. + +.. include:: error-rename.rst.template diff --git a/docs/source-1.0/spec/aws/aws-restjson1-protocol.rst b/docs/source-1.0/spec/aws/aws-restjson1-protocol.rst index 5e513180bd5..d258b717f3b 100644 --- a/docs/source-1.0/spec/aws/aws-restjson1-protocol.rst +++ b/docs/source-1.0/spec/aws/aws-restjson1-protocol.rst @@ -352,3 +352,5 @@ the expected serialized HTTP requests and responses for each case. *TODO: Add event stream handling specifications.* .. _`Application-Layer Protocol Negotiation (ALPN) Protocol ID`: https://www.iana.org/assignments/tls-extensiontype-values/tls-extensiontype-values.xhtml#alpn-protocol-ids + +.. include:: error-rename.rst.template diff --git a/docs/source-1.0/spec/aws/aws-restxml-protocol.rst b/docs/source-1.0/spec/aws/aws-restxml-protocol.rst index 736a0447bf3..10e7bf85c15 100644 --- a/docs/source-1.0/spec/aws/aws-restxml-protocol.rst +++ b/docs/source-1.0/spec/aws/aws-restxml-protocol.rst @@ -316,3 +316,5 @@ the expected serialized HTTP requests and responses for each case. *TODO: Add event stream handling specifications.* .. _`Application-Layer Protocol Negotiation (ALPN) Protocol ID`: https://www.iana.org/assignments/tls-extensiontype-values/tls-extensiontype-values.xhtml#alpn-protocol-ids + +.. include:: error-rename.rst.template diff --git a/docs/source-1.0/spec/aws/error-rename-simple.rst.template b/docs/source-1.0/spec/aws/error-rename-simple.rst.template new file mode 100644 index 00000000000..13c1f051c4e --- /dev/null +++ b/docs/source-1.0/spec/aws/error-rename-simple.rst.template @@ -0,0 +1,7 @@ +-------------------- +Error shape renaming +-------------------- + +By default, Smithy permits renaming shapes to disambiguate shape ID conflicts in +the :ref:`service closure ` via the ``rename`` property. However, services using this protocol are +not allowed to rename error shapes (shapes with :ref:`error trait ` applied). diff --git a/docs/source-1.0/spec/aws/error-rename.rst.template b/docs/source-1.0/spec/aws/error-rename.rst.template new file mode 100644 index 00000000000..ef3172aa867 --- /dev/null +++ b/docs/source-1.0/spec/aws/error-rename.rst.template @@ -0,0 +1,17 @@ +-------------------- +Error shape renaming +-------------------- + +By default, Smithy permits renaming shapes to disambiguate shape ID conflicts in +the :ref:`service closure ` via the ``rename`` property. However, services using this protocol are +not allowed to rename error shapes (shapes with :ref:`error trait ` applied). + +Client-side implementations rely on the response body field ``code`` or ``__type`` to resolve the error type. +Server-side implementations of this protocol will only send the ``shape name`` for the response body field. + +When there are conflicting shape IDs ``smithy.service#ServiceError`` and ``smithy.other#ServiceError``, +the server will only send the shape name ``ServiceError``. Clients will not be able to resolve +the correct error type without the namespace. + +Server-side implementations of this protocol don't serialize renamed shape names. +As a result, renaming will not resolve the conflicting shape IDs issue, and hence it is not permitted. diff --git a/docs/source-1.0/spec/core/model.rst b/docs/source-1.0/spec/core/model.rst index a9f601d87a8..2cf4c88ea90 100644 --- a/docs/source-1.0/spec/core/model.rst +++ b/docs/source-1.0/spec/core/model.rst @@ -1141,9 +1141,9 @@ The service shape supports the following properties: shape name or the name of a non-renamed shape contained in the service. * Member shapes MAY NOT be renamed. - * Resource, operation, and shapes marked with the :ref:`error-trait` - MAY NOT be renamed. Renaming shapes is intended for incidental naming - conflicts, not for renaming the fundamental concepts of a service. + * Resource and operation shapes MAY NOT be renamed. Renaming shapes is intended + for incidental naming conflicts, not for renaming the fundamental concepts + of a service. * Shapes from other namespaces marked as :ref:`private ` MAY be renamed. * A rename MUST use a name that is case-sensitively different from the diff --git a/docs/source-2.0/aws/protocols/aws-ec2-query-protocol.rst b/docs/source-2.0/aws/protocols/aws-ec2-query-protocol.rst index f4209e9e142..3104874d1ff 100644 --- a/docs/source-2.0/aws/protocols/aws-ec2-query-protocol.rst +++ b/docs/source-2.0/aws/protocols/aws-ec2-query-protocol.rst @@ -360,3 +360,5 @@ the expected serialized HTTP requests and responses for each case. *TODO: Add specifications, protocol examples, etc.* + +.. include:: error-rename.rst.template diff --git a/docs/source-2.0/aws/protocols/aws-json-1_0-protocol.rst b/docs/source-2.0/aws/protocols/aws-json-1_0-protocol.rst index 147f97ddca4..79ea6319cd7 100644 --- a/docs/source-2.0/aws/protocols/aws-json-1_0-protocol.rst +++ b/docs/source-2.0/aws/protocols/aws-json-1_0-protocol.rst @@ -127,5 +127,6 @@ The following example defines a service that requires the use of .. |protocol error type contents| replace:: :ref:`shape-id` .. |protocol test link| replace:: https://github.com/awslabs/smithy/tree/main/smithy-aws-protocol-tests/model/awsJson1_0 .. include:: aws-json.rst.template +.. include:: error-rename-simple.rst.template .. _`Application-Layer Protocol Negotiation (ALPN) Protocol ID`: https://www.iana.org/assignments/tls-extensiontype-values/tls-extensiontype-values.xhtml#alpn-protocol-ids diff --git a/docs/source-2.0/aws/protocols/aws-json-1_1-protocol.rst b/docs/source-2.0/aws/protocols/aws-json-1_1-protocol.rst index 796ca734720..022056a1b3e 100644 --- a/docs/source-2.0/aws/protocols/aws-json-1_1-protocol.rst +++ b/docs/source-2.0/aws/protocols/aws-json-1_1-protocol.rst @@ -127,5 +127,6 @@ The following example defines a service that requires the use of .. |protocol error type contents| replace:: :token:`shape name ` .. |protocol test link| replace:: https://github.com/awslabs/smithy/tree/main/smithy-aws-protocol-tests/model/awsJson1_1 .. include:: aws-json.rst.template +.. include:: error-rename.rst.template .. _`Application-Layer Protocol Negotiation (ALPN) Protocol ID`: https://www.iana.org/assignments/tls-extensiontype-values/tls-extensiontype-values.xhtml#alpn-protocol-ids diff --git a/docs/source-2.0/aws/protocols/aws-query-protocol.rst b/docs/source-2.0/aws/protocols/aws-query-protocol.rst index 2e5f4ca2947..5094454bb86 100644 --- a/docs/source-2.0/aws/protocols/aws-query-protocol.rst +++ b/docs/source-2.0/aws/protocols/aws-query-protocol.rst @@ -596,3 +596,5 @@ reference: https://github.com/awslabs/smithy/tree/main/smithy-aws-protocol-tests These compliance tests define a model that is used to define test cases and the expected serialized HTTP requests and responses for each case. + +.. include:: error-rename.rst.template diff --git a/docs/source-2.0/aws/protocols/aws-restjson1-protocol.rst b/docs/source-2.0/aws/protocols/aws-restjson1-protocol.rst index 730c8e8cb8a..122dffa96c9 100644 --- a/docs/source-2.0/aws/protocols/aws-restjson1-protocol.rst +++ b/docs/source-2.0/aws/protocols/aws-restjson1-protocol.rst @@ -348,3 +348,5 @@ the expected serialized HTTP requests and responses for each case. *TODO: Add event stream handling specifications.* .. _`Application-Layer Protocol Negotiation (ALPN) Protocol ID`: https://www.iana.org/assignments/tls-extensiontype-values/tls-extensiontype-values.xhtml#alpn-protocol-ids + +.. include:: error-rename.rst.template diff --git a/docs/source-2.0/aws/protocols/aws-restxml-protocol.rst b/docs/source-2.0/aws/protocols/aws-restxml-protocol.rst index 210fb9a78f5..1ab4721ff07 100644 --- a/docs/source-2.0/aws/protocols/aws-restxml-protocol.rst +++ b/docs/source-2.0/aws/protocols/aws-restxml-protocol.rst @@ -314,3 +314,5 @@ the expected serialized HTTP requests and responses for each case. *TODO: Add event stream handling specifications.* .. _`Application-Layer Protocol Negotiation (ALPN) Protocol ID`: https://www.iana.org/assignments/tls-extensiontype-values/tls-extensiontype-values.xhtml#alpn-protocol-ids + +.. include:: error-rename.rst.template diff --git a/docs/source-2.0/aws/protocols/error-rename-simple.rst.template b/docs/source-2.0/aws/protocols/error-rename-simple.rst.template new file mode 100644 index 00000000000..13c1f051c4e --- /dev/null +++ b/docs/source-2.0/aws/protocols/error-rename-simple.rst.template @@ -0,0 +1,7 @@ +-------------------- +Error shape renaming +-------------------- + +By default, Smithy permits renaming shapes to disambiguate shape ID conflicts in +the :ref:`service closure ` via the ``rename`` property. However, services using this protocol are +not allowed to rename error shapes (shapes with :ref:`error trait ` applied). diff --git a/docs/source-2.0/aws/protocols/error-rename.rst.template b/docs/source-2.0/aws/protocols/error-rename.rst.template new file mode 100644 index 00000000000..ef3172aa867 --- /dev/null +++ b/docs/source-2.0/aws/protocols/error-rename.rst.template @@ -0,0 +1,17 @@ +-------------------- +Error shape renaming +-------------------- + +By default, Smithy permits renaming shapes to disambiguate shape ID conflicts in +the :ref:`service closure ` via the ``rename`` property. However, services using this protocol are +not allowed to rename error shapes (shapes with :ref:`error trait ` applied). + +Client-side implementations rely on the response body field ``code`` or ``__type`` to resolve the error type. +Server-side implementations of this protocol will only send the ``shape name`` for the response body field. + +When there are conflicting shape IDs ``smithy.service#ServiceError`` and ``smithy.other#ServiceError``, +the server will only send the shape name ``ServiceError``. Clients will not be able to resolve +the correct error type without the namespace. + +Server-side implementations of this protocol don't serialize renamed shape names. +As a result, renaming will not resolve the conflicting shape IDs issue, and hence it is not permitted. diff --git a/docs/source-2.0/spec/service-types.rst b/docs/source-2.0/spec/service-types.rst index fcf0fe28f7b..f41321b39b7 100644 --- a/docs/source-2.0/spec/service-types.rst +++ b/docs/source-2.0/spec/service-types.rst @@ -62,9 +62,9 @@ The service shape supports the following properties: shape name or the name of a non-renamed shape contained in the service. * Member shapes MAY NOT be renamed. - * Resource, operation, and shapes marked with the :ref:`error-trait` - MAY NOT be renamed. Renaming shapes is intended for incidental naming - conflicts, not for renaming the fundamental concepts of a service. + * Resource and operation shapes MAY NOT be renamed. Renaming shapes is intended + for incidental naming conflicts, not for renaming the fundamental concepts + of a service. * Shapes from other namespaces marked as :ref:`private ` MAY be renamed. * A rename MUST use a name that is case-sensitively different from the diff --git a/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/ErrorRenameValidator.java b/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/ErrorRenameValidator.java new file mode 100644 index 00000000000..a78b959e8ec --- /dev/null +++ b/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/ErrorRenameValidator.java @@ -0,0 +1,102 @@ +/* + * Copyright 2022 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.aws.traits; + +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import software.amazon.smithy.aws.traits.protocols.AwsJson1_0Trait; +import software.amazon.smithy.aws.traits.protocols.AwsJson1_1Trait; +import software.amazon.smithy.aws.traits.protocols.AwsQueryTrait; +import software.amazon.smithy.aws.traits.protocols.Ec2QueryTrait; +import software.amazon.smithy.aws.traits.protocols.RestJson1Trait; +import software.amazon.smithy.aws.traits.protocols.RestXmlTrait; +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.traits.ErrorTrait; +import software.amazon.smithy.model.validation.AbstractValidator; +import software.amazon.smithy.model.validation.ValidationEvent; +import software.amazon.smithy.utils.SetUtils; +import software.amazon.smithy.utils.SmithyInternalApi; + +@SmithyInternalApi +public final class ErrorRenameValidator extends AbstractValidator { + + /** + * By default, server-side implementation of AWS Protocols send the error shape ID on the wire, + * and clients can use it to resolve the error type. + * + * However, some protocols implementations, including AWS JSON 1.1, only send the shape name. + * If there are conflicting shape IDs e.g. smithy.service#ServiceError and smithy.other#ServiceError, + * clients are unable to resolve the error type with the shape name alone. + * + * In addition, Server-side implementation of these protocols don't handle or send renamed shape names. + * + * Hence, error shape renaming are not supported for these protocols. + */ + private static final Set UNSUPPORTED_PROTOCOLS = SetUtils.of( + AwsJson1_0Trait.ID, AwsJson1_1Trait.ID, AwsQueryTrait.ID, + Ec2QueryTrait.ID, RestJson1Trait.ID, RestXmlTrait.ID + ); + + @Override + public List validate(Model model) { + List events = new ArrayList<>(); + for (ServiceShape shape : model.getServiceShapes()) { + validate(model, shape, events); + } + return events; + } + + private void validate(Model model, ServiceShape service, List events) { + final Map renames = service.getRename(); + + if (renames.isEmpty()) { + return; + } + + Set unsupportedProtocols = new HashSet<>(); + for (ShapeId protocol: UNSUPPORTED_PROTOCOLS) { + if (service.getAllTraits().containsKey(protocol)) { + unsupportedProtocols.add(protocol.getName()); + } + } + + if (unsupportedProtocols.isEmpty()) { + return; + } + + renames.keySet().forEach(shapeId -> { + Optional shape = model.getShape(shapeId); + + if (!shape.isPresent() || !shape.get().hasTrait(ErrorTrait.class)) { + return; + } + + ShapeId from = shape.get().getId(); + String to = renames.get(from); + events.add(error(service, String.format( + "Service attempts to rename an error shape from `%s` to \"%s\"; " + + "Service protocols %s do not support error renaming.", + from, to, unsupportedProtocols))); + }); + } +} diff --git a/smithy-aws-traits/src/main/resources/META-INF/services/software.amazon.smithy.model.validation.Validator b/smithy-aws-traits/src/main/resources/META-INF/services/software.amazon.smithy.model.validation.Validator index b71e3048f79..eba9e65bd3f 100644 --- a/smithy-aws-traits/src/main/resources/META-INF/services/software.amazon.smithy.model.validation.Validator +++ b/smithy-aws-traits/src/main/resources/META-INF/services/software.amazon.smithy.model.validation.Validator @@ -10,3 +10,4 @@ software.amazon.smithy.aws.traits.tagging.TagEnabledServiceValidator software.amazon.smithy.aws.traits.tagging.TaggableResourceValidator software.amazon.smithy.aws.traits.tagging.TagResourcePropertyTypeValidator software.amazon.smithy.aws.traits.tagging.TagResourcePropertyNameValidator +software.amazon.smithy.aws.traits.ErrorRenameValidator diff --git a/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/error-rename.errors b/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/error-rename.errors new file mode 100644 index 00000000000..947001206a3 --- /dev/null +++ b/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/error-rename.errors @@ -0,0 +1,2 @@ +[ERROR] smithy.example#MyService: Service attempts to rename an error shape from `smithy.example#BadGreeting` to "ThisDoesNotWork"; Service protocols [restXml, awsJson1_1] do not support error renaming. | ErrorRename +[ERROR] smithy.example#MyService: Service attempts to rename an error shape from `smithy.example#ServerError` to "ServerDown"; Service protocols [restXml, awsJson1_1] do not support error renaming. | ErrorRename diff --git a/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/error-rename.smithy b/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/error-rename.smithy new file mode 100644 index 00000000000..5ea61919be5 --- /dev/null +++ b/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/error-rename.smithy @@ -0,0 +1,42 @@ +$version: "2.0" + +namespace smithy.example + +use aws.protocols#awsJson1_1 +use aws.protocols#restXml + +@awsJson1_1 +@restXml +service MyService { + version: "1", + operations: [ + SayHello, + ], + rename: { + "smithy.example#BadGreeting": "ThisDoesNotWork" + "smithy.example#ServerError": "ServerDown" + "smithy.example#Language": "LanguageSettings" + } +} + +operation SayHello { + input: SayHelloInput, + output: SayHelloOutput, + errors: [BadGreeting, ServerError] +} + +@input +structure SayHelloInput { + language: Language +} + +@output +structure SayHelloOutput {} + +@error("client") +structure BadGreeting {} + +@error("server") +structure ServerError {} + +structure Language {} diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/ServiceValidator.java b/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/ServiceValidator.java index 00296706aa9..10e75e79f11 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/ServiceValidator.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/ServiceValidator.java @@ -35,7 +35,6 @@ import software.amazon.smithy.model.shapes.Shape; import software.amazon.smithy.model.shapes.ShapeId; import software.amazon.smithy.model.shapes.SimpleShape; -import software.amazon.smithy.model.traits.ErrorTrait; import software.amazon.smithy.model.traits.Trait; import software.amazon.smithy.model.validation.AbstractValidator; import software.amazon.smithy.model.validation.Severity; @@ -156,8 +155,6 @@ private List validateRenames(ServiceShape service, Map getInvalidRenameReason(Shape shape) { if (shape.isMemberShape() || shape.isResourceShape() || shape.isOperationShape()) { return Optional.of(shape.getType() + "s cannot be renamed"); - } else if (shape.hasTrait(ErrorTrait.class)) { - return Optional.of("errors cannot be renamed"); } else { return Optional.empty(); } diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/service-rename/error-rename-invalid.errors b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/service-rename/error-rename-invalid.errors index fa8cf477639..e69de29bb2d 100644 --- a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/service-rename/error-rename-invalid.errors +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/service-rename/error-rename-invalid.errors @@ -1 +0,0 @@ -[ERROR] smithy.example#MyService: Service attempts to rename a structure shape from `smithy.example#BadGreeting` to "ThisDoesNotWork"; errors cannot be renamed | Service