Jackson Behavior Change involving AUTO_DETECT_GETTERS from 2.7.4 to 2.7.5

[athirupathisc]

The following code

When run against 2.7.4 it prints the correct de-serialized values
{alice=Saved true version 1}

When run against 2.7.5 it prints this
{alice=Saved null version null}

if AUTO_DETECT_GETTERS is not used, all version works fine.

I tried all recent versions (2.8.* ,2.9) and all of them exhibit this issue.

import com.fasterxml.jackson.core.type.TypeReference;
import com.fasterxml.jackson.databind.DeserializationFeature;
import com.fasterxml.jackson.databind.MapperFeature;
import com.fasterxml.jackson.databind.ObjectMapper;

import java.util.Map;

public class TestSerialization {

    private static final TypeReference<Map<String, TestSerialization.SavedState>> SAVED_STATE_TYPE =
            new TypeReference<Map<String, TestSerialization.SavedState>>() {};

    public static ObjectMapper getObjectMapper() {
        ObjectMapper mapper = new ObjectMapper();
        mapper.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false);
        mapper.disable(MapperFeature.AUTO_DETECT_GETTERS);
        return mapper;
    }

    public static void main(String[] args) {
        try {
            Map<String, TestSerialization.SavedState> map =
                    getObjectMapper().readValue("{\"alice\":{\"saved\":true,\"version\":1}}", SAVED_STATE_TYPE);

            System.out.println(map);
        } catch(Exception e) {
            e.printStackTrace();
        }

    }


    public static class SavedState {
        private final Boolean saved;
        private final Integer version;

        public SavedState() {
            saved = null;
            version = null;
        }

        public SavedState(Boolean saved, Integer version) {
            this.saved = saved;
            this.version = version;
        }

        public Boolean getSaved() {
            return saved;
        }

        public Integer getVersion() {
            return version;
        }

        public String toString() {
            return "Saved " + saved + " version " + version;
        }
    }
}

On looking through the issues , this could be a regression from the following code change

#1223

I am not familiar with AUTO_DETECT_GETTERS, so Please do not hesitate to point me to a document/tutorial
where I can learn more about that. If this behavior change by design,
can you please let me know how can I identify all such instances in my code base and how should I fix them.
( we have 500+ objects that are deserialized and persisted in database, so backward compat is important).

Our Application runs on Google AppEngine so I believe getters are disabled due to sandboxing restriction.

Please let me know if you need further information. Also let me know if you would prefer a
gradle project(zip file) to make the repro easier.

[cowtowncoder]

I can not say off-hand what is happening here, as it seems like code should work and set properties.
But here are some initial suggestions...

First, when debugging, it makes sense to remove/disable:

mapper.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false);

since it typically masks all kinds of problems. It may make sense in production as defensive setting, but I have noticed that in many cases it hinders troubleshooting.

As to AUTO_DETECT_GETTERS (and other auto-detect settings): when auto-detection is enabled, methods with certain names -- like X getValue() -- may be recognized as accessors for properties, without annotations, as long as their visibility is high enough (matching or exceeding minimum value for accessor type). This is sort of legacy setting, as @JsonAutoDetect annotation has more granular control.
When this setting is disabled, auto-discovery is NOT done for getters at all, regardless of visibility, unless enabled for type using annotations.

Now: in theory it does not affect auto-discovery of setters or fields (to use for setting value).
So private fields should still accessible for setting values.

I'll have a look to see if I can find more about this.

[cowtowncoder]

Looks like fields are not discovered indeed; changing them to public (or annotating) does bring them in. That seems wrong.

[athirupathisc]

Thanks @cowtowncoder for taking a look

I tried removing the FAIL_ON_UNKNOWN_PROPERTIES . 2.7.4 and before it works. 2.7.5 and later it fails with the exception.

com.fasterxml.jackson.databind.exc.UnrecognizedPropertyException: Unrecognized field "saved" (class TestSerialization$SavedState), not marked as ignorable (0 known properties: ])
 at [Source: {"alice":{"saved":true,"version":1}}; line: 1, column: 23] (through reference chain: java.util.LinkedHashMap["alice"]->SavedState["saved"])
	at com.fasterxml.jackson.databind.exc.UnrecognizedPropertyException.from(UnrecognizedPropertyException.java:62)
	at com.fasterxml.jackson.databind.DeserializationContext.reportUnknownProperty(DeserializationContext.java:851)
	at com.fasterxml.jackson.databind.deser.std.StdDeserializer.handleUnknownProperty(StdDeserializer.java:1085)
	at com.fasterxml.jackson.databind.deser.BeanDeserializerBase.handleUnknownProperty(BeanDeserializerBase.java:1388)
	at com.fasterxml.jackson.databind.deser.BeanDeserializerBase.handleUnknownVanilla(BeanDeserializerBase.java:1366)
	at com.fasterxml.jackson.databind.deser.BeanDeserializer.vanillaDeserialize(BeanDeserializer.java:266)
	at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserialize(BeanDeserializer.java:125)
	at com.fasterxml.jackson.databind.deser.std.MapDeserializer._readAndBindStringMap(MapDeserializer.java:496)
	at com.fasterxml.jackson.databind.deser.std.MapDeserializer.deserialize(MapDeserializer.java:342)
	at com.fasterxml.jackson.databind.deser.std.MapDeserializer.deserialize(MapDeserializer.java:26)
	at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:3807)
	at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:2816)
	at TestSerialization.main(TestSerialization.java:22)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:606)
	at com.intellij.rt.execution.application.AppMain.main(AppMain.java:147)

I started with the failure on production and started removing the unnecessary part, until I got this smaller reproducible steps.

[cowtowncoder]

