#2101 inherited properties need to be analysed against redefined properties when inheriting mappings (#2103)

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

@@ -6,15 +6,16 @@
 package org.mapstruct.ap.internal.model.source;
 
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.stream.Collectors;
 
+import org.mapstruct.ap.internal.gem.CollectionMappingStrategyGem;
 import org.mapstruct.ap.internal.model.common.Type;
 import org.mapstruct.ap.internal.model.common.TypeFactory;
-import org.mapstruct.ap.internal.gem.CollectionMappingStrategyGem;
 import org.mapstruct.ap.internal.util.accessor.Accessor;
 
 import static org.mapstruct.ap.internal.model.source.MappingOptions.getMappingTargetNamesBy;
@@ -136,9 +137,8 @@ public class MappingMethodOptions {
      *
      * @param templateMethod the template method with the options to inherit, may be {@code null}
      * @param isInverse if {@code true}, the specified options are from an inverse method
-     * @param method the source method
      */
-    public void applyInheritedOptions(SourceMethod templateMethod, boolean isInverse, SourceMethod method ) {
+    public void applyInheritedOptions(SourceMethod templateMethod, boolean isInverse) {
         MappingMethodOptions templateOptions = templateMethod.getOptions();
         if ( null != templateOptions ) {
             if ( !getIterableMapping().hasAnnotation() && templateOptions.getIterableMapping().hasAnnotation() ) {
@@ -200,13 +200,56 @@ public class MappingMethodOptions {
             }
 
             // now add all (does not override duplicates and leaves original mappings)
-            mappings.addAll( newMappings );
+            addAllNonRedefined( newMappings );
 
             // filter new mappings
             filterNestedTargetIgnores( mappings );
         }
     }
 
+    private void addAllNonRedefined(Set<MappingOptions> inheritedMappings) {
+        Set<String> redefinedSources = new HashSet<>();
+        Set<String> redefinedTargets = new HashSet<>();
+        for ( MappingOptions redefinedMappings : mappings ) {
+            if ( redefinedMappings.getSourceName() != null ) {
+                redefinedSources.add( redefinedMappings.getSourceName() );
+            }
+            if ( redefinedMappings.getTargetName() != null ) {
+                redefinedTargets.add( redefinedMappings.getTargetName() );
+            }
+        }
+        for ( MappingOptions inheritedMapping : inheritedMappings ) {
+            if ( inheritedMapping.isIgnored()
+                || ( !isRedefined( redefinedSources, inheritedMapping.getSourceName() )
+                && !isRedefined( redefinedTargets, inheritedMapping.getTargetName() ) )
+            ) {
+                mappings.add( inheritedMapping );
+            }
+        }
+    }
+
+    private boolean isRedefined(Set<String> redefinedNames, String inheritedName ) {
+        for ( String redefinedName : redefinedNames ) {
+            if ( elementsAreContainedIn( redefinedName, inheritedName ) ) {
+                return true;
+            }
+        }
+        return false;
+    }
+
+    private boolean elementsAreContainedIn( String redefinedName, String inheritedName ) {
+        if ( redefinedName.startsWith( inheritedName ) ) {
+            // it is possible to redefine an exact matching source name, because the same source can be mapped to
+            // multiple targets. It is not possible for target, but caught by the Set and equals methoded in
+            // MappingOptions
+            if ( redefinedName.length() > inheritedName.length() ) {
+                // redefined.lenght() > inherited.length(), first following character should be separator
+                return '.' == redefinedName.charAt( inheritedName.length() );
+            }
+        }
+        return false;
+    }
+
     public void applyIgnoreAll(SourceMethod method, TypeFactory typeFactory ) {
         CollectionMappingStrategyGem cms = method.getOptions().getMapper().getCollectionMappingStrategy();
         Type writeType = method.getResultType();

processor/src/main/java/org/mapstruct/ap/internal/processor/MapperCreationProcessor.java

@@ -447,10 +447,10 @@ public class MapperCreationProcessor implements ModelElementProcessor<List<Sourc
 
         // apply defined (@InheritConfiguration, @InheritInverseConfiguration) mappings
         if ( forwardTemplateMethod != null ) {
-            mappingOptions.applyInheritedOptions( forwardTemplateMethod, false, method );
+            mappingOptions.applyInheritedOptions( forwardTemplateMethod, false );
         }
         if ( inverseTemplateMethod != null ) {
-            mappingOptions.applyInheritedOptions( inverseTemplateMethod, true, method );
+            mappingOptions.applyInheritedOptions( inverseTemplateMethod, true );
         }
 
         // apply auto inherited options
@@ -460,11 +460,7 @@ public class MapperCreationProcessor implements ModelElementProcessor<List<Sourc
             // but.. there should not be an @InheritedConfiguration
             if ( forwardTemplateMethod == null && inheritanceStrategy.isApplyForward() ) {
                 if ( applicablePrototypeMethods.size() == 1 ) {
-                    mappingOptions.applyInheritedOptions(
+                    mappingOptions.applyInheritedOptions( first( applicablePrototypeMethods ), false );
-                        first( applicablePrototypeMethods ),
-                        false,
-                        method
-                    );
                 }
                 else if ( applicablePrototypeMethods.size() > 1 ) {
                     messager.printMessage(
@@ -477,11 +473,7 @@ public class MapperCreationProcessor implements ModelElementProcessor<List<Sourc
             // or no @InheritInverseConfiguration
             if ( inverseTemplateMethod == null && inheritanceStrategy.isApplyReverse() ) {
                 if ( applicableReversePrototypeMethods.size() == 1 ) {
-                    mappingOptions.applyInheritedOptions(
+                    mappingOptions.applyInheritedOptions( first( applicableReversePrototypeMethods ), true );
-                        first( applicableReversePrototypeMethods ),
-                        true,
-                        method
-                    );
                 }
                 else if ( applicableReversePrototypeMethods.size() > 1 ) {
                     messager.printMessage(

processor/src/test/java/org/mapstruct/ap/test/bugs/_2101/Issue2101AdditionalMapper.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._2101;
+
+import org.mapstruct.InheritConfiguration;
+import org.mapstruct.Mapper;
+import org.mapstruct.Mapping;
+import org.mapstruct.factory.Mappers;
+
+@Mapper
+public interface Issue2101AdditionalMapper {
+
+    Issue2101AdditionalMapper INSTANCE = Mappers.getMapper( Issue2101AdditionalMapper.class );
+
+    @Mapping(target = "value1", source = "value.nestedValue1")
+    @Mapping(target = "value2", source = "value.nestedValue2")
+    @Mapping(target = "value3", source = "valueThrowOffPath")
+    Target map1(Source source);
+
+    @InheritConfiguration
+    @Mapping(target = "value2", source = "value.nestedValue1")
+    @Mapping(target = "value3", constant = "test")
+    Target map2(Source source);
+
+    //CHECKSTYLE:OFF
+    class Source {
+        public String valueThrowOffPath;
+        public NestedSource value;
+    }
+
+    class Target {
+        public String value1;
+        public String value2;
+        public String value3;
+    }
+
+    class NestedSource {
+        public String nestedValue1;
+        public String nestedValue2;
+    }
+    //CHECKSTYLE:ON
+}

processor/src/test/java/org/mapstruct/ap/test/bugs/_2101/Issue2101Mapper.java

@@ -0,0 +1,49 @@
+/*
+ * 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._2101;
+
+import org.mapstruct.InheritInverseConfiguration;
+import org.mapstruct.Mapper;
+import org.mapstruct.Mapping;
+import org.mapstruct.factory.Mappers;
+
+@Mapper
+public interface Issue2101Mapper {
+
+    Issue2101Mapper INSTANCE = Mappers.getMapper( Issue2101Mapper.class );
+
+    @Mapping(target = "value1", source = "codeValue1")
+    @Mapping(target = "value2", source = "codeValue2")
+    Source map(Target target);
+
+    @InheritInverseConfiguration
+    @Mapping(target = "codeValue1.code", constant = "c1")
+    @Mapping(target = "codeValue1.value", source = "value1")
+    @Mapping(target = "codeValue2.code", constant = "c2")
+    @Mapping(target = "codeValue2.value", source = "value2")
+    Target map(Source source);
+
+    default String mapFrom( CodeValuePair cv ) {
+        return cv.code;
+    }
+
+    //CHECKSTYLE:OFF
+    class Source {
+        public String value1;
+        public String value2;
+    }
+
+    class Target {
+        public CodeValuePair codeValue1;
+        public CodeValuePair codeValue2;
+    }
+
+    class CodeValuePair {
+        public String code;
+        public String value;
+    }
+    //CHECKSTYLE:ON
+}

processor/src/test/java/org/mapstruct/ap/test/bugs/_2101/Issue2101Test.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._2101;
+
+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;
+
+@IssueKey("2101")
+@RunWith(AnnotationProcessorTestRunner.class)
+public class Issue2101Test {
+
+    @Test
+    @WithClasses(Issue2101Mapper.class)
+    public void shouldMap() {
+
+        Issue2101Mapper.Source source = new Issue2101Mapper.Source();
+        source.value1 = "v1";
+        source.value2 = "v2";
+
+        Issue2101Mapper.Target target = Issue2101Mapper.INSTANCE.map( source );
+
+        assertThat( target.codeValue1.code ).isEqualTo( "c1" );
+        assertThat( target.codeValue1.value ).isEqualTo( "v1" );
+        assertThat( target.codeValue2.code ).isEqualTo( "c2" );
+        assertThat( target.codeValue2.value ).isEqualTo( "v2" );
+
+    }
+
+    @Test
+    @WithClasses(Issue2101AdditionalMapper.class)
+    public void shouldMapSomeAdditionalTests1() {
+        Issue2101AdditionalMapper.Source source = new Issue2101AdditionalMapper.Source();
+        source.value = new Issue2101AdditionalMapper.NestedSource();
+        source.value.nestedValue1 = "value1";
+        source.value.nestedValue2 = "value2";
+        source.valueThrowOffPath = "value3";
+
+        Issue2101AdditionalMapper.Target target = Issue2101AdditionalMapper.INSTANCE.map1( source );
+        assertThat( target.value1 ).isEqualTo( "value1" );
+        assertThat( target.value2 ).isEqualTo( "value2" );
+        assertThat( target.value3 ).isEqualTo( "value3" );
+    }
+
+    @Test
+    @WithClasses(Issue2101AdditionalMapper.class)
+    public void shouldMapSomeAdditionalTests2() {
+        Issue2101AdditionalMapper.Source source = new Issue2101AdditionalMapper.Source();
+        source.value = new Issue2101AdditionalMapper.NestedSource();
+        source.value.nestedValue1 = "value1";
+        source.value.nestedValue2 = "value2";
+        source.valueThrowOffPath = "value3";
+
+        Issue2101AdditionalMapper.Target target = Issue2101AdditionalMapper.INSTANCE.map2( source );
+        assertThat( target.value1 ).isEqualTo( "value1" );
+        assertThat( target.value2 ).isEqualTo( "value1" );
+        assertThat( target.value3 ).isEqualTo( "test" );
+
+    }
+}