perf(central): set-based ingest, aligned partition purge, KPI query shapes, EF hygiene

2026-08-14 21:07:12 -04:00
parent ee193cd2bb
commit 5db2a810c0
29 changed files with 3790 additions and 266 deletions
@@ -560,6 +560,164 @@ public class AuditLogRepositoryTests : IClassFixture<MsSqlMigrationFixture>
        Assert.Equal(1, count);
    }

+    // ------------------------------------------------------------------------
+    // WP2.2: set-based InsertManyIfNotExistsAsync
+    // ------------------------------------------------------------------------
+
+    [SkippableFact]
+    public async Task InsertManyIfNotExistsAsync_WritesEveryDistinctEvent()
+    {
+        Skip.IfNot(_fixture.Available, _fixture.SkipReason);
+
+        var siteId = NewSiteId();
+        await using var context = CreateContext();
+        var repo = new AuditLogRepository(context);
+
+        var baseTime = new DateTime(2026, 5, 21, 8, 0, 0, DateTimeKind.Utc);
+        var events = Enumerable.Range(0, 12)
+            .Select(i => NewEvent(siteId, occurredAtUtc: baseTime.AddSeconds(i)))
+            .ToList();
+
+        var inserted = await repo.InsertManyIfNotExistsAsync(events);
+
+        Assert.Equal(12, inserted);
+
+        await using var readContext = CreateContext();
+        var rows = await readContext.Set<AuditLogRow>()
+            .Where(e => e.SourceSiteId == siteId)
+            .ToListAsync();
+
+        Assert.Equal(12, rows.Count);
+        Assert.Equal(
+            events.Select(e => e.EventId).OrderBy(g => g).ToArray(),
+            rows.Select(r => r.EventId).OrderBy(g => g).ToArray());
+    }
+
+    [SkippableFact]
+    public async Task InsertManyIfNotExistsAsync_DuplicateEventIdsWithinOnePacket_CollapseToOneRow()
+    {
+        Skip.IfNot(_fixture.Available, _fixture.SkipReason);
+
+        var siteId = NewSiteId();
+        await using var context = CreateContext();
+        var repo = new AuditLogRepository(context);
+
+        // A set-based INSERT … SELECT … WHERE NOT EXISTS only tests rows that are
+        // already COMMITTED, so two copies of one EventId inside a single VALUES
+        // constructor would both pass the anti-semi-join and collide on the
+        // clustered PK — taking the whole statement down with them. The repository
+        // de-duplicates the packet first (first-write-wins, matching the
+        // single-row contract), which this pins.
+        var occurred = new DateTime(2026, 5, 21, 9, 0, 0, DateTimeKind.Utc);
+        var first = NewEvent(siteId, occurredAtUtc: occurred);
+        var duplicate = ScadaBridgeAuditEventFactory.Create(
+            channel: AuditChannel.ApiOutbound,
+            kind: AuditKind.ApiCall,
+            status: AuditStatus.Delivered,
+            eventId: first.EventId,
+            occurredAtUtc: occurred,
+            sourceSiteId: siteId,
+            errorMessage: "duplicate-within-packet-should-be-ignored");
+        var other = NewEvent(siteId, occurredAtUtc: occurred.AddSeconds(1));
+
+        var inserted = await repo.InsertManyIfNotExistsAsync(
+            new[] { first, duplicate, other, duplicate });
+
+        Assert.Equal(2, inserted);
+
+        await using var readContext = CreateContext();
+        var rows = await readContext.Set<AuditLogRow>()
+            .Where(e => e.SourceSiteId == siteId)
+            .ToListAsync();
+
+        Assert.Equal(2, rows.Count);
+
+        // First-write-wins: the surviving row is the FIRST occurrence, so the
+        // duplicate's ErrorMessage never lands.
+        var stored = Assert.Single(rows, r => r.EventId == first.EventId);
+        Assert.Null(AuditDetailsCodec.Deserialize(stored.DetailsJson).ErrorMessage);
+    }
+
+    [SkippableFact]
+    public async Task InsertManyIfNotExistsAsync_DuplicateEventIdsAcrossPackets_AreIdempotent()
+    {
+        Skip.IfNot(_fixture.Available, _fixture.SkipReason);
+
+        var siteId = NewSiteId();
+        await using var context = CreateContext();
+        var repo = new AuditLogRepository(context);
+
+        var baseTime = new DateTime(2026, 5, 21, 10, 0, 0, DateTimeKind.Utc);
+        var packet = Enumerable.Range(0, 5)
+            .Select(i => NewEvent(siteId, occurredAtUtc: baseTime.AddSeconds(i)))
+            .ToList();
+
+        // A site retry / reconciliation pull re-delivers a packet that overlaps
+        // an already-ingested one. The second call must insert only the genuinely
+        // new rows and silently skip the rest (first-write-wins across packets).
+        var firstInserted = await repo.InsertManyIfNotExistsAsync(packet);
+        var extra = NewEvent(siteId, occurredAtUtc: baseTime.AddSeconds(99));
+        var secondInserted = await repo.InsertManyIfNotExistsAsync(
+            packet.Concat(new[] { extra }).ToList());
+
+        // A third, wholly-redundant replay writes nothing at all.
+        var thirdInserted = await repo.InsertManyIfNotExistsAsync(packet);
+
+        Assert.Equal(5, firstInserted);
+        Assert.Equal(1, secondInserted);
+        Assert.Equal(0, thirdInserted);
+
+        await using var readContext = CreateContext();
+        var rows = await readContext.Set<AuditLogRow>()
+            .Where(e => e.SourceSiteId == siteId)
+            .ToListAsync();
+
+        Assert.Equal(6, rows.Count);
+    }
+
+    [SkippableFact]
+    public async Task InsertManyIfNotExistsAsync_ChunksBeyondTheParameterCeiling()
+    {
+        Skip.IfNot(_fixture.Available, _fixture.SkipReason);
+
+        var siteId = NewSiteId();
+        await using var context = CreateContext();
+        var repo = new AuditLogRepository(context);
+
+        // 250 rows × 10 bound parameters = 2,500 — past SQL Server's 2,100
+        // parameter ceiling for a single statement, so this only succeeds if the
+        // repository chunks. Nulls in the optional columns (Target, SourceNode,
+        // CorrelationId, Actor) are left null on purpose: the VALUES constructor
+        // derives its column types from the parameters, so an untyped null would
+        // surface here as a conversion failure rather than silently.
+        var baseTime = new DateTime(2026, 5, 21, 11, 0, 0, DateTimeKind.Utc);
+        var events = Enumerable.Range(0, 250)
+            .Select(i => NewEvent(siteId, occurredAtUtc: baseTime.AddSeconds(i)))
+            .ToList();
+
+        var inserted = await repo.InsertManyIfNotExistsAsync(events);
+
+        Assert.Equal(250, inserted);
+
+        await using var readContext = CreateContext();
+        var count = await readContext.Set<AuditLogRow>()
+            .Where(e => e.SourceSiteId == siteId)
+            .CountAsync();
+
+        Assert.Equal(250, count);
+    }
+
+    [SkippableFact]
+    public async Task InsertManyIfNotExistsAsync_EmptyBatch_IsANoOp()
+    {
+        Skip.IfNot(_fixture.Available, _fixture.SkipReason);
+
+        await using var context = CreateContext();
+        var repo = new AuditLogRepository(context);
+
+        Assert.Equal(0, await repo.InsertManyIfNotExistsAsync(Array.Empty<AuditEvent>()));
+    }
+
    [SkippableFact]
    public async Task QueryAsync_Keyset_SameOccurredAtUtc_TiebreaksOnEventId()
    {
@@ -622,16 +780,17 @@ public class AuditLogRepositoryTests : IClassFixture<MsSqlMigrationFixture>
    // ------------------------------------------------------------------------
    //
    // The partition-switch path replaces M1's NotSupportedException stub with
-    // the production drop-DROP-INDEX → CREATE-staging → SWITCH PARTITION →
-    // DROP-staging → CREATE-INDEX dance documented in alog.md §4. These tests
+    // the production CREATE-staging → SWITCH PARTITION → DROP-staging batch
+    // documented in alog.md §4. WP2.2 removed the index drop/rebuild that used to
+    // bracket it: uniqueness rides the partition-ALIGNED clustered PK
+    // (EventId, OccurredAtUtc), so SWITCH has nothing to object to. These tests
    // verify the side effects an outsider can observe:
    //   * rows in the targeted month are removed
    //   * rows in OTHER months are NOT touched
-    //   * UX_AuditLog_EventId still exists after a successful switch
+    //   * no non-aligned UX_AuditLog_EventId is (re)created by a switch
    //   * InsertIfNotExistsAsync's first-write-wins idempotency still holds
-    //     after a switch (the rebuilt index is real)
-    //   * a thrown SqlException leaves UX_AuditLog_EventId rebuilt (the CATCH
-    //     branch's recovery path runs)
+    //     after a switch (the aligned key really does enforce it)
+    //   * a thrown SqlException leaves the table intact with no orphaned staging

    [SkippableFact]
    public async Task SwitchOutPartitionAsync_OldPartition_RemovesRows_NewPartitionsKept()
@@ -669,7 +828,7 @@ public class AuditLogRepositoryTests : IClassFixture<MsSqlMigrationFixture>
    }

    [SkippableFact]
