Revert should use changeInserter for easier maintenence.

Had to move ApprovalsUtil and ChangeInserter bindings from
GerritRequestModule to GerritGlobalModule to get around
Guice injection issues.

Change-Id: Iffaf87bdc921db8d3e6ed15cb7639ff1a57a16ee
This commit is contained in:
Gustaf Lundh
2013-03-12 14:00:08 +01:00
committed by David Pursehouse
parent 0a5d029d7a
commit c8cdf69fcf
4 changed files with 22 additions and 34 deletions

View File

@@ -16,7 +16,9 @@ package com.google.gerrit.server;
import com.google.common.base.CharMatcher; import com.google.common.base.CharMatcher;
import com.google.gerrit.common.ChangeHooks; import com.google.gerrit.common.ChangeHooks;
import com.google.gerrit.common.data.LabelTypes;
import com.google.gerrit.common.errors.EmailException; import com.google.gerrit.common.errors.EmailException;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.ChangeMessage; import com.google.gerrit.reviewdb.client.ChangeMessage;
import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.PatchSet;
@@ -25,6 +27,7 @@ import com.google.gerrit.reviewdb.client.PatchSetInfo;
import com.google.gerrit.reviewdb.client.RevId; import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.reviewdb.client.TrackingId; import com.google.gerrit.reviewdb.client.TrackingId;
import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.change.ChangeInserter;
import com.google.gerrit.server.change.ChangeMessages; import com.google.gerrit.server.change.ChangeMessages;
import com.google.gerrit.server.config.TrackingFooter; import com.google.gerrit.server.config.TrackingFooter;
import com.google.gerrit.server.config.TrackingFooters; import com.google.gerrit.server.config.TrackingFooters;
@@ -199,9 +202,9 @@ public class ChangeUtil {
IdentifiedUser user, CommitValidators commitValidators, String message, IdentifiedUser user, CommitValidators commitValidators, String message,
ReviewDb db, RevertedSender.Factory revertedSenderFactory, ReviewDb db, RevertedSender.Factory revertedSenderFactory,
ChangeHooks hooks, Repository git, ChangeHooks hooks, Repository git,
PatchSetInfoFactory patchSetInfoFactory, PatchSetInfoFactory patchSetInfoFactory, PersonIdent myIdent,
GitReferenceUpdated gitRefUpdated, PersonIdent myIdent, ChangeInserter changeInserter)
String canonicalWebUrl) throws NoSuchChangeException, EmailException, throws NoSuchChangeException, EmailException,
OrmException, MissingObjectException, IncorrectObjectTypeException, OrmException, MissingObjectException, IncorrectObjectTypeException,
IOException, InvalidChangeOperationException { IOException, InvalidChangeOperationException {
final Change.Id changeId = patchSetId.getParentKey(); final Change.Id changeId = patchSetId.getParentKey();
@@ -275,9 +278,11 @@ public class ChangeUtil {
throw new InvalidChangeOperationException(e.getMessage()); throw new InvalidChangeOperationException(e.getMessage());
} }
change.setCurrentPatchSet(patchSetInfoFactory.get(revertCommit, ps.getId())); PatchSetInfo info = patchSetInfoFactory.get(revertCommit, ps.getId());
change.setCurrentPatchSet(info);
ChangeUtil.updated(change); ChangeUtil.updated(change);
final RefUpdate ru = git.updateRef(ps.getRefName()); final RefUpdate ru = git.updateRef(ps.getRefName());
ru.setExpectedOldObjectId(ObjectId.zeroId()); ru.setExpectedOldObjectId(ObjectId.zeroId());
ru.setNewObjectId(revertCommit); ru.setNewObjectId(revertCommit);
@@ -287,17 +292,6 @@ public class ChangeUtil {
"Failed to create ref %s in %s: %s", ps.getRefName(), "Failed to create ref %s in %s: %s", ps.getRefName(),
change.getDest().getParentKey().get(), ru.getResult())); change.getDest().getParentKey().get(), ru.getResult()));
} }
gitRefUpdated.fire(change.getProject(), ru);
db.changes().beginTransaction(change.getId());
try {
insertAncestors(db, ps.getId(), revertCommit);
db.patchSets().insert(Collections.singleton(ps));
db.changes().insert(Collections.singleton(change));
db.commit();
} finally {
db.rollback();
}
final ChangeMessage cmsg = final ChangeMessage cmsg =
new ChangeMessage(new ChangeMessage.Key(changeId, new ChangeMessage(new ChangeMessage.Key(changeId,
@@ -306,17 +300,18 @@ public class ChangeUtil {
new StringBuilder("Patch Set " + patchSetId.get() + ": Reverted"); new StringBuilder("Patch Set " + patchSetId.get() + ": Reverted");
msgBuf.append("\n\n"); msgBuf.append("\n\n");
msgBuf.append("This patchset was reverted in change: " + change.getKey().get()); msgBuf.append("This patchset was reverted in change: " + change.getKey().get());
cmsg.setMessage(msgBuf.toString()); cmsg.setMessage(msgBuf.toString());
db.changeMessages().insert(Collections.singleton(cmsg));
LabelTypes labelTypes = refControl.getProjectControl().getLabelTypes();
changeInserter.insertChange(db, change, cmsg, ps, revertCommit,
labelTypes, revertCommit.getFooterLines(), info,
Collections.<Account.Id> emptySet());
final RevertedSender cm = revertedSenderFactory.create(change); final RevertedSender cm = revertedSenderFactory.create(change);
cm.setFrom(user.getAccountId()); cm.setFrom(user.getAccountId());
cm.setChangeMessage(cmsg); cm.setChangeMessage(cmsg);
cm.send(); cm.send();
hooks.doPatchsetCreatedHook(change, ps, db);
return change.getId(); return change.getId();
} finally { } finally {
revWalk.release(); revWalk.release();

View File

@@ -27,8 +27,6 @@ import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.GerritPersonIdent; import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.change.Revert.Input; 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.GitRepositoryManager;
import com.google.gerrit.server.git.validators.CommitValidators; import com.google.gerrit.server.git.validators.CommitValidators;
import com.google.gerrit.server.mail.RevertedSender; import com.google.gerrit.server.mail.RevertedSender;
@@ -42,8 +40,6 @@ import com.google.inject.Provider;
import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.lib.Repository;
import javax.annotation.Nullable;
public class Revert implements RestModifyView<ChangeResource, Input> { public class Revert implements RestModifyView<ChangeResource, Input> {
private final ChangeHooks hooks; private final ChangeHooks hooks;
private final RevertedSender.Factory revertedSenderFactory; private final RevertedSender.Factory revertedSenderFactory;
@@ -53,8 +49,7 @@ public class Revert implements RestModifyView<ChangeResource, Input> {
private final GitRepositoryManager gitManager; private final GitRepositoryManager gitManager;
private final PersonIdent myIdent; private final PersonIdent myIdent;
private final PatchSetInfoFactory patchSetInfoFactory; private final PatchSetInfoFactory patchSetInfoFactory;
private final GitReferenceUpdated gitRefUpdated; private final ChangeInserter changeInserter;
private final String canonicalWebUrl;
public static class Input { public static class Input {
public String message; public String message;
@@ -68,9 +63,8 @@ public class Revert implements RestModifyView<ChangeResource, Input> {
ChangeJson json, ChangeJson json,
GitRepositoryManager gitManager, GitRepositoryManager gitManager,
final PatchSetInfoFactory patchSetInfoFactory, final PatchSetInfoFactory patchSetInfoFactory,
final GitReferenceUpdated gitRefUpdated,
@GerritPersonIdent final PersonIdent myIdent, @GerritPersonIdent final PersonIdent myIdent,
@CanonicalWebUrl @Nullable final String canonicalWebUrl) { final ChangeInserter changeInserter) {
this.hooks = hooks; this.hooks = hooks;
this.revertedSenderFactory = revertedSenderFactory; this.revertedSenderFactory = revertedSenderFactory;
this.commitValidatorsFactory = commitValidatorsFactory; this.commitValidatorsFactory = commitValidatorsFactory;
@@ -78,9 +72,8 @@ public class Revert implements RestModifyView<ChangeResource, Input> {
this.json = json; this.json = json;
this.gitManager = gitManager; this.gitManager = gitManager;
this.myIdent = myIdent; this.myIdent = myIdent;
this.gitRefUpdated = gitRefUpdated; this.changeInserter = changeInserter;
this.patchSetInfoFactory = patchSetInfoFactory; this.patchSetInfoFactory = patchSetInfoFactory;
this.canonicalWebUrl = canonicalWebUrl;
} }
@Override @Override
@@ -104,7 +97,7 @@ public class Revert implements RestModifyView<ChangeResource, Input> {
commitValidators, commitValidators,
Strings.emptyToNull(input.message), dbProvider.get(), Strings.emptyToNull(input.message), dbProvider.get(),
revertedSenderFactory, hooks, git, patchSetInfoFactory, revertedSenderFactory, hooks, git, patchSetInfoFactory,
gitRefUpdated, myIdent, canonicalWebUrl); myIdent, changeInserter);
return json.format(revertedChangeId); return json.format(revertedChangeId);
} catch (InvalidChangeOperationException e) { } catch (InvalidChangeOperationException e) {

View File

@@ -28,6 +28,7 @@ import com.google.gerrit.reviewdb.client.AuthType;
import com.google.gerrit.rules.PrologModule; import com.google.gerrit.rules.PrologModule;
import com.google.gerrit.rules.RulesCache; import com.google.gerrit.rules.RulesCache;
import com.google.gerrit.server.AnonymousUser; import com.google.gerrit.server.AnonymousUser;
import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.FileTypeRegistry; import com.google.gerrit.server.FileTypeRegistry;
import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.InternalUser; import com.google.gerrit.server.InternalUser;
@@ -63,6 +64,7 @@ import com.google.gerrit.server.auth.UniversalAuthBackend;
import com.google.gerrit.server.auth.ldap.LdapModule; import com.google.gerrit.server.auth.ldap.LdapModule;
import com.google.gerrit.server.avatar.AvatarProvider; import com.google.gerrit.server.avatar.AvatarProvider;
import com.google.gerrit.server.cache.CacheRemovalListener; import com.google.gerrit.server.cache.CacheRemovalListener;
import com.google.gerrit.server.change.ChangeInserter;
import com.google.gerrit.server.events.EventFactory; import com.google.gerrit.server.events.EventFactory;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated; import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
import com.google.gerrit.server.git.ChangeCache; import com.google.gerrit.server.git.ChangeCache;
@@ -209,6 +211,8 @@ public class GerritGlobalModule extends FactoryModule {
bind(EventFactory.class); bind(EventFactory.class);
bind(TransferConfig.class); bind(TransferConfig.class);
bind(ApprovalsUtil.class);
bind(ChangeInserter.class);
bind(ChangeMergeQueue.class).in(SINGLETON); bind(ChangeMergeQueue.class).in(SINGLETON);
bind(MergeQueue.class).to(ChangeMergeQueue.class).in(SINGLETON); bind(MergeQueue.class).to(ChangeMergeQueue.class).in(SINGLETON);
factory(ReloadSubmitQueueOp.Factory.class); factory(ReloadSubmitQueueOp.Factory.class);

View File

@@ -16,10 +16,8 @@ package com.google.gerrit.server.config;
import static com.google.inject.Scopes.SINGLETON; import static com.google.inject.Scopes.SINGLETON;
import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.RequestCleanup; import com.google.gerrit.server.RequestCleanup;
import com.google.gerrit.server.change.ChangeInserter;
import com.google.gerrit.server.changedetail.DeleteDraftPatchSet; import com.google.gerrit.server.changedetail.DeleteDraftPatchSet;
import com.google.gerrit.server.changedetail.PublishDraft; import com.google.gerrit.server.changedetail.PublishDraft;
import com.google.gerrit.server.git.BanCommit; import com.google.gerrit.server.git.BanCommit;
@@ -39,8 +37,6 @@ public class GerritRequestModule extends FactoryModule {
bind(RequestCleanup.class).in(RequestScoped.class); bind(RequestCleanup.class).in(RequestScoped.class);
bind(RequestScopedReviewDbProvider.class); bind(RequestScopedReviewDbProvider.class);
bind(IdentifiedUser.RequestFactory.class).in(SINGLETON); bind(IdentifiedUser.RequestFactory.class).in(SINGLETON);
bind(ApprovalsUtil.class);
bind(ChangeInserter.class);
bind(PerRequestProjectControlCache.class).in(RequestScoped.class); bind(PerRequestProjectControlCache.class).in(RequestScoped.class);
bind(ChangeControl.Factory.class).in(SINGLETON); bind(ChangeControl.Factory.class).in(SINGLETON);