Extract smithy.rust#serde trait to a new smithy-shapes Gradle subproject
#3897
Conversation
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
| @@ -72,6 +73,7 @@ class RustClientCodegenPlugin : ClientDecoratableBuildPlugin() { | |||
| IdempotencyTokenDecorator(), | |||
| StalledStreamProtectionDecorator(), | |||
| StaticSdkFeatureTrackerDecorator(), | |||
| ClientSerdeDecorator(), | |||
There was a problem hiding this comment.
so this turns it on by default for all clients? I'm a little worried this would accidentally start generating serializers for the Rust SDK if a team ever exported the serde trait in their model. I think we need to make this opt-in in some way, e.g. via a codegen setting if we are going to bundle it like this.
| /** | ||
| * The serde decorators for the client and the server are _identical_, but they live in separate Gradle projects. | ||
| * There's no point in testing everything twice, since the code is the same. Hence this file just contains a simple | ||
| * smoke test to ensure we don't disable the client decorator _somehow_; all tests testing the decorator's logic should | ||
| * live in `SerdeDecoratorTest` of the `codegen-server` project instead. | ||
| */ |
There was a problem hiding this comment.
this isn't really true. The code they generate is identical but client and server shapes are not the same—we really need to test against both.
| /** | ||
| * The entrypoint to both the client and server decorators. | ||
| */ | ||
| fun extrasCommon( |
There was a problem hiding this comment.
can we rename this something related to serde—just because it's being called from an external context.
serdeExtras maybe?
| class SerializeImplGenerator(private val codegenContext: CodegenContext) { | ||
| class SerializeImplGenerator( | ||
| private val codegenContext: CodegenContext, | ||
| private val unwrapConstraints: (Shape) -> Writable, |
There was a problem hiding this comment.
this this is now a public name, we should probably name it better. It's not actually "unwrapping" constraints in that it doesn't fail if the constraints are violated. Maybe constrainedShapeToInner or something?
| @@ -364,8 +379,8 @@ class SerdeDecoratorTest { | |||
| "e": "A", | |||
| "nested": { | |||
| "int": 5, | |||
| "float": "-Infinity", | |||
There was a problem hiding this comment.
are there any remaining tests of the stringified floats?
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
This creates a new
smithy-shapesGradle project, meant to host Smithy shapesvended by smithy-rs. The project must host only their definitions, not any
code-generating implementations. This way model authors can depend on these
shapes and consumers don't have to care about smithy-rs and its dependencies if
they're not generating Rust code with smithy-rs.
So far this module contains only the
smithy.rust#serdetrait definition. Itsimplementation now resides in decorators in
codegen-clientandcodegen-server, respectively, but since the actual logic is identical, theactual implementation is shared from
codegen-core.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.