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 5dffad9d8..229150403 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 @@ -724,7 +724,13 @@ public class PropertyMapping extends ModelElement { } private Assignment forgeMapping(SourceRHS sourceRHS) { - Type sourceType = sourceRHS.getSourceType(); + Type sourceType; + if ( targetWriteAccessorType == TargetWriteAccessorType.ADDER ) { + sourceType = sourceRHS.getSourceTypeForMatching(); + } + else { + sourceType = sourceRHS.getSourceType(); + } if ( forgedNamedBased && !canGenerateAutoSubMappingBetween( sourceType, targetType ) ) { return null; } @@ -744,7 +750,8 @@ public class PropertyMapping extends ModelElement { // They should forge an update method only if we set the forceUpdateMethod. This is set to true, // because we are forging a Mapping for a method with multiple source parameters. // If the target type is enum, then we can't create an update method - if ( !targetType.isEnumType() && ( method.isUpdateMethod() || forceUpdateMethod ) ) { + if ( !targetType.isEnumType() && ( method.isUpdateMethod() || forceUpdateMethod ) + && targetWriteAccessorType != TargetWriteAccessorType.ADDER) { parameters.add( Parameter.forForgedMappingTarget( targetType ) ); returnType = ctx.getTypeFactory().createVoidType(); } diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_1719/Issue1719Mapper.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1719/Issue1719Mapper.java new file mode 100644 index 000000000..f4ac6978f --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1719/Issue1719Mapper.java @@ -0,0 +1,22 @@ +/* + * 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._1719; + +import org.mapstruct.CollectionMappingStrategy; +import org.mapstruct.Mapper; +import org.mapstruct.Mapping; +import org.mapstruct.MappingTarget; +import org.mapstruct.factory.Mappers; + +@Mapper(collectionMappingStrategy = CollectionMappingStrategy.ADDER_PREFERRED) +public interface Issue1719Mapper { + + Issue1719Mapper INSTANCE = Mappers.getMapper( Issue1719Mapper.class ); + + @Mapping(target = "targetElements", source = "sourceElements") + void map(Source source, @MappingTarget Target target); + +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_1719/Issue1719Test.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1719/Issue1719Test.java new file mode 100644 index 000000000..e34c79032 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1719/Issue1719Test.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._1719; + +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; +import static org.assertj.core.api.Assertions.tuple; + +@RunWith(AnnotationProcessorTestRunner.class) +@IssueKey("1719") +@WithClasses({ + Source.class, + SourceElement.class, + Target.class, + TargetElement.class +}) +public class Issue1719Test { + + /** + * For adder methods MapStuct cannot generate an update method. MapStruct would cannot know how to remove objects + * from the child-parent relation. It cannot even assume that the the collection can be cleared at forehand. + * Therefore the only sensible choice is for MapStruct to create a create method for the target elements. + */ + @Test + @WithClasses(Issue1719Mapper.class) + public void testShouldGiveNoErrorMessage() { + Source source = new Source(); + source.getSourceElements().add( new SourceElement( 1, "jim" ) ); + source.getSourceElements().add( new SourceElement( 2, "alice" ) ); + + Target target = new Target(); + TargetElement bob = new TargetElement( 1, "bob" ); + target.addTargetElement( bob ); + TargetElement louise = new TargetElement( 3, "louise" ); + target.addTargetElement( louise ); + + Issue1719Mapper.INSTANCE.map( source, target ); + + assertThat( target.getTargetElements() ).hasSize( 3 ); + assertThat( target.getTargetElements() ) + .extracting( TargetElement::getId, TargetElement::getName ) + .containsOnly( + tuple( 1, "bob" ), + tuple( 2, "alice" ), + tuple( 3, "louise" ) + ); + } + +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_1719/Source.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1719/Source.java new file mode 100644 index 000000000..f3e6b1ea2 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1719/Source.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.bugs._1719; + +import java.util.HashSet; +import java.util.Set; + +public class Source { + + private Set sourceElements = new HashSet<>(); + + public Set getSourceElements() { + return sourceElements; + } + + public void setSourceElements(Set sourceElements) { + this.sourceElements = sourceElements; + } + +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_1719/SourceElement.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1719/SourceElement.java new file mode 100644 index 000000000..fbfe00e82 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1719/SourceElement.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.bugs._1719; + +import java.util.Objects; + +public class SourceElement { + + private int id; + private String name; + + public SourceElement(int id, String name) { + this.id = id; + this.name = name; + } + + public int getId() { + return id; + } + + public void setId(int id) { + this.id = id; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + @Override + public boolean equals(Object o) { + if ( this == o ) { + return true; + } + if ( o == null || getClass() != o.getClass() ) { + return false; + } + SourceElement that = (SourceElement) o; + return id == that.id; + } + + @Override + public int hashCode() { + return Objects.hash( id ); + } +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_1719/Target.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1719/Target.java new file mode 100644 index 000000000..9d14a9a14 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1719/Target.java @@ -0,0 +1,35 @@ +/* + * 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._1719; + +import java.util.HashSet; +import java.util.Set; + +public class Target { + + private Set targetElements = new HashSet<>(); + + public Set getTargetElements() { + return targetElements; + } + + public void setTargetElements(Set targetElements) { + this.targetElements = targetElements; + } + + public TargetElement addTargetElement(TargetElement element) { + element.updateTarget( this ); + getTargetElements().add( element ); + return element; + } + + public TargetElement removeTargetElement(TargetElement element) { + element.updateTarget( null ); + getTargetElements().remove( element ); + return element; + } + +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_1719/TargetElement.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1719/TargetElement.java new file mode 100644 index 000000000..92d1970af --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1719/TargetElement.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.bugs._1719; + +import java.util.Objects; + +public class TargetElement { + + private int id; + private String name; + private Target target; + + public TargetElement() { + } + + public TargetElement(int id, String name) { + this.id = id; + this.name = name; + } + + public int getId() { + return id; + } + + public void setId(int id) { + this.id = id; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + public Target getTarget() { + return target; + } + + /** + * intentionally not a setter, to not further complicate this test case. + * + * @param target + */ + public void updateTarget(Target target) { + this.target = target; + } + + @Override + public boolean equals(Object o) { + if ( this == o ) { + return true; + } + if ( o == null || getClass() != o.getClass() ) { + return false; + } + TargetElement that = (TargetElement) o; + return id == that.id; + } + + @Override + public int hashCode() { + return Objects.hash( id ); + } + +}