Skip to content
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.

Sign up for GitHub

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

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

Merged
merged 9 commits into from
Dec 21, 2024
Merged

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
46e48f2
draft decoder fix
googley42 Nov 15, 2024
3cfa45b
some cleanup
googley42 Nov 16, 2024
9f65755
do not map empty AV map to None
googley42 Nov 16, 2024
aac166d
aws mapping fix
googley42 Nov 21, 2024
f3fe3d7
more debugs
googley42 Nov 22, 2024
ec8bb30
delete dead code
googley42 Nov 22, 2024
d9ce435
some cleanup
googley42 Nov 23, 2024
4cbcea5
scalafmt
googley42 Nov 23, 2024
21eca64
remove dead comment
googley42 Nov 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
some cleanup
  • Loading branch information
@googley42
googley42 committed Nov 16, 2024
commit 3cfa45b3c62dd1f8e1c4d95b3f7105d31d31a849
33 changes: 19 additions & 14 deletions dynamodb/src/it/scala/zio/dynamodb/TypeSafeApiCrudSpec.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,18 @@ import zio.dynamodb.DynamoDBQuery.getItem

object TypeSafeApiCrudSpec extends DynamoDBLocalSpec {

case class PersonAttributes(address: Option[String])
object PersonAttributes {
implicit val schema: Schema.CaseClass1[Option[String], PersonAttributes] = DeriveSchema.gen[PersonAttributes]
val address = ProjectionExpression.accessors[PersonAttributes]
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]
}
// attributes is a MANDATORY field of an item with ALL fields optional
case class PersonWithAttributes(id: String, attributes: PersonAttributes)
object PersonWithAttributes {
implicit val schema: Schema.CaseClass2[String, PersonAttributes, PersonWithAttributes] =
DeriveSchema.gen[PersonWithAttributes]
val (id, attributes) = ProjectionExpression.accessors[PersonWithAttributes]
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)
Expand Down Expand Up @@ -421,17 +422,21 @@ object TypeSafeApiCrudSpec extends DynamoDBLocalSpec {
}
},
test(
"set an empty case class"
"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 = PersonWithAttributes("1", PersonAttributes(None))
val person = PersonWithMetaData("1", PersonMetaData(None, None))
val expected = person
for {
_ <- put(tableName, person).execute
item <- getItem(tableName, PrimaryKey("id" -> "1")).execute
_ = println(s"XXXXXXX item: $item")
p <- get(tableName)(PersonWithAttributes.id.partitionKey === "1").execute.absolve
} yield assertTrue(p == expected)
_ = println(s"item = $item")
_ = println(item)
p <- get(tableName)(PersonWithMetaData.id.partitionKey === "1").execute.absolve
} yield assertTrue(
p == expected //,
//item == Some(Item("id" -> "1"))
)
}
},
test(
Expand Down
88 changes: 34 additions & 54 deletions dynamodb/src/main/scala/zio/dynamodb/Codec.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -403,31 +403,43 @@ private[dynamodb] object Codec {

sealed trait FieldType
object FieldType {
case object Optional extends FieldType
case object Sequence extends FieldType
case object Chunk extends FieldType
case object Map extends FieldType
case object Set extends FieldType
final case class RecordWithAllFieldsOptional[R](s: Schema.Record[R]) extends FieldType
case object Scalar extends FieldType
case object Optional extends FieldType
case object Sequence extends FieldType
case object Chunk extends FieldType
case object Map extends FieldType
case object Set extends FieldType
final case class RecordWithAllFieldsOptional[R](nonLazySchema: Schema.Record[R]) extends FieldType
case object Scalar extends FieldType

def fromSchema[B](schema: Schema[B]): FieldType =
schema match {
case l @ Schema.Lazy(_) =>
case l @ Schema.Lazy(_) =>
fromSchema(l.schema)
case _: Schema.Optional[_] =>
case _: Schema.Optional[_] =>
Optional
case _: Schema.Map[_, _] =>
case _: Schema.Map[_, _] =>
Map
case _: Schema.Set[_] =>
case _: Schema.Set[_] =>
Set
case seq: Schema.Sequence[_, _, _] =>
case seq: Schema.Sequence[_, _, _] =>
if (seq.identity == "Chunk") FieldType.Chunk else FieldType.Sequence
case s: Schema.Record[_] if areAllFieldsInRecordAreOptional(s) =>
case s: Schema.Record[_] if allFieldsInRecordAreOptional(s) =>
RecordWithAllFieldsOptional(s)
case _ =>
case _ =>
Scalar
}

private def allFieldsInRecordAreOptional[R](s: Schema.Record[R]): Boolean = {
def isOptionalSchema(s: Schema[_]): Boolean =
s match {
case l @ Schema.Lazy(_) => isOptionalSchema(l.schema)
case Schema.Optional(_, _) => true
case _ => false
}
s.fields.forall { f =>
isOptionalSchema(f.schema)
}
}
}

def apply[A](schema: Schema[A]): Decoder[A] = decoder(schema)
Expand Down Expand Up @@ -917,15 +929,18 @@ private[dynamodb] object Codec {
av: AttributeValue,
fields: Schema.Field[_, _]*
): Either[ItemError, List[Any]] = {
println(s"XXXXXXXXX decodeFields fields: $fields av: $av")
def createAllOptionalRecordWithNones[R](s: Schema.Record[R]): Either[String, R] = {
val fieldValues: Chunk[Option[_]] = Chunk.fill[Option[_]](s.fields.size)(None)
zio.Unsafe.unsafe(implicit u => s.construct(fieldValues))
}

av match {
case AttributeValue.Map(map) =>
fields.toList.forEach {
case Schema.Field(key, schema, _, _, _, _) =>
val dec = decoder(schema)
val k = key // @fieldName is respected by the zio-schema macro
val maybeAv = map.get(AttributeValue.String(k))
println(s"XXXXXXXXX k: $k maybeAv: $maybeAv map: $map")
val errorOrValue = maybeAv.toRight(DecodingError(s"field '$k' not found in $av")).flatMap(dec)
if (maybeAv.isEmpty)
FieldType.fromSchema(schema) match {
Expand All @@ -935,55 +950,20 @@ private[dynamodb] object Codec {
case FieldType.Map => Right(Map.empty)
case FieldType.Set => Right(Set.empty)
case FieldType.RecordWithAllFieldsOptional(s) =>
println(s"QQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQ")
createAllOptionalRecordWithNones(s).left.map(DecodingError.apply)
case FieldType.Scalar => // TODO: Avi - decode record with empty map - may need a new decoder where field name is absent
// schema match {
// case l @ Schema.Lazy(_) =>
// l.schema match {
// case s: Schema.Record[_] if areAllFieldsInRecordAreOptional(s) =>
// // TODO: check all fields are optional

// println(s"XXXXXXXXX 1 isRecord: schema: ${l.schema} field count: ${s.fields.size}}")
// val x = createAllOptionalRecord(s).left.map(e => DecodingError(e))
// println(s"XXXXXXXXX x: $x")
// x
// case _ =>
// println(s"XXXXXXXXX 2 isRecord: schema: ${l.schema}")
// errorOrValue
// }
// case _ =>
// println(s"XXXXXXXXX 3 isRecord")
// errorOrValue
// }
case FieldType.Scalar =>
errorOrValue
}
else {
println(s"XXXXXXXXX 4 maybeAv is not empty")
else
errorOrValue
}
}
.map(_.toList)
case _ =>
Left(DecodingError(s"$av is not an AttributeValue.Map"))
}
}

private def areAllFieldsInRecordAreOptional[R](s: Schema.Record[R]): Boolean = {
def isOptionalSchema(s: Schema[_]): Boolean =
s match {
case l @ Schema.Lazy(_) => isOptionalSchema(l.schema)
case Schema.Optional(_, _) => true
case _ => false
}
s.fields.forall { f =>
isOptionalSchema(f.schema)
}
}

private def createAllOptionalRecordWithNones[R](s: Schema.Record[R]): Either[String, R] = {
val fieldValues: Chunk[Option[_]] = Chunk.fill[Option[_]](s.fields.size)(None)
zio.Unsafe.unsafe(implicit u => s.construct(fieldValues))
}
} // end Decoder

private def isCaseObject(s: Schema.Case[_, _]) =
Expand Down
32 changes: 26 additions & 6 deletions dynamodb/src/main/scala/zio/dynamodb/DynamoDBExecutorImpl.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,15 @@ private[dynamodb] final case class DynamoDBExecutorImpl private[dynamodb] (dynam
private def executeGetItem(getItem: GetItem): ZIO[Any, Throwable, Option[Item]] =
dynamoDb
.getItem(awsGetItemRequest(getItem))
.mapBoth(_.toThrowable, _.item.map(dynamoDBItem))
.mapBoth(
_.toThrowable,
{ response =>
println(s"ZZZZZZZZ executeGetItem: ${response.item}")
val x = response.item.map(dynamoDBItem)
println(s"ZZZZZZZZ executeGetItem: x=$x")
x
}
)
.map(_.toOption.flatMap(item => if (item.map.isEmpty) None else Some(item)))

private def executeUpdateItem(updateItem: UpdateItem): ZIO[Any, Throwable, Option[Item]] =
Expand Down Expand Up @@ -699,7 +707,11 @@ case object DynamoDBExecutorImpl {
}

private def dynamoDBItem(attrMap: ScalaMap[ZIOAwsAttributeName, ZIOAwsAttributeValue.ReadOnly]): Item =
Item(attrMap.flatMap { case (k, v) => awsAttrValToAttrVal(v).map(attrVal => (k.toString, attrVal)) })
Item(attrMap.flatMap {
case (k, v) =>
println(s"ZZZZZZZZ dynamoDBItem k=$k v=$v")
awsAttrValToAttrVal(v).map(attrVal => (k.toString, attrVal))
})

implicit class ToZioAwsMap(item: AttrMap) {
def toZioAwsMap(): ScalaMap[ZIOAwsAttributeName, ZIOAwsAttributeValue] =
Expand All @@ -722,8 +734,11 @@ case object DynamoDBExecutorImpl {
}

private def awsGetItemRequest(getItem: GetItem): GetItemRequest = {
val (aliasMap, projections: List[String]) =
AliasMapRender.forEach(getItem.projections.toList).execute
val (aliasMap, projections: List[String]) = {
val x = AliasMapRender.forEach(getItem.projections.toList).execute
println(s"ZZZZZZZZZ getItem: $x")
x
}

GetItemRequest(
tableName = TableArn(getItem.tableName.value),
Expand Down Expand Up @@ -977,8 +992,12 @@ case object DynamoDBExecutorImpl {

private[dynamodb] def awsAttributeValueMap(
attrMap: ScalaMap[String, AttributeValue]
): ScalaMap[ZIOAwsAttributeName, ZIOAwsAttributeValue] =
attrMap.flatMap { case (k, v) => awsAttributeValue(v).map(a => (ZIOAwsAttributeName(k), a)) }
): ScalaMap[ZIOAwsAttributeName, ZIOAwsAttributeValue] = {
println(s"ZZZZZZZZ attrMap: $attrMap")
val awsAttrMap = attrMap.flatMap { case (k, v) => awsAttributeValue(v).map(a => (ZIOAwsAttributeName(k), a)) }
println(s"ZZZZZZZZ awsAttrMap: $awsAttrMap")
awsAttrMap
}

private def awsAttrValToAttrVal(attributeValue: ZIOAwsAttributeValue.ReadOnly): Option[AttributeValue] =
attributeValue.s
Expand Down Expand Up @@ -1191,6 +1210,7 @@ case object DynamoDBExecutorImpl {
private def toOption[A](set: Set[A]): Option[Set[A]] =
if (set.isEmpty) None else Some(set)

// TODO: Avinder - WTF!!!!!!!! Why is an empty MAP getting mapped to None ??????? WTF???? WTF ?????
private def toOption[A, B](map: ScalaMap[A, B]): Option[ScalaMap[A, B]] =
if (map.isEmpty) None else Some(map)

Expand Down
2 changes: 1 addition & 1 deletion dynamodb/src/test/scala/zio/dynamodb/codec/ItemEncoderSpec.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ object ItemEncoderSpec extends ZIOSpecDefault with CodecTestFixtures {
test("encodes nested case class of Optional Item") {
//val expectedItem: Item = Item("opt" -> Item())

val item = DynamoDBQuery.toItem(CaseClassOfNestedCaseClassOfOption(opt = CaseClassOfOption(None)))
val item = DynamoDBQuery.toItem(CaseClassOfNestedCaseClassOfOption(id = 1, opt = CaseClassOfOption(None)))

println(s"XXXXXXXXX item: $item")
// with Some -> AttrMap(Map(opt -> Map(Map(String(opt) -> Number(1)))))
Expand Down
2 changes: 1 addition & 1 deletion dynamodb/src/test/scala/zio/dynamodb/codec/models.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ final case class CaseClassOfOption(opt: Option[Int])

final case class CaseClassOfNestedOption(opt: Option[Option[Int]])

final case class CaseClassOfNestedCaseClassOfOption(opt: CaseClassOfOption)
final case class CaseClassOfNestedCaseClassOfOption(id: Int, opt: CaseClassOfOption)

final case class CaseClassOfEither(either: Either[String, Int])

Expand Down