diff --git a/README.md b/README.md index 389e21fa..a76fe7cb 100644 --- a/README.md +++ b/README.md @@ -191,6 +191,7 @@ It is possible to filter issues by: is not created by processing commit and this issue is not supposed to be included to review. 3. Changed lines only - when only several lines are changed in a commit user may not want other lines to be commented by Gerrit. With "Add comments to changed lines only" unchanged in the commit lines will not be commented in Gerrit. +4. Included paths' glob pattern - The glob pattern that file paths should match to be included by the filter. Leaving a blank value or setting value `**/*` means any file path will match. Tested file paths always start with `/`. ![Filter settings](doc/filter-settings.png) diff --git a/doc/filter-settings.png b/doc/filter-settings.png index 38db87a2..8092c912 100644 Binary files a/doc/filter-settings.png and b/doc/filter-settings.png differ diff --git a/src/main/java/org/jenkinsci/plugins/sonargerrit/sonar/ByGlobPatternPredicate.java b/src/main/java/org/jenkinsci/plugins/sonargerrit/sonar/ByGlobPatternPredicate.java new file mode 100644 index 00000000..b8d616ce --- /dev/null +++ b/src/main/java/org/jenkinsci/plugins/sonargerrit/sonar/ByGlobPatternPredicate.java @@ -0,0 +1,23 @@ +package org.jenkinsci.plugins.sonargerrit.sonar; + +import com.google.common.base.Predicate; +import java.nio.file.FileSystems; +import java.nio.file.PathMatcher; +import java.nio.file.Paths; +import org.apache.commons.lang3.StringUtils; + +/** @author Réda Housni Alaoui */ +public class ByGlobPatternPredicate implements Predicate { + + private final PathMatcher pathMatcher; + + public ByGlobPatternPredicate(String pattern) { + this.pathMatcher = FileSystems.getDefault().getPathMatcher("glob:" + pattern); + } + + @Override + public boolean apply(Issue input) { + String filepath = StringUtils.prependIfMissing(input.getFilepath(), "/"); + return pathMatcher.matches(Paths.get(filepath)); + } +} diff --git a/src/main/java/org/jenkinsci/plugins/sonargerrit/sonar/IssueFilter.java b/src/main/java/org/jenkinsci/plugins/sonargerrit/sonar/IssueFilter.java index e7a6685f..aabbe3a4 100644 --- a/src/main/java/org/jenkinsci/plugins/sonargerrit/sonar/IssueFilter.java +++ b/src/main/java/org/jenkinsci/plugins/sonargerrit/sonar/IssueFilter.java @@ -46,6 +46,11 @@ public Iterable filter() { toBeApplied.add(ByMinSeverityPredicate.apply(severity)); } + String includedPathsGlobPattern = filterConfig.getIncludedPathsGlobPattern(); + if (includedPathsGlobPattern != null) { + toBeApplied.add(new ByGlobPatternPredicate(includedPathsGlobPattern)); + } + return Iterables.filter(issues, Predicates.and(toBeApplied)); } } diff --git a/src/main/java/org/jenkinsci/plugins/sonargerrit/sonar/IssueFilterConfig.java b/src/main/java/org/jenkinsci/plugins/sonargerrit/sonar/IssueFilterConfig.java index 1e85b822..7a2502ef 100644 --- a/src/main/java/org/jenkinsci/plugins/sonargerrit/sonar/IssueFilterConfig.java +++ b/src/main/java/org/jenkinsci/plugins/sonargerrit/sonar/IssueFilterConfig.java @@ -3,6 +3,7 @@ import static org.jenkinsci.plugins.sonargerrit.util.Localization.getLocalized; import com.google.common.base.MoreObjects; +import edu.umd.cs.findbugs.annotations.Nullable; import hudson.Extension; import hudson.model.AbstractDescribableImpl; import hudson.model.Descriptor; @@ -18,6 +19,7 @@ public class IssueFilterConfig extends AbstractDescribableImpl { */ @SuppressWarnings(value = "unused") public FormValidation doCheckSeverity(@QueryParameter String value) { - if (value == null || Severity.valueOf(value) == null) { + if (value == null) { return FormValidation.error( getLocalized("jenkins.plugin.error.review.filter.severity.unknown")); + } else { + Severity.valueOf(value); } return FormValidation.ok(); } diff --git a/src/main/resources/org/jenkinsci/plugins/sonargerrit/sonar/IssueFilterConfig/config.jelly b/src/main/resources/org/jenkinsci/plugins/sonargerrit/sonar/IssueFilterConfig/config.jelly index 8f51d647..a8585c88 100644 --- a/src/main/resources/org/jenkinsci/plugins/sonargerrit/sonar/IssueFilterConfig/config.jelly +++ b/src/main/resources/org/jenkinsci/plugins/sonargerrit/sonar/IssueFilterConfig/config.jelly @@ -34,4 +34,11 @@ description="${%jenkins.plugin.settings.gerrit.filter.lines.changed.description}"> - \ No newline at end of file + + + + + diff --git a/src/main/resources/org/jenkinsci/plugins/sonargerrit/sonar/IssueFilterConfig/config.properties b/src/main/resources/org/jenkinsci/plugins/sonargerrit/sonar/IssueFilterConfig/config.properties index 52577cfa..7dc68384 100644 --- a/src/main/resources/org/jenkinsci/plugins/sonargerrit/sonar/IssueFilterConfig/config.properties +++ b/src/main/resources/org/jenkinsci/plugins/sonargerrit/sonar/IssueFilterConfig/config.properties @@ -3,6 +3,8 @@ jenkins.plugin.settings.gerrit.filter.new=Report new issues only? jenkins.plugin.settings.gerrit.filter.new.description=Reports new as well as existing issues when unchecked. Note that pull request analysis only reports new issues and is therefore unaffected by this parameter. jenkins.plugin.settings.gerrit.filter.lines.changed=Affect changed lines only jenkins.plugin.settings.gerrit.filter.lines.changed.description=Reports issues for changed as well as not changed lines in files affected by patchset when unchecked +jenkins.plugin.settings.gerrit.filter.included-paths-glob-pattern=Included paths'' glob pattern +jenkins.plugin.settings.gerrit.filter.included-paths-glob-pattern.description=The glob pattern that file paths should match to be included by the filter. Leaving a blank value or setting value ''**/*'' means any file path will match. Tested file paths always start with ''/''. INFO=Info MINOR=Minor diff --git a/src/test/java/org/jenkinsci/plugins/sonargerrit/gerrit/ReviewTest.java b/src/test/java/org/jenkinsci/plugins/sonargerrit/gerrit/ReviewTest.java index bf6adc0d..858bd492 100644 --- a/src/test/java/org/jenkinsci/plugins/sonargerrit/gerrit/ReviewTest.java +++ b/src/test/java/org/jenkinsci/plugins/sonargerrit/gerrit/ReviewTest.java @@ -11,10 +11,12 @@ import java.io.IOException; import java.nio.file.Path; import java.util.Collections; +import java.util.Optional; import jenkins.model.Jenkins; import jenkins.model.ParameterizedJobMixIn; import me.redaalaoui.gerrit_rest_java_client.thirdparty.com.google.gerrit.extensions.common.ChangeInfo; import me.redaalaoui.gerrit_rest_java_client.thirdparty.com.google.gerrit.extensions.restapi.RestApiException; +import org.apache.commons.lang3.StringUtils; import org.eclipse.jgit.api.errors.GitAPIException; import org.jenkinsci.plugins.sonargerrit.test_infrastructure.cluster.Cluster; import org.jenkinsci.plugins.sonargerrit.test_infrastructure.cluster.EnableCluster; @@ -38,6 +40,8 @@ class ReviewTest { + "-Dsonar.pullrequest.key=${env.GERRIT_CHANGE_NUMBER}-${env.GERRIT_PATCHSET_NUMBER} " + "-Dsonar.pullrequest.base=${env.GERRIT_BRANCH} " + "-Dsonar.pullrequest.branch=${env.GERRIT_REFSPEC}"; + private static final String FILEPATH = + "src/main/java/org/example/UselessConstructorDeclaration.java"; private static Cluster cluster; private static GerritGit git; @@ -86,7 +90,7 @@ void beforeEach() throws GitAPIException { @DisplayName("STANDARD comment type") void test1() throws Exception { GerritChange change = createChangeViolatingS1186(); - triggerAndAssertSuccess(createPipelineJob(change, ReviewCommentType.STANDARD)); + triggerAndAssertSuccess(createPipelineJob(change, ReviewCommentType.STANDARD, null)); ChangeInfo changeDetail = change.getDetail(); assertThat(changeDetail.labels.get(GerritServer.CODE_QUALITY_LABEL).all) @@ -103,7 +107,7 @@ void test1() throws Exception { @DisplayName("ROBOT comment type") void test2() throws Exception { GerritChange change = createChangeViolatingS1186(); - triggerAndAssertSuccess(createPipelineJob(change, ReviewCommentType.ROBOT)); + triggerAndAssertSuccess(createPipelineJob(change, ReviewCommentType.ROBOT, null)); ChangeInfo changeDetail = change.getDetail(); assertThat(changeDetail.labels.get(GerritServer.CODE_QUALITY_LABEL).all) @@ -126,7 +130,7 @@ void test2() throws Exception { @DisplayName("Review tag is autogenerated:sonar") void test3() throws Exception { GerritChange change = createChangeViolatingS1186(); - triggerAndAssertSuccess(createPipelineJob(change, ReviewCommentType.STANDARD)); + triggerAndAssertSuccess(createPipelineJob(change, ReviewCommentType.STANDARD, null)); ChangeInfo changeDetail = change.getDetail(); assertThat(changeDetail.labels.get(GerritServer.CODE_QUALITY_LABEL).all) @@ -135,10 +139,42 @@ void test3() throws Exception { .containsExactly("autogenerated:sonar"); } + @Test + @DisplayName("Issue ignored because of path glob pattern") + void test4() throws Exception { + GerritChange change = createChangeViolatingS1186(); + triggerAndAssertSuccess(createPipelineJob(change, ReviewCommentType.ROBOT, "/[!src]/**")); + + ChangeInfo changeDetail = change.getDetail(); + assertThat(changeDetail.labels.get(GerritServer.CODE_QUALITY_LABEL).all) + .hasSize(1) + .map(approvalInfo -> approvalInfo.value) + .containsExactly(1); + + assertThat(change.listRobotComments()).isEmpty(); + } + + @Test + @DisplayName("Issue considered despite of path glob pattern") + void test5() throws Exception { + GerritChange change = createChangeViolatingS1186(); + triggerAndAssertSuccess(createPipelineJob(change, ReviewCommentType.ROBOT, "/src/**")); + + ChangeInfo changeDetail = change.getDetail(); + assertThat(changeDetail.labels.get(GerritServer.CODE_QUALITY_LABEL).all) + .hasSize(1) + .map(approvalInfo -> approvalInfo.value) + .containsExactly(-1); + + assertThat(change.listRobotComments()) + .filteredOn(comment -> comment.message.contains("S1186")) + .hasSize(1); + } + private GerritChange createChangeViolatingS1186() throws GitAPIException, IOException, RestApiException { git.addAndCommitFile( - "src/main/java/org/example/UselessConstructorDeclaration.java", + FILEPATH, "package org.example; " + "public class UselessConstructorDeclaration { " + "public UselessConstructorDeclaration() {} " @@ -147,10 +183,15 @@ private GerritChange createChangeViolatingS1186() } @SuppressWarnings("rawtypes") - private Job createPipelineJob(GerritChange change, ReviewCommentType commentType) + private Job createPipelineJob( + GerritChange change, ReviewCommentType commentType, String includedPathsGlobPattern) throws IOException { WorkflowJob job = cluster.jenkinsRule().createProject(WorkflowJob.class); int patchSetNumber = 1; + String quotedIncludedPathsGlobPattern = + Optional.ofNullable(includedPathsGlobPattern) + .map(s -> StringUtils.wrap(s, "'")) + .orElse(null); String script = "node {\n" + "stage('Build') {\n" @@ -185,14 +226,16 @@ private Job createPipelineJob(GerritChange change, ReviewCommentType commentType + "issueFilterConfig: [\n" + "severity: 'MINOR',\n" + "newIssuesOnly: false,\n" - + "changedLinesOnly: true\n" + + "changedLinesOnly: true,\n" + + String.format("includedPathsGlobPattern: %s\n", quotedIncludedPathsGlobPattern) + "]\n" // issueFilterConfig + "],\n" // reviewConfig + "scoreConfig: [\n" + "issueFilterConfig: [\n" - + "severity: 'MINOR'," - + "newIssuesOnly: false," - + "changedLinesOnly: true" + + "severity: 'MINOR',\n" + + "newIssuesOnly: false,\n" + + "changedLinesOnly: true,\n" + + String.format("includedPathsGlobPattern: %s\n", quotedIncludedPathsGlobPattern) + "],\n" // issueFilterConfig + String.format("category: '%s',\n", GerritServer.CODE_QUALITY_LABEL) + "noIssuesScore: 1,\n" diff --git a/src/test/java/org/jenkinsci/plugins/sonargerrit/sonar/ByGlobPatternPredicateTest.java b/src/test/java/org/jenkinsci/plugins/sonargerrit/sonar/ByGlobPatternPredicateTest.java new file mode 100644 index 00000000..15f762c3 --- /dev/null +++ b/src/test/java/org/jenkinsci/plugins/sonargerrit/sonar/ByGlobPatternPredicateTest.java @@ -0,0 +1,104 @@ +package org.jenkinsci.plugins.sonargerrit.sonar; + +import static java.util.Objects.requireNonNull; +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.Date; +import java.util.Optional; +import org.junit.jupiter.api.Test; + +/** @author Réda Housni Alaoui */ +class ByGlobPatternPredicateTest { + + @Test + void test() { + assertThat(new ByGlobPatternPredicate("**/*").apply(new DummyIssue("/foo.bar"))).isTrue(); + assertThat(new ByGlobPatternPredicate("/foo.bar").apply(new DummyIssue("/foo.bar"))).isTrue(); + assertThat(new ByGlobPatternPredicate("/foo/*").apply(new DummyIssue("/foo/bar"))).isTrue(); + assertThat(new ByGlobPatternPredicate("/foo/**").apply(new DummyIssue("/foo/bar/baz"))) + .isTrue(); + assertThat(new ByGlobPatternPredicate("/foo/**").apply(new DummyIssue("/foo/bar/baz/bat.toto"))) + .isTrue(); + assertThat(new ByGlobPatternPredicate("/bar/**").apply(new DummyIssue("/foo/bar/baz/bat.toto"))) + .isFalse(); + } + + private static class DummyIssue implements Issue { + + private final String filePath; + + DummyIssue(String filePath) { + this.filePath = requireNonNull(filePath); + } + + @Override + public String inspectorName() { + return null; + } + + @Override + public String inspectionId() { + return null; + } + + @Override + public Optional detailUrl() { + return Optional.empty(); + } + + @Override + public String getFilepath() { + return filePath; + } + + @Override + public String getKey() { + return null; + } + + @Override + public String getComponent() { + return null; + } + + @Override + public Integer getLine() { + return null; + } + + @Override + public String getMessage() { + return null; + } + + @Override + public Severity getSeverity() { + return null; + } + + @Override + public String getRule() { + return null; + } + + @Override + public String getRuleUrl() { + return null; + } + + @Override + public String getStatus() { + return null; + } + + @Override + public boolean isNew() { + return false; + } + + @Override + public Date getCreationDate() { + return null; + } + } +} diff --git a/src/test/java/org/jenkinsci/plugins/sonargerrit/test_infrastructure/jenkins/MavenConfiguration.java b/src/test/java/org/jenkinsci/plugins/sonargerrit/test_infrastructure/jenkins/MavenConfiguration.java index 3ff8c30f..f5d7e865 100644 --- a/src/test/java/org/jenkinsci/plugins/sonargerrit/test_infrastructure/jenkins/MavenConfiguration.java +++ b/src/test/java/org/jenkinsci/plugins/sonargerrit/test_infrastructure/jenkins/MavenConfiguration.java @@ -22,7 +22,7 @@ public String addInstallation() throws IOException { ZipExtractionInstaller installer = new ZipExtractionInstaller( null, - "https://dlcdn.apache.org/maven/maven-3/3.8.4/binaries/apache-maven-3.8.4-bin.tar.gz", + "https://archive.apache.org/dist/maven/maven-3/3.8.4/binaries/apache-maven-3.8.4-bin.tar.gz", "apache-maven-3.8.4"); String name = UUID.randomUUID().toString();