From 4832369148a0ffbebf5781be5fe950dd92c56c4d Mon Sep 17 00:00:00 2001 From: Andreas Gudian Date: Fri, 28 Oct 2016 20:14:51 +0200 Subject: [PATCH] #892 If multiple setters exist, don't override the setter if it doesn't match the property getters type. --- .../ap/internal/model/common/Type.java | 51 +++++++++++----- .../ap/test/bugs/_892/Issue892Mapper.java | 58 +++++++++++++++++++ .../ap/test/bugs/_892/Issue892Test.java | 47 +++++++++++++++ 3 files changed, 142 insertions(+), 14 deletions(-) create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_892/Issue892Mapper.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_892/Issue892Test.java diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/common/Type.java b/processor/src/main/java/org/mapstruct/ap/internal/model/common/Type.java index 1dfdf5f33..5a6b6ac6f 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/common/Type.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/common/Type.java @@ -414,10 +414,8 @@ public class Type extends ModelElement implements Comparable { * @return an unmodifiable map of all write accessors indexed by property name */ public Map getPropertyWriteAccessors( CollectionMappingStrategyPrism cmStrategy ) { - // collect all candidate target accessors - List candidates = new ArrayList(); - candidates.addAll( getSetters() ); + List candidates = new ArrayList( getSetters() ); candidates.addAll( getAlternativeTargetAccessors() ); Map result = new LinkedHashMap(); @@ -425,27 +423,27 @@ public class Type extends ModelElement implements Comparable { for ( ExecutableElement candidate : candidates ) { String targetPropertyName = Executables.getPropertyName( candidate ); + ExecutableElement readAccessor = getPropertyReadAccessors().get( targetPropertyName ); + + Type preferredType = determinePreferredType( readAccessor ); + Type targetType = determineTargetType( candidate ); + // A target access is in general a setter method on the target object. However, in case of collections, // the current target accessor can also be a getter method. // The following if block, checks if the target accessor should be overruled by an add method. if ( cmStrategy == CollectionMappingStrategyPrism.SETTER_PREFERRED - || cmStrategy == CollectionMappingStrategyPrism.ADDER_PREFERRED ) { + || cmStrategy == CollectionMappingStrategyPrism.ADDER_PREFERRED ) { // first check if there's a setter method. ExecutableElement adderMethod = null; - if ( Executables.isSetterMethod( candidate ) ) { - Type targetType = typeFactory.getSingleParameter( (DeclaredType) typeMirror, candidate ).getType(); + if ( Executables.isSetterMethod( candidate ) // ok, the current accessor is a setter. So now the strategy determines what to use - if ( cmStrategy == CollectionMappingStrategyPrism.ADDER_PREFERRED ) { - adderMethod = getAdderForType( targetType, targetPropertyName ); - } + && cmStrategy == CollectionMappingStrategyPrism.ADDER_PREFERRED ) { + adderMethod = getAdderForType( targetType, targetPropertyName ); } else if ( Executables.isGetterMethod( candidate ) ) { - // the current accessor is a getter (no setter available). But still, an add method is according + // the current accessor is a getter (no setter available). But still, an add method is according // to the above strategy (SETTER_PREFERRED || ADDER_PREFERRED) preferred over the getter. - Type targetType = typeFactory.getReturnType( - typeFactory.getMethodType( (DeclaredType) typeMirror, candidate ) - ); adderMethod = getAdderForType( targetType, targetPropertyName ); } if ( adderMethod != null ) { @@ -454,11 +452,36 @@ public class Type extends ModelElement implements Comparable { } } - result.put( targetPropertyName, candidate ); + ExecutableElement previousCandidate = result.get( targetPropertyName ); + if ( previousCandidate == null || preferredType == null || ( targetType != null + && typeUtils.isAssignable( preferredType.getTypeMirror(), targetType.getTypeMirror() ) ) ) { + result.put( targetPropertyName, candidate ); + } } + return result; } + private Type determinePreferredType(ExecutableElement readAccessor) { + if ( readAccessor != null ) { + return typeFactory.getReturnType( + typeFactory.getMethodType( (DeclaredType) typeMirror, readAccessor ) ); + } + return null; + } + + private Type determineTargetType(ExecutableElement candidate) { + Parameter parameter = typeFactory.getSingleParameter( (DeclaredType) typeMirror, candidate ); + if ( parameter != null ) { + return parameter.getType(); + } + else if ( Executables.isGetterMethod( candidate ) ) { + return typeFactory.getReturnType( + typeFactory.getMethodType( (DeclaredType) typeMirror, candidate ) ); + } + return null; + } + private List getAllExecutables() { if ( allExecutables == null ) { allExecutables = Executables.getAllEnclosedExecutableElements( elementUtils, typeElement ); diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_892/Issue892Mapper.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_892/Issue892Mapper.java new file mode 100644 index 000000000..32817ebd6 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_892/Issue892Mapper.java @@ -0,0 +1,58 @@ +/** + * 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._892; + +import org.mapstruct.Mapper; + +/** + * @author Andreas Gudian + * + */ +@Mapper +public interface Issue892Mapper { + Target toTarget(Source source); + + class Source { + private int type; + + public int getType() { + return type; + } + + public void setType(int type) { + this.type = type; + } + } + + class Target { + private int type; + + public int getType() { + return type; + } + + public void setType(int type) { + this.type = type; + } + + public void setType(String typeInHex) { + this.type = Integer.parseInt( typeInHex, 16 ); + } + } +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_892/Issue892Test.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_892/Issue892Test.java new file mode 100644 index 000000000..03f9ea477 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_892/Issue892Test.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._892; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mapstruct.ap.test.bugs._892.Issue892Mapper.Source; +import org.mapstruct.ap.test.bugs._892.Issue892Mapper.Target; +import org.mapstruct.ap.testutil.WithClasses; +import org.mapstruct.ap.testutil.runner.AnnotationProcessorTestRunner; +import org.mapstruct.factory.Mappers; + +/** + * When having two setter methods with the same name, choose the one with the argument type matching the getter method. + * + * @author Andreas Gudian + */ +@RunWith(AnnotationProcessorTestRunner.class) +@WithClasses({ Issue892Mapper.class }) +public class Issue892Test { + @Test + public void compiles() { + Source src = new Source(); + src.setType( 42 ); + + Target target = Mappers.getMapper( Issue892Mapper.class ).toTarget( src ); + assertThat( target.getType() ).isEqualTo( 42 ); + } +}