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.

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

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

Closed
ramnivas opened this issue Nov 15, 2014 · 6 comments

Comments

@ramnivas
Copy link
Contributor

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:

  • If id is None, omit the property. uPickle already does this, since the id property's default is None.
  • If id is Some(value), just serialize the value:
{
   ...
   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?

@lihaoyi
Copy link
Member

lihaoyi commented Nov 15, 2014

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

@ramnivas
Copy link
Contributor Author

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.

@lihaoyi
Copy link
Member

lihaoyi commented Nov 15, 2014

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

689ebcd#diff-5c20a012db0ef74e915344d710882fe5R43

Which was meant to fix

#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 ^_^

ramnivas added a commit to ramnivas/upickle that referenced this issue Nov 18, 2014
+ Fixes com-lihaoyi#40
+ Introduces Js.None, since Js.Null has semantics of Javascript null.
  This allows custom writers to return Js.None to specify that the
  value should be writen to the output.
@ramnivas
Copy link
Contributor Author

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.

ramnivas added a commit to ramnivas/upickle that referenced this issue Nov 18, 2014
+ Fixes com-lihaoyi#40
+ Introduces Js.None, since Js.Null has semantics of Javascript null.
  This allows custom writers to return Js.None to specify that the
  value should not be writen to the output.
@netvl
Copy link

netvl commented Jan 27, 2015

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.

@lihaoyi
Copy link
Member

lihaoyi commented Jan 24, 2016

"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 f159ec3

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants