Jackson Databind Behavioral change of ALLOW_FINAL_FIELDS_AS_MUTATORS in 2.18

interface MyMap<K, V> extends Map<K, V> {}

class MapImpl<K, V> extends HashMap<K, V> implements MyMap<K, V> {}

class MapUtils {

    static <K, V> MyMap<K, V> createMyMap() {
        return new MapImpl<K, V>();
    }
}

class MergeMap {

    @JsonCreator
    MergeMap() {
        System.out.println("creator for " + map.getClass().getSimpleName());
    }

//  @JsonMerge
    private final MyMap<Integer, String> map = MapUtils.createMyMap();

    public MyMap<Integer, String> getMap() {
        System.out.println("getMap");
        return map;
    }

    @Override
    public String toString() {
        return map.toString();
    }

    public static void main(String[] args) throws Exception {
        JsonMapper MAPPER = JsonMapper.builder()
                .configure(Feature.INCLUDE_SOURCE_IN_LOCATION, true)
//              .configure(MapperFeature.ALLOW_FINAL_FIELDS_AS_MUTATORS, false)
                .build();

        var merge = new MergeMap();
        merge.getMap().put(3, "ADS");

        System.out.println(merge);

        System.out.println(" == serializing --");

        var string = MAPPER.writeValueAsString(merge);
        System.out.println(string);

        System.out.println(" == deserializing --");

        var merge2 = MAPPER.readValue(string, MergeMap.class);

        System.out.println(" == checking --");

        System.out.println(merge2);
    }
}

The mapper configuration (MapperFeature.ALLOW_FINAL_FIELDS_AS_MUTATORS, false) changed its behavior and it's unclear which is correct.

In 2.18.2 the code above fails regardless of if ALLOW_FINAL_FIELDS_AS_MUTATORS is configured (and @JsonMerge doesn't help). It throws the following exception on deserialization:

creator for MapImpl
getMap
{3=ADS}
 == serializing --
getMap
{"map":{"3":"ADS"}}
 == deserializing --
creator for MapImpl
Exception in thread "main" com.fasterxml.jackson.databind.exc.InvalidDefinitionException: Cannot construct instance of `example.MyMap` (no Creators, like default constructor, exist): no default constructor found
 at [Source: (String)"{"map":{"3":"ADS"}}"; line: 1, column: 8] (through reference chain: example.MergeMap["map"])
    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.DatabindContext.reportBadDefinition(DatabindContext.java:414)
    at com.fasterxml.jackson.databind.DeserializationContext.handleMissingInstantiator(DeserializationContext.java:1375)
    at com.fasterxml.jackson.databind.deser.std.MapDeserializer.deserialize(MapDeserializer.java:439)
    at com.fasterxml.jackson.databind.deser.std.MapDeserializer.deserialize(MapDeserializer.java:32)
    at com.fasterxml.jackson.databind.deser.impl.FieldProperty.deserializeAndSet(FieldProperty.java:138)
    at com.fasterxml.jackson.databind.deser.BeanDeserializer.vanillaDeserialize(BeanDeserializer.java:310)
    at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserialize(BeanDeserializer.java:177)
    at com.fasterxml.jackson.databind.deser.DefaultDeserializationContext.readRootValue(DefaultDeserializationContext.java:342)
    at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4931)
    at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3868)
    at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3836)
    at example.MergeMap.main(MergeMap.java:67)

In 2.18.3 and above it fails with ALLOW_FINAL_FIELDS_AS_MUTATORS commented out (=true) and passes with it set to false (without @JsonMerge):

creator for MapImpl
getMap
{3=ADS}
 == serializing --
getMap
{"map":{"3":"ADS"}}
 == deserializing --
creator for MapImpl
getMap
 == checking --
{3=ADS}

With @JsonMerge it always passes.

