Skip to content
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 @pattern on string shapes #1998

Merged
merged 64 commits into from
Nov 23, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
64 commits
Select commit Hold shift + click to select a range
c45d0b0
Refactor `ConstrainedStringGenerator` to extract length checking
jjant Nov 16, 2022
bc29310
Extract `TryFrom` rendering into its own function
jjant Nov 17, 2022
44495b9
Extract `ConstraintViolation` definition to separate function
jjant Nov 17, 2022
e140694
Add pattern check mock for `@pattern` trait
jjant Nov 17, 2022
ae21ce7
Add `regex` to list of server dependencies
jjant Nov 17, 2022
4c6384d
Use real regex check in `check_pattern`
jjant Nov 17, 2022
c3e9659
Add `@pattern` to traits that make `string` shapes directly constrained
jjant Nov 17, 2022
b950c19
Remove unsupported validation for `@pattern` on strings
jjant Nov 17, 2022
b9206bf
Fix test cases in `ConstraintsTest`
jjant Nov 17, 2022
1f3d763
Remove test disallowing `@pattern` on strings
jjant Nov 17, 2022
838485b
Add first test case for `@pattern` on strings
jjant Nov 17, 2022
1aa2573
Fix codegen in `ConstraintViolation::as_validation_exception_field`
jjant Nov 17, 2022
d4de234
Use `OnceCell` to store `@pattern`'s regex
jjant Nov 17, 2022
be24407
Tidy up `ConstrainedStringGenerator` to work with many constraints
jjant Nov 17, 2022
77637fd
Rename `handleTrait` -> `fromTrait` and make it a static method
jjant Nov 17, 2022
aae0b75
Add docs for `ConstraintViolation` variants
jjant Nov 17, 2022
116e9de
Fix unescaped slashes in regexes
jjant Nov 17, 2022
55df743
Quick hack to get tests compiling
jjant Nov 17, 2022
67863ca
Fix `sensitive_string_display_implementation` test
jjant Nov 18, 2022
b4331cc
Merge branch 'main' into jjant/implement-length-trait-on-strings
jjant Nov 18, 2022
96ea2c7
Use new fn name: `asType` -> `toType`
jjant Nov 18, 2022
275e2a8
Refactor `ConstrainedStringGenerator` to not pass in `constraints`
jjant Nov 18, 2022
4ffd550
Add `@pattern` test
jjant Nov 18, 2022
0797f4d
Use `Lazy` instead of `OnceCell`
jjant Nov 18, 2022
22fd18f
Store `&'static str` in `Pattern` error instead of `String`
jjant Nov 18, 2022
65139bd
Move `TraitInfo` to the bottom
jjant Nov 18, 2022
d26a877
Extract branches in `TraitInfo.fromTrait` into separate functions
jjant Nov 18, 2022
0d7bbee
Add `PatternTrait.validationErrorMessage`
jjant Nov 18, 2022
85ddc4c
Add more tests for `@pattern`
jjant Nov 18, 2022
e8b47a2
Merge branch 'main' into jjant/implement-length-trait-on-strings
jjant Nov 18, 2022
a6ad449
Add entry to `CHANGELOG.next.toml`
jjant Nov 18, 2022
8ade3a1
Fix a `@length` + `@pattern` test case
jjant Nov 18, 2022
961e3af
Remove unused binding in `ConstrainedStringGeneratorTest`
jjant Nov 21, 2022
8a1f73f
Remove calls to `trimIndent`
jjant Nov 21, 2022
1f02303
Use raw Rust strings instead of `escapedPattern`
jjant Nov 21, 2022
dcd2528
Require `regex 1.5.5` instead of `1.7.0`
jjant Nov 21, 2022
d90f447
Use `Writable`s in `TraitInfo`
jjant Nov 21, 2022
78d6512
Use single `rust` call for `impl $name` block
jjant Nov 21, 2022
4234b6d
Move `supportedStringConstraintTraits` to `Constraints.kt`
jjant Nov 21, 2022
abaf910
Fix error message mentioning wrong `ignoreUnsupportedConstraintTraits…
jjant Nov 21, 2022
46b126b
Fix usage of `:W` in `rustTemplate` and raw Rust strings
jjant Nov 21, 2022
d551168
Use shorthand form for `PatternTrait.validationErrorMessage`
jjant Nov 21, 2022
76e5984
Fix protocol tests by adjusting the validation message
jjant Nov 21, 2022
227ddaf
Add uses of `@pattern` in `constraints.smithy` model
jjant Nov 21, 2022
147af6b
Merge branch 'main' into jjant/implement-length-trait-on-strings
jjant Nov 21, 2022
e89850f
Fix broken `RestJsonMalformedPatternReDOSString` test
jjant Nov 21, 2022
995bf0f
Move `TraitInfo` to its own module and separate string-specific details
jjant Nov 21, 2022
3e45b9e
Update codegen-core/common-test-models/constraints.smithy
jjant Nov 22, 2022
309423f
Fix nits in `constraints.smithy`
jjant Nov 22, 2022
7a95fe2
Add license to `TraitInfo.kt`
jjant Nov 22, 2022
7f5e68f
Merge branch 'jjant/implement-length-trait-on-strings' of github.com:…
jjant Nov 22, 2022
bbb1b81
Make `StringTraitInfo.fromTrait` not return a nullable
jjant Nov 22, 2022
fdcd2b7
Remove overzealous formatting
jjant Nov 22, 2022
4a06c77
Add some padding to json in `fixRestJsonMalformedPatternReDOSString`
jjant Nov 22, 2022
750e49d
Add `compile_regex` function for `@pattern` strings
jjant Nov 22, 2022
11ee326
Add helpful error message when regexes fail to compile
jjant Nov 22, 2022
cb4d4ee
Add missing coverage in `constraints.smithy`
jjant Nov 22, 2022
be4a85a
Fix examples in `constraints.smithy`
jjant Nov 23, 2022
105112a
Fix failing test
jjant Nov 23, 2022
0d2b6b6
Combine writables in `ConstrainedStringGenerator`
jjant Nov 23, 2022
3848d34
Fix uses of `Writable.join`
jjant Nov 23, 2022
fdf88b0
Use `expect` over `unwrap_or_else`
jjant Nov 23, 2022
53cf34c
Use `once_cell::sync::Lazy` over `once_cell::sync::OnceCell`
jjant Nov 23, 2022
7a2576c
Merge branch 'main' into jjant/implement-length-trait-on-strings
jjant Nov 23, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.next.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down
113 changes: 112 additions & 1 deletion codegen-core/common-test-models/constraints.smithy
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,12 @@ service ConstraintsService {
QueryParamsTargetingMapOfListOfLengthStringOperation,
QueryParamsTargetingMapOfSetOfLengthStringOperation,
QueryParamsTargetingMapOfListOfEnumStringOperation,

jjant marked this conversation as resolved.
Show resolved Hide resolved
QueryParamsTargetingMapOfPatternStringOperation,
QueryParamsTargetingMapOfListOfPatternStringOperation,
QueryParamsTargetingMapOfLengthPatternStringOperation,
QueryParamsTargetingMapOfListOfLengthPatternStringOperation,

HttpPrefixHeadersTargetingLengthMapOperation,
// TODO(https://github.com/awslabs/smithy-rs/issues/1431)
// HttpPrefixHeadersTargetingMapOfEnumStringOperation,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -383,6 +468,14 @@ set SetOfLengthString {
member: LengthString
}

set SetOfPatternString {
member: PatternString
}

set SetOfLengthPatternString {
member: LengthPatternString
}

list ListOfLengthString {
member: LengthString
}
Expand All @@ -391,6 +484,14 @@ list ListOfEnumString {
member: EnumString
}

list ListOfPatternString {
member: PatternString
}

list ListOfLengthPatternString {
member: LengthPatternString
}

structure ConB {
@required
nice: String,
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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:
*
Expand All @@ -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
}

Expand All @@ -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
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/*
* 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: {}"
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import software.amazon.smithy.model.shapes.ServiceShape
import software.amazon.smithy.model.shapes.SetShape
import software.amazon.smithy.model.shapes.Shape
import software.amazon.smithy.model.shapes.ShapeId
import software.amazon.smithy.model.shapes.StringShape
import software.amazon.smithy.model.shapes.UnionShape
import software.amazon.smithy.model.traits.EnumTrait
import software.amazon.smithy.model.traits.LengthTrait
Expand All @@ -42,7 +41,7 @@ private sealed class UnsupportedConstraintMessageKind {
This is not supported in the smithy-rs server SDK.
${ if (willSupport) "It will be supported in the future." else "" }
See the tracking issue ($trackingIssue).
If you want to go ahead and generate the server SDK ignoring unsupported constraint traits, set the key `ignoreUnsupportedConstraintTraits`
If you want to go ahead and generate the server SDK ignoring unsupported constraint traits, set the key `ignoreUnsupportedConstraints`
inside the `runtimeConfig.codegenConfig` JSON object in your `smithy-build.json` to `true`.
""".trimIndent().replace("\n", " ")

Expand Down Expand Up @@ -86,10 +85,6 @@ private sealed class UnsupportedConstraintMessageKind {
level,
buildMessageShapeHasUnsupportedConstraintTrait(shape, lengthTrait, constraintTraitsUberIssue),
)
is UnsupportedPatternTraitOnStringShape -> LogMessage(
level,
buildMessageShapeHasUnsupportedConstraintTrait(shape, patternTrait, constraintTraitsUberIssue),
)
is UnsupportedRangeTraitOnShape -> LogMessage(
level,
buildMessageShapeHasUnsupportedConstraintTrait(shape, rangeTrait, constraintTraitsUberIssue),
Expand All @@ -106,7 +101,6 @@ private data class UnsupportedConstraintOnMemberShape(val shape: MemberShape, va
private data class UnsupportedConstraintOnShapeReachableViaAnEventStream(val shape: Shape, val constraintTrait: Trait) : UnsupportedConstraintMessageKind()
private data class UnsupportedLengthTraitOnStreamingBlobShape(val shape: BlobShape, val lengthTrait: LengthTrait, val streamingTrait: StreamingTrait) : UnsupportedConstraintMessageKind()
private data class UnsupportedLengthTraitOnCollectionOrOnBlobShape(val shape: Shape, val lengthTrait: LengthTrait) : UnsupportedConstraintMessageKind()
private data class UnsupportedPatternTraitOnStringShape(val shape: Shape, val patternTrait: PatternTrait) : UnsupportedConstraintMessageKind()
private data class UnsupportedRangeTraitOnShape(val shape: Shape, val rangeTrait: RangeTrait) : UnsupportedConstraintMessageKind()
private data class UnsupportedUniqueItemsTraitOnShape(val shape: Shape, val uniqueItemsTrait: UniqueItemsTrait) : UnsupportedConstraintMessageKind()

Expand Down Expand Up @@ -146,20 +140,20 @@ fun validateOperationsWithConstrainedInputHaveValidationExceptionAttached(model:
LogMessage(
Level.SEVERE,
"""
Operation ${it.shape.id} takes in input that is constrained
(https://awslabs.github.io/smithy/2.0/spec/constraint-traits.html), and as such can fail with a validation
Operation ${it.shape.id} takes in input that is constrained
(https://awslabs.github.io/smithy/2.0/spec/constraint-traits.html), and as such can fail with a validation
exception. You must model this behavior in the operation shape in your model file.
""".trimIndent().replace("\n", "") +
"""
```smithy
use smithy.framework#ValidationException
operation ${it.shape.id.name} {
...
errors: [..., ValidationException] // <-- Add this.
}
```

```smithy
use smithy.framework#ValidationException

