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

@JsonInject fails on trying to find deserializer even if inject-only #962

Closed
david-bakin opened this issue Oct 7, 2015 · 15 comments
Closed
Milestone

Comments

@david-bakin
Copy link

@JsonInject tries to "process" the injected class for deserialization even though it is not deserialized. As a result, you may get errors if the class is not suitable for deserialization, e.g., two getters with the same name but different signature. Jackson 2.6.1.

The following code fails as is - stack trace below - but works if you comment out one of the setA methods on class InjectMe. (Uses TestNG, AssertJ.)

(I believe it is probably a common use case that you might use @JsonInject to inject instances of classes from your application environment that you never expect to serialize/deserialize. They may very well have multiple setters with the same name but different signature. ObjectMapper itself is like this: that's how I discovered this issue.)

import java.io.IOException;

import org.testng.annotations.Test;

import com.fasterxml.jackson.annotation.JacksonInject;
import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.databind.InjectableValues;
import com.fasterxml.jackson.databind.ObjectMapper;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.StrictAssertions.fail;

public class TestInject
{
    public static class InjectMe
    {
        private String a;

        public void setA(String a) {
            this.a = a;
        }

        public void setA(Integer a) {
            this.a = a.toString();
        }

        public String getA() {
            return a;
        }
    }

    public static class Injectee
    {
        private String b;

        @JsonCreator
        public Injectee(@JacksonInject InjectMe injectMe, @JsonProperty("b") String b) {
            this.b = b;
        }

        public String getB() {
            return b;
        }
    }

    @Test
    public void injected() {
        // ARRANGE
        InjectMe im = new InjectMe();
        ObjectMapper sut = new ObjectMapper()
            .setInjectableValues(new InjectableValues.Std().addValue(TestInject.InjectMe.class, im));
        String test = "{\"b\":\"bbb\"}";

        // ACT
        Injectee actual = null;
        try {
            actual = sut.readValue(test, Injectee.class);
        }
        catch (IOException e) {
            fail("failed to deserialize", e);
        }

        // ASSERT
        assertThat(actual.getB()).isEqualTo("bbb");
    }
}

Stacktrace:


FAILED: injected
java.lang.AssertionError: failed to deserialize
    at com.ispot.TestInject.injected(TestInject.java:64)
    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:497)
    at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:85)
    at org.testng.internal.Invoker.invokeMethod(Invoker.java:639)
    at org.testng.internal.Invoker.invokeTestMethod(Invoker.java:821)
    at org.testng.internal.Invoker.invokeTestMethods(Invoker.java:1131)
    at org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:124)
    at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:108)
    at org.testng.TestRunner.privateRun(TestRunner.java:773)
    at org.testng.TestRunner.run(TestRunner.java:623)
    at org.testng.SuiteRunner.runTest(SuiteRunner.java:357)
    at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:352)
    at org.testng.SuiteRunner.privateRun(SuiteRunner.java:310)
    at org.testng.SuiteRunner.run(SuiteRunner.java:259)
    at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:52)
    at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:86)
    at org.testng.TestNG.runSuitesSequentially(TestNG.java:1185)
    at org.testng.TestNG.runSuitesLocally(TestNG.java:1110)
    at org.testng.TestNG.run(TestNG.java:1018)
    at org.testng.remote.RemoteTestNG.run(RemoteTestNG.java:111)
    at org.testng.remote.RemoteTestNG.initAndRun(RemoteTestNG.java:204)
    at org.testng.remote.RemoteTestNG.main(RemoteTestNG.java:175)
