From 53a5c34ed62739feb786a6840b5fe885296f10da Mon Sep 17 00:00:00 2001 From: Filip Hrisafov Date: Wed, 7 Oct 2020 21:42:26 +0200 Subject: [PATCH] #2206, #2214, #2220: Source property should be correctly determined when only target is defined When having multiple source properties and only target is defined then the same rules should be applied as if there was no mapping: * First we check for a matching property in any of the source type * Second we check if the parameter name matches --- .../ap/internal/model/BeanMappingMethod.java | 32 ++++++++++++++- .../ap/test/bugs/_2125/Issue2125Mapper.java | 7 ++++ .../ap/test/bugs/_2125/Issue2125Test.java | 14 ++++--- .../CountryMapperMultipleSources.java | 23 +++++++++++ .../test/defaultvalue/DefaultValueTest.java | 34 ++++++++++++++++ .../ErroneousMissingSourceMapper.java | 39 +++++++++++++++++++ 6 files changed, 141 insertions(+), 8 deletions(-) create mode 100644 processor/src/test/java/org/mapstruct/ap/test/defaultvalue/CountryMapperMultipleSources.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/defaultvalue/ErroneousMissingSourceMapper.java diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/BeanMappingMethod.java b/processor/src/main/java/org/mapstruct/ap/internal/model/BeanMappingMethod.java index deeadc7ea..87b8a28b4 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/BeanMappingMethod.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/BeanMappingMethod.java @@ -1070,8 +1070,36 @@ public class BeanMappingMethod extends NormalTypeMappingMethod { SourceReference sourceRef = mappingRef.getSourceReference(); // sourceRef is not defined, check if a source property has the same name - if ( sourceRef == null && method.getSourceParameters().size() == 1 ) { - sourceRef = getSourceRefByTargetName( method.getSourceParameters().get( 0 ), targetPropertyName ); + if ( sourceRef == null ) { + // Here we follow the same rules as when we implicitly map + // When we implicitly map we first do property name based mapping + // i.e. look for matching properties in the source types + // and then do parameter name based mapping + for ( Parameter sourceParameter : method.getSourceParameters() ) { + SourceReference matchingSourceRef = getSourceRefByTargetName( + sourceParameter, + targetPropertyName + ); + if ( matchingSourceRef != null ) { + if ( sourceRef != null ) { + errorOccured = true; + // This can only happen when the target property matches multiple properties + // within the different source parameters + ctx.getMessager() + .printMessage( + method.getExecutable(), + mappingRef.getMapping().getMirror(), + Message.BEANMAPPING_SEVERAL_POSSIBLE_SOURCES, + targetPropertyName + ); + break; + } + // We can't break here since it is possible that the same property exists in multiple + // source parameters + sourceRef = matchingSourceRef; + } + } + } if ( sourceRef == null ) { diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_2125/Issue2125Mapper.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_2125/Issue2125Mapper.java index 3a867baf4..83cf0e1b2 100644 --- a/processor/src/test/java/org/mapstruct/ap/test/bugs/_2125/Issue2125Mapper.java +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_2125/Issue2125Mapper.java @@ -15,11 +15,18 @@ public interface Issue2125Mapper { Issue2125Mapper INSTANCE = Mappers.getMapper( Issue2125Mapper.class ); + // In this case the issueId from the Comment is used Comment clone(Comment comment, Integer issueId); + // When source is not defined then we will use the issueId from the Comment, + // same as when there was no mapping @Mapping(target = "issueId", qualifiedByName = "mapIssueNumber") Comment cloneWithQualifier(Comment comment, Integer issueId); + // When source is defined then we will source the parameter name + @Mapping(target = "issueId", source = "issueId", qualifiedByName = "mapIssueNumber") + Comment cloneWithQualifierExplicitSource(Comment comment, Integer issueId); + @Named("mapIssueNumber") default Integer mapIssueNumber(Integer issueNumber) { return issueNumber != null ? issueNumber + 1 : null; diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_2125/Issue2125Test.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_2125/Issue2125Test.java index 38f235509..e98afe7d5 100644 --- a/processor/src/test/java/org/mapstruct/ap/test/bugs/_2125/Issue2125Test.java +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_2125/Issue2125Test.java @@ -43,6 +43,14 @@ public class Issue2125Test { 1000 ); + assertThat( comment ).isNotNull(); + assertThat( comment.getIssueId() ).isEqualTo( 2126 ); + + comment = Issue2125Mapper.INSTANCE.cloneWithQualifierExplicitSource( + new Comment( 2125, "Fix issue" ), + 1000 + ); + assertThat( comment ).isNotNull(); assertThat( comment.getIssueId() ).isEqualTo( 1001 ); } @@ -59,12 +67,6 @@ public class Issue2125Test { alternativeLine = 17, // For some reason javac reports the error on the method instead of the annotation message = "The type of parameter \"repository\" has no property named \"issueId\". Please define the " + "source property explicitly."), - @Diagnostic(type = Issue2125ErroneousMapper.class, - kind = javax.tools.Diagnostic.Kind.ERROR, - line = 19, - alternativeLine = 21, // For some reason javac reports the error on the method instead of the annotation - message = "No property named \"issueId\" exists in source parameter(s). Please define the source " + - "explicitly.") }) public void shouldReportErrorWhenMultipleSourcesMatch() { } diff --git a/processor/src/test/java/org/mapstruct/ap/test/defaultvalue/CountryMapperMultipleSources.java b/processor/src/test/java/org/mapstruct/ap/test/defaultvalue/CountryMapperMultipleSources.java new file mode 100644 index 000000000..11a424d92 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/defaultvalue/CountryMapperMultipleSources.java @@ -0,0 +1,23 @@ +/* + * 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.defaultvalue; + +import org.mapstruct.Mapper; +import org.mapstruct.Mapping; +import org.mapstruct.factory.Mappers; + +/** + * @author Filip Hrisafov + */ +@Mapper +public interface CountryMapperMultipleSources { + + CountryMapperMultipleSources INSTANCE = Mappers.getMapper( CountryMapperMultipleSources.class ); + + @Mapping(target = "code", defaultValue = "CH") + @Mapping(target = "region", source = "regionCode") + CountryDts map(CountryEntity entity, String regionCode); +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/defaultvalue/DefaultValueTest.java b/processor/src/test/java/org/mapstruct/ap/test/defaultvalue/DefaultValueTest.java index ff7bf0072..e834df3e3 100644 --- a/processor/src/test/java/org/mapstruct/ap/test/defaultvalue/DefaultValueTest.java +++ b/processor/src/test/java/org/mapstruct/ap/test/defaultvalue/DefaultValueTest.java @@ -159,4 +159,38 @@ public class DefaultValueTest { public void errorOnDefaultValueAndExpression() { } + @Test + @IssueKey("2214") + @WithClasses({ + CountryMapperMultipleSources.class, + Region.class, + }) + public void shouldBeAbleToDetermineDefaultValueBasedOnOnlyTargetType() { + CountryEntity entity = new CountryEntity(); + CountryDts target = CountryMapperMultipleSources.INSTANCE.map( entity, "ZH" ); + + assertThat( target ).isNotNull(); + assertThat( target.getCode() ).isEqualTo( "CH" ); + } + + @Test + @IssueKey("2220") + @WithClasses({ + ErroneousMissingSourceMapper.class, + Region.class, + }) + @ExpectedCompilationOutcome( + value = CompilationResult.FAILED, + diagnostics = { + @Diagnostic( + type = ErroneousMissingSourceMapper.class, + kind = javax.tools.Diagnostic.Kind.ERROR, + line = 17, + message = "The type of parameter \"tenant\" has no property named \"type\"." + + " Please define the source property explicitly."), + } + ) + public void errorWhenOnlyTargetDefinedAndSourceDoesNotHaveProperty() { + } + } diff --git a/processor/src/test/java/org/mapstruct/ap/test/defaultvalue/ErroneousMissingSourceMapper.java b/processor/src/test/java/org/mapstruct/ap/test/defaultvalue/ErroneousMissingSourceMapper.java new file mode 100644 index 000000000..54fd3cc64 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/defaultvalue/ErroneousMissingSourceMapper.java @@ -0,0 +1,39 @@ +/* + * 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.defaultvalue; + +import org.mapstruct.Mapper; +import org.mapstruct.Mapping; + +/** + * @author Filip Hrisafov + */ +@Mapper +public interface ErroneousMissingSourceMapper { + + @Mapping(target = "type", defaultValue = "STANDARD") + Tenant map(TenantDto tenant); + + class Tenant { + + private final String id; + private final String type; + + public Tenant(String id, String type) { + this.id = id; + this.type = type; + } + } + + class TenantDto { + + private final String id; + + public TenantDto(String id) { + this.id = id; + } + } +}