This is probably because of https://github.com/FasterXML/jackson-databind/issues/4922. However, the behavioral change is seen without @JsonMerge: include configure(MapperFeature.ALLOW_FINAL_FIELDS_AS_MUTATORS, false) and switch between 2.18.2. and 2.18.3.

Comment From: cowtowncoder

I think we'd need a proper test instead of printing out with "see what output is".

I am puzzled as to why #4922 is mentioned: fixes involved do not change (I think) logic or handling of MapperFeature.ALLOW_FINAL_FIELDS_AS_MUTATORS. Although since observed changed in behavior is not directly explained, it is bit hard to argue.

So: how did behavior change?

Comment From: nlisker

I think we'd need a proper test instead of printing out with "see what output is".

The code above either throws an exception or doesn't. I can wrap it in a unit test method instead of a main method, but it wouldn't show anything differently.

So: how did behavior change?

Does this not explain the change clearly?

In 2.18.2 the code above fails regardless of if ALLOW_FINAL_FIELDS_AS_MUTATORS is configured (and @JsonMerge doesn't help).

In 2.18.3 and above it fails with ALLOW_FINAL_FIELDS_AS_MUTATORS commented out (=true) and passes with it set to false (without @JsonMerge). With @JsonMerge it always passes.

This is probably because of https://github.com/FasterXML/jackson-databind/issues/4922. However, the behavioral change is seen without @JsonMerge: include configure(MapperFeature.ALLOW_FINAL_FIELDS_AS_MUTATORS, false) and switch between 2.18.2. and 2.18.3.

Configure the builder with .configure(MapperFeature.ALLOW_FINAL_FIELDS_AS_MUTATORS, false). Run the code/test with 2.18.2 where you will get an exception, and run it with 2.18.3 (or 2.20.0) where deserialization will work correctly.

Comment From: cowtowncoder

Does this not explain the change clearly?

Not without running piece of code -- it does not explain what happens, I often browse issues from phone etc without access to execute tests. If I have to, I can spend time copy-pasting code etc but it is useful to include exception message / partial stack tracee.

Comment From: nlisker

I see. I updated the initial example with what a failure and a pass look like.

Comment From: cowtowncoder

Thank you, @nlisker !

Comment From: cowtowncoder

Reading through it, I am still not 100% sure I understand the specific problem is, aside from 2.18.2 / 2.18.3 behavior differing -- which may or may not be a problem: if 2.18.3+ behavior is more correct, it is a fix; if less correct, regression/bug. Latter would be problematic, former not. But the part that is confusing is that 2.18.3 sounds like it works with setting MapperFeature.ALLOW_FINAL_FIELDS_AS_MUTATORS of false -- which seems more correct, actually.

But this is because of another setting that matters here (I think): MapperFeature.USE_GETTERS_AS_SETTERS. It allows "setting" of "map" even without accessible Field, Setter or Constructor Parameter. State of ALLOW_FINAL_FIELDS_AS_MUTATORS does matter, but its behavior shouldn't have changed (and, I think, didn't).

Given this, it would make sense to me that 2.18.3+ behavior is the way it is, combining everything I know.

Does above make sense @nlisker? Thank you for walking through this with me.

Comment From: nlisker

MapperFeature.USE_GETTERS_AS_SETTERS is true by default, so there's only a difference if you turn it off explicitly. If ALLOW_FINAL_FIELDS_AS_MUTATORS is false and USE_GETTERS_AS_SETTERS is false then the run fails with a different exception in 2.18.3:

