From: Julien HENRY Date: Tue, 26 Jun 2018 09:20:54 +0000 (+0200) Subject: SONAR-10874 Disable all notifications for hotspots X-Git-Tag: 7.5~876 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=35acc6b68f1c1a9157483fd129cc8067a6f67311;p=sonarqube.git 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 ccb59f0d386..214cdc3f825 100644 --- 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 issues, Component project, Map 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 2229f229ee3..98ca9bed1f5 100644 --- 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 8b04bb0ade5..0b23c24eee0 100644 --- 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 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 cc7fe251dd8..9af025a683f 100644 --- 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 7e5c5c1ca51..6c015fba1ff 100644 --- 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 2be2da93a22..d24a6c03a08 100644 --- 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 sendNotification(BulkChangeData bulkChangeData, Map 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 60b45778519..470dfc44e0b 100644 --- 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 82026b788c6..1ddcfc86886 100644 --- 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);