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

AnyVal round-trip not working as expected #1140

Closed
anoto-moniz opened this issue May 1, 2024 · 3 comments
Closed

AnyVal round-trip not working as expected #1140

anoto-moniz opened this issue May 1, 2024 · 3 comments
Labels

Comments

@anoto-moniz
Copy link

Hello!
We're working on migrating from json4s to jsoniter-scala, and hitting an issue with inlining. We have a similar setup to #1078 , but instead of a nested object, it's a Map[String, CustomType].

Here's a simplified view of the structure.

trait TopProperty extends Any
trait Property extends Any with TopProperty
case class DoubleProperty(value: Double) extends AnyVal with Property
case class StringProperty(value: String) extends AnyVal with Property

case class Container(
  name: String,
  properties: Map[String, Property],
  other: OtherCustomType
)

The JSON we need to support is

{"name": "foo", "properties": {"a": 4.0, "b": "bar"}, "other": {...}}

The encoding/serialization works as desired, producing the above JSON. But when I try to deserialize it using readFromString[Container], I get expected '{' at the position of the first value (4.0 in the example above). I've been unable to figure out why this asymmetry is occurring.

I've tried a bunch of other approaches, but the only one I've had success with is directly using the JsonWriter and JsonReader objects in a custom codec, which of course isn't ideal. Especially since the real class has about a dozen fields, a handful of which are complex classes.

@plokhotnyuk plokhotnyuk added the bug label May 2, 2024
@plokhotnyuk
Copy link
Owner

plokhotnyuk commented May 2, 2024

@anoto-moniz Thanks for reporting!

If generated codec cannot parse own output then it is a bug definitely.

As a workaround for your case if there are no more other implementation of Property the following custom codec could be used:

implicit val customCodecOfProperty: JsonValueCodec[Property] =
  new JsonValueCodec[Property] {
    def nullValue: Property = null

    def decodeValue(in: JsonReader, default: Property): Property = {
      val t = in.nextToken()
      in.rollbackToken()
      if (t == '"') StringProperty(in.readString(null))
      else DoubleProperty(in.readDouble())
    }

    def encodeValue(x: Property, out: JsonWriter): Unit = x match {
      case DoubleProperty(d) => out.writeVal(d)
      case StringProperty(s) => out.writeVal(s)
    }
  }

Is it an acceptable solution for you?

BTW, I was able to reproduce this bug only after adding sealed for the definition of Property trait to be a kind of a sum type. Unfortunately, jsoniter-scala-macros doesn't support sum types without discriminators, so the bug fix will be just throwing of a compilation error that AnyVal and one value classes with CodecMakerConfig.withInlineOneValueClasses(true) cannot be used as leaf sub-types for a base sum type.

plokhotnyuk added a commit that referenced this issue May 2, 2024
… value classes with 'CodecMakerConfig.withInlineOneValueClasses(true)' are used as leaf classes of sum types
@plokhotnyuk
Copy link
Owner

@anoto-moniz Please peek the latest release where codecs will not be generated for sum types with leaf classes that are AnyVal or one value classes with CodecMakerConfig.withInlineOneValueClasses(true).

@anoto-moniz
Copy link
Author

@plokhotnyuk Thanks so much for the quick response! I updated my code with your suggested codec, and it works like a charm.

And I don't think the limitation you reference should cause my other use cases any problems. This use of sum types without type discriminators is unique in our codebase.

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

2 participants