Move pre-merge validation of refs/meta/config commits to a listener

Move pre-merge validation of commits on refs/meta/config out of the
MergeOp class into a new validation listener.

Change-Id: Id6a7e098098ff0e1d41e3b2f63a258163a6ceb0e

gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java

@@ -82,6 +82,7 @@ import com.google.gerrit.server.git.validators.CommitValidationListener;
 import com.google.gerrit.server.git.validators.CommitValidators;
 import com.google.gerrit.server.git.validators.MergeValidationListener;
 import com.google.gerrit.server.git.validators.MergeValidators;
+import com.google.gerrit.server.git.validators.MergeValidators.ProjectConfigValidator;
 import com.google.gerrit.server.mail.AddReviewerSender;
 import com.google.gerrit.server.mail.CommitMessageEditedSender;
 import com.google.gerrit.server.mail.CreateChangeSender;
@@ -260,6 +261,7 @@ public class GerritGlobalModule extends FactoryModule {
 
     factory(CommitValidators.Factory.class);
     factory(MergeValidators.Factory.class);
+    factory(ProjectConfigValidator.Factory.class);
     factory(NotesBranchUtil.Factory.class);
 
     bind(AccountManager.class);

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

@@ -41,7 +41,6 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.ChangeUtil;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.account.AccountCache;
-import com.google.gerrit.server.config.AllProjectsName;
 import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
 import com.google.gerrit.server.git.validators.MergeValidationException;
 import com.google.gerrit.server.git.validators.MergeValidators;
@@ -155,7 +154,6 @@ public class MergeOp {
   private final SubmoduleOp.Factory subOpFactory;
   private final WorkQueue workQueue;
   private final RequestScopePropagator requestScopePropagator;
-  private final AllProjectsName allProjectsName;
   private final ChangeIndexer indexer;
 
   @Inject
@@ -172,7 +170,6 @@ public class MergeOp {
       final SubmoduleOp.Factory subOpFactory,
       final WorkQueue workQueue,
       final RequestScopePropagator requestScopePropagator,
-      final AllProjectsName allProjectsName,
       final ChangeIndexer indexer,
       final MergeValidators.Factory mergeValidatorsFactory) {
     repoManager = grm;
@@ -193,7 +190,6 @@ public class MergeOp {
     this.subOpFactory = subOpFactory;
     this.workQueue = workQueue;
     this.requestScopePropagator = requestScopePropagator;
-    this.allProjectsName = allProjectsName;
     this.indexer = indexer;
     this.mergeValidatorsFactory = mergeValidatorsFactory;
     destBranch = branch;
@@ -500,52 +496,6 @@ public class MergeOp {
         continue;
       }
 
-      if (GitRepositoryManager.REF_CONFIG.equals(destBranch.get())) {
-        final Project.NameKey newParent;
-        try {
-          ProjectConfig cfg =
-              new ProjectConfig(destProject.getProject().getNameKey());
-          cfg.load(repo, commit);
-          newParent = cfg.getProject().getParent(allProjectsName);
-        } catch (Exception e) {
-          commits.put(changeId, CodeReviewCommit
-              .error(CommitMergeStatus.INVALID_PROJECT_CONFIGURATION));
-          continue;
-        }
-        final Project.NameKey oldParent =
-            destProject.getProject().getParent(allProjectsName);
-        if (oldParent == null) {
-          // update of the 'All-Projects' project
-          if (newParent != null) {
-            commits.put(changeId, CodeReviewCommit
-                .error(CommitMergeStatus.INVALID_PROJECT_CONFIGURATION_ROOT_PROJECT_CANNOT_HAVE_PARENT));
-            continue;
-          }
-        } else {
-          if (!oldParent.equals(newParent)) {
-            final PatchSetApproval psa = getSubmitter(db, ps.getId());
-            if (psa == null) {
-              commits.put(changeId, CodeReviewCommit
-                  .error(CommitMergeStatus.SETTING_PARENT_PROJECT_ONLY_ALLOWED_BY_ADMIN));
-              continue;
-            }
-            final IdentifiedUser submitter =
-                identifiedUserFactory.create(psa.getAccountId());
-            if (!submitter.getCapabilities().canAdministrateServer()) {
-              commits.put(changeId, CodeReviewCommit
-                  .error(CommitMergeStatus.SETTING_PARENT_PROJECT_ONLY_ALLOWED_BY_ADMIN));
-              continue;
-            }
-
-            if (projectCache.get(newParent) == null) {
-              commits.put(changeId, CodeReviewCommit
-                  .error(CommitMergeStatus.INVALID_PROJECT_CONFIGURATION_PARENT_PROJECT_NOT_FOUND));
-              continue;
-            }
-          }
-        }
-      }
-
       commit.change = chg;
       commit.patchsetId = ps.getId();
       commit.originalOrder = commitOrder++;

gerrit-server/src/main/java/com/google/gerrit/server/git/validators/MergeValidators.java

@@ -14,11 +14,22 @@
 
 package com.google.gerrit.server.git.validators;
 
+import static com.google.gerrit.server.git.MergeUtil.getSubmitter;
+
 import com.google.common.collect.Lists;
 import com.google.gerrit.extensions.registration.DynamicSet;
 import com.google.gerrit.reviewdb.client.Branch;
 import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.reviewdb.client.PatchSetApproval;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.config.AllProjectsName;
 import com.google.gerrit.server.git.CodeReviewCommit;
+import com.google.gerrit.server.git.CommitMergeStatus;
+import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.git.ProjectConfig;
+import com.google.gerrit.server.project.ProjectCache;
 import com.google.gerrit.server.project.ProjectState;
 import com.google.inject.Inject;
 
@@ -28,14 +39,17 @@ import java.util.List;
 
 public class MergeValidators {
   private final DynamicSet<MergeValidationListener> mergeValidationListeners;
+  private final ProjectConfigValidator.Factory projectConfigValidatorFactory;
 
   public interface Factory {
     MergeValidators create();
   }
 
   @Inject
-  MergeValidators(DynamicSet<MergeValidationListener> mergeValidationListeners) {
+  MergeValidators(DynamicSet<MergeValidationListener> mergeValidationListeners,
+      ProjectConfigValidator.Factory projectConfigValidatorFactory) {
     this.mergeValidationListeners = mergeValidationListeners;
+    this.projectConfigValidatorFactory = projectConfigValidatorFactory;
   }
 
   public void validatePreMerge(Repository repo,
@@ -47,12 +61,84 @@ public class MergeValidators {
     List<MergeValidationListener> validators = Lists.newLinkedList();
 
     validators.add(new PluginMergeValidationListener(mergeValidationListeners));
+    validators.add(projectConfigValidatorFactory.create());
 
     for (MergeValidationListener validator : validators) {
       validator.onPreMerge(repo, commit, destProject, destBranch, patchSetId);
     }
   }
 
+  public static class ProjectConfigValidator implements
+      MergeValidationListener {
+    private final AllProjectsName allProjectsName;
+    private final ReviewDb db;
+    private final ProjectCache projectCache;
+    private final IdentifiedUser.GenericFactory identifiedUserFactory;
+
+    public interface Factory {
+      ProjectConfigValidator create();
+    }
+
+    @Inject
+    public ProjectConfigValidator(AllProjectsName allProjectsName,
+        ReviewDb db, ProjectCache projectCache,
+        IdentifiedUser.GenericFactory iuf) {
+      this.allProjectsName = allProjectsName;
+      this.db = db;
+      this.projectCache = projectCache;
+      this.identifiedUserFactory = iuf;
+    }
+
+    @Override
+    public void onPreMerge(final Repository repo,
+        final CodeReviewCommit commit,
+        final ProjectState destProject,
+        final Branch.NameKey destBranch,
+        final PatchSet.Id patchSetId)
+        throws MergeValidationException {
+      if (GitRepositoryManager.REF_CONFIG.equals(destBranch.get())) {
+        final Project.NameKey newParent;
+        try {
+          ProjectConfig cfg =
+              new ProjectConfig(destProject.getProject().getNameKey());
+          cfg.load(repo, commit);
+          newParent = cfg.getProject().getParent(allProjectsName);
+        } catch (Exception e) {
+          throw new MergeValidationException(CommitMergeStatus.
+              INVALID_PROJECT_CONFIGURATION);
+        }
+        final Project.NameKey oldParent =
+            destProject.getProject().getParent(allProjectsName);
+        if (oldParent == null) {
+          // update of the 'All-Projects' project
+          if (newParent != null) {
+            throw new MergeValidationException(CommitMergeStatus.
+                INVALID_PROJECT_CONFIGURATION_ROOT_PROJECT_CANNOT_HAVE_PARENT);
+          }
+        } else {
+          if (!oldParent.equals(newParent)) {
+            final PatchSetApproval psa = getSubmitter(db, patchSetId);
+            if (psa == null) {
+              throw new MergeValidationException(CommitMergeStatus.
+                  SETTING_PARENT_PROJECT_ONLY_ALLOWED_BY_ADMIN);
+            }
+            final IdentifiedUser submitter =
+                identifiedUserFactory.create(psa.getAccountId());
+            if (!submitter.getCapabilities().canAdministrateServer()) {
+              throw new MergeValidationException(CommitMergeStatus.
+                  SETTING_PARENT_PROJECT_ONLY_ALLOWED_BY_ADMIN);
+            }
+
+            if (projectCache.get(newParent) == null) {
+              throw new MergeValidationException(CommitMergeStatus.
+                  INVALID_PROJECT_CONFIGURATION_PARENT_PROJECT_NOT_FOUND);
+            }
+          }
+        }
+      }
+    }
+  }
+
   /** Execute merge validation plug-ins */
   public static class PluginMergeValidationListener implements
       MergeValidationListener {