docs(components): accuracy fixes from deep review (batch 3)

NotificationService (Notify.Send returns string not NotificationId;
MaxConcurrentConnections unenforced; AddHttpClient), NotificationOutbox
(one Attempted row always, terminal row only on terminal status), SiteCallAudit
(direct dual-write, no Tell; KPI tiles consumed by CentralUI), HealthMonitoring
(CentralOfflineTimeout 180s = 6x ReportInterval; HealthReportSender gates on
IsActiveNode), SiteEventLogging (active-node purge seam not wired; runs on both
nodes), InboundAPI (whole System.Diagnostics namespace forbidden).
This commit is contained in:
Joseph Doherty
2026-06-03 16:37:15 -04:00
parent 25bae4e43b
commit 9175b0c013
6 changed files with 27 additions and 28 deletions
+11 -11
View File
@@ -7,7 +7,7 @@ The Notification Service is the central-only component that owns notification-li
Notification Service (#8) runs on the central cluster only. Its responsibilities split cleanly into two layers:
- **Definitions** — `NotificationList` and `SmtpConfiguration` entities stored in the central Configuration Database. Notification lists carry a `NotificationType` discriminator (`Email` now; additional types such as `Teams` are planned). Lists and SMTP config are never deployed to sites.
- **Delivery adapters** — stateless, per-type implementations of `INotificationDeliveryAdapter`. The Notification Outbox selects the adapter matching a notification's `Type`, calls `DeliverAsync`, and receives a three-way `DeliveryOutcome` (`Success` / `TransientFailure` / `PermanentFailure`). The adapter owns the full recipient-resolution, connection, authentication, send, and disconnect sequence.
- **Delivery adapters** — per-type implementations of `INotificationDeliveryAdapter`. The Notification Outbox selects the adapter matching a notification's `Type`, calls `DeliverAsync`, and receives a three-way `DeliveryOutcome` (`Success` / `TransientFailure` / `PermanentFailure`). The adapter owns the full recipient-resolution, connection, authentication, send, and disconnect sequence. `EmailNotificationDeliveryAdapter` is registered as scoped (it holds a scoped `INotificationRepository`) and the outbox actor caches a single instance for its lifetime.
The component code lives in `src/ZB.MOM.WW.ScadaBridge.NotificationService/`. The `EmailNotificationDeliveryAdapter` that consumes these primitives lives in `src/ZB.MOM.WW.ScadaBridge.NotificationOutbox/Delivery/`.
@@ -15,12 +15,12 @@ The component code lives in `src/ZB.MOM.WW.ScadaBridge.NotificationService/`. Th
### Central-only delivery
Before the current design, site nodes delivered notifications directly over SMTP. That arrangement required SMTP credentials and notification lists to be deployed to every site. The redesign inverts the path: a site script calls `Notify.To("list").Send(subject, body)`, receives a `NotificationId` GUID immediately, and the notification is store-and-forwarded to central. The Notification Outbox on central ingests it and calls the delivery adapter. Sites never open an SMTP connection.
Before the current design, site nodes delivered notifications directly over SMTP. That arrangement required SMTP credentials and notification lists to be deployed to every site. The redesign inverts the path: a site script calls `Notify.To("list").Send(subject, body)`, receives a `string` notification id immediately, and the notification is store-and-forwarded to central. The Notification Outbox on central ingests it and calls the delivery adapter. Sites never open an SMTP connection.
This means:
- Credential exposure is limited to the central cluster.
- List membership is resolved at delivery time, so a list change takes effect for all future deliveries without redeploying to sites.
- The SMTP `MaxConcurrentConnections` limit is enforced at a single point.
- The SMTP `MaxConcurrentConnections` value is configured at a single point, though it is not currently enforced (no connection gate or semaphore).
### `NotificationType` discriminator
@@ -56,7 +56,7 @@ public static IServiceCollection AddNotificationService(this IServiceCollection
}
```
Three things are registered: the `NotificationOptions` fallback values, the `OAuth2TokenService` token cache, and the `ISmtpClientWrapper` factory. The `EmailNotificationDeliveryAdapter` itself is registered by `ZB.MOM.WW.ScadaBridge.NotificationOutbox`, which depends on this project.
Four things are registered: the `NotificationOptions` fallback values, the `HttpClient` infrastructure (required by `OAuth2TokenService`), the `OAuth2TokenService` token cache, and the `ISmtpClientWrapper` factory. The `EmailNotificationDeliveryAdapter` itself is registered by `ZB.MOM.WW.ScadaBridge.NotificationOutbox`, which depends on this project.
### `INotificationDeliveryAdapter`
@@ -79,7 +79,7 @@ The `DeliveryOutcome` record carries a `DeliveryResult` (`Success` / `TransientF
1. **Resolve list** — calls `INotificationRepository.GetListByNameAsync`. An unknown list returns `Permanent` immediately (the list was deleted; retrying cannot fix it).
2. **Resolve recipients** — calls `GetRecipientsByListIdAsync`. An empty list returns `Permanent`.
3. **Resolve SMTP config** — calls `GetAllSmtpConfigurationsAsync`, takes the first row. No config returns `Permanent`.
4. **Parse TLS mode**`SmtpTlsModeParser.Parse(smtpConfig.TlsMode)`. An unrecognised string returns `Permanent` (config fault, not a transient network condition).
4. **Parse TLS mode**`SmtpTlsModeParser.Parse(smtpConfig.TlsMode)`. An unrecognised string throws `ArgumentException`; `DeliverAsync` catches it and returns `Permanent` (config fault, not a transient network condition).
5. **Validate addresses**`EmailAddressValidator.ValidateAddresses(fromAddress, recipients)`. A malformed address returns `Permanent`.
6. **Send** — calls the private `SendAsync`, which connect/auth/send/disconnects via a fresh `ISmtpClientWrapper`.
@@ -130,18 +130,18 @@ A `Permanent` classification inside `SendAsync` is wrapped in `SmtpPermanentExce
Site scripts do not interact with this component directly. The script surface is:
```csharp
// Returns a NotificationId immediately — does not block for delivery.
NotificationId id = Notify.To("Shift-Supervisors").Send("Tank overflow", "Tank T-03 is at 98%");
// Returns a string notification id immediately — does not block for delivery.
string id = await Notify.To("Shift-Supervisors").Send("Tank overflow", "Tank T-03 is at 98%");
// Site-local while still in the S&F buffer; round-trips to central once forwarded.
NotificationDeliveryStatus status = Notify.Status(id);
NotificationDeliveryStatus status = await Notify.Status(id);
```
`Notify.To("list")` is type-agnostic. The `NotificationId` is a GUID generated at the site. `Notify.Status` returns a `NotificationDeliveryStatus` record with `Status` (`Forwarding` site-local, or `Pending` / `Retrying` / `Delivered` / `Parked` / `Discarded` from central), `RetryCount`, `LastError`, and `DeliveredAt`.
`Notify.To("list")` is type-agnostic. The notification id is a 32-character "N"-format GUID string (`Guid.NewGuid().ToString("N")`) generated at the site. `Notify.Status(string notificationId)` returns a `NotificationDeliveryStatus` record with `Status` (`Forwarding` site-local, `Unknown` if no central row and not in the S&F buffer, or `Pending` / `Retrying` / `Delivered` / `Parked` / `Discarded` from central), `RetryCount`, `LastError`, and `DeliveredAt`.
### Registering the adapter
On the central host, both projects are registered. The Notification Outbox registers `EmailNotificationDeliveryAdapter` as a keyed or enumerated `INotificationDeliveryAdapter` and calls `AddNotificationService` to get its dependencies:
On the central host, both projects are registered. The Notification Outbox registers `EmailNotificationDeliveryAdapter` as a scoped concrete type and as a scoped `INotificationDeliveryAdapter`; the outbox actor resolves adapters by enumerating `IEnumerable<INotificationDeliveryAdapter>` (no keyed/named registration). `AddNotificationService` is called to register the shared SMTP primitives:
```csharp
// Central composition root (simplified)
@@ -156,7 +156,7 @@ services.AddNotificationOutbox(); // registers EmailNotificationDeliveryAdapte
| Section | Key | Default | Description |
|---------|-----|---------|-------------|
| `ScadaBridge:Notification` | `ConnectionTimeoutSeconds` | `30` | SMTP connection/operation timeout in seconds. Applied when `SmtpConfiguration.ConnectionTimeoutSeconds` is zero or negative. |
| `ScadaBridge:Notification` | `MaxConcurrentConnections` | `5` | Maximum concurrent SMTP connections. Used as the documented default; enforcement is in `EmailNotificationDeliveryAdapter`. |
| `ScadaBridge:Notification` | `MaxConcurrentConnections` | `5` | Maximum concurrent SMTP connections. Used as the documented fallback default when the `SmtpConfiguration` row is unset; this limit is not currently enforced by a connection gate or semaphore. |
SMTP retry settings (`MaxRetries`, `RetryDelay`) live on the `SmtpConfiguration` entity and are read by the Notification Outbox dispatcher — they are not part of `NotificationOptions`.