Unverified Commit ec9588b1 authored by Luca Palmieri's avatar Luca Palmieri Committed by GitHub
Browse files

@range implementation for integer shapes (#2005) 



* Draft of @range implementation for integer shapes

* Green tests.

* Fix serialization of constrained integers.

* Fix tagger to include Integer simple shapes.
Fix de/serialization codegen.

* Add range trait to the entry about constraint traits in our changelog.

* Bind a range-constrained integer to various HTTP parts to test our implementation.

* Rename ConstrainedIntGeneratorTest to ConstrainedIntegerGeneratorTest for consistency.

* Remove AsRef implementation.

* Fix constraints models.

* Fix conversion.

* Use ReturnSymbolToParse to dry up.

* Fix builder when constrained types should not be exposed.

* Add customisation to unwrap constrained integers.

* Getting closer - collections need to be handled differently to make everything fall into place.

* Refactor `renderHeaders`

* Fix constraints test.

* Fix ebs.

* Rename for clarity.

* Remove unnecessary From implementation.

* Rename `Size` variant to `Range`.

* Remove stray comments.

* Rename for clarity

* Rename for consistency

* Add test to guard against types for which we do not support range yet

* DRY up branches and the relevant tests.

* Fix header name.

* Fix serialization bug for default values in headers.

* Fix serialization issue for primitive headers.

* Remove SetOfRangeInteger

* Fix pubcrateconstrained unit test.

* Remove duplication

* Revert "Remove SetOfRangeInteger"

This reverts commit f37a560bd0233ad65512c4916292f0f7f8c571e1.

* Reintroduce `SetOfRangeInteger`, but keep them commented out.

* Ignore leading whitespace.

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

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

* Update codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/protocols/parse/JsonParserGenerator.kt

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

* Update codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraints.kt

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

* Update codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedIntegerGeneratorTest.kt

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

* Unify with a single rustTemplate invocation.

* Rename `Type` to `NumberType`

* Update codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ConstrainedShapeSymbolProviderTest.kt

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

* Formatting issue.

* Move and rename test helper.

* Dry up the logic for default serialization.

* Ooops, I swapped the two branches.

* Add a wrapper block

* Fix support detection.

* Fix CHANGELOG.

* Add (failing) protocol tests for @range on byte/integer/long.

* Fix validation message.
Fix test definitions.

* Mark some tests as expected to fail.
Rename service.

* Use ValueExpression everywhere for more granular control of the ownership component.

* Use the new enhanced module facilities

* Fixes.

* Fix formatting

* Remove unnecessary when.

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

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

* Remove unused shapes.

* Avoid mixing concerns in our test shapes for integer constraints.

* Reuse the trait info machinery

* Update stale comment

* Fix unused attribute.

* Update codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/protocols/serialize/JsonSerializerGenerator.kt

Co-authored-by: default avatarZelda Hessler <zhessler@amazon.com>

* Avoid unnecessary bindings by manipulating the serialization context directly in the customisations.

* Avoid unnecessary bindings in header serialization by introducing and manipulating the header value serialization context directly in the customisations.

* Add code examples.

* Move `safeName` call into the if branch.

Co-authored-by: default avatardavid-perez <d@vidp.dev>
Co-authored-by: default avatarZelda Hessler <zhessler@amazon.com>
parent 27020be3

CHANGELOG.next.toml

+2 −1
Original line number Diff line number Diff line
@@ -242,13 +242,14 @@ message = """ 


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

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

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

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



Upon receiving a request that violates the modeled constraints, the server SDK will reject it with a message indicating why.



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#1998"]

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

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

author = "david-perez"

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

+79 −7
Original line number Diff line number Diff line
@@ -19,7 +19,6 @@ service ConstraintsService { 
        // combination.

        QueryParamsTargetingLengthMapOperation,

        QueryParamsTargetingMapOfLengthStringOperation,

        QueryParamsTargetingMapOfEnumStringOperation,

        QueryParamsTargetingMapOfListOfLengthStringOperation,

        QueryParamsTargetingMapOfSetOfLengthStringOperation,

        QueryParamsTargetingMapOfListOfEnumStringOperation,
@@ -30,6 +29,9 @@ service ConstraintsService { 
        QueryParamsTargetingMapOfListOfLengthPatternStringOperation,



        HttpPrefixHeadersTargetingLengthMapOperation,



        QueryParamsTargetingMapOfEnumStringOperation,

        QueryParamsTargetingMapOfListOfEnumStringOperation,

        // TODO(https://github.com/awslabs/smithy-rs/issues/1431)

        // HttpPrefixHeadersTargetingMapOfEnumStringOperation,
@@ -47,7 +49,7 @@ operation ConstrainedShapesOperation { 
    errors: [ValidationException]

}



@http(uri: "/constrained-http-bound-shapes-operation/{lengthStringLabel}/{enumStringLabel}", method: "POST")

@http(uri: "/constrained-http-bound-shapes-operation/{rangeIntegerLabel}/{lengthStringLabel}/{enumStringLabel}", method: "POST")

operation ConstrainedHttpBoundShapesOperation {

    input: ConstrainedHttpBoundShapesOperationInputOutput,

    output: ConstrainedHttpBoundShapesOperationInputOutput,
@@ -173,18 +175,24 @@ structure ConstrainedHttpBoundShapesOperationInputOutput { 
    @httpLabel

    lengthStringLabel: LengthString,



    @required

    @httpLabel

    rangeIntegerLabel: RangeInteger,



    @required

    @httpLabel

    enumStringLabel: EnumString,



    // TODO(https://github.com/awslabs/smithy-rs/issues/1394) `@required` not working

    // @required

    @httpPrefixHeaders("X-Prefix-Headers-")

    @required

    @httpPrefixHeaders("X-Length-String-Prefix-Headers-")

    lengthStringHeaderMap: MapOfLengthString,



    @httpHeader("X-Length")

    lengthStringHeader: LengthString,



    @httpHeader("X-Range-Integer")

    rangeIntegerHeader: RangeInteger,



    // @httpHeader("X-Length-MediaType")

    // lengthStringHeaderWithMediaType: MediaTypeLengthString,
@@ -196,6 +204,14 @@ structure ConstrainedHttpBoundShapesOperationInputOutput { 
    @httpHeader("X-Length-List")

    lengthStringListHeader: ListOfLengthString,



    // 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-Range-Integer-Set")

    // rangeIntegerSetHeader: SetOfRangeInteger,



    @httpHeader("X-Range-Integer-List")

    rangeIntegerListHeader: ListOfRangeInteger,



    // TODO(https://github.com/awslabs/smithy-rs/issues/1431)

    // @httpHeader("X-Enum")

    //enumStringHeader: EnumString,
@@ -206,6 +222,9 @@ structure ConstrainedHttpBoundShapesOperationInputOutput { 
    @httpQuery("lengthString")

    lengthStringQuery: LengthString,



    @httpQuery("rangeInteger")

    rangeIntegerQuery: RangeInteger,



    @httpQuery("enumString")

    enumStringQuery: EnumString,
@@ -217,6 +236,14 @@ structure ConstrainedHttpBoundShapesOperationInputOutput { 
    // @httpQuery("lengthStringSet")

    // lengthStringSetQuery: SetOfLengthString,



    @httpQuery("rangeIntegerList")

    rangeIntegerListQuery: ListOfRangeInteger,



    // 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("rangeIntegerSet")

    // rangeIntegerSetQuery: SetOfRangeInteger,



    @httpQuery("enumStringList")

    enumStringListQuery: ListOfEnumString,

}
@@ -332,6 +359,11 @@ structure ConA { 
    maxLengthString: MaxLengthString,

    fixedLengthString: FixedLengthString,



    rangeInteger: RangeInteger,

    minRangeInteger: MinRangeInteger,

    maxRangeInteger: MaxRangeInteger,

    fixedValueInteger: FixedValueInteger,



    conBList: ConBList,

    conBList2: ConBList2,
@@ -352,7 +384,13 @@ structure ConA { 
    // setOfLengthString: SetOfLengthString,

    mapOfLengthString: MapOfLengthString,



    nonStreamingBlob: NonStreamingBlob,

    listOfRangeInteger: ListOfRangeInteger,

    // 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.

    // setOfRangeInteger: SetOfRangeInteger,

    mapOfRangeInteger: MapOfRangeInteger,



    nonStreamingBlob: NonStreamingBlob



    patternString: PatternString,

    mapOfPatternString: MapOfPatternString,
@@ -374,6 +412,11 @@ map MapOfLengthString { 
    value: LengthString,

}



map MapOfRangeInteger {

    key: String,

    value: RangeInteger,

}



map MapOfEnumString {

    key: EnumString,

    value: EnumString,
@@ -407,6 +450,13 @@ map MapOfSetOfLengthString { 
    value: ListOfLengthString

}



// 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 {

//     key: LengthString,

//     value: SetOfRangeInteger,

// }



@length(min: 2, max: 8)

list LengthListOfLengthString {

    member: LengthString
@@ -418,7 +468,7 @@ string LengthString 
@length(min: 2)

string MinLengthString



@length(min: 69)

@length(max: 69)

string MaxLengthString



@length(min: 69, max: 69)
@@ -435,6 +485,18 @@ string LengthPatternString 
@length(min: 1, max: 69)

string MediaTypeLengthString



@range(min: -0, max: 69)

integer RangeInteger



@range(min: -10)

integer MinRangeInteger



@range(max: 69)

integer MaxRangeInteger



@range(min: 69, max: 69)

integer FixedValueInteger



/// A union with constrained members.

union ConstrainedUnion {

    enumString: EnumString,
@@ -480,6 +542,16 @@ list ListOfLengthString { 
    member: LengthString

}



// 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.

// set SetOfRangeInteger {

//     member: RangeInteger

// }



list ListOfRangeInteger {

    member: RangeInteger

}



list ListOfEnumString {

    member: EnumString

}

codegen-core/common-test-models/malformed-range-extras.smithy

0 → 100644
+662 −0

File added.

Preview size limit exceeded, changes collapsed.

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

+68 −15
Original line number Diff line number Diff line
@@ -15,6 +15,8 @@ import software.amazon.smithy.codegen.core.SymbolWriter.Factory 
import software.amazon.smithy.model.Model

import software.amazon.smithy.model.shapes.BooleanShape

import software.amazon.smithy.model.shapes.CollectionShape

import software.amazon.smithy.model.shapes.DoubleShape

import software.amazon.smithy.model.shapes.FloatShape

import software.amazon.smithy.model.shapes.NumberShape

import software.amazon.smithy.model.shapes.Shape

import software.amazon.smithy.model.shapes.ShapeId
@@ -22,6 +24,7 @@ import software.amazon.smithy.model.traits.DeprecatedTrait 
import software.amazon.smithy.model.traits.DocumentationTrait

import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType

import software.amazon.smithy.rust.codegen.core.smithy.isOptional

import software.amazon.smithy.rust.codegen.core.smithy.protocols.serialize.ValueExpression

import software.amazon.smithy.rust.codegen.core.smithy.rustType

import software.amazon.smithy.rust.codegen.core.util.getTrait

import software.amazon.smithy.rust.codegen.core.util.letIf
@@ -465,33 +468,82 @@ class RustWriter private constructor( 
    }



    /**

     * Generate a wrapping if statement around a field.

     *

     * - If the field is optional, it will only be called if the field is present

     * - If the field is an unboxed primitive, it will only be called if the field is non-zero

     *

     * Generate a wrapping if statement around a nullable value.

     * The provided code block will only be called if the value is not `None`.

     */

    fun ifSet(shape: Shape, member: Symbol, outerField: String, block: RustWriter.(field: String) -> Unit) {

    fun ifSome(member: Symbol, value: ValueExpression, block: RustWriter.(value: ValueExpression) -> Unit) {

        when {

            member.isOptional() -> {

                val derefName = safeName("inner")

                rustBlock("if let Some($derefName) = $outerField") {

                    block(derefName)

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

                rustBlock("if let Some(${innerValue.name}) = ${value.asRef()}") {

                    block(innerValue)

                }

            }



            else -> this.block(value)

        }

    }



            shape is NumberShape -> rustBlock("if ${outerField.removePrefix("&")} != 0") {

                block(outerField)

    /**

     * Generate a wrapping if statement around a primitive field.

     * The specified block will only be called if the field is not set to its default value - `0` for

     * numbers, `false` for booleans.

     */

    fun ifNotDefault(shape: Shape, variable: ValueExpression, block: RustWriter.(field: ValueExpression) -> Unit) {

        when (shape) {

            is FloatShape, is DoubleShape -> rustBlock("if ${variable.asValue()} != 0.0") {

                block(variable)

            }



            shape is BooleanShape -> rustBlock("if ${outerField.removePrefix("&")}") {

                block(outerField)

            is NumberShape -> rustBlock("if ${variable.asValue()} != 0") {

                block(variable)

            }



            else -> this.block(outerField)

            is BooleanShape -> rustBlock("if ${variable.asValue()}") {

                block(variable)

            }



            else -> rustBlock("") {

                this.block(variable)

            }

        }

    }



    /**

     * Generate a wrapping if statement around a field.

     *

     * - If the field is optional, it will only be called if the field is present

     * - If the field is an unboxed primitive, it will only be called if the field is non-zero

     *

     * # Example

     *

     * For a nullable structure shape (e.g. `Option<MyStruct>`), the following code will be generated:

     *

     * ```

     * if let Some(v) = my_nullable_struct {

     *   /* {block(variable)} */

     * }

     * ```

     *

     * # Example

     *

     * For a non-nullable integer shape, the following code will be generated:

     *

     * ```

     * if my_int != 0 {

     *   /* {block(variable)} */

     * }

     * ```

     */

    fun ifSet(

        shape: Shape,

        member: Symbol,

        variable: ValueExpression,

        block: RustWriter.(field: ValueExpression) -> Unit,

    ) {

        ifSome(member, variable) { inner -> ifNotDefault(shape, inner, block) }

    }



    fun listForEach(

        target: Shape,

        outerField: String,
@@ -550,7 +602,8 @@ class RustWriter private constructor( 
    inner class RustWriteableInjector : BiFunction<Any, String, String> {

        override fun apply(t: Any, u: String): String {

            @Suppress("UNCHECKED_CAST")

            val func = t as? Writable ?: throw CodegenException("RustWriteableInjector.apply choked on non-function t ($t)")

            val func =

                t as? Writable ?: throw CodegenException("RustWriteableInjector.apply choked on non-function t ($t)")

            val innerWriter = RustWriter(filename, namespace, printWarning = false)

            func(innerWriter)

            innerWriter.dependencies.forEach { addDependency(it) }

codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/error/ErrorGenerator.kt

+3 −2
Original line number Diff line number Diff line
@@ -25,6 +25,7 @@ import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType.Companion.Std 
import software.amazon.smithy.rust.codegen.core.smithy.RustSymbolProvider

import software.amazon.smithy.rust.codegen.core.smithy.isOptional

import software.amazon.smithy.rust.codegen.core.smithy.mapRustType

import software.amazon.smithy.rust.codegen.core.smithy.protocols.serialize.ValueExpression

import software.amazon.smithy.rust.codegen.core.smithy.rustType

import software.amazon.smithy.rust.codegen.core.util.REDACTION

import software.amazon.smithy.rust.codegen.core.util.dq
@@ -144,8 +145,8 @@ class ErrorGenerator( 
                    if (it.shouldRedact(model)) {

                        write("""write!(f, ": {}", $REDACTION)?;""")

                    } else {

                        ifSet(it, symbolProvider.toSymbol(it), "&self.message") { field ->

                            write("""write!(f, ": {}", $field)?;""")

                        ifSet(it, symbolProvider.toSymbol(it), ValueExpression.Reference("&self.message")) { field ->

                            write("""write!(f, ": {}", ${field.asRef()})?;""")

                        }

                    }

                }