SONAR-11077 add statistics to CE SendIssueNotificationsStep

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 92cee140446909ce60551b082b7819df3b8d869e..851b2301e126774b19d323f7077aad77aeb5868c 100644 (file)
--- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/SendIssueNotificationsStep.java
+++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/SendIssueNotificationsStep.java
@@ -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<DefaultIssue> isOnLeakPredicate = i -> i.isNew() && i.creationDate().getTime() >= truncateToSeconds(analysisDate);
     NewIssuesStatistics newIssuesStats = new NewIssuesStatistics(isOnLeakPredicate);
@@ -113,12 +115,13 @@ public class SendIssueNotificationsStep implements ComputationStep {
       List<String> 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<DefaultIssue> 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<DefaultIssue> issues, Component project, Map<String, UserDto> usersDtoByUuids) {
+  private void processIssues(NewIssuesStatistics newIssuesStats, CloseableIterator<DefaultIssue> issues, Component project, Map<String, UserDto> 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<String, UserDto> usersDtoByUuids) {
+  private void sendIssueChangeNotification(DefaultIssue issue, Component project, Map<String, UserDto> 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<String, UserDto> 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 d908494ac40c422231453c4abc7c696d2968507d..0c4c2db21e293107edead5a335dbc460d0cbc2a5 100644 (file)
--- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/SendIssueNotificationsStepTest.java
+++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/SendIssueNotificationsStepTest.java
@@ -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<NewIssuesStatistics.Stats> 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<String, MyNewIssuesNotification> 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 754380cbbae733898fc250646f84a7f92eaec7c1..56a5794dd20a0a31a27d328d6af17b912b0a9c8b 100644 (file)
--- 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<String, NotificationChannel> 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<String, NotificationChannel> recipients) {
+  private static int dispatch(Notification notification, SetMultimap<String, NotificationChannel> recipients) {
+    int count = 0;
     for (Map.Entry<String, Collection<NotificationChannel>> entry : recipients.asMap().entrySet()) {
       String username = entry.getKey();
       Collection<NotificationChannel> 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 671a507ac83da68323250400b84f6e43f9cb53df..87235eecceb3121a9c3784e601f64326a719496e 100644 (file)
--- 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 bdccede51b4c25476fcf0379cc41ce8261fae010..4e515770ca42d75f936c49882d7e91243dcf475a 100644 (file)
--- 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 6d057aefb5131cbecccb68bae37ebbe5b84e774d..3889e922340f7f4d2eee1d831032b70021d69352 100644 (file)
--- 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<WiserMessage> messages = smtpServer.getMessages();
     assertThat(messages).hasSize(1);
@@ -127,6 +128,7 @@ public class EmailNotificationChannelTest {
     assertThat(email.getHeader("To", null)).isEqualTo("<user@nowhere>");
     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<WiserMessage> messages = smtpServer.getMessages();
     assertThat(messages).hasSize(1);
@@ -155,6 +157,7 @@ public class EmailNotificationChannelTest {
     assertThat(email.getHeader("To", null)).isEqualTo("<user@nowhere>");
     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 6d2b8365c32c827bb50a1e7cd12431408db3fd76..7eb043b9d20b21d880247460345c45ad9e0c40f0 100644 (file)
--- 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 f909bc781e775575d8ca0e183756529e7d3eca37..c3d067432be14cc6d7375a496aa678706a272e8f 100644 (file)
--- 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 f94dce62d3d0682b09cb8a7366faa1dad3eb2e6f..a80aa2d27ef7ef003c5eaa180918565206489b7e 100644 (file)
--- 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 c1705f7f42640719ce239bbc9b4b7507b4779e45..fc2e5d179937b751ecc1af1eda9dd2432e2a8618 100644 (file)
--- 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() {