From 37367fe91f568eb0ea04e1bebabbd3c098d43a02 Mon Sep 17 00:00:00 2001 From: Luck Date: Fri, 13 Dec 2019 13:56:34 +0000 Subject: [PATCH] Invalidate QueryOptions cache when op status changes --- .../bukkit/context/BukkitContextManager.java | 3 +- .../inject/permissible/LPPermissible.java | 32 ++++++++++++++----- .../nukkit/context/NukkitContextManager.java | 3 +- .../inject/permissible/LPPermissible.java | 32 ++++++++++++++----- 4 files changed, 50 insertions(+), 20 deletions(-) diff --git a/bukkit/src/main/java/me/lucko/luckperms/bukkit/context/BukkitContextManager.java b/bukkit/src/main/java/me/lucko/luckperms/bukkit/context/BukkitContextManager.java index 5cdc4c912..380f292f8 100644 --- a/bukkit/src/main/java/me/lucko/luckperms/bukkit/context/BukkitContextManager.java +++ b/bukkit/src/main/java/me/lucko/luckperms/bukkit/context/BukkitContextManager.java @@ -32,7 +32,6 @@ import me.lucko.luckperms.common.cache.LoadingMap; import me.lucko.luckperms.common.config.ConfigKeys; import me.lucko.luckperms.common.context.ContextManager; import me.lucko.luckperms.common.context.QueryOptionsCache; -import me.lucko.luckperms.common.context.QueryOptionsSupplier; import me.lucko.luckperms.common.util.CaffeineFactory; import net.luckperms.api.context.ImmutableContextSet; @@ -70,7 +69,7 @@ public class BukkitContextManager extends ContextManager { } @Override - public QueryOptionsSupplier getCacheFor(Player subject) { + public QueryOptionsCache getCacheFor(Player subject) { if (subject == null) { throw new NullPointerException("subject"); } diff --git a/bukkit/src/main/java/me/lucko/luckperms/bukkit/inject/permissible/LPPermissible.java b/bukkit/src/main/java/me/lucko/luckperms/bukkit/inject/permissible/LPPermissible.java index bba67b0f1..8e75a0e96 100644 --- a/bukkit/src/main/java/me/lucko/luckperms/bukkit/inject/permissible/LPPermissible.java +++ b/bukkit/src/main/java/me/lucko/luckperms/bukkit/inject/permissible/LPPermissible.java @@ -29,13 +29,15 @@ import com.google.common.collect.ImmutableList; import me.lucko.luckperms.bukkit.LPBukkitPlugin; import me.lucko.luckperms.bukkit.calculator.DefaultsProcessor; +import me.lucko.luckperms.bukkit.context.BukkitContextManager; import me.lucko.luckperms.common.calculator.result.TristateResult; import me.lucko.luckperms.common.config.ConfigKeys; -import me.lucko.luckperms.common.context.QueryOptionsSupplier; +import me.lucko.luckperms.common.context.QueryOptionsCache; import me.lucko.luckperms.common.model.User; import me.lucko.luckperms.common.util.ImmutableCollectors; import me.lucko.luckperms.common.verbose.event.PermissionCheckEvent; +import net.luckperms.api.query.QueryOptions; import net.luckperms.api.util.Tristate; import org.bukkit.entity.Player; @@ -93,7 +95,7 @@ public class LPPermissible extends PermissibleBase { private final LPBukkitPlugin plugin; // caches context lookups for the player - private final QueryOptionsSupplier queryOptionsSupplier; + private final QueryOptionsCache queryOptionsSupplier; // the players previous permissible. (the one they had before this one was injected) private PermissibleBase oldPermissible = null; @@ -163,8 +165,14 @@ public class LPPermissible extends PermissibleBase { throw new NullPointerException("permission"); } - Tristate ts = this.user.getCachedData().getPermissionData(this.queryOptionsSupplier.getQueryOptions()).checkPermission(permission, PermissionCheckEvent.Origin.PLATFORM_PERMISSION_CHECK).result(); - return ts != Tristate.UNDEFINED ? ts.asBoolean() : Permission.DEFAULT_PERMISSION.getValue(isOp()); + QueryOptions queryOptions = this.queryOptionsSupplier.getQueryOptions(); + Tristate ts = this.user.getCachedData().getPermissionData(queryOptions).checkPermission(permission, PermissionCheckEvent.Origin.PLATFORM_PERMISSION_CHECK).result(); + if (ts != Tristate.UNDEFINED) { + return ts.asBoolean(); + } + + boolean isOp = queryOptions.option(BukkitContextManager.OP_OPTION).orElse(false); + return Permission.DEFAULT_PERMISSION.getValue(isOp); } @Override @@ -173,15 +181,17 @@ public class LPPermissible extends PermissibleBase { throw new NullPointerException("permission"); } - Tristate ts = this.user.getCachedData().getPermissionData(this.queryOptionsSupplier.getQueryOptions()).checkPermission(permission.getName(), PermissionCheckEvent.Origin.PLATFORM_PERMISSION_CHECK).result(); + QueryOptions queryOptions = this.queryOptionsSupplier.getQueryOptions(); + Tristate ts = this.user.getCachedData().getPermissionData(queryOptions).checkPermission(permission.getName(), PermissionCheckEvent.Origin.PLATFORM_PERMISSION_CHECK).result(); if (ts != Tristate.UNDEFINED) { return ts.asBoolean(); } + boolean isOp = queryOptions.option(BukkitContextManager.OP_OPTION).orElse(false); if (this.plugin.getConfiguration().get(ConfigKeys.APPLY_BUKKIT_DEFAULT_PERMISSIONS)) { - return permission.getDefault().getValue(isOp()); + return permission.getDefault().getValue(isOp); } else { - return Permission.DEFAULT_PERMISSION.getValue(isOp()); + return Permission.DEFAULT_PERMISSION.getValue(isOp); } } @@ -294,7 +304,13 @@ public class LPPermissible extends PermissibleBase { @Override public void recalculatePermissions() { - // do nothing + // this method is called (among other times) when op status is updated. + // because we encapsulate op status within QueryOptions, we need to invalidate + // the contextmanager cache when op status changes. + // (#invalidate is a fast call) + this.queryOptionsSupplier.invalidate(); + + // but we don't need to do anything else in this method, unlike the CB impl. } @Override diff --git a/nukkit/src/main/java/me/lucko/luckperms/nukkit/context/NukkitContextManager.java b/nukkit/src/main/java/me/lucko/luckperms/nukkit/context/NukkitContextManager.java index 909cf99eb..15ababbc7 100644 --- a/nukkit/src/main/java/me/lucko/luckperms/nukkit/context/NukkitContextManager.java +++ b/nukkit/src/main/java/me/lucko/luckperms/nukkit/context/NukkitContextManager.java @@ -31,7 +31,6 @@ import me.lucko.luckperms.common.cache.LoadingMap; import me.lucko.luckperms.common.config.ConfigKeys; import me.lucko.luckperms.common.context.ContextManager; import me.lucko.luckperms.common.context.QueryOptionsCache; -import me.lucko.luckperms.common.context.QueryOptionsSupplier; import me.lucko.luckperms.common.util.CaffeineFactory; import me.lucko.luckperms.nukkit.LPNukkitPlugin; @@ -70,7 +69,7 @@ public class NukkitContextManager extends ContextManager { } @Override - public QueryOptionsSupplier getCacheFor(Player subject) { + public QueryOptionsCache getCacheFor(Player subject) { if (subject == null) { throw new NullPointerException("subject"); } diff --git a/nukkit/src/main/java/me/lucko/luckperms/nukkit/inject/permissible/LPPermissible.java b/nukkit/src/main/java/me/lucko/luckperms/nukkit/inject/permissible/LPPermissible.java index 258d5ddf0..5a34d18e4 100644 --- a/nukkit/src/main/java/me/lucko/luckperms/nukkit/inject/permissible/LPPermissible.java +++ b/nukkit/src/main/java/me/lucko/luckperms/nukkit/inject/permissible/LPPermissible.java @@ -29,14 +29,16 @@ import com.google.common.collect.ImmutableList; import me.lucko.luckperms.common.calculator.result.TristateResult; import me.lucko.luckperms.common.config.ConfigKeys; -import me.lucko.luckperms.common.context.QueryOptionsSupplier; +import me.lucko.luckperms.common.context.QueryOptionsCache; import me.lucko.luckperms.common.model.User; import me.lucko.luckperms.common.util.ImmutableCollectors; import me.lucko.luckperms.common.verbose.event.PermissionCheckEvent; import me.lucko.luckperms.nukkit.LPNukkitPlugin; import me.lucko.luckperms.nukkit.calculator.DefaultsProcessor; +import me.lucko.luckperms.nukkit.context.NukkitContextManager; import me.lucko.luckperms.nukkit.inject.PermissionDefault; +import net.luckperms.api.query.QueryOptions; import net.luckperms.api.util.Tristate; import org.checkerframework.checker.nullness.qual.NonNull; @@ -94,7 +96,7 @@ public class LPPermissible extends PermissibleBase { private final LPNukkitPlugin plugin; // caches context lookups for the player - private final QueryOptionsSupplier queryOptionsSupplier; + private final QueryOptionsCache queryOptionsSupplier; // the players previous permissible. (the one they had before this one was injected) private PermissibleBase oldPermissible = null; @@ -164,8 +166,14 @@ public class LPPermissible extends PermissibleBase { throw new NullPointerException("permission"); } - Tristate ts = this.user.getCachedData().getPermissionData(this.queryOptionsSupplier.getQueryOptions()).checkPermission(permission, PermissionCheckEvent.Origin.PLATFORM_PERMISSION_CHECK).result(); - return ts != Tristate.UNDEFINED ? ts.asBoolean() : PermissionDefault.OP.getValue(isOp()); + QueryOptions queryOptions = this.queryOptionsSupplier.getQueryOptions(); + Tristate ts = this.user.getCachedData().getPermissionData(queryOptions).checkPermission(permission, PermissionCheckEvent.Origin.PLATFORM_PERMISSION_CHECK).result(); + if (ts != Tristate.UNDEFINED) { + return ts.asBoolean(); + } + + boolean isOp = queryOptions.option(NukkitContextManager.OP_OPTION).orElse(false); + return PermissionDefault.OP.getValue(isOp); } @Override @@ -174,16 +182,18 @@ public class LPPermissible extends PermissibleBase { throw new NullPointerException("permission"); } - Tristate ts = this.user.getCachedData().getPermissionData(this.queryOptionsSupplier.getQueryOptions()).checkPermission(permission.getName(), PermissionCheckEvent.Origin.PLATFORM_PERMISSION_CHECK).result(); + QueryOptions queryOptions = this.queryOptionsSupplier.getQueryOptions(); + Tristate ts = this.user.getCachedData().getPermissionData(queryOptions).checkPermission(permission.getName(), PermissionCheckEvent.Origin.PLATFORM_PERMISSION_CHECK).result(); if (ts != Tristate.UNDEFINED) { return ts.asBoolean(); } + boolean isOp = queryOptions.option(NukkitContextManager.OP_OPTION).orElse(false); if (this.plugin.getConfiguration().get(ConfigKeys.APPLY_NUKKIT_DEFAULT_PERMISSIONS)) { PermissionDefault def = PermissionDefault.fromPermission(permission); - return def != null && def.getValue(isOp()); + return def != null && def.getValue(isOp); } else { - return PermissionDefault.OP.getValue(isOp()); + return PermissionDefault.OP.getValue(isOp); } } @@ -280,7 +290,13 @@ public class LPPermissible extends PermissibleBase { @Override public void recalculatePermissions() { - // do nothing + // this method is called (among other times) when op status is updated. + // because we encapsulate op status within QueryOptions, we need to invalidate + // the contextmanager cache when op status changes. + // (#invalidate is a fast call) + this.queryOptionsSupplier.invalidate(); + + // but we don't need to do anything else in this method, unlike the CB impl. } @Override