diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/ServiceCollectionExtensions.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/ServiceCollectionExtensions.cs index 7352e990..3d499a2e 100644 --- a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/ServiceCollectionExtensions.cs +++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/ServiceCollectionExtensions.cs @@ -70,7 +70,11 @@ public static class ServiceCollectionExtensions // reconciliation, Tracking.Status audit-only). Central roots never call // AddSiteRuntime, so this stays absent on central — matching the SITE-ONLY // contract. OperationTrackingOptions is bound + validated by the Host's - // SiteServiceRegistration site-options block. + // SiteServiceRegistration site-options block — but it no longer supplies the + // store's database: the store takes ILocalDb (the consolidated site database + // at LocalDb:Path), so OperationTrackingOptions.ConnectionString is now only + // read by the site config DB. Resolving this store therefore forces ILocalDb + // construction, which is what runs SiteLocalDbSetup.OnReady. services.AddSingleton(); // Site-local repository implementations backed by SQLite diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Tracking/OperationTrackingStore.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Tracking/OperationTrackingStore.cs index 1257ea10..feb1464a 100644 --- a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Tracking/OperationTrackingStore.cs +++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Tracking/OperationTrackingStore.cs @@ -1,7 +1,7 @@ using System.Globalization; using Microsoft.Data.Sqlite; using Microsoft.Extensions.Logging; -using Microsoft.Extensions.Options; +using ZB.MOM.WW.LocalDb; using ZB.MOM.WW.ScadaBridge.Commons.Interfaces; using ZB.MOM.WW.ScadaBridge.Commons.Types; @@ -39,7 +39,7 @@ public class OperationTrackingStore : IOperationTrackingStore, IAsyncDisposable, // _writeGate. Readers do NOT share this connection or gate; see GetStatusAsync. private readonly SqliteConnection _writeConnection; private readonly SemaphoreSlim _writeGate = new(1, 1); - private readonly string _connectionString; + private readonly ILocalDb _localDb; private readonly ILogger _logger; // Dispose-once state shared by the sync Dispose and async @@ -51,29 +51,43 @@ public class OperationTrackingStore : IOperationTrackingStore, IAsyncDisposable, private int _disposeState; /// - /// Initializes the tracking store, opens the SQLite connection, and applies the schema. + /// Initializes the tracking store over the consolidated site database and applies the schema. /// - /// Tracking store configuration (connection string, retention window). + /// + /// The consolidated site database. Every connection it hands out is already open and carries + /// the per-connection pragmas plus the zb_hlc_next() UDF that + /// OperationTracking's capture triggers call — which is exactly why the store no longer + /// opens its own from a connection string. A raw connection + /// would lack the UDF and every write to the replicated table would fail closed. + /// /// Logger for diagnostics. + /// + /// no longer feeds this store — the + /// database location is LocalDb:Path. The option remains bound for the purge/retention + /// settings that share the class. + /// public OperationTrackingStore( - IOptions options, + ILocalDb localDb, ILogger logger) { - ArgumentNullException.ThrowIfNull(options); + ArgumentNullException.ThrowIfNull(localDb); ArgumentNullException.ThrowIfNull(logger); _logger = logger; - _connectionString = options.Value.ConnectionString; - _writeConnection = new SqliteConnection(_connectionString); - _writeConnection.Open(); + _localDb = localDb; + // Already open — CreateConnection returns a live, pragma-configured, + // UDF-registered connection. Calling Open() on it again would throw. + _writeConnection = localDb.CreateConnection(); InitializeSchema(); } // Schema lives in OperationTrackingSchema so the Host's AddZbLocalDb onReady // callback can create this table in the consolidated site database before - // RegisterReplicated installs its capture triggers. Applying it here too keeps a - // directly-constructed store (tests, tooling) self-sufficient; the DDL is - // idempotent, so running it from both places is harmless. + // RegisterReplicated installs its capture triggers. In the host this call is + // therefore always a no-op — onReady runs while ILocalDb is being constructed, + // which is strictly before this constructor can receive it. It stays because it + // keeps a directly-constructed store (tests, tooling) self-sufficient, and the + // DDL is idempotent. private void InitializeSchema() => OperationTrackingSchema.Apply(_writeConnection); /// @@ -225,14 +239,12 @@ public class OperationTrackingStore : IOperationTrackingStore, IAsyncDisposable, { ObjectDisposedException.ThrowIf(Volatile.Read(ref _disposeState) != 0, this); - // Reads open a fresh, ungated SqliteConnection so a - // long-running write doesn't block status queries. The connection - // string is shared with the writer; SQLite handles cross-connection - // isolation natively (a reader sees a consistent snapshot via the - // shared cache lock for in-memory DBs, or a WAL snapshot for file DBs). - // Mirrors the SiteStorageService precedent. - await using var readConnection = new SqliteConnection(_connectionString); - await readConnection.OpenAsync(ct).ConfigureAwait(false); + // Reads open a fresh, ungated connection so a long-running write doesn't + // block status queries. It comes from the same ILocalDb as the writer; + // SQLite handles cross-connection isolation natively (readers see a WAL + // snapshot). Mirrors the SiteStorageService precedent. Already open — do + // not call OpenAsync on it. + await using var readConnection = _localDb.CreateConnection(); await using var cmd = readConnection.CreateCommand(); cmd.CommandText = """ @@ -312,8 +324,8 @@ public class OperationTrackingStore : IOperationTrackingStore, IAsyncDisposable, // the standalone IX_OperationTracking_UpdatedAt index — UpdatedAtUtc is // the cursor. (The composite (Status, UpdatedAtUtc) index cannot satisfy a // status-less UpdatedAtUtc range scan; this dedicated index does.) - await using var readConnection = new SqliteConnection(_connectionString); - await readConnection.OpenAsync(ct).ConfigureAwait(false); + // Already open — do not call OpenAsync on it. + await using var readConnection = _localDb.CreateConnection(); await using var cmd = readConnection.CreateCommand(); diff --git a/tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/Integration/Infrastructure/CombinedTelemetryHarness.cs b/tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/Integration/Infrastructure/CombinedTelemetryHarness.cs index 3c83dd97..c7bdeff9 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/Integration/Infrastructure/CombinedTelemetryHarness.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/Integration/Infrastructure/CombinedTelemetryHarness.cs @@ -1,8 +1,10 @@ using Akka.Actor; using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Options; +using ZB.MOM.WW.LocalDb; using ZB.MOM.WW.ScadaBridge.AuditLog.Central; using ZB.MOM.WW.ScadaBridge.AuditLog.Site; using ZB.MOM.WW.ScadaBridge.AuditLog.Site.Telemetry; @@ -58,6 +60,8 @@ public sealed class CombinedTelemetryHarness : IAsyncDisposable public IServiceProvider ServiceProvider { get; } private readonly MsSqlMigrationFixture _fixture; + private readonly string _trackingDbPath; + private readonly ServiceProvider _trackingLocalDbProvider; private bool _disposed; public CombinedTelemetryHarness( @@ -88,13 +92,23 @@ public sealed class CombinedTelemetryHarness : IAsyncDisposable SqliteWriter, Ring, new NoOpAuditWriteFailureCounter(), NullLogger.Instance); + // The tracking store now runs on the consolidated ILocalDb rather than its own + // connection string, so it needs a real database FILE — LocalDbOptions.Path is + // a filesystem path and there is no in-memory mode. Unique per harness, torn + // down in DisposeAsync alongside its WAL sidecars. + _trackingDbPath = Path.Combine( + Path.GetTempPath(), $"tracking-g-{Guid.NewGuid():N}.db"); + _trackingLocalDbProvider = new ServiceCollection() + .AddZbLocalDb(new ConfigurationBuilder() + .AddInMemoryCollection(new Dictionary + { + ["LocalDb:Path"] = _trackingDbPath, + }) + .Build()) + .BuildServiceProvider(); + TrackingStore = new OperationTrackingStore( - Options.Create(new OperationTrackingOptions - { - // Same shared-in-memory pattern as the audit writer. - ConnectionString = - $"Data Source=file:tracking-g-{Guid.NewGuid():N}?mode=memory&cache=shared", - }), + _trackingLocalDbProvider.GetRequiredService(), NullLogger.Instance); // Central wiring: real repositories backed by the MSSQL fixture's DB. @@ -165,6 +179,15 @@ public sealed class CombinedTelemetryHarness : IAsyncDisposable _disposed = true; await SqliteWriter.DisposeAsync().ConfigureAwait(false); await TrackingStore.DisposeAsync().ConfigureAwait(false); + + // Dispose the ILocalDb before deleting the file — it holds an open master + // connection anchoring the WAL. + await _trackingLocalDbProvider.DisposeAsync().ConfigureAwait(false); + foreach (var suffix in new[] { "", "-wal", "-shm" }) + { + try { File.Delete(_trackingDbPath + suffix); } catch { /* best effort */ } + } + if (ServiceProvider is IAsyncDisposable asyncSp) { await asyncSp.DisposeAsync().ConfigureAwait(false); diff --git a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/AkkaHostedServiceAuditWiringTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/AkkaHostedServiceAuditWiringTests.cs index d31f27a8..31e21242 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/AkkaHostedServiceAuditWiringTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/AkkaHostedServiceAuditWiringTests.cs @@ -290,10 +290,12 @@ public class SiteAuditWiringTests : IDisposable { private readonly WebApplication _host; private readonly string _tempDbPath; + private readonly string _tempLocalDbPath; public SiteAuditWiringTests() { _tempDbPath = Path.Combine(Path.GetTempPath(), $"scadabridge_audit_wiring_{Guid.NewGuid()}.db"); + _tempLocalDbPath = Path.Combine(Path.GetTempPath(), $"scadabridge_audit_wiring_localdb_{Guid.NewGuid()}.db"); var builder = WebApplication.CreateBuilder(); builder.Configuration.Sources.Clear(); @@ -312,6 +314,11 @@ public class SiteAuditWiringTests : IDisposable // resolved; point it at an in-memory connection so the test doesn't // pollute the working directory. ["AuditLog:SiteWriter:DatabasePath"] = ":memory:", + // The cached-call telemetry forwarder pulls in IOperationTrackingStore, + // which now resolves ILocalDb — so the consolidated site database must be + // configured or the whole graph fails to resolve. There is no in-memory + // mode: LocalDbOptions.Path is a filesystem path. + ["LocalDb:Path"] = _tempLocalDbPath, }); builder.Services.AddGrpc(); @@ -326,6 +333,7 @@ public class SiteAuditWiringTests : IDisposable { (_host as IDisposable)?.Dispose(); try { File.Delete(_tempDbPath); } catch { /* best effort */ } + try { File.Delete(_tempLocalDbPath); } catch { /* best effort */ } } [Fact] diff --git a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/CompositionRootTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/CompositionRootTests.cs index 5d89eb9d..488f186f 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/CompositionRootTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/CompositionRootTests.cs @@ -383,11 +383,13 @@ public class SiteCompositionRootTests : IDisposable private readonly WebApplication _host; private readonly string _tempDbPath; private readonly string _tempTrackingDbPath; + private readonly string _tempLocalDbPath; public SiteCompositionRootTests() { _tempDbPath = Path.Combine(Path.GetTempPath(), $"scadabridge_test_{Guid.NewGuid()}.db"); _tempTrackingDbPath = Path.Combine(Path.GetTempPath(), $"scadabridge_track_{Guid.NewGuid()}.db"); + _tempLocalDbPath = Path.Combine(Path.GetTempPath(), $"scadabridge_localdb_{Guid.NewGuid()}.db"); var builder = WebApplication.CreateBuilder(); builder.Configuration.Sources.Clear(); @@ -400,10 +402,13 @@ public class SiteCompositionRootTests : IDisposable ["ScadaBridge:Node:RemotingPort"] = "0", ["ScadaBridge:Node:GrpcPort"] = "0", ["ScadaBridge:Database:SiteDbPath"] = _tempDbPath, - // Point the site-local operation-tracking SQLite store (the ctor opens/creates - // the file eagerly) at a temp path so resolving IOperationTrackingStore in the - // composition-root test does not litter site-tracking.db in the test cwd. + // Retained for the site config DB; it no longer points the tracking store + // anywhere — that store now opens the consolidated database below. ["ScadaBridge:OperationTracking:ConnectionString"] = $"Data Source={_tempTrackingDbPath}", + // The consolidated site database. IOperationTrackingStore resolves ILocalDb + // and creates the file eagerly in its ctor, so this must be a temp path or + // the composition-root test litters the working directory. + ["LocalDb:Path"] = _tempLocalDbPath, // ClusterOptions requires at least one seed node (ClusterOptionsValidator). ["ScadaBridge:Cluster:SeedNodes:0"] = "akka.tcp://scadabridge@localhost:2551", ["ScadaBridge:Cluster:SeedNodes:1"] = "akka.tcp://scadabridge@localhost:2552", @@ -427,6 +432,7 @@ public class SiteCompositionRootTests : IDisposable (_host as IDisposable)?.Dispose(); try { File.Delete(_tempDbPath); } catch { /* best effort */ } try { File.Delete(_tempTrackingDbPath); } catch { /* best effort */ } + try { File.Delete(_tempLocalDbPath); } catch { /* best effort */ } } [Fact] diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Tracking/OperationTrackingStoreTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Tracking/OperationTrackingStoreTests.cs index 8ae7ebb4..c327324b 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Tracking/OperationTrackingStoreTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Tracking/OperationTrackingStoreTests.cs @@ -1,6 +1,8 @@ using Microsoft.Data.Sqlite; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging.Abstractions; -using Microsoft.Extensions.Options; +using ZB.MOM.WW.LocalDb; using ZB.MOM.WW.ScadaBridge.Commons.Types; using ZB.MOM.WW.ScadaBridge.Commons.Types.Enums; using ZB.MOM.WW.ScadaBridge.SiteRuntime.Tracking; @@ -9,30 +11,93 @@ namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.Tracking; /// /// Audit Log #23 (M3 Bundle A — Task A2) — schema + behaviour tests for the -/// site-local . Each test uses a unique -/// shared-cache in-memory SQLite database so the store and the verifier share -/// the same store without touching disk. +/// site-local . /// -public class OperationTrackingStoreTests +/// +/// The store now takes rather than a connection string, so each +/// test gets a real ILocalDb over a unique temp FILE. The previous +/// mode=memory&cache=shared fixture is gone and cannot come back: +/// LocalDbOptions.Path is a filesystem path, and — more importantly — a raw +/// connection to a shared-cache in-memory database would not carry the +/// zb_hlc_next() UDF the capture triggers call, so testing through one would no +/// longer resemble how the store runs in the host. +/// +/// Verifier connections are plain s over the same file. +/// That is deliberate and safe: they only read, or write columns on an unregistered +/// table (this fixture never calls RegisterReplicated), so no trigger fires and +/// the missing UDF is never reached. +/// +/// +public class OperationTrackingStoreTests : IDisposable { - private static (OperationTrackingStore store, string dataSource) CreateStore( - string testName) + // Every ILocalDb and temp file created by this fixture, torn down together. The + // ILocalDb holds an open master connection anchoring the WAL, so it must be + // disposed before the file can be deleted. + private readonly List _providers = []; + private readonly List _tempFiles = []; + + public void Dispose() { - var dataSource = $"file:{testName}-{Guid.NewGuid():N}?mode=memory&cache=shared"; - var connectionString = $"Data Source={dataSource};Cache=Shared"; - var options = new OperationTrackingOptions + foreach (var provider in _providers) { - ConnectionString = connectionString, - }; - var store = new OperationTrackingStore( - Options.Create(options), - NullLogger.Instance); - return (store, dataSource); + try { provider.Dispose(); } catch { /* best effort */ } + } + + foreach (var path in _tempFiles) + { + foreach (var suffix in new[] { "", "-wal", "-shm" }) + { + try { File.Delete(path + suffix); } catch { /* best effort */ } + } + } + + GC.SuppressFinalize(this); } - private static SqliteConnection OpenVerifierConnection(string dataSource) + /// Allocates a unique temp database path, registered for cleanup. + private string NewDatabasePath(string testName) { - var connection = new SqliteConnection($"Data Source={dataSource};Cache=Shared"); + var path = Path.Combine( + Path.GetTempPath(), $"optracking-{testName}-{Guid.NewGuid():N}.db"); + _tempFiles.Add(path); + return path; + } + + /// + /// Builds a real over . No + /// onReady callback and no RegisterReplicated — the store's own + /// InitializeSchema creates the table, which is what keeps a + /// directly-constructed store self-sufficient. + /// + private ILocalDb CreateLocalDb(string databasePath) + { + var configuration = new ConfigurationBuilder() + .AddInMemoryCollection(new Dictionary + { + ["LocalDb:Path"] = databasePath, + }) + .Build(); + + var provider = new ServiceCollection() + .AddZbLocalDb(configuration) + .BuildServiceProvider(); + _providers.Add(provider); + + return provider.GetRequiredService(); + } + + private (OperationTrackingStore store, string dataSource) CreateStore(string testName) + { + var databasePath = NewDatabasePath(testName); + var store = new OperationTrackingStore( + CreateLocalDb(databasePath), + NullLogger.Instance); + return (store, databasePath); + } + + private static SqliteConnection OpenVerifierConnection(string databasePath) + { + var connection = new SqliteConnection($"Data Source={databasePath}"); connection.Open(); return connection; } @@ -111,14 +176,19 @@ public class OperationTrackingStoreTests ON OperationTracking (Status, UpdatedAtUtc); """; - private static SqliteConnection SeedPreSourceNodeSchemaDatabase(string dataSource) + /// + /// Writes the pre-SourceNode schema into and closes + /// the connection again. Unlike the old shared-cache in-memory fixture — where the + /// seeding connection had to stay open or the database evaporated — a file database + /// persists on its own, so nothing is held across the store's construction. + /// + private static void SeedPreSourceNodeSchemaDatabase(string databasePath) { - var connection = new SqliteConnection($"Data Source={dataSource};Cache=Shared"); + using var connection = new SqliteConnection($"Data Source={databasePath}"); connection.Open(); using var cmd = connection.CreateCommand(); cmd.CommandText = OldPreSourceNodeSchema; cmd.ExecuteNonQuery(); - return connection; } private static bool ColumnExists(SqliteConnection connection, string columnName) @@ -129,36 +199,36 @@ public class OperationTrackingStoreTests return Convert.ToInt32(cmd.ExecuteScalar()) > 0; } - private static OperationTrackingStore CreateStoreOver(string dataSource) - { - var connectionString = $"Data Source={dataSource};Cache=Shared"; - var options = new OperationTrackingOptions { ConnectionString = connectionString }; - return new OperationTrackingStore( - Options.Create(options), - NullLogger.Instance); - } + private OperationTrackingStore CreateStoreOver(string databasePath) + => new(CreateLocalDb(databasePath), NullLogger.Instance); [Fact] public async Task Initialize_adds_SourceNode_to_pre_existing_schema() { - var dataSource = $"file:{nameof(Initialize_adds_SourceNode_to_pre_existing_schema)}-{Guid.NewGuid():N}?mode=memory&cache=shared"; + var databasePath = NewDatabasePath(nameof(Initialize_adds_SourceNode_to_pre_existing_schema)); - // A pre-SourceNode deployment: tracking.db already exists with the - // 12-column schema and NO SourceNode column. - using var seedConnection = SeedPreSourceNodeSchemaDatabase(dataSource); - Assert.True(ColumnExists(seedConnection, "SourceInstanceId")); - Assert.True(ColumnExists(seedConnection, "SourceScript")); - Assert.False(ColumnExists(seedConnection, "SourceNode")); + // A pre-SourceNode deployment: the tracking database already exists with + // the 12-column schema and NO SourceNode column. + SeedPreSourceNodeSchemaDatabase(databasePath); + using (var seeded = OpenVerifierConnection(databasePath)) + { + Assert.True(ColumnExists(seeded, "SourceInstanceId")); + Assert.True(ColumnExists(seeded, "SourceScript")); + Assert.False(ColumnExists(seeded, "SourceNode")); + } // Upgrade: a post-branch OperationTrackingStore opens the same database. // Its InitializeSchema must ALTER the missing SourceNode column in — // the CREATE TABLE IF NOT EXISTS alone is a no-op against the existing // table. - await using (var store = CreateStoreOver(dataSource)) + await using (var store = CreateStoreOver(databasePath)) { - Assert.True( - ColumnExists(seedConnection, "SourceNode"), - "OperationTrackingStore must ALTER the SourceNode column into a pre-existing OperationTracking table."); + using (var verifier = OpenVerifierConnection(databasePath)) + { + Assert.True( + ColumnExists(verifier, "SourceNode"), + "OperationTrackingStore must ALTER the SourceNode column into a pre-existing OperationTracking table."); + } // A RecordEnqueueAsync binding $sourceNode must now succeed; without // the ALTER it would fail with "no such column: SourceNode". @@ -178,9 +248,10 @@ public class OperationTrackingStoreTests // Idempotency: a second store over the now-upgraded DB must not error // (the probe sees SourceNode already present and skips the ALTER). - await using (var storeAgain = CreateStoreOver(dataSource)) + await using (var storeAgain = CreateStoreOver(databasePath)) { - Assert.True(ColumnExists(seedConnection, "SourceNode")); + using var verifier = OpenVerifierConnection(databasePath); + Assert.True(ColumnExists(verifier, "SourceNode")); } }