Migrate Move to PermissionBackend
Change-Id: Ice0bc9099f997e7c3c7d8cde03dd8434fa6c7e70
This commit is contained in:
committed by
David Pursehouse
parent
ad62ecbabf
commit
f537319c22
@@ -163,7 +163,7 @@ public class MoveChangeIT extends AbstractDaemonTest {
|
|||||||
systemGroupBackend.getGroup(REGISTERED_USERS).getUUID(),
|
systemGroupBackend.getGroup(REGISTERED_USERS).getUUID(),
|
||||||
"refs/for/" + newBranch.get());
|
"refs/for/" + newBranch.get());
|
||||||
exception.expect(AuthException.class);
|
exception.expect(AuthException.class);
|
||||||
exception.expectMessage("Move not permitted");
|
exception.expectMessage("move not permitted");
|
||||||
move(r.getChangeId(), newBranch.get());
|
move(r.getChangeId(), newBranch.get());
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -179,7 +179,7 @@ public class MoveChangeIT extends AbstractDaemonTest {
|
|||||||
r.getChange().change().getDest().get());
|
r.getChange().change().getDest().get());
|
||||||
setApiUser(user);
|
setApiUser(user);
|
||||||
exception.expect(AuthException.class);
|
exception.expect(AuthException.class);
|
||||||
exception.expectMessage("Move not permitted");
|
exception.expectMessage("move not permitted");
|
||||||
move(r.getChangeId(), newBranch.get());
|
move(r.getChangeId(), newBranch.get());
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -223,7 +223,7 @@ public class MoveChangeIT extends AbstractDaemonTest {
|
|||||||
revision(r).review(new ReviewInput().label("Patch-Set-Lock", 1));
|
revision(r).review(new ReviewInput().label("Patch-Set-Lock", 1));
|
||||||
|
|
||||||
exception.expect(AuthException.class);
|
exception.expect(AuthException.class);
|
||||||
exception.expectMessage("Move not permitted");
|
exception.expectMessage("move not permitted");
|
||||||
move(r.getChangeId(), newBranch.get());
|
move(r.getChangeId(), newBranch.get());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -22,4 +22,12 @@ public class AuthException extends RestApiException {
|
|||||||
public AuthException(String msg) {
|
public AuthException(String msg) {
|
||||||
super(msg);
|
super(msg);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @param msg message to return to the client.
|
||||||
|
* @param cause cause of this exception.
|
||||||
|
*/
|
||||||
|
public AuthException(String msg, Throwable cause) {
|
||||||
|
super(msg, cause);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -33,10 +33,14 @@ import com.google.gerrit.reviewdb.client.RefNames;
|
|||||||
import com.google.gerrit.reviewdb.server.ReviewDb;
|
import com.google.gerrit.reviewdb.server.ReviewDb;
|
||||||
import com.google.gerrit.server.ChangeMessagesUtil;
|
import com.google.gerrit.server.ChangeMessagesUtil;
|
||||||
import com.google.gerrit.server.ChangeUtil;
|
import com.google.gerrit.server.ChangeUtil;
|
||||||
|
import com.google.gerrit.server.IdentifiedUser;
|
||||||
import com.google.gerrit.server.PatchSetUtil;
|
import com.google.gerrit.server.PatchSetUtil;
|
||||||
import com.google.gerrit.server.git.GitRepositoryManager;
|
import com.google.gerrit.server.git.GitRepositoryManager;
|
||||||
import com.google.gerrit.server.notedb.ChangeUpdate;
|
import com.google.gerrit.server.notedb.ChangeUpdate;
|
||||||
import com.google.gerrit.server.project.ChangeControl;
|
import com.google.gerrit.server.permissions.ChangePermission;
|
||||||
|
import com.google.gerrit.server.permissions.PermissionBackend;
|
||||||
|
import com.google.gerrit.server.permissions.PermissionBackendException;
|
||||||
|
import com.google.gerrit.server.permissions.RefPermission;
|
||||||
import com.google.gerrit.server.query.change.InternalChangeQuery;
|
import com.google.gerrit.server.query.change.InternalChangeQuery;
|
||||||
import com.google.gerrit.server.update.BatchUpdate;
|
import com.google.gerrit.server.update.BatchUpdate;
|
||||||
import com.google.gerrit.server.update.BatchUpdateOp;
|
import com.google.gerrit.server.update.BatchUpdateOp;
|
||||||
@@ -57,6 +61,7 @@ import org.eclipse.jgit.revwalk.RevWalk;
|
|||||||
|
|
||||||
@Singleton
|
@Singleton
|
||||||
public class Move extends RetryingRestModifyView<ChangeResource, MoveInput, ChangeInfo> {
|
public class Move extends RetryingRestModifyView<ChangeResource, MoveInput, ChangeInfo> {
|
||||||
|
private final PermissionBackend permissionBackend;
|
||||||
private final Provider<ReviewDb> dbProvider;
|
private final Provider<ReviewDb> dbProvider;
|
||||||
private final ChangeJson.Factory json;
|
private final ChangeJson.Factory json;
|
||||||
private final GitRepositoryManager repoManager;
|
private final GitRepositoryManager repoManager;
|
||||||
@@ -66,6 +71,7 @@ public class Move extends RetryingRestModifyView<ChangeResource, MoveInput, Chan
|
|||||||
|
|
||||||
@Inject
|
@Inject
|
||||||
Move(
|
Move(
|
||||||
|
PermissionBackend permissionBackend,
|
||||||
Provider<ReviewDb> dbProvider,
|
Provider<ReviewDb> dbProvider,
|
||||||
ChangeJson.Factory json,
|
ChangeJson.Factory json,
|
||||||
GitRepositoryManager repoManager,
|
GitRepositoryManager repoManager,
|
||||||
@@ -74,6 +80,7 @@ public class Move extends RetryingRestModifyView<ChangeResource, MoveInput, Chan
|
|||||||
RetryHelper retryHelper,
|
RetryHelper retryHelper,
|
||||||
PatchSetUtil psUtil) {
|
PatchSetUtil psUtil) {
|
||||||
super(retryHelper);
|
super(retryHelper);
|
||||||
|
this.permissionBackend = permissionBackend;
|
||||||
this.dbProvider = dbProvider;
|
this.dbProvider = dbProvider;
|
||||||
this.json = json;
|
this.json = json;
|
||||||
this.repoManager = repoManager;
|
this.repoManager = repoManager;
|
||||||
@@ -84,22 +91,40 @@ public class Move extends RetryingRestModifyView<ChangeResource, MoveInput, Chan
|
|||||||
|
|
||||||
@Override
|
@Override
|
||||||
protected ChangeInfo applyImpl(
|
protected ChangeInfo applyImpl(
|
||||||
BatchUpdate.Factory updateFactory, ChangeResource req, MoveInput input)
|
BatchUpdate.Factory updateFactory, ChangeResource rsrc, MoveInput input)
|
||||||
throws RestApiException, OrmException, UpdateException {
|
throws RestApiException, OrmException, UpdateException, PermissionBackendException {
|
||||||
ChangeControl control = req.getControl();
|
Change change = rsrc.getChange();
|
||||||
|
Project.NameKey project = rsrc.getProject();
|
||||||
|
IdentifiedUser caller = rsrc.getUser();
|
||||||
input.destinationBranch = RefNames.fullName(input.destinationBranch);
|
input.destinationBranch = RefNames.fullName(input.destinationBranch);
|
||||||
if (!control.canMoveTo(input.destinationBranch, dbProvider.get())) {
|
|
||||||
throw new AuthException("Move not permitted");
|
if (change.getStatus().isClosed()) {
|
||||||
|
throw new ResourceConflictException("Change is " + ChangeUtil.status(change));
|
||||||
|
}
|
||||||
|
|
||||||
|
Branch.NameKey newDest = new Branch.NameKey(project, input.destinationBranch);
|
||||||
|
if (change.getDest().equals(newDest)) {
|
||||||
|
throw new ResourceConflictException("Change is already destined for the specified branch");
|
||||||
|
}
|
||||||
|
|
||||||
|
// Move requires abandoning this change, and creating a new change.
|
||||||
|
try {
|
||||||
|
rsrc.permissions().database(dbProvider).check(ChangePermission.ABANDON);
|
||||||
|
permissionBackend
|
||||||
|
.user(caller)
|
||||||
|
.database(dbProvider)
|
||||||
|
.ref(newDest)
|
||||||
|
.check(RefPermission.CREATE_CHANGE);
|
||||||
|
} catch (AuthException denied) {
|
||||||
|
throw new AuthException("move not permitted", denied);
|
||||||
}
|
}
|
||||||
|
|
||||||
try (BatchUpdate u =
|
try (BatchUpdate u =
|
||||||
updateFactory.create(
|
updateFactory.create(dbProvider.get(), project, caller, TimeUtil.nowTs())) {
|
||||||
dbProvider.get(), req.getChange().getProject(), control.getUser(), TimeUtil.nowTs())) {
|
u.addOp(change.getId(), new Op(input));
|
||||||
u.addOp(req.getChange().getId(), new Op(input));
|
|
||||||
u.execute();
|
u.execute();
|
||||||
}
|
}
|
||||||
|
return json.noOptions().format(project, rsrc.getId());
|
||||||
return json.noOptions().format(req.getChange());
|
|
||||||
}
|
}
|
||||||
|
|
||||||
private class Op implements BatchUpdateOp {
|
private class Op implements BatchUpdateOp {
|
||||||
|
|||||||
@@ -252,11 +252,6 @@ public class ChangeControl {
|
|||||||
&& !isPatchSetLocked(db);
|
&& !isPatchSetLocked(db);
|
||||||
}
|
}
|
||||||
|
|
||||||
/** Can this user change the destination branch of this change to the new ref? */
|
|
||||||
public boolean canMoveTo(String ref, ReviewDb db) throws OrmException {
|
|
||||||
return getProjectControl().controlForRef(ref).canUpload() && canAbandon(db);
|
|
||||||
}
|
|
||||||
|
|
||||||
/** Can this user publish this draft change or any draft patch set of this change? */
|
/** Can this user publish this draft change or any draft patch set of this change? */
|
||||||
public boolean canPublish(final ReviewDb db) throws OrmException {
|
public boolean canPublish(final ReviewDb db) throws OrmException {
|
||||||
return (isOwner() || getRefControl().canPublishDrafts()) && isVisible(db);
|
return (isOwner() || getRefControl().canPublishDrafts()) && isVisible(db);
|
||||||
|
|||||||
Reference in New Issue
Block a user