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
This commit is contained in:
Patrick Hiesel
2018-01-15 16:31:44 +01:00
parent f5760b4baf
commit 2539552f58
3 changed files with 23 additions and 54 deletions

View File

@@ -22,6 +22,7 @@ import com.google.common.collect.Sets;
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 @@ class ChangeControl {
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 @@ class ChangeControl {
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 @@ class ChangeControl {
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 @@ class ChangeControl {
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 @@ class ChangeControl {
private boolean canEditAssignee() {
return isOwner()
|| getProjectControl().isOwner()
|| refControl.canEditAssignee()
|| refControl.canPerform(Permission.EDIT_ASSIGNEE)
|| isAssignee();
}
@@ -270,14 +273,15 @@ class ChangeControl {
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();
}

View File

@@ -259,6 +259,10 @@ public class ProjectState {
return config.getMaxObjectSizeLimit();
}
public boolean statePermitsRead() {
return getProject().getState().permitsRead();
}
public boolean statePermitsWrite() {
return getProject().getState().permitsWrite();
}

View File

@@ -100,9 +100,7 @@ class RefControl {
/** 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 @@ class RefControl {
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 @@ class RefControl {
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 @@ class RefControl {
}
}
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 @@ class RefControl {
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 @@ class RefControl {
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