feat(core): cache permission results during some operations that may check the same permissions repeatedly

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<C> implements Stateful<RegistrationState>,
     private final Set<CloudCapability> capabilities = new HashSet<>();
     private final ExceptionController<C> exceptionController = new ExceptionController<>();
     private final CommandExecutor<C> commandExecutor;
+    private final ThreadLocalPermissionCache<C> threadLocalPermissionCache = new ThreadLocalPermissionCache<>(this.settings);
 
     private CaptionFormatter<C, String> captionVariableReplacementHandler = CaptionFormatter.placeholderReplacing();
     private CommandSyntaxFormatter<C> commandSyntaxFormatter = new StandardCommandSyntaxFormatter<>(this);
@@ -176,6 +178,10 @@ protected CommandManager(
         return this.commandExecutor;
     }
 
+    final @NonNull ThreadLocalPermissionCache<C> 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<C>) permission).testPermission(sender);
+            return this.threadLocalPermissionCache.testPermissionCaching(sender, (PredicatePermission<C>) 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());
+        });
     }
 
     /**

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<CommandNode<C>> 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<CommandNode<C>> 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<C> node, final C sender) {

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<C> {
+
+    private final ThreadLocal<Pair<Map<Pair<C, Permission>, PermissionResult>, AtomicInteger>> threadLocalPermissionCache =
+            ThreadLocal.withInitial(() -> Pair.of(new HashMap<>(), new AtomicInteger(0)));
+    private final Configurable<ManagerSetting> settings;
+
+    /**
+     * Create a new cache.
+     *
+     * @param settings settings
+     */
+    public ThreadLocalPermissionCache(final Configurable<ManagerSetting> settings) {
+        this.settings = settings;
+    }
+
+    /**
+     * Perform an action in a cached scope.
+     *
+     * @param action action
+     * @param <T>    result type
+     * @return result
+     */
+    public <T> T withPermissionCache(final Supplier<T> 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<Map<Pair<C, Permission>, 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 <T>        permission type
+     * @return permission result
+     */
+    @SuppressWarnings({"rawtypes", "unchecked"})
+    public @NonNull <T> PermissionResult testPermissionCaching(
+            final @NonNull C sender,
+            final @NonNull T permission,
+            final @NonNull Function<Pair<C, T>, @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);
+    }
+}

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
 }