Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mlbridge: support generating JSON for webrevs #719

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -66,8 +66,9 @@ public class MailingListBridgeBot implements Bot {
MailingListBridgeBot(EmailAddress from, HostedRepository repo, HostedRepository archive, String archiveRef,
HostedRepository censusRepo, String censusRef, List<MailingListConfiguration> lists,
Set<String> ignoredUsers, Set<Pattern> ignoredComments, URI listArchive, String smtpServer,
HostedRepository webrevStorageRepository, String webrevStorageRef,
Path webrevStorageBase, URI webrevStorageBaseUri, Set<String> readyLabels,
HostedRepository webrevStorageHTMLRepository, HostedRepository webrevStorageJSONRepository,
String webrevStorageRef, Path webrevStorageBase, URI webrevStorageBaseUri,
boolean webrevGenerateHTML, boolean webrevGenerateJSON, Set<String> readyLabels,
Map<String, Pattern> readyComments, URI issueTracker, Map<String, String> 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();
}
@@ -43,10 +43,13 @@ public class MailingListBridgeBotBuilder {
private Set<Pattern> 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<String> readyLabels = Set.of();
private Map<String, Pattern> 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<String> 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);
}
}
}
@@ -79,7 +79,8 @@ public List<Bot> 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<Bot> 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<Bot> 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)
@@ -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);
@@ -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())