Tidy up Node add/remove/clear API events

api/src/main/java/net/luckperms/api/event/node/NodeAddEvent.java

@@ -30,6 +30,10 @@ import net.luckperms.api.node.Node;
 
 import org.checkerframework.checker.nullness.qual.NonNull;
 
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
+
 /**
  * Called when a node is added to a holder
  */
@@ -40,7 +44,14 @@ public interface NodeAddEvent extends NodeMutateEvent {
      *
      * @return the node that was added
      */
-    @Param(4)
+    @Param(3)
     @NonNull Node getNode();
 
+    @Override
+    default @NonNull Set<Node> getDataBefore() {
+        // Get data after, then reverse the action
+        Set<Node> nodes = new HashSet<>(this.getDataAfter());
+        nodes.remove(this.getNode());
+        return Collections.unmodifiableSet(nodes);
+    }
 }

api/src/main/java/net/luckperms/api/event/node/NodeClearEvent.java

@@ -25,9 +25,34 @@
 
 package net.luckperms.api.event.node;
 
+import net.luckperms.api.event.util.Param;
+import net.luckperms.api.node.Node;
+
+import org.checkerframework.checker.nullness.qual.NonNull;
+
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
+
 /**
  * Called when a holder has their nodes cleared
  */
 public interface NodeClearEvent extends NodeMutateEvent {
 
+    /**
+     * Gets the nodes that were cleared
+     *
+     * @return the nodes that were removed
+     * @since 5.3
+     */
+    @Param(3)
+    @NonNull Set<Node> getNodes();
+
+    @Override
+    default @NonNull Set<Node> getDataBefore() {
+        // Get data after, then reverse the action
+        Set<Node> nodes = new HashSet<>(this.getDataAfter());
+        nodes.addAll(this.getNodes());
+        return Collections.unmodifiableSet(nodes);
+    }
 }

api/src/main/java/net/luckperms/api/event/node/NodeMutateEvent.java

