diff --git a/src/main/java/fr/openmc/core/features/economy/EconomyManager.java b/src/main/java/fr/openmc/core/features/economy/EconomyManager.java index fe79c9041..ff927a82b 100644 --- a/src/main/java/fr/openmc/core/features/economy/EconomyManager.java +++ b/src/main/java/fr/openmc/core/features/economy/EconomyManager.java @@ -9,13 +9,16 @@ import fr.openmc.core.bootstrap.features.annotations.Credit; import fr.openmc.core.bootstrap.features.types.DatabaseFeature; import fr.openmc.core.bootstrap.features.types.HasCommands; +import fr.openmc.core.bootstrap.integration.OMCLogger; +import fr.openmc.core.OMCPlugin; import fr.openmc.core.features.economy.commands.Baltop; import fr.openmc.core.features.economy.commands.History; import fr.openmc.core.features.economy.commands.Money; import fr.openmc.core.features.economy.commands.Pay; import fr.openmc.core.features.economy.models.EconomyPlayer; import fr.openmc.core.hooks.itemsadder.ItemsAdderHook; -import lombok.Getter; +import org.bukkit.Bukkit; +import org.bukkit.scheduler.BukkitTask; import javax.annotation.Nullable; import java.math.BigDecimal; @@ -26,10 +29,14 @@ @Credit(developers = {"Axeno", "Piquel Chips", "PuppyTransGirl", "Gyro"}) public class EconomyManager extends Feature implements DatabaseFeature, HasCommands { - @Getter private static Map balances; private static Dao playersDao; + private static final Set dirtyBalances = new HashSet<>(); + private static final Object balancesLock = new Object(); + private static final Object saveLock = new Object(); + private static final long AUTO_SAVE_INTERVAL_TICKS = 20L * 60L * 5L; + private static BukkitTask autoSaveTask; private static final DecimalFormat decimalFormat = new DecimalFormat("#.##"); private static final NavigableMap suffixes = new TreeMap<>(Map.of( @@ -43,6 +50,8 @@ public class EconomyManager extends Feature implements DatabaseFeature, HasComma @Override public void init() { balances = loadAllBalances(); + dirtyBalances.clear(); + startAutoSaveTask(); } @Override @@ -61,9 +70,27 @@ public void initDB(ConnectionSource connectionSource) throws SQLException { playersDao = DaoManager.createDao(connectionSource, EconomyPlayer.class); } + @Override + protected void save() { + stopAutoSaveTask(); + saveAllBalances(true); + } + public static double getBalance(UUID playerUUID) { - EconomyPlayer bank = getPlayerBank(playerUUID); - return bank.getBalance(); + synchronized (balancesLock) { + EconomyPlayer bank = balances.get(playerUUID); + return bank == null ? 0 : bank.getBalance(); + } + } + + public static Map getBalances() { + synchronized (balancesLock) { + Map snapshot = new HashMap<>(); + + balances.forEach((playerUUID, player) -> snapshot.put(playerUUID, copyPlayer(player))); + + return Collections.unmodifiableMap(snapshot); + } } public static void addBalance(UUID playerUUID, double amount) { @@ -71,8 +98,11 @@ public static void addBalance(UUID playerUUID, double amount) { } public static void addBalance(UUID playerUUID, double amount, @Nullable String reason) { - EconomyPlayer bank = getPlayerBank(playerUUID); - bank.deposit(amount); + synchronized (balancesLock) { + EconomyPlayer bank = getOrCreatePlayerBank(playerUUID); + bank.deposit(amount); + markPlayerBankDirty(bank); + } if (reason != null) { TransactionsManager.registerTransaction(new Transaction( @@ -83,7 +113,6 @@ public static void addBalance(UUID playerUUID, double amount, @Nullable String r )); } - savePlayerBank(bank); } public static boolean withdrawBalance(UUID playerUUID, double amount) { @@ -91,24 +120,26 @@ public static boolean withdrawBalance(UUID playerUUID, double amount) { } public static boolean withdrawBalance(UUID playerUUID, double amount, @Nullable String reason) { - EconomyPlayer bank = getPlayerBank(playerUUID); + synchronized (balancesLock) { + EconomyPlayer bank = getOrCreatePlayerBank(playerUUID); - if (bank.withdraw(amount)) { - if (reason != null) { - TransactionsManager.registerTransaction(new Transaction( - "CONSOLE", - playerUUID.toString(), - amount, - reason - )); + if (!bank.withdraw(amount)) { + return false; } - savePlayerBank(bank); + markPlayerBankDirty(bank); + } - return true; + if (reason != null) { + TransactionsManager.registerTransaction(new Transaction( + "CONSOLE", + playerUUID.toString(), + amount, + reason + )); } - return false; + return true; } /** @@ -152,10 +183,11 @@ public static boolean transferBalance(UUID fromPlayer, UUID toPlayer, double amo } public static void setBalance(UUID playerUUID, double amount) { - EconomyPlayer bank = getPlayerBank(playerUUID); - bank.withdraw(bank.getBalance()); - bank.deposit(amount); - savePlayerBank(bank); + synchronized (balancesLock) { + EconomyPlayer bank = getOrCreatePlayerBank(playerUUID); + bank.setBalance(amount); + markPlayerBankDirty(bank); + } } public static String getMiniBalance(UUID playerUUID) { @@ -164,20 +196,93 @@ public static String getMiniBalance(UUID playerUUID) { return getFormattedSimplifiedNumber(balance); } - public static void savePlayerBank(EconomyPlayer player) { - try { - balances.put(player.getPlayerUUID(), player); - playersDao.createOrUpdate(player); - } catch (SQLException e) { - throw new RuntimeException(e); + public static void markPlayerBankDirty(EconomyPlayer player) { + synchronized (balancesLock) { + balances.put(player.getPlayerUUID(), copyPlayer(player)); + dirtyBalances.add(player.getPlayerUUID()); } } + /** + * @deprecated Use {@link #markPlayerBankDirty(EconomyPlayer)}. This method + * only updates the in-memory cache and marks the balance dirty; persistence is + * deferred to {@link #saveAllBalances()} or the shutdown save. + */ + @Deprecated(since = "2.5.0", forRemoval = false) + public static void savePlayerBank(EconomyPlayer player) { + markPlayerBankDirty(player); + } + + /** + * Returns a snapshot of a player's economy data. + *

+ * Mutating the returned {@link EconomyPlayer} does not update the cache or mark + * the balance dirty. Use {@link #setBalance(UUID, double)}, + * {@link #addBalance(UUID, double)} or {@link #withdrawBalance(UUID, double)} + * to change a player's balance. + */ public static EconomyPlayer getPlayerBank(UUID playerUUID) { - EconomyPlayer bank = balances.get(playerUUID); - if (bank != null) - return bank; - return new EconomyPlayer(playerUUID); + synchronized (balancesLock) { + EconomyPlayer bank = balances.get(playerUUID); + return bank == null ? new EconomyPlayer(playerUUID) : copyPlayer(bank); + } + } + + private static EconomyPlayer getOrCreatePlayerBank(UUID playerUUID) { + return balances.computeIfAbsent(playerUUID, EconomyPlayer::new); + } + + public static void saveAllBalances() { + saveAllBalances(false); + } + + private static void saveAllBalances(boolean finalSave) { + synchronized (saveLock) { + do { + List playersToSave; + + synchronized (balancesLock) { + if (dirtyBalances.isEmpty()) { + return; + } + + playersToSave = dirtyBalances.stream() + .map(balances::get) + .filter(Objects::nonNull) + .map(EconomyManager::copyPlayer) + .toList(); + dirtyBalances.clear(); + } + + try { + playersDao.callBatchTasks(() -> { + for (EconomyPlayer player : playersToSave) { + playersDao.createOrUpdate(player); + } + + return null; + }); + } catch (Exception e) { + synchronized (balancesLock) { + for (EconomyPlayer player : playersToSave) { + dirtyBalances.add(player.getPlayerUUID()); + } + } + + if (finalSave) { + OMCLogger.error("CRITICAL: Failed to save economy balances during shutdown. Unsaved balances may be lost if the server stops.", e); + } else { + OMCLogger.error("Failed to save economy balances. Dirty balances will be retried on the next save.", e); + } + + return; + } + } while (finalSave); + } + } + + private static EconomyPlayer copyPlayer(EconomyPlayer player) { + return new EconomyPlayer(player.getPlayerUUID(), player.getBalance()); } public static Map loadAllBalances() { @@ -194,6 +299,28 @@ public static Map loadAllBalances() { return balances; } + private static void startAutoSaveTask() { + if (OMCPlugin.isUnitTestVersion() || autoSaveTask != null) { + return; + } + + autoSaveTask = Bukkit.getScheduler().runTaskTimerAsynchronously( + OMCPlugin.getInstance(), + () -> EconomyManager.saveAllBalances(), + AUTO_SAVE_INTERVAL_TICKS, + AUTO_SAVE_INTERVAL_TICKS + ); + } + + private static void stopAutoSaveTask() { + if (autoSaveTask == null) { + return; + } + + autoSaveTask.cancel(); + autoSaveTask = null; + } + public static String getFormattedBalance(UUID playerUUID) { String balance = String.valueOf(getBalance(playerUUID)); Currency currency = Currency.getInstance(Locale.FRANCE); diff --git a/src/main/java/fr/openmc/core/features/economy/models/EconomyPlayer.java b/src/main/java/fr/openmc/core/features/economy/models/EconomyPlayer.java index e523b9166..c897beeb9 100644 --- a/src/main/java/fr/openmc/core/features/economy/models/EconomyPlayer.java +++ b/src/main/java/fr/openmc/core/features/economy/models/EconomyPlayer.java @@ -24,6 +24,11 @@ public EconomyPlayer(UUID playerUUID) { this.balance = 0; } + public EconomyPlayer(UUID playerUUID, double balance) { + this.playerUUID = playerUUID; + this.balance = balance; + } + public void deposit(double amount) { balance += amount; } @@ -35,4 +40,8 @@ public boolean withdraw(double amount) { } return false; } + + public void setBalance(double balance) { + this.balance = balance; + } } diff --git a/src/test/java/fr/openmc/core/features/economy/EconomyManagerDirtySaveTest.java b/src/test/java/fr/openmc/core/features/economy/EconomyManagerDirtySaveTest.java new file mode 100644 index 000000000..89963bcd9 --- /dev/null +++ b/src/test/java/fr/openmc/core/features/economy/EconomyManagerDirtySaveTest.java @@ -0,0 +1,144 @@ +package fr.openmc.core.features.economy; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import com.j256.ormlite.dao.Dao; + +import java.lang.reflect.Field; +import java.lang.reflect.Method; +import java.lang.reflect.Proxy; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; +import java.util.UUID; +import java.util.concurrent.Callable; +import java.util.concurrent.atomic.AtomicBoolean; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import fr.openmc.core.features.economy.models.EconomyPlayer; + +public class EconomyManagerDirtySaveTest { + private Dao previousPlayersDao; + private Map previousBalances; + private Set previousDirtyBalances; + + @BeforeEach + public void setUp() throws Exception { + previousPlayersDao = replacePlayersDao(null); + previousBalances = replaceBalances(new HashMap<>()); + previousDirtyBalances = new HashSet<>(getDirtyBalances()); + + getDirtyBalances().clear(); + } + + @AfterEach + public void tearDown() throws Exception { + replacePlayersDao(previousPlayersDao); + replaceBalances(previousBalances); + + Set dirtyBalances = getDirtyBalances(); + dirtyBalances.clear(); + dirtyBalances.addAll(previousDirtyBalances); + } + + @Test + public void testFinalSaveFlushesBalancesDirtiedDuringSave() throws Exception { + UUID firstPlayerUUID = UUID.randomUUID(); + UUID secondPlayerUUID = UUID.randomUUID(); + Map persistedBalances = new HashMap<>(); + AtomicBoolean dirtiedDuringSave = new AtomicBoolean(false); + + replacePlayersDao(createPlayersDao(persistedBalances, () -> { + if (dirtiedDuringSave.compareAndSet(false, true)) { + EconomyManager.setBalance(secondPlayerUUID, 700.0); + } + })); + + EconomyManager.setBalance(firstPlayerUUID, 500.0); + + saveAllBalances(true); + + assertEquals(500.0, persistedBalances.get(firstPlayerUUID)); + assertEquals(700.0, persistedBalances.get(secondPlayerUUID)); + } + + @Test + public void testFinalSaveFlushesSameBalanceDirtiedDuringSave() throws Exception { + UUID playerUUID = UUID.randomUUID(); + Map persistedBalances = new HashMap<>(); + AtomicBoolean dirtiedDuringSave = new AtomicBoolean(false); + + replacePlayersDao(createPlayersDao(persistedBalances, () -> { + if (dirtiedDuringSave.compareAndSet(false, true)) { + EconomyManager.setBalance(playerUUID, 700.0); + } + })); + + EconomyManager.setBalance(playerUUID, 500.0); + + saveAllBalances(true); + + assertEquals(700.0, persistedBalances.get(playerUUID)); + } + + @SuppressWarnings("unchecked") + private static Dao createPlayersDao(Map persistedBalances, Runnable onCreateOrUpdate) { + return (Dao) Proxy.newProxyInstance( + Dao.class.getClassLoader(), + new Class[]{Dao.class}, + (proxy, method, args) -> switch (method.getName()) { + case "callBatchTasks" -> ((Callable) args[0]).call(); + case "createOrUpdate" -> { + onCreateOrUpdate.run(); + EconomyPlayer player = (EconomyPlayer) args[0]; + persistedBalances.put(player.getPlayerUUID(), player.getBalance()); + yield null; + } + case "toString" -> "InMemoryEconomyPlayerDao"; + case "hashCode" -> System.identityHashCode(proxy); + case "equals" -> proxy == args[0]; + default -> null; + } + ); + } + + @SuppressWarnings("unchecked") + private static Dao replacePlayersDao(Dao playersDao) throws Exception { + Field playersDaoField = EconomyManager.class.getDeclaredField("playersDao"); + playersDaoField.setAccessible(true); + + Dao previousPlayersDao = (Dao) playersDaoField.get(null); + playersDaoField.set(null, playersDao); + + return previousPlayersDao; + } + + @SuppressWarnings("unchecked") + private static Map replaceBalances(Map balances) throws Exception { + Field balancesField = EconomyManager.class.getDeclaredField("balances"); + balancesField.setAccessible(true); + + Map previousBalances = (Map) balancesField.get(null); + balancesField.set(null, balances); + + return previousBalances; + } + + @SuppressWarnings("unchecked") + private static Set getDirtyBalances() throws Exception { + Field dirtyBalancesField = EconomyManager.class.getDeclaredField("dirtyBalances"); + dirtyBalancesField.setAccessible(true); + + return (Set) dirtyBalancesField.get(null); + } + + private static void saveAllBalances(boolean finalSave) throws Exception { + Method saveAllBalancesMethod = EconomyManager.class.getDeclaredMethod("saveAllBalances", boolean.class); + saveAllBalancesMethod.setAccessible(true); + saveAllBalancesMethod.invoke(null, finalSave); + } +} diff --git a/src/test/java/fr/openmc/core/features/economy/EconomyManagerTest.java b/src/test/java/fr/openmc/core/features/economy/EconomyManagerTest.java index c09eddbde..0f47e5449 100644 --- a/src/test/java/fr/openmc/core/features/economy/EconomyManagerTest.java +++ b/src/test/java/fr/openmc/core/features/economy/EconomyManagerTest.java @@ -2,9 +2,15 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import com.j256.ormlite.dao.Dao; + +import java.lang.reflect.Field; import java.util.List; +import java.util.Map; +import java.util.UUID; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -13,6 +19,7 @@ import org.mockbukkit.mockbukkit.entity.PlayerMock; import fr.openmc.core.OMCPlugin; +import fr.openmc.core.features.economy.models.EconomyPlayer; import fr.openmc.mock.MockBukkitHelper; import fr.openmc.mock.ServerMock; @@ -71,6 +78,81 @@ public void testSetBalance() { assertEquals(EconomyManager.getBalance(player1.getUniqueId()), 500.0); } + @Test + public void testGetBalanceDoesNotCreateCachedAccount() { + UUID unknownPlayerUUID = UUID.randomUUID(); + + assertEquals(0.0, EconomyManager.getBalance(unknownPlayerUUID)); + assertFalse(EconomyManager.getBalances().containsKey(unknownPlayerUUID)); + } + + @Test + public void testBalancesSnapshotCannotBeMutated() { + EconomyManager.setBalance(player1.getUniqueId(), 100.0); + + Map balances = EconomyManager.getBalances(); + + assertThrows(UnsupportedOperationException.class, () -> balances.remove(player1.getUniqueId())); + + balances.get(player1.getUniqueId()).setBalance(50.0); + + assertEquals(100.0, EconomyManager.getBalance(player1.getUniqueId())); + } + + @Test + public void testGetPlayerBankReturnsSnapshot() { + EconomyManager.setBalance(player1.getUniqueId(), 100.0); + + EconomyPlayer bank = EconomyManager.getPlayerBank(player1.getUniqueId()); + bank.setBalance(50.0); + + assertEquals(100.0, EconomyManager.getBalance(player1.getUniqueId())); + } + + @Test + public void testMarkPlayerBankDirtyStoresSnapshot() { + EconomyPlayer bank = new EconomyPlayer(player1.getUniqueId(), 100.0); + + EconomyManager.markPlayerBankDirty(bank); + bank.setBalance(50.0); + + assertEquals(100.0, EconomyManager.getBalance(player1.getUniqueId())); + } + + @Test + public void testBalanceChangesAreSavedOnSaveAll() { + UUID playerUUID = player1.getUniqueId(); + + EconomyManager.setBalance(playerUUID, 500.0); + + Map balancesBeforeSave = EconomyManager.loadAllBalances(); + assertFalse(balancesBeforeSave.containsKey(playerUUID)); + + EconomyManager.saveAllBalances(); + + Map balancesAfterSave = EconomyManager.loadAllBalances(); + assertEquals(500.0, balancesAfterSave.get(playerUUID).getBalance()); + } + + @Test + public void testFailedSaveKeepsBalancesDirtyForRetry() throws Exception { + UUID playerUUID = player1.getUniqueId(); + + EconomyManager.setBalance(playerUUID, 500.0); + + Dao playersDao = replacePlayersDao(null); + try { + EconomyManager.saveAllBalances(); + } finally { + replacePlayersDao(playersDao); + } + + EconomyManager.saveAllBalances(); + + Map balancesAfterRetry = EconomyManager.loadAllBalances(); + assertEquals(500.0, balancesAfterRetry.get(playerUUID).getBalance()); + } + @Test public void testAddBalanceWithReasonRegistersTransaction() { EconomyManager.addBalance(player1.getUniqueId(), 100.0, "Test Reason"); @@ -157,4 +239,15 @@ public void testTransferBalanceWithReasonRegistersTransaction() { assertTrue(found); } + + @SuppressWarnings("unchecked") + private static Dao replacePlayersDao(Dao playersDao) throws Exception { + Field playersDaoField = EconomyManager.class.getDeclaredField("playersDao"); + playersDaoField.setAccessible(true); + + Dao previousPlayersDao = (Dao) playersDaoField.get(null); + playersDaoField.set(null, playersDao); + + return previousPlayersDao; + } }