From c698fdc1b308fef1a7af5767e42e6b7e82fb502e Mon Sep 17 00:00:00 2001 From: Julien HENRY Date: Wed, 7 Jan 2015 21:58:46 +0100 Subject: [PATCH] SONAR-6014 Fix regression when persisting dependencies and start dropping violations --- .../batch/design/MavenDependenciesSensor.java | 5 +-- .../sonar/batch/index/ResourcePersister.java | 2 +- .../org/sonar/api/checks/NoSonarFilter.java | 29 +++++++++------- .../sonar/api/checks/NoSonarFilterTest.java | 34 ++++++++++++------- 4 files changed, 42 insertions(+), 28 deletions(-) diff --git a/sonar-batch/src/main/java/org/sonar/batch/design/MavenDependenciesSensor.java b/sonar-batch/src/main/java/org/sonar/batch/design/MavenDependenciesSensor.java index 34624780916..a6de27c9ef2 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/design/MavenDependenciesSensor.java +++ b/sonar-batch/src/main/java/org/sonar/batch/design/MavenDependenciesSensor.java @@ -253,12 +253,13 @@ public class MavenDependenciesSensor implements Sensor { context.saveDependency(dependency); } - protected static Resource toResource(final Project project, Artifact artifact, SensorContext context) { + protected Resource toResource(final Project project, Artifact artifact, SensorContext context) { Project depWithBranch = Project.createFromMavenIds(artifact.getGroupId(), artifact.getArtifactId(), project.getBranch()); Resource result = context.getResource(depWithBranch); if (result == null || !((Project) result).getAnalysisVersion().equals(artifact.getBaseVersion())) { Library lib = Library.createFromMavenIds(artifact.getGroupId(), artifact.getArtifactId(), artifact.getBaseVersion()); - context.saveResource(lib); + index.addResource(lib); + resourcePersister.persist(); result = context.getResource(lib); } return result; diff --git a/sonar-batch/src/main/java/org/sonar/batch/index/ResourcePersister.java b/sonar-batch/src/main/java/org/sonar/batch/index/ResourcePersister.java index ffefb356858..6ba96d73dff 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/index/ResourcePersister.java +++ b/sonar-batch/src/main/java/org/sonar/batch/index/ResourcePersister.java @@ -45,7 +45,7 @@ import javax.persistence.Query; import java.util.Date; import java.util.List; -public final class ResourcePersister implements ScanPersister { +public class ResourcePersister implements ScanPersister { private static final String RESOURCE_ID = "resourceId"; private static final String LAST = "last"; diff --git a/sonar-deprecated/src/main/java/org/sonar/api/checks/NoSonarFilter.java b/sonar-deprecated/src/main/java/org/sonar/api/checks/NoSonarFilter.java index ad2e60488ff..7626ce1e018 100644 --- a/sonar-deprecated/src/main/java/org/sonar/api/checks/NoSonarFilter.java +++ b/sonar-deprecated/src/main/java/org/sonar/api/checks/NoSonarFilter.java @@ -22,9 +22,9 @@ package org.sonar.api.checks; import com.google.common.collect.Maps; import org.apache.commons.lang.StringUtils; import org.sonar.api.batch.SensorContext; +import org.sonar.api.issue.Issue; +import org.sonar.api.issue.batch.IssueFilterChain; import org.sonar.api.resources.Resource; -import org.sonar.api.rules.Violation; -import org.sonar.api.rules.ViolationFilter; import java.util.Map; import java.util.Set; @@ -34,9 +34,9 @@ import java.util.Set; * @deprecated in 3.6. Replaced by {@link org.sonar.api.issue.NoSonarFilter} */ @Deprecated -public class NoSonarFilter implements ViolationFilter { +public class NoSonarFilter implements org.sonar.api.issue.batch.IssueFilter { - private final Map> noSonarLinesByResource = Maps.newHashMap(); + private final Map> noSonarLinesByKey = Maps.newHashMap(); private SensorContext context; public NoSonarFilter(SensorContext context) { @@ -48,21 +48,24 @@ public class NoSonarFilter implements ViolationFilter { // Reload resource to handle backward compatibility of resource keys Resource resource = context.getResource(model); if (resource != null) { - noSonarLinesByResource.put(resource, noSonarLines); + noSonarLinesByKey.put(resource.getEffectiveKey(), noSonarLines); } } } @Override - public boolean isIgnored(Violation violation) { - boolean ignored = false; - if (violation.getResource() != null && violation.getLineId() != null) { - Set noSonarLines = noSonarLinesByResource.get(violation.getResource()); - ignored = noSonarLines != null && noSonarLines.contains(violation.getLineId()); - if (ignored && violation.getRule() != null && StringUtils.containsIgnoreCase(violation.getRule().getKey(), "nosonar")) { - ignored = false; + public boolean accept(Issue issue, IssueFilterChain chain) { + boolean accepted = true; + if (issue.line() != null) { + Set noSonarLines = noSonarLinesByKey.get(issue.componentKey()); + accepted = noSonarLines == null || !noSonarLines.contains(issue.line()); + if (!accepted && StringUtils.containsIgnoreCase(issue.ruleKey().rule(), "nosonar")) { + accepted = true; } } - return ignored; + if (accepted) { + accepted = chain.accept(issue); + } + return accepted; } } diff --git a/sonar-deprecated/src/test/java/org/sonar/api/checks/NoSonarFilterTest.java b/sonar-deprecated/src/test/java/org/sonar/api/checks/NoSonarFilterTest.java index 7f4793b77cd..9805fd20907 100644 --- a/sonar-deprecated/src/test/java/org/sonar/api/checks/NoSonarFilterTest.java +++ b/sonar-deprecated/src/test/java/org/sonar/api/checks/NoSonarFilterTest.java @@ -22,14 +22,16 @@ package org.sonar.api.checks; import org.junit.Before; import org.junit.Test; import org.sonar.api.batch.SensorContext; +import org.sonar.api.issue.Issue; +import org.sonar.api.issue.batch.IssueFilterChain; import org.sonar.api.resources.File; -import org.sonar.api.rules.Rule; -import org.sonar.api.rules.Violation; +import org.sonar.api.rule.RuleKey; import java.util.HashSet; import java.util.Set; import static org.fest.assertions.Assertions.assertThat; +import static org.mockito.Matchers.isA; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -38,10 +40,13 @@ public class NoSonarFilterTest { private SensorContext sensorContext = mock(SensorContext.class); NoSonarFilter filter = new NoSonarFilter(sensorContext); private File javaFile; + IssueFilterChain chain = mock(IssueFilterChain.class); @Before public void prepare() { + when(chain.accept(isA(Issue.class))).thenReturn(true); javaFile = new File("org.foo.Bar"); + javaFile.setEffectiveKey("struts:org.apache.Action"); when(sensorContext.getResource(javaFile)).thenReturn(javaFile); } @@ -52,17 +57,18 @@ public class NoSonarFilterTest { noSonarLines.add(55); filter.addResource(javaFile, noSonarLines); + Issue issue = mock(Issue.class); + when(issue.componentKey()).thenReturn("struts:org.apache.Action"); + when(issue.ruleKey()).thenReturn(RuleKey.of("squid", "Foo")); + // violation on class - assertThat(filter.isIgnored(new Violation(null, javaFile))).isFalse(); + assertThat(filter.accept(issue, chain)).isTrue(); // violation on lines - assertThat(filter.isIgnored(new Violation(null, javaFile).setLineId(30))).isFalse(); - assertThat(filter.isIgnored(new Violation(null, javaFile).setLineId(31))).isTrue(); - } - - @Test - public void doNotIgnoreWhenNotFoundInSquid() { - assertThat(filter.isIgnored(new Violation(null, javaFile).setLineId(30))).isFalse(); + when(issue.line()).thenReturn(30); + assertThat(filter.accept(issue, chain)).isTrue(); + when(issue.line()).thenReturn(31); + assertThat(filter.accept(issue, chain)).isFalse(); } @Test @@ -73,8 +79,12 @@ public class NoSonarFilterTest { noSonarLines.add(31); filter.addResource(javaFile, noSonarLines); - Rule noSonarRule = new Rule("squid", "NoSonarCheck"); - assertThat(filter.isIgnored(new Violation(noSonarRule, javaFile).setLineId(31))).isFalse(); + Issue issue = mock(Issue.class); + when(issue.componentKey()).thenReturn("struts:org.apache.Action"); + when(issue.ruleKey()).thenReturn(RuleKey.of("squid", "NoSonarCheck")); + + when(issue.line()).thenReturn(31); + assertThat(filter.accept(issue, chain)).isTrue(); } } -- 2.39.5