feat: announce v1 homes waiting to be imported

.changeset/clever-tigers-wander.md

@@ -0,0 +1,5 @@
+---
+bump: patch
+---
+
+If Set Homes v1 homes are sitting in plugins/SetHomes/homes.yml and you have not imported them yet, the console now says so at startup, naming the file, how many homes are waiting and the command to run. Anyone with sh2.import-homes gets the same reminder in chat when they join. Both stop on their own as soon as any home exists here, so an upgraded server can no longer look empty without explanation.

README.md

@@ -233,6 +233,8 @@ Your players keep their homes. The old plugin does not even need to be running,
 2. Happy with the numbers? Run it again with `confirm` on the end.
 3. Move the old jar out of `plugins/`. Keep it somewhere safe rather than deleting it, so you can go back if you want to.
 
+**You will not silently end up with an empty homes list.** Once the old jar is out and Set Homes starts, if `plugins/SetHomes/homes.yml` still holds homes and none have been imported here yet, the console says so at startup, naming the file, how many are waiting and the command to run. Anyone holding `sh2.import-homes` gets the same reminder in chat when they join, because plenty of admins never read the console. Both stop for good the moment any home exists here, so there is nothing to switch off afterwards. To reword the chat line, set `v1ImportPending` in `config.yml`.
+
 Existing homes are never overwritten, so re-running the import is always safe. Homes in worlds that no longer exist are skipped with a warning naming the world.
 
 Set Homes v1 told `base` and `Base` apart, while home names here ignore case. A player holding both keeps both: the second one is imported under the next free name, so `Base` arrives as `Base2`, and the report and the server log name it. No home is dropped for a name clash.

src/main/java/com/samleighton/sethomestwo/SetHomesTwo.java

@@ -13,6 +13,7 @@
 import com.samleighton.sethomestwo.events.RightClickHomeItem;
 import com.samleighton.sethomestwo.gui.GuiSession;
 import com.samleighton.sethomestwo.gui.HomesGui;
+import com.samleighton.sethomestwo.importers.PendingV1Import;
 import com.samleighton.sethomestwo.models.TeleportAttempt;
 import com.samleighton.sethomestwo.updates.GitHubReleaseSource;
 import com.samleighton.sethomestwo.updates.UpdateChecker;
@@ -117,6 +118,28 @@ public void onEnable() {
         } else {
             Bukkit.getLogger().severe("Could not create database connection!");
         }
+
+        // Last, because it asks the database whether anything has been imported.
+        announcePendingV1Import();
+    }
+
+    /**
+     * Says so when v1 homes are sitting there unimported. Silent once any home
+     * exists here, so it needs no marker file.
+     */
+    private void announcePendingV1Import() {
+        int waiting = PendingV1Import.waitingToBeImported();
+        if (waiting == 0) return;
+
+        Logger log = Bukkit.getLogger();
+        log.warning("============================================================");
+        log.warning("Set Homes found " + waiting + " home(s) in " + PendingV1Import.SOURCE_PATH);
+        log.warning("and none of its own, so your players cannot see theirs yet.");
+        log.warning("");
+        log.warning("Run /import-homes sethomes for a preview that changes");
+        log.warning("nothing, then /import-homes sethomes confirm to bring");
+        log.warning("them across.");
+        log.warning("============================================================");
     }
 
     /**

src/main/java/com/samleighton/sethomestwo/enums/UserInfo.java

@@ -11,7 +11,8 @@ public enum UserInfo {
     MOVE_HOME_USAGE("Usage: /%s <name>"),
     GO_PLAYER_HOME_USAGE("Usage: /%s <player> <home>"),
     DELETE_PLAYER_HOME_USAGE("Usage: /%s <player> <home>"),
-    MOVE_PLAYER_HOME_USAGE("Usage: /%s <player> <home>");
+    MOVE_PLAYER_HOME_USAGE("Usage: /%s <player> <home>"),
+    V1_IMPORT_PENDING("Set Homes v1 has %s home(s) waiting to be imported. Run /import-homes sethomes for a preview, then add confirm to bring them across.");
 
     private final String value;
 

src/main/java/com/samleighton/sethomestwo/events/PlayerJoin.java

@@ -2,8 +2,12 @@
 
 import com.samleighton.sethomestwo.SetHomesTwo;
 import com.samleighton.sethomestwo.dao.HomesDao;
+import com.samleighton.sethomestwo.enums.UserInfo;
 import com.samleighton.sethomestwo.gui.GuiSession;
 import com.samleighton.sethomestwo.gui.HomesGui;
+import com.samleighton.sethomestwo.importers.PendingV1Import;
+import com.samleighton.sethomestwo.utils.ChatUtils;
+import com.samleighton.sethomestwo.utils.ConfigUtil;
 import com.samleighton.sethomestwo.updates.UpdateChecker;
 import org.bukkit.entity.Player;
 import org.bukkit.event.EventHandler;
@@ -28,5 +32,20 @@ public void onPlayerJoin(PlayerJoinEvent event){
         new HomesDao().refreshPlayerName(player.getUniqueId(), player.getName());
 
         updateChecker.notifyIfUpdateAvailable(player);
+        notifyPendingV1Import(player);
+    }
+
+    /**
+     * Tells an admin that v1 homes are waiting. The permission is checked first
+     * so an ordinary join never queries the database or reads v1's file.
+     */
+    private void notifyPendingV1Import(Player player) {
+        if (!player.hasPermission("sh2.import-homes")) return;
+
+        int waiting = PendingV1Import.waitingToBeImported();
+        if (waiting == 0) return;
+
+        ChatUtils.sendInfo(player, String.format(ConfigUtil.getConfig().getString(
+                "v1ImportPending", UserInfo.V1_IMPORT_PENDING.getValue()), waiting));
     }
 }

