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

Fail to deserialize @JsonUnwrapped properties passed via @JsonCreator #90

Closed
yunglin opened this issue Jul 4, 2013 · 15 comments
Closed
Milestone

Comments

@yunglin
Copy link

yunglin commented Jul 4, 2013

I am trying to use @JsonUnwrapped to make a flatten json object into more structure scala object. However, jackson will throw java.lang.IllegalStateException: Method should never be called on a com.fasterxml.jackson.databind.deser.CreatorProperty

I wrote a test case for this issue and tested on 2.2.2 release and 2.3.0-SNAPSHOT, and neither of them can pass the test.

import com.fasterxml.jackson.annotation.{JsonUnwrapped, JsonProperty, JsonCreator}
import com.fasterxml.jackson.databind.ObjectMapper
import com.fasterxml.jackson.module.scala.DefaultScalaModule
import org.scalatest.testng.TestNGSuite
import org.testng.annotations.Test

/**
 */

case class Person @JsonCreator() (
    @JsonProperty("name") name: String,
    @JsonUnwrapped location: Address,
    @JsonProperty("alias") alias: Option[String] = None
)

case class Address @JsonCreator() (
    @JsonProperty("address1") address1: Option[String] = None,
    @JsonProperty("city") city: Option[String] = None,
    @JsonProperty("state") state:Option[String] = None
 )

class JsonCreatorTest extends TestNGSuite {

  @Test
  def testReadWriteJson {
    val person = new Person("MyName", Address(Some("main street"), Some("springfield")))
    val mapper = new ObjectMapper()
    mapper.registerModule(new DefaultScalaModule)

    val json = mapper.writeValueAsString(person)
    println(json)
    val obj = mapper.readValue(json, classOf[Person])

    assert(obj === person)
  }
}
@christophercurrie
Copy link
Member

Thanks for the report, I'll look into this as soon as I can.

FWIW, the @JsonCreator and @JsonProperty annotations should be redundant in 2.2+. Having them shouldn't hurt either, though, so I don't believe that the issue is in your code. I think the @JsonUnwrapper annotation is under-tested in the Scala module.

@christophercurrie
Copy link
Member

I took some time to look at this today. Part of the process involves recreating the use case in Java, to see how the databind module is designed to work with it. The example I came up with this here:

https://gist.github.com/christophercurrie/5991810

The best guidance I've found for dealing with this issue is to have a constructor that takes the individual properties as parameters.

http://stackoverflow.com/questions/16570073/whats-the-jackson-deserialization-equivalent-of-jsonunwrapped

For your use case, that would (in theory) look something like:

case class Person(name: String, location: Address, alias: Option[String] = None)
{
  @JsonCreator
  def this(@JsonProperty("name") name: String,
           @JsonProperty("address1") address1: Option[String] = None,
           @JsonProperty("city") city: Option[String] = None,
           @JsonProperty("state") state:Option[String] = None,
           @JsonProperty("alias") alias: Option[String] = None) =
    this(name, Address(address1, city, state), alias)
}

However, due to SI-7661, it appears that this particular formulation won't compile, so you'd have to develop a different set of compiler overloads.

I've raised this issue on the Jackson developers list in hopes of finding a better solution, and will provide updates here as I have them.

@christophercurrie
Copy link
Member

The issue in databind has been filed as FasterXML/jackson-databind#265.

@lmmworko
Copy link

Even using the approach you suggest I can't get it to recognize the @JsonUnwrapped annotation only when serializing:

import com.fasterxml.jackson.annotation.{ JsonUnwrapped, JsonProperty, JsonCreator, JsonIgnore }
import com.fasterxml.jackson.databind.ObjectMapper
import com.fasterxml.jackson.module.scala.DefaultScalaModule
import org.junit.Test
import org.junit.Assert.assertEquals
import scala.annotation.meta.field
import scala.annotation.meta.getter

case class Person @JsonIgnore() (
  name: String,
      // @JsonUnwrapped: Test passes, but the structure it prints is nested, not flat as it's supposed to be
      // @(JsonUnwrapped @field): Test fails
      // @(JsonUnwrapped @getter): Test fails
  @JsonUnwrapped
      location: Address,
  alias: Option[String]) {
  @JsonCreator() def this(
    @JsonProperty("name") name: String,
    @JsonProperty("address1") address1: Option[String],
    @JsonProperty("city") city: Option[String],
    @JsonProperty("state") state: Option[String],
    @JsonProperty("alias") alias: Option[String]) =
    this(name, Address(address1, city, state), alias)
}

case class Address(
  address1: Option[String],
  city: Option[String],
  state: Option[String])

class JsonCreatorTest {

  @Test
  def testReadWriteJson {
    val person = new Person("MyName", Address(Some("main street"), Some("springfield"), None), None)
    val mapper = new ObjectMapper()
    mapper.registerModule(new DefaultScalaModule)

    val json = mapper.writeValueAsString(person)
    println(json)
    val obj = mapper.readValue(json, classOf[Person])

    assertEquals(person, obj)
  }
}

How do I make this work?

@christophercurrie
Copy link
Member

