From b2885e93ebd9e6042d9b3f7281c2ddc0ad91a949 Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Tue, 4 Feb 2014 17:06:28 +0100 Subject: [PATCH] SONAR-4785 When a plugin creates an issue without a message, the issue message should be replaced in the DB by the rule name --- .../org/sonar/batch/issue/ModuleIssues.java | 22 ++++- .../sonar/batch/issue/ModuleIssuesTest.java | 87 ++++++++++++++++--- 2 files changed, 95 insertions(+), 14 deletions(-) diff --git a/sonar-batch/src/main/java/org/sonar/batch/issue/ModuleIssues.java b/sonar-batch/src/main/java/org/sonar/batch/issue/ModuleIssues.java index 9e5d7f4e95f..22f35ae3837 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/issue/ModuleIssues.java +++ b/sonar-batch/src/main/java/org/sonar/batch/issue/ModuleIssues.java @@ -19,12 +19,16 @@ */ package org.sonar.batch.issue; +import com.google.common.base.Strings; import org.sonar.api.issue.internal.DefaultIssue; import org.sonar.api.profiles.RulesProfile; import org.sonar.api.resources.Project; import org.sonar.api.rule.RuleKey; 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.MessageException; import org.sonar.batch.technicaldebt.TechnicalDebtCalculator; import org.sonar.core.issue.DefaultIssueBuilder; @@ -40,13 +44,15 @@ public class ModuleIssues { private final Project project; private final IssueFilters filters; private final TechnicalDebtCalculator technicalDebtCalculator; + private final RuleFinder ruleFinder; - public ModuleIssues(RulesProfile qProfile, IssueCache cache, Project project, IssueFilters filters, TechnicalDebtCalculator technicalDebtCalculator) { + public ModuleIssues(RulesProfile qProfile, IssueCache cache, Project project, IssueFilters filters, TechnicalDebtCalculator technicalDebtCalculator, RuleFinder ruleFinder) { this.qProfile = qProfile; this.cache = cache; this.project = project; this.filters = filters; this.technicalDebtCalculator = technicalDebtCalculator; + this.ruleFinder = ruleFinder; } public boolean initAndAddIssue(DefaultIssue issue) { @@ -70,13 +76,20 @@ public class ModuleIssues { } private boolean initAndAddIssue(DefaultIssue issue, @Nullable Violation violation) { - // TODO fail fast : if rule does not exist - - ActiveRule activeRule = qProfile.getActiveRule(issue.ruleKey().repository(), issue.ruleKey().rule()); + RuleKey ruleKey = issue.ruleKey(); + Rule rule = ruleFinder.findByKey(ruleKey); + if (rule == null) { + throw MessageException.of(String.format("The rule '%s' does not exists.", ruleKey)); + } + ActiveRule activeRule = qProfile.getActiveRule(ruleKey.repository(), ruleKey.rule()); if (activeRule == null || activeRule.getRule() == null) { // rule does not exist or is not enabled -> ignore the issue return false; } + + if (Strings.isNullOrEmpty(issue.message())) { + issue.setMessage(rule.getName()); + } issue.setCreationDate(project.getAnalysisDate()); issue.setUpdateDate(project.getAnalysisDate()); if (issue.severity() == null) { @@ -90,4 +103,5 @@ public class ModuleIssues { } return false; } + } diff --git a/sonar-batch/src/test/java/org/sonar/batch/issue/ModuleIssuesTest.java b/sonar-batch/src/test/java/org/sonar/batch/issue/ModuleIssuesTest.java index 2b3ae1c86d6..712c49f80d8 100644 --- a/sonar-batch/src/test/java/org/sonar/batch/issue/ModuleIssuesTest.java +++ b/sonar-batch/src/test/java/org/sonar/batch/issue/ModuleIssuesTest.java @@ -22,7 +22,10 @@ package org.sonar.batch.issue; import org.apache.commons.lang.time.DateUtils; import org.junit.Before; import org.junit.Test; +import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.runners.MockitoJUnitRunner; import org.sonar.api.issue.internal.DefaultIssue; import org.sonar.api.issue.internal.WorkDayDuration; import org.sonar.api.profiles.RulesProfile; @@ -31,41 +34,72 @@ import org.sonar.api.resources.Project; import org.sonar.api.resources.Resource; import org.sonar.api.rule.RuleKey; import org.sonar.api.rule.Severity; -import org.sonar.api.rules.ActiveRule; -import org.sonar.api.rules.Rule; -import org.sonar.api.rules.RulePriority; -import org.sonar.api.rules.Violation; +import org.sonar.api.rules.*; +import org.sonar.api.utils.MessageException; import org.sonar.batch.technicaldebt.TechnicalDebtCalculator; import java.util.Calendar; import java.util.Date; import static org.fest.assertions.Assertions.assertThat; +import static org.fest.assertions.Fail.fail; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.*; +@RunWith(MockitoJUnitRunner.class) public class ModuleIssuesTest { static final RuleKey SQUID_RULE_KEY = RuleKey.of("squid", "AvoidCycle"); - IssueCache cache = mock(IssueCache.class); - RulesProfile qProfile = mock(RulesProfile.class); - Project project = mock(Project.class); - IssueFilters filters = mock(IssueFilters.class); - TechnicalDebtCalculator technicalDebtCalculator = mock(TechnicalDebtCalculator.class); - ModuleIssues moduleIssues = new ModuleIssues(qProfile, cache, project, filters, technicalDebtCalculator); + @Mock + IssueCache cache; + + @Mock + RulesProfile qProfile; + + @Mock + Project project; + + @Mock + IssueFilters filters; + + @Mock + TechnicalDebtCalculator technicalDebtCalculator; + + @Mock + RuleFinder ruleFinder; + + ModuleIssues moduleIssues; @Before public void setUp() { when(project.getAnalysisDate()).thenReturn(new Date()); when(project.getEffectiveKey()).thenReturn("org.apache:struts-core"); + + moduleIssues = new ModuleIssues(qProfile, cache, project, filters, technicalDebtCalculator, ruleFinder); + } + + @Test + public void fail_on_unknown_rule() throws Exception { + when(ruleFinder.findByKey(SQUID_RULE_KEY)).thenReturn(null); + DefaultIssue issue = new DefaultIssue().setRuleKey(SQUID_RULE_KEY); + + try { + moduleIssues.initAndAddIssue(issue); + fail(); + } catch (Exception e) { + assertThat(e).isInstanceOf(MessageException.class); + } + + verifyZeroInteractions(cache); } @Test public void ignore_null_active_rule() throws Exception { when(qProfile.getActiveRule(anyString(), anyString())).thenReturn(null); + when(ruleFinder.findByKey(SQUID_RULE_KEY)).thenReturn(Rule.create("squid", "AvoidCycle")); DefaultIssue issue = new DefaultIssue().setRuleKey(SQUID_RULE_KEY); boolean added = moduleIssues.initAndAddIssue(issue); @@ -79,6 +113,7 @@ public class ModuleIssuesTest { ActiveRule activeRule = mock(ActiveRule.class); when(activeRule.getRule()).thenReturn(null); when(qProfile.getActiveRule(anyString(), anyString())).thenReturn(activeRule); + when(ruleFinder.findByKey(SQUID_RULE_KEY)).thenReturn(Rule.create("squid", "AvoidCycle")); DefaultIssue issue = new DefaultIssue().setRuleKey(SQUID_RULE_KEY); boolean added = moduleIssues.initAndAddIssue(issue); @@ -94,6 +129,7 @@ public class ModuleIssuesTest { when(activeRule.getRule()).thenReturn(rule); when(activeRule.getSeverity()).thenReturn(RulePriority.INFO); when(qProfile.getActiveRule("squid", "AvoidCycle")).thenReturn(activeRule); + when(ruleFinder.findByKey(SQUID_RULE_KEY)).thenReturn(Rule.create("squid", "AvoidCycle")); Date analysisDate = new Date(); when(project.getAnalysisDate()).thenReturn(analysisDate); @@ -120,6 +156,7 @@ public class ModuleIssuesTest { when(activeRule.getRule()).thenReturn(rule); when(activeRule.getSeverity()).thenReturn(RulePriority.INFO); when(qProfile.getActiveRule("squid", "AvoidCycle")).thenReturn(activeRule); + when(ruleFinder.findByKey(SQUID_RULE_KEY)).thenReturn(Rule.create("squid", "AvoidCycle")); Date analysisDate = new Date(); when(project.getAnalysisDate()).thenReturn(analysisDate); @@ -134,6 +171,33 @@ public class ModuleIssuesTest { assertThat(argument.getValue().creationDate()).isEqualTo(DateUtils.truncate(analysisDate, Calendar.SECOND)); } + @Test + public void use_rule_name_if_no_message() throws Exception { + Rule rule = Rule.create("squid", "AvoidCycle"); + ActiveRule activeRule = mock(ActiveRule.class); + when(activeRule.getRule()).thenReturn(rule); + when(activeRule.getSeverity()).thenReturn(RulePriority.INFO); + when(qProfile.getActiveRule("squid", "AvoidCycle")).thenReturn(activeRule); + when(ruleFinder.findByKey(SQUID_RULE_KEY)).thenReturn(Rule.create("squid", "AvoidCycle").setName("Avoid Cycle")); + + Date analysisDate = new Date(); + when(project.getAnalysisDate()).thenReturn(analysisDate); + + DefaultIssue issue = new DefaultIssue() + .setKey("ABCDE") + .setRuleKey(SQUID_RULE_KEY) + .setSeverity(Severity.CRITICAL) + .setMessage(""); + when(filters.accept(issue, null)).thenReturn(true); + + boolean added = moduleIssues.initAndAddIssue(issue); + + assertThat(added).isTrue(); + ArgumentCaptor argument = ArgumentCaptor.forClass(DefaultIssue.class); + verify(cache).put(argument.capture()); + assertThat(argument.getValue().message()).isEqualTo("Avoid Cycle"); + } + @Test public void add_deprecated_violation() throws Exception { Rule rule = Rule.create("squid", "AvoidCycle"); @@ -147,6 +211,7 @@ public class ModuleIssuesTest { when(activeRule.getRule()).thenReturn(rule); when(activeRule.getSeverity()).thenReturn(RulePriority.INFO); when(qProfile.getActiveRule("squid", "AvoidCycle")).thenReturn(activeRule); + when(ruleFinder.findByKey(SQUID_RULE_KEY)).thenReturn(Rule.create("squid", "AvoidCycle")); when(filters.accept(any(DefaultIssue.class), eq(violation))).thenReturn(true); boolean added = moduleIssues.initAndAddViolation(violation); @@ -170,6 +235,7 @@ public class ModuleIssuesTest { when(activeRule.getRule()).thenReturn(rule); when(activeRule.getSeverity()).thenReturn(RulePriority.INFO); when(qProfile.getActiveRule("squid", "AvoidCycle")).thenReturn(activeRule); + when(ruleFinder.findByKey(SQUID_RULE_KEY)).thenReturn(Rule.create("squid", "AvoidCycle")); DefaultIssue issue = new DefaultIssue() .setKey("ABCDE") @@ -191,6 +257,7 @@ public class ModuleIssuesTest { when(activeRule.getRule()).thenReturn(rule); when(activeRule.getSeverity()).thenReturn(RulePriority.INFO); when(qProfile.getActiveRule("squid", "AvoidCycle")).thenReturn(activeRule); + when(ruleFinder.findByKey(SQUID_RULE_KEY)).thenReturn(Rule.create("squid", "AvoidCycle")); Date analysisDate = new Date(); when(project.getAnalysisDate()).thenReturn(analysisDate); -- 2.39.5