You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2022/06/08 07:35:03 UTC

[GitHub] [maven-common-artifact-filters] cstamas opened a new pull request, #26: [MASSEMBLY-955] Bugfix for classifier in pattern

cstamas opened a new pull request, #26:
URL: https://github.com/apache/maven-common-artifact-filters/pull/26

   Parsing of pattern with classifier was busted, that
   prevented use cases like before rewrite, that was again
   busted due early return (not happening since rewrite).
   
   ---
   
   https://issues.apache.org/jira/browse/MASSEMBLY-955


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


[GitHub] [maven-common-artifact-filters] michael-o commented on a diff in pull request #26: [MSHARED-1077] Bugfix for classifier in pattern

Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #26:
URL: https://github.com/apache/maven-common-artifact-filters/pull/26#discussion_r895008393


##########
src/main/java/org/apache/maven/shared/artifact/filter/PatternIncludesArtifactFilter.java:
##########
@@ -255,540 +183,319 @@ public void reportMissedCriteria( final Logger logger )
 
                 for ( Pattern pattern : missed )
                 {
-                    buffer.append( "\no  '" ).append( pattern ).append( "'" );
+                    buffer.append( SEP ) .append( "o  '" ).append( pattern ).append( "'" );
                 }
 
-                buffer.append( "\n" );
+                buffer.append( SEP );
 
                 logger.warn( buffer.toString() );
             }
         }
     }
 
-    /** {@inheritDoc} */
     @Override
     public String toString()
     {
         return "Includes filter:" + getPatternsAsString();
     }
 
-    /**
-     * <p>getPatternsAsString.</p>
-     *
-     * @return pattern as a string.
-     */
     protected String getPatternsAsString()
     {
         final StringBuilder buffer = new StringBuilder();
         for ( Pattern pattern : patterns )
         {
-            buffer.append( "\no '" ).append( pattern ).append( "'" );
+            buffer.append( SEP ).append( "o '" ).append( pattern ).append( "'" );
         }
 
         return buffer.toString();
     }
 
-    /**
-     * <p>getFilterDescription.</p>
-     *
-     * @return description.
-     */
     protected String getFilterDescription()
     {
         return "artifact inclusion filter";
     }
 
-    /** {@inheritDoc} */
+    @Override
     public void reportFilteredArtifacts( final Logger logger )
     {
         if ( !filteredArtifact.isEmpty() && logger.isDebugEnabled() )
         {
-            final StringBuilder buffer =
-                new StringBuilder( "The following artifacts were removed by this " + getFilterDescription() + ": " );
+            final StringBuilder buffer = new StringBuilder(
+                    "The following artifacts were removed by this " + getFilterDescription() + ": " );
 
             for ( Artifact artifactId : filteredArtifact )
             {
-                buffer.append( '\n' ).append( artifactId.getId() );
+                buffer.append( SEP ).append( artifactId.getId() );
             }
 
             logger.debug( buffer.toString() );
         }
     }
 
-    /**
-     * {@inheritDoc}
-     *
-     * @return a boolean.
-     */
+    @Override
     public boolean hasMissedCriteria()
     {
         // if there are no patterns, there is nothing to report.
         if ( !patterns.isEmpty() )
         {
             final List<Pattern> missed = new ArrayList<>( patterns );
             missed.removeAll( patternsTriggered );
-
             return !missed.isEmpty();
         }
 
         return false;
     }
 
-    private static final char[] EMPTY = new char[0];
-
-    private static final char[] ANY = new char[] { '*' };
-
-    static char[] emptyOrChars( String str )
+    private enum Coordinate
     {
-        return str != null && str.length() > 0 ? str.toCharArray() : EMPTY;
+        GROUP_ID, ARTIFACT_ID, TYPE, CLASSIFIER, BASE_VERSION
     }
 
-    static char[] anyOrChars( char[] str )
+    private interface Artifactoid
     {
-        return str.length > 1 || ( str.length == 1 && str[0] != '*' ) ? str : ANY;
-    }
-
-    static char[][] tokenizeAndSplit( String pattern )
-    {
-        String[] stokens = pattern.split( ":" );
-        char[][] tokens = new char[ stokens.length ][];
-        for ( int i = 0; i < stokens.length; i++ )
-        {
-            tokens[i] = emptyOrChars( stokens[i] );
-        }
-        return tokens;
+        String getCoordinate( Coordinate coordinate );
     }
 
-    @SuppressWarnings( "InnerAssignment" )
-    static boolean match( char[] patArr, char[] strArr, boolean isVersion )
+    private static Artifactoid adapt( final Artifact artifact )
     {
-        int patIdxStart = 0;
-        int patIdxEnd = patArr.length - 1;
-        int strIdxStart = 0;
-        int strIdxEnd = strArr.length - 1;
-        char ch;
-
-        boolean containsStar = false;
-        for ( char aPatArr : patArr )
-        {
-            if ( aPatArr == '*' )
-            {
-                containsStar = true;
-                break;
-            }
-        }
-
-        if ( !containsStar )
-        {
-            if ( isVersion && ( patArr[0] == '[' || patArr[0] == '(' ) )
-            {
-                return isVersionIncludedInRange( String.valueOf( strArr ), String.valueOf( patArr ) );
-            }
-            // No '*'s, so we make a shortcut
-            if ( patIdxEnd != strIdxEnd )
-            {
-                return false; // Pattern and string do not have the same size
-            }
-            for ( int i = 0; i <= patIdxEnd; i++ )
-            {
-                ch = patArr[i];
-                if ( ch != '?' && ch != strArr[i] )
-                {
-                    return false; // Character mismatch
-                }
-            }
-            return true; // String matches against pattern
-        }
-
-        if ( patIdxEnd == 0 )
-        {
-            return true; // Pattern contains only '*', which matches anything
-        }
-
-        // Process characters before first star
-        while ( ( ch = patArr[patIdxStart] ) != '*' && strIdxStart <= strIdxEnd )
-        {
-            if ( ch != '?' && ch != strArr[strIdxStart] )
-            {
-                return false; // Character mismatch
-            }
-            patIdxStart++;
-            strIdxStart++;
-        }
-        if ( strIdxStart > strIdxEnd )
+        requireNonNull( artifact );
+        return coordinate ->
         {
-            // All characters in the string are used. Check if only '*'s are
-            // left in the pattern. If so, we succeeded. Otherwise failure.
-            for ( int i = patIdxStart; i <= patIdxEnd; i++ )
+            requireNonNull( coordinate );
+            switch ( coordinate )
             {
-                if ( patArr[i] != '*' )
-                {
-                    return false;
-                }
+                case GROUP_ID:
+                    return artifact.getGroupId();
+                case ARTIFACT_ID:
+                    return artifact.getArtifactId();
+                case BASE_VERSION:
+                    return artifact.getBaseVersion();
+                case CLASSIFIER:
+                    return artifact.hasClassifier() ? artifact.getClassifier() : null;
+                case TYPE:
+                    return artifact.getType();
+                default:
             }
-            return true;
-        }
+            throw new IllegalArgumentException( "unknown coordinate: " + coordinate );
+        };
+    }
 
-        // Process characters after last star
-        while ( ( ch = patArr[patIdxEnd] ) != '*' && strIdxStart <= strIdxEnd )
+    private static Artifactoid adapt( final String depTrailString )
+    {
+        requireNonNull( depTrailString );
+        // G:A:T:C:V
+        String[] coordinates = depTrailString.split( ":" );
+        if ( coordinates.length != 4 && coordinates.length != 5 )
         {
-            if ( ch != '?' && ch != strArr[strIdxEnd] )
-            {
-                return false; // Character mismatch
-            }
-            patIdxEnd--;
-            strIdxEnd--;
+            throw new IllegalArgumentException( "Bad dep trail string: " + depTrailString );
         }
-        if ( strIdxStart > strIdxEnd )
+        final HashMap<Coordinate, String> map = new HashMap<>();
+        map.put( Coordinate.GROUP_ID, coordinates[0] );
+        map.put( Coordinate.ARTIFACT_ID, coordinates[1] );
+        map.put( Coordinate.TYPE, coordinates[2] );
+        if ( coordinates.length == 5 )
         {
-            // All characters in the string are used. Check if only '*'s are
-            // left in the pattern. If so, we succeeded. Otherwise failure.
-            for ( int i = patIdxStart; i <= patIdxEnd; i++ )
-            {
-                if ( patArr[i] != '*' )
-                {
-                    return false;
-                }
-            }
-            return true;
+            map.put( Coordinate.CLASSIFIER, coordinates[3] );
+            map.put( Coordinate.BASE_VERSION, coordinates[4] );
         }
-
-        // process pattern between stars. padIdxStart and patIdxEnd point
-        // always to a '*'.
-        while ( patIdxStart != patIdxEnd && strIdxStart <= strIdxEnd )
+        else
         {
-            int patIdxTmp = -1;
-            for ( int i = patIdxStart + 1; i <= patIdxEnd; i++ )
-            {
-                if ( patArr[i] == '*' )
-                {
-                    patIdxTmp = i;
-                    break;
-                }
-            }
-            if ( patIdxTmp == patIdxStart + 1 )
-            {
-                // Two stars next to each other, skip the first one.
-                patIdxStart++;
-                continue;
-            }
-            // Find the pattern between padIdxStart & padIdxTmp in str between
-            // strIdxStart & strIdxEnd
-            int patLength = ( patIdxTmp - patIdxStart - 1 );
-            int strLength = ( strIdxEnd - strIdxStart + 1 );
-            int foundIdx = -1;
-            strLoop: for ( int i = 0; i <= strLength - patLength; i++ )
-            {
-                for ( int j = 0; j < patLength; j++ )
-                {
-                    ch = patArr[patIdxStart + j + 1];
-                    if ( ch != '?' && ch != strArr[strIdxStart + i + j] )
-                    {
-                        continue strLoop;
-                    }
-                }
-
-                foundIdx = strIdxStart + i;
-                break;
-            }
-
-            if ( foundIdx == -1 )
-            {
-                return false;
-            }
-
-            patIdxStart = patIdxTmp;
-            strIdxStart = foundIdx + patLength;
+            map.put( Coordinate.BASE_VERSION, coordinates[3] );
         }
 
-        // All characters in the string are used. Check if only '*'s are left
-        // in the pattern. If so, we succeeded. Otherwise failure.
-        for ( int i = patIdxStart; i <= patIdxEnd; i++ )
+        return coordinate ->
         {
-            if ( patArr[i] != '*' )
-            {
-                return false;
-            }
-        }
-        return true;
+            requireNonNull( coordinate );
+            return map.get( coordinate );
+        };
     }
 
-    static boolean isVersionIncludedInRange( final String version, final String range )
+    private static final String ANY = "*";
+
+    /**
+     * Splits the pattern string into tokens, replacing empty tokens with {@link #ANY} for patterns like {@code ::val}
+     * so it retains the position of token.
+     */
+    private static String[] splitAndTokenize( String pattern )
     {
-        try
+        String[] stokens = pattern.split( ":" );
+        String[] tokens = new String[stokens.length];
+        for ( int i = 0; i < stokens.length; i++ )
         {
-            return VersionRange.createFromVersionSpec( range ).containsVersion( new DefaultArtifactVersion( version ) );
-        }
-        catch ( final InvalidVersionSpecificationException e )
-        {
-            return false;
+            String str = stokens[i];
+            tokens[i] = str != null && !str.isEmpty() ? str : ANY;
         }
+        return tokens;
     }
 
-    static Pattern compile( String pattern )
+    private static Pattern compile( String pattern )
     {
         if ( pattern.startsWith( "!" ) )
         {
             return new NegativePattern( pattern, compile( pattern.substring( 1 ) ) );
         }
         else
         {
-            char[][] stokens = tokenizeAndSplit( pattern );
-            char[][] tokens = new char[ stokens.length ][];
-            for ( int i = 0; i < stokens.length; i++ )
-            {
-                tokens[i] = anyOrChars( stokens[i] );
-            }
-            if ( tokens.length > 5 )
+            String[] tokens = splitAndTokenize( pattern );
+            if ( tokens.length < 1 || tokens.length > 5 )
             {
                 throw new IllegalArgumentException( "Invalid pattern: " + pattern );
             }
-            // we only accept 5 tokens if the classifier = '*'
+
+            ArrayList<Pattern> patterns = new ArrayList<>( 5 );
+
             if ( tokens.length == 5 )
             {
-                if ( tokens[3] != ANY )
+                // trivial, full pattern w/ classifier: G:A:T:C:V
+                patterns.add( toPattern( tokens[0], Coordinate.GROUP_ID ) );
+                patterns.add( toPattern( tokens[1], Coordinate.ARTIFACT_ID ) );
+                patterns.add( toPattern( tokens[2], Coordinate.TYPE ) );
+                patterns.add( toPattern( tokens[3], Coordinate.CLASSIFIER ) );
+                patterns.add( toPattern( tokens[4], Coordinate.BASE_VERSION ) );
+            }
+            else if ( tokens.length == 4 )
+            {
+                // trivial, full pattern w/o classifier: G:A:T:V
+                patterns.add( toPattern( tokens[0], Coordinate.GROUP_ID ) );
+                patterns.add( toPattern( tokens[1], Coordinate.ARTIFACT_ID ) );
+                patterns.add( toPattern( tokens[2], Coordinate.TYPE ) );
+                patterns.add( toPattern( tokens[3], Coordinate.BASE_VERSION ) );
+            }
+            else if ( tokens.length == 3 )
+            {
+                // tricky: may be "*:artifact:*" but also "*:war:*"
+
+                // *:*:* -> ALL
+                // *:*:xxx -> TC(xxx)
+                // *:xxx:* -> AT(xxx)
+                // *:xxx:yyy -> GA(xxx) + TC(XXX)
+                // xxx:*:* -> GA(xxx)
+                // xxx:*:yyy -> G(xxx) + TC(yyy)
+                // xxx:yyy:* -> G(xxx)+A(yyy)
+                // xxx:yyy:zzz -> G(xxx)+A(yyy)+T(zzz)
+                if ( ANY.equals( tokens[0] ) && ANY.equals( tokens[1] ) && ANY.equals( tokens[2] ) )

Review Comment:
   Alright, agreed. Please put a comment above that those patterns are in question since they aren't documented. An open TODO.



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


[GitHub] [maven-common-artifact-filters] michael-o commented on a diff in pull request #26: [MSHARED-1077] Bugfix for classifier in pattern

Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #26:
URL: https://github.com/apache/maven-common-artifact-filters/pull/26#discussion_r895005394


##########
src/main/java/org/apache/maven/shared/artifact/filter/PatternIncludesArtifactFilter.java:
##########
@@ -255,540 +183,319 @@ public void reportMissedCriteria( final Logger logger )
 
                 for ( Pattern pattern : missed )
                 {
-                    buffer.append( "\no  '" ).append( pattern ).append( "'" );
+                    buffer.append( SEP ) .append( "o  '" ).append( pattern ).append( "'" );
                 }
 
-                buffer.append( "\n" );
+                buffer.append( SEP );
 
                 logger.warn( buffer.toString() );
             }
         }
     }
 
-    /** {@inheritDoc} */
     @Override
     public String toString()
     {
         return "Includes filter:" + getPatternsAsString();
     }
 
-    /**
-     * <p>getPatternsAsString.</p>
-     *
-     * @return pattern as a string.
-     */
     protected String getPatternsAsString()
     {
         final StringBuilder buffer = new StringBuilder();
         for ( Pattern pattern : patterns )
         {
-            buffer.append( "\no '" ).append( pattern ).append( "'" );
+            buffer.append( SEP ).append( "o '" ).append( pattern ).append( "'" );
         }
 
         return buffer.toString();
     }
 
-    /**
-     * <p>getFilterDescription.</p>
-     *
-     * @return description.
-     */
     protected String getFilterDescription()
     {
         return "artifact inclusion filter";
     }
 
-    /** {@inheritDoc} */
+    @Override
     public void reportFilteredArtifacts( final Logger logger )
     {
         if ( !filteredArtifact.isEmpty() && logger.isDebugEnabled() )
         {
-            final StringBuilder buffer =
-                new StringBuilder( "The following artifacts were removed by this " + getFilterDescription() + ": " );
+            final StringBuilder buffer = new StringBuilder(
+                    "The following artifacts were removed by this " + getFilterDescription() + ": " );
 
             for ( Artifact artifactId : filteredArtifact )
             {
-                buffer.append( '\n' ).append( artifactId.getId() );
+                buffer.append( SEP ).append( artifactId.getId() );
             }
 
             logger.debug( buffer.toString() );
         }
     }
 
-    /**
-     * {@inheritDoc}
-     *
-     * @return a boolean.
-     */
+    @Override
     public boolean hasMissedCriteria()
     {
         // if there are no patterns, there is nothing to report.
         if ( !patterns.isEmpty() )
         {
             final List<Pattern> missed = new ArrayList<>( patterns );
             missed.removeAll( patternsTriggered );
-
             return !missed.isEmpty();
         }
 
         return false;
     }
 
-    private static final char[] EMPTY = new char[0];
-
-    private static final char[] ANY = new char[] { '*' };
-
-    static char[] emptyOrChars( String str )
+    private enum Coordinate
     {
-        return str != null && str.length() > 0 ? str.toCharArray() : EMPTY;
+        GROUP_ID, ARTIFACT_ID, TYPE, CLASSIFIER, BASE_VERSION
     }
 
-    static char[] anyOrChars( char[] str )
+    private interface Artifactoid
     {
-        return str.length > 1 || ( str.length == 1 && str[0] != '*' ) ? str : ANY;
-    }
-
-    static char[][] tokenizeAndSplit( String pattern )
-    {
-        String[] stokens = pattern.split( ":" );
-        char[][] tokens = new char[ stokens.length ][];
-        for ( int i = 0; i < stokens.length; i++ )
-        {
-            tokens[i] = emptyOrChars( stokens[i] );
-        }
-        return tokens;
+        String getCoordinate( Coordinate coordinate );
     }
 
-    @SuppressWarnings( "InnerAssignment" )
-    static boolean match( char[] patArr, char[] strArr, boolean isVersion )
+    private static Artifactoid adapt( final Artifact artifact )
     {
-        int patIdxStart = 0;
-        int patIdxEnd = patArr.length - 1;
-        int strIdxStart = 0;
-        int strIdxEnd = strArr.length - 1;
-        char ch;
-
-        boolean containsStar = false;
-        for ( char aPatArr : patArr )
-        {
-            if ( aPatArr == '*' )
-            {
-                containsStar = true;
-                break;
-            }
-        }
-
-        if ( !containsStar )
-        {
-            if ( isVersion && ( patArr[0] == '[' || patArr[0] == '(' ) )
-            {
-                return isVersionIncludedInRange( String.valueOf( strArr ), String.valueOf( patArr ) );
-            }
-            // No '*'s, so we make a shortcut
-            if ( patIdxEnd != strIdxEnd )
-            {
-                return false; // Pattern and string do not have the same size
-            }
-            for ( int i = 0; i <= patIdxEnd; i++ )
-            {
-                ch = patArr[i];
-                if ( ch != '?' && ch != strArr[i] )
-                {
-                    return false; // Character mismatch
-                }
-            }
-            return true; // String matches against pattern
-        }
-
-        if ( patIdxEnd == 0 )
-        {
-            return true; // Pattern contains only '*', which matches anything
-        }
-
-        // Process characters before first star
-        while ( ( ch = patArr[patIdxStart] ) != '*' && strIdxStart <= strIdxEnd )
-        {
-            if ( ch != '?' && ch != strArr[strIdxStart] )
-            {
-                return false; // Character mismatch
-            }
-            patIdxStart++;
-            strIdxStart++;
-        }
-        if ( strIdxStart > strIdxEnd )
+        requireNonNull( artifact );
+        return coordinate ->
         {
-            // All characters in the string are used. Check if only '*'s are
-            // left in the pattern. If so, we succeeded. Otherwise failure.
-            for ( int i = patIdxStart; i <= patIdxEnd; i++ )
+            requireNonNull( coordinate );
+            switch ( coordinate )
             {
-                if ( patArr[i] != '*' )
-                {
-                    return false;
-                }
+                case GROUP_ID:
+                    return artifact.getGroupId();
+                case ARTIFACT_ID:
+                    return artifact.getArtifactId();
+                case BASE_VERSION:
+                    return artifact.getBaseVersion();
+                case CLASSIFIER:
+                    return artifact.hasClassifier() ? artifact.getClassifier() : null;
+                case TYPE:
+                    return artifact.getType();
+                default:
             }
-            return true;
-        }
+            throw new IllegalArgumentException( "unknown coordinate: " + coordinate );
+        };
+    }
 
