From 272440b6dff0c8eda72aa9d217acfd640e8fe636 Mon Sep 17 00:00:00 2001 From: seonwoo_jung Date: Wed, 10 Jun 2026 06:34:56 +0900 Subject: [PATCH] #4037 Stop binding property-level @Condition to unrelated source 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. --- NEXT_RELEASE_CHANGELOG.md | 1 + .../model/PresenceCheckMethodResolver.java | 38 +++++++ .../ap/test/bugs/_4037/Issue4037Mapper.java | 106 ++++++++++++++++++ .../ap/test/bugs/_4037/Issue4037Test.java | 65 +++++++++++ 4 files changed, 210 insertions(+) create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_4037/Issue4037Mapper.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_4037/Issue4037Test.java diff --git a/NEXT_RELEASE_CHANGELOG.md b/NEXT_RELEASE_CHANGELOG.md index 8392b63c60..7798894cd1 100644 --- a/NEXT_RELEASE_CHANGELOG.md +++ b/NEXT_RELEASE_CHANGELOG.md @@ -21,6 +21,7 @@ * Prevent mapper generation from a type with a generic super bound to a type with a generic extends bound (#3994) * Fix location for Javadoc when generating the distribution zip +* Stop binding a property-level `@Condition` method to an unrelated source parameter when its type matches by accident (#4037) ### Documentation diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/PresenceCheckMethodResolver.java b/processor/src/main/java/org/mapstruct/ap/internal/model/PresenceCheckMethodResolver.java index 8500b28d49..e01b045ec3 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/PresenceCheckMethodResolver.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/PresenceCheckMethodResolver.java @@ -7,11 +7,14 @@ import java.util.ArrayList; import java.util.List; +import java.util.Objects; import java.util.stream.Collectors; import org.mapstruct.ap.internal.gem.ConditionStrategyGem; import org.mapstruct.ap.internal.model.common.Parameter; +import org.mapstruct.ap.internal.model.common.ParameterBinding; import org.mapstruct.ap.internal.model.common.PresenceCheck; +import org.mapstruct.ap.internal.model.common.SourceRHS; import org.mapstruct.ap.internal.model.presence.NullPresenceCheck; import org.mapstruct.ap.internal.model.presence.OptionalPresenceCheck; import org.mapstruct.ap.internal.model.source.Method; @@ -47,6 +50,24 @@ public static PresenceCheck getPresenceCheck( ctx ); + // For a property-level @Condition, the candidate parameter bindings include every source + // parameter of the mapping method alongside the property's source value (SourceRHS). When + // the SourceRHS type does not match any parameter of the @Condition method but an unrelated + // sibling source parameter happens to match by type, the type selector used to silently + // bind that sibling and the generated check ran on the wrong value. A sibling source + // parameter is only a valid binding when it is the root of the property's source path + // (e.g. @Mapping(source = "rootParam.nested.value") may bind a method parameter to + // rootParam itself), otherwise reject the match and let the default null check take over. + // See #4037. + SourceRHS sourceRHS = selectionParameters == null ? null : selectionParameters.getSourceRHS(); + if ( sourceRHS != null && !sourceRHS.isSourceReferenceParameter() ) { + String rootSourceParameterName = sourceRHS.getSourceParameterName(); + matchingMethods.removeIf( selected -> bindsUnrelatedSourceParameter( + selected.getParameterBindings(), + rootSourceParameterName + ) ); + } + if ( matchingMethods.isEmpty() ) { return null; } @@ -182,6 +203,23 @@ private static MethodReference getPresenceCheckMethodReference( } } + private static boolean bindsUnrelatedSourceParameter(List bindings, + String rootSourceParameterName) { + for ( ParameterBinding binding : bindings ) { + if ( !binding.isSourceParameter() ) { + continue; + } + if ( binding.isMappingTarget() || binding.isTargetType() || binding.isMappingContext() + || binding.isSourcePropertyName() || binding.isTargetPropertyName() ) { + continue; + } + if ( !Objects.equals( binding.getVariableName(), rootSourceParameterName ) ) { + return true; + } + } + return false; + } + private static List getAllAvailableMethods(Method method, List sourceModelMethods, SelectionCriteria selectionCriteria) { ParameterProvidedMethods contextProvidedMethods = method.getContextProvidedMethods(); diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_4037/Issue4037Mapper.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_4037/Issue4037Mapper.java new file mode 100644 index 0000000000..9e53debad8 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_4037/Issue4037Mapper.java @@ -0,0 +1,106 @@ +/* + * Copyright MapStruct Authors. + * + * Licensed under the Apache License version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0 + */ +package org.mapstruct.ap.test.bugs._4037; + +import org.mapstruct.Condition; +import org.mapstruct.Mapper; +import org.mapstruct.Mapping; +import org.mapstruct.NullValueCheckStrategy; +import org.mapstruct.factory.Mappers; + +@Mapper( + nullValueCheckStrategy = NullValueCheckStrategy.ALWAYS, + uses = Issue4037Mapper.PrechecksSupport.class +) +public interface Issue4037Mapper { + + Issue4037Mapper INSTANCE = Mappers.getMapper( Issue4037Mapper.class ); + + @Mapping(target = "browserId", source = "requestBean.deviceInfo.browserId") + @Mapping(target = "osId", source = "requestBean.deviceInfo.osId") + @Mapping(target = "deviceName", source = "requestBean.deviceInfo.deviceName") + AudienceProfileRequest map(RequestBean requestBean, String slotId); + + class RequestBean { + private DeviceInfo deviceInfo; + + public DeviceInfo getDeviceInfo() { + return deviceInfo; + } + + public void setDeviceInfo(DeviceInfo deviceInfo) { + this.deviceInfo = deviceInfo; + } + } + + class DeviceInfo { + private Integer browserId; + private Integer osId; + private String deviceName; + + public Integer getBrowserId() { + return browserId; + } + + public void setBrowserId(Integer browserId) { + this.browserId = browserId; + } + + public Integer getOsId() { + return osId; + } + + public void setOsId(Integer osId) { + this.osId = osId; + } + + public String getDeviceName() { + return deviceName; + } + + public void setDeviceName(String deviceName) { + this.deviceName = deviceName; + } + } + + class AudienceProfileRequest { + private Integer browserId; + private Integer osId; + private String deviceName; + + public Integer getBrowserId() { + return browserId; + } + + public void setBrowserId(Integer browserId) { + this.browserId = browserId; + } + + public Integer getOsId() { + return osId; + } + + public void setOsId(Integer osId) { + this.osId = osId; + } + + public String getDeviceName() { + return deviceName; + } + + public void setDeviceName(String deviceName) { + this.deviceName = deviceName; + } + } + + class PrechecksSupport { + + @Condition + public boolean isNotBlank(String value) { + return value != null && !value.trim().isEmpty(); + } + } +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_4037/Issue4037Test.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_4037/Issue4037Test.java new file mode 100644 index 0000000000..cf0ad1e2f2 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_4037/Issue4037Test.java @@ -0,0 +1,65 @@ +/* + * Copyright MapStruct Authors. + * + * Licensed under the Apache License version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0 + */ +package org.mapstruct.ap.test.bugs._4037; + +import org.mapstruct.ap.testutil.IssueKey; +import org.mapstruct.ap.testutil.ProcessorTest; +import org.mapstruct.ap.testutil.WithClasses; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Reproducer for #4037. + *

