fix(storage): initialize workspaces under the lock in one transaction

Follows PLAN 4.2: create .ykanban, take the lock, then build the database.
SqliteDatabase.Create writes every migration and the preset columns in a
single transaction and deletes the file again on failure, so no empty
version-0 database is left behind. WorkspaceSession.Initialize keeps the lock
into the session instead of releasing and re-acquiring it.

A lock conflict while initializing shows the lock-conflict page.

USAGE.md

@@ -61,8 +61,8 @@ instead:
 | Invalid arguments | No argument, more than one argument, an empty or whitespace-only argument, an argument starting with `-` (including `--help`), or a path the operating system cannot represent (for example one containing a NUL character). Shows the usage line. |
 | Folder not found | `<path>` does not exist or is not a folder. |
 | No workspace here | `<path>` exists but has no `.ykanban`. The **Initialize workspace** button creates one, with the columns To Do / In Progress / Done (named in the current interface language). |
-| Workspace locked | Another YKanBan window already has this workspace open. Close that window first. |
+| Workspace locked | Another YKanBan window already has this workspace open. Close that window first. Also shown when another window takes the lock while you initialize. |
-| Cannot open workspace | The database was created by a newer YKanBan, is damaged or cannot be opened, or a file-system or permission error occurred (also when initializing fails). The English error details can be selected and copied. |
+| Cannot open workspace | The database was created by a newer YKanBan, is damaged or cannot be opened, or a file-system or permission error occurred (also when initializing fails for a reason other than the lock). The English error details can be selected and copied. |
 
 The window title is `<workspace name> - YKanBan` while a workspace is open and
 just `YKanBan` on these pages.

YKanBan.Tests/Storage/SqliteDatabaseTests.cs

@@ -80,4 +80,48 @@ public class SqliteDatabaseTests
 
         Assert.ThrowsExactly<ArgumentException>(() => SqliteDatabase.Open(databasePath, broken));
     }
+
+    [TestMethod]
+    public void CreateBuildsSchemaAndSeedTogether()
+    {
+        using var directory = new TempDirectory();
+        string databasePath = Path.Combine(directory.FullPath, "create.db");
+
+        using SqliteConnection connection = SqliteDatabase.Create(databasePath, WorkspaceSchema.Migrations,
+            (seedConnection, transaction) =>
+            {
+                using SqliteCommand command = seedConnection.CreateCommand();
+                command.Transaction = transaction;
+                command.CommandText = "INSERT INTO tags (name, color) VALUES ('seed', '#000000');";
+                command.ExecuteNonQuery();
+            });
+        Assert.AreEqual(WorkspaceSchema.CurrentVersion, SqliteDatabase.ReadUserVersion(connection));
+        Assert.AreEqual(1L, SqliteTestHelper.ScalarLong(connection, "SELECT COUNT(*) FROM tags;"));
+        Assert.AreEqual("wal", SqliteTestHelper.ScalarString(connection, "PRAGMA journal_mode;"));
+        Assert.AreEqual(1L, SqliteTestHelper.ScalarLong(connection, "PRAGMA foreign_keys;"));
+    }
+
+    [TestMethod]
+    public void CreateWithFailingSeedLeavesNoDatabaseBehind()
+    {
+        using var directory = new TempDirectory();
+        string databasePath = Path.Combine(directory.FullPath, "failed.db");
+
+        Assert.ThrowsExactly<InvalidOperationException>(() => SqliteDatabase.Create(
+            databasePath, WorkspaceSchema.Migrations, (_, _) => throw new InvalidOperationException("seed failed")));
+        Assert.IsFalse(File.Exists(databasePath));
+        Assert.IsFalse(File.Exists(databasePath + "-wal"));
+        Assert.IsFalse(File.Exists(databasePath + "-shm"));
+    }
+
+    [TestMethod]
+    public void CreateRefusesAnExistingDatabaseAndKeepsIt()
+    {
+        using var directory = new TempDirectory();
+        string databasePath = Path.Combine(directory.FullPath, "existing.db");
+        SqliteDatabase.Create(databasePath, WorkspaceSchema.Migrations).Dispose();
+
+        Assert.ThrowsExactly<InvalidOperationException>(() => SqliteDatabase.Create(databasePath, WorkspaceSchema.Migrations));
+        Assert.IsTrue(File.Exists(databasePath));
+    }
 }

