Cleanup applying bulkupdates to Node objects directly

common/src/main/java/me/lucko/luckperms/common/bulkupdate/BulkUpdate.java

@@ -27,11 +27,16 @@ package me.lucko.luckperms.common.bulkupdate;
 
 import me.lucko.luckperms.common.bulkupdate.action.Action;
 import me.lucko.luckperms.common.bulkupdate.query.Query;
+import me.lucko.luckperms.common.model.HolderType;
 
 import net.luckperms.api.node.Node;
 
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+import java.util.HashSet;
 import java.util.List;
 import java.util.Objects;
+import java.util.Set;
 
 /**
  * Represents a query to be applied to a set of data.
@@ -75,17 +80,55 @@ public final class BulkUpdate {
     }
 
     /**
-     * Applies this query to the given NodeModel, and returns the result.
+     * Applies this query to the given node, and returns the result.
      *
-     * @param from the node to base changes from
+     * @param node the node to apply changes to
-     * @return the new nodemodel instance, or null if the node should be deleted.
+     * @return the transformed node, or null if the node should be deleted
      */
-    public Node apply(Node from) {
+    private Node apply(Node node) {
-        if (!satisfiesConstraints(from)) {
+        if (!satisfiesConstraints(node)) {
-            return from; // make no change
+            return node; // make no change
         }
 
-        return this.action.apply(from);
+        Node result = this.action.apply(node);
+
+        if (this.trackStatistics && result != node) {
+            this.statistics.incrementAffectedNodes();
+        }
+
+        return result;
+    }
+
+    /**
+     * Applies this query to the given set of nodes, and returns the result.
+     *
+     * @param nodes the input nodes
+     * @param holderType the holder type the nodes are from
+     * @return the transformed nodes, or null if no change was made
+     */
+    public @Nullable Set<Node> apply(Set<Node> nodes, HolderType holderType) {
+        Set<Node> results = new HashSet<>();
+        boolean change = false;
+
+        for (Node node : nodes) {
+            Node result = apply(node);
+            if (result != node) {
+                change = true;
+            }
+            if (result != null) {
+                results.add(result);
+            }
+        }
+
+        if (!change) {
+            return null;
+        }
+
+        if (this.trackStatistics) {
+            this.statistics.incrementAffected(holderType);
+        }
+
+        return results;
     }
 
     /**
@@ -148,11 +191,11 @@ public final class BulkUpdate {
     }
 
     public boolean isTrackingStatistics() {
-        return trackStatistics;
+        return this.trackStatistics;
     }
 
     public BulkUpdateStatistics getStatistics() {
-        return statistics;
+        return this.statistics;
     }
 
     @Override

common/src/main/java/me/lucko/luckperms/common/bulkupdate/BulkUpdateStatistics.java

@@ -25,6 +25,8 @@
 
 package me.lucko.luckperms.common.bulkupdate;
 
+import me.lucko.luckperms.common.model.HolderType;
+
 /**
  * Keeps track of the number of nodes, users and groups that were affected in a BulkUpdate operation.
  */
@@ -56,26 +58,43 @@ public final class BulkUpdateStatistics {
     }
 
     public void incrementAffectedNodes() {
-        this.affectedNodes++;
+        incrementAffectedNodes(1);
     }
 
     public void incrementAffectedUsers() {
-        this.affectedUsers++;
+        incrementAffectedUsers(1);
     }
 
     public void incrementAffectedGroups() {
-        this.affectedGroups++;
+        incrementAffectedGroups(1);
     }
 
-    public void incrementAffectedNodesBy(int delta) {
+    public void incrementAffected(HolderType type) {
+        incrementAffected(type, 1);
+    }
+
+    public void incrementAffectedNodes(int delta) {
         this.affectedNodes += delta;
     }
 
-    public void incrementAffectedUsersBy(int delta) {
+    public void incrementAffectedUsers(int delta) {
         this.affectedUsers += delta;
     }
 
-    public void incrementAffectedGroupsBy(int delta) {
+    public void incrementAffectedGroups(int delta) {
         this.affectedGroups += delta;
     }
+
+    public void incrementAffected(HolderType type, int delta) {
+        switch (type) {
+            case USER:
+                incrementAffectedUsers(delta);
+                break;
+            case GROUP:
+                incrementAffectedGroups(delta);
+                break;
+            default:
+                throw new AssertionError();
+        }
+    }
 }

common/src/main/java/me/lucko/luckperms/common/storage/implementation/file/AbstractConfigurateStorage.java

@@ -31,7 +31,6 @@ import com.google.common.collect.Maps;
 
 import me.lucko.luckperms.common.actionlog.Log;
 import me.lucko.luckperms.common.bulkupdate.BulkUpdate;
-import me.lucko.luckperms.common.bulkupdate.BulkUpdateStatistics;
 import me.lucko.luckperms.common.context.ContextSetConfigurateSerializer;
 import me.lucko.luckperms.common.context.contextset.ImmutableContextSetImpl;
 import me.lucko.luckperms.common.model.Group;
@@ -77,12 +76,10 @@ import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
-import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
 import java.util.UUID;
 import java.util.function.Function;
-import java.util.stream.Collectors;
 
 /**
  * Abstract implementation using configurate {@link ConfigurationNode}s to serialize and deserialize
@@ -181,35 +178,16 @@ public abstract class AbstractConfigurateStorage implements StorageImplementatio
         return this.actionLogger.getLog();
     }
 
-    protected ConfigurationNode processBulkUpdate(BulkUpdate bulkUpdate, ConfigurationNode node, HolderType holderType) {
+    protected boolean processBulkUpdate(BulkUpdate bulkUpdate, ConfigurationNode node, HolderType holderType) {
-        BulkUpdateStatistics stats = bulkUpdate.getStatistics();
-
         Set<Node> nodes = readNodes(node);
-        Set<Node> results = nodes.stream()
+        Set<Node> results = bulkUpdate.apply(nodes, holderType);
-                .map(bulkUpdate::apply)
-                .filter(Objects::nonNull)
-                .collect(Collectors.toSet());
 
-        if (bulkUpdate.isTrackingStatistics() && !results.isEmpty()) {
+        if (results == null) {
-            stats.incrementAffectedNodesBy(results.size());
+            return false;
-
-            switch (holderType) {
-                case USER:
-                    stats.incrementAffectedUsers();
-                    break;
-
-                case GROUP:
-                    stats.incrementAffectedGroups();
-                    break;
-            }
-        }
-
-        if (nodes.equals(results)) {
-            return null;
         }
 
         writeNodes(node, results);
-        return node;
+        return true;
     }
 
     @Override

common/src/main/java/me/lucko/luckperms/common/storage/implementation/file/SeparatedConfigurateStorage.java

@@ -213,8 +213,7 @@ public class SeparatedConfigurateStorage extends AbstractConfigurateStorage {
                     try {
                         registerFileAction(StorageLocation.USER, file);
                         ConfigurationNode object = readFile(file);
-                        ConfigurationNode results = processBulkUpdate(bulkUpdate, object, HolderType.USER);
+                        if (processBulkUpdate(bulkUpdate, object, HolderType.USER)) {
-                        if (results != null) {
                             saveFile(file, object);
                         }
                     } catch (Exception e) {
@@ -230,8 +229,7 @@ public class SeparatedConfigurateStorage extends AbstractConfigurateStorage {
                     try {
                         registerFileAction(StorageLocation.GROUP, file);
                         ConfigurationNode object = readFile(file);
-                        ConfigurationNode results = processBulkUpdate(bulkUpdate, object, HolderType.GROUP);
+                        if (processBulkUpdate(bulkUpdate, object, HolderType.GROUP)) {
-                        if (results != null) {
                             saveFile(file, object);
                         }
                     } catch (Exception e) {

common/src/main/java/me/lucko/luckperms/common/storage/implementation/mongodb/MongoStorage.java

@@ -40,10 +40,10 @@ import com.mongodb.client.model.ReplaceOptions;
 import me.lucko.luckperms.common.actionlog.Log;
 import me.lucko.luckperms.common.actionlog.LoggedAction;
 import me.lucko.luckperms.common.bulkupdate.BulkUpdate;
-import me.lucko.luckperms.common.bulkupdate.BulkUpdateStatistics;
 import me.lucko.luckperms.common.context.contextset.MutableContextSetImpl;
 import me.lucko.luckperms.common.locale.Message;
 import me.lucko.luckperms.common.model.Group;
+import me.lucko.luckperms.common.model.HolderType;
 import me.lucko.luckperms.common.model.Track;
 import me.lucko.luckperms.common.model.User;
 import me.lucko.luckperms.common.model.manager.group.GroupManager;
@@ -76,7 +76,6 @@ import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
 import java.util.UUID;
@@ -251,33 +250,15 @@ public class MongoStorage implements StorageImplementation {
 
     @Override
     public void applyBulkUpdate(BulkUpdate bulkUpdate) {
-        BulkUpdateStatistics stats = bulkUpdate.getStatistics();
-
         if (bulkUpdate.getDataType().isIncludingUsers()) {
             MongoCollection<Document> c = this.database.getCollection(this.prefix + "users");
             try (MongoCursor<Document> cursor = c.find().iterator()) {
                 while (cursor.hasNext()) {
                     Document d = cursor.next();
-
                     UUID uuid = getDocumentId(d);
-                    Set<Node> nodes = new HashSet<>(nodesFromDoc(d));
+                    Document results = processBulkUpdate(d, bulkUpdate, HolderType.USER);
-                    Set<Node> results = nodes.stream()
+                    if (results != null) {
-                            .map(bulkUpdate::apply)
+                        c.replaceOne(new Document("_id", uuid), results);
-                            .filter(Objects::nonNull)
-                            .collect(Collectors.toSet());
-
-                    if (bulkUpdate.isTrackingStatistics() && !results.isEmpty()) {
-                        stats.incrementAffectedUsers();
-                        stats.incrementAffectedNodesBy(results.size());
-                    }
-
-                    if (!nodes.equals(results)) {
-                        List<Document> newNodes = results.stream()
-                                .map(MongoStorage::nodeToDoc)
-                                .collect(Collectors.toList());
-
-                        d.append("permissions", newNodes).remove("perms");
-                        c.replaceOne(new Document("_id", uuid), d);
                     }
                 }
             }
@@ -288,32 +269,32 @@ public class MongoStorage implements StorageImplementation {
             try (MongoCursor<Document> cursor = c.find().iterator()) {
                 while (cursor.hasNext()) {
                     Document d = cursor.next();
-
                     String holder = d.getString("_id");
-                    Set<Node> nodes = new HashSet<>(nodesFromDoc(d));
+                    Document results = processBulkUpdate(d, bulkUpdate, HolderType.GROUP);
-                    Set<Node> results = nodes.stream()
+                    if (results != null) {
-                            .map(bulkUpdate::apply)
+                        c.replaceOne(new Document("_id", holder), results);
-                            .filter(Objects::nonNull)
-                            .collect(Collectors.toSet());
-
-                    if (bulkUpdate.isTrackingStatistics() && !results.isEmpty()) {
-                        stats.incrementAffectedGroups();
-                        stats.incrementAffectedNodesBy(results.size());
-                    }
-
-                    if (!nodes.equals(results)) {
-                        List<Document> newNodes = results.stream()
-                                .map(MongoStorage::nodeToDoc)
-                                .collect(Collectors.toList());
-
-                        d.append("permissions", newNodes).remove("perms");
-                        c.replaceOne(new Document("_id", holder), d);
                     }
                 }
             }
         }
     }
 
+    private Document processBulkUpdate(Document document, BulkUpdate bulkUpdate, HolderType holderType) {
+        Set<Node> nodes = new HashSet<>(nodesFromDoc(document));
+        Set<Node> results = bulkUpdate.apply(nodes, holderType);
+
+        if (results == null) {
+            return null;
+        }
+
+        List<Document> newNodes = results.stream()
+                .map(MongoStorage::nodeToDoc)
+                .collect(Collectors.toList());
+
+        document.append("permissions", newNodes).remove("perms");
+        return document;
+    }
+
     @Override
     public User loadUser(UUID uniqueId, String username) {
         User user = this.plugin.getUserManager().getOrMake(uniqueId, username);
@@ -664,8 +645,8 @@ public class MongoStorage implements StorageImplementation {
         return null;
     }
 
-    private static UUID getDocumentId(Document doc) {
+    private static UUID getDocumentId(Document document) {
-        Object id = doc.get("_id");
+        Object id = document.get("_id");
         if (id instanceof UUID) {
             return (UUID) id;
         } else if (id instanceof String) {

common/src/main/java/me/lucko/luckperms/common/storage/implementation/sql/SqlStorage.java

@@ -271,10 +271,10 @@ public class SqlStorage implements StorageImplementation {
                                     uuids.add(Uuids.fromString(rs.getString("uuid")));
                                 }
                                 uuids.remove(null);
-                                stats.incrementAffectedUsersBy(uuids.size());
+                                stats.incrementAffectedUsers(uuids.size());
                             }
                         }
-                        stats.incrementAffectedNodesBy(ps.executeUpdate());
+                        stats.incrementAffectedNodes(ps.executeUpdate());
                     } else {
                         ps.execute();
                     }
@@ -298,10 +298,10 @@ public class SqlStorage implements StorageImplementation {
                                     groups.add(rs.getString("name"));
                                 }
                                 groups.remove(null);
-                                stats.incrementAffectedGroupsBy(groups.size());
+                                stats.incrementAffectedGroups(groups.size());
                             }
                         }
-                        stats.incrementAffectedNodesBy(ps.executeUpdate());
+                        stats.incrementAffectedNodes(ps.executeUpdate());
                     } else {
                         ps.execute();
                     }