Only allow BatchUpdateListener with REPO_BEFORE_DB

The listener interface is currently only used by MergeOp, which is
REPO_BEFORE_DB. Reasoning about the order in which listeners are called
is going to be more difficult if we fuse the repo and change meta ref
update phases; sidestep the problem by just removing support for
listeners in the odd DB_BEFORE_REPO case.

Change-Id: Ia2a1c1f827247602eab1f8527f3cd3118f740f23

gerrit-server/src/main/java/com/google/gerrit/server/update/BatchUpdate.java

@@ -127,6 +127,7 @@ public abstract class BatchUpdate implements AutoCloseable {
         @Nullable RequestId requestId,
         boolean dryRun)
         throws UpdateException, RestApiException {
+      checkNotNull(listener);
       // It's safe to downcast all members of the input collection in this case, because the only
       // way a caller could have gotten any BatchUpdates in the first place is to call the create
       // method above, which always returns instances of the type we expect. Just to be safe,
@@ -158,7 +159,7 @@ public abstract class BatchUpdate implements AutoCloseable {
     }
   }
 
-  static Order getOrder(Collection<? extends BatchUpdate> updates) {
+  static Order getOrder(Collection<? extends BatchUpdate> updates, BatchUpdateListener listener) {
     Order o = null;
     for (BatchUpdate u : updates) {
       if (o == null) {
@@ -167,6 +168,12 @@ public abstract class BatchUpdate implements AutoCloseable {
         throw new IllegalArgumentException("cannot mix execution orders");
       }
     }
+    if (o != Order.REPO_BEFORE_DB) {
+      checkArgument(
+          listener == BatchUpdateListener.NONE,
+          "BatchUpdateListener not supported for order %s",
+          o);
+    }
     return o;
   }
 

gerrit-server/src/main/java/com/google/gerrit/server/update/BatchUpdateListener.java

@@ -19,6 +19,8 @@ package com.google.gerrit.server.update;
  *
  * <p>When used during execution of multiple batch updates, the {@code after*} methods are called
  * after that phase has been completed for <em>all</em> updates.
+ *
+ * <p>Listeners are only supported for the {@link Order#REPO_BEFORE_DB} order.
  */
 public interface BatchUpdateListener {
   public static final BatchUpdateListener NONE = new BatchUpdateListener() {};

gerrit-server/src/main/java/com/google/gerrit/server/update/NoteDbBatchUpdate.java

@@ -82,11 +82,10 @@ class NoteDbBatchUpdate extends BatchUpdate {
     setRequestIds(updates, requestId);
 
     try {
-      Order order = getOrder(updates);
+      Order order = getOrder(updates, listener);
       // TODO(dborowitz): Fuse implementations to use a single BatchRefUpdate between phases. Note
-      // that we will still need to respect the order, since it also dictates the order in which
-      // listener methods are called. We can revisit this later, particularly since the only user of
-      // BatchUpdateListener is MergeOp, which only uses one order.
+      // that we may still need to respect the order, since op implementations may make assumptions
+      // about the order in which their methods are called.
       switch (order) {
         case REPO_BEFORE_DB:
           for (NoteDbBatchUpdate u : updates) {
@@ -106,15 +105,12 @@ class NoteDbBatchUpdate extends BatchUpdate {
           for (NoteDbBatchUpdate u : updates) {
             u.reindexChanges(u.executeChangeOps(dryrun), dryrun);
           }
-          listener.afterUpdateChanges();
           for (NoteDbBatchUpdate u : updates) {
             u.executeUpdateRepo();
           }
-          listener.afterUpdateRepos();
           for (NoteDbBatchUpdate u : updates) {
             u.executeRefUpdates(dryrun);
           }
-          listener.afterUpdateRefs();
           break;
         default:
           throw new IllegalStateException("invalid execution order: " + order);

gerrit-server/src/main/java/com/google/gerrit/server/update/ReviewDbBatchUpdate.java

@@ -269,7 +269,7 @@ class ReviewDbBatchUpdate extends BatchUpdate {
     }
     setRequestIds(updates, requestId);
     try {
-      Order order = getOrder(updates);
+      Order order = getOrder(updates, listener);
       boolean updateChangesInParallel = getUpdateChangesInParallel(updates);
       switch (order) {
         case REPO_BEFORE_DB:
@@ -290,15 +290,12 @@ class ReviewDbBatchUpdate extends BatchUpdate {
           for (ReviewDbBatchUpdate u : updates) {
             u.reindexChanges(u.executeChangeOps(updateChangesInParallel, dryrun));
           }
-          listener.afterUpdateChanges();
           for (ReviewDbBatchUpdate u : updates) {
             u.executeUpdateRepo();
           }
-          listener.afterUpdateRepos();
           for (ReviewDbBatchUpdate u : updates) {
             u.executeRefUpdates(dryrun);
           }
-          listener.afterUpdateRefs();
           break;
         default:
           throw new IllegalStateException("invalid execution order: " + order);