+ * When the mapping method declares more than one source parameter and a {@code @Condition} method + * exists whose parameter type matches one of those sibling parameters, the type selector used to + * silently bind the unrelated sibling to the {@code @Condition} call. The presence check was + * therefore generated against the wrong value (for example {@code isNotBlank(slotId)} as the guard + * for an {@code Integer} property) and could discard valid property values whenever the unrelated + * source happened to be blank. + */ +@IssueKey("4037") +@WithClasses(Issue4037Mapper.class) +public class Issue4037Test { + + @ProcessorTest + public void presenceCheckOnIntegerPropertyDoesNotConsumeUnrelatedStringParameter() { + Issue4037Mapper.RequestBean requestBean = new Issue4037Mapper.RequestBean(); + Issue4037Mapper.DeviceInfo deviceInfo = new Issue4037Mapper.DeviceInfo(); + deviceInfo.setBrowserId( 123 ); + deviceInfo.setOsId( 456 ); + deviceInfo.setDeviceName( "Pixel" ); + requestBean.setDeviceInfo( deviceInfo ); + + // The sibling slotId parameter is blank. Before the fix, isNotBlank(slotId) was generated as + // the guard for every property, so blank slotId silently nulled out browserId and osId. + Issue4037Mapper.AudienceProfileRequest profile = Issue4037Mapper.INSTANCE.map( requestBean, "" ); + + assertThat( profile.getBrowserId() ).isEqualTo( 123 ); + assertThat( profile.getOsId() ).isEqualTo( 456 ); + // String property of matching type still gets the @Condition. + assertThat( profile.getDeviceName() ).isEqualTo( "Pixel" ); + } + + @ProcessorTest + public void presenceCheckOnStringPropertyStillApplies() { + // The @Condition is still applied to property sources of matching type. A blank deviceName + // should be dropped while non-blank ones (and unrelated numeric properties) pass through. + Issue4037Mapper.RequestBean requestBean = new Issue4037Mapper.RequestBean(); + Issue4037Mapper.DeviceInfo deviceInfo = new Issue4037Mapper.DeviceInfo(); + deviceInfo.setBrowserId( 7 ); + deviceInfo.setOsId( null ); + deviceInfo.setDeviceName( " " ); + requestBean.setDeviceInfo( deviceInfo ); + + Issue4037Mapper.AudienceProfileRequest profile = + Issue4037Mapper.INSTANCE.map( requestBean, "anything" ); + + assertThat( profile.getBrowserId() ).isEqualTo( 7 ); + assertThat( profile.getOsId() ).isNull(); + assertThat( profile.getDeviceName() ).isNull(); + } +}