6c11362d9f
P4-2: SqliteAuditWriter.FlushBatch completed each pending row's Task (TrySetResult) inside the per-row insert loop, before transaction.Commit() ran. A later failure in the same batch (another row's insert throwing, or the commit itself failing) rolled the whole transaction back, but the earlier rows' Tasks had already completed successfully -- the caller and FallbackAuditWriter (whose ring-buffer fallback exists precisely for this outage) both saw false success while the rows were actually gone from the database; the outer catch's TrySetException became a silent no-op against an already-completed TaskCompletionSource. Every row's Completion is now deferred and only resolved (TrySetResult for the whole batch, or TrySetException for the whole batch) after transaction.Commit() succeeds or the transaction rolls back, mirroring SiteEventLogger.FlushBatch. P4-4: EventLogPurgeService gated the daily site_events purge on the active-node check, early-exiting on the standby, on the stale premise that "the standby receives no writes". DataConnectionActor runs on both site nodes and writes site_events regardless of active/standby status (register rows 89/90/97), so an unreplicated standby (the rig's site-b/site-c posture) grew its site_events file forever with no retention and no cap. The gate is removed; every node now purges its own file every tick. Safe on a replicated pair because the retention/cap deletes are CDC-captured and converge under LWW regardless of which node issues them; required on an unreplicated pair because each node's file is independent. The active-node check is retained only to annotate the purge tick's log line. Tests: new SqliteAuditWriterCommitFailureTests forces a genuine non-swallowed mid-batch INSERT failure via a real SQLite trigger + custom scalar function on the writer's own connection (reflection seam), proving (a) a row whose own insert succeeded still faults when a later row in the same batch fails, and (b) FallbackAuditWriter rings every faulted row and increments the failure counter for each -- both assertions fail against the pre-fix code (proof-of-failure verified by temporarily reverting the fix). EventLogPurgeServiceTests' standby case is inverted to assert the purge now runs (not skips) on a standby node.
191 lines
9.4 KiB
C#
191 lines
9.4 KiB
C#
using System.Reflection;
|
|
using Microsoft.Data.Sqlite;
|
|
using Microsoft.Extensions.Logging.Abstractions;
|
|
using Microsoft.Extensions.Options;
|
|
using NSubstitute;
|
|
using ZB.MOM.WW.Audit;
|
|
using ZB.MOM.WW.ScadaBridge.AuditLog.Site;
|
|
using ZB.MOM.WW.ScadaBridge.AuditLog.Tests.TestSupport;
|
|
using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services;
|
|
using ZB.MOM.WW.ScadaBridge.Commons.Types.Audit;
|
|
using ZB.MOM.WW.ScadaBridge.Commons.Types.Enums;
|
|
using IAuditWriter = ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services.IAuditWriter;
|
|
|
|
namespace ZB.MOM.WW.ScadaBridge.AuditLog.Tests.Site;
|
|
|
|
/// <summary>
|
|
/// arch-review 2026-08-19 P4-2 regression coverage. <see cref="SqliteAuditWriter.FlushBatch"/>
|
|
/// used to call <c>pending.Completion.TrySetResult()</c> per row BEFORE
|
|
/// <c>transaction.Commit()</c> ran. A later failure in the SAME batch — another row's insert
|
|
/// throwing, or the commit itself failing (SQLITE_FULL, IOERR) — rolled the whole transaction
|
|
/// back, but the earlier row's <see cref="System.Threading.Tasks.Task"/> had already completed
|
|
/// successfully: the caller, and <see cref="FallbackAuditWriter"/> (whose ring-buffer fallback
|
|
/// exists precisely for this outage), both saw false success while the row was actually gone
|
|
/// from the database — <c>TrySetException</c> in the outer catch is a no-op against an
|
|
/// already-completed <see cref="System.Threading.Tasks.TaskCompletionSource"/>.
|
|
/// </summary>
|
|
/// <remarks>
|
|
/// A committed transaction can't practically be made to fail via public SQLite surface without
|
|
/// exhausting disk or injecting a custom VFS, so these tests force a genuine, non-swallowed
|
|
/// mid-batch INSERT failure instead (the "bonus defect" the finding also calls out: a mid-batch
|
|
/// insert failure on a later row loses the rows that inserted fine before it, the same way).
|
|
/// That failure is reached through a real SQLite trigger + a custom scalar function registered
|
|
/// directly on the writer's own connection (via reflection — <see cref="SqliteAuditWriter"/>
|
|
/// deliberately doesn't expose it), so the whole path under test — transaction, per-row
|
|
/// try/catch, rollback, TrySetException — is the writer's real production code, not a mock.
|
|
/// </remarks>
|
|
public class SqliteAuditWriterCommitFailureTests
|
|
{
|
|
private const string FailMarker = "P4-2-FORCED-FAILURE";
|
|
|
|
private static AuditEvent NewEvent(string target) => ScadaBridgeAuditEventFactory.Create(
|
|
channel: AuditChannel.ApiOutbound,
|
|
kind: AuditKind.ApiCall,
|
|
status: AuditStatus.Delivered,
|
|
eventId: Guid.NewGuid(),
|
|
occurredAtUtc: DateTime.UtcNow,
|
|
target: target);
|
|
|
|
/// <summary>
|
|
/// Installs a BEFORE INSERT trigger on <c>audit_event</c> that calls a custom scalar
|
|
/// function throwing whenever the inserted row's <c>Target</c> is <see cref="FailMarker"/>.
|
|
/// An exception thrown from a Microsoft.Data.Sqlite custom function surfaces as a generic
|
|
/// SQLITE_ERROR (code 1), NOT SQLITE_CONSTRAINT (code 19) — so it is NOT caught by
|
|
/// FlushBatch's duplicate-EventId swallow (which matches only code 19) and instead reaches
|
|
/// the outer catch exactly like a real disk-outage commit failure would.
|
|
/// </summary>
|
|
private static void InstallFailureTrigger(SqliteAuditWriter writer)
|
|
{
|
|
var connectionField = typeof(SqliteAuditWriter).GetField(
|
|
"_connection", BindingFlags.NonPublic | BindingFlags.Instance)
|
|
?? throw new InvalidOperationException("SqliteAuditWriter._connection not found — test seam broke.");
|
|
var connection = (SqliteConnection)connectionField.GetValue(writer)!;
|
|
|
|
connection.CreateFunction<string, long>("throw_if_marker", target =>
|
|
{
|
|
if (target == FailMarker)
|
|
{
|
|
throw new InvalidOperationException("P4-2 test-forced insert failure");
|
|
}
|
|
return 0;
|
|
});
|
|
|
|
using var cmd = connection.CreateCommand();
|
|
cmd.CommandText = $"""
|
|
CREATE TRIGGER test_fail_marker
|
|
BEFORE INSERT ON audit_event
|
|
FOR EACH ROW WHEN NEW.Target = '{FailMarker}'
|
|
BEGIN
|
|
SELECT throw_if_marker(NEW.Target);
|
|
END;
|
|
""";
|
|
cmd.ExecuteNonQuery();
|
|
}
|
|
|
|
private static (SqliteAuditWriter writer, string dataSource) CreateWriter(string testName)
|
|
{
|
|
var dataSource = $"file:{testName}-{Guid.NewGuid():N}?mode=memory&cache=shared";
|
|
var opts = new SqliteAuditWriterOptions
|
|
{
|
|
DatabasePath = dataSource,
|
|
// Generous flush window: both writes below are enqueued back-to-back with no
|
|
// await between them, so they are already both in the channel long before the
|
|
// background loop's first drain attempt — this just gives extra margin so the
|
|
// test can't flake into two separate flushes on a slow CI box.
|
|
FlushIntervalMs = 1000,
|
|
};
|
|
var writer = new SqliteAuditWriter(
|
|
Options.Create(opts),
|
|
NullLogger<SqliteAuditWriter>.Instance,
|
|
new FakeNodeIdentityProvider(),
|
|
connectionStringOverride: $"Data Source={dataSource};Cache=Shared");
|
|
return (writer, dataSource);
|
|
}
|
|
|
|
[Fact]
|
|
public async Task FlushBatch_MidBatchInsertFailure_FaultsEveryRowInBatch_NotJustTheFailingRow()
|
|
{
|
|
var (writer, dataSource) = CreateWriter(
|
|
nameof(FlushBatch_MidBatchInsertFailure_FaultsEveryRowInBatch_NotJustTheFailingRow));
|
|
await using var _ = writer;
|
|
|
|
InstallFailureTrigger(writer);
|
|
|
|
// evt1 is an ordinary row — its own two INSERTs succeed. evt2 carries the marker and
|
|
// throws mid-batch, from WITHIN the same transaction as evt1's already-successful
|
|
// inserts. Fired without individually awaiting so both land in the channel before the
|
|
// background loop's first drain — i.e. the same FlushBatch transaction.
|
|
var evt1 = NewEvent("normal-row");
|
|
var evt2 = NewEvent(FailMarker);
|
|
|
|
var t1 = writer.WriteAsync(evt1);
|
|
var t2 = writer.WriteAsync(evt2);
|
|
|
|
try
|
|
{
|
|
await Task.WhenAll(t1, t2).WaitAsync(TimeSpan.FromSeconds(10));
|
|
}
|
|
catch
|
|
{
|
|
// Expected: both tasks fault. Individually asserted below so a failure here
|
|
// reports which one didn't behave as expected, not just "something threw".
|
|
}
|
|
|
|
// THE regression assertion: evt1's own inserts succeeded, but the batch it shared
|
|
// with evt2 rolled back — its Task must fault, not silently report success. Pre-fix,
|
|
// t1 would already be RanToCompletion at this point (TrySetResult ran inside the loop
|
|
// before evt2's insert threw), and the later TrySetException in the outer catch would
|
|
// be a silent no-op against the already-completed TaskCompletionSource.
|
|
Assert.True(t1.IsFaulted,
|
|
"evt1's row was rolled back along with the rest of the batch, so its Task must fault instead of reporting false success.");
|
|
Assert.True(t2.IsFaulted, "evt2's own insert threw directly and must fault its Task.");
|
|
|
|
// No commit ever happened for this batch.
|
|
Assert.Equal(0, writer.FlushCountForTests);
|
|
|
|
// And the database genuinely holds neither row — proves the rollback actually
|
|
// happened, not just that the Task-completion contract changed.
|
|
using var verify = new SqliteConnection($"Data Source={dataSource};Cache=Shared");
|
|
verify.Open();
|
|
using var countCmd = verify.CreateCommand();
|
|
countCmd.CommandText = "SELECT COUNT(*) FROM audit_event;";
|
|
Assert.Equal(0L, (long)countCmd.ExecuteScalar()!);
|
|
}
|
|
|
|
[Fact]
|
|
public async Task FallbackAuditWriter_MidBatchInsertFailure_RingsEveryFaultedRow_AndIncrementsFailureCounterForEach()
|
|
{
|
|
// End-to-end proof the fix actually closes the loop the finding describes:
|
|
// FallbackAuditWriter composes the REAL SqliteAuditWriter as its primary. Before the
|
|
// fix, evt1's primary write would appear to SUCCEED (its Task completed before the
|
|
// rollback), so FallbackAuditWriter would never route it to the ring or count it as a
|
|
// failure — the whole batch would be silently lost with SiteAuditWriteFailures staying
|
|
// at 0, exactly as the finding describes.
|
|
var (writer, _) = CreateWriter(
|
|
nameof(FallbackAuditWriter_MidBatchInsertFailure_RingsEveryFaultedRow_AndIncrementsFailureCounterForEach));
|
|
await using var _w = writer;
|
|
|
|
InstallFailureTrigger(writer);
|
|
|
|
var ring = new RingBufferFallback(capacity: 16);
|
|
var counter = Substitute.For<IAuditWriteFailureCounter>();
|
|
var fallback = new FallbackAuditWriter(
|
|
writer, ring, counter, NullLogger<FallbackAuditWriter>.Instance);
|
|
|
|
var evt1 = NewEvent("normal-row");
|
|
var evt2 = NewEvent(FailMarker);
|
|
|
|
// FallbackAuditWriter.WriteAsync never throws (audit failures are always swallowed at
|
|
// that layer) — fired concurrently, same reasoning as above, so both land in one
|
|
// FlushBatch transaction.
|
|
var t1 = fallback.WriteAsync(evt1);
|
|
var t2 = fallback.WriteAsync(evt2);
|
|
await Task.WhenAll(t1, t2).WaitAsync(TimeSpan.FromSeconds(10));
|
|
|
|
// Both rows — including the one whose own INSERT succeeded before the batch rolled
|
|
// back — must have been routed to the ring and counted as a primary failure.
|
|
Assert.Equal(2, ring.Count);
|
|
counter.Received(2).Increment();
|
|
}
|
|
}
|