Use a common interface for Gerrit permissions

Gerrit has defined permissions for hosts, projects, refs, changes,
labels and plugins. But those permissions are not built on one common
interface even though their behaviors are almost the same. This makes
the life in the PermissionBackend a little bit hard. For example, some
general logic for a permission couldn't be shared with others.

This commit defines the 'GerritPermission' interface and adapts existing
permission class/interface to leverage on this common interface.

Change-Id: I56d5afbf0ddd7703fdbdfa3911929dbfab7ad065
This commit is contained in:
Changcheng Xiao
2018-04-10 13:36:37 +02:00
parent 8c65c396ee
commit 8ded5cb158
10 changed files with 65 additions and 73 deletions

View File

@@ -0,0 +1,31 @@
// Copyright (C) 2018 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.extensions.api.access;
import java.util.Locale;
/** Gerrit permission for hosts, projects, refs, changes, labels and plugins. */
public interface GerritPermission {
/**
* @return the permission name used in {@code project.config} permissions if this permission is
* exposed. Or else, return a lower camel case string of the permission enum.
*/
String permissionName();
/** @return readable identifier of this permission for exception message. */
default String describeForException() {
return toString().toLowerCase(Locale.US).replace('_', ' ');
}
}

View File

@@ -17,10 +17,4 @@ package com.google.gerrit.extensions.api.access;
/**
* A {@link com.google.gerrit.server.permissions.GlobalPermission} or a {@link PluginPermission}.
*/
public interface GlobalOrPluginPermission {
/** @return name used in {@code project.config} permissions. */
public String permissionName();
/** @return readable identifier of this permission for exception message. */
public String describeForException();
}
public interface GlobalOrPluginPermission extends GerritPermission {}

View File

