Skip to content

Commit

Permalink
notify: add handleIntegratedPullRequest callback
Browse files Browse the repository at this point in the history
  • Loading branch information
@edvbld
edvbld committed Jun 8, 2020
1 parent 1d06390 commit 10da516
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 46 deletions.
3 changes: 3 additions & 0 deletions bots/notify/src/main/java/org/openjdk/skara/bots/notify/PullRequestUpdateConsumer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
package org.openjdk.skara.bots.notify;

import org.openjdk.skara.forge.PullRequest;
import org.openjdk.skara.vcs.Hash;
import org.openjdk.skara.vcs.openjdk.Issue;

public interface PullRequestUpdateConsumer extends Notifier {
Expand All @@ -32,4 +33,6 @@ default void handleRemovedIssue(PullRequest pr, Issue issue) {
}
default void handleNewPullRequest(PullRequest pr) {
}
default void handleIntegratedPullRequest(PullRequest pr, Hash hash) {
}
}
12 changes: 12 additions & 0 deletions bots/notify/src/main/java/org/openjdk/skara/bots/notify/PullRequestWorkItem.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,10 @@ private void notifyNewPr(PullRequest pr) {
pullRequestUpdateConsumers.forEach(c -> c.handleNewPullRequest(pr));
}

private void notifyIntegratedPr(PullRequest pr, Hash hash) {
pullRequestUpdateConsumers.forEach(c -> c.handleIntegratedPullRequest(pr, hash));
}

