diff --git a/docs/requirements/Component-NotificationOutbox.md b/docs/requirements/Component-NotificationOutbox.md index 8cc443ab..a5918759 100644 --- a/docs/requirements/Component-NotificationOutbox.md +++ b/docs/requirements/Component-NotificationOutbox.md @@ -125,7 +125,7 @@ The dispatcher loop runs on a fixed interval. On each tick the `NotificationOutb Each delivery attempt also writes a `Notification.Attempt` row to the central `AuditLog` via `ICentralAuditWriter`; a transition to a terminal status (`Delivered` / `Parked` / `Discarded`) writes a `Notification.Terminal` row. Audit writes are **direct** (no telemetry — the dispatcher runs at central), insert-if-not-exists on `EventId`. The site-emitted `Notification.Enqueued` row arrives separately via the standard audit telemetry channel from the site's SQLite write-buffer, so the full per-notification audit trail is `Enqueued` (site-originated) → `Attempt` × N (central direct-write) → `Terminal` (central direct-write). See [Component-AuditLog.md](Component-AuditLog.md), Central direct-write (central-originated events). -The operational `Notifications` table remains the **source of truth** for the dispatcher and for Retry/Discard actions; the `AuditLog` rows are immutable shadows. Operator Retry/Discard still mutates only the `Notifications` row, and each transition emits the corresponding `Notification.Attempt` / `Notification.Terminal` audit row. +The operational `Notifications` table remains the **source of truth** for the dispatcher and for Retry/Discard actions; the `AuditLog` rows are immutable shadows. Operator Retry/Discard still mutates only the `Notifications` row, and each transition emits the corresponding audit row **carrying the operator identity as `Actor`** so an operator action is attributable: an operator **Retry** on a parked notification emits a `Notification`-channel `NotifyDeliver` row with status `Submitted` (recording *who un-parked it* — otherwise the trail reads `Parked → Attempt → Delivered` with no operator on record); a **Discard** emits the `Notification.Terminal` row with the operator as `Actor`. The username is captured at the Central UI and flows in on `RetryNotificationRequest` / `DiscardNotificationRequest` (`RequestedBy`); it is `null` (and the `Actor` falls back to `system`) only where no authenticated identity exists. These operator-attributed rows are best-effort like every other audit write — a writer failure never aborts the Retry/Discard. **Audit-write failure never affects delivery.** If the `ICentralAuditWriter` direct-write fails (transient DB error, schema lock, etc.) the dispatcher logs the failure and increments the `CentralAuditWriteFailures` health metric (see Health Monitoring #11), but the delivery attempt's outcome on the `Notifications` row stands. The audit row is recovered by re-emission on the next dispatcher tick or by the on-startup reconciliation sweep; central never aborts a notification because audit failed. diff --git a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Notifications/NotificationReport.razor b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Notifications/NotificationReport.razor index 53743a8c..b93435a6 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Notifications/NotificationReport.razor +++ b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Notifications/NotificationReport.razor @@ -10,6 +10,7 @@ @inject SiteScopeService SiteScope @inject IDialogService Dialog @inject ILogger Logger +@inject AuthenticationStateProvider AuthStateProvider
@@ -481,8 +482,9 @@ _actionInProgress = true; try { + var requestedBy = await AuthStateProvider.GetCurrentUsernameAsync(); var response = await CommunicationService.RetryNotificationAsync( - new RetryNotificationRequest(Guid.NewGuid().ToString("N"), n.NotificationId)); + new RetryNotificationRequest(Guid.NewGuid().ToString("N"), n.NotificationId, requestedBy)); if (response.Success) { _toast.ShowSuccess($"Notification {ShortId(n.NotificationId)} re-queued for delivery."); @@ -517,8 +519,9 @@ _actionInProgress = true; try { + var requestedBy = await AuthStateProvider.GetCurrentUsernameAsync(); var response = await CommunicationService.DiscardNotificationAsync( - new DiscardNotificationRequest(Guid.NewGuid().ToString("N"), n.NotificationId)); + new DiscardNotificationRequest(Guid.NewGuid().ToString("N"), n.NotificationId, requestedBy)); if (response.Success) { _toast.ShowSuccess($"Notification {ShortId(n.NotificationId)} discarded."); diff --git a/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Notification/NotificationOutboxQueries.cs b/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Notification/NotificationOutboxQueries.cs index 8f2e4abe..6808dafa 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Notification/NotificationOutboxQueries.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Notification/NotificationOutboxQueries.cs @@ -51,9 +51,17 @@ public record NotificationOutboxQueryResponse( /// /// Outbox UI -> Central: request to immediately retry delivery of a notification. /// +/// Request/response correlation id. +/// The notification to re-queue. +/// +/// The authenticated operator who initiated the retry, stamped as the Actor on +/// the emitted audit row so an un-park is attributable (Task 13). Additive; null where +/// no identity is available (legacy senders compile unchanged). +/// public record RetryNotificationRequest( string CorrelationId, - string NotificationId); + string NotificationId, + string? RequestedBy = null); /// /// Central -> Outbox UI: result of a . @@ -66,9 +74,16 @@ public record RetryNotificationResponse( /// /// Outbox UI -> Central: request to discard (cancel) a pending or stuck notification. /// +/// Request/response correlation id. +/// The notification to discard. +/// +/// The authenticated operator who initiated the discard, stamped as the Actor on +/// the emitted terminal audit row (Task 13). Additive; null where no identity is available. +/// public record DiscardNotificationRequest( string CorrelationId, - string NotificationId); + string NotificationId, + string? RequestedBy = null); /// /// Central -> Outbox UI: result of a . diff --git a/src/ZB.MOM.WW.ScadaBridge.NotificationOutbox/NotificationOutboxActor.cs b/src/ZB.MOM.WW.ScadaBridge.NotificationOutbox/NotificationOutboxActor.cs index 7d1b1c6b..22d2da81 100644 --- a/src/ZB.MOM.WW.ScadaBridge.NotificationOutbox/NotificationOutboxActor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.NotificationOutbox/NotificationOutboxActor.cs @@ -587,12 +587,14 @@ public class NotificationOutboxActor : ReceiveActor, IWithTimers private async Task EmitTerminalAuditAsync( Notification notification, DateTimeOffset now, - string? errorMessage) + string? errorMessage, + string? actorOverride = null) { try { var terminalStatus = MapNotificationStatusToAuditStatus(notification.Status); - var evt = BuildNotifyDeliverEvent(notification, now, terminalStatus, errorMessage); + var evt = BuildNotifyDeliverEvent( + notification, now, terminalStatus, errorMessage, actorOverride: actorOverride); await _auditWriter.WriteAsync(evt); } catch (Exception ex) @@ -677,7 +679,8 @@ public class NotificationOutboxActor : ReceiveActor, IWithTimers DateTimeOffset now, AuditStatus status, string? errorMessage, - int? durationMs = null) + int? durationMs = null, + string? actorOverride = null) { Guid? correlationId = Guid.TryParse(notification.NotificationId, out var parsed) ? parsed @@ -688,11 +691,12 @@ public class NotificationOutboxActor : ReceiveActor, IWithTimers kind: AuditKind.NotifyDeliver, status: status, occurredAtUtc: now.UtcDateTime, - // Central dispatch — a system identity per the Actor-column spec; - // there is no per-call authenticated user here. The originating - // script is still captured on SourceScript (and on the upstream - // NotifySend row). - actor: SystemActor, + // Central dispatch is a system identity per the Actor-column spec — + // there is no per-call authenticated user for automatic delivery. An + // operator-initiated transition (Retry/Discard) passes actorOverride + // so the row is attributable to the person who un-parked/cancelled it + // (Task 13). The originating script is still captured on SourceScript. + actor: actorOverride ?? SystemActor, target: notification.ListName, correlationId: correlationId, // ExecutionId: the originating script execution's id, @@ -973,9 +977,41 @@ public class NotificationOutboxActor : ReceiveActor, IWithTimers notification.LastError = null; await repository.UpdateAsync(notification); + // Operator re-queued a parked notification. Emit a Submitted NotifyDeliver + // row attributing the un-park to the operator — otherwise the lifecycle + // reads Parked → Attempted → Delivered with no record of who un-parked it + // (Task 13). Best-effort: audit failure never aborts the retry. + await EmitRetrySubmittedAuditAsync(notification, DateTimeOffset.UtcNow, request.RequestedBy); + return new RetryNotificationResponse(request.CorrelationId, Success: true, ErrorMessage: null); } + /// + /// Emits a single row with + /// recording that + /// re-queued a parked notification. Best-effort with the same + /// audit-never-affects-the-action shape as . + /// + private async Task EmitRetrySubmittedAuditAsync( + Notification notification, + DateTimeOffset now, + string? actorOverride) + { + try + { + var evt = BuildNotifyDeliverEvent( + notification, now, AuditStatus.Submitted, errorMessage: null, actorOverride: actorOverride); + await _auditWriter.WriteAsync(evt); + } + catch (Exception ex) + { + _logger.LogWarning( + ex, + "Failed to emit Submitted (operator retry) audit row for notification {NotificationId}.", + notification.NotificationId); + } + } + /// /// Handles a manual discard request. Only a Parked notification can be discarded; /// it is moved to the terminal Discarded status. @@ -1018,7 +1054,10 @@ public class NotificationOutboxActor : ReceiveActor, IWithTimers // Emit a Discarded NotifyDeliver row to match the dispatcher's // Delivered/Parked emissions; the row carries no error message because // the discard is an operator-driven cancellation, not a delivery error. - await EmitTerminalAuditAsync(notification, DateTimeOffset.UtcNow, errorMessage: null); + // The operator identity is stamped as Actor so the cancellation is + // attributable (Task 13). + await EmitTerminalAuditAsync( + notification, DateTimeOffset.UtcNow, errorMessage: null, actorOverride: request.RequestedBy); return new DiscardNotificationResponse(request.CorrelationId, Success: true, ErrorMessage: null); } diff --git a/tests/ZB.MOM.WW.ScadaBridge.NotificationOutbox.Tests/NotificationOutboxActorRetryEmissionTests.cs b/tests/ZB.MOM.WW.ScadaBridge.NotificationOutbox.Tests/NotificationOutboxActorRetryEmissionTests.cs new file mode 100644 index 00000000..804b939f --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.NotificationOutbox.Tests/NotificationOutboxActorRetryEmissionTests.cs @@ -0,0 +1,155 @@ +using Akka.Actor; +using Akka.TestKit.Xunit2; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging.Abstractions; +using NSubstitute; +using ZB.MOM.WW.Audit; +using ZB.MOM.WW.ScadaBridge.Commons.Entities.Notifications; +using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories; +using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services; +using ZB.MOM.WW.ScadaBridge.Commons.Messages.Notification; +using ZB.MOM.WW.ScadaBridge.Commons.Types.Audit; +using ZB.MOM.WW.ScadaBridge.Commons.Types.Enums; +using ZB.MOM.WW.ScadaBridge.NotificationOutbox.Delivery; + +namespace ZB.MOM.WW.ScadaBridge.NotificationOutbox.Tests; + +/// +/// Task 13 (arch-review 04) — verifies that an operator-initiated Retry on a parked +/// notification emits a / +/// audit row with status and the operator as +/// Actor, closing the forensic gap (previously the lifecycle read +/// Parked → Attempted → Delivered with no record of who un-parked it). Also +/// asserts the audit write is best-effort — a throwing writer never fails the retry. +/// +public class NotificationOutboxActorRetryEmissionTests : TestKit +{ + private readonly INotificationOutboxRepository _outboxRepository = + Substitute.For(); + + private readonly RecordingCentralAuditWriter _auditWriter = new(); + + private sealed class RecordingCentralAuditWriter : ICentralAuditWriter + { + public List Events { get; } = new(); + public Func? OnWrite { get; set; } + + public Task WriteAsync(AuditEvent evt, CancellationToken ct = default) + { + lock (Events) + { + Events.Add(evt.AsRow()); + } + + return OnWrite?.Invoke(evt) ?? Task.CompletedTask; + } + } + + private IServiceProvider BuildServiceProvider() + { + var services = new ServiceCollection(); + services.AddScoped(_ => _outboxRepository); + return services.BuildServiceProvider(); + } + + private IActorRef CreateActor() + { + return Sys.ActorOf(Props.Create(() => new NotificationOutboxActor( + BuildServiceProvider(), + new NotificationOutboxOptions { DispatchInterval = TimeSpan.FromHours(1) }, + (ICentralAuditWriter)_auditWriter, + NullLogger.Instance))); + } + + private static Notification MakeParked(Guid? notificationId = null) + { + return new Notification( + (notificationId ?? Guid.NewGuid()).ToString("D"), + NotificationType.Email, + "ops-team", + "Tank overflow", + "Tank 3 level critical", + "site-1") + { + Status = NotificationStatus.Parked, + RetryCount = 5, + CreatedAt = DateTimeOffset.UtcNow, + }; + } + + private List EventsByStatus(AuditStatus status) + { + lock (_auditWriter.Events) + { + return _auditWriter.Events.Where(e => e.Status == status).ToList(); + } + } + + [Fact] + public void Retry_OnParked_Emits_Submitted_NotifyDeliver_AuditRow_With_Operator() + { + var notificationId = Guid.NewGuid(); + var notification = MakeParked(notificationId); + _outboxRepository.GetByIdAsync(notification.NotificationId, Arg.Any()) + .Returns(notification); + var actor = CreateActor(); + + actor.Tell(new RetryNotificationRequest( + CorrelationId: "test-corr", NotificationId: notification.NotificationId, RequestedBy: "jdoe")); + + ExpectMsg(r => r.Success); + AwaitAssert(() => + { + var submitted = EventsByStatus(AuditStatus.Submitted); + Assert.Single(submitted); + var evt = submitted[0]; + Assert.Equal(AuditChannel.Notification, evt.Channel); + Assert.Equal(AuditKind.NotifyDeliver, evt.Kind); + Assert.Equal("jdoe", evt.Actor); + Assert.Equal(notificationId, evt.CorrelationId); + }); + } + + [Fact] + public void Discard_OnParked_Emits_Discarded_AuditRow_With_Operator() + { + var notification = MakeParked(); + _outboxRepository.GetByIdAsync(notification.NotificationId, Arg.Any()) + .Returns(notification); + var actor = CreateActor(); + + actor.Tell(new DiscardNotificationRequest( + CorrelationId: "test-corr", NotificationId: notification.NotificationId, RequestedBy: "jdoe")); + + ExpectMsg(r => r.Success); + AwaitAssert(() => + { + var discarded = EventsByStatus(AuditStatus.Discarded); + Assert.Single(discarded); + Assert.Equal("jdoe", discarded[0].Actor); + }); + } + + [Fact] + public void Retry_AuditFailure_DoesNotFail_TheRetry() + { + // Audit is best-effort: a throwing writer must NOT abort the retry — the + // row is still flipped back to Pending and the response reports Success. + var notification = MakeParked(); + _outboxRepository.GetByIdAsync(notification.NotificationId, Arg.Any()) + .Returns(notification); + _auditWriter.OnWrite = _ => throw new InvalidOperationException("audit dead"); + var actor = CreateActor(); + + actor.Tell(new RetryNotificationRequest( + CorrelationId: "test-corr", NotificationId: notification.NotificationId, RequestedBy: "jdoe")); + + ExpectMsg(r => r.Success); + AwaitAssert(() => + { + _outboxRepository.Received(1).UpdateAsync( + Arg.Is(n => n.Status == NotificationStatus.Pending), + Arg.Any()); + }); + } +}