From 1bf92e3d9c5e3e2fd18dd4cdc136cd90a30ed18c Mon Sep 17 00:00:00 2001 From: david-perez Date: Tue, 4 Apr 2023 14:50:06 +0200 Subject: [PATCH 1/3] Error out if `ignoreUnsupportedConstraintTraits` has no effect 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. --- CHANGELOG.next.toml | 6 +++++ .../smithy/ValidateUnsupportedConstraints.kt | 16 +++++++++--- ...ateUnsupportedConstraintsAreNotUsedTest.kt | 26 +++++++++++++++++-- 3 files changed, 42 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index 7770e39605..c4880876cb 100644 --- a/CHANGELOG.next.toml +++ b/CHANGELOG.next.toml @@ -80,3 +80,9 @@ message = "Add `into_segments` method to `AggregatedBytes`, for zero-copy conver references = ["smithy-rs#2525"] meta = { "breaking" = false, "tada" = false, "bug" = false } author = "parker-timmerman" + +[[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" diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraints.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraints.kt index 26f298c191..a1a4af6f2d 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraints.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraints.kt @@ -158,8 +158,6 @@ data class LogMessage(val level: Level, val message: String) data class ValidationResult(val shouldAbort: Boolean, val messages: List) : 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,7 +278,7 @@ fun validateUnsupportedConstraints( .toSet() val messages = - unsupportedLengthTraitOnStreamingBlobShapeSet.map { + (unsupportedLengthTraitOnStreamingBlobShapeSet.map { it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints) } + unsupportedConstraintShapeReachableViaAnEventStreamSet.map { @@ -289,7 +287,17 @@ fun validateUnsupportedConstraints( unsupportedRangeTraitOnShapeSet.map { it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints) } + 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) } diff --git a/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraintsAreNotUsedTest.kt b/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraintsAreNotUsedTest.kt index 68b88a7978..3c5e62e40f 100644 --- a/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraintsAreNotUsedTest.kt +++ b/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraintsAreNotUsedTest.kt @@ -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("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", " ") + ) + } } From 4565af7ade8560da3f23bfb9f77890dbad146e69 Mon Sep 17 00:00:00 2001 From: david-perez Date: Tue, 30 May 2023 11:59:46 +0200 Subject: [PATCH 2/3] Fix codegen-server-test:python --- codegen-server-test/python/build.gradle.kts | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/codegen-server-test/python/build.gradle.kts b/codegen-server-test/python/build.gradle.kts index 423d8a4619..469e0e6bd7 100644 --- a/codegen-server-test/python/build.gradle.kts +++ b/codegen-server-test/python/build.gradle.kts @@ -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"), From b48db94cdfe9ecab02a68f4aa4fe62ed7e32efb0 Mon Sep 17 00:00:00 2001 From: david-perez Date: Tue, 30 May 2023 12:00:42 +0200 Subject: [PATCH 3/3] ./gradlew ktlintFormat --- .../smithy/ValidateUnsupportedConstraints.kt | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraints.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraints.kt index a1a4af6f2d..f0f0ee8227 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraints.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraints.kt @@ -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 @@ -278,16 +277,18 @@ fun validateUnsupportedConstraints( .toSet() val messages = - (unsupportedLengthTraitOnStreamingBlobShapeSet.map { - it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints) - } + - unsupportedConstraintShapeReachableViaAnEventStreamSet.map { + ( + unsupportedLengthTraitOnStreamingBlobShapeSet.map { it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints) } + - unsupportedRangeTraitOnShapeSet.map { it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints) } + - mapShapeReachableFromUniqueItemsListShapeSet.map { - it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints) - }).toMutableList() + unsupportedConstraintShapeReachableViaAnEventStreamSet.map { + it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints) + } + + unsupportedRangeTraitOnShapeSet.map { it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints) } + + mapShapeReachableFromUniqueItemsListShapeSet.map { + it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints) + } + ).toMutableList() if (messages.isEmpty() && codegenConfig.ignoreUnsupportedConstraints) { messages += LogMessage( @@ -295,7 +296,7 @@ fun validateUnsupportedConstraints( """ 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", " ") + """.trimIndent().replace("\n", " "), ) }