From 6102d0cc8ee1a62e26462ad78d5cf4fd507a3139 Mon Sep 17 00:00:00 2001 From: Filip Hrisafov Date: Sun, 18 Oct 2020 16:11:18 +0200 Subject: [PATCH] #2236 Different nested target mappings should generate different intermediate methods Make sure that MappingReferences are taken into consideration when comparing whether 2 mapping methods are equal. This makes sure that when using nested target mappings that have the same property mappings, but different mappings 2 distinct methods will be created --- .../ap/internal/model/BeanMappingMethod.java | 23 +++++++- .../model/beanmapping/MappingReferences.java | 33 +++++++++++ .../org/mapstruct/ap/test/bugs/_2236/Car.java | 49 ++++++++++++++++ .../mapstruct/ap/test/bugs/_2236/CarDto.java | 58 +++++++++++++++++++ .../ap/test/bugs/_2236/CarMapper.java | 27 +++++++++ .../ap/test/bugs/_2236/Issue2236Test.java | 57 ++++++++++++++++++ .../mapstruct/ap/test/bugs/_2236/Owner.java | 31 ++++++++++ .../mapstruct/ap/test/bugs/_2236/Vehicle.java | 30 ++++++++++ .../nestedbeans/mixed/FishTankMapperImpl.java | 41 ++++++++++++- 9 files changed, 344 insertions(+), 5 deletions(-) create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_2236/Car.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_2236/CarDto.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_2236/CarMapper.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_2236/Issue2236Test.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_2236/Owner.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_2236/Vehicle.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 87b8a28b4..4d086b512 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 @@ -85,6 +85,8 @@ public class BeanMappingMethod extends NormalTypeMappingMethod { private final BuilderType returnTypeBuilder; private final MethodReference finalizerMethod; + private final MappingReferences mappingReferences; + public static class Builder { private MappingBuilderContext ctx; @@ -337,7 +339,8 @@ public class BeanMappingMethod extends NormalTypeMappingMethod { returnTypeBuilder, beforeMappingMethods, afterMappingMethods, - finalizeMethod + finalizeMethod, + mappingReferences ); } @@ -1488,6 +1491,7 @@ public class BeanMappingMethod extends NormalTypeMappingMethod { } } + //CHECKSTYLE:OFF private BeanMappingMethod(Method method, Collection existingVariableNames, List propertyMappings, @@ -1497,7 +1501,8 @@ public class BeanMappingMethod extends NormalTypeMappingMethod { BuilderType returnTypeBuilder, List beforeMappingReferences, List afterMappingReferences, - MethodReference finalizerMethod) { + MethodReference finalizerMethod, + MappingReferences mappingReferences) { super( method, existingVariableNames, @@ -1506,10 +1511,12 @@ public class BeanMappingMethod extends NormalTypeMappingMethod { beforeMappingReferences, afterMappingReferences ); + //CHECKSTYLE:ON this.propertyMappings = propertyMappings; this.returnTypeBuilder = returnTypeBuilder; this.finalizerMethod = finalizerMethod; + this.mappingReferences = mappingReferences; // intialize constant mappings as all mappings, but take out the ones that can be contributed to a // parameter mapping. @@ -1628,7 +1635,17 @@ public class BeanMappingMethod extends NormalTypeMappingMethod { if ( !super.equals( obj ) ) { return false; } - return Objects.equals( propertyMappings, that.propertyMappings ); + + if ( !Objects.equals( propertyMappings, that.propertyMappings ) ) { + return false; + } + + if ( !Objects.equals( mappingReferences, that.mappingReferences ) ) { + return false; + } + + + return true; } } diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/beanmapping/MappingReferences.java b/processor/src/main/java/org/mapstruct/ap/internal/model/beanmapping/MappingReferences.java index 06f0ed06c..569c75e0e 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/beanmapping/MappingReferences.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/beanmapping/MappingReferences.java @@ -9,6 +9,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.LinkedHashSet; import java.util.List; +import java.util.Objects; import java.util.Set; import org.mapstruct.ap.internal.model.common.Type; @@ -139,6 +140,38 @@ public class MappingReferences { return targetThisReferences; } + @Override + public boolean equals(Object o) { + if ( this == o ) { + return true; + } + if ( !( o instanceof MappingReferences ) ) { + return false; + } + + MappingReferences that = (MappingReferences) o; + + if ( restrictToDefinedMappings != that.restrictToDefinedMappings ) { + return false; + } + if ( forForgedMethods != that.forForgedMethods ) { + return false; + } + if ( !Objects.equals( mappingReferences, that.mappingReferences ) ) { + return false; + } + if ( Objects.equals( targetThisReferences, that.targetThisReferences ) ) { + return false; + } + + return true; + } + + @Override + public int hashCode() { + return mappingReferences != null ? mappingReferences.hashCode() : 0; + } + /** * MapStruct filters automatically inversed invalid methods out. TODO: this is a principle we should discuss! * @param mappingRef diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_2236/Car.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_2236/Car.java new file mode 100644 index 000000000..70bb9a413 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_2236/Car.java @@ -0,0 +1,49 @@ +/* + * 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._2236; + +/** + * @author Filip Hrisafov + */ +public class Car { + + private String name; + private String type; + private Owner ownerA; + private Owner ownerB; + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + public String getType() { + return type; + } + + public void setType(String type) { + this.type = type; + } + + public Owner getOwnerA() { + return ownerA; + } + + public void setOwnerA(Owner ownerA) { + this.ownerA = ownerA; + } + + public Owner getOwnerB() { + return ownerB; + } + + public void setOwnerB(Owner ownerB) { + this.ownerB = ownerB; + } +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_2236/CarDto.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_2236/CarDto.java new file mode 100644 index 000000000..909bb3649 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_2236/CarDto.java @@ -0,0 +1,58 @@ +/* + * 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._2236; + +/** + * @author Filip Hrisafov + */ +public class CarDto { + + private String name; + private String ownerNameA; + private String ownerNameB; + private String ownerCityA; + private String ownerCityB; + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + public String getOwnerNameA() { + return ownerNameA; + } + + public void setOwnerNameA(String ownerNameA) { + this.ownerNameA = ownerNameA; + } + + public String getOwnerNameB() { + return ownerNameB; + } + + public void setOwnerNameB(String ownerNameB) { + this.ownerNameB = ownerNameB; + } + + public String getOwnerCityA() { + return ownerCityA; + } + + public void setOwnerCityA(String ownerCityA) { + this.ownerCityA = ownerCityA; + } + + public String getOwnerCityB() { + return ownerCityB; + } + + public void setOwnerCityB(String ownerCityB) { + this.ownerCityB = ownerCityB; + } +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_2236/CarMapper.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_2236/CarMapper.java new file mode 100644 index 000000000..d3366ea2a --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_2236/CarMapper.java @@ -0,0 +1,27 @@ +/* + * 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._2236; + +import org.mapstruct.Mapper; +import org.mapstruct.Mapping; +import org.mapstruct.factory.Mappers; + +/** + * @author Filip Hrisafov + */ +@Mapper +public interface CarMapper { + + CarMapper INSTANCE = Mappers.getMapper( CarMapper.class ); + + @Mapping(target = "ownerA.name", source = "carDto.ownerNameA") + @Mapping(target = "ownerA.city", source = "carDto.ownerCityA") + @Mapping(target = "ownerB.name", source = "carDto.ownerNameB") + @Mapping(target = "ownerB.city", source = "carDto.ownerCityB") + @Mapping(target = "name", source = "carDto.name") + @Mapping(target = "type", source = "type") + Car vehicleToCar(Vehicle vehicle); +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_2236/Issue2236Test.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_2236/Issue2236Test.java new file mode 100644 index 000000000..17f42c8dc --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_2236/Issue2236Test.java @@ -0,0 +1,57 @@ +/* + * 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._2236; + +import org.junit.Test; +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 static org.assertj.core.api.Assertions.assertThat; + +/** + * @author Filip Hrisafov + */ +@IssueKey("2236") +@RunWith(AnnotationProcessorTestRunner.class) +@WithClasses({ + Car.class, + CarDto.class, + CarMapper.class, + Owner.class, + Vehicle.class, +}) +public class Issue2236Test { + + @Test + public void shouldCorrectlyMapSameTypesWithDifferentNestedMappings() { + + Vehicle vehicle = new Vehicle(); + vehicle.setType( "Sedan" ); + CarDto carDto = new CarDto(); + vehicle.setCarDto( carDto ); + + carDto.setName( "Private car" ); + carDto.setOwnerNameA( "Owner A" ); + carDto.setOwnerCityA( "Zurich" ); + + carDto.setOwnerNameB( "Owner B" ); + carDto.setOwnerCityB( "Bern" ); + + Car car = CarMapper.INSTANCE.vehicleToCar( vehicle ); + + assertThat( car ).isNotNull(); + assertThat( car.getType() ).isEqualTo( "Sedan" ); + assertThat( car.getName() ).isEqualTo( "Private car" ); + assertThat( car.getOwnerA() ).isNotNull(); + assertThat( car.getOwnerA().getName() ).isEqualTo( "Owner A" ); + assertThat( car.getOwnerA().getCity() ).isEqualTo( "Zurich" ); + assertThat( car.getOwnerB() ).isNotNull(); + assertThat( car.getOwnerB().getName() ).isEqualTo( "Owner B" ); + assertThat( car.getOwnerB().getCity() ).isEqualTo( "Bern" ); + } +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_2236/Owner.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_2236/Owner.java new file mode 100644 index 000000000..5f200f527 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_2236/Owner.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.bugs._2236; + +/** + * @author Filip Hrisafov + */ +public class Owner { + + private String name; + private String city; + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + public String getCity() { + return city; + } + + public void setCity(String city) { + this.city = city; + } +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_2236/Vehicle.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_2236/Vehicle.java new file mode 100644 index 000000000..fc8e744e8 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_2236/Vehicle.java @@ -0,0 +1,30 @@ +/* + * 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._2236; + +/** + * @author Filip Hrisafov + */ +public class Vehicle { + private CarDto carDto; + private String type; + + public CarDto getCarDto() { + return carDto; + } + + public void setCarDto(CarDto carDto) { + this.carDto = carDto; + } + + public String getType() { + return type; + } + + public void setType(String type) { + this.type = type; + } +} diff --git a/processor/src/test/resources/fixtures/org/mapstruct/ap/test/nestedbeans/mixed/FishTankMapperImpl.java b/processor/src/test/resources/fixtures/org/mapstruct/ap/test/nestedbeans/mixed/FishTankMapperImpl.java index 2b81718db..3642d1534 100644 --- a/processor/src/test/resources/fixtures/org/mapstruct/ap/test/nestedbeans/mixed/FishTankMapperImpl.java +++ b/processor/src/test/resources/fixtures/org/mapstruct/ap/test/nestedbeans/mixed/FishTankMapperImpl.java @@ -57,9 +57,9 @@ public class FishTankMapperImpl implements FishTankMapper { FishTankDto fishTankDto = new FishTankDto(); - fishTankDto.setFish( fishToFishDto( source.getFish() ) ); + fishTankDto.setFish( fishToFishDto1( source.getFish() ) ); fishTankDto.setMaterial( fishTankToMaterialDto1( source ) ); - fishTankDto.setQuality( waterQualityToWaterQualityDto( source.getQuality() ) ); + fishTankDto.setQuality( waterQualityToWaterQualityDto1( source.getQuality() ) ); fishTankDto.setOrnament( ornamentToOrnamentDto( sourceInteriorOrnament( source ) ) ); fishTankDto.setPlant( waterPlantToWaterPlantDto( source.getPlant() ) ); fishTankDto.setName( source.getName() ); @@ -197,6 +197,18 @@ public class FishTankMapperImpl implements FishTankMapper { return waterPlantDto; } + protected FishDto fishToFishDto1(Fish fish) { + if ( fish == null ) { + return null; + } + + FishDto fishDto = new FishDto(); + + fishDto.setKind( fish.getType() ); + + return fishDto; + } + protected MaterialDto fishTankToMaterialDto1(FishTank fishTank) { if ( fishTank == null ) { return null; @@ -221,6 +233,31 @@ public class FishTankMapperImpl implements FishTankMapper { return waterQualityOrganisationDto; } + protected WaterQualityReportDto waterQualityReportToWaterQualityReportDto1(WaterQualityReport waterQualityReport) { + if ( waterQualityReport == null ) { + return null; + } + + WaterQualityReportDto waterQualityReportDto = new WaterQualityReportDto(); + + waterQualityReportDto.setOrganisation( waterQualityReportToWaterQualityOrganisationDto1( waterQualityReport ) ); + waterQualityReportDto.setVerdict( waterQualityReport.getVerdict() ); + + return waterQualityReportDto; + } + + protected WaterQualityDto waterQualityToWaterQualityDto1(WaterQuality waterQuality) { + if ( waterQuality == null ) { + return null; + } + + WaterQualityDto waterQualityDto = new WaterQualityDto(); + + waterQualityDto.setReport( waterQualityReportToWaterQualityReportDto1( waterQuality.getReport() ) ); + + return waterQualityDto; + } + protected Fish fishDtoToFish(FishDto fishDto) { if ( fishDto == null ) { return null;