-
Notifications
You must be signed in to change notification settings - Fork 201
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
Extract smithy.rust#serde
trait to a new smithy-shapes
Gradle subproject
#3897
base: main
Are you sure you want to change the base?
Changes from all commits
989d78d
b7ef8b5
f059e6d
e895280
76cfed3
d4c8968
fef399b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
/* | ||
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
package software.amazon.smithy.rust.codegen.client.smithy.customizations | ||
|
||
import software.amazon.smithy.rust.codegen.client.smithy.ClientCodegenContext | ||
import software.amazon.smithy.rust.codegen.client.smithy.customize.ClientCodegenDecorator | ||
import software.amazon.smithy.rust.codegen.core.rustlang.writable | ||
import software.amazon.smithy.rust.codegen.core.smithy.RustCrate | ||
import software.amazon.smithy.rust.codegen.core.smithy.customizations.serde.extrasCommon | ||
|
||
class ClientSerdeDecorator : ClientCodegenDecorator { | ||
override val name: String = "ClientSerdeDecorator" | ||
override val order: Byte = 0 | ||
|
||
override fun extras( | ||
codegenContext: ClientCodegenContext, | ||
rustCrate: RustCrate, | ||
) = extrasCommon( | ||
codegenContext, | ||
rustCrate, | ||
// Constraints don't affect client codegen. | ||
unwrapConstraints = { writable { } }, | ||
) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,77 @@ | ||
/* | ||
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
package software.amazon.smithy.rust.codegen.client.smithy.customizations.serde | ||
|
||
import org.junit.jupiter.api.Test | ||
import software.amazon.smithy.rust.codegen.client.testutil.clientIntegrationTest | ||
import software.amazon.smithy.rust.codegen.core.rustlang.CargoDependency | ||
import software.amazon.smithy.rust.codegen.core.rustlang.CratesIo | ||
import software.amazon.smithy.rust.codegen.core.rustlang.RustType | ||
import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate | ||
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.unitTest | ||
|
||
/** | ||
* 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. | ||
*/ | ||
Comment on lines
+19
to
+24
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
class SerdeDecoratorTest { | ||
@Test | ||
fun `smoke test`() { | ||
val model = | ||
""" | ||
namespace com.example | ||
use smithy.rust#serde | ||
use aws.protocols#awsJson1_0 | ||
|
||
@awsJson1_0 | ||
@serde | ||
service MyResourceService { | ||
resources: [MyResource] | ||
} | ||
|
||
resource MyResource { | ||
read: ReadMyResource | ||
} | ||
|
||
@readonly | ||
operation ReadMyResource { | ||
input := { } | ||
} | ||
""".asSmithyModel(smithyVersion = "2") | ||
|
||
val params = | ||
IntegrationTestParams(cargoCommand = "cargo test --all-features", service = "com.example#MyResourceService") | ||
clientIntegrationTest(model, params = params) { ctx, crate -> | ||
val codegenScope = | ||
arrayOf( | ||
"crate" to RustType.Opaque(ctx.moduleUseName()), | ||
"serde_json" to CargoDependency("serde_json", CratesIo("1")).toDevDependency().toType(), | ||
// we need the derive feature | ||
"serde" to CargoDependency.Serde.toDevDependency().toType(), | ||
) | ||
|
||
crate.integrationTest("test_serde") { | ||
unitTest("input_serialized") { | ||
rustTemplate( | ||
""" | ||
use #{crate}::operation::read_my_resource::ReadMyResourceInput; | ||
use #{crate}::serde::*; | ||
let input = ReadMyResourceInput::builder().build().unwrap(); | ||
let settings = SerializationSettings::default(); | ||
let _serialized = #{serde_json}::to_string(&input.serialize_ref(&settings)).expect("failed to serialize"); | ||
""", | ||
*codegenScope, | ||
) | ||
} | ||
} | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,22 +1,15 @@ | ||
/* | ||
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
package software.amazon.smithy.rust.codegen.serde | ||
package software.amazon.smithy.rust.codegen.core.smithy.customizations.serde | ||
|
||
import software.amazon.smithy.model.neighbor.Walker | ||
import software.amazon.smithy.model.shapes.Shape | ||
import software.amazon.smithy.rust.codegen.client.smithy.ClientCodegenContext | ||
import software.amazon.smithy.rust.codegen.client.smithy.customize.ClientCodegenDecorator | ||
import software.amazon.smithy.rust.codegen.core.rustlang.Attribute | ||
import software.amazon.smithy.rust.codegen.core.rustlang.Feature | ||
import software.amazon.smithy.rust.codegen.core.rustlang.RustModule | ||
import software.amazon.smithy.rust.codegen.core.rustlang.Writable | ||
import software.amazon.smithy.rust.codegen.core.smithy.CodegenContext | ||
import software.amazon.smithy.rust.codegen.core.smithy.DirectedWalker | ||
import software.amazon.smithy.rust.codegen.core.smithy.RustCrate | ||
import software.amazon.smithy.rust.codegen.core.util.hasTrait | ||
import software.amazon.smithy.rust.codegen.server.smithy.ServerCodegenContext | ||
import software.amazon.smithy.rust.codegen.server.smithy.customize.ServerCodegenDecorator | ||
import software.amazon.smithy.rust.smithy.shapes.SerdeTrait | ||
|
||
val SerdeFeature = Feature("serde", false, listOf("dep:serde")) | ||
val SerdeModule = | ||
|
@@ -26,35 +19,22 @@ val SerdeModule = | |
documentationOverride = "Implementations of `serde` for model types. NOTE: These implementations are NOT used for wire serialization as part of a Smithy protocol and WILL NOT match the wire format. They are provided for convenience only.", | ||
) | ||
|
||
class ClientSerdeDecorator : ClientCodegenDecorator { | ||
override val name: String = "ClientSerdeDecorator" | ||
override val order: Byte = 0 | ||
|
||
override fun extras( | ||
codegenContext: ClientCodegenContext, | ||
rustCrate: RustCrate, | ||
) = extrasCommon(codegenContext, rustCrate) | ||
} | ||
|
||
class ServerSerdeDecorator : ServerCodegenDecorator { | ||
override val name: String = "ServerSerdeDecorator" | ||
override val order: Byte = 0 | ||
|
||
override fun extras( | ||
codegenContext: ServerCodegenContext, | ||
rustCrate: RustCrate, | ||
) = extrasCommon(codegenContext, rustCrate) | ||
} | ||
|
||
// Just a common function to keep things DRY. | ||
private fun extrasCommon( | ||
/** | ||
* The entrypoint to both the client and server decorators. | ||
*/ | ||
fun extrasCommon( | ||
codegenContext: CodegenContext, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we rename this something related to serde—just because it's being called from an external context.
|
||
rustCrate: RustCrate, | ||
unwrapConstraints: (Shape) -> Writable, | ||
) { | ||
val roots = serializationRoots(codegenContext) | ||
if (roots.isNotEmpty()) { | ||
rustCrate.mergeFeature(SerdeFeature) | ||
val generator = SerializeImplGenerator(codegenContext) | ||
val generator = | ||
SerializeImplGenerator( | ||
codegenContext, | ||
unwrapConstraints, | ||
) | ||
rustCrate.withModule(SerdeModule) { | ||
roots.forEach { | ||
generator.generateRootSerializerForShape(it)(this) | ||
|
@@ -70,6 +50,6 @@ private fun extrasCommon( | |
*/ | ||
fun serializationRoots(ctx: CodegenContext): List<Shape> { | ||
val serviceShape = ctx.serviceShape | ||
val walker = Walker(ctx.model) | ||
val walker = DirectedWalker(ctx.model) | ||
return walker.walkShapes(serviceShape).filter { it.hasTrait<SerdeTrait>() } | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,7 +3,7 @@ | |
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
package software.amazon.smithy.rust.codegen.serde | ||
package software.amazon.smithy.rust.codegen.core.smithy.customizations.serde | ||
|
||
import software.amazon.smithy.codegen.core.Symbol | ||
import software.amazon.smithy.model.knowledge.TopDownIndex | ||
|
@@ -39,7 +39,6 @@ import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate | |
import software.amazon.smithy.rust.codegen.core.rustlang.withBlock | ||
import software.amazon.smithy.rust.codegen.core.rustlang.writable | ||
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.RuntimeType | ||
import software.amazon.smithy.rust.codegen.core.smithy.SimpleShapes | ||
import software.amazon.smithy.rust.codegen.core.smithy.contextName | ||
|
@@ -57,10 +56,11 @@ import software.amazon.smithy.rust.codegen.core.util.isEventStream | |
import software.amazon.smithy.rust.codegen.core.util.isTargetUnit | ||
import software.amazon.smithy.rust.codegen.core.util.letIf | ||
import software.amazon.smithy.rust.codegen.core.util.toPascalCase | ||
import software.amazon.smithy.rust.codegen.server.smithy.ServerRustSettings | ||
import software.amazon.smithy.rust.codegen.server.smithy.hasConstraintTrait | ||
|
||
class SerializeImplGenerator(private val codegenContext: CodegenContext) { | ||
class SerializeImplGenerator( | ||
private val codegenContext: CodegenContext, | ||
private val unwrapConstraints: (Shape) -> Writable, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
) { | ||
private val model = codegenContext.model | ||
private val topIndex = TopDownIndex.of(model) | ||
|
||
|
@@ -123,14 +123,14 @@ class SerializeImplGenerator(private val codegenContext: CodegenContext) { | |
} | ||
if (wrapper != null && applyTo != null) { | ||
rustTemplate( | ||
"&#{wrapper}(#{applyTo}#{unwrapConstraints})", "wrapper" to wrapper, | ||
"&#{wrapper}(#{applyTo})", | ||
"wrapper" to wrapper, | ||
"applyTo" to applyTo, | ||
"unwrapConstraints" to shape.unwrapConstraints(), | ||
) | ||
} else { | ||
deps?.toSymbol().also { addDependency(it) } | ||
applyTo?.invoke(this) | ||
shape.unwrapConstraints()(this) | ||
unwrapConstraints(shape)(this) | ||
} | ||
} | ||
} | ||
|
@@ -270,7 +270,7 @@ class SerializeImplGenerator(private val codegenContext: CodegenContext) { | |
val baseValue = | ||
writable { | ||
rust("self.value") | ||
shape.unwrapConstraints()(this) | ||
unwrapConstraints(shape)(this) | ||
if (shape.isStringShape) { | ||
rust(".as_str()") | ||
} | ||
|
@@ -315,9 +315,7 @@ class SerializeImplGenerator(private val codegenContext: CodegenContext) { | |
val module = serdeSubmodule(shape) | ||
val type = module.toType().resolve(name).toSymbol() | ||
val base = | ||
writable { rust("self.value.0") }.letIf(shape.hasConstraintTrait() && constraintTraitsEnabled()) { | ||
it.plus { rust(".0") } | ||
} | ||
writable { rust("self.value.0") }.plus(unwrapConstraints(shape)) | ||
val serialization = | ||
implSerializeConfiguredWrapper(type) { | ||
body(base)(this) | ||
|
@@ -336,21 +334,6 @@ class SerializeImplGenerator(private val codegenContext: CodegenContext) { | |
return wrapperStruct | ||
} | ||
|
||
private fun constraintTraitsEnabled(): Boolean = | ||
codegenContext.target == CodegenTarget.SERVER && | ||
(codegenContext.settings as ServerRustSettings).codegenConfig.publicConstrainedTypes | ||
|
||
private fun Shape.unwrapConstraints(): Writable { | ||
val shape = this | ||
return writable { | ||
if (constraintTraitsEnabled() && hasConstraintTrait()) { | ||
if (isBlobShape || isTimestampShape || isDocumentShape || shape is NumberShape) { | ||
rust(".0") | ||
} | ||
} | ||
} | ||
} | ||
|
||
/** | ||
* Serialize the field of a structure, union, list or map. | ||
* | ||
|
@@ -455,14 +438,14 @@ class SerializeImplGenerator(private val codegenContext: CodegenContext) { | |
} | ||
|
||
private fun serializeDateTime(shape: TimestampShape): RuntimeType = | ||
RuntimeType.forInlineFun("SerializeDateTime", Companion.PrimitiveShapesModule) { | ||
RuntimeType.forInlineFun("SerializeDateTime", PrimitiveShapesModule) { | ||
implSerializeConfigured(codegenContext.symbolProvider.toSymbol(shape)) { | ||
rust("serializer.serialize_str(&self.value.to_string())") | ||
} | ||
} | ||
|
||
private fun serializeBlob(shape: BlobShape): RuntimeType = | ||
RuntimeType.forInlineFun("SerializeBlob", Companion.PrimitiveShapesModule) { | ||
RuntimeType.forInlineFun("SerializeBlob", PrimitiveShapesModule) { | ||
implSerializeConfigured(codegenContext.symbolProvider.toSymbol(shape)) { | ||
rustTemplate( | ||
""" | ||
|
@@ -478,7 +461,7 @@ class SerializeImplGenerator(private val codegenContext: CodegenContext) { | |
} | ||
|
||
private fun serializeByteStream(shape: BlobShape): RuntimeType = | ||
RuntimeType.forInlineFun("SerializeByteStream", Companion.PrimitiveShapesModule) { | ||
RuntimeType.forInlineFun("SerializeByteStream", PrimitiveShapesModule) { | ||
implSerializeConfigured(RuntimeType.byteStream(codegenContext.runtimeConfig).toSymbol()) { | ||
// This doesn't work yet—there is no way to get data out of a ByteStream from a sync context | ||
rustTemplate( | ||
|
@@ -498,7 +481,7 @@ class SerializeImplGenerator(private val codegenContext: CodegenContext) { | |
} | ||
|
||
private fun serializeDocument(shape: DocumentShape): RuntimeType = | ||
RuntimeType.forInlineFun("SerializeDocument", Companion.PrimitiveShapesModule) { | ||
RuntimeType.forInlineFun("SerializeDocument", PrimitiveShapesModule) { | ||
implSerializeConfigured(codegenContext.symbolProvider.toSymbol(shape)) { | ||
rustTemplate( | ||
""" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.