From 0d8a80aa27f5f13642292f80e691c8a30079f509 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Sun, 19 Jul 2026 09:07:49 -0400 Subject: [PATCH] feat(localdb): rewire SiteEventLogger onto the consolidated site database Completes Task 5a of the LocalDb Phase 1 adoption plan. The GUID-id half landed with Task 2 (727fa48c) because leaving it out would have committed a writer that could not satisfy site_events.id NOT NULL; this is the connection half, which was blocked on Task 3. SiteEventLogger owned a SqliteConnection built from SiteEventLogOptions .DatabasePath. It now takes ILocalDb and gets that connection from CreateConnection() - already open, pragma-configured, and carrying the zb_hlc_next() UDF that site_events' capture triggers call. It still owns and disposes the connection; WithConnection's signature and locking semantics are untouched, so EventLogQueryService and EventLogPurgeService are unaffected. The connectionStringOverride seam is deleted rather than preserved. site_events is a replicated table, so a raw connection lacks the UDF and fails closed on every insert - an override could only produce a logger that cannot write. It had no callers outside this class (the connectionStringOverride hits elsewhere in the tree belong to SqliteAuditWriter, a different type). Tests: a shared TestLocalDb helper gives each fixture a real ILocalDb over a temp file. Real rather than stubbed on purpose - a stub handing back a bare SqliteConnection would pass today and fail closed the moment the table is registered, which is exactly the silent-wiring failure mode this family has hit three times. ServiceWiringTests' DI path also needed AddZbLocalDb, mirroring SiteServiceRegistration. Verified: build 0 warnings; SiteEventLogging 70/70, Host 294/294, SiteRuntime 530/530. Claude-Session: https://claude.ai/code/session_01BL2Vu1ESDQ9SCN4gVKkdts --- .../SiteEventLogger.cs | 38 ++++++---- .../EventLogCoverageTests.cs | 11 ++- .../EventLogPurgeServiceTests.cs | 9 ++- .../EventLogQueryServiceTests.cs | 14 ++-- .../SchemaIndexTests.cs | 7 +- .../ServiceWiringTests.cs | 16 ++++- .../SiteEventLoggerAsyncTests.cs | 18 +++-- .../SiteEventLoggerTests.cs | 7 +- .../TestLocalDb.cs | 70 +++++++++++++++++++ 9 files changed, 154 insertions(+), 36 deletions(-) create mode 100644 tests/ZB.MOM.WW.ScadaBridge.SiteEventLogging.Tests/TestLocalDb.cs diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteEventLogging/SiteEventLogger.cs b/src/ZB.MOM.WW.ScadaBridge.SiteEventLogging/SiteEventLogger.cs index 53ff5add..a5c6e868 100644 --- a/src/ZB.MOM.WW.ScadaBridge.SiteEventLogging/SiteEventLogger.cs +++ b/src/ZB.MOM.WW.ScadaBridge.SiteEventLogging/SiteEventLogger.cs @@ -2,6 +2,7 @@ using System.Threading.Channels; using Microsoft.Data.Sqlite; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; +using ZB.MOM.WW.LocalDb; namespace ZB.MOM.WW.ScadaBridge.SiteEventLogging; @@ -40,29 +41,36 @@ public class SiteEventLogger : ISiteEventLogger, IDisposable private bool _disposed; /// - /// Initializes the event logger, opens the SQLite connection, and starts the background writer loop. + /// Initializes the event logger over the consolidated site database and starts the + /// background writer loop. /// - /// Site event log configuration (database path, retention settings). + /// Site event log configuration (retention settings, queue capacity). /// Logger for write-failure diagnostics. - /// Optional connection string override; uses the configured path when null. + /// + /// The consolidated site database. The connection it hands out is already open and + /// carries the zb_hlc_next() UDF that site_events' capture triggers + /// call. This logger owns the connection for its lifetime and disposes it. + /// + /// + /// The former connectionStringOverride seam is deliberately gone rather than + /// preserved. site_events is a replicated table, so a raw connection lacking + /// the UDF would fail closed on every insert — an override would only let a caller + /// build a logger that cannot write. SiteEventLogOptions.DatabasePath is + /// likewise no longer read: the location is LocalDb:Path. + /// public SiteEventLogger( IOptions options, ILogger logger, - string? connectionStringOverride = null) + ILocalDb localDb) { + ArgumentNullException.ThrowIfNull(options); + ArgumentNullException.ThrowIfNull(localDb); + _logger = logger; - // Cache=Shared is a cross-connection optimisation - // that lets multiple SqliteConnections share an in-process page cache. - // This logger owns exactly one SqliteConnection and serialises all - // access through _writeLock, so the mode is dormant — at best dead - // configuration, at worst a small future foot-gun for any second - // connection opened to the same file. A test path that genuinely - // needs Cache=Shared can still inject it via connectionStringOverride. - var connectionString = connectionStringOverride - ?? $"Data Source={options.Value.DatabasePath}"; - _connection = new SqliteConnection(connectionString); - _connection.Open(); + // Already open — CreateConnection returns a live, pragma-configured, + // UDF-registered connection. Calling Open() on it again would throw. + _connection = localDb.CreateConnection(); InitializeSchema(); diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteEventLogging.Tests/EventLogCoverageTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteEventLogging.Tests/EventLogCoverageTests.cs index da267200..b2a6e9b1 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.SiteEventLogging.Tests/EventLogCoverageTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteEventLogging.Tests/EventLogCoverageTests.cs @@ -11,20 +11,27 @@ namespace ZB.MOM.WW.ScadaBridge.SiteEventLogging.Tests; public class EventLogCoverageTests : IDisposable { private readonly string _dbPath; + private readonly TestLocalDb _localDb; public EventLogCoverageTests() { _dbPath = Path.Combine(Path.GetTempPath(), $"test_coverage_{Guid.NewGuid()}.db"); + _localDb = TestLocalDb.Create(_dbPath); } public void Dispose() { - if (File.Exists(_dbPath)) File.Delete(_dbPath); + _localDb.Dispose(); + TestLocalDb.DeleteFiles(_dbPath); + GC.SuppressFinalize(this); } + // Each call gets its own connection from the one shared local database, so the + // loggers these tests dispose mid-test do not tear down the database itself. private SiteEventLogger NewLogger() => new( Options.Create(new SiteEventLogOptions { DatabasePath = _dbPath }), - NullLogger.Instance); + NullLogger.Instance, + _localDb.Db); private static EventLogQueryRequest MakeRequest() => new( CorrelationId: "corr-err", diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteEventLogging.Tests/EventLogPurgeServiceTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteEventLogging.Tests/EventLogPurgeServiceTests.cs index 8725d83a..22240faa 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.SiteEventLogging.Tests/EventLogPurgeServiceTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteEventLogging.Tests/EventLogPurgeServiceTests.cs @@ -6,6 +6,7 @@ namespace ZB.MOM.WW.ScadaBridge.SiteEventLogging.Tests; public class EventLogPurgeServiceTests : IDisposable { private readonly SiteEventLogger _eventLogger; + private readonly TestLocalDb _localDb; private readonly string _dbPath; private readonly SiteEventLogOptions _options; @@ -27,15 +28,19 @@ public class EventLogPurgeServiceTests : IDisposable RetentionDays = 30, MaxStorageMb = 1024 }; + _localDb = TestLocalDb.Create(_dbPath); _eventLogger = new SiteEventLogger( Options.Create(_options), - NullLogger.Instance); + NullLogger.Instance, + _localDb.Db); } public void Dispose() { _eventLogger.Dispose(); - if (File.Exists(_dbPath)) File.Delete(_dbPath); + _localDb.Dispose(); + TestLocalDb.DeleteFiles(_dbPath); + GC.SuppressFinalize(this); } private EventLogPurgeService CreatePurgeService( diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteEventLogging.Tests/EventLogQueryServiceTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteEventLogging.Tests/EventLogQueryServiceTests.cs index 82cac861..a7b55f96 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.SiteEventLogging.Tests/EventLogQueryServiceTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteEventLogging.Tests/EventLogQueryServiceTests.cs @@ -8,6 +8,7 @@ public class EventLogQueryServiceTests : IDisposable { private readonly SiteEventLogger _eventLogger; private readonly EventLogQueryService _queryService; + private readonly TestLocalDb _localDb; private readonly string _dbPath; public EventLogQueryServiceTests() @@ -18,7 +19,8 @@ public class EventLogQueryServiceTests : IDisposable DatabasePath = _dbPath, QueryPageSize = 500 }); - _eventLogger = new SiteEventLogger(options, NullLogger.Instance); + _localDb = TestLocalDb.Create(_dbPath); + _eventLogger = new SiteEventLogger(options, NullLogger.Instance, _localDb.Db); _queryService = new EventLogQueryService( _eventLogger, options, @@ -28,7 +30,9 @@ public class EventLogQueryServiceTests : IDisposable public void Dispose() { _eventLogger.Dispose(); - if (File.Exists(_dbPath)) File.Delete(_dbPath); + _localDb.Dispose(); + TestLocalDb.DeleteFiles(_dbPath); + GC.SuppressFinalize(this); } private async Task SeedEvents() @@ -367,7 +371,8 @@ public class EventLogQueryServiceTests : IDisposable QueryPageSize = 500, MaxQueryPageSize = 3, }); - using var logger = new SiteEventLogger(options, NullLogger.Instance); + using var localDb = TestLocalDb.Create(dbPath); + using var logger = new SiteEventLogger(options, NullLogger.Instance, localDb.Db); var query = new EventLogQueryService(logger, options, NullLogger.Instance); try @@ -391,7 +396,8 @@ public class EventLogQueryServiceTests : IDisposable finally { logger.Dispose(); - if (File.Exists(dbPath)) File.Delete(dbPath); + localDb.Dispose(); + TestLocalDb.DeleteFiles(dbPath); } } } diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteEventLogging.Tests/SchemaIndexTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteEventLogging.Tests/SchemaIndexTests.cs index 0038ad4c..7f70a4cb 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.SiteEventLogging.Tests/SchemaIndexTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteEventLogging.Tests/SchemaIndexTests.cs @@ -13,12 +13,14 @@ public class SchemaIndexTests : IDisposable private readonly SiteEventLogger _logger; private readonly SqliteConnection _verifyConnection; private readonly string _dbPath; + private readonly TestLocalDb _localDb; public SchemaIndexTests() { _dbPath = Path.Combine(Path.GetTempPath(), $"test_index_{Guid.NewGuid()}.db"); var options = Options.Create(new SiteEventLogOptions { DatabasePath = _dbPath }); - _logger = new SiteEventLogger(options, NullLogger.Instance); + _localDb = TestLocalDb.Create(_dbPath); + _logger = new SiteEventLogger(options, NullLogger.Instance, _localDb.Db); _verifyConnection = new SqliteConnection($"Data Source={_dbPath}"); _verifyConnection.Open(); @@ -28,7 +30,8 @@ public class SchemaIndexTests : IDisposable { _verifyConnection.Dispose(); _logger.Dispose(); - if (File.Exists(_dbPath)) File.Delete(_dbPath); + _localDb.Dispose(); + TestLocalDb.DeleteFiles(_dbPath); } [Fact] diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteEventLogging.Tests/ServiceWiringTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteEventLogging.Tests/ServiceWiringTests.cs index 12546e64..9068edfb 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.SiteEventLogging.Tests/ServiceWiringTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteEventLogging.Tests/ServiceWiringTests.cs @@ -1,6 +1,8 @@ +using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; +using ZB.MOM.WW.LocalDb; namespace ZB.MOM.WW.ScadaBridge.SiteEventLogging.Tests; @@ -23,7 +25,7 @@ public class ServiceWiringTests : IDisposable public void Dispose() { _provider?.Dispose(); - if (File.Exists(_dbPath)) File.Delete(_dbPath); + TestLocalDb.DeleteFiles(_dbPath); } [Fact] @@ -32,6 +34,15 @@ public class ServiceWiringTests : IDisposable var services = new ServiceCollection(); services.AddLogging(); services.Configure(o => o.DatabasePath = _dbPath); + // SiteEventLogger now takes ILocalDb, so the consolidated site database has to + // be in the graph for AddSiteEventLogging to resolve at all. In the host this + // comes from SiteServiceRegistration's AddZbLocalDb. + services.AddZbLocalDb(new ConfigurationBuilder() + .AddInMemoryCollection(new Dictionary + { + ["LocalDb:Path"] = _dbPath, + }) + .Build()); services.AddSiteEventLogging(); _provider = services.BuildServiceProvider(); @@ -57,8 +68,9 @@ public class ServiceWiringTests : IDisposable // concrete SiteEventLogger. Constructing them directly must compile and work. var options = Options.Create(new SiteEventLogOptions { DatabasePath = _dbPath }); using var loggerFactory = LoggerFactory.Create(_ => { }); + using var localDb = TestLocalDb.Create(_dbPath); using var recorder = new SiteEventLogger( - options, loggerFactory.CreateLogger()); + options, loggerFactory.CreateLogger(), localDb.Db); var purge = new EventLogPurgeService( recorder, options, loggerFactory.CreateLogger()); diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteEventLogging.Tests/SiteEventLoggerAsyncTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteEventLogging.Tests/SiteEventLoggerAsyncTests.cs index 4d617ff6..e469a4ec 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.SiteEventLogging.Tests/SiteEventLoggerAsyncTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteEventLogging.Tests/SiteEventLoggerAsyncTests.cs @@ -13,18 +13,21 @@ public class SiteEventLoggerAsyncTests : IDisposable { private readonly SiteEventLogger _logger; private readonly string _dbPath; + private readonly TestLocalDb _localDb; public SiteEventLoggerAsyncTests() { _dbPath = Path.Combine(Path.GetTempPath(), $"test_async_{Guid.NewGuid()}.db"); var options = Options.Create(new SiteEventLogOptions { DatabasePath = _dbPath }); - _logger = new SiteEventLogger(options, NullLogger.Instance); + _localDb = TestLocalDb.Create(_dbPath); + _logger = new SiteEventLogger(options, NullLogger.Instance, _localDb.Db); } public void Dispose() { _logger.Dispose(); - if (File.Exists(_dbPath)) File.Delete(_dbPath); + _localDb.Dispose(); + TestLocalDb.DeleteFiles(_dbPath); } [Fact] @@ -91,11 +94,10 @@ public class SiteEventLoggerAsyncTests : IDisposable // Health Monitoring can surface a logging outage. using var failingConnection = new SqliteConnection("Data Source=:memory:"); failingConnection.Open(); - var options = Options.Create(new SiteEventLogOptions - { - DatabasePath = Path.Combine(Path.GetTempPath(), $"test_fail_{Guid.NewGuid()}.db") - }); - var logger = new SiteEventLogger(options, NullLogger.Instance); + var failDbPath = Path.Combine(Path.GetTempPath(), $"test_fail_{Guid.NewGuid()}.db"); + var options = Options.Create(new SiteEventLogOptions { DatabasePath = failDbPath }); + using var failLocalDb = TestLocalDb.Create(failDbPath); + var logger = new SiteEventLogger(options, NullLogger.Instance, failLocalDb.Db); // Drop the table so every write fails with "no such table". logger.WithConnection(connection => @@ -112,5 +114,7 @@ public class SiteEventLoggerAsyncTests : IDisposable "FailedWriteCount must increment when an event write fails."); logger.Dispose(); + failLocalDb.Dispose(); + TestLocalDb.DeleteFiles(failDbPath); } } diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteEventLogging.Tests/SiteEventLoggerTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteEventLogging.Tests/SiteEventLoggerTests.cs index 81ae9137..c2921663 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.SiteEventLogging.Tests/SiteEventLoggerTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteEventLogging.Tests/SiteEventLoggerTests.cs @@ -9,12 +9,14 @@ public class SiteEventLoggerTests : IDisposable private readonly SiteEventLogger _logger; private readonly SqliteConnection _verifyConnection; private readonly string _dbPath; + private readonly TestLocalDb _localDb; public SiteEventLoggerTests() { _dbPath = Path.Combine(Path.GetTempPath(), $"test_events_{Guid.NewGuid()}.db"); var options = Options.Create(new SiteEventLogOptions { DatabasePath = _dbPath }); - _logger = new SiteEventLogger(options, NullLogger.Instance); + _localDb = TestLocalDb.Create(_dbPath); + _logger = new SiteEventLogger(options, NullLogger.Instance, _localDb.Db); // Separate connection for verification queries _verifyConnection = new SqliteConnection($"Data Source={_dbPath}"); @@ -25,7 +27,8 @@ public class SiteEventLoggerTests : IDisposable { _verifyConnection.Dispose(); _logger.Dispose(); - if (File.Exists(_dbPath)) File.Delete(_dbPath); + _localDb.Dispose(); + TestLocalDb.DeleteFiles(_dbPath); } [Fact] diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteEventLogging.Tests/TestLocalDb.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteEventLogging.Tests/TestLocalDb.cs new file mode 100644 index 00000000..a5df1b4a --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteEventLogging.Tests/TestLocalDb.cs @@ -0,0 +1,70 @@ +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using ZB.MOM.WW.LocalDb; + +namespace ZB.MOM.WW.ScadaBridge.SiteEventLogging.Tests; + +/// +/// A real over a temp file, for tests that construct a +/// directly. +/// +/// +/// +/// takes rather than a connection +/// string, and there is no in-memory mode — LocalDbOptions.Path is a filesystem +/// path. A real one is used rather than a stub on purpose: connections handed out by +/// ILocalDb carry the zb_hlc_next() UDF that site_events' capture +/// triggers call, so a stub returning a bare SqliteConnection would pass these +/// tests while failing closed the moment the table is registered for replication. +/// +/// +/// No onReady callback and no RegisterReplicated: the logger's own +/// InitializeSchema creates the table, which is what keeps a directly-constructed +/// logger self-sufficient. Registration is the host's job +/// (SiteLocalDbSetup.OnReady). +/// +/// +public sealed class TestLocalDb : IDisposable +{ + private readonly ServiceProvider _provider; + + private TestLocalDb(ServiceProvider provider, ILocalDb db) + { + _provider = provider; + Db = db; + } + + /// The live local database. + public ILocalDb Db { get; } + + /// Opens a local database at . + public static TestLocalDb Create(string databasePath) + { + var configuration = new ConfigurationBuilder() + .AddInMemoryCollection(new Dictionary + { + ["LocalDb:Path"] = databasePath, + }) + .Build(); + + var provider = new ServiceCollection() + .AddZbLocalDb(configuration) + .BuildServiceProvider(); + + return new TestLocalDb(provider, provider.GetRequiredService()); + } + + /// + /// Deletes and its WAL sidecars. Call only after the + /// owning is disposed — the master connection anchors the WAL. + /// + public static void DeleteFiles(string databasePath) + { + foreach (var suffix in new[] { "", "-wal", "-shm" }) + { + try { File.Delete(databasePath + suffix); } catch { /* best effort */ } + } + } + + public void Dispose() => _provider.Dispose(); +}