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

Surprising behaviour when deserializing an Option from null when it contains a default value #1130

Open
dleblanc opened this issue Mar 22, 2024 · 6 comments
Labels

Comments

@dleblanc
Copy link

With an example case class like:

case class Ex(opt: Option[String])

If I deserialize an optional value with "null", then I get the object built with 'opt' set to None, as expected.

However, if the optional field has a default value:

case class Ex(opt: Option[String] = Some("hiya"))

Then when I deserialize from json with null for that field, I get opt = Some("hiya") instead of None. This seems surprising to me, as I'd expect the default value to apply only when there is no value present for the field, but not when null is explicitly provided.

Is there a combination of configuration parameters I can use to achieve this? All combos of withTransient* didn't seem to help me here.

@plokhotnyuk
Copy link
Owner

plokhotnyuk commented Mar 25, 2024

@dleblanc First of all using an option that has Some as default value looks a bit odd. Why you cannot just have case class Ex(str: String = "hiya") instead? Could you please share a context that lead to your solution with an option?

It will help to find the best suitable solution. As example, if you want to distinguish null and missing key-value pair, you can use Option[Option[String]] with withSkipNestedOptionValues(true) compile-time configuration for codecs, like here

@andyczerwonka
Copy link
Contributor

If I pass in an explicit null, I would not expect that to use the optional field, I would expect None. I guess we would need some kind of central flag to nominate that behavior.

@lbryan-citrine
Copy link

@plokhotnyuk can we get an update here? We're not able to adjust the data model unfortunately

@plokhotnyuk
Copy link
Owner

plokhotnyuk commented Apr 11, 2024

@dleblanc @andyczerwonka @lbryan-citrine Currently, as a workaround you can use a custom codec for the option type together with the withTransientNone(false) configuration option for Ex codec:

case class Ex(opt: Option[String] = Some("hiya"), level: Option[Int] = Some(10))

object CustomOptionCodecs {
  implicit val intCodec: JsonValueCodec[Int] = make[Int]
  implicit val stringCodec: JsonValueCodec[String] = make[String]

  implicit def optionCodec[A](implicit aCodec: JsonValueCodec[A]): JsonValueCodec[Option[A]] =
    new JsonValueCodec[Option[A]] {
      override def decodeValue(in: JsonReader, default: Option[A]): Option[A] =
        if (in.isNextToken('n')) in.readNullOrError(None, "expected 'null' or JSON value")
        else {
          in.rollbackToken()
          Some(aCodec.decodeValue(in, aCodec.nullValue))
        }

      override def encodeValue(x: Option[A], out: JsonWriter): Unit =
        if (x eq None) out.writeNull()
        else aCodec.encodeValue(x.get, out)

      override def nullValue: Option[A] = None
    }
}

import CustomOptionCodecs._

val codecOfEx = make[Ex](CodecMakerConfig.withTransientNone(false))

The need of manual derivation of codecs for concrete types used as A in Option[A] is the main downside for this cross Scala version implementation.

Changing of current behavior of withTransient*(false) configuration options could be breaking for users that relay on it and seems to be overwhelming for different combination of these options.

@lbryan-citrine
Copy link

Thanks for the clarification and work around

@lbryan-citrine
Copy link

Is there any way to avoid needing to declare the primitive codecs in the body of the custom codec?

e.g.

object CustomOptionCodecs {
  implicit val intCodec: JsonValueCodec[Int] = make[Int]
  implicit val stringCodec: JsonValueCodec[String] = make[String]

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

No branches or pull requests

4 participants