[MNG-7851] Improve error message when modelVersion is 4.0 (#1219)

Improve DefaultModelValidator.compareModelVersions to be able to compare versions which different numbers of segments, allowing it to compare "4", "4.0", and "4.0.0" to each other for example. Signed-off-by: Craig Andrews <candrews@integralblue.com> # Conflicts: # maven-model-builder/src/test/java/org/apache/maven/model/validation/DefaultModelValidatorTest.java Co-authored-by: Craig Andrews <candrews@integralblue.com>
apache · Aug 29, 2023 · b0170a6 · b0170a6
1 parent 7d66d19
commit b0170a6
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 6 deletions.
diff --git a/...-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java b/...-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java
@@ -1531,16 +1531,14 @@ private static int compareModelVersions(String first, String second) {
         // we use a dedicated comparator because we control our model version scheme.
         String[] firstSegments = StringUtils.split(first, ".");
         String[] secondSegments = StringUtils.split(second, ".");
-        for (int i = 0; i < Math.min(firstSegments.length, secondSegments.length); i++) {
-            int result = Long.valueOf(firstSegments[i]).compareTo(Long.valueOf(secondSegments[i]));
+        for (int i = 0; i < Math.max(firstSegments.length, secondSegments.length); i++) {
+            int result = Long.valueOf(i < firstSegments.length ? firstSegments[i] : "0")
+                    .compareTo(Long.valueOf(i < secondSegments.length ? secondSegments[i] : "0"));
             if (result != 0) {
                 return result;
             }
         }
-        if (firstSegments.length == secondSegments.length) {
-            return 0;
-        }
-        return firstSegments.length > secondSegments.length ? -1 : 1;
+        return 0;
     }
 
     @SuppressWarnings("checkstyle:parameternumber")

diff --git a/...el-builder/src/test/java/org/apache/maven/model/validation/DefaultModelValidatorTest.java b/...el-builder/src/test/java/org/apache/maven/model/validation/DefaultModelValidatorTest.java
@@ -122,6 +122,16 @@ void testBadModelVersion() throws Exception {
         assertTrue(result.getFatals().get(0).contains("modelVersion"));
     }
 
+    @Test
+    void testModelVersionMessage() throws Exception {
+        SimpleProblemCollector result =
+                validateRaw("modelVersion-4_0.xml", ModelBuildingRequest.VALIDATION_LEVEL_STRICT);
+
+        assertViolations(result, 0, 1, 0);
+
+        assertTrue(result.getErrors().get(0).contains("'modelVersion' must be one of"));
+    }
+
     @Test
     void testMissingArtifactId() throws Exception {
         SimpleProblemCollector result = validate("missing-artifactId-pom.xml");

diff --git a/maven-model-builder/src/test/resources/poms/validation/modelVersion-4_0.xml b/maven-model-builder/src/test/resources/poms/validation/modelVersion-4_0.xml
@@ -0,0 +1,25 @@
+<!--
+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>
+  <modelVersion>4.0</modelVersion>
+  <groupId>foo</groupId>
+  <artifactId>bar</artifactId>
+  <version>0.1</version>
+</project>