Non-suspending ProtectionManager functions for online players

nova-api/src/main/java/xyz/xenondevs/nova/api/protection/ProtectionIntegration.java

@@ -159,7 +159,7 @@ default boolean canHurtEntity(@NotNull TileEntity tileEntity, @NotNull Entity en
     enum ExecutionMode {
 
         /**
-         * The thread will not be changed in order to call the methods
+         * The method can be called from any thread
          */
         NONE,
 
@@ -170,7 +170,10 @@ enum ExecutionMode {
 
         /**
          * The methods are never called from the server thread
+         * 
+         * @deprecated Use {@link #NONE} instead 
          */
+        @Deprecated
         ASYNC
 
     }

nova-hooks/nova-hook-worldguard/src/main/kotlin/xyz/xenondevs/nova/hook/impl/worldguard/BetterBukkitOfflinePlayer.kt

@@ -44,7 +44,7 @@ internal class BetterBukkitOfflinePlayer(
     }
 
     override fun hasPermission(perm: String): Boolean {
-        return (player.isOp && platform.globalStateManager[world].opPermissions) || PermissionManager.hasPermission(bukkitWorld, player, perm)
+        return (player.isOp && platform.globalStateManager[world].opPermissions) || PermissionManager.hasPermission(bukkitWorld, player, perm).get()
     }
 
     override fun getWorld(): World {

nova/src/main/kotlin/xyz/xenondevs/nova/integration/permission/PermissionManager.kt

@@ -8,11 +8,11 @@ import org.bukkit.OfflinePlayer
 import org.bukkit.World
 import org.bukkit.entity.Player
 import xyz.xenondevs.nova.LOGGER
-import xyz.xenondevs.nova.integration.HooksLoader
 import xyz.xenondevs.nova.initialize.DisableFun
 import xyz.xenondevs.nova.initialize.InitFun
-import xyz.xenondevs.nova.initialize.InternalInitStage
 import xyz.xenondevs.nova.initialize.InternalInit
+import xyz.xenondevs.nova.initialize.InternalInitStage
+import xyz.xenondevs.nova.integration.HooksLoader
 import xyz.xenondevs.nova.util.MINECRAFT_SERVER
 import java.time.Duration
 import java.util.*
@@ -43,10 +43,11 @@ object PermissionManager {
             10, 10,
             0, TimeUnit.MILLISECONDS,
             LinkedBlockingQueue(),
-            ThreadFactoryBuilder().setNameFormat("Nova Protection Worker - %s").build()
+            ThreadFactoryBuilder().setNameFormat("Nova Permission Worker - %s").build()
         )
 
         offlinePermissionCache = Caffeine.newBuilder()
+            .executor(executor)
             .expireAfterAccess(Duration.ofMinutes(30))
             .refreshAfterWrite(Duration.ofMinutes(1))
             .build { hasOfflinePermission(it.world, it.player, it.permission) }
@@ -60,56 +61,22 @@ object PermissionManager {
         executor.shutdown()
     }
 
-    /**
-     * Checks whether the player under the given [UUID][player] has the given [permission] in the given [world].
-     *
-     * This method will use [Player.hasPermission] if the player is online and otherwise use access the offline
-     * permission cache. If the permission is not cached yet, this method will return false and initiate a cache load.
-     */
-    fun hasPermission(world: World, player: UUID, permission: String): Boolean =
-        hasPermission(world, Bukkit.getOfflinePlayer(player), permission)
-
-    /**
-     * Checks whether the given [player] has the given [permission] in the given [world].
-     *
-     * This method will use [Player.hasPermission] if the player is online and otherwise use access the offline
-     * permission cache. If the permission is not cached yet, this method will return false and initiate a cache load.
-     */
-    fun hasPermission(world: World, player: OfflinePlayer, permission: String): Boolean {
-        // online-player permissions are cached by the permissions plugin
-        if (player.isOnline)
-            return player.player!!.hasPermission(permission)
-
-        val args = PermissionArgs(world, player, permission)
-
-        // don't initiate cache loads, as that would cause lag spikes due to database access from the main thread
-        val cachedResult = offlinePermissionCache.getIfPresent(args)
-        if (cachedResult == null) {
-            // load cache async
-            offlinePermissionCache.refresh(args)
-            // return false as we don't know the result yet
-            return false
-        }
-
-        return cachedResult
-    }
-
     /**
      * Checks whether the player under the given [UUID][player] has the given [permission] in the given [world].
      * 
      * This method will use [Player.hasPermission] if the player is online and otherwise use access the offline permission cache,
-     * which accessed the registered [PermissionIntegrations][PermissionIntegration] asynchronously.
+     * which accesses the registered [PermissionIntegrations][PermissionIntegration] asynchronously.
      */
-    fun hasPermissionAsync(world: World, player: UUID, permission: String): CompletableFuture<Boolean> =
-        hasPermissionAsync(world, Bukkit.getOfflinePlayer(player), permission)
+    fun hasPermission(world: World, player: UUID, permission: String): CompletableFuture<Boolean> =
+        hasPermission(world, Bukkit.getOfflinePlayer(player), permission)
 
     /**
      * Checks whether the given [player] has the given [permission] in the given [world].
      * 
      * This method will use [Player.hasPermission] if the player is online and otherwise use access the offline permission cache,
-     * which accessed the registered [PermissionIntegrations][PermissionIntegration] asynchronously.
+     * which accesses the registered [PermissionIntegrations][PermissionIntegration] asynchronously.
      */
-    fun hasPermissionAsync(world: World, player: OfflinePlayer, permission: String): CompletableFuture<Boolean> {
+    fun hasPermission(world: World, player: OfflinePlayer, permission: String): CompletableFuture<Boolean> {
         // online-player permissions are cached by the permissions plugin
         if (player.isOnline)
             return CompletableFuture.completedFuture(player.player!!.hasPermission(permission))

nova/src/main/kotlin/xyz/xenondevs/nova/integration/protection/ProtectionManager.kt

@@ -8,6 +8,7 @@ import org.bukkit.Bukkit
 import org.bukkit.Location
 import org.bukkit.OfflinePlayer
 import org.bukkit.entity.Entity
+import org.bukkit.entity.Player
 import org.bukkit.inventory.ItemStack
 import xyz.xenondevs.nova.NOVA_PLUGIN
 import xyz.xenondevs.nova.api.ApiTileEntityWrapper
@@ -20,8 +21,9 @@ import xyz.xenondevs.nova.initialize.InternalInitStage
 import xyz.xenondevs.nova.integration.HooksLoader
 import xyz.xenondevs.nova.tileentity.TileEntity
 import xyz.xenondevs.nova.util.concurrent.CombinedBooleanFuture
-import xyz.xenondevs.nova.util.concurrent.completeServerThread
+import xyz.xenondevs.nova.util.concurrent.isServerThread
 import xyz.xenondevs.nova.util.isBetweenXZ
+import xyz.xenondevs.nova.util.runTask
 import java.time.Duration
 import java.util.concurrent.CompletableFuture
 import java.util.concurrent.ExecutorService
@@ -53,20 +55,23 @@ private data class CanUseItemTileArgs(override val tileEntity: TileEntity, val i
 private data class CanInteractWithEntityUserArgs(override val player: OfflinePlayer, val entity: Entity, val item: ItemStack?) : ProtectionArgs {
     override val location: Location = entity.location
 }
+
 private data class CanInteractWithEntityTileArgs(override val tileEntity: TileEntity, val entity: Entity, val item: ItemStack?) : ProtectionArgsTileEntity {
     override val location: Location = entity.location
 }
+
 private data class CanHurtEntityUserArgs(override val player: OfflinePlayer, val entity: Entity, val item: ItemStack?) : ProtectionArgs {
     override val location: Location = entity.location
 }
+
 private data class CanHurtEntityTileArgs(override val tileEntity: TileEntity, val entity: Entity, val item: ItemStack?) : ProtectionArgsTileEntity {
     override val location: Location = entity.location
 }
 //</editor-fold>
 
 /**
  * Handles protection checks using registered [ProtectionIntegrations][ProtectionIntegration].
- * 
+ *
  * Protection checks are cached for 60s after the last access.
  * If all protection integrations [can be called asynchronously][ExecutionMode], commonly used protection checks will be refreshed
  * every 30s.
@@ -92,16 +97,13 @@ object ProtectionManager {
 
     @InitFun
     private fun init() {
-        //<editor-fold desc="executor service">
         executor = ThreadPoolExecutor(
             10, 10,
             0, TimeUnit.MILLISECONDS,
             LinkedBlockingQueue(),
             ThreadFactoryBuilder().setNameFormat("Nova Protection Worker - %s").build()
         )
-        //</editor-fold>
 
-        //<editor-fold desc="cache">
         val cacheBuilder = Caffeine.newBuilder()
             .executor(executor)
             .expireAfterAccess(Duration.ofSeconds(60))
@@ -122,7 +124,6 @@ object ProtectionManager {
         cacheCanInteractWithEntityTile = cacheBuilder.build { checkProtection(it) { canInteractWithEntity(it.apiTileEntity, it.entity, it.item) } }
         cacheCanHurtEntityUser = cacheBuilder.build { checkProtection(it) { canHurtEntity(it.player, it.entity, it.item) } }
         cacheCanHurtEntityTile = cacheBuilder.build { checkProtection(it) { canHurtEntity(it.apiTileEntity, it.entity, it.item) } }
-        //</editor-fold>
     }
 
     @DisableFun
@@ -131,89 +132,125 @@ object ProtectionManager {
     }
 
     /**
-     * Checks if the [tileEntity] can place that [item] at that [location]
+     * Checks if the [tileEntity] can place that [item] at that [location].
      */
     suspend fun canPlace(tileEntity: TileEntity, item: ItemStack, location: Location): Boolean {
         if (tileEntity.owner == null) return true
         return cacheCanPlaceTile.get(CanPlaceTileArgs(tileEntity, item.clone(), location.clone())).await()
     }
 
     /**
-     * Checks if the [player] can place that [item] at that [location]
+     * Checks if the [player] can place that [item] at that [location].
      */
     suspend fun canPlace(player: OfflinePlayer, item: ItemStack, location: Location): Boolean =
         cacheCanPlaceUser.get(CanPlaceUserArgs(player, item.clone(), location.clone())).await()
 
     /**
-     * Checks if that [tileEntity] can break a block at that [location] using that [item]
+     * Checks if the [player] can place that [item] at that [location].
+     */
+    fun canPlace(player: Player, item: ItemStack, location: Location): Boolean =
+        cacheCanPlaceUser.get(CanPlaceUserArgs(player, item.clone(), location.clone())).get()
+
+    /**
+     * Checks if that [tileEntity] can break a block at that [location] using that [item].
      */
     suspend fun canBreak(tileEntity: TileEntity, item: ItemStack?, location: Location): Boolean {
         if (tileEntity.owner == null) return true
         return cacheCanBreakTile.get(CanBreakTileArgs(tileEntity, item?.clone(), location.clone())).await()
     }
 
     /**
-     * Checks if that [player] can break a block at that [location] using that [item]
+     * Checks if that [player] can break a block at that [location] using that [item].
      */
     suspend fun canBreak(player: OfflinePlayer, item: ItemStack?, location: Location): Boolean =
         cacheCanBreakUser.get(CanBreakUserArgs(player, item?.clone(), location.clone())).await()
 
     /**
-     * Checks if the [tileEntity] can interact with a block at that [location] using that [item]
+     * Checks if that [player] can break a block at that [location] using that [item].
+     */
+    fun canBreak(player: Player, item: ItemStack?, location: Location): Boolean =
+        cacheCanBreakUser.get(CanBreakUserArgs(player, item?.clone(), location.clone())).get()
+
+    /**
+     * Checks if the [tileEntity] can interact with a block at that [location] using that [item].
      */
     suspend fun canUseBlock(tileEntity: TileEntity, item: ItemStack?, location: Location): Boolean {
         if (tileEntity.owner == null) return true
         return cacheCanUseBlockTile.get(CanUseBlockTileArgs(tileEntity, item?.clone(), location.clone())).await()
     }
 
     /**
-     * Checks if the [player] can interact with a block at that [location] using that [item]
+     * Checks if the [player] can interact with a block at that [location] using that [item].
      */
     suspend fun canUseBlock(player: OfflinePlayer, item: ItemStack?, location: Location): Boolean =
         cacheCanUseBlockUser.get(CanUseBlockUserArgs(player, item?.clone(), location.clone())).await()
 
     /**
-     * Checks if the [tileEntity] can use that [item] at that [location]
+     * Checks if the [player] can interact with a block at that [location] using that [item].
+     */
+    fun canUseBlock(player: Player, item: ItemStack?, location: Location): Boolean =
+        cacheCanUseBlockUser.get(CanUseBlockUserArgs(player, item?.clone(), location.clone())).get()
+
+    /**
+     * Checks if the [tileEntity] can use that [item] at that [location].
      */
     suspend fun canUseItem(tileEntity: TileEntity, item: ItemStack, location: Location): Boolean {
         if (tileEntity.owner == null) return true
         return cacheCanUseItemTile.get(CanUseItemTileArgs(tileEntity, item.clone(), location.clone())).await()
     }
 
     /**
-     * Checks if the [player] can use that [item] at that [location]
+     * Checks if the [player] can use that [item] at that [location].
      */
     suspend fun canUseItem(player: OfflinePlayer, item: ItemStack, location: Location): Boolean =
         cacheCanUseItemUser.get(CanUseItemUserArgs(player, item.clone(), location.clone())).await()
 
     /**
-     * Checks if the [tileEntity] can interact with the [entity] wile holding that [item]
+     * Checks if the [player] can use that [item] at that [location].
+     */
+    fun canUseItem(player: Player, item: ItemStack, location: Location): Boolean =
+        cacheCanUseItemUser.get(CanUseItemUserArgs(player, item.clone(), location.clone())).get()
+
+    /**
+     * Checks if the [tileEntity] can interact with the [entity] wile holding that [item].
      */
     suspend fun canInteractWithEntity(tileEntity: TileEntity, entity: Entity, item: ItemStack?): Boolean {
         if (tileEntity.owner == null) return true
         return cacheCanInteractWithEntityTile.get(CanInteractWithEntityTileArgs(tileEntity, entity, item?.clone())).await()
     }
 
     /**
-     * Checks if the [player] can interact with the [entity] while holding that [item]
+     * Checks if the [player] can interact with the [entity] while holding that [item].
      */
     suspend fun canInteractWithEntity(player: OfflinePlayer, entity: Entity, item: ItemStack?): Boolean =
         cacheCanInteractWithEntityUser.get(CanInteractWithEntityUserArgs(player, entity, item?.clone())).await()
 
     /**
-     * Checks if the [tileEntity] can hurt the [entity] with this [item]
+     * Checks if the [player] can interact with the [entity] while holding that [item].
+     */
+    fun canInteractWithEntity(player: Player, entity: Entity, item: ItemStack?): Boolean =
+        cacheCanInteractWithEntityUser.get(CanInteractWithEntityUserArgs(player, entity, item?.clone())).get()
+
+    /**
+     * Checks if the [tileEntity] can hurt the [entity] with this [item].
      */
     suspend fun canHurtEntity(tileEntity: TileEntity, entity: Entity, item: ItemStack?): Boolean {
         if (tileEntity.owner == null) return true
         return cacheCanHurtEntityTile.get(CanHurtEntityTileArgs(tileEntity, entity, item?.clone())).await()
     }
 
     /**
-     * Checks if the [player] can hurt the [entity] with this [item]
+     * Checks if the [player] can hurt the [entity] with this [item].
      */
     suspend fun canHurtEntity(player: OfflinePlayer, entity: Entity, item: ItemStack?): Boolean =
         cacheCanHurtEntityUser.get(CanHurtEntityUserArgs(player, entity, item?.clone())).await()
 
+    /**
+     * Checks if the [player] can hurt the [entity] with this [item].
+     */
+    fun canHurtEntity(player: Player, entity: Entity, item: ItemStack?): Boolean =
+        cacheCanHurtEntityUser.get(CanHurtEntityUserArgs(player, entity, item?.clone())).get()
+
     private fun checkProtection(
         args: ProtectionArgs,
         check: ProtectionIntegration.() -> Boolean
@@ -222,8 +259,25 @@ object ProtectionManager {
             return CompletableFuture.completedFuture(false)
 
         if (integrations.isNotEmpty()) {
-            val futures = checkIntegrations(check)
-            futures += CompletableFuture.completedFuture(!isVanillaProtected(args.player, args.location))
+            val player = args.player
+            if (!isVanillaProtected(player, args.location))
+                return CompletableFuture.completedFuture(false)
+
+            val futures = ArrayList<CompletableFuture<Boolean>>()
+            for (integration in integrations) {
+                // assumes that queries for online players can be performed on the main thread
+                if (!player.isOnline && isServerThread && integration.executionMode != ExecutionMode.SERVER) {
+                    // player offline, in main thread, async calls allowed -> check protection async 
+                    futures += CompletableFuture.supplyAsync({ integration.check() }, executor)
+                } else if (!isServerThread && integration.executionMode == ExecutionMode.SERVER) {
+                    // not in main thread, no async calls allowed -> check protection on main thread
+                    futures += CompletableFuture<Boolean>().apply { runTask { complete(integration.check()) } }
+                } else {
+                    // check protection in current thread (online player, already async, or no async calls allowed)
+                    futures += CompletableFuture.completedFuture(integration.check())
+                }
+            }
+
             return CombinedBooleanFuture(futures)
         } else {
             return CompletableFuture.completedFuture(!isVanillaProtected(args.player, args.location))
@@ -242,14 +296,5 @@ object ProtectionManager {
             && location.isBetweenXZ(spawnMin, spawnMax)
     }
 
-    private fun checkIntegrations(check: ProtectionIntegration.() -> Boolean): MutableList<CompletableFuture<Boolean>> =
-        integrations.mapTo(ArrayList()) {
-            when (it.executionMode) {
-                ExecutionMode.NONE -> CompletableFuture.completedFuture(it.check())
-                ExecutionMode.ASYNC -> CompletableFuture<Boolean>().apply { completeAsync({ it.check() }, executor) }
-                ExecutionMode.SERVER -> CompletableFuture<Boolean>().apply { completeServerThread { it.check() } }
-            }
-        }
-
 }
 

nova/src/main/kotlin/xyz/xenondevs/nova/item/logic/ItemListener.kt

@@ -1,6 +1,5 @@
 package xyz.xenondevs.nova.item.logic
 
-import kotlinx.coroutines.runBlocking
 import net.minecraft.network.protocol.game.ServerboundPlayerActionPacket
 import org.bukkit.entity.Player
 import org.bukkit.event.EventHandler
@@ -51,7 +50,7 @@ internal object ItemListener : Listener, PacketListener {
         val item = event.item
 
         val location = event.clickedBlock?.location ?: player.location
-        if (item == null || runBlocking { !ProtectionManager.canUseItem(player, item, location) }) // TODO
+        if (item == null || !ProtectionManager.canUseItem(player, item, location))
             return
 
         item.novaItem?.handleInteract(event.player, item, event.action, wrappedEvent)

nova/src/main/kotlin/xyz/xenondevs/nova/util/FakeOnlinePlayer.kt

@@ -32,11 +32,11 @@ abstract class FakeOnlinePlayer(
 ) : Player, OfflinePlayer by offlinePlayer {
 
     override fun hasPermission(name: String): Boolean {
-        return PermissionManager.hasPermission(world, uniqueId, name)
+        return PermissionManager.hasPermission(world, uniqueId, name).get()
     }
 
     override fun hasPermission(perm: Permission): Boolean {
-        return PermissionManager.hasPermission(world, uniqueId, perm.name)
+        return PermissionManager.hasPermission(world, uniqueId, perm.name).get()
     }
 
     override fun isPermissionSet(name: String): Boolean {