diff --git a/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/MailingListBridgeBot.java b/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/MailingListBridgeBot.java index 6b8aee416..8fcf9c5da 100644 --- a/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/MailingListBridgeBot.java +++ b/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/MailingListBridgeBot.java @@ -66,8 +66,9 @@ public class MailingListBridgeBot implements Bot { MailingListBridgeBot(EmailAddress from, HostedRepository repo, HostedRepository archive, String archiveRef, HostedRepository censusRepo, String censusRef, List lists, Set ignoredUsers, Set ignoredComments, URI listArchive, String smtpServer, - HostedRepository webrevStorageRepository, String webrevStorageRef, - Path webrevStorageBase, URI webrevStorageBaseUri, Set readyLabels, + HostedRepository webrevStorageHTMLRepository, HostedRepository webrevStorageJSONRepository, + String webrevStorageRef, Path webrevStorageBase, URI webrevStorageBaseUri, + boolean webrevGenerateHTML, boolean webrevGenerateJSON, Set readyLabels, Map readyComments, URI issueTracker, Map headers, Duration sendInterval, Duration cooldown, boolean repoInSubject, Pattern branchInSubject, Path seedStorage) { @@ -92,8 +93,9 @@ public class MailingListBridgeBot implements Bot { this.branchInSubject = branchInSubject; this.seedStorage = seedStorage; - webrevStorage = new WebrevStorage(webrevStorageRepository, webrevStorageRef, webrevStorageBase, - webrevStorageBaseUri, from); + webrevStorage = new WebrevStorage(webrevStorageHTMLRepository, webrevStorageJSONRepository, webrevStorageRef, + webrevStorageBase, webrevStorageBaseUri, from, + webrevGenerateHTML, webrevGenerateJSON); updateCache = new PullRequestUpdateCache(); cooldownQuarantine = new CooldownQuarantine(); } diff --git a/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/MailingListBridgeBotBuilder.java b/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/MailingListBridgeBotBuilder.java index b2e92d43a..e8fe81b30 100644 --- a/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/MailingListBridgeBotBuilder.java +++ b/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/MailingListBridgeBotBuilder.java @@ -43,10 +43,13 @@ public class MailingListBridgeBotBuilder { private Set ignoredComments = Set.of(); private URI listArchive; private String smtpServer; - private HostedRepository webrevStorageRepository; + private HostedRepository webrevStorageHTMLRepository; + private HostedRepository webrevStorageJSONRepository; private String webrevStorageRef; private Path webrevStorageBase; private URI webrevStorageBaseUri; + private boolean webrevGenerateHTML = true; + private boolean webrevGenerateJSON = false; private Set readyLabels = Set.of(); private Map readyComments = Map.of(); private URI issueTracker; @@ -115,8 +118,13 @@ public MailingListBridgeBotBuilder smtpServer(String smtpServer) { return this; } - public MailingListBridgeBotBuilder webrevStorageRepository(HostedRepository webrevStorageRepository) { - this.webrevStorageRepository = webrevStorageRepository; + public MailingListBridgeBotBuilder webrevStorageHTMLRepository(HostedRepository webrevStorageHTMLRepository) { + this.webrevStorageHTMLRepository = webrevStorageHTMLRepository; + return this; + } + + public MailingListBridgeBotBuilder webrevStorageJSONRepository(HostedRepository webrevStorageJSONRepository) { + this.webrevStorageJSONRepository = webrevStorageJSONRepository; return this; } @@ -135,6 +143,16 @@ public MailingListBridgeBotBuilder webrevStorageBaseUri(URI webrevStorageBaseUri return this; } + public MailingListBridgeBotBuilder webrevGenerateHTML(boolean webrevGenerateHTML) { + this.webrevGenerateHTML = webrevGenerateHTML; + return this; + } + + public MailingListBridgeBotBuilder webrevGenerateJSON(boolean webrevGenerateJSON) { + this.webrevGenerateJSON = webrevGenerateJSON; + return this; + } + public MailingListBridgeBotBuilder readyLabels(Set readyLabels) { this.readyLabels = readyLabels; return this; @@ -183,8 +201,9 @@ public MailingListBridgeBotBuilder seedStorage(Path seedStorage) { public MailingListBridgeBot build() { return new MailingListBridgeBot(from, repo, archive, archiveRef, censusRepo, censusRef, lists, ignoredUsers, ignoredComments, listArchive, smtpServer, - webrevStorageRepository, webrevStorageRef, webrevStorageBase, webrevStorageBaseUri, - readyLabels, readyComments, issueTracker, headers, sendInterval, cooldown, - repoInSubject, branchInSubject, seedStorage); + webrevStorageHTMLRepository, webrevStorageJSONRepository, webrevStorageRef, + webrevStorageBase, webrevStorageBaseUri, webrevGenerateHTML, webrevGenerateJSON, + readyLabels, readyComments, issueTracker, headers, sendInterval, + cooldown, repoInSubject, branchInSubject, seedStorage); } -} \ No newline at end of file +} diff --git a/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/MailingListBridgeBotFactory.java b/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/MailingListBridgeBotFactory.java index afa710009..1e301e03a 100644 --- a/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/MailingListBridgeBotFactory.java +++ b/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/MailingListBridgeBotFactory.java @@ -79,7 +79,8 @@ public List create(BotConfiguration configuration) { var listSmtp = specific.get("server").get("smtp").asString(); var interval = specific.get("server").contains("interval") ? Duration.parse(specific.get("server").get("interval").asString()) : Duration.ofSeconds(1); - var webrevRepo = configuration.repository(specific.get("webrevs").get("repository").asString()); + var webrevHTMLRepo = configuration.repository(specific.get("webrevs").get("repository").get("html").asString()); + var webrevJSONRepo = configuration.repository(specific.get("webrevs").get("repository").get("json").asString()); var webrevRef = configuration.repositoryRef(specific.get("webrevs").get("repository").asString()); var webrevWeb = specific.get("webrevs").get("web").asString(); @@ -110,6 +111,17 @@ public List create(BotConfiguration configuration) { Map.of(); var lists = parseLists(repoConfig.get("lists")); var folder = repoConfig.contains("folder") ? repoConfig.get("folder").asString() : configuration.repositoryName(repo); + + var webrevGenerateHTML = true; + if (repoConfig.contains("webrev") && + repoConfig.get("webrev").contains("html") && + repoConfig.get("webrev").get("html").asBoolean() == false) { + webrevGenerateHTML = false; + } + var webrevGenerateJSON = repoConfig.contains("webrev") && + repoConfig.get("webrev").contains("json") && + repoConfig.get("webrev").get("json").asBoolean(); + var botBuilder = MailingListBridgeBot.newBuilder().from(from) .repo(configuration.repository(repo)) .archive(archiveRepo) @@ -121,10 +133,13 @@ public List create(BotConfiguration configuration) { .ignoredComments(ignoredComments) .listArchive(listArchive) .smtpServer(listSmtp) - .webrevStorageRepository(webrevRepo) + .webrevStorageHTMLRepository(webrevHTMLRepo) + .webrevStorageJSONRepository(webrevJSONRepo) .webrevStorageRef(webrevRef) .webrevStorageBase(Path.of(folder)) .webrevStorageBaseUri(URIBuilder.base(webrevWeb).build()) + .webrevGenerateHTML(webrevGenerateHTML) + .webrevGenerateJSON(webrevGenerateJSON) .readyLabels(readyLabels) .readyComments(readyComments) .issueTracker(issueTracker) diff --git a/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/WebrevStorage.java b/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/WebrevStorage.java index 023d3c055..80b2a0f18 100644 --- a/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/WebrevStorage.java +++ b/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/WebrevStorage.java @@ -43,25 +43,53 @@ import java.util.stream.Collectors; class WebrevStorage { - private final HostedRepository storage; + private final HostedRepository htmlStorage; + private final HostedRepository jsonStorage; private final String storageRef; private final Path baseFolder; private final URI baseUri; private final EmailAddress author; + private final boolean generateHTML; + private final boolean generateJSON; private final Logger log = Logger.getLogger("org.openjdk.skara.bots.mlbridge"); private static final HttpClient client = HttpClient.newBuilder() .connectTimeout(Duration.ofSeconds(10)) .build(); - WebrevStorage(HostedRepository storage, String ref, Path baseFolder, URI baseUri, EmailAddress author) { + WebrevStorage(HostedRepository htmlStorage, + String ref, + Path baseFolder, + URI baseUri, + EmailAddress author) { this.baseFolder = baseFolder; this.baseUri = baseUri; - this.storage = storage; + this.htmlStorage = htmlStorage; + this.jsonStorage = null; storageRef = ref; this.author = author; + this.generateHTML = true; + this.generateJSON = false; } - private void generate(PullRequest pr, Repository localRepository, Path folder, Diff diff, Hash base, Hash head) throws IOException { + WebrevStorage(HostedRepository htmlStorage, + HostedRepository jsonStorage, + String ref, + Path baseFolder, + URI baseUri, + EmailAddress author, + boolean generateHTML, + boolean generateJSON) { + this.baseFolder = baseFolder; + this.baseUri = baseUri; + this.htmlStorage = htmlStorage; + this.jsonStorage = jsonStorage; + storageRef = ref; + this.author = author; + this.generateHTML = generateHTML; + this.generateJSON = generateJSON; + } + + private void generateHTML(PullRequest pr, ReadOnlyRepository localRepository, Path folder, Diff diff, Hash base, Hash head) throws IOException { Files.createDirectories(folder); var fullName = pr.author().fullName(); var builder = Webrev.repository(localRepository) @@ -99,6 +127,24 @@ private void generate(PullRequest pr, Repository localRepository, Path folder, D } } + private void generateJSON(PullRequest pr, ReadOnlyRepository localRepository, Path folder, Diff diff, Hash base, Hash head) throws IOException { + Files.createDirectories(folder); + var builder = Webrev.repository(localRepository) + .output(folder) + .upstream(pr.repository().webUrl(), pr.repository().name()); + var sourceRepository = pr.sourceRepository(); + if (sourceRepository.isEmpty()) { + throw new IllegalArgumentException("Cannot generate JSON for PR without source repository. PR: " + pr.id() + ", repo: " + pr.repository().webUrl()); + } + builder.fork(sourceRepository.get().webUrl(), sourceRepository.get().name()); + + if (diff != null) { + builder.generateJSON(diff); + } else { + builder.generateJSON(base, head); + } + } + private String generatePlaceholder(PullRequest pr, Hash base, Hash head) { return "This file was too large to be included in the published webrev, and has been replaced with " + "this placeholder message. It is possible to generate the original content locally by " + @@ -139,7 +185,7 @@ private boolean shouldBeReplaced(Path file) { } } - private void push(Repository localStorage, Path webrevFolder, String identifier, String placeholder) throws IOException { + private void push(Repository localStorage, URI remote, Path webrevFolder, String identifier, String placeholder) throws IOException { var batchIndex = new AtomicInteger(); // Replace large files (except the index) with placeholders @@ -169,7 +215,7 @@ private void push(Repository localStorage, Path webrevFolder, String identifier, // where some of the files have already been committed. Ignore it and continue. continue; } - localStorage.push(hash, storage.url(), storageRef); + localStorage.push(hash, remote, storageRef); } } } @@ -220,21 +266,37 @@ private void awaitPublication(URI uri, Duration timeout) throws IOException { private URI createAndArchive(PullRequest pr, Repository localRepository, Path scratchPath, Diff diff, Hash base, Hash head, String identifier) { try { - var localStorage = Repository.materialize(scratchPath, storage.url(), - "+" + storageRef + ":mlbridge_webrevs"); var relativeFolder = baseFolder.resolve(String.format("%s/%s", pr.id(), identifier)); var outputFolder = scratchPath.resolve(relativeFolder); - // If a previous operation was interrupted there may be content here already - overwrite if so - if (Files.exists(outputFolder)) { - clearDirectory(outputFolder); - } - generate(pr, localRepository, outputFolder, diff, base, head); var placeholder = generatePlaceholder(pr, base, head); - if (!localStorage.isClean()) { - push(localStorage, outputFolder, baseFolder.resolve(pr.id()).toString(), placeholder); + URI uri = null; + + if (generateJSON) { + var jsonLocalStorage = Repository.materialize(scratchPath, jsonStorage.url(), + "+" + storageRef + ":mlbridge_webrevs"); + if (Files.exists(outputFolder)) { + clearDirectory(outputFolder); + } + generateJSON(pr, localRepository, outputFolder, diff, base, head); + if (!jsonLocalStorage.isClean()) { + push(jsonLocalStorage, jsonStorage.url(), outputFolder, baseFolder.resolve(pr.id()).toString(), placeholder); + } + var repoName = Path.of(pr.repository().name()).getFileName(); + uri = URI.create(baseUri.toString() + "?repo=" + repoName + "&pr=" + pr.id() + "&range=" + identifier); + } + if (generateHTML) { + var htmlLocalStorage = Repository.materialize(scratchPath, htmlStorage.url(), + "+" + storageRef + ":mlbridge_webrevs"); + if (Files.exists(outputFolder)) { + clearDirectory(outputFolder); + } + generateHTML(pr, localRepository, outputFolder, diff, base, head); + if (!htmlLocalStorage.isClean()) { + push(htmlLocalStorage, htmlStorage.url(), outputFolder, baseFolder.resolve(pr.id()).toString(), placeholder); + } + uri = URIBuilder.base(baseUri).appendPath(relativeFolder.toString().replace('\\', '/')).build(); + awaitPublication(uri, Duration.ofMinutes(30)); } - var uri = URIBuilder.base(baseUri).appendPath(relativeFolder.toString().replace('\\', '/')).build(); - awaitPublication(uri, Duration.ofMinutes(30)); return uri; } catch (IOException e) { throw new UncheckedIOException(e); diff --git a/bots/mlbridge/src/test/java/org/openjdk/skara/bots/mlbridge/MailingListArchiveReaderBotTests.java b/bots/mlbridge/src/test/java/org/openjdk/skara/bots/mlbridge/MailingListArchiveReaderBotTests.java index 528ab1639..91eaf501c 100644 --- a/bots/mlbridge/src/test/java/org/openjdk/skara/bots/mlbridge/MailingListArchiveReaderBotTests.java +++ b/bots/mlbridge/src/test/java/org/openjdk/skara/bots/mlbridge/MailingListArchiveReaderBotTests.java @@ -76,7 +76,7 @@ void simpleArchive(TestInfo testInfo) throws IOException { .ignoredUsers(Set.of(ignored.forge().currentUser().userName())) .listArchive(listServer.getArchive()) .smtpServer(listServer.getSMTP()) - .webrevStorageRepository(archive) + .webrevStorageHTMLRepository(archive) .webrevStorageRef("webrev") .webrevStorageBase(Path.of("test")) .webrevStorageBaseUri(webrevServer.uri()) @@ -151,7 +151,7 @@ void rememberBridged(TestInfo testInfo) throws IOException { .ignoredUsers(Set.of(ignored.forge().currentUser().userName())) .listArchive(listServer.getArchive()) .smtpServer(listServer.getSMTP()) - .webrevStorageRepository(archive) + .webrevStorageHTMLRepository(archive) .webrevStorageRef("webrev") .webrevStorageBase(Path.of("test")) .webrevStorageBaseUri(webrevServer.uri()) @@ -228,7 +228,7 @@ void largeEmail(TestInfo testInfo) throws IOException { .ignoredUsers(Set.of(ignored.forge().currentUser().userName())) .listArchive(listServer.getArchive()) .smtpServer(listServer.getSMTP()) - .webrevStorageRepository(archive) + .webrevStorageHTMLRepository(archive) .webrevStorageRef("webrev") .webrevStorageBase(Path.of("test")) .webrevStorageBaseUri(webrevServer.uri()) 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 7b01ce617..fc062a0b6 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 @@ -29,6 +29,7 @@ import org.openjdk.skara.mailinglist.MailingListServerFactory; import org.openjdk.skara.test.*; import org.openjdk.skara.vcs.Repository; +import org.openjdk.skara.json.JSON; import org.junit.jupiter.api.*; @@ -127,7 +128,7 @@ void simpleArchive(TestInfo testInfo) throws IOException { .ignoredComments(Set.of()) .listArchive(listServer.getArchive()) .smtpServer(listServer.getSMTP()) - .webrevStorageRepository(archive) + .webrevStorageHTMLRepository(archive) .webrevStorageRef("webrev") .webrevStorageBase(Path.of("test")) .webrevStorageBaseUri(webrevServer.uri()) @@ -294,7 +295,7 @@ void archiveIntegrated(TestInfo testInfo) throws IOException { .ignoredUsers(Set.of(ignored.forge().currentUser().userName())) .listArchive(listServer.getArchive()) .smtpServer(listServer.getSMTP()) - .webrevStorageRepository(archive) + .webrevStorageHTMLRepository(archive) .webrevStorageRef("webrev") .webrevStorageBase(Path.of("test")) .webrevStorageBaseUri(webrevServer.uri()) @@ -368,7 +369,7 @@ void archiveLegacyIntegrated(TestInfo testInfo) throws IOException { .ignoredUsers(Set.of(ignored.forge().currentUser().userName())) .listArchive(listServer.getArchive()) .smtpServer(listServer.getSMTP()) - .webrevStorageRepository(archive) + .webrevStorageHTMLRepository(archive) .webrevStorageRef("webrev") .webrevStorageBase(Path.of("test")) .webrevStorageBaseUri(webrevServer.uri()) @@ -442,7 +443,7 @@ void archiveDirectToIntegrated(TestInfo testInfo) throws IOException { .ignoredUsers(Set.of(ignored.forge().currentUser().userName())) .listArchive(listServer.getArchive()) .smtpServer(listServer.getSMTP()) - .webrevStorageRepository(archive) + .webrevStorageHTMLRepository(archive) .webrevStorageRef("webrev") .webrevStorageBase(Path.of("test")) .webrevStorageBaseUri(webrevServer.uri()) @@ -521,7 +522,7 @@ void archiveIntegratedRetainPrefix(TestInfo testInfo) throws IOException { .ignoredUsers(Set.of(ignored.forge().currentUser().userName())) .listArchive(listServer.getArchive()) .smtpServer(listServer.getSMTP()) - .webrevStorageRepository(archive) + .webrevStorageHTMLRepository(archive) .webrevStorageRef("webrev") .webrevStorageBase(Path.of("test")) .webrevStorageBaseUri(webrevServer.uri()) @@ -598,7 +599,7 @@ void archiveClosed(TestInfo testInfo) throws IOException { .ignoredUsers(Set.of(ignored.forge().currentUser().userName())) .listArchive(listServer.getArchive()) .smtpServer(listServer.getSMTP()) - .webrevStorageRepository(archive) + .webrevStorageHTMLRepository(archive) .webrevStorageRef("webrev") .webrevStorageBase(Path.of("test")) .webrevStorageBaseUri(webrevServer.uri()) @@ -681,7 +682,7 @@ void archiveFailedAutoMerge(TestInfo testInfo) throws IOException { .ignoredUsers(Set.of(ignored.forge().currentUser().userName())) .listArchive(listServer.getArchive()) .smtpServer(listServer.getSMTP()) - .webrevStorageRepository(archive) + .webrevStorageHTMLRepository(archive) .webrevStorageRef("webrev") .webrevStorageBase(Path.of("test")) .webrevStorageBaseUri(webrevServer.uri()) @@ -735,7 +736,7 @@ void reviewComment(TestInfo testInfo) throws IOException { .ignoredUsers(Set.of(ignored.forge().currentUser().userName())) .listArchive(listServer.getArchive()) .smtpServer(listServer.getSMTP()) - .webrevStorageRepository(archive) + .webrevStorageHTMLRepository(archive) .webrevStorageRef("webrev") .webrevStorageBase(Path.of("test")) .webrevStorageBaseUri(webrevServer.uri()) @@ -829,7 +830,7 @@ void combineComments(TestInfo testInfo) throws IOException { .lists(List.of(new MailingListConfiguration(listAddress, Set.of()))) .listArchive(listServer.getArchive()) .smtpServer(listServer.getSMTP()) - .webrevStorageRepository(archive) + .webrevStorageHTMLRepository(archive) .webrevStorageRef("webrev") .webrevStorageBase(Path.of("test")) .webrevStorageBaseUri(webrevServer.uri()) @@ -924,7 +925,7 @@ void commentThreading(TestInfo testInfo) throws IOException { .lists(List.of(new MailingListConfiguration(listAddress, Set.of()))) .listArchive(listServer.getArchive()) .smtpServer(listServer.getSMTP()) - .webrevStorageRepository(archive) + .webrevStorageHTMLRepository(archive) .webrevStorageRef("webrev") .webrevStorageBase(Path.of("test")) .webrevStorageBaseUri(webrevServer.uri()) @@ -1053,7 +1054,7 @@ void commentThreadingSeparated(TestInfo testInfo) throws IOException { .lists(List.of(new MailingListConfiguration(listAddress, Set.of()))) .listArchive(listServer.getArchive()) .smtpServer(listServer.getSMTP()) - .webrevStorageRepository(archive) + .webrevStorageHTMLRepository(archive) .webrevStorageRef("webrev") .webrevStorageBase(Path.of("test")) .webrevStorageBaseUri(webrevServer.uri()) @@ -1116,7 +1117,7 @@ void commentWithMention(TestInfo testInfo) throws IOException { .lists(List.of(new MailingListConfiguration(listAddress, Set.of()))) .listArchive(listServer.getArchive()) .smtpServer(listServer.getSMTP()) - .webrevStorageRepository(archive) + .webrevStorageHTMLRepository(archive) .webrevStorageRef("webrev") .webrevStorageBase(Path.of("test")) .webrevStorageBaseUri(webrevServer.uri()) @@ -1180,7 +1181,7 @@ void reviewCommentWithMention(TestInfo testInfo) throws IOException { .lists(List.of(new MailingListConfiguration(listAddress, Set.of()))) .listArchive(listServer.getArchive()) .smtpServer(listServer.getSMTP()) - .webrevStorageRepository(archive) + .webrevStorageHTMLRepository(archive) .webrevStorageRef("webrev") .webrevStorageBase(Path.of("test")) .webrevStorageBaseUri(webrevServer.uri()) @@ -1245,7 +1246,7 @@ void commentWithQuote(TestInfo testInfo) throws IOException { .lists(List.of(new MailingListConfiguration(listAddress, Set.of()))) .listArchive(listServer.getArchive()) .smtpServer(listServer.getSMTP()) - .webrevStorageRepository(archive) + .webrevStorageHTMLRepository(archive) .webrevStorageRef("webrev") .webrevStorageBase(Path.of("test")) .webrevStorageBaseUri(webrevServer.uri()) @@ -1308,7 +1309,7 @@ void reviewContext(TestInfo testInfo) throws IOException { .lists(List.of(new MailingListConfiguration(listAddress, Set.of()))) .listArchive(listServer.getArchive()) .smtpServer(listServer.getSMTP()) - .webrevStorageRepository(archive) + .webrevStorageHTMLRepository(archive) .webrevStorageRef("webrev") .webrevStorageBase(Path.of("test")) .webrevStorageBaseUri(webrevServer.uri()) @@ -1365,7 +1366,7 @@ void multipleReviewContexts(TestInfo testInfo) throws IOException { .lists(List.of(new MailingListConfiguration(listAddress, Set.of()))) .listArchive(listServer.getArchive()) .smtpServer(listServer.getSMTP()) - .webrevStorageRepository(archive) + .webrevStorageHTMLRepository(archive) .webrevStorageRef("webrev") .webrevStorageBase(Path.of("test")) .webrevStorageBaseUri(webrevServer.uri()) @@ -1441,7 +1442,7 @@ void filterComments(TestInfo testInfo) throws IOException { .lists(List.of(new MailingListConfiguration(listAddress, Set.of()))) .listArchive(listServer.getArchive()) .smtpServer(listServer.getSMTP()) - .webrevStorageRepository(archive) + .webrevStorageHTMLRepository(archive) .webrevStorageRef("webrev") .webrevStorageBase(Path.of("test")) .webrevStorageBaseUri(webrevServer.uri()) @@ -1507,7 +1508,7 @@ void incrementalChanges(TestInfo testInfo) throws IOException { .lists(List.of(new MailingListConfiguration(listAddress, Set.of()))) .listArchive(listServer.getArchive()) .smtpServer(listServer.getSMTP()) - .webrevStorageRepository(archive) + .webrevStorageHTMLRepository(archive) .webrevStorageRef("webrev") .webrevStorageBase(Path.of("test")) .webrevStorageBaseUri(webrevServer.uri()) @@ -1637,7 +1638,7 @@ void rebased(TestInfo testInfo) throws IOException { .lists(List.of(new MailingListConfiguration(listAddress, Set.of()))) .listArchive(listServer.getArchive()) .smtpServer(listServer.getSMTP()) - .webrevStorageRepository(archive) + .webrevStorageHTMLRepository(archive) .webrevStorageRef("webrev") .webrevStorageBase(Path.of("test")) .webrevStorageBaseUri(webrevServer.uri()) @@ -1734,7 +1735,7 @@ void incrementalAfterRebase(TestInfo testInfo) throws IOException { .lists(List.of(new MailingListConfiguration(listAddress, Set.of()))) .listArchive(listServer.getArchive()) .smtpServer(listServer.getSMTP()) - .webrevStorageRepository(archive) + .webrevStorageHTMLRepository(archive) .webrevStorageRef("webrev") .webrevStorageBase(Path.of("test")) .webrevStorageBaseUri(webrevServer.uri()) @@ -1824,7 +1825,7 @@ void mergeWebrev(TestInfo testInfo) throws IOException { .lists(List.of(new MailingListConfiguration(listAddress, Set.of()))) .listArchive(listServer.getArchive()) .smtpServer(listServer.getSMTP()) - .webrevStorageRepository(archive) + .webrevStorageHTMLRepository(archive) .webrevStorageRef("webrev") .webrevStorageBase(Path.of("test")) .webrevStorageBaseUri(webrevServer.uri()) @@ -1910,7 +1911,7 @@ void mergeWebrevConflict(TestInfo testInfo) throws IOException { .lists(List.of(new MailingListConfiguration(listAddress, Set.of()))) .listArchive(listServer.getArchive()) .smtpServer(listServer.getSMTP()) - .webrevStorageRepository(archive) + .webrevStorageHTMLRepository(archive) .webrevStorageRef("webrev") .webrevStorageBase(Path.of("test")) .webrevStorageBaseUri(webrevServer.uri()) @@ -1982,7 +1983,7 @@ void mergeWebrevNoConflict(TestInfo testInfo) throws IOException { .lists(List.of(new MailingListConfiguration(listAddress, Set.of()))) .listArchive(listServer.getArchive()) .smtpServer(listServer.getSMTP()) - .webrevStorageRepository(archive) + .webrevStorageHTMLRepository(archive) .webrevStorageRef("webrev") .webrevStorageBase(Path.of("test")) .webrevStorageBaseUri(webrevServer.uri()) @@ -2053,7 +2054,7 @@ void skipAddingExistingWebrev(TestInfo testInfo) throws IOException { .ignoredUsers(Set.of(ignored.forge().currentUser().userName())) .listArchive(listServer.getArchive()) .smtpServer(listServer.getSMTP()) - .webrevStorageRepository(archive) + .webrevStorageHTMLRepository(archive) .webrevStorageRef("webrev") .webrevStorageBase(Path.of("test")) .webrevStorageBaseUri(webrevServer.uri()) @@ -2133,7 +2134,7 @@ void notifyReviewVerdicts(TestInfo testInfo) throws IOException { .lists(List.of(new MailingListConfiguration(listAddress, Set.of()))) .listArchive(listServer.getArchive()) .smtpServer(listServer.getSMTP()) - .webrevStorageRepository(archive) + .webrevStorageHTMLRepository(archive) .webrevStorageRef("webrev") .webrevStorageBase(Path.of("test")) .webrevStorageBaseUri(webrevServer.uri()) @@ -2224,7 +2225,7 @@ void ignoreComments(TestInfo testInfo) throws IOException { .ignoredComments(Set.of(Pattern.compile("ignore this comment", Pattern.MULTILINE | Pattern.DOTALL))) .listArchive(listServer.getArchive()) .smtpServer(listServer.getSMTP()) - .webrevStorageRepository(archive) + .webrevStorageHTMLRepository(archive) .webrevStorageRef("webrev") .webrevStorageBase(Path.of("test")) .webrevStorageBaseUri(webrevServer.uri()) @@ -2289,7 +2290,7 @@ void replyToEmptyReview(TestInfo testInfo) throws IOException { .lists(List.of(new MailingListConfiguration(listAddress, Set.of()))) .listArchive(listServer.getArchive()) .smtpServer(listServer.getSMTP()) - .webrevStorageRepository(archive) + .webrevStorageHTMLRepository(archive) .webrevStorageRef("webrev") .webrevStorageBase(Path.of("test")) .webrevStorageBaseUri(webrevServer.uri()) @@ -2350,7 +2351,7 @@ void cooldown(TestInfo testInfo) throws IOException { .lists(List.of(new MailingListConfiguration(listAddress, Set.of()))) .listArchive(listServer.getArchive()) .smtpServer(listServer.getSMTP()) - .webrevStorageRepository(archive) + .webrevStorageHTMLRepository(archive) .webrevStorageRef("webrev") .webrevStorageBase(Path.of("test")) .webrevStorageBaseUri(webrevServer.uri()) @@ -2415,7 +2416,7 @@ void cooldownNewRevision(TestInfo testInfo) throws IOException { .lists(List.of(new MailingListConfiguration(listAddress, Set.of()))) .listArchive(listServer.getArchive()) .smtpServer(listServer.getSMTP()) - .webrevStorageRepository(archive) + .webrevStorageHTMLRepository(archive) .webrevStorageRef("webrev") .webrevStorageBase(Path.of("test")) .webrevStorageBaseUri(webrevServer.uri()) @@ -2482,7 +2483,7 @@ void retryAfterCooldown(TestInfo testInfo) throws IOException, InterruptedExcept .lists(List.of(new MailingListConfiguration(listAddress, Set.of()))) .listArchive(listServer.getArchive()) .smtpServer(listServer.getSMTP()) - .webrevStorageRepository(archive) + .webrevStorageHTMLRepository(archive) .webrevStorageRef("webrev") .webrevStorageBase(Path.of("test")) .webrevStorageBaseUri(webrevServer.uri()) @@ -2571,7 +2572,7 @@ void branchPrefix(TestInfo testInfo) throws IOException { .lists(List.of(new MailingListConfiguration(listAddress, Set.of()))) .listArchive(listServer.getArchive()) .smtpServer(listServer.getSMTP()) - .webrevStorageRepository(archive) + .webrevStorageHTMLRepository(archive) .webrevStorageRef("webrev") .webrevStorageBase(Path.of("test")) .webrevStorageBaseUri(webrevServer.uri()) @@ -2628,7 +2629,7 @@ void repoPrefix(TestInfo testInfo) throws IOException { .lists(List.of(new MailingListConfiguration(listAddress, Set.of()))) .listArchive(listServer.getArchive()) .smtpServer(listServer.getSMTP()) - .webrevStorageRepository(archive) + .webrevStorageHTMLRepository(archive) .webrevStorageRef("webrev") .webrevStorageBase(Path.of("test")) .webrevStorageBaseUri(webrevServer.uri()) @@ -2685,7 +2686,7 @@ void repoAndBranchPrefix(TestInfo testInfo) throws IOException { .lists(List.of(new MailingListConfiguration(listAddress, Set.of()))) .listArchive(listServer.getArchive()) .smtpServer(listServer.getSMTP()) - .webrevStorageRepository(archive) + .webrevStorageHTMLRepository(archive) .webrevStorageRef("webrev") .webrevStorageBase(Path.of("test")) .webrevStorageBaseUri(webrevServer.uri()) @@ -2746,7 +2747,7 @@ void retryNewRevisionAfterCooldown(TestInfo testInfo) throws IOException, Interr .lists(List.of(new MailingListConfiguration(listAddress, Set.of()))) .listArchive(listServer.getArchive()) .smtpServer(listServer.getSMTP()) - .webrevStorageRepository(archive) + .webrevStorageHTMLRepository(archive) .webrevStorageRef("webrev") .webrevStorageBase(Path.of("test")) .webrevStorageBaseUri(webrevServer.uri()) @@ -2838,7 +2839,7 @@ void multipleRecipients(TestInfo testInfo) throws IOException { new MailingListConfiguration(listAddress2, Set.of("list2")))) .listArchive(listServer.getArchive()) .smtpServer(listServer.getSMTP()) - .webrevStorageRepository(archive) + .webrevStorageHTMLRepository(archive) .webrevStorageRef("webrev") .webrevStorageBase(Path.of("test")) .webrevStorageBaseUri(webrevServer.uri()) @@ -2892,4 +2893,205 @@ void multipleRecipients(TestInfo testInfo) throws IOException { assertEquals(List.of(listAddress1, listAddress2), reply.recipients()); } } + + @Test + void jsonArchive(TestInfo testInfo) throws IOException { + try (var credentials = new HostCredentials(testInfo); + var tempFolder = new TemporaryDirectory(); + var archiveFolder = new TemporaryDirectory(); + var webrevFolder = new TemporaryDirectory(false); + var listServer = new TestMailmanServer(); + var webrevServer = new TestWebrevServer()) { + var author = credentials.getHostedRepository(); + var archive = credentials.getHostedRepository(); + var ignored = credentials.getHostedRepository(); + var listAddress = EmailAddress.parse(listServer.createList("test")); + var censusBuilder = credentials.getCensusBuilder() + .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()) + .lists(List.of(new MailingListConfiguration(listAddress, Set.of()))) + .ignoredUsers(Set.of(ignored.forge().currentUser().userName())) + .ignoredComments(Set.of()) + .listArchive(listServer.getArchive()) + .smtpServer(listServer.getSMTP()) + .webrevStorageJSONRepository(archive) + .webrevStorageRef("webrev") + .webrevStorageBase(Path.of("test")) + .webrevStorageBaseUri(webrevServer.uri()) + .webrevGenerateHTML(false) + .webrevGenerateJSON(true) + .readyLabels(Set.of("rfr")) + .readyComments(Map.of(ignored.forge().currentUser().userName(), Pattern.compile("ready"))) + .issueTracker(URIBuilder.base("http://issues.test/browse/").build()) + .headers(Map.of("Extra1", "val1", "Extra2", "val2")) + .sendInterval(Duration.ZERO) + .build(); + + // Populate the projects repository + var localRepo = CheckableRepository.init(tempFolder.path(), author.repositoryType()); + 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, "A simple change", + "Change msg\n\nWith several lines"); + localRepo.push(editHash, author.url(), "edit", true); + var pr = credentials.createPullRequest(archive, "master", "edit", "1234: This is a pull request"); + pr.setBody("This should not be ready"); + + // Run an archive pass + TestBotRunner.runPeriodicItems(mlBot); + + // A PR that isn't ready for review should not be archived + Repository.materialize(archiveFolder.path(), archive.url(), "master"); + assertFalse(archiveContains(archiveFolder.path(), "This is a pull request")); + + // Flag it as ready for review + pr.setBody("This should now be ready"); + pr.addLabel("rfr"); + + // Run another archive pass + TestBotRunner.runPeriodicItems(mlBot); + + // But it should still not be archived + Repository.materialize(archiveFolder.path(), archive.url(), "master"); + assertFalse(archiveContains(archiveFolder.path(), "This is a pull request")); + + // Now post a general comment - not a ready marker + var ignoredPr = ignored.pullRequest(pr.id()); + ignoredPr.addComment("hello there"); + + // Run another archive pass + TestBotRunner.runPeriodicItems(mlBot); + + // It should still not be archived + Repository.materialize(archiveFolder.path(), archive.url(), "master"); + assertFalse(archiveContains(archiveFolder.path(), "This is a pull request")); + + // Now post a ready comment + ignoredPr.addComment("ready"); + + // Run another archive pass + TestBotRunner.runPeriodicItems(mlBot); + + // The archive should now contain an entry + Repository.materialize(archiveFolder.path(), archive.url(), "master"); + assertTrue(archiveContains(archiveFolder.path(), "This is a pull request")); + assertTrue(archiveContains(archiveFolder.path(), "This should now be ready")); + assertTrue(archiveContains(archiveFolder.path(), "Patch:")); + assertTrue(archiveContains(archiveFolder.path(), "Changes:")); + assertTrue(archiveContains(archiveFolder.path(), "Webrev:")); + assertTrue(archiveContains(archiveFolder.path(), webrevServer.uri().toString())); + assertTrue(archiveContains(archiveFolder.path(), "&pr=" + pr.id() + "&range=00")); + assertTrue(archiveContains(archiveFolder.path(), "Issue:")); + assertTrue(archiveContains(archiveFolder.path(), "http://issues.test/browse/TSTPRJ-1234")); + assertTrue(archiveContains(archiveFolder.path(), "Fetch:")); + assertTrue(archiveContains(archiveFolder.path(), "^ - Change msg")); + assertFalse(archiveContains(archiveFolder.path(), "With several lines")); + + // The mailing list as well + listServer.processIncoming(); + var mailmanServer = MailingListServerFactory.createMailmanServer(listServer.getArchive(), listServer.getSMTP(), Duration.ZERO); + var mailmanList = mailmanServer.getList(listAddress.address()); + var conversations = mailmanList.conversations(Duration.ofDays(1)); + assertEquals(1, conversations.size()); + var mail = conversations.get(0).first(); + assertEquals("RFR: 1234: This is a pull request", mail.subject()); + assertEquals(pr.author().fullName(), mail.author().fullName().orElseThrow()); + assertEquals(noreplyAddress(archive), mail.author().address()); + assertEquals(listAddress, mail.sender()); + assertEquals("val1", mail.headerValue("Extra1")); + assertEquals("val2", mail.headerValue("Extra2")); + + // And there should be a JSON version of a webrev + Repository.materialize(webrevFolder.path(), archive.url(), "webrev"); + var jsonDir = webrevFolder.path() + .resolve(author.name()) + .resolve(pr.id()) + .resolve("00"); + assertTrue(Files.exists(jsonDir)); + assertTrue(Files.isDirectory(jsonDir)); + + var commitsFile = jsonDir.resolve("commits.json"); + assertTrue(Files.exists(commitsFile)); + var commits = JSON.parse(Files.readString(commitsFile)); + assertEquals(1, commits.asArray().size()); + var commit = commits.get(0); + assertEquals(editHash.hex(), commit.get("sha").asString()); + assertEquals("Change msg\n\nWith several lines", commit.get("commit").get("message").asString()); + assertEquals(1, commit.get("files").asArray().size()); + + var metadataFile = jsonDir.resolve("metadata.json"); + assertTrue(Files.exists(metadataFile)); + var metadata = JSON.parse(Files.readString(metadataFile)); + assertEquals(masterHash.hex(), metadata.get("base").get("sha").asString()); + assertEquals(editHash.hex(), metadata.get("head").get("sha").asString()); + + var comparisonFile = jsonDir.resolve("comparison.json"); + assertTrue(Files.exists(comparisonFile)); + var comparsion = JSON.parse(Files.readString(comparisonFile)); + assertEquals(1, comparsion.get("files").asArray().size()); + assertEquals("modified", comparsion.get("files").get(0).get("status").asString()); + assertTrue(comparsion.get("files").get(0).get("patch").asString().contains("A simple change")); + + var comments = pr.comments(); + var webrevComments = comments.stream() + .filter(comment -> comment.author().equals(author.forge().currentUser())) + .filter(comment -> comment.body().contains("webrev")) + .filter(comment -> comment.body().contains(editHash.hex())) + .collect(Collectors.toList()); + assertEquals(1, webrevComments.size()); + var comment = webrevComments.get(0); + assertTrue(comment.body().contains("&pr=" + pr.id())); + assertTrue(comment.body().contains("&range=00")); + + // Add a comment + pr.addComment("This is a comment :smile:"); + + // Add a comment from an ignored user as well + ignoredPr.addComment("Don't mind me"); + + // Run another archive pass + TestBotRunner.runPeriodicItems(mlBot); + + // The archive should now contain the comment, but not the ignored one + Repository.materialize(archiveFolder.path(), archive.url(), "master"); + assertTrue(archiveContains(archiveFolder.path(), "This is a comment")); + assertTrue(archiveContains(archiveFolder.path(), "> This should now be ready")); + assertFalse(archiveContains(archiveFolder.path(), "Don't mind me")); + + listServer.processIncoming(); + conversations = mailmanList.conversations(Duration.ofDays(1)); + assertEquals(1, conversations.size()); + assertEquals(2, conversations.get(0).allMessages().size()); + + // Remove the rfr flag and post another comment + pr.addLabel("rfr"); + pr.addComment("This is another comment"); + + // Run another archive pass + TestBotRunner.runPeriodicItems(mlBot); + + // The archive should contain the additional comment + Repository.materialize(archiveFolder.path(), archive.url(), "master"); + assertTrue(archiveContains(archiveFolder.path(), "This is another comment")); + assertTrue(archiveContains(archiveFolder.path(), ">> This should now be ready")); + + listServer.processIncoming(); + conversations = mailmanList.conversations(Duration.ofDays(1)); + assertEquals(1, conversations.size()); + assertEquals(3, conversations.get(0).allMessages().size()); + for (var newMail : conversations.get(0).allMessages()) { + assertEquals(noreplyAddress(archive), newMail.author().address()); + assertEquals(listAddress, newMail.sender()); + } + assertTrue(conversations.get(0).allMessages().get(2).body().contains("This is a comment 😄")); + } + } }