From 9b5a926b9545bbdee8103d93407d28954652246e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Mon, 29 Apr 2024 16:33:48 +0200 Subject: [PATCH] Fix GitHubService when two history issues exist and one is the prefix of the other --- .../lottery/github/GitHubDedicatedIssue.java | 4 + .../lottery/github/GitHubRepository.java | 13 +- .../github/lottery/GitHubServiceTest.java | 129 ++++++++++++++++++ 3 files changed, 145 insertions(+), 1 deletion(-) create mode 100644 src/main/java/io/quarkus/github/lottery/github/GitHubDedicatedIssue.java diff --git a/src/main/java/io/quarkus/github/lottery/github/GitHubDedicatedIssue.java b/src/main/java/io/quarkus/github/lottery/github/GitHubDedicatedIssue.java new file mode 100644 index 0000000..bfb8cb6 --- /dev/null +++ b/src/main/java/io/quarkus/github/lottery/github/GitHubDedicatedIssue.java @@ -0,0 +1,4 @@ +package io.quarkus.github.lottery.github; + +public class GitHubDedicatedIssue { +} 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 095ecfb..55cea41 100644 --- a/src/main/java/io/quarkus/github/lottery/github/GitHubRepository.java +++ b/src/main/java/io/quarkus/github/lottery/github/GitHubRepository.java @@ -302,11 +302,22 @@ private Optional getDedicatedIssue(String assignee, String topic) throw builder.assignee(assignee); } builder.state(GHIssueState.ALL); + // Try exact match first to avoid confusion if there are two issues and one is + // the exact topic while the other just starts with the topic. + // Example: + // topic = Lottery history for quarkusio/quarkus + // issue1.title = Lottery history for quarkusio/quarkusio.github.io + // issue2.title = Lottery history for quarkusio/quarkus for (var issue : builder.list()) { - if (issue.getTitle().startsWith(topic)) { + if (issue.getTitle().equals( topic )) { return Optional.of(issue); } } + for ( var issue : builder.list() ) { + if ( issue.getTitle().startsWith( topic ) ) { + return Optional.of( issue ); + } + } return Optional.empty(); } diff --git a/src/test/java/io/quarkus/github/lottery/GitHubServiceTest.java b/src/test/java/io/quarkus/github/lottery/GitHubServiceTest.java index b1fe697..ed5f598 100644 --- a/src/test/java/io/quarkus/github/lottery/GitHubServiceTest.java +++ b/src/test/java/io/quarkus/github/lottery/GitHubServiceTest.java @@ -815,6 +815,60 @@ void extractCommentsFromDedicatedIssue_dedicatedIssueExists_appCommentsExist() t }); } + @Test + void extractCommentsFromDedicatedIssue_dedicatedIssueExists_appCommentsExist_withConfusingOther() throws Exception { + 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, + withSettings().defaultAnswer(Answers.RETURNS_SELF)); + var queryCommentsBuilderMock = Mockito.mock(GHIssueCommentQueryBuilder.class, + withSettings().defaultAnswer(Answers.RETURNS_SELF)); + + given() + .github(mocks -> { + var repositoryMock = mocks.repository(repoRef.repositoryName()); + + when(repositoryMock.queryIssues()).thenReturn(queryIssuesBuilderMock); + var issue1Mock = mockIssueForNotification(mocks, 1, "Lottery history for quarkusio/quarkusio.github.io"); + var issue2Mock = mockIssueForNotification(mocks, 2, "Lottery history for quarkusio/quarkus"); + var issuesMocks = mockPagedIterable(issue1Mock, issue2Mock); + when(queryIssuesBuilderMock.list()).thenReturn(issuesMocks); + + var mySelfMock = mocks.ghObject(GHUser.class, 1L); + when(mySelfMock.getLogin()).thenReturn(installationRef.appLogin()); + var someoneElseMock = mocks.ghObject(GHUser.class, 2L); + when(someoneElseMock.getLogin()).thenReturn("yrodiere"); + + when(issue2Mock.queryComments()).thenReturn(queryCommentsBuilderMock); + var issue2Comment1Mock = mocks.issueComment(202); + when(issue2Comment1Mock.getUser()).thenReturn(mySelfMock); + when(issue2Comment1Mock.getBody()).thenReturn("issue2Comment1Mock#body"); + var issue2Comment2Mock = mocks.issueComment(203); + when(issue2Comment2Mock.getUser()).thenReturn(mySelfMock); + when(issue2Comment2Mock.getBody()).thenReturn("issue2Comment2Mock#body"); + var issue2Comment3Mock = mocks.issueComment(204); + when(issue2Comment3Mock.getUser()).thenReturn(someoneElseMock); + var issue2CommentMocks = mockPagedIterable(issue2Comment1Mock, issue2Comment2Mock, issue2Comment3Mock); + when(queryCommentsBuilderMock.list()).thenReturn(issue2CommentMocks); + }) + .when(() -> { + var repo = gitHubService.repository(repoRef); + + assertThat(repo.extractCommentsFromDedicatedIssue(null, + "Lottery history for quarkusio/quarkus", since)) + .containsExactly("issue2Comment1Mock#body", "issue2Comment2Mock#body"); + }) + .then().github(mocks -> { + verify(queryIssuesBuilderMock).creator(installationRef.appLogin()); + verify(queryIssuesBuilderMock).state(GHIssueState.ALL); + verify(queryCommentsBuilderMock).since(Date.from(since)); + + verifyNoMoreInteractions(queryIssuesBuilderMock, queryCommentsBuilderMock); + verifyNoMoreInteractions(mocks.ghObjects()); + }); + } + @SuppressWarnings("unchecked") @Test void commentOnDedicatedIssue_dedicatedIssueExists_open() throws Exception { @@ -965,6 +1019,81 @@ void commentOnDedicatedIssue_dedicatedIssueExists_noTopicSuffix() throws Excepti }); } + @SuppressWarnings("unchecked") + @Test + void commentOnDedicatedIssue_dedicatedIssueExists_withConfusingOther() throws Exception { + var repoRef = new GitHubRepositoryRef(installationRef, "quarkusio/quarkus-lottery-reports"); + var commentToMinimizeNodeId = "MDM6Qm90NzUwNjg0Mzg="; + + Instant now = LocalDateTime.of(2017, 11, 6, 6, 0).toInstant(ZoneOffset.UTC); + var clockMock = Clock.fixed(now, ZoneOffset.UTC); + QuarkusMock.installMockForType(clockMock, Clock.class); + + var queryIssuesBuilderMock = Mockito.mock(GHIssueQueryBuilder.ForRepository.class, + withSettings().defaultAnswer(Answers.RETURNS_SELF)); + var queryCommentsBuilderMock = Mockito.mock(GHIssueCommentQueryBuilder.class, + withSettings().defaultAnswer(Answers.RETURNS_SELF)); + + given() + .github(mocks -> { + var repositoryMock = mocks.repository(repoRef.repositoryName()); + + when(repositoryMock.queryIssues()).thenReturn(queryIssuesBuilderMock); + var issue1Mock = mockIssueForNotification(mocks, 1, "Lottery history for quarkusio/quarkusio.github.io"); + var issue2Mock = mockIssueForNotification(mocks, 2, "Lottery history for quarkusio/quarkus"); + var issuesMocks = mockPagedIterable(issue1Mock, issue2Mock); + when(queryIssuesBuilderMock.list()).thenReturn(issuesMocks); + + when(issue2Mock.getState()).thenReturn(GHIssueState.OPEN); + + var mySelfMock = mocks.ghObject(GHUser.class, 1L); + when(mySelfMock.getLogin()).thenReturn(installationRef.appLogin()); + var someoneElseMock = mocks.ghObject(GHUser.class, 2L); + when(someoneElseMock.getLogin()).thenReturn("yrodiere"); + + when(issue2Mock.queryComments()).thenReturn(queryCommentsBuilderMock); + var issue2Comment1Mock = mocks.issueComment(201); + when(issue2Comment1Mock.getUser()).thenReturn(mySelfMock); + var issue2Comment2Mock = mocks.issueComment(202); + when(issue2Comment2Mock.getUser()).thenReturn(mySelfMock); + var issue2Comment3Mock = mocks.issueComment(203); + when(issue2Comment3Mock.getUser()).thenReturn(someoneElseMock); + var issue2CommentMocks = mockPagedIterable(issue2Comment1Mock, issue2Comment2Mock, issue2Comment3Mock); + when(queryCommentsBuilderMock.list()).thenReturn(issue2CommentMocks); + + when(issue2Comment2Mock.getNodeId()).thenReturn(commentToMinimizeNodeId); + + when(messageFormatterMock.formatDedicatedIssueBodyMarkdown("Lottery history for quarkusio/quarkus", + "Some content")) + .thenReturn("Dedicated issue body"); + }) + .when(() -> { + var repo = gitHubService.repository(repoRef); + + repo.commentOnDedicatedIssue("quarkus-github-lottery[bot]", + "Lottery history for quarkusio/quarkus", "", "Some content"); + }) + .then().github(mocks -> { + verify(queryIssuesBuilderMock).creator(installationRef.appLogin()); + verify(queryIssuesBuilderMock).assignee("quarkus-github-lottery[bot]"); + verify(queryIssuesBuilderMock).state(GHIssueState.ALL); + + verify(queryCommentsBuilderMock).since(Date.from(now.minus(21, ChronoUnit.DAYS))); + var mapCaptor = ArgumentCaptor.forClass(Map.class); + verify(mocks.installationGraphQLClient(installationRef.installationId())) + .executeSync(anyString(), mapCaptor.capture()); + + verify(mocks.issue(2)).setBody("Dedicated issue body"); + verify(mocks.issue(2)).comment("Some content"); + + verifyNoMoreInteractions(messageFormatterMock, queryIssuesBuilderMock); + verifyNoMoreInteractions(mocks.ghObjects()); + + assertThat(mapCaptor.getValue()).containsValue(commentToMinimizeNodeId); + }); + } + + @SuppressWarnings("unchecked") @Test void commentOnDedicatedIssue_dedicatedIssueExists_closed() throws Exception {