Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow error rename and disallow error rename for all AWS protocols #1554

Merged
merged 1 commit into from
Jan 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/source-1.0/spec/aws/aws-ec2-query-protocol.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
1 change: 1 addition & 0 deletions docs/source-1.0/spec/aws/aws-json-1_0-protocol.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
1 change: 1 addition & 0 deletions docs/source-1.0/spec/aws/aws-json-1_1-protocol.rst
Original file line number Diff line number Diff line change
Expand Up @@ -136,5 +136,6 @@ The following example defines a service that requires the use of
.. |protocol error type contents| replace:: :token:`shape name <smithy:Identifier>`
.. |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
2 changes: 2 additions & 0 deletions docs/source-1.0/spec/aws/aws-query-protocol.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 2 additions & 0 deletions docs/source-1.0/spec/aws/aws-restjson1-protocol.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 2 additions & 0 deletions docs/source-1.0/spec/aws/aws-restxml-protocol.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
7 changes: 7 additions & 0 deletions docs/source-1.0/spec/aws/error-rename-simple.rst.template
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
--------------------
Error shape renaming
--------------------

By default, Smithy permits renaming shapes to disambiguate shape ID conflicts in
the :ref:`service closure <service-closure>` via the ``rename`` property. However, services using this protocol are
not allowed to rename error shapes (shapes with :ref:`error trait <error-trait>` applied).
Copy link
Contributor Author

@AndrewFossAWS AndrewFossAWS Jan 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will be removed later when AWS SDKs have been verified to support error shape rename.

17 changes: 17 additions & 0 deletions docs/source-1.0/spec/aws/error-rename.rst.template
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
--------------------
Error shape renaming
--------------------

By default, Smithy permits renaming shapes to disambiguate shape ID conflicts in
the :ref:`service closure <service-closure>` via the ``rename`` property. However, services using this protocol are
not allowed to rename error shapes (shapes with :ref:`error trait <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.
6 changes: 3 additions & 3 deletions docs/source-1.0/spec/core/model.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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 <private-trait>`
MAY be renamed.
* A rename MUST use a name that is case-sensitively different from the
Expand Down
2 changes: 2 additions & 0 deletions docs/source-2.0/aws/protocols/aws-ec2-query-protocol.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
1 change: 1 addition & 0 deletions docs/source-2.0/aws/protocols/aws-json-1_0-protocol.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
1 change: 1 addition & 0 deletions docs/source-2.0/aws/protocols/aws-json-1_1-protocol.rst
Original file line number Diff line number Diff line change
Expand Up @@ -127,5 +127,6 @@ The following example defines a service that requires the use of
.. |protocol error type contents| replace:: :token:`shape name <smithy:Identifier>`
.. |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
2 changes: 2 additions & 0 deletions docs/source-2.0/aws/protocols/aws-query-protocol.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 2 additions & 0 deletions docs/source-2.0/aws/protocols/aws-restjson1-protocol.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 2 additions & 0 deletions docs/source-2.0/aws/protocols/aws-restxml-protocol.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
--------------------
Error shape renaming
--------------------

By default, Smithy permits renaming shapes to disambiguate shape ID conflicts in
the :ref:`service closure <service-closure>` via the ``rename`` property. However, services using this protocol are
not allowed to rename error shapes (shapes with :ref:`error trait <error-trait>` applied).
17 changes: 17 additions & 0 deletions docs/source-2.0/aws/protocols/error-rename.rst.template
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
--------------------
Error shape renaming
--------------------

By default, Smithy permits renaming shapes to disambiguate shape ID conflicts in
the :ref:`service closure <service-closure>` via the ``rename`` property. However, services using this protocol are
not allowed to rename error shapes (shapes with :ref:`error trait <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.
6 changes: 3 additions & 3 deletions docs/source-2.0/spec/service-types.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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 <private-trait>`
MAY be renamed.
* A rename MUST use a name that is case-sensitively different from the
Expand Down
Original file line number Diff line number Diff line change
@@ -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<ShapeId> UNSUPPORTED_PROTOCOLS = SetUtils.of(
AwsJson1_0Trait.ID, AwsJson1_1Trait.ID, AwsQueryTrait.ID,
Ec2QueryTrait.ID, RestJson1Trait.ID, RestXmlTrait.ID
);

AndrewFossAWS marked this conversation as resolved.
Show resolved Hide resolved
@Override
public List<ValidationEvent> validate(Model model) {
List<ValidationEvent> events = new ArrayList<>();
for (ServiceShape shape : model.getServiceShapes()) {
validate(model, shape, events);
}
return events;
}

private void validate(Model model, ServiceShape service, List<ValidationEvent> events) {
final Map<ShapeId, String> renames = service.getRename();

if (renames.isEmpty()) {
return;
}

Set<String> 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> 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)));
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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 {}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -156,8 +155,6 @@ private List<ValidationEvent> validateRenames(ServiceShape service, Map<ShapeId,
private Optional<String> 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();
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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