]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-4785 Fail if rule has no name and issue has no message
authorJulien Lancelot <julien.lancelot@sonarsource.com>
Tue, 4 Feb 2014 16:25:38 +0000 (17:25 +0100)
committerJulien Lancelot <julien.lancelot@sonarsource.com>
Tue, 4 Feb 2014 16:25:38 +0000 (17:25 +0100)
sonar-batch/src/main/java/org/sonar/batch/issue/ModuleIssues.java
sonar-batch/src/test/java/org/sonar/batch/issue/ModuleIssuesTest.java
sonar-core/src/main/java/org/sonar/core/measure/MeasureFilter.java
sonar-server/src/main/java/org/sonar/server/issue/ws/IssueShowWsHandler.java

index 22f35ae38370abecc0e0475d4bfb39dd460f833e..e1272f3b91a20a7bc7f5831d96a468f54b913f31 100644 (file)
@@ -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
index 712c49f80d85aa42829472eaf8429c03c4b7fa67..667de9f373a60dd46fa01acdaec09d359d2435f8 100644 (file)
@@ -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);
index 18fe67ceccbb7b788d2dfec216e951a3d3a3d20a..a4d99281d29265e4af2ad5d305987161c7ad0952 100644 (file)
@@ -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
index 2e8678569539a553d0aa38bc879e61c74a1c3df9..1fa32a29f6af3d4f41607ef1d81e7beea8967c50 100644 (file)
@@ -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();