store custom domains thread safe, improve api usability

Author: Joo200

worldguard-core/src/main/java/com/sk89q/worldguard/domains/DefaultDomain.java

@@ -36,12 +36,12 @@
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.UUID;
+import java.util.concurrent.ConcurrentHashMap;
 
 import static com.google.common.base.Preconditions.checkNotNull;
 
@@ -53,7 +53,7 @@ public class DefaultDomain implements Domain, ChangeTracked {
     private PlayerDomain playerDomain = new PlayerDomain();
     private GroupDomain groupDomain = new GroupDomain();
 
-    private Set<CustomDomain> customDomains = new HashSet<>();
+    private final Map<String, CustomDomain> customDomains = new ConcurrentHashMap<>();
     private boolean customDomainsChanged = false;
 
     /**
@@ -70,7 +70,7 @@ public DefaultDomain() {
     public DefaultDomain(DefaultDomain existing) {
         setPlayerDomain(existing.getPlayerDomain());
         setGroupDomain(existing.getGroupDomain());
-        setCustomDomains(existing.getCustomDomains());
+        setCustomDomains(existing.customDomains);
     }
 
     /**
@@ -118,9 +118,8 @@ public void setGroupDomain(GroupDomain groupDomain) {
      */
     public void addCustomDomain(CustomDomain customDomain) {
         checkNotNull(customDomain);
-        removeCustomDomain(customDomain.getName());
-        this.customDomains.add(customDomain);
-        customDomainsChanged = true;
+        this.customDomains.put(customDomain.getName(), customDomain);
+        this.customDomainsChanged = true;
     }
 
     /**
@@ -130,8 +129,8 @@ public void addCustomDomain(CustomDomain customDomain) {
      */
     public void removeCustomDomain(String name) {
         checkNotNull(name);
-        if (this.customDomains.removeIf(d -> d.getName().equalsIgnoreCase(name))) {
-            customDomainsChanged = true;
+        if (this.customDomains.remove(name) != null) {
+            this.customDomainsChanged = true;
         }
     }
 
@@ -142,8 +141,8 @@ public void removeCustomDomain(String name) {
      */
     public void removeCustomDomain(CustomDomain customDomain) {
         checkNotNull(customDomain);
-        if (this.customDomains.remove(customDomain)) {
-            customDomainsChanged = true;
+        if (this.customDomains.remove(customDomain.getName()) != null) {
+            this.customDomainsChanged = true;
         }
     }
 
@@ -152,19 +151,30 @@ public void removeCustomDomain(CustomDomain customDomain) {
      *
      * @param customDomains the domains
      */
-    public void setCustomDomains(Collection<CustomDomain> customDomains) {
+    public void setCustomDomains(Map<String, CustomDomain> customDomains) {
         checkNotNull(customDomains);
-        this.customDomains = new HashSet<>(customDomains);
-        customDomainsChanged = true;
+        this.customDomains.clear();
+        this.customDomains.putAll(customDomains);
+        this.customDomainsChanged = true;
     }
 
     /**
      * Get all api domains
      *
      * @return a unmodifiable copy of the domains
      */
-    public Set<CustomDomain> getCustomDomains() {
-        return Collections.unmodifiableSet(this.customDomains);
+    public Collection<CustomDomain> getCustomDomains() {
+        return Collections.unmodifiableCollection(this.customDomains.values());
+    }
+
+    /**
+     * Get the api domain specified by its name
+     *
+     * @param name the name of the domain
+     * @return the custom domain
+     */
+    public @Nullable CustomDomain getCustomDomain(String name) {
+        return this.customDomains.get(name);
     }
 
     /**
@@ -311,12 +321,12 @@ public Set<String> getGroups() {
 
     @Override
     public boolean contains(LocalPlayer player) {
-        return playerDomain.contains(player) || groupDomain.contains(player) || customDomains.stream().anyMatch(d -> d.contains(player));
+        return playerDomain.contains(player) || groupDomain.contains(player) || customDomains.values().stream().anyMatch(d -> d.contains(player));
     }
 
     @Override
     public boolean contains(UUID uniqueId) {
-        return playerDomain.contains(uniqueId) || customDomains.stream().anyMatch(d -> d.contains(uniqueId));
+        return playerDomain.contains(uniqueId) || customDomains.values().stream().anyMatch(d -> d.contains(uniqueId));
     }
 
     @Override
@@ -384,7 +394,7 @@ public String toGroupsString() {
 
     public String toCustomDomainsString() {
         List<String> output = new ArrayList<>();
-        for (CustomDomain customDomain : customDomains) {
+        for (CustomDomain customDomain : customDomains.values()) {
             output.add(customDomain.getName() + ":" + customDomain.toString());
         }
         output.sort(String.CASE_INSENSITIVE_ORDER);
@@ -513,7 +523,7 @@ private Component toPlayersComponent(ProfileCache cache) {
 
     private Component toCustomDomainsComponent() {
         final TextComponent.Builder builder = TextComponent.builder("");
-        for (Iterator<CustomDomain> it = customDomains.iterator(); it.hasNext(); ) {
+        for (Iterator<CustomDomain> it = customDomains.values().iterator(); it.hasNext(); ) {
             CustomDomain domain = it.next();
             builder.append(TextComponent.of(domain.getName() + ":", TextColor.LIGHT_PURPLE))
                     .append(TextComponent.of(domain.toString(), TextColor.GOLD));
@@ -528,15 +538,15 @@ private Component toCustomDomainsComponent() {
     @Override
     public boolean isDirty() {
         return playerDomain.isDirty() || groupDomain.isDirty() ||
-                customDomainsChanged ||  customDomains.stream().anyMatch(ChangeTracked::isDirty);
+                customDomainsChanged ||  customDomains.values().stream().anyMatch(ChangeTracked::isDirty);
     }
 
     @Override
     public void setDirty(boolean dirty) {
         playerDomain.setDirty(dirty);
         groupDomain.setDirty(dirty);
         customDomainsChanged = dirty;
-        customDomains.forEach(d -> d.setDirty(dirty));
+        customDomains.values().forEach(d -> d.setDirty(dirty));
     }
 
     @Override

worldguard-core/src/main/java/com/sk89q/worldguard/domains/registry/DomainRegistry.java

@@ -83,7 +83,7 @@ public interface DomainRegistry extends Iterable<DomainFactory<?>> {
      * @param createUnknown Whether "just in time" domains should be created for unknown domains
      * @return The unmarshalled domain list
      */
-    List<CustomDomain> unmarshal(Map<String, Object> rawValues, boolean createUnknown);
+    Map<String, CustomDomain> unmarshal(Map<String, Object> rawValues, boolean createUnknown);
 
     /**
      * Get the number of registered domains.

worldguard-core/src/main/java/com/sk89q/worldguard/domains/registry/SimpleDomainRegistry.java

@@ -25,9 +25,8 @@
 import com.sk89q.worldguard.domains.CustomDomain;
 
 import javax.annotation.Nullable;
-import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.Iterator;
-import java.util.List;
 import java.util.Map;
 import java.util.concurrent.ConcurrentMap;
 import java.util.logging.Level;
@@ -139,20 +138,20 @@ private CustomDomain getOrCreate(String name, Object value, boolean createUnknow
         return null;
     }
 
-    public List<CustomDomain> unmarshal(Map<String, Object> rawValues, boolean createUnknown) {
+    public Map<String, CustomDomain> unmarshal(Map<String, Object> rawValues, boolean createUnknown) {
         checkNotNull(rawValues, "rawValues");
 
-        List<CustomDomain> domainList = new ArrayList<>();
+        Map<String, CustomDomain> domains = new HashMap<>();
 
         for (Map.Entry<String, Object> entry : rawValues.entrySet()) {
             try {
                 CustomDomain domain = getOrCreate(entry.getKey(), entry.getValue(), createUnknown);
-                domainList.add(domain);
+                domains.put(domain.getName(), domain);
             } catch (Throwable e) {
                 log.log(Level.WARNING, "Failed to unmarshal domain for " + entry.getKey(), e);
             }
         }
-        return domainList;
+        return domains;
     }
 
     @Override

worldguard-core/src/main/java/com/sk89q/worldguard/protection/managers/storage/file/YamlRegionFile.java

@@ -289,7 +289,7 @@ private DefaultDomain parseDomain(YAMLNode node) {
 
         YAMLNode apiDomains = node.getNode("custom");
         if (apiDomains != null) {
-            List<CustomDomain> parsedDomains = WorldGuard.getInstance().getDomainRegistry().unmarshal(apiDomains.getMap(), true);
+            Map<String, CustomDomain> parsedDomains = WorldGuard.getInstance().getDomainRegistry().unmarshal(apiDomains.getMap(), true);
             domain.setCustomDomains(parsedDomains);
         }