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

Move converters from constraint violations into ValidationException to decorators #2302

Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import software.amazon.smithy.rust.codegen.client.testutil.testCodegenContext
import software.amazon.smithy.rust.codegen.core.smithy.CoreRustSettings
import software.amazon.smithy.rust.codegen.core.smithy.RuntimeCrateLocation
import software.amazon.smithy.rust.codegen.core.smithy.RustCrate
import software.amazon.smithy.rust.codegen.core.testutil.IntegrationTestParams
import software.amazon.smithy.rust.codegen.core.testutil.TestRuntimeConfig
import software.amazon.smithy.rust.codegen.core.testutil.asSmithyModel
import software.amazon.smithy.rust.codegen.core.testutil.testRustSettings
Expand All @@ -39,9 +40,10 @@ fun awsSdkIntegrationTest(
test: (ClientCodegenContext, RustCrate) -> Unit = { _, _ -> },
) =
clientIntegrationTest(
model, runtimeConfig = AwsTestRuntimeConfig,
additionalSettings = ObjectNode.builder()
.withMember(
model,
IntegrationTestParams(
runtimeConfig = AwsTestRuntimeConfig,
additionalSettings = ObjectNode.builder().withMember(
"customizationConfig",
ObjectNode.builder()
.withMember(
Expand All @@ -51,6 +53,7 @@ fun awsSdkIntegrationTest(
.build(),
).build(),
)
.withMember("codegen", ObjectNode.builder().withMember("includeFluentClient", false).build()).build(),
.withMember("codegen", ObjectNode.builder().withMember("includeFluentClient", false).build()).build(),
),
test = test,
)
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import software.amazon.smithy.rust.codegen.client.smithy.customize.NoOpEventStre
import software.amazon.smithy.rust.codegen.client.smithy.customize.RequiredCustomizations
import software.amazon.smithy.rust.codegen.client.smithy.endpoint.EndpointsDecorator
import software.amazon.smithy.rust.codegen.client.smithy.generators.client.FluentClientDecorator
import software.amazon.smithy.rust.codegen.client.testutil.DecoratableBuildPlugin
import software.amazon.smithy.rust.codegen.client.testutil.ClientDecoratableBuildPlugin
import software.amazon.smithy.rust.codegen.core.rustlang.Attribute.Companion.NonExhaustive
import software.amazon.smithy.rust.codegen.core.rustlang.RustReservedWordSymbolProvider
import software.amazon.smithy.rust.codegen.core.smithy.BaseSymbolMetadataProvider
Expand All @@ -36,7 +36,7 @@ import java.util.logging.Logger
* `resources/META-INF.services/software.amazon.smithy.build.SmithyBuildPlugin` refers to this class by name which
* enables the smithy-build plugin to invoke `execute` with all Smithy plugin context + models.
*/
class RustClientCodegenPlugin : DecoratableBuildPlugin() {
class RustClientCodegenPlugin : ClientDecoratableBuildPlugin() {
override fun getName(): String = "rust-client-codegen"

override fun executeWithDecorator(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0
*/

package software.amazon.smithy.rust.codegen.client.testutil

import software.amazon.smithy.build.PluginContext
import software.amazon.smithy.build.SmithyBuildPlugin
import software.amazon.smithy.model.Model
import software.amazon.smithy.rust.codegen.client.smithy.ClientCodegenContext
import software.amazon.smithy.rust.codegen.client.smithy.RustClientCodegenPlugin
import software.amazon.smithy.rust.codegen.client.smithy.customize.ClientCodegenDecorator
import software.amazon.smithy.rust.codegen.core.smithy.RustCrate
import software.amazon.smithy.rust.codegen.core.testutil.IntegrationTestParams
import software.amazon.smithy.rust.codegen.core.testutil.codegenIntegrationTest
import java.nio.file.Path

fun clientIntegrationTest(
model: Model,
params: IntegrationTestParams = IntegrationTestParams(),
additionalDecorators: List<ClientCodegenDecorator> = listOf(),
test: (ClientCodegenContext, RustCrate) -> Unit = { _, _ -> },
): Path {
fun invokeRustCodegenPlugin(ctx: PluginContext) {
val codegenDecorator = object : ClientCodegenDecorator {
override val name: String = "Add tests"
override val order: Byte = 0

override fun classpathDiscoverable(): Boolean = false

override fun extras(codegenContext: ClientCodegenContext, rustCrate: RustCrate) {
test(codegenContext, rustCrate)
}
}
RustClientCodegenPlugin().executeWithDecorator(ctx, codegenDecorator, *additionalDecorators.toTypedArray())
}
return codegenIntegrationTest(model, params, invokePlugin = ::invokeRustCodegenPlugin)
}

/**
* A `SmithyBuildPlugin` that accepts an additional decorator.
*
* This exists to allow tests to easily customize the _real_ build without needing to list out customizations
* or attempt to manually discover them from the path.
*/
abstract class ClientDecoratableBuildPlugin : SmithyBuildPlugin {
abstract fun executeWithDecorator(
context: PluginContext,
vararg decorator: ClientCodegenDecorator,
)

override fun execute(context: PluginContext) {
executeWithDecorator(context)
}
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate
import software.amazon.smithy.rust.codegen.core.rustlang.writable
import software.amazon.smithy.rust.codegen.core.smithy.RuntimeConfig
import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType
import software.amazon.smithy.rust.codegen.core.testutil.IntegrationTestParams
import software.amazon.smithy.rust.codegen.core.testutil.asSmithyModel
import software.amazon.smithy.rust.codegen.core.testutil.integrationTest

Expand Down Expand Up @@ -170,8 +171,8 @@ internal class HttpVersionListGeneratorTest {

clientIntegrationTest(
model,
listOf(FakeSigningDecorator()),
addModuleToEventStreamAllowList = true,
IntegrationTestParams(addModuleToEventStreamAllowList = true),
additionalDecorators = listOf(FakeSigningDecorator()),
) { clientCodegenContext, rustCrate ->
val moduleName = clientCodegenContext.moduleUseName()
rustCrate.integrationTest("validate_eventstream_http") {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import org.junit.jupiter.api.Test
import software.amazon.smithy.rust.codegen.client.testutil.clientIntegrationTest
import software.amazon.smithy.rust.codegen.core.rustlang.Attribute
import software.amazon.smithy.rust.codegen.core.rustlang.rust
import software.amazon.smithy.rust.codegen.core.testutil.IntegrationTestParams
import software.amazon.smithy.rust.codegen.core.testutil.asSmithyModel
import software.amazon.smithy.rust.codegen.core.testutil.integrationTest
import software.amazon.smithy.rust.codegen.core.testutil.runWithWarnings
Expand Down Expand Up @@ -123,8 +124,8 @@ class EndpointsDecoratorTest {
fun `set an endpoint in the property bag`() {
val testDir = clientIntegrationTest(
model,
// just run integration tests
command = { "cargo test --test *".runWithWarnings(it) },
// Just run integration tests.
IntegrationTestParams(command = { "cargo test --test *".runWithWarnings(it) }),
) { clientCodegenContext, rustCrate ->
rustCrate.integrationTest("endpoint_params_test") {
val moduleName = clientCodegenContext.moduleUseName()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0
*/

package software.amazon.smithy.rust.codegen.core.testutil

import software.amazon.smithy.build.PluginContext
import software.amazon.smithy.model.Model
import software.amazon.smithy.model.node.ObjectNode
import software.amazon.smithy.rust.codegen.core.smithy.RuntimeConfig
import software.amazon.smithy.rust.codegen.core.util.runCommand
import java.io.File
import java.nio.file.Path

/**
* A helper class holding common data with defaults that is threaded through several functions, to make their
* signatures shorter.
*/
data class IntegrationTestParams(
val addModuleToEventStreamAllowList: Boolean = false,
val service: String? = null,
val runtimeConfig: RuntimeConfig? = null,
val additionalSettings: ObjectNode = ObjectNode.builder().build(),
val overrideTestDir: File? = null,
val command: ((Path) -> Unit)? = null,
)

/**
* Run cargo test on a true, end-to-end, codegen product of a given model.
*/
fun codegenIntegrationTest(model: Model, params: IntegrationTestParams, invokePlugin: (PluginContext) -> Unit): Path {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

val (ctx, testDir) = generatePluginContext(
model,
params.additionalSettings,
params.addModuleToEventStreamAllowList,
params.service,
params.runtimeConfig,
params.overrideTestDir,
)
invokePlugin(ctx)
ctx.fileManifest.printGeneratedFiles()
params.command?.invoke(testDir) ?: "cargo test".runCommand(testDir, environment = mapOf("RUSTFLAGS" to "-D warnings"))
return testDir
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import software.amazon.smithy.rust.codegen.core.rustlang.RustModule
import software.amazon.smithy.rust.codegen.core.rustlang.RustWriter
import software.amazon.smithy.rust.codegen.core.smithy.CodegenContext
import software.amazon.smithy.rust.codegen.core.smithy.CodegenTarget
import software.amazon.smithy.rust.codegen.core.smithy.DirectedWalker
import software.amazon.smithy.rust.codegen.core.smithy.ErrorsModule
import software.amazon.smithy.rust.codegen.core.smithy.ModelsModule
import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType
Expand Down Expand Up @@ -118,11 +119,15 @@ object EventStreamTestTools {
val symbolProvider = codegenContext.symbolProvider
val operationShape = model.expectShape(ShapeId.from("test#TestStreamOp")) as OperationShape
val unionShape = model.expectShape(ShapeId.from("test#TestStream")) as UnionShape
val walker = DirectedWalker(model)

val project = TestWorkspace.testProject(symbolProvider)
val operationSymbol = symbolProvider.toSymbol(operationShape)
project.withModule(ErrorsModule) {
val errors = model.structureShapes.filter { shape -> shape.hasTrait<ErrorTrait>() }
val errors = model.serviceShapes
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jdisanti What do you think about replacing these tests:

ClientEventStreamUnmarshallerGeneratorTest.kt
ServerEventStreamUnmarshallerGeneratorTest.kt
ClientEventStreamMarshallerGeneratorTest.kt
ServerEventStreamMarshallerGeneratorTest.kt

by simple {client,server}IntegrationTests? (see #1956) They're notoriously hard to get right and a sizable amount of code we could delete.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be great!

.flatMap { walker.walkShapes(it) }
.filterIsInstance<StructureShape>()
.filter { shape -> shape.hasTrait<ErrorTrait>() }
requirements.renderOperationError(this, model, symbolProvider, operationSymbol, errors)
requirements.renderOperationError(this, model, symbolProvider, symbolProvider.toSymbol(unionShape), errors)
for (shape in errors) {
Expand Down
4 changes: 4 additions & 0 deletions codegen-server/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ dependencies {
implementation(project(":codegen-core"))
implementation("software.amazon.smithy:smithy-aws-traits:$smithyVersion")
implementation("software.amazon.smithy:smithy-protocol-test-traits:$smithyVersion")

// `smithy.framework#ValidationException` is defined here, which is used in `constraints.smithy`, which is used
// in `CustomValidationExceptionWithReasonDecoratorTest`.
testImplementation("software.amazon.smithy:smithy-validation-model:$smithyVersion")
}

tasks.compileKotlin { kotlinOptions.jvmTarget = "1.8" }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ class PythonServerCodegenVisitor(
*/
override fun stringShape(shape: StringShape) {
fun pythonServerEnumGeneratorFactory(codegenContext: ServerCodegenContext, writer: RustWriter, shape: StringShape) =
PythonServerEnumGenerator(codegenContext, writer, shape)
PythonServerEnumGenerator(codegenContext, writer, shape, validationExceptionConversionGenerator)
stringShape(shape, ::pythonServerEnumGeneratorFactory)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ import software.amazon.smithy.rust.codegen.server.python.smithy.customizations.D
import software.amazon.smithy.rust.codegen.server.smithy.ConstrainedShapeSymbolMetadataProvider
import software.amazon.smithy.rust.codegen.server.smithy.ConstrainedShapeSymbolProvider
import software.amazon.smithy.rust.codegen.server.smithy.DeriveEqAndHashSymbolMetadataProvider
import software.amazon.smithy.rust.codegen.server.smithy.customizations.CustomValidationExceptionWithReasonDecorator
import software.amazon.smithy.rust.codegen.server.smithy.customizations.ServerRequiredCustomizations
import software.amazon.smithy.rust.codegen.server.smithy.customizations.SmithyValidationExceptionDecorator
import software.amazon.smithy.rust.codegen.server.smithy.customize.CombinedServerCodegenDecorator
import java.util.logging.Level
import java.util.logging.Logger
Expand Down Expand Up @@ -50,7 +52,10 @@ class RustServerCodegenPythonPlugin : SmithyBuildPlugin {
val codegenDecorator: CombinedServerCodegenDecorator =
CombinedServerCodegenDecorator.fromClasspath(
context,
CombinedServerCodegenDecorator(DECORATORS + ServerRequiredCustomizations()),
ServerRequiredCustomizations(),
SmithyValidationExceptionDecorator(),
CustomValidationExceptionWithReasonDecorator(),
*DECORATORS,
)

// PythonServerCodegenVisitor is the main driver of code generation that traverses the model and generates code
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ class PyO3ExtensionModuleDecorator : ServerCodegenDecorator {
}
}

val DECORATORS = listOf(
val DECORATORS = arrayOf(
/**
* Add the [InternalServerError] error to all operations.
* This is done because the Python interpreter can raise exceptions during execution.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import software.amazon.smithy.rust.codegen.core.util.dq
import software.amazon.smithy.rust.codegen.server.python.smithy.PythonServerCargoDependency
import software.amazon.smithy.rust.codegen.server.smithy.ServerCodegenContext
import software.amazon.smithy.rust.codegen.server.smithy.generators.ServerEnumGenerator
import software.amazon.smithy.rust.codegen.server.smithy.generators.ValidationExceptionConversionGenerator

/**
* To share enums defined in Rust with Python, `pyo3` provides the `PyClass` trait.
Expand All @@ -27,7 +28,8 @@ class PythonServerEnumGenerator(
codegenContext: ServerCodegenContext,
private val writer: RustWriter,
shape: StringShape,
) : ServerEnumGenerator(codegenContext, writer, shape) {
validationExceptionConversionGenerator: ValidationExceptionConversionGenerator,
) : ServerEnumGenerator(codegenContext, writer, shape, validationExceptionConversionGenerator) {

private val pyO3 = PythonServerCargoDependency.PyO3.toType()

Expand Down
Loading