jackson-module-kotlin: KotlinObjectSingletonDeserializer fails to deserialize previously serialized JSON as it doesn't delegate deserializeWithType

The introduction of KotlinObjectSingletonDeserializer in 2.10.1 breaks the deserialization of Kotlin structures that could successfully be deserialized in 2.10.0.

I’ve forked jackson-module-kotlin and introduced three branches that demonstrate the problem and show a solution:

• I branched from tag 2.10.0 and introduced a test that passes successfully for this release - see the test here.
• I branched from tag 2.10.1 and in this branch, the same test now fails.
• I branched from master and introduced a change to KotlinObjectSingletonDeserializer that fixes this particular issue such that the test passes again - see the change here.

To quickly try this all out just do:

$ git clone git@github.com:george-hawkins/jackson-module-kotlin.git
$ cd jackson-module-kotlin

$ git checkout 2.10.0-object-id
$ mvn -Dtest=com.fasterxml.jackson.module.kotlin.test.TestGithubX test
...
[INFO] Tests run: 2, Failures: 0, Errors: 0, Skipped: 0
...

$ git checkout 2.10.1-object-id-bug 
$ mvn -Dtest=com.fasterxml.jackson.module.kotlin.test.TestGithubX test
...
[ERROR] Errors: 
[ERROR]   TestGithubX.test reading involving type, id and object:63 » InvalidTypeId Miss...
...

$ git checkout master-object-id-fix 
$ mvn -Dtest=com.fasterxml.jackson.module.kotlin.test.TestGithubX test
...
[INFO] Tests run: 2, Failures: 0, Errors: 0, Skipped: 0
...

Above I check out the three branches in turn and for each ask Maven to run the test that I’ve added - for the 2.10.0 based branch everything works fine, for the 2.10.1 based branch things break and then in the master based branch things are fixed.

You could merge my master-object-id-fix branch to fix this particular problem. But I’m actually going to suggest reverting the addition of KotlinObjectSingletonDeserializer instead.

KotlinObjectSingletonDeserializer was introduced to try and resolve issue #244. It’s certainly confusing for people if they deserialize an object and get a different instance of what was supposed to be a singleton. But Jackson has so many special cases and so much clever logic for dealing with e.g. things like cycles (@JsonIdentityInfo etc.) and with things like generics (@JsonTypeInfo etc.) that it’s extremely difficult to cover all situations where Jackson deserializes, stores and reuses objects. KotlinObjectSingletonDeserializer, as it is, is just the beginning and it would require quite invasive changes elsewhere in Jackson (not just the Kotlin module) to be able to handle all cases.

Even with my change, it’s still fairly trivial to construct a case where a new instance is returned by deserialization rather than the original singleton.

E.g. see this test that fails (irrespective of whether the change I made above to KotlinObjectSingletonDeserializer is applied or not). To try this just do:

$ git checkout master-object-instance-bug
$ mvn -Dtest=com.fasterxml.jackson.module.kotlin.test.TestGithubY test
...
[ERROR] test reading a list of objects with id(com.fasterxml.jackson.module.kotlin.test.TestGithubY)  Time elapsed: 0.321 s  <<< FAILURE!
java.lang.AssertionError: second element is not the same instance as Child.
...

I’ve added a detailed explanation of what’s going on in this particular situation below. But in short, I think the problem is too complex to be easily addressed for all possible cases. So I think it’s maybe better to just accept that unfortunately Kotlin singletons get deserialized to a different instance than to go with a solution that only works in a certain subset of simpler cases. As such I think the addition of KotlinObjectSingletonDeserializer should be reverted.

---

Longer explanation

The new KotlinObjectSingletonDeserializer wraps an instance of JsonDeserializer. JsonDeserializer has quite a number of potentially overrideable methods (see below) but currently KotlinObjectSingletonDeserializer only delegates the three that I’ve marked with an asterisk. Of the remaining 16 methods, all but one of them (marked with a minus below) is overridden in at least one subclass in the standard Jackson libraries. So it would be surprising if the fact that KotlinObjectSingletonDeserializer doesn’t delegate these methods doesn’t cause problems in circumstances where the given behavior is overridden in the wrapped deserializer.

