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

ParameterNamesModule + JsonCreator + inheritance cause deserialization errors #49

Closed
twilson-palantir opened this issue Jan 22, 2018 · 9 comments

Comments

@twilson-palantir
Copy link

Running

public class FooTest {

    public static void main(String[] args) throws IOException {
        new ObjectMapper()
                .registerModule(new ParameterNamesModule())
                .readValue("\"foo\"", Foo.class);
    }

    public interface Foo {
        String getFoo();

        @JsonCreator
        static Foo valueOf(String foo) {
            return new FooImpl(foo);
        }
    }

    public static class FooImpl implements Foo {
        private final String foo;

        public FooImpl(String foo) {
            this.foo = foo;
        }

        @Override
        public String getFoo() {
            return foo;
        }
    }
}

results in

Exception in thread "main" com.fasterxml.jackson.databind.exc.MismatchedInputException: Cannot construct instance of `example.FooTest$Foo` (although at least one Creator exists): no String-argument constructor/factory method to deserialize from String value ('foo')
 at [Source: (String)""foo""; line: 1, column: 1]
	at com.fasterxml.jackson.databind.exc.MismatchedInputException.from(MismatchedInputException.java:63)
	at com.fasterxml.jackson.databind.DeserializationContext.reportInputMismatch(DeserializationContext.java:1342)
	at com.fasterxml.jackson.databind.DeserializationContext.handleMissingInstantiator(DeserializationContext.java:1031)
	at com.fasterxml.jackson.databind.deser.ValueInstantiator._createFromStringFallbacks(ValueInstantiator.java:370)
	at com.fasterxml.jackson.databind.deser.std.StdValueInstantiator.createFromString(StdValueInstantiator.java:314)
	at com.fasterxml.jackson.databind.deser.BeanDeserializerBase.deserializeFromString(BeanDeserializerBase.java:1351)
	at com.fasterxml.jackson.databind.deser.BeanDeserializer._deserializeOther(BeanDeserializer.java:170)
	at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserialize(BeanDeserializer.java:161)
	at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4001)
	at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:2992)
        at example.FooTest.main(FooTest.java:17)

It works correctly if ParamterNamesModule is removed, or if Foo and FooImpl are combined.

@twilson-palantir
Copy link
Author

Hmm... This only repros in eclipse for me, running eclipse 4.6.2.

I'm guessing that the version of ecj bundled with that eclipse version isn't supporting parameter name reflection the same way javac does.

@cowtowncoder
Copy link
Member

Odd. It seems that static method with delegation should work.

But you may want to add explicit mode in @JsonCreator (specifically, mode = DELEGATING), just in case it would otherwise auto-detect it as property-based -- possible, since implicit name is available with parameter names.

