From 1c8fff1475829ad7524cf706d1d0b2ab36dc29bc Mon Sep 17 00:00:00 2001 From: Filip Hrisafov Date: Sat, 24 Apr 2021 17:55:25 +0200 Subject: [PATCH] #2423 Use SetterWrapperForCollectionsAndMapsWithNullCheck if the source has a presence check method --- .../model/CollectionAssignmentBuilder.java | 28 ++++++- .../ap/internal/model/PropertyMapping.java | 5 ++ .../spi/CollectionPresenceMapper.java | 52 +++++++++++++ .../spi/CollectionWithNonDirectMapper.java | 77 +++++++++++++++++++ .../CollectionWithNullValueCheckMapper.java | 53 +++++++++++++ .../presencecheck/spi/NonDirectMapper.java | 76 ++++++++++++++++++ .../spi/PresenceCheckMappingTest.java | 70 +++++++++++++++++ .../DomainDtoWithPresenceCheckMapperImpl.java | 8 +- 8 files changed, 365 insertions(+), 4 deletions(-) create mode 100644 processor/src/test/java/org/mapstruct/ap/test/source/presencecheck/spi/CollectionPresenceMapper.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/source/presencecheck/spi/CollectionWithNonDirectMapper.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/source/presencecheck/spi/CollectionWithNullValueCheckMapper.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/source/presencecheck/spi/NonDirectMapper.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/source/presencecheck/spi/PresenceCheckMappingTest.java diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/CollectionAssignmentBuilder.java b/processor/src/main/java/org/mapstruct/ap/internal/model/CollectionAssignmentBuilder.java index 7ab9814e2..dd6db1b0e 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/CollectionAssignmentBuilder.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/CollectionAssignmentBuilder.java @@ -22,6 +22,7 @@ import org.mapstruct.ap.internal.util.Message; import org.mapstruct.ap.internal.util.accessor.Accessor; import org.mapstruct.ap.internal.util.accessor.AccessorType; +import static org.mapstruct.ap.internal.gem.NullValueCheckStrategyGem.ALWAYS; import static org.mapstruct.ap.internal.gem.NullValuePropertyMappingStrategyGem.SET_TO_DEFAULT; import static org.mapstruct.ap.internal.gem.NullValuePropertyMappingStrategyGem.SET_TO_NULL; @@ -168,8 +169,7 @@ public class CollectionAssignmentBuilder { targetAccessorType.isFieldAssignment() ); } - else if ( result.getType() == Assignment.AssignmentType.DIRECT || - nvcs == NullValueCheckStrategyGem.ALWAYS ) { + else if ( setterWrapperNeedsSourceNullCheck( result ) ) { result = new SetterWrapperForCollectionsAndMapsWithNullCheck( result, @@ -212,4 +212,28 @@ public class CollectionAssignmentBuilder { return result; } + /** + * Checks whether the setter wrapper should include a null / presence check or not + * + * @param rhs the source right hand side + * @return whether to include a null / presence check or not + */ + private boolean setterWrapperNeedsSourceNullCheck(Assignment rhs) { + if ( rhs.getSourcePresenceCheckerReference() != null ) { + // If there is a source presence check then we should do a null check + return true; + } + + if ( nvcs == ALWAYS ) { + // NullValueCheckStrategy is ALWAYS -> do a null check + return true; + } + + if ( rhs.getType().isDirect() ) { + return true; + } + + return false; + } + } 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 48ebc34d2..3e53e2ce0 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 @@ -454,6 +454,11 @@ public class PropertyMapping extends ModelElement { return true; } + if ( rhs.getSourcePresenceCheckerReference() != null ) { + // There is an explicit source presence check method -> do a null / presence check + return true; + } + if ( nvpms == SET_TO_DEFAULT || nvpms == IGNORE ) { // NullValuePropertyMapping is SET_TO_DEFAULT or IGNORE -> do a null check return true; diff --git a/processor/src/test/java/org/mapstruct/ap/test/source/presencecheck/spi/CollectionPresenceMapper.java b/processor/src/test/java/org/mapstruct/ap/test/source/presencecheck/spi/CollectionPresenceMapper.java new file mode 100644 index 000000000..f99cae4d3 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/source/presencecheck/spi/CollectionPresenceMapper.java @@ -0,0 +1,52 @@ +/* + * 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.source.presencecheck.spi; + +import java.util.Collection; + +import org.mapstruct.Mapper; +import org.mapstruct.factory.Mappers; + +/** + * @author Filip Hrisafov + */ +@Mapper +public interface CollectionPresenceMapper { + + CollectionPresenceMapper INSTANCE = Mappers.getMapper( CollectionPresenceMapper.class ); + + Target map(Source source); + + class Target { + + private Collection players; + + public Collection getPlayers() { + return players; + } + + public void setPlayers(Collection players) { + this.players = players; + } + } + + class Source { + + private final Collection players; + + public Source(Collection players) { + this.players = players; + } + + public Collection getPlayers() { + return players; + } + + public boolean hasPlayers() { + return players != null && !players.isEmpty(); + } + } +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/source/presencecheck/spi/CollectionWithNonDirectMapper.java b/processor/src/test/java/org/mapstruct/ap/test/source/presencecheck/spi/CollectionWithNonDirectMapper.java new file mode 100644 index 000000000..67daa2211 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/source/presencecheck/spi/CollectionWithNonDirectMapper.java @@ -0,0 +1,77 @@ +/* + * 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.source.presencecheck.spi; + +import java.util.Collection; + +import org.mapstruct.Mapper; +import org.mapstruct.factory.Mappers; + +/** + * @author Filip Hrisafov + */ +@Mapper +public interface CollectionWithNonDirectMapper { + + CollectionWithNonDirectMapper INSTANCE = Mappers.getMapper( CollectionWithNonDirectMapper.class ); + + Target map(Source source); + + class Target { + + private Collection players; + + public Collection getPlayers() { + return players; + } + + public void setPlayers(Collection players) { + this.players = players; + } + } + + class Player { + private String name; + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + } + + class Source { + + private final Collection players; + + public Source(Collection players) { + this.players = players; + } + + public Collection getPlayers() { + return players; + } + + public boolean hasPlayers() { + return players != null && !players.isEmpty(); + } + } + + class PlayerSource { + + private final String name; + + public PlayerSource(String name) { + this.name = name; + } + + public String getName() { + return name; + } + } +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/source/presencecheck/spi/CollectionWithNullValueCheckMapper.java b/processor/src/test/java/org/mapstruct/ap/test/source/presencecheck/spi/CollectionWithNullValueCheckMapper.java new file mode 100644 index 000000000..ef03be20c --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/source/presencecheck/spi/CollectionWithNullValueCheckMapper.java @@ -0,0 +1,53 @@ +/* + * 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.source.presencecheck.spi; + +import java.util.Collection; + +import org.mapstruct.Mapper; +import org.mapstruct.NullValueCheckStrategy; +import org.mapstruct.factory.Mappers; + +/** + * @author Filip Hrisafov + */ +@Mapper(nullValueCheckStrategy = NullValueCheckStrategy.ALWAYS) +public interface CollectionWithNullValueCheckMapper { + + CollectionWithNullValueCheckMapper INSTANCE = Mappers.getMapper( CollectionWithNullValueCheckMapper.class ); + + Target map(Source source); + + class Target { + + private Collection players; + + public Collection getPlayers() { + return players; + } + + public void setPlayers(Collection players) { + this.players = players; + } + } + + class Source { + + private final Collection players; + + public Source(Collection players) { + this.players = players; + } + + public Collection getPlayers() { + return players; + } + + public boolean hasPlayers() { + return players != null && !players.isEmpty(); + } + } +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/source/presencecheck/spi/NonDirectMapper.java b/processor/src/test/java/org/mapstruct/ap/test/source/presencecheck/spi/NonDirectMapper.java new file mode 100644 index 000000000..100ff3937 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/source/presencecheck/spi/NonDirectMapper.java @@ -0,0 +1,76 @@ +/* + * 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.source.presencecheck.spi; + +import org.mapstruct.Mapper; +import org.mapstruct.factory.Mappers; + +/** + * @author Filip Hrisafov + */ +@Mapper +public interface NonDirectMapper { + + NonDirectMapper INSTANCE = Mappers.getMapper( NonDirectMapper.class ); + + SoccerTeam map(SoccerTeamSource source); + + class SoccerTeam { + + private Goalkeeper goalKeeper; + + public Goalkeeper getGoalKeeper() { + return goalKeeper; + } + + public void setGoalKeeper(Goalkeeper goalKeeper) { + this.goalKeeper = goalKeeper; + } + } + + class Goalkeeper { + + private String name; + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + } + + class SoccerTeamSource { + + private final GoalKeeperSource goalKeeper; + + public SoccerTeamSource(GoalKeeperSource goalKeeper) { + this.goalKeeper = goalKeeper; + } + + public GoalKeeperSource getGoalKeeper() { + return goalKeeper; + } + + public boolean hasGoalKeeper() { + return goalKeeper != null && goalKeeper.getName() != null && !goalKeeper.getName().isEmpty(); + } + } + + class GoalKeeperSource { + + private final String name; + + public GoalKeeperSource(String name) { + this.name = name; + } + + public String getName() { + return name; + } + } +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/source/presencecheck/spi/PresenceCheckMappingTest.java b/processor/src/test/java/org/mapstruct/ap/test/source/presencecheck/spi/PresenceCheckMappingTest.java new file mode 100644 index 000000000..c25dad70b --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/source/presencecheck/spi/PresenceCheckMappingTest.java @@ -0,0 +1,70 @@ +/* + * 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.source.presencecheck.spi; + +import java.util.Collections; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mapstruct.ap.testutil.WithClasses; +import org.mapstruct.ap.testutil.runner.AnnotationProcessorTestRunner; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * @author Filip Hrisafov + */ +@RunWith(AnnotationProcessorTestRunner.class) +public class PresenceCheckMappingTest { + + @Test + @WithClasses({ + CollectionPresenceMapper.class + }) + public void collectionPresenceCheckShouldBeUsedByDefault() { + CollectionPresenceMapper.Source source = new CollectionPresenceMapper.Source( Collections.emptyList() ); + CollectionPresenceMapper.Target target = CollectionPresenceMapper.INSTANCE.map( source ); + + assertThat( target.getPlayers() ).isNull(); + } + + @Test + @WithClasses({ + CollectionWithNullValueCheckMapper.class + }) + public void collectionPresenceCheckShouldBeUsedByWhenNullValueCheckIsAlways() { + CollectionWithNullValueCheckMapper.Source source = + new CollectionWithNullValueCheckMapper.Source( Collections.emptyList() ); + CollectionWithNullValueCheckMapper.Target target = + CollectionWithNullValueCheckMapper.INSTANCE.map( source ); + + assertThat( target.getPlayers() ).isNull(); + } + + @Test + @WithClasses({ + CollectionWithNonDirectMapper.class + }) + public void nonDirectCollectionPresenceCheckShouldBeUsedByDefault() { + CollectionWithNonDirectMapper.Source source = + new CollectionWithNonDirectMapper.Source( Collections.emptyList() ); + CollectionWithNonDirectMapper.Target target = CollectionWithNonDirectMapper.INSTANCE.map( source ); + + assertThat( target.getPlayers() ).isNull(); + } + + @Test + @WithClasses({ + NonDirectMapper.class + }) + public void nonDirectMappingWithPresenceCheckShouldBeUsedByDefault() { + NonDirectMapper.SoccerTeamSource source = + new NonDirectMapper.SoccerTeamSource( new NonDirectMapper.GoalKeeperSource( "" ) ); + NonDirectMapper.SoccerTeam target = NonDirectMapper.INSTANCE.map( source ); + + assertThat( target.getGoalKeeper() ).isNull(); + } +} diff --git a/processor/src/test/resources/fixtures/org/mapstruct/ap/test/bugs/_913/DomainDtoWithPresenceCheckMapperImpl.java b/processor/src/test/resources/fixtures/org/mapstruct/ap/test/bugs/_913/DomainDtoWithPresenceCheckMapperImpl.java index ba18dfda1..8cc41c47d 100644 --- a/processor/src/test/resources/fixtures/org/mapstruct/ap/test/bugs/_913/DomainDtoWithPresenceCheckMapperImpl.java +++ b/processor/src/test/resources/fixtures/org/mapstruct/ap/test/bugs/_913/DomainDtoWithPresenceCheckMapperImpl.java @@ -32,12 +32,16 @@ public class DomainDtoWithPresenceCheckMapperImpl implements DomainDtoWithPresen List list = source.getStrings(); domain.setStrings( new LinkedHashSet( list ) ); } - domain.setLongs( stringListToLongSet( source.getStrings() ) ); + if ( source.hasStrings() ) { + domain.setLongs( stringListToLongSet( source.getStrings() ) ); + } if ( source.hasStringsInitialized() ) { List list1 = source.getStringsInitialized(); domain.setStringsInitialized( new LinkedHashSet( list1 ) ); } - domain.setLongsInitialized( stringListToLongSet( source.getStringsInitialized() ) ); + if ( source.hasStringsInitialized() ) { + domain.setLongsInitialized( stringListToLongSet( source.getStringsInitialized() ) ); + } if ( source.hasStringsWithDefault() ) { List list2 = source.getStringsWithDefault(); domain.setStringsWithDefault( new ArrayList( list2 ) );