-        // Process characters after last star
-        while ( ( ch = patArr[patIdxEnd] ) != '*' && strIdxStart <= strIdxEnd )
+    private static Artifactoid adapt( final String depTrailString )
+    {
+        requireNonNull( depTrailString );
+        // G:A:T:C:V
+        String[] coordinates = depTrailString.split( ":" );
+        if ( coordinates.length != 4 && coordinates.length != 5 )
         {
-            if ( ch != '?' && ch != strArr[strIdxEnd] )
-            {
-                return false; // Character mismatch
-            }
-            patIdxEnd--;
-            strIdxEnd--;
+            throw new IllegalArgumentException( "Bad dep trail string: " + depTrailString );
         }
-        if ( strIdxStart > strIdxEnd )
+        final HashMap<Coordinate, String> map = new HashMap<>();
+        map.put( Coordinate.GROUP_ID, coordinates[0] );
+        map.put( Coordinate.ARTIFACT_ID, coordinates[1] );
+        map.put( Coordinate.TYPE, coordinates[2] );
+        if ( coordinates.length == 5 )
         {
-            // All characters in the string are used. Check if only '*'s are
-            // left in the pattern. If so, we succeeded. Otherwise failure.
-            for ( int i = patIdxStart; i <= patIdxEnd; i++ )
-            {
-                if ( patArr[i] != '*' )
-                {
-                    return false;
-                }
-            }
-            return true;
+            map.put( Coordinate.CLASSIFIER, coordinates[3] );
+            map.put( Coordinate.BASE_VERSION, coordinates[4] );
         }
-
-        // process pattern between stars. padIdxStart and patIdxEnd point
-        // always to a '*'.
-        while ( patIdxStart != patIdxEnd && strIdxStart <= strIdxEnd )
+        else
         {
-            int patIdxTmp = -1;
-            for ( int i = patIdxStart + 1; i <= patIdxEnd; i++ )
-            {
-                if ( patArr[i] == '*' )
-                {
-                    patIdxTmp = i;
-                    break;
-                }
-            }
-            if ( patIdxTmp == patIdxStart + 1 )
-            {
-                // Two stars next to each other, skip the first one.
-                patIdxStart++;
-                continue;
-            }
-            // Find the pattern between padIdxStart & padIdxTmp in str between
-            // strIdxStart & strIdxEnd
-            int patLength = ( patIdxTmp - patIdxStart - 1 );
-            int strLength = ( strIdxEnd - strIdxStart + 1 );
-            int foundIdx = -1;
-            strLoop: for ( int i = 0; i <= strLength - patLength; i++ )
-            {
-                for ( int j = 0; j < patLength; j++ )
-                {
-                    ch = patArr[patIdxStart + j + 1];
-                    if ( ch != '?' && ch != strArr[strIdxStart + i + j] )
-                    {
-                        continue strLoop;
-                    }
-                }
-
-                foundIdx = strIdxStart + i;
-                break;
-            }
-
-            if ( foundIdx == -1 )
-            {
-                return false;
-            }
-
-            patIdxStart = patIdxTmp;
-            strIdxStart = foundIdx + patLength;
+            map.put( Coordinate.BASE_VERSION, coordinates[3] );
         }
 
-        // All characters in the string are used. Check if only '*'s are left
-        // in the pattern. If so, we succeeded. Otherwise failure.
-        for ( int i = patIdxStart; i <= patIdxEnd; i++ )
+        return coordinate ->
         {
-            if ( patArr[i] != '*' )
-            {
-                return false;
-            }
-        }
-        return true;
+            requireNonNull( coordinate );
+            return map.get( coordinate );
+        };
     }
 
-    static boolean isVersionIncludedInRange( final String version, final String range )
+    private static final String ANY = "*";
+
+    /**
+     * Splits the pattern string into tokens, replacing empty tokens with {@link #ANY} for patterns like {@code ::val}
+     * so it retains the position of token.
+     */
+    private static String[] splitAndTokenize( String pattern )
     {
-        try
+        String[] stokens = pattern.split( ":" );
+        String[] tokens = new String[stokens.length];
+        for ( int i = 0; i < stokens.length; i++ )
         {
-            return VersionRange.createFromVersionSpec( range ).containsVersion( new DefaultArtifactVersion( version ) );
-        }
-        catch ( final InvalidVersionSpecificationException e )
-        {
-            return false;
+            String str = stokens[i];
+            tokens[i] = str != null && !str.isEmpty() ? str : ANY;
         }
+        return tokens;
     }
 
