From f0c03289195a46c70c48b4a9f790e95f25eddd1c Mon Sep 17 00:00:00 2001 From: Luck Date: Tue, 4 Sep 2018 20:33:08 +0100 Subject: [PATCH] Refactor MetaAccumulator to be a little more sane --- .../luckperms/bukkit/vault/VaultChatHook.java | 4 +- .../common/caching/type/MetaAccumulator.java | 75 ++++++++++++++----- .../common/caching/type/MetaCache.java | 54 +++++-------- .../generic/meta/MetaSetChatMeta.java | 1 + .../generic/meta/MetaSetTempChatMeta.java | 1 + .../service/internal/HolderSubjectData.java | 1 + 6 files changed, 83 insertions(+), 53 deletions(-) diff --git a/bukkit/src/main/java/me/lucko/luckperms/bukkit/vault/VaultChatHook.java b/bukkit/src/main/java/me/lucko/luckperms/bukkit/vault/VaultChatHook.java index 3f88b2c3..8ba9da73 100644 --- a/bukkit/src/main/java/me/lucko/luckperms/bukkit/vault/VaultChatHook.java +++ b/bukkit/src/main/java/me/lucko/luckperms/bukkit/vault/VaultChatHook.java @@ -262,8 +262,8 @@ public class VaultChatHook extends AbstractVaultChat { // find the max inherited priority & add 10 MetaAccumulator metaAccumulator = holder.accumulateMeta(null, createContextForWorldSet(world)); - int priority = (type == ChatMetaType.PREFIX ? metaAccumulator.getPrefixes() : metaAccumulator.getSuffixes()).keySet().stream() - .mapToInt(e -> e).max().orElse(0) + 10; + metaAccumulator.complete(); + int priority = metaAccumulator.getChatMeta(type).keySet().stream().mapToInt(e -> e).max().orElse(0) + 10; Node.Builder chatMetaNode = NodeFactory.buildChatMetaNode(type, priority, value); chatMetaNode.setServer(this.permissionHook.getVaultServer()); diff --git a/common/src/main/java/me/lucko/luckperms/common/caching/type/MetaAccumulator.java b/common/src/main/java/me/lucko/luckperms/common/caching/type/MetaAccumulator.java index 2b3c36a4..2a7db238 100644 --- a/common/src/main/java/me/lucko/luckperms/common/caching/type/MetaAccumulator.java +++ b/common/src/main/java/me/lucko/luckperms/common/caching/type/MetaAccumulator.java @@ -44,12 +44,14 @@ import java.util.Map; import java.util.Objects; import java.util.SortedMap; import java.util.TreeMap; +import java.util.concurrent.atomic.AtomicReference; /** * Holds temporary mutable meta whilst this object is passed up the * inheritance tree to accumulate meta from parents */ public class MetaAccumulator { + public static MetaAccumulator makeFromConfig(LuckPermsPlugin plugin) { return new MetaAccumulator( new SimpleMetaStack(plugin.getConfiguration().get(ConfigKeys.PREFIX_FORMATTING_OPTIONS), ChatMetaType.PREFIX), @@ -57,6 +59,18 @@ public class MetaAccumulator { ); } + /** + * Represents the current state of a {@link MetaAccumulator}. + */ + private enum State { + /** Marks that the accumulator is still gaining (accumulating) new data. */ + ACCUMULATING, + /** Marks that the process of gaining (accumulating) new data is complete. */ + COMPLETE + } + + private final AtomicReference state = new AtomicReference<>(State.ACCUMULATING); + private final ListMultimap meta; private final SortedMap prefixes; private final SortedMap suffixes; @@ -75,7 +89,34 @@ public class MetaAccumulator { this.suffixStack = suffixStack; } + private void ensureState(State state) { + if (this.state.get() != state) { + throw new IllegalStateException("State must be " + state + ", but is actually " + this.state.get()); + } + } + + /** + * "Completes" the accumulator, preventing any further changes. + * + * Also performs some final processing on the accumulators state, before + * data is read. + */ + public void complete() { + if (!this.state.compareAndSet(State.ACCUMULATING, State.COMPLETE)) { + return; + } + + // perform final changes + if (!this.meta.containsKey(NodeTypes.WEIGHT_KEY) && this.weight != 0) { + this.meta.put(NodeTypes.WEIGHT_KEY, String.valueOf(this.weight)); + } + } + + // accumulate methods + public void accumulateNode(LocalizedNode n) { + ensureState(State.ACCUMULATING); + n.getTypeData(MetaType.KEY).ifPresent(metaType -> this.meta.put(metaType.getKey(), metaType.getValue()) ); @@ -92,60 +133,60 @@ public class MetaAccumulator { } public void accumulateMeta(String key, String value) { + ensureState(State.ACCUMULATING); this.meta.put(key, value); } public void accumulateWeight(int weight) { + ensureState(State.ACCUMULATING); this.weight = Math.max(this.weight, weight); } - // We can assume that if this method is being called, this holder is effectively finalized. - // (it's not going to accumulate more nodes) - // Therefore, it should be ok to set the weight meta key, if not already present. - public ListMultimap getMeta() { - if (!this.meta.containsKey(NodeTypes.WEIGHT_KEY) && this.weight != 0) { - this.meta.put(NodeTypes.WEIGHT_KEY, String.valueOf(this.weight)); - } + // read methods + public ListMultimap getMeta() { + ensureState(State.COMPLETE); return this.meta; } public Map getChatMeta(ChatMetaType type) { + ensureState(State.COMPLETE); return type == ChatMetaType.PREFIX ? this.prefixes : this.suffixes; } - public MetaStack getStack(ChatMetaType type) { - return type == ChatMetaType.PREFIX ? this.prefixStack : this.suffixStack; - } - public SortedMap getPrefixes() { + ensureState(State.COMPLETE); return this.prefixes; } public SortedMap getSuffixes() { + ensureState(State.COMPLETE); return this.suffixes; } public int getWeight() { + ensureState(State.COMPLETE); return this.weight; } public MetaStack getPrefixStack() { + ensureState(State.COMPLETE); return this.prefixStack; } public MetaStack getSuffixStack() { + ensureState(State.COMPLETE); return this.suffixStack; } @Override public String toString() { return "MetaAccumulator(" + - "meta=" + this.getMeta() + ", " + - "prefixes=" + this.getPrefixes() + ", " + - "suffixes=" + this.getSuffixes() + ", " + - "weight=" + this.getWeight() + ", " + - "prefixStack=" + this.getPrefixStack() + ", " + - "suffixStack=" + this.getSuffixStack() + ")"; + "meta=" + this.meta + ", " + + "prefixes=" + this.prefixes + ", " + + "suffixes=" + this.suffixes + ", " + + "weight=" + this.weight + ", " + + "prefixStack=" + this.prefixStack + ", " + + "suffixStack=" + this.suffixStack + ")"; } } diff --git a/common/src/main/java/me/lucko/luckperms/common/caching/type/MetaCache.java b/common/src/main/java/me/lucko/luckperms/common/caching/type/MetaCache.java index 47b64e1f..7479123e 100644 --- a/common/src/main/java/me/lucko/luckperms/common/caching/type/MetaCache.java +++ b/common/src/main/java/me/lucko/luckperms/common/caching/type/MetaCache.java @@ -39,8 +39,6 @@ import me.lucko.luckperms.common.metastacking.MetaStack; import java.util.List; import java.util.Map; import java.util.SortedMap; -import java.util.concurrent.locks.ReadWriteLock; -import java.util.concurrent.locks.ReentrantReadWriteLock; import javax.annotation.Nonnull; @@ -48,7 +46,6 @@ import javax.annotation.Nonnull; * Holds cached meta for a given context */ public class MetaCache implements MetaData { - private final ReadWriteLock lock = new ReentrantReadWriteLock(); /** * The contexts this container is holding data for @@ -67,51 +64,40 @@ public class MetaCache implements MetaData { } public void loadMeta(MetaAccumulator meta) { - this.lock.writeLock().lock(); - try { - this.metaMultimap = ImmutableListMultimap.copyOf(meta.getMeta()); + meta.complete(); - //noinspection unchecked - Map> metaMap = (Map) this.metaMultimap.asMap(); - ImmutableMap.Builder metaMapBuilder = ImmutableMap.builder(); + this.metaMultimap = ImmutableListMultimap.copyOf(meta.getMeta()); - for (Map.Entry> e : metaMap.entrySet()) { - if (e.getValue().isEmpty()) { - continue; - } + //noinspection unchecked + Map> metaMap = (Map) this.metaMultimap.asMap(); + ImmutableMap.Builder metaMapBuilder = ImmutableMap.builder(); - // take the value which was accumulated first - metaMapBuilder.put(e.getKey(), e.getValue().get(0)); + for (Map.Entry> e : metaMap.entrySet()) { + if (e.getValue().isEmpty()) { + continue; } - this.meta = metaMapBuilder.build(); - this.prefixes = ImmutableSortedMap.copyOfSorted(meta.getPrefixes()); - this.suffixes = ImmutableSortedMap.copyOfSorted(meta.getSuffixes()); - this.prefixStack = meta.getPrefixStack(); - this.suffixStack = meta.getSuffixStack(); - } finally { - this.lock.writeLock().unlock(); + // take the value which was accumulated first + metaMapBuilder.put(e.getKey(), e.getValue().get(0)); } + this.meta = metaMapBuilder.build(); + + this.prefixes = ImmutableSortedMap.copyOfSorted(meta.getPrefixes()); + this.suffixes = ImmutableSortedMap.copyOfSorted(meta.getSuffixes()); + this.prefixStack = meta.getPrefixStack(); + this.suffixStack = meta.getSuffixStack(); } @Override public String getPrefix() { - this.lock.readLock().lock(); - try { - return this.prefixStack == null ? null : this.prefixStack.toFormattedString(); - } finally { - this.lock.readLock().unlock(); - } + MetaStack prefixStack = this.prefixStack; + return prefixStack == null ? null : prefixStack.toFormattedString(); } @Override public String getSuffix() { - this.lock.readLock().lock(); - try { - return this.suffixStack == null ? null : this.suffixStack.toFormattedString(); - } finally { - this.lock.readLock().unlock(); - } + MetaStack suffixStack = this.suffixStack; + return suffixStack == null ? null : suffixStack.toFormattedString(); } @Nonnull diff --git a/common/src/main/java/me/lucko/luckperms/common/commands/generic/meta/MetaSetChatMeta.java b/common/src/main/java/me/lucko/luckperms/common/commands/generic/meta/MetaSetChatMeta.java index f7e574b2..5b053887 100644 --- a/common/src/main/java/me/lucko/luckperms/common/commands/generic/meta/MetaSetChatMeta.java +++ b/common/src/main/java/me/lucko/luckperms/common/commands/generic/meta/MetaSetChatMeta.java @@ -107,6 +107,7 @@ public class MetaSetChatMeta extends SharedSubCommand { // determine the priority to set at if (priority == Integer.MIN_VALUE) { MetaAccumulator metaAccumulator = holder.accumulateMeta(null, Contexts.of(context, Contexts.global().getSettings())); + metaAccumulator.complete(); priority = metaAccumulator.getChatMeta(this.type).keySet().stream().mapToInt(e -> e).max().orElse(0) + 1; if (holder instanceof Group) { diff --git a/common/src/main/java/me/lucko/luckperms/common/commands/generic/meta/MetaSetTempChatMeta.java b/common/src/main/java/me/lucko/luckperms/common/commands/generic/meta/MetaSetTempChatMeta.java index 4dbd02ac..bc7d1789 100644 --- a/common/src/main/java/me/lucko/luckperms/common/commands/generic/meta/MetaSetTempChatMeta.java +++ b/common/src/main/java/me/lucko/luckperms/common/commands/generic/meta/MetaSetTempChatMeta.java @@ -118,6 +118,7 @@ public class MetaSetTempChatMeta extends SharedSubCommand { // determine the priority to set at if (priority == Integer.MIN_VALUE) { MetaAccumulator metaAccumulator = holder.accumulateMeta(null, Contexts.of(context, Contexts.global().getSettings())); + metaAccumulator.complete(); priority = metaAccumulator.getChatMeta(this.type).keySet().stream().mapToInt(e -> e).max().orElse(0) + 1; if (holder instanceof Group) { 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 b45f0b8c..56c12221 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 @@ -362,6 +362,7 @@ public class HolderSubjectData implements LPSubjectData { )); MetaAccumulator metaAccumulator = this.holder.accumulateMeta(null, Contexts.of(contexts, Contexts.global().getSettings())); + metaAccumulator.complete(); int priority = metaAccumulator.getChatMeta(type).keySet().stream().mapToInt(e -> e).max().orElse(0); priority += 10;