Search before asking
- [X] 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. https://github.com/FasterXML/jackson-databind/issues/4218#issuecomment-1826461447
Comment From: 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. https://github.com/FasterXML/jackson-databind/blob/be5656901a09b6e31ae6abbd28d5f353c83bd563/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializer.java#L524-L534
Comment From: 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
Comment From: 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.
Comment From: cowtowncoder
This does sound like a bug, and I agree: injection should only happen once per property.
Comment From: 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.
Comment From: 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.
Comment From: hu-chia
This issue also has an impact to java records. If a record has a construct parameter with JacksonInject, the deserialize process will be broken.
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.InjectableValues;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import com.fasterxml.jackson.databind.node.ObjectNode;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
public class InjectableTest {
static final String INJECTED_VALUE = "injected_value";
@Test
void testUninjectable() {
var ivs = new InjectableValues.Std();
var val = "hello,world";
ivs.addValue(INJECTED_VALUE, val);
var jackson = new ObjectMapper().setInjectableValues(ivs);
var json = "{\"x\": 42}";
// For now, this will fail.
jackson.readerFor(UninjectableModel.class).readValue(json)
}
record UninjectableModel(
@com.fasterxml.jackson.annotation.JacksonInject(INJECTED_VALUE) String injectedValue,
int x
) {
}
@Test
void testInjectable() throws JsonProcessingException {
var ivs = new InjectableValues.Std();
var val = "hello,world";
ivs.addValue(INJECTED_VALUE, val);
var jackson = new ObjectMapper().setInjectableValues(ivs);
var json = "{\"x\": 42}";
var model = jackson.readerFor(InjectableModel.class).<InjectableModel>readValue(json);
Assertions.assertEquals(val, model.injectedValue());
}
record InjectableModel(
String injectedValue,
int x
) {
public InjectableModel(
@com.fasterxml.jackson.annotation.JacksonInject(INJECTED_VALUE) String injectedValue,
int x
) {
this.injectedValue = injectedValue;
this.x = x;
}
}
}
Comment From: JooHyukKim
@hu-chia would #5175 fix ur case?
Comment From: hu-chia
@JooHyukKim No.
Comment From: JooHyukKim
@hu-chia hm okay. So passing your test cases is expected behavior? I'm a little bit confused by the use of assertThrows.
Comment From: JooHyukKim
@hu-chia hm okay. So passing your test cases is expected behavior? I'm a little bit confused by the use of assertThrows.
Comment From: hu-chia
@hu-chia hm okay. So passing your test cases is expected behavior? I'm a little bit confused by the use of assertThrows.
@JooHyukKim testUninjectable should failed, I have edited it, thanks for your feedback.
these cases is part of my project which help me follow jackson's update, so when Jackson fixes this issue, I will notice case's failure.