Also: when reporting problems please indicate Jackson version in question (... and if not latest patch of minor version, try out the latest patch, since I'll ask for that anyway :) )

@twilson-palantir
Copy link
Author

Version is 2.9.2. With mode = DELEGATING it works in eclipse. I'm trying to get it to compile with ecj in gradle and run that, not sure if I've done it right, but I haven't gotten that to fail yet.

@twilson-palantir
Copy link
Author

Stepping through a debugger in eclipse, I see in BeanDeserializerFactory#_addExplicitAnyCreator

        // If there's injection or explicit name, should be properties-based
        boolean useProps = (paramName != null) || (injectId != null);
        if (!useProps && (paramDef != null)) {
            // One more thing: if implicit name matches property with a getter
            // or field, we'll consider it property-based as well
            paramName = candidate.findImplicitParamName(0);
            useProps = (paramName != null) && paramDef.couldSerialize();
        }
        if (useProps) {
            SettableBeanProperty[] properties = new SettableBeanProperty[] {
                    constructCreatorProperty(ctxt, beanDesc, paramName, 0, param, injectId)
            };
            creators.addPropertyCreator(candidate.creator(), true, properties);
            return;
        }
        _handleSingleArgumentCreator(creators, candidate.creator(), true, true);

Both paramName and injectId are null, paramDef is

[Property 'foo'; ctors: [parameter #0, annotations: [null]][visible=true,ignore=false,explicitName=false], field(s): null, getter(s): [method example.FooTest$Foo#getFoo(0 params)][visible=true,ignore=false,explicitName=false], setter(s): null]

This ends up passing through both if blocks and setting a property creator instead of going through _handleSingleArgumentCreator which would create a string creator.

@cowtowncoder
Copy link
Member

@twilson-palantir I have a feeling that one of fixes for jackson-databind, for 2.9.4 might be relevant here -- there was a case where single-String-argument was not properly detected:

FasterXML/jackson-databind#1853

I am hoping to release 2.9.4 soon. If you are able to locally build 2.9.4-SNAPSHOT from branch 2.9 it might be worth trying out.

@twilson-palantir
Copy link
Author

I'm a bit skeptical that will help since when it gets to

            switch (creatorMode) {
            case DELEGATING:
                _addExplicitDelegatingCreator(ctxt, beanDesc, creators,
                        CreatorCandidate.construct(intr, ctor, null));
                break;
            case PROPERTIES:
                _addExplicitPropertyCreator(ctxt, beanDesc, creators,
                        CreatorCandidate.construct(intr, ctor, creatorParams.get(ctor)));
                break;
            default:
                _addExplicitAnyCreator(ctxt, beanDesc, creators,
                        CreatorCandidate.construct(intr, ctor, creatorParams.get(ctor)));
                break;
            }

it ends up calling _addExplicitAnyCreator, not _addExplicitDelegatingCreator, but I'm willing to try it out.

I'm not familiar with building maven projects though and I can't find any build instructions anywhere. Just running mvn install in jackson-databind on the 2.9 branch gives me

[INFO] Scanning for projects...
[ERROR] [ERROR] Some problems were encountered while processing the POMs:
[FATAL] Non-resolvable parent POM for com.fasterxml.jackson.core:jackson-databind:2.9.4-SNAPSHOT: Could not find artifact com.fasterxml.jackson:jackson-base:pom:2.9.4-SNAPSHOT and 'parent.relativePath' points at wrong local POM @ line 5, column 11
 @ 
[ERROR] The build could not read 1 project -> [Help 1]
[ERROR]   
[ERROR]   The project com.fasterxml.jackson.core:jackson-databind:2.9.4-SNAPSHOT (/Users/twilson/code/jackson-databind/pom.xml) has 1 error
[ERROR]     Non-resolvable parent POM for com.fasterxml.jackson.core:jackson-databind:2.9.4-SNAPSHOT: Could not find artifact com.fasterxml.jackson:jackson-base:pom:2.9.4-SNAPSHOT and 'parent.relativePath' points at wrong local POM @ line 5, column 11 -> [Help 2]
[ERROR] 
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR] 
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/ProjectBuildingException
[ERROR] [Help 2] http://cwiki.apache.org/confluence/display/MAVEN/UnresolvableModelException

@cowtowncoder
Copy link
Member

Explicit will ever occur with mode laid out; otherwise it will be trying to use heuristics to figure out users intent. So that would not change.

In the end I think that adding explicit mode declaration is the right thing to do: there isn't much benefit in trying to make ambiguous case work.

@twilson-palantir
Copy link
Author

I'm ok with adding the mode declaration as the fix for this case. I'm also ok with having no mode declaration result in an error.

I'm less ok with the actual error that got returned since it's simply wrong, there is a string-argument factory constructor. Based on the error I have no way of knowing I might want to set mode = DELEGATING.

I'm also less ok with this working differently in eclipse and gradle with no explanation as to why they differ. In both cases I ran ecj, I also ran with -verbose:class to verify that the same jdk and jackson versions were being used and nothing else on the classpath was involved.

@cowtowncoder
Copy link
Member

mode is only needed for 1-argument case. And although I would agree that it is unfortunate there is this particular case where combination of parameter names existing (or not!) adding exception would cause way bigger problems for many other users, who are relying on functionality working. And based on feedback (issues reported) I am certain that adding exception even for 3.0 would generate metric tons of complaints on breaking something that used to work.

I don't know how to resolve this for 3.0, either; with one exception that rule for selecting in ambiguous case can be simplified to ALWAYS be Delegating OR ALWAYS Properties-based.
And NOT trying to use heuristics, which are a lost cause due to complexity of everything.

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