From ab0ff8aedf495e2967ac384635057e83115d08b3 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 8 Jun 2026 12:55:36 -0400 Subject: [PATCH] fix(uns): reject driver-bind on unresolvable line + enforce MachineCode uniqueness on update (review) --- .../Uns/UnsTreeService.cs | 25 ++++- .../Uns/UnsTreeServiceEquipmentTests.cs | 93 +++++++++++++++++++ 2 files changed, 114 insertions(+), 4 deletions(-) diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/UnsTreeService.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/UnsTreeService.cs index 54406e2c..85c1b1a6 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/UnsTreeService.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/UnsTreeService.cs @@ -403,6 +403,11 @@ public sealed class UnsTreeService(IDbContextFactory dbF db.Entry(entity).Property(e => e.RowVersion).OriginalValue = rowVersion; + if (await db.Equipment.AnyAsync(e => e.MachineCode == input.MachineCode && e.EquipmentId != equipmentId, ct)) + { + return new UnsMutationResult(false, $"MachineCode '{input.MachineCode}' already exists in this fleet."); + } + var guard = await CheckDriverClusterGuardAsync(db, input, ct); if (guard is not null) { @@ -471,9 +476,14 @@ public sealed class UnsTreeService(IDbContextFactory dbF /// /// Decision #122: an equipment may only bind to a driver in the same cluster as its line. - /// Only runs when a driver is requested. Resolves the line's cluster (line → area → cluster) - /// and the driver's cluster; when both are known and differ, returns the guard failure. - /// Returns null when the bind is allowed (driver-less, unresolvable cluster, or match). + /// Policy: + /// + /// Driver-less (DriverInstanceId empty) → always allowed (returns null). + /// Driver bound but the line/area does not resolve to a cluster → rejected (unresolvable + /// line cannot guarantee cluster alignment, so the bind is unsafe). + /// Driver bound, line resolves, clusters differ → rejected. + /// Driver bound, line resolves, clusters match → allowed (returns null). + /// /// private static async Task CheckDriverClusterGuardAsync( OtOpcUaConfigDbContext db, @@ -489,12 +499,19 @@ public sealed class UnsTreeService(IDbContextFactory dbF var area = line is null ? null : await db.UnsAreas.FirstOrDefaultAsync(a => a.UnsAreaId == line.UnsAreaId, ct); var lineCluster = area?.ClusterId; + if (lineCluster is null) + { + return new UnsMutationResult( + false, + $"Cannot bind driver '{input.DriverInstanceId}': UNS line '{input.UnsLineId}' does not resolve to a cluster (decision #122)."); + } + var driverCluster = await db.DriverInstances .Where(d => d.DriverInstanceId == input.DriverInstanceId) .Select(d => d.ClusterId) .FirstOrDefaultAsync(ct); - if (driverCluster is not null && lineCluster is not null && driverCluster != lineCluster) + if (driverCluster is not null && driverCluster != lineCluster) { return new UnsMutationResult( false, diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/UnsTreeServiceEquipmentTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/UnsTreeServiceEquipmentTests.cs index bb7ee02f..9e2555d6 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/UnsTreeServiceEquipmentTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/UnsTreeServiceEquipmentTests.cs @@ -157,6 +157,41 @@ public sealed class UnsTreeServiceEquipmentTests db.Equipment.Single(e => e.MachineCode == "machine_001").DriverInstanceId.ShouldBeNull(); } + /// + /// The #122 guard blocks binding equipment to a driver when the UNS line does not resolve to + /// a cluster (e.g. the line does not exist in the DB). + /// + [Fact] + public async Task CreateEquipment_driver_bound_unresolvable_line_blocked() + { + var (service, dbName) = Fresh(); + // Seed a driver in MAIN cluster, but do NOT create the UnsLine that the input references. + using (var db = UnsTreeTestDb.CreateNamed(dbName)) + { + db.DriverInstances.Add(new DriverInstance + { + DriverInstanceId = "DRV-1", + ClusterId = "MAIN", + NamespaceId = "NS-1", + Name = "drv", + DriverType = "ModbusTcp", + DriverConfig = "{}", + }); + db.SaveChanges(); + } + + var result = await service.CreateEquipmentAsync( + Input("machine-1", "machine_001", "LINE-BOGUS", "DRV-1")); + + result.Ok.ShouldBeFalse(); + result.Error.ShouldNotBeNull(); + result.Error.ShouldContain("decision #122"); + result.Error.ShouldContain("LINE-BOGUS"); + + using var verify = UnsTreeTestDb.CreateNamed(dbName); + verify.Equipment.Any(e => e.MachineCode == "machine_001").ShouldBeFalse(); + } + // ----- UpdateEquipment ----- /// Updating equipment changes its mutable fields (name, MachineCode, a 40010 field). @@ -235,6 +270,64 @@ public sealed class UnsTreeServiceEquipmentTests verify.Equipment.Single(e => e.EquipmentId == equipmentId).DriverInstanceId.ShouldBeNull(); } + /// Updating equipment with a MachineCode that already belongs to another row is blocked. + [Fact] + public async Task UpdateEquipment_duplicate_machinecode_blocked() + { + var (service, dbName) = Fresh(); + SeedLineAndDriver(dbName, lineCluster: "MAIN", driverCluster: null); + await service.CreateEquipmentAsync(Input("machine-a", "mc_a", "LINE-1", null)); + await service.CreateEquipmentAsync(Input("machine-b", "mc_b", "LINE-1", null)); + + string equipmentId; + byte[] rv; + using (var db = UnsTreeTestDb.CreateNamed(dbName)) + { + var eq = db.Equipment.Single(e => e.MachineCode == "mc_a"); + equipmentId = eq.EquipmentId; + rv = eq.RowVersion; + } + + // Try to rename mc_a → mc_b (which already exists). + var result = await service.UpdateEquipmentAsync( + equipmentId, Input("machine-a", "mc_b", "LINE-1", null), rv); + + result.Ok.ShouldBeFalse(); + result.Error.ShouldNotBeNull(); + result.Error.ShouldBe("MachineCode 'mc_b' already exists in this fleet."); + + // The original row must be unchanged. + using var verify = UnsTreeTestDb.CreateNamed(dbName); + verify.Equipment.Single(e => e.EquipmentId == equipmentId).MachineCode.ShouldBe("mc_a"); + } + + /// Updating equipment to bind a driver that is in the SAME cluster as the line is allowed. + [Fact] + public async Task UpdateEquipment_driver_in_same_cluster_allowed() + { + var (service, dbName) = Fresh(); + SeedLineAndDriver(dbName, lineCluster: "MAIN", driverCluster: "MAIN"); + await service.CreateEquipmentAsync(Input("machine-1", "machine_001", "LINE-1", null)); + + string equipmentId; + byte[] rv; + using (var db = UnsTreeTestDb.CreateNamed(dbName)) + { + var eq = db.Equipment.Single(e => e.MachineCode == "machine_001"); + equipmentId = eq.EquipmentId; + rv = eq.RowVersion; + } + + var result = await service.UpdateEquipmentAsync( + equipmentId, Input("machine-1", "machine_001", "LINE-1", "DRV-1"), rv); + + result.Ok.ShouldBeTrue(); + result.Error.ShouldBeNull(); + + using var verify = UnsTreeTestDb.CreateNamed(dbName); + verify.Equipment.Single(e => e.EquipmentId == equipmentId).DriverInstanceId.ShouldBe("DRV-1"); + } + // ----- DeleteEquipment ----- /// Deleting equipment removes the row.