opendev/gerrit
Code Issues Proposed changes

RebaseChange: simplify calling conventions

Browse Source 
Clean up some methods by dropping static and using the
instance members to access the database.

Pass a single RevisionResource rather than a whole list of
values derived from the resource by the caller.

Change-Id: Ie3f59d7f83b599ed578357c7e0ddfa76fe3dec86
This commit is contained in:
Shawn Pearce
2015-07-17 16:19:19 -07:00
parent 2e0d475f00
commit 99f7c21318
2 changed files with 34 additions and 51 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
gerrit-server/src/main/java/com/google/gerrit/server/change/Rebase.java
View File

@@ -90,8 +90,7 @@ public class Rebase implements RestModifyView<RevisionResource, RebaseInput>,
throw new ResourceConflictException(
"cannot rebase merge commits or commit with no ancestor");
}
rebaseChange.get().rebase(repo, rw, control, rsrc.getPatchSet().getId(),
rsrc.getUser(), findBaseRev(rw, rsrc, input));
rebaseChange.get().rebase(repo, rw, rsrc, findBaseRev(rw, rsrc, input));
} catch (InvalidChangeOperationException e) {
throw new ResourceConflictException(e.getMessage());
} catch (NoSuchChangeException e) {
@@ -129,6 +128,7 @@ public class Rebase implements RestModifyView<RevisionResource, RebaseInput>,
if (baseChange == null) {
return null;
}
if (!baseChange.getProject().equals(change.getProject())) {
throw new ResourceConflictException(
"base change is in wrong project: " + baseChange.getProject());

81
gerrit-server/src/main/java/com/google/gerrit/server/change/RebaseChange.java
View File

@@ -22,7 +22,6 @@ import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Change.Status;
import com.google.gerrit.reviewdb.client.ChangeMessage;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ChangeUtil;
@@ -97,9 +96,7 @@ public class RebaseChange {
*
* @param git the repository.
* @param rw the RevWalk.
* @param changeControl the control of the change to rebase.
* @param patchSetId the patch set ID to rebase.
* @param uploader the user that creates the rebased patch set.
* @param rsrc revision to rebase.
* @param newBaseRev the commit that should be the new base.
* @throws NoSuchChangeException if the change to which the patch set belongs
* does not exist or is not visible to the user.
@@ -110,30 +107,33 @@ public class RebaseChange {
* @throws InvalidChangeOperationException if rebase is not possible or not
* allowed.
*/
public void rebase(Repository git, RevWalk rw, ChangeControl changeControl,
PatchSet.Id patchSetId, IdentifiedUser uploader, String newBaseRev)
throws NoSuchChangeException, EmailException, OrmException, IOException,
ResourceConflictException, InvalidChangeOperationException {
Change change = changeControl.getChange();
public void rebase(Repository git, RevWalk rw, RevisionResource rsrc,
String newBaseRev) throws NoSuchChangeException, EmailException,
OrmException, IOException, ResourceConflictException,
InvalidChangeOperationException {
Change change = rsrc.getChange();
PatchSet patchSet = rsrc.getPatchSet();
IdentifiedUser uploader = (IdentifiedUser) rsrc.getControl().getCurrentUser();
try (ObjectInserter inserter = git.newObjectInserter()) {
String baseRev = newBaseRev;
if (baseRev == null) {
baseRev = findBaseRevision(
patchSetId, db.get(), change.getDest(), git, rw);
baseRev = findBaseRevision(patchSet, change.getDest(), git, rw);
}
ObjectId baseObjectId = git.resolve(baseRev);
if (baseObjectId == null) {
throw new InvalidChangeOperationException(
"Cannot rebase: Failed to resolve baseRev: " + baseRev);
}
RevCommit baseCommit = rw.parseCommit(baseObjectId);
RevCommit baseCommit = rw.parseCommit(baseObjectId);
PersonIdent committerIdent =
uploader.newCommitterIdent(TimeUtil.nowTs(), serverTimeZone);
rebase(git, rw, inserter, change, patchSetId,
rebase(git, rw, inserter, change, patchSet.getId(),
uploader, baseCommit, mergeUtilFactory.create(
changeControl.getProjectControl().getProjectState(), true),
rsrc.getControl().getProjectControl().getProjectState(), true),
committerIdent, true, ValidatePolicy.GERRIT);
} catch (MergeConflictException e) {
throw new ResourceConflictException(e.getMessage());
@@ -147,9 +147,7 @@ public class RebaseChange {
* this commit's parent, or the destination branch tip in the case where the
* parent's change is merged.
*
* @param patchSetId patch set ID for which the new base commit should be
* found.
* @param db the ReviewDb.
* @param patchSet patch set for which the new base commit should be found.
* @param destBranch the destination branch.
* @param git the repository.
* @param rw the RevWalk.
@@ -159,16 +157,10 @@ public class RebaseChange {
* @throws IOException if accessing the repository fails.
* @throws OrmException if accessing the database fails.
*/
private static String findBaseRevision(PatchSet.Id patchSetId,
ReviewDb db, Branch.NameKey destBranch, Repository git, RevWalk rw)
private String findBaseRevision(PatchSet patchSet,
Branch.NameKey destBranch, Repository git, RevWalk rw)
throws InvalidChangeOperationException, IOException, OrmException {
String baseRev = null;
PatchSet patchSet = db.patchSets().get(patchSetId);
if (patchSet == null) {
throw new InvalidChangeOperationException(
"Patch set " + patchSetId + " not found");
}
RevCommit commit = rw.parseCommit(
ObjectId.fromString(patchSet.getRevision().get()));
@@ -183,9 +175,9 @@ public class RebaseChange {
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;
}
@@ -203,7 +195,7 @@ public class RebaseChange {
+ " dependent change.");
}
PatchSet latestDepPatchSet =
db.patchSets().get(depChange.currentPatchSetId());
db.get().patchSets().get(depChange.currentPatchSetId());
baseRev = latestDepPatchSet.getRevision().get();
}
break;
@@ -343,30 +335,21 @@ public class RebaseChange {
return objectId;
}
public boolean canRebase(ChangeResource r) {
Change c = r.getChange();
return canRebase(c.getProject(), c.currentPatchSetId(), c.getDest());
}
public boolean canRebase(RevisionResource r) {
return canRebase(r.getChange().getProject(),
r.getPatchSet().getId(), r.getChange().getDest());
return canRebase(r.getPatchSet(), r.getChange().getDest());
}
public boolean canRebase(Project.NameKey project, PatchSet.Id patchSetId,
Branch.NameKey branch) {
try (Repository git = gitManager.openRepository(project)) {
try (RevWalk rw = new RevWalk(git)) {
findBaseRevision(patchSetId, db.get(), branch, git, rw);
return true;
} catch (InvalidChangeOperationException e) {
return false;
} catch (OrmException | IOException e) {
log.warn("Error checking if patch set " + patchSetId + " on " + branch
+ " can be rebased", e);
return false;
}
} catch (IOException err) {
private boolean canRebase(PatchSet patchSet, Branch.NameKey dest) {
try (Repository git = gitManager.openRepository(dest.getParentKey());
RevWalk rw = new RevWalk(git)) {
findBaseRevision(patchSet, dest, git, rw);
return true;
} catch (InvalidChangeOperationException e) {
return false;
} catch (OrmException | IOException e) {
log.warn(String.format(
"Error checking if patch set %s on %s can be rebased",
patchSet.getId(), dest), e);
return false;
}
}