Incompatible FAIL_ON_MISSING_PRIMITIVE_PROPERTIES and field level @JsonProperty

[GeorgiPetkov]

Describe the bug
The FAIL_ON_MISSING_PRIMITIVE_PROPERTIES is not working properly with field level @JsonProperty annotation. The @JsonProperty annotation is simply ignored.

Version information
Currently latest 2.12, Java 15.0.1

To Reproduce
If you have a way to reproduce this with:

1. You can use the JUnit 5 test below to reproduce the issue:

class JsonPropertyAndFailOnNullForPrimitivesTest {

    @Test
    void test() throws JsonProcessingException {
        assertEquals(
                JsonMapper.builder()
                        .addModules(new ParameterNamesModule())
                        .constructorDetector(ConstructorDetector.USE_PROPERTIES_BASED)
                        .enable(DeserializationFeature.FAIL_ON_NULL_FOR_PRIMITIVES)
                        .build()
                        .readValue("{\"aa\": 8}", TestClass.class)
                        .a,
                8);
    }

    private static class TestClass {

        @JsonProperty("aa")
        private final int a;

        TestClass(int a) {
            this.a = a;
        }
    }
}

2. The test fails with the following error:

com.fasterxml.jackson.databind.exc.MismatchedInputException: Cannot map `null` into type int (set DeserializationConfig.DeserializationFeature.FAIL_ON_NULL_FOR_PRIMITIVES to 'false' to allow)
 at [Source: (String)"{"aa": 8}"; line: 1, column: 9] (through reference chain: jackson.test.JsonPropertyAndFailOnNullForPrimitivesTest$TestClass["a"])

	at com.fasterxml.jackson.databind.exc.MismatchedInputException.from(MismatchedInputException.java:63)
	at com.fasterxml.jackson.databind.DeserializationContext.reportInputMismatch(DeserializationContext.java:1577)
	at com.fasterxml.jackson.databind.deser.std.NumberDeserializers$PrimitiveOrWrapperDeserializer.getNullValue(NumberDeserializers.java:175)
	at com.fasterxml.jackson.databind.deser.impl.PropertyValueBuffer._findMissing(PropertyValueBuffer.java:204)
	at com.fasterxml.jackson.databind.deser.impl.PropertyValueBuffer.getParameters(PropertyValueBuffer.java:160)
	at com.fasterxml.jackson.databind.deser.ValueInstantiator.createFromObjectWith(ValueInstantiator.java:288)
	at com.fasterxml.jackson.databind.deser.impl.PropertyBasedCreator.build(PropertyBasedCreator.java:202)
	at com.fasterxml.jackson.databind.deser.BeanDeserializer._deserializeUsingPropertyBased(BeanDeserializer.java:520)
	at com.fasterxml.jackson.databind.deser.BeanDeserializerBase.deserializeFromObjectUsingNonDefault(BeanDeserializerBase.java:1390)
	at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserializeFromObject(BeanDeserializer.java:362)
	at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserialize(BeanDeserializer.java:195)
	at com.fasterxml.jackson.databind.deser.DefaultDeserializationContext.readRootValue(DefaultDeserializationContext.java:322)
	at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4591)
	at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3546)
	at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3514)
	at jackson.test.JsonPropertyAndFailOnNullForPrimitivesTest.test(JsonPropertyAndFailOnNullForPrimitivesTest.java:23)

3. Note that other combinations work - not using a primitive type, moving the annotation on the constructor parameter or disabling the FAIL_ON_NULL_FOR_PRIMITIVES feature.
   The issue seems to be unrelated to the fact that I use a single argument constructor, I used single property for simlicity in the test. In other words, the new .constructorDetector(ConstructorDetector.USE_PROPERTIES_BASED) appears to be irrelevant for this bug.

Expected behavior
The FAIL_ON_MISSING_PRIMITIVE_PROPERTIES feature should be aware of property names mapping through @JsonProperty when used on fields.

[cowtowncoder]

Thank you for reporting this issue: I will try to figure this out before 2.12.1 patch release.

[cowtowncoder]

Hmmmh. So, the problem here is that the field and constructor parameter are not linked, so that intended/implied relationship (to effectively rename constructor parameter as "aa") does not happen and it is considered to just be a. Because of this, value is missing (there is no property a) and the exception is thrown.

