diff --git a/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/ArchiveItem.java b/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/ArchiveItem.java index ebba79238..455043f6f 100644 --- a/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/ArchiveItem.java +++ b/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/ArchiveItem.java @@ -151,6 +151,28 @@ private static Optional findLastMention(String commentText, List line.startsWith(">")) + .map(line -> line.replaceAll("\\W", "")) + .collect(Collectors.joining()); + if (!compactQuote.isBlank()) { + var compactBody = body.replaceAll("\\W", ""); + return compactBody.contains(compactQuote); + } else { + return false; + } + } + + private static Optional findLastQuoted(String commentText, List eligibleParents) { + for (int i = eligibleParents.size() - 1; i != 0; --i) { + if (containsQuote(commentText, eligibleParents.get(i).body())) { + return Optional.of(eligibleParents.get(i)); + } + } + return Optional.empty(); + } + static ArchiveItem findParent(List generated, Comment comment) { ArchiveItem lastCommentOrReview = generated.get(0); var eligible = new ArrayList(); @@ -168,6 +190,10 @@ static ArchiveItem findParent(List generated, Comment comment) { if (lastMention.isPresent()) { return lastMention.get(); } + var lastQuoted = findLastQuoted(comment.body(), eligible); + if (lastQuoted.isPresent()) { + return lastQuoted.get(); + } return lastCommentOrReview; } diff --git a/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/ReviewArchive.java b/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/ReviewArchive.java index 72fa22128..5c1ebb02e 100644 --- a/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/ReviewArchive.java +++ b/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/ReviewArchive.java @@ -160,7 +160,15 @@ private List parentsToQuote(ArchiveItem item, int quoteLevel, Set parentsToQuote) { + // Parents to quote are provided with the newest item first. If the item already has quoted + // a parent, use that as the quote and return an empty string. + private String quoteSelectedParents(List parentsToQuote, ArchiveItem first) { + if (parentsToQuote.isEmpty()) { + return ""; + } + if (ArchiveItem.containsQuote(first.body(), parentsToQuote.get(0).body())) { + return ""; + } Collections.reverse(parentsToQuote); var ret = ""; for (var parent : parentsToQuote) { @@ -233,7 +241,7 @@ List generateNewEmails(List sentEmails, Duration cooldown, Reposit body.append("\n\n"); } var newQuotes = parentsToQuote(item, 2, quotedParents); - var quote = quoteSelectedParents(newQuotes); + var quote = quoteSelectedParents(newQuotes, item); if (!quote.isBlank()) { body.append(quote); body.append("\n\n"); diff --git a/bots/mlbridge/src/test/java/org/openjdk/skara/bots/mlbridge/MailingListBridgeBotTests.java b/bots/mlbridge/src/test/java/org/openjdk/skara/bots/mlbridge/MailingListBridgeBotTests.java index 5325beff8..9c034c8e5 100644 --- a/bots/mlbridge/src/test/java/org/openjdk/skara/bots/mlbridge/MailingListBridgeBotTests.java +++ b/bots/mlbridge/src/test/java/org/openjdk/skara/bots/mlbridge/MailingListBridgeBotTests.java @@ -779,6 +779,71 @@ void reviewCommentWithMention(TestInfo testInfo) throws IOException { } } + @Test + void commentWithQuote(TestInfo testInfo) throws IOException { + try (var credentials = new HostCredentials(testInfo); + var tempFolder = new TemporaryDirectory(); + var archiveFolder = new TemporaryDirectory(); + var listServer = new TestMailmanServer(); + var webrevServer = new TestWebrevServer()) { + var author = credentials.getHostedRepository(); + var reviewer = credentials.getHostedRepository(); + var archive = credentials.getHostedRepository(); + var listAddress = EmailAddress.parse(listServer.createList("test")); + var censusBuilder = credentials.getCensusBuilder() + .addReviewer(reviewer.forge().currentUser().id()) + .addAuthor(author.forge().currentUser().id()); + var from = EmailAddress.from("test", "test@test.mail"); + var mlBot = MailingListBridgeBot.newBuilder() + .from(from) + .repo(author) + .archive(archive) + .censusRepo(censusBuilder.build()) + .list(listAddress) + .listArchive(listServer.getArchive()) + .smtpServer(listServer.getSMTP()) + .webrevStorageRepository(archive) + .webrevStorageRef("webrev") + .webrevStorageBase(Path.of("test")) + .webrevStorageBaseUri(webrevServer.uri()) + .issueTracker(URIBuilder.base("http://issues.test/browse/").build()) + .build(); + + // Populate the projects repository + var reviewFile = Path.of("reviewfile.txt"); + var localRepo = CheckableRepository.init(tempFolder.path(), author.repositoryType(), reviewFile); + var masterHash = localRepo.resolve("master").orElseThrow(); + localRepo.push(masterHash, author.url(), "master", true); + localRepo.push(masterHash, archive.url(), "webrev", true); + + // Make a change with a corresponding PR + var editHash = CheckableRepository.appendAndCommit(localRepo); + localRepo.push(editHash, author.url(), "edit", true); + var pr = credentials.createPullRequest(archive, "master", "edit", "This is a pull request"); + pr.setBody("This is now ready"); + TestBotRunner.runPeriodicItems(mlBot); + listServer.processIncoming(); + + // Make two comments from different authors + var reviewPr = reviewer.pullRequest(pr.id()); + reviewPr.addComment("First comment\nsecond line"); + pr.addComment("Second comment\nfourth line"); + + TestBotRunner.runPeriodicItems(mlBot); + listServer.processIncoming(); + + pr.addComment(">First comm\n\nreply to first"); + TestBotRunner.runPeriodicItems(mlBot); + listServer.processIncoming(); + + // The first comment should be quoted more often than the second + Repository.materialize(archiveFolder.path(), archive.url(), "master"); + assertEquals(2, archiveContainsCount(archiveFolder.path(), "First comment")); + assertEquals(3, archiveContainsCount(archiveFolder.path(), "First comm")); + assertEquals(1, archiveContainsCount(archiveFolder.path(), "Second comment")); + } + } + @Test void reviewContext(TestInfo testInfo) throws IOException { try (var credentials = new HostCredentials(testInfo);