Caused by: com.fasterxml.jackson.databind.JsonMappingException: Conflicting setter definitions for property "a": com.ispot.TestInject$InjectMe#setA(1 params) vs com.ispot.TestInject$InjectMe#setA(1 params)
    at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCache2(DeserializerCache.java:269)
    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.findContextualValueDeserializer(DeserializationContext.java:428)
    at com.fasterxml.jackson.databind.deser.impl.PropertyBasedCreator.construct(PropertyBasedCreator.java:79)
    at com.fasterxml.jackson.databind.deser.BeanDeserializerBase.resolve(BeanDeserializerBase.java:549)
    at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCache2(DeserializerCache.java:296)
    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:461)
    at com.fasterxml.jackson.databind.ObjectMapper._findRootDeserializer(ObjectMapper.java:3804)
    at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:3698)
    at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:2714)
    at com.ispot.TestInject.injected(TestInject.java:61)
    ... 24 more
Caused by: java.lang.IllegalArgumentException: Conflicting setter definitions for property "a": com.ispot.TestInject$InjectMe#setA(1 params) vs com.ispot.TestInject$InjectMe#setA(1 params)
    at com.fasterxml.jackson.databind.introspect.POJOPropertyBuilder.getSetter(POJOPropertyBuilder.java:299)
    at com.fasterxml.jackson.databind.deser.BeanDeserializerFactory.filterBeanProps(BeanDeserializerFactory.java:592)
    at com.fasterxml.jackson.databind.deser.BeanDeserializerFactory.addBeanProps(BeanDeserializerFactory.java:488)
    at com.fasterxml.jackson.databind.deser.BeanDeserializerFactory.buildBeanDeserializer(BeanDeserializerFactory.java:229)
    at com.fasterxml.jackson.databind.deser.BeanDeserializerFactory.createBeanDeserializer(BeanDeserializerFactory.java:142)
    at com.fasterxml.jackson.databind.deser.DeserializerCache._createDeserializer2(DeserializerCache.java:403)
    at com.fasterxml.jackson.databind.deser.DeserializerCache._createDeserializer(DeserializerCache.java:352)
    at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCache2(DeserializerCache.java:264)
    ... 37 more
@cowtowncoder
Copy link
Member

Yes, this is unfortunate. It may also be tricky to resolve, given that for injectable properties it is actually acceptable to alternatively get an override from content; deserializer is actually constructed just in case this is to occur. That is an intended feature, to allow for default/override behavior.
But at the same time, it is not needed by all (or perhaps even many or most) use cases.

So long and short is that you are right, such ambiguities should not prevent usage.

@david-bakin
Copy link
Author

OK. The workaround turned out to be trivial, once I thought of it - use a mixin. I just add this module to my mapper:

    @JsonIgnoreType
    private static class IgnoreMe {  };

    public Module getIgnoreJacksonModule() {
        return new SimpleModule().setMixInAnnotation(ObjectMapper.class, IgnoreMe.class);
    }

Now I can inject my class which has a property that returns an ObjectMapper.

@cowtowncoder
Copy link
Member

@david-bakin Nice! Very creative & clever use of mix-ins, for good cause. Thank you for sharing it.

cowtowncoder added a commit that referenced this issue May 5, 2016
@cowtowncoder
Copy link
Member

With 2.7.4, does not fail with exact test since String handling is bit refined; but still fails if I add another setA(). Added a test to reproduce; should not fail this early, if at all possible.

@cowtowncoder
Copy link
Member

I think #1381 should help here, or possibly solve the problem. But we'll see once it gets implemented.

@cowtowncoder cowtowncoder added 3.x and removed 2.9 labels Nov 1, 2017
@devinrsmith
Copy link

Is it possible to give a more complete example of the mixin workaround? I can't seem to get it to work.

@cowtowncoder
Copy link
Member

I think question is for @david-bakin .

@david-bakin
Copy link
Author

Ummm. I haven't worked with this library for some years (not a problem with the library: I haven't worked in Java for some years). So I can't help (and I'm not longer at the company where I was writing this code).

But ... the "thumbs up" on my mixin comment above indicate that just that information was sufficient for two other people. So my suggestion is: Pull a jackson-databind from 2015-10-31 and see if it just works with the library as it was back then? Something may have changed with mixins ...

@cowtowncoder
Copy link
Member

@devinrsmith Would probably helpful to know what you tried. It is possible that like @david-bakin something changed since his usage, if no unit tests covered specific combination of usage.

