[GitHub] [maven] eolivelli commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2020-06-14 Thread GitBox


eolivelli commented on a change in pull request #286:
URL: https://github.com/apache/maven/pull/286#discussion_r439825810



##
File path: 
maven-model-builder/src/main/java/org/apache/maven/feature/Features.java
##
@@ -0,0 +1,63 @@
+package org.apache.maven.feature;
+
+/*
+ * 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.
+ */
+
+/**
+ * Centralized class for feature information
+ * 
+ * @author Robert Scholte
+ * @since 3.7.0
+ */
+public final class Features
+{
+private Features() 
+{
+}
+
+private static final Feature BUILDCONSUMER = new Feature( 
"maven.experimental.buildconsumer", "true" );

Review comment:
   Okay to enable it by default





This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [maven] eolivelli commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2020-02-04 Thread GitBox
eolivelli commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r374957776
 
 

 ##
 File path: 
maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java
 ##
 @@ -816,6 +894,81 @@ private void assembleInheritance( List 
lineage, ModelBuildingRequest
 inheritanceAssembler.assembleModelInheritance( child, parent, 
request, problems );
 }
 }
+
+private InputStream transformData( final Path pomFile )
+throws IOException, TransformerException, SAXException, 
ParserConfigurationException
+{
+final TransformerFactory transformerFactory = 
Factories.newTransformerFactory() ;
+
+final PipedOutputStream pipedOutputStream  = new PipedOutputStream();
+final PipedInputStream pipedInputStream  = new PipedInputStream( 
pipedOutputStream );
+
+// Should always be FileSource for reactor poms
+// System.out.println( "transforming " + pomFile );
+
+final SAXSource transformSource =
+new SAXSource( buildPomXMLFilterFactory.get().get( pomFile ),
+   new org.xml.sax.InputSource( new FileInputStream( 
pomFile.toFile() ) ) );
+
+OutputStream out;
+if ( xmlFilterListener != null )
+{
+out = new FilterOutputStream( pipedOutputStream )
+{
+@Override
+public void write( byte[] b, int off, int len )
+throws IOException
+{
+super.write( b, off, len );
+xmlFilterListener.write( pomFile, b, off, len );
+}  
+};
+}
+else
+{
+out = pipedOutputStream;
+}
+
+final StreamResult result = new StreamResult( out );
+
+final Callable callable = () ->
 
 Review comment:
   Se should remove commented lines


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] eolivelli commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2019-10-16 Thread GitBox
eolivelli commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r335714696
 
 

 ##
 File path: maven-xml/src/main/java/org/apache/maven/xml/Factories.java
 ##
 @@ -0,0 +1,118 @@
+package org.apache.maven.xml;
+
+/*
+ * 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.
+ */
+
+import javax.xml.XMLConstants;
+import javax.xml.parsers.ParserConfigurationException;
+import javax.xml.parsers.SAXParser;
+import javax.xml.parsers.SAXParserFactory;
+import javax.xml.transform.TransformerFactory;
+
+import org.xml.sax.SAXException;
+import org.xml.sax.SAXNotRecognizedException;
+import org.xml.sax.SAXNotSupportedException;
+import org.xml.sax.XMLReader;
+
+/**
+ * Creates XML related factories with OWASP advices applied
+ * 
+ * @author Robert Scholte
+ * @since 3.7.0
+ */
+public final class Factories
 
 Review comment:
Very good!


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] eolivelli commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2019-10-16 Thread GitBox
eolivelli commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r335714225
 
 

 ##
 File path: 
maven-model-builder/src/main/java/org/apache/maven/feature/Features.java
 ##
 @@ -0,0 +1,63 @@
+package org.apache.maven.feature;
+
+/*
+ * 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.
+ */
+
+/**
+ * Centralized class for feature information
+ * 
+ * @author Robert Scholte
+ * @since 3.7.0
+ */
+public final class Features
 
 Review comment:
   Cool


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] eolivelli commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2019-10-06 Thread GitBox
eolivelli commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r331789656
 
 

 ##
 File path: 
maven-core/src/main/java/org/apache/maven/internal/aether/DefaultRepositorySystemSessionFactory.java
 ##
 @@ -238,9 +263,83 @@ else if ( request.isUpdateSnapshots() )
 mavenRepositorySystem.injectProxy( session, 
request.getPluginArtifactRepositories() );
 mavenRepositorySystem.injectAuthentication( session, 
request.getPluginArtifactRepositories() );
 
