diff options
11 files changed, 286 insertions, 33 deletions
diff --git a/sonar-core/src/main/java/org/sonar/core/notification/DefaultNotificationManager.java b/sonar-core/src/main/java/org/sonar/core/notification/DefaultNotificationManager.java index b729ca15ec9..e7bf3d62ebb 100644 --- a/sonar-core/src/main/java/org/sonar/core/notification/DefaultNotificationManager.java +++ b/sonar-core/src/main/java/org/sonar/core/notification/DefaultNotificationManager.java @@ -19,13 +19,17 @@ */ package org.sonar.core.notification; +import com.google.common.collect.HashMultimap; +import com.google.common.collect.SetMultimap; import org.sonar.api.database.DatabaseSession; -import org.sonar.api.database.configuration.Property; -import org.sonar.api.database.model.User; import org.sonar.api.notifications.Notification; +import org.sonar.api.notifications.NotificationChannel; +import org.sonar.api.notifications.NotificationDispatcher; import org.sonar.api.notifications.NotificationManager; +import org.sonar.core.properties.PropertiesDao; import org.sonar.jpa.session.DatabaseSessionFactory; +import java.util.Arrays; import java.util.Date; import java.util.List; @@ -34,12 +38,36 @@ import java.util.List; */ public class DefaultNotificationManager implements NotificationManager { + private NotificationChannel[] notificationChannels; private DatabaseSessionFactory sessionFactory; + private PropertiesDao propertiesDao; - public DefaultNotificationManager(DatabaseSessionFactory sessionFactory) { + /** + * Default constructor used by Pico + */ + public DefaultNotificationManager(NotificationChannel[] channels, DatabaseSessionFactory sessionFactory, PropertiesDao propertiesDao) { + this.notificationChannels = channels; this.sessionFactory = sessionFactory; + this.propertiesDao = propertiesDao; } + /** + * Constructor if no notification channel + */ + public DefaultNotificationManager(DatabaseSessionFactory sessionFactory, PropertiesDao propertiesDao) { + this(new NotificationChannel[0], sessionFactory, propertiesDao); + } + + /** + * Returns all the available notification channels + */ + public List<NotificationChannel> getChannels() { + return Arrays.asList(notificationChannels); + } + + /** + * {@inheritDoc} + */ public void scheduleForSending(Notification notification) { NotificationQueueElement notificationQueueElement = new NotificationQueueElement(); notificationQueueElement.setCreatedAt(new Date()); @@ -49,6 +77,9 @@ public class DefaultNotificationManager implements NotificationManager { session.commit(); } + /** + * Give the notification queue so that it can be processed + */ public NotificationQueueElement getFromQueue() { DatabaseSession session = sessionFactory.getSession(); String hql = "FROM " + NotificationQueueElement.class.getSimpleName() + " ORDER BY createdAt ASC"; @@ -68,12 +99,32 @@ public class DefaultNotificationManager implements NotificationManager { } - public boolean isEnabled(String username, String channelKey, String dispatcherKey) { - DatabaseSession session = sessionFactory.getSession(); - User user = session.getSingleResult(User.class, "login", username); - String notificationKey = "notification." + dispatcherKey + "." + channelKey; - Property property = session.getSingleResult(Property.class, "userId", user.getId(), "key", notificationKey); - return property != null && "true".equals(property.getValue()); + /** + * {@inheritDoc} + */ + public SetMultimap<String, NotificationChannel> findSubscribedRecipientsForDispatcher(NotificationDispatcher dispatcher, Integer resourceId) { + String dispatcherKey = dispatcher.getKey(); + + SetMultimap<String, NotificationChannel> recipients = HashMultimap.create(); + for (NotificationChannel channel : notificationChannels) { + String channelKey = channel.getKey(); + + // Find users subscribed globally to the dispatcher (i.e. not on a specific project) + addUsersToRecipientListForChannel(propertiesDao.findUsersForNotification(dispatcherKey, channelKey, null), recipients, channel); + + if (resourceId != null) { + // Find users subscribed to the dispatcher specifically for the resource + addUsersToRecipientListForChannel(propertiesDao.findUsersForNotification(dispatcherKey, channelKey, resourceId.longValue()), recipients, channel); + } + } + + return recipients; + } + + private void addUsersToRecipientListForChannel(List<String> users, SetMultimap<String, NotificationChannel> recipients, NotificationChannel channel) { + for (String username : users) { + recipients.put(username, channel); + } } } diff --git a/sonar-core/src/main/java/org/sonar/core/properties/PropertiesDao.java b/sonar-core/src/main/java/org/sonar/core/properties/PropertiesDao.java index 32f3b60fa4b..3ee901fc9c2 100644 --- a/sonar-core/src/main/java/org/sonar/core/properties/PropertiesDao.java +++ b/sonar-core/src/main/java/org/sonar/core/properties/PropertiesDao.java @@ -27,6 +27,8 @@ import org.sonar.api.BatchComponent; import org.sonar.api.ServerComponent; import org.sonar.core.persistence.MyBatis; +import javax.annotation.Nullable; + import java.util.List; import java.util.Map; @@ -54,6 +56,25 @@ public class PropertiesDao implements BatchComponent, ServerComponent { } } + /** + * 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. + * + * @param notificationDispatcherKey the key of the notification dispatcher + * @param notificationChannelKey the key of the notification channel + * @param resourceId the resource id + * @return the list of logins (maybe be empty - obviously) + */ + public List<String> findUsersForNotification(String notificationDispatcherKey, String notificationChannelKey, @Nullable Long resourceId) { + SqlSession session = mybatis.openSession(); + PropertiesMapper mapper = session.getMapper(PropertiesMapper.class); + try { + return mapper.findUsersForNotification("notification." + notificationDispatcherKey + "." + notificationChannelKey, resourceId); + } finally { + MyBatis.closeQuietly(session); + } + } + public List<PropertyDto> selectGlobalProperties() { SqlSession session = mybatis.openSession(); PropertiesMapper mapper = session.getMapper(PropertiesMapper.class); diff --git a/sonar-core/src/main/java/org/sonar/core/properties/PropertiesMapper.java b/sonar-core/src/main/java/org/sonar/core/properties/PropertiesMapper.java index c80a4d32c0f..a1395833653 100644 --- a/sonar-core/src/main/java/org/sonar/core/properties/PropertiesMapper.java +++ b/sonar-core/src/main/java/org/sonar/core/properties/PropertiesMapper.java @@ -21,18 +21,31 @@ package org.sonar.core.properties; import org.apache.ibatis.annotations.Param; +import javax.annotation.Nullable; + import java.util.List; public interface PropertiesMapper { List<String> findUserIdsForFavouriteResource(Long resourceId); + + List<String> findUsersForNotification(@Param("notifKey") String notificationKey, @Nullable @Param("rId") Long resourceId); + List<PropertyDto> selectGlobalProperties(); + List<PropertyDto> selectProjectProperties(String resourceKey); + List<PropertyDto> selectSetOfResourceProperties(@Param("rId") Long projectId, @Param("propKeys") List<String> propertyKeys); + PropertyDto selectByKey(PropertyDto key); + void update(PropertyDto property); + void insert(PropertyDto property); + void deleteGlobalProperty(String key); + void deleteGlobalProperties(); + void renamePropertyKey(@Param("oldKey") String oldKey, @Param("newKey") String newKey); } diff --git a/sonar-core/src/main/resources/org/sonar/core/properties/PropertiesMapper.xml b/sonar-core/src/main/resources/org/sonar/core/properties/PropertiesMapper.xml index 756892b24f0..d8df028f0f7 100644 --- a/sonar-core/src/main/resources/org/sonar/core/properties/PropertiesMapper.xml +++ b/sonar-core/src/main/resources/org/sonar/core/properties/PropertiesMapper.xml @@ -9,6 +9,18 @@ WHERE P.prop_key = 'favourite' AND P.resource_id = #{id} AND P.user_id = U.id </select> + <select id="findUsersForNotification" parameterType="map" resultType="String"> + SELECT U.login + FROM properties P, users U + WHERE P.user_id = U.id AND P.prop_key = #{notifKey} AND P.text_value = 'true' + <if test="rId == null"> + AND p.resource_id is null + </if> + <if test="rId != null"> + AND p.resource_id = #{rId} + </if> + </select> + <select id="selectGlobalProperties" resultType="Property"> select p.id as id, p.prop_key as "key", p.text_value as value, p.resource_id as resourceId, p.user_id as userId from properties p diff --git a/sonar-core/src/test/java/org/sonar/core/notification/DefaultNotificationManagerTest.java b/sonar-core/src/test/java/org/sonar/core/notification/DefaultNotificationManagerTest.java index 8c2d9c85ccf..75a7b5f9d5b 100644 --- a/sonar-core/src/test/java/org/sonar/core/notification/DefaultNotificationManagerTest.java +++ b/sonar-core/src/test/java/org/sonar/core/notification/DefaultNotificationManagerTest.java @@ -19,22 +19,59 @@ */ package org.sonar.core.notification; +import com.google.common.collect.Lists; +import com.google.common.collect.SetMultimap; import org.junit.Before; import org.junit.Test; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; import org.sonar.api.notifications.Notification; +import org.sonar.api.notifications.NotificationChannel; +import org.sonar.api.notifications.NotificationDispatcher; +import org.sonar.core.properties.PropertiesDao; import org.sonar.jpa.test.AbstractDbUnitTestCase; +import java.util.Collection; +import java.util.Map; + +import static org.fest.assertions.Assertions.assertThat; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.nullValue; import static org.junit.Assert.assertThat; +import static org.mockito.Mockito.when; public class DefaultNotificationManagerTest extends AbstractDbUnitTestCase { private DefaultNotificationManager manager; + @Mock + private PropertiesDao propertiesDao; + + @Mock + private NotificationDispatcher dispatcher; + + @Mock + private NotificationChannel emailChannel; + + @Mock + private NotificationChannel twitterChannel; + @Before public void setUp() { - manager = new DefaultNotificationManager(getSessionFactory()); + MockitoAnnotations.initMocks(this); + when(dispatcher.getKey()).thenReturn("NewViolations"); + when(emailChannel.getKey()).thenReturn("Email"); + when(twitterChannel.getKey()).thenReturn("Twitter"); + + manager = new DefaultNotificationManager(new NotificationChannel[] {emailChannel, twitterChannel}, getSessionFactory(), propertiesDao); + } + + @Test + public void shouldProvideChannelList() { + assertThat(manager.getChannels()).containsOnly(emailChannel, twitterChannel); + + manager = new DefaultNotificationManager(getSessionFactory(), propertiesDao); + assertThat(manager.getChannels()).hasSize(0); } @Test @@ -49,10 +86,44 @@ public class DefaultNotificationManagerTest extends AbstractDbUnitTestCase { } @Test - public void shouldCheckEnablement() { - setupData("fixture"); - assertThat(manager.isEnabled("simon", "email", "CommentOnReviewAssignedToMe"), is(true)); - assertThat(manager.isEnabled("godin", "email", "CommentOnReviewAssignedToMe"), is(false)); + public void shouldFindNoRecipient() { + assertThat(manager.findSubscribedRecipientsForDispatcher(dispatcher, 45).asMap().entrySet()).hasSize(0); + } + + @Test + public void shouldFindSubscribedRecipientForGivenResource() { + when(propertiesDao.findUsersForNotification("NewViolations", "Email", 45L)).thenReturn(Lists.newArrayList("user1", "user2")); + when(propertiesDao.findUsersForNotification("NewViolations", "Email", null)).thenReturn(Lists.newArrayList("user1", "user3")); + when(propertiesDao.findUsersForNotification("NewViolations", "Twitter", 56L)).thenReturn(Lists.newArrayList("user2")); + when(propertiesDao.findUsersForNotification("NewViolations", "Twitter", null)).thenReturn(Lists.newArrayList("user3")); + when(propertiesDao.findUsersForNotification("NewAlerts", "Twitter", null)).thenReturn(Lists.newArrayList("user4")); + + SetMultimap<String, NotificationChannel> multiMap = manager.findSubscribedRecipientsForDispatcher(dispatcher, 45); + assertThat(multiMap.entries()).hasSize(4); + + Map<String, Collection<NotificationChannel>> 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.findUsersForNotification("NewViolations", "Email", 45L)).thenReturn(Lists.newArrayList("user1", "user2")); + when(propertiesDao.findUsersForNotification("NewViolations", "Email", null)).thenReturn(Lists.newArrayList("user1", "user3")); + when(propertiesDao.findUsersForNotification("NewViolations", "Twitter", 56L)).thenReturn(Lists.newArrayList("user2")); + when(propertiesDao.findUsersForNotification("NewViolations", "Twitter", null)).thenReturn(Lists.newArrayList("user3")); + when(propertiesDao.findUsersForNotification("NewAlerts", "Twitter", null)).thenReturn(Lists.newArrayList("user4")); + + SetMultimap<String, NotificationChannel> multiMap = manager.findSubscribedRecipientsForDispatcher(dispatcher, null); + assertThat(multiMap.entries()).hasSize(3); + + Map<String, Collection<NotificationChannel>> map = multiMap.asMap(); + assertThat(map.get("user1")).containsOnly(emailChannel); + assertThat(map.get("user3")).containsOnly(emailChannel, twitterChannel); + assertThat(map.get("user2")).isNull(); + assertThat(map.get("user4")).isNull(); } } diff --git a/sonar-core/src/test/java/org/sonar/core/properties/PropertiesDaoTest.java b/sonar-core/src/test/java/org/sonar/core/properties/PropertiesDaoTest.java index 876d5d33c73..242938e0e84 100644 --- a/sonar-core/src/test/java/org/sonar/core/properties/PropertiesDaoTest.java +++ b/sonar-core/src/test/java/org/sonar/core/properties/PropertiesDaoTest.java @@ -28,6 +28,7 @@ import org.sonar.core.persistence.AbstractDaoTestCase; import java.util.List; +import static org.fest.assertions.Assertions.assertThat; import static org.hamcrest.Matchers.hasItems; import static org.hamcrest.Matchers.is; import static org.junit.Assert.assertThat; @@ -53,6 +54,32 @@ public class PropertiesDaoTest extends AbstractDaoTestCase { } @Test + public void shouldFindUsersForNotification() { + setupData("shouldFindUsersForNotification"); + + List<String> users = dao.findUsersForNotification("NewViolations", "Email", null); + assertThat(users).hasSize(0); + + users = dao.findUsersForNotification("NewViolations", "Email", 78L); + assertThat(users).hasSize(0); + + users = dao.findUsersForNotification("NewViolations", "Email", 45L); + assertThat(users).hasSize(1); + assertThat(users).containsOnly("user2"); + + users = dao.findUsersForNotification("NewViolations", "Twitter", null); + assertThat(users).hasSize(1); + assertThat(users).containsOnly("user3"); + + users = dao.findUsersForNotification("NewViolations", "Twitter", 78L); + assertThat(users).hasSize(0); + + users = dao.findUsersForNotification("NewViolations", "Twitter", 56L); + assertThat(users).hasSize(2); + assertThat(users).containsOnly("user1", "user3"); + } + + @Test public void selectGlobalProperties() { setupData("selectGlobalProperties"); List<PropertyDto> properties = dao.selectGlobalProperties(); diff --git a/sonar-core/src/test/resources/org/sonar/core/notification/DefaultNotificationManagerTest/fixture.xml b/sonar-core/src/test/resources/org/sonar/core/notification/DefaultNotificationManagerTest/fixture.xml deleted file mode 100644 index acae954e294..00000000000 --- a/sonar-core/src/test/resources/org/sonar/core/notification/DefaultNotificationManagerTest/fixture.xml +++ /dev/null @@ -1,8 +0,0 @@ -<dataset> - - <users id="1" login="simon" /> - <users id="2" login="godin" /> - - <properties id="1" user_id="1" prop_key="notification.CommentOnReviewAssignedToMe.email" text_value="true" resource_id="[null]"/> - -</dataset>
\ No newline at end of file diff --git a/sonar-core/src/test/resources/org/sonar/core/properties/PropertiesDaoTest/shouldFindUsersForNotification.xml b/sonar-core/src/test/resources/org/sonar/core/properties/PropertiesDaoTest/shouldFindUsersForNotification.xml new file mode 100644 index 00000000000..e941d29dfca --- /dev/null +++ b/sonar-core/src/test/resources/org/sonar/core/properties/PropertiesDaoTest/shouldFindUsersForNotification.xml @@ -0,0 +1,43 @@ +<dataset> + + <properties + id="1" + prop_key="notification.NewViolations.Email" + text_value="true" + resource_id="45" + user_id="2"/> + + <properties + id="2" + prop_key="notification.NewViolations.Twitter" + text_value="true" + resource_id="[null]" + user_id="3"/> + + <properties + id="3" + prop_key="notification.NewViolations.Twitter" + text_value="true" + resource_id="56" + user_id="1"/> + + <properties + id="4" + prop_key="notification.NewViolations.Twitter" + text_value="true" + resource_id="56" + user_id="3"/> + + <users + id="1" + login="user1"/> + + <users + id="2" + login="user2"/> + + <users + id="3" + login="user3"/> + +</dataset> diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/notifications/NotificationManager.java b/sonar-plugin-api/src/main/java/org/sonar/api/notifications/NotificationManager.java index 10879bf19de..f03dd9b68e5 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/notifications/NotificationManager.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/notifications/NotificationManager.java @@ -19,9 +19,12 @@ */ package org.sonar.api.notifications; +import com.google.common.collect.SetMultimap; import org.sonar.api.BatchComponent; import org.sonar.api.ServerComponent; +import javax.annotation.Nullable; + /** * <p> * The notification manager receives notifications and is in charge of storing them so that they are processed by the notification service. @@ -42,4 +45,19 @@ public interface NotificationManager extends ServerComponent, BatchComponent { */ void scheduleForSending(Notification notification); + /** + * <p> + * Returns the list of users who subscribed to the given dispatcher, along with the notification channels (email, twitter, ...) that they choose + * for this dispatcher. + * </p> + * <p> + * The resource ID can be null in case of notifications that have nothing to do with a specific project (like system notifications). + * </p> + * + * @param dispatcher the dispatcher for which this list of users is requested + * @param resourceId the optional resource which is concerned by this request + * @return the list of user login along with the subscribed channels + */ + SetMultimap<String, NotificationChannel> findSubscribedRecipientsForDispatcher(NotificationDispatcher dispatcher, @Nullable Integer resourceId); + } diff --git a/sonar-server/src/main/java/org/sonar/server/notifications/NotificationService.java b/sonar-server/src/main/java/org/sonar/server/notifications/NotificationService.java index 867a197e54c..49b4e8237c0 100644 --- a/sonar-server/src/main/java/org/sonar/server/notifications/NotificationService.java +++ b/sonar-server/src/main/java/org/sonar/server/notifications/NotificationService.java @@ -65,7 +65,6 @@ public class NotificationService implements ServerComponent { private final long delayInSeconds; private final DefaultNotificationManager manager; private final NotificationDispatcher[] dispatchers; - private final NotificationChannel[] channels; private ScheduledExecutorService executorService; private boolean stopping = false; @@ -73,19 +72,18 @@ public class NotificationService implements ServerComponent { /** * Constructor for {@link NotificationService} */ - public NotificationService(Settings settings, DefaultNotificationManager manager, NotificationDispatcher[] dispatchers, NotificationChannel[] channels) { + public NotificationService(Settings settings, DefaultNotificationManager manager, NotificationDispatcher[] dispatchers) { delayInSeconds = settings.getLong(PROPERTY_DELAY); this.manager = manager; - this.channels = channels; this.dispatchers = dispatchers; } /** * Default constructor when no channels. */ - public NotificationService(Settings settings, DefaultNotificationManager manager, NotificationDispatcher[] dispatchers) { - this(settings, manager, dispatchers, new NotificationChannel[0]); - LOG.warn("There is no channels - all notifications will be ignored!"); + public NotificationService(Settings settings, DefaultNotificationManager manager) { + this(settings, manager, new NotificationDispatcher[0]); + LOG.warn("There is no dispatcher - all notifications will be ignored!"); } public void start() { @@ -171,7 +169,7 @@ public class NotificationService implements ServerComponent { } public List<NotificationChannel> getChannels() { - return Arrays.asList(channels); + return manager.getChannels(); } } diff --git a/sonar-server/src/test/java/org/sonar/server/notifications/NotificationServiceTest.java b/sonar-server/src/test/java/org/sonar/server/notifications/NotificationServiceTest.java index f866a8e81a8..d2eeb8cce86 100644 --- a/sonar-server/src/test/java/org/sonar/server/notifications/NotificationServiceTest.java +++ b/sonar-server/src/test/java/org/sonar/server/notifications/NotificationServiceTest.java @@ -19,6 +19,7 @@ */ package org.sonar.server.notifications; +import com.google.common.collect.Lists; import org.junit.Test; import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; @@ -62,6 +63,7 @@ public class NotificationServiceTest { when(commentOnReviewCreatedByMe.getKey()).thenReturn("comment on review created by me"); when(queueElement.getNotification()).thenReturn(notification); when(manager.getFromQueue()).thenReturn(queueElement).thenReturn(null); + when(manager.getChannels()).thenReturn(Lists.newArrayList(emailChannel, gtalkChannel)); Settings settings = new Settings().setProperty("sonar.notifications.delay", 1L); @@ -178,15 +180,20 @@ public class NotificationServiceTest { @Test public void shouldReturnDispatcherAndChannelListsUsedInWebapp() { - Settings settings = new Settings().setProperty("sonar.notifications.delay", 1L); - NotificationService service = new NotificationService(settings, manager, - new NotificationDispatcher[] {commentOnReviewAssignedToMe, commentOnReviewCreatedByMe}, - new NotificationChannel[] {emailChannel, gtalkChannel}); + setUpMocks(CREATOR_SIMON, ASSIGNEE_SIMON); assertThat(service.getChannels()).containsOnly(emailChannel, gtalkChannel); assertThat(service.getDispatchers()).containsOnly(commentOnReviewAssignedToMe, commentOnReviewCreatedByMe); } + @Test + public void shouldReturnNoDispatcher() { + Settings settings = new Settings().setProperty("sonar.notifications.delay", 1L); + + service = new NotificationService(settings, manager); + assertThat(service.getDispatchers()).hasSize(0); + } + private static Answer<Object> addUser(final String user, final NotificationChannel channel) { return addUser(user, new NotificationChannel[] {channel}); } |