Fallback to node/context ordering in InheritanceComparator, refactor graph traverser classes

common/src/main/java/me/lucko/luckperms/common/graph/Graph.java

@@ -50,7 +50,7 @@ public interface Graph<N> {
      * @return an iterable
      */
     default Iterable<N> traverse(TraversalAlgorithm algorithm, N startNode) {
-        return GraphTraversers.traverseUsing(algorithm, this, startNode);
+        return algorithm.traverse(this, startNode);
     }
 
 }

common/src/main/java/me/lucko/luckperms/common/graph/GraphTraversers.java

@@ -1,214 +0,0 @@
-/*
- * This file is part of LuckPerms, licensed under the MIT License.
- *
- *  Copyright (c) lucko (Luck) <luck@lucko.me>
- *  Copyright (c) contributors
- *
- *  Permission is hereby granted, free of charge, to any person obtaining a copy
- *  of this software and associated documentation files (the "Software"), to deal
- *  in the Software without restriction, including without limitation the rights
- *  to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
- *  copies of the Software, and to permit persons to whom the Software is
- *  furnished to do so, subject to the following conditions:
- *
- *  The above copyright notice and this permission notice shall be included in all
- *  copies or substantial portions of the Software.
- *
- *  THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- *  IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- *  FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
- *  AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- *  LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
- *  OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
- *  SOFTWARE.
- */
-
-/*
- * Copyright (C) 2017 The Guava Authors
- *
- * 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 me.lucko.luckperms.common.graph;
-
-import com.google.common.collect.AbstractIterator;
-
-import java.util.ArrayDeque;
-import java.util.Deque;
-import java.util.HashSet;
-import java.util.Iterator;
-import java.util.Objects;
-import java.util.Queue;
-import java.util.Set;
-
-/**
- * A collection of graph traversal algorithms.
- *
- * @author Jens Nyman
- */
-public final class GraphTraversers {
-    private GraphTraversers() {}
-
-    /**
-     * Returns an unmodifiable {@code Iterable} over the nodes reachable from
-     * {@code startNode}, in the order defined by the {@code algorithm}.
-     */
-    public static <N> Iterable<N> traverseUsing(TraversalAlgorithm algorithm, Graph<N> graph, N startNode) {
-        Objects.requireNonNull(algorithm, "algorithm");
-        switch (algorithm) {
-            case BREADTH_FIRST:
-                return breadthFirst(graph, startNode);
-            case DEPTH_FIRST_PRE_ORDER:
-                return depthFirstPreOrder(graph, startNode);
-            case DEPTH_FIRST_POST_ORDER:
-                return depthFirstPostOrder(graph, startNode);
-            default:
-                throw new AssertionError();
-        }
-    }
-
-    /**
-     * Returns an unmodifiable {@code Iterable} over the nodes reachable from
-     * {@code startNode}, in the order of a breadth-first traversal. That is,
-     * all the nodes of depth 0 are returned, then depth 1, then 2, and so on.
-     *
-     * <p>See <a href="https://en.wikipedia.org/wiki/Breadth-first_search">Wikipedia</a> for more info.</p>
-     */
-    public static <N> Iterable<N> breadthFirst(Graph<N> graph, N startNode) {
-        Objects.requireNonNull(graph, "graph");
-        Objects.requireNonNull(startNode, "startNode");
-        return () -> new BreadthFirstIterator<>(graph, startNode);
-    }
-
-    /**
-     * Returns an unmodifiable {@code Iterable} over the nodes reachable from
-     * {@code startNode}, in the order of a depth-first pre-order traversal.
-     * "Pre-order" implies that nodes appear in the {@code Iterable} in the
-     * order in which they are first visited.
-     *
-     * <p>See <a href="https://en.wikipedia.org/wiki/Depth-first_search">Wikipedia</a> for more info.</p>
-     */
-    public static <N> Iterable<N> depthFirstPreOrder(Graph<N> graph, N startNode) {
-        Objects.requireNonNull(graph, "graph");
-        Objects.requireNonNull(startNode, "startNode");
-        return () -> new DepthFirstIterator<>(graph, startNode, Order.PRE_ORDER);
-    }
-
-    /**
-     * Returns an unmodifiable {@code Iterable} over the nodes reachable from {@code startNode}, in
-     * the order of a depth-first post-order traversal. "Post-order" implies that nodes appear in the
-     * {@code Iterable} in the order in which they are visited for the last time.
-     *
-     * <p>See <a href="https://en.wikipedia.org/wiki/Depth-first_search">Wikipedia</a> for more info.</p>
-     */
-    public static <N> Iterable<N> depthFirstPostOrder(Graph<N> graph, N startNode) {
-        Objects.requireNonNull(graph, "graph");
-        Objects.requireNonNull(startNode, "startNode");
-        return () -> new DepthFirstIterator<>(graph, startNode, Order.POST_ORDER);
-    }
-
-    private static final class BreadthFirstIterator<N> implements Iterator<N> {
-        private final Graph<N> graph;
-
-        private final Queue<N> queue = new ArrayDeque<>();
-        private final Set<N> visited = new HashSet<>();
-
-        BreadthFirstIterator(Graph<N> graph, N root) {
-            this.graph = graph;
-            this.queue.add(root);
-            this.visited.add(root);
-        }
-
-        @Override
-        public boolean hasNext() {
-            return !this.queue.isEmpty();
-        }
-
-        @Override
-        public N next() {
-            N current = this.queue.remove();
-            for (N neighbor : this.graph.successors(current)) {
-                if (this.visited.add(neighbor)) {
-                    this.queue.add(neighbor);
-                }
-            }
-            return current;
-        }
-    }
-
-    private static final class DepthFirstIterator<N> extends AbstractIterator<N> {
-        private final Graph<N> graph;
-
-        private final Deque<NodeAndSuccessors> stack = new ArrayDeque<>();
-        private final Set<N> visited = new HashSet<>();
-        private final Order order;
-
-        DepthFirstIterator(Graph<N> graph, N root, Order order) {
-            this.graph = graph;
-
-            // our invariant is that in computeNext we call next on the iterator at the top first, so we
-            // need to start with one additional item on that iterator
-            this.stack.push(withSuccessors(root));
-            this.order = order;
-        }
-
-        @Override
-        protected N computeNext() {
-            while (true) {
-                if (this.stack.isEmpty()) {
-                    return endOfData();
-                }
-                NodeAndSuccessors node = this.stack.getFirst();
-                boolean firstVisit = this.visited.add(node.node);
-                boolean lastVisit = !node.successorIterator.hasNext();
-                boolean produceNode = (firstVisit && this.order == Order.PRE_ORDER) || (lastVisit && this.order == Order.POST_ORDER);
-                if (lastVisit) {
-                    this.stack.pop();
-                } else {
-                    // we need to push a neighbor, but only if we haven't already seen it
-                    N successor = node.successorIterator.next();
-                    if (!this.visited.contains(successor)) {
-                        this.stack.push(withSuccessors(successor));
-                    }
-                }
-                if (produceNode) {
-                    return node.node;
-                }
-            }
-        }
-
-        NodeAndSuccessors withSuccessors(N node) {
-            return new NodeAndSuccessors(node, this.graph.successors(node));
-        }
-
-        /**
-         * A simple tuple of a node and a partially iterated {@link Iterator} of
-         * its successors
-         */
-        private final class NodeAndSuccessors {
-            final N node;
-            final Iterator<? extends N> successorIterator;
-
-            NodeAndSuccessors(N node, Iterable<? extends N> successors) {
-                this.node = node;
-                this.successorIterator = successors.iterator();
-            }
-        }
-    }
-
-    private enum Order {
-        PRE_ORDER,
-        POST_ORDER
-    }
-
-}

