From 1a1dbc3f00e906cf5e9fc6f75a84509729af8342 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Sat, 1 Aug 2026 11:53:58 -0400 Subject: [PATCH] fix(uns): enforce the UNS segment rule on every authoring path (#535) An area or line could be named "Packaging" or "Line 1", save cleanly, and then fail the NEXT deployment with UnsSegmentInvalid. Because a deployment snapshots the whole database, one badly-named area blocked EVERY cluster's deploy - and whoever hit that was usually not whoever typed the name. Only the equipment form carried the rule; the deploy gate held its own private copy. The rule now lives once, on Configuration.Validation.UnsSegment, and both ends call it. DraftValidator no longer restates it. Enforced in UnsTreeService, not just on the forms: - Create/Update Area, Line and Equipment reject an invalid name before touching the row. - The CSV import reports a bad name as an ERROR, not a skip - a skip means "you already have this row", which would tell the operator the equipment exists when it does not. Good rows in the same batch still insert. A form attribute could never have covered the import, and with no bUnit in this repo a rule only a form enforces is also a rule no test can reach. The forms keep an attribute for the immediate message, but it now references the shared pattern rather than restating it - a second copy is how these drifted apart. One behaviour change beyond the fix: UnsSegment.AttributePattern admits "_default", which the equipment form's hand-written literal rejected. The form was stricter than the gate it was mirroring, so a _default row was uneditable through the AdminUI. Tests: 41, falsified with three production mutations - dropping the CreateArea guard reds exactly the CreateArea rejections, turning the import error back into a skip reds only the import test, and narrowing AttributePattern reds only the attribute/IsValid agreement. Every rejection is paired with a name that must still be accepted, or a guard that refused everything would pass. Live-verified on docker-dev: the area modal now refuses "Packaging" in place with the rule spelled out, "packaging-b" saves, and a CSV import of two rows reports Inserted 1 / Skipped 0 / Errors 1 naming the offending row - the path no form attribute reaches. Rig cleaned up and a further deployment sealed. Claude-Session: https://claude.ai/code/session_015p7wGqy3YpZNCpDzTpGMKo --- docs/Uns.md | 27 ++ .../Validation/DraftValidator.cs | 25 +- .../Validation/UnsSegment.cs | 73 +++++ .../Components/Pages/Uns/EquipmentPage.razor | 6 +- .../Components/Shared/Uns/AreaModal.razor | 5 +- .../Components/Shared/Uns/LineModal.razor | 5 +- .../Uns/UnsTreeService.cs | 40 +++ .../Uns/UnsSegmentGuardTests.cs | 257 ++++++++++++++++++ 8 files changed, 418 insertions(+), 20 deletions(-) create mode 100644 src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/UnsSegment.cs create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/UnsSegmentGuardTests.cs diff --git a/docs/Uns.md b/docs/Uns.md index 5579002e..43689d43 100644 --- a/docs/Uns.md +++ b/docs/Uns.md @@ -51,6 +51,33 @@ area opens the Line modal with that area already selected). Build a branch top-down: Area → Line → Equipment. Tags, virtual tags, and scripted alarms are managed on the equipment page (see below). +### Naming: area, line and equipment names are UNS path segments + +An area, line or equipment **name** becomes a level of the OPC UA NodeId +(`Area/Line/Equipment/EffectiveName`), so it must match +**`^[a-z0-9-]{1,32}$`** — lowercase letters, digits and dashes, 1–32 +characters. The reserved `_default` is also accepted. `Packaging` and +`Line 1` are not valid; `packaging` and `line-1` are. + +**Ids are a different thing and a different rule.** `UnsAreaId` / +`UnsLineId` are opaque handles that never reach a NodeId, so they take +`^[A-Za-z0-9_-]+$` — `SITEA-area1` is a fine id for an area named +`packaging`. + +The rule is enforced in `UnsTreeService` on **every** authoring path — +create, rename, and the CSV import — and again at the deploy gate +(`UnsSegmentInvalid`). Both call the same `UnsSegment` helper, so they +cannot disagree. + +⚠️ **Why it is enforced at authoring and not only at deploy (Gitea #535):** +until 2026-08-01 only the equipment form checked it, so an area or line +could be named `Packaging`, save cleanly, and fail the *next deployment* — +and because a deployment snapshots the **whole database**, one badly-named +area blocked **every** cluster's deploy, usually discovered by someone other +than whoever typed it. In the CSV import a bad name is reported as an +**error**, not a skip: a skip means "you already have this row", which would +say the equipment exists when it does not. + ## Equipment page (`/uns/equipment/{id}`) Clicking **Open** on an Equipment row (or **+ Equipment** on a Line) navigates diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/DraftValidator.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/DraftValidator.cs index 20e6b671..3543bce2 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/DraftValidator.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/DraftValidator.cs @@ -16,8 +16,6 @@ namespace ZB.MOM.WW.OtOpcUa.Configuration.Validation; /// public static class DraftValidator { - private static readonly Regex UnsSegment = new(@"^[a-z0-9-]{1,32}$", RegexOptions.Compiled); - private const string UnsDefaultSegment = "_default"; private const int MaxPathLength = 200; /// @@ -598,28 +596,21 @@ public static class DraftValidator } } - private static bool IsValidSegment(string? s) => - s is not null && (UnsSegment.IsMatch(s) || s == UnsDefaultSegment); - + // The rule itself lives on UnsSegment, which the AdminUI's authoring guards also call — a second + // copy here is exactly how the two drifted apart (Gitea #535). Only the per-level wording is local. private static void ValidateUnsSegments(DraftSnapshot draft, List errors) { foreach (var a in draft.UnsAreas) - if (!IsValidSegment(a.Name)) - errors.Add(new("UnsSegmentInvalid", - $"UnsArea.Name '{a.Name}' does not match [a-z0-9-]{{1,32}} or '_default'", - a.UnsAreaId)); + if (!UnsSegment.IsValid(a.Name)) + errors.Add(new("UnsSegmentInvalid", UnsSegment.Describe("UnsArea.Name", a.Name), a.UnsAreaId)); foreach (var l in draft.UnsLines) - if (!IsValidSegment(l.Name)) - errors.Add(new("UnsSegmentInvalid", - $"UnsLine.Name '{l.Name}' does not match [a-z0-9-]{{1,32}} or '_default'", - l.UnsLineId)); + if (!UnsSegment.IsValid(l.Name)) + errors.Add(new("UnsSegmentInvalid", UnsSegment.Describe("UnsLine.Name", l.Name), l.UnsLineId)); foreach (var e in draft.Equipment) - if (!IsValidSegment(e.Name)) - errors.Add(new("UnsSegmentInvalid", - $"Equipment.Name '{e.Name}' does not match [a-z0-9-]{{1,32}} or '_default'", - e.EquipmentId)); + if (!UnsSegment.IsValid(e.Name)) + errors.Add(new("UnsSegmentInvalid", UnsSegment.Describe("Equipment.Name", e.Name), e.EquipmentId)); } /// Cluster.Enterprise + Site + area + line + equipment + 4 slashes ≤ 200 chars. diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/UnsSegment.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/UnsSegment.cs new file mode 100644 index 00000000..bb21a934 --- /dev/null +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/UnsSegment.cs @@ -0,0 +1,73 @@ +using System.Text.RegularExpressions; + +namespace ZB.MOM.WW.OtOpcUa.Configuration.Validation; + +/// +/// The one authority on what a UNS path segment — a UnsArea.Name, UnsLine.Name or +/// Equipment.Name — may be. +/// +/// +/// +/// A segment becomes a level of the UNS NodeId (Area/Line/Equipment/EffectiveName), so the +/// rule is not cosmetic: it is what keeps a NodeId parseable and stable. +/// +/// +/// ⚠️ This exists because the rule was duplicated and drifted (Gitea #535). The deploy gate +/// held it privately in DraftValidator and the AdminUI restated it on the equipment form +/// alone, so an area or line could be named Packaging or Line 1, saved, and then +/// fail the deploy with UnsSegmentInvalid. Because a deployment snapshots the whole +/// database, one badly-named area blocked every cluster's deploy. Anything that validates +/// a UNS name must reference this type rather than restate the pattern — a second copy is how it +/// drifted the first time. +/// +/// +public static class UnsSegment +{ + /// + /// The reserved segment for an unnamed level. Accepted alongside , so a + /// validator that applies the pattern alone is stricter than the deploy gate and would + /// make such a row uneditable. + /// + public const string DefaultSegment = "_default"; + + /// + /// The segment pattern, as a constant so it can be used in a + /// [RegularExpression] attribute (which needs a compile-time constant). + /// + /// + /// ⚠️ This does not admit — an attribute cannot express the + /// alternation and stay readable. is the one to put on a form; + /// is the one to call in code. + /// + public const string Pattern = "^[a-z0-9-]{1,32}$"; + + /// + /// widened to admit — the form of the rule to + /// put in a [RegularExpression] attribute, so the UI accepts exactly what the deploy gate + /// accepts and no less. + /// + public const string AttributePattern = "^([a-z0-9-]{1,32}|_default)$"; + + /// The message shown for a rejected segment, in the UI and from the service guards. + public const string Message = + "Lowercase letters, digits and dashes only; 1-32 characters. This becomes a level of the OPC UA " + + "NodeId, so a space or a capital would make the deployment fail."; + + private static readonly Regex Rx = new(Pattern, RegexOptions.Compiled); + + /// Whether a name is a usable UNS path segment. + /// The candidate name. + /// when it may be deployed. + public static bool IsValid(string? segment) => + segment is not null && (Rx.IsMatch(segment) || segment == DefaultSegment); + + /// + /// The error for a rejected segment, naming the level and the offending value — the shape the + /// deploy gate reports, so an operator sees the same sentence wherever it is caught. + /// + /// The entity and property, e.g. UnsArea.Name. + /// The rejected value. + /// The message. + public static string Describe(string level, string? segment) => + $"{level} '{segment}' does not match [a-z0-9-]{{1,32}} or '{DefaultSegment}'"; +} diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Uns/EquipmentPage.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Uns/EquipmentPage.razor index 34f99e9d..94452818 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Uns/EquipmentPage.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Uns/EquipmentPage.razor @@ -8,6 +8,7 @@ @rendermode RenderMode.InteractiveServer @using System.ComponentModel.DataAnnotations @using Microsoft.AspNetCore.Components.Forms +@using ZB.MOM.WW.OtOpcUa.Configuration.Validation @using ZB.MOM.WW.OtOpcUa.AdminUI.Uns @using ZB.MOM.WW.OtOpcUa.AdminUI.Components.Shared.Uns @inject IUnsTreeService Svc @@ -567,7 +568,10 @@ else /// The working form for the Details tab — identical to EquipmentModal.FormModel. private sealed class FormModel { - [Required, RegularExpression("^[a-z0-9-]{1,32}$", ErrorMessage = "Lowercase letters, digits, dashes only; max 32 chars.")] + // The shared rule, not a local restatement — a second copy is how the UI and the deploy gate + // drifted apart in the first place (Gitea #535). AttributePattern also admits "_default", which + // the previous literal rejected, making the form stricter than the gate it was mirroring. + [Required, RegularExpression(UnsSegment.AttributePattern, ErrorMessage = UnsSegment.Message)] public string Name { get; set; } = ""; [Required] public string MachineCode { get; set; } = ""; [Required] public string UnsLineId { get; set; } = ""; diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Uns/AreaModal.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Uns/AreaModal.razor index 1f24a6de..7a1c35ee 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Uns/AreaModal.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Uns/AreaModal.razor @@ -3,6 +3,7 @@ served-by cluster list. On a successful save it raises OnSaved so the host can reload the tree. *@ @using System.ComponentModel.DataAnnotations @using Microsoft.AspNetCore.Components.Forms +@using ZB.MOM.WW.OtOpcUa.Configuration.Validation @using ZB.MOM.WW.OtOpcUa.AdminUI.Uns @inject IUnsTreeService Svc @@ -29,6 +30,7 @@ +
UNS path segment: lowercase letters, digits and dashes, 1-32 characters. It becomes a level of the OPC UA NodeId.
@@ -138,7 +140,8 @@ private sealed class FormModel { [Required, RegularExpression("^[A-Za-z0-9_-]+$")] public string UnsAreaId { get; set; } = ""; - [Required] public string Name { get; set; } = ""; + [Required, RegularExpression(UnsSegment.AttributePattern, ErrorMessage = UnsSegment.Message)] + public string Name { get; set; } = ""; [Required] public string ClusterId { get; set; } = ""; public string? Notes { get; set; } } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Uns/LineModal.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Uns/LineModal.razor index 4fbf2265..ed5a4c2b 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Uns/LineModal.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Uns/LineModal.razor @@ -4,6 +4,7 @@ On a successful save it raises OnSaved so the host can reload the tree. *@ @using System.ComponentModel.DataAnnotations @using Microsoft.AspNetCore.Components.Forms +@using ZB.MOM.WW.OtOpcUa.Configuration.Validation @using ZB.MOM.WW.OtOpcUa.AdminUI.Uns @inject IUnsTreeService Svc @@ -40,6 +41,7 @@ +
UNS path segment: lowercase letters, digits and dashes, 1-32 characters. It becomes a level of the OPC UA NodeId.
@@ -140,7 +142,8 @@ { [Required, RegularExpression("^[A-Za-z0-9_-]+$")] public string UnsLineId { get; set; } = ""; [Required] public string UnsAreaId { get; set; } = ""; - [Required] public string Name { get; set; } = ""; + [Required, RegularExpression(UnsSegment.AttributePattern, ErrorMessage = UnsSegment.Message)] + public string Name { get; set; } = ""; public string? Notes { get; set; } } } 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 d0ed2e32..03509caa 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/UnsTreeService.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/UnsTreeService.cs @@ -5,6 +5,7 @@ using ZB.MOM.WW.OtOpcUa.Commons.Types; using ZB.MOM.WW.OtOpcUa.Configuration; using ZB.MOM.WW.OtOpcUa.Configuration.Entities; using ZB.MOM.WW.OtOpcUa.Configuration.Enums; +using ZB.MOM.WW.OtOpcUa.Configuration.Validation; namespace ZB.MOM.WW.OtOpcUa.AdminUI.Uns; @@ -625,6 +626,8 @@ public sealed class UnsTreeService( string? notes, CancellationToken ct = default) { + if (!UnsSegment.IsValid(name)) return SegmentRejected("Area name", name); + await using var db = await dbFactory.CreateDbContextAsync(ct); if (await db.UnsAreas.AnyAsync(a => a.UnsAreaId == unsAreaId, ct)) @@ -652,6 +655,8 @@ public sealed class UnsTreeService( byte[] rowVersion, CancellationToken ct = default) { + if (!UnsSegment.IsValid(name)) return SegmentRejected("Area name", name); + await using var db = await dbFactory.CreateDbContextAsync(ct); var entity = await db.UnsAreas.FirstOrDefaultAsync(a => a.UnsAreaId == unsAreaId, ct); @@ -719,6 +724,8 @@ public sealed class UnsTreeService( string? notes, CancellationToken ct = default) { + if (!UnsSegment.IsValid(name)) return SegmentRejected("Line name", name); + await using var db = await dbFactory.CreateDbContextAsync(ct); if (await db.UnsLines.AnyAsync(l => l.UnsLineId == unsLineId, ct)) @@ -746,6 +753,8 @@ public sealed class UnsTreeService( byte[] rowVersion, CancellationToken ct = default) { + if (!UnsSegment.IsValid(name)) return SegmentRejected("Line name", name); + await using var db = await dbFactory.CreateDbContextAsync(ct); var entity = await db.UnsLines.FirstOrDefaultAsync(l => l.UnsLineId == unsLineId, ct); @@ -808,6 +817,8 @@ public sealed class UnsTreeService( /// public async Task CreateEquipmentAsync(EquipmentInput input, CancellationToken ct = default) { + if (!UnsSegment.IsValid(input.Name)) return SegmentRejected("Equipment name", input.Name); + if (string.IsNullOrEmpty(input.UnsLineId)) { return new UnsMutationResult(false, "Pick a UNS line."); @@ -885,6 +896,15 @@ public sealed class UnsTreeService( continue; } + // An error rather than a skip: a skipped row is one the operator already has, while this is + // one they meant to add and cannot. Left unchecked it would import and then block the next + // deployment — fleet-wide, since a deployment snapshots the whole database (Gitea #535). + if (!UnsSegment.IsValid(row.Name)) + { + errors.Add($"Row '{row.MachineCode}': {UnsSegment.Describe("equipment name", row.Name)}."); + continue; + } + // v3: equipment no longer binds a driver, so there is no per-row driver-cluster guard. var uuid = Guid.NewGuid(); @@ -927,6 +947,8 @@ public sealed class UnsTreeService( byte[] rowVersion, CancellationToken ct = default) { + if (!UnsSegment.IsValid(input.Name)) return SegmentRejected("Equipment name", input.Name); + await using var db = await dbFactory.CreateDbContextAsync(ct); var entity = await db.Equipment.FirstOrDefaultAsync(e => e.EquipmentId == equipmentId, ct); @@ -1741,4 +1763,22 @@ public sealed class UnsTreeService( return new UnsMutationResult(false, $"Delete failed: {ex.Message}."); } } + + /// + /// The rejection for a UNS name that is not a usable path segment. + /// + /// + /// ⚠️ The service is where this is enforced, not the form (Gitea #535). Before this, the + /// rule lived on the equipment form alone, so an area or line could be named "Packaging" or + /// "Line 1", save cleanly, and then fail the deploy — and because a deployment snapshots the + /// whole database, one bad name blocked every cluster's deploy, discovered by someone + /// other than whoever typed it. A form attribute cannot cover the CSV import or any future + /// caller, and this repo has no bUnit, so a rule that only a page enforces is also a rule no + /// test can reach. + /// + /// The field being rejected, e.g. Area name. + /// The rejected value. + /// A failed mutation result carrying the reason. + private static UnsMutationResult SegmentRejected(string level, string? value) => + new(false, $"{UnsSegment.Describe(level, value)}. {UnsSegment.Message}"); } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/UnsSegmentGuardTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/UnsSegmentGuardTests.cs new file mode 100644 index 00000000..c0a421fb --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/UnsSegmentGuardTests.cs @@ -0,0 +1,257 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.AdminUI.Uns; +using ZB.MOM.WW.OtOpcUa.Configuration.Entities; +using ZB.MOM.WW.OtOpcUa.Configuration.Validation; + +namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests.Uns; + +/// +/// Gitea #535 — every UNS authoring path applies the segment rule the deploy gate applies. +/// +/// +/// +/// The bug was not that a bad name crashed anything: it saved perfectly, and the next +/// deployment failed with UnsSegmentInvalid. Because a deployment snapshots the whole +/// database, one badly-named area blocked every cluster's deploy, discovered by someone other +/// than whoever typed it. +/// +/// +/// So these are rejection tests, and each is paired with a name that must still be accepted +/// — a guard that refused everything would satisfy the rejection half alone. They live at the +/// service, not the form, because a form attribute cannot cover the CSV import and this repo has +/// no bUnit to reach a form with. +/// +/// +[Trait("Category", "Unit")] +public sealed class UnsSegmentGuardTests +{ + private static (UnsTreeService Service, string DbName) Fresh() + { + var dbName = $"uns-segment-{Guid.NewGuid():N}"; + return (new UnsTreeService(UnsTreeTestDb.Factory(dbName)), dbName); + } + + private static void SeedAreaAndLine(string dbName) + { + using var db = UnsTreeTestDb.CreateNamed(dbName); + db.UnsAreas.Add(new UnsArea { UnsAreaId = "AREA-1", ClusterId = "MAIN", Name = "a" }); + db.UnsLines.Add(new UnsLine { UnsLineId = "LINE-1", UnsAreaId = "AREA-1", Name = "l" }); + db.SaveChanges(); + } + + private static EquipmentInput Input(string name, string machineCode) => + new(name, machineCode, "LINE-1", + ZTag: null, SAPID: null, Manufacturer: null, Model: null, SerialNumber: null, + HardwareRevision: null, SoftwareRevision: null, YearOfConstruction: null, + AssetLocation: null, ManufacturerUri: null, DeviceManualUri: null, Enabled: true); + + /// The two names that actually caused the outage, plus the neighbouring shapes. + public static TheoryData BadNames() => new() + { + "Packaging", // the reported case: a capital + "Line 1", // the reported case: a space + "line_1", // underscore is not in the segment alphabet + "line/1", // a slash would split into two NodeId levels + "", // empty + " ", // whitespace + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", // 33 chars + }; + + /// Names that must keep working — the control for every rejection below. + public static TheoryData GoodNames() => new() + { + "packaging", + "line-1", + "a", + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", // exactly 32 + UnsSegment.DefaultSegment, // the gate accepts it, so the UI must too + }; + + // --- Area --------------------------------------------------------------------------------- + + [Theory] + [MemberData(nameof(BadNames))] + public async Task CreateArea_rejects_a_name_the_deploy_gate_would_reject(string name) + { + var (service, _) = Fresh(); + + var result = await service.CreateAreaAsync("MAIN", "AREA-NEW", name, null); + + result.Ok.ShouldBeFalse(); + result.Error.ShouldNotBeNull(); + } + + [Theory] + [MemberData(nameof(GoodNames))] + public async Task CreateArea_accepts_a_valid_segment(string name) + { + var (service, _) = Fresh(); + + (await service.CreateAreaAsync("MAIN", "AREA-NEW", name, null)).Ok.ShouldBeTrue(); + } + + /// + /// Renaming is the other way in, and the one an operator reaches after the name is already + /// deployed — so it is guarded too, and the guard runs before the row is touched. + /// + [Fact] + public async Task UpdateArea_rejects_a_rename_to_an_invalid_segment_and_leaves_the_row_alone() + { + var (service, dbName) = Fresh(); + await service.CreateAreaAsync("MAIN", "AREA-1", "packaging", null); + var rowVersion = Load(dbName).UnsAreas.Single(a => a.UnsAreaId == "AREA-1").RowVersion; + + (await service.UpdateAreaAsync("AREA-1", "Packaging", null, "MAIN", rowVersion)).Ok.ShouldBeFalse(); + Load(dbName).UnsAreas.Single(a => a.UnsAreaId == "AREA-1").Name.ShouldBe("packaging"); + + (await service.UpdateAreaAsync("AREA-1", "assembly", null, "MAIN", rowVersion)).Ok.ShouldBeTrue(); + Load(dbName).UnsAreas.Single(a => a.UnsAreaId == "AREA-1").Name.ShouldBe("assembly"); + } + + // --- Line --------------------------------------------------------------------------------- + + [Theory] + [MemberData(nameof(BadNames))] + public async Task CreateLine_rejects_a_name_the_deploy_gate_would_reject(string name) + { + var (service, dbName) = Fresh(); + SeedAreaAndLine(dbName); + + var result = await service.CreateLineAsync("AREA-1", "LINE-NEW", name, null); + + result.Ok.ShouldBeFalse(); + result.Error.ShouldNotBeNull(); + } + + [Fact] + public async Task CreateLine_accepts_a_valid_segment() + { + var (service, dbName) = Fresh(); + SeedAreaAndLine(dbName); + + (await service.CreateLineAsync("AREA-1", "LINE-NEW", "line-2", null)).Ok.ShouldBeTrue(); + } + + [Fact] + public async Task UpdateLine_rejects_a_rename_to_an_invalid_segment() + { + var (service, dbName) = Fresh(); + SeedAreaAndLine(dbName); + var rowVersion = Load(dbName).UnsLines.Single(l => l.UnsLineId == "LINE-1").RowVersion; + + (await service.UpdateLineAsync("LINE-1", "Line 1", null, "AREA-1", rowVersion)).Ok.ShouldBeFalse(); + (await service.UpdateLineAsync("LINE-1", "line-1", null, "AREA-1", rowVersion)).Ok.ShouldBeTrue(); + } + + // --- Equipment ---------------------------------------------------------------------------- + + [Theory] + [MemberData(nameof(BadNames))] + public async Task CreateEquipment_rejects_a_name_the_deploy_gate_would_reject(string name) + { + var (service, dbName) = Fresh(); + SeedAreaAndLine(dbName); + + (await service.CreateEquipmentAsync(Input(name, "mc_1"))).Ok.ShouldBeFalse(); + } + + [Fact] + public async Task CreateEquipment_accepts_a_valid_segment() + { + var (service, dbName) = Fresh(); + SeedAreaAndLine(dbName); + + (await service.CreateEquipmentAsync(Input("packer-1", "mc_1"))).Ok.ShouldBeTrue(); + } + + [Fact] + public async Task UpdateEquipment_rejects_a_rename_to_an_invalid_segment() + { + var (service, dbName) = Fresh(); + SeedAreaAndLine(dbName); + var created = await service.CreateEquipmentAsync(Input("packer-1", "mc_1")); + created.Ok.ShouldBeTrue(); + + var row = Load(dbName).Equipment.Single(); + + (await service.UpdateEquipmentAsync(row.EquipmentId, Input("Packer 1", "mc_1"), row.RowVersion)) + .Ok.ShouldBeFalse(); + (await service.UpdateEquipmentAsync(row.EquipmentId, Input("packer-2", "mc_1"), row.RowVersion)) + .Ok.ShouldBeTrue(); + } + + // --- CSV import --------------------------------------------------------------------------- + + /// + /// The bulk path is the one a form attribute could never have covered, and the one where a bad + /// row does the most damage — an import of hundreds lands, and the fleet stops deploying. + /// + /// + /// A bad name is an error, not a skip: a skip means "you already have this", which would + /// tell the operator the row is present when it is not. + /// + [Fact] + public async Task Import_reports_an_invalid_name_as_an_error_and_still_inserts_the_good_rows() + { + var (service, dbName) = Fresh(); + SeedAreaAndLine(dbName); + + var result = await service.ImportEquipmentAsync( + [ + Input("packer-1", "mc_1"), + Input("Packer 2", "mc_2"), + Input("packer-3", "mc_3"), + ]); + + result.Inserted.ShouldBe(2); + result.Skipped.ShouldBe(0, "a bad name is not a row the operator already has"); + result.Errors.ShouldHaveSingleItem().ShouldContain("mc_2"); + + Load(dbName).Equipment.Select(e => e.Name).OrderBy(n => n) + .ShouldBe(["packer-1", "packer-3"]); + } + + private static Configuration.OtOpcUaConfigDbContext Load(string dbName) => + UnsTreeTestDb.CreateNamed(dbName); +} + +/// The shared rule itself — one authority, so the gate and the AdminUI cannot drift (#535). +[Trait("Category", "Unit")] +public sealed class UnsSegmentTests +{ + /// + /// The attribute pattern the forms use must accept exactly what + /// accepts. They are two expressions of one rule, and a form stricter than the service is a field + /// an operator cannot satisfy; a form looser than it is the #535 bug again. + /// + [Theory] + [InlineData("packaging")] + [InlineData("line-1")] + [InlineData("_default")] + [InlineData("Packaging")] + [InlineData("Line 1")] + [InlineData("line_1")] + [InlineData("")] + [InlineData("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")] + public void The_attribute_pattern_agrees_with_IsValid(string candidate) + { + var byAttribute = System.Text.RegularExpressions.Regex.IsMatch( + candidate, UnsSegment.AttributePattern); + + byAttribute.ShouldBe(UnsSegment.IsValid(candidate)); + } + + /// + /// alone does NOT admit _default. Pinned because using it + /// on a form — the obvious mistake, and what the equipment form did — makes the UI stricter than + /// the deploy gate, so a _default row becomes uneditable through the AdminUI. + /// + [Fact] + public void The_bare_pattern_excludes_the_default_segment_but_IsValid_admits_it() + { + System.Text.RegularExpressions.Regex + .IsMatch(UnsSegment.DefaultSegment, UnsSegment.Pattern).ShouldBeFalse(); + UnsSegment.IsValid(UnsSegment.DefaultSegment).ShouldBeTrue(); + } +}