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

Add response code binding to http binding index #571

Merged
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
19 changes: 14 additions & 5 deletions docs/source/1.0/spec/core/http-traits.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand Down Expand Up @@ -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`.

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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:

Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -431,6 +433,10 @@ private List<HttpBinding> 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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {}

Expand All @@ -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
Expand All @@ -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
Expand All @@ -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 {}
Expand All @@ -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 {}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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<String, HttpBinding> 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);
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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"
}
}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down