-    static Pattern compile( String pattern )
+    private static Pattern compile( String pattern )
     {
         if ( pattern.startsWith( "!" ) )
         {
             return new NegativePattern( pattern, compile( pattern.substring( 1 ) ) );
         }
         else
         {
-            char[][] stokens = tokenizeAndSplit( pattern );
-            char[][] tokens = new char[ stokens.length ][];
-            for ( int i = 0; i < stokens.length; i++ )
-            {
-                tokens[i] = anyOrChars( stokens[i] );
-            }
-            if ( tokens.length > 5 )
+            String[] tokens = splitAndTokenize( pattern );
+            if ( tokens.length < 1 || tokens.length > 5 )
             {
                 throw new IllegalArgumentException( "Invalid pattern: " + pattern );
             }
-            // we only accept 5 tokens if the classifier = '*'
+
+            ArrayList<Pattern> patterns = new ArrayList<>( 5 );
+
             if ( tokens.length == 5 )
             {
-                if ( tokens[3] != ANY )
+                // trivial, full pattern w/ classifier: G:A:T:C:V
+                patterns.add( toPattern( tokens[0], Coordinate.GROUP_ID ) );
+                patterns.add( toPattern( tokens[1], Coordinate.ARTIFACT_ID ) );
+                patterns.add( toPattern( tokens[2], Coordinate.TYPE ) );
+                patterns.add( toPattern( tokens[3], Coordinate.CLASSIFIER ) );
+                patterns.add( toPattern( tokens[4], Coordinate.BASE_VERSION ) );
+            }
+            else if ( tokens.length == 4 )
+            {
+                // trivial, full pattern w/o classifier: G:A:T:V
+                patterns.add( toPattern( tokens[0], Coordinate.GROUP_ID ) );
+                patterns.add( toPattern( tokens[1], Coordinate.ARTIFACT_ID ) );
+                patterns.add( toPattern( tokens[2], Coordinate.TYPE ) );
+                patterns.add( toPattern( tokens[3], Coordinate.BASE_VERSION ) );
+            }
+            else if ( tokens.length == 3 )
+            {
+                // tricky: may be "*:artifact:*" but also "*:war:*"
+
+                // *:*:* -> ALL
+                // *:*:xxx -> TC(xxx)
+                // *:xxx:* -> AT(xxx)
+                // *:xxx:yyy -> GA(xxx) + TC(XXX)
+                // xxx:*:* -> GA(xxx)
+                // xxx:*:yyy -> G(xxx) + TC(yyy)
+                // xxx:yyy:* -> G(xxx)+A(yyy)
+                // xxx:yyy:zzz -> G(xxx)+A(yyy)+T(zzz)
+                if ( ANY.equals( tokens[0] ) && ANY.equals( tokens[1] ) && ANY.equals( tokens[2] ) )

Review Comment:
   I have a problem with those:
   * Those aren't documented anywhere
   * How can we be sure that those aren't ambigious?



##########
src/test/java/org/apache/maven/shared/artifact/filter/GNPatternIncludesArtifactFilter.java:
##########
@@ -0,0 +1,917 @@
+package org.apache.maven.shared.artifact.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.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Objects;
+import java.util.Set;
+
+import org.apache.maven.artifact.Artifact;
+import org.apache.maven.artifact.resolver.filter.ArtifactFilter;
+import org.apache.maven.artifact.versioning.DefaultArtifactVersion;
+import org.apache.maven.artifact.versioning.InvalidVersionSpecificationException;
+import org.apache.maven.artifact.versioning.VersionRange;
+import org.slf4j.Logger;
+
+/**
+ * TODO: include in maven-artifact in future
+ *
+ * @author <a href="mailto:brett@apache.org">Brett Porter</a>
+ * @see StrictPatternIncludesArtifactFilter
+ */
+public class GNPatternIncludesArtifactFilter
+    implements ArtifactFilter, StatisticsReportingArtifactFilter
+{
+    /** Holds the set of compiled patterns */
+    private final Set<Pattern> patterns;
+
+    /** Whether the dependency trail should be checked */
+    private final boolean actTransitively;
+
+    /** Set of patterns that have been triggered */
+    private final Set<Pattern> patternsTriggered = new HashSet<>();
+
+    /** Set of artifacts that have been filtered out */
+    private final List<Artifact> filteredArtifact = new ArrayList<>();
+
+    /**
+     * <p>Constructor for PatternIncludesArtifactFilter.</p>
+     *
+     * @param patterns The pattern to be used.
+     */
+    public GNPatternIncludesArtifactFilter( final Collection<String> patterns )
+    {
+        this( patterns, false );
+    }
+
+    /**
+     * <p>Constructor for PatternIncludesArtifactFilter.</p>
+     *
+     * @param patterns The pattern to be used.
+     * @param actTransitively transitive yes/no.
+     */
+    public GNPatternIncludesArtifactFilter( final Collection<String> patterns, final boolean actTransitively )
+    {
+        this.actTransitively = actTransitively;
+        final Set<Pattern> pat = new LinkedHashSet<>();
+        if ( patterns != null && !patterns.isEmpty() )
+        {
+            for ( String pattern : patterns )
+            {
+
+                Pattern p = compile( pattern );
+                pat.add( p );
+            }
+        }
+        this.patterns = pat;
+    }
+
+    /** {@inheritDoc} */
+    public boolean include( final Artifact artifact )
+    {
+        final boolean shouldInclude = patternMatches( artifact );
+
+        if ( !shouldInclude )
+        {
+            addFilteredArtifact( artifact );
+        }
+
+        return shouldInclude;
+    }
+
+    /**
+     * <p>patternMatches.</p>
+     *
+     * @param artifact to check for.
+     * @return true if the match is true false otherwise.
+     */
+    protected boolean patternMatches( final Artifact artifact )
+    {
+        // Check if the main artifact matches
+        char[][] artifactGatvCharArray = new char[][] {
+                            emptyOrChars( artifact.getGroupId() ),
+                            emptyOrChars( artifact.getArtifactId() ),
+                            emptyOrChars( artifact.getType() ),
+                            emptyOrChars( artifact.getClassifier() ),
+                            emptyOrChars( artifact.getBaseVersion() )
+                    };
+        Boolean match = match( artifactGatvCharArray );
+        if ( match != null )
+        {
+            return match;
+        }
+
+        if ( actTransitively )
+        {
+            final List<String> depTrail = artifact.getDependencyTrail();
+
+            if ( depTrail != null && depTrail.size() > 1 )
+            {
+                for ( String trailItem : depTrail )
+                {
+                    char[][] depGatvCharArray = tokenizeAndSplit( trailItem );
+                    match = match( depGatvCharArray );
+                    if ( match != null )
+                    {
+                        return match;
+                    }
+                }
+            }
+        }
+
+        return false;
+    }
+
+    private Boolean match( char[][] gatvCharArray )
+    {
+        for ( Pattern pattern : patterns )
+        {
+            if ( pattern.matches( gatvCharArray ) )
+            {
+                patternsTriggered.add( pattern );
+                return !( pattern instanceof NegativePattern );
+            }
+        }
+
+        return null;
+    }
+
+    /**
+     * <p>addFilteredArtifact.</p>
+     *
+     * @param artifact add artifact to the filtered artifacts list.
+     */
+    protected void addFilteredArtifact( final Artifact artifact )
+    {
+        filteredArtifact.add( artifact );
+    }
+
+    /** {@inheritDoc} */
+    public void reportMissedCriteria( final Logger logger )
+    {
+        // if there are no patterns, there is nothing to report.
+        if ( !patterns.isEmpty() )
+        {
+            final List<Pattern> missed = new ArrayList<>( patterns );
+            missed.removeAll( patternsTriggered );
+
+            if ( !missed.isEmpty() && logger.isWarnEnabled() )
+            {
+                final StringBuilder buffer = new StringBuilder();
+
+                buffer.append( "The following patterns were never triggered in this " );
+                buffer.append( getFilterDescription() );
+                buffer.append( ':' );
+
+                for ( Pattern pattern : missed )
+                {
+                    buffer.append( "\no  '" ).append( pattern ).append( "'" );
+                }
+
+                buffer.append( "\n" );

Review Comment:
   Please drop this one.



##########
src/main/java/org/apache/maven/shared/artifact/filter/PatternIncludesArtifactFilter.java:
##########
@@ -255,540 +183,319 @@ public void reportMissedCriteria( final Logger logger )
 
                 for ( Pattern pattern : missed )
                 {
-                    buffer.append( "\no  '" ).append( pattern ).append( "'" );
+                    buffer.append( SEP ) .append( "o  '" ).append( pattern ).append( "'" );
                 }
 
-                buffer.append( "\n" );
+                buffer.append( SEP );
 
                 logger.warn( buffer.toString() );
             }
         }
     }
 
-    /** {@inheritDoc} */
     @Override
     public String toString()
     {
         return "Includes filter:" + getPatternsAsString();
     }
 
-    /**
-     * <p>getPatternsAsString.</p>
-     *
-     * @return pattern as a string.
-     */
     protected String getPatternsAsString()
     {
         final StringBuilder buffer = new StringBuilder();
         for ( Pattern pattern : patterns )
         {
-            buffer.append( "\no '" ).append( pattern ).append( "'" );
+            buffer.append( SEP ).append( "o '" ).append( pattern ).append( "'" );
         }
 
         return buffer.toString();
     }
 
-    /**
-     * <p>getFilterDescription.</p>
-     *
-     * @return description.
-     */
     protected String getFilterDescription()
     {
         return "artifact inclusion filter";
     }
 
-    /** {@inheritDoc} */
+    @Override
     public void reportFilteredArtifacts( final Logger logger )
     {
         if ( !filteredArtifact.isEmpty() && logger.isDebugEnabled() )
         {
-            final StringBuilder buffer =
-                new StringBuilder( "The following artifacts were removed by this " + getFilterDescription() + ": " );
+            final StringBuilder buffer = new StringBuilder(
+                    "The following artifacts were removed by this " + getFilterDescription() + ": " );
 
             for ( Artifact artifactId : filteredArtifact )
             {
-                buffer.append( '\n' ).append( artifactId.getId() );
+                buffer.append( SEP ).append( artifactId.getId() );
             }
 
             logger.debug( buffer.toString() );
         }
     }
 
-    /**
-     * {@inheritDoc}
-     *
-     * @return a boolean.
-     */
+    @Override
     public boolean hasMissedCriteria()
     {
         // if there are no patterns, there is nothing to report.
         if ( !patterns.isEmpty() )
         {
             final List<Pattern> missed = new ArrayList<>( patterns );
             missed.removeAll( patternsTriggered );
-
             return !missed.isEmpty();
         }
 
         return false;
     }
 
-    private static final char[] EMPTY = new char[0];
-
-    private static final char[] ANY = new char[] { '*' };
-
-    static char[] emptyOrChars( String str )
+    private enum Coordinate
     {
-        return str != null && str.length() > 0 ? str.toCharArray() : EMPTY;
+        GROUP_ID, ARTIFACT_ID, TYPE, CLASSIFIER, BASE_VERSION
     }
 
-    static char[] anyOrChars( char[] str )
+    private interface Artifactoid
     {
-        return str.length > 1 || ( str.length == 1 && str[0] != '*' ) ? str : ANY;
-    }
-
-    static char[][] tokenizeAndSplit( String pattern )
-    {
-        String[] stokens = pattern.split( ":" );
-        char[][] tokens = new char[ stokens.length ][];
-        for ( int i = 0; i < stokens.length; i++ )
-        {
-            tokens[i] = emptyOrChars( stokens[i] );
-        }
-        return tokens;
+        String getCoordinate( Coordinate coordinate );
     }
 
-    @SuppressWarnings( "InnerAssignment" )
-    static boolean match( char[] patArr, char[] strArr, boolean isVersion )
+    private static Artifactoid adapt( final Artifact artifact )
     {
-        int patIdxStart = 0;
-        int patIdxEnd = patArr.length - 1;
-        int strIdxStart = 0;
-        int strIdxEnd = strArr.length - 1;
-        char ch;
-
-        boolean containsStar = false;
-        for ( char aPatArr : patArr )
-        {
-            if ( aPatArr == '*' )
-            {
-                containsStar = true;
-                break;
-            }
-        }
-
-        if ( !containsStar )
-        {
-            if ( isVersion && ( patArr[0] == '[' || patArr[0] == '(' ) )
-            {
-                return isVersionIncludedInRange( String.valueOf( strArr ), String.valueOf( patArr ) );
-            }
-            // No '*'s, so we make a shortcut
-            if ( patIdxEnd != strIdxEnd )
-            {
-                return false; // Pattern and string do not have the same size
-            }
-            for ( int i = 0; i <= patIdxEnd; i++ )
-            {
-                ch = patArr[i];
-                if ( ch != '?' && ch != strArr[i] )
-                {
-                    return false; // Character mismatch
-                }
-            }
-            return true; // String matches against pattern
-        }
-
-        if ( patIdxEnd == 0 )
-        {
-            return true; // Pattern contains only '*', which matches anything
-        }
-
-        // Process characters before first star
-        while ( ( ch = patArr[patIdxStart] ) != '*' && strIdxStart <= strIdxEnd )
-        {
-            if ( ch != '?' && ch != strArr[strIdxStart] )
-            {
-                return false; // Character mismatch
-            }
-            patIdxStart++;
-            strIdxStart++;
-        }
-        if ( strIdxStart > strIdxEnd )
+        requireNonNull( artifact );
+        return coordinate ->
         {
-            // All characters in the string are used. Check if only '*'s are
-            // left in the pattern. If so, we succeeded. Otherwise failure.
-            for ( int i = patIdxStart; i <= patIdxEnd; i++ )
+            requireNonNull( coordinate );
+            switch ( coordinate )
             {
-                if ( patArr[i] != '*' )
-                {
-                    return false;
-                }
+                case GROUP_ID:
+                    return artifact.getGroupId();
+                case ARTIFACT_ID:
+                    return artifact.getArtifactId();
+                case BASE_VERSION:
+                    return artifact.getBaseVersion();
+                case CLASSIFIER:
+                    return artifact.hasClassifier() ? artifact.getClassifier() : null;
+                case TYPE:
+                    return artifact.getType();
+                default:
             }
-            return true;
-        }
+            throw new IllegalArgumentException( "unknown coordinate: " + coordinate );
+        };
+    }
 
-        // Process characters after last star
-        while ( ( ch = patArr[patIdxEnd] ) != '*' && strIdxStart <= strIdxEnd )
+    private static Artifactoid adapt( final String depTrailString )
+    {
+        requireNonNull( depTrailString );
+        // G:A:T:C:V

Review Comment:
   Please document how the pattern looks like with 4 components



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


[GitHub] [maven-common-artifact-filters] cstamas merged pull request #26: [MSHARED-1077] Bugfix for classifier in pattern

Posted by GitBox <gi...@apache.org>.
cstamas merged PR #26:
URL: https://github.com/apache/maven-common-artifact-filters/pull/26


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


[GitHub] [maven-common-artifact-filters] cstamas commented on a diff in pull request #26: [MSHARED-1077] Bugfix for classifier in pattern

Posted by GitBox <gi...@apache.org>.
cstamas commented on code in PR #26:
URL: https://github.com/apache/maven-common-artifact-filters/pull/26#discussion_r895008158


##########
src/test/java/org/apache/maven/shared/artifact/filter/GNPatternIncludesArtifactFilter.java:
##########
@@ -0,0 +1,917 @@
+package org.apache.maven.shared.artifact.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.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Objects;
+import java.util.Set;
+
+import org.apache.maven.artifact.Artifact;
+import org.apache.maven.artifact.resolver.filter.ArtifactFilter;
+import org.apache.maven.artifact.versioning.DefaultArtifactVersion;
+import org.apache.maven.artifact.versioning.InvalidVersionSpecificationException;
+import org.apache.maven.artifact.versioning.VersionRange;
+import org.slf4j.Logger;
+
+/**
+ * TODO: include in maven-artifact in future
+ *
+ * @author <a href="mailto:brett@apache.org">Brett Porter</a>
+ * @see StrictPatternIncludesArtifactFilter
+ */
+public class GNPatternIncludesArtifactFilter
+    implements ArtifactFilter, StatisticsReportingArtifactFilter
+{
+    /** Holds the set of compiled patterns */
+    private final Set<Pattern> patterns;
+
+    /** Whether the dependency trail should be checked */
+    private final boolean actTransitively;
+
+    /** Set of patterns that have been triggered */
+    private final Set<Pattern> patternsTriggered = new HashSet<>();
+
+    /** Set of artifacts that have been filtered out */
+    private final List<Artifact> filteredArtifact = new ArrayList<>();
+
+    /**
+     * <p>Constructor for PatternIncludesArtifactFilter.</p>
+     *
+     * @param patterns The pattern to be used.
+     */
+    public GNPatternIncludesArtifactFilter( final Collection<String> patterns )
+    {
+        this( patterns, false );
+    }
+
+    /**
+     * <p>Constructor for PatternIncludesArtifactFilter.</p>
+     *
+     * @param patterns The pattern to be used.
+     * @param actTransitively transitive yes/no.
+     */
+    public GNPatternIncludesArtifactFilter( final Collection<String> patterns, final boolean actTransitively )
+    {
+        this.actTransitively = actTransitively;
+        final Set<Pattern> pat = new LinkedHashSet<>();
+        if ( patterns != null && !patterns.isEmpty() )
+        {
+            for ( String pattern : patterns )
+            {
+
+                Pattern p = compile( pattern );
+                pat.add( p );
+            }
+        }
+        this.patterns = pat;
+    }
+
+    /** {@inheritDoc} */
+    public boolean include( final Artifact artifact )
+    {
+        final boolean shouldInclude = patternMatches( artifact );
+
+        if ( !shouldInclude )
+        {
+            addFilteredArtifact( artifact );
+        }
+
+        return shouldInclude;
+    }
+
+    /**
+     * <p>patternMatches.</p>
+     *
+     * @param artifact to check for.
+     * @return true if the match is true false otherwise.
+     */
+    protected boolean patternMatches( final Artifact artifact )
+    {
+        // Check if the main artifact matches
+        char[][] artifactGatvCharArray = new char[][] {
+                            emptyOrChars( artifact.getGroupId() ),
+                            emptyOrChars( artifact.getArtifactId() ),
+                            emptyOrChars( artifact.getType() ),
+                            emptyOrChars( artifact.getClassifier() ),
+                            emptyOrChars( artifact.getBaseVersion() )
+                    };
+        Boolean match = match( artifactGatvCharArray );
+        if ( match != null )
+        {
+            return match;
+        }
+
+        if ( actTransitively )
+        {
+            final List<String> depTrail = artifact.getDependencyTrail();
+
+            if ( depTrail != null && depTrail.size() > 1 )
+            {
+                for ( String trailItem : depTrail )
+                {
+                    char[][] depGatvCharArray = tokenizeAndSplit( trailItem );
+                    match = match( depGatvCharArray );
+                    if ( match != null )
+                    {
+                        return match;
+                    }
+                }
+            }
+        }
+
+        return false;
+    }
+
+    private Boolean match( char[][] gatvCharArray )
+    {
+        for ( Pattern pattern : patterns )
+        {
+            if ( pattern.matches( gatvCharArray ) )
+            {
+                patternsTriggered.add( pattern );
+                return !( pattern instanceof NegativePattern );
+            }
+        }
+
+        return null;
+    }
+
+    /**
+     * <p>addFilteredArtifact.</p>
+     *
+     * @param artifact add artifact to the filtered artifacts list.
+     */
+    protected void addFilteredArtifact( final Artifact artifact )
+    {
+        filteredArtifact.add( artifact );
+    }
+
+    /** {@inheritDoc} */
+    public void reportMissedCriteria( final Logger logger )
+    {
+        // if there are no patterns, there is nothing to report.
+        if ( !patterns.isEmpty() )
+        {
+            final List<Pattern> missed = new ArrayList<>( patterns );
+            missed.removeAll( patternsTriggered );
+
+            if ( !missed.isEmpty() && logger.isWarnEnabled() )
+            {
+                final StringBuilder buffer = new StringBuilder();
+
+                buffer.append( "The following patterns were never triggered in this " );
+                buffer.append( getFilterDescription() );
+                buffer.append( ':' );
+
+                for ( Pattern pattern : missed )
+                {
+                    buffer.append( "\no  '" ).append( pattern ).append( "'" );
+                }
+
+                buffer.append( "\n" );

Review Comment:
   This class IS A COPY of old implementation moved to src/test and retained for perf comparison purposes only. Am not modifying anything in it, it should remain as it was on master before this PR



##########
src/test/java/org/apache/maven/shared/artifact/filter/GNPatternIncludesArtifactFilter.java:
##########
@@ -0,0 +1,917 @@
+package org.apache.maven.shared.artifact.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.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Objects;
+import java.util.Set;
+
+import org.apache.maven.artifact.Artifact;
+import org.apache.maven.artifact.resolver.filter.ArtifactFilter;
+import org.apache.maven.artifact.versioning.DefaultArtifactVersion;
+import org.apache.maven.artifact.versioning.InvalidVersionSpecificationException;
+import org.apache.maven.artifact.versioning.VersionRange;
+import org.slf4j.Logger;
+
+/**
+ * TODO: include in maven-artifact in future
+ *
+ * @author <a href="mailto:brett@apache.org">Brett Porter</a>
+ * @see StrictPatternIncludesArtifactFilter
+ */
+public class GNPatternIncludesArtifactFilter
+    implements ArtifactFilter, StatisticsReportingArtifactFilter
+{
+    /** Holds the set of compiled patterns */
+    private final Set<Pattern> patterns;
+
+    /** Whether the dependency trail should be checked */
+    private final boolean actTransitively;
+
+    /** Set of patterns that have been triggered */
+    private final Set<Pattern> patternsTriggered = new HashSet<>();
+
+    /** Set of artifacts that have been filtered out */
+    private final List<Artifact> filteredArtifact = new ArrayList<>();
+
+    /**
+     * <p>Constructor for PatternIncludesArtifactFilter.</p>
+     *
+     * @param patterns The pattern to be used.
+     */
+    public GNPatternIncludesArtifactFilter( final Collection<String> patterns )
+    {
+        this( patterns, false );
+    }
+
+    /**
+     * <p>Constructor for PatternIncludesArtifactFilter.</p>
+     *
+     * @param patterns The pattern to be used.
+     * @param actTransitively transitive yes/no.
+     */
+    public GNPatternIncludesArtifactFilter( final Collection<String> patterns, final boolean actTransitively )
+    {
+        this.actTransitively = actTransitively;
+        final Set<Pattern> pat = new LinkedHashSet<>();
+        if ( patterns != null && !patterns.isEmpty() )
+        {
+            for ( String pattern : patterns )
+            {
+
+                Pattern p = compile( pattern );
+                pat.add( p );
+            }
+        }
+        this.patterns = pat;
+    }
+
+    /** {@inheritDoc} */
+    public boolean include( final Artifact artifact )
+    {
+        final boolean shouldInclude = patternMatches( artifact );
+
+        if ( !shouldInclude )
+        {
+            addFilteredArtifact( artifact );
+        }
+
+        return shouldInclude;
+    }
+
+    /**
+     * <p>patternMatches.</p>
+     *
+     * @param artifact to check for.
+     * @return true if the match is true false otherwise.
+     */
+    protected boolean patternMatches( final Artifact artifact )
+    {
+        // Check if the main artifact matches
+        char[][] artifactGatvCharArray = new char[][] {
+                            emptyOrChars( artifact.getGroupId() ),
+                            emptyOrChars( artifact.getArtifactId() ),
+                            emptyOrChars( artifact.getType() ),
+                            emptyOrChars( artifact.getClassifier() ),
+                            emptyOrChars( artifact.getBaseVersion() )
+                    };
+        Boolean match = match( artifactGatvCharArray );
+        if ( match != null )
+        {
+            return match;
+        }
+
+        if ( actTransitively )
+        {
+            final List<String> depTrail = artifact.getDependencyTrail();
+
+            if ( depTrail != null && depTrail.size() > 1 )
+            {
+                for ( String trailItem : depTrail )
+                {
+                    char[][] depGatvCharArray = tokenizeAndSplit( trailItem );
+                    match = match( depGatvCharArray );
+                    if ( match != null )
+                    {
+                        return match;
+                    }
+                }
+            }
+        }
+
+        return false;
+    }
+
+    private Boolean match( char[][] gatvCharArray )
+    {
+        for ( Pattern pattern : patterns )
+        {
+            if ( pattern.matches( gatvCharArray ) )
+            {
+                patternsTriggered.add( pattern );
+                return !( pattern instanceof NegativePattern );
+            }
+        }
+
+        return null;
+    }
+
+    /**
+     * <p>addFilteredArtifact.</p>
+     *
+     * @param artifact add artifact to the filtered artifacts list.
+     */
+    protected void addFilteredArtifact( final Artifact artifact )
+    {
+        filteredArtifact.add( artifact );
+    }
+
+    /** {@inheritDoc} */
+    public void reportMissedCriteria( final Logger logger )
+    {
+        // if there are no patterns, there is nothing to report.
+        if ( !patterns.isEmpty() )
+        {
+            final List<Pattern> missed = new ArrayList<>( patterns );
+            missed.removeAll( patternsTriggered );
+
+            if ( !missed.isEmpty() && logger.isWarnEnabled() )
+            {
+                final StringBuilder buffer = new StringBuilder();
+
+                buffer.append( "The following patterns were never triggered in this " );
+                buffer.append( getFilterDescription() );
+                buffer.append( ':' );
+
+                for ( Pattern pattern : missed )
+                {
+                    buffer.append( "\no  '" ).append( pattern ).append( "'" );
+                }
+
+                buffer.append( "\n" );
+
+                logger.warn( buffer.toString() );
+            }
+        }
+    }
+
+    /** {@inheritDoc} */
+    @Override
+    public String toString()
+    {
+        return "Includes filter:" + getPatternsAsString();
+    }
+
+    /**
+     * <p>getPatternsAsString.</p>
+     *
+     * @return pattern as a string.
+     */
+    protected String getPatternsAsString()
+    {
+        final StringBuilder buffer = new StringBuilder();
+        for ( Pattern pattern : patterns )
+        {
+            buffer.append( "\no '" ).append( pattern ).append( "'" );

Review Comment:
   This class IS A COPY of old implementation moved to src/test and retained for perf comparison purposes only. Am not modifying anything in it, it should remain as it was on master before this PR



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


[GitHub] [maven-common-artifact-filters] cstamas commented on pull request #26: [MSHARED-1077] Bugfix for classifier in pattern

Posted by GitBox <gi...@apache.org>.
cstamas commented on PR #26:
URL: https://github.com/apache/maven-common-artifact-filters/pull/26#issuecomment-1152470743

   Um. The GN file is copied from master unmodified. No need to review it


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


[GitHub] [maven-common-artifact-filters] michael-o commented on a diff in pull request #26: [MSHARED-1077] Bugfix for classifier in pattern

Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #26:
URL: https://github.com/apache/maven-common-artifact-filters/pull/26#discussion_r895007680


##########
src/main/java/org/apache/maven/shared/artifact/filter/PatternIncludesArtifactFilter.java:
##########
@@ -255,540 +183,319 @@ public void reportMissedCriteria( final Logger logger )
 
                 for ( Pattern pattern : missed )
                 {
-                    buffer.append( "\no  '" ).append( pattern ).append( "'" );
+                    buffer.append( SEP ) .append( "o  '" ).append( pattern ).append( "'" );
                 }
 
-                buffer.append( "\n" );
+                buffer.append( SEP );
 
                 logger.warn( buffer.toString() );
             }
         }
     }
 
-    /** {@inheritDoc} */
     @Override
     public String toString()
     {
         return "Includes filter:" + getPatternsAsString();
     }
 
-    /**
-     * <p>getPatternsAsString.</p>
-     *
-     * @return pattern as a string.
-     */
     protected String getPatternsAsString()
     {
         final StringBuilder buffer = new StringBuilder();
         for ( Pattern pattern : patterns )
         {
-            buffer.append( "\no '" ).append( pattern ).append( "'" );
+            buffer.append( SEP ).append( "o '" ).append( pattern ).append( "'" );
         }
 
         return buffer.toString();
     }
 
-    /**
-     * <p>getFilterDescription.</p>
-     *
-     * @return description.
-     */
     protected String getFilterDescription()
     {
         return "artifact inclusion filter";
     }
 
-    /** {@inheritDoc} */
+    @Override
     public void reportFilteredArtifacts( final Logger logger )
     {
         if ( !filteredArtifact.isEmpty() && logger.isDebugEnabled() )
         {
-            final StringBuilder buffer =
-                new StringBuilder( "The following artifacts were removed by this " + getFilterDescription() + ": " );
+            final StringBuilder buffer = new StringBuilder(
+                    "The following artifacts were removed by this " + getFilterDescription() + ": " );
 
             for ( Artifact artifactId : filteredArtifact )
             {
-                buffer.append( '\n' ).append( artifactId.getId() );
+                buffer.append( SEP ).append( artifactId.getId() );
             }
 
             logger.debug( buffer.toString() );
         }
     }
 
-    /**
-     * {@inheritDoc}
-     *
-     * @return a boolean.
-     */
+    @Override
     public boolean hasMissedCriteria()
     {
         // if there are no patterns, there is nothing to report.
         if ( !patterns.isEmpty() )
         {
             final List<Pattern> missed = new ArrayList<>( patterns );
             missed.removeAll( patternsTriggered );
-
             return !missed.isEmpty();
         }
 
         return false;
     }
 
-    private static final char[] EMPTY = new char[0];
-
-    private static final char[] ANY = new char[] { '*' };
-
-    static char[] emptyOrChars( String str )
+    private enum Coordinate
     {
-        return str != null && str.length() > 0 ? str.toCharArray() : EMPTY;
+        GROUP_ID, ARTIFACT_ID, TYPE, CLASSIFIER, BASE_VERSION
     }
 
-    static char[] anyOrChars( char[] str )
+    private interface Artifactoid
     {
-        return str.length > 1 || ( str.length == 1 && str[0] != '*' ) ? str : ANY;
-    }
-
-    static char[][] tokenizeAndSplit( String pattern )
-    {
-        String[] stokens = pattern.split( ":" );
-        char[][] tokens = new char[ stokens.length ][];
-        for ( int i = 0; i < stokens.length; i++ )
-        {
-            tokens[i] = emptyOrChars( stokens[i] );
-        }
-        return tokens;
+        String getCoordinate( Coordinate coordinate );
     }
 
-    @SuppressWarnings( "InnerAssignment" )
-    static boolean match( char[] patArr, char[] strArr, boolean isVersion )
+    private static Artifactoid adapt( final Artifact artifact )
     {
-        int patIdxStart = 0;
-        int patIdxEnd = patArr.length - 1;
-        int strIdxStart = 0;
-        int strIdxEnd = strArr.length - 1;
-        char ch;
-
-        boolean containsStar = false;
-        for ( char aPatArr : patArr )
-        {
-            if ( aPatArr == '*' )
-            {
-                containsStar = true;
-                break;
-            }
-        }
-
-        if ( !containsStar )
-        {
-            if ( isVersion && ( patArr[0] == '[' || patArr[0] == '(' ) )
-            {
-                return isVersionIncludedInRange( String.valueOf( strArr ), String.valueOf( patArr ) );
-            }
-            // No '*'s, so we make a shortcut
-            if ( patIdxEnd != strIdxEnd )
-            {
-                return false; // Pattern and string do not have the same size
-            }
-            for ( int i = 0; i <= patIdxEnd; i++ )
-            {
-                ch = patArr[i];
-                if ( ch != '?' && ch != strArr[i] )
-                {
-                    return false; // Character mismatch
-                }
-            }
-            return true; // String matches against pattern
-        }
-
-        if ( patIdxEnd == 0 )
-        {
-            return true; // Pattern contains only '*', which matches anything
-        }
-
-        // Process characters before first star
-        while ( ( ch = patArr[patIdxStart] ) != '*' && strIdxStart <= strIdxEnd )
-        {
-            if ( ch != '?' && ch != strArr[strIdxStart] )
-            {
-                return false; // Character mismatch
-            }
-            patIdxStart++;
-            strIdxStart++;
-        }
-        if ( strIdxStart > strIdxEnd )
+        requireNonNull( artifact );
+        return coordinate ->
         {
-            // All characters in the string are used. Check if only '*'s are
-            // left in the pattern. If so, we succeeded. Otherwise failure.
-            for ( int i = patIdxStart; i <= patIdxEnd; i++ )
+            requireNonNull( coordinate );
+            switch ( coordinate )
             {
-                if ( patArr[i] != '*' )
-                {
-                    return false;
-                }
+                case GROUP_ID:
+                    return artifact.getGroupId();
+                case ARTIFACT_ID:
+                    return artifact.getArtifactId();
+                case BASE_VERSION:
+                    return artifact.getBaseVersion();
+                case CLASSIFIER:
+                    return artifact.hasClassifier() ? artifact.getClassifier() : null;
+                case TYPE:
+                    return artifact.getType();
+                default:
             }
-            return true;
-        }
+            throw new IllegalArgumentException( "unknown coordinate: " + coordinate );
+        };
+    }
 
-        // Process characters after last star
-        while ( ( ch = patArr[patIdxEnd] ) != '*' && strIdxStart <= strIdxEnd )
+    private static Artifactoid adapt( final String depTrailString )
+    {
+        requireNonNull( depTrailString );
+        // G:A:T:C:V

Review Comment:
   No, the code processes two pattern style, no? 4 and 5 component. The comment documents on 5 components.



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


[GitHub] [maven-common-artifact-filters] michael-o commented on a diff in pull request #26: [MSHARED-1077] Bugfix for classifier in pattern

Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #26:
URL: https://github.com/apache/maven-common-artifact-filters/pull/26#discussion_r895007680


##########
src/main/java/org/apache/maven/shared/artifact/filter/PatternIncludesArtifactFilter.java:
##########
@@ -255,540 +183,319 @@ public void reportMissedCriteria( final Logger logger )
 
                 for ( Pattern pattern : missed )
                 {
-                    buffer.append( "\no  '" ).append( pattern ).append( "'" );
+                    buffer.append( SEP ) .append( "o  '" ).append( pattern ).append( "'" );
                 }
 
-                buffer.append( "\n" );
+                buffer.append( SEP );
 
                 logger.warn( buffer.toString() );
             }
         }
     }
 
-    /** {@inheritDoc} */
     @Override
     public String toString()
     {
         return "Includes filter:" + getPatternsAsString();
     }
 
-    /**
-     * <p>getPatternsAsString.</p>
-     *
-     * @return pattern as a string.
-     */
     protected String getPatternsAsString()
     {
         final StringBuilder buffer = new StringBuilder();
         for ( Pattern pattern : patterns )
         {
-            buffer.append( "\no '" ).append( pattern ).append( "'" );
+            buffer.append( SEP ).append( "o '" ).append( pattern ).append( "'" );
         }
 
         return buffer.toString();
     }
 
-    /**
-     * <p>getFilterDescription.</p>
-     *
-     * @return description.
-     */
     protected String getFilterDescription()
     {
         return "artifact inclusion filter";
     }
 
-    /** {@inheritDoc} */
+    @Override
     public void reportFilteredArtifacts( final Logger logger )
     {
         if ( !filteredArtifact.isEmpty() && logger.isDebugEnabled() )
         {
-            final StringBuilder buffer =
-                new StringBuilder( "The following artifacts were removed by this " + getFilterDescription() + ": " );
+            final StringBuilder buffer = new StringBuilder(
+                    "The following artifacts were removed by this " + getFilterDescription() + ": " );
 
             for ( Artifact artifactId : filteredArtifact )
             {
-                buffer.append( '\n' ).append( artifactId.getId() );
+                buffer.append( SEP ).append( artifactId.getId() );
             }
 
             logger.debug( buffer.toString() );
         }
     }
 
-    /**
-     * {@inheritDoc}
-     *
-     * @return a boolean.
-     */
+    @Override
     public boolean hasMissedCriteria()
     {
         // if there are no patterns, there is nothing to report.
         if ( !patterns.isEmpty() )
         {
             final List<Pattern> missed = new ArrayList<>( patterns );
             missed.removeAll( patternsTriggered );
-
             return !missed.isEmpty();
         }
 
         return false;
     }
 
-    private static final char[] EMPTY = new char[0];
-
-    private static final char[] ANY = new char[] { '*' };
-
-    static char[] emptyOrChars( String str )
+    private enum Coordinate
     {
-        return str != null && str.length() > 0 ? str.toCharArray() : EMPTY;
+        GROUP_ID, ARTIFACT_ID, TYPE, CLASSIFIER, BASE_VERSION
     }
 
-    static char[] anyOrChars( char[] str )
+    private interface Artifactoid
     {
-        return str.length > 1 || ( str.length == 1 && str[0] != '*' ) ? str : ANY;
-    }
-
-    static char[][] tokenizeAndSplit( String pattern )
-    {
-        String[] stokens = pattern.split( ":" );
-        char[][] tokens = new char[ stokens.length ][];
-        for ( int i = 0; i < stokens.length; i++ )
-        {
-            tokens[i] = emptyOrChars( stokens[i] );
-        }
-        return tokens;
+        String getCoordinate( Coordinate coordinate );
     }
 
-    @SuppressWarnings( "InnerAssignment" )
-    static boolean match( char[] patArr, char[] strArr, boolean isVersion )
+    private static Artifactoid adapt( final Artifact artifact )
     {
-        int patIdxStart = 0;
-        int patIdxEnd = patArr.length - 1;
-        int strIdxStart = 0;
-        int strIdxEnd = strArr.length - 1;
-        char ch;
-
-        boolean containsStar = false;
-        for ( char aPatArr : patArr )
-        {
-            if ( aPatArr == '*' )
-            {
-                containsStar = true;
-                break;
-            }
-        }
-
-        if ( !containsStar )
-        {
-            if ( isVersion && ( patArr[0] == '[' || patArr[0] == '(' ) )
-            {
-                return isVersionIncludedInRange( String.valueOf( strArr ), String.valueOf( patArr ) );
-            }
-            // No '*'s, so we make a shortcut
-            if ( patIdxEnd != strIdxEnd )
-            {
-                return false; // Pattern and string do not have the same size
-            }
-            for ( int i = 0; i <= patIdxEnd; i++ )
-            {
-                ch = patArr[i];
-                if ( ch != '?' && ch != strArr[i] )
-                {
-                    return false; // Character mismatch
-                }
-            }
-            return true; // String matches against pattern
-        }
-
-        if ( patIdxEnd == 0 )
-        {
-            return true; // Pattern contains only '*', which matches anything
-        }
-
-        // Process characters before first star
-        while ( ( ch = patArr[patIdxStart] ) != '*' && strIdxStart <= strIdxEnd )
-        {
-            if ( ch != '?' && ch != strArr[strIdxStart] )
-            {
-                return false; // Character mismatch
-            }
-            patIdxStart++;
-            strIdxStart++;
-        }
-        if ( strIdxStart > strIdxEnd )
+        requireNonNull( artifact );
+        return coordinate ->
         {
-            // All characters in the string are used. Check if only '*'s are
-            // left in the pattern. If so, we succeeded. Otherwise failure.
-            for ( int i = patIdxStart; i <= patIdxEnd; i++ )
+            requireNonNull( coordinate );
+            switch ( coordinate )
             {
-                if ( patArr[i] != '*' )
-                {
-                    return false;
-                }
+                case GROUP_ID:
+                    return artifact.getGroupId();
+                case ARTIFACT_ID:
+                    return artifact.getArtifactId();
+                case BASE_VERSION:
+                    return artifact.getBaseVersion();
+                case CLASSIFIER:
+                    return artifact.hasClassifier() ? artifact.getClassifier() : null;
+                case TYPE:
+                    return artifact.getType();
+                default:
             }
-            return true;
-        }
+            throw new IllegalArgumentException( "unknown coordinate: " + coordinate );
+        };
+    }
 
-        // Process characters after last star
-        while ( ( ch = patArr[patIdxEnd] ) != '*' && strIdxStart <= strIdxEnd )
+    private static Artifactoid adapt( final String depTrailString )
+    {
+        requireNonNull( depTrailString );
+        // G:A:T:C:V

Review Comment:
   No, the code processes two pattern styles, no? 4 and 5 component. The comment documents on 5 components.



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


[GitHub] [maven-common-artifact-filters] cstamas commented on a diff in pull request #26: [MSHARED-1077] Bugfix for classifier in pattern

Posted by GitBox <gi...@apache.org>.
cstamas commented on code in PR #26:
URL: https://github.com/apache/maven-common-artifact-filters/pull/26#discussion_r895008186


##########
src/test/java/org/apache/maven/shared/artifact/filter/GNPatternIncludesArtifactFilter.java:
##########
@@ -0,0 +1,917 @@
+package org.apache.maven.shared.artifact.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.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Objects;
+import java.util.Set;
+
+import org.apache.maven.artifact.Artifact;
+import org.apache.maven.artifact.resolver.filter.ArtifactFilter;
+import org.apache.maven.artifact.versioning.DefaultArtifactVersion;
+import org.apache.maven.artifact.versioning.InvalidVersionSpecificationException;
+import org.apache.maven.artifact.versioning.VersionRange;
+import org.slf4j.Logger;
+
+/**
+ * TODO: include in maven-artifact in future
+ *
+ * @author <a href="mailto:brett@apache.org">Brett Porter</a>
+ * @see StrictPatternIncludesArtifactFilter
+ */
+public class GNPatternIncludesArtifactFilter
+    implements ArtifactFilter, StatisticsReportingArtifactFilter
+{
+    /** Holds the set of compiled patterns */
+    private final Set<Pattern> patterns;
+
+    /** Whether the dependency trail should be checked */
+    private final boolean actTransitively;
+
+    /** Set of patterns that have been triggered */
+    private final Set<Pattern> patternsTriggered = new HashSet<>();
+
+    /** Set of artifacts that have been filtered out */
+    private final List<Artifact> filteredArtifact = new ArrayList<>();
+
+    /**
+     * <p>Constructor for PatternIncludesArtifactFilter.</p>
+     *
+     * @param patterns The pattern to be used.
+     */
+    public GNPatternIncludesArtifactFilter( final Collection<String> patterns )
+    {
+        this( patterns, false );
+    }
+
+    /**
+     * <p>Constructor for PatternIncludesArtifactFilter.</p>
+     *
+     * @param patterns The pattern to be used.
+     * @param actTransitively transitive yes/no.
+     */
+    public GNPatternIncludesArtifactFilter( final Collection<String> patterns, final boolean actTransitively )
+    {
+        this.actTransitively = actTransitively;
+        final Set<Pattern> pat = new LinkedHashSet<>();
+        if ( patterns != null && !patterns.isEmpty() )
+        {
+            for ( String pattern : patterns )
+            {
+
+                Pattern p = compile( pattern );
+                pat.add( p );
+            }
+        }
+        this.patterns = pat;
+    }
+
+    /** {@inheritDoc} */
+    public boolean include( final Artifact artifact )
+    {
+        final boolean shouldInclude = patternMatches( artifact );
+
+        if ( !shouldInclude )
+        {
+            addFilteredArtifact( artifact );
+        }
+
+        return shouldInclude;
+    }
+
+    /**
+     * <p>patternMatches.</p>
+     *
+     * @param artifact to check for.
+     * @return true if the match is true false otherwise.
+     */
+    protected boolean patternMatches( final Artifact artifact )
+    {
+        // Check if the main artifact matches
+        char[][] artifactGatvCharArray = new char[][] {
+                            emptyOrChars( artifact.getGroupId() ),
+                            emptyOrChars( artifact.getArtifactId() ),
+                            emptyOrChars( artifact.getType() ),
+                            emptyOrChars( artifact.getClassifier() ),
+                            emptyOrChars( artifact.getBaseVersion() )
+                    };
+        Boolean match = match( artifactGatvCharArray );
+        if ( match != null )
+        {
+            return match;
+        }
+
+        if ( actTransitively )
+        {
+            final List<String> depTrail = artifact.getDependencyTrail();
+
+            if ( depTrail != null && depTrail.size() > 1 )
+            {
+                for ( String trailItem : depTrail )
+                {
+                    char[][] depGatvCharArray = tokenizeAndSplit( trailItem );
+                    match = match( depGatvCharArray );
+                    if ( match != null )
+                    {
+                        return match;
+                    }
+                }
+            }
+        }
+
+        return false;
+    }
+
+    private Boolean match( char[][] gatvCharArray )
+    {
+        for ( Pattern pattern : patterns )
+        {
+            if ( pattern.matches( gatvCharArray ) )
+            {
+                patternsTriggered.add( pattern );
+                return !( pattern instanceof NegativePattern );
+            }
+        }
+
+        return null;
+    }
+
+    /**
+     * <p>addFilteredArtifact.</p>
+     *
+     * @param artifact add artifact to the filtered artifacts list.
+     */
+    protected void addFilteredArtifact( final Artifact artifact )
+    {
+        filteredArtifact.add( artifact );
+    }
+
+    /** {@inheritDoc} */
+    public void reportMissedCriteria( final Logger logger )
+    {
+        // if there are no patterns, there is nothing to report.
+        if ( !patterns.isEmpty() )
+        {
+            final List<Pattern> missed = new ArrayList<>( patterns );
+            missed.removeAll( patternsTriggered );
+
+            if ( !missed.isEmpty() && logger.isWarnEnabled() )
+            {
+                final StringBuilder buffer = new StringBuilder();
+
+                buffer.append( "The following patterns were never triggered in this " );
+                buffer.append( getFilterDescription() );
+                buffer.append( ':' );
+
+                for ( Pattern pattern : missed )
+                {
+                    buffer.append( "\no  '" ).append( pattern ).append( "'" );
+                }
+
+                buffer.append( "\n" );
+
+                logger.warn( buffer.toString() );
+            }
+        }
+    }
+
+    /** {@inheritDoc} */
+    @Override
+    public String toString()
+    {
+        return "Includes filter:" + getPatternsAsString();
+    }
+
+    /**
+     * <p>getPatternsAsString.</p>
+     *
+     * @return pattern as a string.
+     */
+    protected String getPatternsAsString()
+    {
+        final StringBuilder buffer = new StringBuilder();
+        for ( Pattern pattern : patterns )
+        {
+            buffer.append( "\no '" ).append( pattern ).append( "'" );
+        }
+
+        return buffer.toString();
+    }
+
+    /**
+     * <p>getFilterDescription.</p>
+     *
+     * @return description.
+     */
+    protected String getFilterDescription()
+    {
+        return "artifact inclusion filter";
+    }
+
+    /** {@inheritDoc} */
+    public void reportFilteredArtifacts( final Logger logger )
+    {
+        if ( !filteredArtifact.isEmpty() && logger.isDebugEnabled() )
+        {
+            final StringBuilder buffer =
+                new StringBuilder( "The following artifacts were removed by this " + getFilterDescription() + ": " );
+
+            for ( Artifact artifactId : filteredArtifact )
+            {
+                buffer.append( '\n' ).append( artifactId.getId() );

Review Comment:
   This class IS A COPY of old implementation moved to src/test and retained for perf comparison purposes only. Am not modifying anything in it, it should remain as it was on master before this PR



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


[GitHub] [maven-common-artifact-filters] cstamas commented on a diff in pull request #26: [MSHARED-1077] Bugfix for classifier in pattern

Posted by GitBox <gi...@apache.org>.
cstamas commented on code in PR #26:
URL: https://github.com/apache/maven-common-artifact-filters/pull/26#discussion_r895007591


##########
src/main/java/org/apache/maven/shared/artifact/filter/PatternIncludesArtifactFilter.java:
##########
@@ -255,540 +183,319 @@ public void reportMissedCriteria( final Logger logger )
 
                 for ( Pattern pattern : missed )
                 {
-                    buffer.append( "\no  '" ).append( pattern ).append( "'" );
+                    buffer.append( SEP ) .append( "o  '" ).append( pattern ).append( "'" );
                 }
 
-                buffer.append( "\n" );
+                buffer.append( SEP );
 
                 logger.warn( buffer.toString() );
             }
         }
     }
 
-    /** {@inheritDoc} */
     @Override
     public String toString()
     {
         return "Includes filter:" + getPatternsAsString();
     }
 
-    /**
-     * <p>getPatternsAsString.</p>
-     *
-     * @return pattern as a string.
-     */
     protected String getPatternsAsString()
     {
         final StringBuilder buffer = new StringBuilder();
         for ( Pattern pattern : patterns )
         {
-            buffer.append( "\no '" ).append( pattern ).append( "'" );
+            buffer.append( SEP ).append( "o '" ).append( pattern ).append( "'" );
         }
 
         return buffer.toString();
     }
 
-    /**
-     * <p>getFilterDescription.</p>
-     *
-     * @return description.
-     */
     protected String getFilterDescription()
     {
         return "artifact inclusion filter";
     }
 
-    /** {@inheritDoc} */
+    @Override
     public void reportFilteredArtifacts( final Logger logger )
     {
         if ( !filteredArtifact.isEmpty() && logger.isDebugEnabled() )
         {
-            final StringBuilder buffer =
-                new StringBuilder( "The following artifacts were removed by this " + getFilterDescription() + ": " );
+            final StringBuilder buffer = new StringBuilder(
+                    "The following artifacts were removed by this " + getFilterDescription() + ": " );
 
             for ( Artifact artifactId : filteredArtifact )
             {
-                buffer.append( '\n' ).append( artifactId.getId() );
+                buffer.append( SEP ).append( artifactId.getId() );
             }
 
             logger.debug( buffer.toString() );
         }
     }
 
-    /**
-     * {@inheritDoc}
-     *
-     * @return a boolean.
-     */
+    @Override
     public boolean hasMissedCriteria()
     {
         // if there are no patterns, there is nothing to report.
         if ( !patterns.isEmpty() )
         {
             final List<Pattern> missed = new ArrayList<>( patterns );
             missed.removeAll( patternsTriggered );
-
             return !missed.isEmpty();
         }
 
         return false;
     }
 
-    private static final char[] EMPTY = new char[0];
-
-    private static final char[] ANY = new char[] { '*' };
-
-    static char[] emptyOrChars( String str )
+    private enum Coordinate
     {
-        return str != null && str.length() > 0 ? str.toCharArray() : EMPTY;
+        GROUP_ID, ARTIFACT_ID, TYPE, CLASSIFIER, BASE_VERSION
     }
 
-    static char[] anyOrChars( char[] str )
+    private interface Artifactoid
     {
-        return str.length > 1 || ( str.length == 1 && str[0] != '*' ) ? str : ANY;
-    }
-
-    static char[][] tokenizeAndSplit( String pattern )
-    {
-        String[] stokens = pattern.split( ":" );
-        char[][] tokens = new char[ stokens.length ][];
-        for ( int i = 0; i < stokens.length; i++ )
-        {
-            tokens[i] = emptyOrChars( stokens[i] );
-        }
-        return tokens;
+        String getCoordinate( Coordinate coordinate );
     }
 
-    @SuppressWarnings( "InnerAssignment" )
-    static boolean match( char[] patArr, char[] strArr, boolean isVersion )
+    private static Artifactoid adapt( final Artifact artifact )
     {
-        int patIdxStart = 0;
-        int patIdxEnd = patArr.length - 1;
-        int strIdxStart = 0;
-        int strIdxEnd = strArr.length - 1;
-        char ch;
-
-        boolean containsStar = false;
-        for ( char aPatArr : patArr )
-        {
-            if ( aPatArr == '*' )
-            {
-                containsStar = true;
-                break;
-            }
-        }
-
-        if ( !containsStar )
-        {
-            if ( isVersion && ( patArr[0] == '[' || patArr[0] == '(' ) )
-            {
-                return isVersionIncludedInRange( String.valueOf( strArr ), String.valueOf( patArr ) );
-            }
-            // No '*'s, so we make a shortcut
-            if ( patIdxEnd != strIdxEnd )
-            {
-                return false; // Pattern and string do not have the same size
-            }
-            for ( int i = 0; i <= patIdxEnd; i++ )
-            {
-                ch = patArr[i];
-                if ( ch != '?' && ch != strArr[i] )
-                {
-                    return false; // Character mismatch
-                }
-            }
-            return true; // String matches against pattern
-        }
-
-        if ( patIdxEnd == 0 )
-        {
-            return true; // Pattern contains only '*', which matches anything
-        }
-
-        // Process characters before first star
-        while ( ( ch = patArr[patIdxStart] ) != '*' && strIdxStart <= strIdxEnd )
-        {
-            if ( ch != '?' && ch != strArr[strIdxStart] )
-            {
-                return false; // Character mismatch
-            }
-            patIdxStart++;
-            strIdxStart++;
-        }
-        if ( strIdxStart > strIdxEnd )
+        requireNonNull( artifact );
+        return coordinate ->
         {
-            // All characters in the string are used. Check if only '*'s are
-            // left in the pattern. If so, we succeeded. Otherwise failure.
-            for ( int i = patIdxStart; i <= patIdxEnd; i++ )
+            requireNonNull( coordinate );
+            switch ( coordinate )
             {
-                if ( patArr[i] != '*' )
-                {
-                    return false;
-                }
+                case GROUP_ID:
+                    return artifact.getGroupId();
+                case ARTIFACT_ID:
+                    return artifact.getArtifactId();
+                case BASE_VERSION:
+                    return artifact.getBaseVersion();
+                case CLASSIFIER:
+                    return artifact.hasClassifier() ? artifact.getClassifier() : null;
+                case TYPE:
+                    return artifact.getType();
+                default:
             }
-            return true;
-        }
+            throw new IllegalArgumentException( "unknown coordinate: " + coordinate );
+        };
+    }
 
-        // Process characters after last star
-        while ( ( ch = patArr[patIdxEnd] ) != '*' && strIdxStart <= strIdxEnd )
+    private static Artifactoid adapt( final String depTrailString )
+    {
+        requireNonNull( depTrailString );
+        // G:A:T:C:V

Review Comment:
   You want me to document `org.apache.maven.artifact.Artifact#getDependencyTrail()` method here?



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


[GitHub] [maven-common-artifact-filters] michael-o commented on a diff in pull request #26: [MSHARED-1077] Bugfix for classifier in pattern

Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #26:
URL: https://github.com/apache/maven-common-artifact-filters/pull/26#discussion_r895008710


##########
src/main/java/org/apache/maven/shared/artifact/filter/PatternIncludesArtifactFilter.java:
##########
@@ -255,540 +183,319 @@ public void reportMissedCriteria( final Logger logger )
 
                 for ( Pattern pattern : missed )
                 {
-                    buffer.append( "\no  '" ).append( pattern ).append( "'" );
+                    buffer.append( SEP ) .append( "o  '" ).append( pattern ).append( "'" );
                 }
 
-                buffer.append( "\n" );
+                buffer.append( SEP );
 
                 logger.warn( buffer.toString() );
             }
         }
     }
 
-    /** {@inheritDoc} */
     @Override
     public String toString()
     {
         return "Includes filter:" + getPatternsAsString();
     }
 
-    /**
-     * <p>getPatternsAsString.</p>
-     *
-     * @return pattern as a string.
-     */
     protected String getPatternsAsString()
     {
         final StringBuilder buffer = new StringBuilder();
         for ( Pattern pattern : patterns )
         {
-            buffer.append( "\no '" ).append( pattern ).append( "'" );
+            buffer.append( SEP ).append( "o '" ).append( pattern ).append( "'" );
         }
 
         return buffer.toString();
     }
 
-    /**
-     * <p>getFilterDescription.</p>
-     *
-     * @return description.
-     */
     protected String getFilterDescription()
     {
         return "artifact inclusion filter";
     }
 
-    /** {@inheritDoc} */
+    @Override
     public void reportFilteredArtifacts( final Logger logger )
     {
         if ( !filteredArtifact.isEmpty() && logger.isDebugEnabled() )
         {
-            final StringBuilder buffer =
-                new StringBuilder( "The following artifacts were removed by this " + getFilterDescription() + ": " );
+            final StringBuilder buffer = new StringBuilder(
+                    "The following artifacts were removed by this " + getFilterDescription() + ": " );
 
             for ( Artifact artifactId : filteredArtifact )
             {
-                buffer.append( '\n' ).append( artifactId.getId() );
+                buffer.append( SEP ).append( artifactId.getId() );
             }
 
             logger.debug( buffer.toString() );
         }
     }
 
-    /**
-     * {@inheritDoc}
-     *
-     * @return a boolean.
-     */
+    @Override
     public boolean hasMissedCriteria()
     {
         // if there are no patterns, there is nothing to report.
         if ( !patterns.isEmpty() )
         {
             final List<Pattern> missed = new ArrayList<>( patterns );
             missed.removeAll( patternsTriggered );
-
             return !missed.isEmpty();
         }
 
         return false;
     }
 
-    private static final char[] EMPTY = new char[0];
-
-    private static final char[] ANY = new char[] { '*' };
-
-    static char[] emptyOrChars( String str )
+    private enum Coordinate
     {
-        return str != null && str.length() > 0 ? str.toCharArray() : EMPTY;
+        GROUP_ID, ARTIFACT_ID, TYPE, CLASSIFIER, BASE_VERSION
     }
 
-    static char[] anyOrChars( char[] str )
+    private interface Artifactoid
     {
-        return str.length > 1 || ( str.length == 1 && str[0] != '*' ) ? str : ANY;
-    }
-
-    static char[][] tokenizeAndSplit( String pattern )
-    {
-        String[] stokens = pattern.split( ":" );
-        char[][] tokens = new char[ stokens.length ][];
-        for ( int i = 0; i < stokens.length; i++ )
-        {
-            tokens[i] = emptyOrChars( stokens[i] );
-        }
-        return tokens;
+        String getCoordinate( Coordinate coordinate );
     }
 
-    @SuppressWarnings( "InnerAssignment" )
-    static boolean match( char[] patArr, char[] strArr, boolean isVersion )
+    private static Artifactoid adapt( final Artifact artifact )
     {
-        int patIdxStart = 0;
-        int patIdxEnd = patArr.length - 1;
-        int strIdxStart = 0;
-        int strIdxEnd = strArr.length - 1;
-        char ch;
-
-        boolean containsStar = false;
-        for ( char aPatArr : patArr )
-        {
-            if ( aPatArr == '*' )
-            {
-                containsStar = true;
-                break;
-            }
-        }
-
-        if ( !containsStar )
-        {
-            if ( isVersion && ( patArr[0] == '[' || patArr[0] == '(' ) )
-            {
-                return isVersionIncludedInRange( String.valueOf( strArr ), String.valueOf( patArr ) );
-            }
-            // No '*'s, so we make a shortcut
-            if ( patIdxEnd != strIdxEnd )
-            {
-                return false; // Pattern and string do not have the same size
-            }
-            for ( int i = 0; i <= patIdxEnd; i++ )
-            {
-                ch = patArr[i];
-                if ( ch != '?' && ch != strArr[i] )
-                {
-                    return false; // Character mismatch
-                }
-            }
-            return true; // String matches against pattern
-        }
-
-        if ( patIdxEnd == 0 )
-        {
-            return true; // Pattern contains only '*', which matches anything
-        }
-
-        // Process characters before first star
-        while ( ( ch = patArr[patIdxStart] ) != '*' && strIdxStart <= strIdxEnd )
-        {
-            if ( ch != '?' && ch != strArr[strIdxStart] )
-            {
-                return false; // Character mismatch
-            }
-            patIdxStart++;
-            strIdxStart++;
-        }
-        if ( strIdxStart > strIdxEnd )
+        requireNonNull( artifact );
+        return coordinate ->
         {
-            // All characters in the string are used. Check if only '*'s are
-            // left in the pattern. If so, we succeeded. Otherwise failure.
-            for ( int i = patIdxStart; i <= patIdxEnd; i++ )
+            requireNonNull( coordinate );
+            switch ( coordinate )
             {
-                if ( patArr[i] != '*' )
-                {
-                    return false;
-                }
+                case GROUP_ID:
+                    return artifact.getGroupId();
+                case ARTIFACT_ID:
+                    return artifact.getArtifactId();
+                case BASE_VERSION:
+                    return artifact.getBaseVersion();
+                case CLASSIFIER:
+                    return artifact.hasClassifier() ? artifact.getClassifier() : null;
+                case TYPE:
+                    return artifact.getType();
+                default:
             }
-            return true;
-        }
+            throw new IllegalArgumentException( "unknown coordinate: " + coordinate );
+        };
+    }
 
-        // Process characters after last star
-        while ( ( ch = patArr[patIdxEnd] ) != '*' && strIdxStart <= strIdxEnd )
+    private static Artifactoid adapt( final String depTrailString )
+    {
+        requireNonNull( depTrailString );
+        // G:A:T:C:V

Review Comment:
   That's be fine, then we have context.



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


[GitHub] [maven-common-artifact-filters] cstamas commented on a diff in pull request #26: [MSHARED-1077] Bugfix for classifier in pattern

Posted by GitBox <gi...@apache.org>.
cstamas commented on code in PR #26:
URL: https://github.com/apache/maven-common-artifact-filters/pull/26#discussion_r895008089


##########
src/main/java/org/apache/maven/shared/artifact/filter/PatternIncludesArtifactFilter.java:
##########
@@ -255,540 +183,319 @@ public void reportMissedCriteria( final Logger logger )
 
                 for ( Pattern pattern : missed )
                 {
-                    buffer.append( "\no  '" ).append( pattern ).append( "'" );
+                    buffer.append( SEP ) .append( "o  '" ).append( pattern ).append( "'" );
                 }
 
-                buffer.append( "\n" );
+                buffer.append( SEP );
 
                 logger.warn( buffer.toString() );
             }
         }
     }
 
