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

Operations with an event stream member shape must have ValidationException #3814

Merged
merged 4 commits into from
Sep 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
9 changes: 9 additions & 0 deletions .changelog/7869753.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
applies_to: ["server"]
authors: ["drganjoo"]
references: ["smithy-rs#3813"]
breaking: true
new_feature: false
bug_fix: true
---
Operations with event stream member shapes must include `ValidationException` in the errors list. This is necessary because the member shape is a required field, and the builder for the operation input or output returns a `std::result::Result` with the error set to `crate::model::ValidationExceptionField`.
1 change: 1 addition & 0 deletions codegen-core/common-test-models/constraints.smithy
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@ operation StreamingBlobOperation {
operation EventStreamsOperation {
input: EventStreamsOperationInputOutput,
output: EventStreamsOperationInputOutput,
errors: [ValidationException]
}

structure ConstrainedShapesOperationInputOutput {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import software.amazon.smithy.model.traits.UniqueItemsTrait
import software.amazon.smithy.rust.codegen.core.smithy.DirectedWalker
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.hasEventStreamMember
import software.amazon.smithy.rust.codegen.core.util.hasTrait
import software.amazon.smithy.rust.codegen.core.util.inputShape
import software.amazon.smithy.rust.codegen.core.util.orNull
Expand Down Expand Up @@ -190,7 +191,7 @@ fun operationShapesThatMustHaveValidationException(
.filter { operationShape ->
// Walk the shapes reachable via this operation input.
walker.walkShapes(operationShape.inputShape(model))
.any { it is SetShape || it is EnumShape || it.hasConstraintTrait() }
.any { it is SetShape || it is EnumShape || it.hasConstraintTrait() || it.hasEventStreamMember(model) }
}
.toSet()
}
Expand All @@ -207,7 +208,6 @@ fun validateOperationsWithConstrainedInputHaveValidationExceptionAttached(
// `ValidationException` attached in `errors`. https://github.com/smithy-lang/smithy-rs/pull/1199#discussion_r809424783
// TODO(https://github.com/smithy-lang/smithy-rs/issues/1401): This check will go away once we add support for
// `disableDefaultValidation` set to `true`, allowing service owners to map from constraint violations to operation errors.
val walker = DirectedWalker(model)
val operationsWithConstrainedInputWithoutValidationExceptionSet =
operationShapesThatMustHaveValidationException(model, service)
.filter { !it.errors.contains(validationExceptionShapeId) }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,51 @@ internal class ValidateUnsupportedConstraintsAreNotUsedTest {
""".trimIndent()
}

@Test
fun `should detect when event streams are used, even without constraints, as the event member is required`() {
val model =
"""
$baseModel
structure TestInputOutput {
eventStream: EventStream
}
@streaming
union EventStream {
message: Message,
error: Error
}
structure Message {
lengthString: String
}
structure Error {
message: String
}
""".asSmithyModel()
val service = model.lookup<ServiceShape>("test#TestService")
val validationResult =
validateOperationsWithConstrainedInputHaveValidationExceptionAttached(
model,
service,
SmithyValidationExceptionConversionGenerator.SHAPE_ID,
)

validationResult.messages shouldHaveSize 1

// Asserts the exact message, to ensure the formatting is appropriate.
validationResult.messages[0].message shouldBe
"""
Operation test#TestOperation takes in input that is constrained (https://awslabs.github.io/smithy/2.0/spec/constraint-traits.html), and as such can fail with a validation exception. You must model this behavior in the operation shape in your model file.
```smithy
use smithy.framework#ValidationException

operation TestOperation {
...
errors: [..., ValidationException] // <-- Add this.
}
```
""".trimIndent()
}

private val constraintTraitOnStreamingBlobShapeModel =
"""
$baseModel
Expand Down