Skip to content

Commit

Permalink
mc1arke#790: Add an ability to edit Summary Note instead of creating …
Browse files Browse the repository at this point in the history
…a new one
  • Loading branch information
alexintech committed Aug 3, 2023
1 parent 344a37f commit a33ff3d
Show file tree
Hide file tree
Showing 10 changed files with 329 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@
public class CommunityBranchPlugin implements Plugin, CoreExtension {

public static final String IMAGE_URL_BASE = "com.github.mc1arke.sonarqube.plugin.branch.image-url-base";
public static final String PR_SUMMARY_NOTE_EDIT = "com.github.mc1arke.sonarqube.plugin.branch.pullrequest.summary.edit";

@Override
public String getName() {
Expand Down Expand Up @@ -157,6 +158,15 @@ public void load(CoreExtension.Context context) {
.build(),
MonoRepoFeature.class);

context.addExtensions(PropertyDefinition.builder(PR_SUMMARY_NOTE_EDIT)
.category(getName())
.subCategory("Merge Request Decoration")
.onQualifiers(Qualifiers.PROJECT)
.name("Edit summary note")
.description("Edit summary discussion thread instead of resolving it and creating a new one (Gitlab only).")
.type(PropertyType.BOOLEAN)
.defaultValue(String.valueOf(false))
.build());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ public interface GitlabClient {

void addMergeRequestDiscussionNote(long projectId, long mergeRequestIid, String discussionId, String noteContent) throws IOException;

void editMergeRequestDisscussionNote(long projectId, long mergeRequestIid, String discussionId, long noteId, String noteContent) throws IOException;

void resolveMergeRequestDiscussion(long projectId, long mergeRequestIid, String discussionId) throws IOException;

void setMergeRequestPipelineStatus(long projectId, String commitRevision, PipelineStatus status) throws IOException;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,15 @@ public void addMergeRequestDiscussionNote(long projectId, long mergeRequestIid,
entity(httpPost, null, httpResponse -> validateResponse(httpResponse, 201, "Commit discussions note added"));
}

@Override
public void editMergeRequestDisscussionNote(long projectId, long mergeRequestIid, String discussionId, long noteId, String noteContent) throws IOException {
String targetUrl = String.format("%s/projects/%s/merge_requests/%s/discussions/%s/notes/%s", baseGitlabApiUrl, projectId, mergeRequestIid, discussionId, noteId);

HttpPut httpPut = new HttpPut(targetUrl);
httpPut.setEntity(new UrlEncodedFormEntity(Collections.singletonList(new BasicNameValuePair("body", noteContent)), StandardCharsets.UTF_8));
entity(httpPut, null);
}

@Override
public void resolveMergeRequestDiscussion(long projectId, long mergeRequestIid, String discussionId) throws IOException {
String discussionIdUrl = String.format("%s/projects/%s/merge_requests/%s/discussions/%s?resolved=true", baseGitlabApiUrl, projectId, mergeRequestIid, discussionId);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ public DecorationResult decorateQualityGateStatus(AnalysisDetails analysis, AlmS
U user = getCurrentUser(client);
List<PostAnalysisIssueVisitor.ComponentIssue> openSonarqubeIssues = analysis.getScmReportableIssues();

List<Triple<D, N, Optional<ProjectIssueIdentifier>>> currentProjectSonarqubeComments = findOpenSonarqubeComments(client,
List<Triple<D, N, Optional<ProjectIssueIdentifier>>> currentProjectSonarqubeComments = findSonarqubeComments(client,
pullRequest,
user)
.stream()
Expand All @@ -82,7 +82,8 @@ public DecorationResult decorateQualityGateStatus(AnalysisDetails analysis, AlmS
user,
currentProjectSonarqubeComments,
openSonarqubeIssues,
pullRequest);
pullRequest,
isEditSummaryNoteEnabled(analysis));

List<String> commitIds = getCommitIdsForPullRequest(client, pullRequest);
List<Pair<PostAnalysisIssueVisitor.ComponentIssue, String>> uncommentedIssues = findIssuesWithoutComments(openSonarqubeIssues,
Expand All @@ -102,14 +103,34 @@ public DecorationResult decorateQualityGateStatus(AnalysisDetails analysis, AlmS
reportGenerator.createAnalysisIssueSummary(issue.getLeft(), analysis)));

AnalysisSummary analysisSummary = reportGenerator.createAnalysisSummary(analysis);
submitSummaryNote(client, pullRequest, analysis, analysisSummary);
editExistingOrSubmitSummaryNote(client, pullRequest, currentProjectSonarqubeComments, analysis, analysisSummary);
submitPipelineStatus(client, pullRequest, analysis, analysisSummary);

DecorationResult.Builder builder = DecorationResult.builder();
createFrontEndUrl(pullRequest, analysis).ifPresent(builder::withPullRequestUrl);
return builder.build();
}

private void editExistingOrSubmitSummaryNote(C client, P pullRequest,
List<Triple<D, N, Optional<ProjectIssueIdentifier>>> currentProjectSonarqubeComments,
AnalysisDetails analysis, AnalysisSummary analysisSummary) {
Optional<Triple<D, N, Optional<ProjectIssueIdentifier>>> existingSummaryNote = currentProjectSonarqubeComments.stream()
.filter(discussion -> isSummaryNote(discussion.getRight()))
.findFirst();

if (isEditSummaryNoteEnabled(analysis) && existingSummaryNote.isPresent()) {
editSummaryNote(client, pullRequest, existingSummaryNote.get().getLeft(), analysis, analysisSummary);
} else {
submitSummaryNote(client, pullRequest, analysis, analysisSummary);
}
}

private boolean isSummaryNote(Optional<ProjectIssueIdentifier> projectIssueIdentifier) {
return projectIssueIdentifier
.map(ProjectIssueIdentifier::isSummary)
.orElse(false);
}

protected abstract C createClient(AlmSettingDto almSettingDto, ProjectAlmSettingDto projectAlmSettingDto);

protected abstract Optional<String> createFrontEndUrl(P pullRequest, AnalysisDetails analysisDetails);
Expand Down Expand Up @@ -139,6 +160,8 @@ protected abstract void submitCommitNoteForIssue(C client, P pullRequest, PostAn

protected abstract void submitSummaryNote(C client, P pullRequest, AnalysisDetails analysis, AnalysisSummary analysisSummary);

protected abstract void editSummaryNote(C client, P pullRequest, D discussion, AnalysisDetails analysis, AnalysisSummary analysisSummary);

protected abstract List<D> getDiscussions(C client, P pullRequest);

protected abstract boolean isNoteFromCurrentUser(N note, U user);
Expand Down Expand Up @@ -171,15 +194,14 @@ private static boolean isIssueFromCommitInCurrentRequest(PostAnalysisIssueVisito
.isPresent();
}

private List<Triple<D, N, Optional<ProjectIssueIdentifier>>> findOpenSonarqubeComments(C client, P pullRequest,
U currentUser) {
private List<Triple<D, N, Optional<ProjectIssueIdentifier>>> findSonarqubeComments(C client, P pullRequest,
U currentUser) {
return getDiscussions(client, pullRequest).stream()
.map(discussion -> {
List<N> commentsForDiscussion = getNotesForDiscussion(client, discussion);
return commentsForDiscussion.stream()
.findFirst()
.filter(note -> isNoteFromCurrentUser(note, currentUser))
.filter(note -> !isResolved(client, discussion, commentsForDiscussion, currentUser))
.map(note -> new ImmutableTriple<>(discussion, note, parseIssueDetails(client, note)));
})
.filter(Optional::isPresent)
Expand All @@ -188,22 +210,31 @@ private List<Triple<D, N, Optional<ProjectIssueIdentifier>>> findOpenSonarqubeCo
}

private List<String> closeOldDiscussionsAndExtractRemainingKeys(C client, U currentUser,
List<Triple<D, N, Optional<ProjectIssueIdentifier>>> openSonarqubeComments,
List<Triple<D, N, Optional<ProjectIssueIdentifier>>> sonarqubeComments,
List<PostAnalysisIssueVisitor.ComponentIssue> openIssues,
P pullRequest) {
P pullRequest,
boolean skipSummaryNote) {
List<String> openIssueKeys = openIssues.stream()
.map(issue -> issue.getIssue().key())
.collect(Collectors.toList());

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

for (Triple<D, N, Optional<ProjectIssueIdentifier>> openSonarqubeComment : openSonarqubeComments) {
Optional<ProjectIssueIdentifier> noteIdentifier = openSonarqubeComment.getRight();
D discussion = openSonarqubeComment.getLeft();
for (Triple<D, N, Optional<ProjectIssueIdentifier>> sonarqubeComment : sonarqubeComments) {
Optional<ProjectIssueIdentifier> noteIdentifier = sonarqubeComment.getRight();
D discussion = sonarqubeComment.getLeft();
if (noteIdentifier.isEmpty()) {
continue;
}

if (skipSummaryNote && isSummaryNote(noteIdentifier)) {
continue;
}

if (isResolved(client, discussion, getNotesForDiscussion(client, discussion), currentUser)) {
continue;
}

String issueKey = noteIdentifier.get().getIssueKey();
if (!openIssueKeys.contains(issueKey)) {
resolveOrPlaceFinalCommentOnDiscussion(client, currentUser, discussion, pullRequest);
Expand Down Expand Up @@ -274,7 +305,7 @@ private static Optional<ProjectIssueIdentifier> parseIssueIdFromUrl(String issue
String projectId = optionalProjectId.get();

if (url.getPath().endsWith("/dashboard")) {
return Optional.of(new ProjectIssueIdentifier(projectId, "decorator-summary-comment"));
return Optional.of(new ProjectIssueIdentifier(projectId, ProjectIssueIdentifier.SUMMARY_NOTE_ISSUE_KEY));
} else if (url.getPath().endsWith("security_hotspots")) {
return parameters.stream()
.filter(parameter -> "hotspots".equals(parameter.getName()))
Expand All @@ -296,6 +327,8 @@ private static boolean isCommentFromCurrentProject(Triple<?, ?, Optional<Project

protected static class ProjectIssueIdentifier {

private static final String SUMMARY_NOTE_ISSUE_KEY = "decorator-summary-comment";

private final String projectKey;
private final String issueKey;

Expand All @@ -311,5 +344,9 @@ public String getProjectKey() {
public String getIssueKey() {
return issueKey;
}

public boolean isSummary() {
return SUMMARY_NOTE_ISSUE_KEY.equals(issueKey);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,8 @@ DecorationResult decorateQualityGateStatus(AnalysisDetails analysisDetails, AlmS
ProjectAlmSettingDto projectAlmSettingDto);

List<ALM> alm();

default boolean isEditSummaryNoteEnabled(AnalysisDetails analysisDetails) {
return false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,11 @@ protected void submitSummaryNote(AzureDevopsClient client, PullRequest pullReque
}
}

@Override
protected void editSummaryNote(AzureDevopsClient client, PullRequest pullRequest, CommentThread discussion, AnalysisDetails analysis, AnalysisSummary analysisSummary) {
// not implemented
}

protected List<CommentThread> getDiscussions(AzureDevopsClient client, PullRequest pullRequest) {
try {
return client.retrieveThreads(pullRequest.getRepository().getProject().getName(), pullRequest.getRepository().getName(), pullRequest.getId());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
*/
package com.github.mc1arke.sonarqube.plugin.ce.pullrequest.gitlab;

import com.github.mc1arke.sonarqube.plugin.CommunityBranchPlugin;
import com.github.mc1arke.sonarqube.plugin.almclient.gitlab.GitlabClient;
import com.github.mc1arke.sonarqube.plugin.almclient.gitlab.GitlabClientFactory;
import com.github.mc1arke.sonarqube.plugin.almclient.gitlab.model.Commit;
Expand Down Expand Up @@ -62,6 +63,13 @@ public GitlabMergeRequestDecorator(ScmInfoRepository scmInfoRepository, GitlabCl
this.formatterFactory = formatterFactory;
}

@Override
public boolean isEditSummaryNoteEnabled(AnalysisDetails analysis) {
return analysis.getScannerProperty(CommunityBranchPlugin.PR_SUMMARY_NOTE_EDIT)
.map(Boolean::parseBoolean)
.orElse(false);
}

@Override
public List<ALM> alm() {
return Collections.singletonList(ALM.GITLAB);
Expand Down Expand Up @@ -169,6 +177,22 @@ protected void submitSummaryNote(GitlabClient client, MergeRequest mergeRequest,

}

@Override
protected void editSummaryNote(GitlabClient client, MergeRequest mergeRequest, Discussion discussion, AnalysisDetails analysis, AnalysisSummary analysisSummary) {
try {
client.editMergeRequestDisscussionNote(mergeRequest.getTargetProjectId(),
mergeRequest.getIid(),
discussion.getId(),
discussion.getNotes().get(0).getId(),
analysisSummary.format(formatterFactory));
if (!isClosed(discussion, discussion.getNotes()) && analysis.getQualityGateStatus() == QualityGate.Status.OK) {
client.resolveMergeRequestDiscussion(mergeRequest.getTargetProjectId(), mergeRequest.getIid(), discussion.getId());
}
} catch (IOException ex) {
throw new IllegalStateException("Could not edit summary comment to Gitlab", ex);
}
}

@Override
protected List<Discussion> getDiscussions(GitlabClient client, MergeRequest pullRequest) {
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
*/
package com.github.mc1arke.sonarqube.plugin.scanner;

import com.github.mc1arke.sonarqube.plugin.CommunityBranchPlugin;
import com.github.mc1arke.sonarqube.plugin.ce.pullrequest.gitlab.GitlabMergeRequestDecorator;
import org.sonar.api.batch.sensor.Sensor;
import org.sonar.api.batch.sensor.SensorContext;
Expand Down Expand Up @@ -54,6 +55,8 @@ public void execute(SensorContext sensorContext) {
Optional.ofNullable(system2.property(GitlabMergeRequestDecorator.PULLREQUEST_GITLAB_PIPELINE_ID)).ifPresent(
v -> sensorContext.addContextProperty(GitlabMergeRequestDecorator.PULLREQUEST_GITLAB_PIPELINE_ID, v));

sensorContext.config().get(CommunityBranchPlugin.PR_SUMMARY_NOTE_EDIT)
.ifPresent(p -> sensorContext.addContextProperty(CommunityBranchPlugin.PR_SUMMARY_NOTE_EDIT, p));
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
package com.github.mc1arke.sonarqube.plugin.ce.pullrequest.gitlab;

import com.github.mc1arke.sonarqube.plugin.almclient.gitlab.model.Discussion;
import com.github.mc1arke.sonarqube.plugin.almclient.gitlab.model.Note;
import com.github.mc1arke.sonarqube.plugin.almclient.gitlab.model.User;

import java.util.*;
import java.util.function.Supplier;
import java.util.stream.Collectors;

class DiscussionMock {
private static final String PROJECT_KEY = "projectKey";
private static final String SONARQUBE_USERNAME = "sonarqube@gitlab.dummy";
private static final String OLD_SONARQUBE_ISSUE_COMMENT = "This issue no longer exists in SonarQube, " +
"but due to other comments being present in this discussion, " +
"the discussion is not being being closed automatically. " +
"Please manually resolve this discussion once the other comments have been reviewed.";
private static final User sonarqubeUser = new User(SONARQUBE_USERNAME);

private static Discussion createIssueComment() {
Note note = createUnresolvedSonarQubeNote(
"Reported issue\n[View in SonarQube](http://domain.url/sonar/issue?issues=issueKey1&id=" +
PROJECT_KEY + ")");
return new Discussion("issue-comment-id", Collections.singletonList(note));
}

private static Discussion createResolvedIssueComment() {
Note note = createResolvedSonarQubeNote(
"Reported issue\n[View in SonarQube](http://domain.url/sonar/issue?issues=issueKey1&id=" +
PROJECT_KEY + ")");
return new Discussion("resolved-issue-comment-id", Collections.singletonList(note));
}

private static Discussion createResolvedByCommentIssueComment() {
Note note = createUnresolvedSonarQubeNote(
"Reported issue\n[View in SonarQube](http://domain.url/sonar/issue?issues=issueKey1&id=" +
PROJECT_KEY + ")");
Note note2 = createUnresolvedSonarQubeNote(OLD_SONARQUBE_ISSUE_COMMENT);
Note note3 = createUnresolvedSonarQubeNote("Some additional comment");
return new Discussion("issue-with-resolved-comment-id", Arrays.asList(note, note2, note3));
}

private static Discussion createUnresolvedSummaryNote() {
Note note = createUnresolvedSonarQubeNote(
"Analysis Details\n[View in SonarQube](http://domain.url/sonar/dashboard?id=" + PROJECT_KEY + ")");
return new Discussion("summary-note-id", Collections.singletonList(note));
}

private static Discussion createResolvedSummaryNote() {
Note note = createResolvedSonarQubeNote(
"Analysis Details\n[View in SonarQube](http://domain.url/sonar/dashboard?id=" + PROJECT_KEY + ")");
return new Discussion("summary-note-id", Collections.singletonList(note));
}

private static Note createUnresolvedSonarQubeNote(String body) {
return new Note(new Random().nextLong(), false, sonarqubeUser, body, false, true);
}

private static Note createResolvedSonarQubeNote(String body) {
return new Note(new Random().nextLong(), false, sonarqubeUser, body, true, true);
}

public static Map<DiscussionType, Discussion> getDiscussionsMap(DiscussionType... discussions) {
return Arrays.stream(discussions)
.collect(Collectors.toMap(k -> k, DiscussionType::create, (e1, e2) -> e1, LinkedHashMap::new));
}

enum DiscussionType {
RESOLVED_SUMMARY_NOTE(DiscussionMock::createResolvedSummaryNote),
UNRESOLVED_SUMMARY_NOTE(DiscussionMock::createUnresolvedSummaryNote),
ISSUE_COMMENT(DiscussionMock::createIssueComment),
RESOLVED_ISSUE_COMMENT(DiscussionMock::createResolvedIssueComment),
RESOLVED_BY_COMMENT_ISSUE_COMMENT(DiscussionMock::createResolvedByCommentIssueComment);

private final Supplier<Discussion> creationMethod;

DiscussionType(Supplier<Discussion> creationMethod) {
this.creationMethod = creationMethod;
}

public Discussion create() {
return creationMethod.get();
}
}

}
Loading

0 comments on commit a33ff3d

Please sign in to comment.