Add missing @JsonIdentityInfo handling for implicit Collections with DeserializationFeature.ACCEPT_SINGLE_VALUE_AS_ARRAY

[MoritzR200]

The issue fixed

The added unit test (see below) fails without my fix, because before in the CollectionDeserializer the CollectionReferringAccumulator (that is to say proper reference handling) was only used in the case where the collection deserialized is represented by a proper JSON-array, which is however not the case for implicit single element collections, when DeserializationFeature.ACCEPT_SINGLE_VALUE_AS_ARRAY is enabled.
So (analogous to the previous handling for proper JSON-arrays) I added handling for implicit collections under the above circumstances, resolving the issue.

The added unit test for reference (MAPPER has DeserializationFeature.ACCEPT_SINGLE_VALUE_AS_ARRAY):

    static class Bean1421C {
        Collection<IdentifiedType> collection;
        IdentifiedType value;

        public void setValue(IdentifiedType value) {
            this.value = value;
        }

        public void setCollection(Collection<IdentifiedType> collection) {
            this.collection = collection;
        }
    }

    @JsonIdentityInfo(generator=ObjectIdGenerators.IntSequenceGenerator.class)
    static class IdentifiedType {
        String entry;

        @JsonCreator
        IdentifiedType(@JsonProperty("entry") String entry)
        {
            this.entry = entry;
        }
    }



    @Test
    public void testCollectionWithObjectId() throws IOException
    {
        Bean1421C result = MAPPER.readValue("{\"collection\":1,\"value\":{\"@id\":1,\"entry\":\"s\"}}", Bean1421C.class);
        assertNotNull(result);
        assertNotNull(result.value);
        assertEquals(1, result.collection.size());
        assertNotNull(result.collection.iterator().next());
        assertEquals("s", result.collection.iterator().next().entry);
    }

Related other issue

While fixing the above issue I also noticed that the ObjectArrayDeserializer has the same issue, because it has no handling of object references whatsoever. So the following test cases, fail both with and without the DeserializationFeature.ACCEPT_SINGLE_VALUE_AS_ARRAY (though the first test should fail without that feature).
I have created issue #5538 for that, because implementing that seams like something best left to the maintainers, since it (as far as I can tell) will not be trivial.

    @JsonIdentityInfo(generator=ObjectIdGenerators.IntSequenceGenerator.class)
    static class IdentifiedType {
        String entry;

        @JsonCreator
        IdentifiedType(@JsonProperty("entry") String entry)
        {
            this.entry = entry;
        }
    }

    static class Bean1421D {
        IdentifiedType[] array;
        IdentifiedType value;

        public void setValue(IdentifiedType value) {
            this.value = value;
        }

        public void setArray(IdentifiedType[] array) {
            this.array = array;
        }
    }



    @Test
    public void testImplicitArrayWithObjectId() throws IOException
    {
        Bean1421D result = MAPPER.readValue("{\"array\":1,\"value\":{\"@id\":1,\"entry\":\"s\"}}", Bean1421D.class);
        assertNotNull(result);
        assertNotNull(result.value);
        assertEquals(1, result.array.length);
        assertNotNull(result.array[0]);
        assertEquals("s", result.array[0].entry);
    }



    @Test
    public void testExplicitArrayWithObjectId() throws IOException
    {
        Bean1421D result = MAPPER.readValue("{\"array\":[1],\"value\":{\"@id\":1,\"entry\":\"s\"}}", Bean1421D.class);
        assertNotNull(result);
        assertNotNull(result.value);
        assertEquals(1, result.array.length);
        assertNotNull(result.array[0]);
        assertEquals("s", result.array[0].entry);
    }

Potentially related issues

I am uncertain, whether #2780 might be related to this or not.
Other that that I found no related issues.

PS

Did I target the right branch? In jackson/CONTRIBUTING.md it says, that I should target the current stable branch (2.17??) which seems outdated. (It also mentions an nonexistent master branch.) So I changed to 2.20, but the other recent pull requests do target 3.x (even if they are small bug fixes), so you might want to consider updating your explanation.

Also It took me some time to figure out, that jackson-base (a required build-dependency) was contained in the jackson-bom project which is not listed as a dependency of this project, so you also might want to explain that somewhere to make creating pull requests a bit easier.

[cowtowncoder]

Or maybe _containerType.getContentType().getRawClass() instead of Object.class?

[MoritzR200]

That was copied from handleNonArray. I adjusted Object.class to _containerType.getContentType().getRawClass() in both places, because it seems more logical (and the tests still pass).

[cowtowncoder]

Did I target the right branch? In jackson/CONTRIBUTING.md it says, that I should target the current stable branch (2.17??) which seems outdated. (It also mentions an nonexistent master branch.) So I changed to 2.20, but the other recent pull requests do target 3.x (even if they are small bug fixes), so you might want to consider updating your explanation.

Good point -- docs are bit out of sync. The current branch for Jackson 2 would actually be 2.x, but 2.20 is fine. 2.18 is the oldest open branch for Jackson 2.
We can relatively easily merge forward from 2.x to 3.x, so 2.20 works quite well (I'll merge to 2.20, from that to 2.x, and from that either 3.0 or 3.x (which is for 3.1.0).

But basically I can handle the details.

[cowtowncoder]

@MoritzR200 Ok: first of all, thank you for contributing this! It seems like a good addition.

And looks like you already sent CLA!!! Great. I'll try to get this reviewed soon (tomorrow if all goes well).

[cowtowncoder]

Merged into 2.20 branch (for 2.20.2); 2.x (for 2.21.0) and 3.x (for 3.1.0).