diff --git a/src/main/java/org/sonar/plugins/stash/StashPluginUtils.java b/src/main/java/org/sonar/plugins/stash/StashPluginUtils.java index 49dc1621..f064fd1e 100644 --- a/src/main/java/org/sonar/plugins/stash/StashPluginUtils.java +++ b/src/main/java/org/sonar/plugins/stash/StashPluginUtils.java @@ -3,7 +3,9 @@ import com.google.common.base.CharMatcher; import java.io.IOException; import java.util.Collection; +import java.util.Optional; import java.util.Properties; +import java.util.stream.Stream; import org.sonar.api.batch.fs.InputComponent; import org.sonar.api.batch.fs.InputModule; import org.sonar.api.batch.postjob.issue.PostJobIssue; @@ -72,4 +74,8 @@ public static String removeEnd(String s, String suffix) { } return s; } + + public static Stream removeEmpty(Optional v) { + return v.map(Stream::of).orElse(Stream.empty()); + } } diff --git a/src/main/java/org/sonar/plugins/stash/StashRequestFacade.java b/src/main/java/org/sonar/plugins/stash/StashRequestFacade.java index 06452827..d4b7f961 100644 --- a/src/main/java/org/sonar/plugins/stash/StashRequestFacade.java +++ b/src/main/java/org/sonar/plugins/stash/StashRequestFacade.java @@ -394,43 +394,45 @@ public StashDiffReport getPullRequestDiffReport(PullRequestRef pr, StashClient s * Reset all comments linked to a pull-request. */ public void resetComments(PullRequestRef pr, - StashDiffReport diffReport, + Collection comments, StashUser sonarUser, StashClient stashClient) { try { - // Let's call this "diffRep_loop" - for (StashComment comment : diffReport.getComments()) { - - // delete comment only if published by the current SQ user - if (sonarUser.getId() != comment.getAuthor().getId()) { - continue; - // Next element in "diffRep_loop" - - // comment contains tasks which cannot be deleted => do nothing - } else if (comment.containsPermanentTasks()) { - LOGGER.debug("Comment \"{}\" (path:\"{}\", line:\"{}\")" - + "CANNOT be deleted because one of its tasks is not deletable.", comment.getId(), - comment.getPath(), - comment.getLine()); - continue; // Next element in "diffRep_loop" - } - - // delete tasks linked to the current comment - for (StashTask task : comment.getTasks()) { - stashClient.deleteTaskOnComment(task); - } - - stashClient.deletePullRequestComment(pr, comment); - } + // FIXME delete tasks on file-wide comments + // resetComments(diffReport.getComments(), pr, sonarUser, stashClient); + resetComments(comments, pr, sonarUser, stashClient); LOGGER.info("SonarQube issues reported to Stash by user \"{}\" have been reset", sonarUser.getName()); - } catch (StashClientException e) { LOGGER.error("Unable to reset comment list", e); } } + private void resetComments(Collection comments, PullRequestRef pr, StashUser sonarUser, StashClient stashClient) + throws StashClientException { + for (StashComment comment : comments) { + if (sonarUser.getId() != comment.getAuthor().getId()) { + continue; + } + + if (comment.containsPermanentTasks()) { + LOGGER.debug("Comment \"{}\" (path:\"{}\", line:\"{}\")" + + "CANNOT be deleted because one of its tasks is not deletable.", comment.getId(), + comment.getPath(), + comment.getLine()); + continue; // Next element in "diffRep_loop" + } + + // delete tasks linked to the current comment + for (StashTask task : comment.getTasks()) { + stashClient.deleteTaskOnComment(task); + } + + stashClient.deletePullRequestComment(pr, comment); + } + } + @Override public String getIssuePath(PostJobIssue issue) { InputComponent ip = issue.inputComponent(); diff --git a/src/main/java/org/sonar/plugins/stash/client/StashClient.java b/src/main/java/org/sonar/plugins/stash/client/StashClient.java index b7c45f01..56118b05 100644 --- a/src/main/java/org/sonar/plugins/stash/client/StashClient.java +++ b/src/main/java/org/sonar/plugins/stash/client/StashClient.java @@ -1,6 +1,8 @@ package org.sonar.plugins.stash.client; +import java.util.Collection; import java.util.Optional; +import java.util.stream.Collectors; import org.asynchttpclient.AsyncHttpClient; import org.asynchttpclient.BoundRequestBuilder; import org.asynchttpclient.DefaultAsyncHttpClient; @@ -16,10 +18,10 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.slf4j.MDC; +import org.sonar.api.batch.rule.Severity; import org.sonar.plugins.stash.PeekableInputStream; import org.sonar.plugins.stash.PluginInfo; import org.sonar.plugins.stash.PullRequestRef; -import org.sonar.plugins.stash.StashPlugin; import org.sonar.plugins.stash.StashPlugin.IssueType; import org.sonar.plugins.stash.StashPluginUtils; import org.sonar.plugins.stash.exceptions.StashClientException; @@ -64,7 +66,8 @@ public class StashClient implements AutoCloseable { private static final String API_ONE_PR_ALL_COMMENTS = API_ONE_PR + "/comments"; private static final String API_ONE_PR_DIFF = API_ONE_PR + "/diff?withComments=true"; private static final String API_ONE_PR_APPROVAL = API_ONE_PR + "/approve"; - private static final String API_ONE_PR_COMMENT_PATH = API_ONE_PR + "/comments?path={4}&start={5,number,#}"; + private static final String API_ONE_PR_COMMENT_PATH = API_ONE_PR + "/comments?path={4}"; + private static final String API_ONE_PR_ACTIVITIES = API_ONE_PR + "/activities"; private static final String API_ONE_PR_ONE_COMMENT = API_ONE_PR_ALL_COMMENTS + "/{4}?version={5}"; @@ -111,31 +114,29 @@ public void postCommentOnPullRequest(PullRequestRef pr, String report) postCreate(request, json, MessageFormat.format(COMMENT_POST_ERROR_MESSAGE, pr.repository(), pr.pullRequestId())); } + public Collection getPullRequestOverviewComments(PullRequestRef pr) throws StashClientException { + return getPaged( + MessageFormat.format(API_ONE_PR_ACTIVITIES, baseUrl, pr.project(), pr.repository(), pr.pullRequestId()), + "Error!" + ).stream().map(StashCollector::extractCommentFromActivity).flatMap(StashPluginUtils::removeEmpty).collect(Collectors.toList()); + } + public StashCommentReport getPullRequestComments(PullRequestRef pr, String path) throws StashClientException { StashCommentReport result = new StashCommentReport(); - long start = 0; - boolean isLastPage = false; + String url = MessageFormat.format(API_ONE_PR_COMMENT_PATH, + baseUrl, + pr.project(), + pr.repository(), + pr.pullRequestId(), + path + ); - while (!isLastPage) { + for (JsonObject jsonComment: getPaged(url, + MessageFormat.format(COMMENT_GET_ERROR_MESSAGE, pr.repository(), pr.pullRequestId()))) { try { - String request = MessageFormat.format(API_ONE_PR_COMMENT_PATH, - baseUrl, - pr.project(), - pr.repository(), - pr.pullRequestId(), - path, - start); - JsonObject jsonComments = get(request, - MessageFormat.format(COMMENT_GET_ERROR_MESSAGE, - pr.repository(), - pr.pullRequestId())); - result.add(StashCollector.extractComments(jsonComments)); - - // Stash pagination: check if you get all comments linked to the pull-request - isLastPage = StashCollector.isLastPage(jsonComments); - start = StashCollector.getNextPageStart(jsonComments); + result.add(StashCollector.extractComment(jsonComment)); } catch (StashReportExtractionException e) { throw new StashClientException(e); } @@ -310,6 +311,10 @@ private JsonObject get(String url, String errorMessage) throws StashClientExcept return performRequest(httpClient.prepareGet(url), null, HttpURLConnection.HTTP_OK, errorMessage); } + private Collection getPaged(String url, String errorMessage) throws StashClientException { + return performPagedRequest(httpClient.prepareGet(url), null, HttpURLConnection.HTTP_OK, errorMessage); + } + private JsonObject post(String url, JsonObject body, String errorMessage) throws StashClientException { return performRequest(httpClient.preparePost(url), body, HttpURLConnection.HTTP_OK, errorMessage); } @@ -456,4 +461,34 @@ AsyncHttpClient createHttpClient(String sonarQubeVersion) { .build() ); } + + private Collection performPagedRequest(BoundRequestBuilder requestBuilder, + JsonObject body, + int expectedStatusCode, + String errorMessage) throws StashClientException { + + Collection result = new ArrayList<>(); + + boolean doneLastPage = false; + int nextPageStart = 0; + // FIXME size/limit support + + while (!doneLastPage) { + requestBuilder.resetQuery(); + if (nextPageStart > 0) { + requestBuilder.addQueryParam("start", String.valueOf(nextPageStart)); + } + + JsonObject res = performRequest(requestBuilder, body, expectedStatusCode, errorMessage); + JsonArray values = ((JsonArray) res.get("values")); + if (values == null) { + throw new StashClientException("Paged response did not include values"); + } + values.asCollection(result); + doneLastPage = res.getBooleanOrDefault("isLastPage", true); + nextPageStart = res.getIntegerOrDefault("nextPageStart", -1); + } + + return result; + } } diff --git a/src/main/java/org/sonar/plugins/stash/issue/collector/StashCollector.java b/src/main/java/org/sonar/plugins/stash/issue/collector/StashCollector.java index 8d4898a4..5d53ebe5 100644 --- a/src/main/java/org/sonar/plugins/stash/issue/collector/StashCollector.java +++ b/src/main/java/org/sonar/plugins/stash/issue/collector/StashCollector.java @@ -1,6 +1,8 @@ package org.sonar.plugins.stash.issue.collector; import java.math.BigDecimal; +import java.util.Objects; +import java.util.Optional; import org.json.simple.JsonArray; import org.json.simple.JsonObject; import org.sonar.plugins.stash.PullRequestRef; @@ -20,9 +22,7 @@ public final class StashCollector { private static final String AUTHOR = "author"; private static final String VERSION = "version"; - private StashCollector() { - // Hiding implicit public constructor with an explicit private one (squid:S1118) - } + private StashCollector() {} public static StashCommentReport extractComments(JsonObject jsonComments) throws StashReportExtractionException { StashCommentReport result = new StashCommentReport(); @@ -41,8 +41,11 @@ public static StashCommentReport extractComments(JsonObject jsonComments) throws return result; } - public static StashComment extractComment(JsonObject jsonComment, String path, Long line) { + public static Optional extractCommentFromActivity(JsonObject json) { + return Optional.ofNullable((JsonObject) json.get("comment")).filter(Objects::nonNull).map(j -> StashCollector.extractComment(j, null, null)); + } + public static StashComment extractComment(JsonObject jsonComment, String path, Long line) { long id = getLong(jsonComment, "id"); String message = (String)jsonComment.get("text"); @@ -51,7 +54,10 @@ public static StashComment extractComment(JsonObject jsonComment, String path, L JsonObject jsonAuthor = (JsonObject)jsonComment.get(AUTHOR); StashUser stashUser = extractUser(jsonAuthor); - return new StashComment(id, message, path, line, stashUser, version); + StashComment result = new StashComment(id, message, path, line, stashUser, version); + // FIXME do this at some central place + updateCommentTasks(result, (JsonArray) jsonComment.get("tasks")); + return result; } public static StashComment extractComment(JsonObject jsonComment) throws StashReportExtractionException { diff --git a/src/test/java/org/sonar/plugins/stash/StashRequestFacadeTest.java b/src/test/java/org/sonar/plugins/stash/StashRequestFacadeTest.java index 13ac61ce..451cd377 100755 --- a/src/test/java/org/sonar/plugins/stash/StashRequestFacadeTest.java +++ b/src/test/java/org/sonar/plugins/stash/StashRequestFacadeTest.java @@ -246,6 +246,7 @@ public void setUp() throws Exception { comments.add(comment3); when(diffReport.getComments()).thenReturn(comments); + when(stashClient.getPullRequestOverviewComments(pr)).thenReturn(comments); stashCommentsReport1 = mock(StashCommentReport.class); when(stashCommentsReport1.getComments()).thenReturn(comments); @@ -713,6 +714,7 @@ public void testResetCommentsWithDifferentStashUsers() throws Exception { comments.add(comment); when(diffReport.getComments()).thenReturn(comments); + when(stashClient.getPullRequestOverviewComments(eq(pr))).thenReturn(comments); myFacade.resetComments(pr, diffReport, stashUser, stashClient); @@ -721,7 +723,8 @@ public void testResetCommentsWithDifferentStashUsers() throws Exception { @Test public void testResetCommentsWithoutAnyComments() throws Exception { - when(diffReport.getComments()).thenReturn(new ArrayList()); + when(diffReport.getComments()).thenReturn(new ArrayList<>()); + when(stashClient.getPullRequestOverviewComments(eq(pr))).thenReturn(new ArrayList<>()); myFacade.resetComments(pr, diffReport, stashUser, stashClient); diff --git a/src/test/java/org/sonar/plugins/stash/client/StashClientTest.java b/src/test/java/org/sonar/plugins/stash/client/StashClientTest.java index 080e1df2..749d39f0 100755 --- a/src/test/java/org/sonar/plugins/stash/client/StashClientTest.java +++ b/src/test/java/org/sonar/plugins/stash/client/StashClientTest.java @@ -150,8 +150,7 @@ public void testGetPullRequestCommentsWithNextPage() throws Exception { + "\"author\": {\"id\":1, \"name\":\"SonarQube\", \"slug\":\"sonarqube\", \"email\":\"sq@email.com\"}, \"version\": 0}], \"isLastPage\": true}"; wireMock.stubFor(get( - urlPathEqualTo("/rest/api/1.0/projects/Project/repos/Repository/pull-requests/1/comments")) - .withQueryParam("start", equalTo(String.valueOf(0))).willReturn( + urlPathEqualTo("/rest/api/1.0/projects/Project/repos/Repository/pull-requests/1/comments")).willReturn( aJsonResponse().withStatus(HttpURLConnection.HTTP_OK).withBody(stashJsonComment1) )); @@ -177,7 +176,7 @@ public void testGetPullRequestCommentsWithNoNextPage() throws Exception { "{\"values\": [{\"id\":4321, \"text\":\"message2\", \"anchor\": {\"path\":\"path\", \"line\":10}," + "\"author\": {\"id\":1, \"name\":\"SonarQube\", \"slug\":\"sonarqube\", \"email\":\"sq@email.com\"}, \"version\": 0}], \"isLastPage\": true}"; - wireMock.stubFor(get(anyUrl()).withQueryParam("start", equalTo(String.valueOf(0))).willReturn( + wireMock.stubFor(get(anyUrl()).willReturn( aJsonResponse().withStatus(HttpURLConnection.HTTP_OK).withBody(stashJsonComment1))); wireMock.stubFor(get(anyUrl()).withQueryParam("start", equalTo(String.valueOf(1))).willReturn( @@ -371,7 +370,7 @@ public void testPullRequestHugePullRequestId() throws Exception { // See https://github.com/AmadeusITGroup/sonar-stash/issues/98 int hugePullRequestId = 1234567890; - wireMock.stubFor(any(anyUrl()).willReturn(aJsonResponse())); + wireMock.stubFor(any(anyUrl()).willReturn(aPagedJsonResponse())); PullRequestRef pr = PullRequestRef.builder() .setProject("Project") @@ -424,7 +423,15 @@ private void addErrorResponse(MappingBuilder mapping, int statusCode) { } public static ResponseDefinitionBuilder aJsonResponse() { - return aResponse().withHeader("Content-Type", "application/json").withBody("{}"); + return aJsonResponse("{}"); + } + + private ResponseDefinitionBuilder aPagedJsonResponse() { + return aJsonResponse("{\"values\":[]}"); + } + + public static ResponseDefinitionBuilder aJsonResponse(String body) { + return aResponse().withHeader("Content-Type", "application/json").withBody(body); } public static ResponseDefinitionBuilder aXMLResponse() {