Failure to deserialise scala Map when default typing is activated

alexwei commented 1 year ago

Scala version: 2.13.10 Jason version: 2.15.2 Jackson-module-scala version: 2.13-2.15.2

Test code:

import com.fasterxml.jackson.annotation.JsonTypeInfo
import com.fasterxml.jackson.databind.ObjectMapper.DefaultTyping
import com.fasterxml.jackson.databind.json.JsonMapper
import com.fasterxml.jackson.module.scala.{ClassTagExtensions, DefaultScalaModule}
import org.junit.Test

case class JacksonTestData(
                            a: String
                            , b: Int
                            , c: Option[String]
                            , d: List[Int]
                            , e: Map[String, String]
                          )

class JsonDataConverterTest {

  private val mapper = JsonMapper.builder()
    .addModule(DefaultScalaModule)
    .build() :: ClassTagExtensions

  mapper
    .activateDefaultTyping(mapper.getPolymorphicTypeValidator, DefaultTyping.NON_FINAL, JsonTypeInfo.As.WRAPPER_ARRAY)
    .registerModule(DefaultScalaModule)

  @Test
  def testJacksonTestDataConversion(): Unit = {
    val t = JacksonTestData(
      a = "abc"
      , b = 1
      , c = Some("zyx")
      , d = List(0, 1, 2)
      , e = Map("a" -> "1", "b" -> "2", "c" -> "3")
    )

    val s = mapper.writeValueAsString(t)

    println(s)

    val tt = mapper.readValue[JacksonTestData](s)

    assert(t == tt)
  }

}

When running the test:

["JacksonTestData",{"a":"abc","b":1,"c":"zyx","d":["scala.collection.immutable.$colon$colon",[0,1,2]],"e":["scala.collection.convert.JavaCollectionWrappers$MapWrapper",{"a":"1","b":"2","c":"3"}]}]

Could not resolve type id 'scala.collection.convert.JavaCollectionWrappers$MapWrapper' as a subtype of `scala.collection.immutable.Map<java.lang.String,java.lang.String>`: Not a subtype
 at [Source: (String)"["JacksonTestData",{"a":"abc","b":1,"c":"zyx","d":["scala.collection.immutable.$colon$colon",[0,1,2]],"e":["scala.collection.convert.JavaCollectionWrappers$MapWrapper",{"a":"1","b":"2","c":"3"}]}]"; line: 1, column: 207] (through reference chain: JacksonTestData["e"])