YKanBan.Tests/Storage/Workspace/WorkspacePresetTests.cs

@@ -29,8 +29,7 @@ public class WorkspacePresetTests
             ];
 
             using var directory = new TempDirectory();
-            WorkspaceInitializer.Initialize(directory.FullPath);
+            WorkspaceSession.Initialize(directory.FullPath).Dispose();
-            WorkspacePreset.AddPresetColumns(directory.FullPath);
 
             using SqliteConnection connection = SqliteTestHelper.OpenWorkspace(directory.FullPath);
             CollectionAssert.AreEqual(expected, SqliteTestHelper.GetColumnTitles(connection));
@@ -56,8 +55,7 @@ public class WorkspacePresetTests
             ];
 
             using var directory = new TempDirectory();
-            WorkspaceInitializer.Initialize(directory.FullPath);
+            WorkspaceSession.Initialize(directory.FullPath).Dispose();
-            WorkspacePreset.AddPresetColumns(directory.FullPath);
 
             using SqliteConnection connection = SqliteTestHelper.OpenWorkspace(directory.FullPath);
             CollectionAssert.AreEqual(expected, SqliteTestHelper.GetColumnTitles(connection));
@@ -83,8 +81,7 @@ public class WorkspacePresetTests
             ];
 
             using var directory = new TempDirectory();
-            WorkspaceInitializer.Initialize(directory.FullPath);
+            WorkspaceSession.Initialize(directory.FullPath).Dispose();
-            WorkspacePreset.AddPresetColumns(directory.FullPath);
 
             // Switching the language after adding the presets must not rewrite stored data.
             Resources.Culture = new CultureInfo("zh-Hans");

YKanBan.Tests/Storage/Workspace/WorkspaceSessionTests.cs

@@ -57,4 +57,36 @@ public class WorkspaceSessionTests
         Assert.AreEqual("bug", session.Repository.GetTags().Single().Name);
         Assert.AreEqual(tag.Id, session.Repository.GetTags().Single().Id);
     }
+
+    [TestMethod]
+    public void InitializeAddsPresetColumnsAndHoldsTheLock()
+    {
+        using var directory = new TempDirectory();
+
+        using WorkspaceSession session = WorkspaceSession.Initialize(directory.FullPath);
+        Assert.AreEqual(3, session.Repository.GetColumns().Count);
+        Assert.ThrowsExactly<WorkspaceLockException>(() => WorkspaceSession.Open(directory.FullPath));
+    }
+
+    [TestMethod]
+    public void InitializeWhileAnotherInstanceHoldsTheLockThrowsLockConflict()
+    {
+        using var directory = new TempDirectory();
+        Directory.CreateDirectory(WorkspacePaths.Root(directory.FullPath));
+        using WorkspaceLock held = WorkspaceLock.Acquire(directory.FullPath);
+
+        Assert.ThrowsExactly<WorkspaceLockException>(() => WorkspaceSession.Initialize(directory.FullPath));
+        Assert.IsFalse(File.Exists(WorkspacePaths.Database(directory.FullPath)));
+    }
+
+    [TestMethod]
+    public void FailedInitializeReleasesTheLock()
+    {
+        using var directory = new TempDirectory();
+        WorkspaceInitializer.Initialize(directory.FullPath);
+
+        // The database already exists, so initialization fails after taking the lock.
+        Assert.ThrowsExactly<InvalidOperationException>(() => WorkspaceSession.Initialize(directory.FullPath));
+        using WorkspaceSession reopened = WorkspaceSession.Open(directory.FullPath);
+    }
 }

YKanBan/Services/AppServices.cs

