From b74bde5c2214cf253f9ae6f7fa47515c20bbe388 Mon Sep 17 00:00:00 2001 From: thunderhook <8238759+thunderhook@users.noreply.github.com> Date: Wed, 21 Aug 2024 23:47:59 +0200 Subject: [PATCH] #3678 Prevent duplicate @BeforeMapping and @AfterMapping calls on classes that use the Builder pattern. --- .../ap/internal/model/BeanMappingMethod.java | 15 ++- .../BuilderLifecycleCallbacksTest.java | 7 ++ .../builder/lifecycle/MappingContext.java | 10 ++ .../builder/lifecycle/OrderMapperImpl.java | 91 +++++++++++++++++++ 4 files changed, 122 insertions(+), 1 deletion(-) create mode 100644 processor/src/test/resources/fixtures/org/mapstruct/ap/test/builder/lifecycle/OrderMapperImpl.java diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/BeanMappingMethod.java b/processor/src/main/java/org/mapstruct/ap/internal/model/BeanMappingMethod.java index 6535b0701..a265408be 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/BeanMappingMethod.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/BeanMappingMethod.java @@ -406,9 +406,11 @@ public class BeanMappingMethod extends NormalTypeMappingMethod { existingVariableNames ) ); - // remove methods without parameters as they are already being invoked + // remove methods that are already being invoked removeMappingReferencesWithoutSourceParameters( beforeMappingReferencesWithFinalizedReturnType ); removeMappingReferencesWithoutSourceParameters( afterMappingReferencesWithFinalizedReturnType ); + removeMappingReferencesWithSingleSourceParameter( beforeMappingReferencesWithFinalizedReturnType ); + removeMappingReferencesWithSingleSourceParameter( afterMappingReferencesWithFinalizedReturnType ); } Map presenceChecksByParameter = new LinkedHashMap<>(); @@ -455,6 +457,17 @@ public class BeanMappingMethod extends NormalTypeMappingMethod { references.removeIf( r -> r.getSourceParameters().isEmpty() && r.getReturnType().isVoid() ); } + private void removeMappingReferencesWithSingleSourceParameter( + List references) { + references.removeIf( Builder::isSingleSourceParameter ); + } + + private static boolean isSingleSourceParameter(LifecycleCallbackMethodReference reference) { + return reference.getParameterBindings().size() == 1 + && reference.getParameterBindings().stream().allMatch( ParameterBinding::isSourceParameter ) + && reference.getReturnType().isVoid(); + } + private boolean doesNotAllowAbstractReturnTypeAndCanBeConstructed(Type returnTypeImpl) { return !isAbstractReturnTypeAllowed() && canReturnTypeBeConstructed( returnTypeImpl ); diff --git a/processor/src/test/java/org/mapstruct/ap/test/builder/lifecycle/BuilderLifecycleCallbacksTest.java b/processor/src/test/java/org/mapstruct/ap/test/builder/lifecycle/BuilderLifecycleCallbacksTest.java index 3d5fb7e53..b60759c11 100644 --- a/processor/src/test/java/org/mapstruct/ap/test/builder/lifecycle/BuilderLifecycleCallbacksTest.java +++ b/processor/src/test/java/org/mapstruct/ap/test/builder/lifecycle/BuilderLifecycleCallbacksTest.java @@ -7,9 +7,11 @@ package org.mapstruct.ap.test.builder.lifecycle; import java.util.Arrays; +import org.junit.jupiter.api.extension.RegisterExtension; import org.mapstruct.ap.testutil.IssueKey; import org.mapstruct.ap.testutil.ProcessorTest; import org.mapstruct.ap.testutil.WithClasses; +import org.mapstruct.ap.testutil.runner.GeneratedSource; import static org.assertj.core.api.Assertions.assertThat; @@ -27,6 +29,9 @@ import static org.assertj.core.api.Assertions.assertThat; } ) public class BuilderLifecycleCallbacksTest { + @RegisterExtension + final GeneratedSource source = new GeneratedSource().addComparisonToFixtureFor( OrderMapper.class ); + @ProcessorTest public void lifecycleMethodsShouldBeInvoked() { OrderDto source = new OrderDto(); @@ -43,6 +48,7 @@ public class BuilderLifecycleCallbacksTest { assertThat( context.getInvokedMethods() ) .contains( "beforeWithoutParameters", + "beforeWithSource", "beforeWithTargetType", "beforeWithBuilderTargetType", "beforeWithBuilderTarget", @@ -50,6 +56,7 @@ public class BuilderLifecycleCallbacksTest { "afterWithBuilderTargetType", "afterWithBuilderTarget", "afterWithBuilderTargetReturningTarget", + "afterWithSource", "afterWithTargetType", "afterWithTarget", "afterWithTargetReturningTarget" diff --git a/processor/src/test/java/org/mapstruct/ap/test/builder/lifecycle/MappingContext.java b/processor/src/test/java/org/mapstruct/ap/test/builder/lifecycle/MappingContext.java index 2065bad47..6006440bc 100644 --- a/processor/src/test/java/org/mapstruct/ap/test/builder/lifecycle/MappingContext.java +++ b/processor/src/test/java/org/mapstruct/ap/test/builder/lifecycle/MappingContext.java @@ -25,6 +25,11 @@ public class MappingContext { invokedMethods.add( "beforeWithoutParameters" ); } + @BeforeMapping + public void beforeWithSource(OrderDto source) { + invokedMethods.add( "beforeWithSource" ); + } + @BeforeMapping public void beforeWithTargetType(OrderDto source, @TargetType Class orderClass) { invokedMethods.add( "beforeWithTargetType" ); @@ -50,6 +55,11 @@ public class MappingContext { invokedMethods.add( "afterWithoutParameters" ); } + @AfterMapping + public void afterWithSource(OrderDto source) { + invokedMethods.add( "afterWithSource" ); + } + @AfterMapping public void afterWithTargetType(OrderDto source, @TargetType Class orderClass) { invokedMethods.add( "afterWithTargetType" ); diff --git a/processor/src/test/resources/fixtures/org/mapstruct/ap/test/builder/lifecycle/OrderMapperImpl.java b/processor/src/test/resources/fixtures/org/mapstruct/ap/test/builder/lifecycle/OrderMapperImpl.java new file mode 100644 index 000000000..6221010b2 --- /dev/null +++ b/processor/src/test/resources/fixtures/org/mapstruct/ap/test/builder/lifecycle/OrderMapperImpl.java @@ -0,0 +1,91 @@ +/* + * 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.builder.lifecycle; + +import java.util.ArrayList; +import java.util.List; +import javax.annotation.processing.Generated; + +@Generated( + value = "org.mapstruct.ap.MappingProcessor", + date = "2024-08-21T23:18:51+0200", + comments = "version: , compiler: javac, environment: Java 21.0.1 (Oracle Corporation)" +) +public class OrderMapperImpl implements OrderMapper { + + @Override + public Order map(OrderDto source, MappingContext context) { + context.beforeWithoutParameters(); + context.beforeWithSource( source ); + context.beforeWithBuilderTargetType( source, Order.Builder.class ); + + context.beforeWithTargetType( source, Order.class ); + + if ( source == null ) { + return null; + } + + Order.Builder order = Order.builder(); + + context.beforeWithBuilderTarget( source, order ); + + order.items( itemDtoListToItemList( source.getItems(), context ) ); + + context.afterWithoutParameters(); + context.afterWithSource( source ); + context.afterWithBuilderTargetType( source, Order.Builder.class ); + context.afterWithBuilderTarget( source, order ); + Order target = context.afterWithBuilderTargetReturningTarget( order ); + if ( target != null ) { + return target; + } + + Order orderResult = order.create(); + + context.afterWithTargetType( source, Order.class ); + context.afterWithTarget( source, orderResult ); + Order target1 = context.afterWithTargetReturningTarget( orderResult ); + if ( target1 != null ) { + return target1; + } + + return orderResult; + } + + @Override + public Item map(ItemDto source, MappingContext context) { + context.beforeWithoutParameters(); + + if ( source == null ) { + return null; + } + + Item.Builder item = Item.builder(); + + item.name( source.getName() ); + + context.afterWithoutParameters(); + + return item.create(); + } + + protected List itemDtoListToItemList(List list, MappingContext context) { + context.beforeWithoutParameters(); + + if ( list == null ) { + return null; + } + + List list1 = new ArrayList( list.size() ); + for ( ItemDto itemDto : list ) { + list1.add( map( itemDto, context ) ); + } + + context.afterWithoutParameters(); + + return list1; + } +}