SONAR-6014 Fix regression when persisting dependencies and start dropping violations

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 3462478091677667e50db5b133abfe36e66d9035..a6de27c9ef2080f51f09204cd68f1d2b1e9dee20 100644 (file)
--- 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 ffefb3568582c59b3c0a5eec5557e1f87269999b..6ba96d73dffdc8b5321d77b6ad0d445b5e37c457 100644 (file)
--- 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 ad2e60488ff217529cfa7f67ec24bbf40b795049..7626ce1e01836a389d9506907987c73f1d35c6a7 100644 (file)
--- 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<Resource, Set<Integer>> noSonarLinesByResource = Maps.newHashMap();
+  private final Map<String, Set<Integer>> 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<Integer> 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<Integer> 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 7f4793b77cde3ed5362a9a2827821906c6ed8fd3..9805fd20907222df32c59c3e4ca167b588d2c439 100644 (file)
--- 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();
 
   }
 }