From 5a5a7c44d6a9c70f56313eafed5c2023e3efd504 Mon Sep 17 00:00:00 2001 From: 82marbag <69267416+82marbag@users.noreply.github.com> Date: Fri, 17 Feb 2023 13:19:34 -0500 Subject: [PATCH] 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 * Add changelog entry Signed-off-by: Daniele Ahmed * Update CHANGELOG Signed-off-by: Daniele Ahmed * Add unit test Signed-off-by: Daniele Ahmed * Add integration test Signed-off-by: Daniele Ahmed * Change method Signed-off-by: Daniele Ahmed * Include comment to describe the test Signed-off-by: Daniele Ahmed --------- Signed-off-by: Daniele Ahmed --- CHANGELOG.next.toml | 7 +++ .../rust/codegen/server/smithy/Constraints.kt | 2 +- .../codegen/server/smithy/ConstraintsTest.kt | 16 +++++- .../ServerBuilderConstraintViolationsTest.kt | 49 +++++++++++++++++++ 4 files changed, 72 insertions(+), 2 deletions(-) create mode 100644 codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerBuilderConstraintViolationsTest.kt diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index c15c52fe6c..23107ade80 100644 --- a/CHANGELOG.next.toml +++ b/CHANGELOG.next.toml @@ -222,3 +222,10 @@ message = "Fluent builder methods on the client are now marked as deprecated whe references = ["aws-sdk-rust#740"] meta = { "breaking" = false, "tada" = true, "bug" = true, "target" = "client"} author = "Velfi" + +[[smithy-rs]] +message = """Fix bug whereby nested server structure member shapes targeting simple shapes with `@default` +produced Rust code that did not compile""" +references = ["smithy-rs#2352", "smithy-rs#2343"] +meta = { "breaking" = false, "tada" = false, "bug" = true, "target" = "server"} +author = "82marbag" diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/Constraints.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/Constraints.kt index f6cc943a9c..14fd6a5d0b 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/Constraints.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/Constraints.kt @@ -83,7 +83,7 @@ fun Shape.isDirectlyConstrained(symbolProvider: SymbolProvider): Boolean = when // The only reason why the functions in this file have // to take in a `SymbolProvider` is because non-`required` blob streaming members are interpreted as // `required`, so we can't use `member.isOptional` here. - this.members().map { symbolProvider.toSymbol(it) }.any { !it.isOptional() } + this.members().any { !symbolProvider.toSymbol(it).isOptional() && !it.hasNonNullDefault() } } is MapShape -> this.hasTrait() diff --git a/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ConstraintsTest.kt b/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ConstraintsTest.kt index 946027ce02..30e5e64813 100644 --- a/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ConstraintsTest.kt +++ b/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ConstraintsTest.kt @@ -8,6 +8,7 @@ package software.amazon.smithy.rust.codegen.server.smithy import io.kotest.inspectors.forAll import io.kotest.matchers.shouldBe import org.junit.jupiter.api.Test +import software.amazon.smithy.model.shapes.BooleanShape import software.amazon.smithy.model.shapes.ListShape import software.amazon.smithy.model.shapes.MapShape import software.amazon.smithy.model.shapes.MemberShape @@ -81,7 +82,12 @@ class ConstraintsTest { @length(min: 1, max: 5) mapAPrecedence: MapA } - """.asSmithyModel() + + structure StructWithInnerDefault { + @default(false) + inner: PrimitiveBoolean + } + """.asSmithyModel(smithyVersion = "2") private val symbolProvider = serverTestSymbolProvider(model) private val testInputOutput = model.lookup("test#TestInputOutput") @@ -93,6 +99,8 @@ class ConstraintsTest { private val structA = model.lookup("test#StructureA") private val structAInt = model.lookup("test#StructureA\$int") private val structAString = model.lookup("test#StructureA\$string") + private val structWithInnerDefault = model.lookup("test#StructWithInnerDefault") + private val primitiveBoolean = model.lookup("smithy.api#PrimitiveBoolean") @Test fun `it should detect supported constrained traits as constrained`() { @@ -119,4 +127,10 @@ class ConstraintsTest { mapB.canReachConstrainedShape(model, symbolProvider) shouldBe true recursiveShape.canReachConstrainedShape(model, symbolProvider) shouldBe true } + + @Test + fun `it should not consider shapes with the default trait as constrained`() { + structWithInnerDefault.canReachConstrainedShape(model, symbolProvider) shouldBe false + primitiveBoolean.isDirectlyConstrained(symbolProvider) shouldBe false + } } diff --git a/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerBuilderConstraintViolationsTest.kt b/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerBuilderConstraintViolationsTest.kt new file mode 100644 index 0000000000..1f685909f3 --- /dev/null +++ b/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerBuilderConstraintViolationsTest.kt @@ -0,0 +1,49 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +package software.amazon.smithy.rust.codegen.server.smithy.generators + +import org.junit.jupiter.api.Test +import software.amazon.smithy.rust.codegen.core.testutil.asSmithyModel +import software.amazon.smithy.rust.codegen.server.smithy.testutil.serverIntegrationTest + +class ServerBuilderConstraintViolationsTest { + + // This test exists not to regress on [this](https://github.com/awslabs/smithy-rs/issues/2343) issue. + // We generated constraint violation variants, pointing to a structure (StructWithInnerDefault below), + // but the structure was not constrained, because the structure's member have a default value + // and default values are validated at generation time from the model. + @Test + fun `it should not generate constraint violations for members with a default value`() { + val model = """ + namespace test + + use aws.protocols#restJson1 + use smithy.framework#ValidationException + + @restJson1 + service SimpleService { + operations: [Operation] + } + + @http(uri: "/operation", method: "POST") + operation Operation { + input: OperationInput + errors: [ValidationException] + } + + structure OperationInput { + @required + requiredStructureWithInnerDefault: StructWithInnerDefault + } + + structure StructWithInnerDefault { + @default(false) + inner: PrimitiveBoolean + } + """.asSmithyModel(smithyVersion = "2") + serverIntegrationTest(model) + } +}