MergeValidationListener: Also provide RevWalk to the onPreMerge method

This way implementors that need a RevWalk do not need to create a new
instance. The callers have the RevWalk anyway available, so it's no
extra effort for them to provide it.

This makes the objects that are available to implementors of
MergeValidationListener more consistent with the objects that are
available to implementors of CommitValidationListener, which already
contains the RevWalk inside of CommitReceivedEvent.

Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: Id93e378be10c91d3dc3365aec0bab9847ee8ef6d
(cherry picked from commit f57cdd0b5d)
This commit is contained in:
Edwin Kempin
2020-10-06 15:57:13 +02:00
committed by Luca Milanesio
parent 21425d0d4d
commit dc7ad8f299
4 changed files with 22 additions and 12 deletions

View File

@@ -19,6 +19,7 @@ import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.extensions.annotations.ExtensionPoint; import com.google.gerrit.extensions.annotations.ExtensionPoint;
import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.git.CodeReviewCommit; import com.google.gerrit.server.git.CodeReviewCommit;
import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk;
import com.google.gerrit.server.project.ProjectState; import com.google.gerrit.server.project.ProjectState;
import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.lib.Repository;
@@ -33,6 +34,7 @@ public interface MergeValidationListener {
* Validate a commit before it is merged. * Validate a commit before it is merged.
* *
* @param repo the repository * @param repo the repository
* @param revWalk the rev walk
* @param commit commit details * @param commit commit details
* @param destProject the destination project * @param destProject the destination project
* @param destBranch the destination branch * @param destBranch the destination branch
@@ -42,6 +44,7 @@ public interface MergeValidationListener {
*/ */
void onPreMerge( void onPreMerge(
Repository repo, Repository repo,
CodeReviewRevWalk revWalk,
CodeReviewCommit commit, CodeReviewCommit commit,
ProjectState destProject, ProjectState destProject,
BranchNameKey destBranch, BranchNameKey destBranch,

View File

@@ -35,6 +35,7 @@ import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.PluginConfig; import com.google.gerrit.server.config.PluginConfig;
import com.google.gerrit.server.config.ProjectConfigEntry; import com.google.gerrit.server.config.ProjectConfigEntry;
import com.google.gerrit.server.git.CodeReviewCommit; import com.google.gerrit.server.git.CodeReviewCommit;
import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk;
import com.google.gerrit.server.permissions.GlobalPermission; import com.google.gerrit.server.permissions.GlobalPermission;
import com.google.gerrit.server.permissions.PermissionBackend; import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException; import com.google.gerrit.server.permissions.PermissionBackendException;
@@ -51,7 +52,6 @@ import java.util.Objects;
import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevWalk;
/** /**
* Collection of validators that run inside Gerrit before a change is submitted. The main purpose is * Collection of validators that run inside Gerrit before a change is submitted. The main purpose is
@@ -92,6 +92,7 @@ public class MergeValidators {
*/ */
public void validatePreMerge( public void validatePreMerge(
Repository repo, Repository repo,
CodeReviewRevWalk revWalk,
CodeReviewCommit commit, CodeReviewCommit commit,
ProjectState destProject, ProjectState destProject,
BranchNameKey destBranch, BranchNameKey destBranch,
@@ -106,7 +107,7 @@ public class MergeValidators {
groupValidatorFactory.create()); groupValidatorFactory.create());
for (MergeValidationListener validator : validators) { for (MergeValidationListener validator : validators) {
validator.onPreMerge(repo, commit, destProject, destBranch, patchSetId, caller); validator.onPreMerge(repo, revWalk, commit, destProject, destBranch, patchSetId, caller);
} }
} }
@@ -168,11 +169,12 @@ public class MergeValidators {
@Override @Override
public void onPreMerge( public void onPreMerge(
final Repository repo, Repository repo,
final CodeReviewCommit commit, CodeReviewRevWalk revWalk,
final ProjectState destProject, CodeReviewCommit commit,
final BranchNameKey destBranch, ProjectState destProject,
final PatchSet.Id patchSetId, BranchNameKey destBranch,
PatchSet.Id patchSetId,
IdentifiedUser caller) IdentifiedUser caller)
throws MergeValidationException { throws MergeValidationException {
if (RefNames.REFS_CONFIG.equals(destBranch.branch())) { if (RefNames.REFS_CONFIG.equals(destBranch.branch())) {
@@ -260,6 +262,7 @@ public class MergeValidators {
@Override @Override
public void onPreMerge( public void onPreMerge(
Repository repo, Repository repo,
CodeReviewRevWalk revWalk,
CodeReviewCommit commit, CodeReviewCommit commit,
ProjectState destProject, ProjectState destProject,
BranchNameKey destBranch, BranchNameKey destBranch,
@@ -267,7 +270,7 @@ public class MergeValidators {
IdentifiedUser caller) IdentifiedUser caller)
throws MergeValidationException { throws MergeValidationException {
mergeValidationListeners.runEach( mergeValidationListeners.runEach(
l -> l.onPreMerge(repo, commit, destProject, destBranch, patchSetId, caller), l -> l.onPreMerge(repo, revWalk, commit, destProject, destBranch, patchSetId, caller),
MergeValidationException.class); MergeValidationException.class);
} }
} }
@@ -294,6 +297,7 @@ public class MergeValidators {
@Override @Override
public void onPreMerge( public void onPreMerge(
Repository repo, Repository repo,
CodeReviewRevWalk revWalk,
CodeReviewCommit commit, CodeReviewCommit commit,
ProjectState destProject, ProjectState destProject,
BranchNameKey destBranch, BranchNameKey destBranch,
@@ -316,8 +320,9 @@ public class MergeValidators {
throw new MergeValidationException("account validation unavailable"); throw new MergeValidationException("account validation unavailable");
} }
try (RevWalk rw = new RevWalk(repo)) { try {
List<String> errorMessages = accountValidator.validate(accountId, repo, rw, null, commit); List<String> errorMessages =
accountValidator.validate(accountId, repo, revWalk, null, commit);
if (!errorMessages.isEmpty()) { if (!errorMessages.isEmpty()) {
throw new MergeValidationException( throw new MergeValidationException(
"invalid account configuration: " + Joiner.on("; ").join(errorMessages)); "invalid account configuration: " + Joiner.on("; ").join(errorMessages));
@@ -345,6 +350,7 @@ public class MergeValidators {
@Override @Override
public void onPreMerge( public void onPreMerge(
Repository repo, Repository repo,
CodeReviewRevWalk revWalk,
CodeReviewCommit commit, CodeReviewCommit commit,
ProjectState destProject, ProjectState destProject,
BranchNameKey destBranch, BranchNameKey destBranch,

View File

@@ -864,7 +864,8 @@ public class MergeOp implements AutoCloseable {
MergeValidators mergeValidators = mergeValidatorsFactory.create(); MergeValidators mergeValidators = mergeValidatorsFactory.create();
try { try {
mergeValidators.validatePreMerge(or.repo, commit, or.project, destBranch, ps.id(), caller); mergeValidators.validatePreMerge(
or.repo, or.rw, commit, or.project, destBranch, ps.id(), caller);
} catch (MergeValidationException mve) { } catch (MergeValidationException mve) {
commitStatus.problem(changeId, mve.getMessage()); commitStatus.problem(changeId, mve.getMessage());
continue; continue;