fix decoder for nested item where all attributes are absent (#538)

* draft decoder fix * some cleanup * do not map empty AV map to None * aws mapping fix * more debugs * delete dead code * some cleanup * scalafmt * remove dead comment
zio · Dec 21, 2024 · f126f81 · f126f81
1 parent 59f9cbe
commit f126f81
Show file tree

Hide file tree

Showing 7 changed files with 80 additions and 35 deletions.
diff --git a/dynamodb/src/it/scala/zio/dynamodb/TypeSafeApiCrudSpec.scala b/dynamodb/src/it/scala/zio/dynamodb/TypeSafeApiCrudSpec.scala
@@ -16,6 +16,19 @@ import zio.Scope
 
 object TypeSafeApiCrudSpec extends DynamoDBLocalSpec {
 
+  case class PersonMetaData(address: Option[String], postcode: Option[String])
+  object PersonMetaData     {
+    implicit val schema: Schema.CaseClass2[Option[String], Option[String], PersonMetaData] =
+      DeriveSchema.gen[PersonMetaData]
+    val address                                                                            = ProjectionExpression.accessors[PersonMetaData]
+  }
+  case class PersonWithMetaData(id: String, personMetaData: PersonMetaData)
+  object PersonWithMetaData {
+    implicit val schema: Schema.CaseClass2[String, PersonMetaData, PersonWithMetaData] =
+      DeriveSchema.gen[PersonWithMetaData]
+    val (id, attributes)                                                               = ProjectionExpression.accessors[PersonWithMetaData]
+  }
+
   final case class Person(id: String, surname: String, forename: Option[String], age: Int)
   object Person {
     implicit val schema: Schema.CaseClass4[String, String, Option[String], Int, Person] = DeriveSchema.gen[Person]
@@ -406,6 +419,18 @@ object TypeSafeApiCrudSpec extends DynamoDBLocalSpec {
         } yield assertTrue(p == expected)
       }
     },
+    test(
+      "set a case class with a mandatory field of a case class where all fields are optional and set to None"
+    ) {
+      withSingleIdKeyTable { tableName =>
+        val person   = PersonWithMetaData("1", PersonMetaData(address = None, postcode = None))
+        val expected = person
+        for {
+          _ <- put(tableName, person).execute
+          p <- get(tableName)(PersonWithMetaData.id.partitionKey === "1").execute.absolve
+        } yield assertTrue(p == expected)
+      }
+    },
     test(
       "set a map element"
     ) {
@@ -414,12 +439,12 @@ object TypeSafeApiCrudSpec extends DynamoDBLocalSpec {
         val person   = PersonWithCollections("1", "Smith")
         val expected = person.copy(addressMap = Map(address1.number -> address1))
         for {
-          _ <- put(tableName, person).execute
-          _ <- update(tableName)(PersonWithCollections.id.partitionKey === "1")(
-                 PersonWithCollections.addressMap.valueAt(address1.number).set(address1)
-               ).execute
-          p <- get(tableName)(PersonWithCollections.id.partitionKey === "1").execute.absolve
-        } yield assertTrue(p == expected)
+          _     <- put(tableName, person).execute
+          _     <- update(tableName)(PersonWithCollections.id.partitionKey === "1")(
+                     PersonWithCollections.addressMap.valueAt(address1.number).set(address1)
+                   ).execute
+          found <- get(tableName)(PersonWithCollections.id.partitionKey === "1").execute.absolve
+        } yield assertTrue(found == expected)
       }
     },
     test(

diff --git a/dynamodb/src/main/scala/zio/dynamodb/Codec.scala b/dynamodb/src/main/scala/zio/dynamodb/Codec.scala
@@ -698,9 +698,11 @@ private[dynamodb] object Codec {
         }
 
     private def sequenceDecoder[Col, A](decoder: Decoder[A], to: Chunk[A] => Col): Decoder[Col] = {
-      case AttributeValue.List(list) =>
+      case AttributeValue.List(list)              =>
         list.forEach(decoder(_)).map(xs => to(Chunk.fromIterable(xs)))
-      case av                        => Left(DecodingError(s"unable to decode $av as a list"))
+      // Low level AWS API will return an empty map for an empty list so we need to handle this case
+      case AttributeValue.Map(map) if map.isEmpty => Right(to(Chunk.empty))
+      case av                                     => Left(DecodingError(s"unable to decode $av as a list"))
     }
 
     private def setDecoder[A](s: Schema[A]): Decoder[Set[A]] = {

diff --git a/dynamodb/src/main/scala/zio/dynamodb/DynamoDBExecutorImpl.scala b/dynamodb/src/main/scala/zio/dynamodb/DynamoDBExecutorImpl.scala
@@ -1001,25 +1001,23 @@ case object DynamoDBExecutorImpl {
       .orElse {
         attributeValue.bs.flatMap(bs => toOption(bs).map(bs => AttributeValue.BinarySet(bs.toSet)))
       }
-      .orElse {
-        attributeValue.m.flatMap(m =>
-          toOption(m).map(m =>
-            AttributeValue.Map(
-              m.flatMap {
-                case (k, v) =>
-                  awsAttrValToAttrVal(v).map(attrVal => (AttributeValue.String(k), attrVal))
-              }
-            )
-          )
-        )
-      }
       .orElse {
         attributeValue.l.flatMap(l =>
           toOption(l).map(l => AttributeValue.List(Chunk.fromIterable(l.flatMap(awsAttrValToAttrVal))))
         )
       }
       .orElse(attributeValue.nul.map(_ => AttributeValue.Null))
       .orElse(attributeValue.bool.map(AttributeValue.Bool.apply))
+      .orElse {
+        attributeValue.m.flatMap(m =>
+          AttributeValue.Map(
+            m.flatMap {
+              case (k, v) =>
+                awsAttrValToAttrVal(v).map(attrVal => (AttributeValue.String(k), attrVal))
+            }
+          )
+        )
+      }
       .toOption
 
   private def awsReturnItemCollectionMetrics(metrics: ReturnItemCollectionMetrics): ZIOAwsReturnItemCollectionMetrics =

diff --git a/dynamodb/src/test/scala/zio/dynamodb/codec/CodecTestFixtures.scala b/dynamodb/src/test/scala/zio/dynamodb/codec/CodecTestFixtures.scala
@@ -38,24 +38,26 @@ trait CodecTestFixtures {
     ]("boolean")(_.asInstanceOf[Boolean])(_.asInstanceOf[Any])(_.isInstanceOf[Boolean])
   )
 
-  lazy implicit val caseClassOfCurrencySchema: Schema[CaseClassOfCurrency]         = DeriveSchema.gen[CaseClassOfCurrency]
-  lazy implicit val nestedCaseClass2Schema: Schema[NestedCaseClass2]               = DeriveSchema.gen[NestedCaseClass2]
-  lazy implicit val simpleCaseClass3Schema: Schema[SimpleCaseClass3]               = DeriveSchema.gen[SimpleCaseClass3]
-  lazy implicit val simpleCaseClass3SchemaOptional: Schema[SimpleCaseClass3Option] =
+  lazy implicit val caseClassOfCurrencySchema: Schema[CaseClassOfCurrency]                         = DeriveSchema.gen[CaseClassOfCurrency]
+  lazy implicit val nestedCaseClass2Schema: Schema[NestedCaseClass2]                               = DeriveSchema.gen[NestedCaseClass2]
+  lazy implicit val simpleCaseClass3Schema: Schema[SimpleCaseClass3]                               = DeriveSchema.gen[SimpleCaseClass3]
+  lazy implicit val simpleCaseClass3SchemaOptional: Schema[SimpleCaseClass3Option]                 =
     DeriveSchema.gen[SimpleCaseClass3Option]
-  lazy implicit val caseClassOfChunk: Schema[CaseClassOfChunk]                     = DeriveSchema.gen[CaseClassOfChunk]
-  lazy implicit val caseClassOfList: Schema[CaseClassOfList]                       = DeriveSchema.gen[CaseClassOfList]
-  lazy implicit val caseClassOfListOfCaseClass: Schema[CaseClassOfListOfCaseClass] =
+  lazy implicit val caseClassOfChunk: Schema[CaseClassOfChunk]                                     = DeriveSchema.gen[CaseClassOfChunk]
+  lazy implicit val caseClassOfList: Schema[CaseClassOfList]                                       = DeriveSchema.gen[CaseClassOfList]
+  lazy implicit val caseClassOfListOfCaseClass: Schema[CaseClassOfListOfCaseClass]                 =
     DeriveSchema.gen[CaseClassOfListOfCaseClass]
-  lazy implicit val caseClassOfOption: Schema[CaseClassOfOption]                   = DeriveSchema.gen[CaseClassOfOption]
-  lazy implicit val caseClassOfNestedOption: Schema[CaseClassOfNestedOption]       = DeriveSchema.gen[CaseClassOfNestedOption]
-  lazy implicit val caseClassOfEither: Schema[CaseClassOfEither]                   = DeriveSchema.gen[CaseClassOfEither]
-  lazy implicit val caseClassOfTuple2: Schema[CaseClassOfTuple2]                   = DeriveSchema.gen[CaseClassOfTuple2]
-  lazy implicit val caseClassOfTuple3: Schema[CaseClassOfTuple3]                   = DeriveSchema.gen[CaseClassOfTuple3]
-  lazy implicit val instantSchema: Schema.Primitive[Instant]                       =
+  lazy implicit val caseClassOfOption: Schema[CaseClassOfOption]                                   = DeriveSchema.gen[CaseClassOfOption]
+  lazy implicit val caseClassOfNestedOption: Schema[CaseClassOfNestedOption]                       = DeriveSchema.gen[CaseClassOfNestedOption]
+  lazy implicit val caseClassOfNestedCaseClassOfOption: Schema[CaseClassOfNestedCaseClassOfOption] =
+    DeriveSchema.gen[CaseClassOfNestedCaseClassOfOption]
+  lazy implicit val caseClassOfEither: Schema[CaseClassOfEither]                                   = DeriveSchema.gen[CaseClassOfEither]
+  lazy implicit val caseClassOfTuple2: Schema[CaseClassOfTuple2]                                   = DeriveSchema.gen[CaseClassOfTuple2]
+  lazy implicit val caseClassOfTuple3: Schema[CaseClassOfTuple3]                                   = DeriveSchema.gen[CaseClassOfTuple3]
+  lazy implicit val instantSchema: Schema.Primitive[Instant]                                       =
     Schema.Primitive(StandardType.InstantType)
-  lazy implicit val caseClassOfInstant: Schema[CaseClassOfInstant]                 = DeriveSchema.gen[CaseClassOfInstant]
-  lazy implicit val caseClassOfStatus: Schema[CaseClassOfStatus]                   = DeriveSchema.gen[CaseClassOfStatus]
+  lazy implicit val caseClassOfInstant: Schema[CaseClassOfInstant]                                 = DeriveSchema.gen[CaseClassOfInstant]
+  lazy implicit val caseClassOfStatus: Schema[CaseClassOfStatus]                                   = DeriveSchema.gen[CaseClassOfStatus]
 
   implicit val caseClassOfMapOfInt: Schema[CaseClassOfMapOfInt] = DeriveSchema.gen[CaseClassOfMapOfInt]
 

diff --git a/dynamodb/src/test/scala/zio/dynamodb/codec/ItemDecoderSpec.scala b/dynamodb/src/test/scala/zio/dynamodb/codec/ItemDecoderSpec.scala
@@ -72,6 +72,15 @@ object ItemDecoderSpec extends ZIOSpecDefault with CodecTestFixtures {
 
       assert(actual)(isRight(equalTo(expected)))
     },
+    test("decodes nested case class of an Optional field which is None") {
+      val expected = CaseClassOfNestedCaseClassOfOption(id = 1, opt = CaseClassOfOption(None))
+
+      val actual = DynamoDBQuery.fromItem[CaseClassOfNestedCaseClassOfOption](
+        Item("id" -> 1, "opt" -> Map.empty[String, AttributeValue])
+      )
+
+      assert(actual)(isRight(equalTo(expected)))
+    },
     test("decoded option of None where value is null") {
       val expected = CaseClassOfOption(None)
 

diff --git a/dynamodb/src/test/scala/zio/dynamodb/codec/ItemEncoderSpec.scala b/dynamodb/src/test/scala/zio/dynamodb/codec/ItemEncoderSpec.scala
@@ -58,6 +58,13 @@ object ItemEncoderSpec extends ZIOSpecDefault with CodecTestFixtures {
 
       assert(item)(equalTo(expectedItem))
     },
+    test("encodes nested case class of all Optional fields which are all None") {
+      val expected = Item("id" -> 1, "opt" -> Map.empty[String, AttributeValue])
+
+      val item = DynamoDBQuery.toItem(CaseClassOfNestedCaseClassOfOption(id = 1, opt = CaseClassOfOption(None)))
+
+      assertTrue(item == expected)
+    },
     test("encodes simple Item") {
       val expectedItem: Item = Item("id" -> 2, "name" -> "Avi", "flag" -> true)
 

diff --git a/dynamodb/src/test/scala/zio/dynamodb/codec/models.scala b/dynamodb/src/test/scala/zio/dynamodb/codec/models.scala
@@ -33,6 +33,8 @@ final case class CaseClassOfOption(opt: Option[Int])
 
 final case class CaseClassOfNestedOption(opt: Option[Option[Int]])
 
+final case class CaseClassOfNestedCaseClassOfOption(id: Int, opt: CaseClassOfOption)
+
 final case class CaseClassOfEither(either: Either[String, Int])
 
 final case class CaseClassOfTuple3(tuple: (Int, Int, Int))