From d47f7eac1a6880a4aeaedfb0bbff906240b72f07 Mon Sep 17 00:00:00 2001 From: =?utf8?q?S=C3=A9bastien=20Lesaint?= Date: Tue, 12 Sep 2017 14:48:36 +0200 Subject: [PATCH] SONAR-9144 send notifications only for issues on the leak period --- .../step/SendIssueNotificationsStep.java | 45 +-- .../notification/NewIssuesNotification.java | 23 +- .../notification/NewIssuesStatistics.java | 20 +- .../step/SendIssueNotificationsStepTest.java | 259 +++++++++++++++--- .../notification/NewIssuesStatisticsTest.java | 38 +-- .../IssueCreationDatePluginChangedTest.java | 73 ++++- .../tests/issue/IssueNotificationsTest.java | 7 + 7 files changed, 374 insertions(+), 91 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/step/SendIssueNotificationsStep.java b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/step/SendIssueNotificationsStep.java index cd1dde632dd..6bf79d4b85d 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/step/SendIssueNotificationsStep.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/step/SendIssueNotificationsStep.java @@ -27,6 +27,7 @@ import java.util.Date; import java.util.Map; import java.util.Optional; import java.util.Set; +import java.util.function.Predicate; import org.sonar.api.issue.Issue; import org.sonar.api.utils.Duration; import org.sonar.core.issue.DefaultIssue; @@ -90,14 +91,12 @@ public class SendIssueNotificationsStep implements ComputationStep { private void doExecute(Component project) { long analysisDate = analysisMetadataHolder.getAnalysisDate(); - NewIssuesStatistics newIssuesStats = new NewIssuesStatistics(i -> i.isNew() && i.creationDate().getTime() >= truncateToSeconds(analysisDate)); - CloseableIterator issues = issueCache.traverse(); - try { + Predicate isOnLeakPredicate = i -> i.isNew() && i.creationDate().getTime() >= truncateToSeconds(analysisDate); + NewIssuesStatistics newIssuesStats = new NewIssuesStatistics(isOnLeakPredicate); + try (CloseableIterator issues = issueCache.traverse()) { processIssues(newIssuesStats, issues, project); - } finally { - issues.close(); } - if (newIssuesStats.hasIssues()) { + if (newIssuesStats.hasIssuesOnLeak()) { sendNewIssuesNotification(newIssuesStats, project, analysisDate); sendNewIssuesNotificationToAssignees(newIssuesStats, project, analysisDate); } @@ -140,26 +139,28 @@ public class SendIssueNotificationsStep implements ComputationStep { .setProject(project.getPublicKey(), project.getUuid(), project.getName(), getBranchName()) .setAnalysisDate(new Date(analysisDate)) .setStatistics(project.getName(), globalStatistics) - .setDebt(Duration.create(globalStatistics.effort().getTotal())); + .setDebt(Duration.create(globalStatistics.effort().getOnLeak())); service.deliver(notification); } private void sendNewIssuesNotificationToAssignees(NewIssuesStatistics statistics, Component project, long analysisDate) { - // send email to each user having issues - for (Map.Entry assigneeAndStatisticsTuple : statistics.assigneesStatistics().entrySet()) { - String assignee = assigneeAndStatisticsTuple.getKey(); - NewIssuesStatistics.Stats assigneeStatistics = assigneeAndStatisticsTuple.getValue(); - MyNewIssuesNotification myNewIssuesNotification = newIssuesNotificationFactory - .newMyNewIssuesNotification() - .setAssignee(assignee); - myNewIssuesNotification - .setProject(project.getPublicKey(), project.getUuid(), project.getName(), getBranchName()) - .setAnalysisDate(new Date(analysisDate)) - .setStatistics(project.getName(), assigneeStatistics) - .setDebt(Duration.create(assigneeStatistics.effort().getTotal())); - - service.deliver(myNewIssuesNotification); - } + statistics.getAssigneesStatistics().entrySet() + .stream() + .filter(e -> e.getValue().hasIssuesOnLeak()) + .forEach(e -> { + String assignee = e.getKey(); + NewIssuesStatistics.Stats assigneeStatistics = e.getValue(); + MyNewIssuesNotification myNewIssuesNotification = newIssuesNotificationFactory + .newMyNewIssuesNotification() + .setAssignee(assignee); + myNewIssuesNotification + .setProject(project.getPublicKey(), project.getUuid(), project.getName(), getBranchName()) + .setAnalysisDate(new Date(analysisDate)) + .setStatistics(project.getName(), assigneeStatistics) + .setDebt(Duration.create(assigneeStatistics.effort().getOnLeak())); + + service.deliver(myNewIssuesNotification); + }); } private Optional getComponentKey(DefaultIssue issue) { diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/notification/NewIssuesNotification.java b/server/sonar-server/src/main/java/org/sonar/server/issue/notification/NewIssuesNotification.java index 6d2df27822e..8d26d8d5187 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/notification/NewIssuesNotification.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/notification/NewIssuesNotification.java @@ -85,7 +85,7 @@ public class NewIssuesNotification extends Notification { } public NewIssuesNotification setStatistics(String projectName, NewIssuesStatistics.Stats stats) { - setDefaultMessage(stats.getDistributedMetricStats(SEVERITY).getTotal() + " new issues on " + projectName + ".\n"); + setDefaultMessage(stats.getDistributedMetricStats(SEVERITY).getOnLeak() + " new issues on " + projectName + ".\n"); try (DbSession dbSession = dbClient.openSession(false)) { setSeverityStatistics(stats); @@ -101,12 +101,12 @@ public class NewIssuesNotification extends Notification { private void setRuleStatistics(DbSession dbSession, NewIssuesStatistics.Stats stats) { Metric metric = Metric.RULE; int i = 1; - for (Map.Entry ruleStats : fiveBiggest(stats.getDistributedMetricStats(metric), MetricStatsInt::getTotal)) { + for (Map.Entry ruleStats : fiveBiggest(stats.getDistributedMetricStats(metric), MetricStatsInt::getOnLeak)) { String ruleKey = ruleStats.getKey(); RuleDefinitionDto rule = dbClient.ruleDao().selectOrFailDefinitionByKey(dbSession, RuleKey.parse(ruleKey)); String name = rule.getName() + " (" + rule.getLanguage() + ")"; setFieldValue(metric + DOT + i + LABEL, name); - setFieldValue(metric + DOT + i + COUNT, String.valueOf(ruleStats.getValue().getTotal())); + setFieldValue(metric + DOT + i + COUNT, String.valueOf(ruleStats.getValue().getOnLeak())); i++; } } @@ -114,11 +114,11 @@ public class NewIssuesNotification extends Notification { private void setComponentsStatistics(DbSession dbSession, NewIssuesStatistics.Stats stats) { Metric metric = Metric.COMPONENT; int i = 1; - for (Map.Entry componentStats : fiveBiggest(stats.getDistributedMetricStats(metric), MetricStatsInt::getTotal)) { + for (Map.Entry componentStats : fiveBiggest(stats.getDistributedMetricStats(metric), MetricStatsInt::getOnLeak)) { String uuid = componentStats.getKey(); String componentName = dbClient.componentDao().selectOrFailByUuid(dbSession, uuid).name(); setFieldValue(metric + DOT + i + LABEL, componentName); - setFieldValue(metric + DOT + i + COUNT, String.valueOf(componentStats.getValue().getTotal())); + setFieldValue(metric + DOT + i + COUNT, String.valueOf(componentStats.getValue().getOnLeak())); i++; } } @@ -126,8 +126,8 @@ public class NewIssuesNotification extends Notification { private void setTagsStatistics(NewIssuesStatistics.Stats stats) { Metric metric = Metric.TAG; int i = 1; - for (Map.Entry tagStats : fiveBiggest(stats.getDistributedMetricStats(metric), MetricStatsInt::getTotal)) { - setFieldValue(metric + DOT + i + COUNT, String.valueOf(tagStats.getValue().getTotal())); + for (Map.Entry tagStats : fiveBiggest(stats.getDistributedMetricStats(metric), MetricStatsInt::getOnLeak)) { + setFieldValue(metric + DOT + i + COUNT, String.valueOf(tagStats.getValue().getOnLeak())); setFieldValue(metric + DOT + i + ".label", tagStats.getKey()); i++; } @@ -135,14 +135,13 @@ public class NewIssuesNotification extends Notification { private void setAssigneesStatistics(NewIssuesStatistics.Stats stats) { Metric metric = Metric.ASSIGNEE; - ToIntFunction biggerCriteria = MetricStatsInt::getTotal; int i = 1; - for (Map.Entry assigneeStats : fiveBiggest(stats.getDistributedMetricStats(metric), biggerCriteria)) { + for (Map.Entry assigneeStats : fiveBiggest(stats.getDistributedMetricStats(metric), MetricStatsInt::getOnLeak)) { String login = assigneeStats.getKey(); UserDoc user = userIndex.getNullableByLogin(login); String name = user == null ? login : user.name(); setFieldValue(metric + DOT + i + LABEL, name); - setFieldValue(metric + DOT + i + COUNT, String.valueOf(biggerCriteria.applyAsInt(assigneeStats.getValue()))); + setFieldValue(metric + DOT + i + COUNT, String.valueOf(assigneeStats.getValue().getOnLeak())); i++; } } @@ -164,11 +163,11 @@ public class NewIssuesNotification extends Notification { private void setSeverityStatistics(NewIssuesStatistics.Stats stats) { DistributedMetricStatsInt distributedMetricStats = stats.getDistributedMetricStats(SEVERITY); - setFieldValue(SEVERITY + COUNT, String.valueOf(distributedMetricStats.getTotal())); + setFieldValue(SEVERITY + COUNT, String.valueOf(distributedMetricStats.getOnLeak())); for (String severity : Severity.ALL) { setFieldValue( SEVERITY + DOT + severity + COUNT, - String.valueOf(distributedMetricStats.getForLabel(severity).map(MetricStatsInt::getTotal).orElse(0))); + String.valueOf(distributedMetricStats.getForLabel(severity).map(MetricStatsInt::getOnLeak).orElse(0))); } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/notification/NewIssuesStatistics.java b/server/sonar-server/src/main/java/org/sonar/server/issue/notification/NewIssuesStatistics.java index 8410d8a9caa..8101f45aeb2 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/notification/NewIssuesStatistics.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/notification/NewIssuesStatistics.java @@ -51,7 +51,7 @@ public class NewIssuesStatistics { } } - public Map assigneesStatistics() { + public Map getAssigneesStatistics() { return assigneesStatistics; } @@ -63,7 +63,15 @@ public class NewIssuesStatistics { return globalStatistics.hasIssues(); } - enum Metric { + public boolean hasIssuesOnLeak() { + return globalStatistics.hasIssuesOnLeak(); + } + + public boolean hasIssuesOffLeak() { + return globalStatistics.hasIssuesOffLeak(); + } + + public enum Metric { SEVERITY(true), TAG(true), COMPONENT(true), ASSIGNEE(true), EFFORT(false), RULE(true); private final boolean isComputedByDistribution; @@ -123,6 +131,14 @@ public class NewIssuesStatistics { return getDistributedMetricStats(SEVERITY).getTotal() > 0; } + public boolean hasIssuesOnLeak() { + return getDistributedMetricStats(SEVERITY).getOnLeak() > 0; + } + + public boolean hasIssuesOffLeak() { + return getDistributedMetricStats(SEVERITY).getOffLeak() > 0; + } + } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/step/SendIssueNotificationsStepTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/step/SendIssueNotificationsStepTest.java index 384f05314cb..5456d7625a9 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/step/SendIssueNotificationsStepTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/step/SendIssueNotificationsStepTest.java @@ -19,12 +19,20 @@ */ package org.sonar.server.computation.task.projectanalysis.step; +import java.util.Arrays; +import java.util.Collections; import java.util.Date; +import java.util.List; +import java.util.Random; +import java.util.stream.Collectors; +import java.util.stream.IntStream; +import java.util.stream.Stream; import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; import org.mockito.ArgumentCaptor; +import org.mockito.exceptions.verification.junit.ArgumentsAreDifferent; import org.sonar.api.notifications.Notification; import org.sonar.api.rule.Severity; import org.sonar.api.utils.Duration; @@ -41,20 +49,22 @@ import org.sonar.server.computation.task.projectanalysis.component.TreeRootHolde import org.sonar.server.computation.task.projectanalysis.issue.IssueCache; import org.sonar.server.computation.task.projectanalysis.issue.RuleRepositoryRule; import org.sonar.server.computation.task.step.ComputationStep; +import org.sonar.server.issue.notification.DistributedMetricStatsInt; import org.sonar.server.issue.notification.IssueChangeNotification; import org.sonar.server.issue.notification.MyNewIssuesNotification; import org.sonar.server.issue.notification.NewIssuesNotification; import org.sonar.server.issue.notification.NewIssuesNotificationFactory; import org.sonar.server.issue.notification.NewIssuesStatistics; import org.sonar.server.notification.NotificationService; +import org.sonar.server.util.cache.DiskCache; +import static org.apache.commons.lang.RandomStringUtils.randomAlphanumeric; import static org.assertj.core.api.Java6Assertions.assertThat; import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; -import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.sonar.db.component.ComponentTesting.newBranchDto; @@ -71,6 +81,7 @@ public class SendIssueNotificationsStepTest extends BaseStepTest { private static final String BRANCH_NAME = "feature"; private static final long ANALYSE_DATE = 123L; + private static final int FIVE_MINUTES_IN_MS = 1000 * 60 * 5; private static final Duration ISSUE_DURATION = Duration.create(100L); private static final String ISSUE_ASSIGNEE = "John"; @@ -81,15 +92,12 @@ public class SendIssueNotificationsStepTest extends BaseStepTest { @Rule public TreeRootHolderRule treeRootHolder = new TreeRootHolderRule() .setRoot(PROJECT); - @Rule public AnalysisMetadataHolderRule analysisMetadataHolder = new AnalysisMetadataHolderRule() .setBranch(new DefaultBranchImpl()) .setAnalysisDate(new Date(ANALYSE_DATE)); - @Rule public RuleRepositoryRule ruleRepository = new RuleRepositoryRule(); - @Rule public TemporaryFolder temp = new TemporaryFolder(); @@ -126,12 +134,11 @@ public class SendIssueNotificationsStepTest extends BaseStepTest { new DefaultIssue().setSeverity(Severity.BLOCKER).setEffort(ISSUE_DURATION) .setCreationDate(new Date(ANALYSE_DATE))) .close(); - when(notificationService.hasProjectSubscribersForTypes(PROJECT.getUuid(), SendIssueNotificationsStep.NOTIF_TYPES)).thenReturn(true); underTest.execute(); - verify(notificationService).deliver(any(NewIssuesNotification.class)); + verify(notificationService).deliver(newIssuesNotificationMock); verify(newIssuesNotificationMock).setProject(PROJECT.getPublicKey(), PROJECT.getUuid(), PROJECT.getName(), null); verify(newIssuesNotificationMock).setAnalysisDate(new Date(ANALYSE_DATE)); verify(newIssuesNotificationMock).setStatistics(eq(PROJECT.getName()), any(NewIssuesStatistics.Stats.class)); @@ -139,39 +146,93 @@ public class SendIssueNotificationsStepTest extends BaseStepTest { } @Test - public void send_global_new_issues_notification_on_branch() throws Exception { - ComponentDto project = newPrivateProjectDto(newOrganizationDto()); - ComponentDto branch = newProjectBranch(project, newBranchDto(project).setKey(BRANCH_NAME)); - ComponentDto file = newFileDto(branch); - treeRootHolder.setRoot(builder(Type.PROJECT, 2).setKey(branch.getDbKey()).setPublicKey(branch.getKey()).setName(branch.longName()).setUuid(branch.uuid()).addChildren( - builder(Component.Type.FILE, 11).setKey(file.getDbKey()).setPublicKey(file.getKey()).setName(file.longName()).build()).build()); + public void send_global_new_issues_notification_only_for_non_backdated_issues() { + Random random = new Random(); + Integer[] efforts = IntStream.range(0, 1 + random.nextInt(10)).mapToObj(i -> 10_000 * i).toArray(Integer[]::new); + Integer[] backDatedEfforts = IntStream.range(0, 1 + random.nextInt(10)).mapToObj(i -> 10 + random.nextInt(100)).toArray(Integer[]::new); + Duration expectedEffort = Duration.create(Arrays.stream(efforts).mapToInt(i -> i).sum()); + List issues = Stream.concat(Arrays.stream(efforts) + .map(effort -> new DefaultIssue().setSeverity(Severity.BLOCKER).setEffort(Duration.create(effort)) + .setCreationDate(new Date(ANALYSE_DATE))), + Arrays.stream(backDatedEfforts) + .map(effort -> new DefaultIssue().setSeverity(Severity.BLOCKER).setEffort(Duration.create(effort)) + .setCreationDate(new Date(ANALYSE_DATE - FIVE_MINUTES_IN_MS)))) + .collect(Collectors.toList()); + Collections.shuffle(issues); + DiskCache.DiskAppender issueCache = this.issueCache.newAppender(); + issues.forEach(issueCache::append); + when(notificationService.hasProjectSubscribersForTypes(PROJECT.getUuid(), SendIssueNotificationsStep.NOTIF_TYPES)).thenReturn(true); + + underTest.execute(); + + verify(notificationService).deliver(newIssuesNotificationMock); + ArgumentCaptor statsCaptor = ArgumentCaptor.forClass(NewIssuesStatistics.Stats.class); + verify(newIssuesNotificationMock).setStatistics(eq(PROJECT.getName()), statsCaptor.capture()); + verify(newIssuesNotificationMock).setDebt(expectedEffort); + NewIssuesStatistics.Stats stats = statsCaptor.getValue(); + assertThat(stats.hasIssues()).isTrue(); + // just checking all issues have been added to the stats + DistributedMetricStatsInt severity = stats.getDistributedMetricStats(NewIssuesStatistics.Metric.SEVERITY); + assertThat(severity.getOnLeak()).isEqualTo(efforts.length); + assertThat(severity.getOffLeak()).isEqualTo(backDatedEfforts.length); + assertThat(severity.getTotal()).isEqualTo(backDatedEfforts.length + efforts.length); + } + + @Test + public void do_not_send_global_new_issues_notification_if_issue_has_been_backdated() { issueCache.newAppender().append( - new DefaultIssue().setSeverity(Severity.BLOCKER).setEffort(ISSUE_DURATION)).close(); + new DefaultIssue().setSeverity(Severity.BLOCKER).setEffort(ISSUE_DURATION) + .setCreationDate(new Date(ANALYSE_DATE - FIVE_MINUTES_IN_MS))) + .close(); + when(notificationService.hasProjectSubscribersForTypes(PROJECT.getUuid(), SendIssueNotificationsStep.NOTIF_TYPES)).thenReturn(true); + + underTest.execute(); + verify(notificationService, never()).deliver(any(Notification.class)); + } + + @Test + public void send_global_new_issues_notification_on_branch() throws Exception { + ComponentDto branch = setUpProjectWithBranch(); + issueCache.newAppender().append( + new DefaultIssue().setSeverity(Severity.BLOCKER).setEffort(ISSUE_DURATION).setCreationDate(new Date(ANALYSE_DATE))).close(); when(notificationService.hasProjectSubscribersForTypes(branch.uuid(), SendIssueNotificationsStep.NOTIF_TYPES)).thenReturn(true); analysisMetadataHolder.setBranch(newBranch()); underTest.execute(); - verify(notificationService).deliver(any(NewIssuesNotification.class)); + verify(notificationService).deliver(newIssuesNotificationMock); verify(newIssuesNotificationMock).setProject(branch.getKey(), branch.uuid(), branch.longName(), BRANCH_NAME); verify(newIssuesNotificationMock).setAnalysisDate(new Date(ANALYSE_DATE)); verify(newIssuesNotificationMock).setStatistics(eq(branch.longName()), any(NewIssuesStatistics.Stats.class)); verify(newIssuesNotificationMock).setDebt(ISSUE_DURATION); } + @Test + public void do_not_send_global_new_issues_notification_on_branch_if_issue_has_been_backdated() throws Exception { + ComponentDto branch = setUpProjectWithBranch(); + issueCache.newAppender().append( + new DefaultIssue().setSeverity(Severity.BLOCKER).setEffort(ISSUE_DURATION).setCreationDate(new Date(ANALYSE_DATE - FIVE_MINUTES_IN_MS))).close(); + when(notificationService.hasProjectSubscribersForTypes(branch.uuid(), SendIssueNotificationsStep.NOTIF_TYPES)).thenReturn(true); + analysisMetadataHolder.setBranch(newBranch()); + + underTest.execute(); + + verify(notificationService, never()).deliver(any(Notification.class)); + } + @Test public void send_new_issues_notification_to_user() throws Exception { issueCache.newAppender().append( new DefaultIssue().setSeverity(Severity.BLOCKER).setEffort(ISSUE_DURATION).setAssignee(ISSUE_ASSIGNEE) .setCreationDate(new Date(ANALYSE_DATE))) .close(); - when(notificationService.hasProjectSubscribersForTypes(PROJECT.getUuid(), SendIssueNotificationsStep.NOTIF_TYPES)).thenReturn(true); underTest.execute(); - verify(notificationService, times(2)).deliver(any(Notification.class)); + verify(notificationService).deliver(newIssuesNotificationMock); + verify(notificationService).deliver(myNewIssuesNotificationMock); verify(myNewIssuesNotificationMock).setAssignee(ISSUE_ASSIGNEE); verify(myNewIssuesNotificationMock).setProject(PROJECT.getPublicKey(), PROJECT.getUuid(), PROJECT.getName(), null); verify(myNewIssuesNotificationMock).setAnalysisDate(new Date(ANALYSE_DATE)); @@ -179,13 +240,126 @@ public class SendIssueNotificationsStepTest extends BaseStepTest { verify(myNewIssuesNotificationMock).setDebt(ISSUE_DURATION); } + @Test + public void send_new_issues_notification_to_user_only_for_those_assigned_to_her() { + Random random = new Random(); + Integer[] assigned = IntStream.range(0, 1 + random.nextInt(10)).mapToObj(i -> 10_000 * i).toArray(Integer[]::new); + Integer[] assignedToOther = IntStream.range(0, 1 + random.nextInt(10)).mapToObj(i -> 10 + random.nextInt(100)).toArray(Integer[]::new); + Duration expectedEffort = Duration.create(Arrays.stream(assigned).mapToInt(i -> i).sum()); + String assignee = randomAlphanumeric(5); + String otherAssignee = randomAlphanumeric(5); + List issues = Stream.concat(Arrays.stream(assigned) + .map(effort -> new DefaultIssue().setSeverity(Severity.BLOCKER).setEffort(Duration.create(effort)) + .setAssignee(assignee) + .setCreationDate(new Date(ANALYSE_DATE))), + Arrays.stream(assignedToOther) + .map(effort -> new DefaultIssue().setSeverity(Severity.BLOCKER).setEffort(Duration.create(effort)) + .setAssignee(otherAssignee) + .setCreationDate(new Date(ANALYSE_DATE)))) + .collect(Collectors.toList()); + Collections.shuffle(issues); + DiskCache.DiskAppender issueCache = this.issueCache.newAppender(); + issues.forEach(issueCache::append); + when(notificationService.hasProjectSubscribersForTypes(PROJECT.getUuid(), SendIssueNotificationsStep.NOTIF_TYPES)).thenReturn(true); + MyNewIssuesNotification myNewIssuesNotificationMock2 = createMyNewIssuesNotificationMock(); + when(newIssuesNotificationFactory.newMyNewIssuesNotification()) + .thenReturn(myNewIssuesNotificationMock) + .thenReturn(myNewIssuesNotificationMock2); + + underTest.execute(); + + verify(notificationService).deliver(newIssuesNotificationMock); + verify(notificationService).deliver(myNewIssuesNotificationMock); + verify(notificationService).deliver(myNewIssuesNotificationMock2); + + MyNewIssuesNotification effectiveMyNewIssuesNotificationMock = this.myNewIssuesNotificationMock; + try { + verify(effectiveMyNewIssuesNotificationMock).setAssignee(assignee); + } catch (ArgumentsAreDifferent e) { + assertThat(e.getMessage()) + .contains("Wanted:\nmyNewIssuesNotification.setAssignee(\"" + assignee + "\")") + .contains("Actual invocation has different arguments:\n" + + "myNewIssuesNotification.setAssignee(\"" + otherAssignee + "\")"); + effectiveMyNewIssuesNotificationMock = myNewIssuesNotificationMock2; + } + ArgumentCaptor statsCaptor = ArgumentCaptor.forClass(NewIssuesStatistics.Stats.class); + verify(effectiveMyNewIssuesNotificationMock).setStatistics(eq(PROJECT.getName()), statsCaptor.capture()); + verify(effectiveMyNewIssuesNotificationMock).setDebt(expectedEffort); + NewIssuesStatistics.Stats stats = statsCaptor.getValue(); + assertThat(stats.hasIssues()).isTrue(); + // just checking all issues have been added to the stats + DistributedMetricStatsInt severity = stats.getDistributedMetricStats(NewIssuesStatistics.Metric.SEVERITY); + assertThat(severity.getOnLeak()).isEqualTo(assigned.length); + assertThat(severity.getOffLeak()).isEqualTo(0); + assertThat(severity.getTotal()).isEqualTo(assigned.length); + } + + @Test + public void send_new_issues_notification_to_user_only_for_non_backdated_issues() throws Exception { + Random random = new Random(); + Integer[] efforts = IntStream.range(0, 1 + random.nextInt(10)).mapToObj(i -> 10_000 * i).toArray(Integer[]::new); + Integer[] backDatedEfforts = IntStream.range(0, 1 + random.nextInt(10)).mapToObj(i -> 10 + random.nextInt(100)).toArray(Integer[]::new); + Duration expectedEffort = Duration.create(Arrays.stream(efforts).mapToInt(i -> i).sum()); + List issues = Stream.concat(Arrays.stream(efforts) + .map(effort -> new DefaultIssue().setSeverity(Severity.BLOCKER).setEffort(Duration.create(effort)) + .setAssignee(ISSUE_ASSIGNEE) + .setCreationDate(new Date(ANALYSE_DATE))), + Arrays.stream(backDatedEfforts) + .map(effort -> new DefaultIssue().setSeverity(Severity.BLOCKER).setEffort(Duration.create(effort)) + .setAssignee(ISSUE_ASSIGNEE) + .setCreationDate(new Date(ANALYSE_DATE - FIVE_MINUTES_IN_MS)))) + .collect(Collectors.toList()); + Collections.shuffle(issues); + DiskCache.DiskAppender issueCache = this.issueCache.newAppender(); + issues.forEach(issueCache::append); + when(notificationService.hasProjectSubscribersForTypes(PROJECT.getUuid(), SendIssueNotificationsStep.NOTIF_TYPES)).thenReturn(true); + + underTest.execute(); + + verify(notificationService).deliver(newIssuesNotificationMock); + verify(notificationService).deliver(myNewIssuesNotificationMock); + verify(myNewIssuesNotificationMock).setAssignee(ISSUE_ASSIGNEE); + ArgumentCaptor statsCaptor = ArgumentCaptor.forClass(NewIssuesStatistics.Stats.class); + verify(myNewIssuesNotificationMock).setStatistics(eq(PROJECT.getName()), statsCaptor.capture()); + verify(myNewIssuesNotificationMock).setDebt(expectedEffort); + NewIssuesStatistics.Stats stats = statsCaptor.getValue(); + assertThat(stats.hasIssues()).isTrue(); + // just checking all issues have been added to the stats + DistributedMetricStatsInt severity = stats.getDistributedMetricStats(NewIssuesStatistics.Metric.SEVERITY); + assertThat(severity.getOnLeak()).isEqualTo(efforts.length); + assertThat(severity.getOffLeak()).isEqualTo(backDatedEfforts.length); + assertThat(severity.getTotal()).isEqualTo(backDatedEfforts.length + efforts.length); + } + + @Test + public void do_not_send_new_issues_notification_to_user_if_issue_is_backdated() throws Exception { + issueCache.newAppender().append( + new DefaultIssue().setSeverity(Severity.BLOCKER).setEffort(ISSUE_DURATION).setAssignee(ISSUE_ASSIGNEE) + .setCreationDate(new Date(ANALYSE_DATE - FIVE_MINUTES_IN_MS))) + .close(); + when(notificationService.hasProjectSubscribersForTypes(PROJECT.getUuid(), SendIssueNotificationsStep.NOTIF_TYPES)).thenReturn(true); + + underTest.execute(); + + verify(notificationService, never()).deliver(any(Notification.class)); + } + @Test public void send_issues_change_notification() throws Exception { + sendIssueChangeNotification(ANALYSE_DATE); + } + + @Test + public void send_issues_change_notification_even_if_issue_is_backdated() throws Exception { + sendIssueChangeNotification(ANALYSE_DATE - FIVE_MINUTES_IN_MS); + } + + private void sendIssueChangeNotification(long issueCreatedAt) { 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(ANALYSE_DATE)); + .setNew(false).setChanged(true).setSendNotifications(true).setCreationDate(new Date(issueCreatedAt)); ruleRepository.add(ruleDefinitionDto.getKey()).setName(ruleDefinitionDto.getName()); issueCache.newAppender().append(issue).close(); when(notificationService.hasProjectSubscribersForTypes(PROJECT.getUuid(), SendIssueNotificationsStep.NOTIF_TYPES)).thenReturn(true); @@ -194,28 +368,39 @@ public class SendIssueNotificationsStepTest extends BaseStepTest { ArgumentCaptor issueChangeNotificationCaptor = ArgumentCaptor.forClass(IssueChangeNotification.class); verify(notificationService).deliver(issueChangeNotificationCaptor.capture()); - assertThat(issueChangeNotificationCaptor.getValue().getFieldValue("key")).isEqualTo(issue.key()); - assertThat(issueChangeNotificationCaptor.getValue().getFieldValue("assignee")).isEqualTo(issue.assignee()); - assertThat(issueChangeNotificationCaptor.getValue().getFieldValue("message")).isEqualTo(issue.message()); - assertThat(issueChangeNotificationCaptor.getValue().getFieldValue("ruleName")).isEqualTo(ruleDefinitionDto.getName()); - assertThat(issueChangeNotificationCaptor.getValue().getFieldValue("projectName")).isEqualTo(project.longName()); - assertThat(issueChangeNotificationCaptor.getValue().getFieldValue("projectKey")).isEqualTo(project.getKey()); - assertThat(issueChangeNotificationCaptor.getValue().getFieldValue("componentKey")).isEqualTo(file.getKey()); - assertThat(issueChangeNotificationCaptor.getValue().getFieldValue("componentName")).isEqualTo(file.longName()); + IssueChangeNotification issueChangeNotification = issueChangeNotificationCaptor.getValue(); + assertThat(issueChangeNotification.getFieldValue("key")).isEqualTo(issue.key()); + assertThat(issueChangeNotification.getFieldValue("assignee")).isEqualTo(issue.assignee()); + assertThat(issueChangeNotification.getFieldValue("message")).isEqualTo(issue.message()); + assertThat(issueChangeNotification.getFieldValue("ruleName")).isEqualTo(ruleDefinitionDto.getName()); + assertThat(issueChangeNotification.getFieldValue("projectName")).isEqualTo(project.longName()); + assertThat(issueChangeNotification.getFieldValue("projectKey")).isEqualTo(project.getKey()); + assertThat(issueChangeNotification.getFieldValue("componentKey")).isEqualTo(file.getKey()); + assertThat(issueChangeNotification.getFieldValue("componentName")).isEqualTo(file.longName()); } @Test public void send_issues_change_notification_on_branch() throws Exception { + sendIssueChangeNotificationOnBranch(ANALYSE_DATE); + } + + @Test + public void send_issues_change_notification_on_branch_even_if_issue_is_backdated() throws Exception { + sendIssueChangeNotificationOnBranch(ANALYSE_DATE - FIVE_MINUTES_IN_MS); + } + + private void sendIssueChangeNotificationOnBranch(long issueCreatedAt) { ComponentDto project = newPrivateProjectDto(newOrganizationDto()); ComponentDto branch = newProjectBranch(project, newBranchDto(project).setKey(BRANCH_NAME)); ComponentDto file = newFileDto(branch); treeRootHolder.setRoot(builder(Type.PROJECT, 2).setKey(branch.getDbKey()).setPublicKey(branch.getKey()).setName(branch.longName()).setUuid(branch.uuid()).addChildren( - builder(Component.Type.FILE, 11).setKey(file.getDbKey()).setPublicKey(file.getKey()).setName(file.longName()).build()).build()); + 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() .setNew(false) .setChanged(true) - .setSendNotifications(true); + .setSendNotifications(true) + .setCreationDate(new Date(issueCreatedAt)); ruleRepository.add(ruleDefinitionDto.getKey()).setName(ruleDefinitionDto.getName()); issueCache.newAppender().append(issue).close(); when(notificationService.hasProjectSubscribersForTypes(branch.uuid(), SendIssueNotificationsStep.NOTIF_TYPES)).thenReturn(true); @@ -225,11 +410,12 @@ public class SendIssueNotificationsStepTest extends BaseStepTest { ArgumentCaptor issueChangeNotificationCaptor = ArgumentCaptor.forClass(IssueChangeNotification.class); verify(notificationService).deliver(issueChangeNotificationCaptor.capture()); - assertThat(issueChangeNotificationCaptor.getValue().getFieldValue("projectName")).isEqualTo(branch.longName()); - assertThat(issueChangeNotificationCaptor.getValue().getFieldValue("projectKey")).isEqualTo(branch.getKey()); - assertThat(issueChangeNotificationCaptor.getValue().getFieldValue("branch")).isEqualTo(BRANCH_NAME); - assertThat(issueChangeNotificationCaptor.getValue().getFieldValue("componentKey")).isEqualTo(file.getKey()); - assertThat(issueChangeNotificationCaptor.getValue().getFieldValue("componentName")).isEqualTo(file.longName()); + IssueChangeNotification issueChangeNotification = issueChangeNotificationCaptor.getValue(); + assertThat(issueChangeNotification.getFieldValue("projectName")).isEqualTo(branch.longName()); + assertThat(issueChangeNotification.getFieldValue("projectKey")).isEqualTo(branch.getKey()); + assertThat(issueChangeNotification.getFieldValue("branch")).isEqualTo(BRANCH_NAME); + assertThat(issueChangeNotification.getFieldValue("componentKey")).isEqualTo(file.getKey()); + assertThat(issueChangeNotification.getFieldValue("componentName")).isEqualTo(file.longName()); } private NewIssuesNotification createNewIssuesNotificationMock() { @@ -258,6 +444,15 @@ public class SendIssueNotificationsStepTest extends BaseStepTest { return branch; } + private ComponentDto setUpProjectWithBranch() { + ComponentDto project = newPrivateProjectDto(newOrganizationDto()); + ComponentDto branch = newProjectBranch(project, newBranchDto(project).setKey(BRANCH_NAME)); + ComponentDto file = newFileDto(branch); + 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()); + return branch; + } + @Override protected ComputationStep step() { return underTest; diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/notification/NewIssuesStatisticsTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/notification/NewIssuesStatisticsTest.java index 6a56f8de7d0..b8417cf04e2 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/notification/NewIssuesStatisticsTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/notification/NewIssuesStatisticsTest.java @@ -74,7 +74,7 @@ public class NewIssuesStatisticsTest { assertThat(underTest.globalStatistics().effort().getTotal()).isEqualTo(15L); assertThat(underTest.globalStatistics().hasIssues()).isTrue(); assertThat(underTest.hasIssues()).isTrue(); - assertThat(underTest.assigneesStatistics().get("maynard").hasIssues()).isTrue(); + assertThat(underTest.getAssigneesStatistics().get("maynard").hasIssues()).isTrue(); } @Test @@ -85,7 +85,7 @@ public class NewIssuesStatisticsTest { .forEach(underTest::add); DistributedMetricStatsInt globalDistribution = underTest.globalStatistics().getDistributedMetricStats(Metric.SEVERITY); - DistributedMetricStatsInt assigneeDistribution = underTest.assigneesStatistics().get(assignee).getDistributedMetricStats(Metric.SEVERITY); + DistributedMetricStatsInt assigneeDistribution = underTest.getAssigneesStatistics().get(assignee).getDistributedMetricStats(Metric.SEVERITY); Stream.of(globalDistribution, assigneeDistribution) .forEach(distribution -> { assertStats(distribution, Severity.INFO, 1, 0, 1); @@ -104,7 +104,7 @@ public class NewIssuesStatisticsTest { .forEach(underTest::add); DistributedMetricStatsInt globalDistribution = underTest.globalStatistics().getDistributedMetricStats(Metric.SEVERITY); - DistributedMetricStatsInt assigneeDistribution = underTest.assigneesStatistics().get(assignee).getDistributedMetricStats(Metric.SEVERITY); + DistributedMetricStatsInt assigneeDistribution = underTest.getAssigneesStatistics().get(assignee).getDistributedMetricStats(Metric.SEVERITY); Stream.of(globalDistribution, assigneeDistribution) .forEach(distribution -> { assertStats(distribution, Severity.INFO, 0, 1, 1); @@ -121,7 +121,7 @@ public class NewIssuesStatisticsTest { underTest.add(new DefaultIssue().setSeverity(null).setAssignee(assignee).setNew(new Random().nextBoolean())); DistributedMetricStatsInt globalDistribution = underTest.globalStatistics().getDistributedMetricStats(Metric.SEVERITY); - DistributedMetricStatsInt assigneeDistribution = underTest.assigneesStatistics().get(assignee).getDistributedMetricStats(Metric.SEVERITY); + DistributedMetricStatsInt assigneeDistribution = underTest.getAssigneesStatistics().get(assignee).getDistributedMetricStats(Metric.SEVERITY); Stream.of(globalDistribution, assigneeDistribution) .forEach(distribution -> { assertThat(distribution.getTotal()).isEqualTo(1); @@ -138,7 +138,7 @@ public class NewIssuesStatisticsTest { .forEach(underTest::add); DistributedMetricStatsInt globalDistribution = underTest.globalStatistics().getDistributedMetricStats(Metric.COMPONENT); - DistributedMetricStatsInt assigneeDistribution = underTest.assigneesStatistics().get(assignee).getDistributedMetricStats(Metric.COMPONENT); + DistributedMetricStatsInt assigneeDistribution = underTest.getAssigneesStatistics().get(assignee).getDistributedMetricStats(Metric.COMPONENT); Stream.of(globalDistribution, assigneeDistribution) .forEach(distribution -> componentUuids.forEach(componentUuid -> assertStats(distribution, componentUuid, 1, 0, 1))); } @@ -152,7 +152,7 @@ public class NewIssuesStatisticsTest { .forEach(underTest::add); DistributedMetricStatsInt globalDistribution = underTest.globalStatistics().getDistributedMetricStats(Metric.COMPONENT); - NewIssuesStatistics.Stats stats = underTest.assigneesStatistics().get(assignee); + NewIssuesStatistics.Stats stats = underTest.getAssigneesStatistics().get(assignee); DistributedMetricStatsInt assigneeDistribution = stats.getDistributedMetricStats(Metric.COMPONENT); Stream.of(globalDistribution, assigneeDistribution) .forEach(distribution -> componentUuids.forEach(componentUuid -> assertStats(distribution, componentUuid, 0, 1, 1))); @@ -164,7 +164,7 @@ public class NewIssuesStatisticsTest { underTest.add(new DefaultIssue().setComponentUuid(null).setAssignee(assignee).setNew(new Random().nextBoolean())); DistributedMetricStatsInt globalDistribution = underTest.globalStatistics().getDistributedMetricStats(Metric.COMPONENT); - DistributedMetricStatsInt assigneeDistribution = underTest.assigneesStatistics().get(assignee).getDistributedMetricStats(Metric.COMPONENT); + DistributedMetricStatsInt assigneeDistribution = underTest.getAssigneesStatistics().get(assignee).getDistributedMetricStats(Metric.COMPONENT); Stream.of(globalDistribution, assigneeDistribution) .forEach(distribution -> { assertThat(distribution.getTotal()).isEqualTo(1); @@ -182,7 +182,7 @@ public class NewIssuesStatisticsTest { .forEach(underTest::add); DistributedMetricStatsInt globalDistribution = underTest.globalStatistics().getDistributedMetricStats(Metric.RULE); - NewIssuesStatistics.Stats stats = underTest.assigneesStatistics().get(assignee); + NewIssuesStatistics.Stats stats = underTest.getAssigneesStatistics().get(assignee); DistributedMetricStatsInt assigneeDistribution = stats.getDistributedMetricStats(Metric.RULE); Stream.of(globalDistribution, assigneeDistribution) .forEach(distribution -> ruleKeys.forEach(ruleKey -> assertStats(distribution, RuleKey.of(repository, ruleKey).toString(), 1, 0, 1))); @@ -198,7 +198,7 @@ public class NewIssuesStatisticsTest { .forEach(underTest::add); DistributedMetricStatsInt globalDistribution = underTest.globalStatistics().getDistributedMetricStats(Metric.RULE); - DistributedMetricStatsInt assigneeDistribution = underTest.assigneesStatistics().get(assignee).getDistributedMetricStats(Metric.RULE); + DistributedMetricStatsInt assigneeDistribution = underTest.getAssigneesStatistics().get(assignee).getDistributedMetricStats(Metric.RULE); Stream.of(globalDistribution, assigneeDistribution) .forEach(distribution -> ruleKeys.forEach(ruleKey -> assertStats(distribution, RuleKey.of(repository, ruleKey).toString(), 0, 1, 1))); } @@ -209,7 +209,7 @@ public class NewIssuesStatisticsTest { underTest.add(new DefaultIssue().setRuleKey(null).setAssignee(assignee).setNew(new Random().nextBoolean())); DistributedMetricStatsInt globalDistribution = underTest.globalStatistics().getDistributedMetricStats(Metric.RULE); - DistributedMetricStatsInt assigneeDistribution = underTest.assigneesStatistics().get(assignee).getDistributedMetricStats(Metric.RULE); + DistributedMetricStatsInt assigneeDistribution = underTest.getAssigneesStatistics().get(assignee).getDistributedMetricStats(Metric.RULE); Stream.of(globalDistribution, assigneeDistribution) .forEach(distribution -> { assertThat(distribution.getTotal()).isEqualTo(0); @@ -227,7 +227,7 @@ public class NewIssuesStatisticsTest { DistributedMetricStatsInt globalDistribution = underTest.globalStatistics().getDistributedMetricStats(Metric.ASSIGNEE); assignees.forEach(assignee -> assertStats(globalDistribution, assignee, 1, 0, 1)); assignees.forEach(assignee -> { - NewIssuesStatistics.Stats stats = underTest.assigneesStatistics().get(assignee); + NewIssuesStatistics.Stats stats = underTest.getAssigneesStatistics().get(assignee); DistributedMetricStatsInt assigneeStats = stats.getDistributedMetricStats(Metric.ASSIGNEE); assertThat(assigneeStats.getOnLeak()).isEqualTo(1); assertThat(assigneeStats.getOffLeak()).isEqualTo(0); @@ -257,7 +257,7 @@ public class NewIssuesStatisticsTest { DistributedMetricStatsInt globalDistribution = underTest.globalStatistics().getDistributedMetricStats(Metric.ASSIGNEE); assignees.forEach(assignee -> assertStats(globalDistribution, assignee, 0, 1, 1)); assignees.forEach(assignee -> { - NewIssuesStatistics.Stats stats = underTest.assigneesStatistics().get(assignee); + NewIssuesStatistics.Stats stats = underTest.getAssigneesStatistics().get(assignee); DistributedMetricStatsInt assigneeStats = stats.getDistributedMetricStats(Metric.ASSIGNEE); assertThat(assigneeStats.getOnLeak()).isEqualTo(0); assertThat(assigneeStats.getOffLeak()).isEqualTo(1); @@ -284,7 +284,7 @@ public class NewIssuesStatisticsTest { DistributedMetricStatsInt globalDistribution = underTest.globalStatistics().getDistributedMetricStats(Metric.ASSIGNEE); assertThat(globalDistribution.getTotal()).isEqualTo(0); assertThat(globalDistribution.getForLabel(null).isPresent()).isFalse(); - assertThat(underTest.assigneesStatistics()).isEmpty(); + assertThat(underTest.getAssigneesStatistics()).isEmpty(); } @Test @@ -294,7 +294,7 @@ public class NewIssuesStatisticsTest { underTest.add(new DefaultIssue().setTags(tags).setAssignee(assignee).setNew(true)); DistributedMetricStatsInt globalDistribution = underTest.globalStatistics().getDistributedMetricStats(Metric.TAG); - DistributedMetricStatsInt assigneeDistribution = underTest.assigneesStatistics().get(assignee).getDistributedMetricStats(Metric.TAG); + DistributedMetricStatsInt assigneeDistribution = underTest.getAssigneesStatistics().get(assignee).getDistributedMetricStats(Metric.TAG); Stream.of(globalDistribution, assigneeDistribution) .forEach(distribution -> tags.forEach(tag -> assertStats(distribution, tag, 1, 0, 1))); } @@ -306,7 +306,7 @@ public class NewIssuesStatisticsTest { underTest.add(new DefaultIssue().setTags(tags).setAssignee(assignee).setNew(false)); DistributedMetricStatsInt globalDistribution = underTest.globalStatistics().getDistributedMetricStats(Metric.TAG); - DistributedMetricStatsInt assigneeDistribution = underTest.assigneesStatistics().get(assignee).getDistributedMetricStats(Metric.TAG); + DistributedMetricStatsInt assigneeDistribution = underTest.getAssigneesStatistics().get(assignee).getDistributedMetricStats(Metric.TAG); Stream.of(globalDistribution, assigneeDistribution) .forEach(distribution -> tags.forEach(tag -> assertStats(distribution, tag, 0, 1, 1))); } @@ -317,7 +317,7 @@ public class NewIssuesStatisticsTest { underTest.add(new DefaultIssue().setTags(Collections.emptyList()).setAssignee(assignee).setNew(new Random().nextBoolean())); DistributedMetricStatsInt globalDistribution = underTest.globalStatistics().getDistributedMetricStats(Metric.TAG); - DistributedMetricStatsInt assigneeDistribution = underTest.assigneesStatistics().get(assignee).getDistributedMetricStats(Metric.TAG); + DistributedMetricStatsInt assigneeDistribution = underTest.getAssigneesStatistics().get(assignee).getDistributedMetricStats(Metric.TAG); Stream.of(globalDistribution, assigneeDistribution) .forEach(distribution -> { assertThat(distribution.getTotal()).isEqualTo(0); @@ -336,7 +336,7 @@ public class NewIssuesStatisticsTest { .forEach(underTest::add); MetricStatsLong globalDistribution = underTest.globalStatistics().effort(); - MetricStatsLong assigneeDistribution = underTest.assigneesStatistics().get(assignee).effort(); + MetricStatsLong assigneeDistribution = underTest.getAssigneesStatistics().get(assignee).effort(); Stream.of(globalDistribution, assigneeDistribution) .forEach(distribution -> { assertThat(distribution.getOnLeak()).isEqualTo(expected); @@ -356,7 +356,7 @@ public class NewIssuesStatisticsTest { .forEach(underTest::add); MetricStatsLong globalDistribution = underTest.globalStatistics().effort(); - MetricStatsLong assigneeDistribution = underTest.assigneesStatistics().get(assignee).effort(); + MetricStatsLong assigneeDistribution = underTest.getAssigneesStatistics().get(assignee).effort(); Stream.of(globalDistribution, assigneeDistribution) .forEach(distribution -> { assertThat(distribution.getOnLeak()).isEqualTo(0); @@ -371,7 +371,7 @@ public class NewIssuesStatisticsTest { underTest.add(new DefaultIssue().setEffort(null).setAssignee(assignee).setNew(new Random().nextBoolean())); MetricStatsLong globalDistribution = underTest.globalStatistics().effort(); - MetricStatsLong assigneeDistribution = underTest.assigneesStatistics().get(assignee).effort(); + MetricStatsLong assigneeDistribution = underTest.getAssigneesStatistics().get(assignee).effort(); Stream.of(globalDistribution, assigneeDistribution) .forEach(distribution -> assertThat(distribution.getTotal()).isEqualTo(0)); } diff --git a/tests/src/test/java/org/sonarqube/tests/issue/IssueCreationDatePluginChangedTest.java b/tests/src/test/java/org/sonarqube/tests/issue/IssueCreationDatePluginChangedTest.java index 98ab45d77d3..e05168fb497 100644 --- a/tests/src/test/java/org/sonarqube/tests/issue/IssueCreationDatePluginChangedTest.java +++ b/tests/src/test/java/org/sonarqube/tests/issue/IssueCreationDatePluginChangedTest.java @@ -19,23 +19,33 @@ */ package org.sonarqube.tests.issue; +import com.google.common.collect.ImmutableList; import com.sonar.orchestrator.Orchestrator; import com.sonar.orchestrator.build.SonarScanner; import java.io.File; +import java.io.IOException; import java.text.ParseException; import java.text.SimpleDateFormat; import java.util.Date; import java.util.List; +import javax.mail.MessagingException; +import org.junit.AfterClass; import org.junit.Before; +import org.junit.BeforeClass; import org.junit.ClassRule; +import org.junit.Rule; import org.junit.Test; import org.sonar.wsclient.issue.Issue; import org.sonar.wsclient.issue.IssueQuery; +import org.sonarqube.tests.Tester; import org.sonarqube.ws.client.PostRequest; +import org.sonarqube.ws.client.WsClient; +import org.subethamail.wiser.Wiser; import util.ItUtils; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.tuple; +import static util.ItUtils.newUserWsClient; import static util.ItUtils.pluginArtifact; import static util.ItUtils.projectDir; import static util.ItUtils.xooPlugin; @@ -55,20 +65,59 @@ public class IssueCreationDatePluginChangedTest { private static final String SAMPLE_PROJECT_NAME = "Creation date sample"; private static final String SAMPLE_QUALITY_PROFILE_NAME = "creation-date-plugin"; + private final static String USER_LOGIN = "tester"; + private final static String USER_PASSWORD = "tester"; + private static final String USER_EMAIL = "tester@example.org"; + @ClassRule public static final Orchestrator ORCHESTRATOR = Orchestrator.builderEnv() .addPlugin(xooPlugin()) .addPlugin(ItUtils.pluginArtifact("backdating-plugin-v1")) .addPlugin(ItUtils.pluginArtifact("backdating-customplugin")) .build(); + @Rule + public Tester tester = new Tester(ORCHESTRATOR); + + private static Wiser smtpServer; + + @BeforeClass + public static void setUp() throws Exception { + smtpServer = new Wiser(0); + smtpServer.start(); + } @Before - public void cleanup() { + public void before() throws InterruptedException, MessagingException, IOException { ORCHESTRATOR.resetData(); + + // Configure Sonar + tester.settings().setGlobalSetting("email.smtp_host.secured", "localhost"); + tester.settings().setGlobalSetting("email.smtp_port.secured", Integer.toString(smtpServer.getServer().getPort())); + + // Create a user and register her to receive notification on NewIssues + tester.users().generate(t -> t.setLogin(USER_LOGIN).setPassword(USER_PASSWORD).setEmail(USER_EMAIL) + .setScmAccounts(ImmutableList.of("jhenry"))); + // Add notifications to the test user + WsClient wsClient = newUserWsClient(ORCHESTRATOR, USER_LOGIN, USER_PASSWORD); + wsClient.wsConnector().call(new PostRequest("api/notifications/add") + .setParam("type", "NewIssues") + .setParam("channel", "EmailNotificationChannel")) + .failIfNotSuccessful(); + wsClient.wsConnector().call(new PostRequest("api/notifications/add") + .setParam("type", "SQ-MyNewIssues") + .setParam("channel", "EmailNotificationChannel")) + .failIfNotSuccessful(); + } + + @AfterClass + public static void stop() { + if (smtpServer != null) { + smtpServer.stop(); + } } @Test - public void should_use_scm_date_for_new_issues_if_plugin_updated() { + public void should_use_scm_date_for_new_issues_if_plugin_updated() throws InterruptedException, MessagingException, IOException { ItUtils.restoreProfile(ORCHESTRATOR, getClass().getResource("/issue/IssueCreationDatePluginChangedTest/profile.xml")); ORCHESTRATOR.getServer().provisionProject(SAMPLE_PROJECT_KEY, SAMPLE_PROJECT_NAME); @@ -80,13 +129,16 @@ public class IssueCreationDatePluginChangedTest { .setProperty("sonar.scm.disabled", "false"); ORCHESTRATOR.executeBuild(scanner); - List issues = getIssues(issueQuery().components("creation-date-sample:src/main/xoo/sample/Sample.xoo")); - // Check that issue is backdated to SCM (because it is the first analysis) + List issues = getIssues(issueQuery().components("creation-date-sample:src/main/xoo/sample/Sample.xoo")); assertThat(issues) .extracting(Issue::line, Issue::creationDate) .containsExactly(tuple(1, dateTimeParse("2005-01-01T00:00:00+0000"))); + // ensure no notification is sent as all issues are off the leak period + waitUntilAllNotificationsAreDelivered(); + assertThat(smtpServer.getMessages()).isEmpty(); + // Update the plugin // uninstall plugin V1 ItUtils.newAdminWsClient(ORCHESTRATOR).wsConnector().call(new PostRequest("api/plugins/uninstall").setParam("key", "backdating")).failIfNotSuccessful(); @@ -104,6 +156,10 @@ public class IssueCreationDatePluginChangedTest { .containsExactly(tuple(1, dateTimeParse("2005-01-01T00:00:00+0000")), tuple(2, dateTimeParse("2005-01-01T00:00:00+0000")), tuple(3, dateTimeParse("2005-01-01T00:00:00+0000"))); + + // ensure no notification is sent as all issues are off the leak period + waitUntilAllNotificationsAreDelivered(); + assertThat(smtpServer.getMessages()).isEmpty(); } private static List getIssues(IssueQuery query) { @@ -122,4 +178,13 @@ public class IssueCreationDatePluginChangedTest { } } + private static void waitUntilAllNotificationsAreDelivered() throws InterruptedException { + for (int i = 0; i < 10; i++) { + if (smtpServer.getMessages().size() == 3) { + break; + } + Thread.sleep(1_000); + } + } + } diff --git a/tests/src/test/java/org/sonarqube/tests/issue/IssueNotificationsTest.java b/tests/src/test/java/org/sonarqube/tests/issue/IssueNotificationsTest.java index 49e4cee8afc..c110478180b 100644 --- a/tests/src/test/java/org/sonarqube/tests/issue/IssueNotificationsTest.java +++ b/tests/src/test/java/org/sonarqube/tests/issue/IssueNotificationsTest.java @@ -172,6 +172,13 @@ public class IssueNotificationsTest extends AbstractIssueTest { @Test public void notifications_for_personalized_emails() throws Exception { setServerProperty(ORCHESTRATOR, "sonar.issues.defaultAssigneeLogin", USER_LOGIN); + // 1st analysis without any issue (because no file is analyzed) + runProjectAnalysis(ORCHESTRATOR, "issue/xoo-with-scm", "sonar.scm.provider", "xoo", "sonar.scm.disabled", "false", "sonar.exclusions", "**/*"); + + waitUntilAllNotificationsAreDelivered(2); + assertThat(smtpServer.getMessages()).isEmpty(); + + // run 2nd analysis which will generate issues on the leak period runProjectAnalysis(ORCHESTRATOR, "issue/xoo-with-scm", "sonar.scm.provider", "xoo", "sonar.scm.disabled", "false"); waitUntilAllNotificationsAreDelivered(2); -- 2.39.5