diff --git a/README.adoc b/README.adoc index 9b7235f7..f19b7d3d 100644 --- a/README.adoc +++ b/README.adoc @@ -210,16 +210,14 @@ 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, @@ -227,7 +225,7 @@ 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: diff --git a/src/main/java/io/quarkus/bot/ApproveWorkflow.java b/src/main/java/io/quarkus/bot/ApproveWorkflow.java index eb9bab60..ffc18746 100644 --- a/src/main/java/io/quarkus/bot/ApproveWorkflow.java +++ b/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, diff --git a/src/main/java/io/quarkus/bot/util/PullRequestFilesMatcher.java b/src/main/java/io/quarkus/bot/util/PullRequestFilesMatcher.java index abfac807..3b0706fc 100644 --- a/src/main/java/io/quarkus/bot/util/PullRequestFilesMatcher.java +++ b/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,22 +20,25 @@ public PullRequestFilesMatcher(GHPullRequest pullRequest) { this.pullRequest = pullRequest; } - public boolean changedFilesMatchDirectory(Collection directories) { - for (GHPullRequestFileDetail changedFile : pullRequest.listFiles()) { - for (String directory : directories) { + public boolean changedFilesMatch(Collection filenamePatterns) { + PagedIterable 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); + } } } } @@ -42,33 +46,8 @@ public boolean changedFilesMatchDirectory(Collection directories) { return false; } - public boolean changedFilesMatchFile(Collection files) { - - PagedIterable 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); } } diff --git a/src/main/java/io/quarkus/bot/util/Triage.java b/src/main/java/io/quarkus/bot/util/Triage.java index c816e7e0..b1848d8c 100644 --- a/src/main/java/io/quarkus/bot/util/Triage.java +++ b/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; } diff --git a/src/test/java/io/quarkus/bot/it/WorkflowApprovalTest.java b/src/test/java/io/quarkus/bot/it/WorkflowApprovalTest.java index d0a32db3..68e9dc81 100644 --- a/src/test/java/io/quarkus/bot/it/WorkflowApprovalTest.java +++ b/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 paths = MockHelper @@ -242,8 +242,8 @@ void changeToAFileInAnAllowedDirectoryWithAnIrrelevantUnlessShouldBeAllowed() th directories: - ./src unless: - files: - - bad.xml + directories: + - "**/bad.xml" """); setupMockQueriesAndCommits(mocks); PagedIterable paths = MockHelper @@ -271,8 +271,8 @@ void changeToAnUnlessedFileInAnAllowedDirectoryShouldBeSoftRejected() throws Exc directories: - ./src unless: - files: - - bad.xml + directories: + - "**/bad.xml" """); setupMockQueriesAndCommits(mocks); PagedIterable 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)