Skip to content

Commit 8fc97f5

Browse files
tangyang9464tangyang9464
authored
mapstruct#3806: Properly apply NullValuePropertyMappingStrategy.IGNORE for collections / maps without setters
Signed-off-by: TangYang <[email protected]>
1 parent 5464c3c commit 8fc97f5

5 files changed

Lines changed: 171 additions & 1 deletion

File tree

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,7 @@ else if ( hasNoArgsConstructor() ) {
240240
result,
241241
method.getThrownTypes(),
242242
targetType,
243+
nvpms,
243244
targetAccessorType.isFieldAssignment()
244245
);
245246
}

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,12 @@
99
import java.util.List;
1010
import java.util.Set;
1111

12+
import org.mapstruct.ap.internal.gem.NullValuePropertyMappingStrategyGem;
1213
import org.mapstruct.ap.internal.model.common.Assignment;
1314
import org.mapstruct.ap.internal.model.common.Type;
1415

16+
import static org.mapstruct.ap.internal.gem.NullValuePropertyMappingStrategyGem.IGNORE;
17+
1518
/**
1619
* This wrapper handles the situation were an assignment must be done via a target getter method because there
1720
* is no setter available.
@@ -26,6 +29,14 @@
2629
* @author Sjaak Derksen
2730
*/
2831
public class GetterWrapperForCollectionsAndMaps extends WrapperForCollectionsAndMaps {
32+
private final boolean ignoreMapNull;
33+
34+
public GetterWrapperForCollectionsAndMaps(Assignment decoratedAssignment,
35+
List<Type> thrownTypesToExclude,
36+
Type targetType,
37+
boolean fieldAssignment) {
38+
this( decoratedAssignment, thrownTypesToExclude, targetType, null, fieldAssignment );
39+
}
2940

3041
/**
3142
* @param decoratedAssignment source RHS
@@ -36,6 +47,7 @@ public class GetterWrapperForCollectionsAndMaps extends WrapperForCollectionsAnd
3647
public GetterWrapperForCollectionsAndMaps(Assignment decoratedAssignment,
3748
List<Type> thrownTypesToExclude,
3849
Type targetType,
50+
NullValuePropertyMappingStrategyGem nvpms,
3951
boolean fieldAssignment) {
4052

4153
super(
@@ -44,6 +56,7 @@ public GetterWrapperForCollectionsAndMaps(Assignment decoratedAssignment,
4456
targetType,
4557
fieldAssignment
4658
);
59+
this.ignoreMapNull = nvpms == IGNORE;
4760
}
4861

4962
@Override
@@ -54,4 +67,8 @@ public Set<Type> getImportTypes() {
5467
}
5568
return imported;
5669
}
70+
71+
public boolean isIgnoreMapNull() {
72+
return ignoreMapNull;
73+
}
5774
}

‎processor/src/main/resources/org/mapstruct/ap/internal/model/assignment/GetterWrapperForCollectionsAndMaps.ftl‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,13 @@
1010
<@lib.sourceLocalVarAssignment/>
1111
if ( ${ext.targetBeanName}.${ext.targetWriteAccessorName}<@lib.handleWriteAccesing /> != null ) {
1212
<@lib.handleExceptions>
13-
<#if ext.existingInstanceMapping>
13+
<#if ext.existingInstanceMapping && !ignoreMapNull>
1414
${ext.targetBeanName}.${ext.targetWriteAccessorName}<@lib.handleWriteAccesing />.clear();
1515
</#if>
1616
<@lib.handleLocalVarNullCheck needs_explicit_local_var=false>
17+
<#if ext.existingInstanceMapping && ignoreMapNull>
18+
${ext.targetBeanName}.${ext.targetWriteAccessorName}<@lib.handleWriteAccesing />.clear();
19+
</#if>
1720
${ext.targetBeanName}.${ext.targetWriteAccessorName}<@lib.handleWriteAccesing />.<#if ext.targetType.collectionType>addAll<#else>putAll</#if>( <@lib.handleWithAssignmentOrNullCheckVar/> );
1821
</@lib.handleLocalVarNullCheck>
1922
</@lib.handleExceptions>

‎processor/src/test/java/org/mapstruct/ap/test/bugs/_3806/Issue3806Mapper.java‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
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._3806;
7+
8+
import java.util.ArrayList;
9+
import java.util.Collection;
10+
import java.util.HashMap;
11+
import java.util.Map;
12+
13+
import org.mapstruct.Mapper;
14+
import org.mapstruct.MappingTarget;
15+
import org.mapstruct.NullValuePropertyMappingStrategy;
16+
import org.mapstruct.factory.Mappers;
17+
18+
@Mapper(nullValuePropertyMappingStrategy = NullValuePropertyMappingStrategy.IGNORE)
19+
public interface Issue3806Mapper {
20+
21+
Issue3806Mapper INSTANCE = Mappers.getMapper( Issue3806Mapper.class );
22+
23+
void update(@MappingTarget Target target, Target source);
24+
25+
class Target {
26+
27+
private final Collection<String> authors;
28+
private final Map<String, String> booksByAuthor;
29+
30+
protected Collection<String> books;
31+
protected Map<String, String> booksByPublisher;
32+
33+
public Target(Collection<String> authors, Map<String, String> booksByAuthor) {
34+
this.authors = authors != null ? new ArrayList<>( authors ) : null;
35+
this.booksByAuthor = booksByAuthor != null ? new HashMap<>( booksByAuthor ) : null;
36+
}
37+
38+
public Collection<String> getAuthors() {
39+
return authors;
40+
}
41+
42+
public Map<String, String> getBooksByAuthor() {
43+
return booksByAuthor;
44+
}
45+
46+
public Collection<String> getBooks() {
47+
return books;
48+
}
49+
50+
public void setBooks(Collection<String> books) {
51+
this.books = books;
52+
}
53+
54+
public Map<String, String> getBooksByPublisher() {
55+
return booksByPublisher;
56+
}
57+
58+
public void setBooksByPublisher(Map<String, String> booksByPublisher) {
59+
this.booksByPublisher = booksByPublisher;
60+
}
61+
}
62+
63+
}

‎processor/src/test/java/org/mapstruct/ap/test/bugs/_3806/Issue3806Test.java‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
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._3806;
7+
8+
import java.util.ArrayList;
9+
import java.util.HashMap;
10+
import java.util.List;
11+
import java.util.Map;
12+
13+
import org.mapstruct.ap.testutil.IssueKey;
14+
import org.mapstruct.ap.testutil.ProcessorTest;
15+
import org.mapstruct.ap.testutil.WithClasses;
16+
17+
import static org.assertj.core.api.Assertions.assertThat;
18+
import static org.assertj.core.api.Assertions.entry;
19+
20+
@WithClasses(Issue3806Mapper.class)
21+
@IssueKey("3806")
22+
class Issue3806Test {
23+
24+
@ProcessorTest
25+
void shouldNotClearGetterOnlyCollectionsInUpdateMapping() {
26+
Map<String, String> booksByAuthor = new HashMap<>();
27+
booksByAuthor.put( "author1", "book1" );
28+
booksByAuthor.put( "author2", "book2" );
29+
List<String> authors = new ArrayList<>();
30+
authors.add( "author1" );
31+
authors.add( "author2" );
32+
33+
List<String> books = new ArrayList<>();
34+
books.add( "book1" );
35+
books.add( "book2" );
36+
Map<String, String> booksByPublisher = new HashMap<>();
37+
booksByPublisher.put( "publisher1", "book1" );
38+
booksByPublisher.put( "publisher2", "book2" );
39+
Issue3806Mapper.Target target = new Issue3806Mapper.Target( authors, booksByAuthor );
40+
target.setBooks( books );
41+
target.setBooksByPublisher( booksByPublisher );
42+
43+
Issue3806Mapper.Target source = new Issue3806Mapper.Target( null, null );
44+
Issue3806Mapper.INSTANCE.update( target, source );
45+
46+
assertThat( target.getAuthors() ).containsExactly( "author1", "author2" );
47+
assertThat( target.getBooksByAuthor() )
48+
.containsOnly(
49+
entry( "author1", "book1" ),
50+
entry( "author2", "book2" )
51+
);
52+
53+
assertThat( target.getBooks() ).containsExactly( "book1", "book2" );
54+
assertThat( target.getBooksByPublisher() )
55+
.containsOnly(
56+
entry( "publisher1", "book1" ),
57+
entry( "publisher2", "book2" )
58+
);
59+
60+
booksByAuthor = new HashMap<>();
61+
booksByAuthor.put( "author3", "book3" );
62+
authors = new ArrayList<>();
63+
authors.add( "author3" );
64+
65+
books = new ArrayList<>();
66+
books.add( "book3" );
67+
booksByPublisher = new HashMap<>();
68+
booksByPublisher.put( "publisher3", "book3" );
69+
source = new Issue3806Mapper.Target( authors, booksByAuthor );
70+
source.setBooks( books );
71+
source.setBooksByPublisher( booksByPublisher );
72+
Issue3806Mapper.INSTANCE.update( target, source );
73+
74+
assertThat( target.getAuthors() ).containsExactly( "author3" );
75+
assertThat( target.getBooksByAuthor() )
76+
.containsOnly(
77+
entry( "author3", "book3" )
78+
);
79+
80+
assertThat( target.getBooks() ).containsExactly( "book3" );
81+
assertThat( target.getBooksByPublisher() )
82+
.containsOnly(
83+
entry( "publisher3", "book3" )
84+
);
85+
}
86+
}

0 commit comments

Comments
 (0)