Check canAbandon and canRestore with PermissionBackend
Change-Id: I22bccfbc3238912521b8e41d83a86cab309a51ea
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
index cbc7cea..4698a80 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
@@ -244,7 +244,7 @@
public void abandon(AbandonInput in) throws RestApiException {
try {
abandon.apply(change, in);
- } catch (OrmException | UpdateException e) {
+ } catch (OrmException | UpdateException | PermissionBackendException e) {
throw new RestApiException("Cannot abandon change", e);
}
}
@@ -258,7 +258,7 @@
public void restore(RestoreInput in) throws RestApiException {
try {
restore.apply(change, in);
- } catch (OrmException | UpdateException e) {
+ } catch (OrmException | UpdateException | PermissionBackendException e) {
throw new RestApiException("Cannot restore change", e);
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java
index 0cafe6d..95e1f2f 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java
@@ -21,7 +21,6 @@
import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.api.changes.RecipientType;
import com.google.gerrit.extensions.common.ChangeInfo;
-import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestModifyView;
@@ -32,6 +31,8 @@
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.git.AbandonOp;
+import com.google.gerrit.server.permissions.ChangePermission;
+import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.UpdateException;
@@ -40,14 +41,10 @@
import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.util.Collection;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
@Singleton
public class Abandon
implements RestModifyView<ChangeResource, AbandonInput>, UiAction<ChangeResource> {
- private static final Logger log = LoggerFactory.getLogger(Abandon.class);
-
private final Provider<ReviewDb> dbProvider;
private final ChangeJson.Factory json;
private final BatchUpdate.Factory batchUpdateFactory;
@@ -70,14 +67,15 @@
@Override
public ChangeInfo apply(ChangeResource req, AbandonInput input)
- throws RestApiException, UpdateException, OrmException {
- ChangeControl control = req.getControl();
- if (!control.canAbandon(dbProvider.get())) {
- throw new AuthException("abandon not permitted");
- }
+ throws RestApiException, UpdateException, OrmException, PermissionBackendException {
+ req.permissions().database(dbProvider).check(ChangePermission.ABANDON);
+
Change change =
abandon(
- control, input.message, input.notify, notifyUtil.resolveAccounts(input.notifyDetails));
+ req.getControl(),
+ input.message,
+ input.notify,
+ notifyUtil.resolveAccounts(input.notifyDetails));
return json.noOptions().format(change);
}
@@ -159,19 +157,14 @@
}
@Override
- public UiAction.Description getDescription(ChangeResource resource) {
- boolean canAbandon = false;
- try {
- canAbandon = resource.getControl().canAbandon(dbProvider.get());
- } catch (OrmException e) {
- log.error("Cannot check canAbandon status. Assuming false.", e);
- }
+ public UiAction.Description getDescription(ChangeResource rsrc) {
+ Change change = rsrc.getChange();
return new UiAction.Description()
.setLabel("Abandon")
.setTitle("Abandon the change")
.setVisible(
- resource.getChange().getStatus().isOpen()
- && resource.getChange().getStatus() != Change.Status.DRAFT
- && canAbandon);
+ change.getStatus().isOpen()
+ && change.getStatus() != Change.Status.DRAFT
+ && rsrc.permissions().database(dbProvider).testOrFalse(ChangePermission.ABANDON));
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Restore.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Restore.java
index b6c4d02..aa3f56d 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Restore.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Restore.java
@@ -18,7 +18,6 @@
import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.extensions.api.changes.RestoreInput;
import com.google.gerrit.extensions.common.ChangeInfo;
-import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestModifyView;
@@ -34,6 +33,8 @@
import com.google.gerrit.server.mail.send.ReplyToChangeSender;
import com.google.gerrit.server.mail.send.RestoredSender;
import com.google.gerrit.server.notedb.ChangeUpdate;
+import com.google.gerrit.server.permissions.ChangePermission;
+import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.BatchUpdateOp;
@@ -80,12 +81,10 @@
@Override
public ChangeInfo apply(ChangeResource req, RestoreInput input)
- throws RestApiException, UpdateException, OrmException {
- ChangeControl ctl = req.getControl();
- if (!ctl.canRestore(dbProvider.get())) {
- throw new AuthException("restore not permitted");
- }
+ throws RestApiException, UpdateException, OrmException, PermissionBackendException {
+ req.permissions().database(dbProvider).check(ChangePermission.RESTORE);
+ ChangeControl ctl = req.getControl();
Op op = new Op(input);
try (BatchUpdate u =
batchUpdateFactory.create(
@@ -150,17 +149,13 @@
}
@Override
- public UiAction.Description getDescription(ChangeResource resource) {
- boolean canRestore = false;
- try {
- canRestore = resource.getControl().canRestore(dbProvider.get());
- } catch (OrmException e) {
- log.error("Cannot check canRestore status. Assuming false.", e);
- }
+ public UiAction.Description getDescription(ChangeResource rsrc) {
return new UiAction.Description()
.setLabel("Restore")
.setTitle("Restore the change")
- .setVisible(resource.getChange().getStatus() == Status.ABANDONED && canRestore);
+ .setVisible(
+ rsrc.getChange().getStatus() == Status.ABANDONED
+ && rsrc.permissions().database(dbProvider).testOrFalse(ChangePermission.RESTORE));
}
private static String status(Change change) {
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 2f5299f..af89d94 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
@@ -246,7 +246,7 @@
}
/** Can this user abandon this change? */
- public boolean canAbandon(ReviewDb db) throws OrmException {
+ private boolean canAbandon(ReviewDb db) throws OrmException {
return (isOwner() // owner (aka creator) of the change can abandon
|| getRefControl().isOwner() // branch owner can abandon
|| getProjectControl().isOwner() // project owner can abandon
@@ -291,7 +291,7 @@
}
/** Can this user restore this change? */
- public boolean canRestore(ReviewDb db) throws OrmException {
+ private boolean canRestore(ReviewDb db) throws OrmException {
return canAbandon(db) // Anyone who can abandon the change can restore it back
&& getRefControl().canUpload(); // as long as you can upload too
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java
index 9f6f8de..50c024e 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java
@@ -402,7 +402,7 @@
}
/** @return true if this user can abandon a change for this ref */
- public boolean canAbandon() {
+ boolean canAbandon() {
return canPerform(Permission.ABANDON);
}
