dohertj2/lmxopcua
1
0
Fork 0
Code Issues 14 Pull Requests Actions Packages Projects Releases Wiki Activity

perf(adminui): one guard round-trip per bulk reference add

Browse Source
This commit is contained in:
Joseph Doherty
2026-08-16 04:03:49 -04:00
parent 541a223b96
commit fa36d64e8f
5 changed files with 251 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/EffectiveNameGuard.cs
+89 -25
View File
@@ -12,8 +12,11 @@ namespace ZB.MOM.WW.OtOpcUa.AdminUI.Uns;
/// <c>DraftValidator</c> <c>UnsEffectiveNameCollision</c> rule.
/// </summary>
/// <remarks>
/// Each call creates and disposes its own pooled context — the same per-call pattern
/// <see cref="UnsTreeService"/> uses — so the guard is safe to register <c>Scoped</c>.
/// Each call creates and disposes its own context — the same per-call pattern
/// <see cref="UnsTreeService"/> uses — so the guard is safe to register <c>Scoped</c>. The factory
/// really is pooled: <c>ServiceCollectionExtensions.AddOtOpcUaConfigDb</c> registers it with
/// <c>AddPooledDbContextFactory</c>, so the per-call create rents from the pool and the dispose
/// returns the context to it.
/// </remarks>
public sealed class EffectiveNameGuard(IDbContextFactory<OtOpcUaConfigDbContext> dbFactory) : IEffectiveNameGuard
{
@@ -27,6 +30,48 @@ public sealed class EffectiveNameGuard(IDbContextFactory<OtOpcUaConfigDbContext>
{
await using var db = await dbFactory.CreateDbContextAsync(ct);
var nameSpace = await LoadNameSpaceAsync(db, equipmentId, ct);
return FirstCollision(nameSpace, proposedEffectiveName, kind, excludeSourceId, equipmentId);
}
/// <inheritdoc />
/// <remarks>
/// The whole point of the batch entry point: ONE context and ONE load of the equipment's name
/// space, then every proposed name tested in memory against it — the per-name rules and message
/// text come from the same <see cref="FirstCollision"/> helper <see cref="CheckAsync"/> uses, so
/// the two can never drift. A batch is a create of N new rows, so there is no self-row to exclude.
/// </remarks>
public async Task<string?> CheckManyAsync(
string equipmentId,
IReadOnlyList<string> proposedEffectiveNames,
EffectiveNameSourceKind kind,
CancellationToken ct = default)
{
ArgumentNullException.ThrowIfNull(proposedEffectiveNames);
if (proposedEffectiveNames.Count == 0) return null;
await using var db = await dbFactory.CreateDbContextAsync(ct);
var nameSpace = await LoadNameSpaceAsync(db, equipmentId, ct);
foreach (var proposed in proposedEffectiveNames)
{
var collision = FirstCollision(nameSpace, proposed, kind, excludeSourceId: null, equipmentId);
if (collision is not null) return collision;
}
return null;
}
/// <summary>
/// Loads the equipment's current effective-name space as a flat, ordered row list: references
/// first (override else the backing raw tag's <c>Name</c>), then VirtualTags, then ScriptedAlarms
/// — the order <see cref="CheckAsync"/> has always reported collisions in.
/// </summary>
private static async Task<List<NameRow>> LoadNameSpaceAsync(
OtOpcUaConfigDbContext db, string equipmentId, CancellationToken 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
@@ -46,47 +91,66 @@ public sealed class EffectiveNameGuard(IDbContextFactory<OtOpcUaConfigDbContext>
.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)
var rows = new List<NameRow>(references.Count + virtualTags.Count + scriptedAlarms.Count);
foreach (var r in references)
{
if (IsSelf(EffectiveNameSourceKind.ScriptedAlarm, a.ScriptedAlarmId, kind, excludeSourceId))
continue;
if (Collides(a.Name, proposedEffectiveName))
return Message(proposedEffectiveName, "ScriptedAlarm", a.ScriptedAlarmId, equipmentId);
var effective = r.DisplayNameOverride
?? (tagNameById.TryGetValue(r.TagId, out var n) ? n : null);
rows.Add(new NameRow(EffectiveNameSourceKind.Reference, r.UnsTagReferenceId, effective, "reference"));
}
foreach (var v in virtualTags)
rows.Add(new NameRow(EffectiveNameSourceKind.VirtualTag, v.VirtualTagId, v.Name, "VirtualTag"));
foreach (var a in scriptedAlarms)
rows.Add(new NameRow(EffectiveNameSourceKind.ScriptedAlarm, a.ScriptedAlarmId, a.Name, "ScriptedAlarm"));
return rows;
}
/// <summary>
/// The single in-memory collision rule shared by <see cref="CheckAsync"/> and
/// <see cref="CheckManyAsync"/> — same self-row exclusion, same ordinal comparison, same message.
/// </summary>
private static string? FirstCollision(
List<NameRow> nameSpace,
string proposedEffectiveName,
EffectiveNameSourceKind kind,
string? excludeSourceId,
string equipmentId)
{
foreach (var row in nameSpace)
{
if (IsSelf(row.Kind, row.SourceId, kind, excludeSourceId)) continue;
if (Collides(row.EffectiveName, proposedEffectiveName))
return Message(proposedEffectiveName, row.SourceLabel, row.SourceId, equipmentId);
}
return null;
}
/// <summary>One existing effective-name-space row: what it is, its logical id, its effective name
/// (null when a reference's backing tag is missing — such a row contributes no name), and the label
/// the collision message uses.</summary>
private readonly record struct NameRow(
EffectiveNameSourceKind Kind,
string SourceId,
string? EffectiveName,
string SourceLabel);
/// <summary>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).</summary>
private static bool IsSelf(
src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/IEffectiveNameGuard.cs
+34
View File
@@ -58,4 +58,38 @@ public interface IEffectiveNameGuard
EffectiveNameSourceKind kind,
string? excludeSourceId,
CancellationToken ct = default);
/// <summary>
/// Checks many proposed names against ONE load of the equipment's name space. Returns the first
/// collision message (in input order), or <see langword="null"/> when all are free.
/// </summary>
/// <remarks>
/// The bulk-authoring entry point (a select-all "add references" over N raw tags), where the
/// per-name <see cref="CheckAsync"/> would cost N contexts × 4 queries. There is no
/// <c>excludeSourceId</c>: a batch is a CREATE of N new rows, so no self-row exists to exclude.
/// The default implementation simply loops <see cref="CheckAsync"/> — correct but not fast — so
/// a stub / no-op guard needs no change; <c>EffectiveNameGuard</c> overrides it with the
/// single-load version.
/// </remarks>
/// <param name="equipmentId">The owning equipment's logical id.</param>
/// <param name="proposedEffectiveNames">The effective names to be authored, in the order the caller wants them reported.</param>
/// <param name="kind">Which surface the proposed names are for (words the error; identifies the self-row kind).</param>
/// <param name="ct">Cancellation token.</param>
/// <returns><see langword="null"/> when every name is free; otherwise the first name's readable error.</returns>
async Task<string?> CheckManyAsync(
string equipmentId,
IReadOnlyList<string> proposedEffectiveNames,
EffectiveNameSourceKind kind,
CancellationToken ct = default)
{
ArgumentNullException.ThrowIfNull(proposedEffectiveNames);
foreach (var proposed in proposedEffectiveNames)
{
var collision = await CheckAsync(equipmentId, proposed, kind, excludeSourceId: null, ct);
if (collision is not null) return collision;
}
return null;
}
}
src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/UnsTreeService.cs
+12 -2
View File
@@ -209,6 +209,11 @@ public sealed class UnsTreeService(
// Track effective names proposed within THIS batch so two raw tags sharing a Name are caught (the
// guard sees only persisted rows; a same-batch collision would otherwise slip through).
var batchNames = new HashSet<string>(StringComparer.Ordinal);
// The proposed effective names, in selection order, for the ONE post-loop guard round-trip. The
// per-name guard call used to sit inside this loop, costing a fresh context + 4 queries per
// selected tag (a select-all of 500 tags ≈ 2,000 queries); the batch check loads the equipment's
// name space once and reports the first collision in this same order.
var proposedNames = new List<string>(distinctTagIds.Count);
var pending = new List<UnsTagReference>();
foreach (var tagId in distinctTagIds)
@@ -228,8 +233,7 @@ public sealed class UnsTreeService(
return new UnsMutationResult(false,
$"Two selected raw tags share the effective name '{tag.Name}'; set a display-name override so they stay unique within the equipment.");
var collision = await _guard.CheckAsync(equipmentId, tag.Name, EffectiveNameSourceKind.Reference, null, ct);
if (collision is not null) return new UnsMutationResult(false, collision);
proposedNames.Add(tag.Name);
pending.Add(new UnsTagReference
{
@@ -241,6 +245,12 @@ public sealed class UnsTreeService(
});
}
// One guard round-trip for the whole batch (one context + one load of the equipment's existing
// effective-name space), regardless of how many tags were selected.
var collision = await _guard.CheckManyAsync(
equipmentId, proposedNames, EffectiveNameSourceKind.Reference, ct);
if (collision is not null) return new UnsMutationResult(false, collision);
db.UnsTagReferences.AddRange(pending);
try
{
tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/EffectiveNameGuardTests.cs
+53
View File
@@ -190,6 +190,59 @@ public sealed class EffectiveNameGuardTests
result.ShouldBeNull();
}
/// <summary>
/// The batch check is a pure round-trip saving: for every proposed name it must return exactly
/// what <see cref="EffectiveNameGuard.CheckAsync"/> returns for the FIRST colliding name in input
/// order — including the source-specific message text — across all three source kinds.
/// </summary>
[Theory]
// First collision is the no-override reference ("speed"), even though an alarm name follows it.
[InlineData(new[] { "free-a", "speed", "highpress", "temperature" }, "speed")]
// First collision is the OVERRIDE-derived reference effective name.
[InlineData(new[] { "free-a", "temperature", "speed" }, "temperature")]
// First collision is the VirtualTag.
[InlineData(new[] { "free-a", "computed", "speed" }, "computed")]
// First collision is the ScriptedAlarm.
[InlineData(new[] { "free-a", "highpress", "speed" }, "highpress")]
public async Task CheckMany_matches_CheckAsync_verdicts(string[] names, string expectedCollidingName)
{
var factory = SeedFactory();
var guard = Guard(factory);
var expected = await guard.CheckAsync(
EquipmentId, expectedCollidingName, EffectiveNameSourceKind.Reference, excludeSourceId: null);
expected.ShouldNotBeNull();
var actual = await guard.CheckManyAsync(EquipmentId, names, EffectiveNameSourceKind.Reference);
actual.ShouldBe(expected);
}
/// <summary>A batch where every proposed name is free reports no collision.</summary>
[Fact]
public async Task CheckMany_all_free_returns_null()
{
var guard = Guard(SeedFactory());
var result = await guard.CheckManyAsync(
EquipmentId,
["free-a", "free-b", "Speed" /* ordinal: capital S does not collide */, "free-c"],
EffectiveNameSourceKind.Reference);
result.ShouldBeNull();
}
/// <summary>An empty batch is trivially free (and must not throw).</summary>
[Fact]
public async Task CheckMany_empty_batch_returns_null()
{
var guard = Guard(SeedFactory());
var result = await guard.CheckManyAsync(EquipmentId, [], EffectiveNameSourceKind.Reference);
result.ShouldBeNull();
}
private sealed class NamedFactory(string name) : IDbContextFactory<OtOpcUaConfigDbContext>
{
public OtOpcUaConfigDbContext CreateDbContext() =>
tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/UnsTreeServiceReferenceTests.cs
+63 -1
View File
@@ -23,13 +23,24 @@ public sealed class UnsTreeServiceReferenceTests
{
private const string EquipmentId = "EQ-1";
/// <summary>A configurable <see cref="IEffectiveNameGuard"/>: returns <see cref="Result"/> from every check.</summary>
/// <summary>
/// A configurable <see cref="IEffectiveNameGuard"/>: returns <see cref="Result"/> from every check.
/// <see cref="CheckManyAsync"/> is implemented explicitly (rather than inheriting the interface's
/// looping default) so <see cref="CheckManyCalls"/> counts BATCH round-trips — that count is what
/// pins "one guard round-trip per bulk add, regardless of how many tags were selected".
/// </summary>
private sealed class FakeGuard : IEffectiveNameGuard
{
public string? Result { get; set; }
public string? LastProposedName { get; private set; }
/// <summary>How many times the BATCH entry point was called.</summary>
public int CheckManyCalls { get; private set; }
/// <summary>The names passed to the last <see cref="CheckManyAsync"/> call, in input order.</summary>
public IReadOnlyList<string> LastProposedNames { get; private set; } = [];
public Task<string?> CheckAsync(
string equipmentId, string proposedEffectiveName, EffectiveNameSourceKind kind,
string? excludeSourceId, CancellationToken ct = default)
@@ -37,6 +48,16 @@ public sealed class UnsTreeServiceReferenceTests
LastProposedName = proposedEffectiveName;
return Task.FromResult(Result);
}
public Task<string?> CheckManyAsync(
string equipmentId, IReadOnlyList<string> proposedEffectiveNames,
EffectiveNameSourceKind kind, CancellationToken ct = default)
{
CheckManyCalls++;
LastProposedNames = [.. proposedEffectiveNames];
LastProposedName = proposedEffectiveNames.Count > 0 ? proposedEffectiveNames[^1] : null;
return Task.FromResult(proposedEffectiveNames.Count > 0 ? Result : null);
}
}
private static OtOpcUaConfigDbContext Db(string name) =>
@@ -178,6 +199,47 @@ public sealed class UnsTreeServiceReferenceTests
db.UnsTagReferences.Any(r => r.EquipmentId == EquipmentId).ShouldBeFalse();
}
/// <summary>
/// A multi-tag add asks the guard ONCE — one batch call carrying every proposed effective name in
/// selection order — instead of one round-trip per selected tag.
/// </summary>
[Fact]
public async Task AddReferences_asks_the_guard_once_for_the_whole_batch()
{
var name = $"ref-batch-guard-{Guid.NewGuid():N}";
Seed(name);
var guard = new FakeGuard();
var service = new UnsTreeService(Factory(name), guard);
var result = await service.AddReferencesAsync(EquipmentId, ["TAG-speed", "TAG-running", "TAG-grouped"]);
result.Ok.ShouldBeTrue();
guard.CheckManyCalls.ShouldBe(1);
guard.LastProposedNames.ShouldBe(new[] { "speed", "running", "nested" });
using var db = Db(name);
db.UnsTagReferences.Count(r => r.EquipmentId == EquipmentId).ShouldBe(3);
}
/// <summary>A collision reported by the batch guard fails the whole add; nothing is persisted.</summary>
[Fact]
public async Task AddReferences_batch_guard_collision_rejected()
{
var name = $"ref-batch-guard-fail-{Guid.NewGuid():N}";
Seed(name);
var guard = new FakeGuard { Result = "'running' already exists on equipment 'machine-1'." };
var service = new UnsTreeService(Factory(name), guard);
var result = await service.AddReferencesAsync(EquipmentId, ["TAG-speed", "TAG-running"]);
result.Ok.ShouldBeFalse();
result.Error.ShouldBe("'running' already exists on equipment 'machine-1'.");
guard.CheckManyCalls.ShouldBe(1);
using var db = Db(name);
db.UnsTagReferences.Any(r => r.EquipmentId == EquipmentId).ShouldBeFalse();
}
// ----- SetReferenceOverride -----
/// <summary>Setting an override changes the effective name; clearing it falls back to the raw name.</summary>