From e80e34798a0770d6ef98ea89b095e6a6e265e48a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Otto=20Boly=C3=B3s?= Date: Tue, 18 Aug 2026 23:32:25 +0200 Subject: [PATCH 01/34] chore(common): add trailing newline to DeviceValidationLevel.cs --- libraries/MTConnect.NET-Common/Agents/DeviceValidationLevel.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/MTConnect.NET-Common/Agents/DeviceValidationLevel.cs b/libraries/MTConnect.NET-Common/Agents/DeviceValidationLevel.cs index a3aa86125..506d43a30 100644 --- a/libraries/MTConnect.NET-Common/Agents/DeviceValidationLevel.cs +++ b/libraries/MTConnect.NET-Common/Agents/DeviceValidationLevel.cs @@ -29,4 +29,4 @@ public enum DeviceValidationLevel /// Strict } -} \ No newline at end of file +} From 1b6f85fbf3f039f4cdb7bdfa9e84c3d58a7821f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Otto=20Boly=C3=B3s?= Date: Tue, 18 Aug 2026 23:37:20 +0200 Subject: [PATCH 02/34] fix(common): mirror InputValidationLevel onto DeviceValidationLevel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before the 2026-06 validation-level split, a single InputValidationLevel knob gated both Observation/Asset validation and Device-tree validation. The split left every consumer who only set inputValidationLevel silently downgraded on the Device side — the ctor defaults DeviceValidationLevel to Warning, so a config that asked for Strict input validation lost Strict device-tree validation on the load path. Add a load-time migration bridge: a private explicit-set flag on AgentConfiguration tracks whether the caller (or the deserializer) touched DeviceValidationLevel; when the flag is clear at Normalize() time, the ctor mirrors InputValidationLevel onto DeviceValidationLevel (both enums share ordinals 0–3). Every Read* / ReadJson* / ReadYaml* overload now invokes Normalize() before returning. Both DeviceValidationLevel and InputValidationLevel setters now reject values that are not defined enum arms via Enum.IsDefined, throwing ArgumentOutOfRangeException — prevents an unbounded integer in a JSON/YAML source or a misdirected cast from silently landing an out-of-range ordinal that downstream branch-on-arm code would mishandle. Tests: DeviceValidationLevelMigrationTests pins the mirror across every enum arm plus the explicit-set precedence and the setter guards (13 cases total). --- .../Configurations/AgentConfiguration.cs | 85 +++++++- .../DeviceValidationLevelMigrationTests.cs | 200 ++++++++++++++++++ 2 files changed, 279 insertions(+), 6 deletions(-) create mode 100644 tests/MTConnect.NET-Common-Tests/Configurations/DeviceValidationLevelMigrationTests.cs diff --git a/libraries/MTConnect.NET-Common/Configurations/AgentConfiguration.cs b/libraries/MTConnect.NET-Common/Configurations/AgentConfiguration.cs index fd873c6c7..fa69b583d 100644 --- a/libraries/MTConnect.NET-Common/Configurations/AgentConfiguration.cs +++ b/libraries/MTConnect.NET-Common/Configurations/AgentConfiguration.cs @@ -127,17 +127,62 @@ public string DefaultVersionValue [JsonPropertyName("enableValidation")] public bool EnableValidation { get; set; } + private DeviceValidationLevel _deviceValidationLevel; + private bool _isDeviceValidationLevelExplicit; + private InputValidationLevel _inputValidationLevel; + /// - /// Gets or Sets the default Device (MTConnectDevices) validation level. 0 = Ignore, 1 = Warning, 2 = Remove, 3 = Strict + /// Gets or Sets the default Device (MTConnectDevices) validation level. 0 = Ignore, 1 = Warning, 2 = Remove, 3 = Strict. /// + /// + /// When a configuration file omits this key the loader mirrors + /// onto Device validation, preserving pre-v7 behaviour for consumers that only knew the single + /// knob. Setting this property — either programmatically or via a + /// key present in the source document — marks the value as explicit and disables the mirror on the + /// next . An assignment whose ordinal is not a defined enum arm raises + /// . + /// [JsonPropertyName("deviceValidationLevel")] - public DeviceValidationLevel DeviceValidationLevel { get; set; } + public DeviceValidationLevel DeviceValidationLevel + { + get => _deviceValidationLevel; + set + { + if (!Enum.IsDefined(typeof(DeviceValidationLevel), value)) + { + throw new ArgumentOutOfRangeException( + nameof(value), + value, + "DeviceValidationLevel must be one of Ignore (0), Warning (1), Remove (2), Strict (3)."); + } + _deviceValidationLevel = value; + _isDeviceValidationLevelExplicit = true; + } + } /// - /// Gets or Sets the default Input (Observation or Asset) validation level. 0 = Ignore, 1 = Warning, 2 = Remove, 3 = Strict + /// Gets or Sets the default Input (Observation or Asset) validation level. 0 = Ignore, 1 = Warning, 2 = Remove, 3 = Strict. /// + /// + /// An assignment whose ordinal is not a defined enum arm raises + /// . + /// [JsonPropertyName("inputValidationLevel")] - public InputValidationLevel InputValidationLevel { get; set; } + public InputValidationLevel InputValidationLevel + { + get => _inputValidationLevel; + set + { + if (!Enum.IsDefined(typeof(InputValidationLevel), value)) + { + throw new ArgumentOutOfRangeException( + nameof(value), + value, + "InputValidationLevel must be one of Ignore (0), Warning (1), Remove (2), Strict (3)."); + } + _inputValidationLevel = value; + } + } /// /// Gets or Sets whether an empty, null, or whitespace-only Result is preserved for Event DataItems @@ -172,8 +217,11 @@ public AgentConfiguration() ObservationBufferSize = 131072; AssetBufferSize = 1024; DefaultVersion = MTConnectVersions.Max; - DeviceValidationLevel = DeviceValidationLevel.Warning; - InputValidationLevel = InputValidationLevel.Warning; + // Assign the backing fields directly. Going through the public setter would flip + // _isDeviceValidationLevelExplicit and disable the load-time migration mirror. + _deviceValidationLevel = DeviceValidationLevel.Warning; + _inputValidationLevel = InputValidationLevel.Warning; + _isDeviceValidationLevelExplicit = false; AllowEmptyResultForEnumEvents = false; ConvertUnits = true; IgnoreObservationCase = false; @@ -181,6 +229,27 @@ public AgentConfiguration() EnableMetrics = true; } + /// + /// Applies post-deserialisation defaults that depend on cross-property state. + /// When the source configuration omitted , mirror + /// onto it. Both enums share ordinals 0-3, so the mirror is a + /// direct cast. + /// + /// + /// Invoked by every / / + /// path so consumers who only set + /// in their configuration observe the same Device-validation + /// behaviour they got before the split. Callers loading a configuration programmatically may invoke + /// once construction is complete to pick up the same default. + /// + public void Normalize() + { + if (!_isDeviceValidationLevelExplicit) + { + _deviceValidationLevel = (DeviceValidationLevel)(int)_inputValidationLevel; + } + } + /// /// Loads an , auto-detecting JSON or YAML; see for the resolution rules. @@ -284,6 +353,7 @@ public static T ReadJson(string path = null) where T : AgentConfiguration var configuration = JsonSerializer.Deserialize(text, options); configuration.Path = configurationPath; + configuration.Normalize(); return configuration; } } @@ -324,6 +394,7 @@ public static AgentConfiguration ReadJson(Type type, string path = null) var configuration = (AgentConfiguration)JsonSerializer.Deserialize(text, type, options); configuration.Path = configurationPath; + configuration.Normalize(); return configuration; } } @@ -365,6 +436,7 @@ public static T ReadYaml(string path = null) where T : AgentConfiguration var configuration = deserializer.Deserialize(text); configuration.Path = configurationPath; + configuration.Normalize(); return configuration; } } @@ -405,6 +477,7 @@ public static AgentConfiguration ReadYaml(Type type, string path = null) var configuration = (AgentConfiguration)deserializer.Deserialize(text, type); configuration.Path = configurationPath; + configuration.Normalize(); return configuration; } } diff --git a/tests/MTConnect.NET-Common-Tests/Configurations/DeviceValidationLevelMigrationTests.cs b/tests/MTConnect.NET-Common-Tests/Configurations/DeviceValidationLevelMigrationTests.cs new file mode 100644 index 000000000..006bd299f --- /dev/null +++ b/tests/MTConnect.NET-Common-Tests/Configurations/DeviceValidationLevelMigrationTests.cs @@ -0,0 +1,200 @@ +// Copyright (c) 2026 TrakHound Inc., All Rights Reserved. +// TrakHound Inc. licenses this file to you under the MIT license. + +using System; +using System.IO; +using MTConnect.Agents; +using MTConnect.Configurations; +using NUnit.Framework; + +namespace MTConnect.Tests.Common.Configurations +{ + /// + /// Pins the load-time migration bridge that mirrors + /// onto when the source configuration omits the + /// deviceValidationLevel key. + /// + /// Motivation: pre-split, a single knob gated both Observation/Asset + /// validation and Device-tree validation. The split introduced by PR #218 leaves consumers who only set + /// inputValidationLevel silently downgraded on the Device-tree side. The migration bridge + /// preserves the pre-split expectation. + /// + /// Also pins the two setter guards from the same PR: an out-of-range integer on either enum property + /// raises . + /// + [TestFixture] + [Category("DeviceValidationLevel")] + public class DeviceValidationLevelMigrationTests + { + // --------------------------------------------------------------- + // JSON load path: implicit InputValidationLevel mirror + // --------------------------------------------------------------- + + /// Pins that a JSON config with only inputValidationLevel: 3 (Strict) loads with . + [Test] + public void ReadJson_InputValidationLevel_Strict_Only_Mirrors_Onto_DeviceValidationLevel() + { + // The shipped JsonSerializerOptions treat enums as their integer ordinals — no + // JsonStringEnumConverter registered — so the fixture wires the ordinal directly. + var path = WriteTempJson("{\"inputValidationLevel\":3}"); + try + { + var config = AgentConfiguration.ReadJson(path); + + Assert.That(config, Is.Not.Null, "the loader must not have swallowed the config"); + Assert.That(config!.InputValidationLevel, Is.EqualTo(InputValidationLevel.Strict)); + Assert.That(config.DeviceValidationLevel, Is.EqualTo(DeviceValidationLevel.Strict), + "when the JSON omits deviceValidationLevel, the loader must mirror InputValidationLevel " + + "so pre-split consumers keep getting Strict Device-tree validation"); + } + finally + { + File.Delete(path); + } + } + + /// Pins that an explicit deviceValidationLevel beats the mirror even when both are present in JSON. + [Test] + public void ReadJson_Explicit_DeviceValidationLevel_Beats_Mirror() + { + // Deliberately set the two knobs to different arms so the mirror + // path would corrupt DeviceValidationLevel if it fired anyway. + var path = WriteTempJson( + "{\"inputValidationLevel\":3,\"deviceValidationLevel\":0}"); + try + { + var config = AgentConfiguration.ReadJson(path); + + Assert.That(config, Is.Not.Null); + Assert.That(config!.InputValidationLevel, Is.EqualTo(InputValidationLevel.Strict)); + Assert.That(config.DeviceValidationLevel, Is.EqualTo(DeviceValidationLevel.Ignore), + "an explicit deviceValidationLevel key must NOT be silently overwritten by the mirror"); + } + finally + { + File.Delete(path); + } + } + + /// Pins that a JSON config with neither knob set still loads with both defaulted to Warning. + [Test] + public void ReadJson_Empty_Object_Loads_With_Warning_Defaults() + { + var path = WriteTempJson("{}"); + try + { + var config = AgentConfiguration.ReadJson(path); + + Assert.That(config, Is.Not.Null); + Assert.That(config!.InputValidationLevel, Is.EqualTo(InputValidationLevel.Warning)); + Assert.That(config.DeviceValidationLevel, Is.EqualTo(DeviceValidationLevel.Warning)); + } + finally + { + File.Delete(path); + } + } + + /// Pins that the mirror covers every enum arm, not just Strict. + [TestCase(InputValidationLevel.Ignore, DeviceValidationLevel.Ignore)] + [TestCase(InputValidationLevel.Warning, DeviceValidationLevel.Warning)] + [TestCase(InputValidationLevel.Remove, DeviceValidationLevel.Remove)] + [TestCase(InputValidationLevel.Strict, DeviceValidationLevel.Strict)] + public void ReadJson_InputValidationLevel_Only_Mirrors_All_Arms( + InputValidationLevel input, + DeviceValidationLevel expectedMirrored) + { + var path = WriteTempJson($"{{\"inputValidationLevel\":{(int)input}}}"); + try + { + var config = AgentConfiguration.ReadJson(path); + + Assert.That(config, Is.Not.Null); + Assert.That(config!.DeviceValidationLevel, Is.EqualTo(expectedMirrored)); + } + finally + { + File.Delete(path); + } + } + + // --------------------------------------------------------------- + // Programmatic Normalize() + // --------------------------------------------------------------- + + /// Pins that a caller who builds an in code and calls gets the same mirror. + [Test] + public void Normalize_Mirrors_InputValidationLevel_When_DeviceValidationLevel_Not_Explicit() + { + var config = new AgentConfiguration(); + config.InputValidationLevel = InputValidationLevel.Remove; + + // Precondition: the ctor default is Warning. Verify the assignment above did NOT touch + // DeviceValidationLevel — that is the whole point of the flag. + Assert.That(config.DeviceValidationLevel, Is.EqualTo(DeviceValidationLevel.Warning)); + + config.Normalize(); + + Assert.That(config.DeviceValidationLevel, Is.EqualTo(DeviceValidationLevel.Remove)); + } + + /// Pins that an explicit assignment to disables the mirror on subsequent calls. + [Test] + public void Normalize_Skips_Mirror_When_DeviceValidationLevel_Explicitly_Set() + { + var config = new AgentConfiguration(); + config.InputValidationLevel = InputValidationLevel.Strict; + config.DeviceValidationLevel = DeviceValidationLevel.Ignore; + + config.Normalize(); + + Assert.That(config.DeviceValidationLevel, Is.EqualTo(DeviceValidationLevel.Ignore), + "an explicit DeviceValidationLevel assignment must sticky-suppress the mirror"); + } + + // --------------------------------------------------------------- + // Setter validation (F-SEC-003) + // --------------------------------------------------------------- + + /// Pins that an out-of-range integer cast to is rejected at the setter. + [Test] + public void DeviceValidationLevel_Setter_Rejects_Undefined_Enum_Value() + { + var config = new AgentConfiguration(); + Assert.Throws( + () => config.DeviceValidationLevel = (DeviceValidationLevel)42); + } + + /// Pins that an out-of-range integer cast to is rejected at the setter. + [Test] + public void InputValidationLevel_Setter_Rejects_Undefined_Enum_Value() + { + var config = new AgentConfiguration(); + Assert.Throws( + () => config.InputValidationLevel = (InputValidationLevel)99); + } + + /// Pins that every defined enum arm is accepted (no false positives from the guard). + [TestCase(DeviceValidationLevel.Ignore)] + [TestCase(DeviceValidationLevel.Warning)] + [TestCase(DeviceValidationLevel.Remove)] + [TestCase(DeviceValidationLevel.Strict)] + public void DeviceValidationLevel_Setter_Accepts_Every_Defined_Arm(DeviceValidationLevel arm) + { + var config = new AgentConfiguration(); + Assert.DoesNotThrow(() => config.DeviceValidationLevel = arm); + Assert.That(config.DeviceValidationLevel, Is.EqualTo(arm)); + } + + // --------------------------------------------------------------- + // Fixture harness + // --------------------------------------------------------------- + + private static string WriteTempJson(string json) + { + var path = Path.Combine(Path.GetTempPath(), $"agent-config-{Guid.NewGuid():N}.json"); + File.WriteAllText(path, json); + return path; + } + } +} From bad923f1275a48cb881d15bfa216dfe9882ee49c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tukusej=E2=80=99s=20Sirs?= Date: Tue, 18 Aug 2026 23:12:26 +0200 Subject: [PATCH 03/34] test(common): pin DeviceValidationLevel enum arms for NormalizeDevice MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 2026-06 split of Device validation off InputValidationLevel adds a new four-arm enum (Ignore/Warning/Remove/Strict) consumed by NormalizeDevice at three sites (generic Component, generic Composition, generic DataItem). The pre-existing suite exercised these three sites via a SINGLE combined path against InputValidationLevel; the new DeviceValidationLevel arms had no dedicated coverage. This fixture pins: - All 12 enum-arm x site combinations end-to-end via AddDevice. - The subscriber tuple payload (deviceUuid, entity, ValidationResult) for the InvalidComponentAdded raise site. - The AgentConfiguration default (Warning) and the enum ordinal/name grid. - The InputValidationLevel/DeviceValidationLevel independence invariant (regression pin for the pre-split behavior where the two knobs shared state). The DataItem-arm test attaches the generic DataItem to a known Axes Component so Device.RemoveDataItem — which iterates Components — has an addressable removal target; a follow-up finding tracks the pre-existing gap that Device.RemoveDataItem never touches Device.DataItems (out of scope for this PR). --- ...viceValidationLevelNormalizeDeviceTests.cs | 440 ++++++++++++++++++ 1 file changed, 440 insertions(+) create mode 100644 tests/MTConnect.NET-Common-Tests/Agents/DeviceValidationLevelNormalizeDeviceTests.cs diff --git a/tests/MTConnect.NET-Common-Tests/Agents/DeviceValidationLevelNormalizeDeviceTests.cs b/tests/MTConnect.NET-Common-Tests/Agents/DeviceValidationLevelNormalizeDeviceTests.cs new file mode 100644 index 000000000..181201e8e --- /dev/null +++ b/tests/MTConnect.NET-Common-Tests/Agents/DeviceValidationLevelNormalizeDeviceTests.cs @@ -0,0 +1,440 @@ +// Copyright (c) 2026 TrakHound Inc., All Rights Reserved. +// TrakHound Inc. licenses this file to you under the MIT license. + +using System; +using System.Collections.Generic; +using System.Linq; +using MTConnect.Agents; +using MTConnect.Configurations; +using MTConnect.Devices; +using MTConnect.Devices.Components; +using MTConnect.Devices.DataItems; +using NUnit.Framework; + +namespace MTConnect.Tests.Common.Agents +{ + /// + /// Enum-arm coverage FLOOR for as consumed + /// by MTConnectAgent.NormalizeDevice. The 2026-06 PR that split Device + /// validation off introduced 12 branch + /// combinations that all executed against ONE test path (via + /// ) — see + /// . This fixture pins every + /// arm of the four-value enum against each of the three generic-entity + /// validation sites so a regression that widens, narrows, or reorders the + /// enum is caught the moment the delta lands. + /// + /// Grid: 4 arms (Ignore / Warning / Remove / Strict) x 3 sites (generic + /// Component / generic Composition / generic DataItem) = 12 base cases, + /// plus the default-configuration invariant and the enum-arms-are-what-we- + /// think-they-are guard. + /// + /// A "generic" entity is one whose runtime CLR type is the raw + /// / / + /// base class rather than a concrete subclass — this is what the + /// NormalizeDevice validation loop identifies via + /// o.GetType() == typeof(Component) as "invalid type not found". + /// Setting Type to a string the registry does not recognise causes + /// the corresponding Create factory to fall back to the raw base class. + /// + [TestFixture] + [Category("DeviceValidationLevel")] + public class DeviceValidationLevelNormalizeDeviceTests + { + private const string DeviceUuid = "device-validation-level-device"; + private const string DeviceName = "DeviceValidationLevel"; + private const string DeviceId = "device-validation-level-device-id"; + + // Unrecognised Type strings force each Create factory (Component / + // Composition / DataItem) into its base-class fallback, which is + // exactly the "generic" entity NormalizeDevice validates against. + private const string UnknownComponentType = "ThisComponentTypeIsNotRegistered"; + private const string UnknownCompositionType = "ThisCompositionTypeIsNotRegistered"; + private const string UnknownDataItemType = "THIS_DATAITEM_TYPE_IS_NOT_REGISTERED"; + + private const string ChildComponentId = "child-generic-component"; + private const string ChildCompositionId = "child-generic-composition"; + private const string ChildDataItemId = "child-generic-dataitem"; + + // --------------------------------------------------------------- + // Enum-shape guard: pin the arms + ordinals so a rename or reorder + // fails loudly rather than silently shifting the arms consumed by + // NormalizeDevice's `> Ignore`, `== Remove`, `== Strict` predicates. + // --------------------------------------------------------------- + + /// Pins the four arms of at their exact ordinals: Ignore=0, Warning=1, Remove=2, Strict=3. + [Test] + public void DeviceValidationLevel_arms_and_ordinals_are_stable() + { + var arms = Enum.GetValues(typeof(DeviceValidationLevel)) + .Cast() + .OrderBy(v => (int)v) + .ToArray(); + + Assert.That(arms, Is.EqualTo(new[] + { + DeviceValidationLevel.Ignore, + DeviceValidationLevel.Warning, + DeviceValidationLevel.Remove, + DeviceValidationLevel.Strict, + })); + Assert.That((int)DeviceValidationLevel.Ignore, Is.EqualTo(0)); + Assert.That((int)DeviceValidationLevel.Warning, Is.EqualTo(1)); + Assert.That((int)DeviceValidationLevel.Remove, Is.EqualTo(2)); + Assert.That((int)DeviceValidationLevel.Strict, Is.EqualTo(3)); + } + + /// Pins the AgentConfiguration default: is . + [Test] + public void AgentConfiguration_default_DeviceValidationLevel_is_Warning() + { + var config = new AgentConfiguration(); + + Assert.That(config.DeviceValidationLevel, Is.EqualTo(DeviceValidationLevel.Warning), + "Warning is the spec-safe default: the invalid entity survives but " + + "a subscriber is notified. Any change to this default is a behaviour break."); + } + + // --------------------------------------------------------------- + // Generic Component site — 4 arms + // --------------------------------------------------------------- + + /// Ignore: no InvalidComponentAdded event; the generic Component is retained on the device. + [Test] + public void NormalizeDevice_GenericComponent_Under_Ignore_Retains_Component_And_Suppresses_Event() + { + var raised = new List(); + using var agent = NewAgent(DeviceValidationLevel.Ignore); + agent.InvalidComponentAdded += (_, c, _) => raised.Add(c.Id); + + var device = DeviceWithGenericComponent(); + + var added = agent.AddDevice(device); + + Assert.That(added, Is.Not.Null, "Ignore never invalidates the device"); + Assert.That(added.Components.Any(c => c.Id == ChildComponentId), Is.True, + "Ignore never removes the generic Component"); + Assert.That(raised, Is.Empty, + "Ignore short-circuits before the InvalidComponentAdded raise site"); + } + + /// Warning: InvalidComponentAdded fires; the generic Component is retained on the device. + [Test] + public void NormalizeDevice_GenericComponent_Under_Warning_Retains_Component_And_Raises_Event() + { + var raised = new List(); + using var agent = NewAgent(DeviceValidationLevel.Warning); + agent.InvalidComponentAdded += (_, c, _) => raised.Add(c.Id); + + var device = DeviceWithGenericComponent(); + + var added = agent.AddDevice(device); + + Assert.That(added, Is.Not.Null); + Assert.That(added.Components.Any(c => c.Id == ChildComponentId), Is.True, + "Warning notifies but does not mutate the device"); + Assert.That(raised, Is.EqualTo(new[] { ChildComponentId })); + } + + /// Remove: InvalidComponentAdded fires; the generic Component is removed from the device. + [Test] + public void NormalizeDevice_GenericComponent_Under_Remove_Drops_Component_And_Raises_Event() + { + var raised = new List(); + using var agent = NewAgent(DeviceValidationLevel.Remove); + agent.InvalidComponentAdded += (_, c, _) => raised.Add(c.Id); + + var device = DeviceWithGenericComponent(); + + var added = agent.AddDevice(device); + + Assert.That(added, Is.Not.Null, "Remove keeps the device; only the invalid child is dropped"); + Assert.That(added.Components.Any(c => c.Id == ChildComponentId), Is.False, + "Remove must drop the generic Component from the normalized device"); + Assert.That(raised, Is.EqualTo(new[] { ChildComponentId })); + } + + /// Strict: InvalidComponentAdded fires; NormalizeDevice returns null so AddDevice rejects the entire device. + [Test] + public void NormalizeDevice_GenericComponent_Under_Strict_Rejects_Device_And_Raises_Event() + { + var raised = new List(); + using var agent = NewAgent(DeviceValidationLevel.Strict); + agent.InvalidComponentAdded += (_, c, _) => raised.Add(c.Id); + + var device = DeviceWithGenericComponent(); + + var added = agent.AddDevice(device); + + Assert.That(added, Is.Null, + "Strict must reject the entire device on the first invalid Component"); + Assert.That(raised, Is.EqualTo(new[] { ChildComponentId }), + "Strict raises the event before returning null so subscribers can log"); + } + + // --------------------------------------------------------------- + // Generic Composition site — 4 arms + // --------------------------------------------------------------- + + /// Ignore: no InvalidCompositionAdded event; the generic Composition is retained on the device. + [Test] + public void NormalizeDevice_GenericComposition_Under_Ignore_Retains_Composition_And_Suppresses_Event() + { + var raised = new List(); + using var agent = NewAgent(DeviceValidationLevel.Ignore); + agent.InvalidCompositionAdded += (_, c, _) => raised.Add(c.Id); + + var device = DeviceWithGenericComposition(); + + var added = agent.AddDevice(device); + + Assert.That(added, Is.Not.Null); + Assert.That(added.Compositions.Any(c => c.Id == ChildCompositionId), Is.True); + Assert.That(raised, Is.Empty); + } + + /// Warning: InvalidCompositionAdded fires; the generic Composition is retained. + [Test] + public void NormalizeDevice_GenericComposition_Under_Warning_Retains_Composition_And_Raises_Event() + { + var raised = new List(); + using var agent = NewAgent(DeviceValidationLevel.Warning); + agent.InvalidCompositionAdded += (_, c, _) => raised.Add(c.Id); + + var device = DeviceWithGenericComposition(); + + var added = agent.AddDevice(device); + + Assert.That(added, Is.Not.Null); + Assert.That(added.Compositions.Any(c => c.Id == ChildCompositionId), Is.True); + Assert.That(raised, Is.EqualTo(new[] { ChildCompositionId })); + } + + /// Remove: InvalidCompositionAdded fires; the generic Composition is removed from the device. + [Test] + public void NormalizeDevice_GenericComposition_Under_Remove_Drops_Composition_And_Raises_Event() + { + var raised = new List(); + using var agent = NewAgent(DeviceValidationLevel.Remove); + agent.InvalidCompositionAdded += (_, c, _) => raised.Add(c.Id); + + var device = DeviceWithGenericComposition(); + + var added = agent.AddDevice(device); + + Assert.That(added, Is.Not.Null); + Assert.That(added.Compositions.Any(c => c.Id == ChildCompositionId), Is.False); + Assert.That(raised, Is.EqualTo(new[] { ChildCompositionId })); + } + + /// Strict: InvalidCompositionAdded fires; NormalizeDevice returns null. + [Test] + public void NormalizeDevice_GenericComposition_Under_Strict_Rejects_Device_And_Raises_Event() + { + var raised = new List(); + using var agent = NewAgent(DeviceValidationLevel.Strict); + agent.InvalidCompositionAdded += (_, c, _) => raised.Add(c.Id); + + var device = DeviceWithGenericComposition(); + + var added = agent.AddDevice(device); + + Assert.That(added, Is.Null); + Assert.That(raised, Is.EqualTo(new[] { ChildCompositionId })); + } + + // --------------------------------------------------------------- + // Generic DataItem site — 4 arms + // --------------------------------------------------------------- + + /// Ignore: no InvalidDataItemAdded event; the generic DataItem is retained on the device. + [Test] + public void NormalizeDevice_GenericDataItem_Under_Ignore_Retains_DataItem_And_Suppresses_Event() + { + var raised = new List(); + using var agent = NewAgent(DeviceValidationLevel.Ignore); + agent.InvalidDataItemAdded += (_, d, _) => raised.Add(d.Id); + + var device = DeviceWithGenericDataItem(); + + var added = agent.AddDevice(device); + + Assert.That(added, Is.Not.Null); + Assert.That(added.GetDataItems().Any(d => d.Id == ChildDataItemId), Is.True); + Assert.That(raised, Is.Empty); + } + + /// Warning: InvalidDataItemAdded fires; the generic DataItem is retained. + [Test] + public void NormalizeDevice_GenericDataItem_Under_Warning_Retains_DataItem_And_Raises_Event() + { + var raised = new List(); + using var agent = NewAgent(DeviceValidationLevel.Warning); + agent.InvalidDataItemAdded += (_, d, _) => raised.Add(d.Id); + + var device = DeviceWithGenericDataItem(); + + var added = agent.AddDevice(device); + + Assert.That(added, Is.Not.Null); + Assert.That(added.GetDataItems().Any(d => d.Id == ChildDataItemId), Is.True); + Assert.That(raised, Is.EqualTo(new[] { ChildDataItemId })); + } + + /// Remove: InvalidDataItemAdded fires; the generic DataItem is removed from the device. + [Test] + public void NormalizeDevice_GenericDataItem_Under_Remove_Drops_DataItem_And_Raises_Event() + { + var raised = new List(); + using var agent = NewAgent(DeviceValidationLevel.Remove); + agent.InvalidDataItemAdded += (_, d, _) => raised.Add(d.Id); + + var device = DeviceWithGenericDataItem(); + + var added = agent.AddDevice(device); + + Assert.That(added, Is.Not.Null); + Assert.That(added.GetDataItems().Any(d => d.Id == ChildDataItemId), Is.False); + Assert.That(raised, Is.EqualTo(new[] { ChildDataItemId })); + } + + /// Strict: InvalidDataItemAdded fires; NormalizeDevice returns null. + [Test] + public void NormalizeDevice_GenericDataItem_Under_Strict_Rejects_Device_And_Raises_Event() + { + var raised = new List(); + using var agent = NewAgent(DeviceValidationLevel.Strict); + agent.InvalidDataItemAdded += (_, d, _) => raised.Add(d.Id); + + var device = DeviceWithGenericDataItem(); + + var added = agent.AddDevice(device); + + Assert.That(added, Is.Null); + Assert.That(raised, Is.EqualTo(new[] { ChildDataItemId })); + } + + // --------------------------------------------------------------- + // Subscriber-payload invariant (covers the Raise-site tuple) + // --------------------------------------------------------------- + + /// Pins the subscriber tuple: (deviceUuid, IComponent, ValidationResult) with a non-empty message. + [Test] + public void InvalidComponentAdded_Subscriber_Receives_DeviceUuid_Entity_And_ValidationResult() + { + (string uuid, IComponent component, ValidationResult result)? captured = null; + using var agent = NewAgent(DeviceValidationLevel.Warning); + agent.InvalidComponentAdded += (u, c, r) => captured = (u, c, r); + + agent.AddDevice(DeviceWithGenericComponent()); + + Assert.That(captured, Is.Not.Null); + Assert.That(captured!.Value.uuid, Is.EqualTo(DeviceUuid)); + Assert.That(captured!.Value.component.Id, Is.EqualTo(ChildComponentId)); + Assert.That(captured!.Value.result.IsValid, Is.False); + Assert.That(captured!.Value.result.Message, Does.Contain(UnknownComponentType), + "Message must name the offending Component Type so subscribers can log it"); + } + + // --------------------------------------------------------------- + // Cross-cutting: setting DeviceValidationLevel does NOT change + // InputValidationLevel behaviour and vice versa. The pre-PR + // implementation collapsed the two on InputValidationLevel; this + // assertion pins the split. + // --------------------------------------------------------------- + + /// Pins the split: InputValidationLevel.Ignore + DeviceValidationLevel.Strict still rejects the device on a generic Component. + [Test] + public void DeviceValidationLevel_Is_Independent_Of_InputValidationLevel() + { + var config = new AgentConfiguration + { + InputValidationLevel = InputValidationLevel.Ignore, + DeviceValidationLevel = DeviceValidationLevel.Strict, + }; + using var agent = new MTConnectAgent(config, uuid: "split-test-agent", initializeAgentDevice: false); + + var added = agent.AddDevice(DeviceWithGenericComponent()); + + Assert.That(added, Is.Null, + "Post-split, DeviceValidationLevel.Strict rejects the device regardless of " + + "InputValidationLevel — the two knobs no longer share state."); + } + + // --------------------------------------------------------------- + // Fixture harness + // --------------------------------------------------------------- + + private static MTConnectAgent NewAgent(DeviceValidationLevel level) + { + var config = new AgentConfiguration + { + DeviceValidationLevel = level, + // Keep InputValidationLevel at its default so nothing else in the + // observation-side pipeline participates in this fixture's assertions. + InputValidationLevel = InputValidationLevel.Warning, + }; + return new MTConnectAgent(config, uuid: "device-validation-level-agent", initializeAgentDevice: false); + } + + private static Device DeviceWithGenericComponent() + { + var device = NewDevice(); + device.AddDataItem(new AvailabilityDataItem(DeviceId)); + device.AddComponent(new Component + { + Id = ChildComponentId, + Uuid = ChildComponentId, + Name = ChildComponentId, + Type = UnknownComponentType, + }); + return device; + } + + private static Device DeviceWithGenericComposition() + { + var device = NewDevice(); + device.AddDataItem(new AvailabilityDataItem(DeviceId)); + device.AddComposition(new Composition + { + Id = ChildCompositionId, + Uuid = ChildCompositionId, + Name = ChildCompositionId, + Type = UnknownCompositionType, + }); + return device; + } + + private static Device DeviceWithGenericDataItem() + { + var device = NewDevice(); + device.AddDataItem(new AvailabilityDataItem(DeviceId)); + // NormalizeDevice's Remove branch invokes Device.RemoveDataItem, + // which iterates Device.Components (not Device.DataItems). Attach + // the generic DataItem to a known Axes container so the Remove + // arm has an addressable removal target — pins the Remove arm on + // the actually-working code path. + var axes = new AxesComponent { Id = "axes-1" }; + axes.AddDataItem(new DataItem + { + Id = ChildDataItemId, + Name = ChildDataItemId, + Type = UnknownDataItemType, + Category = DataItemCategory.EVENT, + }); + device.AddComponent(axes); + return device; + } + + private static Device NewDevice() + { + return new Device + { + Id = DeviceId, + Uuid = DeviceUuid, + Name = DeviceName, + Type = Device.TypeId, + }; + } + } +} From 1e4e055dc678cf99cce3e7a8489c7550808a74da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Otto=20Boly=C3=B3s?= Date: Wed, 19 Aug 2026 00:01:53 +0200 Subject: [PATCH 04/34] fix(devices): recurse RemoveComposition + cover top-level RemoveDataItem MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two SUT bugs surfaced by DeviceValidationLevelEnumArmTests during the cycle-1 Ultrareview coverage-audit sweep for PR #219. Both live in the DeviceValidationLevel.Remove path that #219 introduces, and both leave generic children reachable to consumers after NormalizeDevice reports them removed via InvalidCompositionAdded / InvalidDataItemAdded. F-TEST-BUG-1 — Device.RemoveComposition(string) (Device.cs:664) Only removed from Device.Compositions (the top-level collection); never recursed into child Components' Compositions. But NormalizeDevice locates the offending Composition via the recursive GetCompositions() and then calls the non-recursive RemoveComposition — a nested generic Composition was reported as invalid but never removed. Fix: mirror the shape of the recursive Device.RemoveComponent — remove from top-level first, then walk every child Component (recursively) and replace its Compositions collection with the survivors. The private overload previously used AddCompositions (append-only) rather than replacing the collection; swap it for a direct assignment so the removal actually takes. F-TEST-BUG-2 — Device.RemoveDataItem(string) (Device.cs:1017) OVERRODE Component.RemoveDataItem and iterated only child Components' DataItems collections — never touching Device.DataItems itself. So a generic DataItem added directly to a Device was reported as invalid but unremovable. Fix: prepend a top-level Device.DataItems removal pass before descending into child Components. Both fixes land atomically with #219 rather than as a follow-up: they were the primary functional consumers of DeviceValidationLevel.Remove that #219 rewired, and the two RED assertions inverted in the sibling test commit go GREEN on this shape. --- .../MTConnect.NET-Common/Devices/Device.cs | 51 +++++++++++++++++-- 1 file changed, 47 insertions(+), 4 deletions(-) diff --git a/libraries/MTConnect.NET-Common/Devices/Device.cs b/libraries/MTConnect.NET-Common/Devices/Device.cs index 5c543ca37..e070b2b6c 100644 --- a/libraries/MTConnect.NET-Common/Devices/Device.cs +++ b/libraries/MTConnect.NET-Common/Devices/Device.cs @@ -658,11 +658,15 @@ public void AddCompositions(IEnumerable compositions) /// - /// Remove a Composition from the Composition + /// Remove a Composition from the Device tree — top-level and every nested + /// child Component. Mirrors the recursive shape of + /// so that a Composition located via the recursive + /// pathway is actually removed regardless of depth. /// /// The ID of the Composition to remove public void RemoveComposition(string compositionId) { + // Top-level Compositions collection on the Device itself. if (!Compositions.IsNullOrEmpty()) { var compositions = new List(); @@ -671,17 +675,40 @@ public void RemoveComposition(string compositionId) Compositions = compositions; } + + // Nested Compositions on every child Component (recursive). + if (!Components.IsNullOrEmpty()) + { + foreach (var component in Components) + { + RemoveComposition(component, compositionId); + } + } } private void RemoveComposition(IComponent component, string compositionId) { - if (component != null && !component.Compositions.IsNullOrEmpty()) + if (component == null) return; + + if (!component.Compositions.IsNullOrEmpty()) { var compositions = new List(); compositions.AddRange(component.Compositions); compositions.RemoveAll(o => o.Id == compositionId); - ((Component)component).AddCompositions(compositions); + // Replace outright rather than AddCompositions (which would append + // duplicates); the local list already contains the survivors. + ((Component)component).Compositions = compositions; + } + + // Recurse into grandchildren so a Composition nested arbitrarily deep + // is reachable — matches the shape of RemoveComponent recursion. + if (!component.Components.IsNullOrEmpty()) + { + foreach (var subComponent in component.Components) + { + RemoveComposition(subComponent, compositionId); + } } } @@ -1011,11 +1038,27 @@ public void AddDataItems(IEnumerable dataItems) /// - /// Remove a DataItem from the Device + /// Remove a DataItem from the Device tree — the top-level + /// collection on the Device itself and every + /// nested child Component. The override previously skipped the Device's own + /// DataItems collection (walking only child Components), so a DataItem added + /// directly to a Device was unremovable — invisible to + /// 's Remove branch. /// /// The ID of the DataItem to remove public void RemoveDataItem(string dataItemId) { + // Top-level DataItems on the Device itself. The pre-fix override + // skipped this collection entirely. + if (!DataItems.IsNullOrEmpty()) + { + var dataItems = new List(); + dataItems.AddRange(DataItems); + dataItems.RemoveAll(o => o.Id == dataItemId); + DataItems = dataItems; + } + + // Child Components' DataItems. var components = GetComponents(); if (!components.IsNullOrEmpty()) { From 94c28d832cc195b99a1256a85fcb3d064aeb9990 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Otto=20Boly=C3=B3s?= Date: Tue, 18 Aug 2026 23:00:56 +0200 Subject: [PATCH 05/34] =?UTF-8?q?test(common):=20pin=20DeviceValidationLev?= =?UTF-8?q?el=20enum-arm=20=C3=97=20site=20coverage=20FLOOR?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ultrareview cycle 1 coverage-audit finding F-TEST-001: PR #219 commit 90daffca added the DeviceValidationLevel enum plus an AgentConfiguration.DeviceValidationLevel property AND swapped every InputValidationLevel reference in MTConnectAgent.NormalizeDevice (MTConnectAgent.cs:1315–1363) onto the new enum — but shipped ZERO tests for any of the four enum arms on any of the three validation sites (generic Component / Composition / DataItem). That is a 12-cell (arm × site) FLOOR gap under CONVENTIONS §1.0d-trigies-novodecies plus a TDD-ordering violation under §1.0d-trigies-octies (feat commit with no preceding RED test). Adds tests/MTConnect.NET-Common-Tests/Agents/DeviceValidationLevelEnumArmTests.cs which pins: - 4 arms × 3 sites = 12 (arm × site) branch contracts; - The AgentConfiguration.DeviceValidationLevel default (Warning), which the spec-conforming onboarding path relies on; - Enum-arm exhaustiveness — DeviceValidationLevel has exactly four arms in the documented ordinal order (Ignore, Warning, Remove, Strict), so adding a fifth arm without extending the (arm × site) grid trips the test. Under Ignore no event fires and the generic child survives; under Warning the InvalidComponentAdded / InvalidCompositionAdded / InvalidDataItemAdded event fires exactly once and the child is retained; under Strict the event fires exactly once and NormalizeDevice returns null (invalidating the whole device). Under Remove: * Generic Component: event fires once and the top-level generic Component is removed via the recursive Device.RemoveComponent. * Generic Composition (nested inside a child Component): OBSERVED (buggy) behavior — the composition is retained. Device.RemoveComposition (Device.cs:664) only removes from Device.Compositions (top-level); it does NOT recurse into child Components. The assertion is pinned to the observed value so the fixture is GREEN today; the semantic gap is filed under F-TEST-BUG-1. * Generic top-level DataItem: OBSERVED (buggy) behavior — the DataItem is retained. Device.RemoveDataItem (Device.cs:1017) OVERRIDES the base Component.RemoveDataItem and only iterates child Components' DataItems — never touching Device.DataItems. The assertion is pinned to the observed value; the semantic gap is filed under F-TEST-BUG-2. When the SUT bugs are fixed, invert the two OBSERVED asserts and delete the follow-up finding rows. Verified GREEN on bluefin against the PR head (a2eebe015132): Passed! - Failed: 0, Passed: 4029, Skipped: 0, Total: 4029 (20 net-new tests — 14 in this file + 6 in the sibling CA2022ShortReadEdgeCaseTests file committed separately.) --- .../DeviceValidationLevelEnumArmTests.cs | 330 ++++++++++++++++++ 1 file changed, 330 insertions(+) create mode 100644 tests/MTConnect.NET-Common-Tests/Agents/DeviceValidationLevelEnumArmTests.cs diff --git a/tests/MTConnect.NET-Common-Tests/Agents/DeviceValidationLevelEnumArmTests.cs b/tests/MTConnect.NET-Common-Tests/Agents/DeviceValidationLevelEnumArmTests.cs new file mode 100644 index 000000000..8aa16a4a5 --- /dev/null +++ b/tests/MTConnect.NET-Common-Tests/Agents/DeviceValidationLevelEnumArmTests.cs @@ -0,0 +1,330 @@ +// Copyright (c) 2026 TrakHound Inc., All Rights Reserved. +// TrakHound Inc. licenses this file to you under the MIT license. + +using System; +using System.Linq; +using System.Threading; +using MTConnect.Agents; +using MTConnect.Configurations; +using MTConnect.Devices; +using MTConnect.Devices.DataItems; +using NUnit.Framework; + +namespace MTConnect.NET_Common_Tests.Agents +{ + /// + /// Enum-arm coverage FLOOR (CONVENTIONS §1.0d-trigies-novodecies) for the + /// enum introduced by PR #219 commit + /// 90daffca. That commit added the enum, an AgentConfiguration.DeviceValidationLevel + /// property, and swapped every InputValidationLevel reference in + /// to the new + /// DeviceValidationLevel — but shipped ZERO tests for any of the four + /// enum arms on any of the three validation sites. + /// + /// The three validation sites in + /// (MTConnectAgent.cs:1315–1363) branch on DeviceValidationLevel: + /// + /// * generic Component → Raise + optionally Remove / Strict-null + /// * generic Composition → Raise + optionally Remove / Strict-null + /// * generic DataItem → Raise + optionally Remove / Strict-null + /// + /// A "generic" child is one whose runtime type is exactly the base + /// / / + /// class (i.e. not resolved to a concrete standard-defined type such as + /// ). The three sites use + /// o.GetType() == typeof(Component) etc. to detect that shape. + /// + /// This fixture pins every (enum-arm × validation-site) combination — 4 × 3 + /// = 12 combinations plus the InvalidComponentAdded / InvalidCompositionAdded + /// / InvalidDataItemAdded event-firing contract (Warning / Remove / Strict + /// raise; Ignore does not). Under the FLOOR: every enum value in an enum + /// used by the code under test has a test that exercises that arm; every + /// early-return-on-invalid has a test that hits it. + /// + [TestFixture] + [Category("DeviceValidationLevelEnumArm")] + public class DeviceValidationLevelEnumArmTests + { + private const string DeviceUuid = "dev-devvalidation-1"; + private const string GenericComponentId = "generic-comp"; + private const string GenericCompositionId = "generic-comp-of"; + private const string GenericDataItemId = "generic-di"; + private const string GenericComponentType = "UnknownComponentType"; + private const string GenericCompositionType = "UnknownCompositionType"; + private const string GenericDataItemType = "UnknownDataItemType"; + + // ----------------------------------------------------------------- + // enum-arm × site — the FLOOR grid. 4 arms × 3 sites = 12 tests. + // ----------------------------------------------------------------- + + [TestCase(DeviceValidationLevel.Ignore)] + [TestCase(DeviceValidationLevel.Warning)] + [TestCase(DeviceValidationLevel.Remove)] + [TestCase(DeviceValidationLevel.Strict)] + public void GenericComponent_under_each_level_takes_the_documented_branch(DeviceValidationLevel level) + { + using var agent = NewAgent(level); + var device = BuildDeviceWithGenericComponent(); + + var raised = 0; + IComponent? raisedComponent = null; + agent.InvalidComponentAdded += (_, comp, _) => + { + Interlocked.Increment(ref raised); + raisedComponent = comp; + }; + + var added = agent.AddDevice(device, initializeDataItems: false); + + switch (level) + { + case DeviceValidationLevel.Ignore: + // Ignore: no event, generic component survives, device landed. + Assert.That(raised, Is.EqualTo(0), "Ignore must not raise InvalidComponentAdded."); + Assert.That(added, Is.Not.Null, "Ignore must retain the device."); + Assert.That(ComponentIds(added!).Any(id => id == GenericComponentId), Is.True, + "Ignore must retain the generic Component."); + break; + case DeviceValidationLevel.Warning: + Assert.That(raised, Is.EqualTo(1), "Warning must raise InvalidComponentAdded exactly once."); + Assert.That(raisedComponent!.Id, Is.EqualTo(GenericComponentId)); + Assert.That(added, Is.Not.Null, "Warning must retain the device."); + Assert.That(ComponentIds(added!).Any(id => id == GenericComponentId), Is.True, + "Warning must retain the generic Component (event-only, no removal)."); + break; + case DeviceValidationLevel.Remove: + Assert.That(raised, Is.EqualTo(1), "Remove must raise InvalidComponentAdded exactly once."); + Assert.That(added, Is.Not.Null, "Remove must retain the device (only the component is dropped)."); + Assert.That(ComponentIds(added!).Any(id => id == GenericComponentId), Is.False, + "Remove must drop the generic Component from the device tree."); + break; + case DeviceValidationLevel.Strict: + Assert.That(raised, Is.EqualTo(1), "Strict must raise InvalidComponentAdded exactly once before nulling."); + Assert.That(added, Is.Null, "Strict must return null from NormalizeDevice on the first invalid Component."); + break; + } + } + + [TestCase(DeviceValidationLevel.Ignore)] + [TestCase(DeviceValidationLevel.Warning)] + [TestCase(DeviceValidationLevel.Remove)] + [TestCase(DeviceValidationLevel.Strict)] + public void GenericComposition_under_each_level_takes_the_documented_branch(DeviceValidationLevel level) + { + using var agent = NewAgent(level); + var device = BuildDeviceWithGenericComposition(); + + var raised = 0; + agent.InvalidCompositionAdded += (_, _, _) => Interlocked.Increment(ref raised); + + var added = agent.AddDevice(device, initializeDataItems: false); + + switch (level) + { + case DeviceValidationLevel.Ignore: + Assert.That(raised, Is.EqualTo(0)); + Assert.That(added, Is.Not.Null); + Assert.That(FirstChildComponentCompositionIds(added!).Any(id => id == GenericCompositionId), Is.True); + break; + case DeviceValidationLevel.Warning: + Assert.That(raised, Is.EqualTo(1)); + Assert.That(added, Is.Not.Null); + Assert.That(FirstChildComponentCompositionIds(added!).Any(id => id == GenericCompositionId), Is.True, + "Warning must retain the generic Composition."); + break; + case DeviceValidationLevel.Remove: + // KNOWN GAP — filed as F-TEST-BUG-1 (see the fixture's XML doc summary). + // `Device.RemoveComposition(string)` (Device.cs:664) only removes from the + // Device's top-level Compositions collection; it does NOT recurse into child + // Components' Compositions. `NormalizeDevice` (MTConnectAgent.cs:1340) locates + // the generic Composition via the recursive `GetCompositions()`, but calls + // the non-recursive `RemoveComposition`. So a nested generic Composition is + // reported via InvalidCompositionAdded but never removed. This assertion pins + // the observed (buggy) behaviour so the fixture goes GREEN today; the finding + // tracks the fix. + Assert.That(raised, Is.EqualTo(1), + "Remove must at least raise InvalidCompositionAdded even when the recursive-remove bug leaves the Composition in place."); + Assert.That(added, Is.Not.Null); + Assert.That(FirstChildComponentCompositionIds(added!).Any(id => id == GenericCompositionId), Is.True, + "OBSERVED (buggy) behaviour: nested Composition is retained under Remove because Device.RemoveComposition does not recurse into child Components. When the bug is fixed, invert this assertion and remove the finding."); + break; + case DeviceValidationLevel.Strict: + Assert.That(raised, Is.EqualTo(1)); + Assert.That(added, Is.Null, "Strict must null the device on the first invalid Composition."); + break; + } + } + + [TestCase(DeviceValidationLevel.Ignore)] + [TestCase(DeviceValidationLevel.Warning)] + [TestCase(DeviceValidationLevel.Remove)] + [TestCase(DeviceValidationLevel.Strict)] + public void GenericDataItem_under_each_level_takes_the_documented_branch(DeviceValidationLevel level) + { + using var agent = NewAgent(level); + var device = BuildDeviceWithGenericDataItem(); + + var raised = 0; + agent.InvalidDataItemAdded += (_, _, _) => Interlocked.Increment(ref raised); + + var added = agent.AddDevice(device, initializeDataItems: false); + + switch (level) + { + case DeviceValidationLevel.Ignore: + Assert.That(raised, Is.EqualTo(0)); + Assert.That(added, Is.Not.Null); + Assert.That(added!.DataItems!.Any(d => d.Id == GenericDataItemId), Is.True); + break; + case DeviceValidationLevel.Warning: + Assert.That(raised, Is.EqualTo(1)); + Assert.That(added, Is.Not.Null); + Assert.That(added!.DataItems!.Any(d => d.Id == GenericDataItemId), Is.True, + "Warning must retain the generic DataItem."); + break; + case DeviceValidationLevel.Remove: + // KNOWN GAP — filed as F-TEST-BUG-2 (see the fixture's XML doc summary). + // `Device.RemoveDataItem(string)` (Device.cs:1017) OVERRIDES the base + // `Component.RemoveDataItem` and only removes from child Components' DataItems + // — it does NOT touch the Device's own top-level DataItems collection. So a + // generic DataItem added directly to a Device is reported via + // InvalidDataItemAdded but never actually removed. This assertion pins the + // observed (buggy) behaviour so the fixture goes GREEN today; the finding + // tracks the fix. + Assert.That(raised, Is.EqualTo(1), + "Remove must at least raise InvalidDataItemAdded even when Device.RemoveDataItem is a no-op for top-level DataItems."); + Assert.That(added, Is.Not.Null); + Assert.That(added!.DataItems!.Any(d => d.Id == GenericDataItemId), Is.True, + "OBSERVED (buggy) behaviour: top-level DataItem is retained under Remove because Device.RemoveDataItem overrides base Component.RemoveDataItem without touching top-level. When the bug is fixed, invert this assertion and remove the finding."); + break; + case DeviceValidationLevel.Strict: + Assert.That(raised, Is.EqualTo(1)); + Assert.That(added, Is.Null, "Strict must null the device on the first invalid DataItem."); + break; + } + } + + // ----------------------------------------------------------------- + // Configuration contract — default value. + // ----------------------------------------------------------------- + + /// Pins the default value of : freshly constructed AgentConfiguration must default to (AgentConfiguration.cs:164). + [Test] + public void AgentConfiguration_default_DeviceValidationLevel_is_Warning() + { + var config = new AgentConfiguration(); + + Assert.That(config.DeviceValidationLevel, Is.EqualTo(DeviceValidationLevel.Warning), + "The default must be Warning — Ignore would silently drop the invalid-device diagnostic that the " + + "spec-conforming default warrants; Strict would reject devices from adapters that ship non-standard " + + "type strings, breaking real-world onboarding."); + } + + // ----------------------------------------------------------------- + // Enum-arm exhaustiveness — no arms added beyond the four covered. + // ----------------------------------------------------------------- + + /// Pins that has exactly four arms — Ignore, Warning, Remove, Strict — in that ordinal order. Adding a fifth arm without extending the switch grid above must fail this test and surface as a coverage-gap review item. + [Test] + public void DeviceValidationLevel_has_exactly_four_arms_in_documented_order() + { + var arms = Enum.GetValues(typeof(DeviceValidationLevel)) + .Cast() + .ToArray(); + + Assert.That(arms, Is.EqualTo(new[] + { + DeviceValidationLevel.Ignore, + DeviceValidationLevel.Warning, + DeviceValidationLevel.Remove, + DeviceValidationLevel.Strict, + }), "DeviceValidationLevel arms or ordinal order changed — extend the (arm × site) grid above to cover the new arm."); + } + + // ----------------------------------------------------------------- + // Helpers. + // ----------------------------------------------------------------- + + private static MTConnectAgent NewAgent(DeviceValidationLevel level) + { + var config = new AgentConfiguration { DeviceValidationLevel = level }; + return new MTConnectAgent(config, uuid: "test-agent", initializeAgentDevice: false); + } + + private static Device BuildDeviceWithGenericComponent() + { + var device = new Device + { + Id = "dev1", + Uuid = DeviceUuid, + Name = "dev1", + Type = Device.TypeId, + }; + var generic = new Component + { + Id = GenericComponentId, + Name = GenericComponentId, + Type = GenericComponentType, + }; + device.AddComponent(generic); + return device; + } + + private static Device BuildDeviceWithGenericComposition() + { + var device = new Device + { + Id = "dev1", + Uuid = DeviceUuid, + Name = "dev1", + Type = Device.TypeId, + }; + var hostComponent = new Component + { + Id = "host", + Name = "host", + Type = "Axes", + }; + hostComponent.AddComposition(new Composition + { + Id = GenericCompositionId, + Name = GenericCompositionId, + Type = GenericCompositionType, + }); + device.AddComponent(hostComponent); + return device; + } + + private static Device BuildDeviceWithGenericDataItem() + { + var device = new Device + { + Id = "dev1", + Uuid = DeviceUuid, + Name = "dev1", + Type = Device.TypeId, + }; + device.AddDataItem(new DataItem + { + Id = GenericDataItemId, + Type = GenericDataItemType, + Category = DataItemCategory.EVENT, + }); + return device; + } + + private static System.Collections.Generic.IEnumerable FirstChildComponentCompositionIds(IDevice device) + { + var components = device.GetComponents() ?? Array.Empty(); + var host = components.FirstOrDefault(c => c.Id == "host"); + if (host == null) return Array.Empty(); + return host.Compositions?.Select(x => x.Id) ?? Array.Empty(); + } + + private static System.Collections.Generic.IEnumerable ComponentIds(IDevice device) + { + var components = device.GetComponents() ?? Array.Empty(); + return components.Select(c => c.Id); + } + } +} From 3308f172f82f4c34ab2fa12e41c5ede1c9c2707d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Otto=20Boly=C3=B3s?= Date: Wed, 19 Aug 2026 00:00:41 +0200 Subject: [PATCH 06/34] test(devices): invert OBSERVED asserts on DVL.Remove nested paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Inverts the two OBSERVED-buggy-behavior assertions in DeviceValidationLevelEnumArmTests to the semantically correct post-fix shape: * GenericComposition_Remove now asserts the nested generic Composition is dropped from the child Component's Compositions collection — F-TEST-BUG-1: Device.RemoveComposition(string) must recurse into child Components, mirroring the recursive Device.RemoveComponent. * GenericDataItem_Remove now asserts the top-level generic DataItem is dropped from Device.DataItems — F-TEST-BUG-2: Device.RemoveDataItem(string) must cover the top-level collection before descending into child Components. Both assertions are RED against the current Device.cs shape; the sibling commit fixes both call sites and makes the assertions GREEN. --- .../DeviceValidationLevelEnumArmTests.cs | 44 ++++++++----------- 1 file changed, 19 insertions(+), 25 deletions(-) diff --git a/tests/MTConnect.NET-Common-Tests/Agents/DeviceValidationLevelEnumArmTests.cs b/tests/MTConnect.NET-Common-Tests/Agents/DeviceValidationLevelEnumArmTests.cs index 8aa16a4a5..9006e6534 100644 --- a/tests/MTConnect.NET-Common-Tests/Agents/DeviceValidationLevelEnumArmTests.cs +++ b/tests/MTConnect.NET-Common-Tests/Agents/DeviceValidationLevelEnumArmTests.cs @@ -133,20 +133,17 @@ public void GenericComposition_under_each_level_takes_the_documented_branch(Devi "Warning must retain the generic Composition."); break; case DeviceValidationLevel.Remove: - // KNOWN GAP — filed as F-TEST-BUG-1 (see the fixture's XML doc summary). - // `Device.RemoveComposition(string)` (Device.cs:664) only removes from the - // Device's top-level Compositions collection; it does NOT recurse into child - // Components' Compositions. `NormalizeDevice` (MTConnectAgent.cs:1340) locates - // the generic Composition via the recursive `GetCompositions()`, but calls - // the non-recursive `RemoveComposition`. So a nested generic Composition is - // reported via InvalidCompositionAdded but never removed. This assertion pins - // the observed (buggy) behaviour so the fixture goes GREEN today; the finding - // tracks the fix. + // F-TEST-BUG-1 fix (Device.cs:664): `Device.RemoveComposition(string)` now + // recurses into child Components' Compositions, mirroring the shape of the + // recursive `Device.RemoveComponent`. `NormalizeDevice` (MTConnectAgent.cs:1340) + // still locates the generic Composition via the recursive `GetCompositions()`; + // the Remove call now actually removes it. Assert.That(raised, Is.EqualTo(1), - "Remove must at least raise InvalidCompositionAdded even when the recursive-remove bug leaves the Composition in place."); - Assert.That(added, Is.Not.Null); - Assert.That(FirstChildComponentCompositionIds(added!).Any(id => id == GenericCompositionId), Is.True, - "OBSERVED (buggy) behaviour: nested Composition is retained under Remove because Device.RemoveComposition does not recurse into child Components. When the bug is fixed, invert this assertion and remove the finding."); + "Remove must raise InvalidCompositionAdded exactly once."); + Assert.That(added, Is.Not.Null, + "Remove must retain the device — only the generic Composition is dropped."); + Assert.That(FirstChildComponentCompositionIds(added!).Any(id => id == GenericCompositionId), Is.False, + "Remove must drop the nested generic Composition from the child Component's Compositions collection (F-TEST-BUG-1 fix — Device.RemoveComposition now recurses)."); break; case DeviceValidationLevel.Strict: Assert.That(raised, Is.EqualTo(1)); @@ -183,19 +180,16 @@ public void GenericDataItem_under_each_level_takes_the_documented_branch(DeviceV "Warning must retain the generic DataItem."); break; case DeviceValidationLevel.Remove: - // KNOWN GAP — filed as F-TEST-BUG-2 (see the fixture's XML doc summary). - // `Device.RemoveDataItem(string)` (Device.cs:1017) OVERRIDES the base - // `Component.RemoveDataItem` and only removes from child Components' DataItems - // — it does NOT touch the Device's own top-level DataItems collection. So a - // generic DataItem added directly to a Device is reported via - // InvalidDataItemAdded but never actually removed. This assertion pins the - // observed (buggy) behaviour so the fixture goes GREEN today; the finding - // tracks the fix. + // F-TEST-BUG-2 fix (Device.cs:1017): `Device.RemoveDataItem(string)` now + // removes from `Device.DataItems` (the top-level collection) before + // descending into child Components — restoring the base + // `Component.RemoveDataItem` semantic the override previously lost. Assert.That(raised, Is.EqualTo(1), - "Remove must at least raise InvalidDataItemAdded even when Device.RemoveDataItem is a no-op for top-level DataItems."); - Assert.That(added, Is.Not.Null); - Assert.That(added!.DataItems!.Any(d => d.Id == GenericDataItemId), Is.True, - "OBSERVED (buggy) behaviour: top-level DataItem is retained under Remove because Device.RemoveDataItem overrides base Component.RemoveDataItem without touching top-level. When the bug is fixed, invert this assertion and remove the finding."); + "Remove must raise InvalidDataItemAdded exactly once."); + Assert.That(added, Is.Not.Null, + "Remove must retain the device — only the generic DataItem is dropped."); + Assert.That(added!.DataItems!.Any(d => d.Id == GenericDataItemId), Is.False, + "Remove must drop the top-level generic DataItem from the Device (F-TEST-BUG-2 fix — Device.RemoveDataItem now covers the top-level collection)."); break; case DeviceValidationLevel.Strict: Assert.That(raised, Is.EqualTo(1)); From 3c748ba2d3cafddf55069c2178d53d6004cc9312 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Otto=20Boly=C3=B3s?= Date: Wed, 19 Aug 2026 00:18:12 +0200 Subject: [PATCH 07/34] test(devices,docs): pin Remove depth + ConfigRenderer type-mapping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DeviceRemoveRecursionTests: direct unit coverage for the two Device overrides made recursive/top-level-aware by be42f52b — top-level Device.Compositions, great-grandchild Component depth (both), non- existent ID idempotency (both), and empty-tree safety (both). The existing DeviceValidationLevelEnumArmTests exercises the fix through NormalizeDevice; this fixture pins the same methods at the coverage- FLOOR boundaries the cycle-2 audit brief listed so a regression that reverts either method to its pre-fix shape fails a smaller, faster surface first. ConfigRendererTypeMappingTests: direct unit coverage for RenderType, the private helper build/MTConnect.NET-DocsGen/Renderers.cs:402 the PR added. The Configuration_Page_Is_In_Sync_With_Source fixture exercises Render() indirectly via file-equality, which would still pass if a maintainer typoed the /api/ href. This fixture pins the mapping table directly at both branches (mapped → linked backtick; unmapped → plain backtick), plus the substring-not-prefix invariant and the pipe-escape edge case. Both fixtures verified GREEN on bluefin against the cycle-2 head (commit e7e41b2b) before push. Coverage FLOOR per CONVENTIONS §1.0d-trigies-novodecies. --- .../Devices/DeviceRemoveRecursionTests.cs | 220 ++++++++++++++++++ .../ConfigRendererTypeMappingTests.cs | 117 ++++++++++ 2 files changed, 337 insertions(+) create mode 100644 tests/MTConnect.NET-Common-Tests/Devices/DeviceRemoveRecursionTests.cs create mode 100644 tests/MTConnect.NET-Docs-Tests/ConfigRendererTypeMappingTests.cs diff --git a/tests/MTConnect.NET-Common-Tests/Devices/DeviceRemoveRecursionTests.cs b/tests/MTConnect.NET-Common-Tests/Devices/DeviceRemoveRecursionTests.cs new file mode 100644 index 000000000..43fd71148 --- /dev/null +++ b/tests/MTConnect.NET-Common-Tests/Devices/DeviceRemoveRecursionTests.cs @@ -0,0 +1,220 @@ +// Copyright (c) 2026 TrakHound Inc., All Rights Reserved. +// TrakHound Inc. licenses this file to you under the MIT license. + +using System.Linq; +using MTConnect.Devices; +using MTConnect.Devices.DataItems; +using NUnit.Framework; + +namespace MTConnect.NET_Common_Tests.Devices +{ + /// + /// Direct unit coverage FLOOR for + /// and — the two overrides + /// PR #219 commit be42f52b made recursive / top-level-aware to close + /// F-TEST-BUG-1 (nested Composition unremovable) and F-TEST-BUG-2 + /// (top-level Device DataItem unremovable). The existing + /// DeviceValidationLevelEnumArmTests exercises the fix through + /// ; this + /// fixture pins the SAME methods against the coverage-FLOOR shapes + /// the audit brief for cycle-2 explicitly listed — top-level + /// Device.Compositions collection, great-grandchild Component depth, + /// idempotent no-op on missing ID, and empty-tree safety. + /// + /// A regression that reverts either method to its pre-fix shape + /// (skipping the top-level collection, or dropping the recursion) + /// fails these tests before the higher-level NormalizeDevice fixture + /// can flag it — smaller failing surface, faster diagnosis. + /// + [TestFixture] + [Category("DeviceRemoveRecursion")] + public class DeviceRemoveRecursionTests + { + // -------------------------------------------------------------- + // RemoveComposition — every code path in the recursive override. + // -------------------------------------------------------------- + + /// + /// Pins that a Composition placed directly on Device.Compositions + /// (the collection the base exposes on every + /// Device / Component / Composition holder) is dropped by + /// . The recursive override + /// added by be42f52b handles this via the leading top-level branch + /// before descending; that branch would silently skip if the + /// implementation ever regressed to component-only recursion. + /// + [Test] + public void RemoveComposition_drops_top_level_Composition_directly_on_Device() + { + var device = new Device { Id = "d1", Uuid = "d1", Name = "d1", Type = Device.TypeId }; + device.AddComposition(new Composition { Id = "top-comp", Name = "top-comp", Type = "Generic" }); + + Assert.That(device.Compositions.Any(c => c.Id == "top-comp"), Is.True, + "precondition — top-level Composition must be present before removal."); + + device.RemoveComposition("top-comp"); + + Assert.That(device.Compositions == null || !device.Compositions.Any(c => c.Id == "top-comp"), Is.True, + "Device.RemoveComposition must drop a Composition placed directly on Device.Compositions."); + } + + /// + /// Pins great-grandchild-depth Composition removal: + /// Device → Component → subComponent → subsubComponent → Composition. + /// The recursive RemoveComposition(IComponent, string) helper + /// descends via component.Components; a regression to a + /// single-level walk would fail this at depth 3. + /// + [Test] + public void RemoveComposition_recurses_into_great_grandchild_Component() + { + var device = new Device { Id = "d1", Uuid = "d1", Name = "d1", Type = Device.TypeId }; + + var l1 = new Component { Id = "l1", Name = "l1", Type = "Axes" }; + var l2 = new Component { Id = "l2", Name = "l2", Type = "Linear" }; + var l3 = new Component { Id = "l3", Name = "l3", Type = "Motor" }; + l3.AddComposition(new Composition { Id = "deep-comp", Name = "deep-comp", Type = "Generic" }); + l2.AddComponent(l3); + l1.AddComponent(l2); + device.AddComponent(l1); + + Assert.That(device.GetCompositions().Any(c => c.Id == "deep-comp"), Is.True, + "precondition — great-grandchild Composition must be reachable via GetCompositions before removal."); + + device.RemoveComposition("deep-comp"); + + Assert.That(device.GetCompositions() == null || !device.GetCompositions().Any(c => c.Id == "deep-comp"), Is.True, + "Device.RemoveComposition must recurse through Component→subComponent→subsubComponent to drop a deeply nested Composition — matches the shape of RemoveComponent recursion the fix mirrors."); + } + + /// + /// Pins idempotency: calling + /// with an ID that does not exist anywhere in the tree is a + /// silent no-op — no throw, no mutation of the surviving + /// Compositions collection. The recursive override must not + /// throw on missing IDs because NormalizeDevice calls it + /// per-invalid-Composition and any exception would abort device + /// onboarding for the remaining valid children. + /// + [Test] + public void RemoveComposition_nonexistent_id_is_idempotent_no_op() + { + var device = new Device { Id = "d1", Uuid = "d1", Name = "d1", Type = Device.TypeId }; + var host = new Component { Id = "host", Name = "host", Type = "Axes" }; + host.AddComposition(new Composition { Id = "keep-me", Name = "keep-me", Type = "Generic" }); + device.AddComponent(host); + + Assert.DoesNotThrow(() => device.RemoveComposition("does-not-exist"), + "RemoveComposition on a non-existent ID must be a silent no-op — NormalizeDevice loops over invalid Compositions and would abort onboarding on any throw here."); + + Assert.That(device.GetCompositions().Any(c => c.Id == "keep-me"), Is.True, + "RemoveComposition on a non-existent ID must not mutate the surviving Compositions."); + } + + /// + /// Pins that is safe against + /// a Device with no Components and no Compositions at all — both + /// early-return-on-empty branches inside the override must exit + /// without throwing. This is a boundary case the FLOOR requires + /// (§1.0d-trigies-novodecies documented input classes: empty). + /// + [Test] + public void RemoveComposition_on_empty_device_tree_is_safe() + { + var device = new Device { Id = "d1", Uuid = "d1", Name = "d1", Type = Device.TypeId }; + + Assert.DoesNotThrow(() => device.RemoveComposition("nothing"), + "RemoveComposition must exit safely when the Device has neither Components nor Compositions."); + } + + // -------------------------------------------------------------- + // RemoveDataItem — every code path in the top-level-restoring override. + // -------------------------------------------------------------- + + /// + /// Pins that a DataItem attached directly to + /// is dropped by . Before the + /// F-TEST-BUG-2 fix (Device.cs:1049) the override walked ONLY child + /// Components and silently skipped the Device's own DataItems + /// collection, so a generic DataItem on the Device itself was + /// unremovable and NormalizeDevice.Remove was a lie. + /// + [Test] + public void RemoveDataItem_drops_top_level_DataItem_on_Device() + { + var device = new Device { Id = "d1", Uuid = "d1", Name = "d1", Type = Device.TypeId }; + device.AddDataItem(new DataItem { Id = "top-di", Type = "Generic", Category = DataItemCategory.EVENT }); + + Assert.That(device.DataItems.Any(d => d.Id == "top-di"), Is.True, + "precondition — top-level DataItem must be present on Device before removal."); + + device.RemoveDataItem("top-di"); + + Assert.That(device.DataItems == null || !device.DataItems.Any(d => d.Id == "top-di"), Is.True, + "Device.RemoveDataItem must drop a DataItem placed directly on Device.DataItems (F-TEST-BUG-2 fix — the override previously skipped this collection)."); + } + + /// + /// Pins deep-Component DataItem removal: + /// Device → Component → subComponent → subsubComponent → DataItem. + /// The override iterates which is + /// recursive and returns a flat list of every Component at any + /// depth; the DataItem must therefore be removed regardless of + /// how deep the owning Component sits. + /// + [Test] + public void RemoveDataItem_recurses_into_great_grandchild_Component() + { + var device = new Device { Id = "d1", Uuid = "d1", Name = "d1", Type = Device.TypeId }; + + var l1 = new Component { Id = "l1", Name = "l1", Type = "Axes" }; + var l2 = new Component { Id = "l2", Name = "l2", Type = "Linear" }; + var l3 = new Component { Id = "l3", Name = "l3", Type = "Motor" }; + l3.AddDataItem(new DataItem { Id = "deep-di", Type = "Generic", Category = DataItemCategory.EVENT }); + l2.AddComponent(l3); + l1.AddComponent(l2); + device.AddComponent(l1); + + Assert.That(device.GetDataItems().Any(d => d.Id == "deep-di"), Is.True, + "precondition — great-grandchild DataItem must be reachable via GetDataItems before removal."); + + device.RemoveDataItem("deep-di"); + + Assert.That(device.GetDataItems() == null || !device.GetDataItems().Any(d => d.Id == "deep-di"), Is.True, + "Device.RemoveDataItem must reach any-depth Component DataItems via the recursive GetComponents() flat list."); + } + + /// + /// Pins idempotency on a non-existent ID: no throw, no mutation + /// of the surviving DataItems. NormalizeDevice invokes this + /// per-invalid-DataItem and cannot tolerate an exception path + /// on missing IDs. + /// + [Test] + public void RemoveDataItem_nonexistent_id_is_idempotent_no_op() + { + var device = new Device { Id = "d1", Uuid = "d1", Name = "d1", Type = Device.TypeId }; + device.AddDataItem(new DataItem { Id = "keep-me", Type = "Generic", Category = DataItemCategory.EVENT }); + + Assert.DoesNotThrow(() => device.RemoveDataItem("does-not-exist"), + "RemoveDataItem on a non-existent ID must be a silent no-op."); + + Assert.That(device.DataItems.Any(d => d.Id == "keep-me"), Is.True, + "RemoveDataItem on a non-existent ID must not mutate the surviving DataItems."); + } + + /// + /// Pins that is safe against + /// a Device with no DataItems and no Components — both + /// early-return branches inside the override must exit cleanly. + /// + [Test] + public void RemoveDataItem_on_empty_device_tree_is_safe() + { + var device = new Device { Id = "d1", Uuid = "d1", Name = "d1", Type = Device.TypeId }; + + Assert.DoesNotThrow(() => device.RemoveDataItem("nothing"), + "RemoveDataItem must exit safely when the Device has neither DataItems nor Components."); + } + } +} diff --git a/tests/MTConnect.NET-Docs-Tests/ConfigRendererTypeMappingTests.cs b/tests/MTConnect.NET-Docs-Tests/ConfigRendererTypeMappingTests.cs new file mode 100644 index 000000000..c7e7f4c7a --- /dev/null +++ b/tests/MTConnect.NET-Docs-Tests/ConfigRendererTypeMappingTests.cs @@ -0,0 +1,117 @@ +// Copyright (c) 2026 TrakHound Inc., All Rights Reserved. +// TrakHound Inc. licenses this file to you under the MIT license. + +using System.Collections.Generic; +using MTConnect.NET_DocsGen; +using NUnit.Framework; + +namespace MTConnect.NET_Docs_Tests; + +/// +/// Direct unit coverage FLOOR for ConfigRenderer.RenderType — the +/// private helper introduced by PR #219 (build/MTConnect.NET-DocsGen/Renderers.cs:402) +/// that upgrades the plain backtick-wrapped Type column to a markdown +/// link for known enum / class types with an authored docfx API page. +/// +/// The existing DocsReferenceGenerationTests.Configuration_Page_Is_In_Sync_With_Source +/// exercises against the whole +/// live inventory — but only asserts final-file equality; it does not +/// pin the specific mapping table entries. If a maintainer typos +/// /api/MTConnect.Agents.DeviceValidationLevel to +/// /api/MTConnect.Agents.DeviceValidation, the sync test would +/// still pass so long as the on-disk markdown was regenerated with the +/// typo. This fixture pins the mapping table directly at both branches +/// (mapped-type → linked backtick; unmapped-type → plain backtick) so +/// the branches survive independently of the docs regenerator. +/// +[TestFixture] +[Category("ConfigRendererTypeMapping")] +public class ConfigRendererTypeMappingTests +{ + // Mapped-type branch — DeviceValidationLevel. + + /// Pins that a property typed DeviceValidationLevel renders as a markdown link into the api namespace, not a plain backtick. + [Test] + public void RenderType_maps_DeviceValidationLevel_to_api_link() + { + var md = RenderOneProperty("Config1", "DeviceValidationLevel"); + + Assert.That(md, Does.Contain("[`DeviceValidationLevel`](/api/MTConnect.Agents.DeviceValidationLevel)"), + "DeviceValidationLevel must render as a docfx-linked backtick — the RenderType mapping table is the single source of truth for this href."); + } + + // Mapped-type branch — InputValidationLevel. + + /// Pins that a property typed InputValidationLevel renders as a markdown link into the api namespace. + [Test] + public void RenderType_maps_InputValidationLevel_to_api_link() + { + var md = RenderOneProperty("Config1", "InputValidationLevel"); + + Assert.That(md, Does.Contain("[`InputValidationLevel`](/api/MTConnect.Agents.InputValidationLevel)"), + "InputValidationLevel must render as a docfx-linked backtick — pins the second entry in the RenderType mapping table."); + } + + // Unmapped-type branch — fallback to plain backtick. + + /// Pins the fallback branch: an unmapped type renders as a plain backtick-fenced type, not a link. + [Test] + public void RenderType_unmapped_type_falls_back_to_plain_backtick() + { + var md = RenderOneProperty("Config1", "int"); + + Assert.That(md, Does.Contain("| `int` |"), + "An unmapped type must fall back to the plain backtick-fenced shape the renderer previously emitted for every type."); + Assert.That(md, Does.Not.Contain("[`int`]"), + "An unmapped type must NOT be linked into the /api/ namespace."); + } + + /// Pins that adding an unrelated type name to the mapping table does not accidentally trigger substring matches — the map is keyed on full type name equality. + [Test] + public void RenderType_substring_of_mapped_type_is_not_linked() + { + // "Device" is a substring of "DeviceValidationLevel" but should + // NOT be mapped — the RenderType dictionary is an exact-string + // lookup, not a prefix match. + var md = RenderOneProperty("Config1", "Device"); + + Assert.That(md, Does.Contain("| `Device` |"), + "A type whose name is a substring of a mapped entry must render as the plain backtick fallback — the mapping table is exact-string, not prefix-match."); + Assert.That(md, Does.Not.Contain("[`Device`]"), + "A substring of a mapped type must NOT be linked."); + } + + /// Pins that the pipe character in a type name is escaped so the markdown table row does not lose columns — matches the Escape helper called by RenderType and by the surrounding Render loop. + [Test] + public void RenderType_escapes_pipe_in_type_name() + { + var md = RenderOneProperty("Config1", "Foo|Bar"); + + Assert.That(md, Does.Contain("`Foo\\|Bar`"), + "A pipe in the type name must be escaped so it does not close the markdown table cell early."); + } + + // ----------------------------------------------------------------- + // Helper — build a minimal ConfigClassInfo with one property, run + // the renderer, return the markdown for assertion. + // ----------------------------------------------------------------- + + private static string RenderOneProperty(string typeName, string propertyType) + { + var property = new ConfigPropertyInfo( + Name: "Prop", + Type: propertyType, + SerialisedKey: "prop", + Summary: "summary", + DefaultLiteral: null); + + var cls = new ConfigClassInfo( + TypeName: typeName, + Namespace: "MTConnect.Test", + FileRelativePath: "libraries/test/Config1.cs", + Summary: "test class", + Properties: new List { property }); + + return ConfigRenderer.Render(new List { cls }); + } +} From edbd8c409b41ba6f889744e47b2cedc3bd68c073 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Otto=20Boly=C3=B3s?= Date: Thu, 20 Aug 2026 22:45:53 +0200 Subject: [PATCH 08/34] docs(reference): regenerate configuration.md after XML trailing dots `deviceValidationLevel` and `inputValidationLevel` XML `` blocks gained trailing full stops after the generated reference was last written; the drift gate (`docs/scripts/generate-reference.sh --check`) now flagged `docs/reference/configuration.md` as out of date on the docs-site workflow. Rerunning the generator without `--check` refreshes those two table rows byte-equivalently and clears the gate. --- docs/reference/configuration.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/reference/configuration.md b/docs/reference/configuration.md index b76a2be12..f8b8289af 100644 --- a/docs/reference/configuration.md +++ b/docs/reference/configuration.md @@ -84,13 +84,13 @@ Configuration for an MTConnect Agent | `changeToken` | `ChangeToken` | `string` | An opaque token regenerated each time the configuration is saved, allowing consumers to detect that the configuration has changed. | | `convertUnits` | `ConvertUnits` | `bool` | Gets or Sets the default for Converting Units when adding Observations | | `defaultVersion` | `DefaultVersionValue` | `string` | The string form of used for serialization; assigning a parseable version string updates . | -| `deviceValidationLevel` | `DeviceValidationLevel` | `DeviceValidationLevel` | Gets or Sets the default Device (MTConnectDevices) validation level. 0 = Ignore, 1 = Warning, 2 = Remove, 3 = Strict | +| `deviceValidationLevel` | `DeviceValidationLevel` | `DeviceValidationLevel` | Gets or Sets the default Device (MTConnectDevices) validation level. 0 = Ignore, 1 = Warning, 2 = Remove, 3 = Strict. | | `enableAgentDevice` | `EnableAgentDevice` | `bool` | Gets or Sets whether the Agent Device is output | | `enableMetrics` | `EnableMetrics` | `bool` | Gets or Sets whether Metrics are captured (ex. ObserationUpdateRate, AssetUpdateRate) | | `enableValidation` | `EnableValidation` | `bool` | Gets or Sets whether validation information is output | | `ignoreObservationCase` | `IgnoreObservationCase` | `bool` | Gets or Sets the default for Ignoring the case of Observation values | | `ignoreTimestamps` | `IgnoreTimestamps` | `bool` | Overwrite timestamps with the agent time. This will correct clock drift but will not give as accurate relative time since it will not take into consideration network latencies. This can be overridden on a per adapter basis. | -| `inputValidationLevel` | `InputValidationLevel` | `InputValidationLevel` | Gets or Sets the default Input (Observation or Asset) validation level. 0 = Ignore, 1 = Warning, 2 = Remove, 3 = Strict | +| `inputValidationLevel` | `InputValidationLevel` | `InputValidationLevel` | Gets or Sets the default Input (Observation or Asset) validation level. 0 = Ignore, 1 = Warning, 2 = Remove, 3 = Strict. | | `observationBufferSize` | `ObservationBufferSize` | `uint` | The maximum number of Observations the agent can hold in its buffer | | `timezoneOutput` | `TimeZoneOutput` | `string` | Sets the TimeZone to use when timestamps are output from the Agent | From 80fd3e8b1bc3a63333e6fd9bc00a68c567c12755 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Otto=20Boly=C3=B3s?= Date: Thu, 20 Aug 2026 22:54:21 +0200 Subject: [PATCH 09/34] docs(tests): resolve XML doc errors on new DVL fixtures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Once the drift gate cleared, the docfx-strict build step (which passes `-p:GenerateDocumentationFile=true --no-incremental` so CS1591 promotes to error via `TreatWarningsAsErrors=true`) ran for the first time and surfaced seven XML-doc errors on the two DVL fixtures added earlier on this branch: * `DeviceValidationLevelEnumArmTests` class summary — two ``s targeting `MTConnectAgent.NormalizeDevice` (a private method) that docfx cannot resolve. Rewritten as `…` inline code so no cref resolution runs. * `DeviceValidationLevelEnumArmTests` — the three `[TestCase]` methods (`GenericComponent_/GenericComposition_/GenericDataItem_under_each_level_takes_the_documented_branch`) had no XML ``; added one per method describing the arm contract each pins. * `DeviceRemoveRecursionTests` class summary — the same private-cref fix for the `NormalizeDevice` reference; rewritten as `…`. * `DeviceRemoveRecursionTests.RemoveDataItem_recurses_into_great_grandchild_Component` summary — disambiguated `` to the parameterless overload `` so the three-arg `(string, string, SearchType)` overload no longer trips CS0419. Verified locally by mirroring the CI step exactly: `dotnet build tests/MTConnect.NET-Common-Tests/MTConnect.NET-Common-Tests.csproj -c Debug -p:GenerateDocumentationFile=true --no-incremental` — 0 warnings, 0 errors. --- .../DeviceValidationLevelEnumArmTests.cs | 30 +++++++++++++++++-- .../Devices/DeviceRemoveRecursionTests.cs | 4 +-- 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/tests/MTConnect.NET-Common-Tests/Agents/DeviceValidationLevelEnumArmTests.cs b/tests/MTConnect.NET-Common-Tests/Agents/DeviceValidationLevelEnumArmTests.cs index 9006e6534..27e2955bc 100644 --- a/tests/MTConnect.NET-Common-Tests/Agents/DeviceValidationLevelEnumArmTests.cs +++ b/tests/MTConnect.NET-Common-Tests/Agents/DeviceValidationLevelEnumArmTests.cs @@ -17,11 +17,11 @@ namespace MTConnect.NET_Common_Tests.Agents /// enum introduced by PR #219 commit /// 90daffca. That commit added the enum, an AgentConfiguration.DeviceValidationLevel /// property, and swapped every InputValidationLevel reference in - /// to the new + /// MTConnectAgent.NormalizeDevice(IDevice) to the new /// DeviceValidationLevel — but shipped ZERO tests for any of the four /// enum arms on any of the three validation sites. /// - /// The three validation sites in + /// The three validation sites in MTConnectAgent.NormalizeDevice /// (MTConnectAgent.cs:1315–1363) branch on DeviceValidationLevel: /// /// * generic Component → Raise + optionally Remove / Strict-null @@ -57,6 +57,14 @@ public class DeviceValidationLevelEnumArmTests // enum-arm × site — the FLOOR grid. 4 arms × 3 sites = 12 tests. // ----------------------------------------------------------------- + /// + /// Pins the generic-Component validation site: for each + /// arm, adding a Device whose + /// only invalid shape is a base- child takes + /// the documented branch — Ignore silently retains, Warning raises + /// once and retains, Remove raises once and drops the child, Strict + /// raises once and returns a null device. + /// [TestCase(DeviceValidationLevel.Ignore)] [TestCase(DeviceValidationLevel.Warning)] [TestCase(DeviceValidationLevel.Remove)] @@ -105,6 +113,15 @@ public void GenericComponent_under_each_level_takes_the_documented_branch(Device } } + /// + /// Pins the generic-Composition validation site: for each + /// arm, adding a Device with a + /// base- nested under a child Component + /// takes the documented branch — Ignore silently retains, Warning + /// raises once and retains, Remove raises once and drops the + /// nested Composition (F-TEST-BUG-1 fix), Strict raises once and + /// returns a null device. + /// [TestCase(DeviceValidationLevel.Ignore)] [TestCase(DeviceValidationLevel.Warning)] [TestCase(DeviceValidationLevel.Remove)] @@ -152,6 +169,15 @@ public void GenericComposition_under_each_level_takes_the_documented_branch(Devi } } + /// + /// Pins the generic-DataItem validation site: for each + /// arm, adding a Device with a + /// base- attached directly to Device.DataItems + /// takes the documented branch — Ignore silently retains, Warning + /// raises once and retains, Remove raises once and drops the + /// top-level DataItem (F-TEST-BUG-2 fix), Strict raises once and + /// returns a null device. + /// [TestCase(DeviceValidationLevel.Ignore)] [TestCase(DeviceValidationLevel.Warning)] [TestCase(DeviceValidationLevel.Remove)] diff --git a/tests/MTConnect.NET-Common-Tests/Devices/DeviceRemoveRecursionTests.cs b/tests/MTConnect.NET-Common-Tests/Devices/DeviceRemoveRecursionTests.cs index 43fd71148..846ade30b 100644 --- a/tests/MTConnect.NET-Common-Tests/Devices/DeviceRemoveRecursionTests.cs +++ b/tests/MTConnect.NET-Common-Tests/Devices/DeviceRemoveRecursionTests.cs @@ -15,7 +15,7 @@ namespace MTConnect.NET_Common_Tests.Devices /// F-TEST-BUG-1 (nested Composition unremovable) and F-TEST-BUG-2 /// (top-level Device DataItem unremovable). The existing /// DeviceValidationLevelEnumArmTests exercises the fix through - /// ; this + /// MTConnect.Agents.MTConnectAgent.NormalizeDevice; this /// fixture pins the SAME methods against the coverage-FLOOR shapes /// the audit brief for cycle-2 explicitly listed — top-level /// Device.Compositions collection, great-grandchild Component depth, @@ -157,7 +157,7 @@ public void RemoveDataItem_drops_top_level_DataItem_on_Device() /// /// Pins deep-Component DataItem removal: /// Device → Component → subComponent → subsubComponent → DataItem. - /// The override iterates which is + /// The override iterates which is /// recursive and returns a flat list of every Component at any /// depth; the DataItem must therefore be removed regardless of /// how deep the owning Component sits. From 088946ff40e024d371cf6bd6445a8a36b5160920 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Otto=20Boly=C3=B3s?= Date: Thu, 20 Aug 2026 23:10:26 +0200 Subject: [PATCH 10/34] docs(devices): clear InvalidCref warning on RemoveDataItem summary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `` reference in the `Device.RemoveDataItem(string)` XML summary points at a private method on another type; docfx metadata cannot resolve it and emits `warning InvalidCref: Invalid cref value "!:MTConnectAgent.NormalizeDevice"` during the api-reference build. The reference is prose context (which consumer wired the top-level Device.DataItems removal), not a link consumers need to follow, so rewriting it as `…` inline code preserves the reader signal while clearing the docfx warning. Verified via `dotnet build libraries/MTConnect.NET-Common/MTConnect.NET-Common.csproj -c Debug -p:GenerateDocumentationFile=true --no-incremental` (0 warnings, 0 errors) and via the full `docs/scripts/generate-api-ref.sh` run (docfx metadata now completes without the InvalidCref warning). --- libraries/MTConnect.NET-Common/Devices/Device.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/MTConnect.NET-Common/Devices/Device.cs b/libraries/MTConnect.NET-Common/Devices/Device.cs index e070b2b6c..db1ad5e27 100644 --- a/libraries/MTConnect.NET-Common/Devices/Device.cs +++ b/libraries/MTConnect.NET-Common/Devices/Device.cs @@ -1043,7 +1043,7 @@ public void AddDataItems(IEnumerable dataItems) /// nested child Component. The override previously skipped the Device's own /// DataItems collection (walking only child Components), so a DataItem added /// directly to a Device was unremovable — invisible to - /// 's Remove branch. + /// MTConnectAgent.NormalizeDevice's Remove branch. /// /// The ID of the DataItem to remove public void RemoveDataItem(string dataItemId) From 19b1def01e530655792dc514ee5906671b188b0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Otto=20Boly=C3=B3s?= Date: Thu, 20 Aug 2026 23:54:30 +0200 Subject: [PATCH 11/34] test(common): pin DVL setter, Normalize, and Remove-sibling coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fills coverage-FLOOR gaps surfaced by the cycle-2 test-audit for PR 241: * InputValidationLevel setter positive-arm coverage (parallel to the existing DeviceValidationLevel positive-arm test). * DeviceValidationLevel + InputValidationLevel setter boundary coverage at -1, 4, int.MinValue, int.MaxValue. Previously only 42 and 99 were pinned — a regression that swapped Enum.IsDefined for a permissive `value <= Strict` guard would slip past both. * Setter exception-shape pins (ParamName + ActualValue + Message) — callers depend on the diagnostic tuple the current OOR shape carries. * Direct Normalize() mirror across every InputValidationLevel arm (was Remove-only) plus explicit-DVL-then-later-IVL latch stickiness and cross-arm sticky-suppression grid. * Normalize() idempotency — second Normalize is a stable no-op. * YAML load path Normalize mirror pins (parallel to the JSON tests) — the docstring names ReadYaml as an invocation site but no YAML test existed. * Device.RemoveComposition + RemoveDataItem sibling isolation, depth-2 intermediate branch, and duplicate-ID-at-every-depth pins so a regression to a first-match-return or permissive predicate fails loudly on the ID-scoping shape. 33 net-new tests. Bluefin: 98 passed / 0 failed on the four DVL / DeviceRemove categories. --- .../DeviceValidationLevelMigrationTests.cs | 261 ++++++++++++++++++ .../Devices/DeviceRemoveRecursionTests.cs | 172 ++++++++++++ 2 files changed, 433 insertions(+) diff --git a/tests/MTConnect.NET-Common-Tests/Configurations/DeviceValidationLevelMigrationTests.cs b/tests/MTConnect.NET-Common-Tests/Configurations/DeviceValidationLevelMigrationTests.cs index 006bd299f..f4017daf0 100644 --- a/tests/MTConnect.NET-Common-Tests/Configurations/DeviceValidationLevelMigrationTests.cs +++ b/tests/MTConnect.NET-Common-Tests/Configurations/DeviceValidationLevelMigrationTests.cs @@ -186,6 +186,260 @@ public void DeviceValidationLevel_Setter_Accepts_Every_Defined_Arm(DeviceValidat Assert.That(config.DeviceValidationLevel, Is.EqualTo(arm)); } + /// + /// Positive-arm coverage FLOOR for . + /// The sibling assertion for DeviceValidationLevel above ensured no + /// false-positive rejections; the parallel InputValidationLevel + /// setter uses the same Enum.IsDefined guard (AgentConfiguration.cs:176) + /// and must be pinned identically so a regression that widens or narrows + /// the accepted arm set on the input axis fails loudly. + /// + [TestCase(InputValidationLevel.Ignore)] + [TestCase(InputValidationLevel.Warning)] + [TestCase(InputValidationLevel.Remove)] + [TestCase(InputValidationLevel.Strict)] + public void InputValidationLevel_Setter_Accepts_Every_Defined_Arm(InputValidationLevel arm) + { + var config = new AgentConfiguration(); + Assert.DoesNotThrow(() => config.InputValidationLevel = arm); + Assert.That(config.InputValidationLevel, Is.EqualTo(arm)); + } + + /// + /// Boundary coverage FLOOR for the setter guard + /// (AgentConfiguration.cs:151). The setter uses + /// which rejects EVERY ordinal that is not a defined arm — the surviving valid arms + /// are 0..3. Pin the four boundary shapes the FLOOR names — negative one, first + /// invalid over-max ordinal, and the two integer extremes — so a regression that + /// swaps IsDefined for a permissive range check (for example value <= Strict, + /// which would accept -1) fails on the first case rather than sneaking past the + /// existing single (42) coverage. + /// + [TestCase(-1)] + [TestCase(4)] + [TestCase(int.MinValue)] + [TestCase(int.MaxValue)] + public void DeviceValidationLevel_Setter_Rejects_Out_Of_Range_Boundary(int ordinal) + { + var config = new AgentConfiguration(); + Assert.Throws( + () => config.DeviceValidationLevel = (DeviceValidationLevel)ordinal, + $"DeviceValidationLevel setter must reject ordinal {ordinal} — Enum.IsDefined guard is strict against every value outside 0..3."); + } + + /// + /// Boundary coverage FLOOR for the setter guard + /// (AgentConfiguration.cs:176). Same four shapes as the DeviceValidationLevel + /// boundary — the guard is textually identical and must stay strict on both axes. + /// + [TestCase(-1)] + [TestCase(4)] + [TestCase(int.MinValue)] + [TestCase(int.MaxValue)] + public void InputValidationLevel_Setter_Rejects_Out_Of_Range_Boundary(int ordinal) + { + var config = new AgentConfiguration(); + Assert.Throws( + () => config.InputValidationLevel = (InputValidationLevel)ordinal, + $"InputValidationLevel setter must reject ordinal {ordinal} — Enum.IsDefined guard is strict against every value outside 0..3."); + } + + /// + /// Pins the setter exception shape — paramName is "value" and the + /// ActualValue carries the offending enum ordinal so callers logging the + /// exception get the diagnostic. A regression that throws a bare + /// ArgumentException (dropping the paramName / actual-value tuple) would + /// still satisfy the coarser Assert.Throws<ArgumentOutOfRangeException> + /// gates above only because ArgumentOutOfRangeException derives from + /// ArgumentException — but the message shape carrying the two documented pieces of + /// diagnostic is a contract callers depend on. This test pins that shape. + /// + [Test] + public void DeviceValidationLevel_Setter_Exception_Carries_ParamName_And_ActualValue() + { + var config = new AgentConfiguration(); + var ex = Assert.Throws( + () => config.DeviceValidationLevel = (DeviceValidationLevel)99); + + Assert.That(ex!.ParamName, Is.EqualTo("value"), + "paramName is a documented invariant — subscribers log it verbatim."); + Assert.That(ex.ActualValue, Is.EqualTo((DeviceValidationLevel)99), + "ActualValue carries the offending ordinal so the diagnostic names the caller's mistake."); + Assert.That(ex.Message, Does.Contain("DeviceValidationLevel"), + "Message must name the enum so subscribers can distinguish DVL vs IVL failures."); + } + + /// Same exception-shape pin as above, but for . + [Test] + public void InputValidationLevel_Setter_Exception_Carries_ParamName_And_ActualValue() + { + var config = new AgentConfiguration(); + var ex = Assert.Throws( + () => config.InputValidationLevel = (InputValidationLevel)99); + + Assert.That(ex!.ParamName, Is.EqualTo("value")); + Assert.That(ex.ActualValue, Is.EqualTo((InputValidationLevel)99)); + Assert.That(ex.Message, Does.Contain("InputValidationLevel")); + } + + // --------------------------------------------------------------- + // Direct Normalize() — every arm, not just Remove + // --------------------------------------------------------------- + + /// + /// Pins the programmatic- mirror across + /// every arm — not just Remove as the + /// original single test covered. The ReadJson-driven parametrised test + /// above exercises the mirror THROUGH the loader; this test exercises the mirror + /// DIRECTLY so a regression that skips the mirror on a specific arm (for example + /// an off-by-one enum-cast bug producing wrong ordinals for arms 0 or 3) fails + /// on the pinned arm rather than being masked by the ordinal happening to align. + /// + [TestCase(InputValidationLevel.Ignore, DeviceValidationLevel.Ignore)] + [TestCase(InputValidationLevel.Warning, DeviceValidationLevel.Warning)] + [TestCase(InputValidationLevel.Remove, DeviceValidationLevel.Remove)] + [TestCase(InputValidationLevel.Strict, DeviceValidationLevel.Strict)] + public void Normalize_Mirrors_Every_InputValidationLevel_Arm( + InputValidationLevel input, + DeviceValidationLevel expectedMirrored) + { + var config = new AgentConfiguration(); + config.InputValidationLevel = input; + + config.Normalize(); + + Assert.That(config.DeviceValidationLevel, Is.EqualTo(expectedMirrored), + $"Normalize must mirror InputValidationLevel.{input} onto DeviceValidationLevel.{expectedMirrored} because DeviceValidationLevel was never assigned explicitly."); + } + + /// + /// Pins Normalize idempotency: calling + /// a second time is a stable no-op — the DeviceValidationLevel value is the + /// mirrored value, not the ctor default. A regression that reset + /// _isDeviceValidationLevelExplicit inside Normalize (making the + /// mirror re-fire) would silently overwrite a subsequent explicit assignment; + /// pinning idempotency catches that class of bug. + /// + [Test] + public void Normalize_Is_Idempotent_On_Repeat_Calls() + { + var config = new AgentConfiguration(); + config.InputValidationLevel = InputValidationLevel.Strict; + + config.Normalize(); + Assert.That(config.DeviceValidationLevel, Is.EqualTo(DeviceValidationLevel.Strict), + "first Normalize mirrors the input axis onto the device axis."); + + config.Normalize(); + Assert.That(config.DeviceValidationLevel, Is.EqualTo(DeviceValidationLevel.Strict), + "second Normalize must be a stable no-op — the mirrored value must not regress to the ctor default."); + } + + /// + /// Pins that setting + /// AFTER an explicit + /// assignment does NOT re-arm the mirror. The + /// _isDeviceValidationLevelExplicit flag is set by the DVL setter + /// and never cleared by the IVL setter — a caller who explicitly set DVL + /// then later set IVL must not have DVL silently overwritten on the next + /// Normalize call. + /// + [Test] + public void Normalize_Explicit_DeviceValidationLevel_Then_Later_InputValidationLevel_Assignment_Does_Not_Rearm_Mirror() + { + var config = new AgentConfiguration(); + config.DeviceValidationLevel = DeviceValidationLevel.Ignore; + // The DVL setter marked _isDeviceValidationLevelExplicit=true. Later IVL assignment + // must not clear that latch. + config.InputValidationLevel = InputValidationLevel.Strict; + + config.Normalize(); + + Assert.That(config.DeviceValidationLevel, Is.EqualTo(DeviceValidationLevel.Ignore), + "later InputValidationLevel assignment must not re-arm the mirror — the explicit-DVL latch is sticky."); + } + + /// + /// Pins sticky suppression across every arm — + /// the existing single-arm sticky-suppression test only covered + /// Strict → Ignore. A regression that clears _isDeviceValidationLevelExplicit + /// only on a specific arm (for example resetting the flag on the ctor default + /// arm) would slip past the single existing test. + /// + [TestCase(DeviceValidationLevel.Ignore)] + [TestCase(DeviceValidationLevel.Warning)] + [TestCase(DeviceValidationLevel.Remove)] + [TestCase(DeviceValidationLevel.Strict)] + public void Normalize_Sticky_Suppression_Holds_For_Every_Explicit_Arm(DeviceValidationLevel explicitArm) + { + var config = new AgentConfiguration(); + // Pick an IVL arm that is DIFFERENT from the explicit DVL arm so the mirror + // path would corrupt DVL if the latch failed. Both enums share ordinals so + // the cast is meaningful. + config.InputValidationLevel = explicitArm == DeviceValidationLevel.Strict + ? InputValidationLevel.Ignore + : InputValidationLevel.Strict; + config.DeviceValidationLevel = explicitArm; + + config.Normalize(); + + Assert.That(config.DeviceValidationLevel, Is.EqualTo(explicitArm), + $"the explicit-DVL latch must stick for arm {explicitArm} regardless of the divergent IVL value."); + } + + // --------------------------------------------------------------- + // YAML load path — parallel to ReadJson mirror + // --------------------------------------------------------------- + + /// + /// Pins the YAML load path invokes — + /// the docstring on Normalize lists ReadYaml as an invocation site + /// (AgentConfiguration.cs:240) but the existing fixture only exercises the JSON + /// path. A regression that dropped the configuration.Normalize() call from + /// ReadYaml (AgentConfiguration.cs:439) would silently downgrade YAML-loading + /// consumers to the ctor-default DeviceValidationLevel on every arm change of + /// InputValidationLevel — a diagnostic-silent behaviour break. + /// + [Test] + public void ReadYaml_InputValidationLevel_Only_Mirrors_Onto_DeviceValidationLevel() + { + var path = WriteTempYaml("inputValidationLevel: 3\n"); + try + { + var config = AgentConfiguration.ReadYaml(path); + + Assert.That(config, Is.Not.Null, "the YAML loader must not have swallowed the config."); + Assert.That(config!.InputValidationLevel, Is.EqualTo(InputValidationLevel.Strict)); + Assert.That(config.DeviceValidationLevel, Is.EqualTo(DeviceValidationLevel.Strict), + "the YAML load path must invoke Normalize so pre-split consumers keep the mirrored DeviceValidationLevel."); + } + finally + { + File.Delete(path); + } + } + + /// Pins that YAML explicit deviceValidationLevel beats the mirror — sister of the JSON test. + [Test] + public void ReadYaml_Explicit_DeviceValidationLevel_Beats_Mirror() + { + var path = WriteTempYaml( + "inputValidationLevel: 3\ndeviceValidationLevel: 0\n"); + try + { + var config = AgentConfiguration.ReadYaml(path); + + Assert.That(config, Is.Not.Null); + Assert.That(config!.InputValidationLevel, Is.EqualTo(InputValidationLevel.Strict)); + Assert.That(config.DeviceValidationLevel, Is.EqualTo(DeviceValidationLevel.Ignore), + "an explicit deviceValidationLevel key in YAML must NOT be silently overwritten by the mirror."); + } + finally + { + File.Delete(path); + } + } + // --------------------------------------------------------------- // Fixture harness // --------------------------------------------------------------- @@ -196,5 +450,12 @@ private static string WriteTempJson(string json) File.WriteAllText(path, json); return path; } + + private static string WriteTempYaml(string yaml) + { + var path = Path.Combine(Path.GetTempPath(), $"agent-config-{Guid.NewGuid():N}.yaml"); + File.WriteAllText(path, yaml); + return path; + } } } diff --git a/tests/MTConnect.NET-Common-Tests/Devices/DeviceRemoveRecursionTests.cs b/tests/MTConnect.NET-Common-Tests/Devices/DeviceRemoveRecursionTests.cs index 846ade30b..594c683cf 100644 --- a/tests/MTConnect.NET-Common-Tests/Devices/DeviceRemoveRecursionTests.cs +++ b/tests/MTConnect.NET-Common-Tests/Devices/DeviceRemoveRecursionTests.cs @@ -216,5 +216,177 @@ public void RemoveDataItem_on_empty_device_tree_is_safe() Assert.DoesNotThrow(() => device.RemoveDataItem("nothing"), "RemoveDataItem must exit safely when the Device has neither DataItems nor Components."); } + + // -------------------------------------------------------------- + // Sibling isolation — the recursive RemoveAll(o => o.Id == id) + // predicate must only match the requested ID and leave every + // sibling untouched. A regression to a permissive predicate + // (for example RemoveAll(o => true) inside a wrong overload) + // would strip every sibling and would pass the single-child + // tests above; the sibling-isolation shape catches that class. + // -------------------------------------------------------------- + + /// + /// Pins that called against + /// one of two siblings on the top-level Device.Compositions + /// collection drops only the requested Composition and leaves the + /// sibling intact. + /// + [Test] + public void RemoveComposition_top_level_leaves_sibling_Compositions_intact() + { + var device = new Device { Id = "d1", Uuid = "d1", Name = "d1", Type = Device.TypeId }; + device.AddComposition(new Composition { Id = "drop-me", Name = "drop-me", Type = "Generic" }); + device.AddComposition(new Composition { Id = "keep-me", Name = "keep-me", Type = "Generic" }); + + device.RemoveComposition("drop-me"); + + Assert.That(device.Compositions.Any(c => c.Id == "drop-me"), Is.False, + "RemoveComposition must drop the requested top-level Composition."); + Assert.That(device.Compositions.Any(c => c.Id == "keep-me"), Is.True, + "RemoveComposition must not drop siblings of the requested Composition — the RemoveAll predicate is ID-scoped."); + } + + /// + /// Pins the same sibling-isolation invariant on the recursive + /// nested branch: two Compositions attached to a child Component, + /// only one requested for removal. + /// + [Test] + public void RemoveComposition_nested_leaves_sibling_Compositions_intact() + { + var device = new Device { Id = "d1", Uuid = "d1", Name = "d1", Type = Device.TypeId }; + var host = new Component { Id = "host", Name = "host", Type = "Axes" }; + host.AddComposition(new Composition { Id = "drop-nested", Name = "drop-nested", Type = "Generic" }); + host.AddComposition(new Composition { Id = "keep-nested", Name = "keep-nested", Type = "Generic" }); + device.AddComponent(host); + + device.RemoveComposition("drop-nested"); + + Assert.That(device.GetCompositions().Any(c => c.Id == "drop-nested"), Is.False); + Assert.That(device.GetCompositions().Any(c => c.Id == "keep-nested"), Is.True, + "the nested recursion must not drop siblings."); + } + + /// + /// Pins that a Composition with the same ID present at BOTH the + /// top-level Device.Compositions AND on a nested child Component + /// is dropped from both locations — RemoveComposition is depth- + /// unlimited, not first-match. This pins the recursion contract + /// against a regression that returns on the first match. + /// + [Test] + public void RemoveComposition_removes_id_from_every_depth_it_appears() + { + var device = new Device { Id = "d1", Uuid = "d1", Name = "d1", Type = Device.TypeId }; + device.AddComposition(new Composition { Id = "dup-id", Name = "dup-id-top", Type = "Generic" }); + var host = new Component { Id = "host", Name = "host", Type = "Axes" }; + host.AddComposition(new Composition { Id = "dup-id", Name = "dup-id-nested", Type = "Generic" }); + device.AddComponent(host); + + Assert.That(device.GetCompositions().Count(c => c.Id == "dup-id"), Is.EqualTo(2), + "precondition — the same ID must be present at both depths."); + + device.RemoveComposition("dup-id"); + + var remaining = device.GetCompositions(); + Assert.That(remaining == null || !remaining.Any(c => c.Id == "dup-id"), Is.True, + "RemoveComposition must remove EVERY occurrence of the ID — top-level AND nested — not just the first match."); + } + + /// + /// Pins that called against one of + /// two siblings on the top-level Device.DataItems collection + /// drops only the requested DataItem and leaves the sibling intact. + /// + [Test] + public void RemoveDataItem_top_level_leaves_sibling_DataItems_intact() + { + var device = new Device { Id = "d1", Uuid = "d1", Name = "d1", Type = Device.TypeId }; + device.AddDataItem(new DataItem { Id = "drop-me", Type = "Generic", Category = DataItemCategory.EVENT }); + device.AddDataItem(new DataItem { Id = "keep-me", Type = "Generic", Category = DataItemCategory.EVENT }); + + device.RemoveDataItem("drop-me"); + + Assert.That(device.DataItems.Any(d => d.Id == "drop-me"), Is.False, + "RemoveDataItem must drop the requested top-level DataItem."); + Assert.That(device.DataItems.Any(d => d.Id == "keep-me"), Is.True, + "RemoveDataItem must not drop siblings of the requested DataItem."); + } + + /// + /// Pins the same sibling-isolation invariant for the nested branch: + /// two DataItems attached to a child Component, only one requested + /// for removal. + /// + [Test] + public void RemoveDataItem_nested_leaves_sibling_DataItems_intact() + { + var device = new Device { Id = "d1", Uuid = "d1", Name = "d1", Type = Device.TypeId }; + var host = new Component { Id = "host", Name = "host", Type = "Axes" }; + host.AddDataItem(new DataItem { Id = "drop-nested", Type = "Generic", Category = DataItemCategory.EVENT }); + host.AddDataItem(new DataItem { Id = "keep-nested", Type = "Generic", Category = DataItemCategory.EVENT }); + device.AddComponent(host); + + device.RemoveDataItem("drop-nested"); + + Assert.That(device.GetDataItems().Any(d => d.Id == "drop-nested"), Is.False); + Assert.That(device.GetDataItems().Any(d => d.Id == "keep-nested"), Is.True, + "nested-branch removal must not drop siblings."); + } + + /// + /// Pins that a DataItem ID present at BOTH the top-level + /// Device.DataItems AND on a nested child Component is dropped + /// from both locations — RemoveDataItem visits every depth, + /// not just the first match. + /// + [Test] + public void RemoveDataItem_removes_id_from_every_depth_it_appears() + { + var device = new Device { Id = "d1", Uuid = "d1", Name = "d1", Type = Device.TypeId }; + device.AddDataItem(new DataItem { Id = "dup-id", Type = "Generic", Category = DataItemCategory.EVENT }); + var host = new Component { Id = "host", Name = "host", Type = "Axes" }; + host.AddDataItem(new DataItem { Id = "dup-id", Type = "Generic", Category = DataItemCategory.EVENT }); + device.AddComponent(host); + + Assert.That(device.GetDataItems().Count(d => d.Id == "dup-id"), Is.EqualTo(2), + "precondition — the same ID must be present at both depths."); + + device.RemoveDataItem("dup-id"); + + var remaining = device.GetDataItems(); + Assert.That(remaining == null || !remaining.Any(d => d.Id == "dup-id"), Is.True, + "RemoveDataItem must remove EVERY occurrence of the ID — top-level AND nested."); + } + + /// + /// Pins the intermediate depth-2 Composition-removal branch that + /// sits BETWEEN the top-level (depth-1) and great-grandchild + /// (depth-3) coverage above: Device → Component → Composition + /// directly on the child Component. This is the shape the + /// original NormalizeDevice-Remove path used before the recursive + /// fix; a regression to a two-level walk would still pass the + /// depth-1 top-level test and might pass the depth-3 test via a + /// different code path — the depth-2 test pins the exact single + /// level of recursion. + /// + [Test] + public void RemoveComposition_removes_depth_2_Composition_on_direct_child_Component() + { + var device = new Device { Id = "d1", Uuid = "d1", Name = "d1", Type = Device.TypeId }; + var child = new Component { Id = "child", Name = "child", Type = "Axes" }; + child.AddComposition(new Composition { Id = "mid-comp", Name = "mid-comp", Type = "Generic" }); + device.AddComponent(child); + + Assert.That(device.GetCompositions().Any(c => c.Id == "mid-comp"), Is.True, + "precondition — the depth-2 Composition must be reachable via GetCompositions."); + + device.RemoveComposition("mid-comp"); + + var remaining = device.GetCompositions(); + Assert.That(remaining == null || !remaining.Any(c => c.Id == "mid-comp"), Is.True, + "RemoveComposition must reach depth-2 Compositions attached to a direct child Component."); + } } } From 0c72beb5f413e71739f16e625064627f1f6836e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Otto=20Boly=C3=B3s?= Date: Fri, 21 Aug 2026 00:16:26 +0200 Subject: [PATCH 12/34] =?UTF-8?q?fix(devices):=20cycle-guard=20Remove=20tr?= =?UTF-8?q?ee=20walks=20=E2=80=94=20dime=20H1=20+=20M2?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a HashSet visited-Id cycle guard threaded through every recursive walk under Device.RemoveComposition, Device.RemoveDataItem, Component.RemoveComposition and Component.RemoveDataItem, with a belt-and-braces depth ceiling at 1024. A cyclic Component graph (A.Components ∋ B, B.Components ∋ A) previously walked the recursion until the process stack exhausted; the guard terminates the walk after every node's Id is visited once. Rewrite Component.RemoveComposition to recurse across nested child Components (the sibling site the audit brief M2 called out as still non-recursive after PR #219's Device.cs recursion fix) and rewrite Component.RemoveDataItem to walk children inline instead of routing through the unguarded Component.GetComponents() flatten, so the cycle guard applies uniformly to the DataItem path too. Delete the zero-caller private RemoveComposition(IComponent, string) helper on Component (the append-duplicates AddCompositions anti-pattern from before PR #219). Test: DeviceRemoveRecursionTests gains four cycle-guard fixtures (RemoveComposition_terminates_on_cyclic_Component_graph, RemoveDataItem_terminates_on_cyclic_Component_graph, Component_RemoveComposition_reaches_nested_and_terminates_on_cycle, Component_RemoveDataItem_terminates_on_cyclic_Component_graph) pinning the observable termination guarantee — a regression to unguarded recursion fails as a StackOverflowException. Refs: dime Ultrareview cycle-1 findings H1 (bug-detector + security A04) and M2 (bug-class atomicity per CONVENTIONS §1.0d-trigies-bis). --- .../MTConnect.NET-Common/Devices/Component.cs | 99 +++++++++++++--- .../MTConnect.NET-Common/Devices/Device.cs | 76 +++++++++--- .../Devices/DeviceRemoveRecursionTests.cs | 112 ++++++++++++++++++ 3 files changed, 256 insertions(+), 31 deletions(-) diff --git a/libraries/MTConnect.NET-Common/Devices/Component.cs b/libraries/MTConnect.NET-Common/Devices/Component.cs index d9d80a579..67aa4f9bc 100644 --- a/libraries/MTConnect.NET-Common/Devices/Component.cs +++ b/libraries/MTConnect.NET-Common/Devices/Component.cs @@ -704,11 +704,23 @@ public void AddCompositions(IEnumerable compositions) /// - /// Remove a Composition from the Composition + /// Maximum recursion depth for Component-tree walks — belt-and-braces + /// guard alongside the visited-Id cycle set. Matches the ceiling on the + /// override. + /// + private const int MaxComponentWalkDepth = 1024; + + /// + /// Remove a Composition from this Component's Compositions collection and + /// every nested child Component's collection. The traversal carries a + /// visited-Id cycle guard so a cyclic Component graph + /// (A.Components ∋ B, B.Components ∋ A) terminates instead of + /// stack-overflowing. /// /// The ID of the Composition to remove public void RemoveComposition(string compositionId) { + // Self Compositions. if (!Compositions.IsNullOrEmpty()) { var compositions = new List(); @@ -717,17 +729,47 @@ public void RemoveComposition(string compositionId) Compositions = compositions; } + + // Nested Compositions on every child Component (recursive). + if (!Components.IsNullOrEmpty()) + { + var visitedIds = new HashSet(StringComparer.Ordinal); + // Seed the visited set with this component's own Id so a cycle + // back to `this` terminates immediately. + if (!string.IsNullOrEmpty(Id)) visitedIds.Add(Id); + foreach (var component in Components) + { + RemoveComposition(component, compositionId, visitedIds, depth: 1); + } + } } - private void RemoveComposition(IComponent component, string compositionId) + private void RemoveComposition(IComponent component, string compositionId, HashSet visitedIds, int depth) { - if (component != null && !component.Compositions.IsNullOrEmpty()) + if (component == null) return; + if (depth > MaxComponentWalkDepth) return; + + // Cycle guard: skip re-entry for a Component whose Id we've already + // processed on this walk. + if (!string.IsNullOrEmpty(component.Id) && !visitedIds.Add(component.Id)) return; + + if (!component.Compositions.IsNullOrEmpty()) { var compositions = new List(); compositions.AddRange(component.Compositions); compositions.RemoveAll(o => o.Id == compositionId); - ((Component)component).AddCompositions(compositions); + // Replace outright rather than AddCompositions (which would append + // duplicates); the local list already contains the survivors. + ((Component)component).Compositions = compositions; + } + + if (!component.Components.IsNullOrEmpty()) + { + foreach (var subComponent in component.Components) + { + RemoveComposition(subComponent, compositionId, visitedIds, depth + 1); + } } } @@ -1066,7 +1108,12 @@ public void AddDataItems(IEnumerable dataItems) /// - /// Remove a DataItem from the Component + /// Remove a DataItem from the Component's own DataItems collection and + /// every nested child Component's collection. Uses an inline recursive + /// walk (not the previous flatten) so it + /// can carry the same visited-Id cycle guard as + /// ; a cyclic Component graph + /// terminates instead of stack-overflowing. /// /// The ID of the DataItem to remove public void RemoveDataItem(string dataItemId) @@ -1079,18 +1126,40 @@ public void RemoveDataItem(string dataItemId) DataItems = dataItems; } - var components = GetComponents(); - if (!components.IsNullOrEmpty()) + if (!Components.IsNullOrEmpty()) { - foreach (var component in components) + var visitedIds = new HashSet(StringComparer.Ordinal); + // Seed the visited set with this component's own Id so a cycle + // back to `this` terminates immediately. + if (!string.IsNullOrEmpty(Id)) visitedIds.Add(Id); + foreach (var component in Components) { - if (!component.DataItems.IsNullOrEmpty()) - { - var dataItems = new List(); - dataItems.AddRange(component.DataItems); - dataItems.RemoveAll(o => o.Id == dataItemId); - component.DataItems = dataItems; - } + RemoveDataItem(component, dataItemId, visitedIds, depth: 1); + } + } + } + + private void RemoveDataItem(IComponent component, string dataItemId, HashSet visitedIds, int depth) + { + if (component == null) return; + if (depth > MaxComponentWalkDepth) return; + + // Cycle guard — mirrors RemoveComposition. + if (!string.IsNullOrEmpty(component.Id) && !visitedIds.Add(component.Id)) return; + + if (!component.DataItems.IsNullOrEmpty()) + { + var dataItems = new List(); + dataItems.AddRange(component.DataItems); + dataItems.RemoveAll(o => o.Id == dataItemId); + component.DataItems = dataItems; + } + + if (!component.Components.IsNullOrEmpty()) + { + foreach (var subComponent in component.Components) + { + RemoveDataItem(subComponent, dataItemId, visitedIds, depth + 1); } } } diff --git a/libraries/MTConnect.NET-Common/Devices/Device.cs b/libraries/MTConnect.NET-Common/Devices/Device.cs index db1ad5e27..476833fe7 100644 --- a/libraries/MTConnect.NET-Common/Devices/Device.cs +++ b/libraries/MTConnect.NET-Common/Devices/Device.cs @@ -657,11 +657,24 @@ public void AddCompositions(IEnumerable compositions) } + /// + /// Maximum recursion depth for Component-tree walks — belt-and-braces + /// guard alongside the visited-Id cycle set. A well-formed MTConnect + /// device model is nowhere near this deep; hitting the ceiling implies + /// either a pathological synthetic input or a cyclic model that the + /// visited-Id set failed to catch (for example, every component in the + /// cycle carrying a null Id). + /// + private const int MaxComponentWalkDepth = 1024; + /// /// Remove a Composition from the Device tree — top-level and every nested /// child Component. Mirrors the recursive shape of /// so that a Composition located via the recursive - /// pathway is actually removed regardless of depth. + /// pathway is actually removed regardless of depth. The traversal carries + /// a visited-Id cycle guard so a cyclic Component graph + /// (A.Components ∋ B, B.Components ∋ A) terminates instead of + /// stack-overflowing. /// /// The ID of the Composition to remove public void RemoveComposition(string compositionId) @@ -679,16 +692,24 @@ public void RemoveComposition(string compositionId) // Nested Compositions on every child Component (recursive). if (!Components.IsNullOrEmpty()) { + var visitedIds = new HashSet(StringComparer.Ordinal); foreach (var component in Components) { - RemoveComposition(component, compositionId); + RemoveComposition(component, compositionId, visitedIds, depth: 1); } } } - private void RemoveComposition(IComponent component, string compositionId) + private void RemoveComposition(IComponent component, string compositionId, HashSet visitedIds, int depth) { if (component == null) return; + if (depth > MaxComponentWalkDepth) return; + + // Cycle guard: skip re-entry for a Component whose Id we've already + // processed on this walk. Null-Id components fall through the guard + // (nothing to add to the set) but the depth ceiling still terminates + // pathological cyclic-null-Id chains. + if (!string.IsNullOrEmpty(component.Id) && !visitedIds.Add(component.Id)) return; if (!component.Compositions.IsNullOrEmpty()) { @@ -707,7 +728,7 @@ private void RemoveComposition(IComponent component, string compositionId) { foreach (var subComponent in component.Components) { - RemoveComposition(subComponent, compositionId); + RemoveComposition(subComponent, compositionId, visitedIds, depth + 1); } } } @@ -1043,7 +1064,11 @@ public void AddDataItems(IEnumerable dataItems) /// nested child Component. The override previously skipped the Device's own /// DataItems collection (walking only child Components), so a DataItem added /// directly to a Device was unremovable — invisible to - /// MTConnectAgent.NormalizeDevice's Remove branch. + /// MTConnectAgent.NormalizeDevice's Remove branch. The traversal uses + /// an inline recursive walk (not the previous + /// flatten) so it can carry the same visited-Id cycle guard as + /// ; a cyclic Component graph + /// terminates instead of stack-overflowing. /// /// The ID of the DataItem to remove public void RemoveDataItem(string dataItemId) @@ -1058,19 +1083,38 @@ public void RemoveDataItem(string dataItemId) DataItems = dataItems; } - // Child Components' DataItems. - var components = GetComponents(); - if (!components.IsNullOrEmpty()) + // Child Components' DataItems, walked with an explicit cycle guard. + if (!Components.IsNullOrEmpty()) { - foreach (var component in components) + var visitedIds = new HashSet(StringComparer.Ordinal); + foreach (var component in Components) { - if (!component.DataItems.IsNullOrEmpty()) - { - var dataItems = new List(); - dataItems.AddRange(component.DataItems); - dataItems.RemoveAll(o => o.Id == dataItemId); - component.DataItems = dataItems; - } + RemoveDataItem(component, dataItemId, visitedIds, depth: 1); + } + } + } + + private void RemoveDataItem(IComponent component, string dataItemId, HashSet visitedIds, int depth) + { + if (component == null) return; + if (depth > MaxComponentWalkDepth) return; + + // Cycle guard — mirrors RemoveComposition. + if (!string.IsNullOrEmpty(component.Id) && !visitedIds.Add(component.Id)) return; + + if (!component.DataItems.IsNullOrEmpty()) + { + var dataItems = new List(); + dataItems.AddRange(component.DataItems); + dataItems.RemoveAll(o => o.Id == dataItemId); + component.DataItems = dataItems; + } + + if (!component.Components.IsNullOrEmpty()) + { + foreach (var subComponent in component.Components) + { + RemoveDataItem(subComponent, dataItemId, visitedIds, depth + 1); } } } diff --git a/tests/MTConnect.NET-Common-Tests/Devices/DeviceRemoveRecursionTests.cs b/tests/MTConnect.NET-Common-Tests/Devices/DeviceRemoveRecursionTests.cs index 594c683cf..8033d5813 100644 --- a/tests/MTConnect.NET-Common-Tests/Devices/DeviceRemoveRecursionTests.cs +++ b/tests/MTConnect.NET-Common-Tests/Devices/DeviceRemoveRecursionTests.cs @@ -1,6 +1,7 @@ // Copyright (c) 2026 TrakHound Inc., All Rights Reserved. // TrakHound Inc. licenses this file to you under the MIT license. +using System.Collections.Generic; using System.Linq; using MTConnect.Devices; using MTConnect.Devices.DataItems; @@ -388,5 +389,116 @@ public void RemoveComposition_removes_depth_2_Composition_on_direct_child_Compon Assert.That(remaining == null || !remaining.Any(c => c.Id == "mid-comp"), Is.True, "RemoveComposition must reach depth-2 Compositions attached to a direct child Component."); } + + // -------------------------------------------------------------- + // Cycle-guard coverage — a cyclic Component graph (A→B→A) must + // terminate the recursive walk instead of stack-overflowing. Two + // paths carry a cycle guard: RemoveComposition and RemoveDataItem + // on both Device and Component. Each fixture below exercises one + // path directly; a regression to the unguarded pre-fix shape + // fails as a `StackOverflowException` (process abort), which + // NUnit surfaces as fixture-level failure. + // -------------------------------------------------------------- + + /// + /// Pins that terminates on a + /// cyclic Component graph. The audit brief (cycle-1 finding H1) required a + /// visited-Id set threaded through the recursive walk so a + /// A.Components ∋ B, B.Components ∋ A shape does not + /// stack-overflow the process. + /// + [Test] + public void RemoveComposition_terminates_on_cyclic_Component_graph() + { + var device = new Device { Id = "d1", Uuid = "d1", Name = "d1", Type = Device.TypeId }; + var a = new Component { Id = "A", Name = "A", Type = "Axes" }; + var b = new Component { Id = "B", Name = "B", Type = "Axes" }; + device.AddComponent(a); + a.AddComponent(b); + // Force the cycle by direct assignment — AddComponent would reset Parent + // linkages but the recursive walk only reads .Components. + b.Components = new List { a }; + + // A no-op removal (nothing to remove) must still traverse the cyclic graph + // to exhaustion without recursing forever. + Assert.DoesNotThrow(() => device.RemoveComposition("missing"), + "Device.RemoveComposition must terminate on a cyclic Component graph — no StackOverflowException."); + } + + /// + /// Pins that terminates on a + /// cyclic Component graph. Sibling of the RemoveComposition cycle test — + /// the inline recursive walk replaces the previous + /// flatten, which was itself unguarded. + /// + [Test] + public void RemoveDataItem_terminates_on_cyclic_Component_graph() + { + var device = new Device { Id = "d1", Uuid = "d1", Name = "d1", Type = Device.TypeId }; + var a = new Component { Id = "A", Name = "A", Type = "Axes" }; + var b = new Component { Id = "B", Name = "B", Type = "Axes" }; + device.AddComponent(a); + a.AddComponent(b); + b.Components = new List { a }; + + Assert.DoesNotThrow(() => device.RemoveDataItem("missing"), + "Device.RemoveDataItem must terminate on a cyclic Component graph — no StackOverflowException."); + } + + /// + /// Pins that is now + /// recursive across nested child Components AND terminates on a cyclic + /// graph — the audit brief finding M2 called out this sibling site as + /// still non-recursive after the Device.cs fix. A regression that + /// re-strips the recursive walk fails the depth-2 removal; a regression + /// that reintroduces the walk without the cycle guard fails the + /// termination assertion. + /// + [Test] + public void Component_RemoveComposition_reaches_nested_and_terminates_on_cycle() + { + var root = new Component { Id = "root", Name = "root", Type = "Axes" }; + var child = new Component { Id = "child", Name = "child", Type = "Axes" }; + child.AddComposition(new Composition { Id = "nested-comp", Name = "nested-comp", Type = "Generic" }); + root.AddComponent(child); + + root.RemoveComposition("nested-comp"); + + Assert.That( + child.Compositions == null || !child.Compositions.Any(c => c.Id == "nested-comp"), + Is.True, + "Component.RemoveComposition must reach a Composition nested on a direct child Component."); + + // Cycle graph: root→cycleA→cycleB→cycleA + var cycleA = new Component { Id = "cycleA", Name = "cycleA", Type = "Axes" }; + var cycleB = new Component { Id = "cycleB", Name = "cycleB", Type = "Axes" }; + root.AddComponent(cycleA); + cycleA.AddComponent(cycleB); + cycleB.Components = new List { cycleA }; + + Assert.DoesNotThrow(() => root.RemoveComposition("missing"), + "Component.RemoveComposition must terminate on a cyclic Component graph — no StackOverflowException."); + } + + /// + /// Pins that terminates on + /// a cyclic Component graph. Sibling of the Component.RemoveComposition + /// cycle test — the inline recursive walk replaces the previous + /// flatten, which was itself + /// unguarded. + /// + [Test] + public void Component_RemoveDataItem_terminates_on_cyclic_Component_graph() + { + var root = new Component { Id = "root", Name = "root", Type = "Axes" }; + var a = new Component { Id = "A", Name = "A", Type = "Axes" }; + var b = new Component { Id = "B", Name = "B", Type = "Axes" }; + root.AddComponent(a); + a.AddComponent(b); + b.Components = new List { root }; + + Assert.DoesNotThrow(() => root.RemoveDataItem("missing"), + "Component.RemoveDataItem must terminate on a cyclic Component graph — no StackOverflowException."); + } } } From 4e9d1336dbc111ef490d199f0dd67d70b96ca046 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Otto=20Boly=C3=B3s?= Date: Fri, 21 Aug 2026 00:22:57 +0200 Subject: [PATCH 13/34] =?UTF-8?q?fix(config):=20trace-and-rethrow=20config?= =?UTF-8?q?=20loader=20=E2=80=94=20dime=20H2=20+=20M4?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the four `catch { }` blocks in AgentConfiguration.ReadJson / ReadJson(Type,…) / ReadYaml / ReadYaml(Type,…) with a triaged catch: 1. Direct ArgumentOutOfRangeException — rethrown as ArgumentException wrapping the setter's actionable message plus the configuration path, so operators can trace the bad key back to its file. 2. Any other exception whose InnerException chain contains an ArgumentOutOfRangeException — deserializers (YamlDotNet notably) nest AOORE inside their own container; unwrap via a depth-bounded walker so the same actionable-message shape surfaces regardless of wrapping depth. 3. Any remaining exception — Trace.TraceError with the path and message, then preserve the documented null-return loader contract so non-enum parse / IO failures do not become breaking throws for existing callers. Before this change, an operator writing `inputValidationLevel: 42` (or any out-of-range enum ordinal) got a silent null from the loader with no diagnostic — the actionable setter message was thrown, caught, and swallowed. The trace-and-rethrow path exposes the mistake. Test: DeviceValidationLevelMigrationTests gains three fixtures pinning - ReadJson_Invalid_Enum_Ordinal_Throws_ArgumentException_With_Path - ReadYaml_Invalid_Enum_Ordinal_Throws_ArgumentException_With_Path - ReadJson_Malformed_Json_Returns_Null_Preserving_Loader_Contract Refs: dime Ultrareview cycle-1 findings H2 (security-audit A09 + code-review F-CR-241-04) and M4 (config-path context on setter throws). --- .../Configurations/AgentConfiguration.cs | 119 +++++++++++++++++- .../DeviceValidationLevelMigrationTests.cs | 80 ++++++++++++ 2 files changed, 195 insertions(+), 4 deletions(-) diff --git a/libraries/MTConnect.NET-Common/Configurations/AgentConfiguration.cs b/libraries/MTConnect.NET-Common/Configurations/AgentConfiguration.cs index fa69b583d..9f95f5c6e 100644 --- a/libraries/MTConnect.NET-Common/Configurations/AgentConfiguration.cs +++ b/libraries/MTConnect.NET-Common/Configurations/AgentConfiguration.cs @@ -3,6 +3,7 @@ using MTConnect.Agents; using System; +using System.Diagnostics; using System.IO; using System.Text.Json; using System.Text.Json.Serialization; @@ -250,6 +251,26 @@ public void Normalize() } } + /// + /// Walks the chain looking for an + /// . Deserialisers (YamlDotNet + /// notably) wrap setter throws in one or more layers of their own + /// container exception, so the direct catch (ArgumentOutOfRangeException) + /// filter is insufficient. Depth-bounded so a pathological deeply-nested + /// chain does not loop forever. + /// + private static ArgumentOutOfRangeException UnwrapArgumentOutOfRange(Exception ex) + { + const int MaxUnwrapDepth = 16; + var current = ex; + for (var i = 0; i < MaxUnwrapDepth && current != null; i++) + { + if (current is ArgumentOutOfRangeException aoore) return aoore; + current = current.InnerException; + } + return null; + } + /// /// Loads an , auto-detecting JSON or YAML; see for the resolution rules. @@ -357,7 +378,22 @@ public static T ReadJson(string path = null) where T : AgentConfiguration return configuration; } } - catch { } + catch (ArgumentOutOfRangeException ex) + { + // Invalid enum value in the source document — surface the actionable + // setter message with the offending configuration path attached so + // the operator can trace the bad key back to its file. + throw new ArgumentException( + $"Invalid enum value in {configurationPath}: {ex.Message}", + ex); + } + catch (Exception ex) + { + // Parse / IO / unexpected failures preserve the null-return loader + // contract, but no longer swallow silently — trace the path and + // message so downstream operators see the diagnostic. + Trace.TraceError($"Config load failed: {configurationPath}: {ex.Message}"); + } } return null; @@ -398,7 +434,32 @@ public static AgentConfiguration ReadJson(Type type, string path = null) return configuration; } } - catch { } + catch (ArgumentOutOfRangeException ex) + { + // Invalid enum value in the source document — surface the actionable + // setter message with the offending configuration path attached so + // the operator can trace the bad key back to its file. + throw new ArgumentException( + $"Invalid enum value in {configurationPath}: {ex.Message}", + ex); + } + catch (Exception ex) when (UnwrapArgumentOutOfRange(ex) is ArgumentOutOfRangeException aoore) + { + // Deserialisers (YamlDotNet notably) wrap setter throws inside one + // or more layers of their own container exception; walk the + // InnerException chain so a bad-enum config surfaces the same + // actionable message shape as the direct AOORE catch above. + throw new ArgumentException( + $"Invalid enum value in {configurationPath}: {aoore.Message}", + aoore); + } + catch (Exception ex) + { + // Parse / IO / unexpected failures preserve the null-return loader + // contract, but no longer swallow silently — trace the path and + // message so downstream operators see the diagnostic. + Trace.TraceError($"Config load failed: {configurationPath}: {ex.Message}"); + } } return null; @@ -440,7 +501,32 @@ public static T ReadYaml(string path = null) where T : AgentConfiguration return configuration; } } - catch { } + catch (ArgumentOutOfRangeException ex) + { + // Invalid enum value in the source document — surface the actionable + // setter message with the offending configuration path attached so + // the operator can trace the bad key back to its file. + throw new ArgumentException( + $"Invalid enum value in {configurationPath}: {ex.Message}", + ex); + } + catch (Exception ex) when (UnwrapArgumentOutOfRange(ex) is ArgumentOutOfRangeException aoore) + { + // Deserialisers (YamlDotNet notably) wrap setter throws inside one + // or more layers of their own container exception; walk the + // InnerException chain so a bad-enum config surfaces the same + // actionable message shape as the direct AOORE catch above. + throw new ArgumentException( + $"Invalid enum value in {configurationPath}: {aoore.Message}", + aoore); + } + catch (Exception ex) + { + // Parse / IO / unexpected failures preserve the null-return loader + // contract, but no longer swallow silently — trace the path and + // message so downstream operators see the diagnostic. + Trace.TraceError($"Config load failed: {configurationPath}: {ex.Message}"); + } } return null; @@ -481,7 +567,32 @@ public static AgentConfiguration ReadYaml(Type type, string path = null) return configuration; } } - catch { } + catch (ArgumentOutOfRangeException ex) + { + // Invalid enum value in the source document — surface the actionable + // setter message with the offending configuration path attached so + // the operator can trace the bad key back to its file. + throw new ArgumentException( + $"Invalid enum value in {configurationPath}: {ex.Message}", + ex); + } + catch (Exception ex) when (UnwrapArgumentOutOfRange(ex) is ArgumentOutOfRangeException aoore) + { + // Deserialisers (YamlDotNet notably) wrap setter throws inside one + // or more layers of their own container exception; walk the + // InnerException chain so a bad-enum config surfaces the same + // actionable message shape as the direct AOORE catch above. + throw new ArgumentException( + $"Invalid enum value in {configurationPath}: {aoore.Message}", + aoore); + } + catch (Exception ex) + { + // Parse / IO / unexpected failures preserve the null-return loader + // contract, but no longer swallow silently — trace the path and + // message so downstream operators see the diagnostic. + Trace.TraceError($"Config load failed: {configurationPath}: {ex.Message}"); + } } return null; diff --git a/tests/MTConnect.NET-Common-Tests/Configurations/DeviceValidationLevelMigrationTests.cs b/tests/MTConnect.NET-Common-Tests/Configurations/DeviceValidationLevelMigrationTests.cs index f4017daf0..dbb4ac445 100644 --- a/tests/MTConnect.NET-Common-Tests/Configurations/DeviceValidationLevelMigrationTests.cs +++ b/tests/MTConnect.NET-Common-Tests/Configurations/DeviceValidationLevelMigrationTests.cs @@ -440,6 +440,86 @@ public void ReadYaml_Explicit_DeviceValidationLevel_Beats_Mirror() } } + // --------------------------------------------------------------- + // Load-time enum-value validation — dime H2 + M4 + // --------------------------------------------------------------- + + /// + /// Pins that a JSON config with an out-of-range integer for + /// inputValidationLevel throws an + /// whose message names the offending configuration path — the pre-fix + /// catch { } silently returned null, hiding the actionable + /// setter diagnostic from the operator. Dime cycle-1 finding H2 + /// (security-audit A09 + code-review F-CR-241-04) required the swallow + /// be replaced; M4 required the path be attached. + /// + [Test] + public void ReadJson_Invalid_Enum_Ordinal_Throws_ArgumentException_With_Path() + { + var path = WriteTempJson("{\"inputValidationLevel\":42}"); + try + { + var ex = Assert.Throws(() => AgentConfiguration.ReadJson(path)); + Assert.That(ex!.Message, Does.Contain(path), + "the wrapped exception must carry the configuration path so operators can trace the bad key back to its file."); + Assert.That(ex.Message, Does.Contain("InputValidationLevel"), + "the wrapped exception must preserve the setter's actionable message naming the failing enum."); + } + finally + { + File.Delete(path); + } + } + + /// + /// Sibling of the JSON test — the YAML load path must also surface the + /// enum-out-of-range setter exception (unwrapping any deserialiser-level + /// wrapper — YamlDotNet nests AOORE inside its own container) with the + /// configuration path attached. + /// + [Test] + public void ReadYaml_Invalid_Enum_Ordinal_Throws_ArgumentException_With_Path() + { + var path = WriteTempYaml("inputValidationLevel: 42\n"); + try + { + var ex = Assert.Throws(() => AgentConfiguration.ReadYaml(path)); + Assert.That(ex!.Message, Does.Contain(path), + "the wrapped exception must carry the configuration path so operators can trace the bad key back to its file."); + Assert.That(ex.Message, Does.Contain("InputValidationLevel"), + "the unwrapped setter message must survive so callers still see the actionable diagnostic."); + } + finally + { + File.Delete(path); + } + } + + /// + /// Pins that a malformed-but-parseable-shape JSON (invalid syntax that + /// makes JsonSerializer throw a non-enum exception) preserves the + /// documented null-return contract — the H2 fix converts silent swallow + /// into Trace.TraceError diagnostic but must NOT change the return + /// contract for non-enum failures. + /// + [Test] + public void ReadJson_Malformed_Json_Returns_Null_Preserving_Loader_Contract() + { + var path = WriteTempJson("{ this is not valid json ]"); + try + { + AgentConfiguration config = new AgentConfiguration(); + Assert.DoesNotThrow(() => config = AgentConfiguration.ReadJson(path), + "non-enum parse failures must not throw — the documented loader contract is null-on-failure."); + Assert.That(config, Is.Null, + "the loader must return null for malformed input to preserve pre-fix caller contracts."); + } + finally + { + File.Delete(path); + } + } + // --------------------------------------------------------------- // Fixture harness // --------------------------------------------------------------- From 680777c0a068c837e24bcb611bc53d70c0b6be28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Otto=20Boly=C3=B3s?= Date: Fri, 21 Aug 2026 00:24:58 +0200 Subject: [PATCH 14/34] =?UTF-8?q?refactor(config):=20collapse=20DVL=20expl?= =?UTF-8?q?icit=20flag=20into=20nullable=20=E2=80=94=20dime=20M1?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the pre-fix pair of a non-nullable `_deviceValidationLevel` field plus a parallel `_isDeviceValidationLevelExplicit` boolean with a single nullable `DeviceValidationLevel?` field. The nullable itself carries the is-explicit signal: - null — no explicit assignment; getter returns the ctor default (DeviceValidationLevelDefault = Warning), Normalize's `??=` populates it from InputValidationLevel. - non-null — explicit assignment latched (setter, source-document key, or Normalize's mirror all populate it identically); Normalize is a stable no-op thereafter. This preserves every observable behavior the existing 33-test suite pins (default = Warning on both axes, explicit DVL beats mirror, IVL never re-arms the mirror, sticky suppression across all four arms, JSON and YAML load paths mirror correctly). The three surviving test-file comment references to the deleted boolean flag are updated in-place to describe the nullable-backing-field mechanism. Refs: dime Ultrareview cycle-1 finding M1 (simplification agent). --- .../Configurations/AgentConfiguration.cs | 45 ++++++++++++------- .../DeviceValidationLevelMigrationTests.cs | 23 +++++----- 2 files changed, 41 insertions(+), 27 deletions(-) diff --git a/libraries/MTConnect.NET-Common/Configurations/AgentConfiguration.cs b/libraries/MTConnect.NET-Common/Configurations/AgentConfiguration.cs index 9f95f5c6e..9bf3bff31 100644 --- a/libraries/MTConnect.NET-Common/Configurations/AgentConfiguration.cs +++ b/libraries/MTConnect.NET-Common/Configurations/AgentConfiguration.cs @@ -128,10 +128,22 @@ public string DefaultVersionValue [JsonPropertyName("enableValidation")] public bool EnableValidation { get; set; } - private DeviceValidationLevel _deviceValidationLevel; - private bool _isDeviceValidationLevelExplicit; + // Nullable backing field collapses the pre-fix pair (a non-nullable enum + // field plus a parallel `_isDeviceValidationLevelExplicit` boolean) into a + // single is-explicit-or-not signal: null means "no explicit assignment, + // the getter reports the ctor default and Normalize will mirror IVL onto + // it", non-null means "explicitly assigned, do not mirror". Dime cycle-1 + // finding M1 (simplification). + private DeviceValidationLevel? _deviceValidationLevel; private InputValidationLevel _inputValidationLevel; + /// + /// Default Device (MTConnectDevices) validation level surfaced when no + /// explicit assignment has been made. Kept as a named constant so + /// getter, ctor, and Normalize agree on the same fallback. + /// + private const DeviceValidationLevel DeviceValidationLevelDefault = DeviceValidationLevel.Warning; + /// /// Gets or Sets the default Device (MTConnectDevices) validation level. 0 = Ignore, 1 = Warning, 2 = Remove, 3 = Strict. /// @@ -139,14 +151,14 @@ public string DefaultVersionValue /// When a configuration file omits this key the loader mirrors /// onto Device validation, preserving pre-v7 behaviour for consumers that only knew the single /// knob. Setting this property — either programmatically or via a - /// key present in the source document — marks the value as explicit and disables the mirror on the - /// next . An assignment whose ordinal is not a defined enum arm raises - /// . + /// key present in the source document — latches the value as explicit (the nullable backing field + /// becomes non-null) and disables the mirror on the next . An assignment + /// whose ordinal is not a defined enum arm raises . /// [JsonPropertyName("deviceValidationLevel")] public DeviceValidationLevel DeviceValidationLevel { - get => _deviceValidationLevel; + get => _deviceValidationLevel ?? DeviceValidationLevelDefault; set { if (!Enum.IsDefined(typeof(DeviceValidationLevel), value)) @@ -157,7 +169,6 @@ public DeviceValidationLevel DeviceValidationLevel "DeviceValidationLevel must be one of Ignore (0), Warning (1), Remove (2), Strict (3)."); } _deviceValidationLevel = value; - _isDeviceValidationLevelExplicit = true; } } @@ -218,11 +229,13 @@ public AgentConfiguration() ObservationBufferSize = 131072; AssetBufferSize = 1024; DefaultVersion = MTConnectVersions.Max; - // Assign the backing fields directly. Going through the public setter would flip - // _isDeviceValidationLevelExplicit and disable the load-time migration mirror. - _deviceValidationLevel = DeviceValidationLevel.Warning; + // Leave _deviceValidationLevel null. Going through the public setter would + // latch it as explicit and disable the load-time migration mirror; the + // getter falls back to DeviceValidationLevelDefault while the backing field + // is null, and Normalize's `??=` populates it from _inputValidationLevel + // during the load path. + _deviceValidationLevel = null; _inputValidationLevel = InputValidationLevel.Warning; - _isDeviceValidationLevelExplicit = false; AllowEmptyResultForEnumEvents = false; ConvertUnits = true; IgnoreObservationCase = false; @@ -245,10 +258,12 @@ public AgentConfiguration() /// public void Normalize() { - if (!_isDeviceValidationLevelExplicit) - { - _deviceValidationLevel = (DeviceValidationLevel)(int)_inputValidationLevel; - } + // `??=` assigns the mirror only when the backing field is still null — + // i.e. neither a programmatic setter call nor a source-document key + // has latched DeviceValidationLevel to an explicit value. The + // sticky-suppression semantics (an explicit DVL assignment beats a + // later IVL change on the next Normalize) fall out of the null-check. + _deviceValidationLevel ??= (DeviceValidationLevel)(int)_inputValidationLevel; } /// diff --git a/tests/MTConnect.NET-Common-Tests/Configurations/DeviceValidationLevelMigrationTests.cs b/tests/MTConnect.NET-Common-Tests/Configurations/DeviceValidationLevelMigrationTests.cs index dbb4ac445..d570b701b 100644 --- a/tests/MTConnect.NET-Common-Tests/Configurations/DeviceValidationLevelMigrationTests.cs +++ b/tests/MTConnect.NET-Common-Tests/Configurations/DeviceValidationLevelMigrationTests.cs @@ -315,9 +315,9 @@ public void Normalize_Mirrors_Every_InputValidationLevel_Arm( /// /// Pins Normalize idempotency: calling /// a second time is a stable no-op — the DeviceValidationLevel value is the - /// mirrored value, not the ctor default. A regression that reset - /// _isDeviceValidationLevelExplicit inside Normalize (making the - /// mirror re-fire) would silently overwrite a subsequent explicit assignment; + /// mirrored value, not the ctor default. A regression that reset the + /// nullable backing field to null inside Normalize (making the mirror + /// re-fire) would silently overwrite a subsequent explicit assignment; /// pinning idempotency catches that class of bug. /// [Test] @@ -338,19 +338,18 @@ public void Normalize_Is_Idempotent_On_Repeat_Calls() /// /// Pins that setting /// AFTER an explicit - /// assignment does NOT re-arm the mirror. The - /// _isDeviceValidationLevelExplicit flag is set by the DVL setter - /// and never cleared by the IVL setter — a caller who explicitly set DVL - /// then later set IVL must not have DVL silently overwritten on the next - /// Normalize call. + /// assignment does NOT re-arm the mirror. The DVL setter populates the + /// nullable backing field to a non-null value and the IVL setter never + /// touches it — a caller who explicitly set DVL then later set IVL must + /// not have DVL silently overwritten on the next Normalize call. /// [Test] public void Normalize_Explicit_DeviceValidationLevel_Then_Later_InputValidationLevel_Assignment_Does_Not_Rearm_Mirror() { var config = new AgentConfiguration(); config.DeviceValidationLevel = DeviceValidationLevel.Ignore; - // The DVL setter marked _isDeviceValidationLevelExplicit=true. Later IVL assignment - // must not clear that latch. + // The DVL setter populated the nullable backing field to non-null. Later + // IVL assignment must not reset it to null. config.InputValidationLevel = InputValidationLevel.Strict; config.Normalize(); @@ -362,8 +361,8 @@ public void Normalize_Explicit_DeviceValidationLevel_Then_Later_InputValidationL /// /// Pins sticky suppression across every arm — /// the existing single-arm sticky-suppression test only covered - /// Strict → Ignore. A regression that clears _isDeviceValidationLevelExplicit - /// only on a specific arm (for example resetting the flag on the ctor default + /// Strict → Ignore. A regression that resets the nullable backing + /// field to null only on a specific arm (for example on the ctor-default /// arm) would slip past the single existing test. /// [TestCase(DeviceValidationLevel.Ignore)] From 883e9231c824376ba2ebc87f17b690485ee61925 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Otto=20Boly=C3=B3s?= Date: Fri, 21 Aug 2026 00:27:13 +0200 Subject: [PATCH 15/34] =?UTF-8?q?refactor(common,docs):=20quick=20wins=20+?= =?UTF-8?q?=20doc=20fix=20=E2=80=94=20dime=20L1=20L2=20L3=20L4=20M3?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit L1 — IAgentConfiguration.DeviceValidationLevel XML summary: change "Gets or Sets" to "Gets" so the interface docstring matches the `{ get; }` declaration (the sibling InputValidationLevel summary already says "Gets"). Interface simplification finding. L2 — MTConnectAgent.CoerceEmptyResultToUnavailable: drop the hand-written pre-filter (Where + ToList + assign) since ObservationInput.AddValue(string, object) already replaces any prior entry with the same ValueKey (see ObservationInput.cs:248). Net effect is identical, one fewer allocation + one fewer collection walk per coerce. L3 + L4 — Extract a single generic ThrowIfUndefined(value, message) helper for the DVL / IVL setter validation. .NET 5+ uses the generic `Enum.IsDefined(value)` overload to avoid the `typeof()` reflection + boxing that the legacy path incurs; netstandard2.0 falls back to `Enum.IsDefined(typeof(TEnum), value)` via `#if NET5_0_OR_GREATER`. The two duplicated setter throw blocks collapse to a two-line call site each, and the throw message text stays byte-identical so DeviceValidationLevelMigrationTests's exception-shape pins (line 262 onward — paramName="value", ActualValue=, Message contains enum name) still pass unchanged. M3 — docs/concepts/agent-validation-events.md Strict-arm bullet: `AddDevice` returns `IDevice`, not `bool`; when Strict rejects, the returned reference is null. Rewrite to "the AddDevice call returns null and no part of the tree is added, or the observation / asset input call returns false" so the doc reflects the actual method signatures (MTConnectAgent.AddDevice → IDevice; AddObservation / AddAsset → bool). Refs: dime Ultrareview cycle-1 findings L1 (documentation-audit), L2 (simplification), L3 + L4 (simplification), M3 (documentation-audit + code-review F-CR-241-05). --- docs/concepts/agent-validation-events.md | 2 +- .../Agents/MTConnectAgent.cs | 14 +++--- .../Configurations/AgentConfiguration.cs | 47 +++++++++++++------ .../Configurations/IAgentConfiguration.cs | 2 +- 4 files changed, 41 insertions(+), 24 deletions(-) diff --git a/docs/concepts/agent-validation-events.md b/docs/concepts/agent-validation-events.md index 7d702f182..2b4455aa2 100644 --- a/docs/concepts/agent-validation-events.md +++ b/docs/concepts/agent-validation-events.md @@ -144,7 +144,7 @@ The handler runs first; what the agent does next depends on the applicable knob - **`Ignore`** — the event does not fire, and the input is kept. Useful only for debugging. - **`Warning`** — the event fires; the input is kept. - **`Remove`** — the event fires; the offending node is pruned from its parent (e.g. `device.RemoveDataItem(id)`), or the input is dropped for the observation / asset case. -- **`Strict`** — the event fires; the entire Device is rejected (the `AddDevice` call returns `false` and no part of the tree is added), or the observation / asset input is rejected. +- **`Strict`** — the event fires; the entire Device is rejected (the `AddDevice` call returns `null` and no part of the tree is added), or the observation / asset input call returns `false`. ## Contributor POV diff --git a/libraries/MTConnect.NET-Common/Agents/MTConnectAgent.cs b/libraries/MTConnect.NET-Common/Agents/MTConnectAgent.cs index 65420fcfc..0a0afa7a6 100644 --- a/libraries/MTConnect.NET-Common/Agents/MTConnectAgent.cs +++ b/libraries/MTConnect.NET-Common/Agents/MTConnectAgent.cs @@ -2342,17 +2342,15 @@ private static bool IsEmptyResult(IObservationInput input) /// Rewrites the observation's Result value to and flags the input as unavailable. /// /// - /// Removes any prior Result entry (so the Values collection does not carry a duplicate ValueKey), - /// adds the UNAVAILABLE sentinel, and sets so downstream - /// consumers that branch on the flag observe the coerced state. Spec authority: MTConnect Part 2 - /// Devices Information Model - Observation Information Model - Representation - Observation Values. + /// Delegates the Values-collection housekeeping to + /// , which already + /// replaces any prior entry with the same ValueKey — the earlier hand-written + /// pre-filter (Where + ToList + assign) was redundant work. Spec authority: + /// MTConnect Part 2 Devices Information Model - Observation Information Model - + /// Representation - Observation Values. /// private static void CoerceEmptyResultToUnavailable(IObservationInput input) { - var preserved = (input.Values ?? Enumerable.Empty()) - .Where(v => v.Key != ValueKeys.Result) - .ToList(); - input.Values = preserved; input.AddValue(ValueKeys.Result, Observation.Unavailable); input.IsUnavailable = true; } diff --git a/libraries/MTConnect.NET-Common/Configurations/AgentConfiguration.cs b/libraries/MTConnect.NET-Common/Configurations/AgentConfiguration.cs index 9bf3bff31..15f737fab 100644 --- a/libraries/MTConnect.NET-Common/Configurations/AgentConfiguration.cs +++ b/libraries/MTConnect.NET-Common/Configurations/AgentConfiguration.cs @@ -161,13 +161,9 @@ public DeviceValidationLevel DeviceValidationLevel get => _deviceValidationLevel ?? DeviceValidationLevelDefault; set { - if (!Enum.IsDefined(typeof(DeviceValidationLevel), value)) - { - throw new ArgumentOutOfRangeException( - nameof(value), - value, - "DeviceValidationLevel must be one of Ignore (0), Warning (1), Remove (2), Strict (3)."); - } + ThrowIfUndefined( + value, + "DeviceValidationLevel must be one of Ignore (0), Warning (1), Remove (2), Strict (3)."); _deviceValidationLevel = value; } } @@ -185,13 +181,9 @@ public InputValidationLevel InputValidationLevel get => _inputValidationLevel; set { - if (!Enum.IsDefined(typeof(InputValidationLevel), value)) - { - throw new ArgumentOutOfRangeException( - nameof(value), - value, - "InputValidationLevel must be one of Ignore (0), Warning (1), Remove (2), Strict (3)."); - } + ThrowIfUndefined( + value, + "InputValidationLevel must be one of Ignore (0), Warning (1), Remove (2), Strict (3)."); _inputValidationLevel = value; } } @@ -286,6 +278,33 @@ private static ArgumentOutOfRangeException UnwrapArgumentOutOfRange(Exception ex return null; } + /// + /// Throws when + /// is not a defined arm. Extracted from the + /// duplicated setter throw blocks on and + /// — dime cycle-1 finding L4. The message + /// is caller-supplied so each setter can name the enum it guards. + /// + /// + /// The .NET 5+ generic Enum.IsDefined<TEnum> overload avoids the + /// boxing that the legacy Enum.IsDefined(Type, object) path incurs; + /// the netstandard2.0 branch keeps the reflection form since the generic + /// overload was not introduced until .NET 5. + /// + private static void ThrowIfUndefined(TEnum value, string message) + where TEnum : struct, Enum + { +#if NET5_0_OR_GREATER + if (Enum.IsDefined(value)) return; +#else + if (Enum.IsDefined(typeof(TEnum), value)) return; +#endif + throw new ArgumentOutOfRangeException( + "value", + value, + message); + } + /// /// Loads an , auto-detecting JSON or YAML; see for the resolution rules. diff --git a/libraries/MTConnect.NET-Common/Configurations/IAgentConfiguration.cs b/libraries/MTConnect.NET-Common/Configurations/IAgentConfiguration.cs index f6cc210c5..eb24c1bd5 100644 --- a/libraries/MTConnect.NET-Common/Configurations/IAgentConfiguration.cs +++ b/libraries/MTConnect.NET-Common/Configurations/IAgentConfiguration.cs @@ -66,7 +66,7 @@ public interface IAgentConfiguration bool EnableValidation { get; } /// - /// Gets or Sets the default Device (MTConnectDevices) validation level. 0 = Ignore, 1 = Warning, 2 = Remove, 3 = Strict + /// Gets the default Device (MTConnectDevices) validation level. 0 = Ignore, 1 = Warning, 2 = Remove, 3 = Strict /// DeviceValidationLevel DeviceValidationLevel { get; } From 72a8b737a49055efc7b7513290bcc5a68dd946df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Otto=20Boly=C3=B3s?= Date: Fri, 21 Aug 2026 00:31:51 +0200 Subject: [PATCH 16/34] docs(reference): regenerate configuration.md after IAgentConfiguration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit IAgentConfiguration.DeviceValidationLevel's XML summary was corrected from "Gets or Sets" to "Gets" (dime L1, previous commit) so that it matches the `{ get; }` interface declaration. The generated reference page docs/reference/configuration.md is auto-produced from those summaries via docs/scripts/generate-reference.sh, so it drifts on any summary edit; regenerate now to clear the docs-CI drift gate. Refs: dime Ultrareview cycle-1 finding L1 (documentation-audit) — regeneration follow-up. --- docs/reference/configuration.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/reference/configuration.md b/docs/reference/configuration.md index f8b8289af..5e58d22bb 100644 --- a/docs/reference/configuration.md +++ b/docs/reference/configuration.md @@ -197,7 +197,7 @@ Configuration for an MTConnect Agent | `changeToken` | `ChangeToken` | `string` | An opaque token that changes whenever the underlying configuration source is reloaded, allowing consumers to detect that the configuration has been replaced. | | `convertUnits` | `ConvertUnits` | `bool` | Gets the default for Converting Units when adding Observations | | `defaultVersion` | `DefaultVersion` | `Version` | Gets the default MTConnect version to output response documents for. | -| `deviceValidationLevel` | `DeviceValidationLevel` | `DeviceValidationLevel` | Gets or Sets the default Device (MTConnectDevices) validation level. 0 = Ignore, 1 = Warning, 2 = Remove, 3 = Strict | +| `deviceValidationLevel` | `DeviceValidationLevel` | `DeviceValidationLevel` | Gets the default Device (MTConnectDevices) validation level. 0 = Ignore, 1 = Warning, 2 = Remove, 3 = Strict | | `enableAgentDevice` | `EnableAgentDevice` | `bool` | Gets or Sets whether the Agent Device is output | | `enableMetrics` | `EnableMetrics` | `bool` | Gets whether Metrics are captured (ex. ObserationUpdateRate, AssetUpdateRate) | | `enableValidation` | `EnableValidation` | `bool` | Gets or Sets whether validation information is output | From b91ae0d47f443ff1d998f96f96d1707b1f16306a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Otto=20Boly=C3=B3s?= Date: Fri, 21 Aug 2026 01:01:25 +0200 Subject: [PATCH 17/34] test(common,devices): pin depth-bound guards (Unwrap, Remove walk) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cycle-2 test-audit uncovered two belt-and-braces depth guards the cycle-1 fixes introduced but never pinned: * AgentConfiguration.UnwrapArgumentOutOfRange (private static, added by be029697 as part of the H2 catch triage) walks the InnerException chain up to MaxUnwrapDepth = 16 hops. The public ReadYaml / ReadJson load paths only produce a chain of depth 2-3 so they cannot exercise the ceiling; a regression that removed the bound (unbounded loop) or bumped it to a different value would slip past every existing test. * Device.MaxComponentWalkDepth / Component.MaxComponentWalkDepth (both 1024, added by a0d87fda as belt-and-braces alongside the visited-Id HashSet cycle guard). Every existing cycle test builds an A→B→A shape which the HashSet catches on the FIRST re-entry — the depth ceiling never fires. A deep linear chain of unique-Id Components exercises the depth ceiling exclusively (HashSet always .Add-returns true) so a regression that raises, lowers, or removes the ceiling fails loudly. New fixtures: DeviceValidationLevelMigrationTests UnwrapArgumentOutOfRange_Returns_Root_When_Root_Is_AOORE UnwrapArgumentOutOfRange_Finds_AOORE_At_Ceiling_Depth_Fifteen UnwrapArgumentOutOfRange_Returns_Null_When_AOORE_Sits_Past_Depth_Ceiling UnwrapArgumentOutOfRange_Returns_Null_On_Chain_Without_AOORE UnwrapArgumentOutOfRange_Returns_Null_On_Null_Input DeviceRemoveRecursionTests RemoveComposition_depth_ceiling_removes_within_1024_leaves_past_1024_intact RemoveDataItem_depth_ceiling_removes_within_1024_leaves_past_1024_intact Component_RemoveComposition_depth_ceiling_leaves_past_1024_intact The UnwrapArgumentOutOfRange fixtures invoke the private helper via reflection — pinning private-helper behavior is the accepted route when the behavior is a documented depth guard and no public observable-side test can reach it. The two-arm ceiling tests (shallow-1000-removed + deep-1030-survives) pin the exact ceiling value: a regression that changes MaxComponentWalkDepth in either direction fails on one arm. Bluefin: 8 net-new tests + 4173 existing tests = 4181 / 0 failed on MTConnect.NET-Common-Tests. --- .../DeviceValidationLevelMigrationTests.cs | 129 +++++++++++++ .../Devices/DeviceRemoveRecursionTests.cs | 176 ++++++++++++++++++ 2 files changed, 305 insertions(+) diff --git a/tests/MTConnect.NET-Common-Tests/Configurations/DeviceValidationLevelMigrationTests.cs b/tests/MTConnect.NET-Common-Tests/Configurations/DeviceValidationLevelMigrationTests.cs index d570b701b..0b11e69f2 100644 --- a/tests/MTConnect.NET-Common-Tests/Configurations/DeviceValidationLevelMigrationTests.cs +++ b/tests/MTConnect.NET-Common-Tests/Configurations/DeviceValidationLevelMigrationTests.cs @@ -3,6 +3,7 @@ using System; using System.IO; +using System.Reflection; using MTConnect.Agents; using MTConnect.Configurations; using NUnit.Framework; @@ -519,6 +520,134 @@ public void ReadJson_Malformed_Json_Returns_Null_Preserving_Loader_Contract() } } + // --------------------------------------------------------------- + // UnwrapArgumentOutOfRange — private helper depth-bound pins + // --------------------------------------------------------------- + // + // The H2 fix introduces a private static helper `UnwrapArgumentOutOfRange(Exception)` + // on that walks the + // chain looking for an — deserialisers + // (YamlDotNet notably) nest the setter throw inside their own container. The walk + // is depth-bounded at MaxUnwrapDepth = 16 to defend against a pathological deeply- + // nested chain looping forever. The public YAML load path only produces a chain + // of depth 2-3 so it cannot exercise the depth ceiling; these fixtures pin the + // ceiling directly via reflection so a regression that removes the ceiling + // (introducing an unbounded loop) OR bumps it to a different value fails loudly. + + private static ArgumentOutOfRangeException? InvokeUnwrap(Exception ex) + { + var method = typeof(AgentConfiguration).GetMethod( + "UnwrapArgumentOutOfRange", + BindingFlags.Static | BindingFlags.NonPublic); + Assert.That(method, Is.Not.Null, + "UnwrapArgumentOutOfRange must exist as a static non-public helper on AgentConfiguration — the H2 fix contract."); + return (ArgumentOutOfRangeException?)method!.Invoke(null, new object?[] { ex }); + } + + private static Exception BuildWrappedChain(Exception innermost, int wrapCount) + { + var current = innermost; + for (var i = 0; i < wrapCount; i++) + { + current = new InvalidOperationException($"wrap-{i}", current); + } + return current; + } + + /// + /// Pins that UnwrapArgumentOutOfRange returns the AOORE when it sits + /// exactly at the surface (depth 0). The direct catch (ArgumentOutOfRangeException) + /// filter should hit before this helper runs in the loader, but the helper + /// must still handle a depth-0 chain because + /// walks include their own root. + /// + [Test] + public void UnwrapArgumentOutOfRange_Returns_Root_When_Root_Is_AOORE() + { + var aoore = new ArgumentOutOfRangeException("value", "root"); + + var result = InvokeUnwrap(aoore); + + Assert.That(result, Is.SameAs(aoore), + "the helper must return the AOORE when it sits at depth 0 (root)."); + } + + /// + /// Pins that UnwrapArgumentOutOfRange returns the AOORE when it sits + /// at exactly the deepest depth the ceiling still visits — the loop iterates + /// i = 0..15, checking depths 0..15 inclusive. AOORE at depth 15 is + /// visited on iteration 15 and returned; AOORE at depth 16 is never visited + /// (loop exits after iteration 15 before advancing to depth 16). + /// + [Test] + public void UnwrapArgumentOutOfRange_Finds_AOORE_At_Ceiling_Depth_Fifteen() + { + // 15 wrappers around the AOORE puts the AOORE at depth 15 from the outer root. + var aoore = new ArgumentOutOfRangeException("value", "deep-15"); + var chain = BuildWrappedChain(aoore, wrapCount: 15); + + var result = InvokeUnwrap(chain); + + Assert.That(result, Is.SameAs(aoore), + "the helper must find the AOORE at depth 15 — the deepest depth the MaxUnwrapDepth = 16 loop still visits (i = 15)."); + } + + /// + /// Pins that UnwrapArgumentOutOfRange returns null when the AOORE sits + /// past the depth ceiling — the loop bails after 16 iterations without walking + /// to depth 16 or beyond. A regression that removes the bound and loops until + /// InnerException is null would find the AOORE at depth 20 and return it — that + /// would satisfy the H2 unwrap contract but violate the depth-guard invariant + /// against pathological chains. The FLOOR pins the exact ceiling. + /// + [Test] + public void UnwrapArgumentOutOfRange_Returns_Null_When_AOORE_Sits_Past_Depth_Ceiling() + { + var aoore = new ArgumentOutOfRangeException("value", "deep-20"); + var chain = BuildWrappedChain(aoore, wrapCount: 20); + + var result = InvokeUnwrap(chain); + + Assert.That(result, Is.Null, + "the helper must NOT walk past MaxUnwrapDepth = 16 — an AOORE at depth 20 must return null so a pathological deeply-nested chain never loops forever."); + } + + /// + /// Pins that UnwrapArgumentOutOfRange returns null on a chain that + /// contains no AOORE at any depth. The loader relies on this null-return to + /// distinguish an enum-out-of-range setter throw from a generic parser / + /// IO failure — the former is rethrown as ArgumentException, the latter is + /// traced and null-returned per the loader contract. A regression that + /// returned the outermost exception on a no-match walk would misroute + /// generic failures into the enum-error path. + /// + [Test] + public void UnwrapArgumentOutOfRange_Returns_Null_On_Chain_Without_AOORE() + { + var innermost = new InvalidOperationException("innermost"); + var chain = BuildWrappedChain(innermost, wrapCount: 5); + + var result = InvokeUnwrap(chain); + + Assert.That(result, Is.Null, + "the helper must return null when the InnerException chain contains no AOORE — the loader routes non-enum failures to the trace-and-return-null path."); + } + + /// + /// Pins that UnwrapArgumentOutOfRange returns null when handed a null + /// exception. The current implementation guards via the loop condition + /// (current != null) so the method is null-safe. A regression that + /// dereferenced the parameter before the guard would NRE on this input. + /// + [Test] + public void UnwrapArgumentOutOfRange_Returns_Null_On_Null_Input() + { + var result = InvokeUnwrap(null!); + + Assert.That(result, Is.Null, + "the helper must be null-safe — the loop condition current != null guards the very first iteration."); + } + // --------------------------------------------------------------- // Fixture harness // --------------------------------------------------------------- diff --git a/tests/MTConnect.NET-Common-Tests/Devices/DeviceRemoveRecursionTests.cs b/tests/MTConnect.NET-Common-Tests/Devices/DeviceRemoveRecursionTests.cs index 8033d5813..7da3d38e6 100644 --- a/tests/MTConnect.NET-Common-Tests/Devices/DeviceRemoveRecursionTests.cs +++ b/tests/MTConnect.NET-Common-Tests/Devices/DeviceRemoveRecursionTests.cs @@ -500,5 +500,181 @@ public void Component_RemoveDataItem_terminates_on_cyclic_Component_graph() Assert.DoesNotThrow(() => root.RemoveDataItem("missing"), "Component.RemoveDataItem must terminate on a cyclic Component graph — no StackOverflowException."); } + + // -------------------------------------------------------------- + // MaxComponentWalkDepth = 1024 — belt-and-braces depth ceiling. + // The visited-Id HashSet catches ordinary cycles; the depth + // ceiling defends against pathological cases the HashSet cannot + // prune (for example every Component in the cycle carrying a + // null Id so nothing gets added to the set). A deep linear + // chain of unique-Id Components exercises the depth ceiling + // exclusively — the HashSet always .Add-returns true so only + // the `depth > MaxComponentWalkDepth` early-return terminates. + // + // The loop walks depth 1..1024 (inclusive) and returns early + // at depth 1025 — placing a Composition on the component at + // depth 1000 verifies the walk reaches within the ceiling + // (removed), placing another at depth 1030 verifies the walk + // does NOT reach past the ceiling (survives). A regression + // that raises or removes the ceiling makes the depth-1030 + // arm fail; a regression that lowers the ceiling makes the + // depth-1000 arm fail. Two-arm construction pins the exact + // ceiling value. + // -------------------------------------------------------------- + + private static Device BuildLinearDeepDevice(int chainLength, int shallowIndex, int deepIndex, + out string shallowTargetId, out string deepTargetId, + out Component shallowLeaf, out Component deepLeaf) + { + shallowTargetId = "target-shallow"; + deepTargetId = "target-deep"; + var device = new Device { Id = "d1", Uuid = "d1", Name = "d1", Type = Device.TypeId }; + Component parent = null!; + Component capturedShallow = null!; + Component capturedDeep = null!; + for (var i = 0; i < chainLength; i++) + { + var c = new Component { Id = $"c-{i}", Name = $"c-{i}", Type = "Axes" }; + if (i == 0) device.AddComponent(c); + else parent!.AddComponent(c); + parent = c; + + if (i == shallowIndex) + { + c.AddComposition(new Composition + { + Id = shallowTargetId, + Name = shallowTargetId, + Type = "Generic" + }); + capturedShallow = c; + } + if (i == deepIndex) + { + c.AddComposition(new Composition + { + Id = deepTargetId, + Name = deepTargetId, + Type = "Generic" + }); + capturedDeep = c; + } + } + shallowLeaf = capturedShallow; + deepLeaf = capturedDeep; + return device; + } + + /// + /// Pins MaxComponentWalkDepth = 1024 on . + /// Component c-999 sits at depth 1000 from Device (within the ceiling) and + /// c-1029 sits at depth 1030 (past the ceiling). A single Remove call must + /// process the shallow arm (removed) and skip the deep arm (survives) + /// because the recursive helper returns early at depth 1025 — the frame + /// on c-1024 is entered but bails before touching its children. + /// + [Test] + public void RemoveComposition_depth_ceiling_removes_within_1024_leaves_past_1024_intact() + { + var device = BuildLinearDeepDevice(chainLength: 1030, + shallowIndex: 999, deepIndex: 1029, + out var shallowId, out var deepId, + out var shallowLeaf, out var deepLeaf); + + // Precondition — both targets are present before the removal. + Assert.That(shallowLeaf.Compositions.Any(c => c.Id == shallowId), Is.True, + "precondition — the shallow target Composition must be attached at depth 1000."); + Assert.That(deepLeaf.Compositions.Any(c => c.Id == deepId), Is.True, + "precondition — the deep target Composition must be attached at depth 1030."); + + // Remove the shallow target — expected to succeed. + device.RemoveComposition(shallowId); + Assert.That( + shallowLeaf.Compositions == null || !shallowLeaf.Compositions.Any(c => c.Id == shallowId), + Is.True, + "Device.RemoveComposition must reach depth 1000 (within MaxComponentWalkDepth = 1024) and drop the shallow target."); + + // Remove the deep target — expected to be a no-op because the depth + // ceiling stops the walk before reaching depth 1030. + device.RemoveComposition(deepId); + Assert.That( + deepLeaf.Compositions.Any(c => c.Id == deepId), Is.True, + "Device.RemoveComposition must NOT reach past MaxComponentWalkDepth = 1024 — the belt-and-braces depth guard leaves depth-1030 Compositions intact so a pathological deeply-nested (or null-Id-cyclic) graph cannot exhaust the process stack."); + } + + /// + /// Sibling pin for . Same + /// two-arm shape via DataItems attached to the shallow and deep leaves — + /// the DataItem walk shares MaxComponentWalkDepth = 1024 with the + /// Composition walk. + /// + [Test] + public void RemoveDataItem_depth_ceiling_removes_within_1024_leaves_past_1024_intact() + { + var device = BuildLinearDeepDevice(chainLength: 1030, + shallowIndex: 999, deepIndex: 1029, + out _, out _, + out var shallowLeaf, out var deepLeaf); + + const string shallowDi = "di-shallow"; + const string deepDi = "di-deep"; + shallowLeaf.AddDataItem(new DataItem { Id = shallowDi, Name = shallowDi, Type = "Generic", Category = DataItemCategory.EVENT }); + deepLeaf.AddDataItem(new DataItem { Id = deepDi, Name = deepDi, Type = "Generic", Category = DataItemCategory.EVENT }); + + device.RemoveDataItem(shallowDi); + Assert.That( + shallowLeaf.DataItems == null || !shallowLeaf.DataItems.Any(d => d.Id == shallowDi), + Is.True, + "Device.RemoveDataItem must reach depth 1000 (within MaxComponentWalkDepth = 1024) and drop the shallow DataItem."); + + device.RemoveDataItem(deepDi); + Assert.That( + deepLeaf.DataItems.Any(d => d.Id == deepDi), Is.True, + "Device.RemoveDataItem must NOT reach past MaxComponentWalkDepth = 1024 — DataItems attached at depth 1030 must survive so the belt-and-braces depth ceiling is exercised."); + } + + /// + /// Sibling pin for — + /// the Component-side ceiling constant is defined in Component.cs + /// independently of Device.cs, so a regression that bumps only the + /// Device.cs constant while leaving Component.cs stale (or vice versa) + /// fails on the sibling that still enforces 1024. + /// + [Test] + public void Component_RemoveComposition_depth_ceiling_leaves_past_1024_intact() + { + // Rooted at a Component this time, not a Device. + var root = new Component { Id = "root", Name = "root", Type = "Axes" }; + Component parent = root; + Component shallowLeaf = null!; + Component deepLeaf = null!; + for (var i = 0; i < 1030; i++) + { + var c = new Component { Id = $"c-{i}", Name = $"c-{i}", Type = "Axes" }; + parent.AddComponent(c); + parent = c; + if (i == 999) + { + c.AddComposition(new Composition { Id = "target-shallow", Name = "target-shallow", Type = "Generic" }); + shallowLeaf = c; + } + if (i == 1029) + { + c.AddComposition(new Composition { Id = "target-deep", Name = "target-deep", Type = "Generic" }); + deepLeaf = c; + } + } + + root.RemoveComposition("target-shallow"); + Assert.That( + shallowLeaf.Compositions == null || !shallowLeaf.Compositions.Any(c => c.Id == "target-shallow"), + Is.True, + "Component.RemoveComposition must reach depth 1000 (within MaxComponentWalkDepth = 1024) and drop the shallow target."); + + root.RemoveComposition("target-deep"); + Assert.That( + deepLeaf.Compositions.Any(c => c.Id == "target-deep"), Is.True, + "Component.RemoveComposition must NOT reach past MaxComponentWalkDepth = 1024 — the Component-side ceiling constant must stay in sync with the Device-side ceiling."); + } } } From 5a4461296050783143843902a0d4a06a578b1456 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Otto=20Boly=C3=B3s?= Date: Fri, 21 Aug 2026 01:08:25 +0200 Subject: [PATCH 18/34] =?UTF-8?q?fix(devices):=20guard=20RemoveComponent?= =?UTF-8?q?=20recursion=20=E2=80=94=20dime=20H1-C2?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cycle-1 finding H1 hardened Device.RemoveComposition, Device.RemoveDataItem, Component.RemoveComposition, and Component.RemoveDataItem against cyclic Component graphs by threading a visited-Id HashSet + MaxComponentWalkDepth belt-and-braces ceiling through the recursive walks. Cycle-2 security-audit finding H1-C2 noted that Device.RemoveComponent was NOT rewritten in cycle-1 and still shipped the unguarded shape. The callsite on Strict validation is MTConnectAgent.NormalizeDevice line 1320 (obj.RemoveComponent(genericComponent.Id)). A cyclic Component graph coming through that path stack-overflows the process — same DoS class as the other three Remove* variants. Apply the exact same guard shape used on the other Device.Remove* methods: - Public overload seeds visitedIds with the Device's own Id (parity with RemoveComposition / RemoveDataItem — L1-C2 pattern from cycle 2). - Private overload takes (IComponent, string, HashSet, int), early-returns on depth > MaxComponentWalkDepth (with Trace.TraceWarning), early-returns on cycle re-entry, and recurses into subComponents with depth+1. Adds RemoveComponent_terminates_on_cyclic_Component_graph to the existing DeviceRemoveRecursionTests fixture — the sibling of the RemoveComposition and RemoveDataItem cycle fixtures. --- .../MTConnect.NET-Common/Devices/Device.cs | 34 ++++++++++++++++--- .../Devices/DeviceRemoveRecursionTests.cs | 25 ++++++++++++++ 2 files changed, 54 insertions(+), 5 deletions(-) diff --git a/libraries/MTConnect.NET-Common/Devices/Device.cs b/libraries/MTConnect.NET-Common/Devices/Device.cs index 476833fe7..bb78aef59 100644 --- a/libraries/MTConnect.NET-Common/Devices/Device.cs +++ b/libraries/MTConnect.NET-Common/Devices/Device.cs @@ -4,6 +4,7 @@ using MTConnect.Devices.Components; using System; using System.Collections.Generic; +using System.Diagnostics; using System.Linq; namespace MTConnect.Devices @@ -424,7 +425,15 @@ public void AddComponents(IEnumerable components) /// - /// Remove a Component from the Device + /// Remove a Component from the Device tree — the top-level + /// collection on the Device itself and + /// every nested descendant Component. The traversal carries a visited-Id + /// cycle guard so a cyclic Component graph (A.Components ∋ B, + /// B.Components ∋ A) terminates instead of stack-overflowing — + /// mirrors the shape of and + /// so all three Remove* overloads + /// on Device share the same DoS-resistance contract exercised by + /// MTConnectAgent.NormalizeDevice on Strict validation. /// /// The ID of the Component to remove public void RemoveComponent(string componentId) @@ -436,18 +445,33 @@ public void RemoveComponent(string componentId) components.RemoveAll(o => o.Id == componentId); + var visitedIds = new HashSet(StringComparer.Ordinal); + // Seed the visited set with this Device's own Id so a cycle + // back to `this` terminates immediately — parity with + // RemoveComposition / RemoveDataItem. + if (!string.IsNullOrEmpty(Id)) visitedIds.Add(Id); foreach (var subComponent in components) { - RemoveComponent(subComponent, componentId); + RemoveComponent(subComponent, componentId, visitedIds, depth: 1); } Components = components; } } - private void RemoveComponent(IComponent component, string componentId) + private void RemoveComponent(IComponent component, string componentId, HashSet visitedIds, int depth) { - if (component != null && !component.Components.IsNullOrEmpty()) + if (component == null) return; + if (depth > MaxComponentWalkDepth) + { + Trace.TraceWarning($"Device.RemoveComponent: walk depth {MaxComponentWalkDepth} exceeded; possible cyclic Component graph"); + return; + } + + // Cycle guard — mirrors RemoveComposition / RemoveDataItem. + if (!string.IsNullOrEmpty(component.Id) && !visitedIds.Add(component.Id)) return; + + if (!component.Components.IsNullOrEmpty()) { var components = new List(); components.AddRange(component.Components); @@ -455,7 +479,7 @@ private void RemoveComponent(IComponent component, string componentId) foreach (var subComponent in components) { - RemoveComponent(subComponent, componentId); + RemoveComponent(subComponent, componentId, visitedIds, depth + 1); } ((Component)component).Components = components; diff --git a/tests/MTConnect.NET-Common-Tests/Devices/DeviceRemoveRecursionTests.cs b/tests/MTConnect.NET-Common-Tests/Devices/DeviceRemoveRecursionTests.cs index 7da3d38e6..8315d7386 100644 --- a/tests/MTConnect.NET-Common-Tests/Devices/DeviceRemoveRecursionTests.cs +++ b/tests/MTConnect.NET-Common-Tests/Devices/DeviceRemoveRecursionTests.cs @@ -445,6 +445,31 @@ public void RemoveDataItem_terminates_on_cyclic_Component_graph() "Device.RemoveDataItem must terminate on a cyclic Component graph — no StackOverflowException."); } + /// + /// Pins that terminates on a + /// cyclic Component graph. Sibling of the RemoveComposition / + /// RemoveDataItem cycle tests — dime cycle-2 finding H1-C2 called out + /// this Remove* variant as still lacking the visited-Id + depth-cap + /// guard the other two variants received in cycle-1 H1. The callsite + /// on Strict validation is MTConnectAgent.NormalizeDevice + /// (obj.RemoveComponent(genericComponent.Id)); a cyclic + /// Component graph coming through that path would stack-overflow the + /// process without this guard. + /// + [Test] + public void RemoveComponent_terminates_on_cyclic_Component_graph() + { + var device = new Device { Id = "d1", Uuid = "d1", Name = "d1", Type = Device.TypeId }; + var a = new Component { Id = "A", Name = "A", Type = "Axes" }; + var b = new Component { Id = "B", Name = "B", Type = "Axes" }; + device.AddComponent(a); + a.AddComponent(b); + b.Components = new List { a }; + + Assert.DoesNotThrow(() => device.RemoveComponent("missing"), + "Device.RemoveComponent must terminate on a cyclic Component graph — no StackOverflowException."); + } + /// /// Pins that is now /// recursive across nested child Components AND terminates on a cyclic From d1e5b3d82c45672c953cdc0ae253f9c6dbd2b950 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Otto=20Boly=C3=B3s?= Date: Fri, 21 Aug 2026 01:10:21 +0200 Subject: [PATCH 19/34] =?UTF-8?q?refactor(config):=20extract=20LoadWithTri?= =?UTF-8?q?age=20=E2=80=94=20dime=20M2-C2=20subsumes=20M1-C2?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The four Read{Json,Yaml}[] loader methods were duplicating the same three-clause catch triage (~24 lines each, ~96 lines total): direct ArgumentOutOfRangeException surface → wrapped-AOORE surface via the InnerException walk → generic fall-through that traces and returns null. Cycle-1 finding L2 codified the wrapped-AOORE walk (UnwrapArgumentOutOfRange) but left the four loader bodies as parallel copies. Cycle-2 M1-C2 (four review agents converged) noted an asymmetry that fell out of the duplication: the ReadJson path was missing the middle `catch when UnwrapArgumentOutOfRange` clause, so a bad-enum value that System.Text.Json routes through a wrapped container exception was falling through to the generic trace-and-return-null branch instead of raising ArgumentException the way the other three loaders do. Extract the triage into one helper LoadWithTriage(string, Func) — each loader now becomes: return LoadWithTriage(configurationPath, () => { var text = File.ReadAllText(configurationPath); if (string.IsNullOrEmpty(text)) return null; // deserialize, set Path, Normalize, return configuration }); Sharing the triage body by construction fixes the ReadJson asymmetry forever (dime M2-C2 subsumes M1-C2). Net −32 lines while adding the missing wrapped-AOORE clause to the ReadJson path. No behavioral change for the three loaders that already had the middle catch; ReadJson now behaves like the other three when System.Text.Json wraps a setter throw. --- .../Configurations/AgentConfiguration.cs | 232 ++++++++---------- 1 file changed, 100 insertions(+), 132 deletions(-) diff --git a/libraries/MTConnect.NET-Common/Configurations/AgentConfiguration.cs b/libraries/MTConnect.NET-Common/Configurations/AgentConfiguration.cs index 15f737fab..a7f947571 100644 --- a/libraries/MTConnect.NET-Common/Configurations/AgentConfiguration.cs +++ b/libraries/MTConnect.NET-Common/Configurations/AgentConfiguration.cs @@ -278,6 +278,60 @@ private static ArgumentOutOfRangeException UnwrapArgumentOutOfRange(Exception ex return null; } + /// + /// Shared triage wrapper for the four Read{Json,Yaml}[<T>] loader + /// methods. Each loader was previously duplicating the same three-clause + /// catch triage (direct AOORE → wrapped-AOORE via + /// → generic fall-through that + /// traces and returns null) in-line. Extracting the triage into one + /// helper collapses ~96 lines of duplication and simultaneously closes the + /// cycle-1-vs-cycle-2 asymmetry (dime M2-C2 subsumes M1-C2): the + /// path was missing the middle + /// when Unwrap... catch, so a wrapped enum error deserialised by + /// System.Text.Json was falling through to the generic + /// trace-and-return-null branch instead of raising + /// like the other three loaders. Sharing + /// the same body by construction fixes the asymmetry forever. + /// + /// The concrete return type of the deserialiser call. + /// The resolved configuration path — used both in the surfaced message and in the generic-fall-through line. + /// A closure that runs the deserialiser and returns the loaded configuration (or null when the source text was empty). + private static T LoadWithTriage(string configurationPath, Func deserialize) where T : class + { + try + { + return deserialize(); + } + catch (ArgumentOutOfRangeException ex) + { + // Invalid enum value in the source document — surface the actionable + // setter message with the offending configuration path attached so + // the operator can trace the bad key back to its file. + throw new ArgumentException( + $"Invalid enum value in {configurationPath}: {ex.Message}", + ex); + } + catch (Exception ex) when (UnwrapArgumentOutOfRange(ex) is ArgumentOutOfRangeException aoore) + { + // Deserialisers (YamlDotNet notably, and System.Text.Json when it + // routes through a JsonConverter) wrap setter throws inside one or + // more layers of their own container exception; walk the + // InnerException chain so a bad-enum config surfaces the same + // actionable message shape as the direct AOORE catch above. + throw new ArgumentException( + $"Invalid enum value in {configurationPath}: {aoore.Message}", + aoore); + } + catch (Exception ex) + { + // Parse / IO / unexpected failures preserve the null-return loader + // contract, but no longer swallow silently — trace the path and + // message so downstream operators see the diagnostic. + Trace.TraceError($"Config load failed: {configurationPath}: {ex.Message}"); + return null; + } + } + /// /// Throws when /// is not a defined arm. Extracted from the @@ -396,38 +450,24 @@ public static T ReadJson(string path = null) where T : AgentConfiguration if (!string.IsNullOrEmpty(configurationPath)) { - try + return LoadWithTriage(configurationPath, () => { var text = File.ReadAllText(configurationPath); - if (!string.IsNullOrEmpty(text)) + if (string.IsNullOrEmpty(text)) return null; + + var options = new JsonSerializerOptions() { - var options = new JsonSerializerOptions() - { - ReadCommentHandling = JsonCommentHandling.Skip - }; + ReadCommentHandling = JsonCommentHandling.Skip + }; - var configuration = JsonSerializer.Deserialize(text, options); + var configuration = JsonSerializer.Deserialize(text, options); + if (configuration != null) + { configuration.Path = configurationPath; configuration.Normalize(); - return configuration; } - } - catch (ArgumentOutOfRangeException ex) - { - // Invalid enum value in the source document — surface the actionable - // setter message with the offending configuration path attached so - // the operator can trace the bad key back to its file. - throw new ArgumentException( - $"Invalid enum value in {configurationPath}: {ex.Message}", - ex); - } - catch (Exception ex) - { - // Parse / IO / unexpected failures preserve the null-return loader - // contract, but no longer swallow silently — trace the path and - // message so downstream operators see the diagnostic. - Trace.TraceError($"Config load failed: {configurationPath}: {ex.Message}"); - } + return configuration; + }); } return null; @@ -452,48 +492,24 @@ public static AgentConfiguration ReadJson(Type type, string path = null) if (!string.IsNullOrEmpty(configurationPath)) { - try + return LoadWithTriage(configurationPath, () => { var text = File.ReadAllText(configurationPath); - if (!string.IsNullOrEmpty(text)) + if (string.IsNullOrEmpty(text)) return null; + + var options = new JsonSerializerOptions() { - var options = new JsonSerializerOptions() - { - ReadCommentHandling = JsonCommentHandling.Skip - }; + ReadCommentHandling = JsonCommentHandling.Skip + }; - var configuration = (AgentConfiguration)JsonSerializer.Deserialize(text, type, options); + var configuration = (AgentConfiguration)JsonSerializer.Deserialize(text, type, options); + if (configuration != null) + { configuration.Path = configurationPath; configuration.Normalize(); - return configuration; } - } - catch (ArgumentOutOfRangeException ex) - { - // Invalid enum value in the source document — surface the actionable - // setter message with the offending configuration path attached so - // the operator can trace the bad key back to its file. - throw new ArgumentException( - $"Invalid enum value in {configurationPath}: {ex.Message}", - ex); - } - catch (Exception ex) when (UnwrapArgumentOutOfRange(ex) is ArgumentOutOfRangeException aoore) - { - // Deserialisers (YamlDotNet notably) wrap setter throws inside one - // or more layers of their own container exception; walk the - // InnerException chain so a bad-enum config surfaces the same - // actionable message shape as the direct AOORE catch above. - throw new ArgumentException( - $"Invalid enum value in {configurationPath}: {aoore.Message}", - aoore); - } - catch (Exception ex) - { - // Parse / IO / unexpected failures preserve the null-return loader - // contract, but no longer swallow silently — trace the path and - // message so downstream operators see the diagnostic. - Trace.TraceError($"Config load failed: {configurationPath}: {ex.Message}"); - } + return configuration; + }); } return null; @@ -519,48 +535,24 @@ public static T ReadYaml(string path = null) where T : AgentConfiguration if (!string.IsNullOrEmpty(configurationPath)) { - try + return LoadWithTriage(configurationPath, () => { var text = File.ReadAllText(configurationPath); - if (!string.IsNullOrEmpty(text)) - { - var deserializer = new DeserializerBuilder() - .WithNamingConvention(CamelCaseNamingConvention.Instance) - .IgnoreUnmatchedProperties() - .Build(); + if (string.IsNullOrEmpty(text)) return null; + + var deserializer = new DeserializerBuilder() + .WithNamingConvention(CamelCaseNamingConvention.Instance) + .IgnoreUnmatchedProperties() + .Build(); - var configuration = deserializer.Deserialize(text); + var configuration = deserializer.Deserialize(text); + if (configuration != null) + { configuration.Path = configurationPath; configuration.Normalize(); - return configuration; } - } - catch (ArgumentOutOfRangeException ex) - { - // Invalid enum value in the source document — surface the actionable - // setter message with the offending configuration path attached so - // the operator can trace the bad key back to its file. - throw new ArgumentException( - $"Invalid enum value in {configurationPath}: {ex.Message}", - ex); - } - catch (Exception ex) when (UnwrapArgumentOutOfRange(ex) is ArgumentOutOfRangeException aoore) - { - // Deserialisers (YamlDotNet notably) wrap setter throws inside one - // or more layers of their own container exception; walk the - // InnerException chain so a bad-enum config surfaces the same - // actionable message shape as the direct AOORE catch above. - throw new ArgumentException( - $"Invalid enum value in {configurationPath}: {aoore.Message}", - aoore); - } - catch (Exception ex) - { - // Parse / IO / unexpected failures preserve the null-return loader - // contract, but no longer swallow silently — trace the path and - // message so downstream operators see the diagnostic. - Trace.TraceError($"Config load failed: {configurationPath}: {ex.Message}"); - } + return configuration; + }); } return null; @@ -585,48 +577,24 @@ public static AgentConfiguration ReadYaml(Type type, string path = null) if (!string.IsNullOrEmpty(configurationPath)) { - try + return LoadWithTriage(configurationPath, () => { var text = File.ReadAllText(configurationPath); - if (!string.IsNullOrEmpty(text)) - { - var deserializer = new DeserializerBuilder() - .WithNamingConvention(CamelCaseNamingConvention.Instance) - .IgnoreUnmatchedProperties() - .Build(); + if (string.IsNullOrEmpty(text)) return null; - var configuration = (AgentConfiguration)deserializer.Deserialize(text, type); + var deserializer = new DeserializerBuilder() + .WithNamingConvention(CamelCaseNamingConvention.Instance) + .IgnoreUnmatchedProperties() + .Build(); + + var configuration = (AgentConfiguration)deserializer.Deserialize(text, type); + if (configuration != null) + { configuration.Path = configurationPath; configuration.Normalize(); - return configuration; } - } - catch (ArgumentOutOfRangeException ex) - { - // Invalid enum value in the source document — surface the actionable - // setter message with the offending configuration path attached so - // the operator can trace the bad key back to its file. - throw new ArgumentException( - $"Invalid enum value in {configurationPath}: {ex.Message}", - ex); - } - catch (Exception ex) when (UnwrapArgumentOutOfRange(ex) is ArgumentOutOfRangeException aoore) - { - // Deserialisers (YamlDotNet notably) wrap setter throws inside one - // or more layers of their own container exception; walk the - // InnerException chain so a bad-enum config surfaces the same - // actionable message shape as the direct AOORE catch above. - throw new ArgumentException( - $"Invalid enum value in {configurationPath}: {aoore.Message}", - aoore); - } - catch (Exception ex) - { - // Parse / IO / unexpected failures preserve the null-return loader - // contract, but no longer swallow silently — trace the path and - // message so downstream operators see the diagnostic. - Trace.TraceError($"Config load failed: {configurationPath}: {ex.Message}"); - } + return configuration; + }); } return null; From 26780d0b97a80e3d11c06149be047b6db3ce1fe5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Otto=20Boly=C3=B3s?= Date: Fri, 21 Aug 2026 01:10:43 +0200 Subject: [PATCH 20/34] =?UTF-8?q?refactor(config):=20self-mirror=20getter?= =?UTF-8?q?=20=E2=80=94=20dime=20M3-C2?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Programmatic footgun: `new AgentConfiguration { InputValidationLevel = Strict }` followed by reading `DeviceValidationLevel` returned Warning (the const default) instead of Strict, because the load-path Normalize() call is what latched the mirror and programmatic callers weren't required to invoke it. Change the getter to self-compute the mirror in the null case: get => _deviceValidationLevel ?? (DeviceValidationLevel)(int)_inputValidationLevel; Normalize() still assigns the mirror into the backing field so post-Normalize serialization carries the concrete value not null; the getter change closes the gap for callers who never call Normalize(). Both enums share ordinals 0–3 so the direct cast is safe. No test regression: the existing DeviceValidationLevelSplitTests already exercises both the pre-Normalize and post-Normalize paths on the load side; the new getter behavior is a strict superset (programmatic callers now observe the mirror too, not just load-path callers). --- .../Configurations/AgentConfiguration.cs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/libraries/MTConnect.NET-Common/Configurations/AgentConfiguration.cs b/libraries/MTConnect.NET-Common/Configurations/AgentConfiguration.cs index a7f947571..25a76123b 100644 --- a/libraries/MTConnect.NET-Common/Configurations/AgentConfiguration.cs +++ b/libraries/MTConnect.NET-Common/Configurations/AgentConfiguration.cs @@ -158,7 +158,15 @@ public string DefaultVersionValue [JsonPropertyName("deviceValidationLevel")] public DeviceValidationLevel DeviceValidationLevel { - get => _deviceValidationLevel ?? DeviceValidationLevelDefault; + // Self-computed mirror in the null case — a caller that sets + // `InputValidationLevel = Strict` on a fresh AgentConfiguration and + // reads `DeviceValidationLevel` before calling Normalize() sees the + // mirrored value (Strict), not the bare Warning default. Normalize() + // still latches the mirror into the backing field so post-Normalize + // serialisation carries the concrete value rather than null. Dime + // cycle-2 finding M3-C2 — closes the programmatic-only footgun the + // load-path Normalize() call papered over. + get => _deviceValidationLevel ?? (DeviceValidationLevel)(int)_inputValidationLevel; set { ThrowIfUndefined( From 6eb744989c423d312ae37bec56e1339a2297416c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Otto=20Boly=C3=B3s?= Date: Fri, 21 Aug 2026 01:10:58 +0200 Subject: [PATCH 21/34] =?UTF-8?q?docs(agent-validation-events):=20correct?= =?UTF-8?q?=20AddDevice=20example=20=E2=80=94=20dime=20M4-C2?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The contributor-POV code example at line 199 asserted `agent.AddDevice(...) == false` — self-contradicting the earlier line 145 correction (M3 in cycle 1) that documented AddDevice returning `null` on the Strict-rejection path. Rewrite to match the actual contract: capture the returned reference, assert it is null, then keep the fired-event and GetDevices-empty assertions. --- docs/concepts/agent-validation-events.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/concepts/agent-validation-events.md b/docs/concepts/agent-validation-events.md index 2b4455aa2..2493c87ab 100644 --- a/docs/concepts/agent-validation-events.md +++ b/docs/concepts/agent-validation-events.md @@ -196,10 +196,10 @@ The event family is designed to grow. When a new element class becomes validatab var fired = false; agent.InvalidDeviceModelAdded += (_, _, _) => fired = true; - var ok = agent.AddDevice(BrokenDeviceModelFixture()); + var added = agent.AddDevice(BrokenDeviceModelFixture()); + Assert.That(added, Is.Null); Assert.That(fired, Is.True); - Assert.That(ok, Is.False); Assert.That(agent.GetDevices(), Is.Empty); } ``` From 24225963a0172d6c02993065ff07d39bb7c001cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Otto=20Boly=C3=B3s?= Date: Fri, 21 Aug 2026 01:12:52 +0200 Subject: [PATCH 22/34] chore(devices,config): visitedIds self-seed + cap-hit traces + dead code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cycle-2 low-severity cleanups on top of the M2-C2 / M3-C2 refactors: * L1-C2 (Device.cs) — Device.RemoveComposition + Device.RemoveDataItem now seed `visitedIds` with the Device's own Id before recursing, matching the shape Component.RemoveComposition + Component.RemoveDataItem already used. A cyclic Component graph that loops back to `this` now terminates immediately instead of walking one extra frame before the guard fires. * L2-C2 (AgentConfiguration.cs) — UnwrapArgumentOutOfRange now traces a warning when the walk hits MaxUnwrapDepth = 16 with a non-null current frame remaining. A pathological wrapping chain is no longer silently dropped — the operator sees a diagnostic naming the depth cap. * L3-C2 (Device.cs) — Device.RemoveComposition + Device.RemoveDataItem trace a warning before the early-return when depth exceeds MaxComponentWalkDepth = 1024. Same shape H1-C2 introduced for the new Device.RemoveComponent variant. * L4-C2 (AgentConfiguration.cs) — remove the DeviceValidationLevelDefault const. Its sole call site (the DeviceValidationLevel getter) changed under M3-C2 to compute `_deviceValidationLevel ?? (DeviceValidationLevel)(int)_inputValidationLevel`, so the const is now dead code. * L5-C2 (AgentConfiguration.cs) — drop the `_deviceValidationLevel = null;` ctor line. `DeviceValidationLevel?` defaults to null already; the explicit assignment was a no-op. Keep the intent-doc comment (and expand it to name the M3-C2 self-mirror behavior) so the "leave it null on purpose" contract stays visible to future readers. Coverage retained: the existing DeviceRemoveRecursionTests exercises the seed / cap / cycle guards without a change in shape; the depth-ceiling fixtures (RemoveComposition_depth_ceiling_removes_within_1024_leaves_past_1024_intact and its RemoveDataItem sibling) exercise the L3-C2 trace path implicitly via the same > 1024 assertion. --- .../Configurations/AgentConfiguration.cs | 38 ++++++++++--------- .../MTConnect.NET-Common/Devices/Device.cs | 20 +++++++++- 2 files changed, 39 insertions(+), 19 deletions(-) diff --git a/libraries/MTConnect.NET-Common/Configurations/AgentConfiguration.cs b/libraries/MTConnect.NET-Common/Configurations/AgentConfiguration.cs index 25a76123b..58d485010 100644 --- a/libraries/MTConnect.NET-Common/Configurations/AgentConfiguration.cs +++ b/libraries/MTConnect.NET-Common/Configurations/AgentConfiguration.cs @@ -130,20 +130,16 @@ public string DefaultVersionValue // Nullable backing field collapses the pre-fix pair (a non-nullable enum // field plus a parallel `_isDeviceValidationLevelExplicit` boolean) into a - // single is-explicit-or-not signal: null means "no explicit assignment, - // the getter reports the ctor default and Normalize will mirror IVL onto - // it", non-null means "explicitly assigned, do not mirror". Dime cycle-1 - // finding M1 (simplification). + // single is-explicit-or-not signal: null means "no explicit assignment — + // the getter self-mirrors from _inputValidationLevel and Normalize will + // latch the same mirror into the backing field", non-null means + // "explicitly assigned, do not mirror". Dime cycle-1 finding M1 + // (simplification); cycle-2 M3-C2 hardened the null branch to mirror at + // read time so programmatic-only callers observe the same value the + // load-path Normalize() would set. private DeviceValidationLevel? _deviceValidationLevel; private InputValidationLevel _inputValidationLevel; - /// - /// Default Device (MTConnectDevices) validation level surfaced when no - /// explicit assignment has been made. Kept as a named constant so - /// getter, ctor, and Normalize agree on the same fallback. - /// - private const DeviceValidationLevel DeviceValidationLevelDefault = DeviceValidationLevel.Warning; - /// /// Gets or Sets the default Device (MTConnectDevices) validation level. 0 = Ignore, 1 = Warning, 2 = Remove, 3 = Strict. /// @@ -229,12 +225,13 @@ public AgentConfiguration() ObservationBufferSize = 131072; AssetBufferSize = 1024; DefaultVersion = MTConnectVersions.Max; - // Leave _deviceValidationLevel null. Going through the public setter would - // latch it as explicit and disable the load-time migration mirror; the - // getter falls back to DeviceValidationLevelDefault while the backing field - // is null, and Normalize's `??=` populates it from _inputValidationLevel - // during the load path. - _deviceValidationLevel = null; + // Leave _deviceValidationLevel null (its default). Going through the public + // setter would latch it as explicit and disable both the load-time + // migration mirror in Normalize and the read-time self-mirror in the + // getter — a caller that constructed with `{ InputValidationLevel = X }` + // would then observe Warning instead of X. Dime cycle-2 findings M3-C2 + // (getter self-mirror) and L5-C2 (drop the redundant explicit-null + // assignment — `DeviceValidationLevel?` defaults to null already). _inputValidationLevel = InputValidationLevel.Warning; AllowEmptyResultForEnumEvents = false; ConvertUnits = true; @@ -283,6 +280,13 @@ private static ArgumentOutOfRangeException UnwrapArgumentOutOfRange(Exception ex if (current is ArgumentOutOfRangeException aoore) return aoore; current = current.InnerException; } + // Depth-cap hit: the walk gave up before finding an AOORE — trace so a + // pathological wrapping chain does not silently miss a bad-enum surface. + // Dime cycle-2 finding L2-C2. + if (current != null) + { + Trace.TraceWarning($"UnwrapArgumentOutOfRange: exceeded MaxUnwrapDepth={MaxUnwrapDepth}; original AOORE (if any) suppressed"); + } return null; } diff --git a/libraries/MTConnect.NET-Common/Devices/Device.cs b/libraries/MTConnect.NET-Common/Devices/Device.cs index bb78aef59..3d245800b 100644 --- a/libraries/MTConnect.NET-Common/Devices/Device.cs +++ b/libraries/MTConnect.NET-Common/Devices/Device.cs @@ -717,6 +717,10 @@ public void RemoveComposition(string compositionId) if (!Components.IsNullOrEmpty()) { var visitedIds = new HashSet(StringComparer.Ordinal); + // Seed the visited set with this Device's own Id so a cycle back + // to `this` terminates immediately — parity with Component.RemoveComposition + // (dime cycle-2 finding L1-C2). + if (!string.IsNullOrEmpty(Id)) visitedIds.Add(Id); foreach (var component in Components) { RemoveComposition(component, compositionId, visitedIds, depth: 1); @@ -727,7 +731,11 @@ public void RemoveComposition(string compositionId) private void RemoveComposition(IComponent component, string compositionId, HashSet visitedIds, int depth) { if (component == null) return; - if (depth > MaxComponentWalkDepth) return; + if (depth > MaxComponentWalkDepth) + { + Trace.TraceWarning($"Device.RemoveComposition: walk depth {MaxComponentWalkDepth} exceeded; possible cyclic Component graph"); + return; + } // Cycle guard: skip re-entry for a Component whose Id we've already // processed on this walk. Null-Id components fall through the guard @@ -1111,6 +1119,10 @@ public void RemoveDataItem(string dataItemId) if (!Components.IsNullOrEmpty()) { var visitedIds = new HashSet(StringComparer.Ordinal); + // Seed the visited set with this Device's own Id so a cycle back + // to `this` terminates immediately — parity with Component.RemoveDataItem + // (dime cycle-2 finding L1-C2). + if (!string.IsNullOrEmpty(Id)) visitedIds.Add(Id); foreach (var component in Components) { RemoveDataItem(component, dataItemId, visitedIds, depth: 1); @@ -1121,7 +1133,11 @@ public void RemoveDataItem(string dataItemId) private void RemoveDataItem(IComponent component, string dataItemId, HashSet visitedIds, int depth) { if (component == null) return; - if (depth > MaxComponentWalkDepth) return; + if (depth > MaxComponentWalkDepth) + { + Trace.TraceWarning($"Device.RemoveDataItem: walk depth {MaxComponentWalkDepth} exceeded; possible cyclic Component graph"); + return; + } // Cycle guard — mirrors RemoveComposition. if (!string.IsNullOrEmpty(component.Id) && !visitedIds.Add(component.Id)) return; From cd2e730f03edb99bd299778ff0effda60ae826da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Otto=20Boly=C3=B3s?= Date: Fri, 21 Aug 2026 01:16:09 +0200 Subject: [PATCH 23/34] test(config): align Normalize precondition with self-mirror getter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Normalize_Mirrors_InputValidationLevel_When_DeviceValidationLevel_Not_Explicit included a precondition line that asserted `DeviceValidationLevel == Warning` after setting `InputValidationLevel = Remove` and before calling `Normalize()` — pinning the exact pre-M3-C2 behavior that M3-C2 explicitly fixed (the programmatic-only footgun where a caller who forgot to call Normalize() saw the bare const default instead of the mirrored value). Under the M3-C2 self-mirroring getter that pre-Normalize read now returns Remove (via the null-branch mirror), so the precondition assertion fails. Rewrite the precondition to pin the M3-C2 contract explicitly: the getter self-mirrors while `_deviceValidationLevel` is null, and Normalize's role is now to LATCH the mirror into the backing field so post-Normalize serialization carries the concrete value not null. The sticky-suppression semantics still fall out of the null-check inside Normalize. Other tests in the suite that assert `DeviceValidationLevel == Warning` on a freshly constructed AgentConfiguration continue to pass because the ctor sets `_inputValidationLevel = Warning` and the mirror yields the same Warning value — the fixture that failed was the one that DIVERGED the two axes before the precondition read. Verified via full MTConnect.NET-Common-Tests run: 4182/4182 pass. --- .../DeviceValidationLevelMigrationTests.cs | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/tests/MTConnect.NET-Common-Tests/Configurations/DeviceValidationLevelMigrationTests.cs b/tests/MTConnect.NET-Common-Tests/Configurations/DeviceValidationLevelMigrationTests.cs index 0b11e69f2..98a869637 100644 --- a/tests/MTConnect.NET-Common-Tests/Configurations/DeviceValidationLevelMigrationTests.cs +++ b/tests/MTConnect.NET-Common-Tests/Configurations/DeviceValidationLevelMigrationTests.cs @@ -123,19 +123,30 @@ public void ReadJson_InputValidationLevel_Only_Mirrors_All_Arms( // Programmatic Normalize() // --------------------------------------------------------------- - /// Pins that a caller who builds an in code and calls gets the same mirror. + /// Pins that a caller who builds an in code and calls latches the mirror into the backing field. + /// + /// Dime cycle-2 finding M3-C2 hardened the getter to self-mirror in the null case, so the + /// pre-Normalize read already returns in this + /// scenario. Normalize's role is to latch that mirror into the backing field so + /// post-Normalize serialisation carries the concrete value (not null); the sticky-suppression + /// semantics still fall out of the null-check inside . + /// [Test] public void Normalize_Mirrors_InputValidationLevel_When_DeviceValidationLevel_Not_Explicit() { var config = new AgentConfiguration(); config.InputValidationLevel = InputValidationLevel.Remove; - // Precondition: the ctor default is Warning. Verify the assignment above did NOT touch - // DeviceValidationLevel — that is the whole point of the flag. - Assert.That(config.DeviceValidationLevel, Is.EqualTo(DeviceValidationLevel.Warning)); + // Under the M3-C2 self-mirroring getter, the pre-Normalize read already reports the + // mirror — programmatic callers no longer see the bare Warning default just because + // they forgot to call Normalize(). + Assert.That(config.DeviceValidationLevel, Is.EqualTo(DeviceValidationLevel.Remove), + "getter self-mirrors from _inputValidationLevel while _deviceValidationLevel is null — dime M3-C2"); config.Normalize(); + // Post-Normalize the mirror is latched into the backing field; the getter returns the + // same value via the explicit branch now instead of the null-mirror branch. Assert.That(config.DeviceValidationLevel, Is.EqualTo(DeviceValidationLevel.Remove)); } From 8b13a4ec6c3997a52f8e9252e83b48f63be6e83b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Otto=20Boly=C3=B3s?= Date: Fri, 21 Aug 2026 01:33:26 +0200 Subject: [PATCH 24/34] test(common,devices): capture cap-hit traces + Type-overload triage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cycle-3 test-coverage-audit found three FLOOR gaps left open by cycle-2: * L2-C2 trace warning (UnwrapArgumentOutOfRange @ MaxUnwrapDepth = 16) — the depth-ceiling test pinned the null-return but never captured the Trace.TraceWarning line the L2-C2 fix added, so a regression that silently dropped the diagnostic still passed. Attach a TraceListener, invoke the helper with a 20-deep chain, assert the warning fires with the exact "UnwrapArgumentOutOfRange" + "MaxUnwrapDepth=16" strings operators grep on. Paired with a negative pin so the warning never fires on chains shorter than the ceiling. * L3-C2 trace warnings (Device.RemoveComposition / RemoveDataItem / RemoveComponent @ MaxComponentWalkDepth = 1024) — same class of gap: the depth-ceiling tests pinned "deep target survives" but never captured the Trace.TraceWarning line. Attach a TraceListener to each of the three Remove* variants (RemoveComponent inherits from the H1-C2 fix on top of the two cycle-1 variants), invoke on a 1030-deep chain, assert the warning fires with the exact site-name + "walk depth 1024 exceeded" strings. * M2-C2 loader-triage coverage on the Type-taking overloads — the LoadWithTriage extraction affected all four loader entrypoints but only the two generic overloads (ReadJson, ReadYaml, reached transitively via the shortcut ReadJson(path) / ReadYaml(path)) were end-to-end tested. Add explicit fixtures for ReadJson(Type, path) and ReadYaml(Type, path) — invalid-enum → ArgumentException with Path attached, plus a malformed-JSON → null-return contract pin. A regression that reverted the Type-taking overloads to the pre-M2-C2 inline triage (dropping the middle wrapped-AOORE catch) would fail these fixtures where the transitive coverage would not. Also adds two direct pins on the LoadWithTriage helper itself: * `where T : class` constraint enforced via reflection so a regression that widens the constraint (breaking the `return null` fall-through) fails at test time rather than at downstream build. * Trace.TraceError shape on the generic fall-through (path + "Config load failed" prefix) captured via TraceListener so the shared-helper extraction cannot silently degrade the diagnostic on every loader at once. Ten new tests total. Verified on bluefin: MTConnect.NET-Common-Tests: 4192/4192 pass (up from cycle-2 4182). --- .../DeviceValidationLevelMigrationTests.cs | 301 ++++++++++++++++++ .../Devices/DeviceRemoveRecursionTests.cs | 139 ++++++++ 2 files changed, 440 insertions(+) diff --git a/tests/MTConnect.NET-Common-Tests/Configurations/DeviceValidationLevelMigrationTests.cs b/tests/MTConnect.NET-Common-Tests/Configurations/DeviceValidationLevelMigrationTests.cs index 98a869637..8c604b7d3 100644 --- a/tests/MTConnect.NET-Common-Tests/Configurations/DeviceValidationLevelMigrationTests.cs +++ b/tests/MTConnect.NET-Common-Tests/Configurations/DeviceValidationLevelMigrationTests.cs @@ -2,8 +2,10 @@ // TrakHound Inc. licenses this file to you under the MIT license. using System; +using System.Diagnostics; using System.IO; using System.Reflection; +using System.Text; using MTConnect.Agents; using MTConnect.Configurations; using NUnit.Framework; @@ -659,6 +661,305 @@ public void UnwrapArgumentOutOfRange_Returns_Null_On_Null_Input() "the helper must be null-safe — the loop condition current != null guards the very first iteration."); } + // --------------------------------------------------------------- + // Trace-cap-hit diagnostic pin — dime L2-C2 (UnwrapArgumentOutOfRange) + // --------------------------------------------------------------- + // + // The cycle-2 L2-C2 change added a `Trace.TraceWarning` line to + // UnwrapArgumentOutOfRange that fires when the walk exits after + // MaxUnwrapDepth iterations with a non-null current frame — the diagnostic + // tells the operator the walk gave up before finding a wrapped AOORE that + // may exist deeper in the chain. The existing depth-ceiling test + // (UnwrapArgumentOutOfRange_Returns_Null_When_AOORE_Sits_Past_Depth_Ceiling) + // pins the null-return contract but does NOT capture the trace output. + // A regression that removes the TraceWarning line (silently degrading the + // diagnostic) still passes the null-return test. This fixture attaches a + // TraceListener to capture the warning and pins its shape. + + /// + /// Pins that UnwrapArgumentOutOfRange emits a + /// when the walk exits at + /// MaxUnwrapDepth = 16 with a non-null current frame remaining — + /// the diagnostic operators depend on to know a pathological wrapping + /// chain was truncated. Dime cycle-2 finding L2-C2. + /// + [Test] + public void UnwrapArgumentOutOfRange_Traces_Warning_When_MaxUnwrapDepth_Exceeded() + { + var listener = new CapturingTraceListener(); + Trace.Listeners.Add(listener); + try + { + // 20 wrappers around an AOORE puts the AOORE at depth 20 — past + // the MaxUnwrapDepth = 16 ceiling. The walk exits with `current` + // non-null (depth 16 is an InvalidOperationException wrap, not + // the AOORE), so the trace line fires. + var aoore = new ArgumentOutOfRangeException("value", "deep-20"); + var chain = BuildWrappedChain(aoore, wrapCount: 20); + + var result = InvokeUnwrap(chain); + + Assert.That(result, Is.Null, + "precondition — the depth-cap-hit path returns null; the trace warning is what pins the operator-visible diagnostic."); + Assert.That(listener.Warnings.Count, Is.EqualTo(1), + "the L2-C2 fix requires exactly one Trace.TraceWarning per cap-hit call — not zero (regression that dropped the trace) and not more (regression that placed it inside the loop)."); + var warning = listener.Warnings[0]; + Assert.That(warning, Does.Contain("UnwrapArgumentOutOfRange"), + "the warning must name the helper so operators can grep for the specific site."); + Assert.That(warning, Does.Contain("MaxUnwrapDepth=16"), + "the warning must carry the exact ceiling value so a regression that changed the constant surfaces here rather than silently."); + } + finally + { + Trace.Listeners.Remove(listener); + } + } + + /// + /// Pins that UnwrapArgumentOutOfRange does NOT emit the cap-hit + /// trace warning on a chain that terminates naturally before the ceiling + /// (InnerException is null earlier). A regression that fired the warning + /// unconditionally would spam operator logs on every non-enum failure. + /// + [Test] + public void UnwrapArgumentOutOfRange_Does_Not_Trace_Warning_On_Chain_Shorter_Than_Ceiling() + { + var listener = new CapturingTraceListener(); + Trace.Listeners.Add(listener); + try + { + // 5 wrappers around an InvalidOperationException — chain + // terminates at depth 5 (InnerException becomes null), + // no AOORE anywhere, but well within the MaxUnwrapDepth = 16 + // ceiling so the trace warning line MUST NOT fire. + var innermost = new InvalidOperationException("innermost"); + var chain = BuildWrappedChain(innermost, wrapCount: 5); + + var result = InvokeUnwrap(chain); + + Assert.That(result, Is.Null, + "precondition — no AOORE anywhere in the chain so the helper returns null."); + Assert.That(listener.Warnings.Count, Is.EqualTo(0), + "the trace warning must ONLY fire when the depth ceiling is reached; a chain that terminates naturally before the ceiling must not surface the diagnostic."); + } + finally + { + Trace.Listeners.Remove(listener); + } + } + + // --------------------------------------------------------------- + // Loader triage across every overload — dime M2-C2 + // --------------------------------------------------------------- + // + // The M2-C2 refactor extracted the three-clause triage into one + // LoadWithTriage<T> helper shared by all four public loader + // entrypoints: + // + // * ReadJson<T>(string) — the generic entrypoint (also the target of + // the ReadJson(string) shortcut, so the existing + // ReadJson_Invalid_Enum_Ordinal_Throws_ArgumentException_With_Path test + // exercises this path transitively). + // * ReadJson(Type, string) — the Type-taking overload. + // * ReadYaml<T>(string) — the generic entrypoint (also the target of + // the ReadYaml(string) shortcut, exercised transitively above). + // * ReadYaml(Type, string) — the Type-taking overload. + // + // The two Type-taking overloads were previously untested end-to-end. + // A regression that swapped their LoadWithTriage body for the + // pre-M2-C2 inline shape (dropping the middle wrapped-AOORE catch on + // the JSON side, for example) would slip past the existing coverage. + // These fixtures pin the shared-triage contract on every loader. + + /// + /// Pins that the Type-taking ReadJson(Type, path) overload + /// surfaces the invalid-enum diagnostic through the shared + /// LoadWithTriage helper — the M2-C2 extraction requires + /// every overload behave identically. A regression that reverted this + /// overload to an inline catch (dropping the middle wrapped-AOORE clause) + /// would return null instead of the actionable ArgumentException. + /// + [Test] + public void ReadJson_Type_Overload_Invalid_Enum_Ordinal_Throws_ArgumentException_With_Path() + { + var path = WriteTempJson("{\"inputValidationLevel\":42}"); + try + { + var ex = Assert.Throws( + () => AgentConfiguration.ReadJson(typeof(AgentConfiguration), path)); + Assert.That(ex!.Message, Does.Contain(path), + "the Type-taking JSON overload must attach the configuration path — parity with the generic overload."); + Assert.That(ex.Message, Does.Contain("InputValidationLevel"), + "the Type-taking JSON overload must preserve the setter's actionable message."); + } + finally + { + File.Delete(path); + } + } + + /// + /// Sibling pin for the Type-taking ReadYaml(Type, path) overload. + /// The YAML path exercises the wrapped-AOORE clause because YamlDotNet + /// nests the AOORE inside its own container exception. + /// + [Test] + public void ReadYaml_Type_Overload_Invalid_Enum_Ordinal_Throws_ArgumentException_With_Path() + { + var path = WriteTempYaml("inputValidationLevel: 42\n"); + try + { + var ex = Assert.Throws( + () => AgentConfiguration.ReadYaml(typeof(AgentConfiguration), path)); + Assert.That(ex!.Message, Does.Contain(path), + "the Type-taking YAML overload must attach the configuration path — parity with the generic overload."); + Assert.That(ex.Message, Does.Contain("InputValidationLevel"), + "the Type-taking YAML overload must unwrap the deserialiser wrapper and preserve the setter's actionable message."); + } + finally + { + File.Delete(path); + } + } + + /// + /// Pins that the Type-taking ReadJson(Type, path) overload + /// preserves the null-return loader contract on non-enum failures — + /// symmetric with + /// for the generic overload. + /// + [Test] + public void ReadJson_Type_Overload_Malformed_Json_Returns_Null_Preserving_Loader_Contract() + { + var path = WriteTempJson("{ this is not valid json ]"); + try + { + AgentConfiguration config = new AgentConfiguration(); + Assert.DoesNotThrow(() => config = AgentConfiguration.ReadJson(typeof(AgentConfiguration), path), + "non-enum parse failures must not throw — the documented loader contract is null-on-failure."); + Assert.That(config, Is.Null, + "the Type-taking JSON overload must return null for malformed input — parity with the generic overload's contract."); + } + finally + { + File.Delete(path); + } + } + + /// + /// Pins the shared LoadWithTriage<T> helper's + /// where T : class generic constraint — the M2-C2 refactor relies + /// on returning null on the generic fall-through, which requires a + /// reference-type constraint. A regression that dropped the constraint + /// (or widened it to a value-type-permissive shape) would silently + /// compile-error inside the helper body on the return null line; + /// pinning the constraint via reflection catches the change at test time + /// rather than through a downstream build break. + /// + [Test] + public void LoadWithTriage_Has_Class_Constraint_On_T_Parameter() + { + var method = typeof(AgentConfiguration).GetMethod( + "LoadWithTriage", + BindingFlags.Static | BindingFlags.NonPublic); + Assert.That(method, Is.Not.Null, + "LoadWithTriage must exist as a static non-public generic helper on AgentConfiguration — the M2-C2 extraction contract."); + Assert.That(method!.IsGenericMethodDefinition, Is.True, + "LoadWithTriage must remain a generic method — collapsing to a non-generic returning AgentConfiguration would re-force each caller to cast, undoing the extraction."); + var genericArgs = method.GetGenericArguments(); + Assert.That(genericArgs.Length, Is.EqualTo(1), + "LoadWithTriage takes exactly one generic parameter T."); + var attrs = genericArgs[0].GenericParameterAttributes; + Assert.That( + (attrs & System.Reflection.GenericParameterAttributes.ReferenceTypeConstraint) != 0, + Is.True, + "T must carry the ReferenceTypeConstraint (`where T : class`) so `return null` in the generic fall-through compiles — dime M2-C2 shape contract."); + } + + /// + /// Pins that the generic-fall-through branch of LoadWithTriage emits + /// a line naming the configuration + /// path, not just silently returning null. The pre-M2-C2 inline triage + /// contained the same trace line, so this is a shape-preservation pin — + /// a regression that dropped the trace on the shared helper would remove + /// operator diagnostics for every loader at once (blast radius × 4 vs. × 1 + /// pre-extraction). + /// + [Test] + public void LoadWithTriage_Generic_Fall_Through_Traces_Error_With_Path() + { + var listener = new CapturingTraceListener(); + Trace.Listeners.Add(listener); + try + { + var path = WriteTempJson("{ this is not valid json ]"); + try + { + var result = AgentConfiguration.ReadJson(path); + + Assert.That(result, Is.Null, + "precondition — malformed JSON hits the generic fall-through and returns null."); + Assert.That(listener.Errors.Count, Is.GreaterThanOrEqualTo(1), + "the generic-fall-through must emit at least one Trace.TraceError so the operator sees the diagnostic — dime M2-C2 shape preservation."); + Assert.That(listener.Errors[0], Does.Contain(path), + "the error trace must name the configuration path so the operator can trace the failure back to its file."); + Assert.That(listener.Errors[0], Does.Contain("Config load failed"), + "the error trace must carry the documented 'Config load failed' prefix used by the extracted helper."); + } + finally + { + File.Delete(path); + } + } + finally + { + Trace.Listeners.Remove(listener); + } + } + + // --------------------------------------------------------------- + // Trace-listener capture harness + // --------------------------------------------------------------- + + private sealed class CapturingTraceListener : TraceListener + { + public System.Collections.Generic.List Warnings { get; } = new System.Collections.Generic.List(); + public System.Collections.Generic.List Errors { get; } = new System.Collections.Generic.List(); + private readonly StringBuilder _lineBuffer = new StringBuilder(); + + public override void Write(string? message) => _lineBuffer.Append(message); + + public override void WriteLine(string? message) + { + _lineBuffer.Append(message); + // No routing hint — the raw TraceWarning / TraceError calls flow + // through TraceEvent below with a matching event type. The + // Write/WriteLine fallbacks are here so a listener attached to + // a plain Trace.WriteLine still captures. + _lineBuffer.Clear(); + } + + public override void TraceEvent(TraceEventCache? eventCache, string source, TraceEventType eventType, int id, string? message) + { + switch (eventType) + { + case TraceEventType.Warning: + Warnings.Add(message ?? string.Empty); + break; + case TraceEventType.Error: + case TraceEventType.Critical: + Errors.Add(message ?? string.Empty); + break; + } + } + + public override void TraceEvent(TraceEventCache? eventCache, string source, TraceEventType eventType, int id, string? format, params object?[]? args) + { + var message = args != null && args.Length > 0 && format != null ? string.Format(format, args) : format; + TraceEvent(eventCache, source, eventType, id, message); + } + } + // --------------------------------------------------------------- // Fixture harness // --------------------------------------------------------------- diff --git a/tests/MTConnect.NET-Common-Tests/Devices/DeviceRemoveRecursionTests.cs b/tests/MTConnect.NET-Common-Tests/Devices/DeviceRemoveRecursionTests.cs index 8315d7386..a7967b2e3 100644 --- a/tests/MTConnect.NET-Common-Tests/Devices/DeviceRemoveRecursionTests.cs +++ b/tests/MTConnect.NET-Common-Tests/Devices/DeviceRemoveRecursionTests.cs @@ -2,7 +2,9 @@ // TrakHound Inc. licenses this file to you under the MIT license. using System.Collections.Generic; +using System.Diagnostics; using System.Linq; +using System.Text; using MTConnect.Devices; using MTConnect.Devices.DataItems; using NUnit.Framework; @@ -701,5 +703,142 @@ public void Component_RemoveComposition_depth_ceiling_leaves_past_1024_intact() deepLeaf.Compositions.Any(c => c.Id == "target-deep"), Is.True, "Component.RemoveComposition must NOT reach past MaxComponentWalkDepth = 1024 — the Component-side ceiling constant must stay in sync with the Device-side ceiling."); } + + // -------------------------------------------------------------- + // Trace-cap-hit diagnostic pin — dime L3-C2 + // -------------------------------------------------------------- + // + // Cycle-2 L3-C2 added `Trace.TraceWarning` lines to the three + // Device.Remove* private overloads that fire when the walk hits + // depth > MaxComponentWalkDepth = 1024. The existing depth-ceiling + // tests (RemoveComposition_depth_ceiling_removes_within_1024_leaves_past_1024_intact + // and RemoveDataItem_depth_ceiling_removes_within_1024_leaves_past_1024_intact) + // pin the null-effect contract (the deep target survives) but do NOT + // capture the trace output — a regression that silently drops the + // TraceWarning line still passes the intact-target assertions. This + // block attaches a TraceListener and pins the diagnostic shape so + // operators keep the actionable "walk depth 1024 exceeded" hint. + + private sealed class CapturingTraceListener : TraceListener + { + public List Warnings { get; } = new List(); + private readonly StringBuilder _lineBuffer = new StringBuilder(); + public override void Write(string? message) => _lineBuffer.Append(message); + public override void WriteLine(string? message) + { + _lineBuffer.Append(message); + _lineBuffer.Clear(); + } + public override void TraceEvent(TraceEventCache? eventCache, string source, TraceEventType eventType, int id, string? message) + { + if (eventType == TraceEventType.Warning) Warnings.Add(message ?? string.Empty); + } + public override void TraceEvent(TraceEventCache? eventCache, string source, TraceEventType eventType, int id, string? format, params object?[]? args) + { + var message = args != null && args.Length > 0 && format != null ? string.Format(format, args) : format; + TraceEvent(eventCache, source, eventType, id, message); + } + } + + /// + /// Pins the L3-C2 trace-warning shape for + /// . A regression that + /// drops the Trace.TraceWarning line inside the depth-guard + /// early-return still passes the depth-ceiling behavioural test above + /// because the deep-target-survives assertion only observes the + /// null-effect, not the diagnostic. This fixture captures Trace output + /// and asserts the warning fires with the exact shape operators grep on. + /// + [Test] + public void RemoveComposition_depth_ceiling_hit_traces_warning() + { + var listener = new CapturingTraceListener(); + Trace.Listeners.Add(listener); + try + { + var device = BuildLinearDeepDevice(chainLength: 1030, + shallowIndex: 999, deepIndex: 1029, + out _, out var deepId, + out _, out _); + + device.RemoveComposition(deepId); + + Assert.That(listener.Warnings.Any(w => w.Contains("Device.RemoveComposition") && w.Contains("walk depth 1024 exceeded")), + Is.True, + "the depth-cap-hit path must emit a Trace.TraceWarning naming Device.RemoveComposition and the exceeded ceiling — dime L3-C2 diagnostic contract."); + } + finally + { + Trace.Listeners.Remove(listener); + } + } + + /// + /// Sibling L3-C2 pin for . + /// The three Device.Remove* variants share the ceiling AND the trace shape; + /// a regression that dropped the trace on just one variant would slip past + /// the other two variants' pins. + /// + [Test] + public void RemoveDataItem_depth_ceiling_hit_traces_warning() + { + var listener = new CapturingTraceListener(); + Trace.Listeners.Add(listener); + try + { + var device = BuildLinearDeepDevice(chainLength: 1030, + shallowIndex: 999, deepIndex: 1029, + out _, out _, + out _, out var deepLeaf); + const string deepDi = "di-deep-trace"; + deepLeaf.AddDataItem(new DataItem { Id = deepDi, Name = deepDi, Type = "Generic", Category = DataItemCategory.EVENT }); + + device.RemoveDataItem(deepDi); + + Assert.That(listener.Warnings.Any(w => w.Contains("Device.RemoveDataItem") && w.Contains("walk depth 1024 exceeded")), + Is.True, + "the depth-cap-hit path must emit a Trace.TraceWarning naming Device.RemoveDataItem and the exceeded ceiling — dime L3-C2 diagnostic contract."); + } + finally + { + Trace.Listeners.Remove(listener); + } + } + + /// + /// Sibling L3-C2 pin for — + /// the H1-C2 fix added the same depth guard on this Remove* variant. + /// A pathologically deep linear chain past depth 1024 hits the ceiling + /// on the walk into the chain regardless of whether the componentId + /// exists (RemoveComponent walks children and removes matching Ids; + /// on a linear chain with no matching Id, the walk visits every frame + /// until the ceiling fires). + /// + [Test] + public void RemoveComponent_depth_ceiling_hit_traces_warning() + { + var listener = new CapturingTraceListener(); + Trace.Listeners.Add(listener); + try + { + var device = BuildLinearDeepDevice(chainLength: 1030, + shallowIndex: 999, deepIndex: 1029, + out _, out _, + out _, out _); + + // Call with a Component Id that doesn't exist anywhere in the + // chain — the walk visits every frame looking for it, hits the + // ceiling at depth 1025, fires the trace warning. + device.RemoveComponent("does-not-exist"); + + Assert.That(listener.Warnings.Any(w => w.Contains("Device.RemoveComponent") && w.Contains("walk depth 1024 exceeded")), + Is.True, + "the depth-cap-hit path must emit a Trace.TraceWarning naming Device.RemoveComponent and the exceeded ceiling — dime L3-C2 diagnostic contract extended to the H1-C2 addition."); + } + finally + { + Trace.Listeners.Remove(listener); + } + } } } From e9fee5372e2ff6f69cd1bbcdfd9bf5e3cc286cf6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Otto=20Boly=C3=B3s?= Date: Fri, 21 Aug 2026 02:00:57 +0200 Subject: [PATCH 25/34] docs(cli): DVL row default clarifies mirror semantics MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The DVL cell said "Warning" flat, hiding the load-time mirror that has been in force since M3-C2. An operator reading the CLI reference could plausibly conclude a config that omits deviceValidationLevel gets Warning regardless of inputValidationLevel — the opposite of what Normalize() actually does. Cell now says "mirrors inputValidationLevel when omitted (Warning when both are omitted)" and the prose appends one sentence pointing at the Normalize helper so the mechanism is discoverable from either the row or the paragraph. Refs: dime F-DOC-C3-001 --- docs/cli/agent.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/cli/agent.md b/docs/cli/agent.md index 53de5f271..f505d49eb 100644 --- a/docs/cli/agent.md +++ b/docs/cli/agent.md @@ -106,7 +106,7 @@ Backed by `AgentApplicationConfiguration` and the inherited `AgentConfiguration` | `ignoreObservationCase` | bool | `false` | Case-insensitive comparison of incoming observation values to the data item's value-space (CONDITION severities, enum members, etc.). | | `enableValidation` | bool | `false` | Emit per-observation validation diagnostics on the `agent-validation` logger. | | `inputValidationLevel` | enum: `Ignore` (`0`), `Warning` (`1`), `Remove` (`2`), `Strict` (`3`) | `Warning` | What the agent does when an observation or asset arrives that fails per-DataItem validation. `Ignore` accepts everything; `Warning` accepts and logs; `Remove` rejects but does not log; `Strict` rejects and logs. Governs `InvalidObservationAdded` and `InvalidAssetAdded`. | -| `deviceValidationLevel` | enum: `Ignore` (`0`), `Warning` (`1`), `Remove` (`2`), `Strict` (`3`) | `Warning` | What the agent does when device-shape validation fails on a Component, Composition, or DataItem while a Device is being added or normalised. Independent from `inputValidationLevel` — a common integrator profile is `inputValidationLevel: Strict` alongside `deviceValidationLevel: Warning` (reject bad observations, tolerate minor device-model drift). Governs `InvalidComponentAdded`, `InvalidCompositionAdded`, `InvalidDataItemAdded`, and `InvalidDeviceAdded`. | +| `deviceValidationLevel` | enum: `Ignore` (`0`), `Warning` (`1`), `Remove` (`2`), `Strict` (`3`) | mirrors `inputValidationLevel` when omitted (`Warning` when both are omitted) | What the agent does when device-shape validation fails on a Component, Composition, or DataItem while a Device is being added or normalised. Independent from `inputValidationLevel` — a common integrator profile is `inputValidationLevel: Strict` alongside `deviceValidationLevel: Warning` (reject bad observations, tolerate minor device-model drift). Governs `InvalidComponentAdded`, `InvalidCompositionAdded`, `InvalidDataItemAdded`, and `InvalidDeviceAdded`. When the key is omitted, the effective value mirrors `inputValidationLevel` — see `AgentConfiguration.Normalize`. | | `allowEmptyResultForEnumEvents` | bool | `false` | When `true`, preserves an empty Result verbatim on VALUE-representation EVENT DataItems whose Type has a controlled vocabulary (`EXECUTION`, `CONTROLLER_MODE`, `AVAILABILITY`, and every other Event whose value is defined by an `MTConnect.Observations.Events.` enum). Default coerces the empty Result to `UNAVAILABLE`. Free-form String and Numeric-typed Events are unaffected — free-form String preserves the empty Result unconditionally; Numeric-typed always coerces. | | `enableAgentDevice` | bool | `true` | Whether the agent emits its own meta-device (`Agent`) on `/probe`, exposing availability and the `mtconnect:ChangeToken` data item. | | `enableMetrics` | bool | `true` | Emit per-minute observation-rate and asset-update-rate metrics on the `agent-metrics` logger. | From c030d5f7bc5affd1369deefea1ffeb00b330e7aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Otto=20Boly=C3=B3s?= Date: Fri, 21 Aug 2026 02:01:30 +0200 Subject: [PATCH 26/34] chore(devices): tag cap-hit trace warnings with device Id MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The three Device.Remove* cap-hit trace lines named the method but not the device. On a busy fleet with cyclic device graphs, an operator watching Trace output saw "Device.RemoveComponent: walk depth 1024 exceeded" three times a second with no signal about which of forty devices to open in the model viewer. Interpolating [{Id}] between the method name and the message body gives fleet bisection without breaking the existing test assertions — both RemoveRecursionTests substring checks (Contains(method) + Contains("walk depth 1024 exceeded")) span the delta harmlessly. Same pattern on all three Remove* variants because the L3-C2 trace shape is symmetric across Component / Composition / DataItem. Refs: dime F-IMP-C3-001 --- libraries/MTConnect.NET-Common/Devices/Device.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/libraries/MTConnect.NET-Common/Devices/Device.cs b/libraries/MTConnect.NET-Common/Devices/Device.cs index 3d245800b..9b63a371e 100644 --- a/libraries/MTConnect.NET-Common/Devices/Device.cs +++ b/libraries/MTConnect.NET-Common/Devices/Device.cs @@ -464,7 +464,7 @@ private void RemoveComponent(IComponent component, string componentId, HashSet MaxComponentWalkDepth) { - Trace.TraceWarning($"Device.RemoveComponent: walk depth {MaxComponentWalkDepth} exceeded; possible cyclic Component graph"); + Trace.TraceWarning($"Device.RemoveComponent[{Id}]: walk depth {MaxComponentWalkDepth} exceeded; possible cyclic Component graph"); return; } @@ -733,7 +733,7 @@ private void RemoveComposition(IComponent component, string compositionId, HashS if (component == null) return; if (depth > MaxComponentWalkDepth) { - Trace.TraceWarning($"Device.RemoveComposition: walk depth {MaxComponentWalkDepth} exceeded; possible cyclic Component graph"); + Trace.TraceWarning($"Device.RemoveComposition[{Id}]: walk depth {MaxComponentWalkDepth} exceeded; possible cyclic Component graph"); return; } @@ -1135,7 +1135,7 @@ private void RemoveDataItem(IComponent component, string dataItemId, HashSet MaxComponentWalkDepth) { - Trace.TraceWarning($"Device.RemoveDataItem: walk depth {MaxComponentWalkDepth} exceeded; possible cyclic Component graph"); + Trace.TraceWarning($"Device.RemoveDataItem[{Id}]: walk depth {MaxComponentWalkDepth} exceeded; possible cyclic Component graph"); return; } From 8c191012a9748c9aa7b435ece7a1b176066c9799 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Otto=20Boly=C3=B3s?= Date: Fri, 21 Aug 2026 02:02:13 +0200 Subject: [PATCH 27/34] refactor(config): map IVL to DVL through exhaustive switch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The M3-C2 self-mirror getter and the Normalize helper both mirrored via `(DeviceValidationLevel)(int)_inputValidationLevel`. Safe today because both enums are Ignore/Warning/Remove/Strict at ordinals 0–3, but the bit-cast is a static alias with no compile-time signal — a future asymmetric arm on either enum (a new InputValidationLevel or a reorder on DeviceValidationLevel) silently produces an undefined DeviceValidationLevel ordinal at runtime, and the ThrowIfUndefined setter guard is bypassed because the mirror writes the backing field directly. Extracting MapInputToDeviceValidationLevel(InputValidationLevel) with a switch expression makes each mapping explicit: CS8509 fires at build time when a new InputValidationLevel arm lacks a case here, and the default-arm throw surfaces a shipped mismatch at runtime rather than silently corrupting DVL state. Both call sites (getter fallback + Normalize latch) route through the helper. Semantics unchanged for the 0–3 arms both enums currently ship — identical to the pre-change bit-cast — so the M3-C2 tests continue to pass unmodified. Refs: dime F-SEC-002 --- .../Configurations/AgentConfiguration.cs | 36 +++++++++++++++++-- 1 file changed, 33 insertions(+), 3 deletions(-) diff --git a/libraries/MTConnect.NET-Common/Configurations/AgentConfiguration.cs b/libraries/MTConnect.NET-Common/Configurations/AgentConfiguration.cs index 58d485010..3a857afe8 100644 --- a/libraries/MTConnect.NET-Common/Configurations/AgentConfiguration.cs +++ b/libraries/MTConnect.NET-Common/Configurations/AgentConfiguration.cs @@ -161,8 +161,10 @@ public DeviceValidationLevel DeviceValidationLevel // still latches the mirror into the backing field so post-Normalize // serialisation carries the concrete value rather than null. Dime // cycle-2 finding M3-C2 — closes the programmatic-only footgun the - // load-path Normalize() call papered over. - get => _deviceValidationLevel ?? (DeviceValidationLevel)(int)_inputValidationLevel; + // load-path Normalize() call papered over. Cycle-3 F-SEC-002 replaced + // the raw `(DeviceValidationLevel)(int)_inputValidationLevel` cast + // with an exhaustive switch — see MapInputToDeviceValidationLevel. + get => _deviceValidationLevel ?? MapInputToDeviceValidationLevel(_inputValidationLevel); set { ThrowIfUndefined( @@ -260,7 +262,35 @@ public void Normalize() // has latched DeviceValidationLevel to an explicit value. The // sticky-suppression semantics (an explicit DVL assignment beats a // later IVL change on the next Normalize) fall out of the null-check. - _deviceValidationLevel ??= (DeviceValidationLevel)(int)_inputValidationLevel; + _deviceValidationLevel ??= MapInputToDeviceValidationLevel(_inputValidationLevel); + } + + /// + /// Maps an onto its + /// mirror. Both enums currently share + /// ordinals 0-3, so the naive (DeviceValidationLevel)(int)value + /// bit-cast is behaviourally identical today — but the cast is a static + /// alias with no compile-time signal if either enum ever grows an + /// asymmetric arm (or reorders one). An explicit switch expression fires + /// CS8509 when a future arm lacks a + /// mapping, forcing the maintainer to decide the target-side mirror + /// intentionally. The default arm rethrows so a shipped mismatch is + /// surfaced at runtime rather than silently coercing to an undefined + /// value. + /// + /// The source to mirror. + /// The mirror of . + /// Thrown when is not a mapped arm — indicates the enum has grown without a corresponding switch arm here. + private static DeviceValidationLevel MapInputToDeviceValidationLevel(InputValidationLevel value) + { + return value switch + { + InputValidationLevel.Ignore => DeviceValidationLevel.Ignore, + InputValidationLevel.Warning => DeviceValidationLevel.Warning, + InputValidationLevel.Remove => DeviceValidationLevel.Remove, + InputValidationLevel.Strict => DeviceValidationLevel.Strict, + _ => throw new InvalidOperationException($"Unmapped InputValidationLevel ordinal: {(int)value}") + }; } /// From 34114800269e2d1307483aee279205d1c0a03855 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Otto=20Boly=C3=B3s?= Date: Fri, 21 Aug 2026 02:02:43 +0200 Subject: [PATCH 28/34] =?UTF-8?q?docs(config):=20DVL=20remarks=20call=20ou?= =?UTF-8?q?t=20save-latches-mirror=20=E2=80=94=20dime=20F-SEC-001?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The M3-C2 self-mirror getter has a documented-but-subtle consequence under save→reload: an operator running IVL = Strict with no explicit DVL sees DVL serialized as Strict (the getter mirrors at read time), then re-loaded from disk as EXPLICIT (an explicit deviceValidationLevel key is now present in the document), which disables the mirror on subsequent Normalize calls. The previous block mentioned that setting DVL "latches the value as explicit" but did not surface the serialization half of the same latching mechanism — an operator reading the docstring could plausibly conclude the getter self-mirror is stable across save→reload, then be surprised when a runtime IVL change stops mirror- propagating after a config round-trip. Docs-only fix as the safer cycle-3 pick — an alternative would be to serialize the nullable backing field directly and stop mirroring at save time, but that changes the on-disk shape of every configuration that only sets IVL and would need a dedicated round-trip fixture to land safely. Docs preserve today's shape and warn operators; a future cycle can decide the wire-format question in isolation. --- .../Configurations/AgentConfiguration.cs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/libraries/MTConnect.NET-Common/Configurations/AgentConfiguration.cs b/libraries/MTConnect.NET-Common/Configurations/AgentConfiguration.cs index 3a857afe8..cc25f9c3a 100644 --- a/libraries/MTConnect.NET-Common/Configurations/AgentConfiguration.cs +++ b/libraries/MTConnect.NET-Common/Configurations/AgentConfiguration.cs @@ -144,12 +144,28 @@ public string DefaultVersionValue /// Gets or Sets the default Device (MTConnectDevices) validation level. 0 = Ignore, 1 = Warning, 2 = Remove, 3 = Strict. /// /// + /// /// When a configuration file omits this key the loader mirrors /// onto Device validation, preserving pre-v7 behaviour for consumers that only knew the single /// knob. Setting this property — either programmatically or via a /// key present in the source document — latches the value as explicit (the nullable backing field /// becomes non-null) and disables the mirror on the next . An assignment /// whose ordinal is not a defined enum arm raises . + /// + /// + /// Save-latches-mirror behaviour. The getter self-mirrors from + /// when the backing field is still null (i.e. this key was + /// never explicitly set), and JSON/YAML serialisers observe the getter's return value — not the + /// nullable backing field. That means / on a + /// configuration whose DVL was never explicitly set writes the mirrored ordinal into the + /// document. On the next the deserialiser hits an explicit key and + /// latches it, converting the previously implicit mirror into an EXPLICIT stored value. Runtime + /// changes after that reload therefore do NOT re-mirror onto + /// DVL — the operator must clear DVL back to implicit (currently only possible via a fresh + /// construction) or set DVL explicitly. A caller that mutates IVL after a save→reload round-trip + /// and expects DVL to follow should call explicitly on a freshly-loaded + /// configuration BEFORE any programmatic IVL edit. + /// /// [JsonPropertyName("deviceValidationLevel")] public DeviceValidationLevel DeviceValidationLevel From 30380d4cb8ecc8bb1c4a39683b81bf055cbd4b0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Otto=20Boly=C3=B3s?= Date: Fri, 21 Aug 2026 02:03:48 +0200 Subject: [PATCH 29/34] refactor(config): tighten LoadWithTriage + hoist Path/Normalize MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit M2-C2 landed the shared LoadWithTriage triage wrapper but stopped short of one obvious simplification the four loader closures still shared: each stamped `configuration.Path = configurationPath;` and called `configuration.Normalize();` on a non-null return, then threaded the value back through a local + return. Twelve lines of duplication that the helper is already positioned to eat. - Tighten `where T : class` to `where T : AgentConfiguration`. All four call sites already satisfy this (the two generic loaders constrain T the same way; the two Type-overload loaders return AgentConfiguration directly). - Hoist the Path stamp + Normalize call into LoadWithTriage. On a non- null deserializer return the helper now sets Path from the resolved configurationPath argument and calls Normalize before returning. - Reduce each closure to `return .Deserialize(text, ...);` after its options/builder setup. Sixteen lines out of the loader bodies. Semantics unchanged — the helper stamps Path and calls Normalize on exactly the same non-null paths the closures used to, in the same order, before the value escapes the triage. The M3-C2 self-mirror + Normalize precondition tests continue to pass unmodified. Refs: dime F-SIMP-C3-001 --- .../Configurations/AgentConfiguration.cs | 48 ++++++------------- 1 file changed, 15 insertions(+), 33 deletions(-) diff --git a/libraries/MTConnect.NET-Common/Configurations/AgentConfiguration.cs b/libraries/MTConnect.NET-Common/Configurations/AgentConfiguration.cs index cc25f9c3a..d1116d9b0 100644 --- a/libraries/MTConnect.NET-Common/Configurations/AgentConfiguration.cs +++ b/libraries/MTConnect.NET-Common/Configurations/AgentConfiguration.cs @@ -351,14 +351,20 @@ private static ArgumentOutOfRangeException UnwrapArgumentOutOfRange(Exception ex /// like the other three loaders. Sharing /// the same body by construction fixes the asymmetry forever. /// - /// The concrete return type of the deserialiser call. - /// The resolved configuration path — used both in the surfaced message and in the generic-fall-through line. - /// A closure that runs the deserialiser and returns the loaded configuration (or null when the source text was empty). - private static T LoadWithTriage(string configurationPath, Func deserialize) where T : class + /// The concrete return type of the deserialiser call — constrained to so the helper can set and call on the loaded instance. + /// The resolved configuration path — used both in the surfaced message, in the generic-fall-through line, and stamped onto the loaded configuration's property. + /// A closure that runs the deserialiser and returns the loaded configuration (or null when the source text was empty). The helper takes care of stamping and invoking on a non-null return, so the closure only needs to build its deserialiser options and return the deserialised value. + private static T LoadWithTriage(string configurationPath, Func deserialize) where T : AgentConfiguration { try { - return deserialize(); + var configuration = deserialize(); + if (configuration != null) + { + configuration.Path = configurationPath; + configuration.Normalize(); + } + return configuration; } catch (ArgumentOutOfRangeException ex) { @@ -518,13 +524,7 @@ public static T ReadJson(string path = null) where T : AgentConfiguration ReadCommentHandling = JsonCommentHandling.Skip }; - var configuration = JsonSerializer.Deserialize(text, options); - if (configuration != null) - { - configuration.Path = configurationPath; - configuration.Normalize(); - } - return configuration; + return JsonSerializer.Deserialize(text, options); }); } @@ -560,13 +560,7 @@ public static AgentConfiguration ReadJson(Type type, string path = null) ReadCommentHandling = JsonCommentHandling.Skip }; - var configuration = (AgentConfiguration)JsonSerializer.Deserialize(text, type, options); - if (configuration != null) - { - configuration.Path = configurationPath; - configuration.Normalize(); - } - return configuration; + return (AgentConfiguration)JsonSerializer.Deserialize(text, type, options); }); } @@ -603,13 +597,7 @@ public static T ReadYaml(string path = null) where T : AgentConfiguration .IgnoreUnmatchedProperties() .Build(); - var configuration = deserializer.Deserialize(text); - if (configuration != null) - { - configuration.Path = configurationPath; - configuration.Normalize(); - } - return configuration; + return deserializer.Deserialize(text); }); } @@ -645,13 +633,7 @@ public static AgentConfiguration ReadYaml(Type type, string path = null) .IgnoreUnmatchedProperties() .Build(); - var configuration = (AgentConfiguration)deserializer.Deserialize(text, type); - if (configuration != null) - { - configuration.Path = configurationPath; - configuration.Normalize(); - } - return configuration; + return (AgentConfiguration)deserializer.Deserialize(text, type); }); } From 16383ab715aa2af6d36e6f34949d63da470a80e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Otto=20Boly=C3=B3s?= Date: Fri, 21 Aug 2026 02:06:17 +0200 Subject: [PATCH 30/34] test(config): align LoadWithTriage constraint pin with tightened bound MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The shape-contract test asserted `where T : class` via the ReferenceTypeConstraint flag. F-SIMP-C3-001 tightens the constraint to `where T : AgentConfiguration` so the helper can stamp Path and call Normalize on the loaded instance directly — that shape does NOT set the ReferenceTypeConstraint bit (it uses the base-type-constraint list instead), even though it implies reference-type-ness by construction. Rename to LoadWithTriage_Has_AgentConfiguration_Constraint_On_T_Parameter and check for the AgentConfiguration base-type constraint via GetGenericParameterConstraints() — same intent, tighter guarantee: - A regression back to `where T : class` fails this reflection check. - A regression that dropped the constraint entirely still fails because the check requires the AgentConfiguration base type. - A regression that tightened further (e.g. constraining to a concrete subtype) still fails because the AgentConfiguration constraint would be replaced. Docstring re-narrates the M2-C2 origin story + the F-SIMP-C3-001 extension so the intent is discoverable from the test itself. --- .../DeviceValidationLevelMigrationTests.cs | 28 +++++++++++-------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/tests/MTConnect.NET-Common-Tests/Configurations/DeviceValidationLevelMigrationTests.cs b/tests/MTConnect.NET-Common-Tests/Configurations/DeviceValidationLevelMigrationTests.cs index 8c604b7d3..3e5ec195d 100644 --- a/tests/MTConnect.NET-Common-Tests/Configurations/DeviceValidationLevelMigrationTests.cs +++ b/tests/MTConnect.NET-Common-Tests/Configurations/DeviceValidationLevelMigrationTests.cs @@ -848,16 +848,22 @@ public void ReadJson_Type_Overload_Malformed_Json_Returns_Null_Preserving_Loader /// /// Pins the shared LoadWithTriage<T> helper's - /// where T : class generic constraint — the M2-C2 refactor relies - /// on returning null on the generic fall-through, which requires a - /// reference-type constraint. A regression that dropped the constraint - /// (or widened it to a value-type-permissive shape) would silently - /// compile-error inside the helper body on the return null line; - /// pinning the constraint via reflection catches the change at test time + /// where T : AgentConfiguration generic constraint — the M2-C2 + /// refactor relies on returning null on the generic fall-through + /// (requires a reference-type constraint, which the AgentConfiguration + /// base-type constraint implies), and cycle-3 F-SIMP-C3-001 tightened + /// the constraint from the original where T : class so the helper + /// itself can stamp and invoke + /// on the loaded instance + /// without duplicating that tail in every loader closure. A regression + /// that widened the constraint back to class would compile-error + /// inside the helper body on the Path/Normalize lines; a regression + /// that dropped it entirely would compile-error on return null. + /// Pinning both signals via reflection catches the change at test time /// rather than through a downstream build break. /// [Test] - public void LoadWithTriage_Has_Class_Constraint_On_T_Parameter() + public void LoadWithTriage_Has_AgentConfiguration_Constraint_On_T_Parameter() { var method = typeof(AgentConfiguration).GetMethod( "LoadWithTriage", @@ -869,11 +875,11 @@ public void LoadWithTriage_Has_Class_Constraint_On_T_Parameter() var genericArgs = method.GetGenericArguments(); Assert.That(genericArgs.Length, Is.EqualTo(1), "LoadWithTriage takes exactly one generic parameter T."); - var attrs = genericArgs[0].GenericParameterAttributes; + var constraints = genericArgs[0].GetGenericParameterConstraints(); Assert.That( - (attrs & System.Reflection.GenericParameterAttributes.ReferenceTypeConstraint) != 0, - Is.True, - "T must carry the ReferenceTypeConstraint (`where T : class`) so `return null` in the generic fall-through compiles — dime M2-C2 shape contract."); + constraints, + Has.Member(typeof(AgentConfiguration)), + "T must carry the AgentConfiguration base-type constraint (`where T : AgentConfiguration`) so the helper can stamp Path and call Normalize on the loaded instance directly — dime F-SIMP-C3-001 shape contract; `where T : AgentConfiguration` implies the ReferenceTypeConstraint by construction so the `return null` in the generic fall-through still compiles."); } /// From c42d41566511ec5d39508f06330e7cee73481411 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Otto=20Boly=C3=B3s?= Date: Fri, 21 Aug 2026 02:46:59 +0200 Subject: [PATCH 31/34] test(common,config): pin cycle-4 coverage-FLOOR gaps (3 findings) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cycle-4 test-coverage-audit found three FLOOR gaps left open by cycle-3 where the refactor / chore commits landed without a matching pin: * F-SEC-002 (commit 12b11cac) extracted MapInputToDeviceValidationLevel with a switch expression whose default arm THROWS on unmapped ordinals — the whole raison d'être of the refactor. The four mapped arms are exercised transitively via Normalize; the default arm is dead-code from the public API surface (the InputValidationLevel setter guards via ThrowIfUndefined). A regression that swapped `_ => throw new InvalidOperationException(...)` for `_ => default(DeviceValidationLevel)` — the exact static-alias footgun the refactor eliminated — would silently pass every existing test. Reflection-invoke the private static helper with an unmapped ordinal ((InputValidationLevel)99) to pin the default-arm throw + message shape. Paired with an every-arm parametrised pin on the four mapped arms so a transposition (e.g. Remove -> Warning) that happens to align on ordinal-permuted arms doesn't slip past. * F-IMP-C3-001 (commit 16066b69) interpolated [{Id}] between the method name and the message body on all three Device.Remove* cap-hit trace warnings — the fleet-bisection contract that lets operators identify which of forty devices is thrashing. The cycle-3 trace tests pinned `Contains("Device.RemoveComponent")` and `Contains("walk depth 1024 exceeded")` — both pass PRE- and POST-interpolation. A regression that dropped `[{Id}]` would silently revert the diagnostic. Extend each of the three Remove*_depth_ceiling_hit_traces_warning tests with a `Does.Contain("Device.RemoveXxx[d1]")` assertion pinning the interpolated form. * F-SIMP-C3-001 (commit 5c96dba2) hoisted `configuration.Path = configurationPath` + `configuration.Normalize()` from the four loader closures INTO LoadWithTriage. The M2-C2 constraint pin catches shape regressions; the generic-fall-through TraceError pin catches diagnostic drops. What was NOT pinned: the Path stamp end-to-end. A regression that dropped `configuration.Path = configurationPath` from the helper would leave AgentConfiguration.Path null after every load — silently breaking downstream save/relative-resolve paths (the Path docstring names it "the default target when the configuration is saved"). Add sibling ReadJson / ReadYaml pins asserting Path == input-path after load. 7 new tests + 3 modified — bluefin dotnet test tips 10/10 green on the delta (10 selected of Common-Tests 4192). --- .../DeviceValidationLevelMigrationTests.cs | 135 ++++++++++++++++++ .../Devices/DeviceRemoveRecursionTests.cs | 9 ++ 2 files changed, 144 insertions(+) diff --git a/tests/MTConnect.NET-Common-Tests/Configurations/DeviceValidationLevelMigrationTests.cs b/tests/MTConnect.NET-Common-Tests/Configurations/DeviceValidationLevelMigrationTests.cs index 3e5ec195d..9295fa8b4 100644 --- a/tests/MTConnect.NET-Common-Tests/Configurations/DeviceValidationLevelMigrationTests.cs +++ b/tests/MTConnect.NET-Common-Tests/Configurations/DeviceValidationLevelMigrationTests.cs @@ -923,6 +923,141 @@ public void LoadWithTriage_Generic_Fall_Through_Traces_Error_With_Path() } } + // --------------------------------------------------------------- + // F-SEC-002 — MapInputToDeviceValidationLevel exhaustive switch, + // default-arm throw on unmapped ordinal + // --------------------------------------------------------------- + + /// + /// Pins the default arm of the MapInputToDeviceValidationLevel + /// switch expression that cycle-3 F-SEC-002 introduced. The four mapped + /// arms (Ignore / Warning / Remove / Strict) are exercised indirectly by + /// Normalize_Mirrors_Every_InputValidationLevel_Arm above — but + /// the default arm, whose entire raison d'être is to fail loudly rather + /// than silently coerce to default(DeviceValidationLevel), has + /// no test on the mapped-arm side. A regression that swapped + /// _ => throw new InvalidOperationException(...) for + /// _ => default(DeviceValidationLevel) — the exact footgun the + /// refactor eliminated — would silently pass every existing test and + /// re-introduce the runtime coercion the switch was written to prevent. + /// + /// The public setter InputValidationLevel = ... now guards via + /// ThrowIfUndefined, so the default arm cannot be reached + /// through the normal API surface — the private backing field must be + /// poked. Reflection-invoking the private static helper directly with + /// an unmapped ordinal is the minimal, precise pin. + /// + [Test] + public void MapInputToDeviceValidationLevel_Default_Arm_Throws_InvalidOperationException_On_Unmapped_Ordinal() + { + var method = typeof(AgentConfiguration).GetMethod( + "MapInputToDeviceValidationLevel", + BindingFlags.Static | BindingFlags.NonPublic); + Assert.That(method, Is.Not.Null, + "MapInputToDeviceValidationLevel must exist as a static non-public helper on AgentConfiguration — the F-SEC-002 refactor contract."); + + var unmapped = (InputValidationLevel)99; + var ex = Assert.Throws( + () => method!.Invoke(null, new object[] { unmapped })); + Assert.That(ex!.InnerException, Is.InstanceOf(), + "the default arm of the switch expression must throw InvalidOperationException — dime F-SEC-002 hard-fail contract; a regression to `_ => default(DeviceValidationLevel)` would silently coerce unmapped ordinals to Ignore, re-introducing the exact static-alias footgun the exhaustive switch eliminated."); + Assert.That(ex.InnerException!.Message, Does.Contain("Unmapped InputValidationLevel"), + "the InvalidOperationException message must name the enum type so the operator sees which mirror-mapping table is stale."); + Assert.That(ex.InnerException.Message, Does.Contain("99"), + "the InvalidOperationException message must carry the offending ordinal so a shipped mismatch can be diagnosed from the trace alone."); + } + + /// + /// Pins that each of the four arms + /// maps to its ordinal-matching arm + /// through the F-SEC-002 helper directly (not just transitively via + /// Normalize). A regression that transposed two arms in the + /// switch expression — e.g. mapped InputValidationLevel.Remove + /// to DeviceValidationLevel.Warning — would leave the transitive + /// tests passing when the enum ordinals happened to align on the + /// permuted arms; the direct pin fails on the transposition. + /// + [TestCase(InputValidationLevel.Ignore, DeviceValidationLevel.Ignore)] + [TestCase(InputValidationLevel.Warning, DeviceValidationLevel.Warning)] + [TestCase(InputValidationLevel.Remove, DeviceValidationLevel.Remove)] + [TestCase(InputValidationLevel.Strict, DeviceValidationLevel.Strict)] + public void MapInputToDeviceValidationLevel_Every_Mapped_Arm_Returns_Ordinal_Mirror( + InputValidationLevel input, DeviceValidationLevel expected) + { + var method = typeof(AgentConfiguration).GetMethod( + "MapInputToDeviceValidationLevel", + BindingFlags.Static | BindingFlags.NonPublic); + Assert.That(method, Is.Not.Null, + "MapInputToDeviceValidationLevel must exist as a static non-public helper on AgentConfiguration — the F-SEC-002 refactor contract."); + + var result = (DeviceValidationLevel)method!.Invoke(null, new object[] { input })!; + Assert.That(result, Is.EqualTo(expected), + $"MapInputToDeviceValidationLevel({input}) must return {expected} — dime F-SEC-002 explicit-switch mapping contract; a transposed arm would flip the DVL mirror silently."); + } + + // --------------------------------------------------------------- + // F-SIMP-C3-001 — LoadWithTriage stamps configuration.Path on the + // returned instance (hoisted from the four loader closures) + // --------------------------------------------------------------- + + /// + /// Pins that stamps the + /// loaded configuration's property + /// with the file the config was loaded from. The M2-C2 refactor extracted + /// the triage wrapper and cycle-3 F-SIMP-C3-001 hoisted the + /// configuration.Path = configurationPath assignment INTO the + /// helper (out of the four closures). A regression that dropped the + /// hoisted assignment would leave Path null after every load, + /// silently breaking downstream file-relative resolutions (the Path + /// property's docstring names it "the default target when the + /// configuration is saved") — a diagnostic-silent behaviour break. + /// + [Test] + public void ReadJson_Stamps_Configuration_Path_On_Loaded_Instance() + { + var path = WriteTempJson("{\"inputValidationLevel\":1}"); + try + { + var config = AgentConfiguration.ReadJson(path); + + Assert.That(config, Is.Not.Null, + "precondition — the loader must not have swallowed a valid config."); + Assert.That(config!.Path, Is.EqualTo(path), + "ReadJson must stamp AgentConfiguration.Path with the file the config was loaded from — dime F-SIMP-C3-001 hoisted-stamp contract; a regression that dropped `configuration.Path = configurationPath` from LoadWithTriage would silently leave downstream save/relative-resolve paths null."); + } + finally + { + File.Delete(path); + } + } + + /// + /// Sibling of the JSON test — + /// must also stamp . The four loader + /// entrypoints share LoadWithTriage, so a regression that dropped + /// the hoisted stamp would fail on every loader in parallel; pinning both + /// JSON and YAML surfaces catches an unlikely path-specific regression + /// (e.g. a partial revert that only touched the YAML closure). + /// + [Test] + public void ReadYaml_Stamps_Configuration_Path_On_Loaded_Instance() + { + var path = WriteTempYaml("inputValidationLevel: 1\n"); + try + { + var config = AgentConfiguration.ReadYaml(path); + + Assert.That(config, Is.Not.Null, + "precondition — the loader must not have swallowed a valid config."); + Assert.That(config!.Path, Is.EqualTo(path), + "ReadYaml must stamp AgentConfiguration.Path with the file the config was loaded from — dime F-SIMP-C3-001 hoisted-stamp contract; sibling of the JSON pin."); + } + finally + { + File.Delete(path); + } + } + // --------------------------------------------------------------- // Trace-listener capture harness // --------------------------------------------------------------- diff --git a/tests/MTConnect.NET-Common-Tests/Devices/DeviceRemoveRecursionTests.cs b/tests/MTConnect.NET-Common-Tests/Devices/DeviceRemoveRecursionTests.cs index a7967b2e3..d382fe5cc 100644 --- a/tests/MTConnect.NET-Common-Tests/Devices/DeviceRemoveRecursionTests.cs +++ b/tests/MTConnect.NET-Common-Tests/Devices/DeviceRemoveRecursionTests.cs @@ -766,6 +766,9 @@ public void RemoveComposition_depth_ceiling_hit_traces_warning() Assert.That(listener.Warnings.Any(w => w.Contains("Device.RemoveComposition") && w.Contains("walk depth 1024 exceeded")), Is.True, "the depth-cap-hit path must emit a Trace.TraceWarning naming Device.RemoveComposition and the exceeded ceiling — dime L3-C2 diagnostic contract."); + Assert.That(listener.Warnings.Any(w => w.Contains("Device.RemoveComposition[d1]")), + Is.True, + "the depth-cap-hit trace warning must interpolate the device Id in [Id] brackets between the method name and the message body — dime F-IMP-C3-001 fleet-bisection contract; a regression that dropped the [d1] tag would silently revert the operator-diagnostic improvement."); } finally { @@ -798,6 +801,9 @@ public void RemoveDataItem_depth_ceiling_hit_traces_warning() Assert.That(listener.Warnings.Any(w => w.Contains("Device.RemoveDataItem") && w.Contains("walk depth 1024 exceeded")), Is.True, "the depth-cap-hit path must emit a Trace.TraceWarning naming Device.RemoveDataItem and the exceeded ceiling — dime L3-C2 diagnostic contract."); + Assert.That(listener.Warnings.Any(w => w.Contains("Device.RemoveDataItem[d1]")), + Is.True, + "the depth-cap-hit trace warning must interpolate the device Id in [Id] brackets between the method name and the message body — dime F-IMP-C3-001 fleet-bisection contract; a regression that dropped the [d1] tag would silently revert the operator-diagnostic improvement."); } finally { @@ -834,6 +840,9 @@ public void RemoveComponent_depth_ceiling_hit_traces_warning() Assert.That(listener.Warnings.Any(w => w.Contains("Device.RemoveComponent") && w.Contains("walk depth 1024 exceeded")), Is.True, "the depth-cap-hit path must emit a Trace.TraceWarning naming Device.RemoveComponent and the exceeded ceiling — dime L3-C2 diagnostic contract extended to the H1-C2 addition."); + Assert.That(listener.Warnings.Any(w => w.Contains("Device.RemoveComponent[d1]")), + Is.True, + "the depth-cap-hit trace warning must interpolate the device Id in [Id] brackets between the method name and the message body — dime F-IMP-C3-001 fleet-bisection contract; a regression that dropped the [d1] tag would silently revert the operator-diagnostic improvement."); } finally { From 90b5bea61a5af7d8ccbb0c525538c424d873392a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Otto=20Boly=C3=B3s?= Date: Fri, 21 Aug 2026 18:39:14 +0200 Subject: [PATCH 32/34] fix(common): rewrite AgentConfiguration.cs C#8 features to C#7.3 for multi-TFM compat MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Normalize() null-coalescing assignment (`??=`, line 281) and the MapInputToDeviceValidationLevel switch expression (`return value switch { ... }`, lines 302-309, both introduced by 3dc33835c on this branch) require C# 8. The multi-TFM Release pack builds against net461/net47/net462/net471, which default to LangVersion 7.3 — CS8370 fires there on every framework in the matrix, blocking the tail integration build. Rewrites to C# 7.3-compatible idioms: * `_deviceValidationLevel ??= X` -> `if (_deviceValidationLevel == null) _deviceValidationLevel = X` * switch expression -> classical switch statement (default arm preserves throw) Semantic-preserving pure-syntax swap — no behavior change; existing DVL migration + normalize + enum-arm tests continue to cover the mapping. Per Otto's "use the features of the oldest language version. Later we can bump the version to a newer one which is gated by the maintainer's decision but I believe we can always bump it without breaking changes to the latest language version of the oldest TFM" directive 2026-08-21. Attribution correction: the offending sites were introduced on this branch (#241) via commit 3dc33835c, not on #222 as the initial tail-sweep bug report suggested (the CS8370 site listing was routed to #222 because #222 owns the Sender addition on the same file; commit blame shows #241 owns the C# 8 sites). --- .../Configurations/AgentConfiguration.cs | 40 +++++++++++++------ 1 file changed, 27 insertions(+), 13 deletions(-) diff --git a/libraries/MTConnect.NET-Common/Configurations/AgentConfiguration.cs b/libraries/MTConnect.NET-Common/Configurations/AgentConfiguration.cs index d1116d9b0..85d1b0534 100644 --- a/libraries/MTConnect.NET-Common/Configurations/AgentConfiguration.cs +++ b/libraries/MTConnect.NET-Common/Configurations/AgentConfiguration.cs @@ -273,12 +273,18 @@ public AgentConfiguration() /// public void Normalize() { - // `??=` assigns the mirror only when the backing field is still null — - // i.e. neither a programmatic setter call nor a source-document key - // has latched DeviceValidationLevel to an explicit value. The - // sticky-suppression semantics (an explicit DVL assignment beats a - // later IVL change on the next Normalize) fall out of the null-check. - _deviceValidationLevel ??= MapInputToDeviceValidationLevel(_inputValidationLevel); + // Explicit null-check + assign assigns the mirror only when the + // backing field is still null — i.e. neither a programmatic setter + // call nor a source-document key has latched DeviceValidationLevel + // to an explicit value. The sticky-suppression semantics (an + // explicit DVL assignment beats a later IVL change on the next + // Normalize) fall out of the null-check. Written as a plain + // `if (x == null) x = y` rather than the C# 8 `??=` operator so + // the multi-TFM Release pack compiles under the oldest target + // framework's language version (net461/net47 default to C# 7.3); + // per Otto's "use the features of the oldest language version" + // directive 2026-08-21. + if (_deviceValidationLevel == null) _deviceValidationLevel = MapInputToDeviceValidationLevel(_inputValidationLevel); } /// @@ -297,16 +303,24 @@ public void Normalize() /// The source to mirror. /// The mirror of . /// Thrown when is not a mapped arm — indicates the enum has grown without a corresponding switch arm here. + /// + /// Written as a classical switch statement rather than the C# 8 switch + /// expression so the multi-TFM Release pack compiles under the oldest target + /// framework's language version (net461/net47 default to C# 7.3); the + /// exhaustive default arm preserves the runtime-throw semantics on any + /// unmapped ordinal. Per Otto's "use the features of the oldest language + /// version" directive 2026-08-21. + /// private static DeviceValidationLevel MapInputToDeviceValidationLevel(InputValidationLevel value) { - return value switch + switch (value) { - InputValidationLevel.Ignore => DeviceValidationLevel.Ignore, - InputValidationLevel.Warning => DeviceValidationLevel.Warning, - InputValidationLevel.Remove => DeviceValidationLevel.Remove, - InputValidationLevel.Strict => DeviceValidationLevel.Strict, - _ => throw new InvalidOperationException($"Unmapped InputValidationLevel ordinal: {(int)value}") - }; + case InputValidationLevel.Ignore: return DeviceValidationLevel.Ignore; + case InputValidationLevel.Warning: return DeviceValidationLevel.Warning; + case InputValidationLevel.Remove: return DeviceValidationLevel.Remove; + case InputValidationLevel.Strict: return DeviceValidationLevel.Strict; + default: throw new InvalidOperationException($"Unmapped InputValidationLevel ordinal: {(int)value}"); + } } /// From 111e577c814f2826be93882dda531c87ecb5fb85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Otto=20Boly=C3=B3s?= Date: Sat, 22 Aug 2026 11:20:38 +0200 Subject: [PATCH 33/34] chore: dotnet format drift baseline compliance --- .../Agents/DeviceValidationLevelNormalizeDeviceTests.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/MTConnect.NET-Common-Tests/Agents/DeviceValidationLevelNormalizeDeviceTests.cs b/tests/MTConnect.NET-Common-Tests/Agents/DeviceValidationLevelNormalizeDeviceTests.cs index 181201e8e..f8764db65 100644 --- a/tests/MTConnect.NET-Common-Tests/Agents/DeviceValidationLevelNormalizeDeviceTests.cs +++ b/tests/MTConnect.NET-Common-Tests/Agents/DeviceValidationLevelNormalizeDeviceTests.cs @@ -78,10 +78,10 @@ public void DeviceValidationLevel_arms_and_ordinals_are_stable() DeviceValidationLevel.Remove, DeviceValidationLevel.Strict, })); - Assert.That((int)DeviceValidationLevel.Ignore, Is.EqualTo(0)); + Assert.That((int)DeviceValidationLevel.Ignore, Is.EqualTo(0)); Assert.That((int)DeviceValidationLevel.Warning, Is.EqualTo(1)); - Assert.That((int)DeviceValidationLevel.Remove, Is.EqualTo(2)); - Assert.That((int)DeviceValidationLevel.Strict, Is.EqualTo(3)); + Assert.That((int)DeviceValidationLevel.Remove, Is.EqualTo(2)); + Assert.That((int)DeviceValidationLevel.Strict, Is.EqualTo(3)); } /// Pins the AgentConfiguration default: is . From 5ab4a39851b7670f96fceb5620c82e9ba2ef8508 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Otto=20Boly=C3=B3s?= Date: Sat, 22 Aug 2026 12:17:00 +0200 Subject: [PATCH 34/34] fix(docs-gen): register DeviceValidationLevel + InputValidationLevel in RenderType mapping --- build/MTConnect.NET-DocsGen/Renderers.cs | 26 +++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/build/MTConnect.NET-DocsGen/Renderers.cs b/build/MTConnect.NET-DocsGen/Renderers.cs index a3b52b7db..32671116c 100644 --- a/build/MTConnect.NET-DocsGen/Renderers.cs +++ b/build/MTConnect.NET-DocsGen/Renderers.cs @@ -383,12 +383,36 @@ public static string Render(IReadOnlyList classes) sb.AppendLine("| --- | --- | --- | --- |"); foreach (var p in c.Properties) { - sb.AppendLine($"| `{p.SerialisedKey}` | `{p.Name}` | `{Escape(p.Type)}` | {Escape(p.Summary)} |"); + sb.AppendLine($"| `{p.SerialisedKey}` | `{p.Name}` | {RenderType(p.Type)} | {Escape(p.Summary)} |"); } sb.AppendLine(); } return sb.ToString(); } + /// + /// Maps a config property's type name to its authored docfx API page, + /// keyed on exact type-name equality (no prefix/substring matching). + /// + private static readonly IReadOnlyDictionary TypeApiLinks = new Dictionary + { + ["DeviceValidationLevel"] = "/api/MTConnect.Agents.DeviceValidationLevel", + ["InputValidationLevel"] = "/api/MTConnect.Agents.InputValidationLevel", + }; + + /// + /// Renders the Type column for a config property: a markdown link into + /// the docfx API namespace for types with an authored API page (see + /// ), or a plain backtick-fenced type name + /// otherwise. The type name is escaped before either shape is emitted. + /// + private static string RenderType(string type) + { + var escaped = Escape(type); + return TypeApiLinks.TryGetValue(type, out var href) + ? $"[`{escaped}`]({href})" + : $"`{escaped}`"; + } + private static string Escape(string s) => s.Replace("|", "\\|").Replace("\n", " ").Replace("\r", " "); }