-    public async Task SwitchOutPartitionAsync_RebuildsUxIndex_AfterSwitch()
+    public async Task SwitchOutPartitionAsync_LeavesNoNonAlignedUniqueIndex_AfterSwitch()
    {
        Skip.IfNot(_fixture.Available, _fixture.SkipReason);

@@ -680,12 +839,23 @@ public class AuditLogRepositoryTests : IClassFixture<MsSqlMigrationFixture>
        // the fixture's MSSQL database) don't tread on each other.
        await repo.SwitchOutPartitionAsync(new DateTime(2026, 4, 1, 0, 0, 0, DateTimeKind.Utc));

+        // WP2.2: the switch must NOT recreate UX_AuditLog_EventId. Re-adding it
+        // would reinstate the offline drop/rebuild the purge was freed from and
+        // block the next SWITCH until something dropped it again.
        await using var verifyContext = CreateContext();
        var indexExists = await ScalarAsync<int>(
            verifyContext,
            "SELECT COUNT(*) FROM sys.indexes " +
            "WHERE name = 'UX_AuditLog_EventId' AND object_id = OBJECT_ID('dbo.AuditLog');");
-        Assert.Equal(1, indexExists);
+        Assert.Equal(0, indexExists);
+
+        // The aligned clustered PK is what enforces uniqueness now, and the
+        // switch must leave it untouched.
+        var pkExists = await ScalarAsync<int>(
+            verifyContext,
+            "SELECT COUNT(*) FROM sys.indexes " +
+            "WHERE name = 'PK_AuditLog' AND object_id = OBJECT_ID('dbo.AuditLog') AND is_primary_key = 1;");
+        Assert.Equal(1, pkExists);
    }

    [SkippableFact]
