Fix stale dates in committer field
The server's PersonIdent cannot be held by a singleton such as
SubmitStrategyFactory. Holding it would retain the timestamp of the
first merge in this process, with the time forever stuck in the past.
Instead use Provider<PersonIdent> so Guice can defer creation of
the server's identity until it is required to write a new commit.
This allows the current date to be injected into a newly created
PersonIdent, keeping the committer field current.
Bug: issue 2773
Change-Id: I9e5edd41e42a702ed8541cfef57660204a8c5b57
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 8568b1e..fbbed67 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
@@ -108,7 +108,7 @@
mergeTip = n;
} else {
mergeTip =
- args.mergeUtil.mergeOneCommit(args.myIdent, args.repo,
+ args.mergeUtil.mergeOneCommit(args.serverIdent.get(), args.repo,
args.rw, args.inserter, args.canMergeFlag,
args.destBranch, mergeTip, n);
}
@@ -147,10 +147,10 @@
cherryPickUser =
args.identifiedUserFactory.create(submitAudit.getAccountId());
cherryPickCommitterIdent = cherryPickUser.newCommitterIdent(
- submitAudit.getGranted(), args.myIdent.getTimeZone());
+ submitAudit.getGranted(), args.serverIdent.get().getTimeZone());
} else {
cherryPickUser = args.identifiedUserFactory.create(n.change().getOwner());
- cherryPickCommitterIdent = args.myIdent;
+ cherryPickCommitterIdent = args.serverIdent.get();
}
final String cherryPickCmtMsg = args.mergeUtil.createCherryPickCommitMessage(n);
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 dc0d721..9023623 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
@@ -38,7 +38,7 @@
}
while (!toMerge.isEmpty()) {
mergeTip =
- args.mergeUtil.mergeOneCommit(args.myIdent, args.repo, args.rw,
+ args.mergeUtil.mergeOneCommit(args.serverIdent.get(), args.repo, args.rw,
args.inserter, args.canMergeFlag, args.destBranch, mergeTip,
toMerge.remove(0));
}
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 601c6f8..b84586f6 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
@@ -43,7 +43,7 @@
// For every other commit do a pair-wise merge.
while (!toMerge.isEmpty()) {
mergeTip =
- args.mergeUtil.mergeOneCommit(args.myIdent, args.repo, args.rw,
+ args.mergeUtil.mergeOneCommit(args.serverIdent.get(), args.repo, args.rw,
args.inserter, args.canMergeFlag, args.destBranch, mergeTip,
toMerge.remove(0));
}
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 c38f5f21..130d170 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
@@ -32,7 +32,6 @@
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.HashMap;
@@ -43,17 +42,14 @@
private final PatchSetInfoFactory patchSetInfoFactory;
private final RebaseChange rebaseChange;
private final Map<Change.Id, CodeReviewCommit> newCommits;
- private final PersonIdent committerIdent;
RebaseIfNecessary(SubmitStrategy.Arguments args,
PatchSetInfoFactory patchSetInfoFactory,
- RebaseChange rebaseChange,
- PersonIdent committerIdent) {
+ RebaseChange rebaseChange) {
super(args);
this.patchSetInfoFactory = patchSetInfoFactory;
this.rebaseChange = rebaseChange;
this.newCommits = new HashMap<>();
- this.committerIdent = committerIdent;
}
@Override
@@ -91,7 +87,7 @@
final PatchSet newPatchSet =
rebaseChange.rebase(args.repo, args.rw, args.inserter,
n.getPatchsetId(), n.change(), uploader,
- newMergeTip, args.mergeUtil, committerIdent,
+ newMergeTip, args.mergeUtil, args.serverIdent.get(),
false, false, ValidatePolicy.NONE);
List<PatchSetApproval> approvals = Lists.newArrayList();
@@ -133,7 +129,7 @@
newMergeTip = n;
} else {
newMergeTip = args.mergeUtil.mergeOneCommit(
- args.myIdent, args.repo, args.rw, args.inserter,
+ args.serverIdent.get(), args.repo, args.rw, args.inserter,
args.canMergeFlag, args.destBranch, newMergeTip, n);
}
final PatchSetApproval submitApproval = args.mergeUtil.markCleanMerges(
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 aa08085..a864b6c 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
@@ -27,6 +27,7 @@
import com.google.gerrit.server.git.MergeUtil;
import com.google.gerrit.server.index.ChangeIndexer;
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 +53,7 @@
static class Arguments {
protected final IdentifiedUser.GenericFactory identifiedUserFactory;
- protected final PersonIdent myIdent;
+ protected final Provider<PersonIdent> serverIdent;
protected final ReviewDb db;
protected final ChangeControl.GenericFactory changeControlFactory;
@@ -68,14 +69,14 @@
protected final MergeSorter mergeSorter;
Arguments(final IdentifiedUser.GenericFactory identifiedUserFactory,
- final PersonIdent myIdent, final ReviewDb db,
+ final Provider<PersonIdent> serverIdent, final ReviewDb db,
final ChangeControl.GenericFactory changeControlFactory,
final Repository repo, final RevWalk rw, final ObjectInserter inserter,
final RevFlag canMergeFlag, final Set<RevCommit> alreadyAccepted,
final Branch.NameKey destBranch, final ApprovalsUtil approvalsUtil,
final MergeUtil mergeUtil, final ChangeIndexer indexer) {
this.identifiedUserFactory = identifiedUserFactory;
- this.myIdent = myIdent;
+ this.serverIdent = serverIdent;
this.db = db;
this.changeControlFactory = changeControlFactory;
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 d427b8d..091523b 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
@@ -31,6 +31,7 @@
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectState;
import com.google.inject.Inject;
+import com.google.inject.Provider;
import com.google.inject.Singleton;
import org.eclipse.jgit.lib.ObjectInserter;
@@ -51,7 +52,7 @@
.getLogger(SubmitStrategyFactory.class);
private final IdentifiedUser.GenericFactory identifiedUserFactory;
- private final PersonIdent myIdent;
+ private final Provider<PersonIdent> myIdent;
private final ChangeControl.GenericFactory changeControlFactory;
private final PatchSetInfoFactory patchSetInfoFactory;
private final GitReferenceUpdated gitRefUpdated;
@@ -64,7 +65,7 @@
@Inject
SubmitStrategyFactory(
final IdentifiedUser.GenericFactory identifiedUserFactory,
- @GerritPersonIdent final PersonIdent myIdent,
+ @GerritPersonIdent Provider<PersonIdent> myIdent,
final ChangeControl.GenericFactory changeControlFactory,
final PatchSetInfoFactory patchSetInfoFactory,
final GitReferenceUpdated gitRefUpdated, final RebaseChange rebaseChange,
@@ -105,8 +106,7 @@
case MERGE_IF_NECESSARY:
return new MergeIfNecessary(args);
case REBASE_IF_NECESSARY:
- return new RebaseIfNecessary(
- args, patchSetInfoFactory, rebaseChange, myIdent);
+ return new RebaseIfNecessary(args, patchSetInfoFactory, rebaseChange);
default:
final String errorMsg = "No submit strategy for: " + submitType;
log.error(errorMsg);
