Automatically send email when new patch sets are uploaded

This way if a new reviewer was added by the "repo upload" tool they
get notification of the change in their inbox.

This also gives existing reviewers a "ping" to let them know that the
uploader wants them to look at the change again.

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

2 files changed, 119 insertions, 53 deletions

diff --git a/appjar/src/main/java/com/google/gerrit/server/ChangeMail.java b/appjar/src/main/java/com/google/gerrit/server/ChangeMail.java
index 2dee2b4f7c..46808bb7c5 100644
--- a/appjar/src/main/java/com/google/gerrit/server/ChangeMail.java
+++ b/appjar/src/main/java/com/google/gerrit/server/ChangeMail.java
@@ -123,15 +123,6 @@ public class ChangeMail {
 
   public void sendNewChange() throws MessagingException {
     if (begin("newchange")) {
-      add(RecipientType.TO, reviewers);
-      add(RecipientType.CC, extraCC);
-      if (patchSetInfo != null) {
-        // Make sure the author/committer get notice of a change that
-        // they will be blamed later on for writing.
-        //
-        add(RecipientType.CC, patchSetInfo.getAuthor());
-        add(RecipientType.CC, patchSetInfo.getCommitter());
-      }
       newChangeTo();
       if (!haveRcptTo()) {
         // No destinations at this point makes it very moot to mail,
@@ -140,15 +131,7 @@ public class ChangeMail {
         //
         return;
       }
-
-      // CC the owner/uploader, but in truth these should always match
-      // the sender too. add will strip duplicates (if any).
-      //
-      add(RecipientType.CC, change.getOwner());
-      if (patchSet != null) {
-        add(RecipientType.CC, patchSet.getUploader());
-      }
-      ccSender();
+      newChangeCc();
 
       body.append("New change ");
       body.append(change.getChangeId());
@@ -156,44 +139,76 @@ public class ChangeMail {
       body.append(change.getDest().getShortName());
       body.append(":\n\n");
 
-      if (changeUrl() != null) {
-        body.append("  ");
-        body.append(changeUrl());
-        body.append("\n\n");
-      }
+      newChangePatchSetInfo();
+      newChangeFooter();
 
-      if (patchSetInfo != null) {
-        body.append(patchSetInfo.getMessage().trim());
-        body.append("\n\n");
-      } else {
-        body.append(change.getSubject().trim());
-        body.append("\n\n");
-      }
+      msg.setMessageID(changeMessageThreadId());
+      send();
+    }
+  }
+
+  private void newChangePatchSetInfo() {
+    if (changeUrl() != null) {
+      body.append("  ");
+      body.append(changeUrl());
+      body.append("\n\n");
+    }
 
-      if (db != null && patchSet != null) {
-        body.append("---\n");
-        try {
-          for (Patch p : db.patches().byPatchSet(patchSet.getId())) {
-            body.append(p.getChangeType().getCode());
-            body.append(' ');
-            body.append(p.getFileName());
-            body.append('\n');
-          }
-        } catch (OrmException e) {
-          // Don't bother including the files if we get a failure,
-          // ensure we at least send the notification message.
+    if (patchSetInfo != null) {
+      body.append(patchSetInfo.getMessage().trim());
+      body.append("\n\n");
+    } else {
+      body.append(change.getSubject().trim());
+      body.append("\n\n");
+    }
+
+    if (db != null && patchSet != null) {
+      body.append("---\n");
+      try {
+        for (Patch p : db.patches().byPatchSet(patchSet.getId())) {
+          body.append(p.getChangeType().getCode());
+          body.append(' ');
+          body.append(p.getFileName());
+          body.append('\n');
         }
-        body.append("\n");
+      } catch (OrmException e) {
+        // Don't bother including the files if we get a failure,
+        // ensure we at least send the notification message.
       }
+      body.append("\n");
+    }
+  }
 
-      openFooter();
-      if (changeUrl() != null) {
-        body.append("View this change at ");
-        body.append(changeUrl());
-        body.append("\n");
+  private void newChangeFooter() {
+    openFooter();
+    if (changeUrl() != null) {
+      body.append("View this change at ");
+      body.append(changeUrl());
+      body.append("\n");
+    }
+  }
+
+  public void sendNewPatchSet() throws MessagingException {
+    if (begin("newpatchset")) {
+      newChangeTo();
+      if (!haveRcptTo()) {
+        // No destinations at this point makes it very moot to mail,
+        // nobody was interested in the change or was told to look
+        // at it by the caller.
+        //
+        return;
       }
+      newChangeCc();
 
-      msg.setMessageID(changeMessageThreadId());
+      body.append("Uploaded replacement patch set ");
+      body.append(patchSet.getPatchSetId());
+      body.append(" for change ");
+      body.append(change.getChangeId());
+      body.append(":\n\n");
+
+      newChangePatchSetInfo();
+      newChangeFooter();
+      initInReplyToChange();
       send();
     }
   }
@@ -250,6 +265,16 @@ public class ChangeMail {
   }
 
   private void newChangeTo() throws MessagingException {
+    add(RecipientType.TO, reviewers);
+    add(RecipientType.CC, extraCC);
+    if (patchSetInfo != null) {
+      // Make sure the author/committer get notice of a change that
+      // they will be blamed later on for writing.
+      //
+      add(RecipientType.CC, patchSetInfo.getAuthor());
+      add(RecipientType.CC, patchSetInfo.getCommitter());
+    }
+
     final ProjectCache.Entry cacheEntry =
         Common.getProjectCache().get(change.getDest().getParentKey());
     if (cacheEntry == null) {
@@ -281,6 +306,17 @@ public class ChangeMail {
     }
   }
 
+  private void newChangeCc() throws MessagingException {
+    // CC the owner/uploader, but in truth these should always match
+    // the sender too. add will strip duplicates (if any).
+    //
+    add(RecipientType.CC, change.getOwner());
+    if (patchSet != null) {
+      add(RecipientType.CC, patchSet.getUploader());
+    }
+    ccSender();
+  }
+
   private void commentTo() throws MessagingException {
     // Always to the owner/uploader/author/committer. These people
     // have a vested interest in the change and any remarks made.
diff --git a/appjar/src/main/java/com/google/gerrit/server/ssh/Receive.java b/appjar/src/main/java/com/google/gerrit/server/ssh/Receive.java
index 012191d825..5ead398186 100644
--- a/appjar/src/main/java/com/google/gerrit/server/ssh/Receive.java
+++ b/appjar/src/main/java/com/google/gerrit/server/ssh/Receive.java
@@ -28,6 +28,7 @@ import com.google.gerrit.client.reviewdb.ChangeMessage;
 import com.google.gerrit.client.reviewdb.ContactInformation;
 import com.google.gerrit.client.reviewdb.ContributorAgreement;
 import com.google.gerrit.client.reviewdb.PatchSet;
+import com.google.gerrit.client.reviewdb.PatchSetInfo;
 import com.google.gerrit.client.reviewdb.RevId;
 import com.google.gerrit.client.reviewdb.ReviewDb;
 import com.google.gerrit.client.rpc.Common;
@@ -589,8 +590,10 @@ class Receive extends AbstractGitCommand {
     }
 
     final Account.Id me = userAccount.getId();
-    final PatchSet ps = db.run(new OrmRunnable<PatchSet, ReviewDb>() {
-      public PatchSet run(final ReviewDb db, final Transaction txn,
+    final ReplaceResult result;
+
+    result = db.run(new OrmRunnable<ReplaceResult, ReviewDb>() {
+      public ReplaceResult run(final ReviewDb db, final Transaction txn,
           final boolean isRetry) throws OrmException {
         final Change change;
         if (isRetry) {
@@ -712,19 +715,46 @@ class Receive extends AbstractGitCommand {
         change.setCurrentPatchSet(imp.getPatchSetInfo());
         ChangeUtil.updated(change);
         db.changes().update(Collections.singleton(change), txn);
-        return ps;
+        
+        final ReplaceResult result = new ReplaceResult();
+        result.change = change;
+        result.patchSet = ps;
+        result.info = imp.getPatchSetInfo();
+        result.msg = msg;
+        return result;
       }
     });
-    if (ps != null) {
+    if (result != null) {
+      final PatchSet ps = result.patchSet;
       final RefUpdate ru = repo.updateRef(ps.getRefName());
       ru.setForceUpdate(true);
       ru.setNewObjectId(c);
       ru.update(rp.getRevWalk());
       PushQueue.scheduleUpdate(proj.getNameKey(), ru.getName());
       cmd.setResult(ReceiveCommand.Result.OK);
+
+      try {
+        final ChangeMail cm = new ChangeMail(server, result.change);
+        cm.setFrom(me);
+        cm.setPatchSet(ps, result.info);
+        cm.setChangeMessage(result.msg);
+        cm.setReviewDb(db);
+        cm.addReviewers(reviewerId);
+        cm.addExtraCC(ccId);
+        cm.sendNewPatchSet();
+      } catch (MessagingException e) {
+        log.error("Cannot send email for new patch set " + ps.getId(), e);
+      }
     }
   }
 
+  private static class ReplaceResult {
+    Change change;
+    PatchSet patchSet;
+    PatchSetInfo info;
+    ChangeMessage msg;
+  }
+
   private boolean validCommitter(final ReceiveCommand cmd, final RevCommit c) {
     // Require that committer matches the uploader.
     final PersonIdent committer = c.getCommitterIdent();