diff --git a/bots/notify/src/main/java/org/openjdk/skara/bots/notify/NotifyBot.java b/bots/notify/src/main/java/org/openjdk/skara/bots/notify/NotifyBot.java index 90e3cf69c..e116409b4 100644 --- a/bots/notify/src/main/java/org/openjdk/skara/bots/notify/NotifyBot.java +++ b/bots/notify/src/main/java/org/openjdk/skara/bots/notify/NotifyBot.java @@ -44,11 +44,12 @@ public class NotifyBot implements Bot { private final PullRequestUpdateCache updateCache; private final Set readyLabels; private final Map readyComments; + private final String integratorId; NotifyBot(HostedRepository repository, Path storagePath, Pattern branches, StorageBuilder tagStorageBuilder, StorageBuilder branchStorageBuilder, StorageBuilder prStateStorageBuilder, List updaters, List prUpdaters, - Set readyLabels, Map readyComments) { + Set readyLabels, Map readyComments, String integratorId) { this.repository = repository; this.storagePath = storagePath; this.branches = branches; @@ -60,6 +61,7 @@ public class NotifyBot implements Bot { this.updateCache = new PullRequestUpdateCache(); this.readyLabels = readyLabels; this.readyComments = readyComments; + this.integratorId = integratorId; } public static NotifyBotBuilder newBuilder() { @@ -112,7 +114,11 @@ public List getPeriodicItems() { if (!isReady(pr)) { continue; } - ret.add(new PullRequestWorkItem(pr, prStateStorageBuilder, prUpdaters, e -> updateCache.invalidate(pr))); + ret.add(new PullRequestWorkItem(pr, + prStateStorageBuilder, + prUpdaters, + e -> updateCache.invalidate(pr), + integratorId)); } } diff --git a/bots/notify/src/main/java/org/openjdk/skara/bots/notify/NotifyBotBuilder.java b/bots/notify/src/main/java/org/openjdk/skara/bots/notify/NotifyBotBuilder.java index 6f39bbe3d..b6334b4cb 100644 --- a/bots/notify/src/main/java/org/openjdk/skara/bots/notify/NotifyBotBuilder.java +++ b/bots/notify/src/main/java/org/openjdk/skara/bots/notify/NotifyBotBuilder.java @@ -40,6 +40,7 @@ public class NotifyBotBuilder { private List prUpdaters = List.of(); private Set readyLabels = Set.of(); private Map readyComments = Map.of(); + private String integratorId; public NotifyBotBuilder repository(HostedRepository repository) { this.repository = repository; @@ -91,7 +92,12 @@ public NotifyBotBuilder readyComments(Map readyComments) { return this; } + public NotifyBotBuilder integratorId(String integratorId) { + this.integratorId = integratorId; + return this; + } + public NotifyBot build() { - return new NotifyBot(repository, storagePath, branches, tagStorageBuilder, branchStorageBuilder, prStateStorageBuilder, updaters, prUpdaters, readyLabels, readyComments); + return new NotifyBot(repository, storagePath, branches, tagStorageBuilder, branchStorageBuilder, prStateStorageBuilder, updaters, prUpdaters, readyLabels, readyComments, integratorId); } } diff --git a/bots/notify/src/main/java/org/openjdk/skara/bots/notify/NotifyBotFactory.java b/bots/notify/src/main/java/org/openjdk/skara/bots/notify/NotifyBotFactory.java index b8652a5d4..10b265044 100644 --- a/bots/notify/src/main/java/org/openjdk/skara/bots/notify/NotifyBotFactory.java +++ b/bots/notify/src/main/java/org/openjdk/skara/bots/notify/NotifyBotFactory.java @@ -70,6 +70,7 @@ public List create(BotConfiguration configuration) { .map(JSONValue::asObject) .collect(Collectors.toMap(obj -> obj.get("user").asString(), obj -> Pattern.compile(obj.get("pattern").asString()))); + var integratorId = specific.get("integrator").asString(); // Collect configuration applicable to all instances of a specific notifier var notifierFactories = NotifierFactory.getNotifierFactories(); @@ -135,6 +136,7 @@ public List create(BotConfiguration configuration) { .prUpdaters(prUpdaters) .readyLabels(readyLabels) .readyComments(readyComments) + .integratorId(integratorId) .build(); ret.add(bot); } diff --git a/bots/notify/src/main/java/org/openjdk/skara/bots/notify/PullRequestState.java b/bots/notify/src/main/java/org/openjdk/skara/bots/notify/PullRequestState.java index 2885e949f..3a368724c 100644 --- a/bots/notify/src/main/java/org/openjdk/skara/bots/notify/PullRequestState.java +++ b/bots/notify/src/main/java/org/openjdk/skara/bots/notify/PullRequestState.java @@ -23,21 +23,25 @@ package org.openjdk.skara.bots.notify; import org.openjdk.skara.forge.PullRequest; +import org.openjdk.skara.vcs.Hash; import java.util.*; class PullRequestState { private final String prId; private final Set issueIds; + private final Hash commitId; - PullRequestState(PullRequest pr, Set issueIds) { + PullRequestState(PullRequest pr, Set issueIds, Hash commitId) { this.prId = pr.repository().id() + ":" + pr.id(); this.issueIds = issueIds; + this.commitId = commitId; } - PullRequestState(String prId, Set issueIds) { + PullRequestState(String prId, Set issueIds, Hash commitId) { this.prId = prId; this.issueIds = issueIds; + this.commitId = commitId; } public String prId() { @@ -48,11 +52,16 @@ public Set issueIds() { return issueIds; } + public Optional commitId() { + return Optional.ofNullable(commitId); + } + @Override public String toString() { return "PullRequestState{" + "prId='" + prId + '\'' + ", issueIds=" + issueIds + + ", commitId=" + commitId + '}'; } @@ -66,11 +75,12 @@ public boolean equals(Object o) { } var that = (PullRequestState) o; return prId.equals(that.prId) && - issueIds.equals(that.issueIds); + issueIds.equals(that.issueIds) && + Objects.equals(commitId, that.commitId); } @Override public int hashCode() { - return Objects.hash(prId, issueIds); + return Objects.hash(prId, issueIds, commitId); } } diff --git a/bots/notify/src/main/java/org/openjdk/skara/bots/notify/PullRequestWorkItem.java b/bots/notify/src/main/java/org/openjdk/skara/bots/notify/PullRequestWorkItem.java index 772114db3..fd864869d 100644 --- a/bots/notify/src/main/java/org/openjdk/skara/bots/notify/PullRequestWorkItem.java +++ b/bots/notify/src/main/java/org/openjdk/skara/bots/notify/PullRequestWorkItem.java @@ -24,8 +24,10 @@ import org.openjdk.skara.bot.WorkItem; import org.openjdk.skara.forge.PullRequest; +import org.openjdk.skara.host.HostUser; import org.openjdk.skara.json.*; import org.openjdk.skara.storage.StorageBuilder; +import org.openjdk.skara.vcs.Hash; import org.openjdk.skara.vcs.openjdk.Issue; import java.nio.file.Path; @@ -39,12 +41,31 @@ public class PullRequestWorkItem implements WorkItem { private final StorageBuilder prStateStorageBuilder; private final List pullRequestUpdateConsumers; private final Consumer errorHandler; + private final String integratorId; - PullRequestWorkItem(PullRequest pr, StorageBuilder prStateStorageBuilder, List pullRequestUpdateConsumers, Consumer errorHandler) { + PullRequestWorkItem(PullRequest pr, StorageBuilder prStateStorageBuilder, List pullRequestUpdateConsumers, Consumer errorHandler, String integratorId) { this.pr = pr; this.prStateStorageBuilder = prStateStorageBuilder; this.pullRequestUpdateConsumers = pullRequestUpdateConsumers; this.errorHandler = errorHandler; + this.integratorId = integratorId; + } + + private Hash resultingCommitHashFor(PullRequest pr) { + if (pr.labels().contains("integrated")) { + for (var comment : pr.comments()) { + if (comment.author().id().equals(integratorId)) { + for (var line : comment.body().split("\n")) { + if (line.startsWith("Pushed as commit")) { + var parts = line.split(" "); + var hash = parts[parts.length - 1].replace(".", ""); + return new Hash(hash); + } + } + } + } + } + return null; } private Set deserializePrState(String current) { @@ -54,9 +75,29 @@ private Set deserializePrState(String current) { var data = JSON.parse(current); return data.stream() .map(JSONValue::asObject) - .map(obj -> new PullRequestState(obj.get("pr").asString(), obj.get("issues").stream() - .map(JSONValue::asString) - .collect(Collectors.toSet()))) + .map(obj -> { + var id = obj.get("pr").asString(); + var issues = obj.get("issues").stream() + .map(JSONValue::asString) + .collect(Collectors.toSet()); + + // Storage might be missing commit information + if (!obj.contains("commit")) { + var prId = id.split(":")[1]; + var currentPR = pr.repository().pullRequest(prId); + var hash = resultingCommitHashFor(currentPR); + if (hash == null) { + obj.putNull("commit"); + } else { + obj.put("commit", hash.hex()); + } + } + + var commit = obj.get("commit").isNull() ? + null : new Hash(obj.get("commit").asString()); + + return new PullRequestState(id, issues, commit); + }) .collect(Collectors.toSet()); } @@ -71,10 +112,17 @@ private String serializePrState(Collection added, Set JSON.object().put("pr", pr.prId()).put("issues", new JSONArray( - pr.issueIds().stream() - .map(JSON::of) - .collect(Collectors.toList())))) + .map(pr -> { + var issues = new JSONArray(pr.issueIds() + .stream() + .map(JSON::of) + .collect(Collectors.toList())); + var commit = pr.commitId().isPresent()? + JSON.of(pr.commitId().get().hex()) : JSON.of(); + return JSON.object().put("pr", pr.prId()) + .put("issues",issues) + .put("commit", commit); + }) .map(JSONObject::toString) .collect(Collectors.toList()); return "[\n" + String.join(",\n", entries) + "\n]"; @@ -130,31 +178,32 @@ public void run(Path scratchPath) { .materialize(historyPath); var issues = parseIssues(); - var prIssues = new PullRequestState(pr, issues); - var current = storage.current(); - if (current.contains(prIssues)) { + var commit = resultingCommitHashFor(pr); + var state = new PullRequestState(pr, issues, commit); + var stored = storage.current(); + if (stored.contains(state)) { // Already up to date return; } // Search for an existing - var oldPrIssues = current.stream() - .filter(p -> p.prId().equals(prIssues.prId())) + var storedState = stored.stream() + .filter(ss -> ss.prId().equals(state.prId())) .findAny(); - if (oldPrIssues.isPresent()) { - var oldIssues = oldPrIssues.get().issueIds(); - oldIssues.stream() - .filter(issue -> !issues.contains(issue)) - .forEach(this::notifyListenersRemoved); + if (storedState.isPresent()) { + var storedIssues = storedState.get().issueIds(); + storedIssues.stream() + .filter(issue -> !issues.contains(issue)) + .forEach(this::notifyListenersRemoved); issues.stream() - .filter(issue -> !oldIssues.contains(issue)) + .filter(issue -> !storedIssues.contains(issue)) .forEach(this::notifyListenersAdded); } else { notifyNewPr(pr); issues.forEach(this::notifyListenersAdded); } - storage.put(prIssues); + storage.put(state); } @Override