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

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

Closed
alexwei opened this issue Jul 26, 2023 · 10 comments · Fixed by #658
Closed

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

alexwei opened this issue Jul 26, 2023 · 10 comments · Fixed by #658

Comments

@alexwei
Copy link

alexwei commented Jul 26, 2023

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
Copy link
Member

@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
Copy link
Member

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
Copy link
Member

pjfanning commented Jul 26, 2023

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
Copy link
Contributor

retronym commented Aug 3, 2023

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
Copy link
Member

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
Copy link
Member

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 Serializer for Scala Map converts to Java Map - avoid this conversion #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
Copy link
Contributor

retronym commented Aug 3, 2023

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
Copy link
Member

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

DefaultTyping should be ok if you avoid Scala Maps.

@pjfanning
Copy link
Member

pjfanning commented Aug 3, 2023

@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 added a commit to retronym/jackson-module-scala that referenced this issue Dec 4, 2023
…g is enabled.

This Map serializer is implemented in terms of the Java version and is
somewhat fragile, as discussed in FasterXML#643 / FasterXML#470.

This commit ensures that the wrapper class is a member class in both Scala and Java.

That results in this JSON as the serializer code chooses not to put
the inner class name in the type annotation.

```
{"m":["scala.collection.immutable.Map",{"one":"one","two":"two"}]}
```

Fixes FasterXML#643
retronym added a commit to retronym/jackson-module-scala that referenced this issue Dec 4, 2023
…g is enabled.

This Map serializer is implemented in terms of the Java version and is
somewhat fragile, as discussed in FasterXML#643 / FasterXML#470.

This commit ensures that the wrapper class is a member class in both Scala and Java.

That results in this JSON as the serializer code chooses not to put
the inner class name in the type annotation.

```
{"m":["scala.collection.immutable.Map",{"one":"one","two":"two"}]}
```

Fixes FasterXML#643
retronym added a commit to retronym/jackson-module-scala that referenced this issue Dec 4, 2023
…g is enabled.

This Map serializer is implemented in terms of the Java version and is
somewhat fragile, as discussed in FasterXML#643 / FasterXML#470.

This commit ensures that the wrapper class is a member class in both Scala and Java.

That results in this JSON as the serializer code chooses not to put
the inner class name in the type annotation.

```
{"m":["scala.collection.immutable.Map",{"one":"one","two":"two"}]}
```

Fixes FasterXML#643
retronym added a commit to retronym/jackson-module-scala that referenced this issue Dec 4, 2023
This Map serializer is implemented in terms of the Java version and is
somewhat fragile, as discussed in FasterXML#643 / FasterXML#470.

This commit ensures that the wrapper class is a member class in both Scala and Java.

That results in this JSON as the serializer code chooses not to put
the inner class name in the type annotation.

```
{"m":["scala.collection.immutable.Map",{"one":"one","two":"two"}]}
```

Fixes FasterXML#643
retronym added a commit to retronym/jackson-module-scala that referenced this issue Dec 4, 2023
This Map serializer is implemented in terms of the Java version and is
somewhat fragile, as discussed in FasterXML#643 / FasterXML#470.

This commit ensures that the wrapper class is a member class in both Scala and Java.

That results in this JSON as the serializer code chooses not to put
the inner class name in the type annotation.

```
{"m":["scala.collection.immutable.Map",{"one":"one","two":"two"}]}
```

Fixes FasterXML#643
retronym added a commit to retronym/jackson-module-scala that referenced this issue Dec 4, 2023
This Map serializer is implemented in terms of the Java version and is
somewhat fragile, as discussed in FasterXML#643 / FasterXML#470.

This commit ensures that the wrapper class is a member class in both Scala and Java.

That results in this JSON as the serializer code chooses not to put
the inner class name in the type annotation.

```
{"m":["scala.collection.immutable.Map",{"one":"one","two":"two"}]}
```

Fixes FasterXML#643
@retronym
Copy link
Contributor

retronym commented Dec 4, 2023

@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

pjfanning pushed a commit that referenced this issue Dec 4, 2023
This Map serializer is implemented in terms of the Java version and is
somewhat fragile, as discussed in #643 / #470.

This commit ensures that the wrapper class is a member class in both Scala and Java.

That results in this JSON as the serializer code chooses not to put
the inner class name in the type annotation.

```
{"m":["scala.collection.immutable.Map",{"one":"one","two":"two"}]}
```

Fixes #643
pjfanning pushed a commit that referenced this issue Dec 4, 2023
This Map serializer is implemented in terms of the Java version and is
somewhat fragile, as discussed in #643 / #470.

This commit ensures that the wrapper class is a member class in both Scala and Java.

That results in this JSON as the serializer code chooses not to put
the inner class name in the type annotation.

```
{"m":["scala.collection.immutable.Map",{"one":"one","two":"two"}]}
```

Fixes #643
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants