From 957e62ce77e23d31d59f9369657e91a9dd616c84 Mon Sep 17 00:00:00 2001 From: Evgeny Mandrikov Date: Thu, 21 Jul 2011 11:45:41 +0400 Subject: [PATCH] SONAR-2607 Provide email notifications on review changes * Add email templates. * Add server component - UserFinder. --- .../plugins/email/EmailConfiguration.java | 5 + .../email/EmailNotificationChannel.java | 89 +++-- .../org/sonar/plugins/email/EmailPlugin.java | 12 +- ....java => ChangesInReviewAssignedToMe.java} | 34 +- ...e.java => ChangesInReviewCreatedByMe.java} | 13 +- .../email/reviews/ReviewEmailTemplate.java | 117 +++++++ .../email/EmailNotificationChannelTest.java | 14 +- .../ChangesInReviewAssignedToMeTest.java} | 45 ++- .../ChangesInReviewCreatedByMeTest.java} | 40 ++- .../reviews/ReviewEmailTemplateTest.java | 327 ++++++++++++++++++ .../core/components/DefaultUserFinder.java | 43 +++ .../jpa/entity/NotificationQueueElement.java | 7 +- .../sonar/api/notifications/Notification.java | 4 +- .../org/sonar/api/security/UserFinder.java | 17 +- .../notifications/NotificationService.java | 6 +- .../reviews/ReviewsNotificationManager.java | 66 ++-- .../org/sonar/server/platform/Platform.java | 2 + .../main/webapp/WEB-INF/app/models/review.rb | 65 ++-- .../ReviewsNotificationManagerTest.java | 42 +-- .../server/notifications/reviews/fixture.xml | 15 - 20 files changed, 768 insertions(+), 195 deletions(-) rename plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/reviews/{CommentOnReviewEmailTemplate.java => ChangesInReviewAssignedToMe.java} (52%) rename plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/reviews/{CommentOnReviewCreatedByMe.java => ChangesInReviewCreatedByMe.java} (68%) create mode 100644 plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/reviews/ReviewEmailTemplate.java rename plugins/sonar-email-plugin/src/test/java/org/sonar/plugins/email/{review/CommentOnReviewAssignedToMeTest.java => reviews/ChangesInReviewAssignedToMeTest.java} (50%) rename plugins/sonar-email-plugin/src/test/java/org/sonar/plugins/email/{review/CommentOnReviewCreatedByMeTest.java => reviews/ChangesInReviewCreatedByMeTest.java} (54%) create mode 100644 plugins/sonar-email-plugin/src/test/java/org/sonar/plugins/email/reviews/ReviewEmailTemplateTest.java create mode 100644 sonar-core/src/main/java/org/sonar/core/components/DefaultUserFinder.java rename plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/reviews/CommentOnReviewAssignedToMe.java => sonar-plugin-api/src/main/java/org/sonar/api/security/UserFinder.java (62%) delete mode 100644 sonar-server/src/test/resources/org/sonar/server/notifications/reviews/fixture.xml diff --git a/plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/EmailConfiguration.java b/plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/EmailConfiguration.java index a2c5a458169..63b5f43cf19 100644 --- a/plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/EmailConfiguration.java +++ b/plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/EmailConfiguration.java @@ -20,6 +20,7 @@ package org.sonar.plugins.email; import org.apache.commons.configuration.Configuration; +import org.sonar.api.CoreProperties; import org.sonar.api.ServerExtension; /** @@ -78,4 +79,8 @@ public class EmailConfiguration implements ServerExtension { return configuration.getString(PREFIX, PREFIX_DEFAULT); } + public String getServerBaseURL() { + return configuration.getString(CoreProperties.SERVER_BASE_URL, CoreProperties.SERVER_BASE_URL_DEFAULT_VALUE); + } + } diff --git a/plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/EmailNotificationChannel.java b/plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/EmailNotificationChannel.java index cb31280efdb..4e5c2eed82f 100644 --- a/plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/EmailNotificationChannel.java +++ b/plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/EmailNotificationChannel.java @@ -19,16 +19,18 @@ */ package org.sonar.plugins.email; +import java.net.MalformedURLException; +import java.net.URL; + import org.apache.commons.lang.StringUtils; import org.apache.commons.mail.EmailException; import org.apache.commons.mail.SimpleEmail; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.sonar.api.database.DatabaseSession; import org.sonar.api.database.model.User; -import org.sonar.api.notifications.NotificationChannel; import org.sonar.api.notifications.Notification; -import org.sonar.jpa.session.DatabaseSessionFactory; +import org.sonar.api.notifications.NotificationChannel; +import org.sonar.api.security.UserFinder; import org.sonar.plugins.email.api.EmailMessage; import org.sonar.plugins.email.api.EmailTemplate; @@ -52,28 +54,53 @@ public class EmailNotificationChannel extends NotificationChannel { */ private static final int SOCKET_TIMEOUT = 30000; + /** + * Email Header Field: "List-ID". + * Value of this field should contain mailing list identifier as specified in RFC 2919. + */ + private static String LIST_ID_HEADER = "List-ID"; + + /** + * Email Header Field: "List-Archive". + * Value of this field should contain URL of mailing list archive as specified in RFC 2369. + */ + private static String LIST_ARCHIVE_HEADER = "List-Archive"; + + /** + * Email Header Field: "In-Reply-To". + * Value of this field should contain related message identifier as specified in RFC 2822. + */ + private static String IN_REPLY_TO_HEADER = "In-Reply-To"; + + /** + * Email Header Field: "References". + * Value of this field should contain related message identifier as specified in RFC 2822 + */ + private static String REFERENCES_HEADER = "References"; + private static final String FROM_NAME_DEFAULT = "Sonar"; private static final String SUBJECT_DEFAULT = "Notification"; private EmailConfiguration configuration; private EmailTemplate[] templates; - private DatabaseSessionFactory sessionFactory; + private UserFinder userFinder; - public EmailNotificationChannel(EmailConfiguration configuration, EmailTemplate[] templates, DatabaseSessionFactory sessionFactory) { + public EmailNotificationChannel(EmailConfiguration configuration, EmailTemplate[] templates, UserFinder userFinder) { this.configuration = configuration; this.templates = templates; - this.sessionFactory = sessionFactory; - } - - private User getUserByLogin(String login) { - DatabaseSession session = sessionFactory.getSession(); - return session.getSingleResult(User.class, "login", login); + this.userFinder = userFinder; } @Override public void deliver(Notification notification, String username) { + User user = userFinder.findByLogin(username); + if (StringUtils.isBlank(user.getEmail())) { + LOG.warn("Email not defined for user: " + username); + return; + } EmailMessage emailMessage = format(notification, username); if (emailMessage != null) { + emailMessage.setTo(user.getEmail()); deliver(emailMessage); } } @@ -82,11 +109,10 @@ public class EmailNotificationChannel extends NotificationChannel { for (EmailTemplate template : templates) { EmailMessage email = template.format(notification); if (email != null) { - User user = getUserByLogin(username); - email.setTo(user.getEmail()); return email; } } + LOG.warn("Email template not found for notification: {}", notification); return null; } @@ -107,30 +133,33 @@ public class EmailNotificationChannel extends NotificationChannel { private void send(EmailMessage emailMessage) throws EmailException { LOG.info("Sending email: {}", emailMessage); - // TODO - String domain = "nemo.sonarsource.org"; - String listId = ""; - String serverUrl = "http://nemo.sonarsource.org"; + String host = null; + try { + host = new URL(configuration.getServerBaseURL()).getHost(); + } catch (MalformedURLException e) { + // ignore + } SimpleEmail email = new SimpleEmail(); - /* - * Set headers for proper threading: - * GMail will not group messages, even if they have same subject, but don't have "In-Reply-To" and "References" headers. - * TODO investigate threading in other clients like KMail, Thunderbird, Outlook - */ - if (StringUtils.isNotEmpty(emailMessage.getMessageId())) { - String messageId = "<" + emailMessage.getMessageId() + "@" + domain + ">"; - email.addHeader("In-Reply-To", messageId); - email.addHeader("References", messageId); + if (StringUtils.isNotBlank(host)) { + /* + * Set headers for proper threading: GMail will not group messages, even if they have same subject, but don't have "In-Reply-To" and + * "References" headers. TODO investigate threading in other clients like KMail, Thunderbird, Outlook + */ + if (StringUtils.isNotEmpty(emailMessage.getMessageId())) { + String messageId = "<" + emailMessage.getMessageId() + "@" + host + ">"; + email.addHeader(IN_REPLY_TO_HEADER, messageId); + email.addHeader(REFERENCES_HEADER, messageId); + } + // Set headers for proper filtering + email.addHeader(LIST_ID_HEADER, "Sonar "); + email.addHeader(LIST_ARCHIVE_HEADER, configuration.getServerBaseURL()); } - // Set headers for proper filtering - email.addHeader("List-Id", listId); - email.addHeader("List-Archive", serverUrl); // Set general information email.setFrom(configuration.getFrom(), StringUtils.defaultIfBlank(emailMessage.getFrom(), FROM_NAME_DEFAULT)); email.addTo(emailMessage.getTo(), " "); String subject = StringUtils.defaultIfBlank(StringUtils.trimToEmpty(configuration.getPrefix()) + " ", "") - + StringUtils.defaultString(emailMessage.getSubject(), SUBJECT_DEFAULT); + + StringUtils.defaultString(emailMessage.getSubject(), SUBJECT_DEFAULT); email.setSubject(subject); email.setMsg(emailMessage.getMessage()); // Send diff --git a/plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/EmailPlugin.java b/plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/EmailPlugin.java index 4da910bb9cc..edee6ea8044 100644 --- a/plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/EmailPlugin.java +++ b/plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/EmailPlugin.java @@ -20,9 +20,9 @@ package org.sonar.plugins.email; import org.sonar.api.SonarPlugin; -import org.sonar.plugins.email.reviews.CommentOnReviewAssignedToMe; -import org.sonar.plugins.email.reviews.CommentOnReviewCreatedByMe; -import org.sonar.plugins.email.reviews.CommentOnReviewEmailTemplate; +import org.sonar.plugins.email.reviews.ChangesInReviewAssignedToMe; +import org.sonar.plugins.email.reviews.ChangesInReviewCreatedByMe; +import org.sonar.plugins.email.reviews.ReviewEmailTemplate; import java.util.Arrays; import java.util.List; @@ -34,9 +34,9 @@ public class EmailPlugin extends SonarPlugin { EmailConfiguration.class, EmailNotificationChannel.class, - CommentOnReviewEmailTemplate.class, - CommentOnReviewCreatedByMe.class, - CommentOnReviewAssignedToMe.class); + ReviewEmailTemplate.class, + ChangesInReviewAssignedToMe.class, + ChangesInReviewCreatedByMe.class); } } diff --git a/plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/reviews/CommentOnReviewEmailTemplate.java b/plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/reviews/ChangesInReviewAssignedToMe.java similarity index 52% rename from plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/reviews/CommentOnReviewEmailTemplate.java rename to plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/reviews/ChangesInReviewAssignedToMe.java index 4f2d4c8ed6b..fc56412f31b 100644 --- a/plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/reviews/CommentOnReviewEmailTemplate.java +++ b/plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/reviews/ChangesInReviewAssignedToMe.java @@ -19,26 +19,30 @@ */ package org.sonar.plugins.email.reviews; +import org.apache.commons.lang.StringUtils; import org.sonar.api.notifications.Notification; -import org.sonar.plugins.email.api.EmailMessage; -import org.sonar.plugins.email.api.EmailTemplate; +import org.sonar.api.notifications.NotificationDispatcher; -public class CommentOnReviewEmailTemplate extends EmailTemplate { +/** + * This dispatcher means: "notify me when when someone changes review assigned to me". + * + * @since 2.10 + */ +public class ChangesInReviewAssignedToMe extends NotificationDispatcher { @Override - public EmailMessage format(Notification notification) { - if ("review".equals(notification.getType())) { - String reviewId = notification.getFieldValue("reviewId"); - String author = notification.getFieldValue("author"); - String comment = notification.getFieldValue("comment"); - EmailMessage email = new EmailMessage() - .setFrom(author) - .setMessageId("review/" + reviewId) - .setSubject("Review #" + reviewId) - .setMessage(comment); - return email; + public void dispatch(Notification notification, Context context) { + if (StringUtils.startsWith(notification.getType(), "review")) { + String author = notification.getFieldValue("author"); // author of change + String oldAssignee = notification.getFieldValue("old.assignee"); // previous assignee + String assignee = notification.getFieldValue("assignee"); // current assignee + if (!StringUtils.equals(author, oldAssignee)) { + context.addUser(oldAssignee); + } + if (!StringUtils.equals(author, assignee)) { + context.addUser(assignee); + } } - return null; } } diff --git a/plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/reviews/CommentOnReviewCreatedByMe.java b/plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/reviews/ChangesInReviewCreatedByMe.java similarity index 68% rename from plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/reviews/CommentOnReviewCreatedByMe.java rename to plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/reviews/ChangesInReviewCreatedByMe.java index c91ea5f49be..30c99df3ad2 100644 --- a/plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/reviews/CommentOnReviewCreatedByMe.java +++ b/plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/reviews/ChangesInReviewCreatedByMe.java @@ -19,20 +19,25 @@ */ package org.sonar.plugins.email.reviews; +import org.apache.commons.lang.StringUtils; import org.sonar.api.notifications.Notification; import org.sonar.api.notifications.NotificationDispatcher; /** - * This dispatcher means: "notify me when when someone comments on review created by me". + * This dispatcher means: "notify me when when someone changes review created by me". * * @since 2.10 */ -public class CommentOnReviewCreatedByMe extends NotificationDispatcher { +public class ChangesInReviewCreatedByMe extends NotificationDispatcher { @Override public void dispatch(Notification notification, Context context) { - if ("review".equals(notification.getType())) { - context.addUser(notification.getFieldValue("creator")); + if (StringUtils.startsWith(notification.getType(), "review")) { + String author = notification.getFieldValue("author"); // author of change + String creator = notification.getFieldValue("creator"); // creator of review + if (!StringUtils.equals(author, creator)) { + context.addUser(creator); + } } } diff --git a/plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/reviews/ReviewEmailTemplate.java b/plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/reviews/ReviewEmailTemplate.java new file mode 100644 index 00000000000..b91d00a90cf --- /dev/null +++ b/plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/reviews/ReviewEmailTemplate.java @@ -0,0 +1,117 @@ +/* + * Sonar, open source software quality management tool. + * Copyright (C) 2008-2011 SonarSource + * mailto:contact AT sonarsource DOT com + * + * Sonar is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * Sonar is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with Sonar; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02 + */ +package org.sonar.plugins.email.reviews; + +import org.apache.commons.lang.StringUtils; +import org.sonar.api.database.model.User; +import org.sonar.api.notifications.Notification; +import org.sonar.api.security.UserFinder; +import org.sonar.plugins.email.EmailConfiguration; +import org.sonar.plugins.email.api.EmailMessage; +import org.sonar.plugins.email.api.EmailTemplate; + +/** + * Creates email message for notification "review-changed". + * + * @since 2.10 + */ +public class ReviewEmailTemplate extends EmailTemplate { + + private EmailConfiguration configuration; + private UserFinder userFinder; + + public ReviewEmailTemplate(EmailConfiguration configuration, UserFinder userFinder) { + this.configuration = configuration; + this.userFinder = userFinder; + } + + @Override + public EmailMessage format(Notification notification) { + if ( !"review-changed".equals(notification.getType())) { + return null; + } + String reviewId = notification.getFieldValue("reviewId"); + String author = notification.getFieldValue("author"); + StringBuilder sb = new StringBuilder(); + + append(sb, "Status", notification.getFieldValue("old.status"), notification.getFieldValue("new.status")); + append(sb, "Resolution", notification.getFieldValue("old.resolution"), notification.getFieldValue("new.resolution")); + append(sb, "Assignee", getUserFullName(notification.getFieldValue("old.assignee")), getUserFullName(notification.getFieldValue("new.assignee"))); + appendComment(sb, notification); + appendFooter(sb, notification); + + EmailMessage message = new EmailMessage() + .setMessageId("review/" + reviewId) + .setSubject("Review #" + reviewId) + .setMessage(sb.toString()); + if (author != null) { + message.setFrom(getUserFullName(author)); + } + return message; + } + + private void append(StringBuilder sb, String name, String oldValue, String newValue) { + if (oldValue != null || newValue != null) { + sb.append(name).append(": "); + if (newValue != null) { + sb.append(newValue); + } + if (oldValue != null) { + sb.append(" (was ").append(oldValue).append(")"); + } + sb.append('\n'); + } + } + + private void appendComment(StringBuilder sb, Notification notification) { + String newComment = notification.getFieldValue("new.comment"); + String oldComment = notification.getFieldValue("old.comment"); + + if (newComment != null) { // comment was added or modified + sb.append("Comment:\n ").append(newComment).append('\n'); + if (oldComment != null) { // comment was modified + sb.append("Was:\n ").append(oldComment).append('\n'); + } + } else if (oldComment != null) { // comment was deleted + sb.append("Comment deleted, was:\n ").append(oldComment).append('\n'); + } + } + + private void appendFooter(StringBuilder sb, Notification notification) { + String reviewId = notification.getFieldValue("reviewId"); + sb.append("\n--\n") + .append("See it in Sonar: ").append(configuration.getServerBaseURL()).append("/review/").append(reviewId).append('\n'); + } + + /** + * Visibility has been relaxed for tests. + */ + String getUserFullName(String login) { + if (login == null) { + return null; + } + User user = userFinder.findByLogin(login); + if (user == null) { // most probably user was deleted + return login; + } + return StringUtils.defaultIfBlank(user.getName(), login); + } + +} diff --git a/plugins/sonar-email-plugin/src/test/java/org/sonar/plugins/email/EmailNotificationChannelTest.java b/plugins/sonar-email-plugin/src/test/java/org/sonar/plugins/email/EmailNotificationChannelTest.java index a33b50e43ec..1508dc676f8 100644 --- a/plugins/sonar-email-plugin/src/test/java/org/sonar/plugins/email/EmailNotificationChannelTest.java +++ b/plugins/sonar-email-plugin/src/test/java/org/sonar/plugins/email/EmailNotificationChannelTest.java @@ -25,8 +25,9 @@ import static org.junit.Assert.assertThat; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; -import com.dumbster.smtp.SimpleSmtpServer; -import com.dumbster.smtp.SmtpMessage; +import java.io.IOException; +import java.net.ServerSocket; + import org.apache.commons.mail.EmailException; import org.junit.After; import org.junit.Before; @@ -34,8 +35,8 @@ import org.junit.BeforeClass; import org.junit.Test; import org.sonar.plugins.email.api.EmailMessage; -import java.io.IOException; -import java.net.ServerSocket; +import com.dumbster.smtp.SimpleSmtpServer; +import com.dumbster.smtp.SmtpMessage; public class EmailNotificationChannelTest { @@ -125,7 +126,7 @@ public class EmailNotificationChannelTest { assertThat(email.getHeaderValue("In-Reply-To"), is("")); assertThat(email.getHeaderValue("References"), is("")); - assertThat(email.getHeaderValue("List-Id"), is("")); + assertThat(email.getHeaderValue("List-ID"), is("Sonar ")); assertThat(email.getHeaderValue("List-Archive"), is("http://nemo.sonarsource.org")); assertThat(email.getHeaderValue("From"), is("Full Username ")); @@ -149,7 +150,7 @@ public class EmailNotificationChannelTest { assertThat(email.getHeaderValue("In-Reply-To"), nullValue()); assertThat(email.getHeaderValue("References"), nullValue()); - assertThat(email.getHeaderValue("List-Id"), is("")); + assertThat(email.getHeaderValue("List-ID"), is("Sonar ")); assertThat(email.getHeaderValue("List-Archive"), is("http://nemo.sonarsource.org")); assertThat(email.getHeaderValue("From"), is("Sonar ")); @@ -175,6 +176,7 @@ public class EmailNotificationChannelTest { when(configuration.getSmtpPort()).thenReturn(Integer.toString(port)); when(configuration.getFrom()).thenReturn("server@nowhere"); when(configuration.getPrefix()).thenReturn("[SONAR]"); + when(configuration.getServerBaseURL()).thenReturn("http://nemo.sonarsource.org"); } } diff --git a/plugins/sonar-email-plugin/src/test/java/org/sonar/plugins/email/review/CommentOnReviewAssignedToMeTest.java b/plugins/sonar-email-plugin/src/test/java/org/sonar/plugins/email/reviews/ChangesInReviewAssignedToMeTest.java similarity index 50% rename from plugins/sonar-email-plugin/src/test/java/org/sonar/plugins/email/review/CommentOnReviewAssignedToMeTest.java rename to plugins/sonar-email-plugin/src/test/java/org/sonar/plugins/email/reviews/ChangesInReviewAssignedToMeTest.java index 2c31c1f98bd..d32af196241 100644 --- a/plugins/sonar-email-plugin/src/test/java/org/sonar/plugins/email/review/CommentOnReviewAssignedToMeTest.java +++ b/plugins/sonar-email-plugin/src/test/java/org/sonar/plugins/email/reviews/ChangesInReviewAssignedToMeTest.java @@ -17,42 +17,53 @@ * License along with Sonar; if not, write to the Free Software * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02 */ -package org.sonar.plugins.email.review; +package org.sonar.plugins.email.reviews; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; import org.junit.Before; import org.junit.Test; +import org.sonar.api.notifications.Notification; import org.sonar.api.notifications.NotificationDispatcher; -import org.sonar.plugins.email.reviews.CommentOnReviewAssignedToMe; -public class CommentOnReviewAssignedToMeTest { // FIXME implement me +public class ChangesInReviewAssignedToMeTest { private NotificationDispatcher.Context context; - private CommentOnReviewAssignedToMe dispatcher; + private ChangesInReviewAssignedToMe dispatcher; @Before public void setUp() { context = mock(NotificationDispatcher.Context.class); - dispatcher = new CommentOnReviewAssignedToMe(); + dispatcher = new ChangesInReviewAssignedToMe(); } @Test - public void shouldDispatchToAssignee() { - // CommentOnReviewNotification notification = new CommentOnReviewNotification(new Review().setAssigneeId(1), new User(), "comment"); - // dispatcher.dispatch(notification, context); - // verify(context).addUser(1); - // - // notification = new CommentOnReviewNotification(new Review().setAssigneeId(2), new User(), "comment"); - // dispatcher.dispatch(notification, context); - // verify(context).addUser(2); + public void dispatchToOldAndNewAssignee() { + Notification notification = new Notification("review-assignee-changed") + .setFieldValue("author", "freddy") + .setFieldValue("old.assignee", "godin") + .setFieldValue("assignee", "simon"); + + dispatcher.dispatch(notification, context); + + verify(context).addUser("godin"); + verify(context).addUser("simon"); + verifyNoMoreInteractions(context); } @Test - public void shouldNotDispatchWhenNotAssigned() { - // CommentOnReviewNotification notification = new CommentOnReviewNotification(new Review(), new User(), "comment"); - // dispatcher.dispatch(notification, context); - // verifyNoMoreInteractions(context); + public void doNotDispatchToAuthorOfChanges() { + Notification notification = new Notification("review-assignee-changed") + .setFieldValue("author", "simon") + .setFieldValue("old.assignee", "simon") + .setFieldValue("assignee", "godin"); + + dispatcher.dispatch(notification, context); + + verify(context).addUser("godin"); + verifyNoMoreInteractions(context); } } diff --git a/plugins/sonar-email-plugin/src/test/java/org/sonar/plugins/email/review/CommentOnReviewCreatedByMeTest.java b/plugins/sonar-email-plugin/src/test/java/org/sonar/plugins/email/reviews/ChangesInReviewCreatedByMeTest.java similarity index 54% rename from plugins/sonar-email-plugin/src/test/java/org/sonar/plugins/email/review/CommentOnReviewCreatedByMeTest.java rename to plugins/sonar-email-plugin/src/test/java/org/sonar/plugins/email/reviews/ChangesInReviewCreatedByMeTest.java index 2d345126418..6b0ea61fcda 100644 --- a/plugins/sonar-email-plugin/src/test/java/org/sonar/plugins/email/review/CommentOnReviewCreatedByMeTest.java +++ b/plugins/sonar-email-plugin/src/test/java/org/sonar/plugins/email/reviews/ChangesInReviewCreatedByMeTest.java @@ -17,35 +17,49 @@ * License along with Sonar; if not, write to the Free Software * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02 */ -package org.sonar.plugins.email.review; +package org.sonar.plugins.email.reviews; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; import org.junit.Before; import org.junit.Test; +import org.sonar.api.notifications.Notification; import org.sonar.api.notifications.NotificationDispatcher; -import org.sonar.plugins.email.reviews.CommentOnReviewCreatedByMe; -public class CommentOnReviewCreatedByMeTest { // FIXME implement me +public class ChangesInReviewCreatedByMeTest { private NotificationDispatcher.Context context; - private CommentOnReviewCreatedByMe dispatcher; + private ChangesInReviewCreatedByMe dispatcher; @Before public void setUp() { context = mock(NotificationDispatcher.Context.class); - dispatcher = new CommentOnReviewCreatedByMe(); + dispatcher = new ChangesInReviewCreatedByMe(); } @Test - public void shouldDispatchToCreator() { - // CommentOnReviewNotification notification = new CommentOnReviewNotification(new Review().setUserId(1), new User(), "comment"); - // dispatcher.dispatch(notification, context); - // verify(context).addUser(1); - // - // notification = new CommentOnReviewNotification(new Review().setUserId(2), new User(), "comment"); - // dispatcher.dispatch(notification, context); - // verify(context).addUser(2); + public void dispatchToCreator() { + Notification notification = new Notification("review-comment-added") + .setFieldValue("author", "godin") + .setFieldValue("creator", "simon"); + + dispatcher.dispatch(notification, context); + + verify(context).addUser("simon"); + verifyNoMoreInteractions(context); + } + + @Test + public void doNotDispatchToAuthorOfChanges() { + Notification notification = new Notification("review-comment-added") + .setFieldValue("author", "simon") + .setFieldValue("creator", "simon"); + + dispatcher.dispatch(notification, context); + + verifyNoMoreInteractions(context); } } diff --git a/plugins/sonar-email-plugin/src/test/java/org/sonar/plugins/email/reviews/ReviewEmailTemplateTest.java b/plugins/sonar-email-plugin/src/test/java/org/sonar/plugins/email/reviews/ReviewEmailTemplateTest.java new file mode 100644 index 00000000000..3b65b418f60 --- /dev/null +++ b/plugins/sonar-email-plugin/src/test/java/org/sonar/plugins/email/reviews/ReviewEmailTemplateTest.java @@ -0,0 +1,327 @@ +/* + * Sonar, open source software quality management tool. + * Copyright (C) 2008-2011 SonarSource + * mailto:contact AT sonarsource DOT com + * + * Sonar is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * Sonar is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with Sonar; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02 + */ +package org.sonar.plugins.email.reviews; + +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.nullValue; +import static org.junit.Assert.assertThat; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import org.junit.Before; +import org.junit.Test; +import org.sonar.api.database.model.User; +import org.sonar.api.notifications.Notification; +import org.sonar.api.security.UserFinder; +import org.sonar.plugins.email.EmailConfiguration; +import org.sonar.plugins.email.api.EmailMessage; + +public class ReviewEmailTemplateTest { + + private ReviewEmailTemplate template; + + @Before + public void setUp() { + EmailConfiguration configuration = mock(EmailConfiguration.class); + when(configuration.getServerBaseURL()).thenReturn("http://nemo.sonarsource.org"); + UserFinder userFinder = mock(UserFinder.class); + when(userFinder.findByLogin(eq("freddy.mallet"))).thenReturn(new User().setName("Freddy Mallet")); + when(userFinder.findByLogin(eq("simon.brandhof"))).thenReturn(new User().setName("Simon Brandhof")); + when(userFinder.findByLogin(eq("evgeny.mandrikov"))).thenReturn(new User().setName("Evgeny Mandrikov")); + template = new ReviewEmailTemplate(configuration, userFinder); + } + + /** + * 
+   * Subject: Review #1
+   * From: Freddy Mallet
+   * 
+   * Comment:
+   *   This is my first comment
+   * 
+   * --
+   * See it in Sonar: http://nemo.sonarsource.org/review/1
+   *
+ */ + @Test + public void shouldFormatCommentAdded() { + Notification notification = new Notification("review-changed") + .setFieldValue("reviewId", "1") + .setFieldValue("author", "freddy.mallet") + .setFieldValue("old.comment", null) + .setFieldValue("new.comment", "This is my first comment"); + EmailMessage message = template.format(notification); + assertThat(message.getMessageId(), is("review/1")); + assertThat(message.getSubject(), is("Review #1")); + assertThat(message.getFrom(), is("Freddy Mallet")); + assertThat(message.getMessage(), is("Comment:\n This is my first comment\n\n--\nSee it in Sonar: http://nemo.sonarsource.org/review/1\n")); + } + + /** + * 
+   * Subject: Review #1
+   * From: Freddy Mallet
+   * 
+   * Comment:
+   *   This is another comment
+   * Was:
+   *   This is my first comment
+   * 
+   * --
+   * See it in Sonar: http://nemo.sonarsource.org/review/1
+   *
+ */ + @Test + public void shouldFormatCommentEdited() { + Notification notification = new Notification("review-changed") + .setFieldValue("reviewId", "1") + .setFieldValue("author", "freddy.mallet") + .setFieldValue("old.comment", "This is my first comment") + .setFieldValue("new.comment", "This is another comment"); + EmailMessage message = template.format(notification); + assertThat(message.getMessageId(), is("review/1")); + assertThat(message.getSubject(), is("Review #1")); + assertThat(message.getFrom(), is("Freddy Mallet")); + assertThat(message.getMessage(), is("Comment:\n This is another comment\nWas:\n This is my first comment\n\n--\nSee it in Sonar: http://nemo.sonarsource.org/review/1\n")); + } + + /** + * 
+   * Subject: Review #1
+   * From: Freddy Mallet
+   * 
+   * Comment deleted, was:
+   *   This is deleted comment
+   *   
+   * --
+   * See it in Sonar: http://nemo.sonarsource.org/review/1
+   *
+ */ + @Test + public void shouldFormatCommentDeleted() { + Notification notification = new Notification("review-changed") + .setFieldValue("reviewId", "1") + .setFieldValue("old.comment", "This is deleted comment") + .setFieldValue("new.comment", null) + .setFieldValue("author", "freddy.mallet"); + EmailMessage message = template.format(notification); + assertThat(message.getMessageId(), is("review/1")); + assertThat(message.getSubject(), is("Review #1")); + assertThat(message.getFrom(), is("Freddy Mallet")); + assertThat(message.getMessage(), is("Comment deleted, was:\n This is deleted comment\n\n--\nSee it in Sonar: http://nemo.sonarsource.org/review/1\n")); + } + + /** + * 
+   * Subject: Review #1
+   * From: Freddy Mallet
+   * 
+   * Assignee: Evgeny Mandrikov
+   * 
+   * --
+   * See it in Sonar: http://nemo.sonarsource.org/review/1
+   *
+ */ + @Test + public void shouldFormatAssigneed() { + Notification notification = new Notification("review-changed") + .setFieldValue("reviewId", "1") + .setFieldValue("author", "freddy.mallet") + .setFieldValue("old.assignee", null) + .setFieldValue("new.assignee", "evgeny.mandrikov"); + EmailMessage message = template.format(notification); + assertThat(message.getMessageId(), is("review/1")); + assertThat(message.getSubject(), is("Review #1")); + assertThat(message.getFrom(), is("Freddy Mallet")); + assertThat(message.getMessage(), is("Assignee: Evgeny Mandrikov\n\n--\nSee it in Sonar: http://nemo.sonarsource.org/review/1\n")); + } + + /** + * 
+   * Subject: Review #1
+   * From: Freddy Mallet
+   * 
+   * Assignee: Simon Brandhof (was Evgeny Mandrikov)
+   * 
+   * --
+   * See it in Sonar: http://nemo.sonarsource.org/review/1
+   *
+ */ + @Test + public void shouldFormatAssigneedToAnotherPerson() { + Notification notification = new Notification("review-changed") + .setFieldValue("reviewId", "1") + .setFieldValue("author", "freddy.mallet") + .setFieldValue("old.assignee", "evgeny.mandrikov") + .setFieldValue("new.assignee", "simon.brandhof"); + EmailMessage message = template.format(notification); + assertThat(message.getMessageId(), is("review/1")); + assertThat(message.getSubject(), is("Review #1")); + assertThat(message.getFrom(), is("Freddy Mallet")); + assertThat(message.getMessage(), is("Assignee: Simon Brandhof (was Evgeny Mandrikov)\n\n--\nSee it in Sonar: http://nemo.sonarsource.org/review/1\n")); + } + + /** + * 
+   * Subject: Review #1
+   * From: Freddy Mallet
+   * 
+   * Assignee: (was Simon Brandhof)
+   * 
+   * --
+   * See it in Sonar: http://nemo.sonarsource.org/review/1
+   *
+ */ + @Test + public void shouldFormatUnassigned() { + Notification notification = new Notification("review-changed") + .setFieldValue("reviewId", "1") + .setFieldValue("author", "freddy.mallet") + .setFieldValue("old.assignee", "simon.brandhof") + .setFieldValue("new.assignee", null); + EmailMessage message = template.format(notification); + assertThat(message.getMessageId(), is("review/1")); + assertThat(message.getSubject(), is("Review #1")); + assertThat(message.getFrom(), is("Freddy Mallet")); + assertThat(message.getMessage(), is("Assignee: (was Simon Brandhof)\n\n--\nSee it in Sonar: http://nemo.sonarsource.org/review/1\n")); + } + + /** + * 
+   * Subject: Review #1
+   * From: Sonar
+   * 
+   * Status: CLOSED (was OPEN)
+   * 
+   * --
+   * See it in Sonar: http://nemo.sonarsource.org/review/1
+   *
+ */ + @Test + public void shouldFormatClosed() { + Notification notification = new Notification("review-changed") + .setFieldValue("reviewId", "1") + .setFieldValue("old.status", "OPEN") + .setFieldValue("new.status", "CLOSED"); + EmailMessage message = template.format(notification); + assertThat(message.getMessageId(), is("review/1")); + assertThat(message.getSubject(), is("Review #1")); + assertThat(message.getFrom(), nullValue()); + assertThat(message.getMessage(), is("Status: CLOSED (was OPEN)\n\n--\nSee it in Sonar: http://nemo.sonarsource.org/review/1\n")); + } + + /** + * 
+   * Subject: Review #1
+   * From: Simon Brandhof
+   * 
+   * Status: REOPENED (was RESOLVED)
+   * Resolution: (was FIXED)
+   * 
+   * --
+   * See it in Sonar: http://nemo.sonarsource.org/review/1
+   *
+ */ + @Test + public void shouldFormatReopened() { + Notification notification = new Notification("review-changed") + .setFieldValue("reviewId", "1") + .setFieldValue("old.resolution", "FIXED") + .setFieldValue("new.resolution", null) + .setFieldValue("old.status", "RESOLVED") + .setFieldValue("new.status", "REOPENED"); + EmailMessage message = template.format(notification); + assertThat(message.getMessageId(), is("review/1")); + assertThat(message.getSubject(), is("Review #1")); + assertThat(message.getFrom(), nullValue()); + assertThat(message.getMessage(), is("Status: REOPENED (was RESOLVED)\nResolution: (was FIXED)\n\n--\nSee it in Sonar: http://nemo.sonarsource.org/review/1\n")); + } + + /** + * 
+   * Subject: Review #1
+   * From: Simon Brandhof
+   * 
+   * Status: RESOLVED (was OPEN)
+   * Resolution: FIXED
+   * 
+   * --
+   * See it in Sonar: http://nemo.sonarsource.org/review/1
+   *
+ */ + @Test + public void shouldFormatResolvedAsFixed() { + Notification notification = new Notification("review-changed") + .setFieldValue("reviewId", "1") + .setFieldValue("author", "simon.brandhof") + .setFieldValue("old.status", "OPEN") + .setFieldValue("old.resolution", null) + .setFieldValue("new.status", "RESOLVED") + .setFieldValue("new.resolution", "FIXED"); + EmailMessage message = template.format(notification); + assertThat(message.getMessageId(), is("review/1")); + assertThat(message.getSubject(), is("Review #1")); + assertThat(message.getFrom(), is("Simon Brandhof")); + assertThat(message.getMessage(), is("Status: RESOLVED (was OPEN)\nResolution: FIXED\n\n--\nSee it in Sonar: http://nemo.sonarsource.org/review/1\n")); + } + + /** + * 
+   * Subject: Review #1
+   * From: Simon Brandhof
+   * 
+   * Status: RESOLVED (was REOPENED)
+   * Resolution: FALSE-POSITIVE
+   * 
+   * --
+   * See it in Sonar: http://nemo.sonarsource.org/review/1
+   *
+ */ + @Test + public void shouldFormatResolvedAsFalsePositive() { + Notification notification = new Notification("review-changed") + .setFieldValue("reviewId", "1") + .setFieldValue("author", "freddy.mallet") + .setFieldValue("old.status", "REOPENED") + .setFieldValue("old.resolution", null) + .setFieldValue("new.status", "RESOLVED") + .setFieldValue("new.resolution", "FALSE-POSITIVE"); + EmailMessage message = template.format(notification); + assertThat(message.getMessageId(), is("review/1")); + assertThat(message.getSubject(), is("Review #1")); + assertThat(message.getFrom(), is("Freddy Mallet")); + assertThat(message.getMessage(), is("Status: RESOLVED (was REOPENED)\nResolution: FALSE-POSITIVE\n\n--\nSee it in Sonar: http://nemo.sonarsource.org/review/1\n")); + } + + @Test + public void shouldNotFormat() { + Notification notification = new Notification("other"); + EmailMessage message = template.format(notification); + assertThat(message, nullValue()); + } + + @Test + public void shouldReturnFullNameOrLogin() { + assertThat(template.getUserFullName("freddy.mallet"), is("Freddy Mallet")); + assertThat(template.getUserFullName("deleted"), is("deleted")); + } + +} diff --git a/sonar-core/src/main/java/org/sonar/core/components/DefaultUserFinder.java b/sonar-core/src/main/java/org/sonar/core/components/DefaultUserFinder.java new file mode 100644 index 00000000000..648aa104fb5 --- /dev/null +++ b/sonar-core/src/main/java/org/sonar/core/components/DefaultUserFinder.java @@ -0,0 +1,43 @@ +/* + * Sonar, open source software quality management tool. + * Copyright (C) 2008-2011 SonarSource + * mailto:contact AT sonarsource DOT com + * + * Sonar is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * Sonar is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with Sonar; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02 + */ +package org.sonar.core.components; + +import org.sonar.api.database.DatabaseSession; +import org.sonar.api.database.model.User; +import org.sonar.api.security.UserFinder; +import org.sonar.jpa.session.DatabaseSessionFactory; + +/** + * @since 2.10 + */ +public class DefaultUserFinder implements UserFinder { + + private DatabaseSessionFactory sessionFactory; + + public DefaultUserFinder(DatabaseSessionFactory sessionFactory) { + this.sessionFactory = sessionFactory; + } + + public User findByLogin(String login) { + DatabaseSession session = sessionFactory.getSession(); + return session.getSingleResult(User.class, "login", login); + } + +} diff --git a/sonar-core/src/main/java/org/sonar/jpa/entity/NotificationQueueElement.java b/sonar-core/src/main/java/org/sonar/jpa/entity/NotificationQueueElement.java index 198cd0871ce..ac80cec95e6 100644 --- a/sonar-core/src/main/java/org/sonar/jpa/entity/NotificationQueueElement.java +++ b/sonar-core/src/main/java/org/sonar/jpa/entity/NotificationQueueElement.java @@ -20,6 +20,7 @@ package org.sonar.jpa.entity; import org.sonar.api.notifications.Notification; +import org.sonar.api.utils.SonarException; import java.io.*; import java.util.Date; @@ -65,7 +66,7 @@ public class NotificationQueueElement { objectOutputStream.close(); this.data = byteArrayOutputStream.toByteArray(); } catch (IOException e) { - throw new RuntimeException(e); + throw new SonarException(e); } } @@ -80,9 +81,9 @@ public class NotificationQueueElement { objectInputStream.close(); return (Notification) result; } catch (IOException e) { - throw new RuntimeException(e); + throw new SonarException(e); } catch (ClassNotFoundException e) { - throw new RuntimeException(e); + throw new SonarException(e); } } diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/notifications/Notification.java b/sonar-plugin-api/src/main/java/org/sonar/api/notifications/Notification.java index 8ec915cb0d0..5547acae2bc 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/notifications/Notification.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/notifications/Notification.java @@ -24,7 +24,7 @@ import org.apache.commons.lang.builder.ReflectionToStringBuilder; import org.apache.commons.lang.builder.ToStringStyle; import java.io.Serializable; -import java.util.Map; +import java.util.HashMap; /** * @since 2.10 @@ -33,7 +33,7 @@ public class Notification implements Serializable { private String type; - private Map fields = Maps.newHashMap(); + private HashMap fields = Maps.newHashMap(); // NOSONAR false-positive due to serialization : usage of HashMap instead of Map public Notification(String type) { this.type = type; diff --git a/plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/reviews/CommentOnReviewAssignedToMe.java b/sonar-plugin-api/src/main/java/org/sonar/api/security/UserFinder.java similarity index 62% rename from plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/reviews/CommentOnReviewAssignedToMe.java rename to sonar-plugin-api/src/main/java/org/sonar/api/security/UserFinder.java index 95051533e11..3f283ff504e 100644 --- a/plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/reviews/CommentOnReviewAssignedToMe.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/security/UserFinder.java @@ -17,23 +17,16 @@ * License along with Sonar; if not, write to the Free Software * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02 */ -package org.sonar.plugins.email.reviews; +package org.sonar.api.security; -import org.sonar.api.notifications.Notification; -import org.sonar.api.notifications.NotificationDispatcher; +import org.sonar.api.ServerComponent; +import org.sonar.api.database.model.User; /** - * This dispatcher means: "notify me when when someone comments on review assigned to me". - * * @since 2.10 */ -public class CommentOnReviewAssignedToMe extends NotificationDispatcher { +public interface UserFinder extends ServerComponent { - @Override - public void dispatch(Notification notification, Context context) { - if ("review".equals(notification.getType())) { - context.addUser(notification.getFieldValue("assignee")); - } - } + User findByLogin(String login); } 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 b88cf62146b..b408a05bfd7 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 @@ -136,9 +136,9 @@ public class NotificationService implements ServerComponent { } for (Map.Entry> entry : recipients.asMap().entrySet()) { String username = entry.getKey(); - Collection channels = entry.getValue(); - LOG.info("For user {} via {}", username, channels); - for (NotificationChannel channel : channels) { + Collection userChannels = entry.getValue(); + LOG.info("For user {} via {}", username, userChannels); + for (NotificationChannel channel : userChannels) { channel.deliver(notification, username); } } diff --git a/sonar-server/src/main/java/org/sonar/server/notifications/reviews/ReviewsNotificationManager.java b/sonar-server/src/main/java/org/sonar/server/notifications/reviews/ReviewsNotificationManager.java index e8200bd4b06..a89a5667acf 100644 --- a/sonar-server/src/main/java/org/sonar/server/notifications/reviews/ReviewsNotificationManager.java +++ b/sonar-server/src/main/java/org/sonar/server/notifications/reviews/ReviewsNotificationManager.java @@ -19,49 +19,69 @@ */ package org.sonar.server.notifications.reviews; +import java.util.Map; +import java.util.Set; + +import org.apache.commons.lang.StringUtils; import org.sonar.api.ServerComponent; -import org.sonar.api.database.model.Review; -import org.sonar.api.database.model.User; import org.sonar.api.notifications.Notification; import org.sonar.api.notifications.NotificationManager; -import org.sonar.jpa.session.DatabaseSessionFactory; + +import com.google.common.collect.Sets; /** * @since 2.10 */ public class ReviewsNotificationManager implements ServerComponent { - private DatabaseSessionFactory sessionFactory; private NotificationManager notificationManager; - public ReviewsNotificationManager(DatabaseSessionFactory sessionFactory, NotificationManager notificationManager) { - this.sessionFactory = sessionFactory; + public ReviewsNotificationManager(NotificationManager notificationManager) { this.notificationManager = notificationManager; } /** - * Visibility has been relaxed for tests. + * @param reviewId id of review, which was modified + * @param author author of change (username) + * @param creator author of review (username) + * @param assignee current assignee (username) + * @param oldComment old text of comment + * @param comment new text of comment */ - User getUserById(Integer id) { - return sessionFactory.getSession().getEntity(User.class, id); + public void notifyCommentChanged(Long reviewId, String author, String creator, String assignee, String oldComment, String newComment) { + Notification notification = new Notification("review-changed") + .setFieldValue("reviewId", String.valueOf(reviewId)) + .setFieldValue("author", author) + .setFieldValue("creator", creator) + .setFieldValue("assignee", assignee) + .setFieldValue("old.comment", oldComment) + .setFieldValue("new.comment", newComment); + notificationManager.scheduleForSending(notification); } /** - * Visibility has been relaxed for tests. + * @param reviewId reviewId id of review, which was modified + * @param author author of change (username) + * @param oldValues map of old values + * @param newValues map of new values */ - Review getReviewById(Long id) { - return sessionFactory.getSession().getEntity(Review.class, id); - } - - public void notifyCommentAdded(Long reviewId, Integer userId, String comment) { - Review review = getReviewById(reviewId); - User author = getUserById(userId); - - Notification notification = new Notification("review"); - notification // FIXME include info about review - .setFieldValue("author", author.getLogin()) - .setFieldValue("comment", comment); - + public void notifyChanged(Long reviewId, String author, Map oldValues, Map newValues) { + Notification notification = new Notification("review-changed") + .setFieldValue("reviewId", author) + .setFieldValue("author", author) + .setFieldValue("creator", newValues.get("creator")) + .setFieldValue("assignee", newValues.get("assignee")); + Set fields = Sets.newHashSet(); + fields.addAll(oldValues.keySet()); + fields.addAll(newValues.keySet()); + for (String field : fields) { + String oldValue = oldValues.get(field); + String newValue = newValues.get(field); + if ( !StringUtils.equals(oldValue, newValue)) { + notification.setFieldValue("new." + field, newValue); + notification.setFieldValue("old." + field, oldValue); + } + } notificationManager.scheduleForSending(notification); } diff --git a/sonar-server/src/main/java/org/sonar/server/platform/Platform.java b/sonar-server/src/main/java/org/sonar/server/platform/Platform.java index df234f0c490..7b5ef8fac7f 100644 --- a/sonar-server/src/main/java/org/sonar/server/platform/Platform.java +++ b/sonar-server/src/main/java/org/sonar/server/platform/Platform.java @@ -40,6 +40,7 @@ import org.sonar.api.utils.TimeProfiler; import org.sonar.core.components.DefaultMetricFinder; import org.sonar.core.components.DefaultModelFinder; import org.sonar.core.components.DefaultRuleFinder; +import org.sonar.core.components.DefaultUserFinder; import org.sonar.core.notifications.DefaultNotificationManager; import org.sonar.jpa.dao.DaoFacade; import org.sonar.jpa.dao.MeasuresDao; @@ -185,6 +186,7 @@ public final class Platform { servicesContainer.as(Characteristics.CACHE).addComponent(ProfilesConsole.class); servicesContainer.as(Characteristics.CACHE).addComponent(RulesConsole.class); servicesContainer.as(Characteristics.CACHE).addComponent(JRubyI18n.class); + servicesContainer.as(Characteristics.CACHE).addComponent(DefaultUserFinder.class); // Notifications servicesContainer.as(Characteristics.CACHE).addComponent(NotificationService.class); diff --git a/sonar-server/src/main/webapp/WEB-INF/app/models/review.rb b/sonar-server/src/main/webapp/WEB-INF/app/models/review.rb index 94df7aed6d6..d4fe68c3808 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/models/review.rb +++ b/sonar-server/src/main/webapp/WEB-INF/app/models/review.rb @@ -54,15 +54,14 @@ class Review < ActiveRecord::Base # REVIEW CORE METHODS # # - + # params are mandatory: # - :user # - :text def create_comment(params={}) comment = comments.create!(params) touch - - Java::OrgSonarServerUi::JRubyFacade.getInstance().getReviewsNotificationManager().notifyCommentAdded(id.to_i, comment.user.id.to_i, comment.text.to_java) + # TODO notification_manager.notifyCommentChanged(id.to_i, current_user.login.to_java, self.user.login.to_java, self.assignee.login.to_java, nil, comment.text.to_java) end def edit_comment(comment_id, comment_text) @@ -76,25 +75,61 @@ class Review < ActiveRecord::Base def edit_last_comment(comment_text) comment=comments.last + old_comment_text=comment.text comment.text=comment_text comment.save! touch + # TODO notification_manager.notifyCommentChanged(id.to_i, current_user.login.to_java, self.user.login.to_java, self.assignee.login.to_java, old_comment.to_java, comment.text.to_java) end - + def delete_comment(comment_id) comment=comments.find(comment_id) comments.pop if comment + old_comment_text=comment.text comment.delete touch + # TODO notification_manager.notifyCommentChanged(id.to_i, current_user.login.to_java, self.user.login.to_java, self.assignee.login.to_java, old_comment_text.to_java, nil) end end - + + def notification_manager + Java::OrgSonarServerUi::JRubyFacade.getInstance().getReviewsNotificationManager() + end + + def to_java_map() + map = java.util.HashMap.new({ + :creator => user.login.to_java, + :assignee => assignee == nil ? nil : assignee.login.to_java, + :status => status.to_java, + :resolution => resolution.to_java + }) + map + end + def reassign(user) + old = self.to_java_map self.assignee = user self.save! + # TODO notification_manager.notifyChanged(id.to_i, current_user.login.to_java, old, to_java_map) end - + + def reopen + old = self.to_java_map + self.status = STATUS_REOPENED + self.resolution = nil + self.save! + # TODO notification_manager.notifyChanged(id.to_i, current_user.login.to_java, old, to_java_map) + end + + def resolve + old = self.to_java_map + self.status = STATUS_RESOLVED + self.resolution = 'FIXED' + self.save! + # TODO notification_manager.notifyChanged(id.to_i, current_user.login.to_java, old, to_java_map) + end + # params are mandatory: # - :user (mandatory) # - :text (mandatory) @@ -112,13 +147,15 @@ class Review < ActiveRecord::Base end end create_comment(:user => params[:user], :text => params[:text]) + old = self.to_java_map self.assignee = nil self.status = is_false_positive ? STATUS_RESOLVED : STATUS_REOPENED self.resolution = is_false_positive ? 'FALSE-POSITIVE' : nil self.save! + # TODO notification_manager.notifyChanged(id.to_i, current_user.login.to_java, old, to_java_map) end end - + def false_positive resolution == 'FALSE-POSITIVE' end @@ -143,20 +180,6 @@ class Review < ActiveRecord::Base status == STATUS_OPEN end - def reopen - self.status = STATUS_REOPENED - self.resolution = nil - self.save! - end - - def resolve - self.status = STATUS_RESOLVED - self.resolution = 'FIXED' - self.save! - end - - - # # # SEARCH METHODS diff --git a/sonar-server/src/test/java/org/sonar/server/notifications/reviews/ReviewsNotificationManagerTest.java b/sonar-server/src/test/java/org/sonar/server/notifications/reviews/ReviewsNotificationManagerTest.java index 8e0eaba6953..187d3e965b3 100644 --- a/sonar-server/src/test/java/org/sonar/server/notifications/reviews/ReviewsNotificationManagerTest.java +++ b/sonar-server/src/test/java/org/sonar/server/notifications/reviews/ReviewsNotificationManagerTest.java @@ -19,39 +19,31 @@ */ package org.sonar.server.notifications.reviews; -import static org.hamcrest.Matchers.is; -import static org.junit.Assert.assertThat; +import static org.mockito.Matchers.any; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.mock; import org.junit.Before; -import org.junit.Test; -import org.sonar.api.database.model.Review; -import org.sonar.api.database.model.User; -import org.sonar.jpa.test.AbstractDbUnitTestCase; +import org.mockito.invocation.InvocationOnMock; +import org.mockito.stubbing.Answer; +import org.sonar.api.notifications.Notification; +import org.sonar.api.notifications.NotificationManager; -public class ReviewsNotificationManagerTest extends AbstractDbUnitTestCase { +public class ReviewsNotificationManagerTest { // FIXME implement + private Notification notification; private ReviewsNotificationManager manager; @Before public void setUp() { - setupData(getClass().getResourceAsStream("fixture.xml")); - manager = new ReviewsNotificationManager(getSessionFactory(), null); - } - - @Test - public void shouldGetReviewById() { - Review review = manager.getReviewById(3L); - assertThat(review.getUserId(), is(1)); - assertThat(review.getAssigneeId(), is(2)); - assertThat(review.getTitle(), is("Review #3")); - } - - @Test - public void shouldGetUserById() { - User user = manager.getUserById(1); - assertThat(user.getLogin(), is("simon")); - assertThat(user.getName(), is("Simon Brandhof")); - assertThat(user.getEmail(), is("simon.brandhof@sonarsource.com")); + NotificationManager delegate = mock(NotificationManager.class); + doAnswer(new Answer() { + public Object answer(InvocationOnMock invocation) throws Throwable { + notification = (Notification) invocation.getArguments()[0]; + return null; + } + }).when(delegate).scheduleForSending(any(Notification.class)); + manager = new ReviewsNotificationManager(delegate); } } diff --git a/sonar-server/src/test/resources/org/sonar/server/notifications/reviews/fixture.xml b/sonar-server/src/test/resources/org/sonar/server/notifications/reviews/fixture.xml deleted file mode 100644 index d9a64d90337..00000000000 --- a/sonar-server/src/test/resources/org/sonar/server/notifications/reviews/fixture.xml +++ /dev/null @@ -1,15 +0,0 @@ - - - - - - - - - - - - - - - \ No newline at end of file -- 2.39.5