Skip to content

Commit

Permalink
Detect constraints on event stream errors (#2069)
Browse files Browse the repository at this point in the history
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]: smithy-lang/smithy#1388

Co-authored-by: Luca Palmieri <[email protected]>
  • Loading branch information
david-perez and LukeMathWalker authored Dec 9, 2022
1 parent 381467d commit f54b36b
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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) }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -130,24 +132,39 @@ 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
"""
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 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
Expand Down

0 comments on commit f54b36b

Please sign in to comment.