From cd5c8d7cdfd23b267d4f4504c8dad108f1bc2090 Mon Sep 17 00:00:00 2001
From: Luck <git@lucko.me>
Date: Sat, 21 Dec 2019 19:58:37 -0500
Subject: [PATCH] NodeEqualityPredicate refactoring

---
 .../api/node/DummyNodeEqualityPredicate.java  |  29 ++++
 .../java/net/luckperms/api/node/Node.java     |  13 +-
 .../api/node/NodeEqualityPredicate.java       | 127 ++++++++++++------
 .../luckperms/common/node/AbstractNode.java   |  45 ++++---
 4 files changed, 150 insertions(+), 64 deletions(-)
 create mode 100644 api/src/main/java/net/luckperms/api/node/DummyNodeEqualityPredicate.java

diff --git a/api/src/main/java/net/luckperms/api/node/DummyNodeEqualityPredicate.java b/api/src/main/java/net/luckperms/api/node/DummyNodeEqualityPredicate.java
new file mode 100644
index 000000000..baddb7fba
--- /dev/null
+++ b/api/src/main/java/net/luckperms/api/node/DummyNodeEqualityPredicate.java
@@ -0,0 +1,29 @@
+package net.luckperms.api.node;
+
+import org.checkerframework.checker.nullness.qual.NonNull;
+
+/**
+ * Dummy implementation of {@link NodeEqualityPredicate}, used for the given constant
+ * implementations.
+ *
+ * <p>The implementation rule of not calling {@link Node#equals(Node, NodeEqualityPredicate)} is
+ * intentionally disregarded by this dummy implementation. The equals method has a special case for
+ * the dummy instances, preventing a stack overflow.</p>
+ */
+final class DummyNodeEqualityPredicate implements NodeEqualityPredicate {
+    private final String name;
+
+    DummyNodeEqualityPredicate(String name) {
+        this.name = name;
+    }
+
+    @Override
+    public boolean areEqual(@NonNull Node o1, @NonNull Node o2) {
+        return o1.equals(o2, this);
+    }
+
+    @Override
+    public String toString() {
+        return "NodeEqualityPredicate#" + this.name;
+    }
+}
diff --git a/api/src/main/java/net/luckperms/api/node/Node.java b/api/src/main/java/net/luckperms/api/node/Node.java
index 8e505645f..e1fa62958 100644
--- a/api/src/main/java/net/luckperms/api/node/Node.java
+++ b/api/src/main/java/net/luckperms/api/node/Node.java
@@ -63,15 +63,24 @@ import java.util.stream.Stream;
  * node, as well as methods to query and extract additional state and properties
  * from these settings.</p>
  *