src/main/java/com/samleighton/sethomestwo/importers/PendingV1Import.java

@@ -0,0 +1,54 @@
+package com.samleighton.sethomestwo.importers;
+
+import com.samleighton.sethomestwo.SetHomesTwo;
+import com.samleighton.sethomestwo.dao.HomesDao;
+import org.bukkit.configuration.ConfigurationSection;
+import org.bukkit.configuration.file.YamlConfiguration;
+
+import java.io.File;
+
+/**
+ * Whether a Set Homes v1 homes.yml is sitting there waiting to be imported.
+ * Reads the same file and layout as {@link SetHomesV1Importer}.
+ */
+public final class PendingV1Import {
+
+    public static final String SOURCE_PATH = "plugins/SetHomes/homes.yml";
+
+    private PendingV1Import() {
+    }
+
+    /**
+     * Homes waiting in v1's file while our own database is still empty, or 0
+     * when there is nothing to announce. The database is checked first, so a
+     * server that has already imported never touches the disk.
+     *
+     * @return the number of homes waiting, 0 if none or if we already hold homes
+     */
+    public static int waitingToBeImported() {
+        if (new HomesDao().countAll() > 0) return 0;
+
+        File homesFile = new File(SetHomesTwo.instance().getDataFolder().getParentFile(), "SetHomes/homes.yml");
+        if (!homesFile.exists()) return 0;
+
+        return countHomes(YamlConfiguration.loadConfiguration(homesFile));
+    }
+
+    private static int countHomes(YamlConfiguration source) {
+        int total = 0;
+
+        ConfigurationSection allNamed = source.getConfigurationSection("allNamedHomes");
+        if (allNamed != null) {
+            for (String uuid : allNamed.getKeys(false)) {
+                ConfigurationSection playerSection = allNamed.getConfigurationSection(uuid);
+                if (playerSection != null) total += playerSection.getKeys(false).size();
+            }
+        }
+
+        // One unnamed home per player, which the importer brings across as "default".
+        ConfigurationSection unknown = source.getConfigurationSection("unknownHomes");
+        if (unknown != null) total += unknown.getKeys(false).size();
+
+        return total;
+    }
+}

src/main/resources/default-config.yml

@@ -73,6 +73,12 @@ homeItemLore: "Right click this item to open your home's list."
 playerHomeDeleted: "%s's home '%s' has been deleted."
 playerHomeMoved: "%s's home '%s' has been moved to your location."
 
