diff --git a/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/ModelProcessor.java b/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/ModelProcessor.java index 091e8f45fd67..7cc2d6e81d5e 100644 --- a/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/ModelProcessor.java +++ b/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/ModelProcessor.java @@ -20,12 +20,15 @@ import java.io.IOException; import java.nio.file.Path; +import java.util.Map; import org.apache.maven.api.annotations.Nonnull; import org.apache.maven.api.annotations.Nullable; import org.apache.maven.api.model.Model; +import org.apache.maven.api.services.Source; import org.apache.maven.api.services.xml.XmlReaderException; import org.apache.maven.api.services.xml.XmlReaderRequest; +import org.apache.maven.api.spi.ModelParserException; /** * Locates and reads POM files from the filesystem. @@ -34,6 +37,15 @@ */ public interface ModelProcessor { + /** + * Option key for passing an {@link XmlReaderRequest.Transformer} through the options map. + * When present, the processor should use this transformer for XML parsing instead + * of any default transformer it would otherwise apply. + * + * @since 4.0.0 + */ + String TRANSFORMER = "transformer"; + /** * Returns the file containing the pom to be parsed or null if a pom can not be found * at the given file or in the given directory. @@ -42,13 +54,26 @@ public interface ModelProcessor { Path locateExistingPom(@Nonnull Path project); /** - * Reads the model from the specified byte stream. The stream will be automatically closed before the method - * returns. + * Reads the model from the specified source. + * + * @param source The source to deserialize the model from, must not be {@code null}. + * @param options Possible reading options, may be {@code null}. + * @return The deserialized model, never {@code null}. + * @throws ModelParserException If the model could not be deserialized. + * @since 4.0.0 + */ + @Nonnull + Model read(@Nonnull Source source, @Nullable Map options) throws ModelParserException; + + /** + * Reads the model from the specified reader request. * * @param request The reader request to deserialize the model, must not be {@code null}. * @return The deserialized model, never {@code null}. * @throws IOException If the model could not be deserialized. * @throws XmlReaderException If the input format could not be parsed. + * @deprecated Use {@link #read(Source, Map)} instead. */ + @Deprecated(since = "4.0.0") Model read(XmlReaderRequest request) throws IOException, XmlReaderException; } diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultSuperPomProvider.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultSuperPomProvider.java index 1ca1382d98eb..765b2786b3f9 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultSuperPomProvider.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultSuperPomProvider.java @@ -18,8 +18,6 @@ */ package org.apache.maven.impl; -import java.io.IOException; -import java.io.InputStream; import java.net.URL; import java.util.Map; import java.util.Objects; @@ -29,9 +27,12 @@ import org.apache.maven.api.di.Named; import org.apache.maven.api.di.Singleton; import org.apache.maven.api.model.Model; +import org.apache.maven.api.services.ModelSource; import org.apache.maven.api.services.SuperPomProvider; import org.apache.maven.api.services.model.ModelProcessor; -import org.apache.maven.api.services.xml.XmlReaderRequest; +import org.apache.maven.api.spi.ModelParser; +import org.apache.maven.api.spi.ModelParserException; +import org.apache.maven.impl.model.DefaultModelProcessor; @Named @Singleton @@ -61,16 +62,12 @@ private Model readModel(String version, String v) { throw new IllegalStateException("The super POM " + resource + " was not found" + ", please verify the integrity of your Maven installation"); } - try (InputStream is = url.openStream()) { - String modelId = - "org.apache.maven:maven-api-impl:" + getImplementationVersion(getClass()) + ":super-pom-" + version; - return modelProcessor.read(XmlReaderRequest.builder() - .modelId(modelId) - .location(url.toExternalForm()) - .inputStream(is) - .strict(false) - .build()); - } catch (IOException e) { + String modelId = + "org.apache.maven:maven-api-impl:" + getImplementationVersion(getClass()) + ":super-pom-" + version; + ModelSource source = DefaultModelProcessor.urlModelSource(url, modelId); + try { + return modelProcessor.read(source, Map.of(ModelParser.STRICT, false)); + } catch (ModelParserException e) { throw new IllegalStateException( "The super POM " + resource + " is damaged" + ", please verify the integrity of your Maven installation", diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelBuilder.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelBuilder.java index ae2b6a320753..815ca21bbbf6 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelBuilder.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelBuilder.java @@ -20,13 +20,11 @@ import java.io.File; import java.io.IOException; -import java.io.InputStream; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; import java.util.ArrayDeque; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collection; import java.util.HashMap; import java.util.HashSet; @@ -114,7 +112,7 @@ import org.apache.maven.api.services.model.ProfileSelector; import org.apache.maven.api.services.model.RootLocator; 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.apache.maven.api.spi.ModelTransformer; import org.apache.maven.impl.DefaultRemoteRepository; @@ -1843,31 +1841,27 @@ Model doReadFileModel(Set activeModelReads) throws ModelBuilderException { rootDirectory = rootDirectory.getParent(); } } - try (InputStream is = modelSource.openStream()) { - model = modelProcessor.read(XmlReaderRequest.builder() - .strict(strict) - .location(modelSource.getLocation()) - .modelId(modelSource.getModelId()) - .path(modelSource.getPath()) - .rootDirectory(rootDirectory) - .inputStream(is) - .transformer(new InterningTransformer(session)) - .build()); - } catch (XmlReaderException e) { + try { + model = modelProcessor.read( + modelSource, + Map.of( + ModelParser.STRICT, + strict, + ModelProcessor.TRANSFORMER, + new ModelInterningTransformer(session))); + } catch (ModelParserException e) { if (!strict) { throw e; } - try (InputStream is = modelSource.openStream()) { - model = modelProcessor.read(XmlReaderRequest.builder() - .strict(false) - .location(modelSource.getLocation()) - .modelId(modelSource.getModelId()) - .path(modelSource.getPath()) - .rootDirectory(rootDirectory) - .inputStream(is) - .transformer(new InterningTransformer(session)) - .build()); - } catch (XmlReaderException ne) { + try { + model = modelProcessor.read( + modelSource, + Map.of( + ModelParser.STRICT, + false, + ModelProcessor.TRANSFORMER, + new ModelInterningTransformer(session))); + } catch (ModelParserException ne) { // still unreadable even in non-strict mode, rethrow original error throw e; } @@ -1878,24 +1872,29 @@ Model doReadFileModel(Set activeModelReads) throws ModelBuilderException { "Malformed POM " + modelSource.getLocation() + ": " + e.getMessage(), e); } - } catch (XmlReaderException e) { - add( - Severity.FATAL, - Version.BASE, - "Non-parseable POM " + modelSource.getLocation() + ": " + e.getMessage(), - e); - throw newModelBuilderException(); - } catch (IOException e) { - String msg = e.getMessage(); - if (msg == null || msg.isEmpty()) { - // NOTE: There's java.nio.charset.MalformedInputException and sun.io.MalformedInputException - if (e.getClass().getName().endsWith("MalformedInputException")) { - msg = "Some input bytes do not match the file encoding."; - } else { - msg = e.getClass().getSimpleName(); + } catch (ModelParserException e) { + if (e.getCause() instanceof IOException ioe) { + String msg = ioe.getMessage(); + if (msg == null || msg.isEmpty()) { + // NOTE: There's java.nio.charset.MalformedInputException and sun.io.MalformedInputException + if (ioe.getClass().getName().endsWith("MalformedInputException")) { + msg = "Some input bytes do not match the file encoding."; + } else { + msg = ioe.getClass().getSimpleName(); + } } + add( + Severity.FATAL, + Version.BASE, + "Non-readable POM " + modelSource.getLocation() + ": " + msg, + ioe); + } else { + add( + Severity.FATAL, + Version.BASE, + "Non-parseable POM " + modelSource.getLocation() + ": " + e.getMessage(), + e); } - add(Severity.FATAL, Version.BASE, "Non-readable POM " + modelSource.getLocation() + ": " + msg, e); throw newModelBuilderException(); } @@ -3341,97 +3340,6 @@ public R getRequest() { } } - static class InterningTransformer implements XmlReaderRequest.Transformer { - static final Set DEFAULT_CONTEXTS = Set.of( - // Core Maven coordinates - "groupId", - "artifactId", - "version", - "namespaceUri", - "packaging", - - // Dependency-related fields - "scope", - "type", - "classifier", - - // Build and plugin-related fields - "phase", - "goal", - "execution", - - // Repository-related fields - "layout", - "policy", - "checksumPolicy", - "updatePolicy", - - // Common metadata fields - "modelVersion", - "name", - "url", - "system", - "distribution", - "status", - - // SCM fields - "connection", - "developerConnection", - "tag", - - // Common enum-like values that appear frequently - "id", - "inherited", - "optional"); - - private final Set contexts; - - /** - * Creates an InterningTransformer with default contexts. - */ - InterningTransformer() { - this.contexts = DEFAULT_CONTEXTS; - } - - /** - * Creates an InterningTransformer with contexts from session properties. - * - * @param session the Maven session to read properties from - */ - InterningTransformer(Session session) { - this.contexts = parseContextsFromSession(session); - } - - private Set parseContextsFromSession(Session session) { - String contextsProperty = session.getUserProperties().get(Constants.MAVEN_MODEL_BUILDER_INTERNS); - if (contextsProperty == null) { - contextsProperty = session.getSystemProperties().get(Constants.MAVEN_MODEL_BUILDER_INTERNS); - } - - if (contextsProperty == null || contextsProperty.trim().isEmpty()) { - return DEFAULT_CONTEXTS; - } - - return Arrays.stream(contextsProperty.split(",")) - .map(String::trim) - .filter(s -> !s.isEmpty()) - .collect(Collectors.toSet()); - } - - @Override - public String transform(String input, String context) { - return input != null && contexts.contains(context) ? input.intern() : input; - } - - /** - * Get the contexts that will be interned by this transformer. - * Used for testing purposes. - */ - Set getContexts() { - return contexts; - } - } - /** * Clears REQUEST_SCOPED cache entries for a specific request. *

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 7784211b4a88..e8e0589c3e7b 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 @@ -18,7 +18,11 @@ */ package org.apache.maven.impl.model; +import java.io.ByteArrayInputStream; import java.io.IOException; +import java.io.InputStream; +import java.net.URL; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; @@ -32,6 +36,8 @@ import org.apache.maven.api.di.Named; import org.apache.maven.api.di.Singleton; import org.apache.maven.api.model.Model; +import org.apache.maven.api.services.ModelSource; +import org.apache.maven.api.services.Source; import org.apache.maven.api.services.model.ModelProcessor; import org.apache.maven.api.services.xml.ModelXmlFactory; import org.apache.maven.api.services.xml.XmlReaderException; @@ -95,17 +101,16 @@ public Path locateExistingPom(Path projectDirectory) { } @Override - public Model read(XmlReaderRequest request) throws IOException { - Objects.requireNonNull(request, "source cannot be null"); - Path pomFile = request.getPath(); + public Model read(Source source, Map options) throws ModelParserException { + Objects.requireNonNull(source, "source cannot be null"); + Path pomFile = source.getPath(); if (pomFile != null) { // a parser may locate the project directory itself rather than a file within it Path projectDirectory = Files.isDirectory(pomFile) ? pomFile : pomFile.getParent(); Map exceptions = new LinkedHashMap<>(); for (Map.Entry parser : modelParsers.entrySet()) { try { - Optional model = parser.getValue() - .locateAndParse(projectDirectory, Map.of(ModelParser.STRICT, request.isStrict())); + Optional model = parser.getValue().locateAndParse(projectDirectory, options); if (model.isPresent()) { return model.get().withPomFile(pomFile); } @@ -114,21 +119,166 @@ public Model read(XmlReaderRequest request) throws IOException { } } try { - return doRead(request); - } catch (IOException | XmlReaderException e) { + return doRead(source, options); + } catch (ModelParserException e) { if (!exceptions.isEmpty()) { - IOException ioException = new IOException(buildDetailedErrorMessage(pomFile, exceptions, e)); - exceptions.values().forEach(ioException::addSuppressed); - ioException.addSuppressed(e); - throw ioException; + ModelParserException mpe = + new ModelParserException(buildDetailedErrorMessage(pomFile, exceptions, e), e); + exceptions.values().forEach(mpe::addSuppressed); + mpe.addSuppressed(e); + throw mpe; } throw e; } } else { - return doRead(request); + return doRead(source, options); } } + /** + * Implements the deprecated bridge for backward compatibility. + * Delegates to {@link #read(Source, Map)}, then re-throws {@link XmlReaderException} + * or {@link IOException} from the cause when possible. + * + * @deprecated Use {@link #read(Source, Map)} instead. + */ + @Deprecated(since = "4.0.0") + @Override + public Model read(XmlReaderRequest request) throws IOException, XmlReaderException { + Objects.requireNonNull(request, "request cannot be null"); + Source source = toSource(request); + try { + return read( + source, + request.getTransformer() != null + ? Map.of( + ModelParser.STRICT, + request.isStrict(), + ModelProcessor.TRANSFORMER, + request.getTransformer()) + : Map.of(ModelParser.STRICT, request.isStrict())); + } catch (ModelParserException e) { + // Unwrap one level: doRead wraps XmlReaderException/IOException as the direct cause + if (e.getCause() instanceof XmlReaderException xre) { + throw xre; + } + if (e.getCause() instanceof IOException ioe) { + throw ioe; + } + throw new XmlReaderException(e.getMessage(), null, e); + } + } + + public static ModelSource urlModelSource(URL url, String modelId) { + return new UrlModelSource(url, modelId); + } + + record UrlModelSource(URL url, String modelId) implements ModelSource { + UrlModelSource { + Objects.requireNonNull(url, "url cannot be null"); + } + + @Override + public Path getPath() { + return null; + } + + @Override + public InputStream openStream() throws IOException { + return url.openStream(); + } + + @Override + public String getLocation() { + return url.toExternalForm(); + } + + @Override + public Source resolve(String relative) { + return null; + } + + @Override + public ModelSource resolve(ModelLocator modelLocator, String relative) { + return null; + } + + @Override + public String getModelId() { + return modelId; + } + } + + private static Source toSource(XmlReaderRequest request) { + if (request.getInputStream() == null + && request.getPath() == null + && request.getReader() == null + && request.getURL() != null) { + return urlModelSource(request.getURL(), request.getModelId()); + } + // Always return a ModelSource so modelId is preserved. + return new ModelSource() { + @Override + public Path getPath() { + return request.getPath(); + } + + @Override + public InputStream openStream() throws IOException { + if (request.getInputStream() != null) { + return request.getInputStream(); + } else if (request.getURL() != null) { + return request.getURL().openStream(); + } else if (request.getPath() != null) { + return Files.newInputStream(request.getPath()); + } else if (request.getReader() != null) { + StringBuilder sb = new StringBuilder(); + char[] buf = new char[4096]; + int n; + while ((n = request.getReader().read(buf)) != -1) { + sb.append(buf, 0, n); + } + return new ByteArrayInputStream(sb.toString().getBytes(StandardCharsets.UTF_8)); + } + throw new IOException("No input stream available in request"); + } + + @Override + public String getLocation() { + if (request.getLocation() != null) { + return request.getLocation(); + } + if (request.getPath() != null) { + return request.getPath().toString(); + } + if (request.getURL() != null) { + return request.getURL().toExternalForm(); + } + return ""; + } + + @Override + public Source resolve(String relative) { + if (request.getPath() != null) { + return toSource(XmlReaderRequest.builder() + .path(request.getPath().resolve(relative)) + .build()); + } + return null; + } + + @Override + public ModelSource resolve(ModelLocator modelLocator, String relative) { + return null; + } + + @Override + public String getModelId() { + return request.getModelId(); + } + }; + } + private Path doLocateExistingPom(Path project) { if (project == null) { project = Paths.get(System.getProperty("user.dir")); @@ -143,8 +293,45 @@ private Path doLocateExistingPom(Path project) { } } - private Model doRead(XmlReaderRequest request) throws IOException { - return modelXmlFactory.read(request); + private Model doRead(Source source, Map options) throws ModelParserException { + boolean strict = isStrict(options); + XmlReaderRequest.Transformer transformer = getTransformer(options); + try (InputStream is = source.openStream()) { + XmlReaderRequest.XmlReaderRequestBuilder builder = XmlReaderRequest.builder() + .strict(strict) + .location(source.getLocation()) + .path(source.getPath()) + .inputStream(is) + .transformer(transformer); + if (source instanceof ModelSource ms) { + builder.modelId(ms.getModelId()); + } + return modelXmlFactory.read(builder.build()); + } catch (XmlReaderException e) { + if (e.getLocation() != null) { + throw new ModelParserException( + e.getMessage(), + e.getLocation().getLineNumber(), + e.getLocation().getColumnNumber(), + e); + } + throw new ModelParserException(e.getMessage(), e); + } catch (IOException e) { + throw new ModelParserException(e.getMessage(), e); + } + } + + private boolean isStrict(Map options) { + Object value = (options != null) ? options.get(ModelParser.STRICT) : null; + return value == null || Boolean.parseBoolean(value.toString()); + } + + private XmlReaderRequest.Transformer getTransformer(Map options) { + Object value = (options != null) ? options.get(ModelProcessor.TRANSFORMER) : null; + if (value instanceof XmlReaderRequest.Transformer t) { + return t; + } + return new ModelInterningTransformer(); } private String buildDetailedErrorMessage( diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/ModelInterningTransformer.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/ModelInterningTransformer.java new file mode 100644 index 000000000000..c1a8d4f07077 --- /dev/null +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/ModelInterningTransformer.java @@ -0,0 +1,126 @@ +/* + * 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.util.Arrays; +import java.util.Set; +import java.util.stream.Collectors; + +import org.apache.maven.api.Constants; +import org.apache.maven.api.Session; +import org.apache.maven.api.services.xml.XmlReaderRequest; + +/** + * A {@link XmlReaderRequest.Transformer} that interns frequently repeated XML text values + * to reduce memory allocation and garbage collection pressure in large multi-module builds. + * + *

The set of XML contexts to intern is configurable via the + * {@link Constants#MAVEN_MODEL_BUILDER_INTERNS} session property. If the property is absent + * or empty, {@link #DEFAULT_CONTEXTS} is used. + */ +class ModelInterningTransformer implements XmlReaderRequest.Transformer { + + static final Set DEFAULT_CONTEXTS = Set.of( + // Core Maven coordinates + "groupId", + "artifactId", + "version", + "namespaceUri", + "packaging", + + // Dependency-related fields + "scope", + "type", + "classifier", + + // Build and plugin-related fields + "phase", + "goal", + "execution", + + // Repository-related fields + "layout", + "policy", + "checksumPolicy", + "updatePolicy", + + // Common metadata fields + "modelVersion", + "name", + "url", + "system", + "distribution", + "status", + + // SCM fields + "connection", + "developerConnection", + "tag", + + // Common enum-like values that appear frequently + "id", + "inherited", + "optional"); + + private final Set contexts; + + /** + * Creates a transformer with default contexts. + */ + ModelInterningTransformer() { + this.contexts = DEFAULT_CONTEXTS; + } + + /** + * Creates a transformer with contexts from session properties. + * Reads {@link Constants#MAVEN_MODEL_BUILDER_INTERNS} from user then system properties. + * + * @param session the Maven session to read properties from + */ + ModelInterningTransformer(Session session) { + this.contexts = parseContextsFromSession(session); + } + + private static Set parseContextsFromSession(Session session) { + String contextsProperty = session.getUserProperties().get(Constants.MAVEN_MODEL_BUILDER_INTERNS); + if (contextsProperty == null) { + contextsProperty = session.getSystemProperties().get(Constants.MAVEN_MODEL_BUILDER_INTERNS); + } + if (contextsProperty == null || contextsProperty.trim().isEmpty()) { + return DEFAULT_CONTEXTS; + } + return Arrays.stream(contextsProperty.split(",")) + .map(String::trim) + .filter(s -> !s.isEmpty()) + .collect(Collectors.toSet()); + } + + @Override + public String transform(String input, String context) { + return input != null && contexts.contains(context) ? input.intern() : input; + } + + /** + * Returns the contexts that will be interned by this transformer. + * Used for testing purposes. + */ + Set getContexts() { + return contexts; + } +} 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 index 1e512c37394a..5916d4acb4be 100644 --- 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 @@ -35,9 +35,11 @@ import org.apache.maven.api.spi.ModelParserException; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; +import org.mockito.ArgumentCaptor; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; @@ -79,12 +81,10 @@ void testDetailedErrorMessageWithMultipleParsers() throws IOException { 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)); + ModelParserException exception = assertThrows( + ModelParserException.class, + () -> processor.read(Sources.fromPath(pomFile), Map.of(ModelParser.STRICT, true))); String message = exception.getMessage(); @@ -118,10 +118,9 @@ void testDetailedErrorMessageWithSingleParser() throws IOException { 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)); + ModelParserException exception = assertThrows( + ModelParserException.class, + () -> processor.read(Sources.fromPath(pomFile), Map.of(ModelParser.STRICT, true))); String message = exception.getMessage(); assertTrue(message.contains("Tried 1 parser:"), "Message should mention 1 parser (singular)"); @@ -144,10 +143,7 @@ void testSuccessfulParsingDoesNotThrowException() throws Exception { DefaultModelProcessor processor = new DefaultModelProcessor(xmlFactory, Map.of("yaml", yamlParser)); - XmlReaderRequest request = - XmlReaderRequest.builder().path(pomFile).strict(true).build(); - - Model result = processor.read(request); + Model result = processor.read(Sources.fromPath(pomFile), Map.of(ModelParser.STRICT, true)); assertNotNull(result); verify(xmlFactory, never()).read(any(XmlReaderRequest.class)); } @@ -189,12 +185,68 @@ public Model parse(Source source, Map options) { Path located = processor.locateExistingPom(projectDir); assertEquals(projectDir, located); - Model result = processor.read( - XmlReaderRequest.builder().path(located).strict(true).build()); + Model result = processor.read(Sources.fromPath(located), Map.of(ModelParser.STRICT, true)); assertEquals(1, parseCalls.get(), "ModelParser#parse should be called for the located source"); assertEquals("a", result.getArtifactId()); assertEquals(located, result.getPomFile()); verify(xmlFactory, never()).read(any(XmlReaderRequest.class)); } + + @Test + void testDeprecatedReadDelegatesToNewRead() throws Exception { + Path pomFile = tempDir.resolve("pom.xml"); + Files.writeString( + pomFile, + "4.0.0ga1"); + + Model mockModel = Model.newBuilder() + .modelVersion("4.0.0") + .groupId("g") + .artifactId("a") + .version("1") + .build(); + ModelXmlFactory xmlFactory = mock(ModelXmlFactory.class); + when(xmlFactory.read(any(XmlReaderRequest.class))).thenReturn(mockModel); + + DefaultModelProcessor processor = new DefaultModelProcessor(xmlFactory, Map.of()); + XmlReaderRequest request = + XmlReaderRequest.builder().path(pomFile).strict(true).build(); + + Model result = processor.read(request); + assertNotNull(result); + assertEquals("a", result.getArtifactId()); + } + + @Test + void testDeprecatedReadPassesTransformer() throws Exception { + Path pomFile = tempDir.resolve("pom.xml"); + Files.writeString( + pomFile, + "4.0.0ga1"); + + Model mockModel = Model.newBuilder() + .modelVersion("4.0.0") + .groupId("g") + .artifactId("a") + .version("1") + .build(); + ModelXmlFactory xmlFactory = mock(ModelXmlFactory.class); + when(xmlFactory.read(any(XmlReaderRequest.class))).thenReturn(mockModel); + + DefaultModelProcessor processor = new DefaultModelProcessor(xmlFactory, Map.of()); + XmlReaderRequest.Transformer transformer = mock(XmlReaderRequest.Transformer.class); + XmlReaderRequest request = XmlReaderRequest.builder() + .path(pomFile) + .strict(true) + .transformer(transformer) + .build(); + + Model result = processor.read(request); + assertNotNull(result); + + ArgumentCaptor captor = ArgumentCaptor.forClass(XmlReaderRequest.class); + verify(xmlFactory).read(captor.capture()); + assertSame(transformer, captor.getValue().getTransformer()); + } } diff --git a/impl/maven-impl/src/test/java/org/apache/maven/impl/model/InterningTransformerTest.java b/impl/maven-impl/src/test/java/org/apache/maven/impl/model/InterningTransformerTest.java index 87c10d3e19c4..4ddb692b775a 100644 --- a/impl/maven-impl/src/test/java/org/apache/maven/impl/model/InterningTransformerTest.java +++ b/impl/maven-impl/src/test/java/org/apache/maven/impl/model/InterningTransformerTest.java @@ -41,7 +41,7 @@ import static org.mockito.Mockito.when; /** - * Test class for {@link DefaultModelBuilder.InterningTransformer}. + * Test class for {@link ModelInterningTransformer}. * Verifies that the transformer correctly interns commonly used string values * to reduce memory usage during Maven POM parsing. */ @@ -49,7 +49,7 @@ class InterningTransformerTest { @Test void testTransformerInternsCorrectContexts() { - DefaultModelBuilder.InterningTransformer transformer = new DefaultModelBuilder.InterningTransformer(); + ModelInterningTransformer transformer = new ModelInterningTransformer(); // Test that contexts in the CONTEXTS set are interned String groupId1 = transformer.transform("org.apache.maven", "groupId"); @@ -92,96 +92,87 @@ void testTransformerInternsCorrectContexts() { void testTransformerContainsExpectedContexts() { // Verify that the DEFAULT_CONTEXTS set contains all the expected fields assertTrue( - DefaultModelBuilder.InterningTransformer.DEFAULT_CONTEXTS.contains("groupId"), - "Expected " + DefaultModelBuilder.InterningTransformer.DEFAULT_CONTEXTS + " to contain " + "groupId"); + ModelInterningTransformer.DEFAULT_CONTEXTS.contains("groupId"), + "Expected " + ModelInterningTransformer.DEFAULT_CONTEXTS + " to contain " + "groupId"); assertTrue( - DefaultModelBuilder.InterningTransformer.DEFAULT_CONTEXTS.contains("artifactId"), - "Expected " + DefaultModelBuilder.InterningTransformer.DEFAULT_CONTEXTS + " to contain " - + "artifactId"); + ModelInterningTransformer.DEFAULT_CONTEXTS.contains("artifactId"), + "Expected " + ModelInterningTransformer.DEFAULT_CONTEXTS + " to contain " + "artifactId"); assertTrue( - DefaultModelBuilder.InterningTransformer.DEFAULT_CONTEXTS.contains("version"), - "Expected " + DefaultModelBuilder.InterningTransformer.DEFAULT_CONTEXTS + " to contain " + "version"); + ModelInterningTransformer.DEFAULT_CONTEXTS.contains("version"), + "Expected " + ModelInterningTransformer.DEFAULT_CONTEXTS + " to contain " + "version"); assertTrue( - DefaultModelBuilder.InterningTransformer.DEFAULT_CONTEXTS.contains("packaging"), - "Expected " + DefaultModelBuilder.InterningTransformer.DEFAULT_CONTEXTS + " to contain " + "packaging"); + ModelInterningTransformer.DEFAULT_CONTEXTS.contains("packaging"), + "Expected " + ModelInterningTransformer.DEFAULT_CONTEXTS + " to contain " + "packaging"); assertTrue( - DefaultModelBuilder.InterningTransformer.DEFAULT_CONTEXTS.contains("scope"), - "Expected " + DefaultModelBuilder.InterningTransformer.DEFAULT_CONTEXTS + " to contain " + "scope"); + ModelInterningTransformer.DEFAULT_CONTEXTS.contains("scope"), + "Expected " + ModelInterningTransformer.DEFAULT_CONTEXTS + " to contain " + "scope"); assertTrue( - DefaultModelBuilder.InterningTransformer.DEFAULT_CONTEXTS.contains("type"), - "Expected " + DefaultModelBuilder.InterningTransformer.DEFAULT_CONTEXTS + " to contain " + "type"); + ModelInterningTransformer.DEFAULT_CONTEXTS.contains("type"), + "Expected " + ModelInterningTransformer.DEFAULT_CONTEXTS + " to contain " + "type"); assertTrue( - DefaultModelBuilder.InterningTransformer.DEFAULT_CONTEXTS.contains("classifier"), - "Expected " + DefaultModelBuilder.InterningTransformer.DEFAULT_CONTEXTS + " to contain " - + "classifier"); + ModelInterningTransformer.DEFAULT_CONTEXTS.contains("classifier"), + "Expected " + ModelInterningTransformer.DEFAULT_CONTEXTS + " to contain " + "classifier"); assertTrue( - DefaultModelBuilder.InterningTransformer.DEFAULT_CONTEXTS.contains("goal"), - "Expected " + DefaultModelBuilder.InterningTransformer.DEFAULT_CONTEXTS + " to contain " + "goal"); + ModelInterningTransformer.DEFAULT_CONTEXTS.contains("goal"), + "Expected " + ModelInterningTransformer.DEFAULT_CONTEXTS + " to contain " + "goal"); assertTrue( - DefaultModelBuilder.InterningTransformer.DEFAULT_CONTEXTS.contains("execution"), - "Expected " + DefaultModelBuilder.InterningTransformer.DEFAULT_CONTEXTS + " to contain " + "execution"); + ModelInterningTransformer.DEFAULT_CONTEXTS.contains("execution"), + "Expected " + ModelInterningTransformer.DEFAULT_CONTEXTS + " to contain " + "execution"); assertTrue( - DefaultModelBuilder.InterningTransformer.DEFAULT_CONTEXTS.contains("phase"), - "Expected " + DefaultModelBuilder.InterningTransformer.DEFAULT_CONTEXTS + " to contain " + "phase"); + ModelInterningTransformer.DEFAULT_CONTEXTS.contains("phase"), + "Expected " + ModelInterningTransformer.DEFAULT_CONTEXTS + " to contain " + "phase"); assertTrue( - DefaultModelBuilder.InterningTransformer.DEFAULT_CONTEXTS.contains("modelVersion"), - "Expected " + DefaultModelBuilder.InterningTransformer.DEFAULT_CONTEXTS + " to contain " - + "modelVersion"); + ModelInterningTransformer.DEFAULT_CONTEXTS.contains("modelVersion"), + "Expected " + ModelInterningTransformer.DEFAULT_CONTEXTS + " to contain " + "modelVersion"); assertTrue( - DefaultModelBuilder.InterningTransformer.DEFAULT_CONTEXTS.contains("name"), - "Expected " + DefaultModelBuilder.InterningTransformer.DEFAULT_CONTEXTS + " to contain " + "name"); + ModelInterningTransformer.DEFAULT_CONTEXTS.contains("name"), + "Expected " + ModelInterningTransformer.DEFAULT_CONTEXTS + " to contain " + "name"); assertTrue( - DefaultModelBuilder.InterningTransformer.DEFAULT_CONTEXTS.contains("url"), - "Expected " + DefaultModelBuilder.InterningTransformer.DEFAULT_CONTEXTS + " to contain " + "url"); + ModelInterningTransformer.DEFAULT_CONTEXTS.contains("url"), + "Expected " + ModelInterningTransformer.DEFAULT_CONTEXTS + " to contain " + "url"); assertTrue( - DefaultModelBuilder.InterningTransformer.DEFAULT_CONTEXTS.contains("system"), - "Expected " + DefaultModelBuilder.InterningTransformer.DEFAULT_CONTEXTS + " to contain " + "system"); + ModelInterningTransformer.DEFAULT_CONTEXTS.contains("system"), + "Expected " + ModelInterningTransformer.DEFAULT_CONTEXTS + " to contain " + "system"); assertTrue( - DefaultModelBuilder.InterningTransformer.DEFAULT_CONTEXTS.contains("distribution"), - "Expected " + DefaultModelBuilder.InterningTransformer.DEFAULT_CONTEXTS + " to contain " - + "distribution"); + ModelInterningTransformer.DEFAULT_CONTEXTS.contains("distribution"), + "Expected " + ModelInterningTransformer.DEFAULT_CONTEXTS + " to contain " + "distribution"); assertTrue( - DefaultModelBuilder.InterningTransformer.DEFAULT_CONTEXTS.contains("status"), - "Expected " + DefaultModelBuilder.InterningTransformer.DEFAULT_CONTEXTS + " to contain " + "status"); + ModelInterningTransformer.DEFAULT_CONTEXTS.contains("status"), + "Expected " + ModelInterningTransformer.DEFAULT_CONTEXTS + " to contain " + "status"); assertTrue( - DefaultModelBuilder.InterningTransformer.DEFAULT_CONTEXTS.contains("connection"), - "Expected " + DefaultModelBuilder.InterningTransformer.DEFAULT_CONTEXTS + " to contain " - + "connection"); + ModelInterningTransformer.DEFAULT_CONTEXTS.contains("connection"), + "Expected " + ModelInterningTransformer.DEFAULT_CONTEXTS + " to contain " + "connection"); assertTrue( - DefaultModelBuilder.InterningTransformer.DEFAULT_CONTEXTS.contains("developerConnection"), - "Expected " + DefaultModelBuilder.InterningTransformer.DEFAULT_CONTEXTS + " to contain " - + "developerConnection"); + ModelInterningTransformer.DEFAULT_CONTEXTS.contains("developerConnection"), + "Expected " + ModelInterningTransformer.DEFAULT_CONTEXTS + " to contain " + "developerConnection"); assertTrue( - DefaultModelBuilder.InterningTransformer.DEFAULT_CONTEXTS.contains("tag"), - "Expected " + DefaultModelBuilder.InterningTransformer.DEFAULT_CONTEXTS + " to contain " + "tag"); + ModelInterningTransformer.DEFAULT_CONTEXTS.contains("tag"), + "Expected " + ModelInterningTransformer.DEFAULT_CONTEXTS + " to contain " + "tag"); assertTrue( - DefaultModelBuilder.InterningTransformer.DEFAULT_CONTEXTS.contains("id"), - "Expected " + DefaultModelBuilder.InterningTransformer.DEFAULT_CONTEXTS + " to contain " + "id"); + ModelInterningTransformer.DEFAULT_CONTEXTS.contains("id"), + "Expected " + ModelInterningTransformer.DEFAULT_CONTEXTS + " to contain " + "id"); assertTrue( - DefaultModelBuilder.InterningTransformer.DEFAULT_CONTEXTS.contains("inherited"), - "Expected " + DefaultModelBuilder.InterningTransformer.DEFAULT_CONTEXTS + " to contain " + "inherited"); + ModelInterningTransformer.DEFAULT_CONTEXTS.contains("inherited"), + "Expected " + ModelInterningTransformer.DEFAULT_CONTEXTS + " to contain " + "inherited"); assertTrue( - DefaultModelBuilder.InterningTransformer.DEFAULT_CONTEXTS.contains("optional"), - "Expected " + DefaultModelBuilder.InterningTransformer.DEFAULT_CONTEXTS + " to contain " + "optional"); + ModelInterningTransformer.DEFAULT_CONTEXTS.contains("optional"), + "Expected " + ModelInterningTransformer.DEFAULT_CONTEXTS + " to contain " + "optional"); // Verify that non-interned contexts are not in the set assertFalse( - DefaultModelBuilder.InterningTransformer.DEFAULT_CONTEXTS.contains("nonInterned"), - "Expected " + DefaultModelBuilder.InterningTransformer.DEFAULT_CONTEXTS + " to not contain " - + "nonInterned"); + ModelInterningTransformer.DEFAULT_CONTEXTS.contains("nonInterned"), + "Expected " + ModelInterningTransformer.DEFAULT_CONTEXTS + " to not contain " + "nonInterned"); assertFalse( - DefaultModelBuilder.InterningTransformer.DEFAULT_CONTEXTS.contains("description"), - "Expected " + DefaultModelBuilder.InterningTransformer.DEFAULT_CONTEXTS + " to not contain " - + "description"); + ModelInterningTransformer.DEFAULT_CONTEXTS.contains("description"), + "Expected " + ModelInterningTransformer.DEFAULT_CONTEXTS + " to not contain " + "description"); assertFalse( - DefaultModelBuilder.InterningTransformer.DEFAULT_CONTEXTS.contains("randomField"), - "Expected " + DefaultModelBuilder.InterningTransformer.DEFAULT_CONTEXTS + " to not contain " - + "randomField"); + ModelInterningTransformer.DEFAULT_CONTEXTS.contains("randomField"), + "Expected " + ModelInterningTransformer.DEFAULT_CONTEXTS + " to not contain " + "randomField"); } @Test void testTransformerWithNullAndEmptyValues() { - DefaultModelBuilder.InterningTransformer transformer = new DefaultModelBuilder.InterningTransformer(); + ModelInterningTransformer transformer = new ModelInterningTransformer(); // Test with null value String result1 = transformer.transform(null, "groupId"); @@ -282,7 +273,7 @@ void testInterningTransformerWithRealPomParsing() throws Exception { """; DefaultModelXmlFactory factory = new DefaultModelXmlFactory(); - DefaultModelBuilder.InterningTransformer transformer = new DefaultModelBuilder.InterningTransformer(); + ModelInterningTransformer transformer = new ModelInterningTransformer(); XmlReaderRequest request = XmlReaderRequest.builder() .reader(new StringReader(pomXml)) @@ -314,7 +305,7 @@ void testTransformerWithSessionPropertyUserProperties() { Session session = Mockito.mock(Session.class); when(session.getUserProperties()).thenReturn(userProperties); - DefaultModelBuilder.InterningTransformer transformer = new DefaultModelBuilder.InterningTransformer(session); + ModelInterningTransformer transformer = new ModelInterningTransformer(session); // Test that custom contexts are used assertTrue(transformer.getContexts().contains("groupId")); @@ -353,7 +344,7 @@ void testTransformerWithSessionPropertySystemProperties() { Session session = Mockito.mock(Session.class); when(session.getSystemProperties()).thenReturn(systemProperties); - DefaultModelBuilder.InterningTransformer transformer = new DefaultModelBuilder.InterningTransformer(session); + ModelInterningTransformer transformer = new ModelInterningTransformer(session); // Test that custom contexts are used assertTrue(transformer.getContexts().contains("scope")); @@ -379,7 +370,7 @@ void testTransformerUserPropertiesOverrideSystemProperties() { when(session.getUserProperties()).thenReturn(userProperties); when(session.getSystemProperties()).thenReturn(systemProperties); - DefaultModelBuilder.InterningTransformer transformer = new DefaultModelBuilder.InterningTransformer(session); + ModelInterningTransformer transformer = new ModelInterningTransformer(session); // Test that user properties are used, not system properties assertTrue(transformer.getContexts().contains("groupId")); @@ -398,10 +389,10 @@ void testTransformerWithEmptySessionProperty() { Session session = Mockito.mock(Session.class); when(session.getUserProperties()).thenReturn(userProperties); - DefaultModelBuilder.InterningTransformer transformer = new DefaultModelBuilder.InterningTransformer(session); + ModelInterningTransformer transformer = new ModelInterningTransformer(session); // Should use default contexts - assertEquals(DefaultModelBuilder.InterningTransformer.DEFAULT_CONTEXTS, transformer.getContexts()); + assertEquals(ModelInterningTransformer.DEFAULT_CONTEXTS, transformer.getContexts()); } @Test @@ -413,10 +404,10 @@ void testTransformerWithWhitespaceOnlySessionProperty() { Session session = Mockito.mock(Session.class); when(session.getUserProperties()).thenReturn(userProperties); - DefaultModelBuilder.InterningTransformer transformer = new DefaultModelBuilder.InterningTransformer(session); + ModelInterningTransformer transformer = new ModelInterningTransformer(session); // Should use default contexts - assertEquals(DefaultModelBuilder.InterningTransformer.DEFAULT_CONTEXTS, transformer.getContexts()); + assertEquals(ModelInterningTransformer.DEFAULT_CONTEXTS, transformer.getContexts()); } @Test @@ -424,10 +415,10 @@ void testTransformerWithNoSessionProperty() { // Test with no property set - should use defaults Session session = Mockito.mock(Session.class); - DefaultModelBuilder.InterningTransformer transformer = new DefaultModelBuilder.InterningTransformer(session); + ModelInterningTransformer transformer = new ModelInterningTransformer(session); // Should use default contexts - assertEquals(DefaultModelBuilder.InterningTransformer.DEFAULT_CONTEXTS, transformer.getContexts()); + assertEquals(ModelInterningTransformer.DEFAULT_CONTEXTS, transformer.getContexts()); } @Test @@ -439,7 +430,7 @@ void testTransformerWithCommaSeparatedValues() { Session session = Mockito.mock(Session.class); when(session.getUserProperties()).thenReturn(userProperties); - DefaultModelBuilder.InterningTransformer transformer = new DefaultModelBuilder.InterningTransformer(session); + ModelInterningTransformer transformer = new ModelInterningTransformer(session); // Test that all values are parsed correctly (whitespace trimmed) assertTrue(transformer.getContexts().contains("groupId")); @@ -459,7 +450,7 @@ void testTransformerWithEmptyCommaSeparatedValues() { Session session = Mockito.mock(Session.class); when(session.getUserProperties()).thenReturn(userProperties); - DefaultModelBuilder.InterningTransformer transformer = new DefaultModelBuilder.InterningTransformer(session); + ModelInterningTransformer transformer = new ModelInterningTransformer(session); // Test that empty values are filtered out assertTrue(transformer.getContexts().contains("groupId"));