feat(audit)!: ScadaBridge C5 — collapse central dbo.AuditLog to 10 canonical cols + persisted computed cols; CollapseAuditLogToCanonical migration; repo writes canonical directly (Task 2.5)

2026-06-02 14:06:46 -04:00
parent 1737d15f04
commit 68a6bd1720
12 changed files with 2592 additions and 440 deletions
@@ -70,17 +70,16 @@ public class PartitionPurgeTests : TestKit, IClassFixture<MsSqlMigrationFixture>
        string siteId)
    {
        await using var cmd = conn.CreateCommand();
+        // C5 (Task 2.5): dbo.AuditLog is now the 10 canonical columns + DetailsJson;
+        // the ScadaBridge domain fields (channel/kind/status/sourceSiteId) ride in
+        // DetailsJson and the SourceSiteId/Kind/Status computed columns auto-derive.
+        // Action = "{channel}.{kind}", Category = channel name, Outcome = Success.
        cmd.CommandText = @"
 INSERT INTO dbo.AuditLog
-    (EventId, OccurredAtUtc, IngestedAtUtc, Channel, Kind, CorrelationId,
-     SourceSiteId, SourceInstanceId, SourceScript, Actor, Target, Status,
-     HttpStatus, DurationMs, ErrorMessage, ErrorDetail, RequestSummary,
-     ResponseSummary, PayloadTruncated, Extra, ForwardState)
+    (EventId, OccurredAtUtc, Actor, Action, Outcome, Category, Target, SourceNode, CorrelationId, DetailsJson)
 VALUES
-    (@EventId, @OccurredAtUtc, @IngestedAtUtc, 'ApiOutbound', 'ApiCall', NULL,
-     @SourceSiteId, NULL, NULL, NULL, NULL, 'Delivered',
-     NULL, NULL, NULL, NULL, NULL,
-     NULL, 0, NULL, NULL);";
+    (@EventId, @OccurredAtUtc, NULL, 'ApiOutbound.ApiCall', 'Success', 'ApiOutbound', NULL, NULL, NULL,
+     @DetailsJson);";
        cmd.Parameters.Add("@EventId", System.Data.SqlDbType.UniqueIdentifier).Value = eventId;
        // SqlDbType.DateTime2 with explicit Scale 7 matches the
        // OccurredAtUtc column shape (datetime2(7)) and avoids the implicit
@@ -93,10 +92,14 @@ VALUES
        var occurredParam = cmd.Parameters.Add("@OccurredAtUtc", System.Data.SqlDbType.DateTime2);
        occurredParam.Scale = 7;
        occurredParam.Value = occurredAtUtc;
-        var ingestedParam = cmd.Parameters.Add("@IngestedAtUtc", System.Data.SqlDbType.DateTime2);
-        ingestedParam.Scale = 7;
-        ingestedParam.Value = DateTime.UtcNow;
-        cmd.Parameters.Add("@SourceSiteId", System.Data.SqlDbType.VarChar, 64).Value = siteId;
+        // DetailsJson carries the camelCase domain fields (matching AuditDetailsCodec):
+        // channel/kind/status drive the computed Kind/Status columns; sourceSiteId drives
+        // the computed SourceSiteId column the verify queries scope on. payloadTruncated
+        // is always present (the codec always writes the bool).
+        var detailsJson =
+            "{\"channel\":\"ApiOutbound\",\"kind\":\"ApiCall\",\"status\":\"Delivered\"," +
+            "\"sourceSiteId\":\"" + siteId + "\",\"payloadTruncated\":false}";
+        cmd.Parameters.Add("@DetailsJson", System.Data.SqlDbType.NVarChar, -1).Value = detailsJson;
        await cmd.ExecuteNonQueryAsync();
    }

@@ -3,15 +3,20 @@ using ZB.MOM.WW.ScadaBridge.Commons.Types.Enums;
 using ZB.MOM.WW.ScadaBridge.ConfigurationDatabase;
 using ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Configurations;
 using ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Entities;
+using AuditOutcome = ZB.MOM.WW.Audit.AuditOutcome;

 namespace ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Tests.Configurations;

 /// <summary>
