Use repo from BatchUpdate.Context directly more often

Avoid some ugly indirection where we do nothing in the updateRepo
phase but read something from the repo for use in updateChange.

Change-Id: Ibc646fc9b0e0dfb5e9f956b1919f7d7670ba0d74
This commit is contained in:
Dave Borowitz 2015-12-10 14:25:30 -05:00
parent 7517c70825
commit 8a1cef198a
4 changed files with 22 additions and 29 deletions

View File

@ -84,7 +84,6 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp {
private final RefControl refControl; private final RefControl refControl;
private final IdentifiedUser user; private final IdentifiedUser user;
private final Change change;
private final PatchSet patchSet; private final PatchSet patchSet;
private final RevCommit commit; private final RevCommit commit;
@ -101,6 +100,7 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp {
private boolean updateRef; private boolean updateRef;
// Fields set during the insertion process. // Fields set during the insertion process.
private Change change;
private ChangeMessage changeMessage; private ChangeMessage changeMessage;
private PatchSetInfo patchSetInfo; private PatchSetInfo patchSetInfo;
@ -230,9 +230,6 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp {
public void updateRepo(RepoContext ctx) public void updateRepo(RepoContext ctx)
throws ResourceConflictException, IOException { throws ResourceConflictException, IOException {
validate(ctx); validate(ctx);
patchSetInfo = patchSetInfoFactory.get(
ctx.getRevWalk(), commit, patchSet.getId());
change.setCurrentPatchSet(patchSetInfo);
if (!updateRef) { if (!updateRef) {
return; return;
} }
@ -242,9 +239,14 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp {
@Override @Override
public void updateChange(ChangeContext ctx) throws OrmException, IOException { public void updateChange(ChangeContext ctx) throws OrmException, IOException {
change = ctx.getChange(); // Use defensive copy created by ChangeControl.
ReviewDb db = ctx.getDb(); ReviewDb db = ctx.getDb();
ChangeControl ctl = ctx.getChangeControl(); ChangeControl ctl = ctx.getChangeControl();
ChangeUpdate update = ctx.getChangeUpdate(); ChangeUpdate update = ctx.getChangeUpdate();
patchSetInfo = patchSetInfoFactory.get(
ctx.getRevWalk(), commit, patchSet.getId());
ctx.getChange().setCurrentPatchSet(patchSetInfo);
if (patchSet.getGroups() == null) { if (patchSet.getGroups() == null) {
patchSet.setGroups(GroupCollector.getDefaultGroups(patchSet)); patchSet.setGroups(GroupCollector.getDefaultGroups(patchSet));
} }

View File

@ -200,14 +200,13 @@ public class PatchSetInserter extends BatchUpdate.Op {
throws ResourceConflictException, IOException { throws ResourceConflictException, IOException {
init(); init();
validate(ctx); validate(ctx);
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));
} }
@Override @Override
public void updateChange(ChangeContext ctx) throws OrmException, public void updateChange(ChangeContext ctx) throws OrmException,
InvalidChangeOperationException { InvalidChangeOperationException, IOException {
ChangeControl ctl = ctx.getChangeControl(); ChangeControl ctl = ctx.getChangeControl();
change = ctx.getChange(); change = ctx.getChange();
@ -242,6 +241,7 @@ public class PatchSetInserter extends BatchUpdate.Op {
changeMessage.setMessage(message); changeMessage.setMessage(message);
} }
patchSetInfo = patchSetInfoFactory.get(ctx.getRevWalk(), commit, psId);
// TODO(dborowitz): Throw ResourceConflictException instead of using // TODO(dborowitz): Throw ResourceConflictException instead of using
// AtomicUpdate. // AtomicUpdate.
change = db.changes().atomicUpdate(id, new AtomicUpdate<Change>() { change = db.changes().atomicUpdate(id, new AtomicUpdate<Change>() {

View File

@ -42,7 +42,6 @@ import com.google.gerrit.server.change.PublishDraftPatchSet.Input;
import com.google.gerrit.server.git.BatchUpdate; import com.google.gerrit.server.git.BatchUpdate;
import com.google.gerrit.server.git.BatchUpdate.ChangeContext; import com.google.gerrit.server.git.BatchUpdate.ChangeContext;
import com.google.gerrit.server.git.BatchUpdate.Context; import com.google.gerrit.server.git.BatchUpdate.Context;
import com.google.gerrit.server.git.BatchUpdate.RepoContext;
import com.google.gerrit.server.git.UpdateException; import com.google.gerrit.server.git.UpdateException;
import com.google.gerrit.server.mail.CreateChangeSender; import com.google.gerrit.server.mail.CreateChangeSender;
import com.google.gerrit.server.mail.MailUtil.MailRecipients; import com.google.gerrit.server.mail.MailUtil.MailRecipients;
@ -155,7 +154,6 @@ public class PublishDraftPatchSet implements RestModifyView<RevisionResource, In
private PatchSet patchSet; private PatchSet patchSet;
private Change change; private Change change;
private boolean wasDraftChange; private boolean wasDraftChange;
private RevCommit commit;
private PatchSetInfo patchSetInfo; private PatchSetInfo patchSetInfo;
private MailRecipients recipients; private MailRecipients recipients;
@ -164,29 +162,18 @@ public class PublishDraftPatchSet implements RestModifyView<RevisionResource, In
this.patchSet = patchSet; this.patchSet = patchSet;
} }
@Override
public void updateRepo(RepoContext ctx)
throws RestApiException, OrmException, IOException {
PatchSet ps = patchSet;
if (ps == null) {
// Don't save in patchSet, since we're not in a transaction. Here we
// just need the revision, which is immutable.
ps = ctx.getDb().patchSets().get(psId);
if (ps == null) {
throw new ResourceNotFoundException(psId.toString());
}
}
commit = ctx.getRevWalk().parseCommit(
ObjectId.fromString(ps.getRevision().get()));
patchSetInfo = patchSetInfoFactory.get(ctx.getRevWalk(), commit, psId);
}
@Override @Override
public void updateChange(ChangeContext ctx) public void updateChange(ChangeContext ctx)
throws RestApiException, OrmException { throws RestApiException, OrmException, IOException {
if (!ctx.getChangeControl().canPublish(ctx.getDb())) { if (!ctx.getChangeControl().canPublish(ctx.getDb())) {
throw new AuthException("Cannot publish this draft patch set"); throw new AuthException("Cannot publish this draft patch set");
} }
if (patchSet == null) {
patchSet = ctx.getDb().patchSets().get(psId);
if (patchSet == null) {
throw new ResourceNotFoundException(psId.toString());
}
}
saveChange(ctx); saveChange(ctx);
savePatchSet(ctx); savePatchSet(ctx);
addReviewers(ctx); addReviewers(ctx);
@ -204,7 +191,6 @@ public class PublishDraftPatchSet implements RestModifyView<RevisionResource, In
private void savePatchSet(ChangeContext ctx) private void savePatchSet(ChangeContext ctx)
throws RestApiException, OrmException { throws RestApiException, OrmException {
patchSet = ctx.getDb().patchSets().get(psId);
if (!patchSet.isDraft()) { if (!patchSet.isDraft()) {
throw new ResourceConflictException("Patch set is not a draft"); throw new ResourceConflictException("Patch set is not a draft");
} }
@ -217,10 +203,15 @@ public class PublishDraftPatchSet implements RestModifyView<RevisionResource, In
ctx.getDb().patchSets().update(Collections.singleton(patchSet)); ctx.getDb().patchSets().update(Collections.singleton(patchSet));
} }
private void addReviewers(ChangeContext ctx) throws OrmException { private void addReviewers(ChangeContext ctx)
throws OrmException, IOException {
LabelTypes labelTypes = ctx.getChangeControl().getLabelTypes(); LabelTypes labelTypes = ctx.getChangeControl().getLabelTypes();
Collection<Account.Id> oldReviewers = approvalsUtil.getReviewers( Collection<Account.Id> oldReviewers = approvalsUtil.getReviewers(
ctx.getDb(), ctx.getChangeNotes()).values(); ctx.getDb(), ctx.getChangeNotes()).values();
RevCommit commit = ctx.getRevWalk().parseCommit(
ObjectId.fromString(patchSet.getRevision().get()));
patchSetInfo = patchSetInfoFactory.get(ctx.getRevWalk(), commit, psId);
List<FooterLine> footerLines = commit.getFooterLines(); List<FooterLine> footerLines = commit.getFooterLines();
recipients = recipients =
getRecipientsFromFooters(accountResolver, patchSet, footerLines); getRecipientsFromFooters(accountResolver, patchSet, footerLines);

View File

@ -144,7 +144,7 @@ public class RebaseChangeOp extends BatchUpdate.Op {
@Override @Override
public void updateChange(ChangeContext ctx) public void updateChange(ChangeContext ctx)
throws OrmException, InvalidChangeOperationException { throws OrmException, InvalidChangeOperationException, IOException {
patchSetInserter.updateChange(ctx); patchSetInserter.updateChange(ctx);
rebasedPatchSet = patchSetInserter.getPatchSet(); rebasedPatchSet = patchSetInserter.getPatchSet();
} }