#865 Collection NPE check for a calling update method.

This commit is contained in:
sjaakd 2016-08-31 19:24:23 +02:00
parent ccb0f34575
commit c73d007fc8
13 changed files with 381 additions and 33 deletions

View File

@ -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() );

View File

@ -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<LifecycleCallbackMethodReference> beforeMappingMethods =
LifecycleCallbackFactory.beforeMappingMethods( method, null, ctx );

View File

@ -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
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
);
}

View File

@ -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<Type> thrownTypesToExclude;
private final Type targetType;
public LocalVarWrapper( Assignment decoratedAssignment, List<Type> thrownTypesToExclude ) {
public LocalVarWrapper( Assignment decoratedAssignment, List<Type> 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<Type> getImportTypes() {
Set<Type> imported = new HashSet<Type>( getAssignment().getImportTypes() );
imported.add( targetType );
imported.addAll( targetType.getTypeParameters() );
return imported;
}
}

View File

@ -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<String> 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;
}
}

View File

@ -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>
if ( ${localVarName} != null ) {
${ext.targetBeanName}.${ext.targetReadAccessorName}().<#if ext.targetType.collectionType>addAll<#else>putAll</#if>( ${localVarName} );
}
}
else {
<#if newCollectionOrMapAssignment??>
<@_newCollectionOrMapAssignment/>
<#else>
<@_assignment/>
${ext.targetBeanName}.${ext.targetWriteAccessorName}( ${localVarName} );
</#if>
}
<#else>
<#if newCollectionOrMapAssignment??>
<@_newCollectionOrMapAssignment/>
<#else>
<@_assignment/>
<@_assignment targetWriteAccessorName = ext.targetWriteAccessorName/>
</#if>
</#if>
<#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>
<#macro _newCollectionOrMapAssignment>

View File

@ -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();
}
}

View File

@ -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;
}
}

View File

@ -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;
}
}

View File

@ -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<ProjMemberDto> projectMembers;
String name;
public List<ProjMemberDto> getProjectMembers() {
return projectMembers;
}
public void setProjectMembers(List<ProjMemberDto> projectMembers) {
this.projectMembers = projectMembers;
}
public String getName() {
return name;
}
public void setName(String name) {
this.name = name;
}
}

View File

@ -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<ProjCoreUserEntity> coreUsers;
public String getName() {
return name;
}
public void setName(String name) {
this.name = name;
}
public List<ProjCoreUserEntity> getCoreUsers() {
return coreUsers;
}
public void setCoreUsers(List<ProjCoreUserEntity> coreUsers) {
this.coreUsers = coreUsers;
}
}

View File

@ -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<ProjCoreUserEntity> coreUsers = null;
public String getName() {
return name;
}
public void setName(String name) {
this.name = name;
}
public List<ProjCoreUserEntity> getCoreUsers() {
return coreUsers;
}
}

View File

@ -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);
}