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

Fix codegen for unions with the @httpPayload trait #2969

Merged
merged 1 commit into from
Aug 31, 2023
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
6 changes: 6 additions & 0 deletions CHANGELOG.next.toml
Original file line number Diff line number Diff line change
Expand Up @@ -84,3 +84,9 @@ message = "Generate a region setter when a model uses SigV4."
references = ["smithy-rs#2960"]
meta = { "breaking" = false, "tada" = false, "bug" = true, "target" = "client" }
author = "jdisanti"

[[smithy-rs]]
message = "Fix code generation for union members with the `@httpPayload` trait."
references = ["smithy-rs#2969", "smithy-rs#1896"]
meta = { "breaking" = false, "tada" = false, "bug" = true, "target" = "all" }
author = "jdisanti"
96 changes: 96 additions & 0 deletions codegen-client-test/model/rest-xml-extras.smithy
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ service RestXmlExtras {
StringHeader,
CreateFoo,
RequiredMember,
// TODO(https://github.com/awslabs/smithy-rs/issues/2968): Remove the following once these tests are included in Smithy
// They're being added in https://github.com/smithy-lang/smithy/pull/1908
HttpPayloadWithUnion,
]
}

Expand Down Expand Up @@ -257,3 +260,96 @@ structure RequiredMemberInputOutput {
@required
requiredString: String
}

// TODO(https://github.com/awslabs/smithy-rs/issues/2968): Delete the HttpPayloadWithUnion tests below once Smithy vends them
// They're being added in https://github.com/smithy-lang/smithy/pull/1908

/// This example serializes a union in the payload.
@idempotent
@http(uri: "/HttpPayloadWithUnion", method: "PUT")
operation HttpPayloadWithUnion {
input: HttpPayloadWithUnionInputOutput,
output: HttpPayloadWithUnionInputOutput
}

apply HttpPayloadWithUnion @httpRequestTests([
{
id: "RestXmlHttpPayloadWithUnion",
documentation: "Serializes a union in the payload.",
protocol: restXml,
method: "PUT",
uri: "/HttpPayloadWithUnion",
body: """
<UnionPayload>
<greeting>hello</greeting>
</UnionPayload>""",
bodyMediaType: "application/xml",
headers: {
"Content-Type": "application/xml",
},
requireHeaders: [
"Content-Length"
],
params: {
nested: {
greeting: "hello"
}
}
},
{
id: "RestXmlHttpPayloadWithUnsetUnion",
documentation: "No payload is sent if the union has no value.",
protocol: restXml,
method: "PUT",
uri: "/HttpPayloadWithUnion",
body: "",
headers: {
"Content-Type": "application/xml",
"Content-Length": "0"
},
params: {}
}
])

apply HttpPayloadWithUnion @httpResponseTests([
{
id: "RestXmlHttpPayloadWithUnion",
documentation: "Serializes a union in the payload.",
protocol: restXml,
code: 200,
body: """
<UnionPayload>
<greeting>hello</greeting>
</UnionPayload>""",
bodyMediaType: "application/xml",
headers: {
"Content-Type": "application/xml",
},
params: {
nested: {
greeting: "hello"
}
}
},
{
id: "RestXmlHttpPayloadWithUnsetUnion",
documentation: "No payload is sent if the union has no value.",
protocol: restXml,
code: 200,
body: "",
headers: {
"Content-Type": "application/xml",
"Content-Length": "0"
},
params: {}
}
])

structure HttpPayloadWithUnionInputOutput {
@httpPayload
nested: UnionPayload,
}

