Move ChangeUtil#revert method into Revert class

The Revert class is the only caller of this method. As a next step we
must convert Revert to use BatchUpdate and it's easier when all the
code is in place already.

Also ChangeUtil is now just a collection of static methods. So we can
remove all injections etc.

Change-Id: Ic2beef1fd21bdd7689d69da83e58d0ddb573cd58
Signed-off-by: Edwin Kempin <ekempin@google.com>

gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java

@@ -17,47 +17,18 @@ package com.google.gerrit.server;
 import com.google.common.base.Function;
 import com.google.common.collect.Ordering;
 import com.google.gerrit.common.TimeUtil;
-import com.google.gerrit.extensions.restapi.ResourceConflictException;
-import com.google.gerrit.extensions.restapi.RestApiException;
 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.change.ChangeInserter;
-import com.google.gerrit.server.change.ChangeMessages;
-import com.google.gerrit.server.git.BatchUpdate;
-import com.google.gerrit.server.git.GitRepositoryManager;
-import com.google.gerrit.server.git.UpdateException;
-import com.google.gerrit.server.git.validators.CommitValidators;
-import com.google.gerrit.server.mail.RevertedSender;
-import com.google.gerrit.server.notedb.ChangeUpdate;
-import com.google.gerrit.server.project.ChangeControl;
-import com.google.gerrit.server.project.NoSuchChangeException;
 import com.google.gerrit.server.util.IdGenerator;
 import com.google.gwtorm.server.OrmException;
-import com.google.inject.Inject;
-import com.google.inject.Provider;
 import com.google.inject.Singleton;
 
-import org.eclipse.jgit.errors.IncorrectObjectTypeException;
-import org.eclipse.jgit.errors.MissingObjectException;
-import org.eclipse.jgit.errors.RepositoryNotFoundException;
-import org.eclipse.jgit.lib.CommitBuilder;
-import org.eclipse.jgit.lib.ObjectId;
-import org.eclipse.jgit.lib.ObjectInserter;
-import org.eclipse.jgit.lib.PersonIdent;
 import org.eclipse.jgit.lib.Ref;
 import org.eclipse.jgit.lib.RefDatabase;
 import org.eclipse.jgit.lib.Repository;
-import org.eclipse.jgit.revwalk.RevCommit;
-import org.eclipse.jgit.revwalk.RevWalk;
-import org.eclipse.jgit.util.ChangeIdUtil;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 import java.io.IOException;
-import java.text.MessageFormat;
 import java.util.Collections;
 import java.util.Map;
 