com.fasterxml.jackson.databind.exc.InvalidTypeIdException: Could not resolve type id 'scala.collection.convert.JavaCollectionWrappers$MapWrapper' as a subtype of `scala.collection.immutable.Map<java.lang.String,java.lang.String>`: Not a subtype
 at [Source: (String)"["JacksonTestData",{"a":"abc","b":1,"c":"zyx","d":["scala.collection.immutable.$colon$colon",[0,1,2]],"e":["scala.collection.convert.JavaCollectionWrappers$MapWrapper",{"a":"1","b":"2","c":"3"}]}]"; line: 1, column: 207] (through reference chain: JacksonTestData["e"])
    at com.fasterxml.jackson.databind.exc.InvalidTypeIdException.from(InvalidTypeIdException.java:43)
    at com.fasterxml.jackson.databind.DeserializationContext.invalidTypeIdException(DeserializationContext.java:2084)
    at com.fasterxml.jackson.databind.DatabindContext._throwNotASubtype(DatabindContext.java:290)
    at com.fasterxml.jackson.databind.DatabindContext.resolveAndValidateSubType(DatabindContext.java:251)
    at com.fasterxml.jackson.databind.jsontype.impl.ClassNameIdResolver._typeFromId(ClassNameIdResolver.java:72)
    at com.fasterxml.jackson.databind.jsontype.impl.ClassNameIdResolver.typeFromId(ClassNameIdResolver.java:66)
    at com.fasterxml.jackson.databind.jsontype.impl.TypeDeserializerBase._findDeserializer(TypeDeserializerBase.java:159)
    at com.fasterxml.jackson.databind.jsontype.impl.AsArrayTypeDeserializer._deserialize(AsArrayTypeDeserializer.java:97)
    at com.fasterxml.jackson.databind.jsontype.impl.AsArrayTypeDeserializer.deserializeTypedFromAny(AsArrayTypeDeserializer.java:71)
    at com.fasterxml.jackson.databind.deser.std.StdDeserializer.deserializeWithType(StdDeserializer.java:172)
    at com.fasterxml.jackson.databind.deser.SettableBeanProperty.deserialize(SettableBeanProperty.java:542)
    at com.fasterxml.jackson.databind.deser.BeanDeserializer._deserializeWithErrorWrapping(BeanDeserializer.java:570)
    at com.fasterxml.jackson.databind.deser.BeanDeserializer._deserializeUsingPropertyBased(BeanDeserializer.java:439)
    at com.fasterxml.jackson.databind.deser.BeanDeserializerBase.deserializeFromObjectUsingNonDefault(BeanDeserializerBase.java:1409)
    at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserializeFromObject(BeanDeserializer.java:352)
    at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserialize(BeanDeserializer.java:185)
    at com.fasterxml.jackson.databind.jsontype.impl.AsArrayTypeDeserializer._deserialize(AsArrayTypeDeserializer.java:120)
    at com.fasterxml.jackson.databind.jsontype.impl.AsArrayTypeDeserializer.deserializeTypedFromObject(AsArrayTypeDeserializer.java:61)
    at com.fasterxml.jackson.databind.deser.BeanDeserializerBase.deserializeWithType(BeanDeserializerBase.java:1296)
    at com.fasterxml.jackson.databind.deser.impl.TypeWrappedDeserializer.deserialize(TypeWrappedDeserializer.java:74)
    at com.fasterxml.jackson.databind.deser.DefaultDeserializationContext.readRootValue(DefaultDeserializationContext.java:323)
    at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4825)
    at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3772)
    at com.fasterxml.jackson.module.scala.ClassTagExtensions.readValue(ClassTagExtensions.scala:142)
    at com.fasterxml.jackson.module.scala.ClassTagExtensions.readValue$(ClassTagExtensions.scala:141)
    at com.fasterxml.jackson.module.scala.ClassTagExtensions$Mixin.readValue(ClassTagExtensions.scala:18)
    at JsonDataConverterTest.testJacksonTestDataConversion(JsonDataConverterTest.scala:41)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base/java.lang.reflect.Method.invoke(Method.java:566)
    at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
    at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
    at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
    at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
    at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
    at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
    at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366)
    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:63)
    at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331)
    at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79)
    at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329)
    at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66)
    at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293)
    at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
    at org.junit.runners.ParentRunner.run(ParentRunner.java:413)
    at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecutor.runTestClass(JUnitTestClassExecutor.java:110)
    at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecutor.execute(JUnitTestClassExecutor.java:58)
    at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecutor.execute(JUnitTestClassExecutor.java:38)
    at org.gradle.api.internal.tasks.testing.junit.AbstractJUnitTestClassProcessor.processTestClass(AbstractJUnitTestClassProcessor.java:62)
    at org.gradle.api.internal.tasks.testing.SuiteTestClassProcessor.processTestClass(SuiteTestClassProcessor.java:51)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base/java.lang.reflect.Method.invoke(Method.java:566)
    at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:36)
    at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24)
    at org.gradle.internal.dispatch.ContextClassLoaderDispatch.dispatch(ContextClassLoaderDispatch.java:33)
    at org.gradle.internal.dispatch.ProxyDispatchAdapter$DispatchingInvocationHandler.invoke(ProxyDispatchAdapter.java:94)
    at com.sun.proxy.$Proxy2.processTestClass(Unknown Source)
    at org.gradle.api.internal.tasks.testing.worker.TestWorker.processTestClass(TestWorker.java:118)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base/java.lang.reflect.Method.invoke(Method.java:566)
    at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:36)
    at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24)
    at org.gradle.internal.remote.internal.hub.MessageHubBackedObjectConnection$DispatchWrapper.dispatch(MessageHubBackedObjectConnection.java:182)
    at org.gradle.internal.remote.internal.hub.MessageHubBackedObjectConnection$DispatchWrapper.dispatch(MessageHubBackedObjectConnection.java:164)
    at org.gradle.internal.remote.internal.hub.MessageHub$Handler.run(MessageHub.java:412)
    at org.gradle.internal.concurrent.ExecutorPolicy$CatchAndRecordFailures.onExecute(ExecutorPolicy.java:64)
    at org.gradle.internal.concurrent.ManagedExecutorImpl$1.run(ManagedExecutorImpl.java:48)
    at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
    at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
    at org.gradle.internal.concurrent.ThreadFactoryImpl$ManagedThreadRunnable.run(ThreadFactoryImpl.java:56)
    at java.base/java.lang.Thread.run(Thread.java:829)

Probably related to #470

pjfanning commented 1 year ago

@alexwei jackson-module-scala is an ugly set of hacks around the Java-centric library 'jackson-databind'

With the endless variety of configuration that jackson allows but with Java-centric implementations in jackson-databind, jackson-module-scala simply can't keep up.

Fixing this could take days of fiddly debugging and require changes in both jackson-databind and jackson-module-scala.

My main aim is to get jackson-module-scala to handle the vanilla configuration of ObjectMapper. Once people start tweaking advanced Java-centric configs of jackson-databind - I lose interest. The cost-benefit is heavily skewed against spending time on this.

pjfanning commented 1 year ago

The issue seems to be a serialization issue:

vanilla ObjectMapper writes:

