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

generic property type not resolved and custom deserializer therefore not used #155

Closed
hohwille opened this issue Jun 22, 2015 · 14 comments
Closed

Comments

@hohwille
Copy link

I have a bean that uses a generic that is bound to an interface. A getter uses that generic as return type.
Now I created a custom deserializer and registered for that interface that is the upper bound of the generic and expected that jackson would use this deserializer for the generic getter. However I get this error message:

Caused by: com.fasterxml.jackson.databind.JsonMappingException: Problem deserializing property 'property' (expected type: [simple type, class java.lang.Object]; actual type: java.util.LinkedHashMap), problem: argument type mismatch (through reference chain: java.util.ArrayList[0]->MyBean["property"])

So as a general example:

public interface MyInterface {}
public class MyBean<T extends MyInterface> {
...
public T getProperty() { return this.property; }
}

In Jackson I call this on my SimpleModule:

module.addDeserializer(MyInterface.class, new MyInterfaceJsonDeserializer());

That deserializer is used if I use the Interface directly instead of a generic.

I found that this issue was once reported as JACKSON-190 and was (partially) fixed:
http://markmail.org/message/w3oeneh5tan5tavb
As codehaus is gone I can not find out more.

In my case the issue seems still to be present in jackson (I am using 2.4.5).

@cowtowncoder
Copy link
Member

One problem is that you can not use SimpleModule to handle case of generic types, because it is (as name implies) simple and only uses raw, type-erased Class for finding serializers/deserializers.
Under the hood it is SimpleDeserializers implementation that is using type-erased types; so what you need is Deserializers implementation that instead uses fully resolved JavaType.

But looking at your class declaration, where is T declared?

In the end, what is really needed is a reproducible test case, to see what is wrong, and how to resolve that. Whether older issue 190 was the same is not certain.

@hohwille
Copy link
Author

Thanks for your quick answer. Github did eat up my generic declaration at is was rendered as HTML tag. I updated comment and escaped it.
Also I noticed that I picked the wrong repo for filing the issue (actually it would be better if you have all modules in one repo and use tags for the modules - it seems that you also release all modules together with the same version).

@cowtowncoder
Copy link
Member

Right, noticed this is for xml module.

I wish github had more flexible means to handle repo/issue-tracker relationships, since one-to-one mapping does not work too well. But it is what it is...

Anyway; a small unit test would be useful, but I would suggest seeing if use of custom Module would work better. It's hard to say without knowing more details.

@hohwille
Copy link
Author

I created an isolated unit-test but there it works and I can not find the difference. I am still debugging. So far it seems that the problems come from BeanDeserializerFactory in method constructSettableProperty the statement:

JavaType t0 = beanDesc.resolveType(jdkType);

It gets a TypeVariableImpl as jdkType but is not able to resolve it via the outer class defining it. Are you fully aware of the Java generic type system and how to resolve type variables. It is pretty complex and painful. I have been through this hell fixing my own property introspection framework and came up with this:
http://m-m-m.sourceforge.net/apidocs/net/sf/mmm/util/reflect/api/GenericType.html

However, as it works in my test-case I will do some further investigation and let you know...

@cowtowncoder
Copy link
Member

@hohwille Yes, I am somewhat knowledgeable. And, alas, there are open issues for Jackson type resolution, esp. regarding type aliasing. For example this one:

FasterXML/jackson-databind#76

I did write a better type resolution library:

https://github.com/FasterXML/java-classmate

which handles all cases Jackson has trouble with and has no known bugs.
But due to backwards compatibility reasons it is difficult to make Jackson itself use it... I am still hoping to do that one day but we'll see.

If it does turn out that type resolution is at fault that is more difficult to resolve, since all easy fixes are done already. Fundamental problem, at this point, is that Jackson only carries mutable type resolution context (bindings), instead of properly nested set.

@hohwille
Copy link
Author

When I add the actual bean causing the problem to my isolated test, I get this error:

com.fasterxml.jackson.databind.JsonMappingException: Type variable 'T' can not be resolved (with context of class MyBean)
at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCache2(DeserializerCache.java:266)
at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCacheValueDeserializer(DeserializerCache.java:241)
at com.fasterxml.jackson.databind.deser.DeserializerCache.findValueDeserializer(DeserializerCache.java:142)
at com.fasterxml.jackson.databind.DeserializationContext.findRootValueDeserializer(DeserializationContext.java:394)
at com.fasterxml.jackson.databind.ObjectMapper._findRootDeserializer(ObjectMapper.java:3169)
at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:3062)
at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:2161)
at de.bayern.lfu.adamas.standard.keys.logic.api.ObjectMapperGenericTest.testEto(ObjectMapperGenericTest.java:31)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:497)
at junit.framework.TestCase.runTest(TestCase.java:176)
at junit.framework.TestCase.runBare(TestCase.java:141)
at junit.framework.TestResult$1.protect(TestResult.java:122)
at junit.framework.TestResult.runProtected(TestResult.java:142)
at junit.framework.TestResult.run(TestResult.java:125)
at junit.framework.TestCase.run(TestCase.java:129)
at junit.framework.TestSuite.runTest(TestSuite.java:252)
at junit.framework.TestSuite.run(TestSuite.java:247)
at org.junit.internal.runners.JUnit38ClassRunner.run(JUnit38ClassRunner.java:86)
at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:50)
at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:459)
at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:675)
at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:382)
at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:192)
Caused by: java.lang.IllegalArgumentException: Type variable 'T' can not be resolved (with context of class MyBean)
at com.fasterxml.jackson.databind.type.TypeBindings.findType(TypeBindings.java:170)
at com.fasterxml.jackson.databind.type.TypeFactory._fromVariable(TypeFactory.java:847)
at com.fasterxml.jackson.databind.type.TypeFactory._constructType(TypeFactory.java:400)
at com.fasterxml.jackson.databind.type.TypeFactory._fromParamType(TypeFactory.java:801)
at com.fasterxml.jackson.databind.type.TypeFactory._constructType(TypeFactory.java:391)
at com.fasterxml.jackson.databind.type.TypeBindings._resolveBindings(TypeBindings.java:305)
at com.fasterxml.jackson.databind.type.TypeBindings._resolve(TypeBindings.java:203)
at com.fasterxml.jackson.databind.type.TypeBindings.findType(TypeBindings.java:121)
at com.fasterxml.jackson.databind.type.TypeFactory._fromVariable(TypeFactory.java:847)
at com.fasterxml.jackson.databind.type.TypeFactory._constructType(TypeFactory.java:400)
at com.fasterxml.jackson.databind.type.TypeBindings.resolveType(TypeBindings.java:102)
at com.fasterxml.jackson.databind.introspect.BasicBeanDescription.resolveType(BasicBeanDescription.java:221)
at com.fasterxml.jackson.databind.deser.BeanDeserializerFactory.constructSettableProperty(BeanDeserializerFactory.java:753)
at com.fasterxml.jackson.databind.deser.BeanDeserializerFactory.addBeanProps(BeanDeserializerFactory.java:546)
at com.fasterxml.jackson.databind.deser.BeanDeserializerFactory.buildBeanDeserializer(BeanDeserializerFactory.java:270)
at com.fasterxml.jackson.databind.deser.BeanDeserializerFactory.createBeanDeserializer(BeanDeserializerFactory.java:168)
at com.fasterxml.jackson.databind.deser.DeserializerCache._createDeserializer2(DeserializerCache.java:399)
at com.fasterxml.jackson.databind.deser.DeserializerCache._createDeserializer(DeserializerCache.java:348)
at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCache2(DeserializerCache.java:261)
... 26 more

@hohwille
Copy link
Author

https://github.com/FasterXML/java-classmate

I did not know about that one. I have seen that guava also did the same. There are several other implementations and I know that various other frameworks such as spring or hibernate struggle with these issues. It is really a mess that java has created here. There was actually a JSR to improve that but it has been rejected. Did you test your classmate with overriden methods that specialize the return type? There is a bug (sun and now oracle consider it a feature) that you then get two declated methods with the different return types and the order differs whether you run in debug mode or in regular mode. This bug did cost me > 20 hours. Sorry for getting off topic...

@cowtowncoder
Copy link
Member

Classmate goes well beyond what Guava does (AFAIK). The mistake many libs make (Guava included, I think), is that they assume that passing a java.lang.reflect.Type is enouugh. It is not, as that lacks context needed in many cases. I wrote a bit about this few years back:

http://www.cowtowncoder.com/blog/archives/2010/12/entry_436.html

and expanded on that a bit wrt Classmate:

http://www.cowtowncoder.com/blog/archives/2012/04/entry_471.html

