Add batch mode to Abandon

When an extension wants to abandon more than one changes in the same
project, batch them together inside a BatchUpdate will be much better
than calling Abandon REST API multiple times, so this change provides
the batch mode.

Also changed AbandonUtil to group changes by project and use the new
batchAbandon code.

Change-Id: I9d89b58d4cfa469b666a234af1a564c1f5211b19

gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java

@@ -64,6 +64,7 @@ import com.google.gerrit.server.OutputFormat;
 import com.google.gerrit.server.PatchSetUtil;
 import com.google.gerrit.server.account.AccountCache;
 import com.google.gerrit.server.account.GroupCache;
+import com.google.gerrit.server.change.Abandon;
 import com.google.gerrit.server.change.ChangeResource;
 import com.google.gerrit.server.change.RevisionResource;
 import com.google.gerrit.server.change.Revisions;
@@ -238,6 +239,9 @@ public abstract class AbstractDaemonTest {
   @Inject
   protected ChangeNotes.Factory notesFactory;
 
+  @Inject
+  protected Abandon changeAbandoner;
+
   @Rule
   public ExpectedException exception = ExpectedException.none();
 

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java

@@ -31,6 +31,7 @@ import static java.util.concurrent.TimeUnit.SECONDS;
 import static org.junit.Assert.fail;
 
 import com.google.common.base.Function;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.gerrit.acceptance.AbstractDaemonTest;
@@ -73,6 +74,7 @@ import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.change.ChangeResource;
 import com.google.gerrit.server.config.AnonymousCowardNameProvider;
 import com.google.gerrit.server.git.ProjectConfig;
@@ -184,6 +186,64 @@ public class ChangeIT extends AbstractDaemonTest {
         .abandon();
   }
 
+  @Test
+  public void batchAbandon() throws Exception {
+    CurrentUser user = atrScope.get().getUser();
+    PushOneCommit.Result a = createChange();
+    List<ChangeControl> controlA = changeFinder.find(a.getChangeId(), user);
+    assertThat(controlA).hasSize(1);
+    PushOneCommit.Result b = createChange();
+    List<ChangeControl> controlB = changeFinder.find(b.getChangeId(), user);
+    assertThat(controlB).hasSize(1);
+    List<ChangeControl> list =
+        ImmutableList.of(controlA.get(0), controlB.get(0));
+    changeAbandoner.batchAbandon(
+        controlA.get(0).getProject().getNameKey(), user, list, "deadbeef");
+
+    ChangeInfo info = get(a.getChangeId());
+    assertThat(info.status).isEqualTo(ChangeStatus.ABANDONED);
+    assertThat(Iterables.getLast(info.messages).message.toLowerCase())
+        .contains("abandoned");
+    assertThat(Iterables.getLast(info.messages).message.toLowerCase())
+        .contains("deadbeef");
+
+    info = get(b.getChangeId());
+    assertThat(info.status).isEqualTo(ChangeStatus.ABANDONED);
+    assertThat(Iterables.getLast(info.messages).message.toLowerCase())
+        .contains("abandoned");
+    assertThat(Iterables.getLast(info.messages).message.toLowerCase())
+        .contains("deadbeef");
+  }
+
+  @Test
+  public void batchAbandonChangeProject() throws Exception {
+    String project1Name = name("Project1");
+    String project2Name = name("Project2");
+    gApi.projects().create(project1Name);
+    gApi.projects().create(project2Name);
+    TestRepository<InMemoryRepository> project1 =
+        cloneProject(new Project.NameKey(project1Name));
+    TestRepository<InMemoryRepository> project2 =
+        cloneProject(new Project.NameKey(project2Name));
+
+    CurrentUser user = atrScope.get().getUser();
+    PushOneCommit.Result a =
+        createChange(project1, "master", "x", "x", "x", "");
+    List<ChangeControl> controlA = changeFinder.find(a.getChangeId(), user);
+    assertThat(controlA).hasSize(1);
+    PushOneCommit.Result b =
+        createChange(project2, "master", "x", "x", "x", "");
+    List<ChangeControl> controlB = changeFinder.find(b.getChangeId(), user);
+    assertThat(controlB).hasSize(1);
+    List<ChangeControl> list =
+        ImmutableList.of(controlA.get(0), controlB.get(0));
+    exception.expect(ResourceConflictException.class);
+    exception.expectMessage(String.format(
+        "Project name \"%s\" doesn't match \"%s\"",
+        project2Name, project1Name));
+    changeAbandoner.batchAbandon(new Project.NameKey(project1Name), user, list);
+  }
+
   @Test
   public void abandonDraft() throws Exception {
     PushOneCommit.Result r = createDraftChange();

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

@@ -28,6 +28,7 @@ import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.ChangeMessage;
 import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.ChangeMessagesUtil;
 import com.google.gerrit.server.ChangeUtil;
@@ -46,9 +47,9 @@ import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 import com.google.inject.Singleton;
-
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import java.util.Collection;
 
 @Singleton
 public class Abandon implements RestModifyView<ChangeResource, AbandonInput>,
@@ -91,6 +92,11 @@ public class Abandon implements RestModifyView<ChangeResource, AbandonInput>,
     return json.create(ChangeJson.NO_OPTIONS).format(change);
   }
 
+  public Change abandon(ChangeControl control)
+      throws RestApiException, UpdateException {
+    return abandon(control, "", NotifyHandling.ALL);
+  }
+
   public Change abandon(ChangeControl control, String msgTxt)
       throws RestApiException, UpdateException {
     return abandon(control, msgTxt, NotifyHandling.ALL);
@@ -98,31 +104,79 @@ public class Abandon implements RestModifyView<ChangeResource, AbandonInput>,
 
   public Change abandon(ChangeControl control, String msgTxt,
       NotifyHandling notifyHandling) throws RestApiException, UpdateException {
-    CurrentUser user = control.getUser();
-    Account account = user.isIdentifiedUser()
-        ? user.asIdentifiedUser().getAccount()
-        : null;
-    Op op = new Op(msgTxt, account, notifyHandling);
-    try (BatchUpdate u = batchUpdateFactory.create(dbProvider.get(),
-        control.getProject().getNameKey(), user, TimeUtil.nowTs())) {
+    Op op = new Op(control, msgTxt, notifyHandling);
+    try (BatchUpdate u =
+        batchUpdateFactory.create(
+            dbProvider.get(),
+            control.getProject().getNameKey(),
+            control.getUser(),
+            TimeUtil.nowTs())) {
       u.addOp(control.getId(), op).execute();
     }
     return op.change;
   }
 
+  /**
+   * If an extension has more than one changes to abandon that belong to the
+   * same project, they should use the batch instead of abandoning one by one.
+   * <p>
+   * It's the caller's responsibility to ensure that all jobs inside the same
+   * batch have the matching project from its ChangeControl. Violations will
+   * result in a ResourceConflictException.
+   */
+  public void batchAbandon(Project.NameKey project, CurrentUser user,
+      Collection<ChangeControl> controls, String msgTxt,
+      NotifyHandling notifyHandling) throws RestApiException, UpdateException {
+    if (controls.isEmpty()) {
+      return;
+    }
+    try (BatchUpdate u = batchUpdateFactory.create(
+        dbProvider.get(), project, user, TimeUtil.nowTs())) {
+      for (ChangeControl control : controls) {
+        if (!project.equals(control.getProject().getNameKey())) {
+          throw new ResourceConflictException(
+              String.format(
+                  "Project name \"%s\" doesn't match \"%s\"",
+                  control.getProject().getNameKey().get(),
+                  project.get()));
+        }
+        u.addOp(control.getId(), new Op(control, msgTxt, notifyHandling));
+      }
+      u.execute();
+    }
+  }
+
+  public void batchAbandon(Project.NameKey project, CurrentUser user,
+      Collection<ChangeControl> controls, String msgTxt)
+      throws RestApiException, UpdateException {
+    batchAbandon(project, user, controls, msgTxt, NotifyHandling.ALL);
+  }
+
+  public void batchAbandon(Project.NameKey project, CurrentUser user,
+      Collection<ChangeControl> controls)
+      throws RestApiException, UpdateException {
+    batchAbandon(project, user, controls, "", NotifyHandling.ALL);
+  }
+
   private class Op extends BatchUpdate.Op {
-    private final Account account;
+    private final ChangeControl control;
     private final String msgTxt;
+    private final NotifyHandling notifyHandling;
+    private final Account account;
 
     private Change change;
     private PatchSet patchSet;
     private ChangeMessage message;
-    private NotifyHandling notifyHandling;
 
-    private Op(String msgTxt, Account account, NotifyHandling notifyHandling) {
-      this.account = account;
+    private Op(ChangeControl control, String msgTxt,
+        NotifyHandling notifyHandling) {
+      this.control = control;
       this.msgTxt = msgTxt;
       this.notifyHandling = notifyHandling;
+      CurrentUser user = control.getUser();
+      account = user.isIdentifiedUser()
+          ? user.asIdentifiedUser().getAccount()
+          : null;
     }
 
     @Override

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

@@ -14,7 +14,10 @@
 
 package com.google.gerrit.server.change;
 
+import com.google.common.collect.ImmutableMultimap;
+import com.google.common.collect.Multimap;
 import com.google.gerrit.extensions.restapi.ResourceConflictException;
+import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.server.InternalUser;
 import com.google.gerrit.server.config.ChangeCleanupConfig;
 import com.google.gerrit.server.project.ChangeControl;
@@ -25,10 +28,9 @@ import com.google.gerrit.server.query.change.ChangeQueryProcessor;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Singleton;
-
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-
+import java.util.Collection;
 import java.util.List;
 import java.util.concurrent.TimeUnit;
 
@@ -37,10 +39,10 @@ public class AbandonUtil {
   private static final Logger log = LoggerFactory.getLogger(AbandonUtil.class);
 
   private final ChangeCleanupConfig cfg;
-  private final InternalUser.Factory internalUserFactory;
   private final ChangeQueryProcessor queryProcessor;
   private final ChangeQueryBuilder queryBuilder;
   private final Abandon abandon;
+  private final InternalUser internalUser;
 
   @Inject
   AbandonUtil(
@@ -50,10 +52,10 @@ public class AbandonUtil {
       ChangeQueryBuilder queryBuilder,
       Abandon abandon) {
     this.cfg = cfg;
-    this.internalUserFactory = internalUserFactory;
     this.queryProcessor = queryProcessor;
     this.queryBuilder = queryBuilder;
     this.abandon = abandon;
+    internalUser = internalUserFactory.create();
   }
 
   public void abandonInactiveOpenChanges() {
@@ -68,29 +70,42 @@ public class AbandonUtil {
       if (!cfg.getAbandonIfMergeable()) {
         query += " -is:mergeable";
       }
-      List<ChangeData> changesToAbandon = queryProcessor.enforceVisibility(false)
-          .query(queryBuilder.parse(query)).entities();
-      int count = 0;
+
+      List<ChangeData> changesToAbandon =
+          queryProcessor
+              .enforceVisibility(false)
+              .query(queryBuilder.parse(query))
+              .entities();
+      ImmutableMultimap.Builder<Project.NameKey, ChangeControl> builder =
+          ImmutableMultimap.builder();
       for (ChangeData cd : changesToAbandon) {
+        ChangeControl control = cd.changeControl(internalUser);
+        builder.put(control.getProject().getNameKey(), control);
+      }
+
+      int count = 0;
+      Multimap<Project.NameKey, ChangeControl> abandons = builder.build();
+      String message = cfg.getAbandonMessage();
+      for (Project.NameKey project : abandons.keySet()) {
+        Collection<ChangeControl> changes = abandons.get(project);
         try {
-          abandon.abandon(changeControl(cd), cfg.getAbandonMessage());
-          count++;
-        } catch (ResourceConflictException e) {
-          // Change was already merged or abandoned.
+          abandon.batchAbandon(project, internalUser, changes, message);
+          count += changes.size();
         } catch (Throwable e) {
-          log.error(String.format(
-              "Failed to auto-abandon inactive open change %d.",
-                  cd.getId().get()), e);
+          StringBuilder msg =
+              new StringBuilder("Failed to auto-abandon inactive change(s):");
+          for (ChangeControl change : changes) {
+            msg.append(" ").append(change.getId().get());
+          }
+          msg.append(".");
+          log.error(msg.toString(), e);
         }
       }
       log.info(String.format("Auto-Abandoned %d of %d changes.",
           count, changesToAbandon.size()));
     } catch (QueryParseException | OrmException e) {
-      log.error("Failed to query inactive open changes for auto-abandoning.", e);
+      log.error(
+          "Failed to query inactive open changes for auto-abandoning.", e);
     }
   }
-
-  private ChangeControl changeControl(ChangeData cd) throws OrmException {
-    return cd.changeControl(internalUserFactory.create());
-  }
 }