-/// Schema-level tests for <see cref="AuditLogEntityTypeConfiguration"/> (#23 M1 Bundle B).
-/// Verifies that <see cref="AuditLogRow"/> maps to the AuditLog table with the
-/// PK, property set, column types/lengths, and five named indexes specified in alog.md §4.
-/// Inspects EF model metadata via the existing in-memory SQLite test context — no
-/// database round-trips required.
+/// Schema-level tests for the C5 (Task 2.5) <see cref="AuditLogEntityTypeConfiguration"/>.
+/// After the central <c>dbo.AuditLog</c> collapse the table is the 10 canonical
+/// <c>ZB.MOM.WW.Audit.AuditEvent</c> columns plus six read-only <b>persisted computed
+/// columns</b> derived from <c>DetailsJson</c>. Verifies the PK, the canonical/computed
+/// property split, the Channel→Category column mapping, the computed-column SQL, and the
+/// named indexes — via EF model metadata on the in-memory SQLite test context (the SQLite
+/// test context strips the JSON_VALUE computed SQL so EnsureCreated still succeeds; these
+/// metadata assertions read the production-shaped model before that strip is observable
+/// for column-name/key/index facts).
 /// </summary>
 public class AuditLogEntityTypeConfigurationTests : IDisposable
 {
@@ -33,7 +38,7 @@ public class AuditLogEntityTypeConfigurationTests : IDisposable
    {
        // Composite PK {EventId, OccurredAtUtc} is required by the partitioned
        // AuditLog table — the clustered key must include the partition column
-        // (OccurredAtUtc) so each row can be located in its partition (#23 Bundle C).
+        // (OccurredAtUtc) so each row can be located in its partition.
        var entity = _context.Model.FindEntityType(typeof(AuditLogRow));

        Assert.NotNull(entity);
@@ -50,8 +55,8 @@ public class AuditLogEntityTypeConfigurationTests : IDisposable
    public void Configure_DeclaresUniqueIndex_OnEventIdAlone_ForIdempotencyLookups()
    {
        // EventId remains globally unique (the idempotency key for
-        // InsertIfNotExistsAsync, per M1-T8) via a dedicated unique index that
-        // is independent of the composite PK.
+        // InsertIfNotExistsAsync) via a dedicated unique index independent of the
+        // composite PK.
        var entity = _context.Model.FindEntityType(typeof(AuditLogRow));
        Assert.NotNull(entity);

@@ -75,10 +80,65 @@ public class AuditLogEntityTypeConfigurationTests : IDisposable
            .Where(p => !p.IsShadowProperty())
            .ToList();

-        // AuditLogRow record exposes 24 init-only properties (alog.md §4 plus the
-        // additive ExecutionId universal correlation column, its ParentExecutionId
-        // sibling, and the SourceNode-stamping column).
-        Assert.Equal(24, properties.Count);
+        // C5: 10 canonical columns (EventId, OccurredAtUtc, Actor, Action, Outcome,
+        // Channel[=Category], Target, SourceNode, CorrelationId, DetailsJson) + 6
+        // persisted computed columns (Kind, Status, SourceSiteId, ExecutionId,
+        // ParentExecutionId, IngestedAtUtc) = 16.
+        Assert.Equal(16, properties.Count);
+    }
+
+    [Fact]
+    public void Configure_ChannelMapsToCanonicalCategoryColumn()
+    {
+        // The Channel enum property is stored in the canonical Category column
+        // (Category = channel name for ScadaBridge).
+        var entity = _context.Model.FindEntityType(typeof(AuditLogRow));
+        Assert.NotNull(entity);
+
+        var channel = entity!.FindProperty(nameof(AuditLogRow.Channel));
+        Assert.NotNull(channel);
+        Assert.Equal("Category", channel!.GetColumnName());
+    }
+
+    [Theory]
+    [InlineData(nameof(AuditLogRow.Kind), "JSON_VALUE(DetailsJson,'$.kind')")]
+    [InlineData(nameof(AuditLogRow.Status), "JSON_VALUE(DetailsJson,'$.status')")]
+    [InlineData(nameof(AuditLogRow.SourceSiteId), "JSON_VALUE(DetailsJson,'$.sourceSiteId')")]
+    [InlineData(nameof(AuditLogRow.ExecutionId), "CAST(JSON_VALUE(DetailsJson,'$.executionId') AS uniqueidentifier)")]
+    [InlineData(nameof(AuditLogRow.ParentExecutionId), "CAST(JSON_VALUE(DetailsJson,'$.parentExecutionId') AS uniqueidentifier)")]
+    public void Configure_ComputedColumns_HaveExpectedPersistedSql(string propertyName, string expectedSql)
+    {
+        // Note: the SQLite test context strips computed-column SQL so EnsureCreated
+        // works. Re-build a SQL Server model here to read the production computed SQL.
+        using var sqlServerContext = CreateSqlServerModelContext();
+        var entity = sqlServerContext.Model.FindEntityType(typeof(AuditLogRow));
+        Assert.NotNull(entity);
+
+        var property = entity!.FindProperty(propertyName);
+        Assert.NotNull(property);
+        Assert.Equal(expectedSql, property!.GetComputedColumnSql());
+        // Persisted (stored) computed column.
+        Assert.True(property.GetIsStored());
+        // Read-only — EF never writes it.
+        Assert.Equal(Microsoft.EntityFrameworkCore.Metadata.ValueGenerated.OnAddOrUpdate, property.ValueGenerated);
+    }
+
+    [Fact]
+    public void Configure_IngestedAtUtc_IsNonPersistedComputedColumn()
+    {
+        using var sqlServerContext = CreateSqlServerModelContext();
+        var entity = sqlServerContext.Model.FindEntityType(typeof(AuditLogRow));
+        Assert.NotNull(entity);
+
+        var property = entity!.FindProperty(nameof(AuditLogRow.IngestedAtUtc));
+        Assert.NotNull(property);
+        Assert.Equal(
+            "CAST(SWITCHOFFSET(CAST(JSON_VALUE(DetailsJson,'$.ingestedAtUtc') AS datetimeoffset), 0) AS datetime2(7))",
+            property!.GetComputedColumnSql());
+        // NON-persisted: the datetimeoffset/SWITCHOFFSET cast is non-deterministic, so
+        // SQL Server rejects a PERSISTED column. It is not indexed, so this is fine.
+        Assert.False(property.GetIsStored() ?? true);
+        Assert.Equal(Microsoft.EntityFrameworkCore.Metadata.ValueGenerated.OnAddOrUpdate, property.ValueGenerated);
    }

    [Fact]
@@ -92,12 +152,8 @@ public class AuditLogEntityTypeConfigurationTests : IDisposable
            .OrderBy(n => n, StringComparer.Ordinal)
            .ToList();

-        // Five reconciliation/query indexes from alog.md §4, plus the EventId unique
-        // index introduced alongside the composite PK (Bundle C), plus the additive
-        // IX_AuditLog_Execution index supporting ExecutionId lookups, the
-        // IX_AuditLog_ParentExecution index supporting ParentExecutionId lookups,
-        // and the IX_AuditLog_Node_Occurred composite supporting per-node queries
-        // (SourceNode-stamping).
+        // The index NAMES are preserved across the C5 collapse (the column sets move to
+        // the canonical/computed shape but the discoverable names do not change).
        var expected = new[]
        {
            "IX_AuditLog_Channel_Status_Occurred",
@@ -115,10 +171,9 @@ public class AuditLogEntityTypeConfigurationTests : IDisposable
    }

    [Theory]
-    [InlineData(nameof(AuditLogRow.Channel))]
    [InlineData(nameof(AuditLogRow.Kind))]
    [InlineData(nameof(AuditLogRow.Status))]
-    [InlineData(nameof(AuditLogRow.ForwardState))]
+    [InlineData(nameof(AuditLogRow.Channel))]
    public void Configure_EnumColumns_StoredAsVarchar32(string propertyName)
    {
        var entity = _context.Model.FindEntityType(typeof(AuditLogRow));
@@ -133,56 +188,58 @@ public class AuditLogEntityTypeConfigurationTests : IDisposable
        Assert.False(property.IsUnicode() ?? true);
    }

+    [Fact]
+    public void Configure_Outcome_StoredAsVarchar16()
+    {
+        var entity = _context.Model.FindEntityType(typeof(AuditLogRow));
+        Assert.NotNull(entity);
+
+        var property = entity!.FindProperty(nameof(AuditLogRow.Outcome));
+        Assert.NotNull(property);
+        Assert.Equal(typeof(string), property!.GetProviderClrType() ?? property.ClrType);
+        Assert.Equal(16, property.GetMaxLength());
+        Assert.False(property.IsUnicode() ?? true);
+    }
+
    [Fact]
    public async Task Configure_UtcConverter_HydratesOccurredAtUtcAsKindUtc()
    {
-        // Insert an AuditLogRow with an Unspecified-Kind DateTime, then re-read
-        // it in a fresh context. The UtcConverter on the OccurredAtUtc /
-        // IngestedAtUtc columns must re-tag the round-tripped value as
-        // DateTimeKind.Utc. Without the converter the SQLite (and on production
-        // SQL Server, datetime2) provider would yield Kind=Unspecified — see
-        // ConfigurationDatabase-018/020 and Commons-019.
+        // Insert an AuditLogRow with an Unspecified-Kind DateTime, then re-read it in
+        // a fresh context. The UtcConverter on OccurredAtUtc must re-tag the
+        // round-tripped value as DateTimeKind.Utc. Only the canonical columns are
+        // written here — the computed columns are read-only (and stripped to plain,
+        // always-null columns on the SQLite test model).
        var unspecifiedOccurred = new DateTime(2026, 5, 28, 10, 30, 0, DateTimeKind.Unspecified);
-        var unspecifiedIngested = new DateTime(2026, 5, 28, 10, 31, 0, DateTimeKind.Unspecified);
        var eventId = Guid.NewGuid();
-        var siteId = "test-" + Guid.NewGuid().ToString("N").Substring(0, 8);
+        var corr = Guid.NewGuid();

        var evt = new AuditLogRow
        {
            EventId = eventId,
-            // The AuditLogRow record's init-setter (Commons-019 resolution)
-            // re-tags Unspecified values as Utc on assignment, so the value EF
-            // ultimately writes already has Kind=Utc. The converter's job is
-            // to keep the Kind tag on the READ path, which the assertions
-            // below exercise.
            OccurredAtUtc = unspecifiedOccurred,
-            IngestedAtUtc = unspecifiedIngested,
+            Actor = "system",
+            Action = "ApiOutbound.ApiCall",
+            Outcome = AuditOutcome.Success,
            Channel = AuditChannel.ApiOutbound,
-            Kind = AuditKind.ApiCall,
-            Status = AuditStatus.Delivered,
-            SourceSiteId = siteId,
+            CorrelationId = corr,
+            DetailsJson = "{\"channel\":\"ApiOutbound\",\"kind\":\"ApiCall\",\"status\":\"Delivered\"}",
        };

        _context.Set<AuditLogRow>().Add(evt);
        await _context.SaveChangesAsync();

-        // Detach the tracked entity and re-read in a fresh query so we exercise
-        // the actual hydrate path, not the change-tracker cache.
+        // Detach and re-read so we exercise the hydrate path, not the change tracker.
        _context.ChangeTracker.Clear();

        var loaded = await _context.Set<AuditLogRow>()
            .AsNoTracking()
-            .Where(e => e.SourceSiteId == siteId)
+            .Where(e => e.EventId == eventId)
            .SingleAsync();

        Assert.Equal(DateTimeKind.Utc, loaded.OccurredAtUtc.Kind);
-        Assert.NotNull(loaded.IngestedAtUtc);
-        Assert.Equal(DateTimeKind.Utc, loaded.IngestedAtUtc!.Value.Kind);
-
-        // The timestamp ticks must round-trip unchanged — the converter only
-        // touches the Kind flag, not the wall-clock value.
+        // The timestamp ticks must round-trip unchanged — the converter only touches
+        // the Kind flag, not the wall-clock value.
        Assert.Equal(unspecifiedOccurred.Ticks, loaded.OccurredAtUtc.Ticks);
-        Assert.Equal(unspecifiedIngested.Ticks, loaded.IngestedAtUtc.Value.Ticks);
    }

    [Fact]
@@ -190,8 +247,6 @@ public class AuditLogEntityTypeConfigurationTests : IDisposable
    {
        // Model-metadata cross-check on the converter wiring — guards against a
        // future config refactor accidentally removing the HasConversion calls.
-        // The converter type itself is internal to the configuration, so we
-        // just assert SOME converter is present on each *Utc DateTime column.
        var entity = _context.Model.FindEntityType(typeof(AuditLogRow));
        Assert.NotNull(entity);

@@ -218,12 +273,27 @@ public class AuditLogEntityTypeConfigurationTests : IDisposable
            .Single(i => i.GetDatabaseName() == "IX_AuditLog_Target_Occurred");
        Assert.Equal("[Target] IS NOT NULL", targetIdx.GetFilter());

+        // ExecutionId / ParentExecutionId are computed columns — SQL Server forbids a
+        // filtered-index predicate on a computed column, so these indexes are UNFILTERED.
        var executionIdx = entity.GetIndexes()
            .Single(i => i.GetDatabaseName() == "IX_AuditLog_Execution");
-        Assert.Equal("[ExecutionId] IS NOT NULL", executionIdx.GetFilter());
+        Assert.Null(executionIdx.GetFilter());

        var parentExecutionIdx = entity.GetIndexes()
            .Single(i => i.GetDatabaseName() == "IX_AuditLog_ParentExecution");
-        Assert.Equal("[ParentExecutionId] IS NOT NULL", parentExecutionIdx.GetFilter());
+        Assert.Null(parentExecutionIdx.GetFilter());
+    }
+
+    /// <summary>
+    /// Builds a model-only DbContext on the SQL Server provider so the production
+    /// computed-column SQL (which the SQLite test context strips) can be asserted.
+    /// No database connection is opened — only the model is materialised.
+    /// </summary>
+    private static ScadaBridgeDbContext CreateSqlServerModelContext()
+    {
+        var options = new DbContextOptionsBuilder<ScadaBridgeDbContext>()
+            .UseSqlServer("Server=unused;Database=unused;TrustServerCertificate=true")
+            .Options;
+        return new ScadaBridgeDbContext(options);
    }
 }
@@ -211,9 +211,14 @@ public class AddAuditLogTableMigrationTests : IClassFixture<MsSqlMigrationFixtur
            // WHERE 1=0 guarantees no rows are touched even if the permission check
            // somehow passes — the test asserts the engine rejects the statement
            // at permission-check time, not via a side effect on data.
+            // C5 (Task 2.5): target a CANONICAL (non-computed) column. Status is now a
+            // persisted computed column, and an UPDATE that sets a computed column is
+            // rejected with a "cannot be modified" error BEFORE the permission check —
+            // which would mask the DENY UPDATE this test exercises. Actor is a plain
+            // writable column, so the permission check is the one that fires.
            cmd.CommandText =
                $"EXECUTE AS USER = '{testUser}'; " +
-                $"UPDATE dbo.AuditLog SET Status = 'X' WHERE 1 = 0; " +
+                $"UPDATE dbo.AuditLog SET Actor = 'X' WHERE 1 = 0; " +
                $"REVERT;";
            await cmd.ExecuteNonQueryAsync();
        });
