Inject PermissibleBase instead of using attachments. Resolves #14

bukkit/src/main/java/me/lucko/luckperms/BukkitListener.java

@@ -23,6 +23,8 @@
 package me.lucko.luckperms;
 
 import me.lucko.luckperms.constants.Message;
+import me.lucko.luckperms.inject.LPPermissible;
+import me.lucko.luckperms.inject.Injector;
 import me.lucko.luckperms.users.BukkitUser;
 import me.lucko.luckperms.users.User;
 import me.lucko.luckperms.utils.AbstractListener;
@@ -30,10 +32,6 @@ import org.bukkit.entity.Player;
 import org.bukkit.event.EventHandler;
 import org.bukkit.event.Listener;
 import org.bukkit.event.player.*;
-import org.bukkit.permissions.PermissionAttachment;
-
-import java.util.Map;
-import java.util.concurrent.ConcurrentHashMap;
 
 class BukkitListener extends AbstractListener implements Listener {
     private final LPBukkitPlugin plugin;
@@ -67,20 +65,13 @@ class BukkitListener extends AbstractListener implements Listener {
         if (user instanceof BukkitUser) {
             BukkitUser u = (BukkitUser) user;
 
-            PermissionAttachment attachment = player.addAttachment(plugin);
-            Map<String, Boolean> newPermMap = new ConcurrentHashMap<>();
             try {
-                /* Replace the standard LinkedHashMap in the attachment with a ConcurrentHashMap.
+                LPPermissible lpPermissible = new LPPermissible(player, plugin);
-                   This means that we can iterate over and change the permissions within our attachment asynchronously,
+                Injector.inject(player, lpPermissible);
-                   without worrying about thread safety. The Bukkit side of things should still operate normally. Internal
+                u.setLpPermissible(lpPermissible);
-                   permission stuff should work the same. This is by far the most easy and efficient way to do things, without
-                   having to do tons of reflection. */
-                BukkitUser.getPermissionsField().set(attachment, newPermMap);
             } catch (Throwable t) {
                 t.printStackTrace();
             }
-
-            u.setAttachment(new BukkitUser.PermissionAttachmentHolder(attachment, newPermMap));
         }
 
         plugin.doAsync(user::refreshPermissions);

bukkit/src/main/java/me/lucko/luckperms/inject/Injector.java

@@ -0,0 +1,108 @@
+/*
+ * Copyright (c) 2016 Lucko (Luck) <luck@lucko.me>
+ *
+ *  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.inject;
+
+import lombok.experimental.UtilityClass;
+import org.bukkit.Bukkit;
+import org.bukkit.command.CommandSender;
+import org.bukkit.entity.Player;
+import org.bukkit.permissions.Permissible;
+import org.bukkit.permissions.PermissibleBase;
+
+import java.lang.reflect.Field;
+
+/**
+ * Injects a {@link LPPermissible} into a {@link Player}
+ */
+@UtilityClass
+public class Injector {
+    private static Field HUMAN_ENTITY_FIELD;
+
+    static {
+        try {
+            HUMAN_ENTITY_FIELD = Class.forName(getInternalClassName("entity.CraftHumanEntity")).getDeclaredField("perm");
+            HUMAN_ENTITY_FIELD.setAccessible(true);
+        } catch (Exception e) {
+            e.printStackTrace();
+        }
+    }
+
+    public static boolean inject(CommandSender sender, PermissibleBase permissible) {
+        try {
+            Field f = getPermField(sender);
+            f.set(sender, permissible);
+            return true;
+        } catch (Exception e) {
+            e.printStackTrace();
+            return false;
+        }
+    }
+
+    public static boolean unInject(CommandSender sender) {
+        try {
+            Permissible permissible = getPermissible(sender);
+            if (permissible instanceof LPPermissible) {
+                getPermField(sender).set(sender, new PermissibleBase(sender));
+            }
+            return true;
+        } catch (Exception e) {
+            e.printStackTrace();
+            return false;
+        }
+    }
+
+    private static Permissible getPermissible(CommandSender sender) {
+        try {
+            Field f = getPermField(sender);
+            return (Permissible) f.get(sender);
+        } catch (Exception e) {
+            throw new RuntimeException(e);
+        }
+    }
+
+    private static Field getPermField(CommandSender sender) {
+        if (sender instanceof Player) {
+            return HUMAN_ENTITY_FIELD;
+        }
+        throw new RuntimeException("Couldn't get perm field for sender " + sender.getClass().getName());
+    }
+
+    private static String getInternalClassName(String className) {
+        Class server = Bukkit.getServer().getClass();
+        if (!server.getSimpleName().equals("CraftServer")) {
+            throw new RuntimeException("Couldn't inject into server " + server);
+        }
+
+        String version;
+        if (server.getName().equals("org.bukkit.craftbukkit.CraftServer")) {
+            // Non versioned class
+            version = ".";
+        } else {
+            version = server.getName().substring("org.bukkit.craftbukkit".length());
+            version = version.substring(0, version.length() - "CraftServer".length());
+        }
+
+        return "org.bukkit.craftbukkit" + version + className;
+    }
+
+}

bukkit/src/main/java/me/lucko/luckperms/inject/LPPermissible.java

@@ -0,0 +1,293 @@
+/*
+ * Copyright (c) 2016 Lucko (Luck) <luck@lucko.me>
+ *
+ *  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.inject;
+
+import lombok.Getter;
+import lombok.NonNull;
+import me.lucko.luckperms.LuckPermsPlugin;
+import me.lucko.luckperms.api.Tristate;
+import me.lucko.luckperms.constants.Patterns;
+import org.bukkit.Bukkit;
+import org.bukkit.command.CommandSender;
+import org.bukkit.permissions.*;
+import org.bukkit.plugin.Plugin;
+
+import java.util.*;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.logging.Level;
+import java.util.stream.Collectors;
+
+/**
+ * Modified PermissibleBase for LuckPerms
+ */
+public class LPPermissible extends PermissibleBase {
+
+    @Getter
+    private final CommandSender parent;
+    private final LuckPermsPlugin plugin;
+
+    private final List<PermissionAttachment> attachments = new LinkedList<>();
+    private final Map<String, PermissionAttachmentInfo> attachmentPermissions = new HashMap<>();
+
+    @Getter
+    private final Map<String, Boolean> luckPermsPermissions = new ConcurrentHashMap<>();
+
+    public LPPermissible(@NonNull CommandSender sender, LuckPermsPlugin plugin) {
+        super(sender);
+        this.parent = sender;
+        this.plugin = plugin;
+    }
+
+    @Override
+    public boolean isOp() {
+        return parent.isOp();
+    }
+
+    @Override
+    public void setOp(boolean value) {
+        parent.setOp(value);
+    }
+
+    @Override
+    public boolean isPermissionSet(@NonNull String name) {
+        return luckPermsPermissions.containsKey(name.toLowerCase()) || attachmentPermissions.containsKey(name.toLowerCase());
+    }
+
+    @Override
+    public boolean isPermissionSet(@NonNull Permission perm) {
+        return isPermissionSet(perm.getName());
+    }
+
+    private Tristate getPermissionValue(String permission) {
+        if (plugin.getConfiguration().getDebugPermissionChecks()) {
+            plugin.getLog().info("Checking if " + parent.getName() + " has permission: " + permission);
+        }
+
+        permission = permission.toLowerCase();
+
+        if (luckPermsPermissions.containsKey(permission)) {
+            return Tristate.fromBoolean(luckPermsPermissions.get(permission));
+        }
+
+        if (attachmentPermissions.containsKey(permission)) {
+            return Tristate.fromBoolean(attachmentPermissions.get(permission).getValue());
+        }
+
+        if (plugin.getConfiguration().getApplyWildcards()) {
+            if (luckPermsPermissions.containsKey("*") || luckPermsPermissions.containsKey("'*'")) {
+                return Tristate.TRUE;
+            }
+
+            String node = "";
+            String[] permParts = Patterns.DOT.split(permission);
+            for (String s : permParts) {
+                if (node.equals("")) {
+                    node = s;
+                } else {
+                    node = node + "." + s;
+                }
+
+                if (luckPermsPermissions.containsKey(node + ".*")) {
+                    return Tristate.fromBoolean(luckPermsPermissions.get(node + ".*"));
+                }
+            }
+        }
+
+        Permission defPerm = Bukkit.getServer().getPluginManager().getPermission(permission);
+        if (defPerm != null) {
+            return Tristate.fromBoolean(defPerm.getDefault().getValue(isOp()));
+        }
+
+        return Tristate.UNDEFINED;
+    }
+
+    @Override
+    public boolean hasPermission(@NonNull String name) {
+        Tristate ts = getPermissionValue(name);
+        if (ts != Tristate.UNDEFINED) {
+            return ts.asBoolean();
+        }
+
+        return Permission.DEFAULT_PERMISSION.getValue(isOp());
+    }
+
+    @Override
+    public boolean hasPermission(@NonNull Permission perm) {
+        Tristate ts = getPermissionValue(perm.getName());
+        if (ts != Tristate.UNDEFINED) {
+            return ts.asBoolean();
+        }
+
+        return perm.getDefault().getValue(isOp());
+    }
+
+    @Override
+    public PermissionAttachment addAttachment(@NonNull Plugin plugin, @NonNull String name, boolean value) {
+        if (!plugin.isEnabled()) {
+            throw new IllegalArgumentException("Plugin " + plugin.getDescription().getFullName() + " is not enabled");
+        }
+
+        PermissionAttachment result = addAttachment(plugin);
+        result.setPermission(name, value);
+
+        recalculatePermissions();
+
+        return result;
+    }
+
+    @Override
+    public PermissionAttachment addAttachment(@NonNull Plugin plugin) {
+        if (!plugin.isEnabled()) {
+            throw new IllegalArgumentException("Plugin " + plugin.getDescription().getFullName() + " is not enabled");
+        }
+
+        PermissionAttachment result = new PermissionAttachment(plugin, parent);
+
+        attachments.add(result);
+        recalculatePermissions();
+
+        return result;
+    }
+
+    @Override
+    public PermissionAttachment addAttachment(@NonNull Plugin plugin, @NonNull String name, boolean value, int ticks) {
+        if (!plugin.isEnabled()) {
+            throw new IllegalArgumentException("Plugin " + plugin.getDescription().getFullName() + " is not enabled");
+        }
+
+        PermissionAttachment result = addAttachment(plugin, ticks);
+        if (result != null) {
+            result.setPermission(name, value);
+        }
+
+        return result;
+    }
+
+    @Override
+    public PermissionAttachment addAttachment(@NonNull Plugin plugin, int ticks) {
+        if (!plugin.isEnabled()) {
+            throw new IllegalArgumentException("Plugin " + plugin.getDescription().getFullName() + " is not enabled");
+        }
+
+        PermissionAttachment result = addAttachment(plugin);
+        if (Bukkit.getServer().getScheduler().scheduleSyncDelayedTask(plugin, new RemoveAttachmentRunnable(result), 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;
+        } else {
+            return result;
+        }
+    }
+
+    @Override
+    public void removeAttachment(@NonNull PermissionAttachment attachment) {
+        if (attachments.contains(attachment)) {
+            attachments.remove(attachment);
+            PermissionRemovedExecutor ex = attachment.getRemovalCallback();
+
+            if (ex != null) {
+                ex.attachmentRemoved(attachment);
+            }
+
+            recalculatePermissions();
+        } else {
+            throw new IllegalArgumentException("Given attachment is not part of Permissible object " + parent);
+        }
+    }
+
+    @Override
+    public void recalculatePermissions() {
+        if (attachmentPermissions == null) {
+            return;
+        }
+
+        attachmentPermissions.clear();
+        Set<Permission> defaults = Bukkit.getServer().getPluginManager().getDefaultPermissions(isOp());
+        Bukkit.getServer().getPluginManager().subscribeToDefaultPerms(isOp(), parent);
+
+        for (Permission perm : defaults) {
+            String name = perm.getName().toLowerCase();
+
+            attachmentPermissions.put(name, new PermissionAttachmentInfo(parent, name, null, true));
+            Bukkit.getServer().getPluginManager().subscribeToPermission(name, parent);
+            calculateChildPermissions(perm.getChildren(), false, null);
+        }
+
+        for (PermissionAttachment attachment : attachments) {
+            calculateChildPermissions(attachment.getPermissions(), false, attachment);
+        }
+    }
+
+    @Override
+    public synchronized void clearPermissions() {
+        Set<String> perms = attachmentPermissions.keySet();
+
+        for (String name : perms) {
+            Bukkit.getServer().getPluginManager().unsubscribeFromPermission(name, parent);
+        }
+
+        Bukkit.getServer().getPluginManager().unsubscribeFromDefaultPerms(false, parent);
+        Bukkit.getServer().getPluginManager().unsubscribeFromDefaultPerms(true, parent);
+
+        attachmentPermissions.clear();
+    }
+
+    private void calculateChildPermissions(Map<String, Boolean> children, boolean invert, PermissionAttachment attachment) {
+        for (Map.Entry<String, Boolean> e : children.entrySet()) {
+            Permission perm = Bukkit.getServer().getPluginManager().getPermission(e.getKey());
+            boolean value = e.getValue() ^ invert;
+            String name = e.getKey().toLowerCase();
+
+            attachmentPermissions.put(name, new PermissionAttachmentInfo(parent, name, attachment, value));
+            Bukkit.getServer().getPluginManager().subscribeToPermission(name, parent);
+
+            if (perm != null) {
+                calculateChildPermissions(perm.getChildren(), !value, attachment);
+            }
+        }
+    }
+
+    @Override
+    public Set<PermissionAttachmentInfo> getEffectivePermissions() {
+        Set<PermissionAttachmentInfo> perms = new HashSet<>();
+        perms.addAll(attachmentPermissions.values());
+
+        perms.addAll(luckPermsPermissions.entrySet().stream()
+                .map(e -> new PermissionAttachmentInfo(parent, e.getKey(), null, e.getValue()))
+                .collect(Collectors.toList()));
+
+        return perms;
+    }
+
+    private class RemoveAttachmentRunnable implements Runnable {
+        private PermissionAttachment attachment;
+
+        private RemoveAttachmentRunnable(PermissionAttachment attachment) {
+            this.attachment = attachment;
+        }
+
+        public void run() {
+            attachment.remove();
+        }
+    }
+}

bukkit/src/main/java/me/lucko/luckperms/users/BukkitUser.java

@@ -22,39 +22,24 @@
 
 package me.lucko.luckperms.users;
 
-import lombok.AllArgsConstructor;
-import lombok.EqualsAndHashCode;
 import lombok.Getter;
 import lombok.Setter;
 import me.lucko.luckperms.LPBukkitPlugin;
 import me.lucko.luckperms.api.event.events.UserPermissionRefreshEvent;
 import me.lucko.luckperms.api.implementation.internal.UserLink;
-import org.bukkit.permissions.PermissionAttachment;
+import me.lucko.luckperms.inject.LPPermissible;
+import org.bukkit.permissions.Permissible;
 
-import java.lang.reflect.Field;
+import java.util.Collections;
 import java.util.Map;
 import java.util.UUID;
 
 public class BukkitUser extends User {
-    private static Field permissionsField = null;
-    public static Field getPermissionsField() {
-        if (permissionsField == null) {
-            try {
-                permissionsField = PermissionAttachment.class.getDeclaredField("permissions");
-                permissionsField.setAccessible(true);
-            } catch (SecurityException | NoSuchFieldException e) {
-                e.printStackTrace();
-            }
-        }
-        return permissionsField;
-    }
-
-
     private final LPBukkitPlugin plugin;
 
     @Getter
     @Setter
-    private PermissionAttachmentHolder attachment = null;
+    private LPPermissible lpPermissible = null;
 
     BukkitUser(UUID uuid, LPBukkitPlugin plugin) {
         super(uuid, plugin);
@@ -69,7 +54,7 @@ public class BukkitUser extends User {
     @SuppressWarnings("unchecked")
     @Override
     public void refreshPermissions() {
-        if (attachment == null) {
+        if (lpPermissible == null) {
             return;
         }
 
@@ -80,12 +65,11 @@ public class BukkitUser extends User {
                 null,
                 plugin.getConfiguration().getIncludeGlobalPerms(),
                 true,
-                plugin.getPossiblePermissions()
+                Collections.emptyList()
         );
 
         try {
-            // The map in the LP PermissionAttachment is a ConcurrentHashMap. We can modify and iterate over its contents async.
+            Map<String, Boolean> existing = lpPermissible.getLuckPermsPermissions();
-            Map<String, Boolean> existing = attachment.getPermissions();
 
             boolean different = false;
             if (toApply.size() != existing.size()) {
@@ -105,39 +89,27 @@ public class BukkitUser extends User {
             existing.clear();
             existing.putAll(toApply);
 
-            boolean op = false;
             if (plugin.getConfiguration().getAutoOp()) {
+                boolean op = false;
+
                 for (Map.Entry<String, Boolean> e : toApply.entrySet()) {
                     if (e.getKey().equalsIgnoreCase("luckperms.autoop") && e.getValue()) {
                         op = true;
                         break;
                     }
                 }
-            }
-            boolean finalOp = op;
 
-            /* Must be called sync, as #recalculatePermissions is an unmodified Bukkit API call that is absolutely not thread safe.
+                final boolean finalOp = op;
-               Shouldn't be too taxing on the server. This only gets called when permissions have actually changed,
+                if (lpPermissible.isOp() != op) {
-               which is like once per user per login, assuming their permissions don't get modified. */
+                    final Permissible parent = lpPermissible.getParent();
-            plugin.doSync(() -> {
+                    plugin.doSync(() -> parent.setOp(finalOp));
-                attachment.getAttachment().getPermissible().recalculatePermissions();
+                }
-                if (plugin.getConfiguration().getAutoOp()) {
-                    attachment.getAttachment().getPermissible().setOp(finalOp);
             }
 
             plugin.getApiProvider().fireEventAsync(new UserPermissionRefreshEvent(new UserLink(this)));
-            });
 
         } catch (Exception e) {
             e.printStackTrace();
         }
     }
-
-    @Getter
-    @EqualsAndHashCode
-    @AllArgsConstructor
-    public static class PermissionAttachmentHolder {
-        private final PermissionAttachment attachment;
-        private final Map<String, Boolean> permissions;
-    }
 }

bukkit/src/main/java/me/lucko/luckperms/users/BukkitUserManager.java

@@ -24,6 +24,7 @@ package me.lucko.luckperms.users;
 
 import lombok.Getter;
 import me.lucko.luckperms.LPBukkitPlugin;
+import me.lucko.luckperms.inject.Injector;
 import org.bukkit.entity.Player;
 
 import java.util.Map;
@@ -49,9 +50,9 @@ public class BukkitUserManager extends UserManager {
             BukkitUser u = (BukkitUser) user;
             Player player = plugin.getServer().getPlayer(plugin.getUuidCache().getExternalUUID(u.getUuid()));
             if (player != null) {
-                if (u.getAttachment() != null) {
+                if (u.getLpPermissible() != null) {
-                    player.removeAttachment(u.getAttachment().getAttachment());
+                    Injector.unInject(player);
-                    u.setAttachment(null);
+                    u.setLpPermissible(null);
                 }
 
                 if (plugin.getConfiguration().getAutoOp()) {

bukkit/src/main/resources/config.yml

@@ -46,6 +46,9 @@ apply-shorthand: true
 # If the plugin should send log notifications to users whenever permissions are modified.
 log-notify: true
 
+# If LuckPerms should print to console every time a plugin checks if a player has a permission
+debug-permission-checks: false
+
 # If the vanilla OP system is enabled. If set to false, all users will be de-opped, and the op/deop commands will be disabled.
 enable-ops: true
 

common/src/main/java/me/lucko/luckperms/core/LPConfiguration.java

@@ -111,6 +111,10 @@ public abstract class LPConfiguration<T extends LuckPermsPlugin> {
         return getBoolean("log-notify", true);
     }
 
+    public boolean getDebugPermissionChecks() {
+        return getBoolean("debug-permission-checks", false);
+    }
+
     public boolean getEnableOps() {
         return !getAutoOp() && getBoolean("enable-ops", true);
     }