From 4295aa0c43dfaf8bb1c8da63d6ce90415707ce9f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Olivier=20M=C3=A9lois?= Date: Wed, 17 Apr 2024 19:28:37 +0200 Subject: [PATCH] Prevent default values from being rendered for refined fields (#1487) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes a bug that would happen when a refinement trait would be used on a member rather than a top-level shape, that would allow for default values of the wrong type to be rendered for the case class field. This corrects the behaviour by preventing the rendering of default values altogether for case-class fields that are refined Co-authored-by: Jakub Kozłowski --- CHANGELOG.md | 1 + .../src/generated/smithy4s/example/Age.scala | 4 +- .../smithy4s/example/PersonAge.scala | 4 +- .../example/StructureWithRefinedTypes.scala | 13 ++-- .../codegen/internals/SmithyToIR.scala | 2 +- sampleSpecs/refined.smithy | 69 +++++++++---------- 6 files changed, 44 insertions(+), 49 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d74e69779..1c6b92f84 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ # 0.18.16 +* Fixes bug leading to refined case-class fields being rendered with default values of the wrong type * Adds a `smithy4s-protobuf` module, containing derivation logic for protobuf codecs. See https://github.com/disneystreaming/smithy4s/pull/1455 * Add support for converting smithy4s services and schemas to smithy models * Add `smithy4s.meta#only` annotation allowing to filter operations in diff --git a/modules/bootstrapped/src/generated/smithy4s/example/Age.scala b/modules/bootstrapped/src/generated/smithy4s/example/Age.scala index 40d41cc53..bad4fd0b0 100644 --- a/modules/bootstrapped/src/generated/smithy4s/example/Age.scala +++ b/modules/bootstrapped/src/generated/smithy4s/example/Age.scala @@ -10,9 +10,7 @@ import smithy4s.schema.Schema.int object Age extends Newtype[smithy4s.refined.Age] { val id: ShapeId = ShapeId("smithy4s.example", "Age") - val hints: Hints = Hints( - smithy.api.Default(smithy4s.Document.fromDouble(0.0d)), - ).lazily + val hints: Hints = Hints.empty val underlyingSchema: Schema[smithy4s.refined.Age] = int.refined[smithy4s.refined.Age](smithy4s.example.AgeFormat()).withId(id).addHints(hints) implicit val schema: Schema[Age] = bijection(underlyingSchema, asBijection) } diff --git a/modules/bootstrapped/src/generated/smithy4s/example/PersonAge.scala b/modules/bootstrapped/src/generated/smithy4s/example/PersonAge.scala index ff37b2e9c..4d933506b 100644 --- a/modules/bootstrapped/src/generated/smithy4s/example/PersonAge.scala +++ b/modules/bootstrapped/src/generated/smithy4s/example/PersonAge.scala @@ -10,9 +10,7 @@ import smithy4s.schema.Schema.int object PersonAge extends Newtype[smithy4s.refined.Age] { val id: ShapeId = ShapeId("smithy4s.example", "PersonAge") - val hints: Hints = Hints( - smithy.api.Default(smithy4s.Document.fromDouble(0.0d)), - ).lazily + val hints: Hints = Hints.empty val underlyingSchema: Schema[smithy4s.refined.Age] = int.refined[smithy4s.refined.Age](smithy4s.example.AgeFormat()).withId(id).addHints(hints) implicit val schema: Schema[PersonAge] = bijection(underlyingSchema, asBijection) } diff --git a/modules/bootstrapped/src/generated/smithy4s/example/StructureWithRefinedTypes.scala b/modules/bootstrapped/src/generated/smithy4s/example/StructureWithRefinedTypes.scala index 6de7e3ca7..04b5fe292 100644 --- a/modules/bootstrapped/src/generated/smithy4s/example/StructureWithRefinedTypes.scala +++ b/modules/bootstrapped/src/generated/smithy4s/example/StructureWithRefinedTypes.scala @@ -4,9 +4,11 @@ import smithy4s.Hints import smithy4s.Schema import smithy4s.ShapeId import smithy4s.ShapeTag +import smithy4s.refined.Age.provider._ +import smithy4s.schema.Schema.int import smithy4s.schema.Schema.struct -final case class StructureWithRefinedTypes(age: Age, personAge: PersonAge, requiredAge: Age, fancyList: Option[smithy4s.example.FancyList] = None, unwrappedFancyList: Option[smithy4s.refined.FancyList] = None, name: Option[smithy4s.example.Name] = None, dogName: Option[smithy4s.refined.Name] = None) +final case class StructureWithRefinedTypes(requiredAge: smithy4s.example.Age, personAge: PersonAge, inlineFieldConstraint: smithy4s.refined.Age, age: Option[smithy4s.example.Age] = None, fancyList: Option[smithy4s.example.FancyList] = None, unwrappedFancyList: Option[smithy4s.refined.FancyList] = None, name: Option[smithy4s.example.Name] = None, dogName: Option[smithy4s.refined.Name] = None) object StructureWithRefinedTypes extends ShapeTag.Companion[StructureWithRefinedTypes] { val id: ShapeId = ShapeId("smithy4s.example", "StructureWithRefinedTypes") @@ -14,15 +16,16 @@ object StructureWithRefinedTypes extends ShapeTag.Companion[StructureWithRefined val hints: Hints = Hints.empty // constructor using the original order from the spec - private def make(age: Age, personAge: PersonAge, requiredAge: Age, fancyList: Option[smithy4s.example.FancyList], unwrappedFancyList: Option[smithy4s.refined.FancyList], name: Option[smithy4s.example.Name], dogName: Option[smithy4s.refined.Name]): StructureWithRefinedTypes = StructureWithRefinedTypes(age, personAge, requiredAge, fancyList, unwrappedFancyList, name, dogName) + private def make(age: Option[smithy4s.example.Age], personAge: PersonAge, requiredAge: smithy4s.example.Age, fancyList: Option[smithy4s.example.FancyList], unwrappedFancyList: Option[smithy4s.refined.FancyList], name: Option[smithy4s.example.Name], dogName: Option[smithy4s.refined.Name], inlineFieldConstraint: smithy4s.refined.Age): StructureWithRefinedTypes = StructureWithRefinedTypes(requiredAge, personAge, inlineFieldConstraint, age, fancyList, unwrappedFancyList, name, dogName) implicit val schema: Schema[StructureWithRefinedTypes] = struct( - Age.schema.field[StructureWithRefinedTypes]("age", _.age).addHints(smithy.api.Default(smithy4s.Document.fromDouble(0.0d))), - PersonAge.schema.field[StructureWithRefinedTypes]("personAge", _.personAge).addHints(smithy.api.Default(smithy4s.Document.fromDouble(0.0d))), - Age.schema.required[StructureWithRefinedTypes]("requiredAge", _.requiredAge).addHints(smithy.api.Default(smithy4s.Document.fromDouble(0.0d))), + smithy4s.example.Age.schema.optional[StructureWithRefinedTypes]("age", _.age), + PersonAge.schema.field[StructureWithRefinedTypes]("personAge", _.personAge).addHints(smithy.api.Default(smithy4s.Document.fromDouble(1.0d))), + smithy4s.example.Age.schema.required[StructureWithRefinedTypes]("requiredAge", _.requiredAge), smithy4s.example.FancyList.schema.optional[StructureWithRefinedTypes]("fancyList", _.fancyList), UnwrappedFancyList.underlyingSchema.optional[StructureWithRefinedTypes]("unwrappedFancyList", _.unwrappedFancyList), smithy4s.example.Name.schema.optional[StructureWithRefinedTypes]("name", _.name), DogName.underlyingSchema.optional[StructureWithRefinedTypes]("dogName", _.dogName), + int.refined[smithy4s.refined.Age](smithy4s.example.AgeFormat()).field[StructureWithRefinedTypes]("inlineFieldConstraint", _.inlineFieldConstraint).addHints(smithy.api.Default(smithy4s.Document.fromDouble(1.0d))), )(make).withId(id).addHints(hints) } diff --git a/modules/codegen/src/smithy4s/codegen/internals/SmithyToIR.scala b/modules/codegen/src/smithy4s/codegen/internals/SmithyToIR.scala index 6b97ea80d..0f6997de9 100644 --- a/modules/codegen/src/smithy4s/codegen/internals/SmithyToIR.scala +++ b/modules/codegen/src/smithy4s/codegen/internals/SmithyToIR.scala @@ -862,7 +862,7 @@ private[codegen] class SmithyToIR(model: Model, namespace: String) { } } val node = tr.toNode() - val targetTpe = shape.getTarget.tpe.get + val targetTpe = shape.tpe.get // Constructing the initial value for the refold val nodeAndType = targetTpe match { case Alias(_, _, tpe, true) => NodeAndType(node, tpe) diff --git a/sampleSpecs/refined.smithy b/sampleSpecs/refined.smithy index 067d0eb29..9d6598102 100644 --- a/sampleSpecs/refined.smithy +++ b/sampleSpecs/refined.smithy @@ -1,3 +1,5 @@ +$version: "2" + namespace smithy4s.example use smithy4s.meta#refinement @@ -5,60 +7,51 @@ use smithy4s.meta#unwrap @trait(selector: ":test(integer, member > integer)") @refinement( - targetType: "smithy4s.refined.Age", - providerImport: "smithy4s.refined.Age.provider._" + targetType: "smithy4s.refined.Age", + providerImport: "smithy4s.refined.Age.provider._" ) structure ageFormat {} -@trait(selector: "list:test(> member > string)") // lists with string members -@refinement( - targetType: "smithy4s.refined.FancyList" -) +@trait(selector: "list:test(> member > string)") +// lists with string members +@refinement(targetType: "smithy4s.refined.FancyList") structure fancyListFormat {} @trait(selector: "string") -@refinement( - targetType: "smithy4s.refined.Name" -) +@refinement(targetType: "smithy4s.refined.Name") structure nameFormat {} @trait(selector: "list") -@refinement( - targetType: "smithy4s.refined.NonEmptyList", - parameterised: true -) +@refinement(targetType: "smithy4s.refined.NonEmptyList", parameterised: true) structure nonEmptyListFormat {} @trait(selector: "map") -@refinement( - targetType: "smithy4s.refined.NonEmptyMap", - parameterised: true -) +@refinement(targetType: "smithy4s.refined.NonEmptyMap", parameterised: true) structure nonEmptyMapFormat {} @nonEmptyListFormat list NonEmptyStrings { - member: String + member: String } @nonEmptyListFormat list NonEmptyNames { - member: Name + member: Name } structure Candy { - name: String + name: String } @nonEmptyListFormat list NonEmptyCandies { - member: Candy + member: Candy } @nonEmptyMapFormat map NonEmptyMapNumbers { - key: String - value: Integer + key: String + value: Integer } @ageFormat @@ -69,13 +62,13 @@ integer PersonAge @fancyListFormat list FancyList { - member: String + member: String } @fancyListFormat @unwrap list UnwrappedFancyList { - member: String + member: String } @nameFormat @@ -86,22 +79,24 @@ string Name string DogName structure StructureWithRefinedTypes { - age: Age, - personAge: PersonAge, - @required - requiredAge: Age, - fancyList: FancyList, - unwrappedFancyList: UnwrappedFancyList, - name: Name, - dogName: DogName + age: Age + personAge: PersonAge = 1 + @required + requiredAge: Age + fancyList: FancyList + unwrappedFancyList: UnwrappedFancyList + name: Name + dogName: DogName + @ageFormat + inlineFieldConstraint: Integer = 1 } union UnionWithRefinedTypes { - age: Age, - dogName: DogName + age: Age + dogName: DogName } structure StructureWithRefinedMember { - @ageFormat - otherAge: Integer, + @ageFormat + otherAge: Integer }