fix(notification-outbox): re-align Central UI sandbox Notify API with production

The script-analysis sandbox Notify surface was stale after the Notification
Outbox change: SandboxNotifyTarget.Send returned Task<NotificationResult> and
there was no Status method, while production NotifyTarget.Send returns
Task<string> (a NotificationId) plus NotifyHelper.Status. A script that
test-ran cleanly in the sandbox would not compile against the real site
runtime.

- Move the NotificationDeliveryStatus record from ScadaLink.SiteRuntime.Scripts
  into ScadaLink.Commons.Messages.Notification so both production and the
  CentralUI sandbox reference the exact same type (CentralUI does not, and
  should not, reference SiteRuntime). Production NotifyHelper.Status is
  otherwise untouched.
- Rewrite SandboxNotifyHelper/SandboxNotifyTarget to be a signature-faithful
  no-op fake: Send returns Task<string> (a fake NotificationId), Status returns
  Task<NotificationDeliveryStatus>. Production now enqueues into the site S&F
  engine, which has no central-side equivalent in the sandbox, so the fake no
  longer carries an INotificationDeliveryService.
- Add script-analysis tests proving a script using the new Notify shape both
  diagnoses clean and runs in the sandbox.
This commit is contained in:
Joseph Doherty
2026-05-19 03:44:34 -04:00
parent 4b61e29e27
commit c8b5871782
6 changed files with 106 additions and 42 deletions

View File

@@ -1,5 +1,6 @@
using System.Data.Common; using System.Data.Common;
using ScadaLink.Commons.Interfaces.Services; using ScadaLink.Commons.Interfaces.Services;
using ScadaLink.Commons.Messages.Notification;
namespace ScadaLink.CentralUI.ScriptAnalysis; namespace ScadaLink.CentralUI.ScriptAnalysis;
@@ -80,39 +81,62 @@ public class SandboxDatabaseHelper
} }
} }
/// <summary>
/// Sandbox mirror of <c>ScadaLink.SiteRuntime.Scripts.NotifyHelper</c> — the
/// <c>Notify</c> global. Signature-faithful to production so the same user code
/// (<c>Notify.To(...).Send(...)</c> / <c>Notify.Status(...)</c>) compiles
/// identically against both surfaces.
///
/// In the Notification Outbox design production no longer delivers notification
/// email inline — <c>Notify.Send</c> enqueues into the site Store-and-Forward
/// Engine and returns a <c>NotificationId</c>. The sandbox has no S&amp;F engine
/// and no central, so it is a pure no-op fake: <c>Send</c> returns a generated
/// fake id and <c>Status</c> returns a placeholder <see cref="NotificationDeliveryStatus"/>.
/// Nothing is delivered.
/// </summary>
public class SandboxNotifyHelper public class SandboxNotifyHelper
{ {
private readonly INotificationDeliveryService? _service;
private readonly string _instanceName; private readonly string _instanceName;
public SandboxNotifyHelper(INotificationDeliveryService? service, string instanceName) public SandboxNotifyHelper(string instanceName)
{ {
_service = service;
_instanceName = instanceName; _instanceName = instanceName;
} }
/// <summary>Selects the notification list to send to.</summary>
public SandboxNotifyTarget To(string listName) => public SandboxNotifyTarget To(string listName) =>
new(listName, _service, _instanceName); new(listName, _instanceName);
/// <summary>
/// Queries the delivery status of a previously-sent notification. The
/// sandbox never delivers, so this always reports the placeholder
/// <c>Unknown</c> status — it exists for signature fidelity with
/// <c>NotifyHelper.Status</c>.
/// </summary>
public Task<NotificationDeliveryStatus> Status(string notificationId) =>
Task.FromResult(new NotificationDeliveryStatus("Unknown", 0, null, null));
} }
/// <summary>
/// Sandbox mirror of <c>ScadaLink.SiteRuntime.Scripts.NotifyTarget</c> — the
/// target of <c>Notify.To("listName")</c>.
/// </summary>
public class SandboxNotifyTarget public class SandboxNotifyTarget
{ {
private readonly string _listName; private readonly string _listName;
private readonly INotificationDeliveryService? _service;
private readonly string _instanceName; private readonly string _instanceName;
internal SandboxNotifyTarget(string listName, INotificationDeliveryService? service, string instanceName) internal SandboxNotifyTarget(string listName, string instanceName)
{ {
_listName = listName; _listName = listName;
_service = service;
_instanceName = instanceName; _instanceName = instanceName;
} }
public Task<NotificationResult> Send(string subject, string message, CancellationToken cancellationToken = default) /// <summary>
{ /// Mirrors <c>NotifyTarget.Send</c> — returns a <c>NotificationId</c>. In
if (_service == null) /// the sandbox nothing is enqueued or delivered; a fake id is returned so
throw new ScriptSandboxException( /// the call type-checks identically to production.
$"Notify.To(\"{_listName}\").Send(...) — notification service not configured for Test Run."); /// </summary>
return _service.SendAsync(_listName, subject, message, _instanceName, cancellationToken); public Task<string> Send(string subject, string message, CancellationToken cancellationToken = default) =>
} Task.FromResult(Guid.NewGuid().ToString("N"));
} }