Yes. I can now reproduce the problem, and I think it is valid bug.
Hope it's easy to fix, but very likely that fix will be 2.9(.1) only due to minimize any regression risk for 2.8.x at this point (2.8.10, next patch, likely the last from that branch).

[athirupathisc]

Thanks @cowtowncoder . For us it would be better if we can get it on top of 2.8.X . If I understand correctly 2.9.X is a new version under development with lot more features. But I understand your concern too.

If the fix has narrower scope, then we can try it on 2.8.X . Also I would be happy to validate the fix against our code base once some form of fix is available.

[cowtowncoder]

Actually I think I'll have to change my assessment here. Since the default visibility for fields is not "any fields" but rather "only public fields", what happens here is according to logic -- private fields are not visible, and could only be surfaced if there are visible accessors (getters). When those do not exist, fields are not included.

It would be better if there was a way to indicate different visibility level for serialization/deserialization since typically one does NOT want to expose non-public field serialization (as getters), but MAY want them for deserialization (as setters). But as things are there is no way to make such distinction.
So I don't think I can do anything here for Jackson 2.x -- change is in fact fixing a flaw in handling.

As to handling of your use case I think the main options are:

1. Change visibility of fields to use Visibility.ANY, so that fields are included (but note they may also be used "as getters")
2. Annotate fields with @JsonProperty
3. Annotate getters with @JsonProperty; if names match, fields will also be "pulled in"
4. Use custom AnnotationIntrospector to fake finding @JsonProperty for fields in cases you think they ought to be pulled in

[athirupathisc]

@cowtowncoder let me summarize my understanding to see if I got the issue correctly.

Jackson does auto discovery of fields to see which ones to be set on de-serialization. By default it is public fields, but if there are getters, those fields will be included too. But by disabling the auto getters, now it falls back only to public fields. Is my understanding correct ?

Also this worked until 2.7.4 ( because AUTO_DETECT_GETTERS was ignored). In 2.7.5 this got fixed which broke the behavior.

If my understanding is correct, I am inclined to NOT DISABLE the feature AUTO_DETECT_GETTERS to make it working again. As that never worked and the code that set it did not validate it anyway.

Apologies for asking too many questions, our code base is large and would love to go with least risky option. Most of the cases you mentioned recommended annotating the classes, but I am worried about classes without unit tests, which will be tricky.

[athirupathisc]

Apologies for the reping @cowtowncoder . Can you please confirm whether my understanding is correct.

[cowtowncoder]

@athirupathisc Yes, your understanding is correct. One additional relevant aspect is

 MapperFeature.INFER_PROPERTY_MUTATORS

added in 2.2, enabled by default, which controls this behavior. I don't think it matters for your use case, but disabling of this feature would prevent this "pulling in" of setter/field-for-setting.

And yes, I think you either want to keep AUTO_DETECT_GETTERS enabled, or increase visibility of fields.

[athirupathisc]

@cowtowncoder I tried enabling the AUTO_DETECT_GETTERS and here is what I observed.

Previously the behavior was when AUTO_DETECT_GETTERS was disabled, serialization did not call the getters method for serialization. But de-serialization still inferred those properties were there so was able to deserialize them correctly.

Here are my other observations as well, just for the confirmation.

1. AUTO_DETECT_GETTERS started using the getProperty() methods, though field property was explcitly annotated with JsonProperty annotation. This was mostly fine, but some classes was doing Boolean Object to boolean Primitive conversion and when the property was null, it started throwing NullPointerExceptions. I fixed these code by making them return the Object Boolean.

2. Some classes had field annotated with JsonProperty and given different name. But the getMethods was also present for the field. Now it started getting serialized twice and de-serializers when FAIL_ON_UNKNOWN_PROPERTIES was not set it started throwing errors. I fixed these code by adding the following annotations to the class, JsonAutoDetect(getterVisibility = JsonAutoDetect.Visibility.NONE )

Please correct me if my understanding is correct.

[cowtowncoder]

Ok, some notes:

• Auto-detection never has effect on @JsonProperty: that is explicit annotation and has precedence over auto-detection. Auto-detection could be called "implicit properties" (ones based on combination of visibility and naming convention), and annotations then "explicit properties"
• For property accessors (getter, setter, field) to match, names must match -- and this can get tricky considering there is "implicit name" (one inferred from naming convention) and "explicit name" (from annotations). Names are case-sensitive. If names of accessors do not match, they are not part of same logical property.

I am not sure, however, why naming did not match between fields and getters -- in your examples they did match. So this seems odd.

One aspect that has not been handled well, I admit, is the ambiguity for fields as accessor.
Although it would make intuitive sense to consider field visibility to depend on their role (setter vs getter), this is not how it works: rather, there is just one level defined, which says "only auto-detect public fields". This used in conjunction with "pull in otherwise non-visible setter/field if there is visible/annotated getter with same logical property name" works for most cases, but falls apart if getter is not made visible.

I hope this makes more sense.

[cowtowncoder]

At this point I think Jackson's behavior is actually consistent with how things should work -- deserialization fails because there are neither auto-discoverable mutators (fields or setters) nor explicit annotated ones (with @JsonProperty).
Enabling of auto-detection of getters is also expected to "pull in" non-visible fields as setters, so disabling auto-detection will have described effect.

[athirupathisc]

Thanks @cowtowncoder you are right, jackson's behavior was expected. It was just unfortunate that we relied on a buggy behavior. I am making efforts to cleanup our code base. Thanks for following through. I agree that closing is the right choice.

[cowtowncoder]

@athirupathisc Right, it is unfortunate to have bugs that happen to work in a way that is good for a use case. There are also many one-off convenience features that are not exactly well specified so it really is difficult to know which is which.