SONAR-5794 Send notification step should not use batch repo to get project name

diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/step/SendIssueNotificationsStep.java b/server/sonar-server/src/main/java/org/sonar/server/computation/step/SendIssueNotificationsStep.java
index 84b0a1d117f3219a14175f78779e7c67ab94ab0e..e84ae899a00cf2dadb03f3ae5f7ab6f0464137a6 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/computation/step/SendIssueNotificationsStep.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/computation/step/SendIssueNotificationsStep.java
@@ -24,6 +24,7 @@ import java.util.Date;
 import java.util.Map;
 import java.util.Set;
 import org.sonar.core.issue.DefaultIssue;
+import org.sonar.core.util.CloseableIterator;
 import org.sonar.server.computation.batch.BatchReportReader;
 import org.sonar.server.computation.component.Component;
 import org.sonar.server.computation.component.TreeRootHolder;
@@ -35,7 +36,6 @@ 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.core.util.CloseableIterator;
 
 /**
  * Reads issues from disk cache and send related notifications. For performance reasons,
@@ -77,54 +77,63 @@ public class SendIssueNotificationsStep implements ComputationStep {
   private void doExecute(Component project) {
     NewIssuesStatistics newIssuesStats = new NewIssuesStatistics();
     CloseableIterator<DefaultIssue> issues = issueCache.traverse();
-    String projectName = reportReader.readComponent(reportReader.readMetadata().getRootComponentRef()).getName();
     try {
-      while (issues.hasNext()) {
-        DefaultIssue issue = issues.next();
-        if (issue.isNew() && issue.resolution() == null) {
-          newIssuesStats.add(issue);
-        } else if (issue.isChanged() && issue.mustSendNotifications()) {
-          IssueChangeNotification changeNotification = new IssueChangeNotification();
-          changeNotification.setRuleName(rules.getByKey(issue.ruleKey()).getName());
-          changeNotification.setIssue(issue);
-          changeNotification.setProject(project.getKey(), projectName);
-          service.deliver(changeNotification);
-        }
-      }
-
+      processIssues(newIssuesStats, issues, project);
     } finally {
       issues.close();
     }
-    sendNewIssuesStatistics(newIssuesStats, project, projectName);
+    if (newIssuesStats.hasIssues()) {
+      long analysisDate = reportReader.readMetadata().getAnalysisDate();
+      sendNewIssuesNotification(newIssuesStats, project, analysisDate);
+      sendNewIssuesNotificationToAssignees(newIssuesStats, project, analysisDate);
+    }
   }
 
-  private void sendNewIssuesStatistics(NewIssuesStatistics statistics, Component project, String projectName) {
-    if (statistics.hasIssues()) {
-      NewIssuesStatistics.Stats globalStatistics = statistics.globalStatistics();
-      long analysisDate = reportReader.readMetadata().getAnalysisDate();
-      NewIssuesNotification notification = newIssuesNotificationFactory
-        .newNewIssuesNotication()
-        .setProject(project.getKey(), project.getUuid(), projectName)
-        .setAnalysisDate(new Date(analysisDate))
-        .setStatistics(projectName, globalStatistics)
-        .setDebt(globalStatistics.debt());
-      service.deliver(notification);
+  private void processIssues(NewIssuesStatistics newIssuesStats, CloseableIterator<DefaultIssue> issues, Component project) {
+    while (issues.hasNext()) {
+      DefaultIssue issue = issues.next();
+      if (issue.isNew() && issue.resolution() == null) {
+        newIssuesStats.add(issue);
+      } else if (issue.isChanged() && issue.mustSendNotifications()) {
+        sendIssueChangeNotification(issue, project);
+      }
+    }
+  }
 
-      // send email to each user having issues
-      for (Map.Entry<String, NewIssuesStatistics.Stats> assigneeAndStatisticsTuple : statistics.assigneesStatistics().entrySet()) {
-        String assignee = assigneeAndStatisticsTuple.getKey();
-        NewIssuesStatistics.Stats assigneeStatistics = assigneeAndStatisticsTuple.getValue();
-        MyNewIssuesNotification myNewIssuesNotification = newIssuesNotificationFactory
-          .newMyNewIssuesNotification()
-          .setAssignee(assignee);
-        myNewIssuesNotification
-          .setProject(project.getKey(), project.getUuid(), projectName)
-          .setAnalysisDate(new Date(analysisDate))
-          .setStatistics(projectName, assigneeStatistics)
-          .setDebt(assigneeStatistics.debt());
+  private void sendIssueChangeNotification(DefaultIssue issue, Component project) {
+    IssueChangeNotification changeNotification = new IssueChangeNotification();
+    changeNotification.setRuleName(rules.getByKey(issue.ruleKey()).getName());
+    changeNotification.setIssue(issue);
+    changeNotification.setProject(project.getKey(), project.getName());
+    service.deliver(changeNotification);
+  }
 
-        service.deliver(myNewIssuesNotification);
-      }
+  private void sendNewIssuesNotification(NewIssuesStatistics statistics, Component project, long analysisDate) {
+    NewIssuesStatistics.Stats globalStatistics = statistics.globalStatistics();
+    NewIssuesNotification notification = newIssuesNotificationFactory
+      .newNewIssuesNotication()
+      .setProject(project.getKey(), project.getUuid(), project.getName())
+      .setAnalysisDate(new Date(analysisDate))
+      .setStatistics(project.getName(), globalStatistics)
+      .setDebt(globalStatistics.debt());
+    service.deliver(notification);
+  }
+
+  private void sendNewIssuesNotificationToAssignees(NewIssuesStatistics statistics, Component project, long analysisDate) {
+    // send email to each user having issues
+    for (Map.Entry<String, NewIssuesStatistics.Stats> assigneeAndStatisticsTuple : statistics.assigneesStatistics().entrySet()) {
+      String assignee = assigneeAndStatisticsTuple.getKey();
+      NewIssuesStatistics.Stats assigneeStatistics = assigneeAndStatisticsTuple.getValue();
+      MyNewIssuesNotification myNewIssuesNotification = newIssuesNotificationFactory
+        .newMyNewIssuesNotification()
+        .setAssignee(assignee);
+      myNewIssuesNotification
+        .setProject(project.getKey(), project.getUuid(), project.getName())
+        .setAnalysisDate(new Date(analysisDate))
+        .setStatistics(project.getName(), assigneeStatistics)
+        .setDebt(assigneeStatistics.debt());
+
+      service.deliver(myNewIssuesNotification);
     }
   }
 

diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/step/SendIssueNotificationsStepTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/step/SendIssueNotificationsStepTest.java
index f0d088d383cdb60404ead413e9962222f2ae51a6..cd8512295df9ce259e53be3fe05e95e9af0a069b 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/computation/step/SendIssueNotificationsStepTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/computation/step/SendIssueNotificationsStepTest.java
@@ -19,92 +19,158 @@
  */
 package org.sonar.server.computation.step;
 
+import java.util.Date;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
-import org.mockito.Mockito;
 import org.sonar.api.notifications.Notification;
 import org.sonar.api.rule.Severity;
+import org.sonar.api.utils.Duration;
 import org.sonar.api.utils.System2;
-import org.sonar.batch.protocol.Constants;
 import org.sonar.batch.protocol.output.BatchReport;
 import org.sonar.core.issue.DefaultIssue;
 import org.sonar.server.computation.batch.BatchReportReaderRule;
 import org.sonar.server.computation.batch.TreeRootHolderRule;
 import org.sonar.server.computation.component.Component;
-import org.sonar.server.computation.component.ReportComponent;
+import org.sonar.server.computation.component.Component.Type;
 import org.sonar.server.computation.issue.IssueCache;
 import org.sonar.server.computation.issue.RuleRepository;
 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 static org.mockito.Matchers.anyString;
+import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.any;
-import static org.mockito.Mockito.atLeastOnce;
 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.server.computation.component.ReportComponent.builder;
 
 public class SendIssueNotificationsStepTest extends BaseStepTest {
 
-  private static final String PROJECT_UUID = "PROJECT_UUID";
-  private static final String PROJECT_KEY = "PROJECT_KEY";
+  static final String PROJECT_UUID = "PROJECT_UUID";
+  static final String PROJECT_KEY = "PROJECT_KEY";
+  static final String PROJECT_NAME = "PROJECT_NAME";
+
+  static final long ANALYSE_DATE = 123L;
+
+  static final Duration ISSUE_DURATION = Duration.create(100L);
+  static final String ISSUE_ASSIGNEE = "John";
+
+  static final Component PROJECT = builder(Type.PROJECT, 1).setUuid(PROJECT_UUID).setKey(PROJECT_KEY).setName(PROJECT_NAME).build();
 
   @Rule
   public BatchReportReaderRule reportReader = new BatchReportReaderRule();
 
   @Rule
-  public TreeRootHolderRule treeRootHolder = new TreeRootHolderRule();
+  public TreeRootHolderRule treeRootHolder = new TreeRootHolderRule()
+    .setRoot(PROJECT);
 
   @Rule
   public TemporaryFolder temp = new TemporaryFolder();
 
-  NotificationService notifService = mock(NotificationService.class);
+  NotificationService notificationService = mock(NotificationService.class);
+  NewIssuesNotificationFactory newIssuesNotificationFactory = mock(NewIssuesNotificationFactory.class);
+  NewIssuesNotification newIssuesNotificationMock = createNewIssuesNotificationMock();
+  MyNewIssuesNotification myNewIssuesNotificationMock = createMyNewIssuesNotificationMock();
+
   IssueCache issueCache;
   SendIssueNotificationsStep underTest;
 
   @Before
   public void setUp() throws Exception {
     issueCache = new IssueCache(temp.newFile(), System2.INSTANCE);
-    NewIssuesNotificationFactory newIssuesNotificationFactory = mock(NewIssuesNotificationFactory.class, Mockito.RETURNS_DEEP_STUBS);
-    underTest = new SendIssueNotificationsStep(issueCache, mock(RuleRepository.class), treeRootHolder, notifService, reportReader, newIssuesNotificationFactory);
+    underTest = new SendIssueNotificationsStep(issueCache, mock(RuleRepository.class), treeRootHolder, notificationService, reportReader, newIssuesNotificationFactory);
 
-    treeRootHolder.setRoot(ReportComponent.builder(Component.Type.PROJECT, 1).setUuid(PROJECT_UUID).setKey(PROJECT_KEY).build());
+    when(newIssuesNotificationFactory.newNewIssuesNotication()).thenReturn(newIssuesNotificationMock);
+    when(newIssuesNotificationFactory.newMyNewIssuesNotification()).thenReturn(myNewIssuesNotificationMock);
 
     reportReader.setMetadata(BatchReport.Metadata.newBuilder()
       .setRootComponentRef(1)
-      .build());
-    reportReader.putComponent(BatchReport.Component.newBuilder()
-      .setRef(1)
-      .setType(Constants.ComponentType.PROJECT)
-      .setKey(PROJECT_KEY)
-      .setName("Project name")
+      .setAnalysisDate(ANALYSE_DATE)
       .build());
   }
 
   @Test
   public void do_not_send_notifications_if_no_subscribers() {
-    when(notifService.hasProjectSubscribersForTypes(PROJECT_UUID, SendIssueNotificationsStep.NOTIF_TYPES)).thenReturn(false);
+    when(notificationService.hasProjectSubscribersForTypes(PROJECT_UUID, SendIssueNotificationsStep.NOTIF_TYPES)).thenReturn(false);
+
+    underTest.execute();
+
+    verify(notificationService, never()).deliver(any(Notification.class));
+  }
+
+  @Test
+  public void send_global_new_issues_notification() throws Exception {
+    issueCache.newAppender().append(
+      new DefaultIssue().setSeverity(Severity.BLOCKER).setDebt(ISSUE_DURATION)
+      ).close();
+
+    when(notificationService.hasProjectSubscribersForTypes(PROJECT_UUID, SendIssueNotificationsStep.NOTIF_TYPES)).thenReturn(true);
 
     underTest.execute();
 
-    verify(notifService, never()).deliver(any(Notification.class));
+    verify(notificationService).deliver(any(NewIssuesNotification.class));
+    verify(newIssuesNotificationMock).setProject(PROJECT_KEY, PROJECT_UUID, PROJECT_NAME);
+    verify(newIssuesNotificationMock).setAnalysisDate(new Date(ANALYSE_DATE));
+    verify(newIssuesNotificationMock).setStatistics(eq(PROJECT_NAME), any(NewIssuesStatistics.Stats.class));
+    verify(newIssuesNotificationMock).setDebt(ISSUE_DURATION);
   }
 
   @Test
-  public void send_notifications_if_subscribers() {
-    issueCache.newAppender().append(new DefaultIssue()
-      .setSeverity(Severity.BLOCKER)).close();
+  public void send_new_issues_notification_to_user() throws Exception {
+    issueCache.newAppender().append(
+      new DefaultIssue().setSeverity(Severity.BLOCKER).setDebt(ISSUE_DURATION).setAssignee(ISSUE_ASSIGNEE)
+      ).close();
 
-    when(notifService.hasProjectSubscribersForTypes(PROJECT_UUID, SendIssueNotificationsStep.NOTIF_TYPES)).thenReturn(true);
+    when(notificationService.hasProjectSubscribersForTypes(PROJECT_UUID, SendIssueNotificationsStep.NOTIF_TYPES)).thenReturn(true);
 
     underTest.execute();
 
-    verify(notifService).deliver(any(NewIssuesNotification.class));
-    verify(notifService, atLeastOnce()).deliver(any(IssueChangeNotification.class));
+    verify(notificationService, times(2)).deliver(any(Notification.class));
+    verify(myNewIssuesNotificationMock).setAssignee(ISSUE_ASSIGNEE);
+    verify(myNewIssuesNotificationMock).setProject(PROJECT_KEY, PROJECT_UUID, PROJECT_NAME);
+    verify(myNewIssuesNotificationMock).setAnalysisDate(new Date(ANALYSE_DATE));
+    verify(myNewIssuesNotificationMock).setStatistics(eq(PROJECT_NAME), any(NewIssuesStatistics.Stats.class));
+    verify(myNewIssuesNotificationMock).setDebt(ISSUE_DURATION);
+  }
+
+  @Test
+  public void send_issues_change_notification() throws Exception {
+    DefaultIssue issue = new DefaultIssue().setSeverity(Severity.BLOCKER).setDebt(ISSUE_DURATION).setChanged(true).setSendNotifications(true);
+    issueCache.newAppender().append(issue).close();
+
+    when(notificationService.hasProjectSubscribersForTypes(PROJECT_UUID, SendIssueNotificationsStep.NOTIF_TYPES)).thenReturn(true);
+
+    underTest.execute();
+
+    verify(notificationService).deliver(any(IssueChangeNotification.class));
+  }
+
+  private NewIssuesNotification createNewIssuesNotificationMock() {
+    NewIssuesNotification notification = mock(NewIssuesNotification.class);
+    when(notification.setProject(anyString(), anyString(), anyString())).thenReturn(notification);
+    when(notification.setAnalysisDate(any(Date.class))).thenReturn(notification);
+    when(notification.setStatistics(anyString(), any(NewIssuesStatistics.Stats.class))).thenReturn(notification);
+    when(notification.setDebt(any(Duration.class))).thenReturn(notification);
+    return notification;
+  }
+
+  private MyNewIssuesNotification createMyNewIssuesNotificationMock() {
+    MyNewIssuesNotification notification = mock(MyNewIssuesNotification.class);
+    when(notification.setAssignee(anyString())).thenReturn(notification);
+    when(notification.setProject(anyString(), anyString(), anyString())).thenReturn(notification);
+    when(notification.setAnalysisDate(any(Date.class))).thenReturn(notification);
+    when(notification.setStatistics(anyString(), any(NewIssuesStatistics.Stats.class))).thenReturn(notification);
+    when(notification.setDebt(any(Duration.class))).thenReturn(notification);
+    return notification;
   }
 
   @Override