Unverified Commit 6ca5ed21 authored by Julian Antonielli's avatar Julian Antonielli Committed by GitHub
Browse files

Implement `@length` on collections (#2028) 



* Refactor `ConstrainedMapGenerator` slightly

* Add `@length` list operation in `constraints.smithy`

* Add more setup required for rendering constraint `list`s

* Add initial draft of `ConstrainedCollectionGenerator`

* Add license header to `LengthTraitCommon`

* Add draft of collection constraint violation generation

Copy `MapCostraintViolationGenerator` into
`CollectionConstraintViolationGenerator` for now.

* Add `Visibility.toRustQualifier`

* Implement `CollectionConstraintViolationGenerator`

* Use `TraitInfo` on `CollectionConstraintViolationGenerator`

* Update docs on `ConstrainedCollectionGenerator`

* Improve formatting on rust code

* Don't generate `ConstraintViolation` enum twice

* Make symbol provider work with constrained lists

* Fix call to `ConstraintViolation::Member`

* Render constraint validation functions for lists

* Fix call to `usize::to_string`

* Use map json customisation for collections as well

* Update to use overhauled module system

* Add constraint traits check for collection shapes

* Remove test checking that `@length` is not used in `list`s

* Refactor use of `shape.isDirectlyConstrained`

* Fix failing `UnconstrainedCollectionGeneratorTest` test

* Fix test

* Actually fix the test

* Update changelog

* Fix `constrainedCollectionCheck`

* Add tests for `ConstrainedCollectionGenerator`

* Make tests work for when sets are implemented

* Remove mention of `set` in changelog

Co-authored-by: default avatardavid-perez <d@vidp.dev>

* Fix styling in `constraints.smithy`

Co-authored-by: default avatardavid-perez <d@vidp.dev>

* Use `check` instead of `assert`

* Grammar fix in comment about `Option<Box<_>>`

* Rename unsupported length data class for blobs

- `UnsupportedLengthTraitOnCollectionOrOnBlobShape` -> `UnsupportedLengthTraitOnBlobShape`

* Add TODO about `uniqueItems` not being implemented yet

* Change error message: `unconstrained` -> `not constrained`

* Add imports to fix docs

* Remove unused `modelsModuleWriter` parameter

* Use `filterIsInstance` and coalesce `filter + map`

* Add `check` in json customization

* Use `Set` instead of `List` for supported contraints

* Use `toMutableList` to avoid creating an extra list

* Add `@length` list to `ConA`

* Add `@httpQuery`-bound `@length` list example

* Add `@httpHeader`-bound `@length` list

* Fix `UnconstrainedCollectionGeneratorTest` test

* Fix rendering of constrained lists as header values

* Split very long line

* Add docs for `ConstraintViolation::Member` for lists

* Pass `length` variable name to `LengthTrait.rustCondition`

* Refactor long conditional

* Homogenise conditional

Co-authored-by: default avatardavid-perez <d@vidp.dev>
parent 97b9bb76

CHANGELOG.next.toml

+2 −1
Original line number Diff line number Diff line
@@ -242,6 +242,7 @@ message = """ 


* The `length` trait on `string` shapes.

* The `length` trait on `map` shapes.

* The `length` trait on `list` shapes.

* The `range` trait on `byte` shapes.

* The `range` trait on `short` shapes.

* The `range` trait on `integer` shapes.
@@ -252,7 +253,7 @@ Upon receiving a request that violates the modeled constraints, the server SDK w 


Unsupported (constraint trait, target shape) combinations will now fail at code generation time, whereas previously they were just ignored. This is a breaking change to raise awareness in service owners of their server SDKs behaving differently than what was modeled. To continue generating a server SDK with unsupported constraint traits, set `codegenConfig.ignoreUnsupportedConstraints` to `true` in your `smithy-build.json`.

"""

references = ["smithy-rs#1199", "smithy-rs#1342", "smithy-rs#1401", "smithy-rs#2005", "smithy-rs#1998", "smithy-rs#2034", "smithy-rs#2036"]

references = ["smithy-rs#1199", "smithy-rs#1342", "smithy-rs#1401", "smithy-rs#1998", "smithy-rs#2005", "smithy-rs#2028", "smithy-rs#2034", "smithy-rs#2036"]

meta = { "breaking" = true, "tada" = true, "bug" = false, "target" = "server" }

author = "david-perez"

codegen-core/common-test-models/constraints.smithy

+46 −2
Original line number Diff line number Diff line
@@ -21,6 +21,7 @@ service ConstraintsService { 
        QueryParamsTargetingMapOfLengthStringOperation,

        QueryParamsTargetingMapOfListOfLengthStringOperation,

        QueryParamsTargetingMapOfSetOfLengthStringOperation,

        QueryParamsTargetingMapOfLengthListOfPatternStringOperation,

        QueryParamsTargetingMapOfListOfEnumStringOperation,



        QueryParamsTargetingMapOfPatternStringOperation,
@@ -101,6 +102,13 @@ operation QueryParamsTargetingMapOfSetOfLengthStringOperation { 
    errors: [ValidationException]

}



@http(uri: "/query-params-targeting-map-of-length-list-of-pattern-string-operation", method: "POST")

operation QueryParamsTargetingMapOfLengthListOfPatternStringOperation {

    input: QueryParamsTargetingMapOfLengthListOfPatternStringOperationInputOutput,

    output: QueryParamsTargetingMapOfLengthListOfPatternStringOperationInputOutput,

    errors: [ValidationException]

}



@http(uri: "/query-params-targeting-map-of-list-of-enum-string-operation", method: "POST")

operation QueryParamsTargetingMapOfListOfEnumStringOperation {

    input: QueryParamsTargetingMapOfListOfEnumStringOperationInputOutput,
@@ -225,8 +233,16 @@ structure ConstrainedHttpBoundShapesOperationInputOutput { 
    // @httpHeader("X-Length-Set")

    // lengthStringSetHeader: SetOfLengthString,



    @httpHeader("X-Length-List")

    lengthStringListHeader: ListOfLengthString,

    @httpHeader("X-List-Length-String")

    listLengthStringHeader: ListOfLengthString,



    @httpHeader("X-Length-List-Pattern-String")

    lengthListPatternStringHeader: LengthListOfPatternString,



    // TODO(https://github.com/awslabs/smithy-rs/issues/1401): a `set` shape is

    //  just a `list` shape with `uniqueItems`, which hasn't been implemented yet.

    // @httpHeader("X-Length-Set-Pattern-String")

    // lengthSetPatternStringHeader: LengthSetOfPatternString,



    // TODO(https://github.com/awslabs/smithy-rs/issues/1401): a `set` shape is

    //  just a `list` shape with `uniqueItems`, which hasn't been implemented yet.
@@ -279,6 +295,9 @@ structure ConstrainedHttpBoundShapesOperationInputOutput { 
    @httpQuery("lengthStringList")

    lengthStringListQuery: ListOfLengthString,



    @httpQuery("lengthListPatternString")

    lengthListPatternStringQuery: LengthListOfPatternString,



    // TODO(https://github.com/awslabs/smithy-rs/issues/1401): a `set` shape is

    //  just a `list` shape with `uniqueItems`, which hasn't been implemented yet.

    // @httpQuery("lengthStringSet")
@@ -366,6 +385,11 @@ structure QueryParamsTargetingMapOfSetOfLengthStringOperationInputOutput { 
    mapOfSetOfLengthString: MapOfSetOfLengthString

}



structure QueryParamsTargetingMapOfLengthListOfPatternStringOperationInputOutput {

    @httpQueryParams

    mapOfLengthListOfPatternString: MapOfLengthListOfPatternString

}



structure QueryParamsTargetingMapOfListOfEnumStringOperationInputOutput {

    @httpQueryParams

    mapOfListOfEnumString: MapOfListOfEnumString
@@ -501,6 +525,11 @@ structure ConA { 
    // TODO(https://github.com/awslabs/smithy-rs/issues/1401): a `set` shape is

    //  just a `list` shape with `uniqueItems`, which hasn't been implemented yet.

    // setOfLengthPatternString: SetOfLengthPatternString,



    lengthListOfPatternString: LengthListOfPatternString,

    // TODO(https://github.com/awslabs/smithy-rs/issues/1401): a `set` shape is

    //  just a `list` shape with `uniqueItems`, which hasn't been implemented yet.

    // lengthSetOfPatternString: LengthSetOfPatternString,

}



map MapOfLengthString {
@@ -561,6 +590,11 @@ map MapOfSetOfLengthString { 
    value: ListOfLengthString

}



map MapOfLengthListOfPatternString {

    key: PatternString,

    value: LengthListOfPatternString

}



// TODO(https://github.com/awslabs/smithy-rs/issues/1401): a `set` shape is

//  just a `list` shape with `uniqueItems`, which hasn't been implemented yet.

// map MapOfSetOfRangeInteger {
@@ -706,6 +740,11 @@ set SetOfLengthPatternString { 
    member: LengthPatternString

}



@length(min: 5, max: 9)

set LengthSetOfPatternString {

    member: PatternString

}



list ListOfLengthString {

    member: LengthString

}
@@ -762,6 +801,11 @@ list ListOfLengthPatternString { 
    member: LengthPatternString

}



@length(min: 12, max: 39)

list LengthListOfPatternString {

    member: PatternString

}



structure ConB {

    @required

    nice: String,

codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/RustType.kt

+17 −1
Original line number Diff line number Diff line
@@ -325,7 +325,23 @@ fun RustType.isCopy(): Boolean = when (this) { 
enum class Visibility {

    PRIVATE,

    PUBCRATE,

    PUBLIC;



    companion object {

        fun publicIf(condition: Boolean, ifNot: Visibility): Visibility =

            if (condition) {

                PUBLIC

            } else {

                ifNot

            }

    }



    fun toRustQualifier(): String =

        when (this) {

            PRIVATE -> ""

            PUBCRATE -> "pub(crate)"

            PUBLIC -> "pub"

        }

}



/**

codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/SymbolVisitor.kt

+1 −1
Original line number Diff line number Diff line
@@ -346,7 +346,7 @@ open class SymbolVisitor( 


    override fun memberShape(shape: MemberShape): Symbol {

        val target = model.expectShape(shape.target)

        // Handle boxing first so we end up with Option<Box<_>>, not Box<Option<_>>.

        // Handle boxing first, so we end up with Option<Box<_>>, not Box<Option<_>>.

        return handleOptionality(

            handleRustBoxing(toSymbol(target), shape),

            shape,

codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/http/HttpBindingGenerator.kt

+8 −1
Original line number Diff line number Diff line
@@ -590,7 +590,14 @@ class HttpBindingGenerator( 
        renderErrorMessage: (String) -> Writable,

    ) {

        val loopVariable = ValueExpression.Reference(safeName("inner"))

        rustBlock("for ${loopVariable.name} in ${value.asRef()}") {

        val context = HeaderValueSerializationContext(value, shape)

        for (customization in customizations) {

            customization.section(

                HttpBindingSection.BeforeRenderingHeaderValue(context),

            )(this)

        }



        rustBlock("for ${loopVariable.name} in ${context.valueExpression.asRef()}") {

            this.renderHeaderValue(

                headerName,

                loopVariable,