opendev/gerrit
Code Issues Proposed changes

Avoid casting to CodeReviewCommit

Browse Source 
Override more methods in CodeReviewRevWalk to return or accept only
CodeReviewCommits. Expose this type and use it wherever
CodeReviewCommits are required, to make accidentally passing in a
vanilla RevWalk a compile error.

The only method that could not be effectively changed is the
iterator() method, because generics are no covariant. Callers using
this method would still have to cast, but that operation is still
easier for a human to verify, since they can see that the object being
iterated is actually a CodeReviewRevWalk. However, in practice, only
one caller was using that pattern, so I just converted it to use
next().

Change-Id: I3733240f2fca8236bd8c3894f79e35eff9f53e8c
This commit is contained in:
Dave Borowitz
2015-09-10 11:06:53 -04:00
parent a2583a27bf
commit a0f3ee8bac
12 changed files with 116 additions and 82 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/CherryPick.java
View File

@@ -157,9 +157,9 @@ public class CherryPick extends SubmitStrategy {
PersonIdent committer = args.caller.newCommitterIdent(
TimeUtil.nowTs(), args.serverIdent.get().getTimeZone());
CodeReviewCommit newCommit =
(CodeReviewCommit) args.mergeUtil.createCherryPickFromCommit(args.repo,
args.inserter, mergeTip, n, committer, cherryPickCmtMsg, args.rw);
CodeReviewCommit newCommit = args.mergeUtil.createCherryPickFromCommit(
args.repo, args.inserter, mergeTip, n, committer, cherryPickCmtMsg,
args.rw);
PatchSet.Id id =
ChangeUtil.nextPatchSetId(args.repo, n.change().currentPatchSetId());

2
gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseIfNecessary.java
View File

@@ -97,7 +97,7 @@ public class RebaseIfNecessary extends SubmitStrategy {
// rebaseChange.rebase() may already have copied some approvals,
// use upsert, not insert, to avoid constraint violation on database
args.db.patchSetApprovals().upsert(approvals);
CodeReviewCommit newTip = (CodeReviewCommit) args.rw.parseCommit(
CodeReviewCommit newTip = args.rw.parseCommit(
ObjectId.fromString(newPatchSet.getRevision().get()));
mergeTip.moveTipTo(newTip, newTip);
n.change().setCurrentPatchSet(

6
gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategy.java
View File

@@ -23,6 +23,7 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.git.CodeReviewCommit;
import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk;
import com.google.gerrit.server.git.MergeException;
import com.google.gerrit.server.git.MergeSorter;
import com.google.gerrit.server.git.MergeTip;
@@ -37,7 +38,6 @@ import org.eclipse.jgit.lib.RefUpdate.Result;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevFlag;
import org.eclipse.jgit.revwalk.RevWalk;
import java.util.Collection;
import java.util.Collections;
@@ -58,7 +58,7 @@ public abstract class SubmitStrategy {
protected final ChangeControl.GenericFactory changeControlFactory;
protected final Repository repo;
protected final RevWalk rw;
protected final CodeReviewRevWalk rw;
protected final ObjectInserter inserter;
protected final RevFlag canMergeFlag;
protected final Set<RevCommit> alreadyAccepted;
@@ -72,7 +72,7 @@ public abstract class SubmitStrategy {
Arguments(IdentifiedUser.GenericFactory identifiedUserFactory,
Provider<PersonIdent> serverIdent, ReviewDb db,
ChangeControl.GenericFactory changeControlFactory, Repository repo,
RevWalk rw, ObjectInserter inserter, RevFlag canMergeFlag,
CodeReviewRevWalk rw, ObjectInserter inserter, RevFlag canMergeFlag,
Set<RevCommit> alreadyAccepted, Branch.NameKey destBranch,
ApprovalsUtil approvalsUtil, MergeUtil mergeUtil,
ChangeIndexer indexer, IdentifiedUser caller) {

19
gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategyFactory.java
View File

@@ -22,6 +22,7 @@ import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.change.RebaseChange;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk;
import com.google.gerrit.server.git.MergeException;
import com.google.gerrit.server.git.MergeUtil;
import com.google.gerrit.server.index.ChangeIndexer;
@@ -39,7 +40,6 @@ import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevFlag;
import org.eclipse.jgit.revwalk.RevWalk;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -85,17 +85,16 @@ public class SubmitStrategyFactory {
this.indexer = indexer;
}
public SubmitStrategy create(final SubmitType submitType, final ReviewDb db,
final Repository repo, final RevWalk rw, final ObjectInserter inserter,
final RevFlag canMergeFlag, final Set<RevCommit> alreadyAccepted,
final Branch.NameKey destBranch, final IdentifiedUser caller)
public SubmitStrategy create(SubmitType submitType, ReviewDb db,
Repository repo, CodeReviewRevWalk rw, ObjectInserter inserter,
RevFlag canMergeFlag, Set<RevCommit> alreadyAccepted,
Branch.NameKey destBranch, IdentifiedUser caller)
throws MergeException, NoSuchProjectException {
ProjectState project = getProject(destBranch);
final SubmitStrategy.Arguments args =
new SubmitStrategy.Arguments(identifiedUserFactory, myIdent, db,
changeControlFactory, repo, rw, inserter, canMergeFlag,
alreadyAccepted, destBranch,approvalsUtil,
mergeUtilFactory.create(project), indexer, caller);
SubmitStrategy.Arguments args = new SubmitStrategy.Arguments(
identifiedUserFactory, myIdent, db, changeControlFactory, repo, rw,
inserter, canMergeFlag, alreadyAccepted, destBranch,approvalsUtil,
mergeUtilFactory.create(project), indexer, caller);
switch (submitType) {
case CHERRY_PICK:
return new CherryPick(args, patchSetInfoFactory, gitRefUpdated);