From 8f9e9f58e2c3ad65c30fecbc136b375e7979e7b1 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Thu, 30 May 2013 10:35:24 +0900 Subject: [PATCH] Add merge validation interface Add a new MergeValidationListener interface which can be used to provide additional validation of commits before they are merged to the git repository. Add a MergeValidators class to invoke the merge validator listeners, and call it from MergeOp before the commit is merged. Add a listener to invoke validation provided by plugins. Change-Id: I325d923f5cc0245b60e86a035329b640c1682d48 --- Documentation/config-validation.txt | 17 ++++ .../server/config/GerritGlobalModule.java | 4 + .../com/google/gerrit/server/git/MergeOp.java | 15 +++- .../validators/MergeValidationException.java | 31 ++++++++ .../validators/MergeValidationListener.java | 48 ++++++++++++ .../git/validators/MergeValidators.java | 78 +++++++++++++++++++ 6 files changed, 192 insertions(+), 1 deletion(-) create mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/git/validators/MergeValidationException.java create mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/git/validators/MergeValidationListener.java create mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/git/validators/MergeValidators.java diff --git a/Documentation/config-validation.txt b/Documentation/config-validation.txt index fd8c0bd3a6..1b09d1943b 100644 --- a/Documentation/config-validation.txt +++ b/Documentation/config-validation.txt @@ -4,6 +4,11 @@ Gerrit Code Review - Commit Validation Gerrit supports link:dev-plugins.html[plugin-based] validation of commits. +[[new-commit-validation]] +New commit validation +--------------------- + + Plugins implementing the `CommitValidationListener` interface can perform additional validation checks against new commits. @@ -18,6 +23,18 @@ and cherry-pick buttons. Out of the box, Gerrit includes a plugin that checks the length of the subject and body lines of commit messages on uploaded commits. +[[pre-merge-validation]] +Pre-merge validation +-------------------- + + +Plugins implementing the `MergeValidationListener` interface can +perform additional validation checks against commits before they +are merged to the git repository. + +If the commit fails the validation, the plugin can throw an exception +which will cause the merge to fail. + GERRIT ------ 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 7ccb33f9d2..8aeaa3d8bf 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 @@ -80,6 +80,8 @@ import com.google.gerrit.server.git.TagCache; import com.google.gerrit.server.git.TransferConfig; 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.mail.AddReviewerSender; import com.google.gerrit.server.mail.CommitMessageEditedSender; import com.google.gerrit.server.mail.CreateChangeSender; @@ -251,11 +253,13 @@ public class GerritGlobalModule extends FactoryModule { DynamicSet.bind(binder(), GitReferenceUpdatedListener.class).to(ChangeCache.class); DynamicSet.setOf(binder(), ChangeListener.class); DynamicSet.setOf(binder(), CommitValidationListener.class); + DynamicSet.setOf(binder(), MergeValidationListener.class); DynamicItem.itemOf(binder(), AvatarProvider.class); bind(AnonymousUser.class); factory(CommitValidators.Factory.class); + factory(MergeValidators.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 ff3fe33921..d8ae6cac8e 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 @@ -43,6 +43,8 @@ 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; import com.google.gerrit.server.index.ChangeIndexer; import com.google.gerrit.server.mail.MergeFailSender; import com.google.gerrit.server.mail.MergedSender; @@ -130,6 +132,7 @@ public class MergeOp { private final IdentifiedUser.GenericFactory identifiedUserFactory; private final ChangeControl.GenericFactory changeControlFactory; private final MergeQueue mergeQueue; + private final MergeValidators.Factory mergeValidatorsFactory; private final Branch.NameKey destBranch; private ProjectState destProject; @@ -170,7 +173,8 @@ public class MergeOp { final WorkQueue workQueue, final RequestScopePropagator requestScopePropagator, final AllProjectsName allProjectsName, - final ChangeIndexer indexer) { + final ChangeIndexer indexer, + final MergeValidators.Factory mergeValidatorsFactory) { repoManager = grm; schemaFactory = sf; labelNormalizer = fs; @@ -191,6 +195,7 @@ public class MergeOp { this.requestScopePropagator = requestScopePropagator; this.allProjectsName = allProjectsName; this.indexer = indexer; + this.mergeValidatorsFactory = mergeValidatorsFactory; destBranch = branch; toMerge = ArrayListMultimap.create(); potentiallyStillSubmittable = new ArrayList(); @@ -487,6 +492,14 @@ public class MergeOp { continue; } + MergeValidators mergeValidators = mergeValidatorsFactory.create(); + try { + mergeValidators.validatePreMerge(repo, commit, destProject, destBranch, ps.getId()); + } catch (MergeValidationException mve) { + commits.put(changeId, CodeReviewCommit.error(mve.getStatus())); + continue; + } + if (GitRepositoryManager.REF_CONFIG.equals(destBranch.get())) { final Project.NameKey newParent; try { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/MergeValidationException.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/MergeValidationException.java new file mode 100644 index 0000000000..78819a80d8 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/MergeValidationException.java @@ -0,0 +1,31 @@ +// Copyright (C) 2013 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.server.git.validators; + +import com.google.gerrit.server.git.CommitMergeStatus; + +public class MergeValidationException extends Exception { + private static final long serialVersionUID = 1L; + private final CommitMergeStatus status; + + public MergeValidationException(CommitMergeStatus status) { + super(status.toString()); + this.status = status; + } + + public CommitMergeStatus getStatus() { + return status; + } +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/MergeValidationListener.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/MergeValidationListener.java new file mode 100644 index 0000000000..0a8d245089 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/MergeValidationListener.java @@ -0,0 +1,48 @@ +// Copyright (C) 2013 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.server.git.validators; + +import com.google.gerrit.extensions.annotations.ExtensionPoint; +import com.google.gerrit.reviewdb.client.Branch; +import com.google.gerrit.reviewdb.client.PatchSet; +import com.google.gerrit.server.git.CodeReviewCommit; +import com.google.gerrit.server.project.ProjectState; + +import org.eclipse.jgit.lib.Repository; + +/** + * Listener to provide validation of commits before merging. + * + * Invoked by Gerrit before a commit is merged. + */ +@ExtensionPoint +public interface MergeValidationListener { + /** + * Validate a commit before it is merged. + * + * @param repo the repository + * @param commit commit details + * @param destProject the destination project + * @param destBranch the destination branch + * @param patchSetId the patch set ID + * @throws MergeValidationException if the commit fails to validate + */ + public void onPreMerge(Repository repo, + CodeReviewCommit commit, + ProjectState destProject, + Branch.NameKey destBranch, + PatchSet.Id patchSetId) + throws MergeValidationException; +} 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 new file mode 100644 index 0000000000..6fb75d8aa0 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/MergeValidators.java @@ -0,0 +1,78 @@ +// Copyright (C) 2013 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.server.git.validators; + +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.server.git.CodeReviewCommit; +import com.google.gerrit.server.project.ProjectState; +import com.google.inject.Inject; + +import org.eclipse.jgit.lib.Repository; + +import java.util.List; + +public class MergeValidators { + private final DynamicSet mergeValidationListeners; + + public interface Factory { + MergeValidators create(); + } + + @Inject + MergeValidators(DynamicSet mergeValidationListeners) { + this.mergeValidationListeners = mergeValidationListeners; + } + + public void validatePreMerge(Repository repo, + CodeReviewCommit commit, + ProjectState destProject, + Branch.NameKey destBranch, + PatchSet.Id patchSetId) + throws MergeValidationException { + List validators = Lists.newLinkedList(); + + validators.add(new PluginMergeValidationListener(mergeValidationListeners)); + + for (MergeValidationListener validator : validators) { + validator.onPreMerge(repo, commit, destProject, destBranch, patchSetId); + } + } + + /** Execute merge validation plug-ins */ + public static class PluginMergeValidationListener implements + MergeValidationListener { + private final DynamicSet mergeValidationListeners; + + public PluginMergeValidationListener( + DynamicSet mergeValidationListeners) { + this.mergeValidationListeners = mergeValidationListeners; + } + + @Override + public void onPreMerge(Repository repo, + CodeReviewCommit commit, + ProjectState destProject, + Branch.NameKey destBranch, + PatchSet.Id patchSetId) + throws MergeValidationException { + for (MergeValidationListener validator : mergeValidationListeners) { + validator.onPreMerge(repo, commit, destProject, destBranch, patchSetId); + } + } + } +}