common/src/main/java/me/lucko/luckperms/common/graph/TraversalAlgorithm.java

@@ -23,8 +23,39 @@
  *  SOFTWARE.
  */
 
+/*
+ * Copyright (C) 2017 The Guava Authors
+ *
+ * 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 me.lucko.luckperms.common.graph;
 
+import com.google.common.collect.AbstractIterator;
+
+import java.util.ArrayDeque;
+import java.util.Deque;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.Objects;
+import java.util.Queue;
+import java.util.Set;
+
+/**
+ * A set of traversal algorithm implementations for {@link Graph}s.
+ *
+ * @author Jens Nyman (Guava)
+ */
 public enum TraversalAlgorithm {
 
     /**
@@ -34,7 +65,14 @@ public enum TraversalAlgorithm {
      *
      * <p>See <a href="https://en.wikipedia.org/wiki/Breadth-first_search">Wikipedia</a> for more info.</p>
      */
-    BREADTH_FIRST,
+    BREADTH_FIRST {
+        @Override
+        public <N> Iterable<N> traverse(Graph<N> graph, N startNode) {
+            Objects.requireNonNull(graph, "graph");
+            Objects.requireNonNull(startNode, "startNode");
+            return () -> new BreadthFirstIterator<>(graph, startNode);
+        }
+    },
 
     /**
      * Traverses in depth-first pre-order.
@@ -44,7 +82,14 @@ public enum TraversalAlgorithm {
      *
      * <p>See <a href="https://en.wikipedia.org/wiki/Depth-first_search">Wikipedia</a> for more info.</p>
      */
-    DEPTH_FIRST_PRE_ORDER,
+    DEPTH_FIRST_PRE_ORDER {
+        @Override
+        public <N> Iterable<N> traverse(Graph<N> graph, N startNode) {
+            Objects.requireNonNull(graph, "graph");
+            Objects.requireNonNull(startNode, "startNode");
+            return () -> new DepthFirstIterator<>(graph, startNode, DepthFirstIterator.Order.PRE_ORDER);
+        }
+    },
 
     /**
      * Traverses in depth-first post-order.
@@ -54,6 +99,118 @@ public enum TraversalAlgorithm {
      *
      * <p>See <a href="https://en.wikipedia.org/wiki/Depth-first_search">Wikipedia</a> for more info.</p>
      */
-    DEPTH_FIRST_POST_ORDER
+    DEPTH_FIRST_POST_ORDER {
+        @Override
+        public <N> Iterable<N> traverse(Graph<N> graph, N startNode) {
+            Objects.requireNonNull(graph, "graph");
+            Objects.requireNonNull(startNode, "startNode");
+            return () -> new DepthFirstIterator<>(graph, startNode, DepthFirstIterator.Order.POST_ORDER);
+        }
+    };
+
+    /**
+     * Returns an unmodifiable {@code Iterable} over the nodes reachable from
+     * {@code startNode}, in the order defined by the {@code algorithm}.
+     *
+     * @param graph the graph
+     * @param startNode the start node
+     * @param <N> the node type
+     * @return the traversal
+     */
+    public abstract <N> Iterable<N> traverse(Graph<N> graph, N startNode);
+
+    private static final class BreadthFirstIterator<N> implements Iterator<N> {
+        private final Graph<N> graph;
+
+        private final Queue<N> queue = new ArrayDeque<>();
+        private final Set<N> visited = new HashSet<>();
+
+        BreadthFirstIterator(Graph<N> graph, N root) {
+            this.graph = graph;
+            this.queue.add(root);
+            this.visited.add(root);
+        }
+
+        @Override
+        public boolean hasNext() {
+            return !this.queue.isEmpty();
+        }
+
+        @Override
+        public N next() {
+            N current = this.queue.remove();
+            for (N neighbor : this.graph.successors(current)) {
+                if (this.visited.add(neighbor)) {
+                    this.queue.add(neighbor);
+                }
+            }
+            return current;
+        }
+    }
+
+    private static final class DepthFirstIterator<N> extends AbstractIterator<N> {
+        private final Graph<N> graph;
+
+        private final Deque<NodeAndSuccessors> stack = new ArrayDeque<>();
+        private final Set<N> visited = new HashSet<>();
+        private final Order order;
+
+        DepthFirstIterator(Graph<N> graph, N root, Order order) {
+            this.graph = graph;
+
+            // our invariant is that in computeNext we call next on the iterator at the top first, so we
+            // need to start with one additional item on that iterator
+            this.stack.push(withSuccessors(root));
+            this.order = order;
+        }
+
+        @Override
+        protected N computeNext() {
+            while (true) {
+                if (this.stack.isEmpty()) {
+                    return endOfData();
+                }
+                NodeAndSuccessors node = this.stack.getFirst();
+                boolean firstVisit = this.visited.add(node.node);
+                boolean lastVisit = !node.successorIterator.hasNext();
+                boolean produceNode = (firstVisit && this.order == Order.PRE_ORDER) || (lastVisit && this.order == Order.POST_ORDER);
+                if (lastVisit) {
+                    this.stack.pop();
+                } else {
+                    // we need to push a neighbor, but only if we haven't already seen it
+                    N successor = node.successorIterator.next();
+                    if (!this.visited.contains(successor)) {
+                        this.stack.push(withSuccessors(successor));
+                    }
+                }
+                if (produceNode) {
+                    return node.node;
+                }
+            }
+        }
+
+        NodeAndSuccessors withSuccessors(N node) {
+            return new NodeAndSuccessors(node, this.graph.successors(node));
+        }
+
+        /**
+         * A simple tuple of a node and a partially iterated {@link Iterator} of
+         * its successors
+         */
+        private final class NodeAndSuccessors {
+            final N node;
+            final Iterator<? extends N> successorIterator;
+
+            NodeAndSuccessors(N node, Iterable<? extends N> successors) {
+                this.node = node;
+                this.successorIterator = successors.iterator();
+            }
+        }
+
+        private enum Order {
+            PRE_ORDER,
+            POST_ORDER
+        }
+    }
 
 }

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,12 +36,12 @@ import java.util.Comparator;
 /**
  * Determines the order of group inheritance in {@link PermissionHolder}.
  */
