This is an automated email from the ASF dual-hosted git repository.
gnodet pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/maven.git
The following commit(s) were added to refs/heads/master by this push:
new 8e5e7f1531 Improve DefaultModelProcessor error reporting for
alternative parsers (#11529) (#11532)
8e5e7f1531 is described below
commit 8e5e7f1531dd12d260094261babdb499d57126d1
Author: Guillaume Nodet <[email protected]>
AuthorDate: Tue Dec 9 21:39:03 2025 +0100
Improve DefaultModelProcessor error reporting for alternative parsers
(#11529) (#11532)
When multiple model parsers are registered (e.g., for YAML or TOML POMs)
and all parsers fail to parse a POM file, the error message now provides
detailed information about each parser's failure.
Changes:
- Changed modelParsers from List to Map<String, ModelParser> to preserve
parser names for better error messages
- Added buildDetailedErrorMessage() method that generates a comprehensive
error report including:
- The POM file path
- The number of parsers attempted
- Each parser's error with line/column information when available
- The default XML reader's error
- Updated all call sites to use Map.of() instead of List.of()
This improves debugging when using alternative POM formats by showing
exactly why each parser failed, rather than just the final XML error.
(cherry picked from commit 71261c294828091acc20bc113d9a7a41c4135491)
# Conflicts:
#
impl/maven-impl/src/test/java/org/apache/maven/impl/DefaultPluginXmlFactoryTest.java
---
.../maven/graph/DefaultGraphBuilderTest.java | 2 +-
.../maven/impl/model/DefaultModelProcessor.java | 82 +++++++++--
.../maven/impl/DefaultPluginXmlFactoryTest.java | 4 +-
.../impl/model/DefaultModelProcessorTest.java | 151 +++++++++++++++++++++
.../testing/stubs/RepositorySystemSupplier.java | 2 +-
.../its/mng8220/extension1/DumbModelParser1.java | 2 +-
.../its/mng8220/extension2/DumbModelParser2.java | 2 +-
.../its/mng8220/extension3/DumbModelParser3.java | 2 +-
.../its/mng8220/extension4/DumbModelParser4.java | 2 +-
9 files changed, 229 insertions(+), 20 deletions(-)
diff --git
a/impl/maven-core/src/test/java/org/apache/maven/graph/DefaultGraphBuilderTest.java
b/impl/maven-core/src/test/java/org/apache/maven/graph/DefaultGraphBuilderTest.java
index 07bcbf90d4..72dad0b244 100644
---
a/impl/maven-core/src/test/java/org/apache/maven/graph/DefaultGraphBuilderTest.java
+++
b/impl/maven-core/src/test/java/org/apache/maven/graph/DefaultGraphBuilderTest.java
@@ -105,7 +105,7 @@ class DefaultGraphBuilderTest {
// Not using mocks for these strategies - a mock would just copy the
actual implementation.
- private final ModelProcessor modelProcessor = new
DefaultModelProcessor(null, List.of());
+ private final ModelProcessor modelProcessor = new
DefaultModelProcessor(null, Map.of());
private final PomlessCollectionStrategy pomlessCollectionStrategy = new
PomlessCollectionStrategy(projectBuilder);
private final MultiModuleCollectionStrategy multiModuleCollectionStrategy =
new MultiModuleCollectionStrategy(modelProcessor,
projectsSelector);
diff --git
a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelProcessor.java
b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelProcessor.java
index bcd0a191f8..49ce1c96e6 100644
---
a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelProcessor.java
+++
b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelProcessor.java
@@ -22,8 +22,7 @@
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
-import java.util.ArrayList;
-import java.util.List;
+import java.util.LinkedHashMap;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
@@ -35,6 +34,7 @@
import org.apache.maven.api.model.Model;
import org.apache.maven.api.services.model.ModelProcessor;
import org.apache.maven.api.services.xml.ModelXmlFactory;
+import org.apache.maven.api.services.xml.XmlReaderException;
import org.apache.maven.api.services.xml.XmlReaderRequest;
import org.apache.maven.api.spi.ModelParser;
import org.apache.maven.api.spi.ModelParserException;
@@ -69,10 +69,10 @@
public class DefaultModelProcessor implements ModelProcessor {
private final ModelXmlFactory modelXmlFactory;
- private final List<ModelParser> modelParsers;
+ private final Map<String, ModelParser> modelParsers;
@Inject
- public DefaultModelProcessor(ModelXmlFactory modelXmlFactory, @Nullable
List<ModelParser> modelParsers) {
+ public DefaultModelProcessor(ModelXmlFactory modelXmlFactory, @Nullable
Map<String, ModelParser> modelParsers) {
this.modelXmlFactory = modelXmlFactory;
this.modelParsers = modelParsers;
}
@@ -81,7 +81,7 @@ public DefaultModelProcessor(ModelXmlFactory modelXmlFactory,
@Nullable List<Mod
public Path locateExistingPom(Path projectDirectory) {
// Note that the ModelProcessor#locatePom never returns null
// while the ModelParser#locatePom needs to return an existing path!
- Path pom = modelParsers.stream()
+ Path pom = modelParsers.values().stream()
.map(m -> m.locate(projectDirectory)
.map(org.apache.maven.api.services.Source::getPath)
.orElse(null))
@@ -100,22 +100,27 @@ public Model read(XmlReaderRequest request) throws
IOException {
Path pomFile = request.getPath();
if (pomFile != null) {
Path projectDirectory = pomFile.getParent();
- List<ModelParserException> exceptions = new ArrayList<>();
- for (ModelParser parser : modelParsers) {
+ Map<String, ModelParserException> exceptions = new
LinkedHashMap<>();
+ for (Map.Entry<String, ModelParser> parser :
modelParsers.entrySet()) {
try {
- Optional<Model> model =
- parser.locateAndParse(projectDirectory,
Map.of(ModelParser.STRICT, request.isStrict()));
+ Optional<Model> model = parser.getValue()
+ .locateAndParse(projectDirectory,
Map.of(ModelParser.STRICT, request.isStrict()));
if (model.isPresent()) {
return model.get().withPomFile(pomFile);
}
} catch (ModelParserException e) {
- exceptions.add(e);
+ exceptions.put(parser.getKey(), e);
}
}
try {
return doRead(request);
- } catch (IOException e) {
- exceptions.forEach(e::addSuppressed);
+ } catch (IOException | XmlReaderException e) {
+ if (!exceptions.isEmpty()) {
+ IOException ioException = new
IOException(buildDetailedErrorMessage(pomFile, exceptions, e));
+ exceptions.values().forEach(ioException::addSuppressed);
+ ioException.addSuppressed(e);
+ throw ioException;
+ }
throw e;
}
} else {
@@ -140,4 +145,57 @@ private Path doLocateExistingPom(Path project) {
private Model doRead(XmlReaderRequest request) throws IOException {
return modelXmlFactory.read(request);
}
+
+ private String buildDetailedErrorMessage(
+ Path pomFile, Map<String, ModelParserException> parserExceptions,
Exception defaultReaderException) {
+ StringBuilder message = new StringBuilder();
+ message.append("Unable to parse POM
").append(pomFile).append(System.lineSeparator());
+
+ if (!parserExceptions.isEmpty()) {
+ message.append(" Tried ")
+ .append(parserExceptions.size())
+ .append(" parser")
+ .append(parserExceptions.size() > 1 ? "s" : "")
+ .append(":")
+ .append(System.lineSeparator());
+
+ for (Map.Entry<String, ModelParserException> entry :
parserExceptions.entrySet()) {
+ ModelParserException e = entry.getValue();
+ message.append(" ").append(entry.getKey()).append(")
");
+
+ String parserMessage = e.getMessage();
+ if (parserMessage != null && !parserMessage.isEmpty()) {
+ message.append(parserMessage);
+ } else {
+ message.append(e.getClass().getSimpleName());
+ }
+
+ if (e.getLineNumber() > 0) {
+ message.append(" at line ").append(e.getLineNumber());
+ if (e.getColumnNumber() > 0) {
+ message.append(", column
").append(e.getColumnNumber());
+ }
+ }
+
+ if (e.getCause() != null && e.getCause().getMessage() != null)
{
+ String causeMessage = e.getCause().getMessage();
+ if (parserMessage == null ||
!parserMessage.contains(causeMessage)) {
+ message.append(": ").append(causeMessage);
+ }
+ }
+
+ message.append(System.lineSeparator());
+ }
+ }
+
+ message.append(" default) XML reader also failed: ");
+ String defaultMessage = defaultReaderException.getMessage();
+ if (defaultMessage != null && !defaultMessage.isEmpty()) {
+ message.append(defaultMessage);
+ } else {
+ message.append(defaultReaderException.getClass().getSimpleName());
+ }
+
+ return message.toString();
+ }
}
diff --git
a/impl/maven-impl/src/test/java/org/apache/maven/impl/DefaultPluginXmlFactoryTest.java
b/impl/maven-impl/src/test/java/org/apache/maven/impl/DefaultPluginXmlFactoryTest.java
index bd4d3b7b22..21be40ca31 100644
---
a/impl/maven-impl/src/test/java/org/apache/maven/impl/DefaultPluginXmlFactoryTest.java
+++
b/impl/maven-impl/src/test/java/org/apache/maven/impl/DefaultPluginXmlFactoryTest.java
@@ -27,7 +27,7 @@
import java.io.Writer;
import java.nio.file.Files;
import java.nio.file.Path;
-import java.util.List;
+import java.util.Map;
import com.ctc.wstx.exc.WstxEOFException;
import org.apache.maven.api.plugin.descriptor.PluginDescriptor;
@@ -299,7 +299,7 @@ void readMalformedXmlThrowsXmlReaderException() {
@Test
void locateExistingPomWithFilePathShouldReturnSameFileIfRegularFile()
throws IOException {
Path pomFile = Files.createTempFile(tempDir, "pom", ".xml");
- DefaultModelProcessor processor = new
DefaultModelProcessor(mock(ModelXmlFactory.class), List.of());
+ DefaultModelProcessor processor = new
DefaultModelProcessor(mock(ModelXmlFactory.class), Map.of());
assertEquals(
pomFile, processor.locateExistingPom(pomFile), "Expected
locateExistingPom to return the same file");
}
diff --git
a/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultModelProcessorTest.java
b/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultModelProcessorTest.java
new file mode 100644
index 0000000000..bcef6ba41d
--- /dev/null
+++
b/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultModelProcessorTest.java
@@ -0,0 +1,151 @@
+/*
+ * 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.
+ */
+package org.apache.maven.impl.model;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Map;
+import java.util.Optional;
+
+import org.apache.maven.api.model.Model;
+import org.apache.maven.api.services.xml.ModelXmlFactory;
+import org.apache.maven.api.services.xml.XmlReaderException;
+import org.apache.maven.api.services.xml.XmlReaderRequest;
+import org.apache.maven.api.spi.ModelParser;
+import org.apache.maven.api.spi.ModelParserException;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+/**
+ * Tests for {@link DefaultModelProcessor}.
+ */
+class DefaultModelProcessorTest {
+
+ @TempDir
+ Path tempDir;
+
+ @Test
+ void testDetailedErrorMessageWithMultipleParsers() throws IOException {
+ // Create a test POM file
+ Path pomFile = tempDir.resolve("pom.yaml");
+ Files.writeString(pomFile, "invalid: yaml: content:");
+
+ // Create mock parsers that will fail
+ ModelParser yamlParser = mock(ModelParser.class);
+ when(yamlParser.locateAndParse(any(), any()))
+ .thenThrow(new ModelParserException(
+ "YAML parsing failed", 5, 10, new
RuntimeException("Invalid YAML syntax")));
+
+ ModelParser tomlParser = mock(ModelParser.class);
+ when(tomlParser.locateAndParse(any(), any()))
+ .thenThrow(new ModelParserException("TOML parsing failed", 3,
7, null));
+
+ // Create mock XML factory that will also fail
+ ModelXmlFactory xmlFactory = mock(ModelXmlFactory.class);
+ when(xmlFactory.read(any(XmlReaderRequest.class)))
+ .thenThrow(new XmlReaderException("XML parsing failed", null,
null));
+
+ // Create processor with the mock parsers
+ DefaultModelProcessor processor =
+ new DefaultModelProcessor(xmlFactory, Map.of("yaml",
yamlParser, "toml", tomlParser));
+
+ // Create request
+ XmlReaderRequest request =
+ XmlReaderRequest.builder().path(pomFile).strict(true).build();
+
+ // Execute and verify
+ IOException exception = assertThrows(IOException.class, () ->
processor.read(request));
+
+ String message = exception.getMessage();
+
+ // Verify the message contains information about all parsers
+ assertTrue(message.contains("Unable to parse POM"), "Message should
mention unable to parse POM");
+ assertTrue(message.contains(pomFile.toString()), "Message should
contain the POM file path");
+ assertTrue(message.contains("Tried 2 parsers"), "Message should
mention 2 parsers were tried");
+ assertTrue(message.contains("YAML parsing failed"), "Message should
contain YAML parser error");
+ assertTrue(message.contains("at line 5, column 10"), "Message should
contain YAML line/column info");
+ assertTrue(message.contains("Invalid YAML syntax"), "Message should
contain YAML cause message");
+ assertTrue(message.contains("TOML parsing failed"), "Message should
contain TOML parser error");
+ assertTrue(message.contains("at line 3, column 7"), "Message should
contain TOML line/column info");
+ assertTrue(message.contains("default) XML reader also failed"),
"Message should mention XML reader failure");
+ assertTrue(message.contains("XML parsing failed"), "Message should
contain XML error message");
+
+ // Verify suppressed exceptions are still attached
+ assertEquals(3, exception.getSuppressed().length, "Should have 3
suppressed exceptions");
+ }
+
+ @Test
+ void testDetailedErrorMessageWithSingleParser() throws IOException {
+ Path pomFile = tempDir.resolve("pom.json");
+ Files.writeString(pomFile, "{invalid json}");
+
+ ModelParser jsonParser = mock(ModelParser.class);
+ when(jsonParser.locateAndParse(any(), any())).thenThrow(new
ModelParserException("JSON parsing failed"));
+
+ ModelXmlFactory xmlFactory = mock(ModelXmlFactory.class);
+ when(xmlFactory.read(any(XmlReaderRequest.class)))
+ .thenThrow(new XmlReaderException("Not valid XML", null,
null));
+
+ DefaultModelProcessor processor = new
DefaultModelProcessor(xmlFactory, Map.of("json", jsonParser));
+
+ XmlReaderRequest request =
+ XmlReaderRequest.builder().path(pomFile).strict(true).build();
+
+ IOException exception = assertThrows(IOException.class, () ->
processor.read(request));
+
+ String message = exception.getMessage();
+ assertTrue(message.contains("Tried 1 parser:"), "Message should
mention 1 parser (singular)");
+ assertTrue(message.contains("JSON parsing failed"), "Message should
contain JSON parser error");
+ assertTrue(message.contains("Not valid XML"), "Message should contain
XML error");
+ }
+
+ @Test
+ void testSuccessfulParsingDoesNotThrowException() throws Exception {
+ Path pomFile = tempDir.resolve("pom.yaml");
+ Files.writeString(pomFile, "valid: yaml");
+
+ Model mockModel = mock(Model.class);
+ when(mockModel.withPomFile(any())).thenReturn(mockModel);
+
+ ModelParser yamlParser = mock(ModelParser.class);
+ when(yamlParser.locateAndParse(any(),
any())).thenReturn(Optional.of(mockModel));
+
+ ModelXmlFactory xmlFactory = mock(ModelXmlFactory.class);
+
+ DefaultModelProcessor processor = new
DefaultModelProcessor(xmlFactory, Map.of("yaml", yamlParser));
+
+ XmlReaderRequest request =
+ XmlReaderRequest.builder().path(pomFile).strict(true).build();
+
+ Model result = processor.read(request);
+ assertNotNull(result);
+ verify(xmlFactory, never()).read(any(XmlReaderRequest.class));
+ }
+}
diff --git
a/impl/maven-testing/src/main/java/org/apache/maven/api/plugin/testing/stubs/RepositorySystemSupplier.java
b/impl/maven-testing/src/main/java/org/apache/maven/api/plugin/testing/stubs/RepositorySystemSupplier.java
index c31d266d18..e55785de4e 100644
---
a/impl/maven-testing/src/main/java/org/apache/maven/api/plugin/testing/stubs/RepositorySystemSupplier.java
+++
b/impl/maven-testing/src/main/java/org/apache/maven/api/plugin/testing/stubs/RepositorySystemSupplier.java
@@ -1085,7 +1085,7 @@ public final ModelBuilder getModelBuilder() {
protected ModelBuilder createModelBuilder() {
// from maven-model-builder
- DefaultModelProcessor modelProcessor = new DefaultModelProcessor(new
DefaultModelXmlFactory(), List.of());
+ DefaultModelProcessor modelProcessor = new DefaultModelProcessor(new
DefaultModelXmlFactory(), Map.of());
return new DefaultModelBuilder(
modelProcessor,
new DefaultModelValidator(),
diff --git
a/its/core-it-suite/src/test/resources/mng-8220-extension-with-di/extensions/extension1/src/main/java/org/apache/maven/its/mng8220/extension1/DumbModelParser1.java
b/its/core-it-suite/src/test/resources/mng-8220-extension-with-di/extensions/extension1/src/main/java/org/apache/maven/its/mng8220/extension1/DumbModelParser1.java
index 9c32330980..8ff8319aa9 100644
---
a/its/core-it-suite/src/test/resources/mng-8220-extension-with-di/extensions/extension1/src/main/java/org/apache/maven/its/mng8220/extension1/DumbModelParser1.java
+++
b/its/core-it-suite/src/test/resources/mng-8220-extension-with-di/extensions/extension1/src/main/java/org/apache/maven/its/mng8220/extension1/DumbModelParser1.java
@@ -33,7 +33,7 @@
import org.slf4j.LoggerFactory;
@Singleton
-@Named
+@Named("dumb1")
final class DumbModelParser1 implements ModelParser {
private final Logger logger = LoggerFactory.getLogger(getClass());
diff --git
a/its/core-it-suite/src/test/resources/mng-8220-extension-with-di/extensions/extension2/src/main/java/org/apache/maven/its/mng8220/extension2/DumbModelParser2.java
b/its/core-it-suite/src/test/resources/mng-8220-extension-with-di/extensions/extension2/src/main/java/org/apache/maven/its/mng8220/extension2/DumbModelParser2.java
index 4148be2da3..b8872c2b0c 100644
---
a/its/core-it-suite/src/test/resources/mng-8220-extension-with-di/extensions/extension2/src/main/java/org/apache/maven/its/mng8220/extension2/DumbModelParser2.java
+++
b/its/core-it-suite/src/test/resources/mng-8220-extension-with-di/extensions/extension2/src/main/java/org/apache/maven/its/mng8220/extension2/DumbModelParser2.java
@@ -31,7 +31,7 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-@Named("dumb")
+@Named("dumb2")
final class DumbModelParser2 implements ModelParser {
private final Logger logger = LoggerFactory.getLogger(getClass());
diff --git
a/its/core-it-suite/src/test/resources/mng-8220-extension-with-di/extensions/extension3/src/main/java/org/apache/maven/its/mng8220/extension3/DumbModelParser3.java
b/its/core-it-suite/src/test/resources/mng-8220-extension-with-di/extensions/extension3/src/main/java/org/apache/maven/its/mng8220/extension3/DumbModelParser3.java
index 4d33f4e1a7..df35c55686 100644
---
a/its/core-it-suite/src/test/resources/mng-8220-extension-with-di/extensions/extension3/src/main/java/org/apache/maven/its/mng8220/extension3/DumbModelParser3.java
+++
b/its/core-it-suite/src/test/resources/mng-8220-extension-with-di/extensions/extension3/src/main/java/org/apache/maven/its/mng8220/extension3/DumbModelParser3.java
@@ -32,7 +32,7 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-@Named("dumb")
+@Named("dumb3")
final class DumbModelParser3 implements ModelParser {
private final Logger logger = LoggerFactory.getLogger(getClass());
diff --git
a/its/core-it-suite/src/test/resources/mng-8220-extension-with-di/extensions/extension4/src/main/java/org/apache/maven/its/mng8220/extension4/DumbModelParser4.java
b/its/core-it-suite/src/test/resources/mng-8220-extension-with-di/extensions/extension4/src/main/java/org/apache/maven/its/mng8220/extension4/DumbModelParser4.java
index a6c4951f5f..a0471cec3b 100644
---
a/its/core-it-suite/src/test/resources/mng-8220-extension-with-di/extensions/extension4/src/main/java/org/apache/maven/its/mng8220/extension4/DumbModelParser4.java
+++
b/its/core-it-suite/src/test/resources/mng-8220-extension-with-di/extensions/extension4/src/main/java/org/apache/maven/its/mng8220/extension4/DumbModelParser4.java
@@ -33,7 +33,7 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-@Named
+@Named("dumb4")
@Singleton
final class DumbModelParser4 implements ModelParser {
