From 3c079e412aabb7afb1f11b946e6c85dd874c9777 Mon Sep 17 00:00:00 2001 From: Sjaak Derksen Date: Sun, 10 Feb 2019 10:31:51 +0100 Subject: [PATCH] #1707 fix for not defining local variable in stream-iterable mapping (#1708) --- .../ap/internal/model/StreamMappingMethod.ftl | 10 +-- .../ap/test/bugs/_1707/Converter.java | 89 +++++++++++++++++++ .../ap/test/bugs/_1707/Issue1707Test.java | 32 +++++++ .../ap/test/bugs/_1707/ConverterImpl.java | 67 ++++++++++++++ 4 files changed, 193 insertions(+), 5 deletions(-) create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_1707/Converter.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_1707/Issue1707Test.java create mode 100644 processor/src/test/resources/fixtures/org/mapstruct/ap/test/bugs/_1707/ConverterImpl.java diff --git a/processor/src/main/resources/org/mapstruct/ap/internal/model/StreamMappingMethod.ftl b/processor/src/main/resources/org/mapstruct/ap/internal/model/StreamMappingMethod.ftl index 3eab47794..f9a7309b5 100644 --- a/processor/src/main/resources/org/mapstruct/ap/internal/model/StreamMappingMethod.ftl +++ b/processor/src/main/resources/org/mapstruct/ap/internal/model/StreamMappingMethod.ftl @@ -48,11 +48,11 @@ } - <#-- A variable needs to be defined if there are before mappings and this is not exisitingInstanceMapping --> - <#assign needVarDefine = beforeMappingReferencesWithMappingTarget?has_content && !existingInstanceMapping /> + <#-- A variable needs to be defined if there are before or after mappings and this is not exisitingInstanceMapping --> + <#assign needVarDefine = (beforeMappingReferencesWithMappingTarget?has_content || afterMappingReferences?has_content) && !existingInstanceMapping /> <#if resultType.arrayType> - <#if !existingInstanceMapping && needVarDefine> + <#if needVarDefine> <#assign needVarDefine = false /> <#-- We create a null array which later will be directly assigned from the stream--> ${resultElementType}[] ${resultName} = null; @@ -67,7 +67,7 @@ <#else> <#-- Streams are immutable so we can't update them --> - <#if !existingInstanceMapping && needVarDefine> + <#if needVarDefine> <#assign needVarDefine = false /> <@iterableLocalVarDef/> ${resultName} = Stream.empty(); @@ -180,5 +180,5 @@ <#macro returnLocalVarDefOrUpdate> - <#if canReturnImmediatelly><#if returnType.name != "void">return <#elseif !needVarDefine><@iterableLocalVarDef/> ${resultName} = <#else>${resultName} = <#nested /> + <#if canReturnImmediatelly><#if returnType.name != "void">return <#elseif needVarDefine><@iterableLocalVarDef/> ${resultName} = <#else>${resultName} = <#nested /> diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_1707/Converter.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1707/Converter.java new file mode 100644 index 000000000..45659b9d6 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1707/Converter.java @@ -0,0 +1,89 @@ +/* + * 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._1707; + +import java.util.Set; +import java.util.stream.Stream; + +import org.mapstruct.AfterMapping; +import org.mapstruct.Mapper; +import org.mapstruct.Mapping; +import org.mapstruct.MappingTarget; + +@Mapper +public abstract class Converter { + + public abstract Set convert(Stream source); + + public abstract Target[] convertArray(Stream source); + + @Mapping( target = "custom", ignore = true ) + public abstract Target convert(Source source); + + @AfterMapping + public void addCustomValue(@MappingTarget Set targetList) { + targetList.forEach( t -> t.custom = true ); + } + + @AfterMapping + public void addCustomValue(@MappingTarget Target[] targetArray) { + for ( Target target : targetArray ) { + target.custom = true; + } + } + + public static final class Source { + private String text; + private int number; + + public String getText() { + return text; + } + + public void setText(String text) { + this.text = text; + } + + public int getNumber() { + return number; + } + + public void setNumber(int number) { + this.number = number; + } + } + + public static class Target { + private String text; + private int number; + private boolean custom; + + public String getText() { + return text; + } + + public void setText(String text) { + this.text = text; + } + + public int getNumber() { + return number; + } + + public void setNumber(int number) { + this.number = number; + } + + public boolean isCustom() { + return custom; + } + + public void setCustom(boolean custom) { + this.custom = custom; + } + } + +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_1707/Issue1707Test.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1707/Issue1707Test.java new file mode 100644 index 000000000..5d2083312 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_1707/Issue1707Test.java @@ -0,0 +1,32 @@ +/* + * 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._1707; + +import org.junit.Rule; +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.ap.testutil.runner.GeneratedSource; + +@RunWith(AnnotationProcessorTestRunner.class) +@IssueKey("1707") +@WithClasses({ + Converter.class +}) +public class Issue1707Test { + + @Rule + public final GeneratedSource generatedSource = new GeneratedSource().addComparisonToFixtureFor( + Converter.class + ); + + @Test + public void codeShouldBeGeneratedCorrectly() { + + } +} diff --git a/processor/src/test/resources/fixtures/org/mapstruct/ap/test/bugs/_1707/ConverterImpl.java b/processor/src/test/resources/fixtures/org/mapstruct/ap/test/bugs/_1707/ConverterImpl.java new file mode 100644 index 000000000..898b13418 --- /dev/null +++ b/processor/src/test/resources/fixtures/org/mapstruct/ap/test/bugs/_1707/ConverterImpl.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.test.bugs._1707; + +import java.util.HashSet; +import java.util.Set; +import java.util.stream.Collectors; +import java.util.stream.Stream; +import javax.annotation.Generated; + +@Generated( + value = "org.mapstruct.ap.MappingProcessor", + date = "2019-02-10T09:58:11+0100", + comments = "version: , compiler: javac, environment: Java 1.8.0_181 (Oracle Corporation)" +) +public class ConverterImpl extends Converter { + + @Override + public Set convert(Stream source) { + if ( source == null ) { + return null; + } + + Set set = new HashSet(); + + set.addAll( source.map( source1 -> convert( source1 ) ) + .collect( Collectors.toCollection( HashSet::new ) ) + ); + + addCustomValue( set ); + + return set; + } + + @Override + public org.mapstruct.ap.test.bugs._1707.Converter.Target[] convertArray(Stream source) { + if ( source == null ) { + return null; + } + + org.mapstruct.ap.test.bugs._1707.Converter.Target[] targetTmp = null; + + targetTmp = source.map( source1 -> convert( source1 ) ) + .toArray( Target[]::new ); + + addCustomValue( targetTmp ); + + return targetTmp; + } + + @Override + public Target convert(Source source) { + if ( source == null ) { + return null; + } + + Target target = new Target(); + + target.setText( source.getText() ); + target.setNumber( source.getNumber() ); + + return target; + } +}