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 resolve generic type parameters on serialization #2632

Closed
simonedavico opened this issue Dec 20, 2019 · 17 comments
Closed

Failure to resolve generic type parameters on serialization #2632

simonedavico opened this issue Dec 20, 2019 · 17 comments
Milestone

Comments

@simonedavico
Copy link

simonedavico commented Dec 20, 2019

(note: originally reported against Kotlin module, example in Kotlin but failure in databind, transferred)

I have a Either definition:

@JsonTypeInfo(use = JsonTypeInfo.Id.NAME)
@JsonSubTypes(
    JsonSubTypes.Type(Either.Left::class, name = "left"),
    JsonSubTypes.Type(Either.Right::class, name = "right")
)
sealed class Either<out A, out B> {
    class Left<A>(val value: A): Either<A, Nothing>()
    class Right<B>(val value: B): Either<Nothing, B>()
}

that I use in a simple data class:

data class Foo(
    val bar: Either<String, String>,
    val second: String
)

I create an instance and try to serialize:

return Foo(
    bar = Either.Left("aaa"),
    second = "bbbb"
)

but jackson throws an IndexOutOfBoundsException:

Caused by: java.lang.IndexOutOfBoundsException: Index: 0
        at java.base/java.util.Collections$EmptyList.get(Collections.java:4481)
        at com.fasterxml.jackson.databind.type.TypeFactory._resolveTypePlaceholders(TypeFactory.java:478)
        at com.fasterxml.jackson.databind.type.TypeFactory._bindingsForSubtype(TypeFactory.java:451)
        at com.fasterxml.jackson.databind.type.TypeFactory.constructSpecializedType(TypeFactory.java:423)
        at com.fasterxml.jackson.databind.cfg.MapperConfig.constructSpecializedType(MapperConfig.java:305)
        at com.fasterxml.jackson.databind.DatabindContext.constructSpecializedType(DatabindContext.java:163)
        at com.fasterxml.jackson.databind.ser.BeanPropertyWriter._findAndAddDynamic(BeanPropertyWriter.java:893)
        at com.fasterxml.jackson.databind.ser.BeanPropertyWriter.serializeAsField(BeanPropertyWriter.java:705)
        at com.fasterxml.jackson.databind.ser.std.BeanSerializerBase.serializeFields(BeanSerializerBase.java:722)
        ... 85 common frames omitted

Is it some misconfiguration on my side? I am using jackson 2.10.1.

@cowtowncoder
Copy link
Member

Thank you for submitting this issue!

As a general warning: generic types and polymorphic type handling do not play well together, unfortunately. So I usually avoid doing combination. This is not to say it can not work, and use case here looks reasonable to me.
One possible problem here has to do with whether Kotlin sealed class inner classes are static or not -- non-static inner classes can not be deserialized. I assume they are static for now.

Now... you definitely should not see an IndexOutOfBoundsException: there is something wrong in generic type parameter binding resolution on this particular case.
I'll see if I can reproduce this with Java-only definition.

@cowtowncoder
Copy link
Member

Looking at the original code, there is one part that does not make sense to me (possibly since I do not know Kotlin very well):

data class Foo(
    val bar: Either<String, String>,
    val second: String
)

seems illegal, because Either base definition is

sealed class Either<out A, out B> {
    class Left<A>(val value: A): Either<A, Nothing>()
    class Right<B>(val value: B): Either<Nothing, B>()
}

and String does not extend Nothing (I think ... nothing does, right?). So I am not sure how type assignment would work there -- I am not sure how exactly that would map at JVM/JDK level.

So my problem is that I do not know how to create equivalent Java-only reproduction of the issue.
If that could be created, that'd be nice.
If not, someone needs to figure out how Kotlin maps things, and then translate that into Java side, I think. And finally question would be whether this use case can or can not be supported.

@simonedavico
Copy link
Author

simonedavico commented Dec 22, 2019

Hi @cowtowncoder, thank you for answering :)

