diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index f0a3bf3c70..469b82d380 100644 --- a/CHANGELOG.next.toml +++ b/CHANGELOG.next.toml @@ -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" diff --git a/codegen-core/common-test-models/constraints.smithy b/codegen-core/common-test-models/constraints.smithy index ddfe920cdd..0bdfe6d2a1 100644 --- a/codegen-core/common-test-models/constraints.smithy +++ b/codegen-core/common-test-models/constraints.smithy @@ -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, diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/Constraints.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/Constraints.kt index 82102be18a..9acabf7532 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/Constraints.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/Constraints.kt @@ -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() || hasTrait() +val supportedStringConstraintTraits: List> = 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() - is StringShape -> this.hasTrait() || this.hasTrait() + is StringShape -> this.hasTrait() || 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() - is StringShape -> !this.hasTrait() && (publicConstrainedTypes && this.hasTrait()) + is StringShape -> !this.hasTrait() && (publicConstrainedTypes && supportedStringConstraintTraits.any(this::hasTrait)) is MemberShape -> model.expectShape(this.target).hasPublicConstrainedWrapperTupleType(model, publicConstrainedTypes) else -> false } diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/PatternTraitValidationErrorMessage.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/PatternTraitValidationErrorMessage.kt new file mode 100644 index 0000000000..8bb3cb648e --- /dev/null +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/PatternTraitValidationErrorMessage.kt @@ -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: {}" diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ServerCargoDependency.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ServerCargoDependency.kt index 6058519abf..a5735b81d8 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ServerCargoDependency.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ServerCargoDependency.kt @@ -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") } diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraints.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraints.kt index 871a8d364c..4535616bff 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraints.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraints.kt @@ -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 @@ -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", " ") @@ -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), @@ -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() @@ -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(), ) } @@ -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() - .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) @@ -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) } diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedStringGenerator.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedStringGenerator.kt index a63801cce4..c0ef3c043c 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedStringGenerator.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedStringGenerator.kt @@ -5,26 +5,33 @@ package software.amazon.smithy.rust.codegen.server.smithy.generators +import software.amazon.smithy.codegen.core.Symbol import software.amazon.smithy.model.shapes.StringShape import software.amazon.smithy.model.traits.LengthTrait +import software.amazon.smithy.model.traits.PatternTrait +import software.amazon.smithy.model.traits.Trait import software.amazon.smithy.rust.codegen.core.rustlang.Attribute import software.amazon.smithy.rust.codegen.core.rustlang.RustMetadata import software.amazon.smithy.rust.codegen.core.rustlang.RustModule import software.amazon.smithy.rust.codegen.core.rustlang.RustType import software.amazon.smithy.rust.codegen.core.rustlang.RustWriter import software.amazon.smithy.rust.codegen.core.rustlang.Visibility +import software.amazon.smithy.rust.codegen.core.rustlang.Writable +import software.amazon.smithy.rust.codegen.core.rustlang.docs import software.amazon.smithy.rust.codegen.core.rustlang.documentShape +import software.amazon.smithy.rust.codegen.core.rustlang.join import software.amazon.smithy.rust.codegen.core.rustlang.render import software.amazon.smithy.rust.codegen.core.rustlang.rust -import software.amazon.smithy.rust.codegen.core.rustlang.rustBlock -import software.amazon.smithy.rust.codegen.core.rustlang.rustBlockTemplate import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType import software.amazon.smithy.rust.codegen.core.smithy.makeMaybeConstrained -import software.amazon.smithy.rust.codegen.core.util.expectTrait +import software.amazon.smithy.rust.codegen.core.util.PANIC +import software.amazon.smithy.rust.codegen.core.util.orNull import software.amazon.smithy.rust.codegen.core.util.redactIfNecessary import software.amazon.smithy.rust.codegen.server.smithy.PubCrateConstraintViolationSymbolProvider +import software.amazon.smithy.rust.codegen.server.smithy.ServerCargoDependency import software.amazon.smithy.rust.codegen.server.smithy.ServerCodegenContext +import software.amazon.smithy.rust.codegen.server.smithy.supportedStringConstraintTraits import software.amazon.smithy.rust.codegen.server.smithy.traits.isReachableFromOperationInput import software.amazon.smithy.rust.codegen.server.smithy.validationErrorMessage @@ -49,23 +56,47 @@ class ConstrainedStringGenerator( PubCrateConstraintViolationSymbolProvider(this) } } + private val constraintsInfo: List = + supportedStringConstraintTraits + .mapNotNull { shape.getTrait(it).orNull() } + .map(StringTraitInfo::fromTrait) + .map(StringTraitInfo::toTraitInfo) - fun render() { - val lengthTrait = shape.expectTrait() + private fun renderTryFrom(inner: String, name: String, constraintViolation: Symbol) { + writer.rustTemplate( + """ + impl $name { + #{ValidationFunctions:W} + } + """, + "ValidationFunctions" to constraintsInfo.map { it.validationFunctionDefinition(constraintViolation) }.join("\n"), + ) + + writer.rustTemplate( + """ + impl #{TryFrom}<$inner> for $name { + type Error = #{ConstraintViolation}; + + /// ${rustDocsTryFromMethod(name, inner)} + fn try_from(value: $inner) -> Result { + #{TryFromChecks:W} + Ok(Self(value)) + } + } + """, + "TryFrom" to RuntimeType.TryFrom, + "ConstraintViolation" to constraintViolation, + "TryFromChecks" to constraintsInfo.map { it.tryFromCheck }.join("\n"), + ) + } + + fun render() { val symbol = constrainedShapeSymbolProvider.toSymbol(shape) val name = symbol.name val inner = RustType.String.render() val constraintViolation = constraintViolationSymbolProvider.toSymbol(shape) - val condition = if (lengthTrait.min.isPresent && lengthTrait.max.isPresent) { - "(${lengthTrait.min.get()}..=${lengthTrait.max.get()}).contains(&length)" - } else if (lengthTrait.min.isPresent) { - "${lengthTrait.min.get()} <= length" - } else { - "length <= ${lengthTrait.max.get()}" - } - val constrainedTypeVisibility = if (publicConstrainedTypes) { Visibility.PUBLIC } else { @@ -85,55 +116,47 @@ class ConstrainedStringGenerator( if (constrainedTypeVisibility == Visibility.PUBCRATE) { Attribute.AllowUnused.render(writer) } - writer.rustTemplate( + writer.rust( """ impl $name { /// Extracts a string slice containing the entire underlying `String`. pub fn as_str(&self) -> &str { &self.0 } - + /// ${rustDocsInnerMethod(inner)} pub fn inner(&self) -> &$inner { &self.0 } - + /// ${rustDocsIntoInnerMethod(inner)} pub fn into_inner(self) -> $inner { self.0 } - } - + }""", + ) + + renderTryFrom(inner, name, constraintViolation) + + writer.rustTemplate( + """ impl #{ConstrainedTrait} for $name { type Unconstrained = $inner; } - + impl #{From}<$inner> for #{MaybeConstrained} { fn from(value: $inner) -> Self { Self::Unconstrained(value) } } - + impl #{Display} for $name { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { ${shape.redactIfNecessary(model, "self.0")}.fmt(f) } } - - impl #{TryFrom}<$inner> for $name { - type Error = #{ConstraintViolation}; - - /// ${rustDocsTryFromMethod(name, inner)} - fn try_from(value: $inner) -> Result { - let length = value.chars().count(); - if $condition { - Ok(Self(value)) - } else { - Err(#{ConstraintViolation}::Length(length)) - } - } - } - + + impl #{From}<$name> for $inner { fn from(value: $name) -> Self { value.into_inner() @@ -145,39 +168,167 @@ class ConstrainedStringGenerator( "MaybeConstrained" to symbol.makeMaybeConstrained(), "Display" to RuntimeType.Display, "From" to RuntimeType.From, - "TryFrom" to RuntimeType.TryFrom, ) val constraintViolationModuleName = constraintViolation.namespace.split(constraintViolation.namespaceDelimiter).last() writer.withModule(RustModule(constraintViolationModuleName, RustMetadata(visibility = constrainedTypeVisibility))) { + renderConstraintViolationEnum(this, shape, constraintViolation) + } + } + + private fun renderConstraintViolationEnum(writer: RustWriter, shape: StringShape, constraintViolation: Symbol) { + writer.rustTemplate( + """ + ##[derive(Debug, PartialEq)] + pub enum ${constraintViolation.name} { + #{Variants:W} + } + """, + "Variants" to constraintsInfo.map { it.constraintViolationVariant }.join(",\n"), + ) + + if (shape.isReachableFromOperationInput()) { + writer.rustTemplate( + """ + impl ${constraintViolation.name} { + pub(crate) fn as_validation_exception_field(self, path: #{String}) -> crate::model::ValidationExceptionField { + match self { + #{ValidationExceptionFields:W} + } + } + } + """, + "String" to RuntimeType.String, + "ValidationExceptionFields" to constraintsInfo.map { it.asValidationExceptionField }.join("\n"), + ) + } + } +} +private data class Length(val lengthTrait: LengthTrait) : StringTraitInfo() { + override fun toTraitInfo(): TraitInfo { + return TraitInfo( + { rust("Self::check_length(&value)?;") }, + { + docs("Error when a string doesn't satisfy its `@length` requirements.") + rust("Length(usize)") + }, + { + rust( + """ + Self::Length(length) => crate::model::ValidationExceptionField { + message: format!("${lengthTrait.validationErrorMessage()}", length, &path), + path, + }, + """, + ) + }, + this::renderValidationFunction, + ) + } + + /** + * Renders a `check_length` function to validate the string matches the + * required length indicated by the `@length` trait. + */ + private fun renderValidationFunction(constraintViolation: Symbol): Writable { + return { + val condition = if (lengthTrait.min.isPresent && lengthTrait.max.isPresent) { + "(${lengthTrait.min.get()}..=${lengthTrait.max.get()}).contains(&length)" + } else if (lengthTrait.min.isPresent) { + "${lengthTrait.min.get()} <= length" + } else { + "length <= ${lengthTrait.max.get()}" + } + rust( """ - ##[derive(Debug, PartialEq)] - pub enum ${constraintViolation.name} { - Length(usize), + fn check_length(string: &str) -> Result<(), $constraintViolation> { + let length = string.chars().count(); + + if $condition { + Ok(()) + } else { + Err($constraintViolation::Length(length)) + } } """, ) + } + } +} - if (shape.isReachableFromOperationInput()) { - rustBlock("impl ${constraintViolation.name}") { - rustBlockTemplate( - "pub(crate) fn as_validation_exception_field(self, path: #{String}) -> crate::model::ValidationExceptionField", - "String" to RuntimeType.String, - ) { - rustBlock("match self") { - rust( - """ - Self::Length(length) => crate::model::ValidationExceptionField { - message: format!("${lengthTrait.validationErrorMessage()}", length, &path), - path, - }, - """, - ) - } +private data class Pattern(val patternTrait: PatternTrait) : StringTraitInfo() { + override fun toTraitInfo(): TraitInfo { + val pattern = patternTrait.pattern + + return TraitInfo( + { rust("let value = Self::check_pattern(value)?;") }, + { + docs("Error when a string doesn't satisfy its `@pattern`.") + docs("Contains the String that failed the pattern.") + rust("Pattern(String)") + }, + { + rust( + """ + Self::Pattern(string) => crate::model::ValidationExceptionField { + message: format!("${patternTrait.validationErrorMessage()}", &string, &path, r##"$pattern"##), + path + }, + """, + ) + }, + this::renderValidationFunction, + ) + } + + /** + * Renders a `check_pattern` function to validate the string matches the + * supplied regex in the `@pattern` trait. + */ + private fun renderValidationFunction(constraintViolation: Symbol): Writable { + val pattern = patternTrait.pattern + val errorMessageForUnsupportedRegex = """The regular expression $pattern is not supported by the `regex` crate; feel free to file an issue under https://github.com/awslabs/smithy-rs/issues for support""" + + return { + rustTemplate( + """ + fn check_pattern(string: String) -> Result { + let regex = Self::compile_regex(); + + if regex.is_match(&string) { + Ok(string) + } else { + Err($constraintViolation::Pattern(string)) } } - } + + pub fn compile_regex() -> &'static #{Regex}::Regex { + static REGEX: #{OnceCell}::sync::Lazy<#{Regex}::Regex> = #{OnceCell}::sync::Lazy::new(|| #{Regex}::Regex::new(r##"$pattern"##).expect(r##"$errorMessageForUnsupportedRegex"##)); + + ®EX + } + """, + "Regex" to ServerCargoDependency.Regex.toType(), + "OnceCell" to ServerCargoDependency.OnceCell.toType(), + ) } } } + +private sealed class StringTraitInfo { + companion object { + fun fromTrait(trait: Trait): StringTraitInfo = + when (trait) { + is PatternTrait -> { + Pattern(trait) + } + is LengthTrait -> { + Length(trait) + } + else -> PANIC("StringTraitInfo.fromTrait called with unsupported trait $trait") + } + } + + abstract fun toTraitInfo(): TraitInfo +} diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/TraitInfo.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/TraitInfo.kt new file mode 100644 index 0000000000..3390382ba1 --- /dev/null +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/TraitInfo.kt @@ -0,0 +1,20 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +package software.amazon.smithy.rust.codegen.server.smithy.generators + +import software.amazon.smithy.codegen.core.Symbol +import software.amazon.smithy.rust.codegen.core.rustlang.Writable +import software.amazon.smithy.rust.codegen.core.rustlang.rust + +/** + * Information needed to render a constraint trait as Rust code. + */ +data class TraitInfo( + val tryFromCheck: Writable, + val constraintViolationVariant: Writable, + val asValidationExceptionField: Writable, + val validationFunctionDefinition: (constraintViolation: Symbol) -> Writable, +) diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/protocol/ServerProtocolTestGenerator.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/protocol/ServerProtocolTestGenerator.kt index cf981edb73..01c5df10e5 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/protocol/ServerProtocolTestGenerator.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/protocol/ServerProtocolTestGenerator.kt @@ -18,6 +18,7 @@ import software.amazon.smithy.model.traits.ErrorTrait import software.amazon.smithy.protocoltests.traits.AppliesTo import software.amazon.smithy.protocoltests.traits.HttpMalformedRequestTestCase import software.amazon.smithy.protocoltests.traits.HttpMalformedRequestTestsTrait +import software.amazon.smithy.protocoltests.traits.HttpMalformedResponseBodyDefinition import software.amazon.smithy.protocoltests.traits.HttpMalformedResponseDefinition import software.amazon.smithy.protocoltests.traits.HttpRequestTestCase import software.amazon.smithy.protocoltests.traits.HttpRequestTestsTrait @@ -339,8 +340,13 @@ class ServerProtocolTestGenerator( } is TestCase.MalformedRequestTest -> { - // We haven't found any broken `HttpMalformedRequestTest`s yet. - it + val howToFixIt = BrokenMalformedRequestTests[Pair(codegenContext.serviceShape.id.toString(), it.id)] + if (howToFixIt == null) { + it + } else { + val fixed = howToFixIt(it.testCase) + TestCase.MalformedRequestTest(fixed) + } } } } @@ -969,17 +975,6 @@ class ServerProtocolTestGenerator( FailingTest(RestJsonValidation, "RestJsonMalformedPatternStringOverride_case1", TestType.MalformedRequest), FailingTest(RestJsonValidation, "RestJsonMalformedPatternUnionOverride_case0", TestType.MalformedRequest), FailingTest(RestJsonValidation, "RestJsonMalformedPatternUnionOverride_case1", TestType.MalformedRequest), - FailingTest(RestJsonValidation, "RestJsonMalformedPatternList_case0", TestType.MalformedRequest), - FailingTest(RestJsonValidation, "RestJsonMalformedPatternList_case1", TestType.MalformedRequest), - FailingTest(RestJsonValidation, "RestJsonMalformedPatternMapKey_case0", TestType.MalformedRequest), - FailingTest(RestJsonValidation, "RestJsonMalformedPatternMapKey_case1", TestType.MalformedRequest), - FailingTest(RestJsonValidation, "RestJsonMalformedPatternMapValue_case0", TestType.MalformedRequest), - FailingTest(RestJsonValidation, "RestJsonMalformedPatternMapValue_case1", TestType.MalformedRequest), - FailingTest(RestJsonValidation, "RestJsonMalformedPatternReDOSString", TestType.MalformedRequest), - FailingTest(RestJsonValidation, "RestJsonMalformedPatternString_case0", TestType.MalformedRequest), - FailingTest(RestJsonValidation, "RestJsonMalformedPatternString_case1", TestType.MalformedRequest), - FailingTest(RestJsonValidation, "RestJsonMalformedPatternUnion_case0", TestType.MalformedRequest), - FailingTest(RestJsonValidation, "RestJsonMalformedPatternUnion_case1", TestType.MalformedRequest), FailingTest(RestJsonValidation, "RestJsonMalformedRangeByteOverride_case0", TestType.MalformedRequest), FailingTest(RestJsonValidation, "RestJsonMalformedRangeByteOverride_case1", TestType.MalformedRequest), FailingTest(RestJsonValidation, "RestJsonMalformedRangeFloatOverride_case0", TestType.MalformedRequest), @@ -1176,6 +1171,27 @@ class ServerProtocolTestGenerator( private fun fixRestJsonComplexErrorWithNoMessage(testCase: HttpResponseTestCase): HttpResponseTestCase = testCase.toBuilder().putHeader("X-Amzn-Errortype", "aws.protocoltests.restjson#ComplexError").build() + // TODO(https://github.com/awslabs/smithy/issues/1506) + private fun fixRestJsonMalformedPatternReDOSString(testCase: HttpMalformedRequestTestCase): HttpMalformedRequestTestCase { + val brokenResponse = testCase.response + val brokenBody = brokenResponse.body.get() + val fixedBody = HttpMalformedResponseBodyDefinition.builder() + .mediaType(brokenBody.mediaType) + .contents( + """ + { + "message" : "1 validation error detected. Value 000000000000000000000000000000000000000000000000000000000000000000000000000000000000! at '/evilString' failed to satisfy constraint: Member must satisfy regular expression pattern: ^([0-9]+)+${'$'}", + "fieldList" : [{"message": "Value 000000000000000000000000000000000000000000000000000000000000000000000000000000000000! at '/evilString' failed to satisfy constraint: Member must satisfy regular expression pattern: ^([0-9]+)+${'$'}", "path": "/evilString"}] + } + """.trimIndent(), + ) + .build() + + return testCase.toBuilder() + .response(brokenResponse.toBuilder().body(fixedBody).build()) + .build() + } + // These are tests whose definitions in the `awslabs/smithy` repository are wrong. // This is because they have not been written from a server perspective, and as such the expected `params` field is incomplete. // TODO(https://github.com/awslabs/smithy-rs/issues/1288): Contribute a PR to fix them upstream. @@ -1258,5 +1274,11 @@ class ServerProtocolTestGenerator( Pair(RestJson, "RestJsonEmptyComplexErrorWithNoMessage") to ::fixRestJsonEmptyComplexErrorWithNoMessage, Pair(RestJson, "RestJsonComplexErrorWithNoMessage") to ::fixRestJsonComplexErrorWithNoMessage, ) + + private val BrokenMalformedRequestTests: Map, KFunction1> = + // TODO(https://github.com/awslabs/smithy/issues/1506) + mapOf( + Pair(RestJsonValidation, "RestJsonMalformedPatternReDOSString") to ::fixRestJsonMalformedPatternReDOSString, + ) } } diff --git a/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ConstraintsTest.kt b/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ConstraintsTest.kt index 80e2d93dae..76821a90dd 100644 --- a/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ConstraintsTest.kt +++ b/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ConstraintsTest.kt @@ -26,63 +26,58 @@ class ConstraintsTest { version: "123", operations: [TestOperation] } - + operation TestOperation { input: TestInputOutput, output: TestInputOutput, } - + structure TestInputOutput { map: MapA, - recursive: RecursiveShape } - + structure RecursiveShape { shape: RecursiveShape, mapB: MapB } - + @length(min: 1, max: 69) map MapA { key: String, value: MapB } - + map MapB { key: String, value: StructureA } - + @uniqueItems list ListA { member: MyString } - + @pattern("\\w+") string MyString - + @length(min: 1, max: 69) string LengthString - + structure StructureA { @range(min: 1, max: 69) int: Integer, - @required string: String } - + // This shape is not in the service closure. structure StructureB { @pattern("\\w+") patternString: String, - @required requiredString: String, - mapA: MapA, - @length(min: 1, max: 5) mapAPrecedence: MapA } @@ -94,7 +89,6 @@ class ConstraintsTest { private val mapA = model.lookup("test#MapA") private val mapB = model.lookup("test#MapB") private val listA = model.lookup("test#ListA") - private val myString = model.lookup("test#MyString") private val lengthString = model.lookup("test#LengthString") private val structA = model.lookup("test#StructureA") private val structAInt = model.lookup("test#StructureA\$int") @@ -114,7 +108,7 @@ class ConstraintsTest { @Test fun `it should not detect unsupported constrained traits as constrained`() { - listOf(structAInt, structAString, myString).forAll { + listOf(structAInt, structAString).forAll { it.isDirectlyConstrained(symbolProvider) shouldBe false } } @@ -123,9 +117,7 @@ class ConstraintsTest { fun `it should evaluate reachability of constrained shapes`() { mapA.canReachConstrainedShape(model, symbolProvider) shouldBe true structAInt.canReachConstrainedShape(model, symbolProvider) shouldBe false - - // This should be true when we start supporting the `pattern` trait on string shapes. - listA.canReachConstrainedShape(model, symbolProvider) shouldBe false + listA.canReachConstrainedShape(model, symbolProvider) shouldBe true // All of these eventually reach `StructureA`, which is constrained because one of its members is `required`. testInputOutput.canReachConstrainedShape(model, symbolProvider) shouldBe true diff --git a/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraintsAreNotUsedTest.kt b/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraintsAreNotUsedTest.kt index 78cec7408b..4c3fa0efe1 100644 --- a/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraintsAreNotUsedTest.kt +++ b/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraintsAreNotUsedTest.kt @@ -22,12 +22,12 @@ internal class ValidateUnsupportedConstraintsAreNotUsedTest { private val baseModel = """ namespace test - + service TestService { version: "123", operations: [TestOperation] } - + operation TestOperation { input: TestInputOutput, output: TestInputOutput, @@ -44,7 +44,7 @@ internal class ValidateUnsupportedConstraintsAreNotUsedTest { val model = """ $baseModel - + structure TestInputOutput { @required requiredString: String @@ -62,7 +62,7 @@ internal class ValidateUnsupportedConstraintsAreNotUsedTest { val model = """ $baseModel - + structure TestInputOutput { @length(min: 1, max: 69) lengthString: String @@ -79,7 +79,7 @@ internal class ValidateUnsupportedConstraintsAreNotUsedTest { val model = """ $baseModel - + structure TestInputOutput { @required string: String @@ -93,12 +93,12 @@ internal class ValidateUnsupportedConstraintsAreNotUsedTest { private val constraintTraitOnStreamingBlobShapeModel = """ $baseModel - + structure TestInputOutput { @required streamingBlob: StreamingBlob } - + @streaming @length(min: 69) blob StreamingBlob @@ -123,20 +123,20 @@ internal class ValidateUnsupportedConstraintsAreNotUsedTest { val model = """ $baseModel - + structure TestInputOutput { eventStream: EventStream } - + @streaming union EventStream { message: Message } - + structure Message { lengthString: LengthString } - + @length(min: 1) string LengthString """.asSmithyModel() @@ -155,17 +155,17 @@ internal class ValidateUnsupportedConstraintsAreNotUsedTest { val model = """ $baseModel - + structure TestInputOutput { collection: LengthCollection, blob: LengthBlob } - + @length(min: 1) list LengthCollection { member: String } - + @length(min: 1) blob LengthBlob """.asSmithyModel() @@ -176,35 +176,16 @@ internal class ValidateUnsupportedConstraintsAreNotUsedTest { validationResult.messages.forSome { it.message shouldContain "The blob shape `test#LengthBlob` has the constraint trait `smithy.api#length` attached" } } - @Test - fun `it should detect when the pattern trait on string shapes is used`() { - val model = - """ - $baseModel - - structure TestInputOutput { - patternString: PatternString - } - - @pattern("^[A-Za-z]+$") - string PatternString - """.asSmithyModel() - val validationResult = validateModel(model) - - validationResult.messages shouldHaveSize 1 - validationResult.messages[0].message shouldContain "The string shape `test#PatternString` has the constraint trait `smithy.api#pattern` attached" - } - @Test fun `it should detect when the range trait is used`() { val model = """ $baseModel - + structure TestInputOutput { rangeInteger: RangeInteger } - + @range(min: 1) integer RangeInteger """.asSmithyModel() diff --git a/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedStringGeneratorTest.kt b/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedStringGeneratorTest.kt index 75db6303f7..ddf9a53d07 100644 --- a/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedStringGeneratorTest.kt +++ b/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedStringGeneratorTest.kt @@ -25,7 +25,6 @@ import software.amazon.smithy.rust.codegen.server.smithy.testutil.serverTestCode import java.util.stream.Stream class ConstrainedStringGeneratorTest { - data class TestCase(val model: Model, val validString: String, val invalidString: String) class ConstrainedStringGeneratorTestProvider : ArgumentsProvider { @@ -44,11 +43,20 @@ class ConstrainedStringGeneratorTest { "👍👍👍", // These three emojis are three Unicode scalar values. "👍👍👍👍", ), + Triple("@pattern(\"^[a-z]+$\")", "valid", "123 invalid"), + Triple( + """ + @length(min: 3, max: 10) + @pattern("^a string$") + """, + "a string", "an invalid string", + ), + Triple("@pattern(\"123\")", "some pattern 123 in the middle", "no pattern at all"), ).map { TestCase( """ namespace test - + ${it.first} string ConstrainedString """.asSmithyModel(), @@ -116,10 +124,10 @@ class ConstrainedStringGeneratorTest { fun `type should not be constructible without using a constructor`() { val model = """ namespace test - + @length(min: 1, max: 69) string ConstrainedString - """.asSmithyModel() + """.asSmithyModel() val constrainedStringShape = model.lookup("test#ConstrainedString") val codegenContext = serverTestCodegenContext(model) @@ -136,14 +144,14 @@ class ConstrainedStringGeneratorTest { fun `Display implementation`() { val model = """ namespace test - + @length(min: 1, max: 69) string ConstrainedString - + @sensitive @length(min: 1, max: 78) string SensitiveConstrainedString - """.asSmithyModel() + """.asSmithyModel() val constrainedStringShape = model.lookup("test#ConstrainedString") val sensitiveConstrainedStringShape = model.lookup("test#SensitiveConstrainedString")