Skip to content

Commit a02bab9

Browse files
ysaito1001ysaito1001
authored
Automatically add smithy.framework#ValidationException to constrained operations (#4506)
2 parents 5497cde + 487dd1f commit a02bab9

File tree

10 files changed

+195
-72
lines changed

10 files changed

+195
-72
lines changed

‎.changelog/1769620994.md‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
applies_to:
3+
- server
4+
authors:
5+
- drganjoo
6+
references: [smithy-rs#4494]
7+
breaking: false
8+
new_feature: true
9+
bug_fix: false
10+
---
11+
Automatically add `smithy.framework#ValidationException` to operations with constrained inputs. Previously, users had to either set `addValidationExceptionToConstrainedOperations: true` in codegen settings or manually add `ValidationException` to each operation. Now this happens automatically unless a custom validation exception (a structure with the `@validationException` trait) is defined in the model. When using a custom validation exception, users must explicitly add it to each applicable operation. The `addValidationExceptionToConstrainedOperations` flag is deprecated.

‎AGENTS.md‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,9 @@ gh workflow run "Invoke Canary as Maintainer" --repo smithy-lang/smithy-rs \
215215
Client changes often show the pattern for server-side implementation
216216

217217
**Configuration Debugging:**
218-
- Server codegen settings go under `"codegen"` not `"codegenConfig"` in smithy-build.json
218+
- `codegen` settings are defined in `CoreCodegenConfig`, `ServerCodegenConfig`, or `ClientCodegenConfig` (in respective `*RustSettings.kt` files)
219+
- `customizationConfig` settings: search for `settings.customizationConfig` usages - classify as client/server based on whether a `ClientCodegenDecorator` or `ServerCodegenDecorator` uses it
220+
- In smithy-build[-template].json, use `"codegen"` not `"codegenConfig"` (the latter is only a Kotlin property name)
219221
- When settings aren't working, check the generated smithy-build.json structure first
220222
- Settings placement matters - wrong nesting means settings are ignored silently
221223
- Always verify actual generated configuration matches expectations

‎buildSrc/src/main/kotlin/CrateSet.kt‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,8 @@ object CrateSet {
8585
private val SERVER_SPECIFIC_SMITHY_RUNTIME =
8686
listOf(
8787
Crate("aws-smithy-http-server", UNSTABLE_VERSION_PROP_NAME),
88+
Crate("aws-smithy-http-server-metrics", UNSTABLE_VERSION_PROP_NAME),
89+
Crate("aws-smithy-http-server-metrics-macro", UNSTABLE_VERSION_PROP_NAME),
8890
Crate("aws-smithy-http-server-python", UNSTABLE_VERSION_PROP_NAME),
8991
Crate("aws-smithy-http-server-typescript", UNSTABLE_VERSION_PROP_NAME),
9092
Crate("aws-smithy-legacy-http-server", UNSTABLE_VERSION_PROP_NAME),

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

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,13 @@ data class ServerCodegenConfig(
108108
* able to define the converters in their Rust application code.
109109
*/
110110
val experimentalCustomValidationExceptionWithReasonPleaseDoNotUse: String? = defaultExperimentalCustomValidationExceptionWithReasonPleaseDoNotUse,
111-
val addValidationExceptionToConstrainedOperations: Boolean = DEFAULT_ADD_VALIDATION_EXCEPTION_TO_CONSTRAINED_OPERATIONS,
111+
/**
112+
* @deprecated This flag is deprecated. `smithy.framework#ValidationException` is now automatically added to operations
113+
* with constrained inputs unless a custom validation exception (a structure with the `@validationException`
114+
* trait) is defined in the model. Setting this to false will disable the automatic addition, but this
115+
* behavior is deprecated and may be removed in a future release.
116+
*/
117+
val addValidationExceptionToConstrainedOperations: Boolean? = null,
112118
val alwaysSendEventStreamInitialResponse: Boolean = DEFAULT_SEND_EVENT_STREAM_INITIAL_RESPONSE,
113119
val http1x: Boolean = DEFAULT_HTTP_1X,
114120
) : CoreCodegenConfig(
@@ -118,7 +124,6 @@ data class ServerCodegenConfig(
118124
private const val DEFAULT_PUBLIC_CONSTRAINED_TYPES = true
119125
private const val DEFAULT_IGNORE_UNSUPPORTED_CONSTRAINTS = false
120126
private val defaultExperimentalCustomValidationExceptionWithReasonPleaseDoNotUse = null
121-
private const val DEFAULT_ADD_VALIDATION_EXCEPTION_TO_CONSTRAINED_OPERATIONS = false
122127
private const val DEFAULT_SEND_EVENT_STREAM_INITIAL_RESPONSE = false
123128
const val DEFAULT_HTTP_1X = false
124129

@@ -183,10 +188,9 @@ data class ServerCodegenConfig(
183188
defaultExperimentalCustomValidationExceptionWithReasonPleaseDoNotUse,
184189
),
185190
addValidationExceptionToConstrainedOperations =
186-
node.get().getBooleanMemberOrDefault(
191+
node.get().getBooleanMember(
187192
"addValidationExceptionToConstrainedOperations",
188-
DEFAULT_ADD_VALIDATION_EXCEPTION_TO_CONSTRAINED_OPERATIONS,
189-
),
193+
).orElse(null)?.value,
190194
alwaysSendEventStreamInitialResponse =
191195
node.get().getBooleanMemberOrDefault(
192196
"alwaysSendEventStreamInitialResponse",

‎codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/transformers/AttachValidationExceptionToConstrainedOperationInputs.kt‎

Lines changed: 44 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,18 +5,31 @@
55

66
package software.amazon.smithy.rust.codegen.server.smithy.transformers
77

8+
import software.amazon.smithy.framework.rust.ValidationExceptionTrait
89
import software.amazon.smithy.model.Model
910
import software.amazon.smithy.model.shapes.EnumShape
1011
import software.amazon.smithy.model.shapes.OperationShape
1112
import software.amazon.smithy.model.shapes.ServiceShape
1213
import software.amazon.smithy.model.shapes.SetShape
1314
import software.amazon.smithy.model.shapes.ShapeId
15+
import software.amazon.smithy.model.shapes.StructureShape
1416
import software.amazon.smithy.model.transform.ModelTransformer
1517
import software.amazon.smithy.rust.codegen.core.smithy.DirectedWalker
18+
import software.amazon.smithy.rust.codegen.core.util.hasEventStreamMember
1619
import software.amazon.smithy.rust.codegen.core.util.inputShape
1720
import software.amazon.smithy.rust.codegen.server.smithy.ServerRustSettings
1821
import software.amazon.smithy.rust.codegen.server.smithy.customizations.SmithyValidationExceptionConversionGenerator
1922
import software.amazon.smithy.rust.codegen.server.smithy.hasConstraintTrait
23+
import java.util.logging.Logger
24+
25+
private val logger: Logger = Logger.getLogger("AttachValidationExceptionToConstrainedOperationInputs")
26+
27+
/**
28+
* Checks if the model has a custom validation exception defined.
29+
* A custom validation exception is a structure with the `@validationException` trait.
30+
*/
31+
private fun hasCustomValidationException(model: Model): Boolean =
32+
model.shapes(StructureShape::class.java).anyMatch { it.hasTrait(ValidationExceptionTrait.ID) }
2033

2134
private fun addValidationExceptionToMatchingServiceShapes(
2235
model: Model,
@@ -30,7 +43,7 @@ private fun addValidationExceptionToMatchingServiceShapes(
3043
.map { model.expectShape(it, OperationShape::class.java) }
3144
.filter { operationShape ->
3245
walker.walkShapes(operationShape.inputShape(model))
33-
.any { it is SetShape || it is EnumShape || it.hasConstraintTrait() }
46+
.any { it is SetShape || it is EnumShape || it.hasConstraintTrait() || it.hasEventStreamMember(model) }
3447
}
3548
.filter { !it.errors.contains(SmithyValidationExceptionConversionGenerator.SHAPE_ID) }
3649

@@ -84,15 +97,40 @@ object AttachValidationExceptionToConstrainedOperationInputsInAllowList {
8497
}
8598

8699
/**
87-
* Attach the `smithy.framework#ValidationException` error to operations with constrained inputs if the
88-
* codegen flag `addValidationExceptionToConstrainedOperations` has been set.
100+
* Attach the `smithy.framework#ValidationException` error to operations with constrained inputs.
101+
*
102+
* This transformer automatically adds the default ValidationException to operations that have
103+
* constrained inputs but don't have a validation exception attached. This behavior is skipped
104+
* if the model defines a custom validation exception (a structure with the @validationException trait),
105+
* in which case the user is expected to explicitly add their custom exception to operations.
106+
*
107+
* The `addValidationExceptionToConstrainedOperations` codegen flag is deprecated. The transformer
108+
* now automatically determines whether to add ValidationException based on whether a custom
109+
* validation exception exists in the model.
89110
*/
90-
object AttachValidationExceptionToConstrainedOperationInputsBasedOnCodegenFlag {
111+
object AttachValidationExceptionToConstrainedOperationInput {
91112
fun transform(
92113
model: Model,
93114
settings: ServerRustSettings,
94115
): Model {
95-
if (!settings.codegenConfig.addValidationExceptionToConstrainedOperations) {
116+
// Log deprecation warning if the flag is explicitly set
117+
val addExceptionNullableFlag = settings.codegenConfig.addValidationExceptionToConstrainedOperations
118+
if (addExceptionNullableFlag == true) {
119+
// For backward compatibility, if `addValidationExceptionToConstrainedOperations` is explicitly true,
120+
// add `ValidationException` regardless of whether a custom validation exception exists.
121+
logger.warning(
122+
"The 'addValidationExceptionToConstrainedOperations' codegen flag is deprecated. " +
123+
"`smithy.framework#ValidationException` is now automatically added to operations with constrained inputs " +
124+
"unless a custom validation exception is defined in the model.",
125+
)
126+
} else if (addExceptionNullableFlag == false ||
127+
hasCustomValidationException(model) ||
128+
settings.codegenConfig.experimentalCustomValidationExceptionWithReasonPleaseDoNotUse != null
129+
) {
130+
// Skip adding `ValidationException` when:
131+
// - `addValidationExceptionToConstrainedOperations` is explicitly false (backward compatibility), or
132+
// - A custom validation exception exists (users must explicitly add it to operations), or
133+
// - A custom validation exception is configured via the experimental codegen setting
96134
return model
97135
}
98136

@@ -118,6 +156,6 @@ object AttachValidationExceptionToConstrainedOperationInputs {
118156
settings: ServerRustSettings,
119157
): Model {
120158
val allowListTransformedModel = AttachValidationExceptionToConstrainedOperationInputsInAllowList.transform(model)
121-
return AttachValidationExceptionToConstrainedOperationInputsBasedOnCodegenFlag.transform(allowListTransformedModel, settings)
159+
return AttachValidationExceptionToConstrainedOperationInput.transform(allowListTransformedModel, settings)
122160
}
123161
}

‎codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/customizations/AddValidationExceptionToConstrainedOperationsTest.kt‎

Lines changed: 110 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,13 @@
55

66
package software.amazon.smithy.rust.codegen.server.smithy.customizations
77

8+
import io.kotest.matchers.booleans.shouldBeFalse
9+
import io.kotest.matchers.booleans.shouldBeTrue
810
import org.junit.jupiter.api.Test
9-
import org.junit.jupiter.api.assertThrows
10-
import software.amazon.smithy.codegen.core.CodegenException
11+
import software.amazon.smithy.model.shapes.OperationShape
12+
import software.amazon.smithy.model.shapes.ServiceShape
13+
import software.amazon.smithy.model.shapes.ShapeId
1114
import software.amazon.smithy.rust.codegen.core.testutil.IntegrationTestParams
12-
import software.amazon.smithy.rust.codegen.core.testutil.ServerAdditionalSettings
1315
import software.amazon.smithy.rust.codegen.core.testutil.asSmithyModel
1416
import software.amazon.smithy.rust.codegen.server.smithy.testutil.HttpTestType
1517
import software.amazon.smithy.rust.codegen.server.smithy.testutil.serverIntegrationTest
@@ -18,6 +20,8 @@ import software.amazon.smithy.rust.codegen.server.smithy.testutil.serverIntegrat
1820
* Tests whether the server `codegen` flag `addValidationExceptionToConstrainedOperations` works as expected.
1921
*/
2022
internal class AddValidationExceptionToConstrainedOperationsTest {
23+
private val validationExceptionShapeId = SmithyValidationExceptionConversionGenerator.SHAPE_ID
24+
2125
private val testModelWithValidationExceptionImported =
2226
"""
2327
namespace test
@@ -61,28 +65,116 @@ internal class AddValidationExceptionToConstrainedOperationsTest {
6165
}
6266
""".asSmithyModel(smithyVersion = "2")
6367

64-
@Test
65-
fun `without setting the codegen flag, the model should fail to compile`() {
66-
assertThrows<CodegenException> {
67-
serverIntegrationTest(
68-
testModelWithValidationExceptionImported,
69-
IntegrationTestParams(),
70-
testCoverage = HttpTestType.Default,
68+
/**
69+
* Verify the test model is set up correctly: `SampleOperationWithoutValidation` and
70+
* the service should not have `smithy.framework#ValidationException` in the errors.
71+
*/
72+
private fun verifyTestModelDoesNotHaveValidationException() {
73+
val operation =
74+
testModelWithValidationExceptionImported.expectShape(
75+
ShapeId.from("test#SampleOperationWithoutValidation"),
76+
OperationShape::class.java,
7177
)
72-
}
78+
operation.errors.contains(validationExceptionShapeId).shouldBeFalse()
79+
80+
val service =
81+
testModelWithValidationExceptionImported.expectShape(
82+
ShapeId.from("test#ConstrainedService"),
83+
ServiceShape::class.java,
84+
)
85+
service.errors.contains(validationExceptionShapeId).shouldBeFalse()
7386
}
7487

7588
@Test
7689
fun `operations that do not have ValidationException will automatically have one added to them`() {
90+
verifyTestModelDoesNotHaveValidationException()
91+
7792
serverIntegrationTest(
7893
testModelWithValidationExceptionImported,
79-
IntegrationTestParams(
80-
additionalSettings =
81-
ServerAdditionalSettings.builder()
82-
.addValidationExceptionToConstrainedOperations()
83-
.toObjectNode(),
84-
),
8594
testCoverage = HttpTestType.Default,
86-
)
95+
) { codegenContext, _ ->
96+
// Verify the transformed model now has `smithy.framework#ValidationException` on the operation.
97+
val transformedOperation =
98+
codegenContext.model.expectShape(
99+
ShapeId.from("test#SampleOperationWithoutValidation"),
100+
OperationShape::class.java,
101+
)
102+
transformedOperation.errors.contains(validationExceptionShapeId).shouldBeTrue()
103+
}
104+
}
105+
106+
private val testModelWithCustomValidationException =
107+
"""
108+
namespace test
109+
110+
use smithy.framework.rust#validationException
111+
use smithy.framework.rust#validationFieldList
112+
use aws.protocols#restJson1
113+
114+
@restJson1
115+
service ConstrainedService {
116+
operations: [SampleOperation]
117+
errors: [CustomValidationException]
118+
}
119+
120+
@http(uri: "/sample", method: "POST")
121+
operation SampleOperation {
122+
input: SampleInput
123+
output: SampleOutput
124+
}
125+
126+
structure SampleInput {
127+
@range(min: 0, max: 100)
128+
constrainedInteger: Integer
129+
}
130+
131+
structure SampleOutput {
132+
result: String
133+
}
134+
135+
@error("client")
136+
@validationException
137+
structure CustomValidationException {
138+
message: String
139+
140+
@validationFieldList
141+
fieldList: ValidationFieldList
142+
}
143+
144+
structure ValidationField {
145+
name: String
146+
}
147+
148+
list ValidationFieldList {
149+
member: ValidationField
150+
}
151+
""".asSmithyModel(smithyVersion = "2")
152+
153+
@Test
154+
fun `when custom validation exception exists, ValidationException is not automatically added`() {
155+
// Verify the operation doesn't have ValidationException in the original model
156+
val operation =
157+
testModelWithCustomValidationException.expectShape(
158+
ShapeId.from("test#SampleOperation"),
159+
OperationShape::class.java,
160+
)
161+
operation.errors.contains(validationExceptionShapeId).shouldBeFalse()
162+
163+
// The model has a custom validation exception, so the transformer should NOT add
164+
// `smithy.framework#ValidationException`.
165+
serverIntegrationTest(
166+
testModelWithCustomValidationException,
167+
IntegrationTestParams(),
168+
testCoverage = HttpTestType.Default,
169+
) { codegenContext, _ ->
170+
// Verify the transformed model still doesn't have `smithy.framework#ValidationException`
171+
// on the operation.
172+
val transformedOperation =
173+
codegenContext.model.expectShape(
174+
ShapeId.from("test#SampleOperation"),
175+
OperationShape::class.java,
176+
)
177+
transformedOperation.errors.contains(validationExceptionShapeId).shouldBeFalse()
178+
}
87179
}
88180
}

‎codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/customizations/PostprocessValidationExceptionNotAttachedErrorMessageDecoratorTest.kt‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ import io.kotest.matchers.string.shouldContain
1010
import org.junit.jupiter.api.Test
1111
import org.junit.jupiter.api.assertThrows
1212
import software.amazon.smithy.codegen.core.CodegenException
13+
import software.amazon.smithy.model.node.Node
14+
import software.amazon.smithy.rust.codegen.core.testutil.IntegrationTestParams
1315
import software.amazon.smithy.rust.codegen.core.testutil.asSmithyModel
1416
import software.amazon.smithy.rust.codegen.server.smithy.LogMessage
1517
import software.amazon.smithy.rust.codegen.server.smithy.ValidationResult
@@ -68,6 +70,16 @@ internal class PostprocessValidationExceptionNotAttachedErrorMessageDecoratorTes
6870
assertThrows<CodegenException> {
6971
serverIntegrationTest(
7072
model,
73+
params =
74+
IntegrationTestParams(
75+
additionalSettings =
76+
Node.objectNodeBuilder().withMember(
77+
"codegen",
78+
Node.objectNodeBuilder()
79+
.withMember("addValidationExceptionToConstrainedOperations", false)
80+
.build(),
81+
).build(),
82+
),
7183
additionalDecorators = listOf(validationExceptionNotAttachedErrorMessageDummyPostprocessorDecorator),
7284
testCoverage = HttpTestType.Default,
7385
)

‎gradle.properties‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,4 +17,4 @@ allowLocalDeps=false
1717
# Avoid registering dependencies/plugins/tasks that are only used for testing purposes
1818
isTestingEnabled=true
1919
# codegen publication version
20-
codegenVersion=0.1.11
20+
codegenVersion=0.1.12

0 commit comments

Comments
 (0)