fix: Ignore httpQuery trait on response members but don't ignore member #723
Conversation
|
|
||
| import ClientRuntime | ||
|
|
||
| public struct EndpointParams { |
There was a problem hiding this comment.
Unrelated new file added by codegen changes in another PR.
| @@ -7,9 +7,6 @@ import SmithyReadWrite | |||
| extension InvokeOutput { | |||
|
|
|||
| static func httpOutput(from httpResponse: ClientRuntime.HttpResponse) async throws -> InvokeOutput { | |||
| let data = try await httpResponse.data() | |||
| let responseReader = try SmithyJSON.Reader.from(data: data) | |||
| let reader = responseReader | |||
There was a problem hiding this comment.
This reader is not used; the improved logic in this PR does not create it.
| @@ -6,6 +6,8 @@ import WeatherSDK | |||
| extension ListCitiesInput: Swift.Equatable { | |||
|
|
|||
| public static func ==(lhs: ListCitiesInput, rhs: ListCitiesInput) -> Bool { | |||
| if lhs.nextToken != rhs.nextToken { return false } | |||
| if lhs.pageSize != rhs.pageSize { return false } | |||
There was a problem hiding this comment.
These are members that have the @httpQuery trait and were previously not included in the comparison
| @@ -172,7 +171,7 @@ open class HttpProtocolUnitTestResponseGenerator protected constructor(builder: | |||
| writer.openBlock("public static func ==(lhs: \$L, rhs: \$L) -> Bool {", "}", symbol.fullName, symbol.fullName) { | |||
| when (shape) { | |||
| is StructureShape -> { | |||
| shape.members().filter { !it.hasTrait<HttpQueryTrait>() }.forEach { member -> | |||
| shape.members().forEach { member -> | |||
There was a problem hiding this comment.
Members with the @httpQuery trait are no longer excluded when comparing equality in response tests.
| @@ -66,7 +65,6 @@ class HTTPResponseBindingErrorInitGenerator( | |||
| HTTPResponseHeaders(ctx, true, headerBindings, customizations.defaultTimestampFormat, writer).render() | |||
| HTTPResponsePrefixHeaders(ctx, responseBindings, writer).render() | |||
| httpResponseTraitPayload(ctx, responseBindings, structureShape, writer) | |||
| HTTPResponseTraitQueryParams(ctx, responseBindings, writer).render() | |||
There was a problem hiding this comment.
Response members bound to a @httpQuery trait no longer receive any special rendering; instead they are rendered along with the payload members.
| val targetShape = ctx.model.expectShape(binding.member.target) | ||
| return binding.location == HttpBinding.Location.PAYLOAD && | ||
| (targetShape is StructureShape || targetShape is UnionShape) && | ||
| !targetShape.members().isEmpty() |
There was a problem hiding this comment.
This new logic in the needsAReader() function prevents creating unused readers (and the compiler warnings that come with them) by rendering reader creation only when a payload is a structure or union that has at least one member.
Other payloads (a string, an enum, a blob, smithy Document, etc.) do not require a reader. Also a reader is not required for a structure or union that does not have at least one member.
| * SPDX-License-Identifier: Apache-2.0. | ||
| */ | ||
|
|
||
| package software.amazon.smithy.swift.codegen.integration.httpResponse.bindingTraits |
There was a problem hiding this comment.
This used to render a statement to set a member to nil if it is in the response and has the @httpQuery trait. This was incorrect.
Instead, this renderer is removed and a member with the @httpQuery trait will be rendered to the response like any other member.
| val bodyMembersWithoutQueryTrait = bodyMembers | ||
| .filter { !it.member.hasTrait(HttpQueryTrait::class.java) } | ||
| .toMutableSet() | ||
| val bodyMembers = responseBindings.filter { it.location == HttpBinding.Location.DOCUMENT }.toSet() |
There was a problem hiding this comment.
Simplified this logic since there is no need to separate members with the @httpQuery trait from other members.
| @@ -106,9 +106,6 @@ extension HttpResponseCodeOutput { | |||
| extension InlineDocumentAsPayloadOutput { | |||
|
|
|||
| static func httpOutput(from httpResponse: ClientRuntime.HttpResponse) async throws -> InlineDocumentAsPayloadOutput { | |||
| let data = try await httpResponse.data() | |||
| let responseReader = try SmithyJSON.Reader.from(data: data) | |||
| let reader = responseReader | |||
There was a problem hiding this comment.
This codegen test no longer expects an unused reader to be rendered.
| @@ -16,7 +16,7 @@ import shouldSyntacticSanityCheck | |||
| class HttpResponseBindingIgnoreQuery { | |||
|
|
|||
| @Test | |||
| fun `001 Output httpResponseBinding sets query to nil`() { | |||
| fun `001 Output httpResponseBinding ignores query trait & decodes field`() { | |||
There was a problem hiding this comment.
The premise of this test is incorrect. The member with @httpQuery trait should not be set to nil; it should be read from the response.
The name of the test & the expectation are adjusted accordingly.
Issue #
awslabs/aws-sdk-swift#1500
Description of changes
@httpQuerytrait just like for any other member@httpQuerytraitreaderin the response binding method so that areaderis only created when it is actually used. This reduces dead code and eliminates compiler warnings.Scope
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.