union UnionPayload {
greeting: String
}
96 changes: 96 additions & 0 deletions codegen-core/common-test-models/rest-json-extras.smithy
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,9 @@ service RestJsonExtras {
NullInNonSparse,
CaseInsensitiveErrorOperation,
EmptyStructWithContentOnWireOp,
// TODO(https://github.com/awslabs/smithy-rs/issues/2968): Remove the following once these tests are included in Smithy
// They're being added in https://github.com/smithy-lang/smithy/pull/1908
HttpPayloadWithUnion,
],
errors: [ExtraError]
}
Expand Down Expand Up @@ -348,3 +351,96 @@ structure EmptyStructWithContentOnWireOpOutput {
operation EmptyStructWithContentOnWireOp {
output: EmptyStructWithContentOnWireOpOutput,
}

// TODO(https://github.com/awslabs/smithy-rs/issues/2968): Delete the HttpPayloadWithUnion tests below once Smithy vends them
// They're being added in https://github.com/smithy-lang/smithy/pull/1908

/// This examples serializes a union in the payload.
@idempotent
@http(uri: "/HttpPayloadWithUnion", method: "PUT")
operation HttpPayloadWithUnion {
input: HttpPayloadWithUnionInputOutput,
output: HttpPayloadWithUnionInputOutput
}

structure HttpPayloadWithUnionInputOutput {
@httpPayload
nested: UnionPayload,
}

union UnionPayload {
greeting: String
}

apply HttpPayloadWithUnion @httpRequestTests([
{
id: "RestJsonHttpPayloadWithUnion",
documentation: "Serializes a union in the payload.",
protocol: restJson1,
method: "PUT",
uri: "/HttpPayloadWithUnion",
body: """
{
"greeting": "hello"
}""",
bodyMediaType: "application/json",
headers: {
"Content-Type": "application/json"
},
requireHeaders: [
"Content-Length"
],
params: {
nested: {
greeting: "hello"
}
}
},
{
id: "RestJsonHttpPayloadWithUnsetUnion",
documentation: "No payload is sent if the union has no value.",
protocol: restJson1,
method: "PUT",
uri: "/HttpPayloadWithUnion",
body: "",
headers: {
"Content-Type": "application/json",
"Content-Length": "0"
},
params: {}
}
])

apply HttpPayloadWithUnion @httpResponseTests([
{
id: "RestJsonHttpPayloadWithUnion",
documentation: "Serializes a union in the payload.",
protocol: restJson1,
code: 200,
body: """
{
"greeting": "hello"
}""",
bodyMediaType: "application/json",
headers: {
"Content-Type": "application/json"
},
params: {
nested: {
greeting: "hello"
}
}
},
{
id: "RestJsonHttpPayloadWithUnsetUnion",
documentation: "No payload is sent if the union has no value.",
protocol: restJson1,
code: 200,
body: "",
headers: {
"Content-Type": "application/json",
"Content-Length": "0"
},
params: {}
}
])
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ class HttpBoundProtocolPayloadGenerator(
""",
)
is StructureShape -> rust("#T()", serializerGenerator.unsetStructure(targetShape))
is UnionShape -> throw CodegenException("Currently unsupported. Tracking issue: https://github.com/awslabs/smithy-rs/issues/1896")
is UnionShape -> rust("#T()", serializerGenerator.unsetUnion(targetShape))
else -> throw CodegenException("`httpPayload` on member shapes targeting shapes of type ${targetShape.type} is unsupported")
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import software.amazon.smithy.rust.codegen.core.rustlang.withBlock
import software.amazon.smithy.rust.codegen.core.rustlang.writable
import software.amazon.smithy.rust.codegen.core.smithy.CodegenContext
import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType
import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType.Companion.preludeScope
import software.amazon.smithy.rust.codegen.core.smithy.RustSymbolProvider
import software.amazon.smithy.rust.codegen.core.smithy.customize.NamedCustomization
import software.amazon.smithy.rust.codegen.core.smithy.customize.Section
Expand Down Expand Up @@ -170,7 +171,7 @@ class JsonSerializerGenerator(
private val runtimeConfig = codegenContext.runtimeConfig
private val protocolFunctions = ProtocolFunctions(codegenContext)
private val codegenScope = arrayOf(
"String" to RuntimeType.String,
*preludeScope,
"Error" to runtimeConfig.serializationError(),
"SdkBody" to RuntimeType.sdkBody(runtimeConfig),
"JsonObjectWriter" to RuntimeType.smithyJson(runtimeConfig).resolve("serialize::JsonObjectWriter"),
Expand Down Expand Up @@ -232,7 +233,7 @@ class JsonSerializerGenerator(
}

override fun unsetStructure(structure: StructureShape): RuntimeType =
ProtocolFunctions.crossOperationFn("rest_json_unsetpayload") { fnName ->
ProtocolFunctions.crossOperationFn("rest_json_unset_struct_payload") { fnName ->
rustTemplate(
"""
pub fn $fnName() -> #{ByteSlab} {
Expand All @@ -243,6 +244,14 @@ class JsonSerializerGenerator(
)
}

override fun unsetUnion(union: UnionShape): RuntimeType =
ProtocolFunctions.crossOperationFn("rest_json_unset_union_payload") { fnName ->
rustTemplate(
"pub fn $fnName() -> #{ByteSlab} { #{Vec}::new() }",
*codegenScope,
)
}

override fun operationInputSerializer(operationShape: OperationShape): RuntimeType? {
// Don't generate an operation JSON serializer if there is no JSON body.
val httpDocumentMembers = httpBindingResolver.requestMembers(operationShape, HttpLocation.DOCUMENT)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,10 @@ abstract class QuerySerializerGenerator(codegenContext: CodegenContext) : Struct
TODO("AwsQuery doesn't support payload serialization")
}

override fun unsetUnion(union: UnionShape): RuntimeType {
TODO("AwsQuery doesn't support payload serialization")
}

override fun operationInputSerializer(operationShape: OperationShape): RuntimeType? {
val inputShape = operationShape.inputShape(model)
return protocolFunctions.serializeFn(inputShape, fnNameSuffix = "input") { fnName ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import software.amazon.smithy.model.shapes.MemberShape
import software.amazon.smithy.model.shapes.OperationShape
import software.amazon.smithy.model.shapes.ShapeId
import software.amazon.smithy.model.shapes.StructureShape
import software.amazon.smithy.model.shapes.UnionShape
import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType

interface StructuredDataSerializerGenerator {
Expand All @@ -27,12 +28,22 @@ interface StructuredDataSerializerGenerator {
* Generate the correct data when attempting to serialize a structure that is unset
*
* ```rust
* fn rest_json_unsetpayload() -> Vec<u8> {
* fn rest_json_unset_struct_payload() -> Vec<u8> {
* ...
* }
*/
fun unsetStructure(structure: StructureShape): RuntimeType

/**
* Generate the correct data when attempting to serialize a union that is unset
*
* ```rust
* fn rest_json_unset_union_payload() -> Vec<u8> {
* ...
* }
*/
fun unsetUnion(union: UnionShape): RuntimeType

/**
* Generate a serializer for an operation input structure.
* This serializer is only used by clients.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import software.amazon.smithy.rust.codegen.core.rustlang.stripOuter
import software.amazon.smithy.rust.codegen.core.rustlang.withBlock
import software.amazon.smithy.rust.codegen.core.smithy.CodegenContext
import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType
import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType.Companion.preludeScope
import software.amazon.smithy.rust.codegen.core.smithy.generators.UnionGenerator
import software.amazon.smithy.rust.codegen.core.smithy.generators.renderUnknownVariant
import software.amazon.smithy.rust.codegen.core.smithy.generators.serializationError
Expand Down Expand Up @@ -176,8 +177,8 @@ class XmlBindingTraitSerializerGenerator(
}
}

override fun unsetStructure(structure: StructureShape): RuntimeType {
return ProtocolFunctions.crossOperationFn("rest_xml_unset_payload") { fnName ->
override fun unsetStructure(structure: StructureShape): RuntimeType =
ProtocolFunctions.crossOperationFn("rest_xml_unset_struct_payload") { fnName ->
rustTemplate(
"""
pub fn $fnName() -> #{ByteSlab} {
Expand All @@ -187,7 +188,15 @@ class XmlBindingTraitSerializerGenerator(
"ByteSlab" to RuntimeType.ByteSlab,
)
}
}

override fun unsetUnion(union: UnionShape): RuntimeType =
ProtocolFunctions.crossOperationFn("rest_xml_unset_union_payload") { fnName ->
rustTemplate(
"pub fn $fnName() -> #{ByteSlab} { #{Vec}::new() }",
*preludeScope,
"ByteSlab" to RuntimeType.ByteSlab,
)
}

override fun operationOutputSerializer(operationShape: OperationShape): RuntimeType? {
val outputShape = operationShape.outputShape(model)
Expand Down