test(audit): lock null-provider passthrough on CentralAuditWriter
Two follow-ups flagged by code review on Tasks 11/12: - Lock the back-compat contract for CentralAuditWriter's optional `nodeIdentity = null` ctor parameter with two explicit tests (`WriteAsync_PassesThroughCallerSourceNode_WhenNoProviderInjected` and `WriteAsync_LeavesSourceNodeNull_WhenNoProviderInjected`). The previous null-provider path was only exercised incidentally via legacy CentralAuditWriterTests setups; the new tests make the contract explicit and distinct from the "provider supplied, returns null" path. - Document why the catch-block log references `evt` rather than the post-stamp record: the three logged fields (EventId, Kind, Status) are immutable across the filter+stamp chain, so referencing either name is equivalent — but the comment warns future maintainers to switch names if they ever add a field the chain mutates (e.g. SourceNode).
This commit is contained in:
@@ -136,6 +136,13 @@ public sealed class CentralAuditWriter : ICentralAuditWriter
|
|||||||
// misbehaving custom counter does, swallowing here keeps the
|
// misbehaving custom counter does, swallowing here keeps the
|
||||||
// best-effort contract intact.
|
// best-effort contract intact.
|
||||||
}
|
}
|
||||||
|
// Log the input event's identifying fields. These three (EventId,
|
||||||
|
// Kind, Status) are immutable across the filter+stamp chain — the
|
||||||
|
// `with` clones above touch only SourceNode and IngestedAtUtc — so
|
||||||
|
// referencing `evt` here is intentional and equivalent to the
|
||||||
|
// stamped record for diagnostics. If you add a field here that the
|
||||||
|
// stamp chain DOES mutate (e.g., SourceNode), reference the latest
|
||||||
|
// post-stamp record name instead, not `evt`.
|
||||||
_logger.LogWarning(
|
_logger.LogWarning(
|
||||||
ex,
|
ex,
|
||||||
"CentralAuditWriter failed for EventId {EventId} (Kind={Kind}, Status={Status})",
|
"CentralAuditWriter failed for EventId {EventId} (Kind={Kind}, Status={Status})",
|
||||||
|
|||||||
@@ -180,4 +180,34 @@ public class CentralAuditWriterTests
|
|||||||
Arg.Is<AuditEvent>(e => e.SourceNode == null),
|
Arg.Is<AuditEvent>(e => e.SourceNode == null),
|
||||||
Arg.Any<CancellationToken>());
|
Arg.Any<CancellationToken>());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task WriteAsync_PassesThroughCallerSourceNode_WhenNoProviderInjected()
|
||||||
|
{
|
||||||
|
// Locks the back-compat contract for the optional `nodeIdentity = null`
|
||||||
|
// ctor parameter: when no provider is wired (e.g. legacy M4 test
|
||||||
|
// composition roots), the writer must not stamp — caller value passes
|
||||||
|
// through unmodified. Distinct code path from
|
||||||
|
// "provider supplied, returns null", which the test above covers.
|
||||||
|
var (writer, repo) = BuildWriterWithIdentity(nodeIdentity: null);
|
||||||
|
|
||||||
|
await writer.WriteAsync(NewEvent() with { SourceNode = "node-z" });
|
||||||
|
|
||||||
|
await repo.Received(1).InsertIfNotExistsAsync(
|
||||||
|
Arg.Is<AuditEvent>(e => e.SourceNode == "node-z"),
|
||||||
|
Arg.Any<CancellationToken>());
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task WriteAsync_LeavesSourceNodeNull_WhenNoProviderInjected()
|
||||||
|
{
|
||||||
|
// Same back-compat contract for the null-caller-null-provider case.
|
||||||
|
var (writer, repo) = BuildWriterWithIdentity(nodeIdentity: null);
|
||||||
|
|
||||||
|
await writer.WriteAsync(NewEvent());
|
||||||
|
|
||||||
|
await repo.Received(1).InsertIfNotExistsAsync(
|
||||||
|
Arg.Is<AuditEvent>(e => e.SourceNode == null),
|
||||||
|
Arg.Any<CancellationToken>());
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user