fix(r2-04): Materialise* passes return swallowed-failure tallies (void -> int)

This commit is contained in:
Joseph Doherty
2026-07-13 10:41:19 -04:00
parent b3d57ca363
commit e37abf36a0
3 changed files with 116 additions and 32 deletions
@@ -4,7 +4,7 @@
"tasks": [
{ "id": "T1", "subject": "Extend AddressSpaceApplyOutcome with RebuildFailed/FailedNodes + SafeRebuild returns bool (01/S-1)", "status": "completed", "blockedBy": [] },
{ "id": "T2", "subject": "Safe* helpers return bool; Apply tallies removal-pass failures into FailedNodes", "status": "completed", "blockedBy": ["T1"] },
{ "id": "T3", "subject": "Materialise* passes return swallowed-failure counts (void -> int, five methods)", "status": "pending", "blockedBy": ["T2"] },
{ "id": "T3", "subject": "Materialise* passes return swallowed-failure counts (void -> int, five methods)", "status": "completed", "blockedBy": ["T2"] },
{ "id": "T4", "subject": "OpcUaApplyFailed counter + OpcUaPublishActor Error/meter surfacing of degraded applies", "status": "pending", "blockedBy": ["T3"] },
{ "id": "T5", "subject": "Galaxy internal IMxWriteOps seam under GatewayGalaxyDataWriter (sealed SDK session untestable) (06/S-1)", "status": "pending", "blockedBy": [] },
{ "id": "T6", "subject": "Galaxy fail-closed write on AdviseSupervisory failure -> BadCommunicationError, no raw write", "status": "pending", "blockedBy": ["T5"] },
@@ -401,28 +401,32 @@ public sealed class AddressSpaceApplier
/// present, so re-applies are cheap.
/// </summary>
/// <param name="composition">The composition result containing the hierarchy to materialise.</param>
public void MaterialiseHierarchy(AddressSpaceComposition composition)
/// <returns>The count of swallowed sink failures in this pass (0 on a clean apply) — the publish actor
/// sums it into the degraded-apply surface (archreview 01/S-1).</returns>
public int MaterialiseHierarchy(AddressSpaceComposition composition)
{
ArgumentNullException.ThrowIfNull(composition);
var failed = 0;
foreach (var area in composition.UnsAreas)
{
SafeEnsureFolder(area.UnsAreaId, parentNodeId: null, displayName: area.DisplayName);
if (!SafeEnsureFolder(area.UnsAreaId, parentNodeId: null, displayName: area.DisplayName)) failed++;
}
foreach (var line in composition.UnsLines)
{
SafeEnsureFolder(line.UnsLineId, parentNodeId: line.UnsAreaId, displayName: line.DisplayName);
if (!SafeEnsureFolder(line.UnsLineId, parentNodeId: line.UnsAreaId, displayName: line.DisplayName)) failed++;
}
foreach (var equipment in composition.EquipmentNodes)
{
// Equipment with no UnsLineId (legacy / dev rows) hang under the root.
var parent = string.IsNullOrWhiteSpace(equipment.UnsLineId) ? null : equipment.UnsLineId;
SafeEnsureFolder(equipment.EquipmentId, parentNodeId: parent, displayName: equipment.DisplayName);
if (!SafeEnsureFolder(equipment.EquipmentId, parentNodeId: parent, displayName: equipment.DisplayName)) failed++;
}
_logger.LogInformation(
"AddressSpaceApplier: hierarchy materialised (areas={Areas}, lines={Lines}, equipment={Equipment})",
composition.UnsAreas.Count, composition.UnsLines.Count, composition.EquipmentNodes.Count);
"AddressSpaceApplier: hierarchy materialised (areas={Areas}, lines={Lines}, equipment={Equipment}, failed={Failed})",
composition.UnsAreas.Count, composition.UnsLines.Count, composition.EquipmentNodes.Count, failed);
return failed;
}
/// <summary>
@@ -440,11 +444,14 @@ public sealed class AddressSpaceApplier
/// takes a plain <c>string dataType</c> (not a DriverAttributeInfo).
/// </summary>
/// <param name="composition">The composition result containing the equipment tags to materialise.</param>
public void MaterialiseEquipmentTags(AddressSpaceComposition composition)
/// <returns>The count of swallowed sink failures in this pass (0 on a clean apply) — the publish actor
/// sums it into the degraded-apply surface (archreview 01/S-1).</returns>
public int MaterialiseEquipmentTags(AddressSpaceComposition composition)
{
ArgumentNullException.ThrowIfNull(composition);
if (composition.EquipmentTags.Count == 0) return;
if (composition.EquipmentTags.Count == 0) return 0;
var failed = 0;
// Sub-folders first — a tag's FolderPath becomes one folder UNDER its equipment folder
// (deduped per distinct equipment+path). Tags with no FolderPath hang directly under the
// equipment folder, which MaterialiseHierarchy already created (never re-create
@@ -455,7 +462,7 @@ public sealed class AddressSpaceApplier
if (string.IsNullOrWhiteSpace(tag.FolderPath)) continue;
var folderNodeId = EquipmentNodeIds.SubFolder(tag.EquipmentId, tag.FolderPath);
if (!foldersCreated.Add(folderNodeId)) continue;
SafeEnsureFolder(folderNodeId, parentNodeId: tag.EquipmentId, displayName: tag.FolderPath);
if (!SafeEnsureFolder(folderNodeId, parentNodeId: tag.EquipmentId, displayName: tag.FolderPath)) failed++;
}
// Variables: NodeId is FOLDER-SCOPED ("<parent>/<Name>"), NOT the raw FullName — a driver
@@ -474,7 +481,7 @@ public sealed class AddressSpaceApplier
{
// Native alarm tag → a real Part 9 condition node (reuses the scripted-alarm path),
// NOT a value variable. Parent is the sub-folder when set, else the equipment folder.
SafeMaterialiseAlarmCondition(nodeId, parent, tag.Name, tag.Alarm.AlarmType, tag.Alarm.Severity, isNative: true);
if (!SafeMaterialiseAlarmCondition(nodeId, parent, tag.Name, tag.Alarm.AlarmType, tag.Alarm.Severity, isNative: true)) failed++;
}
else
{
@@ -488,14 +495,16 @@ public sealed class AddressSpaceApplier
// even if authored ReadWrite, so a client write cannot reach the driver write path which
// does not handle arrays (e.g. S7 BoxValueForWrite would crash).
var writable = tag.Writable && !tag.IsArray;
SafeEnsureVariable(nodeId, parent, tag.Name, tag.DataType, writable, historianTagname, tag.IsArray, tag.ArrayLength);
if (!SafeEnsureVariable(nodeId, parent, tag.Name, tag.DataType, writable, historianTagname, tag.IsArray, tag.ArrayLength)) failed++;
}
}
_logger.LogInformation(
"AddressSpaceApplier: equipment tags materialised (tags={Tags}, equipment={Equipment})",
"AddressSpaceApplier: equipment tags materialised (tags={Tags}, equipment={Equipment}, failed={Failed})",
composition.EquipmentTags.Count,
composition.EquipmentTags.Select(t => t.EquipmentId).Distinct(StringComparer.Ordinal).Count());
composition.EquipmentTags.Select(t => t.EquipmentId).Distinct(StringComparer.Ordinal).Count(),
failed);
return failed;
}
/// <summary>
@@ -509,7 +518,9 @@ public sealed class AddressSpaceApplier
/// NodeAdded model-change is announced under this node.</param>
/// <param name="folders">The discovered folders to ensure (parent-first by depth).</param>
/// <param name="variables">The discovered variables to ensure (read-only value nodes).</param>
public void MaterialiseDiscoveredNodes(
/// <returns>The count of swallowed sink failures in this pass (0 on a clean apply) — the publish actor
/// surfaces a non-zero count as a degraded discovered-node injection (archreview 01/S-1).</returns>
public int MaterialiseDiscoveredNodes(
string equipmentRootNodeId,
IReadOnlyList<DiscoveredFolder> folders,
IReadOnlyList<DiscoveredVariable> variables)
@@ -517,25 +528,27 @@ public sealed class AddressSpaceApplier
ArgumentException.ThrowIfNullOrEmpty(equipmentRootNodeId);
ArgumentNullException.ThrowIfNull(folders);
ArgumentNullException.ThrowIfNull(variables);
if (folders.Count == 0 && variables.Count == 0) return;
if (folders.Count == 0 && variables.Count == 0) return 0;
var failed = 0;
// Parent-first: a child folder's parent must exist before it. Ordering by '/' count == depth.
foreach (var f in folders.OrderBy(f => f.NodeId.Count(c => c == '/')))
SafeEnsureFolder(f.NodeId, f.ParentNodeId, f.DisplayName);
if (!SafeEnsureFolder(f.NodeId, f.ParentNodeId, f.DisplayName)) failed++;
foreach (var v in variables)
{
// Mirror MaterialiseEquipmentTags: arrays forced read-only (the driver write path can't handle arrays).
var writable = v.Writable && !v.IsArray;
SafeEnsureVariable(v.NodeId, v.ParentNodeId, v.DisplayName, v.DataType, writable,
historianTagname: null, isArray: v.IsArray, arrayLength: v.ArrayLength);
if (!SafeEnsureVariable(v.NodeId, v.ParentNodeId, v.DisplayName, v.DataType, writable,
historianTagname: null, isArray: v.IsArray, arrayLength: v.ArrayLength)) failed++;
}
_sink.RaiseNodesAddedModelChange(equipmentRootNodeId);
_logger.LogInformation(
"AddressSpaceApplier: discovered nodes materialised under {Equipment} (folders={Folders}, vars={Vars})",
equipmentRootNodeId, folders.Count, variables.Count);
"AddressSpaceApplier: discovered nodes materialised under {Equipment} (folders={Folders}, vars={Vars}, failed={Failed})",
equipmentRootNodeId, folders.Count, variables.Count, failed);
return failed;
}
/// <summary>
@@ -551,11 +564,14 @@ public sealed class AddressSpaceApplier
/// Idempotent (per-variable idempotency relies on the sink's own <c>EnsureVariable</c>).
/// </summary>
/// <param name="composition">The composition result containing the equipment VirtualTags to materialise.</param>
public void MaterialiseEquipmentVirtualTags(AddressSpaceComposition composition)
/// <returns>The count of swallowed sink failures in this pass (0 on a clean apply) — the publish actor
/// sums it into the degraded-apply surface (archreview 01/S-1).</returns>
public int MaterialiseEquipmentVirtualTags(AddressSpaceComposition composition)
{
ArgumentNullException.ThrowIfNull(composition);
if (composition.EquipmentVirtualTags.Count == 0) return;
if (composition.EquipmentVirtualTags.Count == 0) return 0;
var failed = 0;
// Sub-folders first — a VirtualTag's FolderPath becomes one folder UNDER its equipment folder
// (deduped per distinct equipment+path). VirtualTags with no FolderPath hang directly under the
// equipment folder, which MaterialiseHierarchy already created (never re-create it here).
@@ -565,7 +581,7 @@ public sealed class AddressSpaceApplier
if (string.IsNullOrWhiteSpace(v.FolderPath)) continue;
var folderNodeId = EquipmentNodeIds.SubFolder(v.EquipmentId, v.FolderPath);
if (!foldersCreated.Add(folderNodeId)) continue;
SafeEnsureFolder(folderNodeId, parentNodeId: v.EquipmentId, displayName: v.FolderPath);
if (!SafeEnsureFolder(folderNodeId, parentNodeId: v.EquipmentId, displayName: v.FolderPath)) failed++;
}
// Variables: NodeId is FOLDER-SCOPED ("<parent>/<Name>"), mirroring the equipment-tag pass.
@@ -581,13 +597,15 @@ public sealed class AddressSpaceApplier
: EquipmentNodeIds.SubFolder(v.EquipmentId, v.FolderPath);
var nodeId = EquipmentNodeIds.Variable(v.EquipmentId, v.FolderPath, v.Name);
// VirtualTags are computed outputs — read-only nodes (no inbound write).
SafeEnsureVariable(nodeId, parent, v.Name, v.DataType, writable: false);
if (!SafeEnsureVariable(nodeId, parent, v.Name, v.DataType, writable: false)) failed++;
}
_logger.LogInformation(
"AddressSpaceApplier: equipment virtualtags materialised (vtags={Vtags}, equipment={Equipment})",
"AddressSpaceApplier: equipment virtualtags materialised (vtags={Vtags}, equipment={Equipment}, failed={Failed})",
composition.EquipmentVirtualTags.Count,
composition.EquipmentVirtualTags.Select(v => v.EquipmentId).Distinct(StringComparer.Ordinal).Count());
composition.EquipmentVirtualTags.Select(v => v.EquipmentId).Distinct(StringComparer.Ordinal).Count(),
failed);
return failed;
}
/// <summary>
@@ -600,24 +618,29 @@ public sealed class AddressSpaceApplier
/// <c>MaterialiseAlarmCondition</c> re-creates cleanly on re-apply).
/// </summary>
/// <param name="composition">The composition result containing the scripted alarms to materialise.</param>
public void MaterialiseScriptedAlarms(AddressSpaceComposition composition)
/// <returns>The count of swallowed sink failures in this pass (0 on a clean apply) — the publish actor
/// sums it into the degraded-apply surface (archreview 01/S-1).</returns>
public int MaterialiseScriptedAlarms(AddressSpaceComposition composition)
{
ArgumentNullException.ThrowIfNull(composition);
if (composition.EquipmentScriptedAlarms.Count == 0) return;
if (composition.EquipmentScriptedAlarms.Count == 0) return 0;
var materialised = 0;
var failed = 0;
foreach (var alarm in composition.EquipmentScriptedAlarms)
{
if (!alarm.Enabled) continue;
SafeMaterialiseAlarmCondition(alarm.ScriptedAlarmId, alarm.EquipmentId, alarm.Name, alarm.AlarmType, alarm.Severity, isNative: false);
if (!SafeMaterialiseAlarmCondition(alarm.ScriptedAlarmId, alarm.EquipmentId, alarm.Name, alarm.AlarmType, alarm.Severity, isNative: false)) failed++;
materialised++;
}
_logger.LogInformation(
"AddressSpaceApplier: scripted alarms materialised (alarms={Alarms}, equipment={Equipment})",
"AddressSpaceApplier: scripted alarms materialised (alarms={Alarms}, equipment={Equipment}, failed={Failed})",
materialised,
composition.EquipmentScriptedAlarms.Where(a => a.Enabled)
.Select(a => a.EquipmentId).Distinct(StringComparer.Ordinal).Count());
.Select(a => a.EquipmentId).Distinct(StringComparer.Ordinal).Count(),
failed);
return failed;
}
/// <summary>Ensure a folder node, swallowing (and Warning-logging) any sink fault. Returns <c>true</c>
@@ -58,8 +58,69 @@ public sealed class AddressSpaceApplierFailureSurfaceTests
outcome.FailedNodes.ShouldBe(1);
}
// ---------------- T3: Materialise* passes return failed-node counts ----------------
/// <summary>Every EnsureVariable throwing in MaterialiseEquipmentTags is tallied into the pass's int return.</summary>
[Fact]
public void MaterialiseEquipmentTags_WhenEnsureVariableThrows_ReturnsFailedCount()
{
var sink = new ConfigurableThrowingSink { ThrowOnEnsureVariable = true };
var applier = new AddressSpaceApplier(sink, NullLogger<AddressSpaceApplier>.Instance);
var composition = CompositionWithTags(
new EquipmentTagPlan("t1", "eq-1", "drv", FolderPath: "", Name: "A", DataType: "Float", FullName: "40001", Writable: false, Alarm: null),
new EquipmentTagPlan("t2", "eq-1", "drv", FolderPath: "", Name: "B", DataType: "Float", FullName: "40002", Writable: false, Alarm: null),
new EquipmentTagPlan("t3", "eq-1", "drv", FolderPath: "", Name: "C", DataType: "Float", FullName: "40003", Writable: false, Alarm: null));
applier.MaterialiseEquipmentTags(composition).ShouldBe(3);
}
/// <summary>All five Materialise* passes return 0 failures on a clean sink — the happy-path regression.</summary>
[Fact]
public void Materialise_HappyPath_AllPassesReturnZero()
{
var sink = new ConfigurableThrowingSink();
var applier = new AddressSpaceApplier(sink, NullLogger<AddressSpaceApplier>.Instance);
var tags = CompositionWithTags(
new EquipmentTagPlan("t1", "eq-1", "drv", FolderPath: "", Name: "A", DataType: "Float", FullName: "40001", Writable: false, Alarm: null));
var vtags = CompositionWithVirtualTags(
new EquipmentVirtualTagPlan("v1", "eq-1", FolderPath: "", Name: "VA", DataType: "Float", Expression: "1", DependencyRefs: Array.Empty<string>(), Historize: false));
var alarms = CompositionWithScriptedAlarms(
new EquipmentScriptedAlarmPlan("a1", "eq-1", "Alarm", "OffNormalAlarm", 500, "msg", "pred", "true",
Array.Empty<string>(), HistorizeToAveva: false, Retain: true, Enabled: true));
applier.MaterialiseHierarchy(tags).ShouldBe(0);
applier.MaterialiseEquipmentTags(tags).ShouldBe(0);
applier.MaterialiseEquipmentVirtualTags(vtags).ShouldBe(0);
applier.MaterialiseScriptedAlarms(alarms).ShouldBe(0);
applier.MaterialiseDiscoveredNodes(
"eq-1",
Array.Empty<DiscoveredFolder>(),
new[] { new DiscoveredVariable("eq-1/D", "eq-1", "D", "Float", Writable: false, IsArray: false, ArrayLength: null) })
.ShouldBe(0);
}
// ---------------- fixtures ----------------
private static AddressSpaceComposition CompositionWithTags(params EquipmentTagPlan[] tags) =>
new(Array.Empty<EquipmentNode>(), Array.Empty<DriverInstancePlan>(), Array.Empty<ScriptedAlarmPlan>())
{
EquipmentTags = tags,
};
private static AddressSpaceComposition CompositionWithVirtualTags(params EquipmentVirtualTagPlan[] vtags) =>
new(Array.Empty<EquipmentNode>(), Array.Empty<DriverInstancePlan>(), Array.Empty<ScriptedAlarmPlan>())
{
EquipmentVirtualTags = vtags,
};
private static AddressSpaceComposition CompositionWithScriptedAlarms(params EquipmentScriptedAlarmPlan[] alarms) =>
new(Array.Empty<EquipmentNode>(), Array.Empty<DriverInstancePlan>(), Array.Empty<ScriptedAlarmPlan>())
{
EquipmentScriptedAlarms = alarms,
};
private static AddressSpacePlan AddedEquipmentPlan(string id) => new(
AddedEquipment: new[] { new EquipmentNode(id, id, "line-1") },
RemovedEquipment: Array.Empty<EquipmentNode>(),