@@ -72,9 +43,6 @@ public class ChangeUtil {
   private static final String SUBJECT_CROP_APPENDIX = "...";
   private static final int SUBJECT_CROP_RANGE = 10;
 
-  private static final Logger log =
-      LoggerFactory.getLogger(ChangeUtil.class);
-
   public static final Function<PatchSet, Integer> TO_PS_ID =
       new Function<PatchSet, Integer>() {
         @Override
@@ -132,6 +100,10 @@ public class ChangeUtil {
     return next;
   }
 
+  public static PatchSet.Id nextPatchSetId(PatchSet.Id id) {
+    return new PatchSet.Id(id.getParentKey(), id.get() + 1);
+  }
+
   public static PatchSet.Id nextPatchSetId(Repository git, PatchSet.Id id)
       throws IOException {
     return nextPatchSetId(git.getRefDatabase().getRefs(RefDatabase.ALL), id);
@@ -151,139 +123,6 @@ public class ChangeUtil {
     return subject;
   }
 
-  private final Provider<CurrentUser> user;
+  private ChangeUtil() {
-  private final Provider<ReviewDb> db;
-  private final Sequences seq;
-  private final PatchSetUtil psUtil;
-  private final RevertedSender.Factory revertedSenderFactory;
-  private final ChangeInserter.Factory changeInserterFactory;
-  private final GitRepositoryManager gitManager;
-  private final BatchUpdate.Factory updateFactory;
-  private final ChangeMessagesUtil changeMessagesUtil;
-  private final ChangeUpdate.Factory changeUpdateFactory;
-
-  @Inject
-  ChangeUtil(Provider<CurrentUser> user,
-      Provider<ReviewDb> db,
-      Sequences seq,
-      PatchSetUtil psUtil,
-      RevertedSender.Factory revertedSenderFactory,
-      ChangeInserter.Factory changeInserterFactory,
-      GitRepositoryManager gitManager,
-      BatchUpdate.Factory updateFactory,
-      ChangeMessagesUtil changeMessagesUtil,
-      ChangeUpdate.Factory changeUpdateFactory) {
-    this.user = user;
-    this.db = db;
-    this.seq = seq;
-    this.psUtil = psUtil;
-    this.revertedSenderFactory = revertedSenderFactory;
-    this.changeInserterFactory = changeInserterFactory;
-    this.gitManager = gitManager;
-    this.updateFactory = updateFactory;
-    this.changeMessagesUtil = changeMessagesUtil;
-    this.changeUpdateFactory = changeUpdateFactory;
-  }
-
-  public Change.Id revert(ChangeControl ctl, PatchSet.Id patchSetId,
-      String message, PersonIdent myIdent)
-      throws NoSuchChangeException, OrmException,
-      MissingObjectException, IncorrectObjectTypeException, IOException,
-      RestApiException, UpdateException {
-    Change.Id changeIdToRevert = patchSetId.getParentKey();
-    PatchSet patch = psUtil.get(db.get(), ctl.getNotes(), patchSetId);
-    if (patch == null) {
-      throw new NoSuchChangeException(changeIdToRevert);
-    }
-    Change changeToRevert = db.get().changes().get(changeIdToRevert);
-
-    Project.NameKey project = ctl.getProject().getNameKey();
-    try (Repository git = gitManager.openRepository(project);
-        RevWalk revWalk = new RevWalk(git)) {
-      RevCommit commitToRevert =
-          revWalk.parseCommit(ObjectId.fromString(patch.getRevision().get()));
-
-      PersonIdent authorIdent = user.get().asIdentifiedUser()
-          .newCommitterIdent(myIdent.getWhen(), myIdent.getTimeZone());
-
-      if (commitToRevert.getParentCount() == 0) {
-        throw new ResourceConflictException("Cannot revert initial commit");
-      }
-
-      RevCommit parentToCommitToRevert = commitToRevert.getParent(0);
-      revWalk.parseHeaders(parentToCommitToRevert);
-
-      CommitBuilder revertCommitBuilder = new CommitBuilder();
-      revertCommitBuilder.addParentId(commitToRevert);
-      revertCommitBuilder.setTreeId(parentToCommitToRevert.getTree());
-      revertCommitBuilder.setAuthor(authorIdent);
-      revertCommitBuilder.setCommitter(authorIdent);
-
-      if (message == null) {
-        message = MessageFormat.format(
-            ChangeMessages.get().revertChangeDefaultMessage,
-            changeToRevert.getSubject(), patch.getRevision().get());
-      }
-
-      ObjectId computedChangeId =
-          ChangeIdUtil.computeChangeId(parentToCommitToRevert.getTree(),
-              commitToRevert, authorIdent, myIdent, message);
-      revertCommitBuilder.setMessage(
-          ChangeIdUtil.insertId(message, computedChangeId, true));
-
-      RevCommit revertCommit;
-      ChangeInserter ins;
-      Change.Id changeId = new Change.Id(seq.nextChangeId());
-      try (ObjectInserter oi = git.newObjectInserter()) {
-        ObjectId id = oi.insert(revertCommitBuilder);
-        oi.flush();
-        revertCommit = revWalk.parseCommit(id);
-
-        ins = changeInserterFactory.create(
-            changeId, revertCommit, ctl.getChange().getDest().get())
-            .setValidatePolicy(CommitValidators.Policy.GERRIT)
-            .setTopic(changeToRevert.getTopic());
-
-        ChangeMessage changeMessage = new ChangeMessage(
-            new ChangeMessage.Key(
-                patchSetId.getParentKey(), ChangeUtil.messageUUID(db.get())),
-                user.get().getAccountId(), TimeUtil.nowTs(), patchSetId);
-        StringBuilder msgBuf = new StringBuilder();
-        msgBuf.append("Patch Set ").append(patchSetId.get()).append(": Reverted");
-        msgBuf.append("\n\n");
-        msgBuf.append("This patchset was reverted in change: ")
-              .append("I").append(computedChangeId.name());
-        changeMessage.setMessage(msgBuf.toString());
-        ChangeUpdate update = changeUpdateFactory.create(ctl, TimeUtil.nowTs());
-        changeMessagesUtil.addChangeMessage(db.get(), update, changeMessage);
-        update.commit();
-
-        ins.setMessage("Uploaded patch set 1.");
-        try (BatchUpdate bu = updateFactory.create(
-            db.get(), project, ctl.getUser(),
-            TimeUtil.nowTs())) {
-          bu.setRepository(git, revWalk, oi);
-          bu.insertChange(ins);
-          bu.execute();
-        }
-      }
-
-      try {
-        RevertedSender cm = revertedSenderFactory.create(project, changeId);
-        cm.setFrom(user.get().getAccountId());
-        cm.setChangeMessage(ins.getChangeMessage());
-        cm.send();
-      } catch (Exception err) {
-        log.error("Cannot send email for revert change " + changeId, err);
-      }
-
-      return changeId;
-    } catch (RepositoryNotFoundException e) {
-      throw new NoSuchChangeException(changeIdToRevert, e);
-    }
-  }
-
-  public static PatchSet.Id nextPatchSetId(PatchSet.Id id) {
-    return new PatchSet.Id(id.getParentKey(), id.get() + 1);
   }
 }

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

@@ -26,32 +26,89 @@ import com.google.gerrit.extensions.restapi.RestModifyView;
 import com.google.gerrit.extensions.webui.UiAction;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.Change.Status;
+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;
+import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.GerritPersonIdent;
+import com.google.gerrit.server.PatchSetUtil;
+import com.google.gerrit.server.Sequences;
+import com.google.gerrit.server.git.BatchUpdate;
+import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.gerrit.server.git.UpdateException;
+import com.google.gerrit.server.git.validators.CommitValidators;
+import com.google.gerrit.server.mail.RevertedSender;
+import com.google.gerrit.server.notedb.ChangeUpdate;
+import com.google.gerrit.server.project.ChangeControl;
 import com.google.gerrit.server.project.NoSuchChangeException;
 import com.google.gerrit.server.project.RefControl;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
+import com.google.inject.Provider;
 import com.google.inject.Singleton;
 
+import org.eclipse.jgit.errors.IncorrectObjectTypeException;
+import org.eclipse.jgit.errors.MissingObjectException;
+import org.eclipse.jgit.errors.RepositoryNotFoundException;
+import org.eclipse.jgit.lib.CommitBuilder;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.ObjectInserter;
 import org.eclipse.jgit.lib.PersonIdent;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.revwalk.RevWalk;
+import org.eclipse.jgit.util.ChangeIdUtil;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import java.io.IOException;
+import java.text.MessageFormat;
 
 @Singleton
 public class Revert implements RestModifyView<ChangeResource, RevertInput>,
     UiAction<ChangeResource> {
+  private static final Logger log = LoggerFactory.getLogger(Revert.class);
+
+  private final Provider<ReviewDb> db;
+  private final Provider<CurrentUser> user;
+  private final GitRepositoryManager repoManager;
+  private final ChangeInserter.Factory changeInserterFactory;
+  private final ChangeMessagesUtil cmUtil;
+  private final ChangeUpdate.Factory changeUpdateFactory;
+  private final BatchUpdate.Factory updateFactory;
+  private final Sequences seq;
+  private final PatchSetUtil psUtil;
+  private final RevertedSender.Factory revertedSenderFactory;
   private final ChangeJson.Factory json;
-  private final ChangeUtil changeUtil;
   private final PersonIdent myIdent;
 
   @Inject
-  Revert(ChangeJson.Factory json,
+  Revert(Provider<ReviewDb> db,
-      ChangeUtil changeUtil,
+      Provider<CurrentUser> user,
+      GitRepositoryManager repoManager,
+      ChangeInserter.Factory changeInserterFactory,
+      ChangeMessagesUtil cmUtil,
+      ChangeUpdate.Factory changeUpdateFactory,
+      BatchUpdate.Factory updateFactory,
+      Sequences seq,
+      PatchSetUtil psUtil,
+      RevertedSender.Factory revertedSenderFactory,
+      ChangeJson.Factory json,
       @GerritPersonIdent PersonIdent myIdent) {
+    this.db = db;
+    this.user = user;
+    this.repoManager = repoManager;
+    this.changeInserterFactory = changeInserterFactory;
+    this.cmUtil = cmUtil;
+    this.changeUpdateFactory = changeUpdateFactory;
+    this.updateFactory = updateFactory;
+    this.seq = seq;
+    this.psUtil = psUtil;
+    this.revertedSenderFactory = revertedSenderFactory;
     this.json = json;
-    this.changeUtil = changeUtil;
     this.myIdent = myIdent;
   }
 
@@ -69,7 +126,7 @@ public class Revert implements RestModifyView<ChangeResource, RevertInput>,
 
     Change.Id revertedChangeId;
     try {
-      revertedChangeId = changeUtil.revert(req.getControl(),
+      revertedChangeId = revert(req.getControl(),
             change.currentPatchSetId(),
             Strings.emptyToNull(input.message),
             new PersonIdent(myIdent, TimeUtil.nowTs()));
@@ -80,6 +137,104 @@ public class Revert implements RestModifyView<ChangeResource, RevertInput>,
         revertedChangeId);
   }
 
+  private Change.Id revert(ChangeControl ctl, PatchSet.Id patchSetId,
+      String message, PersonIdent myIdent)
+      throws NoSuchChangeException, OrmException,
+      MissingObjectException, IncorrectObjectTypeException, IOException,
+      RestApiException, UpdateException {
+    Change.Id changeIdToRevert = patchSetId.getParentKey();
+    PatchSet patch = psUtil.get(db.get(), ctl.getNotes(), patchSetId);
+    if (patch == null) {
+      throw new NoSuchChangeException(changeIdToRevert);
+    }
+    Change changeToRevert = db.get().changes().get(changeIdToRevert);
+
+    Project.NameKey project = ctl.getProject().getNameKey();
+    try (Repository git = repoManager.openRepository(project);
+        RevWalk revWalk = new RevWalk(git)) {
+      RevCommit commitToRevert =
+          revWalk.parseCommit(ObjectId.fromString(patch.getRevision().get()));
+
+      PersonIdent authorIdent = user.get().asIdentifiedUser()
+          .newCommitterIdent(myIdent.getWhen(), myIdent.getTimeZone());
+
+      if (commitToRevert.getParentCount() == 0) {
+        throw new ResourceConflictException("Cannot revert initial commit");
+      }
+
+      RevCommit parentToCommitToRevert = commitToRevert.getParent(0);
+      revWalk.parseHeaders(parentToCommitToRevert);
+
+      CommitBuilder revertCommitBuilder = new CommitBuilder();
+      revertCommitBuilder.addParentId(commitToRevert);
+      revertCommitBuilder.setTreeId(parentToCommitToRevert.getTree());
+      revertCommitBuilder.setAuthor(authorIdent);
+      revertCommitBuilder.setCommitter(authorIdent);
+
+      if (message == null) {
+        message = MessageFormat.format(
+            ChangeMessages.get().revertChangeDefaultMessage,
+            changeToRevert.getSubject(), patch.getRevision().get());
+      }
+
+      ObjectId computedChangeId =
+          ChangeIdUtil.computeChangeId(parentToCommitToRevert.getTree(),
+              commitToRevert, authorIdent, myIdent, message);
+      revertCommitBuilder.setMessage(
+          ChangeIdUtil.insertId(message, computedChangeId, true));
+
+      RevCommit revertCommit;
+      ChangeInserter ins;
+      Change.Id changeId = new Change.Id(seq.nextChangeId());
+      try (ObjectInserter oi = git.newObjectInserter()) {
+        ObjectId id = oi.insert(revertCommitBuilder);
+        oi.flush();
+        revertCommit = revWalk.parseCommit(id);
+
+        ins = changeInserterFactory.create(
+            changeId, revertCommit, ctl.getChange().getDest().get())
+            .setValidatePolicy(CommitValidators.Policy.GERRIT)
+            .setTopic(changeToRevert.getTopic());
+
+        ChangeMessage changeMessage = new ChangeMessage(
+            new ChangeMessage.Key(
+                patchSetId.getParentKey(), ChangeUtil.messageUUID(db.get())),
+                user.get().getAccountId(), TimeUtil.nowTs(), patchSetId);
+        StringBuilder msgBuf = new StringBuilder();
+        msgBuf.append("Patch Set ").append(patchSetId.get()).append(": Reverted");
+        msgBuf.append("\n\n");
+        msgBuf.append("This patchset was reverted in change: ")
+              .append("I").append(computedChangeId.name());
+        changeMessage.setMessage(msgBuf.toString());
+        ChangeUpdate update = changeUpdateFactory.create(ctl, TimeUtil.nowTs());
+        cmUtil.addChangeMessage(db.get(), update, changeMessage);
+        update.commit();
+
+        ins.setMessage("Uploaded patch set 1.");
+        try (BatchUpdate bu = updateFactory.create(
+            db.get(), project, ctl.getUser(),
+            TimeUtil.nowTs())) {
+          bu.setRepository(git, revWalk, oi);
+          bu.insertChange(ins);
+          bu.execute();
+        }
+      }
+
+      try {
+        RevertedSender cm = revertedSenderFactory.create(project, changeId);
+        cm.setFrom(user.get().getAccountId());
+        cm.setChangeMessage(ins.getChangeMessage());
+        cm.send();
+      } catch (Exception err) {
+        log.error("Cannot send email for revert change " + changeId, err);
+      }
+
+      return changeId;
+    } catch (RepositoryNotFoundException e) {
+      throw new NoSuchChangeException(changeIdToRevert, e);
+    }
+  }
+
   @Override
   public UiAction.Description getDescription(ChangeResource resource) {
     return new UiAction.Description()