-
Notifications
You must be signed in to change notification settings - Fork 196
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
Implement @length
on collections
#2028
Merged
Merged
Changes from all commits
Commits
Show all changes
56 commits
Select commit
Hold shift + click to select a range
91ca287
Refactor `ConstrainedMapGenerator` slightly
jjant 4e79202
Add `@length` list operation in `constraints.smithy`
jjant 000175c
Add more setup required for rendering constraint `list`s
jjant 6299e29
Add initial draft of `ConstrainedCollectionGenerator`
jjant 617591d
Add license header to `LengthTraitCommon`
jjant d0a6fab
Add draft of collection constraint violation generation
jjant b71d1a9
Add `Visibility.toRustQualifier`
jjant e684ce7
Implement `CollectionConstraintViolationGenerator`
jjant c255474
Use `TraitInfo` on `CollectionConstraintViolationGenerator`
jjant c109bb5
Update docs on `ConstrainedCollectionGenerator`
jjant 4ebc39a
Improve formatting on rust code
jjant eaa4e6f
Don't generate `ConstraintViolation` enum twice
jjant ba85018
Make symbol provider work with constrained lists
jjant 94de917
Fix call to `ConstraintViolation::Member`
jjant b8b029f
Render constraint validation functions for lists
jjant 8be886c
Fix call to `usize::to_string`
jjant e414189
Use map json customisation for collections as well
jjant 273d71c
Merge branch 'main' into jjant/implement-length-trait-on-collections
jjant 6c50000
Update to use overhauled module system
jjant acc3c98
Add constraint traits check for collection shapes
jjant 6c1a2c2
Remove test checking that `@length` is not used in `list`s
jjant c190d36
Refactor use of `shape.isDirectlyConstrained`
jjant ad2d5e9
Fix failing `UnconstrainedCollectionGeneratorTest` test
jjant 818477e
Fix test
jjant 715f96b
Actually fix the test
jjant d2c59da
Update changelog
jjant 2d22133
Fix `constrainedCollectionCheck`
jjant 1eb8650
Add tests for `ConstrainedCollectionGenerator`
jjant 072df40
Make tests work for when sets are implemented
jjant 1a37a33
Merge branch 'main' into jjant/implement-length-trait-on-collections
jjant 3f906a2
Merge branch 'main' into jjant/implement-length-trait-on-collections
jjant dfa3979
Remove mention of `set` in changelog
jjant 925c225
Fix styling in `constraints.smithy`
jjant 3b92790
Use `check` instead of `assert`
jjant bb24581
Grammar fix in comment about `Option<Box<_>>`
jjant 5cebcb9
Rename unsupported length data class for blobs
jjant 074276a
Add TODO about `uniqueItems` not being implemented yet
jjant 28a3397
Change error message: `unconstrained` -> `not constrained`
jjant b0679c5
Add imports to fix docs
jjant ebaa3f0
Remove unused `modelsModuleWriter` parameter
jjant 0d5a313
Use `filterIsInstance` and coalesce `filter + map`
jjant ac2f737
Add `check` in json customization
jjant 57cd6f1
Use `Set` instead of `List` for supported contraints
jjant 964d78f
Use `toMutableList` to avoid creating an extra list
jjant 838ada5
Add `@length` list to `ConA`
jjant 1e818e4
Add `@httpQuery`-bound `@length` list example
jjant 111ee98
Add `@httpHeader`-bound `@length` list
jjant a0e561e
Fix `UnconstrainedCollectionGeneratorTest` test
jjant f759439
Fix rendering of constrained lists as header values
jjant 3f97e7c
Split very long line
jjant 9e8c22b
Add docs for `ConstraintViolation::Member` for lists
jjant 0bea362
Merge branch 'main' into jjant/implement-length-trait-on-collections
jjant 92ceba8
Pass `length` variable name to `LengthTrait.rustCondition`
jjant 01cfe23
Refactor long conditional
jjant 1ce32c8
Homogenise conditional
jjant 3b6932f
Merge branch 'main' into jjant/implement-length-trait-on-collections
jjant File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -62,8 +62,8 @@ sealed class JsonSerializerSection(name: String) : Section(name) { | |
JsonSerializerSection("ServerError") | ||
|
||
/** Manipulate the serializer context for a map prior to it being serialized. **/ | ||
data class BeforeIteratingOverMap(val shape: MapShape, val context: JsonSerializerGenerator.Context<MapShape>) : | ||
JsonSerializerSection("BeforeIteratingOverMap") | ||
data class BeforeIteratingOverMapOrCollection(val shape: Shape, val context: JsonSerializerGenerator.Context<Shape>) : | ||
JsonSerializerSection("BeforeIteratingOverMapOrCollection") | ||
|
||
/** Manipulate the serializer context for a non-null member prior to it being serialized. **/ | ||
data class BeforeSerializingNonNullMember(val shape: Shape, val context: JsonSerializerGenerator.MemberContext) : | ||
|
@@ -90,7 +90,7 @@ class JsonSerializerGenerator( | |
private val jsonName: (MemberShape) -> String, | ||
private val customizations: List<JsonSerializerCustomization> = listOf(), | ||
) : StructuredDataSerializerGenerator { | ||
data class Context<T : Shape>( | ||
data class Context<out T : Shape>( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Weird syntax, but this makes In particular, it lets us use |
||
/** Expression that retrieves a JsonValueWriter from either a JsonObjectWriter or JsonArrayWriter */ | ||
val writerExpression: String, | ||
/** Expression representing the value to write to the JsonValueWriter */ | ||
|
@@ -455,6 +455,9 @@ class JsonSerializerGenerator( | |
|
||
private fun RustWriter.serializeCollection(context: Context<CollectionShape>) { | ||
val itemName = safeName("item") | ||
for (customization in customizations) { | ||
customization.section(JsonSerializerSection.BeforeIteratingOverMapOrCollection(context.shape, context))(this) | ||
} | ||
rustBlock("for $itemName in ${context.valueExpression.asRef()}") { | ||
serializeMember(MemberContext.collectionMember(context, itemName)) | ||
} | ||
|
@@ -464,7 +467,7 @@ class JsonSerializerGenerator( | |
val keyName = safeName("key") | ||
val valueName = safeName("value") | ||
for (customization in customizations) { | ||
customization.section(JsonSerializerSection.BeforeIteratingOverMap(context.shape, context))( | ||
customization.section(JsonSerializerSection.BeforeIteratingOverMapOrCollection(context.shape, context))( | ||
this, | ||
) | ||
} | ||
|
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
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
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
Oops, something went wrong.
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.
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.
We need to add a list with the
@length
trait inConA
.We need to add a list with the
@length
trait bound with@httpQuery
.We need to add a list with the
@length
trait bound with@httpHeader
.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.
Done in 964d78f...111ee98.
This uncovered a bug in
http_serde.rs
, I probably need to make the same changes there as for the JSON customization.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.
This should be fixed in f759439.