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

Disallow @uniqueItems-constrained list shapes that reach a map shape #2375

Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ import software.amazon.smithy.model.shapes.BlobShape
import software.amazon.smithy.model.shapes.ByteShape
import software.amazon.smithy.model.shapes.EnumShape
import software.amazon.smithy.model.shapes.IntegerShape
import software.amazon.smithy.model.shapes.ListShape
import software.amazon.smithy.model.shapes.LongShape
import software.amazon.smithy.model.shapes.MapShape
import software.amazon.smithy.model.shapes.MemberShape
import software.amazon.smithy.model.shapes.OperationShape
import software.amazon.smithy.model.shapes.ServiceShape
Expand All @@ -36,13 +38,17 @@ private sealed class UnsupportedConstraintMessageKind {
private val constraintTraitsUberIssue = "https://github.com/awslabs/smithy-rs/issues/1401"

fun intoLogMessage(ignoreUnsupportedConstraints: Boolean): LogMessage {
fun buildMessage(intro: String, willSupport: Boolean, trackingIssue: String, canBeIgnored: Boolean = true): String {
fun buildMessage(intro: String, willSupport: Boolean, trackingIssue: String? = null, canBeIgnored: Boolean = true): String {
var msg = """
$intro
$intro
This is not supported in the smithy-rs server SDK."""
if (willSupport) {
msg += """
It will be supported in the future. See the tracking issue ($trackingIssue)."""
It will be supported in the future."""
}
if (trackingIssue != null) {
msg += """
For more information, and to report if you're affected by this, please use the tracking issue: $trackingIssue."""
}
if (canBeIgnored) {
msg += """
Expand Down Expand Up @@ -106,6 +112,19 @@ private sealed class UnsupportedConstraintMessageKind {
level,
buildMessageShapeHasUnsupportedConstraintTrait(shape, uniqueItemsTrait, constraintTraitsUberIssue),
)

is UnsupportedMapShapeReachableFromUniqueItemsList -> LogMessage(
Level.SEVERE,
buildMessage(
"""
The map shape `${mapShape.id}` is reachable from the list shape `${listShape.id}`, which has the
`@uniqueItems` trait attached.
""".trimIndent().replace("\n", " "),
willSupport = false,
trackingIssue = "https://github.com/awslabs/smithy/issues/1567",
canBeIgnored = false,
)
)
}
}
}
Expand All @@ -129,6 +148,12 @@ private data class UnsupportedRangeTraitOnShape(val shape: Shape, val rangeTrait
private data class UnsupportedUniqueItemsTraitOnShape(val shape: Shape, val uniqueItemsTrait: UniqueItemsTrait) :
UnsupportedConstraintMessageKind()

private data class UnsupportedMapShapeReachableFromUniqueItemsList(
val listShape: ListShape,
val uniqueItemsTrait: UniqueItemsTrait,
val mapShape: MapShape,
) : UnsupportedConstraintMessageKind()

data class LogMessage(val level: Level, val message: String)
data class ValidationResult(val shouldAbort: Boolean, val messages: List<LogMessage>)

Expand Down Expand Up @@ -245,11 +270,29 @@ fun validateUnsupportedConstraints(
.map { (shape, rangeTrait) -> UnsupportedRangeTraitOnShape(shape, rangeTrait as RangeTrait) }
.toSet()

// 5. `@uniqueItems` cannot reach a map shape.
// See https://github.com/awslabs/smithy/issues/1567.
val mapShapeReachableFromUniqueItemsListShapeSet = walker
.walkShapes(service)
.asSequence()
.filterMapShapesToTraits(setOf(UniqueItemsTrait::class.java))
.flatMap { (listShape, uniqueItemsTrait) ->
walker.walkShapes(listShape).filterIsInstance<MapShape>().map { mapShape ->
UnsupportedMapShapeReachableFromUniqueItemsList(
listShape as ListShape,
uniqueItemsTrait as UniqueItemsTrait,
mapShape,
)
}
}
.toSet()

val messages =
unsupportedConstraintOnMemberShapeSet.map { it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints) } +
unsupportedLengthTraitOnStreamingBlobShapeSet.map { it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints) } +
unsupportedConstraintShapeReachableViaAnEventStreamSet.map { it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints) } +
unsupportedRangeTraitOnShapeSet.map { it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints) }
unsupportedRangeTraitOnShapeSet.map { it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints) } +
mapShapeReachableFromUniqueItemsListShapeSet.map { it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints) }

return ValidationResult(shouldAbort = messages.any { it.level == Level.SEVERE }, messages)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ internal class ValidateUnsupportedConstraintsAreNotUsedTest {
namespace test

service TestService {
version: "123",
operations: [TestOperation]
}

Expand Down Expand Up @@ -186,6 +185,47 @@ internal class ValidateUnsupportedConstraintsAreNotUsedTest {
}
}

private val mapShapeReachableFromUniqueItemsListShapeModel =
"""
$baseModel

structure TestInputOutput {
uniqueItemsList: UniqueItemsList
}

@uniqueItems
list UniqueItemsList {
member: Map
}

map Map {
key: String
value: String
}
""".asSmithyModel()

@Test
fun `it should detect when a map shape is reachable from a uniqueItems list shape`() {
val validationResult = validateModel(mapShapeReachableFromUniqueItemsListShapeModel)

validationResult.messages shouldHaveSize 1
validationResult.shouldAbort shouldBe true
validationResult.messages[0].message shouldContain """
The map shape `test#Map` is reachable from the list shape `test#UniqueItemsList`, which has the
`@uniqueItems` trait attached.
""".trimIndent().replace("\n", " ")
}

@Test
fun `it should abort when a map shape is reachable from a uniqueItems list shape, despite opting into ignoreUnsupportedConstraintTraits`() {
val validationResult = validateModel(
mapShapeReachableFromUniqueItemsListShapeModel,
ServerCodegenConfig().copy(ignoreUnsupportedConstraints = true),
)

validationResult.shouldAbort shouldBe true
}

@Test
fun `it should abort when constraint traits in event streams are used, despite opting into ignoreUnsupportedConstraintTraits`() {
val validationResult = validateModel(
Expand Down