From 6d205e5bc46365444774609e860f60819818ef4b Mon Sep 17 00:00:00 2001 From: Oliver Erhart <8238759+thunderhook@users.noreply.github.com> Date: Sun, 21 May 2023 22:49:41 +0200 Subject: [PATCH] #1454 Support for lifecycle methods on type being built with builders Add missing support for lifecycle methods with builders: * `@BeforeMapping` with `@TargetType` the type being build * `@AftereMapping` with `@TargetType` the type being build * `@AfterMapping` with `@MappingTarget` the type being build --- .../resources/build-config/checkstyle.xml | 4 +- .../chapter-12-customizing-mapping.asciidoc | 14 ++-- .../chapter-13-using-mapstruct-spi.asciidoc | 2 +- .../ap/internal/model/BeanMappingMethod.java | 72 ++++++++++++++++++- .../ap/internal/model/MappingMethod.java | 4 +- .../ap/internal/model/BeanMappingMethod.ftl | 21 +++++- .../BuilderLifecycleCallbacksTest.java | 6 +- .../builder/lifecycle/MappingContext.java | 10 ++- 8 files changed, 119 insertions(+), 14 deletions(-) diff --git a/build-config/src/main/resources/build-config/checkstyle.xml b/build-config/src/main/resources/build-config/checkstyle.xml index a4591c39d..a1ff4af23 100644 --- a/build-config/src/main/resources/build-config/checkstyle.xml +++ b/build-config/src/main/resources/build-config/checkstyle.xml @@ -29,7 +29,9 @@ - + + + diff --git a/documentation/src/main/asciidoc/chapter-12-customizing-mapping.asciidoc b/documentation/src/main/asciidoc/chapter-12-customizing-mapping.asciidoc index 0c873eac0..dc07b30e6 100644 --- a/documentation/src/main/asciidoc/chapter-12-customizing-mapping.asciidoc +++ b/documentation/src/main/asciidoc/chapter-12-customizing-mapping.asciidoc @@ -248,9 +248,8 @@ All before/after-mapping methods that *can* be applied to a mapping method *will The order of the method invocation is determined primarily by their variant: -1. `@BeforeMapping` methods without an `@MappingTarget` parameter are called before any null-checks on source -parameters and constructing a new target bean. -2. `@BeforeMapping` methods with an `@MappingTarget` parameter are called after constructing a new target bean. +1. `@BeforeMapping` methods without parameters, a `@MappingTarget` parameter or a `@TargetType` parameter are called before any null-checks on source parameters and constructing a new target bean. +2. `@BeforeMapping` methods with a `@MappingTarget` parameter are called after constructing a new target bean. 3. `@AfterMapping` methods are called at the end of the mapping method before the last `return` statement. Within those groups, the method invocations are ordered by their location of definition: @@ -262,4 +261,11 @@ Within those groups, the method invocations are ordered by their location of def *Important:* the order of methods declared within one type can not be guaranteed, as it depends on the compiler and the processing environment implementation. -*Important:* when using a builder, the `@AfterMapping` annotated method must have the builder as `@MappingTarget` annotated parameter so that the method is able to modify the object going to be build. The `build` method is called when the `@AfterMapping` annotated method scope finishes. MapStruct will not call the `@AfterMapping` annotated method if the real target is used as `@MappingTarget` annotated parameter. \ No newline at end of file +[NOTE] +==== +Before/After-mapping methods can also be used with builders: + +* `@BeforeMapping` methods with a `@MappingTarget` parameter of the real target will not be invoked because it is only available after the mapping was already performed. +* To be able to modify the object that is going to be built, the `@AfterMapping` annotated method must have the builder as `@MappingTarget` annotated parameter. The `build` method is called when the `@AfterMapping` annotated method scope finishes. +* The `@AfterMapping` annotated method can also have the real target as `@TargetType` or `@MappingTarget`. It will be invoked after the real target was built (first the methods annotated with `@TargetType`, then the methods annotated with `@MappingTarget`) +==== \ No newline at end of file diff --git a/documentation/src/main/asciidoc/chapter-13-using-mapstruct-spi.asciidoc b/documentation/src/main/asciidoc/chapter-13-using-mapstruct-spi.asciidoc index ce9b28842..51b2bbff3 100644 --- a/documentation/src/main/asciidoc/chapter-13-using-mapstruct-spi.asciidoc +++ b/documentation/src/main/asciidoc/chapter-13-using-mapstruct-spi.asciidoc @@ -71,7 +71,7 @@ public class GolfPlayerDto { public GolfPlayerDto withName(String name) { this.name = name; - return this + return this; } } ---- 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 cf3e23db9..b6afce74d 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 @@ -94,6 +94,9 @@ public class BeanMappingMethod extends NormalTypeMappingMethod { private final Type returnTypeToConstruct; private final BuilderType returnTypeBuilder; private final MethodReference finalizerMethod; + private final String finalizedResultName; + private final List beforeMappingReferencesWithFinalizedReturnType; + private final List afterMappingReferencesWithFinalizedReturnType; private final MappingReferences mappingReferences; @@ -368,8 +371,35 @@ public class BeanMappingMethod extends NormalTypeMappingMethod { MethodReference finalizeMethod = null; + List beforeMappingReferencesWithFinalizedReturnType = new ArrayList<>(); + List afterMappingReferencesWithFinalizedReturnType = new ArrayList<>(); if ( shouldCallFinalizerMethod( returnTypeToConstruct ) ) { finalizeMethod = getFinalizerMethod(); + + Type actualReturnType = method.getReturnType(); + + beforeMappingReferencesWithFinalizedReturnType.addAll( filterMappingTarget( + LifecycleMethodResolver.beforeMappingMethods( + method, + actualReturnType, + selectionParameters, + ctx, + existingVariableNames + ), + false + ) ); + + afterMappingReferencesWithFinalizedReturnType.addAll( LifecycleMethodResolver.afterMappingMethods( + method, + actualReturnType, + selectionParameters, + ctx, + existingVariableNames + ) ); + + // remove methods without parameters as they are already being invoked + removeMappingReferencesWithoutSourceParameters( beforeMappingReferencesWithFinalizedReturnType ); + removeMappingReferencesWithoutSourceParameters( afterMappingReferencesWithFinalizedReturnType ); } return new BeanMappingMethod( @@ -383,12 +413,18 @@ public class BeanMappingMethod extends NormalTypeMappingMethod { returnTypeBuilder, beforeMappingMethods, afterMappingMethods, + beforeMappingReferencesWithFinalizedReturnType, + afterMappingReferencesWithFinalizedReturnType, finalizeMethod, mappingReferences, subclasses ); } + private void removeMappingReferencesWithoutSourceParameters(List references) { + references.removeIf( r -> r.getSourceParameters().isEmpty() && r.getReturnType().isVoid() ); + } + private boolean doesNotAllowAbstractReturnTypeAndCanBeConstructed(Type returnTypeImpl) { return !isAbstractReturnTypeAllowed() && canReturnTypeBeConstructed( returnTypeImpl ); @@ -706,7 +742,6 @@ public class BeanMappingMethod extends NormalTypeMappingMethod { * Find a factory method for a return type or for a builder. * @param returnTypeImpl the return type implementation to construct * @param @selectionParameters - * @return */ private void initializeFactoryMethod(Type returnTypeImpl, SelectionParameters selectionParameters) { List> matchingFactoryMethods = @@ -1380,7 +1415,7 @@ public class BeanMappingMethod extends NormalTypeMappingMethod { *