Not many developers know about classmate, but it is used by some frameworks; Hibernate is one, jDBI uses it as well I think. Many more could benefit from it, since there are many, many edge cases to cover, and starting from scratch one misses most of them. It's based on my trials and errors, so at least it covers anything I have ever used. For what that is worth.

Now, most of the testing (in Classmate unit test suite) is with either basic usage, or for specific error cases I have found (including ones from Jackson). So I can't say offhand how well co-variant return type works, although I do remember adding checks for bridge methods which are generated to support those. Since the main goal is to support type resolution of POJO members, and since co-variance is sort of simple (since signature of return type is not part of the key), I would hope it works, unifying methods via overrides. But if there really are two methods defined in same class... that is an interesting anomaly.
So I think things should work; but never they say never.

Interesting thing about debug, regular mode. I'll keep that in mind in case I see odd behavior. :-)

Np for it being slightly off-topic. Happy to discuss it.

@cowtowncoder
Copy link
Member

@hohwille Back to the actual problem: while this is probably due to existing, known type resolution problem, I would be interested in an isolated reproduction. If/when we get to work on resolving these problems, it would be great to ensure good coverage. Or, if it turns out something that can not be made to work, at least document the limitation.

@hohwille
Copy link
Author

I am totally with you. I only need to find some time to extract a test-case step by step that proves the error without committing non-public code. I already managed to reproduce the other bug I discovered when you define a generic type variable and then extend without using generics at all (@SuppressWarnings("rawtypes")). I could provide this till Friday.

@hohwille
Copy link
Author

I finally managed to write a isolated test-case to reproduce the error.
In this state the "use-case" does not really make sense, but at least it is a minimal setup showing the error.

import java.io.IOException;

import junit.framework.TestCase;

import org.junit.Test;

import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.core.Version;
import com.fasterxml.jackson.databind.DeserializationContext;
import com.fasterxml.jackson.databind.JsonDeserializer;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.module.SimpleModule;

public class ObjectMapperGenericTest extends TestCase {

  @Test
  public void test() throws Exception {
    ObjectMapper mapper = new ObjectMapper();
    SimpleModule module = new SimpleModule("foo", new Version(1, 0, 0, null, "groupId", "artifactId"));
    module.addDeserializer(MyInterface.class, new MyInterfaceJsonDeserializer());
    mapper.registerModule(module);

    MyBean<?, ?> bean = mapper.readValue("{\"property\": {\"value\": \"foo\"}}", MyBean.class);
    assertNotNull(bean);
    MyInterface<?> property = bean.getProperty();
    assertNotNull(property);
    assertEquals("foo", property.getValue());
  }

  public interface Api<X extends MyInterface<T>, T extends MyInterface<?>> {
    public X getProperty();
  }

  public static class MyBean<X extends MyInterface<T>, T extends MyInterface<?>> implements Api<X, T> {
    private long id;
    private X property;

    public long getId() {
      return this.id;
    }

    public void setId(long id) {
      this.id = id;
    }

    @Override
    public X getProperty() {
      return this.property;
    }

    public void setProperty(X property) {
      this.property = property;
    }
  }

  public interface MyInterface<T> {
    public String getValue();
  }

  public static class MyInterfaceImpl implements MyInterface<String> {
    private String value;

    public MyInterfaceImpl(String value) {
      super();
      this.value = value;
    }

    @Override
    public String getValue() {
      return this.value;
    }
  }

  public static class MyInterfaceJsonDeserializer extends JsonDeserializer<MyInterface> {
    @Override
    public MyInterface deserialize(JsonParser jp, DeserializationContext ctxt) throws IOException,
        JsonProcessingException {
      JsonNode node = jp.getCodec().readTree(jp);
      return new MyInterfaceImpl(node.get("value").asText());
    }
  }
}

@hohwille
Copy link
Author

Seems to be a problem of the inverse recursive generic declaration. So an easy workaround is simply switching the order of the generic declaration.

@cowtowncoder
Copy link
Member

Ok. It may also be the case that using different name for type variable can help, if the underlying issue is incorrect resolution of type aliases, especially with shadowing/renaming of given name.

@cowtowncoder
Copy link
Member

FWIW, type resolution has been rewritten in Jackson 2.7, so this is probably resolved.
If not, please re-file under jackson-databind, as there is nothing XML specific here as far as I can see.

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

2 participants