diff --git a/cloud-core/src/main/java/org/incendo/cloud/CommandManager.java b/cloud-core/src/main/java/org/incendo/cloud/CommandManager.java index b063582ba..71a94508c 100644 --- a/cloud-core/src/main/java/org/incendo/cloud/CommandManager.java +++ b/cloud-core/src/main/java/org/incendo/cloud/CommandManager.java @@ -62,6 +62,7 @@ import org.incendo.cloud.injection.ParameterInjectorRegistry; import org.incendo.cloud.internal.CommandNode; import org.incendo.cloud.internal.CommandRegistrationHandler; +import org.incendo.cloud.internal.ThreadLocalPermissionCache; import org.incendo.cloud.meta.CommandMeta; import org.incendo.cloud.parser.ArgumentParser; import org.incendo.cloud.parser.ParserParameter; @@ -111,6 +112,7 @@ public abstract class CommandManager implements Stateful, private final Set capabilities = new HashSet<>(); private final ExceptionController exceptionController = new ExceptionController<>(); private final CommandExecutor commandExecutor; + private final ThreadLocalPermissionCache threadLocalPermissionCache = new ThreadLocalPermissionCache<>(this.settings); private CaptionFormatter captionVariableReplacementHandler = CaptionFormatter.placeholderReplacing(); private CommandSyntaxFormatter commandSyntaxFormatter = new StandardCommandSyntaxFormatter<>(this); @@ -176,6 +178,10 @@ protected CommandManager( return this.commandExecutor; } + final @NonNull ThreadLocalPermissionCache threadLocalPermissionCache() { + return this.threadLocalPermissionCache; + } + /** * Returns the suggestion factory. * @@ -386,31 +392,40 @@ public boolean hasCapability(final @NonNull CloudCapability capability) { * @return a {@link PermissionResult} representing whether the sender has the permission */ @API(status = API.Status.STABLE) - @SuppressWarnings("unchecked") public @NonNull PermissionResult testPermission( final @NonNull C sender, final @NonNull Permission permission ) { + return this.threadLocalPermissionCache.withPermissionCache(() -> this.testPermissionRaw(sender, permission)); + } + + @SuppressWarnings("unchecked") + private @NonNull PermissionResult testPermissionRaw(final @NonNull C sender, final @NonNull Permission permission) { if (permission instanceof PredicatePermission) { - return ((PredicatePermission) permission).testPermission(sender); + return this.threadLocalPermissionCache.testPermissionCaching(sender, (PredicatePermission) permission, pair -> { + return pair.second().testPermission(pair.first()); + }); } else if (permission instanceof OrPermission) { for (final Permission innerPermission : permission.permissions()) { - final PermissionResult result = this.testPermission(sender, innerPermission); + final PermissionResult result = this.testPermissionRaw(sender, innerPermission); if (result.allowed()) { - return result; // short circuit the first true result + return result; } } - return PermissionResult.denied(permission); // none returned true + return PermissionResult.denied(permission); } else if (permission instanceof AndPermission) { for (final Permission innerPermission : permission.permissions()) { - final PermissionResult result = this.testPermission(sender, innerPermission); + final PermissionResult result = this.testPermissionRaw(sender, innerPermission); if (!result.allowed()) { - return result; // short circuit the first false result + return result; } } - return PermissionResult.allowed(permission); // all returned true + return PermissionResult.allowed(permission); } - return PermissionResult.of(permission.isEmpty() || this.hasPermission(sender, permission.permissionString()), permission); + return this.threadLocalPermissionCache.testPermissionCaching(sender, permission, pair -> { + return PermissionResult.of( + pair.second().isEmpty() || this.hasPermission(pair.first(), pair.second().permissionString()), pair.second()); + }); } /** diff --git a/cloud-core/src/main/java/org/incendo/cloud/CommandTree.java b/cloud-core/src/main/java/org/incendo/cloud/CommandTree.java index f6b6f531a..4c6afda77 100644 --- a/cloud-core/src/main/java/org/incendo/cloud/CommandTree.java +++ b/cloud-core/src/main/java/org/incendo/cloud/CommandTree.java @@ -357,31 +357,33 @@ private Exception noPermissionOrSyntax( ); } - if (this.childPermitted(root, sender)) { - return new InvalidSyntaxException( - this.commandManager.commandSyntaxFormatter().apply(sender, (List) this.getComponentChain(root), root), - sender, this.getComponentChain(root) - ); - } + return this.commandManager.threadLocalPermissionCache().withPermissionCache(() -> { + if (this.childPermitted(root, sender)) { + return new InvalidSyntaxException( + this.commandManager.commandSyntaxFormatter().apply(sender, (List) this.getComponentChain(root), root), + sender, this.getComponentChain(root) + ); + } - final @Nullable List> parentChain = this.permittedParentChain(root, sender); - if (parentChain != null) { - return new InvalidSyntaxException( - this.commandManager.commandSyntaxFormatter().apply( - sender, - parentChain.stream().map(CommandNode::component) - .filter(Objects::nonNull).collect(Collectors.toList()), - root - ), - sender, this.getComponentChain(root) - ); - } + final @Nullable List> parentChain = this.permittedParentChain(root, sender); + if (parentChain != null) { + return new InvalidSyntaxException( + this.commandManager.commandSyntaxFormatter().apply( + sender, + parentChain.stream().map(CommandNode::component) + .filter(Objects::nonNull).collect(Collectors.toList()), + root + ), + sender, this.getComponentChain(root) + ); + } - return new NoPermissionException( - permissionResult, - sender, - this.getComponentChain(root) - ); + return new NoPermissionException( + permissionResult, + sender, + this.getComponentChain(root) + ); + }); } private boolean childPermitted(final CommandNode node, final C sender) { diff --git a/cloud-core/src/main/java/org/incendo/cloud/internal/ThreadLocalPermissionCache.java b/cloud-core/src/main/java/org/incendo/cloud/internal/ThreadLocalPermissionCache.java new file mode 100644 index 000000000..ae73b3d97 --- /dev/null +++ b/cloud-core/src/main/java/org/incendo/cloud/internal/ThreadLocalPermissionCache.java @@ -0,0 +1,104 @@ +// +// MIT License +// +// Copyright (c) 2024 Incendo +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all +// copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +// SOFTWARE. +// +package org.incendo.cloud.internal; + +import java.util.HashMap; +import java.util.Map; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Function; +import java.util.function.Supplier; +import org.apiguardian.api.API; +import org.checkerframework.checker.nullness.qual.NonNull; +import org.incendo.cloud.permission.Permission; +import org.incendo.cloud.permission.PermissionResult; +import org.incendo.cloud.setting.Configurable; +import org.incendo.cloud.setting.ManagerSetting; +import org.incendo.cloud.type.tuple.Pair; + +@API(status = API.Status.INTERNAL) +public final class ThreadLocalPermissionCache { + + private final ThreadLocal, PermissionResult>, AtomicInteger>> threadLocalPermissionCache = + ThreadLocal.withInitial(() -> Pair.of(new HashMap<>(), new AtomicInteger(0))); + private final Configurable settings; + + /** + * Create a new cache. + * + * @param settings settings + */ + public ThreadLocalPermissionCache(final Configurable settings) { + this.settings = settings; + } + + /** + * Perform an action in a cached scope. + * + * @param action action + * @param result type + * @return result + */ + public T withPermissionCache(final Supplier action) { + final boolean cache = this.settings.get(ManagerSetting.REDUCE_REDUNDANT_PERMISSION_CHECKS); + try { + if (cache) { + final int prev = this.threadLocalPermissionCache.get().second().getAndIncrement(); + if (prev == 0) { + // Cleanup from case where cache was enabled mid-permission check + this.threadLocalPermissionCache.get().first().clear(); + } + } + return action.get(); + } finally { + if (cache) { + final Pair, PermissionResult>, AtomicInteger> pair = this.threadLocalPermissionCache.get(); + if (pair.second().getAndDecrement() == 1) { + pair.first().clear(); + } + } + } + } + + /** + * Test permission caching. + * + * @param sender sender + * @param permission permission + * @param tester tester + * @param permission type + * @return permission result + */ + @SuppressWarnings({"rawtypes", "unchecked"}) + public @NonNull PermissionResult testPermissionCaching( + final @NonNull C sender, + final @NonNull T permission, + final @NonNull Function, @NonNull PermissionResult> tester + ) { + if (!this.settings.get(ManagerSetting.REDUCE_REDUNDANT_PERMISSION_CHECKS)) { + return tester.apply(Pair.of(sender, permission)); + } + return this.threadLocalPermissionCache.get().first() + .computeIfAbsent((Pair) Pair.of(sender, permission), (Function) tester); + } +} diff --git a/cloud-core/src/main/java/org/incendo/cloud/setting/ManagerSetting.java b/cloud-core/src/main/java/org/incendo/cloud/setting/ManagerSetting.java index 55dc6ac71..04b48fc78 100644 --- a/cloud-core/src/main/java/org/incendo/cloud/setting/ManagerSetting.java +++ b/cloud-core/src/main/java/org/incendo/cloud/setting/ManagerSetting.java @@ -69,5 +69,8 @@ public enum ManagerSetting implements Setting { * {@link org.incendo.cloud.exception.NoPermissionException}. */ @API(status = API.Status.EXPERIMENTAL) - HIDE_COMMAND_EXISTENCE + HIDE_COMMAND_EXISTENCE, + + @API(status = API.Status.EXPERIMENTAL) + REDUCE_REDUNDANT_PERMISSION_CHECKS }