-    /** {@inheritDoc} */
     @Override
     public String toString()
     {
         return "Includes filter:" + getPatternsAsString();
     }
 
-    /**
-     * <p>getPatternsAsString.</p>
-     *
-     * @return pattern as a string.
-     */
     protected String getPatternsAsString()
     {
         final StringBuilder buffer = new StringBuilder();
         for ( Pattern pattern : patterns )
         {
-            buffer.append( "\no '" ).append( pattern ).append( "'" );
+            buffer.append( SEP ).append( "o '" ).append( pattern ).append( "'" );
         }
 
         return buffer.toString();
     }
 
-    /**
-     * <p>getFilterDescription.</p>
-     *
-     * @return description.
-     */
     protected String getFilterDescription()
     {
         return "artifact inclusion filter";
     }
 
-    /** {@inheritDoc} */
+    @Override
     public void reportFilteredArtifacts( final Logger logger )
     {
         if ( !filteredArtifact.isEmpty() && logger.isDebugEnabled() )
         {
-            final StringBuilder buffer =
-                new StringBuilder( "The following artifacts were removed by this " + getFilterDescription() + ": " );
+            final StringBuilder buffer = new StringBuilder(
+                    "The following artifacts were removed by this " + getFilterDescription() + ": " );
 
             for ( Artifact artifactId : filteredArtifact )
             {
-                buffer.append( '\n' ).append( artifactId.getId() );
+                buffer.append( SEP ).append( artifactId.getId() );
             }
 
             logger.debug( buffer.toString() );
         }
     }
 
