From d78918bfdb73060e585379f07ed8a7ab5d67209c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Koz=C5=82owski?= Date: Fri, 27 Sep 2024 01:34:33 +0200 Subject: [PATCH 1/5] Reproduce #1594 --- .../example/HasConstrainedNewtype.scala | 22 +++++++++++++++++++ .../codegen/internals/ModelLoader.scala | 2 +- sampleSpecs/memberConstraints.smithy | 6 +++++ 3 files changed, 29 insertions(+), 1 deletion(-) create mode 100644 modules/bootstrapped/src/generated/smithy4s/example/HasConstrainedNewtype.scala diff --git a/modules/bootstrapped/src/generated/smithy4s/example/HasConstrainedNewtype.scala b/modules/bootstrapped/src/generated/smithy4s/example/HasConstrainedNewtype.scala new file mode 100644 index 000000000..3139eb114 --- /dev/null +++ b/modules/bootstrapped/src/generated/smithy4s/example/HasConstrainedNewtype.scala @@ -0,0 +1,22 @@ +package smithy4s.example + +import smithy4s.Hints +import smithy4s.Schema +import smithy4s.ShapeId +import smithy4s.ShapeTag +import smithy4s.schema.Schema.struct + +final case class HasConstrainedNewtype(s: CityId) + +object HasConstrainedNewtype extends ShapeTag.Companion[HasConstrainedNewtype] { + val id: ShapeId = ShapeId("smithy4s.example", "HasConstrainedNewtype") + + val hints: Hints = Hints.empty + + // constructor using the original order from the spec + private def make(s: CityId): HasConstrainedNewtype = HasConstrainedNewtype(s) + + implicit val schema: Schema[HasConstrainedNewtype] = struct( + CityId.schema.validated(smithy.api.Length(min = Some(1L), max = None)).required[HasConstrainedNewtype]("s", _.s), + )(make).withId(id).addHints(hints) +} diff --git a/modules/codegen/src/smithy4s/codegen/internals/ModelLoader.scala b/modules/codegen/src/smithy4s/codegen/internals/ModelLoader.scala index 9958dc94c..c2cb7fcf1 100644 --- a/modules/codegen/src/smithy4s/codegen/internals/ModelLoader.scala +++ b/modules/codegen/src/smithy4s/codegen/internals/ModelLoader.scala @@ -51,7 +51,7 @@ private[codegen] object ModelLoader { val modelsInJars = deps.flatMap { file => Using.resource( // Note: On JDK13+, the second parameter is redundant. - FileSystems.newFileSystem(file.toPath(), null) + FileSystems.newFileSystem(file.toPath(), null: ClassLoader) ) { jarFS => val p = jarFS.getPath("META-INF", "smithy", "manifest") diff --git a/sampleSpecs/memberConstraints.smithy b/sampleSpecs/memberConstraints.smithy index a2b75abf7..f0860cbcc 100644 --- a/sampleSpecs/memberConstraints.smithy +++ b/sampleSpecs/memberConstraints.smithy @@ -14,3 +14,9 @@ map ConstrainedMap { value: String } +// Regression test for https://github.com/disneystreaming/smithy4s/issues/1594 +structure HasConstrainedNewtype { + @length(min: 1) + @required + s: CityId +} From 8d48aff131efb121440b4c9c56e223bcf929b1b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Koz=C5=82owski?= Date: Fri, 27 Sep 2024 01:38:07 +0200 Subject: [PATCH 2/5] Use correct implicit parameter order --- modules/core/src/smithy4s/RefinementProvider.scala | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/modules/core/src/smithy4s/RefinementProvider.scala b/modules/core/src/smithy4s/RefinementProvider.scala index b1fa5d829..26d6600f5 100644 --- a/modules/core/src/smithy4s/RefinementProvider.scala +++ b/modules/core/src/smithy4s/RefinementProvider.scala @@ -202,9 +202,14 @@ private[smithy4s] trait LowPriorityImplicits { : RefinementProvider[Pattern, E, E] = new RefinementProvider.PatternConstraint[E](e => e.value) - implicit def isomorphismConstraint[C, A, A0](implicit + @deprecated("Use isomorphismConstraint2 instead", "0.18.25") + private[smithy4s] def isomorphismConstraint[C, A, A0](implicit constraintOnA: RefinementProvider.Simple[C, A], iso: Bijection[A, A0] - ): RefinementProvider[C, A0, A0] = constraintOnA.imapFull[A0, A0](iso, iso) + ): RefinementProvider[C, A0, A0] = isomorphismConstraint2 + implicit def isomorphismConstraint2[C, A, A0](implicit + iso: Bijection[A, A0], + constraintOnA: RefinementProvider.Simple[C, A] + ): RefinementProvider[C, A0, A0] = constraintOnA.imapFull[A0, A0](iso, iso) } From 403d2c7c00bb10f78261890a86c1c336198d6360 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Koz=C5=82owski?= Date: Fri, 27 Sep 2024 01:46:29 +0200 Subject: [PATCH 3/5] Add more examples --- .../example/HasConstrainedNewtypes.scala | 26 +++++++++++++++++++ sampleSpecs/memberConstraints.smithy | 18 +++++++++++-- 2 files changed, 42 insertions(+), 2 deletions(-) create mode 100644 modules/bootstrapped/src/generated/smithy4s/example/HasConstrainedNewtypes.scala diff --git a/modules/bootstrapped/src/generated/smithy4s/example/HasConstrainedNewtypes.scala b/modules/bootstrapped/src/generated/smithy4s/example/HasConstrainedNewtypes.scala new file mode 100644 index 000000000..df513793e --- /dev/null +++ b/modules/bootstrapped/src/generated/smithy4s/example/HasConstrainedNewtypes.scala @@ -0,0 +1,26 @@ +package smithy4s.example + +import smithy4s.Hints +import smithy4s.Schema +import smithy4s.ShapeId +import smithy4s.ShapeTag +import smithy4s.schema.Schema.struct + +final case class HasConstrainedNewtypes(a: BucketName, b: CityId, c: Option[ObjectSize] = None, d: Option[IndexedSeq[String]] = None, e: Option[PNG] = None) + +object HasConstrainedNewtypes extends ShapeTag.Companion[HasConstrainedNewtypes] { + val id: ShapeId = ShapeId("smithy4s.example", "HasConstrainedNewtypes") + + val hints: Hints = Hints.empty + + // constructor using the original order from the spec + private def make(a: BucketName, b: CityId, c: Option[ObjectSize], d: Option[IndexedSeq[String]], e: Option[PNG]): HasConstrainedNewtypes = HasConstrainedNewtypes(a, b, c, d, e) + + implicit val schema: Schema[HasConstrainedNewtypes] = struct( + BucketName.schema.validated(smithy.api.Length(min = Some(1L), max = None)).required[HasConstrainedNewtypes]("a", _.a), + CityId.schema.validated(smithy.api.Length(min = Some(1L), max = None)).required[HasConstrainedNewtypes]("b", _.b), + ObjectSize.schema.validated(smithy.api.Range(min = Some(scala.math.BigDecimal(1.0)), max = None)).optional[HasConstrainedNewtypes]("c", _.c), + SomeIndexSeq.underlyingSchema.validated(smithy.api.Length(min = Some(1L), max = None)).optional[HasConstrainedNewtypes]("d", _.d), + PNG.schema.validated(smithy.api.Length(min = Some(1L), max = None)).optional[HasConstrainedNewtypes]("e", _.e), + )(make).withId(id).addHints(hints) +} diff --git a/sampleSpecs/memberConstraints.smithy b/sampleSpecs/memberConstraints.smithy index f0860cbcc..4e88204f0 100644 --- a/sampleSpecs/memberConstraints.smithy +++ b/sampleSpecs/memberConstraints.smithy @@ -15,8 +15,22 @@ map ConstrainedMap { } // Regression test for https://github.com/disneystreaming/smithy4s/issues/1594 -structure HasConstrainedNewtype { +structure HasConstrainedNewtypes { + // string newtype @length(min: 1) @required - s: CityId + a: BucketName + // string newtype, double-constrained + @length(min: 1) + @required + b: CityId + // int newtype + @range(min: 1) + c: ObjectSize + // list newtype, oh wait these are just lists. Still. + @length(min: 1) + d: SomeIndexSeq + // blob newtype + @length(min: 1) + e: PNG } From 946740939ff6ca931a386924ccedb43217ddfc9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Koz=C5=82owski?= Date: Fri, 27 Sep 2024 01:48:03 +0200 Subject: [PATCH 4/5] Add changelog entry --- CHANGELOG.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 935875af2..6d5a7d18e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,10 +5,14 @@ When adding entries, please treat them as if they could end up in a release any Thank you! +# 0.18.25 + +* Fixes an issue in which refinements wouldn't work on custom simple shapes (newtypes) (see [#1595](https://github.com/disneystreaming/smithy4s/pull/1595)) + # 0.18.24 * Adds missing nanoseconds in Document encoding of EPOCH_SECOND timestamps -* Add support for `alloy#jsonUnknown`, allowing structures to capture unknown JSON fields in one of their members. +* Add support for `alloy#jsonUnknown`, allowing structures to capture unknown JSON fields in one of their members. * Add `getMessage` implementation in `Smithy4sThrowable` which will be overridden in cases where the error structure contains a message field, but otherwise will be used to prevent a useless `null` result when `getMessage` is called. # 0.18.23 From 75052080a7af0376c6e6381bec6c7be7f7e02635 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Koz=C5=82owski?= Date: Fri, 27 Sep 2024 02:23:05 +0200 Subject: [PATCH 5/5] Keep method public, at least it's not implicit --- modules/core/src/smithy4s/RefinementProvider.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/core/src/smithy4s/RefinementProvider.scala b/modules/core/src/smithy4s/RefinementProvider.scala index 26d6600f5..c36af9594 100644 --- a/modules/core/src/smithy4s/RefinementProvider.scala +++ b/modules/core/src/smithy4s/RefinementProvider.scala @@ -203,7 +203,7 @@ private[smithy4s] trait LowPriorityImplicits { new RefinementProvider.PatternConstraint[E](e => e.value) @deprecated("Use isomorphismConstraint2 instead", "0.18.25") - private[smithy4s] def isomorphismConstraint[C, A, A0](implicit + def isomorphismConstraint[C, A, A0](implicit constraintOnA: RefinementProvider.Simple[C, A], iso: Bijection[A, A0] ): RefinementProvider[C, A0, A0] = isomorphismConstraint2