Skip to content

Commit

Permalink
Fix serde on constrained blobs (#3893)
Browse files Browse the repository at this point in the history
## Motivation and Context
#3890 

## Description
Replace the computed type (which will be a constrained shape, not what
we want!) with an absolute type.

## Testing
- [x] New unit test that fails without this fix applied.

## Checklist
<!--- If a checkbox below is not applicable, then please DELETE it
rather than leaving it unchecked -->
- [x] For changes to the smithy-rs codegen or runtime crates, I have
created a changelog entry Markdown file in the `.changelog` directory,
specifying "client," "server," or both in the `applies_to` key.


----

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._
  • Loading branch information
rcoh authored Dec 23, 2024
1 parent c7b1038 commit 66b2311
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 11 deletions.
9 changes: 9 additions & 0 deletions .changelog/1729878769.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
applies_to: ["server"]
authors: ["rcoh"]
references: ["smithy-rs#3890"]
breaking: false
new_feature: false
bug_fix: true
---
Fix bug in `serde` decorator that generated non-compiling code on some models
Original file line number Diff line number Diff line change
Expand Up @@ -462,8 +462,8 @@ class SerializeImplGenerator(private val codegenContext: CodegenContext) {
}

private fun serializeBlob(shape: BlobShape): RuntimeType =
RuntimeType.forInlineFun("SerializeBlob", Companion.PrimitiveShapesModule) {
implSerializeConfigured(codegenContext.symbolProvider.toSymbol(shape)) {
RuntimeType.forInlineFun("SerializeBlob", PrimitiveShapesModule) {
implSerializeConfigured(RuntimeType.blob(codegenContext.runtimeConfig).toSymbol()) {
rustTemplate(
"""
if serializer.is_human_readable() {
Expand All @@ -478,7 +478,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 +498,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 @@ -6,6 +6,7 @@
package software.amazon.smithy.rust.codegen.serde

import org.junit.jupiter.api.Test
import software.amazon.smithy.model.node.Node
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.Attribute.Companion.cfg
Expand Down Expand Up @@ -190,22 +191,22 @@ class SerdeDecoratorTest {
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")
""".asSmithyModel(smithyVersion = "2")

val params =
IntegrationTestParams(cargoCommand = "cargo test --all-features", service = "com.example#MyResourceService")
Expand Down Expand Up @@ -241,14 +242,14 @@ class SerdeDecoratorTest {
"""
namespace com.example
use aws.protocols#awsJson1_0
@awsJson1_0
service MyService {
operations: [MyOperation]
}
operation MyOperation { }
""".asSmithyModel(smithyVersion = "2")
""".asSmithyModel(smithyVersion = "2")

val params =
IntegrationTestParams(cargoCommand = "cargo test --all-features", service = "com.example#MyService")
Expand All @@ -261,6 +262,48 @@ class SerdeDecoratorTest {
}
}

val onlyConstrained =
"""
namespace com.example
use smithy.rust#serde
use aws.protocols#awsJson1_0
use smithy.framework#ValidationException
@awsJson1_0
service HelloService {
operations: [SayHello],
version: "1"
}
@serde
operation SayHello {
input: TestInput
errors: [ValidationException]
}
structure TestInput {
@length(max: 10)
shortBlob: Blob
}
""".asSmithyModel(smithyVersion = "2")

// There is a "race condition" where if the first blob shape serialized is constrained, it triggered unexpected
// behavior where the constrained shape was used instead. This test verifies the fix.
// Fixes https://github.com/smithy-lang/smithy-rs/issues/3890
@Test
fun compilesOnlyConstrainedModel() {
val constrainedShapesSettings =
Node.objectNodeBuilder().withMember(
"codegen",
Node.objectNodeBuilder()
.withMember("publicConstrainedTypes", true)
.withMember("includeFluentClient", false)
.build(),
).build()
serverIntegrationTest(
onlyConstrained,
params.copy(additionalSettings = constrainedShapesSettings),
) { clientCodegenContext, rustCrate ->
}
}

@Test
fun generateSerializersThatWorkServer() {
serverIntegrationTest(simpleModel, params = params) { ctx, crate ->
Expand Down

0 comments on commit 66b2311

Please sign in to comment.