The code is legal Kotlin code, Nothing is a special type which is a subtype of all other types :)
I am also not sure how it is treated at the JVM level though.

@cowtowncoder
Copy link
Member

Right, I sort of understand what Nothing is from reading Kotlin 101, just not sure how it is implemented on JVM. And that part is something that would probably help reproduce the issue.

Exception seems to be from JDK version 9 or above; does exception different on JDK 8? If so, that might give another clue on what might be happening.

@simonedavico
Copy link
Author

@cowtowncoder I tried to run the same code with java 1.8.0_232 (Amazon Corretto JDK), but the error is the same.

@cowtowncoder
Copy link
Member

@simonedavico Ok. I guess that at least makes it easier to reproduce automatically as I work on Java 8 (I have JDK 11 locally and it's easy to switch, but all release builds must be on pre-Java 9).

@simonedavico
Copy link
Author

simonedavico commented Jan 7, 2020

@cowtowncoder Ok, is there any way I can help? Unfortunately at the moment I do not have the time to explore Jackson codebase to understand what causes the bug. I will try to give you more context on how I reproduce it: I am writing a service with Micronaut (version 1.3.0.M2). I am returning the Foo instance as defined above from a controller:

@Get("/foo")
fun getFoo() =  Foo(
    bar = Either.Left("aaa"),
    second = "bbbb"
)

This get serialized by a Micronaut managed ObjectMapper.

The curious thing is that if I define a controller like this

@Get("/plain")
fun getPlain() = Either.Left("aaa")

Jackson can serialize it correctly, and returns the string aaa.

@cowtowncoder
Copy link
Member

I think that even simple Kotlin-only test, PR, would be fine -- I should be able to debug to see why type resolution crashes, and then perhaps create Java-only equivalent. But even without that it should be possible to fix, it's just little bit more prone to regression as we do not yet have good system to rebuild all components (that is: when jackson-databind change pushed to branch like 2.10, to force rebuilding of components that depend on it, from their own repos, branches) so discovery of breakages is delayed and asynchronous in most cases.
I guess another option (aside from dependency-based rebuilds) would be daily rebuild of at least some components like jackson-module-kotlin, jackson-modules-base.

@cowtowncoder
Copy link
Member

Hmmh. One practical problem I have is that:

  1. I know how to debug/prototype tests across 2 projects (databind, kotlin module) fine with Eclipse (to run tests from latter on modified version of former), I don't know how to do that on Idea
  2. Idea has superior support for Kotlin, and I can get kotlin module easily loaded -- but on Eclipse get tons of errors for some reason

so at this point I can reproduce Kotlin test but haven't found an efficient way to really troubleshoot it.
So maybe I will have to consider converting test to the best of my knowledge.

@cowtowncoder
Copy link
Member

Attempt at creating plain Java version not successful either; I can not find a way to satisfy type constraints as Either<String, String> seems fundamentally incompatible with Left and Right -- but maybe Kotlin's Nothing type is the answer here.
Exception I get is more descriptive tho, and I would like to figure out at least better exception message.
But need to figure out how to run Kotlin tests either on Eclipse (to debug the way I usually do), or figure out how Idea would allow modifying too projects concurrently to do the same.

@frost13it
Copy link

@cowtowncoder Kotlin's Nothing always get compiled into a raw type (actual type stays in Kotlin metadata) as described here.
The issue can be reproduced in Java, please look at this gist.

@cowtowncoder
Copy link
Member

@frost13it thank you for repro; updated priority of this issue to hopefully have a look tonight or at least this week.

@cowtowncoder
Copy link
Member

EitherTest.java

import com.fasterxml.jackson.annotation.JsonSubTypes;
import com.fasterxml.jackson.annotation.JsonTypeInfo;
import com.fasterxml.jackson.databind.ObjectMapper;
import org.junit.jupiter.api.Test;

public class EitherTest {
    @Test
    void serialize_either_field() throws Exception {
        ObjectMapper mapper = new ObjectMapper();
        mapper.writeValueAsString(new Foo());
    }

    static class Foo {
        public Either<String, String> getEither() {
            return new Either.Right<String>();
        }
    }

    @JsonTypeInfo(use = JsonTypeInfo.Id.NAME)
    @JsonSubTypes(value = {
                    @JsonSubTypes.Type(value = Either.Left.class, name = "left"),
                    @JsonSubTypes.Type(value = Either.Right.class, name = "right")
    })
    static class Either<L, R> {
        static class Left<T> extends Either {   }
        static class Right<T> extends Either {  }
    }
}

@cowtowncoder cowtowncoder transferred this issue from FasterXML/jackson-module-kotlin Feb 28, 2020
@cowtowncoder cowtowncoder changed the title Why can't this be serialized? Failure to resolve generic type parameters Feb 28, 2020
@cowtowncoder
Copy link
Member

Ok so, first things first: changed code to avoid IndexOutOfBoundsException: will now indicate that type parameters do not match (specialization is impossible).

Now... while type parameterization is, as far as I can see, wrong, this probably should NOT fail when serializing: after all, object instance exists. Failing on deserialization would seem legit.
Unfortunately type specialization is not specific to ser/deser side (that is, TypeResolver does not know), so it is not yet clear if it would be possible to ignore such issues on one operation but not other.

Another possibility, altogether, could also be to consider whether substitution from String to Object, should be accepted as a special case -- perhaps only for "missing type" case which can be detected. I am not sure I like that idea, but that could perhaps cover a few legit cases and has the benefit that "String" is a type that "raw" Object does naturally map to (in theory, then, could map Long and Boolean too ... but String is much more common).

@cowtowncoder cowtowncoder added 2.11 and removed 2.10 labels Mar 26, 2020
cowtowncoder added a commit that referenced this issue Mar 27, 2020
cowtowncoder added a commit that referenced this issue Mar 28, 2020
@cowtowncoder cowtowncoder added this to the 2.11.0 milestone Mar 29, 2020
@cowtowncoder cowtowncoder changed the title Failure to resolve generic type parameters Failure to resolve generic type parameters on serialization Mar 29, 2020
@cowtowncoder
Copy link
Member

Was able to actually prevent this failure: changed the type specialization in such a way that validation problems are ignored on serialization side. That solves this case and in general should not break existing working cases, although it is possible there are other cases where something different is needed to actually resolve type discrepancy. But we'll cross that bridge when we get there -- no known problems exist.

@simonedavico
Copy link
Author

simonedavico commented May 28, 2020

@cowtowncoder thank you! I was finally able to try this and I can confirm it works like a charm! Either.Left("hello") gets serialised as:

value": {
  "@type": "left",
  "value": "hello"
}

which is a nice starting point. Would there be a way for me to configure the annotations in such a way that it serialises to the plain `value? For example, for the above I would like to get:

"value": "hello"

EDIT: I was able to achieve the serialisation format I wanted, I am pasting here my implementation in case anyone needs it. Feedback is welcome of course, I am no expert with Jackson API.

class EitherSerializer: JsonSerializer<Either<Any, Any>>() {
    override fun serialize(value: Either<Any, Any>?, gen: JsonGenerator?, provider: SerializerProvider?) {
        when (value) {
            is Either.Left -> provider!!.defaultSerializeValue(value.value, gen)
            is Either.Right -> provider!!.defaultSerializeValue(value.value, gen)
        }
    }

    override fun serializeWithType(value: Either<Any, Any>?, gen: JsonGenerator?, serializers: SerializerProvider?, typeSer: TypeSerializer?) {
        serialize(value, gen, serializers)
    }
}

@cowtowncoder
Copy link
Member

If you do not want to include Type Id, then just do not use @JsonTypeInfo; there is no point in working around that if you have actual logic in (de)serializer, and use of 2 separate physical type is more of an implementation detail than type hierarchy to support.

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

3 participants