From 4fa6cd25773c38f2bb82ca52a8398183b1b55ec9 Mon Sep 17 00:00:00 2001 From: Luck Date: Sat, 6 Apr 2019 18:20:30 +0100 Subject: [PATCH] Add configuration option to perform a post-traversal sort according to the inheritance (weight) rules --- bukkit/src/main/resources/config.yml | 13 +++++ bungee/src/main/resources/config.yml | 13 +++++ .../luckperms/common/config/ConfigKeys.java | 5 ++ .../inheritance/InheritanceComparator.java | 43 +++++++++++------ .../common/inheritance/InheritanceGraph.java | 48 +++++++++++++++++++ .../inheritance/InheritanceHandler.java | 47 ++++++++---------- .../common/model/PermissionHolder.java | 14 +++--- .../AllParentsByWeightHolder.java | 22 +++------ nukkit/src/main/resources/config.yml | 13 +++++ sponge/src/main/resources/luckperms.conf | 13 +++++ velocity/src/main/resources/config.yml | 13 +++++ 11 files changed, 179 insertions(+), 65 deletions(-) diff --git a/bukkit/src/main/resources/config.yml b/bukkit/src/main/resources/config.yml index 9c1572ce..b9e4d4f5 100644 --- a/bukkit/src/main/resources/config.yml +++ b/bukkit/src/main/resources/config.yml @@ -367,6 +367,19 @@ meta-formatting: # => depth-first-post-order See: https://en.wikipedia.org/wiki/Depth-first_search inheritance-traversal-algorithm: depth-first-pre-order +# If a final sort according to "inheritance rules" should be performed after the traversal algorithm +# has resolved the inheritance tree. +# +# "Inheritance rules" refers to things such as group weightings, primary group status, and the +# natural contextual ordering of the group nodes. +# +# Setting this to 'true' will allow for the inheritance rules to take priority over the structure of +# the inheritance tree. +# +# Effectively when this setting is 'true': the tree is flattened, and rules applied afterwards, +# and when this setting is 'false':, the rules are just applied during each step of the traversal. +post-traversal-inheritance-sort: false + # +----------------------------------------------------------------------------------------------+ # # | Permission resolution settings | # # +----------------------------------------------------------------------------------------------+ # diff --git a/bungee/src/main/resources/config.yml b/bungee/src/main/resources/config.yml index 4a6c2bfc..c04d5870 100644 --- a/bungee/src/main/resources/config.yml +++ b/bungee/src/main/resources/config.yml @@ -375,6 +375,19 @@ meta-formatting: # => depth-first-post-order See: https://en.wikipedia.org/wiki/Depth-first_search inheritance-traversal-algorithm: depth-first-pre-order +# If a final sort according to "inheritance rules" should be performed after the traversal algorithm +# has resolved the inheritance tree. +# +# "Inheritance rules" refers to things such as group weightings, primary group status, and the +# natural contextual ordering of the group nodes. +# +# Setting this to 'true' will allow for the inheritance rules to take priority over the structure of +# the inheritance tree. +# +# Effectively when this setting is 'true': the tree is flattened, and rules applied afterwards, +# and when this setting is 'false':, the rules are just applied during each step of the traversal. +post-traversal-inheritance-sort: false + # +----------------------------------------------------------------------------------------------+ # # | Permission resolution settings | # # +----------------------------------------------------------------------------------------------+ # diff --git a/common/src/main/java/me/lucko/luckperms/common/config/ConfigKeys.java b/common/src/main/java/me/lucko/luckperms/common/config/ConfigKeys.java index 6618925d..70efd817 100644 --- a/common/src/main/java/me/lucko/luckperms/common/config/ConfigKeys.java +++ b/common/src/main/java/me/lucko/luckperms/common/config/ConfigKeys.java @@ -263,6 +263,11 @@ public final class ConfigKeys { } }); + /** + * + */ + public static final ConfigKey POST_TRAVERSAL_INHERITANCE_SORT = booleanKey("post-traversal-inheritance-sort", false); + /** * The configured group weightings */ diff --git a/common/src/main/java/me/lucko/luckperms/common/inheritance/InheritanceComparator.java b/common/src/main/java/me/lucko/luckperms/common/inheritance/InheritanceComparator.java index 26de9eea..71c78d93 100644 --- a/common/src/main/java/me/lucko/luckperms/common/inheritance/InheritanceComparator.java +++ b/common/src/main/java/me/lucko/luckperms/common/inheritance/InheritanceComparator.java @@ -25,10 +25,10 @@ package me.lucko.luckperms.common.inheritance; +import me.lucko.luckperms.common.model.Group; import me.lucko.luckperms.common.model.HolderType; import me.lucko.luckperms.common.model.PermissionHolder; import me.lucko.luckperms.common.model.User; -import me.lucko.luckperms.common.node.comparator.NodeWithContextComparator; import me.lucko.luckperms.common.node.factory.NodeFactory; import java.util.Comparator; @@ -36,10 +36,10 @@ import java.util.Comparator; /** * Determines the order of group inheritance in {@link PermissionHolder}. */ -public class InheritanceComparator implements Comparator { - private static final Comparator NULL_ORIGIN = new InheritanceComparator(null).reversed(); +public class InheritanceComparator implements Comparator { + private static final Comparator NULL_ORIGIN = new InheritanceComparator(null).reversed(); - public static Comparator getFor(PermissionHolder origin) { + public static Comparator getFor(PermissionHolder origin) { if (origin.getType() == HolderType.USER) { return new InheritanceComparator(((User) origin)).reversed(); } @@ -53,25 +53,38 @@ public class InheritanceComparator implements Comparator { } @Override - public int compare(ResolvedGroup o1, ResolvedGroup o2) { - int result = Integer.compare(o1.group().getWeight().orElse(0), o2.group().getWeight().orElse(0)); + public int compare(PermissionHolder o1, PermissionHolder o2) { + // if both users, return 0 + // if one or the other is a user, give a higher priority to the user + boolean o1IsUser = o1.getType() == HolderType.USER; + boolean o2IsUser = o2.getType() == HolderType.USER; + if (o1IsUser && o2IsUser) { + return 0; + } else if (o1IsUser) { + return 1; + } else if (o2IsUser) { + return -1; + } + + Group o1Group = (Group) o1; + Group o2Group = (Group) o2; + + int result = Integer.compare(o1.getWeight().orElse(0), o2.getWeight().orElse(0)); if (result != 0) { return result; } // failing differing group weights, check if one of the groups is a primary group if (this.origin != null) { - result = Boolean.compare( - o1.group().getName().equalsIgnoreCase(this.origin.getPrimaryGroup().getStoredValue().orElse(NodeFactory.DEFAULT_GROUP_NAME)), - o2.group().getName().equalsIgnoreCase(this.origin.getPrimaryGroup().getStoredValue().orElse(NodeFactory.DEFAULT_GROUP_NAME)) + return Boolean.compare( + o1Group.getName().equalsIgnoreCase(this.origin.getPrimaryGroup().getStoredValue().orElse(NodeFactory.DEFAULT_GROUP_NAME)), + o2Group.getName().equalsIgnoreCase(this.origin.getPrimaryGroup().getStoredValue().orElse(NodeFactory.DEFAULT_GROUP_NAME)) ); - - if (result != 0) { - return result; - } } - // failing weight checks, fallback to which group applies in more specific context - return NodeWithContextComparator.normal().compare(o1.node(), o2.node()); + // failing weight/primary group checks, fallback to the node ordering + // this comparator is only ever used by Collections.sort - which is stable. the existing + // ordering of the nodes will therefore apply. + return 0; } } diff --git a/common/src/main/java/me/lucko/luckperms/common/inheritance/InheritanceGraph.java b/common/src/main/java/me/lucko/luckperms/common/inheritance/InheritanceGraph.java index 81efdbe8..92e64dbe 100644 --- a/common/src/main/java/me/lucko/luckperms/common/inheritance/InheritanceGraph.java +++ b/common/src/main/java/me/lucko/luckperms/common/inheritance/InheritanceGraph.java @@ -25,12 +25,60 @@ package me.lucko.luckperms.common.inheritance; +import me.lucko.luckperms.common.config.ConfigKeys; +import me.lucko.luckperms.common.config.LuckPermsConfiguration; import me.lucko.luckperms.common.graph.Graph; +import me.lucko.luckperms.common.graph.TraversalAlgorithm; import me.lucko.luckperms.common.model.PermissionHolder; +import java.util.ArrayList; +import java.util.List; + /** * A {@link Graph} which represents an "inheritance tree". */ public interface InheritanceGraph extends Graph { + /** + * Returns an iterable which will traverse this inheritance graph using the specified + * algorithm starting at the given permission holder start node. + * + * @param algorithm the algorithm to use when traversing + * @param postTraversalSort if a final sort according to inheritance (weight, primary group) rules + * should be performed after the traversal algorithm has completed + * @param startNode the start node in the inheritance graph + * @return an iterable + */ + default Iterable traverse(TraversalAlgorithm algorithm, boolean postTraversalSort, PermissionHolder startNode) { + Iterable traversal = traverse(algorithm, startNode); + + // perform post traversal sort if needed + if (postTraversalSort) { + List resolvedTraversal = new ArrayList<>(); + for (PermissionHolder node : traversal) { + resolvedTraversal.add(node); + } + + resolvedTraversal.sort(startNode.getInheritanceComparator()); + traversal = resolvedTraversal; + } + + return traversal; + } + + /** + * Perform a traversal according to the rules defined in the configuration. + * + * @param configuration the configuration object + * @param startNode the start node in the inheritance graph + * @return an iterable + */ + default Iterable traverse(LuckPermsConfiguration configuration, PermissionHolder startNode) { + return traverse( + configuration.get(ConfigKeys.INHERITANCE_TRAVERSAL_ALGORITHM), + configuration.get(ConfigKeys.POST_TRAVERSAL_INHERITANCE_SORT), + startNode + ); + } + } diff --git a/common/src/main/java/me/lucko/luckperms/common/inheritance/InheritanceHandler.java b/common/src/main/java/me/lucko/luckperms/common/inheritance/InheritanceHandler.java index c4e805aa..b0c35834 100644 --- a/common/src/main/java/me/lucko/luckperms/common/inheritance/InheritanceHandler.java +++ b/common/src/main/java/me/lucko/luckperms/common/inheritance/InheritanceHandler.java @@ -33,9 +33,9 @@ import me.lucko.luckperms.common.model.PermissionHolder; import me.lucko.luckperms.common.plugin.LuckPermsPlugin; import java.util.ArrayList; +import java.util.LinkedHashSet; import java.util.List; import java.util.Set; -import java.util.TreeSet; /** * Provides {@link InheritanceGraph}s. @@ -48,14 +48,12 @@ public class InheritanceHandler { */ private final InheritanceGraph nonContextualGraph; - // some cached contextual graphs for common Contexts - private final InheritanceGraph allowAllContextualGraph; + // cached contextual graphs for common Contexts private final InheritanceGraph globalContextualGraph; public InheritanceHandler(LuckPermsPlugin plugin) { this.plugin = plugin; this.nonContextualGraph = new NonContextualGraph(plugin); - this.allowAllContextualGraph = new ContextualGraph(plugin, Contexts.allowAll()); this.globalContextualGraph = new ContextualGraph(plugin, Contexts.global()); } @@ -65,13 +63,14 @@ public class InheritanceHandler { public InheritanceGraph getGraph(Contexts contexts) { if (contexts == Contexts.allowAll()) { - return this.allowAllContextualGraph; - } - if (contexts == Contexts.global()) { - return this.globalContextualGraph; + throw new IllegalArgumentException("Contexts#allowAll passed to contextual #getGraph method"); } - return new ContextualGraph(this.plugin, contexts); + if (contexts == Contexts.global()) { + return this.globalContextualGraph; + } else { + return new ContextualGraph(this.plugin, contexts); + } } private static final class NonContextualGraph implements InheritanceGraph { @@ -83,15 +82,17 @@ public class InheritanceHandler { @Override public Iterable successors(PermissionHolder holder) { - Set successors = new TreeSet<>(holder.getInheritanceComparator()); - List nodes = holder.getOwnGroupNodes(); - for (Node n : nodes) { + Set successors = new LinkedHashSet<>(); + for (Node n : holder.getOwnGroupNodes()) { Group g = this.plugin.getGroupManager().getIfLoaded(n.getGroupName()); if (g != null) { - successors.add(new ResolvedGroup(n, g)); + successors.add(g); } } - return composeSuccessors(successors); + + List successorsSorted = new ArrayList<>(successors); + successorsSorted.sort(holder.getInheritanceComparator()); + return successorsSorted; } } @@ -110,9 +111,8 @@ public class InheritanceHandler { @Override public Iterable successors(PermissionHolder holder) { - Set successors = new TreeSet<>(holder.getInheritanceComparator()); - List nodes = holder.getOwnGroupNodes(this.context.getContexts()); - for (Node n : nodes) { + Set successors = new LinkedHashSet<>(); + for (Node n : holder.getOwnGroupNodes(this.context.getContexts())) { // effectively: if not (we're applying global groups or it's specific anyways) if (!((this.context.hasSetting(LookupSetting.APPLY_PARENTS_SET_WITHOUT_SERVER) || n.isServerSpecific()) && (this.context.hasSetting(LookupSetting.APPLY_PARENTS_SET_WITHOUT_WORLD) || n.isWorldSpecific()))) { continue; @@ -120,19 +120,14 @@ public class InheritanceHandler { Group g = this.plugin.getGroupManager().getIfLoaded(n.getGroupName()); if (g != null) { - successors.add(new ResolvedGroup(n, g)); + successors.add(g); } } - return composeSuccessors(successors); - } - } - private static Iterable composeSuccessors(Set successors) { - List holders = new ArrayList<>(successors.size()); - for (ResolvedGroup resolvedGroup : successors) { - holders.add(resolvedGroup.group()); + List successorsSorted = new ArrayList<>(successors); + successorsSorted.sort(holder.getInheritanceComparator()); + return successorsSorted; } - return holders; } } diff --git a/common/src/main/java/me/lucko/luckperms/common/model/PermissionHolder.java b/common/src/main/java/me/lucko/luckperms/common/model/PermissionHolder.java index c321c990..e710b01e 100644 --- a/common/src/main/java/me/lucko/luckperms/common/model/PermissionHolder.java +++ b/common/src/main/java/me/lucko/luckperms/common/model/PermissionHolder.java @@ -43,10 +43,8 @@ import me.lucko.luckperms.api.context.ContextSet; import me.lucko.luckperms.api.context.ImmutableContextSet; import me.lucko.luckperms.common.cacheddata.HolderCachedData; import me.lucko.luckperms.common.cacheddata.type.MetaAccumulator; -import me.lucko.luckperms.common.config.ConfigKeys; import me.lucko.luckperms.common.inheritance.InheritanceComparator; import me.lucko.luckperms.common.inheritance.InheritanceGraph; -import me.lucko.luckperms.common.inheritance.ResolvedGroup; import me.lucko.luckperms.common.node.comparator.NodeWithContextComparator; import me.lucko.luckperms.common.node.utils.InheritanceInfo; import me.lucko.luckperms.common.node.utils.MetaType; @@ -130,7 +128,7 @@ public abstract class PermissionHolder { /** * Comparator used to ordering groups when calculating inheritance */ - private final Comparator inheritanceComparator = InheritanceComparator.getFor(this); + private final Comparator inheritanceComparator = InheritanceComparator.getFor(this); /** * Creates a new instance @@ -151,7 +149,7 @@ public abstract class PermissionHolder { return this.ioLock; } - public Comparator getInheritanceComparator() { + public Comparator getInheritanceComparator() { return this.inheritanceComparator; } @@ -321,7 +319,7 @@ public abstract class PermissionHolder { public void accumulateInheritancesTo(List accumulator, Contexts context) { InheritanceGraph graph = this.plugin.getInheritanceHandler().getGraph(context); - Iterable traversal = graph.traverse(this.plugin.getConfiguration().get(ConfigKeys.INHERITANCE_TRAVERSAL_ALGORITHM), this); + Iterable traversal = graph.traverse(this.plugin.getConfiguration(), this); for (PermissionHolder holder : traversal) { List nodes = holder.getOwnNodes(context.getContexts()); accumulator.addAll(nodes); @@ -336,7 +334,7 @@ public abstract class PermissionHolder { public void accumulateInheritancesTo(List accumulator) { InheritanceGraph graph = this.plugin.getInheritanceHandler().getGraph(); - Iterable traversal = graph.traverse(this.plugin.getConfiguration().get(ConfigKeys.INHERITANCE_TRAVERSAL_ALGORITHM), this); + Iterable traversal = graph.traverse(this.plugin.getConfiguration(), this); for (PermissionHolder holder : traversal) { List nodes = holder.getOwnNodes(); accumulator.addAll(nodes); @@ -409,7 +407,7 @@ public abstract class PermissionHolder { } InheritanceGraph graph = this.plugin.getInheritanceHandler().getGraph(context); - Iterable traversal = graph.traverse(this.plugin.getConfiguration().get(ConfigKeys.INHERITANCE_TRAVERSAL_ALGORITHM), this); + Iterable traversal = graph.traverse(this.plugin.getConfiguration(), this); for (PermissionHolder holder : traversal) { List nodes = holder.getOwnNodes(context.getContexts()); for (LocalizedNode node : nodes) { @@ -438,7 +436,7 @@ public abstract class PermissionHolder { } InheritanceGraph graph = this.plugin.getInheritanceHandler().getGraph(); - Iterable traversal = graph.traverse(this.plugin.getConfiguration().get(ConfigKeys.INHERITANCE_TRAVERSAL_ALGORITHM), this); + Iterable traversal = graph.traverse(this.plugin.getConfiguration(), this); for (PermissionHolder holder : traversal) { List nodes = holder.getOwnNodes(); for (LocalizedNode node : nodes) { diff --git a/common/src/main/java/me/lucko/luckperms/common/primarygroup/AllParentsByWeightHolder.java b/common/src/main/java/me/lucko/luckperms/common/primarygroup/AllParentsByWeightHolder.java index 85681aa1..83284c11 100644 --- a/common/src/main/java/me/lucko/luckperms/common/primarygroup/AllParentsByWeightHolder.java +++ b/common/src/main/java/me/lucko/luckperms/common/primarygroup/AllParentsByWeightHolder.java @@ -45,25 +45,15 @@ public class AllParentsByWeightHolder extends ContextualHolder { protected @NonNull Optional calculateValue(Contexts contexts) { InheritanceGraph graph = this.user.getPlugin().getInheritanceHandler().getGraph(contexts); - // fully traverse the graph, obtain a list of permission holders the user inherits from - Iterable traversal = graph.traverse(this.user.getPlugin().getConfiguration().get(ConfigKeys.INHERITANCE_TRAVERSAL_ALGORITHM), this.user); + // fully traverse the graph, obtain a list of permission holders the user inherits from in weight order. + Iterable traversal = graph.traverse(this.user.getPlugin().getConfiguration().get(ConfigKeys.INHERITANCE_TRAVERSAL_ALGORITHM), true, this.user); - Group bestGroup = null; - - int best = 0; + // return the name of the first found group for (PermissionHolder holder : traversal) { - if (!(holder instanceof Group)) { - continue; - } - Group g = ((Group) holder); - - int weight = g.getWeight().orElse(0); - if (bestGroup == null || g.getWeight().orElse(0) > best) { - bestGroup = g; - best = weight; + if (holder instanceof Group) { + return Optional.of(((Group) holder).getName()); } } - - return bestGroup == null ? Optional.empty() : Optional.of(bestGroup.getName()); + return Optional.empty(); } } diff --git a/nukkit/src/main/resources/config.yml b/nukkit/src/main/resources/config.yml index 6bd09500..35590cde 100644 --- a/nukkit/src/main/resources/config.yml +++ b/nukkit/src/main/resources/config.yml @@ -362,6 +362,19 @@ meta-formatting: # => depth-first-post-order See: https://en.wikipedia.org/wiki/Depth-first_search inheritance-traversal-algorithm: depth-first-pre-order +# If a final sort according to "inheritance rules" should be performed after the traversal algorithm +# has resolved the inheritance tree. +# +# "Inheritance rules" refers to things such as group weightings, primary group status, and the +# natural contextual ordering of the group nodes. +# +# Setting this to 'true' will allow for the inheritance rules to take priority over the structure of +# the inheritance tree. +# +# Effectively when this setting is 'true': the tree is flattened, and rules applied afterwards, +# and when this setting is 'false':, the rules are just applied during each step of the traversal. +post-traversal-inheritance-sort: false + # +----------------------------------------------------------------------------------------------+ # # | Permission resolution settings | # # +----------------------------------------------------------------------------------------------+ # diff --git a/sponge/src/main/resources/luckperms.conf b/sponge/src/main/resources/luckperms.conf index 63c3cd21..03818e2b 100644 --- a/sponge/src/main/resources/luckperms.conf +++ b/sponge/src/main/resources/luckperms.conf @@ -376,6 +376,19 @@ meta-formatting { # => depth-first-post-order See: https://en.wikipedia.org/wiki/Depth-first_search inheritance-traversal-algorithm = "depth-first-pre-order" +# If a final sort according to "inheritance rules" should be performed after the traversal algorithm +# has resolved the inheritance tree. +# +# "Inheritance rules" refers to things such as group weightings, primary group status, and the +# natural contextual ordering of the group nodes. +# +# Setting this to 'true' will allow for the inheritance rules to take priority over the structure of +# the inheritance tree. +# +# Effectively when this setting is 'true': the tree is flattened, and rules applied afterwards, +# and when this setting is 'false':, the rules are just applied during each step of the traversal. +post-traversal-inheritance-sort = false + # +----------------------------------------------------------------------------------------------+ # # | Permission resolution settings | # # +----------------------------------------------------------------------------------------------+ # diff --git a/velocity/src/main/resources/config.yml b/velocity/src/main/resources/config.yml index 3f320ff5..ef7b5f5c 100644 --- a/velocity/src/main/resources/config.yml +++ b/velocity/src/main/resources/config.yml @@ -366,6 +366,19 @@ meta-formatting: # => depth-first-post-order See: https://en.wikipedia.org/wiki/Depth-first_search inheritance-traversal-algorithm: depth-first-pre-order +# If a final sort according to "inheritance rules" should be performed after the traversal algorithm +# has resolved the inheritance tree. +# +# "Inheritance rules" refers to things such as group weightings, primary group status, and the +# natural contextual ordering of the group nodes. +# +# Setting this to 'true' will allow for the inheritance rules to take priority over the structure of +# the inheritance tree. +# +# Effectively when this setting is 'true': the tree is flattened, and rules applied afterwards, +# and when this setting is 'false':, the rules are just applied during each step of the traversal. +post-traversal-inheritance-sort: false + # +----------------------------------------------------------------------------------------------+ # # | Permission resolution settings | # # +----------------------------------------------------------------------------------------------+ #