Refactor outgoing email to be constructed by Guice

The outgoing email configuration is now stored in a singleton owned by
our Guice injector.  Unfortunately this is a fairly intrusive change
as a lot of code needs to obtain and pass through the new EmailSender
interface in order to deliver the message.

Signed-off-by: Shawn O. Pearce <sop@google.com>

src/main/java/com/google/gerrit/server/AccountSecurityImpl.java

@@ -30,6 +30,7 @@ import com.google.gerrit.client.rpc.InvalidSshKeyException;
 import com.google.gerrit.client.rpc.NoSuchEntityException;
 import com.google.gerrit.server.mail.EmailException;
 import com.google.gerrit.server.mail.RegisterNewEmailSender;
+import com.google.gerrit.server.mail.EmailSender;
 import com.google.gerrit.server.ssh.SshUtil;
 import com.google.gwt.user.client.rpc.AsyncCallback;
 import com.google.gwtjsonrpc.client.VoidResult;
@@ -64,13 +65,15 @@ class AccountSecurityImpl extends BaseServiceImplementation implements
   private final Logger log = LoggerFactory.getLogger(getClass());
   private final GerritServer server;
   private final ContactStore contactStore;
+  private final EmailSender emailSender;
 
   @Inject
   AccountSecurityImpl(final SchemaFactory<ReviewDb> sf, final GerritServer gs,
-      final ContactStore cs) {
+      final ContactStore cs, final EmailSender es) {
     super(sf);
     server = gs;
     contactStore = cs;
+    emailSender = es;
   }
 
   public void mySshKeys(final AsyncCallback<List<AccountSshKey>> callback) {
@@ -286,7 +289,7 @@ class AccountSecurityImpl extends BaseServiceImplementation implements
         GerritJsonServlet.getCurrentCall().getHttpServletRequest();
     try {
       final RegisterNewEmailSender sender;
-      sender = new RegisterNewEmailSender(server, address, req);
+      sender = new RegisterNewEmailSender(server, emailSender, address, req);
       sender.send();
       cb.onSuccess(VoidResult.INSTANCE);
     } catch (EmailException e) {

src/main/java/com/google/gerrit/server/GerritConfigProvider.java

@@ -20,6 +20,7 @@ import com.google.gerrit.client.data.GitwebLink;
 import com.google.gerrit.client.reviewdb.ApprovalCategory;
 import com.google.gerrit.client.reviewdb.ReviewDb;
 import com.google.gerrit.client.rpc.Common;
+import com.google.gerrit.server.mail.EmailSender;
 import com.google.gerrit.server.ssh.GerritSshDaemon;
 import com.google.gwtorm.client.OrmException;
 import com.google.gwtorm.client.SchemaFactory;
@@ -42,6 +43,7 @@ class GerritConfigProvider implements Provider<GerritConfig> {
   private final GerritServer server;
   private final SchemaFactory<ReviewDb> schema;
   private GerritSshDaemon sshd;
+  private EmailSender emailSender;
 
   @Inject
   GerritConfigProvider(final GerritServer gs, final SchemaFactory<ReviewDb> sf) {
@@ -54,6 +56,11 @@ class GerritConfigProvider implements Provider<GerritConfig> {
     sshd = d;
   }
 
+  @Inject(optional = true)
+  void setEmailSender(final EmailSender d) {
+    emailSender = d;
+  }
+
   private GerritConfig create() throws OrmException {
     final Config cfg = server.getGerritConfig();
     final GerritConfig config = new GerritConfig();
@@ -64,7 +71,8 @@ class GerritConfigProvider implements Provider<GerritConfig> {
     config.setUseRepoDownload(cfg.getBoolean("repo", null,
         "showdownloadcommand", false));
     config.setUseContactInfo(server.getContactStoreURL() != null);
-    config.setAllowRegisterNewEmail(server.isOutgoingMailEnabled());
+    config.setAllowRegisterNewEmail(emailSender != null
+        && emailSender.isEnabled());
     config.setLoginType(server.getLoginType());
 
     final String gitwebUrl = cfg.getString("gitweb", null, "url");

src/main/java/com/google/gerrit/server/GerritServer.java

@@ -23,7 +23,6 @@ import com.google.gerrit.client.reviewdb.SystemConfig.LoginType;
 import com.google.gerrit.client.rpc.Common;
 import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.config.SitePath;
-import com.google.gerrit.server.mail.EmailException;
 import com.google.gerrit.server.patch.DiffCacheEntryFactory;
 import com.google.gerrit.server.ssh.SshKeyCacheEntryFactory;
 import com.google.gwtjsonrpc.server.SignedToken;
@@ -41,9 +40,6 @@ import net.sf.ehcache.config.DiskStoreConfiguration;
 import net.sf.ehcache.constructs.blocking.SelfPopulatingCache;
 import net.sf.ehcache.store.MemoryStoreEvictionPolicy;
 
-import org.apache.commons.net.smtp.AuthSMTPClient;
-import org.apache.commons.net.smtp.SMTPClient;
-import org.apache.commons.net.smtp.SMTPReply;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.spearce.jgit.errors.RepositoryNotFoundException;
@@ -270,60 +266,6 @@ public class GerritServer {
     return r;
   }
 
-  public boolean isOutgoingMailEnabled() {
-    return getGerritConfig().getBoolean("sendemail", null, "enable", true);
-  }
-
-  public SMTPClient createOutgoingMail() throws EmailException {
-    if (!isOutgoingMailEnabled()) {
-      throw new EmailException("Sending email is disabled");
-    }
-
-    final Config cfg = getGerritConfig();
-    String smtpHost = cfg.getString("sendemail", null, "smtpserver");
-    if (smtpHost == null) {
-      smtpHost = "127.0.0.1";
-    }
-    int smtpPort = cfg.getInt("sendemail", null, "smtpserverport", 25);
-
-    String smtpUser = cfg.getString("sendemail", null, "smtpuser");
-    String smtpPass = cfg.getString("sendemail", null, "smtpuserpass");
-
-    final AuthSMTPClient client = new AuthSMTPClient("UTF-8");
-    client.setAllowRcpt(cfg.getStringList("sendemail", null, "allowrcpt"));
-    try {
-      client.connect(smtpHost, smtpPort);
-      if (!SMTPReply.isPositiveCompletion(client.getReplyCode())) {
-        throw new EmailException("SMTP server rejected connection");
-      }
-      if (!client.login()) {
-        String e = client.getReplyString();
-        throw new EmailException("SMTP server rejected login: " + e);
-      }
-      if (smtpUser != null && !client.auth(smtpUser, smtpPass)) {
-        String e = client.getReplyString();
-        throw new EmailException("SMTP server rejected auth: " + e);
-      }
-    } catch (IOException e) {
-      if (client.isConnected()) {
-        try {
-          client.disconnect();
-        } catch (IOException e2) {
-        }
-      }
-      throw new EmailException(e.getMessage(), e);
-    } catch (EmailException e) {
-      if (client.isConnected()) {
-        try {
-          client.disconnect();
-        } catch (IOException e2) {
-        }
-      }
-      throw e;
-    }
-    return client;
-  }
-
   /** Time (in seconds) that user sessions stay "signed in". */
   public int getSessionAge() {
     return sessionAge;

src/main/java/com/google/gerrit/server/GerritServerModule.java

@@ -27,6 +27,8 @@ import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.config.GerritServerConfigProvider;
 import com.google.gerrit.server.config.SitePath;
 import com.google.gerrit.server.config.SitePathProvider;
+import com.google.gerrit.server.mail.EmailSender;
+import com.google.gerrit.server.mail.SmtpEmailSender;
 import com.google.gwtorm.client.SchemaFactory;
 import com.google.gwtorm.jdbc.Database;
 import com.google.inject.AbstractModule;
@@ -65,6 +67,7 @@ public class GerritServerModule extends AbstractModule {
     bind(FileTypeRegistry.class).to(MimeUtilFileTypeRegistry.class);
     bind(ReplicationQueue.class).to(PushReplication.class).in(SINGLETON);
     bind(MergeQueue.class).to(ChangeMergeQueue.class).in(SINGLETON);
+    bind(EmailSender.class).to(SmtpEmailSender.class).in(SINGLETON);
     bind(GerritConfig.class).toProvider(GerritConfigProvider.class).in(
         SINGLETON);
   }

src/main/java/com/google/gerrit/server/mail/AbandonedSender.java

@@ -19,8 +19,8 @@ import com.google.gerrit.server.GerritServer;
 
 /** Send notice about a change being abandoned by its owner. */
 public class AbandonedSender extends ReplyToChangeSender {
-  public AbandonedSender(GerritServer gs, Change c) {
-    super(gs, c, "abandon");
+  public AbandonedSender(GerritServer gs, EmailSender sf, Change c) {
+    super(gs, sf, c, "abandon");
   }
 
   @Override

src/main/java/com/google/gerrit/server/mail/AddReviewerSender.java

@@ -19,8 +19,8 @@ import com.google.gerrit.server.GerritServer;
 
 /** Asks a user to review a change. */
 public class AddReviewerSender extends NewChangeSender {
-  public AddReviewerSender(GerritServer gs, Change c) {
-    super(gs, c);
+  public AddReviewerSender(GerritServer gs, EmailSender sf, Change c) {
+    super(gs, sf, c);
   }
 
   @Override

src/main/java/com/google/gerrit/server/mail/CommentSender.java

@@ -33,8 +33,8 @@ import java.util.Map;
 public class CommentSender extends ReplyToChangeSender {
   private List<PatchLineComment> inlineComments = Collections.emptyList();
 
-  public CommentSender(GerritServer gs, Change c) {
-    super(gs, c, "comment");
+  public CommentSender(GerritServer gs, EmailSender sf, Change c) {
+    super(gs, sf, c, "comment");
   }
 
   public void setPatchLineComments(final List<PatchLineComment> plc) {

src/main/java/com/google/gerrit/server/mail/CreateChangeSender.java

@@ -28,8 +28,8 @@ import java.util.Set;
 
 /** Notify interested parties of a brand new change. */
 public class CreateChangeSender extends NewChangeSender {
-  public CreateChangeSender(GerritServer gs, Change c) {
-    super(gs, c);
+  public CreateChangeSender(GerritServer gs, EmailSender sf, Change c) {
+    super(gs, sf, c);
   }
 
   @Override

src/main/java/com/google/gerrit/server/mail/EmailSender.java

@@ -0,0 +1,36 @@
+// Copyright (C) 2009 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.mail;
+
+import java.util.Collection;
+import java.util.Map;
+
+/** Sends email messages to third parties. */
+public interface EmailSender {
+  boolean isEnabled();
+
+  /**
+   * Sends an email message.
+   * 
+   * @param from who the message is from.
+   * @param rcpt one or more address where the message will be delivered to.
+   *        This list overrides any To or CC headers in {@code headers}.
+   * @param headers message headers.
+   * @param body text to appear in the body of the message.
+   * @throws EmailException the message cannot be sent.
+   */
+  void send(Address from, Collection<Address> rcpt,
+      Map<String, EmailHeader> headers, String body) throws EmailException;
+}

src/main/java/com/google/gerrit/server/mail/MergeFailSender.java

@@ -19,8 +19,8 @@ import com.google.gerrit.server.GerritServer;
 
 /** Send notice about a change failing to merged. */
 public class MergeFailSender extends ReplyToChangeSender {
-  public MergeFailSender(GerritServer gs, Change c) {
-    super(gs, c, "comment");
+  public MergeFailSender(GerritServer gs, EmailSender sf, Change c) {
+    super(gs, sf, c, "comment");
   }
 
   @Override

src/main/java/com/google/gerrit/server/mail/MergedSender.java

@@ -34,8 +34,8 @@ import java.util.Map;
 public class MergedSender extends ReplyToChangeSender {
   private Branch.NameKey dest;
 
-  public MergedSender(GerritServer gs, Change c) {
-    super(gs, c, "merged");
+  public MergedSender(GerritServer gs, EmailSender sf, Change c) {
+    super(gs, sf, c, "merged");
     dest = c.getDest();
   }
 

src/main/java/com/google/gerrit/server/mail/NewChangeSender.java

@@ -28,8 +28,8 @@ public abstract class NewChangeSender extends OutgoingEmail {
   private final Set<Account.Id> reviewers = new HashSet<Account.Id>();
   private final Set<Account.Id> extraCC = new HashSet<Account.Id>();
 
-  protected NewChangeSender(GerritServer gs, Change c) {
-    super(gs, c, "newchange");
+  protected NewChangeSender(GerritServer gs, EmailSender sf, Change c) {
+    super(gs, sf, c, "newchange");
   }
 
   public void addReviewers(final Collection<Account.Id> cc) {

src/main/java/com/google/gerrit/server/mail/OutgoingEmail.java

@@ -29,16 +29,12 @@ import com.google.gerrit.client.reviewdb.ReviewDb;
 import com.google.gerrit.client.reviewdb.StarredChange;
 import com.google.gerrit.client.reviewdb.UserIdentity;
 import com.google.gerrit.client.rpc.Common;
-import com.google.gerrit.pgm.Version;
 import com.google.gerrit.server.GerritServer;
 import com.google.gwtorm.client.OrmException;
 
-import org.apache.commons.net.smtp.SMTPClient;
 import org.spearce.jgit.lib.PersonIdent;
+import org.spearce.jgit.util.SystemReader;
 
-import java.io.BufferedWriter;
-import java.io.IOException;
-import java.io.Writer;
 import java.net.InetAddress;
 import java.net.MalformedURLException;
 import java.net.URL;
@@ -64,11 +60,12 @@ public abstract class OutgoingEmail {
   private static final Random RNG = new Random();
   private final String messageClass;
   protected final GerritServer server;
+  private final EmailSender emailSender;
   protected final Change change;
   protected final String projectName;
   private final HashSet<Account.Id> rcptTo = new HashSet<Account.Id>();
   private final Map<String, EmailHeader> headers;
-  private final List<String> smtpRcptTo = new ArrayList<String>();
+  private final List<Address> smtpRcptTo = new ArrayList<Address>();
   private Address smtpFromAddress;
   private StringBuilder body;
   private boolean inFooter;
@@ -80,8 +77,10 @@ public abstract class OutgoingEmail {
   protected ChangeMessage changeMessage;
   protected ReviewDb db;
 
-  protected OutgoingEmail(final GerritServer gs, final Change c, final String mc) {
+  protected OutgoingEmail(final GerritServer gs, final EmailSender es,
+      final Change c, final String mc) {
     server = gs;
+    emailSender = es;
     change = c;
     projectName = change != null ? change.getDest().getParentKey().get() : null;
     messageClass = mc;
@@ -115,7 +114,7 @@ public abstract class OutgoingEmail {
    * @throws EmailException
    */
   public void send() throws EmailException {
-    if (!server.isOutgoingMailEnabled()) {
+    if (!emailSender.isEnabled()) {
       // Server has explicitly disabled email sending.
       //
       return;
@@ -155,65 +154,19 @@ public abstract class OutgoingEmail {
         appendText("Gerrit-Branch: " + change.getDest().getShortName() + "\n");
       }
 
-      try {
-        final SMTPClient client = server.createOutgoingMail();
-        try {
-          if (!client.setSender(smtpFromAddress.email)) {
-            throw new EmailException("SMTP server rejected from "
-                + smtpFromAddress);
-          }
-
-          for (String emailAddress : smtpRcptTo) {
-            if (!client.addRecipient(emailAddress)) {
-              String error = client.getReplyString();
-              throw new EmailException("SMTP server rejected rcpt "
-                  + emailAddress + ": " + error);
-            }
-          }
-
-          if (headers.get("Message-ID").isEmpty()) {
-            final StringBuilder rndid = new StringBuilder();
-            rndid.append("<");
-            rndid.append(System.currentTimeMillis());
-            rndid.append("-");
-            rndid.append(Integer.toString(RNG.nextInt(999999), 36));
-            rndid.append("@");
-            rndid.append(InetAddress.getLocalHost().getCanonicalHostName());
-            rndid.append(">");
-            setHeader("Message-ID", rndid.toString());
-          }
-
-          Writer w = client.sendMessageData();
-          if (w == null) {
-            throw new EmailException("SMTP server rejected message body");
-          }
-          w = new BufferedWriter(w);
-
-          for (Map.Entry<String, EmailHeader> h : headers.entrySet()) {
-            if (!h.getValue().isEmpty()) {
-              w.write(h.getKey());
-              w.write(": ");
-              h.getValue().write(w);
-              w.write("\r\n");
-            }
-          }
-
-          w.write("\r\n");
-          w.write(body.toString());
-          w.flush();
-          w.close();
-
-          if (!client.completePendingCommand()) {
-            throw new EmailException("SMTP server rejected message body");
-          }
-
-          client.logout();
-        } finally {
-          client.disconnect();
-        }
-      } catch (IOException e) {
-        throw new EmailException("Cannot send outgoing email", e);
+      if (headers.get("Message-ID").isEmpty()) {
+        final StringBuilder rndid = new StringBuilder();
+        rndid.append("<");
+        rndid.append(System.currentTimeMillis());
+        rndid.append("-");
+        rndid.append(Integer.toString(RNG.nextInt(999999), 36));
+        rndid.append("@");
+        rndid.append(SystemReader.getInstance().getHostname());
+        rndid.append(">");
+        setHeader("Message-ID", rndid.toString());
       }
+
+      emailSender.send(smtpFromAddress, smtpRcptTo, headers, body.toString());
     }
   }
 
@@ -235,11 +188,6 @@ public abstract class OutgoingEmail {
       setChangeSubjectHeader();
     }
     setHeader("Message-ID", "");
-    setHeader("MIME-Version", "1.0");
-    setHeader("Content-Type", "text/plain; charset=UTF-8");
-    setHeader("Content-Transfer-Encoding", "8bit");
-    setHeader("Content-Disposition", "inline");
-    setHeader("User-Agent", "Gerrit/" + Version.getVersion());
     setHeader("X-Gerrit-MessageType", messageClass);
     if (change != null) {
       setHeader("X-Gerrit-ChangeId", "" + change.getChangeId());
@@ -626,7 +574,7 @@ public abstract class OutgoingEmail {
   /** Schedule delivery of this message to the given account. */
   protected void add(final RecipientType rt, final Address addr) {
     if (addr != null && addr.email != null && addr.email.length() > 0) {
-      smtpRcptTo.add(addr.email);
+      smtpRcptTo.add(addr);
       switch (rt) {
         case TO:
           ((EmailHeader.AddressList) headers.get(HDR_TO)).add(addr);

src/main/java/com/google/gerrit/server/mail/RegisterNewEmailSender.java

@@ -27,9 +27,9 @@ public class RegisterNewEmailSender extends OutgoingEmail {
   private final HttpServletRequest req;
   private final String addr;
 
-  public RegisterNewEmailSender(final GerritServer srv, final String address,
-      final HttpServletRequest request) {
-    super(srv, null, "registernewemail");
+  public RegisterNewEmailSender(final GerritServer gs, final EmailSender sf,
+      final String address, final HttpServletRequest request) {
+    super(gs, sf, null, "registernewemail");
     addr = address;
     req = request;
   }

src/main/java/com/google/gerrit/server/mail/ReplacePatchSetSender.java

@@ -28,8 +28,8 @@ public class ReplacePatchSetSender extends ReplyToChangeSender {
   private final Set<Account.Id> reviewers = new HashSet<Account.Id>();
   private final Set<Account.Id> extraCC = new HashSet<Account.Id>();
 
-  public ReplacePatchSetSender(GerritServer gs, Change c) {
-    super(gs, c, "newpatchset");
+  public ReplacePatchSetSender(GerritServer gs, EmailSender sf, Change c) {
+    super(gs, sf, c, "newpatchset");
   }
 
   public void addReviewers(final Collection<Account.Id> cc) {

src/main/java/com/google/gerrit/server/mail/ReplyToChangeSender.java

@@ -19,8 +19,9 @@ import com.google.gerrit.server.GerritServer;
 
 /** Alert a user to a reply to a change, usually commentary made during review. */
 public abstract class ReplyToChangeSender extends OutgoingEmail {
-  protected ReplyToChangeSender(GerritServer gs, Change c, String mc) {
-    super(gs, c, mc);
+  protected ReplyToChangeSender(GerritServer gs, EmailSender sf, Change c,
+      String mc) {
+    super(gs, sf, c, mc);
   }
 
   @Override

src/main/java/com/google/gerrit/server/mail/SmtpEmailSender.java

@@ -0,0 +1,171 @@
+// Copyright (C) 2009 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.mail;
+
+import com.google.gerrit.pgm.Version;
+import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+
+import org.apache.commons.net.smtp.AuthSMTPClient;
+import org.apache.commons.net.smtp.SMTPClient;
+import org.apache.commons.net.smtp.SMTPReply;
+import org.spearce.jgit.lib.Config;
+
+import java.io.BufferedWriter;
+import java.io.IOException;
+import java.io.Writer;
+import java.util.Collection;
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+/** Sends email via a nearby SMTP server. */
+@Singleton
+public class SmtpEmailSender implements EmailSender {
+  private final boolean enabled;
+
+  private String smtpHost;
+  private int smtpPort;
+  private String smtpUser;
+  private String smtpPass;
+  private String[] allowrcpt;
+
+  @Inject
+  SmtpEmailSender(@GerritServerConfig final Config cfg) {
+    enabled = cfg.getBoolean("sendemail", null, "enable", true);
+
+    smtpHost = cfg.getString("sendemail", null, "smtpserver");
+    if (smtpHost == null) {
+      smtpHost = "127.0.0.1";
+    }
+    smtpPort = cfg.getInt("sendemail", null, "smtpserverport", 25);
+    smtpUser = cfg.getString("sendemail", null, "smtpuser");
+    smtpPass = cfg.getString("sendemail", null, "smtpuserpass");
+    allowrcpt = cfg.getStringList("sendemail", null, "allowrcpt");
+  }
+
+  @Override
+  public boolean isEnabled() {
+    return enabled;
+  }
+
+  @Override
+  public void send(final Address from, final Collection<Address> rcpt,
+      final Map<String, EmailHeader> callerHeaders, final String body)
+      throws EmailException {
+    if (!isEnabled()) {
+      throw new EmailException("Sending email is disabled");
+    }
+
+    final Map<String, EmailHeader> hdrs =
+        new LinkedHashMap<String, EmailHeader>(callerHeaders);
+    setMissingHeader(hdrs, "MIME-Version", "1.0");
+    setMissingHeader(hdrs, "Content-Type", "text/plain; charset=UTF-8");
+    setMissingHeader(hdrs, "Content-Transfer-Encoding", "8bit");
+    setMissingHeader(hdrs, "Content-Disposition", "inline");
+    setMissingHeader(hdrs, "User-Agent", "Gerrit/" + Version.getVersion());
+
+    try {
+      final SMTPClient client = open();
+      try {
+        if (!client.setSender(from.email)) {
+          throw new EmailException("Server " + smtpHost
+              + " rejected from address " + from.email);
+        }
+
+        for (Address addr : rcpt) {
+          if (!client.addRecipient(addr.email)) {
+            String error = client.getReplyString();
+            throw new EmailException("Server " + smtpHost
+                + " rejected recipient " + addr + ": " + error);
+          }
+        }
+
+        Writer w = client.sendMessageData();
+        if (w == null) {
+          throw new EmailException("Server " + smtpHost + " rejected body");
+        }
+        w = new BufferedWriter(w);
+
+        for (Map.Entry<String, EmailHeader> h : hdrs.entrySet()) {
+          if (!h.getValue().isEmpty()) {
+            w.write(h.getKey());
+            w.write(": ");
+            h.getValue().write(w);
+            w.write("\r\n");
+          }
+        }
+
+        w.write("\r\n");
+        w.write(body);
+        w.flush();
+        w.close();
+
+        if (!client.completePendingCommand()) {
+          throw new EmailException("Server " + smtpHost + " rejected body");
+        }
+
+        client.logout();
+      } finally {
+        client.disconnect();
+      }
+    } catch (IOException e) {
+      throw new EmailException("Cannot send outgoing email", e);
+    }
+  }
+
+  private void setMissingHeader(final Map<String, EmailHeader> hdrs,
+      final String name, final String value) {
+    if (!hdrs.containsKey(name) || hdrs.get(name).isEmpty()) {
+      hdrs.put(name, new EmailHeader.String(value));
+    }
+  }
+
+  private SMTPClient open() throws EmailException {
+    final AuthSMTPClient client = new AuthSMTPClient("UTF-8");
+    client.setAllowRcpt(allowrcpt);
+    try {
+      client.connect(smtpHost, smtpPort);
+      if (!SMTPReply.isPositiveCompletion(client.getReplyCode())) {
+        throw new EmailException("SMTP server rejected connection");
+      }
+      if (!client.login()) {
+        String e = client.getReplyString();
+        throw new EmailException("SMTP server rejected login: " + e);
+      }
+      if (smtpUser != null && !client.auth(smtpUser, smtpPass)) {
+        String e = client.getReplyString();
+        throw new EmailException("SMTP server rejected auth: " + e);
+      }
+    } catch (IOException e) {
+      if (client.isConnected()) {
+        try {
+          client.disconnect();
+        } catch (IOException e2) {
+        }
+      }
+      throw new EmailException(e.getMessage(), e);
+    } catch (EmailException e) {
+      if (client.isConnected()) {
+        try {
+          client.disconnect();
+        } catch (IOException e2) {
+        }
+      }
+      throw e;
+    }
+    return client;
+  }
+}

src/main/java/com/google/gerrit/server/patch/PatchDetailServiceImpl.java

@@ -47,6 +47,7 @@ import com.google.gerrit.server.mail.AbandonedSender;
 import com.google.gerrit.server.mail.AddReviewerSender;
 import com.google.gerrit.server.mail.CommentSender;
 import com.google.gerrit.server.mail.EmailException;
+import com.google.gerrit.server.mail.EmailSender;
 import com.google.gwt.user.client.rpc.AsyncCallback;
 import com.google.gwtjsonrpc.client.VoidResult;
 import com.google.gwtorm.client.OrmException;
@@ -70,13 +71,15 @@ public class PatchDetailServiceImpl extends BaseServiceImplementation implements
   private final Logger log = LoggerFactory.getLogger(getClass());
   private final GerritServer server;
   private final FileTypeRegistry registry;
+  private final EmailSender emailSender;
 
   @Inject
   PatchDetailServiceImpl(final SchemaFactory<ReviewDb> sf,
-      final GerritServer gs, final FileTypeRegistry ftr) {
+      final GerritServer gs, final FileTypeRegistry ftr, final EmailSender es) {
     super(sf);
     server = gs;
     registry = ftr;
+    emailSender = es;
   }
 
   public void patchScript(final Patch.Key patchKey, final PatchSet.Id psa,
@@ -173,7 +176,8 @@ public class PatchDetailServiceImpl extends BaseServiceImplementation implements
         });
 
         try {
-          final CommentSender cm = new CommentSender(server, r.change);
+          final CommentSender cm;
+          cm = new CommentSender(server, emailSender, r.change);
           cm.setFrom(Common.getAccountId());
           cm.setPatchSet(r.patchSet, r.info);
           cm.setChangeMessage(r.message);
@@ -357,7 +361,8 @@ public class PatchDetailServiceImpl extends BaseServiceImplementation implements
 
         // Email the reviewer
         try {
-          final AddReviewerSender cm = new AddReviewerSender(server, change);
+          final AddReviewerSender cm;
+          cm = new AddReviewerSender(server, emailSender, change);
           cm.setFrom(Common.getAccountId());
           cm.setReviewDb(db);
           cm.addReviewers(reviewerIds);
@@ -439,7 +444,8 @@ public class PatchDetailServiceImpl extends BaseServiceImplementation implements
         if (dbSuccess) {
           // Email the reviewers
           try {
-            final AbandonedSender cm = new AbandonedSender(server, change);
+            final AbandonedSender cm;
+            cm = new AbandonedSender(server, emailSender, change);
             cm.setFrom(me);
             cm.setReviewDb(db);
             cm.setChangeMessage(cmsg);

src/main/java/com/google/gerrit/server/ssh/Receive.java

@@ -47,6 +47,7 @@ import com.google.gerrit.server.mail.CreateChangeSender;
 import com.google.gerrit.server.mail.EmailException;
 import com.google.gerrit.server.mail.MergedSender;
 import com.google.gerrit.server.mail.ReplacePatchSetSender;
+import com.google.gerrit.server.mail.EmailSender;
 import com.google.gwtorm.client.OrmException;
 import com.google.gwtorm.client.OrmRunnable;
 import com.google.gwtorm.client.Transaction;
@@ -128,6 +129,9 @@ class Receive extends AbstractGitCommand {
     }
   }
 
+  @Inject
+  private EmailSender emailSender;
+
   @Inject
   private ReplicationQueue replication;
 
@@ -764,7 +768,8 @@ class Receive extends AbstractGitCommand {
     allNewChanges.add(change.getId());
 
     try {
-      final CreateChangeSender cm = new CreateChangeSender(server, change);
+      final CreateChangeSender cm;
+      cm = new CreateChangeSender(server, emailSender, change);
       cm.setFrom(me);
       cm.setPatchSet(ps, imp.getPatchSetInfo());
       cm.setReviewDb(db);
@@ -1023,8 +1028,8 @@ class Receive extends AbstractGitCommand {
       cmd.setResult(ReceiveCommand.Result.OK);
 
       try {
-        final ReplacePatchSetSender cm =
-            new ReplacePatchSetSender(server, result.change);
+        final ReplacePatchSetSender cm;
+        cm = new ReplacePatchSetSender(server, emailSender, result.change);
         cm.setFrom(me);
         cm.setPatchSet(ps, result.info);
         cm.setChangeMessage(result.msg);
@@ -1293,7 +1298,8 @@ class Receive extends AbstractGitCommand {
   private void sendMergedEmail(final ReplaceResult result) {
     if (result != null && result.mergedIntoRef != null) {
       try {
-        final MergedSender cm = new MergedSender(server, result.change);
+        final MergedSender cm;
+        cm = new MergedSender(server, emailSender, result.change);
         cm.setFrom(getAccountId());
         cm.setReviewDb(db);
         cm.setPatchSet(result.patchSet, result.info);