Migrate Move to PermissionBackend
Change-Id: Ice0bc9099f997e7c3c7d8cde03dd8434fa6c7e70
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/MoveChangeIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/MoveChangeIT.java
index 36f8452..552d7a4 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/MoveChangeIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/MoveChangeIT.java
@@ -163,7 +163,7 @@
systemGroupBackend.getGroup(REGISTERED_USERS).getUUID(),
"refs/for/" + newBranch.get());
exception.expect(AuthException.class);
- exception.expectMessage("Move not permitted");
+ exception.expectMessage("move not permitted");
move(r.getChangeId(), newBranch.get());
}
@@ -179,7 +179,7 @@
r.getChange().change().getDest().get());
setApiUser(user);
exception.expect(AuthException.class);
- exception.expectMessage("Move not permitted");
+ exception.expectMessage("move not permitted");
move(r.getChangeId(), newBranch.get());
}
@@ -223,7 +223,7 @@
revision(r).review(new ReviewInput().label("Patch-Set-Lock", 1));
exception.expect(AuthException.class);
- exception.expectMessage("Move not permitted");
+ exception.expectMessage("move not permitted");
move(r.getChangeId(), newBranch.get());
}
diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/restapi/AuthException.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/restapi/AuthException.java
index 1d4cda7..0b4f459 100644
--- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/restapi/AuthException.java
+++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/restapi/AuthException.java
@@ -22,4 +22,12 @@
public AuthException(String 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);
+ }
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Move.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Move.java
index 98b79d1..ffc0dc4 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Move.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Move.java
@@ -33,10 +33,14 @@
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.ChangeUtil;
+import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.git.GitRepositoryManager;
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.update.BatchUpdate;
import com.google.gerrit.server.update.BatchUpdateOp;
@@ -57,6 +61,7 @@
@Singleton
public class Move extends RetryingRestModifyView<ChangeResource, MoveInput, ChangeInfo> {
+ private final PermissionBackend permissionBackend;
private final Provider<ReviewDb> dbProvider;
private final ChangeJson.Factory json;
private final GitRepositoryManager repoManager;
@@ -66,6 +71,7 @@
@Inject
Move(
+ PermissionBackend permissionBackend,
Provider<ReviewDb> dbProvider,
ChangeJson.Factory json,
GitRepositoryManager repoManager,
@@ -74,6 +80,7 @@
RetryHelper retryHelper,
PatchSetUtil psUtil) {
super(retryHelper);
+ this.permissionBackend = permissionBackend;
this.dbProvider = dbProvider;
this.json = json;
this.repoManager = repoManager;
@@ -84,22 +91,40 @@
@Override
protected ChangeInfo applyImpl(
- BatchUpdate.Factory updateFactory, ChangeResource req, MoveInput input)
- throws RestApiException, OrmException, UpdateException {
- ChangeControl control = req.getControl();
+ BatchUpdate.Factory updateFactory, ChangeResource rsrc, MoveInput input)
+ throws RestApiException, OrmException, UpdateException, PermissionBackendException {
+ Change change = rsrc.getChange();
+ Project.NameKey project = rsrc.getProject();
+ IdentifiedUser caller = rsrc.getUser();
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 =
- updateFactory.create(
- dbProvider.get(), req.getChange().getProject(), control.getUser(), TimeUtil.nowTs())) {
- u.addOp(req.getChange().getId(), new Op(input));
+ updateFactory.create(dbProvider.get(), project, caller, TimeUtil.nowTs())) {
+ u.addOp(change.getId(), new Op(input));
u.execute();
}
-
- return json.noOptions().format(req.getChange());
+ return json.noOptions().format(project, rsrc.getId());
}
private class Op implements BatchUpdateOp {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java
index 16c63c8..0b4c199 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java
@@ -252,11 +252,6 @@
&& !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? */
public boolean canPublish(final ReviewDb db) throws OrmException {
return (isOwner() || getRefControl().canPublishDrafts()) && isVisible(db);
