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..06de455c0 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; // 5 minutes + 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,83 @@ 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()); } } + /** + * 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("CRITIQUE: Impossible de sauvegarder les soldes de l'economie pendant l'arret. Des soldes non sauvegardes peuvent etre perdus si le serveur s'arrete.", e); + } else { + OMCLogger.error("Impossible de sauvegarder les soldes de l'economie. Les soldes modifies seront reessayes a la prochaine sauvegarde.", e); + } + + return; + } + } while (finalSave); + } + } + + private static EconomyPlayer copyPlayer(EconomyPlayer player) { + return new EconomyPlayer(player.getPlayerUUID(), player.getBalance()); } public static Map loadAllBalances() { @@ -194,6 +289,28 @@ public static Map loadAllBalances() { return balances; } + private static void startAutoSaveTask() { + if (OMCPlugin.getInstance() == null || 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..3bd67a83b --- /dev/null +++ b/src/test/java/fr/openmc/core/features/economy/EconomyManagerDirtySaveTest.java @@ -0,0 +1,124 @@ +package fr.openmc.core.features.economy; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import com.j256.ormlite.jdbc.JdbcConnectionSource; +import com.j256.ormlite.support.ConnectionSource; + +import java.sql.Connection; +import java.sql.DriverManager; +import java.sql.SQLException; +import java.sql.Statement; +import java.util.Map; +import java.util.UUID; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; + +import org.h2.api.Trigger; +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 ConnectionSource connectionSource; + private EconomyManager economyManager; + private String databaseUrl; + + @BeforeEach + public void setUp() throws Exception { + databaseUrl = "jdbc:h2:mem:economy_dirty_save_" + UUID.randomUUID() + ";MODE=MySQL;DB_CLOSE_DELAY=-1"; + connectionSource = new JdbcConnectionSource(databaseUrl); + economyManager = new EconomyManager(); + + economyManager.initDB(connectionSource); + economyManager.init(); + createDirtyDuringSaveTriggers(); + } + + @AfterEach + public void tearDown() throws Exception { + DirtyDuringSaveTrigger.clear(); + + if (connectionSource != null) { + connectionSource.close(); + } + } + + @Test + public void testFinalSaveFlushesBalancesDirtiedDuringSave() throws Exception { + UUID firstPlayerUUID = UUID.randomUUID(); + UUID secondPlayerUUID = UUID.randomUUID(); + + DirtyDuringSaveTrigger.runOnce(() -> EconomyManager.setBalance(secondPlayerUUID, 700.0)); + + EconomyManager.setBalance(firstPlayerUUID, 500.0); + + economyManager.save(); + + Map balancesAfterSave = EconomyManager.loadAllBalances(); + assertEquals(500.0, balancesAfterSave.get(firstPlayerUUID).getBalance()); + assertEquals(700.0, balancesAfterSave.get(secondPlayerUUID).getBalance()); + } + + @Test + public void testFinalSaveFlushesSameBalanceDirtiedDuringSave() throws Exception { + UUID playerUUID = UUID.randomUUID(); + + DirtyDuringSaveTrigger.runOnce(() -> EconomyManager.setBalance(playerUUID, 700.0)); + + EconomyManager.setBalance(playerUUID, 500.0); + + economyManager.save(); + + Map balancesAfterSave = EconomyManager.loadAllBalances(); + assertEquals(700.0, balancesAfterSave.get(playerUUID).getBalance()); + } + + private void createDirtyDuringSaveTriggers() throws SQLException { + try (Connection connection = DriverManager.getConnection(databaseUrl); + Statement statement = connection.createStatement()) { + String triggerClassName = DirtyDuringSaveTrigger.class.getName(); + + statement.execute("CREATE TRIGGER dirty_during_insert BEFORE INSERT ON balances FOR EACH ROW CALL '" + triggerClassName + "'"); + statement.execute("CREATE TRIGGER dirty_during_update BEFORE UPDATE ON balances FOR EACH ROW CALL '" + triggerClassName + "'"); + } + } + + public static final class DirtyDuringSaveTrigger implements Trigger { + private static final AtomicReference callback = new AtomicReference<>(); + private static final AtomicBoolean callbackInvoked = new AtomicBoolean(false); + + static void runOnce(Runnable callback) { + DirtyDuringSaveTrigger.callback.set(callback); + callbackInvoked.set(false); + } + + static void clear() { + callback.set(null); + callbackInvoked.set(false); + } + + @Override + public void init(Connection connection, String schemaName, String triggerName, String tableName, boolean before, int type) { + } + + @Override + public void fire(Connection connection, Object[] oldRow, Object[] newRow) { + Runnable currentCallback = callback.get(); + + if (currentCallback != null && callbackInvoked.compareAndSet(false, true)) { + currentCallback.run(); + } + } + + @Override + public void close() { + } + + @Override + public void remove() { + } + } +} 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; + } }