From 6c8e2c385fabf69678e3eb7f7f9639990a496e6e Mon Sep 17 00:00:00 2001 From: gtolontop Date: Mon, 22 Jun 2026 18:09:37 +0200 Subject: [PATCH 1/9] fix(economy): batch balance persistence --- .../core/features/economy/EconomyManager.java | 133 ++++++++++++++---- .../economy/models/EconomyPlayer.java | 9 ++ .../features/economy/EconomyManagerTest.java | 18 +++ 3 files changed, 132 insertions(+), 28 deletions(-) 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..6b1aacfa1 100644 --- a/src/main/java/fr/openmc/core/features/economy/EconomyManager.java +++ b/src/main/java/fr/openmc/core/features/economy/EconomyManager.java @@ -9,6 +9,8 @@ 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; @@ -16,6 +18,8 @@ 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; @@ -30,6 +34,10 @@ public class EconomyManager extends Feature implements DatabaseFeature, HasComma 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 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 +51,8 @@ public class EconomyManager extends Feature implements DatabaseFeature, HasComma @Override public void init() { balances = loadAllBalances(); + dirtyBalances.clear(); + startAutoSaveTask(); } @Override @@ -61,9 +71,17 @@ public void initDB(ConnectionSource connectionSource) throws SQLException { playersDao = DaoManager.createDao(connectionSource, EconomyPlayer.class); } + @Override + protected void save() { + stopAutoSaveTask(); + saveAllBalances(); + } + public static double getBalance(UUID playerUUID) { - EconomyPlayer bank = getPlayerBank(playerUUID); - return bank.getBalance(); + synchronized (balancesLock) { + EconomyPlayer bank = getPlayerBank(playerUUID); + return bank.getBalance(); + } } public static void addBalance(UUID playerUUID, double amount) { @@ -71,8 +89,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 = getPlayerBank(playerUUID); + bank.deposit(amount); + savePlayerBank(bank); + } if (reason != null) { TransactionsManager.registerTransaction(new Transaction( @@ -83,7 +104,6 @@ public static void addBalance(UUID playerUUID, double amount, @Nullable String r )); } - savePlayerBank(bank); } public static boolean withdrawBalance(UUID playerUUID, double amount) { @@ -91,24 +111,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 = getPlayerBank(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); + } - return true; + if (reason != null) { + TransactionsManager.registerTransaction(new Transaction( + "CONSOLE", + playerUUID.toString(), + amount, + reason + )); } - return false; + return true; } /** @@ -152,10 +174,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 = getPlayerBank(playerUUID); + bank.setBalance(amount); + savePlayerBank(bank); + } } public static String getMiniBalance(UUID playerUUID) { @@ -165,19 +188,51 @@ public static String getMiniBalance(UUID playerUUID) { } public static void savePlayerBank(EconomyPlayer player) { - try { + synchronized (balancesLock) { balances.put(player.getPlayerUUID(), player); - playersDao.createOrUpdate(player); - } catch (SQLException e) { - throw new RuntimeException(e); + dirtyBalances.add(player.getPlayerUUID()); } } public static EconomyPlayer getPlayerBank(UUID playerUUID) { - EconomyPlayer bank = balances.get(playerUUID); - if (bank != null) - return bank; - return new EconomyPlayer(playerUUID); + synchronized (balancesLock) { + return balances.computeIfAbsent(playerUUID, EconomyPlayer::new); + } + } + + public static void saveAllBalances() { + List playersToSave; + + synchronized (balancesLock) { + if (dirtyBalances.isEmpty()) { + return; + } + + playersToSave = dirtyBalances.stream() + .map(balances::get) + .filter(Objects::nonNull) + .map(player -> new EconomyPlayer(player.getPlayerUUID(), player.getBalance())) + .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()); + } + } + + OMCLogger.error("Failed to save economy balances", e); + } } public static Map loadAllBalances() { @@ -194,6 +249,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/EconomyManagerTest.java b/src/test/java/fr/openmc/core/features/economy/EconomyManagerTest.java index c09eddbde..8c747bb22 100644 --- a/src/test/java/fr/openmc/core/features/economy/EconomyManagerTest.java +++ b/src/test/java/fr/openmc/core/features/economy/EconomyManagerTest.java @@ -5,6 +5,8 @@ import static org.junit.jupiter.api.Assertions.assertTrue; 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 +15,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 +74,21 @@ public void testSetBalance() { assertEquals(EconomyManager.getBalance(player1.getUniqueId()), 500.0); } + @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 testAddBalanceWithReasonRegistersTransaction() { EconomyManager.addBalance(player1.getUniqueId(), 100.0, "Test Reason"); From a2a7db98073e5c0618ecbad8bfc4ce136eb0f677 Mon Sep 17 00:00:00 2001 From: gtolontop Date: Mon, 22 Jun 2026 18:32:07 +0200 Subject: [PATCH 2/9] fix(economy): serialize balance saves --- .../core/features/economy/EconomyManager.java | 70 +++++++++++-------- .../features/economy/EconomyManagerTest.java | 8 +++ 2 files changed, 47 insertions(+), 31 deletions(-) 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 6b1aacfa1..365f9d6fa 100644 --- a/src/main/java/fr/openmc/core/features/economy/EconomyManager.java +++ b/src/main/java/fr/openmc/core/features/economy/EconomyManager.java @@ -36,6 +36,7 @@ public class EconomyManager extends Feature implements DatabaseFeature, HasComma 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; @@ -79,8 +80,8 @@ protected void save() { public static double getBalance(UUID playerUUID) { synchronized (balancesLock) { - EconomyPlayer bank = getPlayerBank(playerUUID); - return bank.getBalance(); + EconomyPlayer bank = balances.get(playerUUID); + return bank == null ? 0 : bank.getBalance(); } } @@ -90,7 +91,7 @@ public static void addBalance(UUID playerUUID, double amount) { public static void addBalance(UUID playerUUID, double amount, @Nullable String reason) { synchronized (balancesLock) { - EconomyPlayer bank = getPlayerBank(playerUUID); + EconomyPlayer bank = getOrCreatePlayerBank(playerUUID); bank.deposit(amount); savePlayerBank(bank); } @@ -112,7 +113,7 @@ public static boolean withdrawBalance(UUID playerUUID, double amount) { public static boolean withdrawBalance(UUID playerUUID, double amount, @Nullable String reason) { synchronized (balancesLock) { - EconomyPlayer bank = getPlayerBank(playerUUID); + EconomyPlayer bank = getOrCreatePlayerBank(playerUUID); if (!bank.withdraw(amount)) { return false; @@ -175,7 +176,7 @@ public static boolean transferBalance(UUID fromPlayer, UUID toPlayer, double amo public static void setBalance(UUID playerUUID, double amount) { synchronized (balancesLock) { - EconomyPlayer bank = getPlayerBank(playerUUID); + EconomyPlayer bank = getOrCreatePlayerBank(playerUUID); bank.setBalance(amount); savePlayerBank(bank); } @@ -196,42 +197,49 @@ public static void savePlayerBank(EconomyPlayer player) { public static EconomyPlayer getPlayerBank(UUID playerUUID) { synchronized (balancesLock) { - return balances.computeIfAbsent(playerUUID, EconomyPlayer::new); + EconomyPlayer bank = balances.get(playerUUID); + return bank == null ? new EconomyPlayer(playerUUID) : bank; } } + private static EconomyPlayer getOrCreatePlayerBank(UUID playerUUID) { + return balances.computeIfAbsent(playerUUID, EconomyPlayer::new); + } + public static void saveAllBalances() { - List playersToSave; + synchronized (saveLock) { + List playersToSave; - synchronized (balancesLock) { - if (dirtyBalances.isEmpty()) { - return; - } + synchronized (balancesLock) { + if (dirtyBalances.isEmpty()) { + return; + } - playersToSave = dirtyBalances.stream() - .map(balances::get) - .filter(Objects::nonNull) - .map(player -> new EconomyPlayer(player.getPlayerUUID(), player.getBalance())) - .toList(); - dirtyBalances.clear(); - } + playersToSave = dirtyBalances.stream() + .map(balances::get) + .filter(Objects::nonNull) + .map(player -> new EconomyPlayer(player.getPlayerUUID(), player.getBalance())) + .toList(); + dirtyBalances.clear(); + } - try { - playersDao.callBatchTasks(() -> { - for (EconomyPlayer player : playersToSave) { - playersDao.createOrUpdate(player); + 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()); + } } - return null; - }); - } catch (Exception e) { - synchronized (balancesLock) { - for (EconomyPlayer player : playersToSave) { - dirtyBalances.add(player.getPlayerUUID()); - } + OMCLogger.error("Failed to save economy balances", e); } - - OMCLogger.error("Failed to save economy balances", e); } } 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 8c747bb22..a56d7b388 100644 --- a/src/test/java/fr/openmc/core/features/economy/EconomyManagerTest.java +++ b/src/test/java/fr/openmc/core/features/economy/EconomyManagerTest.java @@ -74,6 +74,14 @@ 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 testBalanceChangesAreSavedOnSaveAll() { UUID playerUUID = player1.getUniqueId(); From 609702df9b07ef2c7310f35112ca77dabd72d01b Mon Sep 17 00:00:00 2001 From: gtolontop Date: Mon, 22 Jun 2026 18:38:38 +0200 Subject: [PATCH 3/9] fix(economy): expose balance snapshot --- .../openmc/core/features/economy/EconomyManager.java | 8 ++++++-- .../core/features/economy/EconomyManagerTest.java | 11 +++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) 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 365f9d6fa..3a62633e0 100644 --- a/src/main/java/fr/openmc/core/features/economy/EconomyManager.java +++ b/src/main/java/fr/openmc/core/features/economy/EconomyManager.java @@ -17,7 +17,6 @@ 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; @@ -30,7 +29,6 @@ @Credit(developers = {"Axeno", "Piquel Chips", "PuppyTransGirl", "Gyro"}) public class EconomyManager extends Feature implements DatabaseFeature, HasCommands { - @Getter private static Map balances; private static Dao playersDao; @@ -85,6 +83,12 @@ public static double getBalance(UUID playerUUID) { } } + public static Map getBalances() { + synchronized (balancesLock) { + return Collections.unmodifiableMap(new HashMap<>(balances)); + } + } + public static void addBalance(UUID playerUUID, double amount) { addBalance(playerUUID, amount, null); } 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 a56d7b388..26b2cf3d9 100644 --- a/src/test/java/fr/openmc/core/features/economy/EconomyManagerTest.java +++ b/src/test/java/fr/openmc/core/features/economy/EconomyManagerTest.java @@ -2,6 +2,7 @@ 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 java.util.List; @@ -82,6 +83,16 @@ public void testGetBalanceDoesNotCreateCachedAccount() { 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())); + assertEquals(100.0, EconomyManager.getBalance(player1.getUniqueId())); + } + @Test public void testBalanceChangesAreSavedOnSaveAll() { UUID playerUUID = player1.getUniqueId(); From 6edadb1f72ad421260565f7d0f3ffa368ad7df52 Mon Sep 17 00:00:00 2001 From: gtolontop Date: Mon, 22 Jun 2026 18:55:52 +0200 Subject: [PATCH 4/9] fix(economy): protect balance snapshots --- .../core/features/economy/EconomyManager.java | 28 +++++++++++++++---- .../features/economy/EconomyManagerTest.java | 13 +++++++++ 2 files changed, 35 insertions(+), 6 deletions(-) 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 3a62633e0..f3c0fbfa3 100644 --- a/src/main/java/fr/openmc/core/features/economy/EconomyManager.java +++ b/src/main/java/fr/openmc/core/features/economy/EconomyManager.java @@ -73,7 +73,7 @@ public void initDB(ConnectionSource connectionSource) throws SQLException { @Override protected void save() { stopAutoSaveTask(); - saveAllBalances(); + saveAllBalances(true); } public static double getBalance(UUID playerUUID) { @@ -85,7 +85,11 @@ public static double getBalance(UUID playerUUID) { public static Map getBalances() { synchronized (balancesLock) { - return Collections.unmodifiableMap(new HashMap<>(balances)); + Map snapshot = new HashMap<>(); + + balances.forEach((playerUUID, player) -> snapshot.put(playerUUID, copyPlayer(player))); + + return Collections.unmodifiableMap(snapshot); } } @@ -202,7 +206,7 @@ public static void savePlayerBank(EconomyPlayer player) { public static EconomyPlayer getPlayerBank(UUID playerUUID) { synchronized (balancesLock) { EconomyPlayer bank = balances.get(playerUUID); - return bank == null ? new EconomyPlayer(playerUUID) : bank; + return bank == null ? new EconomyPlayer(playerUUID) : copyPlayer(bank); } } @@ -211,6 +215,10 @@ private static EconomyPlayer getOrCreatePlayerBank(UUID playerUUID) { } public static void saveAllBalances() { + saveAllBalances(false); + } + + private static void saveAllBalances(boolean finalSave) { synchronized (saveLock) { List playersToSave; @@ -222,7 +230,7 @@ public static void saveAllBalances() { playersToSave = dirtyBalances.stream() .map(balances::get) .filter(Objects::nonNull) - .map(player -> new EconomyPlayer(player.getPlayerUUID(), player.getBalance())) + .map(EconomyManager::copyPlayer) .toList(); dirtyBalances.clear(); } @@ -242,11 +250,19 @@ public static void saveAllBalances() { } } - OMCLogger.error("Failed to save economy balances", e); + 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); + } } } } + private static EconomyPlayer copyPlayer(EconomyPlayer player) { + return new EconomyPlayer(player.getPlayerUUID(), player.getBalance()); + } + public static Map loadAllBalances() { Map balances = new HashMap<>(); try { @@ -268,7 +284,7 @@ private static void startAutoSaveTask() { autoSaveTask = Bukkit.getScheduler().runTaskTimerAsynchronously( OMCPlugin.getInstance(), - EconomyManager::saveAllBalances, + () -> EconomyManager.saveAllBalances(), AUTO_SAVE_INTERVAL_TICKS, AUTO_SAVE_INTERVAL_TICKS ); 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 26b2cf3d9..808d0d8e7 100644 --- a/src/test/java/fr/openmc/core/features/economy/EconomyManagerTest.java +++ b/src/test/java/fr/openmc/core/features/economy/EconomyManagerTest.java @@ -90,6 +90,19 @@ public void testBalancesSnapshotCannotBeMutated() { 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())); } From 43dc10f5472efba018a5d52a130ab6d57ea231c9 Mon Sep 17 00:00:00 2001 From: gtolontop Date: Mon, 22 Jun 2026 20:10:46 +0200 Subject: [PATCH 5/9] fix(economy): copy saved balance entries --- .../core/features/economy/EconomyManager.java | 2 +- .../features/economy/EconomyManagerTest.java | 43 +++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) 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 f3c0fbfa3..060163bc1 100644 --- a/src/main/java/fr/openmc/core/features/economy/EconomyManager.java +++ b/src/main/java/fr/openmc/core/features/economy/EconomyManager.java @@ -198,7 +198,7 @@ public static String getMiniBalance(UUID playerUUID) { public static void savePlayerBank(EconomyPlayer player) { synchronized (balancesLock) { - balances.put(player.getPlayerUUID(), player); + balances.put(player.getPlayerUUID(), copyPlayer(player)); dirtyBalances.add(player.getPlayerUUID()); } } 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 808d0d8e7..d972c262f 100644 --- a/src/test/java/fr/openmc/core/features/economy/EconomyManagerTest.java +++ b/src/test/java/fr/openmc/core/features/economy/EconomyManagerTest.java @@ -5,6 +5,9 @@ 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; @@ -106,6 +109,16 @@ public void testGetPlayerBankReturnsSnapshot() { assertEquals(100.0, EconomyManager.getBalance(player1.getUniqueId())); } + @Test + public void testSavePlayerBankStoresSnapshot() { + EconomyPlayer bank = new EconomyPlayer(player1.getUniqueId(), 100.0); + + EconomyManager.savePlayerBank(bank); + bank.setBalance(50.0); + + assertEquals(100.0, EconomyManager.getBalance(player1.getUniqueId())); + } + @Test public void testBalanceChangesAreSavedOnSaveAll() { UUID playerUUID = player1.getUniqueId(); @@ -121,6 +134,25 @@ public void testBalanceChangesAreSavedOnSaveAll() { 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"); @@ -207,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; + } } From d6a10d550e8394c8b5321ac72b9ce1662073785f Mon Sep 17 00:00:00 2001 From: gtolontop Date: Mon, 22 Jun 2026 20:16:42 +0200 Subject: [PATCH 6/9] refactor(economy): clarify dirty balance cache update --- .../fr/openmc/core/features/economy/EconomyManager.java | 8 ++++---- .../openmc/core/features/economy/EconomyManagerTest.java | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) 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 060163bc1..31a9a201e 100644 --- a/src/main/java/fr/openmc/core/features/economy/EconomyManager.java +++ b/src/main/java/fr/openmc/core/features/economy/EconomyManager.java @@ -101,7 +101,7 @@ public static void addBalance(UUID playerUUID, double amount, @Nullable String r synchronized (balancesLock) { EconomyPlayer bank = getOrCreatePlayerBank(playerUUID); bank.deposit(amount); - savePlayerBank(bank); + markPlayerBankDirty(bank); } if (reason != null) { @@ -127,7 +127,7 @@ public static boolean withdrawBalance(UUID playerUUID, double amount, @Nullable return false; } - savePlayerBank(bank); + markPlayerBankDirty(bank); } if (reason != null) { @@ -186,7 +186,7 @@ public static void setBalance(UUID playerUUID, double amount) { synchronized (balancesLock) { EconomyPlayer bank = getOrCreatePlayerBank(playerUUID); bank.setBalance(amount); - savePlayerBank(bank); + markPlayerBankDirty(bank); } } @@ -196,7 +196,7 @@ public static String getMiniBalance(UUID playerUUID) { return getFormattedSimplifiedNumber(balance); } - public static void savePlayerBank(EconomyPlayer player) { + public static void markPlayerBankDirty(EconomyPlayer player) { synchronized (balancesLock) { balances.put(player.getPlayerUUID(), copyPlayer(player)); dirtyBalances.add(player.getPlayerUUID()); 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 d972c262f..0f47e5449 100644 --- a/src/test/java/fr/openmc/core/features/economy/EconomyManagerTest.java +++ b/src/test/java/fr/openmc/core/features/economy/EconomyManagerTest.java @@ -110,10 +110,10 @@ public void testGetPlayerBankReturnsSnapshot() { } @Test - public void testSavePlayerBankStoresSnapshot() { + public void testMarkPlayerBankDirtyStoresSnapshot() { EconomyPlayer bank = new EconomyPlayer(player1.getUniqueId(), 100.0); - EconomyManager.savePlayerBank(bank); + EconomyManager.markPlayerBankDirty(bank); bank.setBalance(50.0); assertEquals(100.0, EconomyManager.getBalance(player1.getUniqueId())); From 60ffe0ffd2307ccd36d8c4b378a54f87068cbea3 Mon Sep 17 00:00:00 2001 From: gtolontop Date: Mon, 22 Jun 2026 20:21:38 +0200 Subject: [PATCH 7/9] docs(economy): document bank snapshot access --- .../fr/openmc/core/features/economy/EconomyManager.java | 8 ++++++++ 1 file changed, 8 insertions(+) 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 31a9a201e..67d12e522 100644 --- a/src/main/java/fr/openmc/core/features/economy/EconomyManager.java +++ b/src/main/java/fr/openmc/core/features/economy/EconomyManager.java @@ -203,6 +203,14 @@ public static void markPlayerBankDirty(EconomyPlayer 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) { synchronized (balancesLock) { EconomyPlayer bank = balances.get(playerUUID); From 2dcc126e7bb2d6fc047013045a7b07a577ad0135 Mon Sep 17 00:00:00 2001 From: gtolontop Date: Mon, 22 Jun 2026 20:32:39 +0200 Subject: [PATCH 8/9] fix(economy): drain dirty balances on shutdown save --- .../core/features/economy/EconomyManager.java | 68 ++++++---- .../economy/EconomyManagerDirtySaveTest.java | 125 ++++++++++++++++++ 2 files changed, 165 insertions(+), 28 deletions(-) create mode 100644 src/test/java/fr/openmc/core/features/economy/EconomyManagerDirtySaveTest.java 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 67d12e522..dca060ba1 100644 --- a/src/main/java/fr/openmc/core/features/economy/EconomyManager.java +++ b/src/main/java/fr/openmc/core/features/economy/EconomyManager.java @@ -203,6 +203,14 @@ public static void markPlayerBankDirty(EconomyPlayer player) { } } + /** + * @deprecated Use {@link #markPlayerBankDirty(EconomyPlayer)}. + */ + @Deprecated(since = "2.5.0", forRemoval = false) + public static void savePlayerBank(EconomyPlayer player) { + markPlayerBankDirty(player); + } + /** * Returns a snapshot of a player's economy data. *

@@ -228,42 +236,46 @@ public static void saveAllBalances() { private static void saveAllBalances(boolean finalSave) { synchronized (saveLock) { - List playersToSave; + do { + List playersToSave; - synchronized (balancesLock) { - if (dirtyBalances.isEmpty()) { - return; - } + synchronized (balancesLock) { + if (dirtyBalances.isEmpty()) { + return; + } - playersToSave = dirtyBalances.stream() - .map(balances::get) - .filter(Objects::nonNull) - .map(EconomyManager::copyPlayer) - .toList(); - dirtyBalances.clear(); - } + 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); + 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()); + } } - 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); } - } - 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); } } 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..e8b5cc3e2 --- /dev/null +++ b/src/test/java/fr/openmc/core/features/economy/EconomyManagerDirtySaveTest.java @@ -0,0 +1,125 @@ +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)); + } + + @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); + } +} From 55cb4049321cf2a9714b759c5444b3e24afb719c Mon Sep 17 00:00:00 2001 From: gtolontop Date: Mon, 22 Jun 2026 20:40:54 +0200 Subject: [PATCH 9/9] test(economy): cover final save same balance retry --- .../core/features/economy/EconomyManager.java | 4 +++- .../economy/EconomyManagerDirtySaveTest.java | 19 +++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) 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 dca060ba1..ff927a82b 100644 --- a/src/main/java/fr/openmc/core/features/economy/EconomyManager.java +++ b/src/main/java/fr/openmc/core/features/economy/EconomyManager.java @@ -204,7 +204,9 @@ public static void markPlayerBankDirty(EconomyPlayer player) { } /** - * @deprecated Use {@link #markPlayerBankDirty(EconomyPlayer)}. + * @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) { diff --git a/src/test/java/fr/openmc/core/features/economy/EconomyManagerDirtySaveTest.java b/src/test/java/fr/openmc/core/features/economy/EconomyManagerDirtySaveTest.java index e8b5cc3e2..89963bcd9 100644 --- a/src/test/java/fr/openmc/core/features/economy/EconomyManagerDirtySaveTest.java +++ b/src/test/java/fr/openmc/core/features/economy/EconomyManagerDirtySaveTest.java @@ -66,6 +66,25 @@ public void testFinalSaveFlushesBalancesDirtiedDuringSave() throws Exception { 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(