From 2dbbea4993c3ce03a5d3637e0f6f9b8ac8f786d6 Mon Sep 17 00:00:00 2001 From: Luck Date: Fri, 4 May 2018 16:16:12 +0100 Subject: [PATCH] Remove the locks in NodeMap to ease thread contention when lots of processes are resolving inheritance & refactor the way LocalizedNodes are created (#734) --- .../java/me/lucko/luckperms/api/Node.java | 2 +- .../luckperms/api/StandardNodeEquality.java | 6 +- .../delegates/model/ApiPermissionHolder.java | 25 +- .../common/caching/AbstractCachedData.java | 4 +- .../common/caching/HolderCachedData.java | 5 +- .../parent/UserSwitchPrimaryGroup.java | 8 +- .../common/commands/log/LogNotify.java | 2 +- .../luckperms/common/event/EventFactory.java | 6 +- .../inheritance/InheritanceHandler.java | 4 +- .../lucko/luckperms/common/model/NodeMap.java | 292 ++++++------------ .../common/model/PermissionHolder.java | 173 +++++------ .../node/model/ImmutableLocalizedNode.java | 6 + .../service/internal/HolderSubjectData.java | 6 +- 13 files changed, 219 insertions(+), 320 deletions(-) diff --git a/api/src/main/java/me/lucko/luckperms/api/Node.java b/api/src/main/java/me/lucko/luckperms/api/Node.java index ad277631a..7963b4e9c 100644 --- a/api/src/main/java/me/lucko/luckperms/api/Node.java +++ b/api/src/main/java/me/lucko/luckperms/api/Node.java @@ -556,7 +556,7 @@ public interface Node { * * @param value the value * @return the builder - * @see Node#getValuePrimitive() + * @see Node#getValue() */ @Nonnull Builder setValue(boolean value); diff --git a/api/src/main/java/me/lucko/luckperms/api/StandardNodeEquality.java b/api/src/main/java/me/lucko/luckperms/api/StandardNodeEquality.java index b84b32abc..b389802fc 100644 --- a/api/src/main/java/me/lucko/luckperms/api/StandardNodeEquality.java +++ b/api/src/main/java/me/lucko/luckperms/api/StandardNodeEquality.java @@ -44,7 +44,7 @@ public enum StandardNodeEquality implements NodeEqualityPredicate { /** * All attributes must match, except for - * {@link Node#getValuePrimitive() value}, which is ignored. + * {@link Node#getValue() value}, which is ignored. */ IGNORE_VALUE, @@ -59,14 +59,14 @@ public enum StandardNodeEquality implements NodeEqualityPredicate { /** * All attributes must match, except for - * {@link Node#getValuePrimitive() value} and the + * {@link Node#getValue() value} and the * {@link Node#getExpiry() expiry time}, which are ignored. */ IGNORE_EXPIRY_TIME_AND_VALUE, /** * All attributes must match, except for - * {@link Node#getValuePrimitive() value} and the if the node is + * {@link Node#getValue() value} and the if the node is * {@link Node#isTemporary() temporary}, which are ignored. */ IGNORE_VALUE_OR_IF_TEMPORARY; diff --git a/common/src/main/java/me/lucko/luckperms/common/api/delegates/model/ApiPermissionHolder.java b/common/src/main/java/me/lucko/luckperms/common/api/delegates/model/ApiPermissionHolder.java index 00e18a6eb..3176ef319 100644 --- a/common/src/main/java/me/lucko/luckperms/common/api/delegates/model/ApiPermissionHolder.java +++ b/common/src/main/java/me/lucko/luckperms/common/api/delegates/model/ApiPermissionHolder.java @@ -46,6 +46,7 @@ import me.lucko.luckperms.common.model.NodeMapType; import me.lucko.luckperms.common.model.PermissionHolder; import me.lucko.luckperms.common.model.User; import me.lucko.luckperms.common.node.comparator.NodeWithContextComparator; +import me.lucko.luckperms.common.node.factory.NodeFactory; import me.lucko.luckperms.common.node.utils.MetaType; import me.lucko.luckperms.common.node.utils.NodeTools; import me.lucko.luckperms.common.utils.ImmutableCollectors; @@ -104,13 +105,15 @@ public class ApiPermissionHolder implements me.lucko.luckperms.api.PermissionHol @Nonnull @Override public ImmutableSetMultimap getNodes() { - return this.handle.enduringData().immutable(); + //noinspection unchecked + return (ImmutableSetMultimap) this.handle.enduringData().immutable(); } @Nonnull @Override public ImmutableSetMultimap getTransientNodes() { - return this.handle.transientData().immutable(); + //noinspection unchecked + return (ImmutableSetMultimap) this.handle.transientData().immutable(); } @Nonnull @@ -183,7 +186,7 @@ public class ApiPermissionHolder implements me.lucko.luckperms.api.PermissionHol @Override public Map exportNodes(@Nonnull Contexts contexts, boolean lowerCase) { Objects.requireNonNull(contexts, "contexts"); - return ImmutableMap.copyOf(this.handle.exportNodesAndShorthand(contexts, lowerCase)); + return ImmutableMap.copyOf(this.handle.exportPermissions(contexts, lowerCase, true)); } @Nonnull @@ -234,14 +237,26 @@ public class ApiPermissionHolder implements me.lucko.luckperms.api.PermissionHol @Override public boolean inheritsGroup(@Nonnull me.lucko.luckperms.api.Group group) { Objects.requireNonNull(group, "group"); - return this.handle.inheritsGroup(ApiGroup.cast(group)); + + Group g = ApiGroup.cast(group); + if (this.handle.getType().isGroup() && g.getName().equals(this.handle.getObjectName())) { + return true; + } + + return this.handle.hasPermission(NodeMapType.ENDURING, NodeFactory.buildGroupNode(g.getName()).build(), StandardNodeEquality.IGNORE_EXPIRY_TIME_AND_VALUE).asBoolean(); } @Override public boolean inheritsGroup(@Nonnull me.lucko.luckperms.api.Group group, @Nonnull ContextSet contextSet) { Objects.requireNonNull(group, "group"); Objects.requireNonNull(contextSet, "contextSet"); - return this.handle.inheritsGroup(ApiGroup.cast(group), contextSet); + + Group g = ApiGroup.cast(group); + if (this.handle.getType().isGroup() && g.getName().equals(this.handle.getObjectName())) { + return true; + } + + return this.handle.hasPermission(NodeMapType.ENDURING, NodeFactory.buildGroupNode(g.getName()).withExtraContext(contextSet).build(), StandardNodeEquality.IGNORE_EXPIRY_TIME_AND_VALUE).asBoolean(); } @Nonnull diff --git a/common/src/main/java/me/lucko/luckperms/common/caching/AbstractCachedData.java b/common/src/main/java/me/lucko/luckperms/common/caching/AbstractCachedData.java index c1d3b1c2b..27eb9ae77 100644 --- a/common/src/main/java/me/lucko/luckperms/common/caching/AbstractCachedData.java +++ b/common/src/main/java/me/lucko/luckperms/common/caching/AbstractCachedData.java @@ -190,7 +190,7 @@ public abstract class AbstractCachedData implements CachedData { Objects.requireNonNull(contexts, "contexts"); //noinspection ConstantConditions - return this.permission.get(contexts).join(); + return this.permission.synchronous().get(contexts); } @Nonnull @@ -199,7 +199,7 @@ public abstract class AbstractCachedData implements CachedData { Objects.requireNonNull(contexts, "contexts"); //noinspection ConstantConditions - return this.meta.get(contexts).join(); + return this.meta.synchronous().get(contexts); } @Nonnull diff --git a/common/src/main/java/me/lucko/luckperms/common/caching/HolderCachedData.java b/common/src/main/java/me/lucko/luckperms/common/caching/HolderCachedData.java index f0c0e372b..856051838 100644 --- a/common/src/main/java/me/lucko/luckperms/common/caching/HolderCachedData.java +++ b/common/src/main/java/me/lucko/luckperms/common/caching/HolderCachedData.java @@ -29,6 +29,7 @@ import me.lucko.luckperms.api.Contexts; import me.lucko.luckperms.api.caching.MetaContexts; import me.lucko.luckperms.common.caching.type.MetaAccumulator; import me.lucko.luckperms.common.calculators.CalculatorFactory; +import me.lucko.luckperms.common.config.ConfigKeys; import me.lucko.luckperms.common.model.PermissionHolder; import java.util.Map; @@ -60,12 +61,12 @@ public abstract class HolderCachedData extends Abstr @Override protected Map resolvePermissions() { - return this.holder.exportNodesAndShorthand(true); + return this.holder.exportPermissions(true, this.plugin.getConfiguration().get(ConfigKeys.APPLYING_SHORTHAND)); } @Override protected Map resolvePermissions(Contexts contexts) { - return this.holder.exportNodesAndShorthand(contexts, true); + return this.holder.exportPermissions(contexts, true, this.plugin.getConfiguration().get(ConfigKeys.APPLYING_SHORTHAND)); } @Override diff --git a/common/src/main/java/me/lucko/luckperms/common/commands/generic/parent/UserSwitchPrimaryGroup.java b/common/src/main/java/me/lucko/luckperms/common/commands/generic/parent/UserSwitchPrimaryGroup.java index 0b4854a11..b23ff27ad 100644 --- a/common/src/main/java/me/lucko/luckperms/common/commands/generic/parent/UserSwitchPrimaryGroup.java +++ b/common/src/main/java/me/lucko/luckperms/common/commands/generic/parent/UserSwitchPrimaryGroup.java @@ -25,6 +25,8 @@ package me.lucko.luckperms.common.commands.generic.parent; +import me.lucko.luckperms.api.Node; +import me.lucko.luckperms.api.StandardNodeEquality; import me.lucko.luckperms.common.actionlog.ExtendedLogEntry; import me.lucko.luckperms.common.command.CommandResult; import me.lucko.luckperms.common.command.abstraction.SharedSubCommand; @@ -36,6 +38,7 @@ import me.lucko.luckperms.common.locale.LocaleManager; import me.lucko.luckperms.common.locale.command.CommandSpec; import me.lucko.luckperms.common.locale.message.Message; import me.lucko.luckperms.common.model.Group; +import me.lucko.luckperms.common.model.NodeMapType; import me.lucko.luckperms.common.model.PermissionHolder; import me.lucko.luckperms.common.model.User; import me.lucko.luckperms.common.node.factory.NodeFactory; @@ -80,9 +83,10 @@ public class UserSwitchPrimaryGroup extends SharedSubCommand { return CommandResult.STATE_ERROR; } - if (!user.inheritsGroup(group)) { + Node node = NodeFactory.buildGroupNode(group.getName()).build(); + if (!user.hasPermission(NodeMapType.ENDURING, node, StandardNodeEquality.IGNORE_VALUE).asBoolean()) { Message.USER_PRIMARYGROUP_ERROR_NOTMEMBER.send(sender, user.getFriendlyName(), group.getName()); - user.setPermission(NodeFactory.buildGroupNode(group.getName()).build()); + user.setPermission(node); } user.getPrimaryGroup().setStoredValue(group.getName()); diff --git a/common/src/main/java/me/lucko/luckperms/common/commands/log/LogNotify.java b/common/src/main/java/me/lucko/luckperms/common/commands/log/LogNotify.java index 15e2f7949..b7757b0c8 100644 --- a/common/src/main/java/me/lucko/luckperms/common/commands/log/LogNotify.java +++ b/common/src/main/java/me/lucko/luckperms/common/commands/log/LogNotify.java @@ -54,7 +54,7 @@ public class LogNotify extends SubCommand { return false; } - Optional ret = user.getOwnNodes().stream() + Optional ret = user.getOwnNodes().stream() .filter(n -> n.getPermission().equalsIgnoreCase("luckperms.log.notify.ignoring")) .findFirst(); diff --git a/common/src/main/java/me/lucko/luckperms/common/event/EventFactory.java b/common/src/main/java/me/lucko/luckperms/common/event/EventFactory.java index 7239f5487..638508783 100644 --- a/common/src/main/java/me/lucko/luckperms/common/event/EventFactory.java +++ b/common/src/main/java/me/lucko/luckperms/common/event/EventFactory.java @@ -170,17 +170,17 @@ public final class EventFactory { fireEventAsync(event); } - public void handleNodeAdd(Node node, PermissionHolder target, Collection before, Collection after) { + public void handleNodeAdd(Node node, PermissionHolder target, Collection before, Collection after) { EventNodeAdd event = new EventNodeAdd(node, getDelegate(target), ImmutableSet.copyOf(before), ImmutableSet.copyOf(after)); fireEventAsync(event); } - public void handleNodeClear(PermissionHolder target, Collection before, Collection after) { + public void handleNodeClear(PermissionHolder target, Collection before, Collection after) { EventNodeClear event = new EventNodeClear(getDelegate(target), ImmutableSet.copyOf(before), ImmutableSet.copyOf(after)); fireEventAsync(event); } - public void handleNodeRemove(Node node, PermissionHolder target, Collection before, Collection after) { + public void handleNodeRemove(Node node, PermissionHolder target, Collection before, Collection after) { EventNodeRemove event = new EventNodeRemove(node, getDelegate(target), ImmutableSet.copyOf(before), ImmutableSet.copyOf(after)); fireEventAsync(event); } diff --git a/common/src/main/java/me/lucko/luckperms/common/inheritance/InheritanceHandler.java b/common/src/main/java/me/lucko/luckperms/common/inheritance/InheritanceHandler.java index 42ed23fd3..4588c955f 100644 --- a/common/src/main/java/me/lucko/luckperms/common/inheritance/InheritanceHandler.java +++ b/common/src/main/java/me/lucko/luckperms/common/inheritance/InheritanceHandler.java @@ -82,7 +82,7 @@ public class InheritanceHandler { @Override public Iterable successors(PermissionHolder holder) { Set successors = new TreeSet<>(holder.getInheritanceComparator()); - List nodes = holder.getOwnGroupNodes(); + List nodes = holder.getOwnGroupNodes(); for (Node n : nodes) { Group g = this.plugin.getGroupManager().getIfLoaded(n.getGroupName()); if (g != null) { @@ -109,7 +109,7 @@ public class InheritanceHandler { @Override public Iterable successors(PermissionHolder holder) { Set successors = new TreeSet<>(holder.getInheritanceComparator()); - List nodes = holder.getOwnGroupNodes(this.context.getContexts()); + List nodes = holder.getOwnGroupNodes(this.context.getContexts()); for (Node n : nodes) { // effectively: if not (we're applying global groups or it's specific anyways) if (!((this.context.hasSetting(LookupSetting.APPLY_PARENTS_SET_WITHOUT_SERVER) || n.isServerSpecific()) && (this.context.hasSetting(LookupSetting.APPLY_PARENTS_SET_WITHOUT_WORLD) || n.isWorldSpecific()))) { 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 6bdd19b19..887c5fb00 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 @@ -25,9 +25,10 @@ package me.lucko.luckperms.common.model; +import com.google.common.base.Supplier; import com.google.common.collect.ImmutableSetMultimap; import com.google.common.collect.Multimap; -import com.google.common.collect.MultimapBuilder; +import com.google.common.collect.Multimaps; import com.google.common.collect.SortedSetMultimap; import me.lucko.luckperms.api.LocalizedNode; @@ -50,7 +51,8 @@ import java.util.Map; import java.util.Set; import java.util.SortedSet; import java.util.TreeSet; -import java.util.concurrent.locks.ReentrantLock; +import java.util.concurrent.ConcurrentSkipListMap; +import java.util.concurrent.ConcurrentSkipListSet; import java.util.function.Predicate; import javax.annotation.Nonnull; @@ -66,6 +68,8 @@ import javax.annotation.Nullable; *

Each holder has two of these maps, one for enduring and transient nodes.

*/ public final class NodeMap { + @SuppressWarnings("Guava") + private static final Supplier> VALUE_SET_SUPPLIER = () -> new ConcurrentSkipListSet<>(NodeComparator.reverse()); /** * The holder which this map is for @@ -80,24 +84,19 @@ public final class NodeMap { * key, and finally by the overall size of the set. Nodes are ordered according to the priority rules * defined in {@link NodeComparator}.

*/ - private final SortedSetMultimap map = MultimapBuilder - .treeKeys(ContextSetComparator.reverse()) - .treeSetValues(NodeComparator.reverse()) - .build(); + private final SortedSetMultimap map = Multimaps.newSortedSetMultimap( + new ConcurrentSkipListMap<>(ContextSetComparator.reverse()), + VALUE_SET_SUPPLIER + ); /** * Copy of {@link #map} which only contains group nodes * @see Node#isGroupNode() */ - private final SortedSetMultimap inheritanceMap = MultimapBuilder - .treeKeys(ContextSetComparator.reverse()) - .treeSetValues(NodeComparator.reverse()) - .build(); - - /** - * The lock which synchronizes the instance - */ - private final ReentrantLock lock = new ReentrantLock(); + private final SortedSetMultimap inheritanceMap = Multimaps.newSortedSetMultimap( + new ConcurrentSkipListMap<>(ContextSetComparator.reverse()), + VALUE_SET_SUPPLIER + ); /** * A cache which holds an immutable copy of the backing map @@ -108,82 +107,41 @@ public final class NodeMap { this.holder = holder; } - public List asList() { - this.lock.lock(); - try { - return new ArrayList<>(this.map.values()); - } finally { - this.lock.unlock(); - } + public List asList() { + return new ArrayList<>(this.map.values()); } - public LinkedHashSet asSet() { - this.lock.lock(); - try { - return new LinkedHashSet<>(this.map.values()); - } finally { - this.lock.unlock(); - } + public LinkedHashSet asSet() { + return new LinkedHashSet<>(this.map.values()); } public SortedSet asSortedSet() { SortedSet ret = new TreeSet<>(NodeWithContextComparator.reverse()); - copyToLocalized(ret); + copyTo(ret); return ret; } - public void copyTo(Collection collection) { - this.lock.lock(); - try { - collection.addAll(this.map.values()); - } finally { - this.lock.unlock(); - } + public void copyTo(Collection collection) { + collection.addAll(this.map.values()); } - public void copyTo(Collection collection, ContextSet filter) { - this.lock.lock(); - try { - for (Map.Entry> e : this.map.asMap().entrySet()) { - if (e.getKey().isSatisfiedBy(filter)) { - collection.addAll(e.getValue()); - } + public void copyTo(Collection collection, ContextSet filter) { + for (Map.Entry> e : this.map.asMap().entrySet()) { + if (e.getKey().isSatisfiedBy(filter)) { + collection.addAll(e.getValue()); } - } finally { - this.lock.unlock(); } } - public void copyGroupNodesTo(Collection collection) { - this.lock.lock(); - try { - collection.addAll(this.inheritanceMap.values()); - } finally { - this.lock.unlock(); - } + public void copyGroupNodesTo(Collection collection) { + collection.addAll(this.inheritanceMap.values()); } - public void copyGroupNodesTo(Collection collection, ContextSet filter) { - this.lock.lock(); - try { - for (Map.Entry> e : this.inheritanceMap.asMap().entrySet()) { - if (e.getKey().isSatisfiedBy(filter)) { - collection.addAll(e.getValue()); - } + public void copyGroupNodesTo(Collection collection, ContextSet filter) { + for (Map.Entry> e : this.inheritanceMap.asMap().entrySet()) { + if (e.getKey().isSatisfiedBy(filter)) { + collection.addAll(e.getValue()); } - } finally { - this.lock.unlock(); - } - } - - public void copyToLocalized(Collection collection) { - this.lock.lock(); - try { - for (Node node : this.map.values()) { - collection.add(ImmutableLocalizedNode.of(node, this.holder.getObjectName())); - } - } finally { - this.lock.unlock(); } } @@ -192,7 +150,7 @@ public final class NodeMap { * * @return an immutable copy */ - public ImmutableSetMultimap immutable() { + public ImmutableSetMultimap immutable() { return this.cache.get(); } @@ -203,161 +161,108 @@ public final class NodeMap { this.cache.invalidate(); } - void add(Node node) { - this.lock.lock(); - try { - ImmutableContextSet context = node.getFullContexts().makeImmutable(); - this.map.put(context, node); - if (node.isGroupNode() && node.getValue()) { - this.inheritanceMap.put(context, node); + private LocalizedNode localise(Node node) { + if (node instanceof LocalizedNode) { + LocalizedNode localizedNode = (LocalizedNode) node; + if (this.holder.getObjectName().equals(localizedNode.getLocation())) { + return localizedNode; } - } finally { - this.lock.unlock(); + } + + // localise + return ImmutableLocalizedNode.of(node, this.holder.getObjectName()); + } + + void add(Node node) { + ImmutableContextSet context = node.getFullContexts().makeImmutable(); + LocalizedNode n = localise(node); + + this.map.put(context, n); + if (node.isGroupNode() && node.getValue()) { + this.inheritanceMap.put(context, n); } } void remove(Node node) { - this.lock.lock(); - try { - ImmutableContextSet context = node.getFullContexts().makeImmutable(); - this.map.get(context).removeIf(e -> e.equals(node, StandardNodeEquality.IGNORE_EXPIRY_TIME_AND_VALUE)); - if (node.isGroupNode()) { - this.inheritanceMap.get(context).removeIf(e -> e.equals(node, StandardNodeEquality.IGNORE_EXPIRY_TIME_AND_VALUE)); - } - } finally { - this.lock.unlock(); + ImmutableContextSet context = node.getFullContexts().makeImmutable(); + this.map.get(context).removeIf(e -> e.equals(node, StandardNodeEquality.IGNORE_EXPIRY_TIME_AND_VALUE)); + if (node.isGroupNode()) { + this.inheritanceMap.get(context).removeIf(e -> e.equals(node, StandardNodeEquality.IGNORE_EXPIRY_TIME_AND_VALUE)); } } private void removeExact(Node node) { - this.lock.lock(); - try { - ImmutableContextSet context = node.getFullContexts().makeImmutable(); - this.map.remove(context, node); - if (node.isGroupNode() && node.getValue()) { - this.inheritanceMap.remove(context, node); - } - } finally { - this.lock.unlock(); + ImmutableContextSet context = node.getFullContexts().makeImmutable(); + this.map.remove(context, node); + if (node.isGroupNode() && node.getValue()) { + this.inheritanceMap.remove(context, node); } } void replace(Node node, Node previous) { - this.lock.lock(); - try { - removeExact(previous); - add(node); - } finally { - this.lock.unlock(); - } + removeExact(previous); + add(node); } void clear() { - this.lock.lock(); - try { - this.map.clear(); - this.inheritanceMap.clear(); - } finally { - this.lock.unlock(); - } + this.map.clear(); + this.inheritanceMap.clear(); } void clear(ContextSet contextSet) { - this.lock.lock(); - try { - ImmutableContextSet context = contextSet.makeImmutable(); - this.map.removeAll(context); - this.inheritanceMap.removeAll(context); - } finally { - this.lock.unlock(); + ImmutableContextSet context = contextSet.makeImmutable(); + this.map.removeAll(context); + this.inheritanceMap.removeAll(context); + } + + void setContent(Collection set) { + this.map.clear(); + this.inheritanceMap.clear(); + for (Node n : set) { + add(n); } } - void setContent(Set set) { - this.lock.lock(); - try { - this.map.clear(); - this.inheritanceMap.clear(); - for (Node n : set) { - add(n); - } - } finally { - this.lock.unlock(); - } - } - - void setContent(Multimap multimap) { - this.lock.lock(); - try { - this.map.clear(); - this.inheritanceMap.clear(); - - this.map.putAll(multimap); - for (Map.Entry entry : this.map.entries()) { - if (entry.getValue().isGroupNode() && entry.getValue().getValue()) { - this.inheritanceMap.put(entry.getKey(), entry.getValue()); - } - } - } finally { - this.lock.unlock(); - } + void setContent(Multimap multimap) { + setContent(multimap.values()); } boolean removeIf(Predicate predicate) { - this.lock.lock(); - try { - boolean ret = this.map.values().removeIf(predicate); - if (ret) { - this.inheritanceMap.values().removeIf(predicate); - } - return ret; - } finally { - this.lock.unlock(); - } + boolean ret = this.map.values().removeIf(predicate); + this.inheritanceMap.values().removeIf(predicate); + return ret; } boolean removeIf(ContextSet contextSet, Predicate predicate) { - this.lock.lock(); - try { - ImmutableContextSet context = contextSet.makeImmutable(); - SortedSet nodes = this.map.get(context); - boolean ret = nodes.removeIf(predicate); - if (ret) { - this.inheritanceMap.get(context).removeIf(predicate); - } - return ret; - } finally { - this.lock.unlock(); - } + ImmutableContextSet context = contextSet.makeImmutable(); + SortedSet nodes = this.map.get(context); + boolean ret = nodes.removeIf(predicate); + this.inheritanceMap.get(context).removeIf(predicate); + return ret; } boolean auditTemporaryNodes(@Nullable Set removed) { boolean work = false; - this.lock.lock(); - try { - Iterator it = this.map.values().iterator(); - while (it.hasNext()) { - Node entry = it.next(); - if (entry.hasExpired()) { - if (removed != null) { - removed.add(entry); - } - if (entry.isGroupNode() && entry.getValue()) { - this.inheritanceMap.remove(entry.getFullContexts().makeImmutable(), entry); - } - work = true; - it.remove(); + Iterator it = this.map.values().iterator(); + while (it.hasNext()) { + Node entry = it.next(); + if (entry.hasExpired()) { + if (removed != null) { + removed.add(entry); } + if (entry.isGroupNode() && entry.getValue()) { + this.inheritanceMap.remove(entry.getFullContexts().makeImmutable(), entry); + } + work = true; + it.remove(); } - } finally { - this.lock.unlock(); } return work; } - private static final class NodeMapCache extends Cache> { + private static final class NodeMapCache extends Cache> { private final NodeMap handle; private NodeMapCache(NodeMap handle) { @@ -366,13 +271,8 @@ public final class NodeMap { @Nonnull @Override - protected ImmutableSetMultimap supply() { - this.handle.lock.lock(); - try { - return ImmutableSetMultimap.copyOf(this.handle.map); - } finally { - this.handle.lock.unlock(); - } + protected ImmutableSetMultimap supply() { + return ImmutableSetMultimap.copyOf(this.handle.map); } } 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 bd7bd3795..7229caa2b 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 @@ -46,8 +46,6 @@ import me.lucko.luckperms.common.config.ConfigKeys; import me.lucko.luckperms.common.inheritance.InheritanceComparator; import me.lucko.luckperms.common.inheritance.InheritanceGraph; import me.lucko.luckperms.common.node.comparator.NodeWithContextComparator; -import me.lucko.luckperms.common.node.factory.NodeFactory; -import me.lucko.luckperms.common.node.model.ImmutableLocalizedNode; import me.lucko.luckperms.common.node.utils.InheritanceInfo; import me.lucko.luckperms.common.node.utils.MetaType; import me.lucko.luckperms.common.node.utils.NodeTools; @@ -218,39 +216,39 @@ public abstract class PermissionHolder { reloadCachedData(); } - public void setNodes(NodeMapType type, Set set) { + public void setNodes(NodeMapType type, Set set) { getData(type).setContent(set); invalidateCache(); } - public void replaceNodes(NodeMapType type, Multimap multimap) { + public void replaceNodes(NodeMapType type, Multimap multimap) { getData(type).setContent(multimap); invalidateCache(); } - public List getOwnNodes() { - List ret = new ArrayList<>(); + public List getOwnNodes() { + List ret = new ArrayList<>(); this.transientNodes.copyTo(ret); this.enduringNodes.copyTo(ret); return ret; } - public List getOwnNodes(ContextSet filter) { - List ret = new ArrayList<>(); + public List getOwnNodes(ContextSet filter) { + List ret = new ArrayList<>(); this.transientNodes.copyTo(ret, filter); this.enduringNodes.copyTo(ret, filter); return ret; } - public List getOwnGroupNodes() { - List ret = new ArrayList<>(); + public List getOwnGroupNodes() { + List ret = new ArrayList<>(); this.transientNodes.copyGroupNodesTo(ret); this.enduringNodes.copyGroupNodesTo(ret); return ret; } - public List getOwnGroupNodes(ContextSet filter) { - List ret = new ArrayList<>(); + public List getOwnGroupNodes(ContextSet filter) { + List ret = new ArrayList<>(); this.transientNodes.copyGroupNodesTo(ret, filter); this.enduringNodes.copyGroupNodesTo(ret, filter); return ret; @@ -258,17 +256,17 @@ public abstract class PermissionHolder { public SortedSet getOwnNodesSorted() { SortedSet ret = new TreeSet<>(NodeWithContextComparator.reverse()); - this.transientNodes.copyToLocalized(ret); - this.enduringNodes.copyToLocalized(ret); + this.transientNodes.copyTo(ret); + this.enduringNodes.copyTo(ret); return ret; } - public boolean removeIf(Predicate predicate) { + public boolean removeIf(Predicate predicate) { return removeIf(predicate, null); } - public boolean removeIf(Predicate predicate, Runnable taskIfSuccess) { - ImmutableCollection before = enduringData().immutable().values(); + public boolean removeIf(Predicate predicate, Runnable taskIfSuccess) { + ImmutableCollection before = enduringData().immutable().values(); if (!this.enduringNodes.removeIf(predicate)) { return false; } @@ -276,18 +274,18 @@ public abstract class PermissionHolder { taskIfSuccess.run(); } invalidateCache(); - ImmutableCollection after = enduringData().immutable().values(); + ImmutableCollection after = enduringData().immutable().values(); this.plugin.getEventFactory().handleNodeClear(this, before, after); 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) { - ImmutableCollection before = enduringData().immutable().values(); + public boolean removeIf(ContextSet contextSet, Predicate predicate, Runnable taskIfSuccess) { + ImmutableCollection before = enduringData().immutable().values(); if (!this.enduringNodes.removeIf(contextSet, predicate)) { return false; } @@ -295,13 +293,13 @@ public abstract class PermissionHolder { taskIfSuccess.run(); } invalidateCache(); - ImmutableCollection after = enduringData().immutable().values(); + ImmutableCollection after = enduringData().immutable().values(); this.plugin.getEventFactory().handleNodeClear(this, before, after); return true; } - public boolean removeIfTransient(Predicate predicate) { + public boolean removeIfTransient(Predicate predicate) { boolean result = this.transientNodes.removeIf(predicate); if (result) { invalidateCache(); @@ -309,15 +307,12 @@ public abstract class PermissionHolder { return result; } - public void accumulateInheritancesTo(List accumulator, Contexts context) { + public void accumulateInheritancesTo(List accumulator, Contexts context) { InheritanceGraph graph = this.plugin.getInheritanceHandler().getGraph(context); Iterable traversal = graph.traverse(this.plugin.getConfiguration().get(ConfigKeys.INHERITANCE_TRAVERSAL_ALGORITHM), this); for (PermissionHolder holder : traversal) { - List nodes = holder.getOwnNodes(context.getContexts()); - for (Node node : nodes) { - ImmutableLocalizedNode localizedNode = ImmutableLocalizedNode.of(node, holder.getObjectName()); - accumulator.add(localizedNode); - } + List nodes = holder.getOwnNodes(context.getContexts()); + accumulator.addAll(nodes); } } @@ -327,15 +322,12 @@ public abstract class PermissionHolder { return accumulator; } - public void accumulateInheritancesTo(List accumulator) { + public void accumulateInheritancesTo(List accumulator) { InheritanceGraph graph = this.plugin.getInheritanceHandler().getGraph(); Iterable traversal = graph.traverse(this.plugin.getConfiguration().get(ConfigKeys.INHERITANCE_TRAVERSAL_ALGORITHM), this); for (PermissionHolder holder : traversal) { - List nodes = holder.getOwnNodes(); - for (Node node : nodes) { - ImmutableLocalizedNode localizedNode = ImmutableLocalizedNode.of(node, holder.getObjectName()); - accumulator.add(localizedNode); - } + List nodes = holder.getOwnNodes(); + accumulator.addAll(nodes); } } @@ -350,10 +342,7 @@ public abstract class PermissionHolder { if (context.hasSetting(LookupSetting.RESOLVE_INHERITANCE)) { accumulateInheritancesTo(entries, context); } else { - for (Node n : getOwnNodes(context.getContexts())) { - ImmutableLocalizedNode localizedNode = ImmutableLocalizedNode.of(n, getObjectName()); - entries.add(localizedNode); - } + entries.addAll(getOwnNodes(context.getContexts())); } if (!context.hasSetting(LookupSetting.INCLUDE_NODES_SET_WITHOUT_SERVER)) { @@ -366,42 +355,34 @@ public abstract class PermissionHolder { return entries; } - public Map exportNodesAndShorthand(Contexts context, boolean lowerCase) { + public Map exportPermissions(Contexts context, boolean convertToLowercase, boolean resolveShorthand) { List entries = getAllEntries(context); + return processExportedPermissions(entries, convertToLowercase, resolveShorthand); + } - Map perms = new HashMap<>(); - boolean applyShorthand = this.plugin.getConfiguration().get(ConfigKeys.APPLYING_SHORTHAND); + public Map exportPermissions(boolean convertToLowercase, boolean resolveShorthand) { + List entries = resolveInheritances(); + return processExportedPermissions(entries, convertToLowercase, resolveShorthand); + } + + private static ImmutableMap processExportedPermissions(List entries, boolean convertToLowercase, boolean resolveShorthand) { + Map perms = new HashMap<>(entries.size()); for (Node node : entries) { - String perm = lowerCase ? node.getPermission().toLowerCase() : node.getPermission(); - - if (perms.putIfAbsent(perm, node.getValue()) == null) { - if (applyShorthand) { - List shorthand = node.resolveShorthand(); - if (!shorthand.isEmpty()) { - for (String s : shorthand) { - perms.putIfAbsent(lowerCase ? s.toLowerCase() : s, node.getValue()); - } - } - } + if (convertToLowercase) { + perms.putIfAbsent(node.getPermission().toLowerCase(), node.getValue()); + } else { + perms.putIfAbsent(node.getPermission(), node.getValue()); } } - return ImmutableMap.copyOf(perms); - } - - public Map exportNodesAndShorthand(boolean lowerCase) { - List entries = resolveInheritances(); - - Map perms = new HashMap<>(); - boolean applyShorthand = this.plugin.getConfiguration().get(ConfigKeys.APPLYING_SHORTHAND); - for (Node node : entries) { - String perm = lowerCase ? node.getPermission().toLowerCase().intern() : node.getPermission(); - - if (perms.putIfAbsent(perm, node.getValue()) == null && applyShorthand) { + if (resolveShorthand) { + for (Node node : entries) { List shorthand = node.resolveShorthand(); - if (!shorthand.isEmpty()) { - for (String s : shorthand) { - perms.putIfAbsent((lowerCase ? s.toLowerCase() : s).intern(), node.getValue()); + for (String s : shorthand) { + if (convertToLowercase) { + perms.putIfAbsent(s.toLowerCase(), node.getValue()); + } else { + perms.putIfAbsent(s, node.getValue()); } } } @@ -418,8 +399,8 @@ public abstract class PermissionHolder { InheritanceGraph graph = this.plugin.getInheritanceHandler().getGraph(context); Iterable traversal = graph.traverse(this.plugin.getConfiguration().get(ConfigKeys.INHERITANCE_TRAVERSAL_ALGORITHM), this); for (PermissionHolder holder : traversal) { - List nodes = holder.getOwnNodes(context.getContexts()); - for (Node node : nodes) { + List nodes = holder.getOwnNodes(context.getContexts()); + for (LocalizedNode node : nodes) { if (!node.getValue()) continue; if (!node.isMeta() && !node.isPrefix() && !node.isSuffix()) continue; @@ -427,7 +408,7 @@ public abstract class PermissionHolder { continue; } - accumulator.accumulateNode(ImmutableLocalizedNode.of(node, holder.getObjectName())); + accumulator.accumulateNode(node); } OptionalInt w = holder.getWeight(); @@ -447,12 +428,12 @@ public abstract class PermissionHolder { InheritanceGraph graph = this.plugin.getInheritanceHandler().getGraph(); Iterable traversal = graph.traverse(this.plugin.getConfiguration().get(ConfigKeys.INHERITANCE_TRAVERSAL_ALGORITHM), this); for (PermissionHolder holder : traversal) { - List nodes = holder.getOwnNodes(); - for (Node node : nodes) { + List nodes = holder.getOwnNodes(); + for (LocalizedNode node : nodes) { if (!node.getValue()) continue; if (!node.isMeta() && !node.isPrefix() && !node.isSuffix()) continue; - accumulator.accumulateNode(ImmutableLocalizedNode.of(node, holder.getObjectName())); + accumulator.accumulateNode(node); } OptionalInt w = getWeight(); @@ -474,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); @@ -483,7 +464,7 @@ public abstract class PermissionHolder { invalidateCache(); // call event - ImmutableCollection after = enduringData().immutable().values(); + ImmutableCollection after = enduringData().immutable().values(); for (Node r : removed) { this.plugin.getEventFactory().handleNodeRemove(r, this, before, after); } @@ -496,8 +477,8 @@ public abstract class PermissionHolder { return transientWork || enduringWork; } - private Optional searchForMatch(NodeMapType type, Node node, NodeEqualityPredicate equalityPredicate) { - for (Node n : getData(type).immutable().values()) { + private Optional searchForMatch(NodeMapType type, Node node, NodeEqualityPredicate equalityPredicate) { + for (LocalizedNode n : getData(type).immutable().values()) { if (n.equals(node, equalityPredicate)) { return Optional.of(n); } @@ -559,10 +540,10 @@ public abstract class PermissionHolder { return DataMutateResult.ALREADY_HAS; } - ImmutableCollection before = enduringData().immutable().values(); + ImmutableCollection before = enduringData().immutable().values(); this.enduringNodes.add(node); invalidateCache(); - ImmutableCollection after = enduringData().immutable().values(); + ImmutableCollection after = enduringData().immutable().values(); this.plugin.getEventFactory().handleNodeAdd(node, this, before, after); return DataMutateResult.SUCCESS; @@ -579,7 +560,7 @@ public abstract class PermissionHolder { if (node.isTemporary()) { if (modifier == TemporaryModifier.ACCUMULATE) { // Try to accumulate with an existing node - Optional existing = searchForMatch(NodeMapType.ENDURING, node, StandardNodeEquality.IGNORE_EXPIRY_TIME_AND_VALUE); + Optional existing = searchForMatch(NodeMapType.ENDURING, node, StandardNodeEquality.IGNORE_EXPIRY_TIME_AND_VALUE); // An existing node was found if (existing.isPresent()) { @@ -589,10 +570,10 @@ public abstract class PermissionHolder { Node newNode = node.toBuilder().setExpiry(previous.getExpiryUnixTime() + node.getSecondsTilExpiry()).build(); // Remove the old node & add the new one. - ImmutableCollection before = enduringData().immutable().values(); + ImmutableCollection before = enduringData().immutable().values(); this.enduringNodes.replace(newNode, previous); invalidateCache(); - ImmutableCollection after = enduringData().immutable().values(); + ImmutableCollection after = enduringData().immutable().values(); this.plugin.getEventFactory().handleNodeAdd(newNode, this, before, after); return Maps.immutableEntry(DataMutateResult.SUCCESS, newNode); @@ -600,7 +581,7 @@ public abstract class PermissionHolder { } else if (modifier == TemporaryModifier.REPLACE) { // Try to replace an existing node - Optional existing = searchForMatch(NodeMapType.ENDURING, node, StandardNodeEquality.IGNORE_EXPIRY_TIME_AND_VALUE); + Optional existing = searchForMatch(NodeMapType.ENDURING, node, StandardNodeEquality.IGNORE_EXPIRY_TIME_AND_VALUE); // An existing node was found if (existing.isPresent()) { @@ -609,10 +590,10 @@ public abstract class PermissionHolder { // Only replace if the new expiry time is greater than the old one. if (node.getExpiryUnixTime() > previous.getExpiryUnixTime()) { - ImmutableCollection before = enduringData().immutable().values(); + ImmutableCollection before = enduringData().immutable().values(); this.enduringNodes.replace(node, previous); invalidateCache(); - ImmutableCollection after = enduringData().immutable().values(); + ImmutableCollection after = enduringData().immutable().values(); this.plugin.getEventFactory().handleNodeAdd(node, this, before, after); return Maps.immutableEntry(DataMutateResult.SUCCESS, node); @@ -652,10 +633,10 @@ public abstract class PermissionHolder { return DataMutateResult.LACKS; } - ImmutableCollection before = enduringData().immutable().values(); + ImmutableCollection before = enduringData().immutable().values(); this.enduringNodes.remove(node); invalidateCache(); - ImmutableCollection after = enduringData().immutable().values(); + ImmutableCollection after = enduringData().immutable().values(); this.plugin.getEventFactory().handleNodeRemove(node, this, before, after); return DataMutateResult.SUCCESS; @@ -676,22 +657,14 @@ public abstract class PermissionHolder { return DataMutateResult.SUCCESS; } - public boolean inheritsGroup(Group group) { - return group.getName().equalsIgnoreCase(this.getObjectName()) || hasPermission(NodeMapType.ENDURING, NodeFactory.buildGroupNode(group.getName()).build(), StandardNodeEquality.IGNORE_EXPIRY_TIME_AND_VALUE).asBoolean(); - } - - public boolean inheritsGroup(Group group, ContextSet contextSet) { - return group.getName().equalsIgnoreCase(this.getObjectName()) || hasPermission(NodeMapType.ENDURING, NodeFactory.buildGroupNode(group.getName()).withExtraContext(contextSet).build(), StandardNodeEquality.IGNORE_EXPIRY_TIME_AND_VALUE).asBoolean(); - } - /** * Clear all of the holders permission nodes */ public boolean clearNodes() { - ImmutableCollection before = enduringData().immutable().values(); + ImmutableCollection before = enduringData().immutable().values(); this.enduringNodes.clear(); invalidateCache(); - ImmutableCollection after = enduringData().immutable().values(); + ImmutableCollection after = enduringData().immutable().values(); if (before.size() == after.size()) { return false; @@ -702,10 +675,10 @@ public abstract class PermissionHolder { } public boolean clearNodes(ContextSet contextSet) { - ImmutableCollection before = enduringData().immutable().values(); + ImmutableCollection before = enduringData().immutable().values(); this.enduringNodes.clear(contextSet); invalidateCache(); - ImmutableCollection after = enduringData().immutable().values(); + ImmutableCollection after = enduringData().immutable().values(); if (before.size() == after.size()) { return false; 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 1de4bc80e..7468db2fa 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 @@ -39,6 +39,12 @@ public final class ImmutableLocalizedNode extends ForwardingNode implements Loca public static ImmutableLocalizedNode of(Node node, String location) { 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/sponge/src/main/java/me/lucko/luckperms/sponge/service/internal/HolderSubjectData.java b/sponge/src/main/java/me/lucko/luckperms/sponge/service/internal/HolderSubjectData.java index 1b4473bfe..96bb85a8b 100644 --- a/sponge/src/main/java/me/lucko/luckperms/sponge/service/internal/HolderSubjectData.java +++ b/sponge/src/main/java/me/lucko/luckperms/sponge/service/internal/HolderSubjectData.java @@ -72,7 +72,7 @@ public class HolderSubjectData implements LPSubjectData { this.parentSubject = parentSubject; } - private Stream streamNodes() { + private Stream streamNodes() { return this.holder.getData(this.type).immutable().values().stream(); } @@ -94,7 +94,7 @@ public class HolderSubjectData implements LPSubjectData { @Override public ImmutableMap> getAllPermissions() { ImmutableMap.Builder> ret = ImmutableMap.builder(); - for (Map.Entry> entry : this.holder.getData(this.type).immutable().asMap().entrySet()) { + for (Map.Entry> entry : this.holder.getData(this.type).immutable().asMap().entrySet()) { ImmutableMap.Builder builder = ImmutableMap.builder(); for (Node n : entry.getValue()) { builder.put(n.getPermission(), n.getValue()); @@ -183,7 +183,7 @@ public class HolderSubjectData implements LPSubjectData { @Override public ImmutableMap> getAllParents() { ImmutableMap.Builder> ret = ImmutableMap.builder(); - for (Map.Entry> entry : this.holder.getData(this.type).immutable().asMap().entrySet()) { + for (Map.Entry> entry : this.holder.getData(this.type).immutable().asMap().entrySet()) { ImmutableList.Builder builder = ImmutableList.builder(); for (Node n : entry.getValue()) { if (n.isGroupNode()) {