Skip to content

Commit ec874d5

Browse files
authored
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 #2516 (comment). ## 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._
1 parent 79ead48 commit ec874d5

File tree

4 files changed

+52
-18
lines changed

4 files changed

+52
-18
lines changed

CHANGELOG.next.toml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,3 +68,9 @@ message = "The `SigningInstructions` in the `aws-sigv4` module are now public. T
6868
references = ["smithy-rs#2730"]
6969
author = "cholcombe973"
7070
meta = { "breaking" = false, "tada" = false, "bug" = true }
71+
72+
[[smithy-rs]]
73+
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."
74+
references = ["smithy-rs#2539"]
75+
meta = { "breaking" = true, "tada" = false, "bug" = false, "target" = "server" }
76+
author = "david-perez"

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

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -47,21 +47,18 @@ val allCodegenTests = "../../codegen-core/common-test-models".let { commonModels
4747
imports = listOf("$commonModels/pokemon.smithy", "$commonModels/pokemon-common.smithy"),
4848
),
4949
CodegenTest(
50-
"com.amazonaws.ebs#Ebs", "ebs",
50+
"com.amazonaws.ebs#Ebs",
51+
"ebs",
5152
imports = listOf("$commonModels/ebs.json"),
52-
extraConfig = """, "codegen": { "ignoreUnsupportedConstraints": true } """,
5353
),
5454
CodegenTest(
5555
"aws.protocoltests.misc#MiscService",
5656
"misc",
5757
imports = listOf("$commonModels/misc.smithy"),
58-
// TODO(https://github.com/awslabs/smithy-rs/issues/1401) `@uniqueItems` is used.
59-
extraConfig = """, "codegen": { "ignoreUnsupportedConstraints": true } """,
6058
),
6159
CodegenTest(
6260
"aws.protocoltests.json#JsonProtocol",
6361
"json_rpc11",
64-
extraConfig = """, "codegen": { "ignoreUnsupportedConstraints": true } """,
6562
),
6663
CodegenTest("aws.protocoltests.json10#JsonRpc10", "json_rpc10"),
6764
CodegenTest("aws.protocoltests.restjson#RestJson", "rest_json"),

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

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ import software.amazon.smithy.model.shapes.ShapeId
2222
import software.amazon.smithy.model.shapes.ShortShape
2323
import software.amazon.smithy.model.traits.LengthTrait
2424
import software.amazon.smithy.model.traits.RangeTrait
25-
import software.amazon.smithy.model.traits.RequiredTrait
2625
import software.amazon.smithy.model.traits.StreamingTrait
2726
import software.amazon.smithy.model.traits.Trait
2827
import software.amazon.smithy.model.traits.UniqueItemsTrait
@@ -158,8 +157,6 @@ data class LogMessage(val level: Level, val message: String)
158157
data class ValidationResult(val shouldAbort: Boolean, val messages: List<LogMessage>) :
159158
Throwable(message = messages.joinToString("\n") { it.message })
160159

161-
private val unsupportedConstraintsOnMemberShapes = allConstraintTraits - RequiredTrait::class.java
162-
163160
/**
164161
* Validate that all constrained operations have the shape [validationExceptionShapeId] shape attached to their errors.
165162
*/
@@ -280,16 +277,28 @@ fun validateUnsupportedConstraints(
280277
.toSet()
281278

282279
val messages =
283-
unsupportedLengthTraitOnStreamingBlobShapeSet.map {
284-
it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints)
285-
} +
286-
unsupportedConstraintShapeReachableViaAnEventStreamSet.map {
280+
(
281+
unsupportedLengthTraitOnStreamingBlobShapeSet.map {
287282
it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints)
288283
} +
289-
unsupportedRangeTraitOnShapeSet.map { it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints) } +
290-
mapShapeReachableFromUniqueItemsListShapeSet.map {
291-
it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints)
292-
}
284+
unsupportedConstraintShapeReachableViaAnEventStreamSet.map {
285+
it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints)
286+
} +
287+
unsupportedRangeTraitOnShapeSet.map { it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints) } +
288+
mapShapeReachableFromUniqueItemsListShapeSet.map {
289+
it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints)
290+
}
291+
).toMutableList()
292+
293+
if (messages.isEmpty() && codegenConfig.ignoreUnsupportedConstraints) {
294+
messages += LogMessage(
295+
Level.SEVERE,
296+
"""
297+
The `ignoreUnsupportedConstraints` flag in the `codegen` configuration is set to `true`, but it has no
298+
effect. All the constraint traits used in the model are well-supported, please remove this flag.
299+
""".trimIndent().replace("\n", " "),
300+
)
301+
}
293302

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

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

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import software.amazon.smithy.rust.codegen.core.smithy.transformers.EventStreamN
1919
import software.amazon.smithy.rust.codegen.core.testutil.asSmithyModel
2020
import software.amazon.smithy.rust.codegen.core.util.lookup
2121
import software.amazon.smithy.rust.codegen.server.smithy.customizations.SmithyValidationExceptionConversionGenerator
22+
import java.io.File
2223
import java.util.logging.Level
2324

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

3940
private fun validateModel(model: Model, serverCodegenConfig: ServerCodegenConfig = ServerCodegenConfig()): ValidationResult {
40-
val service = model.lookup<ServiceShape>("test#TestService")
41+
val service = model.serviceShapes.first()
4142
return validateUnsupportedConstraints(model, service, serverCodegenConfig)
4243
}
4344

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

103-
val constrainedShapesInEventStreamModel =
104+
private val constrainedShapesInEventStreamModel =
104105
"""
105106
$baseModel
106107
@@ -242,4 +243,25 @@ internal class ValidateUnsupportedConstraintsAreNotUsedTest {
242243
validationResult.messages shouldHaveAtLeastSize 1
243244
validationResult.messages.shouldForAll { it.level shouldBe Level.WARNING }
244245
}
246+
247+
@Test
248+
fun `it should abort when ignoreUnsupportedConstraints is true and all used constraints are supported`() {
249+
val allConstraintTraitsAreSupported = File("../codegen-core/common-test-models/constraints.smithy")
250+
.readText()
251+
.asSmithyModel()
252+
253+
val validationResult = validateModel(
254+
allConstraintTraitsAreSupported,
255+
ServerCodegenConfig().copy(ignoreUnsupportedConstraints = true),
256+
)
257+
258+
validationResult.messages shouldHaveSize 1
259+
validationResult.shouldAbort shouldBe true
260+
validationResult.messages[0].message shouldContain(
261+
"""
262+
The `ignoreUnsupportedConstraints` flag in the `codegen` configuration is set to `true`, but it has no
263+
effect. All the constraint traits used in the model are well-supported, please remove this flag.
264+
""".trimIndent().replace("\n", " ")
265+
)
266+
}
245267
}

0 commit comments

Comments
 (0)