Skip to content

Commit

Permalink
#377: Allow decorating Gitlab Mono Repository Merge Requests
Browse files Browse the repository at this point in the history 
The Gitlab decorator treated any comment posted by the current Gitlab user as having been submitted for the current Sonarqube project, and therefore resolved any threads that no active issue could be found for in the current analysis. This prevented the decorator being used for a mono-repo as issues from decorations for other projects based in the repo would be resolved as subsequent projects completed their analysis.

The decorator has been changed to include the project ID in the details extracted from the existing comments, and to filter out any comments with project keys that do not match the project key the analysis is being performed for, before closing any remaining discussions, so that comments from other Sonarqube projects are not prematurely closed.
  • Loading branch information
@mc1arke
mc1arke committed Jul 10, 2021
1 parent 9d83cbe commit aa61c11
Show file tree
Hide file tree
Showing 6 changed files with 130 additions and 41 deletions.
2 changes: 1 addition & 1 deletion build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ dependencies {
compileOnly fileTree(dir: sonarLibraries, include: '**/*.jar', exclude: 'extensions/*.jar')
testCompile fileTree(dir: sonarLibraries, include: '**/*.jar', exclude: 'extensions/*.jar')
testCompile group: 'org.mockito', name: 'mockito-core', version: '3.1.0'
testCompile group: 'org.assertj', name: 'assertj-core', version: '3.13.2'
testCompile group: 'org.assertj', name: 'assertj-core', version: '3.20.2'
testCompile group: 'com.github.tomakehurst', name: 'wiremock', version: '2.24.1'
zip "sonarqube:sonarqube:${sonarqubeVersion}@zip"
compile 'org.bouncycastle:bcpkix-jdk15on:1.62'
Expand Down
46 changes: 40 additions & 6 deletions src/main/java/com/github/mc1arke/sonarqube/plugin/ce/pullrequest/AnalysisDetails.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,21 +127,34 @@ public String getIssueUrl(PostAnalysisIssueVisitor.LightIssue issue) {
}
}

public Optional<String> parseIssueIdFromUrl(String issueUrl) {
public Optional<ProjectIssueIdentifier> parseIssueIdFromUrl(String issueUrl) {
URI url = URI.create(issueUrl);
List<NameValuePair> parameters = URLEncodedUtils.parse(url, StandardCharsets.UTF_8);
Optional<String> optionalProjectId = parameters.stream()
.filter(parameter -> "id".equals(parameter.getName()))
.map(NameValuePair::getValue)
.findFirst();

if (!optionalProjectId.isPresent()) {
return Optional.empty();
}

String projectId = optionalProjectId.get();

if (url.getPath().endsWith("/dashboard")) {
return Optional.of("decorator-summary-comment");
return Optional.of(new ProjectIssueIdentifier(projectId, "decorator-summary-comment"));
} else if (url.getPath().endsWith("security_hotspots")) {
return parameters.stream()
.filter(parameter -> "hotspots".equals(parameter.getName()))
.map(NameValuePair::getValue)
.findFirst();
.findFirst()
.map(issueId -> new ProjectIssueIdentifier(projectId, issueId));
} else {
return parameters.stream()
.filter(parameter -> "issues".equals(parameter.getName()))
.map(NameValuePair::getValue)
.findFirst();
.findFirst()
.map(issueId -> new ProjectIssueIdentifier(projectId, issueId));
}
}

Expand Down Expand Up @@ -237,7 +250,8 @@ public String createAnalysisSummary(FormatterFactory formatterFactory) {
.orElse("No duplication information") + " (" +
decimalFormat.format(duplications) +
"% Estimated after merge)"))),
new Link(getDashboardUrl(), new Text("View in SonarQube")));
new Paragraph(new Text(String.format("**Project ID:** %s", project.getKey()))),
new Paragraph(new Link(getDashboardUrl(), new Text("View in SonarQube"))));

return formatterFactory.documentFormatter().format(document, formatterFactory);
}
Expand All @@ -259,7 +273,8 @@ public String createAnalysisIssueSummary(PostAnalysisIssueVisitor.ComponentIssue
new Paragraph(new Text(String.format("**Message:** %s", issue.getMessage()))),
effortNode,
resolutionNode,
new Link(getIssueUrl(issue), new Text("View in SonarQube"))
new Paragraph(new Text(String.format("**Project ID:** %s **Issue ID:** %s", project.getKey(), issue.key()))),
new Paragraph(new Link(getIssueUrl(issue), new Text("View in SonarQube")))
);
return formatterFactory.documentFormatter().format(document, formatterFactory);
}
Expand Down Expand Up @@ -466,4 +481,23 @@ private String getImageName() {
}
}