@@ -377,7 +377,7 @@ class ChangeControl {
case REMOVE_REVIEWER:
case SUBMIT_AS:
return refControl.canPerform(perm.permissionName().get());
return refControl.canPerform(perm.permissionName());
}
} catch (OrmException e) {
throw new PermissionBackendException("unavailable", e);
@@ -386,11 +386,11 @@ class ChangeControl {
}
private boolean can(LabelPermission perm) {
return !label(perm.permissionName().get()).isEmpty();
return !label(perm.permissionName()).isEmpty();
}
private boolean can(LabelPermission.WithValue perm) {
PermissionRange r = label(perm.permissionName().get());
PermissionRange r = label(perm.permissionName());
if (perm.forUser() == ON_BEHALF_OF && r.isEmpty()) {
return false;
}

View File

@@ -14,9 +14,8 @@
package com.google.gerrit.server.permissions;
import com.google.common.base.CaseFormat;
import com.google.gerrit.common.data.Permission;
import java.util.Locale;
import java.util.Optional;
public enum ChangePermission implements ChangePermissionOrLabel {
READ(Permission.READ),
@@ -36,21 +35,15 @@ public enum ChangePermission implements ChangePermissionOrLabel {
private final String name;
ChangePermission() {
name = null;
name = CaseFormat.UPPER_UNDERSCORE.to(CaseFormat.LOWER_CAMEL, name());
}
ChangePermission(String name) {
this.name = name;
}
/** @return name used in {@code project.config} permissions. */
@Override
public Optional<String> permissionName() {
return Optional.ofNullable(name);
}
@Override
public String describeForException() {
return toString().toLowerCase(Locale.US).replace('_', ' ');
public String permissionName() {
return name;
}
}

View File

@@ -14,13 +14,7 @@
package com.google.gerrit.server.permissions;
import java.util.Optional;
import com.google.gerrit.extensions.api.access.GerritPermission;
/** A {@link ChangePermission} or a {@link LabelPermission}. */
public interface ChangePermissionOrLabel {
/** @return name used in {@code project.config} permissions. */
public Optional<String> permissionName();
/** @return readable identifier of this permission for exception message. */
public String describeForException();
}
public interface ChangePermissionOrLabel extends GerritPermission {}

View File

@@ -25,7 +25,6 @@ import com.google.gerrit.extensions.api.access.PluginPermission;
import java.lang.annotation.Annotation;
import java.util.Collections;
import java.util.LinkedHashSet;
import java.util.Locale;
import java.util.Set;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -133,11 +132,6 @@ public enum GlobalPermission implements GlobalOrPluginPermission {
return name;
}
@Override
public String describeForException() {
return toString().toLowerCase(Locale.US).replace('_', ' ');
}
private static GlobalOrPluginPermission resolve(
@Nullable String pluginName,
String capability,

View File

@@ -22,7 +22,6 @@ import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.common.data.LabelValue;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.server.util.LabelVote;
import java.util.Optional;
/** Permission representing a label. */
public class LabelPermission implements ChangePermissionOrLabel {
@@ -85,14 +84,11 @@ public class LabelPermission implements ChangePermissionOrLabel {
/** @return name used in {@code project.config} permissions. */
@Override
public Optional<String> permissionName() {
switch (forUser) {
case SELF:
return Optional.of(Permission.forLabel(name));
case ON_BEHALF_OF:
return Optional.of(Permission.forLabelAs(name));
public String permissionName() {
if (forUser == ON_BEHALF_OF) {
return Permission.forLabelAs(name);
}
return Optional.empty();
return Permission.forLabel(name);
}
@Override
@@ -230,14 +226,11 @@ public class LabelPermission implements ChangePermissionOrLabel {
/** @return name used in {@code project.config} permissions. */
@Override
public Optional<String> permissionName() {
switch (forUser) {
case SELF:
return Optional.of(Permission.forLabel(label()));
case ON_BEHALF_OF:
return Optional.of(Permission.forLabelAs(label()));
public String permissionName() {
if (forUser == ON_BEHALF_OF) {
return Permission.forLabelAs(label());
}
return Optional.empty();
return Permission.forLabel(label());
}
@Override

View File

@@ -14,11 +14,11 @@
package com.google.gerrit.server.permissions;
import com.google.common.base.CaseFormat;
import com.google.gerrit.common.data.Permission;
import java.util.Locale;
import java.util.Optional;
import com.google.gerrit.extensions.api.access.GerritPermission;
public enum ProjectPermission {
public enum ProjectPermission implements GerritPermission {
/**
* Can access at least one reference or change within the repository.
*
@@ -88,19 +88,15 @@ public enum ProjectPermission {
private final String name;
ProjectPermission() {
name = null;
name = CaseFormat.UPPER_UNDERSCORE.to(CaseFormat.LOWER_CAMEL, name());
}
ProjectPermission(String name) {
this.name = name;
}
/** @return name used in {@code project.config} permissions. */
public Optional<String> permissionName() {
return Optional.ofNullable(name);
}
public String describeForException() {
return toString().toLowerCase(Locale.US).replace('_', ' ');
@Override
public String permissionName() {
return name;
}
}

View File

@@ -467,7 +467,7 @@ class RefControl {
return isVisible();
case CREATE:
// TODO This isn't an accurate test.
return canPerform(perm.permissionName().get());
return canPerform(perm.permissionName());
case DELETE:
return canDelete();
case UPDATE:
@@ -491,7 +491,7 @@ class RefControl {
case CREATE_TAG:
case CREATE_SIGNED_TAG:
return canPerform(perm.permissionName().get());
return canPerform(perm.permissionName());
case UPDATE_BY_SUBMIT:
return projectControl.controlForRef(MagicBranch.NEW_CHANGE + refName).canSubmit(true);

View File

@@ -14,12 +14,13 @@
package com.google.gerrit.server.permissions;
import com.google.common.base.CaseFormat;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.extensions.api.access.GerritPermission;
import java.util.Arrays;
import java.util.Locale;
import java.util.Optional;
public enum RefPermission {
public enum RefPermission implements GerritPermission {
READ(Permission.READ),
CREATE(Permission.CREATE),
@@ -78,20 +79,16 @@ public enum RefPermission {
private final String name;
RefPermission() {
name = null;
name = CaseFormat.UPPER_UNDERSCORE.to(CaseFormat.LOWER_CAMEL, name());
}
RefPermission(String name) {
this.name = name;
}
/** @return name used in {@code project.config} permissions. */
public Optional<String> permissionName() {
return Optional.ofNullable(name);
}
public String describeForException() {
return toString().toLowerCase(Locale.US).replace('_', ' ');
@Override
public String permissionName() {
return name;
}
/** @return the enum constant for a given permission name if present. */