Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error out if ignoreUnsupportedConstraintTraits has no effect #2539

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.next.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
7 changes: 2 additions & 5 deletions codegen-server-test/python/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
*/
Expand Down Expand Up @@ -280,16 +277,28 @@ 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)
}
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(
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)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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)
}

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

val constrainedShapesInEventStreamModel =
private val constrainedShapesInEventStreamModel =
"""
$baseModel

Expand Down Expand Up @@ -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", " ")
)
}
}