Fix nullable decoding

CHANGELOG.md

@@ -5,6 +5,10 @@ When adding entries, please treat them as if they could end up in a release any
 
 Thank you!
 
+# 0.18.29
+
+* Fix for decoding of required nullable fields and some combinations of refinements with nullable fields (see [#1637](https://github.com/disneystreaming/smithy4s/pull/1637))
+
 # 0.18.28
 
 * Better support for timestamps before Linux Epoch and trimming the Timestamp nanosecond part (see [#1623](https://github.com/disneystreaming/smithy4s/pull/1623))

modules/bootstrapped/test/src/smithy4s/DocumentSpec.scala

@@ -953,6 +953,120 @@ class DocumentSpec() extends FunSuite {
     assertEquals(doc, expected)
   }
 
+  test(
+    "Required nullable field with NO default should NOT infer null when value missing"
+  ) {
+    case class Foo(str: Nullable[String])
+    implicit val fieldSchema: Schema[Foo] =
+      Schema
+        .struct[Foo](
+          Schema.string.nullable
+            .required[Foo]("str", _.str)
+        )(Foo.apply)
+
+    val result = Document.decode[Foo](Document.DObject(Map.empty))
+    expect.same(
+      result,
+      Left(
+        smithy4s.codecs.PayloadError(
+          smithy4s.codecs.PayloadPath.parse(".str"),
+          "str",
+          "Required field not found"
+        )
+      )
+    )
+  }
+
+  test(
+    "Required nullable field WITH null default SHOULD infer null when value missing"
+  ) {
+    case class Foo(str: Nullable[String])
+    implicit val fieldSchema: Schema[Foo] =
+      Schema
+        .struct[Foo](
+          Schema.string.nullable
+            .required[Foo]("str", _.str)
+            .addHints(smithy.api.Default(Document.DNull))
+        )(Foo.apply)
+
+    val result = Document.decode[Foo](Document.DObject(Map.empty))
+    expect.same(
+      result,
+      Right(Foo(Nullable.Null))
+    )
+  }
+
+  test(
+    "Required refined field with null default"
+  ) {
+    case class Test()
+    object Test extends ShapeTag.Companion[Test] {
+      def id: ShapeId = ShapeId("test", "Test")
+      def schema: Schema[Test] = Schema.constant(Test())
+    }
+    case class Foo(str: String)
+    case class Bar(foo: Foo)
+    implicit val provider: RefinementProvider[Test, String, Foo] =
+      Refinement.drivenBy[Test](str => Right(Foo.apply(str)), _.str)
+    val fieldSchema: smithy4s.schema.Field[Bar, Foo] =
+      Schema.string
+        .refined[Foo](
+          Test()
+        )
+        .required[Bar]("foo", _.foo)
+        .addHints(smithy.api.Default(Document.DNull))
+    implicit val schema: Schema[Bar] =
+      Schema.struct[Bar](fieldSchema)(Bar.apply)
+
+    expect.same(
+      Document.decode[Bar](
+        Document.DObject(Map("foo" -> Document.fromString("test")))
+      ),
+      Right(Bar(Foo("test")))
+    )
+    expect.same(
+      Document.decode[Bar](Document.DObject(Map.empty)),
+      // Empty string here because null default is implied to be empty string
+      // for a non-nullable string field
+      Right(Bar(Foo("")))
+    )
+  }
+
+  test(
+    "Nullable required refined field with null default"
+  ) {
+    case class Test()
+    object Test extends ShapeTag.Companion[Test] {
+      def id: ShapeId = ShapeId("test", "Test")
+      def schema: Schema[Test] = Schema.constant(Test())
+    }
+    case class Foo(str: String)
+    case class Bar(foo: Nullable[Foo])
+    implicit val provider: RefinementProvider[Test, String, Foo] =
+      Refinement.drivenBy[Test](str => Right(Foo.apply(str)), _.str)
+    val fieldSchema: smithy4s.schema.Field[Bar, Nullable[Foo]] =
+      Schema.string
+        .refined[Foo](
+          Test()
+        )
+        .nullable
+        .required[Bar]("foo", _.foo)
+        .addHints(smithy.api.Default(Document.DNull))
+    implicit val schema: Schema[Bar] =
+      Schema.struct[Bar](fieldSchema)(Bar.apply)
+
+    expect.same(
+      Document.decode[Bar](
+        Document.DObject(Map("foo" -> Document.fromString("test")))
+      ),
+      Right(Bar(Nullable.value(Foo("test"))))
+    )
+    expect.same(
+      Document.decode[Bar](Document.DObject(Map.empty)),
+      Right(Bar(Nullable.Null))
+    )
+  }
+
   private def inside[A, B](
       a: A
   )(assertPF: PartialFunction[A, Unit])(implicit loc: munit.Location) = {

modules/bootstrapped/test/src/smithy4s/schema/DefaultValueSpec.scala

@@ -133,7 +133,7 @@ final class DefaultValueSpec extends FunSuite {
   test("refined") {
     val b: Schema[Int] =
       Schema.int.refined(smithy.api.Range(None, Option(BigDecimal(1))))
-    testCaseOpt(b, None)
+    testCaseOpt(b, Some(0))
   }
 
   test("recursive") {

modules/core/src/smithy4s/schema/DefaultValueSchemaVisitor.scala

@@ -89,7 +89,8 @@ private[schema] object DefaultValueSchemaVisitor extends SchemaVisitor[Option] {
   def refine[A, B](
       schema: Schema[A],
       refinement: Refinement[A, B]
-  ): Option[B] = None
+  ): Option[B] =
+    schema.compile(this).flatMap(refinement.apply(_).toOption)
 
   def lazily[A](suspend: Lazy[Schema[A]]): Option[A] =
     suspend.map(_.compile(this)).value

modules/core/src/smithy4s/schema/Schema.scala

@@ -354,8 +354,9 @@ object Schema {
   private object OptionDefaultVisitor extends SchemaVisitor.Default[Option] {
     def default[A] : Option[A] = None
     override def option[A](schema: Schema[A]) : Option[Option[A]] = Some(None)
-    override def biject[A, B](schema: Schema[A], bijection: Bijection[A, B]): Option[B] =
-      this.apply(schema).map(bijection.to)
+    override def biject[A, B](schema: Schema[A], bijection: Bijection[A, B]): Option[B] = {
+      if (schema.hints.has[alloy.Nullable]) None else this.apply(schema).map(bijection.to)
+    }
   }
 
   def operation(id: ShapeId): OperationSchema[Unit, Nothing, Unit, Nothing, Nothing] =

modules/json/test/src/smithy4s/json/SchemaVisitorJCodecTests.scala

@@ -43,6 +43,7 @@ import scala.collection.immutable.ListMap
 import scala.util.Try
 import smithy4s.json.internals.JsoniterCodecCompilerImpl
 import smithy4s.schema.Schema
+import smithy4s.schema.Field
 
 class SchemaVisitorJCodecTests() extends FunSuite {
 
@@ -836,6 +837,116 @@ class SchemaVisitorJCodecTests() extends FunSuite {
     expect.same(result, json)
   }
 
+  test(
+    "Required nullable field with NO default should NOT infer null when value missing"
+  ) {
+    case class Foo(str: Nullable[String])
+    implicit val fieldSchema: Schema[Foo] =
+      Schema
+        .struct[Foo](
+          Schema.string.nullable
+            .required[Foo]("str", _.str)
+        )(Foo.apply)
+
+    val result = util.Try(readFromString[Foo]("{}")).toEither
+    expect.same(
+      result,
+      Left(
+        PayloadError(
+          PayloadPath.parse(".str"),
+          "str",
+          "Missing required field"
+        )
+      )
+    )
+  }
+
+  test(
+    "Required nullable field WITH null default SHOULD infer null when value missing"
+  ) {
+    case class Foo(str: Nullable[String])
+    implicit val fieldSchema: Schema[Foo] =
+      Schema
+        .struct[Foo](
+          Schema.string.nullable
+            .required[Foo]("str", _.str)
+            .addHints(smithy.api.Default(Document.DNull))
+        )(Foo.apply)
+
+    val result = readFromString[Foo]("{}")
+    expect.same(
+      result,
+      Foo(Nullable.Null)
+    )
+  }
+
+  test(
+    "Required refined field with null default"
+  ) {
+    case class Test()
+    object Test extends ShapeTag.Companion[Test] {
+      def id: ShapeId = ShapeId("test", "Test")
+      def schema: Schema[Test] = Schema.constant(Test())
+    }
+    case class Foo(str: String)
+    case class Bar(foo: Foo)
+    implicit val provider: RefinementProvider[Test, String, Foo] =
+      Refinement.drivenBy[Test](str => Right(Foo.apply(str)), _.str)
+    val fieldSchema: Field[Bar, Foo] =
+      Schema.string
+        .refined[Foo](
+          Test()
+        )
+        .required[Bar]("foo", _.foo)
+        .addHints(smithy.api.Default(Document.DNull))
+    implicit val schema: Schema[Bar] =
+      Schema.struct[Bar](fieldSchema)(Bar.apply)
+
+    expect.same(
+      readFromString[Bar]("{\"foo\":\"test\"}"),
+      Bar(Foo("test"))
+    )
+    expect.same(
+      readFromString[Bar]("{}"),
+      // Empty string here because null default is implied to be empty string
+      // for a non-nullable string field
+      Bar(Foo(""))
+    )
+  }
+
+  test(
+    "Nullable required refined field with null default"
+  ) {
+    case class Test()
+    object Test extends ShapeTag.Companion[Test] {
+      def id: ShapeId = ShapeId("test", "Test")
+      def schema: Schema[Test] = Schema.constant(Test())
+    }
+    case class Foo(str: String)
+    case class Bar(foo: Nullable[Foo])
+    implicit val provider: RefinementProvider[Test, String, Foo] =
+      Refinement.drivenBy[Test](str => Right(Foo.apply(str)), _.str)
+    val fieldSchema: Field[Bar, Nullable[Foo]] =
+      Schema.string
+        .refined[Foo](
+          Test()
+        )
+        .nullable
+        .required[Bar]("foo", _.foo)
+        .addHints(smithy.api.Default(Document.DNull))
+    implicit val schema: Schema[Bar] =
+      Schema.struct[Bar](fieldSchema)(Bar.apply)
+
+    expect.same(
+      readFromString[Bar]("{\"foo\":\"test\"}"),
+      Bar(Nullable.value(Foo("test")))
+    )
+    expect.same(
+      readFromString[Bar]("{}"),
+      Bar(Nullable.Null)
+    )
+  }
+
   case class Patchable(a: Option[Nullable[Int]])
 
   object Patchable {