#3678 improve removal of duplicate lifecycle callback methods

processor/src/main/java/org/mapstruct/ap/internal/model/BeanMappingMethod.java

@@ -407,10 +407,8 @@ public class BeanMappingMethod extends NormalTypeMappingMethod {
                 ) );
 
                 // remove methods that are already being invoked
-                removeMappingReferencesWithoutSourceParameters( beforeMappingReferencesWithFinalizedReturnType );
+                beforeMappingReferencesWithFinalizedReturnType.removeAll( beforeMappingMethods );
-                removeMappingReferencesWithoutSourceParameters( afterMappingReferencesWithFinalizedReturnType );
+                afterMappingReferencesWithFinalizedReturnType.removeAll( afterMappingMethods );
-                removeMappingReferencesWithSingleSourceParameter( beforeMappingReferencesWithFinalizedReturnType );
-                removeMappingReferencesWithSingleSourceParameter( afterMappingReferencesWithFinalizedReturnType );
             }
 
             Map<String, PresenceCheck> presenceChecksByParameter = new LinkedHashMap<>();
@@ -453,21 +451,6 @@ public class BeanMappingMethod extends NormalTypeMappingMethod {
             );
         }
 
-        private void removeMappingReferencesWithoutSourceParameters(List<LifecycleCallbackMethodReference> references) {
-            references.removeIf( r -> r.getSourceParameters().isEmpty() && r.getReturnType().isVoid() );
-        }
-
-        private void removeMappingReferencesWithSingleSourceParameter(
-            List<LifecycleCallbackMethodReference> 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 );

processor/src/main/java/org/mapstruct/ap/internal/model/LifecycleCallbackMethodReference.java

@@ -5,6 +5,7 @@
  */
 package org.mapstruct.ap.internal.model;
 
+import java.util.Objects;
 import java.util.Set;
 
 import org.mapstruct.ap.internal.model.common.Parameter;
@@ -118,4 +119,36 @@ public class LifecycleCallbackMethodReference extends MethodReference {
             containingMethod,
             existingVariableNames );
     }
+
+    @Override
+    public boolean equals(Object obj) {
+        if ( this == obj ) {
+            return true;
+        }
+        if ( !super.equals( obj ) ) {
+            return false;
+        }
+        if ( getClass() != obj.getClass() ) {
+            return false;
+        }
+        LifecycleCallbackMethodReference other = (LifecycleCallbackMethodReference) obj;
+        if ( !Objects.equals( declaringType, other.declaringType )) {
+            return false;
+        }
+        if ( !Objects.equals( methodReturnType, other.methodReturnType )) {
+            return false;
+        }
+        if ( !Objects.equals( methodResultType, other.methodResultType )) {
+            return false;
+        }
+        if ( !Objects.equals( targetVariableName, other.targetVariableName )) {
+            return false;
+        }
+        return true;
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash( super.hashCode(), declaringType, methodReturnType, methodResultType, targetVariableName );
+    }
 }

processor/src/main/java/org/mapstruct/ap/internal/model/MethodReference.java

