Unverified Commit f54b36b7 authored by david-perez's avatar david-perez Committed by GitHub
Browse files

Detect constraints on event stream errors (#2069) 

Constrained shapes in the closure of an event stream are not supported
[0]. However, the `ValidateUnsupportedConstraints` validator was not
detecting constrained shapes in event stream errors because the
`EventStreamNormalizer` model transformer pulls them out of the
`@streaming` union shape.

This commit makes it so that the validator will detect such usages, by
leveraging the `SyntheticEventStreamUnionTrait` trait that gets attached
in the model transformer.

[0]: https://github.com/awslabs/smithy/issues/1388



Co-authored-by: default avatarLuca Palmieri <20745048+LukeMathWalker@users.noreply.github.com>
parent 381467d2

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

+14 −5
Original line number Diff line number Diff line
@@ -19,13 +19,13 @@ import software.amazon.smithy.model.shapes.SetShape 
import software.amazon.smithy.model.shapes.Shape

import software.amazon.smithy.model.shapes.ShapeId

import software.amazon.smithy.model.shapes.ShortShape

import software.amazon.smithy.model.shapes.UnionShape

import software.amazon.smithy.model.traits.LengthTrait

import software.amazon.smithy.model.traits.RangeTrait

import software.amazon.smithy.model.traits.RequiredTrait

import software.amazon.smithy.model.traits.StreamingTrait

import software.amazon.smithy.model.traits.Trait

import software.amazon.smithy.model.traits.UniqueItemsTrait

import software.amazon.smithy.rust.codegen.core.smithy.traits.SyntheticEventStreamUnionTrait

import software.amazon.smithy.rust.codegen.core.util.expectTrait

import software.amazon.smithy.rust.codegen.core.util.getTrait

import software.amazon.smithy.rust.codegen.core.util.hasTrait
@@ -213,15 +213,24 @@ fun validateUnsupportedConstraints( 


    // 3. Constraint traits in event streams are used. Their semantics are unclear.

    // TODO(https://github.com/awslabs/smithy/issues/1388)

    val unsupportedConstraintOnShapeReachableViaAnEventStreamSet = walker

    val eventStreamShapes = walker

        .walkShapes(service)

        .asSequence()

        .filterIsInstance<UnionShape>()

        .filter { it.hasTrait<StreamingTrait>() }

        .filter { it.hasTrait<SyntheticEventStreamUnionTrait>() }

    val unsupportedConstraintOnNonErrorShapeReachableViaAnEventStreamSet = eventStreamShapes

        .flatMap { walker.walkShapes(it) }

        .filterMapShapesToTraits(allConstraintTraits)

        .map { (shape, trait) -> UnsupportedConstraintOnShapeReachableViaAnEventStream(shape, trait) }

        .toSet()

    val eventStreamErrors = eventStreamShapes.map { it.expectTrait<SyntheticEventStreamUnionTrait>() }.map { it.errorMembers }

    val unsupportedConstraintErrorShapeReachableViaAnEventStreamSet = eventStreamErrors

        .flatMap { it }

        .flatMap { walker.walkShapes(it) }

        .filterMapShapesToTraits(allConstraintTraits)

        .map { (shape, trait) -> UnsupportedConstraintOnShapeReachableViaAnEventStream(shape, trait) }

        .toSet()

    val unsupportedConstraintShapeReachableViaAnEventStreamSet =

        unsupportedConstraintOnNonErrorShapeReachableViaAnEventStreamSet + unsupportedConstraintErrorShapeReachableViaAnEventStreamSet



    // 4. Length trait on blob shapes is used. It has not been implemented yet.

    // TODO(https://github.com/awslabs/smithy-rs/issues/1401)
@@ -261,7 +270,7 @@ fun validateUnsupportedConstraints( 
    val messages =

        unsupportedConstraintOnMemberShapeSet.map { it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints) } +

            unsupportedLengthTraitOnStreamingBlobShapeSet.map { it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints) } +

            unsupportedConstraintOnShapeReachableViaAnEventStreamSet.map { it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints) } +

            unsupportedConstraintShapeReachableViaAnEventStreamSet.map { it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints) } +

            unsupportedLengthTraitOnBlobShapeSet.map { it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints) } +

            unsupportedRangeTraitOnShapeSet.map { it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints) } +

            unsupportedUniqueItemsTraitOnShapeSet.map { it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints) }

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

+25 −8
Original line number Diff line number Diff line
@@ -5,6 +5,7 @@ 


package software.amazon.smithy.rust.codegen.server.smithy



import io.kotest.inspectors.forOne

import io.kotest.inspectors.forSome

import io.kotest.inspectors.shouldForAll

import io.kotest.matchers.collections.shouldHaveAtLeastSize
@@ -14,6 +15,7 @@ import io.kotest.matchers.string.shouldContain 
import org.junit.jupiter.api.Test

import software.amazon.smithy.model.Model

import software.amazon.smithy.model.shapes.ServiceShape

import software.amazon.smithy.rust.codegen.core.smithy.transformers.EventStreamNormalizer

import software.amazon.smithy.rust.codegen.core.testutil.asSmithyModel

import software.amazon.smithy.rust.codegen.core.util.lookup

import java.util.logging.Level
@@ -130,25 +132,40 @@ internal class ValidateUnsupportedConstraintsAreNotUsedTest { 


            @streaming

            union EventStream {

                message: Message

                message: Message,

                error: Error

            }



            structure Message {

                lengthString: LengthString

            }



            structure Error {

                @required

                message: String

            }



            @length(min: 1)

            string LengthString

            """.asSmithyModel()

        val validationResult = validateModel(model)

        val validationResult = validateModel(EventStreamNormalizer.transform(model))



        validationResult.messages shouldHaveSize 1

        validationResult.messages[0].message shouldContain

        validationResult.messages shouldHaveSize 2

        validationResult.messages.forOne {

            it.message shouldContain

                """

                The string shape `test#LengthString` has the constraint trait `smithy.api#length` attached.

                This shape is also part of an event stream; it is unclear what the semantics for constrained shapes in event streams are.

                """.trimIndent().replace("\n", " ")

        }

        validationResult.messages.forOne {

            it.message shouldContain

                """

                The member shape `test#Error${"$"}message` has the constraint trait `smithy.api#required` attached.

                This shape is also part of an event stream; it is unclear what the semantics for constrained shapes in event streams are.

                """.trimIndent().replace("\n", " ")

        }

    }



    @Test

    fun `it should detect when the length trait on blob shapes is used`() {