Unverified Commit 2be10ed9 authored by Russell Cohen's avatar Russell Cohen Committed by GitHub
Browse files

fix(codegen): Fix bug triggered by operation input/output naming (#699) 

* fix(codegen): Fix bug triggered by operation input/output naming

During codegeneration, synthetic copies of input & output shapes are created. However, due to a Smithy shape id conflict, these
shapes were overwriting existing shapes in the model instead of creating new shapes.

This lead to a number of bugs exposed by the new s3control model. A minimal example of this model is included as a test in rest-xml-extras.

This commit augments the namespace of synthetic shapes to exist in the `synthetic` Smithy namespace, removing the conflict. After this bug was fixed, a subsequent bug in the serializer function naming was exposed where two shapes with the same name but in different modules generated a conflicting serializer.

Tests:
- [x] new s3control model compiles
- [x] test case added to rest-xml-extras

* cl: update changelog

* fix(codegen): Fix smithy-rs#662 & cleanup

* Update CHANGELOG.md

* fix another unit test
parent 53b35afd

CHANGELOG.md

+1 −0
Original line number Diff line number Diff line
@@ -3,6 +3,7 @@ vNext (Month Day, Year) 


**New This Week**

- :bug: Fixes issue where `Content-Length` header could be duplicated leading to signing failure (aws-sdk-rust#220, smithy-rs#697)

- :bug: Fixes naming collision during generation of model shapes that collide with `<operationname>Input` and `<operationname>Output` (#699)



v0.22 (September 2nd, 2021)

===========================

codegen-test/model/rest-xml-extras.smithy

+13 −0
Original line number Diff line number Diff line
@@ -19,9 +19,22 @@ service RestXmlExtras { 
        PrimitiveIntOpXml,

        ChecksumRequired,

        StringHeader,

        CreateFoo,

    ]

}



/// This operation triggers a name collision between the synthetic `CreateFooInput` and `CreateFooInput`

@http(uri: "/reused-input", method: "POST")

operation CreateFoo {

    input: CreateFooRequest,

}



structure CreateFooRequest {

    input: CreateFooInput

}



structure CreateFooInput {}



@httpRequestTests([{

    id: "RestXmlSerPrimitiveIntUnset",

    protocol: "aws.protocols#restXml",

codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/StructureGenerator.kt

+1 −1
Original line number Diff line number Diff line
@@ -60,7 +60,7 @@ class StructureGenerator( 
    }



    companion object {

        /** Returns whether or not a structure shape requires a fallible builder to be generated. */

        /** Returns whether a structure shape requires a fallible builder to be generated. */

        fun fallibleBuilder(structureShape: StructureShape, symbolProvider: SymbolProvider): Boolean =

            // All inputs should have fallible builders in case a new required field is added in the future

            structureShape.hasTrait<SyntheticInputTrait>() ||

codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/protocols/InlineFunctionNamer.kt

+10 −5
Original line number Diff line number Diff line
@@ -11,6 +11,7 @@ import software.amazon.smithy.model.shapes.MemberShape 
import software.amazon.smithy.model.shapes.OperationShape

import software.amazon.smithy.model.shapes.SetShape

import software.amazon.smithy.model.shapes.Shape

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.smithy.RustSymbolProvider
@@ -44,14 +45,18 @@ fun RustSymbolProvider.serializeFunctionName(shape: Shape): String = shapeFuncti 
 */

fun RustSymbolProvider.deserializeFunctionName(shape: Shape): String = shapeFunctionName("deser", shape)



fun ShapeId.toRustIdentifier(): String {

    return "${namespace.replace(".", "_").toSnakeCase()}_${name.toSnakeCase()}"

}



private fun RustSymbolProvider.shapeFunctionName(prefix: String, shape: Shape): String {

    val symbolNameSnakeCase = toSymbol(shape).name.toSnakeCase()

    val symbolNameSnakeCase = toSymbol(shape).fullName.replace("::", "_").toSnakeCase()

    return prefix + "_" + when (shape) {

        is ListShape -> "list_${shape.id.name.toSnakeCase()}"

        is MapShape -> "map_${shape.id.name.toSnakeCase()}"

        is MemberShape -> "member_${shape.container.name.toSnakeCase()}_${shape.memberName.toSnakeCase()}"

        is ListShape -> "list_${shape.id.toRustIdentifier()}"

        is MapShape -> "map_${shape.id.toRustIdentifier()}"

        is MemberShape -> "member_${shape.container.toRustIdentifier()}_${shape.memberName.toSnakeCase()}"

        is OperationShape -> "operation_$symbolNameSnakeCase"

        is SetShape -> "set_${shape.id.name.toSnakeCase()}"

        is SetShape -> "set_${shape.id.toRustIdentifier()}"

        is StructureShape -> "structure_$symbolNameSnakeCase"

        is UnionShape -> "union_$symbolNameSnakeCase"

        else -> TODO("SerializerFunctionNamer.name: $shape")

codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/protocols/parse/XmlBindingTraitParserGenerator.kt

+7 −1
Original line number Diff line number Diff line
@@ -434,7 +434,13 @@ class XmlBindingTraitParserGenerator( 
                }

                withBlock("Ok(builder.build()", ")") {

                    if (StructureGenerator.fallibleBuilder(shape, symbolProvider)) {

                        rustTemplate(""".map_err(|_|{XmlError}::custom("missing field"))?""", *codegenScope)

                        // NOTE:(rcoh) This branch is unreachable given the current nullability rules.

                        // Only synthetic inputs can have fallible builders, but synthetic inputs can never be parsed

                        // (because they're inputs, only outputs will be parsed!)



                        // I'm leaving this branch here so that the binding trait parser generator would work for a server

                        // side implementation in the future.

                        rustTemplate(""".map_err(|_|#{XmlError}::custom("missing field"))?""", *codegenScope)

                    }

                }

            }