From 201386e57d1103b08ac838a2180f91673a10f985 Mon Sep 17 00:00:00 2001 From: Luck Date: Sun, 9 Oct 2016 20:28:59 +0100 Subject: [PATCH] Clarify some parts of the PermissionService documentation --- .../service/permission/PermissionService.java | 5 ++ .../api/service/permission/Subject.java | 48 ++++++++++++++++--- .../service/permission/SubjectCollection.java | 9 +++- .../api/service/permission/SubjectData.java | 2 +- 4 files changed, 55 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/spongepowered/api/service/permission/PermissionService.java b/src/main/java/org/spongepowered/api/service/permission/PermissionService.java index c9ac68debb1..b39ed3614f6 100644 --- a/src/main/java/org/spongepowered/api/service/permission/PermissionService.java +++ b/src/main/java/org/spongepowered/api/service/permission/PermissionService.java @@ -64,6 +64,11 @@ public interface PermissionService extends ContextualService { * subject is at the root of all inheritance trees, above even subject type-specific * defaults, meaning it has the lowest priority when all other weighting is equal. * + *

Note: This data should be persisted, so plugins that add permissions to this subject + * must take care to not override permissions already set or modified. It is also + * recommended to use {@link Subject#getTransientSubjectData()} where possible to + * avoid persisting unnecessary data. + * * @return The default subject data */ Subject getDefaults(); diff --git a/src/main/java/org/spongepowered/api/service/permission/Subject.java b/src/main/java/org/spongepowered/api/service/permission/Subject.java index f3cc759d061..af025978971 100644 --- a/src/main/java/org/spongepowered/api/service/permission/Subject.java +++ b/src/main/java/org/spongepowered/api/service/permission/Subject.java @@ -48,6 +48,11 @@ * child levels would granted but {@code example.access} would denied (for * example). * + *

It is the responsibility of the {@link PermissionService} implementation + * to provide this behavior, and resolve the implicit permission node inheritance + * explained above when a Subject is queried for permissions. Use of a + * {@link NodeTree} is recommended. + * *

Plugins may opt to implement "dynamic" permissions such as {@code * example.region.define.<region>} where {@code region} would be a * dynamically added level based on the context, though some attention should be @@ -59,7 +64,15 @@ * administrators to grant {@code example.function.self} to permit usage on * one's self and grant {@code example.function} to grant usage on other users. * - *

Use a {@link PermissionService} to create instances. + *

All methods are expected unlike {@link SubjectData}, to account for data + * inherited from parent subjects. For a representation of the data that the + * subject explicitly holds, obtain the {@link SubjectData} for the object. + * + *

Additionally, all methods are expected to account for the defaults + * defined in the {@link SubjectCollection} containing this subject, as well + * as the defaults set globally in {@link PermissionService#getDefaults()}. + * + *

Use a {@link SubjectCollection} to create instances. * * @see PermissionService */ @@ -101,14 +114,17 @@ public interface Subject extends Contextual { SubjectData getTransientSubjectData(); /** - * Test whether the subject is permitted to perform an action given as the - * given permission string. + * Test whether the subject is permitted to perform an action corresponding to the + * given permission string. This must return the same boolean equivalent as + * {@link #getPermissionValue(Set, String)}. * * @param contexts The set of contexts that represents the subject's current environment * @param permission The permission string * @return True if permission is granted */ - boolean hasPermission(Set contexts, String permission); + default boolean hasPermission(Set contexts, String permission) { + return getPermissionValue(contexts, permission).asBoolean(); + } /** * Test whether the subject is permitted to perform an action given as the @@ -125,6 +141,17 @@ default boolean hasPermission(String permission) { /** * Returns the calculated value set for a given permission. * + *

It is expected that this method will also account for values + * inherited from parent subjects, as well as permission nodes inherited + * implicitly from a more generic level. + * + *

Additionally, the defaults defined the {@link SubjectCollection} + * that holds this subject, as well as defaults defined in + * {@link PermissionService#getDefaults()} should be considered for this lookup. + * + *

This method is likely to be called frequently, so it is desirable + * that implementations cache the results to method calls. + * * @param contexts The contexts to check for permissions in * @param permission The permission to check * @return The tristate true/false/unset value for permissions @@ -134,7 +161,7 @@ default boolean hasPermission(String permission) { /** * Check if this subject is a child of the given parent in the subject's * current context, traversing inheritance. This must return the same value as - * {@link #hasPermission(Set, String)} using {@link #getActiveContexts()}. + * {@link #isChildOf(Set, Subject)} using {@link #getActiveContexts()}. * * @param parent The parent to check for inheritance * @return Whether this is a child of the given parent @@ -157,7 +184,7 @@ default boolean isChildOf(Subject parent) { * Return all parents that this group has in its current context * combination. This must include inherited values if the permissions * service supports inheritance. This must return the same value as - * {@link #hasPermission(Set, String)} using {@link #getActiveContexts()}. + * {@link #getParents(Set)} using {@link #getActiveContexts()}. * * @return An immutable list of parents */ @@ -177,6 +204,13 @@ default List getParents() { /** * Get the value of a given option in the given context. * + *

It is expected that this method will account for options + * inherited from parent subjects. + * + *

Additionally, the default options defined the {@link SubjectCollection} + * that holds this subject, as well as defaults defined in + * {@link PermissionService#getDefaults()} should be considered for this lookup. + * * @param contexts The contexts to get the options from * @param key The key to get an option by. Case-insensitive. * @return The value of the option, if any is present @@ -185,7 +219,7 @@ default List getParents() { /** * Get the value of a given option in the subject's current context - * This must return the same value as {@link #hasPermission(Set, String)} + * This must return the same value as {@link #getOption(Set, String)} * using {@link #getActiveContexts()}. * * @param key The key to get an option by. Case-insensitive. diff --git a/src/main/java/org/spongepowered/api/service/permission/SubjectCollection.java b/src/main/java/org/spongepowered/api/service/permission/SubjectCollection.java index 4b4176de294..b698f7c9f15 100644 --- a/src/main/java/org/spongepowered/api/service/permission/SubjectCollection.java +++ b/src/main/java/org/spongepowered/api/service/permission/SubjectCollection.java @@ -92,7 +92,14 @@ public interface SubjectCollection { Map getAllWithPermission(Set contexts, String permission); /** - * Get the subject that provides defaults for subjects of this type. This subject is placed at the root of any inheritance tree involving subjects of this type. + * Get the subject holding data that is applied by default for subjects of this type. + * This subject is placed at the root of any inheritance tree involving subjects of + * this type, but has a higher priority than {@link PermissionService#getDefaults()}. + * + *

Note: This data may be persisted, so plugins that add permissions to this subject + * must take care to not override permissions already set or modified. It is also + * recommended to use {@link Subject#getTransientSubjectData()} where possible to + * avoid persisting unnecessary data. * * @return The subject holding defaults */ diff --git a/src/main/java/org/spongepowered/api/service/permission/SubjectData.java b/src/main/java/org/spongepowered/api/service/permission/SubjectData.java index 4947e5e45b1..23e962d157c 100644 --- a/src/main/java/org/spongepowered/api/service/permission/SubjectData.java +++ b/src/main/java/org/spongepowered/api/service/permission/SubjectData.java @@ -179,7 +179,7 @@ public interface SubjectData { * Clear all options in the given context combination. * * @param contexts The context combination - * @return Whether the operation was successful (any options were remowed) + * @return Whether the operation was successful (any options were removed) */ boolean clearOptions(Set contexts);