Skip to content

Commit 15c73d5

Browse files
drganjooFahad Zubair
andauthored
Fix CBOR codegen for recursive types by applying Box to union variants (#4116)
# Fix CBOR recursive data structure handling with Box implementation This PR addresses an issue in the CBOR codegen where recursive union data structures aren't properly boxed during deserialization, resulting in uncompilable Rust code. ## Problem The current CBOR implementation correctly defines recursive data structures with `Box` in type definitions, but fails to apply the corresponding `Box::new` wrapper during deserialization. This causes type mismatches when deserializing recursive Smithy models. For example, with this Smithy model: ```smithy structure RecursiveOperationInputOutputNested1 { variant: FooChoice, } union FooChoice { choice1: String choice2: RecursiveOperationInputOutputNested1 } ``` The generated Rust type correctly uses `Box`: ```rust pub enum FooChoice { Choice1(::std::string::String), Choice2(::std::boxed::Box<crate::model::RecursiveOperationInputOutputNested1>), } ``` But the deserializer doesn't wrap the result with `Box::new`: ```rust // In the deserializer function: FooChoice::Choice2( crate::protocol_serde::shape_recursive_operation_input_output_nested1::de_recursive_operation_input_output_nested1(decoder) ?) ``` This leads to the compiler error: ``` error[E0308]: `?` operator has incompatible types --> shape_foo_choice.rs:14:9 | 14 | / crate::protocol_serde::shape_recursive_operation_input_output_nested1::de_recursive_operation_input_output_nested1(decoder) 15 | | ?), | |_____^ expected `Box<RecursiveOperationInputOutputNested1>`, found `RecursiveOperationInputOutputNested1` ``` ## Solution This PR modifies the CBOR codegen to add `.map(Box::new)` when deserializing values for union variants that are defined with `Box`. The fixed code will generate: ```rust FooChoice::Choice2( crate::protocol_serde::shape_recursive_operation_input_output_nested1::de_recursive_operation_input_output_nested1(decoder) .map(Box::new)? ) ``` This ensures type compatibility between the defined structure and the deserialized data. ## Testing Added a protocol test in `rpcv2Cbor-extras.smithy` file. Closes: #4115 --------- Co-authored-by: Fahad Zubair <fahadzub@amazon.com>
1 parent ad97759 commit 15c73d5

File tree

2 files changed

+77
-0
lines changed

2 files changed

+77
-0
lines changed

codegen-core/common-test-models/rpcv2Cbor-extras.smithy

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ service RpcV2CborService {
1515
ComplexStructOperation
1616
EmptyStructOperation
1717
SingleMemberStructOperation
18+
RecursiveUnionOperation
1819
]
1920
}
2021

@@ -49,6 +50,11 @@ operation SingleMemberStructOperation {
4950
output: SingleMemberStruct
5051
}
5152

53+
operation RecursiveUnionOperation {
54+
input: RecursiveOperationInputOutput
55+
output: RecursiveOperationInputOutput
56+
}
57+
5258
apply EmptyStructOperation @httpMalformedRequestTests([
5359
{
5460
id: "AdditionalTokensEmptyStruct",
@@ -244,6 +250,52 @@ apply SimpleStructOperation @httpResponseTests([
244250
}
245251
])
246252

253+
apply RecursiveUnionOperation @httpResponseTests([
254+
{
255+
id: "RpcV2CborRecursiveShapesWithUnion",
256+
documentation: "Serializes recursive structures with union",
257+
protocol: rpcv2Cbor,
258+
code: 200,
259+
body: "v2ZuZXN0ZWS/Y2Zvb2RGb28xZ3ZhcmlhbnS/Z2Nob2ljZTFrT3V0ZXJDaG9pY2X/Zm5lc3RlZL9jYmFyZEJhcjFvcmVjdXJzaXZlTWVtYmVyv2Nmb29kRm9vMmd2YXJpYW50v2djaG9pY2Uyv2Nmb29kRm9vM2ZuZXN0ZWS/Y2JhcmRCYXIzb3JlY3Vyc2l2ZU1lbWJlcr9jZm9vZEZvbzRndmFyaWFudL9nY2hvaWNlMWtJbm5lckNob2ljZf//////Zm5lc3RlZL9jYmFyZEJhcjL//////w==",
260+
bodyMediaType: "application/cbor",
261+
headers: {
262+
"smithy-protocol": "rpc-v2-cbor",
263+
"Content-Type": "application/cbor"
264+
},
265+
params: {
266+
nested: {
267+
foo: "Foo1",
268+
variant: {
269+
choice1: "OuterChoice"
270+
},
271+
nested: {
272+
bar: "Bar1",
273+
recursiveMember: {
274+
foo: "Foo2",
275+
variant: {
276+
choice2: {
277+
foo: "Foo3",
278+
nested: {
279+
bar: "Bar3",
280+
recursiveMember: {
281+
foo: "Foo4",
282+
variant: {
283+
choice1: "InnerChoice"
284+
}
285+
}
286+
}
287+
}
288+
},
289+
nested: {
290+
bar: "Bar2"
291+
}
292+
}
293+
}
294+
}
295+
}
296+
}
297+
])
298+
247299
structure ErrorSerializationOperationOutput {
248300
errorShape: ValidationException
249301
}
@@ -360,3 +412,23 @@ enum Suit {
360412
HEART
361413
SPADE
362414
}
415+
416+
structure RecursiveOperationInputOutput {
417+
nested: RecursiveOperationInputOutputNested1
418+
}
419+
420+
structure RecursiveOperationInputOutputNested1 {
421+
foo: String,
422+
nested: RecursiveOperationInputOutputNested2,
423+
variant: FooChoice,
424+
}
425+
426+
structure RecursiveOperationInputOutputNested2 {
427+
bar: String,
428+
recursiveMember: RecursiveOperationInputOutputNested1,
429+
}
430+
431+
union FooChoice {
432+
choice1: String
433+
choice2: RecursiveOperationInputOutputNested1
434+
}

codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/protocols/parse/CborParserGenerator.kt

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -366,6 +366,11 @@ class CborParserGenerator(
366366
} else {
367367
withBlock("${member.memberName.dq()} => #T::$variantName(", "?),", returnSymbolToParse.symbol) {
368368
deserializeMember(member).invoke(this)
369+
370+
val symbol = symbolProvider.toSymbol(member)
371+
if (symbol.isRustBoxed()) {
372+
rust(".map(Box::new)")
373+
}
369374
}
370375
}
371376
}

0 commit comments

Comments
 (0)