#4037 Stop binding property-level @Condition to unrelated source parameter#4069
Open
seonwooj0810 wants to merge 1 commit into
Open
#4037 Stop binding property-level @Condition to unrelated source parameter#4069seonwooj0810 wants to merge 1 commit into
seonwooj0810 wants to merge 1 commit into
Conversation
…urce parameter For property-level @condition resolution the TypeSelector permutes every candidate ParameterBinding, which for a mapping method declaring multiple source parameters includes those siblings alongside the property's own SourceRHS. When the SourceRHS type does not match any parameter of the @condition method but a sibling source parameter happens to match by type, the selector silently bound the sibling and the check ran on an unrelated value (e.g. isNotBlank(slotId) was generated as the guard for an Integer property). After the matching methods are found, drop any whose chosen binding consumes a sibling source parameter that is not the root of the property's source path. Bindings to the SourceRHS itself, to context/target/source-property-name parameters, and to the source parameter whose property is being mapped (rootParam in @mapping(source = "rootParam.nested.value")) remain valid; with no acceptable binding the resolver falls back to the default null check.
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
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #4037.
Why this happens
For a property-level
@Condition,PresenceCheckMethodResolverasksTypeSelectorto bind every parameter of the@Conditionmethod to an availableParameterBinding. The bindings published for a property mapping include both the property's source value (SourceRHS) and every other source parameter of the mapping method.TypeSelectordoes not know which binding is the value being checked. It enumerates permutations and keeps the ones whose types are assignable. WhenSourceRHSdoes not match the@Conditionparameter's type but an unrelated sibling source parameter happens to match, that sibling is silently selected — even though it has nothing to do with the property being mapped.In the reporter's example:
browserIdis anInteger, soSourceRHSdoes not matchisNotBlank(String). The siblingslotIdparameter does, so MapStruct generatesA blank
slotIdwould then silently null out everyIntegerproperty of the result even when the actual source values are present.What changes
After
PresenceCheckMethodResolver.getPresenceCheckcollects the matching methods, discard any whose chosen binding consumes a sibling source parameter that is not the root of the property's source path. The root isSourceRHS#getSourceParameterName()— e.g.requestBeanforsource = "requestBean.deviceInfo.browserId". All other binding kinds (theSourceRHSitself,@MappingContext,@TargetType,@SourcePropertyName,@TargetPropertyName,@MappingTarget) remain valid candidates.This keeps every existing pattern working:
@Condition isNotBlank(String value)on aStringproperty —valuebinds toSourceRHS.@Condition shouldMap(EmployeeDto source)on a property ofemployee—sourcebinds to the mapper parameter, which is the root of the property's source path.@Condition isAmericanCitizen(EmployeeDto employerDto)referenced viaconditionQualifiedByName— same as above.@Condition isPresent(EmployeeDto source, String value)—sourcebinds to the parameter root,valuebinds toSourceRHS.When the unrelated-sibling case is the only match left, the resolver returns
nulland the property falls back to the default null check (or no check, depending onnullValueCheckStrategy).Tests
org.mapstruct.ap.test.bugs._4037.Issue4037Test— multi-source-parameter mapper withnullValueCheckStrategy = ALWAYSand a@Condition isNotBlank(String). Confirms theIntegerproperties no longer useisNotBlank(slotId)as their guard while theStringproperty still gets the@Condition.processormodule test suite runs green (3633 tests). The conditional/qualifier/expression and presence-check suites in particular continue to pass without changes.Verification done
./mvnw -pl processor -Dtest='Issue4037Test' test— 4/4 pass../mvnw -pl processor -Dtest='*Conditional*Test,*Condition*Test,Issue4037Test' test— 58/58 pass../mvnw -pl processor -Dtest='*NullCheck*Test,*NullValueCheckStrategy*Test,*Selection*Test,*Presence*Test,*PresenceCheck*Test,*Selector*Test' test— 136/136 pass../mvnw -pl processor -Dtest='org.mapstruct.ap.test.bugs.**,*MultiSource*Test,*SourcePresence*Test' test— 696/696 pass../mvnw -pl processor test— 3633/3633 pass../mvnw -pl processor checkstyle:check— 0 violations.Issue4037MapperImpl.java) and confirmed the previously buggyif ( prechecksSupport.isNotBlank( slotId ) )guard is replaced with the defaultif ( browserId != null )/if ( osId != null )check, while the matching String property still emitsif ( prechecksSupport.isNotBlank( deviceName ) ).