Skip to content

Commit

Permalink
Error out if ignoreUnsupportedConstraintTraits has no effect
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
david-perez committed Apr 4, 2023
1 parent 421488e commit a1040c6
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 6 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.next.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,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,7 +278,7 @@ fun validateUnsupportedConstraints(
.toSet()

val messages =
unsupportedLengthTraitOnStreamingBlobShapeSet.map {
(unsupportedLengthTraitOnStreamingBlobShapeSet.map {
it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints)
} +
unsupportedConstraintShapeReachableViaAnEventStreamSet.map {
Expand All @@ -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)
}
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", " ")
)
}
}

0 comments on commit a1040c6

Please sign in to comment.