diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java index d20da3c58c..ede8e74d8a 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java @@ -31,9 +31,9 @@ import com.google.gerrit.server.events.CommitReceivedEvent; import com.google.gerrit.server.extensions.events.GitReferenceUpdated; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.MergeOp; -import com.google.gerrit.server.mail.CommitMessageEditedSender; import com.google.gerrit.server.git.validators.CommitValidationException; import com.google.gerrit.server.git.validators.CommitValidators; +import com.google.gerrit.server.mail.CommitMessageEditedSender; import com.google.gerrit.server.mail.RevertedSender; import com.google.gerrit.server.patch.PatchSetInfoFactory; import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException; @@ -47,7 +47,6 @@ import com.google.gwtorm.server.OrmException; import org.eclipse.jgit.errors.IncorrectObjectTypeException; import org.eclipse.jgit.errors.MissingObjectException; -import org.eclipse.jgit.errors.RepositoryNotFoundException; import org.eclipse.jgit.lib.CommitBuilder; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectInserter; @@ -192,14 +191,17 @@ public class ChangeUtil { db.patchSetAncestors().insert(toInsert); } - public static Change.Id revert(final PatchSet.Id patchSetId, - final IdentifiedUser user, final String message, final ReviewDb db, + public static Change.Id revert(final RefControl refControl, + final PatchSet.Id patchSetId, final IdentifiedUser user, + final CommitValidators commitValidators, + final String message, final ReviewDb db, final RevertedSender.Factory revertedSenderFactory, - final ChangeHooks hooks, GitRepositoryManager gitManager, + final ChangeHooks hooks, Repository git, final PatchSetInfoFactory patchSetInfoFactory, - final GitReferenceUpdated replication, PersonIdent myIdent) - throws NoSuchChangeException, EmailException, OrmException, - MissingObjectException, IncorrectObjectTypeException, IOException { + final GitReferenceUpdated replication, PersonIdent myIdent, + String canonicalWebUrl) throws NoSuchChangeException, EmailException, + OrmException, MissingObjectException, IncorrectObjectTypeException, + IOException, InvalidChangeOperationException { final Change.Id changeId = patchSetId.getParentKey(); final PatchSet patch = db.patchSets().get(patchSetId); if (patch == null) { @@ -207,13 +209,6 @@ public class ChangeUtil { } final Change changeToRevert = db.changes().get(changeId); - final Repository git; - try { - git = gitManager.openRepository(changeToRevert.getProject()); - } catch (RepositoryNotFoundException e) { - throw new NoSuchChangeException(changeId, e); - } - final RevWalk revWalk = new RevWalk(git); try { RevCommit commitToRevert = @@ -260,6 +255,18 @@ public class ChangeUtil { ps.setUploader(change.getOwner()); ps.setRevision(new RevId(revertCommit.name())); + CommitReceivedEvent commitReceivedEvent = + new CommitReceivedEvent(new ReceiveCommand(ObjectId.zeroId(), + revertCommit.getId(), ps.getRefName()), refControl + .getProjectControl().getProject(), refControl.getRefName(), + revertCommit, user); + + try { + commitValidators.validateForRevertCommits(commitReceivedEvent); + } catch (CommitValidationException e) { + throw new InvalidChangeOperationException(e.getMessage()); + } + change.setCurrentPatchSet(patchSetInfoFactory.get(revertCommit, ps.getId())); ChangeUtil.updated(change); @@ -305,7 +312,6 @@ public class ChangeUtil { return change.getId(); } finally { revWalk.release(); - git.close(); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Revert.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Revert.java index de7cfd40dc..3614b37ba4 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Revert.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Revert.java @@ -17,6 +17,7 @@ package com.google.gerrit.server.change; import com.google.common.base.Strings; import com.google.gerrit.common.ChangeHooks; import com.google.gerrit.extensions.restapi.AuthException; +import com.google.gerrit.extensions.restapi.BadRequestException; import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.extensions.restapi.RestModifyView; import com.google.gerrit.reviewdb.client.Change; @@ -26,25 +27,34 @@ import com.google.gerrit.server.ChangeUtil; import com.google.gerrit.server.GerritPersonIdent; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.change.Revert.Input; +import com.google.gerrit.server.config.CanonicalWebUrl; import com.google.gerrit.server.extensions.events.GitReferenceUpdated; import com.google.gerrit.server.git.GitRepositoryManager; +import com.google.gerrit.server.git.validators.CommitValidators; import com.google.gerrit.server.mail.RevertedSender; import com.google.gerrit.server.patch.PatchSetInfoFactory; import com.google.gerrit.server.project.ChangeControl; +import com.google.gerrit.server.project.InvalidChangeOperationException; +import com.google.gerrit.server.ssh.NoSshInfo; import com.google.inject.Inject; import com.google.inject.Provider; import org.eclipse.jgit.lib.PersonIdent; +import org.eclipse.jgit.lib.Repository; + +import javax.annotation.Nullable; public class Revert implements RestModifyView { private final ChangeHooks hooks; private final RevertedSender.Factory revertedSenderFactory; + private final CommitValidators.Factory commitValidatorsFactory; private final Provider dbProvider; private final ChangeJson json; private final GitRepositoryManager gitManager; private final PersonIdent myIdent; private final PatchSetInfoFactory patchSetInfoFactory; private final GitReferenceUpdated replication; + private final String canonicalWebUrl; public static class Input { public String message; @@ -53,20 +63,24 @@ public class Revert implements RestModifyView { @Inject Revert(ChangeHooks hooks, RevertedSender.Factory revertedSenderFactory, + final CommitValidators.Factory commitValidatorsFactory, Provider dbProvider, ChangeJson json, GitRepositoryManager gitManager, final PatchSetInfoFactory patchSetInfoFactory, final GitReferenceUpdated replication, - @GerritPersonIdent final PersonIdent myIdent) { + @GerritPersonIdent final PersonIdent myIdent, + @CanonicalWebUrl @Nullable final String canonicalWebUrl) { this.hooks = hooks; this.revertedSenderFactory = revertedSenderFactory; + this.commitValidatorsFactory = commitValidatorsFactory; this.dbProvider = dbProvider; this.json = json; this.gitManager = gitManager; this.myIdent = myIdent; this.replication = replication; this.patchSetInfoFactory = patchSetInfoFactory; + this.canonicalWebUrl = canonicalWebUrl; } @Override @@ -75,8 +89,7 @@ public class Revert implements RestModifyView { } @Override - public Object apply(ChangeResource req, Input input) - throws Exception { + public Object apply(ChangeResource req, Input input) throws Exception { ChangeControl control = req.getControl(); Change change = req.getChange(); if (!control.canAddPatchSet()) { @@ -85,14 +98,25 @@ public class Revert implements RestModifyView { throw new ResourceConflictException("change is " + status(change)); } - Change.Id revertedChangeId = ChangeUtil.revert( - change.currentPatchSetId(), - (IdentifiedUser) control.getCurrentUser(), - Strings.emptyToNull(input.message), - dbProvider.get(), - revertedSenderFactory, hooks, gitManager, - patchSetInfoFactory, replication, myIdent); - return json.format(revertedChangeId); + final Repository git = gitManager.openRepository(control.getProject().getNameKey()); + try { + CommitValidators commitValidators = + commitValidatorsFactory.create(control.getRefControl(), new NoSshInfo(), git); + + Change.Id revertedChangeId = + ChangeUtil.revert(control.getRefControl(), change.currentPatchSetId(), + (IdentifiedUser) control.getCurrentUser(), + commitValidators, + Strings.emptyToNull(input.message), dbProvider.get(), + revertedSenderFactory, hooks, git, patchSetInfoFactory, + replication, myIdent, canonicalWebUrl); + + return json.format(revertedChangeId); + } catch (InvalidChangeOperationException e) { + throw new BadRequestException(e.getMessage()); + } finally { + git.close(); + } } private static String status(Change change) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/CommitValidators.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/CommitValidators.java index c626663b87..a53548b325 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/CommitValidators.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/CommitValidators.java @@ -117,6 +117,36 @@ public class CommitValidators { return messages; } + public List validateForRevertCommits( + CommitReceivedEvent receiveEvent) throws CommitValidationException { + + List validators = + new LinkedList(); + + validators.add(new UploadMergesPermissionValidator(refControl)); + validators.add(new AmendedGerritMergeCommitValidationListener( + refControl, gerritIdent)); + validators.add(new AuthorUploaderValidator(refControl, canonicalWebUrl)); + validators.add(new SignedOffByValidator(refControl, canonicalWebUrl)); + validators.add(new ChangeIdValidator(refControl, canonicalWebUrl, sshInfo)); + validators.add(new ConfigValidator(refControl, repo)); + validators.add(new PluginCommitValidationListener(commitValidationListeners)); + + List messages = + new LinkedList(); + + try { + for (CommitValidationListener commitValidator : validators) { + messages.addAll(commitValidator.onCommitReceived(receiveEvent)); + } + } catch (CommitValidationException e) { + // Keep the old messages (and their order) in case of an exception + messages.addAll(e.getMessages()); + throw new CommitValidationException(e.getMessage(), messages); + } + return messages; + } + public static class ChangeIdValidator implements CommitValidationListener { private final RefControl refControl; private final String canonicalWebUrl;