- * <p>Nodes have the following attributes:</p>
+ * <p>Nodes have the following 4 key attributes:</p>
  * <p></p>
  * <ul>
  * <li>{@link #getKey() key} - the key of the node</li>
  * <li>{@link #getValue() value} - the value of the node (false for negated)</li>
- * <li>{@link #getContexts() context} - the contexts required for this node to apply </li>
+ * <li>{@link #getContexts() context} - the contexts required for this node to apply</li>
  * <li>{@link #getExpiry() expiry} - the time when this node should expire</li>
  * </ul>
  *
+ * <p>These are the key attributes which are considered when evaluating node
+ * {@link #equals(Object) equality}.</p>
+ *
+ * <p>Nodes can also optionally have {@link #metadata(NodeMetadataKey) metadata} attached to them,
+ * added during construction using {@link NodeBuilder#withMetadata(NodeMetadataKey, Object)}, and
+ * queried using {@link #metadata(NodeMetadataKey)} and {@link #getMetadata(NodeMetadataKey)}.
+ * Such metadata is never considered when evaluating {@link #equals(Object)} or
+ * {@link #equals(Node, NodeEqualityPredicate)} (any form of equality check).</p>
+ *
  * <p>There are a number of node types, all of which extend from this class:</p>
  * <p></p>
  * <ul>
diff --git a/api/src/main/java/net/luckperms/api/node/NodeEqualityPredicate.java b/api/src/main/java/net/luckperms/api/node/NodeEqualityPredicate.java
index 2a1a7297a..f0579d7f2 100644
--- a/api/src/main/java/net/luckperms/api/node/NodeEqualityPredicate.java
+++ b/api/src/main/java/net/luckperms/api/node/NodeEqualityPredicate.java
@@ -30,7 +30,18 @@ import org.checkerframework.checker.nullness.qual.NonNull;
 import java.util.function.Predicate;
 
 /**
- * A rule for determining if two nodes are equal.
+ * An equality test for determining if two nodes are to be considered equal.
+ *
+ * <p>Recall that {@link Node}s have 4 key attributes: key, value, context, expiry.</p>
+ *
+ * <p>In the default {@link Node#equals(Object)} implementation (equivalent to {@link #EXACT}),
+ * all 4 of these key attributes are considered. However, there are occasions where such strict
+ * equality checking is not desired, hence the use of this class.</p>
+ *
+ * <p>{@link NodeEqualityPredicate}s can either be used inline, by directly calling the
+ * {@link #areEqual(Node, Node)} method, or can be passed as a parameter to the
+ * {@link Node#equals(Node, NodeEqualityPredicate)} method. Either approach is valid, and both will
+ * result in the same result.</p>
  *
  * <p>Generally, implementations of this interface should fulfil the same
  * requirements as the {@link Object#equals(Object)} contract.</p>
@@ -61,60 +72,88 @@ public interface NodeEqualityPredicate {
         return other -> areEqual(node, other);
     }
 
-
-    /*
-     * Some 'default' implementations of NodeEqualityPredicate are provided below.
-     *
-     * These are implemented in the common code, by a special case in the
-     * implementation of Node#equals. As noted above, this should generally be
-     * avoided.
-     */
-
     /**
      * Represents an exact match.
      *
-     * <p>All attributes of the nodes must match for them to be considered
-     * equal.</p>
-     */
-    NodeEqualityPredicate EXACT = new NodeEqualityPredicate() {
-        @Override public boolean areEqual(@NonNull Node o1, @NonNull Node o2) { return o1.equals(o2, this); }
-    };
-
-    /**
-     * All attributes must match, except for
-     * {@link Node#getValue() value}, which is ignored.
-     */
-    NodeEqualityPredicate IGNORE_VALUE = new NodeEqualityPredicate() {
-        @Override public boolean areEqual(@NonNull Node o1, @NonNull Node o2) { return o1.equals(o2, this); }
-    };
-
-    /**
-     * All attributes must match, except for the
-     * {@link Node#getExpiry() expiry time}, which is ignored.
+     * <p>Returns true if: (and)</p>
+     * <p></p>
+     * <ul>
+     * <li>{@link Node#getKey() key} = key</li>
+     * <li>{@link Node#getValue() value} = value</li>
+     * <li>{@link Node#getContexts() context} = context</li>
+     * <li>{@link Node#getExpiry() expiry} = expiry</li>
+     * </ul>
      *
-     * <p>Note that with this setting, whether a node is temporary or not is
-     * still considered.</p>
+     * <p>All 4 attributes of the nodes must match to be considered equal.</p>
+     *
+     * <p>This is the default form of equality, used by {@link Node#equals(Object)}.</p>
      */
-    NodeEqualityPredicate IGNORE_EXPIRY_TIME = new NodeEqualityPredicate() {
-        @Override public boolean areEqual(@NonNull Node o1, @NonNull Node o2) { return o1.equals(o2, this); }
-    };
+    NodeEqualityPredicate EXACT = new DummyNodeEqualityPredicate("EXACT");
 
     /**
-     * All attributes must match, except for
-     * {@link Node#getValue() value} and the
+     * Only the {@link Node#getKey() key}s need match, all other attributes are ignored.
+     */
+    NodeEqualityPredicate ONLY_KEY = new DummyNodeEqualityPredicate("ONLY_KEY");
+
+    /**
+     * All attributes must match, except for {@link Node#getValue() value}, which is ignored.
+     *
+     * <p>Returns true if: (and)</p>
+     * <p></p>
+     * <ul>
+     * <li>{@link Node#getKey() key} = key</li>
+     * <li>{@link Node#getContexts() context} = context</li>
+     * <li>{@link Node#getExpiry() expiry} = expiry</li>
+     * </ul>
+     */
+    NodeEqualityPredicate IGNORE_VALUE = new DummyNodeEqualityPredicate("IGNORE_VALUE");
+
+    /**
+     * All attributes must match, except for the {@link Node#getExpiry() expiry time}, which is
+     * ignored.
+     *
+     * <p>Note that with this setting, whether a node has an expiry or not is still considered.</p>
+     *
+     * <p>Returns true if: (and)</p>
+     * <p></p>
+     * <ul>
+     * <li>{@link Node#getKey() key} = key</li>
+     * <li>{@link Node#getValue() value} = value</li>
+     * <li>{@link Node#getContexts() context} = context</li>
+     * <li>{@link Node#hasExpiry() has expiry} = has expiry</li>
+     * </ul>
+     */
+    NodeEqualityPredicate IGNORE_EXPIRY_TIME = new DummyNodeEqualityPredicate("IGNORE_EXPIRY_TIME");
+
+    /**
+     * All attributes must match, except for {@link Node#getValue() value} and the
      * {@link Node#getExpiry() expiry time}, which are ignored.
+     *
+     * <p>Note that with this setting, whether a node has an expiry or not is still considered.</p>
+     *
+     * <p>Returns true if: (and)</p>
+     * <p></p>
+     * <ul>
+     * <li>{@link Node#getKey() key} = key</li>
+     * <li>{@link Node#getContexts() context} = context</li>
+     * <li>{@link Node#hasExpiry() has expiry} = has expiry</li>
+     * </ul>
      */
-    NodeEqualityPredicate IGNORE_EXPIRY_TIME_AND_VALUE = new NodeEqualityPredicate() {
-        @Override public boolean areEqual(@NonNull Node o1, @NonNull Node o2) { return o1.equals(o2, this); }
-    };
+    NodeEqualityPredicate IGNORE_EXPIRY_TIME_AND_VALUE = new DummyNodeEqualityPredicate("IGNORE_EXPIRY_TIME_AND_VALUE");
 
     /**
-     * All attributes must match, except for
-     * {@link Node#getValue() value} and the if the node is
-     * {@link Node#hasExpiry() temporary}, which are ignored.
+     * All attributes must match, except for {@link Node#getValue() value} and the if the node
+     * {@link Node#hasExpiry() has an expiry}, which are ignored.
+     *
+     * <p>Effectively only considers the key and the context.</p>
+     *
+     * <p>Returns true if: (and)</p>
+     * <p></p>
+     * <ul>
+     * <li>{@link Node#getKey() key} = key</li>
+     * <li>{@link Node#getContexts() context} = context</li>
+     * </ul>
      */
-    NodeEqualityPredicate IGNORE_VALUE_OR_IF_TEMPORARY = new NodeEqualityPredicate() {
-        @Override public boolean areEqual(@NonNull Node o1, @NonNull Node o2) { return o1.equals(o2, this); }
-    };
+    NodeEqualityPredicate IGNORE_VALUE_OR_IF_TEMPORARY = new DummyNodeEqualityPredicate("IGNORE_VALUE_OR_IF_TEMPORARY");
 
 }
diff --git a/common/src/main/java/me/lucko/luckperms/common/node/AbstractNode.java b/common/src/main/java/me/lucko/luckperms/common/node/AbstractNode.java
index 218ddf78e..730e46865 100644
--- a/common/src/main/java/me/lucko/luckperms/common/node/AbstractNode.java
+++ b/common/src/main/java/me/lucko/luckperms/common/node/AbstractNode.java
@@ -125,22 +125,24 @@ public abstract class AbstractNode<N extends ScopedNode<N, B>, B extends NodeBui
     public boolean equals(Object o) {
         if (o == this) return true;
         if (!(o instanceof Node)) return false;
-        return Equality.EXACT.areEqual(this, ((AbstractNode) o));
+        return Equality.KEY_VALUE_EXPIRY_CONTEXTS.equals(this, ((AbstractNode) o));
     }
 
     @Override
     public boolean equals(@NonNull Node o, @NonNull NodeEqualityPredicate equalityPredicate) {
-        AbstractNode other = (AbstractNode) o;
+        AbstractNode<?, ?> other = (AbstractNode<?, ?>) o;
         if (equalityPredicate == NodeEqualityPredicate.EXACT) {
-            return Equality.EXACT.areEqual(this, other);
+            return Equality.KEY_VALUE_EXPIRY_CONTEXTS.equals(this, other);
         } else if (equalityPredicate == NodeEqualityPredicate.IGNORE_VALUE) {
-            return Equality.IGNORE_VALUE.areEqual(this, other);
+            return Equality.KEY_EXPIRY_CONTEXTS.equals(this, other);
         } else if (equalityPredicate == NodeEqualityPredicate.IGNORE_EXPIRY_TIME) {
-            return Equality.IGNORE_EXPIRY_TIME.areEqual(this, other);
+            return Equality.KEY_VALUE_HASEXPIRY_CONTEXTS.equals(this, other);
         } else if (equalityPredicate == NodeEqualityPredicate.IGNORE_EXPIRY_TIME_AND_VALUE) {
-            return Equality.IGNORE_EXPIRY_TIME_AND_VALUE.areEqual(this, other);
+            return Equality.KEY_HASEXPIRY_CONTEXTS.equals(this, other);
         } else if (equalityPredicate == NodeEqualityPredicate.IGNORE_VALUE_OR_IF_TEMPORARY) {
-            return Equality.IGNORE_VALUE_OR_IF_TEMPORARY.areEqual(this, other);
+            return Equality.KEY_CONTEXTS.equals(this, other);
+        } else if (equalityPredicate == NodeEqualityPredicate.ONLY_KEY) {
+            return Equality.KEY.equals(this, other);
         } else {
             return equalityPredicate.areEqual(this, o);
         }
@@ -162,9 +164,9 @@ public abstract class AbstractNode<N extends ScopedNode<N, B>, B extends NodeBui
     }
 
     private enum Equality {
-        EXACT {
+        KEY_VALUE_EXPIRY_CONTEXTS {
             @Override
-            public boolean areEqual(@NonNull AbstractNode o1, @NonNull AbstractNode o2) {
+            public boolean equals(AbstractNode<?, ?> o1, AbstractNode<?, ?> o2) {
                 return o1 == o2 ||
                         o1.key.equals(o2.key) &&
                         o1.value == o2.value &&
@@ -172,18 +174,18 @@ public abstract class AbstractNode<N extends ScopedNode<N, B>, B extends NodeBui
                         o1.getContexts().equals(o2.getContexts());
             }
         },
-        IGNORE_VALUE {
+        KEY_EXPIRY_CONTEXTS {
             @Override
-            public boolean areEqual(@NonNull AbstractNode o1, @NonNull AbstractNode o2) {
+            public boolean equals(AbstractNode<?, ?> o1, AbstractNode<?, ?> o2) {
                 return o1 == o2 ||
                         o1.key.equals(o2.key) &&
                         o1.expireAt == o2.expireAt &&
                         o1.getContexts().equals(o2.getContexts());
             }
         },
-        IGNORE_EXPIRY_TIME {
+        KEY_VALUE_HASEXPIRY_CONTEXTS {
             @Override
-            public boolean areEqual(@NonNull AbstractNode o1, @NonNull AbstractNode o2) {
+            public boolean equals(AbstractNode<?, ?> o1, AbstractNode<?, ?> o2) {
                 return o1 == o2 ||
                         o1.key.equals(o2.key) &&
                         o1.value == o2.value &&
@@ -191,25 +193,32 @@ public abstract class AbstractNode<N extends ScopedNode<N, B>, B extends NodeBui
                         o1.getContexts().equals(o2.getContexts());
             }
         },
-        IGNORE_EXPIRY_TIME_AND_VALUE {
+        KEY_HASEXPIRY_CONTEXTS {
             @Override
-            public boolean areEqual(@NonNull AbstractNode o1, @NonNull AbstractNode o2) {
+            public boolean equals(AbstractNode<?, ?> o1, AbstractNode<?, ?> o2) {
                 return o1 == o2 ||
                         o1.key.equals(o2.key) &&
                         o1.hasExpiry() == o2.hasExpiry() &&
                         o1.getContexts().equals(o2.getContexts());
             }
         },
-        IGNORE_VALUE_OR_IF_TEMPORARY {
+        KEY_CONTEXTS {
             @Override
-            public boolean areEqual(@NonNull AbstractNode o1, @NonNull AbstractNode o2) {
+            public boolean equals(AbstractNode<?, ?> o1, AbstractNode<?, ?> o2) {
                 return o1 == o2 ||
                         o1.key.equals(o2.key) &&
                         o1.getContexts().equals(o2.getContexts());
             }
+        },
+        KEY {
+            @Override
+            public boolean equals(AbstractNode<?, ?> o1, AbstractNode<?, ?> o2) {
+                return o1 == o2 ||
+                        o1.key.equals(o2.key);
+            }
         };
 
-        public abstract boolean areEqual(@NonNull AbstractNode o1, @NonNull AbstractNode o2);
+        public abstract boolean equals(AbstractNode<?, ?> o1, AbstractNode<?, ?> o2);
     }
 
     @Override