amnaredo / test

0 stars 0 forks source link

Control serialization of a certain property across many classes in a DRY manner #94

Open amnaredo opened 3 years ago

amnaredo commented 3 years ago

I am wondering what would be a good (=DRY) way to modify serialization and deserialization of a certain property in many classes.

I have a bunch of model classes of the following structure (somewhat simplified). I am persisting them using Slick, where such a structure is fairly common:

trait Entity {
  def id: Option[Long]
}

case class Person(fName: String, lName: String, id: Option[Long] = None) extends Entity

case class Organization(name: String, id: Option[Long] = None) extends Entity

// and so on...

Currently, uPickle serializes the id property as an array:

{
   ...
   id: [1234]
}

I am looking for a way to map the id property as follows:

{
   ...
   id: 1234
}

What would be a good way, short of writing either a custom apply() and unapply() and the attendant parallel set of classes or writing a Reader and Writer for each model class?

ID: 40 Original Author: ramnivas

amnaredo commented 3 years ago

If you want to override the serialization of something like option, you could shadow the option reader and writer with your own. Would that work? That would override the serialization of all options, but it sounds like maybe that's what you want

Original Author: lihaoyi

amnaredo commented 3 years ago

That might work in this particular situation, but a more precise control would have been better. For example, I would have liked to have properties with only the Entity.id path have the special treatment. Perhaps something akin to @key would be fine as well.

In any case, I am not sure how to get this working where I preserve the behavior of setting the deserialize object's property to the default value (None in this case).

package com.example

import upickle._

object Hello {
  trait Entity {
    def id: Option[Long]
  }

  case class Person(fName: String, lName: String, id: Option[Long] = None) extends Entity

  case class Organization(name: String, id: Option[Long] = None) extends Entity

  implicit val option2Writer = upickle.Writer[Option[Long]]{
    case None => Js.Null
    case Some(value) => Js.Num(value)
  }

  implicit val option2Reader = upickle.Reader[Option[Long]]{
    case Js.Null => None
    case Js.Num(num) => Some(num.toLong)
  }

  def main(args: Array[String]): Unit = {
    val p1 = Person("F", "L")
    val p2 = Person("F", "L", Some(1234))

    val p1written = write(p1)
    val p2written= write(p2)

    val p1read = read[Person](p1written)
    val p2read = read[Person](p2written)

    println(s"$p1 -> $p1written -> $p1read")
    println(s"$p2 -> $p2written -> $p2read")
  }
}

Running which, I get:

Person(F,L,None) -> {"fName":"F","lName":"L"} -> Person(F,L,null)
Person(F,L,Some(1234)) -> {"fName":"F","lName":"L","id":1234} -> Person(F,L,Some(1234))

Here, the first object's id property gets set to null instead of expected None.

Original Author: ramnivas

amnaredo commented 3 years ago

I'm not sure what that would look like. I guess you could extend @key to pass in a type optionally (can you even do that?) in addition to the string value; then you'd call @key[CustomReadWriter]("my-id") and instantiate CustomReadWriters to read and write the thing when necessary? It'd be tricky, because currently the macro Reader/Writer have lower priority than the implicit version, and this would necessitate a higher-priority macro to do a pre-processor pass on serializing every field before the implicits kick in.

For the second problem, it looks like you're being bitten by

https://github.com/lihaoyi/upickle/commit/689ebcdbdc2484410a1b5d81de1d1691b75c16f0#diff-5c20a012db0ef74e915344d710882fe5R43

Which was meant to fix

https://github.com/lihaoyi/upickle/issues/12

It should be possible to refactor the library to make it work the other way, i.e. by flipping the order to make the user-defined read get tried first before the default of null. I'm not sure what the semantics are for pattern matching on null, but I think it should be ok. If you could fix it and send a PR with a test that'd be awesome ^_^

Original Author: lihaoyi

amnaredo commented 3 years ago

Just made a PR to fix this problem. A part of the issue was Js.Null was semantically incapable of specifying that the value shouldn't be written. So introduced Js.None.

Original Author: ramnivas

amnaredo commented 3 years ago

I have some work on this in my branch here. It depends on #65, so I'm not submitting it yet.

It is based on a suggestion by @lihaoyi in #43: a new implicit configuration called Filter is added. It is applied to each field upon serialization and deserialization and it controls the way this field is serialized. For example, it can omit the field from the output entirely. There are already some Filter implementations. One of them, for example, allows serializing Option fields as absent keys. These filters can be combined with orElse, so it is possible to define custom behavior only for some kinds of fields (currently based on the type erasure only).

It required some rewrite of macros, and now most of GeneratedUtil and all of CaseNR and CaseNW are obsolete.

Original Author: netvl

amnaredo commented 3 years ago

"Serialization of properties across all classes" can now be coarsely controlled at the Js.Value level using Custom Configurations, an example of which is provided in https://github.com/lihaoyi/upickle-pprint/commit/f159ec35a59a8cacfb97da0c2665fcea033467b7

Let me know if that doesn't work for you or if you want something more fine-grained

Original Author: lihaoyi