From ee935389c41f004f4d472b63baad3191287b5212 Mon Sep 17 00:00:00 2001 From: Gerard Ryan Date: Wed, 5 Apr 2023 01:08:56 +0100 Subject: [PATCH 1/4] Update to newer quarkus(-github-app) for features We'll need this to be able to use the ConfigFileProvider in the StalledDiscussionFlow, since the @ConfigFile annotation only seems to work well for injection into event-listener methods. The `org.jetbrains:annotations` dependency doesn't get pulled in implicitly any longer, so I've just switched usage of the @NotNull && @Nullable annotations from there to the SmallRye equivalents, which are available. Something that I'm not 100% sure about is the test changes here: I guess the Jackson/SnakeYAML handling of null fields changed somewhere along the way. --- pom.xml | 6 +++--- .../java/org/bf2/arch/bot/CreateDraftRecordFlow.java | 11 ++++++----- src/main/java/org/bf2/arch/bot/Util.java | 3 ++- .../org/bf2/arch/bot/CreateDraftRecordFlowTest.java | 6 +++++- 4 files changed, 16 insertions(+), 10 deletions(-) diff --git a/pom.xml b/pom.xml index c93f73b..15d8ead 100644 --- a/pom.xml +++ b/pom.xml @@ -13,9 +13,9 @@ UTF-8 quarkus-bom io.quarkus.platform - 2.7.5.Final - 3.0.0-M5 - 1.8.4 + 2.16.6.Final + 3.0.0 + 1.16.0 diff --git a/src/main/java/org/bf2/arch/bot/CreateDraftRecordFlow.java b/src/main/java/org/bf2/arch/bot/CreateDraftRecordFlow.java index e85cfae..ae39c1b 100644 --- a/src/main/java/org/bf2/arch/bot/CreateDraftRecordFlow.java +++ b/src/main/java/org/bf2/arch/bot/CreateDraftRecordFlow.java @@ -11,14 +11,10 @@ import java.util.stream.Collectors; import java.util.stream.Stream; -import io.quarkiverse.githubapp.ConfigFile; -import io.quarkiverse.githubapp.event.IssueComment; -import org.bf2.arch.bot.model.record.RecordPage; import org.bf2.arch.bot.model.record.RecordId; +import org.bf2.arch.bot.model.record.RecordPage; import org.bf2.arch.bot.model.record.RecordType; import org.eclipse.microprofile.config.inject.ConfigProperty; -import org.jetbrains.annotations.NotNull; -import org.jetbrains.annotations.Nullable; import org.kohsuke.github.GHBranch; import org.kohsuke.github.GHCommit; import org.kohsuke.github.GHContent; @@ -32,6 +28,11 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import io.quarkiverse.githubapp.ConfigFile; +import io.quarkiverse.githubapp.event.IssueComment; +import io.smallrye.common.constraint.NotNull; +import io.smallrye.common.constraint.Nullable; + /** * Flow for creating a Draft record (AP, ADR, PADR), and possibly superseding an existing record. * diff --git a/src/main/java/org/bf2/arch/bot/Util.java b/src/main/java/org/bf2/arch/bot/Util.java index 0103c7d..048cdc5 100644 --- a/src/main/java/org/bf2/arch/bot/Util.java +++ b/src/main/java/org/bf2/arch/bot/Util.java @@ -21,7 +21,6 @@ import java.util.Set; import java.util.stream.Collectors; -import org.jetbrains.annotations.NotNull; import org.kohsuke.github.GHIssue; import org.kohsuke.github.GHLabel; import org.kohsuke.github.GHPullRequest; @@ -30,6 +29,8 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import io.smallrye.common.constraint.NotNull; + /** * Common helper functions */ diff --git a/src/test/java/org/bf2/arch/bot/CreateDraftRecordFlowTest.java b/src/test/java/org/bf2/arch/bot/CreateDraftRecordFlowTest.java index 9a24c95..dea1f84 100644 --- a/src/test/java/org/bf2/arch/bot/CreateDraftRecordFlowTest.java +++ b/src/test/java/org/bf2/arch/bot/CreateDraftRecordFlowTest.java @@ -70,6 +70,8 @@ public void renderTemplateTest() throws IOException { "- \"me\"\n" + "tags:\n" + "- \"bar\"\n" + + "applies_padrs: null\n" + + "applies_patterns: null\n" + "---\n" + "Hello, world\n", rendered.toContentString()); } @@ -92,6 +94,8 @@ public void renderSupersededTemplateTest() throws IOException { "- \"me\"\n" + "tags:\n" + "- \"bar\"\n" + + "applies_padrs: null\n" + + "applies_patterns: null\n" + "---\n" + "Hello, world\n", rendered.toContentString()); } @@ -137,4 +141,4 @@ public void testGetPage() throws IOException { // TODO test supersededContent -} \ No newline at end of file +} From a23ecf8eec8f854f8a94ca45b9ae4a9faf1cd8c6 Mon Sep 17 00:00:00 2001 From: Gerard Ryan Date: Wed, 5 Apr 2023 13:30:06 +0100 Subject: [PATCH 2/4] Disregard comments from the bot itself --- .../bf2/arch/bot/StalledDiscussionFlow.java | 46 ++++++++++--------- 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/src/main/java/org/bf2/arch/bot/StalledDiscussionFlow.java b/src/main/java/org/bf2/arch/bot/StalledDiscussionFlow.java index 20ec510..7f115f5 100644 --- a/src/main/java/org/bf2/arch/bot/StalledDiscussionFlow.java +++ b/src/main/java/org/bf2/arch/bot/StalledDiscussionFlow.java @@ -16,15 +16,15 @@ */ package org.bf2.arch.bot; -import javax.enterprise.context.ApplicationScoped; -import javax.inject.Inject; import java.io.IOException; import java.net.URISyntaxException; import java.util.Date; +import java.util.Optional; import java.util.Set; -import io.quarkiverse.githubapp.runtime.github.GitHubService; -import io.quarkus.scheduler.Scheduled; +import javax.enterprise.context.ApplicationScoped; +import javax.inject.Inject; + import org.eclipse.microprofile.config.inject.ConfigProperty; import org.kohsuke.github.GHDirection; import org.kohsuke.github.GHIssue; @@ -34,6 +34,11 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import io.quarkiverse.githubapp.ConfigFile; +import io.quarkiverse.githubapp.GitHubConfigFileProvider; +import io.quarkiverse.githubapp.runtime.github.GitHubService; +import io.quarkus.scheduler.Scheduled; + @ApplicationScoped public class StalledDiscussionFlow { @@ -52,16 +57,20 @@ public class StalledDiscussionFlow { private Date lastRan = new Date(0); GitHub client; - //ArchBotConfig config; + ArchBotConfig config; @Inject - void init(GitHubService service) { + void init(GitHubService service, GitHubConfigFileProvider configFileProvider) throws IOException { if (!enabled) { LOG.debug("Ignoring init: disabled due to {}=false", ENABLE); } else if (installationId != null) { // TODO parameterise this installationId client = service.getInstallationClient(installationId); - // TODO load the config + + Optional oconfig = configFileProvider.fetchConfigFile(client.getRepository(repositoryPath), + Util.CONFIG_REPO_PATH, ConfigFile.Source.DEFAULT, ArchBotConfig.class); + config = oconfig.orElseThrow(); + LOG.info("CONFIGG: {}", config); } else { throw new RuntimeException("installation id is requied"); } @@ -112,14 +121,8 @@ public void checkForStalledDiscussions() throws IOException { .order(GHDirection.ASC) .list(); LOG.info("Top-level query found {} PRs", results.getTotalCount()); - int processed = 0; for (GHIssue issue : results) { -// if () { -// lastRan = new Date(); -// break; -// } - processed++; - try { +try { GHPullRequest pullRequest = Util.findPullRequest(issue); if (pullRequest == null) { LOG.info("Issue#{} is not a PR, ignoring", issue.getNumber()); @@ -132,14 +135,15 @@ public void checkForStalledDiscussions() throws IOException { // https://docs.github.com/en/rest/pulls/comments#list-review-comments-in-a-repository // but the client doesn't expose them Date lastCommentDate; + LOG.info("COMMENTS COUNT: {}", pullRequest.listReviewComments().toList().size()); var mostRecent = pullRequest.listReviewComments().toList().stream() -// .filter(pr -> { -// try { -// return !Util.isThisBot(config, pr.getUser()); -// } catch (IOException e) { -// return true; -// } -// }) + .filter(pr -> { + try { + return !Util.isThisBot(config, pr.getUser()); + } catch (IOException e) { + return true; + } + }) .map(comment -> { try { LOG.info("Comment: {}", comment.getBody()); From 322104a1ed4fe213d670603ce3c1ca8f95ab14dc Mon Sep 17 00:00:00 2001 From: Gerard Ryan Date: Sun, 16 Apr 2023 19:11:54 +0100 Subject: [PATCH 3/4] Add missing annotation to config fields Without these, the affected fields were always being set to a null or default value for the type. For example, botUserLogin would always be null, and stalledDiscussionPollTimeMins would always be 0. --- .../java/org/bf2/arch/bot/ArchBotConfig.java | 5 ++- .../bf2/arch/bot/StalledDiscussionFlow.java | 35 +++++++++---------- 2 files changed, 21 insertions(+), 19 deletions(-) diff --git a/src/main/java/org/bf2/arch/bot/ArchBotConfig.java b/src/main/java/org/bf2/arch/bot/ArchBotConfig.java index 0f6218c..2b3e782 100644 --- a/src/main/java/org/bf2/arch/bot/ArchBotConfig.java +++ b/src/main/java/org/bf2/arch/bot/ArchBotConfig.java @@ -15,11 +15,13 @@ public class ArchBotConfig { /** * Github login name of the bot itself. */ + @JsonDeserialize String botUserLogin; /** * The time, in minutes, to wait between checking for stalled discussions. */ + @JsonDeserialize long stalledDiscussionPollTimeMins; /** @@ -31,7 +33,8 @@ public class ArchBotConfig { /** * The URL at which the site is published. */ - String publishedUrl = "https://architecture.appservices.tech"; + @JsonDeserialize + String publishedUrl; @Override public String toString() { diff --git a/src/main/java/org/bf2/arch/bot/StalledDiscussionFlow.java b/src/main/java/org/bf2/arch/bot/StalledDiscussionFlow.java index 7f115f5..9068989 100644 --- a/src/main/java/org/bf2/arch/bot/StalledDiscussionFlow.java +++ b/src/main/java/org/bf2/arch/bot/StalledDiscussionFlow.java @@ -70,7 +70,6 @@ void init(GitHubService service, GitHubConfigFileProvider configFileProvider) th Optional oconfig = configFileProvider.fetchConfigFile(client.getRepository(repositoryPath), Util.CONFIG_REPO_PATH, ConfigFile.Source.DEFAULT, ArchBotConfig.class); config = oconfig.orElseThrow(); - LOG.info("CONFIGG: {}", config); } else { throw new RuntimeException("installation id is requied"); } @@ -122,7 +121,7 @@ public void checkForStalledDiscussions() throws IOException { .list(); LOG.info("Top-level query found {} PRs", results.getTotalCount()); for (GHIssue issue : results) { -try { + try { GHPullRequest pullRequest = Util.findPullRequest(issue); if (pullRequest == null) { LOG.info("Issue#{} is not a PR, ignoring", issue.getNumber()); @@ -137,14 +136,14 @@ public void checkForStalledDiscussions() throws IOException { Date lastCommentDate; LOG.info("COMMENTS COUNT: {}", pullRequest.listReviewComments().toList().size()); var mostRecent = pullRequest.listReviewComments().toList().stream() - .filter(pr -> { - try { - return !Util.isThisBot(config, pr.getUser()); - } catch (IOException e) { - return true; - } - }) - .map(comment -> { + .filter(pr -> { + try { + return !Util.isThisBot(config, pr.getUser()); + } catch (IOException e) { + return true; + } + }) + .map(comment -> { try { LOG.info("Comment: {}", comment.getBody()); LOG.info("User: {}", comment.getUser().getLogin()); @@ -155,16 +154,16 @@ public void checkForStalledDiscussions() throws IOException { }).max(Date::compareTo); lastCommentDate = mostRecent.orElseGet(() -> { - try { - return pullRequest.getUpdatedAt(); - } catch (IOException e) { try { - return pullRequest.getCreatedAt(); - } catch (IOException ex) { - return new Date(0); + return pullRequest.getUpdatedAt(); + } catch (IOException e) { + try { + return pullRequest.getCreatedAt(); + } catch (IOException ex) { + return new Date(0); + } } - } - }); + }); LOG.info("PR#{}: Last comment time {}", pullRequest.getNumber(), lastCommentDate); if (lastCommentDate.getTime() < thresh) { From cc252135ab1473d773a58499ad9b32dba734bee9 Mon Sep 17 00:00:00 2001 From: Gerard Ryan Date: Tue, 18 Apr 2023 01:07:26 +0100 Subject: [PATCH 4/4] Handle poll interval and API token better - Token expires every hour, so a new token needs to be fetched every time, rather than just being able to get one at object initialization. - The "stalledDiscussionPollTimeMins" property set in the bf2-arch-bot.yml seemingly can't be used in the @Scheduled annotation, since it's not available early enough. --- .env | 1 + .../org/bf2/arch/bot/StalledDiscussionFlow.java | 13 ++++++++----- src/main/resources/application.properties | 5 ++++- src/test/resources/application.properties | 3 ++- 4 files changed, 15 insertions(+), 7 deletions(-) diff --git a/.env b/.env index 9e739c8..ebc49e0 100644 --- a/.env +++ b/.env @@ -20,6 +20,7 @@ bot.enable.pr-review=false bot.enable.state-machine=false bot.enable.create-draft=true +BOT_STALLED_DISCUSSION_FLOW_POLL_DURATION=86400s ## Stuff required by the github app framework diff --git a/src/main/java/org/bf2/arch/bot/StalledDiscussionFlow.java b/src/main/java/org/bf2/arch/bot/StalledDiscussionFlow.java index 9068989..2a47941 100644 --- a/src/main/java/org/bf2/arch/bot/StalledDiscussionFlow.java +++ b/src/main/java/org/bf2/arch/bot/StalledDiscussionFlow.java @@ -60,7 +60,10 @@ public class StalledDiscussionFlow { ArchBotConfig config; @Inject - void init(GitHubService service, GitHubConfigFileProvider configFileProvider) throws IOException { + GitHubService service; + + @Inject + void init(GitHubConfigFileProvider configFileProvider) throws IOException { if (!enabled) { LOG.debug("Ignoring init: disabled due to {}=false", ENABLE); } else if (installationId != null) { @@ -75,8 +78,6 @@ void init(GitHubService service, GitHubConfigFileProvider configFileProvider) th } } - - /** * When * every N hours @@ -97,7 +98,7 @@ void init(GitHubService service, GitHubConfigFileProvider configFileProvider) th * If the PR has been opened for > Y hours then "stalled-discussion" */ // TODO similar method as this, but for OVERDUE - @Scheduled(every="60s") + @Scheduled(every = "{BOT_STALLED_DISCUSSION_FLOW_POLL_DURATION}") public void checkForStalledDiscussions() throws IOException { if (!enabled) { LOG.debug("Ignoring scheduled trigger: disabled due to {}=false", ENABLE); @@ -106,7 +107,9 @@ public void checkForStalledDiscussions() throws IOException { long now = System.currentTimeMillis(); long thresh = now - 24*40*60*1000L; LOG.info("Checking for stalled discussions"); - // TODO need a parameter for this installation id + + LOG.debug("Updating installation client to get new token (expires every 1h)"); + client = service.getInstallationClient(installationId); var results = client.searchIssues() .isOpen() diff --git a/src/main/resources/application.properties b/src/main/resources/application.properties index d70cd82..9c5d6ae 100644 --- a/src/main/resources/application.properties +++ b/src/main/resources/application.properties @@ -1 +1,4 @@ -quarkus.log.level=DEBUG \ No newline at end of file +quarkus.log.level=DEBUG +bot.enable.stalled-discussion=true +bot.enable.create-draft=true +BOT_STALLED_DISCUSSION_FLOW_POLL_DURATION=86400s diff --git a/src/test/resources/application.properties b/src/test/resources/application.properties index 960ad43..6f82249 100644 --- a/src/test/resources/application.properties +++ b/src/test/resources/application.properties @@ -27,4 +27,5 @@ FqD6xNtjmuaS5enErcCAMbZtzA7TNzvGaVO+xB/GfQ2QHS8/mrTesvQsTUZwC+ji\ E0/FAoGATJvuAfgy9uiKR7za7MigYVacE0u4aD1sF7v6D4AFqBOGquPQQhePSdz9\ G/UUwySoo+AQ+rd2EPhyexjqXBhRGe+EDGFVFivaQzTT8/5bt/VddbTcw2IpmXYj\ LW6V8BbcP5MRhd2JQSRh16nWwSQJ2BdpUZFwayEEQ6UcrMfqvA0=\ ------END RSA PRIVATE KEY----- \ No newline at end of file +-----END RSA PRIVATE KEY----- +BOT_STALLED_DISCUSSION_FLOW_POLL_DURATION=86400s