{"a":"abc","b":1,"c":"zyx","d":[0,1,2],"e":{"a":"1","b":"2","c":"3"}}

.activateDefaultTyping(mapper.getPolymorphicTypeValidator, DefaultTyping.NON_FINAL, JsonTypeInfo.As.WRAPPER_ARRAY) causes this output with terrible class choices

["com.fasterxml.jackson.module.scala.ser.DefaultTypingSerializationTest$JacksonTestData",{"a":"abc","b":1,"c":"zyx","d":["scala.collection.immutable.$colon$colon",[0,1,2]],"e":["scala.collection.convert.JavaCollectionWrappers$MapWrapper",{"a":"1","b":"2","c":"3"}]}]

pjfanning commented 1 year ago

I, personally, would never use a Map when dealing when serializing/deserializing data. I would try to use collections that contain strongly typed data - Seq[MyCaseClass] or Seq[(A, B)] or something like that.

Putting class names in JSON output doesn't feel great.

If I had to get something working with JacksonTestData, I would change the Map[String, String] to Seq[String, String] or java.util.Map[String, String]

Or just use a vanilla mapper instance (ie without the DefaultTyping set).

retronym commented 1 year ago

Noting that this used to work in Scala 2.12 through a bit of good fortune.

AFAICT

jackon-module-scala's MapConverter calls map.asJava to convert (well, wrap) the Scala Map as a Java Map.
jackson-databind-s MapSerializer.serializeWithType decides what class name to emit in the JSON by calling ClassNameIdResolver.idFromType in a resolver with this._baseType = MapLikeType("scala.collection.immutable.Map", ...)
In Scala 2.12, the .asJava call gives an inner class scala.collection.convert.Wrappers$MapWrapper. In 2.13 we get an non-inner class (as it enclosed in an scala object, not class) scala.collection.convert.JavaCollectionWrappers$MapWrapper.
idFromType falls back to using the base type (which is what we're after, I think), in 2.12. In 2.13 it emits the type id of JavaCollectionWrappers$MapWrapper, but this leads to a failure in deserializing.

A workaround would be to replace the .asJava call with an explicit wrapping of the value in a custom class that a) implements java.util.Map, and b) is an inner class. 🤮!

cowtowncoder commented 1 year ago

The big problem really is that Jackson's databind has pretty decent support for Map and Collection implementations, but Scala's counterparts do not implement them. As such there's much less support for "Map-like" and "Collection-like" types so implementation is by definition more involved. Although if existence of these was known at the time original Map/Collection (de)serializers were built they could probably have been implemented in a way to improve reuse.

Now doing 2-phase processing using Java Map/Collection intermediates works ok for most other aspects but NOT for polymorphic typing (esp. via Default Typing) because it by definition is very tightly coupled to implementation types -- and in this case information for Wrapper is used.

I guess I am just repeating much of what @retronym just said. :)

pjfanning commented 1 year ago

I spent some time trying to hack a solution but I just kept hitting issues. I have many higher priority things to work on so I'm not planning to spend much time on this.

It is very easy to workaround - don't enable default typing when deserializing such JSON
this is not a new issue and so far only 1 person has raised an issue
probably the best solution is https://github.com/FasterXML/jackson-module-scala/issues/470 but that involves forking a 1000+ line Java serializer and adding tonnes of test coverage for that 1000 lines of code to cover all the various code paths through that 1000 lines

retronym commented 1 year ago

Thanks for the notes.

I'm doing maintenance work across a large codebase and luckily there only seems to be one small spot using Default Typing within fairly extensive usage of jackson-scala. It looks like this was done to serialise a Scala sealed hierarchy.

I'm going to use an alternative mechanism for that instead. I believe @ JsonTypeInfo + @ JsonSubtypes should work as per https://stackoverflow.com/questions/18027233/jackson-scala-json-deserialization-to-case-classes .

pjfanning commented 1 year ago

Thanks @retronym - JsonTypeInfo/JsonSubtypes annotations are the best way to do this.

DefaultTyping should be ok if you avoid Scala Maps.

pjfanning commented 1 year ago

@retronym I'm not sure if it's a great idea but I have reflection based libs that extend jackson-module-scala and that auto-discover subtypes for sealed traits. You still need the JsonTypeInfo annotation (but don't need to list the sub types). I was hoping to create a jackson-databind enhancement that would allow you to avoid the JsonTypeInfo annotation too but I haven't gotten around to it.

https://github.com/FasterXML/jackson-module-scala#related-projects (separate projects for Scala 2 and Scala 3 - because the code differs quite a bit depending on which Scala version you are using).

retronym commented 12 months ago

@pjfanning I finally got back to trying to work around this bug. It proved awkward so I took another look at finding a workaround in the library:. Here's my PR: #658

FasterXML / jackson-module-scala

Failure to deserialise scala Map when default typing is activated #643