-    /**
-     * {@inheritDoc}
-     *
-     * @return a boolean.
-     */
+    @Override
     public boolean hasMissedCriteria()
     {
         // if there are no patterns, there is nothing to report.
         if ( !patterns.isEmpty() )
         {
             final List<Pattern> missed = new ArrayList<>( patterns );
             missed.removeAll( patternsTriggered );
-
             return !missed.isEmpty();
         }
 
         return false;
     }
 
-    private static final char[] EMPTY = new char[0];
-
-    private static final char[] ANY = new char[] { '*' };
-
-    static char[] emptyOrChars( String str )
+    private enum Coordinate
     {
-        return str != null && str.length() > 0 ? str.toCharArray() : EMPTY;
+        GROUP_ID, ARTIFACT_ID, TYPE, CLASSIFIER, BASE_VERSION
     }
 
-    static char[] anyOrChars( char[] str )
+    private interface Artifactoid
     {
-        return str.length > 1 || ( str.length == 1 && str[0] != '*' ) ? str : ANY;
-    }
-
-    static char[][] tokenizeAndSplit( String pattern )
-    {
-        String[] stokens = pattern.split( ":" );
-        char[][] tokens = new char[ stokens.length ][];
-        for ( int i = 0; i < stokens.length; i++ )
-        {
-            tokens[i] = emptyOrChars( stokens[i] );
-        }
-        return tokens;
+        String getCoordinate( Coordinate coordinate );
     }
 
-    @SuppressWarnings( "InnerAssignment" )
-    static boolean match( char[] patArr, char[] strArr, boolean isVersion )
+    private static Artifactoid adapt( final Artifact artifact )
     {
-        int patIdxStart = 0;
-        int patIdxEnd = patArr.length - 1;
-        int strIdxStart = 0;
-        int strIdxEnd = strArr.length - 1;
-        char ch;
-
-        boolean containsStar = false;
-        for ( char aPatArr : patArr )
-        {
-            if ( aPatArr == '*' )
-            {
-                containsStar = true;
-                break;
-            }
-        }
-
-        if ( !containsStar )
-        {
-            if ( isVersion && ( patArr[0] == '[' || patArr[0] == '(' ) )
-            {
-                return isVersionIncludedInRange( String.valueOf( strArr ), String.valueOf( patArr ) );
-            }
-            // No '*'s, so we make a shortcut
-            if ( patIdxEnd != strIdxEnd )
-            {
-                return false; // Pattern and string do not have the same size
-            }
-            for ( int i = 0; i <= patIdxEnd; i++ )
-            {
-                ch = patArr[i];
-                if ( ch != '?' && ch != strArr[i] )
-                {
-                    return false; // Character mismatch
-                }
-            }
-            return true; // String matches against pattern
-        }
-
-        if ( patIdxEnd == 0 )
-        {
-            return true; // Pattern contains only '*', which matches anything
-        }
-
-        // Process characters before first star
-        while ( ( ch = patArr[patIdxStart] ) != '*' && strIdxStart <= strIdxEnd )
-        {
-            if ( ch != '?' && ch != strArr[strIdxStart] )
-            {
-                return false; // Character mismatch
-            }
-            patIdxStart++;
-            strIdxStart++;
-        }
-        if ( strIdxStart > strIdxEnd )
+        requireNonNull( artifact );
+        return coordinate ->
         {
-            // All characters in the string are used. Check if only '*'s are
-            // left in the pattern. If so, we succeeded. Otherwise failure.
-            for ( int i = patIdxStart; i <= patIdxEnd; i++ )
+            requireNonNull( coordinate );
+            switch ( coordinate )
             {
-                if ( patArr[i] != '*' )
-                {
-                    return false;
-                }
+                case GROUP_ID:
+                    return artifact.getGroupId();
+                case ARTIFACT_ID:
+                    return artifact.getArtifactId();
+                case BASE_VERSION:
+                    return artifact.getBaseVersion();
+                case CLASSIFIER:
+                    return artifact.hasClassifier() ? artifact.getClassifier() : null;
+                case TYPE:
+                    return artifact.getType();
+                default:
             }
-            return true;
-        }
+            throw new IllegalArgumentException( "unknown coordinate: " + coordinate );
+        };
+    }
 
-        // Process characters after last star
-        while ( ( ch = patArr[patIdxEnd] ) != '*' && strIdxStart <= strIdxEnd )
+    private static Artifactoid adapt( final String depTrailString )
+    {
+        requireNonNull( depTrailString );
+        // G:A:T:C:V
+        String[] coordinates = depTrailString.split( ":" );
+        if ( coordinates.length != 4 && coordinates.length != 5 )
         {
-            if ( ch != '?' && ch != strArr[strIdxEnd] )
-            {
-                return false; // Character mismatch
-            }
-            patIdxEnd--;
-            strIdxEnd--;
+            throw new IllegalArgumentException( "Bad dep trail string: " + depTrailString );
         }
-        if ( strIdxStart > strIdxEnd )
+        final HashMap<Coordinate, String> map = new HashMap<>();
+        map.put( Coordinate.GROUP_ID, coordinates[0] );
+        map.put( Coordinate.ARTIFACT_ID, coordinates[1] );
+        map.put( Coordinate.TYPE, coordinates[2] );
+        if ( coordinates.length == 5 )
         {
-            // All characters in the string are used. Check if only '*'s are
-            // left in the pattern. If so, we succeeded. Otherwise failure.
-            for ( int i = patIdxStart; i <= patIdxEnd; i++ )
-            {
-                if ( patArr[i] != '*' )
-                {
-                    return false;
-                }
-            }
-            return true;
+            map.put( Coordinate.CLASSIFIER, coordinates[3] );
+            map.put( Coordinate.BASE_VERSION, coordinates[4] );
         }
-
-        // process pattern between stars. padIdxStart and patIdxEnd point
-        // always to a '*'.
-        while ( patIdxStart != patIdxEnd && strIdxStart <= strIdxEnd )
+        else
         {
-            int patIdxTmp = -1;
-            for ( int i = patIdxStart + 1; i <= patIdxEnd; i++ )
-            {
-                if ( patArr[i] == '*' )
-                {
-                    patIdxTmp = i;
-                    break;
-                }
-            }
-            if ( patIdxTmp == patIdxStart + 1 )
-            {
-                // Two stars next to each other, skip the first one.
-                patIdxStart++;
-                continue;
-            }
-            // Find the pattern between padIdxStart & padIdxTmp in str between
-            // strIdxStart & strIdxEnd
-            int patLength = ( patIdxTmp - patIdxStart - 1 );
-            int strLength = ( strIdxEnd - strIdxStart + 1 );
-            int foundIdx = -1;
-            strLoop: for ( int i = 0; i <= strLength - patLength; i++ )
-            {
-                for ( int j = 0; j < patLength; j++ )
-                {
-                    ch = patArr[patIdxStart + j + 1];
-                    if ( ch != '?' && ch != strArr[strIdxStart + i + j] )
-                    {
-                        continue strLoop;
-                    }
-                }
-
-                foundIdx = strIdxStart + i;
-                break;
-            }
-
-            if ( foundIdx == -1 )
-            {
-                return false;
-            }
-
-            patIdxStart = patIdxTmp;
-            strIdxStart = foundIdx + patLength;
+            map.put( Coordinate.BASE_VERSION, coordinates[3] );
         }
 
-        // All characters in the string are used. Check if only '*'s are left
-        // in the pattern. If so, we succeeded. Otherwise failure.
-        for ( int i = patIdxStart; i <= patIdxEnd; i++ )
+        return coordinate ->
         {
-            if ( patArr[i] != '*' )
-            {
-                return false;
-            }
-        }
-        return true;
+            requireNonNull( coordinate );
+            return map.get( coordinate );
+        };
     }
 
-    static boolean isVersionIncludedInRange( final String version, final String range )
+    private static final String ANY = "*";
+
+    /**
+     * Splits the pattern string into tokens, replacing empty tokens with {@link #ANY} for patterns like {@code ::val}
+     * so it retains the position of token.
+     */
+    private static String[] splitAndTokenize( String pattern )
     {
-        try
+        String[] stokens = pattern.split( ":" );
+        String[] tokens = new String[stokens.length];
+        for ( int i = 0; i < stokens.length; i++ )
         {
-            return VersionRange.createFromVersionSpec( range ).containsVersion( new DefaultArtifactVersion( version ) );
-        }
-        catch ( final InvalidVersionSpecificationException e )
-        {
-            return false;
+            String str = stokens[i];
+            tokens[i] = str != null && !str.isEmpty() ? str : ANY;
         }
+        return tokens;
     }
 
