Some small fixes and changes for ContextSet implementations

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<String, String> backing();
+    protected abstract SetMultimap<String, String> backing();
 
-    @Nonnull
+    protected abstract void copyTo(SetMultimap<String, String> other);
-    @Override
-    @Deprecated
-    public Map<String, String> toMap() {
-        ImmutableMap.Builder<String, String> m = ImmutableMap.builder();
-        for (Map.Entry<String, String> e : backing().entries()) {
-            m.put(e.getKey(), e.getValue());
-        }
-        return m.build();
-    }
 
     @Override
     public boolean containsKey(@Nonnull String key) {

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<Map.Entry<String, String>> {
 
     /**
      * Creates an {@link ImmutableContextSet} from a context pair.
@@ -230,6 +231,20 @@ public interface ContextSet {
     @Nonnull
     Multimap<String, String> toMultimap();
 
+    /**
+     * Returns an {@link Iterator} over each of the context pairs in this set.
+     *
+     * <p>The returned iterator represents the state of the set at the time of creation. It is not
+     * updated as the set changes.</p>
+     *
+     * <p>The iterator does not support {@link Iterator#remove()} calls.</p>
+     *
+     * @return an iterator
+     */
+    @Nonnull
+    @Override
+    Iterator<Map.Entry<String, String>> iterator();
+
     /**
      * Returns if the {@link ContextSet} contains at least one value for the
      * given key.

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<String, String> backing() {
+    protected SetMultimap<String, String> backing() {
         return this.map;
     }
 
+    @Override
+    protected void copyTo(SetMultimap<String, String> 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<String, String> toMap() {
+        ImmutableMap.Builder<String, String> m = ImmutableMap.builder();
+        for (Map.Entry<String, String> e : this.map.entries()) {
+            m.put(e.getKey(), e.getValue());
+        }
+        return m.build();
+    }
+
     @Nonnull
     @Override
     public Multimap<String, String> toMultimap() {
         return this.map;
     }
 
+    @Nonnull
+    @Override
+    public Iterator<Map.Entry<String, String>> iterator() {
+        return this.map.entries().iterator();
+    }
+
+    @Override
+    public Spliterator<Map.Entry<String, String>> spliterator() {
+        return this.map.entries().spliterator();
+    }
+
     @Override
     public int hashCode() {
         return this.hashCode;

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,10 +146,18 @@ public final class MutableContextSet extends AbstractContextSet implements Conte
     @Nonnull
     public static MutableContextSet fromSet(@Nonnull ContextSet contextSet) {
         Objects.requireNonNull(contextSet, "contextSet");
+
+        if (contextSet instanceof ImmutableContextSet) {
+            SetMultimap<String, String> 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;
         }
+    }
 
     /**
      * Creates a new empty MutableContextSet.
@@ -164,15 +175,22 @@ public final class MutableContextSet extends AbstractContextSet implements Conte
         this.map = Multimaps.synchronizedSetMultimap(HashMultimap.create());
     }
 
-    private MutableContextSet(MutableContextSet other) {
+    private MutableContextSet(SetMultimap<String, String> other) {
-        this.map = Multimaps.synchronizedSetMultimap(HashMultimap.create(other.map));
+        this.map = Multimaps.synchronizedSetMultimap(HashMultimap.create(other));
     }
 
     @Override
-    protected Multimap<String, String> backing() {
+    protected SetMultimap<String, String> backing() {
         return this.map;
     }
 
+    @Override
+    protected void copyTo(SetMultimap<String, String> other) {
+        synchronized (this.map) {
+            other.putAll(this.map);
+        }
+    }
+
     @Override
     public boolean isImmutable() {
         return false;
@@ -185,26 +203,59 @@ public final class MutableContextSet extends AbstractContextSet implements Conte
         if (this.map.isEmpty()) {
             return ImmutableContextSet.empty();
         }
+        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<Map.Entry<String, String>> toSet() {
+        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<String, String> toMap() {
+        ImmutableMap.Builder<String, String> m = ImmutableMap.builder();
+        synchronized (this.map) {
+            for (Map.Entry<String, String> e : this.map.entries()) {
+                m.put(e.getKey(), e.getValue());
+            }
+        }
+        return m.build();
+    }
 
     @Nonnull
     @Override
     public Multimap<String, String> toMultimap() {
+        synchronized (this.map) {
             return ImmutableSetMultimap.copyOf(this.map);
         }
+    }
+
+    @Nonnull
+    @Override
+    public Iterator<Map.Entry<String, String>> iterator() {
+        return toSet().iterator();
+    }
+
+    @Override
+    public Spliterator<Map.Entry<String, String>> spliterator() {
+        return toSet().spliterator();
+    }
 
     /**
      * Adds a context to this set.
@@ -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());
         }

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<Context> {
-        private final Iterator<Map.Entry<String, String>> it = DelegatingImmutableContextSet.this.delegate.toSet().iterator();
+        private final Iterator<Map.Entry<String, String>> it = DelegatingImmutableContextSet.this.delegate.iterator();
 
         @Override
         public boolean hasNext() {

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<Context> {
-        private final Iterator<Map.Entry<String, String>> it = DelegatingMutableContextSet.this.delegate.toSet().iterator();
+        private final Iterator<Map.Entry<String, String>> it = DelegatingMutableContextSet.this.delegate.iterator();
         private Context current;
 
         @Override