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

Make clearer what HTTP bindings are ignored if not used in top-level operation input / output #1379

Closed
david-perez opened this issue Sep 2, 2022 · 1 comment · Fixed by #1962
Labels
documentation This is a problem with documentation.

Comments

@david-perez
Copy link
Contributor

david-perez commented Sep 2, 2022

The following Smithy model builds without any warnings:

$version: "2.0"

namespace com.amazonaws.simple

use aws.protocols#restJson1

@restJson1
@title("SimpleService")
service SimpleService {
    operations: [
        AnOperation
    ]
}

@http(uri: "/operation/{label}" method: "GET")
operation AnOperation {
    input: AnOperationInput
    output: AnOperationOutput
}

@input
structure AnOperationInput {
    nested: Nested

    @required
    @httpLabel
    label: String
}

structure Nested {
    @required
    @httpHeader("header")
    header: String

    @required
    @httpLabel
    label: String

    @httpResponseCode
    responseCode: Integer

    @httpQuery("query")
    query: String

    @httpQueryParams
    queryParams: MapOfStrings

    @httpPrefixHeaders("X-Foo-")
    prefixHeaders: MapOfStrings
}

@output
structure AnOperationOutput {
    nested: Nested
}

map MapOfStrings {
    key: String
    value: String
}

What HTTP bindings are ignored if not used as top-level operation input or output?

We know for sure that httpLabel is, because if I remove the label member from AnOperationInput in the model above, Smithy emits an error:

ERROR: com.amazonaws.simple#AnOperation (HttpLabelTrait)
     @ /local/home/davidpz/workplace/smithy-ws/src/SmithyRsSource/codegen-server-test/model/simple.smithy
     |
  16 | operation AnOperation {
     | ^
     = This operation uses `com.amazonaws.simple#AnOperationInput` as input, but the following URI labels found in the operation's `http` trait do not have a corresponding member marked with the `httpLabel` trait: `label`

However, what about the other HTTP binding traits? Many contain a phrase in the spec indicating that a trait "is only used on {input,output}". For example, take httpResponseCode:

httpResponseCode is only used on output

httpResponseCode is ignored when resolving the HTTP bindings of any structure except an operation's output structure. This means that if a structure that contains members marked with the httpResponseCode trait is not used as an output structure of an operation, then those members are sent as part of the protocol-specific document sent in the body of the request.

So it's ignored in an operation's input or error, that much is clear and reasonable. However, I don't think it's abundantly clear whether it should be ignored when used in a shape that is not top-level operation output. If so, if "httpResponseCode is only used on output" could be rephrased as "httpResponseCode is only used on top-level operation output", then it'd be much clearer.

Same with all the other HTTP binding traits and especially with httpHeader. Since httpHeader can be used in operation inputs and outputs, does it get ignored when used in nested shapes? The spec does not clarify.

I'd also like to propose that Smithy emit a warning or a notice everywhere where an HTTP binding gets ignored: I know shape reusability is a goal and that you can e.g. reuse a nested shape in another operation's top-level input, but reusing shapes that are bound to parts of the HTTP message across operations is probably a modeling concern.

@mtdowling
Copy link
Member

We can reiterate this in each trait. We do call out at the very top of the HTTP binding page:

The members of an operation input, output, and errors MUST NOT be bound to multiple HTTP message locations (e.g., a member cannot be bound to both a URI label and to a header). Only top-level members of an operation's input, output, and error structures are considered when serializing HTTP messages.

Emitting a warning for ignored bindings sounds like a good idea.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation This is a problem with documentation.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants