Skip to content

fix: resolve MissingFieldException when mapping primitive field into …#641

Merged
fborriello merged 12 commits intoExpediaGroup:masterfrom
vijaygovindaraja:fix/559-primitive-field-nested-mapping
Mar 28, 2026
Merged

fix: resolve MissingFieldException when mapping primitive field into …#641
fborriello merged 12 commits intoExpediaGroup:masterfrom
vijaygovindaraja:fix/559-primitive-field-nested-mapping

Conversation

@vijaygovindaraja
Copy link
Copy Markdown
Contributor

@vijaygovindaraja vijaygovindaraja commented Mar 26, 2026 •
edited by fborriello
Loading

Fixes #599

Summary

When using withFieldMapping(new FieldMapping<>("x", "nestedObject.x")) to map a root-level source field into a
nested destination field, the transformer incorrectly looked for field x in the nested source object (FromSubBean)
instead of the root source (FromBean), throwing MissingFieldException.

Fix

  • Store the root source object in a ThreadLocal during transformation
  • When resolving field mappings, check the full breadcrumb path (e.g. nestedObject.x) against the mapping table
  • If a breadcrumb-qualified mapping is found, resolve the source field from the root source object instead of the
    current nested context
  • Cleaned up with try/finally to ensure ThreadLocal is always removed

Test

Added testPrimitiveFieldMappedIntoNestedObject in MixedObjectTransformationTest reproducing the exact scenario
from the issue. All 96 functional tests pass.

fix: resolve MissingFieldException when mapping primitive field into …
d21d072 
…nested object (ExpediaGroup#559)

When using withFieldMapping to map a root-level source field into a nested
destination field (e.g. "x" -> "nestedObject.x"), the transformer incorrectly
looked for the field in the nested source object instead of the root source.

The fix stores the root source object in a ThreadLocal during transformation
and uses it to resolve field mappings that reference breadcrumb-qualified
destination paths. This ensures root-level source fields are correctly
accessible when processing nested destination objects.

Closes ExpediaGroup#559
@vijaygovindaraja vijaygovindaraja requested review from a team and fborriello as code owners March 26, 2026 22:39
fborriello
fborriello reviewed Mar 28, 2026
View reviewed changes
refactor: address review feedback — extract helper, remove Optional n…
9363080 
…o-op, add test coverage

- Extract resolveEffectiveSource() helper to centralize breadcrumb-based
  mapping resolution, eliminating duplication between getConstructorArgsValues
  and getFieldValue
- Remove unnecessary Optional wrapping (ofNullable(...).orElse(null)) in
  favor of direct Map.get()
- Add Javadoc note about ThreadLocal reentrant edge case
- Add test: mutable bean setter-injection path
- Add test: differing source/destination field names (x -> nestedObject.y)
- Add test: zero/default primitive value maps correctly
- Add test: no-mapping throws MissingFieldException
- Add test: multiple root fields mapped into same nested object
@vijaygovindaraja
Copy link
Copy Markdown
Contributor Author

vijaygovindaraja commented Mar 28, 2026

@fborriello hey, thanks for the detailed review. pushed a commit addressing everything:

  • dropped the unnecessary ofNullable(...).orElse(null), just using Map.get() now
  • pulled out a resolveEffectiveSource() helper so the breadcrumb logic isn't duplicated anymore
  • added the javadoc note about the reentrant ThreadLocal thing
  • added tests for the mutable setter path, differing field names, zero value, and the no-mapping exception case

let me know if anything else needs changing

fborriello
fborriello requested changes Mar 28, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@fborriello fborriello left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added a new comment

@fborriello
Copy link
Copy Markdown
Member

fborriello commented Mar 28, 2026 •
edited
Loading

@vijaygovindaraja, please also add the changes as part of the CHANGELOG.md and CHANGELOG-JDK11.md so that in the next release, it will be clear the things part of it.
It would be much appreciated if you can mark the comments addressed as resolved.
thanks

fborriello and others added 2 commits March 28, 2026 17:06
@fborriello
Merge branch 'master' into fix/559-primitive-field-nested-mapping
bd4ed97
chore: add changelog entries and remove unused method
2ff1c8a 
- Add changelog entries for ExpediaGroup#559 fix in CHANGELOG.md and CHANGELOG-JDK11.md
- Remove unused getSourceFieldName(String, String) two-arg overload
@vijaygovindaraja
Copy link
Copy Markdown
Contributor Author

vijaygovindaraja commented Mar 28, 2026

@fborriello done — added changelog entries to both files and removed the unused two-arg getSourceFieldName method you caught. also resolved all the review threads.

fix: replace record with inner class to satisfy JaCoCo method coverage
2c4e287 
Record auto-generates equals/hashCode/toString that are not exercised
by tests, causing JaCoCo method coverage to drop below the required
100% threshold.
@vijaygovindaraja
Copy link
Copy Markdown
Contributor Author

vijaygovindaraja commented Mar 28, 2026

@fborriello the build was failing because the record auto-generates equals/hashCode/toString methods that weren't exercised by tests, dropping JaCoCo method coverage below 100%. replaced it with a plain inner class so there are no uncovered methods. build should be green now.

thanks

fix: resolve checkstyle errors in EffectiveSource inner class
31c32c4 
- Add Javadoc comments to fields
- Rename constructor params to avoid HiddenField violation
@fborriello
Copy link
Copy Markdown
Member

fborriello commented Mar 28, 2026

getSourceFieldName(Field field) in the TransformerImpl is now dead code — your refactoring replaced the only call site (getFieldValue(sourceObj, targetObject, targetClass, field, breadcrumb)) with resolveEffectiveSource(...), which calls getSourceFieldName(String) directly. Please remove the Field overload
and its Javadoc.

@vijaygovindaraja
Copy link
Copy Markdown
Contributor Author

vijaygovindaraja commented Mar 28, 2026

@fborriello good catch, removed it. thanks

@fborriello fborriello enabled auto-merge (squash) March 28, 2026 16:48
@fborriello
Copy link
Copy Markdown
Member

fborriello commented Mar 28, 2026

@vijaygovindaraja the actions haven't triggered, would you mind doing an empty push so that we can merge this? A release of a new bull version will be performed early next week

@vijaygovindaraja
Copy link
Copy Markdown
Contributor Author

vijaygovindaraja commented Mar 28, 2026

@fborriello done, pushed an empty commit to trigger the actions

@fborriello fborriello disabled auto-merge March 28, 2026 17:01
@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 28, 2026 •
edited
Loading

Test Coverage Report

Coverage Baseline Build Diff
Lines 100.00% 1205/1205 100.00% 1224/1224 +0.00% 💙
Branches 95.47% 443/464 94.79% 455/480 -0.68% 🔴
Instructions 99.80% 5976/5988 99.79% 6067/6080 -0.01% 💙
Complexity 97.41% 751/771 96.92% 756/780 -0.49% 🔴
Methods 100.00% 539/539 100.00% 540/540 +0.00% 💙
Classes 100.00% 49/49 100.00% 50/50 +0.00% 💙
Normalised Score 98.78% 98.58% -0.20% 🔴

@fborriello fborriello enabled auto-merge (squash) March 28, 2026 17:08
@fborriello fborriello disabled auto-merge March 28, 2026 17:12
@fborriello fborriello merged commit 6fa0d96 into ExpediaGroup:master Mar 28, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fborriello fborriello fborriello approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@vijaygovindaraja @fborriello