From facb3bce58e9ef3128dc6cac74e3e97f34e523d2 Mon Sep 17 00:00:00 2001 From: Giulio Longfils Date: Sun, 13 Jul 2025 22:13:17 +0200 Subject: [PATCH 1/4] fix(#4218): avoid duplicate injection on fields when a corresponding injected constructor parameter is available --- .../deser/BeanDeserializerFactory.java | 3 ++ .../databind/introspect/AnnotatedMember.java | 21 +++++++++++ .../introspect/POJOPropertiesCollector.java | 37 +++++++++++++++++++ .../databind/records/RecordBasicsTest.java | 10 +---- .../inject}/JacksonInject2678Test.java | 4 +- .../inject}/JacksonInject4218Test.java | 4 +- 6 files changed, 64 insertions(+), 15 deletions(-) rename src/test/java/com/fasterxml/jackson/databind/{tofix => deser/inject}/JacksonInject2678Test.java (93%) rename src/test/java/com/fasterxml/jackson/databind/{tofix => deser/inject}/JacksonInject4218Test.java (91%) diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerFactory.java b/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerFactory.java index c90bdef271..21d252331a 100644 --- a/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerFactory.java +++ b/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerFactory.java @@ -816,6 +816,9 @@ protected void addInjectables(DeserializationContext ctxt, for (Map.Entry entry : raw.entrySet()) { AnnotatedMember m = entry.getValue(); + if (m.isIgnoreInjection()) { + continue; + } final JacksonInject.Value injectableValue = introspector.findInjectableValue(m); final Boolean optional = injectableValue == null ? null : injectableValue.getOptional(); diff --git a/src/main/java/com/fasterxml/jackson/databind/introspect/AnnotatedMember.java b/src/main/java/com/fasterxml/jackson/databind/introspect/AnnotatedMember.java index e2da0c8cca..c8f5eae63b 100644 --- a/src/main/java/com/fasterxml/jackson/databind/introspect/AnnotatedMember.java +++ b/src/main/java/com/fasterxml/jackson/databind/introspect/AnnotatedMember.java @@ -32,6 +32,13 @@ public abstract class AnnotatedMember // no need to persist protected final transient AnnotationMap _annotations; + /** + * Flag to avoid duplicate injection. See issue #4218 + * + * @since 2.20 + */ + protected boolean _ignoreInjection; + protected AnnotatedMember(TypeResolutionContext ctxt, AnnotationMap annotations) { super(); _typeContext = ctxt; @@ -140,6 +147,20 @@ public final void fixAccess(boolean force) { } } + /** + * @since 2.20 + */ + public void ignoreInjection() { + _ignoreInjection = true; + } + + /** + * @since 2.20 + */ + public boolean isIgnoreInjection() { + return _ignoreInjection; + } + /** * Optional method that can be used to assign value of * this member on given object, if this is a supported diff --git a/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java b/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java index 2e1bc148c0..9222de3700 100644 --- a/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java +++ b/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java @@ -447,6 +447,9 @@ protected void collectAll() _addCreators(props); } + // Mark injected fields that are already injected via constructor properties + _ignoreDuplicateInjection(props); + // Remove ignored properties, first; this MUST precede annotation merging // since logic relies on knowing exactly which accessor has which annotation _removeUnwantedProperties(props); @@ -1290,6 +1293,40 @@ private String _checkRenameByField(String implName) { /********************************************************** */ + /** + * Method to mark injected fields as ignored if there's a corresponding + * creator property already injecting the same value + */ + protected void _ignoreDuplicateInjection(final Map props) + { + for (POJOPropertyBuilder prop : props.values()) { + final AnnotatedField field = prop.getFieldUnchecked(); + if (field == null) { + continue; + } + + final JacksonInject.Value injectableValue = + _annotationIntrospector.findInjectableValue(field); + if (injectableValue == null) { + continue; + } + + for (POJOPropertyBuilder creatorProperty : _creatorProperties) { + if (creatorProperty == null) { + continue; + } + + final AnnotatedParameter parameter = creatorProperty.getConstructorParameter(); + if (parameter != null + && injectableValue.equals( + _annotationIntrospector.findInjectableValue(parameter))) { + field.ignoreInjection(); + break; + } + } + } + } + /** * Method called to get rid of candidate properties that are marked * as ignored. diff --git a/src/test-jdk17/java/com/fasterxml/jackson/databind/records/RecordBasicsTest.java b/src/test-jdk17/java/com/fasterxml/jackson/databind/records/RecordBasicsTest.java index afafaa4734..74aafada29 100644 --- a/src/test-jdk17/java/com/fasterxml/jackson/databind/records/RecordBasicsTest.java +++ b/src/test-jdk17/java/com/fasterxml/jackson/databind/records/RecordBasicsTest.java @@ -188,15 +188,7 @@ public void testDeserializeJsonRename() throws Exception { @Test public void testDeserializeHeaderInjectRecord_WillFail() throws Exception { MAPPER.setInjectableValues(new InjectableValues.Std().addValue(String.class, "Bob")); - - try { - MAPPER.readValue("{\"id\":123}", RecordWithHeaderInject.class); - - fail("should not pass"); - } catch (IllegalArgumentException e) { - verifyException(e, "RecordWithHeaderInject#name"); - verifyException(e, "Can not set final java.lang.String field"); - } + MAPPER.readValue("{\"id\":123}", RecordWithHeaderInject.class); } @Test diff --git a/src/test/java/com/fasterxml/jackson/databind/tofix/JacksonInject2678Test.java b/src/test/java/com/fasterxml/jackson/databind/deser/inject/JacksonInject2678Test.java similarity index 93% rename from src/test/java/com/fasterxml/jackson/databind/tofix/JacksonInject2678Test.java rename to src/test/java/com/fasterxml/jackson/databind/deser/inject/JacksonInject2678Test.java index 20f1d41abc..4d352724b9 100644 --- a/src/test/java/com/fasterxml/jackson/databind/tofix/JacksonInject2678Test.java +++ b/src/test/java/com/fasterxml/jackson/databind/deser/inject/JacksonInject2678Test.java @@ -1,4 +1,4 @@ -package com.fasterxml.jackson.databind.tofix; +package com.fasterxml.jackson.databind.deser.inject; import java.util.Objects; @@ -10,7 +10,6 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.json.JsonMapper; import com.fasterxml.jackson.databind.testutil.DatabindTestUtil; -import com.fasterxml.jackson.databind.testutil.failure.JacksonTestFailureExpected; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -41,7 +40,6 @@ public String getField2() { } // [databind#2678] - @JacksonTestFailureExpected @Test void readValueInjectables() throws Exception { final InjectableValues injectableValues = diff --git a/src/test/java/com/fasterxml/jackson/databind/tofix/JacksonInject4218Test.java b/src/test/java/com/fasterxml/jackson/databind/deser/inject/JacksonInject4218Test.java similarity index 91% rename from src/test/java/com/fasterxml/jackson/databind/tofix/JacksonInject4218Test.java rename to src/test/java/com/fasterxml/jackson/databind/deser/inject/JacksonInject4218Test.java index 473f089598..c406337a9d 100644 --- a/src/test/java/com/fasterxml/jackson/databind/tofix/JacksonInject4218Test.java +++ b/src/test/java/com/fasterxml/jackson/databind/deser/inject/JacksonInject4218Test.java @@ -1,4 +1,4 @@ -package com.fasterxml.jackson.databind.tofix; +package com.fasterxml.jackson.databind.deser.inject; import org.junit.jupiter.api.Test; @@ -8,7 +8,6 @@ import com.fasterxml.jackson.databind.*; import com.fasterxml.jackson.databind.testutil.DatabindTestUtil; -import com.fasterxml.jackson.databind.testutil.failure.JacksonTestFailureExpected; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -51,7 +50,6 @@ public Object findInjectableValue( } // [databind#4218] - @JacksonTestFailureExpected @Test void injectFail4218() throws Exception { From 7a0ac6a575ccb192910049cc8b6bf4e56513e520 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Thu, 21 Aug 2025 20:58:01 -0700 Subject: [PATCH 2/4] Rewrote parts of solution, added release notes --- release-notes/CREDITS-2.x | 6 ++++++ release-notes/VERSION-2.x | 4 ++++ .../deser/BeanDeserializerFactory.java | 3 --- .../databind/introspect/AnnotatedMember.java | 21 ------------------- .../introspect/POJOPropertiesCollector.java | 18 +++++++++++++--- .../deser/inject/JacksonInject4218Test.java | 2 +- 6 files changed, 26 insertions(+), 28 deletions(-) diff --git a/release-notes/CREDITS-2.x b/release-notes/CREDITS-2.x index 85bdf3764a..b13d7b0a98 100644 --- a/release-notes/CREDITS-2.x +++ b/release-notes/CREDITS-2.x @@ -1869,6 +1869,9 @@ wrongwrong (@k163377) * Contributed fix for #5202: #5202: `JsonSetter.contentNulls` ignored for `Object[]`, `String[]` and `Collection` (2.19.2) + * Reported #4218: If `@JacksonInject` is specified for field and deserialized by + the Creator, the inject process will be executed twice + (2.20.0) Bernd Ahlers (@bernd) * Reported #4742: Deserialization with Builder, External type id, `@JsonCreator` failing @@ -1955,3 +1958,6 @@ Giulio Longfils (@giulong) * Contributed #3072: Allow specifying `@JacksonInject` does not fail when there's no corresponding value (2.20.0) + * Contributed #4218: If `@JacksonInject` is specified for field and deserialized by + the Creator, the inject process will be executed twice + (2.20.0) diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x index 80e7c939c6..126b91ddfc 100644 --- a/release-notes/VERSION-2.x +++ b/release-notes/VERSION-2.x @@ -8,6 +8,10 @@ Not yet released #2692: Should never call `set()` on setterless property during deserialization (reported by @lbonco) +#4218: If `@JacksonInject` is specified for field and deserialized by the Creator, + the inject process will be executed twice + (reported by @wrongwrong) + (fix by Giulio L) #5237: Failing `@JsonMerge` with a custom Map with a `@JsonCreator` constructor (reported by @nlisker) #5238: Immutable classes with `@JsonIdentityInfo` can be deserialized; records cannot diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerFactory.java b/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerFactory.java index 1ff4e891c9..bf74f6e317 100644 --- a/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerFactory.java +++ b/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerFactory.java @@ -821,9 +821,6 @@ protected void addInjectables(DeserializationContext ctxt, for (Map.Entry entry : raw.entrySet()) { AnnotatedMember m = entry.getValue(); - if (m.isIgnoreInjection()) { - continue; - } final JacksonInject.Value injectableValue = introspector.findInjectableValue(m); final Boolean optional = injectableValue == null ? null : injectableValue.getOptional(); diff --git a/src/main/java/com/fasterxml/jackson/databind/introspect/AnnotatedMember.java b/src/main/java/com/fasterxml/jackson/databind/introspect/AnnotatedMember.java index c8f5eae63b..e2da0c8cca 100644 --- a/src/main/java/com/fasterxml/jackson/databind/introspect/AnnotatedMember.java +++ b/src/main/java/com/fasterxml/jackson/databind/introspect/AnnotatedMember.java @@ -32,13 +32,6 @@ public abstract class AnnotatedMember // no need to persist protected final transient AnnotationMap _annotations; - /** - * Flag to avoid duplicate injection. See issue #4218 - * - * @since 2.20 - */ - protected boolean _ignoreInjection; - protected AnnotatedMember(TypeResolutionContext ctxt, AnnotationMap annotations) { super(); _typeContext = ctxt; @@ -147,20 +140,6 @@ public final void fixAccess(boolean force) { } } - /** - * @since 2.20 - */ - public void ignoreInjection() { - _ignoreInjection = true; - } - - /** - * @since 2.20 - */ - public boolean isIgnoreInjection() { - return _ignoreInjection; - } - /** * Optional method that can be used to assign value of * this member on given object, if this is a supported diff --git a/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java b/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java index 6e85c4f542..c5965d3a84 100644 --- a/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java +++ b/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java @@ -450,8 +450,6 @@ protected void collectAll() if (_config.isEnabled(MapperFeature.FIX_FIELD_NAME_UPPER_CASE_PREFIX)) { _fixLeadingFieldNameCase(props); } - // Mark injected fields that are already injected via constructor properties - _ignoreDuplicateInjection(props); // Remove ignored properties, first; this MUST precede annotation merging // since logic relies on knowing exactly which accessor has which annotation _removeUnwantedProperties(props); @@ -1284,6 +1282,20 @@ protected void _addInjectables(Map props) } _doAddInjectable(_annotationIntrospector.findInjectableValue(m), m); } + + // 21-Aug-2025, tatu: [databind#4218] avoid duplicate injectables + if (_injectables != null) { + for (POJOPropertyBuilder creatorProperty : _creatorProperties) { + if (creatorProperty == null) { + continue; + } + final AnnotatedParameter parameter = creatorProperty.getConstructorParameter(); + JacksonInject.Value injectable = _annotationIntrospector.findInjectableValue(parameter); + if (injectable != null) { + _injectables.remove(injectable.getId()); + } + } + } } protected void _doAddInjectable(JacksonInject.Value injectable, AnnotatedMember m) @@ -1446,7 +1458,7 @@ protected void _ignoreDuplicateInjection(final Map if (parameter != null && injectableValue.equals( _annotationIntrospector.findInjectableValue(parameter))) { - field.ignoreInjection(); +// field.ignoreInjection(); break; } } diff --git a/src/test/java/com/fasterxml/jackson/databind/deser/inject/JacksonInject4218Test.java b/src/test/java/com/fasterxml/jackson/databind/deser/inject/JacksonInject4218Test.java index c406337a9d..b48b7c3d34 100644 --- a/src/test/java/com/fasterxml/jackson/databind/deser/inject/JacksonInject4218Test.java +++ b/src/test/java/com/fasterxml/jackson/databind/deser/inject/JacksonInject4218Test.java @@ -51,7 +51,7 @@ public Object findInjectableValue( // [databind#4218] @Test - void injectFail4218() throws Exception + void injectNoDups4218() throws Exception { ObjectReader reader = newJsonMapper() .readerFor(Dto.class) From 83a1e56aa30bdd895bc7b0205ab562344bfd76c4 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Thu, 21 Aug 2025 20:59:40 -0700 Subject: [PATCH 3/4] Dead code elimination --- .../introspect/POJOPropertiesCollector.java | 34 ------------------- 1 file changed, 34 deletions(-) diff --git a/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java b/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java index c5965d3a84..538ed57527 100644 --- a/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java +++ b/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java @@ -1431,40 +1431,6 @@ private boolean _firstOrSecondCharUpperCase(String name) { /********************************************************** */ - /** - * Method to mark injected fields as ignored if there's a corresponding - * creator property already injecting the same value - */ - protected void _ignoreDuplicateInjection(final Map props) - { - for (POJOPropertyBuilder prop : props.values()) { - final AnnotatedField field = prop.getFieldUnchecked(); - if (field == null) { - continue; - } - - final JacksonInject.Value injectableValue = - _annotationIntrospector.findInjectableValue(field); - if (injectableValue == null) { - continue; - } - - for (POJOPropertyBuilder creatorProperty : _creatorProperties) { - if (creatorProperty == null) { - continue; - } - - final AnnotatedParameter parameter = creatorProperty.getConstructorParameter(); - if (parameter != null - && injectableValue.equals( - _annotationIntrospector.findInjectableValue(parameter))) { -// field.ignoreInjection(); - break; - } - } - } - } - /** * Method called to get rid of candidate properties that are marked * as ignored. From 01dda7c902f3f5dab9cd295730848deba16c636b Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Thu, 21 Aug 2025 21:03:01 -0700 Subject: [PATCH 4/4] Update release notes --- release-notes/CREDITS-2.x | 8 ++++++++ release-notes/VERSION-2.x | 4 ++++ 2 files changed, 12 insertions(+) diff --git a/release-notes/CREDITS-2.x b/release-notes/CREDITS-2.x index b13d7b0a98..d626a3e176 100644 --- a/release-notes/CREDITS-2.x +++ b/release-notes/CREDITS-2.x @@ -1955,9 +1955,17 @@ Eddú Meléndez Gonzales (@eddumelendez) (2.19.2) Giulio Longfils (@giulong) + * Contributed #2678: `@JacksonInject` added to property overrides value from the JSON + even if `useInput` is `OptBoolean.TRUE` + (2.20.0) * Contributed #3072: Allow specifying `@JacksonInject` does not fail when there's no corresponding value (2.20.0) * Contributed #4218: If `@JacksonInject` is specified for field and deserialized by the Creator, the inject process will be executed twice (2.20.0) + +Plamen Tanov (@ptanov) + * Reported #2678: `@JacksonInject` added to property overrides value from the JSON + even if `useInput` is `OptBoolean.TRUE` + (2.20.0) diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x index 126b91ddfc..554d5fb14a 100644 --- a/release-notes/VERSION-2.x +++ b/release-notes/VERSION-2.x @@ -6,6 +6,10 @@ Project: jackson-databind Not yet released +#2678: `@JacksonInject` added to property overrides value from the JSON + even if `useInput` is `OptBoolean.TRUE` + (reported by Plamen T) + (fix by Giulio L) #2692: Should never call `set()` on setterless property during deserialization (reported by @lbonco) #4218: If `@JacksonInject` is specified for field and deserialized by the Creator,