FasterXML/jackson-module-parameter-names

Jackson JDK8 Data Type and Parameter Names modules don't play together?

ekis opened this issue · 20 comments

ekis commented

Not sure if I should be putting this issue on this project or jdk8-data-type, so I'll crosspost.

The issue synopsis is given here - essentially, it seems as Parameter Names module does not work with JDK8 Optional<T>, requiring the constructor to be peppered with @JsonProperty("propertyName") code.

Am I doing something wrong, is there a limitation in javac or in one of the modules?

As far as I'm aware, jdk 8 code is not compiled with -parameters option meaning that none of the jdk 8 classes have parameter names stored in their class files.

ekis commented

Indeed, but the as the synopsis states - javac had been passed -parameters. That didn't help.

Yes, but you are only compiling your project java files with your compiler, not the jdk ones.

Added my commentary on https://github.com/FasterXML/jackson-datatype-jdk8/issues/16 but the long & short:

I suspect this is due to a known issue, which requires use of @JsonCreator for constructors, even though ideally it should not be used. Problem was supposed to be resolved in 2.6, but I realized too late that it was not completely resolved, and there remains work to be done for 2.7.
(underlying issue is that existing constructors are pruned too early, so that auto-detection that would detect either explicit annotations of parameters (names etc), or presence of implicit names).

So, with 2.6, @JsonCreator is still needed. @JsonProperty, on the other hand, should not be needed, as long as javac has compiled parameter names in.

Thanks for input, Tatu. Would you also provide the answer to the SO question, or should I?

ekis commented

I left a comment on https://github.com/FasterXML/jackson-datatype-jdk8/issues/16, testing with @JsonCreator - in short, adding the annotation to constructor did not help.

I think Lovro made a salient point here about JDK8 lacking the -parameters option - it makes sense to me, I have completely forgotten about that.

ekis commented

I will close the issue here - anyone interested in further details may head over to the other thread.

Hvala Lovro and thanks Tatu for your help and time.

I've tested the following code and the test passes:

public class OptionalTest {

    @Test
    public void shouldDeserialize() throws IOException {

        // given
        ObjectMapper objectMapper = new ObjectMapper();
        objectMapper.registerModule(new Jdk8Module());
        objectMapper.registerModule(new ParameterNamesModule());

        // when
        String json = "{\"s1\":\"a\",\"map\":{}}";
        SimpleTest simpleTest = objectMapper.readValue(json, SimpleTest.class);

        then(simpleTest).isEqualToComparingFieldByField(new SimpleTest(Optional.of("a"), Optional.empty(), new HashMap<>()));
    }

    private static class SimpleTest {
        private Optional<String> s1;
        private Optional<String> s2;
        private Map<String, String> map;

        private SimpleTest(Optional<String> s1, Optional<String> s2, Map<String, String> map) {
            this.s1 = s1;
            this.s2 = s2;
            this.map = map;
        }

        static SimpleTest of(Optional<String> s1, Optional<String> s2, Map<String, String> m) {
            return new SimpleTest(s1, s2, m);
        }
    }
}

Note that this was tested against latest state in jackson-parameter-name-modules with all dependencies set to 2.6.2.

ekis commented

Yes, it does and I can confirm your test - out of the blue, IDEA simply decided to remove the javac parameter from compile settings, which is why I reported false failures.

So I guess this is a false alarm then?

IDEA simply decided to remove the javac parameter from compile settings, which is why I reported false failures.

Check if you have changed default and project settings in IDEA.
To change default settings close all open projects and in the entry IDEA window select settings and add parameters option to the compiler (on the top of tab it should state 'For default project' and not 'For current project')

ekis commented

It seems like it - this is infuriating but at least gave us a chance to test this area one tiny bit better.

I did not change any default/project settings but have just confirmed that Default Project settings has javac parameter, yet the Current Project settings does not. Why does it disappear?

Change in default does not change settings for existing projects, only for new projects, maybe that is the issue?

ekis commented

Are you saying there is no way to 'inherit' the default settings?

There is, but only for new projects that you checkout in IDEA. Try deleting IDEA files in your project and checkout the project again in IDEA.

ekis commented

Well, that will be left for some other day - for now, I'll just set the current project settings and see if misbehaves again. It shouldn't, especially now that defaults are identical to current.

👍

Ok. On plus side, problem diagnosed and resolved. Downside is that this handling seems very spotty -- not just with IDEA, but I recall Eclipse also being somewhat unreliable in this respect. I moved to only using command-line Maven for running tests for this project, fwtw.

ekis commented

I have done the same - SBT-based build passes the argument to javac during compilation but given existing IDEs, I don't think there is anything realistic what we can do, short of doing a spot-check during bootstrap of jackson-module-parameter-names, detecting the bytecode missing the compile data and . throwing a (runtime) exception? Spot-check could be done on the class that constructs the ObjectMapper or perhaps even the module itself?

At least that would save future hours of ghostbusting but I don't know if that is even feasible.

Yeah it would be nice to have some sanity checks, but I suspect it may be difficult to do reliably. Detecting absence of things, especially since it may legitimately affect some things and not others (that is to say, there may be libs that do not require inclusion and are compiled without that info) makes it difficult to know when exactly we have a problem.