FasterXML/jackson-module-parameter-names

An exception is thrown when serializing POJOS with Enum properties where the Enum declares a method whose arguments are captured by a lambda.

jmax01 opened this issue · 15 comments

An exception is thrown when serializing POJOS with Enum properties where the Enum declares a method whose arguments are captured by a lambda.

    @Test
    public void testWithEnumProperty() throws JsonProcessingException {

        final ObjectMapper objectMapper = new ObjectMapper().registerModule(new ParameterNamesModule());

        String asJsonString = objectMapper.writeValueAsString(new WithEnumProperty(MyEnum.VALUE0));
    }
import java.util.function.Supplier;

public enum MyEnum {

    VALUE0;

    public static void methodWithArgCapturedByLambda(final String argument) {

        Supplier<?> supplier = () -> argument+ "captured argument causes exception";
    }
}
public class WithEnumProperty {

    private final MyEnum myEnum;

    public WithEnumProperty (MyEnum myEnum) {
        this.myEnum= myEnum;
    }

    public MyEnum getMyEnum () {
        return this.myEnum;
    }
}
java.lang.reflect.MalformedParametersException: Invalid parameter name ""
    at java.lang.reflect.Executable.verifyParameters(Executable.java:336)
    at java.lang.reflect.Executable.privateGetParameters(Executable.java:366)
    at java.lang.reflect.Executable.getParameters(Executable.java:307)
    at com.fasterxml.jackson.module.paramnames.ParameterNamesAnnotationIntrospector.findParameterName(ParameterNamesAnnotationIntrospector.java:56)
    at com.fasterxml.jackson.module.paramnames.ParameterNamesAnnotationIntrospector.findImplicitPropertyName(ParameterNamesAnnotationIntrospector.java:36)
    at com.fasterxml.jackson.databind.introspect.AnnotationIntrospectorPair.findImplicitPropertyName(AnnotationIntrospectorPair.java:445)
    at com.fasterxml.jackson.databind.introspect.POJOPropertiesCollector._addCreatorParam(POJOPropertiesCollector.java:467)
    at com.fasterxml.jackson.databind.introspect.POJOPropertiesCollector._addCreators(POJOPropertiesCollector.java:455)
    at com.fasterxml.jackson.databind.introspect.POJOPropertiesCollector.collect(POJOPropertiesCollector.java:245)
    at com.fasterxml.jackson.databind.introspect.BasicClassIntrospector.collectProperties(BasicClassIntrospector.java:197)
    at com.fasterxml.jackson.databind.introspect.BasicClassIntrospector.forSerialization(BasicClassIntrospector.java:90)
    at com.fasterxml.jackson.databind.introspect.BasicClassIntrospector.forSerialization(BasicClassIntrospector.java:15)
    at com.fasterxml.jackson.databind.SerializationConfig.introspect(SerializationConfig.java:695)
    at com.fasterxml.jackson.databind.ser.BeanSerializerFactory.createSerializer(BeanSerializerFactory.java:133)
    at com.fasterxml.jackson.databind.SerializerProvider._createUntypedSerializer(SerializerProvider.java:1152)
    at com.fasterxml.jackson.databind.SerializerProvider._createAndCacheUntypedSerializer(SerializerProvider.java:1131)
    at com.fasterxml.jackson.databind.SerializerProvider.findValueSerializer(SerializerProvider.java:507)
    at com.fasterxml.jackson.databind.ser.std.BeanSerializerBase.resolve(BeanSerializerBase.java:316)
    at com.fasterxml.jackson.databind.ser.SerializerCache.addAndResolveNonTypedSerializer(SerializerCache.java:148)
    at com.fasterxml.jackson.databind.SerializerProvider._createAndCacheUntypedSerializer(SerializerProvider.java:1121)
    at com.fasterxml.jackson.databind.SerializerProvider.findValueSerializer(SerializerProvider.java:471)
    at com.fasterxml.jackson.databind.SerializerProvider.findTypedValueSerializer(SerializerProvider.java:669)
    at com.fasterxml.jackson.databind.ser.DefaultSerializerProvider.serializeValue(DefaultSerializerProvider.java:107)
    at com.fasterxml.jackson.databind.ObjectMapper._configAndWriteValue(ObjectMapper.java:3385)
    at com.fasterxml.jackson.databind.ObjectMapper.writeValueAsString(ObjectMapper.java:2779)
    at com.theice.chill.cds.api.account.segaccount.SegAccountsTest.testWithEnumProperty(SegAccountsTest.java:75)
    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:483)
    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.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: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)

Environment:
Eclipse Luna
Java 8u25
Jackson 2.5.1
Windows 7

Hello jmax01,

thank you for reporting this issue and providing a detailed explanation with a test.

I've tried the original test and it passes! Additionally, I've enhanced it with an assertion to make sure serialization works as expected:

  @Test
  public void testWithEnumProperty() throws JsonProcessingException {

      final ObjectMapper objectMapper = new ObjectMapper().registerModule(new ParameterNamesModule());

      String asJsonString = objectMapper.writeValueAsString(new WithEnumProperty(MyEnum.VALUE0));

      then(asJsonString).isEqualTo("{\"myEnum\":\"VALUE0\"}");
  }

