search

FasterXML / jackson-databind

General data-binding package for Jackson (2.x): works on streaming API (core) implementation(s)
Apache License 2.0
3.51k stars 1.37k forks source link

Delegating `@JsonCreator` does not prevent unnecessary introspection of properties, leading to bogus failure #3216

Open awildturtok opened 3 years ago

awildturtok commented 3 years ago

Describe the bug

I am trying to Serialize a class that has weird map-Types (eg. String[] as key), instead of implementing a Map(De)Serializer, I went the way of a @JsonValue and @JsonCreator, using an intermediate Container class as intermediary. This does work, however Jackson is giving me Errors about not finding appropriate Key-Deserializers (Which of course is true, but shouldn't happen as I am explicitly forgoing them). When I mark the Map field with @JsonIgnore, the Objects are properly (de)serialized.

com.fasterxml.jackson.databind.exc.InvalidDefinitionException: Cannot find a (Map) Key deserializer for type [array type, component type: [simple type, class java.lang.String]]
 at [Source: (String)"{"keys":[["a","b"]],"values":["c"]}"; line: 1, column: 1]

    at com.fasterxml.jackson.databind.exc.InvalidDefinitionException.from(InvalidDefinitionException.java:67)
    at com.fasterxml.jackson.databind.DeserializationContext.reportBadDefinition(DeserializationContext.java:1592)
    at com.fasterxml.jackson.databind.deser.DeserializerCache._handleUnknownKeyDeserializer(DeserializerCache.java:602)
    at com.fasterxml.jackson.databind.deser.DeserializerCache.findKeyDeserializer(DeserializerCache.java:168)
    at com.fasterxml.jackson.databind.DeserializationContext.findKeyDeserializer(DeserializationContext.java:502)
    at com.fasterxml.jackson.databind.deser.std.MapDeserializer.createContextual(MapDeserializer.java:248)
    at com.fasterxml.jackson.databind.DeserializationContext.handlePrimaryContextualization(DeserializationContext.java:653)
    at com.fasterxml.jackson.databind.deser.BeanDeserializerBase.resolve(BeanDeserializerBase.java:489)
    at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCache2(DeserializerCache.java:293)
    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:479)
    at com.fasterxml.jackson.databind.ObjectReader._findRootDeserializer(ObjectReader.java:2050)
    at com.fasterxml.jackson.databind.ObjectReader._bindAndClose(ObjectReader.java:1714)
    at com.fasterxml.jackson.databind.ObjectReader.readValue(ObjectReader.java:1261)
    at com.bakdata.conquery.models.identifiable.mapping.EntityIdMapTest.test(EntityIdMapTest.java:70)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base/java.lang.reflect.Method.invoke(Method.java:566)
    at org.junit.platform.commons.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:688)
    at org.junit.jupiter.engine.execution.MethodInvocation.proceed(MethodInvocation.java:60)
    at org.junit.jupiter.engine.execution.InvocationInterceptorChain$ValidatingInvocation.proceed(InvocationInterceptorChain.java:131)
    at org.junit.jupiter.engine.extension.TimeoutExtension.intercept(TimeoutExtension.java:149)
    at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestableMethod(TimeoutExtension.java:140)
    at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestMethod(TimeoutExtension.java:84)
    at org.junit.jupiter.engine.execution.ExecutableInvoker$ReflectiveInterceptorCall.lambda$ofVoidMethod$0(ExecutableInvoker.java:115)
    at org.junit.jupiter.engine.execution.ExecutableInvoker.lambda$invoke$0(ExecutableInvoker.java:105)
    at org.junit.jupiter.engine.execution.InvocationInterceptorChain$InterceptedInvocation.proceed(InvocationInterceptorChain.java:106)
    at org.junit.jupiter.engine.execution.InvocationInterceptorChain.proceed(InvocationInterceptorChain.java:64)
    at org.junit.jupiter.engine.execution.InvocationInterceptorChain.chainAndInvoke(InvocationInterceptorChain.java:45)
    at org.junit.jupiter.engine.execution.InvocationInterceptorChain.invoke(InvocationInterceptorChain.java:37)
    at org.junit.jupiter.engine.execution.ExecutableInvoker.invoke(ExecutableInvoker.java:104)
    at org.junit.jupiter.engine.execution.ExecutableInvoker.invoke(ExecutableInvoker.java:98)
    at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.lambda$invokeTestMethod$6(TestMethodTestDescriptor.java:210)
    at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
    at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.invokeTestMethod(TestMethodTestDescriptor.java:206)
    at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:131)
    at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:65)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:139)
    at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:129)
    at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:127)
    at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:126)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:84)
    at java.base/java.util.ArrayList.forEach(ArrayList.java:1540)
    at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:38)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:143)
    at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:129)
    at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:127)
    at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:126)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:84)
    at java.base/java.util.ArrayList.forEach(ArrayList.java:1540)
    at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:38)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:143)
    at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:129)
    at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:127)
    at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:126)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:84)
    at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.submit(SameThreadHierarchicalTestExecutorService.java:32)
    at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor.execute(HierarchicalTestExecutor.java:57)
    at org.junit.platform.engine.support.hierarchical.HierarchicalTestEngine.execute(HierarchicalTestEngine.java:51)
    at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:108)
    at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:88)
    at org.junit.platform.launcher.core.EngineExecutionOrchestrator.lambda$execute$0(EngineExecutionOrchestrator.java:54)
    at org.junit.platform.launcher.core.EngineExecutionOrchestrator.withInterceptedStreams(EngineExecutionOrchestrator.java:67)
    at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:52)
    at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:96)
    at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:75)
    at com.intellij.junit5.JUnit5IdeaTestRunner.startRunnerWithArgs(JUnit5IdeaTestRunner.java:71)
    at com.intellij.rt.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:33)
    at com.intellij.rt.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:220)
    at com.intellij.rt.junit.JUnitStarter.main(JUnitStarter.java:53)

