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..ba68b03a3a6 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 @@ -137,4 +137,28 @@ The following example defines a service that requires the use of .. |protocol test link| replace:: https://github.com/awslabs/smithy/tree/main/smithy-aws-protocol-tests/model/awsJson1_1 .. include:: aws-json.rst.template + +-------------------- +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 the ``awsJson1_1`` protocol are not +allowed to rename error shapes (shapes with :ref:`error trait ` applied). + +Client-side implementation relies on the response body field ``code`` or ``__type`` to resolve the error type. +Server-side implementation of ``awsJson1_1`` 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``, +server-side will only send shape name ``ServiceError``. Client-side implementation will not be able to resolve +the correct error type without the ``namespace``. + +Server-side implementation of ``awsJson1_1`` protocol doesn't handle or send renamed shape names. +As a result, renaming will not resolve the conflicting shape IDs issue, and hence it is not permitted. + +The resolution should be to avoid having conflicting error shape IDs. + + .. _`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/core/model.rst b/docs/source-1.0/spec/core/model.rst index 497a06f13c6..eafd7028a21 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-json-1_1-protocol.rst b/docs/source-2.0/aws/protocols/aws-json-1_1-protocol.rst index 796ca734720..88f4516ce28 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 @@ -128,4 +128,28 @@ The following example defines a service that requires the use of .. |protocol test link| replace:: https://github.com/awslabs/smithy/tree/main/smithy-aws-protocol-tests/model/awsJson1_1 .. include:: aws-json.rst.template + +-------------------- +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 the ``awsJson1_1`` protocol are not +allowed to rename error shapes (shapes with :ref:`error trait ` applied). + +According to ``Operation error serialization``, +client-side implementation relies on the response body field ``code`` or ``__type`` to resolve the error type. +Server-side implementation of ``awsJson1_1`` 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``, +server-side will only send the shape name ``ServiceError``. Client-side implementation will not be able to resolve +the correct error type without the ``namespace``. + +Server-side implementation of ``awsJson1_1`` protocol doesn't handle or send renamed shape names. +As a result, renaming will not resolve the conflicting shape IDs issue, and hence it is not permitted. + +The resolution should be to avoid having conflicting error shape IDs. + .. _`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/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..9796a6bbc78 --- /dev/null +++ b/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/ErrorRenameValidator.java @@ -0,0 +1,96 @@ +/* + * 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.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import java.util.stream.Collectors; +import software.amazon.smithy.aws.traits.protocols.AwsJson1_1Trait; +import software.amazon.smithy.aws.traits.protocols.AwsProtocolTrait; +import software.amazon.smithy.model.Model; +import software.amazon.smithy.model.shapes.ServiceShape; +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_1Trait.class); + + @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 = getUnsupportedProtocols(service); + + if (unSupportedProtocols.isEmpty()) { + return; + } + + renames.keySet().stream() + .map(model::getShape) + .filter(Optional::isPresent) + .map(Optional::get) + .filter(shape -> shape.hasTrait(ErrorTrait.class)) + .forEach(shape -> { + ShapeId from = shape.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))); + }); + } + + private Set getUnsupportedProtocols(ServiceShape service) { + return service.getAllTraits().values().stream() + .filter(trait -> UNSUPPORTED_PROTOCOLS.contains(trait.getClass())) + .map(trait -> trait.toShapeId().getName()) + .collect(Collectors.toSet()); + } +} 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..db2f160fb25 --- /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 [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 [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