Tweak PatchSetInserter to look more like ChangeInserter

Don't pass in the repo and RevWalk, since those should come from the
RepoContext. Change callers to use the same Repository/RevWalk/
ObjectInserter instances and call setRepository on the enclosing
BatchUpdate.

Don't lazily compute patch set ID; have callers compute it themselves
and pass it in at construction time, since we no longer have a
Repository instance in the PatchSetInserter.

Change-Id: I845b887ae7ffc2c1a14bad13c62c41ff1ff6fc34
This commit is contained in:
Dave Borowitz
2015-10-19 14:02:55 -04:00
parent 15eb1e58fd
commit 0b73d356c5
10 changed files with 114 additions and 87 deletions

View File

@@ -176,7 +176,7 @@ public class CherryPickChange {
} else if (destChanges.size() == 1) { } else if (destChanges.size() == 1) {
// The change key exists on the destination branch. The cherry pick // The change key exists on the destination branch. The cherry pick
// will be added as a new patch set. // will be added as a new patch set.
return insertPatchSet(git, revWalk, destChanges.get(0).change(), return insertPatchSet(git, revWalk, oi, destChanges.get(0).change(),
cherryPickCommit, refControl, identifiedUser); cherryPickCommit, refControl, identifiedUser);
} else { } else {
// Change key not found on destination branch. We can create a new // Change key not found on destination branch. We can create a new
@@ -203,19 +203,20 @@ public class CherryPickChange {
} }
private Change.Id insertPatchSet(Repository git, RevWalk revWalk, private Change.Id insertPatchSet(Repository git, RevWalk revWalk,
Change change, CodeReviewCommit cherryPickCommit, RefControl refControl, ObjectInserter oi, Change change, CodeReviewCommit cherryPickCommit,
IdentifiedUser identifiedUser) RefControl refControl, IdentifiedUser identifiedUser)
throws IOException, OrmException, UpdateException, RestApiException { throws IOException, OrmException, UpdateException, RestApiException {
final ChangeControl changeControl = PatchSet.Id psId =
refControl.getProjectControl().controlFor(change); ChangeUtil.nextPatchSetId(git, change.currentPatchSetId());
final PatchSetInserter inserter = patchSetInserterFactory PatchSetInserter inserter = patchSetInserterFactory
.create(git, revWalk, changeControl, cherryPickCommit); .create(refControl, psId, cherryPickCommit);
final PatchSet.Id newPatchSetId = inserter.getPatchSetId(); PatchSet.Id newPatchSetId = inserter.getPatchSetId();
PatchSet current = db.get().patchSets().get(change.currentPatchSetId()); PatchSet current = db.get().patchSets().get(change.currentPatchSetId());
try (BatchUpdate bu = batchUpdateFactory.create( try (BatchUpdate bu = batchUpdateFactory.create(
db.get(), change.getDest().getParentKey(), identifiedUser, db.get(), change.getDest().getParentKey(), identifiedUser,
TimeUtil.nowTs())) { TimeUtil.nowTs())) {
bu.setRepository(git, revWalk, oi);
bu.addOp(change.getId(), inserter bu.addOp(change.getId(), inserter
.setMessage("Uploaded patch set " + newPatchSetId.get() + ".") .setMessage("Uploaded patch set " + newPatchSetId.get() + ".")
.setDraft(current.isDraft()) .setDraft(current.isDraft())

View File

@@ -47,8 +47,9 @@ import com.google.gerrit.server.git.UpdateException;
import com.google.gerrit.server.git.validators.CommitValidators; import com.google.gerrit.server.git.validators.CommitValidators;
import com.google.gerrit.server.patch.PatchSetInfoFactory; import com.google.gerrit.server.patch.PatchSetInfoFactory;
import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException; import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.project.NoSuchChangeException; import com.google.gerrit.server.project.ProjectControl;
import com.google.gerrit.server.project.RefControl;
import com.google.gerrit.server.query.change.ChangeData; import com.google.gerrit.server.query.change.ChangeData;
import com.google.gwtorm.server.AtomicUpdate; import com.google.gwtorm.server.AtomicUpdate;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
@@ -59,6 +60,7 @@ import org.eclipse.jgit.errors.IncorrectObjectTypeException;
import org.eclipse.jgit.errors.MissingObjectException; import org.eclipse.jgit.errors.MissingObjectException;
import org.eclipse.jgit.errors.RepositoryNotFoundException; import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectInserter;
import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.RefUpdate; import org.eclipse.jgit.lib.RefUpdate;
@@ -108,7 +110,7 @@ public class ConsistencyChecker {
private final GitRepositoryManager repoManager; private final GitRepositoryManager repoManager;
private final Provider<CurrentUser> user; private final Provider<CurrentUser> user;
private final Provider<PersonIdent> serverIdent; private final Provider<PersonIdent> serverIdent;
private final ChangeControl.GenericFactory changeControlFactory; private final ProjectControl.GenericFactory projectControlFactory;
private final PatchSetInfoFactory patchSetInfoFactory; private final PatchSetInfoFactory patchSetInfoFactory;
private final PatchSetInserter.Factory patchSetInserterFactory; private final PatchSetInserter.Factory patchSetInserterFactory;
private final BatchUpdate.Factory updateFactory; private final BatchUpdate.Factory updateFactory;
@@ -130,7 +132,7 @@ public class ConsistencyChecker {
GitRepositoryManager repoManager, GitRepositoryManager repoManager,
Provider<CurrentUser> user, Provider<CurrentUser> user,
@GerritPersonIdent Provider<PersonIdent> serverIdent, @GerritPersonIdent Provider<PersonIdent> serverIdent,
ChangeControl.GenericFactory changeControlFactory, ProjectControl.GenericFactory projectControlFactory,
PatchSetInfoFactory patchSetInfoFactory, PatchSetInfoFactory patchSetInfoFactory,
PatchSetInserter.Factory patchSetInserterFactory, PatchSetInserter.Factory patchSetInserterFactory,
BatchUpdate.Factory updateFactory) { BatchUpdate.Factory updateFactory) {
@@ -138,7 +140,7 @@ public class ConsistencyChecker {
this.repoManager = repoManager; this.repoManager = repoManager;
this.user = user; this.user = user;
this.serverIdent = serverIdent; this.serverIdent = serverIdent;
this.changeControlFactory = changeControlFactory; this.projectControlFactory = projectControlFactory;
this.patchSetInfoFactory = patchSetInfoFactory; this.patchSetInfoFactory = patchSetInfoFactory;
this.patchSetInserterFactory = patchSetInserterFactory; this.patchSetInserterFactory = patchSetInserterFactory;
this.updateFactory = updateFactory; this.updateFactory = updateFactory;
@@ -461,29 +463,33 @@ public class ConsistencyChecker {
} }
try { try {
ChangeControl ctl = changeControlFactory.controlFor(change, user.get()); RefControl ctl = projectControlFactory
.controlFor(change.getProject(), user.get())
.controlForRef(change.getDest());
PatchSet.Id psId =
ChangeUtil.nextPatchSetId(repo, change.currentPatchSetId());
PatchSetInserter inserter = PatchSetInserter inserter =
patchSetInserterFactory.create(repo, rw, ctl, commit); patchSetInserterFactory.create(ctl, psId, commit);
try (BatchUpdate bu = updateFactory.create( try (BatchUpdate bu = updateFactory.create(
db.get(), change.getDest().getParentKey(), user.get(), db.get(), change.getProject(), ctl.getUser(), TimeUtil.nowTs());
TimeUtil.nowTs())) { ObjectInserter oi = repo.newObjectInserter()) {
bu.setRepository(repo, rw, oi);
bu.addOp(change.getId(), inserter bu.addOp(change.getId(), inserter
.setValidatePolicy(CommitValidators.Policy.NONE) .setValidatePolicy(CommitValidators.Policy.NONE)
.setRunHooks(false) .setRunHooks(false)
.setSendMail(false) .setSendMail(false)
.setAllowClosed(true) .setAllowClosed(true)
.setUploader(user.get().getAccountId()) .setUploader(user.get().getAccountId())
// TODO: fix setMessage to work without init()
.setMessage( .setMessage(
"Patch set for merged commit inserted by consistency checker")); "Patch set for merged commit inserted by consistency checker"));
bu.execute(); bu.execute();
} }
change = inserter.getChange(); change = inserter.getChange();
PatchSet.Id psId = change.currentPatchSetId();
p.status = Status.FIXED; p.status = Status.FIXED;
p.outcome = "Inserted as patch set " + psId.get(); p.outcome = "Inserted as patch set " + psId.get();
return psId; return psId;
} catch (NoSuchChangeException | UpdateException | RestApiException e) { } catch (IOException | NoSuchProjectException | UpdateException
| RestApiException e) {
warn(e); warn(e);
p.status = Status.FIX_FAILED; p.status = Status.FIX_FAILED;
p.outcome = "Error inserting new patch set"; p.outcome = "Error inserting new patch set";

View File

@@ -31,7 +31,6 @@ import com.google.gerrit.server.ApprovalCopier;
import com.google.gerrit.server.ApprovalsUtil; import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.ChangeMessagesUtil; import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.ChangeUtil; import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.events.CommitReceivedEvent; import com.google.gerrit.server.events.CommitReceivedEvent;
import com.google.gerrit.server.git.BanCommit; import com.google.gerrit.server.git.BanCommit;
import com.google.gerrit.server.git.BatchUpdate; import com.google.gerrit.server.git.BatchUpdate;
@@ -47,6 +46,7 @@ import com.google.gerrit.server.patch.PatchSetInfoFactory;
import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.ChangeModifiedException; import com.google.gerrit.server.project.ChangeModifiedException;
import com.google.gerrit.server.project.InvalidChangeOperationException; import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gerrit.server.project.RefControl;
import com.google.gerrit.server.ssh.NoSshInfo; import com.google.gerrit.server.ssh.NoSshInfo;
import com.google.gerrit.server.ssh.SshInfo; import com.google.gerrit.server.ssh.SshInfo;
import com.google.gwtorm.server.AtomicUpdate; import com.google.gwtorm.server.AtomicUpdate;
@@ -55,10 +55,8 @@ import com.google.inject.assistedinject.Assisted;
import com.google.inject.assistedinject.AssistedInject; import com.google.inject.assistedinject.AssistedInject;
import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.notes.NoteMap; import org.eclipse.jgit.notes.NoteMap;
import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.transport.ReceiveCommand; import org.eclipse.jgit.transport.ReceiveCommand;
import org.slf4j.Logger; import org.slf4j.Logger;
import org.slf4j.LoggerFactory; import org.slf4j.LoggerFactory;
@@ -71,7 +69,7 @@ public class PatchSetInserter extends BatchUpdate.Op {
LoggerFactory.getLogger(PatchSetInserter.class); LoggerFactory.getLogger(PatchSetInserter.class);
public static interface Factory { public static interface Factory {
PatchSetInserter create(Repository git, RevWalk revWalk, ChangeControl ctl, PatchSetInserter create(RefControl refControl, PatchSet.Id psId,
RevCommit commit); RevCommit commit);
} }
@@ -86,14 +84,9 @@ public class PatchSetInserter extends BatchUpdate.Op {
private final ChangeMessagesUtil cmUtil; private final ChangeMessagesUtil cmUtil;
// Assisted-injected fields. // Assisted-injected fields.
private final PatchSet.Id psId;
private final RevCommit commit; private final RevCommit commit;
private final ChangeControl ctl; private final RefControl refControl;
private final IdentifiedUser user;
private final Repository git;
private final RevWalk revWalk;
// Lazily initialized fields.
private PatchSet.Id psId;
// Fields exposed as setters. // Fields exposed as setters.
private SshInfo sshInfo; private SshInfo sshInfo;
@@ -123,9 +116,8 @@ public class PatchSetInserter extends BatchUpdate.Op {
PatchSetInfoFactory patchSetInfoFactory, PatchSetInfoFactory patchSetInfoFactory,
CommitValidators.Factory commitValidatorsFactory, CommitValidators.Factory commitValidatorsFactory,
ReplacePatchSetSender.Factory replacePatchSetFactory, ReplacePatchSetSender.Factory replacePatchSetFactory,
@Assisted Repository git, @Assisted RefControl refControl,
@Assisted RevWalk revWalk, @Assisted PatchSet.Id psId,
@Assisted ChangeControl ctl,
@Assisted RevCommit commit) { @Assisted RevCommit commit) {
this.hooks = hooks; this.hooks = hooks;
this.db = db; this.db = db;
@@ -136,18 +128,12 @@ public class PatchSetInserter extends BatchUpdate.Op {
this.commitValidatorsFactory = commitValidatorsFactory; this.commitValidatorsFactory = commitValidatorsFactory;
this.replacePatchSetFactory = replacePatchSetFactory; this.replacePatchSetFactory = replacePatchSetFactory;
this.git = git; this.refControl = refControl;
this.revWalk = revWalk; this.psId = psId;
this.commit = commit; this.commit = commit;
this.ctl = ctl;
this.user = ctl.getUser().asIdentifiedUser();
} }
public PatchSet.Id getPatchSetId() throws IOException { public PatchSet.Id getPatchSetId() {
if (psId == null) {
psId =
ChangeUtil.nextPatchSetId(git, ctl.getChange().currentPatchSetId());
}
return psId; return psId;
} }
@@ -205,7 +191,7 @@ public class PatchSetInserter extends BatchUpdate.Op {
public void updateRepo(RepoContext ctx) public void updateRepo(RepoContext ctx)
throws InvalidChangeOperationException, IOException { throws InvalidChangeOperationException, IOException {
init(); init();
validate(); validate(ctx);
patchSetInfo = patchSetInfoFactory.get(ctx.getRevWalk(), commit, psId); patchSetInfo = patchSetInfoFactory.get(ctx.getRevWalk(), commit, psId);
ctx.addRefUpdate(new ReceiveCommand(ObjectId.zeroId(), ctx.addRefUpdate(new ReceiveCommand(ObjectId.zeroId(),
commit, getPatchSetId().toRefName(), ReceiveCommand.Type.CREATE)); commit, getPatchSetId().toRefName(), ReceiveCommand.Type.CREATE));
@@ -214,6 +200,8 @@ public class PatchSetInserter extends BatchUpdate.Op {
@Override @Override
public void updateChange(ChangeContext ctx) throws OrmException, public void updateChange(ChangeContext ctx) throws OrmException,
InvalidChangeOperationException { InvalidChangeOperationException {
ChangeControl ctl = ctx.getChangeControl();
change = ctx.getChange(); change = ctx.getChange();
Change.Id id = change.getId(); Change.Id id = change.getId();
final PatchSet.Id currentPatchSetId = change.currentPatchSetId(); final PatchSet.Id currentPatchSetId = change.currentPatchSetId();
@@ -244,7 +232,7 @@ public class PatchSetInserter extends BatchUpdate.Op {
changeMessage = new ChangeMessage( changeMessage = new ChangeMessage(
new ChangeMessage.Key( new ChangeMessage.Key(
ctl.getChange().getId(), ChangeUtil.messageUUID(db)), ctl.getChange().getId(), ChangeUtil.messageUUID(db)),
user.getAccountId(), ctx.getWhen(), patchSet.getId()); ctx.getUser().getAccountId(), ctx.getWhen(), patchSet.getId());
changeMessage.setMessage(message); changeMessage.setMessage(message);
} }
@@ -284,7 +272,7 @@ public class PatchSetInserter extends BatchUpdate.Op {
try { try {
ReplacePatchSetSender cm = replacePatchSetFactory.create( ReplacePatchSetSender cm = replacePatchSetFactory.create(
change.getId()); change.getId());
cm.setFrom(user.getAccountId()); cm.setFrom(ctx.getUser().getAccountId());
cm.setPatchSet(patchSet, patchSetInfo); cm.setPatchSet(patchSet, patchSetInfo);
cm.setChangeMessage(changeMessage); cm.setChangeMessage(changeMessage);
cm.addReviewers(oldReviewers.get(ReviewerState.REVIEWER)); cm.addReviewers(oldReviewers.get(ReviewerState.REVIEWER));
@@ -307,9 +295,10 @@ public class PatchSetInserter extends BatchUpdate.Op {
} }
} }
private void validate() throws InvalidChangeOperationException, IOException { private void validate(RepoContext ctx)
CommitValidators cv = throws InvalidChangeOperationException, IOException {
commitValidatorsFactory.create(ctl.getRefControl(), sshInfo, git); CommitValidators cv = commitValidatorsFactory.create(
refControl, sshInfo, ctx.getRepository());
String refName = getPatchSetId().toRefName(); String refName = getPatchSetId().toRefName();
CommitReceivedEvent event = new CommitReceivedEvent( CommitReceivedEvent event = new CommitReceivedEvent(
@@ -317,13 +306,14 @@ public class PatchSetInserter extends BatchUpdate.Op {
ObjectId.zeroId(), ObjectId.zeroId(),
commit.getId(), commit.getId(),
refName.substring(0, refName.lastIndexOf('/') + 1) + "new"), refName.substring(0, refName.lastIndexOf('/') + 1) + "new"),
ctl.getProjectControl().getProject(), ctl.getRefControl().getRefName(), refControl.getProjectControl().getProject(), refControl.getRefName(),
commit, user); commit, ctx.getUser().asIdentifiedUser());
try { try {
switch (validatePolicy) { switch (validatePolicy) {
case RECEIVE_COMMITS: case RECEIVE_COMMITS:
NoteMap rejectCommits = BanCommit.loadRejectCommitsMap(git, revWalk); NoteMap rejectCommits = BanCommit.loadRejectCommitsMap(
ctx.getRepository(), ctx.getRevWalk());
cv.validateForReceiveCommits(event, rejectCommits); cv.validateForReceiveCommits(event, rejectCommits);
break; break;
case GERRIT: case GERRIT:

View File

@@ -31,7 +31,7 @@ import com.google.gerrit.server.edit.ChangeEdit;
import com.google.gerrit.server.edit.ChangeEditUtil; import com.google.gerrit.server.edit.ChangeEditUtil;
import com.google.gerrit.server.git.UpdateException; import com.google.gerrit.server.git.UpdateException;
import com.google.gerrit.server.project.InvalidChangeOperationException; import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gerrit.server.project.NoSuchChangeException; import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.Singleton; import com.google.inject.Singleton;
@@ -85,7 +85,7 @@ public class PublishChangeEdit implements
@Override @Override
public Response<?> apply(ChangeResource rsrc, Publish.Input in) public Response<?> apply(ChangeResource rsrc, Publish.Input in)
throws NoSuchChangeException, throws NoSuchProjectException,
IOException, InvalidChangeOperationException, OrmException, IOException, InvalidChangeOperationException, OrmException,
RestApiException, UpdateException { RestApiException, UpdateException {
Capable r = Capable r =

View File

@@ -36,6 +36,7 @@ import com.google.gerrit.server.git.UpdateException;
import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.InvalidChangeOperationException; import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gerrit.server.project.NoSuchChangeException; import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.Provider; import com.google.inject.Provider;
@@ -95,7 +96,7 @@ public class Rebase implements RestModifyView<RevisionResource, RebaseInput>,
rebaseChange.get().rebase(repo, rw, rsrc, findBaseRev(rw, rsrc, input)); rebaseChange.get().rebase(repo, rw, rsrc, findBaseRev(rw, rsrc, input));
} catch (InvalidChangeOperationException e) { } catch (InvalidChangeOperationException e) {
throw new ResourceConflictException(e.getMessage()); throw new ResourceConflictException(e.getMessage());
} catch (NoSuchChangeException e) { } catch (NoSuchChangeException | NoSuchProjectException e) {
throw new ResourceNotFoundException(change.getId().toString()); throw new ResourceNotFoundException(change.getId().toString());
} }

View File

@@ -24,6 +24,7 @@ import com.google.gerrit.reviewdb.client.Change.Status;
import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.RevId; import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.reviewdb.server.ReviewDb;
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.git.BatchUpdate; import com.google.gerrit.server.git.BatchUpdate;
@@ -32,9 +33,11 @@ import com.google.gerrit.server.git.MergeConflictException;
import com.google.gerrit.server.git.MergeUtil; import com.google.gerrit.server.git.MergeUtil;
import com.google.gerrit.server.git.UpdateException; import com.google.gerrit.server.git.UpdateException;
import com.google.gerrit.server.git.validators.CommitValidators; import com.google.gerrit.server.git.validators.CommitValidators;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.InvalidChangeOperationException; import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gerrit.server.project.NoSuchChangeException; import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.project.ProjectControl;
import com.google.gerrit.server.project.RefControl;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.Provider; import com.google.inject.Provider;
@@ -59,7 +62,7 @@ import java.util.TimeZone;
public class RebaseChange { public class RebaseChange {
private static final Logger log = LoggerFactory.getLogger(RebaseChange.class); private static final Logger log = LoggerFactory.getLogger(RebaseChange.class);
private final ChangeControl.GenericFactory changeControlFactory; private final ProjectControl.GenericFactory projectControlFactory;
private final Provider<ReviewDb> db; private final Provider<ReviewDb> db;
private final GitRepositoryManager gitManager; private final GitRepositoryManager gitManager;
private final TimeZone serverTimeZone; private final TimeZone serverTimeZone;
@@ -68,14 +71,14 @@ public class RebaseChange {
private final BatchUpdate.Factory updateFactory; private final BatchUpdate.Factory updateFactory;
@Inject @Inject
RebaseChange(ChangeControl.GenericFactory changeControlFactory, RebaseChange(ProjectControl.GenericFactory projectControlFactory,
Provider<ReviewDb> db, Provider<ReviewDb> db,
@GerritPersonIdent PersonIdent myIdent, @GerritPersonIdent PersonIdent myIdent,
GitRepositoryManager gitManager, GitRepositoryManager gitManager,
MergeUtil.Factory mergeUtilFactory, MergeUtil.Factory mergeUtilFactory,
PatchSetInserter.Factory patchSetInserterFactory, PatchSetInserter.Factory patchSetInserterFactory,
BatchUpdate.Factory updateFactory) { BatchUpdate.Factory updateFactory) {
this.changeControlFactory = changeControlFactory; this.projectControlFactory = projectControlFactory;
this.db = db; this.db = db;
this.gitManager = gitManager; this.gitManager = gitManager;
this.serverTimeZone = myIdent.getTimeZone(); this.serverTimeZone = myIdent.getTimeZone();
@@ -104,6 +107,8 @@ public class RebaseChange {
* @param newBaseRev the commit that should be the new base. * @param newBaseRev the commit that should be the new base.
* @throws NoSuchChangeException if the change to which the patch set belongs * @throws NoSuchChangeException if the change to which the patch set belongs
* does not exist or is not visible to the user. * does not exist or is not visible to the user.
* @throws NoSuchProjectException if the project to which the patch set
* belongs does not exist or is not visible to the user.
* @throws EmailException if sending the e-mail to notify about the new patch * @throws EmailException if sending the e-mail to notify about the new patch
* set fails. * set fails.
* @throws OrmException if accessing the database fails. * @throws OrmException if accessing the database fails.
@@ -115,8 +120,8 @@ public class RebaseChange {
* @throws UpdateException if updating the change fails. * @throws UpdateException if updating the change fails.
*/ */
public void rebase(Repository git, RevWalk rw, RevisionResource rsrc, public void rebase(Repository git, RevWalk rw, RevisionResource rsrc,
String newBaseRev) throws NoSuchChangeException, EmailException, String newBaseRev) throws NoSuchChangeException, NoSuchProjectException,
OrmException, IOException, InvalidChangeOperationException, EmailException, OrmException, IOException, InvalidChangeOperationException,
UpdateException, RestApiException { UpdateException, RestApiException {
Change change = rsrc.getChange(); Change change = rsrc.getChange();
PatchSet patchSet = rsrc.getPatchSet(); PatchSet patchSet = rsrc.getPatchSet();
@@ -246,8 +251,8 @@ public class RebaseChange {
* @param validate if commit validation should be run for the new patch set. * @param validate if commit validation should be run for the new patch set.
* @param rw the RevWalk. * @param rw the RevWalk.
* @return the new patch set, which is based on the given base commit. * @return the new patch set, which is based on the given base commit.
* @throws NoSuchChangeException if the change to which the patch set belongs * @throws NoSuchProjectException if the project to which the patch set
* does not exist or is not visible to the user. * belongs does not exist or is not visible to the user.
* @throws OrmException if accessing the database fails. * @throws OrmException if accessing the database fails.
* @throws IOException if rebase is not possible. * @throws IOException if rebase is not possible.
* @throws InvalidChangeOperationException if rebase is not possible or not * @throws InvalidChangeOperationException if rebase is not possible or not
@@ -261,7 +266,7 @@ public class RebaseChange {
IdentifiedUser uploader, RevCommit baseCommit, MergeUtil mergeUtil, IdentifiedUser uploader, RevCommit baseCommit, MergeUtil mergeUtil,
PersonIdent committerIdent, boolean runHooks, PersonIdent committerIdent, boolean runHooks,
CommitValidators.Policy validate) CommitValidators.Policy validate)
throws NoSuchChangeException, OrmException, IOException, throws NoSuchProjectException, OrmException, IOException,
InvalidChangeOperationException, MergeConflictException, UpdateException, InvalidChangeOperationException, MergeConflictException, UpdateException,
RestApiException { RestApiException {
if (!change.currentPatchSetId().equals(patchSetId)) { if (!change.currentPatchSetId().equals(patchSetId)) {
@@ -278,11 +283,14 @@ public class RebaseChange {
rebasedCommit = rw.parseCommit(newId); rebasedCommit = rw.parseCommit(newId);
ChangeControl changeControl = RefControl ctl = projectControlFactory
changeControlFactory.validateFor(change, uploader); .controlFor(change.getProject(), uploader)
.controlForRef(change.getDest());
PatchSet.Id psId =
ChangeUtil.nextPatchSetId(git, change.currentPatchSetId());
PatchSetInserter patchSetInserter = patchSetInserterFactory PatchSetInserter patchSetInserter = patchSetInserterFactory
.create(git, rw, changeControl, rebasedCommit) .create(ctl, psId, rebasedCommit)
.setValidatePolicy(validate) .setValidatePolicy(validate)
.setDraft(originalPatchSet.isDraft()) .setDraft(originalPatchSet.isDraft())
.setUploader(uploader.getAccountId()) .setUploader(uploader.getAccountId())

View File

@@ -26,6 +26,7 @@ import com.google.gerrit.reviewdb.client.Change.Status;
import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.RefNames; import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.change.ChangeKind; import com.google.gerrit.server.change.ChangeKind;
@@ -35,10 +36,11 @@ import com.google.gerrit.server.git.BatchUpdate;
import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.UpdateException; import com.google.gerrit.server.git.UpdateException;
import com.google.gerrit.server.index.ChangeIndexer; import com.google.gerrit.server.index.ChangeIndexer;
import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.project.ProjectCache; import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectControl;
import com.google.gerrit.server.project.ProjectState; import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.project.RefControl;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.Provider; import com.google.inject.Provider;
@@ -65,7 +67,7 @@ import java.io.IOException;
public class ChangeEditUtil { public class ChangeEditUtil {
private final GitRepositoryManager gitManager; private final GitRepositoryManager gitManager;
private final PatchSetInserter.Factory patchSetInserterFactory; private final PatchSetInserter.Factory patchSetInserterFactory;
private final ChangeControl.GenericFactory changeControlFactory; private final ProjectControl.GenericFactory projectControlFactory;
private final ChangeIndexer indexer; private final ChangeIndexer indexer;
private final ProjectCache projectCache; private final ProjectCache projectCache;
private final Provider<ReviewDb> db; private final Provider<ReviewDb> db;
@@ -76,7 +78,7 @@ public class ChangeEditUtil {
@Inject @Inject
ChangeEditUtil(GitRepositoryManager gitManager, ChangeEditUtil(GitRepositoryManager gitManager,
PatchSetInserter.Factory patchSetInserterFactory, PatchSetInserter.Factory patchSetInserterFactory,
ChangeControl.GenericFactory changeControlFactory, ProjectControl.GenericFactory projectControlFactory,
ChangeIndexer indexer, ChangeIndexer indexer,
ProjectCache projectCache, ProjectCache projectCache,
Provider<ReviewDb> db, Provider<ReviewDb> db,
@@ -85,7 +87,7 @@ public class ChangeEditUtil {
BatchUpdate.Factory updateFactory) { BatchUpdate.Factory updateFactory) {
this.gitManager = gitManager; this.gitManager = gitManager;
this.patchSetInserterFactory = patchSetInserterFactory; this.patchSetInserterFactory = patchSetInserterFactory;
this.changeControlFactory = changeControlFactory; this.projectControlFactory = projectControlFactory;
this.indexer = indexer; this.indexer = indexer;
this.projectCache = projectCache; this.projectCache = projectCache;
this.db = db; this.db = db;
@@ -147,13 +149,13 @@ public class ChangeEditUtil {
* its parent. * its parent.
* *
* @param edit change edit to publish * @param edit change edit to publish
* @throws NoSuchChangeException * @throws NoSuchProjectException
* @throws IOException * @throws IOException
* @throws OrmException * @throws OrmException
* @throws UpdateException * @throws UpdateException
* @throws RestApiException * @throws RestApiException
*/ */
public void publish(ChangeEdit edit) throws NoSuchChangeException, public void publish(ChangeEdit edit) throws NoSuchProjectException,
IOException, OrmException, RestApiException, UpdateException { IOException, OrmException, RestApiException, UpdateException {
Change change = edit.getChange(); Change change = edit.getChange();
try (Repository repo = gitManager.openRepository(change.getProject()); try (Repository repo = gitManager.openRepository(change.getProject());
@@ -166,7 +168,7 @@ public class ChangeEditUtil {
} }
Change updatedChange = Change updatedChange =
insertPatchSet(edit, change, repo, rw, basePatchSet, insertPatchSet(edit, change, repo, rw, inserter, basePatchSet,
squashEdit(rw, inserter, edit.getEditCommit(), basePatchSet)); squashEdit(rw, inserter, edit.getEditCommit(), basePatchSet));
// TODO(davido): This should happen in the same BatchRefUpdate. // TODO(davido): This should happen in the same BatchRefUpdate.
deleteRef(repo, edit); deleteRef(repo, edit);
@@ -215,12 +217,16 @@ public class ChangeEditUtil {
} }
private Change insertPatchSet(ChangeEdit edit, Change change, private Change insertPatchSet(ChangeEdit edit, Change change,
Repository repo, RevWalk rw, PatchSet basePatchSet, RevCommit squashed) Repository repo, RevWalk rw, ObjectInserter oi, PatchSet basePatchSet,
throws NoSuchChangeException, RestApiException, UpdateException, RevCommit squashed) throws NoSuchProjectException, RestApiException,
IOException { UpdateException, IOException {
ChangeControl ctl = changeControlFactory.controlFor(change, edit.getUser()); RefControl ctl = projectControlFactory
.controlFor(change.getProject(), edit.getUser())
.controlForRef(change.getDest());
PatchSet.Id psId =
ChangeUtil.nextPatchSetId(repo, change.currentPatchSetId());
PatchSetInserter inserter = PatchSetInserter inserter =
patchSetInserterFactory.create(repo, rw, ctl, squashed); patchSetInserterFactory.create(ctl, psId, squashed);
StringBuilder message = new StringBuilder("Patch set ") StringBuilder message = new StringBuilder("Patch set ")
.append(inserter.getPatchSetId().get()) .append(inserter.getPatchSetId().get())
@@ -241,6 +247,7 @@ public class ChangeEditUtil {
try (BatchUpdate bu = updateFactory.create( try (BatchUpdate bu = updateFactory.create(
db.get(), change.getProject(), ctl.getUser(), db.get(), change.getProject(), ctl.getUser(),
TimeUtil.nowTs())) { TimeUtil.nowTs())) {
bu.setRepository(repo, rw, oi);
bu.addOp(change.getId(), inserter bu.addOp(change.getId(), inserter
.setDraft(change.getStatus() == Status.DRAFT || .setDraft(change.getStatus() == Status.DRAFT ||
basePatchSet.isDraft()) basePatchSet.isDraft())

View File

@@ -79,6 +79,10 @@ public class BatchUpdate implements AutoCloseable {
} }
public class Context { public class Context {
public Project.NameKey getProject() {
return project;
}
public Timestamp getWhen() { public Timestamp getWhen() {
return when; return when;
} }
@@ -237,6 +241,10 @@ public class BatchUpdate implements AutoCloseable {
} }
} }
public CurrentUser getUser() {
return user;
}
public Repository getRepository() throws IOException { public Repository getRepository() throws IOException {
initRepository(); initRepository();
return repo; return repo;

View File

@@ -31,6 +31,7 @@ import com.google.gerrit.server.git.validators.CommitValidators;
import com.google.gerrit.server.patch.PatchSetInfoFactory; import com.google.gerrit.server.patch.PatchSetInfoFactory;
import com.google.gerrit.server.project.InvalidChangeOperationException; import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gerrit.server.project.NoSuchChangeException; import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectId;
@@ -117,9 +118,9 @@ public class RebaseIfNecessary extends SubmitStrategy {
setRefLogIdent(); setRefLogIdent();
} catch (MergeConflictException e) { } catch (MergeConflictException e) {
n.setStatusCode(CommitMergeStatus.REBASE_MERGE_CONFLICT); n.setStatusCode(CommitMergeStatus.REBASE_MERGE_CONFLICT);
} catch (NoSuchChangeException | OrmException | IOException } catch (NoSuchChangeException | NoSuchProjectException
| InvalidChangeOperationException | UpdateException | OrmException | IOException | InvalidChangeOperationException
| RestApiException e) { | UpdateException | RestApiException e) {
// TODO(dborowitz): Allow Submit to unwrap ResourceConflictException // TODO(dborowitz): Allow Submit to unwrap ResourceConflictException
// so it can turn into a 409. // so it can turn into a 409.
throw new MergeException("Cannot rebase " + n.name(), e); throw new MergeException("Cannot rebase " + n.name(), e);

View File

@@ -17,6 +17,7 @@ package com.google.gerrit.server.query.change;
import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.TruthJUnit.assume; import static com.google.common.truth.TruthJUnit.assume;
import static com.google.gerrit.extensions.client.ListChangesOption.REVIEWED; import static com.google.gerrit.extensions.client.ListChangesOption.REVIEWED;
import static java.nio.charset.StandardCharsets.UTF_8; import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.concurrent.TimeUnit.HOURS; import static java.util.concurrent.TimeUnit.HOURS;
import static java.util.concurrent.TimeUnit.MILLISECONDS; import static java.util.concurrent.TimeUnit.MILLISECONDS;
@@ -75,6 +76,7 @@ import com.google.inject.util.Providers;
import org.eclipse.jgit.junit.TestRepository; import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.ObjectInserter;
import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.revwalk.RevCommit;
import org.joda.time.DateTime; import org.joda.time.DateTime;
import org.joda.time.DateTimeUtils; import org.joda.time.DateTimeUtils;
@@ -1338,15 +1340,18 @@ public abstract class AbstractQueryChangesTest {
.message("message") .message("message")
.add("file" + n, "contents " + n) .add("file" + n, "contents " + n)
.create()); .create());
ChangeControl ctl = changeControlFactory.controlFor(c.getId(), user); RefControl ctl = projectControlFactory.controlFor(c.getProject(), user)
.controlForRef(c.getDest());
PatchSetInserter inserter = patchSetFactory.create( PatchSetInserter inserter = patchSetFactory.create(
repo.getRepository(), repo.getRevWalk(), ctl, commit) ctl, new PatchSet.Id(c.getId(), n), commit)
.setSendMail(false) .setSendMail(false)
.setRunHooks(false) .setRunHooks(false)
.setValidatePolicy(CommitValidators.Policy.NONE); .setValidatePolicy(CommitValidators.Policy.NONE);
try (BatchUpdate bu = updateFactory.create( try (BatchUpdate bu = updateFactory.create(
db, c.getDest().getParentKey(), user, TimeUtil.nowTs())) { db, c.getProject(), user, TimeUtil.nowTs());
ObjectInserter oi = repo.getRepository().newObjectInserter()) {
bu.setRepository(repo.getRepository(), repo.getRevWalk(), oi);
bu.addOp(c.getId(), inserter); bu.addOp(c.getId(), inserter);
bu.execute(); bu.execute();
} }