Make project state check in REBASE explicit
The majority of code in {Project,Ref,Change}Control is now about
permissions, but not all. Exceptions include checks for a project's
state. This is confusing, because users are presented with an exception
telling them that they lack some kind of permission while the real
reason for the failed operation is that the project's current state
doesn't permit the operation.
This is part of a series of commits to remove all project state checks
from *Control classes and make explicit checks instead.
Change-Id: I50d5f6608c55838bf8afe9f19432d97b41130d21
This commit is contained in:
@@ -116,7 +116,7 @@ class RefControl {
|
|||||||
|
|
||||||
/** @return true if this user can rebase changes on this ref */
|
/** @return true if this user can rebase changes on this ref */
|
||||||
boolean canRebase() {
|
boolean canRebase() {
|
||||||
return canPerform(Permission.REBASE) && isProjectStatePermittingWrite();
|
return canPerform(Permission.REBASE);
|
||||||
}
|
}
|
||||||
|
|
||||||
/** @return true if this user can submit patch sets to this ref */
|
/** @return true if this user can submit patch sets to this ref */
|
||||||
|
|||||||
@@ -47,6 +47,7 @@ import com.google.gerrit.server.permissions.ChangePermission;
|
|||||||
import com.google.gerrit.server.permissions.PermissionBackend;
|
import com.google.gerrit.server.permissions.PermissionBackend;
|
||||||
import com.google.gerrit.server.permissions.PermissionBackendException;
|
import com.google.gerrit.server.permissions.PermissionBackendException;
|
||||||
import com.google.gerrit.server.project.NoSuchChangeException;
|
import com.google.gerrit.server.project.NoSuchChangeException;
|
||||||
|
import com.google.gerrit.server.project.ProjectCache;
|
||||||
import com.google.gerrit.server.update.BatchUpdate;
|
import com.google.gerrit.server.update.BatchUpdate;
|
||||||
import com.google.gerrit.server.update.RetryHelper;
|
import com.google.gerrit.server.update.RetryHelper;
|
||||||
import com.google.gerrit.server.update.RetryingRestModifyView;
|
import com.google.gerrit.server.update.RetryingRestModifyView;
|
||||||
@@ -79,6 +80,7 @@ public class Rebase extends RetryingRestModifyView<RevisionResource, RebaseInput
|
|||||||
private final ChangeJson.Factory json;
|
private final ChangeJson.Factory json;
|
||||||
private final Provider<ReviewDb> dbProvider;
|
private final Provider<ReviewDb> dbProvider;
|
||||||
private final PermissionBackend permissionBackend;
|
private final PermissionBackend permissionBackend;
|
||||||
|
private final ProjectCache projectCache;
|
||||||
|
|
||||||
@Inject
|
@Inject
|
||||||
public Rebase(
|
public Rebase(
|
||||||
@@ -88,7 +90,8 @@ public class Rebase extends RetryingRestModifyView<RevisionResource, RebaseInput
|
|||||||
RebaseUtil rebaseUtil,
|
RebaseUtil rebaseUtil,
|
||||||
ChangeJson.Factory json,
|
ChangeJson.Factory json,
|
||||||
Provider<ReviewDb> dbProvider,
|
Provider<ReviewDb> dbProvider,
|
||||||
PermissionBackend permissionBackend) {
|
PermissionBackend permissionBackend,
|
||||||
|
ProjectCache projectCache) {
|
||||||
super(retryHelper);
|
super(retryHelper);
|
||||||
this.repoManager = repoManager;
|
this.repoManager = repoManager;
|
||||||
this.rebaseFactory = rebaseFactory;
|
this.rebaseFactory = rebaseFactory;
|
||||||
@@ -96,6 +99,7 @@ public class Rebase extends RetryingRestModifyView<RevisionResource, RebaseInput
|
|||||||
this.json = json;
|
this.json = json;
|
||||||
this.dbProvider = dbProvider;
|
this.dbProvider = dbProvider;
|
||||||
this.permissionBackend = permissionBackend;
|
this.permissionBackend = permissionBackend;
|
||||||
|
this.projectCache = projectCache;
|
||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
@@ -104,6 +108,7 @@ public class Rebase extends RetryingRestModifyView<RevisionResource, RebaseInput
|
|||||||
throws EmailException, OrmException, UpdateException, RestApiException, IOException,
|
throws EmailException, OrmException, UpdateException, RestApiException, IOException,
|
||||||
NoSuchChangeException, PermissionBackendException {
|
NoSuchChangeException, PermissionBackendException {
|
||||||
rsrc.permissions().database(dbProvider).check(ChangePermission.REBASE);
|
rsrc.permissions().database(dbProvider).check(ChangePermission.REBASE);
|
||||||
|
projectCache.checkedGet(rsrc.getProject()).checkStatePermitsWrite();
|
||||||
|
|
||||||
Change change = rsrc.getChange();
|
Change change = rsrc.getChange();
|
||||||
try (Repository repo = repoManager.openRepository(change.getProject());
|
try (Repository repo = repoManager.openRepository(change.getProject());
|
||||||
@@ -205,6 +210,13 @@ public class Rebase extends RetryingRestModifyView<RevisionResource, RebaseInput
|
|||||||
boolean visible = change.getStatus().isOpen() && resource.isCurrent();
|
boolean visible = change.getStatus().isOpen() && resource.isCurrent();
|
||||||
boolean enabled = false;
|
boolean enabled = false;
|
||||||
|
|
||||||
|
try {
|
||||||
|
visible &= projectCache.checkedGet(resource.getProject()).statePermitsWrite();
|
||||||
|
} catch (IOException e) {
|
||||||
|
log.error("Failed to check if project state permits write: " + resource.getProject(), e);
|
||||||
|
visible = false;
|
||||||
|
}
|
||||||
|
|
||||||
if (visible) {
|
if (visible) {
|
||||||
try (Repository repo = repoManager.openRepository(dest.getParentKey());
|
try (Repository repo = repoManager.openRepository(dest.getParentKey());
|
||||||
RevWalk rw = new RevWalk(repo)) {
|
RevWalk rw = new RevWalk(repo)) {
|
||||||
|
|||||||
Reference in New Issue
Block a user