diff --git a/CHANGELOG.md b/CHANGELOG.md index 51cbfd695..a5d01e027 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -126,13 +126,22 @@ config toggles. `config.yml` and `messages.yml` migrate automatically. - Placeholder substitution is literal, not regex-based. - Inventory clicks are matched against the menu inventory only; a click in the player's own inventory no longer triggers the menu button in the same slot. +- A world whose stored difficulty is unreadable no longer vanishes on load; it + falls back to Peaceful, like the other world enums. +- World settings absent from disk (block protections, and the last-edited/loaded/ + unloaded timestamps) now load with the same defaults a freshly created world + gets, instead of loading disabled / at epoch zero. +- Failed world-directory deletions are reported instead of silently leaving files + behind. ### Security - Path-traversal guards on template and world directory resolution. World names are also checked at the public `WorldService.newWorld(String)` and on deletion, so a name resolving outside the world container (e.g. `../../plugins/x`) is - rejected rather than creating or deleting a directory outside it (#481). + rejected rather than creating or deleting a directory outside it (#481). World + creation also rejects reserved/degenerate names — the `.`/`..` aliases and + Windows device names such as `CON`/`PRN`. ### Performance @@ -140,6 +149,8 @@ config toggles. `config.yml` and `messages.yml` migrate automatically. - NoClip check no longer allocates a `Location` per tick. - Reduced allocation/work in scoreboard rendering, color processing, and the template menu. +- World, folder and player file I/O share a single bounded background thread pool + instead of the unbounded common pool. ### Migration (server admins) diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/BuildSystemPlugin.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/BuildSystemPlugin.java index 3ac6eb689..e319a8af6 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/BuildSystemPlugin.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/BuildSystemPlugin.java @@ -128,6 +128,9 @@ public void onDisable() { this.configSaveTask.cancel(); } + // Shut the shared background pool down only after the final saves above have completed. + services.scheduler().shutdown(); + this.integrations.deactivate(); getServer().getServicesManager().unregister(BuildSystem.class, api); diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/Services.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/Services.java index 6d48dfd9f..61f7e2dfd 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/Services.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/Services.java @@ -51,6 +51,7 @@ public final class Services { private final BuildSystemPlugin plugin; + private final TaskScheduler taskScheduler; private @Nullable ConfigService configService; private @Nullable Messages messages; @@ -76,6 +77,15 @@ public final class Services { Services(BuildSystemPlugin plugin) { this.plugin = plugin; + this.taskScheduler = new TaskScheduler(plugin); + } + + /** + * The plugin's single {@link TaskScheduler}, owning the shared background executor. Available for the whole plugin + * lifetime; {@link TaskScheduler#shutdown() shut down} on disable. + */ + public TaskScheduler scheduler() { + return taskScheduler; } /** @@ -107,13 +117,13 @@ void initClasses() { this.customBlockManager = new CustomBlockManager(plugin, this::world); this.playerLookupService = new PlayerLookupService(plugin); - (this.playerService = new PlayerServiceImpl(plugin, config(), this::world)).init(); + (this.playerService = new PlayerServiceImpl(plugin, config(), this::world, taskScheduler)).init(); this.navigatorEditorService = new NavigatorEditorService(); this.noClipService = new NoClipService(plugin); this.worldService = new WorldServiceImpl(plugin, this); this.backupService = new BackupServiceImpl(plugin, config(), messages(), world(), this::spawn); this.settingsService = new SettingsService(plugin, config(), messages(), player(), world()); - this.spawnService = new SpawnService(plugin, world()); + this.spawnService = new SpawnService(plugin, world(), taskScheduler); this.menuItems = new MenuItems(plugin, messages(), settings()); this.navigatorItems = new NavigatorItems(plugin, config(), messages()); this.navigatorService = new NavigatorService( @@ -122,11 +132,11 @@ void initClasses() { navigatorItems(), player(), messages(), - new TaskScheduler(plugin), + taskScheduler, new NamespacedKey(plugin, "owner"), new NamespacedKey(plugin, "category")); this.menus = new Menus(plugin, this); - this.prompts = new Prompts(messages(), config(), new TaskScheduler(plugin)); + this.prompts = new Prompts(messages(), config(), taskScheduler); // Load persisted worlds/folders last: world entities pull collaborators from a WorldContext that bundles // services created above (e.g. MenuItems, SpawnService), so the whole service graph must exist before loading. @@ -231,7 +241,7 @@ public WorldContext worldContext() { spawn(), worldStatusRegistry(), customizableIcons(), - new TaskScheduler(plugin), + taskScheduler, plugin.getLogger()); } return worldContext; diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/command/WorldsCommand.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/command/WorldsCommand.java index 25435b2e6..49d088358 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/command/WorldsCommand.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/command/WorldsCommand.java @@ -62,7 +62,7 @@ public WorldsCommand(BuildSystemPlugin plugin, Services services) { BackupServiceImpl backupService = services.backup(); Logger logger = plugin.getLogger(); File dataFolder = plugin.getDataFolder(); - TaskScheduler scheduler = new TaskScheduler(plugin); + TaskScheduler scheduler = services.scheduler(); this.dispatcher = new SubCommandDispatcher( messages, diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/command/subcommand/worlds/SaveTemplateSubCommand.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/command/subcommand/worlds/SaveTemplateSubCommand.java index 8b9803900..59a15ab96 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/command/subcommand/worlds/SaveTemplateSubCommand.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/command/subcommand/worlds/SaveTemplateSubCommand.java @@ -100,7 +100,7 @@ public void execute(Player player, String worldName, String[] args) { "worlds_savetemplate_started", Map.entry("%world%", buildWorld.getName()), Map.entry("%template%", templateName)); - CompletableFuture.runAsync(() -> FileUtils.copy(worldDir, templateDir)) + CompletableFuture.runAsync(() -> FileUtils.copy(worldDir, templateDir), scheduler.background()) .whenComplete((ignored, throwable) -> scheduler.run(() -> { if (throwable != null) { logger.log( diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/config/ConfigService.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/config/ConfigService.java index 789400edc..8d401f387 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/config/ConfigService.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/config/ConfigService.java @@ -21,14 +21,12 @@ import com.cryptomorin.xseries.XMaterial; import de.eintosti.buildsystem.BuildSystemPlugin; import de.eintosti.buildsystem.world.menu.GameRuleEntry; -import java.io.File; import java.util.*; import java.util.logging.Logger; import java.util.stream.Collectors; import org.bukkit.Difficulty; import org.bukkit.GameMode; import org.bukkit.configuration.file.FileConfiguration; -import org.bukkit.configuration.file.YamlConfiguration; import org.jspecify.annotations.NullMarked; import org.jspecify.annotations.Nullable; @@ -36,12 +34,14 @@ public class ConfigService { private final BuildSystemPlugin plugin; + private final WorldEditWandDetector wandDetector; private volatile PluginConfig current; public ConfigService(BuildSystemPlugin plugin) { this.plugin = plugin; + this.wandDetector = new WorldEditWandDetector(plugin.getDataFolder().getParentFile()); // Initialize with a placeholder; load() must be called before current() is used. - this.current = parse(plugin.getConfig(), plugin.getLogger()); + this.current = parse(plugin.getConfig(), plugin.getLogger(), wandDetector.detect()); } /** @@ -57,7 +57,7 @@ public PluginConfig current() { * Reloads the configuration from disk and re-parses it into a new {@link PluginConfig}. */ public void load() { - this.current = parse(plugin.getConfig(), plugin.getLogger()); + this.current = parse(plugin.getConfig(), plugin.getLogger(), wandDetector.detect()); } /** @@ -97,34 +97,20 @@ public void saveConfig() { } /** - * Parses the given {@link FileConfiguration} into a {@link PluginConfig} record tree. + * Parses the given {@link FileConfiguration} into a {@link PluginConfig} record tree. Pure: it reads only from + * {@code config}. The WorldEdit wand is resolved separately (see {@link WorldEditWandDetector}) and passed in, so + * parsing never touches the filesystem. * * @param config The file configuration to parse * @param logger The logger to use for warnings + * @param worldEditWand The resolved WorldEdit wand material * @return The parsed {@link PluginConfig} */ - PluginConfig parse(FileConfiguration config, Logger logger) { - return parse(config, logger, plugin.getDataFolder().getParentFile()); + static PluginConfig parse(FileConfiguration config, Logger logger, XMaterial worldEditWand) { + return new PluginConfig(parseSettings(config, worldEditWand), parseWorld(config, logger), parseFolder(config)); } - /** - * Parses a config for testing purposes (WorldEdit wand detection skipped — returns default - * {@link XMaterial#WOODEN_AXE}). - * - * @param config The file configuration to parse - * @param logger The logger to use for warnings - * @return The parsed {@link PluginConfig} - */ - static PluginConfig parseForTest(FileConfiguration config, Logger logger) { - return parse(config, logger, null); - } - - private static PluginConfig parse(FileConfiguration config, Logger logger, @Nullable File pluginParentDir) { - return new PluginConfig( - parseSettings(config, pluginParentDir), parseWorld(config, logger), parseFolder(config)); - } - - private static PluginConfig.Settings parseSettings(FileConfiguration config, @Nullable File pluginParentDir) { + private static PluginConfig.Settings parseSettings(FileConfiguration config, XMaterial worldEditWand) { PluginConfig.Settings.Archive archive = new PluginConfig.Settings.Archive( config.getBoolean("settings.archive.vanish", true), config.getBoolean("settings.archive.change-gamemode", true), @@ -139,8 +125,7 @@ private static PluginConfig.Settings parseSettings(FileConfiguration config, @Nu config.getBoolean("settings.build-mode.move-items", true)); PluginConfig.Settings.Builder builder = new PluginConfig.Settings.Builder( - config.getBoolean("settings.builder.block-worldedit-non-builder", true), - parseWorldEditWand(pluginParentDir)); + config.getBoolean("settings.builder.block-worldedit-non-builder", true), worldEditWand); PluginConfig.Settings.Navigator navigator = new PluginConfig.Settings.Navigator( XMaterial.valueOf(Objects.requireNonNullElse(config.getString("settings.navigator.item"), "CLOCK")), @@ -211,7 +196,7 @@ private static PluginConfig.World parseWorld(FileConfiguration config, Logger lo PluginConfig.World.Backup backup = new PluginConfig.World.Backup( Math.min(config.getInt("world.backup.max-backups-per-world", 5), 18), - parseStorageSettings(config, logger), + StorageSettingsFactory.fromConfig(config, logger), autoBackup); Set deletionBlacklist = config.getStringList("world.deletion-blacklist").stream() @@ -280,76 +265,6 @@ private static PluginConfig.Folder parseFolder(FileConfiguration config) { config.getBoolean("folder.override-projects", false)); } - private static PluginConfig.World.Backup.StorageSettings parseStorageSettings( - FileConfiguration config, Logger logger) { - String type = Objects.requireNonNullElse(config.getString("world.backup.storage.type"), "local") - .toLowerCase(); - - return switch (type) { - case "s3" -> - new PluginConfig.World.Backup.S3( - config.getString("world.backup.storage.s3.url"), - config.getString("world.backup.storage.s3.access-key"), - config.getString("world.backup.storage.s3.secret-key"), - config.getString("world.backup.storage.s3.region"), - config.getString("world.backup.storage.s3.bucket"), - config.getString("world.backup.storage.s3.path")); - case "sftp" -> - new PluginConfig.World.Backup.Sftp( - config.getString("world.backup.storage.sftp.host"), - config.getInt("world.backup.storage.sftp.port", 22), - config.getString("world.backup.storage.sftp.username"), - config.getString("world.backup.storage.sftp.password"), - config.getString("world.backup.storage.sftp.path")); - default -> { - if (!type.equals("local")) { - logger.warning("Unknown backup storage type '" + type + "', defaulting to local storage."); - } - yield new PluginConfig.World.Backup.Local(); - } - }; - } - - /** - * Parses the WorldEdit wand material from the WorldEdit or FastAsyncWorldEdit config file. - * - * @param pluginDir The parent directory of all plugins, or null to skip WorldEdit detection - * @return The parsed {@link XMaterial} for the WorldEdit wand - */ - static XMaterial parseWorldEditWand(@Nullable File pluginDir) { - File configFile = null; - - if (pluginDir != null) { - File weConfig = new File(pluginDir + File.separator + "WorldEdit", "config.yml"); - if (weConfig.exists()) { - configFile = weConfig; - } - - File faweConfig = new File(pluginDir + File.separator + "FastAsyncWorldEdit", "worldedit-config.yml"); - if (faweConfig.exists()) { - configFile = faweConfig; - } - } - - XMaterial defaultWand = XMaterial.WOODEN_AXE; - if (configFile == null) { - return defaultWand; - } - - YamlConfiguration weYaml = YamlConfiguration.loadConfiguration(configFile); - String wand = weYaml.getString("wand-item"); - if (wand == null) { - return defaultWand; - } - - String namespace = "minecraft:"; - if (wand.toLowerCase(Locale.ROOT).startsWith(namespace)) { - wand = wand.substring(namespace.length()); - } - - return XMaterial.matchXMaterial(wand).orElse(defaultWand); - } - /** * Parses the {@link GameMode} from a string, defaulting to {@link GameMode#ADVENTURE} if the string is not valid. * diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/config/StorageSettingsFactory.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/config/StorageSettingsFactory.java new file mode 100644 index 000000000..5a85acd1a --- /dev/null +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/config/StorageSettingsFactory.java @@ -0,0 +1,115 @@ +/* + * Copyright (c) 2018-2026, Thomas Meaney + * Copyright (c) contributors + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package de.eintosti.buildsystem.config; + +import de.eintosti.buildsystem.config.PluginConfig.World.Backup.Local; +import de.eintosti.buildsystem.config.PluginConfig.World.Backup.S3; +import de.eintosti.buildsystem.config.PluginConfig.World.Backup.Sftp; +import de.eintosti.buildsystem.config.PluginConfig.World.Backup.StorageSettings; +import java.util.LinkedHashMap; +import java.util.Locale; +import java.util.Map; +import java.util.logging.Logger; +import org.bukkit.configuration.file.FileConfiguration; +import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; + +/** + * Builds the backup {@link StorageSettings} from config, validating that a chosen remote backend actually has its + * required credentials before selecting it. A misconfigured remote logs exactly which key is missing and falls back to + * {@link Local local} storage, rather than constructing a remote with {@code null} fields that fails only later at + * connection time. + */ +@NullMarked +final class StorageSettingsFactory { + + private static final String BASE = "world.backup.storage."; + + private StorageSettingsFactory() {} + + static StorageSettings fromConfig(FileConfiguration config, Logger logger) { + String type = config.getString(BASE + "type", "local").toLowerCase(Locale.ROOT); + return switch (type) { + case "s3" -> s3(config, logger); + case "sftp" -> sftp(config, logger); + case "local" -> new Local(); + default -> { + logger.warning("Unknown backup storage type '" + type + "', defaulting to local storage."); + yield new Local(); + } + }; + } + + private static StorageSettings s3(FileConfiguration config, Logger logger) { + String prefix = BASE + "s3."; + Map required = new LinkedHashMap<>(); + required.put(prefix + "url", config.getString(prefix + "url")); + required.put(prefix + "access-key", config.getString(prefix + "access-key")); + required.put(prefix + "secret-key", config.getString(prefix + "secret-key")); + required.put(prefix + "bucket", config.getString(prefix + "bucket")); + + if (fallbackOnMissing(required, "s3", logger)) { + return new Local(); + } + return new S3( + config.getString(prefix + "url"), + config.getString(prefix + "access-key"), + config.getString(prefix + "secret-key"), + config.getString(prefix + "region"), + config.getString(prefix + "bucket"), + config.getString(prefix + "path")); + } + + private static StorageSettings sftp(FileConfiguration config, Logger logger) { + String prefix = BASE + "sftp."; + Map required = new LinkedHashMap<>(); + required.put(prefix + "host", config.getString(prefix + "host")); + required.put(prefix + "username", config.getString(prefix + "username")); + required.put(prefix + "password", config.getString(prefix + "password")); + + if (fallbackOnMissing(required, "sftp", logger)) { + return new Local(); + } + + return new Sftp( + config.getString(prefix + "host"), + config.getInt(prefix + "port", 22), + config.getString(prefix + "username"), + config.getString(prefix + "password"), + config.getString(prefix + "path")); + } + + /** + * {@return whether a required value is missing} Logs the first blank/absent key together with the backend name when + * so, signalling the caller to fall back to local storage. + */ + private static boolean fallbackOnMissing(Map required, String backend, Logger logger) { + for (Map.Entry entry : required.entrySet()) { + if (isBlank(entry.getValue())) { + logger.warning("Backup storage '" + backend + "' is missing required setting '" + entry.getKey() + + "'; falling back to local storage."); + return true; + } + } + return false; + } + + private static boolean isBlank(@Nullable String value) { + return value == null || value.isBlank(); + } +} diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/config/WorldEditWandDetector.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/config/WorldEditWandDetector.java new file mode 100644 index 000000000..aaa0ed20b --- /dev/null +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/config/WorldEditWandDetector.java @@ -0,0 +1,92 @@ +/* + * Copyright (c) 2018-2026, Thomas Meaney + * Copyright (c) contributors + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package de.eintosti.buildsystem.config; + +import com.cryptomorin.xseries.XMaterial; +import java.io.File; +import java.util.Locale; +import org.bukkit.configuration.file.YamlConfiguration; +import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; + +/** + * Resolves the WorldEdit wand material by reading WorldEdit's (or FastAsyncWorldEdit's) own config file, so the plugin + * can recognise that item inside protected worlds. Kept out of {@link ConfigService}'s parsing so config parsing stays + * a pure, side-effect-free transform of the {@code FileConfiguration} — the filesystem lookup lives here and is easy to + * stub in tests by constructing the detector with a controlled directory (or {@code null} to skip detection). + */ +@NullMarked +public final class WorldEditWandDetector { + + /** Used when no WorldEdit install is found or its wand cannot be resolved. */ + public static final XMaterial DEFAULT_WAND = XMaterial.WOODEN_AXE; + + private final @Nullable File pluginsDir; + + /** + * @param pluginsDir The server's plugins directory (the parent of this plugin's data folder), or {@code null} to + * skip detection and always return {@link #DEFAULT_WAND} + */ + public WorldEditWandDetector(@Nullable File pluginsDir) { + this.pluginsDir = pluginsDir; + } + + /** + * {@return the configured WorldEdit wand, or {@link #DEFAULT_WAND} when WorldEdit is absent or its wand cannot be + * resolved} + */ + public XMaterial detect() { + File configFile = locateConfig(); + if (configFile == null) { + return DEFAULT_WAND; + } + + YamlConfiguration weConfig = YamlConfiguration.loadConfiguration(configFile); + String wand = weConfig.getString("wand-item"); + if (wand == null) { + return DEFAULT_WAND; + } + + String namespace = "minecraft:"; + if (wand.toLowerCase(Locale.ROOT).startsWith(namespace)) { + wand = wand.substring(namespace.length()); + } + return XMaterial.matchXMaterial(wand).orElse(DEFAULT_WAND); + } + + /** + * FastAsyncWorldEdit takes precedence over WorldEdit when both are installed, matching the prior behaviour. + */ + private @Nullable File locateConfig() { + if (pluginsDir == null) { + return null; + } + + File faweConfig = new File(pluginsDir + File.separator + "FastAsyncWorldEdit", "worldedit-config.yml"); + if (faweConfig.exists()) { + return faweConfig; + } + + File weConfig = new File(pluginsDir + File.separator + "WorldEdit", "config.yml"); + if (weConfig.exists()) { + return weConfig; + } + + return null; + } +} diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/listener/ListenerRegistrar.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/listener/ListenerRegistrar.java index 57148de03..ad5c055da 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/listener/ListenerRegistrar.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/listener/ListenerRegistrar.java @@ -76,7 +76,7 @@ public void registerAll() { NavigatorEditorService navigatorEditorService = services.navigatorEditor(); NoClipService noClipService = services.noClip(); SpawnService spawnService = services.spawn(); - TaskScheduler scheduler = new TaskScheduler(plugin); + TaskScheduler scheduler = services.scheduler(); PlayerLookupService playerLookupService = services.playerLookup(); UpdateChecker updateChecker = plugin.getUpdateChecker(); diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/menu/Menus.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/menu/Menus.java index 647938814..c1dfea238 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/menu/Menus.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/menu/Menus.java @@ -82,7 +82,7 @@ public final class Menus { public Menus(BuildSystemPlugin plugin, Services services) { this.plugin = plugin; this.services = services; - this.scheduler = new TaskScheduler(plugin); + this.scheduler = services.scheduler(); this.builderNameKey = new NamespacedKey(plugin, "builder_name"); } diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/player/PlayerServiceImpl.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/player/PlayerServiceImpl.java index 1706f67ef..b90750bb3 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/player/PlayerServiceImpl.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/player/PlayerServiceImpl.java @@ -25,6 +25,7 @@ import de.eintosti.buildsystem.storage.PlayerStorageImpl; import de.eintosti.buildsystem.storage.WorldStorageImpl; import de.eintosti.buildsystem.storage.yaml.YamlPlayerStorage; +import de.eintosti.buildsystem.util.TaskScheduler; import de.eintosti.buildsystem.world.WorldServiceImpl; import java.util.Collections; import java.util.HashSet; @@ -48,11 +49,14 @@ public class PlayerServiceImpl implements PlayerService { private final Set buildModePlayers; public PlayerServiceImpl( - BuildSystemPlugin plugin, ConfigService configService, Supplier worldService) { + BuildSystemPlugin plugin, + ConfigService configService, + Supplier worldService, + TaskScheduler scheduler) { this.plugin = plugin; this.configService = configService; this.worldService = worldService; - this.playerStorage = new YamlPlayerStorage(plugin); + this.playerStorage = new YamlPlayerStorage(plugin, scheduler); this.maxWorldsResolver = new MaxWorldsResolver(plugin.getLogger()); this.buildModePlayers = new HashSet<>(); } diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/storage/PlayerStorageImpl.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/storage/PlayerStorageImpl.java index 42a902a46..726a11936 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/storage/PlayerStorageImpl.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/storage/PlayerStorageImpl.java @@ -34,6 +34,12 @@ import org.jspecify.annotations.NullMarked; import org.jspecify.annotations.Nullable; +/** + * In-memory cache of {@link BuildPlayer}s keyed by UUID. Like the world storage, lookups may run off the main thread — + * {@code AsyncPlayerPreLoginEvent} reads a connecting player's settings on Bukkit's async login thread — so the cache is + * a {@link ConcurrentHashMap}. The startup {@link #loadPlayers() load} publishes its entries with {@code putAll}: saved + * data wins over any blank entry a concurrent lookup may have created in the brief load window. + */ @NullMarked public abstract class PlayerStorageImpl implements PlayerStorage { diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/storage/WorldStorageImpl.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/storage/WorldStorageImpl.java index 2276c0b82..881772167 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/storage/WorldStorageImpl.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/storage/WorldStorageImpl.java @@ -37,6 +37,13 @@ import org.jspecify.annotations.NullMarked; import org.jspecify.annotations.Nullable; +/** + * In-memory index of the server's {@link BuildWorld}s, keyed both by UUID and by lower-cased name. Mutations + * ({@link #addBuildWorld}, {@link #removeBuildWorld}, {@link #rename}) run on the main thread, but lookups may be called + * off it: {@code AsyncPlayerPreLoginEvent} resolves a returning player's last world on Bukkit's async login thread. The + * indexes are therefore {@link ConcurrentHashMap}s so concurrent reads stay safe and consistently published, and the + * compound mutations are ordered so a concurrent reader never observes a world's two index entries out of step. + */ @NullMarked public abstract class WorldStorageImpl implements WorldStorage { @@ -99,8 +106,14 @@ public synchronized void removeBuildWorld(BuildWorld buildWorld) { } public synchronized void rename(BuildWorld buildWorld, String oldName, String newName) { - this.uuidByName.remove(oldName.toLowerCase()); - this.uuidByName.put(newName.toLowerCase(), buildWorld.getUniqueId()); + String oldKey = oldName.toLowerCase(); + String newKey = newName.toLowerCase(); + // Publish the new name before dropping the old one so a concurrent (async) reader never sees the world vanish. + // Guard the removal: a case-only rename maps both names to the same key, which must stay resolvable. + this.uuidByName.put(newKey, buildWorld.getUniqueId()); + if (!oldKey.equals(newKey)) { + this.uuidByName.remove(oldKey); + } } @Override diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/storage/codec/WorldCodec.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/storage/codec/WorldCodec.java index 655096b55..ca13cc1ca 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/storage/codec/WorldCodec.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/storage/codec/WorldCodec.java @@ -24,6 +24,7 @@ import de.eintosti.buildsystem.api.world.data.BuildWorldStatus; import de.eintosti.buildsystem.api.world.data.BuildWorldType; import de.eintosti.buildsystem.api.world.data.Visibility; +import de.eintosti.buildsystem.api.world.data.WorldDataKey; import de.eintosti.buildsystem.api.world.data.WorldStatusRegistry; import de.eintosti.buildsystem.player.PlayerLookupService; import de.eintosti.buildsystem.world.BuildWorldImpl; @@ -47,10 +48,10 @@ * {@link Codec} for {@link BuildWorld}s, mapping a world to and from its section. Since v4 the section is keyed by the * world's UUID and the name is carried as a {@code name} field (a rename is then a field update, not a key move). * - *

The bulk of a world's state lives under the nested {@code data} section, whose keys mirror the property keys - * registered in {@link WorldDataImpl} — the {@code DATA_*} constants here must stay in lock-step with those. Reads are - * defensive: unknown enums fall back to safe defaults and a single unparseable entry surfaces as an exception for the - * storage to skip rather than aborting the whole load. + *

The bulk of a world's state lives under the nested {@code data} section, whose keys come straight from + * {@link WorldDataKey} via {@link #dataPath} — the same catalog {@link WorldDataImpl} registers and serializes by, so + * there is no parallel key list to keep in sync. Reads are defensive: unknown enums fall back to safe defaults and a + * single unparseable entry surfaces as an exception for the storage to skip rather than aborting the whole load. */ @NullMarked public final class WorldCodec implements Codec { @@ -65,27 +66,8 @@ public final class WorldCodec implements Codec { private static final String CHUNK_GENERATOR = "chunk-generator"; private static final String DATA = "data"; - private static final String DATA_SPAWN = "spawn"; - private static final String DATA_PERMISSION = "permission"; - private static final String DATA_PROJECT = "project"; - private static final String DATA_DIFFICULTY = "difficulty"; - private static final String DATA_MATERIAL = "material"; - private static final String DATA_ICON_SKULL_TEXTURE = "icon-skull-texture"; - private static final String DATA_STATUS = "status"; - private static final String DATA_BLOCK_BREAKING = "block-breaking"; - private static final String DATA_BLOCK_INTERACTIONS = "block-interactions"; - private static final String DATA_BLOCK_PLACEMENT = "block-placement"; - private static final String DATA_BUILDERS_ENABLED = "builders-enabled"; - private static final String DATA_EXPLOSIONS = "explosions"; - private static final String DATA_MOB_AI = "mob-ai"; - private static final String DATA_PHYSICS = "physics"; - private static final String DATA_PINNED = "pinned"; - private static final String DATA_VISIBILITY = "visibility"; - private static final String DATA_PRIVATE = "private"; - private static final String DATA_TIME_SINCE_BACKUP = "time-since-backup"; - private static final String DATA_LAST_LOADED = "last-loaded"; - private static final String DATA_LAST_UNLOADED = "last-unloaded"; - private static final String DATA_LAST_EDITED = "last-edited"; + // The data keys come straight from WorldDataKey (see dataPath); only the pre-4.0 private boolean has no key. + private static final String LEGACY_PRIVATE = "private"; private final WorldContext context; private final PlayerLookupService playerLookup; @@ -158,26 +140,31 @@ public BuildWorldImpl deserialize(String key, ConfigurationSection section) { private WorldDataImpl parseWorldData(String worldName, ConfigurationSection section) { return new WorldDataBuilder(worldName) .withCustomSpawn(parseCustomSpawn(section)) - .withPermission(section.getString(DATA + "." + DATA_PERMISSION, "-")) - .withProject(section.getString(DATA + "." + DATA_PROJECT, "-")) - .withDifficulty(Difficulty.valueOf(section.getString(DATA + "." + DATA_DIFFICULTY, "PEACEFUL") - .toUpperCase(Locale.ROOT))) + .withPermission(section.getString(dataPath(WorldDataKey.PERMISSION), "-")) + .withProject(section.getString(dataPath(WorldDataKey.PROJECT), "-")) + .withDifficulty(parseDifficulty(section, worldName)) .withMaterial(parseMaterial(section, worldName)) - .withIconSkullTexture(section.getString(DATA + "." + DATA_ICON_SKULL_TEXTURE, "")) + .withIconSkullTexture(section.getString(dataPath(WorldDataKey.ICON_SKULL_TEXTURE), "")) .withStatus(parseStatus(section, worldName)) - .withBlockBreaking(section.getBoolean(DATA + "." + DATA_BLOCK_BREAKING)) - .withBlockInteractions(section.getBoolean(DATA + "." + DATA_BLOCK_INTERACTIONS)) - .withBlockPlacement(section.getBoolean(DATA + "." + DATA_BLOCK_PLACEMENT)) - .withBuildersEnabled(section.getBoolean(DATA + "." + DATA_BUILDERS_ENABLED)) - .withExplosions(section.getBoolean(DATA + "." + DATA_EXPLOSIONS)) - .withMobAi(section.getBoolean(DATA + "." + DATA_MOB_AI)) - .withPhysics(section.getBoolean(DATA + "." + DATA_PHYSICS)) - .withPinned(section.getBoolean(DATA + "." + DATA_PINNED, false)) + .withBlockBreaking( + section.getBoolean(dataPath(WorldDataKey.BLOCK_BREAKING), WorldDataImpl.DEFAULT_BLOCK_BREAKING)) + .withBlockInteractions(section.getBoolean( + dataPath(WorldDataKey.BLOCK_INTERACTIONS), WorldDataImpl.DEFAULT_BLOCK_INTERACTIONS)) + .withBlockPlacement(section.getBoolean( + dataPath(WorldDataKey.BLOCK_PLACEMENT), WorldDataImpl.DEFAULT_BLOCK_PLACEMENT)) + .withBuildersEnabled(section.getBoolean( + dataPath(WorldDataKey.BUILDERS_ENABLED), WorldDataImpl.DEFAULT_BUILDERS_ENABLED)) + .withExplosions(section.getBoolean(dataPath(WorldDataKey.EXPLOSIONS), WorldDataImpl.DEFAULT_EXPLOSIONS)) + .withMobAi(section.getBoolean(dataPath(WorldDataKey.MOB_AI), WorldDataImpl.DEFAULT_MOB_AI)) + .withPhysics(section.getBoolean(dataPath(WorldDataKey.PHYSICS), WorldDataImpl.DEFAULT_PHYSICS)) + .withPinned(section.getBoolean(dataPath(WorldDataKey.PINNED), WorldDataImpl.DEFAULT_PINNED)) .withVisibility(parseVisibility(section)) - .withTimeSinceBackup(section.getInt(DATA + "." + DATA_TIME_SINCE_BACKUP, 0)) - .withLastLoaded(section.getLong(DATA + "." + DATA_LAST_LOADED)) - .withLastUnloaded(section.getLong(DATA + "." + DATA_LAST_UNLOADED)) - .withLastEdited(section.getLong(DATA + "." + DATA_LAST_EDITED)) + .withTimeSinceBackup(section.getInt( + dataPath(WorldDataKey.TIME_SINCE_BACKUP), WorldDataImpl.DEFAULT_TIME_SINCE_BACKUP)) + .withLastLoaded(section.getLong(dataPath(WorldDataKey.LAST_LOADED), WorldDataImpl.DEFAULT_TIMESTAMP)) + .withLastUnloaded( + section.getLong(dataPath(WorldDataKey.LAST_UNLOADED), WorldDataImpl.DEFAULT_TIMESTAMP)) + .withLastEdited(section.getLong(dataPath(WorldDataKey.LAST_EDITED), WorldDataImpl.DEFAULT_TIMESTAMP)) .withPermissionOverrideEnabled( () -> context.configService().current().folder().overridePermissions()) .withProjectOverrideEnabled( @@ -185,13 +172,21 @@ private WorldDataImpl parseWorldData(String worldName, ConfigurationSection sect .build(); } + /** + * {@return the nested {@code data.} path for a key} The data keys are owned by {@link WorldDataKey}, so the + * codec never duplicates the on-disk strings. + */ + private static String dataPath(WorldDataKey key) { + return DATA + "." + key.id(); + } + /** * Reads a world's custom spawn. It is serialized under {@code data.spawn} (its property key), but pre-property-map * files stored it at the top-level {@code spawn} key, so that location is the fallback. */ private String parseCustomSpawn(ConfigurationSection section) { - String dataSpawn = section.getString(DATA + "." + DATA_SPAWN); - return dataSpawn != null ? dataSpawn : section.getString(DATA_SPAWN, ""); + String dataSpawn = section.getString(dataPath(WorldDataKey.CUSTOM_SPAWN)); + return dataSpawn != null ? dataSpawn : section.getString(WorldDataKey.CUSTOM_SPAWN.id(), ""); } private BuildWorldType parseType(String worldName, ConfigurationSection section) { @@ -208,13 +203,28 @@ private BuildWorldType parseType(String worldName, ConfigurationSection section) } } + /** + * Resolves a world's {@link Difficulty}, falling back to {@link Difficulty#PEACEFUL} when the persisted value is + * unknown. Like the other enums, an unparseable difficulty must not abort the world's load. + */ + private Difficulty parseDifficulty(ConfigurationSection section, String worldName) { + String raw = section.getString(dataPath(WorldDataKey.DIFFICULTY), Difficulty.PEACEFUL.name()); + try { + return Difficulty.valueOf(raw.toUpperCase(Locale.ROOT)); + } catch (IllegalArgumentException e) { + context.logger() + .warning("Unknown difficulty \"" + raw + "\" for \"" + worldName + "\". Defaulting to PEACEFUL."); + return Difficulty.PEACEFUL; + } + } + /** * Resolves a world's status from its persisted id, migrating pre-4.0 enum names (e.g. {@code NOT_STARTED}) to the * equivalent lower-case status id. Falls back to the registry default when the id is unknown. */ private BuildWorldStatus parseStatus(ConfigurationSection section, String worldName) { WorldStatusRegistry registry = context.statusRegistry(); - String raw = section.getString(DATA + "." + DATA_STATUS); + String raw = section.getString(dataPath(WorldDataKey.STATUS)); if (raw == null) { return registry.getDefaultStatus(); } @@ -232,7 +242,7 @@ private BuildWorldStatus parseStatus(ConfigurationSection section, String worldN * {@code private} boolean when the new key is absent. */ private Visibility parseVisibility(ConfigurationSection section) { - String raw = section.getString(DATA + "." + DATA_VISIBILITY); + String raw = section.getString(dataPath(WorldDataKey.VISIBILITY)); if (raw != null) { try { return Visibility.valueOf(raw.toUpperCase(Locale.ROOT)); @@ -240,11 +250,11 @@ private Visibility parseVisibility(ConfigurationSection section) { // Fall through to the legacy private flag. } } - return Visibility.matchVisibility(section.getBoolean(DATA + "." + DATA_PRIVATE)); + return Visibility.matchVisibility(section.getBoolean(DATA + "." + LEGACY_PRIVATE)); } private XMaterial parseMaterial(ConfigurationSection section, String worldName) { - String itemString = section.getString(DATA + "." + DATA_MATERIAL); + String itemString = section.getString(dataPath(WorldDataKey.MATERIAL)); if (itemString == null) { itemString = XMaterial.BEDROCK.name(); context.logger().warning("Could not find material for \"" + worldName + "\". Defaulting to BEDROCK."); diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/storage/yaml/YamlFolderStorage.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/storage/yaml/YamlFolderStorage.java index 9e4519875..abe377475 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/storage/yaml/YamlFolderStorage.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/storage/yaml/YamlFolderStorage.java @@ -30,9 +30,11 @@ import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.Map; import java.util.Set; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.Executor; import java.util.logging.Level; import org.bukkit.configuration.ConfigurationSection; import org.bukkit.configuration.file.FileConfiguration; @@ -49,6 +51,7 @@ public class YamlFolderStorage extends FolderStorageImpl { private final Services services; private final YamlStore store; private final FileConfiguration config; + private final Executor background; private @Nullable FolderCodec codec; public YamlFolderStorage(BuildSystemPlugin plugin, WorldStorage worldStorage, Services services) { @@ -56,6 +59,7 @@ public YamlFolderStorage(BuildSystemPlugin plugin, WorldStorage worldStorage, Se this.services = services; this.store = new YamlStore(plugin.getDataFolder(), "folders.yml", plugin.getLogger()); this.config = store.config(); + this.background = services.scheduler().background(); } /** @@ -77,57 +81,71 @@ protected Folder newFolder(String name, NavigatorCategory category, @Nullable Fo @Override public CompletableFuture save(Folder folder) { - return CompletableFuture.runAsync(() -> store.atomicSave(() -> { - config.set(StorageMigration.VERSION_KEY, StorageMigration.CURRENT_VERSION); - config.set(FOLDERS_KEY + "." + codec().key(folder), codec().serialize(folder)); - })); + // Serialize on the calling (main) thread; the async block only writes the captured map to disk. + String folderKey = codec().key(folder); + Map serialized = codec().serialize(folder); + return CompletableFuture.runAsync( + () -> store.atomicSave(() -> { + config.set(StorageMigration.VERSION_KEY, StorageMigration.CURRENT_VERSION); + config.set(FOLDERS_KEY + "." + folderKey, serialized); + }), + background); } @Override public CompletableFuture save(Collection folders) { - return CompletableFuture.runAsync(() -> store.atomicSave(() -> { - config.set(StorageMigration.VERSION_KEY, StorageMigration.CURRENT_VERSION); - folders.forEach(folder -> config.set(FOLDERS_KEY + "." + codec().key(folder), codec().serialize(folder))); - })); + Map serialized = new LinkedHashMap<>(); + for (Folder folder : folders) { + serialized.put(codec().key(folder), codec().serialize(folder)); + } + return CompletableFuture.runAsync( + () -> store.atomicSave(() -> { + config.set(StorageMigration.VERSION_KEY, StorageMigration.CURRENT_VERSION); + serialized.forEach((folderKey, value) -> config.set(FOLDERS_KEY + "." + folderKey, value)); + }), + background); } @Override @Contract("-> new") public CompletableFuture> load() { - return CompletableFuture.supplyAsync(() -> store.locked(() -> { - Set folderKeys = loadFolderKeys(); - - // First pass: load each folder by its key (UUID) without its parent reference. - Map loadedByKey = new HashMap<>(); - for (String folderKey : folderKeys) { - ConfigurationSection section = config.getConfigurationSection(FOLDERS_KEY + "." + folderKey); - if (section == null) { - continue; - } - try { - loadedByKey.put(folderKey, codec().deserialize(folderKey, section)); - } catch (Exception e) { - logger.log(Level.WARNING, "Skipping folder \"" + folderKey + "\": could not be loaded", e); - } - } - - // Second pass: link parents (referenced by UUID) now that every folder exists. - for (Map.Entry entry : loadedByKey.entrySet()) { - ConfigurationSection section = config.getConfigurationSection(FOLDERS_KEY + "." + entry.getKey()); - if (section == null) { - continue; - } - String parentKey = codec().parentReference(section); - if (parentKey != null) { - Folder parent = loadedByKey.get(parentKey); - if (parent != null) { - entry.getValue().setParent(parent); + return CompletableFuture.supplyAsync( + () -> store.locked(() -> { + Set folderKeys = loadFolderKeys(); + + // First pass: load each folder by its key (UUID) without its parent reference. + Map loadedByKey = new HashMap<>(); + for (String folderKey : folderKeys) { + ConfigurationSection section = config.getConfigurationSection(FOLDERS_KEY + "." + folderKey); + if (section == null) { + continue; + } + try { + loadedByKey.put(folderKey, codec().deserialize(folderKey, section)); + } catch (Exception e) { + logger.log(Level.WARNING, "Skipping folder \"" + folderKey + "\": could not be loaded", e); + } + } + + // Second pass: link parents (referenced by UUID) now that every folder exists. + for (Map.Entry entry : loadedByKey.entrySet()) { + ConfigurationSection section = + config.getConfigurationSection(FOLDERS_KEY + "." + entry.getKey()); + if (section == null) { + continue; + } + String parentKey = codec().parentReference(section); + if (parentKey != null) { + Folder parent = loadedByKey.get(parentKey); + if (parent != null) { + entry.getValue().setParent(parent); + } + } } - } - } - return new ArrayList<>(loadedByKey.values()); - })); + return new ArrayList<>(loadedByKey.values()); + }), + background); } private Set loadFolderKeys() { @@ -166,6 +184,6 @@ public CompletableFuture delete(Folder folder) { @Override public CompletableFuture delete(String folderKey) { return CompletableFuture.runAsync( - () -> store.atomicSave(() -> config.set(FOLDERS_KEY + "." + folderKey, null))); + () -> store.atomicSave(() -> config.set(FOLDERS_KEY + "." + folderKey, null)), background); } } diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/storage/yaml/YamlPlayerStorage.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/storage/yaml/YamlPlayerStorage.java index 5220e8b9e..bd52a8881 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/storage/yaml/YamlPlayerStorage.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/storage/yaml/YamlPlayerStorage.java @@ -21,10 +21,14 @@ import de.eintosti.buildsystem.api.player.BuildPlayer; import de.eintosti.buildsystem.storage.PlayerStorageImpl; import de.eintosti.buildsystem.storage.codec.PlayerCodec; +import de.eintosti.buildsystem.util.TaskScheduler; import java.util.ArrayList; import java.util.Collection; +import java.util.LinkedHashMap; +import java.util.Map; import java.util.Set; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.Executor; import java.util.logging.Level; import org.bukkit.configuration.ConfigurationSection; import org.bukkit.configuration.file.FileConfiguration; @@ -38,43 +42,57 @@ public class YamlPlayerStorage extends PlayerStorageImpl { private final YamlStore store; private final FileConfiguration config; private final PlayerCodec codec; + private final Executor background; - public YamlPlayerStorage(BuildSystemPlugin plugin) { + public YamlPlayerStorage(BuildSystemPlugin plugin, TaskScheduler scheduler) { super(plugin.getLogger()); this.store = new YamlStore(plugin.getDataFolder(), "players.yml", plugin.getLogger()); this.config = store.config(); this.codec = new PlayerCodec(plugin.getLogger()); + this.background = scheduler.background(); } @Override public CompletableFuture save(BuildPlayer buildPlayer) { - return CompletableFuture.runAsync(() -> store.atomicSave( - () -> config.set(PLAYERS_KEY + "." + codec.key(buildPlayer), codec.serialize(buildPlayer)))); + // Serialize on the calling (main) thread; the async block only writes the captured map to disk. + String playerKey = codec.key(buildPlayer); + Map serialized = codec.serialize(buildPlayer); + return CompletableFuture.runAsync( + () -> store.atomicSave(() -> config.set(PLAYERS_KEY + "." + playerKey, serialized)), background); } @Override public CompletableFuture save(Collection players) { - return CompletableFuture.runAsync(() -> store.atomicSave(() -> - players.forEach(player -> config.set(PLAYERS_KEY + "." + codec.key(player), codec.serialize(player))))); + Map serialized = new LinkedHashMap<>(); + for (BuildPlayer player : players) { + serialized.put(codec.key(player), codec.serialize(player)); + } + return CompletableFuture.runAsync( + () -> store.atomicSave(() -> + serialized.forEach((playerKey, value) -> config.set(PLAYERS_KEY + "." + playerKey, value))), + background); } @Override public CompletableFuture> load() { - return CompletableFuture.supplyAsync(() -> store.locked(() -> { - Collection players = new ArrayList<>(); - for (String playerUuid : loadPlayerKeys()) { - try { - ConfigurationSection section = config.getConfigurationSection(PLAYERS_KEY + "." + playerUuid); - if (section == null) { - continue; + return CompletableFuture.supplyAsync( + () -> store.locked(() -> { + Collection players = new ArrayList<>(); + for (String playerUuid : loadPlayerKeys()) { + try { + ConfigurationSection section = + config.getConfigurationSection(PLAYERS_KEY + "." + playerUuid); + if (section == null) { + continue; + } + players.add(codec.deserialize(playerUuid, section)); + } catch (Exception e) { + logger.log(Level.WARNING, "Skipping player \"" + playerUuid + "\": could not be loaded", e); + } } - players.add(codec.deserialize(playerUuid, section)); - } catch (Exception e) { - logger.log(Level.WARNING, "Skipping player \"" + playerUuid + "\": could not be loaded", e); - } - } - return players; - })); + return players; + }), + background); } private Set loadPlayerKeys() { @@ -98,6 +116,6 @@ public CompletableFuture delete(BuildPlayer buildPlayer) { @Override public CompletableFuture delete(String playerKey) { return CompletableFuture.runAsync( - () -> store.atomicSave(() -> config.set(PLAYERS_KEY + "." + playerKey, null))); + () -> store.atomicSave(() -> config.set(PLAYERS_KEY + "." + playerKey, null)), background); } } diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/storage/yaml/YamlWorldStorage.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/storage/yaml/YamlWorldStorage.java index 0c5b9158f..1a31c42f9 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/storage/yaml/YamlWorldStorage.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/storage/yaml/YamlWorldStorage.java @@ -25,8 +25,11 @@ import de.eintosti.buildsystem.storage.migration.StorageMigration; import java.util.ArrayList; import java.util.Collection; +import java.util.LinkedHashMap; +import java.util.Map; import java.util.Set; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.Executor; import java.util.logging.Level; import org.bukkit.configuration.ConfigurationSection; import org.bukkit.configuration.file.FileConfiguration; @@ -42,6 +45,7 @@ public class YamlWorldStorage extends WorldStorageImpl { private final Services services; private final YamlStore store; private final FileConfiguration config; + private final Executor background; private @Nullable WorldCodec codec; public YamlWorldStorage(BuildSystemPlugin plugin, Services services) { @@ -49,6 +53,7 @@ public YamlWorldStorage(BuildSystemPlugin plugin, Services services) { this.services = services; this.store = new YamlStore(plugin.getDataFolder(), "worlds.yml", plugin.getLogger()); this.config = store.config(); + this.background = services.scheduler().background(); } /** @@ -65,38 +70,51 @@ private WorldCodec codec() { @Override public CompletableFuture save(BuildWorld buildWorld) { - return CompletableFuture.runAsync(() -> store.atomicSave(() -> { - config.set(StorageMigration.VERSION_KEY, StorageMigration.CURRENT_VERSION); - config.set(WORLDS_KEY + "." + codec().key(buildWorld), codec().serialize(buildWorld)); - })); + // Serialize on the calling (main) thread, where the world's data is owned: the async block must only write the + // already-captured map to disk, never read live domain state off the main thread. + String worldKey = codec().key(buildWorld); + Map serialized = codec().serialize(buildWorld); + return CompletableFuture.runAsync( + () -> store.atomicSave(() -> { + config.set(StorageMigration.VERSION_KEY, StorageMigration.CURRENT_VERSION); + config.set(WORLDS_KEY + "." + worldKey, serialized); + }), + background); } @Override public CompletableFuture save(Collection buildWorlds) { - return CompletableFuture.runAsync(() -> store.atomicSave(() -> { - config.set(StorageMigration.VERSION_KEY, StorageMigration.CURRENT_VERSION); - buildWorlds.forEach(buildWorld -> - config.set(WORLDS_KEY + "." + codec().key(buildWorld), codec().serialize(buildWorld))); - })); + Map serialized = new LinkedHashMap<>(); + for (BuildWorld buildWorld : buildWorlds) { + serialized.put(codec().key(buildWorld), codec().serialize(buildWorld)); + } + return CompletableFuture.runAsync( + () -> store.atomicSave(() -> { + config.set(StorageMigration.VERSION_KEY, StorageMigration.CURRENT_VERSION); + serialized.forEach((worldKey, value) -> config.set(WORLDS_KEY + "." + worldKey, value)); + }), + background); } @Override public CompletableFuture> load() { - return CompletableFuture.supplyAsync(() -> store.locked(() -> { - Collection worlds = new ArrayList<>(); - for (String worldKey : loadWorldKeys()) { - try { - ConfigurationSection section = config.getConfigurationSection(WORLDS_KEY + "." + worldKey); - if (section == null) { - continue; + return CompletableFuture.supplyAsync( + () -> store.locked(() -> { + Collection worlds = new ArrayList<>(); + for (String worldKey : loadWorldKeys()) { + try { + ConfigurationSection section = config.getConfigurationSection(WORLDS_KEY + "." + worldKey); + if (section == null) { + continue; + } + worlds.add(codec().deserialize(worldKey, section)); + } catch (Exception e) { + logger.log(Level.WARNING, "Skipping world \"" + worldKey + "\": could not be loaded", e); + } } - worlds.add(codec().deserialize(worldKey, section)); - } catch (Exception e) { - logger.log(Level.WARNING, "Skipping world \"" + worldKey + "\": could not be loaded", e); - } - } - return worlds; - })); + return worlds; + }), + background); } private Set loadWorldKeys() { @@ -134,6 +152,7 @@ public CompletableFuture delete(BuildWorld buildWorld) { @Override public CompletableFuture delete(String worldKey) { - return CompletableFuture.runAsync(() -> store.atomicSave(() -> config.set(WORLDS_KEY + "." + worldKey, null))); + return CompletableFuture.runAsync( + () -> store.atomicSave(() -> config.set(WORLDS_KEY + "." + worldKey, null)), background); } } diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/util/FileUtils.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/util/FileUtils.java index 35465d107..33ae349b9 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/util/FileUtils.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/util/FileUtils.java @@ -150,7 +150,18 @@ public static void deleteDirectory(File directory) throws IOException { */ public static void deleteDirectory(Path directoryPath) throws IOException { try (Stream walk = Files.walk(directoryPath)) { - walk.sorted(Comparator.reverseOrder()).map(Path::toFile).forEach(File::delete); + // Children before parents, and every delete is checked: File#delete swallows failures (a locked or + // permission-denied file would leave orphaned data and report success), so use Files#delete and surface + // the first failure as the IOException this method already advertises. + walk.sorted(Comparator.reverseOrder()).forEach(path -> { + try { + Files.delete(path); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + }); + } catch (UncheckedIOException e) { + throw e.getCause(); } } diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/util/StringCleaner.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/util/StringCleaner.java index 381be164a..2668fe59d 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/util/StringCleaner.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/util/StringCleaner.java @@ -20,6 +20,8 @@ import java.io.File; import java.io.IOException; import java.util.Arrays; +import java.util.Locale; +import java.util.Set; import java.util.regex.Pattern; import org.jspecify.annotations.NullMarked; import org.jspecify.annotations.Nullable; @@ -32,6 +34,11 @@ public final class StringCleaner { private static final Pattern INVALID_NAME_PATTERN = Pattern.compile(INVALID_NAME_CHARACTERS); + /** Windows device names that cannot back a directory regardless of extension. */ + private static final Set RESERVED_NAMES = Set.of( + "CON", "PRN", "AUX", "NUL", "COM1", "COM2", "COM3", "COM4", "COM5", "COM6", "COM7", "COM8", "COM9", "LPT1", + "LPT2", "LPT3", "LPT4", "LPT5", "LPT6", "LPT7", "LPT8", "LPT9"); + private StringCleaner() {} public static boolean hasInvalidNameCharacters(String input, String configuredPattern) { @@ -57,6 +64,23 @@ public static String sanitize(String input, String configuredPattern) { .trim(); } + /** + * Checks whether a name is reserved or degenerate as a directory name. {@link #isPathEscape} already rejects names + * that traverse out of the container; this rejects ones that stay inside it but still cannot back a world folder: + * the current/parent aliases ({@code .}/{@code ..}), the empty name, and the Windows device names (CON, PRN, AUX, + * NUL, COM1-9, LPT1-9) that are unusable even on a server that later moves between platforms. + * + * @param name the candidate world or folder name + * @return {@code true} if the name must not be used + */ + public static boolean isReservedName(String name) { + String trimmed = name.trim(); + if (trimmed.isEmpty() || trimmed.equals(".") || trimmed.equals("..")) { + return true; + } + return RESERVED_NAMES.contains(trimmed.toUpperCase(Locale.ROOT)); + } + /** * Checks whether a resolved file escapes a given base directory. * diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/util/TaskScheduler.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/util/TaskScheduler.java index 5932a7db2..e13a30085 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/util/TaskScheduler.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/util/TaskScheduler.java @@ -17,23 +17,41 @@ */ package de.eintosti.buildsystem.util; +import java.util.concurrent.Executor; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.atomic.AtomicInteger; import org.bukkit.Bukkit; import org.bukkit.plugin.Plugin; import org.bukkit.scheduler.BukkitTask; import org.jspecify.annotations.NullMarked; /** - * Thin facade over the Bukkit scheduler that owns the {@link Plugin} handle, so collaborators can schedule work by - * depending on this single injectable instead of the whole {@code BuildSystemPlugin} (and its service-locator surface). - * Confines the {@code (plugin, ...)} scheduling boilerplate to one place. + * The plugin's single scheduling seam. Wraps the Bukkit scheduler (owning the {@link Plugin} handle so collaborators + * depend on this one injectable instead of the whole {@code BuildSystemPlugin}) and owns the bounded executor that backs + * all off-main work. Exposing {@link #mainThread()} and {@link #background()} as {@link Executor}s lets + * {@link java.util.concurrent.CompletableFuture} stages hop threads through the same seam, so there is no longer a mix of + * raw {@code Bukkit.getScheduler()} calls, ad-hoc {@code mainThreadExecutor} lambdas and the common ForkJoinPool. + * + *

The background pool is bounded and daemon-threaded; nothing running on it blocks on another background task (chains + * use non-blocking {@code CompletableFuture} composition), so a single shared pool cannot self-deadlock. Call + * {@link #shutdown()} once on plugin disable, after the final saves have completed. */ @NullMarked public final class TaskScheduler { private final Plugin plugin; + private final ExecutorService background; public TaskScheduler(Plugin plugin) { this.plugin = plugin; + int threads = Math.max(4, Runtime.getRuntime().availableProcessors()); + AtomicInteger counter = new AtomicInteger(); + this.background = Executors.newFixedThreadPool(threads, runnable -> { + Thread thread = new Thread(runnable, "BuildSystem-Worker-" + counter.incrementAndGet()); + thread.setDaemon(true); + return thread; + }); } public BukkitTask run(Runnable task) { @@ -55,4 +73,27 @@ public BukkitTask runAsync(Runnable task) { public BukkitTask runTimerAsync(Runnable task, long delayTicks, long periodTicks) { return Bukkit.getScheduler().runTaskTimerAsynchronously(plugin, task, delayTicks, periodTicks); } + + /** + * {@return an {@link Executor} that runs each task on the server main thread} For marshalling a + * {@link java.util.concurrent.CompletableFuture} stage back onto the main thread, e.g. + * {@code future.thenAcceptAsync(consumer, scheduler.mainThread())}. + */ + public Executor mainThread() { + return this::run; + } + + /** + * {@return the shared bounded executor for off-main work} Used as the explicit executor for + * {@code CompletableFuture.supplyAsync/runAsync} so storage I/O and backups share one observable, bounded pool + * instead of the unbounded common ForkJoinPool. + */ + public Executor background() { + return background; + } + + /** Shuts the background executor down; call once on plugin disable, after the final saves have completed. */ + public void shutdown() { + background.shutdown(); + } } diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/WorldServiceImpl.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/WorldServiceImpl.java index 75dc57827..c0dd4d8ce 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/WorldServiceImpl.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/WorldServiceImpl.java @@ -114,6 +114,9 @@ public WorldStorageImpl getWorldStorage() { public WorldBuilder newWorld(String name) { // Defense-in-depth: the menus sanitize names before they reach here, but this is public API. Reject any name // that would resolve outside the world container so a caller cannot create a directory in, say, plugins/. + if (StringCleaner.isReservedName(name)) { + throw new IllegalArgumentException("World name '" + name + "' is reserved and cannot be used"); + } File worldDirectory = new File(Bukkit.getWorldContainer(), name); if (StringCleaner.isPathEscape(Bukkit.getWorldContainer(), worldDirectory)) { throw new IllegalArgumentException("World name '" + name + "' resolves outside the world container"); @@ -256,20 +259,27 @@ public CompletableFuture deleteWorld(BuildWorld buildWorld) { // Bukkit statics from a pool thread. BukkitScheduler scheduler = Bukkit.getScheduler(); - // Registry removal and metadata persistence are awaited before folder deletion - // so a crash mid-delete leaves an orphaned folder (re-importable) rather than - // an orphaned registry entry pointing at a deleted folder. - return unimportWorld(buildWorld, SaveBehavior.DISCARD).thenRunAsync(() -> { - try { - FileUtils.deleteDirectory(deleteFolder); - } catch (IOException e) { - throw new CompletionException(new WorldDeletionException( - "An unexpected error occurred during directory deletion for world: " + worldName, e)); - } - scheduler.runTask( - plugin, - () -> Bukkit.getServer().getPluginManager().callEvent(new BuildWorldPostDeleteEvent(buildWorld))); - }); + // Registry removal and metadata persistence are awaited before folder deletion so a crash mid-delete leaves an + // orphaned folder (re-importable) rather than an orphaned registry entry pointing at a deleted folder. The + // directory delete is disk I/O, so it runs on the shared background pool. + return unimportWorld(buildWorld, SaveBehavior.DISCARD) + .thenRunAsync( + () -> { + try { + FileUtils.deleteDirectory(deleteFolder); + } catch (IOException e) { + throw new CompletionException(new WorldDeletionException( + "An unexpected error occurred during directory deletion for world: " + + worldName, + e)); + } + scheduler.runTask( + plugin, + () -> Bukkit.getServer() + .getPluginManager() + .callEvent(new BuildWorldPostDeleteEvent(buildWorld))); + }, + services.scheduler().background()); } /** @@ -280,7 +290,14 @@ public CompletableFuture deleteWorld(BuildWorld buildWorld) { * @param newName The name the world should be renamed to */ public void renameWorld(Player player, BuildWorld buildWorld, String newName) { - new WorldRenamer(plugin, this, worldStorage, services.config(), services.messages(), services.spawn()) + new WorldRenamer( + plugin, + this, + worldStorage, + services.config(), + services.messages(), + services.spawn(), + services.scheduler()) .rename(player, buildWorld, newName); } diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/data/WorldDataImpl.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/data/WorldDataImpl.java index 2fe09c514..3db9decfe 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/data/WorldDataImpl.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/data/WorldDataImpl.java @@ -44,6 +44,26 @@ @NullMarked public class WorldDataImpl implements WorldData { + /** + * Default values for the persisted world settings. These are the single source of truth for both world creation + * (the {@link WorldDataBuilder} field initializers) and deserialization ({@code WorldCodec} passes them to + * {@code getBoolean}/{@code getLong}/{@code getInt} so a key absent from disk restores the same value a freshly + * created world would have). Keeping them here keeps the two paths from drifting apart. + */ + public static final boolean DEFAULT_BLOCK_BREAKING = true; + + public static final boolean DEFAULT_BLOCK_INTERACTIONS = true; + public static final boolean DEFAULT_BLOCK_PLACEMENT = true; + public static final boolean DEFAULT_BUILDERS_ENABLED = false; + public static final boolean DEFAULT_EXPLOSIONS = true; + public static final boolean DEFAULT_MOB_AI = true; + public static final boolean DEFAULT_PHYSICS = true; + public static final boolean DEFAULT_PINNED = false; + public static final int DEFAULT_TIME_SINCE_BACKUP = 0; + + /** Sentinel for the {@code last-*} timestamps meaning "never". */ + public static final long DEFAULT_TIMESTAMP = -1L; + private final Map> data = new HashMap<>(); private String worldName; @@ -177,19 +197,19 @@ public static class WorldDataBuilder { private XMaterial material = XMaterial.GRASS_BLOCK; private String iconSkullTexture = ""; private @Nullable BuildWorldStatus status; - private boolean blockBreaking = true; - private boolean blockInteractions = true; - private boolean blockPlacement = true; - private boolean buildersEnabled = false; - private boolean explosions = true; - private boolean mobAi = true; - private boolean physics = true; - private boolean pinned = false; + private boolean blockBreaking = DEFAULT_BLOCK_BREAKING; + private boolean blockInteractions = DEFAULT_BLOCK_INTERACTIONS; + private boolean blockPlacement = DEFAULT_BLOCK_PLACEMENT; + private boolean buildersEnabled = DEFAULT_BUILDERS_ENABLED; + private boolean explosions = DEFAULT_EXPLOSIONS; + private boolean mobAi = DEFAULT_MOB_AI; + private boolean physics = DEFAULT_PHYSICS; + private boolean pinned = DEFAULT_PINNED; private Visibility visibility = Visibility.EVERYONE; - private int timeSinceBackup = 0; - private long lastEdited = -1L; - private long lastLoaded = -1L; - private long lastUnloaded = -1L; + private int timeSinceBackup = DEFAULT_TIME_SINCE_BACKUP; + private long lastEdited = DEFAULT_TIMESTAMP; + private long lastLoaded = DEFAULT_TIMESTAMP; + private long lastUnloaded = DEFAULT_TIMESTAMP; BooleanSupplier permissionOverrideEnabled = () -> false; BooleanSupplier projectOverrideEnabled = () -> false; diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/folder/FolderImpl.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/folder/FolderImpl.java index 6699f405b..3ebde28a3 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/folder/FolderImpl.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/folder/FolderImpl.java @@ -42,7 +42,7 @@ public class FolderImpl implements Folder { private final Builder creator; private final long creation; private NavigatorCategory category; - private final List worlds; + private final Set worlds; private final List subfolders; private @Nullable Folder parent; @@ -88,15 +88,15 @@ public FolderImpl( this.creation = creation; this.category = category; this.parent = parent; - if (parent != null && !parent.getSubFolders().contains(this)) { - ((FolderImpl) parent).subfolders.add(this); - } this.creator = creator; - this.worlds = new ArrayList<>(worlds); + this.worlds = new LinkedHashSet<>(worlds); this.material = material; this.permission = permission; this.project = project; - this.subfolders = subfolders; + this.subfolders = new ArrayList<>(subfolders); + if (parent != null) { + ((FolderImpl) parent).addSubFolder(this); + } } @Override @@ -195,10 +195,10 @@ public void setParent(@Nullable Folder parent) { .formatted(this.category.getId(), parent.getCategory().getId())); } - if (parent != null && !parent.getSubFolders().contains(this)) { - ((FolderImpl) parent).subfolders.add(this); - } else if (parent == null && this.parent != null) { - ((FolderImpl) this.parent).subfolders.remove(this); + if (parent != null) { + ((FolderImpl) parent).addSubFolder(this); + } else if (this.parent != null) { + ((FolderImpl) this.parent).removeSubFolder(this); } this.parent = parent; @@ -212,7 +212,7 @@ public boolean hasParent() { @Override @Unmodifiable public List getWorldUUIDs() { - return Collections.unmodifiableList(this.worlds); + return List.copyOf(this.worlds); } @Override @@ -227,9 +227,7 @@ public boolean containsWorld(UUID uuid) { @Override public void addWorld(BuildWorld buildWorld) { - if (!this.worlds.contains(buildWorld.getUniqueId())) { - this.worlds.add(buildWorld.getUniqueId()); - } + this.worlds.add(buildWorld.getUniqueId()); // BuildWorld owns the back-reference; only update it when it is not already pointing here, which also // terminates the addWorld <-> setFolder handshake. if (buildWorld.getFolder() != this) { @@ -257,6 +255,24 @@ public List getSubFolders() { return Collections.unmodifiableList(this.subfolders); } + /** + * Adds {@code child} to this folder's sub-folder list, ignoring duplicates. Package-private: membership is driven by + * {@link #setParent}, so a folder owns its own sub-folder list rather than having it mutated through a cast from + * outside. + */ + void addSubFolder(FolderImpl child) { + if (!this.subfolders.contains(child)) { + this.subfolders.add(child); + } + } + + /** + * Removes {@code child} from this folder's sub-folder list. Counterpart to {@link #addSubFolder(FolderImpl)}. + */ + void removeSubFolder(FolderImpl child) { + this.subfolders.remove(child); + } + @Override public int getWorldCount() { int total = this.worlds.size(); diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/lifecycle/WorldRenamer.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/lifecycle/WorldRenamer.java index 80507cd02..ad1256c00 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/lifecycle/WorldRenamer.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/lifecycle/WorldRenamer.java @@ -26,6 +26,7 @@ import de.eintosti.buildsystem.storage.WorldStorageImpl; import de.eintosti.buildsystem.util.FileUtils; import de.eintosti.buildsystem.util.StringCleaner; +import de.eintosti.buildsystem.util.TaskScheduler; import de.eintosti.buildsystem.world.WorldServiceImpl; import de.eintosti.buildsystem.world.creation.BukkitWorldFactory; import de.eintosti.buildsystem.world.spawn.SpawnService; @@ -57,6 +58,7 @@ public class WorldRenamer { private final ConfigService configService; private final Messages messages; private final SpawnService spawnService; + private final TaskScheduler scheduler; public WorldRenamer( BuildSystemPlugin plugin, @@ -64,13 +66,15 @@ public WorldRenamer( WorldStorageImpl worldStorage, ConfigService configService, Messages messages, - SpawnService spawnService) { + SpawnService spawnService, + TaskScheduler scheduler) { this.plugin = plugin; this.worldService = worldService; this.worldStorage = worldStorage; this.configService = configService; this.messages = messages; this.spawnService = spawnService; + this.scheduler = scheduler; } public void rename(Player player, BuildWorld buildWorld, String newName) { @@ -124,25 +128,26 @@ private void prepareAndMove( File oldWorldFile = new File(Bukkit.getWorldContainer(), oldName); File newWorldFile = new File(Bukkit.getWorldContainer(), sanitizedNewName); - CompletableFuture.runAsync(() -> { - try { - FileUtils.copy(oldWorldFile, newWorldFile); - FileUtils.deleteDirectory(oldWorldFile); - } catch (Exception e) { - throw new RuntimeException("Failed to rename world directory", e); - } - }) - .thenRun(() -> Bukkit.getScheduler() - .runTask( - plugin, - () -> reconstruct( - player, - buildWorld, - oldName, - sanitizedNewName, - oldWorld, - oldSpawnLocation, - removedPlayers))); + CompletableFuture.runAsync( + () -> { + try { + FileUtils.copy(oldWorldFile, newWorldFile); + FileUtils.deleteDirectory(oldWorldFile); + } catch (Exception e) { + throw new RuntimeException("Failed to rename world directory", e); + } + }, + scheduler.background()) + .thenRunAsync( + () -> reconstruct( + player, + buildWorld, + oldName, + sanitizedNewName, + oldWorld, + oldSpawnLocation, + removedPlayers), + scheduler.mainThread()); } private void reconstruct( diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/spawn/SpawnService.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/spawn/SpawnService.java index 7ac34dbd3..93924841c 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/spawn/SpawnService.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/spawn/SpawnService.java @@ -22,9 +22,11 @@ import de.eintosti.buildsystem.api.storage.WorldStorage; import de.eintosti.buildsystem.api.world.BuildWorld; import de.eintosti.buildsystem.storage.yaml.YamlSpawnStorage; +import de.eintosti.buildsystem.util.TaskScheduler; import de.eintosti.buildsystem.world.WorldServiceImpl; import io.papermc.lib.PaperLib; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.Executor; import org.bukkit.Bukkit; import org.bukkit.Location; import org.bukkit.World; @@ -39,14 +41,16 @@ public class SpawnService { private final BuildSystemPlugin plugin; private final WorldStorage worldStorage; private final YamlSpawnStorage spawnStorage; + private final Executor background; private @Nullable String spawnName; private @Nullable Location spawn; - public SpawnService(BuildSystemPlugin plugin, WorldServiceImpl worldService) { + public SpawnService(BuildSystemPlugin plugin, WorldServiceImpl worldService, TaskScheduler scheduler) { this.plugin = plugin; this.worldStorage = worldService.getWorldStorage(); this.spawnStorage = new YamlSpawnStorage(plugin); + this.background = scheduler.background(); load(); } @@ -98,7 +102,7 @@ public void remove() { } public CompletableFuture save() { - return CompletableFuture.runAsync(() -> spawnStorage.saveSpawn(spawn)); + return CompletableFuture.runAsync(() -> spawnStorage.saveSpawn(spawn), background); } private void load() { diff --git a/buildsystem-core/src/test/java/de/eintosti/buildsystem/config/PluginConfigTest.java b/buildsystem-core/src/test/java/de/eintosti/buildsystem/config/PluginConfigTest.java index c87385d0f..a26a4b4b1 100644 --- a/buildsystem-core/src/test/java/de/eintosti/buildsystem/config/PluginConfigTest.java +++ b/buildsystem-core/src/test/java/de/eintosti/buildsystem/config/PluginConfigTest.java @@ -35,7 +35,7 @@ private PluginConfig parse(String yaml) { } catch (Exception e) { throw new RuntimeException(e); } - return ConfigService.parseForTest(config, LOGGER); + return ConfigService.parse(config, LOGGER, XMaterial.WOODEN_AXE); } // ----------------------------------------------------------------------- @@ -275,6 +275,25 @@ void backupStorage_sftpType_returnsSftpRecord() { assertEquals("/backups/", sftp.path()); } + @Test + void backupStorage_s3MissingRequiredKey_fallsBackToLocal() { + // 'bucket' is required for S3; without it the remote cannot work, so it must fall back to local rather than + // construct an S3 record with a null bucket that only fails later at connection time. + PluginConfig cfg = parse(""" + world: + backup: + storage: + type: s3 + s3: + url: "https://example.com" + access-key: "MYACCESSKEY" + secret-key: "MYSECRETKEY" + """); + + assertInstanceOf( + PluginConfig.World.Backup.Local.class, cfg.world().backup().storage()); + } + // ----------------------------------------------------------------------- // 5. Backup storage: unknown type defaults to Local // ----------------------------------------------------------------------- diff --git a/buildsystem-core/src/test/java/de/eintosti/buildsystem/config/WorldEditWandDetectorTest.java b/buildsystem-core/src/test/java/de/eintosti/buildsystem/config/WorldEditWandDetectorTest.java new file mode 100644 index 000000000..017d24c60 --- /dev/null +++ b/buildsystem-core/src/test/java/de/eintosti/buildsystem/config/WorldEditWandDetectorTest.java @@ -0,0 +1,68 @@ +/* + * Copyright (c) 2018-2026, Thomas Meaney + * Copyright (c) contributors + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package de.eintosti.buildsystem.config; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import com.cryptomorin.xseries.XMaterial; +import java.nio.file.Files; +import java.nio.file.Path; +import org.jspecify.annotations.NullMarked; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +@NullMarked +class WorldEditWandDetectorTest { + + @TempDir + Path pluginsDir; + + private void writeConfig(String pluginDir, String fileName, String yaml) throws Exception { + Path dir = Files.createDirectories(pluginsDir.resolve(pluginDir)); + Files.writeString(dir.resolve(fileName), yaml); + } + + @Test + void detect_nullDir_returnsDefault() { + assertEquals(WorldEditWandDetector.DEFAULT_WAND, new WorldEditWandDetector(null).detect()); + } + + @Test + void detect_noWorldEditInstalled_returnsDefault() { + assertEquals(WorldEditWandDetector.DEFAULT_WAND, new WorldEditWandDetector(pluginsDir.toFile()).detect()); + } + + @Test + void detect_worldEditConfig_readsAndStripsNamespace() throws Exception { + writeConfig("WorldEdit", "config.yml", "wand-item: minecraft:blaze_rod"); + assertEquals(XMaterial.BLAZE_ROD, new WorldEditWandDetector(pluginsDir.toFile()).detect()); + } + + @Test + void detect_fastAsyncWorldEditTakesPrecedence() throws Exception { + writeConfig("WorldEdit", "config.yml", "wand-item: minecraft:blaze_rod"); + writeConfig("FastAsyncWorldEdit", "worldedit-config.yml", "wand-item: minecraft:stick"); + assertEquals(XMaterial.STICK, new WorldEditWandDetector(pluginsDir.toFile()).detect()); + } + + @Test + void detect_missingWandKey_returnsDefault() throws Exception { + writeConfig("WorldEdit", "config.yml", "some-other-key: 5"); + assertEquals(WorldEditWandDetector.DEFAULT_WAND, new WorldEditWandDetector(pluginsDir.toFile()).detect()); + } +} diff --git a/buildsystem-core/src/test/java/de/eintosti/buildsystem/storage/WorldStorageImplTest.java b/buildsystem-core/src/test/java/de/eintosti/buildsystem/storage/WorldStorageImplTest.java index 0cf4c495b..09f0f3afa 100644 --- a/buildsystem-core/src/test/java/de/eintosti/buildsystem/storage/WorldStorageImplTest.java +++ b/buildsystem-core/src/test/java/de/eintosti/buildsystem/storage/WorldStorageImplTest.java @@ -173,6 +173,19 @@ void concurrentAddRemoveIsConsistent() throws InterruptedException { } } + @Test + void rename_caseOnlyChange_keepsNameResolvable() { + // Old and new names lower-case to the same key; the world must stay resolvable after the rename. + BuildWorld w = world("India"); + storage.addBuildWorld(w); + + when(w.getName()).thenReturn("INDIA"); + storage.rename(w, "India", "INDIA"); + + assertSame(w, storage.getBuildWorld("india")); + assertSame(w, storage.getBuildWorld("INDIA")); + } + @Test void rename_remapsNameLookup() { BuildWorld buildWorld = world("oldName"); diff --git a/buildsystem-core/src/test/java/de/eintosti/buildsystem/storage/yaml/YamlPlayerStorageRoundTripTest.java b/buildsystem-core/src/test/java/de/eintosti/buildsystem/storage/yaml/YamlPlayerStorageRoundTripTest.java index 3ac5f0d4f..49829104d 100644 --- a/buildsystem-core/src/test/java/de/eintosti/buildsystem/storage/yaml/YamlPlayerStorageRoundTripTest.java +++ b/buildsystem-core/src/test/java/de/eintosti/buildsystem/storage/yaml/YamlPlayerStorageRoundTripTest.java @@ -26,6 +26,7 @@ import de.eintosti.buildsystem.player.BuildPlayerImpl; import de.eintosti.buildsystem.player.LogoutLocation; import de.eintosti.buildsystem.player.settings.SettingsImpl; +import de.eintosti.buildsystem.util.TaskScheduler; import java.io.File; import java.util.Collection; import java.util.UUID; @@ -39,11 +40,17 @@ class YamlPlayerStorageRoundTripTest { File dataFolder; private BuildSystemPlugin plugin; + private TaskScheduler scheduler; @BeforeEach void setUp() { plugin = mock(BuildSystemPlugin.class, RETURNS_DEEP_STUBS); when(plugin.getDataFolder()).thenReturn(dataFolder); + scheduler = new TaskScheduler(plugin); + } + + private YamlPlayerStorage newStorage() { + return new YamlPlayerStorage(plugin, scheduler); } private BuildPlayerImpl samplePlayer(UUID uuid) { @@ -65,9 +72,9 @@ void saveAndLoad_roundTripsSettingsAndLogoutLocation() throws Exception { UUID uuid = UUID.randomUUID(); BuildPlayerImpl original = samplePlayer(uuid); - new YamlPlayerStorage(plugin).save(original).join(); + newStorage().save(original).join(); - Collection loaded = new YamlPlayerStorage(plugin).load().get(); + Collection loaded = newStorage().load().get(); assertEquals(1, loaded.size()); BuildPlayerImpl reloaded = BuildPlayerImpl.of(loaded.iterator().next()); @@ -87,7 +94,7 @@ void saveAndLoad_roundTripsSettingsAndLogoutLocation() throws Exception { @Test void load_missingFile_returnsEmptyCollection() throws Exception { - Collection loaded = new YamlPlayerStorage(plugin).load().get(); + Collection loaded = newStorage().load().get(); assertTrue(loaded.isEmpty()); } diff --git a/buildsystem-core/src/test/java/de/eintosti/buildsystem/storage/yaml/YamlWorldStorageRoundTripTest.java b/buildsystem-core/src/test/java/de/eintosti/buildsystem/storage/yaml/YamlWorldStorageRoundTripTest.java index 4c37da137..872b5d9de 100644 --- a/buildsystem-core/src/test/java/de/eintosti/buildsystem/storage/yaml/YamlWorldStorageRoundTripTest.java +++ b/buildsystem-core/src/test/java/de/eintosti/buildsystem/storage/yaml/YamlWorldStorageRoundTripTest.java @@ -29,6 +29,7 @@ import de.eintosti.buildsystem.api.world.data.Visibility; import de.eintosti.buildsystem.api.world.data.WorldDataKey; import de.eintosti.buildsystem.test.TestData; +import de.eintosti.buildsystem.util.TaskScheduler; import de.eintosti.buildsystem.world.BuildWorldImpl; import de.eintosti.buildsystem.world.WorldContext; import de.eintosti.buildsystem.world.data.WorldDataImpl; @@ -167,10 +168,12 @@ void load_emptyFile_returnsEmptyCollection() { } @Test - void construction_doesNotResolveServices() { - // Worlds are loaded during plugin enable, before services such as MenuItems/SpawnService exist. Constructing - // the storage must not resolve any service (the codec is built lazily on first load); otherwise startup throws. + void construction_doesNotResolveLateServices() { + // Worlds are loaded during plugin enable, before the late services the codec needs (MenuItems/SpawnService via + // the WorldContext, and PlayerLookupService) exist. Construction must not resolve those (the codec is built + // lazily on first load); only the always-available scheduler may be pulled. Services strict = mock(Services.class); + when(strict.scheduler()).thenReturn(new TaskScheduler(plugin)); when(strict.worldContext()).thenThrow(new IllegalStateException("service not initialized yet")); when(strict.playerLookup()).thenThrow(new IllegalStateException("service not initialized yet")); @@ -205,6 +208,45 @@ void load_missingStatus_defaultsToNotStarted() throws Exception { assertEquals(TestData.NOT_STARTED, loaded.iterator().next().getData().get(WorldDataKey.STATUS)); } + @Test + void load_invalidDifficultyEnum_defaultsToPeaceful() throws Exception { + YamlConfiguration yaml = new YamlConfiguration(); + yaml.set("worlds.Bad.uuid", UUID.randomUUID().toString()); + yaml.set("worlds.Bad.type", "NORMAL"); + yaml.set("worlds.Bad.date", 1L); + yaml.set("worlds.Bad.data.status", "FINISHED"); + yaml.set("worlds.Bad.data.difficulty", "NOT_A_DIFFICULTY"); + yaml.save(new File(dataFolder, "worlds.yml")); + + Collection loaded = newStorage().load().join(); + assertEquals(1, loaded.size()); + assertEquals(Difficulty.PEACEFUL, loaded.iterator().next().getData().get(WorldDataKey.DIFFICULTY)); + } + + @Test + void load_missingDataKeys_restoreCreationDefaults() throws Exception { + // A world entry that predates these keys (or was hand-edited) must come back with the same defaults a freshly + // created world has — not getBoolean/getLong's implicit false/0. + YamlConfiguration yaml = new YamlConfiguration(); + yaml.set("worlds.Sparse.uuid", UUID.randomUUID().toString()); + yaml.set("worlds.Sparse.type", "NORMAL"); + yaml.set("worlds.Sparse.date", 1L); + yaml.set("worlds.Sparse.data.status", "FINISHED"); + yaml.save(new File(dataFolder, "worlds.yml")); + + BuildWorld world = newStorage().load().join().iterator().next(); + assertTrue(world.getData().get(WorldDataKey.BLOCK_BREAKING)); + assertTrue(world.getData().get(WorldDataKey.BLOCK_INTERACTIONS)); + assertTrue(world.getData().get(WorldDataKey.BLOCK_PLACEMENT)); + assertTrue(world.getData().get(WorldDataKey.EXPLOSIONS)); + assertTrue(world.getData().get(WorldDataKey.MOB_AI)); + assertTrue(world.getData().get(WorldDataKey.PHYSICS)); + assertFalse(world.getData().get(WorldDataKey.BUILDERS_ENABLED)); + assertEquals(WorldDataImpl.DEFAULT_TIMESTAMP, world.getData().get(WorldDataKey.LAST_EDITED)); + assertEquals(WorldDataImpl.DEFAULT_TIMESTAMP, world.getData().get(WorldDataKey.LAST_LOADED)); + assertEquals(WorldDataImpl.DEFAULT_TIMESTAMP, world.getData().get(WorldDataKey.LAST_UNLOADED)); + } + @Test void load_unparseableEntry_isSkipped_othersStillLoad() throws Exception { // Persist a good world, then inject a sibling entry whose UUID cannot be parsed. diff --git a/buildsystem-core/src/test/java/de/eintosti/buildsystem/test/TestData.java b/buildsystem-core/src/test/java/de/eintosti/buildsystem/test/TestData.java index 6813f133f..b30b92ca8 100644 --- a/buildsystem-core/src/test/java/de/eintosti/buildsystem/test/TestData.java +++ b/buildsystem-core/src/test/java/de/eintosti/buildsystem/test/TestData.java @@ -202,6 +202,7 @@ public static Services mockServices() { Prompts prompts = mock(Prompts.class); Services services = mock(Services.class); lenient().when(services.worldContext()).thenReturn(context); + lenient().when(services.scheduler()).thenReturn(context.scheduler()); lenient().when(services.config()).thenReturn(context.configService()); lenient().when(services.messages()).thenReturn(context.messages()); lenient().when(services.player()).thenReturn(context.playerService()); diff --git a/buildsystem-core/src/test/java/de/eintosti/buildsystem/util/FileUtilsTest.java b/buildsystem-core/src/test/java/de/eintosti/buildsystem/util/FileUtilsTest.java index e57ed00f7..e47d6c1b1 100644 --- a/buildsystem-core/src/test/java/de/eintosti/buildsystem/util/FileUtilsTest.java +++ b/buildsystem-core/src/test/java/de/eintosti/buildsystem/util/FileUtilsTest.java @@ -18,6 +18,7 @@ package de.eintosti.buildsystem.util; import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assumptions.assumeTrue; import java.io.ByteArrayInputStream; import java.io.File; @@ -92,6 +93,22 @@ void deleteDirectory_missingDirectoryThrows() { assertThrows(IOException.class, () -> FileUtils.deleteDirectory(missing)); } + @Test + void deleteDirectory_failedDeleteIsReportedNotSwallowed() throws IOException { + File world = createWorldLikeDirectory("locked"); + Path region = world.toPath().resolve("region"); + + // Clear write permission on the directory so its contents cannot be deleted. Skip where the platform or user + // does not enforce it (e.g. running as root, or a filesystem ignoring the bit) so the test stays deterministic. + assumeTrue(region.toFile().setWritable(false, false), "could not make directory read-only"); + try { + assumeTrue(!Files.isWritable(region), "directory write permission is not enforced here"); + assertThrows(IOException.class, () -> FileUtils.deleteDirectory(world)); + } finally { + region.toFile().setWritable(true, false); + } + } + @Test void getDirectoryCreation_returnsPlausibleTimestamp() throws IOException { File source = createWorldLikeDirectory("created"); diff --git a/buildsystem-core/src/test/java/de/eintosti/buildsystem/util/StringCleanerTest.java b/buildsystem-core/src/test/java/de/eintosti/buildsystem/util/StringCleanerTest.java index f82c57cf5..2e90ecd76 100644 --- a/buildsystem-core/src/test/java/de/eintosti/buildsystem/util/StringCleanerTest.java +++ b/buildsystem-core/src/test/java/de/eintosti/buildsystem/util/StringCleanerTest.java @@ -84,6 +84,31 @@ void sanitize_trailingWhitespace_trimmed() { assertEquals("abc", StringCleaner.sanitize(" abc ", NO_OP)); } + @Test + void isReservedName_windowsDeviceNames_returnTrueCaseInsensitively() { + assertTrue(StringCleaner.isReservedName("CON")); + assertTrue(StringCleaner.isReservedName("con")); + assertTrue(StringCleaner.isReservedName("CoM1")); + assertTrue(StringCleaner.isReservedName("lpt9")); + assertTrue(StringCleaner.isReservedName("NUL")); + } + + @Test + void isReservedName_dotAndEmpty_returnTrue() { + assertTrue(StringCleaner.isReservedName(".")); + assertTrue(StringCleaner.isReservedName("..")); + assertTrue(StringCleaner.isReservedName("")); + assertTrue(StringCleaner.isReservedName(" ")); + } + + @Test + void isReservedName_ordinaryNames_returnFalse() { + assertFalse(StringCleaner.isReservedName("World")); + assertFalse(StringCleaner.isReservedName("COM")); // no trailing digit + assertFalse(StringCleaner.isReservedName("COM10")); // outside COM1-9 + assertFalse(StringCleaner.isReservedName("console")); + } + @Test void isPathEscape_normalChild_returnsFalse() { File base = new File("/srv/templates"); diff --git a/buildsystem-core/src/test/java/de/eintosti/buildsystem/world/folder/FolderImplTest.java b/buildsystem-core/src/test/java/de/eintosti/buildsystem/world/folder/FolderImplTest.java index 65ad3e556..46b49565c 100644 --- a/buildsystem-core/src/test/java/de/eintosti/buildsystem/world/folder/FolderImplTest.java +++ b/buildsystem-core/src/test/java/de/eintosti/buildsystem/world/folder/FolderImplTest.java @@ -28,10 +28,7 @@ import de.eintosti.buildsystem.world.WorldContext; import de.eintosti.buildsystem.world.data.WorldDataImpl; import de.eintosti.buildsystem.world.data.WorldDataImpl.WorldDataBuilder; -import java.util.HashSet; -import java.util.List; -import java.util.Set; -import java.util.UUID; +import java.util.*; import org.bukkit.Difficulty; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -137,6 +134,63 @@ void removeWorld_clearsBothSides() { assertFalse(folder.containsWorld(world)); } + @Test + void addWorld_isIdempotent() { + FolderImpl folder = folder("Target"); + BuildWorldImpl world = world("world"); + + folder.addWorld(world); + folder.addWorld(world); + + assertEquals(1, folder.getWorldUUIDs().size()); + assertEquals(1, folder.getWorldCount()); + } + + @Test + void setParent_registersThenDeregistersSubFolder() { + FolderImpl parent = folder("Parent"); + FolderImpl child = folder("Child"); + + child.setParent(parent); + assertTrue(parent.getSubFolders().contains(child)); + + child.setParent(null); + assertTrue(parent.getSubFolders().isEmpty()); + } + + @Test + void setParent_repeatedDoesNotDuplicate() { + FolderImpl parent = folder("Parent"); + FolderImpl child = folder("Child"); + + child.setParent(parent); + child.setParent(parent); + + assertEquals(1, parent.getSubFolders().size()); + } + + @Test + void constructor_defensivelyCopiesWorlds() { + List worlds = new ArrayList<>(List.of(UUID.randomUUID())); + FolderImpl folder = new FolderImpl( + context, + UUID.randomUUID(), + "Detached", + 0L, + TestData.PUBLIC, + null, + Builder.of(UUID.randomUUID(), "Creator"), + XMaterial.CHEST, + "-", + "-", + worlds, + new ArrayList<>()); + + worlds.clear(); + + assertEquals(1, folder.getWorldUUIDs().size()); + } + @Test void identity_survivesRename() { FolderImpl folder = folder("Original");