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

Getter that returns an abstract collection breaks a delegating @JsonCreator #2251

Closed
2is10 opened this issue Jan 25, 2019 · 6 comments
Closed
Milestone

Comments

@2is10
Copy link

2is10 commented Jan 25, 2019

Using version 2.9.2 or 2.9.8 (and probably many other versions), the following test throws:

JsonMappingException: Cannot find a deserializer for non-concrete Collection type [collection type; class java.util.AbstractSet, contains [simple type, class java.lang.Integer]]

    @Test
    public void testDeserializeValSet() throws IOException {
        new ObjectMapper().readValue("[1,2,3]", ValSet.class);
    }

    static class ValSet {
        @JsonValue
        private final AbstractSet<Integer> vals;

        @JsonCreator(mode = Mode.DELEGATING)
        ValSet(Collection<Integer> vals) {
            this.vals = new HashSet<>(vals);
        }

        public AbstractSet<Integer> getVals() {
            return vals;
        }
    }

This is a spurious error since the @JsonCreator constructor only requires Collection<Integer>, not AbstractSet<Integer>.

Some workarounds:

  • Delete getVals().
  • Make getVals() non-public.
  • Move @JsonValue to getVals().
  • Add @JsonIgnore to getVals().
  • Add @JsonAutoDetect(getterVisibility = Visibility.NONE) to ValSet.
@cowtowncoder
Copy link
Member

@2is10 thank you for reporting this: with quick glance this does seem like a bug: for deserialization, the highest precedence information should be constructor parameter which declares expected type of Collection<Integer>, and whatever type serialization side has (getter) should not override this (not @JsonValue, which should not even come to play here).

@cowtowncoder cowtowncoder transferred this issue from FasterXML/jackson-datatypes-collections Feb 9, 2019
@cowtowncoder cowtowncoder added this to the 2.9.9 milestone Feb 10, 2019
@cowtowncoder
Copy link
Member

Ok. So, one thing I fixed first is that for some reason there were no fallback mappings for AbstractSet (or AbstractList or AbstractMap). There should be -- same ones used for Set, List and Map. Doing that works around this issue.

But I'll see if I can also find why type detection prefers AbstractSet here: while it is more refined, it is not from the most specific accessor to use for deserialization.
It is possible this is related to one known problem with Creator, creator properties being separate set from "regular" properties (and unification occurring too late in the process), and if so, that can't be solved before 3.0. But it is not yet clear if that is the case or not.

@cowtowncoder
Copy link
Member

Hmmh. Ok, so accessor problem comes via "setterless" property, which is concept wherein "getter" may be used as sort-of setter if nothing else is available. And since what we have here is delegating constructor (and NOT setter-based), there is attempt to find deserializer for theoretical use for property vals...

This is not optimal, but I don't think I can solve that for time being, so I will let the preliminary fix take care of short-term problem here. But I'll see if I can write a separate failing test for this problem.

@cowtowncoder
Copy link
Member

Created #2252 as followup for this issue.

@cowtowncoder cowtowncoder changed the title A getter that returns an abstract collection breaks a delegating @JsonCreator Getter that returns an abstract collection breaks a delegating @JsonCreator Feb 10, 2019
cowtowncoder added a commit that referenced this issue Feb 10, 2019
@2is10
Copy link
Author

2is10 commented Feb 10, 2019

Thanks @cowtowncoder!

Note: In my actual code, the abstract Set class that triggered the problem was Guava's ImmutableSet, so I'm personally still invested in a solution to #2252. (I didn’t want to introduce a dependency on jackson-datatypes-collections/guava as a workaround.)

@cowtowncoder
Copy link
Member

@2is10 Ok. Yes, I hope to solve that one still.

And just in case it might help, there is the SimpleModule.addAbstractTypeMapping() that allows specifying a mapping -- the reason it could potentially help (if you can think of a concrete subtype) is that although it is not necessary for deserializer to actually work, existence is required: so you could map abstract type to any other subtype for which a valid deserializer could be created (regardless of whether it would actually work if ever called).

Or, I guess you could also register bogus custom deserializer, which similarly should never actually get called.

Still, I hope I can fix #2252 itself in future.

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