* createContextual(ctxt: DeserializationContext, property: BeanProperty ): JsonDeserializer<*>
* deserialize(p: JsonParser, ctxt: DeserializationContext): T
  deserialize(p: JsonParser, ctxt: DeserializationContext, intoValue: T): T
  deserializeWithType(p: JsonParser, ctxt: DeserializationContext, typeDeserializer: TypeDeserializer): Any
- deserializeWithType(p: JsonParser, ctxt: DeserializationContext, typeDeserializer: TypeDeserializer, intoValue: T): Any
  findBackReference(refName: String): SettableBeanProperty
  getDelegatee(): JsonDeserializer<*>
  getEmptyAccessPattern(): AccessPattern
  getEmptyValue(ctxt: DeserializationContext): Any
  getKnownPropertyNames(): Collection<Any>
  getNullAccessPattern(): AccessPattern
  getNullValue(ctxt: DeserializationContext): Any
  getObjectIdReader(ctxt: DeserializationContext): ObjectIdReader
  handledType(): Class<*>
  isCachable(): Boolean
  replaceDelegatee(delegatee: JsonDeserializer<*>): JsonDeserializer<*>
* resolve(ctxt: DeserializationContext)
  supportsUpdate(config: DeserializationConfig): Boolean
  unwrappingDeserializer(ctxt: DeserializationContext, unwrapper: NameTransformer): JsonDeserializer<T>

And even if you do delegate to the wrapped deserializer for these methods, there are still issues that KotlinObjectSingletonDeserializer can’t address.

E.g. in the test I mentioned above @JsonIdentityInfo is used. When using @JsonIdentityInfo it’s the delegate that stores identified instances. The following stacktrace shows where the ID based storage happens initially in this test. As you can see it happens in ObjectIdValueProperty.deserializeSetAndReturn which is called fairly deep down after KotlinObjectSingletonDeserializer has passed over control to the delegate:

* deserializeSetAndReturn:101, ObjectIdValueProperty (com.fasterxml.jackson.databind.deser.impl)
  deserializeAndSet:83, ObjectIdValueProperty (com.fasterxml.jackson.databind.deser.impl)
  deserializeFromObject:510, BeanDeserializer (com.fasterxml.jackson.databind.deser)
  deserializeWithObjectId:1212, BeanDeserializerBase (com.fasterxml.jackson.databind.deser)
  _deserializeOther:220, BeanDeserializer (com.fasterxml.jackson.databind.deser)
  deserialize:189, BeanDeserializer (com.fasterxml.jackson.databind.deser)
* deserialize:28, KotlinObjectSingletonDeserializer (com.fasterxml.jackson.module.kotlin)
  _deserializeTypedForId:122, AsPropertyTypeDeserializer (com.fasterxml.jackson.databind.jsontype.impl)
  deserializeTypedFromObject:89, AsPropertyTypeDeserializer (com.fasterxml.jackson.databind.jsontype.impl)
  deserializeWithType:237, AbstractDeserializer (com.fasterxml.jackson.databind.deser)
  _deserializeWithObjectId:384, CollectionDeserializer (com.fasterxml.jackson.databind.deser.std)

In the case of a singleton, the delegate stores its deserialized duplicate of the original object rather than the original singleton that only KotlinObjectSingletonDeserializer knows about.

When Jackson then encounters an ID for an already stored instance it will use this deserialized duplicate - it doesn’t even give KotlinObjectSingletonDeserializer a chance to be involved in this process. In the above stacktrace we can see that we’re deserializing a list (so CollectionDeserializer is being used) and that we’re storing an element of this list that’s identified with an ID. In this stacktrace below the same CollectionDeserializer has hit that ID again and, as you can see, it just looks up the ID directly, via findObjectId - it doesn’t involve KotlinObjectSingletonDeserializer at all, so the stored duplicate is retrieved rather than the original singleton instance:

* findObjectId:78, DefaultDeserializationContext (com.fasterxml.jackson.databind.deser)
  _deserializeFromObjectId:303, AbstractDeserializer (com.fasterxml.jackson.databind.deser)
  deserializeWithType:220, AbstractDeserializer (com.fasterxml.jackson.databind.deser)
* _deserializeWithObjectId:384, CollectionDeserializer (com.fasterxml.jackson.databind.deser.std)

Hence if you look at the test you’ll see the problem just with the second occurrence of the given singleton object (and you would see the same issue with any further occurrences in a longer list).