From bef3482af5e539ac0e5412cf9ba42037ea19a521 Mon Sep 17 00:00:00 2001 From: Filip Hrisafov Date: Sat, 16 Jan 2021 19:43:22 +0100 Subject: [PATCH] #2250, #2324 Do not throw error when qualifier is missing on iterable mapping With this change we are relaxing the error handling when qualifiers are used in `@Mapping` and we allow defining qualifiers for collections / array mapping to mean the same as if it was defined on `@IterableMapping` i.e. apply the qualifier on the element mapping --- .../model/ContainerMappingMethodBuilder.java | 10 +++- .../ap/internal/model/PropertyMapping.java | 1 + .../creation/MappingResolverImpl.java | 8 +++- ...neousMessageByNamedWithIterableMapper.java | 31 ++++++++++++ .../qualifier/errors/QualfierMessageTest.java | 23 +++++++++ .../selection/qualifier/iterable/Cities.java | 23 +++++++++ .../iterable/IterableAndQualifiersTest.java | 40 +++++++++++++++- .../selection/qualifier/iterable/Rivers.java | 23 +++++++++ .../qualifier/iterable/TopologyMapper.java | 17 ------- .../TopologyWithoutIterableMappingMapper.java | 47 +++++++++++++++++++ 10 files changed, 203 insertions(+), 20 deletions(-) create mode 100644 processor/src/test/java/org/mapstruct/ap/test/selection/qualifier/errors/ErroneousMessageByNamedWithIterableMapper.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/selection/qualifier/iterable/Cities.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/selection/qualifier/iterable/Rivers.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/selection/qualifier/iterable/TopologyWithoutIterableMappingMapper.java diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/ContainerMappingMethodBuilder.java b/processor/src/main/java/org/mapstruct/ap/internal/model/ContainerMappingMethodBuilder.java index b9bbf6d09..be08ac1ca 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/ContainerMappingMethodBuilder.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/ContainerMappingMethodBuilder.java @@ -22,6 +22,8 @@ import org.mapstruct.ap.internal.util.Strings; import static org.mapstruct.ap.internal.util.Collections.first; +import javax.lang.model.element.AnnotationMirror; + /** * Builder that can be used to build {@link ContainerMappingMethod}(s). * @@ -37,6 +39,7 @@ public abstract class ContainerMappingMethodBuilder selfType, String errorMessagePart) { super( selfType ); @@ -58,6 +61,11 @@ public abstract class ContainerMappingMethodBuilder forge( sourceRHS, sourceElementType, targetElementType ) ); diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/PropertyMapping.java b/processor/src/main/java/org/mapstruct/ap/internal/model/PropertyMapping.java index dbda3daac..459c42805 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/PropertyMapping.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/PropertyMapping.java @@ -617,6 +617,7 @@ public class PropertyMapping extends ModelElement { .method( methodRef ) .selectionParameters( selectionParameters ) .callingContextTargetPropertyName( targetPropertyName ) + .positionHint( positionHint ) .build(); return createForgedAssignment( source, methodRef, iterableMappingMethod ); diff --git a/processor/src/main/java/org/mapstruct/ap/internal/processor/creation/MappingResolverImpl.java b/processor/src/main/java/org/mapstruct/ap/internal/processor/creation/MappingResolverImpl.java index 73392a00f..4394ab7b4 100755 --- a/processor/src/main/java/org/mapstruct/ap/internal/processor/creation/MappingResolverImpl.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/processor/creation/MappingResolverImpl.java @@ -298,7 +298,13 @@ public class MappingResolverImpl implements MappingResolver { } if ( hasQualfiers() ) { - printQualifierMessage( selectionCriteria ); + if ((sourceType.isCollectionType() || sourceType.isArrayType()) && targetType.isIterableType()) { + // Allow forging iterable mapping when no iterable mapping already found + return forger.get(); + } + else { + printQualifierMessage( selectionCriteria ); + } } else if ( allowMappingMethod() ) { // only forge if we would allow mapping method diff --git a/processor/src/test/java/org/mapstruct/ap/test/selection/qualifier/errors/ErroneousMessageByNamedWithIterableMapper.java b/processor/src/test/java/org/mapstruct/ap/test/selection/qualifier/errors/ErroneousMessageByNamedWithIterableMapper.java new file mode 100644 index 000000000..088207c61 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/selection/qualifier/errors/ErroneousMessageByNamedWithIterableMapper.java @@ -0,0 +1,31 @@ +/* + * 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.selection.qualifier.errors; + +import java.util.Collection; + +import org.mapstruct.Mapper; +import org.mapstruct.Mapping; + +@Mapper +public interface ErroneousMessageByNamedWithIterableMapper { + + @Mapping(target = "elements", qualifiedByName = "SelectMe") + Target map(Source source); + + // CHECKSTYLE:OFF + class Source { + + public Collection elements; + } + + class Target { + + public Collection elements; + } + // CHECKSTYLE ON + +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/selection/qualifier/errors/QualfierMessageTest.java b/processor/src/test/java/org/mapstruct/ap/test/selection/qualifier/errors/QualfierMessageTest.java index 4c8edc64d..48295ed4f 100644 --- a/processor/src/test/java/org/mapstruct/ap/test/selection/qualifier/errors/QualfierMessageTest.java +++ b/processor/src/test/java/org/mapstruct/ap/test/selection/qualifier/errors/QualfierMessageTest.java @@ -87,4 +87,27 @@ public class QualfierMessageTest { ) public void testNoQualifyingMethodByAnnotationAndNamedFound() { } + + @Test + @WithClasses(ErroneousMessageByNamedWithIterableMapper.class) + @ExpectedCompilationOutcome( + value = CompilationResult.FAILED, + diagnostics = { + @Diagnostic( + type = ErroneousMessageByNamedWithIterableMapper.class, + kind = ERROR, + line = 16, + message = "Qualifier error. No method found annotated with @Named#value: [ SelectMe ]. " + + "See https://mapstruct.org/faq/#qualifier for more info."), + @Diagnostic( + type = ErroneousMessageByNamedWithIterableMapper.class, + kind = ERROR, + line = 16, + messageRegExp = "Can't map property.*"), + + } + ) + public void testNoQualifyingMethodByNamedForForgedIterableFound() { + } + } diff --git a/processor/src/test/java/org/mapstruct/ap/test/selection/qualifier/iterable/Cities.java b/processor/src/test/java/org/mapstruct/ap/test/selection/qualifier/iterable/Cities.java new file mode 100644 index 000000000..828750399 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/selection/qualifier/iterable/Cities.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.selection.qualifier.iterable; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +import org.mapstruct.Qualifier; + +/** + * @author Filip Hrisafov + */ +@Qualifier +@Target(ElementType.METHOD) +@Retention(RetentionPolicy.CLASS) +public @interface Cities { + +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/selection/qualifier/iterable/IterableAndQualifiersTest.java b/processor/src/test/java/org/mapstruct/ap/test/selection/qualifier/iterable/IterableAndQualifiersTest.java index db8de9496..c2465b255 100644 --- a/processor/src/test/java/org/mapstruct/ap/test/selection/qualifier/iterable/IterableAndQualifiersTest.java +++ b/processor/src/test/java/org/mapstruct/ap/test/selection/qualifier/iterable/IterableAndQualifiersTest.java @@ -15,6 +15,7 @@ import org.junit.runner.RunWith; import org.mapstruct.ap.testutil.IssueKey; import org.mapstruct.ap.testutil.WithClasses; import org.mapstruct.ap.testutil.runner.AnnotationProcessorTestRunner; +import org.mapstruct.factory.Mappers; /** * @@ -22,20 +23,22 @@ import org.mapstruct.ap.testutil.runner.AnnotationProcessorTestRunner; */ @IssueKey( "707" ) @WithClasses( { + Cities.class, CityDto.class, CityEntity.class, RiverDto.class, RiverEntity.class, + Rivers.class, TopologyDto.class, TopologyEntity.class, TopologyFeatureDto.class, TopologyFeatureEntity.class, - TopologyMapper.class } ) @RunWith( AnnotationProcessorTestRunner.class ) public class IterableAndQualifiersTest { @Test + @WithClasses(TopologyMapper.class) public void testGenerationBasedOnQualifier() { TopologyDto topologyDto1 = new TopologyDto(); @@ -67,4 +70,39 @@ public class IterableAndQualifiersTest { assertThat( ( (CityEntity) result2.getTopologyFeatures().get( 0 ) ).getPopulation() ).isEqualTo( 800000 ); } + + @Test + @WithClasses(TopologyWithoutIterableMappingMapper.class) + public void testIterableGeneratorBasedOnQualifier() { + + TopologyWithoutIterableMappingMapper mapper = Mappers.getMapper( TopologyWithoutIterableMappingMapper.class ); + + TopologyDto riverTopologyDto = new TopologyDto(); + List topologyFeatures1 = new ArrayList<>(); + RiverDto riverDto = new RiverDto(); + riverDto.setName( "Rhine" ); + riverDto.setLength( 5 ); + topologyFeatures1.add( riverDto ); + riverTopologyDto.setTopologyFeatures( topologyFeatures1 ); + + TopologyEntity riverTopology = mapper.mapTopologyAsRiver( riverTopologyDto ); + assertThat( riverTopology.getTopologyFeatures() ).hasSize( 1 ); + assertThat( riverTopology.getTopologyFeatures().get( 0 ).getName() ).isEqualTo( "Rhine" ); + assertThat( riverTopology.getTopologyFeatures().get( 0 ) ).isInstanceOf( RiverEntity.class ); + assertThat( ( (RiverEntity) riverTopology.getTopologyFeatures().get( 0 ) ).getLength() ).isEqualTo( 5 ); + + TopologyDto cityTopologyDto = new TopologyDto(); + List topologyFeatures2 = new ArrayList<>(); + CityDto cityDto = new CityDto(); + cityDto.setName( "Amsterdam" ); + cityDto.setPopulation( 800000 ); + topologyFeatures2.add( cityDto ); + cityTopologyDto.setTopologyFeatures( topologyFeatures2 ); + + TopologyEntity cityTopology = mapper.mapTopologyAsCity( cityTopologyDto ); + assertThat( cityTopology.getTopologyFeatures() ).hasSize( 1 ); + assertThat( cityTopology.getTopologyFeatures().get( 0 ).getName() ).isEqualTo( "Amsterdam" ); + assertThat( cityTopology.getTopologyFeatures().get( 0 ) ).isInstanceOf( CityEntity.class ); + assertThat( ( (CityEntity) cityTopology.getTopologyFeatures().get( 0 ) ).getPopulation() ).isEqualTo( 800000 ); + } } diff --git a/processor/src/test/java/org/mapstruct/ap/test/selection/qualifier/iterable/Rivers.java b/processor/src/test/java/org/mapstruct/ap/test/selection/qualifier/iterable/Rivers.java new file mode 100644 index 000000000..f6a56951a --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/selection/qualifier/iterable/Rivers.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.selection.qualifier.iterable; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +import org.mapstruct.Qualifier; + +/** + * @author Filip Hrisafov + */ +@Qualifier +@Target(ElementType.METHOD) +@Retention(RetentionPolicy.CLASS) +public @interface Rivers { + +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/selection/qualifier/iterable/TopologyMapper.java b/processor/src/test/java/org/mapstruct/ap/test/selection/qualifier/iterable/TopologyMapper.java index 22e87dd08..84c4bc58a 100644 --- a/processor/src/test/java/org/mapstruct/ap/test/selection/qualifier/iterable/TopologyMapper.java +++ b/processor/src/test/java/org/mapstruct/ap/test/selection/qualifier/iterable/TopologyMapper.java @@ -5,16 +5,11 @@ */ package org.mapstruct.ap.test.selection.qualifier.iterable; -import java.lang.annotation.ElementType; -import java.lang.annotation.Retention; -import java.lang.annotation.RetentionPolicy; -import java.lang.annotation.Target; import java.util.List; import org.mapstruct.IterableMapping; import org.mapstruct.Mapper; import org.mapstruct.Mapping; -import org.mapstruct.Qualifier; import org.mapstruct.factory.Mappers; /** @@ -64,16 +59,4 @@ public abstract class TopologyMapper { return topologyFeatureEntity; } - @Qualifier - @Target(ElementType.METHOD) - @Retention(RetentionPolicy.CLASS) - public @interface Rivers { - } - - @Qualifier - @Target(ElementType.METHOD) - @Retention(RetentionPolicy.CLASS) - public @interface Cities { - } - } diff --git a/processor/src/test/java/org/mapstruct/ap/test/selection/qualifier/iterable/TopologyWithoutIterableMappingMapper.java b/processor/src/test/java/org/mapstruct/ap/test/selection/qualifier/iterable/TopologyWithoutIterableMappingMapper.java new file mode 100644 index 000000000..4b72b3466 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/selection/qualifier/iterable/TopologyWithoutIterableMappingMapper.java @@ -0,0 +1,47 @@ +/* + * 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.selection.qualifier.iterable; + +import org.mapstruct.Mapper; +import org.mapstruct.Mapping; + +/** + * @author Filip Hrisafov + */ +@Mapper +public interface TopologyWithoutIterableMappingMapper { + + @Mapping( target = "topologyFeatures", qualifiedBy = Rivers.class ) + TopologyEntity mapTopologyAsRiver(TopologyDto dto); + + @Mapping( target = "topologyFeatures", qualifiedBy = Cities.class ) + TopologyEntity mapTopologyAsCity(TopologyDto dto); + + @Rivers + default TopologyFeatureEntity mapRiver( TopologyFeatureDto dto ) { + TopologyFeatureEntity topologyFeatureEntity = null; + if ( dto instanceof RiverDto ) { + RiverEntity riverEntity = new RiverEntity(); + riverEntity.setLength( ( (RiverDto) dto ).getLength() ); + topologyFeatureEntity = riverEntity; + topologyFeatureEntity.setName( dto.getName() ); + } + return topologyFeatureEntity; + } + + @Cities + default TopologyFeatureEntity mapCity( TopologyFeatureDto dto ) { + TopologyFeatureEntity topologyFeatureEntity = null; + if ( dto instanceof CityDto ) { + CityEntity cityEntity = new CityEntity(); + cityEntity.setPopulation( ( (CityDto) dto ).getPopulation() ); + topologyFeatureEntity = cityEntity; + topologyFeatureEntity.setName( dto.getName() ); + } + return topologyFeatureEntity; + } + +}