From 8071723bae3d440d2cfa2027dcee04bc73ac2e09 Mon Sep 17 00:00:00 2001
From: Andreas Gudian <agudian@apache.org>
Date: Thu, 1 Sep 2016 22:31:21 +0200
Subject: [PATCH] #880 Fix override behaviour for componentModel and
 unmappedTargetPolicy.

---
 .../org/mapstruct/ap/MappingProcessor.java    |  5 +-
 .../ap/internal/model/BeanMappingMethod.java  | 22 +------
 ...nnotationBasedComponentModelProcessor.java | 10 +--
 .../processor/MapperServiceProcessor.java     | 10 +--
 .../ap/internal/util/MapperConfiguration.java | 44 ++++++++-----
 .../mapstruct/ap/test/bugs/_880/Config.java   | 30 +++++++++
 .../DefaultsToProcessorOptionsMapper.java     | 30 +++++++++
 .../ap/test/bugs/_880/Issue880Test.java       | 63 +++++++++++++++++++
 .../mapstruct/ap/test/bugs/_880/Poodle.java}  | 26 +++-----
 .../_880/UsesConfigFromAnnotationMapper.java  | 30 +++++++++
 10 files changed, 200 insertions(+), 70 deletions(-)
 create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_880/Config.java
 create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_880/DefaultsToProcessorOptionsMapper.java
 create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_880/Issue880Test.java
 rename processor/src/{main/java/org/mapstruct/ap/internal/option/OptionsHelper.java => test/java/org/mapstruct/ap/test/bugs/_880/Poodle.java} (57%)
 create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_880/UsesConfigFromAnnotationMapper.java

