-
Notifications
You must be signed in to change notification settings - Fork 98
Dynamic validation #1991
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
base: series/0.19
Are you sure you want to change the base?
Dynamic validation #1991
Changes from 3 commits
f3fefe1
dd0c5d1
7b4d055
afa2f7a
356174d
93b9371
3439438
5ae2ab4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,6 +27,20 @@ private[dynamic] trait DynamicSchemaIndexCompanionPlatform { | |
| */ | ||
| def loadModel( | ||
| model: software.amazon.smithy.model.Model | ||
| ): DynamicSchemaIndex = loadModel(model, performValidation = false) | ||
|
|
||
| /** | ||
| * Loads a dynamic schema index model from a smithy model. | ||
| * | ||
| * @param performValidation when true, constraint traits (`@length`, `@range`, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My first thought seeing this name was that we'd be doing model validations rather than schema validations... maybe |
||
| * `@pattern` etc) are reified into Schema objects that get enforced upon | ||
| * decoding, instead of being kept as inert hints (which is the default dynamic | ||
| * behaviour), mirroring the fact that codegen-produced schemas enforce these | ||
| * constraints but dynamically-loaded do not). | ||
| */ | ||
| def loadModel( | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We also have a platform-agnostic variant that works on our own model of |
||
| model: software.amazon.smithy.model.Model, | ||
| performValidation: Boolean | ||
| ): DynamicSchemaIndex = { | ||
| val flattenedModel = | ||
| ModelTransformer.create().flattenAndRemoveMixins(model); | ||
|
|
@@ -35,8 +49,10 @@ private[dynamic] trait DynamicSchemaIndexCompanionPlatform { | |
| smithy4s.Document | ||
| .decode[smithy4s.dynamic.model.Model](document) | ||
| .map(load(_)) match { | ||
| case Left(error) => throw error | ||
| case Right(value) => value | ||
| case Left(error) => throw error | ||
| case Right(value) => | ||
| if (performValidation) DynamicSchemaValidation.reifyConstraints(value) | ||
| else value | ||
| } | ||
| } | ||
|
|
||
|
|
||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we need to limit ourselves to JVM here, this could work on all platforms. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,148 @@ | ||
| /* | ||
| * Copyright 2021-2026 Disney Streaming | ||
| * | ||
| * Licensed under the Tomorrow Open Source Technology License, Version 1.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * https://disneystreaming.github.io/TOST-1.0.txt | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| package smithy4s.dynamic | ||
|
|
||
| import smithy4s.Document | ||
| import smithy4s.Refinement | ||
| import smithy4s.RefinementProvider | ||
| import smithy4s.ShapeId | ||
| import smithy4s.Surjection | ||
| import smithy4s.schema.CollectionTag._ | ||
| import smithy4s.schema.Primitive._ | ||
| import smithy4s.schema.Schema | ||
| import smithy4s.schema.Schema._ | ||
| import smithy4s.~> | ||
|
|
||
| /** | ||
| * Dynamically-loaded schemas only carry constraint traits (`@length`, `@range`, | ||
| * `@pattern` etc) as hints: by default, `DynamicModelCompiler` only attaches them via | ||
| * `addHints`, unlike smithy4s codegen, which reifies such traits into `RefinementSchema` | ||
| * wrappers that get enforced upon decoding. | ||
| * | ||
| * This object provides a transformation for dynamically-loaded schemas, so that validation | ||
| * hints are reintroduced at a `Schema` level. | ||
| */ | ||
| private[dynamic] object DynamicSchemaValidation { | ||
|
|
||
| def reifyConstraints(index: DynamicSchemaIndex): DynamicSchemaIndex = | ||
| new DynamicSchemaIndex { | ||
| def allServices: Iterable[DynamicSchemaIndex.ServiceWrapper] = | ||
| index.allServices | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
To be safe, we could convert those services to builders and map each endpoint's input/output/errors, reifying the constraints right there. I think a simpler alternative to this would be embedding the reification process directly in the DynamicCompiler rather than in a "middleware" (here) |
||
| def allSchemas: Iterable[Schema[_]] = | ||
| index.allSchemas.map(reifySchema(_)) | ||
| def getSchema(shapeId: ShapeId): Option[Schema[_]] = | ||
| index.getSchema(shapeId).map(reifySchema(_)) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't like that we map the schemas on read. Not only is it per-call overhead, but it can also produce different instances of schemas when you call it multiple times, which can break things like schema compilation caches. |
||
| def metadata: Map[String, Document] = index.metadata | ||
| } | ||
|
|
||
| private def reifySchema[A](schema: Schema[A]): Schema[A] = | ||
| schema.transformTransitivelyK(ReifyConstraints) | ||
|
|
||
| private object ReifyConstraints extends (Schema ~> Schema) { | ||
|
|
||
| private def void[C, A]( | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think at this point we should move this to |
||
| underlying: RefinementProvider[C, A, ?] | ||
| ): RefinementProvider.Simple[C, A] = | ||
| Refinement | ||
| .drivenBy[C] | ||
| .contextual[A, A](c => | ||
| Surjection(v => underlying.make(c).apply(v).map(_ => v), identity) | ||
| )(underlying.tag) | ||
|
|
||
| private implicit class SchemaOps[A](schema: Schema[A]) { | ||
| def reifyHint[B](rp: RefinementProvider[B, A, ?]): Schema[A] = | ||
| schema.hints.get(rp.tag).fold(schema)(schema.validated(_)(void(rp))) | ||
| } | ||
|
|
||
| private def collection[C[_], B]( | ||
| schema: Schema.CollectionSchema[C, B] | ||
| ): Schema[C[B]] = | ||
| schema.tag match { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. AFAIK that's not an exhaustive match ( |
||
| case ListTag => | ||
| schema.reifyHint(RefinementProvider.iterableLengthConstraint[List, B]) | ||
| case VectorTag => | ||
| schema.reifyHint( | ||
| RefinementProvider.iterableLengthConstraint[Vector, B] | ||
| ) | ||
| case SetTag => | ||
| schema.reifyHint(RefinementProvider.iterableLengthConstraint[Set, B]) | ||
| case IndexedSeqTag => | ||
| schema.reifyHint( | ||
| RefinementProvider.iterableLengthConstraint[IndexedSeq, B] | ||
| ) | ||
| } | ||
|
|
||
| private def enumSchema[B <: Enum[?]]( | ||
| schema: Schema.EnumerationSchema[B] | ||
| ): Schema[B] = | ||
| schema | ||
| .reifyHint(RefinementProvider.lengthConstraint[B](_.toString.length)) | ||
| .reifyHint(RefinementProvider.rangeConstraint[B, Int](_.ordinal())) | ||
| .reifyHint(RefinementProvider.patternConstraint[B](e => e.toString)) | ||
|
|
||
| def apply[A](schema: Schema[A]): Schema[A] = | ||
| schema match { | ||
| case t @ PrimitiveSchema(_, _, tag) => | ||
| tag match { | ||
| case PString => | ||
| t.reifyHint(RefinementProvider.stringLengthConstraint) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. also: |
||
| .reifyHint(RefinementProvider.stringPatternConstraints) | ||
| case PByte => | ||
| schema.reifyHint(RefinementProvider.numericRangeConstraints[Byte]) | ||
| case PShort => | ||
| schema.reifyHint( | ||
| RefinementProvider.numericRangeConstraints[Short] | ||
| ) | ||
| case PInt => | ||
| schema.reifyHint(RefinementProvider.numericRangeConstraints[Int]) | ||
| case PLong => | ||
| schema.reifyHint(RefinementProvider.numericRangeConstraints[Long]) | ||
| case PFloat => | ||
| schema.reifyHint( | ||
| RefinementProvider.numericRangeConstraints[Float] | ||
| ) | ||
| case PDouble => | ||
| schema.reifyHint( | ||
| RefinementProvider.numericRangeConstraints[Double] | ||
| ) | ||
| case PBigInt => | ||
| schema.reifyHint( | ||
| RefinementProvider.numericRangeConstraints[BigInt] | ||
| ) | ||
| case PBigDecimal => | ||
| schema.reifyHint( | ||
| RefinementProvider.numericRangeConstraints[BigDecimal] | ||
| ) | ||
| case PBlob => | ||
| schema.reifyHint(RefinementProvider.blobLengthConstraint) | ||
| case PTimestamp | PDocument | PBoolean | PUUID | PLocalDate | | ||
| PLocalTime | PDuration | POffsetDateTime => | ||
| schema | ||
| } | ||
| case e: EnumerationSchema[?] => | ||
| enumSchema(e.asInstanceOf[EnumerationSchema[Enum[?]]]) | ||
| .asInstanceOf[Schema[A]] | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Casts are pretty risky in Dynamic land. Any way we can make this work without them? |
||
| case c @ CollectionSchema(_, _, _, _) => collection(c) | ||
| case m: MapSchema[c, k, v] => | ||
| m.reifyHint( | ||
| RefinementProvider.lengthConstraint[c[k, v]](m.tag.iterator(_).size) | ||
| ) | ||
| case other => other | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd handle the remainder explicitly, like in smithy-playground |
||
| } | ||
| } | ||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| /* | ||
| * Copyright 2021-2026 Disney Streaming | ||
| * | ||
| * Licensed under the Tomorrow Open Source Technology License, Version 1.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * https://disneystreaming.github.io/TOST-1.0.txt | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| package smithy4s.dynamic | ||
|
|
||
| import smithy4s.Document | ||
| import smithy4s.ShapeId | ||
| import software.amazon.smithy.model.Model | ||
|
|
||
| class DynamicValidationSpec extends DummyIO.Suite { | ||
|
|
||
| test("loadModel does not enforce constraint traits by default") { | ||
| val decoded = decodeInvalidDocument(performValidation = false) | ||
| assert(decoded.isRight, s"Expected decoding to succeed, got: $decoded") | ||
| } | ||
|
|
||
| test("loadModel(performValidation = true) enforces constraint traits") { | ||
| val decoded = decodeInvalidDocument(performValidation = true) | ||
| assert(decoded.isLeft, s"Expected decoding to fail, got: $decoded") | ||
| } | ||
|
|
||
| private def decodeInvalidDocument(performValidation: Boolean) = { | ||
|
|
||
| val smithy = """ | ||
| $version: "2" | ||
| namespace example | ||
|
|
||
| structure Foo { | ||
| bar: ShortString | ||
| } | ||
|
|
||
| @length(min: 1, max: 3) | ||
| string ShortString | ||
| """ | ||
|
|
||
| val model = | ||
| Model | ||
| .assembler() | ||
| .addUnparsedModel("dynamic.smithy", smithy) | ||
| .discoverModels(this.getClass().getClassLoader()) | ||
| .assemble() | ||
| .unwrap() | ||
|
|
||
| val index = | ||
| DynamicSchemaIndex.loadModel(model, performValidation = performValidation) | ||
|
|
||
| val fooShapeId = ShapeId("example", "Foo") | ||
|
|
||
| val invalidDocument = Document.obj( | ||
| "bar" -> Document.fromString("foobar") | ||
| ) | ||
| val schema = index | ||
| .getSchema(fooShapeId) | ||
| .getOrElse(fail("Error: shape missing")) | ||
| Document.Decoder.fromSchema(schema).decode(invalidDocument) | ||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.11 is already out, we need a new section :)