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

(mrbean) Problem with Optional<T>, AtomicReference<T> valued properties #8

Closed
tvk opened this issue Apr 8, 2016 · 8 comments
Closed
Milestone

Comments

@tvk
Copy link

tvk commented Apr 8, 2016

Hello,

I'd really like to use your mrbean module, but it does not seem to support java 8 optionals. Is this correct? Do you have planned adding support for this?

I've created a small testcase and that does not seem to work. It throws an GenericSignatureFormatError...
`

import java.util.Optional;

import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
import org.springframework.http.converter.json.MappingJackson2HttpMessageConverter;
import org.springframework.mock.http.MockHttpInputMessage;

import com.fasterxml.jackson.datatype.jdk8.Jdk8Module;
import com.fasterxml.jackson.module.mrbean.MrBeanModule;

public class ReadOptionalTest {

    public static final String json = "{\"string\":\"value\",\"optionalString\":\"anotherValue\"}";
    public static interface MyInterface {

        String getString();
        Optional<String> getOptionalString();
    }
    private MappingJackson2HttpMessageConverter messageConverter;

    @Before
    public void setup() {
        this.messageConverter = new MappingJackson2HttpMessageConverter();
        this.messageConverter.getObjectMapper().registerModules(new Jdk8Module(), new MrBeanModule());
    }

    @Test
    public void testReadObject() throws Exception {
        final MyInterface object = (MyInterface) messageConverter.read(MyInterface.class, new MockHttpInputMessage(json.getBytes()));
        Assert.assertNotNull(object);
    }
}

`

Best regards,
Thomas

@cowtowncoder
Copy link
Member

What happens with the test?

Mr Bean should not have to be involved with Optional handling, as that should be something JDK8 module (which you do register) takes care of. Beans are just generated as declared and standard databinding should handle them as if developer had written very simple manual implementation.
So in that sense Optionals definitely should work, unless I am missing something specific about their handling.

@tvk
Copy link
Author

tvk commented Apr 9, 2016

Good morning and thanks for your support!

Like I wrote, the test throws an GenericSignatureFormatError. Here the complete stacktrace:

java.lang.reflect.GenericSignatureFormatError: Signature Parse error: Expected Field Type Signature
    Remaining input: ;)V
    at sun.reflect.generics.parser.SignatureParser.error(Unknown Source)
    at sun.reflect.generics.parser.SignatureParser.parseFieldTypeSignature(Unknown Source)
    at sun.reflect.generics.parser.SignatureParser.parseFieldTypeSignature(Unknown Source)
    at sun.reflect.generics.parser.SignatureParser.parseTypeArgument(Unknown Source)
    at sun.reflect.generics.parser.SignatureParser.parseTypeArguments(Unknown Source)
    at sun.reflect.generics.parser.SignatureParser.parsePackageNameAndSimpleClassTypeSignature(Unknown Source)
    at sun.reflect.generics.parser.SignatureParser.parseClassTypeSignature(Unknown Source)
    at sun.reflect.generics.parser.SignatureParser.parseFieldTypeSignature(Unknown Source)
    at sun.reflect.generics.parser.SignatureParser.parseFieldTypeSignature(Unknown Source)
    at sun.reflect.generics.parser.SignatureParser.parseTypeSignature(Unknown Source)
    at sun.reflect.generics.parser.SignatureParser.parseZeroOrMoreTypeSignatures(Unknown Source)
    at sun.reflect.generics.parser.SignatureParser.parseFormalParameters(Unknown Source)
    at sun.reflect.generics.parser.SignatureParser.parseMethodTypeSignature(Unknown Source)
    at sun.reflect.generics.parser.SignatureParser.parseMethodSig(Unknown Source)
    at sun.reflect.generics.repository.ConstructorRepository.parse(Unknown Source)
    at sun.reflect.generics.repository.ConstructorRepository.parse(Unknown Source)
    at sun.reflect.generics.repository.AbstractRepository.<init>(Unknown Source)
    at sun.reflect.generics.repository.GenericDeclRepository.<init>(Unknown Source)
    at sun.reflect.generics.repository.ConstructorRepository.<init>(Unknown Source)
    at sun.reflect.generics.repository.MethodRepository.<init>(Unknown Source)
    at sun.reflect.generics.repository.MethodRepository.make(Unknown Source)
    at java.lang.reflect.Method.getGenericInfo(Unknown Source)
    at java.lang.reflect.Method.getGenericInfo(Unknown Source)
    at java.lang.reflect.Executable.getGenericParameterTypes(Unknown Source)
    at java.lang.reflect.Method.getGenericParameterTypes(Unknown Source)
    at com.fasterxml.jackson.databind.introspect.AnnotatedMethod.getParameterType(AnnotatedMethod.java:195)
    at com.fasterxml.jackson.databind.deser.BeanDeserializerFactory.addBeanProps(BeanDeserializerFactory.java:505)
    at com.fasterxml.jackson.databind.deser.BeanDeserializerFactory.buildBeanDeserializer(BeanDeserializerFactory.java:228)
    at com.fasterxml.jackson.databind.deser.BeanDeserializerFactory.createBeanDeserializer(BeanDeserializerFactory.java:127)
    at com.fasterxml.jackson.databind.deser.DeserializerCache._createDeserializer2(DeserializerCache.java:406)
    at com.fasterxml.jackson.databind.deser.DeserializerCache._createDeserializer(DeserializerCache.java:352)
    at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCache2(DeserializerCache.java:264)
    at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCacheValueDeserializer(DeserializerCache.java:244)
    at com.fasterxml.jackson.databind.deser.DeserializerCache.findValueDeserializer(DeserializerCache.java:142)
    at com.fasterxml.jackson.databind.DeserializationContext.findRootValueDeserializer(DeserializationContext.java:477)
    at com.fasterxml.jackson.databind.ObjectMapper._findRootDeserializer(ObjectMapper.java:3889)
    at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:3784)
    at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:2863)
    at org.springframework.http.converter.json.AbstractJackson2HttpMessageConverter.readJavaType(AbstractJackson2HttpMessageConverter.java:222)
    at org.springframework.http.converter.json.AbstractJackson2HttpMessageConverter.readInternal(AbstractJackson2HttpMessageConverter.java:201)
    at org.springframework.http.converter.AbstractHttpMessageConverter.read(AbstractHttpMessageConverter.java:161)
    at com.senselessweb.ReadOptionalTest.testReadObject(ReadOptionalTest.java:32)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
    at java.lang.reflect.Method.invoke(Unknown Source)
    at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
    at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
    at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
    at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
    at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
    at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
    at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
    at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
    at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
    at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
    at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
    at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
    at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:86)
    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)

