diff --git a/.changelog/1769620994.md b/.changelog/1769620994.md new file mode 100644 index 00000000000..951f977b4ed --- /dev/null +++ b/.changelog/1769620994.md @@ -0,0 +1,11 @@ +--- +applies_to: +- server +authors: +- drganjoo +references: [smithy-rs#4494] +breaking: false +new_feature: true +bug_fix: false +--- +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. diff --git a/AGENTS.md b/AGENTS.md index 8e287acaa85..dbec1a0bf82 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -215,7 +215,9 @@ gh workflow run "Invoke Canary as Maintainer" --repo smithy-lang/smithy-rs \ Client changes often show the pattern for server-side implementation **Configuration Debugging:** -- Server codegen settings go under `"codegen"` not `"codegenConfig"` in smithy-build.json +- `codegen` settings are defined in `CoreCodegenConfig`, `ServerCodegenConfig`, or `ClientCodegenConfig` (in respective `*RustSettings.kt` files) +- `customizationConfig` settings: search for `settings.customizationConfig` usages - classify as client/server based on whether a `ClientCodegenDecorator` or `ServerCodegenDecorator` uses it +- In smithy-build[-template].json, use `"codegen"` not `"codegenConfig"` (the latter is only a Kotlin property name) - When settings aren't working, check the generated smithy-build.json structure first - Settings placement matters - wrong nesting means settings are ignored silently - Always verify actual generated configuration matches expectations diff --git a/buildSrc/src/main/kotlin/CrateSet.kt b/buildSrc/src/main/kotlin/CrateSet.kt index a5ba336067e..a661cdcbb39 100644 --- a/buildSrc/src/main/kotlin/CrateSet.kt +++ b/buildSrc/src/main/kotlin/CrateSet.kt @@ -85,6 +85,8 @@ object CrateSet { private val SERVER_SPECIFIC_SMITHY_RUNTIME = listOf( Crate("aws-smithy-http-server", UNSTABLE_VERSION_PROP_NAME), + Crate("aws-smithy-http-server-metrics", UNSTABLE_VERSION_PROP_NAME), + Crate("aws-smithy-http-server-metrics-macro", UNSTABLE_VERSION_PROP_NAME), Crate("aws-smithy-http-server-python", UNSTABLE_VERSION_PROP_NAME), Crate("aws-smithy-http-server-typescript", UNSTABLE_VERSION_PROP_NAME), Crate("aws-smithy-legacy-http-server", UNSTABLE_VERSION_PROP_NAME), diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ServerRustSettings.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ServerRustSettings.kt index 1d646d91ad7..a5c312467b9 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ServerRustSettings.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ServerRustSettings.kt @@ -108,7 +108,13 @@ data class ServerCodegenConfig( * able to define the converters in their Rust application code. */ val experimentalCustomValidationExceptionWithReasonPleaseDoNotUse: String? = defaultExperimentalCustomValidationExceptionWithReasonPleaseDoNotUse, - val addValidationExceptionToConstrainedOperations: Boolean = DEFAULT_ADD_VALIDATION_EXCEPTION_TO_CONSTRAINED_OPERATIONS, + /** + * @deprecated This flag is deprecated. `smithy.framework#ValidationException` is now automatically added to operations + * with constrained inputs unless a custom validation exception (a structure with the `@validationException` + * trait) is defined in the model. Setting this to false will disable the automatic addition, but this + * behavior is deprecated and may be removed in a future release. + */ + val addValidationExceptionToConstrainedOperations: Boolean? = null, val alwaysSendEventStreamInitialResponse: Boolean = DEFAULT_SEND_EVENT_STREAM_INITIAL_RESPONSE, val http1x: Boolean = DEFAULT_HTTP_1X, ) : CoreCodegenConfig( @@ -118,7 +124,6 @@ data class ServerCodegenConfig( private const val DEFAULT_PUBLIC_CONSTRAINED_TYPES = true private const val DEFAULT_IGNORE_UNSUPPORTED_CONSTRAINTS = false private val defaultExperimentalCustomValidationExceptionWithReasonPleaseDoNotUse = null - private const val DEFAULT_ADD_VALIDATION_EXCEPTION_TO_CONSTRAINED_OPERATIONS = false private const val DEFAULT_SEND_EVENT_STREAM_INITIAL_RESPONSE = false const val DEFAULT_HTTP_1X = false @@ -183,10 +188,9 @@ data class ServerCodegenConfig( defaultExperimentalCustomValidationExceptionWithReasonPleaseDoNotUse, ), addValidationExceptionToConstrainedOperations = - node.get().getBooleanMemberOrDefault( + node.get().getBooleanMember( "addValidationExceptionToConstrainedOperations", - DEFAULT_ADD_VALIDATION_EXCEPTION_TO_CONSTRAINED_OPERATIONS, - ), + ).orElse(null)?.value, alwaysSendEventStreamInitialResponse = node.get().getBooleanMemberOrDefault( "alwaysSendEventStreamInitialResponse", diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/transformers/AttachValidationExceptionToConstrainedOperationInputs.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/transformers/AttachValidationExceptionToConstrainedOperationInputs.kt index 645f9864f4c..2d2f596d952 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/transformers/AttachValidationExceptionToConstrainedOperationInputs.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/transformers/AttachValidationExceptionToConstrainedOperationInputs.kt @@ -5,18 +5,31 @@ package software.amazon.smithy.rust.codegen.server.smithy.transformers +import software.amazon.smithy.framework.rust.ValidationExceptionTrait import software.amazon.smithy.model.Model import software.amazon.smithy.model.shapes.EnumShape import software.amazon.smithy.model.shapes.OperationShape import software.amazon.smithy.model.shapes.ServiceShape import software.amazon.smithy.model.shapes.SetShape import software.amazon.smithy.model.shapes.ShapeId +import software.amazon.smithy.model.shapes.StructureShape import software.amazon.smithy.model.transform.ModelTransformer import software.amazon.smithy.rust.codegen.core.smithy.DirectedWalker +import software.amazon.smithy.rust.codegen.core.util.hasEventStreamMember import software.amazon.smithy.rust.codegen.core.util.inputShape import software.amazon.smithy.rust.codegen.server.smithy.ServerRustSettings import software.amazon.smithy.rust.codegen.server.smithy.customizations.SmithyValidationExceptionConversionGenerator import software.amazon.smithy.rust.codegen.server.smithy.hasConstraintTrait +import java.util.logging.Logger + +private val logger: Logger = Logger.getLogger("AttachValidationExceptionToConstrainedOperationInputs") + +/** + * Checks if the model has a custom validation exception defined. + * A custom validation exception is a structure with the `@validationException` trait. + */ +private fun hasCustomValidationException(model: Model): Boolean = + model.shapes(StructureShape::class.java).anyMatch { it.hasTrait(ValidationExceptionTrait.ID) } private fun addValidationExceptionToMatchingServiceShapes( model: Model, @@ -30,7 +43,7 @@ private fun addValidationExceptionToMatchingServiceShapes( .map { model.expectShape(it, OperationShape::class.java) } .filter { operationShape -> walker.walkShapes(operationShape.inputShape(model)) - .any { it is SetShape || it is EnumShape || it.hasConstraintTrait() } + .any { it is SetShape || it is EnumShape || it.hasConstraintTrait() || it.hasEventStreamMember(model) } } .filter { !it.errors.contains(SmithyValidationExceptionConversionGenerator.SHAPE_ID) } @@ -84,15 +97,40 @@ object AttachValidationExceptionToConstrainedOperationInputsInAllowList { } /** - * Attach the `smithy.framework#ValidationException` error to operations with constrained inputs if the - * codegen flag `addValidationExceptionToConstrainedOperations` has been set. + * Attach the `smithy.framework#ValidationException` error to operations with constrained inputs. + * + * This transformer automatically adds the default ValidationException to operations that have + * constrained inputs but don't have a validation exception attached. This behavior is skipped + * if the model defines a custom validation exception (a structure with the @validationException trait), + * in which case the user is expected to explicitly add their custom exception to operations. + * + * The `addValidationExceptionToConstrainedOperations` codegen flag is deprecated. The transformer + * now automatically determines whether to add ValidationException based on whether a custom + * validation exception exists in the model. */ -object AttachValidationExceptionToConstrainedOperationInputsBasedOnCodegenFlag { +object AttachValidationExceptionToConstrainedOperationInput { fun transform( model: Model, settings: ServerRustSettings, ): Model { - if (!settings.codegenConfig.addValidationExceptionToConstrainedOperations) { + // Log deprecation warning if the flag is explicitly set + val addExceptionNullableFlag = settings.codegenConfig.addValidationExceptionToConstrainedOperations + if (addExceptionNullableFlag == true) { + // For backward compatibility, if `addValidationExceptionToConstrainedOperations` is explicitly true, + // add `ValidationException` regardless of whether a custom validation exception exists. + logger.warning( + "The 'addValidationExceptionToConstrainedOperations' codegen flag is deprecated. " + + "`smithy.framework#ValidationException` is now automatically added to operations with constrained inputs " + + "unless a custom validation exception is defined in the model.", + ) + } else if (addExceptionNullableFlag == false || + hasCustomValidationException(model) || + settings.codegenConfig.experimentalCustomValidationExceptionWithReasonPleaseDoNotUse != null + ) { + // Skip adding `ValidationException` when: + // - `addValidationExceptionToConstrainedOperations` is explicitly false (backward compatibility), or + // - A custom validation exception exists (users must explicitly add it to operations), or + // - A custom validation exception is configured via the experimental codegen setting return model } @@ -118,6 +156,6 @@ object AttachValidationExceptionToConstrainedOperationInputs { settings: ServerRustSettings, ): Model { val allowListTransformedModel = AttachValidationExceptionToConstrainedOperationInputsInAllowList.transform(model) - return AttachValidationExceptionToConstrainedOperationInputsBasedOnCodegenFlag.transform(allowListTransformedModel, settings) + return AttachValidationExceptionToConstrainedOperationInput.transform(allowListTransformedModel, settings) } } diff --git a/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/customizations/AddValidationExceptionToConstrainedOperationsTest.kt b/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/customizations/AddValidationExceptionToConstrainedOperationsTest.kt index 11da8ff70a3..3cbaa6746bc 100644 --- a/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/customizations/AddValidationExceptionToConstrainedOperationsTest.kt +++ b/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/customizations/AddValidationExceptionToConstrainedOperationsTest.kt @@ -5,11 +5,13 @@ package software.amazon.smithy.rust.codegen.server.smithy.customizations +import io.kotest.matchers.booleans.shouldBeFalse +import io.kotest.matchers.booleans.shouldBeTrue import org.junit.jupiter.api.Test -import org.junit.jupiter.api.assertThrows -import software.amazon.smithy.codegen.core.CodegenException +import software.amazon.smithy.model.shapes.OperationShape +import software.amazon.smithy.model.shapes.ServiceShape +import software.amazon.smithy.model.shapes.ShapeId import software.amazon.smithy.rust.codegen.core.testutil.IntegrationTestParams -import software.amazon.smithy.rust.codegen.core.testutil.ServerAdditionalSettings import software.amazon.smithy.rust.codegen.core.testutil.asSmithyModel import software.amazon.smithy.rust.codegen.server.smithy.testutil.HttpTestType import software.amazon.smithy.rust.codegen.server.smithy.testutil.serverIntegrationTest @@ -18,6 +20,8 @@ import software.amazon.smithy.rust.codegen.server.smithy.testutil.serverIntegrat * Tests whether the server `codegen` flag `addValidationExceptionToConstrainedOperations` works as expected. */ internal class AddValidationExceptionToConstrainedOperationsTest { + private val validationExceptionShapeId = SmithyValidationExceptionConversionGenerator.SHAPE_ID + private val testModelWithValidationExceptionImported = """ namespace test @@ -61,28 +65,116 @@ internal class AddValidationExceptionToConstrainedOperationsTest { } """.asSmithyModel(smithyVersion = "2") - @Test - fun `without setting the codegen flag, the model should fail to compile`() { - assertThrows { - serverIntegrationTest( - testModelWithValidationExceptionImported, - IntegrationTestParams(), - testCoverage = HttpTestType.Default, + /** + * Verify the test model is set up correctly: `SampleOperationWithoutValidation` and + * the service should not have `smithy.framework#ValidationException` in the errors. + */ + private fun verifyTestModelDoesNotHaveValidationException() { + val operation = + testModelWithValidationExceptionImported.expectShape( + ShapeId.from("test#SampleOperationWithoutValidation"), + OperationShape::class.java, ) - } + operation.errors.contains(validationExceptionShapeId).shouldBeFalse() + + val service = + testModelWithValidationExceptionImported.expectShape( + ShapeId.from("test#ConstrainedService"), + ServiceShape::class.java, + ) + service.errors.contains(validationExceptionShapeId).shouldBeFalse() } @Test fun `operations that do not have ValidationException will automatically have one added to them`() { + verifyTestModelDoesNotHaveValidationException() + serverIntegrationTest( testModelWithValidationExceptionImported, - IntegrationTestParams( - additionalSettings = - ServerAdditionalSettings.builder() - .addValidationExceptionToConstrainedOperations() - .toObjectNode(), - ), testCoverage = HttpTestType.Default, - ) + ) { codegenContext, _ -> + // Verify the transformed model now has `smithy.framework#ValidationException` on the operation. + val transformedOperation = + codegenContext.model.expectShape( + ShapeId.from("test#SampleOperationWithoutValidation"), + OperationShape::class.java, + ) + transformedOperation.errors.contains(validationExceptionShapeId).shouldBeTrue() + } + } + + private val testModelWithCustomValidationException = + """ + namespace test + + use smithy.framework.rust#validationException + use smithy.framework.rust#validationFieldList + use aws.protocols#restJson1 + + @restJson1 + service ConstrainedService { + operations: [SampleOperation] + errors: [CustomValidationException] + } + + @http(uri: "/sample", method: "POST") + operation SampleOperation { + input: SampleInput + output: SampleOutput + } + + structure SampleInput { + @range(min: 0, max: 100) + constrainedInteger: Integer + } + + structure SampleOutput { + result: String + } + + @error("client") + @validationException + structure CustomValidationException { + message: String + + @validationFieldList + fieldList: ValidationFieldList + } + + structure ValidationField { + name: String + } + + list ValidationFieldList { + member: ValidationField + } + """.asSmithyModel(smithyVersion = "2") + + @Test + fun `when custom validation exception exists, ValidationException is not automatically added`() { + // Verify the operation doesn't have ValidationException in the original model + val operation = + testModelWithCustomValidationException.expectShape( + ShapeId.from("test#SampleOperation"), + OperationShape::class.java, + ) + operation.errors.contains(validationExceptionShapeId).shouldBeFalse() + + // The model has a custom validation exception, so the transformer should NOT add + // `smithy.framework#ValidationException`. + serverIntegrationTest( + testModelWithCustomValidationException, + IntegrationTestParams(), + testCoverage = HttpTestType.Default, + ) { codegenContext, _ -> + // Verify the transformed model still doesn't have `smithy.framework#ValidationException` + // on the operation. + val transformedOperation = + codegenContext.model.expectShape( + ShapeId.from("test#SampleOperation"), + OperationShape::class.java, + ) + transformedOperation.errors.contains(validationExceptionShapeId).shouldBeFalse() + } } } diff --git a/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/customizations/PostprocessValidationExceptionNotAttachedErrorMessageDecoratorTest.kt b/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/customizations/PostprocessValidationExceptionNotAttachedErrorMessageDecoratorTest.kt index 6dc4bcf4017..a3af80efd1e 100644 --- a/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/customizations/PostprocessValidationExceptionNotAttachedErrorMessageDecoratorTest.kt +++ b/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/customizations/PostprocessValidationExceptionNotAttachedErrorMessageDecoratorTest.kt @@ -10,6 +10,8 @@ import io.kotest.matchers.string.shouldContain import org.junit.jupiter.api.Test import org.junit.jupiter.api.assertThrows import software.amazon.smithy.codegen.core.CodegenException +import software.amazon.smithy.model.node.Node +import software.amazon.smithy.rust.codegen.core.testutil.IntegrationTestParams import software.amazon.smithy.rust.codegen.core.testutil.asSmithyModel import software.amazon.smithy.rust.codegen.server.smithy.LogMessage import software.amazon.smithy.rust.codegen.server.smithy.ValidationResult @@ -68,6 +70,16 @@ internal class PostprocessValidationExceptionNotAttachedErrorMessageDecoratorTes assertThrows { serverIntegrationTest( model, + params = + IntegrationTestParams( + additionalSettings = + Node.objectNodeBuilder().withMember( + "codegen", + Node.objectNodeBuilder() + .withMember("addValidationExceptionToConstrainedOperations", false) + .build(), + ).build(), + ), additionalDecorators = listOf(validationExceptionNotAttachedErrorMessageDummyPostprocessorDecorator), testCoverage = HttpTestType.Default, ) diff --git a/gradle.properties b/gradle.properties index 82a1578d48f..172906a6fbe 100644 --- a/gradle.properties +++ b/gradle.properties @@ -17,4 +17,4 @@ allowLocalDeps=false # Avoid registering dependencies/plugins/tasks that are only used for testing purposes isTestingEnabled=true # codegen publication version -codegenVersion=0.1.11 +codegenVersion=0.1.12 diff --git a/tools/ci-build/sdk-lockfiles/false-positives.txt b/tools/ci-build/sdk-lockfiles/false-positives.txt index 3803e7e19a0..41eee1db3cc 100644 --- a/tools/ci-build/sdk-lockfiles/false-positives.txt +++ b/tools/ci-build/sdk-lockfiles/false-positives.txt @@ -1,7 +1,6 @@ aws-smithy-experimental -> pin-project aws-smithy-experimental -> pin-project-internal aws-runtime -> wit-bindgen-rt -aws-smithy-legacy-http-server -> sync_wrapper aws-smithy-http-client -> sync_wrapper aws-smithy-mocks -> sync_wrapper aws-smithy-runtime -> sync_wrapper @@ -12,42 +11,3 @@ aws-smithy-http-server -> metrique-writer aws-smithy-http-server -> metrique-writer-core aws-smithy-http-server -> metrique-writer-format-emf aws-smithy-http-server -> metrique-writer-macro -aws-smithy-http-server-metrics -> ahash -aws-smithy-http-server-metrics -> aws-smithy-http-server -aws-smithy-http-server-metrics -> aws-smithy-http-server-metrics-macro -aws-smithy-http-server-metrics -> crossbeam-queue -aws-smithy-http-server-metrics -> darling -aws-smithy-http-server-metrics -> darling_core -aws-smithy-http-server-metrics -> darling_macro -aws-smithy-http-server-metrics -> derive-where -aws-smithy-http-server-metrics -> dtoa -aws-smithy-http-server-metrics -> encoding_rs -aws-smithy-http-server-metrics -> endian-type -aws-smithy-http-server-metrics -> ident_case -aws-smithy-http-server-metrics -> Inflector -aws-smithy-http-server-metrics -> metrics -aws-smithy-http-server-metrics -> metrics-util -aws-smithy-http-server-metrics -> metrique -aws-smithy-http-server-metrics -> metrique-core -aws-smithy-http-server-metrics -> metrique-macro -aws-smithy-http-server-metrics -> metrique-service-metrics -aws-smithy-http-server-metrics -> metrique-timesource -aws-smithy-http-server-metrics -> metrique-writer -aws-smithy-http-server-metrics -> metrique-writer-core -aws-smithy-http-server-metrics -> metrique-writer-format-emf -aws-smithy-http-server-metrics -> metrique-writer-macro -aws-smithy-http-server-metrics -> mime -aws-smithy-http-server-metrics -> nibble_vec -aws-smithy-http-server-metrics -> ordered-float -aws-smithy-http-server-metrics -> pin-project -aws-smithy-http-server-metrics -> pin-project-internal -aws-smithy-http-server-metrics -> quanta -aws-smithy-http-server-metrics -> query_map -aws-smithy-http-server-metrics -> radix_trie -aws-smithy-http-server-metrics -> rand_xoshiro -aws-smithy-http-server-metrics -> raw-cpuid -aws-smithy-http-server-metrics -> serde_path_to_error -aws-smithy-http-server-metrics -> serde_urlencoded -aws-smithy-http-server-metrics -> sketches-ddsketch -aws-smithy-http-server-metrics -> str_inflector -aws-smithy-http-server-metrics -> sync_wrapper diff --git a/tools/ci-build/sdk-lockfiles/src/audit.rs b/tools/ci-build/sdk-lockfiles/src/audit.rs index e9b88bfc9ff..1ac664b9bb1 100644 --- a/tools/ci-build/sdk-lockfiles/src/audit.rs +++ b/tools/ci-build/sdk-lockfiles/src/audit.rs @@ -122,8 +122,10 @@ const AWS_SDK_RUNTIMES: &[&str] = &[ // https://github.com/smithy-lang/smithy-rs/blob/main/buildSrc/src/main/kotlin/CrateSet.kt#L42 const SERVER_SPECIFIC_RUNTIMES: &[&str] = &[ "aws-smithy-http-server", + "aws-smithy-http-server-metrics", + "aws-smithy-http-server-metrics-macro", "aws-smithy-http-server-python", - "aws-smithy-http-typescript", + "aws-smithy-http-server-typescript", "aws-smithy-legacy-http-server", ];