From cc13364a62a5a2ad6a06f63ab8bcf1b2e31f99ee Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Wed, 6 Jan 2016 16:29:58 -0500 Subject: [PATCH] SubmitStrategy: Eagerly evaluate server ident Calling get() multiple times may return skewed results, and can clutter code. Running submit strategies is already pretty expensive; don't worry about the cost of one more eager allocation. Change-Id: I7b0b74ab376656f7e48d6343162af0a86765201a --- .../google/gerrit/server/git/strategy/CherryPick.java | 5 ++--- .../google/gerrit/server/git/strategy/MergeAlways.java | 5 ++--- .../gerrit/server/git/strategy/MergeIfNecessary.java | 5 ++--- .../gerrit/server/git/strategy/RebaseIfNecessary.java | 10 ++++------ .../gerrit/server/git/strategy/SubmitStrategy.java | 5 ++--- .../server/git/strategy/SubmitStrategyFactory.java | 2 +- 6 files changed, 13 insertions(+), 19 deletions(-) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/CherryPick.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/CherryPick.java index bb19d0dc5d..03c3eb1d95 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/CherryPick.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/CherryPick.java @@ -150,7 +150,7 @@ public class CherryPick extends SubmitStrategy { args.mergeUtil.createCherryPickCommitMessage(toMerge); PersonIdent committer = args.caller.newCommitterIdent( - ctx.getWhen(), args.serverIdent.get().getTimeZone()); + ctx.getWhen(), args.serverIdent.getTimeZone()); try { newCommit = args.mergeUtil.createCherryPickFromCommit( args.repo, args.inserter, mergeTip.getCurrentTip(), toMerge, @@ -230,8 +230,7 @@ public class CherryPick extends SubmitStrategy { if (args.rw.isMergedInto(mergeTip.getCurrentTip(), toMerge)) { mergeTip.moveTipTo(toMerge, toMerge); } else { - PersonIdent myIdent = - new PersonIdent(args.serverIdent.get(), ctx.getWhen()); + PersonIdent myIdent = new PersonIdent(args.serverIdent, ctx.getWhen()); CodeReviewCommit result = args.mergeUtil.mergeOneCommit(myIdent, myIdent, args.repo, args.rw, args.inserter, args.canMergeFlag, args.destBranch, mergeTip.getCurrentTip(), diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/MergeAlways.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/MergeAlways.java index 4a91b93af7..94fc588a05 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/MergeAlways.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/MergeAlways.java @@ -43,11 +43,10 @@ public class MergeAlways extends SubmitStrategy { } while (!sorted.isEmpty()) { CodeReviewCommit mergedFrom = sorted.remove(0); - PersonIdent serverIdent = args.serverIdent.get(); PersonIdent caller = args.caller.newCommitterIdent( - serverIdent.getWhen(), serverIdent.getTimeZone()); + args.serverIdent.getWhen(), args.serverIdent.getTimeZone()); CodeReviewCommit newTip = - args.mergeUtil.mergeOneCommit(caller, serverIdent, + args.mergeUtil.mergeOneCommit(caller, args.serverIdent, args.repo, args.rw, args.inserter, args.canMergeFlag, args.destBranch, mergeTip.getCurrentTip(), mergedFrom); mergeTip.moveTipTo(newTip, mergedFrom); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/MergeIfNecessary.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/MergeIfNecessary.java index 64e5be10aa..ce9dd5b2d3 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/MergeIfNecessary.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/MergeIfNecessary.java @@ -49,11 +49,10 @@ public class MergeIfNecessary extends SubmitStrategy { // For every other commit do a pair-wise merge. while (!sorted.isEmpty()) { CodeReviewCommit mergedFrom = sorted.remove(0); - PersonIdent serverIdent = args.serverIdent.get(); PersonIdent caller = args.caller.newCommitterIdent( - serverIdent.getWhen(), serverIdent.getTimeZone()); + args.serverIdent.getWhen(), args.serverIdent.getTimeZone()); branchTip = - args.mergeUtil.mergeOneCommit(caller, serverIdent, + args.mergeUtil.mergeOneCommit(caller, args.serverIdent, args.repo, args.rw, args.inserter, args.canMergeFlag, args.destBranch, branchTip, mergedFrom); mergeTip.moveTipTo(branchTip, mergedFrom); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseIfNecessary.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseIfNecessary.java index 6acbab9fe4..b611274341 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseIfNecessary.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseIfNecessary.java @@ -38,7 +38,6 @@ import com.google.gerrit.server.project.NoSuchChangeException; import com.google.gwtorm.server.OrmException; import org.eclipse.jgit.lib.ObjectId; -import org.eclipse.jgit.lib.PersonIdent; import java.io.IOException; import java.util.Collection; @@ -164,7 +163,7 @@ public class RebaseIfNecessary extends SubmitStrategy { // Racy read of patch set is ok; see comments in RebaseChangeOp. args.db.patchSets().get(toMerge.getPatchsetId()), mergeTip.getCurrentTip().name()) - .setCommitterIdent(args.serverIdent.get()) + .setCommitterIdent(args.serverIdent) .setRunHooks(false) .setValidatePolicy(CommitValidators.Policy.NONE); try { @@ -243,12 +242,11 @@ public class RebaseIfNecessary extends SubmitStrategy { mergeTip.moveTipTo(toMerge, toMerge); acceptMergeTip(mergeTip); } else { - PersonIdent myIdent = args.serverIdent.get(); // TODO(dborowitz): Can't use repo from ctx due to canMergeFlag. CodeReviewCommit newTip = args.mergeUtil.mergeOneCommit( - myIdent, myIdent, args.repo, args.rw, args.inserter, - args.canMergeFlag, args.destBranch, mergeTip.getCurrentTip(), - toMerge); + args.serverIdent, args.serverIdent, args.repo, args.rw, + args.inserter, args.canMergeFlag, args.destBranch, + mergeTip.getCurrentTip(), toMerge); mergeTip.moveTipTo(newTip, toMerge); } args.mergeUtil.markCleanMerges(args.rw, args.canMergeFlag, diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategy.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategy.java index c8dd655454..560d55f545 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategy.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategy.java @@ -30,7 +30,6 @@ import com.google.gerrit.server.git.MergeSorter; import com.google.gerrit.server.git.MergeTip; import com.google.gerrit.server.git.MergeUtil; import com.google.gerrit.server.project.ChangeControl; -import com.google.inject.Provider; import org.eclipse.jgit.lib.ObjectInserter; import org.eclipse.jgit.lib.PersonIdent; @@ -52,7 +51,7 @@ import java.util.Set; */ public abstract class SubmitStrategy { static class Arguments { - protected final Provider serverIdent; + protected final PersonIdent serverIdent; protected final ReviewDb db; protected final BatchUpdate.Factory batchUpdateFactory; protected final ChangeControl.GenericFactory changeControlFactory; @@ -68,7 +67,7 @@ public abstract class SubmitStrategy { protected final MergeSorter mergeSorter; protected final IdentifiedUser caller; - Arguments(Provider serverIdent, ReviewDb db, + Arguments(PersonIdent serverIdent, ReviewDb db, BatchUpdate.Factory batchUpdateFactory, ChangeControl.GenericFactory changeControlFactory, Repository repo, CodeReviewRevWalk rw, ObjectInserter inserter, RevFlag canMergeFlag, diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategyFactory.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategyFactory.java index 485e265c45..4eeed6725a 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategyFactory.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategyFactory.java @@ -86,7 +86,7 @@ public class SubmitStrategyFactory { throws IntegrationException, NoSuchProjectException { ProjectState project = getProject(destBranch); SubmitStrategy.Arguments args = new SubmitStrategy.Arguments( - myIdent, db, batchUpdateFactory, changeControlFactory, + myIdent.get(), db, batchUpdateFactory, changeControlFactory, repo, rw, inserter, canMergeFlag, alreadyAccepted, destBranch,approvalsUtil, mergeUtilFactory.create(project), caller); switch (submitType) {