From 5a9d2364785090379ad5e59a78b061f9d8e2866e Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Mon, 30 Jul 2018 15:06:46 +0200 Subject: [PATCH] SONAR-11077 add statistics to CE SendIssueNotificationsStep --- .../step/SendIssueNotificationsStep.java | 51 ++++++++++---- .../step/SendIssueNotificationsStepTest.java | 69 +++++++++++++------ .../notification/NotificationService.java | 12 ++-- .../email/EmailNotificationChannel.java | 16 ++--- .../notification/NotificationChannelTest.java | 3 +- .../email/EmailNotificationChannelTest.java | 13 ++-- .../server/notification/ws/AddActionTest.java | 3 +- .../notification/ws/ListActionTest.java | 3 +- .../notification/ws/RemoveActionTest.java | 3 +- .../notifications/NotificationChannel.java | 3 +- 10 files changed, 122 insertions(+), 54 deletions(-) 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 92cee140446..851b2301e12 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 @@ -98,12 +98,14 @@ public class SendIssueNotificationsStep implements ComputationStep { @Override public void execute(ComputationStep.Context context) { Component project = treeRootHolder.getRoot(); + NotificationStatistics notificationStatistics = new NotificationStatistics(); if (service.hasProjectSubscribersForTypes(project.getUuid(), NOTIF_TYPES)) { - doExecute(project); + doExecute(notificationStatistics, project); } + notificationStatistics.dumpTo(context); } - private void doExecute(Component project) { + private void doExecute(NotificationStatistics notificationStatistics, Component project) { long analysisDate = analysisMetadataHolder.getAnalysisDate(); Predicate isOnLeakPredicate = i -> i.isNew() && i.creationDate().getTime() >= truncateToSeconds(analysisDate); NewIssuesStatistics newIssuesStats = new NewIssuesStatistics(isOnLeakPredicate); @@ -113,12 +115,13 @@ public class SendIssueNotificationsStep implements ComputationStep { List assigneeUuids = stream(iterable.spliterator(), false).map(DefaultIssue::assignee).filter(Objects::nonNull).collect(toList()); usersDtoByUuids = dbClient.userDao().selectByUuids(dbSession, assigneeUuids).stream().collect(toMap(UserDto::getUuid, dto -> dto)); } + try (CloseableIterator issues = issueCache.traverse()) { - processIssues(newIssuesStats, issues, project, usersDtoByUuids); + processIssues(newIssuesStats, issues, project, usersDtoByUuids, notificationStatistics); } if (newIssuesStats.hasIssuesOnLeak()) { - sendNewIssuesNotification(newIssuesStats, project, analysisDate); - sendNewIssuesNotificationToAssignees(newIssuesStats, project, analysisDate); + sendNewIssuesNotification(newIssuesStats, project, analysisDate, notificationStatistics); + sendMyNewIssuesNotification(newIssuesStats, project, analysisDate, notificationStatistics); } } @@ -132,30 +135,32 @@ public class SendIssueNotificationsStep implements ComputationStep { return Date.from(instant).getTime(); } - private void processIssues(NewIssuesStatistics newIssuesStats, CloseableIterator issues, Component project, Map usersDtoByUuids) { + private void processIssues(NewIssuesStatistics newIssuesStats, CloseableIterator issues, Component project, Map usersDtoByUuids, + NotificationStatistics notificationStatistics) { while (issues.hasNext()) { DefaultIssue issue = issues.next(); 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); + sendIssueChangeNotification(issue, project, usersDtoByUuids, notificationStatistics); } } } } - private void sendIssueChangeNotification(DefaultIssue issue, Component project, Map usersDtoByUuids) { + private void sendIssueChangeNotification(DefaultIssue issue, Component project, Map usersDtoByUuids, NotificationStatistics notificationStatistics) { IssueChangeNotification changeNotification = new IssueChangeNotification(); changeNotification.setRuleName(rules.getByKey(issue.ruleKey()).getName()); changeNotification.setIssue(issue); changeNotification.setAssignee(usersDtoByUuids.get(issue.assignee())); changeNotification.setProject(project.getPublicKey(), project.getName(), getBranchName(), getPullRequest()); getComponentKey(issue).ifPresent(c -> changeNotification.setComponent(c.getPublicKey(), c.getName())); - service.deliver(changeNotification); + notificationStatistics.issueChangesDeliveries += service.deliver(changeNotification); + notificationStatistics.issueChanges++; } - private void sendNewIssuesNotification(NewIssuesStatistics statistics, Component project, long analysisDate) { + private void sendNewIssuesNotification(NewIssuesStatistics statistics, Component project, long analysisDate, NotificationStatistics notificationStatistics) { NewIssuesStatistics.Stats globalStatistics = statistics.globalStatistics(); NewIssuesNotification notification = newIssuesNotificationFactory .newNewIssuesNotification() @@ -164,10 +169,11 @@ public class SendIssueNotificationsStep implements ComputationStep { .setAnalysisDate(new Date(analysisDate)) .setStatistics(project.getName(), globalStatistics) .setDebt(Duration.create(globalStatistics.effort().getOnLeak())); - service.deliver(notification); + notificationStatistics.newIssuesDeliveries += service.deliver(notification); + notificationStatistics.newIssues++; } - private void sendNewIssuesNotificationToAssignees(NewIssuesStatistics statistics, Component project, long analysisDate) { + private void sendMyNewIssuesNotification(NewIssuesStatistics statistics, Component project, long analysisDate, NotificationStatistics notificationStatistics) { Map userDtoByUuid = loadUserDtoByUuid(statistics); statistics.getAssigneesStatistics().entrySet() .stream() @@ -185,7 +191,8 @@ public class SendIssueNotificationsStep implements ComputationStep { .setStatistics(project.getName(), assigneeStatistics) .setDebt(Duration.create(assigneeStatistics.effort().getOnLeak())); - service.deliver(myNewIssuesNotification); + notificationStatistics.myNewIssuesDeliveries += service.deliver(myNewIssuesNotification); + notificationStatistics.myNewIssues++; }); } @@ -230,4 +237,22 @@ public class SendIssueNotificationsStep implements ComputationStep { return branch.getType() == PULL_REQUEST ? analysisMetadataHolder.getPullRequestKey() : null; } + private static class NotificationStatistics { + private int issueChanges = 0; + private int issueChangesDeliveries = 0; + private int newIssues = 0; + private int newIssuesDeliveries = 0; + private int myNewIssues = 0; + private int myNewIssuesDeliveries = 0; + + private void dumpTo(ComputationStep.Context context) { + context.getStatistics() + .add("newIssuesNotifs", newIssues) + .add("newIssuesDeliveries", newIssuesDeliveries) + .add("myNewIssuesNotifs", myNewIssues) + .add("myNewIssuesDeliveries", myNewIssuesDeliveries) + .add("changesNotifs", issueChanges) + .add("changesDeliveries", issueChangesDeliveries); + } + } } 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 d908494ac40..0c4c2db21e2 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 @@ -33,7 +33,6 @@ import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; import org.mockito.ArgumentCaptor; -import org.sonar.api.notifications.Notification; import org.sonar.api.rules.RuleType; import org.sonar.api.utils.Duration; import org.sonar.api.utils.System2; @@ -139,9 +138,11 @@ public class SendIssueNotificationsStepTest extends BaseStepTest { public void do_not_send_notifications_if_no_subscribers() { when(notificationService.hasProjectSubscribersForTypes(PROJECT.getUuid(), NOTIF_TYPES)).thenReturn(false); - underTest.execute(new TestComputationStepContext()); + TestComputationStepContext context = new TestComputationStepContext(); + underTest.execute(context); - verify(notificationService, never()).deliver(any(Notification.class)); + verify(notificationService, never()).deliver(any()); + verifyStatistics(context, 0, 0, 0); } @Test @@ -152,13 +153,15 @@ public class SendIssueNotificationsStepTest extends BaseStepTest { .close(); when(notificationService.hasProjectSubscribersForTypes(eq(PROJECT.getUuid()), any())).thenReturn(true); - underTest.execute(new TestComputationStepContext()); + TestComputationStepContext context = new TestComputationStepContext(); + underTest.execute(context); verify(notificationService).deliver(newIssuesNotificationMock); verify(newIssuesNotificationMock).setProject(PROJECT.getPublicKey(), PROJECT.getName(), null, null); verify(newIssuesNotificationMock).setAnalysisDate(new Date(ANALYSE_DATE)); verify(newIssuesNotificationMock).setStatistics(eq(PROJECT.getName()), any()); verify(newIssuesNotificationMock).setDebt(ISSUE_DURATION); + verifyStatistics(context, 1, 0, 0); } @Test @@ -179,7 +182,8 @@ public class SendIssueNotificationsStepTest extends BaseStepTest { issues.forEach(issueCache::append); when(notificationService.hasProjectSubscribersForTypes(PROJECT.getUuid(), NOTIF_TYPES)).thenReturn(true); - underTest.execute(new TestComputationStepContext()); + TestComputationStepContext context = new TestComputationStepContext(); + underTest.execute(context); verify(notificationService).deliver(newIssuesNotificationMock); ArgumentCaptor statsCaptor = forClass(NewIssuesStatistics.Stats.class); @@ -191,6 +195,7 @@ public class SendIssueNotificationsStepTest extends BaseStepTest { DistributedMetricStatsInt severity = stats.getDistributedMetricStats(NewIssuesStatistics.Metric.RULE_TYPE); assertThat(severity.getOnLeak()).isEqualTo(efforts.length); assertThat(severity.getTotal()).isEqualTo(backDatedEfforts.length + efforts.length); + verifyStatistics(context, 1, 0, 0); } @Test @@ -201,9 +206,11 @@ public class SendIssueNotificationsStepTest extends BaseStepTest { .close(); when(notificationService.hasProjectSubscribersForTypes(PROJECT.getUuid(), NOTIF_TYPES)).thenReturn(true); - underTest.execute(new TestComputationStepContext()); + TestComputationStepContext context = new TestComputationStepContext(); + underTest.execute(context); - verify(notificationService, never()).deliver(any(Notification.class)); + verify(notificationService, never()).deliver(any()); + verifyStatistics(context, 0, 0, 0); } @Test @@ -214,13 +221,15 @@ public class SendIssueNotificationsStepTest extends BaseStepTest { when(notificationService.hasProjectSubscribersForTypes(branch.uuid(), NOTIF_TYPES)).thenReturn(true); analysisMetadataHolder.setBranch(newBranch()); - underTest.execute(new TestComputationStepContext()); + TestComputationStepContext context = new TestComputationStepContext(); + underTest.execute(context); verify(notificationService).deliver(newIssuesNotificationMock); verify(newIssuesNotificationMock).setProject(branch.getKey(), branch.longName(), BRANCH_NAME, null); verify(newIssuesNotificationMock).setAnalysisDate(new Date(ANALYSE_DATE)); verify(newIssuesNotificationMock).setStatistics(eq(branch.longName()), any(NewIssuesStatistics.Stats.class)); verify(newIssuesNotificationMock).setDebt(ISSUE_DURATION); + verifyStatistics(context, 1, 0, 0); } @Test @@ -232,13 +241,15 @@ public class SendIssueNotificationsStepTest extends BaseStepTest { analysisMetadataHolder.setBranch(newPullRequest()); analysisMetadataHolder.setPullRequestKey(PULL_REQUEST_ID); - underTest.execute(new TestComputationStepContext()); + TestComputationStepContext context = new TestComputationStepContext(); + underTest.execute(context); verify(notificationService).deliver(newIssuesNotificationMock); verify(newIssuesNotificationMock).setProject(branch.getKey(), branch.longName(), null, PULL_REQUEST_ID); verify(newIssuesNotificationMock).setAnalysisDate(new Date(ANALYSE_DATE)); verify(newIssuesNotificationMock).setStatistics(eq(branch.longName()), any(NewIssuesStatistics.Stats.class)); verify(newIssuesNotificationMock).setDebt(ISSUE_DURATION); + verifyStatistics(context, 1, 0, 0); } @Test @@ -249,14 +260,15 @@ public class SendIssueNotificationsStepTest extends BaseStepTest { when(notificationService.hasProjectSubscribersForTypes(branch.uuid(), NOTIF_TYPES)).thenReturn(true); analysisMetadataHolder.setBranch(newBranch()); - underTest.execute(new TestComputationStepContext()); + TestComputationStepContext context = new TestComputationStepContext(); + underTest.execute(context); - verify(notificationService, never()).deliver(any(Notification.class)); + verify(notificationService, never()).deliver(any()); + verifyStatistics(context, 0, 0, 0); } @Test public void send_new_issues_notification_to_user() { - UserDto user = db.users().insertUser(); issueCache.newAppender().append( @@ -265,7 +277,8 @@ public class SendIssueNotificationsStepTest extends BaseStepTest { .close(); when(notificationService.hasProjectSubscribersForTypes(eq(PROJECT.getUuid()), any())).thenReturn(true); - underTest.execute(new TestComputationStepContext()); + TestComputationStepContext context = new TestComputationStepContext(); + underTest.execute(context); verify(notificationService).deliver(newIssuesNotificationMock); verify(notificationService).deliver(myNewIssuesNotificationMock); @@ -274,11 +287,11 @@ public class SendIssueNotificationsStepTest extends BaseStepTest { verify(myNewIssuesNotificationMock).setAnalysisDate(new Date(ANALYSE_DATE)); verify(myNewIssuesNotificationMock).setStatistics(eq(PROJECT.getName()), any(NewIssuesStatistics.Stats.class)); verify(myNewIssuesNotificationMock).setDebt(ISSUE_DURATION); + verifyStatistics(context, 1, 1, 0); } @Test public void send_new_issues_notification_to_user_only_for_those_assigned_to_her() throws IOException { - UserDto perceval = db.users().insertUser(u -> u.setLogin("perceval")); Integer[] assigned = IntStream.range(0, 5).mapToObj(i -> 10_000 * i).toArray(Integer[]::new); Duration expectedEffort = Duration.create(stream(assigned).mapToInt(i -> i).sum()); @@ -310,8 +323,9 @@ public class SendIssueNotificationsStepTest extends BaseStepTest { MyNewIssuesNotification myNewIssuesNotificationMock2 = createMyNewIssuesNotificationMock(); when(newIssuesNotificationFactory.newMyNewIssuesNotification()).thenReturn(myNewIssuesNotificationMock1).thenReturn(myNewIssuesNotificationMock2); + TestComputationStepContext context = new TestComputationStepContext(); new SendIssueNotificationsStep(issueCache, ruleRepository, treeRootHolder, notificationService, analysisMetadataHolder, newIssuesNotificationFactory, db.getDbClient()) - .execute(new TestComputationStepContext()); + .execute(context); verify(notificationService).deliver(myNewIssuesNotificationMock1); Map myNewIssuesNotificationMocksByUsersName = new HashMap<>(); @@ -335,6 +349,8 @@ public class SendIssueNotificationsStepTest extends BaseStepTest { DistributedMetricStatsInt severity = stats.getDistributedMetricStats(NewIssuesStatistics.Metric.RULE_TYPE); assertThat(severity.getOnLeak()).isEqualTo(assigned.length); assertThat(severity.getTotal()).isEqualTo(assigned.length); + + verifyStatistics(context, 1, 2, 0); } @Test @@ -358,7 +374,8 @@ public class SendIssueNotificationsStepTest extends BaseStepTest { issues.forEach(issueCache::append); when(notificationService.hasProjectSubscribersForTypes(PROJECT.getUuid(), NOTIF_TYPES)).thenReturn(true); - underTest.execute(new TestComputationStepContext()); + TestComputationStepContext context = new TestComputationStepContext(); + underTest.execute(context); verify(notificationService).deliver(newIssuesNotificationMock); verify(notificationService).deliver(myNewIssuesNotificationMock); @@ -372,6 +389,8 @@ public class SendIssueNotificationsStepTest extends BaseStepTest { DistributedMetricStatsInt severity = stats.getDistributedMetricStats(NewIssuesStatistics.Metric.RULE_TYPE); assertThat(severity.getOnLeak()).isEqualTo(efforts.length); assertThat(severity.getTotal()).isEqualTo(backDatedEfforts.length + efforts.length); + + verifyStatistics(context, 1, 1, 0); } @Test @@ -383,9 +402,11 @@ public class SendIssueNotificationsStepTest extends BaseStepTest { .close(); when(notificationService.hasProjectSubscribersForTypes(PROJECT.getUuid(), NOTIF_TYPES)).thenReturn(true); - underTest.execute(new TestComputationStepContext()); + TestComputationStepContext context = new TestComputationStepContext(); + underTest.execute(context); - verify(notificationService, never()).deliver(any(Notification.class)); + verify(notificationService, never()).deliver(any()); + verifyStatistics(context, 0, 0, 0); } @Test @@ -401,9 +422,11 @@ public class SendIssueNotificationsStepTest extends BaseStepTest { RuleDefinitionDto ruleDefinitionDto = newRule(); DefaultIssue issue = prepareIssue(ANALYSE_DATE, user, project, file, ruleDefinitionDto, RuleType.SECURITY_HOTSPOT); - underTest.execute(new TestComputationStepContext()); + TestComputationStepContext context = new TestComputationStepContext(); + underTest.execute(context); - verify(notificationService, never()).deliver(any(IssueChangeNotification.class)); + verify(notificationService, never()).deliver(any()); + verifyStatistics(context, 0, 0, 0); } @Test @@ -529,6 +552,12 @@ public class SendIssueNotificationsStepTest extends BaseStepTest { return branch; } + private static void verifyStatistics(TestComputationStepContext context, int expectedNewIssuesNotifications, int expectedMyNewIssuesNotifications, int expectedIssueChangesNotifications) { + context.getStatistics().assertValue("newIssuesNotifs", expectedNewIssuesNotifications); + context.getStatistics().assertValue("myNewIssuesNotifs", expectedMyNewIssuesNotifications); + context.getStatistics().assertValue("changesNotifs", expectedIssueChangesNotifications); + } + @Override protected ComputationStep step() { return underTest; diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/notification/NotificationService.java b/server/sonar-server-common/src/main/java/org/sonar/server/notification/NotificationService.java index 754380cbbae..56a5794dd20 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/notification/NotificationService.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/notification/NotificationService.java @@ -59,7 +59,7 @@ public class NotificationService { this(dbClient, new NotificationDispatcher[0]); } - public void deliver(Notification notification) { + public int deliver(Notification notification) { SetMultimap recipients = HashMultimap.create(); for (NotificationDispatcher dispatcher : dispatchers) { NotificationDispatcher.Context context = new ContextImpl(recipients); @@ -70,23 +70,27 @@ public class NotificationService { LOG.warn(String.format("Unable to dispatch notification %s using %s", notification, dispatcher), e); } } - dispatch(notification, recipients); + return dispatch(notification, recipients); } - private static void dispatch(Notification notification, SetMultimap recipients) { + private static int dispatch(Notification notification, SetMultimap recipients) { + int count = 0; for (Map.Entry> entry : recipients.asMap().entrySet()) { String username = entry.getKey(); Collection userChannels = entry.getValue(); LOG.debug("For user {} via {}", username, userChannels); for (NotificationChannel channel : userChannels) { try { - channel.deliver(notification, username); + if (channel.deliver(notification, username)) { + count++; + } } catch (Exception e) { // catch all exceptions in order to deliver via other channels LOG.warn("Unable to deliver notification " + notification + " for user " + username + " via " + channel, e); } } } + return count; } @VisibleForTesting diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/notification/email/EmailNotificationChannel.java b/server/sonar-server-common/src/main/java/org/sonar/server/notification/email/EmailNotificationChannel.java index 671a507ac83..87235eecceb 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/notification/email/EmailNotificationChannel.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/notification/email/EmailNotificationChannel.java @@ -95,17 +95,18 @@ public class EmailNotificationChannel extends NotificationChannel { } @Override - public void deliver(Notification notification, String username) { + public boolean deliver(Notification notification, String username) { User user = findByLogin(username); if (user == null || StringUtils.isBlank(user.email())) { LOG.debug("User does not exist or has no email: {}", username); - return; + return false; } EmailMessage emailMessage = format(notification); if (emailMessage != null) { emailMessage.setTo(user.email()); - deliver(emailMessage); + return deliver(emailMessage); } + return false; } @CheckForNull @@ -127,18 +128,17 @@ public class EmailNotificationChannel extends NotificationChannel { return null; } - /** - * Visibility has been relaxed for tests. - */ - void deliver(EmailMessage emailMessage) { + boolean deliver(EmailMessage emailMessage) { if (StringUtils.isBlank(configuration.getSmtpHost())) { LOG.debug("SMTP host was not configured - email will not be sent"); - return; + return false; } try { send(emailMessage); + return true; } catch (EmailException e) { LOG.error("Unable to send email", e); + return false; } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/notification/NotificationChannelTest.java b/server/sonar-server/src/test/java/org/sonar/server/notification/NotificationChannelTest.java index bdccede51b4..4e515770ca4 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/notification/NotificationChannelTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/notification/NotificationChannelTest.java @@ -36,7 +36,8 @@ public class NotificationChannelTest { private class FakeNotificationChannel extends NotificationChannel { @Override - public void deliver(Notification notification, String username) { + public boolean deliver(Notification notification, String username) { + return true; } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/notification/email/EmailNotificationChannelTest.java b/server/sonar-server/src/test/java/org/sonar/server/notification/email/EmailNotificationChannelTest.java index 6d057aefb51..3889e922340 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/notification/email/EmailNotificationChannelTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/notification/email/EmailNotificationChannelTest.java @@ -95,8 +95,9 @@ public class EmailNotificationChannelTest { .setTo("user@nowhere") .setSubject("Foo") .setMessage("Bar"); - underTest.deliver(emailMessage); + boolean delivered = underTest.deliver(emailMessage); assertThat(smtpServer.getMessages()).isEmpty(); + assertThat(delivered).isFalse(); } @Test @@ -108,7 +109,7 @@ public class EmailNotificationChannelTest { .setTo("user@nowhere") .setSubject("Review #3") .setMessage("I'll take care of this violation."); - underTest.deliver(emailMessage); + boolean delivered = underTest.deliver(emailMessage); List messages = smtpServer.getMessages(); assertThat(messages).hasSize(1); @@ -127,6 +128,7 @@ public class EmailNotificationChannelTest { assertThat(email.getHeader("To", null)).isEqualTo(""); assertThat(email.getHeader("Subject", null)).isEqualTo("[SONARQUBE] Review #3"); assertThat((String) email.getContent()).startsWith("I'll take care of this violation."); + assertThat(delivered).isTrue(); } @Test @@ -136,7 +138,7 @@ public class EmailNotificationChannelTest { .setTo("user@nowhere") .setSubject("Foo") .setMessage("Bar"); - underTest.deliver(emailMessage); + boolean delivered = underTest.deliver(emailMessage); List messages = smtpServer.getMessages(); assertThat(messages).hasSize(1); @@ -155,6 +157,7 @@ public class EmailNotificationChannelTest { assertThat(email.getHeader("To", null)).isEqualTo(""); assertThat(email.getHeader("Subject", null)).isEqualTo("[SONARQUBE] Foo"); assertThat((String) email.getContent()).startsWith("Bar"); + assertThat(delivered).isTrue(); } @Test @@ -166,7 +169,9 @@ public class EmailNotificationChannelTest { .setTo("user@nowhere") .setSubject("Foo") .setMessage("Bar"); - underTest.deliver(emailMessage); + boolean delivered = underTest.deliver(emailMessage); + + assertThat(delivered).isFalse(); } @Test diff --git a/server/sonar-server/src/test/java/org/sonar/server/notification/ws/AddActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/notification/ws/AddActionTest.java index 6d2b8365c32..7eb043b9d20 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/notification/ws/AddActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/notification/ws/AddActionTest.java @@ -376,8 +376,9 @@ public class AddActionTest { } @Override - public void deliver(Notification notification, String username) { + public boolean deliver(Notification notification, String username) { // do nothing + return true; } } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/notification/ws/ListActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/notification/ws/ListActionTest.java index f909bc781e7..c3d067432be 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/notification/ws/ListActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/notification/ws/ListActionTest.java @@ -311,8 +311,9 @@ public class ListActionTest { } @Override - public void deliver(org.sonar.api.notifications.Notification notification, String username) { + public boolean deliver(org.sonar.api.notifications.Notification notification, String username) { // do nothing + return true; } } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/notification/ws/RemoveActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/notification/ws/RemoveActionTest.java index f94dce62d3d..a80aa2d27ef 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/notification/ws/RemoveActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/notification/ws/RemoveActionTest.java @@ -345,8 +345,9 @@ public class RemoveActionTest { } @Override - public void deliver(Notification notification, String username) { + public boolean deliver(Notification notification, String username) { // do nothing + return true; } } } diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/notifications/NotificationChannel.java b/sonar-plugin-api/src/main/java/org/sonar/api/notifications/NotificationChannel.java index c1705f7f426..fc2e5d17993 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/notifications/NotificationChannel.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/notifications/NotificationChannel.java @@ -55,8 +55,9 @@ public abstract class NotificationChannel { * * @param notification the notification to deliver * @param userlogin the login of the user who should receive the notification + * @return whether the notification was sent or not */ - public abstract void deliver(Notification notification, String userlogin); + public abstract boolean deliver(Notification notification, String userlogin); @Override public String toString() { -- 2.39.5