-public class InheritanceComparator implements Comparator<Group> {
+public class InheritanceComparator implements Comparator<ResolvedGroup> {
-    private static final Comparator<Group> NULL_ORIGIN = new InheritanceComparator(null);
+    private static final Comparator<ResolvedGroup> NULL_ORIGIN = new InheritanceComparator(null).reversed();
 
-    public static Comparator<Group> getFor(PermissionHolder origin) {
+    public static Comparator<ResolvedGroup> getFor(PermissionHolder origin) {
         if (origin.getType() == HolderType.USER) {
-            return new InheritanceComparator(((User) origin));
+            return new InheritanceComparator(((User) origin)).reversed();
         }
         return NULL_ORIGIN;
     }
@@ -53,19 +53,17 @@ public class InheritanceComparator implements Comparator<Group> {
     }
 
     @Override
-    public int compare(Group o1, Group o2) {
+    public int compare(ResolvedGroup o1, ResolvedGroup o2) {
-        int result = Integer.compare(o1.getWeight().orElse(0), o2.getWeight().orElse(0));
+        int result = Integer.compare(o1.group().getWeight().orElse(0), o2.group().getWeight().orElse(0));
         if (result != 0) {
-            // note negated value - we want higher weights first!
+            return result;
-            return -result;
         }
 
         // failing differing group weights, check if one of the groups is a primary group
         if (this.origin != null) {
-            // note negative
+            result = Boolean.compare(
-            result = -Boolean.compare(
+                    o1.group().getName().equalsIgnoreCase(this.origin.getPrimaryGroup().getStoredValue().orElse(NodeFactory.DEFAULT_GROUP_NAME)),
-                    o1.getName().equalsIgnoreCase(this.origin.getPrimaryGroup().getStoredValue().orElse(NodeFactory.DEFAULT_GROUP_NAME)),
+                    o2.group().getName().equalsIgnoreCase(this.origin.getPrimaryGroup().getStoredValue().orElse(NodeFactory.DEFAULT_GROUP_NAME))
-                    o2.getName().equalsIgnoreCase(this.origin.getPrimaryGroup().getStoredValue().orElse(NodeFactory.DEFAULT_GROUP_NAME))
             );
 
             if (result != 0) {
@@ -73,7 +71,7 @@ public class InheritanceComparator implements Comparator<Group> {
             }
         }
 
-        // fallback to string based comparison
+        // failing weight checks, fallback to which group applies in more specific context
-        return o1.getName().compareTo(o2.getName());
+        return NodeWithContextComparator.normal().compare(o1.node(), o2.node());
     }
 }

common/src/main/java/me/lucko/luckperms/common/inheritance/InheritanceHandler.java

@@ -32,6 +32,7 @@ import me.lucko.luckperms.common.model.Group;
 import me.lucko.luckperms.common.model.PermissionHolder;
 import me.lucko.luckperms.common.plugin.LuckPermsPlugin;
 
+import java.util.ArrayList;
 import java.util.List;
 import java.util.Set;
 import java.util.TreeSet;
@@ -82,15 +83,15 @@ public class InheritanceHandler {
 
         @Override
         public Iterable<? extends PermissionHolder> successors(PermissionHolder holder) {
-            Set<Group> successors = new TreeSet<>(holder.getInheritanceComparator());
+            Set<ResolvedGroup> successors = new TreeSet<>(holder.getInheritanceComparator());
             List<? extends Node> nodes = holder.getOwnGroupNodes();
             for (Node n : nodes) {
                 Group g = this.plugin.getGroupManager().getIfLoaded(n.getGroupName());
                 if (g != null) {
-                    successors.add(g);
+                    successors.add(new ResolvedGroup(n, g));
                 }
             }
-            return successors;
+            return composeSuccessors(successors);
         }
     }
 
@@ -109,7 +110,7 @@ public class InheritanceHandler {
 
         @Override
         public Iterable<? extends PermissionHolder> successors(PermissionHolder holder) {
-            Set<Group> successors = new TreeSet<>(holder.getInheritanceComparator());
+            Set<ResolvedGroup> successors = new TreeSet<>(holder.getInheritanceComparator());
             List<? extends Node> nodes = holder.getOwnGroupNodes(this.context.getContexts());
             for (Node n : nodes) {
                 // effectively: if not (we're applying global groups or it's specific anyways)
@@ -119,11 +120,19 @@ public class InheritanceHandler {
 
                 Group g = this.plugin.getGroupManager().getIfLoaded(n.getGroupName());
                 if (g != null) {
-                    successors.add(g);
+                    successors.add(new ResolvedGroup(n, g));
                 }
             }
-            return successors;
+            return composeSuccessors(successors);
         }
     }
 
+    private static Iterable<PermissionHolder> composeSuccessors(Set<ResolvedGroup> successors) {
+        List<PermissionHolder> holders = new ArrayList<>(successors.size());
+        for (ResolvedGroup resolvedGroup : successors) {
+            holders.add(resolvedGroup.group());
+        }
+        return holders;
+    }
+
 }

common/src/main/java/me/lucko/luckperms/common/inheritance/ResolvedGroup.java

@@ -0,0 +1,60 @@
+/*
+ * This file is part of LuckPerms, licensed under the MIT License.
+ *
+ *  Copyright (c) lucko (Luck) <luck@lucko.me>
+ *  Copyright (c) contributors
+ *
+ *  Permission is hereby granted, free of charge, to any person obtaining a copy
+ *  of this software and associated documentation files (the "Software"), to deal
+ *  in the Software without restriction, including without limitation the rights
+ *  to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ *  copies of the Software, and to permit persons to whom the Software is
+ *  furnished to do so, subject to the following conditions:
+ *
+ *  The above copyright notice and this permission notice shall be included in all
+ *  copies or substantial portions of the Software.
+ *
+ *  THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ *  IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ *  FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ *  AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ *  LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ *  OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ *  SOFTWARE.
+ */
+
+package me.lucko.luckperms.common.inheritance;
+
+import me.lucko.luckperms.api.Node;
+import me.lucko.luckperms.common.model.Group;
+
+public final class ResolvedGroup {
+    private final Node node;
+    private final Group group;
+
+    ResolvedGroup(Node node, Group group) {
+        this.node = node;
+        this.group = group;
+    }
+
+    Node node() {
+        return this.node;
+    }
+
+    Group group() {
+        return this.group;
+    }
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) return true;
+        if (o == null || getClass() != o.getClass()) return false;
+        ResolvedGroup that = (ResolvedGroup) o;
+        return this.group.equals(that.group);
+    }
+
+    @Override
+    public int hashCode() {
+        return this.group.hashCode();
+    }
+}

common/src/main/java/me/lucko/luckperms/common/model/PermissionHolder.java

@@ -46,6 +46,7 @@ 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;
@@ -129,7 +130,7 @@ public abstract class PermissionHolder {
     /**
      * Comparator used to ordering groups when calculating inheritance
      */
-    private final Comparator<Group> inheritanceComparator = InheritanceComparator.getFor(this);
+    private final Comparator<ResolvedGroup> inheritanceComparator = InheritanceComparator.getFor(this);
 
     /**
      * Creates a new instance
@@ -150,7 +151,7 @@ public abstract class PermissionHolder {
         return this.ioLock;
     }
 
-    public Comparator<Group> getInheritanceComparator() {
+    public Comparator<ResolvedGroup> getInheritanceComparator() {
         return this.inheritanceComparator;
     }