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

Extract smithy.rust#serde trait to a new smithy-shapes Gradle subproject #3897

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
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 @@ -10,6 +10,7 @@ import software.amazon.smithy.codegen.core.ReservedWordSymbolProvider
import software.amazon.smithy.model.Model
import software.amazon.smithy.model.shapes.ServiceShape
import software.amazon.smithy.rust.codegen.client.smithy.customizations.ClientCustomizations
import software.amazon.smithy.rust.codegen.client.smithy.customizations.ClientSerdeDecorator
import software.amazon.smithy.rust.codegen.client.smithy.customizations.HttpAuthDecorator
import software.amazon.smithy.rust.codegen.client.smithy.customizations.HttpConnectorConfigDecorator
import software.amazon.smithy.rust.codegen.client.smithy.customizations.IdempotencyTokenDecorator
Expand Down Expand Up @@ -72,6 +73,7 @@ class RustClientCodegenPlugin : ClientDecoratableBuildPlugin() {
IdempotencyTokenDecorator(),
StalledStreamProtectionDecorator(),
StaticSdkFeatureTrackerDecorator(),
ClientSerdeDecorator(),
Copy link
Collaborator

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.

*decorator,
)

Expand Down
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
Copy link
Collaborator

Choose a reason for hiding this comment

The 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,
)
}
}
}
}
}
1 change: 1 addition & 0 deletions codegen-core/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ val smithyVersion: String by project

dependencies {
implementation(kotlin("stdlib-jdk8"))
implementation(project(":smithy-shapes"))
implementation("org.jsoup:jsoup:1.16.2")
api("software.amazon.smithy:smithy-codegen-core:$smithyVersion")
api("com.moandjiezana.toml:toml4j:0.7.2")
Expand Down
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 =
Expand All @@ -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,
Copy link
Collaborator

Choose a reason for hiding this comment

The 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.

serdeExtras maybe?

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)
Expand All @@ -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
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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,
Copy link
Collaborator

Choose a reason for hiding this comment

The 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 constrainedShapeToInner or something?

) {
private val model = codegenContext.model
private val topIndex = TopDownIndex.of(model)

Expand Down Expand Up @@ -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)
}
}
}
Expand Down Expand Up @@ -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()")
}
Expand Down Expand Up @@ -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)
Expand All @@ -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.
*
Expand Down Expand Up @@ -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(
"""
Expand All @@ -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(
Expand All @@ -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(
"""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.rust.codegen.core.rustlang.CargoDependency
import software.amazon.smithy.rust.codegen.core.rustlang.DependencyScope
Expand Down
Loading
Loading