diff --git a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Monitoring/EventLogs.razor b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Monitoring/EventLogs.razor index 83adeefb..357f4d80 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Monitoring/EventLogs.razor +++ b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Monitoring/EventLogs.razor @@ -178,7 +178,10 @@ private List? _entries; private bool _hasMore; - private long? _continuationToken; + // Opaque keyset cursor from the previous response — passed back verbatim, never + // parsed. It became a string in LocalDb Phase 1 when site_events ids turned into + // GUIDs and the cursor had to carry the timestamp too. + private string? _continuationToken; private bool _searching; private string? _errorMessage; private ToastNotification _toast = default!; diff --git a/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/RemoteQuery/EventLogQueryRequest.cs b/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/RemoteQuery/EventLogQueryRequest.cs index a7493f6e..628cb4bf 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/RemoteQuery/EventLogQueryRequest.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/RemoteQuery/EventLogQueryRequest.cs @@ -3,13 +3,19 @@ namespace ZB.MOM.WW.ScadaBridge.Commons.Messages.RemoteQuery; /// /// Request to query site event logs from central. /// Supports filtering by event type, severity, instance, time range, and keyword search. -/// Uses keyset pagination via continuation token (last event ID). +/// Uses keyset pagination via an opaque continuation token. /// /// /// Instance filter matched against the site event log's instance_id column, /// which stores the instance UniqueName (InstanceActor.LogLifecycleEvent passes /// _instanceUniqueName; EventLogQueryService matches instance_id = $instanceId). /// +/// +/// Opaque cursor from the previous response's ContinuationToken, or +/// to start from the oldest matching event. Treat as opaque — +/// it encodes timestamp and id together and its format is not part of the contract. +/// An unparseable token is treated as "start from the beginning" rather than an error. +/// public record EventLogQueryRequest( string CorrelationId, string SiteId, @@ -19,6 +25,6 @@ public record EventLogQueryRequest( string? Severity, string? InstanceId, string? KeywordFilter, - long? ContinuationToken, + string? ContinuationToken, int PageSize, DateTimeOffset Timestamp); diff --git a/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/RemoteQuery/EventLogQueryResponse.cs b/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/RemoteQuery/EventLogQueryResponse.cs index f807d373..8e4a8c78 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/RemoteQuery/EventLogQueryResponse.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/RemoteQuery/EventLogQueryResponse.cs @@ -3,8 +3,18 @@ namespace ZB.MOM.WW.ScadaBridge.Commons.Messages.RemoteQuery; /// /// A single event log entry returned from a site query. /// +/// +/// The event's primary key: a GUID string minted by the recording site node. +/// +/// This was a long autoincrement id until LocalDb Phase 1. Site pairs now +/// replicate site_events with last-writer-wins on this key, so a +/// server-minted integer would have both nodes issuing the same ids for unrelated +/// events and silently overwriting each other on sync. Consumers must treat it as +/// an opaque identifier — it carries no ordering. +/// +/// public record EventLogEntry( - long Id, + string Id, DateTimeOffset Timestamp, string EventType, string Severity, @@ -15,13 +25,18 @@ public record EventLogEntry( /// /// Response containing paginated event log entries from a site. -/// Uses keyset pagination: ContinuationToken is the last event ID in the result set. /// +/// +/// Opaque keyset-pagination cursor: pass it back verbatim on the next request to +/// continue after the last returned row, or to start from the +/// beginning. Encodes timestamp and id together, because GUID ids do not +/// sort chronologically and timestamps alone are not unique. Do not parse it. +/// public record EventLogQueryResponse( string CorrelationId, string SiteId, IReadOnlyList Entries, - long? ContinuationToken, + string? ContinuationToken, bool HasMore, bool Success, string? ErrorMessage, diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteEventLogging/EventLogPurgeService.cs b/src/ZB.MOM.WW.ScadaBridge.SiteEventLogging/EventLogPurgeService.cs index 1bc3431b..5668d75f 100644 --- a/src/ZB.MOM.WW.ScadaBridge.SiteEventLogging/EventLogPurgeService.cs +++ b/src/ZB.MOM.WW.ScadaBridge.SiteEventLogging/EventLogPurgeService.cs @@ -159,9 +159,16 @@ public class EventLogPurgeService : BackgroundService var deleted = _eventLogger.WithConnection(connection => { using var cmd = connection.CreateCommand(); + // Order by timestamp, NOT by id. The id was a monotonic autoincrement + // when this was written, so "lowest id" meant "oldest"; since LocalDb + // Phase 1 it is a GUID and ordering by it would delete an ARBITRARY + // batch of events rather than the oldest ones. id remains in the ORDER + // BY only as a tie-break so the batch is deterministic. cmd.CommandText = $""" DELETE FROM site_events WHERE id IN ( - SELECT id FROM site_events ORDER BY id ASC LIMIT {CapPurgeBatchSize} + SELECT id FROM site_events + ORDER BY timestamp ASC, id ASC + LIMIT {CapPurgeBatchSize} ) """; var rows = cmd.ExecuteNonQuery(); diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteEventLogging/EventLogQueryService.cs b/src/ZB.MOM.WW.ScadaBridge.SiteEventLogging/EventLogQueryService.cs index c2685dd5..e7f0f0a4 100644 --- a/src/ZB.MOM.WW.ScadaBridge.SiteEventLogging/EventLogQueryService.cs +++ b/src/ZB.MOM.WW.ScadaBridge.SiteEventLogging/EventLogQueryService.cs @@ -48,6 +48,52 @@ public class EventLogQueryService : IEventLogQueryService .Replace("_", "\\_"); } + /// + /// Separator between the timestamp and id halves of a continuation token. Chosen + /// because it cannot occur in an ISO 8601 "o" timestamp or in a GUID "N" string, + /// so the split is unambiguous. + /// + private const char TokenSeparator = '|'; + + /// + /// Builds the opaque keyset cursor for the last row of a page. The token carries + /// the exact stored timestamp string (not a re-formatted ) + /// so it compares byte-for-byte against the column under SQLite's BINARY collation. + /// + private static string FormatContinuationToken(EventLogEntry last) => + string.Concat( + last.Timestamp.ToUniversalTime().ToString("o", CultureInfo.InvariantCulture), + TokenSeparator, + last.Id); + + /// + /// Parses a continuation token back into its timestamp and id halves. + /// Returns for null, empty, or malformed tokens — including + /// a legacy numeric token from a client that predates the GUID id change — in which + /// case the caller starts from the beginning instead of failing the query. + /// + private static bool TryParseContinuationToken( + string? token, out string timestamp, out string id) + { + timestamp = ""; + id = ""; + + if (string.IsNullOrWhiteSpace(token)) + { + return false; + } + + var separator = token.IndexOf(TokenSeparator); + if (separator <= 0 || separator == token.Length - 1) + { + return false; + } + + timestamp = token[..separator]; + id = token[(separator + 1)..]; + return true; + } + /// public EventLogQueryResponse ExecuteQuery(EventLogQueryRequest request) { @@ -64,11 +110,23 @@ public class EventLogQueryService : IEventLogQueryService var whereClauses = new List(); var parameters = new List(); - // Keyset pagination: only return events with id > continuation token - if (request.ContinuationToken.HasValue) + // Keyset pagination on the composite (timestamp, id) key. + // + // This used to be a plain "id > $afterId" against a monotonic autoincrement + // id. Since LocalDb Phase 1 the id is a GUID, which does not sort + // chronologically — a naive "id > x" would return an arbitrary subset and + // SILENTLY DROP ROWS from the page-through. Timestamps alone are not unique + // (two events can share a tick), so the tie is broken on id to guarantee a + // total order and exactly-once paging. + // + // A malformed token is ignored rather than throwing: an old long-typed token + // from a client mid-upgrade degrades to "start from the beginning", which is + // a visible repeat rather than silent data loss. + if (TryParseContinuationToken(request.ContinuationToken, out var afterTs, out var afterId)) { - whereClauses.Add("id > $afterId"); - parameters.Add(new SqliteParameter("$afterId", request.ContinuationToken.Value)); + whereClauses.Add("(timestamp > $afterTs OR (timestamp = $afterTs AND id > $afterId))"); + parameters.Add(new SqliteParameter("$afterTs", afterTs)); + parameters.Add(new SqliteParameter("$afterId", afterId)); } if (request.From.HasValue) @@ -138,7 +196,7 @@ public class EventLogQueryService : IEventLogQueryService SELECT id, timestamp, event_type, severity, instance_id, source, message, details FROM site_events {whereClause} - ORDER BY id ASC + ORDER BY timestamp ASC, id ASC LIMIT $limit """; cmd.Parameters.AddWithValue("$limit", pageSize + 1); @@ -150,7 +208,7 @@ public class EventLogQueryService : IEventLogQueryService while (reader.Read()) { rows.Add(new EventLogEntry( - Id: reader.GetInt64(0), + Id: reader.GetString(0), // Parse with explicit invariant culture and round-trip style. // Stored values are ISO 8601 "o" UTC // (see SiteEventLogger.LogEventAsync), and the recorder's @@ -178,7 +236,9 @@ public class EventLogQueryService : IEventLogQueryService entries.RemoveAt(entries.Count - 1); } - var continuationToken = entries.Count > 0 ? entries[^1].Id : (long?)null; + var continuationToken = entries.Count > 0 + ? FormatContinuationToken(entries[^1]) + : null; return new EventLogQueryResponse( CorrelationId: request.CorrelationId, diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteEventLogging/SiteEventLogSchema.cs b/src/ZB.MOM.WW.ScadaBridge.SiteEventLogging/SiteEventLogSchema.cs new file mode 100644 index 00000000..cde099b0 --- /dev/null +++ b/src/ZB.MOM.WW.ScadaBridge.SiteEventLogging/SiteEventLogSchema.cs @@ -0,0 +1,71 @@ +using Microsoft.Data.Sqlite; + +namespace ZB.MOM.WW.ScadaBridge.SiteEventLogging; + +/// +/// DDL for the site_events table, extracted from so +/// it can be applied by whoever owns the database file. +/// +/// Under LocalDb Phase 1 that owner is the Host's AddZbLocalDb onReady callback: +/// the table must exist before ILocalDb.RegisterReplicated installs its capture +/// triggers, and rows written before registration are never captured or snapshotted. +/// +/// +/// Schema change. The primary key is a TEXT GUID minted by the writer, replacing +/// the historical INTEGER PRIMARY KEY AUTOINCREMENT. This is required, not +/// cosmetic: replication resolves conflicts by last-writer-wins on the primary key, so +/// two site nodes each minting autoincrement id 1 for unrelated events would be treated +/// as one row and silently overwrite each other. GUID keys make the event log a pure +/// union across the pair. Consumers that relied on the id being monotonic — the keyset +/// paging cursor and the storage-cap purge — order by timestamp instead. +/// +/// +/// Deliberately depends only on Microsoft.Data.Sqlite, not on the LocalDb +/// library — the Host needs to apply this DDL to a LocalDb-managed connection, but +/// nothing about the schema itself is LocalDb-specific. +/// +/// +public static class SiteEventLogSchema +{ + /// + /// Creates the site_events table and its indexes when absent. Idempotent — + /// safe to run on every startup. + /// + /// An open connection to the database that should hold the table. + public static void Apply(SqliteConnection connection) + { + ArgumentNullException.ThrowIfNull(connection); + + // auto_vacuum must be set before any table is created for it to take effect + // on a fresh database. With INCREMENTAL mode, PRAGMA incremental_vacuum can + // later reclaim free pages so the storage-cap purge can shrink the file. + using (var pragmaCmd = connection.CreateCommand()) + { + pragmaCmd.CommandText = "PRAGMA auto_vacuum = INCREMENTAL"; + pragmaCmd.ExecuteNonQuery(); + } + + using var cmd = connection.CreateCommand(); + cmd.CommandText = """ + CREATE TABLE IF NOT EXISTS site_events ( + id TEXT NOT NULL PRIMARY KEY, + timestamp TEXT NOT NULL, + event_type TEXT NOT NULL, + severity TEXT NOT NULL, + instance_id TEXT, + source TEXT NOT NULL, + message TEXT NOT NULL, + details TEXT + ); + CREATE INDEX IF NOT EXISTS idx_events_timestamp ON site_events(timestamp); + CREATE INDEX IF NOT EXISTS idx_events_type ON site_events(event_type); + CREATE INDEX IF NOT EXISTS idx_events_instance ON site_events(instance_id); + CREATE INDEX IF NOT EXISTS idx_events_severity ON site_events(severity); + """; + // The query service also supports keyword search via leading-wildcard + // LIKE on message/source. A leading-wildcard LIKE cannot use a B-tree + // index, so that path intentionally full-scans; severity/event_type/ + // instance_id/timestamp filters above are all covered. + cmd.ExecuteNonQuery(); + } +} diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteEventLogging/SiteEventLogger.cs b/src/ZB.MOM.WW.ScadaBridge.SiteEventLogging/SiteEventLogger.cs index 23ce86e5..53ff5add 100644 --- a/src/ZB.MOM.WW.ScadaBridge.SiteEventLogging/SiteEventLogger.cs +++ b/src/ZB.MOM.WW.ScadaBridge.SiteEventLogging/SiteEventLogger.cs @@ -130,40 +130,11 @@ public class SiteEventLogger : ISiteEventLogger, IDisposable } } - private void InitializeSchema() - { - // auto_vacuum must be set before any table is created for it to take effect - // on a fresh database. With INCREMENTAL mode, PRAGMA incremental_vacuum can - // later reclaim free pages so the storage-cap purge can shrink the file. - using (var pragmaCmd = _connection.CreateCommand()) - { - pragmaCmd.CommandText = "PRAGMA auto_vacuum = INCREMENTAL"; - pragmaCmd.ExecuteNonQuery(); - } - - using var cmd = _connection.CreateCommand(); - cmd.CommandText = """ - CREATE TABLE IF NOT EXISTS site_events ( - id INTEGER PRIMARY KEY AUTOINCREMENT, - timestamp TEXT NOT NULL, - event_type TEXT NOT NULL, - severity TEXT NOT NULL, - instance_id TEXT, - source TEXT NOT NULL, - message TEXT NOT NULL, - details TEXT - ); - CREATE INDEX IF NOT EXISTS idx_events_timestamp ON site_events(timestamp); - CREATE INDEX IF NOT EXISTS idx_events_type ON site_events(event_type); - CREATE INDEX IF NOT EXISTS idx_events_instance ON site_events(instance_id); - CREATE INDEX IF NOT EXISTS idx_events_severity ON site_events(severity); - """; - // The query service also supports keyword search via leading-wildcard - // LIKE on message/source. A leading-wildcard LIKE cannot use a B-tree - // index, so that path intentionally full-scans; severity/event_type/ - // instance_id/timestamp filters above are all covered. - cmd.ExecuteNonQuery(); - } + // Schema lives in SiteEventLogSchema 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 + // logger (tests, tooling) self-sufficient; the DDL is idempotent. + private void InitializeSchema() => SiteEventLogSchema.Apply(_connection); /// /// Closed set of allowed severities. Case-sensitive to @@ -198,7 +169,13 @@ public class SiteEventLogger : ISiteEventLogger, IDisposable nameof(severity)); } + // The id is minted here, by the application, rather than by SQLite. Under + // LocalDb replication the site pair converges by last-writer-wins on the + // primary key, so a server-minted AUTOINCREMENT would have both nodes + // independently issuing id 1, 2, 3... for unrelated events and silently + // overwriting each other on sync. A GUID makes the event log a pure union. var pending = new PendingEvent( + Guid.NewGuid().ToString("N"), DateTimeOffset.UtcNow.ToString("o"), eventType, severity, @@ -235,9 +212,10 @@ public class SiteEventLogger : ISiteEventLogger, IDisposable { using var cmd = connection.CreateCommand(); cmd.CommandText = """ - INSERT INTO site_events (timestamp, event_type, severity, instance_id, source, message, details) - VALUES ($timestamp, $event_type, $severity, $instance_id, $source, $message, $details) + INSERT INTO site_events (id, timestamp, event_type, severity, instance_id, source, message, details) + VALUES ($id, $timestamp, $event_type, $severity, $instance_id, $source, $message, $details) """; + cmd.Parameters.AddWithValue("$id", pending.Id); cmd.Parameters.AddWithValue("$timestamp", pending.Timestamp); cmd.Parameters.AddWithValue("$event_type", pending.EventType); cmd.Parameters.AddWithValue("$severity", pending.Severity); @@ -311,6 +289,7 @@ public class SiteEventLogger : ISiteEventLogger, IDisposable /// An event awaiting persistence by the background writer. private sealed record PendingEvent( + string Id, string Timestamp, string EventType, string Severity, diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Tracking/OperationTrackingSchema.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Tracking/OperationTrackingSchema.cs new file mode 100644 index 00000000..3cbd65e1 --- /dev/null +++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Tracking/OperationTrackingSchema.cs @@ -0,0 +1,105 @@ +using Microsoft.Data.Sqlite; + +namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Tracking; + +/// +/// DDL for the OperationTracking table, extracted from +/// so it can be applied by whoever owns the +/// database file. +/// +/// Under LocalDb Phase 1 that owner is the Host's AddZbLocalDb onReady callback: +/// the table must exist before ILocalDb.RegisterReplicated installs its capture +/// triggers, and rows written before registration are never captured or snapshotted. +/// The store still calls this during its own initialization, so a store constructed +/// directly (tests, tooling) remains self-sufficient. +/// +/// +/// Deliberately depends only on Microsoft.Data.Sqlite, not on the LocalDb +/// library — the Host needs to apply this DDL to a LocalDb-managed connection, but +/// nothing about the schema itself is LocalDb-specific. +/// +/// +public static class OperationTrackingSchema +{ + /// + /// Creates the OperationTracking table and its indexes when absent, and + /// additively upgrades a table created by an older build. Idempotent — safe to run + /// on every startup. + /// + /// An open connection to the database that should hold the table. + public static void Apply(SqliteConnection connection) + { + ArgumentNullException.ThrowIfNull(connection); + + using (var cmd = connection.CreateCommand()) + { + // TrackedOperationId is a TEXT GUID, which is also what lets this table + // replicate as-is: RegisterReplicated requires an explicit primary key and + // rejects BLOB columns, and there are none here. + cmd.CommandText = """ + CREATE TABLE IF NOT EXISTS OperationTracking ( + TrackedOperationId TEXT NOT NULL PRIMARY KEY, + Kind TEXT NOT NULL, + TargetSummary TEXT NULL, + Status TEXT NOT NULL, + RetryCount INTEGER NOT NULL DEFAULT 0, + LastError TEXT NULL, + HttpStatus INTEGER NULL, + CreatedAtUtc TEXT NOT NULL, + UpdatedAtUtc TEXT NOT NULL, + TerminalAtUtc TEXT NULL, + SourceInstanceId TEXT NULL, + SourceScript TEXT NULL, + SourceNode TEXT NULL + ); + CREATE INDEX IF NOT EXISTS IX_OperationTracking_Status_Updated + ON OperationTracking (Status, UpdatedAtUtc); + CREATE INDEX IF NOT EXISTS IX_OperationTracking_UpdatedAt + ON OperationTracking (UpdatedAtUtc); + """; + cmd.ExecuteNonQuery(); + } + + // SourceNode stamping: additively add the SourceNode column. + // CREATE TABLE IF NOT EXISTS above does NOT add columns to an + // OperationTracking table that already exists from a pre-SourceNode + // build, so a tracking.db created by an older build needs the column + // ALTER-ed in. The file is durable across restart/failover by design + // (retention window default 7 days), so without this step every + // RecordEnqueueAsync on an upgraded deployment would bind $sourceNode + // against a missing column and the write would fail. + // 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 SourceNode = null (back-compat). + AddColumnIfMissing(connection, "SourceNode", "TEXT NULL"); + } + + /// + /// Additively adds a column to OperationTracking only when it is not + /// already present. SQLite lacks ADD COLUMN IF NOT EXISTS, so the + /// schema is probed via PRAGMA table_info first. Mirrors the + /// SqliteAuditWriter.AddColumnIfMissing precedent. + /// + private static void AddColumnIfMissing( + SqliteConnection connection, string columnName, string columnDefinition) + { + using (var probe = connection.CreateCommand()) + { + probe.CommandText = + "SELECT COUNT(*) FROM pragma_table_info('OperationTracking') WHERE name = $name"; + probe.Parameters.AddWithValue("$name", columnName); + if (Convert.ToInt32(probe.ExecuteScalar()) > 0) + { + 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 OperationTracking ADD COLUMN {columnName} {columnDefinition}"; + alter.ExecuteNonQuery(); + } +} diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Tracking/OperationTrackingStore.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Tracking/OperationTrackingStore.cs index db2d4462..1257ea10 100644 --- a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Tracking/OperationTrackingStore.cs +++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Tracking/OperationTrackingStore.cs @@ -69,76 +69,12 @@ public class OperationTrackingStore : IOperationTrackingStore, IAsyncDisposable, InitializeSchema(); } - private void InitializeSchema() - { - using var cmd = _writeConnection.CreateCommand(); - cmd.CommandText = """ - CREATE TABLE IF NOT EXISTS OperationTracking ( - TrackedOperationId TEXT NOT NULL PRIMARY KEY, - Kind TEXT NOT NULL, - TargetSummary TEXT NULL, - Status TEXT NOT NULL, - RetryCount INTEGER NOT NULL DEFAULT 0, - LastError TEXT NULL, - HttpStatus INTEGER NULL, - CreatedAtUtc TEXT NOT NULL, - UpdatedAtUtc TEXT NOT NULL, - TerminalAtUtc TEXT NULL, - SourceInstanceId TEXT NULL, - SourceScript TEXT NULL, - SourceNode TEXT NULL - ); - CREATE INDEX IF NOT EXISTS IX_OperationTracking_Status_Updated - ON OperationTracking (Status, UpdatedAtUtc); - CREATE INDEX IF NOT EXISTS IX_OperationTracking_UpdatedAt - ON OperationTracking (UpdatedAtUtc); - """; - cmd.ExecuteNonQuery(); - - // SourceNode stamping: additively add the SourceNode column. - // CREATE TABLE IF NOT EXISTS above does NOT add columns to an - // OperationTracking table that already exists from a pre-SourceNode - // build, so a tracking.db created by an older build needs the column - // ALTER-ed in. The file is durable across restart/failover by design - // (retention window default 7 days), so without this step every - // RecordEnqueueAsync on an upgraded deployment would bind $sourceNode - // against a missing column and the write would fail. - // 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 SourceNode = null (back-compat). - // - // NOTE: This is the FIRST idempotent column-upgrade in - // OperationTrackingStore — prior schema changes pre-dated any - // production rollout and relied solely on CREATE TABLE IF NOT EXISTS. - // The helper mirrors the SqliteAuditWriter precedent. - AddColumnIfMissing("SourceNode", "TEXT NULL"); - } - - /// - /// Additively adds a column to OperationTracking 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 the - /// SqliteAuditWriter.AddColumnIfMissing precedent. - /// - private void AddColumnIfMissing(string columnName, string columnDefinition) - { - using var probe = _writeConnection.CreateCommand(); - probe.CommandText = "SELECT COUNT(*) FROM pragma_table_info('OperationTracking') WHERE name = $name"; - probe.Parameters.AddWithValue("$name", columnName); - var exists = Convert.ToInt32(probe.ExecuteScalar()) > 0; - if (exists) - { - return; - } - - using var alter = _writeConnection.CreateCommand(); - // Column name + definition are caller-controlled constants, never user - // input — safe to interpolate (parameters are not permitted in DDL). - alter.CommandText = $"ALTER TABLE OperationTracking ADD COLUMN {columnName} {columnDefinition}"; - alter.ExecuteNonQuery(); - } + // 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. + private void InitializeSchema() => OperationTrackingSchema.Apply(_writeConnection); /// public async Task RecordEnqueueAsync( diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteEventLogging.Tests/EventLogPurgeServiceTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteEventLogging.Tests/EventLogPurgeServiceTests.cs index dbd7522a..8725d83a 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.SiteEventLogging.Tests/EventLogPurgeServiceTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteEventLogging.Tests/EventLogPurgeServiceTests.cs @@ -56,9 +56,10 @@ public class EventLogPurgeServiceTests : IDisposable { using var cmd = connection.CreateCommand(); cmd.CommandText = """ - INSERT INTO site_events (timestamp, event_type, severity, source, message) - VALUES ($ts, 'script', 'Info', 'Test', 'Test message') + INSERT INTO site_events (id, timestamp, event_type, severity, source, message) + VALUES ($id, $ts, 'script', 'Info', 'Test', 'Test message') """; + cmd.Parameters.AddWithValue("$id", Guid.NewGuid().ToString("N")); cmd.Parameters.AddWithValue("$ts", timestamp.ToString("o")); cmd.ExecuteNonQuery(); }); @@ -135,6 +136,29 @@ public class EventLogPurgeServiceTests : IDisposable Assert.True(size > 0); } + /// + /// Base instant for bulk-seeded events: one day ago, evaluated once so a run is + /// self-consistent. + /// + /// Must stay INSIDE the retention window (RetentionDays, default 30). + /// applies the retention purge before + /// the storage-cap purge, so a fixed calendar date in the past would be deleted + /// wholesale as stale and the storage-cap assertions would silently test an empty + /// table rather than cap-trimming behaviour. + /// + /// + private static readonly DateTimeOffset BulkSeedStart = + DateTimeOffset.UtcNow.AddDays(-1); + + /// + /// Timestamp of the i-th bulk-seeded event. Each row is one second newer than the + /// last, so "oldest" is unambiguous — the storage-cap purge orders by timestamp, + /// and previously every bulk row shared the same UtcNow, which left the + /// oldest-first guarantee untestable. + /// + private static DateTimeOffset BulkSeedTimestamp(int index) => + BulkSeedStart.AddSeconds(index); + private void InsertBulkEvents(int count) { // Each event carries a sizeable details payload so the database grows @@ -146,24 +170,30 @@ public class EventLogPurgeServiceTests : IDisposable { using var cmd = connection.CreateCommand(); cmd.CommandText = """ - INSERT INTO site_events (timestamp, event_type, severity, source, message, details) - VALUES ($ts, 'script', 'Info', 'Test', 'Bulk event', $details) + INSERT INTO site_events (id, timestamp, event_type, severity, source, message, details) + VALUES ($id, $ts, 'script', 'Info', 'Test', 'Bulk event', $details) """; - cmd.Parameters.AddWithValue("$ts", DateTimeOffset.UtcNow.ToString("o")); + cmd.Parameters.AddWithValue("$id", Guid.NewGuid().ToString("N")); + cmd.Parameters.AddWithValue("$ts", BulkSeedTimestamp(i).ToString("o")); cmd.Parameters.AddWithValue("$details", details); cmd.ExecuteNonQuery(); } }); } - private long MinEventId() + /// + /// Oldest surviving event timestamp. Replaces the former MIN(id) probe: + /// ids are GUIDs since LocalDb Phase 1 and carry no ordering, so age is measured + /// on the timestamp column the purge itself orders by. + /// + private string MinEventTimestamp() { return _eventLogger.WithConnection(connection => { using var cmd = connection.CreateCommand(); - cmd.CommandText = "SELECT MIN(id) FROM site_events"; + cmd.CommandText = "SELECT MIN(timestamp) FROM site_events"; var result = cmd.ExecuteScalar(); - return result is long l ? l : 0; + return result as string ?? ""; }); } @@ -204,9 +234,15 @@ public class EventLogPurgeServiceTests : IDisposable [Fact] public void PurgeByStorageCap_RemovesOldestEventsFirst() { - // Regression test for SiteEventLogging-002: only the oldest events - // (lowest ids) should be removed when trimming to the cap. - InsertBulkEvents(3000); + // Regression test for SiteEventLogging-002: only the oldest events should be + // removed when trimming to the cap. + // + // "Oldest" used to mean "lowest autoincrement id". Since LocalDb Phase 1 the id + // is a GUID with no ordering, so both the purge and this test key on timestamp. + // Ordering by id here would delete an arbitrary batch, which is precisely the + // regression this test now guards. + const int eventCount = 3000; + InsertBulkEvents(eventCount); var purge = CreatePurgeService(); var totalSize = purge.GetDatabaseSizeBytes(); @@ -218,20 +254,23 @@ public class EventLogPurgeServiceTests : IDisposable MaxStorageMb = (int)Math.Max(1, (totalSize / 2) / (1024 * 1024)) }; - var minIdBefore = MinEventId(); + var oldestBefore = MinEventTimestamp(); var cappedPurge = CreatePurgeService(capOptions); cappedPurge.RunPurge(); - var minIdAfter = MinEventId(); + var oldestAfter = MinEventTimestamp(); - // The surviving rows must be the newest ones — minimum id has advanced. - Assert.True(minIdAfter > minIdBefore, - "Oldest events (lowest ids) must be purged first."); + // The surviving rows must be the newest ones — the oldest timestamp advanced. + Assert.True( + string.CompareOrdinal(oldestAfter, oldestBefore) > 0, + $"Oldest events must be purged first; oldest went from '{oldestBefore}' to '{oldestAfter}'."); - // The newest event (highest id) must still be present. + // The newest event must still be present. + var newestTimestamp = BulkSeedTimestamp(eventCount - 1).ToString("o"); var newestPresent = _eventLogger.WithConnection(connection => { using var cmd = connection.CreateCommand(); - cmd.CommandText = "SELECT COUNT(*) FROM site_events WHERE id = 3000"; + cmd.CommandText = "SELECT COUNT(*) FROM site_events WHERE timestamp = $ts"; + cmd.Parameters.AddWithValue("$ts", newestTimestamp); return (long)cmd.ExecuteScalar()!; }); Assert.Equal(1L, newestPresent); diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteEventLogging.Tests/EventLogQueryServiceTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteEventLogging.Tests/EventLogQueryServiceTests.cs index b3622d99..82cac861 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.SiteEventLogging.Tests/EventLogQueryServiceTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteEventLogging.Tests/EventLogQueryServiceTests.cs @@ -45,7 +45,7 @@ public class EventLogQueryServiceTests : IDisposable string? severity = null, string? instanceId = null, string? keyword = null, - long? continuationToken = null, + string? continuationToken = null, int pageSize = 500, DateTimeOffset? from = null, DateTimeOffset? to = null) => @@ -281,7 +281,7 @@ public class EventLogQueryServiceTests : IDisposable Assert.Single(response.Entries); var entry = response.Entries[0]; - Assert.True(entry.Id > 0); + Assert.False(string.IsNullOrWhiteSpace(entry.Id)); Assert.Equal("script", entry.EventType); Assert.Equal("Error", entry.Severity); Assert.Equal("inst-1", entry.InstanceId); @@ -340,9 +340,10 @@ public class EventLogQueryServiceTests : IDisposable { using var cmd = connection.CreateCommand(); cmd.CommandText = """ - INSERT INTO site_events (timestamp, event_type, severity, instance_id, source, message) - VALUES ($ts, $et, $sev, $iid, $src, $msg) + INSERT INTO site_events (id, timestamp, event_type, severity, instance_id, source, message) + VALUES ($id, $ts, $et, $sev, $iid, $src, $msg) """; + cmd.Parameters.AddWithValue("$id", Guid.NewGuid().ToString("N")); cmd.Parameters.AddWithValue("$ts", timestamp.ToString("o")); cmd.Parameters.AddWithValue("$et", eventType); cmd.Parameters.AddWithValue("$sev", severity); diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteEventLogging.Tests/SiteEventLogSchemaTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteEventLogging.Tests/SiteEventLogSchemaTests.cs new file mode 100644 index 00000000..daed0b52 --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteEventLogging.Tests/SiteEventLogSchemaTests.cs @@ -0,0 +1,133 @@ +using Microsoft.Data.Sqlite; + +namespace ZB.MOM.WW.ScadaBridge.SiteEventLogging.Tests; + +/// +/// LocalDb Phase 1 (Task 2) — the site_events DDL is extracted out of +/// so the Host's AddZbLocalDb onReady callback can +/// create it in the consolidated site database. +/// +/// The extraction also carries the one deliberate schema CHANGE of Phase 1: the primary +/// key moves from INTEGER PRIMARY KEY AUTOINCREMENT to a TEXT GUID. Under the +/// library's last-writer-wins replication, two site nodes each minting autoincrement id 1 +/// would be treated as the same row and silently overwrite one another. GUID keys make +/// the event log a pure union across the pair instead. +/// +/// +public class SiteEventLogSchemaTests +{ + private static SqliteConnection OpenConnection() + { + var conn = new SqliteConnection("Data Source=:memory:"); + conn.Open(); + return conn; + } + + private static List<(string Name, string Type, bool NotNull, bool Pk)> TableInfo( + SqliteConnection conn, string table) + { + using var cmd = conn.CreateCommand(); + cmd.CommandText = $"PRAGMA table_info('{table}')"; + using var reader = cmd.ExecuteReader(); + var cols = new List<(string, string, bool, bool)>(); + while (reader.Read()) + { + cols.Add((reader.GetString(1), reader.GetString(2), reader.GetInt32(3) == 1, + reader.GetInt32(5) > 0)); + } + + return cols; + } + + [Fact] + public void Apply_CreatesTableWithExpectedColumns() + { + using var conn = OpenConnection(); + + SiteEventLogSchema.Apply(conn); + + Assert.Equal( + ["id", "timestamp", "event_type", "severity", "instance_id", "source", "message", "details"], + TableInfo(conn, "site_events").Select(c => c.Name)); + } + + [Fact] + public void Apply_IdIsTextPrimaryKey_NotAutoincrement() + { + using var conn = OpenConnection(); + + SiteEventLogSchema.Apply(conn); + + var pk = Assert.Single(TableInfo(conn, "site_events"), c => c.Pk); + Assert.Equal("id", pk.Name); + Assert.Equal("TEXT", pk.Type); + + // AUTOINCREMENT creates the sqlite_sequence bookkeeping table. Its absence + // is the load-bearing assertion: ids must be application-minted GUIDs, not + // server-minted integers that collide across the replicating pair. + using var cmd = conn.CreateCommand(); + cmd.CommandText = + "SELECT COUNT(*) FROM sqlite_master WHERE type = 'table' AND name = 'sqlite_sequence'"; + Assert.Equal(0L, Convert.ToInt64(cmd.ExecuteScalar())); + } + + [Fact] + public void Apply_RejectsIntegerIdInserts() + { + using var conn = OpenConnection(); + + SiteEventLogSchema.Apply(conn); + + // A TEXT PK with no AUTOINCREMENT means an INSERT omitting id is an error, + // which is what forces the writer to mint one (Task 5a). + using var cmd = conn.CreateCommand(); + cmd.CommandText = """ + INSERT INTO site_events (timestamp, event_type, severity, source, message) + VALUES ('2026-07-19T00:00:00Z', 'Test', 'Info', 'test', 'no id supplied') + """; + Assert.Throws(() => cmd.ExecuteNonQuery()); + } + + [Fact] + public void Apply_HasNoBlobColumns() + { + using var conn = OpenConnection(); + + SiteEventLogSchema.Apply(conn); + + Assert.DoesNotContain( + TableInfo(conn, "site_events"), + c => c.Type.Contains("BLOB", StringComparison.OrdinalIgnoreCase)); + } + + [Fact] + public void Apply_CreatesExpectedIndexes() + { + using var conn = OpenConnection(); + + SiteEventLogSchema.Apply(conn); + + using var cmd = conn.CreateCommand(); + cmd.CommandText = + "SELECT name FROM sqlite_master WHERE type = 'index' AND tbl_name = 'site_events' " + + "AND name NOT LIKE 'sqlite_%' ORDER BY name"; + using var reader = cmd.ExecuteReader(); + var indexes = new List(); + while (reader.Read()) indexes.Add(reader.GetString(0)); + + Assert.Equal( + ["idx_events_instance", "idx_events_severity", "idx_events_timestamp", "idx_events_type"], + indexes); + } + + [Fact] + public void Apply_IsIdempotent() + { + using var conn = OpenConnection(); + + SiteEventLogSchema.Apply(conn); + SiteEventLogSchema.Apply(conn); + + Assert.Equal(8, TableInfo(conn, "site_events").Count); + } +} diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteEventLogging.Tests/SiteEventLoggerTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteEventLogging.Tests/SiteEventLoggerTests.cs index 76dd1059..81ae9137 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.SiteEventLogging.Tests/SiteEventLoggerTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteEventLogging.Tests/SiteEventLoggerTests.cs @@ -110,20 +110,30 @@ public class SiteEventLoggerTests : IDisposable } [Fact] - public async Task LogEventAsync_MultipleEvents_AutoIncrementIds() + public async Task LogEventAsync_MultipleEvents_GetDistinctApplicationMintedIds() { + // Replaces LogEventAsync_MultipleEvents_AutoIncrementIds. Ids are no longer + // server-minted autoincrement integers: under LocalDb replication the site pair + // converges by last-writer-wins on the primary key, so both nodes issuing 1, 2, + // 3... for unrelated events would silently overwrite each other on sync. + // + // The invariant is therefore uniqueness, NOT monotonicity — ordering is carried + // by the timestamp column, which is what the query and purge paths sort on. await _logger.LogEventAsync("script", "Info", null, "S1", "First"); await _logger.LogEventAsync("script", "Info", null, "S2", "Second"); await _logger.LogEventAsync("script", "Info", null, "S3", "Third"); using var cmd = _verifyConnection.CreateCommand(); - cmd.CommandText = "SELECT id FROM site_events ORDER BY id"; + cmd.CommandText = "SELECT id FROM site_events ORDER BY timestamp"; using var reader = cmd.ExecuteReader(); - var ids = new List(); - while (reader.Read()) ids.Add(reader.GetInt64(0)); + var ids = new List(); + while (reader.Read()) ids.Add(reader.GetString(0)); Assert.Equal(3, ids.Count); - Assert.True(ids[0] < ids[1] && ids[1] < ids[2]); + Assert.All(ids, id => Assert.False(string.IsNullOrWhiteSpace(id))); + Assert.Equal(3, ids.Distinct(StringComparer.Ordinal).Count()); + Assert.All(ids, id => Assert.True(Guid.TryParseExact(id, "N", out _), + $"Event id '{id}' is not a GUID in 'N' format.")); } [Fact] diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Tracking/OperationTrackingSchemaTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Tracking/OperationTrackingSchemaTests.cs new file mode 100644 index 00000000..49ae95af --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Tracking/OperationTrackingSchemaTests.cs @@ -0,0 +1,147 @@ +using Microsoft.Data.Sqlite; +using ZB.MOM.WW.ScadaBridge.SiteRuntime.Tracking; + +namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.Tracking; + +/// +/// LocalDb Phase 1 (Task 2) — the OperationTracking DDL is extracted out of +/// so it can also be applied from the Host's +/// AddZbLocalDb onReady callback, which owns table creation for the consolidated +/// site database. These tests pin the extracted schema against the shape the store has +/// always created: the move must be behaviour-preserving. +/// +public class OperationTrackingSchemaTests +{ + private static SqliteConnection OpenConnection() + { + var conn = new SqliteConnection("Data Source=:memory:"); + conn.Open(); + return conn; + } + + private static List<(string Name, string Type, bool NotNull, bool Pk)> TableInfo( + SqliteConnection conn, string table) + { + using var cmd = conn.CreateCommand(); + cmd.CommandText = $"PRAGMA table_info('{table}')"; + using var reader = cmd.ExecuteReader(); + var cols = new List<(string, string, bool, bool)>(); + while (reader.Read()) + { + cols.Add((reader.GetString(1), reader.GetString(2), reader.GetInt32(3) == 1, + reader.GetInt32(5) > 0)); + } + + return cols; + } + + [Fact] + public void Apply_CreatesTableWithExpectedColumns() + { + using var conn = OpenConnection(); + + OperationTrackingSchema.Apply(conn); + + var cols = TableInfo(conn, "OperationTracking"); + Assert.Equal( + [ + "TrackedOperationId", "Kind", "TargetSummary", "Status", "RetryCount", + "LastError", "HttpStatus", "CreatedAtUtc", "UpdatedAtUtc", "TerminalAtUtc", + "SourceInstanceId", "SourceScript", "SourceNode" + ], + cols.Select(c => c.Name)); + } + + [Fact] + public void Apply_UsesTextPrimaryKey() + { + using var conn = OpenConnection(); + + OperationTrackingSchema.Apply(conn); + + // RegisterReplicated requires an explicit primary key and rejects BLOB + // columns; a TEXT PK is what makes this table replicate as-is. + var pk = Assert.Single(TableInfo(conn, "OperationTracking"), c => c.Pk); + Assert.Equal("TrackedOperationId", pk.Name); + Assert.Equal("TEXT", pk.Type); + } + + [Fact] + public void Apply_HasNoBlobColumns() + { + using var conn = OpenConnection(); + + OperationTrackingSchema.Apply(conn); + + // ILocalDb.RegisterReplicated throws on any column whose declared type + // contains BLOB (json_object cannot capture them). + Assert.DoesNotContain( + TableInfo(conn, "OperationTracking"), + c => c.Type.Contains("BLOB", StringComparison.OrdinalIgnoreCase)); + } + + [Fact] + public void Apply_CreatesExpectedIndexes() + { + using var conn = OpenConnection(); + + OperationTrackingSchema.Apply(conn); + + using var cmd = conn.CreateCommand(); + cmd.CommandText = + "SELECT name FROM sqlite_master WHERE type = 'index' AND tbl_name = 'OperationTracking' " + + "AND name NOT LIKE 'sqlite_%' ORDER BY name"; + using var reader = cmd.ExecuteReader(); + var indexes = new List(); + while (reader.Read()) indexes.Add(reader.GetString(0)); + + Assert.Equal( + ["IX_OperationTracking_Status_Updated", "IX_OperationTracking_UpdatedAt"], + indexes); + } + + [Fact] + public void Apply_IsIdempotent() + { + using var conn = OpenConnection(); + + OperationTrackingSchema.Apply(conn); + OperationTrackingSchema.Apply(conn); + + Assert.Equal(13, TableInfo(conn, "OperationTracking").Count); + } + + [Fact] + public void Apply_AddsSourceNodeToLegacyTableMissingIt() + { + using var conn = OpenConnection(); + + // A tracking DB created by a pre-SourceNode build. CREATE TABLE IF NOT + // EXISTS will not add the column, so Apply must ALTER it in — the + // additive-migration behaviour the store has today must survive extraction. + using (var legacy = conn.CreateCommand()) + { + legacy.CommandText = """ + CREATE TABLE OperationTracking ( + TrackedOperationId TEXT NOT NULL PRIMARY KEY, + Kind TEXT NOT NULL, + TargetSummary TEXT NULL, + Status TEXT NOT NULL, + RetryCount INTEGER NOT NULL DEFAULT 0, + LastError TEXT NULL, + HttpStatus INTEGER NULL, + CreatedAtUtc TEXT NOT NULL, + UpdatedAtUtc TEXT NOT NULL, + TerminalAtUtc TEXT NULL, + SourceInstanceId TEXT NULL, + SourceScript TEXT NULL + ); + """; + legacy.ExecuteNonQuery(); + } + + OperationTrackingSchema.Apply(conn); + + Assert.Contains(TableInfo(conn, "OperationTracking"), c => c.Name == "SourceNode"); + } +}