Skip to content

Commit

Permalink
Switch to GitHub's new review comment positioning API
Browse files Browse the repository at this point in the history 
Reviewed-by: ehelin
  • Loading branch information
@rwestberg
rwestberg committed Nov 26, 2019
1 parent 59405b7 commit fe273af
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 461 deletions.
13 changes: 8 additions & 5 deletions bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/ReviewArchive.java
View file
Expand Up @@ -373,13 +373,16 @@ void addReviewComment(ReviewComment reviewComment) {

// Add some context to the first post
if (reviewComment.parent().isEmpty()) {
var contents = prInstance.pr().repository().fileContents(reviewComment.path(), reviewComment.hash().hex()).lines().collect(Collectors.toList());

body.append(reviewComment.path()).append(" line ").append(reviewComment.line()).append(":\n\n");
for (int i = Math.max(0, reviewComment.line() - 2); i < Math.min(contents.size(), reviewComment.line() + 1); ++i) {
body.append("> ").append(i + 1).append(": ").append(contents.get(i)).append("\n");
try {
var contents = prInstance.pr().repository().fileContents(reviewComment.path(), reviewComment.hash().hex()).lines().collect(Collectors.toList());
for (int i = Math.max(0, reviewComment.line() - 2); i < Math.min(contents.size(), reviewComment.line() + 1); ++i) {
body.append("> ").append(i + 1).append(": ").append(contents.get(i)).append("\n");
}
body.append("\n");
} catch (RuntimeException e) {
body.append("> (failed to retrieve contents of file, check the PR for context)\n");
}
body.append("\n");
}
body.append(reviewComment.body());

Expand Down
120 changes: 57 additions & 63 deletions forge/src/main/java/org/openjdk/skara/forge/github/GitHubPullRequest.java
View file
Expand Up @@ -29,7 +29,6 @@
import org.openjdk.skara.network.*;
import org.openjdk.skara.vcs.Hash;

import java.io.*;
import java.net.URI;
import java.time.*;
import java.time.format.DateTimeFormatter;
Expand Down Expand Up @@ -119,14 +118,35 @@ public void addReview(Review.Verdict verdict, String body) {
.execute();
}

private ReviewComment parseReviewComment(ReviewComment parent, JSONObject json, PositionMapper diff) {
private ReviewComment parseReviewComment(ReviewComment parent, JSONObject json) {
var author = host.parseUserField(json);
var threadId = parent == null ? json.get("id").toString() : parent.threadId();

int line = json.get("original_line").asInt();
var hash = new Hash(json.get("original_commit_id").asString());
var path = json.get("path").asString();

if (json.get("side").asString().equals("LEFT")) {
var commitInfo = request.get("commits/" + hash).execute();

// It's possible that the file in question was renamed / deleted in an earlier commit, would
// need to parse all the commits in the PR to be sure. But this should cover most cases.
hash = new Hash(commitInfo.get("parents").asArray().get(0).get("sha").asString());
for (var file : commitInfo.get("files").asArray()) {
if (file.get("filename").asString().equals(path)) {
if (file.get("status").asString().equals("renamed")) {
path = file.get("previous_filename").asString();
}
break;
}
}
}

var comment = new ReviewComment(parent,
threadId,
new Hash(json.get("commit_id").asString()),
json.get("path").asString(),
diff.positionToLine(json.get("path").asString(), json.get("original_position").asInt()),
hash,
path,
line,
json.get("id").toString(),
json.get("body").asString(),
author,
Expand All @@ -137,74 +157,48 @@ private ReviewComment parseReviewComment(ReviewComment parent, JSONObject json,

@Override
public ReviewComment addReviewComment(Hash base, Hash hash, String path, int line, String body) {
try {
var rawDiff = request.get("pulls/" + json.get("number").toString())
.header("Accept", "application/vnd.github.v3.diff")
.executeUnparsed();
var diff = PositionMapper.parse(rawDiff);

var query = JSON.object()
.put("body", body)
.put("commit_id", hash.hex())
.put("path", path)
.put("position", diff.lineToPosition(path, line));
var response = request.post("pulls/" + json.get("number").toString() + "/comments")
.body(query)
.execute();
return parseReviewComment(null, response.asObject(), diff);
} catch (IOException e) {
throw new UncheckedIOException(e);
}
var query = JSON.object()
.put("body", body)
.put("commit_id", hash.hex())
.put("path", path)
.put("side", "RIGHT")
.put("line", line);
var response = request.post("pulls/" + json.get("number").toString() + "/comments")
.body(query)
.execute();
return parseReviewComment(null, response.asObject());
}

@Override
public ReviewComment addReviewCommentReply(ReviewComment parent, String body) {
try {
var rawDiff = request.get("pulls/" + json.get("number").toString())
.header("Accept", "application/vnd.github.v3.diff")
.executeUnparsed();
var diff = PositionMapper.parse(rawDiff);

var query = JSON.object()
.put("body", body)
.put("in_reply_to", Integer.parseInt(parent.threadId()));
var response = request.post("pulls/" + json.get("number").toString() + "/comments")
.body(query)
.execute();
return parseReviewComment(parent, response.asObject(), diff);
} catch (IOException e) {
throw new UncheckedIOException(e);
}
var query = JSON.object()
.put("body", body)
.put("in_reply_to", Integer.parseInt(parent.threadId()));
var response = request.post("pulls/" + json.get("number").toString() + "/comments")
.body(query)
.execute();
return parseReviewComment(parent, response.asObject());
}

@Override
public List<ReviewComment> reviewComments() {
try {
var rawDiff = request.get("pulls/" + json.get("number").toString())
.header("Accept", "application/vnd.github.v3.diff")
.executeUnparsed();
var diff = PositionMapper.parse(rawDiff);

var ret = new ArrayList<ReviewComment>();
var reviewComments = request.get("pulls/" + json.get("number").toString() + "/comments").execute().stream()
.map(JSONValue::asObject)
.collect(Collectors.toList());
var idToComment = new HashMap<String, ReviewComment>();

for (var reviewComment : reviewComments) {
ReviewComment parent = null;
if (reviewComment.contains("in_reply_to_id")) {
parent = idToComment.get(reviewComment.get("in_reply_to_id").toString());
}
var comment = parseReviewComment(parent, reviewComment, diff);
idToComment.put(comment.id(), comment);
ret.add(comment);
}
var ret = new ArrayList<ReviewComment>();
var reviewComments = request.get("pulls/" + json.get("number").toString() + "/comments").execute().stream()
.map(JSONValue::asObject)
.collect(Collectors.toList());
var idToComment = new HashMap<String, ReviewComment>();

return ret;
} catch (IOException e) {
throw new UncheckedIOException(e);
for (var reviewComment : reviewComments) {
ReviewComment parent = null;
if (reviewComment.contains("in_reply_to_id")) {
parent = idToComment.get(reviewComment.get("in_reply_to_id").toString());
}
var comment = parseReviewComment(parent, reviewComment);
idToComment.put(comment.id(), comment);
ret.add(comment);
}

return ret;
}

@Override
Expand Down
3 changes: 2 additions & 1 deletion forge/src/main/java/org/openjdk/skara/forge/github/GitHubRepository.java
View file
Expand Up @@ -53,7 +53,8 @@ public class GitHubRepository implements HostedRepository {
"Authorization", "token " + gitHubHost.getInstallationToken(),
"Accept", "application/vnd.github.machine-man-preview+json",
"Accept", "application/vnd.github.antiope-preview+json",
"Accept", "application/vnd.github.shadow-cat-preview+json"));
"Accept", "application/vnd.github.shadow-cat-preview+json",
"Accept", "application/vnd.github.comfort-fade-preview+json"));
json = gitHubHost.getProjectInfo(repository);
var urlPattern = gitHubHost.getWebURI("/" + repository + "/pull/").toString();
pullRequestPattern = Pattern.compile(urlPattern + "(\\d+)");
Expand Down
110 changes: 0 additions & 110 deletions forge/src/main/java/org/openjdk/skara/forge/github/PositionMapper.java
View file

This file was deleted.

0 comments on commit fe273af

Please sign in to comment.