Move commit validation into PatchSetInserter

Change-Id: I6af1ad62d9ad2448d935708087f26b4bc6c2dfff

gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/EditCommitMessageHandler.java

@@ -26,14 +26,12 @@ import com.google.gerrit.server.GerritPersonIdent;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.change.PatchSetInserter;
 import com.google.gerrit.server.git.GitRepositoryManager;
-import com.google.gerrit.server.git.validators.CommitValidators;
 import com.google.gerrit.server.mail.CommitMessageEditedSender;
 import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
 import com.google.gerrit.server.project.ChangeControl;
 import com.google.gerrit.server.project.InvalidChangeOperationException;
 import com.google.gerrit.server.project.NoSuchChangeException;
 import com.google.gerrit.server.project.NoSuchProjectException;
-import com.google.gerrit.server.ssh.NoSshInfo;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.assistedinject.Assisted;
@@ -61,7 +59,6 @@ class EditCommitMessageHandler extends Handler<ChangeDetail> {
   private final PatchSet.Id patchSetId;
   @Nullable
   private final String message;
-  private final CommitValidators.Factory commitValidatorsFactory;
   private final GitRepositoryManager gitManager;
   private final PersonIdent myIdent;
   private final PatchSetInserter.Factory patchSetInserterFactory;
@@ -73,7 +70,6 @@ class EditCommitMessageHandler extends Handler<ChangeDetail> {
       final CommitMessageEditedSender.Factory commitMessageEditedSenderFactory,
       @Assisted final PatchSet.Id patchSetId,
       @Assisted @Nullable final String message,
-      final CommitValidators.Factory commitValidatorsFactory,
       final GitRepositoryManager gitManager,
       @GerritPersonIdent final PersonIdent myIdent,
       final PatchSetInserter.Factory patchSetInserterFactory) {
@@ -84,7 +80,6 @@ class EditCommitMessageHandler extends Handler<ChangeDetail> {
     this.commitMessageEditedSenderFactory = commitMessageEditedSenderFactory;
     this.patchSetId = patchSetId;
     this.message = message;
-    this.commitValidatorsFactory = commitValidatorsFactory;
     this.gitManager = gitManager;
     this.myIdent = myIdent;
     this.patchSetInserterFactory = patchSetInserterFactory;
@@ -110,13 +105,9 @@ class EditCommitMessageHandler extends Handler<ChangeDetail> {
       throw new NoSuchChangeException(changeId, e);
     }
     try {
-      CommitValidators commitValidators =
-          commitValidatorsFactory.create(control.getRefControl(), new NoSshInfo(), git);
-
       ChangeUtil.editCommitMessage(patchSetId, control.getRefControl(),
-          commitValidators, currentUser, message, db,
+          currentUser, message, db, commitMessageEditedSenderFactory, git,
-          commitMessageEditedSenderFactory, git, myIdent, patchSetInserterFactory);
+          myIdent, patchSetInserterFactory);
-
       return changeDetailFactory.create(changeId).call();
     } finally {
       git.close();

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

@@ -311,8 +311,8 @@ public class ChangeUtil {
   }
 
   public static Change.Id editCommitMessage(final PatchSet.Id patchSetId,
-      final RefControl refControl, CommitValidators commitValidators,
+      final RefControl refControl, final IdentifiedUser user,
-      final IdentifiedUser user, final String message, final ReviewDb db,
+      final String message, final ReviewDb db,
       final CommitMessageEditedSender.Factory commitMessageEditedSenderFactory,
       Repository git, PersonIdent myIdent,
       PatchSetInserter.Factory patchSetInserterFactory)
@@ -369,18 +369,6 @@ public class ChangeUtil {
       newPatchSet.setRevision(new RevId(newCommit.name()));
       newPatchSet.setDraft(originalPS.isDraft());
 
-      CommitReceivedEvent commitReceivedEvent =
-          new CommitReceivedEvent(new ReceiveCommand(ObjectId.zeroId(),
-              newCommit.getId(), newPatchSet.getRefName()), refControl
-              .getProjectControl().getProject(), refControl.getRefName(),
-              newCommit, user);
-
-      try {
-        commitValidators.validateForReceiveCommits(commitReceivedEvent);
-      } catch (CommitValidationException e) {
-        throw new InvalidChangeOperationException(e.getMessage());
-      }
-
       final String msg =
           "Patch Set " + newPatchSet.getPatchSetId()
               + ": Commit message was updated";
@@ -390,6 +378,7 @@ public class ChangeUtil {
           .setPatchSet(newPatchSet)
           .setMessage(msg)
           .setCopyLabels(true)
+          .setValidateForReceiveCommits(true)
           .insert();
 
       return change.getId();

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

@@ -122,9 +122,6 @@ public class CherryPickChange {
     }
 
     try {
-      CommitValidators commitValidators =
-          commitValidatorsFactory.create(refControl, new NoSshInfo(), git);
-
       RevWalk revWalk = new RevWalk(git);
       try {
         Ref destRef = git.getRef(destinationBranch);
@@ -186,12 +183,12 @@ public class CherryPickChange {
           // The change key exists on the destination branch. The cherry pick
           // will be added as a new patch set.
           return insertPatchSet(git, revWalk, destChanges.get(0), patchSetId,
-              cherryPickCommit, refControl, commitValidators);
+              cherryPickCommit, refControl);
         } else {
           // Change key not found on destination branch. We can create a new
           // change.
           return createNewChange(git, revWalk, changeKey, project, patchSetId, destRef,
-              cherryPickCommit, refControl, commitValidators);
+              cherryPickCommit, refControl);
         }
       } finally {
         revWalk.release();
@@ -203,26 +200,14 @@ public class CherryPickChange {
 
   private Change.Id insertPatchSet(Repository git, RevWalk revWalk, Change change,
       PatchSet.Id patchSetId, RevCommit cherryPickCommit,
-      RefControl refControl, CommitValidators commitValidators)
+      RefControl refControl) throws InvalidChangeOperationException,
-      throws InvalidChangeOperationException, IOException, OrmException,
+      IOException, OrmException, NoSuchChangeException {
-      NoSuchChangeException {
     PatchSet.Id id = ChangeUtil.nextPatchSetId(git, change.currentPatchSetId());
     PatchSet newPatchSet = new PatchSet(id);
     newPatchSet.setCreatedOn(new Timestamp(System.currentTimeMillis()));
     newPatchSet.setUploader(change.getOwner());
     newPatchSet.setRevision(new RevId(cherryPickCommit.name()));
 
-    CommitReceivedEvent commitReceivedEvent =
-        new CommitReceivedEvent(new ReceiveCommand(ObjectId.zeroId(),
-            cherryPickCommit.getId(), newPatchSet.getRefName()), refControl
-            .getProjectControl().getProject(), refControl.getRefName(),
-            cherryPickCommit, currentUser);
-
-    try {
-      commitValidators.validateForGerritCommits(commitReceivedEvent);
-    } catch (CommitValidationException e) {
-      throw new InvalidChangeOperationException(e.getMessage());
-    }
     patchSetInserterFactory
         .create(git, revWalk, refControl, change, cherryPickCommit)
         .setPatchSet(newPatchSet)
@@ -233,9 +218,8 @@ public class CherryPickChange {
 
   private Change.Id createNewChange(Repository git, RevWalk revWalk,
       Change.Key changeKey, Project.NameKey project, PatchSet.Id patchSetId,
-      Ref destRef, RevCommit cherryPickCommit, RefControl refControl,
+      Ref destRef, RevCommit cherryPickCommit, RefControl refControl)
-      CommitValidators commitValidators) throws OrmException,
+      throws OrmException, InvalidChangeOperationException, IOException {
-      InvalidChangeOperationException, IOException {
     Change change =
         new Change(changeKey, new Change.Id(db.nextChangeId()),
             currentUser.getAccountId(), new Branch.NameKey(project,
@@ -246,6 +230,8 @@ public class CherryPickChange {
     newPatchSet.setUploader(change.getOwner());
     newPatchSet.setRevision(new RevId(cherryPickCommit.name()));
 
+    CommitValidators commitValidators =
+        commitValidatorsFactory.create(refControl, new NoSshInfo(), git);
     CommitReceivedEvent commitReceivedEvent =
         new CommitReceivedEvent(new ReceiveCommand(ObjectId.zeroId(),
             cherryPickCommit.getId(), newPatchSet.getRefName()), refControl

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

@@ -26,10 +26,15 @@ import com.google.gerrit.server.ApprovalsUtil;
 import com.google.gerrit.server.ChangeUtil;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.config.TrackingFooters;
+import com.google.gerrit.server.events.CommitReceivedEvent;
 import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
+import com.google.gerrit.server.git.validators.CommitValidationException;
+import com.google.gerrit.server.git.validators.CommitValidators;
 import com.google.gerrit.server.patch.PatchSetInfoFactory;
 import com.google.gerrit.server.project.InvalidChangeOperationException;
 import com.google.gerrit.server.project.RefControl;
+import com.google.gerrit.server.ssh.NoSshInfo;
+import com.google.gerrit.server.ssh.SshInfo;
 import com.google.gwtorm.server.AtomicUpdate;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
@@ -41,6 +46,7 @@ import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.revwalk.FooterLine;
 import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.revwalk.RevWalk;
+import org.eclipse.jgit.transport.ReceiveCommand;
 
 import java.io.IOException;
 import java.util.Collections;
@@ -58,6 +64,8 @@ public class PatchSetInserter {
   private final ReviewDb db;
   private final IdentifiedUser user;
   private final GitReferenceUpdated gitRefUpdated;
+  private final CommitValidators.Factory commitValidatorsFactory;
+  private boolean validateForReceiveCommits;
 
   private final Repository git;
   private final RevWalk revWalk;
@@ -68,6 +76,7 @@ public class PatchSetInserter {
   private PatchSet patchSet;
   private ChangeMessage changeMessage;
   private boolean copyLabels;
+  private SshInfo sshInfo;
 
   @Inject
   public PatchSetInserter(ChangeHooks hooks,
@@ -76,6 +85,7 @@ public class PatchSetInserter {
       PatchSetInfoFactory patchSetInfoFactory,
       IdentifiedUser user,
       GitReferenceUpdated gitRefUpdated,
+      CommitValidators.Factory commitValidatorsFactory,
       @Assisted Repository git,
       @Assisted RevWalk revWalk,
       @Assisted RefControl refControl,
@@ -87,6 +97,7 @@ public class PatchSetInserter {
     this.patchSetInfoFactory = patchSetInfoFactory;
     this.user = user;
     this.gitRefUpdated = gitRefUpdated;
+    this.commitValidatorsFactory = commitValidatorsFactory;
 
     this.git = git;
     this.revWalk = revWalk;
@@ -123,9 +134,21 @@ public class PatchSetInserter {
     return this;
   }
 
+  public PatchSetInserter setSshInfo(SshInfo sshInfo) {
+    this.sshInfo = sshInfo;
+    return this;
+  }
+
+  public PatchSetInserter setValidateForReceiveCommits(boolean validate) {
+    this.validateForReceiveCommits = validate;
+    return this;
+  }
+
   public Change insert() throws InvalidChangeOperationException, OrmException,
       IOException {
     checkState(patchSet != null, "patch set not set");
+    init();
+    validate();
 
     Change updatedChange;
     RefUpdate ru = git.updateRef(patchSet.getRefName());
@@ -196,6 +219,32 @@ public class PatchSetInserter {
     return updatedChange;
   }
 
+  private void init() {
+    if (sshInfo == null) {
+      sshInfo = new NoSshInfo();
+    }
+  }
+
+  private void validate() throws InvalidChangeOperationException {
+    CommitValidators cv = commitValidatorsFactory.create(refControl, sshInfo, git);
+
+    CommitReceivedEvent event = new CommitReceivedEvent(
+        new ReceiveCommand(ObjectId.zeroId(), commit.getId(),
+            patchSet.getRefName()),
+        refControl.getProjectControl().getProject(), refControl.getRefName(),
+        commit, user);
+
+    try {
+      if (validateForReceiveCommits) {
+        cv.validateForReceiveCommits(event);
+      } else {
+        cv.validateForGerritCommits(event);
+      }
+    } catch (CommitValidationException e) {
+      throw new InvalidChangeOperationException(e.getMessage());
+    }
+  }
+
   public class ChangeModifiedException extends InvalidChangeOperationException {
     private static final long serialVersionUID = 1L;