diff --git a/src/main/kotlin/net/trueog/diamondbankog/Events.kt b/src/main/kotlin/net/trueog/diamondbankog/Events.kt index 766df27..c5436de 100644 --- a/src/main/kotlin/net/trueog/diamondbankog/Events.kt +++ b/src/main/kotlin/net/trueog/diamondbankog/Events.kt @@ -1,6 +1,5 @@ package net.trueog.diamondbankog -import kotlinx.coroutines.DelicateCoroutinesApi import kotlinx.coroutines.launch import net.trueog.diamondbankog.DiamondBankOG.Companion.balanceManager import net.trueog.diamondbankog.DiamondBankOG.Companion.config @@ -24,6 +23,7 @@ import org.bukkit.event.EventHandler import org.bukkit.event.EventPriority import org.bukkit.event.Listener import org.bukkit.event.entity.EntityPickupItemEvent +import org.bukkit.event.entity.PlayerDeathEvent import org.bukkit.event.inventory.InventoryClickEvent import org.bukkit.event.inventory.InventoryCloseEvent import org.bukkit.event.inventory.InventoryType @@ -32,7 +32,6 @@ import org.bukkit.event.player.* import org.bukkit.inventory.ItemStack import org.bukkit.persistence.PersistentDataType -@OptIn(DelicateCoroutinesApi::class) internal class Events : Listener { @EventHandler(priority = EventPriority.MONITOR) fun onPlayerJoin(event: PlayerJoinEvent) { @@ -75,15 +74,14 @@ internal class Events : Listener { } transactionLock.withLockSuspend(event.player.uniqueId) { - val inventoryShards = event.player.inventory.countTotal() + val (inventoryShards, enderChestShards) = + runOnMainThread { Pair(event.player.inventory.countTotal(), event.player.enderChest.countTotal()) } balanceManager.setPlayerShards(event.player.uniqueId, inventoryShards, ShardType.INVENTORY).getOrElse { handleError(it) return@withLockSuspend } - - val enderChestDiamonds = event.player.enderChest.countTotal() balanceManager - .setPlayerShards(event.player.uniqueId, enderChestDiamonds, ShardType.ENDER_CHEST) + .setPlayerShards(event.player.uniqueId, enderChestShards, ShardType.ENDER_CHEST) .getOrElse { handleError(it) return@withLockSuspend @@ -96,13 +94,55 @@ internal class Events : Listener { @EventHandler(priority = EventPriority.MONITOR) fun onPlayerQuit(event: PlayerQuitEvent) { + val worldName = event.player.world.name + val isVanillaWorld = worldName != "world" && worldName != "world_nether" && worldName != "world_the_end" scope.launch { transactionLock.withLockSuspend(event.player.uniqueId) { + if (isVanillaWorld) { + val inventoryShards = event.player.inventory.countTotal() + balanceManager + .setPlayerShards(event.player.uniqueId, inventoryShards, ShardType.INVENTORY) + .getOrElse { + handleError(it) + return@withLockSuspend + } + + val enderChestDiamonds = event.player.enderChest.countTotal() + balanceManager + .setPlayerShards(event.player.uniqueId, enderChestDiamonds, ShardType.ENDER_CHEST) + .getOrElse { + handleError(it) + return@withLockSuspend + } + } + balanceManager.removeCacheForPlayer(event.player.uniqueId) } } } + @EventHandler(priority = EventPriority.MONITOR) + fun onPlayerDeath(event: PlayerDeathEvent) { + val worldName = event.player.world.name + if (worldName != "world" && worldName != "world_nether" && worldName != "world_the_end") return + scope.launch { + transactionLock.withLockSuspend(event.player.uniqueId) { + val (inventoryShards, enderChestShards) = + runOnMainThread { Pair(event.player.inventory.countTotal(), event.player.enderChest.countTotal()) } + balanceManager.setPlayerShards(event.player.uniqueId, inventoryShards, ShardType.INVENTORY).getOrElse { + handleError(it) + return@withLockSuspend + } + balanceManager + .setPlayerShards(event.player.uniqueId, enderChestShards, ShardType.ENDER_CHEST) + .getOrElse { + handleError(it) + return@withLockSuspend + } + } + } + } + @EventHandler(priority = EventPriority.HIGHEST) fun onEntityPickupItem(event: EntityPickupItemEvent) { val player = event.entity diff --git a/src/main/kotlin/net/trueog/diamondbankog/api/DiamondBankAPIJava.kt b/src/main/kotlin/net/trueog/diamondbankog/api/DiamondBankAPIJava.kt index f2c3283..346e3fe 100644 --- a/src/main/kotlin/net/trueog/diamondbankog/api/DiamondBankAPIJava.kt +++ b/src/main/kotlin/net/trueog/diamondbankog/api/DiamondBankAPIJava.kt @@ -1,7 +1,6 @@ package net.trueog.diamondbankog.api import java.util.* -import kotlinx.coroutines.DelicateCoroutinesApi import kotlinx.coroutines.runBlocking import net.trueog.diamondbankog.DiamondBankException import net.trueog.diamondbankog.DiamondBankException.* @@ -19,7 +18,6 @@ import net.trueog.diamondbankog.util.ErrorHandler.handleError import net.trueog.diamondbankog.util.MainThreadBlock.runOnMainThread import org.bukkit.Bukkit -@OptIn(DelicateCoroutinesApi::class) class DiamondBankAPIJava { /** * WARNING: if the player has a transaction lock applied this function will wait until its released. @@ -27,6 +25,7 @@ class DiamondBankAPIJava { * This function also blocks for the database call, this is so you don't have to manually run .get() on a * CompletableFuture * + * @param shards must not be negative * @param transactionReason the reason for this transaction for in the transaction log * @param notes any specifics for this transaction that may be nice to know for in the transaction log * @throws DiamondBankException.EconomyDisabledException @@ -34,6 +33,7 @@ class DiamondBankAPIJava { @Throws(EconomyDisabledException::class) @Suppress("unused") fun addToPlayerBankShards(uuid: UUID, shards: Long, transactionReason: String, notes: String?) { + require(shards >= 0) { "shards must not be negative" } if (economyDisabled) throw EconomyDisabledException() return runBlocking { @@ -56,6 +56,7 @@ class DiamondBankAPIJava { * This function also blocks for the database call, this is so you don't have to manually run .get() on a * CompletableFuture * + * @param shards must not be negative * @param transactionReason the reason for this transaction for in the transaction log * @param notes any specifics for this transaction that may be nice to know for in the transaction log * @throws DiamondBankException.EconomyDisabledException @@ -64,6 +65,7 @@ class DiamondBankAPIJava { @Throws(EconomyDisabledException::class, InsufficientBalanceException::class) @Suppress("unused") fun subtractFromPlayerBankShards(uuid: UUID, shards: Long, transactionReason: String, notes: String?) { + require(shards >= 0) { "shards must not be negative" } if (economyDisabled) throw EconomyDisabledException() return runBlocking { @@ -199,6 +201,7 @@ class DiamondBankAPIJava { * This function also blocks for the database call, this is so you don't have to manually run .get() on a * CompletableFuture * + * @param shards must not be negative * @param transactionReason the reason for this transaction for in the transaction log * @param notes any specifics for this transaction that may be nice to know for in the transaction log * @throws DiamondBankException.EconomyDisabledException @@ -214,6 +217,7 @@ class DiamondBankAPIJava { ) @Suppress("unused") fun consumeFromPlayer(uuid: UUID, shards: Long, transactionReason: String, notes: String?) { + require(shards >= 0) { "shards must not be negative" } if (economyDisabled) throw EconomyDisabledException() return runBlocking { @@ -253,6 +257,7 @@ class DiamondBankAPIJava { * This function also blocks for the database call, this is so you don't have to manually run .get() on a * CompletableFuture * + * @param shards must not be negative * @param transactionReason the reason for this transaction for in the transaction log * @param notes any specifics for this transaction that may be nice to know for in the transaction log * @throws DiamondBankException.EconomyDisabledException @@ -268,6 +273,7 @@ class DiamondBankAPIJava { ) @Suppress("unused") fun playerPayPlayer(payerUuid: UUID, receiverUuid: UUID, shards: Long, transactionReason: String, notes: String?) { + require(shards >= 0) { "shards must not be negative" } if (economyDisabled) throw EconomyDisabledException() return runBlocking { diff --git a/src/main/kotlin/net/trueog/diamondbankog/api/DiamondBankAPIKotlin.kt b/src/main/kotlin/net/trueog/diamondbankog/api/DiamondBankAPIKotlin.kt index 742f3f5..3c79b30 100644 --- a/src/main/kotlin/net/trueog/diamondbankog/api/DiamondBankAPIKotlin.kt +++ b/src/main/kotlin/net/trueog/diamondbankog/api/DiamondBankAPIKotlin.kt @@ -1,7 +1,6 @@ package net.trueog.diamondbankog.api import java.util.* -import kotlinx.coroutines.DelicateCoroutinesApi import net.trueog.diamondbankog.DiamondBankException.* import net.trueog.diamondbankog.DiamondBankOG.Companion.balanceManager import net.trueog.diamondbankog.DiamondBankOG.Companion.economyDisabled @@ -17,7 +16,6 @@ import net.trueog.diamondbankog.util.ErrorHandler.handleError import net.trueog.diamondbankog.util.MainThreadBlock.runOnMainThread import org.bukkit.Bukkit -@OptIn(DelicateCoroutinesApi::class) class DiamondBankAPIKotlin { /** * WARNING: if the player has a transaction lock applied this function will wait until its released @@ -28,19 +26,20 @@ class DiamondBankAPIKotlin { @Suppress("unused") suspend fun addToPlayerBankShards( uuid: UUID, - shards: Long, + shards: ULong, transactionReason: String, notes: String?, ): Result { + require(shards <= Long.MAX_VALUE.toULong()) { "shards must not be above the max value of a Long" } if (economyDisabled) return Result.failure(EconomyDisabledException()) return transactionLock.withLockSuspend(uuid) { - balanceManager.addToBankShards(uuid, shards).getOrElse { + balanceManager.addToBankShards(uuid, shards.toLong()).getOrElse { handleError(it) return@withLockSuspend Result.failure(EconomyDisabledException()) } - balanceManager.insertTransactionLog(uuid, shards, null, transactionReason, notes).getOrElse { + balanceManager.insertTransactionLog(uuid, shards.toLong(), null, transactionReason, notes).getOrElse { handleError(it) } @@ -57,19 +56,20 @@ class DiamondBankAPIKotlin { @Suppress("unused") suspend fun subtractFromPlayerBankShards( uuid: UUID, - shards: Long, + shards: ULong, transactionReason: String, notes: String?, ): Result { + require(shards <= Long.MAX_VALUE.toULong()) { "shards must not be above the max value of a Long" } if (economyDisabled) return Result.failure(EconomyDisabledException()) return transactionLock.withLockSuspend(uuid) { - balanceManager.subtractFromBankShards(uuid, shards).getOrElse { + balanceManager.subtractFromBankShards(uuid, shards.toLong()).getOrElse { handleError(it) return@withLockSuspend Result.failure(EconomyDisabledException()) } - balanceManager.insertTransactionLog(uuid, shards, null, transactionReason, notes).getOrElse { + balanceManager.insertTransactionLog(uuid, shards.toLong(), null, transactionReason, notes).getOrElse { handleError(it) } @@ -143,7 +143,8 @@ class DiamondBankAPIKotlin { * @param notes any specifics for this transaction that may be nice to know for in the transaction log */ @Suppress("unused") - suspend fun consumeFromPlayer(uuid: UUID, shards: Long, transactionReason: String, notes: String?): Result { + suspend fun consumeFromPlayer(uuid: UUID, shards: ULong, transactionReason: String, notes: String?): Result { + require(shards <= Long.MAX_VALUE.toULong()) { "shards must not be above the max value of a Long" } if (economyDisabled) return Result.failure(EconomyDisabledException()) return transactionLock.withLockSuspend(uuid) { @@ -155,7 +156,7 @@ class DiamondBankAPIKotlin { InventorySnapshot.from(player.inventory, balanceManager) } - CommonOperations.consume(player.uniqueId, shards, inventorySnapshot).getOrElse { + CommonOperations.consume(player.uniqueId, shards.toLong(), inventorySnapshot).getOrElse { player.inventory.unlock() if (it is DatabaseException) { handleError(it) @@ -169,7 +170,7 @@ class DiamondBankAPIKotlin { player.inventory.unlock() } - balanceManager.insertTransactionLog(uuid, shards, null, transactionReason, notes).getOrElse { + balanceManager.insertTransactionLog(uuid, shards.toLong(), null, transactionReason, notes).getOrElse { handleError(it) } @@ -191,10 +192,11 @@ class DiamondBankAPIKotlin { suspend fun playerPayPlayer( payerUuid: UUID, receiverUuid: UUID, - shards: Long, + shards: ULong, transactionReason: String, notes: String?, ): Result { + require(shards <= Long.MAX_VALUE.toULong()) { "shards must not be above the max value of a Long" } if (economyDisabled) return Result.failure(EconomyDisabledException()) return transactionLock.withLockSuspend(payerUuid) { @@ -209,7 +211,7 @@ class DiamondBankAPIKotlin { InventorySnapshot.from(payer.inventory, balanceManager) } - CommonOperations.consume(payer.uniqueId, shards, inventorySnapshot).getOrElse { + CommonOperations.consume(payer.uniqueId, shards.toLong(), inventorySnapshot).getOrElse { payer.inventory.unlock() if (it is DatabaseException) { handleError(it) @@ -218,7 +220,7 @@ class DiamondBankAPIKotlin { return@withLockSuspend Result.failure(it) } - balanceManager.addToBankShards(receiverUuid, shards).getOrElse { + balanceManager.addToBankShards(receiverUuid, shards.toLong()).getOrElse { payer.inventory.unlock() handleError(it) return@withLockSuspend Result.failure(it) @@ -229,9 +231,9 @@ class DiamondBankAPIKotlin { payer.inventory.unlock() } - balanceManager.insertTransactionLog(payerUuid, shards, receiverUuid, transactionReason, notes).getOrElse { - handleError(it) - } + balanceManager + .insertTransactionLog(payerUuid, shards.toLong(), receiverUuid, transactionReason, notes) + .getOrElse { handleError(it) } Result.success(Unit) } diff --git a/src/main/kotlin/net/trueog/diamondbankog/api/README.md b/src/main/kotlin/net/trueog/diamondbankog/api/README.md index da4a3c8..0556da8 100644 --- a/src/main/kotlin/net/trueog/diamondbankog/api/README.md +++ b/src/main/kotlin/net/trueog/diamondbankog/api/README.md @@ -35,6 +35,7 @@ Exceptions are only applicable when using the Java API, when using the Kotlin AP * This function also blocks for the database call, this is so you don't have to manually run .get() on a * CompletableFuture * + * @param shards must not be negative * @param transactionReason the reason for this transaction for in the transaction log * @param notes any specifics for this transaction that may be nice to know for in the transaction log * @throws DiamondBankException.EconomyDisabledException @@ -42,7 +43,7 @@ Exceptions are only applicable when using the Java API, when using the Kotlin AP ``` Kotlin: ```kotlin -suspend fun addToPlayerBankShards(uuid: UUID, shards: Long, transactionReason: String, notes: String?): Result +suspend fun addToPlayerBankShards(uuid: UUID, shards: ULong, transactionReason: String, notes: String?): Result ``` Java: ```java @@ -57,6 +58,7 @@ public void addToPlayerBankShards(UUID uuid, long shards, String transactionReas * This function also blocks for the database call, this is so you don't have to manually run .get() on a * CompletableFuture * + * @param shards must not be negative * @param transactionReason the reason for this transaction for in the transaction log * @param notes any specifics for this transaction that may be nice to know for in the transaction log * @throws DiamondBankException.EconomyDisabledException @@ -65,7 +67,7 @@ public void addToPlayerBankShards(UUID uuid, long shards, String transactionReas ``` Kotlin: ```kotlin -suspend fun subtractFromPlayerBankShards(uuid: UUID, shards: Long, transactionReason: String, notes: String?): Result +suspend fun subtractFromPlayerBankShards(uuid: UUID, shards: ULong, transactionReason: String, notes: String?): Result ``` Java: ```java @@ -181,6 +183,7 @@ public Map<@Nullable UUID, Long> getBaltop(int offset) * This function also blocks for the database call, this is so you don't have to manually run .get() on a * CompletableFuture * + * @param shards must not be negative * @param transactionReason the reason for this transaction for in the transaction log * @param notes any specifics for this transaction that may be nice to know for in the transaction log * @throws DiamondBankException.EconomyDisabledException @@ -191,7 +194,7 @@ public Map<@Nullable UUID, Long> getBaltop(int offset) ``` Kotlin: ```kotlin -suspend fun consumeFromPlayer(uuid: UUID, shards: Long, transactionReason: String, notes: String?): Result +suspend fun consumeFromPlayer(uuid: UUID, shards: ULong, transactionReason: String, notes: String?): Result ``` Java: ```java @@ -206,6 +209,7 @@ public void consumeFromPlayer(UUID uuid, long shards, String transactionReason, * This function also blocks for the database call, this is so you don't have to manually run .get() on a * CompletableFuture * + * @param shards must not be negative * @param transactionReason the reason for this transaction for in the transaction log * @param notes any specifics for this transaction that may be nice to know for in the transaction log * @throws DiamondBankException.EconomyDisabledException @@ -216,7 +220,7 @@ public void consumeFromPlayer(UUID uuid, long shards, String transactionReason, ``` Kotlin: ```kotlin -suspend fun playerPayPlayer(payerUuid: UUID, receiverUuid: UUID, shards: Long, transactionReason: String, notes: String?): Result +suspend fun playerPayPlayer(payerUuid: UUID, receiverUuid: UUID, shards: ULong, transactionReason: String, notes: String?): Result ``` Java: ```java diff --git a/src/main/kotlin/net/trueog/diamondbankog/balance/BalanceManager.kt b/src/main/kotlin/net/trueog/diamondbankog/balance/BalanceManager.kt index a590aea..39be648 100644 --- a/src/main/kotlin/net/trueog/diamondbankog/balance/BalanceManager.kt +++ b/src/main/kotlin/net/trueog/diamondbankog/balance/BalanceManager.kt @@ -41,7 +41,7 @@ interface BalanceManager { suspend fun cacheForPlayer(uuid: UUID): Result - fun removeCacheForPlayer(uuid: UUID) + suspend fun removeCacheForPlayer(uuid: UUID) fun shutdown() } diff --git a/src/main/kotlin/net/trueog/diamondbankog/balance/CachingBalanceManager.kt b/src/main/kotlin/net/trueog/diamondbankog/balance/CachingBalanceManager.kt index 7969192..e9e3b31 100644 --- a/src/main/kotlin/net/trueog/diamondbankog/balance/CachingBalanceManager.kt +++ b/src/main/kotlin/net/trueog/diamondbankog/balance/CachingBalanceManager.kt @@ -2,8 +2,10 @@ package net.trueog.diamondbankog.balance import java.util.* import java.util.concurrent.ConcurrentHashMap -import java.util.concurrent.atomic.AtomicInteger +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock import net.trueog.diamondbankog.DiamondBankException.EconomyDisabledException +import net.trueog.diamondbankog.DiamondBankException.InsufficientBalanceException import net.trueog.diamondbankog.DiamondBankException.InvalidArgumentException import net.trueog.diamondbankog.DiamondBankOG.Companion.debug import net.trueog.diamondbankog.DiamondBankOG.Companion.economyDisabled @@ -15,7 +17,6 @@ import net.trueog.diamondbankog.persistence.PostgreSQL internal class CachingBalanceManager private constructor() : BalanceManager { val cache = Cache() lateinit var postgreSQL: PostgreSQL - val beingModified = ConcurrentHashMap, AtomicInteger>() companion object : BalanceManagerFactory { override fun create(): BalanceManager? { @@ -25,44 +26,49 @@ internal class CachingBalanceManager private constructor() : BalanceManager { } } - private fun increment(uuid: UUID, type: ShardType) { - beingModified.computeIfAbsent(uuid to type) { AtomicInteger(0) }.incrementAndGet() - } + val mutexMap = ConcurrentHashMap>() - private fun decrement(uuid: UUID, type: ShardType) { - beingModified.computeIfPresent(uuid to type) { _, counter -> - val newValue = counter.decrementAndGet() - if (newValue == 0) null else counter - } + private suspend fun withLock(uuid: UUID, vararg types: ShardType, block: suspend () -> T): T { + val shardTypeToMutex = mutexMap.computeIfAbsent(uuid) { ConcurrentHashMap() } + val mutexes = types.sorted().map { shardTypeToMutex.computeIfAbsent(it) { Mutex() } } + return mutexes.foldRight(block) { mutex, acc -> { mutex.withLock { acc() } } }() } override suspend fun setPlayerShards(uuid: UUID, shards: Long, type: ShardType): Result { if (type == ShardType.TOTAL) return Result.failure(InvalidArgumentException()) if (economyDisabled) return Result.failure(EconomyDisabledException()) - increment(uuid, type) - postgreSQL.setPlayerShards(uuid, shards, type).getOrElse { - return Result.failure(it) + return withLock(uuid, type) { + postgreSQL.setPlayerShards(uuid, shards, type).getOrElse { + return@withLock Result.failure(it) + } + cache.setBalance(uuid, shards, type) + return@withLock Result.success(Unit) } - cache.setBalance(uuid, shards, type) - decrement(uuid, type) - return Result.success(Unit) } private suspend fun addToPlayerShards(uuid: UUID, shards: Long, type: ShardType): Result { if (type == ShardType.TOTAL) return Result.failure(InvalidArgumentException()) if (economyDisabled) return Result.failure(EconomyDisabledException()) - increment(uuid, type) - val newBalance = - postgreSQL.addToPlayerShards(uuid, shards, type).getOrElse { - return Result.failure(it) + return withLock(uuid, type) { + val currentBalance = + postgreSQL.getShardTypeShards(uuid, type).getOrElse { + return@withLock Result.failure(it) + } + if (currentBalance + shards < 0) { + return@withLock Result.failure(InsufficientBalanceException(currentBalance)) } - cache.setBalance(uuid, newBalance, type).getOrElse { - return Result.failure(it) + + val newBalance = + postgreSQL.addToPlayerShards(uuid, shards, type).getOrElse { + return@withLock Result.failure(it) + } + cache.setBalance(uuid, newBalance, type).getOrElse { + return@withLock Result.failure(it) + } + return@withLock Result.success(Unit) } - decrement(uuid, type) - return Result.success(Unit) } override suspend fun addToBankShards(uuid: UUID, shards: Long): Result = @@ -78,44 +84,28 @@ internal class CachingBalanceManager private constructor() : BalanceManager { override suspend fun getEnderChestShards(uuid: UUID): Result = getShardTypeShards(uuid, ShardType.ENDER_CHEST) override suspend fun getShardTypeShards(uuid: UUID, type: ShardType): Result { - if ((beingModified[uuid to type]?.get() ?: 0) > 0) { - if (debug) { - plugin.logger.info("Cache miss (being modified) for $uuid!") - } - return postgreSQL.getShardTypeShards(uuid, type) - } val cacheBalance = cache.getBalance(uuid, type).getOrElse { return Result.failure(it) } - if (cacheBalance == -1L) { + if (cacheBalance != -1L) return Result.success(cacheBalance) + + return withLock(uuid, type) { if (debug) { plugin.logger.info("Cache miss for $uuid!") } - increment(uuid, type) val dbBalance = postgreSQL.getShardTypeShards(uuid, type).getOrElse { - return Result.failure(it) + return@withLock Result.failure(it) } cache.setBalance(uuid, dbBalance, type) - decrement(uuid, type) - return Result.success(dbBalance) - } else if (debug) { - plugin.logger.info("Cache hit for $uuid!") + return@withLock Result.success(dbBalance) } - return Result.success(cacheBalance) } override suspend fun getTotalShards(uuid: UUID): Result = getAllShards(uuid).map { it.total } override suspend fun getAllShards(uuid: UUID): Result { - val anyBeingModified = beingModified.any { it.key.first == uuid && it.value.get() > 0 } - if (anyBeingModified) { - if (debug) { - plugin.logger.info("Cache miss (being modified) for $uuid!") - } - return postgreSQL.getAllShards(uuid) - } val cacheBankBalance = cache.getBalance(uuid, ShardType.BANK).getOrElse { return Result.failure(it) @@ -128,28 +118,24 @@ internal class CachingBalanceManager private constructor() : BalanceManager { cache.getBalance(uuid, ShardType.ENDER_CHEST).getOrElse { return Result.failure(it) } - if (cacheBankBalance == -1L || cacheInventoryBalance == -1L || cacheEnderChestBalance == -1L) { + + if (cacheBankBalance != -1L && cacheInventoryBalance != -1L && cacheEnderChestBalance != -1L) { + return Result.success(PlayerShards(cacheBankBalance, cacheInventoryBalance, cacheEnderChestBalance)) + } + + return withLock(uuid, ShardType.BANK, ShardType.INVENTORY, ShardType.ENDER_CHEST) { if (debug) { plugin.logger.info("Cache miss for $uuid!") } - increment(uuid, ShardType.BANK) - increment(uuid, ShardType.INVENTORY) - increment(uuid, ShardType.ENDER_CHEST) val dbPlayerShards = postgreSQL.getAllShards(uuid).getOrElse { - return Result.failure(it) + return@withLock Result.failure(it) } cache.setBalance(uuid, dbPlayerShards.bank, ShardType.BANK) cache.setBalance(uuid, dbPlayerShards.inventory, ShardType.INVENTORY) cache.setBalance(uuid, dbPlayerShards.enderChest, ShardType.ENDER_CHEST) - decrement(uuid, ShardType.BANK) - decrement(uuid, ShardType.INVENTORY) - decrement(uuid, ShardType.ENDER_CHEST) - return Result.success(dbPlayerShards) - } else if (debug) { - plugin.logger.info("Cache hit for $uuid!") + return@withLock Result.success(dbPlayerShards) } - return Result.success(PlayerShards(cacheBankBalance, cacheInventoryBalance, cacheEnderChestBalance)) } override suspend fun getBaltop(offset: Int): Result> = postgreSQL.getBaltop(offset) @@ -170,20 +156,20 @@ internal class CachingBalanceManager private constructor() : BalanceManager { override suspend fun hasEntry(uuid: UUID): Result = postgreSQL.hasEntry(uuid) - override suspend fun cacheForPlayer(uuid: UUID): Result { - val playerShards = - postgreSQL.getAllShards(uuid).getOrElse { - return Result.failure(it) - } - cache.setBalance(uuid, playerShards.bank, ShardType.BANK) - cache.setBalance(uuid, playerShards.inventory, ShardType.INVENTORY) - cache.setBalance(uuid, playerShards.enderChest, ShardType.ENDER_CHEST) - return Result.success(Unit) - } + override suspend fun cacheForPlayer(uuid: UUID) = + withLock(uuid, ShardType.BANK, ShardType.INVENTORY, ShardType.ENDER_CHEST) { + val playerShards = + postgreSQL.getAllShards(uuid).getOrElse { + return@withLock Result.failure(it) + } + cache.setBalance(uuid, playerShards.bank, ShardType.BANK) + cache.setBalance(uuid, playerShards.inventory, ShardType.INVENTORY) + cache.setBalance(uuid, playerShards.enderChest, ShardType.ENDER_CHEST) + return@withLock Result.success(Unit) + } - override fun removeCacheForPlayer(uuid: UUID) { - cache.removeAll(uuid) - } + override suspend fun removeCacheForPlayer(uuid: UUID) = + withLock(uuid, ShardType.BANK, ShardType.INVENTORY, ShardType.ENDER_CHEST) { cache.removeAll(uuid) } override fun shutdown() { postgreSQL.pool.disconnect().get() diff --git a/src/main/kotlin/net/trueog/diamondbankog/balance/command/Balance.kt b/src/main/kotlin/net/trueog/diamondbankog/balance/command/Balance.kt index 1562ff2..34f57dd 100644 --- a/src/main/kotlin/net/trueog/diamondbankog/balance/command/Balance.kt +++ b/src/main/kotlin/net/trueog/diamondbankog/balance/command/Balance.kt @@ -1,6 +1,5 @@ package net.trueog.diamondbankog.balance.command -import kotlinx.coroutines.DelicateCoroutinesApi import kotlinx.coroutines.launch import net.trueog.diamondbankog.DiamondBankOG import net.trueog.diamondbankog.DiamondBankOG.Companion.balanceManager @@ -16,7 +15,6 @@ import org.bukkit.command.CommandSender import org.bukkit.entity.Player internal class Balance : CommandExecutor { - @OptIn(DelicateCoroutinesApi::class) override fun onCommand(sender: CommandSender, command: Command, label: String, args: Array?): Boolean { scope.launch { if (economyDisabled) { diff --git a/src/main/kotlin/net/trueog/diamondbankog/balance/command/Balancetop.kt b/src/main/kotlin/net/trueog/diamondbankog/balance/command/Balancetop.kt index 09dd7e8..7c75524 100644 --- a/src/main/kotlin/net/trueog/diamondbankog/balance/command/Balancetop.kt +++ b/src/main/kotlin/net/trueog/diamondbankog/balance/command/Balancetop.kt @@ -1,7 +1,6 @@ package net.trueog.diamondbankog.balance.command import kotlin.math.ceil -import kotlinx.coroutines.DelicateCoroutinesApi import kotlinx.coroutines.launch import net.trueog.diamondbankog.DiamondBankOG import net.trueog.diamondbankog.DiamondBankOG.Companion.balanceManager @@ -18,7 +17,6 @@ import org.bukkit.command.CommandExecutor import org.bukkit.command.CommandSender internal class Balancetop : CommandExecutor { - @OptIn(DelicateCoroutinesApi::class) override fun onCommand(sender: CommandSender, command: Command, label: String, args: Array?): Boolean { scope.launch { if (economyDisabled) { diff --git a/src/main/kotlin/net/trueog/diamondbankog/balance/command/SetBankBalance.kt b/src/main/kotlin/net/trueog/diamondbankog/balance/command/SetBankBalance.kt index 3bd2f75..671cbdf 100644 --- a/src/main/kotlin/net/trueog/diamondbankog/balance/command/SetBankBalance.kt +++ b/src/main/kotlin/net/trueog/diamondbankog/balance/command/SetBankBalance.kt @@ -1,6 +1,5 @@ package net.trueog.diamondbankog.balance.command -import kotlinx.coroutines.DelicateCoroutinesApi import kotlinx.coroutines.launch import net.trueog.diamondbankog.DiamondBankOG import net.trueog.diamondbankog.DiamondBankOG.Companion.balanceManager @@ -18,7 +17,6 @@ import org.bukkit.command.CommandSender import org.bukkit.entity.Player internal class SetBankBalance : CommandExecutor { - @OptIn(DelicateCoroutinesApi::class) override fun onCommand(sender: CommandSender, command: Command, label: String, args: Array?): Boolean { scope.launch { if (economyDisabled) { diff --git a/src/main/kotlin/net/trueog/diamondbankog/transaction/command/Deposit.kt b/src/main/kotlin/net/trueog/diamondbankog/transaction/command/Deposit.kt index 85b88f9..4f2e8bb 100644 --- a/src/main/kotlin/net/trueog/diamondbankog/transaction/command/Deposit.kt +++ b/src/main/kotlin/net/trueog/diamondbankog/transaction/command/Deposit.kt @@ -1,7 +1,6 @@ package net.trueog.diamondbankog.transaction.command import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.DelicateCoroutinesApi import kotlinx.coroutines.launch import net.kyori.adventure.text.minimessage.MiniMessage import net.trueog.diamondbankog.* @@ -27,7 +26,6 @@ internal class Deposit( val scope: CoroutineScope = DiamondBankOG.scope, val transactionLock: TransactionLock = DiamondBankOG.transactionLock, ) : CommandExecutor { - @OptIn(DelicateCoroutinesApi::class) override fun onCommand(sender: CommandSender, command: Command, label: String, args: Array?): Boolean { if (CommonCommandInterlude.run(sender, "deposit", config, mm)) { return true diff --git a/src/main/kotlin/net/trueog/diamondbankog/transaction/command/Pay.kt b/src/main/kotlin/net/trueog/diamondbankog/transaction/command/Pay.kt index 6bc95f9..ec7a266 100644 --- a/src/main/kotlin/net/trueog/diamondbankog/transaction/command/Pay.kt +++ b/src/main/kotlin/net/trueog/diamondbankog/transaction/command/Pay.kt @@ -1,7 +1,6 @@ package net.trueog.diamondbankog.transaction.command import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.DelicateCoroutinesApi import kotlinx.coroutines.launch import net.kyori.adventure.text.minimessage.MiniMessage import net.luckperms.api.LuckPerms @@ -29,7 +28,6 @@ internal class Pay( val luckPerms: LuckPerms = DiamondBankOG.luckPerms, val transactionLock: TransactionLock = DiamondBankOG.transactionLock, ) : CommandExecutor { - @OptIn(DelicateCoroutinesApi::class) override fun onCommand(sender: CommandSender, command: Command, label: String, args: Array?): Boolean { if (CommonCommandInterlude.run(sender, "pay", config, mm)) { return true diff --git a/src/main/kotlin/net/trueog/diamondbankog/transaction/command/Withdraw.kt b/src/main/kotlin/net/trueog/diamondbankog/transaction/command/Withdraw.kt index 374cca5..3224d23 100644 --- a/src/main/kotlin/net/trueog/diamondbankog/transaction/command/Withdraw.kt +++ b/src/main/kotlin/net/trueog/diamondbankog/transaction/command/Withdraw.kt @@ -2,7 +2,6 @@ package net.trueog.diamondbankog.transaction.command import kotlin.math.floor import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.DelicateCoroutinesApi import kotlinx.coroutines.launch import net.kyori.adventure.text.minimessage.MiniMessage import net.trueog.diamondbankog.* @@ -30,7 +29,6 @@ internal class Withdraw( val scope: CoroutineScope = DiamondBankOG.scope, val transactionLock: TransactionLock = DiamondBankOG.transactionLock, ) : CommandExecutor { - @OptIn(DelicateCoroutinesApi::class) override fun onCommand(sender: CommandSender, command: Command, label: String, args: Array?): Boolean { if (CommonCommandInterlude.run(sender, "withdraw", config, mm)) { return true diff --git a/src/test/kotlin/net/trueog/diamondbankog/CachingBalanceManagerTest.kt b/src/test/kotlin/net/trueog/diamondbankog/CachingBalanceManagerTest.kt index fdb6ae1..ee8cbe1 100644 --- a/src/test/kotlin/net/trueog/diamondbankog/CachingBalanceManagerTest.kt +++ b/src/test/kotlin/net/trueog/diamondbankog/CachingBalanceManagerTest.kt @@ -2,7 +2,6 @@ package net.trueog.diamondbankog import io.mockk.* import io.mockk.impl.annotations.MockK -import java.util.concurrent.atomic.AtomicInteger import kotlinx.coroutines.test.runTest import net.trueog.diamondbankog.Constants.playerUuid import net.trueog.diamondbankog.DiamondBankOG.Companion.economyDisabled @@ -86,6 +85,7 @@ class CachingBalanceManagerTest { @Test @DisplayName("Add To Bank Shards") fun addToBankShards() = runTest { + coEvery { postgreSQL.getShardTypeShards(playerUuid, ShardType.BANK) } returns Result.success(0) coEvery { postgreSQL.addToPlayerShards(playerUuid, 5, ShardType.BANK) } returns Result.success(5) val result = manager.addToBankShards(playerUuid, 5) @@ -100,6 +100,7 @@ class CachingBalanceManagerTest { @Test @DisplayName("Add To Bank Shards (with existing balance)") fun addToBankShardsWithExistingBalance() = runTest { + coEvery { postgreSQL.getShardTypeShards(playerUuid, ShardType.BANK) } returns Result.success(5) coEvery { postgreSQL.addToPlayerShards(playerUuid, 5, ShardType.BANK) } returns Result.success(10) val result = manager.addToBankShards(playerUuid, 5) @@ -114,7 +115,7 @@ class CachingBalanceManagerTest { @Test @DisplayName("Subtract From Bank Shards") fun subtractFromBankShards() = runTest { - manager.cache.setBalance(playerUuid, 10, ShardType.BANK) + coEvery { postgreSQL.getShardTypeShards(playerUuid, ShardType.BANK) } returns Result.success(10) coEvery { postgreSQL.addToPlayerShards(playerUuid, -4, ShardType.BANK) } returns Result.success(6) val result = manager.subtractFromBankShards(playerUuid, 4) @@ -126,9 +127,31 @@ class CachingBalanceManagerTest { ) } + @Test + @DisplayName("Subtract From Bank Shards (insufficient balance)") + fun subtractFromBankShardsInsufficientBalance() = runTest { + manager.cache.setBalance(playerUuid, 2, ShardType.BANK) + coEvery { postgreSQL.getShardTypeShards(playerUuid, ShardType.BANK) } returns Result.success(2) + coEvery { postgreSQL.addToPlayerShards(playerUuid, -4, ShardType.BANK) } returns Result.failure(Exception()) + + val result = manager.subtractFromBankShards(playerUuid, 4) + + assertAll( + { + assertTrue( + result.exceptionOrNull() is DiamondBankException.InsufficientBalanceException, + "subtractFromBankShards result should be an InsufficientBalanceException", + ) + }, + { assertEquals(2, manager.cache.getBalance(playerUuid, ShardType.BANK).getOrNull()) }, + { coVerify(exactly = 0) { postgreSQL.addToPlayerShards(playerUuid, -4, ShardType.BANK) } }, + ) + } + @Test @DisplayName("Add To Bank Shards DB failure propagates") fun addToBankShardsDbFail() = runTest { + coEvery { postgreSQL.getShardTypeShards(playerUuid, ShardType.BANK) } returns Result.success(5) coEvery { postgreSQL.addToPlayerShards(playerUuid, 5, ShardType.BANK) } returns Result.failure(RuntimeException("db error")) @@ -154,21 +177,6 @@ class CachingBalanceManagerTest { ) } - @Test - @DisplayName("Get Bank Shards while being modified bypasses cache") - fun getBankShardsWhileBeingModified() = runTest { - manager.cache.setBalance(playerUuid, 5, ShardType.BANK) - manager.beingModified[playerUuid to ShardType.BANK] = AtomicInteger(1) - coEvery { postgreSQL.getShardTypeShards(playerUuid, ShardType.BANK) } returns Result.success(99) - - val result = manager.getBankShards(playerUuid) - - assertAll( - { assertEquals(99, result.getOrNull()) }, - { coVerify { postgreSQL.getShardTypeShards(playerUuid, ShardType.BANK) } }, - ) - } - @Test @DisplayName("Get Total Shards (cache miss then hit)") fun getTotalShardsCacheMissThenHit() = runTest { @@ -184,18 +192,6 @@ class CachingBalanceManagerTest { ) } - @Test - @DisplayName("Get Total Shards while being modified bypasses cache") - fun getTotalShardsWhileBeingModified() = runTest { - manager.cache.setBalance(playerUuid, 3, ShardType.TOTAL) - manager.beingModified[playerUuid to ShardType.BANK] = AtomicInteger(1) - coEvery { postgreSQL.getAllShards(playerUuid) } returns Result.success(PlayerShards(20, 20, 10)) - - val result = manager.getTotalShards(playerUuid) - - assertAll({ assertEquals(50, result.getOrNull()) }, { coVerify { postgreSQL.getAllShards(playerUuid) } }) - } - @Test @DisplayName("Get All Shards (cache miss then hit)") fun getAllShardsCacheMissThenHit() = runTest { @@ -242,7 +238,7 @@ class CachingBalanceManagerTest { @Test @DisplayName("Remove Cache For Player") - fun removeCacheForPlayer() { + suspend fun removeCacheForPlayer() { manager.cache.setBalance(playerUuid, 1, ShardType.BANK) manager.cache.setBalance(playerUuid, 2, ShardType.INVENTORY) manager.cache.setBalance(playerUuid, 3, ShardType.ENDER_CHEST)