fix: register economy command permissions and save synchronously on shutdown (1.19.2)

pom.xml

@@ -65,7 +65,7 @@
         <!-- Do not change unless you want different name for local builds. -->
         <build.number>-LOCAL</build.number>
         <!-- This allows to change between versions. -->
-        <build.version>1.19.1</build.version>
+        <build.version>1.19.2</build.version>
         <!-- Sonar Cloud -->
         <sonar.projectKey>BentoBoxWorld_addon-invSwitcher</sonar.projectKey>
         <sonar.organization>bentobox-world</sonar.organization>
@@ -323,7 +323,7 @@
             <plugin>
                 <groupId>org.jacoco</groupId>
                 <artifactId>jacoco-maven-plugin</artifactId>
-                <version>0.8.12</version>
+                <version>0.8.15</version>
                 <configuration>
                     <append>true</append>
                     <excludes>

src/main/java/com/wasteofplastic/invswitcher/Store.java

@@ -474,10 +474,36 @@ public void storeAndSave(Player player, World world, boolean shutdown) {
         }
         if (settings.isStatistics()) {
             String k = settings.isIslandsStatistics() ? islandKey : worldKey;
-            saveStats(store, player, k, shutdown).thenAccept(database::saveObjectAsync);
+            // On shutdown saveStats() gathers synchronously and returns an already-completed
+            // future, so thenAccept runs on this thread and persist() writes before we return.
+            saveStats(store, player, k, shutdown).thenAccept(s -> persist(s, shutdown));
             return;
         }
-        database.saveObjectAsync(store);
+        persist(store, shutdown);
+    }
+
+    /**
+     * Writes the store to the database, synchronously when the server is shutting down.
+     * <p>
+     * Saves are normally asynchronous, but a shutdown save must not be. BentoBox closes its
+     * database immediately after addons are disabled, and players are only kicked afterwards, so an
+     * asynchronous write issued from {@link #saveOnShutdown()} loses the race and is silently
+     * dropped — and the {@code PlayerQuitEvent} that would otherwise save them fires after the
+     * database is already closed.
+     * <p>
+     * The effect was that everything a player did since their last world change went unsaved when
+     * the server stopped. Because {@code PlayerListener.onPlayerJoin} re-applies the stored
+     * inventory on login, the stale snapshot then overwrote the player's real inventory and they
+     * were rolled back to their last world change.
+     * @param store - the store to write
+     * @param shutdown - true if this is a shutdown save, which must be synchronous
+     */
+    private void persist(InventoryStorage store, boolean shutdown) {
+        if (shutdown) {
+            database.saveObject(store);
+        } else {
+            database.saveObjectAsync(store);
+        }
     }
 
     private CompletableFuture<InventoryStorage> saveStats(InventoryStorage store, Player player, String worldName,

src/main/resources/addon.yml

@@ -5,4 +5,30 @@ api-version: 3.17.0
 
 authors: tastybento
 
-softdepend: AcidIsland, BSkyBlock, SkyGrid, CaveBock, AOneBlock
+softdepend: AcidIsland, BSkyBlock, SkyGrid, CaveBock, AOneBlock
+
+# Economy command permissions. These are only usable if options.money is enabled and Vault is
+# installed - InvSwitcher does not register the commands otherwise. The [gamemode] placeholder is
+# expanded by BentoBox into every game mode's permission prefix, e.g. bskyblock.invswitcher.balance
+permissions:
+  '[gamemode].invswitcher.balance':
+    description: Player can use the balance command
+    default: true
+  '[gamemode].invswitcher.pay':
+    description: Player can use the pay command
+    default: true
+  '[gamemode].invswitcher.admin.eco':
+    description: Player can use the admin eco command
+    default: op
+  '[gamemode].invswitcher.admin.eco.balance':
+    description: Player can use the admin eco balance command
+    default: op
+  '[gamemode].invswitcher.admin.eco.give':
+    description: Player can use the admin eco give command
+    default: op
+  '[gamemode].invswitcher.admin.eco.take':
+    description: Player can use the admin eco take command
+    default: op
+  '[gamemode].invswitcher.admin.eco.set':
+    description: Player can use the admin eco set command
+    default: op

src/test/java/com/wasteofplastic/invswitcher/StoreTest.java

@@ -18,6 +18,7 @@
 import static org.mockito.Mockito.when;
 
 import java.io.File;
+import java.lang.reflect.Field;
 import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.Path;
@@ -54,6 +55,7 @@
 import org.mockito.quality.Strictness;
 
 import world.bentobox.bentobox.BentoBox;
+import world.bentobox.bentobox.database.Database;
 import world.bentobox.bentobox.database.DatabaseSetup.DatabaseType;
 import com.wasteofplastic.invswitcher.dataobjects.InventoryStorage;
 
@@ -905,4 +907,74 @@ void testGetStorageKeyForEventMultipleIslandsNotOwner() {
         }
     }
 
