From 30c2dadec7b84895498db7f99308f169f2f516e4 Mon Sep 17 00:00:00 2001 From: Sebastian Date: Mon, 24 Sep 2018 22:23:34 +0200 Subject: [PATCH] #1561 add support for adders in combination with streams - Extended Type#getAlternativeTargetAccessors to recognize stream read accessors for which no corresponding setter exists (there is only an add method) - Extended SourceRHS#getSourceTypeForMatching to return the correct source type for streams too - Add StreamAdderWrapper to map Stream -> Adder - Extended PropertyMapping$PropertyMappingBuilder#assignToPlainViaAdder to return StreamAdderWrapper if source type is stream --- .../ap/internal/model/PropertyMapping.java | 5 ++ .../model/assignment/StreamAdderWrapper.java | 67 ++++++++++++++ .../ap/internal/model/common/SourceRHS.java | 12 ++- .../ap/internal/model/common/Type.java | 90 ++++++++++++------- .../model/assignment/StreamAdderWrapper.ftl | 14 +++ .../bugs/_1561/java8/Issue1561Mapper.java | 24 +++++ .../test/bugs/_1561/java8/Issue1561Test.java | 45 ++++++++++ .../ap/test/bugs/_1561/java8/Source.java | 25 ++++++ .../ap/test/bugs/_1561/java8/Target.java | 26 ++++++ 9 files changed, 276 insertions(+), 32 deletions(-) create mode 100644 processor/src/main/java/org/mapstruct/ap/internal/model/assignment/StreamAdderWrapper.java create mode 100644 processor/src/main/resources/org/mapstruct/ap/internal/model/assignment/StreamAdderWrapper.ftl create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_1561/java8/Issue1561Mapper.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_1561/java8/Issue1561Test.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_1561/java8/Source.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_1561/java8/Target.java diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/PropertyMapping.java b/processor/src/main/java/org/mapstruct/ap/internal/model/PropertyMapping.java index 09627b5c4..54da3723c 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/PropertyMapping.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/PropertyMapping.java @@ -25,6 +25,7 @@ import org.mapstruct.ap.internal.model.assignment.ArrayCopyWrapper; import org.mapstruct.ap.internal.model.assignment.EnumConstantWrapper; import org.mapstruct.ap.internal.model.assignment.GetterWrapperForCollectionsAndMaps; import org.mapstruct.ap.internal.model.assignment.SetterWrapper; +import org.mapstruct.ap.internal.model.assignment.StreamAdderWrapper; import org.mapstruct.ap.internal.model.assignment.UpdateWrapper; import org.mapstruct.ap.internal.model.common.Assignment; import org.mapstruct.ap.internal.model.common.FormattingParameters; @@ -438,6 +439,10 @@ public class PropertyMapping extends ModelElement { if ( result.getSourceType().isCollectionType() ) { result = new AdderWrapper( result, method.getThrownTypes(), isFieldAssignment(), targetPropertyName ); } + else if ( result.getSourceType().isStreamType() ) { + result = new StreamAdderWrapper( + result, method.getThrownTypes(), isFieldAssignment(), targetPropertyName ); + } else { // Possibly adding null to a target collection. So should be surrounded by an null check. result = new SetterWrapper( result, method.getThrownTypes(), ALWAYS, isFieldAssignment(), targetType ); diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/assignment/StreamAdderWrapper.java b/processor/src/main/java/org/mapstruct/ap/internal/model/assignment/StreamAdderWrapper.java new file mode 100644 index 000000000..6c38f05a2 --- /dev/null +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/assignment/StreamAdderWrapper.java @@ -0,0 +1,67 @@ +/* + * Copyright MapStruct Authors. + * + * Licensed under the Apache License version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0 + */ +package org.mapstruct.ap.internal.model.assignment; + +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import java.util.stream.Stream; + +import org.mapstruct.ap.internal.model.common.Assignment; +import org.mapstruct.ap.internal.model.common.Type; +import org.mapstruct.ap.internal.util.Nouns; + +import static org.mapstruct.ap.internal.util.Collections.first; + +/** + * Wraps the assignment in a target setter. + * + * @author Sebastian Haberey + */ +public class StreamAdderWrapper extends AssignmentWrapper { + + private final List thrownTypesToExclude; + private final Type adderType; + + public StreamAdderWrapper(Assignment rhs, + List thrownTypesToExclude, + boolean fieldAssignment, + String targetPropertyName ) { + super( rhs, fieldAssignment ); + this.thrownTypesToExclude = thrownTypesToExclude; + String desiredName = Nouns.singularize( targetPropertyName ); + rhs.setSourceLocalVarName( rhs.createLocalVarName( desiredName ) ); + adderType = first( getSourceType().determineTypeArguments( Stream.class ) ); + } + + @Override + public List getThrownTypes() { + List parentThrownTypes = super.getThrownTypes(); + List result = new ArrayList( parentThrownTypes ); + for ( Type thrownTypeToExclude : thrownTypesToExclude ) { + for ( Type parentExceptionType : parentThrownTypes ) { + if ( parentExceptionType.isAssignableTo( thrownTypeToExclude ) ) { + result.remove( parentExceptionType ); + } + } + } + return result; + } + + public Type getAdderType() { + return adderType; + } + + @Override + public Set getImportTypes() { + Set imported = new HashSet(); + imported.addAll( super.getImportTypes() ); + imported.add( adderType.getTypeBound() ); + return imported; + } + +} diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/common/SourceRHS.java b/processor/src/main/java/org/mapstruct/ap/internal/model/common/SourceRHS.java index dd36fb2cc..fddf0a4b1 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/common/SourceRHS.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/common/SourceRHS.java @@ -9,6 +9,7 @@ import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.Set; +import java.util.stream.Stream; import org.mapstruct.ap.internal.util.Strings; @@ -124,8 +125,15 @@ public class SourceRHS extends ModelElement implements Assignment { * @return the source type to be used in the matching process. */ public Type getSourceTypeForMatching() { - return useElementAsSourceTypeForMatching && sourceType.isCollectionType() ? - first( sourceType.determineTypeArguments( Collection.class ) ) : sourceType; + if ( useElementAsSourceTypeForMatching ) { + if ( sourceType.isCollectionType() ) { + return first( sourceType.determineTypeArguments( Collection.class ) ); + } + else if ( sourceType.isStreamType() ) { + return first( sourceType.determineTypeArguments( Stream.class ) ); + } + } + return sourceType; } /** 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 d3f4e2aee..997277ab5 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 @@ -13,6 +13,7 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.stream.Stream; import javax.lang.model.element.AnnotationMirror; import javax.lang.model.element.Element; import javax.lang.model.element.ElementKind; @@ -33,6 +34,7 @@ import org.mapstruct.ap.internal.prism.CollectionMappingStrategyPrism; import org.mapstruct.ap.internal.util.AccessorNamingUtils; import org.mapstruct.ap.internal.util.Executables; import org.mapstruct.ap.internal.util.Filters; +import org.mapstruct.ap.internal.util.JavaStreamConstants; import org.mapstruct.ap.internal.util.Nouns; import org.mapstruct.ap.internal.util.accessor.Accessor; import org.mapstruct.ap.internal.util.accessor.ExecutableElementAccessor; @@ -625,46 +627,67 @@ public class Type extends ModelElement implements Comparable { */ private Accessor getAdderForType(Type collectionProperty, String pluralPropertyName) { - List candidates = new ArrayList(); - if ( collectionProperty.isCollectionType ) { + List candidates; - // this is a collection, so this can be done always - TypeMirror typeArg = first( collectionProperty.determineTypeArguments( Iterable.class ) ).getTypeBound() - .getTypeMirror(); - // now, look for a method that - // 1) starts with add, - // 2) and has typeArg as one and only arg - List adderList = getAdders(); - for ( Accessor adder : adderList ) { - ExecutableElement executable = adder.getExecutable(); - if ( executable == null ) { - // it should not be null, but to be safe - continue; - } - VariableElement arg = executable.getParameters().get( 0 ); - if ( typeUtils.isSameType( arg.asType(), typeArg ) ) { - candidates.add( adder ); - } - } + if ( collectionProperty.isCollectionType() ) { + candidates = getAccessorCandidates( collectionProperty, Iterable.class ); } + else if ( collectionProperty.isStreamType() ) { + candidates = getAccessorCandidates( collectionProperty, Stream.class ); + } + else { + return null; + } + if ( candidates.isEmpty() ) { return null; } - else if ( candidates.size() == 1 ) { + + if ( candidates.size() == 1 ) { return candidates.get( 0 ); } - else { - for ( Accessor candidate : candidates ) { - String elementName = accessorNaming.getElementNameForAdder( candidate ); - if ( elementName != null && elementName.equals( Nouns.singularize( pluralPropertyName ) ) ) { - return candidate; - } + + for ( Accessor candidate : candidates ) { + String elementName = accessorNaming.getElementNameForAdder( candidate ); + if ( elementName != null && elementName.equals( Nouns.singularize( pluralPropertyName ) ) ) { + return candidate; } } return null; } + /** + * Returns all accessor candidates that start with "add" and have exactly one argument + * whose type matches the collection or stream property's type argument. + * + * @param property the collection or stream property + * @param superclass the superclass to use for type argument lookup + * + * @return accessor candidates + */ + private List getAccessorCandidates(Type property, Class superclass) { + TypeMirror typeArg = first( property.determineTypeArguments( superclass ) ).getTypeBound() + .getTypeMirror(); + // now, look for a method that + // 1) starts with add, + // 2) and has typeArg as one and only arg + List adderList = getAdders(); + List candidateList = new ArrayList(); + for ( Accessor adder : adderList ) { + ExecutableElement executable = adder.getExecutable(); + if ( executable == null ) { + // it should not be null, but to be safe + continue; + } + VariableElement arg = executable.getParameters().get( 0 ); + if ( typeUtils.isSameType( arg.asType(), typeArg ) ) { + candidateList.add( adder ); + } + } + return candidateList; + } + /** * getSetters * @@ -716,7 +739,7 @@ public class Type extends ModelElement implements Comparable { // an accessor could substitute the setter in that case and act as setter. // (assuming it is initialized) for ( Accessor readAccessor : readAccessors ) { - if ( isCollectionOrMap( readAccessor ) && + if ( isCollectionOrMapOrStream( readAccessor ) && !correspondingSetterMethodExists( readAccessor, setterMethods ) ) { result.add( readAccessor ); } @@ -745,14 +768,21 @@ public class Type extends ModelElement implements Comparable { return false; } - private boolean isCollectionOrMap(Accessor getterMethod) { - return isCollection( getterMethod.getAccessedType() ) || isMap( getterMethod.getAccessedType() ); + private boolean isCollectionOrMapOrStream(Accessor getterMethod) { + return isCollection( getterMethod.getAccessedType() ) || isMap( getterMethod.getAccessedType() ) || + isStream( getterMethod.getAccessedType() ); } private boolean isCollection(TypeMirror candidate) { return isSubType( candidate, Collection.class ); } + private boolean isStream(TypeMirror candidate) { + TypeElement streamTypeElement = elementUtils.getTypeElement( JavaStreamConstants.STREAM_FQN ); + TypeMirror streamType = streamTypeElement == null ? null : typeUtils.erasure( streamTypeElement.asType() ); + return streamType != null && typeUtils.isSubtype( candidate, streamType ); + } + private boolean isMap(TypeMirror candidate) { return isSubType( candidate, Map.class ); } diff --git a/processor/src/main/resources/org/mapstruct/ap/internal/model/assignment/StreamAdderWrapper.ftl b/processor/src/main/resources/org/mapstruct/ap/internal/model/assignment/StreamAdderWrapper.ftl new file mode 100644 index 000000000..1a308844c --- /dev/null +++ b/processor/src/main/resources/org/mapstruct/ap/internal/model/assignment/StreamAdderWrapper.ftl @@ -0,0 +1,14 @@ +<#-- + + Copyright MapStruct Authors. + + Licensed under the Apache License version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0 + +--> +<#-- @ftlvariable name="" type="org.mapstruct.ap.internal.model.assignment.StreamAdderWrapper" --> +<#import "../macro/CommonMacros.ftl" as lib> +<@lib.handleExceptions> + if ( ${sourceReference} != null ) { + ${sourceReference}.forEach( ${ext.targetBeanName}::${ext.targetWriteAccessorName} ); + } + \ No newline at end of file diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_1561/java8/Issue1561Mapper.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1561/java8/Issue1561Mapper.java new file mode 100644 index 000000000..b50015337 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1561/java8/Issue1561Mapper.java @@ -0,0 +1,24 @@ +/* + * Copyright MapStruct Authors. + * + * Licensed under the Apache License version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0 + */ +package org.mapstruct.ap.test.bugs._1561.java8; + +import org.mapstruct.CollectionMappingStrategy; +import org.mapstruct.Mapper; +import org.mapstruct.factory.Mappers; + +/** + * @author Sebastian Haberey + */ +@Mapper(collectionMappingStrategy = CollectionMappingStrategy.ADDER_PREFERRED) +public interface Issue1561Mapper { + + Issue1561Mapper + INSTANCE = Mappers.getMapper( Issue1561Mapper.class ); + + Target map(Source source); + + Source map(Target target); +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_1561/java8/Issue1561Test.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1561/java8/Issue1561Test.java new file mode 100644 index 000000000..d15a53f48 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1561/java8/Issue1561Test.java @@ -0,0 +1,45 @@ +/* + * Copyright MapStruct Authors. + * + * Licensed under the Apache License version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0 + */ +package org.mapstruct.ap.test.bugs._1561.java8; + +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 static org.assertj.core.api.Assertions.assertThat; + +/** + * @author Sebastian Haberey + */ +@RunWith(AnnotationProcessorTestRunner.class) +@IssueKey("1561") +@WithClasses({ + Issue1561Mapper.class, + Source.class, + Target.class +}) +public class Issue1561Test { + + @Test + public void shouldCorrectlyUseAdder() { + + Source source = new Source(); + source.addProperty( "first" ); + source.addProperty( "second" ); + + Target target = Issue1561Mapper.INSTANCE.map( source ); + + assertThat( target.getProperties() ) + .containsExactly( "first", "second" ); + + Source mapped = Issue1561Mapper.INSTANCE.map( target ); + + assertThat( mapped.getProperties() ) + .containsExactly( "first", "second" ); + } +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_1561/java8/Source.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1561/java8/Source.java new file mode 100644 index 000000000..5f52689ce --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1561/java8/Source.java @@ -0,0 +1,25 @@ +/* + * Copyright MapStruct Authors. + * + * Licensed under the Apache License version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0 + */ +package org.mapstruct.ap.test.bugs._1561.java8; + +import java.util.ArrayList; +import java.util.List; + +/** + * @author Sebastian Haberey + */ +public class Source { + + private List properties = new ArrayList(); + + public List getProperties() { + return properties; + } + + public void addProperty(String property) { + properties.add( property ); + } +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_1561/java8/Target.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1561/java8/Target.java new file mode 100644 index 000000000..1a46477dd --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1561/java8/Target.java @@ -0,0 +1,26 @@ +/* + * Copyright MapStruct Authors. + * + * Licensed under the Apache License version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0 + */ +package org.mapstruct.ap.test.bugs._1561.java8; + +import java.util.ArrayList; +import java.util.List; +import java.util.stream.Stream; + +/** + * @author Sebastian Haberey + */ +public class Target { + + private List properties = new ArrayList(); + + public Stream getProperties() { + return properties.stream(); + } + + public void addProperty(String property) { + properties.add( property ); + } +}