Revert "[MNG-8141] Model builder should report problems it finds duri…

…ng build (#1556)" This reverts commit 7fcd8c5.
apache · Jun 7, 2024 · 758e054 · 758e054
1 parent 7fcd8c5
commit 758e054
Show file tree

Hide file tree

Showing 3 changed files with 6 additions and 131 deletions.
diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java b/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java
@@ -97,30 +97,6 @@
 @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;
 
@@ -277,7 +253,6 @@ 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);
@@ -331,7 +306,7 @@ protected ModelBuildingResult build(ModelBuildingRequest request, Collection<Str
             profileActivationContext.setProjectProperties(tmpModel.getProperties());
 
             Map<String, Activation> interpolatedActivations =
-                    getInterpolatedActivations(rawModel, profileActivationContext, failOnInvalidModel, problems);
+                    getInterpolatedActivations(rawModel, profileActivationContext, problems);
             injectProfileActivations(tmpModel, interpolatedActivations);
 
             List<Profile> activePomProfiles =
@@ -455,12 +430,8 @@ private interface InterpolateString {
     }
 
     private Map<String, Activation> getInterpolatedActivations(
-            Model rawModel,
-            DefaultProfileActivationContext context,
-            boolean failOnInvalidModel,
-            DefaultModelProblemCollector problems) {
-        Map<String, Activation> interpolatedActivations =
-                getProfileActivations(rawModel, true, failOnInvalidModel, problems);
+            Model rawModel, DefaultProfileActivationContext context, DefaultModelProblemCollector problems) {
+        Map<String, Activation> interpolatedActivations = getProfileActivations(rawModel, true);
 
         if (interpolatedActivations.isEmpty()) {
             return Collections.emptyMap();
@@ -782,8 +753,7 @@ private void assembleInheritance(
         }
     }
 
-    private Map<String, Activation> getProfileActivations(
-            Model model, boolean clone, boolean failOnInvalidModel, ModelProblemCollector problems) {
+    private Map<String, Activation> getProfileActivations(Model model, boolean clone) {
         Map<String, Activation> activations = new HashMap<>();
         for (Profile profile : model.getProfiles()) {
             Activation activation = profile.getActivation();
@@ -796,11 +766,7 @@ private Map<String, Activation> getProfileActivations(
                 activation = activation.clone();
             }
 
-            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()));
-            }
+            activations.put(profile.getId(), activation);
         }
 
         return activations;
@@ -821,8 +787,7 @@ 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
-        // at this stage we already failed if wanted to
-        Map<String, Activation> originalActivations = getProfileActivations(model, true, false, problems);
+        Map<String, Activation> originalActivations = getProfileActivations(model, true);
 
         Model interpolatedModel =
                 modelInterpolator.interpolateModel(model, model.getProjectDirectory(), request, problems);

diff --git a/...-model-builder/src/test/java/org/apache/maven/model/building/DefaultModelBuilderTest.java b/...-model-builder/src/test/java/org/apache/maven/model/building/DefaultModelBuilderTest.java
@@ -18,8 +18,6 @@
  */
 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;
@@ -29,8 +27,6 @@
 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
@@ -91,38 +87,6 @@ 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 {

diff --git a/maven-model-builder/src/test/resources/poms/building/badprofiles.xml b/maven-model-builder/src/test/resources/poms/building/badprofiles.xml