-    static Pattern compile( String pattern )
+    private static Pattern compile( String pattern )
     {
         if ( pattern.startsWith( "!" ) )
         {
             return new NegativePattern( pattern, compile( pattern.substring( 1 ) ) );
         }
         else
         {
-            char[][] stokens = tokenizeAndSplit( pattern );
-            char[][] tokens = new char[ stokens.length ][];
-            for ( int i = 0; i < stokens.length; i++ )
-            {
-                tokens[i] = anyOrChars( stokens[i] );
-            }
-            if ( tokens.length > 5 )
+            String[] tokens = splitAndTokenize( pattern );
+            if ( tokens.length < 1 || tokens.length > 5 )
             {
                 throw new IllegalArgumentException( "Invalid pattern: " + pattern );
             }
-            // we only accept 5 tokens if the classifier = '*'
+
+            ArrayList<Pattern> patterns = new ArrayList<>( 5 );
+
             if ( tokens.length == 5 )
             {
-                if ( tokens[3] != ANY )
+                // trivial, full pattern w/ classifier: G:A:T:C:V
+                patterns.add( toPattern( tokens[0], Coordinate.GROUP_ID ) );
+                patterns.add( toPattern( tokens[1], Coordinate.ARTIFACT_ID ) );
+                patterns.add( toPattern( tokens[2], Coordinate.TYPE ) );
+                patterns.add( toPattern( tokens[3], Coordinate.CLASSIFIER ) );
+                patterns.add( toPattern( tokens[4], Coordinate.BASE_VERSION ) );
+            }
+            else if ( tokens.length == 4 )
+            {
+                // trivial, full pattern w/o classifier: G:A:T:V
+                patterns.add( toPattern( tokens[0], Coordinate.GROUP_ID ) );
+                patterns.add( toPattern( tokens[1], Coordinate.ARTIFACT_ID ) );
+                patterns.add( toPattern( tokens[2], Coordinate.TYPE ) );
+                patterns.add( toPattern( tokens[3], Coordinate.BASE_VERSION ) );
+            }
+            else if ( tokens.length == 3 )
+            {
+                // tricky: may be "*:artifact:*" but also "*:war:*"
+
+                // *:*:* -> ALL
+                // *:*:xxx -> TC(xxx)
+                // *:xxx:* -> AT(xxx)
+                // *:xxx:yyy -> GA(xxx) + TC(XXX)
+                // xxx:*:* -> GA(xxx)
+                // xxx:*:yyy -> G(xxx) + TC(yyy)
+                // xxx:yyy:* -> G(xxx)+A(yyy)
+                // xxx:yyy:zzz -> G(xxx)+A(yyy)+T(zzz)
+                if ( ANY.equals( tokens[0] ) && ANY.equals( tokens[1] ) && ANY.equals( tokens[2] ) )

Review Comment:
   This logic just re-implement what happened before (old and older code). My intention is not to fix the pattern spec or change any behavior (as test proves).



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


[GitHub] [maven-common-artifact-filters] cstamas commented on a diff in pull request #26: [MSHARED-1077] Bugfix for classifier in pattern

Posted by GitBox <gi...@apache.org>.
cstamas commented on code in PR #26:
URL: https://github.com/apache/maven-common-artifact-filters/pull/26#discussion_r895009388


##########
src/main/java/org/apache/maven/shared/artifact/filter/PatternIncludesArtifactFilter.java:
##########
@@ -255,540 +183,319 @@ public void reportMissedCriteria( final Logger logger )
 
                 for ( Pattern pattern : missed )
                 {
-                    buffer.append( "\no  '" ).append( pattern ).append( "'" );
+                    buffer.append( SEP ) .append( "o  '" ).append( pattern ).append( "'" );
                 }
 
-                buffer.append( "\n" );
+                buffer.append( SEP );
 
                 logger.warn( buffer.toString() );
             }
         }
     }
 
-    /** {@inheritDoc} */
     @Override
     public String toString()
     {
         return "Includes filter:" + getPatternsAsString();
     }
 
-    /**
-     * <p>getPatternsAsString.</p>
-     *
-     * @return pattern as a string.
-     */
     protected String getPatternsAsString()
     {
         final StringBuilder buffer = new StringBuilder();
         for ( Pattern pattern : patterns )
         {
-            buffer.append( "\no '" ).append( pattern ).append( "'" );
+            buffer.append( SEP ).append( "o '" ).append( pattern ).append( "'" );
         }
 
         return buffer.toString();
     }
 
-    /**
-     * <p>getFilterDescription.</p>
-     *
-     * @return description.
-     */
     protected String getFilterDescription()
     {
         return "artifact inclusion filter";
     }
 
-    /** {@inheritDoc} */
+    @Override
     public void reportFilteredArtifacts( final Logger logger )
     {
         if ( !filteredArtifact.isEmpty() && logger.isDebugEnabled() )
         {
-            final StringBuilder buffer =
-                new StringBuilder( "The following artifacts were removed by this " + getFilterDescription() + ": " );
+            final StringBuilder buffer = new StringBuilder(
+                    "The following artifacts were removed by this " + getFilterDescription() + ": " );
 
             for ( Artifact artifactId : filteredArtifact )
             {
-                buffer.append( '\n' ).append( artifactId.getId() );
+                buffer.append( SEP ).append( artifactId.getId() );
             }
 
             logger.debug( buffer.toString() );
         }
     }
 
-    /**
-     * {@inheritDoc}
-     *
-     * @return a boolean.
-     */
+    @Override
     public boolean hasMissedCriteria()
     {
         // if there are no patterns, there is nothing to report.
         if ( !patterns.isEmpty() )
         {
             final List<Pattern> missed = new ArrayList<>( patterns );
             missed.removeAll( patternsTriggered );
-
             return !missed.isEmpty();
         }
 
         return false;
     }
 
-    private static final char[] EMPTY = new char[0];
-
-    private static final char[] ANY = new char[] { '*' };
-
-    static char[] emptyOrChars( String str )
+    private enum Coordinate
     {
-        return str != null && str.length() > 0 ? str.toCharArray() : EMPTY;
+        GROUP_ID, ARTIFACT_ID, TYPE, CLASSIFIER, BASE_VERSION
     }
 
-    static char[] anyOrChars( char[] str )
+    private interface Artifactoid
     {
-        return str.length > 1 || ( str.length == 1 && str[0] != '*' ) ? str : ANY;
-    }
-
-    static char[][] tokenizeAndSplit( String pattern )
-    {
-        String[] stokens = pattern.split( ":" );
-        char[][] tokens = new char[ stokens.length ][];
-        for ( int i = 0; i < stokens.length; i++ )
-        {
-            tokens[i] = emptyOrChars( stokens[i] );
-        }
-        return tokens;
+        String getCoordinate( Coordinate coordinate );
     }
 
-    @SuppressWarnings( "InnerAssignment" )
-    static boolean match( char[] patArr, char[] strArr, boolean isVersion )
+    private static Artifactoid adapt( final Artifact artifact )
     {
-        int patIdxStart = 0;
-        int patIdxEnd = patArr.length - 1;
-        int strIdxStart = 0;
-        int strIdxEnd = strArr.length - 1;
-        char ch;
-
-        boolean containsStar = false;
-        for ( char aPatArr : patArr )
-        {
-            if ( aPatArr == '*' )
-            {
-                containsStar = true;
-                break;
-            }
-        }
-
-        if ( !containsStar )
-        {
-            if ( isVersion && ( patArr[0] == '[' || patArr[0] == '(' ) )
-            {
-                return isVersionIncludedInRange( String.valueOf( strArr ), String.valueOf( patArr ) );
-            }
-            // No '*'s, so we make a shortcut
-            if ( patIdxEnd != strIdxEnd )
-            {
-                return false; // Pattern and string do not have the same size
-            }
-            for ( int i = 0; i <= patIdxEnd; i++ )
-            {
-                ch = patArr[i];
-                if ( ch != '?' && ch != strArr[i] )
-                {
-                    return false; // Character mismatch
-                }
-            }
-            return true; // String matches against pattern
-        }
-
-        if ( patIdxEnd == 0 )
-        {
-            return true; // Pattern contains only '*', which matches anything
-        }
-
-        // Process characters before first star
-        while ( ( ch = patArr[patIdxStart] ) != '*' && strIdxStart <= strIdxEnd )
-        {
-            if ( ch != '?' && ch != strArr[strIdxStart] )
-            {
-                return false; // Character mismatch
-            }
-            patIdxStart++;
-            strIdxStart++;
-        }
-        if ( strIdxStart > strIdxEnd )
+        requireNonNull( artifact );
+        return coordinate ->
         {
-            // All characters in the string are used. Check if only '*'s are
-            // left in the pattern. If so, we succeeded. Otherwise failure.
-            for ( int i = patIdxStart; i <= patIdxEnd; i++ )
+            requireNonNull( coordinate );
+            switch ( coordinate )
             {
-                if ( patArr[i] != '*' )
-                {
-                    return false;
-                }
+                case GROUP_ID:
+                    return artifact.getGroupId();
+                case ARTIFACT_ID:
+                    return artifact.getArtifactId();
+                case BASE_VERSION:
+                    return artifact.getBaseVersion();
+                case CLASSIFIER:
+                    return artifact.hasClassifier() ? artifact.getClassifier() : null;
+                case TYPE:
+                    return artifact.getType();
+                default:
             }
-            return true;
-        }
+            throw new IllegalArgumentException( "unknown coordinate: " + coordinate );
+        };
+    }
 
-        // Process characters after last star
-        while ( ( ch = patArr[patIdxEnd] ) != '*' && strIdxStart <= strIdxEnd )
+    private static Artifactoid adapt( final String depTrailString )
+    {
+        requireNonNull( depTrailString );
+        // G:A:T:C:V
+        String[] coordinates = depTrailString.split( ":" );
+        if ( coordinates.length != 4 && coordinates.length != 5 )
         {
-            if ( ch != '?' && ch != strArr[strIdxEnd] )
-            {
-                return false; // Character mismatch
-            }
-            patIdxEnd--;
-            strIdxEnd--;
+            throw new IllegalArgumentException( "Bad dep trail string: " + depTrailString );
         }
-        if ( strIdxStart > strIdxEnd )
+        final HashMap<Coordinate, String> map = new HashMap<>();
+        map.put( Coordinate.GROUP_ID, coordinates[0] );
+        map.put( Coordinate.ARTIFACT_ID, coordinates[1] );
+        map.put( Coordinate.TYPE, coordinates[2] );
+        if ( coordinates.length == 5 )
         {
-            // All characters in the string are used. Check if only '*'s are
-            // left in the pattern. If so, we succeeded. Otherwise failure.
-            for ( int i = patIdxStart; i <= patIdxEnd; i++ )
-            {
-                if ( patArr[i] != '*' )
-                {
-                    return false;
-                }
-            }
-            return true;
+            map.put( Coordinate.CLASSIFIER, coordinates[3] );
+            map.put( Coordinate.BASE_VERSION, coordinates[4] );
         }
-
-        // process pattern between stars. padIdxStart and patIdxEnd point
-        // always to a '*'.
-        while ( patIdxStart != patIdxEnd && strIdxStart <= strIdxEnd )
+        else
         {
-            int patIdxTmp = -1;
-            for ( int i = patIdxStart + 1; i <= patIdxEnd; i++ )
-            {
-                if ( patArr[i] == '*' )
-                {
-                    patIdxTmp = i;
-                    break;
-                }
-            }
-            if ( patIdxTmp == patIdxStart + 1 )
-            {
-                // Two stars next to each other, skip the first one.
-                patIdxStart++;
-                continue;
-            }
-            // Find the pattern between padIdxStart & padIdxTmp in str between
-            // strIdxStart & strIdxEnd
-            int patLength = ( patIdxTmp - patIdxStart - 1 );
-            int strLength = ( strIdxEnd - strIdxStart + 1 );
-            int foundIdx = -1;
-            strLoop: for ( int i = 0; i <= strLength - patLength; i++ )
-            {
-                for ( int j = 0; j < patLength; j++ )
-                {
-                    ch = patArr[patIdxStart + j + 1];
-                    if ( ch != '?' && ch != strArr[strIdxStart + i + j] )
-                    {
-                        continue strLoop;
-                    }
-                }
-
-                foundIdx = strIdxStart + i;
-                break;
-            }
-
-            if ( foundIdx == -1 )
-            {
-                return false;
-            }
-
-            patIdxStart = patIdxTmp;
-            strIdxStart = foundIdx + patLength;
+            map.put( Coordinate.BASE_VERSION, coordinates[3] );
         }
 
-        // All characters in the string are used. Check if only '*'s are left
-        // in the pattern. If so, we succeeded. Otherwise failure.
-        for ( int i = patIdxStart; i <= patIdxEnd; i++ )
+        return coordinate ->
         {
-            if ( patArr[i] != '*' )
-            {
-                return false;
-            }
-        }
-        return true;
+            requireNonNull( coordinate );
+            return map.get( coordinate );
+        };
     }
 
-    static boolean isVersionIncludedInRange( final String version, final String range )
+    private static final String ANY = "*";
+
+    /**
+     * Splits the pattern string into tokens, replacing empty tokens with {@link #ANY} for patterns like {@code ::val}
+     * so it retains the position of token.
+     */
+    private static String[] splitAndTokenize( String pattern )
     {
-        try
+        String[] stokens = pattern.split( ":" );
+        String[] tokens = new String[stokens.length];
+        for ( int i = 0; i < stokens.length; i++ )
         {
-            return VersionRange.createFromVersionSpec( range ).containsVersion( new DefaultArtifactVersion( version ) );
-        }
-        catch ( final InvalidVersionSpecificationException e )
-        {
-            return false;
+            String str = stokens[i];
+            tokens[i] = str != null && !str.isEmpty() ? str : ANY;
         }
+        return tokens;
     }
 
-    static Pattern compile( String pattern )
+    private static Pattern compile( String pattern )
     {
         if ( pattern.startsWith( "!" ) )
         {
             return new NegativePattern( pattern, compile( pattern.substring( 1 ) ) );
         }
         else
         {
-            char[][] stokens = tokenizeAndSplit( pattern );
-            char[][] tokens = new char[ stokens.length ][];
-            for ( int i = 0; i < stokens.length; i++ )
-            {
-                tokens[i] = anyOrChars( stokens[i] );
-            }
-            if ( tokens.length > 5 )
+            String[] tokens = splitAndTokenize( pattern );
+            if ( tokens.length < 1 || tokens.length > 5 )
             {
                 throw new IllegalArgumentException( "Invalid pattern: " + pattern );
             }
-            // we only accept 5 tokens if the classifier = '*'
+
+            ArrayList<Pattern> patterns = new ArrayList<>( 5 );
+
             if ( tokens.length == 5 )
             {
-                if ( tokens[3] != ANY )
+                // trivial, full pattern w/ classifier: G:A:T:C:V
+                patterns.add( toPattern( tokens[0], Coordinate.GROUP_ID ) );
+                patterns.add( toPattern( tokens[1], Coordinate.ARTIFACT_ID ) );
+                patterns.add( toPattern( tokens[2], Coordinate.TYPE ) );
+                patterns.add( toPattern( tokens[3], Coordinate.CLASSIFIER ) );
+                patterns.add( toPattern( tokens[4], Coordinate.BASE_VERSION ) );
+            }
+            else if ( tokens.length == 4 )
+            {
+                // trivial, full pattern w/o classifier: G:A:T:V
+                patterns.add( toPattern( tokens[0], Coordinate.GROUP_ID ) );
+                patterns.add( toPattern( tokens[1], Coordinate.ARTIFACT_ID ) );
+                patterns.add( toPattern( tokens[2], Coordinate.TYPE ) );
+                patterns.add( toPattern( tokens[3], Coordinate.BASE_VERSION ) );
+            }
+            else if ( tokens.length == 3 )
+            {
+                // tricky: may be "*:artifact:*" but also "*:war:*"
+
+                // *:*:* -> ALL
+                // *:*:xxx -> TC(xxx)
+                // *:xxx:* -> AT(xxx)
+                // *:xxx:yyy -> GA(xxx) + TC(XXX)
+                // xxx:*:* -> GA(xxx)
+                // xxx:*:yyy -> G(xxx) + TC(yyy)
+                // xxx:yyy:* -> G(xxx)+A(yyy)
+                // xxx:yyy:zzz -> G(xxx)+A(yyy)+T(zzz)
+                if ( ANY.equals( tokens[0] ) && ANY.equals( tokens[1] ) && ANY.equals( tokens[2] ) )

Review Comment:
   pushed 349dd8031f0fed3e9f4e842c7f14da5e1220d52f



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


[GitHub] [maven-common-artifact-filters] cstamas commented on a diff in pull request #26: [MSHARED-1077] Bugfix for classifier in pattern

Posted by GitBox <gi...@apache.org>.
cstamas commented on code in PR #26:
URL: https://github.com/apache/maven-common-artifact-filters/pull/26#discussion_r895008555


##########
src/main/java/org/apache/maven/shared/artifact/filter/PatternIncludesArtifactFilter.java:
##########
@@ -255,540 +183,319 @@ public void reportMissedCriteria( final Logger logger )
 
                 for ( Pattern pattern : missed )
                 {
-                    buffer.append( "\no  '" ).append( pattern ).append( "'" );
+                    buffer.append( SEP ) .append( "o  '" ).append( pattern ).append( "'" );
                 }
 
-                buffer.append( "\n" );
+                buffer.append( SEP );
 
                 logger.warn( buffer.toString() );
             }
         }
     }
 
-    /** {@inheritDoc} */
     @Override
     public String toString()
     {
         return "Includes filter:" + getPatternsAsString();
     }
 
-    /**
-     * <p>getPatternsAsString.</p>
-     *
-     * @return pattern as a string.
-     */
     protected String getPatternsAsString()
     {
         final StringBuilder buffer = new StringBuilder();
         for ( Pattern pattern : patterns )
         {
-            buffer.append( "\no '" ).append( pattern ).append( "'" );
+            buffer.append( SEP ).append( "o '" ).append( pattern ).append( "'" );
         }
 
         return buffer.toString();
     }
 
-    /**
-     * <p>getFilterDescription.</p>
-     *
-     * @return description.
-     */
     protected String getFilterDescription()
     {
         return "artifact inclusion filter";
     }
 
-    /** {@inheritDoc} */
+    @Override
     public void reportFilteredArtifacts( final Logger logger )
     {
         if ( !filteredArtifact.isEmpty() && logger.isDebugEnabled() )
         {
-            final StringBuilder buffer =
-                new StringBuilder( "The following artifacts were removed by this " + getFilterDescription() + ": " );
+            final StringBuilder buffer = new StringBuilder(
+                    "The following artifacts were removed by this " + getFilterDescription() + ": " );
 
             for ( Artifact artifactId : filteredArtifact )
             {
-                buffer.append( '\n' ).append( artifactId.getId() );
+                buffer.append( SEP ).append( artifactId.getId() );
             }
 
             logger.debug( buffer.toString() );
         }
     }
 
