From e5cfb07af52f459b950a5b75169c16813b914403 Mon Sep 17 00:00:00 2001 From: sjaakd Date: Mon, 13 Jan 2014 11:23:35 +0100 Subject: [PATCH] issue 92 solution for list getter without setter --- .../mapstruct/ap/model/PropertyMapping.java | 6 ++++ .../java/org/mapstruct/ap/model/Type.java | 9 ++++- .../ap/processor/MapperCreationProcessor.java | 13 +++++-- .../org/mapstruct/ap/util/Executables.java | 1 + .../java/org/mapstruct/ap/util/Filters.java | 24 +++++++++++++ .../org/mapstruct/ap/util/TypeFactory.java | 8 +++++ ...org.mapstruct.ap.model.PropertyMapping.ftl | 10 ++++++ .../collection/CollectionMappingTest.java | 12 +++++++ .../mapstruct/ap/test/collection/Source.java | 11 ++++++ .../test/collection/SourceTargetMapper.java | 3 +- .../mapstruct/ap/test/collection/Target.java | 10 ++++++ .../DefaultCollectionImplementationTest.java | 13 +++++++ .../defaultimplementation/Source.java | 35 +++++++++++++++++++ .../SourceTargetMapper.java | 4 +++ .../defaultimplementation/Target.java | 34 ++++++++++++++++++ 15 files changed, 189 insertions(+), 4 deletions(-) create mode 100644 processor/src/test/java/org/mapstruct/ap/test/collection/defaultimplementation/Source.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/collection/defaultimplementation/Target.java diff --git a/processor/src/main/java/org/mapstruct/ap/model/PropertyMapping.java b/processor/src/main/java/org/mapstruct/ap/model/PropertyMapping.java index a4fe8e4c8..780348a23 100644 --- a/processor/src/main/java/org/mapstruct/ap/model/PropertyMapping.java +++ b/processor/src/main/java/org/mapstruct/ap/model/PropertyMapping.java @@ -39,6 +39,7 @@ public class PropertyMapping extends AbstractModelElement { private final String targetName; private final String targetAccessorName; private final Type targetType; + private final boolean isHasTargetSetter; private final MappingMethodReference mappingMethod; private final TypeConversion conversion; @@ -55,6 +56,7 @@ public class PropertyMapping extends AbstractModelElement { this.targetName = targetName; this.targetAccessorName = targetAccessorName; this.targetType = targetType; + this.isHasTargetSetter = targetAccessorName.startsWith( "set" ); this.mappingMethod = mappingMethod; this.conversion = conversion; @@ -96,6 +98,10 @@ public class PropertyMapping extends AbstractModelElement { return conversion; } + public boolean isHasTargetSetter() { + return isHasTargetSetter; + } + @Override public Set getImportTypes() { Set importTypes = new HashSet(); diff --git a/processor/src/main/java/org/mapstruct/ap/model/Type.java b/processor/src/main/java/org/mapstruct/ap/model/Type.java index 670586403..666a0a361 100644 --- a/processor/src/main/java/org/mapstruct/ap/model/Type.java +++ b/processor/src/main/java/org/mapstruct/ap/model/Type.java @@ -53,6 +53,7 @@ public class Type extends AbstractModelElement implements Comparable { private final boolean isEnumType; private final boolean isIterableType; private final boolean isCollectionType; + private final boolean isListType; private final boolean isMapType; private final Type implementationType; private final TypeMirror typeMirror; @@ -60,12 +61,14 @@ public class Type extends AbstractModelElement implements Comparable { private final TypeElement typeElement; public Type(TypeMirror typeMirror, List typeParameters, Type implementationType, boolean isIterableType, - boolean isCollectionType, boolean isMapType, Types typeUtils, Elements elementUtils) { + boolean isCollectionType, boolean isListType, boolean isMapType, Types typeUtils, + Elements elementUtils) { this.typeMirror = typeMirror; this.implementationType = implementationType; this.typeParameters = typeParameters; this.isIterableType = isIterableType; this.isCollectionType = isCollectionType; + this.isListType = isListType; this.isMapType = isMapType; this.typeUtils = typeUtils; @@ -149,6 +152,10 @@ public class Type extends AbstractModelElement implements Comparable { return isCollectionType; } + public boolean isListType() { + return isListType; + } + public boolean isMapType() { return isMapType; } diff --git a/processor/src/main/java/org/mapstruct/ap/processor/MapperCreationProcessor.java b/processor/src/main/java/org/mapstruct/ap/processor/MapperCreationProcessor.java index 0b227911f..6764863d1 100644 --- a/processor/src/main/java/org/mapstruct/ap/processor/MapperCreationProcessor.java +++ b/processor/src/main/java/org/mapstruct/ap/processor/MapperCreationProcessor.java @@ -472,14 +472,23 @@ public class MapperCreationProcessor implements ModelElementProcessor setterMethodsIn(Iterable elements) { List setterMethods = new LinkedList(); + List getterMethods = new LinkedList(); for ( ExecutableElement method : methodsIn( elements ) ) { if ( executables.isSetterMethod( method ) ) { setterMethods.add( method ); } + else if ( executables.isGetterMethod( method ) ) { + getterMethods.add( method ); + } + } + + if (getterMethods.size() > setterMethods.size()) { + // there could be a getter method for a list that is not present as setter. + // a getter could substitue the setter in that case and act as setter. + // (asuming its intitialized) + for ( ExecutableElement getterMethod : getterMethods ) { + boolean matchFound = false; + String getterPropertyName = executables.getPropertyName( getterMethod ); + for ( ExecutableElement setterMethod : setterMethods ) { + String setterPropertyName = executables.getPropertyName( setterMethod ); + if ( getterPropertyName.equals( setterPropertyName ) ) { + matchFound = true; + break; + } + } + if ( !matchFound && executables.retrieveReturnType( getterMethod ).isListType() ) { + setterMethods.add( getterMethod ); + } + } } return setterMethods; diff --git a/processor/src/main/java/org/mapstruct/ap/util/TypeFactory.java b/processor/src/main/java/org/mapstruct/ap/util/TypeFactory.java index 728c0ad76..64bfe6c0b 100644 --- a/processor/src/main/java/org/mapstruct/ap/util/TypeFactory.java +++ b/processor/src/main/java/org/mapstruct/ap/util/TypeFactory.java @@ -57,6 +57,7 @@ public class TypeFactory { private final TypeMirror iterableType; private final TypeMirror collectionType; + private final TypeMirror listType; private final TypeMirror mapType; private final Map implementationTypes = new HashMap(); @@ -70,6 +71,7 @@ public class TypeFactory { elementUtils.getTypeElement( Collection.class.getCanonicalName() ) .asType() ); + listType = typeUtils.erasure( elementUtils.getTypeElement( List.class.getCanonicalName() ).asType() ); mapType = typeUtils.erasure( elementUtils.getTypeElement( Map.class.getCanonicalName() ).asType() ); implementationTypes.put( Iterable.class.getName(), getType( ArrayList.class ) ); @@ -115,6 +117,10 @@ public class TypeFactory { mirror, collectionType ); + boolean isListType = typeUtils.isSubtype( + mirror, + listType + ); boolean isMapType = typeUtils.isSubtype( mirror, mapType @@ -126,6 +132,7 @@ public class TypeFactory { implementationType, isIterableType, isCollectionType, + isListType, isMapType, typeUtils, elementUtils @@ -181,6 +188,7 @@ public class TypeFactory { null, implementationType.isIterableType(), implementationType.isCollectionType(), + implementationType.isListType(), implementationType.isMapType(), typeUtils, elementUtils diff --git a/processor/src/main/resources/org.mapstruct.ap.model.PropertyMapping.ftl b/processor/src/main/resources/org.mapstruct.ap.model.PropertyMapping.ftl index 58190b30a..84660f74c 100644 --- a/processor/src/main/resources/org.mapstruct.ap.model.PropertyMapping.ftl +++ b/processor/src/main/resources/org.mapstruct.ap.model.PropertyMapping.ftl @@ -20,7 +20,13 @@ --> <#-- a) invoke mapping method --> <#if mappingMethod??> +<#if hasTargetSetter> ${ext.targetBeanName}.${targetAccessorName}( <@includeModel object=mappingMethod input="${sourceBeanName}.${sourceAccessorName}()"/> ); +<#else> +if ( ${sourceBeanName}.${sourceAccessorName}() != null ) { +${ext.targetBeanName}.${targetAccessorName}().addAll( <@includeModel object=mappingMethod input="${sourceBeanName}.${sourceAccessorName}()"/> ); +} + <#-- b) simple conversion --> <#elseif conversion??> <#if sourceType.primitive == false> @@ -34,7 +40,11 @@ if ( ${sourceBeanName}.${sourceAccessorName}() != null ) { <#else> <#if targetType.collectionType || targetType.mapType> if ( ${sourceBeanName}.${sourceAccessorName}() != null ) { +<#if hasTargetSetter> ${ext.targetBeanName}.${targetAccessorName}( new <#if targetType.implementationType??><@includeModel object=targetType.implementationType/><#else><@includeModel object=targetType/>( ${sourceBeanName}.${sourceAccessorName}() ) ); +<#else> + ${ext.targetBeanName}.${targetAccessorName}().addAll( new <#if targetType.implementationType??><@includeModel object=targetType.implementationType/><#else><@includeModel object=targetType/>( ${sourceBeanName}.${sourceAccessorName}() ) ); + } <#else> ${ext.targetBeanName}.${targetAccessorName}( ${sourceBeanName}.${sourceAccessorName}() ); diff --git a/processor/src/test/java/org/mapstruct/ap/test/collection/CollectionMappingTest.java b/processor/src/test/java/org/mapstruct/ap/test/collection/CollectionMappingTest.java index e7170c402..a616a192e 100644 --- a/processor/src/test/java/org/mapstruct/ap/test/collection/CollectionMappingTest.java +++ b/processor/src/test/java/org/mapstruct/ap/test/collection/CollectionMappingTest.java @@ -70,6 +70,18 @@ public class CollectionMappingTest extends MapperTestBase { assertThat( target.getStringList() ).containsExactly( "Bob", "Alice" ); } + @Test + @IssueKey("6") + public void shouldMapListWithoutSetter() { + Source source = new Source(); + source.setStringList2( Arrays.asList( "Bob", "Alice" ) ); + + Target target = SourceTargetMapper.INSTANCE.sourceToTarget( source ); + + assertThat( target ).isNotNull(); + assertThat( target.getStringListNoSetter() ).containsExactly( "Bob", "Alice" ); + } + @Test @IssueKey("6") public void shouldReverseMapList() { diff --git a/processor/src/test/java/org/mapstruct/ap/test/collection/Source.java b/processor/src/test/java/org/mapstruct/ap/test/collection/Source.java index 7cc84b4fa..8146b5dd7 100644 --- a/processor/src/test/java/org/mapstruct/ap/test/collection/Source.java +++ b/processor/src/test/java/org/mapstruct/ap/test/collection/Source.java @@ -45,6 +45,8 @@ public class Source { private Map stringLongMap; + private List stringList2; + public List getStringList() { return stringList; } @@ -124,4 +126,13 @@ public class Source { public void setStringLongMap(Map stringLongMap) { this.stringLongMap = stringLongMap; } + + public List getStringList2() { + return stringList2; + } + + public void setStringList2( List stringList2 ) { + this.stringList2 = stringList2; + } + } diff --git a/processor/src/test/java/org/mapstruct/ap/test/collection/SourceTargetMapper.java b/processor/src/test/java/org/mapstruct/ap/test/collection/SourceTargetMapper.java index 723609d69..2ff7368cc 100644 --- a/processor/src/test/java/org/mapstruct/ap/test/collection/SourceTargetMapper.java +++ b/processor/src/test/java/org/mapstruct/ap/test/collection/SourceTargetMapper.java @@ -33,7 +33,8 @@ public interface SourceTargetMapper { @Mappings({ @Mapping(source = "integerList", target = "integerCollection"), @Mapping(source = "integerSet", target = "set"), - @Mapping(source = "anotherIntegerSet", target = "anotherStringSet") + @Mapping(source = "anotherIntegerSet", target = "anotherStringSet"), + @Mapping(source = "stringList2", target = "stringListNoSetter") }) Target sourceToTarget(Source source); diff --git a/processor/src/test/java/org/mapstruct/ap/test/collection/Target.java b/processor/src/test/java/org/mapstruct/ap/test/collection/Target.java index a88ad60e8..0efcf66f4 100644 --- a/processor/src/test/java/org/mapstruct/ap/test/collection/Target.java +++ b/processor/src/test/java/org/mapstruct/ap/test/collection/Target.java @@ -43,6 +43,8 @@ public class Target { private Map stringLongMap; + private List stringListNoSetter; + @SuppressWarnings("rawtypes") private Set set; @@ -127,4 +129,12 @@ public class Target { public void setStringLongMap(Map stringLongMap) { this.stringLongMap = stringLongMap; } + + public List getStringListNoSetter() { + if ( stringListNoSetter == null ) { + stringListNoSetter = new ArrayList(); + } + return stringListNoSetter; + } + } diff --git a/processor/src/test/java/org/mapstruct/ap/test/collection/defaultimplementation/DefaultCollectionImplementationTest.java b/processor/src/test/java/org/mapstruct/ap/test/collection/defaultimplementation/DefaultCollectionImplementationTest.java index 80f4931b0..0d2750b97 100644 --- a/processor/src/test/java/org/mapstruct/ap/test/collection/defaultimplementation/DefaultCollectionImplementationTest.java +++ b/processor/src/test/java/org/mapstruct/ap/test/collection/defaultimplementation/DefaultCollectionImplementationTest.java @@ -42,6 +42,8 @@ import org.mapstruct.ap.testutil.WithClasses; import org.testng.annotations.Test; @WithClasses({ + Source.class, + Target.class, SourceFoo.class, TargetFoo.class, SourceTargetMapper.class @@ -168,6 +170,17 @@ public class DefaultCollectionImplementationTest extends MapperTestBase { assertResultList( target ); } + @Test + @IssueKey("6") + public void shouldUseDefaultImplementationForListWithoutSetter() { + Source source = new Source(); + source.setFooList( createSourceFooList() ); + Target target = SourceTargetMapper.INSTANCE.sourceToTarget( source ); + + assertThat( target ).isNotNull(); + assertThat( target.getFooListNoSetter()).containsExactly( new TargetFoo( "Bob" ), new TargetFoo( "Alice" ) ); + } + private void assertResultList(Iterable fooIterable) { assertThat( fooIterable ).isNotNull(); assertThat( fooIterable ).containsOnly( new TargetFoo( "Bob" ), new TargetFoo( "Alice" ) ); diff --git a/processor/src/test/java/org/mapstruct/ap/test/collection/defaultimplementation/Source.java b/processor/src/test/java/org/mapstruct/ap/test/collection/defaultimplementation/Source.java new file mode 100644 index 000000000..1336d7ccc --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/collection/defaultimplementation/Source.java @@ -0,0 +1,35 @@ +/** + * Copyright 2012-2014 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.collection.defaultimplementation; + +import java.util.List; + +public class Source { + + private List fooList; + + public List getFooList() { + return fooList; + } + + public void setFooList( List fooList ) { + this.fooList = fooList; + } + +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/collection/defaultimplementation/SourceTargetMapper.java b/processor/src/test/java/org/mapstruct/ap/test/collection/defaultimplementation/SourceTargetMapper.java index d65de0047..7af2ec2b7 100644 --- a/processor/src/test/java/org/mapstruct/ap/test/collection/defaultimplementation/SourceTargetMapper.java +++ b/processor/src/test/java/org/mapstruct/ap/test/collection/defaultimplementation/SourceTargetMapper.java @@ -30,6 +30,7 @@ import java.util.concurrent.ConcurrentMap; import java.util.concurrent.ConcurrentNavigableMap; import org.mapstruct.Mapper; +import org.mapstruct.Mapping; import org.mapstruct.MappingTarget; import org.mapstruct.factory.Mappers; @@ -38,6 +39,9 @@ public interface SourceTargetMapper { SourceTargetMapper INSTANCE = Mappers.getMapper( SourceTargetMapper.class ); + @Mapping(source = "fooList", target = "fooListNoSetter") + Target sourceToTarget(Source source); + TargetFoo sourceFooToTargetFoo(SourceFoo sourceFoo); List sourceFoosToTargetFoos(List foos); diff --git a/processor/src/test/java/org/mapstruct/ap/test/collection/defaultimplementation/Target.java b/processor/src/test/java/org/mapstruct/ap/test/collection/defaultimplementation/Target.java new file mode 100644 index 000000000..1bc98e9ee --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/collection/defaultimplementation/Target.java @@ -0,0 +1,34 @@ +/** + * Copyright 2012-2014 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.collection.defaultimplementation; + +import java.util.ArrayList; +import java.util.List; + +public class Target { + + private List fooListNoSetter; + + public List getFooListNoSetter() { + if ( fooListNoSetter == null ) { + fooListNoSetter = new ArrayList(); + } + return fooListNoSetter; + } +}