View File

@@ -13,9 +13,9 @@ namespace ScadaLink.CentralUI.ScriptAnalysis;
/// instance. With no instance bound they throw <see cref="ScriptSandboxException"/>; /// instance. With no instance bound they throw <see cref="ScriptSandboxException"/>;
/// with one bound (see <see cref="SandboxInstanceContext"/>) they route to it. /// with one bound (see <see cref="SandboxInstanceContext"/>) they route to it.
/// ///
/// <c>ExternalSystem</c>, <c>Database</c>, <c>Notify</c>, and /// <c>ExternalSystem</c>, <c>Database</c>, and <c>Scripts.CallShared</c> run
/// <c>Scripts.CallShared</c> run against central's real services and fire for /// against central's real services and fire for real; <c>Notify</c> is a
/// real — they do not depend on a bound instance. /// signature-faithful no-op fake. None of them depend on a bound instance.
/// </summary> /// </summary>
public class SandboxScriptHost public class SandboxScriptHost
{ {
@@ -58,8 +58,8 @@ public interface ISandboxInstanceGateway
/// the <c>Instance</c> global. Attribute and sibling-script access needs a real /// the <c>Instance</c> global. Attribute and sibling-script access needs a real
/// deployed instance: with no gateway wired it throws; with one (a bound /// deployed instance: with no gateway wired it throws; with one (a bound
/// instance) it routes cross-site. <c>ExternalSystem</c>/<c>Database</c>/ /// instance) it routes cross-site. <c>ExternalSystem</c>/<c>Database</c>/
/// <c>Notify</c>/<c>Scripts</c> run against central's real services regardless /// <c>Scripts</c> run against central's real services regardless of binding;
/// of binding. /// <c>Notify</c> is a signature-faithful no-op fake.
/// </summary> /// </summary>
public class SandboxInstanceContext public class SandboxInstanceContext
{ {
@@ -80,7 +80,7 @@ public class SandboxInstanceContext
_gateway = gateway; _gateway = gateway;
ExternalSystem = external ?? new SandboxExternalHelper(null, "<sandbox>"); ExternalSystem = external ?? new SandboxExternalHelper(null, "<sandbox>");
Database = database ?? new SandboxDatabaseHelper(null, "<sandbox>"); Database = database ?? new SandboxDatabaseHelper(null, "<sandbox>");
Notify = notify ?? new SandboxNotifyHelper(null, "<sandbox>"); Notify = notify ?? new SandboxNotifyHelper("<sandbox>");
Scripts = scripts ?? new SandboxScriptCallHelper(null); Scripts = scripts ?? new SandboxScriptCallHelper(null);
} }

View File

@@ -154,11 +154,12 @@ public class ScriptAnalysisService
/// scripts against <see cref="SandboxInboundScriptHost"/>. /// scripts against <see cref="SandboxInboundScriptHost"/>.
/// Pure logic + the supplied Parameters always work. /// Pure logic + the supplied Parameters always work.
/// For the SandboxScriptHost surface, <c>Attributes</c> still throws while /// For the SandboxScriptHost surface, <c>Attributes</c> still throws while
/// <c>External</c>, <c>Database</c>, and <c>Notify</c> are wired to /// <c>External</c> and <c>Database</c> are wired to central's real
/// central's real <see cref="IExternalSystemClient"/>, /// <see cref="IExternalSystemClient"/> and <see cref="IDatabaseGateway"/> —
/// <see cref="IDatabaseGateway"/>, and /// calls fire for real and have production-equivalent side effects (HTTP,
/// <see cref="INotificationDeliveryService"/> — calls fire for real and /// SQL). <c>Notify</c> is a signature-faithful no-op fake (production
/// have production-equivalent side effects (HTTP, SQL, SMTP). /// enqueues into the site Store-and-Forward Engine, which has no
/// central-side equivalent in the sandbox).
/// <c>CallShared</c> compiles and executes the named shared script in the /// <c>CallShared</c> compiles and executes the named shared script in the
/// same sandbox, with a recursion limit of /// same sandbox, with a recursion limit of
/// <see cref="SandboxMaxCallSharedDepth"/>. <c>CallScript</c> still throws /// <see cref="SandboxMaxCallSharedDepth"/>. <c>CallScript</c> still throws
@@ -269,10 +270,13 @@ public class ScriptAnalysisService
var externalClient = _services.GetService<IExternalSystemClient>(); var externalClient = _services.GetService<IExternalSystemClient>();
var databaseGateway = _services.GetService<IDatabaseGateway>(); var databaseGateway = _services.GetService<IDatabaseGateway>();
var notifyService = _services.GetService<INotificationDeliveryService>();
var external = new SandboxExternalHelper(externalClient, instanceLabel); var external = new SandboxExternalHelper(externalClient, instanceLabel);
var database = new SandboxDatabaseHelper(databaseGateway, instanceLabel); var database = new SandboxDatabaseHelper(databaseGateway, instanceLabel);
var notify = new SandboxNotifyHelper(notifyService, instanceLabel); // The Notification Outbox sandbox Notify is a pure no-op fake — it
// mirrors production signatures so scripts compile identically, but it
// does not deliver (production now enqueues into the site S&F engine,
// which has no central-side equivalent here).
var notify = new SandboxNotifyHelper(instanceLabel);
var compileCache = new Dictionary<string, Script<object>>(StringComparer.Ordinal); var compileCache = new Dictionary<string, Script<object>>(StringComparer.Ordinal);
var compileCacheLock = new object(); var compileCacheLock = new object();

View File

@@ -40,3 +40,18 @@ public record NotificationStatusResponse(
int RetryCount, int RetryCount,
string? LastError, string? LastError,
DateTimeOffset? DeliveredAt); DateTimeOffset? DeliveredAt);
/// <summary>
/// Notification Outbox: the delivery status of a notification, as returned to a
/// script by <c>Notify.Status(id)</c>.
///
/// <see cref="Status"/> is either a central status (<c>Pending</c>, <c>Retrying</c>,
/// <c>Delivered</c>, <c>Parked</c>, <c>Discarded</c>), the site-local <c>Forwarding</c>
/// state (the notification is still buffered at the site and has not yet been
/// forwarded/acked), or <c>Unknown</c> (no central row and not buffered locally).
/// </summary>
public record NotificationDeliveryStatus(
string Status,
int RetryCount,
string? LastError,
DateTimeOffset? DeliveredAt);

View File

@@ -529,18 +529,3 @@ public class ScriptRuntimeContext
} }
} }
} }
/// <summary>
/// Notification Outbox: the delivery status of a notification, as returned to a
/// script by <c>Notify.Status(id)</c>.
///
/// <see cref="Status"/> is either a central status (<c>Pending</c>, <c>Retrying</c>,
/// <c>Delivered</c>, <c>Parked</c>, <c>Discarded</c>), the site-local <c>Forwarding</c>
/// state (the notification is still buffered at the site and has not yet been
/// forwarded/acked), or <c>Unknown</c> (no central row and not buffered locally).
/// </summary>
public record NotificationDeliveryStatus(
string Status,
int RetryCount,
string? LastError,
DateTimeOffset? DeliveredAt);