public static class ProjectIssueIdentifier {

private final String projectKey;
private final String issueKey;

public ProjectIssueIdentifier(String projectKey, String issueKey) {
this.projectKey = projectKey;
this.issueKey = issueKey;
}

public String getProjectKey() {
return projectKey;
}

public String getIssueKey() {
return issueKey;
}
}

}
43 changes: 27 additions & 16 deletions ...om/github/mc1arke/sonarqube/plugin/ce/pullrequest/gitlab/GitlabMergeRequestDecorator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,10 +117,13 @@ public DecorationResult decorateQualityGateStatus(AnalysisDetails analysis, AlmS
.filter(i -> OPEN_ISSUE_STATUSES.contains(i.getIssue().getStatus()))
.collect(Collectors.toList());

List<Triple<Discussion, Note, Optional<String>>> currentProjectSonarqueComments = findOpenSonarqubeComments(gitlabClient,
List<Triple<Discussion, Note, Optional<AnalysisDetails.ProjectIssueIdentifier>>> currentProjectSonarqueComments = findOpenSonarqubeComments(gitlabClient,
mergeRequest,
user.getUsername(),
analysis);
analysis)
.stream()
.filter(comment -> commentIsFromCurrentProject(comment, analysis.getAnalysisProjectKey()))
.collect(Collectors.toList());

List<String> commentKeysForOpenComments = closeOldDiscussionsAndExtractRemainingKeys(gitlabClient,
user.getUsername(),
Expand Down Expand Up @@ -155,6 +158,10 @@ public DecorationResult decorateQualityGateStatus(AnalysisDetails analysis, AlmS
return DecorationResult.builder().withPullRequestUrl(mergeRequestHtmlUrl).build();
}

private static boolean commentIsFromCurrentProject(Triple<Discussion, Note, Optional<AnalysisDetails.ProjectIssueIdentifier>> comment, String projectId) {
return comment.getRight().filter(projectIssueIdentifier -> projectId.equals(projectIssueIdentifier.getProjectKey())).isPresent();
}

@Override
public List<ALM> alm() {
return Collections.singletonList(ALM.GITLAB);
Expand Down Expand Up @@ -283,10 +290,10 @@ private static void submitSummaryComment(GitlabClient gitlabClient, AnalysisDeta

}

private static List<Triple<Discussion, Note, Optional<String>>> findOpenSonarqubeComments(GitlabClient gitlabClient,
MergeRequest mergeRequest,
String currentUsername,
AnalysisDetails analysisDetails) {
private static List<Triple<Discussion, Note, Optional<AnalysisDetails.ProjectIssueIdentifier>>> findOpenSonarqubeComments(GitlabClient gitlabClient,
MergeRequest mergeRequest,
String currentUsername,
AnalysisDetails analysisDetails) {
try {
return gitlabClient.getMergeRequestDiscussions(mergeRequest.getSourceProjectId(), mergeRequest.getIid()).stream()
.map(discussion -> discussion.getNotes().stream()
Expand All @@ -304,30 +311,34 @@ private static List<Triple<Discussion, Note, Optional<String>>> findOpenSonarqub
}

private static List<String> closeOldDiscussionsAndExtractRemainingKeys(GitlabClient gitlabClient, String currentUsername,
List<Triple<Discussion, Note, Optional<String>>> openSonarqubeComments,
List<PostAnalysisIssueVisitor.ComponentIssue> openIssues,
MergeRequest mergeRequest) {
List<Triple<Discussion, Note, Optional<AnalysisDetails.ProjectIssueIdentifier>>> openSonarqubeComments,
List<PostAnalysisIssueVisitor.ComponentIssue> openIssues,
MergeRequest mergeRequest) {
List<String> openIssueKeys = openIssues.stream()
.map(issue -> issue.getIssue().key())
.collect(Collectors.toList());

List<String> remainingCommentKeys = new ArrayList<>();

for (Triple<Discussion, Note, Optional<String>> openSonarqubeComment : openSonarqubeComments) {
Optional<String> issueKey = openSonarqubeComment.getRight();
for (Triple<Discussion, Note, Optional<AnalysisDetails.ProjectIssueIdentifier>> openSonarqubeComment : openSonarqubeComments) {
Optional<AnalysisDetails.ProjectIssueIdentifier> noteIdentifier = openSonarqubeComment.getRight();
Discussion discussion = openSonarqubeComment.getLeft();
if (!issueKey.isPresent()) {
if (!noteIdentifier.isPresent()) {
LOGGER.warn("Note {} was found on discussion {} in Merge Request {} posted by Sonarqube user {}, " +
"but Sonarqube issue details could not be parsed from it. " +
"This discussion will therefore will not be cleaned up.",
openSonarqubeComment.getMiddle().getId(),
openSonarqubeComment.getLeft().getId(),
mergeRequest.getIid(),
currentUsername);
} else if (!openIssueKeys.contains(issueKey.get())) {
continue;
}

String issueKey = noteIdentifier.get().getIssueKey();
if (!openIssueKeys.contains(issueKey)) {
resolveOrPlaceFinalCommentOnDiscussion(gitlabClient, currentUsername, discussion, mergeRequest);
} else {
remainingCommentKeys.add(issueKey.get());
remainingCommentKeys.add(issueKey);
}
}

Expand Down Expand Up @@ -355,7 +366,7 @@ private static void resolveOrPlaceFinalCommentOnDiscussion(GitlabClient gitlabCl
}
}

private static Optional<String> parseIssueDetails(Note note, AnalysisDetails analysisDetails) {
private static Optional<AnalysisDetails.ProjectIssueIdentifier> parseIssueDetails(Note note, AnalysisDetails analysisDetails) {
try (BufferedReader reader = new BufferedReader(new StringReader(note.getBody()))) {
return reader.lines()
.filter(line -> line.contains("View in SonarQube"))
Expand All @@ -368,7 +379,7 @@ private static Optional<String> parseIssueDetails(Note note, AnalysisDetails ana
}
}

private static Optional<String> parseIssueLineDetails(String noteLine, AnalysisDetails analysisDetails) {
private static Optional<AnalysisDetails.ProjectIssueIdentifier> parseIssueLineDetails(String noteLine, AnalysisDetails analysisDetails) {
Matcher identifierMatcher = NOTE_VIEW_LINK_PATTERN.matcher(noteLine);

if (identifierMatcher.matches()) {
Expand Down
Loading

0 comments on commit aa61c11

Please sign in to comment.