SONAR-5531 Fix bug when adding a comment on an issue linked on a removed rule

3 files changed, 33 insertions, 8 deletions

diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/IssueService.java b/server/sonar-server/src/main/java/org/sonar/server/issue/IssueService.java
index 8a8ebbe5934..3344994ce30 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/issue/IssueService.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/issue/IssueService.java
@@ -247,7 +247,10 @@ public class IssueService implements ServerComponent {
       if (!ruleKey.isManual()) {
         throw new IllegalArgumentException("Issues can be created only on rules marked as 'manual': " + ruleKey);
       }
-      Rule rule = getRuleByKey(ruleKey);
+      Rule rule = getNullableRuleByKey(ruleKey);
+      if (rule == null) {
+        throw new IllegalArgumentException("Unknown rule: " + ruleKey);
+      }
 
       DefaultIssue issue = new DefaultIssueBuilder()
         .componentKey(component.getKey())
@@ -319,7 +322,7 @@ public class IssueService implements ServerComponent {
     }
     issueStorage.save(session, issue);
     issueNotifications.sendChanges(issue, context,
-      getRuleByKey(issue.ruleKey()),
+      getNullableRuleByKey(issue.ruleKey()),
       dbClient.componentDao().getByKey(session, projectKey),
       dbClient.componentDao().getNullableByKey(session, issue.componentKey()),
       comment);
@@ -328,13 +331,10 @@ public class IssueService implements ServerComponent {
 
   /**
    * Should use {@link org.sonar.server.rule.RuleService#getByKey(org.sonar.api.rule.RuleKey)}, but it's not possible as IssueNotifications is still used by the batch.
+   * Can be null for removed rules
    */
-  private Rule getRuleByKey(RuleKey ruleKey) {
-    Rule rule = ruleFinder.findByKey(ruleKey);
-    if (rule == null) {
-      throw new IllegalArgumentException("Unknown rule: " + ruleKey);
-    }
-    return rule;
+  private Rule getNullableRuleByKey(RuleKey ruleKey) {
+    return ruleFinder.findByKey(ruleKey);
   }
 
   public org.sonar.server.search.Result<Issue> search(IssueQuery query, QueryContext options) {
diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/IssueCommentServiceMediumTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/IssueCommentServiceMediumTest.java
index cb731587dc9..bfdf88108bf 100644
--- a/server/sonar-server/src/test/java/org/sonar/server/issue/IssueCommentServiceMediumTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/issue/IssueCommentServiceMediumTest.java
@@ -24,7 +24,10 @@ import org.junit.After;
 import org.junit.Before;
 import org.junit.ClassRule;
 import org.junit.Test;
+import org.sonar.api.issue.Issue;
 import org.sonar.api.issue.internal.DefaultIssueComment;
+import org.sonar.api.rule.RuleKey;
+import org.sonar.api.rule.RuleStatus;
 import org.sonar.api.security.DefaultGroups;
 import org.sonar.api.web.UserRole;
 import org.sonar.core.component.ComponentDto;
@@ -109,4 +112,20 @@ public class IssueCommentServiceMediumTest {
     assertThat(comments.get(0).markdownText()).isEqualTo("my comment");
   }
 
+  @Test
+  public void add_comment_on_removed_issue() throws Exception {
+    RuleDto removedRule = RuleTesting.newDto(RuleKey.of("removed", "rule")).setStatus(RuleStatus.REMOVED);
+    tester.get(RuleDao.class).insert(session, removedRule);
+
+    IssueDto issue = IssueTesting.newDto(removedRule, file, project).setStatus(Issue.STATUS_CLOSED).setResolution(Issue.RESOLUTION_REMOVED);
+    tester.get(IssueDao.class).insert(session, issue);
+    session.commit();
+
+    service.addComment(issue.getKey(), "my comment", MockUserSession.get());
+
+    List<DefaultIssueComment> comments = service.findComments(issue.getKey());
+    assertThat(comments).hasSize(1);
+    assertThat(comments.get(0).markdownText()).isEqualTo("my comment");
+  }
+
 }
diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/IssueServiceMediumTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/IssueServiceMediumTest.java
index b935d1fe797..c7141e421e4 100644
--- a/server/sonar-server/src/test/java/org/sonar/server/issue/IssueServiceMediumTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/issue/IssueServiceMediumTest.java
@@ -28,6 +28,7 @@ import org.sonar.api.issue.DefaultTransitions;
 import org.sonar.api.issue.Issue;
 import org.sonar.api.resources.Qualifiers;
 import org.sonar.api.resources.Scopes;
+import org.sonar.api.rule.RuleKey;
 import org.sonar.api.rule.Severity;
 import org.sonar.api.security.DefaultGroups;
 import org.sonar.api.utils.DateUtils;
@@ -342,6 +343,11 @@ public class IssueServiceMediumTest {
     }
   }
 
+  @Test(expected = IllegalArgumentException.class)
+  public void fail_create_manual_issue_if_rule_does_not_exists() {
+    service.createManualIssue(file.key(), RuleKey.of("rule", "unknown"), 10, "Fix it", null, 2d);
+  }
+
   @Test(expected = ForbiddenException.class)
   public void fail_create_manual_issue_if_not_having_required_role() {
     // User has not the 'user' role on the project