From 803439a16e9b40da4ad9a5baa9cd84a407c607b5 Mon Sep 17 00:00:00 2001 From: Eric Hartmann Date: Wed, 13 Sep 2017 17:23:33 +0200 Subject: [PATCH] SONAR-4824 Prevent notification when user does not have Browse permission --- .../sonar/db/permission/AuthorizationDao.java | 7 ++ .../db/permission/AuthorizationMapper.java | 4 + .../org/sonar/db/property/PropertiesDao.java | 12 +- .../sonar/db/property/PropertiesMapper.java | 5 +- .../db/permission/AuthorizationMapper.xml | 35 ++++++ .../sonar/db/property/PropertiesMapper.xml | 61 ++++------ .../db/permission/AuthorizationDaoTest.java | 65 ++++++++++ .../sonar/db/property/PropertiesDaoTest.java | 111 +++++++----------- .../step/SendIssueNotificationsStep.java | 2 +- .../org/sonar/server/issue/IssueUpdater.java | 2 +- ...hangesOnMyIssueNotificationDispatcher.java | 4 +- .../DoNotFixNotificationDispatcher.java | 4 +- .../notification/IssueChangeNotification.java | 5 +- .../MyNewIssuesNotificationDispatcher.java | 4 +- .../NewIssuesNotificationDispatcher.java | 4 +- .../DefaultNotificationManager.java | 66 ++++------- .../notification/NotificationManager.java | 3 - .../sonar/server/issue/IssueUpdaterTest.java | 1 + ...esOnMyIssueNotificationDispatcherTest.java | 16 ++- .../DoNotFixNotificationDispatcherTest.java | 8 +- .../IssueChangeNotificationTest.java | 6 +- .../IssueChangesEmailTemplateTest.java | 4 +- ...MyNewIssuesNotificationDispatcherTest.java | 3 +- .../NewIssuesNotificationDispatcherTest.java | 6 +- .../server/issue/ws/BulkChangeActionTest.java | 4 +- .../DefaultNotificationManagerTest.java | 108 ++++++++--------- .../sonar/api/config/ConfigurationTest.java | 6 +- 27 files changed, 302 insertions(+), 254 deletions(-) diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/permission/AuthorizationDao.java b/server/sonar-db-dao/src/main/java/org/sonar/db/permission/AuthorizationDao.java index d92a139a1b5..56ff394b3eb 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/permission/AuthorizationDao.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/permission/AuthorizationDao.java @@ -177,6 +177,13 @@ public class AuthorizationDao implements Dao { return mapper(dbSession).selectLoginsWithGlobalPermission(ADMINISTER.getKey()); } + public Set keepAuthorizedLoginsOnProject(DbSession dbSession, Set logins, String projectUuid, String permission) { + return executeLargeInputsIntoSet( + logins, + partitionOfLogins -> mapper(dbSession).keepAuthorizedLoginsOnProject(partitionOfLogins, projectUuid, permission), + partitionSize -> partitionSize / 3); + } + private static AuthorizationMapper mapper(DbSession dbSession) { return dbSession.getMapper(AuthorizationMapper.class); } diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/permission/AuthorizationMapper.java b/server/sonar-db-dao/src/main/java/org/sonar/db/permission/AuthorizationMapper.java index da8ecbb5356..1c788ea1019 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/permission/AuthorizationMapper.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/permission/AuthorizationMapper.java @@ -61,5 +61,9 @@ public interface AuthorizationMapper { Set selectProjectPermissionsOfAnonymous(@Param("projectUuid") String projectUuid); + List selectQualityProfileAdministratorLogins(@Param("permission") String permission); + + Set keepAuthorizedLoginsOnProject(@Param("logins") List logins, @Param("projectUuid") String projectUuid, @Param("permission") String permission); + List selectLoginsWithGlobalPermission(@Param("permission") String permission); } diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/property/PropertiesDao.java b/server/sonar-db-dao/src/main/java/org/sonar/db/property/PropertiesDao.java index f12f1027cfb..4ee31513608 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/property/PropertiesDao.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/property/PropertiesDao.java @@ -32,6 +32,7 @@ import javax.annotation.CheckForNull; import javax.annotation.Nullable; import org.sonar.api.resources.Scopes; import org.sonar.api.utils.System2; +import org.sonar.api.web.UserRole; import org.sonar.db.Dao; import org.sonar.db.DbSession; import org.sonar.db.MyBatis; @@ -60,20 +61,17 @@ public class PropertiesDao implements Dao { * Returns the logins of users who have subscribed to the given notification dispatcher with the given notification channel. * If a resource ID is passed, the search is made on users who have specifically subscribed for the given resource. * + * Note that {@link UserRole#USER} permission is not checked here, filter the results with + * {@link org.sonar.db.permission.AuthorizationDao#keepAuthorizedLoginsOnProject} + * * @return the list of logins (maybe be empty - obviously) */ - public List selectUsersForNotification(String notificationDispatcherKey, String notificationChannelKey, @Nullable String projectUuid) { + public Set findUsersForNotification(String notificationDispatcherKey, String notificationChannelKey, @Nullable String projectUuid) { try (DbSession session = mybatis.openSession(false)) { return getMapper(session).findUsersForNotification(NOTIFICATION_PREFIX + notificationDispatcherKey + "." + notificationChannelKey, projectUuid); } } - public List selectNotificationSubscribers(String notificationDispatcherKey, String notificationChannelKey, @Nullable String componentKey) { - try (DbSession session = mybatis.openSession(false)) { - return getMapper(session).findNotificationSubscribers(NOTIFICATION_PREFIX + notificationDispatcherKey + "." + notificationChannelKey, componentKey); - } - } - public boolean hasProjectNotificationSubscribersForDispatchers(String projectUuid, Collection dispatcherKeys) { try (DbSession session = mybatis.openSession(false); Connection connection = session.getConnection(); diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/property/PropertiesMapper.java b/server/sonar-db-dao/src/main/java/org/sonar/db/property/PropertiesMapper.java index ac422e3cd1d..b15395951f9 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/property/PropertiesMapper.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/property/PropertiesMapper.java @@ -20,14 +20,13 @@ package org.sonar.db.property; import java.util.List; +import java.util.Set; import javax.annotation.Nullable; import org.apache.ibatis.annotations.Param; public interface PropertiesMapper { - List findUsersForNotification(@Param("notifKey") String notificationKey, @Nullable @Param("projectUuid") String projectUuid); - - List findNotificationSubscribers(@Param("propKey") String propertyKey, @Nullable @Param("componentKey") String componentKey); + Set findUsersForNotification(@Param("notifKey") String notificationKey, @Nullable @Param("projectUuid") String projectUuid); List selectGlobalProperties(); diff --git a/server/sonar-db-dao/src/main/resources/org/sonar/db/permission/AuthorizationMapper.xml b/server/sonar-db-dao/src/main/resources/org/sonar/db/permission/AuthorizationMapper.xml index 072fefe4e3d..b43ede52e69 100644 --- a/server/sonar-db-dao/src/main/resources/org/sonar/db/permission/AuthorizationMapper.xml +++ b/server/sonar-db-dao/src/main/resources/org/sonar/db/permission/AuthorizationMapper.xml @@ -396,4 +396,39 @@ gr.role = #{permission,jdbcType=VARCHAR} and gr.group_id is not null + + diff --git a/server/sonar-db-dao/src/main/resources/org/sonar/db/property/PropertiesMapper.xml b/server/sonar-db-dao/src/main/resources/org/sonar/db/property/PropertiesMapper.xml index a9a520eb1ee..43c4ee3a8d7 100644 --- a/server/sonar-db-dao/src/main/resources/org/sonar/db/property/PropertiesMapper.xml +++ b/server/sonar-db-dao/src/main/resources/org/sonar/db/property/PropertiesMapper.xml @@ -3,51 +3,30 @@ - + SELECT u.login - from + FROM users u - inner join properties p on - p.user_id=u.id - - - where - p.prop_key = #{notifKey,jdbcType=VARCHAR} - and p.text_value = 'true' - and p.resource_id is null - - - inner join projects c on - c.id=p.resource_id - where - p.prop_key = #{notifKey,jdbcType=VARCHAR} - and p.text_value = 'true' - and c.uuid = #{projectUuid,jdbcType=VARCHAR} - and p.resource_id is not null - - - + INNER JOIN properties p ON p.user_id = u.id + WHERE + p.prop_key = #{notifKey,jdbcType=VARCHAR} + AND p.text_value = 'true' + AND p.resource_id IS NULL - diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/permission/AuthorizationDaoTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/permission/AuthorizationDaoTest.java index 36595bc5d72..693266f6722 100644 --- a/server/sonar-db-dao/src/test/java/org/sonar/db/permission/AuthorizationDaoTest.java +++ b/server/sonar-db-dao/src/test/java/org/sonar/db/permission/AuthorizationDaoTest.java @@ -1028,4 +1028,69 @@ public class AuthorizationDaoTest { assertThat(logins).containsExactlyInAnyOrder(user1.getLogin(), user2.getLogin(), user3.getLogin()); } + @Test + public void keepAuthorizedLoginsOnProject_return_correct_users_on_public_project() { + ComponentDto project = db.components().insertPublicProject(organization); + + UserDto user1 = db.users().insertUser(); + + // admin with "direct" ADMIN role + UserDto admin1 = db.users().insertUser(); + db.users().insertProjectPermissionOnUser(admin1, UserRole.ADMIN, project); + + // admin2 with ADMIN role through group + UserDto admin2 = db.users().insertUser(); + GroupDto adminGroup = db.users().insertGroup(organization, "ADMIN"); + db.users().insertMember(adminGroup, admin2); + db.users().insertProjectPermissionOnGroup(adminGroup, UserRole.ADMIN, project); + + assertThat(underTest.keepAuthorizedLoginsOnProject(dbSession, newHashSet(user1.getLogin()), project.uuid(), UserRole.USER)) + .containsOnly(user1.getLogin()); + assertThat(underTest.keepAuthorizedLoginsOnProject(dbSession, newHashSet(user1.getLogin(), admin1.getLogin(), admin2.getLogin()), project.uuid(), UserRole.USER)) + .containsOnly(user1.getLogin(), admin1.getLogin(), admin2.getLogin()); + assertThat(underTest.keepAuthorizedLoginsOnProject(dbSession, newHashSet(user1.getLogin(), admin1.getLogin(), admin2.getLogin()), project.uuid(), UserRole.ADMIN)) + .containsOnly(admin1.getLogin(), admin2.getLogin()); + } + + @Test + public void keepAuthorizedLoginsOnProject_return_correct_users_on_private_project() { + ComponentDto project = db.components().insertPrivateProject(organization); + + GroupDto userGroup = db.users().insertGroup(organization, "USERS"); + GroupDto adminGroup = db.users().insertGroup(organization, "ADMIN"); + db.users().insertProjectPermissionOnGroup(userGroup, UserRole.USER, project); + db.users().insertProjectPermissionOnGroup(adminGroup, UserRole.ADMIN, project); + + // admin with "direct" ADMIN role + UserDto admin1 = db.users().insertUser(); + db.users().insertProjectPermissionOnUser(admin1, UserRole.ADMIN, project); + + // admin2 with ADMIN role through group + UserDto admin2 = db.users().insertUser(); + db.users().insertMember(adminGroup, admin2); + + // user1 with "direct" USER role + UserDto user1 = db.users().insertUser(); + db.users().insertProjectPermissionOnUser(user1, UserRole.USER, project); + + // user2 with USER role through group + UserDto user2 = db.users().insertUser(); + db.users().insertMember(userGroup, user2); + + // user without role + UserDto userWithNoRole = db.users().insertUser(); + + assertThat(underTest.keepAuthorizedLoginsOnProject(dbSession, newHashSet(userWithNoRole.getLogin()), project.uuid(), UserRole.USER)) + .isEmpty(); + assertThat(underTest.keepAuthorizedLoginsOnProject(dbSession, newHashSet(user1.getLogin()), project.uuid(), UserRole.USER)) + .containsOnly(user1.getLogin()); + + Set allLogins = newHashSet(admin1.getLogin(), admin2.getLogin(), user1.getLogin(), user2.getLogin(), userWithNoRole.getLogin()); + + // Admin does not have the USER permission set + assertThat(underTest.keepAuthorizedLoginsOnProject(dbSession, allLogins, project.uuid(), UserRole.USER)) + .containsOnly(user1.getLogin(), user2.getLogin()); + assertThat(underTest.keepAuthorizedLoginsOnProject(dbSession, allLogins, project.uuid(), UserRole.ADMIN)) + .containsOnly(admin1.getLogin(), admin2.getLogin()); + } } diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/property/PropertiesDaoTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/property/PropertiesDaoTest.java index 8b0c455d92f..84ec3a718d7 100644 --- a/server/sonar-db-dao/src/test/java/org/sonar/db/property/PropertiesDaoTest.java +++ b/server/sonar-db-dao/src/test/java/org/sonar/db/property/PropertiesDaoTest.java @@ -35,6 +35,7 @@ import org.junit.Test; import org.junit.rules.ExpectedException; import org.junit.runner.RunWith; import org.sonar.api.utils.System2; +import org.sonar.api.web.UserRole; import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.DbTester; @@ -80,79 +81,50 @@ public class PropertiesDaoTest { @Test public void shouldFindUsersForNotification() throws SQLException { - ComponentDto project1 = insertProject("uuid_45"); - ComponentDto project2 = insertProject("uuid_56"); - int userId1 = insertUser("user1"); - int userId2 = insertUser("user2"); - int userId3 = insertUser("user3"); - insertProperty("notification.NewViolations.Email", "true", project1.getId(), userId2); - insertProperty("notification.NewViolations.Twitter", "true", null, userId3); - insertProperty("notification.NewViolations.Twitter", "true", project2.getId(), userId1); - insertProperty("notification.NewViolations.Twitter", "true", project2.getId(), userId3); - - assertThat(underTest.selectUsersForNotification("NewViolations", "Email", null)) + ComponentDto project1 = insertPrivateProject("uuid_45"); + ComponentDto project2 = insertPrivateProject("uuid_56"); + UserDto user1 = insertUser("user1"); + UserDto user2 = insertUser("user2"); + UserDto user3 = insertUser("user3"); + insertProperty("notification.NewViolations.Email", "true", project1.getId(), user2.getId()); + insertProperty("notification.NewViolations.Twitter", "true", null, user3.getId()); + insertProperty("notification.NewViolations.Twitter", "true", project2.getId(), user1.getId()); + insertProperty("notification.NewViolations.Twitter", "true", project1.getId(), user2.getId()); + insertProperty("notification.NewViolations.Twitter", "true", project2.getId(), user3.getId()); + dbTester.users().insertProjectPermissionOnUser(user2, UserRole.USER, project1); + dbTester.users().insertProjectPermissionOnUser(user3, UserRole.USER, project2); + dbTester.users().insertProjectPermissionOnUser(user1, UserRole.USER, project2); + + assertThat(underTest.findUsersForNotification("NewViolations", "Email", null)) .isEmpty(); - assertThat(underTest.selectUsersForNotification("NewViolations", "Email", "uuid_78")) + assertThat(underTest.findUsersForNotification("NewViolations", "Email", "uuid_78")) .isEmpty(); - assertThat(underTest.selectUsersForNotification("NewViolations", "Email", "uuid_45")) - .hasSize(1).containsOnly("user2"); - - assertThat(underTest.selectUsersForNotification("NewViolations", "Twitter", null)) - .hasSize(1) - .containsOnly("user3"); - - assertThat(underTest.selectUsersForNotification("NewViolations", "Twitter", "uuid_78")) - .isEmpty(); - - assertThat(underTest.selectUsersForNotification("NewViolations", "Twitter", "uuid_56")) - .hasSize(2) - .containsOnly("user1", "user3"); - } - - @Test - public void findNotificationSubscribers() throws SQLException { - int userId1 = insertUser("user1"); - int userId2 = insertUser("user2"); - ComponentDto projectDto = insertProject("PROJECT_A"); - long projectId = projectDto.getId(); - String projectKey = projectDto.getDbKey(); - - // global subscription - insertProperty("notification.DispatcherWithGlobalSubscribers.Email", "true", null, userId2); - // project subscription - insertProperty("notification.DispatcherWithProjectSubscribers.Email", "true", projectId, userId1); - insertProperty("notification.DispatcherWithGlobalAndProjectSubscribers.Email", "true", 56L, userId1); - insertProperty("notification.DispatcherWithGlobalAndProjectSubscribers.Email", "true", projectId, userId1); - // global subscription - insertProperty("notification.DispatcherWithGlobalAndProjectSubscribers.Email", "true", null, userId2); + assertThat(underTest.findUsersForNotification("NewViolations", "Email", project1.uuid())) + .containsOnly("user2"); - // Nobody is subscribed - assertThat(underTest.selectNotificationSubscribers("NotSexyDispatcher", "Email", projectKey)) + assertThat(underTest.findUsersForNotification("NewViolations", "Email", project2.uuid())) .isEmpty(); - // Global subscribers - assertThat(underTest.selectNotificationSubscribers("DispatcherWithGlobalSubscribers", "Email", projectKey)) - .containsOnly("user2"); + assertThat(underTest.findUsersForNotification("NewViolations", "Twitter", null)) + .containsOnly("user3"); - assertThat(underTest.selectNotificationSubscribers("DispatcherWithGlobalSubscribers", "Email", null)) - .containsOnly("user2"); + assertThat(underTest.findUsersForNotification("NewViolations", "Twitter", "uuid_78")) + .containsOnly("user3"); - // Project subscribers - assertThat(underTest.selectNotificationSubscribers("DispatcherWithProjectSubscribers", "Email", projectKey)) - .containsOnly("user1"); + assertThat(underTest.findUsersForNotification("NewViolations", "Twitter", project1.uuid())) + .containsOnly("user2", "user3"); - // Global + Project subscribers - assertThat(underTest.selectNotificationSubscribers("DispatcherWithGlobalAndProjectSubscribers", "Email", projectKey)) - .containsOnly("user1", "user2"); + assertThat(underTest.findUsersForNotification("NewViolations", "Twitter", project2.uuid())) + .containsOnly("user1", "user3"); } @Test public void hasNotificationSubscribers() throws SQLException { - int userId1 = insertUser("user1"); - int userId2 = insertUser("user2"); - Long projectId = insertProject("PROJECT_A").getId(); + int userId1 = insertUser("user1").getId(); + int userId2 = insertUser("user2").getId(); + Long projectId = insertPrivateProject("PROJECT_A").getId(); // global subscription insertProperty("notification.DispatcherWithGlobalSubscribers.Email", "true", null, userId2); // project subscription @@ -254,7 +226,7 @@ public class PropertiesDaoTest { @Test public void selectProjectProperties() throws SQLException { - ComponentDto projectDto = insertProject("A"); + ComponentDto projectDto = insertPrivateProject("A"); long projectId = projectDto.getId(); // global insertProperty("global.one", "one", null, null); @@ -279,7 +251,7 @@ public class PropertiesDaoTest { @Test @UseDataProvider("allValuesForSelect") public void selectProjectProperties_supports_all_values(String dbValue, String expected) throws SQLException { - ComponentDto projectDto = insertProject("A"); + ComponentDto projectDto = insertPrivateProject("A"); insertProperty("project.one", dbValue, projectDto.getId(), null); List dtos = underTest.selectProjectProperties(projectDto.getDbKey()); @@ -379,8 +351,8 @@ public class PropertiesDaoTest { @Test public void select_global_properties_by_keys() throws Exception { - insertProject("A"); - int userId = insertUser("B"); + insertPrivateProject("A"); + int userId = insertUser("B").getId(); String key = "key"; String anotherKey = "anotherKey"; @@ -705,9 +677,9 @@ public class PropertiesDaoTest { @Test public void delete_project_property() throws SQLException { - long projectId1 = insertProject("A").getId(); - long projectId2 = insertProject("B").getId(); - long projectId3 = insertProject("C").getId(); + long projectId1 = insertPrivateProject("A").getId(); + long projectId2 = insertPrivateProject("B").getId(); + long projectId3 = insertPrivateProject("C").getId(); long id1 = insertProperty("global.one", "one", null, null); long id2 = insertProperty("global.two", "two", null, null); long id3 = insertProperty("struts.one", "one", projectId1, null); @@ -822,7 +794,6 @@ public class PropertiesDaoTest { .hasNoResourceId() .hasUserId(100) .hasTextValue("new_user"); - } @Test @@ -1068,7 +1039,7 @@ public class PropertiesDaoTest { " and resource_id" + (resourceId == null ? " is null" : "='" + resourceId + "'")).get("id"); } - private ComponentDto insertProject(String uuid) { + private ComponentDto insertPrivateProject(String uuid) { String key = "project" + uuid; ComponentDto project = ComponentTesting.newPrivateProjectDto(dbTester.getDefaultOrganization(), uuid).setDbKey(key); dbClient.componentDao().insert(session, project); @@ -1076,12 +1047,12 @@ public class PropertiesDaoTest { return project; } - private int insertUser(String login) { + private UserDto insertUser(String login) { UserDto dto = new UserDto().setLogin(login); DbSession session = dbTester.getSession(); dbClient.userDao().insert(session, dto); session.commit(); - return dto.getId(); + return dto; } private static PropertyDtoAssert assertThatDto(@Nullable PropertyDto dto) { 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 930a75192be..6e26dcbad07 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 @@ -114,7 +114,7 @@ public class SendIssueNotificationsStep implements ComputationStep { IssueChangeNotification changeNotification = new IssueChangeNotification(); changeNotification.setRuleName(rules.getByKey(issue.ruleKey()).getName()); changeNotification.setIssue(issue); - changeNotification.setProject(project.getPublicKey(), project.getName(), getBranchName()); + changeNotification.setProject(project.getPublicKey(), project.getName(), getBranchName(), project.getUuid()); getComponentKey(issue).ifPresent(c -> changeNotification.setComponent(c.getPublicKey(), c.getName())); service.deliver(changeNotification); } diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/IssueUpdater.java b/server/sonar-server/src/main/java/org/sonar/server/issue/IssueUpdater.java index a368b01ca7a..4115aaac9b8 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/IssueUpdater.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/IssueUpdater.java @@ -80,7 +80,7 @@ public class IssueUpdater { .setIssue(issue) .setChangeAuthorLogin(context.login()) .setRuleName(rule.map(RuleDefinitionDto::getName).orElse(null)) - .setProject(project.getKey(), project.name(), project.getBranch()) + .setProject(project) .setComponent(component) .setComment(comment)); return issueDto; diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/notification/ChangesOnMyIssueNotificationDispatcher.java b/server/sonar-server/src/main/java/org/sonar/server/issue/notification/ChangesOnMyIssueNotificationDispatcher.java index 86bd287e230..d9115a37619 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/notification/ChangesOnMyIssueNotificationDispatcher.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/notification/ChangesOnMyIssueNotificationDispatcher.java @@ -57,8 +57,8 @@ public class ChangesOnMyIssueNotificationDispatcher extends NotificationDispatch @Override public void dispatch(Notification notification, Context context) { - String projectKey = notification.getFieldValue("projectKey"); - Multimap subscribedRecipients = notificationManager.findNotificationSubscribers(this, projectKey); + String projectUuid = notification.getFieldValue("projectUuid"); + Multimap subscribedRecipients = notificationManager.findSubscribedRecipientsForDispatcher(this, projectUuid); // See available fields in the class IssueNotifications. diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/notification/DoNotFixNotificationDispatcher.java b/server/sonar-server/src/main/java/org/sonar/server/issue/notification/DoNotFixNotificationDispatcher.java index 4e2dec4edc2..87e1d62599a 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/notification/DoNotFixNotificationDispatcher.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/notification/DoNotFixNotificationDispatcher.java @@ -60,8 +60,8 @@ public class DoNotFixNotificationDispatcher extends NotificationDispatcher { String newResolution = notification.getFieldValue("new.resolution"); if (Objects.equals(newResolution, Issue.RESOLUTION_FALSE_POSITIVE) || Objects.equals(newResolution, Issue.RESOLUTION_WONT_FIX)) { String author = notification.getFieldValue("changeAuthor"); - String projectKey = notification.getFieldValue("projectKey"); - Multimap subscribedRecipients = notifications.findNotificationSubscribers(this, projectKey); + String projectUuid = notification.getFieldValue("projectUuid"); + Multimap subscribedRecipients = notifications.findSubscribedRecipientsForDispatcher(this, projectUuid); notify(author, context, subscribedRecipients); } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/notification/IssueChangeNotification.java b/server/sonar-server/src/main/java/org/sonar/server/issue/notification/IssueChangeNotification.java index 477bde3049c..58d6b7296c3 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/notification/IssueChangeNotification.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/notification/IssueChangeNotification.java @@ -54,11 +54,12 @@ public class IssueChangeNotification extends Notification { } public IssueChangeNotification setProject(ComponentDto project) { - return setProject(project.getKey(), project.longName(), project.getBranch()); + return setProject(project.getKey(), project.name(), project.getBranch(), project.uuid()); } - public IssueChangeNotification setProject(String projectKey, String projectName, @Nullable String branch) { + public IssueChangeNotification setProject(String projectKey, String projectName, @Nullable String branch, String projectUuid) { setFieldValue("projectName", projectName); + setFieldValue("projectUuid", projectUuid); setFieldValue("projectKey", projectKey); if (branch != null) { setFieldValue("branch", branch); diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/notification/MyNewIssuesNotificationDispatcher.java b/server/sonar-server/src/main/java/org/sonar/server/issue/notification/MyNewIssuesNotificationDispatcher.java index a7d6357a13e..ee980d09b51 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/notification/MyNewIssuesNotificationDispatcher.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/notification/MyNewIssuesNotificationDispatcher.java @@ -53,9 +53,9 @@ public class MyNewIssuesNotificationDispatcher extends NotificationDispatcher { @Override public void dispatch(Notification notification, Context context) { - String projectKey = notification.getFieldValue("projectKey"); + String projectUuid = notification.getFieldValue("projectUuid"); String assignee = notification.getFieldValue("assignee"); - Multimap subscribedRecipients = manager.findNotificationSubscribers(this, projectKey); + Multimap subscribedRecipients = manager.findSubscribedRecipientsForDispatcher(this, projectUuid); Collection channels = subscribedRecipients.get(assignee); for (NotificationChannel channel : channels) { diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/notification/NewIssuesNotificationDispatcher.java b/server/sonar-server/src/main/java/org/sonar/server/issue/notification/NewIssuesNotificationDispatcher.java index 58e4a3a9fb9..8b7787ea98a 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/notification/NewIssuesNotificationDispatcher.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/notification/NewIssuesNotificationDispatcher.java @@ -54,8 +54,8 @@ public class NewIssuesNotificationDispatcher extends NotificationDispatcher { @Override public void dispatch(Notification notification, Context context) { - String projectKey = notification.getFieldValue("projectKey"); - Multimap subscribedRecipients = manager.findNotificationSubscribers(this, projectKey); + String projectUuid = notification.getFieldValue("projectUuid"); + Multimap subscribedRecipients = manager.findSubscribedRecipientsForDispatcher(this, projectUuid); for (Map.Entry> channelsByRecipients : subscribedRecipients.asMap().entrySet()) { String userLogin = channelsByRecipients.getKey(); diff --git a/server/sonar-server/src/main/java/org/sonar/server/notification/DefaultNotificationManager.java b/server/sonar-server/src/main/java/org/sonar/server/notification/DefaultNotificationManager.java index fa819e856c8..cb000d0cf9e 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/notification/DefaultNotificationManager.java +++ b/server/sonar-server/src/main/java/org/sonar/server/notification/DefaultNotificationManager.java @@ -21,24 +21,26 @@ package org.sonar.server.notification; import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.HashMultimap; -import com.google.common.collect.Lists; import com.google.common.collect.Multimap; import com.google.common.collect.SetMultimap; import java.io.IOException; import java.io.InvalidClassException; import java.util.Arrays; +import java.util.Collection; import java.util.List; -import javax.annotation.Nullable; +import java.util.Set; import org.sonar.api.notifications.Notification; import org.sonar.api.notifications.NotificationChannel; import org.sonar.api.utils.SonarException; import org.sonar.api.utils.log.Logger; import org.sonar.api.utils.log.Loggers; -import org.sonar.db.notification.NotificationQueueDao; +import org.sonar.api.web.UserRole; +import org.sonar.db.DbClient; +import org.sonar.db.DbSession; import org.sonar.db.notification.NotificationQueueDto; -import org.sonar.db.property.PropertiesDao; import static java.util.Collections.singletonList; +import static java.util.Objects.requireNonNull; public class DefaultNotificationManager implements NotificationManager { @@ -47,25 +49,17 @@ public class DefaultNotificationManager implements NotificationManager { private static final String UNABLE_TO_READ_NOTIFICATION = "Unable to read notification"; private NotificationChannel[] notificationChannels; - private NotificationQueueDao notificationQueueDao; - private PropertiesDao propertiesDao; + private final DbClient dbClient; private boolean alreadyLoggedDeserializationIssue = false; /** * Default constructor used by Pico */ - public DefaultNotificationManager(NotificationChannel[] channels, NotificationQueueDao notificationQueueDao, PropertiesDao propertiesDao) { + public DefaultNotificationManager(NotificationChannel[] channels, + DbClient dbClient) { this.notificationChannels = channels; - this.notificationQueueDao = notificationQueueDao; - this.propertiesDao = propertiesDao; - } - - /** - * Constructor if no notification channel - */ - public DefaultNotificationManager(NotificationQueueDao notificationQueueDao, PropertiesDao propertiesDao) { - this(new NotificationChannel[0], notificationQueueDao, propertiesDao); + this.dbClient = dbClient; } /** @@ -74,18 +68,18 @@ public class DefaultNotificationManager implements NotificationManager { @Override public void scheduleForSending(Notification notification) { NotificationQueueDto dto = NotificationQueueDto.toNotificationQueueDto(notification); - notificationQueueDao.insert(singletonList(dto)); + dbClient.notificationQueueDao().insert(singletonList(dto)); } /** * Give the notification queue so that it can be processed */ public Notification getFromQueue() { int batchSize = 1; - List notificationDtos = notificationQueueDao.selectOldest(batchSize); + List notificationDtos = dbClient.notificationQueueDao().selectOldest(batchSize); if (notificationDtos.isEmpty()) { return null; } - notificationQueueDao.delete(notificationDtos); + dbClient.notificationQueueDao().delete(notificationDtos); return convertToNotification(notificationDtos); } @@ -112,15 +106,15 @@ public class DefaultNotificationManager implements NotificationManager { } public long count() { - return notificationQueueDao.count(); + return dbClient.notificationQueueDao().count(); } /** * {@inheritDoc} */ @Override - public Multimap findSubscribedRecipientsForDispatcher(NotificationDispatcher dispatcher, - @Nullable String projectUuid) { + public Multimap findSubscribedRecipientsForDispatcher(NotificationDispatcher dispatcher, String projectUuid) { + requireNonNull(projectUuid, "ProjectUUID is mandatory"); String dispatcherKey = dispatcher.getKey(); SetMultimap recipients = HashMultimap.create(); @@ -128,38 +122,28 @@ public class DefaultNotificationManager implements NotificationManager { String channelKey = channel.getKey(); // Find users subscribed globally to the dispatcher (i.e. not on a specific project) - addUsersToRecipientListForChannel(propertiesDao.selectUsersForNotification(dispatcherKey, channelKey, null), recipients, channel); - - if (projectUuid != null) { - // Find users subscribed to the dispatcher specifically for the project - addUsersToRecipientListForChannel(propertiesDao.selectUsersForNotification(dispatcherKey, channelKey, projectUuid), recipients, channel); + // And users subscribed to the dispatcher specifically for the project + Set subscribedUsers = dbClient.propertiesDao().findUsersForNotification(dispatcherKey, channelKey, projectUuid); + + if (!subscribedUsers.isEmpty()) { + try (DbSession dbSession = dbClient.openSession(false)) { + Set filteredSubscribedUsers = dbClient.authorizationDao().keepAuthorizedLoginsOnProject(dbSession, subscribedUsers, projectUuid, UserRole.USER); + addUsersToRecipientListForChannel(filteredSubscribedUsers, recipients, channel); + } } } return recipients; } - @Override - public Multimap findNotificationSubscribers(NotificationDispatcher dispatcher, @Nullable String componentKey) { - String dispatcherKey = dispatcher.getKey(); - - SetMultimap recipients = HashMultimap.create(); - for (NotificationChannel channel : notificationChannels) { - addUsersToRecipientListForChannel(propertiesDao.selectNotificationSubscribers(dispatcherKey, channel.getKey(), componentKey), recipients, channel); - } - - return recipients; - } - @VisibleForTesting protected List getChannels() { return Arrays.asList(notificationChannels); } - private static void addUsersToRecipientListForChannel(List users, SetMultimap recipients, NotificationChannel channel) { + private static void addUsersToRecipientListForChannel(Collection users, SetMultimap recipients, NotificationChannel channel) { for (String username : users) { recipients.put(username, channel); } } - } diff --git a/server/sonar-server/src/main/java/org/sonar/server/notification/NotificationManager.java b/server/sonar-server/src/main/java/org/sonar/server/notification/NotificationManager.java index 181c6bce56a..e06c4083d64 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/notification/NotificationManager.java +++ b/server/sonar-server/src/main/java/org/sonar/server/notification/NotificationManager.java @@ -20,7 +20,6 @@ package org.sonar.server.notification; import com.google.common.collect.Multimap; -import javax.annotation.Nullable; import org.sonar.api.notifications.Notification; import org.sonar.api.notifications.NotificationChannel; @@ -54,6 +53,4 @@ public interface NotificationManager { * @return the list of user login along with the subscribed channels */ Multimap findSubscribedRecipientsForDispatcher(NotificationDispatcher dispatcher, String projectUuid); - - Multimap findNotificationSubscribers(NotificationDispatcher dispatcher, @Nullable String componentKey); } diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/IssueUpdaterTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/IssueUpdaterTest.java index 8e33f079c96..bf2b55409d6 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/IssueUpdaterTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/IssueUpdaterTest.java @@ -117,6 +117,7 @@ public class IssueUpdaterTest { assertThat(issueChangeNotification.getFieldValue("componentKey")).isEqualTo(file.getDbKey()); assertThat(issueChangeNotification.getFieldValue("componentName")).isEqualTo(file.longName()); assertThat(issueChangeNotification.getFieldValue("projectKey")).isEqualTo(project.getDbKey()); + assertThat(issueChangeNotification.getFieldValue("projectUuid")).isEqualTo(project.uuid()); assertThat(issueChangeNotification.getFieldValue("projectName")).isEqualTo(project.name()); assertThat(issueChangeNotification.getFieldValue("ruleName")).isEqualTo(rule.getName()); assertThat(issueChangeNotification.getFieldValue("changeAuthor")).isEqualTo("john"); diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/notification/ChangesOnMyIssueNotificationDispatcherTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/notification/ChangesOnMyIssueNotificationDispatcherTest.java index 5bcb7421a66..fc2b0fc13a2 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/notification/ChangesOnMyIssueNotificationDispatcherTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/notification/ChangesOnMyIssueNotificationDispatcherTest.java @@ -83,9 +83,11 @@ public class ChangesOnMyIssueNotificationDispatcherTest { recipients.put("simon", emailChannel); recipients.put("freddy", twitterChannel); recipients.put("godin", twitterChannel); - when(notifications.findNotificationSubscribers(dispatcher, "struts")).thenReturn(recipients); + when(notifications.findSubscribedRecipientsForDispatcher(dispatcher, "uuid1")).thenReturn(recipients); - Notification notification = new IssueChangeNotification().setFieldValue("projectKey", "struts") + Notification notification = new IssueChangeNotification() + .setFieldValue("projectKey", "struts") + .setFieldValue("projectUuid", "uuid1") .setFieldValue("changeAuthor", "olivier") .setFieldValue("assignee", "freddy"); dispatcher.performDispatch(notification, context); @@ -101,11 +103,15 @@ public class ChangesOnMyIssueNotificationDispatcherTest { recipients.put("simon", emailChannel); recipients.put("freddy", twitterChannel); recipients.put("godin", twitterChannel); - when(notifications.findNotificationSubscribers(dispatcher, "struts")).thenReturn(recipients); + when(notifications.findSubscribedRecipientsForDispatcher(dispatcher, "uuid1")).thenReturn(recipients); // change author is the assignee - dispatcher.performDispatch(new IssueChangeNotification().setFieldValue("projectKey", "struts") - .setFieldValue("changeAuthor", "simon").setFieldValue("assignee", "simon"), context); + dispatcher.performDispatch( + new IssueChangeNotification() + .setFieldValue("projectKey", "struts") + .setFieldValue("projectUuid", "uuid1") + .setFieldValue("changeAuthor", "simon") + .setFieldValue("assignee", "simon"), context); // no change author dispatcher.performDispatch(new IssueChangeNotification().setFieldValue("projectKey", "struts") diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/notification/DoNotFixNotificationDispatcherTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/notification/DoNotFixNotificationDispatcherTest.java index 9c323065340..aaaf004ffb2 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/notification/DoNotFixNotificationDispatcherTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/notification/DoNotFixNotificationDispatcherTest.java @@ -61,9 +61,10 @@ public class DoNotFixNotificationDispatcherTest { recipients.put("simon", emailChannel); recipients.put("freddy", twitterChannel); recipients.put("godin", twitterChannel); - when(notifications.findNotificationSubscribers(underTest, "struts")).thenReturn(recipients); + when(notifications.findSubscribedRecipientsForDispatcher(underTest, "uuid1")).thenReturn(recipients); Notification fpNotif = new IssueChangeNotification().setFieldValue("projectKey", "struts") + .setFieldValue("projectUuid", "uuid1") .setFieldValue("changeAuthor", "godin") .setFieldValue("new.resolution", Issue.RESOLUTION_FALSE_POSITIVE) .setFieldValue("assignee", "freddy"); @@ -81,11 +82,6 @@ public class DoNotFixNotificationDispatcherTest { */ @Test public void ignore_other_resolutions() { - Multimap recipients = HashMultimap.create(); - recipients.put("simon", emailChannel); - recipients.put("freddy", twitterChannel); - when(notifications.findNotificationSubscribers(underTest, "struts")).thenReturn(recipients); - Notification fixedNotif = new IssueChangeNotification().setFieldValue("projectKey", "struts") .setFieldValue("changeAuthor", "godin") .setFieldValue("new.resolution", Issue.RESOLUTION_FIXED) diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/notification/IssueChangeNotificationTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/notification/IssueChangeNotificationTest.java index 18a513ca9ff..94ff9af63e8 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/notification/IssueChangeNotificationTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/notification/IssueChangeNotificationTest.java @@ -84,16 +84,18 @@ public class IssueChangeNotificationTest { @Test public void set_project_without_branch() { - IssueChangeNotification result = notification.setProject("MyService", "My Service", null); + IssueChangeNotification result = notification.setProject("MyService", "My Service", null, "uuid1"); assertThat(result.getFieldValue("projectKey")).isEqualTo("MyService"); + assertThat(result.getFieldValue("projectUuid")).isEqualTo("uuid1"); assertThat(result.getFieldValue("projectName")).isEqualTo("My Service"); assertThat(result.getFieldValue("branch")).isNull(); } @Test public void set_project_with_branch() { - IssueChangeNotification result = notification.setProject("MyService", "My Service", "feature1"); + IssueChangeNotification result = notification.setProject("MyService", "My Service", "feature1", "uuid2"); assertThat(result.getFieldValue("projectKey")).isEqualTo("MyService"); + assertThat(result.getFieldValue("projectUuid")).isEqualTo("uuid2"); assertThat(result.getFieldValue("projectName")).isEqualTo("My Service"); assertThat(result.getFieldValue("branch")).isEqualTo("feature1"); } diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/notification/IssueChangesEmailTemplateTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/notification/IssueChangesEmailTemplateTest.java index 3e858e61915..90ac0f3d1a4 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/notification/IssueChangesEmailTemplateTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/notification/IssueChangesEmailTemplateTest.java @@ -170,7 +170,7 @@ public class IssueChangesEmailTemplateTest { Notification notification = new IssueChangeNotification() .setChangeAuthorLogin("simon") - .setProject("Struts", "org.apache:struts", null); + .setProject("Struts", "org.apache:struts", null, ""); EmailMessage message = underTest.format(notification); assertThat(message.getFrom()).isEqualTo("Simon"); @@ -182,7 +182,7 @@ public class IssueChangesEmailTemplateTest { Notification notification = new IssueChangeNotification() .setChangeAuthorLogin("simon") - .setProject("Struts", "org.apache:struts", null); + .setProject("Struts", "org.apache:struts", null, ""); EmailMessage message = underTest.format(notification); assertThat(message.getFrom()).isEqualTo("simon"); diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/notification/MyNewIssuesNotificationDispatcherTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/notification/MyNewIssuesNotificationDispatcherTest.java index edb16207832..1ef6e596dbe 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/notification/MyNewIssuesNotificationDispatcherTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/notification/MyNewIssuesNotificationDispatcherTest.java @@ -58,10 +58,11 @@ public class MyNewIssuesNotificationDispatcherTest { Multimap recipients = HashMultimap.create(); recipients.put("user1", emailChannel); recipients.put("user2", twitterChannel); - when(notificationManager.findNotificationSubscribers(underTest, "struts")).thenReturn(recipients); + when(notificationManager.findSubscribedRecipientsForDispatcher(underTest, "uuid1")).thenReturn(recipients); Notification notification = new Notification(MyNewIssuesNotification.MY_NEW_ISSUES_NOTIF_TYPE) .setFieldValue("projectKey", "struts") + .setFieldValue("projectUuid", "uuid1") .setFieldValue("assignee", "user1"); underTest.performDispatch(notification, context); diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/notification/NewIssuesNotificationDispatcherTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/notification/NewIssuesNotificationDispatcherTest.java index f98b8536baa..54905e244f0 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/notification/NewIssuesNotificationDispatcherTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/notification/NewIssuesNotificationDispatcherTest.java @@ -56,9 +56,11 @@ public class NewIssuesNotificationDispatcherTest { Multimap recipients = HashMultimap.create(); recipients.put("user1", emailChannel); recipients.put("user2", twitterChannel); - when(notifications.findNotificationSubscribers(dispatcher, "struts")).thenReturn(recipients); + when(notifications.findSubscribedRecipientsForDispatcher(dispatcher, "uuid1")).thenReturn(recipients); - Notification notification = new Notification(NewIssuesNotification.TYPE).setFieldValue("projectKey", "struts"); + Notification notification = new Notification(NewIssuesNotification.TYPE) + .setFieldValue("projectKey", "struts") + .setFieldValue("projectUuid", "uuid1"); dispatcher.performDispatch(notification, context); verify(context).addUser("user1", emailChannel); diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/ws/BulkChangeActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/ws/BulkChangeActionTest.java index 5225acb63db..df7bf696472 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/ws/BulkChangeActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/ws/BulkChangeActionTest.java @@ -254,7 +254,7 @@ public class BulkChangeActionTest { verify(notificationManager).scheduleForSending(issueChangeNotificationCaptor.capture()); assertThat(issueChangeNotificationCaptor.getValue().getFieldValue("key")).isEqualTo(issueDto.getKey()); assertThat(issueChangeNotificationCaptor.getValue().getFieldValue("componentName")).isEqualTo(file.longName()); - assertThat(issueChangeNotificationCaptor.getValue().getFieldValue("projectName")).isEqualTo(project.longName()); + assertThat(issueChangeNotificationCaptor.getValue().getFieldValue("projectName")).isEqualTo(project.name()); assertThat(issueChangeNotificationCaptor.getValue().getFieldValue("projectKey")).isEqualTo(project.getDbKey()); assertThat(issueChangeNotificationCaptor.getValue().getFieldValue("ruleName")).isEqualTo(rule.getName()); assertThat(issueChangeNotificationCaptor.getValue().getFieldValue("changeAuthor")).isEqualTo(user.getLogin()); @@ -282,7 +282,7 @@ public class BulkChangeActionTest { verify(notificationManager).scheduleForSending(issueChangeNotificationCaptor.capture()); assertThat(issueChangeNotificationCaptor.getValue().getFieldValue("key")).isEqualTo(issueDto.getKey()); assertThat(issueChangeNotificationCaptor.getValue().getFieldValue("componentName")).isEqualTo(fileOnBranch.longName()); - assertThat(issueChangeNotificationCaptor.getValue().getFieldValue("projectName")).isEqualTo(project.longName()); + assertThat(issueChangeNotificationCaptor.getValue().getFieldValue("projectName")).isEqualTo(project.name()); assertThat(issueChangeNotificationCaptor.getValue().getFieldValue("projectKey")).isEqualTo(project.getDbKey()); assertThat(issueChangeNotificationCaptor.getValue().getFieldValue("ruleName")).isEqualTo(rule.getName()); assertThat(issueChangeNotificationCaptor.getValue().getFieldValue("changeAuthor")).isEqualTo(user.getLogin()); diff --git a/server/sonar-server/src/test/java/org/sonar/server/notification/DefaultNotificationManagerTest.java b/server/sonar-server/src/test/java/org/sonar/server/notification/DefaultNotificationManagerTest.java index 327db72773a..da159a1d3e8 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/notification/DefaultNotificationManagerTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/notification/DefaultNotificationManagerTest.java @@ -19,7 +19,6 @@ */ package org.sonar.server.notification; -import com.google.common.collect.Lists; import com.google.common.collect.Multimap; import java.io.InvalidClassException; import java.util.Arrays; @@ -27,29 +26,41 @@ import java.util.Collection; import java.util.List; import java.util.Map; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; +import org.mockito.ArgumentCaptor; +import org.mockito.Captor; import org.mockito.InOrder; import org.mockito.Mock; import org.mockito.MockitoAnnotations; import org.sonar.api.notifications.Notification; import org.sonar.api.notifications.NotificationChannel; +import org.sonar.api.utils.System2; +import org.sonar.db.DbClient; +import org.sonar.db.DbTester; import org.sonar.db.notification.NotificationQueueDao; import org.sonar.db.notification.NotificationQueueDto; +import org.sonar.db.permission.AuthorizationDao; import org.sonar.db.property.PropertiesDao; +import static com.google.common.collect.Sets.newHashSet; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Matchers.any; +import static org.mockito.Matchers.eq; import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.only; import static org.mockito.Mockito.spy; -import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static org.mockito.internal.verification.VerificationModeFactory.times; public class DefaultNotificationManagerTest { - private DefaultNotificationManager manager; + private DefaultNotificationManager underTest; + + @Rule + public DbTester db = DbTester.create(System2.INSTANCE); @Mock private PropertiesDao propertiesDao; @@ -66,28 +77,40 @@ public class DefaultNotificationManagerTest { @Mock private NotificationQueueDao notificationQueueDao; + @Mock + private AuthorizationDao authorizationDao; + + @Mock + private DbClient dbClient; + + @Captor + private ArgumentCaptor> captorLogins; + @Before public void setUp() { MockitoAnnotations.initMocks(this); when(dispatcher.getKey()).thenReturn("NewViolations"); when(emailChannel.getKey()).thenReturn("Email"); when(twitterChannel.getKey()).thenReturn("Twitter"); + when(dbClient.propertiesDao()).thenReturn(propertiesDao); + when(dbClient.notificationQueueDao()).thenReturn(notificationQueueDao); + when(dbClient.authorizationDao()).thenReturn(authorizationDao); - manager = new DefaultNotificationManager(new NotificationChannel[] {emailChannel, twitterChannel}, notificationQueueDao, propertiesDao); + underTest = new DefaultNotificationManager(new NotificationChannel[] {emailChannel, twitterChannel}, dbClient); } @Test public void shouldProvideChannelList() { - assertThat(manager.getChannels()).containsOnly(emailChannel, twitterChannel); + assertThat(underTest.getChannels()).containsOnly(emailChannel, twitterChannel); - manager = new DefaultNotificationManager(notificationQueueDao, propertiesDao); - assertThat(manager.getChannels()).hasSize(0); + underTest = new DefaultNotificationManager(new NotificationChannel[] {} , db.getDbClient()); + assertThat(underTest.getChannels()).hasSize(0); } @Test public void shouldPersist() { Notification notification = new Notification("test"); - manager.scheduleForSending(notification); + underTest.scheduleForSending(notification); verify(notificationQueueDao, only()).insert(any(List.class)); } @@ -99,13 +122,14 @@ public class DefaultNotificationManagerTest { List dtos = Arrays.asList(dto); when(notificationQueueDao.selectOldest(1)).thenReturn(dtos); - assertThat(manager.getFromQueue()).isNotNull(); + assertThat(underTest.getFromQueue()).isNotNull(); InOrder inOrder = inOrder(notificationQueueDao); inOrder.verify(notificationQueueDao).selectOldest(1); inOrder.verify(notificationQueueDao).delete(dtos); } + // SONAR-4739 @Test public void shouldNotFailWhenUnableToDeserialize() throws Exception { @@ -114,65 +138,41 @@ public class DefaultNotificationManagerTest { List dtos = Arrays.asList(dto1); when(notificationQueueDao.selectOldest(1)).thenReturn(dtos); - manager = spy(manager); - assertThat(manager.getFromQueue()).isNull(); - assertThat(manager.getFromQueue()).isNull(); + underTest = spy(underTest); + assertThat(underTest.getFromQueue()).isNull(); + assertThat(underTest.getFromQueue()).isNull(); - verify(manager, times(1)).logDeserializationIssue(); + verify(underTest, times(1)).logDeserializationIssue(); } @Test public void shouldFindNoRecipient() { - assertThat(manager.findSubscribedRecipientsForDispatcher(dispatcher, "uuid_45").asMap().entrySet()).hasSize(0); + assertThat(underTest.findSubscribedRecipientsForDispatcher(dispatcher, "uuid_45").asMap().entrySet()).hasSize(0); } @Test public void shouldFindSubscribedRecipientForGivenResource() { - when(propertiesDao.selectUsersForNotification("NewViolations", "Email", "uuid_45")).thenReturn(Lists.newArrayList("user1", "user2")); - when(propertiesDao.selectUsersForNotification("NewViolations", "Email", null)).thenReturn(Lists.newArrayList("user1", "user3")); - when(propertiesDao.selectUsersForNotification("NewViolations", "Twitter", "uuid_56")).thenReturn(Lists.newArrayList("user2")); - when(propertiesDao.selectUsersForNotification("NewViolations", "Twitter", null)).thenReturn(Lists.newArrayList("user3")); - when(propertiesDao.selectUsersForNotification("NewAlerts", "Twitter", null)).thenReturn(Lists.newArrayList("user4")); - - Multimap multiMap = manager.findSubscribedRecipientsForDispatcher(dispatcher, "uuid_45"); - assertThat(multiMap.entries()).hasSize(4); - - Map> map = multiMap.asMap(); - assertThat(map.get("user1")).containsOnly(emailChannel); - assertThat(map.get("user2")).containsOnly(emailChannel); - assertThat(map.get("user3")).containsOnly(emailChannel, twitterChannel); - assertThat(map.get("user4")).isNull(); - } - - @Test - public void shouldFindSubscribedRecipientForNoResource() { - when(propertiesDao.selectUsersForNotification("NewViolations", "Email", "uuid_45")).thenReturn(Lists.newArrayList("user1", "user2")); - when(propertiesDao.selectUsersForNotification("NewViolations", "Email", null)).thenReturn(Lists.newArrayList("user1", "user3")); - when(propertiesDao.selectUsersForNotification("NewViolations", "Twitter", "uuid_56")).thenReturn(Lists.newArrayList("user2")); - when(propertiesDao.selectUsersForNotification("NewViolations", "Twitter", null)).thenReturn(Lists.newArrayList("user3")); - when(propertiesDao.selectUsersForNotification("NewAlerts", "Twitter", null)).thenReturn(Lists.newArrayList("user4")); - - Multimap multiMap = manager.findSubscribedRecipientsForDispatcher(dispatcher, null); + when(propertiesDao.findUsersForNotification("NewViolations", "Email", "uuid_45")) + .thenReturn(newHashSet("user1", "user3")); + when(propertiesDao.findUsersForNotification("NewViolations", "Twitter", "uuid_56")) + .thenReturn(newHashSet("user2")); + when(propertiesDao.findUsersForNotification("NewViolations", "Twitter", "uuid_45")) + .thenReturn(newHashSet("user3")); + when(propertiesDao.findUsersForNotification("NewAlerts", "Twitter", "uuid_45")) + .thenReturn(newHashSet("user4")); + + when(authorizationDao.keepAuthorizedLoginsOnProject(any(), eq(newHashSet("user1", "user3")), eq("uuid_45"), eq("user"))) + .thenReturn(newHashSet("user1", "user3")); + when(authorizationDao.keepAuthorizedLoginsOnProject(any(), eq(newHashSet("user3")), eq("uuid_45"), eq("user"))) + .thenReturn(newHashSet("user3")); + + Multimap multiMap = underTest.findSubscribedRecipientsForDispatcher(dispatcher, "uuid_45"); assertThat(multiMap.entries()).hasSize(3); Map> map = multiMap.asMap(); assertThat(map.get("user1")).containsOnly(emailChannel); - assertThat(map.get("user3")).containsOnly(emailChannel, twitterChannel); assertThat(map.get("user2")).isNull(); + assertThat(map.get("user3")).containsOnly(emailChannel, twitterChannel); assertThat(map.get("user4")).isNull(); } - - @Test - public void findNotificationSubscribers() { - when(propertiesDao.selectNotificationSubscribers("NewViolations", "Email", "struts")).thenReturn(Lists.newArrayList("user1", "user2")); - when(propertiesDao.selectNotificationSubscribers("NewViolations", "Twitter", "struts")).thenReturn(Lists.newArrayList("user2")); - - Multimap multiMap = manager.findNotificationSubscribers(dispatcher, "struts"); - assertThat(multiMap.entries()).hasSize(3); - - Map> map = multiMap.asMap(); - assertThat(map.get("user1")).containsOnly(emailChannel); - assertThat(map.get("user2")).containsOnly(emailChannel, twitterChannel); - assertThat(map.get("other")).isNull(); - } } diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/config/ConfigurationTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/config/ConfigurationTest.java index 6950f8dd5fc..53bc9ea4778 100644 --- a/sonar-plugin-api/src/test/java/org/sonar/api/config/ConfigurationTest.java +++ b/sonar-plugin-api/src/test/java/org/sonar/api/config/ConfigurationTest.java @@ -73,9 +73,9 @@ public class ConfigurationTest { String randomKey = RandomStringUtils.randomAlphabetic(3); String randomNumberOfWhitespaces = StringUtils.repeat(" ", 1 + new Random().nextInt(10)); - assertThat(t.apply(underTest.put(randomKey, randomNumberOfWhitespaces + String.valueOf(value)), randomKey)).contains(value); - assertThat(t.apply(underTest.put(randomKey, String.valueOf(value) + randomNumberOfWhitespaces), randomKey)).contains(value); - assertThat(t.apply(underTest.put(randomKey, randomNumberOfWhitespaces + String.valueOf(value) + randomNumberOfWhitespaces), randomKey)).contains(value); + assertThat(t.apply(underTest.put(randomKey, randomNumberOfWhitespaces + String.valueOf(value)), randomKey)).isEqualTo(Optional.of(value)); + assertThat(t.apply(underTest.put(randomKey, String.valueOf(value) + randomNumberOfWhitespaces), randomKey)).isEqualTo(Optional.of(value)); + assertThat(t.apply(underTest.put(randomKey, randomNumberOfWhitespaces + String.valueOf(value) + randomNumberOfWhitespaces), randomKey)).isEqualTo(Optional.of(value)); } private static class DumpMapConfiguration implements Configuration { -- 2.39.5