I debugged a little bit and the method in question is the generated MyInterface.setOptionalString(java.util.Optional). So you think, this should work and the problem is maybe related to the JDK8Module?

@cowtowncoder
Copy link
Member

@tvk Hmmh. No, you are right, it does look like Mr Bean somehow generated invalid type declaration.

The main reason I am hoping to avoid using Optional, specifically, is that adding a dependency from Mr Bean to jdk8 module would make it significantly more difficult to build modules. However, I think that use of AtomicReference<String> might work as well -- it is supported directly by jackson-databind, and its behavior (from Jackson perspective) is very similar; both are considered so-called "reference types", and have similar single-type parameterization.

So: if it was possible to create a minimal test case that neither relies on Spring classes nor Optional, it would be bit easier to reproduce and solve the issue.

As to root cause, I guess there are two main possibilities: either version of asm library produces something problematic with Java 8; or it's something within type handling of mr bean itself.
Wrt asm: version used is 5.0.4, but there seems to be 5.1 available. I think API is compatible, so it should be easy to recompile with a newer version (module shades asm in, so simple maven override is not enough).

One other things: which version are you using? 2.7 had significant changes in type handling, which could be relevant here, depending on version.

@tvk
Copy link
Author

tvk commented Apr 11, 2016

I'm currently working with version 2.7.2. I also tried version 2.6.5 but there was no difference.

I just tried to replace the Optional by an AtomicReference (allthough this would not be a solution for me) and removed the JDK8Module, but surprisingly I got the same exception. Here my updated testcase:

import java.util.concurrent.atomic.AtomicReference;

import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;

import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.module.mrbean.MrBeanModule;

public class ReadOptionalTest {

  public static final String json = "{\"string\":\"value\",\"optionalString\":\"anotherValue\"}";

  public static interface MyInterface {

    String getString();

    AtomicReference<String> getOptionalString();
  }

  private ObjectMapper objectMapper;

  @Before
  public void setup() {
    this.objectMapper = new ObjectMapper();
    this.objectMapper.registerModules(new MrBeanModule());
  }

  @Test
  public void testReadObject() throws Exception {
    final MyInterface object = this.objectMapper.readValue(json, MyInterface.class);
    Assert.assertNotNull(object);
  }
}

.. still throws an GenericSignatureFormatError. Maybe this helps you.

@cowtowncoder
Copy link
Member

@tvk thank you for verifying this. I was actually expecting it to fail the same way, as that makes sense to me: in a way there is nothing particular special (from Jackson perspective) about Optional, and handling is very similar to AtomicReference. So yes, this should help me.

@cowtowncoder
Copy link
Member

Yes, I can reproduce this with AtomicReference<String>, as instructed.

@cowtowncoder
Copy link
Member

Ah. The problem is indeed with construction of generic type; but the minor bug is in jackson-databind.
Need to figure out what is the best fix, should be fixable for 2.7.4.

@cowtowncoder cowtowncoder changed the title (mrbean) Support for java8 optionals? (mrbean) Problem with Optional<T>, AtomicReference<T> valued properties Apr 12, 2016
@cowtowncoder cowtowncoder added this to the 2.7.4 milestone Apr 12, 2016
@cowtowncoder
Copy link
Member

Ok: turns out the problem was with newly (in 2.6) added ReferenceType, used for Optional (for Java 8, Guava, Scala) and AtomicReference, but not other generic types -- this is why generic type tests did not catch the issue unfortunately.
Fix will be in 2.7.4; proper fix in jackson-databind and mrbean module depends on that version.

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