@devinrsmith
Copy link

Thanks for your insight David.

@cowtowncoder , I've essentially got a recursive parsing issue. I'd like my json structure to reference a file, which in turn is this parsed w/ the same mapper as the original.

{
  "file": "/tmp/the-real-type.json"
}

My hope was to inject ObjectMapper and use that (after registering as injectable value):

@JsonCreator
public static RefType create(
  @JacksonInject ObjectMapper mapper,
  @JsonProperty(value = "file") File reference) throws IOException {
  return new RefType(mapper.readValue(reference, RealType.class));
}

but I was getting the same error as seen here (and a few other places).

My workaround for now is to use a custom deserializer:

public RefType deserialize(JsonParser p, DeserializationContext ctxt)
  throws IOException, JsonProcessingException {
  final ObjectMapper mapper = (ObjectMapper) p.getCodec(); // hacky
  ...

Maybe there's a more idiomatic way to accomplish this?

@cowtowncoder
Copy link
Member

@devinrsmith Hmmh. Ok, so your use case looks much like original one... I'll have a look.

Of other possible features: "Attributes" almost work, but require access to DeserializationContext (which then means custom deserializer.

But with custom deserializer (if and when that needs to be used) one thing to note is that it is usually better to read values using methods exposed by DeserializationContext: this is both more efficient and retains all configuration of mapper/ObjectReader that started the process (these are related: when going via ObjectMapper, new DeserializationContext is established, leading to some additional lookups).

@cowtowncoder
Copy link
Member

Ok looks like addition of @JsonIgnoreType, or its equivalent like so:

 mapper.configOverride(ObjectMapper.class)
    .setIsIgnoredType(Boolean.TRUE);

does NOT prevent the issue with 2.11.0.rc1 at least.

@cowtowncoder cowtowncoder changed the title @JsonInject tries to "process" injected class for deserialization even though it is not deserialized @JsonInject fails on trying to find deserializer even if inject-only Apr 15, 2020
@cowtowncoder cowtowncoder removed the 3.x label Apr 15, 2020
@cowtowncoder cowtowncoder added this to the 2.11.0 milestone Apr 15, 2020
@cowtowncoder
Copy link
Member

Ok, was actually able to resolve this after digging into codebase for a... while. Introduced accessor for "inject-only" creator properties. Things to note:

  1. Probably will not work for field/method injection (different code paths) -- may file separate issue for those
  2. Important to add useInput=OptBoolean.FALSE property to denote that can not map from input (and hence can safely skip deserializer lookup)

both of these can be addressed in future, but at least usage expressed here should now work.

@devinrsmith
Copy link

Thanks for the quick fix - always been impressed w/ your management of this library :)

@cowtowncoder
Copy link
Member

@devinrsmith Thank you! This took a while... but better late than never. :)

josephlbarnett added a commit to josephlbarnett/jackson-databind that referenced this issue May 6, 2021
As many AnnotationIntrospector implementations use default values
for useInput, allow the secondary introspector's useInput value to
combine with the primary's id to prevent losing the useInput value.

Fixes a special case of FasterXML#962 seen by the GuiceAnnotationInspector
in FasterXML/jackson-modules-base#134
josephlbarnett added a commit to josephlbarnett/jackson-databind that referenced this issue May 6, 2021
As many AnnotationIntrospector implementations use default values
for useInput, allow the secondary introspector's useInput value to
combine with the primary's id to prevent losing the useInput value.

Fixes a special case of FasterXML#962 seen by the GuiceAnnotationInspector
in FasterXML/jackson-modules-base#134
cowtowncoder pushed a commit that referenced this issue May 7, 2021
…3146)

As many AnnotationIntrospector implementations use default values
for useInput, allow the secondary introspector's useInput value to
combine with the primary's id to prevent losing the useInput value.

Fixes a special case of #962 seen by the GuiceAnnotationInspector
in FasterXML/jackson-modules-base#134
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

3 participants