operation ${it.shape.id.name} {
...
errors: [..., ValidationException] // <-- Add this.
}
```
""".trimIndent(),
)
}
Expand Down Expand Up @@ -214,17 +208,7 @@ fun validateUnsupportedConstraints(model: Model, service: ServiceShape, codegenC
.map { UnsupportedLengthTraitOnCollectionOrOnBlobShape(it, it.expectTrait()) }
.toSet()

// 5. Pattern trait on string shapes is used. It has not been implemented yet.
// TODO(https://github.com/awslabs/smithy-rs/issues/1401)
val unsupportedPatternTraitOnStringShapeSet = walker
.walkShapes(service)
.asSequence()
.filterIsInstance<StringShape>()
.filterMapShapesToTraits(setOf(PatternTrait::class.java))
.map { (shape, patternTrait) -> UnsupportedPatternTraitOnStringShape(shape, patternTrait as PatternTrait) }
.toSet()

// 6. Range trait on any shape is used. It has not been implemented yet.
// 5. Range trait on any shape is used. It has not been implemented yet.
// TODO(https://github.com/awslabs/smithy-rs/issues/1401)
val unsupportedRangeTraitOnShapeSet = walker
.walkShapes(service)
Expand All @@ -247,7 +231,6 @@ fun validateUnsupportedConstraints(model: Model, service: ServiceShape, codegenC
unsupportedLengthTraitOnStreamingBlobShapeSet.map { it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints) } +
unsupportedConstraintOnShapeReachableViaAnEventStreamSet.map { it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints) } +
unsupportedLengthTraitOnCollectionOrOnBlobShapeSet.map { it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints) } +
unsupportedPatternTraitOnStringShapeSet.map { it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints) } +
unsupportedRangeTraitOnShapeSet.map { it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints) } +
unsupportedUniqueItemsTraitOnShapeSet.map { it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints) }

Expand Down
Loading