Unverified Commit 63a1e78f authored by david-perez's avatar david-perez Committed by GitHub
Browse files

Fix server code generation of `@httpPayload`-bound constrained shapes (#2584) 

The issue is we're not changing the return type of the payload
deserializing function to be the unconstrained type (e.g. the builder in
case of an `@httpPayload`-bound structure shape) when the shape is
constrained.

Yet another example of why code-generating `constraints.smithy` (see
#2101)
is important.

Closes #2583.

## Testing

The added integration test operation fails to compile without this
patch.

## Checklist
<!--- If a checkbox below is not applicable, then please DELETE it
rather than leaving it unchecked -->
- [x] I have updated `CHANGELOG.next.toml` if I made changes to the
smithy-rs codegen or runtime crates

----

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._
parent d8cda153

CHANGELOG.next.toml

+6 −0
Original line number Diff line number Diff line
@@ -122,3 +122,9 @@ message = "Fix generation of constrained shapes reaching `@sensitive` shapes" 
references = ["smithy-rs#2582", "smithy-rs#2585"]

meta = { "breaking" = false, "tada" = false, "bug" = true, "target" = "server" }

author = "david-perez"



[[smithy-rs]]

message = "Fix server code generation bug affecting constrained shapes bound with `@httpPayload`"

references = ["smithy-rs#2583", "smithy-rs#2584"]

meta = { "breaking" = false, "tada" = false, "bug" = true, "target" = "server" }

author = "david-perez"

codegen-core/common-test-models/constraints.smithy

+15 −0
Original line number Diff line number Diff line
@@ -12,7 +12,9 @@ service ConstraintsService { 
    operations: [

        ConstrainedShapesOperation,

        ConstrainedHttpBoundShapesOperation,

        ConstrainedHttpPayloadBoundShapeOperation,

        ConstrainedRecursiveShapesOperation,



        // `httpQueryParams` and `httpPrefixHeaders` are structurually

        // exclusive, so we need one operation per target shape type

        // combination.
@@ -59,6 +61,13 @@ operation ConstrainedHttpBoundShapesOperation { 
    errors: [ValidationException]

}



@http(uri: "/constrained-http-payload-bound-shape-operation", method: "POST")

operation ConstrainedHttpPayloadBoundShapeOperation {

    input: ConstrainedHttpPayloadBoundShapeOperationInputOutput,

    output: ConstrainedHttpPayloadBoundShapeOperationInputOutput,

    errors: [ValidationException]

}



@http(uri: "/constrained-recursive-shapes-operation", method: "POST")

operation ConstrainedRecursiveShapesOperation {

    input: ConstrainedRecursiveShapesOperationInputOutput,
@@ -311,6 +320,12 @@ structure ConstrainedHttpBoundShapesOperationInputOutput { 
    enumStringListQuery: ListOfEnumString,

}



structure ConstrainedHttpPayloadBoundShapeOperationInputOutput {

    @required

    @httpPayload

    httpPayloadBoundConstrainedShape: ConA

}



structure QueryParamsTargetingMapOfPatternStringOperationInputOutput {

    @httpQueryParams

    mapOfPatternString: MapOfPatternString

codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/http/HttpBindingGenerator.kt

+1 −1
Original line number Diff line number Diff line
@@ -235,7 +235,7 @@ class HttpBindingGenerator( 
                // The output needs to be Optional when deserializing the payload body or the caller signature

                // will not match.

                val outputT = symbolProvider.toSymbol(binding.member).makeOptional()

                rustBlock("pub fn $fnName(body: &[u8]) -> std::result::Result<#T, #T>", outputT, errorSymbol) {

                rustBlock("pub(crate) fn $fnName(body: &[u8]) -> std::result::Result<#T, #T>", outputT, errorSymbol) {

                    deserializePayloadBody(

                        binding,

                        errorSymbol,

codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/protocols/HttpBoundProtocolPayloadGenerator.kt

+5 −1
Original line number Diff line number Diff line
@@ -239,7 +239,11 @@ class HttpBoundProtocolPayloadGenerator( 
    ) {

        val ref = if (payloadMetadata.takesOwnership) "" else "&"

        val serializer = protocolFunctions.serializeFn(member, fnNameSuffix = "http_payload") { fnName ->

            val outputT = if (member.isStreaming(model)) symbolProvider.toSymbol(member) else RuntimeType.ByteSlab.toSymbol()

            val outputT = if (member.isStreaming(model)) {

                symbolProvider.toSymbol(member)

            } else {

                RuntimeType.ByteSlab.toSymbol()

            }

            rustBlockTemplate(

                "pub fn $fnName(payload: $ref#{Member}) -> Result<#{outputT}, #{BuildError}>",

                "Member" to symbolProvider.toSymbol(member),

codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/protocols/parse/JsonParserGenerator.kt

+4 −3
Original line number Diff line number Diff line
@@ -154,14 +154,15 @@ class JsonParserGenerator( 


    override fun payloadParser(member: MemberShape): RuntimeType {

        val shape = model.expectShape(member.target)

        val returnSymbolToParse = returnSymbolToParse(shape)

        check(shape is UnionShape || shape is StructureShape || shape is DocumentShape) {

            "payload parser should only be used on structures & unions"

            "Payload parser should only be used on structure shapes, union shapes, and document shapes."

        }

        return protocolFunctions.deserializeFn(shape, fnNameSuffix = "payload") { fnName ->

            rustBlockTemplate(

                "pub fn $fnName(input: &[u8]) -> Result<#{Shape}, #{Error}>",

                "pub(crate) fn $fnName(input: &[u8]) -> Result<#{ReturnType}, #{Error}>",

                *codegenScope,

                "Shape" to symbolProvider.toSymbol(shape),

                "ReturnType" to returnSymbolToParse.symbol,

            ) {

                val input = if (shape is DocumentShape) {

                    "input"