From 82253c6e514a9ac546e5305534859d15e28ad188 Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Tue, 12 Jan 2016 14:20:14 -0500 Subject: [PATCH] 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 --- .../com/google/gerrit/server/git/MergeOp.java | 38 +++++++++---------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java index f9902b223f..785cc88292 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java @@ -276,6 +276,7 @@ public class MergeOp implements AutoCloseable { } private Timestamp ts; private String submissionId; + private IdentifiedUser caller; private ReviewDb db; @@ -475,6 +476,7 @@ public class MergeOp implements AutoCloseable { public void merge(ReviewDb db, Change change, IdentifiedUser caller, boolean checkSubmitRules) throws NoSuchChangeException, OrmException, ResourceConflictException { + this.caller = caller; updateSubmissionId(change); this.db = db; logDebug("Beginning integration of {}", change); @@ -488,7 +490,7 @@ public class MergeOp implements AutoCloseable { failFast(cs); // Done checks that don't involve opening repo. } try { - integrateIntoHistory(cs, caller); + integrateIntoHistory(cs); } catch (IntegrationException e) { logError("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)); } - private void integrateIntoHistory(ChangeSet cs, IdentifiedUser caller) + private void integrateIntoHistory(ChangeSet cs) throws IntegrationException, NoSuchChangeException, ResourceConflictException { logDebug("Beginning merge attempt on {}", cs); @@ -531,7 +533,7 @@ public class MergeOp implements AutoCloseable { Multimap cbb = cs.changesByBranch(); for (Branch.NameKey branch : cbb.keySet()) { 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. @@ -540,7 +542,7 @@ public class MergeOp implements AutoCloseable { OpenBranch ob = or.getBranch(branch); BranchBatch submitting = toSubmit.get(branch); SubmitStrategy strategy = createStrategy(or, branch, - submitting.submitType(), ob.oldTip, caller); + submitting.submitType(), ob.oldTip); ob.mergeTip = preMerge(strategy, submitting.changes(), ob.oldTip); } checkMergeStrategyResults(cs, toSubmit.values()); @@ -556,10 +558,10 @@ public class MergeOp implements AutoCloseable { OpenRepo or = openRepo(project); for (Branch.NameKey branch : br.get(project)) { OpenBranch ob = or.getBranch(branch); - boolean updated = updateBranch(or, branch, caller); + boolean updated = updateBranch(or, branch); BranchBatch submitting = toSubmit.get(branch); - updateChangeStatus(ob, submitting.changes(), caller); + updateChangeStatus(ob, submitting.changes()); updateSubmoduleSubscriptions(ob, subOp); if (updated) { fireRefUpdated(ob); @@ -600,7 +602,7 @@ public class MergeOp implements AutoCloseable { private SubmitStrategy createStrategy(OpenRepo or, Branch.NameKey destBranch, SubmitType submitType, - CodeReviewCommit branchTip, IdentifiedUser caller) + CodeReviewCommit branchTip) throws IntegrationException, NoSuchProjectException { return submitStrategyFactory.create(submitType, db, or.repo, or.rw, or.ins, or.canMergeFlag, getAlreadyAccepted(or, branchTip), destBranch, caller, @@ -640,8 +642,7 @@ public class MergeOp implements AutoCloseable { } private BranchBatch validateChangeList(OpenRepo or, - Collection submitted, IdentifiedUser caller) - throws IntegrationException { + Collection submitted) throws IntegrationException { logDebug("Validating {} changes", submitted.size()); List toSubmit = new ArrayList<>(submitted.size()); Multimap revisions = getRevisions(or, submitted); @@ -777,8 +778,8 @@ public class MergeOp implements AutoCloseable { } } - private boolean updateBranch(OpenRepo or, Branch.NameKey destBranch, - IdentifiedUser caller) throws IntegrationException { + private boolean updateBranch(OpenRepo or, Branch.NameKey destBranch) + throws IntegrationException { OpenBranch ob = or.getBranch(destBranch); CodeReviewCommit currentTip = ob.getCurrentTip(); if (Objects.equals(ob.oldTip, currentTip)) { @@ -924,8 +925,8 @@ public class MergeOp implements AutoCloseable { failFast(cs); } - private void updateChangeStatus(OpenBranch ob, List submitted, - IdentifiedUser caller) throws ResourceConflictException { + private void updateChangeStatus(OpenBranch ob, List submitted) + throws ResourceConflictException { List problemChanges = new ArrayList<>(submitted.size()); logDebug("Updating change status for {} changes", submitted.size()); @@ -940,7 +941,7 @@ public class MergeOp implements AutoCloseable { checkState(s != null, "status not set for change %s; expected to previously fail fast", id); - setApproval(cd, caller); + setApproval(cd); ObjectId mergeResultRev = ob.mergeTip != null ? ob.mergeTip.getMergeResults().get(commit) : null; @@ -1093,18 +1094,17 @@ public class MergeOp implements AutoCloseable { }); } - private void setApproval(ChangeData cd, IdentifiedUser user) - throws OrmException, IOException { + private void setApproval(ChangeData cd) throws OrmException, IOException { Timestamp timestamp = TimeUtil.nowTs(); ChangeControl control = cd.changeControl(); PatchSet.Id psId = cd.currentPatchSet().getId(); PatchSet.Id psIdNewRev = commits.get(cd.change().getId()) .change().currentPatchSetId(); - logDebug("Add approval for " + cd + " from user " + user); + logDebug("Add approval for " + cd); ChangeUpdate update = updateFactory.create(control, timestamp); update.setPatchSetId(psId); - update.putReviewer(user.getAccountId(), REVIEWER); + update.putReviewer(caller.getAccountId(), REVIEWER); Optional okRecord = findOkRecord(cd.getSubmitRecords()); if (okRecord.isPresent()) { update.merge(ImmutableList.of(okRecord.get())); @@ -1113,7 +1113,7 @@ public class MergeOp implements AutoCloseable { try { BatchMetaDataUpdate batch = update.openUpdate(); LabelNormalizer.Result normalized = - approve(control, psId, user, update, timestamp); + approve(control, psId, caller, update, timestamp); batch.write(update, new CommitBuilder()); // If the submit strategy created a new revision (rebase, cherry-pick)