From 647232c44753318622500c429579bc23f237b4ee Mon Sep 17 00:00:00 2001 From: yyc12345 Date: Sun, 4 Oct 2026 16:11:51 +0800 Subject: [PATCH] fix(storage): initialize a workspace under the lock and refuse a missing database --- .../Workspace/WorkspaceSessionTests.cs | 36 +++++++++++++++ YKanBan/Services/AppServices.cs | 24 ++++++++++ YKanBan/Storage/StorageExceptions.cs | 20 ++++++++ .../Storage/Workspace/WorkspaceInitializer.cs | 6 +++ YKanBan/Storage/Workspace/WorkspaceSession.cs | 46 +++++++++++++++++++ YKanBan/ViewModels/MainWindowViewModel.cs | 15 +++--- 6 files changed, 141 insertions(+), 6 deletions(-) diff --git a/YKanBan.Tests/Storage/Workspace/WorkspaceSessionTests.cs b/YKanBan.Tests/Storage/Workspace/WorkspaceSessionTests.cs index e24c618..e362c34 100644 --- a/YKanBan.Tests/Storage/Workspace/WorkspaceSessionTests.cs +++ b/YKanBan.Tests/Storage/Workspace/WorkspaceSessionTests.cs @@ -41,6 +41,42 @@ public class WorkspaceSessionTests { () => WorkspaceSession.Open(directory.FullPath)); } + [TestMethod] + public void OpenWithoutDatabaseThrowsDatabaseMissing() { + using var directory = new TempDirectory(); + WorkspaceInitializer.Initialize(directory.FullPath); + File.Delete(WorkspacePaths.Database(directory.FullPath)); + + Assert.ThrowsExactly( + () => WorkspaceSession.Open(directory.FullPath)); + } + + [TestMethod] + public void InitializeHoldsLockAndSeedsPreset() { + using var directory = new TempDirectory(); + + using WorkspaceSession session = WorkspaceSession.Initialize(directory.FullPath); + + // The lock is held for the whole session, so a second initialize cannot interleave. + Assert.ThrowsExactly(() => WorkspaceSession.Initialize(directory.FullPath)); + + // The preset columns and tags were written under the same lock. + Assert.AreEqual(3, session.Repository.GetColumns().Count); + Assert.AreEqual(8, session.Repository.GetTags().Count); + } + + [TestMethod] + public void InitializeExistingWorkspaceThrowsAndReleasesLock() { + using var directory = new TempDirectory(); + WorkspaceInitializer.Initialize(directory.FullPath); + + Assert.ThrowsExactly(() => WorkspaceSession.Initialize(directory.FullPath)); + + // The failed initialize released the lock, so the workspace can still be opened. + using WorkspaceSession session = WorkspaceSession.Open(directory.FullPath); + Assert.AreEqual(directory.FullPath, session.FolderPath); + } + [TestMethod] public void RepositoryIsUsableThroughSession() { using var directory = new TempDirectory(); diff --git a/YKanBan/Services/AppServices.cs b/YKanBan/Services/AppServices.cs index e9c1bb2..e998ab4 100644 --- a/YKanBan/Services/AppServices.cs +++ b/YKanBan/Services/AppServices.cs @@ -45,9 +45,33 @@ public sealed class AppServices : IDisposable { return Session; } + /// + /// Initializes a fresh workspace and opens it, holding the lock throughout. + /// + /// The folder to initialize. + /// The open session. + /// The folder does not exist on disk. + /// Another instance already holds the lock. + public WorkspaceSession InitializeWorkspace(string folderPath) { + Session = WorkspaceSession.Initialize(folderPath); + return Session; + } + + /// + /// Releases the workspace session, if any, and forgets it. + /// + public void CloseWorkspace() { + Session?.Dispose(); + Session = null; + } + /// /// Persists the configuration; called once at shutdown. /// + // YYC MARK: the whole app.json is rewritten unconditionally at shutdown. + // Running several instances would make the last one to exit overwrite every + // field with its own in-memory state. Multi-instance use is not supported, + // so this last-writer-wins behavior is accepted. public void SaveConfiguration() => ConfigStore.Save(Config); /// diff --git a/YKanBan/Storage/StorageExceptions.cs b/YKanBan/Storage/StorageExceptions.cs index bd53e05..aab1f8d 100644 --- a/YKanBan/Storage/StorageExceptions.cs +++ b/YKanBan/Storage/StorageExceptions.cs @@ -55,6 +55,26 @@ public sealed class WorkspaceNotInitializedException : YKanBanException { } } +/// +/// Signals that .ykanban exists but its database file does not, so the +/// workspace data is missing; a new empty database is never created silently. +/// +public sealed class WorkspaceDatabaseMissingException : YKanBanException { + /// + /// Gets the database file that was expected to exist. + /// + public string DatabasePath { get; } + + /// + /// Initializes the exception for a missing workspace database. + /// + /// The missing database file path. + public WorkspaceDatabaseMissingException(string databasePath) + : base($"The workspace database is missing: {databasePath}") { + DatabasePath = databasePath; + } +} + /// /// Signals that another instance already holds the workspace lock, so this /// process must not touch the workspace. diff --git a/YKanBan/Storage/Workspace/WorkspaceInitializer.cs b/YKanBan/Storage/Workspace/WorkspaceInitializer.cs index 80534c4..7bf532e 100644 --- a/YKanBan/Storage/Workspace/WorkspaceInitializer.cs +++ b/YKanBan/Storage/Workspace/WorkspaceInitializer.cs @@ -25,6 +25,12 @@ public static class WorkspaceInitializer { /// The folder does not exist on disk. /// The workspace database already exists. public static void Initialize(string folderPath) { + // YYC MARK: this class is deliberately a lock-free, functional unit; it + // does not acquire the workspace lock. Ensuring the lock is held before + // initialization is the caller's responsibility (see + // WorkspaceSession.Initialize). The schema migration and the later preset + // writes are also not one transaction, so a crash mid-initialization can + // leave a partially built database; that is accepted here. if (!Directory.Exists(folderPath)) { throw new WorkspaceDirectoryMissingException(folderPath); } diff --git a/YKanBan/Storage/Workspace/WorkspaceSession.cs b/YKanBan/Storage/Workspace/WorkspaceSession.cs index 1a6cc70..6bf034c 100644 --- a/YKanBan/Storage/Workspace/WorkspaceSession.cs +++ b/YKanBan/Storage/Workspace/WorkspaceSession.cs @@ -38,9 +38,55 @@ public sealed class WorkspaceSession : IDisposable { /// The workspace folder to open. /// The opened session; dispose it to release the lock and connection. /// The folder has no .ykanban structure. + /// .ykanban exists but its database file does not. /// Another instance already holds the lock. public static WorkspaceSession Open(string folderPath) { WorkspaceLock workspaceLock = WorkspaceLock.Acquire(folderPath); + + // Opening would otherwise create an empty database and show an empty board as if nothing was lost. + string databasePath = WorkspacePaths.Database(folderPath); + if (!File.Exists(databasePath)) { + workspaceLock.Dispose(); + throw new WorkspaceDatabaseMissingException(databasePath); + } + + return OpenLocked(folderPath, workspaceLock); + } + + /// + /// Initializes a workspace with its preset columns and tags, then opens it. + /// The lock is taken right after .ykanban is created and is never released in + /// between, so no other instance can interleave with the initialization. + /// + /// The existing folder to initialize. + /// The opened session; dispose it to release the lock and connection. + /// The folder does not exist on disk. + /// Another instance holds the workspace lock. + /// The workspace database already exists. + public static WorkspaceSession Initialize(string folderPath) { + // .ykanban first: the lock file lives inside it. Taking the lock here, + // before any database work, keeps two instances from initializing the same + // folder at once; WorkspaceInitializer stays a lock-free functional class. + Directory.CreateDirectory(WorkspacePaths.Root(folderPath)); + WorkspaceLock workspaceLock = WorkspaceLock.Acquire(folderPath); + try { + WorkspaceInitializer.Initialize(folderPath); + WorkspacePreset.AddPresetColumns(folderPath); + WorkspacePreset.AddPresetTags(folderPath); + return OpenLocked(folderPath, workspaceLock); + } catch { + workspaceLock.Dispose(); + throw; + } + } + + /// + /// Opens the repository under an already held lock; the lock is released if that fails. + /// + /// The workspace folder. + /// The held workspace lock. + /// The opened session. + private static WorkspaceSession OpenLocked(string folderPath, WorkspaceLock workspaceLock) { try { var repository = new WorkspaceRepository(folderPath); return new WorkspaceSession(folderPath, workspaceLock, repository); diff --git a/YKanBan/ViewModels/MainWindowViewModel.cs b/YKanBan/ViewModels/MainWindowViewModel.cs index ab4db4d..1d352c4 100644 --- a/YKanBan/ViewModels/MainWindowViewModel.cs +++ b/YKanBan/ViewModels/MainWindowViewModel.cs @@ -87,8 +87,14 @@ public sealed partial class MainWindowViewModel : ViewModelBase { /// Opens the workspace and switches the content to the normal view. /// /// The workspace folder to open. - private void OpenWorkspace(string folderPath) { - WorkspaceSession session = _services.OpenWorkspace(folderPath); + private void OpenWorkspace(string folderPath) => OpenWorkspace(folderPath, _services.OpenWorkspace(folderPath)); + + /// + /// Switches the content to the normal view of an already opened workspace. + /// + /// The workspace folder. + /// The opened workspace session. + private void OpenWorkspace(string folderPath, WorkspaceSession session) { Content = new WorkspaceViewModel(_services, session, ShowSettings, ShowAbout); Title = $"{new DirectoryInfo(folderPath).Name} - {Resources.App_Name}"; } @@ -104,10 +110,7 @@ public sealed partial class MainWindowViewModel : ViewModelBase { } try { - WorkspaceInitializer.Initialize(page.FolderPath); - WorkspacePreset.AddPresetColumns(page.FolderPath); - WorkspacePreset.AddPresetTags(page.FolderPath); - OpenWorkspace(page.FolderPath); + OpenWorkspace(page.FolderPath, _services.InitializeWorkspace(page.FolderPath)); } catch (Exception exception) when (exception is YKanBanException or IOException or UnauthorizedAccessException) { // Initialization failed; the page remains so the user can retry. Debug.WriteLine(exception);