From 770b7dc282be6e4f0cb66f22072f8d64f3161247 Mon Sep 17 00:00:00 2001 From: Luck Date: Sun, 4 Sep 2016 17:59:01 +0100 Subject: [PATCH] Fix permissions not being removed on Sponge --- .../luckperms/users/BukkitUserManager.java | 21 +++--- .../luckperms/users/BungeeUserManager.java | 9 --- .../lucko/luckperms/tracks/TrackManager.java | 2 +- .../me/lucko/luckperms/users/UserManager.java | 5 +- .../luckperms/utils/AbstractManager.java | 66 +++++++++++++------ .../me/lucko/luckperms/SpongeListener.java | 10 --- .../api/sponge/LuckPermsSubject.java | 34 +++++----- .../sponge/collections/GroupCollection.java | 22 ++----- .../sponge/collections/UserCollection.java | 23 ++----- .../luckperms/users/SpongeUserManager.java | 9 +-- 10 files changed, 84 insertions(+), 117 deletions(-) diff --git a/bukkit/src/main/java/me/lucko/luckperms/users/BukkitUserManager.java b/bukkit/src/main/java/me/lucko/luckperms/users/BukkitUserManager.java index 95546bbad..ad935effb 100644 --- a/bukkit/src/main/java/me/lucko/luckperms/users/BukkitUserManager.java +++ b/bukkit/src/main/java/me/lucko/luckperms/users/BukkitUserManager.java @@ -44,23 +44,18 @@ public class BukkitUserManager extends UserManager { } @Override - public void unload(User user) { - if (user != null) { + public void preUnload(User user) { + if (user instanceof BukkitUser) { + BukkitUser u = (BukkitUser) user; - if (user instanceof BukkitUser) { - BukkitUser u = (BukkitUser) user; + if (u.getAttachment() != null) { + Player player = plugin.getServer().getPlayer(plugin.getUuidCache().getExternalUUID(u.getUuid())); - if (u.getAttachment() != null) { - Player player = plugin.getServer().getPlayer(plugin.getUuidCache().getExternalUUID(u.getUuid())); - - if (player != null) { - player.removeAttachment(u.getAttachment()); - } - u.setAttachment(null); + if (player != null) { + player.removeAttachment(u.getAttachment()); } + u.setAttachment(null); } - - getAll().remove(user.getUuid()); } } diff --git a/bungee/src/main/java/me/lucko/luckperms/users/BungeeUserManager.java b/bungee/src/main/java/me/lucko/luckperms/users/BungeeUserManager.java index bb6f4ce7b..fe1f3a012 100644 --- a/bungee/src/main/java/me/lucko/luckperms/users/BungeeUserManager.java +++ b/bungee/src/main/java/me/lucko/luckperms/users/BungeeUserManager.java @@ -34,15 +34,6 @@ public class BungeeUserManager extends UserManager { this.plugin = plugin; } - @Override - public void unload(User user) { - if (user != null) { - // Cannot clear the ProxiedPlayer's permission map, they're leaving so that will get GCed anyway - // Calling getPermissions.clear() throws an UnsupportedOperationException - getAll().remove(user.getUuid()); - } - } - @Override public void cleanup(User user) { if (plugin.getProxy().getPlayer(plugin.getUuidCache().getExternalUUID(user.getUuid())) == null) { diff --git a/common/src/main/java/me/lucko/luckperms/tracks/TrackManager.java b/common/src/main/java/me/lucko/luckperms/tracks/TrackManager.java index 35b3fe3c2..05ab31169 100644 --- a/common/src/main/java/me/lucko/luckperms/tracks/TrackManager.java +++ b/common/src/main/java/me/lucko/luckperms/tracks/TrackManager.java @@ -35,7 +35,7 @@ public class TrackManager extends AbstractManager { * @return a set of tracks that the groups could be a member of */ public Set getApplicableTracks(String group) { - return objects.values().stream() + return getAll().values().stream() .filter(t -> t.containsGroup(group)) .collect(Collectors.toSet()); } diff --git a/common/src/main/java/me/lucko/luckperms/users/UserManager.java b/common/src/main/java/me/lucko/luckperms/users/UserManager.java index b91ec657c..ff8d15654 100644 --- a/common/src/main/java/me/lucko/luckperms/users/UserManager.java +++ b/common/src/main/java/me/lucko/luckperms/users/UserManager.java @@ -45,7 +45,7 @@ public abstract class UserManager extends AbstractManager { @SuppressWarnings("OptionalGetWithoutIsPresent") public User get(String name) { try { - return objects.values().stream() + return getAll().values().stream() .filter(u -> u.getName().equalsIgnoreCase(name)) .limit(1).findAny().get(); } catch (NoSuchElementException e) { @@ -54,9 +54,8 @@ public abstract class UserManager extends AbstractManager { } @Override - public void set(User u) { + public void preSet(User u) { giveDefaultIfNeeded(u, true); - super.set(u); } @Override diff --git a/common/src/main/java/me/lucko/luckperms/utils/AbstractManager.java b/common/src/main/java/me/lucko/luckperms/utils/AbstractManager.java index d4d2423d1..14f8e6538 100644 --- a/common/src/main/java/me/lucko/luckperms/utils/AbstractManager.java +++ b/common/src/main/java/me/lucko/luckperms/utils/AbstractManager.java @@ -22,8 +22,10 @@ package me.lucko.luckperms.utils; +import com.google.common.collect.ImmutableMap; + +import java.util.HashMap; import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; /** * An abstract manager class @@ -31,10 +33,12 @@ import java.util.concurrent.ConcurrentHashMap; * @param the class this manager is "managing" */ public abstract class AbstractManager> { - protected final Map objects = new ConcurrentHashMap<>(); + private final Map objects = new HashMap<>(); - public Map getAll() { - return objects; + public final Map getAll() { + synchronized (objects) { + return ImmutableMap.copyOf(objects); + } } /** @@ -42,28 +46,39 @@ public abstract class AbstractManager> { * @param id The id to search by * @return a {@link T} object if the object is loaded, returns null if the object is not loaded */ - public T get(I id) { - return objects.get(id); + public final T get(I id) { + synchronized (objects) { + return objects.get(id); + } } /** * Add a object to the loaded objects map * @param t The object to add */ - public void set(T t) { - objects.put(t.getId(), t); + public final void set(T t) { + preSet(t); + synchronized (objects) { + objects.put(t.getId(), t); + } + } + + public void preSet(T t) { + } /** * Updates (or sets if the object wasn't already loaded) an object in the objects map * @param t The object to update or set */ - public void updateOrSet(T t) { - if (!isLoaded(t.getId())) { - // The object isn't already loaded - set(t); - } else { - copy(t, objects.get(t.getId())); + public final void updateOrSet(T t) { + synchronized (objects) { + if (!isLoaded(t.getId())) { + // The object isn't already loaded + set(t); + } else { + copy(t, objects.get(t.getId())); + } } } @@ -74,25 +89,36 @@ public abstract class AbstractManager> { * @param id The id of the object * @return true if the object is loaded */ - public boolean isLoaded(I id) { - return objects.containsKey(id); + public final boolean isLoaded(I id) { + synchronized (objects) { + return objects.containsKey(id); + } } /** * Removes and unloads the object from the manager * @param t The object to unload */ - public void unload(T t) { + public final void unload(T t) { if (t != null) { - objects.remove(t.getId()); + preUnload(t); + synchronized (objects) { + objects.remove(t.getId()); + } } } + public void preUnload(T t) { + + } + /** * Unloads all objects from the manager */ - public void unloadAll() { - objects.clear(); + public final void unloadAll() { + synchronized (objects) { + objects.clear(); + } } /** diff --git a/sponge/src/main/java/me/lucko/luckperms/SpongeListener.java b/sponge/src/main/java/me/lucko/luckperms/SpongeListener.java index 90c1e0b6a..7d5043da2 100644 --- a/sponge/src/main/java/me/lucko/luckperms/SpongeListener.java +++ b/sponge/src/main/java/me/lucko/luckperms/SpongeListener.java @@ -22,7 +22,6 @@ package me.lucko.luckperms; -import me.lucko.luckperms.api.sponge.LuckPermsSubject; import me.lucko.luckperms.constants.Message; import me.lucko.luckperms.users.User; import me.lucko.luckperms.utils.AbstractListener; @@ -31,8 +30,6 @@ import org.spongepowered.api.event.network.ClientConnectionEvent; import org.spongepowered.api.profile.GameProfile; import org.spongepowered.api.text.serializer.TextSerializers; -import java.util.Iterator; - @SuppressWarnings("WeakerAccess") public class SpongeListener extends AbstractListener { private final LPSpongePlugin plugin; @@ -78,12 +75,5 @@ public class SpongeListener extends AbstractListener { @Listener public void onClientLeave(ClientConnectionEvent.Disconnect e) { onLeave(e.getTargetEntity().getUniqueId()); - Iterator iterator = plugin.getService().getUserSubjects().getCache().iterator(); - while (iterator.hasNext()) { - LuckPermsSubject subject = iterator.next(); - if (subject.getIdentifier().equalsIgnoreCase(e.getTargetEntity().getUniqueId().toString())) { - iterator.remove(); - } - } } } diff --git a/sponge/src/main/java/me/lucko/luckperms/api/sponge/LuckPermsSubject.java b/sponge/src/main/java/me/lucko/luckperms/api/sponge/LuckPermsSubject.java index 61d06794c..0104ae555 100644 --- a/sponge/src/main/java/me/lucko/luckperms/api/sponge/LuckPermsSubject.java +++ b/sponge/src/main/java/me/lucko/luckperms/api/sponge/LuckPermsSubject.java @@ -52,13 +52,17 @@ import static me.lucko.luckperms.utils.ArgumentChecker.unescapeCharacters; @EqualsAndHashCode(of = {"holder"}) public class LuckPermsSubject implements Subject { + public static Subject wrapHolder(PermissionHolder holder, LuckPermsService service) { + return new LuckPermsSubject(holder, service); + } + @Getter private final PermissionHolder holder; private final EnduringData enduringData; private final TransientData transientData; private final LuckPermsService service; - public LuckPermsSubject(PermissionHolder holder, LuckPermsService service) { + private LuckPermsSubject(PermissionHolder holder, LuckPermsService service) { this.holder = holder; this.enduringData = new EnduringData(this, service, holder); this.transientData = new TransientData(service, holder); @@ -77,13 +81,13 @@ public class LuckPermsSubject implements Subject { @Override public String getIdentifier() { - return enduringData.getHolder().getObjectName(); + return holder.getObjectName(); } @Override public Optional getCommandSource() { - if (enduringData.getHolder() instanceof User) { - final UUID uuid = ((User) enduringData.getHolder()).getUuid(); + if (holder instanceof User) { + final UUID uuid = ((User) holder).getUuid(); Optional p = Sponge.getServer().getPlayer(uuid); if (p.isPresent()) { @@ -96,7 +100,7 @@ public class LuckPermsSubject implements Subject { @Override public SubjectCollection getContainingCollection() { - if (enduringData.getHolder() instanceof Group) { + if (holder instanceof Group) { return service.getGroupSubjects(); } else { return service.getUserSubjects(); @@ -130,18 +134,16 @@ public class LuckPermsSubject implements Subject { context.put(c.getKey(), c.getValue()); } - me.lucko.luckperms.api.Tristate t = enduringData.getHolder().inheritsPermission(new me.lucko.luckperms.utils.Node.Builder(node).withExtraContext(context).build()); - if (t == me.lucko.luckperms.api.Tristate.UNDEFINED) { - return Tristate.UNDEFINED; + switch (holder.inheritsPermission(new me.lucko.luckperms.utils.Node.Builder(node).withExtraContext(context).build())) { + case UNDEFINED: + return Tristate.UNDEFINED; + case TRUE: + return Tristate.TRUE; + case FALSE: + return Tristate.FALSE; + default: + return null; } - if (t == me.lucko.luckperms.api.Tristate.TRUE) { - return Tristate.TRUE; - } - if (t == me.lucko.luckperms.api.Tristate.FALSE) { - return Tristate.FALSE; - } - - return null; } @Override diff --git a/sponge/src/main/java/me/lucko/luckperms/api/sponge/collections/GroupCollection.java b/sponge/src/main/java/me/lucko/luckperms/api/sponge/collections/GroupCollection.java index 9cd31a082..0d6168292 100644 --- a/sponge/src/main/java/me/lucko/luckperms/api/sponge/collections/GroupCollection.java +++ b/sponge/src/main/java/me/lucko/luckperms/api/sponge/collections/GroupCollection.java @@ -27,7 +27,6 @@ import lombok.NonNull; import me.lucko.luckperms.api.sponge.LuckPermsService; import me.lucko.luckperms.api.sponge.LuckPermsSubject; import me.lucko.luckperms.api.sponge.simple.SimpleSubject; -import me.lucko.luckperms.core.PermissionHolder; import me.lucko.luckperms.groups.GroupManager; import org.spongepowered.api.service.context.Context; import org.spongepowered.api.service.permission.PermissionService; @@ -35,17 +34,14 @@ import org.spongepowered.api.service.permission.Subject; import org.spongepowered.api.service.permission.SubjectCollection; import org.spongepowered.api.service.permission.SubjectData; -import java.util.AbstractMap; import java.util.Map; import java.util.Set; -import java.util.concurrent.ConcurrentHashMap; import java.util.stream.Collectors; @AllArgsConstructor public class GroupCollection implements SubjectCollection { private final LuckPermsService service; private final GroupManager manager; - private final Set cache = ConcurrentHashMap.newKeySet(); @Override public String getIdentifier() { @@ -55,16 +51,7 @@ public class GroupCollection implements SubjectCollection { @Override public Subject get(@NonNull String id) { if (manager.isLoaded(id)) { - PermissionHolder holder = manager.get(id); - for (LuckPermsSubject subject : cache) { - if (subject.getHolder().getObjectName().equalsIgnoreCase(holder.getObjectName())) { - return subject; - } - } - - LuckPermsSubject subject = new LuckPermsSubject(manager.get(id), service); - cache.add(subject); - return subject; + return LuckPermsSubject.wrapHolder(manager.get(id), service); } return new SimpleSubject(id, service, this); @@ -78,7 +65,7 @@ public class GroupCollection implements SubjectCollection { @Override public Iterable getAllSubjects() { return manager.getAll().values().stream() - .map(u -> new LuckPermsSubject(u, service)) + .map(u -> LuckPermsSubject.wrapHolder(u, service)) .collect(Collectors.toList()); } @@ -90,10 +77,9 @@ public class GroupCollection implements SubjectCollection { @Override public Map getAllWithPermission(@NonNull Set contexts, @NonNull String node) { return manager.getAll().values().stream() - .map(u -> new LuckPermsSubject(u, service)) + .map(u -> LuckPermsSubject.wrapHolder(u, service)) .filter(sub -> sub.hasPermission(contexts, node)) - .map(sub -> new AbstractMap.SimpleEntry(sub, sub.getPermissionValue(contexts, node).asBoolean())) - .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); + .collect(Collectors.toMap(sub -> sub, sub -> sub.getPermissionValue(contexts, node).asBoolean())); } @Override diff --git a/sponge/src/main/java/me/lucko/luckperms/api/sponge/collections/UserCollection.java b/sponge/src/main/java/me/lucko/luckperms/api/sponge/collections/UserCollection.java index 08fb263aa..06f3ae744 100644 --- a/sponge/src/main/java/me/lucko/luckperms/api/sponge/collections/UserCollection.java +++ b/sponge/src/main/java/me/lucko/luckperms/api/sponge/collections/UserCollection.java @@ -23,7 +23,6 @@ package me.lucko.luckperms.api.sponge.collections; import lombok.AllArgsConstructor; -import lombok.Getter; import lombok.NonNull; import me.lucko.luckperms.api.sponge.LuckPermsService; import me.lucko.luckperms.api.sponge.LuckPermsSubject; @@ -37,11 +36,9 @@ import org.spongepowered.api.service.permission.Subject; import org.spongepowered.api.service.permission.SubjectCollection; import org.spongepowered.api.service.permission.SubjectData; -import java.util.AbstractMap; import java.util.Map; import java.util.Set; import java.util.UUID; -import java.util.concurrent.ConcurrentHashMap; import java.util.stream.Collectors; @AllArgsConstructor @@ -49,9 +46,6 @@ public class UserCollection implements SubjectCollection { private final LuckPermsService service; private final UserManager manager; - @Getter - private final Set cache = ConcurrentHashMap.newKeySet(); - @Override public String getIdentifier() { return PermissionService.SUBJECTS_USER; @@ -74,15 +68,7 @@ public class UserCollection implements SubjectCollection { } if (holder != null) { - for (LuckPermsSubject subject : cache) { - if (subject.getHolder().getObjectName().equalsIgnoreCase(holder.getObjectName())) { - return subject; - } - } - - LuckPermsSubject subject = new LuckPermsSubject(holder, service); - cache.add(subject); - return subject; + return LuckPermsSubject.wrapHolder(holder, service); } service.getPlugin().getLog().warn("Couldn't get subject for: " + id); @@ -106,7 +92,7 @@ public class UserCollection implements SubjectCollection { @Override public Iterable getAllSubjects() { return manager.getAll().values().stream() - .map(u -> new LuckPermsSubject(u, service)) + .map(u -> LuckPermsSubject.wrapHolder(u, service)) .collect(Collectors.toList()); } @@ -118,10 +104,9 @@ public class UserCollection implements SubjectCollection { @Override public Map getAllWithPermission(@NonNull Set contexts, @NonNull String node) { return manager.getAll().values().stream() - .map(u -> new LuckPermsSubject(u, service)) + .map(u -> LuckPermsSubject.wrapHolder(u, service)) .filter(sub -> sub.hasPermission(contexts, node)) - .map(sub -> new AbstractMap.SimpleEntry(sub, sub.getPermissionValue(contexts, node).asBoolean())) - .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); + .collect(Collectors.toMap(sub -> sub, sub -> sub.getPermissionValue(contexts, node).asBoolean())); } @Override diff --git a/sponge/src/main/java/me/lucko/luckperms/users/SpongeUserManager.java b/sponge/src/main/java/me/lucko/luckperms/users/SpongeUserManager.java index 227be20fc..964aaa818 100644 --- a/sponge/src/main/java/me/lucko/luckperms/users/SpongeUserManager.java +++ b/sponge/src/main/java/me/lucko/luckperms/users/SpongeUserManager.java @@ -34,16 +34,9 @@ public class SpongeUserManager extends UserManager { this.plugin = plugin; } - @Override - public void unload(User user) { - if (user != null) { - getAll().remove(user.getUuid()); - } - } - @Override public void cleanup(User user) { - if (plugin.getGame().getServer().getPlayer(plugin.getUuidCache().getExternalUUID(user.getUuid())).isPresent()) { + if (!plugin.getGame().getServer().getPlayer(plugin.getUuidCache().getExternalUUID(user.getUuid())).isPresent()) { unload(user); } }