From 7b1d9ce1e44a17d716ed116de504f5528cc54cdc Mon Sep 17 00:00:00 2001 From: Marek Skacelik Date: Wed, 1 Jul 2026 09:32:31 +0200 Subject: [PATCH 1/5] Rename format check message to failMessage --- .../bot/PullRequestFormatProcessor.java | 4 +-- .../bot/format/CommitMessagesCheck.java | 6 ++-- .../wildfly/bot/format/DescriptionCheck.java | 10 +++--- .../org/wildfly/bot/format/TitleCheck.java | 6 ++-- .../org/wildfly/bot/model/Description.java | 2 +- .../java/org/wildfly/bot/model/Format.java | 6 ++-- .../wildfly/bot/model/RegexDefinition.java | 6 ++-- .../java/org/wildfly/bot/PRChecksTest.java | 4 +-- .../org/wildfly/bot/PRCommitCheckTest.java | 4 +-- .../org/wildfly/bot/PRDependabotTest.java | 2 +- .../wildfly/bot/PRDescriptionCheckTest.java | 32 +++++++++---------- .../org/wildfly/bot/PRTitleCheckTest.java | 2 +- 12 files changed, 42 insertions(+), 42 deletions(-) diff --git a/src/main/java/org/wildfly/bot/PullRequestFormatProcessor.java b/src/main/java/org/wildfly/bot/PullRequestFormatProcessor.java index 6e31d481..5c39b374 100644 --- a/src/main/java/org/wildfly/bot/PullRequestFormatProcessor.java +++ b/src/main/java/org/wildfly/bot/PullRequestFormatProcessor.java @@ -150,12 +150,12 @@ private List initializeChecks(WildFlyConfigFile wildflyConfigFile) { if (wildflyConfigFile.wildfly.format.title.enabled) { checks.add(new TitleCheck(new RegexDefinition(wildflyConfigFile.wildfly.getProjectPattern(), - wildflyConfigFile.wildfly.format.title.message))); + wildflyConfigFile.wildfly.format.title.failMessage))); } if (wildflyConfigFile.wildfly.format.commit.enabled) { checks.add(new CommitMessagesCheck(new RegexDefinition(wildflyConfigFile.wildfly.getProjectPattern(), - wildflyConfigFile.wildfly.format.commit.message))); + wildflyConfigFile.wildfly.format.commit.failMessage))); } if (wildflyConfigFile.wildfly.format.description != null) { diff --git a/src/main/java/org/wildfly/bot/format/CommitMessagesCheck.java b/src/main/java/org/wildfly/bot/format/CommitMessagesCheck.java index b2e51771..4d45685f 100644 --- a/src/main/java/org/wildfly/bot/format/CommitMessagesCheck.java +++ b/src/main/java/org/wildfly/bot/format/CommitMessagesCheck.java @@ -16,14 +16,14 @@ public class CommitMessagesCheck implements Check { private static final String ELLIPSIS = "..."; private final Pattern pattern; - private final String message; + private final String failMessage; public CommitMessagesCheck(RegexDefinition description) { if (description.pattern == null) { throw new IllegalArgumentException("Input argument cannot be null"); } pattern = description.pattern; - message = description.message; + failMessage = description.failMessage; } @Override @@ -50,7 +50,7 @@ public String check(GHPullRequest pullRequest) throws IOException { } } if (!oneMatched) { - return formatMessageWithDetailsIfNeeded(String.format(this.message, pattern.pattern())); + return formatMessageWithDetailsIfNeeded(String.format(this.failMessage, pattern.pattern())); } } diff --git a/src/main/java/org/wildfly/bot/format/DescriptionCheck.java b/src/main/java/org/wildfly/bot/format/DescriptionCheck.java index 37e2d6be..85d8715b 100644 --- a/src/main/java/org/wildfly/bot/format/DescriptionCheck.java +++ b/src/main/java/org/wildfly/bot/format/DescriptionCheck.java @@ -11,7 +11,7 @@ public class DescriptionCheck implements Check { static final String DEFAULT_MESSAGE = "Invalid description content"; private Description description; - private String message = DEFAULT_MESSAGE; + private String failMessage = DEFAULT_MESSAGE; public DescriptionCheck(Description description) { if (description == null) { @@ -23,8 +23,8 @@ public DescriptionCheck(Description description) { throw new IllegalArgumentException("Input argument cannot be null"); } - if (description.message != null) { - message = description.message; + if (description.failMessage != null) { + failMessage = description.failMessage; } } @@ -47,11 +47,11 @@ public String check(GHPullRequest pullRequest) { } if (!regexMatched) { - return regexDefinition.message != null ? regexDefinition.message : message; + return regexDefinition.failMessage != null ? regexDefinition.failMessage : failMessage; } } } catch (NullPointerException e) { - return message; + return failMessage; } return null; diff --git a/src/main/java/org/wildfly/bot/format/TitleCheck.java b/src/main/java/org/wildfly/bot/format/TitleCheck.java index 5cc65271..a74641b8 100644 --- a/src/main/java/org/wildfly/bot/format/TitleCheck.java +++ b/src/main/java/org/wildfly/bot/format/TitleCheck.java @@ -9,20 +9,20 @@ public class TitleCheck implements Check { private final Pattern pattern; - private final String message; + private final String failMessage; public TitleCheck(RegexDefinition title) { if (title.pattern == null) { throw new IllegalArgumentException("Input argument cannot be null"); } pattern = title.pattern; - message = title.message; + failMessage = title.failMessage; } @Override public String check(GHPullRequest pullRequest) { if (!Patterns.matches(pattern, pullRequest.getTitle())) { - return message.formatted(pattern.pattern()); + return failMessage.formatted(pattern.pattern()); } return null; diff --git a/src/main/java/org/wildfly/bot/model/Description.java b/src/main/java/org/wildfly/bot/model/Description.java index a7a42925..5b8965a6 100644 --- a/src/main/java/org/wildfly/bot/model/Description.java +++ b/src/main/java/org/wildfly/bot/model/Description.java @@ -3,6 +3,6 @@ import java.util.List; public class Description { - public String message; + public String failMessage; public List regexes; } diff --git a/src/main/java/org/wildfly/bot/model/Format.java b/src/main/java/org/wildfly/bot/model/Format.java index a1037ceb..0c946825 100644 --- a/src/main/java/org/wildfly/bot/model/Format.java +++ b/src/main/java/org/wildfly/bot/model/Format.java @@ -17,11 +17,11 @@ public final class Format { public CommitPattern commit = new CommitPattern(); public abstract static class RegexPattern { - public String message; + public String failMessage; public boolean enabled = true; - public RegexPattern(String message) { - this.message = message; + public RegexPattern(String failMessage) { + this.failMessage = failMessage; } } diff --git a/src/main/java/org/wildfly/bot/model/RegexDefinition.java b/src/main/java/org/wildfly/bot/model/RegexDefinition.java index 6a3c1d40..dd6ee143 100644 --- a/src/main/java/org/wildfly/bot/model/RegexDefinition.java +++ b/src/main/java/org/wildfly/bot/model/RegexDefinition.java @@ -5,13 +5,13 @@ public class RegexDefinition { public Pattern pattern; - public String message; + public String failMessage; public RegexDefinition() { } - public RegexDefinition(Pattern pattern, String message) { + public RegexDefinition(Pattern pattern, String failMessage) { this.pattern = pattern; - this.message = message; + this.failMessage = failMessage; } } diff --git a/src/test/java/org/wildfly/bot/PRChecksTest.java b/src/test/java/org/wildfly/bot/PRChecksTest.java index c252ca06..3f1a2b34 100644 --- a/src/test/java/org/wildfly/bot/PRChecksTest.java +++ b/src/test/java/org/wildfly/bot/PRChecksTest.java @@ -71,7 +71,7 @@ void testAllChecksPass() throws Throwable { description: regexes: - pattern: "JIRA:\\\\s+https://redhat.atlassian.net/browse/WFLY-\\\\d+|https://redhat.atlassian.net/browse/WFLY-\\\\d+" - message: "The PR description must contain a link to the JIRA issue" + failMessage: "The PR description must contain a link to the JIRA issue" """; pullRequestJson = TestModel.getPullRequestJson(); @@ -100,7 +100,7 @@ void testAllChecksFail() throws Throwable { description: regexes: - pattern: "JIRA:\\\\s+https://redhat.atlassian.net/browse/WFLY-\\\\d+|https://redhat.atlassian.net/browse/WFLY-\\\\d+" - message: "The PR description must contain a link to the JIRA issue" + failMessage: "The PR description must contain a link to the JIRA issue" """; pullRequestJson = TestModel.setPullRequestJsonBuilder( pullRequestJsonBuilder -> pullRequestJsonBuilder.title(TestConstants.INVALID_TITLE) diff --git a/src/test/java/org/wildfly/bot/PRCommitCheckTest.java b/src/test/java/org/wildfly/bot/PRCommitCheckTest.java index f66f1373..cb8b8f10 100644 --- a/src/test/java/org/wildfly/bot/PRCommitCheckTest.java +++ b/src/test/java/org/wildfly/bot/PRCommitCheckTest.java @@ -169,7 +169,7 @@ public void testOverridingCommitMessage() throws Throwable { wildfly: format: commit: - message: "Lorem ipsum dolor sit amet" + failMessage: "Lorem ipsum dolor sit amet" """; mockedContext = MockedGHPullRequest.builder(pullRequestJson.id()) @@ -211,7 +211,7 @@ public void testLongErrorMessageTruncationInFailedCommitCheck() throws Throwable wildfly: format: commit: - message: "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789" + failMessage: "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789" """; mockedContext = MockedGHPullRequest.builder(pullRequestJson.id()) .commit(INVALID_COMMIT_MESSAGE); diff --git a/src/test/java/org/wildfly/bot/PRDependabotTest.java b/src/test/java/org/wildfly/bot/PRDependabotTest.java index 5b49aabb..ac64f188 100644 --- a/src/test/java/org/wildfly/bot/PRDependabotTest.java +++ b/src/test/java/org/wildfly/bot/PRDependabotTest.java @@ -52,7 +52,7 @@ void testDependabotOnPREdited() throws Throwable { description: regexes: - pattern: "https://redhat.atlassian.net/browse/WFLY-\\\\d+" - message: "The PR description must contain a link to the JIRA issue" + failMessage: "The PR description must contain a link to the JIRA issue" """; pullRequestJson = TestModel.setPullRequestJsonBuilder(pullRequestJsonBuilder -> pullRequestJsonBuilder diff --git a/src/test/java/org/wildfly/bot/PRDescriptionCheckTest.java b/src/test/java/org/wildfly/bot/PRDescriptionCheckTest.java index 8901b21f..187efda8 100644 --- a/src/test/java/org/wildfly/bot/PRDescriptionCheckTest.java +++ b/src/test/java/org/wildfly/bot/PRDescriptionCheckTest.java @@ -49,10 +49,10 @@ void testDescriptionCheckFail() throws Throwable { wildfly: format: description: - message: Default fail message + failMessage: Default fail message regexes: - pattern: "https://redhat.atlassian.net/browse/WFLY-\\\\d+" - message: "The PR description must contain a link to the JIRA issue" + failMessage: "The PR description must contain a link to the JIRA issue" """; pullRequestJson = TestModel.setPullRequestJsonBuilder( @@ -97,10 +97,10 @@ void testDescriptionCheckSuccess() throws Throwable { wildfly: format: description: - message: Default fail message + failMessage: Default fail message regexes: - pattern: "https://redhat.atlassian.net/browse/WFLY-\\\\d+" - message: "The PR description must contain a link to the JIRA issue" + failMessage: "The PR description must contain a link to the JIRA issue" """; pullRequestJson = TestModel.getPullRequestJson(); @@ -137,10 +137,10 @@ void testMultipleLineDescription() throws Throwable { wildfly: format: description: - message: Default fail message + failMessage: Default fail message regexes: - pattern: "https://redhat.atlassian.net/browse/WFLY-\\\\d+" - message: "The PR description must contain a link to the JIRA issue" + failMessage: "The PR description must contain a link to the JIRA issue" """; pullRequestJson = TestModel.setPullRequestJsonBuilder(pullRequestJsonBuilder -> pullRequestJsonBuilder @@ -164,10 +164,10 @@ void testMultipleRegexesFirstPatternHit() throws Throwable { wildfly: format: description: - message: Default fail message + failMessage: Default fail message regexes: - pattern: "https://redhat.atlassian.net/browse/WFLY-\\\\d+" - message: "The PR description must contain a link to the JIRA issue" + failMessage: "The PR description must contain a link to the JIRA issue" - pattern: "JIRA" """; @@ -190,10 +190,10 @@ void testMultipleRegexesSecondPatternHit() throws Throwable { wildfly: format: description: - message: Lorem ipsum dolor sit amet + failMessage: Lorem ipsum dolor sit amet regexes: - pattern: "https://redhat.atlassian.net/browse/WFLY-\\\\d+" - message: "The PR description must contain a link to the JIRA issue" + failMessage: "The PR description must contain a link to the JIRA issue" - pattern: "JIRA" """; @@ -216,10 +216,10 @@ void testMultipleRegexesAllPatternsHit() throws Throwable { wildfly: format: description: - message: Default fail message + failMessage: Default fail message regexes: - pattern: "https://redhat.atlassian.net/browse/WFLY-\\\\d+" - message: "The PR description must contain a link to the JIRA issue" + failMessage: "The PR description must contain a link to the JIRA issue" - pattern: "JIRA" """; @@ -242,10 +242,10 @@ void testMultipleRegexesNoPatternsHit() throws Throwable { wildfly: format: description: - message: Default fail message + failMessage: Default fail message regexes: - pattern: "https://redhat.atlassian.net/browse/WFLY-\\\\d+" - message: "The PR description must contain a link to the JIRA issue" + failMessage: "The PR description must contain a link to the JIRA issue" - pattern: "JIRA" """; @@ -266,10 +266,10 @@ public void testDisableGlobalFormatCheck() throws Throwable { format: enabled: false description: - message: Default fail message + failMessage: Default fail message regexes: - pattern: "https://redhat.atlassian.net/browse/WFLY-\\\\d+" - message: "The PR description must contain a link to the JIRA issue" + failMessage: "The PR description must contain a link to the JIRA issue" - pattern: "JIRA" """; diff --git a/src/test/java/org/wildfly/bot/PRTitleCheckTest.java b/src/test/java/org/wildfly/bot/PRTitleCheckTest.java index aa7fabbc..571c30ae 100644 --- a/src/test/java/org/wildfly/bot/PRTitleCheckTest.java +++ b/src/test/java/org/wildfly/bot/PRTitleCheckTest.java @@ -302,7 +302,7 @@ public void testOverridingTitleMessage() throws Throwable { wildfly: format: title: - message: "Custom title message" + failMessage: "Custom title message" """; pullRequestJson = TestModel.setPullRequestJsonBuilder( pullRequestJsonBuilder -> pullRequestJsonBuilder.title(TestConstants.INVALID_TITLE)); From 767bea8f6bc1878ea357b92d1bd57907380a82aa Mon Sep 17 00:00:00 2001 From: Marek Skacelik Date: Wed, 1 Jul 2026 09:46:41 +0200 Subject: [PATCH 2/5] Remove bot command for skiping format checks --- .../org/wildfly/bot/util/GithubProcessor.java | 14 -------- .../wildfly/bot/PRSkipPullRequestTest.java | 29 ----------------- .../bot/PRUpdateCommentOnEditTest.java | 32 ------------------- 3 files changed, 75 deletions(-) diff --git a/src/main/java/org/wildfly/bot/util/GithubProcessor.java b/src/main/java/org/wildfly/bot/util/GithubProcessor.java index bf0614a4..55587394 100644 --- a/src/main/java/org/wildfly/bot/util/GithubProcessor.java +++ b/src/main/java/org/wildfly/bot/util/GithubProcessor.java @@ -2,7 +2,6 @@ import io.quarkus.mailer.Mail; import io.quarkus.mailer.Mailer; -import jakarta.annotation.PostConstruct; import jakarta.enterprise.context.Dependent; import jakarta.inject.Inject; import org.eclipse.microprofile.config.inject.ConfigProperty; @@ -35,7 +34,6 @@ import java.util.Random; import java.util.SequencedMap; import java.util.Set; -import java.util.regex.Pattern; import java.util.stream.Collectors; import java.util.LinkedHashMap; @@ -56,7 +54,6 @@ public class GithubProcessor { private static final Logger LOG_DELEGATE = Logger.getLogger(GithubProcessor.class); public final PullRequestLogger LOG = new PullRequestLogger(LOG_DELEGATE); - private Pattern SKIP_FORMAT_COMMAND; @Inject WildFlyBotConfig wildFlyBotConfig; @@ -70,12 +67,6 @@ public class GithubProcessor { @ConfigProperty(name = "quarkus.mailer.username") Optional username; - @PostConstruct - void construct() { - SKIP_FORMAT_COMMAND = Pattern.compile("@%s skip format".formatted(botContextProvider.getBotName()), - Pattern.DOTALL | Pattern.LITERAL); - } - public void commitStatusSuccess(GHPullRequest pullRequest, String checkName, String description) throws IOException { String sha = pullRequest.getHead().getSha(); @@ -277,11 +268,6 @@ public void sendEmail(String subject, String body, List emails) { } public String skipPullRequest(GHPullRequest pullRequest) throws IOException { - String body = pullRequest.getBody(); - if (body != null && SKIP_FORMAT_COMMAND.matcher(body).find()) { - return "skip format command found"; - } - if (pullRequest.isDraft()) { return "pull request being a draft"; } diff --git a/src/test/java/org/wildfly/bot/PRSkipPullRequestTest.java b/src/test/java/org/wildfly/bot/PRSkipPullRequestTest.java index bff95b20..ca9f59d1 100644 --- a/src/test/java/org/wildfly/bot/PRSkipPullRequestTest.java +++ b/src/test/java/org/wildfly/bot/PRSkipPullRequestTest.java @@ -61,34 +61,6 @@ static void setPullRequestJson() throws Exception { TestModel.defaultBeforeEachJsons(); } - @Test - void testSkippingFormatCheck() throws Throwable { - pullRequestJson = TestModel.setPullRequestJsonBuilder(pullRequestJsonBuilder -> pullRequestJsonBuilder - .title(TestConstants.INVALID_TITLE) - .description(""" - Hi - - - line start @wildfly-bot[bot] skip format random things - - to pass on my local env @wildfly-github-bot-fork[bot] skip format thanks - - finished""")); - mockedContext = MockedGHPullRequest.builder(pullRequestJson.id()).commit(TestConstants.INVALID_COMMIT_MESSAGE); - - TestModel.given(mocks -> WildflyGitHubBotTesting.mockRepo(mocks, wildflyConfigFile, pullRequestJson, mockedContext)) - .pullRequestEvent(pullRequestJson) - .then(mocks -> { - verify(mocks.pullRequest(pullRequestJson.id()), times(2)).getBody(); - verify(mocks.pullRequest(pullRequestJson.id())).listFiles(); - verify(mocks.pullRequest(pullRequestJson.id())).listComments(); - // Following invocations are used for logging - verify(mocks.pullRequest(pullRequestJson.id()), times(2)).getNumber(); - // commit status should not be set - verifyNoMoreInteractions(mocks.pullRequest(pullRequestJson.id())); - }); - } - @Test void testSkippingFormatCheckOnDraft() throws Throwable { pullRequestJson = TestModel.setPullRequestJsonBuilder( @@ -99,7 +71,6 @@ void testSkippingFormatCheckOnDraft() throws Throwable { mocks -> WildflyGitHubBotTesting.mockRepo(mocks, wildflyConfigFile, pullRequestJson, mockedContext)) .pullRequestEvent(pullRequestJson) .then(mocks -> { - verify(mocks.pullRequest(pullRequestJson.id()), times(2)).getBody(); verify(mocks.pullRequest(pullRequestJson.id())).listFiles(); verify(mocks.pullRequest(pullRequestJson.id()), times(2)).isDraft(); verify(mocks.pullRequest(pullRequestJson.id())).listComments(); diff --git a/src/test/java/org/wildfly/bot/PRUpdateCommentOnEditTest.java b/src/test/java/org/wildfly/bot/PRUpdateCommentOnEditTest.java index 965a099c..7c73e6e2 100644 --- a/src/test/java/org/wildfly/bot/PRUpdateCommentOnEditTest.java +++ b/src/test/java/org/wildfly/bot/PRUpdateCommentOnEditTest.java @@ -12,8 +12,6 @@ import org.wildfly.bot.utils.WildflyGitHubBotTesting; import org.wildfly.bot.utils.mocking.Mockable; import org.wildfly.bot.utils.mocking.MockedGHPullRequest; -import org.wildfly.bot.utils.mocking.MockedGHRepository; -import org.wildfly.bot.utils.model.Action; import org.wildfly.bot.utils.testing.PullRequestJson; import org.wildfly.bot.utils.testing.internal.TestModel; @@ -76,34 +74,4 @@ void testUpdateToValid() throws Throwable { }); } - @Test - void testRemoveCommentAndUpdateCommitStatusOnEditToSkipFormatCheck() throws Throwable { - wildflyConfigFile = """ - wildfly: - format: - """; - pullRequestJson = TestModel - .setPullRequestJsonBuilder(pullRequestJsonBuilder -> pullRequestJsonBuilder.title(INVALID_TITLE) - .description("@%s skip format".formatted(botContextProvider.getBotName())) - .action(Action.EDITED)); - mockedContext = MockedGHPullRequest.builder(pullRequestJson.id()) - .comment(FAILED_FORMAT_COMMENT.formatted(Stream.of( - DEFAULT_COMMIT_MESSAGE.formatted(PROJECT_PATTERN_REGEX.formatted("WFLY")), - DEFAULT_TITLE_MESSAGE.formatted(PROJECT_PATTERN_REGEX.formatted("WFLY")), - "The PR description must contain a link to the JIRA issue") - .map("- %s"::formatted) - .collect(Collectors.joining("\n\n"))), botContextProvider.getBotName()) - .mockNext(MockedGHRepository.builder()) - .commitStatuses(pullRequestJson.commitSHA(), "Format") - .commitStatusCreator(botContextProvider.getBotName()); - - TestModel.given( - mocks -> WildflyGitHubBotTesting.mockRepo(mocks, wildflyConfigFile, pullRequestJson, mockedContext)) - .pullRequestEvent(pullRequestJson) - .then(mocks -> { - GHIssueComment comment = mocks.issueComment(0); - Mockito.verify(comment).delete(); - WildflyGitHubBotTesting.verifyFormatSkipped(mocks.repository(TEST_REPO), pullRequestJson); - }); - } } From d6b50a2320bb29eaad4aa2f4f13e148342fde8d4 Mon Sep 17 00:00:00 2001 From: Marek Skacelik Date: Wed, 1 Jul 2026 13:55:43 +0200 Subject: [PATCH 3/5] Rename LOG to logger and refactor utility methods --- .../bot/ConfigFileChangeProcessor.java | 17 ++- .../org/wildfly/bot/InstallationObserver.java | 6 +- .../org/wildfly/bot/LifecycleProcessor.java | 59 +++++---- .../bot/PullRequestReviewProcessor.java | 22 +--- .../bot/util/GitHubBotContextProvider.java | 2 - .../org/wildfly/bot/util/GithubProcessor.java | 105 +++++++-------- .../wildfly/bot/util/PullRequestLogger.java | 6 +- .../util/PullRequestMergableProcessor.java | 122 +++++++++--------- 8 files changed, 160 insertions(+), 179 deletions(-) diff --git a/src/main/java/org/wildfly/bot/ConfigFileChangeProcessor.java b/src/main/java/org/wildfly/bot/ConfigFileChangeProcessor.java index 4701d815..55bbaec4 100644 --- a/src/main/java/org/wildfly/bot/ConfigFileChangeProcessor.java +++ b/src/main/java/org/wildfly/bot/ConfigFileChangeProcessor.java @@ -11,7 +11,6 @@ import org.wildfly.bot.util.PullRequestLogger; import jakarta.enterprise.context.RequestScoped; import jakarta.inject.Inject; -import org.jboss.logging.Logger; import org.kohsuke.github.GHContent; import org.kohsuke.github.GHEventPayload; import org.kohsuke.github.GHFileNotFoundException; @@ -33,11 +32,11 @@ public class ConfigFileChangeProcessor { private static final String CHECK_NAME = "Configuration File"; - private static final Logger LOG_DELEGATE = Logger.getLogger(ConfigFileChangeProcessor.class); - private final PullRequestLogger LOG = new PullRequestLogger(LOG_DELEGATE); private static final String WARN_RULE = "[WARN] - %s"; private static final String ERROR_RULE = "[ERROR] - %s"; + private final PullRequestLogger logger = PullRequestLogger.getLogger(ConfigFileChangeProcessor.class); + @Inject GitHubConfigFileProviderImpl fileProvider; @@ -52,7 +51,7 @@ void onFileChanged( @PullRequest.Opened @PullRequest.Edited @PullRequest.Synchronize @PullRequest.Reopened @PullRequest.ReadyForReview GHEventPayload.PullRequest pullRequestPayload, GitHub gitHub) throws IOException { GHPullRequest pullRequest = pullRequestPayload.getPullRequest(); - LOG.setPullRequest(pullRequest); + logger.setPullRequest(pullRequest); GHRepository repository = pullRequest.getRepository(); for (GHPullRequestFileDetail changedFile : pullRequest.listFiles()) { @@ -69,20 +68,20 @@ void onFileChanged( List problems = validateFile(file.get(), repository); if (problems.isEmpty()) { githubProcessor.commitStatusSuccess(pullRequest, CHECK_NAME, "Valid"); - LOG.info("Configuration File check successful"); + logger.info("Configuration File check successful"); } else { githubProcessor.commitStatusError(pullRequest, CHECK_NAME, "One or multiple rules are invalid, please see the comment stating the problems"); - LOG.warnf("Configuration File check unsuccessful. %s", String.join(",", problems)); + logger.warnf("Configuration File check unsuccessful. %s", String.join(",", problems)); } githubProcessor.formatComment(pullRequest, RuntimeConstants.FAILED_CONFIGFILE_COMMENT, problems); } else { String message = "Configuration File check unsuccessful. Unable to correctly map loaded file to YAML."; githubProcessor.commitStatusError(pullRequest, CHECK_NAME, message); - LOG.debugf(message); + logger.debugf(message); } } catch (JsonProcessingException e) { - LOG.errorf(e, "Unable to parse the configuration file from the repository %s", + logger.errorf(e, "Unable to parse the configuration file from the repository %s", pullRequest.getHead().getRepository().getFullName()); githubProcessor.commitStatusError(pullRequest, CHECK_NAME, "Unable to parse the configuration file. " + "Make sure it can be loaded to model at https://github.com/wildfly/wildfly-github-bot/blob/main/CONFIGURATION.yml"); @@ -131,7 +130,7 @@ List validateFile(WildFlyConfigFile file, GHRepository repository) throw "Server returned HTTP response code: 200, message: 'null' for URL: https://api.github.com/repos/"))) { problems.add(ERROR_RULE.formatted("Rule [" + rule.toPrettyString() + "] has the following non-existing directory specified: " + directory)); - LOG.debugf(e, "Exception on directories check caught"); + logger.debugf(e, "Exception on directories check caught"); } } } diff --git a/src/main/java/org/wildfly/bot/InstallationObserver.java b/src/main/java/org/wildfly/bot/InstallationObserver.java index ea998a49..8ad68a3b 100644 --- a/src/main/java/org/wildfly/bot/InstallationObserver.java +++ b/src/main/java/org/wildfly/bot/InstallationObserver.java @@ -12,21 +12,21 @@ @ApplicationScoped public class InstallationObserver { - private static final Logger LOG = Logger.getLogger(InstallationObserver.class); + private static final Logger logger = Logger.getLogger(InstallationObserver.class); @Inject GitHubBotContextProvider botContextProvider; void suspendedInstallation(@Installation.Suspend GHEventPayload.Installation installationPayload) { GHAppInstallation installation = installationPayload.getInstallation(); - LOG.infof( + logger.infof( "%s has been suspended for following installation id: %d and will not be able to listen for any incoming Events.", botContextProvider.getBotName(), installation.getAppId()); } void unsuspendedInstallation(@Installation.Unsuspend GHEventPayload.Installation installationPayload) { GHAppInstallation installation = installationPayload.getInstallation(); - LOG.infof( + logger.infof( "%s has been unsuspended for following installation id: %d and has started to listen for new incoming Events.", botContextProvider.getBotName(), installation.getAppId()); } diff --git a/src/main/java/org/wildfly/bot/LifecycleProcessor.java b/src/main/java/org/wildfly/bot/LifecycleProcessor.java index fe68bc2b..be73323f 100644 --- a/src/main/java/org/wildfly/bot/LifecycleProcessor.java +++ b/src/main/java/org/wildfly/bot/LifecycleProcessor.java @@ -4,7 +4,6 @@ import io.quarkiverse.githubapp.GitHubClientProvider; import io.quarkiverse.githubapp.GitHubConfigFileProvider; -import io.quarkus.logging.Log; import io.quarkus.runtime.StartupEvent; import org.wildfly.bot.config.WildFlyBotConfig; import org.wildfly.bot.model.RuntimeConstants; @@ -14,7 +13,6 @@ import jakarta.enterprise.context.ApplicationScoped; import jakarta.enterprise.event.Observes; -import jakarta.inject.Inject; import org.jboss.logging.Logger; import org.kohsuke.github.GHAppInstallation; import org.kohsuke.github.GHRepository; @@ -40,29 +38,30 @@ public class LifecycleProcessor { --- This is a generated message, please do not respond."""; - private static final Logger LOG = Logger.getLogger(LifecycleProcessor.class); - - @Inject - WildFlyBotConfig wildFlyBotConfig; - - @Inject - GitHubClientProvider clientProvider; - - @Inject - GitHubConfigFileProvider fileProvider; - - @Inject - GitHubBotContextProvider botContextProvider; - - @Inject - ConfigFileChangeProcessor configFileChangeProcessor; - - @Inject - GithubProcessor githubProcessor; + private static final Logger logger = Logger.getLogger(LifecycleProcessor.class); + + private final WildFlyBotConfig wildFlyBotConfig; + private final GitHubClientProvider clientProvider; + private final GitHubConfigFileProvider fileProvider; + private final GitHubBotContextProvider botContextProvider; + private final ConfigFileChangeProcessor configFileChangeProcessor; + private final GithubProcessor githubProcessor; + + public LifecycleProcessor(WildFlyBotConfig wildFlyBotConfig, GitHubClientProvider clientProvider, + GitHubConfigFileProvider fileProvider, GitHubBotContextProvider botContextProvider, + ConfigFileChangeProcessor configFileChangeProcessor, GithubProcessor githubProcessor) { + // constructor injection + this.wildFlyBotConfig = wildFlyBotConfig; + this.clientProvider = clientProvider; + this.fileProvider = fileProvider; + this.botContextProvider = botContextProvider; + this.configFileChangeProcessor = configFileChangeProcessor; + this.githubProcessor = githubProcessor; + } void onStart(@Observes StartupEvent event) { if (wildFlyBotConfig.isDryRun()) { - Log.info(RuntimeConstants.DRY_RUN_PREPEND + logger.info(RuntimeConstants.DRY_RUN_PREPEND .formatted("GitHub requests will only log statements and not send actual requests to GitHub.")); } @@ -87,10 +86,10 @@ void onStart(@Observes StartupEvent event) { Set.of(RuntimeConstants.LABEL_NEEDS_REBASE, RuntimeConstants.LABEL_FIX_ME)); if (problems.isEmpty()) { - LOG.infof("The configuration file from the repository %s was parsed successfully.", + logger.infof("The configuration file from the repository %s was parsed successfully.", repository.getFullName()); } else { - LOG.errorf( + logger.errorf( "The configuration file from the repository %s was not parsed successfully due to following problems: %s", repository.getFullName(), problems); githubProcessor.sendEmail( @@ -100,25 +99,25 @@ void onStart(@Observes StartupEvent event) { emailAddresses); } } catch (IllegalStateException e) { - LOG.errorf(e, "Unable to retrieve or parse the configuration file from the repository %s", + logger.errorf(e, "Unable to retrieve or parse the configuration file from the repository %s", repository.getFullName()); } } } } catch (IOException | IllegalStateException e) { if (e instanceof IOException) { - LOG.warnf(e, "Unable to verify rules in repository."); + logger.warnf(e, "Unable to verify rules in repository."); } else { if (e.getCause() instanceof HttpException && e.getCause() != null && e.getCause().getMessage().contains("suspended")) { - LOG.warnf( + logger.warnf( "Your installation has been suspended. No events will be received until you unsuspend the github app installation."); } else { - LOG.errorf(e, "%s is unable to start.", botContextProvider.getBotName()); + logger.errorf(e, "%s is unable to start.", botContextProvider.getBotName()); } } - LOG.errorf(e, "Unable to correctly start %s for following installation id [%d]", botContextProvider.getBotName(), - installationId); + logger.errorf(e, "Unable to correctly start %s for following installation id [%d]", + botContextProvider.getBotName(), installationId); } } diff --git a/src/main/java/org/wildfly/bot/PullRequestReviewProcessor.java b/src/main/java/org/wildfly/bot/PullRequestReviewProcessor.java index 7dce16b9..9b61f96d 100644 --- a/src/main/java/org/wildfly/bot/PullRequestReviewProcessor.java +++ b/src/main/java/org/wildfly/bot/PullRequestReviewProcessor.java @@ -3,11 +3,9 @@ import io.quarkiverse.githubapp.event.PullRequestReview; import org.wildfly.bot.config.WildFlyBotConfig; import org.wildfly.bot.model.RuntimeConstants; -import org.wildfly.bot.util.GithubProcessor; import org.wildfly.bot.util.PullRequestLogger; import jakarta.enterprise.context.RequestScoped; import jakarta.inject.Inject; -import org.jboss.logging.Logger; import org.kohsuke.github.GHEventPayload; import org.kohsuke.github.GHPullRequest; import org.kohsuke.github.GHPullRequestReview; @@ -19,12 +17,7 @@ @RequestScoped public class PullRequestReviewProcessor { - private static final Logger LOG_DELEGATE = Logger.getLogger(PullRequestReviewProcessor.class); - - private final PullRequestLogger LOG = new PullRequestLogger(LOG_DELEGATE); - - @Inject - GithubProcessor githubProcessor; + private final PullRequestLogger logger = PullRequestLogger.getLogger(PullRequestReviewProcessor.class); @Inject WildFlyBotConfig wildFlyBotConfig; @@ -34,21 +27,20 @@ void pullRequestReviewCheck( throws IOException { GHPullRequestReview pullRequestReview = pullRequestPayload.getReview(); GHPullRequest pullRequest = pullRequestPayload.getPullRequest(); - LOG.setPullRequest(pullRequest); - githubProcessor.LOG.setPullRequest(pullRequest); + logger.setPullRequest(pullRequest); - String message = githubProcessor.skipPullRequest(pullRequest); - if (message != null) { - LOG.infof("Skipping format due to %s", message); + if (pullRequest.isDraft()) { + logger.info("Skipping review handling due to pull request being a draft"); return; } if (pullRequestReview.getState() == CHANGES_REQUESTED) { if (pullRequest.getLabels().stream() .noneMatch(ghLabel -> ghLabel.getName().equals(RuntimeConstants.LABEL_FIX_ME))) { - LOG.infof("Changes requested, applying following labels: %s.", RuntimeConstants.LABEL_FIX_ME); + logger.infof("Changes requested, applying following labels: %s.", RuntimeConstants.LABEL_FIX_ME); if (wildFlyBotConfig.isDryRun()) { - LOG.infof(RuntimeConstants.DRY_RUN_PREPEND.formatted("The following labels have been applied: %s"), + logger.infof( + RuntimeConstants.DRY_RUN_PREPEND.formatted("The following labels have been applied: %s"), RuntimeConstants.LABEL_FIX_ME); } else { pullRequest.addLabels(RuntimeConstants.LABEL_FIX_ME); diff --git a/src/main/java/org/wildfly/bot/util/GitHubBotContextProvider.java b/src/main/java/org/wildfly/bot/util/GitHubBotContextProvider.java index 627804e8..18d6c14b 100644 --- a/src/main/java/org/wildfly/bot/util/GitHubBotContextProvider.java +++ b/src/main/java/org/wildfly/bot/util/GitHubBotContextProvider.java @@ -2,7 +2,6 @@ import io.quarkiverse.githubapp.GitHubClientProvider; import jakarta.enterprise.context.ApplicationScoped; -import jakarta.inject.Inject; import org.jboss.logging.Logger; import java.io.IOException; @@ -20,7 +19,6 @@ public class GitHubBotContextProvider { private final String botName; - @Inject public GitHubBotContextProvider(GitHubClientProvider clientProvider) { this.botName = initializeBotName(clientProvider) + BOT_SUFFIX; LOG.infof("GitHub Bot name initialized to: %s", this.botName); diff --git a/src/main/java/org/wildfly/bot/util/GithubProcessor.java b/src/main/java/org/wildfly/bot/util/GithubProcessor.java index 55587394..b6fde299 100644 --- a/src/main/java/org/wildfly/bot/util/GithubProcessor.java +++ b/src/main/java/org/wildfly/bot/util/GithubProcessor.java @@ -3,9 +3,7 @@ import io.quarkus.mailer.Mail; import io.quarkus.mailer.Mailer; import jakarta.enterprise.context.Dependent; -import jakarta.inject.Inject; import org.eclipse.microprofile.config.inject.ConfigProperty; -import org.jboss.logging.Logger; import org.kohsuke.github.GHCommit; import org.kohsuke.github.GHCommitQueryBuilder; import org.kohsuke.github.GHCommitState; @@ -21,8 +19,6 @@ import org.kohsuke.github.PagedIterable; import org.wildfly.bot.config.WildFlyBotConfig; import org.wildfly.bot.model.RuntimeConstants; -import org.wildfly.bot.model.WildFlyConfigFile; - import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; @@ -41,7 +37,6 @@ public class GithubProcessor { public static final String COLLABORATOR_MISSING_SUBJECT = "Missing collaborator in the %s repository"; - public static final String COLLABORATOR_MISSING_BODY = """ Hello, @@ -52,26 +47,27 @@ public class GithubProcessor { --- This is generated message, please do not respond."""; - private static final Logger LOG_DELEGATE = Logger.getLogger(GithubProcessor.class); - public final PullRequestLogger LOG = new PullRequestLogger(LOG_DELEGATE); - - @Inject - WildFlyBotConfig wildFlyBotConfig; - - @Inject - GitHubBotContextProvider botContextProvider; + public final PullRequestLogger logger = PullRequestLogger.getLogger(GithubProcessor.class); - @Inject - Mailer mailer; + private final WildFlyBotConfig wildFlyBotConfig; + private final GitHubBotContextProvider botContextProvider; + private final Mailer mailer; @ConfigProperty(name = "quarkus.mailer.username") Optional username; + public GithubProcessor(WildFlyBotConfig wildFlyBotConfig, GitHubBotContextProvider botContextProvider, Mailer mailer) { + // constructor injection + this.wildFlyBotConfig = wildFlyBotConfig; + this.botContextProvider = botContextProvider; + this.mailer = mailer; + } + public void commitStatusSuccess(GHPullRequest pullRequest, String checkName, String description) throws IOException { String sha = pullRequest.getHead().getSha(); if (wildFlyBotConfig.isDryRun()) { - LOG.infof(RuntimeConstants.DRY_RUN_PREPEND.formatted("Commit status success {%s, %s, %s}"), sha, checkName, + logger.infof(RuntimeConstants.DRY_RUN_PREPEND.formatted("Commit status success {%s, %s, %s}"), sha, checkName, description); } else { pullRequest.getRepository().createCommitStatus(sha, GHCommitState.SUCCESS, "", description, checkName); @@ -82,7 +78,7 @@ public void commitStatusError(GHPullRequest pullRequest, String checkName, Strin String sha = pullRequest.getHead().getSha(); if (wildFlyBotConfig.isDryRun()) { - LOG.infof(RuntimeConstants.DRY_RUN_PREPEND.formatted("Commit status failure {%s, %s, %s}"), sha, checkName, + logger.infof(RuntimeConstants.DRY_RUN_PREPEND.formatted("Commit status failure {%s, %s, %s}"), sha, checkName, description); } else { pullRequest.getRepository().createCommitStatus(sha, GHCommitState.ERROR, "", description, checkName); @@ -104,17 +100,17 @@ public void processNotifies(GHPullRequest pullRequest, GitHub gitHub, .map(GHPerson::getLogin) .toList(); - LOG.infof("Current reviewers already added to the PR: %s", currentReviewers); + logger.infof("Current reviewers already added to the PR: %s", currentReviewers); currentReviewers.forEach(reviewers::remove); - LOG.infof("Reviewers to be added to the PR: %s", reviewers); + logger.infof("Reviewers to be added to the PR: %s", reviewers); updateCCMentions(pullRequest, ccMentionsWithRules); if (!reviewers.isEmpty()) { if (wildFlyBotConfig.isDryRun()) { - LOG.infof(RuntimeConstants.DRY_RUN_PREPEND.formatted("PR review requested from \"%s\""), + logger.infof(RuntimeConstants.DRY_RUN_PREPEND.formatted("PR review requested from \"%s\""), String.join(",", reviewers)); } else { List failedReviewers = new ArrayList<>(); @@ -125,12 +121,12 @@ public void processNotifies(GHPullRequest pullRequest, GitHub gitHub, } catch (HttpException e) { String responseMessage = e.getResponseMessage() != null ? e.getResponseMessage() : "No response message available."; - LOG.warnf( + logger.warnf( "Failed to add reviewer '%s'. Error: %s, Response: %s", requestedReviewer, e.getMessage(), responseMessage); failedReviewers.add(requestedReviewer); } catch (RuntimeException e) { - LOG.warnf( + logger.warnf( "Unexpected error while adding reviewer '%s'. Error: %s", requestedReviewer, e.getMessage()); failedReviewers.add(requestedReviewer); @@ -143,7 +139,8 @@ public void processNotifies(GHPullRequest pullRequest, GitHub gitHub, // Check if actually failedReviewers are not collaborators for (String failedReviewer : failedReviewers) { if (repository.isCollaborator(gitHub.getUser(failedReviewer))) { - LOG.warnf("Reviewer '%s' failed to be added, but they are a collaborator in the '%s' repository.", + logger.warnf( + "Reviewer '%s' failed to be added, but they are a collaborator in the '%s' repository.", failedReviewer, repository.getFullName()); } } @@ -152,14 +149,14 @@ public void processNotifies(GHPullRequest pullRequest, GitHub gitHub, .map(GHPerson::getLogin) .toList(); - LOG.infof("Final reviewers added to the PR: %s", finalRequestedReviewers); + logger.infof("Final reviewers added to the PR: %s", finalRequestedReviewers); // Remove successfully added reviewers from the failed list failedReviewers.removeAll(finalRequestedReviewers); // Log the actual failed reviewers that were not added if (!failedReviewers.isEmpty()) { - LOG.warnf("Bot failed to request PR review from the following people: %s", failedReviewers); + logger.warnf("Bot failed to request PR review from the following people: %s", failedReviewers); sendEmail( COLLABORATOR_MISSING_SUBJECT.formatted(repository.getFullName()), @@ -167,7 +164,7 @@ public void processNotifies(GHPullRequest pullRequest, GitHub gitHub, failedReviewers), emails); } else { - LOG.info("All initially failed reviewers were successfully added to the PR after verification."); + logger.info("All initially failed reviewers were successfully added to the PR after verification."); } } } @@ -181,7 +178,7 @@ private void updateCCMentions(GHPullRequest pullRequest, SequencedMap la .toList(); if (!missingLabels.isEmpty()) { - LOG.debugf("The following labels will be created as they are not in the repository [%s] %s.", repository.getName(), + logger.debugf("The following labels will be created as they are not in the repository [%s] %s.", + repository.getName(), missingLabels); for (String name : missingLabels) { String color = String.format("%06x", new Random().nextInt(0xffffff + 1)); @@ -256,33 +255,17 @@ public void createLabelsIfMissing(GHRepository repository, Collection la public void sendEmail(String subject, String body, List emails) { if (username.isPresent() && emails != null && !emails.isEmpty()) { - LOG.infof("Sending email to the following emails [%s].", String.join(", ", emails)); + logger.infof("Sending email to the following emails [%s].", String.join(", ", emails)); mailer.send( new Mail() .setSubject(subject) .setText(body) .setTo(emails)); } else { - LOG.debug("No emails setup to receive warnings or no email address setup to send emails from."); + logger.debug("No emails setup to receive warnings or no email address setup to send emails from."); } } - public String skipPullRequest(GHPullRequest pullRequest) throws IOException { - if (pullRequest.isDraft()) { - return "pull request being a draft"; - } - - return null; - } - - public String skipPullRequest(GHPullRequest pullRequest, WildFlyConfigFile wildFlyConfigFile) throws IOException { - if (wildFlyConfigFile == null) { - return "no configuration file found"; - } - - return skipPullRequest(pullRequest); - } - /** * This method might get executed in parallel, thus we prepend Pull Request specific info * instead of relying on the {@code org.wildfly.bot.util.PullRequestLogger#setPullRequest} @@ -300,12 +283,12 @@ public void updateLabels(GHPullRequest pullRequest, List labelsToAdd, Li if (!labelsToAdd.isEmpty()) { String logMessage = "Adding the following labels: %s".formatted(labelsToAdd); - if (!LOG.isPullRequestSet()) { + if (!logger.isPullRequestSet()) { logMessage = "Pull Request [#%d] - %s".formatted(pullRequest.getNumber(), logMessage); } - LOG.info(logMessage); + logger.info(logMessage); if (wildFlyBotConfig.isDryRun()) { - LOG.infof(RuntimeConstants.DRY_RUN_PREPEND.formatted("Added the following labels: %s"), labelsToAdd); + logger.infof(RuntimeConstants.DRY_RUN_PREPEND.formatted("Added the following labels: %s"), labelsToAdd); } else { pullRequest.addLabels(labelsToAdd.toArray(String[]::new)); } @@ -313,12 +296,13 @@ public void updateLabels(GHPullRequest pullRequest, List labelsToAdd, Li if (!labelsToRemove.isEmpty()) { String logMessage = "Removing the following labels: %s".formatted(labelsToRemove); - if (!LOG.isPullRequestSet()) { + if (!logger.isPullRequestSet()) { logMessage = "Pull Request [#%d] - %s".formatted(pullRequest.getNumber(), logMessage); } - LOG.info(logMessage); + logger.info(logMessage); if (wildFlyBotConfig.isDryRun()) { - LOG.infof(RuntimeConstants.DRY_RUN_PREPEND.formatted("Removed the following labels: %s"), labelsToRemove); + logger.infof(RuntimeConstants.DRY_RUN_PREPEND.formatted("Removed the following labels: %s"), + labelsToRemove); } else { pullRequest.removeLabels(labelsToRemove.toArray(String[]::new)); } @@ -329,7 +313,8 @@ public void deleteFormatComment(GHPullRequest pullRequest, String commentBody) t formatComment(pullRequest, commentBody, null); } - public void formatComment(GHPullRequest pullRequest, String commentBody, Collection errors) throws IOException { + public void formatComment(GHPullRequest pullRequest, String commentBody, Collection errors) + throws IOException { boolean update = false; String firstLine = commentBody.split("\n")[0]; for (GHIssueComment comment : pullRequest.listComments()) { @@ -337,7 +322,7 @@ public void formatComment(GHPullRequest pullRequest, String commentBody, Collect && comment.getBody().startsWith(firstLine)) { if (errors == null || errors.isEmpty()) { if (wildFlyBotConfig.isDryRun()) { - LOG.infof(RuntimeConstants.DRY_RUN_PREPEND.formatted("Delete comment %s"), comment); + logger.infof(RuntimeConstants.DRY_RUN_PREPEND.formatted("Delete comment %s"), comment); } else { comment.delete(); } @@ -350,8 +335,8 @@ public void formatComment(GHPullRequest pullRequest, String commentBody, Collect .collect(Collectors.joining("\n\n"))); if (wildFlyBotConfig.isDryRun()) { - LOG.infof(RuntimeConstants.DRY_RUN_PREPEND.formatted("Update comment \"%s\" to \"%s\""), comment.getBody(), - updatedBody); + logger.infof(RuntimeConstants.DRY_RUN_PREPEND.formatted("Update comment \"%s\" to \"%s\""), + comment.getBody(), updatedBody); } else { comment.update(updatedBody); } @@ -365,7 +350,7 @@ public void formatComment(GHPullRequest pullRequest, String commentBody, Collect .map("- %s"::formatted) .collect(Collectors.joining("\n\n"))); if (wildFlyBotConfig.isDryRun()) { - LOG.infof(RuntimeConstants.DRY_RUN_PREPEND.formatted("Add new comment %s"), updatedBody); + logger.infof(RuntimeConstants.DRY_RUN_PREPEND.formatted("Add new comment %s"), updatedBody); } else { pullRequest.comment(updatedBody); } @@ -399,7 +384,7 @@ public boolean hasDuplicateCommitInBase(GHPullRequest pullRequest, GHRepository for (GHPullRequestCommitDetail prCommit : pullRequest.listCommits()) { String prSha = prCommit.getSha(); if (baseCommitSHAs.contains(prSha)) { - LOG.infof("Skipping rules due to incorrect rebase detected: commit %s is already in the base branch %s", + logger.infof("Skipping rules due to incorrect rebase detected: commit %s is already in the base branch %s", prSha, baseBranch); return true; diff --git a/src/main/java/org/wildfly/bot/util/PullRequestLogger.java b/src/main/java/org/wildfly/bot/util/PullRequestLogger.java index 74d09a58..da7c23dd 100644 --- a/src/main/java/org/wildfly/bot/util/PullRequestLogger.java +++ b/src/main/java/org/wildfly/bot/util/PullRequestLogger.java @@ -7,7 +7,11 @@ public class PullRequestLogger { private final Logger delegate; private GHPullRequest pullRequest; - public PullRequestLogger(Logger delegate) { + public static PullRequestLogger getLogger(Class clazz) { + return new PullRequestLogger(Logger.getLogger(clazz)); + } + + private PullRequestLogger(Logger delegate) { this.delegate = delegate; } diff --git a/src/main/java/org/wildfly/bot/util/PullRequestMergableProcessor.java b/src/main/java/org/wildfly/bot/util/PullRequestMergableProcessor.java index 935c9a63..5791b52c 100644 --- a/src/main/java/org/wildfly/bot/util/PullRequestMergableProcessor.java +++ b/src/main/java/org/wildfly/bot/util/PullRequestMergableProcessor.java @@ -2,7 +2,6 @@ import io.smallrye.mutiny.Uni; import io.smallrye.mutiny.infrastructure.Infrastructure; -import jakarta.inject.Inject; import jakarta.inject.Singleton; import org.jboss.logging.Logger; import org.kohsuke.github.GHEventPayload; @@ -32,7 +31,7 @@ *

* In case a re-queried pull request fails, it will be only logged. *

- * Note: Do not call githubProcessor.LOG.setPullRequest inside parallel + * Note: Do not call githubProcessor.logger.setPullRequest inside parallel * Uni-s, i.e. inside the parameter `uniToExecute` in method * {@code combineUnis(Function> uniToExecute)} * due to possible race condition of the {@code org.wildfly.bot.util.PullRequestLogger}. @@ -40,61 +39,18 @@ @Singleton public class PullRequestMergableProcessor { - private static final Logger LOGGER = Logger.getLogger(PullRequestMergableProcessor.class); + private static final Logger logger = Logger.getLogger(PullRequestMergableProcessor.class); private static final Queue>> pushPayloadsQueue = new LinkedList<>(); - private boolean currentlyExecuting = false; - - @Inject - GithubProcessor githubProcessor; - @Inject - WildFlyBotConfig wildFlyBotConfig; - - private final Function> pollGitHub = pullRequest -> Uni.createFrom() - .item(pullRequest) - .invoke(pullRequest1 -> { - try { - if (wildFlyBotConfig.isDryRun()) { - LOGGER.info( - RuntimeConstants.DRY_RUN_PREPEND.formatted("Sending a request to GitHub for mergeable status")); - } else { - pullRequest1.getMergeable(); - } - } catch (IOException e) { - throw new RuntimeException(e); - } - }) - .runSubscriptionOn(Infrastructure.getDefaultWorkerPool()); + private boolean currentlyExecuting = false; - private final Function> applyLabels = pullRequest -> { - try { - List labelsToAdd = new ArrayList<>(); - List labelsToRemove = new ArrayList<>(); - if (wildFlyBotConfig.isDryRun()) { - LOGGER.info(RuntimeConstants.DRY_RUN_PREPEND - .formatted("Retrieving mergeable status and then we would apply labels accordingly")); - } else { - Optional mergeable = Optional.ofNullable(pullRequest.getMergeable()); - if (mergeable.isPresent()) { - if (mergeable.get()) { - labelsToRemove.add(RuntimeConstants.LABEL_NEEDS_REBASE); - } else { - labelsToAdd.add(RuntimeConstants.LABEL_NEEDS_REBASE); - } - } - } + private final GithubProcessor githubProcessor; + private final WildFlyBotConfig wildFlyBotConfig; - return Uni.createFrom().voidItem().invoke(() -> { - try { - githubProcessor.updateLabels(pullRequest, labelsToAdd, labelsToRemove); - } catch (IOException e) { - throw new RuntimeException(e); - } - }); - } catch (IOException e) { - throw new RuntimeException(e); - } - }; + public PullRequestMergableProcessor(WildFlyBotConfig wildFlyBotConfig, GithubProcessor githubProcessor) { + this.wildFlyBotConfig = wildFlyBotConfig; + this.githubProcessor = githubProcessor; + } public void addPushPayload(GHEventPayload.Push pushPayload) { GHRepository repository = pushPayload.getRepository(); @@ -103,7 +59,7 @@ public void addPushPayload(GHEventPayload.Push pushPayload) { Uni> mergeableStatusUpdateUni = Uni.createFrom() // Collect all Pull Requests .item(repository.queryPullRequests().state(GHIssueState.OPEN).base(RuntimeConstants.MAIN_BRANCH).list()) - .invoke(() -> LOGGER.infof( + .invoke(() -> logger.infof( "Scheduling a mergeable status update for open pull requests for new head [%s - \"%s\"]", headCommit.getSha(), headCommit.getMessage())) .map(ghPullRequests -> { @@ -114,18 +70,66 @@ public void addPushPayload(GHEventPayload.Push pushPayload) { } }) // Prompt GitHub to recalculate mergeable status - .call(combineUnis(pollGitHub::apply)::apply) + .call(combineUnis(this::pollGitHub)::apply) // Give GitHub some time .onItem().delayIt().by(Duration.ofSeconds(wildFlyBotConfig.timeout())) // Retrieve the results from GitHub and apply labels accordingly - .call(combineUnis(pollGitHub::apply)::apply) + .call(combineUnis(this::pollGitHub)::apply) // Filter failed Pull Requests - .call(combineUnis(applyLabels::apply)::apply); + .call(combineUnis(this::applyLabels)::apply); pushPayloadsQueue.add(mergeableStatusUpdateUni); subscription(null, headCommit); } + private Uni pollGitHub(GHPullRequest pullRequest) { + return Uni.createFrom() + .item(pullRequest) + .invoke(pr -> { + try { + if (wildFlyBotConfig.isDryRun()) { + logger.info( + RuntimeConstants.DRY_RUN_PREPEND + .formatted("Sending a request to GitHub for mergeable status")); + } else { + pr.getMergeable(); + } + } catch (IOException e) { + throw new RuntimeException(e); + } + }) + .runSubscriptionOn(Infrastructure.getDefaultWorkerPool()); + } + + private Uni applyLabels(GHPullRequest pullRequest) { + try { + List labelsToAdd = new ArrayList<>(); + List labelsToRemove = new ArrayList<>(); + if (wildFlyBotConfig.isDryRun()) { + logger.info(RuntimeConstants.DRY_RUN_PREPEND + .formatted("Retrieving mergeable status and then we would apply labels accordingly")); + } else { + Optional.ofNullable(pullRequest.getMergeable()).ifPresent(mergeable -> { + if (mergeable) { + labelsToRemove.add(RuntimeConstants.LABEL_NEEDS_REBASE); + return; + } + labelsToAdd.add(RuntimeConstants.LABEL_NEEDS_REBASE); + }); + } + + return Uni.createFrom().voidItem().invoke(() -> { + try { + githubProcessor.updateLabels(pullRequest, labelsToAdd, labelsToRemove); + } catch (IOException e) { + throw new RuntimeException(e); + } + }); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + /** * Subscribes to the Uni> and after the execution it subscribes to the next * such Uni, if available. @@ -162,11 +166,11 @@ private void logResult(List ghPullRequests, GHEventPayload.Push.P .map(String::valueOf) .toList(); String concatenatedPRs = String.join(", ", missedPRs); - LOGGER.warnf( + logger.warnf( "Unable to verify the mergeable status for new %s branch head [%s - \"%s\"] for following pull requests: %s", RuntimeConstants.MAIN_BRANCH_REF, headCommit.getSha(), headCommit.getMessage(), concatenatedPRs); } else { - LOGGER.infof( + logger.infof( "Successfully scanned all pull requests for new %s branch head [%s - \"%s\"] and updated '%s' label accordingly.", RuntimeConstants.MAIN_BRANCH_REF, headCommit.getSha(), headCommit.getMessage(), RuntimeConstants.LABEL_NEEDS_REBASE); From e10d001a5aed868d39023d05d6a7787610d82dd5 Mon Sep 17 00:00:00 2001 From: Marek Skacelik Date: Wed, 1 Jul 2026 13:55:52 +0200 Subject: [PATCH 4/5] Extract description processor and add skip pattern support --- .../bot/PullRequestDescriptionProcessor.java | 147 ++++++++++++++++++ .../bot/PullRequestFormatProcessor.java | 79 ++++------ .../bot/PullRequestLabelProcessor.java | 23 ++- ...> PullRequestRuleActivationProcessor.java} | 46 +++--- .../java/org/wildfly/bot/model/Format.java | 31 ++++ .../util/PullRequestDescriptionHandler.java | 127 --------------- .../wildfly/bot/PRSkipPullRequestTest.java | 121 +++++++++++++- 7 files changed, 374 insertions(+), 200 deletions(-) create mode 100644 src/main/java/org/wildfly/bot/PullRequestDescriptionProcessor.java rename src/main/java/org/wildfly/bot/{PullRequestRuleProcessor.java => PullRequestRuleActivationProcessor.java} (67%) delete mode 100644 src/main/java/org/wildfly/bot/util/PullRequestDescriptionHandler.java diff --git a/src/main/java/org/wildfly/bot/PullRequestDescriptionProcessor.java b/src/main/java/org/wildfly/bot/PullRequestDescriptionProcessor.java new file mode 100644 index 00000000..ac7e43d8 --- /dev/null +++ b/src/main/java/org/wildfly/bot/PullRequestDescriptionProcessor.java @@ -0,0 +1,147 @@ +package org.wildfly.bot; + +import io.quarkiverse.githubapp.ConfigFile; +import io.quarkiverse.githubapp.event.PullRequest; +import jakarta.enterprise.context.RequestScoped; +import jakarta.inject.Inject; +import org.kohsuke.github.GHEventPayload; +import org.kohsuke.github.GHPullRequest; +import org.wildfly.bot.config.WildFlyBotConfig; +import org.wildfly.bot.model.WildFlyConfigFile; +import org.wildfly.bot.util.GitHubBotContextProvider; +import org.wildfly.bot.util.PullRequestLogger; +import org.wildfly.bot.util.Strings; + +import java.io.IOException; +import java.io.UncheckedIOException; +import java.util.ArrayList; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Optional; +import java.util.SequencedSet; +import java.util.regex.MatchResult; +import java.util.regex.Pattern; +import java.util.stream.Collectors; + +import static org.wildfly.bot.model.RuntimeConstants.BOT_JIRA_LINKS_HEADER; +import static org.wildfly.bot.model.RuntimeConstants.BOT_JIRA_LINK_COMMENT_TEMPLATE; +import static org.wildfly.bot.model.RuntimeConstants.BOT_JIRA_LINK_TEMPLATE; +import static org.wildfly.bot.model.RuntimeConstants.BOT_MESSAGE_DELIMITER; +import static org.wildfly.bot.model.RuntimeConstants.BOT_MESSAGE_WARNING; +import static org.wildfly.bot.model.RuntimeConstants.BOT_REPO_REF_FOOTER; +import static org.wildfly.bot.model.RuntimeConstants.CONFIG_FILE_NAME; +import static org.wildfly.bot.model.RuntimeConstants.DRY_RUN_PREPEND; + +@RequestScoped +public class PullRequestDescriptionProcessor { + + private final PullRequestLogger logger = PullRequestLogger.getLogger(PullRequestDescriptionProcessor.class); + + @Inject + WildFlyBotConfig wildFlyBotConfig; + + @Inject + GitHubBotContextProvider botContextProvider; + + void appendJiraLinks( + @PullRequest.Edited @PullRequest.Opened @PullRequest.Synchronize @PullRequest.Reopened @PullRequest.ReadyForReview GHEventPayload.PullRequest pullRequestPayload, + @ConfigFile(CONFIG_FILE_NAME) WildFlyConfigFile wildflyConfigFile) throws IOException { + GHPullRequest pullRequest = pullRequestPayload.getPullRequest(); + logger.setPullRequest(pullRequest); + + if (shouldSkipDescriptionAppending(pullRequest, wildflyConfigFile)) { + return; + } + + generateUpdatedDescription(pullRequest, wildflyConfigFile.wildfly.getProjectPattern()) + .ifPresent(newBody -> { + if (wildFlyBotConfig.isDryRun()) { + logger.infof(DRY_RUN_PREPEND.formatted("Updated PR body:\n%s"), newBody); + return; + } + try { + pullRequest.setBody(newBody); + } catch (IOException e) { + logger.errorf(e, "Failed to set body for pull request #%s", pullRequest.getNumber()); + throw new UncheckedIOException(e); + } + }); + } + + private boolean shouldSkipDescriptionAppending(GHPullRequest pullRequest, + WildFlyConfigFile configFile) throws IOException { + if (configFile == null) { + logger.info("Skipping JIRA link appending: no configuration file found"); + return true; + } + if (pullRequest.isDraft()) { + logger.info("Skipping JIRA link appending: pull request is a draft"); + return true; + } + return false; + } + + private Optional generateUpdatedDescription(GHPullRequest pullRequest, Pattern projectPattern) { + String body = pullRequest.getBody(); + String normalizedFullBody = (body != null) ? body.replace("\r", "") : ""; + int startOfBotBodyIndex = normalizedFullBody.lastIndexOf("\n" + BOT_MESSAGE_DELIMITER); + + String userBody; + String existingBotBody; + if (startOfBotBodyIndex != -1) { + userBody = normalizedFullBody.substring(0, startOfBotBodyIndex); + existingBotBody = normalizedFullBody.substring(startOfBotBodyIndex); + } else { + userBody = normalizedFullBody; + existingBotBody = ""; + } + + SequencedSet missingIssueLinks = findMissingIssueLinks(pullRequest, projectPattern, userBody); + String newBotBody = generateBotBodySection(missingIssueLinks); + + if (newBotBody.equals(existingBotBody)) { + return Optional.empty(); + } + return Optional.of(userBody + newBotBody); + } + + private SequencedSet findMissingIssueLinks(GHPullRequest pullRequest, Pattern projectPattern, + String userBody) { + List textSources = new ArrayList<>(); + textSources.add(pullRequest.getTitle()); + for (var commit : pullRequest.listCommits()) { + textSources.add(commit.getCommit().getMessage()); + } + return textSources.stream() + .flatMap(text -> projectPattern.matcher(text).results().map(MatchResult::group)) + .distinct() + .filter(issueKey -> !userBody.contains(BOT_JIRA_LINK_TEMPLATE.formatted(issueKey))) + .collect(Collectors.toCollection(LinkedHashSet::new)); + } + + private String generateBotBodySection(SequencedSet missingIssueLinks) { + if (missingIssueLinks.isEmpty()) { + return ""; + } + + StringBuilder botBodySection = new StringBuilder(); + botBodySection + .append("\n") + .append(BOT_MESSAGE_DELIMITER) + .append("\n\n") + .append(Strings.blockQuoted(BOT_MESSAGE_WARNING)) + .append("\n\n") + .append(Strings.blockQuoted(BOT_JIRA_LINKS_HEADER)); + + missingIssueLinks.stream() + .map(BOT_JIRA_LINK_COMMENT_TEMPLATE::formatted) + .map(Strings::blockQuoted) + .forEach(botBodySection::append); + + botBodySection + .append("\n\n") + .append(BOT_REPO_REF_FOOTER.formatted(botContextProvider.getBotName())); + + return botBodySection.toString(); + } +} diff --git a/src/main/java/org/wildfly/bot/PullRequestFormatProcessor.java b/src/main/java/org/wildfly/bot/PullRequestFormatProcessor.java index 5c39b374..c784265b 100644 --- a/src/main/java/org/wildfly/bot/PullRequestFormatProcessor.java +++ b/src/main/java/org/wildfly/bot/PullRequestFormatProcessor.java @@ -5,7 +5,6 @@ import jakarta.enterprise.context.RequestScoped; import jakarta.inject.Inject; -import org.jboss.logging.Logger; import org.kohsuke.github.GHCommitStatus; import org.kohsuke.github.GHEventPayload; import org.kohsuke.github.GHPullRequest; @@ -18,16 +17,13 @@ import org.wildfly.bot.model.WildFlyConfigFile; import org.wildfly.bot.util.GitHubBotContextProvider; import org.wildfly.bot.util.GithubProcessor; -import org.wildfly.bot.util.PullRequestDescriptionHandler; import org.wildfly.bot.util.PullRequestLogger; import java.io.IOException; -import java.io.UncheckedIOException; import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.regex.Pattern; import static org.wildfly.bot.model.RuntimeConstants.CONFIG_FILE_NAME; import static org.wildfly.bot.model.RuntimeConstants.DEPENDABOT; @@ -37,8 +33,7 @@ @RequestScoped public class PullRequestFormatProcessor { - private static final Logger LOG_DELEGATE = Logger.getLogger(PullRequestFormatProcessor.class); - private final PullRequestLogger LOG = new PullRequestLogger(LOG_DELEGATE); + private final PullRequestLogger logger = PullRequestLogger.getLogger(PullRequestFormatProcessor.class); private static final String CHECK_NAME = "Format"; @Inject @@ -53,16 +48,16 @@ public class PullRequestFormatProcessor { void postDependabotInfo(@PullRequest.Opened GHEventPayload.PullRequest pullRequestPayload, @ConfigFile(CONFIG_FILE_NAME) WildFlyConfigFile wildflyConfigFile) throws IOException { GHPullRequest pullRequest = pullRequestPayload.getPullRequest(); - LOG.setPullRequest(pullRequest); - githubProcessor.LOG.setPullRequest(pullRequest); + logger.setPullRequest(pullRequest); + githubProcessor.logger.setPullRequest(pullRequest); if (pullRequest.getUser().getLogin().equals(DEPENDABOT)) { - LOG.infof("Dependabot detected."); + logger.infof("Dependabot detected."); String comment = ("WildFly Bot recognized this PR as dependabot dependency update. Please create a %s issue" + " and add new comment containing this JIRA link please.") .formatted(wildflyConfigFile.wildfly.projectKey); if (wildFlyBotConfig.isDryRun()) { - LOG.infof(DRY_RUN_PREPEND.formatted("Add new comment %s"), comment); + logger.infof(DRY_RUN_PREPEND.formatted("Add new comment %s"), comment); } else { pullRequest.comment(comment); } @@ -74,32 +69,19 @@ void pullRequestFormatCheck( @PullRequest.Edited @PullRequest.Opened @PullRequest.Synchronize @PullRequest.Reopened @PullRequest.ReadyForReview GHEventPayload.PullRequest pullRequestPayload, @ConfigFile(CONFIG_FILE_NAME) WildFlyConfigFile wildflyConfigFile) throws IOException { GHPullRequest pullRequest = pullRequestPayload.getPullRequest(); - LOG.setPullRequest(pullRequest); - githubProcessor.LOG.setPullRequest(pullRequest); + logger.setPullRequest(pullRequest); + githubProcessor.logger.setPullRequest(pullRequest); - String message = githubProcessor.skipPullRequest(pullRequest, wildflyConfigFile); - if (message != null) { + if (shouldSkipFormatCheck(pullRequest, wildflyConfigFile, pullRequestPayload.getAction())) { + githubProcessor.deleteFormatComment(pullRequest, FAILED_FORMAT_COMMENT); String sha = pullRequest.getHead().getSha(); for (GHCommitStatus commitStatus : pullRequest.getRepository().listCommitStatuses(sha)) { - // Only update format check if it was created if (CHECK_NAME.equals(commitStatus.getContext()) && commitStatus.getCreator().getLogin().equals(botContextProvider.getBotName())) { githubProcessor.commitStatusSuccess(pullRequest, CHECK_NAME, "Valid [Skipped]"); + break; } } - githubProcessor.deleteFormatComment(pullRequest, FAILED_FORMAT_COMMENT); - LOG.infof("Skipping format due to %s", message); - return; - } - - if (pullRequest.getUser().getLogin().equals(DEPENDABOT) - && pullRequestPayload.getAction().equals(PullRequest.Opened.NAME)) { - LOG.info("Skipping format check on newly opened dependabot PRs."); - return; - } - - if (!wildflyConfigFile.wildfly.format.enabled) { - LOG.info("Skipping format due to format being disabled"); return; } @@ -113,8 +95,6 @@ void pullRequestFormatCheck( } } - generateAppendedMessage(pullRequest, wildflyConfigFile.wildfly.getProjectPattern()); - if (errors.isEmpty()) { githubProcessor.commitStatusSuccess(pullRequest, CHECK_NAME, "Valid"); } else { @@ -123,22 +103,29 @@ void pullRequestFormatCheck( githubProcessor.formatComment(pullRequest, FAILED_FORMAT_COMMENT, errors.values()); } - private void generateAppendedMessage(GHPullRequest pullRequest, Pattern projectPattern) { - new PullRequestDescriptionHandler(pullRequest, projectPattern, botContextProvider.getBotName()) - .generateFullDescriptionBody() - .ifPresent(newBody -> { - if (wildFlyBotConfig.isDryRun()) { - LOG.infof("Pull Request #%s - Updated PR body:\n%s", pullRequest.getNumber(), newBody); - return; - } - try { - pullRequest.setBody(newBody); - } catch (IOException e) { - LOG.errorf(e, "Failed to set body for pull request #%s", pullRequest.getNumber()); - throw new UncheckedIOException(e); - } - - }); + private boolean shouldSkipFormatCheck(GHPullRequest pullRequest, WildFlyConfigFile configFile, + String action) throws IOException { + if (configFile == null) { + logger.info("Skipping format check: no configuration file found"); + return true; + } + if (pullRequest.isDraft()) { + logger.info("Skipping format check: pull request is a draft"); + return true; + } + if (!configFile.wildfly.format.enabled) { + logger.info("Skipping format check: format checking is disabled"); + return true; + } + if (configFile.wildfly.format.matchesSkipPattern(pullRequest.getBody())) { + logger.info("Skipping format check: skip pattern matched in PR description"); + return true; + } + if (DEPENDABOT.equals(pullRequest.getUser().getLogin()) && "opened".equals(action)) { + logger.info("Skipping format check on newly opened dependabot PRs."); + return true; + } + return false; } private List initializeChecks(WildFlyConfigFile wildflyConfigFile) { diff --git a/src/main/java/org/wildfly/bot/PullRequestLabelProcessor.java b/src/main/java/org/wildfly/bot/PullRequestLabelProcessor.java index 1d567564..9df8846f 100644 --- a/src/main/java/org/wildfly/bot/PullRequestLabelProcessor.java +++ b/src/main/java/org/wildfly/bot/PullRequestLabelProcessor.java @@ -3,8 +3,9 @@ import io.quarkiverse.githubapp.event.PullRequest; import org.wildfly.bot.util.GithubProcessor; import org.wildfly.bot.util.PullRequestLogger; + +import jakarta.enterprise.context.RequestScoped; import jakarta.inject.Inject; -import org.jboss.logging.Logger; import org.kohsuke.github.GHEventPayload; import org.kohsuke.github.GHPullRequest; import org.wildfly.bot.model.RuntimeConstants; @@ -14,10 +15,10 @@ import java.util.List; import java.util.Optional; +@RequestScoped public class PullRequestLabelProcessor { - private static final Logger LOG_DELEGATE = Logger.getLogger(PullRequestFormatProcessor.class); - private final PullRequestLogger LOG = new PullRequestLogger(LOG_DELEGATE); + private final PullRequestLogger logger = PullRequestLogger.getLogger(PullRequestLabelProcessor.class); @Inject GithubProcessor githubProcessor; @@ -25,12 +26,10 @@ public class PullRequestLabelProcessor { void pullRequestLabelCheck(@PullRequest.Synchronize @PullRequest.Reopened GHEventPayload.PullRequest pullRequestPayload) throws IOException { GHPullRequest pullRequest = pullRequestPayload.getPullRequest(); - LOG.setPullRequest(pullRequest); - githubProcessor.LOG.setPullRequest(pullRequest); + logger.setPullRequest(pullRequest); + githubProcessor.logger.setPullRequest(pullRequest); - String message = githubProcessor.skipPullRequest(pullRequest); - if (message != null) { - LOG.infof("Skipping labelling due to %s", message); + if (shouldSkipLabelCheck(pullRequest)) { return; } @@ -46,4 +45,12 @@ void pullRequestLabelCheck(@PullRequest.Synchronize @PullRequest.Reopened GHEven githubProcessor.updateLabels(pullRequest, labelsToAdd, labelsToRemove); } + + private boolean shouldSkipLabelCheck(GHPullRequest pullRequest) throws IOException { + if (pullRequest.isDraft()) { + logger.info("Skipping label check due to pull request being a draft"); + return true; + } + return false; + } } diff --git a/src/main/java/org/wildfly/bot/PullRequestRuleProcessor.java b/src/main/java/org/wildfly/bot/PullRequestRuleActivationProcessor.java similarity index 67% rename from src/main/java/org/wildfly/bot/PullRequestRuleProcessor.java rename to src/main/java/org/wildfly/bot/PullRequestRuleActivationProcessor.java index 8668edb9..1e114d37 100644 --- a/src/main/java/org/wildfly/bot/PullRequestRuleProcessor.java +++ b/src/main/java/org/wildfly/bot/PullRequestRuleActivationProcessor.java @@ -9,7 +9,6 @@ import org.wildfly.bot.util.PullRequestLogger; import jakarta.enterprise.context.RequestScoped; import jakarta.inject.Inject; -import org.jboss.logging.Logger; import org.kohsuke.github.GHEventPayload; import org.kohsuke.github.GHLabel; import org.kohsuke.github.GHPullRequest; @@ -25,28 +24,21 @@ import java.util.Set; @RequestScoped -public class PullRequestRuleProcessor { - private static final Logger LOG_DELEGATE = Logger.getLogger(PullRequestRuleProcessor.class); - private final PullRequestLogger LOG = new PullRequestLogger(LOG_DELEGATE); +public class PullRequestRuleActivationProcessor { + private final PullRequestLogger logger = PullRequestLogger.getLogger(PullRequestRuleActivationProcessor.class); @Inject GithubProcessor githubProcessor; - void pullRequestRuleCheck( + void pullRequestRuleActivation( @PullRequest.Edited @PullRequest.Opened @PullRequest.Synchronize @PullRequest.Reopened @PullRequest.ReadyForReview GHEventPayload.PullRequest pullRequestPayload, @ConfigFile(RuntimeConstants.CONFIG_FILE_NAME) WildFlyConfigFile wildflyBotConfigFile, GitHub gitHub) throws IOException { GHPullRequest pullRequest = pullRequestPayload.getPullRequest(); - LOG.setPullRequest(pullRequest); - githubProcessor.LOG.setPullRequest(pullRequest); + logger.setPullRequest(pullRequest); + githubProcessor.logger.setPullRequest(pullRequest); - String message = githubProcessor.skipPullRequest(pullRequest, wildflyBotConfigFile); - if (message != null) { - LOG.infof("Skipping rules due to %s", message); - return; - } - - if (githubProcessor.hasDuplicateCommitInBase(pullRequest, pullRequestPayload.getRepository())) { + if (shouldSkipRuleActivation(pullRequest, wildflyBotConfigFile, pullRequestPayload)) { return; } @@ -58,16 +50,17 @@ void pullRequestRuleCheck( for (WildFlyConfigFile.WildFlyRule rule : wildflyBotConfigFile.wildfly.rules) { if (Matcher.notifyRequestReview(pullRequest, rule)) { if (!rule.notify.isEmpty()) { - LOG.infof("title \"%s\" was matched with a rule, containing notify, with the id: %s.", + logger.infof("title \"%s\" was matched with a rule, containing notify, with the id: %s.", pullRequest.getTitle(), rule.id != null ? rule.id : "N/A"); reviewers.addAll(rule.notify); } labels.addAll(rule.labels); } else if (Matcher.notifyComment(pullRequest, rule)) { if (!rule.notify.isEmpty()) { - LOG.infof("title \"%s\" was matched with a rule, containing notify, with the id: %s.", + logger.infof("title \"%s\" was matched with a rule, containing notify, with the id: %s.", pullRequest.getTitle(), rule.id != null ? rule.id : "N/A"); - rule.notify.forEach(user -> ccMentionsWithRules.computeIfAbsent(user, v -> new ArrayList<>()).add(rule.id)); + rule.notify.forEach( + user -> ccMentionsWithRules.computeIfAbsent(user, v -> new ArrayList<>()).add(rule.id)); } labels.addAll(rule.labels); } @@ -84,11 +77,28 @@ void pullRequestRuleCheck( labels.removeIf(currentLabels::contains); if (!labels.isEmpty()) { - LOG.debugf("Adding following labels: %s.", labels); + logger.debugf("Adding following labels: %s.", labels); pullRequest.addLabels(labels.toArray(String[]::new)); } githubProcessor.processNotifies(pullRequest, gitHub, ccMentionsWithRules, reviewers, wildflyBotConfigFile.wildfly.emails); } + + private boolean shouldSkipRuleActivation(GHPullRequest pullRequest, + WildFlyConfigFile configFile, GHEventPayload.PullRequest pullRequestPayload) throws IOException { + if (configFile == null) { + logger.info("Skipping rule activation: no configuration file found"); + return true; + } + if (pullRequest.isDraft()) { + logger.info("Skipping rule activation: pull request is a draft"); + return true; + } + if (githubProcessor.hasDuplicateCommitInBase(pullRequest, pullRequestPayload.getRepository())) { + logger.info("Skipping rule activation: pull request has duplicate commits in base"); + return true; + } + return false; + } } diff --git a/src/main/java/org/wildfly/bot/model/Format.java b/src/main/java/org/wildfly/bot/model/Format.java index 0c946825..1070e87e 100644 --- a/src/main/java/org/wildfly/bot/model/Format.java +++ b/src/main/java/org/wildfly/bot/model/Format.java @@ -1,12 +1,25 @@ package org.wildfly.bot.model; +import com.fasterxml.jackson.annotation.JsonFormat; +import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonSetter; import com.fasterxml.jackson.annotation.Nulls; +import java.util.ArrayList; +import java.util.List; +import java.util.regex.Pattern; + public final class Format { public boolean enabled = true; + @JsonFormat(with = JsonFormat.Feature.ACCEPT_SINGLE_VALUE_AS_ARRAY) + @JsonSetter(nulls = Nulls.SKIP) + public List skip = new ArrayList<>(); + + @JsonIgnore + private List compiledSkipPatterns; + @JsonSetter(nulls = Nulls.SKIP) public TitlePattern title = new TitlePattern(); @@ -16,6 +29,24 @@ public final class Format { @JsonSetter(nulls = Nulls.SKIP) public CommitPattern commit = new CommitPattern(); + private List getCompiledSkipPatterns() { + // lazy + if (compiledSkipPatterns == null) { + compiledSkipPatterns = skip.stream() + .map(p -> Pattern.compile(p, Pattern.CASE_INSENSITIVE)) + .toList(); + } + return compiledSkipPatterns; + } + + public boolean matchesSkipPattern(String text) { + if (text == null || skip.isEmpty()) { + return false; + } + return getCompiledSkipPatterns().stream() + .anyMatch(pattern -> pattern.matcher(text).find()); + } + public abstract static class RegexPattern { public String failMessage; public boolean enabled = true; diff --git a/src/main/java/org/wildfly/bot/util/PullRequestDescriptionHandler.java b/src/main/java/org/wildfly/bot/util/PullRequestDescriptionHandler.java deleted file mode 100644 index c57807ef..00000000 --- a/src/main/java/org/wildfly/bot/util/PullRequestDescriptionHandler.java +++ /dev/null @@ -1,127 +0,0 @@ -package org.wildfly.bot.util; - -import static java.util.Objects.requireNonNull; -import static org.wildfly.bot.model.RuntimeConstants.BOT_JIRA_LINKS_HEADER; -import static org.wildfly.bot.model.RuntimeConstants.BOT_JIRA_LINK_COMMENT_TEMPLATE; -import static org.wildfly.bot.model.RuntimeConstants.BOT_JIRA_LINK_TEMPLATE; -import static org.wildfly.bot.model.RuntimeConstants.BOT_MESSAGE_DELIMITER; -import static org.wildfly.bot.model.RuntimeConstants.BOT_MESSAGE_WARNING; -import static org.wildfly.bot.model.RuntimeConstants.BOT_REPO_REF_FOOTER; -import static org.wildfly.bot.util.Strings.blockQuoted; - -import java.util.ArrayList; -import java.util.LinkedHashSet; -import java.util.List; -import java.util.Optional; -import java.util.SequencedSet; -import java.util.regex.MatchResult; -import java.util.regex.Pattern; -import java.util.stream.Collectors; - -import org.kohsuke.github.GHPullRequest; - -/** - * Handles the description body of a pull request. - * - *

- * - * This class is responsible for managing the user-provided body and the - * bot-generated body - * of a pull request. It checks for missing issue links in the user body and - * generates - * a full description body that combines both user and bot sections. - * - * @author mskacelik - */ -public final class PullRequestDescriptionHandler { - private final String userBody; - private final String botBody; - private final String gitHubAppName; - private final SequencedSet missingIssueLinksWithinUserBody; - - public PullRequestDescriptionHandler(GHPullRequest pullRequest, Pattern issueKeyPattern, String githubAppName) { - requireNonNull(pullRequest, "Pull request must not be null"); - requireNonNull(issueKeyPattern, "Issue link pattern must not be null"); - - this.gitHubAppName = githubAppName; - - String body = pullRequest.getBody(); - final String normalizedFullBody = (body != null) ? body.replaceAll("\\r", "") : ""; - final int startOfBotBodyIndex = normalizedFullBody.lastIndexOf("\n" + BOT_MESSAGE_DELIMITER); - - if (startOfBotBodyIndex != -1) { - this.userBody = normalizedFullBody.substring(0, startOfBotBodyIndex); - this.botBody = normalizedFullBody.substring(startOfBotBodyIndex); - } else { - this.userBody = normalizedFullBody; - this.botBody = ""; // Empty - } - - List textSources = new ArrayList<>(); // title + commit messages - textSources.add(pullRequest.getTitle()); - for (var commit : pullRequest.listCommits()) { - textSources.add(commit.getCommit().getMessage()); - } - this.missingIssueLinksWithinUserBody = textSources.stream() - .flatMap(text -> issueKeyPattern.matcher(text).results().map(MatchResult::group)) - .distinct() - .filter(issueKey -> !userBody.contains(BOT_JIRA_LINK_TEMPLATE.formatted(issueKey))) - .collect(Collectors.toCollection(LinkedHashSet::new)); - } - - /** - * Generates the full description body for the pull request. - *

- * This method combines the user-provided body with the bot-generated body, - * ensuring that any missing issue links are included in the bot section. - * If there are no changes in the bot body, it returns an empty {@code Optional} - * because - * no updates are needed to the pull request description. - * For example, if the user body already contains all the necessary issue links, - * and was updated by the user, the bot won't change the description. - * - * @return an {@code Optional} containing the full description body if changes - * are made, or an empty Optional if no changes are needed. - */ - public Optional generateFullDescriptionBody() { - StringBuilder newBotBody = generateBotBodySection(); - if (newBotBody.toString().equals(botBody)) { - return Optional.empty(); // No changes in the bot body - } - String fullDescription = generateUserBodySection() - .append(newBotBody) - .toString(); - return Optional.of(fullDescription); - } - - private StringBuilder generateUserBodySection() { - return new StringBuilder(userBody); - } - - private StringBuilder generateBotBodySection() { - StringBuilder botBodySection = new StringBuilder(); - - if (missingIssueLinksWithinUserBody.isEmpty()) { - return botBodySection; // Empty bot body section if no issues are missing - } - - botBodySection - .append("\n") - .append(BOT_MESSAGE_DELIMITER) - .append("\n\n") - .append(blockQuoted(BOT_MESSAGE_WARNING)) - .append("\n\n") - .append(blockQuoted(BOT_JIRA_LINKS_HEADER)); - - missingIssueLinksWithinUserBody.stream() - .map(BOT_JIRA_LINK_COMMENT_TEMPLATE::formatted) - .map(Strings::blockQuoted) - .forEach(link -> botBodySection.append(link)); - - botBodySection - .append("\n\n") - .append(BOT_REPO_REF_FOOTER.formatted(gitHubAppName)); - - return botBodySection; - } -} diff --git a/src/test/java/org/wildfly/bot/PRSkipPullRequestTest.java b/src/test/java/org/wildfly/bot/PRSkipPullRequestTest.java index ca9f59d1..5216702b 100644 --- a/src/test/java/org/wildfly/bot/PRSkipPullRequestTest.java +++ b/src/test/java/org/wildfly/bot/PRSkipPullRequestTest.java @@ -6,9 +6,12 @@ import org.junit.jupiter.api.Test; import org.kohsuke.github.GHCommit; import org.kohsuke.github.GHCommitQueryBuilder; +import org.kohsuke.github.GHPullRequest; import org.kohsuke.github.GHPullRequestCommitDetail; +import org.kohsuke.github.GHRepository; import org.kohsuke.github.PagedIterable; import org.kohsuke.github.PagedIterator; +import org.mockito.Mockito; import org.wildfly.bot.utils.TestConstants; import org.wildfly.bot.utils.WildflyGitHubBotTesting; import org.wildfly.bot.utils.mocking.Mockable; @@ -42,6 +45,31 @@ public class PRSkipPullRequestTest { commit: enabled: true """; + private static final String wildflyConfigFileWithSkipPattern = """ + wildfly: + format: + skip: "No JIRA required" + title: + enabled: true + commit: + enabled: true + """; + private static final String wildflyConfigFileWithSkipPatternAndRules = """ + wildfly: + rules: + - id: "test-rule" + title: "WFLY" + notify: [Tadpole] + labels: [label1] + format: + skip: + - "No JIRA required" + - "No Issue (Required|Needed)" + title: + enabled: true + commit: + enabled: true + """; private static final String wildflyConfigFileWithRules = """ wildfly: rules: @@ -72,13 +100,104 @@ void testSkippingFormatCheckOnDraft() throws Throwable { .pullRequestEvent(pullRequestJson) .then(mocks -> { verify(mocks.pullRequest(pullRequestJson.id())).listFiles(); - verify(mocks.pullRequest(pullRequestJson.id()), times(2)).isDraft(); + verify(mocks.pullRequest(pullRequestJson.id()), times(3)).isDraft(); verify(mocks.pullRequest(pullRequestJson.id())).listComments(); // commit status should not be set verifyNoMoreInteractions(mocks.pullRequest(pullRequestJson.id())); }); } + @Test + void testSkippingFormatCheckOnSkipPattern() throws Throwable { + pullRequestJson = TestModel.setPullRequestJsonBuilder(pullRequestJsonBuilder -> pullRequestJsonBuilder + .title(TestConstants.INVALID_TITLE) + .description("No JIRA required")); + mockedContext = MockedGHPullRequest.builder(pullRequestJson.id()).commit(TestConstants.INVALID_COMMIT_MESSAGE); + + TestModel.given( + mocks -> WildflyGitHubBotTesting.mockRepo(mocks, wildflyConfigFileWithSkipPattern, pullRequestJson, + mockedContext)) + .pullRequestEvent(pullRequestJson) + .then(mocks -> { + GHRepository repo = mocks.repository(TestConstants.TEST_REPO); + Mockito.verify(repo, never()).createCommitStatus(eq(pullRequestJson.commitSHA()), + eq(org.kohsuke.github.GHCommitState.ERROR), Mockito.anyString(), Mockito.anyString(), + Mockito.anyString()); + }); + } + + @Test + void testSkipPatternCaseInsensitive() throws Throwable { + pullRequestJson = TestModel.setPullRequestJsonBuilder(pullRequestJsonBuilder -> pullRequestJsonBuilder + .title(TestConstants.INVALID_TITLE) + .description("NO JIRA REQUIRED")); + mockedContext = MockedGHPullRequest.builder(pullRequestJson.id()).commit(TestConstants.INVALID_COMMIT_MESSAGE); + + TestModel.given( + mocks -> WildflyGitHubBotTesting.mockRepo(mocks, wildflyConfigFileWithSkipPattern, pullRequestJson, + mockedContext)) + .pullRequestEvent(pullRequestJson) + .then(mocks -> { + GHRepository repo = mocks.repository(TestConstants.TEST_REPO); + Mockito.verify(repo, never()).createCommitStatus(eq(pullRequestJson.commitSHA()), + eq(org.kohsuke.github.GHCommitState.ERROR), Mockito.anyString(), Mockito.anyString(), + Mockito.anyString()); + }); + } + + @Test + void testSkipPatternRegexAlternation() throws Throwable { + pullRequestJson = TestModel.setPullRequestJsonBuilder(pullRequestJsonBuilder -> pullRequestJsonBuilder + .title(TestConstants.INVALID_TITLE) + .description("No Issue Needed")); + mockedContext = MockedGHPullRequest.builder(pullRequestJson.id()).commit(TestConstants.INVALID_COMMIT_MESSAGE); + + TestModel.given( + mocks -> WildflyGitHubBotTesting.mockRepo(mocks, wildflyConfigFileWithSkipPatternAndRules, pullRequestJson, + mockedContext)) + .pullRequestEvent(pullRequestJson) + .then(mocks -> { + GHRepository repo = mocks.repository(TestConstants.TEST_REPO); + Mockito.verify(repo, never()).createCommitStatus(eq(pullRequestJson.commitSHA()), + eq(org.kohsuke.github.GHCommitState.ERROR), Mockito.anyString(), Mockito.anyString(), + Mockito.anyString()); + }); + } + + @Test + void testSkipPatternDoesNotSkipRuleActivation() throws Throwable { + pullRequestJson = TestModel.setPullRequestJsonBuilder(pullRequestJsonBuilder -> pullRequestJsonBuilder + .title("[WFLY-123] Valid title") + .description("No JIRA required")); + mockedContext = MockedGHPullRequest.builder(pullRequestJson.id()).commit(TestConstants.INVALID_COMMIT_MESSAGE); + + TestModel.given( + mocks -> WildflyGitHubBotTesting.mockRepo(mocks, wildflyConfigFileWithSkipPatternAndRules, pullRequestJson, + mockedContext)) + .pullRequestEvent(pullRequestJson) + .then(mocks -> { + GHPullRequest mockedPR = mocks.pullRequest(pullRequestJson.id()); + Mockito.verify(mockedPR).comment(startsWith("/cc")); + }); + } + + @Test + void testNoSkipPatternMatchRunsFormatChecks() throws Throwable { + pullRequestJson = TestModel.setPullRequestJsonBuilder(pullRequestJsonBuilder -> pullRequestJsonBuilder + .title(TestConstants.INVALID_TITLE) + .description("Some random description")); + mockedContext = MockedGHPullRequest.builder(pullRequestJson.id()).commit(TestConstants.INVALID_COMMIT_MESSAGE); + + TestModel.given( + mocks -> WildflyGitHubBotTesting.mockRepo(mocks, wildflyConfigFileWithSkipPattern, pullRequestJson, + mockedContext)) + .pullRequestEvent(pullRequestJson) + .then(mocks -> { + GHRepository repo = mocks.repository(TestConstants.TEST_REPO); + WildflyGitHubBotTesting.verifyFormatFailure(repo, pullRequestJson, "commit, title"); + }); + } + @Test void testSkippingRulesOnIncorrectRebase() throws Throwable { final String duplicateSHA = "sha1"; From 7377bc38cd29b00ef9bb3b820a2ebf1bd90339b5 Mon Sep 17 00:00:00 2001 From: Marek Skacelik Date: Wed, 1 Jul 2026 14:21:55 +0200 Subject: [PATCH 5/5] Update configuration example with skip and failMessage fields --- README.md | 17 +++++++++-------- wildfly-bot-config-example.yml | 11 +++++++---- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 6e61a267..3d596d52 100644 --- a/README.md +++ b/README.md @@ -4,10 +4,11 @@ WildFly GitHub Bot helps you to keep your pull requests in the correct format. This project is built with usage of Quarkus GitHub App: https://quarkiverse.github.io/quarkiverse-docs/quarkus-github-app/dev/index.html ## Functionality -* Expected format for Pull Requests - Defined in config file: - * Title - Regex, we expect to match for title of the Pull Request. - * Commit - Regex, we expect to match for at least one commit in the Pull Request. - * Description - Regexes, we expect to match for description of the Pull Request. +* _Format_ checks for Pull Requests - Defined in config file under `format`: + * _Title_ - Checks the title of the Pull Request against the `projectKey` pattern. + * _Commit_ - Checks at least one commit message against the `projectKey` pattern. + * _Description_ - Checks the description of the Pull Request against configured regexes. + * _Skip_ - Regex pattern(s) matched against PR description to skip format checks. * _Activation Rules_ for matching on Pull Requests and applying __functionality__ - Defined in config file: * __Notify__ - List of people to request Pull Request review. People, we can't request review from, we only cc notify them - Rule functionality. * __Labels__ - Labels to apply on the Pull Request - Rule functionality. @@ -15,7 +16,6 @@ This project is built with usage of Quarkus GitHub App: https://quarkiverse.gith * _Body_ - Regex, used on body of the Pull Request - Rule activation. * _TitleBody_ - Regex, used on either title or body of the Pull Request - Rule activation. * _Directories_ - List of directories, if corresponding files are changed in the Pull Request - Rule activation. -* Option to disable format checks on Pull Request by adding message `@[bot] skip format` in the description. * Automatically append JIRA links into description of the Pull Request, if issue tracker number is detected in Title, Description or Commit. * Automatically applies/removes labels on a Pull Request: * `rebase-this` - depending on conflicts with `main` branch. @@ -115,8 +115,9 @@ wildfly: - github-user-handle1 - github-user-handle2 format: + skip: "\\[skip format\\]" title: - message: Wrong content of the PR title + failMessage: Wrong content of the PR title description: # 3 regexes: - pattern: JIRA:\s+https://redhat.atlassian.net/browse/WFLY-\d+|https://redhat.atlassian.net/browse/WFLY-\d+ @@ -129,7 +130,7 @@ wildfly: 2. `title` - Checks the title of a PR by using a regular expression generated from `projectKey` field, which is by default "WFLY". You can find more information in [wildfly-bot-config-example.yml](wildfly-bot-config-example.yml) 3. `description` - Checks comments of a PR by using individual regular expressions in the `pattern` fields under `regexes`. > For example, the correct format is "https://issues.jboss.org/browse/WFLY-11". -4. `message` - The text of an error message in the respective check. +4. `failMessage` - The message displayed when the respective check fails. 5. `emails` - List of emails to receive notifications. > [!IMPORTANT] @@ -149,7 +150,7 @@ wildfly: - github-user-handle2 format: title: - message: Wrong content of the PR title + failMessage: Wrong content of the PR title emails: - nonexistingemail@whatever.com - whoever@nonexistingmailingservice.com diff --git a/wildfly-bot-config-example.yml b/wildfly-bot-config-example.yml index e1b027f3..e849ba54 100644 --- a/wildfly-bot-config-example.yml +++ b/wildfly-bot-config-example.yml @@ -17,9 +17,12 @@ wildfly: notify: [ another-random-person ] format: # Validation check for a correct format of the Pull Request + skip: # Regex pattern(s) matched against PR description to skip format checks. Accepts a single value or a list + - "\\[skip format\\]" + - "^Revert \".+\"$" title: # Enabled by default. You have to disable explicitly. - enabled: false # If check enabled. By default, this setting is set true - message: "Wrong content of the title" # Override default message. Message is display if check does not succeed + enabled: false # If check enabled. By default, this setting is set to true + failMessage: "Wrong content of the title" # Message displayed when the check fails description: regexes: # List of objects, where the object is as follows: @@ -28,8 +31,8 @@ wildfly: message: "The PR description is not correct" # Default message to display if a object is missing message field commit: # Enabled by default. You have to disable explicitly. - enabled: true # If check enabled. By default, this setting is set true - message: "Wrong commit message!" # Override default message. Message is display if check does not succeed + enabled: true # If check enabled. By default, this setting is set to true + failMessage: "Wrong commit message!" # Message displayed when the check fails emails: # List of email addresses, which will receive updates from this app - random-person@bar.baz