Skip to content

Commit cec8741

Browse files
82marbagaws-sdk-rust-ci
authored andcommitted
Nested server structure member shapes targeting simple shapes with default trait (#2352)
* Nested server structure member shapes targeting simple shapes with `@default` Signed-off-by: Daniele Ahmed <ahmeddan@amazon.de> * Add changelog entry Signed-off-by: Daniele Ahmed <ahmeddan@amazon.de> * Update CHANGELOG Signed-off-by: Daniele Ahmed <ahmeddan@amazon.de> * Add unit test Signed-off-by: Daniele Ahmed <ahmeddan@amazon.de> * Add integration test Signed-off-by: Daniele Ahmed <ahmeddan@amazon.de> * Change method Signed-off-by: Daniele Ahmed <ahmeddan@amazon.de> * Include comment to describe the test Signed-off-by: Daniele Ahmed <ahmeddan@amazon.de> --------- Signed-off-by: Daniele Ahmed <ahmeddan@amazon.de>
1 parent 2e2f51f commit cec8741

File tree

4 files changed

+72
-2
lines changed

4 files changed

+72
-2
lines changed

CHANGELOG.next.toml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,3 +10,10 @@
1010
# references = ["smithy-rs#920"]
1111
# meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "client | server | all"}
1212
# author = "rcoh"
13+
14+
[[smithy-rs]]
15+
message = """Fix bug whereby nested server structure member shapes targeting simple shapes with `@default`
16+
produced Rust code that did not compile"""
17+
references = ["smithy-rs#2352", "smithy-rs#2343"]
18+
meta = { "breaking" = false, "tada" = false, "bug" = true, "target" = "server"}
19+
author = "82marbag"

codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/Constraints.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ fun Shape.isDirectlyConstrained(symbolProvider: SymbolProvider): Boolean = when
8383
// The only reason why the functions in this file have
8484
// to take in a `SymbolProvider` is because non-`required` blob streaming members are interpreted as
8585
// `required`, so we can't use `member.isOptional` here.
86-
this.members().map { symbolProvider.toSymbol(it) }.any { !it.isOptional() }
86+
this.members().any { !symbolProvider.toSymbol(it).isOptional() && !it.hasNonNullDefault() }
8787
}
8888

8989
is MapShape -> this.hasTrait<LengthTrait>()

codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ConstraintsTest.kt

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ package software.amazon.smithy.rust.codegen.server.smithy
88
import io.kotest.inspectors.forAll
99
import io.kotest.matchers.shouldBe
1010
import org.junit.jupiter.api.Test
11+
import software.amazon.smithy.model.shapes.BooleanShape
1112
import software.amazon.smithy.model.shapes.ListShape
1213
import software.amazon.smithy.model.shapes.MapShape
1314
import software.amazon.smithy.model.shapes.MemberShape
@@ -81,7 +82,12 @@ class ConstraintsTest {
8182
@length(min: 1, max: 5)
8283
mapAPrecedence: MapA
8384
}
84-
""".asSmithyModel()
85+
86+
structure StructWithInnerDefault {
87+
@default(false)
88+
inner: PrimitiveBoolean
89+
}
90+
""".asSmithyModel(smithyVersion = "2")
8591
private val symbolProvider = serverTestSymbolProvider(model)
8692

8793
private val testInputOutput = model.lookup<StructureShape>("test#TestInputOutput")
@@ -93,6 +99,8 @@ class ConstraintsTest {
9399
private val structA = model.lookup<StructureShape>("test#StructureA")
94100
private val structAInt = model.lookup<MemberShape>("test#StructureA\$int")
95101
private val structAString = model.lookup<MemberShape>("test#StructureA\$string")
102+
private val structWithInnerDefault = model.lookup<StructureShape>("test#StructWithInnerDefault")
103+
private val primitiveBoolean = model.lookup<BooleanShape>("smithy.api#PrimitiveBoolean")
96104

97105
@Test
98106
fun `it should detect supported constrained traits as constrained`() {
@@ -119,4 +127,10 @@ class ConstraintsTest {
119127
mapB.canReachConstrainedShape(model, symbolProvider) shouldBe true
120128
recursiveShape.canReachConstrainedShape(model, symbolProvider) shouldBe true
121129
}
130+
131+
@Test
132+
fun `it should not consider shapes with the default trait as constrained`() {
133+
structWithInnerDefault.canReachConstrainedShape(model, symbolProvider) shouldBe false
134+
primitiveBoolean.isDirectlyConstrained(symbolProvider) shouldBe false
135+
}
122136
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
/*
2+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package software.amazon.smithy.rust.codegen.server.smithy.generators
7+
8+
import org.junit.jupiter.api.Test
9+
import software.amazon.smithy.rust.codegen.core.testutil.asSmithyModel
10+
import software.amazon.smithy.rust.codegen.server.smithy.testutil.serverIntegrationTest
11+
12+
class ServerBuilderConstraintViolationsTest {
13+
14+
// This test exists not to regress on [this](https://github.com/awslabs/smithy-rs/issues/2343) issue.
15+
// We generated constraint violation variants, pointing to a structure (StructWithInnerDefault below),
16+
// but the structure was not constrained, because the structure's member have a default value
17+
// and default values are validated at generation time from the model.
18+
@Test
19+
fun `it should not generate constraint violations for members with a default value`() {
20+
val model = """
21+
namespace test
22+
23+
use aws.protocols#restJson1
24+
use smithy.framework#ValidationException
25+
26+
@restJson1
27+
service SimpleService {
28+
operations: [Operation]
29+
}
30+
31+
@http(uri: "/operation", method: "POST")
32+
operation Operation {
33+
input: OperationInput
34+
errors: [ValidationException]
35+
}
36+
37+
structure OperationInput {
38+
@required
39+
requiredStructureWithInnerDefault: StructWithInnerDefault
40+
}
41+
42+
structure StructWithInnerDefault {
43+
@default(false)
44+
inner: PrimitiveBoolean
45+
}
46+
""".asSmithyModel(smithyVersion = "2")
47+
serverIntegrationTest(model)
48+
}
49+
}

0 commit comments

Comments
 (0)