Clean up RefControl by inlining trivial methods and moving state checks
This commit cleans up RefControl by inlining one-line methods directly
where called if the method only has a single caller.
It delegates the project state check for isVisible to ChangeControl and
ForRef.READ as this is the second last occurence of a state check in
RefControl. This splits up further refactoring into two steps which will
happen in subsequent commits.
Change-Id: Ieccf0622beaec2fc5202d660d87e53630469c8ac
diff --git a/java/com/google/gerrit/server/project/ChangeControl.java b/java/com/google/gerrit/server/project/ChangeControl.java
index 9bc59e2..0b1b2a8 100644
--- a/java/com/google/gerrit/server/project/ChangeControl.java
+++ b/java/com/google/gerrit/server/project/ChangeControl.java
@@ -22,6 +22,7 @@
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.data.LabelFunction;
import com.google.gerrit.common.data.LabelType;
+import com.google.gerrit.common.data.Permission;
import com.google.gerrit.common.data.PermissionRange;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.reviewdb.client.Account;
@@ -129,7 +130,7 @@
if (getChange().isPrivate() && !isPrivateVisible(db, cd)) {
return false;
}
- return refControl.isVisible();
+ return refControl.isVisible() && getProjectControl().getProject().getState().permitsRead();
}
/** Can this user abandon this change? */
@@ -137,7 +138,7 @@
return (isOwner() // owner (aka creator) of the change can abandon
|| refControl.isOwner() // branch owner can abandon
|| getProjectControl().isOwner() // project owner can abandon
- || refControl.canAbandon() // user can abandon a specific ref
+ || refControl.canPerform(Permission.ABANDON) // user can abandon a specific ref
|| getProjectControl().isAdmin())
&& !isPatchSetLocked(db);
}
@@ -147,7 +148,8 @@
switch (status) {
case NEW:
case ABANDONED:
- return (isOwner() && refControl.canDeleteOwnChanges()) || getProjectControl().isAdmin();
+ return (isOwner() && refControl.canPerform(Permission.DELETE_OWN_CHANGES))
+ || getProjectControl().isAdmin();
case MERGED:
default:
return false;
@@ -241,7 +243,8 @@
return isOwner() // owner (aka creator) of the change can edit topic
|| refControl.isOwner() // branch owner can edit topic
|| getProjectControl().isOwner() // project owner can edit topic
- || refControl.canEditTopicName() // user can edit topic on a specific ref
+ || refControl.canPerform(
+ Permission.EDIT_TOPIC_NAME) // user can edit topic on a specific ref
|| getProjectControl().isAdmin();
}
return refControl.canForceEditTopicName();
@@ -261,7 +264,7 @@
private boolean canEditAssignee() {
return isOwner()
|| getProjectControl().isOwner()
- || refControl.canEditAssignee()
+ || refControl.canPerform(Permission.EDIT_ASSIGNEE)
|| isAssignee();
}
@@ -270,14 +273,15 @@
return isOwner() // owner (aka creator) of the change can edit hashtags
|| refControl.isOwner() // branch owner can edit hashtags
|| getProjectControl().isOwner() // project owner can edit hashtags
- || refControl.canEditHashtags() // user can edit hashtag on a specific ref
+ || refControl.canPerform(
+ Permission.EDIT_HASHTAGS) // user can edit hashtag on a specific ref
|| getProjectControl().isAdmin();
}
private boolean isPrivateVisible(ReviewDb db, ChangeData cd) throws OrmException {
return isOwner()
|| isReviewer(db, cd)
- || refControl.canViewPrivateChanges()
+ || refControl.canPerform(Permission.VIEW_PRIVATE_CHANGES)
|| getUser().isInternalUser();
}
diff --git a/java/com/google/gerrit/server/project/ProjectState.java b/java/com/google/gerrit/server/project/ProjectState.java
index e3d6078..6a5ecd8 100644
--- a/java/com/google/gerrit/server/project/ProjectState.java
+++ b/java/com/google/gerrit/server/project/ProjectState.java
@@ -259,6 +259,10 @@
return config.getMaxObjectSizeLimit();
}
+ public boolean statePermitsRead() {
+ return getProject().getState().permitsRead();
+ }
+
public boolean statePermitsWrite() {
return getProject().getState().permitsWrite();
}
diff --git a/java/com/google/gerrit/server/project/RefControl.java b/java/com/google/gerrit/server/project/RefControl.java
index c65eb10..ab77ccd 100644
--- a/java/com/google/gerrit/server/project/RefControl.java
+++ b/java/com/google/gerrit/server/project/RefControl.java
@@ -100,9 +100,7 @@
/** Can this user see this reference exists? */
boolean isVisible() {
if (isVisible == null) {
- isVisible =
- (getUser().isInternalUser() || canPerform(Permission.READ))
- && isProjectStatePermittingRead();
+ isVisible = getUser().isInternalUser() || canPerform(Permission.READ);
}
return isVisible;
}
@@ -132,35 +130,6 @@
return canPerform(Permission.SUBMIT, isChangeOwner);
}
- /** @return true if this user can abandon a change for this ref */
- boolean canAbandon() {
- return canPerform(Permission.ABANDON);
- }
-
- /** @return true if this user can view private changes. */
- boolean canViewPrivateChanges() {
- return canPerform(Permission.VIEW_PRIVATE_CHANGES);
- }
-
- /** @return true if this user can delete their own changes. */
- boolean canDeleteOwnChanges() {
- return canPerform(Permission.DELETE_OWN_CHANGES);
- }
-
- /** @return true if this user can edit topic names. */
- boolean canEditTopicName() {
- return canPerform(Permission.EDIT_TOPIC_NAME);
- }
-
- /** @return true if this user can edit hashtag names. */
- boolean canEditHashtags() {
- return canPerform(Permission.EDIT_HASHTAGS);
- }
-
- boolean canEditAssignee() {
- return canPerform(Permission.EDIT_ASSIGNEE);
- }
-
/** @return true if this user can force edit topic names. */
boolean canForceEditTopicName() {
return canForcePerform(Permission.EDIT_TOPIC_NAME);
@@ -189,17 +158,17 @@
return canPerform(permissionName, false);
}
- boolean canPerform(String permissionName, boolean isChangeOwner) {
- return doCanPerform(permissionName, isChangeOwner, false);
- }
-
ForRef asForRef() {
return new ForRefImpl();
}
+ private boolean canPerform(String permissionName, boolean isChangeOwner) {
+ return doCanPerform(permissionName, isChangeOwner, false);
+ }
+
private boolean canUpload() {
return projectControl.controlForRef("refs/for/" + refName).canPerform(Permission.PUSH)
- && isProjectStatePermittingWrite();
+ && getProjectControl().getProject().getState().permitsWrite();
}
/** @return true if this user can submit merge patch sets to this ref */
@@ -246,14 +215,6 @@
}
}
- private boolean isProjectStatePermittingWrite() {
- return getProjectControl().getProject().getState().permitsWrite();
- }
-
- private boolean isProjectStatePermittingRead() {
- return getProjectControl().getProject().getState().permitsRead();
- }
-
private boolean canPushWithForce() {
if (RefNames.REFS_CONFIG.equals(refName) && !projectControl.isOwner()) {
// Pushing requires being at least project owner, in addition to push.
@@ -530,7 +491,7 @@
private boolean can(RefPermission perm) throws PermissionBackendException {
switch (perm) {
case READ:
- return isVisible();
+ return isVisible() && getProjectControl().getProjectState().statePermitsRead();
case CREATE:
// TODO This isn't an accurate test.
return canPerform(perm.permissionName().get());
@@ -562,7 +523,7 @@
return projectControl.controlForRef(MagicBranch.NEW_CHANGE + refName).canSubmit(true);
case READ_PRIVATE_CHANGES:
- return canViewPrivateChanges();
+ return canPerform(Permission.VIEW_PRIVATE_CHANGES);
case READ_CONFIG:
return projectControl
