From 48181ae09e95be4e84c4e03c6e98b2a105c7bedd Mon Sep 17 00:00:00 2001 From: sjaakd Date: Sun, 13 Nov 2016 20:30:59 +0100 Subject: [PATCH 1/3] #913 New property mapping collection handling, unit test extension --- ...ssue913SetterMapperForCollectionsTest.java | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_913/Issue913SetterMapperForCollectionsTest.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_913/Issue913SetterMapperForCollectionsTest.java index 011a31219..53fd509fb 100644 --- a/processor/src/test/java/org/mapstruct/ap/test/bugs/_913/Issue913SetterMapperForCollectionsTest.java +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_913/Issue913SetterMapperForCollectionsTest.java @@ -18,6 +18,8 @@ */ package org.mapstruct.ap.test.bugs._913; +import java.util.HashSet; +import java.util.Set; import static org.assertj.core.api.Assertions.assertThat; import org.junit.Test; import org.junit.runner.RunWith; @@ -70,6 +72,8 @@ public class Issue913SetterMapperForCollectionsTest { Dto dto = new Dto(); Domain domain = new Domain(); + domain.setLongs( new HashSet() ); + domain.setStrings( new HashSet() ); DomainDtoWithNvmsNullMapper.INSTANCE.update( dto, domain ); doControlAsserts( domain ); @@ -78,6 +82,7 @@ public class Issue913SetterMapperForCollectionsTest { } + /** * The null value mapping strategy on type level (Mapper) should generate forged methods for the * conversion from string to long that return null in the entire mapper, so also for the forged @@ -92,6 +97,8 @@ public class Issue913SetterMapperForCollectionsTest { Dto dto = new Dto(); dto.setStringsInitialized( null ); Domain domain = new Domain(); + domain.setLongs( new HashSet() ); + domain.setStrings( new HashSet() ); DomainDtoWithNvmsNullMapper.INSTANCE.update( dto, domain ); assertThat( domain.getStringsInitialized() ).isNull(); @@ -112,6 +119,8 @@ public class Issue913SetterMapperForCollectionsTest { Dto dto = new Dto(); Domain domain1 = new Domain(); + domain1.setLongs( new HashSet() ); + domain1.setStrings( new HashSet() ); Domain domain2 = DomainDtoWithNvmsNullMapper.INSTANCE.updateWithReturn( dto, domain1 ); doControlAsserts( domain1, domain2 ); @@ -151,11 +160,15 @@ public class Issue913SetterMapperForCollectionsTest { Dto dto = new Dto(); Domain domain = new Domain(); + Set longIn = new HashSet(); + domain.setLongs( longIn ); + domain.setStrings( new HashSet() ); DomainDtoWithNvmsDefaultMapper.INSTANCE.update( dto, domain ); doControlAsserts( domain ); assertThat( domain.getStrings() ).isNull(); assertThat( domain.getLongs() ).isEmpty(); + assertThat( domain.getLongs() ).isSameAs( longIn ); // make sure add all is used. } /** @@ -172,6 +185,9 @@ public class Issue913SetterMapperForCollectionsTest { Dto dto = new Dto(); Domain domain1 = new Domain(); + Set longIn = new HashSet(); + domain1.setLongs( longIn ); + domain1.setStrings( new HashSet() ); Domain domain2 = DomainDtoWithNvmsDefaultMapper.INSTANCE.updateWithReturn( dto, domain1 ); doControlAsserts( domain1, domain2 ); @@ -180,6 +196,8 @@ public class Issue913SetterMapperForCollectionsTest { assertThat( domain1.getLongs() ).isEmpty(); assertThat( domain2.getStrings() ).isNull(); assertThat( domain2.getLongs() ).isEmpty(); + assertThat( domain1.getLongs() ).isSameAs( longIn ); // make sure that add all is used + assertThat( domain2.getLongs() ).isSameAs( longIn ); // make sure that add all is used } /** @@ -206,6 +224,8 @@ public class Issue913SetterMapperForCollectionsTest { DtoWithPresenceCheck dto = new DtoWithPresenceCheck(); Domain domain = new Domain(); + domain.setLongs( new HashSet() ); + domain.setStrings( new HashSet() ); DomainDtoWithPresenceCheckMapper.INSTANCE.update( dto, domain ); doControlAsserts( domain ); @@ -222,6 +242,8 @@ public class Issue913SetterMapperForCollectionsTest { DtoWithPresenceCheck dto = new DtoWithPresenceCheck(); Domain domain1 = new Domain(); + domain1.setLongs( new HashSet() ); + domain1.setStrings( new HashSet() ); Domain domain2 = DomainDtoWithPresenceCheckMapper.INSTANCE.updateWithReturn( dto, domain1 ); doControlAsserts( domain1, domain2 ); From 9186978e1f6fe49f53d74e14623ef54adf251523 Mon Sep 17 00:00:00 2001 From: sjaakd Date: Sun, 13 Nov 2016 20:51:37 +0100 Subject: [PATCH 2/3] #954 Explicit set to null NOT for NVCS.ALWAYS and presence-checkers --- .../ap/internal/model/PropertyMapping.java | 3 +- .../SetterWrapperForCollectionsAndMaps.java | 9 +- .../SetterWrapperForCollectionsAndMaps.ftl | 3 +- .../_913/DomainDtoWithNcvsAlwaysMapper.java | 52 ++++++++++++ ...ssue913SetterMapperForCollectionsTest.java | 83 +++++++++++++++++-- 5 files changed, 139 insertions(+), 11 deletions(-) create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_913/DomainDtoWithNcvsAlwaysMapper.java 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 c2a0f76f5..050fa2eab 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 @@ -461,7 +461,8 @@ public class PropertyMapping extends ModelElement { method.getThrownTypes(), getSourcePresenceCheckerRef(), existingVariableNames, - targetType + targetType, + ALWAYS.equals( method.getMapperConfiguration().getNullValueCheckStrategy() ) ); } diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/assignment/SetterWrapperForCollectionsAndMaps.java b/processor/src/main/java/org/mapstruct/ap/internal/model/assignment/SetterWrapperForCollectionsAndMaps.java index aa9662a51..2ddb8020d 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/assignment/SetterWrapperForCollectionsAndMaps.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/assignment/SetterWrapperForCollectionsAndMaps.java @@ -38,16 +38,19 @@ import org.mapstruct.ap.internal.model.common.Type; public class SetterWrapperForCollectionsAndMaps extends WrapperForCollectionsAndMaps { private final Assignment newCollectionOrMapAssignment; + private final boolean allwaysIncludeNullCheck; public SetterWrapperForCollectionsAndMaps(Assignment decoratedAssignment, Assignment newCollectionOrMapAssignment, List thrownTypesToExclude, String sourcePresenceChecker, Set existingVariableNames, - Type targetType ) { + Type targetType, + boolean allwaysIncludeNullCheck ) { super( decoratedAssignment, thrownTypesToExclude, sourcePresenceChecker, existingVariableNames, targetType ); this.newCollectionOrMapAssignment = newCollectionOrMapAssignment; + this.allwaysIncludeNullCheck = allwaysIncludeNullCheck; } @Override @@ -63,5 +66,9 @@ public class SetterWrapperForCollectionsAndMaps extends WrapperForCollectionsAnd return newCollectionOrMapAssignment; } + public boolean isAllwaysIncludeNullCheck() { + return allwaysIncludeNullCheck; + } + } diff --git a/processor/src/main/resources/org/mapstruct/ap/internal/model/assignment/SetterWrapperForCollectionsAndMaps.ftl b/processor/src/main/resources/org/mapstruct/ap/internal/model/assignment/SetterWrapperForCollectionsAndMaps.ftl index f0acf604b..7d2ed09df 100644 --- a/processor/src/main/resources/org/mapstruct/ap/internal/model/assignment/SetterWrapperForCollectionsAndMaps.ftl +++ b/processor/src/main/resources/org/mapstruct/ap/internal/model/assignment/SetterWrapperForCollectionsAndMaps.ftl @@ -26,8 +26,7 @@ ${ext.targetBeanName}.${ext.targetReadAccessorName}().clear(); ${ext.targetBeanName}.${ext.targetReadAccessorName}().<#if ext.targetType.collectionType>addAll<#else>putAll( ${localVarName} ); - <#if !ext.defaultValueAssignment??> <#-- the opposite (defaultValueAssignment) case is handeld inside lib.handleNullCheck --> - else { + <#if !ext.defaultValueAssignment?? && !sourcePresenceChecker?? && !allwaysIncludeNullCheck>else {<#-- the opposite (defaultValueAssignment) case is handeld inside lib.handleNullCheck --> ${ext.targetBeanName}.${ext.targetWriteAccessorName}( null ); } diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_913/DomainDtoWithNcvsAlwaysMapper.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_913/DomainDtoWithNcvsAlwaysMapper.java new file mode 100644 index 000000000..55a51856b --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_913/DomainDtoWithNcvsAlwaysMapper.java @@ -0,0 +1,52 @@ +/** + * Copyright 2012-2016 Gunnar Morling (http://www.gunnarmorling.de/) + * and/or other contributors as indicated by the @authors tag. See the + * copyright.txt file in the distribution for a full listing of all + * contributors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.mapstruct.ap.test.bugs._913; + +import org.mapstruct.InheritConfiguration; +import org.mapstruct.Mapper; +import org.mapstruct.Mapping; +import org.mapstruct.MappingTarget; +import org.mapstruct.Mappings; +import org.mapstruct.NullValueCheckStrategy; +import org.mapstruct.factory.Mappers; + +/** + * + * @author Sjaak Derksen + */ +@Mapper(uses = Helper.class, nullValueCheckStrategy = NullValueCheckStrategy.ALWAYS ) +public interface DomainDtoWithNcvsAlwaysMapper { + + DomainDtoWithNcvsAlwaysMapper INSTANCE = Mappers.getMapper( DomainDtoWithNcvsAlwaysMapper.class ); + + @Mappings({ + @Mapping(target = "strings", source = "strings"), + @Mapping(target = "longs", source = "strings"), + @Mapping(target = "stringsInitialized", source = "stringsInitialized"), + @Mapping(target = "longsInitialized", source = "stringsInitialized"), + @Mapping(target = "stringsWithDefault", source = "stringsWithDefault", defaultValue = "3") + }) + Domain create(DtoWithPresenceCheck source); + + @InheritConfiguration( name = "create" ) + void update(DtoWithPresenceCheck source, @MappingTarget Domain target); + + @InheritConfiguration( name = "create" ) + Domain updateWithReturn(DtoWithPresenceCheck source, @MappingTarget Domain target); +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_913/Issue913SetterMapperForCollectionsTest.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_913/Issue913SetterMapperForCollectionsTest.java index 53fd509fb..c3cc504bf 100644 --- a/processor/src/test/java/org/mapstruct/ap/test/bugs/_913/Issue913SetterMapperForCollectionsTest.java +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_913/Issue913SetterMapperForCollectionsTest.java @@ -42,6 +42,7 @@ import org.mapstruct.ap.testutil.runner.AnnotationProcessorTestRunner; DomainDtoWithNvmsNullMapper.class, DomainDtoWithNvmsDefaultMapper.class, DomainDtoWithPresenceCheckMapper.class, + DomainDtoWithNcvsAlwaysMapper.class, Helper.class}) @IssueKey( "913" ) public class Issue913SetterMapperForCollectionsTest { @@ -201,7 +202,7 @@ public class Issue913SetterMapperForCollectionsTest { } /** - * Test create method ICW presence checker + * Test create method ICW presence checker. The presence checker is responsible for the null check. * */ @Test @@ -218,7 +219,10 @@ public class Issue913SetterMapperForCollectionsTest { /** * Test update method ICW presence checker * + * Similar as in regular mappings, the target property should be left as-is. + * */ + @IssueKey( "#954") @Test public void shouldReturnNullForUpdateWithPresenceChecker() { @@ -229,14 +233,17 @@ public class Issue913SetterMapperForCollectionsTest { DomainDtoWithPresenceCheckMapper.INSTANCE.update( dto, domain ); doControlAsserts( domain ); - assertThat( domain.getStrings() ).isNull(); - assertThat( domain.getLongs() ).isNull(); + assertThat( domain.getStrings() ).isEmpty(); + assertThat( domain.getLongs() ).isEmpty(); } /** * Test update with return method ICW presence checker * + * Similar as in regular mappings, the target property should be left as-is. + * */ + @IssueKey( "#954") @Test public void shouldReturnNullForUpdateWithReturnWithPresenceChecker() { @@ -248,12 +255,74 @@ public class Issue913SetterMapperForCollectionsTest { doControlAsserts( domain1, domain2 ); assertThat( domain1.getLongs() ).isEqualTo( domain2.getLongs() ); - assertThat( domain1.getStrings() ).isNull(); - assertThat( domain1.getLongs() ).isNull(); - assertThat( domain2.getStrings() ).isNull(); - assertThat( domain2.getLongs() ).isNull(); + assertThat( domain1.getStrings() ).isEmpty(); + assertThat( domain1.getLongs() ).isEmpty(); + assertThat( domain2.getStrings() ).isEmpty(); + assertThat( domain2.getLongs() ).isEmpty(); } + /** + * Test create method ICW NullValueCheckStrategy.ALWAYS. + * + */ + @IssueKey( "#954") + @Test + public void shouldReturnNullForCreateWithNcvsAlways() { + + DtoWithPresenceCheck dto = new DtoWithPresenceCheck(); + Domain domain = DomainDtoWithNcvsAlwaysMapper.INSTANCE.create( dto ); + + doControlAsserts( domain ); + assertThat( domain.getStrings() ).isNull(); + assertThat( domain.getLongs() ).isNull(); + } + + /** + * Test update method ICW presence checker + * + * Similar as in regular mappings, the target property should be left as-is. + * + */ + @IssueKey( "#954") + @Test + public void shouldReturnNullForUpdateWithNcvsAlways() { + + DtoWithPresenceCheck dto = new DtoWithPresenceCheck(); + Domain domain = new Domain(); + domain.setLongs( new HashSet() ); + domain.setStrings( new HashSet() ); + DomainDtoWithNcvsAlwaysMapper.INSTANCE.update( dto, domain ); + + doControlAsserts( domain ); + assertThat( domain.getStrings() ).isEmpty(); + assertThat( domain.getLongs() ).isEmpty(); + } + + /** + * Test update with return method ICW presence checker + * + * Similar as in regular mappings, the target property should be left as-is. + * + */ + @IssueKey( "#954") + @Test + public void shouldReturnNullForUpdateWithReturnWithNcvsAlways() { + + DtoWithPresenceCheck dto = new DtoWithPresenceCheck(); + Domain domain1 = new Domain(); + domain1.setLongs( new HashSet() ); + domain1.setStrings( new HashSet() ); + Domain domain2 = DomainDtoWithNcvsAlwaysMapper.INSTANCE.updateWithReturn( dto, domain1 ); + + doControlAsserts( domain1, domain2 ); + assertThat( domain1.getLongs() ).isEqualTo( domain2.getLongs() ); + assertThat( domain1.getStrings() ).isEmpty(); + assertThat( domain1.getLongs() ).isEmpty(); + assertThat( domain2.getStrings() ).isEmpty(); + assertThat( domain2.getLongs() ).isEmpty(); + } + + /** From 4c4a9ea934976d0208c02c01a1ba1a0571d888ad Mon Sep 17 00:00:00 2001 From: sjaakd Date: Sun, 13 Nov 2016 22:40:11 +0100 Subject: [PATCH 3/3] #954 cleanup --- .../ap/internal/model/PropertyMapping.java | 33 ++--------- .../model/assignment/EnumSetCopyWrapper.java | 55 ------------------- .../assignment/NewCollectionOrMapWrapper.java | 47 ---------------- .../SetterWrapperForCollectionsAndMaps.java | 37 +++++++++---- .../model/assignment/EnumSetCopyWrapper.ftl | 21 ------- .../assignment/NewCollectionOrMapWrapper.ftl | 28 ---------- .../SetterWrapperForCollectionsAndMaps.ftl | 51 ++++++++--------- ...ssue913SetterMapperForCollectionsTest.java | 8 +-- 8 files changed, 59 insertions(+), 221 deletions(-) delete mode 100644 processor/src/main/java/org/mapstruct/ap/internal/model/assignment/EnumSetCopyWrapper.java delete mode 100644 processor/src/main/java/org/mapstruct/ap/internal/model/assignment/NewCollectionOrMapWrapper.java delete mode 100644 processor/src/main/resources/org/mapstruct/ap/internal/model/assignment/EnumSetCopyWrapper.ftl delete mode 100644 processor/src/main/resources/org/mapstruct/ap/internal/model/assignment/NewCollectionOrMapWrapper.ftl 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 050fa2eab..fe01e70c3 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 @@ -35,9 +35,7 @@ import org.mapstruct.ap.internal.model.assignment.AdderWrapper; import org.mapstruct.ap.internal.model.assignment.ArrayCopyWrapper; import org.mapstruct.ap.internal.model.assignment.Assignment; import org.mapstruct.ap.internal.model.assignment.EnumConstantWrapper; -import org.mapstruct.ap.internal.model.assignment.EnumSetCopyWrapper; import org.mapstruct.ap.internal.model.assignment.GetterWrapperForCollectionsAndMaps; -import org.mapstruct.ap.internal.model.assignment.NewCollectionOrMapWrapper; import org.mapstruct.ap.internal.model.assignment.NullCheckWrapper; import org.mapstruct.ap.internal.model.assignment.SetterWrapper; import org.mapstruct.ap.internal.model.assignment.SetterWrapperForCollectionsAndMaps; @@ -419,53 +417,30 @@ public class PropertyMapping extends ModelElement { Assignment result = rhs; - // wrap the setter in the collection / map initializers if ( targetAccessorType == TargetWriteAccessorType.SETTER ) { - // wrap the assignment in a new Map or Collection implementation if this is not done in a - // mapping method. Note, typeconversons do not apply to collections or maps - Assignment newCollectionOrMap = null; - if ( result.getType() == DIRECT ) { - Set implementationTypes; - if ( targetType.getImplementationType() != null ) { - implementationTypes = targetType.getImplementationType().getImportTypes(); - } - else { - implementationTypes = targetType.getImportTypes(); - } - - if ( "java.util.EnumSet".equals( targetType.getFullyQualifiedName() ) ) { - newCollectionOrMap = new EnumSetCopyWrapper( ctx.getTypeFactory(), result ); - } - else { - newCollectionOrMap = new NewCollectionOrMapWrapper( result, implementationTypes ); - } - - newCollectionOrMap = new SetterWrapper( newCollectionOrMap, method.getThrownTypes() ); - } if ( result.isUpdateMethod() ) { + // call to an update method if ( targetReadAccessor == null ) { ctx.getMessager().printMessage( method.getExecutable(), Message.PROPERTYMAPPING_NO_READ_ACCESSOR_FOR_TARGET_TYPE, targetPropertyName ); } Assignment factoryMethod = ctx.getMappingResolver().getFactoryMethod( method, targetType, null ); - result = new UpdateWrapper( result, method.getThrownTypes(), factoryMethod, - targetType ); + result = new UpdateWrapper( result, method.getThrownTypes(), factoryMethod, targetType ); } else { // target accessor is setter, so wrap the setter in setter map/ collection handling result = new SetterWrapperForCollectionsAndMaps( result, - newCollectionOrMap, method.getThrownTypes(), getSourcePresenceCheckerRef(), existingVariableNames, targetType, - ALWAYS.equals( method.getMapperConfiguration().getNullValueCheckStrategy() ) + ALWAYS == method.getMapperConfiguration().getNullValueCheckStrategy(), + ctx.getTypeFactory() ); } - } else { // target accessor is getter, so wrap the setter in getter map/ collection handling diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/assignment/EnumSetCopyWrapper.java b/processor/src/main/java/org/mapstruct/ap/internal/model/assignment/EnumSetCopyWrapper.java deleted file mode 100644 index 11fa41fb8..000000000 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/assignment/EnumSetCopyWrapper.java +++ /dev/null @@ -1,55 +0,0 @@ -/** - * Copyright 2012-2016 Gunnar Morling (http://www.gunnarmorling.de/) - * and/or other contributors as indicated by the @authors tag. See the - * copyright.txt file in the distribution for a full listing of all - * contributors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.mapstruct.ap.internal.model.assignment; - -import java.util.EnumSet; -import java.util.HashSet; -import java.util.Set; - -import org.mapstruct.ap.internal.model.common.Type; -import org.mapstruct.ap.internal.model.common.TypeFactory; - -/** - * Invokes the copy method for enum sets. - * - * @author Gunnar Morling - */ -public class EnumSetCopyWrapper extends AssignmentWrapper { - - private Type enumSetType; - - public EnumSetCopyWrapper(TypeFactory typeFactory, Assignment decoratedAssignment) { - super( decoratedAssignment ); - enumSetType = typeFactory.getType( EnumSet.class ); - } - - @Override - public Set getImportTypes() { - Set imported = new HashSet( getAssignment().getImportTypes().size() + 1 ); - imported.addAll( getAssignment().getImportTypes() ); - imported.add( enumSetType ); - - return imported; - } - - @Override - public String toString() { - return "EnumSet.copyOf( " + getAssignment() + " )"; - } -} diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/assignment/NewCollectionOrMapWrapper.java b/processor/src/main/java/org/mapstruct/ap/internal/model/assignment/NewCollectionOrMapWrapper.java deleted file mode 100644 index 48bd004d3..000000000 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/assignment/NewCollectionOrMapWrapper.java +++ /dev/null @@ -1,47 +0,0 @@ -/** - * Copyright 2012-2016 Gunnar Morling (http://www.gunnarmorling.de/) - * and/or other contributors as indicated by the @authors tag. See the - * copyright.txt file in the distribution for a full listing of all - * contributors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.mapstruct.ap.internal.model.assignment; - -import java.util.HashSet; -import java.util.Set; - -import org.mapstruct.ap.internal.model.common.Type; - -/** - * Decorates the assignment as a Map or Collection constructor - * - * @author Sjaak Derksen - */ -public class NewCollectionOrMapWrapper extends AssignmentWrapper { - - private final Set implementationTypes; - - public NewCollectionOrMapWrapper(Assignment decoratedAssignment, Set implementationTypes) { - super( decoratedAssignment ); - this.implementationTypes = implementationTypes; - } - - @Override - public Set getImportTypes() { - Set imported = new HashSet(); - imported.addAll( getAssignment().getImportTypes() ); - imported.addAll( implementationTypes ); - return imported; - } -} diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/assignment/SetterWrapperForCollectionsAndMaps.java b/processor/src/main/java/org/mapstruct/ap/internal/model/assignment/SetterWrapperForCollectionsAndMaps.java index 2ddb8020d..d8c2d4549 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/assignment/SetterWrapperForCollectionsAndMaps.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/assignment/SetterWrapperForCollectionsAndMaps.java @@ -18,10 +18,13 @@ */ package org.mapstruct.ap.internal.model.assignment; +import java.util.EnumSet; import java.util.List; import java.util.Set; +import static org.mapstruct.ap.internal.model.assignment.Assignment.AssignmentType.DIRECT; import org.mapstruct.ap.internal.model.common.Type; +import org.mapstruct.ap.internal.model.common.TypeFactory; /** * This wrapper handles the situation were an assignment is done via the setter. @@ -37,38 +40,52 @@ import org.mapstruct.ap.internal.model.common.Type; */ public class SetterWrapperForCollectionsAndMaps extends WrapperForCollectionsAndMaps { - private final Assignment newCollectionOrMapAssignment; private final boolean allwaysIncludeNullCheck; + private final Type targetType; + private final TypeFactory typeFactory; public SetterWrapperForCollectionsAndMaps(Assignment decoratedAssignment, - Assignment newCollectionOrMapAssignment, List thrownTypesToExclude, String sourcePresenceChecker, Set existingVariableNames, Type targetType, - boolean allwaysIncludeNullCheck ) { + boolean allwaysIncludeNullCheck, + TypeFactory typeFactory ) { super( decoratedAssignment, thrownTypesToExclude, sourcePresenceChecker, existingVariableNames, targetType ); - this.newCollectionOrMapAssignment = newCollectionOrMapAssignment; this.allwaysIncludeNullCheck = allwaysIncludeNullCheck; + this.targetType = targetType; + this.typeFactory = typeFactory; } @Override public Set getImportTypes() { Set imported = super.getImportTypes(); - if ( newCollectionOrMapAssignment != null ) { - imported.addAll( newCollectionOrMapAssignment.getImportTypes() ); + if ( isDirectAssignment() ) { + if ( targetType.getImplementationType() != null ) { + imported.addAll( targetType.getImplementationType().getImportTypes() ); + } + else { + imported.addAll( targetType.getImportTypes() ); + } + + if ( isEnumSet() ) { + imported.add( typeFactory.getType( EnumSet.class ) ); + } } return imported; } - public Assignment getNewCollectionOrMapAssignment() { - return newCollectionOrMapAssignment; - } - public boolean isAllwaysIncludeNullCheck() { return allwaysIncludeNullCheck; } + public boolean isDirectAssignment() { + return getType() == DIRECT; + } + + public boolean isEnumSet() { + return "java.util.EnumSet".equals( targetType.getFullyQualifiedName() ); + } } diff --git a/processor/src/main/resources/org/mapstruct/ap/internal/model/assignment/EnumSetCopyWrapper.ftl b/processor/src/main/resources/org/mapstruct/ap/internal/model/assignment/EnumSetCopyWrapper.ftl deleted file mode 100644 index 4f692c7b7..000000000 --- a/processor/src/main/resources/org/mapstruct/ap/internal/model/assignment/EnumSetCopyWrapper.ftl +++ /dev/null @@ -1,21 +0,0 @@ -<#-- - - Copyright 2012-2016 Gunnar Morling (http://www.gunnarmorling.de/) - and/or other contributors as indicated by the @authors tag. See the - copyright.txt file in the distribution for a full listing of all - contributors. - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. - ---> -EnumSet.copyOf( <@includeModel object=assignment targetBeanName=ext.targetBeanName targetReadAccessorName=ext.targetReadAccessorName targetWriteAccessorName=ext.targetWriteAccessorName targetType=ext.targetType/> ) diff --git a/processor/src/main/resources/org/mapstruct/ap/internal/model/assignment/NewCollectionOrMapWrapper.ftl b/processor/src/main/resources/org/mapstruct/ap/internal/model/assignment/NewCollectionOrMapWrapper.ftl deleted file mode 100644 index 6581b9761..000000000 --- a/processor/src/main/resources/org/mapstruct/ap/internal/model/assignment/NewCollectionOrMapWrapper.ftl +++ /dev/null @@ -1,28 +0,0 @@ -<#-- - - Copyright 2012-2016 Gunnar Morling (http://www.gunnarmorling.de/) - and/or other contributors as indicated by the @authors tag. See the - copyright.txt file in the distribution for a full listing of all - contributors. - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. - ---> -<@compress single_line=true> -new <#if ext.targetType.implementationType??> - <@includeModel object=ext.targetType.implementationType/> -<#else> - <@includeModel object=ext.targetType/> - -( <@includeModel object=assignment targetBeanName=ext.targetBeanName targetReadAccessorName=ext.targetReadAccessorName targetWriteAccessorName=ext.targetWriteAccessorName targetType=ext.targetType/> ) - \ No newline at end of file diff --git a/processor/src/main/resources/org/mapstruct/ap/internal/model/assignment/SetterWrapperForCollectionsAndMaps.ftl b/processor/src/main/resources/org/mapstruct/ap/internal/model/assignment/SetterWrapperForCollectionsAndMaps.ftl index 7d2ed09df..4eda962f0 100644 --- a/processor/src/main/resources/org/mapstruct/ap/internal/model/assignment/SetterWrapperForCollectionsAndMaps.ftl +++ b/processor/src/main/resources/org/mapstruct/ap/internal/model/assignment/SetterWrapperForCollectionsAndMaps.ftl @@ -20,7 +20,7 @@ --> <#import "../macro/CommonMacros.ftl" as lib> <@lib.handleExceptions> -<#if ( ext.existingInstanceMapping ) > + <#if ext.existingInstanceMapping> if ( ${ext.targetBeanName}.${ext.targetReadAccessorName}() != null ) { <@lib.handleNullCheck> ${ext.targetBeanName}.${ext.targetReadAccessorName}().clear(); @@ -32,30 +32,31 @@ } else { - <@lib.handleNullCheck> - <#if newCollectionOrMapAssignment??> - <@_newCollectionOrMapAssignment/> - <#else> - ${ext.targetBeanName}.${ext.targetWriteAccessorName}( ${localVarName} ); - - + <@callTargetWriteAccessor/> } -<#else> - <@lib.handleNullCheck> - <#if newCollectionOrMapAssignment??> - <@_newCollectionOrMapAssignment/> - <#else> - ${ext.targetBeanName}.${ext.targetWriteAccessorName}( ${localVarName} ); - - - + <#else> + <@callTargetWriteAccessor/> + - -<#macro _newCollectionOrMapAssignment> - <@includeModel object=newCollectionOrMapAssignment - targetBeanName=ext.targetBeanName - existingInstanceMapping=ext.existingInstanceMapping - targetReadAccessorName=ext.targetReadAccessorName - targetWriteAccessorName=ext.targetWriteAccessorName - targetType=ext.targetType/> +<#-- + assigns the target via the regular target write accessor (usually the setter) +--> +<#macro callTargetWriteAccessor> + <@lib.handleNullCheck> + <#if directAssignment> + ${ext.targetBeanName}.${ext.targetWriteAccessorName}( <@wrapLocalVarInCollectionInitializer/> ); + <#else> + ${ext.targetBeanName}.${ext.targetWriteAccessorName}( ${localVarName} ); + + + +<#-- + wraps the local variable in a collection initializer (new collection, or EnumSet.copyOf) +--> +<#macro wrapLocalVarInCollectionInitializer> + <#if enumSet> + EnumSet.copyOf( ${localVarName} ) + <#else> + new <#if ext.targetType.implementationType??><@includeModel object=ext.targetType.implementationType/><#else><@includeModel object=ext.targetType/>( ${localVarName} ) + diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_913/Issue913SetterMapperForCollectionsTest.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_913/Issue913SetterMapperForCollectionsTest.java index c3cc504bf..20b9df33b 100644 --- a/processor/src/test/java/org/mapstruct/ap/test/bugs/_913/Issue913SetterMapperForCollectionsTest.java +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_913/Issue913SetterMapperForCollectionsTest.java @@ -47,7 +47,7 @@ import org.mapstruct.ap.testutil.runner.AnnotationProcessorTestRunner; @IssueKey( "913" ) public class Issue913SetterMapperForCollectionsTest { - /** + /** * The null value mapping strategy on type level (Mapper) should generate forged methods for the * conversion from string to long that return null in the entire mapper, so also for the forged * mapper. Note the default NVMS is RETURN_NULL. @@ -83,7 +83,6 @@ public class Issue913SetterMapperForCollectionsTest { } - /** * The null value mapping strategy on type level (Mapper) should generate forged methods for the * conversion from string to long that return null in the entire mapper, so also for the forged @@ -131,7 +130,7 @@ public class Issue913SetterMapperForCollectionsTest { assertThat( domain2.getLongs() ).isNull(); } - /** + /** * The null value mapping strategy on type level (Mapper) should generate forged methods for the * conversion from string to long that return default in the entire mapper, so also for the forged * mapper. Note the default NVMS is RETURN_NULL. @@ -322,9 +321,6 @@ public class Issue913SetterMapperForCollectionsTest { assertThat( domain2.getLongs() ).isEmpty(); } - - - /** * These assert check if non-null and default mapping is working as expected. *