dohertj2/ScadaBridge
1
0
Fork 0
Code Issues 2 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

feat(localdb): rewire OperationTrackingStore onto the consolidated site database

Browse Source 
Task 4 of the LocalDb Phase 1 adoption plan.

OperationTrackingStore took IOptions<OperationTrackingOptions> and opened its own
SqliteConnection from ConnectionString. It now takes ILocalDb and gets every
connection - writer and the two ad-hoc reader paths - from CreateConnection().

This is not cosmetic. OperationTracking is a RegisterReplicated table, so its
capture triggers call zb_hlc_next(). That UDF is registered per connection by
ILocalDb and by nothing else, so a raw SqliteConnection would fail closed on
every write. Connections from CreateConnection() also arrive already open -
calling Open()/OpenAsync() on one throws - hence the removed OpenAsync calls on
the reader paths.

OperationTrackingOptions.ConnectionString is now vestigial for this store; the
database location is LocalDb:Path. The options class stays (retention settings)
and the config key stays bound for the site config DB.

InitializeSchema is kept but is now always a no-op in the host: onReady runs
while ILocalDb is being constructed, strictly before this constructor can
receive it. It remains so a directly-constructed store (tests, tooling) is
self-sufficient.

Tests: the store fixture moves off mode=memory&cache=shared onto a real ILocalDb
over a temp file. There is no in-memory mode - LocalDbOptions.Path is a
filesystem path - and testing through a raw in-memory connection would no longer
resemble the host. Verifier connections stay raw on purpose: this fixture never
registers the table, so no trigger fires and the UDF is never reached.

Three DI fixtures needed LocalDb:Path added, because resolving
IOperationTrackingStore now forces ILocalDb construction:
SiteCompositionRootTests, SiteAuditWiringTests, and the AuditLog
CombinedTelemetryHarness.

Verified: build 0 warnings; Host 294/294, SiteRuntime 530/530, AuditLog 354/354.

Claude-Session: https://claude.ai/code/session_01BL2Vu1ESDQ9SCN4gVKkdts
This commit is contained in:
Joseph Doherty
2026-07-19 08:59:38 -04:00
parent 2bff527247
commit 0b5e9b44f6
6 changed files with 198 additions and 74 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/Integration/Infrastructure/CombinedTelemetryHarness.cs
+29 -6
View File
@@ -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<FallbackAuditWriter>.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<string, string?>
{
["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<ILocalDb>(),
NullLogger<OperationTrackingStore>.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);
tests/ZB.MOM.WW.ScadaBridge.Host.Tests/AkkaHostedServiceAuditWiringTests.cs
+8
View File
@@ -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]
tests/ZB.MOM.WW.ScadaBridge.Host.Tests/CompositionRootTests.cs
+9 -3
View File
@@ -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]
tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Tracking/OperationTrackingStoreTests.cs
+113 -42
View File
@@ -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;
/// <summary>
/// Audit Log #23 (M3 Bundle A — Task A2) — schema + behaviour tests for the
/// site-local <see cref="OperationTrackingStore"/>. 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 <see cref="OperationTrackingStore"/>.
/// </summary>
public class OperationTrackingStoreTests
/// <remarks>
/// The store now takes <see cref="ILocalDb"/> rather than a connection string, so each
/// test gets a real <c>ILocalDb</c> over a unique temp FILE. The previous
/// <c>mode=memory&amp;cache=shared</c> fixture is gone and cannot come back:
/// <c>LocalDbOptions.Path</c> is a filesystem path, and — more importantly — a raw
/// connection to a shared-cache in-memory database would not carry the
/// <c>zb_hlc_next()</c> UDF the capture triggers call, so testing through one would no
/// longer resemble how the store runs in the host.
/// <para>
/// Verifier connections are plain <see cref="SqliteConnection"/>s over the same file.
/// That is deliberate and safe: they only read, or write columns on an unregistered
/// table (this fixture never calls <c>RegisterReplicated</c>), so no trigger fires and
/// the missing UDF is never reached.
/// </para>
/// </remarks>
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<ServiceProvider> _providers = [];
private readonly List<string> _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<OperationTrackingStore>.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)
/// <summary>Allocates a unique temp database path, registered for cleanup.</summary>
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;
}
/// <summary>
/// Builds a real <see cref="ILocalDb"/> over <paramref name="databasePath"/>. No
/// <c>onReady</c> callback and no <c>RegisterReplicated</c> — the store's own
/// <c>InitializeSchema</c> creates the table, which is what keeps a
/// directly-constructed store self-sufficient.
/// </summary>
private ILocalDb CreateLocalDb(string databasePath)
{
var configuration = new ConfigurationBuilder()
.AddInMemoryCollection(new Dictionary<string, string?>
{
["LocalDb:Path"] = databasePath,
})
.Build();
var provider = new ServiceCollection()
.AddZbLocalDb(configuration)
.BuildServiceProvider();
_providers.Add(provider);
return provider.GetRequiredService<ILocalDb>();
}
private (OperationTrackingStore store, string dataSource) CreateStore(string testName)
{
var databasePath = NewDatabasePath(testName);
var store = new OperationTrackingStore(
CreateLocalDb(databasePath),
NullLogger<OperationTrackingStore>.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)
/// <summary>
/// Writes the pre-SourceNode schema into <paramref name="databasePath"/> 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.
/// </summary>
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<OperationTrackingStore>.Instance);
}
private OperationTrackingStore CreateStoreOver(string databasePath)
=> new(CreateLocalDb(databasePath), NullLogger<OperationTrackingStore>.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"));
}
}