Version information

2.10.5

To Reproduce


import java.util.*;

import com.fasterxml.jackson.annotation.*
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import org.junit.jupiter.api.Test;

class Test {

    public static class Container {

        public List<String[]> keys;
        public List<String> values;

        public Container() {}

        public Container(List<String[]> keys, List<String> values) {
            this.keys = keys;
            this.values = values;
        }
    }

    public static class Data {
        @JsonIgnore // Without this, deserialization will fail
        public Map<String[], String> map = new HashMap<>();

        public Data() {

        }

        @JsonValue
        public Container getContainer() {
            List<String[]> keys = new ArrayList<>();
            List<String> values = new ArrayList<>();

            map.forEach((key, value) -> {
                keys.add(key);
                values.add(value);
            });

            return new Container(keys,values);
        }

        @JsonCreator
        public Data(Container container) {
            for (int index = 0; index < container.keys.size(); index++) {
                map.put(container.keys.get(index), container.values.get(index));
            }
        }
    }

    @Test
    public void test() throws JsonProcessingException {
        final ObjectMapper mapper = new ObjectMapper();

        final Data data = new Data();
        data.map = Map.of(new String[]{"a", "b"}, "c");

        final String serialized = mapper.writeValueAsString(data);

        final Object data2 = mapper.readerFor(Data.class).readValue(serialized);
    }

}

Expected behavior

I would expect that a class annotated with @JsonCreator will not try to find deserializers for it's fields, or at least only for those properties not covered by the @JsonCreator (ie after the fact).

cowtowncoder commented 3 years ago

Hmmhm.

Unfortunately this may actually work as expected: existence of @JsonCreator does not mean that no properties would be used: it is expected that only some of properties would be passed through constructor. But this would only apply for "as-properties" case, however; in this case I think mode is expected to be delegation style, as if creator annotation was:

@JsonCreator(mode = JsonCreator.Mode.DELEGATING)

and if so, no real properties will be used. Code probably is not structured in a way to be able to avoid property introspection, however.

One thing to try is to use an up-to-date version of databind -- 2.12.4 is the latest. But I don't expect that to make a difference here.

So, this seems like something that would be good to improve on but not necessarily easy to do.

awildturtok commented 3 years ago

Thanks for your answer. I suspected as much, after reading other issues related to JsonCreator.

So, this seems like something that would be good to improve on but not necessarily easy to do.

I agree it's not extremely important, but would lazy/ad-hoc loading property deserializers not be sufficient here? It seems to me, that at the moment Jackson tries to cover all bases before deserializing, but if the Creator already consumed all properties we shouldn't try to introspect the others?

cowtowncoder commented 3 years ago

@awildturtok Right, the challenge tho is that public property with name map would infer existence of property map which is not covered by another property. Logically of course "delegating" approach will prevent need and use of any properties, so this SHOULD be detected by Jackson. So it is more of a limitation. It is also true that in some cases processing may be deferred, but this is bit of double-edged sword -- many users prefer early failure on detected issues; whereas in some cases the opposite is desired (theoretical problem never encountered).

I think there is a valid improvement to do here so I changed the title slightly, marked for consideration in 2.14, and I hope this can be tackled in future.

In the meantime, use of @JsonIgnore is the recommended work-around if anyone else needs it.