Skip to content

Commit

Permalink
Implement @range on long and byte shapes (#2036)
Browse files Browse the repository at this point in the history
* Group all override-related test cases together.

* Add @range examples for both byte and long in constraints.smithy.

* Implement @range for long and byte shapes.

* Update CHANGELOG

* Fix unit tests.

* Address comments

* Dry up further.
  • Loading branch information
LukeMathWalker committed Nov 29, 2022
1 parent 3a9a42b commit 4c77965
Show file tree
Hide file tree
Showing 16 changed files with 280 additions and 133 deletions.
6 changes: 4 additions & 2 deletions CHANGELOG.next.toml
Expand Up @@ -242,15 +242,17 @@ message = """
* The `length` trait on `string` shapes.
* The `length` trait on `map` shapes.
* The `range` trait on `integer` shapes.
* The `range` trait on `byte` shapes.
* The `range` trait on `short` shapes.
* The `range` trait on `integer` shapes.
* The `range` trait on `long` 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#2005", "smithy-rs#1998", "smithy-rs#2034"]
references = ["smithy-rs#1199", "smithy-rs#1342", "smithy-rs#1401", "smithy-rs#2005", "smithy-rs#1998", "smithy-rs#2034", "smithy-rs#2036"]
meta = { "breaking" = true, "tada" = true, "bug" = false, "target" = "server" }
author = "david-perez"

Expand Down
132 changes: 131 additions & 1 deletion codegen-core/common-test-models/constraints.smithy
Expand Up @@ -50,7 +50,7 @@ operation ConstrainedShapesOperation {
}

@http(
uri: "/constrained-http-bound-shapes-operation/{rangeIntegerLabel}/{rangeShortLabel}/{lengthStringLabel}/{enumStringLabel}",
uri: "/constrained-http-bound-shapes-operation/{rangeIntegerLabel}/{rangeShortLabel}/{rangeLongLabel}/{rangeByteLabel}/{lengthStringLabel}/{enumStringLabel}",
method: "POST"
)
operation ConstrainedHttpBoundShapesOperation {
Expand Down Expand Up @@ -186,6 +186,14 @@ structure ConstrainedHttpBoundShapesOperationInputOutput {
@httpLabel
rangeShortLabel: RangeShort,

@required
@httpLabel
rangeLongLabel: RangeLong,

@required
@httpLabel
rangeByteLabel: RangeByte,

@required
@httpLabel
enumStringLabel: EnumString,
Expand All @@ -203,6 +211,12 @@ structure ConstrainedHttpBoundShapesOperationInputOutput {
@httpHeader("X-Range-Short")
rangeShortHeader: RangeShort,

@httpHeader("X-Range-Long")
rangeLongHeader: RangeLong,

@httpHeader("X-Range-Byte")
rangeByteHeader: RangeByte,

// @httpHeader("X-Length-MediaType")
// lengthStringHeaderWithMediaType: MediaTypeLengthString,

Expand All @@ -220,13 +234,23 @@ structure ConstrainedHttpBoundShapesOperationInputOutput {
// rangeIntegerSetHeader: SetOfRangeInteger,
// @httpHeader("X-Range-Short-Set")
// rangeShortSetHeader: SetOfShortInteger,
// @httpHeader("X-Range-Long-Set")
// rangeLongSetHeader: SetOfRangeLong,
// @httpHeader("X-Range-Byte-Set")
// rangeByteSetHeader: SetOfByteInteger,

@httpHeader("X-Range-Integer-List")
rangeIntegerListHeader: ListOfRangeInteger,

@httpHeader("X-Range-Short-List")
rangeShortListHeader: ListOfRangeShort,

@httpHeader("X-Range-Long-List")
rangeLongListHeader: ListOfRangeLong,

@httpHeader("X-Range-Byte-List")
rangeByteListHeader: ListOfRangeByte,

// TODO(https://github.com/awslabs/smithy-rs/issues/1431)
// @httpHeader("X-Enum")
//enumStringHeader: EnumString,
Expand All @@ -243,6 +267,12 @@ structure ConstrainedHttpBoundShapesOperationInputOutput {
@httpQuery("rangeShort")
rangeShortQuery: RangeShort,

@httpQuery("rangeLong")
rangeLongQuery: RangeLong,

@httpQuery("rangeByte")
rangeByteQuery: RangeByte,

@httpQuery("enumString")
enumStringQuery: EnumString,

Expand All @@ -260,12 +290,22 @@ structure ConstrainedHttpBoundShapesOperationInputOutput {
@httpQuery("rangeShortList")
rangeShortListQuery: ListOfRangeShort,

@httpQuery("rangeLongList")
rangeLongListQuery: ListOfRangeLong,

@httpQuery("rangeByteList")
rangeByteListQuery: ListOfRangeByte,

// 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("rangeShortSet")
// rangeShortSetQuery: SetOfRangeShort,
// @httpQuery("rangeLongSet")
// rangeLongSetQuery: SetOfRangeLong,
// @httpQuery("rangeByteSet")
// rangeByteSetQuery: SetOfRangeByte,

@httpQuery("enumStringList")
enumStringListQuery: ListOfEnumString,
Expand Down Expand Up @@ -392,6 +432,16 @@ structure ConA {
maxRangeShort: MaxRangeShort,
fixedValueShort: FixedValueShort,

rangeLong: RangeLong,
minRangeLong: MinRangeLong,
maxRangeLong: MaxRangeLong,
fixedValueLong: FixedValueLong,

rangeByte: RangeByte,
minRangeByte: MinRangeByte,
maxRangeByte: MaxRangeByte,
fixedValueByte: FixedValueByte,

conBList: ConBList,
conBList2: ConBList2,

Expand Down Expand Up @@ -424,6 +474,18 @@ structure ConA {
// setOfRangeShort: SetOfRangeShort,
mapOfRangeShort: MapOfRangeShort,

listOfRangeLong: ListOfRangeLong,
// 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.
// setOfRangeLong: SetOfRangeLong,
mapOfRangeLong: MapOfRangeLong,

listOfRangeByte: ListOfRangeByte,
// 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.
// setOfRangeByte: SetOfRangeByte,
mapOfRangeByte: MapOfRangeByte,

nonStreamingBlob: NonStreamingBlob

patternString: PatternString,
Expand Down Expand Up @@ -456,6 +518,16 @@ map MapOfRangeShort {
value: RangeShort,
}

map MapOfRangeLong {
key: String,
value: RangeLong,
}

map MapOfRangeByte {
key: String,
value: RangeByte,
}

map MapOfEnumString {
key: EnumString,
value: EnumString,
Expand Down Expand Up @@ -503,6 +575,20 @@ map MapOfSetOfLengthString {
// value: SetOfRangeShort,
// }

// 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 MapOfSetOfRangeLong {
// key: String,
// value: SetOfRangeLong,
// }

// 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 MapOfSetOfRangeByte {
// key: String,
// value: SetOfRangeByte,
// }

@length(min: 2, max: 8)
list LengthListOfLengthString {
member: LengthString
Expand Down Expand Up @@ -555,6 +641,30 @@ short MaxRangeShort
@range(min: 10, max: 10)
short FixedValueShort

@range(min: -0, max: 10)
long RangeLong

@range(min: -10)
long MinRangeLong

@range(max: 11)
long MaxRangeLong

@range(min: 10, max: 10)
long FixedValueLong

@range(min: -0, max: 10)
byte RangeByte

@range(min: -10)
byte MinRangeByte

@range(max: 11)
byte MaxRangeByte

@range(min: 10, max: 10)
byte FixedValueByte

/// A union with constrained members.
union ConstrainedUnion {
enumString: EnumString,
Expand Down Expand Up @@ -620,6 +730,26 @@ list ListOfRangeShort {
member: RangeShort
}

// 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 SetOfRangeLong {
// member: RangeLong
// }

list ListOfRangeLong {
member: RangeLong
}

// 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 SetOfRangeByte {
// member: RangeByte
// }

list ListOfRangeByte {
member: RangeByte
}

list ListOfEnumString {
member: EnumString
}
Expand Down
Expand Up @@ -8,8 +8,10 @@ package software.amazon.smithy.rust.codegen.server.smithy
import software.amazon.smithy.codegen.core.Symbol
import software.amazon.smithy.model.Model
import software.amazon.smithy.model.knowledge.NullableIndex
import software.amazon.smithy.model.shapes.ByteShape
import software.amazon.smithy.model.shapes.CollectionShape
import software.amazon.smithy.model.shapes.IntegerShape
import software.amazon.smithy.model.shapes.LongShape
import software.amazon.smithy.model.shapes.MapShape
import software.amazon.smithy.model.shapes.MemberShape
import software.amazon.smithy.model.shapes.ServiceShape
Expand Down Expand Up @@ -98,7 +100,7 @@ class ConstrainedShapeSymbolProvider(
}
}

is StringShape, is IntegerShape, is ShortShape -> {
is StringShape, is IntegerShape, is ShortShape, is LongShape, is ByteShape -> {
if (shape.isDirectlyConstrained(base)) {
val rustType = RustType.Opaque(shape.contextName(serviceShape).toPascalCase())
symbolBuilder(shape, rustType).locatedIn(ModelsModule).build()
Expand Down
Expand Up @@ -7,8 +7,10 @@ package software.amazon.smithy.rust.codegen.server.smithy

import software.amazon.smithy.codegen.core.Symbol
import software.amazon.smithy.model.Model
import software.amazon.smithy.model.shapes.ByteShape
import software.amazon.smithy.model.shapes.CollectionShape
import software.amazon.smithy.model.shapes.IntegerShape
import software.amazon.smithy.model.shapes.LongShape
import software.amazon.smithy.model.shapes.MapShape
import software.amazon.smithy.model.shapes.ServiceShape
import software.amazon.smithy.model.shapes.Shape
Expand Down Expand Up @@ -115,7 +117,7 @@ class ConstraintViolationSymbolProvider(
.build()
}

is StringShape, is IntegerShape, is ShortShape -> {
is StringShape, is IntegerShape, is ShortShape, is LongShape, is ByteShape -> {
val module = shape.shapeModule()
val rustType = RustType.Opaque(constraintViolationName, module.fullyQualifiedPath())
Symbol.builder()
Expand Down
Expand Up @@ -8,8 +8,10 @@ package software.amazon.smithy.rust.codegen.server.smithy
import software.amazon.smithy.codegen.core.SymbolProvider
import software.amazon.smithy.model.Model
import software.amazon.smithy.model.neighbor.Walker
import software.amazon.smithy.model.shapes.ByteShape
import software.amazon.smithy.model.shapes.CollectionShape
import software.amazon.smithy.model.shapes.IntegerShape
import software.amazon.smithy.model.shapes.LongShape
import software.amazon.smithy.model.shapes.MapShape
import software.amazon.smithy.model.shapes.MemberShape
import software.amazon.smithy.model.shapes.Shape
Expand Down Expand Up @@ -73,7 +75,7 @@ fun Shape.isDirectlyConstrained(symbolProvider: SymbolProvider): Boolean = when

is MapShape -> this.hasTrait<LengthTrait>()
is StringShape -> this.hasTrait<EnumTrait>() || supportedStringConstraintTraits.any { this.hasTrait(it) }
is IntegerShape, is ShortShape -> this.hasTrait<RangeTrait>()
is IntegerShape, is ShortShape, is LongShape, is ByteShape -> this.hasTrait<RangeTrait>()
else -> false
}

Expand All @@ -99,7 +101,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 && supportedStringConstraintTraits.any(this::hasTrait))
is IntegerShape, is ShortShape -> publicConstrainedTypes && this.hasTrait<RangeTrait>()
is IntegerShape, is ShortShape, is LongShape, is ByteShape -> publicConstrainedTypes && this.hasTrait<RangeTrait>()
is MemberShape -> model.expectShape(this.target).hasPublicConstrainedWrapperTupleType(model, publicConstrainedTypes)
else -> false
}
Expand Down
Expand Up @@ -10,9 +10,11 @@ import software.amazon.smithy.codegen.core.CodegenException
import software.amazon.smithy.model.Model
import software.amazon.smithy.model.knowledge.NullableIndex
import software.amazon.smithy.model.neighbor.Walker
import software.amazon.smithy.model.shapes.ByteShape
import software.amazon.smithy.model.shapes.CollectionShape
import software.amazon.smithy.model.shapes.IntegerShape
import software.amazon.smithy.model.shapes.ListShape
import software.amazon.smithy.model.shapes.LongShape
import software.amazon.smithy.model.shapes.MapShape
import software.amazon.smithy.model.shapes.ServiceShape
import software.amazon.smithy.model.shapes.SetShape
Expand Down Expand Up @@ -44,9 +46,8 @@ import software.amazon.smithy.rust.codegen.core.smithy.transformers.RecursiveSha
import software.amazon.smithy.rust.codegen.core.util.CommandFailed
import software.amazon.smithy.rust.codegen.core.util.hasTrait
import software.amazon.smithy.rust.codegen.core.util.runCommand
import software.amazon.smithy.rust.codegen.server.smithy.generators.ConstrainedIntegerGenerator
import software.amazon.smithy.rust.codegen.server.smithy.generators.ConstrainedMapGenerator
import software.amazon.smithy.rust.codegen.server.smithy.generators.ConstrainedShortGenerator
import software.amazon.smithy.rust.codegen.server.smithy.generators.ConstrainedNumberGenerator
import software.amazon.smithy.rust.codegen.server.smithy.generators.ConstrainedStringGenerator
import software.amazon.smithy.rust.codegen.server.smithy.generators.ConstrainedTraitForEnumGenerator
import software.amazon.smithy.rust.codegen.server.smithy.generators.MapConstraintViolationGenerator
Expand Down Expand Up @@ -358,7 +359,7 @@ open class ServerCodegenVisitor(
if (shape.isDirectlyConstrained(codegenContext.symbolProvider)) {
logger.info("[rust-server-codegen] Generating a constrained integer $shape")
rustCrate.withModule(ModelsModule) {
ConstrainedIntegerGenerator(codegenContext, this, shape).render()
ConstrainedNumberGenerator(codegenContext, this, shape).render()
}
}
}
Expand All @@ -367,7 +368,25 @@ open class ServerCodegenVisitor(
if (shape.isDirectlyConstrained(codegenContext.symbolProvider)) {
logger.info("[rust-server-codegen] Generating a constrained short $shape")
rustCrate.withModule(ModelsModule) {
ConstrainedShortGenerator(codegenContext, this, shape).render()
ConstrainedNumberGenerator(codegenContext, this, shape).render()
}
}
}

override fun longShape(shape: LongShape) {
if (shape.isDirectlyConstrained(codegenContext.symbolProvider)) {
logger.info("[rust-server-codegen] Generating a constrained long $shape")
rustCrate.withModule(ModelsModule) {
ConstrainedNumberGenerator(codegenContext, this, shape).render()
}
}
}

override fun byteShape(shape: ByteShape) {
if (shape.isDirectlyConstrained(codegenContext.symbolProvider)) {
logger.info("[rust-server-codegen] Generating a constrained byte $shape")
rustCrate.withModule(ModelsModule) {
ConstrainedNumberGenerator(codegenContext, this, shape).render()
}
}
}
Expand Down

0 comments on commit 4c77965

Please sign in to comment.