Set Reply-To on outgoing email such that users reply to Gerrit directly

Change-Id: I4640a3342aa1b2b3014f9b61812e7d633e33d80c

Documentation/config-gerrit.txt

@@ -3534,7 +3534,6 @@ If the IMAP protocol is used for retrieving emails, IMAPv4 IDLE can be used to
 keep the connection with the email server alive and receive a push when a new
 email is delivered to the inbox. In this case, Gerrit will process the email
 immediately and will not have a fetch delay.
-
 +
 Defaults to false.
 
@@ -3727,6 +3726,16 @@ should expire.
 +
 By default, unset, so no Expiry-Date header is generated.
 
+[[sendemail.replyToAddress]]sendemail.replyToAddress::
++
+A custom Reply-To address should only be provided if Gerrit is set up to
+receive emails and the inbound address differs from
+<<sendemail.from,sendemail.from>>.
+It will be set as Reply-To header on all types of outgoing email where
+Gerrit can parse back a user's reply.
++
+Defaults to an empty string which adds <<sendemail.from,sendemail.from>> as
+Reply-To if inbound email is enabled and the review's author otherwise.
 
 [[site]]
 === Section site

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/AbstractMailIT.java

@@ -17,6 +17,7 @@ package com.google.gerrit.acceptance.server.mail;
 import com.google.common.collect.ImmutableList;
 import com.google.gerrit.acceptance.AbstractDaemonTest;
 import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.acceptance.TestAccount;
 import com.google.gerrit.extensions.api.changes.ReviewInput;
 import com.google.gerrit.extensions.api.changes.ReviewInput.CommentInput;
 import com.google.gerrit.extensions.client.Comment;
@@ -40,6 +41,10 @@ public class AbstractMailIT extends AbstractDaemonTest {
   }
 
   protected String createChangeWithReview() throws Exception {
+    return createChangeWithReview(admin);
+  }
+
+  protected String createChangeWithReview(TestAccount reviewer) throws Exception {
     // Create change
     String file = "gerrit-server/test.txt";
     String contents = "contents \nlorem \nipsum \nlorem";
@@ -49,6 +54,7 @@ public class AbstractMailIT extends AbstractDaemonTest {
     String changeId = r.getChangeId();
 
     // Review it
+    setApiUser(reviewer);
     ReviewInput input = new ReviewInput();
     input.message = "I have two comments";
     input.comments = new HashMap<>();

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/MailSenderIT.java

@@ -0,0 +1,48 @@
+// Copyright (C) 2017 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.acceptance.server.mail;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.gerrit.acceptance.GerritConfig;
+import com.google.gerrit.server.mail.send.EmailHeader;
+import java.util.Map;
+import org.junit.Test;
+
+public class MailSenderIT extends AbstractMailIT {
+
+  @Test
+  @GerritConfig(name = "sendemail.replyToAddress", value = "custom@gerritcodereview.com")
+  @GerritConfig(name = "receiveemail.protocol", value = "POP3")
+  public void outgoingMailHasCustomReplyToHeader() throws Exception {
+    createChangeWithReview(user);
+    // Check that the custom address was added as Reply-To
+    assertThat(sender.getMessages()).hasSize(1);
+    Map<String, EmailHeader> headers = sender.getMessages().iterator().next().headers();
+    assertThat(headers.get("Reply-To")).isInstanceOf(EmailHeader.String.class);
+    assertThat(((EmailHeader.String) headers.get("Reply-To")).getString())
+        .isEqualTo("custom@gerritcodereview.com");
+  }
+
+  @Test
+  public void outgoingMailHasUserEmailInReplyToHeader() throws Exception {
+    createChangeWithReview(user);
+    // Check that the user's email was added as Reply-To
+    assertThat(sender.getMessages()).hasSize(1);
+    Map<String, EmailHeader> headers = sender.getMessages().iterator().next().headers();
+    assertThat(headers.get("Reply-To")).isInstanceOf(EmailHeader.String.class);
+    assertThat(((EmailHeader.String) headers.get("Reply-To")).getString()).contains(user.email);
+  }
+}

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

@@ -29,7 +29,9 @@ import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.reviewdb.client.RobotComment;
 import com.google.gerrit.server.CommentsUtil;
 import com.google.gerrit.server.account.WatchConfig.NotifyType;
+import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.mail.MailUtil;
+import com.google.gerrit.server.mail.receive.Protocol;
 import com.google.gerrit.server.patch.PatchFile;
 import com.google.gerrit.server.patch.PatchList;
 import com.google.gerrit.server.patch.PatchListNotAvailableException;
@@ -50,6 +52,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
+import org.eclipse.jgit.lib.Config;
 import org.eclipse.jgit.lib.Repository;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -104,16 +107,23 @@ public class CommentSender extends ReplyToChangeSender {
   private String patchSetComment;
   private List<LabelVote> labels = Collections.emptyList();
   private final CommentsUtil commentsUtil;
+  private final boolean incomingEmailEnabled;
+  private final String replyToAddress;
 
   @Inject
   public CommentSender(
       EmailArguments ea,
       CommentsUtil commentsUtil,
+      @GerritServerConfig Config cfg,
       @Assisted Project.NameKey project,
       @Assisted Change.Id id)
       throws OrmException {
     super(ea, "comment", newChangeData(ea, project, id));
     this.commentsUtil = commentsUtil;
+    this.incomingEmailEnabled =
+        cfg.getEnum("receiveemail", null, "protocol", Protocol.NONE).ordinal()
+            > Protocol.NONE.ordinal();
+    this.replyToAddress = cfg.getString("sendemail", null, "replyToAddress");
   }
 
   public void setComments(List<Comment> comments) throws OrmException {
@@ -151,6 +161,15 @@ public class CommentSender extends ReplyToChangeSender {
     // Add header that enables identifying comments on parsed email.
     // Grouping is currently done by timestamp.
     setHeader("X-Gerrit-Comment-Date", timestamp);
+
+    if (incomingEmailEnabled) {
+      if (replyToAddress == null) {
+        // Remove Reply-To and use outbound SMTP (default) instead.
+        removeHeader("Reply-To");
+      } else {
+        setHeader("Reply-To", replyToAddress);
+      }
+    }
   }
 
   @Override

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

@@ -319,6 +319,11 @@ public abstract class OutgoingEmail {
     headers.put(name, new EmailHeader.String(value));
   }
 
+  /** Remove a header from the outgoing message. */
+  protected void removeHeader(final String name) {
+    headers.remove(name);
+  }
+
   protected void setHeader(final String name, final Date date) {
     headers.put(name, new EmailHeader.Date(date));
   }