View File

@@ -466,6 +466,42 @@ public class ScriptAnalysisServiceTests
Assert.Equal("42", result.ReturnValueJson); Assert.Equal("42", result.ReturnValueJson);
} }
[Fact]
public void NotifyOutboxShape_DiagnosesClean()
{
// Notification Outbox: the sandbox Notify surface must be
// signature-faithful to production NotifyHelper/NotifyTarget —
// Send returns Task<string> (a NotificationId) and Status takes that
// id. A script using the new shape must compile clean in the sandbox,
// exactly as it would against the real site runtime.
var resp = _svc.Diagnose(new DiagnoseRequest(
"var id = await Notify.To(\"ops\").Send(\"subj\", \"body\"); " +
"var st = await Notify.Status(id); " +
"return st.Status;"));
Assert.DoesNotContain(resp.Markers, m => m.Code.StartsWith("CS"));
Assert.DoesNotContain(resp.Markers, m => m.Code.StartsWith("SCADA"));
}
[Fact]
public async Task RunInSandbox_NotifyOutboxShape_StillRuns()
{
// The new Notify shape must also run end-to-end in the no-op sandbox:
// Send yields a fake NotificationId, Status yields a placeholder
// NotificationDeliveryStatus.
var result = await _svc.RunInSandboxAsync(
new SandboxRunRequest(
"var id = await Notify.To(\"ops\").Send(\"subj\", \"body\"); " +
"var st = await Notify.Status(id); " +
"return st.Status;",
Parameters: null,
TimeoutSeconds: null),
CancellationToken.None);
Assert.True(result.Success);
Assert.Equal("\"Unknown\"", result.ReturnValueJson);
}
[Fact] [Fact]
public async Task RunInSandbox_CapturesConsoleOutput() public async Task RunInSandbox_CapturesConsoleOutput()
{ {