-
Notifications
You must be signed in to change notification settings - Fork 225
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
Add trait that can constrain shape closures #2156
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
856c399
to
8d692a4
Compare
This commit adds a new meta-trait called `constrainShapes` that is used to constrain the closure of shapes that can be used when a trait is applied to a shape. This can be used on a protocol definition trait to disallow the use of certain kinds of shapes that are incompatible with the protocol (e.g., some AWS protocols that use XML don't support document types). This trait is a replacement for the `noInlineDocument` property of the `protocolDefinition` trait, but generalizes the concept to work with any kind of shape or trait. While this could have been acheived with metadata based validators before, introducing a trait makes it easier and more performant to constrain shapes. For example: ``` metadata validators = [ { name: "EmitEachSelector" id: "MyProtocol.NoDocuments" configuration: { selector: "service [trait|smithy.example#xProto] $service(*) ~> member :test(> document)" messageTemplate: "xProto found on @{var|service} does not support document types" } } ] ``` Becomes: ``` @trait(selector: "service") @protocolDefinition @constrainShapes( "MyProtocol.NoDocuments": { selector: "member :test(> document)" message: "This protocol does not support document types" } ) structure xProto {} ``` The advantage of a trait is that you get good validation messages for free, more performant scans of models when multiple constraints are needed (no need to crawl shapes multiple times), and the validation is reusable since it can be applied and shared via mixins. To prove out the concept, several MQTT trait validators were converted from Java code to in-model constraints. Partially addresses #2111
8d692a4
to
b247054
Compare
haydenbaker
approved these changes
Feb 22, 2024
This is great ! |
To better reflect that this trait is a way to apply additional validators to a trait, it's renamed to traitValidators. To make it easier to define validators that start from the applied shape, selector validators are now only given the applied shape as the starting shape rather than the closure of shapes. This makes it easier to write very specific selectors from the starting shape, more performant to write these kinds of specific selectors (they're what we used in the MQTT traits), and you can still validate the closure of a shape using "~>". Lots of closure validation might not be as performant, but we can address that if we ever need to using caches.
25a92bf
to
79956d4
Compare
Rather than just allow for starting shapes on select(), this commit allows starting shapes on any of the Select methods so that it also works with matches and consumeMatches. This would be necessary if we ever wanted to support message templates on traitValidators like we do on EmitEachSelector (i.e., you emit events for each match and not just each matched shape). I took care to ensure that these interface updates wouldn't break previously implemented Selector classes (it made it a little clunkier than I like, but it's backward compatible). This change also makes "message" optional for traitValidators in case we ever add support for something like a "messageTemplate" property.
79956d4
to
609d718
Compare
haydenbaker
approved these changes
Feb 23, 2024
This also allows actually emitting errors, right? |
Yes it allows ERROR |
kubukoz
reviewed
Apr 11, 2024
@@ -218,6 +218,7 @@ structure protocolDefinition { | |||
traits: TraitShapeIdList | |||
|
|||
/// Set to true if inline documents are not supported by this protocol. | |||
@deprecated(message: "Use the `@constrainShapes` trait instead") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oops 😅
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit adds a new meta-trait called
traitValidators
that is used to constrain shapes to which a trait is applied.This can be used on a protocol definition trait to disallow the use of certain kinds of shapes that are incompatible with the protocol (e.g., some AWS protocols that use XML don't support document types). This trait is a replacement for the
noInlineDocument
property of theprotocolDefinition
trait, but generalizes the concept to work with any kind of shape or trait.While this could have been achieved with metadata based validators before, introducing a trait makes it easier to constrain shapes. For example:
Becomes:
The advantage of a trait is that you get good validation messages for free, more performant scans of models (no need to every shape in the model to the selector), and the validation is reusable since it can be applied and shared via mixins.
Partially addresses #2111
Background
Testing
Links
map
shape validation to allow for other key types than "string" #2111By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.