diff --git a/processor/src/main/java/org/mapstruct/ap/MappingProcessor.java b/processor/src/main/java/org/mapstruct/ap/MappingProcessor.java
index b1deaa41e..9188fb417 100644
--- a/processor/src/main/java/org/mapstruct/ap/MappingProcessor.java
+++ b/processor/src/main/java/org/mapstruct/ap/MappingProcessor.java
@@ -127,13 +127,12 @@ public class MappingProcessor extends AbstractProcessor {
 
     private Options createOptions() {
         String unmappedTargetPolicy = processingEnv.getOptions().get( UNMAPPED_TARGET_POLICY );
-        String defaultComponentModel = processingEnv.getOptions().get( DEFAULT_COMPONENT_MODEL );
 
         return new Options(
             Boolean.valueOf( processingEnv.getOptions().get( SUPPRESS_GENERATOR_TIMESTAMP ) ),
             Boolean.valueOf( processingEnv.getOptions().get( SUPPRESS_GENERATOR_VERSION_INFO_COMMENT ) ),
-            unmappedTargetPolicy != null ? ReportingPolicy.valueOf( unmappedTargetPolicy ) : null,
-            defaultComponentModel == null ? "default" : defaultComponentModel,
+            unmappedTargetPolicy != null ? ReportingPolicy.valueOf( unmappedTargetPolicy.toUpperCase() ) : null,
+            processingEnv.getOptions().get( DEFAULT_COMPONENT_MODEL ),
             Boolean.valueOf( processingEnv.getOptions().get( ALWAYS_GENERATE_SERVICE_FILE ) )
         );
     }
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 cf5f50252..13ad6037d 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
@@ -508,32 +508,16 @@ public class BeanMappingMethod extends MappingMethod {
             return method.getResultType().getPropertyReadAccessors().get( propertyName );
         }
 
-        /**
-         * Returns the effective policy for reporting unmapped getReturnType properties. If explicitly set via
-         * {@code Mapper}, this value will be returned. Otherwise the value from the corresponding processor option will
-         * be returned. If that is not set either, the default value from {@code Mapper#unmappedTargetPolicy()} will be
-         * returned.
-         *
-         * @return The effective policy for reporting unmapped target properties.
-         */
-        private ReportingPolicy getEffectiveUnmappedTargetPolicy() {
+        private ReportingPolicy getUnmappedTargetPolicy() {
             MapperConfiguration mapperSettings = MapperConfiguration.getInstanceOn( ctx.getMapperTypeElement() );
-            boolean setViaAnnotation = mapperSettings.isSetUnmappedTargetPolicy();
-            ReportingPolicy annotationValue = ReportingPolicy.valueOf( mapperSettings.unmappedTargetPolicy() );
 
-            if ( setViaAnnotation
-                || ctx.getOptions().getUnmappedTargetPolicy() == null ) {
-                return annotationValue;
-            }
-            else {
-                return ctx.getOptions().getUnmappedTargetPolicy();
-            }
+            return mapperSettings.unmappedTargetPolicy( ctx.getOptions() );
         }
 
         private void reportErrorForUnmappedTargetPropertiesIfRequired() {
 
             // fetch settings from element to implement
-            ReportingPolicy unmappedTargetPolicy = getEffectiveUnmappedTargetPolicy();
+            ReportingPolicy unmappedTargetPolicy = getUnmappedTargetPolicy();
 
             if ( !unprocessedTargetProperties.isEmpty() && unmappedTargetPolicy.requiresReport() ) {
 
diff --git a/processor/src/main/java/org/mapstruct/ap/internal/processor/AnnotationBasedComponentModelProcessor.java b/processor/src/main/java/org/mapstruct/ap/internal/processor/AnnotationBasedComponentModelProcessor.java
index 812eee806..cf0f3d8ef 100644
--- a/processor/src/main/java/org/mapstruct/ap/internal/processor/AnnotationBasedComponentModelProcessor.java
+++ b/processor/src/main/java/org/mapstruct/ap/internal/processor/AnnotationBasedComponentModelProcessor.java
@@ -32,7 +32,6 @@ import org.mapstruct.ap.internal.model.Field;
 import org.mapstruct.ap.internal.model.Mapper;
 import org.mapstruct.ap.internal.model.MapperReference;
 import org.mapstruct.ap.internal.model.common.TypeFactory;
-import org.mapstruct.ap.internal.option.OptionsHelper;
 import org.mapstruct.ap.internal.util.MapperConfiguration;
 
 /**
@@ -51,13 +50,10 @@ public abstract class AnnotationBasedComponentModelProcessor implements ModelEle
     public Mapper process(ProcessorContext context, TypeElement mapperTypeElement, Mapper mapper) {
         this.typeFactory = context.getTypeFactory();
 
-        String componentModel = MapperConfiguration.getInstanceOn( mapperTypeElement ).componentModel();
-        String effectiveComponentModel = OptionsHelper.getEffectiveComponentModel(
-            context.getOptions(),
-            componentModel
-        );
+        String componentModel = MapperConfiguration.getInstanceOn( mapperTypeElement )
+                                                   .componentModel( context.getOptions() );
 
-        if ( !getComponentModelIdentifier().equalsIgnoreCase( effectiveComponentModel ) ) {
+        if ( !getComponentModelIdentifier().equalsIgnoreCase( componentModel ) ) {
             return mapper;
         }
 
diff --git a/processor/src/main/java/org/mapstruct/ap/internal/processor/MapperServiceProcessor.java b/processor/src/main/java/org/mapstruct/ap/internal/processor/MapperServiceProcessor.java
index 6c301f0e8..b8533e967 100644
--- a/processor/src/main/java/org/mapstruct/ap/internal/processor/MapperServiceProcessor.java
+++ b/processor/src/main/java/org/mapstruct/ap/internal/processor/MapperServiceProcessor.java
@@ -28,7 +28,6 @@ import javax.tools.StandardLocation;
 import org.mapstruct.ap.internal.model.GeneratedType;
 import org.mapstruct.ap.internal.model.Mapper;
 import org.mapstruct.ap.internal.model.ServicesEntry;
-import org.mapstruct.ap.internal.option.OptionsHelper;
 import org.mapstruct.ap.internal.util.MapperConfiguration;
 import org.mapstruct.ap.internal.writer.ModelWriter;
 
@@ -50,13 +49,10 @@ public class MapperServiceProcessor  implements ModelElementProcessor<Mapper, Vo
             spiGenerationNeeded = true;
         }
         else {
-            String componentModel = MapperConfiguration.getInstanceOn( mapperTypeElement ).componentModel();
-            String effectiveComponentModel = OptionsHelper.getEffectiveComponentModel(
-                    context.getOptions(),
-                    componentModel
-            );
+            String componentModel =
+                MapperConfiguration.getInstanceOn( mapperTypeElement ).componentModel( context.getOptions() );
 
-            spiGenerationNeeded = "default".equals( effectiveComponentModel );
+            spiGenerationNeeded = "default".equals( componentModel );
         }
 
         if ( !context.isErroneous() && spiGenerationNeeded && mapper.hasCustomImplementation() ) {
diff --git a/processor/src/main/java/org/mapstruct/ap/internal/util/MapperConfiguration.java b/processor/src/main/java/org/mapstruct/ap/internal/util/MapperConfiguration.java
index d52b0d8e0..81037bc18 100644
--- a/processor/src/main/java/org/mapstruct/ap/internal/util/MapperConfiguration.java
+++ b/processor/src/main/java/org/mapstruct/ap/internal/util/MapperConfiguration.java
@@ -27,12 +27,14 @@ import javax.lang.model.element.Element;
 import javax.lang.model.type.DeclaredType;
 import javax.lang.model.type.TypeMirror;
 
+import org.mapstruct.ap.internal.option.Options;
+import org.mapstruct.ap.internal.option.ReportingPolicy;
 import org.mapstruct.ap.internal.prism.CollectionMappingStrategyPrism;
 import org.mapstruct.ap.internal.prism.MapperConfigPrism;
 import org.mapstruct.ap.internal.prism.MapperPrism;
 import org.mapstruct.ap.internal.prism.MappingInheritanceStrategyPrism;
-import org.mapstruct.ap.internal.prism.NullValueMappingStrategyPrism;
 import org.mapstruct.ap.internal.prism.NullValueCheckStrategyPrism;
+import org.mapstruct.ap.internal.prism.NullValueMappingStrategyPrism;
 
 /**
  * Provides an aggregated view to the settings given via {@link org.mapstruct.Mapper} and
@@ -109,13 +111,21 @@ public class MapperConfiguration {
         return mapperPrism.imports();
     }
 
-    public String unmappedTargetPolicy() {
-        if ( mapperConfigPrism != null && mapperPrism.values.unmappedTargetPolicy() == null ) {
-            return mapperConfigPrism.unmappedTargetPolicy();
+    public ReportingPolicy unmappedTargetPolicy(Options options) {
+        if ( mapperPrism.values.unmappedTargetPolicy() != null ) {
+            return ReportingPolicy.valueOf( mapperPrism.unmappedTargetPolicy() );
         }
-        else {
-            return mapperPrism.unmappedTargetPolicy();
+
+        if ( mapperConfigPrism != null && mapperConfigPrism.values.unmappedTargetPolicy() != null ) {
+            return ReportingPolicy.valueOf( mapperConfigPrism.unmappedTargetPolicy() );
         }
+
+        if ( options.getUnmappedTargetPolicy() != null ) {
+            return options.getUnmappedTargetPolicy();
+        }
+
+        // fall back to default defined in the annotation
+        return ReportingPolicy.valueOf( mapperPrism.unmappedTargetPolicy() );
     }
 
     public CollectionMappingStrategyPrism getCollectionMappingStrategy() {
@@ -169,13 +179,20 @@ public class MapperConfiguration {
     }
 
 
-    public String componentModel() {
-        if ( mapperConfigPrism != null && mapperPrism.values.componentModel() == null ) {
-            return mapperConfigPrism.componentModel();
-        }
-        else {
+    public String componentModel(Options options) {
+        if ( mapperPrism.values.componentModel() != null ) {
             return mapperPrism.componentModel();
         }
+
+        if ( mapperConfigPrism != null && mapperConfigPrism.values.componentModel() != null ) {
+            return mapperConfigPrism.componentModel();
+        }
+
+        if ( options.getDefaultComponentModel() != null ) {
+            return options.getDefaultComponentModel();
+        }
+
+        return mapperPrism.componentModel(); // fall back to default defined in the annotation
     }
 
     public DeclaredType config() {
@@ -186,12 +203,7 @@ public class MapperConfiguration {
         return mapperPrism.isValid;
     }
 
-    public boolean isSetUnmappedTargetPolicy() {
-        return mapperPrism.values.unmappedTargetPolicy() != null;
-    }
-
     public AnnotationMirror getAnnotationMirror() {
         return mapperPrism.mirror;
     }
-
 }
diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_880/Config.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_880/Config.java
new file mode 100644
index 000000000..960bf4e47
--- /dev/null
+++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_880/Config.java
@@ -0,0 +1,30 @@
+/**
+ *  Copyright 2012-2016 Gunnar Morling (http://www.gunnarmorling.de/)
+ *  and/or other contributors as indicated by the @authors tag. See the
+ *  copyright.txt file in the distribution for a full listing of all
+ *  contributors.
+ *
+ *  Licensed under the Apache License, Version 2.0 (the "License");
+ *  you may not use this file except in compliance with the License.
+ *  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+package org.mapstruct.ap.test.bugs._880;
+
+import org.mapstruct.MapperConfig;
+import org.mapstruct.ReportingPolicy;
+
+/**
+ * @author Andreas Gudian
+ */
+@MapperConfig(unmappedTargetPolicy = ReportingPolicy.WARN)
+public interface Config {
+
+}
diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_880/DefaultsToProcessorOptionsMapper.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_880/DefaultsToProcessorOptionsMapper.java
new file mode 100644
index 000000000..550e49c46
--- /dev/null
+++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_880/DefaultsToProcessorOptionsMapper.java
@@ -0,0 +1,30 @@
+/**
+ *  Copyright 2012-2016 Gunnar Morling (http://www.gunnarmorling.de/)
+ *  and/or other contributors as indicated by the @authors tag. See the
+ *  copyright.txt file in the distribution for a full listing of all
+ *  contributors.
+ *
+ *  Licensed under the Apache License, Version 2.0 (the "License");
+ *  you may not use this file except in compliance with the License.
+ *  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+package org.mapstruct.ap.test.bugs._880;
+
+import org.mapstruct.Mapper;
+
+/**
+ * @author Andreas Gudian
+ *
+ */
+@Mapper
+public interface DefaultsToProcessorOptionsMapper {
+    Poodle metamorph(Object essence);
+}
diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_880/Issue880Test.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_880/Issue880Test.java
new file mode 100644
index 000000000..7af00c5a5
--- /dev/null
+++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_880/Issue880Test.java
@@ -0,0 +1,63 @@
+/**
+ *  Copyright 2012-2016 Gunnar Morling (http://www.gunnarmorling.de/)
+ *  and/or other contributors as indicated by the @authors tag. See the
+ *  copyright.txt file in the distribution for a full listing of all
+ *  contributors.
+ *
+ *  Licensed under the Apache License, Version 2.0 (the "License");
+ *  you may not use this file except in compliance with the License.
+ *  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+package org.mapstruct.ap.test.bugs._880;
+
+import javax.tools.Diagnostic.Kind;
+
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mapstruct.ap.testutil.WithClasses;
+import org.mapstruct.ap.testutil.compilation.annotation.CompilationResult;
+import org.mapstruct.ap.testutil.compilation.annotation.Diagnostic;
+import org.mapstruct.ap.testutil.compilation.annotation.ExpectedCompilationOutcome;
+import org.mapstruct.ap.testutil.compilation.annotation.ProcessorOption;
+import org.mapstruct.ap.testutil.compilation.annotation.ProcessorOptions;
+import org.mapstruct.ap.testutil.runner.AnnotationProcessorTestRunner;
+import org.mapstruct.ap.testutil.runner.GeneratedSource;
+import org.springframework.stereotype.Component;
+
+/**
+ * @author Andreas Gudian
+ */
+@RunWith(AnnotationProcessorTestRunner.class)
+@WithClasses({
+    UsesConfigFromAnnotationMapper.class,
+    DefaultsToProcessorOptionsMapper.class,
+    Poodle.class,
+    Config.class })
+@ProcessorOptions({
+    @ProcessorOption(name = "mapstruct.defaultComponentModel", value = "spring"),
+    @ProcessorOption(name = "mapstruct.unmappedTargetPolicy", value = "ignore") })
+public class Issue880Test {
+    @Rule
+    public GeneratedSource generatedSource = new GeneratedSource();
+
+    @Test
+    @ExpectedCompilationOutcome(
+        value = CompilationResult.SUCCEEDED,
+        diagnostics = @Diagnostic(kind = Kind.WARNING,
+            type = UsesConfigFromAnnotationMapper.class, line = 29,
+            messageRegExp = "Unmapped target property: \"core\"\\."))
+    public void compilationSucceedsAndAppliesCorrectComponentModel() {
+        generatedSource.forMapper( UsesConfigFromAnnotationMapper.class ).containsNoImportFor( Component.class );
+
+        generatedSource.forMapper( DefaultsToProcessorOptionsMapper.class ).containsImportFor( Component.class );
+    }
+}
diff --git a/processor/src/main/java/org/mapstruct/ap/internal/option/OptionsHelper.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_880/Poodle.java
similarity index 57%
rename from processor/src/main/java/org/mapstruct/ap/internal/option/OptionsHelper.java
rename to processor/src/test/java/org/mapstruct/ap/test/bugs/_880/Poodle.java
index 9145e17c5..0da9f6e7b 100644
--- a/processor/src/main/java/org/mapstruct/ap/internal/option/OptionsHelper.java
+++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_880/Poodle.java
@@ -16,30 +16,20 @@
  *  See the License for the specific language governing permissions and
  *  limitations under the License.
  */
-package org.mapstruct.ap.internal.option;
-
+package org.mapstruct.ap.test.bugs._880;
 
 /**
- * Helper class for dealing with {@link Options}.
- *
  * @author Andreas Gudian
+ *
  */
-public class OptionsHelper {
+public class Poodle {
+    private String core;
 
-    private OptionsHelper() {
+    public String getCore() {
+        return core;
     }
 
-    /**
-     * @param options the options
-     * @param locallyDeclaredComponentModel the locally declared component model
-     *
-     * @return the effective component model to be used
-     */
-    public static String getEffectiveComponentModel(Options options, String locallyDeclaredComponentModel) {
-        if ( "default".equals( locallyDeclaredComponentModel ) ) {
-            return options.getDefaultComponentModel();
-        }
-
-        return locallyDeclaredComponentModel;
+    public void setCore(String core) {
+        this.core = core;
     }
 }
diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_880/UsesConfigFromAnnotationMapper.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_880/UsesConfigFromAnnotationMapper.java
new file mode 100644
index 000000000..6b945419e
--- /dev/null
+++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_880/UsesConfigFromAnnotationMapper.java
@@ -0,0 +1,30 @@
+/**
+ *  Copyright 2012-2016 Gunnar Morling (http://www.gunnarmorling.de/)
+ *  and/or other contributors as indicated by the @authors tag. See the
+ *  copyright.txt file in the distribution for a full listing of all
+ *  contributors.
+ *
+ *  Licensed under the Apache License, Version 2.0 (the "License");
+ *  you may not use this file except in compliance with the License.
+ *  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+package org.mapstruct.ap.test.bugs._880;
+
+import org.mapstruct.Mapper;
+
+/**
+ * @author Andreas Gudian
+ *
+ */
+@Mapper(componentModel = "default", config = Config.class)
+public interface UsesConfigFromAnnotationMapper {
+    Poodle metamorph(Object essence);
+}