Skip to content

Commit 6806f42

Browse files
david-perezunexge
authored andcommitted
Fix server code generation of @httpPayload-bound constrained shapes (#2584)
The issue is we're not changing the return type of the payload deserializing function to be the unconstrained type (e.g. the builder in case of an `@httpPayload`-bound structure shape) when the shape is constrained. Yet another example of why code-generating `constraints.smithy` (see #2101) is important. Closes #2583. ## Testing The added integration test operation fails to compile without this patch. ## Checklist <!--- If a checkbox below is not applicable, then please DELETE it rather than leaving it unchecked --> - [x] I have updated `CHANGELOG.next.toml` if I made changes to the smithy-rs codegen or runtime crates ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._
1 parent e0f71ca commit 6806f42

File tree

5 files changed

+31
-5
lines changed

5 files changed

+31
-5
lines changed

CHANGELOG.next.toml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,3 +122,9 @@ message = "Fix generation of constrained shapes reaching `@sensitive` shapes"
122122
references = ["smithy-rs#2582", "smithy-rs#2585"]
123123
meta = { "breaking" = false, "tada" = false, "bug" = true, "target" = "server" }
124124
author = "david-perez"
125+
126+
[[smithy-rs]]
127+
message = "Fix server code generation bug affecting constrained shapes bound with `@httpPayload`"
128+
references = ["smithy-rs#2583", "smithy-rs#2584"]
129+
meta = { "breaking" = false, "tada" = false, "bug" = true, "target" = "server" }
130+
author = "david-perez"

codegen-core/common-test-models/constraints.smithy

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,9 @@ service ConstraintsService {
1212
operations: [
1313
ConstrainedShapesOperation,
1414
ConstrainedHttpBoundShapesOperation,
15+
ConstrainedHttpPayloadBoundShapeOperation,
1516
ConstrainedRecursiveShapesOperation,
17+
1618
// `httpQueryParams` and `httpPrefixHeaders` are structurually
1719
// exclusive, so we need one operation per target shape type
1820
// combination.
@@ -59,6 +61,13 @@ operation ConstrainedHttpBoundShapesOperation {
5961
errors: [ValidationException]
6062
}
6163

64+
@http(uri: "/constrained-http-payload-bound-shape-operation", method: "POST")
65+
operation ConstrainedHttpPayloadBoundShapeOperation {
66+
input: ConstrainedHttpPayloadBoundShapeOperationInputOutput,
67+
output: ConstrainedHttpPayloadBoundShapeOperationInputOutput,
68+
errors: [ValidationException]
69+
}
70+
6271
@http(uri: "/constrained-recursive-shapes-operation", method: "POST")
6372
operation ConstrainedRecursiveShapesOperation {
6473
input: ConstrainedRecursiveShapesOperationInputOutput,
@@ -311,6 +320,12 @@ structure ConstrainedHttpBoundShapesOperationInputOutput {
311320
enumStringListQuery: ListOfEnumString,
312321
}
313322

323+
structure ConstrainedHttpPayloadBoundShapeOperationInputOutput {
324+
@required
325+
@httpPayload
326+
httpPayloadBoundConstrainedShape: ConA
327+
}
328+
314329
structure QueryParamsTargetingMapOfPatternStringOperationInputOutput {
315330
@httpQueryParams
316331
mapOfPatternString: MapOfPatternString

codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/http/HttpBindingGenerator.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ class HttpBindingGenerator(
235235
// The output needs to be Optional when deserializing the payload body or the caller signature
236236
// will not match.
237237
val outputT = symbolProvider.toSymbol(binding.member).makeOptional()
238-
rustBlock("pub fn $fnName(body: &[u8]) -> std::result::Result<#T, #T>", outputT, errorSymbol) {
238+
rustBlock("pub(crate) fn $fnName(body: &[u8]) -> std::result::Result<#T, #T>", outputT, errorSymbol) {
239239
deserializePayloadBody(
240240
binding,
241241
errorSymbol,

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,11 @@ class HttpBoundProtocolPayloadGenerator(
239239
) {
240240
val ref = if (payloadMetadata.takesOwnership) "" else "&"
241241
val serializer = protocolFunctions.serializeFn(member, fnNameSuffix = "http_payload") { fnName ->
242-
val outputT = if (member.isStreaming(model)) symbolProvider.toSymbol(member) else RuntimeType.ByteSlab.toSymbol()
242+
val outputT = if (member.isStreaming(model)) {
243+
symbolProvider.toSymbol(member)
244+
} else {
245+
RuntimeType.ByteSlab.toSymbol()
246+
}
243247
rustBlockTemplate(
244248
"pub fn $fnName(payload: $ref#{Member}) -> Result<#{outputT}, #{BuildError}>",
245249
"Member" to symbolProvider.toSymbol(member),

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -154,14 +154,15 @@ class JsonParserGenerator(
154154

155155
override fun payloadParser(member: MemberShape): RuntimeType {
156156
val shape = model.expectShape(member.target)
157+
val returnSymbolToParse = returnSymbolToParse(shape)
157158
check(shape is UnionShape || shape is StructureShape || shape is DocumentShape) {
158-
"payload parser should only be used on structures & unions"
159+
"Payload parser should only be used on structure shapes, union shapes, and document shapes."
159160
}
160161
return protocolFunctions.deserializeFn(shape, fnNameSuffix = "payload") { fnName ->
161162
rustBlockTemplate(
162-
"pub fn $fnName(input: &[u8]) -> Result<#{Shape}, #{Error}>",
163+
"pub(crate) fn $fnName(input: &[u8]) -> Result<#{ReturnType}, #{Error}>",
163164
*codegenScope,
164-
"Shape" to symbolProvider.toSymbol(shape),
165+
"ReturnType" to returnSymbolToParse.symbol,
165166
) {
166167
val input = if (shape is DocumentShape) {
167168
"input"

0 commit comments

Comments
 (0)