Skip to content

Commit 40459ba

Browse files
committed
Cache redundant permission checks
1 parent 42827d4 commit 40459ba

2 files changed

Lines changed: 42 additions & 8 deletions

File tree

cloud-core/src/main/java/org/incendo/cloud/CommandManager.java

Lines changed: 38 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,13 @@
2929
import java.util.HashSet;
3030
import java.util.LinkedList;
3131
import java.util.List;
32+
import java.util.Map;
3233
import java.util.Objects;
3334
import java.util.Set;
35+
import java.util.concurrent.ConcurrentHashMap;
3436
import java.util.concurrent.atomic.AtomicReference;
3537
import java.util.function.Consumer;
38+
import java.util.function.Function;
3639
import java.util.stream.Collectors;
3740
import org.apiguardian.api.API;
3841
import org.checkerframework.checker.nullness.qual.NonNull;
@@ -378,6 +381,21 @@ public boolean hasCapability(final @NonNull CloudCapability capability) {
378381
return Collections.unmodifiableSet(new HashSet<>(this.capabilities));
379382
}
380383

384+
private final ThreadLocal<Map<Pair<C, Permission>, PermissionResult>> threadLocalPermissionCache =
385+
ThreadLocal.withInitial(ConcurrentHashMap::new);
386+
387+
@SuppressWarnings("rawtypes")
388+
private @NonNull <T> PermissionResult testPermissionCaching(
389+
final @NonNull C sender,
390+
final @NonNull T permission,
391+
final @NonNull Function<Pair<C, T>, @NonNull PermissionResult> tester
392+
) {
393+
if (!this.settings.get(ManagerSetting.REDUCE_REDUNDANT_PERMISSION_CHECKS)) {
394+
return tester.apply(Pair.of(sender, permission));
395+
}
396+
return this.threadLocalPermissionCache.get().computeIfAbsent((Pair) Pair.of(sender, permission), (Function) tester);
397+
}
398+
381399
/**
382400
* Checks if the command sender has the required permission and returns the result.
383401
*
@@ -386,31 +404,44 @@ public boolean hasCapability(final @NonNull CloudCapability capability) {
386404
* @return a {@link PermissionResult} representing whether the sender has the permission
387405
*/
388406
@API(status = API.Status.STABLE)
389-
@SuppressWarnings("unchecked")
390407
public @NonNull PermissionResult testPermission(
391408
final @NonNull C sender,
392409
final @NonNull Permission permission
393410
) {
411+
try {
412+
return this.testPermission_(sender, permission);
413+
} finally {
414+
this.threadLocalPermissionCache.get().clear();
415+
}
416+
}
417+
418+
@SuppressWarnings("unchecked")
419+
private @NonNull PermissionResult testPermission_(final @NonNull C sender, final @NonNull Permission permission) {
394420
if (permission instanceof PredicatePermission) {
395-
return ((PredicatePermission<C>) permission).testPermission(sender);
421+
return this.testPermissionCaching(sender, (PredicatePermission<C>) permission, pair -> {
422+
return pair.second().testPermission(pair.first());
423+
});
396424
} else if (permission instanceof OrPermission) {
397425
for (final Permission innerPermission : permission.permissions()) {
398426
final PermissionResult result = this.testPermission(sender, innerPermission);
399427
if (result.allowed()) {
400-
return result; // short circuit the first true result
428+
return result;
401429
}
402430
}
403-
return PermissionResult.denied(permission); // none returned true
431+
return PermissionResult.denied(permission);
404432
} else if (permission instanceof AndPermission) {
405433
for (final Permission innerPermission : permission.permissions()) {
406434
final PermissionResult result = this.testPermission(sender, innerPermission);
407435
if (!result.allowed()) {
408-
return result; // short circuit the first false result
436+
return result;
409437
}
410438
}
411-
return PermissionResult.allowed(permission); // all returned true
439+
return PermissionResult.allowed(permission);
412440
}
413-
return PermissionResult.of(permission.isEmpty() || this.hasPermission(sender, permission.permissionString()), permission);
441+
return this.testPermissionCaching(sender, permission, pair -> {
442+
return PermissionResult.of(
443+
pair.second().isEmpty() || this.hasPermission(pair.first(), pair.second().permissionString()), pair.second());
444+
});
414445
}
415446

416447
/**

cloud-core/src/main/java/org/incendo/cloud/setting/ManagerSetting.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,5 +69,8 @@ public enum ManagerSetting implements Setting {
6969
* {@link org.incendo.cloud.exception.NoPermissionException}.
7070
*/
7171
@API(status = API.Status.EXPERIMENTAL)
72-
HIDE_COMMAND_EXISTENCE
72+
HIDE_COMMAND_EXISTENCE,
73+
74+
@API(status = API.Status.EXPERIMENTAL)
75+
REDUCE_REDUNDANT_PERMISSION_CHECKS
7376
}

0 commit comments

Comments
 (0)