From 454abec46890456d7382ff59707b124f15bc5603 Mon Sep 17 00:00:00 2001 From: Luck Date: Thu, 5 Jan 2017 18:19:14 +0000 Subject: [PATCH] Cleanup vault hook --- .../luckperms/bukkit/model/LPPermissible.java | 2 +- .../luckperms/bukkit/vault/VaultHook.java | 17 +- .../bukkit/vault/VaultPermissionHook.java | 163 ++++++++++-------- .../bukkit/vault/VaultScheduler.java | 29 +++- 4 files changed, 120 insertions(+), 91 deletions(-) diff --git a/bukkit/src/main/java/me/lucko/luckperms/bukkit/model/LPPermissible.java b/bukkit/src/main/java/me/lucko/luckperms/bukkit/model/LPPermissible.java index 77fdd060..067b0711 100644 --- a/bukkit/src/main/java/me/lucko/luckperms/bukkit/model/LPPermissible.java +++ b/bukkit/src/main/java/me/lucko/luckperms/bukkit/model/LPPermissible.java @@ -208,7 +208,7 @@ public class LPPermissible extends PermissibleBase { } PermissionAttachment result = addAttachment(plugin); - if (Bukkit.getServer().getScheduler().scheduleSyncDelayedTask(plugin, () -> result.remove(), ticks) == -1) { + if (Bukkit.getServer().getScheduler().scheduleSyncDelayedTask(plugin, result::remove, ticks) == -1) { Bukkit.getServer().getLogger().log(Level.WARNING, "Could not add PermissionAttachment to " + parent + " for plugin " + plugin.getDescription().getFullName() + ": Scheduler returned -1"); result.remove(); return null; diff --git a/bukkit/src/main/java/me/lucko/luckperms/bukkit/vault/VaultHook.java b/bukkit/src/main/java/me/lucko/luckperms/bukkit/vault/VaultHook.java index ec6dc306..c4ff232e 100644 --- a/bukkit/src/main/java/me/lucko/luckperms/bukkit/vault/VaultHook.java +++ b/bukkit/src/main/java/me/lucko/luckperms/bukkit/vault/VaultHook.java @@ -32,6 +32,9 @@ import net.milkbowl.vault.permission.Permission; import org.bukkit.plugin.ServicePriority; import org.bukkit.plugin.ServicesManager; +/** + * Handles hooking with the Vault API + */ @Getter public class VaultHook { private VaultChatHook chatHook = null; @@ -40,17 +43,8 @@ public class VaultHook { public void hook(LPBukkitPlugin plugin) { try { if (permissionHook == null) { - permissionHook = new VaultPermissionHook(); + permissionHook = new VaultPermissionHook(plugin); } - permissionHook.setPlugin(plugin); - permissionHook.setServer(plugin.getConfiguration().getVaultServer()); - permissionHook.setIncludeGlobal(plugin.getConfiguration().isVaultIncludingGlobal()); - permissionHook.setIgnoreWorld(plugin.getConfiguration().isVaultIgnoreWorld()); - permissionHook.setPgo(plugin.getConfiguration().isVaultPrimaryGroupOverrides()); - permissionHook.setPgoCheckInherited(plugin.getConfiguration().isVaultPrimaryGroupOverridesCheckInherited()); - permissionHook.setPgoCheckExists(plugin.getConfiguration().isVaultPrimaryGroupOverridesCheckExists()); - permissionHook.setPgoCheckMemberOf(plugin.getConfiguration().isVaultPrimaryGroupOverridesCheckMemberOf()); - permissionHook.setup(); if (chatHook == null) { chatHook = new VaultChatHook(permissionHook); @@ -69,9 +63,12 @@ public class VaultHook { final ServicesManager sm = plugin.getServer().getServicesManager(); if (permissionHook != null) { sm.unregister(Permission.class, permissionHook); + permissionHook.getScheduler().cancelTask(); + permissionHook = null; } if (chatHook != null) { sm.unregister(Chat.class, chatHook); + chatHook = null; } } diff --git a/bukkit/src/main/java/me/lucko/luckperms/bukkit/vault/VaultPermissionHook.java b/bukkit/src/main/java/me/lucko/luckperms/bukkit/vault/VaultPermissionHook.java index b0d68127..4a9e0e0c 100644 --- a/bukkit/src/main/java/me/lucko/luckperms/bukkit/vault/VaultPermissionHook.java +++ b/bukkit/src/main/java/me/lucko/luckperms/bukkit/vault/VaultPermissionHook.java @@ -22,10 +22,8 @@ package me.lucko.luckperms.bukkit.vault; -import lombok.AccessLevel; import lombok.Getter; import lombok.NonNull; -import lombok.Setter; import me.lucko.luckperms.api.Contexts; import me.lucko.luckperms.api.LocalizedNode; @@ -44,31 +42,42 @@ import net.milkbowl.vault.permission.Permission; import java.util.HashMap; import java.util.Map; +import java.util.concurrent.CompletableFuture; +import java.util.function.Function; /** * The LuckPerms Vault Permission implementation * Most lookups are cached. */ @Getter -@Setter public class VaultPermissionHook extends Permission { private LPBukkitPlugin plugin; - - @Setter(value = AccessLevel.NONE) private VaultScheduler scheduler; + private final String name = "LuckPerms"; + private String server = "global"; private boolean includeGlobal = true; private boolean ignoreWorld = false; - - // Primary Group override settings private boolean pgo = false; private boolean pgoCheckInherited = false; private boolean pgoCheckExists = true; private boolean pgoCheckMemberOf = true; - public void setup() { - scheduler = new VaultScheduler(plugin); + private Function WORLD_CORRECTION_FUNCTION = s -> ignoreWorld ? null : s; + + public VaultPermissionHook(LPBukkitPlugin plugin) { + this.plugin = plugin; + this.scheduler = new VaultScheduler(plugin); + + // Config options + this.server = plugin.getConfiguration().getVaultServer(); + this.includeGlobal = plugin.getConfiguration().isVaultIncludingGlobal(); + this.ignoreWorld = plugin.getConfiguration().isVaultIgnoreWorld(); + this.pgo = plugin.getConfiguration().isVaultPrimaryGroupOverrides(); + this.pgoCheckInherited = plugin.getConfiguration().isVaultPrimaryGroupOverridesCheckInherited(); + this.pgoCheckExists = plugin.getConfiguration().isVaultPrimaryGroupOverridesCheckExists(); + this.pgoCheckMemberOf = plugin.getConfiguration().isVaultPrimaryGroupOverridesCheckMemberOf(); } public void log(String s) { @@ -77,14 +86,9 @@ public class VaultPermissionHook extends Permission { } } - @Override - public String getName() { - return "LuckPerms"; - } - @Override public boolean isEnabled() { - return plugin.getStorage().isAcceptingLogins(); + return plugin.isEnabled(); } @Override @@ -99,16 +103,18 @@ public class VaultPermissionHook extends Permission { * @param holder the holder to add the permission to * @param permission the permission to add */ - private void add(String world, PermissionHolder holder, String permission) { - try { - if (world != null && !world.equals("")) { - holder.setPermission(permission, true, server, world); - } else { - holder.setPermission(permission, true, server); - } + private CompletableFuture add(String world, PermissionHolder holder, String permission) { + return CompletableFuture.runAsync(() -> { + try { + if (world != null && !world.equals("") && !world.equalsIgnoreCase("global")) { + holder.setPermission(permission, true, server, world); + } else { + holder.setPermission(permission, true, server); + } - save(holder); - } catch (ObjectAlreadyHasException ignored) {} + save(holder); + } catch (ObjectAlreadyHasException ignored) {} + }, scheduler); } /** @@ -118,35 +124,37 @@ public class VaultPermissionHook extends Permission { * @param holder the holder to remove the permission from * @param permission the permission to remove */ - private void remove(String world, PermissionHolder holder, String permission) { - try { - if (world != null && !world.equals("")) { - holder.unsetPermission(permission, server, world); - } else { - holder.unsetPermission(permission, server); - } + private CompletableFuture remove(String world, PermissionHolder holder, String permission) { + return CompletableFuture.runAsync(() -> { + try { + if (world != null && !world.equals("") && !world.equalsIgnoreCase("global")) { + holder.unsetPermission(permission, server, world); + } else { + holder.unsetPermission(permission, server); + } - save(holder); - } catch (ObjectLacksException ignored) {} + save(holder); + } catch (ObjectLacksException ignored) {} + }, scheduler); } /** - * Utility method for saving a user or group + * Utility method to asynchronously save a user or group * * @param holder the holder instance */ - void save(PermissionHolder holder) { + public void save(PermissionHolder holder) { if (holder instanceof User) { - plugin.getStorage().saveUser(((User) holder)) - .thenRunAsync(() -> ((User) holder).getRefreshBuffer().request(), plugin.getAsyncExecutor()); + User u = (User) holder; + plugin.getStorage().saveUser(u).thenRunAsync(() -> u.getRefreshBuffer().request(), plugin.getAsyncExecutor()); } if (holder instanceof Group) { - plugin.getStorage().saveGroup(((Group) holder)) - .thenRunAsync(() -> plugin.getUpdateTaskBuffer().request(), plugin.getAsyncExecutor()); + Group g = (Group) holder; + plugin.getStorage().saveGroup(g).thenRunAsync(() -> plugin.getUpdateTaskBuffer().request(), plugin.getAsyncExecutor()); } } - Contexts createContext(String server, String world) { + public Contexts createContextForWorld(String world) { Map context = new HashMap<>(); if (world != null && !world.equals("")) { context.put("world", world); @@ -157,7 +165,7 @@ public class VaultPermissionHook extends Permission { @Override public boolean playerHas(String world, @NonNull String player, @NonNull String permission) { - world = ignoreWorld ? null : world; // Correct world value + world = WORLD_CORRECTION_FUNCTION.apply(world); log("Checking if player " + player + " has permission: " + permission + " on world " + world + ", server " + server); User user = plugin.getUserManager().getByUsername(player); @@ -168,91 +176,91 @@ public class VaultPermissionHook extends Permission { } // Effectively fallback to the standard Bukkit #hasPermission check. - return user.getUserData().getPermissionData(createContext(server, world)).getPermissionValue(permission).asBoolean(); + return user.getUserData().getPermissionData(createContextForWorld(world)).getPermissionValue(permission).asBoolean(); } @Override public boolean playerAdd(String world, @NonNull String player, @NonNull String permission) { - String finalWorld = ignoreWorld ? null : world; // Correct world value - log("Adding permission to player " + player + ": '" + permission + "' on world " + finalWorld + ", server " + server); + world = WORLD_CORRECTION_FUNCTION.apply(world); + log("Adding permission to player " + player + ": '" + permission + "' on world " + world + ", server " + server); final User user = plugin.getUserManager().getByUsername(player); if (user == null) return false; - scheduler.scheduleTask(() -> add(finalWorld, user, permission)); + add(world, user, permission); return true; } @Override public boolean playerRemove(String world, @NonNull String player, @NonNull String permission) { - String finalWorld = ignoreWorld ? null : world; // Correct world value - log("Removing permission from player " + player + ": '" + permission + "' on world " + finalWorld + ", server " + server); + world = WORLD_CORRECTION_FUNCTION.apply(world); + log("Removing permission from player " + player + ": '" + permission + "' on world " + world + ", server " + server); final User user = plugin.getUserManager().getByUsername(player); if (user == null) return false; - scheduler.scheduleTask(() -> remove(finalWorld, user, permission)); + remove(world, user, permission); return true; } @Override public boolean groupHas(String world, @NonNull String groupName, @NonNull String permission) { - world = ignoreWorld ? null : world; // Correct world value + world = WORLD_CORRECTION_FUNCTION.apply(world); log("Checking if group " + groupName + " has permission: " + permission + " on world " + world + ", server " + server); final Group group = plugin.getGroupManager().getIfLoaded(groupName); if (group == null) return false; // This is a nasty call. Groups aren't cached. :( - Map permissions = group.exportNodes(createContext(server, world), true); - - return permissions.containsKey(permission) && permissions.get(permission); + Map permissions = group.exportNodes(createContextForWorld(world), true); + return permissions.containsKey(permission.toLowerCase()) && permissions.get(permission.toLowerCase()); } @Override public boolean groupAdd(String world, @NonNull String groupName, @NonNull String permission) { - String finalWorld = ignoreWorld ? null : world; // Correct world value - log("Adding permission to group " + groupName + ": '" + permission + "' on world " + finalWorld + ", server " + server); + world = WORLD_CORRECTION_FUNCTION.apply(world); + log("Adding permission to group " + groupName + ": '" + permission + "' on world " + world + ", server " + server); final Group group = plugin.getGroupManager().getIfLoaded(groupName); if (group == null) return false; - scheduler.scheduleTask(() -> add(finalWorld, group, permission)); + add(world, group, permission); return true; } @Override public boolean groupRemove(String world, @NonNull String groupName, @NonNull String permission) { - String finalWorld = ignoreWorld ? null : world; // Correct world value - log("Removing permission from group " + groupName + ": '" + permission + "' on world " + finalWorld + ", server " + server); + world = WORLD_CORRECTION_FUNCTION.apply(world); + log("Removing permission from group " + groupName + ": '" + permission + "' on world " + world + ", server " + server); final Group group = plugin.getGroupManager().getIfLoaded(groupName); if (group == null) return false; - scheduler.scheduleTask(() -> remove(finalWorld, group, permission)); + remove(world, group, permission); return true; } @Override public boolean playerInGroup(String world, @NonNull String player, @NonNull String group) { - String finalWorld = ignoreWorld ? null : world; // Correct world value - log("Checking if player " + player + " is in group: " + group + " on world " + finalWorld + ", server " + server); + world = WORLD_CORRECTION_FUNCTION.apply(world); + log("Checking if player " + player + " is in group: " + group + " on world " + world + ", server " + server); final User user = plugin.getUserManager().getByUsername(player); if (user == null) return false; + String w = world; // screw effectively final return user.getNodes().stream() .filter(Node::isGroupNode) .filter(n -> n.shouldApplyOnServer(server, isIncludeGlobal(), false)) - .filter(n -> n.shouldApplyOnWorld(finalWorld, true, false)) + .filter(n -> n.shouldApplyOnWorld(w, true, false)) .map(Node::getGroupName) .anyMatch(s -> s.equalsIgnoreCase(group)); } @Override public boolean playerAddGroup(String world, @NonNull String player, @NonNull String groupName) { - String finalWorld = ignoreWorld ? null : world; // Correct world value - log("Adding player " + player + " to group: '" + groupName + "' on world " + finalWorld + ", server " + server); + world = WORLD_CORRECTION_FUNCTION.apply(world); + log("Adding player " + player + " to group: '" + groupName + "' on world " + world + ", server " + server); final User user = plugin.getUserManager().getByUsername(player); if (user == null) return false; @@ -260,10 +268,11 @@ public class VaultPermissionHook extends Permission { final Group group = plugin.getGroupManager().getIfLoaded(groupName); if (group == null) return false; - scheduler.scheduleTask(() -> { + String w = world; + scheduler.execute(() -> { try { - if (finalWorld != null && !finalWorld.equals("")) { - user.setInheritGroup(group, server, finalWorld); + if (w != null && !w.equals("") && !w.equalsIgnoreCase("global")) { + user.setInheritGroup(group, server, w); } else { user.setInheritGroup(group, server); } @@ -276,8 +285,8 @@ public class VaultPermissionHook extends Permission { @Override public boolean playerRemoveGroup(String world, @NonNull String player, @NonNull String groupName) { - String finalWorld = ignoreWorld ? null : world; // Correct world value - log("Removing player " + player + " from group: '" + groupName + "' on world " + finalWorld + ", server " + server); + world = WORLD_CORRECTION_FUNCTION.apply(world); + log("Removing player " + player + " from group: '" + groupName + "' on world " + world + ", server " + server); final User user = plugin.getUserManager().getByUsername(player); if (user == null) return false; @@ -285,10 +294,11 @@ public class VaultPermissionHook extends Permission { final Group group = plugin.getGroupManager().getIfLoaded(groupName); if (group == null) return false; - scheduler.scheduleTask(() -> { + String w = world; + scheduler.execute(() -> { try { - if (finalWorld != null && !finalWorld.equals("")) { - user.unsetInheritGroup(group, server, finalWorld); + if (w != null && !w.equals("") && !w.equalsIgnoreCase("global")) { + user.unsetInheritGroup(group, server, w); } else { user.unsetInheritGroup(group, server); } @@ -301,23 +311,24 @@ public class VaultPermissionHook extends Permission { @Override public String[] getPlayerGroups(String world, @NonNull String player) { - String finalWorld = ignoreWorld ? null : world; // Correct world value - log("Getting groups of player: " + player + ", on world " + finalWorld + ", server " + server); + world = WORLD_CORRECTION_FUNCTION.apply(world); + log("Getting groups of player: " + player + ", on world " + world + ", server " + server); User user = plugin.getUserManager().getByUsername(player); if (user == null) return new String[0]; + String w = world; // screw effectively final return user.getNodes().stream() .filter(Node::isGroupNode) .filter(n -> n.shouldApplyOnServer(server, isIncludeGlobal(), false)) - .filter(n -> n.shouldApplyOnWorld(finalWorld, true, false)) + .filter(n -> n.shouldApplyOnWorld(w, true, false)) .map(Node::getGroupName) .toArray(String[]::new); } @Override public String getPrimaryGroup(String world, @NonNull String player) { - world = ignoreWorld ? null : world; // Correct world value + world = WORLD_CORRECTION_FUNCTION.apply(world); log("Getting primary group of player: " + player); final User user = plugin.getUserManager().getByUsername(player); @@ -331,7 +342,7 @@ public class VaultPermissionHook extends Permission { } if (pgoCheckInherited) { - PermissionData data = user.getUserData().getPermissionData(createContext(server, world)); + PermissionData data = user.getUserData().getPermissionData(createContextForWorld(world)); for (Map.Entry e : data.getImmutableBacking().entrySet()) { if (!e.getValue()) { continue; diff --git a/bukkit/src/main/java/me/lucko/luckperms/bukkit/vault/VaultScheduler.java b/bukkit/src/main/java/me/lucko/luckperms/bukkit/vault/VaultScheduler.java index 87e42d2e..24239aff 100644 --- a/bukkit/src/main/java/me/lucko/luckperms/bukkit/vault/VaultScheduler.java +++ b/bukkit/src/main/java/me/lucko/luckperms/bukkit/vault/VaultScheduler.java @@ -24,17 +24,26 @@ package me.lucko.luckperms.bukkit.vault; import me.lucko.luckperms.bukkit.LPBukkitPlugin; +import org.bukkit.scheduler.BukkitTask; + import java.util.ArrayList; import java.util.List; +import java.util.concurrent.Executor; -public class VaultScheduler implements Runnable { +/** + * Sequential executor for Vault modifications + */ +public class VaultScheduler implements Runnable, Executor { + + private BukkitTask task = null; private final List tasks = new ArrayList<>(); public VaultScheduler(LPBukkitPlugin plugin) { - plugin.getServer().getScheduler().runTaskTimerAsynchronously(plugin, this, 1L, 1L); + task = plugin.getServer().getScheduler().runTaskTimerAsynchronously(plugin, this, 1L, 1L); } - public void scheduleTask(Runnable r) { + @Override + public void execute(Runnable r) { synchronized (tasks) { tasks.add(r); } @@ -42,12 +51,24 @@ public class VaultScheduler implements Runnable { @Override public void run() { - List toRun = new ArrayList<>(); + List toRun; synchronized (tasks) { + if (tasks.isEmpty()) { + return; + } + + toRun = new ArrayList<>(); toRun.addAll(tasks); tasks.clear(); } toRun.forEach(Runnable::run); } + + public void cancelTask() { + if (task != null) { + task.cancel(); + task = null; + } + } }