From e235ad48bd054e85e0bdb3f3b56bbc55ad0ab812 Mon Sep 17 00:00:00 2001 From: Michael Dowling Date: Wed, 19 Feb 2020 21:51:42 -0800 Subject: [PATCH] Add protocol trait requirements Protocol traits now list the traits that protocol implementations must understand in order to successfully implement the protocol. This provides the ability to better ensure correctness of protocol implementations as protocols evolve. --- docs/source/spec/core.rst | 16 +++- .../META-INF/smithy/aws.protocols.json | 58 +++++++++++-- .../model/traits/ProtocolDefinitionTrait.java | 85 +++++++++++++++++-- .../smithy/model/loader/prelude-traits.smithy | 15 +++- .../traits/ProtocolDefinitionTraitTest.java | 37 ++++++++ 5 files changed, 195 insertions(+), 16 deletions(-) create mode 100644 smithy-model/src/test/java/software/amazon/smithy/model/traits/ProtocolDefinitionTraitTest.java diff --git a/docs/source/spec/core.rst b/docs/source/spec/core.rst index 2b9fc5a72f3..37e98f4025d 100644 --- a/docs/source/spec/core.rst +++ b/docs/source/spec/core.rst @@ -4675,7 +4675,21 @@ Summary Trait selector ``[trait|trait]`` Value type - Annotation trait. + An object with the following properties: + + .. list-table:: + :header-rows: 1 + :widths: 10 23 67 + + * - Property + - Type + - Description + * - traits + - [:ref:`shape-id`] + - List of shape IDs that protocol implementations MUST understand + in order to successfully use the protocol. Each shape MUST exist + and MUST be a trait. Code generators SHOULD ensure that they + support each listed trait. Smithy is protocol agnostic, which means it focuses on the interfaces and abstractions that are provided to end-users rather than how the data is sent diff --git a/smithy-aws-traits/src/main/resources/META-INF/smithy/aws.protocols.json b/smithy-aws-traits/src/main/resources/META-INF/smithy/aws.protocols.json index 321d07458a2..2b6e79744f3 100644 --- a/smithy-aws-traits/src/main/resources/META-INF/smithy/aws.protocols.json +++ b/smithy-aws-traits/src/main/resources/META-INF/smithy/aws.protocols.json @@ -15,7 +15,17 @@ "smithy.api#trait": { "selector": "service" }, - "smithy.api#protocolDefinition": true, + "smithy.api#protocolDefinition": { + "traits": [ + "smithy.api#httpError", + "smithy.api#httpHeader", + "smithy.api#httpLabel", + "smithy.api#httpPayload", + "smithy.api#httpPrefixHeaders", + "smithy.api#httpQuery", + "smithy.api#jsonName" + ] + }, "smithy.api#documentation": "A RESTful protocol that sends JSON in structured payloads." } }, @@ -33,7 +43,20 @@ "smithy.api#trait": { "selector": "service" }, - "smithy.api#protocolDefinition": true, + "smithy.api#protocolDefinition": { + "traits": [ + "smithy.api#httpError", + "smithy.api#httpHeader", + "smithy.api#httpLabel", + "smithy.api#httpPayload", + "smithy.api#httpPrefixHeaders", + "smithy.api#httpQuery", + "smithy.api#xmlAttribute", + "smithy.api#xmlFlattened", + "smithy.api#xmlName", + "smithy.api#xmlNamespace" + ] + }, "smithy.api#documentation": "A RESTful protocol that sends XML in structured payloads.", "smithy.api#deprecated": true } @@ -52,7 +75,11 @@ "smithy.api#trait": { "selector": "service" }, - "smithy.api#protocolDefinition": true, + "smithy.api#protocolDefinition": { + "traits": [ + "smithy.api#jsonName" + ] + }, "smithy.api#documentation": "An RPC-based protocol that sends JSON payloads. This protocol does not use HTTP binding traits." } }, @@ -70,7 +97,11 @@ "smithy.api#trait": { "selector": "service" }, - "smithy.api#protocolDefinition": true, + "smithy.api#protocolDefinition": { + "traits": [ + "smithy.api#jsonName" + ] + }, "smithy.api#documentation": "An RPC-based protocol that sends JSON payloads. This protocol does not use HTTP binding traits." } }, @@ -80,7 +111,14 @@ "smithy.api#trait": { "selector": "service" }, - "smithy.api#protocolDefinition": true, + "smithy.api#protocolDefinition": { + "traits": [ + "smithy.api#xmlAttribute", + "smithy.api#xmlFlattened", + "smithy.api#xmlName", + "smithy.api#xmlNamespace" + ] + }, "smithy.api#documentation": "An RPC-based protocol that sends query string requests and XML responses. This protocol does not use HTTP binding traits.", "smithy.api#deprecated": true } @@ -91,7 +129,15 @@ "smithy.api#trait": { "selector": "service" }, - "smithy.api#protocolDefinition": true, + "smithy.api#protocolDefinition": { + "traits": [ + "aws.api#ec2QueryName", + "smithy.api#xmlAttribute", + "smithy.api#xmlFlattened", + "smithy.api#xmlName", + "smithy.api#xmlNamespace" + ] + }, "smithy.api#documentation": "An RPC-based protocol that sends Amazon EC2 formatted query string requests and XML responses. This protocol does not use HTTP binding traits.", "smithy.api#deprecated": true } diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/traits/ProtocolDefinitionTrait.java b/smithy-model/src/main/java/software/amazon/smithy/model/traits/ProtocolDefinitionTrait.java index 69b666f5190..457b2a0af04 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/traits/ProtocolDefinitionTrait.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/traits/ProtocolDefinitionTrait.java @@ -15,27 +15,96 @@ package software.amazon.smithy.model.traits; -import software.amazon.smithy.model.SourceLocation; +import java.util.ArrayList; +import java.util.List; +import software.amazon.smithy.model.node.ArrayNode; +import software.amazon.smithy.model.node.Node; +import software.amazon.smithy.model.node.ObjectNode; import software.amazon.smithy.model.shapes.ShapeId; +import software.amazon.smithy.utils.ListUtils; +import software.amazon.smithy.utils.ToSmithyBuilder; /** * A trait that is attached to other traits to define a Smithy protocol. */ -public final class ProtocolDefinitionTrait extends BooleanTrait { +public final class ProtocolDefinitionTrait extends AbstractTrait implements ToSmithyBuilder { public static final ShapeId ID = ShapeId.from("smithy.api#protocolDefinition"); + private final List traits; - public ProtocolDefinitionTrait(SourceLocation sourceLocation) { - super(ID, sourceLocation); + public ProtocolDefinitionTrait(Builder builder) { + super(ID, builder.getSourceLocation()); + traits = ListUtils.copyOf(builder.traits); } - public ProtocolDefinitionTrait() { - this(SourceLocation.NONE); + /** + * Gets the list of shape IDs that protocol implementations must know about + * in order to successfully utilize the protocol. + * + * @return Returns the protocol traits. + */ + public List getTraits() { + return traits; } - public static final class Provider extends BooleanTrait.Provider { + public static Builder builder() { + return new Builder(); + } + + @Override + protected Node createNode() { + if (traits.isEmpty()) { + return Node.objectNode(); + } + + ArrayNode ids = traits.stream() + .map(ShapeId::toString) + .map(Node::from) + .collect(ArrayNode.collect()); + + return Node.objectNode().withMember("traits", ids); + } + + @Override + public Builder toBuilder() { + return builder().sourceLocation(getSourceLocation()).traits(traits); + } + + public static final class Provider extends AbstractTrait.Provider { public Provider() { - super(ID, ProtocolDefinitionTrait::new); + super(ID); + } + + @Override + public ProtocolDefinitionTrait createTrait(ShapeId target, Node value) { + Builder builder = builder().sourceLocation(value); + ObjectNode objectNode = value.expectObjectNode(); + objectNode.getArrayMember("traits").ifPresent(traits -> { + for (String string : Node.loadArrayOfString("traits", traits)) { + builder.addTrait(ShapeId.from(string)); + } + }); + return builder.build(); + } + } + + public static final class Builder extends AbstractTraitBuilder { + private final List traits = new ArrayList<>(); + + @Override + public ProtocolDefinitionTrait build() { + return new ProtocolDefinitionTrait(this); + } + + public Builder traits(List traits) { + this.traits.clear(); + this.traits.addAll(traits); + return this; + } + + public Builder addTrait(ShapeId trait) { + traits.add(trait); + return this; } } } diff --git a/smithy-model/src/main/resources/software/amazon/smithy/model/loader/prelude-traits.smithy b/smithy-model/src/main/resources/software/amazon/smithy/model/loader/prelude-traits.smithy index eb0459ad37e..e5dadc72ad5 100644 --- a/smithy-model/src/main/resources/software/amazon/smithy/model/loader/prelude-traits.smithy +++ b/smithy-model/src/main/resources/software/amazon/smithy/model/loader/prelude-traits.smithy @@ -62,7 +62,20 @@ string AuthTraitReference /// structure, and must have the `trait` trait. @trait(selector: "structure[trait|trait]") @tags(["diff.error.add", "diff.error.remove"]) -structure protocolDefinition {} +structure protocolDefinition { + /// Defines a list of traits that protocol implementations must + /// understand in order to successfully use the protocol. + traits: TraitShapeIdList, +} + +@private +list TraitShapeIdList { + member: TraitShapeId, +} + +@private +@idRef(failWhenMissing: true, selector: "[trait|trait]") +string TraitShapeId /// Marks a trait as an auth scheme defining trait. /// diff --git a/smithy-model/src/test/java/software/amazon/smithy/model/traits/ProtocolDefinitionTraitTest.java b/smithy-model/src/test/java/software/amazon/smithy/model/traits/ProtocolDefinitionTraitTest.java new file mode 100644 index 00000000000..8767e08be2a --- /dev/null +++ b/smithy-model/src/test/java/software/amazon/smithy/model/traits/ProtocolDefinitionTraitTest.java @@ -0,0 +1,37 @@ +package software.amazon.smithy.model.traits; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.instanceOf; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.Optional; +import org.junit.jupiter.api.Test; +import software.amazon.smithy.model.node.ArrayNode; +import software.amazon.smithy.model.node.Node; +import software.amazon.smithy.model.shapes.ShapeId; + +public class ProtocolDefinitionTraitTest { + @Test + public void loadsTrait() { + TraitFactory provider = TraitFactory.createServiceFactory(); + ArrayNode values = Node.fromStrings( + JsonNameTrait.ID.toString(), + XmlNameTrait.ID.toString()); + Node node = Node.objectNode().withMember("traits", values); + Optional trait = provider.createTrait( + ShapeId.from("smithy.api#protocolDefinition"), + ShapeId.from("ns.qux#foo"), + node); + + assertTrue(trait.isPresent()); + assertThat(trait.get(), instanceOf(ProtocolDefinitionTrait.class)); + ProtocolDefinitionTrait protocolDefinitionTrait = (ProtocolDefinitionTrait) trait.get(); + assertThat(protocolDefinitionTrait.getTraits(), containsInAnyOrder( + JsonNameTrait.ID, XmlNameTrait.ID)); + assertThat(protocolDefinitionTrait.toNode(), equalTo(node)); + assertThat(protocolDefinitionTrait.toBuilder().build(), equalTo(protocolDefinitionTrait)); + } +}