From 8420e1087f18f08d54d94e4f1a46770ca684ff9b Mon Sep 17 00:00:00 2001 From: =?utf8?q?S=C3=A9bastien=20Lesaint?= Date: Tue, 2 Apr 2019 10:53:34 +0200 Subject: [PATCH] SONAR-11753 dont retrieve all subscribers if only interested in subset --- .../org/sonar/db/property/PropertiesDao.java | 19 ++- .../sonar/db/property/PropertiesMapper.java | 5 +- .../sonar/db/property/PropertiesMapper.xml | 6 + .../sonar/db/property/PropertiesDaoTest.java | 135 +++++++++++++++++ .../ChangesOnMyIssueNotificationHandler.java | 6 +- .../MyNewIssuesNotificationHandler.java | 6 +- .../DefaultNotificationManager.java | 35 ++++- .../notification/NotificationManager.java | 14 ++ ...angesOnMyIssueNotificationHandlerTest.java | 35 ++--- .../MyNewIssuesNotificationHandlerTest.java | 49 ++++--- .../DefaultNotificationManagerTest.java | 138 ++++++++++++++++-- 11 files changed, 386 insertions(+), 62 deletions(-) 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 465c8e4d9e2..bad536ddf3e 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 @@ -25,6 +25,7 @@ import java.sql.PreparedStatement; import java.sql.ResultSet; import java.sql.SQLException; import java.util.Collection; +import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Set; @@ -40,6 +41,7 @@ import org.sonar.db.MyBatis; import static com.google.common.base.Preconditions.checkArgument; import static org.apache.commons.lang.StringUtils.repeat; import static org.sonar.db.DatabaseUtils.executeLargeInputs; +import static org.sonar.db.DatabaseUtils.executeLargeInputsIntoSet; import static org.sonar.db.DatabaseUtils.executeLargeInputsWithoutOutput; public class PropertiesDao implements Dao { @@ -72,7 +74,22 @@ public class PropertiesDao implements Dao { public Set findEmailSubscribersForNotification(DbSession dbSession, String notificationDispatcherKey, String notificationChannelKey, @Nullable String projectKey) { - return getMapper(dbSession).findEmailRecipientsForNotification(NOTIFICATION_PREFIX + notificationDispatcherKey + "." + notificationChannelKey, projectKey); + return getMapper(dbSession).findEmailRecipientsForNotification(NOTIFICATION_PREFIX + notificationDispatcherKey + "." + notificationChannelKey, projectKey, null); + } + + public Set findEmailSubscribersForNotification(DbSession dbSession, String notificationDispatcherKey, String notificationChannelKey, + @Nullable String projectKey, Set logins) { + if (logins.isEmpty()) { + return Collections.emptySet(); + } + + return executeLargeInputsIntoSet( + logins, + loginsPartition -> { + String notificationKey = NOTIFICATION_PREFIX + notificationDispatcherKey + "." + notificationChannelKey; + return getMapper(dbSession).findEmailRecipientsForNotification(notificationKey, projectKey, loginsPartition); + }, + partitionSize -> projectKey == null ? partitionSize : (partitionSize / 2)); } public boolean hasProjectNotificationSubscribersForDispatchers(String projectUuid, Collection dispatcherKeys) { 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 5009b2e25ee..ccd2bb5f2e0 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 @@ -29,7 +29,8 @@ public interface PropertiesMapper { Set findUsersForNotification(@Param("notifKey") String notificationKey, @Nullable @Param("projectKey") String projectKey); - Set findEmailRecipientsForNotification(@Param("notifKey") String notificationKey, @Nullable @Param("projectKey") String projectKey); + Set findEmailRecipientsForNotification(@Param("notifKey") String notificationKey, @Nullable @Param("projectKey") String projectKey, + @Nullable @Param("logins") List logins); List selectGlobalProperties(); @@ -52,7 +53,7 @@ public interface PropertiesMapper { List selectIdsByOrganizationAndUser(@Param("organizationUuid") String organizationUuid, @Param("userId") int userId); List selectIdsByOrganizationAndMatchingLogin(@Param("organizationUuid") String organizationUuid, @Param("login") String login, - @Param("propertyKeys") List propertyKeys); + @Param("propertyKeys") List propertyKeys); void insertAsEmpty(@Param("key") String key, @Nullable @Param("userId") Integer userId, @Nullable @Param("componentId") Long componentId, @Param("now") long now); 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 bd1f93556d3..4360431ea57 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 @@ -46,6 +46,9 @@ and p.resource_id IS NULL WHERE u.email is not null + + and u.login in #{login,jdbcType=VARCHAR} + UNION @@ -65,6 +68,9 @@ and p.resource_id = c.id WHERE u.email is not null + + and u.login in #{login,jdbcType=VARCHAR} + 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 4c7d01ac062..b93f129625d 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 @@ -47,6 +47,7 @@ import org.sonar.db.component.ComponentTesting; import org.sonar.db.organization.OrganizationDto; import org.sonar.db.user.UserDto; +import static com.google.common.collect.ImmutableSet.of; import static com.google.common.collect.Lists.newArrayList; import static com.google.common.collect.Sets.newHashSet; import static java.util.Collections.singletonList; @@ -171,6 +172,19 @@ public class PropertiesDaoTest { assertThat(subscribers).isEmpty(); } + @Test + public void findEmailRecipientsForNotification_with_logins_returns_empty_on_empty_properties_table() { + db.users().insertUser(); + String dispatcherKey = randomAlphabetic(5); + String channelKey = randomAlphabetic(6); + String projectKey = randomAlphabetic(7); + Set logins = of("user1", "user2"); + + Set subscribers = underTest.findEmailSubscribersForNotification(db.getSession(), dispatcherKey, channelKey, projectKey, logins); + + assertThat(subscribers).isEmpty(); + } + @Test public void findEmailRecipientsForNotification_finds_only_globally_subscribed_users_if_projectKey_is_null() { int userId1 = db.users().insertUser(withEmail("user1")).getId(); @@ -203,6 +217,47 @@ public class PropertiesDaoTest { .isEmpty(); } + @Test + public void findEmailRecipientsForNotification_with_logins_finds_only_globally_subscribed_specified_users_if_projectKey_is_null() { + int userId1 = db.users().insertUser(withEmail("user1")).getId(); + int userId2 = db.users().insertUser(withEmail("user2")).getId(); + int userId3 = db.users().insertUser(withEmail("user3")).getId(); + int userId4 = db.users().insertUser(withEmail("user4")).getId(); + long projectId = insertPrivateProject("PROJECT_A").getId(); + String dispatcherKey = randomAlphabetic(5); + String otherDispatcherKey = randomAlphabetic(6); + String channelKey = randomAlphabetic(7); + String otherChannelKey = randomAlphabetic(8); + // user1 subscribed only globally + insertProperty(propertyKeyOf(dispatcherKey, channelKey), "true", null, userId1); + // user2 subscribed on project and globally + insertProperty(propertyKeyOf(dispatcherKey, channelKey), "true", null, userId2); + insertProperty(propertyKeyOf(dispatcherKey, channelKey), "true", projectId, userId2); + // user3 subscribed on project only + insertProperty(propertyKeyOf(dispatcherKey, channelKey), "true", projectId, userId3); + // user4 did not subscribe + insertProperty(propertyKeyOf(dispatcherKey, channelKey), "false", projectId, userId4); + Set allLogins = of("user1", "user2", "user3", "user4"); + + assertThat(underTest.findEmailSubscribersForNotification(db.getSession(), dispatcherKey, channelKey, null, allLogins)) + .containsOnly(new EmailSubscriberDto("user1", true, emailOf("user1")), new EmailSubscriberDto("user2", true, emailOf("user2"))); + assertThat(underTest.findEmailSubscribersForNotification(db.getSession(), dispatcherKey, channelKey, null, of("user1", "user2"))) + .containsOnly(new EmailSubscriberDto("user1", true, emailOf("user1")), new EmailSubscriberDto("user2", true, emailOf("user2"))); + assertThat(underTest.findEmailSubscribersForNotification(db.getSession(), dispatcherKey, channelKey, null, of("user2"))) + .containsOnly(new EmailSubscriberDto("user2", true, emailOf("user2"))); + assertThat(underTest.findEmailSubscribersForNotification(db.getSession(), dispatcherKey, channelKey, null, of("user1"))) + .containsOnly(new EmailSubscriberDto("user1", true, emailOf("user1"))); + assertThat(underTest.findEmailSubscribersForNotification(db.getSession(), dispatcherKey, channelKey, null, of())) + .isEmpty(); + + assertThat(underTest.findEmailSubscribersForNotification(db.getSession(), dispatcherKey, otherChannelKey, null, allLogins)) + .isEmpty(); + assertThat(underTest.findEmailSubscribersForNotification(db.getSession(), otherDispatcherKey, channelKey, null, allLogins)) + .isEmpty(); + assertThat(underTest.findEmailSubscribersForNotification(db.getSession(), channelKey, dispatcherKey, null, allLogins)) + .isEmpty(); + } + @Test public void findEmailRecipientsForNotification_finds_global_and_project_subscribed_users_when_projectKey_is_non_null() { int userId1 = db.users().insertUser(withEmail("user1")).getId(); @@ -242,6 +297,56 @@ public class PropertiesDaoTest { .isEmpty(); } + @Test + public void findEmailRecipientsForNotification_with_logins_finds_global_and_project_subscribed_specified_users_when_projectKey_is_non_null() { + int userId1 = db.users().insertUser(withEmail("user1")).getId(); + int userId2 = db.users().insertUser(withEmail("user2")).getId(); + int userId3 = db.users().insertUser(withEmail("user3")).getId(); + int userId4 = db.users().insertUser(withEmail("user4")).getId(); + String projectKey = randomAlphabetic(3); + String otherProjectKey = randomAlphabetic(4); + long projectId = insertPrivateProject(projectKey).getId(); + String dispatcherKey = randomAlphabetic(5); + String otherDispatcherKey = randomAlphabetic(6); + String channelKey = randomAlphabetic(7); + String otherChannelKey = randomAlphabetic(8); + // user1 subscribed only globally + insertProperty(propertyKeyOf(dispatcherKey, channelKey), "true", null, userId1); + // user2 subscribed on project and globally + insertProperty(propertyKeyOf(dispatcherKey, channelKey), "true", null, userId2); + insertProperty(propertyKeyOf(dispatcherKey, channelKey), "true", projectId, userId2); + // user3 subscribed on project only + insertProperty(propertyKeyOf(dispatcherKey, channelKey), "true", projectId, userId3); + // user4 did not subscribe + insertProperty(propertyKeyOf(dispatcherKey, channelKey), "false", projectId, userId4); + Set allLogins = of("user1", "user2", "user3", "user4"); + + assertThat(underTest.findEmailSubscribersForNotification(db.getSession(), dispatcherKey, channelKey, projectKey, allLogins)) + .containsOnly( + new EmailSubscriberDto("user1", true, emailOf("user1")), + new EmailSubscriberDto("user2", true, emailOf("user2")), new EmailSubscriberDto("user2", false, "user2@foo"), + new EmailSubscriberDto("user3", false, emailOf("user3"))); + assertThat(underTest.findEmailSubscribersForNotification(db.getSession(), dispatcherKey, channelKey, projectKey, of("user1"))) + .containsOnly( + new EmailSubscriberDto("user1", true, emailOf("user1"))); + assertThat(underTest.findEmailSubscribersForNotification(db.getSession(), dispatcherKey, channelKey, projectKey, of("user2"))) + .containsOnly( + new EmailSubscriberDto("user2", true, emailOf("user2")), new EmailSubscriberDto("user2", false, "user2@foo")); + assertThat(underTest.findEmailSubscribersForNotification(db.getSession(), dispatcherKey, channelKey, projectKey, of("user3"))) + .containsOnly(new EmailSubscriberDto("user3", false, emailOf("user3"))); + assertThat(underTest.findEmailSubscribersForNotification(db.getSession(), dispatcherKey, channelKey, projectKey, of())) + .isEmpty(); + + assertThat(underTest.findEmailSubscribersForNotification(db.getSession(), dispatcherKey, channelKey, otherProjectKey, allLogins)) + .containsOnly( + new EmailSubscriberDto("user1", true, emailOf("user1")), + new EmailSubscriberDto("user2", true, emailOf("user2"))); + assertThat(underTest.findEmailSubscribersForNotification(db.getSession(), dispatcherKey, otherChannelKey, otherProjectKey, allLogins)) + .isEmpty(); + assertThat(underTest.findEmailSubscribersForNotification(db.getSession(), otherDispatcherKey, channelKey, otherProjectKey, allLogins)) + .isEmpty(); + } + @Test public void findEmailRecipientsForNotification_ignores_subscribed_users_without_email() { int userId1 = db.users().insertUser(withEmail("user1")).getId(); @@ -271,6 +376,36 @@ public class PropertiesDaoTest { new EmailSubscriberDto("user3", true, emailOf("user3"))); } + @Test + public void findEmailRecipientsForNotification_with_logins_ignores_subscribed_users_without_email() { + int userId1 = db.users().insertUser(withEmail("user1")).getId(); + int userId2 = db.users().insertUser(noEmail("user2")).getId(); + int userId3 = db.users().insertUser(withEmail("user3")).getId(); + int userId4 = db.users().insertUser(noEmail("user4")).getId(); + Set allLogins = of("user1", "user2", "user3"); + String projectKey = randomAlphabetic(3); + long projectId = insertPrivateProject(projectKey).getId(); + String dispatcherKey = randomAlphabetic(4); + String channelKey = randomAlphabetic(5); + // user1 and user2 subscribed on project and globally + insertProperty(propertyKeyOf(dispatcherKey, channelKey), "true", null, userId1); + insertProperty(propertyKeyOf(dispatcherKey, channelKey), "true", projectId, userId1); + insertProperty(propertyKeyOf(dispatcherKey, channelKey), "true", null, userId2); + insertProperty(propertyKeyOf(dispatcherKey, channelKey), "true", projectId, userId2); + // user3 and user4 subscribed only globally + insertProperty(propertyKeyOf(dispatcherKey, channelKey), "true", null, userId3); + insertProperty(propertyKeyOf(dispatcherKey, channelKey), "true", null, userId4); + + assertThat(underTest.findEmailSubscribersForNotification(db.getSession(), dispatcherKey, channelKey, projectKey, allLogins)) + .containsOnly( + new EmailSubscriberDto("user1", true, emailOf("user1")), new EmailSubscriberDto("user1", false, emailOf("user1")), + new EmailSubscriberDto("user3", true, emailOf("user3"))); + assertThat(underTest.findEmailSubscribersForNotification(db.getSession(), dispatcherKey, channelKey, null, allLogins)) + .containsOnly( + new EmailSubscriberDto("user1", true, emailOf("user1")), + new EmailSubscriberDto("user3", true, emailOf("user3"))); + } + @Test public void selectGlobalProperties() { // global diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/issue/notification/ChangesOnMyIssueNotificationHandler.java b/server/sonar-server-common/src/main/java/org/sonar/server/issue/notification/ChangesOnMyIssueNotificationHandler.java index f6e69d39f9b..7d5e5243d74 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/issue/notification/ChangesOnMyIssueNotificationHandler.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/issue/notification/ChangesOnMyIssueNotificationHandler.java @@ -26,6 +26,7 @@ import java.util.Map; import java.util.Objects; import java.util.Optional; import java.util.Set; +import java.util.stream.Collectors; import java.util.stream.Stream; import javax.annotation.CheckForNull; import org.sonar.server.notification.EmailNotificationHandler; @@ -89,8 +90,11 @@ public class ChangesOnMyIssueNotificationHandler extends EmailNotificationHandle } private Stream toEmailDeliveryRequests(String projectKey, Collection notifications) { + Set assignees = notifications.stream() + .map(IssueChangeNotification::getAssignee) + .collect(Collectors.toSet()); Map recipientsByLogin = notificationManager - .findSubscribedEmailRecipients(KEY, projectKey, ALL_MUST_HAVE_ROLE_USER) + .findSubscribedEmailRecipients(KEY, projectKey, assignees, ALL_MUST_HAVE_ROLE_USER) .stream() .collect(uniqueIndex(EmailRecipient::getLogin)); return notifications.stream() diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/issue/notification/MyNewIssuesNotificationHandler.java b/server/sonar-server-common/src/main/java/org/sonar/server/issue/notification/MyNewIssuesNotificationHandler.java index 84382720b83..9e4a19cc044 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/issue/notification/MyNewIssuesNotificationHandler.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/issue/notification/MyNewIssuesNotificationHandler.java @@ -25,6 +25,7 @@ import java.util.Map; import java.util.Objects; import java.util.Optional; import java.util.Set; +import java.util.stream.Collectors; import java.util.stream.Stream; import javax.annotation.CheckForNull; import org.sonar.core.util.stream.MoreCollectors; @@ -84,8 +85,11 @@ public class MyNewIssuesNotificationHandler extends EmailNotificationHandler toEmailDeliveryRequests(String projectKey, Collection notifications) { + Set assignees = notifications.stream() + .map(MyNewIssuesNotification::getAssignee) + .collect(Collectors.toSet()); Map recipientsByLogin = notificationManager - .findSubscribedEmailRecipients(KEY, projectKey, ALL_MUST_HAVE_ROLE_USER) + .findSubscribedEmailRecipients(KEY, projectKey, assignees, ALL_MUST_HAVE_ROLE_USER) .stream() .collect(MoreCollectors.uniqueIndex(NotificationManager.EmailRecipient::getLogin)); return notifications.stream() diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/notification/DefaultNotificationManager.java b/server/sonar-server-common/src/main/java/org/sonar/server/notification/DefaultNotificationManager.java index e1a2eae5b7b..1106e19e0fb 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/notification/DefaultNotificationManager.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/notification/DefaultNotificationManager.java @@ -188,16 +188,39 @@ public class DefaultNotificationManager implements NotificationManager { try (DbSession dbSession = dbClient.openSession(false)) { Set emailSubscribers = dbClient.propertiesDao().findEmailSubscribersForNotification( dbSession, dispatcherKey, EmailNotificationChannel.class.getSimpleName(), projectKey); - if (emailSubscribers.isEmpty()) { - return emptySet(); - } - return keepAuthorizedEmailSubscribers(dbSession, projectKey, emailSubscribers, subscriberPermissionsOnProject) - .map(emailSubscriber -> new EmailRecipient(emailSubscriber.getLogin(), emailSubscriber.getEmail())) - .collect(MoreCollectors.toSet()); + return keepAuthorizedEmailSubscribers(dbSession, projectKey, subscriberPermissionsOnProject, emailSubscribers); + } + } + + @Override + public Set findSubscribedEmailRecipients(String dispatcherKey, String projectKey, Set logins, + SubscriberPermissionsOnProject subscriberPermissionsOnProject) { + verifyProjectKey(projectKey); + requireNonNull(logins, "logins can't be null"); + if (logins.isEmpty()) { + return emptySet(); + } + + try (DbSession dbSession = dbClient.openSession(false)) { + Set emailSubscribers = dbClient.propertiesDao().findEmailSubscribersForNotification( + dbSession, dispatcherKey, EmailNotificationChannel.class.getSimpleName(), projectKey, logins); + + return keepAuthorizedEmailSubscribers(dbSession, projectKey, subscriberPermissionsOnProject, emailSubscribers); } } + private Set keepAuthorizedEmailSubscribers(DbSession dbSession, String projectKey, + SubscriberPermissionsOnProject subscriberPermissionsOnProject, Set emailSubscribers) { + if (emailSubscribers.isEmpty()) { + return emptySet(); + } + + return keepAuthorizedEmailSubscribers(dbSession, projectKey, emailSubscribers, subscriberPermissionsOnProject) + .map(emailSubscriber -> new EmailRecipient(emailSubscriber.getLogin(), emailSubscriber.getEmail())) + .collect(MoreCollectors.toSet()); + } + private Stream keepAuthorizedEmailSubscribers(DbSession dbSession, String projectKey, Set emailSubscribers, SubscriberPermissionsOnProject requiredPermissions) { if (requiredPermissions.getGlobalSubscribers().equals(requiredPermissions.getProjectSubscribers())) { diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/notification/NotificationManager.java b/server/sonar-server-common/src/main/java/org/sonar/server/notification/NotificationManager.java index 6135dd4eb0c..13181fb4f2a 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/notification/NotificationManager.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/notification/NotificationManager.java @@ -63,6 +63,7 @@ public interface NotificationManager { Multimap findSubscribedRecipientsForDispatcher(NotificationDispatcher dispatcher, String projectKey, SubscriberPermissionsOnProject subscriberPermissionsOnProject); + @Immutable final class EmailRecipient { private final String login; @@ -104,8 +105,21 @@ public interface NotificationManager { } } + /** + * Find login and email of users which have subscribed to the email notification of the specified {@code dispatcherKey}. + *

+ * Obviously, only subscribers which have an email are returned. + */ Set findSubscribedEmailRecipients(String dispatcherKey, String projectKey, SubscriberPermissionsOnProject subscriberPermissionsOnProject); + /** + * Find email of users with the specified {@code logins} which have subscribed to the email notification of the + * specified {@code dispatcherKey}. + *

+ * Obviously, only subscribers which have an email are returned. + */ + Set findSubscribedEmailRecipients(String dispatcherKey, String projectKey, Set logins, SubscriberPermissionsOnProject subscriberPermissionsOnProject); + final class SubscriberPermissionsOnProject { public static final SubscriberPermissionsOnProject ALL_MUST_HAVE_ROLE_USER = new SubscriberPermissionsOnProject(UserRole.USER); diff --git a/server/sonar-server-common/src/test/java/org/sonar/server/issue/notification/ChangesOnMyIssueNotificationHandlerTest.java b/server/sonar-server-common/src/test/java/org/sonar/server/issue/notification/ChangesOnMyIssueNotificationHandlerTest.java index 6857262e9d8..60e859a8f5f 100644 --- a/server/sonar-server-common/src/test/java/org/sonar/server/issue/notification/ChangesOnMyIssueNotificationHandlerTest.java +++ b/server/sonar-server-common/src/test/java/org/sonar/server/issue/notification/ChangesOnMyIssueNotificationHandlerTest.java @@ -36,7 +36,9 @@ import org.mockito.Mockito; import org.sonar.server.notification.NotificationDispatcherMetadata; import org.sonar.server.notification.NotificationManager; import org.sonar.server.notification.email.EmailNotificationChannel; +import org.sonar.server.notification.email.EmailNotificationChannel.EmailDeliveryRequest; +import static java.util.Collections.singleton; import static java.util.stream.Collectors.toSet; import static org.apache.commons.lang.RandomStringUtils.randomAlphabetic; import static org.assertj.core.api.Assertions.assertThat; @@ -192,8 +194,8 @@ public class ChangesOnMyIssueNotificationHandlerTest { int deliver = underTest.deliver(Stream.concat(notifications1.stream(), notifications2.stream()).collect(toSet())); assertThat(deliver).isZero(); - verify(notificationManager).findSubscribedEmailRecipients(CHANGE_ON_MY_ISSUES_DISPATCHER_KEY, projectKey1, ALL_MUST_HAVE_ROLE_USER); - verify(notificationManager).findSubscribedEmailRecipients(CHANGE_ON_MY_ISSUES_DISPATCHER_KEY, projectKey2, ALL_MUST_HAVE_ROLE_USER); + verify(notificationManager).findSubscribedEmailRecipients(CHANGE_ON_MY_ISSUES_DISPATCHER_KEY, projectKey1, singleton(assignee1), ALL_MUST_HAVE_ROLE_USER); + verify(notificationManager).findSubscribedEmailRecipients(CHANGE_ON_MY_ISSUES_DISPATCHER_KEY, projectKey2, singleton(assignee2), ALL_MUST_HAVE_ROLE_USER); verifyNoMoreInteractions(notificationManager); verify(emailNotificationChannel).isActivated(); verifyNoMoreInteractions(emailNotificationChannel); @@ -205,17 +207,15 @@ public class ChangesOnMyIssueNotificationHandlerTest { String projectKey = randomAlphabetic(5); String assignee1 = randomAlphabetic(6); String assignee2 = randomAlphabetic(7); - String assignee3 = randomAlphabetic(8); // assignee1 is not authorized Set assignee1Notifications = randomSetOfNotifications(projectKey, assignee1, noOrDifferentChangeAuthor); // assignee2 is authorized Set assignee2Notifications = randomSetOfNotifications(projectKey, assignee2, noOrDifferentChangeAuthor); - // assignee3 is authorized but has no notification when(emailNotificationChannel.isActivated()).thenReturn(true); - when(notificationManager.findSubscribedEmailRecipients(CHANGE_ON_MY_ISSUES_DISPATCHER_KEY, projectKey, ALL_MUST_HAVE_ROLE_USER)) - .thenReturn(ImmutableSet.of(emailRecipientOf(assignee2), emailRecipientOf(assignee3))); - Set expectedRequests = assignee2Notifications.stream() - .map(t -> new EmailNotificationChannel.EmailDeliveryRequest(emailOf(t.getAssignee()), t)) + when(notificationManager.findSubscribedEmailRecipients(CHANGE_ON_MY_ISSUES_DISPATCHER_KEY, projectKey, ImmutableSet.of(assignee1, assignee2), ALL_MUST_HAVE_ROLE_USER)) + .thenReturn(ImmutableSet.of(emailRecipientOf(assignee2))); + Set expectedRequests = assignee2Notifications.stream() + .map(t -> new EmailDeliveryRequest(emailOf(t.getAssignee()), t)) .collect(toSet()); int deliveredCount = new Random().nextInt(expectedRequests.size()); when(emailNotificationChannel.deliver(expectedRequests)).thenReturn(deliveredCount); @@ -223,7 +223,7 @@ public class ChangesOnMyIssueNotificationHandlerTest { int deliver = underTest.deliver(Stream.concat(assignee1Notifications.stream(), assignee2Notifications.stream()).collect(toSet())); assertThat(deliver).isEqualTo(deliveredCount); - verify(notificationManager).findSubscribedEmailRecipients(CHANGE_ON_MY_ISSUES_DISPATCHER_KEY, projectKey, ALL_MUST_HAVE_ROLE_USER); + verify(notificationManager).findSubscribedEmailRecipients(CHANGE_ON_MY_ISSUES_DISPATCHER_KEY, projectKey, ImmutableSet.of(assignee1, assignee2), ALL_MUST_HAVE_ROLE_USER); verifyNoMoreInteractions(notificationManager); verify(emailNotificationChannel).isActivated(); verify(emailNotificationChannel).deliver(expectedRequests); @@ -238,22 +238,23 @@ public class ChangesOnMyIssueNotificationHandlerTest { String assignee3 = randomAlphabetic(8); // assignee1 is the changeAuthor of every notification he's the assignee of Set assignee1ChangeAuthor = randomSetOfNotifications(projectKey, assignee1, assignee1); - // assignee1 is the changeAuthor of some notification he's the assignee of + // assignee2 is the changeAuthor of some notification he's the assignee of Set assignee2ChangeAuthor = randomSetOfNotifications(projectKey, assignee2, assignee2); Set assignee2NotChangeAuthor = randomSetOfNotifications(projectKey, assignee2, randomAlphabetic(10)); Set assignee2NoChangeAuthor = randomSetOfNotifications(projectKey, assignee2, NO_CHANGE_AUTHOR); - // assignee2 is never the changeAuthor of the notification he's the assignee of + // assignee3 is never the changeAuthor of the notification he's the assignee of Set assignee3NotChangeAuthor = randomSetOfNotifications(projectKey, assignee3, randomAlphabetic(11)); Set assignee3NoChangeAuthor = randomSetOfNotifications(projectKey, assignee3, NO_CHANGE_AUTHOR); when(emailNotificationChannel.isActivated()).thenReturn(true); - // all assignees have subscribed - when(notificationManager.findSubscribedEmailRecipients(CHANGE_ON_MY_ISSUES_DISPATCHER_KEY, projectKey, ALL_MUST_HAVE_ROLE_USER)) - .thenReturn(ImmutableSet.of(emailRecipientOf(assignee1), emailRecipientOf(assignee2), emailRecipientOf(assignee3))); - Set expectedRequests = Stream.of( + // assignees which are not changeAuthor have subscribed + Set assigneesChangeAuthor = ImmutableSet.of(assignee2, assignee3); + when(notificationManager.findSubscribedEmailRecipients(CHANGE_ON_MY_ISSUES_DISPATCHER_KEY, projectKey, assigneesChangeAuthor, ALL_MUST_HAVE_ROLE_USER)) + .thenReturn(ImmutableSet.of(emailRecipientOf(assignee2), emailRecipientOf(assignee3))); + Set expectedRequests = Stream.of( assignee2NotChangeAuthor.stream(), assignee2NoChangeAuthor.stream(), assignee3NotChangeAuthor.stream(), assignee3NoChangeAuthor.stream()) .flatMap(t -> t) - .map(t -> new EmailNotificationChannel.EmailDeliveryRequest(emailOf(t.getAssignee()), t)) + .map(t -> new EmailDeliveryRequest(emailOf(t.getAssignee()), t)) .collect(toSet()); int deliveredCount = new Random().nextInt(expectedRequests.size()); when(emailNotificationChannel.deliver(expectedRequests)).thenReturn(deliveredCount); @@ -266,7 +267,7 @@ public class ChangesOnMyIssueNotificationHandlerTest { int deliver = underTest.deliver(notifications); assertThat(deliver).isEqualTo(deliveredCount); - verify(notificationManager).findSubscribedEmailRecipients(CHANGE_ON_MY_ISSUES_DISPATCHER_KEY, projectKey, ALL_MUST_HAVE_ROLE_USER); + verify(notificationManager).findSubscribedEmailRecipients(CHANGE_ON_MY_ISSUES_DISPATCHER_KEY, projectKey, assigneesChangeAuthor, ALL_MUST_HAVE_ROLE_USER); verifyNoMoreInteractions(notificationManager); verify(emailNotificationChannel).isActivated(); verify(emailNotificationChannel).deliver(expectedRequests); diff --git a/server/sonar-server-common/src/test/java/org/sonar/server/issue/notification/MyNewIssuesNotificationHandlerTest.java b/server/sonar-server-common/src/test/java/org/sonar/server/issue/notification/MyNewIssuesNotificationHandlerTest.java index 6a85a4ddc9f..365d3e867f9 100644 --- a/server/sonar-server-common/src/test/java/org/sonar/server/issue/notification/MyNewIssuesNotificationHandlerTest.java +++ b/server/sonar-server-common/src/test/java/org/sonar/server/issue/notification/MyNewIssuesNotificationHandlerTest.java @@ -19,7 +19,6 @@ */ package org.sonar.server.issue.notification; -import com.google.common.collect.ImmutableSet; import java.util.Collections; import java.util.Random; import java.util.Set; @@ -35,6 +34,7 @@ import org.sonar.server.notification.NotificationManager.EmailRecipient; import org.sonar.server.notification.email.EmailNotificationChannel; import org.sonar.server.notification.email.EmailNotificationChannel.EmailDeliveryRequest; +import static com.google.common.collect.ImmutableSet.of; import static java.util.Collections.emptySet; import static java.util.stream.Collectors.toSet; import static org.apache.commons.lang.RandomStringUtils.randomAlphabetic; @@ -156,13 +156,13 @@ public class MyNewIssuesNotificationHandlerTest { String assignee = randomAlphabetic(10); MyNewIssuesNotification notification = newNotification(projectKey, assignee); when(emailNotificationChannel.isActivated()).thenReturn(true); - when(notificationManager.findSubscribedEmailRecipients(MY_NEW_ISSUES_DISPATCHER_KEY, projectKey, ALL_MUST_HAVE_ROLE_USER)) + when(notificationManager.findSubscribedEmailRecipients(MY_NEW_ISSUES_DISPATCHER_KEY, projectKey, of(assignee), ALL_MUST_HAVE_ROLE_USER)) .thenReturn(emptySet()); int deliver = underTest.deliver(Collections.singleton(notification)); assertThat(deliver).isZero(); - verify(notificationManager).findSubscribedEmailRecipients(MY_NEW_ISSUES_DISPATCHER_KEY, projectKey, ALL_MUST_HAVE_ROLE_USER); + verify(notificationManager).findSubscribedEmailRecipients(MY_NEW_ISSUES_DISPATCHER_KEY, projectKey, of(assignee), ALL_MUST_HAVE_ROLE_USER); verifyNoMoreInteractions(notificationManager); verify(emailNotificationChannel).isActivated(); verifyNoMoreInteractions(emailNotificationChannel); @@ -185,7 +185,8 @@ public class MyNewIssuesNotificationHandlerTest { .map(n -> new EmailDeliveryRequest(n.getAssignee() + "@foo", n)) .collect(toSet()); when(emailNotificationChannel.isActivated()).thenReturn(true); - when(notificationManager.findSubscribedEmailRecipients(MY_NEW_ISSUES_DISPATCHER_KEY, projectKey, ALL_MUST_HAVE_ROLE_USER)) + Set assignees = withProjectKey.stream().map(MyNewIssuesNotification::getAssignee).collect(toSet()); + when(notificationManager.findSubscribedEmailRecipients(MY_NEW_ISSUES_DISPATCHER_KEY, projectKey, assignees, ALL_MUST_HAVE_ROLE_USER)) .thenReturn(authorizedRecipients); Set notifications = Stream.of(withProjectKey.stream(), noProjectKey.stream(), noProjectKeyNoAssignee.stream()) @@ -194,7 +195,7 @@ public class MyNewIssuesNotificationHandlerTest { int deliver = underTest.deliver(notifications); assertThat(deliver).isZero(); - verify(notificationManager).findSubscribedEmailRecipients(MY_NEW_ISSUES_DISPATCHER_KEY, projectKey, ALL_MUST_HAVE_ROLE_USER); + verify(notificationManager).findSubscribedEmailRecipients(MY_NEW_ISSUES_DISPATCHER_KEY, projectKey, assignees, ALL_MUST_HAVE_ROLE_USER); verifyNoMoreInteractions(notificationManager); verify(emailNotificationChannel).isActivated(); verify(emailNotificationChannel).deliver(expectedRequests); @@ -216,7 +217,8 @@ public class MyNewIssuesNotificationHandlerTest { .map(n -> new EmailDeliveryRequest(n.getAssignee() + "@foo", n)) .collect(toSet()); when(emailNotificationChannel.isActivated()).thenReturn(true); - when(notificationManager.findSubscribedEmailRecipients(MY_NEW_ISSUES_DISPATCHER_KEY, projectKey, ALL_MUST_HAVE_ROLE_USER)) + Set assignees = withAssignee.stream().map(MyNewIssuesNotification::getAssignee).collect(toSet()); + when(notificationManager.findSubscribedEmailRecipients(MY_NEW_ISSUES_DISPATCHER_KEY, projectKey, assignees, ALL_MUST_HAVE_ROLE_USER)) .thenReturn(authorizedRecipients); Set notifications = Stream.of(withAssignee.stream(), noAssignee.stream(), noProjectKeyNoAssignee.stream()) @@ -225,7 +227,7 @@ public class MyNewIssuesNotificationHandlerTest { int deliver = underTest.deliver(notifications); assertThat(deliver).isZero(); - verify(notificationManager).findSubscribedEmailRecipients(MY_NEW_ISSUES_DISPATCHER_KEY, projectKey, ALL_MUST_HAVE_ROLE_USER); + verify(notificationManager).findSubscribedEmailRecipients(MY_NEW_ISSUES_DISPATCHER_KEY, projectKey, assignees, ALL_MUST_HAVE_ROLE_USER); verifyNoMoreInteractions(notificationManager); verify(emailNotificationChannel).isActivated(); verify(emailNotificationChannel).deliver(expectedRequests); @@ -241,16 +243,16 @@ public class MyNewIssuesNotificationHandlerTest { Set notifications1 = randomSetOfNotifications(projectKey1, assignee1); Set notifications2 = randomSetOfNotifications(projectKey2, assignee2); when(emailNotificationChannel.isActivated()).thenReturn(true); - when(notificationManager.findSubscribedEmailRecipients(MY_NEW_ISSUES_DISPATCHER_KEY, projectKey1, ALL_MUST_HAVE_ROLE_USER)) + when(notificationManager.findSubscribedEmailRecipients(MY_NEW_ISSUES_DISPATCHER_KEY, projectKey1, of(assignee1), ALL_MUST_HAVE_ROLE_USER)) .thenReturn(emptySet()); - when(notificationManager.findSubscribedEmailRecipients(MY_NEW_ISSUES_DISPATCHER_KEY, projectKey2, ALL_MUST_HAVE_ROLE_USER)) + when(notificationManager.findSubscribedEmailRecipients(MY_NEW_ISSUES_DISPATCHER_KEY, projectKey2, of(assignee2),ALL_MUST_HAVE_ROLE_USER)) .thenReturn(emptySet()); int deliver = underTest.deliver(Stream.concat(notifications1.stream(), notifications2.stream()).collect(toSet())); assertThat(deliver).isZero(); - verify(notificationManager).findSubscribedEmailRecipients(MY_NEW_ISSUES_DISPATCHER_KEY, projectKey1, ALL_MUST_HAVE_ROLE_USER); - verify(notificationManager).findSubscribedEmailRecipients(MY_NEW_ISSUES_DISPATCHER_KEY, projectKey2, ALL_MUST_HAVE_ROLE_USER); + verify(notificationManager).findSubscribedEmailRecipients(MY_NEW_ISSUES_DISPATCHER_KEY, projectKey1, of(assignee1), ALL_MUST_HAVE_ROLE_USER); + verify(notificationManager).findSubscribedEmailRecipients(MY_NEW_ISSUES_DISPATCHER_KEY, projectKey2, of(assignee2), ALL_MUST_HAVE_ROLE_USER); verifyNoMoreInteractions(notificationManager); verify(emailNotificationChannel).isActivated(); verifyNoMoreInteractions(emailNotificationChannel); @@ -261,15 +263,14 @@ public class MyNewIssuesNotificationHandlerTest { String projectKey = randomAlphabetic(5); String assignee1 = randomAlphabetic(6); String assignee2 = randomAlphabetic(7); - String assignee3 = randomAlphabetic(8); + Set assignees = of(assignee1, assignee2); // assignee1 is not authorized Set assignee1Notifications = randomSetOfNotifications(projectKey, assignee1); // assignee2 is authorized Set assignee2Notifications = randomSetOfNotifications(projectKey, assignee2); - // assignee 3 is authorized but has no notification when(emailNotificationChannel.isActivated()).thenReturn(true); - when(notificationManager.findSubscribedEmailRecipients(MY_NEW_ISSUES_DISPATCHER_KEY, projectKey, ALL_MUST_HAVE_ROLE_USER)) - .thenReturn(ImmutableSet.of(emailRecipientOf(assignee2), emailRecipientOf(assignee3))); + when(notificationManager.findSubscribedEmailRecipients(MY_NEW_ISSUES_DISPATCHER_KEY, projectKey, assignees, ALL_MUST_HAVE_ROLE_USER)) + .thenReturn(of(emailRecipientOf(assignee2))); Set expectedRequests = assignee2Notifications.stream() .map(t -> new EmailDeliveryRequest(emailOf(t.getAssignee()), t)) .collect(toSet()); @@ -279,7 +280,7 @@ public class MyNewIssuesNotificationHandlerTest { int deliver = underTest.deliver(Stream.concat(assignee1Notifications.stream(), assignee2Notifications.stream()).collect(toSet())); assertThat(deliver).isEqualTo(deliveredCount); - verify(notificationManager).findSubscribedEmailRecipients(MY_NEW_ISSUES_DISPATCHER_KEY, projectKey, ALL_MUST_HAVE_ROLE_USER); + verify(notificationManager).findSubscribedEmailRecipients(MY_NEW_ISSUES_DISPATCHER_KEY, projectKey, assignees, ALL_MUST_HAVE_ROLE_USER); verifyNoMoreInteractions(notificationManager); verify(emailNotificationChannel).isActivated(); verify(emailNotificationChannel).deliver(expectedRequests); @@ -305,11 +306,11 @@ public class MyNewIssuesNotificationHandlerTest { Set assignee3Project2 = randomSetOfNotifications(projectKey2, assignee3); Set assignee3Project3 = randomSetOfNotifications(projectKey3, assignee3); when(emailNotificationChannel.isActivated()).thenReturn(true); - when(notificationManager.findSubscribedEmailRecipients(MY_NEW_ISSUES_DISPATCHER_KEY, projectKey1, ALL_MUST_HAVE_ROLE_USER)) - .thenReturn(ImmutableSet.of(emailRecipientOf(assignee1), emailRecipientOf(assignee2))); - when(notificationManager.findSubscribedEmailRecipients(MY_NEW_ISSUES_DISPATCHER_KEY, projectKey2, ALL_MUST_HAVE_ROLE_USER)) - .thenReturn(ImmutableSet.of(emailRecipientOf(assignee2), emailRecipientOf(assignee3))); - when(notificationManager.findSubscribedEmailRecipients(MY_NEW_ISSUES_DISPATCHER_KEY, projectKey3, ALL_MUST_HAVE_ROLE_USER)) + when(notificationManager.findSubscribedEmailRecipients(MY_NEW_ISSUES_DISPATCHER_KEY, projectKey1, of(assignee1, assignee2), ALL_MUST_HAVE_ROLE_USER)) + .thenReturn(of(emailRecipientOf(assignee1), emailRecipientOf(assignee2))); + when(notificationManager.findSubscribedEmailRecipients(MY_NEW_ISSUES_DISPATCHER_KEY, projectKey2, of(assignee1, assignee2, assignee3), ALL_MUST_HAVE_ROLE_USER)) + .thenReturn(of(emailRecipientOf(assignee2), emailRecipientOf(assignee3))); + when(notificationManager.findSubscribedEmailRecipients(MY_NEW_ISSUES_DISPATCHER_KEY, projectKey3, of(assignee2, assignee3), ALL_MUST_HAVE_ROLE_USER)) .thenReturn(emptySet()); Set expectedRequests = Stream.of( assignee1Project1.stream(), assignee2Project1.stream(), assignee2Project2.stream(), assignee3Project2.stream()) @@ -328,9 +329,9 @@ public class MyNewIssuesNotificationHandlerTest { int deliver = underTest.deliver(notifications); assertThat(deliver).isEqualTo(deliveredCount); - verify(notificationManager).findSubscribedEmailRecipients(MY_NEW_ISSUES_DISPATCHER_KEY, projectKey1, ALL_MUST_HAVE_ROLE_USER); - verify(notificationManager).findSubscribedEmailRecipients(MY_NEW_ISSUES_DISPATCHER_KEY, projectKey2, ALL_MUST_HAVE_ROLE_USER); - verify(notificationManager).findSubscribedEmailRecipients(MY_NEW_ISSUES_DISPATCHER_KEY, projectKey3, ALL_MUST_HAVE_ROLE_USER); + verify(notificationManager).findSubscribedEmailRecipients(MY_NEW_ISSUES_DISPATCHER_KEY, projectKey1, of(assignee1, assignee2), ALL_MUST_HAVE_ROLE_USER); + verify(notificationManager).findSubscribedEmailRecipients(MY_NEW_ISSUES_DISPATCHER_KEY, projectKey2, of(assignee1, assignee2, assignee3), ALL_MUST_HAVE_ROLE_USER); + verify(notificationManager).findSubscribedEmailRecipients(MY_NEW_ISSUES_DISPATCHER_KEY, projectKey3, of(assignee2, assignee3), ALL_MUST_HAVE_ROLE_USER); verifyNoMoreInteractions(notificationManager); verify(emailNotificationChannel).isActivated(); verify(emailNotificationChannel).deliver(expectedRequests); diff --git a/server/sonar-server-common/src/test/java/org/sonar/server/notification/DefaultNotificationManagerTest.java b/server/sonar-server-common/src/test/java/org/sonar/server/notification/DefaultNotificationManagerTest.java index a78a8c5e985..d1520314553 100644 --- a/server/sonar-server-common/src/test/java/org/sonar/server/notification/DefaultNotificationManagerTest.java +++ b/server/sonar-server-common/src/test/java/org/sonar/server/notification/DefaultNotificationManagerTest.java @@ -271,6 +271,37 @@ public class DefaultNotificationManagerTest { underTest.findSubscribedEmailRecipients(dispatcherKey, null, ALL_MUST_HAVE_ROLE_USER); } + @Test + public void findSubscribedEmailRecipients_with_logins_fails_with_NPE_if_projectKey_is_null() { + String dispatcherKey = randomAlphabetic(12); + + expectedException.expect(NullPointerException.class); + expectedException.expectMessage("projectKey is mandatory"); + + underTest.findSubscribedEmailRecipients(dispatcherKey, null, ImmutableSet.of(), ALL_MUST_HAVE_ROLE_USER); + } + + @Test + public void findSubscribedEmailRecipients_with_logins_fails_with_NPE_if_logins_is_null() { + String dispatcherKey = randomAlphabetic(12); + String projectKey = randomAlphabetic(6); + + expectedException.expect(NullPointerException.class); + expectedException.expectMessage("logins can't be null"); + + underTest.findSubscribedEmailRecipients(dispatcherKey, projectKey, null, ALL_MUST_HAVE_ROLE_USER); + } + + @Test + public void findSubscribedEmailRecipients_with_logins_returns_empty_if_login_set_is_empty() { + String dispatcherKey = randomAlphabetic(12); + String projectKey = randomAlphabetic(6); + + Set recipients = underTest.findSubscribedEmailRecipients(dispatcherKey, projectKey, ImmutableSet.of(), ALL_MUST_HAVE_ROLE_USER); + + assertThat(recipients).isEmpty(); + } + @Test public void findSubscribedEmailRecipients_returns_empty_if_no_email_recipients_in_project_for_dispatcher_key() { String dispatcherKey = randomAlphabetic(12); @@ -287,22 +318,34 @@ public class DefaultNotificationManagerTest { verify(authorizationDao, times(0)).keepAuthorizedLoginsOnProject(any(DbSession.class), anySet(), anyString(), anyString()); } + @Test + public void findSubscribedEmailRecipients_with_logins_returns_empty_if_no_email_recipients_in_project_for_dispatcher_key() { + String dispatcherKey = randomAlphabetic(12); + String globalPermission = randomAlphanumeric(4); + String projectPermission = randomAlphanumeric(5); + String projectKey = randomAlphabetic(6); + Set logins = IntStream.range(0, 1 + new Random().nextInt(10)) + .mapToObj(i -> "login_" + i) + .collect(Collectors.toSet()); + when(propertiesDao.findEmailSubscribersForNotification(dbSession, dispatcherKey, "EmailNotificationChannel", projectKey, logins)) + .thenReturn(Collections.emptySet()); + + Set emailRecipients = underTest.findSubscribedEmailRecipients(dispatcherKey, projectKey, logins, + new SubscriberPermissionsOnProject(globalPermission, projectPermission)); + assertThat(emailRecipients).isEmpty(); + + verify(authorizationDao, times(0)).keepAuthorizedLoginsOnProject(any(DbSession.class), anySet(), anyString(), anyString()); + } + @Test public void findSubscribedEmailRecipients_applies_distinct_permission_filtering_global_or_project_subscribers() { String dispatcherKey = randomAlphabetic(12); - String otherDispatcherKey = randomAlphabetic(13); String globalPermission = randomAlphanumeric(4); String projectPermission = randomAlphanumeric(5); String projectKey = randomAlphabetic(6); - String otherProjectKey = randomAlphabetic(7); when(propertiesDao.findEmailSubscribersForNotification(dbSession, dispatcherKey, "EmailNotificationChannel", projectKey)) .thenReturn( newHashSet(new EmailSubscriberDto("user1", false, "user1@foo"), new EmailSubscriberDto("user3", false, "user3@foo"), new EmailSubscriberDto("user3", true, "user3@foo"))); - when(propertiesDao.findEmailSubscribersForNotification(dbSession, dispatcherKey, "EmailNotificationChannel", otherProjectKey)) - .thenReturn(newHashSet(new EmailSubscriberDto("user2", false, "user2@foo"))); - when(propertiesDao.findEmailSubscribersForNotification(dbSession, otherDispatcherKey, "EmailNotificationChannel", projectKey)) - .thenReturn(newHashSet(new EmailSubscriberDto("user4", true, "user4@foo"))); - when(authorizationDao.keepAuthorizedLoginsOnProject(dbSession, newHashSet("user3", "user4"), projectKey, globalPermission)) .thenReturn(newHashSet("user3")); when(authorizationDao.keepAuthorizedLoginsOnProject(dbSession, newHashSet("user1", "user3"), projectKey, projectPermission)) @@ -319,7 +362,32 @@ public class DefaultNotificationManagerTest { } @Test - public void findSubscribedEmailRecipients_do_not_call_db_for_project_permission_filtering_if_there_is_no_project_subscriber() { + public void findSubscribedEmailRecipients_with_logins_applies_distinct_permission_filtering_global_or_project_subscribers() { + String dispatcherKey = randomAlphabetic(12); + String globalPermission = randomAlphanumeric(4); + String projectPermission = randomAlphanumeric(5); + String projectKey = randomAlphabetic(6); + Set logins = ImmutableSet.of("user1", "user2", "user3"); + when(propertiesDao.findEmailSubscribersForNotification(dbSession, dispatcherKey, "EmailNotificationChannel", projectKey, logins)) + .thenReturn( + newHashSet(new EmailSubscriberDto("user1", false, "user1@foo"), new EmailSubscriberDto("user3", false, "user3@foo"), new EmailSubscriberDto("user3", true, "user3@foo"))); + when(authorizationDao.keepAuthorizedLoginsOnProject(dbSession, newHashSet("user3", "user4"), projectKey, globalPermission)) + .thenReturn(newHashSet("user3")); + when(authorizationDao.keepAuthorizedLoginsOnProject(dbSession, newHashSet("user1", "user3"), projectKey, projectPermission)) + .thenReturn(newHashSet("user1", "user3")); + + Set emailRecipients = underTest.findSubscribedEmailRecipients(dispatcherKey, projectKey, logins, + new SubscriberPermissionsOnProject(globalPermission, projectPermission)); + assertThat(emailRecipients) + .isEqualTo(ImmutableSet.of(new EmailRecipient("user1", "user1@foo"), new EmailRecipient("user3", "user3@foo"))); + + // code is optimized to perform only 2 SQL requests for all channels + verify(authorizationDao, times(1)).keepAuthorizedLoginsOnProject(eq(dbSession), anySet(), anyString(), eq(globalPermission)); + verify(authorizationDao, times(1)).keepAuthorizedLoginsOnProject(eq(dbSession), anySet(), anyString(), eq(projectPermission)); + } + + @Test + public void findSubscribedEmailRecipients_does_not_call_db_for_project_permission_filtering_if_there_is_no_project_subscriber() { String dispatcherKey = randomAlphabetic(12); String globalPermission = randomAlphanumeric(4); String projectPermission = randomAlphanumeric(5); @@ -327,9 +395,9 @@ public class DefaultNotificationManagerTest { Set subscribers = IntStream.range(0, 1 + new Random().nextInt(10)) .mapToObj(i -> new EmailSubscriberDto("user" + i, true, "user" + i + "@sonarsource.com")) .collect(Collectors.toSet()); + Set logins = subscribers.stream().map(EmailSubscriberDto::getLogin).collect(Collectors.toSet()); when(propertiesDao.findEmailSubscribersForNotification(dbSession, dispatcherKey, "EmailNotificationChannel", projectKey)) .thenReturn(subscribers); - Set logins = subscribers.stream().map(EmailSubscriberDto::getLogin).collect(Collectors.toSet()); when(authorizationDao.keepAuthorizedLoginsOnProject(dbSession, logins, projectKey, globalPermission)) .thenReturn(logins); @@ -343,6 +411,31 @@ public class DefaultNotificationManagerTest { verify(authorizationDao, times(0)).keepAuthorizedLoginsOnProject(eq(dbSession), anySet(), anyString(), eq(projectPermission)); } + @Test + public void findSubscribedEmailRecipients_with_logins_does_not_call_db_for_project_permission_filtering_if_there_is_no_project_subscriber() { + String dispatcherKey = randomAlphabetic(12); + String globalPermission = randomAlphanumeric(4); + String projectPermission = randomAlphanumeric(5); + String projectKey = randomAlphabetic(6); + Set subscribers = IntStream.range(0, 1 + new Random().nextInt(10)) + .mapToObj(i -> new EmailSubscriberDto("user" + i, true, "user" + i + "@sonarsource.com")) + .collect(Collectors.toSet()); + Set logins = subscribers.stream().map(EmailSubscriberDto::getLogin).collect(Collectors.toSet()); + when(propertiesDao.findEmailSubscribersForNotification(dbSession, dispatcherKey, "EmailNotificationChannel", projectKey, logins)) + .thenReturn(subscribers); + when(authorizationDao.keepAuthorizedLoginsOnProject(dbSession, logins, projectKey, globalPermission)) + .thenReturn(logins); + + Set emailRecipients = underTest.findSubscribedEmailRecipients(dispatcherKey, projectKey, logins, + new SubscriberPermissionsOnProject(globalPermission, projectPermission)); + Set expected = subscribers.stream().map(i -> new EmailRecipient(i.getLogin(), i.getEmail())).collect(Collectors.toSet()); + assertThat(emailRecipients) + .isEqualTo(expected); + + verify(authorizationDao, times(1)).keepAuthorizedLoginsOnProject(eq(dbSession), anySet(), anyString(), eq(globalPermission)); + verify(authorizationDao, times(0)).keepAuthorizedLoginsOnProject(eq(dbSession), anySet(), anyString(), eq(projectPermission)); + } + @Test public void findSubscribedEmailRecipients_does_not_call_DB_for_project_permission_filtering_if_there_is_no_global_subscriber() { String dispatcherKey = randomAlphabetic(12); @@ -352,9 +445,9 @@ public class DefaultNotificationManagerTest { Set subscribers = IntStream.range(0, 1 + new Random().nextInt(10)) .mapToObj(i -> new EmailSubscriberDto("user" + i, false, "user" + i + "@sonarsource.com")) .collect(Collectors.toSet()); + Set logins = subscribers.stream().map(EmailSubscriberDto::getLogin).collect(Collectors.toSet()); when(propertiesDao.findEmailSubscribersForNotification(dbSession, dispatcherKey, "EmailNotificationChannel", projectKey)) .thenReturn(subscribers); - Set logins = subscribers.stream().map(EmailSubscriberDto::getLogin).collect(Collectors.toSet()); when(authorizationDao.keepAuthorizedLoginsOnProject(dbSession, logins, projectKey, projectPermission)) .thenReturn(logins); @@ -367,4 +460,29 @@ public class DefaultNotificationManagerTest { verify(authorizationDao, times(0)).keepAuthorizedLoginsOnProject(eq(dbSession), anySet(), anyString(), eq(globalPermission)); verify(authorizationDao, times(1)).keepAuthorizedLoginsOnProject(eq(dbSession), anySet(), anyString(), eq(projectPermission)); } + + @Test + public void findSubscribedEmailRecipients_with_logins_does_not_call_DB_for_project_permission_filtering_if_there_is_no_global_subscriber() { + String dispatcherKey = randomAlphabetic(12); + String globalPermission = randomAlphanumeric(4); + String projectPermission = randomAlphanumeric(5); + String projectKey = randomAlphabetic(6); + Set subscribers = IntStream.range(0, 1 + new Random().nextInt(10)) + .mapToObj(i -> new EmailSubscriberDto("user" + i, false, "user" + i + "@sonarsource.com")) + .collect(Collectors.toSet()); + Set logins = subscribers.stream().map(EmailSubscriberDto::getLogin).collect(Collectors.toSet()); + when(propertiesDao.findEmailSubscribersForNotification(dbSession, dispatcherKey, "EmailNotificationChannel", projectKey, logins)) + .thenReturn(subscribers); + when(authorizationDao.keepAuthorizedLoginsOnProject(dbSession, logins, projectKey, projectPermission)) + .thenReturn(logins); + + Set emailRecipients = underTest.findSubscribedEmailRecipients(dispatcherKey, projectKey, logins, + new SubscriberPermissionsOnProject(globalPermission, projectPermission)); + Set expected = subscribers.stream().map(i -> new EmailRecipient(i.getLogin(), i.getEmail())).collect(Collectors.toSet()); + assertThat(emailRecipients) + .isEqualTo(expected); + + verify(authorizationDao, times(0)).keepAuthorizedLoginsOnProject(eq(dbSession), anySet(), anyString(), eq(globalPermission)); + verify(authorizationDao, times(1)).keepAuthorizedLoginsOnProject(eq(dbSession), anySet(), anyString(), eq(projectPermission)); + } } -- 2.39.5