Centralize http binding matching and omit empty payloads #503
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,172 @@ | ||
| # Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
| from dataclasses import dataclass | ||
| from enum import Enum | ||
|
|
||
| from smithy_core.schemas import Schema | ||
| from smithy_core.shapes import ShapeType | ||
| from smithy_core.traits import ( | ||
| ErrorFault, | ||
| ErrorTrait, | ||
| HostLabelTrait, | ||
| HTTPErrorTrait, | ||
| HTTPHeaderTrait, | ||
| HTTPLabelTrait, | ||
| HTTPPayloadTrait, | ||
| HTTPPrefixHeadersTrait, | ||
| HTTPQueryParamsTrait, | ||
| HTTPQueryTrait, | ||
| HTTPResponseCodeTrait, | ||
| StreamingTrait, | ||
| ) | ||
|
|
||
|
|
||
| class Binding(Enum): | ||
| """HTTP binding locations.""" | ||
|
|
||
| HEADER = 0 | ||
| """Indicates the member is bound to a header.""" | ||
|
|
||
| QUERY = 1 | ||
| """Indicates the member is bound to a query parameter.""" | ||
|
|
||
| PAYLOAD = 2 | ||
| """Indicates the member is bound to the entire HTTP payload.""" | ||
|
|
||
| BODY = 3 | ||
| """Indicates the member is a property in the HTTP payload structure.""" | ||
|
|
||
| LABEL = 4 | ||
| """Indicates the member is bound to a path segment in the URI.""" | ||
|
|
||
| STATUS = 5 | ||
| """Indicates the member is bound to the response status code.""" | ||
|
|
||
| PREFIX_HEADERS = 6 | ||
| """Indicates the member is bound to multiple headers with a shared prefix.""" | ||
|
|
||
| QUERY_PARAMS = 7 | ||
| """Indicates the member is bound to the query string as multiple key-value pairs.""" | ||
|
|
||
| HOST = 8 | ||
| """Indicates the member is bound to a prefix to the host AND to the body.""" | ||
|
|
||
|
|
||
| @dataclass(init=False) | ||
| class _BindingMatcher: | ||
| bindings: list[Binding] | ||
| """A list of bindings where the index matches the index of the member schema.""" | ||
|
|
||
| response_status: int | ||
| """The default response status code.""" | ||
|
|
||
| has_body: bool | ||
| """Whether the HTTP message has members bound to the body.""" | ||
|
|
||
| has_payload: bool | ||
| """Whether the HTTP message has a member bound to the entire payload.""" | ||
|
|
||
| payload_member: Schema | None | ||
| """The member bound to the payload, if one exists.""" | ||
|
|
||
| event_stream_member: Schema | None | ||
| """The member bound to the event stream, if one exists.""" | ||
|
|
||
| def __init__(self, struct: Schema, response_status: int) -> None: | ||
| self.response_status = response_status | ||
| found_body = False | ||
| found_payload = False | ||
| self.bindings = [Binding.BODY] * len(struct.members) | ||
| self.payload_member = None | ||
| self.event_stream_member = None | ||
|
|
||
| for member in struct.members.values(): | ||
| binding = self._do_match(member) | ||
| self.bindings[member.expect_member_index()] = binding | ||
| found_body = ( | ||
| found_body or binding is Binding.BODY or binding is Binding.HOST | ||
| ) | ||
| if binding is Binding.PAYLOAD: | ||
| found_payload = True | ||
| self.payload_member = member | ||
| if ( | ||
| StreamingTrait.id in member.traits | ||
| and member.shape_type is ShapeType.UNION | ||
| ): | ||
| self.event_stream_member = member | ||
|
|
||
| self.has_body = found_body | ||
| self.has_payload = found_payload | ||
|
|
||
| def should_write_body(self, omit_empty_payload: bool) -> bool: | ||
|
There was a problem hiding this comment. nit; We may want to potentially make this a keyword only arg? Calling |
||
| """Determines whether a body should be written. | ||
|
|
||
| :param omit_empty_payload: Whether a body should be skipped in the case of an | ||
| empty payload. | ||
| """ | ||
| return self.has_body or (not omit_empty_payload and not self.has_payload) | ||
|
|
||
| def match(self, member: Schema) -> Binding: | ||
| """Determines which part of the HTTP message the given member is bound to.""" | ||
| return self.bindings[member.expect_member_index()] | ||
|
|
||
| def _do_match(self, member: Schema) -> Binding: ... | ||
|
|
||
|
|
||
| @dataclass(init=False) | ||
| class RequestBindingMatcher(_BindingMatcher): | ||
| """Matches structure members to HTTP request binding locations.""" | ||
|
|
||
| def __init__(self, struct: Schema) -> None: | ||
| """Initialize a RequestBindingMatcher. | ||
|
|
||
| :param struct: The structure to examine for HTTP bindings. | ||
| """ | ||
| super().__init__(struct, -1) | ||
|
|
||
| def _do_match(self, member: Schema) -> Binding: | ||
| if HTTPLabelTrait.id in member.traits: | ||
| return Binding.LABEL | ||
| if HTTPQueryTrait.id in member.traits: | ||
| return Binding.QUERY | ||
| if HTTPQueryParamsTrait.id in member.traits: | ||
| return Binding.QUERY_PARAMS | ||
| if HTTPHeaderTrait.id in member.traits: | ||
| return Binding.HEADER | ||
| if HTTPPrefixHeadersTrait.id in member.traits: | ||
| return Binding.PREFIX_HEADERS | ||
| if HTTPPayloadTrait.id in member.traits: | ||
| return Binding.PAYLOAD | ||
| if HostLabelTrait.id in member.traits: | ||
| return Binding.HOST | ||
| return Binding.BODY | ||
|
|
||
|
|
||
| @dataclass(init=False) | ||
| class ResponseBindingMatcher(_BindingMatcher): | ||
| """Matches structure members to HTTP response binding locations.""" | ||
|
|
||
| def __init__(self, struct: Schema) -> None: | ||
| """Initialize a ResponseBindingMatcher. | ||
|
|
||
| :param struct: The structure to examine for HTTP bindings. | ||
| """ | ||
| super().__init__(struct, self._compute_response(struct)) | ||
|
|
||
| def _compute_response(self, struct: Schema) -> int: | ||
| if (http_error := struct.get_trait(HTTPErrorTrait)) is not None: | ||
| return http_error.code | ||
| if (error := struct.get_trait(ErrorTrait)) is not None: | ||
| return 400 if error.fault is ErrorFault.CLIENT else 500 | ||
| return -1 | ||
|
|
||
| def _do_match(self, member: Schema) -> Binding: | ||
| if HTTPResponseCodeTrait.id in member.traits: | ||
| return Binding.STATUS | ||
| if HTTPHeaderTrait.id in member.traits: | ||
| return Binding.HEADER | ||
| if HTTPPrefixHeadersTrait.id in member.traits: | ||
| return Binding.PREFIX_HEADERS | ||
| if HTTPPayloadTrait.id in member.traits: | ||
| return Binding.PAYLOAD | ||
| return Binding.BODY | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there we're a reason we're initializing this list here if we're going to swap things out below? Is there a benefit to not just initialize it all at once?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The order of the members in the dictionary isn't the same as the modeled index order, so we can't just append and expect the order to be right. I've got some ideas on how to change schemas so the member order is represented in the dict though. Perhaps I should put that up, it'll reduce the amount of generated code too.