Skip to content

Commit 9943035

Browse files
javoirejavoire
authored
feat: list review comments (#244)
1 parent e93bfda commit 9943035

File tree

2 files changed

+99
-35
lines changed

2 files changed

+99
-35
lines changed

‎src/main/java/com/spotify/github/v3/clients/PullRequestClient.java‎

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@
2727
import static com.spotify.github.v3.clients.GitHubClient.LIST_PR_TYPE_REFERENCE;
2828
import static com.spotify.github.v3.clients.GitHubClient.LIST_REVIEW_REQUEST_TYPE_REFERENCE;
2929
import static com.spotify.github.v3.clients.GitHubClient.LIST_REVIEW_TYPE_REFERENCE;
30-
import static java.util.Objects.isNull;
3130
import static java.lang.Math.toIntExact;
31+
import static java.util.Objects.isNull;
3232

3333
import com.google.common.base.Strings;
3434
import com.google.common.collect.ImmutableMap;
@@ -66,6 +66,8 @@ public class PullRequestClient {
6666
private static final String PR_NUMBER_TEMPLATE = "/repos/%s/%s/pulls/%s";
6767
private static final String PR_COMMITS_TEMPLATE = "/repos/%s/%s/pulls/%s/commits";
6868
private static final String PR_REVIEWS_TEMPLATE = "/repos/%s/%s/pulls/%s/reviews";
69+
private static final String PR_REVIEW_COMMENTS_TEMPLATE =
70+
"/repos/%s/%s/pulls/%s/reviews/%s/comments";
6971
private static final String PR_COMMENTS_TEMPLATE = "/repos/%s/%s/pulls/%s/comments";
7072
private static final String PR_CHANGED_FILES_TEMPLATE = "/repos/%s/%s/pulls/%s/files";
7173
private static final String PR_REVIEW_REQUESTS_TEMPLATE =
@@ -192,14 +194,19 @@ public CompletableFuture<List<CommitItem>> listCommits(final int prNumber) {
192194
public CompletableFuture<List<CommitItem>> listCommits(final long prNumber) {
193195
final String path = String.format(PR_COMMITS_TEMPLATE, owner, repo, prNumber);
194196
log.debug("Fetching pull request commits from " + path);
195-
return github.request(path).thenApply(
196-
response -> Json.create().fromJsonUncheckedNotNull(response.bodyString(), LIST_COMMIT_TYPE_REFERENCE));
197+
return github
198+
.request(path)
199+
.thenApply(
200+
response ->
201+
Json.create()
202+
.fromJsonUncheckedNotNull(response.bodyString(), LIST_COMMIT_TYPE_REFERENCE));
197203
}
198204

199205
public Iterator<AsyncPage<CommitItem>> listCommits(final long prNumber, final int itemsPerPage) {
200206
final String path = String.format(PR_COMMITS_TEMPLATE, owner, repo, prNumber);
201207

202-
return new GithubPageIterator<>(new GithubPage<>(github, path, LIST_COMMIT_TYPE_REFERENCE, itemsPerPage));
208+
return new GithubPageIterator<>(
209+
new GithubPage<>(github, path, LIST_COMMIT_TYPE_REFERENCE, itemsPerPage));
203210
}
204211

205212
/**
@@ -249,7 +256,8 @@ public Iterator<AsyncPage<Review>> listReviews(final int prNumber, final int ite
249256
public Iterator<AsyncPage<Review>> listReviews(final long prNumber, final long itemsPerPage) {
250257
final String path = String.format(PR_REVIEWS_TEMPLATE, owner, repo, prNumber);
251258
log.debug("Fetching pull request reviews from " + path);
252-
return new GithubPageIterator<>(new GithubPage<>(github, path, LIST_REVIEW_TYPE_REFERENCE, toIntExact(itemsPerPage)));
259+
return new GithubPageIterator<>(
260+
new GithubPage<>(github, path, LIST_REVIEW_TYPE_REFERENCE, toIntExact(itemsPerPage)));
253261
}
254262

255263
/**
@@ -506,4 +514,17 @@ public CompletableFuture<Comment> createCommentReply(
506514
log.debug("Creating reply to PR comment: " + path);
507515
return github.post(path, jsonPayload, Comment.class);
508516
}
517+
518+
/**
519+
* List pull request review comments for a specific review with pagination.
520+
*
521+
* @param prNumber pull request number
522+
* @param reviewId the ID of the review
523+
* @return iterator of comments for the review
524+
* @see "https://docs.github.com/en/rest/pulls/comments#list-comments-for-a-pull-request-review"
525+
*/
526+
public Iterator<AsyncPage<Comment>> listReviewComments(final long prNumber, final long reviewId) {
527+
final String path = String.format(PR_REVIEW_COMMENTS_TEMPLATE, owner, repo, prNumber, reviewId);
528+
return new GithubPageIterator<>(new GithubPage<>(github, path, LIST_PR_COMMENT_TYPE_REFERENCE));
529+
}
509530
}

‎src/test/java/com/spotify/github/v3/clients/PullRequestClientTest.java‎

Lines changed: 73 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,8 @@
7474

7575
public class PullRequestClientTest {
7676
private static final String MOCK_GITHUB_HOST = "bogus.host";
77-
private static final URI MOCK_GITHUB_URI = URI.create(String.format("http://%s/api/v3", MOCK_GITHUB_HOST));
77+
private static final URI MOCK_GITHUB_URI =
78+
URI.create(String.format("http://%s/api/v3", MOCK_GITHUB_HOST));
7879
private static final URI MOCK_GITHUB_URI_GQL = MOCK_GITHUB_URI.resolve("/graphql");
7980

8081
private static final String PR_CHANGED_FILES_TEMPLATE = "/repos/%s/%s/pulls/%s/files";
@@ -89,9 +90,7 @@ private static String getFixture(String resource) throws IOException {
8990
@BeforeEach
9091
public void setUp() {
9192
client = mock(OkHttpClient.class);
92-
github =
93-
GitHubClient.create(
94-
client,MOCK_GITHUB_URI, MOCK_GITHUB_URI_GQL, "token");
93+
github = GitHubClient.create(client, MOCK_GITHUB_URI, MOCK_GITHUB_URI_GQL, "token");
9594
mockGithub = mock(GitHubClient.class);
9695
}
9796

@@ -429,7 +428,8 @@ void testChangedFiles() throws IOException {
429428

430429
final HttpResponse firstPageResponse = createMockResponse(pageLink, expectedBody);
431430

432-
when(mockGithub.request(format(PR_CHANGED_FILES_TEMPLATE + "?per_page=30", "owner", "repo", "1")))
431+
when(mockGithub.request(
432+
format(PR_CHANGED_FILES_TEMPLATE + "?per_page=30", "owner", "repo", "1")))
433433
.thenReturn(completedFuture(firstPageResponse));
434434

435435
when(mockGithub.json()).thenReturn(github.json());
@@ -449,7 +449,8 @@ public void testListComments() throws Throwable {
449449
final String expectedBody = "[" + getFixture("pull_request_review_comment_reply.json") + "]";
450450

451451
final String pageLink =
452-
"<https://github.com/api/v3/repos/owner/repo/pulls/1/comments?page=1&per_page=30>; rel=\"first\"";
452+
"<https://github.com/api/v3/repos/owner/repo/pulls/1/comments?page=1&per_page=30>;"
453+
+ " rel=\"first\"";
453454

454455
final HttpResponse firstPageResponse = createMockResponse(pageLink, expectedBody);
455456

@@ -470,60 +471,98 @@ public void testListComments() throws Throwable {
470471
assertThat(comments.get(0).user().login(), is("octocat"));
471472
}
472473

474+
@Test
475+
public void testListReviewComments() throws Throwable {
476+
final String expectedBody = "[" + getFixture("pull_request_review_comment_reply.json") + "]";
477+
478+
final String pageLink =
479+
"<https://github.com/api/v3/repos/owner/repo/pulls/1/reviews/123/comments?page=1&per_page=30>;"
480+
+ " rel=\"first\"";
481+
482+
final HttpResponse firstPageResponse = createMockResponse(pageLink, expectedBody);
483+
484+
when(mockGithub.request("/repos/owner/repo/pulls/1/reviews/123/comments?per_page=30"))
485+
.thenReturn(completedFuture(firstPageResponse));
486+
487+
when(mockGithub.json()).thenReturn(github.json());
488+
489+
final PullRequestClient pullRequestClient =
490+
PullRequestClient.create(mockGithub, "owner", "repo");
491+
492+
final Iterable<AsyncPage<Comment>> pageIterator =
493+
() -> pullRequestClient.listReviewComments(1L, 123L);
494+
List<Comment> comments = Async.streamFromPaginatingIterable(pageIterator).collect(toList());
495+
496+
assertEquals(1, comments.size());
497+
assertThat(comments.get(0).body(), is("Great stuff!"));
498+
assertThat(comments.get(0).id(), is(10L));
499+
assertThat(comments.get(0).user().login(), is("octocat"));
500+
}
501+
473502
@Test
474503
public void listCommitsWithoutSpecifyingPages() throws Exception {
475504
// Given
476505
final int COMMIT_PER_PAGE = 30;
477506

478507
final String firstPageLink =
479-
"<https://api.github.com/repos/owner/repo/pulls/1/commits?page=2>; rel=\"next\", <https://api.github.com/repos/owner/repo/pulls/1/commits?page=4>; rel=\"last\"";
508+
"<https://api.github.com/repos/owner/repo/pulls/1/commits?page=2>; rel=\"next\","
509+
+ " <https://api.github.com/repos/owner/repo/pulls/1/commits?page=4>; rel=\"last\"";
480510
final String firstPageBody =
481-
Resources.toString(getResource(this.getClass(), "pull_request_commits_page1.json"), defaultCharset());
511+
Resources.toString(
512+
getResource(this.getClass(), "pull_request_commits_page1.json"), defaultCharset());
482513
final HttpResponse firstPageResponse = createMockResponse(firstPageLink, firstPageBody);
483514

484515
when(mockGithub.request("/repos/owner/repo/pulls/1/commits"))
485516
.thenReturn(completedFuture(firstPageResponse));
486517

487-
final PullRequestClient pullRequestClient = PullRequestClient.create(mockGithub, "owner", "repo");
518+
final PullRequestClient pullRequestClient =
519+
PullRequestClient.create(mockGithub, "owner", "repo");
488520

489521
// When
490522
final List<CommitItem> commits = pullRequestClient.listCommits(1L).get();
491523

492524
// Then
493525
assertThat(commits.size(), is(COMMIT_PER_PAGE));
494526
assertThat(
495-
commits.get(COMMIT_PER_PAGE - 1).commit().tree().sha(), is("219cb4c1ffada21259876d390df1a85767481617"));
527+
commits.get(COMMIT_PER_PAGE - 1).commit().tree().sha(),
528+
is("219cb4c1ffada21259876d390df1a85767481617"));
496529
}
497530

498531
@Test
499532
public void listCommitsSpecifyingPages() throws Exception {
500533
// Given
501534
final int COMMIT_PER_PAGE = 30;
502535

503-
final String firstPageLink = String.format(
504-
"<%s/repos/owner/repo/pulls/1/commits?page=2&per_page=30>; rel=\"next\", <%s/repos/owner/repo/pulls/1/commits?page=3&per_page=30>; rel=\"last\"",
505-
MOCK_GITHUB_URI,
506-
MOCK_GITHUB_URI);
536+
final String firstPageLink =
537+
String.format(
538+
"<%s/repos/owner/repo/pulls/1/commits?page=2&per_page=30>; rel=\"next\","
539+
+ " <%s/repos/owner/repo/pulls/1/commits?page=3&per_page=30>; rel=\"last\"",
540+
MOCK_GITHUB_URI, MOCK_GITHUB_URI);
507541
final String firstPageBody =
508-
Resources.toString(getResource(this.getClass(), "pull_request_commits_page1.json"), defaultCharset());
542+
Resources.toString(
543+
getResource(this.getClass(), "pull_request_commits_page1.json"), defaultCharset());
509544
final HttpResponse firstPageResponse = createMockResponse(firstPageLink, firstPageBody);
510545

511-
final String secondPageLink = String.format(
512-
"<%s/repos/owner/repo/pulls/1/commits?page=1&per_page=30>; rel=\"prev\", <%s/repos/owner/repo/pulls/1/commits?page=3&per_page=30>; rel=\"next\", <%s/repos/owner/repo/pulls/1/commits?page=3&per_page=30>; rel=\"last\", <%s/repos/owner/repo/pulls/1/commits?page=1&per_page=30>; rel=\"first\"",
513-
MOCK_GITHUB_URI,
514-
MOCK_GITHUB_URI,
515-
MOCK_GITHUB_URI,
516-
MOCK_GITHUB_URI);
546+
final String secondPageLink =
547+
String.format(
548+
"<%s/repos/owner/repo/pulls/1/commits?page=1&per_page=30>; rel=\"prev\","
549+
+ " <%s/repos/owner/repo/pulls/1/commits?page=3&per_page=30>; rel=\"next\","
550+
+ " <%s/repos/owner/repo/pulls/1/commits?page=3&per_page=30>; rel=\"last\","
551+
+ " <%s/repos/owner/repo/pulls/1/commits?page=1&per_page=30>; rel=\"first\"",
552+
MOCK_GITHUB_URI, MOCK_GITHUB_URI, MOCK_GITHUB_URI, MOCK_GITHUB_URI);
517553
final String secondPageBody =
518-
Resources.toString(getResource(this.getClass(), "pull_request_commits_page2.json"), defaultCharset());
554+
Resources.toString(
555+
getResource(this.getClass(), "pull_request_commits_page2.json"), defaultCharset());
519556
final HttpResponse secondPageResponse = createMockResponse(secondPageLink, secondPageBody);
520557

521558
final String thirdPageLink =
522-
String.format("<%s/repos/owner/repo/pulls/1/commits?page=2&per_page=30>; rel=\"prev\", <%s/repos/owner/repo/pulls/1/commits?page=1&per_page=30>; rel=\"first\"",
523-
MOCK_GITHUB_URI,
524-
MOCK_GITHUB_URI);
559+
String.format(
560+
"<%s/repos/owner/repo/pulls/1/commits?page=2&per_page=30>; rel=\"prev\","
561+
+ " <%s/repos/owner/repo/pulls/1/commits?page=1&per_page=30>; rel=\"first\"",
562+
MOCK_GITHUB_URI, MOCK_GITHUB_URI);
525563
final String thirdPageBody =
526-
Resources.toString(getResource(this.getClass(), "pull_request_commits_page3.json"), defaultCharset());
564+
Resources.toString(
565+
getResource(this.getClass(), "pull_request_commits_page3.json"), defaultCharset());
527566
final HttpResponse thirdPageResponse = createMockResponse(thirdPageLink, thirdPageBody);
528567

529568
when(mockGithub.urlFor("")).thenReturn(MOCK_GITHUB_URI.toString());
@@ -537,15 +576,19 @@ public void listCommitsSpecifyingPages() throws Exception {
537576
when(mockGithub.request("/repos/owner/repo/pulls/1/commits?page=3&per_page=30"))
538577
.thenReturn(completedFuture(thirdPageResponse));
539578

540-
final PullRequestClient pullRequestClient = PullRequestClient.create(mockGithub, "owner", "repo");
579+
final PullRequestClient pullRequestClient =
580+
PullRequestClient.create(mockGithub, "owner", "repo");
541581

542582
// When
543-
final Iterable<AsyncPage<CommitItem>> pageIterator = () -> pullRequestClient.listCommits(1L, 30);
544-
final List<CommitItem> commits = Async.streamFromPaginatingIterable(pageIterator).collect(toList());
583+
final Iterable<AsyncPage<CommitItem>> pageIterator =
584+
() -> pullRequestClient.listCommits(1L, 30);
585+
final List<CommitItem> commits =
586+
Async.streamFromPaginatingIterable(pageIterator).collect(toList());
545587

546588
// Then
547589
assertThat(commits.size(), is(COMMIT_PER_PAGE * 3));
548590
assertThat(
549-
commits.get(COMMIT_PER_PAGE - 1).commit().tree().sha(), is("219cb4c1ffada21259876d390df1a85767481617"));
591+
commits.get(COMMIT_PER_PAGE - 1).commit().tree().sha(),
592+
is("219cb4c1ffada21259876d390df1a85767481617"));
550593
}
551594
}

0 commit comments

Comments
 (0)