4 Commits

19 changed files with 1192 additions and 213 deletions
+9 -17
View File
@@ -40,10 +40,10 @@ module file and counted in **Total**.
| Severity | Open findings | | Severity | Open findings |
|----------|---------------| |----------|---------------|
| Critical | 0 | | Critical | 0 |
| High | 12 | | High | 3 |
| Medium | 100 | | Medium | 100 |
| Low | 89 | | Low | 90 |
| **Total** | **201** | | **Total** | **193** |
## Module Status ## Module Status
@@ -65,9 +65,9 @@ module file and counted in **Total**.
| [NotificationService](NotificationService/findings.md) | 2026-05-16 | `9c60592` | 0/0/5/3 | 8 | 12 | | [NotificationService](NotificationService/findings.md) | 2026-05-16 | `9c60592` | 0/0/5/3 | 8 | 12 |
| [Security](Security/findings.md) | 2026-05-16 | `9c60592` | 0/0/4/4 | 8 | 11 | | [Security](Security/findings.md) | 2026-05-16 | `9c60592` | 0/0/4/4 | 8 | 11 |
| [SiteEventLogging](SiteEventLogging/findings.md) | 2026-05-16 | `9c60592` | 0/0/4/3 | 7 | 11 | | [SiteEventLogging](SiteEventLogging/findings.md) | 2026-05-16 | `9c60592` | 0/0/4/3 | 7 | 11 |
| [SiteRuntime](SiteRuntime/findings.md) | 2026-05-16 | `9c60592` | 0/3/8/5 | 16 | 16 | | [SiteRuntime](SiteRuntime/findings.md) | 2026-05-16 | `9c60592` | 0/0/8/5 | 13 | 16 |
| [StoreAndForward](StoreAndForward/findings.md) | 2026-05-16 | `9c60592` | 0/2/4/6 | 12 | 14 | | [StoreAndForward](StoreAndForward/findings.md) | 2026-05-16 | `9c60592` | 0/0/4/7 | 11 | 14 |
| [TemplateEngine](TemplateEngine/findings.md) | 2026-05-16 | `9c60592` | 0/5/5/4 | 14 | 14 | | [TemplateEngine](TemplateEngine/findings.md) | 2026-05-16 | `9c60592` | 0/1/5/4 | 10 | 14 |
## Pending Findings ## Pending Findings
@@ -80,22 +80,13 @@ description, location, recommendation — lives in the module's `findings.md`.
_None open._ _None open._
### High (12) ### High (3)
| ID | Module | Title | | ID | Module | Title |
|----|--------|-------| |----|--------|-------|
| ClusterInfrastructure-001 | [ClusterInfrastructure](ClusterInfrastructure/findings.md) | Module implements none of its documented responsibilities | | ClusterInfrastructure-001 | [ClusterInfrastructure](ClusterInfrastructure/findings.md) | Module implements none of its documented responsibilities |
| DeploymentManager-006 | [DeploymentManager](DeploymentManager/findings.md) | Query-the-site-before-redeploy idempotency requirement not implemented | | DeploymentManager-006 | [DeploymentManager](DeploymentManager/findings.md) | Query-the-site-before-redeploy idempotency requirement not implemented |
| SiteRuntime-001 | [SiteRuntime](SiteRuntime/findings.md) | `Instance.SetAttribute` never writes to the Data Connection Layer |
| SiteRuntime-002 | [SiteRuntime](SiteRuntime/findings.md) | `RouteInboundApiSetAttributes` always treats writes as static overrides |
| SiteRuntime-003 | [SiteRuntime](SiteRuntime/findings.md) | Redeployment relies on a fixed 500 ms reschedule and can collide on the child actor name |
| StoreAndForward-002 | [StoreAndForward](StoreAndForward/findings.md) | Messages enqueued with no registered handler are buffered but never deliverable |
| StoreAndForward-003 | [StoreAndForward](StoreAndForward/findings.md) | Off-by-one in retry accounting: immediate failure pre-counts as retry 1 |
| TemplateEngine-001 | [TemplateEngine](TemplateEngine/findings.md) | Deeply nested composed members are dropped during flattening |
| TemplateEngine-002 | [TemplateEngine](TemplateEngine/findings.md) | Derived templates omit all base alarms; composed alarms cannot be overridden per slot | | TemplateEngine-002 | [TemplateEngine](TemplateEngine/findings.md) | Derived templates omit all base alarms; composed alarms cannot be overridden per slot |
| TemplateEngine-003 | [TemplateEngine](TemplateEngine/findings.md) | `UpdateAttributeAsync` lets a non-locked attribute change its fixed DataType / DataSourceReference |
| TemplateEngine-004 | [TemplateEngine](TemplateEngine/findings.md) | Alarm on-trigger script references are never resolved (empty placeholder) |
| TemplateEngine-005 | [TemplateEngine](TemplateEngine/findings.md) | Collision validation is skipped when creating a child template |
### Medium (100) ### Medium (100)
@@ -202,7 +193,7 @@ _None open._
| TemplateEngine-009 | [TemplateEngine](TemplateEngine/findings.md) | N+1 query in `TemplateDeletionService.CanDeleteTemplateAsync` | | TemplateEngine-009 | [TemplateEngine](TemplateEngine/findings.md) | N+1 query in `TemplateDeletionService.CanDeleteTemplateAsync` |
| TemplateEngine-010 | [TemplateEngine](TemplateEngine/findings.md) | `InstanceService` documents optimistic concurrency that is not implemented | | TemplateEngine-010 | [TemplateEngine](TemplateEngine/findings.md) | `InstanceService` documents optimistic concurrency that is not implemented |
### Low (89) ### Low (90)
| ID | Module | Title | | ID | Module | Title |
|----|--------|-------| |----|--------|-------|
@@ -285,6 +276,7 @@ _None open._
| SiteRuntime-014 | [SiteRuntime](SiteRuntime/findings.md) | Trigger-expression evaluation blocks the coordinator actor thread | | SiteRuntime-014 | [SiteRuntime](SiteRuntime/findings.md) | Trigger-expression evaluation blocks the coordinator actor thread |
| SiteRuntime-015 | [SiteRuntime](SiteRuntime/findings.md) | `LoggerFactory` created per Instance Actor and never disposed | | SiteRuntime-015 | [SiteRuntime](SiteRuntime/findings.md) | `LoggerFactory` created per Instance Actor and never disposed |
| SiteRuntime-016 | [SiteRuntime](SiteRuntime/findings.md) | Short-lived execution actors, replication actor, and repositories are untested | | SiteRuntime-016 | [SiteRuntime](SiteRuntime/findings.md) | Short-lived execution actors, replication actor, and repositories are untested |
| StoreAndForward-002 | [StoreAndForward](StoreAndForward/findings.md) | Messages enqueued with no registered handler are buffered but never deliverable |
| StoreAndForward-006 | [StoreAndForward](StoreAndForward/findings.md) | `GetParkedMessagesAsync` count and page run without a transaction | | StoreAndForward-006 | [StoreAndForward](StoreAndForward/findings.md) | `GetParkedMessagesAsync` count and page run without a transaction |
| StoreAndForward-007 | [StoreAndForward](StoreAndForward/findings.md) | Async work in `ParkedMessageHandlerActor` uses `ContinueWith` without scheduler/affinity guarantees | | StoreAndForward-007 | [StoreAndForward](StoreAndForward/findings.md) | Async work in `ParkedMessageHandlerActor` uses `ContinueWith` without scheduler/affinity guarantees |
| StoreAndForward-008 | [StoreAndForward](StoreAndForward/findings.md) | A SQLite connection is opened and torn down on every storage call | | StoreAndForward-008 | [StoreAndForward](StoreAndForward/findings.md) | A SQLite connection is opened and torn down on every storage call |
+29 -7
View File
@@ -8,7 +8,7 @@
| Last reviewed | 2026-05-16 | | Last reviewed | 2026-05-16 |
| Reviewer | claude-agent | | Reviewer | claude-agent |
| Commit reviewed | `9c60592` | | Commit reviewed | `9c60592` |
| Open findings | 16 | | Open findings | 13 |
## Summary ## Summary
@@ -51,7 +51,7 @@ actor, and the repositories are untested.
|--|--| |--|--|
| Severity | High | | Severity | High |
| Category | Design-document adherence | | Category | Design-document adherence |
| Status | Open | | Status | Resolved |
| Location | `src/ScadaLink.SiteRuntime/Scripts/ScriptRuntimeContext.cs:106`, `src/ScadaLink.SiteRuntime/Actors/InstanceActor.cs:204` | | Location | `src/ScadaLink.SiteRuntime/Scripts/ScriptRuntimeContext.cs:106`, `src/ScadaLink.SiteRuntime/Actors/InstanceActor.cs:204` |
**Description** **Description**
@@ -84,7 +84,15 @@ or branching inside the handler.
**Resolution** **Resolution**
_Unresolved._ Resolved 2026-05-16 (`<pending>`): `InstanceActor.HandleSetStaticAttribute` now resolves
the target attribute's data binding from `_configuration`. Data-sourced attributes are
routed via a new `HandleSetDataAttribute` that Asks the DCL with a `WriteTagRequest` and
pipes the device-write outcome back to the caller as a `SetStaticAttributeResponse`
no override is persisted and `_attributes` is not optimistically mutated. Static
attributes keep the override path and now also reply with a `SetStaticAttributeResponse`.
`ScriptRuntimeContext.SetAttribute` is now `async Task` and Asks the Instance Actor,
throwing `InvalidOperationException` on a failed device write so scripts get the failure
synchronously.
### SiteRuntime-002 — `RouteInboundApiSetAttributes` always treats writes as static overrides ### SiteRuntime-002 — `RouteInboundApiSetAttributes` always treats writes as static overrides
@@ -92,7 +100,7 @@ _Unresolved._
|--|--| |--|--|
| Severity | High | | Severity | High |
| Category | Correctness & logic bugs | | Category | Correctness & logic bugs |
| Status | Open | | Status | Resolved |
| Location | `src/ScadaLink.SiteRuntime/Actors/DeploymentManagerActor.cs:632` | | Location | `src/ScadaLink.SiteRuntime/Actors/DeploymentManagerActor.cs:632` |
**Description** **Description**
@@ -115,7 +123,13 @@ response path.
**Resolution** **Resolution**
_Unresolved._ Resolved 2026-05-16 (`<pending>`): `RouteInboundApiSetAttributes` now Asks the Instance
Actor per attribute (instead of fire-and-forget Tell) and aggregates the
`SetStaticAttributeResponse` results. Because the Instance Actor handler is the
SiteRuntime-001 corrected handler, data-sourced attributes now reach the DCL and the
`RouteToSetAttributesResponse` reflects the real per-attribute outcome — a non-existent
attribute or a failed device write is reported as failure rather than an unconditional
optimistic `true`.
### SiteRuntime-003 — Redeployment relies on a fixed 500 ms reschedule and can collide on the child actor name ### SiteRuntime-003 — Redeployment relies on a fixed 500 ms reschedule and can collide on the child actor name
@@ -123,7 +137,7 @@ _Unresolved._
|--|--| |--|--|
| Severity | High | | Severity | High |
| Category | Akka.NET conventions | | Category | Akka.NET conventions |
| Status | Open | | Status | Resolved |
| Location | `src/ScadaLink.SiteRuntime/Actors/DeploymentManagerActor.cs:222` | | Location | `src/ScadaLink.SiteRuntime/Actors/DeploymentManagerActor.cs:222` |
**Description** **Description**
@@ -148,7 +162,15 @@ instance) until termination completes.
**Resolution** **Resolution**
_Unresolved._ Resolved 2026-05-16 (`<pending>`): `HandleDeploy` no longer uses a fixed 500 ms
reschedule. When a redeployment targets a running instance, the existing Instance Actor
is `Context.Watch`-ed and stopped, and the in-flight `DeployInstanceCommand` is buffered
in a `_pendingRedeploys` map keyed by the terminating actor ref. A new `Terminated`
handler recreates the Instance Actor only after the predecessor (and its whole subtree)
has fully stopped, eliminating the `InvalidActorNameException` race and the
unconditional redeploy-latency penalty. The shared `ApplyDeployment` helper also skips
the `_totalDeployedCount` increment for redeployments, so the deployed-instance count no
longer drifts (this additionally addresses the root cause behind SiteRuntime-004).
### SiteRuntime-004 — `_totalDeployedCount` is incremented on redeployment of an existing instance ### SiteRuntime-004 — `_totalDeployedCount` is incremented on redeployment of an existing instance
+50 -5
View File
@@ -8,7 +8,7 @@
| Last reviewed | 2026-05-16 | | Last reviewed | 2026-05-16 |
| Reviewer | claude-agent | | Reviewer | claude-agent |
| Commit reviewed | `9c60592` | | Commit reviewed | `9c60592` |
| Open findings | 12 | | Open findings | 11 |
## Summary ## Summary
@@ -94,7 +94,8 @@ commit whose message references `StoreAndForward-001`.
| | | | | |
|--|--| |--|--|
| Severity | High | | Severity | Low |
| Original severity | High (re-triaged down to Low on 2026-05-16 — see Re-triage note) |
| Category | Error handling & resilience | | Category | Error handling & resilience |
| Status | Open | | Status | Open |
| Location | `src/ScadaLink.StoreAndForward/StoreAndForwardService.cs:162`, `:201` | | Location | `src/ScadaLink.StoreAndForward/StoreAndForwardService.cs:162`, `:201` |
@@ -121,9 +122,39 @@ handler exists rather than silently buffering an undeliverable message, and wire
registration is intended, the retry sweep should treat a still-missing handler as a registration is intended, the retry sweep should treat a still-missing handler as a
transient condition with bounded logging rather than a permanent no-op. transient condition with bounded logging rather than a permanent no-op.
**Re-triage note (2026-05-16)**
The finding's central factual claim — *"No caller in the codebase ever calls
`RegisterDeliveryHandler`"* and therefore *"every buffered message lands in this dead
state"* — is **no longer true at the reviewed code**. `ScadaLink.Host`
(`AkkaHostedService.RegisterSiteActors`, `AkkaHostedService.cs:353-379`) registers all
three delivery handlers (`ExternalSystem`, `CachedDbWrite`, `Notification`) at site
startup, immediately after `StoreAndForwardService.StartAsync()`. The finding was
written against commit `9c60592` before that wiring existed; the High-severity
"engine cannot deliver anything" outcome no longer occurs.
The remaining residual risk is narrow: a message enqueued for a category that genuinely
has no handler (e.g. an enqueue racing ahead of `RegisterDeliveryHandler`, or a future
category added without a handler) is still buffered and then skipped by the sweep
forever. That is a real but minor robustness gap, hence the **downgrade to Low**.
It is left **Open** rather than fixed in this pass because the finding's recommended
fix — making `EnqueueAsync` reject when no handler is registered — is a behavioural
contract change, not a localised bug fix: the "buffer with no handler yet" path is
exercised by `StoreAndForwardReplicationTests` and by three NotificationService and
ExternalSystemGateway tests (`Send_TransientError_WithStoreAndForward_BuffersMessage`,
`Send_Smtp4xxCommandException_ClassifiedTransientAndBuffered`,
`Send_SmtpProtocolException_ClassifiedTransient`) which construct a real
`StoreAndForwardService` without registering a handler and assert `WasBuffered`.
Changing the contract requires deciding whether late handler registration is supported
and updating tests in modules outside this review's edit scope — a design decision that
should be made deliberately rather than forced here.
**Resolution** **Resolution**
_Unresolved._ _Open — re-triaged to Low. Premise (no handler registration anywhere) is stale: Host
now wires all three handlers. Residual gap is minor and the prescribed fix is a
cross-module contract change needing a design decision._
### StoreAndForward-003 — Off-by-one in retry accounting: immediate failure pre-counts as retry 1 ### StoreAndForward-003 — Off-by-one in retry accounting: immediate failure pre-counts as retry 1
@@ -131,7 +162,7 @@ _Unresolved._
|--|--| |--|--|
| Severity | High | | Severity | High |
| Category | Correctness & logic bugs | | Category | Correctness & logic bugs |
| Status | Open | | Status | Resolved |
| Location | `src/ScadaLink.StoreAndForward/StoreAndForwardService.cs:153`, `:229`, `:233` | | Location | `src/ScadaLink.StoreAndForward/StoreAndForwardService.cs:153`, `:229`, `:233` |
**Description** **Description**
@@ -159,7 +190,21 @@ the comparison. Update the affected test to match the chosen semantics.
**Resolution** **Resolution**
_Unresolved._ Resolved 2026-05-16 (commit `<pending>`). `RetryCount` now consistently means "number
of background retry-sweep attempts so far"; the initial immediate (or caller-made)
delivery attempt is attempt 0 and is not counted, and `MaxRetries` bounds retry-sweep
attempts after that initial attempt. `EnqueueAsync` no longer seeds `RetryCount = 1` on
either the transient-immediate-failure path or the `attemptImmediateDelivery: false`
path — a freshly buffered message has `RetryCount = 0`. `RetryMessageAsync` already
increments before the `>= MaxRetries` check, which is now correct, so a message with
`MaxRetries = 1` gets exactly one real retry before parking (previously zero). The
`StoreAndForwardMessage.RetryCount` XML doc was corrected to match. Regression test
`RetryPendingMessagesAsync_MaxRetriesOne_PerformsExactlyOneRetryBeforeParking` asserts
the immediate attempt plus exactly one retry occur before parking; the affected
existing tests (`EnqueueAsync_TransientFailure_BuffersForRetry`,
`EnqueueAsync_AttemptImmediateDeliveryFalse_BuffersWithoutInvokingHandler`,
`RetryPendingMessagesAsync_MaxRetriesReached_ParksMessage`) were updated to the
corrected semantics.
### StoreAndForward-004 — `RegisterDeliveryHandler` XML doc contradicts the implemented contract ### StoreAndForward-004 — `RegisterDeliveryHandler` XML doc contradicts the implemented contract
+52 -10
View File
@@ -8,7 +8,7 @@
| Last reviewed | 2026-05-16 | | Last reviewed | 2026-05-16 |
| Reviewer | claude-agent | | Reviewer | claude-agent |
| Commit reviewed | `9c60592` | | Commit reviewed | `9c60592` |
| Open findings | 14 | | Open findings | 10 |
## Summary ## Summary
@@ -52,7 +52,7 @@ of attributes vs. alarms vs. scripts throughout the resolve/flatten/derive paths
|--|--| |--|--|
| Severity | High | | Severity | High |
| Category | Correctness & logic bugs | | Category | Correctness & logic bugs |
| Status | Open | | Status | Resolved |
| Location | `src/ScadaLink.TemplateEngine/Flattening/FlatteningService.cs:211`, `src/ScadaLink.TemplateEngine/Flattening/FlatteningService.cs:535`, `src/ScadaLink.TemplateEngine/Flattening/FlatteningService.cs:609` | | Location | `src/ScadaLink.TemplateEngine/Flattening/FlatteningService.cs:211`, `src/ScadaLink.TemplateEngine/Flattening/FlatteningService.cs:535`, `src/ScadaLink.TemplateEngine/Flattening/FlatteningService.cs:609` |
**Description** **Description**
@@ -77,7 +77,13 @@ the recursion already in `TemplateResolver.AddComposedMembers` and
**Resolution** **Resolution**
_Unresolved._ Resolved 2026-05-16 (commit `<pending>`): replaced the hand-unrolled
one/two-level composition loops in `ResolveComposedAttributes`,
`ResolveComposedAlarms`, and `ResolveComposedScripts` with single recursive
walks (`*Recursive` helpers) carrying the accumulated path prefix and a
`visited` set, so composed members at arbitrary nesting depth are resolved.
Regression tests: `Flatten_ThreeLevelComposition_AttributesAlarmsScriptsAllResolved`,
`Flatten_NestedComposedAlarm_TriggerAttributePrefixed`.
### TemplateEngine-002 — Derived templates omit all base alarms; composed alarms cannot be overridden per slot ### TemplateEngine-002 — Derived templates omit all base alarms; composed alarms cannot be overridden per slot
@@ -110,7 +116,22 @@ already do.
**Resolution** **Resolution**
_Unresolved._ _Unresolved (re-triaged 2026-05-16)._ Partially mis-stated and out of the
current fix scope. Correction to the description: composed/inherited alarms
are **not** dropped from the flattened deployment output — `FlatteningService`
resolves alarms from the entire inheritance chain (`ResolveInheritedAlarms`
walks `templateChain`, which includes the base of a derived template), so an
instance of a derived template still receives the base template's alarms. The
real, valid gap is narrower: there is no per-slot **alarm override**
mechanism. The fix genuinely requires adding `IsInherited` / `LockedInDerived`
fields to the `TemplateAlarm` entity, which lives in `ScadaLink.Commons`
(a different module). Adding an alarm copy loop to `BuildDerivedTemplate`
without those fields would be actively harmful: copied alarm rows on the
derived template would shadow the live base alarm with stale data during
flattening (`ResolveInheritedAlarms` has no `IsInherited` skip for alarms,
unlike attributes/scripts). Resolving this safely is a cross-module change
(`Commons` + `TemplateEngine`) and must be scheduled as a coordinated edit;
left **Open** pending that.
### TemplateEngine-003 — `UpdateAttributeAsync` lets a non-locked attribute change its fixed DataType / DataSourceReference ### TemplateEngine-003 — `UpdateAttributeAsync` lets a non-locked attribute change its fixed DataType / DataSourceReference
@@ -118,7 +139,7 @@ _Unresolved._
|--|--| |--|--|
| Severity | High | | Severity | High |
| Category | Correctness & logic bugs | | Category | Correctness & logic bugs |
| Status | Open | | Status | Resolved |
| Location | `src/ScadaLink.TemplateEngine/TemplateService.cs:285` | | Location | `src/ScadaLink.TemplateEngine/TemplateService.cs:285` |
**Description** **Description**
@@ -147,7 +168,12 @@ apply block.
**Resolution** **Resolution**
_Unresolved._ Resolved 2026-05-16 (commit `<pending>`): removed the `&& existing.IsLocked`
guard in `UpdateAttributeAsync` so the fixed-field granularity error is always
honoured, and removed the unconditional `existing.DataType` /
`existing.DataSourceReference` assignments from the apply block. Regression
tests: `UpdateAttribute_UnlockedAttribute_DataTypeChangeRejected`,
`UpdateAttribute_UnlockedAttribute_DataSourceReferenceChangeRejected`.
### TemplateEngine-004 — Alarm on-trigger script references are never resolved (empty placeholder) ### TemplateEngine-004 — Alarm on-trigger script references are never resolved (empty placeholder)
@@ -155,7 +181,7 @@ _Unresolved._
|--|--| |--|--|
| Severity | High | | Severity | High |
| Category | Correctness & logic bugs | | Category | Correctness & logic bugs |
| Status | Open | | Status | Resolved |
| Location | `src/ScadaLink.TemplateEngine/Flattening/FlatteningService.cs:695` | | Location | `src/ScadaLink.TemplateEngine/Flattening/FlatteningService.cs:695` |
**Description** **Description**
@@ -179,7 +205,15 @@ and implement that consistently.
**Resolution** **Resolution**
_Unresolved._ Resolved 2026-05-16 (commit `<pending>`): implemented `ResolveAlarmScriptReferences`.
Alarm resolution now records each resolved alarm's `OnTriggerScriptId` keyed by
canonical name, and script resolution records each resolved `TemplateScript.Id`
keyed by its canonical name (both honour composition path prefixes). Step 7
joins the two maps to set `ResolvedAlarm.OnTriggerScriptCanonicalName`, so the
revision hash, diff, and `SemanticValidator` on-trigger-script-exists check now
all see the reference. Regression tests:
`Flatten_AlarmOnTriggerScript_ResolvedToCanonicalName`,
`Flatten_ComposedAlarmOnTriggerScript_ResolvedWithPrefix`.
### TemplateEngine-005 — Collision validation is skipped when creating a child template ### TemplateEngine-005 — Collision validation is skipped when creating a child template
@@ -187,7 +221,7 @@ _Unresolved._
|--|--| |--|--|
| Severity | High | | Severity | High |
| Category | Correctness & logic bugs | | Category | Correctness & logic bugs |
| Status | Open | | Status | Resolved |
| Location | `src/ScadaLink.TemplateEngine/TemplateService.cs:56` | | Location | `src/ScadaLink.TemplateEngine/TemplateService.cs:56` |
**Description** **Description**
@@ -210,7 +244,15 @@ that explicitly instead of leaving a no-op.
**Resolution** **Resolution**
_Unresolved._ Resolved 2026-05-16 (commit `<pending>`): deleted the dead `if
(parentTemplateId.HasValue)` block and its unused `GetAllTemplatesAsync`
read in `CreateTemplateAsync`. A create-time collision check on a child is a
guaranteed no-op — a freshly created template has no members of its own, the
parent's members were already collision-validated on every member-mutating
call, and a new child cannot be an ancestor of its parent. Replaced the no-op
with an explanatory comment documenting that collision detection is enforced
on `AddAttribute`/`AddAlarm`/`AddScript`/`AddComposition` and on rename.
Regression test: `CreateTemplate_WithParent_DoesNotRunDeadCollisionQuery`.
### TemplateEngine-006 — Forbidden-API enforcement is a naive substring scan (bypassable and false-positive prone) ### TemplateEngine-006 — Forbidden-API enforcement is a naive substring scan (bypassable and false-positive prone)
@@ -39,6 +39,12 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers
private readonly ISiteHealthCollector? _healthCollector; private readonly ISiteHealthCollector? _healthCollector;
private readonly IServiceProvider? _serviceProvider; private readonly IServiceProvider? _serviceProvider;
private readonly Dictionary<string, IActorRef> _instanceActors = new(); private readonly Dictionary<string, IActorRef> _instanceActors = new();
/// <summary>
/// Tracks Instance Actors that are terminating as part of a redeployment, keyed by
/// the terminating actor ref. The buffered command is applied once <see cref="Terminated"/>
/// confirms the child has fully stopped (SiteRuntime-003).
/// </summary>
private readonly Dictionary<IActorRef, PendingRedeploy> _pendingRedeploys = new();
private int _totalDeployedCount; private int _totalDeployedCount;
public ITimerScheduler Timers { get; set; } = null!; public ITimerScheduler Timers { get; set; } = null!;
@@ -94,6 +100,10 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers
// Internal deploy persistence result // Internal deploy persistence result
Receive<DeployPersistenceResult>(HandleDeployPersistenceResult); Receive<DeployPersistenceResult>(HandleDeployPersistenceResult);
// Terminated signal — drains a buffered redeployment once the previous
// Instance Actor has fully stopped (SiteRuntime-003).
Receive<Terminated>(HandleTerminated);
} }
protected override void PreStart() protected override void PreStart()
@@ -211,6 +221,13 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers
/// <summary> /// <summary>
/// Handles a new deployment: stores config in SQLite, clears previous static overrides, /// Handles a new deployment: stores config in SQLite, clears previous static overrides,
/// and creates or replaces the Instance Actor. /// and creates or replaces the Instance Actor.
///
/// Redeployment of an already-running instance must wait for the previous Instance
/// Actor to fully terminate (including PostStop on its descendants) before the
/// replacement is created — otherwise <see cref="Context.ActorOf"/> can collide on
/// the still-registered child name. Instead of guessing with a fixed timer, the
/// terminating child is watched and the in-flight command is buffered until the
/// <see cref="Terminated"/> signal arrives.
/// </summary> /// </summary>
private void HandleDeploy(DeployInstanceCommand command) private void HandleDeploy(DeployInstanceCommand command)
{ {
@@ -219,28 +236,54 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers
"Deploying instance {Instance}, deploymentId={DeploymentId}", "Deploying instance {Instance}, deploymentId={DeploymentId}",
instanceName, command.DeploymentId); instanceName, command.DeploymentId);
// Stop existing actor if present (redeployment replaces) // Redeployment replaces a running instance. Watch + stop the existing actor
// and buffer this command until its Terminated signal confirms the child
// (and its whole subtree) has fully stopped and freed its actor name.
if (_instanceActors.TryGetValue(instanceName, out var existing)) if (_instanceActors.TryGetValue(instanceName, out var existing))
{ {
Context.Stop(existing);
_instanceActors.Remove(instanceName); _instanceActors.Remove(instanceName);
// Wait for the child to be removed from the children collection _pendingRedeploys[existing] = new PendingRedeploy(command, Sender);
// by yielding and retrying — Context.Stop is processed before the next message Context.Watch(existing);
Context.System.Scheduler.ScheduleTellOnce( Context.Stop(existing);
TimeSpan.FromMilliseconds(500), Self, command, Sender); UpdateInstanceCounts();
return; return;
} }
// Fresh deployment — no existing actor to replace.
ApplyDeployment(command, Sender, isRedeploy: false);
}
/// <summary>
/// Recreates an Instance Actor once its predecessor has fully terminated during a
/// redeployment, draining the buffered <see cref="DeployInstanceCommand"/>.
/// </summary>
private void HandleTerminated(Terminated terminated)
{
if (!_pendingRedeploys.Remove(terminated.ActorRef, out var pending))
return;
ApplyDeployment(pending.Command, pending.OriginalSender, isRedeploy: true);
}
/// <summary>
/// Creates the Instance Actor, persists the config, and replies to the deployer.
/// A redeployment is an update of an existing instance, so the deployed-instance
/// counter is only incremented for genuinely new deployments.
/// </summary>
private void ApplyDeployment(DeployInstanceCommand command, IActorRef sender, bool isRedeploy)
{
var instanceName = command.InstanceUniqueName;
// Ensure DCL connections exist for any data-sourced attributes // Ensure DCL connections exist for any data-sourced attributes
EnsureDclConnections(command.FlattenedConfigurationJson); EnsureDclConnections(command.FlattenedConfigurationJson);
// Create the Instance Actor immediately (no existing actor to replace) // Create the Instance Actor immediately
CreateInstanceActor(instanceName, command.FlattenedConfigurationJson); CreateInstanceActor(instanceName, command.FlattenedConfigurationJson);
_totalDeployedCount++; if (!isRedeploy)
_totalDeployedCount++;
UpdateInstanceCounts(); UpdateInstanceCounts();
// Persist to SQLite and clear static overrides asynchronously // Persist to SQLite and clear static overrides asynchronously
var sender = Sender;
Task.Run(async () => Task.Run(async () =>
{ {
await _storage.StoreDeployedConfigAsync( await _storage.StoreDeployedConfigAsync(
@@ -614,9 +657,11 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers
/// <summary> /// <summary>
/// Writes attribute values on a deployed instance for a Route.To().SetAttribute(s) /// Writes attribute values on a deployed instance for a Route.To().SetAttribute(s)
/// call (or a central Test Run bound to the instance). Writes are Tell'd to the /// call (or a central Test Run bound to the instance). Each write is Ask'd to the
/// Instance Actor — serialized through its mailbox — and acknowledged optimistically, /// Instance Actor, which routes data-sourced attributes through the DCL and static
/// matching the fire-and-forget semantics of Instance.SetAttribute. /// attributes to a persisted override. The response reflects the real per-attribute
/// outcome (a non-existent attribute or a failed device write reports failure),
/// rather than an unconditional optimistic ack.
/// </summary> /// </summary>
private void RouteInboundApiSetAttributes(RouteToSetAttributesRequest request) private void RouteInboundApiSetAttributes(RouteToSetAttributesRequest request)
{ {
@@ -629,14 +674,33 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers
return; return;
} }
foreach (var (name, value) in request.AttributeValues) var sender = Sender;
{ var correlationId = request.CorrelationId;
instanceActor.Tell(new SetStaticAttributeCommand( var asks = request.AttributeValues
request.CorrelationId, request.InstanceUniqueName, name, value, DateTimeOffset.UtcNow)); .Select(kvp => instanceActor.Ask<SetStaticAttributeResponse>(
} new SetStaticAttributeCommand(
correlationId, request.InstanceUniqueName, kvp.Key, kvp.Value, DateTimeOffset.UtcNow),
TimeSpan.FromSeconds(30)))
.ToArray();
Sender.Tell(new RouteToSetAttributesResponse( Task.WhenAll(asks).ContinueWith(t =>
request.CorrelationId, true, null, DateTimeOffset.UtcNow)); {
if (!t.IsCompletedSuccessfully)
return new RouteToSetAttributesResponse(
correlationId, false,
t.Exception?.GetBaseException().Message ?? "Attribute write timed out",
DateTimeOffset.UtcNow);
var failures = t.Result
.Where(r => !r.Success)
.Select(r => $"{r.AttributeName}: {r.ErrorMessage}")
.ToArray();
return failures.Length == 0
? new RouteToSetAttributesResponse(correlationId, true, null, DateTimeOffset.UtcNow)
: new RouteToSetAttributesResponse(
correlationId, false, string.Join("; ", failures), DateTimeOffset.UtcNow);
}).PipeTo(sender);
} }
/// <summary> /// <summary>
@@ -789,4 +853,9 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers
EnableInstanceCommand Command, DeployedInstance? Config, string? Error, IActorRef OriginalSender); EnableInstanceCommand Command, DeployedInstance? Config, string? Error, IActorRef OriginalSender);
internal record DeployPersistenceResult( internal record DeployPersistenceResult(
string DeploymentId, string InstanceName, bool Success, string? Error, IActorRef OriginalSender); string DeploymentId, string InstanceName, bool Success, string? Error, IActorRef OriginalSender);
/// <summary>
/// A redeployment command buffered until the previous Instance Actor terminates.
/// </summary>
internal record PendingRedeploy(DeployInstanceCommand Command, IActorRef OriginalSender);
} }
@@ -198,10 +198,44 @@ public class InstanceActor : ReceiveActor
} }
/// <summary> /// <summary>
/// Updates a static attribute in memory and persists the override to SQLite. /// Handles an attribute write (<c>Instance.SetAttribute</c> / Inbound API).
/// WP-24: State mutation serialized through this actor's mailbox. /// WP-24: State mutation serialized through this actor's mailbox.
///
/// The write is routed by the attribute's data binding:
/// * Data-sourced attribute → forwards a <see cref="WriteTagRequest"/> to the
/// DCL, which writes the physical device. The in-memory value is NOT
/// optimistically updated and NO static override is persisted — the
/// confirmed device value arrives later via the subscription. Success or
/// failure of the device write is returned to the caller.
/// * Static attribute → updates the in-memory value and persists the override
/// to SQLite.
///
/// Either way the caller receives a <see cref="SetStaticAttributeResponse"/>.
/// </summary> /// </summary>
private void HandleSetStaticAttribute(SetStaticAttributeCommand command) private void HandleSetStaticAttribute(SetStaticAttributeCommand command)
{
// Resolve the target attribute's data binding from the flattened config.
var resolved = _configuration?.Attributes
.FirstOrDefault(a => a.CanonicalName == command.AttributeName);
var isDataSourced = resolved != null
&& !string.IsNullOrEmpty(resolved.DataSourceReference)
&& !string.IsNullOrEmpty(resolved.BoundDataConnectionName);
if (isDataSourced)
{
HandleSetDataAttribute(command, resolved!);
return;
}
HandleSetStaticAttributeCore(command);
}
/// <summary>
/// Static attribute write: updates in-memory state, publishes the change,
/// persists the override to SQLite, and replies with success.
/// </summary>
private void HandleSetStaticAttributeCore(SetStaticAttributeCommand command)
{ {
_attributes[command.AttributeName] = command.Value; _attributes[command.AttributeName] = command.Value;
@@ -216,8 +250,7 @@ public class InstanceActor : ReceiveActor
PublishAndNotifyChildren(changed); PublishAndNotifyChildren(changed);
// Persist asynchronously -- fire and forget since the actor is the source of truth // Persist asynchronously -- fire and forget since the actor is the source of truth.
// and SetAttribute is called from scripts via Tell (no response consumer).
var instanceName = _instanceUniqueName; var instanceName = _instanceUniqueName;
var attributeName = command.AttributeName; var attributeName = command.AttributeName;
var logger = _logger; var logger = _logger;
@@ -230,6 +263,58 @@ public class InstanceActor : ReceiveActor
instanceName, instanceName,
attributeName); attributeName);
}, TaskContinuationOptions.OnlyOnFaulted); }, TaskContinuationOptions.OnlyOnFaulted);
Sender.Tell(new SetStaticAttributeResponse(
command.CorrelationId, _instanceUniqueName, command.AttributeName,
true, null, DateTimeOffset.UtcNow));
}
/// <summary>
/// Data-sourced attribute write: forwards a write request to the DCL and pipes
/// the device write result back to the caller. The in-memory value is left
/// untouched (it is refreshed by the subscription when the device confirms);
/// no static override is persisted for a data-sourced attribute.
/// </summary>
private void HandleSetDataAttribute(SetStaticAttributeCommand command, ResolvedAttribute resolved)
{
var caller = Sender;
var correlationId = command.CorrelationId;
var attributeName = command.AttributeName;
var instanceName = _instanceUniqueName;
if (_dclManager == null)
{
_logger.LogWarning(
"SetAttribute on data-sourced attribute {Instance}.{Attribute} cannot be routed — no DCL manager configured",
instanceName, attributeName);
caller.Tell(new SetStaticAttributeResponse(
correlationId, instanceName, attributeName, false,
"Data Connection Layer not available for write.", DateTimeOffset.UtcNow));
return;
}
var writeRequest = new WriteTagRequest(
correlationId,
resolved.BoundDataConnectionName!,
resolved.DataSourceReference!,
command.Value,
DateTimeOffset.UtcNow);
// Ask the DCL and pipe the result back to the original caller. The DCL
// returns the failure synchronously so the script can handle it.
_dclManager.Ask<WriteTagResponse>(writeRequest, TimeSpan.FromSeconds(30))
.ContinueWith(t =>
{
if (t.IsCompletedSuccessfully)
return new SetStaticAttributeResponse(
correlationId, instanceName, attributeName,
t.Result.Success, t.Result.ErrorMessage, DateTimeOffset.UtcNow);
return new SetStaticAttributeResponse(
correlationId, instanceName, attributeName, false,
t.Exception?.GetBaseException().Message ?? "DCL write timed out",
DateTimeOffset.UtcNow);
}).PipeTo(caller);
} }
/// <summary> /// <summary>
@@ -25,17 +25,17 @@ public class AttributeAccessor
public object? this[string key] public object? this[string key]
{ {
// Both reads and writes block on the actor Ask; the write also blocks
// on the DCL round-trip for data-connected attributes. The async
// variants (GetAsync/SetAsync) are preferred where awaiting is possible.
get => _ctx.GetAttribute(Resolve(key)).GetAwaiter().GetResult(); get => _ctx.GetAttribute(Resolve(key)).GetAwaiter().GetResult();
set => _ctx.SetAttribute(Resolve(key), value?.ToString() ?? string.Empty); set => _ctx.SetAttribute(Resolve(key), value?.ToString() ?? string.Empty).GetAwaiter().GetResult();
} }
public Task<object?> GetAsync(string key) => _ctx.GetAttribute(Resolve(key)); public Task<object?> GetAsync(string key) => _ctx.GetAttribute(Resolve(key));
public Task SetAsync(string key, object? value) public Task SetAsync(string key, object? value)
{ => _ctx.SetAttribute(Resolve(key), value?.ToString() ?? string.Empty);
_ctx.SetAttribute(Resolve(key), value?.ToString() ?? string.Empty);
return Task.CompletedTask;
}
} }
/// <summary> /// <summary>
@@ -99,18 +99,31 @@ public class ScriptRuntimeContext
} }
/// <summary> /// <summary>
/// Sets an attribute value. For data-connected attributes, forwards to DCL via Instance Actor. /// Sets an attribute value. For data-connected attributes the Instance Actor
/// For static attributes, updates in-memory and persists to SQLite via Instance Actor. /// forwards the write to the DCL, which writes the physical device; the
/// All mutations serialized through the Instance Actor mailbox. /// in-memory value is not optimistically updated. For static attributes the
/// Instance Actor updates the in-memory value and persists the override to
/// SQLite. All mutations are serialized through the Instance Actor mailbox.
///
/// The write is awaited so that a device-write failure on a data-connected
/// attribute is surfaced synchronously to the calling script as an
/// <see cref="InvalidOperationException"/>.
/// </summary> /// </summary>
public void SetAttribute(string attributeName, string value) public async Task SetAttribute(string attributeName, string value)
{ {
var correlationId = Guid.NewGuid().ToString(); var correlationId = Guid.NewGuid().ToString();
var command = new SetStaticAttributeCommand( var command = new SetStaticAttributeCommand(
correlationId, _instanceName, attributeName, value, DateTimeOffset.UtcNow); correlationId, _instanceName, attributeName, value, DateTimeOffset.UtcNow);
// Tell (fire-and-forget) — mutation serialized through Instance Actor // Ask — mutation serialized through the Instance Actor mailbox; the reply
_instanceActor.Tell(command); // carries the device-write outcome for data-connected attributes.
var response = await _instanceActor.Ask<SetStaticAttributeResponse>(command, _askTimeout);
if (!response.Success)
{
throw new InvalidOperationException(
$"SetAttribute('{attributeName}') failed: {response.ErrorMessage}");
}
} }
/// <summary> /// <summary>
@@ -20,10 +20,14 @@ public class StoreAndForwardMessage
/// <summary>JSON-serialized payload containing the call details.</summary> /// <summary>JSON-serialized payload containing the call details.</summary>
public string PayloadJson { get; set; } = string.Empty; public string PayloadJson { get; set; } = string.Empty;
/// <summary>Number of delivery attempts so far.</summary> /// <summary>
/// Number of retry-sweep attempts performed so far. The initial (immediate or
/// caller-made) delivery attempt is attempt 0 and is not counted here; this
/// field counts only background retry attempts (StoreAndForward-003).
/// </summary>
public int RetryCount { get; set; } public int RetryCount { get; set; }
/// <summary>Maximum retry attempts before parking (0 = no limit).</summary> /// <summary>Maximum retry-sweep attempts before parking (0 = no limit).</summary>
public int MaxRetries { get; set; } public int MaxRetries { get; set; }
/// <summary>Retry interval in milliseconds.</summary> /// <summary>Retry interval in milliseconds.</summary>
@@ -148,13 +148,14 @@ public class StoreAndForwardService
} }
catch (Exception ex) catch (Exception ex)
{ {
// Transient failure — buffer for retry // Transient failure — buffer for retry. The immediate attempt is
// attempt 0; RetryCount tracks only sweep retries, so it stays 0
// here (StoreAndForward-003).
_logger.LogWarning(ex, _logger.LogWarning(ex,
"Immediate delivery to {Target} failed (transient), buffering for retry", "Immediate delivery to {Target} failed (transient), buffering for retry",
target); target);
message.LastAttemptAt = DateTimeOffset.UtcNow; message.LastAttemptAt = DateTimeOffset.UtcNow;
message.RetryCount = 1;
message.LastError = ex.Message; message.LastError = ex.Message;
await BufferAsync(message); await BufferAsync(message);
@@ -165,11 +166,11 @@ public class StoreAndForwardService
// Either no handler is registered yet, or the caller already attempted // Either no handler is registered yet, or the caller already attempted
// delivery itself — buffer for the background retry sweep to deliver. // delivery itself — buffer for the background retry sweep to deliver.
// The initial attempt (caller-made, or skipped because no handler is
// registered) is attempt 0; RetryCount tracks only sweep retries and
// therefore stays 0 here (StoreAndForward-003).
if (!attemptImmediateDelivery) if (!attemptImmediateDelivery)
{ {
// The caller made (and failed) one attempt before handing the
// message over, so it counts as the first retry.
message.RetryCount = 1;
message.LastAttemptAt = DateTimeOffset.UtcNow; message.LastAttemptAt = DateTimeOffset.UtcNow;
} }
await BufferAsync(message); await BufferAsync(message);
@@ -78,17 +78,23 @@ public class FlatteningService
// Step 4: Apply connection bindings // Step 4: Apply connection bindings
ApplyConnectionBindings(instance.ConnectionBindings, attributes, dataConnections); ApplyConnectionBindings(instance.ConnectionBindings, attributes, dataConnections);
// Step 5: Resolve alarms from inheritance chain // Step 5: Resolve alarms from inheritance chain.
var alarms = ResolveInheritedAlarms(templateChain); // alarmScriptIds maps a resolved alarm's canonical name to the
ResolveComposedAlarms(templateChain, compositionMap, composedTemplateChains, alarms); // TemplateScript.Id of its on-trigger script (if any).
var alarmScriptIds = new Dictionary<string, int>(StringComparer.Ordinal);
var alarms = ResolveInheritedAlarms(templateChain, prefix: null, alarmScriptIds);
ResolveComposedAlarms(templateChain, compositionMap, composedTemplateChains, alarms, alarmScriptIds);
ApplyInstanceAlarmOverrides(instance.AlarmOverrides, alarms); ApplyInstanceAlarmOverrides(instance.AlarmOverrides, alarms);
// Step 6: Resolve scripts from inheritance chain // Step 6: Resolve scripts from inheritance chain.
var scripts = ResolveInheritedScripts(templateChain); // scriptCanonicalById maps a TemplateScript.Id to its resolved
ResolveComposedScripts(templateChain, compositionMap, composedTemplateChains, scripts); // canonical name, used to wire up alarm on-trigger script refs.
var scriptCanonicalById = new Dictionary<int, string>();
var scripts = ResolveInheritedScripts(templateChain, prefix: null, scriptCanonicalById);
ResolveComposedScripts(templateChain, compositionMap, composedTemplateChains, scripts, scriptCanonicalById);
// Step 7: Resolve alarm on-trigger script references to canonical names // Step 7: Resolve alarm on-trigger script references to canonical names
ResolveAlarmScriptReferences(alarms, scripts); ResolveAlarmScriptReferences(alarms, alarmScriptIds, scriptCanonicalById);
// Step 8: Collect connection configurations for deployment packaging // Step 8: Collect connection configurations for deployment packaging
var connections = new Dictionary<string, ConnectionConfig>(); var connections = new Dictionary<string, ConnectionConfig>();
@@ -221,57 +227,56 @@ public class FlatteningService
continue; continue;
foreach (var composition in compositions) foreach (var composition in compositions)
ResolveComposedAttributesRecursive(
composition, composition.InstanceName,
compositionMap, composedTemplateChains, attributes, new HashSet<int>());
}
}
/// <summary>
/// Recursively resolves the attributes of a composed module and every
/// module nested inside it (to arbitrary depth), path-qualifying each
/// canonical name with the accumulated <paramref name="prefix"/>.
/// </summary>
private static void ResolveComposedAttributesRecursive(
TemplateComposition composition,
string prefix,
IReadOnlyDictionary<int, IReadOnlyList<TemplateComposition>> compositionMap,
IReadOnlyDictionary<int, IReadOnlyList<Template>> composedTemplateChains,
Dictionary<string, ResolvedAttribute> attributes,
HashSet<int> visited)
{
if (!composedTemplateChains.TryGetValue(composition.ComposedTemplateId, out var composedChain))
return;
var composedAttrs = ResolveInheritedAttributes(composedChain);
foreach (var (name, attr) in composedAttrs)
{
var canonicalName = $"{prefix}.{name}";
// Don't overwrite if already defined (most-derived wins)
if (!attributes.ContainsKey(canonicalName))
{ {
if (!composedTemplateChains.TryGetValue(composition.ComposedTemplateId, out var composedChain)) attributes[canonicalName] = attr with
continue;
var prefix = composition.InstanceName;
var composedAttrs = ResolveInheritedAttributes(composedChain);
foreach (var (name, attr) in composedAttrs)
{ {
var canonicalName = $"{prefix}.{name}"; CanonicalName = canonicalName,
// Don't overwrite if already defined (most-derived wins) Source = "Composed"
if (!attributes.ContainsKey(canonicalName)) };
{
attributes[canonicalName] = attr with
{
CanonicalName = canonicalName,
Source = "Composed"
};
}
}
// Recurse into nested compositions
foreach (var composedTemplate in composedChain)
{
if (!compositionMap.TryGetValue(composedTemplate.Id, out var nestedCompositions))
continue;
foreach (var nested in nestedCompositions)
{
if (!composedTemplateChains.TryGetValue(nested.ComposedTemplateId, out var nestedChain))
continue;
var nestedPrefix = $"{prefix}.{nested.InstanceName}";
var nestedAttrs = ResolveInheritedAttributes(nestedChain);
foreach (var (name, attr) in nestedAttrs)
{
var canonicalName = $"{nestedPrefix}.{name}";
if (!attributes.ContainsKey(canonicalName))
{
attributes[canonicalName] = attr with
{
CanonicalName = canonicalName,
Source = "Composed"
};
}
}
}
}
} }
} }
// Descend into nested compositions of every template in the chain.
foreach (var composedTemplate in composedChain)
{
if (!visited.Add(composedTemplate.Id))
continue;
if (!compositionMap.TryGetValue(composedTemplate.Id, out var nestedCompositions))
continue;
foreach (var nested in nestedCompositions)
ResolveComposedAttributesRecursive(
nested, $"{prefix}.{nested.InstanceName}",
compositionMap, composedTemplateChains, attributes, visited);
}
} }
private static void ApplyInstanceOverrides( private static void ApplyInstanceOverrides(
@@ -356,10 +361,22 @@ public class FlatteningService
} }
} }
/// <summary>
/// Resolves alarms from an inheritance chain. When <paramref name="prefix"/>
/// is non-null the alarm names are returned bare (caller path-qualifies);
/// the keys of the returned dictionary are always bare alarm names.
/// <paramref name="alarmScriptIds"/> is populated with the on-trigger
/// script id of each resolved alarm keyed by the canonical name the alarm
/// will ultimately carry (bare name when <paramref name="prefix"/> is null,
/// otherwise <c>prefix.name</c>).
/// </summary>
private static Dictionary<string, ResolvedAlarm> ResolveInheritedAlarms( private static Dictionary<string, ResolvedAlarm> ResolveInheritedAlarms(
IReadOnlyList<Template> templateChain) IReadOnlyList<Template> templateChain,
string? prefix,
Dictionary<string, int> alarmScriptIds)
{ {
var result = new Dictionary<string, ResolvedAlarm>(StringComparer.Ordinal); var result = new Dictionary<string, ResolvedAlarm>(StringComparer.Ordinal);
var scriptIdByName = new Dictionary<string, int>(StringComparer.Ordinal);
for (int i = templateChain.Count - 1; i >= 0; i--) for (int i = templateChain.Count - 1; i >= 0; i--)
{ {
@@ -394,9 +411,20 @@ public class FlatteningService
OnTriggerScriptCanonicalName = null, // Resolved later OnTriggerScriptCanonicalName = null, // Resolved later
Source = source Source = source
}; };
if (alarm.OnTriggerScriptId.HasValue)
scriptIdByName[alarm.Name] = alarm.OnTriggerScriptId.Value;
else
scriptIdByName.Remove(alarm.Name);
} }
} }
foreach (var (name, scriptId) in scriptIdByName)
{
var canonical = prefix == null ? name : $"{prefix}.{name}";
alarmScriptIds[canonical] = scriptId;
}
return result; return result;
} }
@@ -536,7 +564,8 @@ public class FlatteningService
IReadOnlyList<Template> templateChain, IReadOnlyList<Template> templateChain,
IReadOnlyDictionary<int, IReadOnlyList<TemplateComposition>> compositionMap, IReadOnlyDictionary<int, IReadOnlyList<TemplateComposition>> compositionMap,
IReadOnlyDictionary<int, IReadOnlyList<Template>> composedTemplateChains, IReadOnlyDictionary<int, IReadOnlyList<Template>> composedTemplateChains,
Dictionary<string, ResolvedAlarm> alarms) Dictionary<string, ResolvedAlarm> alarms,
Dictionary<string, int> alarmScriptIds)
{ {
foreach (var template in templateChain) foreach (var template in templateChain)
{ {
@@ -544,34 +573,74 @@ public class FlatteningService
continue; continue;
foreach (var composition in compositions) foreach (var composition in compositions)
{ ResolveComposedAlarmsRecursive(
if (!composedTemplateChains.TryGetValue(composition.ComposedTemplateId, out var composedChain)) composition, composition.InstanceName,
continue; compositionMap, composedTemplateChains, alarms, alarmScriptIds,
new HashSet<int>());
var prefix = composition.InstanceName;
var composedAlarms = ResolveInheritedAlarms(composedChain);
foreach (var (name, alarm) in composedAlarms)
{
var canonicalName = $"{prefix}.{name}";
if (!alarms.ContainsKey(canonicalName))
{
alarms[canonicalName] = alarm with
{
CanonicalName = canonicalName,
TriggerConfiguration = PrefixTriggerAttribute(alarm.TriggerConfiguration, prefix),
Source = "Composed"
};
}
}
}
} }
} }
/// <summary>
/// Recursively resolves the alarms of a composed module and every module
/// nested inside it, path-qualifying each canonical name with the
/// accumulated <paramref name="prefix"/>.
/// </summary>
private static void ResolveComposedAlarmsRecursive(
TemplateComposition composition,
string prefix,
IReadOnlyDictionary<int, IReadOnlyList<TemplateComposition>> compositionMap,
IReadOnlyDictionary<int, IReadOnlyList<Template>> composedTemplateChains,
Dictionary<string, ResolvedAlarm> alarms,
Dictionary<string, int> alarmScriptIds,
HashSet<int> visited)
{
if (!composedTemplateChains.TryGetValue(composition.ComposedTemplateId, out var composedChain))
return;
var composedAlarms = ResolveInheritedAlarms(composedChain, prefix, alarmScriptIds);
foreach (var (name, alarm) in composedAlarms)
{
var canonicalName = $"{prefix}.{name}";
if (!alarms.ContainsKey(canonicalName))
{
alarms[canonicalName] = alarm with
{
CanonicalName = canonicalName,
TriggerConfiguration = PrefixTriggerAttribute(alarm.TriggerConfiguration, prefix),
Source = "Composed"
};
}
}
// Descend into nested compositions of every template in the chain.
foreach (var composedTemplate in composedChain)
{
if (!visited.Add(composedTemplate.Id))
continue;
if (!compositionMap.TryGetValue(composedTemplate.Id, out var nestedCompositions))
continue;
foreach (var nested in nestedCompositions)
ResolveComposedAlarmsRecursive(
nested, $"{prefix}.{nested.InstanceName}",
compositionMap, composedTemplateChains, alarms, alarmScriptIds, visited);
}
}
/// <summary>
/// Resolves scripts from an inheritance chain. The returned dictionary is
/// keyed by bare script name. <paramref name="scriptCanonicalById"/> is
/// populated with each resolved script's <see cref="TemplateScript.Id"/>
/// mapped to the canonical name it will ultimately carry (bare when
/// <paramref name="prefix"/> is null, otherwise <c>prefix.name</c>).
/// </summary>
private static Dictionary<string, ResolvedScript> ResolveInheritedScripts( private static Dictionary<string, ResolvedScript> ResolveInheritedScripts(
IReadOnlyList<Template> templateChain) IReadOnlyList<Template> templateChain,
string? prefix,
Dictionary<int, string> scriptCanonicalById)
{ {
var result = new Dictionary<string, ResolvedScript>(StringComparer.Ordinal); var result = new Dictionary<string, ResolvedScript>(StringComparer.Ordinal);
var idByName = new Dictionary<string, int>(StringComparer.Ordinal);
for (int i = templateChain.Count - 1; i >= 0; i--) for (int i = templateChain.Count - 1; i >= 0; i--)
{ {
@@ -600,9 +669,17 @@ public class FlatteningService
MinTimeBetweenRuns = script.MinTimeBetweenRuns, MinTimeBetweenRuns = script.MinTimeBetweenRuns,
Source = source Source = source
}; };
idByName[script.Name] = script.Id;
} }
} }
foreach (var (name, id) in idByName)
{
if (id == 0) continue; // unsaved row — no stable id to map
var canonical = prefix == null ? name : $"{prefix}.{name}";
scriptCanonicalById[id] = canonical;
}
return result; return result;
} }
@@ -610,7 +687,8 @@ public class FlatteningService
IReadOnlyList<Template> templateChain, IReadOnlyList<Template> templateChain,
IReadOnlyDictionary<int, IReadOnlyList<TemplateComposition>> compositionMap, IReadOnlyDictionary<int, IReadOnlyList<TemplateComposition>> compositionMap,
IReadOnlyDictionary<int, IReadOnlyList<Template>> composedTemplateChains, IReadOnlyDictionary<int, IReadOnlyList<Template>> composedTemplateChains,
Dictionary<string, ResolvedScript> scripts) Dictionary<string, ResolvedScript> scripts,
Dictionary<int, string> scriptCanonicalById)
{ {
foreach (var template in templateChain) foreach (var template in templateChain)
{ {
@@ -618,28 +696,58 @@ public class FlatteningService
continue; continue;
foreach (var composition in compositions) foreach (var composition in compositions)
ResolveComposedScriptsRecursive(
composition, composition.InstanceName,
compositionMap, composedTemplateChains, scripts, scriptCanonicalById,
new HashSet<int>());
}
}
/// <summary>
/// Recursively resolves the scripts of a composed module and every module
/// nested inside it, path-qualifying each canonical name with the
/// accumulated <paramref name="prefix"/>.
/// </summary>
private static void ResolveComposedScriptsRecursive(
TemplateComposition composition,
string prefix,
IReadOnlyDictionary<int, IReadOnlyList<TemplateComposition>> compositionMap,
IReadOnlyDictionary<int, IReadOnlyList<Template>> composedTemplateChains,
Dictionary<string, ResolvedScript> scripts,
Dictionary<int, string> scriptCanonicalById,
HashSet<int> visited)
{
if (!composedTemplateChains.TryGetValue(composition.ComposedTemplateId, out var composedChain))
return;
var composedScripts = ResolveInheritedScripts(composedChain, prefix, scriptCanonicalById);
foreach (var (name, script) in composedScripts)
{
var canonicalName = $"{prefix}.{name}";
if (!scripts.ContainsKey(canonicalName))
{ {
if (!composedTemplateChains.TryGetValue(composition.ComposedTemplateId, out var composedChain)) scripts[canonicalName] = script with
continue;
var prefix = composition.InstanceName;
var composedScripts = ResolveInheritedScripts(composedChain);
foreach (var (name, script) in composedScripts)
{ {
var canonicalName = $"{prefix}.{name}"; CanonicalName = canonicalName,
if (!scripts.ContainsKey(canonicalName)) Source = "Composed",
{ Scope = new Commons.Types.Scripts.ScriptScope(SelfPath: prefix, ParentPath: "")
scripts[canonicalName] = script with };
{
CanonicalName = canonicalName,
Source = "Composed",
Scope = new Commons.Types.Scripts.ScriptScope(SelfPath: prefix, ParentPath: "")
};
}
}
} }
} }
// Descend into nested compositions of every template in the chain.
foreach (var composedTemplate in composedChain)
{
if (!visited.Add(composedTemplate.Id))
continue;
if (!compositionMap.TryGetValue(composedTemplate.Id, out var nestedCompositions))
continue;
foreach (var nested in nestedCompositions)
ResolveComposedScriptsRecursive(
nested, $"{prefix}.{nested.InstanceName}",
compositionMap, composedTemplateChains, scripts, scriptCanonicalById, visited);
}
} }
/// <summary> /// <summary>
@@ -688,18 +796,30 @@ public class FlatteningService
} }
/// <summary> /// <summary>
/// Resolves alarm on-trigger script references from script IDs to canonical names. /// Resolves alarm on-trigger script references from <see cref="TemplateScript.Id"/>
/// This is done by finding the script in the template chain whose ID matches the alarm's OnTriggerScriptId, /// values to the canonical (path-qualified) names of the corresponding
/// then mapping to the corresponding canonical name in the resolved scripts. /// resolved scripts. <paramref name="alarmScriptIds"/> maps an alarm's
/// canonical name to the id of its on-trigger script; <paramref name="scriptCanonicalById"/>
/// maps a script id to the canonical name it carries in the flattened
/// configuration. An alarm whose on-trigger script id has no matching
/// resolved script is left with a <c>null</c> reference — semantic
/// validation then reports the dangling reference.
/// </summary> /// </summary>
private static void ResolveAlarmScriptReferences( private static void ResolveAlarmScriptReferences(
Dictionary<string, ResolvedAlarm> alarms, Dictionary<string, ResolvedAlarm> alarms,
Dictionary<string, ResolvedScript> scripts) Dictionary<string, int> alarmScriptIds,
Dictionary<int, string> scriptCanonicalById)
{ {
// Build a lookup of script names (we only have canonical names at this point) foreach (var (alarmCanonicalName, scriptId) in alarmScriptIds)
// The alarm's OnTriggerScriptCanonicalName will be set by the caller or validation step {
// For now, this is a placeholder — the actual resolution depends on how alarm trigger configs if (!alarms.TryGetValue(alarmCanonicalName, out var alarm))
// reference scripts (by name within the same scope). continue;
// The trigger configuration JSON may contain a "scriptName" field.
scriptCanonicalById.TryGetValue(scriptId, out var scriptCanonicalName);
alarms[alarmCanonicalName] = alarm with
{
OnTriggerScriptCanonicalName = scriptCanonicalName
};
}
} }
} }
+13 -14
View File
@@ -51,15 +51,12 @@ public class TemplateService
FolderId = folderId FolderId = folderId
}; };
// Check acyclicity (inheritance) — for new templates this is mostly a parent-exists check, // No collision or acyclicity check is needed here: a freshly created
// but we validate anyway for consistency // template has no members of its own, the parent (validated above to
if (parentTemplateId.HasValue) // exist) was already collision-checked when its members were added,
{ // and a brand-new child cannot be an ancestor of its parent. Naming
var allTemplates = await _repository.GetAllTemplatesAsync(cancellationToken); // collisions are enforced on every member-mutating call (AddAttribute,
// The new template doesn't exist yet, so we simulate by adding it to the list // AddAlarm, AddScript, AddComposition) and on rename in UpdateTemplate.
// with a temporary ID. Since it has no children yet, the only cycle would be
// if parentTemplateId somehow pointed at itself (already handled above).
}
await _repository.AddTemplateAsync(template, cancellationToken); await _repository.AddTemplateAsync(template, cancellationToken);
await _auditService.LogAsync(user, "Create", "Template", "0", name, template, cancellationToken); await _auditService.LogAsync(user, "Create", "Template", "0", name, template, cancellationToken);
@@ -281,17 +278,19 @@ public class TemplateService
if (lockError != null) if (lockError != null)
return Result<TemplateAttribute>.Failure(lockError); return Result<TemplateAttribute>.Failure(lockError);
// Validate fixed-field granularity // Validate fixed-field granularity. DataType and DataSourceReference are
// fixed by the defining level for every attribute — locked or not — so
// the error is always honoured (a locked attribute is already rejected
// earlier inside the helper).
var granularityError = LockEnforcer.ValidateAttributeOverride(existing, proposed); var granularityError = LockEnforcer.ValidateAttributeOverride(existing, proposed);
if (granularityError != null && existing.IsLocked) if (granularityError != null)
return Result<TemplateAttribute>.Failure(granularityError); return Result<TemplateAttribute>.Failure(granularityError);
// Apply overridable fields // Apply overridable fields. DataType / DataSourceReference are fixed and
// are deliberately not copied from the proposed attribute.
existing.Value = proposed.Value; existing.Value = proposed.Value;
existing.Description = proposed.Description; existing.Description = proposed.Description;
existing.IsLocked = proposed.IsLocked; existing.IsLocked = proposed.IsLocked;
existing.DataType = proposed.DataType;
existing.DataSourceReference = proposed.DataSourceReference;
if (template?.IsDerived == true) if (template?.IsDerived == true)
existing.IsInherited = proposed.IsInherited; existing.IsInherited = proposed.IsInherited;
else else
@@ -0,0 +1,155 @@
using Akka.Actor;
using Akka.TestKit.Xunit2;
using Microsoft.Extensions.Logging.Abstractions;
using ScadaLink.Commons.Messages.Deployment;
using ScadaLink.Commons.Messages.Health;
using ScadaLink.Commons.Messages.Lifecycle;
using ScadaLink.Commons.Types.Enums;
using ScadaLink.Commons.Types.Flattening;
using ScadaLink.HealthMonitoring;
using ScadaLink.SiteRuntime.Actors;
using ScadaLink.SiteRuntime.Persistence;
using ScadaLink.SiteRuntime.Scripts;
using System.Text.Json;
namespace ScadaLink.SiteRuntime.Tests.Actors;
/// <summary>
/// Regression tests for SiteRuntime-003: redeployment of an existing instance must
/// wait for the terminating Instance Actor before recreating the child, instead of
/// relying on a fixed 500 ms reschedule that can collide on the child actor name.
/// </summary>
public class DeploymentManagerRedeployTests : TestKit, IDisposable
{
private readonly SiteStorageService _storage;
private readonly ScriptCompilationService _compilationService;
private readonly SharedScriptLibrary _sharedScriptLibrary;
private readonly string _dbFile;
public DeploymentManagerRedeployTests()
{
_dbFile = Path.Combine(Path.GetTempPath(), $"dm-redeploy-test-{Guid.NewGuid():N}.db");
_storage = new SiteStorageService(
$"Data Source={_dbFile}",
NullLogger<SiteStorageService>.Instance);
_storage.InitializeAsync().GetAwaiter().GetResult();
_compilationService = new ScriptCompilationService(
NullLogger<ScriptCompilationService>.Instance);
_sharedScriptLibrary = new SharedScriptLibrary(
_compilationService, NullLogger<SharedScriptLibrary>.Instance);
}
void IDisposable.Dispose()
{
Shutdown();
try { File.Delete(_dbFile); } catch { /* cleanup */ }
}
private IActorRef CreateDeploymentManager(ISiteHealthCollector? healthCollector = null)
{
return ActorOf(Props.Create(() => new DeploymentManagerActor(
_storage,
_compilationService,
_sharedScriptLibrary,
null,
new SiteRuntimeOptions(),
NullLogger<DeploymentManagerActor>.Instance,
null,
null,
healthCollector,
null)));
}
/// <summary>
/// Minimal fake that records the most recent deployed-instance count.
/// </summary>
private sealed class CountCapturingHealthCollector : ISiteHealthCollector
{
public int LastDeployedCount { get; private set; }
public void IncrementScriptError() { }
public void IncrementAlarmError() { }
public void IncrementDeadLetter() { }
public void UpdateConnectionHealth(string connectionName, ConnectionHealth health) { }
public void RemoveConnection(string connectionName) { }
public void UpdateTagResolution(string connectionName, int totalSubscribed, int successfullyResolved) { }
public void UpdateConnectionEndpoint(string connectionName, string endpoint) { }
public void UpdateTagQuality(string connectionName, int good, int bad, int uncertain) { }
public void SetStoreAndForwardDepths(IReadOnlyDictionary<string, int> depths) { }
public void SetInstanceCounts(int deployed, int enabled, int disabled) => LastDeployedCount = deployed;
public void SetParkedMessageCount(int count) { }
public void SetNodeHostname(string hostname) { }
public void SetClusterNodes(IReadOnlyList<NodeStatus> nodes) { }
public void SetActiveNode(bool isActive) { }
public bool IsActiveNode => true;
public SiteHealthReport CollectReport(string siteId) => throw new NotSupportedException();
}
private static string MakeConfigJson(string instanceName)
{
var config = new FlattenedConfiguration
{
InstanceUniqueName = instanceName,
Attributes =
[
new ResolvedAttribute { CanonicalName = "TestAttr", Value = "1", DataType = "Int32" }
]
};
return JsonSerializer.Serialize(config);
}
[Fact]
public async Task Redeploy_ExistingInstance_SucceedsWithoutNameCollision()
{
var actor = CreateDeploymentManager();
await Task.Delay(500); // empty startup
// Initial deploy.
actor.Tell(new DeployInstanceCommand(
"dep-1", "RedeployPump", "h1", MakeConfigJson("RedeployPump"), "admin", DateTimeOffset.UtcNow));
var first = ExpectMsg<DeploymentStatusResponse>(TimeSpan.FromSeconds(5));
Assert.Equal(DeploymentStatus.Success, first.Status);
await Task.Delay(500);
// Redeploy the same instance — must replace the existing actor cleanly.
actor.Tell(new DeployInstanceCommand(
"dep-2", "RedeployPump", "h2", MakeConfigJson("RedeployPump"), "admin", DateTimeOffset.UtcNow));
var second = ExpectMsg<DeploymentStatusResponse>(TimeSpan.FromSeconds(10));
Assert.Equal(DeploymentStatus.Success, second.Status);
// The redeployed instance must still be operable (no orphaned/broken actor).
actor.Tell(new DisableInstanceCommand("cmd-1", "RedeployPump", DateTimeOffset.UtcNow));
var disable = ExpectMsg<InstanceLifecycleResponse>(TimeSpan.FromSeconds(5));
Assert.True(disable.Success);
}
[Fact]
public async Task Redeploy_ExistingInstance_DoesNotOverCountDeployedInstances()
{
var health = new CountCapturingHealthCollector();
var actor = CreateDeploymentManager(health);
await Task.Delay(500);
// Deploy once.
actor.Tell(new DeployInstanceCommand(
"dep-1", "CountPump", "h1", MakeConfigJson("CountPump"), "admin", DateTimeOffset.UtcNow));
ExpectMsg<DeploymentStatusResponse>(TimeSpan.FromSeconds(5));
await Task.Delay(500);
// Redeploy several times.
for (var i = 2; i <= 4; i++)
{
actor.Tell(new DeployInstanceCommand(
$"dep-{i}", "CountPump", $"h{i}", MakeConfigJson("CountPump"), "admin", DateTimeOffset.UtcNow));
ExpectMsg<DeploymentStatusResponse>(TimeSpan.FromSeconds(10));
await Task.Delay(500);
}
// Storage uses UPSERT — exactly one deployed config row should exist.
var configs = await _storage.GetAllDeployedConfigsAsync();
Assert.Single(configs, c => c.InstanceUniqueName == "CountPump");
// The reported deployed count must be exactly 1 — a redeploy is an update,
// not a new instance, so the in-memory counter must not drift upward.
Assert.Equal(1, health.LastDeployedCount);
}
}
@@ -123,9 +123,12 @@ public class InstanceActorIntegrationTests : TestKit, IDisposable
$"corr-{i}", "Pump1", "Temperature", $"{i}", DateTimeOffset.UtcNow)); $"corr-{i}", "Pump1", "Temperature", $"{i}", DateTimeOffset.UtcNow));
} }
// SetStaticAttributeCommand is fire-and-forget; the GetAttributeRequest // Each static write replies with a SetStaticAttributeResponse; drain all
// round-trip below is the sync point — the FIFO mailbox guarantees all // 50 — the FIFO mailbox guarantees they are processed in order.
// 50 sets are processed before the get is. for (int i = 0; i < 50; i++)
{
ExpectMsg<SetStaticAttributeResponse>(TimeSpan.FromSeconds(5));
}
// The last value should be the final one // The last value should be the final one
actor.Tell(new GetAttributeRequest( actor.Tell(new GetAttributeRequest(
@@ -0,0 +1,179 @@
using Akka.Actor;
using Akka.TestKit.Xunit2;
using Microsoft.Extensions.Logging.Abstractions;
using Akka.TestKit;
using ScadaLink.Commons.Messages.DataConnection;
using ScadaLink.Commons.Messages.Instance;
using ScadaLink.Commons.Types.Flattening;
using ScadaLink.SiteRuntime.Actors;
using ScadaLink.SiteRuntime.Persistence;
using ScadaLink.SiteRuntime.Scripts;
using System.Text.Json;
namespace ScadaLink.SiteRuntime.Tests.Actors;
/// <summary>
/// Regression tests for SiteRuntime-001: Instance.SetAttribute must route writes
/// to the Data Connection Layer for data-sourced attributes instead of persisting
/// a local static override.
/// </summary>
public class InstanceActorSetAttributeTests : TestKit, IDisposable
{
private readonly SiteStorageService _storage;
private readonly ScriptCompilationService _compilationService;
private readonly SharedScriptLibrary _sharedScriptLibrary;
private readonly SiteRuntimeOptions _options;
private readonly string _dbFile;
public InstanceActorSetAttributeTests()
{
_dbFile = Path.Combine(Path.GetTempPath(), $"instance-setattr-test-{Guid.NewGuid():N}.db");
_storage = new SiteStorageService(
$"Data Source={_dbFile}",
NullLogger<SiteStorageService>.Instance);
_storage.InitializeAsync().GetAwaiter().GetResult();
_compilationService = new ScriptCompilationService(
NullLogger<ScriptCompilationService>.Instance);
_sharedScriptLibrary = new SharedScriptLibrary(
_compilationService, NullLogger<SharedScriptLibrary>.Instance);
_options = new SiteRuntimeOptions();
}
void IDisposable.Dispose()
{
Shutdown();
try { File.Delete(_dbFile); } catch { /* cleanup */ }
}
private IActorRef CreateInstanceActor(string instanceName, FlattenedConfiguration config, IActorRef? dclManager)
{
return ActorOf(Props.Create(() => new InstanceActor(
instanceName,
JsonSerializer.Serialize(config),
_storage,
_compilationService,
_sharedScriptLibrary,
null,
_options,
NullLogger<InstanceActor>.Instance,
dclManager)));
}
/// <summary>
/// Drains the startup <see cref="SubscribeTagsRequest"/> the Instance Actor emits
/// to the DCL in PreStart, then returns the next <see cref="WriteTagRequest"/>.
/// </summary>
private static WriteTagRequest ExpectWriteTag(TestProbe dclProbe)
=> dclProbe.FishForMessage<WriteTagRequest>(_ => true, TimeSpan.FromSeconds(5));
private static FlattenedConfiguration DataSourcedConfig(string instanceName) => new()
{
InstanceUniqueName = instanceName,
Attributes =
[
new ResolvedAttribute
{
CanonicalName = "Setpoint",
Value = "10",
DataType = "Double",
DataSourceReference = "/Motor/Setpoint",
BoundDataConnectionName = "OpcServer1"
}
]
};
[Fact]
public async Task SetAttribute_DataSourcedAttribute_IssuesDclWriteAndDoesNotPersistOverride()
{
var config = DataSourcedConfig("PumpDcl1");
var dclProbe = CreateTestProbe();
var actor = CreateInstanceActor("PumpDcl1", config, dclProbe.Ref);
actor.Tell(new SetStaticAttributeCommand(
"corr-dcl", "PumpDcl1", "Setpoint", "55", DateTimeOffset.UtcNow));
// The Instance Actor must forward a WriteTagRequest to the DCL manager.
var write = ExpectWriteTag(dclProbe);
Assert.Equal("OpcServer1", write.ConnectionName);
Assert.Equal("/Motor/Setpoint", write.TagPath);
Assert.Equal("55", write.Value);
// DCL confirms the write.
dclProbe.Reply(new WriteTagResponse(write.CorrelationId, true, null, DateTimeOffset.UtcNow));
var response = ExpectMsg<SetStaticAttributeResponse>(TimeSpan.FromSeconds(5));
Assert.True(response.Success);
// No static override should be persisted for a data-sourced attribute.
await Task.Delay(300);
var overrides = await _storage.GetStaticOverridesAsync("PumpDcl1");
Assert.Empty(overrides);
}
[Fact]
public void SetAttribute_DataSourcedAttribute_DoesNotOptimisticallyUpdateMemory()
{
var config = DataSourcedConfig("PumpDcl2");
var dclProbe = CreateTestProbe();
var actor = CreateInstanceActor("PumpDcl2", config, dclProbe.Ref);
actor.Tell(new SetStaticAttributeCommand(
"corr-dcl2", "PumpDcl2", "Setpoint", "999", DateTimeOffset.UtcNow));
var write = ExpectWriteTag(dclProbe);
dclProbe.Reply(new WriteTagResponse(write.CorrelationId, true, null, DateTimeOffset.UtcNow));
ExpectMsg<SetStaticAttributeResponse>(TimeSpan.FromSeconds(5));
// In-memory value must still be the original config value — it is only
// updated when the subscription delivers the confirmed device value.
actor.Tell(new GetAttributeRequest("corr-get", "PumpDcl2", "Setpoint", DateTimeOffset.UtcNow));
var get = ExpectMsg<GetAttributeResponse>(TimeSpan.FromSeconds(5));
Assert.Equal("10", get.Value?.ToString());
}
[Fact]
public void SetAttribute_DataSourcedAttribute_DclWriteFailure_ReturnedToCaller()
{
var config = DataSourcedConfig("PumpDcl3");
var dclProbe = CreateTestProbe();
var actor = CreateInstanceActor("PumpDcl3", config, dclProbe.Ref);
actor.Tell(new SetStaticAttributeCommand(
"corr-dcl3", "PumpDcl3", "Setpoint", "42", DateTimeOffset.UtcNow));
var write = ExpectWriteTag(dclProbe);
dclProbe.Reply(new WriteTagResponse(write.CorrelationId, false, "device rejected write", DateTimeOffset.UtcNow));
var response = ExpectMsg<SetStaticAttributeResponse>(TimeSpan.FromSeconds(5));
Assert.False(response.Success);
Assert.Contains("device rejected write", response.ErrorMessage);
}
[Fact]
public async Task SetAttribute_StaticAttribute_StillPersistsOverrideAndDoesNotCallDcl()
{
var config = new FlattenedConfiguration
{
InstanceUniqueName = "PumpStatic1",
Attributes =
[
new ResolvedAttribute { CanonicalName = "Label", Value = "Main", DataType = "String" }
]
};
var dclProbe = CreateTestProbe();
var actor = CreateInstanceActor("PumpStatic1", config, dclProbe.Ref);
actor.Tell(new SetStaticAttributeCommand(
"corr-static", "PumpStatic1", "Label", "Backup", DateTimeOffset.UtcNow));
var response = ExpectMsg<SetStaticAttributeResponse>(TimeSpan.FromSeconds(5));
Assert.True(response.Success);
// DCL must NOT receive a write for a static attribute.
dclProbe.ExpectNoMsg(TimeSpan.FromMilliseconds(300));
await Task.Delay(300);
var overrides = await _storage.GetStaticOverridesAsync("PumpStatic1");
Assert.Single(overrides);
Assert.Equal("Backup", overrides["Label"]);
}
}
@@ -113,11 +113,11 @@ public class InstanceActorTests : TestKit, IDisposable
var actor = CreateInstanceActor("Pump1", config); var actor = CreateInstanceActor("Pump1", config);
// SetStaticAttributeCommand is fire-and-forget (no reply); the // A static attribute write replies with SetStaticAttributeResponse.
// GetAttributeRequest round-trip below confirms it was applied — the
// actor mailbox is FIFO, so the set is processed before the get.
actor.Tell(new SetStaticAttributeCommand( actor.Tell(new SetStaticAttributeCommand(
"corr-3", "Pump1", "Temperature", "100.0", DateTimeOffset.UtcNow)); "corr-3", "Pump1", "Temperature", "100.0", DateTimeOffset.UtcNow));
var setResponse = ExpectMsg<SetStaticAttributeResponse>();
Assert.True(setResponse.Success);
// Verify the value changed in memory // Verify the value changed in memory
actor.Tell(new GetAttributeRequest( actor.Tell(new GetAttributeRequest(
@@ -145,12 +145,9 @@ public class InstanceActorTests : TestKit, IDisposable
actor.Tell(new SetStaticAttributeCommand( actor.Tell(new SetStaticAttributeCommand(
"corr-persist", "PumpPersist1", "Temperature", "100.0", DateTimeOffset.UtcNow)); "corr-persist", "PumpPersist1", "Temperature", "100.0", DateTimeOffset.UtcNow));
// SetStaticAttributeCommand is fire-and-forget; round-trip a // A static attribute write replies with SetStaticAttributeResponse once the
// GetAttributeRequest to confirm the command was processed (FIFO // in-memory state is updated; then wait for the async SQLite persist.
// mailbox), then wait for the async SQLite persist to complete. ExpectMsg<SetStaticAttributeResponse>(TimeSpan.FromSeconds(5));
actor.Tell(new GetAttributeRequest(
"corr-persist-get", "PumpPersist1", "Temperature", DateTimeOffset.UtcNow));
ExpectMsg<GetAttributeResponse>(TimeSpan.FromSeconds(5));
await Task.Delay(500); await Task.Delay(500);
// Verify it persisted to SQLite // Verify it persisted to SQLite
@@ -86,7 +86,9 @@ public class StoreAndForwardServiceTests : IAsyncLifetime, IDisposable
var msg = await _storage.GetMessageByIdAsync(result.MessageId); var msg = await _storage.GetMessageByIdAsync(result.MessageId);
Assert.NotNull(msg); Assert.NotNull(msg);
Assert.Equal(StoreAndForwardMessageStatus.Pending, msg!.Status); Assert.Equal(StoreAndForwardMessageStatus.Pending, msg!.Status);
Assert.Equal(1, msg.RetryCount); // StoreAndForward-003: RetryCount counts sweep retries only; the immediate
// attempt is attempt 0, so a freshly buffered message has RetryCount 0.
Assert.Equal(0, msg.RetryCount);
} }
[Fact] [Fact]
@@ -134,6 +136,12 @@ public class StoreAndForwardServiceTests : IAsyncLifetime, IDisposable
StoreAndForwardCategory.ExternalSystem, "api", """{}""", StoreAndForwardCategory.ExternalSystem, "api", """{}""",
maxRetries: 2); maxRetries: 2);
// StoreAndForward-003: MaxRetries bounds sweep retries (not the immediate
// attempt), so a message with MaxRetries=2 needs two retry sweeps to park.
await _service.RetryPendingMessagesAsync();
var afterFirst = await _storage.GetMessageByIdAsync(result.MessageId);
Assert.Equal(StoreAndForwardMessageStatus.Pending, afterFirst!.Status);
await _service.RetryPendingMessagesAsync(); await _service.RetryPendingMessagesAsync();
var msg = await _storage.GetMessageByIdAsync(result.MessageId); var msg = await _storage.GetMessageByIdAsync(result.MessageId);
@@ -141,6 +149,34 @@ public class StoreAndForwardServiceTests : IAsyncLifetime, IDisposable
Assert.Equal(StoreAndForwardMessageStatus.Parked, msg!.Status); Assert.Equal(StoreAndForwardMessageStatus.Parked, msg!.Status);
} }
// ── StoreAndForward-003: retry-count accounting ──
[Fact]
public async Task RetryPendingMessagesAsync_MaxRetriesOne_PerformsExactlyOneRetryBeforeParking()
{
// The immediate attempt is attempt 0; MaxRetries=1 must allow exactly one
// retry sweep before parking. The pre-fix off-by-one parked with zero retries.
var attempts = 0;
_service.RegisterDeliveryHandler(StoreAndForwardCategory.ExternalSystem,
_ => { Interlocked.Increment(ref attempts); throw new HttpRequestException("always fails"); });
var result = await _service.EnqueueAsync(
StoreAndForwardCategory.ExternalSystem, "api", """{}""",
maxRetries: 1);
// After the immediate failed attempt the message is buffered, not parked.
var buffered = await _storage.GetMessageByIdAsync(result.MessageId);
Assert.Equal(StoreAndForwardMessageStatus.Pending, buffered!.Status);
Assert.Equal(1, attempts); // only the immediate attempt so far
await _service.RetryPendingMessagesAsync();
var msg = await _storage.GetMessageByIdAsync(result.MessageId);
Assert.Equal(StoreAndForwardMessageStatus.Parked, msg!.Status);
Assert.Equal(2, attempts); // immediate attempt + exactly one retry
Assert.Equal(1, msg.RetryCount); // one sweep retry recorded
}
[Fact] [Fact]
public async Task RetryPendingMessagesAsync_PermanentFailureOnRetry_ParksMessage() public async Task RetryPendingMessagesAsync_PermanentFailureOnRetry_ParksMessage()
{ {
@@ -332,6 +368,8 @@ public class StoreAndForwardServiceTests : IAsyncLifetime, IDisposable
var msg = await _storage.GetMessageByIdAsync(result.MessageId); var msg = await _storage.GetMessageByIdAsync(result.MessageId);
Assert.NotNull(msg); Assert.NotNull(msg);
Assert.Equal(StoreAndForwardMessageStatus.Pending, msg!.Status); Assert.Equal(StoreAndForwardMessageStatus.Pending, msg!.Status);
Assert.Equal(1, msg.RetryCount); // counts as the caller's first attempt // StoreAndForward-003: the caller's own attempt is attempt 0; RetryCount
// counts only sweep retries, so a freshly buffered message has RetryCount 0.
Assert.Equal(0, msg.RetryCount);
} }
} }
@@ -370,4 +370,154 @@ public class FlatteningServiceTests
var script = result.Value.Scripts.First(s => s.CanonicalName == "Sample"); var script = result.Value.Scripts.First(s => s.CanonicalName == "Sample");
Assert.Equal("return base;", script.Code); Assert.Equal("return base;", script.Code);
} }
// ── TemplateEngine-001: deep composition nesting ───────────────────────
[Fact]
public void Flatten_ThreeLevelComposition_AttributesAlarmsScriptsAllResolved()
{
// Station composes Pump (level 1); Pump composes Motor (level 2);
// Motor composes Bearing (level 3).
var bearing = CreateTemplate(4, "Bearing");
bearing.Attributes.Add(new TemplateAttribute("Vibration") { DataType = DataType.Double, Value = "0.1" });
bearing.Alarms.Add(new TemplateAlarm("HighVibration")
{
TriggerType = AlarmTriggerType.RangeViolation,
TriggerConfiguration = "{\"attributeName\":\"Vibration\",\"high\":5}",
PriorityLevel = 1
});
bearing.Scripts.Add(new TemplateScript("MonitorBearing", "// monitor") { Id = 40 });
var motor = CreateTemplate(3, "Motor");
motor.Attributes.Add(new TemplateAttribute("Current") { DataType = DataType.Double, Value = "10" });
var pump = CreateTemplate(2, "Pump");
pump.Attributes.Add(new TemplateAttribute("RPM") { DataType = DataType.Double, Value = "1500" });
var station = CreateTemplate(1, "Station");
var compositions = new Dictionary<int, IReadOnlyList<TemplateComposition>>
{
[1] = new List<TemplateComposition> { new("MainPump") { ComposedTemplateId = 2 } },
[2] = new List<TemplateComposition> { new("DriveMotor") { ComposedTemplateId = 3 } },
[3] = new List<TemplateComposition> { new("FrontBearing") { ComposedTemplateId = 4 } },
};
var composedChains = new Dictionary<int, IReadOnlyList<Template>>
{
[2] = [pump],
[3] = [motor],
[4] = [bearing],
};
var instance = CreateInstance();
var result = _sut.Flatten(instance, [station], compositions, composedChains,
new Dictionary<int, DataConnection>());
Assert.True(result.IsSuccess);
// Level 3 attribute must be present with the full path-qualified name.
Assert.Contains(result.Value.Attributes,
a => a.CanonicalName == "MainPump.DriveMotor.FrontBearing.Vibration");
// Level 3 alarm must be present (was dropped entirely before).
Assert.Contains(result.Value.Alarms,
a => a.CanonicalName == "MainPump.DriveMotor.FrontBearing.HighVibration");
// Level 3 script must be present (was dropped entirely before).
Assert.Contains(result.Value.Scripts,
s => s.CanonicalName == "MainPump.DriveMotor.FrontBearing.MonitorBearing");
}
[Fact]
public void Flatten_NestedComposedAlarm_TriggerAttributePrefixed()
{
var bearing = CreateTemplate(4, "Bearing");
bearing.Attributes.Add(new TemplateAttribute("Vibration") { DataType = DataType.Double, Value = "0.1" });
bearing.Alarms.Add(new TemplateAlarm("HighVibration")
{
TriggerType = AlarmTriggerType.RangeViolation,
TriggerConfiguration = "{\"attributeName\":\"Vibration\",\"high\":5}",
PriorityLevel = 1
});
var motor = CreateTemplate(3, "Motor");
var pump = CreateTemplate(2, "Pump");
var station = CreateTemplate(1, "Station");
var compositions = new Dictionary<int, IReadOnlyList<TemplateComposition>>
{
[1] = new List<TemplateComposition> { new("MainPump") { ComposedTemplateId = 2 } },
[2] = new List<TemplateComposition> { new("DriveMotor") { ComposedTemplateId = 3 } },
[3] = new List<TemplateComposition> { new("FrontBearing") { ComposedTemplateId = 4 } },
};
var composedChains = new Dictionary<int, IReadOnlyList<Template>>
{
[2] = [pump], [3] = [motor], [4] = [bearing],
};
var instance = CreateInstance();
var result = _sut.Flatten(instance, [station], compositions, composedChains,
new Dictionary<int, DataConnection>());
Assert.True(result.IsSuccess);
var alarm = result.Value.Alarms.First(a => a.CanonicalName.EndsWith("HighVibration"));
// The trigger's attribute reference must carry the full nested prefix.
Assert.Contains("MainPump.DriveMotor.FrontBearing.Vibration", alarm.TriggerConfiguration);
}
// ── TemplateEngine-004: alarm on-trigger script resolution ─────────────
[Fact]
public void Flatten_AlarmOnTriggerScript_ResolvedToCanonicalName()
{
var template = CreateTemplate(1, "Base");
template.Scripts.Add(new TemplateScript("HandleAlarm", "// handle") { Id = 50 });
template.Alarms.Add(new TemplateAlarm("HighTemp")
{
TriggerType = AlarmTriggerType.RangeViolation,
TriggerConfiguration = "{\"attributeName\":\"Temp\",\"high\":100}",
PriorityLevel = 1,
OnTriggerScriptId = 50
});
var instance = CreateInstance();
var result = _sut.Flatten(instance, [template],
new Dictionary<int, IReadOnlyList<TemplateComposition>>(),
new Dictionary<int, IReadOnlyList<Template>>(),
new Dictionary<int, DataConnection>());
Assert.True(result.IsSuccess);
var alarm = result.Value.Alarms.First(a => a.CanonicalName == "HighTemp");
Assert.Equal("HandleAlarm", alarm.OnTriggerScriptCanonicalName);
}
[Fact]
public void Flatten_ComposedAlarmOnTriggerScript_ResolvedWithPrefix()
{
var composedTemplate = CreateTemplate(2, "Pump");
composedTemplate.Scripts.Add(new TemplateScript("PumpAlarmHandler", "// h") { Id = 60 });
composedTemplate.Alarms.Add(new TemplateAlarm("PumpFault")
{
TriggerType = AlarmTriggerType.ValueMatch,
TriggerConfiguration = "{\"attributeName\":\"State\",\"value\":\"FAULT\"}",
PriorityLevel = 5,
OnTriggerScriptId = 60
});
var station = CreateTemplate(1, "Station");
var compositions = new Dictionary<int, IReadOnlyList<TemplateComposition>>
{
[1] = new List<TemplateComposition> { new("MainPump") { ComposedTemplateId = 2 } }
};
var composedChains = new Dictionary<int, IReadOnlyList<Template>>
{
[2] = [composedTemplate]
};
var instance = CreateInstance();
var result = _sut.Flatten(instance, [station], compositions, composedChains,
new Dictionary<int, DataConnection>());
Assert.True(result.IsSuccess);
var alarm = result.Value.Alarms.First(a => a.CanonicalName == "MainPump.PumpFault");
Assert.Equal("MainPump.PumpAlarmHandler", alarm.OnTriggerScriptCanonicalName);
}
} }
@@ -60,6 +60,23 @@ public class TemplateServiceTests
Assert.Equal(1, result.Value.ParentTemplateId); Assert.Equal(1, result.Value.ParentTemplateId);
} }
[Fact]
public async Task CreateTemplate_WithParent_DoesNotRunDeadCollisionQuery()
{
// A freshly created child has no members of its own, and the parent's
// members were already collision-validated when they were added — so
// create-time collision detection on a child is a guaranteed no-op.
// The previous code allocated an unused full-table read; the fix
// removes it. This guards against the dead query being reintroduced.
var parent = new Template("Base") { Id = 1 };
_repoMock.Setup(r => r.GetTemplateByIdAsync(1, It.IsAny<CancellationToken>())).ReturnsAsync(parent);
var result = await _service.CreateTemplateAsync("Child", null, 1, "admin");
Assert.True(result.IsSuccess);
_repoMock.Verify(r => r.GetAllTemplatesAsync(It.IsAny<CancellationToken>()), Times.Never);
}
[Fact] [Fact]
public async Task CreateTemplate_NonexistentParent_Fails() public async Task CreateTemplate_NonexistentParent_Fails()
{ {
@@ -668,6 +685,54 @@ public class TemplateServiceTests
Assert.True(result.Value.IsLocked); Assert.True(result.Value.IsLocked);
} }
[Fact]
public async Task UpdateAttribute_UnlockedAttribute_DataTypeChangeRejected()
{
// An unlocked attribute must still not be able to change its fixed DataType.
var existing = new TemplateAttribute("Temperature")
{
Id = 1, TemplateId = 1, DataType = DataType.Float, IsLocked = false
};
_repoMock.Setup(r => r.GetTemplateAttributeByIdAsync(1, It.IsAny<CancellationToken>())).ReturnsAsync(existing);
var template = new Template("Pump") { Id = 1 };
_repoMock.Setup(r => r.GetTemplateByIdAsync(1, It.IsAny<CancellationToken>())).ReturnsAsync(template);
var proposed = new TemplateAttribute("Temperature")
{
DataType = DataType.Int32, IsLocked = false, Value = "42"
};
var result = await _service.UpdateAttributeAsync(1, proposed, "admin");
Assert.True(result.IsFailure);
Assert.Contains("DataType", result.Error);
// The fixed field must not have been mutated.
Assert.Equal(DataType.Float, existing.DataType);
}
[Fact]
public async Task UpdateAttribute_UnlockedAttribute_DataSourceReferenceChangeRejected()
{
var existing = new TemplateAttribute("Temperature")
{
Id = 1, TemplateId = 1, DataType = DataType.Float, IsLocked = false,
DataSourceReference = "/Motor/Temp"
};
_repoMock.Setup(r => r.GetTemplateAttributeByIdAsync(1, It.IsAny<CancellationToken>())).ReturnsAsync(existing);
var template = new Template("Pump") { Id = 1 };
_repoMock.Setup(r => r.GetTemplateByIdAsync(1, It.IsAny<CancellationToken>())).ReturnsAsync(template);
var proposed = new TemplateAttribute("Temperature")
{
DataType = DataType.Float, IsLocked = false, Value = "42",
DataSourceReference = "/Motor/Other"
};
var result = await _service.UpdateAttributeAsync(1, proposed, "admin");
Assert.True(result.IsFailure);
Assert.Contains("DataSourceReference", result.Error);
Assert.Equal("/Motor/Temp", existing.DataSourceReference);
}
[Fact] [Fact]
public async Task UpdateAttribute_ParentLocked_CannotOverride() public async Task UpdateAttribute_ParentLocked_CannotOverride()
{ {