Skip to content

Commit

Permalink
Nested server structure member shapes targeting simple shapes with de…
Browse files Browse the repository at this point in the history
…fault trait (smithy-lang#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>
  • Loading branch information
82marbag committed Feb 17, 2023
1 parent 198c500 commit 5a5a7c4
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 2 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.next.toml
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"
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
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
primitiveBoolean.isDirectlyConstrained(symbolProvider) shouldBe false
}
}
@@ -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)
}
}

0 comments on commit 5a5a7c4

Please sign in to comment.