If `JacksonInject` is specified for field and deserialized by the Creator, the inject process will be executed twice.

[k163377]

Search before asking

• I searched in the issues and found nothing similar.

Describe the bug

SSIA

Version Information

2.16.0

Reproduction

The following is a sample using InjectableValues that is made to count up when findInjectableValue is called.
Since it is deserialized only once, the first ID is expected, but it is actually the second ID.

import com.fasterxml.jackson.annotation.JacksonInject;
import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.*;
import org.junit.jupiter.api.Test;

import java.util.Arrays;
import java.util.List;
import java.util.UUID;

import static org.junit.jupiter.api.Assertions.assertEquals;

public class JacksonInjectForFieldTest {
    static class Dto {
        @JacksonInject("uuid")
        private final UUID uuid;

        @JsonCreator
        Dto(
                // @JacksonInject("uuid")
                @JsonProperty("uuid")
                UUID uuid
        ) {
            this.uuid = uuid;
        }
    }

    List<UUID> ids = Arrays.asList(UUID.randomUUID(), UUID.randomUUID());
    class MyInjectableValues extends InjectableValues.Std {
        int count = 0; // count up if injected

        @Override
        public Object findInjectableValue(
                Object valueId,
                DeserializationContext ctxt,
                BeanProperty forProperty,
                Object beanInstance
        ) throws JsonMappingException {
            if (valueId.equals("uuid")) {
                return ids.get(count++);
            } else {
                return super.findInjectableValue(valueId, ctxt, forProperty, beanInstance);
            }
        }
    }

    @Test
    void test() throws JsonProcessingException {
        ObjectReader reader = new ObjectMapper()
                .readerFor(Dto.class)
                .with(new MyInjectableValues());

        Dto dto = reader.readValue("{}");
        UUID actual = dto.uuid;

        System.out.println(ids);
        System.out.println(ids.get(0) == actual);
        System.out.println(ids.get(1) == actual);

        assertEquals(ids.get(0), actual);
    }
}

Expected behavior

The inject process is executed only once.
In fact, if a specification is made for a parameter, the result is as expected.

Additional context

After processing in the creator, another injection is performed on the field, so it is called twice in total.

I commented but forgot to mention the above initially.
#4218 (comment)

[mukham12]

Investigated this, and it appears the hiccup occurs when we apply @JacksonInject to both the constructor parameter and the class field. Things work smoothly if we annotate either-or. This issue is quite similar to #2678.

@k163377,
Any particular reason for double-annotating 🤔? Curious to know more and thanks for flagging this. Let's figure it out together 🚀.

@cowtowncoder,
Identified the issue in the snippet: at line 526, the bean returns with an initialized class and an injected value. We subsequently inject again at line 533, replacing the old injected value.

jackson-databind/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializer.java

Lines 524 to 534 in be56569

	Object bean;
	try {
	bean = creator.build(ctxt, buffer);
	} catch (Exception e) {
	wrapInstantiationProblem(e, ctxt);
	bean = null; // never gets here
	}
	// 13-Apr-2020, tatu: [databind#2678] need to handle injection here
	if (_injectables != null) {
	injectValues(ctxt, bean);
	}

[k163377]

Sorry, I forgot to mention an important fact.
After processing in the creator, another injection is performed on the field, so it is called twice in total.
Perhaps that is the code you are analyzing.

Any particular reason for double-annotating 🤔? Curious to know more and thanks for flagging this. Let's figure it out together 🚀.

Is this a question about the separate annotations for creator and field in the sample code?
The reason is that this code is a Java reproduction of a behavior discovered during the development of jackson-module-kotlin, which was originally Kotlin code.

In the special case of Kotlin (constructors with value class as an argument), annotations added to constructor parameters cannot be parsed, so I added them explicitly to the field and found this behavior.
https://github.com/ProjectMapK/jackson-module-kogera/blob/develop/src/test/kotlin/io/github/projectmapk/jackson/module/kogera/zIntegration/deser/valueClass/JacksonInjectTest.kt

[mukham12]

Yes, that is what I was wondering about. You have a commented @JacksonInject annotation in the constructor.

But if that is the case then the test ran successfully for me in Java.

[cowtowncoder]

This does sound like a bug, and I agree: injection should only happen once per property.

[cowtowncoder]

Added failing test. Quick note on double processing: commenting out injection does not work as that fails an existing test for #471 (and as per comment next to injection, also #2678 but that is still failing for other reasons).

So fix isn't quite as easy.

[cowtowncoder]

Ok, did some more digging. It does look like POJOPropertiesCollector handles introspection and merging of injectables; and does collect _creatorProperties too. But no attempt seems to be made to try to avoid duplication between creator properties (that is, associating Injectable Id with Creator parameter) and regular property accessors (Fields, Setters).
I think conceptually it should be possible to just iterate over creator parameters and see if they have inject values and remove matching id from Field/Setter set... but that does not seem as straight-forward as it should.

Part of the problem is that instead of associating Injectables with POJOPropertyBuilder (instance of which represents information collected for one logical property), they are collected separately by POJOPropertiesCollector (which keeps track of set of all POJO Properties, that is, POJOPropertyBuilders.