From 3784d904fb772c8e3ff557ae749954cbafac47a0 Mon Sep 17 00:00:00 2001 From: Luck Date: Fri, 27 Apr 2018 18:50:36 +0100 Subject: [PATCH] Some small fixes and changes for ContextSet implementations --- .../api/context/AbstractContextSet.java | 16 +--- .../luckperms/api/context/ContextSet.java | 17 ++++- .../api/context/ImmutableContextSet.java | 33 +++++++- .../api/context/MutableContextSet.java | 75 ++++++++++++++++--- .../DelegatingImmutableContextSet.java | 2 +- .../context/DelegatingMutableContextSet.java | 2 +- 6 files changed, 117 insertions(+), 28 deletions(-) diff --git a/api/src/main/java/me/lucko/luckperms/api/context/AbstractContextSet.java b/api/src/main/java/me/lucko/luckperms/api/context/AbstractContextSet.java index 9e667031..637af6b2 100644 --- a/api/src/main/java/me/lucko/luckperms/api/context/AbstractContextSet.java +++ b/api/src/main/java/me/lucko/luckperms/api/context/AbstractContextSet.java @@ -25,12 +25,11 @@ package me.lucko.luckperms.api.context; -import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Multimap; +import com.google.common.collect.SetMultimap; import java.util.Collection; -import java.util.Map; import java.util.Objects; import java.util.Set; @@ -38,18 +37,9 @@ import javax.annotation.Nonnull; abstract class AbstractContextSet implements ContextSet { - protected abstract Multimap backing(); + protected abstract SetMultimap backing(); - @Nonnull - @Override - @Deprecated - public Map toMap() { - ImmutableMap.Builder m = ImmutableMap.builder(); - for (Map.Entry e : backing().entries()) { - m.put(e.getKey(), e.getValue()); - } - return m.build(); - } + protected abstract void copyTo(SetMultimap other); @Override public boolean containsKey(@Nonnull String key) { diff --git a/api/src/main/java/me/lucko/luckperms/api/context/ContextSet.java b/api/src/main/java/me/lucko/luckperms/api/context/ContextSet.java index e50dcbca..e74792c2 100644 --- a/api/src/main/java/me/lucko/luckperms/api/context/ContextSet.java +++ b/api/src/main/java/me/lucko/luckperms/api/context/ContextSet.java @@ -27,6 +27,7 @@ package me.lucko.luckperms.api.context; import com.google.common.collect.Multimap; +import java.util.Iterator; import java.util.Map; import java.util.Objects; import java.util.Optional; @@ -60,7 +61,7 @@ import javax.annotation.Nonnull; * * @since 2.13 */ -public interface ContextSet { +public interface ContextSet extends Iterable> { /** * Creates an {@link ImmutableContextSet} from a context pair. @@ -230,6 +231,20 @@ public interface ContextSet { @Nonnull Multimap toMultimap(); + /** + * Returns an {@link Iterator} over each of the context pairs in this set. + * + *

The returned iterator represents the state of the set at the time of creation. It is not + * updated as the set changes.

+ * + *

The iterator does not support {@link Iterator#remove()} calls.

+ * + * @return an iterator + */ + @Nonnull + @Override + Iterator> iterator(); + /** * Returns if the {@link ContextSet} contains at least one value for the * given key. diff --git a/api/src/main/java/me/lucko/luckperms/api/context/ImmutableContextSet.java b/api/src/main/java/me/lucko/luckperms/api/context/ImmutableContextSet.java index e60929bf..e8234245 100644 --- a/api/src/main/java/me/lucko/luckperms/api/context/ImmutableContextSet.java +++ b/api/src/main/java/me/lucko/luckperms/api/context/ImmutableContextSet.java @@ -25,12 +25,16 @@ package me.lucko.luckperms.api.context; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSetMultimap; import com.google.common.collect.Multimap; +import com.google.common.collect.SetMultimap; +import java.util.Iterator; import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.Spliterator; import javax.annotation.Nonnull; import javax.annotation.concurrent.Immutable; @@ -164,10 +168,15 @@ public final class ImmutableContextSet extends AbstractContextSet implements Con } @Override - protected Multimap backing() { + protected SetMultimap backing() { return this.map; } + @Override + protected void copyTo(SetMultimap other) { + other.putAll(this.map); + } + @Override public boolean isImmutable() { return true; @@ -192,12 +201,34 @@ public final class ImmutableContextSet extends AbstractContextSet implements Con return this.map.entries(); } + @Nonnull + @Override + @Deprecated + public Map toMap() { + ImmutableMap.Builder m = ImmutableMap.builder(); + for (Map.Entry e : this.map.entries()) { + m.put(e.getKey(), e.getValue()); + } + return m.build(); + } + @Nonnull @Override public Multimap toMultimap() { return this.map; } + @Nonnull + @Override + public Iterator> iterator() { + return this.map.entries().iterator(); + } + + @Override + public Spliterator> spliterator() { + return this.map.entries().spliterator(); + } + @Override public int hashCode() { return this.hashCode; diff --git a/api/src/main/java/me/lucko/luckperms/api/context/MutableContextSet.java b/api/src/main/java/me/lucko/luckperms/api/context/MutableContextSet.java index 3b3aa2e8..ed2b9446 100644 --- a/api/src/main/java/me/lucko/luckperms/api/context/MutableContextSet.java +++ b/api/src/main/java/me/lucko/luckperms/api/context/MutableContextSet.java @@ -26,15 +26,18 @@ package me.lucko.luckperms.api.context; import com.google.common.collect.HashMultimap; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSetMultimap; import com.google.common.collect.Multimap; import com.google.common.collect.Multimaps; import com.google.common.collect.SetMultimap; +import java.util.Iterator; import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.Spliterator; import javax.annotation.Nonnull; @@ -143,9 +146,17 @@ public final class MutableContextSet extends AbstractContextSet implements Conte @Nonnull public static MutableContextSet fromSet(@Nonnull ContextSet contextSet) { Objects.requireNonNull(contextSet, "contextSet"); - MutableContextSet set = create(); - set.addAll(contextSet); - return set; + + if (contextSet instanceof ImmutableContextSet) { + SetMultimap map = ((ImmutableContextSet) contextSet).backing(); + return new MutableContextSet(map); + } else if (contextSet instanceof MutableContextSet) { + return contextSet.mutableCopy(); + } else { + MutableContextSet set = create(); + set.addAll(contextSet); + return set; + } } /** @@ -164,15 +175,22 @@ public final class MutableContextSet extends AbstractContextSet implements Conte this.map = Multimaps.synchronizedSetMultimap(HashMultimap.create()); } - private MutableContextSet(MutableContextSet other) { - this.map = Multimaps.synchronizedSetMultimap(HashMultimap.create(other.map)); + private MutableContextSet(SetMultimap other) { + this.map = Multimaps.synchronizedSetMultimap(HashMultimap.create(other)); } @Override - protected Multimap backing() { + protected SetMultimap backing() { return this.map; } + @Override + protected void copyTo(SetMultimap other) { + synchronized (this.map) { + other.putAll(this.map); + } + } + @Override public boolean isImmutable() { return false; @@ -185,25 +203,58 @@ public final class MutableContextSet extends AbstractContextSet implements Conte if (this.map.isEmpty()) { return ImmutableContextSet.empty(); } - return new ImmutableContextSet(ImmutableSetMultimap.copyOf(this.map)); + synchronized (this.map) { + return new ImmutableContextSet(ImmutableSetMultimap.copyOf(this.map)); + } } @Nonnull @Override public MutableContextSet mutableCopy() { - return new MutableContextSet(this); + synchronized (this.map) { + return new MutableContextSet(this.map); + } } @Nonnull @Override public Set> toSet() { - return ImmutableSet.copyOf(this.map.entries()); + synchronized (this.map) { + // map.entries() returns immutable Map.Entry instances, so we can just call copyOf + return ImmutableSet.copyOf(this.map.entries()); + } + } + + @Nonnull + @Override + @Deprecated + public Map toMap() { + ImmutableMap.Builder m = ImmutableMap.builder(); + synchronized (this.map) { + for (Map.Entry e : this.map.entries()) { + m.put(e.getKey(), e.getValue()); + } + } + return m.build(); } @Nonnull @Override public Multimap toMultimap() { - return ImmutableSetMultimap.copyOf(this.map); + synchronized (this.map) { + return ImmutableSetMultimap.copyOf(this.map); + } + } + + @Nonnull + @Override + public Iterator> iterator() { + return toSet().iterator(); + } + + @Override + public Spliterator> spliterator() { + return toSet().spliterator(); } /** @@ -271,7 +322,9 @@ public final class MutableContextSet extends AbstractContextSet implements Conte Objects.requireNonNull(contextSet, "contextSet"); if (contextSet instanceof AbstractContextSet) { AbstractContextSet other = ((AbstractContextSet) contextSet); - this.map.putAll(other.backing()); + synchronized (this.map) { + other.copyTo(this.map); + } } else { addAll(contextSet.toMultimap()); } diff --git a/sponge/sponge-service/src/main/java/me/lucko/luckperms/sponge/service/context/DelegatingImmutableContextSet.java b/sponge/sponge-service/src/main/java/me/lucko/luckperms/sponge/service/context/DelegatingImmutableContextSet.java index 81c7ba0d..1c2cfde3 100644 --- a/sponge/sponge-service/src/main/java/me/lucko/luckperms/sponge/service/context/DelegatingImmutableContextSet.java +++ b/sponge/sponge-service/src/main/java/me/lucko/luckperms/sponge/service/context/DelegatingImmutableContextSet.java @@ -77,7 +77,7 @@ public class DelegatingImmutableContextSet extends AbstractDelegatingContextSet } private final class ContextSetIterator implements Iterator { - private final Iterator> it = DelegatingImmutableContextSet.this.delegate.toSet().iterator(); + private final Iterator> it = DelegatingImmutableContextSet.this.delegate.iterator(); @Override public boolean hasNext() { diff --git a/sponge/sponge-service/src/main/java/me/lucko/luckperms/sponge/service/context/DelegatingMutableContextSet.java b/sponge/sponge-service/src/main/java/me/lucko/luckperms/sponge/service/context/DelegatingMutableContextSet.java index af7055c0..56126ae7 100644 --- a/sponge/sponge-service/src/main/java/me/lucko/luckperms/sponge/service/context/DelegatingMutableContextSet.java +++ b/sponge/sponge-service/src/main/java/me/lucko/luckperms/sponge/service/context/DelegatingMutableContextSet.java @@ -96,7 +96,7 @@ public class DelegatingMutableContextSet extends AbstractDelegatingContextSet { } private final class ContextSetIterator implements Iterator { - private final Iterator> it = DelegatingMutableContextSet.this.delegate.toSet().iterator(); + private final Iterator> it = DelegatingMutableContextSet.this.delegate.iterator(); private Context current; @Override