diff --git a/docs/requirements/Component-AuditLog.md b/docs/requirements/Component-AuditLog.md index b8627d8..7fdeb66 100644 --- a/docs/requirements/Component-AuditLog.md +++ b/docs/requirements/Component-AuditLog.md @@ -103,7 +103,7 @@ row per lifecycle event across all channels. - `IX_AuditLog_OccurredAtUtc` — primary time-range index for global scans. - `IX_AuditLog_Site_Occurred (SourceSiteId, OccurredAtUtc)` — per-site filters. -- `IX_AuditLog_Correlation (CorrelationId)` — drilldown from a single operation. +- `IX_AuditLog_CorrelationId (CorrelationId)` — drilldown from a single operation. - `IX_AuditLog_Execution (ExecutionId)` — drilldown to every action of one script execution / inbound request. - `IX_AuditLog_Channel_Status_Occurred (Channel, Status, OccurredAtUtc)` — KPI / dashboard tiles. - `IX_AuditLog_Target_Occurred (Target, OccurredAtUtc)` — "what did we send to system X". diff --git a/src/ScadaLink.AuditLog/Site/SqliteAuditWriter.cs b/src/ScadaLink.AuditLog/Site/SqliteAuditWriter.cs index cf2b216..72c69d0 100644 --- a/src/ScadaLink.AuditLog/Site/SqliteAuditWriter.cs +++ b/src/ScadaLink.AuditLog/Site/SqliteAuditWriter.cs @@ -121,6 +121,46 @@ public class SqliteAuditWriter : IAuditWriter, ISiteAuditQueue, IAsyncDisposable ON AuditLog (ForwardState, OccurredAtUtc); """; cmd.ExecuteNonQuery(); + + // Audit Log #23 (ExecutionId): additively add the ExecutionId column. + // CREATE TABLE IF NOT EXISTS above does NOT add columns to an AuditLog + // table that already exists from a pre-ExecutionId build, so an + // auditlog.db created by an older build needs the column ALTER-ed in. + // The file is durable across restart/failover by design (7-day + // retention), so without this step every WriteAsync on an upgraded + // deployment would bind $ExecutionId against a missing column and the + // best-effort write path would silently drop every site audit row. + // SQLite has no "ADD COLUMN IF NOT EXISTS"; the column presence is + // probed first and the ALTER skipped when already there. The column is + // nullable with no default, so any row written before this migration + // reads back ExecutionId = null (back-compat). + AddColumnIfMissing("ExecutionId", "TEXT NULL"); + } + + /// + /// Audit Log #23 (ExecutionId): adds a column to AuditLog only when + /// it is not already present. SQLite lacks ADD COLUMN IF NOT EXISTS, + /// so the schema is probed via PRAGMA table_info first. Idempotent — + /// safe to run on every . Mirrors + /// StoreAndForwardStorage.AddColumnIfMissingAsync; kept synchronous + /// here to match the rest of this writer's bootstrap DDL. + /// + private void AddColumnIfMissing(string columnName, string columnDefinition) + { + using var probe = _connection.CreateCommand(); + probe.CommandText = "SELECT COUNT(*) FROM pragma_table_info('AuditLog') WHERE name = $name"; + probe.Parameters.AddWithValue("$name", columnName); + var exists = Convert.ToInt32(probe.ExecuteScalar()) > 0; + if (exists) + { + return; + } + + using var alter = _connection.CreateCommand(); + // Column name + definition are caller-controlled constants, never user + // input — safe to interpolate (parameters are not permitted in DDL). + alter.CommandText = $"ALTER TABLE AuditLog ADD COLUMN {columnName} {columnDefinition}"; + alter.ExecuteNonQuery(); } /// diff --git a/tests/ScadaLink.AuditLog.Tests/Site/SqliteAuditWriterSchemaTests.cs b/tests/ScadaLink.AuditLog.Tests/Site/SqliteAuditWriterSchemaTests.cs index e6015fd..13120e3 100644 --- a/tests/ScadaLink.AuditLog.Tests/Site/SqliteAuditWriterSchemaTests.cs +++ b/tests/ScadaLink.AuditLog.Tests/Site/SqliteAuditWriterSchemaTests.cs @@ -2,6 +2,8 @@ using Microsoft.Data.Sqlite; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Options; using ScadaLink.AuditLog.Site; +using ScadaLink.Commons.Entities.Audit; +using ScadaLink.Commons.Types.Enums; namespace ScadaLink.AuditLog.Tests.Site; @@ -125,4 +127,122 @@ public class SqliteAuditWriterSchemaTests Assert.Equal(2, value); } } + + // ----- ExecutionId schema-upgrade regression (persistent auditlog.db) ----- // + + /// + /// The OLD pre-ExecutionId-branch AuditLog schema — the 20-column + /// CREATE TABLE WITHOUT the ExecutionId column. A real deployment's + /// on-disk auditlog.db already contains exactly this shape, and + /// CREATE TABLE IF NOT EXISTS is a no-op against it. + /// + private const string OldPreExecutionIdSchema = """ + CREATE TABLE IF NOT EXISTS AuditLog ( + EventId TEXT NOT NULL, + OccurredAtUtc TEXT NOT NULL, + Channel TEXT NOT NULL, + Kind TEXT NOT NULL, + CorrelationId TEXT NULL, + SourceSiteId TEXT NULL, + SourceInstanceId TEXT NULL, + SourceScript TEXT NULL, + Actor TEXT NULL, + Target TEXT NULL, + Status TEXT NOT NULL, + HttpStatus INTEGER NULL, + DurationMs INTEGER NULL, + ErrorMessage TEXT NULL, + ErrorDetail TEXT NULL, + RequestSummary TEXT NULL, + ResponseSummary TEXT NULL, + PayloadTruncated INTEGER NOT NULL, + Extra TEXT NULL, + ForwardState TEXT NOT NULL, + PRIMARY KEY (EventId) + ); + CREATE INDEX IF NOT EXISTS IX_SiteAuditLog_ForwardState_Occurred + ON AuditLog (ForwardState, OccurredAtUtc); + """; + + /// + /// Seeds a shared-cache in-memory database with the OLD 20-column schema and + /// returns the open connection. The connection MUST stay open for the + /// lifetime of the test: a shared-cache in-memory database is dropped once + /// its last connection closes, so closing this would discard the seeded + /// schema before the writer opens its own connection. + /// + private static SqliteConnection SeedOldSchemaDatabase(string dataSource) + { + var connection = new SqliteConnection($"Data Source={dataSource};Cache=Shared"); + connection.Open(); + using var cmd = connection.CreateCommand(); + cmd.CommandText = OldPreExecutionIdSchema; + cmd.ExecuteNonQuery(); + return connection; + } + + private static SqliteAuditWriter CreateWriterOver(string dataSource) + { + var options = new SqliteAuditWriterOptions { DatabasePath = dataSource }; + return new SqliteAuditWriter( + Options.Create(options), + NullLogger.Instance, + connectionStringOverride: $"Data Source={dataSource};Cache=Shared"); + } + + private static bool ColumnExists(SqliteConnection connection, string columnName) + { + using var cmd = connection.CreateCommand(); + cmd.CommandText = "SELECT COUNT(*) FROM pragma_table_info('AuditLog') WHERE name = $name"; + cmd.Parameters.AddWithValue("$name", columnName); + return Convert.ToInt32(cmd.ExecuteScalar()) > 0; + } + + [Fact] + public async Task Opening_Over_PreExisting_OldSchema_Db_Adds_ExecutionId_Column_And_WriteAsync_RoundTrips() + { + var dataSource = $"file:{nameof(Opening_Over_PreExisting_OldSchema_Db_Adds_ExecutionId_Column_And_WriteAsync_RoundTrips)}-{Guid.NewGuid():N}?mode=memory&cache=shared"; + + // A pre-branch deployment: auditlog.db already exists with the 20-column + // schema and NO ExecutionId column. + using var seedConnection = SeedOldSchemaDatabase(dataSource); + Assert.False(ColumnExists(seedConnection, "ExecutionId")); + + // Upgrade: a post-branch SqliteAuditWriter opens the same database. Its + // InitializeSchema must ALTER the missing ExecutionId column in — the + // CREATE TABLE IF NOT EXISTS alone is a no-op against the existing table. + var executionId = Guid.NewGuid(); + await using (var writer = CreateWriterOver(dataSource)) + { + Assert.True( + ColumnExists(seedConnection, "ExecutionId"), + "SqliteAuditWriter must ALTER the ExecutionId column into a pre-existing AuditLog table."); + + // A WriteAsync binding $ExecutionId must now succeed and round-trip; + // without the ALTER it would fail with "no such column: ExecutionId" + // and — because audit writes are best-effort — silently drop the row. + var evt = new AuditEvent + { + EventId = Guid.NewGuid(), + OccurredAtUtc = DateTime.UtcNow, + Channel = AuditChannel.ApiOutbound, + Kind = AuditKind.ApiCall, + Status = AuditStatus.Delivered, + PayloadTruncated = false, + ExecutionId = executionId, + }; + await writer.WriteAsync(evt); + + var rows = await writer.ReadPendingAsync(limit: 10); + var row = Assert.Single(rows); + Assert.Equal(executionId, row.ExecutionId); + } + + // Idempotency: a second writer over the now-upgraded DB must not error + // (the probe sees ExecutionId already present and skips the ALTER). + await using (var writerAgain = CreateWriterOver(dataSource)) + { + Assert.True(ColumnExists(seedConnection, "ExecutionId")); + } + } }