Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nested server structure member shapes targeting simple shapes with default trait #2352

Merged
merged 10 commits into from
Feb 17, 2023
7 changes: 7 additions & 0 deletions CHANGELOG.next.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Original file line number Diff line number Diff line change
Expand Up @@ -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>()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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")
Expand All @@ -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`() {
Expand All @@ -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
82marbag marked this conversation as resolved.
Show resolved Hide resolved
primitiveBoolean.isDirectlyConstrained(symbolProvider) shouldBe false
}
}
Original file line number Diff line number Diff line change
@@ -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 = """
82marbag marked this conversation as resolved.
Show resolved Hide resolved
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)
}
}