fix(uns): reject driver-bind on unresolvable line + enforce MachineCode uniqueness on update (review)

This commit is contained in:
Joseph Doherty
2026-06-08 12:55:36 -04:00
parent 2836a0704b
commit ab0ff8aedf
2 changed files with 114 additions and 4 deletions
@@ -403,6 +403,11 @@ public sealed class UnsTreeService(IDbContextFactory<OtOpcUaConfigDbContext> 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<OtOpcUaConfigDbContext> dbF
/// <summary>
/// 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 <c>null</c> when the bind is allowed (driver-less, unresolvable cluster, or match).
/// Policy:
/// <list type="bullet">
/// <item>Driver-less (<c>DriverInstanceId</c> empty) → always allowed (returns <c>null</c>).</item>
/// <item>Driver bound but the line/area does not resolve to a cluster → rejected (unresolvable
/// line cannot guarantee cluster alignment, so the bind is unsafe).</item>
/// <item>Driver bound, line resolves, clusters differ → rejected.</item>
/// <item>Driver bound, line resolves, clusters match → allowed (returns <c>null</c>).</item>
/// </list>
/// </summary>
private static async Task<UnsMutationResult?> CheckDriverClusterGuardAsync(
OtOpcUaConfigDbContext db,
@@ -489,12 +499,19 @@ public sealed class UnsTreeService(IDbContextFactory<OtOpcUaConfigDbContext> 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,
@@ -157,6 +157,41 @@ public sealed class UnsTreeServiceEquipmentTests
db.Equipment.Single(e => e.MachineCode == "machine_001").DriverInstanceId.ShouldBeNull();
}
/// <summary>
/// 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).
/// </summary>
[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 -----
/// <summary>Updating equipment changes its mutable fields (name, MachineCode, a 40010 field).</summary>
@@ -235,6 +270,64 @@ public sealed class UnsTreeServiceEquipmentTests
verify.Equipment.Single(e => e.EquipmentId == equipmentId).DriverInstanceId.ShouldBeNull();
}
/// <summary>Updating equipment with a MachineCode that already belongs to another row is blocked.</summary>
[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");
}
/// <summary>Updating equipment to bind a driver that is in the SAME cluster as the line is allowed.</summary>
[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 -----
/// <summary>Deleting equipment removes the row.</summary>