CommitValidatorCache: rename to BranchCommitValidator
Remove the commit cache, since the validator is instantiated per
branch. This means the cache can only do something useful if there are
two pushes to the same ref, eg.
git push origin sha1-1:refs/head/master sha1-2:refs/head/master
this is not a useful-enough action to warrant the complexity of a
cache.
Change-Id: I53f5807ef20538ad855cce81a89f3806a5016047
diff --git a/java/com/google/gerrit/server/git/receive/AsyncReceiveCommits.java b/java/com/google/gerrit/server/git/receive/AsyncReceiveCommits.java
index 0238480..832b8cd 100644
--- a/java/com/google/gerrit/server/git/receive/AsyncReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/AsyncReceiveCommits.java
@@ -94,7 +94,7 @@
// Don't expose the binding for ReceiveCommits.Factory. All callers should
// be using AsyncReceiveCommits.Factory instead.
install(new FactoryModuleBuilder().build(ReceiveCommits.Factory.class));
- install(new FactoryModuleBuilder().build(CommitValidatorCache.Factory.class));
+ install(new FactoryModuleBuilder().build(BranchCommitValidator.Factory.class));
}
@Provides
diff --git a/java/com/google/gerrit/server/git/receive/CommitValidatorCache.java b/java/com/google/gerrit/server/git/receive/BranchCommitValidator.java
similarity index 84%
rename from java/com/google/gerrit/server/git/receive/CommitValidatorCache.java
rename to java/com/google/gerrit/server/git/receive/BranchCommitValidator.java
index ff5b04b..24b6ab1 100644
--- a/java/com/google/gerrit/server/git/receive/CommitValidatorCache.java
+++ b/java/com/google/gerrit/server/git/receive/BranchCommitValidator.java
@@ -16,7 +16,6 @@
import static org.eclipse.jgit.transport.ReceiveCommand.Result.REJECTED_OTHER_REASON;
-import com.google.auto.value.AutoValue;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.reviewdb.client.Branch;
@@ -35,58 +34,48 @@
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
import java.io.IOException;
-import java.util.HashSet;
import java.util.List;
-import java.util.Set;
-import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectReader;
import org.eclipse.jgit.notes.NoteMap;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.transport.ReceiveCommand;
-/** Validates single commits, but uses a cache to avoid duplicating work. */
-public class CommitValidatorCache {
+/** Validates single commits for a branch. */
+public class BranchCommitValidator {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private final CommitValidators.Factory commitValidatorsFactory;
private final IdentifiedUser user;
private final PermissionBackend.ForProject permissions;
private final Project project;
+ private final Branch.NameKey branch;
private final SshInfo sshInfo;
- @AutoValue
- protected abstract static class ValidCommitKey {
- abstract ObjectId getObjectId();
-
- abstract Branch.NameKey getBranch();
- }
-
- private final Set<CommitValidatorCache.ValidCommitKey> validCommits;
-
interface Factory {
- CommitValidatorCache create(ProjectState projectState, IdentifiedUser user);
+ BranchCommitValidator create(
+ ProjectState projectState, Branch.NameKey branch, IdentifiedUser user);
}
@Inject
- CommitValidatorCache(
+ BranchCommitValidator(
CommitValidators.Factory commitValidatorsFactory,
PermissionBackend permissionBackend,
SshInfo sshInfo,
@Assisted ProjectState projectState,
+ @Assisted Branch.NameKey branch,
@Assisted IdentifiedUser user) {
this.sshInfo = sshInfo;
this.user = user;
+ this.branch = branch;
this.commitValidatorsFactory = commitValidatorsFactory;
project = projectState.getProject();
permissions = permissionBackend.user(user).project(project.getNameKey());
- validCommits = new HashSet<>();
}
/**
* Validates a single commit. If the commit does not validate, the command is rejected.
*
* @param objectReader the object reader to use.
- * @param branch the branch to which this commit is pushed
* @param cmd the ReceiveCommand executing the push.
* @param commit the commit being validated.
* @param isMerged whether this is a merge commit created by magicBranch --merge option
@@ -94,7 +83,6 @@
*/
public boolean validCommit(
ObjectReader objectReader,
- Branch.NameKey branch,
ReceiveCommand cmd,
RevCommit commit,
boolean isMerged,
@@ -102,11 +90,6 @@
NoteMap rejectCommits,
@Nullable Change change)
throws IOException {
- ValidCommitKey key = new AutoValue_CommitValidatorCache_ValidCommitKey(commit.copy(), branch);
- if (validCommits.contains(key)) {
- return true;
- }
-
try (CommitReceivedEvent receiveEvent =
new CommitReceivedEvent(cmd, project, branch.get(), objectReader, commit, user)) {
CommitValidators validators;
@@ -140,7 +123,6 @@
cmd.setResult(REJECTED_OTHER_REASON, messageForCommit(commit, e.getMessage()));
return false;
}
- validCommits.add(key);
return true;
}
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index 174cb67..f68f858 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -301,7 +301,7 @@
private final ChangeNotes.Factory notesFactory;
private final ChangeReportFormatter changeFormatter;
private final CmdLineParser.Factory optionParserFactory;
- private final CommitValidatorCache.Factory commitValidatorFactory;
+ private final BranchCommitValidator.Factory commitValidatorFactory;
private final CreateGroupPermissionSyncer createGroupPermissionSyncer;
private final CreateRefControl createRefControl;
private final DynamicMap<ProjectConfigEntry> pluginConfigEntries;
@@ -378,7 +378,7 @@
ChangeNotes.Factory notesFactory,
DynamicItem<ChangeReportFormatter> changeFormatterProvider,
CmdLineParser.Factory optionParserFactory,
- CommitValidatorCache.Factory commitValidatorFactory,
+ BranchCommitValidator.Factory commitValidatorFactory,
CreateGroupPermissionSyncer createGroupPermissionSyncer,
CreateRefControl createRefControl,
DynamicMap<ProjectConfigEntry> pluginConfigEntries,
@@ -1878,11 +1878,11 @@
return;
}
- CommitValidatorCache validator = commitValidatorFactory.create(projectState, user);
+ BranchCommitValidator validator =
+ commitValidatorFactory.create(projectState, changeEnt.getDest(), user);
try {
if (validator.validCommit(
receivePack.getRevWalk().getObjectReader(),
- changeEnt.getDest(),
cmd,
newCommit,
false,
@@ -1945,7 +1945,9 @@
GroupCollector groupCollector =
GroupCollector.create(changeRefsById(), db, psUtil, notesFactory, project.getNameKey());
- CommitValidatorCache validator = commitValidatorFactory.create(projectState, user);
+ BranchCommitValidator validator =
+ commitValidatorFactory.create(projectState, magicBranch.dest, user);
+
try {
RevCommit start = setUpWalkForSelectingChanges();
if (start == null) {
@@ -2047,7 +2049,6 @@
if (!validator.validCommit(
receivePack.getRevWalk().getObjectReader(),
- magicBranch.dest,
magicBranch.cmd,
c,
magicBranch.merged,
@@ -3012,7 +3013,7 @@
return;
}
- CommitValidatorCache validator = commitValidatorFactory.create(projectState, user);
+ BranchCommitValidator validator = commitValidatorFactory.create(projectState, branch, user);
boolean missingFullName = Strings.isNullOrEmpty(user.getAccount().getFullName());
RevWalk walk = receivePack.getRevWalk();
walk.reset();
@@ -3041,7 +3042,7 @@
}
if (!validator.validCommit(
- walk.getObjectReader(), branch, cmd, c, false, messages, rejectCommits, null)) {
+ walk.getObjectReader(), cmd, c, false, messages, rejectCommits, null)) {
break;
}
