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

Error out if `ignoreUnsupportedConstraintTraits` has no effect (#2539) 

Now that constraint traits are supported in server SDKs (with some
corner case caveats, see #1401), this flag will almost always be useless
for those early adopters of constraint traits. It is thus convenient to
inform the user that they should remove it.

See
https://github.com/awslabs/smithy-rs/pull/2516#issuecomment-1490393056.

## 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 79ead488

CHANGELOG.next.toml

+6 −0
Original line number Diff line number Diff line
@@ -68,3 +68,9 @@ message = "The `SigningInstructions` in the `aws-sigv4` module are now public. T 
references = ["smithy-rs#2730"]

author = "cholcombe973"

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



[[smithy-rs]]

message = "Code generation will abort if the `ignoreUnsupportedConstraints` codegen flag has no effect, that is, if all constraint traits used in your model are well-supported. Please remove the flag in such case."

references = ["smithy-rs#2539"]

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

author = "david-perez"

codegen-server-test/python/build.gradle.kts

+2 −5
Original line number Diff line number Diff line
@@ -47,21 +47,18 @@ val allCodegenTests = "../../codegen-core/common-test-models".let { commonModels 
            imports = listOf("$commonModels/pokemon.smithy", "$commonModels/pokemon-common.smithy"),

        ),

        CodegenTest(

            "com.amazonaws.ebs#Ebs", "ebs",

            "com.amazonaws.ebs#Ebs",

            "ebs",

            imports = listOf("$commonModels/ebs.json"),

            extraConfig = """, "codegen": { "ignoreUnsupportedConstraints": true } """,

        ),

        CodegenTest(

            "aws.protocoltests.misc#MiscService",

            "misc",

            imports = listOf("$commonModels/misc.smithy"),

            // TODO(https://github.com/awslabs/smithy-rs/issues/1401) `@uniqueItems` is used.

            extraConfig = """, "codegen": { "ignoreUnsupportedConstraints": true } """,

        ),

        CodegenTest(

            "aws.protocoltests.json#JsonProtocol",

            "json_rpc11",

            extraConfig = """, "codegen": { "ignoreUnsupportedConstraints": true } """,

        ),

        CodegenTest("aws.protocoltests.json10#JsonRpc10", "json_rpc10"),

        CodegenTest("aws.protocoltests.restjson#RestJson", "rest_json"),

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

+20 −11
Original line number Diff line number Diff line
@@ -22,7 +22,6 @@ import software.amazon.smithy.model.shapes.ShapeId 
import software.amazon.smithy.model.shapes.ShortShape

import software.amazon.smithy.model.traits.LengthTrait

import software.amazon.smithy.model.traits.RangeTrait

import software.amazon.smithy.model.traits.RequiredTrait

import software.amazon.smithy.model.traits.StreamingTrait

import software.amazon.smithy.model.traits.Trait

import software.amazon.smithy.model.traits.UniqueItemsTrait
@@ -158,8 +157,6 @@ data class LogMessage(val level: Level, val message: String) 
data class ValidationResult(val shouldAbort: Boolean, val messages: List<LogMessage>) :

    Throwable(message = messages.joinToString("\n") { it.message })



private val unsupportedConstraintsOnMemberShapes = allConstraintTraits - RequiredTrait::class.java



/**

 * Validate that all constrained operations have the shape [validationExceptionShapeId] shape attached to their errors.

 */
@@ -280,6 +277,7 @@ fun validateUnsupportedConstraints( 
        .toSet()



    val messages =

        (

            unsupportedLengthTraitOnStreamingBlobShapeSet.map {

                it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints)

            } +
@@ -290,6 +288,17 @@ fun validateUnsupportedConstraints( 
                mapShapeReachableFromUniqueItemsListShapeSet.map {

                    it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints)

                }

            ).toMutableList()



    if (messages.isEmpty() && codegenConfig.ignoreUnsupportedConstraints) {

        messages += LogMessage(

            Level.SEVERE,

            """

            The `ignoreUnsupportedConstraints` flag in the `codegen` configuration is set to `true`, but it has no 

            effect. All the constraint traits used in the model are well-supported, please remove this flag.

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

        )

    }



    return ValidationResult(shouldAbort = messages.any { it.level == Level.SEVERE }, messages)

}

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

+24 −2
Original line number Diff line number Diff line
@@ -19,6 +19,7 @@ import software.amazon.smithy.rust.codegen.core.smithy.transformers.EventStreamN 
import software.amazon.smithy.rust.codegen.core.testutil.asSmithyModel

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

import software.amazon.smithy.rust.codegen.server.smithy.customizations.SmithyValidationExceptionConversionGenerator

import java.io.File

import java.util.logging.Level



internal class ValidateUnsupportedConstraintsAreNotUsedTest {
@@ -37,7 +38,7 @@ internal class ValidateUnsupportedConstraintsAreNotUsedTest { 
        """



    private fun validateModel(model: Model, serverCodegenConfig: ServerCodegenConfig = ServerCodegenConfig()): ValidationResult {

        val service = model.lookup<ServiceShape>("test#TestService")

        val service = model.serviceShapes.first()

        return validateUnsupportedConstraints(model, service, serverCodegenConfig)

    }
@@ -100,7 +101,7 @@ internal class ValidateUnsupportedConstraintsAreNotUsedTest { 
        """.trimIndent().replace("\n", " ")

    }



    val constrainedShapesInEventStreamModel =

    private val constrainedShapesInEventStreamModel =

        """

        $baseModel
@@ -242,4 +243,25 @@ internal class ValidateUnsupportedConstraintsAreNotUsedTest { 
        validationResult.messages shouldHaveAtLeastSize 1

        validationResult.messages.shouldForAll { it.level shouldBe Level.WARNING }

    }



    @Test

    fun `it should abort when ignoreUnsupportedConstraints is true and all used constraints are supported`() {

        val allConstraintTraitsAreSupported = File("../codegen-core/common-test-models/constraints.smithy")

            .readText()

            .asSmithyModel()



        val validationResult = validateModel(

            allConstraintTraitsAreSupported,

            ServerCodegenConfig().copy(ignoreUnsupportedConstraints = true),

        )



        validationResult.messages shouldHaveSize 1

        validationResult.shouldAbort shouldBe true

        validationResult.messages[0].message shouldContain(

            """

            The `ignoreUnsupportedConstraints` flag in the `codegen` configuration is set to `true`, but it has no 

            effect. All the constraint traits used in the model are well-supported, please remove this flag.

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

            )

    }

}