diff options
4 files changed, 37 insertions, 11 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 diff --git a/sonar-core/src/main/java/org/sonar/core/issue/IssueNotifications.java b/sonar-core/src/main/java/org/sonar/core/issue/IssueNotifications.java index d084741ac90..96e012b8e49 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/IssueNotifications.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/IssueNotifications.java @@ -68,12 +68,13 @@ public class IssueNotifications implements BatchComponent, ServerComponent { } @CheckForNull - public List<Notification> sendChanges(DefaultIssue issue, IssueChangeContext context, Rule rule, Component project, @Nullable Component component) { + public List<Notification> sendChanges(DefaultIssue issue, IssueChangeContext context, @Nullable Rule rule, Component project, @Nullable Component component) { return sendChanges(issue, context, rule, project, component, null); } @CheckForNull - public List<Notification> sendChanges(DefaultIssue issue, IssueChangeContext context, Rule rule, Component project, @Nullable Component component, @Nullable String comment) { + public List<Notification> sendChanges(DefaultIssue issue, IssueChangeContext context, @Nullable Rule rule, Component project, @Nullable Component component, + @Nullable String comment) { Map<DefaultIssue, Rule> issues = Maps.newHashMap(); issues.put(issue, rule); return sendChanges(issues, context, project, component, comment); @@ -95,7 +96,7 @@ public class IssueNotifications implements BatchComponent, ServerComponent { } @CheckForNull - private Notification createChangeNotification(DefaultIssue issue, IssueChangeContext context, Rule rule, Component project, + private Notification createChangeNotification(DefaultIssue issue, IssueChangeContext context, @Nullable Rule rule, Component project, @Nullable Component component, @Nullable String comment) { Notification notification = null; if (comment != null || issue.mustSendNotifications()) { |