@@ -387,9 +387,6 @@ public class MethodReference extends ModelElement implements Assignment {
         if ( this == obj ) {
             return true;
         }
-        if ( !super.equals( obj ) ) {
-            return false;
-        }
         if ( getClass() != obj.getClass() ) {
             return false;
         }
@@ -400,6 +397,9 @@ public class MethodReference extends ModelElement implements Assignment {
         if ( !Objects.equals( providingParameter, other.providingParameter ) ) {
             return false;
         }
+        if ( !Objects.equals( name, other.name ) ) {
+            return false;
+        }
 
         return true;
     }

processor/src/main/java/org/mapstruct/ap/internal/model/source/selector/SelectionCriteria.java

@@ -7,7 +7,6 @@ package org.mapstruct.ap.internal.model.source.selector;
 
 import java.util.Collections;
 import java.util.List;
-
 import javax.lang.model.type.TypeMirror;
 
 import org.mapstruct.ap.internal.model.common.SourceRHS;
@@ -116,7 +115,7 @@ public class SelectionCriteria {
         this.type = preferUpdateMapping ? Type.PREFER_UPDATE_MAPPING : null;
     }
 
-    public boolean hasQualfiers() {
+    public boolean hasQualifiers() {
         return !qualifyingInfo.qualifiedByNames().isEmpty() || !qualifyingInfo.qualifiers().isEmpty();
     }
 

processor/src/main/java/org/mapstruct/ap/internal/processor/creation/MappingResolverImpl.java

@@ -5,10 +5,6 @@
  */
 package org.mapstruct.ap.internal.processor.creation;
 
-import static org.mapstruct.ap.internal.util.Collections.first;
-import static org.mapstruct.ap.internal.util.Collections.firstKey;
-import static org.mapstruct.ap.internal.util.Collections.firstValue;
-
 import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.Iterator;
@@ -20,7 +16,6 @@ import java.util.Set;
 import java.util.function.Function;
 import java.util.function.Supplier;
 import java.util.stream.Collectors;
-
 import javax.lang.model.element.AnnotationMirror;
 import javax.lang.model.element.Element;
 import javax.lang.model.element.ExecutableElement;
@@ -67,6 +62,10 @@ import org.mapstruct.ap.internal.util.NativeTypes;
 import org.mapstruct.ap.internal.util.Strings;
 import org.mapstruct.ap.internal.util.TypeUtils;
 
+import static org.mapstruct.ap.internal.util.Collections.first;
+import static org.mapstruct.ap.internal.util.Collections.firstKey;
+import static org.mapstruct.ap.internal.util.Collections.firstValue;
+
 /**
  * The one and only implementation of {@link MappingResolver}. The class has been split into an interface an
  * implementation for the sake of avoiding package dependencies. Specifically, this implementation refers to classes
@@ -335,7 +334,7 @@ public class MappingResolverImpl implements MappingResolver {
         }
 
         private boolean hasQualfiers() {
-            return selectionCriteria != null && selectionCriteria.hasQualfiers();
+            return selectionCriteria != null && selectionCriteria.hasQualifiers();
         }
 
         private void printQualifierMessage(SelectionCriteria selectionCriteria ) {

processor/src/test/java/org/mapstruct/ap/test/bugs/_3678/Issue3678Mapper.java

@@ -11,54 +11,76 @@ import java.util.List;
 import org.mapstruct.AfterMapping;
 import org.mapstruct.BeforeMapping;
 import org.mapstruct.Mapper;
-import org.mapstruct.ReportingPolicy;
+import org.mapstruct.Mapping;
 
-@Mapper(unmappedSourcePolicy = ReportingPolicy.ERROR)
+@Mapper
 public abstract class Issue3678Mapper {
 
-    abstract SimpleDestination sourceToDestination(SimpleSource source);
+    @Mapping( target = "name", source = "sourceA.name")
+    @Mapping( target = "description", source = "sourceB.description")
+    abstract Target mapTwoSources(SourceA sourceA, SourceB sourceB);
+
+    @Mapping( target = "description", constant = "some description")
+    abstract Target mapSingleSource(SourceA sourceA);
 
     List<String> invocations = new ArrayList<>();
 
     @BeforeMapping
-    void beforeMapping(SimpleSource simpleSource) {
+    void beforeMappingSourceA(SourceA sourceA) {
-        invocations.add( "beforeMapping" );
+        invocations.add( "beforeMappingSourceA" );
     }
 
     @AfterMapping
-    void afterMapping(SimpleSource simpleSource) {
+    void afterMappingSourceB(SourceA sourceA) {
-        invocations.add( "afterMapping" );
+        invocations.add( "afterMappingSourceA" );
+    }
+
+    @BeforeMapping
+    void beforeMappingSourceB(SourceB sourceB) {
+        invocations.add( "beforeMappingSourceB" );
+    }
+
+    @AfterMapping
+    void afterMappingSourceB(SourceB sourceB) {
+        invocations.add( "afterMappingSourceB" );
     }
 
     public List<String> getInvocations() {
         return invocations;
     }
 
-    public static class SimpleSource {
+    public static class SourceA {
 
         private final String name;
-        private final String description;
 
-        public SimpleSource(String name, String description) {
+        public SourceA(String name) {
             this.name = name;
-            this.description = description;
         }
 
         public String getName() {
             return name;
         }
+    }
+
+    public static class SourceB {
+
+        private final String description;
+
+        public SourceB(String description) {
+            this.description = description;
+        }
 
         public String getDescription() {
             return description;
         }
     }
 
-    public static final class SimpleDestination {
+    public static final class Target {
 
         private final String name;
         private final String description;
 
-        private SimpleDestination(String name, String description) {
+        private Target(String name, String description) {
             this.name = name;
             this.description = description;
         }
@@ -93,8 +115,8 @@ public abstract class Issue3678Mapper {
                 return this;
             }
 
-            public SimpleDestination build() {
+            public Target build() {
-                return new SimpleDestination( this.name, this.description );
+                return new Target( this.name, this.description );
             }
         }
     }

processor/src/test/java/org/mapstruct/ap/test/bugs/_3678/Issue3678Test.java

@@ -14,21 +14,38 @@ import org.mapstruct.factory.Mappers;
 import static org.assertj.core.api.Assertions.assertThat;
 
 @IssueKey("3678")
+@WithClasses(Issue3678Mapper.class)
 public class Issue3678Test {
 
-    @WithClasses({
-        Issue3678Mapper.class,
-    })
     @ProcessorTest
-    void ignoreMappingsWithoutSourceShouldBeInvertible() {
+    void beforeAndAfterMappingOnlyCalledOnceForTwoSources() {
 
         Issue3678Mapper mapper = Mappers.getMapper( Issue3678Mapper.class );
-        Issue3678Mapper.SimpleSource source = new Issue3678Mapper.SimpleSource( "name", "description" );
+        Issue3678Mapper.SourceA sourceA = new Issue3678Mapper.SourceA( "name" );
-        Issue3678Mapper.SimpleDestination simpleDestination = mapper.sourceToDestination( source );
+        Issue3678Mapper.SourceB sourceB = new Issue3678Mapper.SourceB( "description" );
+        Issue3678Mapper.Target target = mapper.mapTwoSources( sourceA, sourceB );
 
         assertThat( mapper.getInvocations() )
-            .containsExactly( "beforeMapping", "afterMapping" )
+            .containsExactly(
-            .doesNotHaveDuplicates();
+                "beforeMappingSourceA",
+                "beforeMappingSourceB",
+                "afterMappingSourceA",
+                "afterMappingSourceB"
+            );
+    }
+
+    @ProcessorTest
+    void beforeAndAfterMappingOnlyCalledOnceForSingleSource() {
+
+        Issue3678Mapper mapper = Mappers.getMapper( Issue3678Mapper.class );
+        Issue3678Mapper.SourceA sourceA = new Issue3678Mapper.SourceA( "name" );
+        Issue3678Mapper.Target target = mapper.mapSingleSource( sourceA );
+
+        assertThat( mapper.getInvocations() )
+            .containsExactly(
+                "beforeMappingSourceA",
+                "afterMappingSourceA"
+            );
     }
 
 }