-    /**
-     * {@inheritDoc}
-     *
-     * @return a boolean.
-     */
+    @Override
     public boolean hasMissedCriteria()
     {
         // if there are no patterns, there is nothing to report.
         if ( !patterns.isEmpty() )
         {
             final List<Pattern> missed = new ArrayList<>( patterns );
             missed.removeAll( patternsTriggered );
-
             return !missed.isEmpty();
         }
 
         return false;
     }
 
-    private static final char[] EMPTY = new char[0];
-
-    private static final char[] ANY = new char[] { '*' };
-
-    static char[] emptyOrChars( String str )
+    private enum Coordinate
     {
-        return str != null && str.length() > 0 ? str.toCharArray() : EMPTY;
+        GROUP_ID, ARTIFACT_ID, TYPE, CLASSIFIER, BASE_VERSION
     }
 
-    static char[] anyOrChars( char[] str )
+    private interface Artifactoid
     {
-        return str.length > 1 || ( str.length == 1 && str[0] != '*' ) ? str : ANY;
-    }
-
-    static char[][] tokenizeAndSplit( String pattern )
-    {
-        String[] stokens = pattern.split( ":" );
-        char[][] tokens = new char[ stokens.length ][];
-        for ( int i = 0; i < stokens.length; i++ )
-        {
-            tokens[i] = emptyOrChars( stokens[i] );
-        }
-        return tokens;
+        String getCoordinate( Coordinate coordinate );
     }
 
-    @SuppressWarnings( "InnerAssignment" )
-    static boolean match( char[] patArr, char[] strArr, boolean isVersion )
+    private static Artifactoid adapt( final Artifact artifact )
     {
-        int patIdxStart = 0;
-        int patIdxEnd = patArr.length - 1;
-        int strIdxStart = 0;
-        int strIdxEnd = strArr.length - 1;
-        char ch;
-
-        boolean containsStar = false;
-        for ( char aPatArr : patArr )
-        {
-            if ( aPatArr == '*' )
-            {
-                containsStar = true;
-                break;
-            }
-        }
-
-        if ( !containsStar )
-        {
-            if ( isVersion && ( patArr[0] == '[' || patArr[0] == '(' ) )
-            {
-                return isVersionIncludedInRange( String.valueOf( strArr ), String.valueOf( patArr ) );
-            }
-            // No '*'s, so we make a shortcut
-            if ( patIdxEnd != strIdxEnd )
-            {
-                return false; // Pattern and string do not have the same size
-            }
-            for ( int i = 0; i <= patIdxEnd; i++ )
-            {
-                ch = patArr[i];
-                if ( ch != '?' && ch != strArr[i] )
-                {
-                    return false; // Character mismatch
-                }
-            }
-            return true; // String matches against pattern
-        }
-
-        if ( patIdxEnd == 0 )
-        {
-            return true; // Pattern contains only '*', which matches anything
-        }
-
-        // Process characters before first star
-        while ( ( ch = patArr[patIdxStart] ) != '*' && strIdxStart <= strIdxEnd )
-        {
-            if ( ch != '?' && ch != strArr[strIdxStart] )
-            {
-                return false; // Character mismatch
-            }
-            patIdxStart++;
-            strIdxStart++;
-        }
-        if ( strIdxStart > strIdxEnd )
+        requireNonNull( artifact );
+        return coordinate ->
         {
-            // All characters in the string are used. Check if only '*'s are
-            // left in the pattern. If so, we succeeded. Otherwise failure.
-            for ( int i = patIdxStart; i <= patIdxEnd; i++ )
+            requireNonNull( coordinate );
+            switch ( coordinate )
             {
-                if ( patArr[i] != '*' )
-                {
-                    return false;
-                }
+                case GROUP_ID:
+                    return artifact.getGroupId();
+                case ARTIFACT_ID:
+                    return artifact.getArtifactId();
+                case BASE_VERSION:
+                    return artifact.getBaseVersion();
+                case CLASSIFIER:
+                    return artifact.hasClassifier() ? artifact.getClassifier() : null;
+                case TYPE:
+                    return artifact.getType();
+                default:
             }
-            return true;
-        }
+            throw new IllegalArgumentException( "unknown coordinate: " + coordinate );
+        };
+    }
 
