[MNG-8141] Model builder should report problems it finds during build

maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java

@@ -97,6 +97,30 @@
 @Named
 @Singleton
 public class DefaultModelBuilder implements ModelBuilder {
+    /**
+     * Key for "fail on invalid model" property.
+     * <p>
+     * Visible for testing.
+     */
+    static final String FAIL_ON_INVALID_MODEL = "maven.modelBuilder.failOnInvalidModel";
+
+    /**
+     * Checks user and system properties (in this order) for value of {@link #FAIL_ON_INVALID_MODEL} property key, if
+     * set and returns it. If not set, defaults to {@code true}.
+     * <p>
+     * This is only meant to provide "escape hatch" for those builds, that are for some reason stuck with invalid models.
+     */
+    private static boolean isFailOnInvalidModel(ModelBuildingRequest request) {
+        String val = request.getUserProperties().getProperty(FAIL_ON_INVALID_MODEL);
+        if (val == null) {
+            val = request.getSystemProperties().getProperty(FAIL_ON_INVALID_MODEL);
+        }
+        if (val != null) {
+            return Boolean.parseBoolean(val);
+        }
+        return true;
+    }
+
     @Inject
     private ModelProcessor modelProcessor;
 
@@ -253,6 +277,7 @@ public ModelBuildingResult build(ModelBuildingRequest request) throws ModelBuild
     protected ModelBuildingResult build(ModelBuildingRequest request, Collection<String> importIds)
             throws ModelBuildingException {
         // phase 1
+        boolean failOnInvalidModel = isFailOnInvalidModel(request);
         DefaultModelBuildingResult result = new DefaultModelBuildingResult();
 
         DefaultModelProblemCollector problems = new DefaultModelProblemCollector(result);
@@ -306,7 +331,7 @@ protected ModelBuildingResult build(ModelBuildingRequest request, Collection<Str
             profileActivationContext.setProjectProperties(tmpModel.getProperties());
 
             Map<String, Activation> interpolatedActivations =
-                    getInterpolatedActivations(rawModel, profileActivationContext, problems);
+                    getInterpolatedActivations(rawModel, profileActivationContext, failOnInvalidModel, problems);
             injectProfileActivations(tmpModel, interpolatedActivations);
 
             List<Profile> activePomProfiles =
@@ -430,8 +455,12 @@ private interface InterpolateString {
     }
 
     private Map<String, Activation> getInterpolatedActivations(
-            Model rawModel, DefaultProfileActivationContext context, DefaultModelProblemCollector problems) {
-        Map<String, Activation> interpolatedActivations = getProfileActivations(rawModel, true);
+            Model rawModel,
+            DefaultProfileActivationContext context,
+            boolean failOnInvalidModel,
+            DefaultModelProblemCollector problems) {
+        Map<String, Activation> interpolatedActivations =
+                getProfileActivations(rawModel, true, failOnInvalidModel, problems);
 
         if (interpolatedActivations.isEmpty()) {
             return Collections.emptyMap();
@@ -753,7 +782,8 @@ private void assembleInheritance(
         }
     }
 
-    private Map<String, Activation> getProfileActivations(Model model, boolean clone) {
+    private Map<String, Activation> getProfileActivations(
+            Model model, boolean clone, boolean failOnInvalidModel, ModelProblemCollector problems) {
         Map<String, Activation> activations = new HashMap<>();
         for (Profile profile : model.getProfiles()) {
             Activation activation = profile.getActivation();
@@ -766,7 +796,11 @@ private Map<String, Activation> getProfileActivations(Model model, boolean clone
                 activation = activation.clone();
             }
 
-            activations.put(profile.getId(), activation);
+            if (activations.put(profile.getId(), activation) != null) {
+                problems.add(new ModelProblemCollectorRequest(
+                                failOnInvalidModel ? Severity.FATAL : Severity.WARNING, ModelProblem.Version.BASE)
+                        .setMessage("Duplicate activation for profile " + profile.getId()));
+            }
         }
 
         return activations;
@@ -787,7 +821,8 @@ private void injectProfileActivations(Model model, Map<String, Activation> activ
 
     private Model interpolateModel(Model model, ModelBuildingRequest request, ModelProblemCollector problems) {
         // save profile activations before interpolation, since they are evaluated with limited scope
-        Map<String, Activation> originalActivations = getProfileActivations(model, true);
+        // at this stage we already failed if wanted to
+        Map<String, Activation> originalActivations = getProfileActivations(model, true, false, problems);
 
         Model interpolatedModel =
                 modelInterpolator.interpolateModel(model, model.getProjectDirectory(), request, problems);

maven-model-builder/src/test/java/org/apache/maven/model/building/DefaultModelBuilderTest.java

@@ -18,6 +18,8 @@
  */
 package org.apache.maven.model.building;
 
+import java.io.File;
+
 import org.apache.maven.model.Dependency;
 import org.apache.maven.model.Parent;
 import org.apache.maven.model.Repository;
@@ -27,6 +29,8 @@
 import org.junit.Test;
 
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 /**
  * @author Guillaume Nodet
@@ -87,6 +91,38 @@ public void testCycleInImports() throws Exception {
         builder.build(request);
     }
 
+    @Test
+    public void testBadProfiles() {
+        ModelBuilder builder = new DefaultModelBuilderFactory().newInstance();
+        assertNotNull(builder);
+
+        DefaultModelBuildingRequest request = new DefaultModelBuildingRequest();
+        request.setValidationLevel(ModelBuildingRequest.VALIDATION_LEVEL_MINIMAL);
+        request.setModelSource(new FileModelSource(new File("src/test/resources/poms/building/badprofiles.xml")));
+        request.setModelResolver(new BaseModelResolver());
+
+        try {
+            builder.build(request); // throw, making "pom not available"
+            fail();
+        } catch (ModelBuildingException e) {
+            assertTrue(e.getMessage().contains("Duplicate activation for profile badprofile"));
+        }
+    }
+
+    @Test
+    public void testBadProfilesCheckDisabled() throws Exception {
+        ModelBuilder builder = new DefaultModelBuilderFactory().newInstance();
+        assertNotNull(builder);
+
+        DefaultModelBuildingRequest request = new DefaultModelBuildingRequest();
+        request.getUserProperties().setProperty(DefaultModelBuilder.FAIL_ON_INVALID_MODEL, "false");
+        request.setValidationLevel(ModelBuildingRequest.VALIDATION_LEVEL_MINIMAL);
+        request.setModelSource(new FileModelSource(new File("src/test/resources/poms/building/badprofiles.xml")));
+        request.setModelResolver(new BaseModelResolver());
+
+        builder.build(request); // does not throw, old behaviour (but result may be fully off)
+    }
+
     static class CycleInImportsResolver extends BaseModelResolver {
         @Override
         public ModelSource resolveModel(Dependency dependency) throws UnresolvableModelException {

maven-model-builder/src/test/resources/poms/building/badprofiles.xml

@@ -0,0 +1,54 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements.  See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership.  The ASF licenses this file
+to you 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.
+-->
+
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
+  <modelVersion>4.0.0</modelVersion>
+
+  <groupId>test</groupId>
+  <artifactId>test</artifactId>
+  <version>0.1-SNAPSHOT</version>
+  <packaging>pom</packaging>
+
+  <profiles>
+    <profile>
+      <id>badprofile</id>
+      <activation>
+        <activeByDefault>true</activeByDefault>
+      </activation>
+      <properties>
+        <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
+      </properties>
+    </profile>
+    <profile>
+      <id>badprofile</id>
+      <activation>
+        <file>
+          <exists>simple.xml</exists>
+        </file>
+      </activation>
+      <properties>
+        <profile.file>activated</profile.file>
+      </properties>
+    </profile>
+  </profiles>
+</project>