FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

#4111 Fix Optional<Map>/Optional<Collection> source properties calling Optional#get() unguarded by seonwooj0810 · Pull Request #4115 · mapstruct/mapstruct · GitHub

#4111 Fix Optional<Map>/Optional<Collection> source properties calling Optional#get() unguarded - #4115

Open
seonwooj0810 wants to merge 3 commits into
mapstruct:mainfrom
seonwooj0810:fix/issue-4111-optional-map-collection-get
Open

#4111 Fix Optional<Map>/Optional<Collection> source properties calling Optional#get() unguarded#4115
seonwooj0810 wants to merge 3 commits into
mapstruct:mainfrom
seonwooj0810:fix/issue-4111-optional-map-collection-get

Conversation

Copy link
Copy Markdown
Contributor

Fixes #4111

Root cause

For a source property typed Optional<T>, scalar properties are guarded with an isPresent() check before the generated code calls Optional#get() (via handleSourceReferenceNullCheck in CommonMacros.ftl). Optional<Map<K, V>> / Optional<Collection<E>> source properties don't go through that macro though — CollectionAssignmentBuilder.setterWrapperNeedsSourceNullCheck() only requested a null-check wrapper when there's a source presence checker, NullValueCheckStrategy.ALWAYS, or a DIRECT assignment, so an Optional-wrapped collection/map fell through to the bare SetterWrapperForCollectionsAndMaps, which calls the assignment (source.getAttributes().get()) unconditionally. Mapping a source whose Optional is empty throws NoSuchElementException instead of leaving the target property unset, exactly as described in the issue.

Fix

  • CollectionAssignmentBuilder.setterWrapperNeedsSourceNullCheck() now also returns true when the source type is an Optional, so the SetterWrapperForCollectionsAndMapsWithNullCheck wrapper (and its handleLocalVarNullCheck macro) is used instead of the bare setter wrapper.
  • handleLocalVarNullCheck in CommonMacros.ftl gained an <#elseif sourceType.optionalType> branch that guards the local-var assignment with Optional#isPresent() before calling Optional#get(), mirroring the existing scalar behavior in handleSourceReferenceNullCheck.

Tests

Added processor/src/test/java/org/mapstruct/ap/test/bugs/_4111/ with a mapper that has Optional<String>, Optional<Map<String, String>> and Optional<List<String>> source properties mapped to plain target properties:

  • emptyOptionalMapAndCollectionShouldNotThrow — reproduces the reported bug: fails with NoSuchElementException before the fix, target properties are left null after the fix.
  • presentOptionalMapAndCollectionShouldBeMapped — confirms present Optionals still map through correctly.

Verification done:

  1. No in-flight PR/branch for Optional<Map> / Optional<Collection> source property generates unguarded Optional#get, throwing NoSuchElementException when empty #4111 (checked via gh pr list --search and closedByPullRequestsReferences/linkedBranches GraphQL query — both empty).
  2. No active claim on the issue (unassigned, single non-maintainer comment tracing the same root cause, no PR referencing it).
  3. Code-focused fix (.java + .ftl template), not docs/config.
  4. Confirmed the bug reproduces on current main (211b2be) by running the new test against the pre-fix code — both new-test iterations failed with NoSuchElementException at the exact reported call site; after the fix, ./mvnw -pl processor -am test -Dtest=Issue4111Test passes (4/4, JDK + Eclipse compiler).
  5. No related open epic/tracking issue found for this specific divergence.
  6. Ran the full optional.* and collection.* test packages (OptionalSimpleTest, OptionalDifferentTypesTest, OptionalNestedTest, OptionalNullCheckAlwaysTest, CollectionMappingTest, MapMappingTest, AdderTest, etc. — ~24 test classes) after the fix: all green, no regressions. ./mvnw -pl processor checkstyle:check passes.

seonwooj0810 and others added 3 commits August 19, 2026 23:30
…es calling Optional#get() unguarded

Map and Collection source properties wrapped in Optional went through
CollectionAssignmentBuilder's bare SetterWrapperForCollectionsAndMaps,
which evaluates the assignment (including the trailing Optional#get())
unconditionally, unlike scalar properties which are guarded by an
isPresent() check. An empty Optional therefore threw
NoSuchElementException instead of leaving the target property unset.

setterWrapperNeedsSourceNullCheck now requests a null-check wrapper for
Optional-typed sources, and handleLocalVarNullCheck guards the local var
assignment with isPresent() when the source is an Optional, mirroring
the existing handleSourceReferenceNullCheck behavior for scalars.
The new <#elseif sourceType.optionalType> branch in handleLocalVarNullCheck
assumed sourceType is always present, but expression-based mappings
(@mapping(expression = "java(...)")) don't provide one, causing
FreeMarker to throw InvalidReferenceException and fail compilation of
generated mappers.

Use the default-value operator so the branch is skipped when sourceType
is missing instead of blowing up.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Optional<Map> / Optional<Collection> source property generates unguarded Optional#get, throwing NoSuchElementException when empty

1 participant


Back | FazBrowse Home | New Git URL