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
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java
index 8aeaa3d..581b067 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java
@@ -82,6 +82,7 @@
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 @@
factory(CommitValidators.Factory.class);
factory(MergeValidators.Factory.class);
+ factory(ProjectConfigValidator.Factory.class);
factory(NotesBranchUtil.Factory.class);
bind(AccountManager.class);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java
index d8ae6ca..312f2b4 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java
@@ -41,7 +41,6 @@
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 @@
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 @@
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 @@
this.subOpFactory = subOpFactory;
this.workQueue = workQueue;
this.requestScopePropagator = requestScopePropagator;
- this.allProjectsName = allProjectsName;
this.indexer = indexer;
this.mergeValidatorsFactory = mergeValidatorsFactory;
destBranch = branch;
@@ -500,52 +496,6 @@
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++;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/MergeValidators.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/MergeValidators.java
index 6fb75d8..6eed909 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/MergeValidators.java
+++ b/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 @@
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 @@
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 {
