MergeOp: Store caller in an instance field

In this case, the tradeoff in method signature simplification is worth
the extra bit of state. Like submissionId, this is initialized first
thing in the public merge call.

Change-Id: I886e657686767337cb2a94117a69f872a879808e
This commit is contained in:
Dave Borowitz 2016-01-12 14:20:14 -05:00 committed by David Pursehouse
parent 9fc3ee97a4
commit 82253c6e51

View File

@ -276,6 +276,7 @@ public class MergeOp implements AutoCloseable {
} }
private Timestamp ts; private Timestamp ts;
private String submissionId; private String submissionId;
private IdentifiedUser caller;
private ReviewDb db; private ReviewDb db;
@ -475,6 +476,7 @@ public class MergeOp implements AutoCloseable {
public void merge(ReviewDb db, Change change, IdentifiedUser caller, public void merge(ReviewDb db, Change change, IdentifiedUser caller,
boolean checkSubmitRules) throws NoSuchChangeException, boolean checkSubmitRules) throws NoSuchChangeException,
OrmException, ResourceConflictException { OrmException, ResourceConflictException {
this.caller = caller;
updateSubmissionId(change); updateSubmissionId(change);
this.db = db; this.db = db;
logDebug("Beginning integration of {}", change); logDebug("Beginning integration of {}", change);
@ -488,7 +490,7 @@ public class MergeOp implements AutoCloseable {
failFast(cs); // Done checks that don't involve opening repo. failFast(cs); // Done checks that don't involve opening repo.
} }
try { try {
integrateIntoHistory(cs, caller); integrateIntoHistory(cs);
} catch (IntegrationException e) { } catch (IntegrationException e) {
logError("Merge Conflict", e); logError("Merge Conflict", e);
throw new ResourceConflictException("Merge Conflict", e); throw new ResourceConflictException("Merge Conflict", e);
@ -520,7 +522,7 @@ public class MergeOp implements AutoCloseable {
throw new ResourceConflictException(msg + Joiner.on('\n').join(ps)); throw new ResourceConflictException(msg + Joiner.on('\n').join(ps));
} }
private void integrateIntoHistory(ChangeSet cs, IdentifiedUser caller) private void integrateIntoHistory(ChangeSet cs)
throws IntegrationException, NoSuchChangeException, throws IntegrationException, NoSuchChangeException,
ResourceConflictException { ResourceConflictException {
logDebug("Beginning merge attempt on {}", cs); logDebug("Beginning merge attempt on {}", cs);
@ -531,7 +533,7 @@ public class MergeOp implements AutoCloseable {
Multimap<Branch.NameKey, ChangeData> cbb = cs.changesByBranch(); Multimap<Branch.NameKey, ChangeData> cbb = cs.changesByBranch();
for (Branch.NameKey branch : cbb.keySet()) { for (Branch.NameKey branch : cbb.keySet()) {
OpenRepo or = openRepo(branch.getParentKey()); OpenRepo or = openRepo(branch.getParentKey());
toSubmit.put(branch, validateChangeList(or, cbb.get(branch), caller)); toSubmit.put(branch, validateChangeList(or, cbb.get(branch)));
} }
failFast(cs); // Done checks that don't involve running submit strategies. failFast(cs); // Done checks that don't involve running submit strategies.
@ -540,7 +542,7 @@ public class MergeOp implements AutoCloseable {
OpenBranch ob = or.getBranch(branch); OpenBranch ob = or.getBranch(branch);
BranchBatch submitting = toSubmit.get(branch); BranchBatch submitting = toSubmit.get(branch);
SubmitStrategy strategy = createStrategy(or, branch, SubmitStrategy strategy = createStrategy(or, branch,
submitting.submitType(), ob.oldTip, caller); submitting.submitType(), ob.oldTip);
ob.mergeTip = preMerge(strategy, submitting.changes(), ob.oldTip); ob.mergeTip = preMerge(strategy, submitting.changes(), ob.oldTip);
} }
checkMergeStrategyResults(cs, toSubmit.values()); checkMergeStrategyResults(cs, toSubmit.values());
@ -556,10 +558,10 @@ public class MergeOp implements AutoCloseable {
OpenRepo or = openRepo(project); OpenRepo or = openRepo(project);
for (Branch.NameKey branch : br.get(project)) { for (Branch.NameKey branch : br.get(project)) {
OpenBranch ob = or.getBranch(branch); OpenBranch ob = or.getBranch(branch);
boolean updated = updateBranch(or, branch, caller); boolean updated = updateBranch(or, branch);
BranchBatch submitting = toSubmit.get(branch); BranchBatch submitting = toSubmit.get(branch);
updateChangeStatus(ob, submitting.changes(), caller); updateChangeStatus(ob, submitting.changes());
updateSubmoduleSubscriptions(ob, subOp); updateSubmoduleSubscriptions(ob, subOp);
if (updated) { if (updated) {
fireRefUpdated(ob); fireRefUpdated(ob);
@ -600,7 +602,7 @@ public class MergeOp implements AutoCloseable {
private SubmitStrategy createStrategy(OpenRepo or, private SubmitStrategy createStrategy(OpenRepo or,
Branch.NameKey destBranch, SubmitType submitType, Branch.NameKey destBranch, SubmitType submitType,
CodeReviewCommit branchTip, IdentifiedUser caller) CodeReviewCommit branchTip)
throws IntegrationException, NoSuchProjectException { throws IntegrationException, NoSuchProjectException {
return submitStrategyFactory.create(submitType, db, or.repo, or.rw, or.ins, return submitStrategyFactory.create(submitType, db, or.repo, or.rw, or.ins,
or.canMergeFlag, getAlreadyAccepted(or, branchTip), destBranch, caller, or.canMergeFlag, getAlreadyAccepted(or, branchTip), destBranch, caller,
@ -640,8 +642,7 @@ public class MergeOp implements AutoCloseable {
} }
private BranchBatch validateChangeList(OpenRepo or, private BranchBatch validateChangeList(OpenRepo or,
Collection<ChangeData> submitted, IdentifiedUser caller) Collection<ChangeData> submitted) throws IntegrationException {
throws IntegrationException {
logDebug("Validating {} changes", submitted.size()); logDebug("Validating {} changes", submitted.size());
List<ChangeData> toSubmit = new ArrayList<>(submitted.size()); List<ChangeData> toSubmit = new ArrayList<>(submitted.size());
Multimap<ObjectId, PatchSet.Id> revisions = getRevisions(or, submitted); Multimap<ObjectId, PatchSet.Id> revisions = getRevisions(or, submitted);
@ -777,8 +778,8 @@ public class MergeOp implements AutoCloseable {
} }
} }
private boolean updateBranch(OpenRepo or, Branch.NameKey destBranch, private boolean updateBranch(OpenRepo or, Branch.NameKey destBranch)
IdentifiedUser caller) throws IntegrationException { throws IntegrationException {
OpenBranch ob = or.getBranch(destBranch); OpenBranch ob = or.getBranch(destBranch);
CodeReviewCommit currentTip = ob.getCurrentTip(); CodeReviewCommit currentTip = ob.getCurrentTip();
if (Objects.equals(ob.oldTip, currentTip)) { if (Objects.equals(ob.oldTip, currentTip)) {
@ -924,8 +925,8 @@ public class MergeOp implements AutoCloseable {
failFast(cs); failFast(cs);
} }
private void updateChangeStatus(OpenBranch ob, List<ChangeData> submitted, private void updateChangeStatus(OpenBranch ob, List<ChangeData> submitted)
IdentifiedUser caller) throws ResourceConflictException { throws ResourceConflictException {
List<Change.Id> problemChanges = new ArrayList<>(submitted.size()); List<Change.Id> problemChanges = new ArrayList<>(submitted.size());
logDebug("Updating change status for {} changes", submitted.size()); logDebug("Updating change status for {} changes", submitted.size());
@ -940,7 +941,7 @@ public class MergeOp implements AutoCloseable {
checkState(s != null, checkState(s != null,
"status not set for change %s; expected to previously fail fast", "status not set for change %s; expected to previously fail fast",
id); id);
setApproval(cd, caller); setApproval(cd);
ObjectId mergeResultRev = ob.mergeTip != null ObjectId mergeResultRev = ob.mergeTip != null
? ob.mergeTip.getMergeResults().get(commit) : null; ? ob.mergeTip.getMergeResults().get(commit) : null;
@ -1093,18 +1094,17 @@ public class MergeOp implements AutoCloseable {
}); });
} }
private void setApproval(ChangeData cd, IdentifiedUser user) private void setApproval(ChangeData cd) throws OrmException, IOException {
throws OrmException, IOException {
Timestamp timestamp = TimeUtil.nowTs(); Timestamp timestamp = TimeUtil.nowTs();
ChangeControl control = cd.changeControl(); ChangeControl control = cd.changeControl();
PatchSet.Id psId = cd.currentPatchSet().getId(); PatchSet.Id psId = cd.currentPatchSet().getId();
PatchSet.Id psIdNewRev = commits.get(cd.change().getId()) PatchSet.Id psIdNewRev = commits.get(cd.change().getId())
.change().currentPatchSetId(); .change().currentPatchSetId();
logDebug("Add approval for " + cd + " from user " + user); logDebug("Add approval for " + cd);
ChangeUpdate update = updateFactory.create(control, timestamp); ChangeUpdate update = updateFactory.create(control, timestamp);
update.setPatchSetId(psId); update.setPatchSetId(psId);
update.putReviewer(user.getAccountId(), REVIEWER); update.putReviewer(caller.getAccountId(), REVIEWER);
Optional<SubmitRecord> okRecord = findOkRecord(cd.getSubmitRecords()); Optional<SubmitRecord> okRecord = findOkRecord(cd.getSubmitRecords());
if (okRecord.isPresent()) { if (okRecord.isPresent()) {
update.merge(ImmutableList.of(okRecord.get())); update.merge(ImmutableList.of(okRecord.get()));
@ -1113,7 +1113,7 @@ public class MergeOp implements AutoCloseable {
try { try {
BatchMetaDataUpdate batch = update.openUpdate(); BatchMetaDataUpdate batch = update.openUpdate();
LabelNormalizer.Result normalized = LabelNormalizer.Result normalized =
approve(control, psId, user, update, timestamp); approve(control, psId, caller, update, timestamp);
batch.write(update, new CommitBuilder()); batch.write(update, new CommitBuilder());
// If the submit strategy created a new revision (rebase, cherry-pick) // If the submit strategy created a new revision (rebase, cherry-pick)