@@ -48,6 +48,20 @@ public sealed class AppServices : IDisposable
         return Session;
     }
 
+    /// <summary>
+    /// Initializes a workspace with its preset columns and opens it, holding
+    /// the lock from the creation of .ykanban onwards.
+    /// </summary>
+    /// <param name="folderPath">The existing folder to initialize.</param>
+    /// <returns>The open session.</returns>
+    /// <exception cref="YKanBan.Storage.WorkspaceDirectoryMissingException">The folder does not exist on disk.</exception>
+    /// <exception cref="YKanBan.Storage.WorkspaceLockException">Another instance already holds the lock.</exception>
+    public WorkspaceSession InitializeWorkspace(string folderPath)
+    {
+        Session = WorkspaceSession.Initialize(folderPath);
+        return Session;
+    }
+
     /// <summary>
     /// Persists the configuration when this session changed a setting; called once at shutdown.
     /// </summary>

YKanBan/Storage/SqliteDatabase.cs

@@ -33,16 +33,7 @@ public static class SqliteDatabase
     /// <returns>An open connection with all PRAGMAs applied and the schema migrated.</returns>
     public static SqliteConnection Open(string databasePath, IReadOnlyList<SchemaMigration> migrations)
     {
-        var connection = new SqliteConnection(new SqliteConnectionStringBuilder
+        SqliteConnection connection = OpenConnection(databasePath);
-        {
-            DataSource = databasePath,
-            Mode = SqliteOpenMode.ReadWriteCreate,
-
-            // Each process holds its own single connection; a pool would only add noise.
-            Pooling = false,
-        }.ToString());
-
-        connection.Open();
         try
         {
             ApplyPragmas(connection);
@@ -57,6 +48,74 @@ public static class SqliteDatabase
         }
     }
 
+    /// <summary>
+    /// Creates a new database file with the mandatory PRAGMAs, then applies
+    /// every migration and the optional seed in one transaction, so the file
+    /// ends up either fully built and seeded or still at version 0.
+    /// </summary>
+    /// <param name="databasePath">Path of the SQLite database file; it must not exist yet.</param>
+    /// <param name="migrations">Contiguous migration list ordered by version, starting at 1.</param>
+    /// <param name="seed">Initial content written in the same transaction, when any.</param>
+    /// <returns>An open connection with all PRAGMAs applied and the schema migrated.</returns>
+    /// <exception cref="ArgumentException">The migration list is not contiguous from version 1.</exception>
+    /// <exception cref="InvalidOperationException">The database file already exists.</exception>
+    public static SqliteConnection Create(
+        string databasePath,
+        IReadOnlyList<SchemaMigration> migrations,
+        Action<SqliteConnection, SqliteTransaction>? seed = null)
+    {
+        ValidateMigrations(migrations);
+        if (File.Exists(databasePath))
+        {
+            throw new InvalidOperationException($"The database already exists: {databasePath}");
+        }
+
+        SqliteConnection connection = OpenConnection(databasePath);
+        try
+        {
+            ApplyPragmas(connection);
+            RegisterFunctions(connection);
+
+            using var transaction = connection.BeginTransaction();
+            foreach (SchemaMigration migration in migrations)
+            {
+                Execute(connection, migration.Sql, transaction);
+                Execute(connection, $"PRAGMA user_version={migration.Version};", transaction);
+            }
+            seed?.Invoke(connection, transaction);
+            transaction.Commit();
+            return connection;
+        }
+        catch
+        {
+            connection.Dispose();
+
+            // Leave no empty version-0 file behind that a later open would migrate into an empty board.
+            DeleteDatabaseFilesBestEffort(databasePath);
+            throw;
+        }
+    }
+
+    /// <summary>
+    /// Deletes a database file together with its WAL and shared-memory files,
+    /// ignoring failures.
+    /// </summary>
+    /// <param name="databasePath">Path of the SQLite database file.</param>
+    private static void DeleteDatabaseFilesBestEffort(string databasePath)
+    {
+        foreach (string path in new[] { databasePath, databasePath + "-wal", databasePath + "-shm", databasePath + "-journal" })
+        {
+            try
+            {
+                File.Delete(path);
+            }
+            catch (Exception exception) when (exception is IOException or UnauthorizedAccessException)
+            {
+                // Best effort: the original failure is what gets reported.
+            }
+        }
+    }
+
     /// <summary>
     /// Reads the stored <c>PRAGMA user_version</c> value.
     /// </summary>
