diff options
| author | Sho Sakuma <me@m1sk9.dev> | 2026-08-05 03:32:11 +0900 |
|---|---|---|
| committer | Sho Sakuma <me@m1sk9.dev> | 2026-08-05 03:32:22 +0900 |
| commit | 7ed4bbbcf375c4f136a3b90bb6105c278901f654 (patch) | |
| tree | 7244202ebd1dd8915d34f1be0cd7d6e7aa423041 | |
| parent | 5a20137f7822e7aa3a37716da2e3a450bc6cea96 (diff) | |
| download | LunaticChat-7ed4bbbcf375c4f136a3b90bb6105c278901f654.tar.gz LunaticChat-7ed4bbbcf375c4f136a3b90bb6105c278901f654.tar.bz2 LunaticChat-7ed4bbbcf375c4f136a3b90bb6105c278901f654.zip | |
refactor: make durability and teardown properties of the layer, not habits
Atomicity was opt-in per write site, so a file added later was safe only if its
author noticed the convention. Worse, DebouncedSaver drops a request while one is
pending and so serves exactly one file - a rule held up only by the wiring
happening to construct a separate saver per file, and written down nowhere. A
FileStore now owns its file, its atomic write and its own saver, so neither can
be got wrong by wiring; writeTextAtomically is internal to the package.
Taking the Bukkit plugin out of DebouncedSaver in favour of an AsyncScheduler
makes the debounce testable at all: ChannelStorage's "the snapshot is taken when
the write runs" now runs against the real thing rather than a mocked saver.
Teardown gets the same treatment. The five services with shutdown work spelled it
saveToDisk() three times and shutdown() twice, and the list of them was
hand-maintained against a fourteen-field container - so a new service was not
stopped unless someone remembered a second place. They now implement
StoppableService and register as they are built, and shutdown iterates that list.
stop() delegates rather than renames, because the conversion cache's periodic
flush is a different caller from shutdown.
Also here, on files this commit already touches: player settings carry a dirty
flag, since updateSettings is the only writer and queues its own save, so every
quit was re-serializing every stored player to write identical bytes; and
setPlayerChannel returns early when nothing moved, because the quit path clears
the active channel for every player whether or not they had one, and a mass
disconnect paid a full snapshot per player in one tick.
Co-Authored-By: Claude <noreply@anthropic.com>
21 files changed, 441 insertions, 192 deletions
diff --git a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/ServiceContainer.kt b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/ServiceContainer.kt index 48f6e37..13cb31b 100644 --- a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/ServiceContainer.kt +++ b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/ServiceContainer.kt @@ -51,4 +51,11 @@ data class ServiceContainer( val crossServerChatManager: CrossServerChatManager? = null, val crossServerDirectMessageManager: CrossServerDirectMessageManager? = null, val remotePlayerRegistry: RemotePlayerRegistry? = null, + /** + * The services with teardown, in the order it must happen. + * + * Built as the services are, so a service that needs stopping is stopped because it was + * registered where it was created - not because someone remembered to extend a second list. + */ + val stoppables: List<StoppableService> = emptyList(), ) diff --git a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/ServiceInitializer.kt b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/ServiceInitializer.kt index bbfaab2..cac49c7 100644 --- a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/ServiceInitializer.kt +++ b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/ServiceInitializer.kt @@ -14,6 +14,8 @@ import dev.m1sk9.lunaticChat.paper.converter.RomanjiConverter import dev.m1sk9.lunaticChat.paper.i18n.LanguageManager import dev.m1sk9.lunaticChat.paper.settings.PlayerSettingsManager import dev.m1sk9.lunaticChat.paper.settings.YamlPlayerSettingsStorage +import dev.m1sk9.lunaticChat.paper.storage.AsyncScheduler +import dev.m1sk9.lunaticChat.paper.storage.FileStore import dev.m1sk9.lunaticChat.paper.velocity.CrossServerChatManager import dev.m1sk9.lunaticChat.paper.velocity.CrossServerDirectMessageManager import dev.m1sk9.lunaticChat.paper.velocity.RemotePlayerRegistry @@ -54,6 +56,14 @@ class ServiceInitializer( ) { private val handshakeCompleted = AtomicBoolean(false) + private val asyncScheduler = + AsyncScheduler { delaySeconds, task -> + plugin.server.asyncScheduler.runDelayed(plugin, { task() }, delaySeconds, TimeUnit.SECONDS) + } + + /** A store for [relativePath] under the plugin's data folder, with its own debounced saver. */ + private fun fileStore(relativePath: String) = FileStore(plugin.dataFolder.resolve(relativePath).toPath(), asyncScheduler, logger) + private companion object { /** Matches the value documented in config.yml. */ const val DEFAULT_CACHE_SAVE_INTERVAL_SECONDS = 300L @@ -119,26 +129,19 @@ class ServiceInitializer( } // 7. Initialize cross-server chat manager (optional) + // + // Gated on velocityManager alone: it is non-null only when velocityIntegration.enabled, so + // testing that flag again here would let the two conditions disagree. val crossServerManager = - if (configuration.features.velocityIntegration.enabled && - configuration.features.velocityIntegration.crossServerGlobalChat && - velocityManager != null - ) { - initializeCrossServerChatManager(velocityManager) - } else { - null - } + velocityManager + ?.takeIf { configuration.features.velocityIntegration.crossServerGlobalChat } + ?.let { initializeCrossServerChatManager(it) } // 8. Initialize cross-server direct message manager and presence registry (optional) val crossServerDirectMessage = - if (configuration.features.velocityIntegration.enabled && - configuration.features.velocityIntegration.crossServerDirectMessage && - velocityManager != null - ) { - initializeCrossServerDirectMessage(velocityManager, directMessageHandler, languageManager) - } else { - null - } + velocityManager + ?.takeIf { configuration.features.velocityIntegration.crossServerDirectMessage } + ?.let { initializeCrossServerDirectMessage(it, directMessageHandler, languageManager) } return ServiceContainer( languageManager = languageManager, @@ -155,6 +158,16 @@ class ServiceInitializer( crossServerChatManager = crossServerManager, crossServerDirectMessageManager = crossServerDirectMessage?.first, remotePlayerRegistry = crossServerDirectMessage?.second, + // Ordered: player-visible state is persisted first, then the log is flushed, and the + // proxy connection is closed last so a relay in flight still has somewhere to go. + stoppables = + listOfNotNull( + playerSettingsManager, + japaneseConversion?.second, + channelComponents?.channelManager, + channelComponents?.channelMessageLogger, + velocityManager, + ), ) } @@ -163,11 +176,9 @@ class ServiceInitializer( * This is always needed for features like DM notifications. */ private fun initializePlayerSettingsManager(): PlayerSettingsManager { - val settingsFile = plugin.dataFolder.resolve(configuration.userSettingsFilePath).toPath() val storage = YamlPlayerSettingsStorage( - settingsFile = settingsFile, - saver = DebouncedSaver(plugin), + store = fileStore(configuration.userSettingsFilePath), logger = logger, ) @@ -190,7 +201,7 @@ class ServiceInitializer( // Initialize conversion cache val cache = ConversionCache( - cacheFile = plugin.dataFolder.resolve(configuration.features.japaneseConversion.cache.filePath).toPath(), + store = fileStore(configuration.features.japaneseConversion.cache.filePath), maxEntries = configuration.features.japaneseConversion.cache.maxEntries, logger = logger, ) @@ -223,11 +234,9 @@ class ServiceInitializer( settingsManager: PlayerSettingsManager, languageManager: LanguageManager, ): ChannelComponents { - val channelsFile = plugin.dataFolder.resolve("channels.json").toPath() val storage = ChannelStorage( - channelsFile = channelsFile, - saver = DebouncedSaver(plugin), + store = fileStore("channels.json"), logger = logger, ) @@ -453,24 +462,15 @@ class ServiceInitializer( * Performs shutdown tasks, including saving all caches to disk. */ fun shutdown(services: ServiceContainer) { - shutdownStep("save player settings") { services.playerSettingsManager.saveToDisk() } - shutdownStep("save the conversion cache") { services.conversionCache?.saveToDisk() } - shutdownStep("save channel data") { services.channelManager?.saveToDisk() } - shutdownStep("shut down the channel message logger") { services.channelMessageLogger?.shutdown() } - shutdownStep("shut down the Velocity connection") { services.velocityConnectionManager?.shutdown() } - } - - // The steps are independent, so one that throws must not skip the ones after it - which is what - // an exception escaping onDisable would do, leaving the log flusher and the Velocity connection - // to be torn down by the server instead. - private fun shutdownStep( - what: String, - step: () -> Unit, - ) { - try { - step() - } catch (e: Exception) { - logger.log(Level.SEVERE, "Failed to $what during shutdown", e) + // The services are independent, so one that throws must not skip the ones after it - which is + // what an exception escaping onDisable would do, leaving the log flusher and the Velocity + // connection to be torn down by the server instead. + services.stoppables.forEach { service -> + try { + service.stop() + } catch (e: Exception) { + logger.log(Level.SEVERE, "Failed to stop ${service::class.simpleName} during shutdown", e) + } } } } diff --git a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/StoppableService.kt b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/StoppableService.kt new file mode 100644 index 0000000..7ebd25f --- /dev/null +++ b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/StoppableService.kt @@ -0,0 +1,16 @@ +package dev.m1sk9.lunaticChat.paper + +/** + * A service with work to finish before the server stops - a cache to flush, a connection to close. + * + * Implementing this is how a service gets torn down: [ServiceInitializer] registers each one as it + * builds it, so shutdown follows from construction rather than from a second hand-maintained list + * that a new service is silently missing from. + * + * The five services that had teardown before this spelled it `saveToDisk()` three times and + * `shutdown()` twice, so nothing but a reader could tell they were the same obligation. + */ +interface StoppableService { + /** Finishes outstanding work. Called once, on the shutdown path, off the tick thread. */ + fun stop() +} diff --git a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/chat/channel/ChannelManager.kt b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/chat/channel/ChannelManager.kt index 663984c..2feb87e 100644 --- a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/chat/channel/ChannelManager.kt +++ b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/chat/channel/ChannelManager.kt @@ -13,6 +13,7 @@ import dev.m1sk9.lunaticChat.engine.exception.ChannelNoOwnerPermissionException import dev.m1sk9.lunaticChat.engine.exception.ChannelNotFoundException import dev.m1sk9.lunaticChat.engine.exception.ChannelPlayerAlreadyBannedException import dev.m1sk9.lunaticChat.engine.exception.ChannelPlayerNotBannedException +import dev.m1sk9.lunaticChat.paper.StoppableService import dev.m1sk9.lunaticChat.paper.config.key.ChannelChatFeatureConfig import java.util.UUID import java.util.concurrent.ConcurrentHashMap @@ -24,7 +25,7 @@ class ChannelManager( private val storage: ChannelStorage, private val logger: Logger, private val config: ChannelChatFeatureConfig, -) { +) : StoppableService { private val channelsCache = ConcurrentHashMap<String, Channel>() private val membersCache = ConcurrentHashMap<String, CopyOnWriteArrayList<ChannelMember>>() private val activeChannels = ConcurrentHashMap<UUID, String>() @@ -464,6 +465,8 @@ class ChannelManager( * Saves the current state of channels and members to storage synchronously. * Should only br called during server shutdown. */ + override fun stop() = saveToDisk() + fun saveToDisk() { storage.saveToDisk(snapshot()) } @@ -542,11 +545,18 @@ class ChannelManager( playerId: UUID, channelId: String?, ) { - if (channelId == null) { - activeChannels.remove(playerId) - } else { - activeChannels[playerId] = channelId - } + // Returning early when nothing moved matters on the quit path, which clears the active + // channel for every player whether or not they had one: a snapshot copies all three caches + // and stringifies every active channel's UUID, and a mass disconnect would pay that once per + // player in a single tick. + val changed = + if (channelId == null) { + activeChannels.remove(playerId) != null + } else { + activeChannels.put(playerId, channelId) != channelId + } + if (!changed) return + saveToStorage() } } diff --git a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/chat/channel/ChannelMessageLogger.kt b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/chat/channel/ChannelMessageLogger.kt index a100d93..885f9ce 100644 --- a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/chat/channel/ChannelMessageLogger.kt +++ b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/chat/channel/ChannelMessageLogger.kt @@ -1,6 +1,7 @@ package dev.m1sk9.lunaticChat.paper.chat.channel import dev.m1sk9.lunaticChat.engine.chat.channel.ChannelMessageLogEntry +import dev.m1sk9.lunaticChat.paper.StoppableService import io.ktor.util.logging.Logger import io.papermc.paper.threadedregions.scheduler.ScheduledTask import kotlinx.serialization.encodeToString @@ -38,7 +39,7 @@ class ChannelMessageLogger( private val logger: Logger, private val maxFileSizeBytes: Long, private val retentionDays: Int, -) { +) : StoppableService { private val pendingEntries = ConcurrentLinkedQueue<ChannelMessageLogEntry>() private val json = Json { encodeDefaults = true } private var flushTask: ScheduledTask? = null @@ -161,7 +162,7 @@ class ChannelMessageLogger( * Shuts down the logger by cancelling scheduled tasks and flushing pending entries. * Should be called during plugin shutdown. */ - fun shutdown() { + override fun stop() { // Cancel scheduled tasks flushTask?.cancel() cleanupTask?.cancel() diff --git a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/chat/channel/ChannelStorage.kt b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/chat/channel/ChannelStorage.kt index 4fb18e8..fd18b9d 100644 --- a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/chat/channel/ChannelStorage.kt +++ b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/chat/channel/ChannelStorage.kt @@ -3,24 +3,18 @@ package dev.m1sk9.lunaticChat.paper.chat.channel import dev.m1sk9.lunaticChat.engine.chat.channel.ChannelData import dev.m1sk9.lunaticChat.engine.exception.ChannelStorageLoadException import dev.m1sk9.lunaticChat.engine.exception.ChannelStorageSaveException -import dev.m1sk9.lunaticChat.paper.DebouncedSaver -import dev.m1sk9.lunaticChat.paper.writeTextAtomically +import dev.m1sk9.lunaticChat.paper.storage.FileStore import kotlinx.serialization.json.Json -import java.nio.file.Path import java.util.logging.Logger -import kotlin.io.path.bufferedReader -import kotlin.io.path.exists /** * Manages the storage of channel data on disk. * - * @property channelsFile The path to the file where channel data is stored. - * @property saver Coalesces bursts of save requests into one asynchronous write. + * @property store The file channel data is read from and written to. * @property logger The logger for logging messages. */ class ChannelStorage( - private val channelsFile: Path, - private val saver: DebouncedSaver, + private val store: FileStore, private val logger: Logger, ) { private val json = @@ -36,22 +30,19 @@ class ChannelStorage( * @throws ChannelStorageLoadException if there is an error loading the data. */ fun loadFromDisk(): ChannelData { - if (!channelsFile.exists()) { - logger.warning("Channel storage not found, will create a new one.") - return ChannelData() - } + val jsonContent = + store.read() ?: run { + logger.warning("Channel storage not found, will create a new one.") + return ChannelData() + } return try { - val jsonContent = - channelsFile.bufferedReader().use { - it.readText() - } json.decodeFromString(ChannelData.serializer(), jsonContent).also { - logger.info("Successfully loaded channels from ${channelsFile.fileName}.") + logger.info("Successfully loaded channels from ${store.name}.") } } catch (e: Exception) { throw ChannelStorageLoadException( - "Failed to load channels from ${channelsFile.fileName}: ${e.message}", + "Failed to load channels from ${store.name}: ${e.message}", e, ) } @@ -65,12 +56,11 @@ class ChannelStorage( */ fun saveToDisk(data: ChannelData) { try { - val jsonContent = json.encodeToString(ChannelData.serializer(), data) - channelsFile.writeTextAtomically(jsonContent) - logger.fine("Successfully saved channels from ${channelsFile.fileName}.") + store.write(json.encodeToString(ChannelData.serializer(), data)) + logger.fine("Successfully saved channels from ${store.name}.") } catch (e: Exception) { throw ChannelStorageSaveException( - "Failed to save channels to ${channelsFile.fileName}: ${e.message}", + "Failed to save channels to ${store.name}: ${e.message}", e, ) } @@ -84,12 +74,6 @@ class ChannelStorage( * write instead of one of each per change. */ fun queueAsyncSave(data: () -> ChannelData) { - saver.request { - try { - saveToDisk(data()) - } catch (e: ChannelStorageSaveException) { - logger.severe("Error saving channel data asynchronously: ${e.message}") - } - } + store.queueWrite { json.encodeToString(ChannelData.serializer(), data()) } } } diff --git a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/converter/ConversionCache.kt b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/converter/ConversionCache.kt index cba62af..7be97d8 100644 --- a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/converter/ConversionCache.kt +++ b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/converter/ConversionCache.kt @@ -1,19 +1,17 @@ package dev.m1sk9.lunaticChat.paper.converter -import dev.m1sk9.lunaticChat.paper.writeTextAtomically +import dev.m1sk9.lunaticChat.paper.StoppableService +import dev.m1sk9.lunaticChat.paper.storage.FileStore import kotlinx.serialization.json.Json -import java.nio.file.Path import java.util.concurrent.ConcurrentHashMap import java.util.concurrent.atomic.AtomicBoolean import java.util.logging.Logger -import kotlin.io.path.bufferedReader -import kotlin.io.path.exists class ConversionCache( - private val cacheFile: Path, + private val store: FileStore, private val maxEntries: Int = 500, private val logger: Logger, -) { +) : StoppableService { private val conversionMemoryCache = ConcurrentHashMap<String, String>() private val dirty = AtomicBoolean(false) @@ -26,14 +24,14 @@ class ConversionCache( * If the cache file does not exist or version is incompatible, initializes it with an empty cache. */ fun loadFromDisk() { - if (!cacheFile.exists()) { - logger.info("Cache file not found, initializing new cache file at: $cacheFile") - initializeEmptyCache() - return - } + val jsonBuffer = + store.read() ?: run { + logger.info("Cache file not found, initializing new cache file at: ${store.name}") + initializeEmptyCache() + return + } try { - val jsonBuffer = cacheFile.bufferedReader().use { it.readText() } val cacheData = Json.decodeFromString<CacheData>(jsonBuffer) if (cacheData.version != CACHE_VERSION) { @@ -53,8 +51,7 @@ class ConversionCache( private fun initializeEmptyCache() { val emptyData = CacheData(version = CACHE_VERSION, entries = emptyMap()) - val jsonBuffer = Json.encodeToString(CacheData.serializer(), emptyData) - cacheFile.writeTextAtomically(jsonBuffer) + store.write(Json.encodeToString(CacheData.serializer(), emptyData)) } /** @@ -101,8 +98,7 @@ class ConversionCache( version = CACHE_VERSION, entries = conversionMemoryCache.toMap(), ) - val jsonBuffer = Json.encodeToString(CacheData.serializer(), data) - cacheFile.writeTextAtomically(jsonBuffer) + store.write(Json.encodeToString(CacheData.serializer(), data)) logger.info("Saved ${conversionMemoryCache.size} cache entries to disk.") } catch (e: Exception) { dirty.set(true) @@ -110,6 +106,9 @@ class ConversionCache( } } + /** Flushes on the shutdown path; the periodic task calls [saveToDisk] directly. */ + override fun stop() = saveToDisk() + // FIXME: ConcurrentHashMap keys are unordered, so evicting "oldest" entries // actually evicts random entries. Consider using LinkedHashMap with access-order // or implement proper LRU cache with timestamp tracking. diff --git a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/settings/PlayerSettingsManager.kt b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/settings/PlayerSettingsManager.kt index 110390d..2821f6f 100644 --- a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/settings/PlayerSettingsManager.kt +++ b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/settings/PlayerSettingsManager.kt @@ -2,8 +2,10 @@ package dev.m1sk9.lunaticChat.paper.settings import dev.m1sk9.lunaticChat.engine.settings.PlayerChatSettings import dev.m1sk9.lunaticChat.engine.settings.PlayerSettingsData +import dev.m1sk9.lunaticChat.paper.StoppableService import java.util.UUID import java.util.concurrent.ConcurrentHashMap +import java.util.concurrent.atomic.AtomicBoolean import java.util.logging.Logger /** @@ -16,8 +18,9 @@ import java.util.logging.Logger class PlayerSettingsManager( private val storage: YamlPlayerSettingsStorage, private val logger: Logger, -) { +) : StoppableService { private val settings = ConcurrentHashMap<UUID, PlayerChatSettings>() + private val dirty = AtomicBoolean(false) // Written back unchanged: nothing migrates on it yet, but rewriting the file must not // silently relabel a schema this build does not understand. @@ -61,17 +64,22 @@ class PlayerSettingsManager( */ fun updateSettings(settings: PlayerChatSettings) { this.settings[settings.uuid] = settings + dirty.set(true) storage.queueAsyncSave(::snapshot) logger.fine("Updated settings for player ${settings.uuid}") } /** - * Queues a debounced asynchronous save without changing any setting. + * Queues a debounced asynchronous save, unless nothing has changed since the last write. * * Used where the caller wants what is already in memory flushed soon - a player leaving, say - - * rather than paying for a write it does not need. + * rather than paying for a write it does not need. [updateSettings] is the only thing that + * changes a setting, and it queues its own save, so a quit almost always has nothing to persist: + * without the guard every quit re-serialized every player ever stored in the file to write bytes + * identical to the ones already there. */ fun queueSave() { + if (!dirty.get()) return storage.queueAsyncSave(::snapshot) } @@ -85,11 +93,17 @@ class PlayerSettingsManager( storage.saveToDisk(snapshot()) } - private fun snapshot(): PlayerSettingsData = - PlayerSettingsData( + override fun stop() = saveToDisk() + + private fun snapshot(): PlayerSettingsData { + // Cleared where the snapshot is taken rather than after the write: a change made while the + // write is in flight must leave the flag set so the next queueSave still fires. + dirty.set(false) + return PlayerSettingsData( version = schemaVersion, japaneseConversion = settings.mapValues { it.value.japaneseConversionEnabled }, directMessageNotification = settings.mapValues { it.value.directMessageNotificationEnabled }, channelMessageNotification = settings.mapValues { it.value.channelMessageNotificationEnabled }, ) + } } diff --git a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/settings/YamlPlayerSettingsStorage.kt b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/settings/YamlPlayerSettingsStorage.kt index 6e28f6f..86c1a94 100644 --- a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/settings/YamlPlayerSettingsStorage.kt +++ b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/settings/YamlPlayerSettingsStorage.kt @@ -2,24 +2,17 @@ package dev.m1sk9.lunaticChat.paper.settings import com.charleskorn.kaml.Yaml import dev.m1sk9.lunaticChat.engine.settings.PlayerSettingsData -import dev.m1sk9.lunaticChat.paper.DebouncedSaver -import dev.m1sk9.lunaticChat.paper.writeTextAtomically -import java.nio.file.Path +import dev.m1sk9.lunaticChat.paper.storage.FileStore import java.util.logging.Logger -import kotlin.io.path.bufferedReader -import kotlin.io.path.exists /** - * Handles YAML file I/O operations for player settings. - * Provides async save with debouncing. + * Handles YAML serialization for player settings. * - * @property settingsFile The path to the YAML settings file - * @property saver Coalesces bursts of save requests into one asynchronous write + * @property store The file settings are read from and written to * @property logger The logger for logging operations */ class YamlPlayerSettingsStorage( - private val settingsFile: Path, - private val saver: DebouncedSaver, + private val store: FileStore, private val logger: Logger, ) { private val yaml = Yaml.default @@ -31,13 +24,13 @@ class YamlPlayerSettingsStorage( * @return The loaded settings or empty settings if file doesn't exist */ fun loadFromDisk(): PlayerSettingsData { - if (!settingsFile.exists()) { - logger.info("Settings file not found, will create on first save") - return PlayerSettingsData() - } + val yamlContent = + store.read() ?: run { + logger.info("Settings file not found, will create on first save") + return PlayerSettingsData() + } return try { - val yamlContent = settingsFile.bufferedReader().use { it.readText() } yaml.decodeFromString(PlayerSettingsData.serializer(), yamlContent) } catch (e: Exception) { logger.severe("Failed to load settings file: ${e.message}") @@ -50,15 +43,11 @@ class YamlPlayerSettingsStorage( * Saves player settings to the YAML file synchronously. * This should only be called from async context or during shutdown. * - * A failed write leaves the previous file untouched: loading falls back to empty settings when - * the YAML does not parse, so a torn file would silently discard every player's settings. - * * @param data The settings data to save */ fun saveToDisk(data: PlayerSettingsData) { try { - val yamlContent = yaml.encodeToString(PlayerSettingsData.serializer(), data) - settingsFile.writeTextAtomically(yamlContent) + store.write(yaml.encodeToString(PlayerSettingsData.serializer(), data)) logger.fine("Saved player settings to disk") } catch (e: Exception) { logger.severe("Failed to save settings: ${e.message}") @@ -66,14 +55,13 @@ class YamlPlayerSettingsStorage( } /** - * Queues an async save operation with 5-second debouncing. - * Multiple save requests within 5 seconds are batched into a single save. + * Queues a debounced asynchronous save. * * @param data Supplies the settings to write. It is called when the write runs rather than * when it is queued, so the batched write persists every change made during the delay - not * just the one that started it. */ fun queueAsyncSave(data: () -> PlayerSettingsData) { - saver.request { saveToDisk(data()) } + store.queueWrite { yaml.encodeToString(PlayerSettingsData.serializer(), data()) } } } diff --git a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/storage/AsyncScheduler.kt b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/storage/AsyncScheduler.kt new file mode 100644 index 0000000..e458008 --- /dev/null +++ b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/storage/AsyncScheduler.kt @@ -0,0 +1,15 @@ +package dev.m1sk9.lunaticChat.paper.storage + +/** + * Runs a task off the tick thread after a delay. + * + * An interface rather than the Bukkit scheduler directly so that persistence can be exercised + * without a running server: the debounce is a rule about when writes happen, and asserting it + * through a mocked plugin proved nothing about the rule. + */ +fun interface AsyncScheduler { + fun runDelayed( + delaySeconds: Long, + task: () -> Unit, + ) +} diff --git a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/AtomicWrite.kt b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/storage/AtomicWrite.kt index c3cd28b..3029598 100644 --- a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/AtomicWrite.kt +++ b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/storage/AtomicWrite.kt @@ -1,4 +1,4 @@ -package dev.m1sk9.lunaticChat.paper +package dev.m1sk9.lunaticChat.paper.storage import java.nio.file.AtomicMoveNotSupportedException import java.nio.file.Files @@ -15,7 +15,7 @@ import kotlin.io.path.writeText * reason: a fixed sibling would only move the interleaving from the destination to the temporary * file, and the losing move would then fail with it already gone. */ -fun Path.writeTextAtomically(content: String) { +internal fun Path.writeTextAtomically(content: String) { val temporaryFile = Files.createTempFile(parent, fileName.toString(), ".tmp") try { temporaryFile.writeText(content) diff --git a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/DebouncedSaver.kt b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/storage/DebouncedSaver.kt index bc31785..5057575 100644 --- a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/DebouncedSaver.kt +++ b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/storage/DebouncedSaver.kt @@ -1,7 +1,5 @@ -package dev.m1sk9.lunaticChat.paper +package dev.m1sk9.lunaticChat.paper.storage -import org.bukkit.plugin.java.JavaPlugin -import java.util.concurrent.TimeUnit import java.util.concurrent.atomic.AtomicBoolean /** @@ -10,9 +8,13 @@ import java.util.concurrent.atomic.AtomicBoolean * The first [request] after an idle period schedules the write [delaySeconds] later; requests * arriving before it fires are absorbed by it, so a player toggling a setting repeatedly costs one * file write rather than one per toggle. + * + * A request arriving while a write is pending is dropped rather than queued, so one saver serves + * exactly one file - sharing it would silently lose the other file's save. [FileStore] owns one + * each so that rule cannot be broken by wiring. */ class DebouncedSaver( - private val plugin: JavaPlugin, + private val scheduler: AsyncScheduler, private val delaySeconds: Long = 5, ) { private val pending = AtomicBoolean(false) @@ -23,14 +25,9 @@ class DebouncedSaver( fun request(save: () -> Unit) { if (!pending.compareAndSet(false, true)) return - plugin.server.asyncScheduler.runDelayed( - plugin, - { - pending.set(false) - save() - }, - delaySeconds, - TimeUnit.SECONDS, - ) + scheduler.runDelayed(delaySeconds) { + pending.set(false) + save() + } } } diff --git a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/storage/FileStore.kt b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/storage/FileStore.kt new file mode 100644 index 0000000..f57cfde --- /dev/null +++ b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/storage/FileStore.kt @@ -0,0 +1,60 @@ +package dev.m1sk9.lunaticChat.paper.storage + +import java.nio.file.Path +import java.util.logging.Level +import java.util.logging.Logger +import kotlin.io.path.bufferedReader +import kotlin.io.path.exists + +/** + * One persisted file, written whole and written atomically. + * + * Durability lives here rather than at each write site: a file added later is written atomically + * because it is a [FileStore], not because its author remembered to reach for the right helper. + * + * Each store also owns its own [DebouncedSaver] rather than accepting one, because a saver drops a + * request while a write is pending and so serves exactly one file. That rule used to hold only + * because the wiring happened to construct a separate saver per file. + * + * Decoding is left to the caller: the stores differ in what an unreadable file means - channels fail + * loudly, settings fall back to empty - and that is a policy the file cannot know. + */ +class FileStore( + private val file: Path, + scheduler: AsyncScheduler, + private val logger: Logger, + debounceSeconds: Long = 5, +) { + private val saver = DebouncedSaver(scheduler, debounceSeconds) + + /** The file's name, for log messages that tell the operator which file went wrong. */ + val name: String get() = file.fileName.toString() + + /** Reads the file, or returns null when it is not there yet. */ + fun read(): String? { + if (!file.exists()) return null + return file.bufferedReader().use { it.readText() } + } + + /** Replaces the file with [contents] so nothing ever reads a half-written file. */ + fun write(contents: String) = file.writeTextAtomically(contents) + + /** + * Queues a debounced asynchronous write. + * + * A failure is reported rather than thrown: there is no caller left to hand it to by the time the + * write runs, and because the write is atomic the previous file is still intact, so the next save + * simply tries again. + * + * @param contents Supplies what to write. It is called when the write runs rather than when it is + * queued, so a burst of changes costs one snapshot and one file write instead of one of each. + */ + fun queueWrite(contents: () -> String) = + saver.request { + try { + write(contents()) + } catch (e: Exception) { + logger.log(Level.SEVERE, "Failed to save $name", e) + } + } +} diff --git a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/velocity/VelocityConnectionManager.kt b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/velocity/VelocityConnectionManager.kt index bdedb6f..85e2827 100644 --- a/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/velocity/VelocityConnectionManager.kt +++ b/platform-paper/src/main/kotlin/dev/m1sk9/lunaticChat/paper/velocity/VelocityConnectionManager.kt @@ -4,6 +4,7 @@ import dev.m1sk9.lunaticChat.engine.protocol.PluginMessage import dev.m1sk9.lunaticChat.engine.protocol.PluginMessageChannel import dev.m1sk9.lunaticChat.engine.protocol.PluginMessageCodec import dev.m1sk9.lunaticChat.engine.protocol.ProtocolVersion +import dev.m1sk9.lunaticChat.paper.StoppableService import org.bukkit.entity.Player import org.bukkit.plugin.Plugin import org.bukkit.plugin.messaging.PluginMessageListener @@ -20,7 +21,8 @@ class VelocityConnectionManager( private var crossServerChatManager: CrossServerChatManager? = null, private var crossServerDirectMessageManager: CrossServerDirectMessageManager? = null, private var remotePlayerRegistry: RemotePlayerRegistry? = null, -) : PluginMessageListener { +) : StoppableService, + PluginMessageListener { companion object { private val CHANNEL = PluginMessageChannel.ID private const val HANDSHAKE_TIMEOUT_SECONDS = 5L @@ -306,7 +308,7 @@ class VelocityConnectionManager( /** * Shutdown */ - fun shutdown() { + override fun stop() { plugin.server.messenger.unregisterOutgoingPluginChannel(plugin, CHANNEL) plugin.server.messenger.unregisterIncomingPluginChannel(plugin, CHANNEL) logger.info("Velocity integration channel unregistered") diff --git a/platform-paper/src/test/kotlin/dev/m1sk9/lunaticChat/paper/ServiceShutdownTest.kt b/platform-paper/src/test/kotlin/dev/m1sk9/lunaticChat/paper/ServiceShutdownTest.kt new file mode 100644 index 0000000..58c1e30 --- /dev/null +++ b/platform-paper/src/test/kotlin/dev/m1sk9/lunaticChat/paper/ServiceShutdownTest.kt @@ -0,0 +1,67 @@ +package dev.m1sk9.lunaticChat.paper + +import io.mockk.mockk +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +class ServiceShutdownTest { + private class RecordingService( + private val name: String, + private val stopped: MutableList<String>, + private val failing: Boolean = false, + ) : StoppableService { + override fun stop() { + stopped.add(name) + if (failing) error("$name could not be stopped") + } + } + + private fun shutdown(stoppables: List<StoppableService>): TestUtils.TestLogger { + val logger = TestUtils.TestLogger() + val initializer = ServiceInitializer(mockk(relaxed = true), TestUtils.createTestConfiguration(), lazy { mockk() }, logger) + initializer.shutdown( + ServiceContainer( + languageManager = mockk(), + playerSettingsManager = mockk(), + directMessageHandler = mockk(), + stoppables = stoppables, + ), + ) + return logger + } + + @Test + fun `every service is stopped, in the order it was registered`() { + val stopped = mutableListOf<String>() + + shutdown( + listOf( + RecordingService("settings", stopped), + RecordingService("cache", stopped), + RecordingService("velocity", stopped), + ), + ) + + assertEquals(listOf("settings", "cache", "velocity"), stopped) + } + + @Test + fun `a service that fails to stop is reported and does not skip the rest`() { + val stopped = mutableListOf<String>() + + // An exception escaping onDisable left the log flusher and the proxy connection to be torn + // down by the server instead of by us. + val logger = + shutdown( + listOf( + RecordingService("settings", stopped, failing = true), + RecordingService("logger", stopped), + RecordingService("velocity", stopped), + ), + ) + + assertEquals(listOf("settings", "logger", "velocity"), stopped) + assertTrue(logger.severeMessages.any { it.contains("Failed to stop") }) + } +} diff --git a/platform-paper/src/test/kotlin/dev/m1sk9/lunaticChat/paper/TestUtils.kt b/platform-paper/src/test/kotlin/dev/m1sk9/lunaticChat/paper/TestUtils.kt index 4bca21f..420391f 100644 --- a/platform-paper/src/test/kotlin/dev/m1sk9/lunaticChat/paper/TestUtils.kt +++ b/platform-paper/src/test/kotlin/dev/m1sk9/lunaticChat/paper/TestUtils.kt @@ -13,6 +13,7 @@ import dev.m1sk9.lunaticChat.paper.config.key.MessageFormatConfig import dev.m1sk9.lunaticChat.paper.config.key.QuickRepliesFeatureConfig import dev.m1sk9.lunaticChat.paper.config.key.VelocityIntegrationConfig import dev.m1sk9.lunaticChat.paper.i18n.Language +import dev.m1sk9.lunaticChat.paper.storage.AsyncScheduler import io.mockk.mockk import org.bukkit.entity.Player import org.bukkit.plugin.java.JavaPlugin @@ -222,4 +223,30 @@ object TestUtils { throw AssertionError("Expected list to contain at least one matching item") } } + + /** + * An [AsyncScheduler] that holds the work until a test releases it. + * + * Lets a test assert what the debounce actually does - that the snapshot is taken when the write + * runs, not when it was queued - which mocking the saver could only assume. + */ + class ManualScheduler : AsyncScheduler { + private val pending = mutableListOf<() -> Unit>() + + val pendingCount: Int get() = pending.size + + override fun runDelayed( + delaySeconds: Long, + task: () -> Unit, + ) { + pending.add(task) + } + + /** Runs everything scheduled so far, as the server's async scheduler eventually would. */ + fun runPending() { + val due = pending.toList() + pending.clear() + due.forEach { it() } + } + } } diff --git a/platform-paper/src/test/kotlin/dev/m1sk9/lunaticChat/paper/chat/channel/ChannelStorageTest.kt b/platform-paper/src/test/kotlin/dev/m1sk9/lunaticChat/paper/chat/channel/ChannelStorageTest.kt index 2211eef..d35f9c8 100644 --- a/platform-paper/src/test/kotlin/dev/m1sk9/lunaticChat/paper/chat/channel/ChannelStorageTest.kt +++ b/platform-paper/src/test/kotlin/dev/m1sk9/lunaticChat/paper/chat/channel/ChannelStorageTest.kt @@ -5,12 +5,9 @@ import dev.m1sk9.lunaticChat.engine.chat.channel.ChannelData import dev.m1sk9.lunaticChat.engine.chat.channel.ChannelMember import dev.m1sk9.lunaticChat.engine.chat.channel.ChannelRole import dev.m1sk9.lunaticChat.engine.exception.ChannelStorageLoadException -import dev.m1sk9.lunaticChat.paper.DebouncedSaver import dev.m1sk9.lunaticChat.paper.TestUtils -import io.mockk.mockk -import io.mockk.slot -import io.mockk.verify -import java.nio.file.Files +import dev.m1sk9.lunaticChat.paper.storage.FileStore +import org.junit.jupiter.api.io.TempDir import java.nio.file.Path import java.util.UUID import kotlin.io.path.listDirectoryEntries @@ -42,19 +39,19 @@ class ChannelStorageTest { activeChannels = mapOf(owner.toString() to "general"), ) - private fun withStorage(block: (ChannelStorage, Path) -> Unit) { - val directory = Files.createTempDirectory("channel-storage-test") - try { - val channelsFile = directory.resolve("channels.json") - block(ChannelStorage(channelsFile, mockk(relaxed = true), TestUtils.TestLogger()), channelsFile) - } finally { - directory.toFile().deleteRecursively() - } + @TempDir + lateinit var directory: Path + + private fun withStorage(block: (ChannelStorage, Path, TestUtils.ManualScheduler) -> Unit) { + val channelsFile = directory.resolve("channels.json") + val scheduler = TestUtils.ManualScheduler() + val store = FileStore(channelsFile, scheduler, TestUtils.TestLogger()) + block(ChannelStorage(store, TestUtils.TestLogger()), channelsFile, scheduler) } @Test fun `saved data is loaded back unchanged`() = - withStorage { storage, _ -> + withStorage { storage, _, _ -> val data = sampleData() storage.saveToDisk(data) @@ -64,7 +61,7 @@ class ChannelStorageTest { @Test fun `saving leaves no temporary file beside the channel file`() = - withStorage { storage, channelsFile -> + withStorage { storage, channelsFile, _ -> storage.saveToDisk(sampleData()) assertEquals(listOf(channelsFile), channelsFile.parent.listDirectoryEntries()) @@ -72,13 +69,13 @@ class ChannelStorageTest { @Test fun `loading a missing file yields empty data rather than failing`() = - withStorage { storage, _ -> + withStorage { storage, _, _ -> assertEquals(ChannelData(), storage.loadFromDisk()) } @Test fun `loading an unparseable file fails loudly`() = - withStorage { storage, channelsFile -> + withStorage { storage, channelsFile, _ -> channelsFile.writeText("{ this is not json") assertFailsWith<ChannelStorageLoadException> { storage.loadFromDisk() } @@ -86,20 +83,15 @@ class ChannelStorageTest { @Test fun `unknown fields in the file are ignored`() = - withStorage { storage, channelsFile -> + withStorage { storage, channelsFile, _ -> channelsFile.writeText("""{"version":1,"channels":{},"members":{},"activeChannels":{},"future":true}""") assertEquals(ChannelData(), storage.loadFromDisk()) } @Test - fun `a queued save reads the data when the write runs, not when it is queued`() { - val directory = Files.createTempDirectory("channel-storage-test") - try { - val channelsFile = directory.resolve("channels.json") - val saver = mockk<DebouncedSaver>(relaxed = true) - val storage = ChannelStorage(channelsFile, saver, TestUtils.TestLogger()) - val queued = slot<() -> Unit>() + fun `a queued save reads the data when the write runs, not when it is queued`() = + withStorage { storage, _, scheduler -> var supplied = false storage.queueAsyncSave { @@ -107,15 +99,12 @@ class ChannelStorageTest { sampleData() } - verify { saver.request(capture(queued)) } + assertEquals(1, scheduler.pendingCount) assertTrue(!supplied, "the snapshot must not be taken while queueing") - queued.captured.invoke() + scheduler.runPending() assertTrue(supplied) assertEquals(sampleData(), storage.loadFromDisk()) - } finally { - directory.toFile().deleteRecursively() } - } } diff --git a/platform-paper/src/test/kotlin/dev/m1sk9/lunaticChat/paper/converter/ConversionCacheTest.kt b/platform-paper/src/test/kotlin/dev/m1sk9/lunaticChat/paper/converter/ConversionCacheTest.kt index b76c050..5041550 100644 --- a/platform-paper/src/test/kotlin/dev/m1sk9/lunaticChat/paper/converter/ConversionCacheTest.kt +++ b/platform-paper/src/test/kotlin/dev/m1sk9/lunaticChat/paper/converter/ConversionCacheTest.kt @@ -1,6 +1,8 @@ package dev.m1sk9.lunaticChat.paper.converter import dev.m1sk9.lunaticChat.paper.TestUtils +import dev.m1sk9.lunaticChat.paper.storage.FileStore +import org.junit.jupiter.api.io.TempDir import java.nio.file.Files import java.nio.file.Path import kotlin.io.path.listDirectoryEntries @@ -13,19 +15,19 @@ import kotlin.test.assertNull import kotlin.test.assertTrue class ConversionCacheTest { - private fun withCacheFile(block: (Path) -> Unit) { - val directory = Files.createTempDirectory("conversion-cache-test") - try { - block(directory.resolve("cache.json")) - } finally { - directory.toFile().deleteRecursively() - } - } + @TempDir + lateinit var directory: Path + + private fun withCacheFile(block: (Path) -> Unit) = block(directory.resolve("cache.json")) private fun createCache( cacheFile: Path, maxEntries: Int = 500, - ) = ConversionCache(cacheFile, maxEntries, TestUtils.TestLogger()) + ) = ConversionCache( + FileStore(cacheFile, TestUtils.ManualScheduler(), TestUtils.TestLogger()), + maxEntries, + TestUtils.TestLogger(), + ) @Test fun `entries survive a save and reload`() = diff --git a/platform-paper/src/test/kotlin/dev/m1sk9/lunaticChat/paper/settings/PlayerSettingsManagerTest.kt b/platform-paper/src/test/kotlin/dev/m1sk9/lunaticChat/paper/settings/PlayerSettingsManagerTest.kt index 3c34295..db59cf0 100644 --- a/platform-paper/src/test/kotlin/dev/m1sk9/lunaticChat/paper/settings/PlayerSettingsManagerTest.kt +++ b/platform-paper/src/test/kotlin/dev/m1sk9/lunaticChat/paper/settings/PlayerSettingsManagerTest.kt @@ -4,6 +4,7 @@ import dev.m1sk9.lunaticChat.engine.settings.PlayerChatSettings import dev.m1sk9.lunaticChat.engine.settings.PlayerSettingsData import dev.m1sk9.lunaticChat.paper.TestUtils import dev.m1sk9.lunaticChat.paper.TestUtils.createTestUUID +import io.mockk.clearMocks import io.mockk.every import io.mockk.mockk import io.mockk.slot @@ -157,6 +158,8 @@ class PlayerSettingsManagerTest { fun `queueSave should not write on the calling thread`() { val (manager, storage, _) = createManager() manager.initialize() + manager.updateSettings(PlayerChatSettings(uuid = createTestUUID(1), japaneseConversionEnabled = false)) + clearMocks(storage, answers = false) manager.queueSave() @@ -165,6 +168,19 @@ class PlayerSettingsManagerTest { } @Test + fun `queueSave should not write when nothing has changed`() { + val (manager, storage, _) = createManager() + manager.initialize() + clearMocks(storage, answers = false) + + manager.queueSave() + + // updateSettings is the only thing that changes a setting and it queues its own save, so a + // quit would otherwise re-serialize every stored player to write identical bytes. + verify(exactly = 0) { storage.queueAsyncSave(any()) } + } + + @Test fun `multiple players should have independent settings`() { val (manager, _, _) = createManager() manager.initialize() diff --git a/platform-paper/src/test/kotlin/dev/m1sk9/lunaticChat/paper/AtomicWriteTest.kt b/platform-paper/src/test/kotlin/dev/m1sk9/lunaticChat/paper/storage/AtomicWriteTest.kt index ea0c35f..d8d8a99 100644 --- a/platform-paper/src/test/kotlin/dev/m1sk9/lunaticChat/paper/AtomicWriteTest.kt +++ b/platform-paper/src/test/kotlin/dev/m1sk9/lunaticChat/paper/storage/AtomicWriteTest.kt @@ -1,10 +1,9 @@ -package dev.m1sk9.lunaticChat.paper +package dev.m1sk9.lunaticChat.paper.storage -import java.nio.file.Files +import org.junit.jupiter.api.io.TempDir import java.nio.file.Path import java.util.concurrent.ConcurrentLinkedQueue import java.util.concurrent.CyclicBarrier -import kotlin.io.path.exists import kotlin.io.path.listDirectoryEntries import kotlin.io.path.readText import kotlin.io.path.writeText @@ -14,14 +13,10 @@ import kotlin.test.assertEquals import kotlin.test.assertTrue class AtomicWriteTest { - private fun withTemporaryDirectory(block: (Path) -> Unit) { - val directory = Files.createTempDirectory("atomic-write-test") - try { - block(directory) - } finally { - directory.toFile().deleteRecursively() - } - } + @TempDir + lateinit var temporaryDirectory: Path + + private fun withTemporaryDirectory(block: (Path) -> Unit) = block(temporaryDirectory) @Test fun `writes the content and leaves no temporary file behind`() = @@ -71,6 +66,5 @@ class AtomicWriteTest { assertTrue(failures.isEmpty(), "writes failed: ${failures.map { it.toString() }}") assertContains(contents, target.readText()) assertEquals(listOf(target), directory.listDirectoryEntries()) - assertTrue(target.exists()) } } diff --git a/platform-paper/src/test/kotlin/dev/m1sk9/lunaticChat/paper/storage/DebouncedSaverTest.kt b/platform-paper/src/test/kotlin/dev/m1sk9/lunaticChat/paper/storage/DebouncedSaverTest.kt new file mode 100644 index 0000000..01814ed --- /dev/null +++ b/platform-paper/src/test/kotlin/dev/m1sk9/lunaticChat/paper/storage/DebouncedSaverTest.kt @@ -0,0 +1,61 @@ +package dev.m1sk9.lunaticChat.paper.storage + +import dev.m1sk9.lunaticChat.paper.TestUtils +import kotlin.test.Test +import kotlin.test.assertEquals + +class DebouncedSaverTest { + @Test + fun `a burst of requests costs one write`() { + val scheduler = TestUtils.ManualScheduler() + val saver = DebouncedSaver(scheduler) + var writes = 0 + + repeat(10) { saver.request { writes++ } } + scheduler.runPending() + + assertEquals(1, writes) + } + + @Test + fun `the write runs at the scheduled time, not when it was requested`() { + val scheduler = TestUtils.ManualScheduler() + val saver = DebouncedSaver(scheduler) + var writes = 0 + + saver.request { writes++ } + + assertEquals(0, writes) + scheduler.runPending() + assertEquals(1, writes) + } + + @Test + fun `a request after the write lands is scheduled again`() { + val scheduler = TestUtils.ManualScheduler() + val saver = DebouncedSaver(scheduler) + var writes = 0 + + saver.request { writes++ } + scheduler.runPending() + saver.request { writes++ } + scheduler.runPending() + + assertEquals(2, writes) + } + + @Test + fun `a saver serves one file only`() { + val scheduler = TestUtils.ManualScheduler() + val saver = DebouncedSaver(scheduler) + val written = mutableListOf<String>() + + // Why FileStore owns one saver each: a request arriving while a write is pending is dropped, + // so a shared saver would silently lose the second file's save. + saver.request { written.add("channels.json") } + saver.request { written.add("settings.yml") } + scheduler.runPending() + + assertEquals(listOf("channels.json"), written) + } +} |
