From 47a6f055f2cc17abce6dfced8eaa7782d4965acc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Tue, 20 Sep 2022 17:15:33 +0200 Subject: [PATCH 1/8] Fix the scope of the quarkus-github-app-testing extension --- pom.xml | 1 + 1 file changed, 1 insertion(+) diff --git a/pom.xml b/pom.xml index 64af735..e8169ee 100644 --- a/pom.xml +++ b/pom.xml @@ -75,6 +75,7 @@ io.quarkiverse.githubapp quarkus-github-app-testing ${quarkus-github-app.version} + test From 4b34c07db42726e3889ca7d44a50f3d4a540c052 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Tue, 20 Sep 2022 17:26:08 +0200 Subject: [PATCH 2/8] Add comment-based command to trigger a draw manually --- pom.xml | 5 ++++ .../io/quarkus/github/lottery/LotteryCli.java | 30 +++++++++++++++++++ 2 files changed, 35 insertions(+) create mode 100644 src/main/java/io/quarkus/github/lottery/LotteryCli.java diff --git a/pom.xml b/pom.xml index e8169ee..0c18cb8 100644 --- a/pom.xml +++ b/pom.xml @@ -71,6 +71,11 @@ quarkus-github-app ${quarkus-github-app.version} + + io.quarkiverse.githubapp + quarkus-github-app-command-airline + ${quarkus-github-app.version} + io.quarkiverse.githubapp quarkus-github-app-testing diff --git a/src/main/java/io/quarkus/github/lottery/LotteryCli.java b/src/main/java/io/quarkus/github/lottery/LotteryCli.java new file mode 100644 index 0000000..b716040 --- /dev/null +++ b/src/main/java/io/quarkus/github/lottery/LotteryCli.java @@ -0,0 +1,30 @@ +package io.quarkus.github.lottery; + +import java.io.IOException; + +import org.kohsuke.github.GHPermissionType; + +import com.github.rvesse.airline.annotations.Cli; +import com.github.rvesse.airline.annotations.Command; + +import io.quarkiverse.githubapp.command.airline.Permission; +import io.quarkus.arc.Arc; + +@Cli(name = "/lottery", commands = { LotteryCli.DrawCommand.class }) +public class LotteryCli { + + interface Commands { + void run() throws IOException; + } + + @Command(name = "draw") + @Permission(GHPermissionType.ADMIN) + static class DrawCommand implements Commands { + @Override + public void run() throws IOException { + // Cannot inject the service for some reason, + // as Airline uses reflection and performs calls to setAccessible recursively. + Arc.container().instance(LotteryService.class).get().draw(); + } + } +} \ No newline at end of file From a593c29206ab096ec262e9567e8f7e21269cbddb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Tue, 20 Sep 2022 17:27:55 +0200 Subject: [PATCH 3/8] Protect against concurrent execution of lottery draws --- src/main/java/io/quarkus/github/lottery/LotteryService.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/io/quarkus/github/lottery/LotteryService.java b/src/main/java/io/quarkus/github/lottery/LotteryService.java index 3794cd8..1009b79 100644 --- a/src/main/java/io/quarkus/github/lottery/LotteryService.java +++ b/src/main/java/io/quarkus/github/lottery/LotteryService.java @@ -47,8 +47,8 @@ public class LotteryService { /** * Draws the lottery and sends lists of tickets to participants as necessary. */ - @Scheduled(cron = "0 0 * ? * *") // Every hour - public void draw() throws IOException { + @Scheduled(every = "1H", concurrentExecution = Scheduled.ConcurrentExecution.SKIP) // Every hour + public synchronized void draw() throws IOException { Log.info("Starting draw..."); List refs = gitHubService.listRepositories(); Log.infof("Will draw for the following repositories: %s", refs); From 8a46b5fea6bba981e8256f1657e0a8ff9fc943ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Tue, 20 Sep 2022 17:48:40 +0200 Subject: [PATCH 4/8] Use installation clients to look up installation repositories --- .../github/lottery/github/GitHubService.java | 4 ++- .../github/lottery/GitHubServiceTest.java | 36 +++++++++++++------ 2 files changed, 28 insertions(+), 12 deletions(-) diff --git a/src/main/java/io/quarkus/github/lottery/github/GitHubService.java b/src/main/java/io/quarkus/github/lottery/github/GitHubService.java index 90dcd36..3424853 100644 --- a/src/main/java/io/quarkus/github/lottery/github/GitHubService.java +++ b/src/main/java/io/quarkus/github/lottery/github/GitHubService.java @@ -24,7 +24,9 @@ public List listRepositories() throws IOException { List result = new ArrayList<>(); GitHub client = clientProvider.getApplicationClient(); for (GHAppInstallation installation : client.getApp().listInstallations()) { - for (GHRepository repository : installation.listRepositories()) { + long installationId = installation.getId(); + for (GHRepository repository : clientProvider.getInstallationClient(installationId).getApp() + .getInstallationById(installationId).listRepositories()) { result.add(new GitHubRepositoryRef(installation.getId(), repository.getFullName())); } } diff --git a/src/test/java/io/quarkus/github/lottery/GitHubServiceTest.java b/src/test/java/io/quarkus/github/lottery/GitHubServiceTest.java index 88624f4..21a53c9 100644 --- a/src/test/java/io/quarkus/github/lottery/GitHubServiceTest.java +++ b/src/test/java/io/quarkus/github/lottery/GitHubServiceTest.java @@ -71,17 +71,31 @@ void listRepositories() throws IOException { given() .github(mocks -> { var applicationClient = mocks.applicationClient(); - var appMock = mocks.ghObject(GHApp.class, repoRef.installationId()); - when(applicationClient.getApp()).thenReturn(appMock); - - var installationMock = mocks.ghObject(GHAppInstallation.class, repoRef.installationId()); - var installationsMocks = mockPagedIterable(installationMock); - when(appMock.listInstallations()).thenReturn(installationsMocks); - when(installationMock.getId()).thenReturn(repoRef.installationId()); - var installationRepositoryMock = Mockito.mock(GHRepository.class); - var installationRepositoryMocks = mockPagedIterable(installationRepositoryMock); - when(installationMock.listRepositories()).thenReturn(installationRepositoryMocks); - when(installationRepositoryMock.getFullName()).thenReturn(repoRef.repositoryName()); + { + // Scope: application client + var appMock = mocks.ghObject(GHApp.class, 1); + when(applicationClient.getApp()).thenReturn(appMock); + + var installationMock = Mockito.mock(GHAppInstallation.class); + when(installationMock.getId()).thenReturn(repoRef.installationId()); + var installationsMocks = mockPagedIterable(installationMock); + when(appMock.listInstallations()).thenReturn(installationsMocks); + } + + var installationClient = mocks.installationClient(repoRef.installationId()); + { + // Scope: installation client + var appMock = mocks.ghObject(GHApp.class, 2); + when(installationClient.getApp()).thenReturn(appMock); + + var installationMock = Mockito.mock(GHAppInstallation.class); + when(appMock.getInstallationById(repoRef.installationId())).thenReturn(installationMock); + + var installationRepositoryMock = Mockito.mock(GHRepository.class); + var installationRepositoryMocks = mockPagedIterable(installationRepositoryMock); + when(installationMock.listRepositories()).thenReturn(installationRepositoryMocks); + when(installationRepositoryMock.getFullName()).thenReturn(repoRef.repositoryName()); + } }) .when(() -> { assertThat(gitHubService.listRepositories()) From ad25d64abd1cdb6110c4f2080a36127e2c715afc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Wed, 21 Sep 2022 17:25:13 +0200 Subject: [PATCH 5/8] Use the latest snapshot of github-api --- .github/workflows/build.yml | 6 ++++++ pom.xml | 5 +++++ 2 files changed, 11 insertions(+) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index ff713bc..d8919ec 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -49,6 +49,12 @@ jobs: # refresh cache every month to avoid unlimited growth key: maven-repo-pr-${{ runner.os }}-${{ steps.get-date.outputs.date }} + - name: Check out yrodiere:github-api i1082-listRepositories + run: git clone -b i1082-listRepositories https://github.com/yrodiere/github-api.git + + - name: Build github-api SNAPSHOT + run: cd github-api && mvn -B clean install -DskipTests && cd - + - name: Validate formatting run: mvn -B clean formatter:validate diff --git a/pom.xml b/pom.xml index 0c18cb8..6eece9b 100644 --- a/pom.xml +++ b/pom.xml @@ -51,6 +51,11 @@ pom import + + org.kohsuke + github-api + 1.309-SNAPSHOT + From 94454cef400c219564f3181d902689eb1a4637ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Wed, 21 Sep 2022 17:29:08 +0200 Subject: [PATCH 6/8] Use .yml extension for YAML files For consistency with Quarkus GitHub Bot --- .../java/io/quarkus/github/lottery/config/LotteryConfig.java | 2 +- src/test/java/io/quarkus/github/lottery/GitHubServiceTest.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/io/quarkus/github/lottery/config/LotteryConfig.java b/src/main/java/io/quarkus/github/lottery/config/LotteryConfig.java index 8e0f486..bb6d66c 100644 --- a/src/main/java/io/quarkus/github/lottery/config/LotteryConfig.java +++ b/src/main/java/io/quarkus/github/lottery/config/LotteryConfig.java @@ -16,7 +16,7 @@ public record LotteryConfig( @JsonProperty(required = true) BucketsConfig buckets, List participants) { - public static final String FILE_NAME = "quarkus-github-lottery.yaml"; + public static final String FILE_NAME = "quarkus-github-lottery.yml"; public record BucketsConfig( @JsonProperty(required = true) TriageBucketConfig triage) { diff --git a/src/test/java/io/quarkus/github/lottery/GitHubServiceTest.java b/src/test/java/io/quarkus/github/lottery/GitHubServiceTest.java index 21a53c9..581d133 100644 --- a/src/test/java/io/quarkus/github/lottery/GitHubServiceTest.java +++ b/src/test/java/io/quarkus/github/lottery/GitHubServiceTest.java @@ -114,7 +114,7 @@ void fetchLotteryConfig() throws IOException { given() .github(mocks -> { var repositoryMock = mocks.repository(repoRef.repositoryName()); - mocks.configFile(repositoryMock, "quarkus-github-lottery.yaml") + mocks.configFile(repositoryMock, "quarkus-github-lottery.yml") .fromString(""" notifications: createIssues: From 64047b3bc169d84dc579bbcf5b12a318a41c500e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Wed, 21 Sep 2022 18:14:31 +0200 Subject: [PATCH 7/8] Fix GitHubService after some real-life manual tests --- .../lottery/github/GitHubInstallationRef.java | 10 +++ .../lottery/github/GitHubRepository.java | 20 ++--- .../lottery/github/GitHubRepositoryRef.java | 6 +- .../github/lottery/github/GitHubService.java | 12 ++- .../lottery/history/HistoryService.java | 4 +- .../notification/NotificationService.java | 2 +- .../github/lottery/GitHubServiceTest.java | 87 ++++++++----------- .../github/lottery/HistoryServiceTest.java | 25 +++--- .../lottery/LotterySingleRepositoryTest.java | 5 +- .../github/lottery/MessageFormatterTest.java | 5 +- .../lottery/NotificationServiceTest.java | 7 +- 11 files changed, 97 insertions(+), 86 deletions(-) create mode 100644 src/main/java/io/quarkus/github/lottery/github/GitHubInstallationRef.java diff --git a/src/main/java/io/quarkus/github/lottery/github/GitHubInstallationRef.java b/src/main/java/io/quarkus/github/lottery/github/GitHubInstallationRef.java new file mode 100644 index 0000000..0fed9e0 --- /dev/null +++ b/src/main/java/io/quarkus/github/lottery/github/GitHubInstallationRef.java @@ -0,0 +1,10 @@ +package io.quarkus.github.lottery.github; + +/** + * A reference to a GitHub application installation. + * + * @param appName The application name. + * @param installationId The installation ID. + */ +public record GitHubInstallationRef(String appName, long installationId) { +} diff --git a/src/main/java/io/quarkus/github/lottery/github/GitHubRepository.java b/src/main/java/io/quarkus/github/lottery/github/GitHubRepository.java index 896e5c0..bc6b7bc 100644 --- a/src/main/java/io/quarkus/github/lottery/github/GitHubRepository.java +++ b/src/main/java/io/quarkus/github/lottery/github/GitHubRepository.java @@ -61,13 +61,13 @@ public GitHubRepositoryRef ref() { return ref; } - public String selfUsername() throws IOException { - return client().getMyself().getLogin(); + public String selfLogin() { + return ref.installationRef().appName() + "[bot]"; } private GitHub client() { if (client == null) { - client = clientProvider.getInstallationClient(ref.installationId()); + client = clientProvider.getInstallationClient(ref.installationRef().installationId()); } return client; } @@ -81,7 +81,7 @@ private GHRepository repository() throws IOException { private DynamicGraphQLClient graphQLClient() { if (graphQLClient == null) { - graphQLClient = clientProvider.getInstallationGraphQLClient(ref.installationId()); + graphQLClient = clientProvider.getInstallationGraphQLClient(ref.installationRef().installationId()); } return graphQLClient; } @@ -120,9 +120,9 @@ public void commentOnDedicatedIssue(String username, String topic, String markdo issue.comment(markdownBody); } - public Stream extractCommentsFromDedicatedIssue(String username, String topic, Instant since) + public Stream extractCommentsFromDedicatedIssue(String login, String topic, Instant since) throws IOException { - return getDedicatedIssue(username, topic) + return getDedicatedIssue(login, topic) .map(uncheckedIO(issue -> getAppCommentsSince(issue, since))) .orElse(Stream.of()) .map(GHIssueComment::getBody); @@ -145,14 +145,14 @@ private GHIssue createDedicatedIssue(String username, String topic) throws IOExc } private Optional getLastAppComment(GHIssue issue) throws IOException { - long selfId = client().getMyself().getId(); + String selfLogin = selfLogin(); // TODO ideally we'd use the "since" API parameter to ignore // older comments (e.g. 1+ year old) that are unlikely to be relevant // (see 'since' in https://docs.github.com/en/rest/issues/comments#list-issue-comments) // but that's not supported yet in the library we're using... GHIssueComment lastNotificationComment = null; for (GHIssueComment comment : issue.listComments()) { - if (selfId == comment.getUser().getId()) { + if (selfLogin.equals(comment.getUser().getLogin())) { lastNotificationComment = comment; } } @@ -160,12 +160,12 @@ private Optional getLastAppComment(GHIssue issue) throws IOExcep } private Stream getAppCommentsSince(GHIssue issue, Instant since) throws IOException { - long selfId = client().getMyself().getId(); + String selfLogin = selfLogin(); // TODO ideally we'd use the "since" API parameter to ignore older comments // (see 'since' in https://docs.github.com/en/rest/issues/comments#list-issue-comments) // but that's not supported yet in the library we're using... return toStream(issue.listComments()) - .filter(uncheckedIO((GHIssueComment comment) -> selfId == comment.getUser().getId() + .filter(uncheckedIO((GHIssueComment comment) -> selfLogin.equals(comment.getUser().getLogin()) && !comment.getCreatedAt().toInstant().isBefore(since))::apply); } diff --git a/src/main/java/io/quarkus/github/lottery/github/GitHubRepositoryRef.java b/src/main/java/io/quarkus/github/lottery/github/GitHubRepositoryRef.java index 8fb8d5b..778e5b4 100644 --- a/src/main/java/io/quarkus/github/lottery/github/GitHubRepositoryRef.java +++ b/src/main/java/io/quarkus/github/lottery/github/GitHubRepositoryRef.java @@ -3,13 +3,13 @@ /** * A reference to a GitHub repository as viewed from a GitHub App installation. * - * @param installationId The installation ID. + * @param installationRef A reference to the GitHub installation. * @param repositoryName The full name of the GitHub repository. */ -public record GitHubRepositoryRef(long installationId, String repositoryName) { +public record GitHubRepositoryRef(GitHubInstallationRef installationRef, String repositoryName) { @Override public String toString() { - return repositoryName + "(through installation " + installationId + ")"; + return repositoryName + "(through " + installationRef + ")"; } } diff --git a/src/main/java/io/quarkus/github/lottery/github/GitHubService.java b/src/main/java/io/quarkus/github/lottery/github/GitHubService.java index 3424853..271b57b 100644 --- a/src/main/java/io/quarkus/github/lottery/github/GitHubService.java +++ b/src/main/java/io/quarkus/github/lottery/github/GitHubService.java @@ -8,6 +8,7 @@ import io.quarkiverse.githubapp.GitHubClientProvider; import io.quarkiverse.githubapp.GitHubConfigFileProvider; +import org.kohsuke.github.GHApp; import org.kohsuke.github.GHAppInstallation; import org.kohsuke.github.GHRepository; import org.kohsuke.github.GitHub; @@ -23,11 +24,14 @@ public class GitHubService { public List listRepositories() throws IOException { List result = new ArrayList<>(); GitHub client = clientProvider.getApplicationClient(); - for (GHAppInstallation installation : client.getApp().listInstallations()) { + GHApp app = client.getApp(); + String appName = app.getName(); + for (GHAppInstallation installation : app.listInstallations()) { long installationId = installation.getId(); - for (GHRepository repository : clientProvider.getInstallationClient(installationId).getApp() - .getInstallationById(installationId).listRepositories()) { - result.add(new GitHubRepositoryRef(installation.getId(), repository.getFullName())); + var installationRef = new GitHubInstallationRef(appName, installationId); + for (GHRepository repository : clientProvider.getInstallationClient(installationId) + .getInstallation().listRepositories()) { + result.add(new GitHubRepositoryRef(installationRef, repository.getFullName())); } } return result; diff --git a/src/main/java/io/quarkus/github/lottery/history/HistoryService.java b/src/main/java/io/quarkus/github/lottery/history/HistoryService.java index 0581d1b..384d911 100644 --- a/src/main/java/io/quarkus/github/lottery/history/HistoryService.java +++ b/src/main/java/io/quarkus/github/lottery/history/HistoryService.java @@ -28,7 +28,7 @@ public LotteryHistory fetch(DrawRef drawRef, LotteryConfig config) throws IOExce var persistenceRepo = persistenceRepo(drawRef, config); var history = new LotteryHistory(drawRef.instant(), config.buckets()); String historyTopic = messageFormatter.formatHistoryTopicText(drawRef); - persistenceRepo.extractCommentsFromDedicatedIssue(persistenceRepo.selfUsername(), historyTopic, history.since()) + persistenceRepo.extractCommentsFromDedicatedIssue(persistenceRepo.selfLogin(), historyTopic, history.since()) .flatMap(uncheckedIO(message -> messageFormatter.extractPayloadFromHistoryBodyMarkdown(message).stream())) .forEach(history::add); return history; @@ -38,7 +38,7 @@ public void append(DrawRef drawRef, LotteryConfig config, List { @@ -168,7 +170,7 @@ void fetchLotteryConfig() throws IOException { @Test void issuesWithLabel() throws IOException { - GitHubRepositoryRef repoRef = new GitHubRepositoryRef(1234L, "quarkusio/quarkus"); + var repoRef = new GitHubRepositoryRef(installationRef, "quarkusio/quarkus"); var queryIssuesBuilderMock = Mockito.mock(GHIssueQueryBuilder.ForRepository.class, withSettings().defaultAnswer(Answers.RETURNS_SELF)); @@ -223,7 +225,7 @@ void issuesWithLabel() throws IOException { @Test void extractCommentsFromDedicatedIssue_dedicatedIssueDoesNotExist() throws Exception { - var repoRef = new GitHubRepositoryRef(1234L, "quarkusio/quarkus-lottery-reports"); + var repoRef = new GitHubRepositoryRef(installationRef, "quarkusio/quarkus-lottery-reports"); var since = LocalDateTime.of(2017, 11, 6, 19, 0).toInstant(ZoneOffset.UTC); var queryIssuesBuilderMock = Mockito.mock(GHIssueQueryBuilder.ForRepository.class, @@ -242,12 +244,12 @@ void extractCommentsFromDedicatedIssue_dedicatedIssueDoesNotExist() throws Excep .when(() -> { var repo = gitHubService.repository(repoRef); - assertThat(repo.extractCommentsFromDedicatedIssue("quarkus-lottery-bot", + assertThat(repo.extractCommentsFromDedicatedIssue(installationRef.appName() + "[bot]", "Lottery history for quarkusio/quarkus", since)) .isEmpty(); }) .then().github(mocks -> { - verify(queryIssuesBuilderMock).assignee("quarkus-lottery-bot"); + verify(queryIssuesBuilderMock).assignee(installationRef.appName() + "[bot]"); verifyNoMoreInteractions(queryIssuesBuilderMock); verifyNoMoreInteractions(mocks.ghObjects()); @@ -256,7 +258,7 @@ void extractCommentsFromDedicatedIssue_dedicatedIssueDoesNotExist() throws Excep @Test void extractCommentsFromDedicatedIssue_dedicatedIssueExists_appCommentsDoNotExist() throws Exception { - var repoRef = new GitHubRepositoryRef(1234L, "quarkusio/quarkus-lottery-reports"); + var repoRef = new GitHubRepositoryRef(installationRef, "quarkusio/quarkus-lottery-reports"); var since = LocalDateTime.of(2017, 11, 6, 19, 0).toInstant(ZoneOffset.UTC); var queryIssuesBuilderMock = Mockito.mock(GHIssueQueryBuilder.ForRepository.class, @@ -272,13 +274,8 @@ void extractCommentsFromDedicatedIssue_dedicatedIssueExists_appCommentsDoNotExis var issuesMocks = mockPagedIterable(issue1Mock, issue2Mock); when(queryIssuesBuilderMock.list()).thenReturn(issuesMocks); - var clientMock = mocks.installationClient(repoRef.installationId()); - var mySelfMock = mocks.ghObject(GHMyself.class, 1L); - when(clientMock.getMyself()).thenReturn(mySelfMock); - when(mySelfMock.getId()).thenReturn(1L); - var someoneElseMock = mocks.ghObject(GHUser.class, 2L); - when(someoneElseMock.getId()).thenReturn(2L); + when(someoneElseMock.getLogin()).thenReturn("yrodiere"); var issue2Comment1Mock = mocks.issueComment(201); when(issue2Comment1Mock.getUser()).thenReturn(someoneElseMock); @@ -290,12 +287,12 @@ void extractCommentsFromDedicatedIssue_dedicatedIssueExists_appCommentsDoNotExis .when(() -> { var repo = gitHubService.repository(repoRef); - assertThat(repo.extractCommentsFromDedicatedIssue("quarkus-lottery-bot", + assertThat(repo.extractCommentsFromDedicatedIssue(installationRef.appName() + "[bot]", "Lottery history for quarkusio/quarkus", since)) .isEmpty(); }) .then().github(mocks -> { - verify(queryIssuesBuilderMock).assignee("quarkus-lottery-bot"); + verify(queryIssuesBuilderMock).assignee(installationRef.appName() + "[bot]"); verifyNoMoreInteractions(queryIssuesBuilderMock); verifyNoMoreInteractions(mocks.ghObjects()); @@ -304,7 +301,7 @@ void extractCommentsFromDedicatedIssue_dedicatedIssueExists_appCommentsDoNotExis @Test void extractCommentsFromDedicatedIssue_dedicatedIssueExists_appCommentsExist_allTooOld() throws Exception { - var repoRef = new GitHubRepositoryRef(1234L, "quarkusio/quarkus-lottery-reports"); + var repoRef = new GitHubRepositoryRef(installationRef, "quarkusio/quarkus-lottery-reports"); var since = LocalDateTime.of(2017, 11, 6, 19, 0).toInstant(ZoneOffset.UTC); var queryIssuesBuilderMock = Mockito.mock(GHIssueQueryBuilder.ForRepository.class, @@ -320,13 +317,10 @@ void extractCommentsFromDedicatedIssue_dedicatedIssueExists_appCommentsExist_all var issuesMocks = mockPagedIterable(issue1Mock, issue2Mock); when(queryIssuesBuilderMock.list()).thenReturn(issuesMocks); - var clientMock = mocks.installationClient(repoRef.installationId()); var mySelfMock = mocks.ghObject(GHMyself.class, 1L); - when(clientMock.getMyself()).thenReturn(mySelfMock); - when(mySelfMock.getId()).thenReturn(1L); - + when(mySelfMock.getLogin()).thenReturn(installationRef.appName() + "[bot]"); var someoneElseMock = mocks.ghObject(GHUser.class, 2L); - when(someoneElseMock.getId()).thenReturn(2L); + when(someoneElseMock.getLogin()).thenReturn("yrodiere"); var issue2Comment1Mock = mocks.issueComment(201); when(issue2Comment1Mock.getUser()).thenReturn(mySelfMock); @@ -342,12 +336,12 @@ void extractCommentsFromDedicatedIssue_dedicatedIssueExists_appCommentsExist_all .when(() -> { var repo = gitHubService.repository(repoRef); - assertThat(repo.extractCommentsFromDedicatedIssue("quarkus-lottery-bot", + assertThat(repo.extractCommentsFromDedicatedIssue(installationRef.appName() + "[bot]", "Lottery history for quarkusio/quarkus", since)) .isEmpty(); }) .then().github(mocks -> { - verify(queryIssuesBuilderMock).assignee("quarkus-lottery-bot"); + verify(queryIssuesBuilderMock).assignee(installationRef.appName() + "[bot]"); verifyNoMoreInteractions(queryIssuesBuilderMock); verifyNoMoreInteractions(mocks.ghObjects()); @@ -356,7 +350,7 @@ void extractCommentsFromDedicatedIssue_dedicatedIssueExists_appCommentsExist_all @Test void extractCommentsFromDedicatedIssue_dedicatedIssueExists_appCommentsExist() throws Exception { - var repoRef = new GitHubRepositoryRef(1234L, "quarkusio/quarkus-lottery-reports"); + var repoRef = new GitHubRepositoryRef(installationRef, "quarkusio/quarkus-lottery-reports"); var since = LocalDateTime.of(2017, 11, 6, 19, 0).toInstant(ZoneOffset.UTC); var queryIssuesBuilderMock = Mockito.mock(GHIssueQueryBuilder.ForRepository.class, @@ -372,13 +366,10 @@ void extractCommentsFromDedicatedIssue_dedicatedIssueExists_appCommentsExist() t var issuesMocks = mockPagedIterable(issue1Mock, issue2Mock); when(queryIssuesBuilderMock.list()).thenReturn(issuesMocks); - var clientMock = mocks.installationClient(repoRef.installationId()); var mySelfMock = mocks.ghObject(GHMyself.class, 1L); - when(clientMock.getMyself()).thenReturn(mySelfMock); - when(mySelfMock.getId()).thenReturn(1L); - + when(mySelfMock.getLogin()).thenReturn(installationRef.appName() + "[bot]"); var someoneElseMock = mocks.ghObject(GHUser.class, 2L); - when(someoneElseMock.getId()).thenReturn(2L); + when(someoneElseMock.getLogin()).thenReturn("yrodiere"); var issue2Comment1Mock = mocks.issueComment(201); when(issue2Comment1Mock.getUser()).thenReturn(mySelfMock); @@ -415,7 +406,7 @@ void extractCommentsFromDedicatedIssue_dedicatedIssueExists_appCommentsExist() t @SuppressWarnings("unchecked") @Test void commentOnDedicatedIssue_dedicatedIssueExists_open() throws Exception { - var repoRef = new GitHubRepositoryRef(1234L, "quarkusio/quarkus-lottery-reports"); + var repoRef = new GitHubRepositoryRef(installationRef, "quarkusio/quarkus-lottery-reports"); var commentToMinimizeNodeId = "MDM6Qm90NzUwNjg0Mzg="; var queryIssuesBuilderMock = Mockito.mock(GHIssueQueryBuilder.ForRepository.class, @@ -433,13 +424,10 @@ void commentOnDedicatedIssue_dedicatedIssueExists_open() throws Exception { when(issue2Mock.getState()).thenReturn(GHIssueState.OPEN); - var clientMock = mocks.installationClient(repoRef.installationId()); var mySelfMock = mocks.ghObject(GHMyself.class, 1L); - when(clientMock.getMyself()).thenReturn(mySelfMock); - when(mySelfMock.getId()).thenReturn(1L); - + when(mySelfMock.getLogin()).thenReturn(installationRef.appName() + "[bot]"); var someoneElseMock = mocks.ghObject(GHUser.class, 2L); - when(someoneElseMock.getId()).thenReturn(2L); + when(someoneElseMock.getLogin()).thenReturn("yrodiere"); var issue2Comment1Mock = mocks.issueComment(201); when(issue2Comment1Mock.getUser()).thenReturn(mySelfMock); @@ -462,7 +450,7 @@ void commentOnDedicatedIssue_dedicatedIssueExists_open() throws Exception { verify(queryIssuesBuilderMock).assignee("yrodiere"); var mapCaptor = ArgumentCaptor.forClass(Map.class); - verify(mocks.installationGraphQLClient(repoRef.installationId())) + verify(mocks.installationGraphQLClient(installationRef.installationId())) .executeSync(anyString(), mapCaptor.capture()); verify(mocks.issue(2)).comment("Some content"); @@ -477,7 +465,7 @@ void commentOnDedicatedIssue_dedicatedIssueExists_open() throws Exception { @SuppressWarnings("unchecked") @Test void commentOnDedicatedIssue_dedicatedIssueExists_closed() throws Exception { - var repoRef = new GitHubRepositoryRef(1234L, "quarkusio/quarkus-lottery-reports"); + var repoRef = new GitHubRepositoryRef(installationRef, "quarkusio/quarkus-lottery-reports"); var commentToMinimizeNodeId = "MDM6Qm90NzUwNjg0Mzg="; var queryIssuesBuilderMock = Mockito.mock(GHIssueQueryBuilder.ForRepository.class, @@ -495,13 +483,10 @@ void commentOnDedicatedIssue_dedicatedIssueExists_closed() throws Exception { when(issue2Mock.getState()).thenReturn(GHIssueState.CLOSED); - var clientMock = mocks.installationClient(repoRef.installationId()); var mySelfMock = mocks.ghObject(GHMyself.class, 1L); - when(clientMock.getMyself()).thenReturn(mySelfMock); - when(mySelfMock.getId()).thenReturn(1L); - + when(mySelfMock.getLogin()).thenReturn(installationRef.appName() + "[bot]"); var someoneElseMock = mocks.ghObject(GHUser.class, 2L); - when(someoneElseMock.getId()).thenReturn(2L); + when(someoneElseMock.getLogin()).thenReturn("yrodiere"); var issue2Comment1Mock = mocks.issueComment(201); when(issue2Comment1Mock.getUser()).thenReturn(mySelfMock); @@ -526,7 +511,7 @@ void commentOnDedicatedIssue_dedicatedIssueExists_closed() throws Exception { verify(mocks.issue(2)).reopen(); var mapCaptor = ArgumentCaptor.forClass(Map.class); - verify(mocks.installationGraphQLClient(repoRef.installationId())) + verify(mocks.installationGraphQLClient(installationRef.installationId())) .executeSync(anyString(), mapCaptor.capture()); verify(mocks.issue(2)).comment("Some content"); @@ -540,7 +525,7 @@ void commentOnDedicatedIssue_dedicatedIssueExists_closed() throws Exception { @Test void commentOnDedicatedIssue_dedicatedIssueDoesNotExist() throws IOException { - var repoRef = new GitHubRepositoryRef(1234L, "quarkusio/quarkus-lottery-reports"); + var repoRef = new GitHubRepositoryRef(installationRef, "quarkusio/quarkus-lottery-reports"); var queryIssuesBuilderMock = Mockito.mock(GHIssueQueryBuilder.ForRepository.class, withSettings().defaultAnswer(Answers.RETURNS_SELF)); var issueBuilderMock = Mockito.mock(GHIssueBuilder.class, diff --git a/src/test/java/io/quarkus/github/lottery/HistoryServiceTest.java b/src/test/java/io/quarkus/github/lottery/HistoryServiceTest.java index 66d69ea..68fe084 100644 --- a/src/test/java/io/quarkus/github/lottery/HistoryServiceTest.java +++ b/src/test/java/io/quarkus/github/lottery/HistoryServiceTest.java @@ -20,6 +20,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import io.quarkus.github.lottery.github.GitHubInstallationRef; import org.mockito.Mockito; import org.mockito.junit.jupiter.MockitoExtension; @@ -42,6 +43,7 @@ public class HistoryServiceTest { MessageFormatter messageFormatterMock; + GitHubInstallationRef installationRef; GitHubRepositoryRef repoRef; Instant now; DrawRef drawRef; @@ -53,7 +55,8 @@ public class HistoryServiceTest { void setup() { gitHubServiceMock = Mockito.mock(GitHubService.class); QuarkusMock.installMockForType(gitHubServiceMock, GitHubService.class); - repoRef = new GitHubRepositoryRef(1L, "quarkusio/quarkus"); + installationRef = new GitHubInstallationRef("quarkus-github-lottery", 1L); + repoRef = new GitHubRepositoryRef(installationRef, "quarkusio/quarkus"); persistenceRepoMock = Mockito.mock(GitHubRepository.class); @@ -73,14 +76,14 @@ void lastNotificationInstantForUser_noHistory() throws Exception { new LotteryConfig.BucketsConfig.TriageBucketConfig("needs-triage", Duration.ofDays(3))), List.of()); - var persistenceRepoRef = new GitHubRepositoryRef(repoRef.installationId(), + var persistenceRepoRef = new GitHubRepositoryRef(installationRef, config.notifications().createIssues().repository()); when(gitHubServiceMock.repository(persistenceRepoRef)).thenReturn(persistenceRepoMock); String topic = "Lottery history for quarkusio/quarkus"; when(messageFormatterMock.formatHistoryTopicText(drawRef)).thenReturn(topic); String selfUsername = "quarkus-lottery-bot"; - when(persistenceRepoMock.selfUsername()).thenReturn(selfUsername); + when(persistenceRepoMock.selfLogin()).thenReturn(selfUsername); when(persistenceRepoMock.extractCommentsFromDedicatedIssue(eq(selfUsername), eq(topic), any())) .thenAnswer(ignored -> Stream.of()); @@ -101,14 +104,14 @@ void lastNotificationInstantForUser_notNotified() throws Exception { new LotteryConfig.BucketsConfig.TriageBucketConfig("needs-triage", Duration.ofDays(3))), List.of()); - var persistenceRepoRef = new GitHubRepositoryRef(repoRef.installationId(), + var persistenceRepoRef = new GitHubRepositoryRef(installationRef, config.notifications().createIssues().repository()); when(gitHubServiceMock.repository(persistenceRepoRef)).thenReturn(persistenceRepoMock); String topic = "Lottery history for quarkusio/quarkus"; when(messageFormatterMock.formatHistoryTopicText(drawRef)).thenReturn(topic); String selfUsername = "quarkus-lottery-bot"; - when(persistenceRepoMock.selfUsername()).thenReturn(selfUsername); + when(persistenceRepoMock.selfLogin()).thenReturn(selfUsername); String historyBody = "Some content"; when(persistenceRepoMock.extractCommentsFromDedicatedIssue(eq(selfUsername), eq(topic), any())) .thenAnswer(ignored -> Stream.of(historyBody)); @@ -134,14 +137,14 @@ void lastNotificationInstantForUser_notifiedRecently() throws Exception { new LotteryConfig.BucketsConfig.TriageBucketConfig("needs-triage", Duration.ofDays(3))), List.of()); - var persistenceRepoRef = new GitHubRepositoryRef(repoRef.installationId(), + var persistenceRepoRef = new GitHubRepositoryRef(installationRef, config.notifications().createIssues().repository()); when(gitHubServiceMock.repository(persistenceRepoRef)).thenReturn(persistenceRepoMock); String topic = "Lottery history for quarkusio/quarkus"; when(messageFormatterMock.formatHistoryTopicText(drawRef)).thenReturn(topic); String selfUsername = "quarkus-lottery-bot"; - when(persistenceRepoMock.selfUsername()).thenReturn(selfUsername); + when(persistenceRepoMock.selfLogin()).thenReturn(selfUsername); String historyBody = "Some content"; when(persistenceRepoMock.extractCommentsFromDedicatedIssue(eq(selfUsername), eq(topic), any())) .thenAnswer(ignored -> Stream.of(historyBody)); @@ -169,14 +172,14 @@ void lastNotificationExpiredForIssueNumber_noHistory() throws Exception { new LotteryConfig.BucketsConfig.TriageBucketConfig("needs-triage", Duration.ofDays(3))), List.of()); - var persistenceRepoRef = new GitHubRepositoryRef(repoRef.installationId(), + var persistenceRepoRef = new GitHubRepositoryRef(installationRef, config.notifications().createIssues().repository()); when(gitHubServiceMock.repository(persistenceRepoRef)).thenReturn(persistenceRepoMock); String topic = "Lottery history for quarkusio/quarkus"; when(messageFormatterMock.formatHistoryTopicText(drawRef)).thenReturn(topic); String selfUsername = "quarkus-lottery-bot"; - when(persistenceRepoMock.selfUsername()).thenReturn(selfUsername); + when(persistenceRepoMock.selfLogin()).thenReturn(selfUsername); when(persistenceRepoMock.extractCommentsFromDedicatedIssue(eq(selfUsername), eq(topic), any())) .thenAnswer(ignored -> Stream.of()); @@ -197,14 +200,14 @@ void lastNotificationExpiredForIssueNumber() throws Exception { new LotteryConfig.BucketsConfig.TriageBucketConfig("needs-triage", Duration.ofDays(3))), List.of()); - var persistenceRepoRef = new GitHubRepositoryRef(repoRef.installationId(), + var persistenceRepoRef = new GitHubRepositoryRef(installationRef, config.notifications().createIssues().repository()); when(gitHubServiceMock.repository(persistenceRepoRef)).thenReturn(persistenceRepoMock); String topic = "Lottery history for quarkusio/quarkus"; when(messageFormatterMock.formatHistoryTopicText(drawRef)).thenReturn(topic); String selfUsername = "quarkus-lottery-bot"; - when(persistenceRepoMock.selfUsername()).thenReturn(selfUsername); + when(persistenceRepoMock.selfLogin()).thenReturn(selfUsername); String historyBody = "Some content"; when(persistenceRepoMock.extractCommentsFromDedicatedIssue(eq(selfUsername), eq(topic), any())) .thenAnswer(ignored -> Stream.of(historyBody)); diff --git a/src/test/java/io/quarkus/github/lottery/LotterySingleRepositoryTest.java b/src/test/java/io/quarkus/github/lottery/LotterySingleRepositoryTest.java index a502dd2..48974da 100644 --- a/src/test/java/io/quarkus/github/lottery/LotterySingleRepositoryTest.java +++ b/src/test/java/io/quarkus/github/lottery/LotterySingleRepositoryTest.java @@ -30,6 +30,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import io.quarkus.github.lottery.github.GitHubInstallationRef; import org.mockito.ArgumentCaptor; import org.mockito.Mockito; import org.mockito.junit.jupiter.MockitoExtension; @@ -57,6 +58,7 @@ public class LotterySingleRepositoryTest { NotificationService notificationServiceMock; HistoryService historyServiceMock; + GitHubInstallationRef installationRef; GitHubRepositoryRef repoRef; DrawRef drawRef; @@ -67,7 +69,8 @@ public class LotterySingleRepositoryTest { void setup() throws IOException { gitHubServiceMock = Mockito.mock(GitHubService.class); QuarkusMock.installMockForType(gitHubServiceMock, GitHubService.class); - repoRef = new GitHubRepositoryRef(1L, "quarkusio/quarkus"); + installationRef = new GitHubInstallationRef("quarkus-github-lottery", 1L); + repoRef = new GitHubRepositoryRef(installationRef, "quarkusio/quarkus"); when(gitHubServiceMock.listRepositories()).thenReturn(List.of(repoRef)); repoMock = Mockito.mock(GitHubRepository.class); diff --git a/src/test/java/io/quarkus/github/lottery/MessageFormatterTest.java b/src/test/java/io/quarkus/github/lottery/MessageFormatterTest.java index a2d23a4..016d323 100644 --- a/src/test/java/io/quarkus/github/lottery/MessageFormatterTest.java +++ b/src/test/java/io/quarkus/github/lottery/MessageFormatterTest.java @@ -15,6 +15,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import io.quarkus.github.lottery.github.GitHubInstallationRef; import org.mockito.junit.jupiter.MockitoExtension; import io.quarkus.github.lottery.draw.DrawRef; @@ -28,6 +29,7 @@ @ExtendWith(MockitoExtension.class) public class MessageFormatterTest { + GitHubInstallationRef installationRef; GitHubRepositoryRef repoRef; DrawRef drawRef; @@ -36,7 +38,8 @@ public class MessageFormatterTest { @BeforeEach void setup() { - repoRef = new GitHubRepositoryRef(1L, "quarkusio/quarkus"); + installationRef = new GitHubInstallationRef("quarkus-github-lottery", 1L); + repoRef = new GitHubRepositoryRef(installationRef, "quarkusio/quarkus"); var now = LocalDateTime.of(2017, 11, 6, 6, 0).toInstant(ZoneOffset.UTC); drawRef = new DrawRef(repoRef, now); } diff --git a/src/test/java/io/quarkus/github/lottery/NotificationServiceTest.java b/src/test/java/io/quarkus/github/lottery/NotificationServiceTest.java index 4ec6e5d..74f78e0 100644 --- a/src/test/java/io/quarkus/github/lottery/NotificationServiceTest.java +++ b/src/test/java/io/quarkus/github/lottery/NotificationServiceTest.java @@ -16,6 +16,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import io.quarkus.github.lottery.github.GitHubInstallationRef; import org.mockito.Mockito; import org.mockito.junit.jupiter.MockitoExtension; @@ -40,6 +41,7 @@ public class NotificationServiceTest { MessageFormatter messageFormatterMock; + GitHubInstallationRef installationRef; GitHubRepositoryRef repoRef; DrawRef drawRef; @@ -50,7 +52,8 @@ public class NotificationServiceTest { void setup() { gitHubServiceMock = Mockito.mock(GitHubService.class); QuarkusMock.installMockForType(gitHubServiceMock, GitHubService.class); - repoRef = new GitHubRepositoryRef(1L, "quarkusio/quarkus"); + installationRef = new GitHubInstallationRef("quarkus-github-lottery", 1L); + repoRef = new GitHubRepositoryRef(installationRef, "quarkusio/quarkus"); notificationRepoMock = Mockito.mock(GitHubRepository.class); @@ -65,7 +68,7 @@ void simple() throws IOException { var config = new LotteryConfig.NotificationsConfig( new LotteryConfig.NotificationsConfig.CreateIssuesConfig("quarkusio/quarkus-lottery-reports")); - var notificationRepoRef = new GitHubRepositoryRef(repoRef.installationId(), config.createIssues().repository()); + var notificationRepoRef = new GitHubRepositoryRef(installationRef, config.createIssues().repository()); when(gitHubServiceMock.repository(notificationRepoRef)).thenReturn(notificationRepoMock); Notifier notifier = notificationService.notifier(drawRef, config); From 0d48fe39c823b60ca275c74d2b0a6f9e82b99c3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Wed, 21 Sep 2022 18:48:10 +0200 Subject: [PATCH 8/8] Enable linkifying in lottery history comments --- .../github/lottery/message/MessageFormatter.java | 11 ++++++----- .../quarkus/github/lottery/MessageFormatterTest.java | 8 ++++---- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/main/java/io/quarkus/github/lottery/message/MessageFormatter.java b/src/main/java/io/quarkus/github/lottery/message/MessageFormatter.java index 9158b92..3b11cb6 100644 --- a/src/main/java/io/quarkus/github/lottery/message/MessageFormatter.java +++ b/src/main/java/io/quarkus/github/lottery/message/MessageFormatter.java @@ -60,23 +60,24 @@ public String formatHistoryBodyMarkdown(DrawRef drawRef, List this.formatHistoryBodyReport(drawRef, report)) .collect(Collectors.joining("\n")) + "\n" + PAYLOAD_BEGIN + jsonObjectMapper.writeValueAsString(reports) + PAYLOAD_END; } - private String formatHistoryBodyReport(LotteryReport.Serialized report) { + private String formatHistoryBodyReport(DrawRef drawRef, LotteryReport.Serialized report) { StringBuilder builder = new StringBuilder("# ").append(report.username()).append('\n'); - builder.append(formatHistoryBodyBucket("Triage", report.triage())); + builder.append(formatHistoryBodyBucket(drawRef, "Triage", report.triage())); return builder.toString(); } - private String formatHistoryBodyBucket(String title, LotteryReport.Bucket.Serialized bucket) { + private String formatHistoryBodyBucket(DrawRef drawRef, String title, LotteryReport.Bucket.Serialized bucket) { + String repoName = drawRef.repositoryRef().repositoryName(); StringBuilder builder = new StringBuilder("## ").append(title).append('\n'); var issueNumbers = bucket.issueNumbers(); if (!issueNumbers.isEmpty()) { builder.append(issueNumbers.stream() - .map(issueId -> "#" + issueId) + .map(issueId -> repoName + "#" + issueId) .collect(MARKDOWN_BULLET_LIST_COLLECTOR)); } return builder.toString(); diff --git a/src/test/java/io/quarkus/github/lottery/MessageFormatterTest.java b/src/test/java/io/quarkus/github/lottery/MessageFormatterTest.java index 016d323..7f928e0 100644 --- a/src/test/java/io/quarkus/github/lottery/MessageFormatterTest.java +++ b/src/test/java/io/quarkus/github/lottery/MessageFormatterTest.java @@ -114,13 +114,13 @@ void formatHistoryBodyMarkdown() throws IOException { # yrodiere ## Triage - - #1 - - #3 + - quarkusio/quarkus#1 + - quarkusio/quarkus#3 # gsmet ## Triage - - #2 - - #4 + - quarkusio/quarkus#2 + - quarkusio/quarkus#4 # rick ## Triage