Run change hooks and ref-updated events after indexing is done.

The change hooks and ref-updated events were run parallel to the
change (re)indexing. This meant that an event-stream sent events
to the clients before the change indexing was finished. If a client
queried for that change immediately after receiving the event it
happened that the response was not-found as the indexing wasn't yet
done.

Although we haven't had issues with a ref-updated event processors,
firing this event will also wait for the indexing task to finish
as we never know if a ref-updated event processor may trigger a
change query.

Change-Id: Iff17c5aeb9675f3991f938d31ef5048ef5b3b956

gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java

@@ -119,13 +119,13 @@ public class Abandon implements RestModifyView<ChangeResource, AbandonInput>,
     } catch (Exception e) {
       log.error("Cannot email update for change " + change.getChangeId(), e);
     }
+    indexFuture.checkedGet();
     hooks.doChangeAbandonedHook(change,
         caller.getAccount(),
         db.patchSets().get(change.currentPatchSetId()),
         Strings.emptyToNull(input.message),
         db);
     ChangeInfo result = json.format(change);
-    indexFuture.checkedGet();
     return result;
   }
 

gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java

@@ -189,12 +189,6 @@ public class ChangeInserter {
             changeMessage.getKey().getParentKey(), db);
       }
     }
-    gitRefUpdated.fire(change.getProject(), patchSet.getRefName(),
-        ObjectId.zeroId(), commit);
-
-    if (runHooks) {
-      hooks.doPatchsetCreatedHook(change, patchSet, db);
-    }
 
     if (sendMail) {
       try {
@@ -210,6 +204,14 @@ public class ChangeInserter {
       }
     }
     f.checkedGet();
+
+    gitRefUpdated.fire(change.getProject(), patchSet.getRefName(),
+        ObjectId.zeroId(), commit);
+
+    if (runHooks) {
+      hooks.doPatchsetCreatedHook(change, patchSet, db);
+    }
+
     return change;
   }
 

gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java

@@ -18,7 +18,6 @@ import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
 
 import com.google.common.collect.SetMultimap;
-import com.google.common.util.concurrent.CheckedFuture;
 import com.google.gerrit.common.ChangeHooks;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.Change;
@@ -306,14 +305,13 @@ public class PatchSetInserter {
     } finally {
       db.rollback();
     }
-    CheckedFuture<?, IOException> f = mergeabilityChecker.newCheck()
+    mergeabilityChecker.newCheck()
         .addChange(updatedChange)
         .reindex()
-        .runAsync();
+        .run();
     if (runHooks) {
       hooks.doPatchsetCreatedHook(updatedChange, patchSet, db);
     }
-    f.checkedGet();
     return updatedChange;
   }
 

gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java

@@ -156,8 +156,7 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
     } else {
       indexWrite = Futures.<Void, IOException> immediateCheckedFuture(null);
     }
-    if (message != null) {
+    if (message != null && input.notify.compareTo(NotifyHandling.NONE) > 0) {
-      if (input.notify.compareTo(NotifyHandling.NONE) > 0) {
       email.create(
           input.notify,
           change,
@@ -166,12 +165,13 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
           message,
           comments).sendAsync();
     }
-      fireCommentAddedHook(revision);
-    }
 
     Output output = new Output();
     output.labels = input.labels;
     indexWrite.checkedGet();
+    if (message != null) {
+      fireCommentAddedHook(revision);
+    }
     return output;
   }
 

gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java

@@ -245,24 +245,23 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
           ImmutableList.of(psa)));
     }
     accountLoaderFactory.create(true).fill(result.reviewers);
-    postAdd(rsrc.getChange(), added);
+    emailReviewers(rsrc.getChange(), added);
     indexFuture.checkedGet();
+    if (!added.isEmpty()) {
+      PatchSet patchSet = dbProvider.get().patchSets().get(rsrc.getChange().currentPatchSetId());
+      for (PatchSetApproval psa : added) {
+        Account account = accountCache.get(psa.getAccountId()).getAccount();
+        hooks.doReviewerAddedHook(rsrc.getChange(), account, patchSet, dbProvider.get());
+      }
+    }
   }
 