@@ -133,7 +133,9 @@ public class AuditLogRepositoryTests : IClassFixture<MsSqlMigrationFixture>
            .ToListAsync();

        Assert.Single(loaded);
-        Assert.Equal("first", loaded[0].ErrorMessage);
+        // C5 (Task 2.5): ErrorMessage rides in DetailsJson now — decode it to assert
+        // first-write-wins kept the original payload.
+        Assert.Equal("first", AuditDetailsCodec.Deserialize(loaded[0].DetailsJson).ErrorMessage);
    }

    [SkippableFact]
@@ -726,8 +728,9 @@ public class AuditLogRepositoryTests : IClassFixture<MsSqlMigrationFixture>

        Assert.Single(rows);
        Assert.Equal(preExisting.EventId, rows[0].EventId);
-        // First-write-wins: the original ErrorMessage (null) survives.
-        Assert.Null(rows[0].ErrorMessage);
+        // First-write-wins: the original ErrorMessage (null) survives. C5 (Task 2.5):
+        // ErrorMessage rides in DetailsJson — decode it to assert.
+        Assert.Null(AuditDetailsCodec.Deserialize(rows[0].DetailsJson).ErrorMessage);
    }

    [SkippableFact]
@@ -38,6 +38,12 @@ public class SqliteTestDbContext : ScadaBridgeDbContext
                .ValueGeneratedNever();
        });

+        // Note (C5 / Task 2.5): the central dbo.AuditLog persisted computed columns
+        // (JSON_VALUE-based) are neutralized for non-SQL-Server providers inside
+        // ScadaBridgeDbContext.OnModelCreating itself, so every SQLite test context —
+        // including ones that construct ScadaBridgeDbContext directly rather than via
+        // this helper — gets the strip. Nothing to do here.
+
        // Convert DateTimeOffset to ISO 8601 string for SQLite so ORDER BY works
        var converter = new ValueConverter<DateTimeOffset, string>(
            v => v.UtcDateTime.ToString("o"),