Exception in thread "main" com.fasterxml.jackson.databind.exc.UnrecognizedPropertyException: Unrecognized field "map" (class example.MergeMap), not marked as ignorable (0 known properties: ])
 at [Source: (String)"{"map":{"3":"ADS"}}"; line: 1, column: 9] (through reference chain: example.MergeMap["map"])
    at com.fasterxml.jackson.databind.exc.UnrecognizedPropertyException.from(UnrecognizedPropertyException.java:61)
    at com.fasterxml.jackson.databind.DeserializationContext.handleUnknownProperty(DeserializationContext.java:1153)
    at com.fasterxml.jackson.databind.deser.std.StdDeserializer.handleUnknownProperty(StdDeserializer.java:2245)
    at com.fasterxml.jackson.databind.deser.BeanDeserializerBase.handleUnknownProperty(BeanDeserializerBase.java:1821)
    at com.fasterxml.jackson.databind.deser.BeanDeserializerBase.handleUnknownVanilla(BeanDeserializerBase.java:1799)
    at com.fasterxml.jackson.databind.deser.BeanDeserializer.vanillaDeserialize(BeanDeserializer.java:316)
    at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserialize(BeanDeserializer.java:177)
    at com.fasterxml.jackson.databind.deser.DefaultDeserializationContext.readRootValue(DefaultDeserializationContext.java:342)
    at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4931)
    at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3868)
    at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3836)
    at example.MergeMap.main(MergeMap.java:68)

From my understanding, there are different deserialization paths that Jackson is trying to take and it depends on the order in which it tries them. For 2.18.3: * If both are true (default), you get the "Cannot construct instance" exception. I wouldn't think that this is correct because "all paths are enabled". But... * If ALLOW_FINAL_FIELDS_AS_MUTATORS is turned off, then it works, probably via the USE_GETTERS_AS_SETTERS path. This tells me that ALLOW_FINAL_FIELDS_AS_MUTATORS takes precedence and fails the attempt without falling back on other paths. * If USE_GETTERS_AS_SETTERS is turned off, it's the same results as in the first case because as long as ALLOW_FINAL_FIELDS_AS_MUTATORS is on, it will try that. * If both are false, the "Unrecognized field map" exception is thrown because there's another path attempted.

Using @JsonMerge makes any configuration work, either because it's another path that takes precedence over ALLOW_FINAL_FIELDS_AS_MUTATORS, or because it "fixes" it.

For 2.18.2: * For the first 3 cases above, you get a "Cannot find a deserializer for non-concrete Map type" exception with or without @JsonMerge. * For the case where both are false you still get "Unrecognized field map", but here @JsonMerge changes the exception to "Cannot find a deserializer" as above.

What is the correct behavior will depend on what precedence is correct.

Comment From: cowtowncoder

TL;DNR;: as far as I can see, 2.18.3 works as I would expect.

There are no explicit paths based on configuration: effects are indirect. But both ALLOW_FINAL_FIELDS_AS_MUTATORS and USE_GETTERS_AS_SETTERS affect Property Discovery.

Logical properties are discovered by considering Class definitions: Java Fields, Methods and Constructor Parameters are considered as potential accessors (Mutator for setting/deserialization; Accessor for getting/serialization), based on filtering ("no static Fields"; "getter Method takes no arguments") and visibility (rule-based).

In case of ALLOW_FINAL_FIELDS_AS_MUTATORS Fields that are final are either accepted (if true), or not (false).

Further, if no mutator exists for a property but USE_GETTERS_AS_SETTERS is true AND there exists matching Getter method, it can be used instead for deserialization.

So in this case working case is one where either:

1. ALLOW_FINAL_FIELDS_AS_MUTATORS is true -> Field map is used for deserialization, OR
2. USE_GETTERS_AS_SETTERS is true -> value returned by getMap() is modified
3. If neither true, @JsonMerge works similar to (2), if used (after fixing of #4922 )

but if none succeeds, there is failure as there is nothing to indicate existence of "map" property for deserialization purposes.

As to 2.18.2 and earlier working differently: there are 11 fixes in 2.18.3 so one of them (or combination) changed handling in the way that I think is more correct.

But it probably did not change handling of ALLOW_FINAL_FIELDS_AS_MUTATORS specifically.

Comment From: nlisker

Alright, since this is the correct behavior I'll close it as not a bug.