@@ -105,14 +164,7 @@ public static class SqliteDatabase
     /// <exception cref="SchemaVersionException">The database was written by a newer build.</exception>
     internal static void ApplyMigrations(SqliteConnection connection, IReadOnlyList<SchemaMigration> migrations)
     {
-        // Contract check: the list must be ordered, contiguous and start at version 1.
+        ValidateMigrations(migrations);
-        for (int index = 0; index < migrations.Count; index++)
-        {
-            if (migrations[index].Version != index + 1)
-            {
-                throw new ArgumentException("Migrations must be contiguous and start at version 1.", nameof(migrations));
-            }
-        }
 
         long current = ReadUserVersion(connection);
         int latest = migrations.Count;
@@ -138,6 +190,42 @@ public static class SqliteDatabase
         }
     }
 
+    /// <summary>
+    /// Opens a connection to the file, creating it if needed, without any configuration.
+    /// </summary>
+    /// <param name="databasePath">Path of the SQLite database file.</param>
+    /// <returns>The open connection.</returns>
+    private static SqliteConnection OpenConnection(string databasePath)
+    {
+        var connection = new SqliteConnection(new SqliteConnectionStringBuilder
+        {
+            DataSource = databasePath,
+            Mode = SqliteOpenMode.ReadWriteCreate,
+
+            // Each process holds its own single connection; a pool would only add noise.
+            Pooling = false,
+        }.ToString());
+
+        connection.Open();
+        return connection;
+    }
+
+    /// <summary>
+    /// Checks that the migration list is ordered, contiguous and starts at version 1.
+    /// </summary>
+    /// <param name="migrations">The migration list.</param>
+    /// <exception cref="ArgumentException">The list breaks the contract.</exception>
+    private static void ValidateMigrations(IReadOnlyList<SchemaMigration> migrations)
+    {
+        for (int index = 0; index < migrations.Count; index++)
+        {
+            if (migrations[index].Version != index + 1)
+            {
+                throw new ArgumentException("Migrations must be contiguous and start at version 1.", nameof(migrations));
+            }
+        }
+    }
+
     /// <summary>
     /// Executes a non-query SQL script, optionally inside an explicit transaction.
     /// </summary>

YKanBan/Storage/Workspace/WorkspaceInitializer.cs

@@ -3,10 +3,10 @@ using Microsoft.Data.Sqlite;
 namespace YKanBan.Storage.Workspace;
 
 /// <summary>
