[GitHub] [maven] mthmulders commented on a diff in pull request #744: [MNG-7360] Fix xml transformation to ensure proper context

[GitBox <gi...@apache.org>]

mthmulders commented on code in PR #744:
URL: https://github.com/apache/maven/pull/744#discussion_r875647056


##########
maven-model-transform/src/test/java/org/apache/maven/model/transform/ParentXMLFilterTest.java:
##########
@@ -33,30 +33,55 @@
 public class ParentXMLFilterTest
     extends AbstractXMLFilterTests
 {
-    private Function<XmlPullParser, ParentXMLFilter> filterCreator;
+    private Function<XmlPullParser, XmlPullParser> filterCreator;
 
     @BeforeEach
     void reset() {
         filterCreator = null;
     }
 
     @Override
-    protected ParentXMLFilter getFilter( XmlPullParser parser )
+    protected XmlPullParser getFilter( XmlPullParser parser )
     {
-        Function<XmlPullParser, ParentXMLFilter> filterCreator =
+        Function<XmlPullParser, XmlPullParser> filterCreator =
             (this.filterCreator != null ? this.filterCreator : this::createFilter);
         return filterCreator.apply(parser);
     }
 
-    protected ParentXMLFilter createFilter( XmlPullParser parser ) {
+    protected XmlPullParser createFilter( XmlPullParser parser ) {
         return createFilter( parser,
                 x -> Optional.of(new RelativeProject("GROUPID", "ARTIFACTID", "1.0.0")),
                 Paths.get( "pom.xml").toAbsolutePath() );
     }
 
-    protected ParentXMLFilter createFilter( XmlPullParser parser, Function<Path, Optional<RelativeProject>> pathMapper, Path projectPath ) {
+    protected XmlPullParser createFilter( XmlPullParser parser, Function<Path, Optional<RelativeProject>> pathMapper, Path projectPath ) {
         ParentXMLFilter filter = new ParentXMLFilter( parser, pathMapper, projectPath );
-        return filter;
+        return new FastForwardFilter( filter );
+    }
+
+    @Test
+    public void testWithFastForward()

Review Comment:
   Just to check: would it make sense to also add a test for the opposite case, i.e. where the ParentXMLFilter is _not_ supposed to bypass other filters?



##########
maven-model-transform/src/main/java/org/apache/maven/model/transform/pull/BufferingParser.java:
##########
@@ -430,7 +430,7 @@ public int nextToken() throws XmlPullParserException, IOException
                 throw new XmlPullParserException( "already reached end of XML input", this, null );
             }
             int currentEvent = xmlPullParser.nextToken();
-            if ( accept() )
+            if ( disabled || accept() )

Review Comment:
   I like `bypass` - and I'd like it even more if the variable name would be in the same terminology as the class Javadoc. So maybe we should rephrase the Javadoc a bit and say something like "This filter will bypass all following filters and write directly to the output."



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

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