diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/IterableMappingMethod.java b/processor/src/main/java/org/mapstruct/ap/internal/model/IterableMappingMethod.java index 50443908f..2152e7f08 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/IterableMappingMethod.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/IterableMappingMethod.java @@ -128,7 +128,7 @@ public class IterableMappingMethod extends MappingMethod { } // target accessor is setter, so decorate assignment as setter if ( resultType.isArrayType() ) { - assignment = new LocalVarWrapper( assignment, method.getThrownTypes() ); + assignment = new LocalVarWrapper( assignment, method.getThrownTypes(), resultType ); } else { assignment = new SetterWrapper( assignment, method.getThrownTypes() ); diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/MapMappingMethod.java b/processor/src/main/java/org/mapstruct/ap/internal/model/MapMappingMethod.java index 515f18eab..465e2c578 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/MapMappingMethod.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/MapMappingMethod.java @@ -174,8 +174,8 @@ public class MapMappingMethod extends MappingMethod { factoryMethod = ctx.getMappingResolver().getFactoryMethod( method, method.getResultType(), null ); } - keyAssignment = new LocalVarWrapper( keyAssignment, method.getThrownTypes() ); - valueAssignment = new LocalVarWrapper( valueAssignment, method.getThrownTypes() ); + keyAssignment = new LocalVarWrapper( keyAssignment, method.getThrownTypes(), keyTargetType ); + valueAssignment = new LocalVarWrapper( valueAssignment, method.getThrownTypes(), valueTargetType ); List beforeMappingMethods = LifecycleCallbackFactory.beforeMappingMethods( method, null, ctx ); 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 0dc269c87..d3da6de7f 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 @@ -38,6 +38,7 @@ 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.LocalVarWrapper; import org.mapstruct.ap.internal.model.assignment.NewCollectionOrMapWrapper; import org.mapstruct.ap.internal.model.assignment.NullCheckWrapper; import org.mapstruct.ap.internal.model.assignment.SetterWrapper; @@ -447,14 +448,24 @@ public class PropertyMapping extends ModelElement { targetType ); } else { - // wrap the assignment in the setter method - result = new SetterWrapper( result, method.getThrownTypes() ); + + if ( method.isUpdateMethod() ) { + // if the calling method is an update method and accesses a getter, make a local variable to + // test NPE first. + result = new LocalVarWrapper( result, method.getThrownTypes(), targetType ); + } + else { + // if not, asssign as new collecitin or direct + result = new SetterWrapper( result, method.getThrownTypes() ); + } // target accessor is setter, so wrap the setter in setter map/ collection handling result = new SetterWrapperForCollectionsAndMaps( result, - targetWriteAccessor, - newCollectionOrMap + Executables.getCollectionGetterName( targetWriteAccessor ), + newCollectionOrMap, + targetType, + existingVariableNames ); } diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/assignment/LocalVarWrapper.java b/processor/src/main/java/org/mapstruct/ap/internal/model/assignment/LocalVarWrapper.java index 25134fd57..a0b732c00 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/assignment/LocalVarWrapper.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/assignment/LocalVarWrapper.java @@ -19,7 +19,9 @@ package org.mapstruct.ap.internal.model.assignment; import java.util.ArrayList; +import java.util.HashSet; import java.util.List; +import java.util.Set; import org.mapstruct.ap.internal.model.common.Type; @@ -31,10 +33,12 @@ import org.mapstruct.ap.internal.model.common.Type; public class LocalVarWrapper extends AssignmentWrapper { private final List thrownTypesToExclude; + private final Type targetType; - public LocalVarWrapper( Assignment decoratedAssignment, List thrownTypesToExclude ) { + public LocalVarWrapper( Assignment decoratedAssignment, List thrownTypesToExclude, Type targetType ) { super( decoratedAssignment ); this.thrownTypesToExclude = thrownTypesToExclude; + this.targetType = targetType; } @Override @@ -50,4 +54,13 @@ public class LocalVarWrapper extends AssignmentWrapper { } return result; } + + @Override + public Set getImportTypes() { + Set imported = new HashSet( getAssignment().getImportTypes() ); + imported.add( targetType ); + imported.addAll( targetType.getTypeParameters() ); + 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 a6ab91028..a7c590d53 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,13 +18,12 @@ */ package org.mapstruct.ap.internal.model.assignment; +import java.util.Collection; import java.util.HashSet; import java.util.Set; -import javax.lang.model.element.ExecutableElement; - import org.mapstruct.ap.internal.model.common.Type; -import org.mapstruct.ap.internal.util.Executables; +import org.mapstruct.ap.internal.util.Strings; /** * This wrapper handles the situation were an assignment is done via the setter. @@ -42,14 +41,21 @@ public class SetterWrapperForCollectionsAndMaps extends AssignmentWrapper { private final String targetGetterName; private final Assignment newCollectionOrMapAssignment; + private final Type targetType; + private final String localVarName; public SetterWrapperForCollectionsAndMaps(Assignment decoratedAssignment, - ExecutableElement targetSetter, - Assignment newCollectionOrMapAssignment) { + String targetGetterName, + Assignment newCollectionOrMapAssignment, + Type targetType, + Collection existingVariableNames) { super( decoratedAssignment ); - this.targetGetterName = Executables.getCollectionGetterName( targetSetter ); + this.targetGetterName = targetGetterName; this.newCollectionOrMapAssignment = newCollectionOrMapAssignment; + this.targetType = targetType; + this.localVarName = Strings.getSaveVariableName( targetType.getName(), existingVariableNames ); + existingVariableNames.add( localVarName ); } public String getTargetGetterName() { @@ -69,4 +75,9 @@ public class SetterWrapperForCollectionsAndMaps extends AssignmentWrapper { } return imported; } + + public String getLocalVarName() { + return localVarName; + } + } 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 709e3423e..27eba45a2 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 @@ -19,44 +19,33 @@ --> <#if ( ext.existingInstanceMapping ) > + <@_assignment targetWriteAccessorName = localVarName/> if ( ${ext.targetBeanName}.${targetGetterName}() != null ) { ${ext.targetBeanName}.${targetGetterName}().clear(); - <#if ext.targetType.collectionType> - <@includeModel object=assignment - targetBeanName=ext.targetBeanName - existingInstanceMapping=ext.existingInstanceMapping - targetReadAccessorName=ext.targetReadAccessorName - targetWriteAccessorName="${targetGetterName}().addAll" - targetType=ext.targetType/> - <#else> - <@includeModel object=assignment - targetBeanName=ext.targetBeanName - existingInstanceMapping=ext.existingInstanceMapping - targetReadAccessorName=ext.targetReadAccessorName - targetWriteAccessorName="${targetGetterName}().putAll" - targetType=ext.targetType/> - + if ( ${localVarName} != null ) { + ${ext.targetBeanName}.${ext.targetReadAccessorName}().<#if ext.targetType.collectionType>addAll<#else>putAll( ${localVarName} ); + } } else { <#if newCollectionOrMapAssignment??> <@_newCollectionOrMapAssignment/> <#else> - <@_assignment/> + ${ext.targetBeanName}.${ext.targetWriteAccessorName}( ${localVarName} ); } <#else> <#if newCollectionOrMapAssignment??> <@_newCollectionOrMapAssignment/> <#else> - <@_assignment/> + <@_assignment targetWriteAccessorName = ext.targetWriteAccessorName/> -<#macro _assignment> +<#macro _assignment targetWriteAccessorName> <@includeModel object=assignment targetBeanName=ext.targetBeanName existingInstanceMapping=ext.existingInstanceMapping targetReadAccessorName=ext.targetReadAccessorName - targetWriteAccessorName=ext.targetWriteAccessorName + targetWriteAccessorName=targetWriteAccessorName targetType=ext.targetType/> <#macro _newCollectionOrMapAssignment> diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_865/Issue865Test.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_865/Issue865Test.java new file mode 100644 index 000000000..085ad9092 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_865/Issue865Test.java @@ -0,0 +1,69 @@ +/** + * 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._865; + +import static org.assertj.core.api.Assertions.assertThat; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mapstruct.ap.testutil.WithClasses; +import org.mapstruct.ap.testutil.runner.AnnotationProcessorTestRunner; + +/** + * + * @author Sjaak Derksen + */ +@RunWith(AnnotationProcessorTestRunner.class) +@WithClasses( { + ProjectDto.class, + ProjectEntity.class, + ProjectEntityWithoutSetter.class, + ProjCoreUserEntity.class, + ProjMemberDto.class, + ProjectMapper.class +} ) +public class Issue865Test { + + @Test + public void shouldGenerateNpeCheckBeforCallingAddAllWhenInUpdateMethods() { + + ProjectDto dto = new ProjectDto(); + dto.setName( "myProject" ); + + ProjectEntity entity = new ProjectEntity(); + entity.setCoreUsers( null ); + + ProjectMapper.INSTANCE.mapProjectUpdate( dto, entity ); + + assertThat( entity.getName() ).isEqualTo( "myProject" ); + assertThat( entity.getCoreUsers() ).isNull(); + } + + public void shouldGenerateNpeCheckBeforCallingAddAllWhenInUpdateMethodsAndTargetWithoutSetter() { + + ProjectDto dto = new ProjectDto(); + dto.setName( "myProject" ); + + ProjectEntityWithoutSetter entity = new ProjectEntityWithoutSetter(); + + ProjectMapper.INSTANCE.mapProjectUpdateWithoutGetter( dto, entity ); + + assertThat( entity.getName() ).isEqualTo( "myProject" ); + assertThat( entity.getCoreUsers() ).isNull(); + } +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_865/ProjCoreUserEntity.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_865/ProjCoreUserEntity.java new file mode 100644 index 000000000..abc5e49f3 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_865/ProjCoreUserEntity.java @@ -0,0 +1,37 @@ +/** + * 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._865; + +/** + * + * @author Sjaak Derksen + */ +public class ProjCoreUserEntity { + + private String memberName; + + public String getMemberName() { + return memberName; + } + + public void setMemberName(String memberName) { + this.memberName = memberName; + } + +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_865/ProjMemberDto.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_865/ProjMemberDto.java new file mode 100644 index 000000000..1101ab463 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_865/ProjMemberDto.java @@ -0,0 +1,37 @@ +/** + * 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._865; + +/** + * + * @author Sjaak Derksen + */ +public class ProjMemberDto { + + private String memberName; + + public String getMemberName() { + return memberName; + } + + public void setMemberName(String memberName) { + this.memberName = memberName; + } + +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_865/ProjectDto.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_865/ProjectDto.java new file mode 100644 index 000000000..abb1f4910 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_865/ProjectDto.java @@ -0,0 +1,48 @@ +/** + * 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._865; + +import java.util.List; + +/** + * + * @author Sjaak Derksen + */ +public class ProjectDto { + + List projectMembers; + String name; + + public List getProjectMembers() { + return projectMembers; + } + + public void setProjectMembers(List projectMembers) { + this.projectMembers = projectMembers; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_865/ProjectEntity.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_865/ProjectEntity.java new file mode 100644 index 000000000..2327ffc74 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_865/ProjectEntity.java @@ -0,0 +1,47 @@ +/** + * 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._865; + +import java.util.List; + +/** + * + * @author Sjaak Derksen + */ +public class ProjectEntity { + + private String name; + private List coreUsers; + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + public List getCoreUsers() { + return coreUsers; + } + + public void setCoreUsers(List coreUsers) { + this.coreUsers = coreUsers; + } +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_865/ProjectEntityWithoutSetter.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_865/ProjectEntityWithoutSetter.java new file mode 100644 index 000000000..c3bbffdf8 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_865/ProjectEntityWithoutSetter.java @@ -0,0 +1,44 @@ +/** + * 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._865; + +import java.util.List; + +/** + * + * @author Sjaak Derksen + */ +public class ProjectEntityWithoutSetter { + + private String name; + private List coreUsers = null; + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + public List getCoreUsers() { + return coreUsers; + } + +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_865/ProjectMapper.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_865/ProjectMapper.java new file mode 100644 index 000000000..1ab8bfb27 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_865/ProjectMapper.java @@ -0,0 +1,42 @@ +/** + * 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._865; + +import org.mapstruct.Mapper; +import org.mapstruct.Mapping; +import org.mapstruct.MappingTarget; +import org.mapstruct.factory.Mappers; + +/** + * + * @author Sjaak Derksen + */ +@Mapper +public interface ProjectMapper { + + ProjectMapper INSTANCE = Mappers.getMapper( ProjectMapper.class ); + + @Mapping(target = "coreUsers", source = "projectMembers") + void mapProjectUpdate(ProjectDto dto, @MappingTarget ProjectEntity entity); + + @Mapping(target = "coreUsers", source = "projectMembers") + void mapProjectUpdateWithoutGetter(ProjectDto dto, @MappingTarget ProjectEntityWithoutSetter entity); + + ProjCoreUserEntity mapUser(ProjMemberDto dto); +}