-/// Creates the .ykanban structure for a folder: the folder itself and an
+/// Creates the .ykanban structure for a folder in the mandated order: the
-/// empty, fully migrated database. Adding preset content is deliberately a
+/// .ykanban folder, then the exclusive lock, then the database. The database
-/// separate concern handled by <see cref="WorkspacePreset"/>, so the
+/// schema and the optional preset content are written in one transaction, so a
-/// database can be created without any rows.
+/// failure never leaves a half-built database behind.
 /// </summary>
 public static class WorkspaceInitializer
 {
@@ -19,27 +19,50 @@ public static class WorkspaceInitializer
     public static bool IsWorkspace(string folderPath) => Directory.Exists(WorkspacePaths.Root(folderPath));
 
     /// <summary>
-    /// Initializes a fresh workspace: creates .ykanban and an empty migrated
+    /// Initializes a fresh workspace without any preset content and releases
-    /// database, without adding any preset content.
+    /// the lock again.
     /// </summary>
     /// <param name="folderPath">The existing folder to initialize.</param>
     /// <exception cref="WorkspaceDirectoryMissingException">The folder does not exist on disk.</exception>
+    /// <exception cref="WorkspaceLockException">Another instance holds the workspace lock.</exception>
     /// <exception cref="InvalidOperationException">The workspace database already exists.</exception>
     public static void Initialize(string folderPath)
+    {
+        using WorkspaceLock workspaceLock = InitializeLocked(folderPath, addPresetColumns: false);
+    }
+
+    /// <summary>
+    /// Initializes a fresh workspace and returns the lock still held, so the
+    /// caller can open the workspace without ever releasing it.
+    /// </summary>
+    /// <param name="folderPath">The existing folder to initialize.</param>
+    /// <param name="addPresetColumns">Whether to add the preset columns in the schema transaction.</param>
+    /// <returns>The held workspace lock.</returns>
+    /// <exception cref="WorkspaceDirectoryMissingException">The folder does not exist on disk.</exception>
+    /// <exception cref="WorkspaceLockException">Another instance holds the workspace lock.</exception>
+    /// <exception cref="InvalidOperationException">The workspace database already exists.</exception>
+    internal static WorkspaceLock InitializeLocked(string folderPath, bool addPresetColumns)
     {
         if (!Directory.Exists(folderPath))
         {
             throw new WorkspaceDirectoryMissingException(folderPath);
         }
 
-        string databasePath = WorkspacePaths.Database(folderPath);
+        // .ykanban first: the lock file lives inside it.
-        if (File.Exists(databasePath))
-        {
-            throw new InvalidOperationException($"The workspace database already exists: {databasePath}");
-        }
-
-        // Create the .ykanban folder, then the migrated but still empty database.
         Directory.CreateDirectory(WorkspacePaths.Root(folderPath));
-        using SqliteConnection connection = SqliteDatabase.Open(databasePath, WorkspaceSchema.Migrations);
+        WorkspaceLock workspaceLock = WorkspaceLock.Acquire(folderPath);
+        try
+        {
+            using SqliteConnection connection = SqliteDatabase.Create(
+                WorkspacePaths.Database(folderPath),
+                WorkspaceSchema.Migrations,
+                addPresetColumns ? WorkspacePreset.AddPresetColumns : null);
+            return workspaceLock;
+        }
+        catch
+        {
+            workspaceLock.Dispose();
+            throw;
+        }
     }
 }

YKanBan/Storage/Workspace/WorkspacePreset.cs

@@ -12,15 +12,14 @@ public static class WorkspacePreset
 {
     /// <summary>
     /// Inserts the three preset columns (the current language's equivalents of
-    /// To Do / In Progress / Done) in a single transaction.
+    /// To Do / In Progress / Done) inside the caller's transaction, so the
+    /// presets land together with the schema or not at all.
     /// </summary>
-    /// <param name="folderPath">The initialized workspace folder.</param>
+    /// <param name="connection">The connection to the freshly created database.</param>
-    /// <exception cref="SqliteException">The workspace database does not exist or a title collides with an existing column.</exception>
+    /// <param name="transaction">The transaction that also builds the schema.</param>
-    public static void AddPresetColumns(string folderPath)
+    /// <exception cref="SqliteException">A title collides with an existing column.</exception>
+    public static void AddPresetColumns(SqliteConnection connection, SqliteTransaction transaction)
     {
-        using SqliteConnection connection = SqliteDatabase.Open(
-            WorkspacePaths.Database(folderPath), WorkspaceSchema.Migrations);
-
         long now = UnixTime.Now;
 
         // Preset titles are data at creation time: whatever the current language says gets stored.
@@ -31,8 +30,6 @@ public static class WorkspacePreset
             Resources.PresetColumn_Done,
         ];
 
-        // Insert all three atomically so a partially populated workspace can never exist.
-        using var transaction = connection.BeginTransaction();
         foreach (string title in titles)
         {
             using var command = connection.CreateCommand();
@@ -45,6 +42,5 @@ public static class WorkspacePreset
             command.Parameters.AddWithValue("$now", now);
             command.ExecuteNonQuery();
         }
-        transaction.Commit();
     }
 }

