From: Julien Lancelot Date: Tue, 4 Feb 2014 16:25:38 +0000 (+0100) Subject: SONAR-4785 Fail if rule has no name and issue has no message X-Git-Tag: 4.2~264 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=7db29fdd2081290ae1a8bafdc475d30e3692adbb;p=sonarqube.git SONAR-4785 Fail if rule has no name and issue has no message --- 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 22f35ae3837..e1272f3b91a 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 @@ -79,8 +79,12 @@ public class ModuleIssues { RuleKey ruleKey = issue.ruleKey(); Rule rule = ruleFinder.findByKey(ruleKey); if (rule == null) { - throw MessageException.of(String.format("The rule '%s' does not exists.", ruleKey)); + throw MessageException.of(String.format("The rule '%s' does not exist.", ruleKey)); } + if (Strings.isNullOrEmpty(rule.getName()) && Strings.isNullOrEmpty(issue.message())) { + throw MessageException.of(String.format("The rule '%s' has no name and the related issue has no message.", 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 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 712c49f80d8..667de9f373a 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 @@ -52,6 +52,7 @@ import static org.mockito.Mockito.*; public class ModuleIssuesTest { static final RuleKey SQUID_RULE_KEY = RuleKey.of("squid", "AvoidCycle"); + static final Rule SQUID_RULE = Rule.create("squid", "AvoidCycle").setName("Avoid Cycle"); @Mock IssueCache cache; @@ -96,10 +97,25 @@ public class ModuleIssuesTest { verifyZeroInteractions(cache); } + @Test + public void fail_if_rule_has_no_name_and_issue_has_no_message() throws Exception { + when(ruleFinder.findByKey(SQUID_RULE_KEY)).thenReturn(Rule.create("squid", "AvoidCycle")); + DefaultIssue issue = new DefaultIssue().setRuleKey(SQUID_RULE_KEY).setMessage(""); + + 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")); + when(ruleFinder.findByKey(SQUID_RULE_KEY)).thenReturn(SQUID_RULE); DefaultIssue issue = new DefaultIssue().setRuleKey(SQUID_RULE_KEY); boolean added = moduleIssues.initAndAddIssue(issue); @@ -113,7 +129,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")); + when(ruleFinder.findByKey(SQUID_RULE_KEY)).thenReturn(SQUID_RULE); DefaultIssue issue = new DefaultIssue().setRuleKey(SQUID_RULE_KEY); boolean added = moduleIssues.initAndAddIssue(issue); @@ -124,12 +140,12 @@ public class ModuleIssuesTest { @Test public void add_issue_to_cache() throws Exception { - Rule rule = Rule.create("squid", "AvoidCycle"); + Rule rule = SQUID_RULE; 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")); + when(ruleFinder.findByKey(SQUID_RULE_KEY)).thenReturn(rule); Date analysisDate = new Date(); when(project.getAnalysisDate()).thenReturn(analysisDate); @@ -151,12 +167,12 @@ public class ModuleIssuesTest { @Test public void use_severity_from_active_rule_if_no_severity() throws Exception { - Rule rule = Rule.create("squid", "AvoidCycle"); + Rule rule = SQUID_RULE; 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")); + when(ruleFinder.findByKey(SQUID_RULE_KEY)).thenReturn(rule); Date analysisDate = new Date(); when(project.getAnalysisDate()).thenReturn(analysisDate); @@ -173,12 +189,12 @@ public class ModuleIssuesTest { @Test public void use_rule_name_if_no_message() throws Exception { - Rule rule = Rule.create("squid", "AvoidCycle"); + Rule rule = SQUID_RULE; 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")); + when(ruleFinder.findByKey(SQUID_RULE_KEY)).thenReturn(rule); Date analysisDate = new Date(); when(project.getAnalysisDate()).thenReturn(analysisDate); @@ -200,7 +216,7 @@ public class ModuleIssuesTest { @Test public void add_deprecated_violation() throws Exception { - Rule rule = Rule.create("squid", "AvoidCycle"); + Rule rule = SQUID_RULE; Resource resource = new JavaFile("org.struts.Action").setEffectiveKey("struts:org.struts.Action"); Violation violation = new Violation(rule, resource); violation.setLineId(42); @@ -211,7 +227,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(ruleFinder.findByKey(SQUID_RULE_KEY)).thenReturn(rule); when(filters.accept(any(DefaultIssue.class), eq(violation))).thenReturn(true); boolean added = moduleIssues.initAndAddViolation(violation); @@ -230,12 +246,12 @@ public class ModuleIssuesTest { @Test public void filter_issue() throws Exception { - Rule rule = Rule.create("squid", "AvoidCycle"); + Rule rule = SQUID_RULE; 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")); + when(ruleFinder.findByKey(SQUID_RULE_KEY)).thenReturn(rule); DefaultIssue issue = new DefaultIssue() .setKey("ABCDE") @@ -252,12 +268,12 @@ public class ModuleIssuesTest { @Test public void set_remediation_cost() throws Exception { - Rule rule = Rule.create("squid", "AvoidCycle"); + Rule rule = SQUID_RULE; 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")); + when(ruleFinder.findByKey(SQUID_RULE_KEY)).thenReturn(rule); Date analysisDate = new Date(); when(project.getAnalysisDate()).thenReturn(analysisDate); diff --git a/sonar-core/src/main/java/org/sonar/core/measure/MeasureFilter.java b/sonar-core/src/main/java/org/sonar/core/measure/MeasureFilter.java index 18fe67ceccb..a4d99281d29 100644 --- a/sonar-core/src/main/java/org/sonar/core/measure/MeasureFilter.java +++ b/sonar-core/src/main/java/org/sonar/core/measure/MeasureFilter.java @@ -37,7 +37,6 @@ public class MeasureFilter { // conditions on resources private String baseResourceKey; - private Long baseResourceId; // only if baseResourceKey or baseResourceId are set private boolean onBaseResourceChildren = false; @@ -187,7 +186,7 @@ public class MeasureFilter { } public boolean isEmpty() { - return resourceQualifiers.isEmpty() && resourceScopes.isEmpty() && StringUtils.isEmpty(baseResourceKey) && baseResourceId == null && !userFavourites; + return resourceQualifiers.isEmpty() && resourceScopes.isEmpty() && StringUtils.isEmpty(baseResourceKey) && !userFavourites; } @VisibleForTesting diff --git a/sonar-server/src/main/java/org/sonar/server/issue/ws/IssueShowWsHandler.java b/sonar-server/src/main/java/org/sonar/server/issue/ws/IssueShowWsHandler.java index 2e867856953..1fa32a29f6a 100644 --- a/sonar-server/src/main/java/org/sonar/server/issue/ws/IssueShowWsHandler.java +++ b/sonar-server/src/main/java/org/sonar/server/issue/ws/IssueShowWsHandler.java @@ -100,6 +100,7 @@ public class IssueShowWsHandler implements RequestHandler { Component component = result.component(issue); Component project = result.project(issue); String actionPlanKey = issue.actionPlanKey(); + ActionPlan actionPlan = result.actionPlan(issue); WorkDayDuration technicalDebt = issue.technicalDebt(); Date updateDate = issue.updateDate(); Date closeDate = issue.closeDate(); @@ -120,8 +121,8 @@ public class IssueShowWsHandler implements RequestHandler { .prop("severity", issue.severity()) .prop("author", issue.authorLogin()) .prop("actionPlan", actionPlanKey) + .prop("actionPlanName", actionPlan != null ? actionPlan.name() : null) .prop("debt", technicalDebt != null ? debtFormatter.format(UserSession.get().locale(), technicalDebt) : null) - .prop("actionPlanName", actionPlanKey != null ? result.actionPlan(issue).name() : null) .prop("creationDate", DateUtils.formatDateTime(issue.creationDate())) .prop("fCreationDate", formatDate(issue.creationDate())) .prop("updateDate", updateDate != null ? DateUtils.formatDateTime(updateDate) : null) @@ -138,7 +139,7 @@ public class IssueShowWsHandler implements RequestHandler { private void addCharacteristics(IssueQueryResult result, DefaultIssue issue, JsonWriter json) { Characteristic requirement = technicalDebtManager.findRequirementByRule(result.rule(issue)); // Requirement can be null if it has been disabled - if (requirement != null) { + if (requirement != null && requirement.rootId() != null && requirement.parentId() != null) { Characteristic characteristic = technicalDebtManager.findCharacteristicById(requirement.rootId()); json.prop("characteristic", characteristic != null ? characteristic.name() : null); Characteristic subCharacteristic = technicalDebtManager.findCharacteristicById(requirement.parentId()); @@ -194,15 +195,16 @@ public class IssueShowWsHandler implements RequestHandler { String login = UserSession.get().login(); for (IssueComment comment : issue.comments()) { String userLogin = comment.userLogin(); + User user = userLogin != null ? queryResult.user(userLogin) : null; json .beginObject() .prop("key", comment.key()) - .prop("userName", userLogin != null ? queryResult.user(userLogin).name() : null) + .prop("userName", user != null ? user.name() : null) .prop("raw", comment.markdownText()) .prop("html", Markdown.convertToHtml(comment.markdownText())) .prop("createdAt", DateUtils.formatDateTime(comment.createdAt())) .prop("fCreatedAge", formatAgeDate(comment.createdAt())) - .prop("updatable", login != null && login.equals(comment.userLogin())) + .prop("updatable", login != null && login.equals(userLogin)) .endObject(); } json.endArray();