#891 Implementation and unit test

This commit is contained in:
sjaakd 2016-09-14 21:52:15 +02:00
parent ecd6411f93
commit 750af10900
18 changed files with 317 additions and 61 deletions

View File

@ -50,8 +50,5 @@ public class AssignmentFactory {
return new MethodReference( method, contextParam );
}
public static Direct createDirect(String sourceRef) {
return new Direct( sourceRef );
}
}

View File

@ -34,9 +34,12 @@ import org.mapstruct.ap.internal.model.common.Type;
public class Direct extends ModelElement implements Assignment {
private final String sourceReference;
private final Type sourceType;
private String sourceLocalVarName;
public Direct( String sourceReference ) {
public Direct(String sourceReference, Type sourceType ) {
this.sourceReference = sourceReference;
this.sourceType = sourceType;
}
@Override
@ -44,6 +47,21 @@ public class Direct extends ModelElement implements Assignment {
return sourceReference;
}
@Override
public Type getSourceType() {
return sourceType;
}
@Override
public String getSourceLocalVarName() {
return sourceLocalVarName;
}
@Override
public void setSourceLocalVarName(String sourceLocalVarName) {
this.sourceLocalVarName = sourceLocalVarName;
}
@Override
public Set<Type> getImportTypes() {
return Collections.emptySet();

View File

@ -108,7 +108,7 @@ public class IterableMappingMethod extends MappingMethod {
null, // there is no targetPropertyName
formattingParameters,
selectionParameters,
loopVariableName,
new Direct( loopVariableName, sourceElementType ),
false
);

View File

@ -111,7 +111,7 @@ public class MapMappingMethod extends MappingMethod {
null, // there is no targetPropertyName
keyFormattingParameters,
keySelectionParameters,
"entry.getKey()",
new Direct( "entry.getKey()", keySourceType ),
false
);
@ -138,7 +138,7 @@ public class MapMappingMethod extends MappingMethod {
null, // there is no targetPropertyName
valueFormattingParameters,
valueSelectionParameters,
"entry.getValue()",
new Direct( "entry.getValue()", valueSourceType ),
false
);

View File

@ -81,7 +81,7 @@ public class MappingBuilderContext {
* @param targetPropertyName name of the target property
* @param formattingParameters used for formatting dates and numbers
* @param selectionParameters parameters used in the selection process
* @param sourceReference call to source type as string
* @param sourceReference source information
* @param preferUpdateMethods selection should prefer update methods when present.
*
* @return an assignment to a method parameter, which can either be:
@ -95,7 +95,7 @@ public class MappingBuilderContext {
@SuppressWarnings("checkstyle:parameternumber")
Assignment getTargetAssignment(Method mappingMethod, String mappedElement, Type sourceType, Type targetType,
String targetPropertyName, FormattingParameters formattingParameters,
SelectionParameters selectionParameters, String sourceReference,
SelectionParameters selectionParameters, Direct sourceReference,
boolean preferUpdateMethods);
/**

View File

@ -128,6 +128,21 @@ public class MethodReference extends MappingMethod implements Assignment {
return assignment.getSourceReference();
}
@Override
public Type getSourceType() {
return assignment.getSourceType();
}
@Override
public String getSourceLocalVarName() {
return assignment.getSourceLocalVarName();
}
@Override
public void setSourceLocalVarName(String sourceLocalVarName) {
assignment.setSourceLocalVarName( sourceLocalVarName );
}
/**
* @return the type of the single source parameter that is not the {@code @TargetType} parameter
*/

View File

@ -216,15 +216,15 @@ public class PropertyMapping extends ModelElement {
// handle source
String sourceElement = getSourceElement();
Type sourceType = getSourceType();
String sourceRefStr;
Direct source;
if ( targetWriteAccessorType == TargetWriteAccessorType.ADDER && sourceType.isCollectionType() ) {
// handle adder, if source is collection then use iterator element type as source type.
// sourceRef becomes a local variable in the itereation.
sourceType = sourceType.getTypeParameters().get( 0 );
sourceRefStr = Executables.getElementNameForAdder( targetWriteAccessor );
source = new Direct( Executables.getElementNameForAdder( targetWriteAccessor ), sourceType );
}
else {
sourceRefStr = getSourceRef();
source = getSource();
}
// all the tricky cases will be excluded for the time being.
@ -244,17 +244,17 @@ public class PropertyMapping extends ModelElement {
targetPropertyName,
formattingParameters,
selectionParameters,
sourceRefStr,
source,
preferUpdateMethods
);
// No mapping found. Try to forge a mapping
if ( assignment == null ) {
if ( (sourceType.isCollectionType() || sourceType.isArrayType()) && targetType.isIterableType() ) {
assignment = forgeIterableMapping( sourceType, targetType, sourceRefStr, method.getExecutable() );
assignment = forgeIterableMapping( sourceType, targetType, source, method.getExecutable() );
}
else if ( sourceType.isMapType() && targetType.isMapType() ) {
assignment = forgeMapMapping( sourceType, targetType, sourceRefStr, method.getExecutable() );
assignment = forgeMapMapping( sourceType, targetType, source, method.getExecutable() );
}
}
@ -354,12 +354,6 @@ public class PropertyMapping extends ModelElement {
return result;
}
// a nested source reference will take the bean mapping parameter itself, no null check is required
// since this is handled by the BeanMapping
if ( sourceReference.getPropertyEntries().size() > 1 ) {
return result;
}
// add a null / presence checked when required
if ( sourceType.isPrimitive() ) {
if ( getSourcePresenceCheckerRef() != null ) {
@ -376,6 +370,7 @@ public class PropertyMapping extends ModelElement {
}
else if ( ALWAYS.equals( method.getMapperConfiguration().getNullValueCheckStrategy() ) ) {
result = new NullCheckWrapper( result, getSourcePresenceCheckerRef() );
useLocalVarWhenNested( result );
}
else if ( result.getType() == TYPE_CONVERTED
|| result.getType() == TYPE_CONVERTED_MAPPED
@ -384,12 +379,20 @@ public class PropertyMapping extends ModelElement {
// for primitive types null check is not possible at all, but a conversion needs
// a null check.
result = new NullCheckWrapper( result, getSourcePresenceCheckerRef() );
useLocalVarWhenNested( result );
}
}
return result;
}
private void useLocalVarWhenNested(Assignment rightHandSide) {
if ( sourceReference.getPropertyEntries().size() > 1 ) {
String sourceTypeName = rightHandSide.getSourceType().getName();
String safeName = Strings.getSaveVariableName( sourceTypeName, existingVariableNames );
rightHandSide.setSourceLocalVarName( safeName );
}
}
private Assignment assignToPlainViaAdder(Type sourceType, Assignment rightHandSide) {
@ -399,7 +402,7 @@ public class PropertyMapping extends ModelElement {
result = new AdderWrapper(
result,
method.getThrownTypes(),
getSourceRef(),
getSource().getSourceReference(),
sourceType
);
result = new NullCheckWrapper( result, getSourcePresenceCheckerRef() );
@ -524,18 +527,19 @@ public class PropertyMapping extends ModelElement {
}
}
private String getSourceRef() {
private Direct getSource() {
Parameter sourceParam = sourceReference.getParameter();
List<PropertyEntry> propertyEntries = sourceReference.getPropertyEntries();
// parameter reference
if ( propertyEntries.isEmpty() ) {
return sourceParam.getName();
return new Direct( sourceParam.getName(), getSourceType() );
}
// simple property
else if ( propertyEntries.size() == 1 ) {
PropertyEntry propertyEntry = propertyEntries.get( 0 );
return sourceParam.getName() + "." + propertyEntry.getReadAccessor().getSimpleName() + "()";
return new Direct( sourceParam.getName()
+ "." + propertyEntry.getReadAccessor().getSimpleName() + "()", propertyEntry.getType() );
}
// nested property given as dot path
else {
@ -567,8 +571,8 @@ public class PropertyMapping extends ModelElement {
else {
forgedName = ctx.getExistingMappingMethod( nestedPropertyMapping ).getName();
}
return new Direct( forgedName + "( " + sourceParam.getName() + " )", getSourceType() );
return forgedName + "( " + sourceParam.getName() + " )";
}
}
@ -606,7 +610,7 @@ public class PropertyMapping extends ModelElement {
return sourcePresenceChecker;
}
private Assignment forgeIterableMapping(Type sourceType, Type targetType, String sourceReference,
private Assignment forgeIterableMapping(Type sourceType, Type targetType, Direct source,
ExecutableElement element) {
Assignment assignment = null;
@ -635,13 +639,13 @@ public class PropertyMapping extends ModelElement {
}
assignment = AssignmentFactory.createMethodReference( methodRef, null, targetType );
assignment.setAssignment( AssignmentFactory.createDirect( sourceReference ) );
assignment.setAssignment( source );
}
return assignment;
}
private Assignment forgeMapMapping(Type sourceType, Type targetType, String sourceReference,
private Assignment forgeMapMapping(Type sourceType, Type targetType, Direct source,
ExecutableElement element) {
Assignment assignment = null;
@ -668,7 +672,7 @@ public class PropertyMapping extends ModelElement {
methodRef = new ForgedMethod( existingName, methodRef );
}
assignment = AssignmentFactory.createMethodReference( methodRef, null, targetType );
assignment.setAssignment( AssignmentFactory.createDirect( sourceReference ) );
assignment.setAssignment( source );
}
return assignment;
@ -726,7 +730,7 @@ public class PropertyMapping extends ModelElement {
targetPropertyName,
formattingParameters,
selectionParameters,
constantExpression,
new Direct( constantExpression, sourceType ),
method.getMappingTargetParameter() != null
);
}
@ -793,7 +797,7 @@ public class PropertyMapping extends ModelElement {
// String String quotation marks.
String enumExpression = constantExpression.substring( 1, constantExpression.length() - 1 );
if ( targetType.getEnumConstants().contains( enumExpression ) ) {
assignment = AssignmentFactory.createDirect( enumExpression );
assignment = new Direct( enumExpression, targetType );
assignment = new EnumConstantWrapper( assignment, targetType );
}
else {
@ -819,7 +823,7 @@ public class PropertyMapping extends ModelElement {
}
public PropertyMapping build() {
Assignment assignment = AssignmentFactory.createDirect( javaExpression );
Assignment assignment = new Direct( javaExpression, null );
if ( Executables.isSetterMethod( targetWriteAccessor ) ) {
// setter, so wrap in setter

View File

@ -86,6 +86,21 @@ public class TypeConversion extends ModelElement implements Assignment {
return assignment.getSourceReference();
}
@Override
public Type getSourceType() {
return assignment.getSourceType();
}
@Override
public String getSourceLocalVarName() {
return assignment.getSourceLocalVarName();
}
@Override
public void setSourceLocalVarName(String sourceLocalVarName) {
assignment.setSourceLocalVarName( sourceLocalVarName );
}
@Override
public void setAssignment( Assignment assignment ) {
this.assignment = assignment;

View File

@ -71,12 +71,32 @@ public interface Assignment {
void setAssignment(Assignment assignment);
/**
* the source reference being a source-getter, a constant, etc.
* the source reference being a source-getter, a constant, nested method call, etc.
*
* @return source reference
*/
String getSourceReference();
/**
* the source type.
*
* @return source type (can be null)
*/
Type getSourceType();
/**
* a local variable name for supporting a null check and avoiding executing a nested method forged method twice
*
* @return local variable name (can be null)
*/
String getSourceLocalVarName();
/**
* Use sourceLocalVarName iso sourceReference
* @param sourceLocalVarName source local variable name
*/
void setSourceLocalVarName(String sourceLocalVarName);
/**
* Returns whether the type of assignment
*

View File

@ -61,6 +61,21 @@ public abstract class AssignmentWrapper extends ModelElement implements Assignme
return decoratedAssignment.getSourceReference();
}
@Override
public Type getSourceType() {
return decoratedAssignment.getSourceType();
}
@Override
public String getSourceLocalVarName() {
return decoratedAssignment.getSourceLocalVarName();
}
@Override
public void setSourceLocalVarName(String sourceLocalVarName) {
decoratedAssignment.setSourceLocalVarName( sourceLocalVarName );
}
@Override
public AssignmentType getType() {
return decoratedAssignment.getType();

View File

@ -36,6 +36,7 @@ import javax.lang.model.util.Types;
import org.mapstruct.ap.internal.conversion.ConversionProvider;
import org.mapstruct.ap.internal.conversion.Conversions;
import org.mapstruct.ap.internal.model.AssignmentFactory;
import org.mapstruct.ap.internal.model.Direct;
import org.mapstruct.ap.internal.model.MapperReference;
import org.mapstruct.ap.internal.model.MappingBuilderContext.MappingResolver;
import org.mapstruct.ap.internal.model.MethodReference;
@ -108,7 +109,7 @@ public class MappingResolverImpl implements MappingResolver {
@SuppressWarnings("checkstyle:parameternumber")
public Assignment getTargetAssignment(Method mappingMethod, String mappedElement, Type sourceType,
Type targetType, String targetPropertyName, FormattingParameters formattingParameters,
SelectionParameters selectionParameters, String sourceReference, boolean preferUpdateMapping) {
SelectionParameters selectionParameters, Direct sourceReference, boolean preferUpdateMapping) {
SelectionCriteria criteria =
new SelectionCriteria( selectionParameters, targetPropertyName, preferUpdateMapping );
@ -171,7 +172,7 @@ public class MappingResolverImpl implements MappingResolver {
private final String dateFormat;
private final String numberFormat;
private final SelectionCriteria selectionCriteria;
private final String sourceReference;
private final Direct sourceReference;
private final boolean savedPreferUpdateMapping;
// resolving via 2 steps creates the possibillity of wrong matches, first builtin method matches,
@ -180,7 +181,7 @@ public class MappingResolverImpl implements MappingResolver {
private final Set<VirtualMappingMethod> virtualMethodCandidates;
private ResolvingAttempt(List<SourceMethod> sourceModel, Method mappingMethod, String mappedElement,
String dateFormat, String numberFormat, String sourceReference, SelectionCriteria criteria) {
String dateFormat, String numberFormat, Direct sourceReference, SelectionCriteria criteria) {
this.mappingMethod = mappingMethod;
this.mappedElement = mappedElement;
@ -209,28 +210,28 @@ public class MappingResolverImpl implements MappingResolver {
// first simple mapping method
Assignment referencedMethod = resolveViaMethod( sourceType, targetType, false );
if ( referencedMethod != null ) {
referencedMethod.setAssignment( AssignmentFactory.createDirect( sourceReference ) );
referencedMethod.setAssignment( sourceReference );
return referencedMethod;
}
// then direct assignable
if ( sourceType.isAssignableTo( targetType ) ||
isAssignableThroughCollectionCopyConstructor( sourceType, targetType ) ) {
Assignment simpleAssignment = AssignmentFactory.createDirect( sourceReference );
Assignment simpleAssignment = sourceReference;
return simpleAssignment;
}
// then type conversion
Assignment conversion = resolveViaConversion( sourceType, targetType );
if ( conversion != null ) {
conversion.setAssignment( AssignmentFactory.createDirect( sourceReference ) );
conversion.setAssignment( sourceReference );
return conversion;
}
// check for a built-in method
Assignment builtInMethod = resolveViaBuiltInMethod( sourceType, targetType );
if ( builtInMethod != null ) {
builtInMethod.setAssignment( AssignmentFactory.createDirect( sourceReference ) );
builtInMethod.setAssignment( sourceReference );
usedVirtualMappings.addAll( virtualMethodCandidates );
return builtInMethod;
}
@ -309,7 +310,7 @@ public class MappingResolverImpl implements MappingResolver {
sourceType,
targetType, dateFormat, numberFormat);
Assignment methodReference = AssignmentFactory.createMethodReference( matchingBuiltInMethod, ctx );
methodReference.setAssignment( AssignmentFactory.createDirect( sourceReference ) );
methodReference.setAssignment( sourceReference );
return methodReference;
}
@ -350,7 +351,7 @@ public class MappingResolverImpl implements MappingResolver {
selectionCriteria.setPreferUpdateMapping( savedPreferUpdateMapping );
if ( methodRefX != null ) {
methodRefY.setAssignment( methodRefX );
methodRefX.setAssignment( AssignmentFactory.createDirect( sourceReference ) );
methodRefX.setAssignment( sourceReference );
break;
}
else {
@ -387,7 +388,7 @@ public class MappingResolverImpl implements MappingResolver {
resolveViaConversion( sourceType, methodYCandidate.getSourceParameters().get( 0 ).getType() );
if ( conversionXRef != null ) {
methodRefY.setAssignment( conversionXRef );
conversionXRef.setAssignment( AssignmentFactory.createDirect( sourceReference ) );
conversionXRef.setAssignment( sourceReference );
break;
}
else {
@ -430,7 +431,7 @@ public class MappingResolverImpl implements MappingResolver {
conversionYRef = resolveViaConversion( methodXCandidate.getReturnType(), targetType );
if ( conversionYRef != null ) {
conversionYRef.setAssignment( methodRefX );
methodRefX.setAssignment( AssignmentFactory.createDirect( sourceReference ) );
methodRefX.setAssignment( sourceReference );
break;
}
else {

View File

@ -18,4 +18,4 @@
limitations under the License.
-->
${sourceReference}
<#if sourceLocalVarName??>${sourceLocalVarName}<#else>${sourceReference}</#if>

View File

@ -18,22 +18,27 @@
limitations under the License.
-->
if ( <#if sourcePresenceChecker?? >${sourcePresenceChecker}<#else>${sourceReference} != null</#if> ) {
<@includeModel object=assignment
targetBeanName=ext.targetBeanName
existingInstanceMapping=ext.existingInstanceMapping
targetReadAccessorName=ext.targetReadAccessorName
targetWriteAccessorName=ext.targetWriteAccessorName
targetType=ext.targetType
defaultValue=ext.defaultValueAssignment/>
<#if sourceLocalVarName??>
<@includeModel object=sourceType/> ${sourceLocalVarName} = ${sourceReference};
if ( ${sourceLocalVarName} != null ) {
<@_assignment object=assignment defaultValue=ext.defaultValueAssignment/>
}
<#else>
if ( <#if sourcePresenceChecker?? >${sourcePresenceChecker}<#else>${sourceReference} != null</#if> ) {
<@_assignment object=assignment defaultValue=ext.defaultValueAssignment/>
}
</#if>
<#if ext.defaultValueAssignment?? >
else {
<@includeModel object=ext.defaultValueAssignment
targetBeanName=ext.targetBeanName
existingInstanceMapping=ext.existingInstanceMapping
targetReadAccessorName=ext.targetReadAccessorName
targetWriteAccessorName=ext.targetWriteAccessorName
targetType=ext.targetType/>
<@_assignment object=ext.defaultValueAssignment/>
}
</#if>
</#if>
<#macro _assignment object defaultValue="">
<@includeModel object=object
targetBeanName=ext.targetBeanName
existingInstanceMapping=ext.existingInstanceMapping
targetReadAccessorName=ext.targetReadAccessorName
targetWriteAccessorName=ext.targetWriteAccessorName
targetType=ext.targetType
defaultValue=defaultValue/>
</#macro>

View File

@ -0,0 +1,36 @@
/**
* 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._891;
import org.mapstruct.Mapper;
import org.mapstruct.Mapping;
import org.mapstruct.factory.Mappers;
/**
*
* @author Sjaak Derksen
*/
@Mapper
public interface BuggyMapper {
BuggyMapper INSTANCE = Mappers.getMapper( BuggyMapper.class );
@Mapping(source = "nested.propInt", target = "propLong")
Dest convert(Src src);
}

View File

@ -0,0 +1,30 @@
/**
* 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._891;
/**
*
* @author Sjaak Derksen
*/
public class Dest {
public void setPropLong(Long l) {
}
}

View File

@ -0,0 +1,38 @@
/**
* 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._891;
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({BuggyMapper.class, Dest.class, Src.class, SrcNested.class})
public class Issue891Test {
@Test
public void shouldNotThrowNPE() {
BuggyMapper.INSTANCE.convert( new Src() );
}
}

View File

@ -0,0 +1,31 @@
/**
* 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._891;
/**
*
* @author Sjaak Derksen
*/
public class Src {
public SrcNested getNested() {
return null;
}
}

View File

@ -0,0 +1,31 @@
/**
* 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._891;
/**
*
* @author Sjaak Derksen
*/
public class SrcNested {
public Integer getPropInt() {
return 1;
}
}