-        // Process characters after last star
-        while ( ( ch = patArr[patIdxEnd] ) != '*' && strIdxStart <= strIdxEnd )
+    private static Artifactoid adapt( final String depTrailString )
+    {
+        requireNonNull( depTrailString );
+        // G:A:T:C:V

Review Comment:
   https://github.com/apache/maven/blob/master/maven-artifact/src/main/java/org/apache/maven/artifact/DefaultArtifact.java#L224-L248
   
   So it is `G:A:T:C:V` or `G:A:T:V`



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


[GitHub] [maven-common-artifact-filters] cstamas commented on pull request #26: [MSHARED-1077] Bugfix for classifier in pattern

Posted by GitBox <gi...@apache.org>.
cstamas commented on PR #26:
URL: https://github.com/apache/maven-common-artifact-filters/pull/26#issuecomment-1152048957

   Aside of build passing above, @gnodet made a nice perf test as well (to prove that his rewrite was better than "old" one). So, this PR continues along (hence, copies the existing code from master as GN prefixed) and now perf tests had 3 classes testing. You can run perf test locally as well to ensure, but locally my results are:
   https://gist.github.com/cstamas/844fbe97bf861ffab045c43e9f734c4a
   
   Legend:
   "old" - in src/test the "old" pattern filter code
   "gn" - this PR moves current master from src/main to src/test the code it replaces
   "new" - the src/main "newest" pattern filter code


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


[GitHub] [maven-common-artifact-filters] michael-o commented on a diff in pull request #26: [MSHARED-1077] Bugfix for classifier in pattern

Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #26:
URL: https://github.com/apache/maven-common-artifact-filters/pull/26#discussion_r894614846


##########
src/test/java/org/apache/maven/shared/artifact/filter/GNPatternIncludesArtifactFilter.java:
##########
@@ -0,0 +1,917 @@
+package org.apache.maven.shared.artifact.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.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Objects;
+import java.util.Set;
+
+import org.apache.maven.artifact.Artifact;
+import org.apache.maven.artifact.resolver.filter.ArtifactFilter;
+import org.apache.maven.artifact.versioning.DefaultArtifactVersion;
+import org.apache.maven.artifact.versioning.InvalidVersionSpecificationException;
+import org.apache.maven.artifact.versioning.VersionRange;
+import org.slf4j.Logger;
+
+/**
+ * TODO: include in maven-artifact in future
+ *
+ * @author <a href="mailto:brett@apache.org">Brett Porter</a>
+ * @see StrictPatternIncludesArtifactFilter
+ */
+public class GNPatternIncludesArtifactFilter
+    implements ArtifactFilter, StatisticsReportingArtifactFilter
+{
+    /** Holds the set of compiled patterns */
+    private final Set<Pattern> patterns;
+
+    /** Whether the dependency trail should be checked */
+    private final boolean actTransitively;
+
+    /** Set of patterns that have been triggered */
+    private final Set<Pattern> patternsTriggered = new HashSet<>();
+
+    /** Set of artifacts that have been filtered out */
+    private final List<Artifact> filteredArtifact = new ArrayList<>();
+
+    /**
+     * <p>Constructor for PatternIncludesArtifactFilter.</p>
+     *
+     * @param patterns The pattern to be used.
+     */
+    public GNPatternIncludesArtifactFilter( final Collection<String> patterns )
+    {
+        this( patterns, false );
+    }
+
+    /**
+     * <p>Constructor for PatternIncludesArtifactFilter.</p>
+     *
+     * @param patterns The pattern to be used.
+     * @param actTransitively transitive yes/no.
+     */
+    public GNPatternIncludesArtifactFilter( final Collection<String> patterns, final boolean actTransitively )
+    {
+        this.actTransitively = actTransitively;
+        final Set<Pattern> pat = new LinkedHashSet<>();
+        if ( patterns != null && !patterns.isEmpty() )
+        {
+            for ( String pattern : patterns )
+            {
+
+                Pattern p = compile( pattern );
+                pat.add( p );
+            }
+        }
+        this.patterns = pat;
+    }
+
+    /** {@inheritDoc} */
+    public boolean include( final Artifact artifact )
+    {
+        final boolean shouldInclude = patternMatches( artifact );
+
+        if ( !shouldInclude )
+        {
+            addFilteredArtifact( artifact );
+        }
+
+        return shouldInclude;
+    }
+
+    /**
+     * <p>patternMatches.</p>
+     *
+     * @param artifact to check for.
+     * @return true if the match is true false otherwise.
+     */
+    protected boolean patternMatches( final Artifact artifact )
+    {
+        // Check if the main artifact matches
+        char[][] artifactGatvCharArray = new char[][] {
+                            emptyOrChars( artifact.getGroupId() ),
+                            emptyOrChars( artifact.getArtifactId() ),
+                            emptyOrChars( artifact.getType() ),
+                            emptyOrChars( artifact.getClassifier() ),
+                            emptyOrChars( artifact.getBaseVersion() )
+                    };
+        Boolean match = match( artifactGatvCharArray );
+        if ( match != null )
+        {
+            return match;
+        }
+
+        if ( actTransitively )
+        {
+            final List<String> depTrail = artifact.getDependencyTrail();
+
+            if ( depTrail != null && depTrail.size() > 1 )
+            {
+                for ( String trailItem : depTrail )
+                {
+                    char[][] depGatvCharArray = tokenizeAndSplit( trailItem );
+                    match = match( depGatvCharArray );
+                    if ( match != null )
+                    {
+                        return match;
+                    }
+                }
+            }
+        }
+
+        return false;
+    }
+
+    private Boolean match( char[][] gatvCharArray )
+    {
+        for ( Pattern pattern : patterns )
+        {
+            if ( pattern.matches( gatvCharArray ) )
+            {
+                patternsTriggered.add( pattern );
+                return !( pattern instanceof NegativePattern );
+            }
+        }
+
+        return null;
+    }
+
+    /**
+     * <p>addFilteredArtifact.</p>
+     *
+     * @param artifact add artifact to the filtered artifacts list.
+     */
+    protected void addFilteredArtifact( final Artifact artifact )
+    {
+        filteredArtifact.add( artifact );
+    }
+
+    /** {@inheritDoc} */
+    public void reportMissedCriteria( final Logger logger )
+    {
+        // if there are no patterns, there is nothing to report.
+        if ( !patterns.isEmpty() )
+        {
+            final List<Pattern> missed = new ArrayList<>( patterns );
+            missed.removeAll( patternsTriggered );
+
+            if ( !missed.isEmpty() && logger.isWarnEnabled() )
+            {
+                final StringBuilder buffer = new StringBuilder();
+
+                buffer.append( "The following patterns were never triggered in this " );
+                buffer.append( getFilterDescription() );
+                buffer.append( ':' );
+
+                for ( Pattern pattern : missed )
+                {
+                    buffer.append( "\no  '" ).append( pattern ).append( "'" );
+                }
+
+                buffer.append( "\n" );

Review Comment:
   This is a redundant LS since logger will add it anyway, no?



##########
src/test/java/org/apache/maven/shared/artifact/filter/GNPatternIncludesArtifactFilter.java:
##########
@@ -0,0 +1,917 @@
+package org.apache.maven.shared.artifact.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.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Objects;
+import java.util.Set;
+
+import org.apache.maven.artifact.Artifact;
+import org.apache.maven.artifact.resolver.filter.ArtifactFilter;
+import org.apache.maven.artifact.versioning.DefaultArtifactVersion;
+import org.apache.maven.artifact.versioning.InvalidVersionSpecificationException;
+import org.apache.maven.artifact.versioning.VersionRange;
+import org.slf4j.Logger;
+
+/**
+ * TODO: include in maven-artifact in future
+ *
+ * @author <a href="mailto:brett@apache.org">Brett Porter</a>
+ * @see StrictPatternIncludesArtifactFilter
+ */
+public class GNPatternIncludesArtifactFilter
+    implements ArtifactFilter, StatisticsReportingArtifactFilter
+{
+    /** Holds the set of compiled patterns */
+    private final Set<Pattern> patterns;
+
+    /** Whether the dependency trail should be checked */
+    private final boolean actTransitively;
+
+    /** Set of patterns that have been triggered */
+    private final Set<Pattern> patternsTriggered = new HashSet<>();
+
+    /** Set of artifacts that have been filtered out */
+    private final List<Artifact> filteredArtifact = new ArrayList<>();
+
+    /**
+     * <p>Constructor for PatternIncludesArtifactFilter.</p>
+     *
+     * @param patterns The pattern to be used.
+     */
+    public GNPatternIncludesArtifactFilter( final Collection<String> patterns )
+    {
+        this( patterns, false );
+    }
+
+    /**
+     * <p>Constructor for PatternIncludesArtifactFilter.</p>
+     *
+     * @param patterns The pattern to be used.
+     * @param actTransitively transitive yes/no.
+     */
+    public GNPatternIncludesArtifactFilter( final Collection<String> patterns, final boolean actTransitively )
+    {
+        this.actTransitively = actTransitively;
+        final Set<Pattern> pat = new LinkedHashSet<>();
+        if ( patterns != null && !patterns.isEmpty() )
+        {
+            for ( String pattern : patterns )
+            {
+
+                Pattern p = compile( pattern );
+                pat.add( p );
+            }
+        }
+        this.patterns = pat;
+    }
+
+    /** {@inheritDoc} */
+    public boolean include( final Artifact artifact )
+    {
+        final boolean shouldInclude = patternMatches( artifact );
+
+        if ( !shouldInclude )
+        {
+            addFilteredArtifact( artifact );
+        }
+
+        return shouldInclude;
+    }
+
+    /**
+     * <p>patternMatches.</p>
+     *
+     * @param artifact to check for.
+     * @return true if the match is true false otherwise.
+     */
+    protected boolean patternMatches( final Artifact artifact )
+    {
+        // Check if the main artifact matches
+        char[][] artifactGatvCharArray = new char[][] {
+                            emptyOrChars( artifact.getGroupId() ),
+                            emptyOrChars( artifact.getArtifactId() ),
+                            emptyOrChars( artifact.getType() ),
+                            emptyOrChars( artifact.getClassifier() ),
+                            emptyOrChars( artifact.getBaseVersion() )
+                    };
+        Boolean match = match( artifactGatvCharArray );
+        if ( match != null )
+        {
+            return match;
+        }
+
+        if ( actTransitively )
+        {
+            final List<String> depTrail = artifact.getDependencyTrail();
+
+            if ( depTrail != null && depTrail.size() > 1 )
+            {
+                for ( String trailItem : depTrail )
+                {
+                    char[][] depGatvCharArray = tokenizeAndSplit( trailItem );
+                    match = match( depGatvCharArray );
+                    if ( match != null )
+                    {
+                        return match;
+                    }
+                }
+            }
+        }
+
+        return false;
+    }
+
+    private Boolean match( char[][] gatvCharArray )
+    {
+        for ( Pattern pattern : patterns )
+        {
+            if ( pattern.matches( gatvCharArray ) )
+            {
+                patternsTriggered.add( pattern );
+                return !( pattern instanceof NegativePattern );
+            }
+        }
+
+        return null;
+    }
+
+    /**
+     * <p>addFilteredArtifact.</p>
+     *
+     * @param artifact add artifact to the filtered artifacts list.
+     */
+    protected void addFilteredArtifact( final Artifact artifact )
+    {
+        filteredArtifact.add( artifact );
+    }
+
+    /** {@inheritDoc} */
+    public void reportMissedCriteria( final Logger logger )
+    {
+        // if there are no patterns, there is nothing to report.
+        if ( !patterns.isEmpty() )
+        {
+            final List<Pattern> missed = new ArrayList<>( patterns );
+            missed.removeAll( patternsTriggered );
+
+            if ( !missed.isEmpty() && logger.isWarnEnabled() )
+            {
+                final StringBuilder buffer = new StringBuilder();
+
+                buffer.append( "The following patterns were never triggered in this " );
+                buffer.append( getFilterDescription() );
+                buffer.append( ':' );
+
+                for ( Pattern pattern : missed )
+                {
+                    buffer.append( "\no  '" ).append( pattern ).append( "'" );
+                }
+
+                buffer.append( "\n" );
+
+                logger.warn( buffer.toString() );
+            }
+        }
+    }
+
+    /** {@inheritDoc} */
+    @Override
+    public String toString()
+    {
+        return "Includes filter:" + getPatternsAsString();
+    }
+
+    /**
+     * <p>getPatternsAsString.</p>
+     *
+     * @return pattern as a string.
+     */
+    protected String getPatternsAsString()
+    {
+        final StringBuilder buffer = new StringBuilder();
+        for ( Pattern pattern : patterns )
+        {
+            buffer.append( "\no '" ).append( pattern ).append( "'" );

Review Comment:
   `System.lineSeparator()`



##########
src/main/java/org/apache/maven/shared/artifact/filter/PatternIncludesArtifactFilter.java:
##########
@@ -315,480 +230,270 @@ public void reportFilteredArtifacts( final Logger logger )
         }
     }
 
-    /**
-     * {@inheritDoc}
-     *
-     * @return a boolean.
-     */
+    @Override
     public boolean hasMissedCriteria()
     {
         // if there are no patterns, there is nothing to report.
         if ( !patterns.isEmpty() )
         {
             final List<Pattern> missed = new ArrayList<>( patterns );
             missed.removeAll( patternsTriggered );
-
             return !missed.isEmpty();
         }
 
         return false;
     }
 
-    private static final char[] EMPTY = new char[0];
-
-    private static final char[] ANY = new char[] { '*' };
-
-    static char[] emptyOrChars( String str )
+    private enum Coordinate
     {
-        return str != null && str.length() > 0 ? str.toCharArray() : EMPTY;
+        GROUP_ID, ARTIFACT_ID, TYPE, CLASSIFIER, BASE_VERSION
     }
 
-    static char[] anyOrChars( char[] str )
+    private interface Artifactoid
     {
-        return str.length > 1 || ( str.length == 1 && str[0] != '*' ) ? str : ANY;
+        String getCoordinate( Coordinate coordinate );
     }
 
-    static char[][] tokenizeAndSplit( String pattern )
+    private static Artifactoid adapt( final Artifact artifact )
     {
-        String[] stokens = pattern.split( ":" );
-        char[][] tokens = new char[ stokens.length ][];
-        for ( int i = 0; i < stokens.length; i++ )
+        requireNonNull( artifact );
+        return coordinate ->
         {
-            tokens[i] = emptyOrChars( stokens[i] );
-        }
-        return tokens;
-    }
-
-    @SuppressWarnings( "InnerAssignment" )
-    static boolean match( char[] patArr, char[] strArr, boolean isVersion )
-    {
-        int patIdxStart = 0;
-        int patIdxEnd = patArr.length - 1;
-        int strIdxStart = 0;
-        int strIdxEnd = strArr.length - 1;
-        char ch;
-
-        boolean containsStar = false;
-        for ( char aPatArr : patArr )
-        {
-            if ( aPatArr == '*' )
-            {
-                containsStar = true;
-                break;
-            }
-        }
-
-        if ( !containsStar )
-        {
-            if ( isVersion && ( patArr[0] == '[' || patArr[0] == '(' ) )
+            requireNonNull( coordinate );
+            switch ( coordinate )
             {
-                return isVersionIncludedInRange( String.valueOf( strArr ), String.valueOf( patArr ) );
+                case GROUP_ID:
+                    return artifact.getGroupId();
+                case ARTIFACT_ID:
+                    return artifact.getArtifactId();
+                case BASE_VERSION:
+                    return artifact.getBaseVersion();
+                case CLASSIFIER:
+                    return artifact.hasClassifier() ? artifact.getClassifier() : null;
+                case TYPE:
+                    return artifact.getType();
+                default:
             }
-            // No '*'s, so we make a shortcut
-            if ( patIdxEnd != strIdxEnd )
-            {
-                return false; // Pattern and string do not have the same size
-            }
-            for ( int i = 0; i <= patIdxEnd; i++ )
-            {
-                ch = patArr[i];
-                if ( ch != '?' && ch != strArr[i] )
-                {
-                    return false; // Character mismatch
-                }
-            }
-            return true; // String matches against pattern
-        }
-
-        if ( patIdxEnd == 0 )
-        {
-            return true; // Pattern contains only '*', which matches anything
-        }
-
-        // Process characters before first star
-        while ( ( ch = patArr[patIdxStart] ) != '*' && strIdxStart <= strIdxEnd )
-        {
-            if ( ch != '?' && ch != strArr[strIdxStart] )
-            {
-                return false; // Character mismatch
-            }
-            patIdxStart++;
-            strIdxStart++;
-        }
-        if ( strIdxStart > strIdxEnd )
-        {
-            // All characters in the string are used. Check if only '*'s are
-            // left in the pattern. If so, we succeeded. Otherwise failure.
-            for ( int i = patIdxStart; i <= patIdxEnd; i++ )
-            {
-                if ( patArr[i] != '*' )
-                {
-                    return false;
-                }
-            }
-            return true;
-        }
+            throw new IllegalArgumentException( "unknown coordinate: " + coordinate );
+        };
+    }
 
-        // Process characters after last star
-        while ( ( ch = patArr[patIdxEnd] ) != '*' && strIdxStart <= strIdxEnd )
+    private static Artifactoid adapt( final String depTrailString )
+    {
+        requireNonNull( depTrailString );
+        // G:A:T:C:V
+        String[] coordinates = depTrailString.split( ":" );
+        if ( coordinates.length != 4 && coordinates.length != 5 )
         {
-            if ( ch != '?' && ch != strArr[strIdxEnd] )
-            {
-                return false; // Character mismatch
-            }
-            patIdxEnd--;
-            strIdxEnd--;
+            throw new IllegalArgumentException( "Bad dep trail string: " + depTrailString );
         }
-        if ( strIdxStart > strIdxEnd )
+        final HashMap<Coordinate, String> map = new HashMap<>();
+        map.put( Coordinate.GROUP_ID, coordinates[0] );
+        map.put( Coordinate.ARTIFACT_ID, coordinates[1] );
+        map.put( Coordinate.TYPE, coordinates[2] );
+        if ( coordinates.length == 5 )
         {
-            // All characters in the string are used. Check if only '*'s are
-            // left in the pattern. If so, we succeeded. Otherwise failure.
-            for ( int i = patIdxStart; i <= patIdxEnd; i++ )
-            {
-                if ( patArr[i] != '*' )
-                {
-                    return false;
-                }
-            }
-            return true;
+            map.put( Coordinate.CLASSIFIER, coordinates[3] );
+            map.put( Coordinate.BASE_VERSION, coordinates[4] );
         }
-
-        // process pattern between stars. padIdxStart and patIdxEnd point
-        // always to a '*'.
-        while ( patIdxStart != patIdxEnd && strIdxStart <= strIdxEnd )
+        else
         {
-            int patIdxTmp = -1;
-            for ( int i = patIdxStart + 1; i <= patIdxEnd; i++ )
-            {
-                if ( patArr[i] == '*' )
-                {
-                    patIdxTmp = i;
-                    break;
-                }
-            }
-            if ( patIdxTmp == patIdxStart + 1 )
-            {
-                // Two stars next to each other, skip the first one.
-                patIdxStart++;
-                continue;
-            }
-            // Find the pattern between padIdxStart & padIdxTmp in str between
-            // strIdxStart & strIdxEnd
-            int patLength = ( patIdxTmp - patIdxStart - 1 );
-            int strLength = ( strIdxEnd - strIdxStart + 1 );
-            int foundIdx = -1;
-            strLoop: for ( int i = 0; i <= strLength - patLength; i++ )
-            {
-                for ( int j = 0; j < patLength; j++ )
-                {
-                    ch = patArr[patIdxStart + j + 1];
-                    if ( ch != '?' && ch != strArr[strIdxStart + i + j] )
-                    {
-                        continue strLoop;
-                    }
-                }
-
-                foundIdx = strIdxStart + i;
-                break;
-            }
-
-            if ( foundIdx == -1 )
-            {
-                return false;
-            }
-
-            patIdxStart = patIdxTmp;
-            strIdxStart = foundIdx + patLength;
+            map.put( Coordinate.BASE_VERSION, coordinates[3] );
         }
 
-        // All characters in the string are used. Check if only '*'s are left
-        // in the pattern. If so, we succeeded. Otherwise failure.
-        for ( int i = patIdxStart; i <= patIdxEnd; i++ )
+        return coordinate ->
         {
-            if ( patArr[i] != '*' )
-            {
-                return false;
-            }
-        }
-        return true;
+            requireNonNull( coordinate );
+            return map.get( coordinate );
+        };
     }
 
-    static boolean isVersionIncludedInRange( final String version, final String range )
+    private static final String ANY = "*";
+
+    /**
+     * Splits the pattern string into tokens, replacing empty tokens with {@link #ANY} for patterns like {@code ::val}
+     * so it retains the position of token.
+     */
+    private static String[] splitAndTokenize( String pattern )
     {
-        try
+        String[] stokens = pattern.split( ":" );
+        String[] tokens = new String[stokens.length];
+        for ( int i = 0; i < stokens.length; i++ )
         {
-            return VersionRange.createFromVersionSpec( range ).containsVersion( new DefaultArtifactVersion( version ) );
-        }
-        catch ( final InvalidVersionSpecificationException e )
-        {
-            return false;
+            String str = stokens[i];
+            tokens[i] = str != null && !str.isEmpty() ? str : ANY;
         }
+        return tokens;
     }
 
-    static Pattern compile( String pattern )
+    private static Pattern compile( String pattern )
     {
         if ( pattern.startsWith( "!" ) )
         {
             return new NegativePattern( pattern, compile( pattern.substring( 1 ) ) );
         }
         else
         {
-            char[][] stokens = tokenizeAndSplit( pattern );
-            char[][] tokens = new char[ stokens.length ][];
-            for ( int i = 0; i < stokens.length; i++ )
-            {
-                tokens[i] = anyOrChars( stokens[i] );
-            }
-            if ( tokens.length > 5 )
+            String[] tokens = splitAndTokenize( pattern );
+            if ( tokens.length < 1 || tokens.length > 5 )
             {
                 throw new IllegalArgumentException( "Invalid pattern: " + pattern );
             }
-            // we only accept 5 tokens if the classifier = '*'
+
+            ArrayList<Pattern> patterns = new ArrayList<>( 5 );
+
             if ( tokens.length == 5 )
             {
-                if ( tokens[3] != ANY )
+                // trivial, full pattern w/ classifier: G:A:T:C:V
+                patterns.add( toPattern( tokens[0], Coordinate.GROUP_ID ) );
+                patterns.add( toPattern( tokens[1], Coordinate.ARTIFACT_ID ) );
+                patterns.add( toPattern( tokens[2], Coordinate.TYPE ) );
+                patterns.add( toPattern( tokens[3], Coordinate.CLASSIFIER ) );
+                patterns.add( toPattern( tokens[4], Coordinate.BASE_VERSION ) );
+            }
+            else if ( tokens.length == 4 )
+            {
+                // trivial, full pattern w/p classifier: G:A:T:V

Review Comment:
   w/o



##########
src/test/java/org/apache/maven/shared/artifact/filter/GNPatternIncludesArtifactFilter.java:
##########
@@ -0,0 +1,917 @@
+package org.apache.maven.shared.artifact.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.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Objects;
+import java.util.Set;
+
+import org.apache.maven.artifact.Artifact;
+import org.apache.maven.artifact.resolver.filter.ArtifactFilter;
+import org.apache.maven.artifact.versioning.DefaultArtifactVersion;
+import org.apache.maven.artifact.versioning.InvalidVersionSpecificationException;
+import org.apache.maven.artifact.versioning.VersionRange;
+import org.slf4j.Logger;
+
+/**
+ * TODO: include in maven-artifact in future
+ *
+ * @author <a href="mailto:brett@apache.org">Brett Porter</a>
+ * @see StrictPatternIncludesArtifactFilter
+ */
+public class GNPatternIncludesArtifactFilter
+    implements ArtifactFilter, StatisticsReportingArtifactFilter
+{
+    /** Holds the set of compiled patterns */
+    private final Set<Pattern> patterns;
+
+    /** Whether the dependency trail should be checked */
+    private final boolean actTransitively;
+
+    /** Set of patterns that have been triggered */
+    private final Set<Pattern> patternsTriggered = new HashSet<>();
+
+    /** Set of artifacts that have been filtered out */
+    private final List<Artifact> filteredArtifact = new ArrayList<>();
+
+    /**
+     * <p>Constructor for PatternIncludesArtifactFilter.</p>
+     *
+     * @param patterns The pattern to be used.
+     */
+    public GNPatternIncludesArtifactFilter( final Collection<String> patterns )
+    {
+        this( patterns, false );
+    }
+
+    /**
+     * <p>Constructor for PatternIncludesArtifactFilter.</p>
+     *
+     * @param patterns The pattern to be used.
+     * @param actTransitively transitive yes/no.
+     */
+    public GNPatternIncludesArtifactFilter( final Collection<String> patterns, final boolean actTransitively )
+    {
+        this.actTransitively = actTransitively;
+        final Set<Pattern> pat = new LinkedHashSet<>();
+        if ( patterns != null && !patterns.isEmpty() )
+        {
+            for ( String pattern : patterns )
+            {
+
+                Pattern p = compile( pattern );
+                pat.add( p );
+            }
+        }
+        this.patterns = pat;
+    }
+
+    /** {@inheritDoc} */
+    public boolean include( final Artifact artifact )
+    {
+        final boolean shouldInclude = patternMatches( artifact );
+
+        if ( !shouldInclude )
+        {
+            addFilteredArtifact( artifact );
+        }
+
+        return shouldInclude;
+    }
+
+    /**
+     * <p>patternMatches.</p>
+     *
+     * @param artifact to check for.
+     * @return true if the match is true false otherwise.
+     */
+    protected boolean patternMatches( final Artifact artifact )
+    {
+        // Check if the main artifact matches
+        char[][] artifactGatvCharArray = new char[][] {
+                            emptyOrChars( artifact.getGroupId() ),
+                            emptyOrChars( artifact.getArtifactId() ),
+                            emptyOrChars( artifact.getType() ),
+                            emptyOrChars( artifact.getClassifier() ),
+                            emptyOrChars( artifact.getBaseVersion() )
+                    };
+        Boolean match = match( artifactGatvCharArray );
+        if ( match != null )
+        {
+            return match;
+        }
+
+        if ( actTransitively )
+        {
+            final List<String> depTrail = artifact.getDependencyTrail();
+
+            if ( depTrail != null && depTrail.size() > 1 )
+            {
+                for ( String trailItem : depTrail )
+                {
+                    char[][] depGatvCharArray = tokenizeAndSplit( trailItem );
+                    match = match( depGatvCharArray );
+                    if ( match != null )
+                    {
+                        return match;
+                    }
+                }
+            }
+        }
+
+        return false;
+    }
+
+    private Boolean match( char[][] gatvCharArray )
+    {
+        for ( Pattern pattern : patterns )
+        {
+            if ( pattern.matches( gatvCharArray ) )
+            {
+                patternsTriggered.add( pattern );
+                return !( pattern instanceof NegativePattern );
+            }
+        }
+
+        return null;
+    }
+
+    /**
+     * <p>addFilteredArtifact.</p>
+     *
+     * @param artifact add artifact to the filtered artifacts list.
+     */
+    protected void addFilteredArtifact( final Artifact artifact )
+    {
+        filteredArtifact.add( artifact );
+    }
+
+    /** {@inheritDoc} */
+    public void reportMissedCriteria( final Logger logger )
+    {
+        // if there are no patterns, there is nothing to report.
+        if ( !patterns.isEmpty() )
+        {
+            final List<Pattern> missed = new ArrayList<>( patterns );
+            missed.removeAll( patternsTriggered );
+
+            if ( !missed.isEmpty() && logger.isWarnEnabled() )
+            {
+                final StringBuilder buffer = new StringBuilder();
+
+                buffer.append( "The following patterns were never triggered in this " );
+                buffer.append( getFilterDescription() );
+                buffer.append( ':' );
+
+                for ( Pattern pattern : missed )
+                {
+                    buffer.append( "\no  '" ).append( pattern ).append( "'" );

Review Comment:
   `System.lineSeparator()`



##########
src/test/java/org/apache/maven/shared/artifact/filter/GNPatternIncludesArtifactFilter.java:
##########
@@ -0,0 +1,917 @@
+package org.apache.maven.shared.artifact.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.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Objects;
+import java.util.Set;
+
+import org.apache.maven.artifact.Artifact;
+import org.apache.maven.artifact.resolver.filter.ArtifactFilter;
+import org.apache.maven.artifact.versioning.DefaultArtifactVersion;
+import org.apache.maven.artifact.versioning.InvalidVersionSpecificationException;
+import org.apache.maven.artifact.versioning.VersionRange;
+import org.slf4j.Logger;
+
+/**
+ * TODO: include in maven-artifact in future
+ *
+ * @author <a href="mailto:brett@apache.org">Brett Porter</a>
+ * @see StrictPatternIncludesArtifactFilter
+ */
+public class GNPatternIncludesArtifactFilter
+    implements ArtifactFilter, StatisticsReportingArtifactFilter
+{
+    /** Holds the set of compiled patterns */
+    private final Set<Pattern> patterns;
+
+    /** Whether the dependency trail should be checked */
+    private final boolean actTransitively;
+
+    /** Set of patterns that have been triggered */
+    private final Set<Pattern> patternsTriggered = new HashSet<>();
+
+    /** Set of artifacts that have been filtered out */
+    private final List<Artifact> filteredArtifact = new ArrayList<>();
+
+    /**
+     * <p>Constructor for PatternIncludesArtifactFilter.</p>
+     *
+     * @param patterns The pattern to be used.
+     */
+    public GNPatternIncludesArtifactFilter( final Collection<String> patterns )
+    {
+        this( patterns, false );
+    }
+
+    /**
+     * <p>Constructor for PatternIncludesArtifactFilter.</p>
+     *
+     * @param patterns The pattern to be used.
+     * @param actTransitively transitive yes/no.
+     */
+    public GNPatternIncludesArtifactFilter( final Collection<String> patterns, final boolean actTransitively )
+    {
+        this.actTransitively = actTransitively;
+        final Set<Pattern> pat = new LinkedHashSet<>();
+        if ( patterns != null && !patterns.isEmpty() )
+        {
+            for ( String pattern : patterns )
+            {
+
+                Pattern p = compile( pattern );
+                pat.add( p );
+            }
+        }
+        this.patterns = pat;
+    }
+
+    /** {@inheritDoc} */
+    public boolean include( final Artifact artifact )
+    {
+        final boolean shouldInclude = patternMatches( artifact );
+
+        if ( !shouldInclude )
+        {
+            addFilteredArtifact( artifact );
+        }
+
+        return shouldInclude;
+    }
+
+    /**
+     * <p>patternMatches.</p>
+     *
+     * @param artifact to check for.
+     * @return true if the match is true false otherwise.
+     */
+    protected boolean patternMatches( final Artifact artifact )
+    {
+        // Check if the main artifact matches
+        char[][] artifactGatvCharArray = new char[][] {
+                            emptyOrChars( artifact.getGroupId() ),
+                            emptyOrChars( artifact.getArtifactId() ),
+                            emptyOrChars( artifact.getType() ),
+                            emptyOrChars( artifact.getClassifier() ),
+                            emptyOrChars( artifact.getBaseVersion() )
+                    };
+        Boolean match = match( artifactGatvCharArray );
+        if ( match != null )
+        {
+            return match;
+        }
+
+        if ( actTransitively )
+        {
+            final List<String> depTrail = artifact.getDependencyTrail();
+
+            if ( depTrail != null && depTrail.size() > 1 )
+            {
+                for ( String trailItem : depTrail )
+                {
+                    char[][] depGatvCharArray = tokenizeAndSplit( trailItem );
+                    match = match( depGatvCharArray );
+                    if ( match != null )
+                    {
+                        return match;
+                    }
+                }
+            }
+        }
+
+        return false;
+    }
+
+    private Boolean match( char[][] gatvCharArray )
+    {
+        for ( Pattern pattern : patterns )
+        {
+            if ( pattern.matches( gatvCharArray ) )
+            {
+                patternsTriggered.add( pattern );
+                return !( pattern instanceof NegativePattern );
+            }
+        }
+
+        return null;
+    }
+
+    /**
+     * <p>addFilteredArtifact.</p>
+     *
+     * @param artifact add artifact to the filtered artifacts list.
+     */
+    protected void addFilteredArtifact( final Artifact artifact )
+    {
+        filteredArtifact.add( artifact );
+    }
+
+    /** {@inheritDoc} */
+    public void reportMissedCriteria( final Logger logger )
+    {
+        // if there are no patterns, there is nothing to report.
+        if ( !patterns.isEmpty() )
+        {
+            final List<Pattern> missed = new ArrayList<>( patterns );
+            missed.removeAll( patternsTriggered );
+
+            if ( !missed.isEmpty() && logger.isWarnEnabled() )
+            {
+                final StringBuilder buffer = new StringBuilder();
+
+                buffer.append( "The following patterns were never triggered in this " );
+                buffer.append( getFilterDescription() );
+                buffer.append( ':' );
+
+                for ( Pattern pattern : missed )
+                {
+                    buffer.append( "\no  '" ).append( pattern ).append( "'" );
+                }
+
+                buffer.append( "\n" );
+
+                logger.warn( buffer.toString() );
+            }
+        }
+    }
+
+    /** {@inheritDoc} */
+    @Override
+    public String toString()
+    {
+        return "Includes filter:" + getPatternsAsString();
+    }
+
+    /**
+     * <p>getPatternsAsString.</p>
+     *
+     * @return pattern as a string.
+     */
+    protected String getPatternsAsString()
+    {
+        final StringBuilder buffer = new StringBuilder();
+        for ( Pattern pattern : patterns )
+        {
+            buffer.append( "\no '" ).append( pattern ).append( "'" );
+        }
+
+        return buffer.toString();
+    }
+
+    /**
+     * <p>getFilterDescription.</p>
+     *
+     * @return description.
+     */
+    protected String getFilterDescription()
+    {
+        return "artifact inclusion filter";
+    }
+
+    /** {@inheritDoc} */
+    public void reportFilteredArtifacts( final Logger logger )
+    {
+        if ( !filteredArtifact.isEmpty() && logger.isDebugEnabled() )
+        {
+            final StringBuilder buffer =
+                new StringBuilder( "The following artifacts were removed by this " + getFilterDescription() + ": " );
+
+            for ( Artifact artifactId : filteredArtifact )
+            {
+                buffer.append( '\n' ).append( artifactId.getId() );

Review Comment:
   `System.lineSeparator()`



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