Unverified Commit 8733038a authored by Fahad Zubair's avatar Fahad Zubair Committed by GitHub
Browse files

Include error shapes in member constraint transformation (#3787) 



Constrained members are extracted into standalone shapes with
constraints on them, and the original shape's targets are replaced by
the newly created shapes in the model.

This PR adds support for including error shapes in this transformation.

Co-authored-by: default avatarFahad Zubair <fahadzub@amazon.com>
parent a107c015

codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/transformers/ConstrainedMemberTransform.kt

+5 −8
Original line number Diff line number Diff line
@@ -18,8 +18,6 @@ import software.amazon.smithy.model.traits.RequiredTrait 
import software.amazon.smithy.model.traits.Trait

import software.amazon.smithy.model.transform.ModelTransformer

import software.amazon.smithy.rust.codegen.core.smithy.DirectedWalker

import software.amazon.smithy.rust.codegen.core.smithy.traits.SyntheticInputTrait

import software.amazon.smithy.rust.codegen.core.smithy.traits.SyntheticOutputTrait

import software.amazon.smithy.rust.codegen.core.util.UNREACHABLE

import software.amazon.smithy.rust.codegen.core.util.orNull

import software.amazon.smithy.rust.codegen.server.smithy.allConstraintTraits
@@ -75,16 +73,15 @@ object ConstrainedMemberTransform { 
        val additionalNames = HashSet<ShapeId>()

        val walker = DirectedWalker(model)



        // Find all synthetic input / output structures that have been added by

        // the OperationNormalizer, get constrained members out of those structures,

        // convert them into non-constrained members and then pass them to the transformer.

        // The transformer will add new shapes, and will replace existing member shapes' target

        // with the newly added shapes.

        // Begin with the operations listed in the service and traverse the inputs, outputs, and errors specified

        // in each operation. Extract the target shapes of constrained members from structures, lists, maps, and

        // unions into standalone shapes with constraints. Update the model by incorporating these extracted shapes

        // and adjust the target shapes of the original constrained members to reference the newly created

        // constrained types.

        val transformations =

            model.operationShapes

                .flatMap { listOfNotNull(it.input.orNull(), it.output.orNull()) + it.errors }

                .mapNotNull { model.expectShape(it).asStructureShape().orElse(null) }

                .filter { it.hasTrait(SyntheticInputTrait.ID) || it.hasTrait(SyntheticOutputTrait.ID) }

                .flatMap { walker.walkShapes(it) }

                .filter { it is StructureShape || it is ListShape || it is UnionShape || it is MapShape }

                .flatMap { it.constrainedMembers() }

codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ConstraintsMemberShapeTest.kt

+38 −28
Original line number Diff line number Diff line
@@ -24,25 +24,29 @@ import software.amazon.smithy.rust.codegen.core.util.toPascalCase 
import software.amazon.smithy.rust.codegen.core.util.toSnakeCase

import software.amazon.smithy.rust.codegen.server.smithy.testutil.serverIntegrationTest

import software.amazon.smithy.rust.codegen.server.smithy.transformers.ConstrainedMemberTransform

import kotlin.streams.toList



class ConstraintsMemberShapeTest {

    private val outputModelOnly =

    private val sampleModel =

        """

        namespace constrainedMemberShape



        use smithy.framework#ValidationException

        use aws.protocols#restJson1

        use aws.api#data



        @restJson1

        service ConstrainedService {

            operations: [OperationUsingGet]

            operations: [SampleOperation]

        }



        @http(uri: "/anOperation", method: "GET")

        operation OperationUsingGet {

            output : OperationUsingGetOutput

        @http(uri: "/anOperation", method: "POST")

        operation SampleOperation {

            output: SampleInputOutput

            input: SampleInputOutput

            errors: [ValidationException, ErrorWithMemberConstraint]

        }

        structure OperationUsingGetOutput {

        structure SampleInputOutput {

            plainLong : Long

            plainInteger : Integer

            plainShort : Short
@@ -148,6 +152,12 @@ class ConstraintsMemberShapeTest { 
        string PatternString

        @range(min: 0, max:1000)

        integer RangedInteger



        @error("server")

        structure ErrorWithMemberConstraint {

            @range(min: 100, max: 999)

            statusCode: Integer

        }

        """.asSmithyModel()



    private fun loadModel(model: Model): Model =
@@ -155,16 +165,16 @@ class ConstraintsMemberShapeTest { 


    @Test

    fun `non constrained fields should not be changed`() {

        val transformedModel = loadModel(outputModelOnly)

        val transformedModel = loadModel(sampleModel)



        fun checkFieldTargetRemainsSame(fieldName: String) {

            checkMemberShapeIsSame(

                transformedModel,

                outputModelOnly,

                "constrainedMemberShape.synthetic#OperationUsingGetOutput\$$fieldName",

                "constrainedMemberShape#OperationUsingGetOutput\$$fieldName",

                sampleModel,

                "constrainedMemberShape.synthetic#SampleOperationOutput\$$fieldName",

                "constrainedMemberShape#SampleInputOutput\$$fieldName",

            ) {

                "OperationUsingGetOutput$fieldName has changed whereas it is not constrained and should have remained same"

                "SampleInputOutput$fieldName has changed whereas it is not constrained and should have remained same"

            }

        }
@@ -189,7 +199,7 @@ class ConstraintsMemberShapeTest { 


        checkMemberShapeIsSame(

            transformedModel,

            outputModelOnly,

            sampleModel,

            "constrainedMemberShape#StructWithConstrainedMember\$long",

            "constrainedMemberShape#StructWithConstrainedMember\$long",

        )
@@ -197,10 +207,10 @@ class ConstraintsMemberShapeTest { 


    @Test

    fun `constrained members should have a different target now`() {

        val transformedModel = loadModel(outputModelOnly)

        val transformedModel = loadModel(sampleModel)

        checkMemberShapeChanged(

            transformedModel,

            outputModelOnly,

            sampleModel,

            "constrainedMemberShape#PatternStringListOverride\$member",

            "constrainedMemberShape#PatternStringListOverride\$member",

        )
@@ -208,9 +218,9 @@ class ConstraintsMemberShapeTest { 
        fun checkSyntheticFieldTargetChanged(fieldName: String) {

            checkMemberShapeChanged(

                transformedModel,

                outputModelOnly,

                "constrainedMemberShape.synthetic#OperationUsingGetOutput\$$fieldName",

                "constrainedMemberShape#OperationUsingGetOutput\$$fieldName",

                sampleModel,

                "constrainedMemberShape.synthetic#SampleOperationOutput\$$fieldName",

                "constrainedMemberShape#SampleInputOutput\$$fieldName",

            ) {

                "constrained member $fieldName should have been changed into a new type."

            }
@@ -219,7 +229,7 @@ class ConstraintsMemberShapeTest { 
        fun checkFieldTargetChanged(memberNameWithContainer: String) {

            checkMemberShapeChanged(

                transformedModel,

                outputModelOnly,

                sampleModel,

                "constrainedMemberShape#$memberNameWithContainer",

                "constrainedMemberShape#$memberNameWithContainer",

            ) {
@@ -251,36 +261,36 @@ class ConstraintsMemberShapeTest { 


    @Test

    fun `extra trait on a constrained member should remain on it`() {

        val transformedModel = loadModel(outputModelOnly)

        val transformedModel = loadModel(sampleModel)

        checkShapeHasTrait(

            transformedModel,

            outputModelOnly,

            "constrainedMemberShape.synthetic#OperationUsingGetOutput\$constrainedPatternString",

            "constrainedMemberShape#OperationUsingGetOutput\$constrainedPatternString",

            sampleModel,

            "constrainedMemberShape.synthetic#SampleOperationOutput\$constrainedPatternString",

            "constrainedMemberShape#SampleInputOutput\$constrainedPatternString",

            DataTrait("content", SourceLocation.NONE),

        )

    }



    @Test

    fun `required remains on constrained member shape`() {

        val transformedModel = loadModel(outputModelOnly)

        val transformedModel = loadModel(sampleModel)

        checkShapeHasTrait(

            transformedModel,

            outputModelOnly,

            "constrainedMemberShape.synthetic#OperationUsingGetOutput\$requiredConstrainedString",

            "constrainedMemberShape#OperationUsingGetOutput\$requiredConstrainedString",

            sampleModel,

            "constrainedMemberShape.synthetic#SampleOperationOutput\$requiredConstrainedString",

            "constrainedMemberShape#SampleInputOutput\$requiredConstrainedString",

            RequiredTrait(),

        )

    }



    @Test

    fun `generate code and check member constrained shapes are in the right modules`() {

        serverIntegrationTest(outputModelOnly, IntegrationTestParams(service = "constrainedMemberShape#ConstrainedService")) { codegenContext, rustCrate ->

        serverIntegrationTest(sampleModel, IntegrationTestParams(service = "constrainedMemberShape#ConstrainedService")) { codegenContext, rustCrate ->

            fun RustWriter.testTypeExistsInBuilderModule(typeName: String) {

                unitTest(

                    "builder_module_has_${typeName.toSnakeCase()}",

                    """

                    #[allow(unused_imports)] use crate::output::operation_using_get_output::$typeName;

                    #[allow(unused_imports)] use crate::output::sample_operation_output::$typeName;

                    """,

                )

            }