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 protocol tests relating to Content-Type and @httpPayload #2314

Conversation

david-perez
Copy link
Contributor

Add protocol tests relating to Content-Type and @httpPayload

Initial motivation for this stems off #2310, but for when the input is
@httpPayload-bound. It's important that we test Content-Type
specifically with @httpPayload operations, because the header can be
checked without having to inspect the body, and because the spec has
special provisions
(https://smithy.io/2.0/aws/protocols/aws-restjson1-protocol.html#content-type).

There were no protocol tests exercising Content-Type when the input is
@httpPayload-bound:

  • In the case of blobs, servers must accept any (test already exists)
    and no Content-Type (test added in this commit).
  • In other cases, there were protocol tests for union and structure
    shapes. I've added an operation with an @httpPayload-bound string
    shape. Note that document shapes remain untested.

Testing

  • Ran tests against smithy-rs servers.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Initial motivation for this stems off smithy-lang#2310, but for when the input is
`@httpPayload`-bound. It's important that we test `Content-Type`
specifically with `@httpPayload` operations, because the header can be
checked without having to inspect the body, and because the spec has
special provisions
(https://smithy.io/2.0/aws/protocols/aws-restjson1-protocol.html#content-type).

There were no protocol tests exercising `Content-Type` when the input is
`@httpPayload`-bound:

- In the case of blobs, servers must accept any (test already exists)
  and no `Content-Type` (test added in this commit).
- In other cases, there were protocol tests for `union` and `structure`
  shapes. I've added an operation with an `@httpPayload`-bound string
  shape. Note that `document` shapes remain untested.
@david-perez david-perez requested a review from a team as a code owner June 7, 2024 14:27
@david-perez david-perez requested a review from syall June 7, 2024 14:27
As per the specs for these protocols, a `Content-Type` header should be
set. Server should reject requests when an expected `Content-Type`
header is not found (protcol tests for that are added in smithy-lang#2314).

- https://smithy.io/2.0/aws/protocols/aws-restjson1-protocol.html#content-type
- https://smithy.io/2.0/aws/protocols/aws-restxml-protocol.html#content-type
david-perez added a commit to david-perez/smithy that referenced this pull request Jun 11, 2024
As per the specs for these protocols, a `Content-Type` header should be
set. Server should reject requests when an expected `Content-Type`
header is not found (protcol tests for that are added in smithy-lang#2314).

- https://smithy.io/2.0/aws/protocols/aws-restjson1-protocol.html#content-type
- https://smithy.io/2.0/aws/protocols/aws-restxml-protocol.html#content-type
JordonPhillips pushed a commit that referenced this pull request Jun 13, 2024
As per the specs for these protocols, a `Content-Type` header should be
set. Server should reject requests when an expected `Content-Type`
header is not found (protcol tests for that are added in #2314).

- https://smithy.io/2.0/aws/protocols/aws-restjson1-protocol.html#content-type
- https://smithy.io/2.0/aws/protocols/aws-restxml-protocol.html#content-type
@JordonPhillips JordonPhillips merged commit e2e7684 into smithy-lang:main Jun 17, 2024
13 checks passed
@david-perez david-perez deleted the davidpz/rest-json-with-bound-body-expects-application-json-content-type-no-headers branch June 18, 2024 10:54
david-perez added a commit to david-perez/smithy that referenced this pull request Jun 18, 2024
In smithy-lang#2314, I added an operation featuring an `@httpPayload`-bound shape
to test `Content-Type` header checking, among other things. However, I
failed to notice there is already a `http-string-payload.smithy` file in
the test suite to test `@httpPayload`-bound string shapes.

This PR merges and applies the tests I added in smithy-lang#2314 to the operation
that already existed.
JordonPhillips pushed a commit that referenced this pull request Jun 18, 2024
In #2314, I added an operation featuring an `@httpPayload`-bound shape
to test `Content-Type` header checking, among other things. However, I
failed to notice there is already a `http-string-payload.smithy` file in
the test suite to test `@httpPayload`-bound string shapes.

This PR merges and applies the tests I added in #2314 to the operation
that already existed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants