From 71030c3e284c9ae264cd5e3b697c2e29060e9731 Mon Sep 17 00:00:00 2001 From: =?utf8?q?S=C3=A9bastien=20Lesaint?= Date: Wed, 20 Sep 2017 19:00:09 +0200 Subject: [PATCH] SONAR-7024 global report failure notif only on administrated projects --- ...AnalysisFailureNotificationDispatcher.java | 5 +- ...ysisFailureNotificationDispatcherTest.java | 5 +- .../ce/ReportFailureNotificationTest.java | 49 +++++++++++++------ 3 files changed, 40 insertions(+), 19 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/ce/notification/ReportAnalysisFailureNotificationDispatcher.java b/server/sonar-server/src/main/java/org/sonar/ce/notification/ReportAnalysisFailureNotificationDispatcher.java index 2481d9937ae..ff3ad8346c2 100644 --- a/server/sonar-server/src/main/java/org/sonar/ce/notification/ReportAnalysisFailureNotificationDispatcher.java +++ b/server/sonar-server/src/main/java/org/sonar/ce/notification/ReportAnalysisFailureNotificationDispatcher.java @@ -24,13 +24,16 @@ import java.util.Collection; import java.util.Map; import org.sonar.api.notifications.Notification; import org.sonar.api.notifications.NotificationChannel; +import org.sonar.api.web.UserRole; import org.sonar.server.notification.NotificationDispatcher; import org.sonar.server.notification.NotificationDispatcherMetadata; import org.sonar.server.notification.NotificationManager; +import org.sonar.server.notification.NotificationManager.SubscriberPermissionsOnProject; public class ReportAnalysisFailureNotificationDispatcher extends NotificationDispatcher { public static final String KEY = "CeReportTaskFailure"; + private static final SubscriberPermissionsOnProject REQUIRED_SUBSCRIBER_PERMISSIONS = new SubscriberPermissionsOnProject(UserRole.ADMIN, UserRole.USER); private final NotificationManager manager; @@ -54,7 +57,7 @@ public class ReportAnalysisFailureNotificationDispatcher extends NotificationDis public void dispatch(Notification notification, Context context) { String projectUuid = notification.getFieldValue("project.uuid"); Multimap subscribedRecipients = manager.findSubscribedRecipientsForDispatcher( - this, projectUuid, NotificationManager.SubscriberPermissionsOnProject.ALL_MUST_HAVE_ROLE_USER); + this, projectUuid, REQUIRED_SUBSCRIBER_PERMISSIONS); for (Map.Entry> channelsByRecipients : subscribedRecipients.asMap().entrySet()) { String userLogin = channelsByRecipients.getKey(); diff --git a/server/sonar-server/src/test/java/org/sonar/ce/notification/ReportAnalysisFailureNotificationDispatcherTest.java b/server/sonar-server/src/test/java/org/sonar/ce/notification/ReportAnalysisFailureNotificationDispatcherTest.java index 92c233a8fc8..82cad44378c 100644 --- a/server/sonar-server/src/test/java/org/sonar/ce/notification/ReportAnalysisFailureNotificationDispatcherTest.java +++ b/server/sonar-server/src/test/java/org/sonar/ce/notification/ReportAnalysisFailureNotificationDispatcherTest.java @@ -27,6 +27,7 @@ import org.sonar.api.web.UserRole; import org.sonar.server.notification.NotificationDispatcher; import org.sonar.server.notification.NotificationDispatcherMetadata; import org.sonar.server.notification.NotificationManager; +import org.sonar.server.notification.NotificationManager.SubscriberPermissionsOnProject; import static org.apache.commons.lang.RandomStringUtils.randomAlphanumeric; import static org.assertj.core.api.Assertions.assertThat; @@ -89,7 +90,7 @@ public class ReportAnalysisFailureNotificationDispatcherTest { multimap.put(login1, channel2); multimap.put(login2, channel2); multimap.put(login2, channel3); - when(notificationManager.findSubscribedRecipientsForDispatcher(underTest, projectUuid, new NotificationManager.SubscriberPermissionsOnProject(UserRole.USER))) + when(notificationManager.findSubscribedRecipientsForDispatcher(underTest, projectUuid, new SubscriberPermissionsOnProject(UserRole.ADMIN, UserRole.USER))) .thenReturn(multimap); underTest.performDispatch(notificationMock, contextMock); @@ -107,7 +108,7 @@ public class ReportAnalysisFailureNotificationDispatcherTest { String projectUuid = randomAlphanumeric(9); when(notificationMock.getFieldValue("project.uuid")).thenReturn(projectUuid); HashMultimap multimap = HashMultimap.create(); - when(notificationManager.findSubscribedRecipientsForDispatcher(underTest, projectUuid, new NotificationManager.SubscriberPermissionsOnProject(UserRole.USER))) + when(notificationManager.findSubscribedRecipientsForDispatcher(underTest, projectUuid, new SubscriberPermissionsOnProject(UserRole.ADMIN, UserRole.USER))) .thenReturn(multimap); underTest.performDispatch(notificationMock, contextMock); diff --git a/tests/src/test/java/org/sonarqube/tests/ce/ReportFailureNotificationTest.java b/tests/src/test/java/org/sonarqube/tests/ce/ReportFailureNotificationTest.java index 877b1334b0a..46727f2c375 100644 --- a/tests/src/test/java/org/sonarqube/tests/ce/ReportFailureNotificationTest.java +++ b/tests/src/test/java/org/sonarqube/tests/ce/ReportFailureNotificationTest.java @@ -43,10 +43,12 @@ import org.junit.Rule; import org.junit.Test; import org.sonarqube.tests.Category6Suite; import org.sonarqube.tests.Tester; +import org.sonarqube.ws.Organizations; import org.sonarqube.ws.WsProjects; import org.sonarqube.ws.WsUsers; import org.sonarqube.ws.client.PostRequest; import org.sonarqube.ws.client.WsClient; +import org.sonarqube.ws.client.permission.AddUserWsRequest; import org.subethamail.wiser.Wiser; import org.subethamail.wiser.WiserMessage; @@ -85,7 +87,10 @@ public class ReportFailureNotificationTest { "email.smtp_port.secured", valueOf(smtpServer.getServer().getPort())); smtpServer.getMessages().clear(); + } + @After + public void tearDown() throws Exception { tester.elasticsearch().unlockWrites("components"); } @@ -96,12 +101,24 @@ public class ReportFailureNotificationTest { @Test public void send_notification_on_report_processing_failures_to_global_and_project_subscribers() throws Exception { - WsUsers.CreateWsResponse.User user1 = tester.users().generate(t -> t.setPassword("user1").setEmail("user1@bar.com")); - WsUsers.CreateWsResponse.User user2 = tester.users().generate(t -> t.setPassword("user2").setEmail("user2@bar.com")); - WsUsers.CreateWsResponse.User user3 = tester.users().generate(t -> t.setPassword("user3").setEmail("user3@bar.com")); - WsProjects.CreateWsResponse.Project project1 = tester.projects().generate(null, t -> t.setName("Project1")); - WsProjects.CreateWsResponse.Project project2 = tester.projects().generate(null, t -> t.setName("Project2")); - WsProjects.CreateWsResponse.Project project3 = tester.projects().generate(null, t -> t.setName("Project3")); + Organizations.Organization organization = tester.organizations().getDefaultOrganization(); + WsUsers.CreateWsResponse.User user1 = tester.users().generateMember(organization, t -> t.setPassword("user1").setEmail("user1@bar.com")); + WsUsers.CreateWsResponse.User user2 = tester.users().generateMember(organization, t -> t.setPassword("user2").setEmail("user2@bar.com")); + WsUsers.CreateWsResponse.User user3 = tester.users().generateMember(organization, t -> t.setPassword("user3").setEmail("user3@bar.com")); + WsProjects.CreateWsResponse.Project project1 = tester.projects().generate(organization, t -> t.setName("Project1")); + WsProjects.CreateWsResponse.Project project2 = tester.projects().generate(organization, t -> t.setName("Project2")); + WsProjects.CreateWsResponse.Project project3 = tester.projects().generate(organization, t -> t.setName("Project3")); + // user 1 is admin of project 1 and will subscribe to global notifications + tester.wsClient().permissions().addUser(new AddUserWsRequest() + .setLogin(user1.getLogin()) + .setPermission("admin") + .setProjectKey(project1.getKey())); + // user 2 is admin of project 2 but won't subscribe to global notifications + tester.wsClient().permissions().addUser(new AddUserWsRequest() + .setLogin(user2.getLogin()) + .setPermission("admin") + .setProjectKey(project2.getKey())); + // user 3 is no admin of any project and will subscribe to global notifications // analyses successful and no-one subscribed => no email executeAnalysis(project1); @@ -116,29 +133,28 @@ public class ReportFailureNotificationTest { executeAnalysis(project3); assertThat(waitForEmails()).isEmpty(); - // Add notifications to users: user1 globally, user2 for project1, user3 for project2 + // Add notifications to users: user1 and user3 globally, user2 for project1, user3 for project2 subscribeToReportFailures(user1, "user1", null); subscribeToReportFailures(user2, "user2", project1); subscribeToReportFailures(user3, "user3", project2); + subscribeToReportFailures(user3, "user3", null); - // analysis failed and 1 global subscriber + 1 project subscriber => 2 emails + // analysis failed and 1 global subscriber with admin permission + 1 project subscriber => 2 emails executeAnalysis(project1); List messages = waitForEmails(); assertThat(messages.stream().flatMap(toToRecipients()).collect(Collectors.toSet())) .containsOnly("user1@bar.com", "user2@bar.com"); assertSubjectAndContent(project1, messages); - // analysis failed and 1 global subscriber + 1 project subscriber => 2 emails + // analysis failed and no global subscriber with admin permission + 1 project subscriber => 1 emails executeAnalysis(project2); messages = waitForEmails(); assertThat(messages.stream().flatMap(toToRecipients()).collect(Collectors.toSet())) - .containsOnly("user1@bar.com", "user3@bar.com"); + .containsOnly("user3@bar.com"); assertSubjectAndContent(project2, messages); - // analysis fails and 1 global subscriber => 1 email + // analysis fails and no global subscriber with admin permission => 0 email executeAnalysis(project3); messages = waitForEmails(); - assertThat(messages.stream().flatMap(toToRecipients()).collect(Collectors.toSet())) - .containsOnly("user1@bar.com"); - assertSubjectAndContent(project3, messages); + assertThat(messages).isEmpty(); // global and project subscribed but analysis is successful => no email tester.elasticsearch().unlockWrites("components"); @@ -147,12 +163,13 @@ public class ReportFailureNotificationTest { executeAnalysis(project3); assertThat(waitForEmails()).isEmpty(); - // Add notifications to users: user1 globally, user2 for project1, user3 for project2 + // remove notifications from users: user1 globally, user2 for project1, user3 for project2 + // => no email anyway unsubscribeFromReportFailures(user1, "user1", null); unsubscribeFromReportFailures(user2, "user2", project1); unsubscribeFromReportFailures(user3, "user3", project2); - // analyses fail and no-one subscribed => no email + // analyses fail and no subscriber but user3 globally but is not admin of any project => no email tester.elasticsearch().lockWrites("components"); executeAnalysis(project1); executeAnalysis(project2); -- 2.39.5