docs(plan): design for F10b surgical in-place tag-attribute writes (Writable/Historizing)
This commit is contained in:
Joseph Doherty
@@ -0,0 +1,221 @@
|
||||
# F10b surgical in-place tag-attribute writes (Writable / Historizing) — Design
|
||||
|
||||
**Date:** 2026-06-18
|
||||
**Branch:** `feat/f10b-surgical-tag-attribute-writes` (off master `bbba911b`)
|
||||
**Backlog item:** `stillpending.md` §A #11 (F10b — the deferred surgical-write tier; **safe subset**, user-approved)
|
||||
|
||||
The next F10b slice after the shipped vtag rebuild-skip. Scope was narrowed by exploration + a user
|
||||
scope-decision to the **safe, valuable subset**: surgical in-place updates for equipment-tag **`Writable`**
|
||||
and **`IsHistorized`/`HistorianTagname`** toggles. Explicitly **excluded**:
|
||||
- **Alarm severity** — a deploy-time authored severity is immediately overwritten by the live alarm engine
|
||||
(`ScriptedAlarmHostActor` → `AlarmStateUpdate` → `SetSeverity`), so a surgical write is operationally
|
||||
invisible. Dropped.
|
||||
- **Tag `DataType` / `IsArray` / `ArrayLength`** — changing a node's declared type/rank in place leaves the
|
||||
cached `Value` briefly type-mismatched and clients get no `ModelChangeEvents`; "dirty" for rare edits.
|
||||
These keep the full rebuild.
|
||||
|
||||
### Why this differs from the shipped vtag-skip
|
||||
|
||||
The vtag-skip was a *pure skip* — the vtag node was byte-identical, so skipping `RebuildAddressSpace` +
|
||||
letting the idempotent `Materialise*` passes no-op was correct. A tag-attribute change is different: an
|
||||
attribute (AccessLevel / Historizing) **actually changes**, and `MaterialiseEquipmentTags` →
|
||||
`EnsureVariable` **early-returns on an existing node** (`OtOpcUaNodeManager:1324`), so it will NOT update an
|
||||
existing node's attributes. Therefore this slice must **actively mutate the existing node** (a real surgical
|
||||
write) — skipping the rebuild *without* a surgical write would leave a stale node (the H1a bug). The win
|
||||
remains the same: a `Writable`/`Historize` toggle no longer tears down + rebuilds the whole server address
|
||||
space, preserving every client's subscriptions.
|
||||
|
||||
---
|
||||
|
||||
## Standing constraints (in force)
|
||||
|
||||
- NO Commons wire/proto contract change · NO **breaking** interface change (we ADD a new optional interface,
|
||||
we do NOT modify `IOpcUaAddressSpaceSink`) · NO EF migration · NO bUnit.
|
||||
- Stage by explicit path (never `git add .`); never stage the never-stage files. No force-push / no
|
||||
`--no-verify`. Finish = merge + push. `dangerouslyDisableSandbox: true` for build/test/rig.
|
||||
|
||||
---
|
||||
|
||||
## Component A — the surgical-write capability (node manager + sink)
|
||||
|
||||
### New optional interface (Commons, NON-breaking)
|
||||
|
||||
`src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/ISurgicalAddressSpaceSink.cs`:
|
||||
|
||||
```csharp
|
||||
namespace ZB.MOM.WW.OtOpcUa.Commons.OpcUa;
|
||||
|
||||
/// <summary>Optional capability on an address-space sink: surgical in-place attribute updates on an
|
||||
/// EXISTING variable node, used by Phase7Applier to avoid a full RebuildAddressSpace for pure-property
|
||||
/// tag changes (Writable / Historizing). A sink that does not implement it ⇒ caller falls back to a
|
||||
/// full rebuild (safe default).</summary>
|
||||
public interface ISurgicalAddressSpaceSink
|
||||
{
|
||||
/// <summary>Update an existing variable node's Writable (AccessLevel + inbound-write handler) and
|
||||
/// Historizing (+ historian-tagname binding) IN PLACE, notifying subscribers (ClearChangeMasks)
|
||||
/// without a rebuild. Returns false if the node does not exist (caller should rebuild instead).</summary>
|
||||
bool UpdateTagAttributes(string variableNodeId, bool writable, string? historianTagname);
|
||||
}
|
||||
```
|
||||
|
||||
We add a NEW interface rather than a method on `IOpcUaAddressSpaceSink` so no existing implementer breaks,
|
||||
and the `is ISurgicalAddressSpaceSink` probe gives a built-in rebuild fallback.
|
||||
|
||||
### `OtOpcUaNodeManager.UpdateTagAttributes` (mirrors `EnsureVariable` exactly)
|
||||
|
||||
`EnsureVariable` (`:1318-1370`) composes attributes thus: `historized = historianTagname is not null`;
|
||||
`access = writable ? (CurrentRead|CurrentWrite) : CurrentRead`, `|= HistoryRead` if historized;
|
||||
`Historizing = historized`; `OnWriteValue = writable ? OnEquipmentTagWrite : null`; and
|
||||
`_historizedTagnames[nodeId] = historianTagname` when historized.
|
||||
|
||||
To prevent drift, **extract a private static `ComposeAccessLevel(bool writable, bool historized) -> byte`**
|
||||
used by both `EnsureVariable` and the new method. Then:
|
||||
|
||||
```csharp
|
||||
public bool UpdateTagAttributes(string variableNodeId, bool writable, string? historianTagname)
|
||||
{
|
||||
lock (Lock)
|
||||
{
|
||||
if (!_variables.TryGetValue(variableNodeId, out var v)) return false; // caller falls back to rebuild
|
||||
var historized = historianTagname is not null;
|
||||
var access = ComposeAccessLevel(writable, historized);
|
||||
v.AccessLevel = access;
|
||||
v.UserAccessLevel = access;
|
||||
v.Historizing = historized;
|
||||
v.OnWriteValue = writable ? OnEquipmentTagWrite : null; // attach/detach the inbound-write handler
|
||||
if (historized) _historizedTagnames[variableNodeId] = historianTagname!;
|
||||
else _historizedTagnames.TryRemove(variableNodeId, out _);
|
||||
v.ClearChangeMasks(SystemContext, includeChildren: false); // notify subscribers; node object preserved
|
||||
return true;
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
This mutates the SAME `BaseDataVariableState` object that existing client MonitoredItems reference (the
|
||||
proven `WriteValue`/`ClearChangeMasks` pattern), so subscriptions survive. ModelChangeEvent emission is out
|
||||
of scope — consistent with the rest of the server (it doesn't emit them on rebuild either).
|
||||
|
||||
### `SdkAddressSpaceSink` implements `ISurgicalAddressSpaceSink`
|
||||
|
||||
Add `: ISurgicalAddressSpaceSink` and a one-line delegation:
|
||||
`public bool UpdateTagAttributes(string n, bool w, string? h) => _nodeManager.UpdateTagAttributes(n, w, h);`
|
||||
`NullOpcUaAddressSpaceSink` does NOT implement it (→ Phase7Applier rebuilds, a no-op on Null).
|
||||
|
||||
---
|
||||
|
||||
## Component B — Phase7Applier surgical-eligibility + apply/rebuild-fallback
|
||||
|
||||
### Eligibility (whitelist-of-may-differ, same safe-default discipline as the vtag-skip)
|
||||
|
||||
`EquipmentTagPlan` uses auto-generated record equality. A changed tag delta is **surgical-eligible** iff it
|
||||
is a plain value variable (no condition node) AND its ONLY differences are `Writable` / `IsHistorized` /
|
||||
`HistorianTagname`:
|
||||
|
||||
```csharp
|
||||
private static bool TagDeltaIsSurgicalEligible(Phase7Plan.EquipmentTagDelta d) =>
|
||||
d.Previous.Alarm is null && d.Current.Alarm is null && // value variables only (alarm presence ⇒ structural)
|
||||
(d.Previous with
|
||||
{
|
||||
Writable = d.Current.Writable,
|
||||
IsHistorized = d.Current.IsHistorized,
|
||||
HistorianTagname = d.Current.HistorianTagname,
|
||||
}).Equals(d.Current);
|
||||
```
|
||||
|
||||
Any other difference — `DataType`, `IsArray`/`ArrayLength`, `FullName`, `DriverInstanceId`, identity
|
||||
(`TagId`/`EquipmentId`/`FolderPath`/`Name`), or `Alarm` — makes the override unequal ⇒ **rebuild** (safe
|
||||
default; covers future fields too).
|
||||
|
||||
### Control flow in `Apply`
|
||||
|
||||
`needsRebuild`'s `ChangedEquipmentTags.Count > 0` term becomes
|
||||
`ChangedEquipmentTags.Any(d => !TagDeltaIsSurgicalEligible(d))` (mirroring the existing vtag term). Then:
|
||||
|
||||
```
|
||||
structuralRebuild = <all existing terms, with the surgical-aware tag + vtag terms>
|
||||
surgicalTagDeltas = plan.ChangedEquipmentTags.Where(TagDeltaIsSurgicalEligible)
|
||||
rebuilt = false
|
||||
if (structuralRebuild) { _sink.RebuildAddressSpace(); rebuilt = true; }
|
||||
else if (surgicalTagDeltas.Any())
|
||||
{
|
||||
if (_sink is ISurgicalAddressSpaceSink surgical)
|
||||
{
|
||||
var allApplied = true;
|
||||
foreach (var d in surgicalTagDeltas)
|
||||
{
|
||||
var nodeId = EquipmentNodeIds.Variable(d.Current.EquipmentId, d.Current.FolderPath, d.Current.Name);
|
||||
var writable = d.Current.Writable && !d.Current.IsArray; // mirror MaterialiseEquipmentTags
|
||||
var historian = d.Current.IsHistorized
|
||||
? (string.IsNullOrWhiteSpace(d.Current.HistorianTagname) ? d.Current.FullName : d.Current.HistorianTagname)
|
||||
: null;
|
||||
if (!surgical.UpdateTagAttributes(nodeId, writable, historian)) { allApplied = false; break; }
|
||||
}
|
||||
if (!allApplied) { _sink.RebuildAddressSpace(); rebuilt = true; } // anomaly (node missing) ⇒ safe rebuild
|
||||
}
|
||||
else { _sink.RebuildAddressSpace(); rebuilt = true; } // sink lacks the capability ⇒ rebuild
|
||||
}
|
||||
return new Phase7ApplyOutcome(removedCount, addedCount, changedCount, RebuildCalled: rebuilt);
|
||||
```
|
||||
|
||||
`changedCount`/`ChangedNodes` is unchanged (the edit still counts). The effective `writable`/`historian`
|
||||
computation is byte-identical to `MaterialiseEquipmentTags` so the surgical node matches what a rebuild
|
||||
would have produced.
|
||||
|
||||
### Note on the downstream Materialise* passes
|
||||
|
||||
`OpcUaPublishActor.HandleRebuild` still runs `Materialise*` unconditionally after `Apply`. On the surgical
|
||||
path the node already exists, so `EnsureVariable` early-returns — it will NOT clobber the surgical
|
||||
attributes. Correct.
|
||||
|
||||
---
|
||||
|
||||
## Testing
|
||||
|
||||
### Phase7Applier (offline, `RecordingSink` now also implements `ISurgicalAddressSpaceSink`)
|
||||
|
||||
Extend the test `RecordingSink` to implement `ISurgicalAddressSpaceSink`, recording
|
||||
`(nodeId, writable, historianTagname)` of each `UpdateTagAttributes` call (and a separate
|
||||
`RecordingSink` variant that does NOT, to test the fallback). Facts:
|
||||
- **Writable-only** toggle (non-array, non-alarm) ⇒ no rebuild; one `UpdateTagAttributes(writable=new, historian=…)`.
|
||||
- **IsHistorized** toggle ⇒ no rebuild; `UpdateTagAttributes(historian = FullName-or-override / null)`.
|
||||
- **HistorianTagname-only** change (already historized) ⇒ no rebuild; `UpdateTagAttributes(historian=new)`.
|
||||
- **DataType** change ⇒ rebuild (excluded).
|
||||
- **IsArray / ArrayLength** change ⇒ rebuild.
|
||||
- **FullName** change ⇒ rebuild (excluded from may-differ).
|
||||
- **Name / FolderPath / EquipmentId** change ⇒ rebuild.
|
||||
- **Alarm presence** change (null↔non-null) ⇒ rebuild.
|
||||
- surgical-eligible delta **mixed with another change** ⇒ rebuild.
|
||||
- sink **without** `ISurgicalAddressSpaceSink` + surgical-eligible delta ⇒ rebuild (fallback).
|
||||
- `UpdateTagAttributes` returns **false** (node missing) ⇒ rebuild (fallback).
|
||||
|
||||
### OtOpcUaNodeManager (if a unit harness exists)
|
||||
|
||||
If the node manager can be constructed in a test (check for an existing fixture), add a direct test: create
|
||||
a variable via `EnsureVariable`, call `UpdateTagAttributes` to flip writable/historized, assert
|
||||
`AccessLevel`/`UserAccessLevel`/`Historizing`/`OnWriteValue`(attached/null)/`_historizedTagnames` updated +
|
||||
`ComposeAccessLevel` parity with `EnsureVariable`. If no harness exists, rely on the Phase7Applier tests +
|
||||
the live `/run`.
|
||||
|
||||
### Live `/run` (confirmatory — proves the SDK honors it)
|
||||
|
||||
docker-dev runs the real `SdkAddressSpaceSink`. Rebuild BOTH central nodes (the `:9200` round-robin fact),
|
||||
subscribe a Client.CLI to an equipment tag, toggle that tag's Historize (or Writable) in `/uns`, Deploy →
|
||||
the server log shows `rebuild=False` for that deploy, the subscription is NOT dropped, the server stays
|
||||
healthy, and the node remains readable. Confirms the surgical mutation + subscription survival.
|
||||
|
||||
---
|
||||
|
||||
## Task slicing
|
||||
|
||||
| Task | Component | Project(s) | Class | Depends on |
|
||||
|---|---|---|---|---|
|
||||
| T1 | `ISurgicalAddressSpaceSink` + `OtOpcUaNodeManager.UpdateTagAttributes` (+ `ComposeAccessLevel` extract) + `SdkAddressSpaceSink` impl | Commons + OpcUaServer | standard | — |
|
||||
| T2 | Phase7Applier `TagDeltaIsSurgicalEligible` + apply/rebuild-fallback control flow + RecordingSink tests | OpcUaServer (+ .Tests) | standard | T1 (interface) |
|
||||
| T3 | Reconcile stillpending #11 + memory + build + tests + live `/run` + finish (merge+push) | docs (never-staged) | small | T1, T2 |
|
||||
|
||||
T1→T2 are coupled by the new interface ⇒ serial. T3 last.
|
||||
|
||||
## Done =
|
||||
|
||||
Build clean + `dotnet test` green (Commons + OpcUaServer.Tests) + live `/run` shows a Writable/Historize
|
||||
toggle deploy logs `rebuild=False` with subscriptions intact + merged to master + pushed.
|
||||
Reference in New Issue
Block a user