Skip to content

Commit

Permalink
#141: Resolve or retain Gitlab comments rather than deleting them
Browse files Browse the repository at this point in the history
The Gitlab decorator currently deletes all non-system comments posted by the user that Sonarqube is connected as and then creates comments for any issues reported in the current analysis, even where an issue is the same across multiple analyses of a Merge Request so would have an identical comment re-created under a new discussion. This causes issues with orphaned system comments - such as `UserXXX changed this line in version X of the diff` - and user discussions referring to Sonarqube's finding that no longer have the finding in the thread.

The decorator will now attempt to parse the comment's issue ID from the SonarQube link included in each comment and will close any discussion where the ID no longer exists in the analysis results, unless another user has also commented on the discussion in which case the decorator will comment that the issue is resolved but will leave the discussion open for manual review. Where the ID is not parsable a warning is logged and the discussion is left open.

To simplify the code within the decorator, the interactions with the Gitlab API have been pulled out into a separate GitlabClient, thereby allowing consistent authentication and error handling for Gitlab calls.
  • Loading branch information
mc1arke committed Jul 7, 2021
1 parent 732ba92 commit a032a91
Show file tree
Hide file tree
Showing 34 changed files with 2,199 additions and 815 deletions.
11 changes: 10 additions & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ compileJava {
dependencies {
compileOnly fileTree(dir: sonarLibraries, include: '**/*.jar', exclude: 'extensions/*.jar')
testCompile fileTree(dir: sonarLibraries, include: '**/*.jar', exclude: 'extensions/*.jar')
testCompile group: 'junit', name: 'junit', version: '4.12'
testCompile group: 'org.mockito', name: 'mockito-core', version: '3.1.0'
testCompile group: 'org.assertj', name: 'assertj-core', version: '3.13.2'
testCompile group: 'com.github.tomakehurst', name: 'wiremock', version: '2.24.1'
Expand All @@ -66,6 +65,9 @@ dependencies {
compileOnly 'com.google.code.findbugs:jsr305:3.0.2'
compile 'org.javassist:javassist:3.27.0-GA'
implementation 'com.squareup.okhttp3:logging-interceptor:4.9.0'
testImplementation(platform('org.junit:junit-bom:5.7.2'))
testImplementation('org.junit.jupiter:junit-jupiter')
testRuntime("org.junit.vintage:junit-vintage-engine")
}


Expand Down Expand Up @@ -137,3 +139,10 @@ jacocoTestReport {
plugins.withType(JacocoPlugin) {
tasks["test"].finalizedBy 'jacocoTestReport'
}

test {
useJUnitPlatform()
testLogging {
events "passed", "skipped", "failed"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,16 @@
*/
package com.github.mc1arke.sonarqube.plugin.ce;

import com.github.mc1arke.sonarqube.plugin.ce.pullrequest.DefaultLinkHeaderReader;
import com.github.mc1arke.sonarqube.plugin.ce.pullrequest.PostAnalysisIssueVisitor;
import com.github.mc1arke.sonarqube.plugin.ce.pullrequest.PullRequestPostAnalysisTask;
import com.github.mc1arke.sonarqube.plugin.ce.pullrequest.azuredevops.AzureDevOpsServerPullRequestDecorator;
import com.github.mc1arke.sonarqube.plugin.ce.pullrequest.bitbucket.BitbucketPullRequestDecorator;
import com.github.mc1arke.sonarqube.plugin.ce.pullrequest.github.GithubPullRequestDecorator;
import com.github.mc1arke.sonarqube.plugin.ce.pullrequest.github.v3.DefaultLinkHeaderReader;
import com.github.mc1arke.sonarqube.plugin.ce.pullrequest.github.v3.RestApplicationAuthenticationProvider;
import com.github.mc1arke.sonarqube.plugin.ce.pullrequest.github.v4.GraphqlCheckRunProvider;
import com.github.mc1arke.sonarqube.plugin.ce.pullrequest.gitlab.GitlabServerPullRequestDecorator;
import com.github.mc1arke.sonarqube.plugin.ce.pullrequest.gitlab.DefaultGitlabClientFactory;
import com.github.mc1arke.sonarqube.plugin.ce.pullrequest.gitlab.GitlabMergeRequestDecorator;
import org.sonar.ce.task.projectanalysis.container.ReportAnalysisComponentProvider;

import java.util.Arrays;
Expand All @@ -42,7 +43,7 @@ public List<Object> getComponents() {
return Arrays.asList(CommunityBranchLoaderDelegate.class, PullRequestPostAnalysisTask.class,
PostAnalysisIssueVisitor.class, GithubPullRequestDecorator.class,
GraphqlCheckRunProvider.class, DefaultLinkHeaderReader.class, RestApplicationAuthenticationProvider.class,
BitbucketPullRequestDecorator.class, GitlabServerPullRequestDecorator.class,
BitbucketPullRequestDecorator.class, GitlabMergeRequestDecorator.class, DefaultGitlabClientFactory.class,
AzureDevOpsServerPullRequestDecorator.class);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2020 Michael Clarke
* Copyright (C) 2020-2021 Michael Clarke
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
Expand Down Expand Up @@ -30,6 +30,8 @@
import com.github.mc1arke.sonarqube.plugin.ce.pullrequest.markup.Paragraph;
import com.github.mc1arke.sonarqube.plugin.ce.pullrequest.markup.Text;
import org.apache.commons.lang.StringUtils;
import org.apache.http.NameValuePair;
import org.apache.http.client.utils.URLEncodedUtils;
import org.sonar.api.ce.posttask.Analysis;
import org.sonar.api.ce.posttask.Project;
import org.sonar.api.ce.posttask.QualityGate;
Expand All @@ -49,6 +51,7 @@

import java.io.UnsupportedEncodingException;
import java.math.BigDecimal;
import java.net.URI;
import java.net.URLEncoder;
import java.nio.charset.StandardCharsets;
import java.text.DecimalFormat;
Expand Down Expand Up @@ -124,6 +127,24 @@ public String getIssueUrl(PostAnalysisIssueVisitor.LightIssue issue) {
}
}

public Optional<String> parseIssueIdFromUrl(String issueUrl) {
URI url = URI.create(issueUrl);
List<NameValuePair> parameters = URLEncodedUtils.parse(url, StandardCharsets.UTF_8);
if (url.getPath().endsWith("/dashboard")) {
return Optional.of("decorator-summary-comment");
} else if (url.getPath().endsWith("security_hotspots")) {
return parameters.stream()
.filter(parameter -> "hotspots".equals(parameter.getName()))
.map(NameValuePair::getValue)
.findFirst();
} else {
return parameters.stream()
.filter(parameter -> "issues".equals(parameter.getName()))
.map(NameValuePair::getValue)
.findFirst();
}
}

public Optional<String> getPullRequestBase() {
return Optional.ofNullable(scannerContext.getProperties().get(SCANNERROPERTY_PULLREQUEST_BASE));
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2020 Michael Clarke
* Copyright (C) 2020-2021 Michael Clarke
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
Expand All @@ -16,7 +16,7 @@
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*
*/
package com.github.mc1arke.sonarqube.plugin.ce.pullrequest.github.v3;
package com.github.mc1arke.sonarqube.plugin.ce.pullrequest;

import java.util.Arrays;
import java.util.Optional;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2020 Michael Clarke
* Copyright (C) 2020-2021 Michael Clarke
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
Expand All @@ -16,11 +16,11 @@
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*
*/
package com.github.mc1arke.sonarqube.plugin.ce.pullrequest.github.v3;
package com.github.mc1arke.sonarqube.plugin.ce.pullrequest;

import java.util.Optional;

interface LinkHeaderReader {
public interface LinkHeaderReader {

Optional<String> findNextLink(String linkHeader);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

import com.fasterxml.jackson.databind.DeserializationFeature;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.github.mc1arke.sonarqube.plugin.ce.pullrequest.LinkHeaderReader;
import com.github.mc1arke.sonarqube.plugin.ce.pullrequest.github.GithubApplicationAuthenticationProvider;
import com.github.mc1arke.sonarqube.plugin.ce.pullrequest.github.RepositoryAuthenticationToken;
import com.github.mc1arke.sonarqube.plugin.ce.pullrequest.github.v3.model.AppInstallation;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/*
* Copyright (C) 2021 Michael Clarke
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*
*/
package com.github.mc1arke.sonarqube.plugin.ce.pullrequest.gitlab;

import com.fasterxml.jackson.databind.DeserializationFeature;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.github.mc1arke.sonarqube.plugin.ce.pullrequest.DefaultLinkHeaderReader;

public class DefaultGitlabClientFactory implements GitlabClientFactory {

private final ObjectMapper objectMapper;

public DefaultGitlabClientFactory() {
super();
this.objectMapper = new ObjectMapper()
.configure(DeserializationFeature.ACCEPT_EMPTY_ARRAY_AS_NULL_OBJECT, true)
.configure(DeserializationFeature.ACCEPT_SINGLE_VALUE_AS_ARRAY, true)
.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false);
}

@Override
public GitlabClient createClient(String baseApiUrl, String authToken) {
return new GitlabRestClient(baseApiUrl, authToken, new DefaultLinkHeaderReader(), objectMapper);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/*
* Copyright (C) 2021 Michael Clarke
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*
*/
package com.github.mc1arke.sonarqube.plugin.ce.pullrequest.gitlab;

import com.github.mc1arke.sonarqube.plugin.ce.pullrequest.gitlab.model.Commit;
import com.github.mc1arke.sonarqube.plugin.ce.pullrequest.gitlab.model.Discussion;
import com.github.mc1arke.sonarqube.plugin.ce.pullrequest.gitlab.model.MergeRequest;
import com.github.mc1arke.sonarqube.plugin.ce.pullrequest.gitlab.model.MergeRequestNote;
import com.github.mc1arke.sonarqube.plugin.ce.pullrequest.gitlab.model.PipelineStatus;
import com.github.mc1arke.sonarqube.plugin.ce.pullrequest.gitlab.model.User;

import java.io.IOException;
import java.util.List;

public interface GitlabClient {

User getCurrentUser() throws IOException;

MergeRequest getMergeRequest(String projectId, long mergeRequestIid) throws IOException;

List<Commit> getMergeRequestCommits(long projectId, long mergeRequestIid) throws IOException;

List<Discussion> getMergeRequestDiscussions(long projectId, long mergeRequestIid) throws IOException;

Discussion addMergeRequestDiscussion(long projectId, long mergeRequestIid, MergeRequestNote commitNote) throws IOException;

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

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

void setMergeRequestPipelineStatus(long projectId, String commitRevision, PipelineStatus status) throws IOException;

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/*
* Copyright (C) 2021 Michael Clarke
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*
*/
package com.github.mc1arke.sonarqube.plugin.ce.pullrequest.gitlab;

public interface GitlabClientFactory {

GitlabClient createClient(String baseApiUrl, String authToken);
}
Loading

0 comments on commit a032a91

Please sign in to comment.