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. *