YKanBan/Storage/Workspace/WorkspaceSession.cs

@@ -41,9 +41,29 @@ public sealed class WorkspaceSession : IDisposable
     /// <returns>The opened session; dispose it to release the lock and connection.</returns>
     /// <exception cref="WorkspaceNotInitializedException">The folder has no .ykanban structure.</exception>
     /// <exception cref="WorkspaceLockException">Another instance already holds the lock.</exception>
-    public static WorkspaceSession Open(string folderPath)
+    public static WorkspaceSession Open(string folderPath) => OpenLocked(folderPath, WorkspaceLock.Acquire(folderPath));
+
+    /// <summary>
+    /// Initializes a workspace with its preset columns and 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.
+    /// </summary>
+    /// <param name="folderPath">The existing folder to initialize.</param>
+    /// <returns>The opened session; dispose it to release the lock and connection.</returns>
+    /// <exception cref="WorkspaceDirectoryMissingException">The folder does not exist on disk.</exception>
+    /// <exception cref="WorkspaceLockException">Another instance holds the workspace lock.</exception>
+    /// <exception cref="InvalidOperationException">The workspace database already exists.</exception>
+    public static WorkspaceSession Initialize(string folderPath) =>
+        OpenLocked(folderPath, WorkspaceInitializer.InitializeLocked(folderPath, addPresetColumns: true));
+
+    /// <summary>
+    /// Opens the repository under an already held lock; the lock is released if that fails.
+    /// </summary>
+    /// <param name="folderPath">The workspace folder.</param>
+    /// <param name="workspaceLock">The held workspace lock.</param>
+    /// <returns>The opened session.</returns>
+    private static WorkspaceSession OpenLocked(string folderPath, WorkspaceLock workspaceLock)
     {
-        WorkspaceLock workspaceLock = WorkspaceLock.Acquire(folderPath);
         try
         {
             var repository = new WorkspaceRepository(folderPath);

YKanBan/ViewModels/MainWindowViewModel.cs

@@ -105,15 +105,21 @@ public sealed partial class MainWindowViewModel : ViewModelBase
     /// Opens the workspace and switches the content to the normal view.
     /// </summary>
     /// <param name="folderPath">The workspace folder to open.</param>
-    private void OpenWorkspace(string folderPath)
+    private void OpenWorkspace(string folderPath) => ShowWorkspace(folderPath, _services.OpenWorkspace(folderPath));
+
+    /// <summary>
+    /// Switches the content to the normal view of an opened workspace.
+    /// </summary>
+    /// <param name="folderPath">The workspace folder.</param>
+    /// <param name="session">The opened workspace session.</param>
+    private void ShowWorkspace(string folderPath, WorkspaceSession session)
     {
-        WorkspaceSession session = _services.OpenWorkspace(folderPath);
         Content = new WorkspaceViewModel(session, ShowSettings, ShowAbout, _services.Config, _dialogs);
         Title = $"{new DirectoryInfo(folderPath).Name} - {Resources.App_Name}";
     }
 
     /// <summary>
-    /// Initializes the folder shown by the not-initialized page, adds the preset
+    /// Initializes the folder shown by the not-initialized page with its preset
     /// columns and opens the resulting workspace.
     /// </summary>
     /// <returns>A completed task once initialization has been attempted.</returns>
@@ -126,9 +132,12 @@ public sealed partial class MainWindowViewModel : ViewModelBase
 
         try
         {
-            WorkspaceInitializer.Initialize(page.FolderPath);
+            ShowWorkspace(page.FolderPath, _services.InitializeWorkspace(page.FolderPath));
-            WorkspacePreset.AddPresetColumns(page.FolderPath);
+        }
-            OpenWorkspace(page.FolderPath);
+        catch (WorkspaceLockException)
+        {
+            // Another instance got there first and holds the workspace: same page as at startup.
+            Content = new LockConflictPageViewModel(page.FolderPath);
         }
         catch (Exception exception)
         {