#1273 Fix for NullValueMappingStrategy#RETURN_DEFAULT when using collections

When mapping a collection using NullValueMappingStrategy#RETURN_DEFAULT and the source is null, the target will be an empty collection.
This commit is contained in:
Kevin Grüneberg 2017-09-17 23:03:24 +02:00 committed by Filip Hrisafov
parent af8d48c797
commit 499dbd4561
16 changed files with 271 additions and 34 deletions

View File

@ -30,6 +30,7 @@ import org.mapstruct.ap.internal.model.source.Method;
import org.mapstruct.ap.internal.model.source.SelectionParameters;
import org.mapstruct.ap.internal.prism.CollectionMappingStrategyPrism;
import org.mapstruct.ap.internal.prism.NullValueCheckStrategyPrism;
import org.mapstruct.ap.internal.prism.NullValueMappingStrategyPrism;
import org.mapstruct.ap.internal.util.Message;
import org.mapstruct.ap.internal.util.accessor.Accessor;
@ -139,6 +140,7 @@ public class CollectionAssignmentBuilder {
targetPropertyName
);
}
Assignment factoryMethod = ctx.getMappingResolver()
.getFactoryMethod( method, targetType, SelectionParameters.forSourceRHS( sourceRHS ) );
result = new UpdateWrapper(
@ -147,31 +149,35 @@ public class CollectionAssignmentBuilder {
factoryMethod,
PropertyMapping.TargetWriteAccessorType.isFieldAssignment( targetAccessorType ),
targetType,
true
true,
mapNullToDefault()
);
}
else if ( method.isUpdateMethod() && !targetImmutable ) {
result = new ExistingInstanceSetterWrapperForCollectionsAndMaps(
result,
method.getThrownTypes(),
targetType,
method.getMapperConfiguration().getNullValueCheckStrategy(),
ctx.getTypeFactory(),
PropertyMapping.TargetWriteAccessorType.isFieldAssignment( targetAccessorType )
PropertyMapping.TargetWriteAccessorType.isFieldAssignment( targetAccessorType ),
mapNullToDefault()
);
}
else if ( result.getType() == Assignment.AssignmentType.DIRECT ||
method.getMapperConfiguration().getNullValueCheckStrategy() == NullValueCheckStrategyPrism.ALWAYS ) {
result = new SetterWrapperForCollectionsAndMapsWithNullCheck(
result,
method.getThrownTypes(),
targetType,
ctx.getTypeFactory(),
PropertyMapping.TargetWriteAccessorType.isFieldAssignment( targetAccessorType )
PropertyMapping.TargetWriteAccessorType.isFieldAssignment( targetAccessorType ),
mapNullToDefault()
);
}
else {
// target accessor is setter, so wrap the setter in setter map/ collection handling
result = new SetterWrapperForCollectionsAndMaps(
result,
@ -201,4 +207,9 @@ public class CollectionAssignmentBuilder {
return result;
}
private boolean mapNullToDefault() {
return method.getMapperConfiguration().getNullValueMappingStrategy()
== NullValueMappingStrategyPrism.RETURN_DEFAULT;
}
}

View File

@ -53,6 +53,7 @@ import org.mapstruct.ap.internal.model.source.PropertyEntry;
import org.mapstruct.ap.internal.model.source.SelectionParameters;
import org.mapstruct.ap.internal.model.source.SourceReference;
import org.mapstruct.ap.internal.prism.NullValueCheckStrategyPrism;
import org.mapstruct.ap.internal.prism.NullValueMappingStrategyPrism;
import org.mapstruct.ap.internal.util.Executables;
import org.mapstruct.ap.internal.util.MapperConfiguration;
import org.mapstruct.ap.internal.util.Message;
@ -394,10 +395,14 @@ public class PropertyMapping extends ModelElement {
targetPropertyName
);
}
boolean mapNullToDefault = method.getMapperConfiguration().
getNullValueMappingStrategy() == NullValueMappingStrategyPrism.RETURN_DEFAULT;
Assignment factory = ctx.getMappingResolver()
.getFactoryMethod( method, targetType, SelectionParameters.forSourceRHS( rightHandSide ) );
return new UpdateWrapper( rhs, method.getThrownTypes(), factory, isFieldAssignment(), targetType,
!rhs.isSourceReferenceParameter() );
!rhs.isSourceReferenceParameter(), mapNullToDefault );
}
else {
NullValueCheckStrategyPrism nvcs = method.getMapperConfiguration().getNullValueCheckStrategy();
@ -754,10 +759,15 @@ public class PropertyMapping extends ModelElement {
targetPropertyName
);
}
boolean mapNullToDefault = method.getMapperConfiguration().
getNullValueMappingStrategy() == NullValueMappingStrategyPrism.RETURN_DEFAULT;
Assignment factoryMethod =
ctx.getMappingResolver().getFactoryMethod( method, targetType, null );
assignment = new UpdateWrapper( assignment, method.getThrownTypes(), factoryMethod,
isFieldAssignment(), targetType, false );
isFieldAssignment(), targetType, false, mapNullToDefault );
}
else {
assignment = new SetterWrapper( assignment, method.getThrownTypes(), isFieldAssignment() );

View File

@ -18,6 +18,8 @@
*/
package org.mapstruct.ap.internal.model.assignment;
import static org.mapstruct.ap.internal.prism.NullValueCheckStrategyPrism.ALWAYS;
import java.util.List;
import org.mapstruct.ap.internal.model.common.Assignment;
@ -25,8 +27,6 @@ import org.mapstruct.ap.internal.model.common.Type;
import org.mapstruct.ap.internal.model.common.TypeFactory;
import org.mapstruct.ap.internal.prism.NullValueCheckStrategyPrism;
import static org.mapstruct.ap.internal.prism.NullValueCheckStrategyPrism.ALWAYS;
/**
* This wrapper handles the situation where an assignment is done for an update method.
*
@ -49,14 +49,16 @@ public class ExistingInstanceSetterWrapperForCollectionsAndMaps
Type targetType,
NullValueCheckStrategyPrism nvms,
TypeFactory typeFactory,
boolean fieldAssignment) {
boolean fieldAssignment,
boolean mapNullToDefault) {
super(
decoratedAssignment,
thrownTypesToExclude,
targetType,
typeFactory,
fieldAssignment
fieldAssignment,
mapNullToDefault
);
this.includeSourceNullCheck = ALWAYS == nvms;
}

View File

@ -18,6 +18,8 @@
*/
package org.mapstruct.ap.internal.model.assignment;
import static org.mapstruct.ap.internal.model.common.Assignment.AssignmentType.DIRECT;
import java.util.EnumSet;
import java.util.HashSet;
import java.util.List;
@ -27,8 +29,6 @@ import org.mapstruct.ap.internal.model.common.Assignment;
import org.mapstruct.ap.internal.model.common.Type;
import org.mapstruct.ap.internal.model.common.TypeFactory;
import static org.mapstruct.ap.internal.model.common.Assignment.AssignmentType.DIRECT;
/**
* This wrapper handles the situation where an assignment is done via the setter and a null check is needed.
* This is needed when a direct assignment is used, or if the user has chosen the appropriate strategy
@ -39,12 +39,14 @@ public class SetterWrapperForCollectionsAndMapsWithNullCheck extends WrapperForC
private final Type targetType;
private final TypeFactory typeFactory;
private final boolean mapNullToDefault;
public SetterWrapperForCollectionsAndMapsWithNullCheck(Assignment decoratedAssignment,
List<Type> thrownTypesToExclude,
Type targetType,
TypeFactory typeFactory,
boolean fieldAssignment) {
boolean fieldAssignment,
boolean mapNullToDefault) {
super(
decoratedAssignment,
thrownTypesToExclude,
@ -53,6 +55,7 @@ public class SetterWrapperForCollectionsAndMapsWithNullCheck extends WrapperForC
);
this.targetType = targetType;
this.typeFactory = typeFactory;
this.mapNullToDefault = mapNullToDefault;
}
@Override
@ -83,4 +86,9 @@ public class SetterWrapperForCollectionsAndMapsWithNullCheck extends WrapperForC
public boolean isEnumSet() {
return "java.util.EnumSet".equals( targetType.getFullyQualifiedName() );
}
public boolean isMapNullToDefault() {
return mapNullToDefault;
}
}

View File

@ -37,18 +37,21 @@ public class UpdateWrapper extends AssignmentWrapper {
private final Assignment factoryMethod;
private final Type targetImplementationType;
private final boolean includeSourceNullCheck;
private final boolean mapNullToDefault;
public UpdateWrapper( Assignment decoratedAssignment,
List<Type> thrownTypesToExclude,
Assignment factoryMethod,
boolean fieldAssignment,
Type targetType,
boolean includeSourceNullCheck ) {
boolean includeSourceNullCheck,
boolean mapNullToDefault ) {
super( decoratedAssignment, fieldAssignment );
this.thrownTypesToExclude = thrownTypesToExclude;
this.factoryMethod = factoryMethod;
this.targetImplementationType = determineImplType( factoryMethod, targetType );
this.includeSourceNullCheck = includeSourceNullCheck;
this.mapNullToDefault = mapNullToDefault;
}
private static Type determineImplType(Assignment factoryMethod, Type targetType) {
@ -100,4 +103,8 @@ public class UpdateWrapper extends AssignmentWrapper {
public boolean isIncludeSourceNullCheck() {
return includeSourceNullCheck;
}
public boolean isMapNullToDefault() {
return mapNullToDefault;
}
}

View File

@ -155,6 +155,15 @@ public class MapperConfiguration {
}
}
public NullValueMappingStrategyPrism getNullValueMappingStrategy() {
if ( mapperConfigPrism != null && mapperPrism.values.nullValueMappingStrategy() == null ) {
return NullValueMappingStrategyPrism.valueOf( mapperConfigPrism.nullValueMappingStrategy() );
}
else {
return NullValueMappingStrategyPrism.valueOf( mapperPrism.nullValueMappingStrategy() );
}
}
public boolean isMapToDefault(NullValueMappingStrategyPrism mapNullToDefault) {
// check on method level

View File

@ -28,7 +28,7 @@
${ext.targetBeanName}.${ext.targetReadAccessorName}.<#if ext.targetType.collectionType>addAll<#else>putAll</#if>( <@lib.handleWithAssignmentOrNullCheckVar/> );
</@lib.handleLocalVarNullCheck>
<#if !ext.defaultValueAssignment?? && !sourcePresenceCheckerReference?? && !includeSourceNullCheck>else {<#-- the opposite (defaultValueAssignment) case is handeld inside lib.handleLocalVarNullCheck -->
${ext.targetBeanName}.${ext.targetWriteAccessorName}<@lib.handleWrite>null</@lib.handleWrite>;
${ext.targetBeanName}.${ext.targetWriteAccessorName}<@lib.handleWrite><#if mapNullToDefault><@lib.initTargetObject/><#else>null</#if></@lib.handleWrite>;
}
</#if>
}

View File

@ -24,7 +24,7 @@
<@lib.handleExceptions>
<@callTargetWriteAccessor/>
<#if !ext.defaultValueAssignment??>else {<#-- the opposite (defaultValueAssignment) case is handeld inside lib.handleLocalVarNullCheck -->
${ext.targetBeanName}.${ext.targetWriteAccessorName}<@lib.handleWrite>null</@lib.handleWrite>;
${ext.targetBeanName}.${ext.targetWriteAccessorName}<@lib.handleWrite><#if mapNullToDefault><@lib.initTargetObject/><#else>null</#if></@lib.handleWrite>;
}
</#if>
</@lib.handleExceptions>

View File

@ -27,7 +27,7 @@
<@lib.handleAssignment/>;
}
else {
${ext.targetBeanName}.${ext.targetWriteAccessorName}<@lib.handleWrite>null</@lib.handleWrite>;
${ext.targetBeanName}.${ext.targetWriteAccessorName}<@lib.handleWrite><#if mapNullToDefault><@lib.initTargetObject/><#else>null</#if></@lib.handleWrite>;
}
<#else>
<@assignToExistingTarget/>

View File

@ -0,0 +1,35 @@
/**
* Copyright 2012-2017 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._1273;
import java.util.ArrayList;
import java.util.List;
public class Dto {
List<Long> longs = new ArrayList<Long>();
public List<Long> getLongs() {
return longs;
}
public void setLongs(List<Long> longs) {
this.longs = longs;
}
}

View File

@ -0,0 +1,35 @@
/**
* Copyright 2012-2017 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._1273;
import java.util.ArrayList;
import java.util.List;
public class Entity {
List<Long> longs = new ArrayList<Long>();
public List<Long> getLongs() {
return longs;
}
public void setLongs(List<Long> longs) {
this.longs = longs;
}
}

View File

@ -0,0 +1,29 @@
/**
* Copyright 2012-2017 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._1273;
import org.mapstruct.Mapper;
import org.mapstruct.NullValueMappingStrategy;
@Mapper( nullValueMappingStrategy = NullValueMappingStrategy.RETURN_DEFAULT )
public interface EntityMapperReturnDefault {
Dto asTarget(Entity entity);
}

View File

@ -0,0 +1,29 @@
/**
* Copyright 2012-2017 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._1273;
import org.mapstruct.Mapper;
import org.mapstruct.NullValueMappingStrategy;
@Mapper( nullValueMappingStrategy = NullValueMappingStrategy.RETURN_NULL )
public interface EntityMapperReturnNull {
Dto asTarget(Entity entity);
}

View File

@ -0,0 +1,62 @@
/**
* Copyright 2012-2017 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._1273;
import static org.assertj.core.api.AssertionsForInterfaceTypes.assertThat;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mapstruct.ap.testutil.IssueKey;
import org.mapstruct.ap.testutil.WithClasses;
import org.mapstruct.ap.testutil.runner.AnnotationProcessorTestRunner;
import org.mapstruct.factory.Mappers;
@IssueKey( "1273" )
@RunWith( AnnotationProcessorTestRunner.class )
@WithClasses( { EntityMapperReturnDefault.class, EntityMapperReturnNull.class, Dto.class, Entity.class } )
public class Issue1273Test {
@Test
public void shouldCorrectlyMapCollectionWithNullValueMappingStrategyReturnDefault() {
EntityMapperReturnDefault entityMapper = Mappers.getMapper( EntityMapperReturnDefault.class );
Entity entity = createEntityWithEmptyList();
Dto dto = entityMapper.asTarget( entity );
assertThat( dto.getLongs() ).isNotNull();
}
@Test
public void shouldCorrectlyMapCollectionWithNullValueMappingStrategyReturnNull() {
EntityMapperReturnNull entityMapper = Mappers.getMapper( EntityMapperReturnNull.class );
Entity entity = createEntityWithEmptyList();
Dto dto = entityMapper.asTarget( entity );
assertThat( dto.getLongs() ).isNull();
}
private Entity createEntityWithEmptyList() {
Entity entity = new Entity();
entity.setLongs( null );
return entity;
}
}

View File

@ -151,7 +151,7 @@ public class Issue913SetterMapperForCollectionsTest {
* 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.
*
* However, for plain mappings (strings to strings) the result will be null.
* However, for plain mappings (strings to strings) the result will also be an empty collection.
*/
@Test
public void shouldReturnDefaultForNvmsReturnDefaultForCreate() {
@ -160,7 +160,7 @@ public class Issue913SetterMapperForCollectionsTest {
Domain domain = DomainDtoWithNvmsDefaultMapper.INSTANCE.create( dto );
doControlAsserts( domain );
assertThat( domain.getStrings() ).isNull();
assertThat( domain.getStrings() ).isEmpty();
assertThat( domain.getLongs() ).isEmpty();
}
@ -169,7 +169,7 @@ public class Issue913SetterMapperForCollectionsTest {
* string to long that return default in the entire mapper, so also for the forged mapper. Note the default NVMS is
* RETURN_NULL.
*
* However, for plain mappings (strings to strings) the result will be null.
* However, for plain mappings (strings to strings) the result will also be an empty collection.
*/
@Test
public void shouldReturnDefaultForNvmsReturnDefaultForUpdate() {
@ -183,7 +183,7 @@ public class Issue913SetterMapperForCollectionsTest {
DomainDtoWithNvmsDefaultMapper.INSTANCE.update( dto, domain );
doControlAsserts( domain );
assertThat( domain.getStrings() ).isNull();
assertThat( domain.getStrings() ).isEmpty();
assertThat( domain.getLongs() ).isEmpty();
assertThat( domain.getLongs() ).isSameAs( longIn ); // make sure add all is used.
}
@ -194,7 +194,7 @@ public class Issue913SetterMapperForCollectionsTest {
* mapper. Note the default NVMS is
* RETURN_NULL.
*
* However, for plain mappings (strings to strings) the result will be null.
* However, for plain mappings (strings to strings) the result will also be an empty collection.
*
*/
@Test
@ -212,9 +212,9 @@ public class Issue913SetterMapperForCollectionsTest {
assertThat( domain2 ).isSameAs( domain1 );
doControlAsserts( domain1, domain2 );
assertThat( domain1.getLongs() ).isEqualTo( domain2.getLongs() );
assertThat( domain1.getStrings() ).isNull();
assertThat( domain1.getStrings() ).isEmpty();
assertThat( domain1.getLongs() ).isEmpty();
assertThat( domain2.getStrings() ).isNull();
assertThat( domain2.getStrings() ).isEmpty();
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

View File

@ -46,7 +46,7 @@ public class DomainDtoWithNvmsDefaultMapperImpl implements DomainDtoWithNvmsDefa
domain.setStrings( new HashSet<String>( list ) );
}
else {
domain.setStrings( null );
domain.setStrings( new HashSet<String>() );
}
List<String> list1 = source.getStringsWithDefault();
if ( list1 != null ) {
@ -60,7 +60,7 @@ public class DomainDtoWithNvmsDefaultMapperImpl implements DomainDtoWithNvmsDefa
domain.setStringsInitialized( new HashSet<String>( list2 ) );
}
else {
domain.setStringsInitialized( null );
domain.setStringsInitialized( new HashSet<String>() );
}
}
@ -78,7 +78,7 @@ public class DomainDtoWithNvmsDefaultMapperImpl implements DomainDtoWithNvmsDefa
target.getLongs().addAll( set );
}
else {
target.setLongs( null );
target.setLongs( new HashSet<Long>() );
}
}
else {
@ -94,7 +94,7 @@ public class DomainDtoWithNvmsDefaultMapperImpl implements DomainDtoWithNvmsDefa
target.getStrings().addAll( list );
}
else {
target.setStrings( null );
target.setStrings( new HashSet<String>() );
}
}
else {
@ -110,7 +110,7 @@ public class DomainDtoWithNvmsDefaultMapperImpl implements DomainDtoWithNvmsDefa
target.getLongsInitialized().addAll( set1 );
}
else {
target.setLongsInitialized( null );
target.setLongsInitialized( new HashSet<Long>() );
}
}
else {
@ -145,7 +145,7 @@ public class DomainDtoWithNvmsDefaultMapperImpl implements DomainDtoWithNvmsDefa
target.getStringsInitialized().addAll( list2 );
}
else {
target.setStringsInitialized( null );
target.setStringsInitialized( new HashSet<String>() );
}
}
else {
@ -168,7 +168,7 @@ public class DomainDtoWithNvmsDefaultMapperImpl implements DomainDtoWithNvmsDefa
target.getLongs().addAll( set );
}
else {
target.setLongs( null );
target.setLongs( new HashSet<Long>() );
}
}
else {
@ -184,7 +184,7 @@ public class DomainDtoWithNvmsDefaultMapperImpl implements DomainDtoWithNvmsDefa
target.getStrings().addAll( list );
}
else {
target.setStrings( null );
target.setStrings( new HashSet<String>() );
}
}
else {
@ -200,7 +200,7 @@ public class DomainDtoWithNvmsDefaultMapperImpl implements DomainDtoWithNvmsDefa
target.getLongsInitialized().addAll( set1 );
}
else {
target.setLongsInitialized( null );
target.setLongsInitialized( new HashSet<Long>() );
}
}
else {
@ -235,7 +235,7 @@ public class DomainDtoWithNvmsDefaultMapperImpl implements DomainDtoWithNvmsDefa
target.getStringsInitialized().addAll( list2 );
}
else {
target.setStringsInitialized( null );
target.setStringsInitialized( new HashSet<String>() );
}
}
else {