Perhaps we're making things more complex than they need to be. Since Jackson isn't doing the wrapping for us, and we have to do it by hand, then we might as well make the code adapt to the JSON:

case class Person(
    name: String,
    address1: Option[String],
    city: Option[String],
    state: Option[String],
    alias: Option[String]
) {
   def this(name: String, a: Address, alias: String) = this(name, a.address1, a.city, a.state, alias)

   @JsonIgnore
   val location = Address(address1, city, state)
}

This has some unfortunate wrapping/unwrapping when the constructor supporting Address is used, but I think it's the best we can do, until the underlying Jackson issue is fixed.

@yunglin
Copy link
Author

yunglin commented Aug 22, 2013

Chris,

This approach works on most case as long as you have no more than 22 attributes in a json object/case class.

@cowtowncoder
Copy link
Member

Also note that whether problems with combination of @JsonUnwrapped and @JsonCreator are fixed is open: there are possible fundamental issues in getting the two to play nicely -- there's chicken-and-egg problem since "unwapped" stuff is done after instantiating parent object; and for that occur, creator properties must have been collected. This is not a fundamental limitation, but is something where implementation as-is may become practical limitation on short-to-medium term (and, if enough other work remains, longer).

Above is just to suggest that to solve urgent practical problem, work-around is the way to go.

christophercurrie added a commit that referenced this issue Aug 23, 2013
christophercurrie added a commit that referenced this issue Aug 23, 2013
@christophercurrie
Copy link
Member

@yunglin True, though I would find a JSON object with more than 22 properties a scary thing indeed.

@lmmworko also has another restriction that his field layout is fixed because of database marshaling. I always recommend separate types for wire formats and database formats, even if they are similar today, because variations like this always happen. But writing the monkey code to convert from one to the other is no fun, so I understand why designers shy away from this (even though I would still recommend you write it anyway).

As @cowtowncoder mentions, there's still no permanent fix in Jackson for @JsonUnwrapped in creators. To work around it you can use mutable objects:

class NonCreatorPerson
{
  var name: String = _
  @JsonUnwrapped var location: Address = _
  var alias: Option[String] = _
}

This works just fine for the JSON, but I don't know if it would muck with whatever database marshaling might be in play.

The other way, that keeps the class more or less structurally the same, requires the fix I just made in 75d9970 for @JsonIgnore, as well as a pretty hacky implementation of your Person class:

case class Person(name: String, @JsonIgnore location: Address, alias: Option[String])
{
  private def this() = this("", Address(None, None, None), None)

  def address1 = location.address1
  private def address1_=(value: Option[String]) {
    setAddressField("address1", value)
  }

  def city = location.city
  private def city_=(value: Option[String]) {
    setAddressField("city", value)
  }

  def state = location.state
  private def state_= (value: Option[String]) {
    setAddressField("state", value)
  }

  private def setAddressField(name: String, value: Option[String])
  {
    val f = location.getClass.getDeclaredField(name)
    f.setAccessible(true)
    f.set(location, value)
  }
}

This basically adds some private setters for Jackson to use to stuff the Address properties into, and some low level JVM reflection hacks to force assignments to Address's vals. It's awful stuff, but it works.

However, I'm going to leave this issue as unresolved until I can either come up with better solutions for the Scala module, or a permanent fix in the databind library.

@awhitford
Copy link

I have stumbled across this issue too... For what it is worth, I created a simple example with just @JsonUnwrapped:

import com.fasterxml.jackson.annotation.JsonUnwrapped

case class Point (x: Int, y: Int)

case class Circle (
  @JsonUnwrapped
  point: Point,
  radius: Int
)

Then, create the Scala Mapper:

import com.fasterxml.jackson.databind.ObjectMapper
import com.fasterxml.jackson.module.scala.DefaultScalaModule
import com.fasterxml.jackson.module.scala.experimental.ScalaObjectMapper

val mapper = new ObjectMapper() with ScalaObjectMapper
mapper.registerModule(DefaultScalaModule)

Finally, try to read:

val p = mapper.readValue[Point]("""
{
  "x": 5,
  "y": 10
}
""")

Reading a Point works, but not a Circle:

val c = mapper.readValue[Circle]("""
{
  "x": 50,
  "y": 100,
  "radius": 30
}
""")

I get the stack trace:

