Possibly wrong behavior of @JsonMerge

[nlisker]

Search before asking

• I searched in the issues and found nothing similar.

Describe the bug

From #4756:

Test case:

import java.util.ArrayList;
import java.util.List;

import com.fasterxml.jackson.annotation.JsonMerge;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.databind.json.JsonMapper;

// dummy interface to "confuse" type resolution
interface MyList<T> extends List<T> {}

class MergeList {

	// dummy implementation of MyList that Jackson shouldn't care about with a black-box creator
	private static class MyArrayList<T> extends ArrayList<T> implements MyList<T> {}
	static <T> MyList<T> create() {
		return new MyArrayList<T>();
	}

	@JsonMerge
	@JsonProperty
	MyList<String> values = create();

	{
		values.add("a");
	}

	public static void main(String[] args) throws Exception {
		JsonMapper MAPPER = JsonMapper.builder().build();
		MergeList mergeList = new MergeList();
		var string = MAPPER.writeValueAsString(mergeList);
		System.out.println(string);
		MergeList w = MAPPER.readValue(("{\"values\":[\"x\"]}"), MergeList.class);
		System.out.println(w.values);
	}
}

Throws

Exception in thread "main" com.fasterxml.jackson.databind.exc.InvalidDefinitionException: Cannot find a deserializer for non-concrete Collection type [collection type; class example.MyList, contains [simple type, class java.lang.String]]
 at [Source: REDACTED (`StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION` disabled); line: 1, column: 1]
	at com.fasterxml.jackson.databind.exc.InvalidDefinitionException.from(InvalidDefinitionException.java:67)
	at com.fasterxml.jackson.databind.DeserializationContext.reportBadDefinition(DeserializationContext.java:1888)
	at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCache2(DeserializerCache.java:321)
	at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCacheValueDeserializer(DeserializerCache.java:284)
	at com.fasterxml.jackson.databind.deser.DeserializerCache.findValueDeserializer(DeserializerCache.java:174)
	at com.fasterxml.jackson.databind.DeserializationContext.findNonContextualValueDeserializer(DeserializationContext.java:659)
	at com.fasterxml.jackson.databind.deser.BeanDeserializerBase.resolve(BeanDeserializerBase.java:552)
	at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCache2(DeserializerCache.java:347)
	at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCacheValueDeserializer(DeserializerCache.java:284)
	at com.fasterxml.jackson.databind.deser.DeserializerCache.findValueDeserializer(DeserializerCache.java:174)
	at com.fasterxml.jackson.databind.DeserializationContext.findRootValueDeserializer(DeserializationContext.java:669)
	at com.fasterxml.jackson.databind.ObjectMapper._findRootDeserializer(ObjectMapper.java:5048)
	at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4918)
	at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3860)
	at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3828)
	at example.MergeList.main(MergeList.java:33)

If I understood @JsonMerge correctly, it should have resolved correctly:

merging meaning that the current value is first accessed (with a getter or field) and then modified with incoming data. If merging is not used assignment happens without considering current state.
...
For Collection and Arrays, merging is done by appending incoming data into contents of existing Collection/array

But the error is that Jackson cannot find a deserializer for MyList. It doesn't need to because it can access the current value (with a getter or @JsonProperty as in this example) and modify it. Without @JsonMerge, the current state of the values field is not considered and so I expect exactly this kind of failure.
Perhaps I misunderstood @JsonMerge, but the docs are quite clear I think.

A workaround is possible via a custom setter:

private void setMyList(List<Integer> ints) { ints.forEach(MyList::add); }

But this "pollutes" the class and also requires opens declarations in module-info to be able to read the private setter.

Version Information

2.17 and 2.18

[JooHyukKim]

// dummy interface to "confuse" type resolution

Why would you try to confuse type resolution?

Below seems enough for reprouction.

class MergeListTest {
    private static class MyArrayList<T> extends ArrayList<T> {
        static int constructorCount = 0;
        public MyArrayList() {
            if (constructorCount > 0) {
                throw new Error("Should only be called once");
            }
            ++constructorCount;
        }
    }

    public static class MergeList {
        @JsonMerge
        @JsonProperty
        List<String> values = create();

    }

    public static List<String> create() {
        List<String> list = new MyArrayList<>();
        list.add("a");
        return list;
    }

    public static void main(String[] args) throws Exception {
        JsonMapper MAPPER = JsonMapper.builder().build();
        MergeList mergeList = new MergeList();

        // Fails with exception here
        MergeList w = MAPPER.readValue(("{\"values\":[\"x\"]}"), MergeList.class);
    }
}

[JooHyukKim]

I don't know honestly. The documentation does not mention anything about instantiation. The part of JavaDoc shared...

For Collection and Arrays, merging is done by appending incoming data into contents of existing Collection/array

... also could be understood as "incoming collection into existing collection", though the expressions may be ambiguous?
Just because we read and understood JavaDoc this way and implementation works the other way shouldn't mean it's bug --that would be too harsh. Let's wait for @cowtowncoder. We weren't there to help write documentations.

Additionally, here is blog written about, when @JsonMerge first came out. You may find some more hint on what original intention would be.

[cowtowncoder]

I think this case should work so it seems like a legit bug. There are quite a few @JsonMerge tests under com.fasterxml.jackson.databind.deser.merge, including CollectionMergeTest. But I suspect none tests custom List for which deserializer can't be created -- so you may have uncovered an edge case @nlisker .

Checking in simplified/minimal reproduction under src/test/java/.../tofix would make sense.