+# Shown on join to anyone holding sh2.import-homes while Set Homes v1 homes are
+# sitting in plugins/SetHomes/homes.yml and none have been imported here yet.
+# The single %s is how many are waiting. It stops on its own once any home
+# exists here, so there is nothing to switch off after the import.
+v1ImportPending: "Set Homes v1 has %s home(s) waiting to be imported. Run /import-homes sethomes for a preview, then add confirm to bring them across."
+
 # -- ERROR MESSAGES --
 
 # Vestigial. No code path reaches this message any more: create-home treats a

src/test/java/com/samleighton/sethomestwo/PendingV1ImportNoticeTest.java

@@ -0,0 +1,228 @@
+package com.samleighton.sethomestwo;
+
+import com.samleighton.sethomestwo.support.FailOnUnimplemented;
+import com.samleighton.sethomestwo.support.HomeFixtures;
+import com.samleighton.sethomestwo.support.PluginBoot;
+import com.samleighton.sethomestwo.support.TestPlayer;
+import org.bukkit.Bukkit;
+import org.bukkit.World;
+import org.bukkit.configuration.file.YamlConfiguration;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockbukkit.mockbukkit.MockBukkit;
+import org.mockbukkit.mockbukkit.ServerMock;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.UUID;
+import java.util.logging.Handler;
+import java.util.logging.Level;
+import java.util.logging.LogRecord;
+import java.util.logging.Logger;
+import java.util.stream.Collectors;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * Boots the plugin by hand rather than through ServerTestBase, because v1's
+ * homes.yml has to be on disk before our onEnable looks for it.
+ */
+@ExtendWith(FailOnUnimplemented.class)
+class PendingV1ImportNoticeTest {
+
+    private ServerMock server;
+
+    @BeforeEach
+    void startServer() {
+        server = MockBukkit.mock();
+
+        // Same overworld, nether, end order ServerTestBase uses: ServerUtil maps
+        // environments onto worlds by list position.
+        server.addSimpleWorld("world").setEnvironment(World.Environment.NORMAL);
+        server.addSimpleWorld("world_nether").setEnvironment(World.Environment.NETHER);
+        server.addSimpleWorld("world_the_end").setEnvironment(World.Environment.THE_END);
+    }
+
+    @AfterEach
+    void stopServer() {
+        MockBukkit.unmock();
+    }
+
+    @Test
+    void saysSoAtStartupWhenV1HomesAreWaitingAndNothingIsImported() {
+        writeV1Homes(2, 0);
+
+        String block = warningText(captureLog(PluginBoot::load));
+
+        assertTrue(block.contains("plugins/SetHomes/homes.yml"), "should name the file it found");
+        assertTrue(block.contains("2"), "should say how many homes are waiting");
+        assertTrue(block.contains("/import-homes sethomes"), "should name the command to run");
+    }
+
+    @Test
+    void v1UnnamedHomesAreCountedToo() {
+        writeV1Homes(1, 1);
+
+        String block = warningText(captureLog(PluginBoot::load));
+
+        assertTrue(block.contains("2 home"),
+                "v1 keeps a player's unnamed home under unknownHomes, and it imports like any other");
+    }
+
+    @Test
+    void aJoiningAdminIsToldTheImportIsWaiting() {
+        writeV1Homes(2, 0);
+        SetHomesTwo plugin = PluginBoot.load();
+
+        TestPlayer admin = join(plugin, "Admin", true);
+
+        assertTrue(messagesTo(admin).contains("/import-homes sethomes"),
+                "plenty of admins never read the console, so the notice has to reach them in chat");
+    }
+
+    @Test
+    void aJoiningPlayerWithoutTheImportPermissionIsNotTold() {
+        writeV1Homes(2, 0);
+        SetHomesTwo plugin = PluginBoot.load();
+
+        TestPlayer player = join(plugin, "Regular", false);
+
+        assertFalse(messagesTo(player).contains("import-homes"),
+                "only someone who can run the import has any use for the notice");
+    }
+
+    @Test
+    void theNoticeRepeatsOnEveryJoin() {
+        writeV1Homes(2, 0);
+        SetHomesTwo plugin = PluginBoot.load();
+        TestPlayer admin = join(plugin, "Admin", true);
+        messagesTo(admin);
+        admin.disconnect();
+
+        // MockBukkit drops the attachment on disconnect; a real op or permissions
+        // group survives a reconnect, so grant it again before the second join.
+        admin.addAttachment(plugin, "sh2.import-homes", true);
+        server.addPlayer(admin);
+
+        assertTrue(messagesTo(admin).contains("/import-homes sethomes"),
+                "nothing is remembered per player, so an admin who missed it sees it next time");
+    }
+
+    @Test
+    void theNoticeStopsOnceAHomeExistsHere() {
+        writeV1Homes(2, 0);
+        SetHomesTwo plugin = PluginBoot.load();
+        TestPlayer first = join(plugin, "First", true);
+        HomeFixtures.persist(first, "base");
+
+        TestPlayer second = join(plugin, "Second", true);
+
+        assertFalse(messagesTo(second).contains("import-homes"),
+                "an empty database is the whole condition, so one home ends the notice");
+    }
+
+    @Test
+    void nothingIsSaidWhenThereIsNoV1File() {
+        String block = warningText(captureLog(PluginBoot::load));
+
+        assertFalse(block.contains("import-homes"), "most servers have never had v1 installed");
+    }
+
+    @Test
+    void nothingIsSaidWhenTheV1FileHoldsNoHomes() {
+        writeV1Homes(0, 0);
+
+        String block = warningText(captureLog(PluginBoot::load));
+
+        assertFalse(block.contains("import-homes"), "an empty homes.yml has nothing to offer");
+    }
+
+    /**
+     * Joins a player, granting sh2.import-homes outright rather than opping, so
+     * the tests pin the permission the notice is actually gated on.
+     */
+    private TestPlayer join(SetHomesTwo plugin, String name, boolean mayImport) {
+        TestPlayer player = new TestPlayer(server, name);
+        if (mayImport) player.addAttachment(plugin, "sh2.import-homes", true);
+        server.addPlayer(player);
+        return player;
+    }
+
+    private String messagesTo(TestPlayer player) {
+        StringBuilder all = new StringBuilder();
+        String message;
+        while ((message = player.nextMessage()) != null) all.append(message).append("\n");
+        return all.toString();
+    }
+
+    /**
+     * Writes a v1 homes.yml in the layout SetHomesV1Importer reads: named homes
+     * under allNamedHomes.uuid.name, unnamed ones under unknownHomes.uuid.
+     */
+    private void writeV1Homes(int named, int unnamed) {
+        YamlConfiguration source = new YamlConfiguration();
+        for (int i = 0; i < named; i++) {
+            writeOne(source, "allNamedHomes." + UUID.randomUUID() + ".home" + i);
+        }
+        for (int i = 0; i < unnamed; i++) {
+            writeOne(source, "unknownHomes." + UUID.randomUUID());
+        }
+
+        File v1Dir = new File(server.getPluginsFolder(), "SetHomes");
+        if (!v1Dir.isDirectory() && !v1Dir.mkdirs())
+            throw new IllegalStateException("could not create " + v1Dir);
+        try {
+            source.save(new File(v1Dir, "homes.yml"));
+        } catch (IOException e) {
+            throw new IllegalStateException("could not write the v1 fixture", e);
+        }
+    }
+
+    private void writeOne(YamlConfiguration source, String path) {
+        source.set(path + ".world", "world");
+        source.set(path + ".x", 1.0);
+        source.set(path + ".y", 64.0);
+        source.set(path + ".z", 1.0);
+        source.set(path + ".pitch", 0.0);
+        source.set(path + ".yaw", 0.0);
+    }
+
+    private List<LogRecord> captureLog(Runnable action) {
+        List<LogRecord> captured = new ArrayList<>();
+        Handler handler = new Handler() {
+            @Override
+            public void publish(LogRecord record) {
+                captured.add(record);
+            }
+
+            @Override
+            public void flush() {
+            }
+
+            @Override
+            public void close() {
+            }
+        };
+
+        Logger logger = Bukkit.getLogger();
+        logger.addHandler(handler);
+        try {
+            action.run();
+        } finally {
+            logger.removeHandler(handler);
+        }
+        return captured;
+    }
+
+    private String warningText(List<LogRecord> records) {
+        return records.stream()
+                .filter(record -> record.getLevel() == Level.WARNING)
+                .map(LogRecord::getMessage)
+                .collect(Collectors.joining("\n"));
+    }
+}