Unverified Commit 5a5a7c44 authored by 82marbag's avatar 82marbag Committed by GitHub
Browse files

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: default avatarDaniele Ahmed <ahmeddan@amazon.de>

* Add changelog entry

Signed-off-by: default avatarDaniele Ahmed <ahmeddan@amazon.de>

* Update CHANGELOG

Signed-off-by: default avatarDaniele Ahmed <ahmeddan@amazon.de>

* Add unit test

Signed-off-by: default avatarDaniele Ahmed <ahmeddan@amazon.de>

* Add integration test

Signed-off-by: default avatarDaniele Ahmed <ahmeddan@amazon.de>

* Change method

Signed-off-by: default avatarDaniele Ahmed <ahmeddan@amazon.de>

* Include comment to describe the test

Signed-off-by: default avatarDaniele Ahmed <ahmeddan@amazon.de>

---------

Signed-off-by: default avatarDaniele Ahmed <ahmeddan@amazon.de>
parent 198c5009
Loading
Loading
Loading
Loading
+7 −0
Original line number Diff line number Diff line
@@ -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"
+1 −1
Original line number Diff line number Diff line
@@ -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<LengthTrait>()
+15 −1
Original line number Diff line number Diff line
@@ -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<StructureShape>("test#TestInputOutput")
@@ -93,6 +99,8 @@ class ConstraintsTest {
    private val structA = model.lookup<StructureShape>("test#StructureA")
    private val structAInt = model.lookup<MemberShape>("test#StructureA\$int")
    private val structAString = model.lookup<MemberShape>("test#StructureA\$string")
    private val structWithInnerDefault = model.lookup<StructureShape>("test#StructWithInnerDefault")
    private val primitiveBoolean = model.lookup<BooleanShape>("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
    }
}
+49 −0
Original line number Diff line number Diff line
/*
 * 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)
    }
}