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

Implement `@pattern` on `string` shapes (#1998) 



* Refactor `ConstrainedStringGenerator` to extract length checking

* Extract `TryFrom` rendering into its own function

* Extract `ConstraintViolation` definition to separate function

* Add pattern check mock for `@pattern` trait

* Add `regex` to list of server dependencies

* Use real regex check in `check_pattern`

* Add `@pattern` to traits that make `string` shapes directly constrained

* Remove unsupported validation for `@pattern` on strings

* Fix test cases in `ConstraintsTest`

* Remove test disallowing `@pattern` on strings

* Add first test case for `@pattern` on strings

* Fix codegen in `ConstraintViolation::as_validation_exception_field`

* Use `OnceCell` to store `@pattern`'s regex

* Tidy up `ConstrainedStringGenerator` to work with many constraints

* Rename `handleTrait` -> `fromTrait` and make it a static method

* Add docs for `ConstraintViolation` variants

* Fix unescaped slashes in regexes

* Quick hack to get tests compiling

* Fix `sensitive_string_display_implementation` test

* Use new fn name: `asType` -> `toType`

* Refactor `ConstrainedStringGenerator` to not pass in `constraints`

* Add `@pattern` test

* Use `Lazy` instead of `OnceCell`

* Store `&'static str` in `Pattern` error instead of `String`

* Move `TraitInfo` to the bottom

* Extract branches in `TraitInfo.fromTrait` into separate functions

* Add `PatternTrait.validationErrorMessage`

* Add more tests for `@pattern`

* Add entry to `CHANGELOG.next.toml`

* Fix a `@length` + `@pattern` test case

* Remove unused binding in `ConstrainedStringGeneratorTest`

* Remove calls to `trimIndent`

* Use raw Rust strings instead of `escapedPattern`

* Require `regex 1.5.5` instead of `1.7.0`

* Use `Writable`s in `TraitInfo`

* Use single `rust` call for `impl $name` block

* Move `supportedStringConstraintTraits` to `Constraints.kt`

* Fix error message mentioning wrong `ignoreUnsupportedConstraintTraits` key

* Fix usage of `:W` in `rustTemplate` and raw Rust strings

* Use shorthand form for `PatternTrait.validationErrorMessage`

* Fix protocol tests by adjusting the validation message

* Add uses of `@pattern` in `constraints.smithy` model

* Fix broken `RestJsonMalformedPatternReDOSString` test

* Move `TraitInfo` to its own module and separate string-specific details

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

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

* Fix nits in `constraints.smithy`

* Add license to `TraitInfo.kt`

* Make `StringTraitInfo.fromTrait` not return a nullable

* Remove overzealous formatting

* Add some padding to json in `fixRestJsonMalformedPatternReDOSString`

* Add `compile_regex` function for `@pattern` strings

* Add helpful error message when regexes fail to compile

* Add missing coverage in `constraints.smithy`

* Fix examples in `constraints.smithy`

* Fix failing test

* Combine writables in `ConstrainedStringGenerator`

* Fix uses of `Writable.join`

* Use `expect` over `unwrap_or_else`

* Use `once_cell::sync::Lazy` over `once_cell::sync::OnceCell`

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

CHANGELOG.next.toml

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


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

* The `length` trait on `map` 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"]

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

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

author = "david-perez"

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

+112 −1
Original line number Diff line number Diff line
@@ -23,6 +23,12 @@ service ConstraintsService { 
        QueryParamsTargetingMapOfListOfLengthStringOperation,

        QueryParamsTargetingMapOfSetOfLengthStringOperation,

        QueryParamsTargetingMapOfListOfEnumStringOperation,



        QueryParamsTargetingMapOfPatternStringOperation,

        QueryParamsTargetingMapOfListOfPatternStringOperation,

        QueryParamsTargetingMapOfLengthPatternStringOperation,

        QueryParamsTargetingMapOfListOfLengthPatternStringOperation,



        HttpPrefixHeadersTargetingLengthMapOperation,

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

        // HttpPrefixHeadersTargetingMapOfEnumStringOperation,
@@ -97,6 +103,34 @@ operation QueryParamsTargetingMapOfListOfEnumStringOperation { 
    errors: [ValidationException]

}



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

operation QueryParamsTargetingMapOfPatternStringOperation {

    input: QueryParamsTargetingMapOfPatternStringOperationInputOutput,

    output: QueryParamsTargetingMapOfPatternStringOperationInputOutput,

    errors: [ValidationException]

}



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

operation QueryParamsTargetingMapOfListOfPatternStringOperation {

    input: QueryParamsTargetingMapOfListOfPatternStringOperationInputOutput,

    output: QueryParamsTargetingMapOfListOfPatternStringOperationInputOutput,

    errors: [ValidationException]

}



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

operation QueryParamsTargetingMapOfLengthPatternStringOperation {

    input: QueryParamsTargetingMapOfLengthPatternStringOperationInputOutput,

    output: QueryParamsTargetingMapOfLengthPatternStringOperationInputOutput,

    errors: [ValidationException],

}



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

operation QueryParamsTargetingMapOfListOfLengthPatternStringOperation {

    input: QueryParamsTargetingMapOfListOfLengthPatternStringOperationInputOutput,

    output: QueryParamsTargetingMapOfListOfLengthPatternStringOperationInputOutput,

    errors: [ValidationException]

}



@http(uri: "/http-prefix-headers-targeting-length-map-operation", method: "POST")

operation HttpPrefixHeadersTargetingLengthMapOperation {

    input: HttpPrefixHeadersTargetingLengthMapOperationInputOutput,
@@ -187,6 +221,26 @@ structure ConstrainedHttpBoundShapesOperationInputOutput { 
    enumStringListQuery: ListOfEnumString,

}



structure QueryParamsTargetingMapOfPatternStringOperationInputOutput {

    @httpQueryParams

    mapOfPatternString: MapOfPatternString

}



structure QueryParamsTargetingMapOfListOfPatternStringOperationInputOutput {

    @httpQueryParams

    mapOfListOfPatternString: MapOfListOfPatternString

}



structure QueryParamsTargetingMapOfLengthPatternStringOperationInputOutput {

    @httpQueryParams

    mapOfLengthPatternString: MapOfLengthPatternString,

}



structure QueryParamsTargetingMapOfListOfLengthPatternStringOperationInputOutput {

    @httpQueryParams

    mapOfLengthPatternString: MapOfListOfLengthPatternString,

}



structure HttpPrefixHeadersTargetingLengthMapOperationInputOutput {

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

    lengthMap: ConBMap,
@@ -298,7 +352,21 @@ structure ConA { 
    // setOfLengthString: SetOfLengthString,

    mapOfLengthString: MapOfLengthString,



    nonStreamingBlob: NonStreamingBlob

    nonStreamingBlob: NonStreamingBlob,



    patternString: PatternString,

    mapOfPatternString: MapOfPatternString,

    listOfPatternString: ListOfPatternString,

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

    // setOfPatternString: SetOfPatternString,



    lengthLengthPatternString: LengthPatternString,

    mapOfLengthPatternString: MapOfLengthPatternString,

    listOfLengthPatternString: ListOfLengthPatternString

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

}



map MapOfLengthString {
@@ -321,6 +389,16 @@ map MapOfListOfEnumString { 
    value: ListOfEnumString,

}



map MapOfListOfPatternString {

    key: PatternString,

    value: ListOfPatternString

}



map MapOfListOfLengthPatternString {

    key: LengthPatternString,

    value: ListOfLengthPatternString

}



map MapOfSetOfLengthString {

    key: LengthString,

    // TODO(https://github.com/awslabs/smithy-rs/issues/1401): a `set` shape is
@@ -346,6 +424,13 @@ string MaxLengthString 
@length(min: 69, max: 69)

string FixedLengthString



@pattern("[a-d]{5}")

string PatternString



@pattern("[a-f0-5]*")

@length(min: 5, max: 10)

string LengthPatternString



@mediaType("video/quicktime")

@length(min: 1, max: 69)

string MediaTypeLengthString
@@ -383,6 +468,14 @@ set SetOfLengthString { 
    member: LengthString

}



set SetOfPatternString {

    member: PatternString

}



set SetOfLengthPatternString {

    member: LengthPatternString

}



list ListOfLengthString {

    member: LengthString

}
@@ -391,6 +484,14 @@ list ListOfEnumString { 
    member: EnumString

}



list ListOfPatternString {

    member: PatternString

}



list ListOfLengthPatternString {

    member: LengthPatternString

}



structure ConB {

    @required

    nice: String,
@@ -443,6 +544,16 @@ list NestedList { 
//     member: String

// }



map MapOfPatternString {

    key: PatternString,

    value: PatternString,

}



map MapOfLengthPatternString {

    key: LengthPatternString,

    value: LengthPatternString,

}



@length(min: 1, max: 69)

map ConBMap {

    key: String,

codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/Constraints.kt

+5 −2
Original line number Diff line number Diff line
@@ -21,6 +21,7 @@ import software.amazon.smithy.model.traits.LengthTrait 
import software.amazon.smithy.model.traits.PatternTrait

import software.amazon.smithy.model.traits.RangeTrait

import software.amazon.smithy.model.traits.RequiredTrait

import software.amazon.smithy.model.traits.Trait

import software.amazon.smithy.model.traits.UniqueItemsTrait

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

import software.amazon.smithy.rust.codegen.core.util.UNREACHABLE
@@ -42,6 +43,8 @@ fun Shape.hasConstraintTrait() = 
        hasTrait<RangeTrait>() ||

        hasTrait<RequiredTrait>()



val supportedStringConstraintTraits: List<Class<out Trait>> = listOf(LengthTrait::class.java, PatternTrait::class.java)



/**

 * We say a shape is _directly_ constrained if:

 *
@@ -66,7 +69,7 @@ fun Shape.isDirectlyConstrained(symbolProvider: SymbolProvider): Boolean = when 
        this.members().map { symbolProvider.toSymbol(it) }.any { !it.isOptional() }

    }

    is MapShape -> this.hasTrait<LengthTrait>()

    is StringShape -> this.hasTrait<EnumTrait>() || this.hasTrait<LengthTrait>()

    is StringShape -> this.hasTrait<EnumTrait>() || supportedStringConstraintTraits.any { this.hasTrait(it) }

    else -> false

}
@@ -91,7 +94,7 @@ fun MemberShape.targetCanReachConstrainedShape(model: Model, symbolProvider: Sym 


fun Shape.hasPublicConstrainedWrapperTupleType(model: Model, publicConstrainedTypes: Boolean): Boolean = when (this) {

    is MapShape -> publicConstrainedTypes && this.hasTrait<LengthTrait>()

    is StringShape -> !this.hasTrait<EnumTrait>() && (publicConstrainedTypes && this.hasTrait<LengthTrait>())

    is StringShape -> !this.hasTrait<EnumTrait>() && (publicConstrainedTypes && supportedStringConstraintTraits.any(this::hasTrait))

    is MemberShape -> model.expectShape(this.target).hasPublicConstrainedWrapperTupleType(model, publicConstrainedTypes)

    else -> false

}

codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/PatternTraitValidationErrorMessage.kt

0 → 100644
+12 −0
Original line number Diff line number Diff line 
/*

 * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.

 * SPDX-License-Identifier: Apache-2.0

 */



package software.amazon.smithy.rust.codegen.server.smithy



import software.amazon.smithy.model.traits.PatternTrait



@Suppress("UnusedReceiverParameter")

fun PatternTrait.validationErrorMessage(): String =

    "Value {} at '{}' failed to satisfy constraint: Member must satisfy regular expression pattern: {}"

codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ServerCargoDependency.kt

+1 −0
Original line number Diff line number Diff line
@@ -25,6 +25,7 @@ object ServerCargoDependency { 
    val PinProjectLite: CargoDependency = CargoDependency("pin-project-lite", CratesIo("0.2"))

    val Tower: CargoDependency = CargoDependency("tower", CratesIo("0.4"))

    val TokioDev: CargoDependency = CargoDependency("tokio", CratesIo("1.8.4"), scope = DependencyScope.Dev)

    val Regex: CargoDependency = CargoDependency("regex", CratesIo("1.5.5"))



    fun SmithyHttpServer(runtimeConfig: RuntimeConfig) = runtimeConfig.runtimeCrate("http-server")

}