Merge pull request #249 from basil/JENKINS-63846

[JENKINS-63846] From address can't be changed

src/main/java/hudson/plugins/emailext/ExtendedEmailPublisherDescriptor.java

@@ -192,6 +192,22 @@ private Object readResolve(){
         if(smtpAuthUsername != null) mailAccount.setSmtpUsername(smtpAuthUsername);
         if(smtpAuthPassword != null) mailAccount.setSmtpPassword(smtpAuthPassword);
         if(useSsl) mailAccount.setUseSsl(useSsl);
+
+        /*
+         * Versions 2.71 and earlier correctly left the address unset for the default account,
+         * relying solely on the system admin email address from the Jenkins Location settings for
+         * the default account and using the address specified on the account only for additional
+         * accounts. Versions 2.72 through 2.77 incorrectly set the address for the default account
+         * to the system admin email address from the Jenkins Location settings at the time the
+         * descriptor was first saved without propagating further changes from the Jenkins Location
+         * settings to the default account. To clear up this bad state, we unconditionally clear the
+         * address and rely once again solely on the system admin email address from the Jenkins
+         * Location settings for the default account.
+         */
+        if (mailAccount.getAddress() != null) {
+            mailAccount.setAddress(null);
+        }
+
         return this;
     }
 
@@ -207,7 +223,6 @@ public ExtendedEmailPublisherDescriptor() {
 
         if(mailAccount == null) {
             mailAccount = new MailAccount();
-            mailAccount.setAddress(getAdminAddress());
         }
 
         mailAccount.setDefaultAccount(true);
@@ -255,7 +270,7 @@ public String getAdminAddress() {
         JenkinsLocationConfiguration config = JenkinsLocationConfiguration.get();
         if(config != null) {
             if(StringUtils.isBlank(mailAccount.getAddress())) {
-                mailAccount.setAddress(config.getAdminAddress());
+                return config.getAdminAddress();
             }
         }
         return mailAccount.getAddress();
@@ -524,7 +539,6 @@ public MailAccount getMailAccount() {
     @DataBoundSetter
     public void setMailAccount(MailAccount mailAccount) {
         this.mailAccount = mailAccount;
-        this.mailAccount.setAddress(getAdminAddress());
         this.mailAccount.setDefaultAccount(true);
     }
 

src/main/java/hudson/plugins/emailext/MailAccount.java

@@ -42,7 +42,7 @@ public MailAccount() {
     }
 
     public boolean isValid() {
-        return StringUtils.isNotBlank(address) && StringUtils.isNotBlank(smtpHost) && (!isAuth() || (StringUtils.isNotBlank(smtpUsername) && smtpPassword != null));
+        return (isDefaultAccount() || StringUtils.isNotBlank(address)) && StringUtils.isNotBlank(smtpHost) && (!isAuth() || (StringUtils.isNotBlank(smtpUsername) && smtpPassword != null));
     }
 
     public boolean isDefaultAccount() {

src/test/java/hudson/plugins/emailext/ExtendedEmailPublisherDescriptorTest.java

@@ -510,7 +510,7 @@ public void testFixEmptyAndTrimEmptyString() throws Exception {
         descriptor.setEmergencyReroute("");
         j.submit(j.createWebClient().goTo("configure").getFormByName("config"));
 
-        assertEquals("address not configured yet <nobody@nowhere>",descriptor.getMailAccount().getAddress());
+        assertNull(descriptor.getMailAccount().getAddress());
         assertNull(descriptor.getMailAccount().getSmtpHost());
         assertEquals("25",descriptor.getMailAccount().getSmtpPort());
         assertNull(descriptor.getMailAccount().getSmtpUsername());
@@ -534,7 +534,7 @@ public void testFixEmptyAndTrimNull() throws Exception {
         descriptor.setEmergencyReroute(null);
         j.submit(j.createWebClient().goTo("configure").getFormByName("config"));
 
-        assertEquals("address not configured yet <nobody@nowhere>",descriptor.getMailAccount().getAddress());
+        assertNull(descriptor.getMailAccount().getAddress());
         assertNull(descriptor.getMailAccount().getSmtpHost());
         assertEquals("25",descriptor.getMailAccount().getSmtpPort());
         assertNull(descriptor.getMailAccount().getSmtpUsername());
@@ -877,8 +877,8 @@ public void persistedConfigurationBeforeDefaultAddress() throws Exception {
         */
         ExtendedEmailPublisherDescriptor descriptor =
                 j.jenkins.getDescriptorByType(ExtendedEmailPublisherDescriptor.class);
-        // TODO: Fails pending the fix for JENKINS-63846
-        // assertEquals("admin@example.com", descriptor.getAdminAddress());
+        assertEquals("admin@example.com", descriptor.getAdminAddress());
+        assertNull(descriptor.getMailAccount().getAddress());
         assertEquals("smtp.example.com", descriptor.getMailAccount().getSmtpHost());
         assertEquals("@example.com", descriptor.getDefaultSuffix());
         assertEquals("admin", descriptor.getMailAccount().getSmtpUsername());

src/test/java/hudson/plugins/emailext/ExtendedEmailPublisherTest.java

@@ -32,6 +32,7 @@
 import hudson.tasks.Builder;
 import hudson.tasks.Mailer;
 import jenkins.model.Jenkins;
+import jenkins.model.JenkinsLocationConfiguration;
 import net.sf.json.JSONObject;
 import org.apache.commons.io.IOUtils;
 import org.jenkinsci.plugins.scriptsecurity.scripts.ScriptApproval;
@@ -55,6 +56,7 @@
 import javax.mail.Address;
 import javax.mail.BodyPart;
 import javax.mail.Message;
+import javax.mail.MessagingException;
 import javax.mail.Session;
 import javax.mail.internet.MimeBodyPart;
 import javax.mail.internet.MimeMessage;
@@ -876,11 +878,8 @@ private void verifyPostsendScriptModifiesMessageId() throws Exception {
         assertEquals(2, mailbox.size());
 
         Message msg = mailbox.get(1);
-        String[] headers = msg.getHeader("In-Reply-To");
-        assertNotNull(headers);
-        assertEquals(1, headers.length);
 
-        assertEquals("<12345@xxx.com>", headers[0]);
+        assertEquals("<12345@xxx.com>", getHeader(msg, "In-Reply-To"));
     }
 
     @Test
@@ -1306,6 +1305,40 @@ public void testScriptConstructorsAreNotExecutedOutsideOfSandbox() throws Except
         assertNull(j.jenkins.getItem("should-not-exist2"));
     }
 
+    @Issue("JENKINS-63846")
+    @Test
+    public void testSystemAdminEmailChange() throws Exception {
+        SuccessTrigger successTrigger =
+                new SuccessTrigger(
+                        recProviders,
+                        "$DEFAULT_RECIPIENTS",
+                        "$DEFAULT_REPLYTO",
+                        "$DEFAULT_SUBJECT",
+                        "$DEFAULT_CONTENT",
+                        "",
+                        0,
+                        "project");
+        addEmailType(successTrigger);
+        publisher.getConfiguredTriggers().add(successTrigger);
+
+        JenkinsLocationConfiguration locationConfiguration = JenkinsLocationConfiguration.get();
+        locationConfiguration.setAdminAddress("Foo <foo@example.com>");
+        FreeStyleBuild build = j.buildAndAssertSuccess(project);
+        j.assertLogContains("Email was triggered for: Success", build);
+
+        locationConfiguration.setAdminAddress("Bar <bar@example.com>");
+        build = j.buildAndAssertSuccess(project);
+        j.assertLogContains("Email was triggered for: Success", build);
+
+        Mailbox mailbox = Mailbox.get("ashlux@gmail.com");
+        assertEquals(2, mailbox.size());
+
+        Message message = mailbox.get(0);
+        assertEquals("Foo <foo@example.com>", getHeader(message, "From"));
+        message = mailbox.get(1);
+        assertEquals("Bar <bar@example.com>", getHeader(message, "From"));
+    }
+
     /**
      * Similar to {@link SleepBuilder} but only on the first build. (Removing
      * the builder between builds is tricky since you would have to wait for the
@@ -1339,4 +1372,11 @@ private void addEmailType(EmailTrigger trigger) {
             }
         });
     }
+
+    private static String getHeader(Message message, String headerName) throws MessagingException {
+        String[] headers = message.getHeader(headerName);
+        assertNotNull(headers);
+        assertEquals(1, headers.length);
+        return headers[0];
+    }
 }