SONAR-10874 Disable all notifications for hotspots

diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/SendIssueNotificationsStep.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/SendIssueNotificationsStep.java
index ccb59f0d38628e844d41ba02421293ad27bb0f48..214cdc3f825317e15c5700ebe31f39c209e88c1a 100644 (file)
--- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/SendIssueNotificationsStep.java
+++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/SendIssueNotificationsStep.java
@@ -135,10 +135,12 @@ public class SendIssueNotificationsStep implements ComputationStep {
   private void processIssues(NewIssuesStatistics newIssuesStats, CloseableIterator<DefaultIssue> issues, Component project, Map<String, UserDto> usersDtoByUuids) {
     while (issues.hasNext()) {
       DefaultIssue issue = issues.next();
-      if (issue.isNew() && issue.resolution() == null && issue.type() != RuleType.SECURITY_HOTSPOT) {
-        newIssuesStats.add(issue);
-      } else if (issue.isChanged() && issue.mustSendNotifications()) {
-        sendIssueChangeNotification(issue, project, usersDtoByUuids);
+      if (issue.type() != RuleType.SECURITY_HOTSPOT) {
+        if (issue.isNew() && issue.resolution() == null) {
+          newIssuesStats.add(issue);
+        } else if (issue.isChanged() && issue.mustSendNotifications()) {
+          sendIssueChangeNotification(issue, project, usersDtoByUuids);
+        }
       }
     }
   }

diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/SendIssueNotificationsStepTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/SendIssueNotificationsStepTest.java
index 2229f229ee30374883422d7d418d1bb6cc26077d..98ca9bed1f5861a02889b03f6d35adcfad0b29ae 100644 (file)
--- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/SendIssueNotificationsStepTest.java
+++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/SendIssueNotificationsStepTest.java
@@ -63,6 +63,7 @@ import static java.util.Arrays.stream;
 import static java.util.Collections.shuffle;
 import static java.util.stream.Collectors.toList;
 import static java.util.stream.Stream.concat;
+import static org.apache.commons.lang.math.RandomUtils.nextInt;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.ArgumentCaptor.forClass;
 import static org.mockito.ArgumentMatchers.eq;
@@ -394,6 +395,19 @@ public class SendIssueNotificationsStepTest extends BaseStepTest {
     sendIssueChangeNotification(ANALYSE_DATE);
   }
 
+  @Test
+  public void dont_send_issues_change_notification_for_hotspot() {
+    UserDto user = db.users().insertUser();
+    ComponentDto project = newPrivateProjectDto(newOrganizationDto()).setDbKey(PROJECT.getKey()).setLongName(PROJECT.getName());
+    ComponentDto file = newFileDto(project).setDbKey(FILE.getKey()).setLongName(FILE.getName());
+    RuleDefinitionDto ruleDefinitionDto = newRule();
+    DefaultIssue issue = prepareIssue(ANALYSE_DATE, user, project, file, ruleDefinitionDto, RuleType.SECURITY_HOTSPOT);
+
+    underTest.execute();
+
+    verify(notificationService, never()).deliver(any(IssueChangeNotification.class));
+  }
+
   @Test
   public void send_issues_change_notification_even_if_issue_is_backdated() {
     sendIssueChangeNotification(ANALYSE_DATE - FIVE_MINUTES_IN_MS);
@@ -404,11 +418,8 @@ public class SendIssueNotificationsStepTest extends BaseStepTest {
     ComponentDto project = newPrivateProjectDto(newOrganizationDto()).setDbKey(PROJECT.getKey()).setLongName(PROJECT.getName());
     ComponentDto file = newFileDto(project).setDbKey(FILE.getKey()).setLongName(FILE.getName());
     RuleDefinitionDto ruleDefinitionDto = newRule();
-    DefaultIssue issue = newIssue(ruleDefinitionDto, project, file).toDefaultIssue()
-      .setNew(false).setChanged(true).setSendNotifications(true).setCreationDate(new Date(issueCreatedAt)).setAssigneeUuid(user.getUuid());
-    ruleRepository.add(ruleDefinitionDto.getKey()).setName(ruleDefinitionDto.getName());
-    issueCache.newAppender().append(issue).close();
-    when(notificationService.hasProjectSubscribersForTypes(PROJECT.getUuid(), NOTIF_TYPES)).thenReturn(true);
+    RuleType randomTypeExceptHotspot = RuleType.values()[nextInt(RuleType.values().length - 1)];
+    DefaultIssue issue = prepareIssue(issueCreatedAt, user, project, file, ruleDefinitionDto, randomTypeExceptHotspot);
 
     underTest.execute();
 
@@ -425,6 +436,15 @@ public class SendIssueNotificationsStepTest extends BaseStepTest {
     assertThat(issueChangeNotification.getFieldValue("assignee")).isEqualTo(user.getLogin());
   }
 
+  private DefaultIssue prepareIssue(long issueCreatedAt, UserDto user, ComponentDto project, ComponentDto file, RuleDefinitionDto ruleDefinitionDto, RuleType type) {
+    DefaultIssue issue = newIssue(ruleDefinitionDto, project, file).setType(type).toDefaultIssue()
+      .setNew(false).setChanged(true).setSendNotifications(true).setCreationDate(new Date(issueCreatedAt)).setAssigneeUuid(user.getUuid());
+    ruleRepository.add(ruleDefinitionDto.getKey()).setName(ruleDefinitionDto.getName());
+    issueCache.newAppender().append(issue).close();
+    when(notificationService.hasProjectSubscribersForTypes(PROJECT.getUuid(), NOTIF_TYPES)).thenReturn(true);
+    return issue;
+  }
+
   @Test
   public void send_issues_change_notification_on_branch() {
     sendIssueChangeNotificationOnBranch(ANALYSE_DATE);
@@ -442,7 +462,8 @@ public class SendIssueNotificationsStepTest extends BaseStepTest {
     treeRootHolder.setRoot(builder(Type.PROJECT, 2).setKey(branch.getDbKey()).setPublicKey(branch.getKey()).setName(branch.longName()).setUuid(branch.uuid()).addChildren(
       builder(Type.FILE, 11).setKey(file.getDbKey()).setPublicKey(file.getKey()).setName(file.longName()).build()).build());
     RuleDefinitionDto ruleDefinitionDto = newRule();
-    DefaultIssue issue = newIssue(ruleDefinitionDto, branch, file).toDefaultIssue()
+    RuleType randomTypeExceptHotspot = RuleType.values()[nextInt(RuleType.values().length - 1)];
+    DefaultIssue issue = newIssue(ruleDefinitionDto, branch, file).setType(randomTypeExceptHotspot).toDefaultIssue()
       .setNew(false)
       .setChanged(true)
       .setSendNotifications(true)

diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/IssueUpdater.java b/server/sonar-server/src/main/java/org/sonar/server/issue/IssueUpdater.java
index 8b04bb0ade5274c901def6989c4a0c0726aa449d..0b23c24eee09bf45f8844bbe41a3cda4183ed161 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/issue/IssueUpdater.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/issue/IssueUpdater.java
@@ -24,6 +24,7 @@ import java.util.Optional;
 import javax.annotation.Nullable;
 import org.sonar.api.rule.RuleKey;
 import org.sonar.api.rule.RuleStatus;
+import org.sonar.api.rules.RuleType;
 import org.sonar.core.issue.DefaultIssue;
 import org.sonar.core.issue.IssueChangeContext;
 import org.sonar.core.util.stream.MoreCollectors;
@@ -91,18 +92,20 @@ public class IssueUpdater {
   private IssueDto doSaveIssue(DbSession session, DefaultIssue issue, IssueChangeContext context, @Nullable String comment,
     Optional<RuleDefinitionDto> rule, ComponentDto project, ComponentDto component) {
     IssueDto issueDto = issueStorage.save(session, issue);
-    String assigneeUuid = issue.assignee();
-    UserDto assignee = assigneeUuid == null ? null : dbClient.userDao().selectByUuid(session, assigneeUuid);
-    String authorUuid = context.userUuid();
-    UserDto author = authorUuid == null ? null : dbClient.userDao().selectByUuid(session, authorUuid);
-    notificationService.scheduleForSending(new IssueChangeNotification()
-      .setIssue(issue)
-      .setAssignee(assignee)
-      .setChangeAuthor(author)
-      .setRuleName(rule.map(RuleDefinitionDto::getName).orElse(null))
-      .setProject(project)
-      .setComponent(component)
-      .setComment(comment));
+    if (issue.type() != RuleType.SECURITY_HOTSPOT) {
+      String assigneeUuid = issue.assignee();
+      UserDto assignee = assigneeUuid == null ? null : dbClient.userDao().selectByUuid(session, assigneeUuid);
+      String authorUuid = context.userUuid();
+      UserDto author = authorUuid == null ? null : dbClient.userDao().selectByUuid(session, authorUuid);
+      notificationService.scheduleForSending(new IssueChangeNotification()
+        .setIssue(issue)
+        .setAssignee(assignee)
+        .setChangeAuthor(author)
+        .setRuleName(rule.map(RuleDefinitionDto::getName).orElse(null))
+        .setProject(project)
+        .setComponent(component)
+        .setComment(comment));
+    }
     return issueDto;
   }
 

diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/SetSeverityAction.java b/server/sonar-server/src/main/java/org/sonar/server/issue/SetSeverityAction.java
index cc7fe251dd81ff0220fced12a0b5f79a1c9bfc19..9af025a683f3649f576e9f2b8cc1e873991203a9 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/issue/SetSeverityAction.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/issue/SetSeverityAction.java
@@ -45,12 +45,12 @@ public class SetSeverityAction extends Action {
     super(SET_SEVERITY_KEY);
     this.issueUpdater = issueUpdater;
     this.userSession = userSession;
-    super.setConditions(new IsUnResolved(), this::isCurrentUserIssueAdminOrSecurityAuditor);
+    super.setConditions(new IsUnResolved(), this::isCurrentUserIssueAdminAndNotSecurityHotspot);
   }
 
-  private boolean isCurrentUserIssueAdminOrSecurityAuditor(Issue issue) {
+  private boolean isCurrentUserIssueAdminAndNotSecurityHotspot(Issue issue) {
     DefaultIssue defaultIssue = (DefaultIssue) issue;
-    return ((defaultIssue.type() != RuleType.SECURITY_HOTSPOT && userSession.hasComponentUuidPermission(ISSUE_ADMIN, issue.projectUuid())));
+    return (defaultIssue.type() != RuleType.SECURITY_HOTSPOT && userSession.hasComponentUuidPermission(ISSUE_ADMIN, issue.projectUuid()));
   }
 
   @Override

diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueIndex.java b/server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueIndex.java
index 7e5c5c1ca5126f8edf36482b57839472de71e563..6c015fba1ff55186402e523ad0cfa64144ef7c8a 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueIndex.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueIndex.java
@@ -63,6 +63,7 @@ import org.elasticsearch.search.aggregations.metrics.valuecount.InternalValueCou
 import org.elasticsearch.search.sort.FieldSortBuilder;
 import org.joda.time.DateTimeZone;
 import org.joda.time.Duration;
+import org.sonar.api.rules.RuleType;
 import org.sonar.api.utils.DateUtils;
 import org.sonar.api.utils.System2;
 import org.sonar.core.util.stream.MoreCollectors;
@@ -710,7 +711,8 @@ public class IssueIndex {
       .setQuery(
         boolQuery()
           .mustNot(existsQuery(IssueIndexDefinition.FIELD_ISSUE_RESOLUTION))
-          .filter(termQuery(IssueIndexDefinition.FIELD_ISSUE_ASSIGNEE_UUID, assigneeUuid)))
+          .filter(termQuery(IssueIndexDefinition.FIELD_ISSUE_ASSIGNEE_UUID, assigneeUuid))
+          .mustNot(termQuery(IssueIndexDefinition.FIELD_ISSUE_TYPE, RuleType.SECURITY_HOTSPOT.name())))
       .setSize(0);
     IntStream.range(0, projectUuids.size()).forEach(i -> {
       String projectUuid = projectUuids.get(i);

diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/ws/BulkChangeAction.java b/server/sonar-server/src/main/java/org/sonar/server/issue/ws/BulkChangeAction.java
index 2be2da93a2200da305c98b01d0ccc20b1c71ad5b..d24a6c03a0833ebff6c6c5826a7d559bc828c389 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/issue/ws/BulkChangeAction.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/issue/ws/BulkChangeAction.java
@@ -256,7 +256,7 @@ public class BulkChangeAction implements IssuesWsAction {
 
   private Consumer<DefaultIssue> sendNotification(BulkChangeData bulkChangeData, Map<String, UserDto> userDtoByUuid, UserDto author) {
     return issue -> {
-      if (bulkChangeData.sendNotification) {
+      if (bulkChangeData.sendNotification && issue.type() != RuleType.SECURITY_HOTSPOT) {
         notificationService.scheduleForSending(new IssueChangeNotification()
           .setIssue(issue)
           .setAssignee(userDtoByUuid.get(issue.assignee()))

diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/IssueUpdaterTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/IssueUpdaterTest.java
index 60b45778519c4cccdab0d6c672e80ecb955bf3b4..470dfc44e0b2c15b136e57fe485b94c503ff3e8a 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/issue/IssueUpdaterTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/issue/IssueUpdaterTest.java
@@ -25,6 +25,7 @@ import org.junit.Test;
 import org.junit.rules.ExpectedException;
 import org.mockito.ArgumentCaptor;
 import org.sonar.api.rule.RuleStatus;
+import org.sonar.api.rules.RuleType;
 import org.sonar.api.utils.System2;
 import org.sonar.core.issue.DefaultIssue;
 import org.sonar.core.issue.IssueChangeContext;
@@ -46,8 +47,11 @@ import org.sonar.server.organization.DefaultOrganizationProvider;
 import org.sonar.server.organization.TestDefaultOrganizationProvider;
 import org.sonar.server.rule.DefaultRuleFinder;
 
+import static org.apache.commons.lang.math.RandomUtils.nextInt;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
 import static org.sonar.api.rule.Severity.BLOCKER;
 import static org.sonar.api.rule.Severity.MAJOR;
@@ -96,7 +100,9 @@ public class IssueUpdaterTest {
     RuleDto rule = db.rules().insertRule();
     ComponentDto project = db.components().insertPrivateProject();
     ComponentDto file = db.components().insertComponent(newFileDto(project));
-    DefaultIssue issue = db.issues().insertIssue(IssueTesting.newIssue(rule.getDefinition(), project, file))
+    RuleType randomTypeExceptHotspot = RuleType.values()[nextInt(RuleType.values().length - 1)];
+    DefaultIssue issue = db.issues().insertIssue(IssueTesting.newIssue(rule.getDefinition(), project, file)
+      .setType(randomTypeExceptHotspot))
       .setSeverity(MAJOR)
       .setAssigneeUuid(assignee.getUuid())
       .toDefaultIssue();
@@ -121,13 +127,35 @@ public class IssueUpdaterTest {
     assertThat(issueChangeNotification.getFieldValue("assignee")).isEqualTo(assignee.getLogin());
   }
 
+  @Test
+  public void verify_no_notification_on_hotspot() {
+    UserDto assignee = db.users().insertUser();
+    RuleDto rule = db.rules().insertRule();
+    ComponentDto project = db.components().insertPrivateProject();
+    ComponentDto file = db.components().insertComponent(newFileDto(project));
+    DefaultIssue issue = db.issues().insertIssue(IssueTesting.newIssue(rule.getDefinition(), project, file)
+      .setType(RuleType.SECURITY_HOTSPOT))
+      .setSeverity(MAJOR)
+      .setAssigneeUuid(assignee.getUuid())
+      .toDefaultIssue();
+    UserDto changeAuthor = db.users().insertUser();
+    IssueChangeContext context = IssueChangeContext.createUser(new Date(), changeAuthor.getUuid());
+    issueFieldsSetter.setSeverity(issue, BLOCKER, context);
+
+    underTest.saveIssue(db.getSession(), issue, context, "increase severity");
+
+    verify(notificationManager, never()).scheduleForSending(any());
+  }
+
   @Test
   public void verify_notification_on_branch() {
     RuleDto rule = db.rules().insertRule();
     ComponentDto project = db.components().insertMainBranch();
     ComponentDto branch = db.components().insertProjectBranch(project);
     ComponentDto file = db.components().insertComponent(newFileDto(branch));
-    DefaultIssue issue = db.issues().insertIssue(IssueTesting.newIssue(rule.getDefinition(), branch, file)).setSeverity(MAJOR).toDefaultIssue();
+    RuleType randomTypeExceptHotspot = RuleType.values()[nextInt(RuleType.values().length - 1)];
+    DefaultIssue issue = db.issues().insertIssue(IssueTesting.newIssue(rule.getDefinition(), branch, file)
+      .setType(randomTypeExceptHotspot)).setSeverity(MAJOR).toDefaultIssue();
     IssueChangeContext context = IssueChangeContext.createUser(new Date(), "user_uuid");
     issueFieldsSetter.setSeverity(issue, BLOCKER, context);
 
@@ -146,7 +174,9 @@ public class IssueUpdaterTest {
     RuleDto rule = db.rules().insertRule(r -> r.setStatus(RuleStatus.REMOVED));
     ComponentDto project = db.components().insertPrivateProject();
     ComponentDto file = db.components().insertComponent(newFileDto(project));
-    DefaultIssue issue = db.issues().insertIssue(IssueTesting.newIssue(rule.getDefinition(), project, file)).setSeverity(MAJOR).toDefaultIssue();
+    RuleType randomTypeExceptHotspot = RuleType.values()[nextInt(RuleType.values().length - 1)];
+    DefaultIssue issue = db.issues().insertIssue(IssueTesting.newIssue(rule.getDefinition(), project, file)
+    .setType(randomTypeExceptHotspot)).setSeverity(MAJOR).toDefaultIssue();
     IssueChangeContext context = IssueChangeContext.createUser(new Date(), "user_uuid");
     issueFieldsSetter.setSeverity(issue, BLOCKER, context);
 
@@ -162,7 +192,9 @@ public class IssueUpdaterTest {
     RuleDto rule = db.rules().insertRule();
     ComponentDto project = db.components().insertPrivateProject();
     ComponentDto file = db.components().insertComponent(newFileDto(project));
-    DefaultIssue issue = db.issues().insertIssue(IssueTesting.newIssue(rule.getDefinition(), project, file))
+    RuleType randomTypeExceptHotspot = RuleType.values()[nextInt(RuleType.values().length - 1)];
+    DefaultIssue issue = db.issues().insertIssue(IssueTesting.newIssue(rule.getDefinition(), project, file)
+    .setType(randomTypeExceptHotspot))
       .setAssigneeUuid(oldAssignee.getUuid())
       .toDefaultIssue();
     UserDto changeAuthor = db.users().insertUser();

diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/ws/BulkChangeActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/ws/BulkChangeActionTest.java
index 82026b788c689a285fbdc4a03adcaeefc79f0761..1ddcfc868862d813d1d03f5146234823ddac32de 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/issue/ws/BulkChangeActionTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/issue/ws/BulkChangeActionTest.java
@@ -73,7 +73,9 @@ import static java.util.Collections.singletonList;
 import static java.util.Objects.requireNonNull;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.tuple;
+import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 import static org.sonar.api.issue.Issue.RESOLUTION_FIXED;
@@ -82,6 +84,7 @@ import static org.sonar.api.issue.Issue.STATUS_OPEN;
 import static org.sonar.api.rule.Severity.MAJOR;
 import static org.sonar.api.rule.Severity.MINOR;
 import static org.sonar.api.rules.RuleType.BUG;
+import static org.sonar.api.rules.RuleType.SECURITY_HOTSPOT;
 import static org.sonar.api.rules.RuleType.VULNERABILITY;
 import static org.sonar.api.web.UserRole.ISSUE_ADMIN;
 import static org.sonar.api.web.UserRole.USER;
@@ -286,6 +289,22 @@ public class BulkChangeActionTest {
     assertThat(issueChangeNotificationCaptor.getValue().getFieldValue("branch")).isNull();
   }
 
+  @Test
+  public void no_notification_for_hotspots() {
+    setUserProjectPermissions(USER);
+    IssueDto issueDto = db.issues().insertIssue(newUnresolvedIssue().setType(SECURITY_HOTSPOT));
+
+    BulkChangeWsResponse response = call(builder()
+      .setIssues(singletonList(issueDto.getKey()))
+      .setDoTransition("dismiss")
+      .setSendNotifications(true)
+      .build());
+
+    checkResponse(response, 1, 1, 0, 0);
+
+    verify(notificationManager, never()).scheduleForSending(any());
+  }
+
   @Test
   public void send_notification_on_branch() {
     setUserProjectPermissions(USER);