This is an automated email from the ASF dual-hosted git repository. rfscholte pushed a commit to branch MNG-6999 in repository https://gitbox.apache.org/repos/asf/maven.git
commit 1809ae05d86ac5f194d0d1b2235006b525af3f87 Author: rfscholte <[email protected]> AuthorDate: Wed Oct 21 20:13:02 2020 +0200 [MNG-6999] Chained (consumer) XMLFilters can result in "floating" comments --- .../aether/ConsumerModelSourceTransformer.java | 13 +++-- .../aether/ConsumerModelSourceTransformerTest.java | 22 +++----- .../{transform.pom => transform/after.pom} | 2 + .../{transform.pom => transform/before.pom} | 5 ++ .../building/AbstractModelSourceTransformer.java | 64 +++++++++++++--------- .../building/BuildModelSourceTransformer.java | 11 +++- .../building/DefaultBuildPomXMLFilterFactory.java | 6 +- .../DefaultInheritanceAssemblerTest.java | 7 ++- .../xml/sax/filter/BuildPomXMLFilterFactory.java | 4 +- .../apache/maven/xml/sax/ChainedFilterTest.java | 2 +- 10 files changed, 83 insertions(+), 53 deletions(-) diff --git a/maven-core/src/main/java/org/apache/maven/internal/aether/ConsumerModelSourceTransformer.java b/maven-core/src/main/java/org/apache/maven/internal/aether/ConsumerModelSourceTransformer.java index 728c78e..d81571d 100644 --- a/maven-core/src/main/java/org/apache/maven/internal/aether/ConsumerModelSourceTransformer.java +++ b/maven-core/src/main/java/org/apache/maven/internal/aether/ConsumerModelSourceTransformer.java @@ -23,6 +23,7 @@ import java.io.IOException; import java.io.InputStream; import java.nio.file.Files; import java.nio.file.Path; +import java.util.function.Consumer; import javax.xml.parsers.ParserConfigurationException; import javax.xml.stream.XMLInputFactory; @@ -37,18 +38,21 @@ import javax.xml.transform.sax.TransformerHandler; import org.apache.maven.model.building.AbstractModelSourceTransformer; import org.apache.maven.model.building.DefaultBuildPomXMLFilterFactory; import org.apache.maven.model.building.TransformerContext; -import org.apache.maven.xml.Factories; import org.apache.maven.xml.internal.DefaultConsumerPomXMLFilterFactory; import org.apache.maven.xml.sax.filter.AbstractSAXFilter; import org.xml.sax.SAXException; +import org.xml.sax.ext.LexicalHandler; class ConsumerModelSourceTransformer extends AbstractModelSourceTransformer { @Override - protected AbstractSAXFilter getSAXFilter( Path pomFile, TransformerContext context ) + protected AbstractSAXFilter getSAXFilter( Path pomFile, + TransformerContext context, + Consumer<LexicalHandler> lexicalHandlerConsumer ) throws TransformerConfigurationException, SAXException, ParserConfigurationException { - return new DefaultConsumerPomXMLFilterFactory( new DefaultBuildPomXMLFilterFactory( context ) ).get( pomFile ); + return new DefaultConsumerPomXMLFilterFactory( new DefaultBuildPomXMLFilterFactory( context, + lexicalHandlerConsumer ) ).get( pomFile ); } /** @@ -65,8 +69,7 @@ class ConsumerModelSourceTransformer extends AbstractModelSourceTransformer { final TransformerHandler transformerHandler; - final SAXTransformerFactory transformerFactory = - (SAXTransformerFactory) Factories.newTransformerFactory(); + final SAXTransformerFactory transformerFactory = getTransformerFactory(); // Keep same encoding+version try ( InputStream input = Files.newInputStream( pomFile ) ) diff --git a/maven-core/src/test/java/org/apache/maven/internal/aether/ConsumerModelSourceTransformerTest.java b/maven-core/src/test/java/org/apache/maven/internal/aether/ConsumerModelSourceTransformerTest.java index 4863264..9476f3e 100644 --- a/maven-core/src/test/java/org/apache/maven/internal/aether/ConsumerModelSourceTransformerTest.java +++ b/maven-core/src/test/java/org/apache/maven/internal/aether/ConsumerModelSourceTransformerTest.java @@ -1,11 +1,5 @@ package org.apache.maven.internal.aether; -import java.io.BufferedReader; -import java.io.InputStream; -import java.io.InputStreamReader; -import java.nio.charset.StandardCharsets; -import java.nio.file.Files; - /* * Licensed to the Apache Software Foundation (ASF) under one * or more contributor license agreements. See the NOTICE file @@ -25,9 +19,10 @@ import java.nio.file.Files; * under the License. */ +import java.io.InputStream; +import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; -import java.util.stream.Collectors; import org.apache.maven.model.Model; import org.apache.maven.model.building.TransformerContext; @@ -41,14 +36,13 @@ public class ConsumerModelSourceTransformerTest @Test public void test() throws Exception { - Path pomFile = Paths.get( "src/test/resources/projects/transform.pom").toAbsolutePath(); - InputStream in = transformer.transform( pomFile, new NoTransformerContext() ); - - String text = - new BufferedReader( new InputStreamReader( in, - StandardCharsets.UTF_8 ) ).lines().collect( Collectors.joining( "\n" ) ); + Path beforePomFile = Paths.get( "src/test/resources/projects/transform/before.pom").toAbsolutePath(); + Path afterPomFile = Paths.get( "src/test/resources/projects/transform/after.pom").toAbsolutePath(); - XmlAssert.assertThat( in ).and( Files.newInputStream( pomFile ) ).areIdentical(); + try( InputStream in = transformer.transform( beforePomFile, new NoTransformerContext() ) ) + { + XmlAssert.assertThat( in ).and( Files.newInputStream( afterPomFile ) ).areIdentical(); + } } private static class NoTransformerContext implements TransformerContext diff --git a/maven-core/src/test/resources/projects/transform.pom b/maven-core/src/test/resources/projects/transform/after.pom similarity index 99% copy from maven-core/src/test/resources/projects/transform.pom copy to maven-core/src/test/resources/projects/transform/after.pom index f48aba3..a66fca8 100644 --- a/maven-core/src/test/resources/projects/transform.pom +++ b/maven-core/src/test/resources/projects/transform/after.pom @@ -29,6 +29,8 @@ under the License. <version>0.1-SNAPSHOT</version> <packaging>pom</packaging> + + <build> <pluginManagement> <plugins> diff --git a/maven-core/src/test/resources/projects/transform.pom b/maven-core/src/test/resources/projects/transform/before.pom similarity index 95% rename from maven-core/src/test/resources/projects/transform.pom rename to maven-core/src/test/resources/projects/transform/before.pom index f48aba3..c6b2bc2 100644 --- a/maven-core/src/test/resources/projects/transform.pom +++ b/maven-core/src/test/resources/projects/transform/before.pom @@ -29,6 +29,11 @@ under the License. <version>0.1-SNAPSHOT</version> <packaging>pom</packaging> + <modules> + <module>lib</module> <!-- the library --> + <module>app</module> <!-- the application --> + </modules> + <build> <pluginManagement> <plugins> diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/building/AbstractModelSourceTransformer.java b/maven-model-builder/src/main/java/org/apache/maven/model/building/AbstractModelSourceTransformer.java index 6f0f5d4..b4e8698 100644 --- a/maven-model-builder/src/main/java/org/apache/maven/model/building/AbstractModelSourceTransformer.java +++ b/maven-model-builder/src/main/java/org/apache/maven/model/building/AbstractModelSourceTransformer.java @@ -19,22 +19,25 @@ package org.apache.maven.model.building; * under the License. */ -import java.io.FileInputStream; import java.io.FilterInputStream; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; import java.io.PipedInputStream; import java.io.PipedOutputStream; +import java.nio.file.Files; import java.nio.file.Path; import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Consumer; import javax.xml.parsers.ParserConfigurationException; +import javax.xml.transform.Transformer; import javax.xml.transform.TransformerConfigurationException; import javax.xml.transform.TransformerException; import javax.xml.transform.TransformerFactory; import javax.xml.transform.sax.SAXResult; import javax.xml.transform.sax.SAXSource; +import javax.xml.transform.sax.SAXTransformerFactory; import javax.xml.transform.sax.TransformerHandler; import javax.xml.transform.stream.StreamResult; @@ -44,6 +47,7 @@ import org.apache.maven.xml.sax.filter.AbstractSAXFilter; import org.xml.sax.ErrorHandler; import org.xml.sax.SAXException; import org.xml.sax.SAXParseException; +import org.xml.sax.ext.LexicalHandler; /** * Offers a transformation implementation based on PipelineStreams. @@ -59,7 +63,9 @@ public abstract class AbstractModelSourceTransformer private final TransformerFactory transformerFactory = Factories.newTransformerFactory(); - protected abstract AbstractSAXFilter getSAXFilter( Path pomFile, TransformerContext context ) + protected abstract AbstractSAXFilter getSAXFilter( Path pomFile, + TransformerContext context, + Consumer<LexicalHandler> lexicalHandlerConsumer ) throws TransformerConfigurationException, SAXException, ParserConfigurationException; protected OutputStream filterOutputStream( OutputStream outputStream, Path pomFile ) @@ -67,6 +73,11 @@ public abstract class AbstractModelSourceTransformer return outputStream; } + public SAXTransformerFactory getTransformerFactory() + { + return ( SAXTransformerFactory ) transformerFactory; + } + protected TransformerHandler getTransformerHandler( Path pomFile ) throws IOException, org.apache.maven.model.building.TransformerException { @@ -79,10 +90,29 @@ public abstract class AbstractModelSourceTransformer { final TransformerHandler transformerHandler = getTransformerHandler( pomFile ); + final PipedOutputStream pout = new PipedOutputStream(); + + OutputStream out = filterOutputStream( pout, pomFile ); + + final javax.xml.transform.Result result; + final Consumer<LexicalHandler> lexConsumer; + if ( transformerHandler == null ) + { + result = new StreamResult( out ); + lexConsumer = null; + } + else + { + result = new SAXResult( transformerHandler ); + lexConsumer = l -> ( (SAXResult) result ).setLexicalHandler( new CommentRenormalizer( l ) ); + transformerHandler.setResult( new StreamResult( out ) ); + } + + final Transformer transformer; final AbstractSAXFilter filter; try { - filter = getSAXFilter( pomFile, context ); + filter = getSAXFilter( pomFile, context, lexConsumer ); filter.setLexicalHandler( transformerHandler ); // By default errors are written to stderr. // Hence set custom errorHandler to reduce noice @@ -109,6 +139,7 @@ public abstract class AbstractModelSourceTransformer throw exception; } } ); + transformer = transformerFactory.newTransformer(); } catch ( TransformerConfigurationException | SAXException | ParserConfigurationException e ) { @@ -116,33 +147,16 @@ public abstract class AbstractModelSourceTransformer } final SAXSource transformSource = - new SAXSource( filter, - new org.xml.sax.InputSource( new FileInputStream( pomFile.toFile() ) ) ); - - final PipedOutputStream pout = new PipedOutputStream(); - final PipedInputStream pipedInputStream = new PipedInputStream( pout ); - - OutputStream out = filterOutputStream( pout, pomFile ); - - final javax.xml.transform.Result result; - if ( transformerHandler == null ) - { - result = new StreamResult( out ); - } - else - { - result = new SAXResult( transformerHandler ); - ( (SAXResult) result ).setLexicalHandler( new CommentRenormalizer( transformerHandler ) ); - transformerHandler.setResult( new StreamResult( out ) ); - } + new SAXSource( filter, new org.xml.sax.InputSource( Files.newInputStream( pomFile ) ) ); IOExceptionHandler eh = new IOExceptionHandler(); + Thread transformThread = new Thread( () -> { try ( PipedOutputStream pos = pout ) { - transformerFactory.newTransformer().transform( transformSource, result ); + transformer.transform( transformSource, result ); } catch ( TransformerException | IOException e ) { @@ -152,8 +166,8 @@ public abstract class AbstractModelSourceTransformer transformThread.setUncaughtExceptionHandler( eh ); transformThread.setDaemon( true ); transformThread.start(); - - return new ThreadAwareInputStream( pipedInputStream, eh ); + + return new ThreadAwareInputStream( new PipedInputStream( pout ), eh ); } private static class IOExceptionHandler diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/building/BuildModelSourceTransformer.java b/maven-model-builder/src/main/java/org/apache/maven/model/building/BuildModelSourceTransformer.java index dbf9211..44d4c26 100644 --- a/maven-model-builder/src/main/java/org/apache/maven/model/building/BuildModelSourceTransformer.java +++ b/maven-model-builder/src/main/java/org/apache/maven/model/building/BuildModelSourceTransformer.java @@ -23,6 +23,7 @@ import java.io.FilterOutputStream; import java.io.IOException; import java.io.OutputStream; import java.nio.file.Path; +import java.util.function.Consumer; import javax.inject.Inject; import javax.inject.Named; @@ -35,6 +36,7 @@ import org.apache.maven.xml.sax.filter.BuildPomXMLFilterFactory; import org.apache.maven.xml.sax.filter.BuildPomXMLFilterListener; import org.eclipse.sisu.Nullable; import org.xml.sax.SAXException; +import org.xml.sax.ext.LexicalHandler; /** * ModelSourceTransformer for the build pom @@ -50,11 +52,14 @@ class BuildModelSourceTransformer extends AbstractModelSourceTransformer @Nullable private BuildPomXMLFilterListener xmlFilterListener; - protected AbstractSAXFilter getSAXFilter( Path pomFile, TransformerContext context ) + protected AbstractSAXFilter getSAXFilter( Path pomFile, + TransformerContext context, + Consumer<LexicalHandler> lexicalHandlerConsumer ) throws TransformerConfigurationException, SAXException, ParserConfigurationException { - BuildPomXMLFilterFactory buildPomXMLFilterFactory = new DefaultBuildPomXMLFilterFactory( context ); - + BuildPomXMLFilterFactory buildPomXMLFilterFactory = + new DefaultBuildPomXMLFilterFactory( context, lexicalHandlerConsumer ); + return buildPomXMLFilterFactory.get( pomFile ); } diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultBuildPomXMLFilterFactory.java b/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultBuildPomXMLFilterFactory.java index 3de90de..8f9f6ae 100644 --- a/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultBuildPomXMLFilterFactory.java +++ b/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultBuildPomXMLFilterFactory.java @@ -23,11 +23,13 @@ package org.apache.maven.model.building; import java.nio.file.Path; import java.util.Optional; import java.util.function.BiFunction; +import java.util.function.Consumer; import java.util.function.Function; import org.apache.maven.model.Model; import org.apache.maven.xml.sax.filter.BuildPomXMLFilterFactory; import org.apache.maven.xml.sax.filter.RelativeProject; +import org.xml.sax.ext.LexicalHandler; /** * @@ -38,8 +40,10 @@ public class DefaultBuildPomXMLFilterFactory extends BuildPomXMLFilterFactory { private final TransformerContext context; - public DefaultBuildPomXMLFilterFactory( TransformerContext context ) + public DefaultBuildPomXMLFilterFactory( TransformerContext context, + Consumer<LexicalHandler> lexicalHandlerConsumer ) { + super( lexicalHandlerConsumer ); this.context = context; } diff --git a/maven-model-builder/src/test/java/org/apache/maven/model/inheritance/DefaultInheritanceAssemblerTest.java b/maven-model-builder/src/test/java/org/apache/maven/model/inheritance/DefaultInheritanceAssemblerTest.java index 9924471..131b762 100644 --- a/maven-model-builder/src/test/java/org/apache/maven/model/inheritance/DefaultInheritanceAssemblerTest.java +++ b/maven-model-builder/src/test/java/org/apache/maven/model/inheritance/DefaultInheritanceAssemblerTest.java @@ -28,6 +28,7 @@ import org.apache.maven.model.io.DefaultModelWriter; import org.apache.maven.model.io.ModelWriter; import org.apache.maven.xml.sax.filter.AbstractSAXFilter; import org.xml.sax.SAXException; +import org.xml.sax.ext.LexicalHandler; import org.xmlunit.matchers.CompareMatcher; import junit.framework.TestCase; @@ -35,11 +36,12 @@ import junit.framework.TestCase; import java.io.File; import java.io.IOException; import java.nio.file.Path; +import java.util.function.Consumer; import javax.xml.parsers.ParserConfigurationException; import javax.xml.transform.TransformerConfigurationException; -import static org.junit.Assert.assertThat; +import static org.hamcrest.MatcherAssert.assertThat; /** * @author Hervé Boutemy @@ -63,7 +65,8 @@ public class DefaultInheritanceAssemblerTest reader.setTransformer( new AbstractModelSourceTransformer() { @Override - protected AbstractSAXFilter getSAXFilter( Path pomFile, TransformerContext context ) + protected AbstractSAXFilter getSAXFilter( Path pomFile, TransformerContext context, + Consumer<LexicalHandler> lexicalHandlerConsumer ) throws TransformerConfigurationException, SAXException, ParserConfigurationException { return null; diff --git a/maven-xml/src/main/java/org/apache/maven/xml/sax/filter/BuildPomXMLFilterFactory.java b/maven-xml/src/main/java/org/apache/maven/xml/sax/filter/BuildPomXMLFilterFactory.java index 4468da2..23dc240 100644 --- a/maven-xml/src/main/java/org/apache/maven/xml/sax/filter/BuildPomXMLFilterFactory.java +++ b/maven-xml/src/main/java/org/apache/maven/xml/sax/filter/BuildPomXMLFilterFactory.java @@ -76,7 +76,7 @@ public class BuildPomXMLFilterFactory ReactorDependencyXMLFilter reactorDependencyXMLFilter = new ReactorDependencyXMLFilter( getDependencyKeyToVersionMapper() ); reactorDependencyXMLFilter.setParent( parent ); - reactorDependencyXMLFilter.setLexicalHandler( parent ); + parent.setLexicalHandler( reactorDependencyXMLFilter ); parent = reactorDependencyXMLFilter; } @@ -85,7 +85,7 @@ public class BuildPomXMLFilterFactory ParentXMLFilter parentFilter = new ParentXMLFilter( getRelativePathMapper() ); parentFilter.setProjectPath( projectFile.getParent() ); parentFilter.setParent( parent ); - parentFilter.setLexicalHandler( parent ); + parent.setLexicalHandler( parentFilter ); parent = parentFilter; } diff --git a/maven-xml/src/test/java/org/apache/maven/xml/sax/ChainedFilterTest.java b/maven-xml/src/test/java/org/apache/maven/xml/sax/ChainedFilterTest.java index 87fb6de..9af2e7c 100644 --- a/maven-xml/src/test/java/org/apache/maven/xml/sax/ChainedFilterTest.java +++ b/maven-xml/src/test/java/org/apache/maven/xml/sax/ChainedFilterTest.java @@ -60,7 +60,6 @@ public class ChainedFilterTest StreamResult result = new StreamResult( writer ); transformerHandler.setResult( result ); - Transformer transformer = transformerFactory.newTransformer(); SAXResult transformResult = new SAXResult( transformerHandler ); // Watch the order of filters! In reverse order the values would be 'AweSome' @@ -78,6 +77,7 @@ public class ChainedFilterTest SAXSource transformSource = new SAXSource( filter, new InputSource( new StringReader( input ) ) ); + Transformer transformer = transformerFactory.newTransformer(); transformer.transform( transformSource, transformResult ); String expected = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n"
