From 2f78d3f4e2f493239ed15e6fe2bb85daab4bfed8 Mon Sep 17 00:00:00 2001 From: Zegveld <41897697+Zegveld@users.noreply.github.com> Date: Sun, 30 Apr 2023 16:33:00 +0200 Subject: [PATCH] #3125: Allow subclassmapping inheritance for methods with identical signature Co-authored-by: Ben Zegveld --- .../model/source/MappingMethodOptions.java | 36 ++++++++++++++++-- .../model/source/SubclassMappingOptions.java | 17 +++++++++ .../InheritedSubclassMapper.java | 38 +++++++++++++++++++ .../subclassmapping/SubclassMappingTest.java | 20 +++++++++- 4 files changed, 107 insertions(+), 4 deletions(-) create mode 100644 processor/src/test/java/org/mapstruct/ap/test/subclassmapping/InheritedSubclassMapper.java diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/source/MappingMethodOptions.java b/processor/src/main/java/org/mapstruct/ap/internal/model/source/MappingMethodOptions.java index 2124dd974..73474a847 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/source/MappingMethodOptions.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/source/MappingMethodOptions.java @@ -5,12 +5,15 @@ */ package org.mapstruct.ap.internal.model.source; +import static org.mapstruct.ap.internal.model.source.MappingOptions.getMappingTargetNamesBy; + import java.util.Collections; import java.util.HashSet; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Set; + import javax.lang.model.element.AnnotationMirror; import org.mapstruct.ap.internal.gem.CollectionMappingStrategyGem; @@ -20,8 +23,6 @@ import org.mapstruct.ap.internal.util.FormattingMessager; import org.mapstruct.ap.internal.util.Message; import org.mapstruct.ap.internal.util.accessor.Accessor; -import static org.mapstruct.ap.internal.model.source.MappingOptions.getMappingTargetNamesBy; - /** * Encapsulates all options specifiable on a mapping method * @@ -205,12 +206,19 @@ public class MappingMethodOptions { } if ( isInverse ) { - // normal inheritence of subclass mappings will result runtime in infinite loops. List inheritedMappings = SubclassMappingOptions.copyForInverseInheritance( templateOptions.getSubclassMappings(), getBeanMapping() ); addAllNonRedefined( sourceMethod, annotationMirror, inheritedMappings ); } + else if ( methodsHaveIdenticalSignature( templateMethod, sourceMethod ) ) { + List inheritedMappings = + SubclassMappingOptions + .copyForInheritance( + templateOptions.getSubclassMappings(), + getBeanMapping() ); + addAllNonRedefined( sourceMethod, annotationMirror, inheritedMappings ); + } Set newMappings = new LinkedHashSet<>(); for ( MappingOptions mapping : templateOptions.getMappings() ) { @@ -232,6 +240,28 @@ public class MappingMethodOptions { } } + private boolean methodsHaveIdenticalSignature(SourceMethod templateMethod, SourceMethod sourceMethod) { + return parametersAreOfIdenticalTypeAndOrder( templateMethod, sourceMethod ) + && resultTypeIsTheSame( templateMethod, sourceMethod ); + } + + private boolean parametersAreOfIdenticalTypeAndOrder(SourceMethod templateMethod, SourceMethod sourceMethod) { + if (templateMethod.getParameters().size() != sourceMethod.getParameters().size()) { + return false; + } + for ( int i = 0; i < templateMethod.getParameters().size(); i++ ) { + if (!templateMethod.getParameters().get( i ).getType().equals( + sourceMethod.getParameters().get( i ).getType() ) ) { + return false; + } + } + return true; + } + + private boolean resultTypeIsTheSame(SourceMethod templateMethod, SourceMethod sourceMethod) { + return templateMethod.getResultType().equals( sourceMethod.getResultType() ); + } + private void addAllNonRedefined(SourceMethod sourceMethod, AnnotationMirror annotationMirror, List inheritedMappings) { Set redefinedSubclassMappings = new HashSet<>( subclassMappings ); diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/source/SubclassMappingOptions.java b/processor/src/main/java/org/mapstruct/ap/internal/model/source/SubclassMappingOptions.java index 11b4b283c..2b0e6dd13 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/source/SubclassMappingOptions.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/source/SubclassMappingOptions.java @@ -202,6 +202,23 @@ public class SubclassMappingOptions extends DelegatingOptions { ) ).collect( Collectors.toCollection( ArrayList::new ) ); } + public static List copyForInheritance(Set subclassMappings, + BeanMappingOptions beanMappingOptions) { + // we are not interested in keeping it unique at this point. + List mappings = new ArrayList<>(); + for ( SubclassMappingOptions subclassMapping : subclassMappings ) { + mappings.add( + new SubclassMappingOptions( + subclassMapping.source, + subclassMapping.target, + subclassMapping.typeUtils, + beanMappingOptions, + subclassMapping.selectionParameters, + subclassMapping.subclassMapping ) ); + } + return mappings; + } + @Override public boolean equals(Object obj) { if ( obj == null || !( obj instanceof SubclassMappingOptions ) ) { diff --git a/processor/src/test/java/org/mapstruct/ap/test/subclassmapping/InheritedSubclassMapper.java b/processor/src/test/java/org/mapstruct/ap/test/subclassmapping/InheritedSubclassMapper.java new file mode 100644 index 000000000..6e4aca98b --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/subclassmapping/InheritedSubclassMapper.java @@ -0,0 +1,38 @@ +/* + * 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.subclassmapping; + +import org.mapstruct.InheritConfiguration; +import org.mapstruct.Mapper; +import org.mapstruct.Mapping; +import org.mapstruct.ReportingPolicy; +import org.mapstruct.SubclassMapping; +import org.mapstruct.ap.test.subclassmapping.mappables.Bike; +import org.mapstruct.ap.test.subclassmapping.mappables.BikeDto; +import org.mapstruct.ap.test.subclassmapping.mappables.Car; +import org.mapstruct.ap.test.subclassmapping.mappables.CarDto; +import org.mapstruct.ap.test.subclassmapping.mappables.HatchBack; +import org.mapstruct.ap.test.subclassmapping.mappables.Vehicle; +import org.mapstruct.ap.test.subclassmapping.mappables.VehicleDto; +import org.mapstruct.factory.Mappers; + +@Mapper( unmappedTargetPolicy = ReportingPolicy.IGNORE ) +public interface InheritedSubclassMapper { + InheritedSubclassMapper INSTANCE = Mappers.getMapper( InheritedSubclassMapper.class ); + + @SubclassMapping( source = HatchBack.class, target = CarDto.class ) + @SubclassMapping( source = Car.class, target = CarDto.class ) + @SubclassMapping( source = Bike.class, target = BikeDto.class ) + @Mapping( source = "vehicleManufacturingCompany", target = "maker" ) + VehicleDto map(Vehicle vehicle); + + @InheritConfiguration( name = "map" ) + VehicleDto mapInherited(Vehicle dto); + + @SubclassMapping( source = Bike.class, target = CarDto.class ) + @InheritConfiguration( name = "map" ) + VehicleDto mapInheritedOverride(Vehicle dto); +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/subclassmapping/SubclassMappingTest.java b/processor/src/test/java/org/mapstruct/ap/test/subclassmapping/SubclassMappingTest.java index 5a5ce1ef9..ba64d2cc7 100644 --- a/processor/src/test/java/org/mapstruct/ap/test/subclassmapping/SubclassMappingTest.java +++ b/processor/src/test/java/org/mapstruct/ap/test/subclassmapping/SubclassMappingTest.java @@ -130,7 +130,7 @@ public class SubclassMappingTest { HatchBack.class, InverseOrderSubclassMapper.class } ) - void subclassMappingOverridesInverseInheritsMapping() { + void subclassMappingOverridesInverseInheritedMapping() { VehicleCollectionDto vehicleDtos = new VehicleCollectionDto(); CarDto carDto = new CarDto(); carDto.setMaker( "BenZ" ); @@ -143,6 +143,24 @@ public class SubclassMappingTest { .containsExactly( Car.class ); } + @IssueKey( "3125" ) + @ProcessorTest + @WithClasses( { + HatchBack.class, + InheritedSubclassMapper.class + } ) + void subclassMappingOverridesInheritedMapping() { + Vehicle bike = new Bike(); + + VehicleDto result = InheritedSubclassMapper.INSTANCE.map( bike ); + VehicleDto resultInherited = InheritedSubclassMapper.INSTANCE.mapInherited( bike ); + VehicleDto resultOverride = InheritedSubclassMapper.INSTANCE.mapInheritedOverride( bike ); + + assertThat( result ).isInstanceOf( BikeDto.class ); + assertThat( resultInherited ).isInstanceOf( BikeDto.class ); + assertThat( resultOverride ).isInstanceOf( CarDto.class ); + } + @ProcessorTest @WithClasses( { SubclassCompositeMapper.class, CompositeSubclassMappingAnnotation.class }) void mappingIsDoneUsingSubclassMappingWithCompositeMapping() {