elharo commented on code in PR #1200:
URL: https://github.com/apache/maven/pull/1200#discussion_r1264391463


##########
maven-core/src/test/java/org/apache/maven/artifact/resolver/filter/ExclusionArtifactFilterTest.java:
##########
@@ -140,4 +145,15 @@ void testMultipleExclusionsExcludeGroupIdWildcard() {
 
         assertThat(filter.include(artifact), is(false));
     }
+
+    @Test
+    void testExcludeWithGlob() {
+        Exclusion exclusion = new Exclusion();
+        exclusion.setGroupId("*");

Review Comment:
   needs tests for ** as well here, or maybe you're not using globstar? Docs on 
the format are needed here. 



##########
maven-core/src/main/java/org/apache/maven/artifact/resolver/filter/ExclusionArtifactFilter.java:
##########
@@ -28,37 +30,120 @@
  * Filter to exclude from a list of artifact patterns.
  */
 public class ExclusionArtifactFilter implements ArtifactFilter {
-    private static final String WILDCARD = "*";
 
     private final List<Exclusion> exclusions;
+    private final List<Predicate<Artifact>> predicates;
 
     public ExclusionArtifactFilter(List<Exclusion> exclusions) {
         this.exclusions = exclusions;
+        this.predicates =
+                
exclusions.stream().map(ExclusionArtifactFilter::toPredicate).collect(Collectors.toList());
     }
 
-    private Predicate<Exclusion> sameArtifactId(Artifact artifact) {
-        return exclusion -> 
exclusion.getArtifactId().equals(artifact.getArtifactId());
+    @Override
+    public boolean include(Artifact artifact) {
+        return predicates.stream().noneMatch(p -> p.test(artifact));
     }
 
-    private Predicate<Exclusion> sameGroupId(Artifact artifact) {
-        return exclusion -> 
exclusion.getGroupId().equals(artifact.getGroupId());
+    private static Predicate<Artifact> toPredicate(Exclusion exclusion) {
+        Pattern groupId = 
Pattern.compile(convertGlobToRegex(exclusion.getGroupId()));
+        Pattern artifactId = 
Pattern.compile(convertGlobToRegex(exclusion.getArtifactId()));
+        Predicate<Artifact> predGroupId = a -> 
groupId.matcher(a.getGroupId()).matches();
+        Predicate<Artifact> predArtifactId =
+                a -> artifactId.matcher(a.getArtifactId()).matches();
+        return predGroupId.and(predArtifactId);
     }
 
-    private Predicate<Exclusion> groupIdIsWildcard = exclusion -> 
WILDCARD.equals(exclusion.getGroupId());
-
-    private Predicate<Exclusion> artifactIdIsWildcard = exclusion -> 
WILDCARD.equals(exclusion.getArtifactId());
-
-    private Predicate<Exclusion> groupIdAndArtifactIdIsWildcard = 
groupIdIsWildcard.and(artifactIdIsWildcard);
-
-    private Predicate<Exclusion> exclude(Artifact artifact) {
-        return groupIdAndArtifactIdIsWildcard
-                .or(groupIdIsWildcard.and(sameArtifactId(artifact)))
-                .or(artifactIdIsWildcard.and(sameGroupId(artifact)))
-                .or(sameGroupId(artifact).and(sameArtifactId(artifact)));
-    }
-
-    @Override
-    public boolean include(Artifact artifact) {
-        return !exclusions.stream().anyMatch(exclude(artifact));
+    /**
+     * Converts a standard POSIX Shell globbing pattern into a regular 
expression

Review Comment:
   I agree we shouldn't add a dependency on maven-shared-utils, but I do think 
PathMatcher is the way to go. This code is very complex, and if we can push 
concerns about its correctness into the JDK that's a big win, even if we have 
to do a but of hacking to adapt the string to paths.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to