Allow multiple @JacksonInject targets for same id (#5217) by dlwldnjs1009 · Pull Request #5609 · FasterXML/jackson-databind

dlwldnjs1009 · 2026-01-25T05:39:31Z

This PR changes injectable collection to allow multiple injection targets for the same id (#5217).

Behavioral goal (#5217)

Remove the inconsistency where field-field could fail with "Duplicate injectable value" while creator-param injection fails due to missing InjectableValues. After this change, errors are consistent across field/param cases.

Implementation

Internally collect injectables as Map<Object, List<AnnotatedMember>> to support multiple targets per id.
Post-processing keeps only the If @JacksonInject is specified for field and deserialized by the Creator, the inject process will be executed twice #4218 fix: creator parameter masks same-property field/setter injectables.
Defensive copy + freeze: getAllInjectables() returns an unmodifiable copy of the map; value lists are frozen during collectAll().

API compatibility (revised per review feedback)

BeanDescription#findInjectables() signature restored to Map<Object, AnnotatedMember> and deprecated.
New BeanDescription#findAllInjectables() added as a concrete default method returning Map<Object, List<AnnotatedMember>>.
This avoids breaking custom BeanDescription implementations that only override the legacy method.
POJOPropertiesCollector#getInjectables() deprecated; new getAllInjectables() added.
Internal caller (BeanDeserializerFactory) switched to findAllInjectables().

Complexity reduction (revised per review feedback)

Removed _deduplicateSamePropertyInjectables() (setter-masks-field + reportProblem policy) as it was beyond the scope of A Duplicate injectable value ... error occurs only when the same ID JacksonInject is specified for a field multiple times #5217.
Removed _isMethod() helper (only used by the above).
Kept only _removeCreatorPropertyInjectables() for the If @JacksonInject is specified for field and deserialized by the Creator, the inject process will be executed twice #4218 duplicate-injection bug fix.
Note: This PR does not attempt general property-level dedup (field+setter); it focuses on A Duplicate injectable value ... error occurs only when the same ID JacksonInject is specified for a field multiple times #5217 consistency + If @JacksonInject is specified for field and deserialized by the Creator, the inject process will be executed twice #4218.

Tests

Existing field-field / param-param / param-field / creator-masking (If @JacksonInject is specified for field and deserialized by the Creator, the inject process will be executed twice #4218) / record tests retained.
Removed Rule 2 tests (testFieldAndSetterSameProperty, testFieldSetterWithJsonProperty, testTwoInjectableSettersSamePropertyFails) since the policy was removed.
Added 3 new tests for the original A Duplicate injectable value ... error occurs only when the same ID JacksonInject is specified for a field multiple times #5217 reproduction with default IDs (type-based):
- testDefaultIdFieldFieldNoDuplicateError (regression: no "Duplicate injectable value" error)
- testDefaultIdFieldFieldBothInjected (behavior: both fields receive injected value)
- testFindAllInjectablesMultipleMembersForDefaultId (API shape: findAllInjectables() exposes 2+ members per ID)

Revised per review feedback from @cowtowncoder.

Switch injectables map value to List<AnnotatedMember> to support multiple @JacksonInject targets for same id. Keep existing masking rules (creator param masks same-property members, setter masks field) and add explicit failure for setter-setter conflicts on same property/id. Add memoized member->property lookup + tests.

Verify field-field and param-param don't fail with "Duplicate injectable value" error when InjectableValues is not configured. This directly tests the original issue's expected behavior: consistent error across all cases.

Verify field-field and param-param both throw MissingInjectableValueExcepion (same exception type), proving error path convergence per issue's expected behavior: "The same error should be made in all cases."

- testRecordWithRenamedInjectableProperty: record with renamed injectable - testRecordRenamedPlusDifferentFieldBothInjected: renamed creator + different property field with same ID should both be injected

github-actions · 2026-01-26T21:42:57Z

🧪 Code Coverage Report

Metric	Coverage	Change
Instructions	80.24%	📈 +0.040%
Branches	73.26%	📈 +0.020%

cowtowncoder · 2026-01-26T21:47:21Z

src/main/java/tools/jackson/databind/introspect/POJOPropertiesCollector.java

    }

-    public Map<Object, AnnotatedMember> getInjectables() {
+    public Map<Object, List<AnnotatedMember>> getInjectables() {


This is, technically speaking, incompatible API change.

For compatibility we probably need to add new method (with different name since cannot overload) (getAllInjectables()?) with simple implementation but then re-implement old "old" version which needs to take and return first value of List members. Old version should be deprecated.

Agreed, this can't work due to compatibility issue At least AS-IS.

cowtowncoder · 2026-01-26T21:48:47Z

src/main/java/tools/jackson/databind/introspect/POJOPropertiesCollector.java

+            return;
+        }
+        for (POJOPropertyBuilder creatorProp : _creatorProperties) {
+            if (creatorProp == null) {


Does this happen?

src/main/java/tools/jackson/databind/introspect/POJOPropertyBuilder.java

cowtowncoder · 2026-01-26T21:52:00Z

src/main/java/tools/jackson/databind/BeanDescription.java

     */

-    public abstract Map<Object, AnnotatedMember> findInjectables();
+    public abstract Map<Object, List<AnnotatedMember>> findInjectables();


Ouch. And even here we have API change... same approach.

cowtowncoder · 2026-01-26T21:54:58Z

Hmmh. Good job figuring out how to make this work.

My main concern is the whole addition of complexity and additional significant extra-processing. Almost to the degree I wonder if it's all worth it, or if different re-factoring/rethinking was needed.

I'll have to think about this; probably should want after 3.1 at this point.

github-actions · 2026-01-27T01:30:38Z

🧪 Code Coverage Report

Metric	Coverage	Change
Instructions	80.24%	📈 +0.040%
Branches	73.25%	📈 +0.010%

dlwldnjs1009 · 2026-01-27T15:09:01Z

@cowtowncoder
Minor note (separate from this PR):

I noticed the exception class name MissingInjectableValueExcepion (missing the “t” in “Exception”).
File: src/main/java/tools/jackson/databind/exc/MissingInjectableValueExcepion.java

I didn’t change it here since renaming would be a public API change.
If you’d like, I can open a follow-up issue/PR to discuss a backward-compatible path
(e.g., add a correctly named alias, deprecate the old name, and migrate gradually).

@deprecated

…asterXML#4218) - Restore findInjectables() signature to Map<Object, AnnotatedMember> with @deprecated; add findAllInjectables() as concrete default method - Remove _deduplicateSamePropertyInjectables (setter-masks-field policy) and _isMethod helper to reduce complexity per review feedback - Keep only _removeCreatorPropertyInjectables for FasterXML#4218 fix - Add getAllInjectables() / getInjectables() pair on POJOPropertiesCollector with defensive copy and freeze of value lists - Switch BeanDeserializerFactory to findAllInjectables() - Add default-id field-field regression, behavior, and API shape tests - Remove Rule 2 tests (testFieldAndSetterSameProperty etc.)

github-actions · 2026-01-27T15:26:18Z

🧪 Code Coverage Report

Metric	Coverage	Change
Instructions	80.18%	📉 -0.030%
Branches	73.18%	📉 -0.090%

cowtowncoder · 2026-01-27T18:39:06Z

Ah. I think I must have noticed it at some point but not before 3.0 release.

Yes, a separate PR would be make sense, targeting 3.x (for 3.1), creating correctly named sub-class (I think?) exception, deprecating misspelled one and so on.

github-actions · 2026-01-27T18:55:54Z

🧪 Code Coverage Report

Metric	Coverage	Change
Instructions	80.18%	📉 -0.030%
Branches	73.18%	📉 -0.090%

github-actions · 2026-02-04T03:09:54Z

🧪 Code Coverage Report

Metric	Coverage	Change
Instructions	80.38%	📉 -0.030%
Branches	73.47%	📉 -0.090%

github-actions · 2026-02-08T20:48:32Z

🧪 Code Coverage Report

Metric	Coverage	Change
Instructions	80.85%	📉 -0.040%
Branches	74.01%	📉 -0.110%

github-actions · 2026-02-10T05:44:27Z

🧪 Code Coverage Report

Metric	Coverage	Change
Instructions	80.98%	📉 -0.030%
Branches	74.10%	📉 -0.090%

github-actions · 2026-03-11T21:08:59Z

🧪 Code Coverage Report

Metric	Coverage	Change
Instructions	81.24%	📉 -0.040%
Branches	74.34%	📉 -0.080%

github-actions · 2026-03-24T03:17:46Z

🧪 Code Coverage Report

Metric	Coverage	Change
Instructions	81.33%	📉 -0.030%
Branches	74.54%	📉 -0.090%

dlwldnjs1009 and others added 5 commits January 25, 2026 14:35

Add regression tests for FasterXML#5217 without InjectableValues

5bad658

Verify field-field and param-param don't fail with "Duplicate injectable value" error when InjectableValues is not configured. This directly tests the original issue's expected behavior: consistent error across all cases.

Use specific exception type in regression tests (FasterXML#5217)

60eaada

Verify field-field and param-param both throw MissingInjectableValueExcepion (same exception type), proving error path convergence per issue's expected behavior: "The same error should be made in all cases."

Add record + @JsonProperty rename edge case tests (FasterXML#5217)

bc2f392

- testRecordWithRenamedInjectableProperty: record with renamed injectable - testRecordRenamedPlusDifferentFieldBothInjected: renamed creator + different property field with same ID should both be injected

Merge branch '3.x' into fix/5217-duplicate-injectable-value

4f5e882

cowtowncoder reviewed Jan 26, 2026

View reviewed changes

src/main/java/tools/jackson/databind/introspect/POJOPropertyBuilder.java Outdated Show resolved Hide resolved

cowtowncoder reviewed Jan 26, 2026

View reviewed changes

Update @SInCE tag to 3.1

650622b

Merge branch '3.x' into fix/5217-duplicate-injectable-value

c829841

Merge branch '3.x' into fix/5217-duplicate-injectable-value

4b0805b

Merge branch '3.x' into fix/5217-duplicate-injectable-value

e110efd

Merge branch '3.x' into fix/5217-duplicate-injectable-value

f1a3f37

Merge branch '3.x' into fix/5217-duplicate-injectable-value

9ff7644

Merge branch '3.x' into fix/5217-duplicate-injectable-value

4f055fb

Uh oh!

Conversation

dlwldnjs1009 commented Jan 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jan 26, 2026

🧪 Code Coverage Report

Uh oh!

cowtowncoder Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

JooHyukKim Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

cowtowncoder Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cowtowncoder Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

cowtowncoder commented Jan 26, 2026

Uh oh!

github-actions bot commented Jan 27, 2026

🧪 Code Coverage Report

Uh oh!

dlwldnjs1009 commented Jan 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jan 27, 2026

🧪 Code Coverage Report

Uh oh!

cowtowncoder commented Jan 27, 2026

Uh oh!

github-actions bot commented Jan 27, 2026

🧪 Code Coverage Report

Uh oh!

github-actions bot commented Feb 4, 2026

🧪 Code Coverage Report

Uh oh!

github-actions bot commented Feb 8, 2026

🧪 Code Coverage Report

Uh oh!

github-actions bot commented Feb 10, 2026

🧪 Code Coverage Report

Uh oh!

github-actions bot commented Mar 11, 2026

🧪 Code Coverage Report

Uh oh!

github-actions bot commented Mar 24, 2026

🧪 Code Coverage Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dlwldnjs1009 commented Jan 25, 2026 •

edited

Loading

dlwldnjs1009 commented Jan 27, 2026 •

edited

Loading