From 310ed53e7f5abec4fffb4998879769c1d733ac96 Mon Sep 17 00:00:00 2001 From: =?utf8?q?S=C3=A9bastien=20Lesaint?= Date: Thu, 14 Sep 2017 14:35:34 +0200 Subject: [PATCH] SONAR-9771 add property email.fromName and use it in email notifications --- .../ComputeEngineContainerImplTest.java | 2 +- .../email/EmailNotificationChannel.java | 4 +- .../email/EmailNotificationChannelTest.java | 7 ++-- .../config/CorePropertyDefinitionsTest.java | 2 +- .../org/sonar/api/config/EmailSettings.java | 13 ++++++ .../sonar/api/config/EmailSettingsTest.java | 3 +- .../QualityGateNotificationTest.java | 12 ++++-- .../sonarqube/tests/settings/EmailsTest.java | 40 ++++++++++++++++--- 8 files changed, 65 insertions(+), 18 deletions(-) diff --git a/server/sonar-ce/src/test/java/org/sonar/ce/container/ComputeEngineContainerImplTest.java b/server/sonar-ce/src/test/java/org/sonar/ce/container/ComputeEngineContainerImplTest.java index 235bd4cc071..d0c8c0b478c 100644 --- a/server/sonar-ce/src/test/java/org/sonar/ce/container/ComputeEngineContainerImplTest.java +++ b/server/sonar-ce/src/test/java/org/sonar/ce/container/ComputeEngineContainerImplTest.java @@ -153,7 +153,7 @@ public class ComputeEngineContainerImplTest { + 26 // level 1 + 49 // content of DaoModule + 3 // content of EsSearchModule - + 64 // content of CorePropertyDefinitions + + 65 // content of CorePropertyDefinitions + 1 // StopFlagContainer ); assertThat( diff --git a/server/sonar-server/src/main/java/org/sonar/server/notification/email/EmailNotificationChannel.java b/server/sonar-server/src/main/java/org/sonar/server/notification/email/EmailNotificationChannel.java index 3275e258a99..c532511382c 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/notification/email/EmailNotificationChannel.java +++ b/server/sonar-server/src/main/java/org/sonar/server/notification/email/EmailNotificationChannel.java @@ -79,7 +79,6 @@ public class EmailNotificationChannel extends NotificationChannel { */ private static final String REFERENCES_HEADER = "References"; - private static final String FROM_NAME_DEFAULT = "SonarQube"; private static final String SUBJECT_DEFAULT = "Notification"; private EmailSettings configuration; @@ -163,7 +162,8 @@ public class EmailNotificationChannel extends NotificationChannel { } // Set general information email.setCharset("UTF-8"); - String from = StringUtils.isBlank(emailMessage.getFrom()) ? FROM_NAME_DEFAULT : (emailMessage.getFrom() + " (SonarQube)"); + String fromName = configuration.getFromName(); + String from = StringUtils.isBlank(emailMessage.getFrom()) ? fromName : (emailMessage.getFrom() + " (" + fromName + ")"); email.setFrom(configuration.getFrom(), from); email.addTo(emailMessage.getTo(), " "); String subject = StringUtils.defaultIfBlank(StringUtils.trimToEmpty(configuration.getPrefix()) + " ", "") diff --git a/server/sonar-server/src/test/java/org/sonar/server/notification/email/EmailNotificationChannelTest.java b/server/sonar-server/src/test/java/org/sonar/server/notification/email/EmailNotificationChannelTest.java index d64a2a6d2f5..8a370c9681b 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/notification/email/EmailNotificationChannelTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/notification/email/EmailNotificationChannelTest.java @@ -70,7 +70,7 @@ public class EmailNotificationChannelTest { MimeMessage email = messages.get(0).getMimeMessage(); assertThat(email.getHeader("Content-Type", null)).isEqualTo("text/plain; charset=UTF-8"); - assertThat(email.getHeader("From", ",")).isEqualTo("SonarQube "); + assertThat(email.getHeader("From", ",")).isEqualTo("SonarQube from NoWhere "); assertThat(email.getHeader("To", null)).isEqualTo(""); assertThat(email.getHeader("Subject", null)).isEqualTo("[SONARQUBE] Test Message from SonarQube"); assertThat((String) email.getContent()).startsWith("This is a test message from SonarQube."); @@ -123,7 +123,7 @@ public class EmailNotificationChannelTest { assertThat(email.getHeader("List-ID", null)).isEqualTo("SonarQube "); assertThat(email.getHeader("List-Archive", null)).isEqualTo("http://nemo.sonarsource.org"); - assertThat(email.getHeader("From", ",")).isEqualTo("\"Full Username (SonarQube)\" "); + assertThat(email.getHeader("From", ",")).isEqualTo("\"Full Username (SonarQube from NoWhere)\" "); assertThat(email.getHeader("To", null)).isEqualTo(""); assertThat(email.getHeader("Subject", null)).isEqualTo("[SONARQUBE] Review #3"); assertThat((String) email.getContent()).startsWith("I'll take care of this violation."); @@ -151,7 +151,7 @@ public class EmailNotificationChannelTest { assertThat(email.getHeader("List-ID", null)).isEqualTo("SonarQube "); assertThat(email.getHeader("List-Archive", null)).isEqualTo("http://nemo.sonarsource.org"); - assertThat(email.getHeader("From", null)).isEqualTo("SonarQube "); + assertThat(email.getHeader("From", null)).isEqualTo("SonarQube from NoWhere "); assertThat(email.getHeader("To", null)).isEqualTo(""); assertThat(email.getHeader("Subject", null)).isEqualTo("[SONARQUBE] Foo"); assertThat((String) email.getContent()).startsWith("Bar"); @@ -189,6 +189,7 @@ public class EmailNotificationChannelTest { when(configuration.getSmtpHost()).thenReturn("localhost"); when(configuration.getSmtpPort()).thenReturn(smtpServer.getServer().getPort()); when(configuration.getFrom()).thenReturn("server@nowhere"); + when(configuration.getFromName()).thenReturn("SonarQube from NoWhere"); when(configuration.getPrefix()).thenReturn("[SONARQUBE]"); when(configuration.getServerBaseURL()).thenReturn("http://nemo.sonarsource.org"); } diff --git a/sonar-core/src/test/java/org/sonar/core/config/CorePropertyDefinitionsTest.java b/sonar-core/src/test/java/org/sonar/core/config/CorePropertyDefinitionsTest.java index 53e84aa8bbe..c26a8cda61b 100644 --- a/sonar-core/src/test/java/org/sonar/core/config/CorePropertyDefinitionsTest.java +++ b/sonar-core/src/test/java/org/sonar/core/config/CorePropertyDefinitionsTest.java @@ -33,7 +33,7 @@ public class CorePropertyDefinitionsTest { @Test public void all() { List defs = CorePropertyDefinitions.all(); - assertThat(defs).hasSize(64); + assertThat(defs).hasSize(65); } @Test diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/config/EmailSettings.java b/sonar-plugin-api/src/main/java/org/sonar/api/config/EmailSettings.java index f079a49dc67..db04d12b285 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/config/EmailSettings.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/config/EmailSettings.java @@ -50,6 +50,8 @@ public class EmailSettings { public static final String SMTP_PASSWORD_DEFAULT = ""; public static final String FROM = "email.from"; public static final String FROM_DEFAULT = "noreply@nowhere"; + public static final String FROM_NAME = "email.fromName"; + public static final String FROM_NAME_DEFAULT = "SonarQube"; public static final String PREFIX = "email.prefix"; public static final String PREFIX_DEFAULT = "[SONARQUBE]"; @@ -83,6 +85,10 @@ public class EmailSettings { return get(FROM, FROM_DEFAULT); } + public String getFromName() { + return get(FROM_NAME, FROM_NAME_DEFAULT); + } + public String getPrefix() { return get(PREFIX, PREFIX_DEFAULT); } @@ -146,6 +152,13 @@ public class EmailSettings { .category(CATEGORY_GENERAL) .subCategory(SUBCATEGORY_EMAIL) .build(), + PropertyDefinition.builder(FROM_NAME) + .name("From name") + .description("Emails will come from this address name. For example - \"SonarQube\". Note that server may ignore this setting.") + .defaultValue(FROM_NAME_DEFAULT) + .category(CATEGORY_GENERAL) + .subCategory(SUBCATEGORY_EMAIL) + .build(), PropertyDefinition.builder(PREFIX) .name("Email prefix") .description("Prefix will be prepended to all outgoing email subjects.") diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/config/EmailSettingsTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/config/EmailSettingsTest.java index 0ce63c67563..b569fd1db2a 100644 --- a/sonar-plugin-api/src/test/java/org/sonar/api/config/EmailSettingsTest.java +++ b/sonar-plugin-api/src/test/java/org/sonar/api/config/EmailSettingsTest.java @@ -37,12 +37,13 @@ public class EmailSettingsTest { assertThat(underTest.getSmtpPassword()).isEmpty(); assertThat(underTest.getSecureConnection()).isEmpty(); assertThat(underTest.getFrom()).isEqualTo("noreply@nowhere"); + assertThat(underTest.getFromName()).isEqualTo("SonarQube"); assertThat(underTest.getPrefix()).isEqualTo("[SONARQUBE]"); assertThat(underTest.getServerBaseURL()).isEqualTo(CoreProperties.SERVER_BASE_URL_DEFAULT_VALUE); } @Test public void return_definitions() { - assertThat(EmailSettings.definitions()).hasSize(7); + assertThat(EmailSettings.definitions()).hasSize(8); } } diff --git a/tests/src/test/java/org/sonarqube/tests/qualityGate/QualityGateNotificationTest.java b/tests/src/test/java/org/sonarqube/tests/qualityGate/QualityGateNotificationTest.java index 48b27237c12..d946a40e5f2 100644 --- a/tests/src/test/java/org/sonarqube/tests/qualityGate/QualityGateNotificationTest.java +++ b/tests/src/test/java/org/sonarqube/tests/qualityGate/QualityGateNotificationTest.java @@ -86,7 +86,8 @@ public class QualityGateNotificationTest { // Create quality gate with conditions on variations WsQualityGates.CreateWsResponse simple = tester.qGates().generate(); - tester.qGates().service().createCondition(CreateConditionRequest.builder().setQualityGateId(simple.getId()).setMetricKey("ncloc").setPeriod(1).setOperator("EQ").setWarning("0").build()); + tester.qGates().service() + .createCondition(CreateConditionRequest.builder().setQualityGateId(simple.getId()).setMetricKey("ncloc").setPeriod(1).setOperator("EQ").setWarning("0").build()); Project project = tester.projects().generate(null); tester.qGates().associateProject(simple, project); @@ -108,9 +109,12 @@ public class QualityGateNotificationTest { assertThat(emails.hasNext()).isTrue(); message = emails.next().getMimeMessage(); assertThat(message.getHeader("To", null)).isEqualTo(""); - assertThat((String) message.getContent()).contains("Quality gate status: Orange (was Green)"); - assertThat((String) message.getContent()).contains("Quality gate threshold: Lines of Code variation = 0 since previous analysis"); - assertThat((String) message.getContent()).contains("/dashboard?id=" + project.getKey()); + assertThat((String) message.getContent()) + .contains("Project: Sample") + .contains("Version: 1.0-SNAPSHOT") + .contains("Quality gate status: Orange (was Green)") + .contains("Quality gate threshold: Lines of Code variation = 0 since previous analysis") + .contains("/dashboard?id=" + project.getKey()); assertThat(emails.hasNext()).isFalse(); } diff --git a/tests/src/test/java/org/sonarqube/tests/settings/EmailsTest.java b/tests/src/test/java/org/sonarqube/tests/settings/EmailsTest.java index 50b895fbae2..8ce6439178b 100644 --- a/tests/src/test/java/org/sonarqube/tests/settings/EmailsTest.java +++ b/tests/src/test/java/org/sonarqube/tests/settings/EmailsTest.java @@ -20,8 +20,10 @@ package org.sonarqube.tests.settings; import com.sonar.orchestrator.Orchestrator; +import java.util.Arrays; import java.util.Iterator; import javax.annotation.Nullable; +import javax.mail.Address; import javax.mail.internet.MimeMessage; import org.junit.AfterClass; import org.junit.Before; @@ -37,7 +39,10 @@ import org.sonarqube.ws.client.setting.ValuesRequest; import org.subethamail.wiser.Wiser; import org.subethamail.wiser.WiserMessage; +import static java.lang.String.format; import static junit.framework.TestCase.fail; +import static org.apache.commons.lang.RandomStringUtils.randomAlphabetic; +import static org.apache.commons.lang.RandomStringUtils.randomAlphanumeric; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.groups.Tuple.tuple; @@ -72,11 +77,11 @@ public class EmailsTest { @Test public void update_email_settings() throws Exception { - updateEmailSettings("localhost", "42", "noreply@email.com", "[EMAIL]", "ssl", "john", "123456"); + updateEmailSettings("localhost", "42", "noreply@email.com", "The Devil", "[EMAIL]", "ssl", "john", "123456"); Settings.ValuesWsResponse response = tester.settings().service().values(ValuesRequest.builder() .setKeys("email.smtp_host.secured", "email.smtp_port.secured", "email.smtp_secure_connection.secured", "email.smtp_username.secured", "email.smtp_password.secured", - "email.from", "email.prefix") + "email.from", "email.fromName", "email.prefix") .build()); assertThat(response.getSettingsList()).extracting(Settings.Setting::getKey, Settings.Setting::getValue) @@ -87,12 +92,13 @@ public class EmailsTest { tuple("email.smtp_username.secured", "john"), tuple("email.smtp_password.secured", "123456"), tuple("email.from", "noreply@email.com"), + tuple("email.fromName", "The Devil"), tuple("email.prefix", "[EMAIL]")); } @Test public void send_test_email() throws Exception { - updateEmailSettings("localhost", Integer.toString(SMTP_SERVER.getServer().getPort()), null, null, null, null, null); + updateEmailSettings("localhost", Integer.toString(SMTP_SERVER.getServer().getPort()), null, null, null, null, null, null); sendEmail("test@example.org", "Test Message from SonarQube", "This is a test message from SonarQube"); @@ -100,8 +106,29 @@ public class EmailsTest { waitUntilAllNotificationsAreDelivered(1); Iterator emails = SMTP_SERVER.getMessages().iterator(); MimeMessage message = emails.next().getMimeMessage(); + assertThat(Arrays.stream(message.getFrom()).map(Address::toString)).containsOnly("SonarQube "); assertThat(message.getHeader("To", null)).isEqualTo(""); - assertThat(message.getSubject()).contains("Test Message from SonarQube"); + assertThat(message.getSubject()).isEqualTo("[SONARQUBE] Test Message from SonarQube"); + assertThat((String) message.getContent()).contains("This is a test message from SonarQube"); + assertThat(emails.hasNext()).isFalse(); + } + + @Test + public void send_customized_test_email() throws Exception { + String from = randomAlphanumeric(4) + "@" + randomAlphabetic(5); + String fromName = randomAlphanumeric(5); + String prefix = randomAlphanumeric(6); + updateEmailSettings("localhost", Integer.toString(SMTP_SERVER.getServer().getPort()), from, fromName, prefix, null, null, null); + + sendEmail("test@example.org", "Test Message from SonarQube", "This is a test message from SonarQube"); + + // We need to wait until all notifications will be delivered + waitUntilAllNotificationsAreDelivered(1); + Iterator emails = SMTP_SERVER.getMessages().iterator(); + MimeMessage message = emails.next().getMimeMessage(); + assertThat(Arrays.stream(message.getFrom()).map(Address::toString)).containsOnly(format("%s <%s>", fromName, from)); + assertThat(message.getHeader("To", null)).isEqualTo(""); + assertThat(message.getSubject()).isEqualTo(prefix + " Test Message from SonarQube"); assertThat((String) message.getContent()).contains("This is a test message from SonarQube"); assertThat(emails.hasNext()).isFalse(); } @@ -113,10 +140,10 @@ public class EmailsTest { } Thread.sleep(1_000); } - fail(String.format("Received %d emails, expected %d", SMTP_SERVER.getMessages().size(), expectedNumberOfEmails)); + fail(format("Received %d emails, expected %d", SMTP_SERVER.getMessages().size(), expectedNumberOfEmails)); } - private void updateEmailSettings(@Nullable String host, @Nullable String port, @Nullable String from, @Nullable String prefix, @Nullable String secure, + private void updateEmailSettings(@Nullable String host, @Nullable String port, @Nullable String from, @Nullable String fromName, @Nullable String prefix, @Nullable String secure, @Nullable String username, @Nullable String password) { tester.settings().setGlobalSettings( "email.smtp_host.secured", host, @@ -125,6 +152,7 @@ public class EmailsTest { "email.smtp_username.secured", username, "email.smtp_password.secured", password, "email.from", from, + "email.fromName", fromName, "email.prefix", prefix); } -- 2.39.5