From df7a55b13d75fa346a16604a20054b473a4ac035 Mon Sep 17 00:00:00 2001 From: Ada Date: Sat, 9 May 2026 06:47:57 -0400 Subject: [PATCH] Backups: stop double-opening the SQLite connection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BackupService had a redundant conn.Open() call in every method that used _db.CreateConnection(). The factory already returns an opened connection, so this was double-opening on each call. The custom-built System.Data.SQLite that ships with this mod silently drops subsequent INSERT/UPDATE statements when Open is invoked twice. No exception is thrown, the API returns 200, but no row ends up in the table. Reads kept working — which is exactly why this slipped past initial testing: GetAll never blew up, the panel rendered "No backups yet", and the empty list looked normal because the table really was empty. Symptom that surfaced today: clicking Create Backup made a ZIP file on disk but nothing in the backups table. The Auto Backup toggle in the panel persisted to UI state but SaveSettings silently lost the write, so no backup_settings row in the settings table, so the scheduler never started either. Confirmed via diagnostic logging on prod: removing the second .Open() made the INSERT persist (rows=1, last_insert_rowid=2, total=1). Verified the bug isn't elsewhere — grep across all repository code shows BackupService is the only place that calls conn.Open() inside the using block. Added a class-level doc comment so future-me doesn't re-introduce it. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/KitsuneCommand/Services/BackupService.cs | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/KitsuneCommand/Services/BackupService.cs b/src/KitsuneCommand/Services/BackupService.cs index f227c3b..0acf263 100644 --- a/src/KitsuneCommand/Services/BackupService.cs +++ b/src/KitsuneCommand/Services/BackupService.cs @@ -13,6 +13,15 @@ namespace KitsuneCommand.Services /// /// Manages world save backups: create, restore, delete, and schedule auto-backups. /// Stores backup metadata in SQLite for the management UI. + /// + /// Connection lifetime note: + /// returns an *already-opened* connection. Do NOT call conn.Open() again + /// inside the using block. The custom-built System.Data.SQLite that ships with + /// this mod silently drops subsequent INSERT/UPDATE statements when Open is + /// invoked twice — no exception is thrown, the controller returns 200, but no + /// row ends up in the table. (Reads happen to keep working, which is why the + /// bug went unnoticed long enough to ship a UI for the feature.) Always trust + /// the factory to return a ready-to-use connection. /// public class BackupService { @@ -105,7 +114,6 @@ public BackupRecord CreateBackup(string backupType = "manual", string notes = nu // Save to database using (var conn = _db.CreateConnection()) { - conn.Open(); Dapper.SqlMapper.Execute(conn, @"INSERT INTO backups (filename, world_name, size_bytes, created_at, backup_type, notes) VALUES (@Filename, @WorldName, @SizeBytes, @CreatedAt, @BackupType, @Notes)", @@ -167,7 +175,6 @@ public void DeleteBackup(int backupId) // Delete record using (var conn = _db.CreateConnection()) { - conn.Open(); Dapper.SqlMapper.Execute(conn, "DELETE FROM backups WHERE id = @Id", new { Id = backupId }); @@ -181,7 +188,6 @@ public List GetAll() { using (var conn = _db.CreateConnection()) { - conn.Open(); return Dapper.SqlMapper.Query(conn, "SELECT id as Id, filename as Filename, world_name as WorldName, size_bytes as SizeBytes, " + "created_at as CreatedAt, backup_type as BackupType, notes as Notes " + @@ -197,7 +203,6 @@ public BackupRecord GetBackup(int id) { using (var conn = _db.CreateConnection()) { - conn.Open(); return Dapper.SqlMapper.QueryFirstOrDefault(conn, "SELECT id as Id, filename as Filename, world_name as WorldName, size_bytes as SizeBytes, " + "created_at as CreatedAt, backup_type as BackupType, notes as Notes " + @@ -228,8 +233,7 @@ public void LoadSettings() { using (var conn = _db.CreateConnection()) { - conn.Open(); - var json = Dapper.SqlMapper.QueryFirstOrDefault(conn, + var json = Dapper.SqlMapper.QueryFirstOrDefault(conn, "SELECT value FROM settings WHERE name = 'backup_settings'"); if (!string.IsNullOrEmpty(json)) @@ -246,7 +250,6 @@ private void SaveSettings() { using (var conn = _db.CreateConnection()) { - conn.Open(); var json = Newtonsoft.Json.JsonConvert.SerializeObject(_settings); Dapper.SqlMapper.Execute(conn, @"INSERT OR REPLACE INTO settings (name, value) VALUES ('backup_settings', @Value)",