RebaseUtil: Make all methods non-static
Do depend on the injected ReviewDb provider; callers are all just
passing in their own from their own provider.
Do not depend on the injected GitRepositoryManager. In all but one
existing caller of any method in this class, there is already a repo
available. The consistency in the API is worth shuffling a bit of code
into RevisionApiImpl.
Change-Id: I3df8e7f35ce577a2ba52d60feb2cae8edd33e8cf
diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/RevisionApi.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/RevisionApi.java
index 6ff7889..627917c 100644
--- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/RevisionApi.java
+++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/RevisionApi.java
@@ -39,7 +39,7 @@
ChangeApi cherryPick(CherryPickInput in) throws RestApiException;
ChangeApi rebase() throws RestApiException;
ChangeApi rebase(RebaseInput in) throws RestApiException;
- boolean canRebase();
+ boolean canRebase() throws RestApiException;
void setReviewed(String path, boolean reviewed) throws RestApiException;
Set<String> reviewed() throws RestApiException;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java
index eb16ffb..fe4721a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java
@@ -57,12 +57,16 @@
import com.google.gerrit.server.change.RevisionResource;
import com.google.gerrit.server.change.Submit;
import com.google.gerrit.server.change.TestSubmitType;
+import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.UpdateException;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.assistedinject.Assisted;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevWalk;
+
import java.io.IOException;
import java.util.List;
import java.util.Map;
@@ -73,6 +77,7 @@
RevisionApiImpl create(RevisionResource r);
}
+ private final GitRepositoryManager repoManager;
private final Changes changes;
private final CherryPick cherryPick;
private final DeleteDraftPatchSet deleteDraft;
@@ -101,7 +106,8 @@
private final TestSubmitType.Get getSubmitType;
@Inject
- RevisionApiImpl(Changes changes,
+ RevisionApiImpl(GitRepositoryManager repoManager,
+ Changes changes,
CherryPick cherryPick,
DeleteDraftPatchSet deleteDraft,
Rebase rebase,
@@ -127,6 +133,7 @@
Provider<TestSubmitType> testSubmitType,
TestSubmitType.Get getSubmitType,
@Assisted RevisionResource r) {
+ this.repoManager = repoManager;
this.changes = changes;
this.cherryPick = cherryPick;
this.deleteDraft = deleteDraft;
@@ -213,8 +220,14 @@
}
@Override
- public boolean canRebase() {
- return rebaseUtil.canRebase(revision);
+ public boolean canRebase() throws RestApiException {
+ try (Repository repo = repoManager.openRepository(revision.getProject());
+ RevWalk rw = new RevWalk(repo)) {
+ return rebaseUtil.canRebase(
+ revision.getPatchSet(), revision.getChange().getDest(), repo, rw);
+ } catch (IOException e) {
+ throw new RestApiException("Cannot check if rebase is possible", e);
+ }
}
@Override
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Rebase.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Rebase.java
index 5bcd23c..e01be62 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Rebase.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Rebase.java
@@ -63,6 +63,7 @@
private final BatchUpdate.Factory updateFactory;
private final GitRepositoryManager repoManager;
private final RebaseChangeOp.Factory rebaseFactory;
+ private final RebaseUtil rebaseUtil;
private final ChangeJson.Factory json;
private final Provider<ReviewDb> dbProvider;
@@ -70,11 +71,13 @@
public Rebase(BatchUpdate.Factory updateFactory,
GitRepositoryManager repoManager,
RebaseChangeOp.Factory rebaseFactory,
+ RebaseUtil rebaseUtil,
ChangeJson.Factory json,
Provider<ReviewDb> dbProvider) {
this.updateFactory = updateFactory;
this.repoManager = repoManager;
this.rebaseFactory = rebaseFactory;
+ this.rebaseUtil = rebaseUtil;
this.json = json;
this.dbProvider = dbProvider;
}
@@ -224,8 +227,7 @@
try (Repository repo = repoManager.openRepository(dest.getParentKey());
RevWalk rw = new RevWalk(repo)) {
visible = hasOneParent(rw, resource.getPatchSet());
- enabled =
- RebaseUtil.canRebase(patchSet, dest, repo, rw, dbProvider.get());
+ enabled = rebaseUtil.canRebase(patchSet, dest, repo, rw);
} catch (IOException e) {
log.error("Failed to check if patch set can be rebased: "
+ resource.getPatchSet(), e);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/RebaseChangeOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/RebaseChangeOp.java
index 4164fc0..4f5c787 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/RebaseChangeOp.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/RebaseChangeOp.java
@@ -53,6 +53,7 @@
private final PatchSetInserter.Factory patchSetInserterFactory;
private final MergeUtil.Factory mergeUtilFactory;
+ private final RebaseUtil rebaseUtil;
private final ChangeControl ctl;
private final PatchSet originalPatchSet;
@@ -73,11 +74,13 @@
RebaseChangeOp(
PatchSetInserter.Factory patchSetInserterFactory,
MergeUtil.Factory mergeUtilFactory,
+ RebaseUtil rebaseUtil,
@Assisted ChangeControl ctl,
@Assisted PatchSet originalPatchSet,
@Assisted @Nullable String baseCommitish) {
this.patchSetInserterFactory = patchSetInserterFactory;
this.mergeUtilFactory = mergeUtilFactory;
+ this.rebaseUtil = rebaseUtil;
this.ctl = ctl;
this.originalPatchSet = originalPatchSet;
this.baseCommitish = baseCommitish;
@@ -123,9 +126,9 @@
if (baseCommitish != null) {
baseCommit = rw.parseCommit(ctx.getRepository().resolve(baseCommitish));
} else {
- baseCommit = rw.parseCommit(RebaseUtil.findBaseRevision(
+ baseCommit = rw.parseCommit(rebaseUtil.findBaseRevision(
originalPatchSet, ctl.getChange().getDest(),
- ctx.getRepository(), ctx.getRevWalk(), ctx.getDb()));
+ ctx.getRepository(), ctx.getRevWalk()));
}
ObjectId newId = rebaseCommit(ctx, original, baseCommit);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/RebaseUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/RebaseUtil.java
index aea49f1..fecd776 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/RebaseUtil.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/RebaseUtil.java
@@ -23,7 +23,6 @@
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.reviewdb.server.ReviewDb;
-import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -43,34 +42,16 @@
private static final Logger log = LoggerFactory.getLogger(RebaseUtil.class);
private final Provider<ReviewDb> db;
- private final GitRepositoryManager gitManager;
@Inject
- RebaseUtil(Provider<ReviewDb> db,
- GitRepositoryManager gitManager) {
+ RebaseUtil(Provider<ReviewDb> db) {
this.db = db;
- this.gitManager = gitManager;
}
- public boolean canRebase(RevisionResource r) {
- PatchSet patchSet = r.getPatchSet();
- Branch.NameKey dest = r.getChange().getDest();
- try (Repository git = gitManager.openRepository(dest.getParentKey());
- RevWalk rw = new RevWalk(git)) {
- return canRebase(
- r.getPatchSet(), dest, git, rw, db.get());
- } catch (IOException e) {
- log.warn(String.format(
- "Error checking if patch set %s on %s can be rebased",
- patchSet.getId(), dest), e);
- return false;
- }
- }
-
- public static boolean canRebase(PatchSet patchSet, Branch.NameKey dest,
- Repository git, RevWalk rw, ReviewDb db) {
+ public boolean canRebase(PatchSet patchSet, Branch.NameKey dest,
+ Repository git, RevWalk rw) {
try {
- findBaseRevision(patchSet, dest, git, rw, db);
+ findBaseRevision(patchSet, dest, git, rw);
return true;
} catch (RestApiException e) {
return false;
@@ -98,8 +79,8 @@
* @throws IOException if accessing the repository fails.
* @throws OrmException if accessing the database fails.
*/
- static ObjectId findBaseRevision(PatchSet patchSet,
- Branch.NameKey destBranch, Repository git, RevWalk rw, ReviewDb db)
+ ObjectId findBaseRevision(PatchSet patchSet, Branch.NameKey destBranch,
+ Repository git, RevWalk rw)
throws RestApiException, IOException, OrmException {
String baseRev = null;
RevCommit commit = rw.parseCommit(
@@ -116,9 +97,9 @@
RevId parentRev = new RevId(commit.getParent(0).name());
- for (PatchSet depPatchSet : db.patchSets().byRevision(parentRev)) {
+ for (PatchSet depPatchSet : db.get().patchSets().byRevision(parentRev)) {
Change.Id depChangeId = depPatchSet.getId().getParentKey();
- Change depChange = db.changes().get(depChangeId);
+ Change depChange = db.get().changes().get(depChangeId);
if (!depChange.getDest().equals(destBranch)) {
continue;
}
@@ -136,7 +117,7 @@
+ " dependent change.");
}
PatchSet latestDepPatchSet =
- db.patchSets().get(depChange.currentPatchSetId());
+ db.get().patchSets().get(depChange.currentPatchSetId());
baseRev = latestDepPatchSet.getRevision().get();
}
break;
