From b9f48a82a0f9b9f982028d8ecbad914fe3511786 Mon Sep 17 00:00:00 2001 From: JordonPhillips Date: Thu, 17 Sep 2020 10:37:32 -0700 Subject: [PATCH 1/3] Add response code binding to http binding index This adds support for the httpResponseCode trait to the http binding index, including a new RESPONSE_CODE location. --- .../smithy/model/knowledge/HttpBinding.java | 2 +- .../model/knowledge/HttpBindingIndex.java | 10 +++++-- .../model/knowledge/HttpBindingIndexTest.java | 27 +++++++++++++++++++ .../smithy/model/knowledge/http-index.json | 26 ++++++++++++++++++ 4 files changed, 62 insertions(+), 3 deletions(-) diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/knowledge/HttpBinding.java b/smithy-model/src/main/java/software/amazon/smithy/model/knowledge/HttpBinding.java index 278e81b3234..5fd61f559e2 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/knowledge/HttpBinding.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/knowledge/HttpBinding.java @@ -39,7 +39,7 @@ public final class HttpBinding { } /** HTTP binding types. */ - public enum Location { LABEL, DOCUMENT, PAYLOAD, HEADER, PREFIX_HEADERS, QUERY, UNBOUND } + public enum Location { LABEL, DOCUMENT, PAYLOAD, HEADER, PREFIX_HEADERS, QUERY, RESPONSE_CODE, UNBOUND } public MemberShape getMember() { return member; diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/knowledge/HttpBindingIndex.java b/smithy-model/src/main/java/software/amazon/smithy/model/knowledge/HttpBindingIndex.java index af39e8594fc..ad5cb7cf559 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/knowledge/HttpBindingIndex.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/knowledge/HttpBindingIndex.java @@ -38,6 +38,7 @@ import software.amazon.smithy.model.traits.HttpPayloadTrait; import software.amazon.smithy.model.traits.HttpPrefixHeadersTrait; import software.amazon.smithy.model.traits.HttpQueryTrait; +import software.amazon.smithy.model.traits.HttpResponseCodeTrait; import software.amazon.smithy.model.traits.HttpTrait; import software.amazon.smithy.model.traits.MediaTypeTrait; import software.amazon.smithy.model.traits.StreamingTrait; @@ -110,8 +111,9 @@ public static boolean hasHttpRequestBindings(Shape shape) { */ public static boolean hasHttpResponseBindings(Shape shape) { return shape.hasTrait(HttpHeaderTrait.class) - || shape.hasTrait(HttpPrefixHeadersTrait.class) - || shape.hasTrait(HttpPayloadTrait.class); + || shape.hasTrait(HttpPrefixHeadersTrait.class) + || shape.hasTrait(HttpPayloadTrait.class) + || shape.hasTrait(HttpResponseCodeTrait.class); } private HttpTrait getHttpTrait(ToShapeId operation) { @@ -431,6 +433,10 @@ private List createStructureBindings(StructureShape struct, boolean } else if (isRequest && member.getTrait(HttpLabelTrait.class).isPresent()) { HttpLabelTrait trait = member.getTrait(HttpLabelTrait.class).get(); bindings.add(new HttpBinding(member, HttpBinding.Location.LABEL, member.getMemberName(), trait)); + } else if (!isRequest && member.getTrait(HttpResponseCodeTrait.class).isPresent()) { + HttpResponseCodeTrait trait = member.getTrait(HttpResponseCodeTrait.class).get(); + bindings.add(new HttpBinding( + member, HttpBinding.Location.RESPONSE_CODE, member.getMemberName(), trait)); } else { unbound.add(member); } diff --git a/smithy-model/src/test/java/software/amazon/smithy/model/knowledge/HttpBindingIndexTest.java b/smithy-model/src/test/java/software/amazon/smithy/model/knowledge/HttpBindingIndexTest.java index d3f130975f6..c8d64888a3c 100644 --- a/smithy-model/src/test/java/software/amazon/smithy/model/knowledge/HttpBindingIndexTest.java +++ b/smithy-model/src/test/java/software/amazon/smithy/model/knowledge/HttpBindingIndexTest.java @@ -43,6 +43,7 @@ import software.amazon.smithy.model.traits.HttpLabelTrait; import software.amazon.smithy.model.traits.HttpPayloadTrait; import software.amazon.smithy.model.traits.HttpPrefixHeadersTrait; +import software.amazon.smithy.model.traits.HttpResponseCodeTrait; import software.amazon.smithy.model.traits.HttpTrait; import software.amazon.smithy.model.traits.TimestampFormatTrait; @@ -152,6 +153,20 @@ public void returnsResponseMemberBindingsWithExplicitBody() { new HttpPayloadTrait()))); } + @Test + public void returnsResponseCodeBinding() { + HttpBindingIndex index = HttpBindingIndex.of(model); + ShapeId id = ShapeId.from("ns.foo#OperationWithBoundResponseCode"); + Map responseBindings = index.getResponseBindings(id); + + assertThat(responseBindings.size(), is(1)); + assertThat(responseBindings.get("Status"), equalTo(new HttpBinding( + expectMember(model, "ns.foo#StructureWithBoundResponseCode$Status"), + HttpBinding.Location.RESPONSE_CODE, + "Status", + new HttpResponseCodeTrait()))); + } + @Test public void returnsErrorResponseCode() { HttpBindingIndex index = HttpBindingIndex.of(model); @@ -233,6 +248,18 @@ public void checksForHttpResponseBindings() { assertThat(HttpBindingIndex.hasHttpResponseBindings(shape), is(true)); } + @Test + public void checksForHttpResponseCodeBindings() { + Shape shape = MemberShape.builder() + .target("smithy.api#Integer") + .id("smithy.example#Baz$bar") + .addTrait(new HttpResponseCodeTrait()) + .build(); + + assertThat(HttpBindingIndex.hasHttpRequestBindings(shape), is(false)); + assertThat(HttpBindingIndex.hasHttpResponseBindings(shape), is(true)); + } + @Test public void resolvesStructureBodyContentType() { HttpBindingIndex index = HttpBindingIndex.of(model); diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/knowledge/http-index.json b/smithy-model/src/test/resources/software/amazon/smithy/model/knowledge/http-index.json index eb47052b3ad..c0453dc542d 100644 --- a/smithy-model/src/test/resources/software/amazon/smithy/model/knowledge/http-index.json +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/knowledge/http-index.json @@ -22,9 +22,35 @@ }, { "target": "ns.foo#ServiceOperationWithEventStream" + }, + { + "target": "ns.foo#OperationWithBoundResponseCode" } ] }, + "ns.foo#OperationWithBoundResponseCode": { + "type": "operation", + "output": { + "target": "ns.foo#StructureWithBoundResponseCode" + }, + "traits": { + "smithy.api#http": { + "uri": "/OperationWithBoundResponseCode", + "method": "PUT" + } + } + }, + "ns.foo#StructureWithBoundResponseCode": { + "type": "structure", + "members": { + "Status": { + "target": "smithy.api#Integer", + "traits": { + "smithy.api#httpResponseCode": {} + } + } + } + }, "ns.foo#ServiceOperationNoInputOutput": { "type": "operation", "traits": { From 8ba8add7710421c623d8e0c7b2976421ec1dbe56 Mon Sep 17 00:00:00 2001 From: JordonPhillips Date: Thu, 17 Sep 2020 13:25:58 -0700 Subject: [PATCH 2/3] Update http binding trait conflicts This adds a trait conflict list for httpResponseCode and adds that trait to the conflict lists for the other http binding traits. --- .../amazon/smithy/model/loader/prelude.smithy | 13 ++++++------ .../validators/http-trait-conflicts.errors | 1 + .../validators/http-trait-conflicts.json | 20 +++++++++++++++++++ 3 files changed, 28 insertions(+), 6 deletions(-) create mode 100644 smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/http-trait-conflicts.errors create mode 100644 smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/http-trait-conflicts.json diff --git a/smithy-model/src/main/resources/software/amazon/smithy/model/loader/prelude.smithy b/smithy-model/src/main/resources/software/amazon/smithy/model/loader/prelude.smithy index f8157931912..f3a5d1fe9c1 100644 --- a/smithy-model/src/main/resources/software/amazon/smithy/model/loader/prelude.smithy +++ b/smithy-model/src/main/resources/software/amazon/smithy/model/loader/prelude.smithy @@ -530,7 +530,7 @@ structure http { /// Binds an operation input structure member to an HTTP label. @trait(selector: "structure > member[trait|required] :test(> :test(string, number, boolean, timestamp))", - conflicts: [httpHeader, httpQuery, httpPrefixHeaders, httpPayload]) + conflicts: [httpHeader, httpQuery, httpPrefixHeaders, httpPayload, httpResponseCode]) @tags(["diff.error.const"]) structure httpLabel {} @@ -539,7 +539,7 @@ structure httpLabel {} structure > member :test(> :test(string, number, boolean, timestamp), > collection > member > :test(string, number, boolean, timestamp))""", - conflicts: [httpLabel, httpHeader, httpPrefixHeaders, httpPayload]) + conflicts: [httpLabel, httpHeader, httpPrefixHeaders, httpPayload, httpResponseCode]) @length(min: 1) @tags(["diff.error.const"]) string httpQuery @@ -548,7 +548,7 @@ string httpQuery @trait(selector: """ structure > :test(member > :test(boolean, number, string, timestamp, collection > member > :test(boolean, number, string, timestamp)))""", - conflicts: [httpLabel, httpQuery, httpPrefixHeaders, httpPayload]) + conflicts: [httpLabel, httpQuery, httpPrefixHeaders, httpPayload, httpResponseCode]) @length(min: 1) @tags(["diff.error.const"]) string httpHeader @@ -558,13 +558,13 @@ string httpHeader structure > member :test(> map > member[id|member=value] > :test(string, collection > member > string))""", structurallyExclusive: "member", - conflicts: [httpLabel, httpQuery, httpHeader, httpPayload]) + conflicts: [httpLabel, httpQuery, httpHeader, httpPayload, httpResponseCode]) @tags(["diff.error.const"]) string httpPrefixHeaders /// Binds a single structure member to the body of an HTTP request. @trait(selector: "structure > :test(member > :test(string, blob, structure, union, document))", - conflicts: [httpLabel, httpQuery, httpHeader, httpPrefixHeaders], + conflicts: [httpLabel, httpQuery, httpHeader, httpPrefixHeaders, httpResponseCode], structurallyExclusive: "member") @tags(["diff.error.const"]) structure httpPayload {} @@ -578,7 +578,8 @@ integer httpError /// status code. The value MAY differ from the HTTP status code provided /// on the response. @trait(selector: "structure > member :test(> integer)", - structurallyExclusive: "member") + structurallyExclusive: "member", + conflicts: [httpLabel, httpQuery, httpHeader, httpPrefixHeaders, httpPayload]) @tags(["diff.error.const"]) structure httpResponseCode {} diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/http-trait-conflicts.errors b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/http-trait-conflicts.errors new file mode 100644 index 00000000000..28cf8acde8c --- /dev/null +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/http-trait-conflicts.errors @@ -0,0 +1 @@ +[ERROR] ns.foo#HttpOperationInput$Int: Found conflicting traits on member shape: `httpHeader` conflicts with `httpLabel`, `httpHeader` conflicts with `httpQuery`, `httpHeader` conflicts with `httpResponseCode`, `httpLabel` conflicts with `httpHeader`, `httpLabel` conflicts with `httpQuery`, `httpLabel` conflicts with `httpResponseCode`, `httpQuery` conflicts with `httpHeader`, `httpQuery` conflicts with `httpLabel`, `httpQuery` conflicts with `httpResponseCode`, `httpResponseCode` conflicts with `httpHeader`, `httpResponseCode` conflicts with `httpLabel`, `httpResponseCode` conflicts with `httpQuery` | TraitConflict diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/http-trait-conflicts.json b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/http-trait-conflicts.json new file mode 100644 index 00000000000..a2004ca7743 --- /dev/null +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/http-trait-conflicts.json @@ -0,0 +1,20 @@ +{ + "smithy": "1.0", + "shapes": { + "ns.foo#HttpOperationInput": { + "type": "structure", + "members": { + "Int": { + "target": "smithy.api#Integer", + "traits": { + "smithy.api#required": {}, + "smithy.api#httpResponseCode": {}, + "smithy.api#httpLabel": {}, + "smithy.api#httpQuery": "foo", + "smithy.api#httpHeader": "x-amz-foo" + } + } + } + } + } +} From f61c08696a91b51d43702f74acc1d9d5bd4c22c9 Mon Sep 17 00:00:00 2001 From: JordonPhillips Date: Thu, 17 Sep 2020 13:36:04 -0700 Subject: [PATCH 3/3] Update http trait conflict docs --- docs/source/1.0/spec/core/http-traits.rst | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/docs/source/1.0/spec/core/http-traits.rst b/docs/source/1.0/spec/core/http-traits.rst index dc7b5a7834b..366a33f51d2 100644 --- a/docs/source/1.0/spec/core/http-traits.rst +++ b/docs/source/1.0/spec/core/http-traits.rst @@ -492,7 +492,8 @@ Conflicts with :ref:`httpLabel-trait`, :ref:`httpQuery-trait`, :ref:`httpPrefixHeaders-trait`, - :ref:`httpPayload-trait` + :ref:`httpPayload-trait`, + :ref:`httpResponseCode-trait` Serialization rules: @@ -585,7 +586,8 @@ Conflicts with :ref:`httpHeader-trait`, :ref:`httpQuery-trait`, :ref:`httpPrefixHeaders-trait`, - :ref:`httpPayload-trait` + :ref:`httpPayload-trait`, + :ref:`httpResponseCode-trait` ``httpLabel`` members MUST be marked as :ref:`required-trait`. @@ -649,7 +651,8 @@ Value type Annotation trait. Conflicts with :ref:`httpLabel-trait`, :ref:`httpQuery-trait`, - :ref:`httpHeader-trait`, :ref:`httpPrefixHeaders-trait` + :ref:`httpHeader-trait`, :ref:`httpPrefixHeaders-trait`, + :ref:`httpResponseCode-trait` By default, all structure members that are not bound as part of the HTTP message are serialized in a protocol-specific document sent in the body of @@ -699,7 +702,8 @@ Value type field name serialized in the message is "X-Amz-Meta-Baz". Conflicts with :ref:`httpLabel-trait`, :ref:`httpQuery-trait`, - :ref:`httpHeader-trait`, :ref:`httpPayload-trait` + :ref:`httpHeader-trait`, :ref:`httpPayload-trait`, + :ref:`httpResponseCode-trait` In order to differentiate ``httpPrefixHeaders`` from other headers, when ``httpPrefixHeaders`` are used, no other :ref:`httpHeader-trait` bindings can @@ -774,7 +778,8 @@ Value type resolving the HTTP bindings of an operation's output or an error. Conflicts with :ref:`httpLabel-trait`, :ref:`httpHeader-trait`, - :ref:`httpPrefixHeaders-trait`, :ref:`httpPayload-trait` + :ref:`httpPrefixHeaders-trait`, :ref:`httpPayload-trait`, + :ref:`httpResponseCode-trait` Serialization rules: @@ -817,6 +822,10 @@ Trait selector ``structure > member :test(> integer)`` Value type Annotation trait. +Conflicts with + :ref:`httpLabel-trait`, :ref:`httpHeader-trait`, + :ref:`httpPrefixHeaders-trait`, :ref:`httpPayload-trait`, + :ref:`httpQuery-trait` The value MAY differ from the HTTP status code provided on the response. Explicitly modeling this as a field can be helpful for services that wish to