@@ -705,10 +875,11 @@ public class AuditLogRepositoryTests : IClassFixture<MsSqlMigrationFixture>
        // Switch out the June 2026 partition (different month, empty).
        await repo.SwitchOutPartitionAsync(new DateTime(2026, 6, 1, 0, 0, 0, DateTimeKind.Utc));

-        // Re-attempting the same EventId after the switch must STILL be a no-op
-        // (UX_AuditLog_EventId is the index that enables idempotency; if the
-        // rebuild left it broken, this insert would silently produce a duplicate
-        // row and the count assertion below would catch it).
+        // Re-attempting the same EventId after the switch must STILL be a no-op.
+        // The idempotency probe now seeks the aligned clustered PK's leading
+        // column instead of a dedicated unique index; if the switch had disturbed
+        // that key, this insert would silently produce a duplicate row and the
+        // count assertion below would catch it.
        // C3 (Task 2.5): rebuild a sibling row with the same EventId via the factory
        // (ErrorMessage rides in DetailsJson, so a top-level `with` no longer applies).
        var dup = ScadaBridgeAuditEventFactory.Create(
@@ -734,7 +905,7 @@ public class AuditLogRepositoryTests : IClassFixture<MsSqlMigrationFixture>
    }

    [SkippableFact]
-    public async Task SwitchOutPartitionAsync_PartialFailure_RebuildsUxIndex_RaisesException()
+    public async Task SwitchOutPartitionAsync_PartialFailure_RaisesException_LeavesNoOrphanedStaging()
    {
        Skip.IfNot(_fixture.Available, _fixture.SkipReason);

@@ -745,8 +916,8 @@ public class AuditLogRepositoryTests : IClassFixture<MsSqlMigrationFixture>
        // ALTER TABLE … SWITCH refuses to move rows out of a partition that's
        // referenced by an FK from another table, raising msg 4928
        // ("ALTER TABLE SWITCH statement failed because target table … has a
-        // foreign key …"). The CATCH branch then rolls back and rebuilds the
-        // unique index — which the assertion below verifies.
+        // foreign key …"). The CATCH branch then rolls back and drops the staging
+        // table — which the assertion below verifies.
        //
        // The probe table is uniquely named with a guid suffix so reruns of
        // this test inside the same fixture DB never collide. We clean it up
@@ -786,14 +957,23 @@ public class AuditLogRepositoryTests : IClassFixture<MsSqlMigrationFixture>
            await cmd.ExecuteNonQueryAsync();
        }

-        // The CATCH block in the production SQL guarantees UX_AuditLog_EventId
-        // is rebuilt regardless of which step failed inside the TRY.
+        // The CATCH block drops the GUID-suffixed staging table regardless of
+        // which step failed inside the TRY, so a failed purge leaves no orphaned
+        // AuditLog_Staging_* object behind for the next tick to trip over.
        await using var verifyContext = CreateContext();
+        var orphanedStaging = await ScalarAsync<int>(
+            verifyContext,
+            "SELECT COUNT(*) FROM sys.tables WHERE name LIKE 'AuditLog\\_Staging\\_%' ESCAPE '\\';");
+        Assert.Equal(0, orphanedStaging);
+
+        // And it must NOT have (re)created the non-aligned unique index: WP2.2
+        // deleted that rebuild entirely, so a failed switch cannot reintroduce
+        // the object that blocks the NEXT switch.
        var indexExists = await ScalarAsync<int>(
            verifyContext,
            "SELECT COUNT(*) FROM sys.indexes " +
            "WHERE name = 'UX_AuditLog_EventId' AND object_id = OBJECT_ID('dbo.AuditLog');");
-        Assert.Equal(1, indexExists);
+        Assert.Equal(0, indexExists);
    }

    // ------------------------------------------------------------------------