+if ( Boolean.getBoolean( "maven.experimental.buildconsumer" ) )
 
 Review comment:
   No.
   I feel that usually we don't have much string constants in Maven.
   I see the reasons.
   Let's not open this case by now


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] eolivelli commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2019-10-06 Thread GitBox
eolivelli commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r331789710
 
 

 ##
 File path: 
maven-core/src/main/java/org/apache/maven/xml/filter/FastForwardFilter.java
 ##
 @@ -0,0 +1,128 @@
+package org.apache.maven.xml.filter;
+
+/*
+ * 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.
+ */
+
+import java.util.ArrayDeque;
+import java.util.Deque;
+
+import org.xml.sax.Attributes;
+import org.xml.sax.ContentHandler;
+import org.xml.sax.SAXException;
+import org.xml.sax.XMLFilter;
+import org.xml.sax.XMLReader;
+import org.xml.sax.helpers.XMLFilterImpl;
+
+/**
+ * This filter will skip all following filters and write directly to the 
output.
+ * Should be used in case of a DOM that should not be effected by other 
filters, even though the elements match 
+ * 
+ * @author Robert Scholte
+ * @since 4.0.0
+ */
+class FastForwardFilter extends XMLFilterImpl
+{
+/**
+ * DOM elements of pom
+ * 
+ * 
+ *  execution.configuration
+ *  plugin.configuration
+ *  plugin.goals
+ *  profile.reports
+ *  project.reports
+ *  reportSet.configuration
+ * 
+ */
+private final Deque state = new ArrayDeque<>();
+
+private int domDepth = 0;
+
+private ContentHandler originalHandler;
+
+FastForwardFilter()
+{
+super();
+}
+
+FastForwardFilter( XMLReader parent )
+{
+super( parent );
+}
+
+@Override
+public void startElement( String uri, String localName, String qName, 
Attributes atts )
+throws SAXException
+{
+super.startElement( uri, localName, qName, atts );
+if ( domDepth > 0 )
 
 Review comment:
   Ok


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] eolivelli commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2019-10-01 Thread GitBox
eolivelli commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r330304611
 
 

 ##
 File path: 
maven-xml/src/test/java/org/apache/maven/xml/filter/AbstractXMLFilterTests.java
 ##
 @@ -0,0 +1,95 @@
+package org.apache.maven.xml.filter;
+
+/*
+ * 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.
+ */
+
+import java.io.Reader;
+import java.io.StringReader;
+import java.io.StringWriter;
+import java.io.Writer;
+
+import javax.xml.parsers.ParserConfigurationException;
+import javax.xml.parsers.SAXParserFactory;
+import javax.xml.transform.OutputKeys;
+import javax.xml.transform.Transformer;
+import javax.xml.transform.TransformerException;
+import javax.xml.transform.TransformerFactory;
+import javax.xml.transform.sax.SAXSource;
+import javax.xml.transform.stream.StreamResult;
+
+import org.xml.sax.InputSource;
+import org.xml.sax.SAXException;
+import org.xml.sax.XMLFilter;
+
+public abstract class AbstractXMLFilterTests
+{
+public AbstractXMLFilterTests()
+{
+super();
+}
+
+protected abstract XMLFilter getFilter() throws TransformerException, 
SAXException, ParserConfigurationException;
+
+private void setParent( XMLFilter filter ) throws SAXException, 
ParserConfigurationException
+{
+if( filter.getParent() == null )
+{
+filter.setParent( 
SAXParserFactory.newInstance().newSAXParser().getXMLReader() );
+filter.setFeature( "http://xml.org/sax/features/namespaces;, true 
);
+}
+}
+
+protected String transform( String input )
+throws TransformerException, SAXException, ParserConfigurationException
+{
+return transform( new StringReader( input ) );
+}
+
+protected String transform( Reader input ) throws TransformerException, 
SAXException, ParserConfigurationException
+{
+XMLFilter filter = getFilter();
+setParent( filter );
+return transform( input, filter );
+}
+
+protected String transform( String input, XMLFilter filter ) 
+throws TransformerException, SAXException, ParserConfigurationException
+{
+setParent( filter );
+return transform( new StringReader( input ), filter );
+}
+
+protected String transform( Reader input, XMLFilter filter )
+throws TransformerException, SAXException, ParserConfigurationException
+{
+
+Writer writer = new StringWriter();
+StreamResult result = new StreamResult( writer );
+
+TransformerFactory transformerFactory = 
TransformerFactory.newInstance();
 
 Review comment:
   We should have some common factory for XML stuff, in order to ensure a 
consistent configuration.
   IIRC some objects are reusable, this will save memory


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] eolivelli commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2019-10-01 Thread GitBox
eolivelli commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r330303476
 
 

 ##
 File path: maven-xml/src/main/java/org/apache/maven/xml/SAXEventFactory.java
 ##
 @@ -0,0 +1,111 @@
+package org.apache.maven.xml;
+
+/*
+ * 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.
+ */
+
+import org.xml.sax.Attributes;
+import org.xml.sax.ContentHandler;
+import org.xml.sax.Locator;
+
+/**
+ * Factory for SAXEvents
+ * 
+ * @author Robert Scholte
+ * @since 4.0.0
 
 Review comment:
   3.7.0


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] eolivelli commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2019-10-01 Thread GitBox
eolivelli commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r330296938
 
 

 ##
 File path: 
maven-core/src/main/java/org/apache/maven/xml/filter/FastForwardFilter.java
 ##
 @@ -0,0 +1,128 @@
+package org.apache.maven.xml.filter;
+
+/*
+ * 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.
+ */
+
+import java.util.ArrayDeque;
+import java.util.Deque;
+
+import org.xml.sax.Attributes;
+import org.xml.sax.ContentHandler;
+import org.xml.sax.SAXException;
+import org.xml.sax.XMLFilter;
+import org.xml.sax.XMLReader;
+import org.xml.sax.helpers.XMLFilterImpl;
+
+/**
+ * This filter will skip all following filters and write directly to the 
output.
+ * Should be used in case of a DOM that should not be effected by other 
filters, even though the elements match 
+ * 
+ * @author Robert Scholte
+ * @since 4.0.0
 
 Review comment:
   3.7.0?


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] eolivelli commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2019-10-01 Thread GitBox
eolivelli commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r330296720
 
 

 ##
 File path: 
maven-core/src/main/java/org/apache/maven/xml/filter/ConsumerPomXMLFilter.java
 ##
 @@ -0,0 +1,73 @@
+package org.apache.maven.xml.filter;
+
+/*
+ * 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.
+ */
+
+import javax.xml.parsers.ParserConfigurationException;
+import javax.xml.parsers.SAXParserFactory;
+
+import org.xml.sax.SAXException;
+import org.xml.sax.XMLFilter;
+import org.xml.sax.XMLReader;
+
+import org.xml.sax.helpers.XMLFilterImpl;
+
+/**
+ * XML Filter to transform pom.xml to consumer pom.
+ * This often means stripping of build-specific information.
+ * 
+ * This filter is used at 2 locations:
+ * - {@link 
org.apache.maven.internal.aether.DefaultRepositorySystemSessionFactory} when 
publishing pom files.
+ * - TODO ???Class when a reactor module is used as dependency. This ensures 
consistency of dependency handling
 
 Review comment:
   should this todo  be linked to some JIRA?


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] eolivelli commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2019-10-01 Thread GitBox
eolivelli commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r330296309
 
 

 ##
 File path: 
maven-core/src/main/java/org/apache/maven/internal/aether/DefaultRepositorySystemSessionFactory.java
 ##
 @@ -238,9 +263,83 @@ else if ( request.isUpdateSnapshots() )
 mavenRepositorySystem.injectProxy( session, 
request.getPluginArtifactRepositories() );
 mavenRepositorySystem.injectAuthentication( session, 
request.getPluginArtifactRepositories() );
 
+if ( Boolean.getBoolean( "maven.experimental.buildconsumer" ) )
+{
+session.setFileTransformerManager( newFileTransformerManager() );
+}
 return session;
 }
 
+private FileTransformerManager newFileTransformerManager()
+{
+return new FileTransformerManager()
+{
+@Override
+public Collection getTransformersForArtifact( 
final Artifact artifact )
+{
+Collection transformers = new ArrayList<>();
+if ( "pom".equals( artifact.getExtension() ) )
+{
+final TransformerFactory transformerFactory = 
TransformerFactory.newInstance();
+
+transformers.add( new FileTransformer()
+{
+@Override
+public InputStream transformData( File file )
+throws IOException, TransformException
+{
+final PipedOutputStream pipedOutputStream  = new 
PipedOutputStream();
+final PipedInputStream pipedInputStream  = new 
PipedInputStream( pipedOutputStream );
+
+final SAXSource transformSource;
+try
+{
+transformSource =
+new SAXSource( 
consumerPomXMLFilterFactory.get().get( file.toPath() ),
+   new InputSource( new 
FileReader( file ) ) );
+}
+catch ( SAXException | 
ParserConfigurationException e )
+{   
+e.printStackTrace();
+throw new TransformException( "Failed to 
create a consumerPomXMLFilter", e );
+}
+
+final StreamResult result = new StreamResult( 
pipedOutputStream );
+
+final Runnable runnable = new Runnable()
+{
+@Override
+public void run()
+{
+try ( PipedOutputStream out = 
pipedOutputStream )
+{
+
transformerFactory.newTransformer().transform( transformSource, result );
+}
+catch ( TransformerException | IOException 
e )
+{
+e.printStackTrace();
+throw new RuntimeException( e );
+}
+}
+};
+
+new Thread( runnable ).start();
 
 Review comment:
   maybe it is better to give a name to this thread and consider make it a 
daemon thread.
   I don't know if it is worth to use a thread pool, in order to save the 
creation/destruction of a thread for each pom to transform


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] eolivelli commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2019-10-01 Thread GitBox
eolivelli commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r330297864
 
 

 ##
 File path: 
maven-core/src/main/java/org/apache/maven/xml/internal/DefaultBuildPomXMLFilterFactory.java
 ##
 @@ -0,0 +1,117 @@
+package org.apache.maven.xml.internal;
+
+/*
+ * 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.
+ */
+
+
+import java.nio.file.Path;
+import java.util.Optional;
+import java.util.function.Function;
+
+import javax.inject.Inject;
+import javax.inject.Named;
+import javax.inject.Singleton;
+
+import org.apache.maven.execution.MavenSession;
+import org.apache.maven.model.Model;
+import org.apache.maven.model.building.ModelCacheManager;
+import org.apache.maven.xml.filter.BuildPomXMLFilterFactory;
+import org.apache.maven.xml.filter.DependencyKey;
+import org.apache.maven.xml.filter.RelativeProject;
+
+/**
+ * 
+ * @author Robert Scholte
+ * @since 3.7.0
+ */
+@Named
+@Singleton
+public class DefaultBuildPomXMLFilterFactory extends BuildPomXMLFilterFactory
+{
+private MavenSession session;
+
+@Inject
+private ModelCacheManager rawModelCache; 
+
+@Inject
+public DefaultBuildPomXMLFilterFactory( MavenSession session )
+{
+this.session = session;
+}
+
+@Override
+protected Optional getChangelist()
+{
+return Optional.ofNullable( session.getUserProperties().getProperty( 
"changelist" ) );
+}
+
+@Override
+protected Optional getRevision()
+{
+return Optional.ofNullable( session.getUserProperties().getProperty( 
"revision" ) );
+}
+
+@Override
+protected Optional getSha1()
+{
+return Optional.ofNullable( session.getUserProperties().getProperty( 
"sha1" ) );
+}
+
+@Override
+protected Function> getRelativePathMapper()
+{
+return p -> Optional.ofNullable( rawModelCache.get( p ) ).map( m -> 
toRelativeProject( m ) );
+}
+
+@Override
+protected Function getDependencyKeyToVersionMapper()
+{
+return k -> Optional.ofNullable( rawModelCache.get( k ) )
+.map( m -> toVersion( m ) )
+.orElse( null );
+}
+
+private RelativeProject toRelativeProject( final Model m )
 
 Review comment:
   static?


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] eolivelli commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2019-10-01 Thread GitBox
eolivelli commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r330298724
 
 

 ##
 File path: maven-xml/src/main/java/org/apache/maven/xml/SAXEventUtils.java
 ##
 @@ -0,0 +1,38 @@
+package org.apache.maven.xml;
+
+/*
+ * 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.
+ */
+
+/**
+ * Utility class for SAXEvents
+ * 
+ * @author Robert Scholte
+ * @since 4.0.0
 
 Review comment:
   3.7.0


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] eolivelli commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2019-10-01 Thread GitBox
eolivelli commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r330297691
 
 

 ##
 File path: 
maven-core/src/main/java/org/apache/maven/xml/internal/DefaultBuildPomXMLFilterFactory.java
 ##
 @@ -0,0 +1,117 @@
+package org.apache.maven.xml.internal;
+
+/*
+ * 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.
+ */
+
+
+import java.nio.file.Path;
+import java.util.Optional;
+import java.util.function.Function;
+
+import javax.inject.Inject;
+import javax.inject.Named;
+import javax.inject.Singleton;
+
+import org.apache.maven.execution.MavenSession;
+import org.apache.maven.model.Model;
+import org.apache.maven.model.building.ModelCacheManager;
+import org.apache.maven.xml.filter.BuildPomXMLFilterFactory;
+import org.apache.maven.xml.filter.DependencyKey;
+import org.apache.maven.xml.filter.RelativeProject;
+
+/**
+ * 
+ * @author Robert Scholte
+ * @since 3.7.0
+ */
+@Named
+@Singleton
+public class DefaultBuildPomXMLFilterFactory extends BuildPomXMLFilterFactory
+{
+private MavenSession session;
+
+@Inject
+private ModelCacheManager rawModelCache; 
+
+@Inject
+public DefaultBuildPomXMLFilterFactory( MavenSession session )
+{
+this.session = session;
+}
+
+@Override
+protected Optional getChangelist()
+{
+return Optional.ofNullable( session.getUserProperties().getProperty( 
"changelist" ) );
+}
+
+@Override
+protected Optional getRevision()
+{
+return Optional.ofNullable( session.getUserProperties().getProperty( 
"revision" ) );
+}
+
+@Override
+protected Optional getSha1()
+{
+return Optional.ofNullable( session.getUserProperties().getProperty( 
"sha1" ) );
+}
+
+@Override
+protected Function> getRelativePathMapper()
+{
+return p -> Optional.ofNullable( rawModelCache.get( p ) ).map( m -> 
toRelativeProject( m ) );
+}
+
+@Override
+protected Function getDependencyKeyToVersionMapper()
+{
+return k -> Optional.ofNullable( rawModelCache.get( k ) )
+.map( m -> toVersion( m ) )
+.orElse( null );
+}
+
+private RelativeProject toRelativeProject( final Model m )
+{
+String groupId = m.getGroupId();
+if ( groupId == null && m.getParent() != null )
+{
+groupId = m.getParent().getGroupId();
+}
+
+String version = m.getVersion();
+if ( version == null && m.getParent() != null )
+{
+version = m.getParent().getVersion();
+}
+
+return new RelativeProject( groupId, m.getArtifactId(), version );
+}
+
+private String toVersion( final Model m )
 
 Review comment:
   static?


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] eolivelli commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2019-10-01 Thread GitBox
eolivelli commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r330297344
 
 

 ##
 File path: 
maven-core/src/main/java/org/apache/maven/xml/filter/FastForwardFilter.java
 ##
 @@ -0,0 +1,128 @@
+package org.apache.maven.xml.filter;
+
+/*
+ * 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.
+ */
+
+import java.util.ArrayDeque;
+import java.util.Deque;
+
+import org.xml.sax.Attributes;
+import org.xml.sax.ContentHandler;
+import org.xml.sax.SAXException;
+import org.xml.sax.XMLFilter;
+import org.xml.sax.XMLReader;
+import org.xml.sax.helpers.XMLFilterImpl;
+
+/**
+ * This filter will skip all following filters and write directly to the 
output.
+ * Should be used in case of a DOM that should not be effected by other 
filters, even though the elements match 
+ * 
+ * @author Robert Scholte
+ * @since 4.0.0
+ */
+class FastForwardFilter extends XMLFilterImpl
+{
+/**
+ * DOM elements of pom
+ * 
+ * 
+ *  execution.configuration
+ *  plugin.configuration
+ *  plugin.goals
+ *  profile.reports
+ *  project.reports
+ *  reportSet.configuration
+ * 
+ */
+private final Deque state = new ArrayDeque<>();
+
+private int domDepth = 0;
+
+private ContentHandler originalHandler;
+
+FastForwardFilter()
+{
+super();
+}
+
+FastForwardFilter( XMLReader parent )
+{
+super( parent );
+}
+
+@Override
+public void startElement( String uri, String localName, String qName, 
Attributes atts )
+throws SAXException
+{
+super.startElement( uri, localName, qName, atts );
+if ( domDepth > 0 )
 
 Review comment:
   This is somehow tricky, but in the end it works


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] eolivelli commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2019-10-01 Thread GitBox
eolivelli commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r330298491
 
 

 ##
 File path: 
maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java
 ##
 @@ -733,12 +758,81 @@ private void checkPluginVersions( List 
lineage, ModelBuildingRequest
 private void assembleInheritance( List lineage, 
ModelBuildingRequest request,
   ModelProblemCollector problems )
 {
-for ( int i = lineage.size() - 2; i >= 0; i-- )
+for ( int i = lineage.size() - 2; i >= 1; i-- )
 {
 Model parent = lineage.get( i + 1 ).getModel();
 Model child = lineage.get( i ).getModel();
 inheritanceAssembler.assembleModelInheritance( child, parent, 
request, problems );
 }
+
+// re-read model from file
+if ( Boolean.getBoolean( "maven.experimental.buildconsumer" ) && 
request.isTransformPom() )
+{
+try
+{
+// TODO: parent might be part of reactor... better read all 
lineage items like this?
+Model parent = lineage.get( 1 ).getModel();
+
+Model child = modelProcessor.read( transformData( lineage.get( 
0 ) ), null );
+inheritanceAssembler.assembleModelInheritance( child, parent, 
request, problems );
+
+// sync pomfile, is transient
+child.setPomFile( lineage.get( 0 ).getModel().getPomFile() );
+// overwrite child
+lineage.get( 0 ).setModel( child );
+}
+catch ( IOException | TransformException | SAXException | 
ParserConfigurationException e )
+{
+// this is second read, should not happen
+e.printStackTrace();
 
 Review comment:
   use logger


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] eolivelli commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2019-10-01 Thread GitBox
eolivelli commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r330295497
 
 

 ##
 File path: 
maven-core/src/main/java/org/apache/maven/internal/aether/DefaultRepositorySystemSessionFactory.java
 ##
 @@ -238,9 +263,83 @@ else if ( request.isUpdateSnapshots() )
 mavenRepositorySystem.injectProxy( session, 
request.getPluginArtifactRepositories() );
 mavenRepositorySystem.injectAuthentication( session, 
request.getPluginArtifactRepositories() );
 
+if ( Boolean.getBoolean( "maven.experimental.buildconsumer" ) )
+{
+session.setFileTransformerManager( newFileTransformerManager() );
+}
 return session;
 }
 
+private FileTransformerManager newFileTransformerManager()
+{
+return new FileTransformerManager()
+{
+@Override
+public Collection getTransformersForArtifact( 
final Artifact artifact )
+{
+Collection transformers = new ArrayList<>();
+if ( "pom".equals( artifact.getExtension() ) )
+{
+final TransformerFactory transformerFactory = 
TransformerFactory.newInstance();
 
 Review comment:
   This shuld deserve a separate discussion, but aren't we following OWASP 
security guidelines for parsing XML potentially downloaded from remote sources? 
   Stuff like 
https://github.com/OWASP/CheatSheetSeries/blob/master/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.md


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [maven] eolivelli commented on a change in pull request #286: [MNG-6656] Introduce base for build/consumer process

2019-10-01 Thread GitBox
eolivelli commented on a change in pull request #286: [MNG-6656] Introduce base 
for build/consumer process
URL: https://github.com/apache/maven/pull/286#discussion_r330293977
 
 

 ##
 File path: 
maven-core/src/main/java/org/apache/maven/internal/aether/DefaultRepositorySystemSessionFactory.java
 ##
 @@ -238,9 +263,83 @@ else if ( request.isUpdateSnapshots() )
 mavenRepositorySystem.injectProxy( session, 
request.getPluginArtifactRepositories() );
 mavenRepositorySystem.injectAuthentication( session, 
request.getPluginArtifactRepositories() );
 
+if ( Boolean.getBoolean( "maven.experimental.buildconsumer" ) )
 
 Review comment:
   Can we have a constant for this property name?


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services