From b68c3133727c79b5a0a774a6527b8517bb964039 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 16 Jul 2026 06:01:10 -0400 Subject: [PATCH] feat(uns-v3): implement EffectiveNameGuard authoring-time uniqueness guard (B3-WP2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements IEffectiveNameGuard (the Wave-A contract): per-equipment effective-name uniqueness across UnsTagReferences (DisplayNameOverride else backing raw tag Name), VirtualTags (Name), and ScriptedAlarms (Name). Ordinal comparison, self-row exclusion by (kind, id), one pooled context per CheckAsync call. Registered Scoped in EndpointRouteBuilderExtensions so WP1's UnsTreeService consumer goes live. Verified the deploy-time DraftValidator UnsEffectiveNameCollision rule already computes reference effective names from the backing raw tag's current Name (catches rename-induced collisions), spans all three sources, compares ordinal, and names both colliding sources + the equipment — no hardening needed. DraftSnapshotFactory already loads Tags/UnsTagReferences/VirtualTags/ScriptedAlarms. Tests: 8 guard tests (in-memory EF) + 3 deploy-gate tests (rename-induced collision, override-vs-VT, ordinal case-sensitivity). All green. Claude-Session: https://claude.ai/code/session_01LVneM3eh1UtJxEisFXgmox --- .../EndpointRouteBuilderExtensions.cs | 4 + .../Uns/EffectiveNameGuard.cs | 106 +++++++++ .../DraftValidatorTests.cs | 83 +++++++ .../Uns/EffectiveNameGuardTests.cs | 203 ++++++++++++++++++ 4 files changed, 396 insertions(+) create mode 100644 src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/EffectiveNameGuard.cs create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/EffectiveNameGuardTests.cs diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/EndpointRouteBuilderExtensions.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/EndpointRouteBuilderExtensions.cs index 7239ee9e..72a33228 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/EndpointRouteBuilderExtensions.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/EndpointRouteBuilderExtensions.cs @@ -49,6 +49,10 @@ public static class EndpointRouteBuilderExtensions services.AddScoped(); services.AddScoped(); services.AddScoped(); + // v3 Batch 3 (WP2): authoring-time UNS effective-name uniqueness guard. Consumed by + // UnsTreeService's reference/VirtualTag/ScriptedAlarm mutations (WP1); mirrors the + // deploy-time DraftValidator UnsEffectiveNameCollision rule. + services.AddScoped(); // WP5 CSV export enumeration (read-only; does not extend the closed IRawTreeService prelude). services.AddScoped(); services.AddSingleton(); diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/EffectiveNameGuard.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/EffectiveNameGuard.cs new file mode 100644 index 00000000..97c8d124 --- /dev/null +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/EffectiveNameGuard.cs @@ -0,0 +1,106 @@ +using Microsoft.EntityFrameworkCore; +using ZB.MOM.WW.OtOpcUa.Configuration; + +namespace ZB.MOM.WW.OtOpcUa.AdminUI.Uns; + +/// +/// Default . Loads the owning equipment's current effective-name +/// set — references (DisplayNameOverride else the backing raw tag's Name), +/// VirtualTags (Name), and ScriptedAlarms (Name) — and reports the first collision +/// with a proposed name. Comparison is to match the +/// {EquipmentId}/{EffectiveName} NodeId semantics and the deploy-time +/// DraftValidator UnsEffectiveNameCollision rule. +/// +/// +/// Each call creates and disposes its own pooled context — the same per-call pattern +/// uses — so the guard is safe to register Scoped. +/// +public sealed class EffectiveNameGuard(IDbContextFactory dbFactory) : IEffectiveNameGuard +{ + /// + public async Task CheckAsync( + string equipmentId, + string proposedEffectiveName, + EffectiveNameSourceKind kind, + string? excludeSourceId, + CancellationToken ct = default) + { + await using var db = await dbFactory.CreateDbContextAsync(ct); + + // References for this equipment. The effective name is the display-name override else the + // backing raw tag's current Name, so the two sets (references + tags) are loaded and joined + // in memory — the per-equipment set is small and this mirrors the deploy rule exactly + // (a reference whose backing tag is missing contributes no effective name, so it is skipped). + var references = await db.UnsTagReferences + .AsNoTracking() + .Where(r => r.EquipmentId == equipmentId) + .Select(r => new { r.UnsTagReferenceId, r.TagId, r.DisplayNameOverride }) + .ToListAsync(ct); + + var tagIds = references.Select(r => r.TagId).Distinct(StringComparer.Ordinal).ToList(); + var tagNameById = (await db.Tags + .AsNoTracking() + .Where(t => tagIds.Contains(t.TagId)) + .Select(t => new { t.TagId, t.Name }) + .ToListAsync(ct)) + .GroupBy(t => t.TagId, StringComparer.Ordinal) + .ToDictionary(g => g.Key, g => g.First().Name, StringComparer.Ordinal); + + foreach (var r in references) + { + if (IsSelf(EffectiveNameSourceKind.Reference, r.UnsTagReferenceId, kind, excludeSourceId)) + continue; + var effective = r.DisplayNameOverride + ?? (tagNameById.TryGetValue(r.TagId, out var n) ? n : null); + if (Collides(effective, proposedEffectiveName)) + return Message(proposedEffectiveName, "reference", r.UnsTagReferenceId, equipmentId); + } + + var virtualTags = await db.VirtualTags + .AsNoTracking() + .Where(v => v.EquipmentId == equipmentId) + .Select(v => new { v.VirtualTagId, v.Name }) + .ToListAsync(ct); + + foreach (var v in virtualTags) + { + if (IsSelf(EffectiveNameSourceKind.VirtualTag, v.VirtualTagId, kind, excludeSourceId)) + continue; + if (Collides(v.Name, proposedEffectiveName)) + return Message(proposedEffectiveName, "VirtualTag", v.VirtualTagId, equipmentId); + } + + var scriptedAlarms = await db.ScriptedAlarms + .AsNoTracking() + .Where(a => a.EquipmentId == equipmentId) + .Select(a => new { a.ScriptedAlarmId, a.Name }) + .ToListAsync(ct); + + foreach (var a in scriptedAlarms) + { + if (IsSelf(EffectiveNameSourceKind.ScriptedAlarm, a.ScriptedAlarmId, kind, excludeSourceId)) + continue; + if (Collides(a.Name, proposedEffectiveName)) + return Message(proposedEffectiveName, "ScriptedAlarm", a.ScriptedAlarmId, equipmentId); + } + + return null; + } + + /// True when a row is the self-row being edited — same kind AND same logical id — so it + /// is excluded from the collision set (a rename / override-change of a row cannot collide with itself). + private static bool IsSelf( + EffectiveNameSourceKind rowKind, + string rowId, + EffectiveNameSourceKind editedKind, + string? excludeSourceId) => + excludeSourceId is not null + && rowKind == editedKind + && string.Equals(rowId, excludeSourceId, StringComparison.Ordinal); + + private static bool Collides(string? existing, string proposed) => + existing is not null && string.Equals(existing, proposed, StringComparison.Ordinal); + + private static string Message(string name, string sourceLabel, string sourceId, string equipmentId) => + $"effective name '{name}' already used by {sourceLabel} '{sourceId}' in equipment '{equipmentId}'"; +} diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Configuration.Tests/DraftValidatorTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Configuration.Tests/DraftValidatorTests.cs index adcf5dd8..6c5ca786 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Configuration.Tests/DraftValidatorTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Configuration.Tests/DraftValidatorTests.cs @@ -205,6 +205,71 @@ public sealed class DraftValidatorTests DraftValidator.Validate(draft).ShouldNotContain(e => e.Code == "UnsEffectiveNameCollision"); } + /// The rename-induced case the authoring guard never sees: an authoring-clean draft where + /// a reference (no override, effective name = backing raw tag's current Name) ends up sharing an + /// effective name with a VirtualTag after a raw-tag rename. The deploy gate is the backstop. + [Fact] + public void Reference_effective_from_raw_name_collides_with_virtualtag_after_rename() + { + var draft = new DraftSnapshot + { + GenerationId = 1, ClusterId = "c", + Tags = [BuildTag(tagId: "tag-1", name: "speed")], // raw tag renamed to "speed" + UnsTagReferences = + [ + // No override → effective name is the backing raw tag's Name ("speed"). + BuildReference(refId: "ref-1", equipmentId: "eq-1", tagId: "tag-1", overrideName: null), + ], + VirtualTags = [BuildVirtualTag(equipmentId: "eq-1", name: "speed")], + }; + + var errors = DraftValidator.Validate(draft); + errors.ShouldContain(e => e.Code == "UnsEffectiveNameCollision"); + // Names both colliding sources + the equipment. + var msg = errors.First(e => e.Code == "UnsEffectiveNameCollision").Message; + msg.ShouldContain("ref-1"); + msg.ShouldContain("vtag-eq-1-speed"); + msg.ShouldContain("eq-1"); + } + + /// A reference's DisplayNameOverride (not its backing raw name) is the effective name and + /// must be unique against a VirtualTag in the same equipment. + [Fact] + public void Reference_override_collides_with_virtualtag() + { + var draft = new DraftSnapshot + { + GenerationId = 1, ClusterId = "c", + Tags = [BuildTag(tagId: "tag-1", name: "raw-name")], // backing raw name differs + UnsTagReferences = + [ + BuildReference(refId: "ref-1", equipmentId: "eq-1", tagId: "tag-1", overrideName: "computed"), + ], + VirtualTags = [BuildVirtualTag(equipmentId: "eq-1", name: "computed")], + }; + + DraftValidator.Validate(draft).ShouldContain(e => e.Code == "UnsEffectiveNameCollision"); + } + + /// Effective-name comparison is ordinal: "Speed" (VirtualTag) and "speed" (reference's + /// backing raw name) do NOT collide — they are distinct OPC UA browse names. + [Fact] + public void Effective_name_collision_is_ordinal_case_sensitive() + { + var draft = new DraftSnapshot + { + GenerationId = 1, ClusterId = "c", + Tags = [BuildTag(tagId: "tag-1", name: "speed")], + UnsTagReferences = + [ + BuildReference(refId: "ref-1", equipmentId: "eq-1", tagId: "tag-1", overrideName: null), + ], + VirtualTags = [BuildVirtualTag(equipmentId: "eq-1", name: "Speed")], + }; + + DraftValidator.Validate(draft).ShouldNotContain(e => e.Code == "UnsEffectiveNameCollision"); + } + private static VirtualTag BuildVirtualTag(string equipmentId, string name, string suffix = "") => new() { VirtualTagId = $"vtag-{equipmentId}-{name}{suffix}", @@ -214,6 +279,24 @@ public sealed class DraftValidatorTests ScriptId = "s-1", }; + private static Tag BuildTag(string tagId, string name) => new() + { + TagId = tagId, + DeviceId = "dev-1", + Name = name, + DataType = "Float", + AccessLevel = TagAccessLevel.Read, + TagConfig = "{}", + }; + + private static UnsTagReference BuildReference(string refId, string equipmentId, string tagId, string? overrideName) => new() + { + UnsTagReferenceId = refId, + EquipmentId = equipmentId, + TagId = tagId, + DisplayNameOverride = overrideName, + }; + // ------------------------------------------------------------------------------------ // Phase 6.3 task #148 part 2 — ValidateClusterTopology (unchanged in v3) // ------------------------------------------------------------------------------------ diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/EffectiveNameGuardTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/EffectiveNameGuardTests.cs new file mode 100644 index 00000000..fe1ec492 --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/EffectiveNameGuardTests.cs @@ -0,0 +1,203 @@ +using Microsoft.EntityFrameworkCore; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.AdminUI.Uns; +using ZB.MOM.WW.OtOpcUa.Configuration; +using ZB.MOM.WW.OtOpcUa.Configuration.Entities; +using ZB.MOM.WW.OtOpcUa.Configuration.Enums; + +namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests.Uns; + +/// +/// Unit tests for — the authoring-time UNS effective-name +/// uniqueness guard. Backed by an InMemory EF database seeded with one equipment carrying a +/// reference (override + backing raw tag), a VirtualTag, and a ScriptedAlarm. +/// +public sealed class EffectiveNameGuardTests +{ + private const string EquipmentId = "EQ-000000000001"; + + // Reference REF-1 → raw tag TAG-1 (Name "speed"), NO override → effective "speed". + // Reference REF-2 → raw tag TAG-2 (Name "raw-temp"), override "temperature" → effective "temperature". + // VirtualTag VT-1 (Name "computed"). ScriptedAlarm AL-1 (Name "highpress"). + private const string RefNoOverrideId = "REF-1"; + private const string RefWithOverrideId = "REF-2"; + private const string VirtualTagId = "VT-1"; + private const string ScriptedAlarmId = "AL-1"; + + private static IDbContextFactory SeedFactory() + { + var name = $"engname-{Guid.NewGuid():N}"; + var factory = new NamedFactory(name); + using var db = factory.CreateDbContext(); + + db.Tags.Add(new Tag + { + TagId = "TAG-1", + DeviceId = "DEV-1", + Name = "speed", + DataType = "Float", + AccessLevel = TagAccessLevel.Read, + TagConfig = "{}", + }); + db.Tags.Add(new Tag + { + TagId = "TAG-2", + DeviceId = "DEV-1", + Name = "raw-temp", + DataType = "Float", + AccessLevel = TagAccessLevel.Read, + TagConfig = "{}", + }); + + db.UnsTagReferences.Add(new UnsTagReference + { + UnsTagReferenceId = RefNoOverrideId, + EquipmentId = EquipmentId, + TagId = "TAG-1", + DisplayNameOverride = null, // effective = raw tag Name "speed" + }); + db.UnsTagReferences.Add(new UnsTagReference + { + UnsTagReferenceId = RefWithOverrideId, + EquipmentId = EquipmentId, + TagId = "TAG-2", + DisplayNameOverride = "temperature", // effective = "temperature" + }); + + db.VirtualTags.Add(new VirtualTag + { + VirtualTagId = VirtualTagId, + EquipmentId = EquipmentId, + Name = "computed", + DataType = "Float", + ScriptId = "SCR-1", + }); + + db.ScriptedAlarms.Add(new ScriptedAlarm + { + ScriptedAlarmId = ScriptedAlarmId, + EquipmentId = EquipmentId, + Name = "highpress", + AlarmType = "AlarmCondition", + MessageTemplate = "{TagPath} high", + PredicateScriptId = "SCR-2", + }); + + db.SaveChanges(); + return factory; + } + + private static EffectiveNameGuard Guard(IDbContextFactory factory) => new(factory); + + [Fact] + public async Task Free_name_returns_null() + { + var guard = Guard(SeedFactory()); + + var result = await guard.CheckAsync( + EquipmentId, "brand-new-name", EffectiveNameSourceKind.VirtualTag, excludeSourceId: null); + + result.ShouldBeNull(); + } + + [Fact] + public async Task Reference_vs_new_virtualtag_clash_returns_error_naming_the_reference() + { + var guard = Guard(SeedFactory()); + + // A new VirtualTag proposing "speed" collides with reference REF-1's effective name. + var result = await guard.CheckAsync( + EquipmentId, "speed", EffectiveNameSourceKind.VirtualTag, excludeSourceId: null); + + result.ShouldNotBeNull(); + result.ShouldBe($"effective name 'speed' already used by reference '{RefNoOverrideId}' in equipment '{EquipmentId}'"); + } + + [Fact] + public async Task Override_effective_name_vs_new_reference_clash_returns_error() + { + var guard = Guard(SeedFactory()); + + // A new reference proposing "temperature" collides with REF-2's OVERRIDE effective name. + var result = await guard.CheckAsync( + EquipmentId, "temperature", EffectiveNameSourceKind.Reference, excludeSourceId: null); + + result.ShouldNotBeNull(); + result.ShouldBe($"effective name 'temperature' already used by reference '{RefWithOverrideId}' in equipment '{EquipmentId}'"); + } + + [Fact] + public async Task New_virtualtag_vs_scripted_alarm_clash_returns_error_naming_the_alarm() + { + var guard = Guard(SeedFactory()); + + var result = await guard.CheckAsync( + EquipmentId, "highpress", EffectiveNameSourceKind.VirtualTag, excludeSourceId: null); + + result.ShouldNotBeNull(); + result.ShouldBe($"effective name 'highpress' already used by ScriptedAlarm '{ScriptedAlarmId}' in equipment '{EquipmentId}'"); + } + + [Fact] + public async Task Self_exclude_on_override_change_returns_null() + { + var guard = Guard(SeedFactory()); + + // Editing REF-2 to change its override to "temperature" (its own current effective name) + // must NOT collide with itself. + var result = await guard.CheckAsync( + EquipmentId, "temperature", EffectiveNameSourceKind.Reference, excludeSourceId: RefWithOverrideId); + + result.ShouldBeNull(); + } + + [Fact] + public async Task Self_exclude_only_applies_to_same_kind() + { + var guard = Guard(SeedFactory()); + + // Excluding a VirtualTag whose id happens to equal a reference id must NOT excuse the + // reference collision — the self-row is identified by (kind, id) together. + var result = await guard.CheckAsync( + EquipmentId, "speed", EffectiveNameSourceKind.VirtualTag, excludeSourceId: RefNoOverrideId); + + result.ShouldNotBeNull(); + result.ShouldBe($"effective name 'speed' already used by reference '{RefNoOverrideId}' in equipment '{EquipmentId}'"); + } + + [Fact] + public async Task Ordinal_case_sensitivity_Speed_does_not_collide_with_speed() + { + var guard = Guard(SeedFactory()); + + // "Speed" (capital S) must NOT collide with the existing "speed" (ordinal comparison). + var result = await guard.CheckAsync( + EquipmentId, "Speed", EffectiveNameSourceKind.VirtualTag, excludeSourceId: null); + + result.ShouldBeNull(); + } + + [Fact] + public async Task Collision_is_scoped_to_the_equipment() + { + var guard = Guard(SeedFactory()); + + // Same name, different equipment → no collision (per-equipment NodeId space). + var result = await guard.CheckAsync( + "EQ-000000000002", "speed", EffectiveNameSourceKind.VirtualTag, excludeSourceId: null); + + result.ShouldBeNull(); + } + + private sealed class NamedFactory(string name) : IDbContextFactory + { + public OtOpcUaConfigDbContext CreateDbContext() => + new(new DbContextOptionsBuilder() + .UseInMemoryDatabase(name) + .Options); + + public Task CreateDbContextAsync(CancellationToken cancellationToken = default) => + Task.FromResult(CreateDbContext()); + } +}