@@ -63,7 +63,6 @@ public interface NodeMutateEvent extends LuckPermsEvent {
      *
      * @return the data before the change
      */
-    @Param(2)
     @NonNull Set<Node> getDataBefore();
 
     /**
@@ -71,7 +70,7 @@ public interface NodeMutateEvent extends LuckPermsEvent {
      *
      * @return the data after the change
      */
-    @Param(3)
+    @Param(2)
     @NonNull Set<Node> getDataAfter();
 
     /**

api/src/main/java/net/luckperms/api/event/node/NodeRemoveEvent.java

@@ -30,6 +30,10 @@ import net.luckperms.api.node.Node;
 
 import org.checkerframework.checker.nullness.qual.NonNull;
 
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
+
 /**
  * Called when a node is removed from a holder
  */
@@ -40,7 +44,15 @@ public interface NodeRemoveEvent extends NodeMutateEvent {
      *
      * @return the node that was removed
      */
-    @Param(4)
+    @Param(3)
     @NonNull Node getNode();
 
+    @Override
+    default @NonNull Set<Node> getDataBefore() {
+        // Get data after, then reverse the action
+        Set<Node> nodes = new HashSet<>(this.getDataAfter());
+        nodes.add(this.getNode());
+        return Collections.unmodifiableSet(nodes);
+    }
+
 }

common/src/main/java/me/lucko/luckperms/common/event/EventDispatcher.java

@@ -40,6 +40,7 @@ import me.lucko.luckperms.common.model.HolderType;
 import me.lucko.luckperms.common.model.PermissionHolder;
 import me.lucko.luckperms.common.model.Track;
 import me.lucko.luckperms.common.model.User;
+import me.lucko.luckperms.common.model.nodemap.MutateResult;
 import me.lucko.luckperms.common.sender.Sender;
 
 import net.luckperms.api.actionlog.Action;
@@ -61,6 +62,7 @@ import net.luckperms.api.event.log.LogPublishEvent;
 import net.luckperms.api.event.log.LogReceiveEvent;
 import net.luckperms.api.event.node.NodeAddEvent;
 import net.luckperms.api.event.node.NodeClearEvent;
+import net.luckperms.api.event.node.NodeMutateEvent;
 import net.luckperms.api.event.node.NodeRemoveEvent;
 import net.luckperms.api.event.player.PlayerDataSaveEvent;
 import net.luckperms.api.event.player.PlayerLoginProcessEvent;
@@ -96,8 +98,10 @@ import net.luckperms.api.node.Node;
 import org.checkerframework.checker.nullness.qual.Nullable;
 
 import java.util.Collection;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Optional;
+import java.util.Set;
 import java.util.UUID;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicReference;
@@ -223,16 +227,47 @@ public final class EventDispatcher {
         postAsync(LogReceiveEvent.class, id, entry);
     }
 
-    public void dispatchNodeAdd(Node node, PermissionHolder target, DataType dataType, Collection<? extends Node> before, Collection<? extends Node> after) {
+    public void dispatchNodeChanges(PermissionHolder target, DataType dataType, MutateResult changes) {
-        postAsync(NodeAddEvent.class, proxy(target), dataType, ImmutableSet.copyOf(before), ImmutableSet.copyOf(after), node);
+        if (!this.eventBus.shouldPost(NodeAddEvent.class) && !this.eventBus.shouldPost(NodeRemoveEvent.class)) {
+            return;
+        }
+
+        if (changes.isEmpty()) {
+            return;
+        }
+
+        ApiPermissionHolder proxy = proxy(target);
+        ImmutableSet<Node> state = target.getData(dataType).asImmutableSet();
+
+        // call an event for each recorded change
+        for (MutateResult.Change change : changes.getChanges()) {
+            Class<? extends NodeMutateEvent> type = change.getType() == MutateResult.ChangeType.ADD ?
+                    NodeAddEvent.class : NodeRemoveEvent.class;
+
+            postAsync(type, proxy, dataType, state, change.getNode());
+        }
     }
 
-    public void dispatchNodeClear(PermissionHolder target, DataType dataType, Collection<? extends Node> before, Collection<? extends Node> after) {
+    public void dispatchNodeClear(PermissionHolder target, DataType dataType, MutateResult changes) {
-        postAsync(NodeClearEvent.class, proxy(target), dataType, ImmutableSet.copyOf(before), ImmutableSet.copyOf(after));
+        if (!this.eventBus.shouldPost(NodeClearEvent.class)) {
-    }
+            return;
+        }
 
-    public void dispatchNodeRemove(Node node, PermissionHolder target, DataType dataType, Collection<? extends Node> before, Collection<? extends Node> after) {
+        if (changes.isEmpty()) {
-        postAsync(NodeRemoveEvent.class, proxy(target), dataType, ImmutableSet.copyOf(before), ImmutableSet.copyOf(after), node);
+            return;
+        }
+
+        ApiPermissionHolder proxy = proxy(target);
+        ImmutableSet<Node> state = target.getData(dataType).asImmutableSet();
+
+        // call clear event
+        ImmutableSet<Node> nodes = ImmutableSet.copyOf(changes.getRemoved());
+        postAsync(NodeClearEvent.class, proxy, dataType, state, nodes);
+
+        // call add event if needed for any nodes that were added
+        for (Node added : changes.getAdded()) {
+            postAsync(NodeAddEvent.class, proxy, dataType, state, added);
+        }
     }
 
     public void dispatchConfigReload() {

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

@@ -426,17 +426,9 @@ public abstract class PermissionHolder {
     }
 
     private boolean auditTemporaryNodes(DataType dataType) {
-        ImmutableSet<Node> before = getData(dataType).asImmutableSet();
-
         MutateResult result = getData(dataType).removeIf(Node::hasExpired);
+        this.plugin.getEventDispatcher().dispatchNodeChanges(this, dataType, result);
         if (!result.isEmpty()) {
-            // call event
-            ImmutableSet<Node> after = getData(dataType).asImmutableSet();
-            for (Node r : result.getRemoved()) {
-                this.plugin.getEventDispatcher().dispatchNodeRemove(r, this, dataType, before, after);
-            }
-
-            // invalidate
             invalidateCache();
         }
         return !result.isEmpty();
@@ -468,14 +460,9 @@ public abstract class PermissionHolder {
             return DataMutateResult.FAIL_ALREADY_HAS;
         }
 
-        NodeMap data = getData(dataType);
+        MutateResult changes = getData(dataType).add(node);
-
-        ImmutableSet<Node> before = data.asImmutableSet();
-        data.add(node);
-        ImmutableSet<Node> after = data.asImmutableSet();
-
         if (callEvent) {
-            this.plugin.getEventDispatcher().dispatchNodeAdd(node, this, dataType, before, after);
+            this.plugin.getEventDispatcher().dispatchNodeChanges(this, dataType, changes);
         }
 
         invalidateCache();
@@ -511,11 +498,8 @@ public abstract class PermissionHolder {
 
                 if (newNode != null) {
                     // Remove the old Node & add the new one.
-                    ImmutableSet<Node> before = data.asImmutableSet();
+                    MutateResult changes = data.removeThenAdd(otherMatch, newNode);
-                    data.removeThenAdd(otherMatch, newNode);
+                    this.plugin.getEventDispatcher().dispatchNodeChanges(this, dataType, changes);
-                    ImmutableSet<Node> after = data.asImmutableSet();
-
-                    this.plugin.getEventDispatcher().dispatchNodeAdd(newNode, this, dataType, before, after);
 
                     invalidateCache();
 
@@ -533,13 +517,8 @@ public abstract class PermissionHolder {
             return DataMutateResult.FAIL_LACKS;
         }
 
-        NodeMap data = getData(dataType);
+        MutateResult changes = getData(dataType).remove(node);
-
+        this.plugin.getEventDispatcher().dispatchNodeChanges(this, dataType, changes);
-        ImmutableSet<Node> before = data.asImmutableSet();
-        data.remove(node);
-        ImmutableSet<Node> after = data.asImmutableSet();
-
-        this.plugin.getEventDispatcher().dispatchNodeRemove(node, this, dataType, before, after);
 
         invalidateCache();
 
@@ -561,12 +540,8 @@ public abstract class PermissionHolder {
                     Node newNode = node.toBuilder().expiry(newExpiry).build();
 
                     // Remove the old Node & add the new one.
-                    ImmutableSet<Node> before = data.asImmutableSet();
+                    MutateResult changes = data.removeThenAdd(otherMatch, newNode);
-                    data.removeThenAdd(otherMatch, newNode);
+                    this.plugin.getEventDispatcher().dispatchNodeChanges(this, dataType, changes);
-                    ImmutableSet<Node> after = data.asImmutableSet();
-
-                    this.plugin.getEventDispatcher().dispatchNodeRemove(otherMatch, this, dataType, before, after);
-                    this.plugin.getEventDispatcher().dispatchNodeAdd(newNode, this, dataType, before, after);
 
                     invalidateCache();
 
@@ -580,50 +555,40 @@ public abstract class PermissionHolder {
     }
 
     public boolean removeIf(DataType dataType, @Nullable ContextSet contextSet, Predicate<? super Node> predicate, boolean giveDefault) {
-        NodeMap data = getData(dataType);
+        MutateResult changes;
-        ImmutableSet<Node> before = data.asImmutableSet();
-
         if (contextSet == null) {
-            if (data.removeIf(predicate).isEmpty()) {
+            changes = getData(dataType).removeIf(predicate);
-                return false;
-            }
         } else {
-            if (data.removeIf(contextSet, predicate).isEmpty()) {
+            changes = getData(dataType).removeIf(contextSet, predicate);
-                return false;
+        }
-            }
+
+        if (changes.isEmpty()) {
+            return false;
         }
 
         if (getType() == HolderType.USER && giveDefault) {
             getPlugin().getUserManager().giveDefaultIfNeeded((User) this);
         }
 
-        ImmutableSet<Node> after = data.asImmutableSet();
+        this.plugin.getEventDispatcher().dispatchNodeClear(this, dataType, changes);
-        this.plugin.getEventDispatcher().dispatchNodeClear(this, dataType, before, after);
-
         invalidateCache();
-
         return true;
     }
 
     public boolean clearNodes(DataType dataType, ContextSet contextSet, boolean giveDefault) {
-        NodeMap data = getData(dataType);
+        MutateResult changes;
-        ImmutableSet<Node> before = data.asImmutableSet();
-
         if (contextSet == null) {
-            data.clear();
+            changes = getData(dataType).clear();
         } else {
-            data.clear(contextSet);
+            changes = getData(dataType).clear(contextSet);
         }
 
         if (getType() == HolderType.USER && giveDefault) {
             getPlugin().getUserManager().giveDefaultIfNeeded((User) this);
         }
 
-        ImmutableSet<Node> after = data.asImmutableSet();
+        this.plugin.getEventDispatcher().dispatchNodeClear(this, dataType, changes);
-        this.plugin.getEventDispatcher().dispatchNodeClear(this, dataType, before, after);
-
         invalidateCache();
-
         return true;
     }