-  private void postAdd(Change change, List<PatchSetApproval> added)
+  private void emailReviewers(Change change, List<PatchSetApproval> added)
       throws OrmException, EmailException {
     if (added.isEmpty()) {
       return;
     }
 
-    // Execute hook for added reviewers
-    //
-    PatchSet patchSet = dbProvider.get().patchSets().get(change.currentPatchSetId());
-    for (PatchSetApproval psa : added) {
-      Account account = accountCache.get(psa.getAccountId()).getAccount();
-      hooks.doReviewerAddedHook(change, account, patchSet, dbProvider.get());
-    }
-
     // Email the reviewers
     //
     // The user knows they added themselves, don't bother emailing them.

gerrit-server/src/main/java/com/google/gerrit/server/change/Publish.java

@@ -94,12 +94,13 @@ public class Publish implements RestModifyView<RevisionResource, Input>,
           || updatedChange.getStatus() == Change.Status.NEW) {
         CheckedFuture<?, IOException> indexFuture =
             indexer.indexAsync(updatedChange.getId());
-        hooks.doDraftPublishedHook(updatedChange, updatedPatchSet, dbProvider.get());
         sender.send(rsrc.getNotes(), update,
             rsrc.getChange().getStatus() == Change.Status.DRAFT,
             rsrc.getUser(), updatedChange, updatedPatchSet,
             rsrc.getControl().getLabelTypes());
         indexFuture.checkedGet();
+        hooks.doDraftPublishedHook(updatedChange, updatedPatchSet,
+            dbProvider.get());
       }
     } catch (PatchSetInfoNotAvailableException e) {
       throw new ResourceNotFoundException(e.getMessage());

gerrit-server/src/main/java/com/google/gerrit/server/change/PutTopic.java

@@ -15,7 +15,6 @@
 package com.google.gerrit.server.change;
 
 import com.google.common.base.Strings;
-import com.google.common.util.concurrent.CheckedFuture;
 import com.google.gerrit.common.ChangeHooks;
 import com.google.gerrit.extensions.restapi.AuthException;
 import com.google.gerrit.extensions.restapi.DefaultInput;
@@ -109,11 +108,9 @@ class PutTopic implements RestModifyView<ChangeResource, Input>,
       } finally {
         db.rollback();
       }
-      CheckedFuture<?, IOException> indexFuture =
+      indexer.index(db, change);
-          indexer.indexAsync(change.getId());
       hooks.doTopicChangedHook(change, currentUser.getAccount(),
           oldTopicName, db);
-      indexFuture.checkedGet();
     }
     return Strings.isNullOrEmpty(newTopicName)
         ? Response.<String>none()

gerrit-server/src/main/java/com/google/gerrit/server/change/Restore.java

@@ -120,13 +120,13 @@ public class Restore implements RestModifyView<ChangeResource, RestoreInput>,
     } catch (Exception e) {
       log.error("Cannot email update for change " + change.getChangeId(), e);
     }
+    f.checkedGet();
     hooks.doChangeRestoredHook(change,
         caller.getAccount(),
         db.patchSets().get(change.currentPatchSetId()),
         Strings.emptyToNull(input.message),
         dbProvider.get());
     ChangeInfo result = json.format(change);
-    f.checkedGet();
     return result;
   }
 

gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java

@@ -840,6 +840,7 @@ public class MergeOp {
       db.commit();
 
       sendMergedEmail(c, submitter);
+      indexer.index(db, c);
       if (submitter != null) {
         try {
           hooks.doChangeMergedHook(c,
@@ -852,7 +853,6 @@ public class MergeOp {
     } finally {
       db.rollback();
     }
-    indexer.index(db, c);
   }
 
   private Change setMergedPatchSet(Change.Id changeId, final PatchSet.Id merged)
@@ -1065,6 +1065,14 @@ public class MergeOp {
       }
     }));
 
+    if (indexFuture != null) {
+      try {
+        indexFuture.checkedGet();
+      } catch (IOException e) {
+        log.error("Failed to index new change message", e);
+      }
+    }
+
     if (submitter != null) {
       try {
         hooks.doMergeFailedHook(c,
@@ -1074,13 +1082,6 @@ public class MergeOp {
         log.error("Cannot run hook for merge failed " + c.getId(), ex);
       }
     }
-    if (indexFuture != null) {
-      try {
-        indexFuture.checkedGet();
-      } catch (IOException e) {
-        log.error("Failed to index new change message", e);
-      }
-    }
   }
 
   private void abandonAllOpenChanges() {

gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java

@@ -2002,13 +2002,6 @@ public class ReceiveCommits {
           .addChange(change)
           .reindex()
           .runAsync();
-      gitRefUpdated.fire(project.getNameKey(), newPatchSet.getRefName(),
-          ObjectId.zeroId(), newCommit);
-      hooks.doPatchsetCreatedHook(change, newPatchSet, db);
-      if (mergedIntoRef != null) {
-        hooks.doChangeMergedHook(
-            change, currentUser.getAccount(), newPatchSet, db);
-      }
       workQueue.getDefaultQueue()
           .submit(requestScopePropagator.wrap(new Runnable() {
         @Override
@@ -2037,6 +2030,14 @@ public class ReceiveCommits {
       }));
       f.checkedGet();
 
+      gitRefUpdated.fire(project.getNameKey(), newPatchSet.getRefName(),
+          ObjectId.zeroId(), newCommit);
+      hooks.doPatchsetCreatedHook(change, newPatchSet, db);
+      if (mergedIntoRef != null) {
+        hooks.doChangeMergedHook(
+            change, currentUser.getAccount(), newPatchSet, db);
+      }
+
       if (magicBranch != null && magicBranch.isSubmit()) {
         submit(changeCtl, newPatchSet);
       }