From 9ea5cad94dcaaad0bc1e4c2466c39970b77b67c2 Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Mon, 25 Feb 2013 09:51:18 +0100 Subject: [PATCH] SONAR-3583 do not fail if violation has no rule id --- .../org/sonar/batch/index/DefaultIndex.java | 20 +++++++-- .../sonar/batch/index/DefaultIndexTest.java | 43 +++++++++++++++---- 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> 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 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"); -- 2.39.5