From a452a20947312e933b9a01e7b943f12bf8e5a3df Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 13 Jul 2026 10:42:08 -0400 Subject: [PATCH] refactor(opcuaserver): composer parses TagConfig once per tag via TagConfigIntent (R2-11, 01/P-1) Deletes the composer's ExtractTagFullName/Alarm/Historize/Array statics and (co-located to keep the build green, per plan T5) the three OpcUaServer.Tests ExtractTag* suites whose tables now live in Commons.Tests/TagConfigIntentTests. T10 is the grep-sweep verification. --- ...tagconfig-consolidation-plan.md.tasks.json | 2 +- .../AddressSpaceComposer.cs | 137 ++------------- .../ExtractTagAlarmTests.cs | 37 ---- .../ExtractTagArrayTests.cs | 159 ------------------ .../ExtractTagHistorizeTests.cs | 35 ---- 5 files changed, 12 insertions(+), 358 deletions(-) delete mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/ExtractTagAlarmTests.cs delete mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/ExtractTagArrayTests.cs delete mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/ExtractTagHistorizeTests.cs diff --git a/archreview/plans/R2-11-tagconfig-consolidation-plan.md.tasks.json b/archreview/plans/R2-11-tagconfig-consolidation-plan.md.tasks.json index 42326c7b..8db81f17 100644 --- a/archreview/plans/R2-11-tagconfig-consolidation-plan.md.tasks.json +++ b/archreview/plans/R2-11-tagconfig-consolidation-plan.md.tasks.json @@ -6,7 +6,7 @@ { "id": "T2", "subject": "FullName-semantics characterization for EquipmentNodeWalker (Core.Tests) + DraftValidator Galaxy rule (Configuration.Tests)", "status": "completed", "blockedBy": [] }, { "id": "T3", "subject": "TagConfigIntent + TagAlarmIntent in Commons/Types (TDD; port composer semantics verbatim + OpcUaServer ExtractTag* test tables)", "status": "completed", "blockedBy": [] }, { "id": "T4", "subject": "DeviceConfigIntent (TryExtractHost/NormalizeHost) in Commons/Types (TDD)", "status": "completed", "blockedBy": [] }, - { "id": "T5", "subject": "AddressSpaceComposer swap: single TagConfigIntent.Parse per tag, delete 4 Extract* statics (P-1 parse-once)", "status": "pending", "blockedBy": ["T1", "T3"] }, + { "id": "T5", "subject": "AddressSpaceComposer swap: single TagConfigIntent.Parse per tag, delete 4 Extract* statics (P-1 parse-once)", "status": "completed", "blockedBy": ["T1", "T3"] }, { "id": "T6", "subject": "Device-host re-home: composer TryExtractDeviceHost/NormalizeDeviceHost → DeviceConfigIntent; update DriverHostActor + DeploymentArtifact callers", "status": "pending", "blockedBy": ["T4", "T5"] }, { "id": "T7", "subject": "DeploymentArtifact swap: delete 4 private mirrors, parse once per tag element via TagConfigIntent", "status": "pending", "blockedBy": ["T5"] }, { "id": "T8", "subject": "DraftValidator swap: Configuration→Commons ProjectReference; ExtractTagConfigFullName → TagConfigIntent.ExplicitFullName (null-on-absent preserved)", "status": "pending", "blockedBy": ["T2", "T3"] }, diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceComposer.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceComposer.cs index 4ef942e9..fe429e78 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceComposer.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceComposer.cs @@ -415,8 +415,11 @@ public static class AddressSpaceComposer .ThenBy(t => t.Name, StringComparer.Ordinal) .Select(t => { - var (isHistorized, historianTagname) = ExtractTagHistorize(t.TagConfig); - var (isArray, arrayLength) = ExtractTagArray(t.TagConfig); + // Parse the schemaless TagConfig blob ONCE per tag (01/P-1) via the shared byte-parity + // authority (01/C-1). TagConfigIntent.Parse is the SINGLE SOURCE OF TRUTH the + // artifact-decode seam (DeploymentArtifact), the draft gate (DraftValidator), and the + // walker all consume — so the live-compose and artifact-decode plans stay byte-equal. + var intent = TagConfigIntent.Parse(t.TagConfig); return new EquipmentTagPlan( TagId: t.TagId, EquipmentId: t.EquipmentId!, @@ -424,13 +427,13 @@ public static class AddressSpaceComposer FolderPath: t.FolderPath ?? string.Empty, Name: t.Name, DataType: t.DataType, - FullName: ExtractTagFullName(t.TagConfig), + FullName: intent.FullName, Writable: t.AccessLevel == TagAccessLevel.ReadWrite, - Alarm: ExtractTagAlarm(t.TagConfig), - IsHistorized: isHistorized, - HistorianTagname: historianTagname, - IsArray: isArray, - ArrayLength: arrayLength); + Alarm: intent.Alarm is { } a ? new EquipmentTagAlarmInfo(a.AlarmType, a.Severity, a.HistorizeToAveva) : null, + IsHistorized: intent.IsHistorized, + HistorianTagname: intent.HistorianTagname, + IsArray: intent.IsArray, + ArrayLength: intent.ArrayLength); }) .ToList(); @@ -523,33 +526,6 @@ public static class AddressSpaceComposer }; } - /// - /// Extract the driver-side full reference from a JSON blob: the - /// CK_Tag_TagConfig_IsJson constraint guarantees a JSON object, and every shipped - /// driver stores the wire-level address in a top-level FullName field. Replicated from - /// EquipmentNodeWalker.ExtractFullName because OpcUaServer does not reference the Core - /// driver assembly (kept in sync with the artifact-decode copy in DeploymentArtifact). - /// Falls back to the raw blob when it is not a JSON object with a string FullName. - /// - /// The tag's wire-level address JSON. - /// The extracted full reference, or the raw blob when no FullName is present. - private static string ExtractTagFullName(string tagConfig) - { - if (string.IsNullOrWhiteSpace(tagConfig)) return tagConfig; - try - { - using var doc = JsonDocument.Parse(tagConfig); - if (doc.RootElement.ValueKind == JsonValueKind.Object - && doc.RootElement.TryGetProperty("FullName", out var fullName) - && fullName.ValueKind == JsonValueKind.String) - { - return fullName.GetString() ?? tagConfig; - } - } - catch (JsonException) { /* fall through to raw blob */ } - return tagConfig; - } - /// /// Extract a 's connection host from its schemaless DeviceConfig JSON: /// the top-level "HostAddress" string (e.g. "10.201.31.5:8193") — the same value a @@ -593,95 +569,4 @@ public static class AddressSpaceComposer /// The normalized host (trimmed + lower-cased). public static string NormalizeDeviceHost(string host) => host.Trim().ToLowerInvariant(); - /// Parses the optional alarm object from a tag's TagConfig JSON. Returns null - /// when absent, non-object, or non-JSON (the tag is then a plain variable). Never throws. The - /// artifact-decode side (DeploymentArtifact.ExtractTagAlarm) MUST parse identically (byte-parity). - /// The tag's raw TagConfig JSON (nullable/blank tolerated). - /// The parsed native-alarm intent, or when the tag has no alarm object. - internal static EquipmentTagAlarmInfo? ExtractTagAlarm(string? tagConfig) - { - if (string.IsNullOrWhiteSpace(tagConfig)) return null; - try - { - using var doc = JsonDocument.Parse(tagConfig); - if (doc.RootElement.ValueKind != JsonValueKind.Object) return null; - if (!doc.RootElement.TryGetProperty("alarm", out var a) || a.ValueKind != JsonValueKind.Object) return null; - var type = a.TryGetProperty("alarmType", out var tEl) && tEl.ValueKind == JsonValueKind.String - ? (tEl.GetString() ?? "AlarmCondition") : "AlarmCondition"; - var sev = a.TryGetProperty("severity", out var sEl) && sEl.ValueKind == JsonValueKind.Number - && sEl.TryGetInt32(out var sv) ? sv : 500; - // historizeToAveva (bool?, absent ⇒ null ⇒ historize): only an explicit false suppresses the - // durable AVEVA write at the HistorianAdapterActor gate; a non-bool node ⇒ null (default-on). - bool? historize = a.TryGetProperty("historizeToAveva", out var hEl) - && hEl.ValueKind is JsonValueKind.True or JsonValueKind.False - ? hEl.GetBoolean() - : null; - return new EquipmentTagAlarmInfo(type, sev, historize); - } - catch (JsonException) { return null; } - } - - /// Parses the optional server-side HistoryRead intent from a tag's TagConfig JSON: - /// the isHistorized bool (absent / not a bool / non-object root / blank / malformed ⇒ - /// false) and the optional historianTagname string override (absent / not a string / - /// whitespace-or-empty ⇒ null, meaning the historian tagname defaults to the tag's FullName, - /// resolved later). The raw string value is used — not trimmed — matching ExtractTagFullName / - /// ExtractTagAlarm. Never throws. The artifact-decode side - /// (DeploymentArtifact.ExtractTagHistorize) MUST parse identically (byte-parity). - /// The tag's raw TagConfig JSON (nullable/blank tolerated). - /// The parsed historize flag and optional historian tagname override. - internal static (bool IsHistorized, string? HistorianTagname) ExtractTagHistorize(string? tagConfig) - { - if (string.IsNullOrWhiteSpace(tagConfig)) return (false, null); - try - { - using var doc = JsonDocument.Parse(tagConfig); - if (doc.RootElement.ValueKind != JsonValueKind.Object) return (false, null); - var isHistorized = doc.RootElement.TryGetProperty("isHistorized", out var hEl) - && (hEl.ValueKind == JsonValueKind.True || hEl.ValueKind == JsonValueKind.False) - && hEl.GetBoolean(); - string? tagname = null; - if (doc.RootElement.TryGetProperty("historianTagname", out var nEl) - && nEl.ValueKind == JsonValueKind.String) - { - var raw = nEl.GetString(); - if (!string.IsNullOrWhiteSpace(raw)) tagname = raw; - } - return (isHistorized, tagname); - } - catch (JsonException) { return (false, null); } - } - - /// Parses the optional array intent from a tag's TagConfig JSON: the isArray - /// bool (absent / not a bool / non-object root / blank / malformed ⇒ false) and the optional - /// arrayLength uint. The length is honoured ONLY when isArray is true AND the prop is a - /// JSON number that fits uint (else null ⇒ unbounded 1-D array, - /// ArrayDimensions=[0] at materialisation). Mirrors - /// exactly in structure + null/blank/non-object/malformed-JSON - /// tolerance. Never throws. The artifact-decode side - /// (DeploymentArtifact.ExtractTagArray) MUST parse identically (byte-parity). - /// The tag's raw TagConfig JSON (nullable/blank tolerated). - /// The parsed array flag and optional array length. - internal static (bool IsArray, uint? ArrayLength) ExtractTagArray(string? tagConfig) - { - if (string.IsNullOrWhiteSpace(tagConfig)) return (false, null); - try - { - using var doc = JsonDocument.Parse(tagConfig); - if (doc.RootElement.ValueKind != JsonValueKind.Object) return (false, null); - var isArray = doc.RootElement.TryGetProperty("isArray", out var aEl) - && (aEl.ValueKind == JsonValueKind.True || aEl.ValueKind == JsonValueKind.False) - && aEl.GetBoolean(); - uint? arrayLength = null; - if (isArray - && doc.RootElement.TryGetProperty("arrayLength", out var lEl) - && lEl.ValueKind == JsonValueKind.Number - && lEl.TryGetUInt32(out var len)) - { - arrayLength = len; - } - return (isArray, arrayLength); - } - catch (JsonException) { return (false, null); } - } } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/ExtractTagAlarmTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/ExtractTagAlarmTests.cs deleted file mode 100644 index 49dfd56a..00000000 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/ExtractTagAlarmTests.cs +++ /dev/null @@ -1,37 +0,0 @@ -using Shouldly; -using Xunit; -using ZB.MOM.WW.OtOpcUa.OpcUaServer; - -namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests; - -public class ExtractTagAlarmTests -{ - [Theory] - [InlineData("{\"FullName\":\"X.Y\"}", false, null, 0)] - [InlineData("{\"FullName\":\"X.Y\",\"alarm\":{}}", true, "AlarmCondition", 500)] - [InlineData("{\"FullName\":\"X.Y\",\"alarm\":{\"alarmType\":\"OffNormalAlarm\",\"severity\":700}}", true, "OffNormalAlarm", 700)] - [InlineData("not json", false, null, 0)] - [InlineData("{\"FullName\":\"X.Y\",\"alarm\":\"oops\"}", false, null, 0)] - public void ExtractTagAlarm_parses_or_returns_null(string cfg, bool present, string? type, int sev) - { - var info = AddressSpaceComposer.ExtractTagAlarm(cfg); - if (!present) { info.ShouldBeNull(); return; } - info!.AlarmType.ShouldBe(type); - info.Severity.ShouldBe(sev); - } - - /// historizeToAveva (bool?, absent ⇒ null ⇒ historize): an explicit true/false parses - /// through; a missing or non-bool node yields null (the HistorianAdapterActor gate then treats it as - /// default-on). Mirrors the scripted-alarm opt-out posture. - [Theory] - [InlineData("{\"alarm\":{\"alarmType\":\"LimitAlarm\",\"severity\":500}}", null)] - [InlineData("{\"alarm\":{\"alarmType\":\"LimitAlarm\",\"severity\":500,\"historizeToAveva\":true}}", true)] - [InlineData("{\"alarm\":{\"alarmType\":\"LimitAlarm\",\"severity\":500,\"historizeToAveva\":false}}", false)] - [InlineData("{\"alarm\":{\"alarmType\":\"LimitAlarm\",\"severity\":500,\"historizeToAveva\":\"oops\"}}", null)] - public void ExtractTagAlarm_parses_historizeToAveva(string cfg, bool? expected) - { - var info = AddressSpaceComposer.ExtractTagAlarm(cfg); - info.ShouldNotBeNull(); - info!.HistorizeToAveva.ShouldBe(expected); - } -} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/ExtractTagArrayTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/ExtractTagArrayTests.cs deleted file mode 100644 index 2855eb34..00000000 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/ExtractTagArrayTests.cs +++ /dev/null @@ -1,159 +0,0 @@ -using Shouldly; -using Xunit; -using ZB.MOM.WW.OtOpcUa.Configuration.Entities; -using ZB.MOM.WW.OtOpcUa.Configuration.Enums; -using ZB.MOM.WW.OtOpcUa.OpcUaServer; - -namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests; - -/// -/// Verifies parses the optional array intent from a -/// tag's TagConfig JSON exactly as parses -/// the historize intent: the isArray bool (absent / not a bool / non-object root / blank / -/// malformed ⇒ false) and the optional arrayLength uint (only honoured when -/// isArray is true AND the prop is a JSON number that fits uint; else null). Never -/// throws. Also pins the end-to-end thread-through onto -/// / . -/// -public class ExtractTagArrayTests -{ - [Theory] - // isArray true with an explicit arrayLength. - [InlineData("{\"FullName\":\"T.A\",\"isArray\":true,\"arrayLength\":16}", true, (uint)16)] - // isArray true, no arrayLength ⇒ length null. - [InlineData("{\"FullName\":\"T.A\",\"isArray\":true}", true, null)] - // Absent isArray ⇒ false (arrayLength ignored even if present). - [InlineData("{\"FullName\":\"T.A\"}", false, null)] - // arrayLength present but isArray false ⇒ length null (only honoured when the flag is true). - [InlineData("{\"FullName\":\"T.A\",\"isArray\":false,\"arrayLength\":16}", false, null)] - // arrayLength absent-with-flag honoured-as-null when isArray true but no length. - [InlineData("{\"FullName\":\"T.A\",\"isArray\":true,\"arrayLength\":0}", true, (uint)0)] - // null / empty / malformed-JSON / array-root ⇒ (false, null), never throws. - [InlineData(null, false, null)] - [InlineData("", false, null)] - [InlineData("not json {", false, null)] - [InlineData("[1,2]", false, null)] - // Wrong type for isArray (string, not bool) ⇒ false. - [InlineData("{\"isArray\":\"yes\"}", false, null)] - // Wrong type for arrayLength (string, not number) ⇒ length null, flag still honoured. - [InlineData("{\"isArray\":true,\"arrayLength\":\"16\"}", true, null)] - // Negative arrayLength does not fit uint ⇒ length null, flag still honoured. - [InlineData("{\"isArray\":true,\"arrayLength\":-1}", true, null)] - // Float arrayLength (16.5) is not an exact uint ⇒ TryGetUInt32 rejects it ⇒ length null. - [InlineData("{\"isArray\":true,\"arrayLength\":16.5}", true, null)] - // Overflow arrayLength (uint.MaxValue + 1 = 4294967296) does not fit uint ⇒ length null. - [InlineData("{\"isArray\":true,\"arrayLength\":4294967296}", true, null)] - public void ExtractTagArray_parses_or_returns_defaults(string? cfg, bool expectedIsArray, uint? expectedLength) - { - var (isArray, arrayLength) = AddressSpaceComposer.ExtractTagArray(cfg); - isArray.ShouldBe(expectedIsArray); - arrayLength.ShouldBe(expectedLength); - } - - /// End-to-end: an equipment tag whose TagConfig carries isArray/arrayLength - /// surfaces those on its through , - /// exactly as the historize keys thread through. - [Fact] - public void Compose_threads_array_keys_onto_equipment_tag_plan() - { - var ns = new Namespace - { - NamespaceId = "ns-eq", - ClusterId = "c1", - Kind = NamespaceKind.Equipment, - NamespaceUri = "urn:eq", - }; - var driver = new DriverInstance - { - DriverInstanceId = "drv-1", - ClusterId = "c1", - NamespaceId = "ns-eq", - Name = "Modbus1", - DriverType = "Modbus", - DriverConfig = "{}", - }; - var area = new UnsArea { UnsAreaId = "area-1", ClusterId = "c1", Name = "filling" }; - var line = new UnsLine { UnsLineId = "line-1", UnsAreaId = "area-1", Name = "line-1" }; - var equip = new Equipment - { - EquipmentId = "eq-1", - DriverInstanceId = "drv-1", - UnsLineId = "line-1", - Name = "Machine_001", - MachineCode = "MACHINE_001", - }; - var arrayTag = new Tag - { - TagId = "tag-arr", - DriverInstanceId = "drv-1", - EquipmentId = "eq-1", - FolderPath = null, - Name = "Buffer", - DataType = "Int16", - AccessLevel = TagAccessLevel.Read, - TagConfig = "{\"FullName\":\"40001\",\"isArray\":true,\"arrayLength\":16}", - }; - - var result = AddressSpaceComposer.Compose( - new[] { area }, new[] { line }, new[] { equip }, - new[] { driver }, Array.Empty(), - new[] { arrayTag }, new[] { ns }); - - var tag = result.EquipmentTags.ShouldHaveSingleItem(); - tag.TagId.ShouldBe("tag-arr"); - tag.IsArray.ShouldBeTrue(); - tag.ArrayLength.ShouldBe((uint)16); - } - - /// End-to-end: a scalar equipment tag (no array keys) yields IsArray=false, ArrayLength=null. - [Fact] - public void Compose_leaves_scalar_equipment_tag_plan_unflagged() - { - var ns = new Namespace - { - NamespaceId = "ns-eq", - ClusterId = "c1", - Kind = NamespaceKind.Equipment, - NamespaceUri = "urn:eq", - }; - var driver = new DriverInstance - { - DriverInstanceId = "drv-1", - ClusterId = "c1", - NamespaceId = "ns-eq", - Name = "Modbus1", - DriverType = "Modbus", - DriverConfig = "{}", - }; - var area = new UnsArea { UnsAreaId = "area-1", ClusterId = "c1", Name = "filling" }; - var line = new UnsLine { UnsLineId = "line-1", UnsAreaId = "area-1", Name = "line-1" }; - var equip = new Equipment - { - EquipmentId = "eq-1", - DriverInstanceId = "drv-1", - UnsLineId = "line-1", - Name = "Machine_001", - MachineCode = "MACHINE_001", - }; - var scalarTag = new Tag - { - TagId = "tag-scalar", - DriverInstanceId = "drv-1", - EquipmentId = "eq-1", - FolderPath = null, - Name = "Speed", - DataType = "Float", - AccessLevel = TagAccessLevel.Read, - TagConfig = "{\"FullName\":\"40005\"}", - }; - - var result = AddressSpaceComposer.Compose( - new[] { area }, new[] { line }, new[] { equip }, - new[] { driver }, Array.Empty(), - new[] { scalarTag }, new[] { ns }); - - var tag = result.EquipmentTags.ShouldHaveSingleItem(); - tag.IsArray.ShouldBeFalse(); - tag.ArrayLength.ShouldBeNull(); - } -} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/ExtractTagHistorizeTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/ExtractTagHistorizeTests.cs deleted file mode 100644 index a48ce961..00000000 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/ExtractTagHistorizeTests.cs +++ /dev/null @@ -1,35 +0,0 @@ -using Shouldly; -using Xunit; -using ZB.MOM.WW.OtOpcUa.OpcUaServer; - -namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests; - -public class ExtractTagHistorizeTests -{ - [Theory] - // isHistorized true, no explicit tagname ⇒ tagname null (defaults to FullName later). - [InlineData("{\"FullName\":\"T.A\",\"isHistorized\":true}", true, null)] - // isHistorized true with an explicit historian-tagname override. - [InlineData("{\"FullName\":\"T.A\",\"isHistorized\":true,\"historianTagname\":\"WW.Tag\"}", true, "WW.Tag")] - // Absent isHistorized ⇒ false. - [InlineData("{\"FullName\":\"T.A\"}", false, null)] - // historianTagname parses independently of the flag. - [InlineData("{\"FullName\":\"T.A\",\"isHistorized\":false,\"historianTagname\":\"WW.Tag\"}", false, "WW.Tag")] - // Blank/whitespace tagname ⇒ null. - [InlineData("{\"FullName\":\"T.A\",\"isHistorized\":true,\"historianTagname\":\" \"}", true, null)] - // null / empty / malformed-JSON / array-root ⇒ (false, null), never throws. - [InlineData(null, false, null)] - [InlineData("", false, null)] - [InlineData("not json {", false, null)] - [InlineData("[1,2]", false, null)] - // Wrong type for isHistorized (string, not bool) ⇒ false. - [InlineData("{\"isHistorized\":\"yes\"}", false, null)] - // Wrong type for historianTagname (number, not string) ⇒ tagname null, flag still honoured. - [InlineData("{\"isHistorized\":true,\"historianTagname\":123}", true, null)] - public void ExtractTagHistorize_parses_or_returns_defaults(string? cfg, bool expectedHistorized, string? expectedTagname) - { - var (isHistorized, historianTagname) = AddressSpaceComposer.ExtractTagHistorize(cfg); - isHistorized.ShouldBe(expectedHistorized); - historianTagname.ShouldBe(expectedTagname); - } -}