Skip to content

Commit

Permalink
[MNG-8513] Move xml combination mode validation to validator (#2069)
Browse files Browse the repository at this point in the history
As Central has POMs using wrong values, causing even dependency collation to fail. For now plugin/pluginMtg is handled, but it seems extensions, ReportPlugins and ReportSets are not even validated...

---

https://issues.apache.org/jira/browse/MNG-8513
  • Loading branch information
cstamas authored Jan 29, 2025
1 parent 89ae320 commit 7fac8fc
Showing 6 changed files with 159 additions and 15 deletions.
Original file line number Diff line number Diff line change
@@ -195,6 +195,6 @@ public interface StringFormatter {

@Override
public String toString() {
return String.format("%s @ %d:%d", source.getLocation(), lineNumber, columnNumber);
return String.format("%s @ %d:%d", source != null ? source.getLocation() : "n/a", lineNumber, columnNumber);
}
}
Original file line number Diff line number Diff line change
@@ -70,6 +70,7 @@
import org.apache.maven.api.services.ModelProblem.Version;
import org.apache.maven.api.services.ModelProblemCollector;
import org.apache.maven.api.services.model.ModelValidator;
import org.apache.maven.api.xml.XmlNode;
import org.apache.maven.model.v4.MavenModelVersion;
import org.apache.maven.model.v4.MavenTransformer;

@@ -752,6 +753,14 @@ private void validate20RawPlugins(

Set<String> executionIds = new HashSet<>();

if (validationLevel >= ModelValidator.VALIDATION_LEVEL_MAVEN_4_0 && plugin.getConfiguration() != null) {
validateXmlNodeRecursively(
problems,
prefix + prefix2 + "[" + plugin.getKey() + "].configuration",
plugin,
plugin.getConfiguration());
}

for (PluginExecution exec : plugin.getExecutions()) {
if (!executionIds.add(exec.getId())) {
addViolation(
@@ -763,10 +772,67 @@ private void validate20RawPlugins(
"must be unique but found duplicate execution with id " + exec.getId(),
exec);
}
if (validationLevel >= ModelValidator.VALIDATION_LEVEL_MAVEN_4_0 && exec.getConfiguration() != null) {
validateXmlNodeRecursively(
problems,
prefix + prefix2 + "[" + plugin.getKey() + "].executions.execution." + exec.getId(),
exec,
exec.getConfiguration());
}
}
}
}

private void validateXmlNodeRecursively(
ModelProblemCollector problems, String fieldPathPrefix, InputLocationTracker tracker, XmlNode xmlNode) {
validateXmlNode(problems, fieldPathPrefix, tracker, xmlNode);
for (XmlNode child : xmlNode.getChildren()) {
validateXmlNodeRecursively(problems, fieldPathPrefix + "." + xmlNode.getName(), tracker, child);
}
}

private void validateXmlNode(
ModelProblemCollector problems, String fieldPathPrefix, InputLocationTracker tracker, XmlNode xmlNode) {
String childrenCombinationModeAttribute = xmlNode.getAttributes()
.getOrDefault(XmlNode.CHILDREN_COMBINATION_MODE_ATTRIBUTE, XmlNode.DEFAULT_CHILDREN_COMBINATION_MODE);
if (!(XmlNode.CHILDREN_COMBINATION_APPEND.equals(childrenCombinationModeAttribute)
|| XmlNode.CHILDREN_COMBINATION_MERGE.equals(childrenCombinationModeAttribute))) {
addViolation(
problems,
Severity.ERROR,
Version.V40,
fieldPathPrefix + "." + xmlNode.getName(),
xmlNode.getInputLocation() != null
? xmlNode.getInputLocation().toString()
: null,
"Unsupported value '" + childrenCombinationModeAttribute + "' for "
+ XmlNode.CHILDREN_COMBINATION_MODE_ATTRIBUTE + " attribute. " + "Valid values are: "
+ XmlNode.CHILDREN_COMBINATION_APPEND + ", and " + XmlNode.CHILDREN_COMBINATION_MERGE
+ " (default is: " + XmlNode.DEFAULT_SELF_COMBINATION_MODE + ")",
tracker);
}
String selfCombinationModeAttribute = xmlNode.getAttributes()
.getOrDefault(XmlNode.SELF_COMBINATION_MODE_ATTRIBUTE, XmlNode.DEFAULT_SELF_COMBINATION_MODE);
if (!(XmlNode.SELF_COMBINATION_OVERRIDE.equals(selfCombinationModeAttribute)
|| XmlNode.SELF_COMBINATION_MERGE.equals(selfCombinationModeAttribute)
|| XmlNode.SELF_COMBINATION_REMOVE.equals(selfCombinationModeAttribute))) {
addViolation(
problems,
Severity.ERROR,
Version.V40,
fieldPathPrefix + "." + xmlNode.getName(),
xmlNode.getInputLocation() != null
? xmlNode.getInputLocation().toString()
: null,
"Unsupported value '" + selfCombinationModeAttribute + "' for "
+ XmlNode.SELF_COMBINATION_MODE_ATTRIBUTE + " attribute. " + "Valid values are: "
+ XmlNode.SELF_COMBINATION_OVERRIDE + ", " + XmlNode.SELF_COMBINATION_MERGE + ", and "
+ XmlNode.SELF_COMBINATION_REMOVE
+ " (default is: " + XmlNode.DEFAULT_SELF_COMBINATION_MODE + ")",
tracker);
}
}

@Override
@SuppressWarnings("checkstyle:MethodLength")
public void validateEffectiveModel(Model m, int validationLevel, ModelProblemCollector problems) {
Original file line number Diff line number Diff line change
@@ -861,4 +861,16 @@ void profileActivationPropertyWithProjectExpression() throws Exception {
+ "${project.version} expressions are not supported during profile activation.",
result.getWarnings().get(1));
}

@Test
void selfCombineOk() throws Exception {
SimpleProblemCollector result = validateFile("raw-model/self-combine-ok.xml");
assertViolations(result, 0, 0, 0);
}

@Test
void selfCombineBad() throws Exception {
SimpleProblemCollector result = validateFile("raw-model/self-combine-bad.xml");
assertViolations(result, 0, 1, 0);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
<!--
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>com.example.group</groupId>
<artifactId>self-combine-bad</artifactId>
<version>1.0</version>

<build>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
<version>3.13.0</version>
<configuration combine.self="append">
<release>bar</release>
</configuration>
</plugin>
</plugins>
</build>
</project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
<!--
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>com.example.group</groupId>
<artifactId>self-combine-ok</artifactId>
<version>1.0</version>

<build>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
<version>3.13.0</version>
<configuration combine.self="override">
<release>bar</release>
</configuration>
</plugin>
</plugins>
</build>
</project>
Original file line number Diff line number Diff line change
@@ -374,32 +374,20 @@ public static XmlNode merge(XmlNode dominant, XmlNode recessive, Boolean childMe

private static String getChildCombinationMode(Map<String, String> attrs) {
String attribute = attrs.get(CHILDREN_COMBINATION_MODE_ATTRIBUTE);
if (attribute == null) {
return DEFAULT_CHILDREN_COMBINATION_MODE;
}
if (CHILDREN_COMBINATION_APPEND.equals(attribute) || CHILDREN_COMBINATION_MERGE.equals(attribute)) {
return attribute;
}
throw new IllegalArgumentException("Unsupported value '" + attribute + "' for "
+ CHILDREN_COMBINATION_MODE_ATTRIBUTE + " attribute. " + "Valid values are: "
+ CHILDREN_COMBINATION_APPEND + ", and " + CHILDREN_COMBINATION_MERGE
+ " (default is: " + DEFAULT_SELF_COMBINATION_MODE + ")");
return DEFAULT_CHILDREN_COMBINATION_MODE;
}

private static String getSelfCombinationMode(XmlNode node) {
String attribute = node.getAttribute(SELF_COMBINATION_MODE_ATTRIBUTE);
if (attribute == null) {
return DEFAULT_SELF_COMBINATION_MODE;
}
if (SELF_COMBINATION_OVERRIDE.equals(attribute)
|| SELF_COMBINATION_MERGE.equals(attribute)
|| SELF_COMBINATION_REMOVE.equals(attribute)) {
return attribute;
}
throw new IllegalArgumentException("Unsupported value '" + attribute + "' for "
+ SELF_COMBINATION_MODE_ATTRIBUTE + " attribute. " + "Valid values are: "
+ SELF_COMBINATION_OVERRIDE + ", " + SELF_COMBINATION_MERGE + ", and " + SELF_COMBINATION_REMOVE
+ " (default is: " + DEFAULT_SELF_COMBINATION_MODE + ")");
return DEFAULT_SELF_COMBINATION_MODE;
}

/**

0 comments on commit 7fac8fc

Please sign in to comment.