diff --git a/src/main/java/com/spotify/github/v3/clients/PullRequestClient.java b/src/main/java/com/spotify/github/v3/clients/PullRequestClient.java index e54f9180..36d2783e 100644 --- a/src/main/java/com/spotify/github/v3/clients/PullRequestClient.java +++ b/src/main/java/com/spotify/github/v3/clients/PullRequestClient.java @@ -27,8 +27,8 @@ import static com.spotify.github.v3.clients.GitHubClient.LIST_PR_TYPE_REFERENCE; import static com.spotify.github.v3.clients.GitHubClient.LIST_REVIEW_REQUEST_TYPE_REFERENCE; import static com.spotify.github.v3.clients.GitHubClient.LIST_REVIEW_TYPE_REFERENCE; -import static java.util.Objects.isNull; import static java.lang.Math.toIntExact; +import static java.util.Objects.isNull; import com.google.common.base.Strings; import com.google.common.collect.ImmutableMap; @@ -66,6 +66,8 @@ public class PullRequestClient { private static final String PR_NUMBER_TEMPLATE = "/repos/%s/%s/pulls/%s"; private static final String PR_COMMITS_TEMPLATE = "/repos/%s/%s/pulls/%s/commits"; private static final String PR_REVIEWS_TEMPLATE = "/repos/%s/%s/pulls/%s/reviews"; + private static final String PR_REVIEW_COMMENTS_TEMPLATE = + "/repos/%s/%s/pulls/%s/reviews/%s/comments"; private static final String PR_COMMENTS_TEMPLATE = "/repos/%s/%s/pulls/%s/comments"; private static final String PR_CHANGED_FILES_TEMPLATE = "/repos/%s/%s/pulls/%s/files"; private static final String PR_REVIEW_REQUESTS_TEMPLATE = @@ -192,14 +194,19 @@ public CompletableFuture> listCommits(final int prNumber) { public CompletableFuture> listCommits(final long prNumber) { final String path = String.format(PR_COMMITS_TEMPLATE, owner, repo, prNumber); log.debug("Fetching pull request commits from " + path); - return github.request(path).thenApply( - response -> Json.create().fromJsonUncheckedNotNull(response.bodyString(), LIST_COMMIT_TYPE_REFERENCE)); + return github + .request(path) + .thenApply( + response -> + Json.create() + .fromJsonUncheckedNotNull(response.bodyString(), LIST_COMMIT_TYPE_REFERENCE)); } public Iterator> listCommits(final long prNumber, final int itemsPerPage) { final String path = String.format(PR_COMMITS_TEMPLATE, owner, repo, prNumber); - return new GithubPageIterator<>(new GithubPage<>(github, path, LIST_COMMIT_TYPE_REFERENCE, itemsPerPage)); + return new GithubPageIterator<>( + new GithubPage<>(github, path, LIST_COMMIT_TYPE_REFERENCE, itemsPerPage)); } /** @@ -249,7 +256,8 @@ public Iterator> listReviews(final int prNumber, final int ite public Iterator> listReviews(final long prNumber, final long itemsPerPage) { final String path = String.format(PR_REVIEWS_TEMPLATE, owner, repo, prNumber); log.debug("Fetching pull request reviews from " + path); - return new GithubPageIterator<>(new GithubPage<>(github, path, LIST_REVIEW_TYPE_REFERENCE, toIntExact(itemsPerPage))); + return new GithubPageIterator<>( + new GithubPage<>(github, path, LIST_REVIEW_TYPE_REFERENCE, toIntExact(itemsPerPage))); } /** @@ -506,4 +514,17 @@ public CompletableFuture createCommentReply( log.debug("Creating reply to PR comment: " + path); return github.post(path, jsonPayload, Comment.class); } + + /** + * List pull request review comments for a specific review with pagination. + * + * @param prNumber pull request number + * @param reviewId the ID of the review + * @return iterator of comments for the review + * @see "https://docs.github.com/en/rest/pulls/comments#list-comments-for-a-pull-request-review" + */ + public Iterator> listReviewComments(final long prNumber, final long reviewId) { + final String path = String.format(PR_REVIEW_COMMENTS_TEMPLATE, owner, repo, prNumber, reviewId); + return new GithubPageIterator<>(new GithubPage<>(github, path, LIST_PR_COMMENT_TYPE_REFERENCE)); + } } diff --git a/src/test/java/com/spotify/github/v3/clients/PullRequestClientTest.java b/src/test/java/com/spotify/github/v3/clients/PullRequestClientTest.java index 1edaa80c..a203a602 100644 --- a/src/test/java/com/spotify/github/v3/clients/PullRequestClientTest.java +++ b/src/test/java/com/spotify/github/v3/clients/PullRequestClientTest.java @@ -74,7 +74,8 @@ public class PullRequestClientTest { private static final String MOCK_GITHUB_HOST = "bogus.host"; - private static final URI MOCK_GITHUB_URI = URI.create(String.format("http://%s/api/v3", MOCK_GITHUB_HOST)); + private static final URI MOCK_GITHUB_URI = + URI.create(String.format("http://%s/api/v3", MOCK_GITHUB_HOST)); private static final URI MOCK_GITHUB_URI_GQL = MOCK_GITHUB_URI.resolve("/graphql"); 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 { @BeforeEach public void setUp() { client = mock(OkHttpClient.class); - github = - GitHubClient.create( - client,MOCK_GITHUB_URI, MOCK_GITHUB_URI_GQL, "token"); + github = GitHubClient.create(client, MOCK_GITHUB_URI, MOCK_GITHUB_URI_GQL, "token"); mockGithub = mock(GitHubClient.class); } @@ -429,7 +428,8 @@ void testChangedFiles() throws IOException { final HttpResponse firstPageResponse = createMockResponse(pageLink, expectedBody); - when(mockGithub.request(format(PR_CHANGED_FILES_TEMPLATE + "?per_page=30", "owner", "repo", "1"))) + when(mockGithub.request( + format(PR_CHANGED_FILES_TEMPLATE + "?per_page=30", "owner", "repo", "1"))) .thenReturn(completedFuture(firstPageResponse)); when(mockGithub.json()).thenReturn(github.json()); @@ -449,7 +449,8 @@ public void testListComments() throws Throwable { final String expectedBody = "[" + getFixture("pull_request_review_comment_reply.json") + "]"; final String pageLink = - "; rel=\"first\""; + ";" + + " rel=\"first\""; final HttpResponse firstPageResponse = createMockResponse(pageLink, expectedBody); @@ -470,21 +471,52 @@ public void testListComments() throws Throwable { assertThat(comments.get(0).user().login(), is("octocat")); } + @Test + public void testListReviewComments() throws Throwable { + final String expectedBody = "[" + getFixture("pull_request_review_comment_reply.json") + "]"; + + final String pageLink = + ";" + + " rel=\"first\""; + + final HttpResponse firstPageResponse = createMockResponse(pageLink, expectedBody); + + when(mockGithub.request("/repos/owner/repo/pulls/1/reviews/123/comments?per_page=30")) + .thenReturn(completedFuture(firstPageResponse)); + + when(mockGithub.json()).thenReturn(github.json()); + + final PullRequestClient pullRequestClient = + PullRequestClient.create(mockGithub, "owner", "repo"); + + final Iterable> pageIterator = + () -> pullRequestClient.listReviewComments(1L, 123L); + List comments = Async.streamFromPaginatingIterable(pageIterator).collect(toList()); + + assertEquals(1, comments.size()); + assertThat(comments.get(0).body(), is("Great stuff!")); + assertThat(comments.get(0).id(), is(10L)); + assertThat(comments.get(0).user().login(), is("octocat")); + } + @Test public void listCommitsWithoutSpecifyingPages() throws Exception { // Given final int COMMIT_PER_PAGE = 30; final String firstPageLink = - "; rel=\"next\", ; rel=\"last\""; + "; rel=\"next\"," + + " ; rel=\"last\""; final String firstPageBody = - Resources.toString(getResource(this.getClass(), "pull_request_commits_page1.json"), defaultCharset()); + Resources.toString( + getResource(this.getClass(), "pull_request_commits_page1.json"), defaultCharset()); final HttpResponse firstPageResponse = createMockResponse(firstPageLink, firstPageBody); when(mockGithub.request("/repos/owner/repo/pulls/1/commits")) .thenReturn(completedFuture(firstPageResponse)); - final PullRequestClient pullRequestClient = PullRequestClient.create(mockGithub, "owner", "repo"); + final PullRequestClient pullRequestClient = + PullRequestClient.create(mockGithub, "owner", "repo"); // When final List commits = pullRequestClient.listCommits(1L).get(); @@ -492,7 +524,8 @@ public void listCommitsWithoutSpecifyingPages() throws Exception { // Then assertThat(commits.size(), is(COMMIT_PER_PAGE)); assertThat( - commits.get(COMMIT_PER_PAGE - 1).commit().tree().sha(), is("219cb4c1ffada21259876d390df1a85767481617")); + commits.get(COMMIT_PER_PAGE - 1).commit().tree().sha(), + is("219cb4c1ffada21259876d390df1a85767481617")); } @Test @@ -500,30 +533,36 @@ public void listCommitsSpecifyingPages() throws Exception { // Given final int COMMIT_PER_PAGE = 30; - final String firstPageLink = String.format( - "<%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\"", - MOCK_GITHUB_URI, - MOCK_GITHUB_URI); + final String firstPageLink = + String.format( + "<%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\"", + MOCK_GITHUB_URI, MOCK_GITHUB_URI); final String firstPageBody = - Resources.toString(getResource(this.getClass(), "pull_request_commits_page1.json"), defaultCharset()); + Resources.toString( + getResource(this.getClass(), "pull_request_commits_page1.json"), defaultCharset()); final HttpResponse firstPageResponse = createMockResponse(firstPageLink, firstPageBody); - final String secondPageLink = String.format( - "<%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\"", - MOCK_GITHUB_URI, - MOCK_GITHUB_URI, - MOCK_GITHUB_URI, - MOCK_GITHUB_URI); + final String secondPageLink = + String.format( + "<%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\"", + MOCK_GITHUB_URI, MOCK_GITHUB_URI, MOCK_GITHUB_URI, MOCK_GITHUB_URI); final String secondPageBody = - Resources.toString(getResource(this.getClass(), "pull_request_commits_page2.json"), defaultCharset()); + Resources.toString( + getResource(this.getClass(), "pull_request_commits_page2.json"), defaultCharset()); final HttpResponse secondPageResponse = createMockResponse(secondPageLink, secondPageBody); final String thirdPageLink = - 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\"", - MOCK_GITHUB_URI, - MOCK_GITHUB_URI); + 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\"", + MOCK_GITHUB_URI, MOCK_GITHUB_URI); final String thirdPageBody = - Resources.toString(getResource(this.getClass(), "pull_request_commits_page3.json"), defaultCharset()); + Resources.toString( + getResource(this.getClass(), "pull_request_commits_page3.json"), defaultCharset()); final HttpResponse thirdPageResponse = createMockResponse(thirdPageLink, thirdPageBody); when(mockGithub.urlFor("")).thenReturn(MOCK_GITHUB_URI.toString()); @@ -537,15 +576,19 @@ public void listCommitsSpecifyingPages() throws Exception { when(mockGithub.request("/repos/owner/repo/pulls/1/commits?page=3&per_page=30")) .thenReturn(completedFuture(thirdPageResponse)); - final PullRequestClient pullRequestClient = PullRequestClient.create(mockGithub, "owner", "repo"); + final PullRequestClient pullRequestClient = + PullRequestClient.create(mockGithub, "owner", "repo"); // When - final Iterable> pageIterator = () -> pullRequestClient.listCommits(1L, 30); - final List commits = Async.streamFromPaginatingIterable(pageIterator).collect(toList()); + final Iterable> pageIterator = + () -> pullRequestClient.listCommits(1L, 30); + final List commits = + Async.streamFromPaginatingIterable(pageIterator).collect(toList()); // Then assertThat(commits.size(), is(COMMIT_PER_PAGE * 3)); assertThat( - commits.get(COMMIT_PER_PAGE - 1).commit().tree().sha(), is("219cb4c1ffada21259876d390df1a85767481617")); + commits.get(COMMIT_PER_PAGE - 1).commit().tree().sha(), + is("219cb4c1ffada21259876d390df1a85767481617")); } }