@@ -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); | |||
} | |||
} | |||
} | |||
} |
@@ -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) |
@@ -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; | |||
} | |||
@@ -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 |
@@ -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); |
@@ -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())) |
@@ -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(); |
@@ -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); |