From a520c41a41f876797d53bce671fc76ad3e88a519 Mon Sep 17 00:00:00 2001 From: Piotr Wach Date: Wed, 22 Mar 2017 16:59:23 +0000 Subject: [PATCH 1/2] Property to set project id so that multiple SonarQube projects can comment on the same pull request. --- .../sonar/plugins/github/GitHubPlugin.java | 9 +++- .../github/GitHubPluginConfiguration.java | 4 ++ .../sonar/plugins/github/GlobalReport.java | 23 +++++++--- .../plugins/github/MarkDownReportBuilder.java | 6 +++ .../sonar/plugins/github/MarkDownUtils.java | 6 +++ .../plugins/github/PullRequestFacade.java | 11 +++++ .../github/PullRequestIssuePostJob.java | 9 +++- .../sonar/plugins/github/ReportBuilder.java | 8 ++++ .../plugins/github/GlobalReportTest.java | 44 +++++++++++++++---- .../github/PullRequestIssuePostJobTest.java | 20 ++++++--- 10 files changed, 115 insertions(+), 25 deletions(-) diff --git a/src/main/java/org/sonar/plugins/github/GitHubPlugin.java b/src/main/java/org/sonar/plugins/github/GitHubPlugin.java index ff4d840..53d34ca 100644 --- a/src/main/java/org/sonar/plugins/github/GitHubPlugin.java +++ b/src/main/java/org/sonar/plugins/github/GitHubPlugin.java @@ -59,7 +59,13 @@ description = "Issues will not be reported as inline comments but only in the global summary comment", project = true, global = true, - type = PropertyType.BOOLEAN) + type = PropertyType.BOOLEAN), + @Property( + key = GitHubPlugin.GITHUB_PROJECT_ID, + name = "Project id", + description = "Project id used to distinguish between comments from different SonarQube projects", + project = true, + global = true) }) public class GitHubPlugin implements Plugin { @@ -68,6 +74,7 @@ public class GitHubPlugin implements Plugin { public static final String GITHUB_REPO = "sonar.github.repository"; public static final String GITHUB_PULL_REQUEST = "sonar.github.pullRequest"; public static final String GITHUB_DISABLE_INLINE_COMMENTS = "sonar.github.disableInlineComments"; + public static final String GITHUB_PROJECT_ID = "sonar.github.projectId"; @Override diff --git a/src/main/java/org/sonar/plugins/github/GitHubPluginConfiguration.java b/src/main/java/org/sonar/plugins/github/GitHubPluginConfiguration.java index b627be4..3a634b8 100644 --- a/src/main/java/org/sonar/plugins/github/GitHubPluginConfiguration.java +++ b/src/main/java/org/sonar/plugins/github/GitHubPluginConfiguration.java @@ -136,6 +136,10 @@ public boolean tryReportIssuesInline() { return !settings.getBoolean(GitHubPlugin.GITHUB_DISABLE_INLINE_COMMENTS); } + public String projectId() { + return settings.getString(GitHubPlugin.GITHUB_PROJECT_ID); + } + /** * Checks if a proxy was passed with command line parameters or configured in the system. * If only an HTTP proxy was configured then it's properties are copied to the HTTPS proxy (like SonarQube configuration) diff --git a/src/main/java/org/sonar/plugins/github/GlobalReport.java b/src/main/java/org/sonar/plugins/github/GlobalReport.java index 2b6b104..b8d9df2 100644 --- a/src/main/java/org/sonar/plugins/github/GlobalReport.java +++ b/src/main/java/org/sonar/plugins/github/GlobalReport.java @@ -21,6 +21,8 @@ import java.util.Locale; import javax.annotation.Nullable; + +import org.apache.commons.lang.StringUtils; import org.kohsuke.github.GHCommitState; import org.sonar.api.batch.postjob.issue.PostJobIssue; import org.sonar.api.batch.rule.Severity; @@ -30,15 +32,17 @@ public class GlobalReport { private int[] newIssuesBySeverity = new int[Severity.values().length]; private int extraIssueCount = 0; private int maxGlobalReportedIssues; + private String projectId; private final ReportBuilder builder; - public GlobalReport(MarkDownUtils markDownUtils, boolean tryReportIssuesInline) { - this(markDownUtils, tryReportIssuesInline, GitHubPluginConfiguration.MAX_GLOBAL_ISSUES); + public GlobalReport(MarkDownUtils markDownUtils, boolean tryReportIssuesInline, String projectId) { + this(markDownUtils, tryReportIssuesInline, GitHubPluginConfiguration.MAX_GLOBAL_ISSUES, projectId); } - public GlobalReport(MarkDownUtils markDownUtils, boolean tryReportIssuesInline, int maxGlobalReportedIssues) { + public GlobalReport(MarkDownUtils markDownUtils, boolean tryReportIssuesInline, int maxGlobalReportedIssues, String projectId) { this.tryReportIssuesInline = tryReportIssuesInline; this.maxGlobalReportedIssues = maxGlobalReportedIssues; + this.projectId = projectId; this.builder = new MarkDownReportBuilder(markDownUtils); } @@ -48,13 +52,18 @@ private void increment(Severity severity) { public String formatForMarkdown() { int newIssues = newIssues(Severity.BLOCKER) + newIssues(Severity.CRITICAL) + newIssues(Severity.MAJOR) + newIssues(Severity.MINOR) + newIssues(Severity.INFO); - if (newIssues == 0) { - return "SonarQube analysis reported no issues."; - } boolean hasInlineIssues = newIssues > extraIssueCount; boolean extraIssuesTruncated = extraIssueCount > maxGlobalReportedIssues; - builder.append("SonarQube analysis reported ").append(newIssues).append(" issue").append(newIssues > 1 ? "s" : "").append("\n"); + if (!StringUtils.isEmpty(projectId)) { + builder.appendProjectId(projectId).append(" "); + } + if (newIssues == 0) { + builder.append("SonarQube analysis reported no issues."); + return builder.toString(); + } else { + builder.append("SonarQube analysis reported ").append(newIssues).append(" issue").append(newIssues > 1 ? "s" : "").append("\n"); + } if (hasInlineIssues || extraIssuesTruncated) { appendSummaryBySeverity(builder); } diff --git a/src/main/java/org/sonar/plugins/github/MarkDownReportBuilder.java b/src/main/java/org/sonar/plugins/github/MarkDownReportBuilder.java index c288b57..f06b1ce 100644 --- a/src/main/java/org/sonar/plugins/github/MarkDownReportBuilder.java +++ b/src/main/java/org/sonar/plugins/github/MarkDownReportBuilder.java @@ -50,6 +50,12 @@ private IssueHolder(PostJobIssue issue, String gitHubUrl) { this.markDownUtils = markDownUtils; } + @Override + public ReportBuilder appendProjectId(String projectId) { + sb.append(markDownUtils.projectId(projectId)); + return this; + } + @Override public ReportBuilder append(Object o) { sb.append(o); diff --git a/src/main/java/org/sonar/plugins/github/MarkDownUtils.java b/src/main/java/org/sonar/plugins/github/MarkDownUtils.java index 7fcca91..df6ea75 100644 --- a/src/main/java/org/sonar/plugins/github/MarkDownUtils.java +++ b/src/main/java/org/sonar/plugins/github/MarkDownUtils.java @@ -47,6 +47,12 @@ public MarkDownUtils(Settings settings) { this.ruleUrlPrefix = baseUrl; } + public static String projectId(String projectId) { + StringBuilder sb = new StringBuilder(); + sb.append("[").append(projectId).append("]"); + return sb.toString(); + } + public String inlineIssue(Severity severity, String message, String ruleKey) { String ruleLink = getRuleLink(ruleKey); StringBuilder sb = new StringBuilder(); diff --git a/src/main/java/org/sonar/plugins/github/PullRequestFacade.java b/src/main/java/org/sonar/plugins/github/PullRequestFacade.java index 5e5ae9c..3e23ad9 100644 --- a/src/main/java/org/sonar/plugins/github/PullRequestFacade.java +++ b/src/main/java/org/sonar/plugins/github/PullRequestFacade.java @@ -31,6 +31,8 @@ import java.util.regex.Pattern; import javax.annotation.CheckForNull; import javax.annotation.Nullable; + +import org.apache.commons.lang.StringUtils; import org.kohsuke.github.GHCommitState; import org.kohsuke.github.GHCommitStatus; import org.kohsuke.github.GHIssueComment; @@ -72,6 +74,7 @@ public class PullRequestFacade { public PullRequestFacade(GitHubPluginConfiguration config) { this.config = config; + } public void init(int pullRequestNumber, File projectBaseDir) { @@ -136,6 +139,10 @@ private void loadExistingReviewComments() throws IOException { // Ignore comments from other users continue; } + if (!StringUtils.isEmpty(config.projectId()) && !comment.getBody().startsWith(MarkDownUtils.projectId(config.projectId()))) { + // Ignore comments that don't contain projectId + continue; + } if (!existingReviewCommentsByLocationByFile.containsKey(comment.getPath())) { existingReviewCommentsByLocationByFile.put(comment.getPath(), new HashMap()); } @@ -255,6 +262,10 @@ private boolean findAndDeleteOthers(@Nullable String markup) throws IOException boolean found = false; for (GHIssueComment comment : pr.listComments()) { if (myself.equals(comment.getUser().getLogin())) { + if (!StringUtils.isEmpty(config.projectId()) && !comment.getBody().startsWith(MarkDownUtils.projectId(config.projectId()))) { + // Ignore comments that don't contain projectId + continue; + } if (markup == null || found || !markup.equals(comment.getBody())) { comment.delete(); continue; diff --git a/src/main/java/org/sonar/plugins/github/PullRequestIssuePostJob.java b/src/main/java/org/sonar/plugins/github/PullRequestIssuePostJob.java index ffcc023..f0c367f 100644 --- a/src/main/java/org/sonar/plugins/github/PullRequestIssuePostJob.java +++ b/src/main/java/org/sonar/plugins/github/PullRequestIssuePostJob.java @@ -24,6 +24,7 @@ import java.util.Map; import java.util.stream.StreamSupport; import org.apache.commons.lang.StringUtils; + import org.kohsuke.github.GHCommitState; import org.sonar.api.batch.fs.InputComponent; import org.sonar.api.batch.fs.InputFile; @@ -61,7 +62,7 @@ public void describe(PostJobDescriptor descriptor) { @Override public void execute(PostJobContext context) { - GlobalReport report = new GlobalReport(markDownUtils, gitHubPluginConfiguration.tryReportIssuesInline()); + GlobalReport report = new GlobalReport(markDownUtils, gitHubPluginConfiguration.tryReportIssuesInline(), gitHubPluginConfiguration.projectId()); try { Map> commentsToBeAddedByLine = processIssues(report, context.issues()); @@ -117,7 +118,11 @@ private boolean tryReportInline(Map> comm } Map commentsByLine = commentToBeAddedByFileAndByLine.get(inputFile); if (!commentsByLine.containsKey(line)) { - commentsByLine.put(line, new StringBuilder()); + StringBuilder stringBuilder = new StringBuilder(); + if (!StringUtils.isEmpty(gitHubPluginConfiguration.projectId())) { + stringBuilder.append(markDownUtils.projectId(gitHubPluginConfiguration.projectId())).append("\n"); + } + commentsByLine.put(line, stringBuilder); } commentsByLine.get(line).append(markDownUtils.inlineIssue(issue.severity(), message, ruleKey)).append("\n"); return true; diff --git a/src/main/java/org/sonar/plugins/github/ReportBuilder.java b/src/main/java/org/sonar/plugins/github/ReportBuilder.java index f5cdc15..d2d32fc 100644 --- a/src/main/java/org/sonar/plugins/github/ReportBuilder.java +++ b/src/main/java/org/sonar/plugins/github/ReportBuilder.java @@ -24,6 +24,14 @@ import org.sonar.api.batch.rule.Severity; public interface ReportBuilder { + /** + * Append project id to the report. + * + * @param projectId Project id to append + * @return a reference to this object + */ + ReportBuilder appendProjectId(String projectId); + /** * Append an object to the report, using its toString() method. * diff --git a/src/test/java/org/sonar/plugins/github/GlobalReportTest.java b/src/test/java/org/sonar/plugins/github/GlobalReportTest.java index 007d3cf..b8f20ce 100644 --- a/src/test/java/org/sonar/plugins/github/GlobalReportTest.java +++ b/src/test/java/org/sonar/plugins/github/GlobalReportTest.java @@ -71,7 +71,7 @@ private PostJobIssue newMockedIssue(String componentKey, @CheckForNull DefaultIn @Test public void noIssues() { - GlobalReport globalReport = new GlobalReport(new MarkDownUtils(settings), true); + GlobalReport globalReport = new GlobalReport(new MarkDownUtils(settings), true, ""); String desiredMarkdown = "SonarQube analysis reported no issues."; @@ -80,9 +80,20 @@ public void noIssues() { assertThat(formattedGlobalReport).isEqualTo(desiredMarkdown); } + @Test + public void noIssuesWithProjectId() { + GlobalReport globalReport = new GlobalReport(new MarkDownUtils(settings), true, "project-id"); + + String desiredMarkdown = "[project-id] SonarQube analysis reported no issues."; + + String formattedGlobalReport = globalReport.formatForMarkdown(); + + assertThat(formattedGlobalReport).isEqualTo(desiredMarkdown); + } + @Test public void oneIssue() { - GlobalReport globalReport = new GlobalReport(new MarkDownUtils(settings), true); + GlobalReport globalReport = new GlobalReport(new MarkDownUtils(settings), true, ""); globalReport.process(newMockedIssue("component", null, null, Severity.INFO, true, "Issue", "rule"), GITHUB_URL, true); String desiredMarkdown = "SonarQube analysis reported 1 issue\n" + @@ -95,9 +106,24 @@ public void oneIssue() { assertThat(formattedGlobalReport).isEqualTo(desiredMarkdown); } + @Test + public void oneIssueWithProjectId() { + GlobalReport globalReport = new GlobalReport(new MarkDownUtils(settings), true, "project-id"); + globalReport.process(newMockedIssue("component", null, null, Severity.INFO, true, "Issue", "rule"), GITHUB_URL, true); + + String desiredMarkdown = "[project-id] SonarQube analysis reported 1 issue\n" + + "* ![INFO][INFO] 1 info\n" + + "\nWatch the comments in this conversation to review them.\n" + + "\n[INFO]: https://sonarsource.github.io/sonar-github/severity-info.png 'Severity: INFO'"; + + String formattedGlobalReport = globalReport.formatForMarkdown(); + + assertThat(formattedGlobalReport).isEqualTo(desiredMarkdown); + } + @Test public void oneIssueOnDir() { - GlobalReport globalReport = new GlobalReport(new MarkDownUtils(settings), true); + GlobalReport globalReport = new GlobalReport(new MarkDownUtils(settings), true, ""); globalReport.process(newMockedIssue("component0", null, null, Severity.INFO, true, "Issue0", "rule0"), null, false); String desiredMarkdown = "SonarQube analysis reported 1 issue\n\n" + @@ -113,7 +139,7 @@ public void oneIssueOnDir() { @Test public void shouldFormatIssuesForMarkdownNoInline() { - GlobalReport globalReport = new GlobalReport(new MarkDownUtils(settings), true); + GlobalReport globalReport = new GlobalReport(new MarkDownUtils(settings), true, ""); globalReport.process(newMockedIssue("component", null, null, Severity.INFO, true, "Issue", "rule"), GITHUB_URL, true); globalReport.process(newMockedIssue("component", null, null, Severity.MINOR, true, "Issue", "rule"), GITHUB_URL, true); globalReport.process(newMockedIssue("component", null, null, Severity.MAJOR, true, "Issue", "rule"), GITHUB_URL, true); @@ -141,7 +167,7 @@ public void shouldFormatIssuesForMarkdownNoInline() { @Test public void shouldFormatIssuesForMarkdownMixInlineGlobal() { - GlobalReport globalReport = new GlobalReport(new MarkDownUtils(settings), true); + GlobalReport globalReport = new GlobalReport(new MarkDownUtils(settings), true, ""); globalReport.process(newMockedIssue("component", null, null, Severity.INFO, true, "Issue 0", "rule0"), GITHUB_URL, true); globalReport.process(newMockedIssue("component", null, null, Severity.MINOR, true, "Issue 1", "rule1"), GITHUB_URL, false); globalReport.process(newMockedIssue("component", null, null, Severity.MAJOR, true, "Issue 2", "rule2"), GITHUB_URL, true); @@ -175,7 +201,7 @@ public void shouldFormatIssuesForMarkdownMixInlineGlobal() { @Test public void shouldFormatIssuesForMarkdownWhenInlineCommentsDisabled() { - GlobalReport globalReport = new GlobalReport(new MarkDownUtils(settings), false); + GlobalReport globalReport = new GlobalReport(new MarkDownUtils(settings), false, ""); globalReport.process(newMockedIssue("component", null, null, Severity.INFO, true, "Issue 0", "rule0"), GITHUB_URL, false); globalReport.process(newMockedIssue("component", null, null, Severity.MINOR, true, "Issue 1", "rule1"), GITHUB_URL, false); globalReport.process(newMockedIssue("component", null, null, Severity.MAJOR, true, "Issue 2", "rule2"), GITHUB_URL, false); @@ -206,7 +232,7 @@ public void shouldFormatIssuesForMarkdownWhenInlineCommentsDisabled() { @Test public void shouldFormatIssuesForMarkdownWhenInlineCommentsDisabledAndLimitReached() { - GlobalReport globalReport = new GlobalReport(new MarkDownUtils(settings), false, 4); + GlobalReport globalReport = new GlobalReport(new MarkDownUtils(settings), false, 4, ""); globalReport.process(newMockedIssue("component", null, null, Severity.INFO, true, "Issue 0", "rule0"), GITHUB_URL, false); globalReport.process(newMockedIssue("component", null, null, Severity.MINOR, true, "Issue 1", "rule1"), GITHUB_URL, false); globalReport.process(newMockedIssue("component", null, null, Severity.MAJOR, true, "Issue 2", "rule2"), GITHUB_URL, false); @@ -241,7 +267,7 @@ public void shouldFormatIssuesForMarkdownWhenInlineCommentsDisabledAndLimitReach @Test public void shouldLimitGlobalIssues() { - GlobalReport globalReport = new GlobalReport(new MarkDownUtils(settings), true); + GlobalReport globalReport = new GlobalReport(new MarkDownUtils(settings), true, ""); for (int i = 0; i < 17; i++) { globalReport.process(newMockedIssue("component", null, null, Severity.MAJOR, true, "Issue number:" + i, "rule" + i), GITHUB_URL + "/File.java#L" + i, false); } @@ -280,7 +306,7 @@ public void shouldLimitGlobalIssues() { @Test public void shouldLimitGlobalIssuesWhenInlineCommentsDisabled() { - GlobalReport globalReport = new GlobalReport(new MarkDownUtils(settings), false); + GlobalReport globalReport = new GlobalReport(new MarkDownUtils(settings), false, ""); for (int i = 0; i < 17; i++) { globalReport.process(newMockedIssue("component", null, null, Severity.MAJOR, true, "Issue number:" + i, "rule" + i), GITHUB_URL + "/File.java#L" + i, false); } diff --git a/src/test/java/org/sonar/plugins/github/PullRequestIssuePostJobTest.java b/src/test/java/org/sonar/plugins/github/PullRequestIssuePostJobTest.java index c9278f8..38931fa 100644 --- a/src/test/java/org/sonar/plugins/github/PullRequestIssuePostJobTest.java +++ b/src/test/java/org/sonar/plugins/github/PullRequestIssuePostJobTest.java @@ -56,17 +56,25 @@ public class PullRequestIssuePostJobTest { @Before public void prepare() throws Exception { pullRequestFacade = mock(PullRequestFacade.class); - Settings settings = new Settings(new PropertyDefinitions(PropertyDefinition.builder(CoreProperties.SERVER_BASE_URL) - .name("Server base URL") - .description("HTTP URL of this SonarQube server, such as http://yourhost.yourdomain/sonar. This value is used i.e. to create links in emails.") - .category(CoreProperties.CATEGORY_GENERAL) - .defaultValue(CoreProperties.SERVER_BASE_URL_DEFAULT_VALUE) - .build())); + Settings settings = new Settings(new PropertyDefinitions( + PropertyDefinition.builder(CoreProperties.SERVER_BASE_URL) + .name("Server base URL") + .description("HTTP URL of this SonarQube server, such as http://yourhost.yourdomain/sonar. This value is used i.e. to create links in emails.") + .category(CoreProperties.CATEGORY_GENERAL) + .defaultValue(CoreProperties.SERVER_BASE_URL_DEFAULT_VALUE) + .build(), + PropertyDefinition.builder(GitHubPlugin.GITHUB_PROJECT_ID) + .name("Project Id") + .description("Project Id") + .category(CoreProperties.CATEGORY_GENERAL) + .defaultValue("") + .build())); GitHubPluginConfiguration config = new GitHubPluginConfiguration(settings, new System2()); context = mock(PostJobContext.class); settings.setProperty("sonar.host.url", "http://192.168.0.1"); settings.setProperty(CoreProperties.SERVER_BASE_URL, "http://myserver"); + settings.setProperty(GitHubPlugin.GITHUB_PROJECT_ID, "project-id"); pullRequestIssuePostJob = new PullRequestIssuePostJob(config, pullRequestFacade, new MarkDownUtils(settings)); } From 7f4c9bfcd4a5ffc67d19c9cc59d4ce8c389dac4e Mon Sep 17 00:00:00 2001 From: TK-999 Date: Mon, 23 Oct 2017 12:47:19 +0200 Subject: [PATCH 2/2] Use project key in review comments --- .../sonar/plugins/github/GitHubPlugin.java | 9 +-- .../github/GitHubPluginConfiguration.java | 5 +- .../sonar/plugins/github/GlobalReport.java | 23 ++++---- .../plugins/github/MarkDownReportBuilder.java | 6 +- .../sonar/plugins/github/MarkDownUtils.java | 2 +- .../plugins/github/PullRequestFacade.java | 6 +- .../github/PullRequestIssuePostJob.java | 30 ++++++---- .../plugins/github/GlobalReportTest.java | 58 +++++++------------ .../github/PullRequestIssuePostJobTest.java | 4 +- 9 files changed, 65 insertions(+), 78 deletions(-) diff --git a/src/main/java/org/sonar/plugins/github/GitHubPlugin.java b/src/main/java/org/sonar/plugins/github/GitHubPlugin.java index 53d34ca..ff4d840 100644 --- a/src/main/java/org/sonar/plugins/github/GitHubPlugin.java +++ b/src/main/java/org/sonar/plugins/github/GitHubPlugin.java @@ -59,13 +59,7 @@ description = "Issues will not be reported as inline comments but only in the global summary comment", project = true, global = true, - type = PropertyType.BOOLEAN), - @Property( - key = GitHubPlugin.GITHUB_PROJECT_ID, - name = "Project id", - description = "Project id used to distinguish between comments from different SonarQube projects", - project = true, - global = true) + type = PropertyType.BOOLEAN) }) public class GitHubPlugin implements Plugin { @@ -74,7 +68,6 @@ public class GitHubPlugin implements Plugin { public static final String GITHUB_REPO = "sonar.github.repository"; public static final String GITHUB_PULL_REQUEST = "sonar.github.pullRequest"; public static final String GITHUB_DISABLE_INLINE_COMMENTS = "sonar.github.disableInlineComments"; - public static final String GITHUB_PROJECT_ID = "sonar.github.projectId"; @Override diff --git a/src/main/java/org/sonar/plugins/github/GitHubPluginConfiguration.java b/src/main/java/org/sonar/plugins/github/GitHubPluginConfiguration.java index 3a634b8..b77928f 100644 --- a/src/main/java/org/sonar/plugins/github/GitHubPluginConfiguration.java +++ b/src/main/java/org/sonar/plugins/github/GitHubPluginConfiguration.java @@ -28,6 +28,7 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; import javax.annotation.CheckForNull; +import javax.annotation.Nullable; import org.sonar.api.CoreProperties; import org.sonar.api.batch.BatchSide; import org.sonar.api.batch.InstantiationStrategy; @@ -136,8 +137,8 @@ public boolean tryReportIssuesInline() { return !settings.getBoolean(GitHubPlugin.GITHUB_DISABLE_INLINE_COMMENTS); } - public String projectId() { - return settings.getString(GitHubPlugin.GITHUB_PROJECT_ID); + public @Nullable String projectKey() { + return settings.getString("sonar.projectKey"); } /** diff --git a/src/main/java/org/sonar/plugins/github/GlobalReport.java b/src/main/java/org/sonar/plugins/github/GlobalReport.java index b8d9df2..9f7a26f 100644 --- a/src/main/java/org/sonar/plugins/github/GlobalReport.java +++ b/src/main/java/org/sonar/plugins/github/GlobalReport.java @@ -32,17 +32,17 @@ public class GlobalReport { private int[] newIssuesBySeverity = new int[Severity.values().length]; private int extraIssueCount = 0; private int maxGlobalReportedIssues; - private String projectId; + private String projectKey; private final ReportBuilder builder; - public GlobalReport(MarkDownUtils markDownUtils, boolean tryReportIssuesInline, String projectId) { - this(markDownUtils, tryReportIssuesInline, GitHubPluginConfiguration.MAX_GLOBAL_ISSUES, projectId); + public GlobalReport(MarkDownUtils markDownUtils, boolean tryReportIssuesInline, @Nullable String projectKey) { + this(markDownUtils, tryReportIssuesInline, GitHubPluginConfiguration.MAX_GLOBAL_ISSUES, projectKey); } - public GlobalReport(MarkDownUtils markDownUtils, boolean tryReportIssuesInline, int maxGlobalReportedIssues, String projectId) { + public GlobalReport(MarkDownUtils markDownUtils, boolean tryReportIssuesInline, int maxGlobalReportedIssues, @Nullable String projectKey) { this.tryReportIssuesInline = tryReportIssuesInline; this.maxGlobalReportedIssues = maxGlobalReportedIssues; - this.projectId = projectId; + this.projectKey = projectKey; this.builder = new MarkDownReportBuilder(markDownUtils); } @@ -55,15 +55,14 @@ public String formatForMarkdown() { boolean hasInlineIssues = newIssues > extraIssueCount; boolean extraIssuesTruncated = extraIssueCount > maxGlobalReportedIssues; - if (!StringUtils.isEmpty(projectId)) { - builder.appendProjectId(projectId).append(" "); - } + if (newIssues == 0) { - builder.append("SonarQube analysis reported no issues."); + builder.append("SonarQube analysis reported no issues.").appendProjectId(projectKey); return builder.toString(); - } else { - builder.append("SonarQube analysis reported ").append(newIssues).append(" issue").append(newIssues > 1 ? "s" : "").append("\n"); } + + builder.append("SonarQube analysis reported ").append(newIssues).append(" issue").append(newIssues > 1 ? "s" : "").append("\n"); + if (hasInlineIssues || extraIssuesTruncated) { appendSummaryBySeverity(builder); } @@ -75,7 +74,7 @@ public String formatForMarkdown() { appendExtraIssues(builder, hasInlineIssues, extraIssuesTruncated); } - return builder.toString(); + return builder.appendProjectId(projectKey).toString(); } private void appendExtraIssues(ReportBuilder builder, boolean hasInlineIssues, boolean extraIssuesTruncated) { diff --git a/src/main/java/org/sonar/plugins/github/MarkDownReportBuilder.java b/src/main/java/org/sonar/plugins/github/MarkDownReportBuilder.java index f06b1ce..614760c 100644 --- a/src/main/java/org/sonar/plugins/github/MarkDownReportBuilder.java +++ b/src/main/java/org/sonar/plugins/github/MarkDownReportBuilder.java @@ -23,6 +23,7 @@ import java.util.List; import java.util.Set; import java.util.TreeSet; +import org.apache.commons.lang.StringUtils; import org.sonar.api.batch.postjob.issue.PostJobIssue; import org.sonar.api.batch.rule.Severity; @@ -52,7 +53,10 @@ private IssueHolder(PostJobIssue issue, String gitHubUrl) { @Override public ReportBuilder appendProjectId(String projectId) { - sb.append(markDownUtils.projectId(projectId)); + if (!StringUtils.isEmpty(projectId)) { + sb.append(MarkDownUtils.projectId(projectId)); + } + return this; } diff --git a/src/main/java/org/sonar/plugins/github/MarkDownUtils.java b/src/main/java/org/sonar/plugins/github/MarkDownUtils.java index df6ea75..11be936 100644 --- a/src/main/java/org/sonar/plugins/github/MarkDownUtils.java +++ b/src/main/java/org/sonar/plugins/github/MarkDownUtils.java @@ -49,7 +49,7 @@ public MarkDownUtils(Settings settings) { public static String projectId(String projectId) { StringBuilder sb = new StringBuilder(); - sb.append("[").append(projectId).append("]"); + sb.append("\n[project-id]: ").append(projectId); return sb.toString(); } diff --git a/src/main/java/org/sonar/plugins/github/PullRequestFacade.java b/src/main/java/org/sonar/plugins/github/PullRequestFacade.java index 3e23ad9..18891ee 100644 --- a/src/main/java/org/sonar/plugins/github/PullRequestFacade.java +++ b/src/main/java/org/sonar/plugins/github/PullRequestFacade.java @@ -139,12 +139,12 @@ private void loadExistingReviewComments() throws IOException { // Ignore comments from other users continue; } - if (!StringUtils.isEmpty(config.projectId()) && !comment.getBody().startsWith(MarkDownUtils.projectId(config.projectId()))) { + if (!StringUtils.isEmpty(config.projectKey()) && !comment.getBody().contains(MarkDownUtils.projectId(config.projectKey()))) { // Ignore comments that don't contain projectId continue; } if (!existingReviewCommentsByLocationByFile.containsKey(comment.getPath())) { - existingReviewCommentsByLocationByFile.put(comment.getPath(), new HashMap()); + existingReviewCommentsByLocationByFile.put(comment.getPath(), new HashMap<>()); } // By default all previous comments will be marked for deletion reviewCommentToBeDeletedById.put(comment.getId(), comment); @@ -262,7 +262,7 @@ private boolean findAndDeleteOthers(@Nullable String markup) throws IOException boolean found = false; for (GHIssueComment comment : pr.listComments()) { if (myself.equals(comment.getUser().getLogin())) { - if (!StringUtils.isEmpty(config.projectId()) && !comment.getBody().startsWith(MarkDownUtils.projectId(config.projectId()))) { + if (!StringUtils.isEmpty(config.projectKey()) && !comment.getBody().contains(MarkDownUtils.projectId(config.projectKey()))) { // Ignore comments that don't contain projectId continue; } diff --git a/src/main/java/org/sonar/plugins/github/PullRequestIssuePostJob.java b/src/main/java/org/sonar/plugins/github/PullRequestIssuePostJob.java index f0c367f..f46758e 100644 --- a/src/main/java/org/sonar/plugins/github/PullRequestIssuePostJob.java +++ b/src/main/java/org/sonar/plugins/github/PullRequestIssuePostJob.java @@ -23,6 +23,7 @@ import java.util.HashMap; import java.util.Map; import java.util.stream.StreamSupport; +import javax.annotation.Nullable; import org.apache.commons.lang.StringUtils; import org.kohsuke.github.GHCommitState; @@ -62,9 +63,11 @@ public void describe(PostJobDescriptor descriptor) { @Override public void execute(PostJobContext context) { - GlobalReport report = new GlobalReport(markDownUtils, gitHubPluginConfiguration.tryReportIssuesInline(), gitHubPluginConfiguration.projectId()); + String projectKey = gitHubPluginConfiguration.projectKey(); + + GlobalReport report = new GlobalReport(markDownUtils, gitHubPluginConfiguration.tryReportIssuesInline(), projectKey); try { - Map> commentsToBeAddedByLine = processIssues(report, context.issues()); + Map> commentsToBeAddedByLine = processIssues(report, context.issues(), projectKey); updateReviewComments(commentsToBeAddedByLine); @@ -79,7 +82,8 @@ public void execute(PostJobContext context) { } } - private Map> processIssues(GlobalReport report, Iterable issues) { + private Map> processIssues( + GlobalReport report, Iterable issues, @Nullable String projectKey) { Map> commentToBeAddedByFileAndByLine = new HashMap<>(); StreamSupport.stream(issues.spliterator(), false) @@ -92,21 +96,21 @@ private Map> processIssues(GlobalReport r pullRequestFacade.hasFile((InputFile) inputComponent); }) .sorted(ISSUE_COMPARATOR) - .forEach(i -> processIssue(report, commentToBeAddedByFileAndByLine, i)); + .forEach(i -> processIssue(report, commentToBeAddedByFileAndByLine, i, projectKey)); return commentToBeAddedByFileAndByLine; } - private void processIssue(GlobalReport report, Map> commentToBeAddedByFileAndByLine, PostJobIssue issue) { + private void processIssue(GlobalReport report, Map> commentToBeAddedByFileAndByLine, PostJobIssue issue, @Nullable String projectKey) { boolean reportedInline = false; InputComponent inputComponent = issue.inputComponent(); if (gitHubPluginConfiguration.tryReportIssuesInline() && inputComponent != null && inputComponent.isFile()) { - reportedInline = tryReportInline(commentToBeAddedByFileAndByLine, issue, (InputFile) inputComponent); + reportedInline = tryReportInline(commentToBeAddedByFileAndByLine, issue, (InputFile) inputComponent, projectKey); } report.process(issue, pullRequestFacade.getGithubUrl(inputComponent, issue.line()), reportedInline); } - private boolean tryReportInline(Map> commentToBeAddedByFileAndByLine, PostJobIssue issue, InputFile inputFile) { + private boolean tryReportInline(Map> commentToBeAddedByFileAndByLine, PostJobIssue issue, InputFile inputFile, @Nullable String projectKey) { Integer lineOrNull = issue.line(); if (lineOrNull != null) { int line = lineOrNull.intValue(); @@ -114,17 +118,21 @@ private boolean tryReportInline(Map> comm String message = issue.message(); String ruleKey = issue.ruleKey().toString(); if (!commentToBeAddedByFileAndByLine.containsKey(inputFile)) { - commentToBeAddedByFileAndByLine.put(inputFile, new HashMap()); + commentToBeAddedByFileAndByLine.put(inputFile, new HashMap<>()); } Map commentsByLine = commentToBeAddedByFileAndByLine.get(inputFile); + if (!commentsByLine.containsKey(line)) { StringBuilder stringBuilder = new StringBuilder(); - if (!StringUtils.isEmpty(gitHubPluginConfiguration.projectId())) { - stringBuilder.append(markDownUtils.projectId(gitHubPluginConfiguration.projectId())).append("\n"); - } commentsByLine.put(line, stringBuilder); } + commentsByLine.get(line).append(markDownUtils.inlineIssue(issue.severity(), message, ruleKey)).append("\n"); + + if (!StringUtils.isEmpty(projectKey)) { + commentsByLine.get(line).append(MarkDownUtils.projectId(projectKey)).append("\n"); + } + return true; } } diff --git a/src/test/java/org/sonar/plugins/github/GlobalReportTest.java b/src/test/java/org/sonar/plugins/github/GlobalReportTest.java index b8f20ce..b6e94e6 100644 --- a/src/test/java/org/sonar/plugins/github/GlobalReportTest.java +++ b/src/test/java/org/sonar/plugins/github/GlobalReportTest.java @@ -36,7 +36,7 @@ import static org.mockito.Mockito.when; public class GlobalReportTest { - + private static final String PROJECT_KEY = "test.key"; private static final String GITHUB_URL = "https://github.com/SonarSource/sonar-github"; private Settings settings; @@ -69,37 +69,11 @@ private PostJobIssue newMockedIssue(String componentKey, @CheckForNull DefaultIn return issue; } - @Test - public void noIssues() { - GlobalReport globalReport = new GlobalReport(new MarkDownUtils(settings), true, ""); - - String desiredMarkdown = "SonarQube analysis reported no issues."; - - String formattedGlobalReport = globalReport.formatForMarkdown(); - - assertThat(formattedGlobalReport).isEqualTo(desiredMarkdown); - } - @Test public void noIssuesWithProjectId() { - GlobalReport globalReport = new GlobalReport(new MarkDownUtils(settings), true, "project-id"); + GlobalReport globalReport = new GlobalReport(new MarkDownUtils(settings), true, PROJECT_KEY); - String desiredMarkdown = "[project-id] SonarQube analysis reported no issues."; - - String formattedGlobalReport = globalReport.formatForMarkdown(); - - assertThat(formattedGlobalReport).isEqualTo(desiredMarkdown); - } - - @Test - public void oneIssue() { - GlobalReport globalReport = new GlobalReport(new MarkDownUtils(settings), true, ""); - globalReport.process(newMockedIssue("component", null, null, Severity.INFO, true, "Issue", "rule"), GITHUB_URL, true); - - String desiredMarkdown = "SonarQube analysis reported 1 issue\n" + - "* ![INFO][INFO] 1 info\n" + - "\nWatch the comments in this conversation to review them.\n" + - "\n[INFO]: https://sonarsource.github.io/sonar-github/severity-info.png 'Severity: INFO'"; + String desiredMarkdown = "SonarQube analysis reported no issues.\n[project-id]: test.key"; String formattedGlobalReport = globalReport.formatForMarkdown(); @@ -108,12 +82,13 @@ public void oneIssue() { @Test public void oneIssueWithProjectId() { - GlobalReport globalReport = new GlobalReport(new MarkDownUtils(settings), true, "project-id"); + GlobalReport globalReport = new GlobalReport(new MarkDownUtils(settings), true, PROJECT_KEY); globalReport.process(newMockedIssue("component", null, null, Severity.INFO, true, "Issue", "rule"), GITHUB_URL, true); - String desiredMarkdown = "[project-id] SonarQube analysis reported 1 issue\n" + + String desiredMarkdown = "SonarQube analysis reported 1 issue\n" + "* ![INFO][INFO] 1 info\n" + "\nWatch the comments in this conversation to review them.\n" + + "\n[project-id]: test.key" + "\n[INFO]: https://sonarsource.github.io/sonar-github/severity-info.png 'Severity: INFO'"; String formattedGlobalReport = globalReport.formatForMarkdown(); @@ -123,13 +98,14 @@ public void oneIssueWithProjectId() { @Test public void oneIssueOnDir() { - GlobalReport globalReport = new GlobalReport(new MarkDownUtils(settings), true, ""); + GlobalReport globalReport = new GlobalReport(new MarkDownUtils(settings), true, PROJECT_KEY); globalReport.process(newMockedIssue("component0", null, null, Severity.INFO, true, "Issue0", "rule0"), null, false); String desiredMarkdown = "SonarQube analysis reported 1 issue\n\n" + "Note: The following issues were found on lines that were not modified in the pull request. Because these issues can't be reported as line comments, they are summarized here:\n\n" + "1. ![INFO][INFO] component0: Issue0 [![rule](https://sonarsource.github.io/sonar-github/rule.png)](http://myserver/coding_rules#rule_key=repo%3Arule0)\n" + + "\n[project-id]: test.key" + "\n[INFO]: https://sonarsource.github.io/sonar-github/severity-info.png 'Severity: INFO'"; String formattedGlobalReport = globalReport.formatForMarkdown(); @@ -139,7 +115,7 @@ public void oneIssueOnDir() { @Test public void shouldFormatIssuesForMarkdownNoInline() { - GlobalReport globalReport = new GlobalReport(new MarkDownUtils(settings), true, ""); + GlobalReport globalReport = new GlobalReport(new MarkDownUtils(settings), true, PROJECT_KEY); globalReport.process(newMockedIssue("component", null, null, Severity.INFO, true, "Issue", "rule"), GITHUB_URL, true); globalReport.process(newMockedIssue("component", null, null, Severity.MINOR, true, "Issue", "rule"), GITHUB_URL, true); globalReport.process(newMockedIssue("component", null, null, Severity.MAJOR, true, "Issue", "rule"), GITHUB_URL, true); @@ -154,6 +130,7 @@ public void shouldFormatIssuesForMarkdownNoInline() { "* ![INFO][INFO] 1 info\n" + "\nWatch the comments in this conversation to review them.\n" + "\n" + + "[project-id]: test.key\n" + "[BLOCKER]: https://sonarsource.github.io/sonar-github/severity-blocker.png 'Severity: BLOCKER'\n" + "[CRITICAL]: https://sonarsource.github.io/sonar-github/severity-critical.png 'Severity: CRITICAL'\n" + "[INFO]: https://sonarsource.github.io/sonar-github/severity-info.png 'Severity: INFO'\n" @@ -167,7 +144,7 @@ public void shouldFormatIssuesForMarkdownNoInline() { @Test public void shouldFormatIssuesForMarkdownMixInlineGlobal() { - GlobalReport globalReport = new GlobalReport(new MarkDownUtils(settings), true, ""); + GlobalReport globalReport = new GlobalReport(new MarkDownUtils(settings), true, PROJECT_KEY); globalReport.process(newMockedIssue("component", null, null, Severity.INFO, true, "Issue 0", "rule0"), GITHUB_URL, true); globalReport.process(newMockedIssue("component", null, null, Severity.MINOR, true, "Issue 1", "rule1"), GITHUB_URL, false); globalReport.process(newMockedIssue("component", null, null, Severity.MAJOR, true, "Issue 2", "rule2"), GITHUB_URL, true); @@ -188,6 +165,7 @@ public void shouldFormatIssuesForMarkdownMixInlineGlobal() { + "1. ![CRITICAL][CRITICAL] [sonar-github](https://github.com/SonarSource/sonar-github): Issue 3 [![rule](https://sonarsource.github.io/sonar-github/rule.png)](http://myserver/coding_rules#rule_key=repo%3Arule3)\n" + "\n" + + "[project-id]: test.key\n" + "[BLOCKER]: https://sonarsource.github.io/sonar-github/severity-blocker.png 'Severity: BLOCKER'\n" + "[CRITICAL]: https://sonarsource.github.io/sonar-github/severity-critical.png 'Severity: CRITICAL'\n" + "[INFO]: https://sonarsource.github.io/sonar-github/severity-info.png 'Severity: INFO'\n" @@ -201,7 +179,7 @@ public void shouldFormatIssuesForMarkdownMixInlineGlobal() { @Test public void shouldFormatIssuesForMarkdownWhenInlineCommentsDisabled() { - GlobalReport globalReport = new GlobalReport(new MarkDownUtils(settings), false, ""); + GlobalReport globalReport = new GlobalReport(new MarkDownUtils(settings), false, PROJECT_KEY); globalReport.process(newMockedIssue("component", null, null, Severity.INFO, true, "Issue 0", "rule0"), GITHUB_URL, false); globalReport.process(newMockedIssue("component", null, null, Severity.MINOR, true, "Issue 1", "rule1"), GITHUB_URL, false); globalReport.process(newMockedIssue("component", null, null, Severity.MAJOR, true, "Issue 2", "rule2"), GITHUB_URL, false); @@ -219,6 +197,7 @@ public void shouldFormatIssuesForMarkdownWhenInlineCommentsDisabled() { + "1. ![BLOCKER][BLOCKER] [sonar-github](https://github.com/SonarSource/sonar-github): Issue 4 [![rule](https://sonarsource.github.io/sonar-github/rule.png)](http://myserver/coding_rules#rule_key=repo%3Arule4)\n" + "\n" + + "[project-id]: test.key\n" + "[BLOCKER]: https://sonarsource.github.io/sonar-github/severity-blocker.png 'Severity: BLOCKER'\n" + "[CRITICAL]: https://sonarsource.github.io/sonar-github/severity-critical.png 'Severity: CRITICAL'\n" + "[INFO]: https://sonarsource.github.io/sonar-github/severity-info.png 'Severity: INFO'\n" @@ -232,7 +211,7 @@ public void shouldFormatIssuesForMarkdownWhenInlineCommentsDisabled() { @Test public void shouldFormatIssuesForMarkdownWhenInlineCommentsDisabledAndLimitReached() { - GlobalReport globalReport = new GlobalReport(new MarkDownUtils(settings), false, 4, ""); + GlobalReport globalReport = new GlobalReport(new MarkDownUtils(settings), false, 4, PROJECT_KEY); globalReport.process(newMockedIssue("component", null, null, Severity.INFO, true, "Issue 0", "rule0"), GITHUB_URL, false); globalReport.process(newMockedIssue("component", null, null, Severity.MINOR, true, "Issue 1", "rule1"), GITHUB_URL, false); globalReport.process(newMockedIssue("component", null, null, Severity.MAJOR, true, "Issue 2", "rule2"), GITHUB_URL, false); @@ -254,6 +233,7 @@ public void shouldFormatIssuesForMarkdownWhenInlineCommentsDisabledAndLimitReach + "1. ![CRITICAL][CRITICAL] [sonar-github](https://github.com/SonarSource/sonar-github): Issue 3 [![rule](https://sonarsource.github.io/sonar-github/rule.png)](http://myserver/coding_rules#rule_key=repo%3Arule3)\n" + "\n" + + "[project-id]: test.key\n" + "[BLOCKER]: https://sonarsource.github.io/sonar-github/severity-blocker.png 'Severity: BLOCKER'\n" + "[CRITICAL]: https://sonarsource.github.io/sonar-github/severity-critical.png 'Severity: CRITICAL'\n" + "[INFO]: https://sonarsource.github.io/sonar-github/severity-info.png 'Severity: INFO'\n" @@ -267,7 +247,7 @@ public void shouldFormatIssuesForMarkdownWhenInlineCommentsDisabledAndLimitReach @Test public void shouldLimitGlobalIssues() { - GlobalReport globalReport = new GlobalReport(new MarkDownUtils(settings), true, ""); + GlobalReport globalReport = new GlobalReport(new MarkDownUtils(settings), true, PROJECT_KEY); for (int i = 0; i < 17; i++) { globalReport.process(newMockedIssue("component", null, null, Severity.MAJOR, true, "Issue number:" + i, "rule" + i), GITHUB_URL + "/File.java#L" + i, false); } @@ -297,6 +277,7 @@ public void shouldLimitGlobalIssues() { + "1. ![MAJOR][MAJOR] [File.java#L9](https://github.com/SonarSource/sonar-github/File.java#L9): Issue number:9 [![rule](https://sonarsource.github.io/sonar-github/rule.png)](http://myserver/coding_rules#rule_key=repo%3Arule9)\n" + "\n" + + "[project-id]: test.key\n" + "[MAJOR]: https://sonarsource.github.io/sonar-github/severity-major.png 'Severity: MAJOR'"; String formattedGlobalReport = globalReport.formatForMarkdown(); @@ -306,7 +287,7 @@ public void shouldLimitGlobalIssues() { @Test public void shouldLimitGlobalIssuesWhenInlineCommentsDisabled() { - GlobalReport globalReport = new GlobalReport(new MarkDownUtils(settings), false, ""); + GlobalReport globalReport = new GlobalReport(new MarkDownUtils(settings), false, PROJECT_KEY); for (int i = 0; i < 17; i++) { globalReport.process(newMockedIssue("component", null, null, Severity.MAJOR, true, "Issue number:" + i, "rule" + i), GITHUB_URL + "/File.java#L" + i, false); } @@ -334,6 +315,7 @@ public void shouldLimitGlobalIssuesWhenInlineCommentsDisabled() { + "1. ![MAJOR][MAJOR] [File.java#L9](https://github.com/SonarSource/sonar-github/File.java#L9): Issue number:9 [![rule](https://sonarsource.github.io/sonar-github/rule.png)](http://myserver/coding_rules#rule_key=repo%3Arule9)\n" + "\n" + + "[project-id]: test.key\n" + "[MAJOR]: https://sonarsource.github.io/sonar-github/severity-major.png 'Severity: MAJOR'"; String formattedGlobalReport = globalReport.formatForMarkdown(); diff --git a/src/test/java/org/sonar/plugins/github/PullRequestIssuePostJobTest.java b/src/test/java/org/sonar/plugins/github/PullRequestIssuePostJobTest.java index 38931fa..b40dfb4 100644 --- a/src/test/java/org/sonar/plugins/github/PullRequestIssuePostJobTest.java +++ b/src/test/java/org/sonar/plugins/github/PullRequestIssuePostJobTest.java @@ -63,7 +63,7 @@ public void prepare() throws Exception { .category(CoreProperties.CATEGORY_GENERAL) .defaultValue(CoreProperties.SERVER_BASE_URL_DEFAULT_VALUE) .build(), - PropertyDefinition.builder(GitHubPlugin.GITHUB_PROJECT_ID) + PropertyDefinition.builder(CoreProperties.PROJECT_KEY_PROPERTY) .name("Project Id") .description("Project Id") .category(CoreProperties.CATEGORY_GENERAL) @@ -74,7 +74,7 @@ public void prepare() throws Exception { settings.setProperty("sonar.host.url", "http://192.168.0.1"); settings.setProperty(CoreProperties.SERVER_BASE_URL, "http://myserver"); - settings.setProperty(GitHubPlugin.GITHUB_PROJECT_ID, "project-id"); + settings.setProperty(CoreProperties.PROJECT_KEY_PROPERTY, "project-id"); pullRequestIssuePostJob = new PullRequestIssuePostJob(config, pullRequestFacade, new MarkDownUtils(settings)); }