From a0be1c7c48134e501032e34ccac3c93d27d9c54b Mon Sep 17 00:00:00 2001 From: Luck Date: Sun, 13 May 2018 14:06:05 +0100 Subject: [PATCH] Fix issue with Bukkit attachment permissions never being removed (#991) --- .../java/me/lucko/luckperms/api/LocalizedNode.java | 14 ++++++++++++++ .../model/permissible/LPPermissionAttachment.java | 6 ++++-- .../me/lucko/luckperms/common/model/NodeMap.java | 10 +++++----- .../luckperms/common/model/PermissionHolder.java | 12 ++++++------ .../common/node/model/ImmutableLocalizedNode.java | 5 ----- .../model/permissible/LPPermissionAttachment.java | 5 +++-- 6 files changed, 32 insertions(+), 20 deletions(-) diff --git a/api/src/main/java/me/lucko/luckperms/api/LocalizedNode.java b/api/src/main/java/me/lucko/luckperms/api/LocalizedNode.java index b6c3cab4..15f15570 100644 --- a/api/src/main/java/me/lucko/luckperms/api/LocalizedNode.java +++ b/api/src/main/java/me/lucko/luckperms/api/LocalizedNode.java @@ -25,6 +25,8 @@ package me.lucko.luckperms.api; +import java.util.function.Predicate; + import javax.annotation.Nonnull; import javax.annotation.concurrent.Immutable; @@ -36,6 +38,18 @@ import javax.annotation.concurrent.Immutable; @Immutable public interface LocalizedNode extends Node { + /** + * Returns a predicate which unwraps the localised node parameter before delegating + * the handling to the provided predicate. + * + * @param delegate the delegate predicate. + * @return the composed predicate + * @since 4.3 + */ + static Predicate composedPredicate(Predicate delegate) { + return localizedNode -> delegate.test(localizedNode.getNode()); + } + /** * Gets the delegate node * diff --git a/bukkit/src/main/java/me/lucko/luckperms/bukkit/model/permissible/LPPermissionAttachment.java b/bukkit/src/main/java/me/lucko/luckperms/bukkit/model/permissible/LPPermissionAttachment.java index 5331f9d6..45fe09d8 100644 --- a/bukkit/src/main/java/me/lucko/luckperms/bukkit/model/permissible/LPPermissionAttachment.java +++ b/bukkit/src/main/java/me/lucko/luckperms/bukkit/model/permissible/LPPermissionAttachment.java @@ -27,6 +27,7 @@ package me.lucko.luckperms.bukkit.model.permissible; import com.google.common.base.Preconditions; +import me.lucko.luckperms.api.LocalizedNode; import me.lucko.luckperms.api.Node; import me.lucko.luckperms.bukkit.model.dummy.DummyPlugin; import me.lucko.luckperms.common.config.ConfigKeys; @@ -188,13 +189,14 @@ public class LPPermissionAttachment extends PermissionAttachment { // remove transient permissions from the holder which were added by this attachment & equal the permission User user = this.permissible.getUser(); - user.removeIfTransient(n -> n instanceof ImmutableTransientNode && ((ImmutableTransientNode) n).getOwner() == this && n.getPermission().equals(name)); + + user.removeIfTransient(LocalizedNode.composedPredicate(n -> n instanceof ImmutableTransientNode && ((ImmutableTransientNode) n).getOwner() == this && n.getPermission().equals(name))); } private void clearInternal() { // remove all transient permissions added by this attachment User user = this.permissible.getUser(); - user.removeIfTransient(n -> n instanceof ImmutableTransientNode && ((ImmutableTransientNode) n).getOwner() == this); + user.removeIfTransient(LocalizedNode.composedPredicate(n -> n instanceof ImmutableTransientNode && ((ImmutableTransientNode) n).getOwner() == this)); } @Override diff --git a/common/src/main/java/me/lucko/luckperms/common/model/NodeMap.java b/common/src/main/java/me/lucko/luckperms/common/model/NodeMap.java index 887c5fb0..2996c335 100644 --- a/common/src/main/java/me/lucko/luckperms/common/model/NodeMap.java +++ b/common/src/main/java/me/lucko/luckperms/common/model/NodeMap.java @@ -227,13 +227,13 @@ public final class NodeMap { setContent(multimap.values()); } - boolean removeIf(Predicate predicate) { + boolean removeIf(Predicate predicate) { boolean ret = this.map.values().removeIf(predicate); this.inheritanceMap.values().removeIf(predicate); return ret; } - boolean removeIf(ContextSet contextSet, Predicate predicate) { + boolean removeIf(ContextSet contextSet, Predicate predicate) { ImmutableContextSet context = contextSet.makeImmutable(); SortedSet nodes = this.map.get(context); boolean ret = nodes.removeIf(predicate); @@ -241,12 +241,12 @@ public final class NodeMap { return ret; } - boolean auditTemporaryNodes(@Nullable Set removed) { + boolean auditTemporaryNodes(@Nullable Set removed) { boolean work = false; - Iterator it = this.map.values().iterator(); + Iterator it = this.map.values().iterator(); while (it.hasNext()) { - Node entry = it.next(); + LocalizedNode entry = it.next(); if (entry.hasExpired()) { if (removed != null) { removed.add(entry); diff --git a/common/src/main/java/me/lucko/luckperms/common/model/PermissionHolder.java b/common/src/main/java/me/lucko/luckperms/common/model/PermissionHolder.java index 1beea55e..84ef2490 100644 --- a/common/src/main/java/me/lucko/luckperms/common/model/PermissionHolder.java +++ b/common/src/main/java/me/lucko/luckperms/common/model/PermissionHolder.java @@ -261,11 +261,11 @@ public abstract class PermissionHolder { return ret; } - public boolean removeIf(Predicate predicate) { + public boolean removeIf(Predicate predicate) { return removeIf(predicate, null); } - public boolean removeIf(Predicate predicate, Runnable taskIfSuccess) { + public boolean removeIf(Predicate predicate, Runnable taskIfSuccess) { ImmutableCollection before = enduringData().immutable().values(); if (!this.enduringNodes.removeIf(predicate)) { return false; @@ -280,11 +280,11 @@ public abstract class PermissionHolder { return true; } - public boolean removeIf(ContextSet contextSet, Predicate predicate) { + public boolean removeIf(ContextSet contextSet, Predicate predicate) { return removeIf(contextSet, predicate, null); } - public boolean removeIf(ContextSet contextSet, Predicate predicate, Runnable taskIfSuccess) { + public boolean removeIf(ContextSet contextSet, Predicate predicate, Runnable taskIfSuccess) { ImmutableCollection before = enduringData().immutable().values(); if (!this.enduringNodes.removeIf(contextSet, predicate)) { return false; @@ -299,7 +299,7 @@ public abstract class PermissionHolder { return true; } - public boolean removeIfTransient(Predicate predicate) { + public boolean removeIfTransient(Predicate predicate) { boolean result = this.transientNodes.removeIf(predicate); if (result) { invalidateCache(); @@ -455,7 +455,7 @@ public abstract class PermissionHolder { // we don't call events for transient nodes boolean transientWork = this.transientNodes.auditTemporaryNodes(null); - ImmutableCollection before = enduringData().immutable().values(); + ImmutableCollection before = enduringData().immutable().values(); Set removed = new HashSet<>(); boolean enduringWork = this.enduringNodes.auditTemporaryNodes(removed); diff --git a/common/src/main/java/me/lucko/luckperms/common/node/model/ImmutableLocalizedNode.java b/common/src/main/java/me/lucko/luckperms/common/node/model/ImmutableLocalizedNode.java index 7468db2f..0810dee8 100644 --- a/common/src/main/java/me/lucko/luckperms/common/node/model/ImmutableLocalizedNode.java +++ b/common/src/main/java/me/lucko/luckperms/common/node/model/ImmutableLocalizedNode.java @@ -40,11 +40,6 @@ public final class ImmutableLocalizedNode extends ForwardingNode implements Loca Objects.requireNonNull(node, "node"); Objects.requireNonNull(location, "location"); - // unwrap - while (node instanceof LocalizedNode) { - node = ((LocalizedNode) node).getNode(); - } - return new ImmutableLocalizedNode(node, location); } diff --git a/nukkit/src/main/java/me/lucko/luckperms/nukkit/model/permissible/LPPermissionAttachment.java b/nukkit/src/main/java/me/lucko/luckperms/nukkit/model/permissible/LPPermissionAttachment.java index 7308f58b..6cf4b8ef 100644 --- a/nukkit/src/main/java/me/lucko/luckperms/nukkit/model/permissible/LPPermissionAttachment.java +++ b/nukkit/src/main/java/me/lucko/luckperms/nukkit/model/permissible/LPPermissionAttachment.java @@ -27,6 +27,7 @@ package me.lucko.luckperms.nukkit.model.permissible; import com.google.common.base.Preconditions; +import me.lucko.luckperms.api.LocalizedNode; import me.lucko.luckperms.api.Node; import me.lucko.luckperms.common.config.ConfigKeys; import me.lucko.luckperms.common.model.User; @@ -189,13 +190,13 @@ public class LPPermissionAttachment extends PermissionAttachment { // remove transient permissions from the holder which were added by this attachment & equal the permission User user = this.permissible.getUser(); - user.removeIfTransient(n -> n instanceof ImmutableTransientNode && ((ImmutableTransientNode) n).getOwner() == this && n.getPermission().equals(name)); + user.removeIfTransient(LocalizedNode.composedPredicate(n -> n instanceof ImmutableTransientNode && ((ImmutableTransientNode) n).getOwner() == this && n.getPermission().equals(name))); } private void clearInternal() { // remove all transient permissions added by this attachment User user = this.permissible.getUser(); - user.removeIfTransient(n -> n instanceof ImmutableTransientNode && ((ImmutableTransientNode) n).getOwner() == this); + user.removeIfTransient(LocalizedNode.composedPredicate(n -> n instanceof ImmutableTransientNode && ((ImmutableTransientNode) n).getOwner() == this)); } @Override