From 46442682140282278e321f7cc374c0b51c2527ed Mon Sep 17 00:00:00 2001 From: stonar96 Date: Mon, 9 Aug 2021 04:58:09 +0200 Subject: [PATCH] Fix member inheritance for non-player associables (#1804) * Add options to query region sets unsorted and without parents * Fix member inheritance for non-player associables * Add member inheritance for non-player associables * Rename Option to QueryOption, remove functional definitions, bit of cleanup. Co-authored-by: wizjany --- .../protection/RegionResultSet.java | 13 +- .../AbstractRegionOverlapAssociation.java | 44 +++-- .../DelayedRegionOverlapAssociation.java | 3 +- .../protection/managers/RegionManager.java | 48 ++++- .../protection/regions/QueryCache.java | 16 +- .../protection/regions/RegionQuery.java | 185 +++++++++++++++++- .../util/RegionCollectionConsumer.java | 4 +- .../protection/RegionOverlapTest.java | 2 +- 8 files changed, 263 insertions(+), 52 deletions(-) diff --git a/worldguard-core/src/main/java/com/sk89q/worldguard/protection/RegionResultSet.java b/worldguard-core/src/main/java/com/sk89q/worldguard/protection/RegionResultSet.java index 7afec721..f4346ddf 100644 --- a/worldguard-core/src/main/java/com/sk89q/worldguard/protection/RegionResultSet.java +++ b/worldguard-core/src/main/java/com/sk89q/worldguard/protection/RegionResultSet.java @@ -19,6 +19,7 @@ package com.sk89q.worldguard.protection; +import com.google.common.collect.ImmutableSet; import com.sk89q.worldguard.LocalPlayer; import com.sk89q.worldguard.protection.association.RegionAssociable; import com.sk89q.worldguard.protection.flags.Flag; @@ -29,7 +30,11 @@ import com.sk89q.worldguard.protection.util.NormativeOrders; import javax.annotation.Nullable; -import java.util.*; +import java.util.Collection; +import java.util.Collections; +import java.util.Iterator; +import java.util.List; +import java.util.Set; import static com.google.common.base.Preconditions.checkNotNull; @@ -64,7 +69,7 @@ public RegionResultSet(List applicable, @Nullable ProtectedRegi */ public RegionResultSet(Set applicable, @Nullable ProtectedRegion globalRegion) { this(NormativeOrders.fromSet(applicable), globalRegion, true); - this.regionSet = applicable; + this.regionSet = ImmutableSet.copyOf(applicable); } /** @@ -83,7 +88,7 @@ public RegionResultSet(List applicable, @Nullable ProtectedRegi if (!sorted) { NormativeOrders.sort(applicable); } - this.applicable = applicable; + this.applicable = Collections.unmodifiableList(applicable); this.flagValueCalculator = new FlagValueCalculator(applicable, globalRegion); } @@ -157,7 +162,7 @@ public Set getRegions() { if (regionSet != null) { return regionSet; } - regionSet = Collections.unmodifiableSet(new HashSet<>(applicable)); + regionSet = ImmutableSet.copyOf(applicable); return regionSet; } diff --git a/worldguard-core/src/main/java/com/sk89q/worldguard/protection/association/AbstractRegionOverlapAssociation.java b/worldguard-core/src/main/java/com/sk89q/worldguard/protection/association/AbstractRegionOverlapAssociation.java index b33c0ac3..9b0a42e0 100644 --- a/worldguard-core/src/main/java/com/sk89q/worldguard/protection/association/AbstractRegionOverlapAssociation.java +++ b/worldguard-core/src/main/java/com/sk89q/worldguard/protection/association/AbstractRegionOverlapAssociation.java @@ -87,31 +87,35 @@ private boolean checkNonplayerProtectionDomains(Iterable regions) { checkNotNull(source); for (ProtectedRegion region : regions) { - if ((region.getId().equals(ProtectedRegion.GLOBAL_REGION) && source.isEmpty())) { - return Association.OWNER; - } - - if (source.contains(region)) { - if (useMaxPriorityAssociation) { - int priority = region.getPriority(); - if (priority == maxPriority) { - return Association.OWNER; - } - } else { + while (region != null) { + if ((region.getId().equals(ProtectedRegion.GLOBAL_REGION) && source.isEmpty())) { return Association.OWNER; } - } - Set source; + if (source.contains(region)) { + if (useMaxPriorityAssociation) { + int priority = region.getPriority(); + if (priority == maxPriority) { + return Association.OWNER; + } + } else { + return Association.OWNER; + } + } - if (useMaxPriorityAssociation) { - source = maxPriorityRegions; - } else { - source = this.source; - } + Set source; - if (checkNonplayerProtectionDomains(source, region.getFlag(Flags.NONPLAYER_PROTECTION_DOMAINS))) { - return Association.OWNER; + if (useMaxPriorityAssociation) { + source = maxPriorityRegions; + } else { + source = this.source; + } + + if (checkNonplayerProtectionDomains(source, region.getFlag(Flags.NONPLAYER_PROTECTION_DOMAINS))) { + return Association.OWNER; + } + + region = region.getParent(); } } diff --git a/worldguard-core/src/main/java/com/sk89q/worldguard/protection/association/DelayedRegionOverlapAssociation.java b/worldguard-core/src/main/java/com/sk89q/worldguard/protection/association/DelayedRegionOverlapAssociation.java index 49af1f78..05ac70fc 100644 --- a/worldguard-core/src/main/java/com/sk89q/worldguard/protection/association/DelayedRegionOverlapAssociation.java +++ b/worldguard-core/src/main/java/com/sk89q/worldguard/protection/association/DelayedRegionOverlapAssociation.java @@ -26,6 +26,7 @@ import com.sk89q.worldguard.protection.ApplicableRegionSet; import com.sk89q.worldguard.protection.regions.ProtectedRegion; import com.sk89q.worldguard.protection.regions.RegionQuery; +import com.sk89q.worldguard.protection.regions.RegionQuery.QueryOption; import java.util.List; @@ -67,7 +68,7 @@ public DelayedRegionOverlapAssociation(RegionQuery query, Location location, boo @Override public Association getAssociation(List regions) { if (source == null) { - ApplicableRegionSet result = query.getApplicableRegions(location); + ApplicableRegionSet result = query.getApplicableRegions(location, QueryOption.NONE); source = result.getRegions(); calcMaxPriority(); } diff --git a/worldguard-core/src/main/java/com/sk89q/worldguard/protection/managers/RegionManager.java b/worldguard-core/src/main/java/com/sk89q/worldguard/protection/managers/RegionManager.java index eb8a399e..2f22bdd4 100644 --- a/worldguard-core/src/main/java/com/sk89q/worldguard/protection/managers/RegionManager.java +++ b/worldguard-core/src/main/java/com/sk89q/worldguard/protection/managers/RegionManager.java @@ -34,7 +34,7 @@ import com.sk89q.worldguard.protection.managers.storage.RegionDatabase; import com.sk89q.worldguard.protection.managers.storage.StorageException; import com.sk89q.worldguard.protection.regions.ProtectedRegion; -import com.sk89q.worldguard.protection.util.RegionCollectionConsumer; +import com.sk89q.worldguard.protection.regions.RegionQuery.QueryOption; import com.sk89q.worldguard.util.Normal; import javax.annotation.Nullable; @@ -293,32 +293,60 @@ public Set removeRegion(String id, RemovalStrategy strategy) { } /** - * Query for effective flags and owners for the given positive. + * Query for effective flags and members for the given position. + * + *

{@link QueryOption#COMPUTE_PARENTS} is used.

* * @param position the position * @return the query object */ public ApplicableRegionSet getApplicableRegions(BlockVector3 position) { - checkNotNull(position); - - Set regions = Sets.newHashSet(); - index.applyContaining(position, new RegionCollectionConsumer(regions, true)); - return new RegionResultSet(regions, index.get("__global__")); + return getApplicableRegions(position, QueryOption.COMPUTE_PARENTS); } /** - * Query for effective flags and owners for the area represented + * Return a region set for the given position. + * + * @param position the position + * @param option the option + * @return a region set + */ + public ApplicableRegionSet getApplicableRegions(BlockVector3 position, QueryOption option) { + checkNotNull(position); + checkNotNull(option); + + Set regions = Sets.newHashSet(); + index.applyContaining(position, option.createIndexConsumer(regions)); + return new RegionResultSet(option.constructResult(regions), index.get("__global__"), true); + } + + /** + * Query for effective flags and members for the area represented * by the given region. * + *

{@link QueryOption#COMPUTE_PARENTS} is used.

+ * * @param region the region * @return the query object */ public ApplicableRegionSet getApplicableRegions(ProtectedRegion region) { + return getApplicableRegions(region, QueryOption.COMPUTE_PARENTS); + } + + /** + * Return a region set for the area represented by the given region. + * + * @param region the region + * @param option the option + * @return a region set + */ + public ApplicableRegionSet getApplicableRegions(ProtectedRegion region, QueryOption option) { checkNotNull(region); + checkNotNull(option); Set regions = Sets.newHashSet(); - index.applyIntersecting(region, new RegionCollectionConsumer(regions, true)); - return new RegionResultSet(regions, index.get("__global__")); + index.applyIntersecting(region, option.createIndexConsumer(regions)); + return new RegionResultSet(option.constructResult(regions), index.get("__global__"), true); } /** diff --git a/worldguard-core/src/main/java/com/sk89q/worldguard/protection/regions/QueryCache.java b/worldguard-core/src/main/java/com/sk89q/worldguard/protection/regions/QueryCache.java index e6fd5d0b..0dc29078 100644 --- a/worldguard-core/src/main/java/com/sk89q/worldguard/protection/regions/QueryCache.java +++ b/worldguard-core/src/main/java/com/sk89q/worldguard/protection/regions/QueryCache.java @@ -24,7 +24,9 @@ import com.sk89q.worldguard.protection.ApplicableRegionSet; import com.sk89q.worldguard.protection.RegionResultSet; import com.sk89q.worldguard.protection.managers.RegionManager; +import com.sk89q.worldguard.protection.regions.RegionQuery.QueryOption; +import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; @@ -38,7 +40,7 @@ */ public class QueryCache { - private final ConcurrentMap cache = new ConcurrentHashMap<>(16, 0.75f, 2); + private final ConcurrentMap> cache = new ConcurrentHashMap<>(16, 0.75f, 2); /** * Get from the cache a {@code ApplicableRegionSet} if an entry exists; @@ -46,20 +48,16 @@ public class QueryCache { * * @param manager the region manager * @param location the location + * @param option the option * @return a result */ - public ApplicableRegionSet queryContains(RegionManager manager, Location location) { + public ApplicableRegionSet queryContains(RegionManager manager, Location location, QueryOption option) { checkNotNull(manager); checkNotNull(location); + checkNotNull(option); CacheKey key = new CacheKey(location); - ApplicableRegionSet result = cache.get(key); - if (result == null) { - result = manager.getApplicableRegions(location.toVector().toBlockPoint()); - cache.put(key, result); - } - - return result; + return cache.compute(key, (k, v) -> option.createCache(manager, location, v)).get(option); } /** diff --git a/worldguard-core/src/main/java/com/sk89q/worldguard/protection/regions/RegionQuery.java b/worldguard-core/src/main/java/com/sk89q/worldguard/protection/regions/RegionQuery.java index 03a41a0f..2ee030c1 100644 --- a/worldguard-core/src/main/java/com/sk89q/worldguard/protection/regions/RegionQuery.java +++ b/worldguard-core/src/main/java/com/sk89q/worldguard/protection/regions/RegionQuery.java @@ -21,6 +21,7 @@ import static com.google.common.base.Preconditions.checkNotNull; +import com.google.common.collect.ImmutableList; import com.sk89q.worldedit.util.Location; import com.sk89q.worldedit.world.World; import com.sk89q.worldguard.LocalPlayer; @@ -39,8 +40,16 @@ import com.sk89q.worldguard.protection.flags.StateFlag; import com.sk89q.worldguard.protection.flags.StateFlag.State; import com.sk89q.worldguard.protection.managers.RegionManager; +import com.sk89q.worldguard.protection.managers.index.RegionIndex; +import com.sk89q.worldguard.protection.util.NormativeOrders; +import com.sk89q.worldguard.protection.util.RegionCollectionConsumer; import java.util.Collection; +import java.util.EnumMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; import javax.annotation.Nullable; @@ -74,17 +83,37 @@ public RegionQuery(QueryCache cache) { /** * Query for regions containing the given location. * - *

An instance of {@link RegionResultSet} will always be returned, + *

{@link QueryOption#COMPUTE_PARENTS} is used.

+ * + *

An instance of {@link ApplicableRegionSet} will always be returned, * even if regions are disabled or region data failed to load. An - * appropriate "virtual" set will be returned in such a case - * (for example, if regions are disabled, the returned set - * would permit all activities).

+ * appropriate "virtual" set will be returned in such a case (for example, + * if regions are disabled, the returned set would permit all + * activities).

* * @param location the location * @return a region set */ public ApplicableRegionSet getApplicableRegions(Location location) { + return getApplicableRegions(location, QueryOption.COMPUTE_PARENTS); + } + + /** + * Query for regions containing the given location. + * + *

An instance of {@link ApplicableRegionSet} will always be returned, + * even if regions are disabled or region data failed to load. An + * appropriate "virtual" set will be returned in such a case (for example, + * if regions are disabled, the returned set would permit all + * activities).

+ * + * @param location the location + * @param option the option + * @return a region set + */ + public ApplicableRegionSet getApplicableRegions(Location location, QueryOption option) { checkNotNull(location); + checkNotNull(option); World world = (World) location.getExtent(); WorldConfiguration worldConfig = config.get(world); @@ -95,7 +124,7 @@ public ApplicableRegionSet getApplicableRegions(Location location) { RegionManager manager = WorldGuard.getInstance().getPlatform().getRegionContainer().get((World) location.getExtent()); if (manager != null) { - return cache.queryContains(manager, location); + return cache.queryContains(manager, location, option); } else { return FailedLoadRegionSet.getInstance(); } @@ -465,4 +494,150 @@ public Collection queryAllValues(Location location, @Nullable RegionAssoc return getApplicableRegions(location).queryAllValues(associable, flag); } + /** + * Options for constructing a region set via + * {@link #getApplicableRegions(Location, QueryOption)} for example. + */ + public enum QueryOption { + /** + * Constructs a region set that does not include parent regions and + * may be left unsorted (but a cached, sorted set of the same regions + * may be returned). + */ + NONE(false) { + @Override + public List constructResult(Set applicable) { + return ImmutableList.copyOf(applicable); + } + + @Override + Map createCache(RegionManager manager, Location location, Map cache) { + if (cache == null) { + cache = new EnumMap<>(QueryOption.class); + cache.put(QueryOption.NONE, manager.getApplicableRegions(location.toVector().toBlockPoint(), QueryOption.NONE)); + } + + // If c != null, we can assume that Option.NONE is present. + return cache; + } + }, + + /** + * Constructs a region set that does not include parent regions and is + * sorted by {@link NormativeOrders}. + */ + SORT(false) { + @Override + Map createCache(RegionManager manager, Location location, Map cache) { + if (cache == null) { + Map newCache = new EnumMap<>(QueryOption.class); + ApplicableRegionSet result = manager.getApplicableRegions(location.toVector().toBlockPoint(), QueryOption.SORT); + newCache.put(QueryOption.NONE, result); + newCache.put(QueryOption.SORT, result); + return newCache; + } else { + // If c != null, we can assume that Option.NONE is present. + cache.computeIfAbsent(QueryOption.SORT, k -> new RegionResultSet(cache.get(QueryOption.NONE).getRegions(), manager.getRegion("__global__"))); + return cache; + } + } + }, + + /** + * Constructs a region set that includes parent regions and is sorted by + * {@link NormativeOrders}. + */ + COMPUTE_PARENTS(true) { + @Override + Map createCache(RegionManager manager, Location location, Map cache) { + if (cache == null) { + Map newCache = new EnumMap<>(QueryOption.class); + ApplicableRegionSet noParResult = manager.getApplicableRegions(location.toVector().toBlockPoint(), QueryOption.NONE); + Set noParRegions = noParResult.getRegions(); + Set regions = new HashSet<>(); + noParRegions.forEach(new RegionCollectionConsumer(regions, true)::apply); + ApplicableRegionSet result = new RegionResultSet(regions, manager.getRegion("__global__")); + + if (regions.size() == noParRegions.size()) { + newCache.put(QueryOption.NONE, result); + newCache.put(QueryOption.SORT, result); + } else { + newCache.put(QueryOption.NONE, noParResult); + } + + newCache.put(QueryOption.COMPUTE_PARENTS, result); + return newCache; + } + + cache.computeIfAbsent(QueryOption.COMPUTE_PARENTS, k -> { + Set regions = new HashSet<>(); + ApplicableRegionSet result = cache.get(QueryOption.SORT); + boolean sorted = true; + + if (result == null) { + // If c != null, we can assume that Option.NONE is present. + result = cache.get(QueryOption.NONE); + sorted = false; + } + + Set noParRegions = result.getRegions(); + noParRegions.forEach(new RegionCollectionConsumer(regions, true)::apply); + + if (sorted && regions.size() == noParRegions.size()) { + return result; + } + + result = new RegionResultSet(regions, manager.getRegion("__global__")); + + if (regions.size() == noParRegions.size()) { + cache.put(QueryOption.SORT, result); + } + + return result; + }); + return cache; + } + }; + + private final boolean collectParents; + + QueryOption(boolean collectParents) { + this.collectParents = collectParents; + } + + /** + * Create a {@link RegionCollectionConsumer} with the given collection + * used for the {@link RegionIndex}. Internal API. + * + * @param collection the collection + * @return a region collection consumer + */ + public RegionCollectionConsumer createIndexConsumer(Collection collection) { + return new RegionCollectionConsumer(collection, collectParents); + } + + /** + * Convert the set of regions to a list. Sort and add parents if + * necessary. Internal API. + * + * @param applicable the set of regions + * @return a list of regions + */ + public List constructResult(Set applicable) { + return NormativeOrders.fromSet(applicable); + } + + /** + * Create (if null) or update the given cache map with at least an entry + * for this option if necessary and return it. + * + * @param manager the manager + * @param location the location + * @param cache the cache map + * @return a cache map + */ + abstract Map createCache(RegionManager manager, Location location, Map cache); + + } + } diff --git a/worldguard-core/src/main/java/com/sk89q/worldguard/protection/util/RegionCollectionConsumer.java b/worldguard-core/src/main/java/com/sk89q/worldguard/protection/util/RegionCollectionConsumer.java index a36554ff..7c23a148 100644 --- a/worldguard-core/src/main/java/com/sk89q/worldguard/protection/util/RegionCollectionConsumer.java +++ b/worldguard-core/src/main/java/com/sk89q/worldguard/protection/util/RegionCollectionConsumer.java @@ -35,7 +35,7 @@ */ public class RegionCollectionConsumer implements Predicate { - private final Collection collection; + private final Collection collection; private final boolean addParents; /** @@ -44,7 +44,7 @@ public class RegionCollectionConsumer implements Predicate { * @param collection the collection to add regions to * @param addParents true to also add the parents to the collection */ - public RegionCollectionConsumer(Collection collection, boolean addParents) { + public RegionCollectionConsumer(Collection collection, boolean addParents) { checkNotNull(collection); this.collection = collection; diff --git a/worldguard-core/src/test/java/com/sk89q/worldguard/protection/RegionOverlapTest.java b/worldguard-core/src/test/java/com/sk89q/worldguard/protection/RegionOverlapTest.java index 7ddd5fd8..c9955b29 100644 --- a/worldguard-core/src/test/java/com/sk89q/worldguard/protection/RegionOverlapTest.java +++ b/worldguard-core/src/test/java/com/sk89q/worldguard/protection/RegionOverlapTest.java @@ -212,7 +212,7 @@ public void testNonPlayerBuildAccessInOneRegion(boolean useMaxPriorityAssociatio assertTrue(appl.testState(assoc, Flags.BUILD)); // Inside fountain appl = manager.getApplicableRegions(inFountain); - assertFalse(appl.testState(assoc, Flags.BUILD)); + assertTrue(appl.testState(assoc, Flags.BUILD)); } @ParameterizedTest(name = "useMaxPriorityAssociation={0}")