Are you sure you are passing -parameters option to your compiler?

It appears only to fail in eclipse and I think I know why:
The eclipse byte code is different:
//javac
private static synthetic java.lang.Object lambda$methodWithArgCapturedByLambda$0(java.lang.String arg0);
//Eclipse
private static synthetic java.lang.Object lambda$0(java.lang.String );

Should the ParameterNamesAnnotationIntrospector be looking at private static synthetic java.lang.Object lambda$ methods?

Here is what I modified to address the issue

   private String findParameterName(final AnnotatedParameter annotatedParameter) {

        final AnnotatedWithParams owner = annotatedParameter.getOwner();
        Parameter[] params;

        if (owner instanceof AnnotatedConstructor) {
            params = ((AnnotatedConstructor) owner).getAnnotated()
                .getParameters();
        } else if (owner instanceof AnnotatedMethod) {
           //Added to ignore synthetics. Is this always safe?
            final AnnotatedMethod annotatedMethod = (AnnotatedMethod) owner;
            if (annotatedMethod.getMember()
                .isSynthetic()) {
                return null;
            }
            params = annotatedMethod.getAnnotated()
                .getParameters();
        } else {
            return null;
        }
        final Parameter p = params[annotatedParameter.getIndex()];
        return p.isNamePresent() ? p.getName() : null;
    }

My original question

Are you sure you are passing -parameters option to your compiler?

was a bad one, if you didn't provide that option, the reflection API would say there was no name present on the parameter, introspector would return null and you'd get an exception. From the

java.lang.reflect.MalformedParametersException: Invalid parameter name ""

it seems that the parameter name was actually written into the bytecode and was an empty string, which is weird.

Should the ParameterNamesAnnotationIntrospector be looking at private static synthetic java.lang.Object lambda$ methods?

The real question is, should the jackson-databind query parameter names introspector for parameter names on synthetic methods.

@cowtowncoder comments?

private synthetics with names that start lambda$ are likely never valid.
On Mar 7, 2015 10:23 AM, "Lovro Pandžić" notifications@github.com wrote:

My original question

Are you sure you are passing -parameters option to your compiler?

was a bad one, if you didn't provide that option, the reflection API would
say there was no name present on the parameter, introspector would return
null and you'd get an exception. From the

java.lang.reflect.MalformedParametersException: Invalid parameter name ""

it seems that the parameter name was actually written into the bytecode
and was an empty string, which is weird.

Should the ParameterNamesAnnotationIntrospector be looking at private
static synthetic java.lang.Object lambda$ methods?

The real question is, should the jackson-databind query parameter names
introspector for parameter names on synthetic methods.

@cowtowncoder https://github.com/cowtowncoder comments?


Reply to this email directly or view it on GitHub
#16 (comment)
.

I think that synthetic and bridge methods are to be generally skipped: I thought core Jackson databind code explicitly checks for this and ignores them. I don't think there is any reason to include them.
So perhaps this is just a question of how to skip such methods, and whether Java8 detection differs from earlier.

Nevertheless, I think this is a bug in the eclipse compiler and should be reported to the eclipse support.
It should either generate the parameter names or not, generating name as empty string is bogus.
@jmax01 ?

I would agree that it is a bug in the eclipse compiler and that jackson should not consider synthetic or bridge methods.

Ok, then this issue should be closed. You should open a new one on jackson-databind. @cowtowncoder, agreed?

If that is where the problem occurs yes. But databind explicitly prunes out bridge- and synthetic methods, as far as I remember. So it'd be good to have a unit test to reproduce the issue.
Further complication is that databind can not use Java8 (in theory it is possible to build java6 bytecode, but there are enough caveats that it's safer to use java6 or at most -7 as the build system).

Actually I would be ok if unit test remained here, even if fix ends up to be in databind, if that makes detection easier?

Sure, but the source of the problem is in the caller that passes the synthetic method to the introspector.

Then it would be up to caller to check for that I'd think, as I don't want to have to add checks in introspector for that. But all that is really needed is a reproducible test case: I can work on whatever side needs changes. I just can not synthesize test case.

But assuming earlier stack trace was valid I'll take a look at why checks for synthetic occur, and if this case is by-passing them. I guess it is possible for creator handling, which has its own path sort of. And that could explain why it happers only there.

Well you have a reproducible test case, all you have to do is install eclipse :).
Possible simpler alternative is to use maven eclipse compiler instead of the regular one.

Running a test in Eclipse is not automatable however, and so while I can do one-off verification, there's no way to guard against regression. But once I have time I can look into this, eventually.

I hope fix for FasterXML/jackson-databind#1005 also covers this problem; included in 2.6.4 and upcoming 2.7.

Please reopen (hopefully with a reproduction) if problem (or part of it) still remains.