From 7fcd8c5d154bbf7163350d4d3fbfd03fbe17cf93 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Thu, 6 Jun 2024 21:21:17 +0200 Subject: [PATCH] [MNG-8141] Model builder should report problems it finds during build (#1556) And not rely that model was validated, which is not true in some cases. Model builder can still easily detect issues with models while building them. Provides "escape hatch" for projects stuck on invalid models in form of user property that can be enabled with `-Dmaven.modelBuilder.failOnInvalidModel=false`, this reverts to _old_ behaviour of maven, and the JavaFX reproducer goes back to error "unable to resolve" errors with uninterpolated `${javafx.platform}` property as classifier. --- https://issues.apache.org/jira/browse/MNG-8141 --- .../model/building/DefaultModelBuilder.java | 47 +++++++++++++--- .../building/DefaultModelBuilderTest.java | 36 +++++++++++++ .../resources/poms/building/badprofiles.xml | 54 +++++++++++++++++++ 3 files changed, 131 insertions(+), 6 deletions(-) create mode 100644 maven-model-builder/src/test/resources/poms/building/badprofiles.xml 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 index 3043a76a0e..04a6a9e622 100644 --- 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,6 +97,30 @@ import static org.apache.maven.model.building.Result.newResult; @Named @Singleton public class DefaultModelBuilder implements ModelBuilder { + /** + * Key for "fail on invalid model" property. + *

+ * 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}. + *

+ * 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 class DefaultModelBuilder implements ModelBuilder { protected ModelBuildingResult build(ModelBuildingRequest request, Collection importIds) throws ModelBuildingException { // phase 1 + boolean failOnInvalidModel = isFailOnInvalidModel(request); DefaultModelBuildingResult result = new DefaultModelBuildingResult(); DefaultModelProblemCollector problems = new DefaultModelProblemCollector(result); @@ -306,7 +331,7 @@ public class DefaultModelBuilder implements ModelBuilder { profileActivationContext.setProjectProperties(tmpModel.getProperties()); Map interpolatedActivations = - getInterpolatedActivations(rawModel, profileActivationContext, problems); + getInterpolatedActivations(rawModel, profileActivationContext, failOnInvalidModel, problems); injectProfileActivations(tmpModel, interpolatedActivations); List activePomProfiles = @@ -430,8 +455,12 @@ public class DefaultModelBuilder implements ModelBuilder { } private Map getInterpolatedActivations( - Model rawModel, DefaultProfileActivationContext context, DefaultModelProblemCollector problems) { - Map interpolatedActivations = getProfileActivations(rawModel, true); + Model rawModel, + DefaultProfileActivationContext context, + boolean failOnInvalidModel, + DefaultModelProblemCollector problems) { + Map interpolatedActivations = + getProfileActivations(rawModel, true, failOnInvalidModel, problems); if (interpolatedActivations.isEmpty()) { return Collections.emptyMap(); @@ -753,7 +782,8 @@ public class DefaultModelBuilder implements ModelBuilder { } } - private Map getProfileActivations(Model model, boolean clone) { + private Map getProfileActivations( + Model model, boolean clone, boolean failOnInvalidModel, ModelProblemCollector problems) { Map activations = new HashMap<>(); for (Profile profile : model.getProfiles()) { Activation activation = profile.getActivation(); @@ -766,7 +796,11 @@ public class DefaultModelBuilder implements ModelBuilder { 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 @@ public class DefaultModelBuilder implements ModelBuilder { private Model interpolateModel(Model model, ModelBuildingRequest request, ModelProblemCollector problems) { // save profile activations before interpolation, since they are evaluated with limited scope - Map originalActivations = getProfileActivations(model, true); + // at this stage we already failed if wanted to + Map originalActivations = getProfileActivations(model, true, false, problems); Model interpolatedModel = modelInterpolator.interpolateModel(model, model.getProjectDirectory(), request, problems); diff --git a/maven-model-builder/src/test/java/org/apache/maven/model/building/DefaultModelBuilderTest.java b/maven-model-builder/src/test/java/org/apache/maven/model/building/DefaultModelBuilderTest.java index 59fb447500..13a8ac0e47 100644 --- a/maven-model-builder/src/test/java/org/apache/maven/model/building/DefaultModelBuilderTest.java +++ b/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.apache.maven.model.resolution.UnresolvableModelException; 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 class DefaultModelBuilderTest { 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 new file mode 100644 index 0000000000..e098a70fb9 --- /dev/null +++ b/maven-model-builder/src/test/resources/poms/building/badprofiles.xml @@ -0,0 +1,54 @@ + + + + + + 4.0.0 + + test + test + 0.1-SNAPSHOT + pom + + + + badprofile + + true + + + UTF-8 + + + + badprofile + + + simple.xml + + + + activated + + + +