SONAR-3583 do not fail if violation has no rule id

2 files changed, 51 insertions, 12 deletions

diff --git a/sonar-batch/src/main/java/org/sonar/batch/index/DefaultIndex.java b/sonar-batch/src/main/java/org/sonar/batch/index/DefaultIndex.java
index 8e6067ef39b..1d42fb3a8db 100644
--- a/sonar-batch/src/main/java/org/sonar/batch/index/DefaultIndex.java
+++ b/sonar-batch/src/main/java/org/sonar/batch/index/DefaultIndex.java
@@ -44,6 +44,8 @@ import org.sonar.api.resources.Resource;
 import org.sonar.api.resources.ResourceUtils;
 import org.sonar.api.resources.Scopes;
 import org.sonar.api.rules.ActiveRule;
+import org.sonar.api.rules.Rule;
+import org.sonar.api.rules.RuleFinder;
 import org.sonar.api.rules.Violation;
 import org.sonar.api.utils.SonarException;
 import org.sonar.api.violations.ViolationQuery;
@@ -70,6 +72,7 @@ public class DefaultIndex extends SonarIndex {
   private PersistenceManager persistence;
   private DefaultResourceCreationLock lock;
   private MetricFinder metricFinder;
+  private RuleFinder ruleFinder;
   private ScanGraph graph;
 
   // filters
@@ -84,11 +87,12 @@ public class DefaultIndex extends SonarIndex {
   private Map<Resource, Map<Resource, Dependency>> incomingDependenciesByResource = Maps.newHashMap();
   private ProjectTree projectTree;
 
-  public DefaultIndex(PersistenceManager persistence, DefaultResourceCreationLock lock, ProjectTree projectTree, MetricFinder metricFinder, ScanGraph graph) {
+  public DefaultIndex(PersistenceManager persistence, DefaultResourceCreationLock lock, ProjectTree projectTree, MetricFinder metricFinder, RuleFinder ruleFinder, ScanGraph graph) {
     this.persistence = persistence;
     this.lock = lock;
     this.projectTree = projectTree;
     this.metricFinder = metricFinder;
+    this.ruleFinder = ruleFinder;
     this.graph = graph;
   }
 
@@ -352,11 +356,21 @@ public class DefaultIndex extends SonarIndex {
       throw new IllegalArgumentException("Violations are only supported on files, directories and project");
     }
 
-    if (violation.getRule() == null || violation.getRule().getId() == null) {
-      LOG.warn("Rule does not exist (it is null or its ID is null): ignoring violation {}", violation);
+    Rule rule = violation.getRule();
+    if (rule == null) {
+      LOG.warn("Rule is null. Ignoring violation {}", violation);
       return;
     }
 
+    if (rule.getId()==null) {
+      Rule persistedRule = ruleFinder.findByKey(rule.getRepositoryKey(), rule.getKey());
+      if (persistedRule == null) {
+        LOG.warn("Rule does not exist. Ignoring violation {}", violation);
+        return;
+      }
+      violation.setRule(persistedRule);
+    }
+
     Bucket bucket = checkIndexed(resource);
     if (bucket == null || bucket.isExcluded()) {
       return;
diff --git a/sonar-batch/src/test/java/org/sonar/batch/index/DefaultIndexTest.java b/sonar-batch/src/test/java/org/sonar/batch/index/DefaultIndexTest.java
index 8ea5f04e9c1..030588c8025 100644
--- a/sonar-batch/src/test/java/org/sonar/batch/index/DefaultIndexTest.java
+++ b/sonar-batch/src/test/java/org/sonar/batch/index/DefaultIndexTest.java
@@ -19,13 +19,8 @@
  */
 package org.sonar.batch.index;
 
-import static org.hamcrest.Matchers.nullValue;
-import static org.hamcrest.core.Is.is;
-import static org.junit.Assert.assertThat;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
-
 import org.apache.commons.lang.StringUtils;
+import org.hamcrest.Matchers;
 import org.junit.Before;
 import org.junit.Test;
 import org.sonar.api.batch.ResourceFilter;
@@ -45,6 +40,7 @@ import org.sonar.api.resources.Project;
 import org.sonar.api.resources.Qualifiers;
 import org.sonar.api.resources.Resource;
 import org.sonar.api.rules.Rule;
+import org.sonar.api.rules.RuleFinder;
 import org.sonar.api.rules.Violation;
 import org.sonar.api.utils.SonarException;
 import org.sonar.api.violations.ViolationQuery;
@@ -54,19 +50,29 @@ import org.sonar.batch.ResourceFilters;
 import org.sonar.batch.ViolationFilters;
 import org.sonar.core.component.ScanGraph;
 
+import java.util.List;
+
+import static org.hamcrest.Matchers.nullValue;
+import static org.hamcrest.core.Is.is;
+import static org.junit.Assert.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
 public class DefaultIndexTest {
 
   private DefaultIndex index = null;
   private DefaultResourceCreationLock lock;
   private Rule rule;
+  private RuleFinder ruleFinder;
 
   @Before
   public void createIndex() {
     lock = new DefaultResourceCreationLock(new Settings());
     MetricFinder metricFinder = mock(MetricFinder.class);
     when(metricFinder.findByKey("ncloc")).thenReturn(CoreMetrics.NCLOC);
+    ruleFinder = mock(RuleFinder.class);
 
-    index = new DefaultIndex(mock(PersistenceManager.class), lock, mock(ProjectTree.class), metricFinder, mock(ScanGraph.class));
+    index = new DefaultIndex(mock(PersistenceManager.class), lock, mock(ProjectTree.class), metricFinder, ruleFinder, mock(ScanGraph.class));
     Project project = new Project("project");
 
     ResourceFilter filter = new ResourceFilter() {
@@ -79,7 +85,7 @@ public class DefaultIndexTest {
     rule = Rule.create("repoKey", "ruleKey", "Rule");
     rule.setId(1);
     rulesProfile.activateRule(rule, null);
-    index.setCurrentProject(project, new ResourceFilters(new ResourceFilter[] {filter}), new ViolationFilters(), rulesProfile);
+    index.setCurrentProject(project, new ResourceFilters(new ResourceFilter[]{filter}), new ViolationFilters(), rulesProfile);
     index.doStart(project);
   }
 
@@ -207,7 +213,26 @@ public class DefaultIndexTest {
    * See https://jira.codehaus.org/browse/SONAR-3583
    */
   @Test
-  public void shouldNotFailWhenSavingViolationOnRuleThatDoesNotExistInDB() {
+  public void should_support_violations_with_missing_rule_id() {
+    Rule ruleWithoutId = Rule.create("repoKey", "ruleKey", "Rule");
+    Rule ruleWithId = Rule.create("repoKey", "ruleKey", "Rule");
+    ruleWithId.setId(123);
+    when(ruleFinder.findByKey("repoKey", "ruleKey")).thenReturn(ruleWithId);
+
+    File file = new File("org/foo/Bar.java");
+    Violation violation = Violation.create(ruleWithoutId, file);
+    index.addViolation(violation);
+
+    List<Violation> violations = index.getViolations(file);
+    assertThat(violations.size(), is(1));
+    assertThat(violations.get(0).getRule().getId(), Matchers.is(123));
+  }
+
+  /**
+   * See https://jira.codehaus.org/browse/SONAR-3583
+   */
+  @Test
+  public void should_ignore_violation_on_unknown_rules() {
     Rule ruleWithoutID = Rule.create("repoKey", "ruleKey", "Rule");
 
     File file = new File("org/foo/Bar.java");