The reason for missing linkage gets bit complicated but it is a limitation of the way Creator methods are discovered too late to be linked with other accessors UNLESS constructor parameters are explicitly annotated.
I hope to tackle this issue in future (hopefully for 2.13), but it requires a big rewrite of introspection and cannot be done as 2.12.x patch.

Work-around that should work here is to annotate constructor parameter with @JsonProperty (that is, move from field to constructor parameter).

I plan to add a reproduction under failing, but need to change it slightly not to rely on parameter names module (as databind tests cannot depend on it).

[GeorgiPetkov]

... Creator methods are discovered too late to be linked with other accessors ...

This seems strange since it's working if not using a primitive type for the field. Perhaps it's something much simpler and easier to fix?

[cowtowncoder]

@GeorgiPetkov that makes sense since FAIL_ON_NULL_FOR_PRIMITIVES only affects primitives doesn't it?

But while this prevents exception, it will not change the fact that field is used, not constructor. Interestingly enough final keyword does not actually prevent reflection from modifying value of a field under certain conditions (I don't know the specific constraints but seems to be "immediately after instantiation", whatever that means).

[GeorgiPetkov]

Reopening the issue, I closed it by accident.

@cowtowncoder If I understand correctly FAIL_ON_NULL_FOR_PRIMITIVESs validation should be applied at later stage then? Anyway, I guess the issue is not urgent so if its easier to be done after some major redesign I'm OK with that.

@cowtowncoder AFAIK the final keyword on fields never prevents anything from happening at runtime (unlike private). It's used only for compile-time checks.

Meanwhile for other people that may encounter this and if you're using Lombok already you can simply use the following property to avoid the necessity of any changes in your code before/after the fix:

lombok.copyableannotations += com.fasterxml.jackson.annotation.JsonProperty

[cowtowncoder]

@GeorgiPetkov No the problem is that:

1. JSON has property "aa"
2. Constructor expects property "a"
3. Due to essentially missing "a", null must be passed as placeholder (constructor must get some value for the argument
4. Nulls are not allowed for primitive values like int since you have FAIL_ON_NULL_PRIMITIVES

In addition there is field a renamed as aa into which incoming value is actually mapped (as to final, it's just a detour for discussion but it is possible to prevent use of final fields as setters).
So null comes from absence of value in this case.

Now: normally there should be association between field and constructor parameter, as their implicit names match: and after this linkage exist, they are part of a single logical property and that property would get renamed as aa. Further, constructor parameter having higher precedence would make it be used and field essentially ignored for deserialization.

This linkage, in turn, is missing because fields, methods, and name-annotated constructor parameters are introspected by POJOPropertiesCollector (originally all ctor parameters had to be annotated, before Java 5) -- but ones which implicit names are only handled at a later point.
The end result is that while both accessors are found, annotation "sharing" across accessors does not work -- annotation merging occurs in POJOPropertiesCollector

This does give me an idea, however, regarding possibly (fingers crossed) finding linkage via implicit name, in addition to explicit name (explicit meaning "defined by annotation" and implicit "name derived from field/method name in bytecode").
While it would not enable annotation merging per se, it could cover this case where the only effect of annotation is renaming.

[GeorgiPetkov]

@cowtowncoder Seems like I'm doing a really great job as a rubber duck 🤞

[cowtowncoder]

... ok some minor further developments. Reproducing the issue, I can verify 2 main work-arounds (one that Lombok would use I think):

1. Add @JsonProperty on constructor parameter
2. Add @JsonCreator (with or without mode = , although if you add anything might as well add it all)

I also noticed that one part of validation for CreatorDetector part was more lenient than main path so added another check, mostly to give bit more meaningful error message.
Message gotten with reproduction is not optimal either, as "can not map null" does not indicate target property (which would be helpful); omission is due to problem that the place where issue is detected is one where name is probably not available. But I'll have a look just in case.

[cowtowncoder]

Ok. So, looks like POJOPropertiesCollector specifically misses cases where constructor has no @JsonCreator annotation at all AND no explicitly annotated name, as per symptoms. To fix that, more/most of detection from BasicDeserializerFactory needs to be moved there.

As to "can not map null": problem is indeed that failure is detected in JsonDeserializer.getNullValue() method which has no usable context. To improve detection, caller should probably do more work in figuring out the issue. At the same time, in most similar cases there is code handling property setting (catch issue, add more context) but problem in this particular case is with buffering required for constructor.
Would be good to improve, but will need to file a separate issue if that is desired; will leave this open for the main problem.