fix(uns): enforce the UNS segment rule on every authoring path (#535)
v2-ci / build (push) Successful in 4m30s
v2-ci / unit-tests (push) Failing after 16m11s

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
This commit is contained in:
Joseph Doherty
2026-08-01 11:53:58 -04:00
parent 9303289d05
commit 1a1dbc3f00
8 changed files with 418 additions and 20 deletions
@@ -16,8 +16,6 @@ namespace ZB.MOM.WW.OtOpcUa.Configuration.Validation;
/// </summary>
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;
/// <summary>
@@ -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<ValidationError> 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));
}
/// <summary>Cluster.Enterprise + Site + area + line + equipment + 4 slashes ≤ 200 chars.</summary>
@@ -0,0 +1,73 @@
using System.Text.RegularExpressions;
namespace ZB.MOM.WW.OtOpcUa.Configuration.Validation;
/// <summary>
/// The one authority on what a UNS path segment — a <c>UnsArea.Name</c>, <c>UnsLine.Name</c> or
/// <c>Equipment.Name</c> — may be.
/// </summary>
/// <remarks>
/// <para>
/// A segment becomes a level of the UNS NodeId (<c>Area/Line/Equipment/EffectiveName</c>), so the
/// rule is not cosmetic: it is what keeps a NodeId parseable and stable.
/// </para>
/// <para>
/// ⚠️ <b>This exists because the rule was duplicated and drifted (Gitea #535).</b> The deploy gate
/// held it privately in <c>DraftValidator</c> and the AdminUI restated it on the equipment form
/// alone, so an area or line could be named <c>Packaging</c> or <c>Line 1</c>, saved, and then
/// fail the deploy with <c>UnsSegmentInvalid</c>. Because a deployment snapshots the whole
/// database, one badly-named area blocked <b>every</b> 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.
/// </para>
/// </remarks>
public static class UnsSegment
{
/// <summary>
/// The reserved segment for an unnamed level. Accepted alongside <see cref="Pattern"/>, so a
/// validator that applies the pattern <em>alone</em> is stricter than the deploy gate and would
/// make such a row uneditable.
/// </summary>
public const string DefaultSegment = "_default";
/// <summary>
/// The segment pattern, as a constant so it can be used in a
/// <c>[RegularExpression]</c> attribute (which needs a compile-time constant).
/// </summary>
/// <remarks>
/// ⚠️ This does <b>not</b> admit <see cref="DefaultSegment"/> — an attribute cannot express the
/// alternation and stay readable. <see cref="AttributePattern"/> is the one to put on a form;
/// <see cref="IsValid"/> is the one to call in code.
/// </remarks>
public const string Pattern = "^[a-z0-9-]{1,32}$";
/// <summary>
/// <see cref="Pattern"/> widened to admit <see cref="DefaultSegment"/> — the form of the rule to
/// put in a <c>[RegularExpression]</c> attribute, so the UI accepts exactly what the deploy gate
/// accepts and no less.
/// </summary>
public const string AttributePattern = "^([a-z0-9-]{1,32}|_default)$";
/// <summary>The message shown for a rejected segment, in the UI and from the service guards.</summary>
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);
/// <summary>Whether a name is a usable UNS path segment.</summary>
/// <param name="segment">The candidate name.</param>
/// <returns><see langword="true"/> when it may be deployed.</returns>
public static bool IsValid(string? segment) =>
segment is not null && (Rx.IsMatch(segment) || segment == DefaultSegment);
/// <summary>
/// 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.
/// </summary>
/// <param name="level">The entity and property, e.g. <c>UnsArea.Name</c>.</param>
/// <param name="segment">The rejected value.</param>
/// <returns>The message.</returns>
public static string Describe(string level, string? segment) =>
$"{level} '{segment}' does not match [a-z0-9-]{{1,32}} or '{DefaultSegment}'";
}
@@ -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
/// <summary>The working form for the Details tab — identical to EquipmentModal.FormModel.</summary>
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; } = "";
@@ -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 @@
<label class="form-label" for="area-name">Name</label>
<InputText id="area-name" @bind-Value="_form.Name" class="form-control form-control-sm" />
<ValidationMessage For="@(() => _form.Name)" />
<div class="form-text">UNS path segment: lowercase letters, digits and dashes, 1-32 characters. It becomes a level of the OPC UA NodeId.</div>
</div>
<div class="mb-3">
<label class="form-label" for="area-cluster">Served by cluster</label>
@@ -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; }
}
@@ -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 @@
<label class="form-label" for="line-name">Name</label>
<InputText id="line-name" @bind-Value="_form.Name" class="form-control form-control-sm" />
<ValidationMessage For="@(() => _form.Name)" />
<div class="form-text">UNS path segment: lowercase letters, digits and dashes, 1-32 characters. It becomes a level of the OPC UA NodeId.</div>
</div>
<div class="mb-3">
<label class="form-label" for="line-notes">Notes</label>
@@ -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; }
}
}
@@ -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(
/// <inheritdoc />
public async Task<UnsMutationResult> 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}.");
}
}
/// <summary>
/// The rejection for a UNS name that is not a usable path segment.
/// </summary>
/// <remarks>
/// ⚠️ <b>The service is where this is enforced, not the form (Gitea #535).</b> 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 <b>every</b> 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.
/// </remarks>
/// <param name="level">The field being rejected, e.g. <c>Area name</c>.</param>
/// <param name="value">The rejected value.</param>
/// <returns>A failed mutation result carrying the reason.</returns>
private static UnsMutationResult SegmentRejected(string level, string? value) =>
new(false, $"{UnsSegment.Describe(level, value)}. {UnsSegment.Message}");
}