Simplify file matching to not distinguish between files and directories

README.adoc

@@ -210,24 +210,22 @@ workflows:
         - allow:
             directories:
              - ./src
-             - ./documentation
-             files:
-             - README.md
+             - ./doc*
+             - "**/README.md"
             users:
               minContributions: 5
         unless:
            directories:
-             - ./github
-           files:
-             - bad.xml
+             - ./.github
+             - "**/pom.xml"
 ----
 
 Workflows will be allowed if they meet one of the rules in the `allow` section,
 unless one of the rules in the `unless` section is triggered.
 
 In the example above, any file called `README.md` would be allowed, except for `./github/README.md`.
 Users who had made at least 5 commits to the repository would be allowed to make any changes,
-except to `bad.xml` and any files in `.github`. Other users could make changes to `./src` or `./documentation`.
+except to a `pom.xml` or any files in `.github`. Other users could make changes to `./src` or directories whose name started with `./doc`.
 
 If the rule is triggered, the following actions will be executed:
 

src/main/java/io/quarkus/bot/ApproveWorkflow.java

@@ -132,38 +132,12 @@ public static boolean matchRuleFromChangedFiles(GHPullRequest pullRequest,
             return false;
         }
 
-        boolean matches = false;
-
-        PullRequestFilesMatcher prMatcher = new PullRequestFilesMatcher(pullRequest);
-        if (matchDirectories(prMatcher, rule)) {
-            matches = true;
-        } else if (matchFiles(prMatcher, rule)) {
-            matches = true;
-        }
-
-        return matches;
-    }
-
-    private static boolean matchDirectories(PullRequestFilesMatcher prMatcher,
-            QuarkusGitHubBotConfigFile.WorkflowApprovalCondition rule) {
         if (rule.directories == null || rule.directories.isEmpty()) {
             return false;
         }
-        if (prMatcher.changedFilesMatchDirectory(rule.directories)) {
-            return true;
-        }
-        return false;
-    }
 
