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", " "); } 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. | diff --git a/docs/concepts/agent-validation-events.md b/docs/concepts/agent-validation-events.md index 7d702f182..2493c87ab 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 @@ -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); } ``` diff --git a/docs/reference/configuration.md b/docs/reference/configuration.md index b76a2be12..5e58d22bb 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 | @@ -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 | 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 +} 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 fd873c6c7..85d1b0534 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; @@ -127,17 +128,87 @@ public string DefaultVersionValue [JsonPropertyName("enableValidation")] public bool EnableValidation { get; set; } - /// - /// Gets or Sets the default Device (MTConnectDevices) validation level. 0 = Ignore, 1 = Warning, 2 = Remove, 3 = Strict - /// + // 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 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; + + /// + /// 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 { get; set; } + public DeviceValidationLevel DeviceValidationLevel + { + // 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. Cycle-3 F-SEC-002 replaced + // the raw `(DeviceValidationLevel)(int)_inputValidationLevel` cast + // with an exhaustive switch — see MapInputToDeviceValidationLevel. + get => _deviceValidationLevel ?? MapInputToDeviceValidationLevel(_inputValidationLevel); + set + { + ThrowIfUndefined( + value, + "DeviceValidationLevel must be one of Ignore (0), Warning (1), Remove (2), Strict (3)."); + _deviceValidationLevel = value; + } + } /// - /// 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 + { + ThrowIfUndefined( + 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 +243,14 @@ public AgentConfiguration() ObservationBufferSize = 131072; AssetBufferSize = 1024; DefaultVersion = MTConnectVersions.Max; - DeviceValidationLevel = DeviceValidationLevel.Warning; - InputValidationLevel = InputValidationLevel.Warning; + // 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; IgnoreObservationCase = false; @@ -181,6 +258,185 @@ 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() + { + // 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); + } + + /// + /// 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. + /// + /// 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) + { + switch (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}"); + } + } + + /// + /// 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; + } + // 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; + } + + /// + /// 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 — 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 + { + var configuration = deserialize(); + 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, 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 + /// 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. @@ -272,22 +528,18 @@ 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 - }; - - var configuration = JsonSerializer.Deserialize(text, options); - configuration.Path = configurationPath; - return configuration; - } - } - catch { } + ReadCommentHandling = JsonCommentHandling.Skip + }; + + return JsonSerializer.Deserialize(text, options); + }); } return null; @@ -312,22 +564,18 @@ 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 - }; - - var configuration = (AgentConfiguration)JsonSerializer.Deserialize(text, type, options); - configuration.Path = configurationPath; - return configuration; - } - } - catch { } + ReadCommentHandling = JsonCommentHandling.Skip + }; + + return (AgentConfiguration)JsonSerializer.Deserialize(text, type, options); + }); } return null; @@ -353,22 +601,18 @@ 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(); - - var configuration = deserializer.Deserialize(text); - configuration.Path = configurationPath; - return configuration; - } - } - catch { } + if (string.IsNullOrEmpty(text)) return null; + + var deserializer = new DeserializerBuilder() + .WithNamingConvention(CamelCaseNamingConvention.Instance) + .IgnoreUnmatchedProperties() + .Build(); + + return deserializer.Deserialize(text); + }); } return null; @@ -393,22 +637,18 @@ 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(); - - var configuration = (AgentConfiguration)deserializer.Deserialize(text, type); - configuration.Path = configurationPath; - return configuration; - } - } - catch { } + if (string.IsNullOrEmpty(text)) return null; + + var deserializer = new DeserializerBuilder() + .WithNamingConvention(CamelCaseNamingConvention.Instance) + .IgnoreUnmatchedProperties() + .Build(); + + return (AgentConfiguration)deserializer.Deserialize(text, type); + }); } return null; 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; } 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 5c543ca37..9b63a371e 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[{Id}]: 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; @@ -658,11 +682,28 @@ 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. 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. 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) { + // Top-level Compositions collection on the Device itself. if (!Compositions.IsNullOrEmpty()) { var compositions = new List(); @@ -671,17 +712,56 @@ 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 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); + } + } } - 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) + { + Trace.TraceWarning($"Device.RemoveComposition[{Id}]: 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 + // (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()) { 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, visitedIds, depth + 1); + } } } @@ -1011,23 +1091,70 @@ 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 + /// 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) { - var components = GetComponents(); - if (!components.IsNullOrEmpty()) + // Top-level DataItems on the Device itself. The pre-fix override + // skipped this collection entirely. + if (!DataItems.IsNullOrEmpty()) { - foreach (var component in components) + var dataItems = new List(); + dataItems.AddRange(DataItems); + dataItems.RemoveAll(o => o.Id == dataItemId); + DataItems = dataItems; + } + + // Child Components' DataItems, walked with an explicit cycle guard. + 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) { - 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) + { + Trace.TraceWarning($"Device.RemoveDataItem[{Id}]: walk depth {MaxComponentWalkDepth} exceeded; possible cyclic Component graph"); + 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/Agents/DeviceValidationLevelEnumArmTests.cs b/tests/MTConnect.NET-Common-Tests/Agents/DeviceValidationLevelEnumArmTests.cs new file mode 100644 index 000000000..27e2955bc --- /dev/null +++ b/tests/MTConnect.NET-Common-Tests/Agents/DeviceValidationLevelEnumArmTests.cs @@ -0,0 +1,350 @@ +// 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 + /// 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 MTConnectAgent.NormalizeDevice + /// (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. + // ----------------------------------------------------------------- + + /// + /// 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)] + [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; + } + } + + /// + /// 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)] + [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: + // 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 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)); + Assert.That(added, Is.Null, "Strict must null the device on the first invalid Composition."); + break; + } + } + + /// + /// 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)] + [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: + // 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 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)); + 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); + } + } +} 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..f8764db65 --- /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, + }; + } + } +} 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..9295fa8b4 --- /dev/null +++ b/tests/MTConnect.NET-Common-Tests/Configurations/DeviceValidationLevelMigrationTests.cs @@ -0,0 +1,1122 @@ +// Copyright (c) 2026 TrakHound Inc., All Rights Reserved. +// 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; + +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 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; + + // 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)); + } + + /// 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)); + } + + /// + /// 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 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] + 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 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 populated the nullable backing field to non-null. Later + // IVL assignment must not reset it to null. + 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 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)] + [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); + } + } + + // --------------------------------------------------------------- + // 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); + } + } + + // --------------------------------------------------------------- + // 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."); + } + + // --------------------------------------------------------------- + // 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 : 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_AgentConfiguration_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 constraints = genericArgs[0].GetGenericParameterConstraints(); + Assert.That( + 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."); + } + + /// + /// 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); + } + } + + // --------------------------------------------------------------- + // 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 + // --------------------------------------------------------------- + + 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 + // --------------------------------------------------------------- + + private static string WriteTempJson(string json) + { + var path = Path.Combine(Path.GetTempPath(), $"agent-config-{Guid.NewGuid():N}.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 new file mode 100644 index 000000000..d382fe5cc --- /dev/null +++ b/tests/MTConnect.NET-Common-Tests/Devices/DeviceRemoveRecursionTests.cs @@ -0,0 +1,853 @@ +// 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.Diagnostics; +using System.Linq; +using System.Text; +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 + /// 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, + /// 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."); + } + + // -------------------------------------------------------------- + // 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."); + } + + // -------------------------------------------------------------- + // 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 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 + /// 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."); + } + + // -------------------------------------------------------------- + // 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."); + } + + // -------------------------------------------------------------- + // 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."); + 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 + { + 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."); + 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 + { + 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."); + 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 + { + Trace.Listeners.Remove(listener); + } + } + } +} 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 }); + } +}