+    // --- Shutdown save tests ---
+
+    /**
+     * Replaces the Store's database with a mock so the save path can be observed.
+     */
+    @SuppressWarnings("unchecked")
+    private Database<InventoryStorage> injectMockDatabase() throws Exception {
+        Database<InventoryStorage> db = mock(Database.class);
+        Field field = Store.class.getDeclaredField("database");
+        field.setAccessible(true);
+        field.set(s, db);
+        return db;
+    }
+
+    /**
+     * A shutdown save must be synchronous. BentoBox closes its database immediately after addons
+     * are disabled, and players are only kicked afterwards, so an asynchronous write issued from
+     * saveOnShutdown() loses the race and is silently dropped. Everything the player did since
+     * their last world change was then lost, and because onPlayerJoin re-applies the stored
+     * inventory on login, the stale snapshot overwrote their real inventory on the next restart.
+     */
+    @Test
+    void testShutdownSaveIsSynchronous() throws Exception {
+        sets.setStatistics(false);
+        sets.setAdvancements(false);
+        Database<InventoryStorage> db = injectMockDatabase();
+
+        try (MockedStatic<Bukkit> mockedBukkit = mockStatic(Bukkit.class, Mockito.RETURNS_MOCKS)) {
+            s.storeAndSave(player, world, true);
+        }
+
+        verify(db).saveObject(any(InventoryStorage.class));
+        verify(db, never()).saveObjectAsync(any(InventoryStorage.class));
+    }
+
+    /**
+     * The statistics branch returns early, so it needs its own check that a shutdown save is
+     * written synchronously.
+     */
+    @Test
+    void testShutdownSaveIsSynchronousWithStatistics() throws Exception {
+        sets.setStatistics(true);
+        sets.setAdvancements(false);
+        Database<InventoryStorage> db = injectMockDatabase();
+
+        try (MockedStatic<Bukkit> mockedBukkit = mockStatic(Bukkit.class, Mockito.RETURNS_MOCKS)) {
+            s.storeAndSave(player, world, true);
+        }
+
+        verify(db).saveObject(any(InventoryStorage.class));
+        verify(db, never()).saveObjectAsync(any(InventoryStorage.class));
+    }
+
+    /**
+     * Normal (non-shutdown) saves must stay asynchronous so they do not block the main thread.
+     */
+    @Test
+    void testNormalSaveStaysAsynchronous() throws Exception {
+        sets.setStatistics(false);
+        sets.setAdvancements(false);
+        Database<InventoryStorage> db = injectMockDatabase();
+
+        try (MockedStatic<Bukkit> mockedBukkit = mockStatic(Bukkit.class, Mockito.RETURNS_MOCKS)) {
+            s.storeAndSave(player, world, false);
+        }
+
+        verify(db).saveObjectAsync(any(InventoryStorage.class));
+        verify(db, never()).saveObject(any(InventoryStorage.class));
+    }
+
 }