Skip to content

Commit 51bd43f

Browse files
sjaakdsjaakd
authored
mapstruct#1719 strange error message for selecting collection update methods (mapstruct#1724)
1 parent 002a8b0 commit 51bd43f

File tree

7 files changed

+268
-2
lines changed

7 files changed

+268
-2
lines changed

‎processor/src/main/java/org/mapstruct/ap/internal/model/PropertyMapping.java‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -724,7 +724,13 @@ private Assignment forgeMapMapping(Type sourceType, Type targetType, SourceRHS s
724724
}
725725

726726
private Assignment forgeMapping(SourceRHS sourceRHS) {
727-
Type sourceType = sourceRHS.getSourceType();
727+
Type sourceType;
728+
if ( targetWriteAccessorType == TargetWriteAccessorType.ADDER ) {
729+
sourceType = sourceRHS.getSourceTypeForMatching();
730+
}
731+
else {
732+
sourceType = sourceRHS.getSourceType();
733+
}
728734
if ( forgedNamedBased && !canGenerateAutoSubMappingBetween( sourceType, targetType ) ) {
729735
return null;
730736
}
@@ -744,7 +750,8 @@ private Assignment forgeMapping(SourceRHS sourceRHS) {
744750
// They should forge an update method only if we set the forceUpdateMethod. This is set to true,
745751
// because we are forging a Mapping for a method with multiple source parameters.
746752
// If the target type is enum, then we can't create an update method
747-
if ( !targetType.isEnumType() && ( method.isUpdateMethod() || forceUpdateMethod ) ) {
753+
if ( !targetType.isEnumType() && ( method.isUpdateMethod() || forceUpdateMethod )
754+
&& targetWriteAccessorType != TargetWriteAccessorType.ADDER) {
748755
parameters.add( Parameter.forForgedMappingTarget( targetType ) );
749756
returnType = ctx.getTypeFactory().createVoidType();
750757
}

‎processor/src/test/java/org/mapstruct/ap/test/bugs/_1719/Issue1719Mapper.java‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
/*
2+
* Copyright MapStruct Authors.
3+
*
4+
* Licensed under the Apache License version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0
5+
*/
6+
package org.mapstruct.ap.test.bugs._1719;
7+
8+
import org.mapstruct.CollectionMappingStrategy;
9+
import org.mapstruct.Mapper;
10+
import org.mapstruct.Mapping;
11+
import org.mapstruct.MappingTarget;
12+
import org.mapstruct.factory.Mappers;
13+
14+
@Mapper(collectionMappingStrategy = CollectionMappingStrategy.ADDER_PREFERRED)
15+
public interface Issue1719Mapper {
16+
17+
Issue1719Mapper INSTANCE = Mappers.getMapper( Issue1719Mapper.class );
18+
19+
@Mapping(target = "targetElements", source = "sourceElements")
20+
void map(Source source, @MappingTarget Target target);
21+
22+
}

‎processor/src/test/java/org/mapstruct/ap/test/bugs/_1719/Issue1719Test.java‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
/*
2+
* Copyright MapStruct Authors.
3+
*
4+
* Licensed under the Apache License version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0
5+
*/
6+
package org.mapstruct.ap.test.bugs._1719;
7+
8+
import org.junit.Test;
9+
import org.junit.runner.RunWith;
10+
import org.mapstruct.ap.testutil.IssueKey;
11+
import org.mapstruct.ap.testutil.WithClasses;
12+
import org.mapstruct.ap.testutil.runner.AnnotationProcessorTestRunner;
13+
14+
import static org.assertj.core.api.Assertions.assertThat;
15+
import static org.assertj.core.api.Assertions.tuple;
16+
17+
@RunWith(AnnotationProcessorTestRunner.class)
18+
@IssueKey("1719")
19+
@WithClasses({
20+
Source.class,
21+
SourceElement.class,
22+
Target.class,
23+
TargetElement.class
24+
})
25+
public class Issue1719Test {
26+
27+
/**
28+
* For adder methods MapStuct cannot generate an update method. MapStruct would cannot know how to remove objects
29+
* from the child-parent relation. It cannot even assume that the the collection can be cleared at forehand.
30+
* Therefore the only sensible choice is for MapStruct to create a create method for the target elements.
31+
*/
32+
@Test
33+
@WithClasses(Issue1719Mapper.class)
34+
public void testShouldGiveNoErrorMessage() {
35+
Source source = new Source();
36+
source.getSourceElements().add( new SourceElement( 1, "jim" ) );
37+
source.getSourceElements().add( new SourceElement( 2, "alice" ) );
38+
39+
Target target = new Target();
40+
TargetElement bob = new TargetElement( 1, "bob" );
41+
target.addTargetElement( bob );
42+
TargetElement louise = new TargetElement( 3, "louise" );
43+
target.addTargetElement( louise );
44+
45+
Issue1719Mapper.INSTANCE.map( source, target );
46+
47+
assertThat( target.getTargetElements() ).hasSize( 3 );
48+
assertThat( target.getTargetElements() )
49+
.extracting( TargetElement::getId, TargetElement::getName )
50+
.containsOnly(
51+
tuple( 1, "bob" ),
52+
tuple( 2, "alice" ),
53+
tuple( 3, "louise" )
54+
);
55+
}
56+
57+
}

‎processor/src/test/java/org/mapstruct/ap/test/bugs/_1719/Source.java‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
/*
2+
* Copyright MapStruct Authors.
3+
*
4+
* Licensed under the Apache License version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0
5+
*/
6+
package org.mapstruct.ap.test.bugs._1719;
7+
8+
import java.util.HashSet;
9+
import java.util.Set;
10+
11+
public class Source {
12+
13+
private Set<SourceElement> sourceElements = new HashSet<>();
14+
15+
public Set<SourceElement> getSourceElements() {
16+
return sourceElements;
17+
}
18+
19+
public void setSourceElements(Set<SourceElement> sourceElements) {
20+
this.sourceElements = sourceElements;
21+
}
22+
23+
}

‎processor/src/test/java/org/mapstruct/ap/test/bugs/_1719/SourceElement.java‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
/*
2+
* Copyright MapStruct Authors.
3+
*
4+
* Licensed under the Apache License version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0
5+
*/
6+
package org.mapstruct.ap.test.bugs._1719;
7+
8+
import java.util.Objects;
9+
10+
public class SourceElement {
11+
12+
private int id;
13+
private String name;
14+
15+
public SourceElement(int id, String name) {
16+
this.id = id;
17+
this.name = name;
18+
}
19+
20+
public int getId() {
21+
return id;
22+
}
23+
24+
public void setId(int id) {
25+
this.id = id;
26+
}
27+
28+
public String getName() {
29+
return name;
30+
}
31+
32+
public void setName(String name) {
33+
this.name = name;
34+
}
35+
36+
@Override
37+
public boolean equals(Object o) {
38+
if ( this == o ) {
39+
return true;
40+
}
41+
if ( o == null || getClass() != o.getClass() ) {
42+
return false;
43+
}
44+
SourceElement that = (SourceElement) o;
45+
return id == that.id;
46+
}
47+
48+
@Override
49+
public int hashCode() {
50+
return Objects.hash( id );
51+
}
52+
}

‎processor/src/test/java/org/mapstruct/ap/test/bugs/_1719/Target.java‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
/*
2+
* Copyright MapStruct Authors.
3+
*
4+
* Licensed under the Apache License version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0
5+
*/
6+
package org.mapstruct.ap.test.bugs._1719;
7+
8+
import java.util.HashSet;
9+
import java.util.Set;
10+
11+
public class Target {
12+
13+
private Set<TargetElement> targetElements = new HashSet<>();
14+
15+
public Set<TargetElement> getTargetElements() {
16+
return targetElements;
17+
}
18+
19+
public void setTargetElements(Set<TargetElement> targetElements) {
20+
this.targetElements = targetElements;
21+
}
22+
23+
public TargetElement addTargetElement(TargetElement element) {
24+
element.updateTarget( this );
25+
getTargetElements().add( element );
26+
return element;
27+
}
28+
29+
public TargetElement removeTargetElement(TargetElement element) {
30+
element.updateTarget( null );
31+
getTargetElements().remove( element );
32+
return element;
33+
}
34+
35+
}

‎processor/src/test/java/org/mapstruct/ap/test/bugs/_1719/TargetElement.java‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
/*
2+
* Copyright MapStruct Authors.
3+
*
4+
* Licensed under the Apache License version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0
5+
*/
6+
package org.mapstruct.ap.test.bugs._1719;
7+
8+
import java.util.Objects;
9+
10+
public class TargetElement {
11+
12+
private int id;
13+
private String name;
14+
private Target target;
15+
16+
public TargetElement() {
17+
}
18+
19+
public TargetElement(int id, String name) {
20+
this.id = id;
21+
this.name = name;
22+
}
23+
24+
public int getId() {
25+
return id;
26+
}
27+
28+
public void setId(int id) {
29+
this.id = id;
30+
}
31+
32+
public String getName() {
33+
return name;
34+
}
35+
36+
public void setName(String name) {
37+
this.name = name;
38+
}
39+
40+
public Target getTarget() {
41+
return target;
42+
}
43+
44+
/**
45+
* intentionally not a setter, to not further complicate this test case.
46+
*
47+
* @param target
48+
*/
49+
public void updateTarget(Target target) {
50+
this.target = target;
51+
}
52+
53+
@Override
54+
public boolean equals(Object o) {
55+
if ( this == o ) {
56+
return true;
57+
}
58+
if ( o == null || getClass() != o.getClass() ) {
59+
return false;
60+
}
61+
TargetElement that = (TargetElement) o;
62+
return id == that.id;
63+
}
64+
65+
@Override
66+
public int hashCode() {
67+
return Objects.hash( id );
68+
}
69+
70+
}

0 commit comments

Comments
 (0)