java.lang.IllegalStateException: Method should never be called on a com.fasterxml.jackson.databind.deser.CreatorProperty
at com.fasterxml.jackson.databind.deser.CreatorProperty.set(CreatorProperty.java:182)
at com.fasterxml.jackson.databind.deser.CreatorProperty.deserializeAndSet(CreatorProperty.java:165)
at com.fasterxml.jackson.databind.deser.impl.UnwrappedPropertyHandler.processUnwrapped(UnwrappedPropertyHandler.java:61)
at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserializeUsingPropertyBasedWithUnwrapped(BeanDeserializer.java:648)
at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserializeWithUnwrapped(BeanDeserializer.java:478)
at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserializeFromObject(BeanDeserializer.java:271)
at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserialize(BeanDeserializer.java:121)
at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:2888)
at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:2048)
at com.fasterxml.jackson.module.scala.experimental.ScalaObjectMapper$class.readValue(ScalaObjectMapper.scala:180)
at $anon$1.readValue(:13)
at .(:14)
at .()
at .(:7)
at .()
at $print()
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:606)
at scala.tools.nsc.interpreter.IMain$ReadEvalPrint.call(IMain.scala:734)
at scala.tools.nsc.interpreter.IMain$Request.loadAndRun(IMain.scala:983)
at scala.tools.nsc.interpreter.IMain.loadAndRunReq$1(IMain.scala:573)
at scala.tools.nsc.interpreter.IMain.interpret(IMain.scala:604)
at scala.tools.nsc.interpreter.IMain.interpret(IMain.scala:568)
at scala.tools.nsc.interpreter.ILoop.reallyInterpret$1(ILoop.scala:756)
at scala.tools.nsc.interpreter.ILoop.interpretStartingWith(ILoop.scala:801)
at scala.tools.nsc.interpreter.ILoop.reallyInterpret$1(ILoop.scala:774)
at scala.tools.nsc.interpreter.ILoop.interpretStartingWith(ILoop.scala:801)
at scala.tools.nsc.interpreter.ILoop.reallyInterpret$1(ILoop.scala:774)
at scala.tools.nsc.interpreter.ILoop.interpretStartingWith(ILoop.scala:801)
at scala.tools.nsc.interpreter.ILoop.reallyInterpret$1(ILoop.scala:774)
at scala.tools.nsc.interpreter.ILoop.interpretStartingWith(ILoop.scala:801)
at scala.tools.nsc.interpreter.ILoop.reallyInterpret$1(ILoop.scala:774)
at scala.tools.nsc.interpreter.ILoop.interpretStartingWith(ILoop.scala:801)
at scala.tools.nsc.interpreter.ILoop.reallyInterpret$1(ILoop.scala:774)
at scala.tools.nsc.interpreter.ILoop.interpretStartingWith(ILoop.scala:801)
at scala.tools.nsc.interpreter.ILoop.reallyInterpret$1(ILoop.scala:774)
at scala.tools.nsc.interpreter.ILoop.interpretStartingWith(ILoop.scala:801)
at scala.tools.nsc.interpreter.ILoop.command(ILoop.scala:713)
at scala.tools.nsc.interpreter.ILoop.processLine$1(ILoop.scala:577)
at scala.tools.nsc.interpreter.ILoop.innerLoop$1(ILoop.scala:584)
at scala.tools.nsc.interpreter.ILoop.loop(ILoop.scala:587)
at scala.tools.nsc.interpreter.ILoop$$anonfun$process$1.apply$mcZ$sp(ILoop.scala:878)
at scala.tools.nsc.interpreter.ILoop$$anonfun$process$1.apply(ILoop.scala:833)
at scala.tools.nsc.interpreter.ILoop$$anonfun$process$1.apply(ILoop.scala:833)
at scala.tools.nsc.util.ScalaClassLoader$.savingContextLoader(ScalaClassLoader.scala:135)
at scala.tools.nsc.interpreter.ILoop.process(ILoop.scala:833)
at scala.tools.nsc.interpreter.ILoop.main(ILoop.scala:900)
at xsbt.ConsoleInterface.run(ConsoleInterface.scala:69)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:606)
at sbt.compiler.AnalyzingCompiler.call(AnalyzingCompiler.scala:102)
at sbt.compiler.AnalyzingCompiler.console(AnalyzingCompiler.scala:77)
at sbt.Console.sbt$Console$$console0$1(Console.scala:23)
at sbt.Console$$anonfun$apply$2$$anonfun$apply$1.apply$mcV$sp(Console.scala:24)
at sbt.TrapExit$.sbt$TrapExit$$executeMain$1(TrapExit.scala:33)
at sbt.TrapExit$$anon$1.run(TrapExit.scala:42)

@christophercurrie
Copy link
Member

Thanks for your feedback. This is a known issue that requires additional work in the databind library. That work is being tracked in FasterXML/jackson-databind#265. When that issue is resolved, it's expected that this one will be resolved as well.

@christophercurrie christophercurrie modified the milestones: 2.6, 2.4 Nov 26, 2014
@mbknor
Copy link
Member

mbknor commented Apr 10, 2019

I also hit this issue today... Now that FasterXML/jackson-databind#265 is fixed, maybe we can get this to work?

@mbknor
Copy link
Member

mbknor commented Apr 10, 2019

I now see that scala has the same problem as Kotlin and that FasterXML/jackson-databind#1467 must be fixed..

@cowtowncoder cowtowncoder changed the title Fail to deserialize JsonUnwrapped properties. Fail to deserialize @JsonUnwrapped properties passed via @JsonCreator Sep 22, 2019
@zt1983811
Copy link

I hit this issue today, any update on this as now is 2021 almost 9 years

@pjfanning
Copy link
Member

I presume this was closed so that discussion can continue on FasterXML/jackson-databind#1467

@cowtowncoder
Copy link
Member

Yes; this is not Scala-specific as far as I know.

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

No branches or pull requests

8 participants