-    private static boolean matchFiles(PullRequestFilesMatcher prMatcher,
-            QuarkusGitHubBotConfigFile.WorkflowApprovalCondition rule) {
-        if (rule.files == null || rule.files.isEmpty()) {
-            return false;
-        }
-        if (prMatcher.changedFilesMatchFile(rule.files)) {
-            return true;
-        }
-        return false;
+        PullRequestFilesMatcher prMatcher = new PullRequestFilesMatcher(pullRequest);
+        return prMatcher.changedFilesMatch(rule.directories);
     }
 
     private boolean matchRuleForUser(GHRepositoryStatistics.ContributorStats stats,

src/main/java/io/quarkus/bot/util/PullRequestFilesMatcher.java

@@ -2,6 +2,7 @@
 
 import com.hrakaroo.glob.GlobPattern;
 import com.hrakaroo.glob.MatchingEngine;
+import io.quarkus.cache.CacheResult;
 import org.jboss.logging.Logger;
 import org.kohsuke.github.GHPullRequest;
 import org.kohsuke.github.GHPullRequestFileDetail;
@@ -19,56 +20,34 @@ public PullRequestFilesMatcher(GHPullRequest pullRequest) {
         this.pullRequest = pullRequest;
     }
 
-    public boolean changedFilesMatchDirectory(Collection<String> directories) {
-        for (GHPullRequestFileDetail changedFile : pullRequest.listFiles()) {
-            for (String directory : directories) {
+    public boolean changedFilesMatch(Collection<String> filenamePatterns) {
+        PagedIterable<GHPullRequestFileDetail> prFiles = pullRequest.listFiles();
+        if (prFiles != null) {
+            for (GHPullRequestFileDetail changedFile : prFiles) {
+                for (String filenamePattern : filenamePatterns) {
 
-                if (!directory.contains("*")) {
-                    if (changedFile.getFilename().startsWith(directory)) {
-                        return true;
-                    }
-                } else {
-                    try {
-                        MatchingEngine matchingEngine = GlobPattern.compile(directory);
-                        if (matchingEngine.matches(changedFile.getFilename())) {
+                    if (!filenamePattern.contains("*")) {
+                        if (changedFile.getFilename().startsWith(filenamePattern)) {
                             return true;
                         }
-                    } catch (Exception e) {
-                        LOG.error("Error evaluating glob expression: " + directory, e);
+                    } else {
+                        try {
+                            MatchingEngine matchingEngine = compileGlob(filenamePattern);
+                            if (matchingEngine.matches(changedFile.getFilename())) {
+                                return true;
+                            }
+                        } catch (Exception e) {
+                            LOG.error("Error evaluating glob expression: " + filenamePattern, e);
+                        }
                     }
                 }
             }
         }
         return false;
     }
 
-    public boolean changedFilesMatchFile(Collection<String> files) {
-
-        PagedIterable<GHPullRequestFileDetail> prFiles = pullRequest.listFiles();
-
-        if (prFiles == null || files == null) {
-            return false;
-        }
-
-        for (GHPullRequestFileDetail changedFile : prFiles) {
-            for (String file : files) {
-
-                if (!file.contains("*")) {
-                    if (changedFile.getFilename().endsWith(file)) {
-                        return true;
-                    }
-                } else {
-                    try {
-                        MatchingEngine matchingEngine = GlobPattern.compile(file);
-                        if (matchingEngine.matches(changedFile.getFilename())) {
-                            return true;
-                        }
-                    } catch (Exception e) {
-                        LOG.error("Error evaluating glob expression: " + file, e);
-                    }
-                }
-            }
-        }
-        return false;
+    @CacheResult(cacheName = "glob-cache")
+    MatchingEngine compileGlob(String filenamePattern) {
+        return GlobPattern.compile(filenamePattern);
     }
 }

src/main/java/io/quarkus/bot/util/Triage.java

@@ -83,7 +83,7 @@ public static boolean matchRuleFromChangedFiles(GHPullRequest pullRequest, Triag
         }
 
         PullRequestFilesMatcher prMatcher = new PullRequestFilesMatcher(pullRequest);
-        if (prMatcher.changedFilesMatchDirectory(rule.directories)) {
+        if (prMatcher.changedFilesMatch(rule.directories)) {
             return true;
         }
 

src/test/java/io/quarkus/bot/it/WorkflowApprovalTest.java

@@ -213,8 +213,8 @@ void changeToAnAllowedFileShouldBeApproved() throws Exception {
                             workflows:
                                   rules:
                                     - allow:
-                                        files:
-                                            - pom.xml
+                                        directories:
+                                            - "**/pom.xml"
                                       """);
             setupMockQueriesAndCommits(mocks);
             PagedIterable<GHPullRequestFileDetail> paths = MockHelper
@@ -242,8 +242,8 @@ void changeToAFileInAnAllowedDirectoryWithAnIrrelevantUnlessShouldBeAllowed() th
                                         directories:
                                           - ./src
                                       unless:
-                                        files:
-                                            - bad.xml
+                                        directories:
+                                            - "**/bad.xml"
                                       """);
             setupMockQueriesAndCommits(mocks);
             PagedIterable<GHPullRequestFileDetail> paths = MockHelper
@@ -271,8 +271,8 @@ void changeToAnUnlessedFileInAnAllowedDirectoryShouldBeSoftRejected() throws Exc
                                         directories:
                                           - ./src
                                       unless:
-                                        files:
-                                            - bad.xml
+                                        directories:
+                                            - "**/bad.xml"
                                       """);
             setupMockQueriesAndCommits(mocks);
             PagedIterable<GHPullRequestFileDetail> paths = MockHelper
@@ -375,8 +375,8 @@ void changeFromAnEstablishedUserToADangerousFileShouldBeSoftRejected() throws Ex
                                         users:
                                           minContributions: 5
                                       unless:
-                                        files:
-                                         - bad.xml
+                                        directories:
+                                         - "**/bad.xml"
                               """);
             setupMockQueriesAndCommits(mocks);
             setupMockUsers(mocks);
@@ -405,8 +405,8 @@ void workflowIsPreApprovedShouldDoNothing() throws Exception {
                                     directories:
                                       - ./src
                                   unless:
-                                    files:
-                                      - bad.xml
+                                    directories:
+                                      - "**/bad.xml"
                           """))
                 .when().payloadFromClasspath("/workflow-from-committer.json")
                 .event(GHEvent.WORKFLOW_RUN)