* When a target property matches its name with the (nested) source property, it is added to the list if and * only if it is an unprocessed target property. - * + *

* duplicates will be handled by {@link #applyPropertyNameBasedMapping(List)} */ private void applyTargetThisMapping() { @@ -1766,6 +1801,8 @@ public class BeanMappingMethod extends NormalTypeMappingMethod { BuilderType returnTypeBuilder, List beforeMappingReferences, List afterMappingReferences, + List beforeMappingReferencesWithFinalizedReturnType, + List afterMappingReferencesWithFinalizedReturnType, MethodReference finalizerMethod, MappingReferences mappingReferences, List subclassMappings) { @@ -1783,9 +1820,20 @@ public class BeanMappingMethod extends NormalTypeMappingMethod { this.propertyMappings = propertyMappings; this.returnTypeBuilder = returnTypeBuilder; this.finalizerMethod = finalizerMethod; + if ( this.finalizerMethod != null ) { + this.finalizedResultName = + Strings.getSafeVariableName( getResultName() + "Result", existingVariableNames ); + existingVariableNames.add( this.finalizedResultName ); + } + else { + this.finalizedResultName = null; + } this.mappingReferences = mappingReferences; - // intialize constant mappings as all mappings, but take out the ones that can be contributed to a + this.beforeMappingReferencesWithFinalizedReturnType = beforeMappingReferencesWithFinalizedReturnType; + this.afterMappingReferencesWithFinalizedReturnType = afterMappingReferencesWithFinalizedReturnType; + + // initialize constant mappings as all mappings, but take out the ones that can be contributed to a // parameter mapping. this.mappingsByParameter = new HashMap<>(); this.constantMappings = new ArrayList<>( propertyMappings.size() ); @@ -1830,6 +1878,18 @@ public class BeanMappingMethod extends NormalTypeMappingMethod { return subclassMappings; } + public String getFinalizedResultName() { + return finalizedResultName; + } + + public List getBeforeMappingReferencesWithFinalizedReturnType() { + return beforeMappingReferencesWithFinalizedReturnType; + } + + public List getAfterMappingReferencesWithFinalizedReturnType() { + return afterMappingReferencesWithFinalizedReturnType; + } + public List propertyMappingsByParameter(Parameter parameter) { // issues: #909 and #1244. FreeMarker has problem getting values from a map when the search key is size or value return mappingsByParameter.getOrDefault( parameter.getName(), Collections.emptyList() ); @@ -1882,6 +1942,12 @@ public class BeanMappingMethod extends NormalTypeMappingMethod { if ( returnTypeBuilder != null ) { types.add( returnTypeBuilder.getOwningType() ); } + for ( LifecycleCallbackMethodReference reference : beforeMappingReferencesWithFinalizedReturnType ) { + types.addAll( reference.getImportTypes() ); + } + for ( LifecycleCallbackMethodReference reference : afterMappingReferencesWithFinalizedReturnType ) { + types.addAll( reference.getImportTypes() ); + } return types; } diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/MappingMethod.java b/processor/src/main/java/org/mapstruct/ap/internal/model/MappingMethod.java index 5b9eda644..4c59216e0 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/model/MappingMethod.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/model/MappingMethod.java @@ -186,8 +186,8 @@ public abstract class MappingMethod extends ModelElement { return returnType + " " + getName() + "(" + join( parameters, ", " ) + ")"; } - private List filterMappingTarget(List methods, - boolean mustHaveMappingTargetParameter) { + protected static List filterMappingTarget( + List methods, boolean mustHaveMappingTargetParameter) { if ( methods == null ) { return Collections.emptyList(); } diff --git a/processor/src/main/resources/org/mapstruct/ap/internal/model/BeanMappingMethod.ftl b/processor/src/main/resources/org/mapstruct/ap/internal/model/BeanMappingMethod.ftl index 1b402a57f..a0fbe24b3 100644 --- a/processor/src/main/resources/org/mapstruct/ap/internal/model/BeanMappingMethod.ftl +++ b/processor/src/main/resources/org/mapstruct/ap/internal/model/BeanMappingMethod.ftl @@ -21,6 +21,12 @@ + <#list beforeMappingReferencesWithFinalizedReturnType as callback> + <@includeModel object=callback targetBeanName=finalizedResultName targetType=returnType/> + <#if !callback_has_next> + + + <#if !mapNullToDefault> if ( <#list sourceParametersExcludingPrimitives as sourceParam>${sourceParam.name} == null<#if sourceParam_has_next> && ) { return<#if returnType.name != "void"> <#if existingInstanceMapping>${resultName}<#if finalizerMethod??>.<@includeModel object=finalizerMethod /><#else>null; @@ -129,7 +135,20 @@ <#if returnType.name != "void"> <#if finalizerMethod??> - return ${resultName}.<@includeModel object=finalizerMethod />; + <#if (afterMappingReferencesWithFinalizedReturnType?size > 0)> + ${returnType.name} ${finalizedResultName} = ${resultName}.<@includeModel object=finalizerMethod />; + + <#list afterMappingReferencesWithFinalizedReturnType as callback> + <#if callback_index = 0> + + + <@includeModel object=callback targetBeanName=finalizedResultName targetType=returnType/> + + + return ${finalizedResultName}; + <#else> + return ${resultName}.<@includeModel object=finalizerMethod />; + <#else> return ${resultName}; 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 e3892f215..3d5fb7e53 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 @@ -43,12 +43,16 @@ public class BuilderLifecycleCallbacksTest { assertThat( context.getInvokedMethods() ) .contains( "beforeWithoutParameters", + "beforeWithTargetType", "beforeWithBuilderTargetType", "beforeWithBuilderTarget", "afterWithoutParameters", "afterWithBuilderTargetType", "afterWithBuilderTarget", - "afterWithBuilderTargetReturningTarget" + "afterWithBuilderTargetReturningTarget", + "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 96b9b30db..079be90a8 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 @@ -74,7 +74,15 @@ public class MappingContext { public Order afterWithBuilderTargetReturningTarget(@MappingTarget Order.Builder orderBuilder) { invokedMethods.add( "afterWithBuilderTargetReturningTarget" ); - return orderBuilder.create(); + // return null, so that @AfterMapping methods on the finalized object will be called in the tests + return null; + } + + @AfterMapping + public Order afterWithTargetReturningTarget(@MappingTarget Order order) { + invokedMethods.add( "afterWithTargetReturningTarget" ); + + return order; } public List getInvokedMethods() {