Unverified Commit 3e6adcd4 authored by david-perez's avatar david-perez Committed by GitHub
Browse files

Disallow ignoring a constrained shape in an event stream's closure (#2113) 

The server SDK will otherwise produce Rust code that cannot be compiled,
so it doesn't make sense to prompt users to disregard the warning and
opt into `ignoreUnsupportedConstraints`. To continue synthesizing the
model, the user must remove any constrained traits from the event
stream's closure.

This commit also tweaks formatting of errors yielded by
`ValidateUnsupportedConstraints`. Prior to this commit, they were a bit
off due to carelessness when interpolation occurs. For example:

```
[SEVERE] Operation com.amazonaws.constraints#ConstrainedShapesOperation takes in input that is constrained(https://awslabs.github.io/smithy/2.0/spec/constraint-traits.html), and as such can fail with a validationexception. You must model this behavior in the operation shape in your model file.
```
parent 1deb644a

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

+21 −13
Original line number Diff line number Diff line
@@ -37,15 +37,21 @@ private sealed class UnsupportedConstraintMessageKind { 
    private val constraintTraitsUberIssue = "https://github.com/awslabs/smithy-rs/issues/1401"



    fun intoLogMessage(ignoreUnsupportedConstraints: Boolean): LogMessage {

        fun buildMessage(intro: String, willSupport: Boolean, trackingIssue: String) =

            """

        fun buildMessage(intro: String, willSupport: Boolean, trackingIssue: String, canBeIgnored: Boolean = true): String {

            var msg = """

                    $intro

            This is not supported in the smithy-rs server SDK.

            ${if (willSupport) "It will be supported in the future." else ""}

            See the tracking issue ($trackingIssue).

                    This is not supported in the smithy-rs server SDK."""

            if (willSupport) {

                msg += """

                    It will be supported in the future. See the tracking issue ($trackingIssue)."""

            }

            if (canBeIgnored) {

                msg += """

                    If you want to go ahead and generate the server SDK ignoring unsupported constraint traits, set the key `ignoreUnsupportedConstraints`

            inside the `runtimeConfig.codegenConfig` JSON object in your `smithy-build.json` to `true`.

            """.trimIndent().replace("\n", " ")

                    inside the `runtimeConfig.codegenConfig` JSON object in your `smithy-build.json` to `true`."""

            }

            return msg.trimIndent().replace("\n", " ")

        }



        fun buildMessageShapeHasUnsupportedConstraintTrait(

            shape: Shape,
@@ -67,14 +73,16 @@ private sealed class UnsupportedConstraintMessageKind { 
            )



            is UnsupportedConstraintOnShapeReachableViaAnEventStream -> LogMessage(

                level,

                Level.SEVERE,

                buildMessage(

                    """

                    The ${shape.type} shape `${shape.id}` has the constraint trait `${constraintTrait.toShapeId()}` attached.

                    This shape is also part of an event stream; it is unclear what the semantics for constrained shapes in event streams are.

                    Please remove the trait from the shape to synthesize your model.

                    """.trimIndent().replace("\n", " "),

                    willSupport = false,

                    "https://github.com/awslabs/smithy/issues/1388",

                    canBeIgnored = false,

                ),

            )
@@ -162,8 +170,8 @@ fun validateOperationsWithConstrainedInputHaveValidationExceptionAttached( 
                Level.SEVERE,

                """

                Operation ${it.shape.id} takes in input that is constrained 

                (https://awslabs.github.io/smithy/2.0/spec/constraint-traits.html), and as such can fail with a validation

                exception. You must model this behavior in the operation shape in your model file.

                (https://awslabs.github.io/smithy/2.0/spec/constraint-traits.html), and as such can fail with a 

                validation exception. You must model this behavior in the operation shape in your model file.

                """.trimIndent().replace("\n", "") +

                    """

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

+54 −26
Original line number Diff line number Diff line
@@ -12,6 +12,7 @@ import io.kotest.matchers.collections.shouldHaveAtLeastSize 
import io.kotest.matchers.collections.shouldHaveSize

import io.kotest.matchers.shouldBe

import io.kotest.matchers.string.shouldContain

import io.kotest.matchers.string.shouldNotContain

import org.junit.jupiter.api.Test

import software.amazon.smithy.model.Model

import software.amazon.smithy.model.shapes.ServiceShape
@@ -56,7 +57,17 @@ internal class ValidateUnsupportedConstraintsAreNotUsedTest { 
        val validationResult = validateOperationsWithConstrainedInputHaveValidationExceptionAttached(model, service)



        validationResult.messages shouldHaveSize 1

        validationResult.messages[0].message shouldContain "Operation test#TestOperation takes in input that is constrained"



        // Asserts the exact message, to ensure the formatting is appropriate.

        validationResult.messages[0].message shouldBe """Operation test#TestOperation takes in input that is constrained (https://awslabs.github.io/smithy/2.0/spec/constraint-traits.html), and as such can fail with a validation exception. You must model this behavior in the operation shape in your model file.

```smithy

use smithy.framework#ValidationException



operation TestOperation {

    ...

    errors: [..., ValidationException] // <-- Add this.

}

```"""

    }



    @Test
@@ -120,9 +131,7 @@ internal class ValidateUnsupportedConstraintsAreNotUsedTest { 
        }

    }



    @Test

    fun `it should detect when constraint traits in event streams are used`() {

        val model =

    val constrainedShapesInEventStreamModel =

        """

        $baseModel
@@ -148,7 +157,10 @@ internal class ValidateUnsupportedConstraintsAreNotUsedTest { 
        @length(min: 1)

        string LengthString

        """.asSmithyModel()

        val validationResult = validateModel(EventStreamNormalizer.transform(model))



    @Test

    fun `it should detect when constraint traits in event streams are used`() {

        val validationResult = validateModel(EventStreamNormalizer.transform(constrainedShapesInEventStreamModel))



        validationResult.messages shouldHaveSize 2

        validationResult.messages.forOne {
@@ -156,17 +168,31 @@ internal class ValidateUnsupportedConstraintsAreNotUsedTest { 
                """

                The string shape `test#LengthString` has the constraint trait `smithy.api#length` attached.

                This shape is also part of an event stream; it is unclear what the semantics for constrained shapes in event streams are.

                Please remove the trait from the shape to synthesize your model.

                """.trimIndent().replace("\n", " ")

            it.message shouldNotContain "If you want to go ahead and generate the server SDK ignoring unsupported constraint traits"

        }

        validationResult.messages.forOne {

            it.message shouldContain

                """

                The member shape `test#Error${"$"}message` has the constraint trait `smithy.api#required` attached.

                This shape is also part of an event stream; it is unclear what the semantics for constrained shapes in event streams are.

                Please remove the trait from the shape to synthesize your model.

                """.trimIndent().replace("\n", " ")

            it.message shouldNotContain "If you want to go ahead and generate the server SDK ignoring unsupported constraint traits"

        }

    }



    @Test

    fun `it should abort when constraint traits in event streams are used, despite opting into ignoreUnsupportedConstraintTraits`() {

        val validationResult = validateModel(

            EventStreamNormalizer.transform(constrainedShapesInEventStreamModel),

            ServerCodegenConfig().copy(ignoreUnsupportedConstraints = true),

        )



        validationResult.shouldAbort shouldBe true

    }



    @Test

    fun `it should detect when the length trait on blob shapes is used`() {

        val model =
@@ -183,7 +209,9 @@ internal class ValidateUnsupportedConstraintsAreNotUsedTest { 
        val validationResult = validateModel(model)



        validationResult.messages shouldHaveSize 1

        validationResult.messages[0].message shouldContain "The blob shape `test#LengthBlob` has the constraint trait `smithy.api#length` attached"



        // This test additionally asserts the exact message, to ensure the formatting is appropriate.

        validationResult.messages[0].message shouldBe "The blob shape `test#LengthBlob` has the constraint trait `smithy.api#length` attached. This is not supported in the smithy-rs server SDK. It will be supported in the future. See the tracking issue (https://github.com/awslabs/smithy-rs/issues/1401). If you want to go ahead and generate the server SDK ignoring unsupported constraint traits, set the key `ignoreUnsupportedConstraints` inside the `runtimeConfig.codegenConfig` JSON object in your `smithy-build.json` to `true`."

    }



    @Test