Skip to content

Commit 7fcd8c5

Browse files
cstamascstamas
authored
[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
1 parent 66266e5 commit 7fcd8c5

File tree

3 files changed

+131
-6
lines changed

3 files changed

+131
-6
lines changed

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

Lines changed: 41 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,30 @@
9797
@Named
9898
@Singleton
9999
public class DefaultModelBuilder implements ModelBuilder {
100+
/**
101+
* Key for "fail on invalid model" property.
102+
* <p>
103+
* Visible for testing.
104+
*/
105+
static final String FAIL_ON_INVALID_MODEL = "maven.modelBuilder.failOnInvalidModel";
106+
107+
/**
108+
* Checks user and system properties (in this order) for value of {@link #FAIL_ON_INVALID_MODEL} property key, if
109+
* set and returns it. If not set, defaults to {@code true}.
110+
* <p>
111+
* This is only meant to provide "escape hatch" for those builds, that are for some reason stuck with invalid models.
112+
*/
113+
private static boolean isFailOnInvalidModel(ModelBuildingRequest request) {
114+
String val = request.getUserProperties().getProperty(FAIL_ON_INVALID_MODEL);
115+
if (val == null) {
116+
val = request.getSystemProperties().getProperty(FAIL_ON_INVALID_MODEL);
117+
}
118+
if (val != null) {
119+
return Boolean.parseBoolean(val);
120+
}
121+
return true;
122+
}
123+
100124
@Inject
101125
private ModelProcessor modelProcessor;
102126

@@ -253,6 +277,7 @@ public ModelBuildingResult build(ModelBuildingRequest request) throws ModelBuild
253277
protected ModelBuildingResult build(ModelBuildingRequest request, Collection<String> importIds)
254278
throws ModelBuildingException {
255279
// phase 1
280+
boolean failOnInvalidModel = isFailOnInvalidModel(request);
256281
DefaultModelBuildingResult result = new DefaultModelBuildingResult();
257282

258283
DefaultModelProblemCollector problems = new DefaultModelProblemCollector(result);
@@ -306,7 +331,7 @@ protected ModelBuildingResult build(ModelBuildingRequest request, Collection<Str
306331
profileActivationContext.setProjectProperties(tmpModel.getProperties());
307332

308333
Map<String, Activation> interpolatedActivations =
309-
getInterpolatedActivations(rawModel, profileActivationContext, problems);
334+
getInterpolatedActivations(rawModel, profileActivationContext, failOnInvalidModel, problems);
310335
injectProfileActivations(tmpModel, interpolatedActivations);
311336

312337
List<Profile> activePomProfiles =
@@ -430,8 +455,12 @@ private interface InterpolateString {
430455
}
431456

432457
private Map<String, Activation> getInterpolatedActivations(
433-
Model rawModel, DefaultProfileActivationContext context, DefaultModelProblemCollector problems) {
434-
Map<String, Activation> interpolatedActivations = getProfileActivations(rawModel, true);
458+
Model rawModel,
459+
DefaultProfileActivationContext context,
460+
boolean failOnInvalidModel,
461+
DefaultModelProblemCollector problems) {
462+
Map<String, Activation> interpolatedActivations =
463+
getProfileActivations(rawModel, true, failOnInvalidModel, problems);
435464

436465
if (interpolatedActivations.isEmpty()) {
437466
return Collections.emptyMap();
@@ -753,7 +782,8 @@ private void assembleInheritance(
753782
}
754783
}
755784

756-
private Map<String, Activation> getProfileActivations(Model model, boolean clone) {
785+
private Map<String, Activation> getProfileActivations(
786+
Model model, boolean clone, boolean failOnInvalidModel, ModelProblemCollector problems) {
757787
Map<String, Activation> activations = new HashMap<>();
758788
for (Profile profile : model.getProfiles()) {
759789
Activation activation = profile.getActivation();
@@ -766,7 +796,11 @@ private Map<String, Activation> getProfileActivations(Model model, boolean clone
766796
activation = activation.clone();
767797
}
768798

769-
activations.put(profile.getId(), activation);
799+
if (activations.put(profile.getId(), activation) != null) {
800+
problems.add(new ModelProblemCollectorRequest(
801+
failOnInvalidModel ? Severity.FATAL : Severity.WARNING, ModelProblem.Version.BASE)
802+
.setMessage("Duplicate activation for profile " + profile.getId()));
803+
}
770804
}
771805

772806
return activations;
@@ -787,7 +821,8 @@ private void injectProfileActivations(Model model, Map<String, Activation> activ
787821

788822
private Model interpolateModel(Model model, ModelBuildingRequest request, ModelProblemCollector problems) {
789823
// save profile activations before interpolation, since they are evaluated with limited scope
790-
Map<String, Activation> originalActivations = getProfileActivations(model, true);
824+
// at this stage we already failed if wanted to
825+
Map<String, Activation> originalActivations = getProfileActivations(model, true, false, problems);
791826

792827
Model interpolatedModel =
793828
modelInterpolator.interpolateModel(model, model.getProjectDirectory(), request, problems);

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

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818
*/
1919
package org.apache.maven.model.building;
2020

21+
import java.io.File;
22+
2123
import org.apache.maven.model.Dependency;
2224
import org.apache.maven.model.Parent;
2325
import org.apache.maven.model.Repository;
@@ -27,6 +29,8 @@
2729
import org.junit.Test;
2830

2931
import static org.junit.Assert.assertNotNull;
32+
import static org.junit.Assert.assertTrue;
33+
import static org.junit.Assert.fail;
3034

3135
/**
3236
* @author Guillaume Nodet
@@ -87,6 +91,38 @@ public void testCycleInImports() throws Exception {
8791
builder.build(request);
8892
}
8993

94+
@Test
95+
public void testBadProfiles() {
96+
ModelBuilder builder = new DefaultModelBuilderFactory().newInstance();
97+
assertNotNull(builder);
98+
99+
DefaultModelBuildingRequest request = new DefaultModelBuildingRequest();
100+
request.setValidationLevel(ModelBuildingRequest.VALIDATION_LEVEL_MINIMAL);
101+
request.setModelSource(new FileModelSource(new File("src/test/resources/poms/building/badprofiles.xml")));
102+
request.setModelResolver(new BaseModelResolver());
103+
104+
try {
105+
builder.build(request); // throw, making "pom not available"
106+
fail();
107+
} catch (ModelBuildingException e) {
108+
assertTrue(e.getMessage().contains("Duplicate activation for profile badprofile"));
109+
}
110+
}
111+
112+
@Test
113+
public void testBadProfilesCheckDisabled() throws Exception {
114+
ModelBuilder builder = new DefaultModelBuilderFactory().newInstance();
115+
assertNotNull(builder);
116+
117+
DefaultModelBuildingRequest request = new DefaultModelBuildingRequest();
118+
request.getUserProperties().setProperty(DefaultModelBuilder.FAIL_ON_INVALID_MODEL, "false");
119+
request.setValidationLevel(ModelBuildingRequest.VALIDATION_LEVEL_MINIMAL);
120+
request.setModelSource(new FileModelSource(new File("src/test/resources/poms/building/badprofiles.xml")));
121+
request.setModelResolver(new BaseModelResolver());
122+
123+
builder.build(request); // does not throw, old behaviour (but result may be fully off)
124+
}
125+
90126
static class CycleInImportsResolver extends BaseModelResolver {
91127
@Override
92128
public ModelSource resolveModel(Dependency dependency) throws UnresolvableModelException {

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

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
3+
<!--
4+
Licensed to the Apache Software Foundation (ASF) under one
5+
or more contributor license agreements. See the NOTICE file
6+
distributed with this work for additional information
7+
regarding copyright ownership. The ASF licenses this file
8+
to you under the Apache License, Version 2.0 (the
9+
"License"); you may not use this file except in compliance
10+
with the License. You may obtain a copy of the License at
11+
12+
http://www.apache.org/licenses/LICENSE-2.0
13+
14+
Unless required by applicable law or agreed to in writing,
15+
software distributed under the License is distributed on an
16+
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
17+
KIND, either express or implied. See the License for the
18+
specific language governing permissions and limitations
19+
under the License.
20+
-->
21+
22+
<project xmlns="http://maven.apache.org/POM/4.0.0"
23+
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
24+
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
25+
<modelVersion>4.0.0</modelVersion>
26+
27+
<groupId>test</groupId>
28+
<artifactId>test</artifactId>
29+
<version>0.1-SNAPSHOT</version>
30+
<packaging>pom</packaging>
31+
32+
<profiles>
33+
<profile>
34+
<id>badprofile</id>
35+
<activation>
36+
<activeByDefault>true</activeByDefault>
37+
</activation>
38+
<properties>
39+
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
40+
</properties>
41+
</profile>
42+
<profile>
43+
<id>badprofile</id>
44+
<activation>
45+
<file>
46+
<exists>simple.xml</exists>
47+
</file>
48+
</activation>
49+
<properties>
50+
<profile.file>activated</profile.file>
51+
</properties>
52+
</profile>
53+
</profiles>
54+
</project>

0 commit comments

Comments
 (0)