@Override
public void run(Path scratchPath) {
var historyPath = scratchPath.resolve("notify").resolve("history");
Expand Down Expand Up @@ -198,9 +202,17 @@ public void run(Path scratchPath) {
issues.stream()
.filter(issue -> !storedIssues.contains(issue))
.forEach(this::notifyListenersAdded);

var storedCommit = storedState.get().commitId();
if (!storedCommit.isPresent() && state.commitId().isPresent()) {
notifyIntegratedPr(pr, state.commitId().get());
}
} else {
notifyNewPr(pr);
issues.forEach(this::notifyListenersAdded);
if (state.commitId().isPresent()) {
notifyIntegratedPr(pr, state.commitId().get());
}
}

storage.put(state);
Expand Down
78 changes: 34 additions & 44 deletions bots/notify/src/main/java/org/openjdk/skara/bots/notify/issue/IssueUpdater.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
import java.util.*;
import java.util.logging.Logger;

public class IssueUpdater implements RepositoryUpdateConsumer, PullRequestUpdateConsumer {
public class IssueUpdater implements PullRequestUpdateConsumer {
private final IssueProject issueProject;
private final boolean reviewLink;
private final URI reviewIcon;
Expand All @@ -58,7 +58,7 @@ public String name() {
return "issue";
}

private Optional<String> findIssueUsername(Commit commit) {
private Optional<String> findIssueUsername(CommitMetadata commit) {
var authorEmail = EmailAddress.from(commit.author().email());
if (authorEmail.domain().equals("openjdk.org")) {
return Optional.of(authorEmail.localPart());
Expand All @@ -73,50 +73,40 @@ private Optional<String> findIssueUsername(Commit commit) {
}

@Override
public void handleCommits(HostedRepository repository, Repository localRepository, List<Commit> commits, Branch branch) {
for (var commit : commits) {
var commitMessage = CommitMessageParsers.v1.parse(commit);
for (var commitIssue : commitMessage.issues()) {
var optionalIssue = issueProject.issue(commitIssue.shortId());
if (optionalIssue.isEmpty()) {
log.severe("Cannot update issue " + commitIssue.id() + " with commit " + commit.hash().abbreviate()
+ " - issue not found in issue project");
continue;
}
var issue = optionalIssue.get();

var candidates = repository.findPullRequestsWithComment(null, "Pushed as commit " + commit.hash() + ".");
if (candidates.size() != 1) {
log.info("IssueUpdater@" + issue.id() + ": Skipping commit " + commit.hash().abbreviate() + " for repository " + repository.name() +
" on branch " + branch.name() + " - " + candidates.size() + " matching PRs found (needed 1)");
continue;
}
var candidate = candidates.get(0);
var prLink = candidate.webUrl();
if (!candidate.targetRef().equals(branch.name())) {
log.info("IssueUpdater@" + issue.id() + ": Pull request " + prLink + " targets " + candidate.targetRef() + " - commit is on " + branch.toString() + " - skipping");
continue;
}

if (commitLink) {
var linkBuilder = Link.create(repository.webUrl(commit.hash()), "Commit")
.summary(repository.name() + "/" + commit.hash().abbreviate());
if (commitIcon != null) {
linkBuilder.iconTitle("Commit");
linkBuilder.iconUrl(commitIcon);
}
issue.addLink(linkBuilder.build());
public void handleIntegratedPullRequest(PullRequest pr, Hash hash) {
var repository = pr.repository();
var commit = repository.commitMetadata(hash).orElseThrow(() ->
new IllegalStateException("Integrated commit " + hash +
" not present in repository " + repository.webUrl())
);
var commitMessage = CommitMessageParsers.v1.parse(commit);
for (var commitIssue : commitMessage.issues()) {
var optionalIssue = issueProject.issue(commitIssue.shortId());
if (optionalIssue.isEmpty()) {
log.severe("Cannot update issue " + commitIssue.id() + " with commit " + commit.hash().abbreviate()
+ " - issue not found in issue project");
continue;
}
var issue = optionalIssue.get();

if (commitLink) {
var linkBuilder = Link.create(repository.webUrl(hash), "Commit")
.summary(repository.name() + "/" + hash.abbreviate());
if (commitIcon != null) {
linkBuilder.iconTitle("Commit");
linkBuilder.iconUrl(commitIcon);
}
issue.addLink(linkBuilder.build());
}

if (issue.state() == Issue.State.OPEN) {
issue.setState(Issue.State.RESOLVED);
if (issue.assignees().isEmpty()) {
var username = findIssueUsername(commit);
if (username.isPresent()) {
var assignee = issueProject.issueTracker().user(username.get());
if (assignee.isPresent()) {
issue.setAssignees(List.of(assignee.get()));
}
if (issue.state() == Issue.State.OPEN) {
issue.setState(Issue.State.RESOLVED);
if (issue.assignees().isEmpty()) {
var username = findIssueUsername(commit);
if (username.isPresent()) {
var assignee = issueProject.issueTracker().user(username.get());
if (assignee.isPresent()) {
issue.setAssignees(List.of(assignee.get()));
}
}
}
Expand Down
7 changes: 5 additions & 2 deletions bots/notify/src/test/java/org/openjdk/skara/bots/notify/IssueUpdaterTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,8 @@ void testIssueIdempotence(TestInfo testInfo) throws IOException {
.tagStorageBuilder(tagStorage)
.branchStorageBuilder(branchStorage)
.prStateStorageBuilder(prStateStorage)
.updaters(List.of(updater))
.prUpdaters(List.of(updater))
.integratorId(repo.forge().currentUser().id())
.build();

// Initialize history
Expand All @@ -80,6 +81,7 @@ void testIssueIdempotence(TestInfo testInfo) throws IOException {
localRepo.push(editHash, repo.url(), "master");
var pr = credentials.createPullRequest(repo, "master", "master", issue.id() + ": Fix that issue");
pr.setBody("\n\n### Issue\n * [" + issue.id() + "](http://www.test.test/): The issue");
pr.addLabel("integrated");
pr.addComment("Pushed as commit " + editHash.hex() + ".");
TestBotRunner.runPeriodicItems(notifyBot);

Expand Down Expand Up @@ -294,8 +296,8 @@ void testPullRequestPROnly(TestInfo testInfo) throws IOException {
.tagStorageBuilder(tagStorage)
.branchStorageBuilder(branchStorage)
.prStateStorageBuilder(prStateStorage)
.updaters(List.of(updater))
.prUpdaters(List.of(updater))
.integratorId(repo.forge().currentUser().id())
.build();

// Initialize history
Expand All @@ -318,6 +320,7 @@ void testPullRequestPROnly(TestInfo testInfo) throws IOException {

// Simulate integration
pr.addComment("Pushed as commit " + editHash.hex() + ".");
pr.addLabel("integrated");
localRepo.push(editHash, repo.url(), "other");
TestBotRunner.runPeriodicItems(notifyBot);

Expand Down
4 changes: 4 additions & 0 deletions vcs/src/main/java/org/openjdk/skara/vcs/openjdk/CommitMessageParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
package org.openjdk.skara.vcs.openjdk;

import org.openjdk.skara.vcs.Commit;
import org.openjdk.skara.vcs.CommitMetadata;

import java.util.List;

Expand All @@ -32,4 +33,7 @@ public interface CommitMessageParser {
default CommitMessage parse(Commit c) {
return parse(c.message());
}
default CommitMessage parse(CommitMetadata metadata) {
return parse(metadata.message());
}
}

0 comments on commit 10da516

Please sign in to comment.