From ec57df1009001a19715223359758680d331584d4 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Sat, 25 Apr 2026 01:51:02 -0400 Subject: [PATCH] =?UTF-8?q?Task=20#155=20=E2=80=94=20TagService=20+=20Tags?= =?UTF-8?q?Tab=20CRUD=20UI=20for=20Modbus=20tags?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the remaining loop on user-visible Modbus tag editing. Pre-#155 tags arrived only via SQL seeding or runtime ITagDiscovery; the Admin UI had no interactive surface for creating / editing / deleting tag rows. Changes: - TagService.cs (Admin/Services/) — CRUD wrapper around OtOpcUaConfigDbContext.Tags. ListAsync supports optional driver / equipment filters; CreateAsync auto-derives TagId; UpdateAsync persists editable fields; DeleteAsync removes the row. Mirrors the EquipmentService shape. - TagsTab.razor (Components/Pages/Clusters/) — list + filter + add/edit/remove form. The address/config editor is conditional: when the selected DriverInstance is Modbus, ModbusAddressEditor (#145) renders with live-parse preview; otherwise a generic JSON textarea (matches the DriversTab pattern from #147). Save-side serializes the address-string into TagConfig as `{"addressString":"..."}` JSON. - ClusterDetail.razor — new "Tags" tab in the cluster-detail nav strip + the routing switch. - Program.cs — TagService registered as a scoped DI service. Drive-by fix: ModbusDriverFactoryExtensions.CreateInstance promoted from internal to public — Admin.Tests was using it via reflection-friendly internal access that broke under the #153 logger overload addition. Public is the right access modifier anyway since the Server-side bootstrapper calls it from a different assembly. Drive-by fix #2: ModbusDriverConfigDto was missing MaxReadGap (#143) — surfaced by the #147 round-trip test that flips MaxReadGap=12 in the view model and asserts it lands on the resolved options. Added the field + binding line. Confirms #143's DriverConfig JSON binding was incomplete since the original commit; no production deployment configured this knob through JSON until now so the gap stayed hidden. Tests (4 new TagServiceTests): - Create_And_List_Surfaces_The_Tag — CreateAsync auto-assigns TagId; list returns the row. - List_Filters_By_DriverInstance — driver-scoped filter works. - Update_Persists_Editable_Fields — Name / DataType / AccessLevel / TagConfig all persist through Update. - Delete_Removes_The_Row — basic delete verification. 113 + 4 (TagService) + 2 (DriversTab round-trip restored after compile fix) = 119 Admin tests green. Solution build clean. Caveat: bUnit-style render tests for TagsTab still aren't included — Admin.Tests doesn't have bUnit set up. The TagService logic is fully covered; the razor component's parser/save glue is exercised by hand at runtime for now. --- .../Pages/Clusters/ClusterDetail.razor | 5 + .../Components/Pages/Clusters/TagsTab.razor | 271 ++++++++++++++++++ src/ZB.MOM.WW.OtOpcUa.Admin/Program.cs | 1 + .../Services/TagService.cs | 71 +++++ .../ModbusDriverFactoryExtensions.cs | 8 +- .../TagServiceTests.cs | 96 +++++++ 6 files changed, 450 insertions(+), 2 deletions(-) create mode 100644 src/ZB.MOM.WW.OtOpcUa.Admin/Components/Pages/Clusters/TagsTab.razor create mode 100644 src/ZB.MOM.WW.OtOpcUa.Admin/Services/TagService.cs create mode 100644 tests/ZB.MOM.WW.OtOpcUa.Admin.Tests/TagServiceTests.cs diff --git a/src/ZB.MOM.WW.OtOpcUa.Admin/Components/Pages/Clusters/ClusterDetail.razor b/src/ZB.MOM.WW.OtOpcUa.Admin/Components/Pages/Clusters/ClusterDetail.razor index 72cd9ba..57ca593 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Admin/Components/Pages/Clusters/ClusterDetail.razor +++ b/src/ZB.MOM.WW.OtOpcUa.Admin/Components/Pages/Clusters/ClusterDetail.razor @@ -51,6 +51,7 @@ else
    • +
    • @@ -89,6 +90,10 @@ else { } + else if (_tab == "tags" && _currentDraft is not null) + { + + } else if (_tab == "acls" && _currentDraft is not null) { diff --git a/src/ZB.MOM.WW.OtOpcUa.Admin/Components/Pages/Clusters/TagsTab.razor b/src/ZB.MOM.WW.OtOpcUa.Admin/Components/Pages/Clusters/TagsTab.razor new file mode 100644 index 0000000..ddef975 --- /dev/null +++ b/src/ZB.MOM.WW.OtOpcUa.Admin/Components/Pages/Clusters/TagsTab.razor @@ -0,0 +1,271 @@ +@using System.Text.Json +@using ZB.MOM.WW.OtOpcUa.Admin.Components.Pages.Modbus +@using ZB.MOM.WW.OtOpcUa.Admin.Services +@using ZB.MOM.WW.OtOpcUa.Configuration.Entities +@using ZB.MOM.WW.OtOpcUa.Configuration.Enums +@using ZB.MOM.WW.OtOpcUa.Driver.Modbus +@inject TagService TagSvc +@inject DriverInstanceService DriverSvc +@inject EquipmentService EquipmentSvc + +@* + #155 — interactive Tag CRUD scoped to a draft generation. Conditional editor: when the + selected DriverInstance is Modbus, the address input switches to ModbusAddressEditor (#145) + so users get the live-parse preview + grammar validation. Other driver types fall back to + a generic JSON textarea, matching the DriversTab pattern from #147. +*@ + +
    +

    Tags (draft gen @GenerationId)

    + +
    + +
    +
    + + +
    +
    + +@if (_tags is null) {

    Loading…

    } +else if (_tags.Count == 0 && !_showForm) {

    No tags in this filter.

    } +else if (_tags.Count > 0) +{ + + + + + + @foreach (var t in _tags) + { + + + + + + + + + + } + +
    NameDriverEquipmentDataTypeAccessTagConfig
    @t.Name@t.DriverInstanceId@(t.EquipmentId ?? "—")@t.DataType@t.AccessLevel@t.TagConfig + + +
    +} + +@if (_showForm) +{ +
    +
    +
    @(_editMode ? "Edit tag" : "New tag")
    +
    +
    + + +
    +
    + + +
    +
    + + +
    +
    + + +
    +
    + + +
    +
    +
    + + +
    +
    +
    + +
    + @if (_isModbus) + { + + } + else + { + + + } +
    + + @if (_error is not null) {
    @_error
    } + +
    + + +
    +
    +
    +} + +@code { + [Parameter] public long GenerationId { get; set; } + [Parameter] public string ClusterId { get; set; } = string.Empty; + + private List? _tags; + private List? _drivers; + private List? _equipment; + private string _filterDriverId = string.Empty; + + private bool _showForm; + private bool _editMode; + private Tag _draft = NewBlankDraft(); + private string? _error; + private bool _isModbus; + private string? _modbusAddress; + + private static Tag NewBlankDraft() => new() + { + TagId = string.Empty, DriverInstanceId = string.Empty, Name = string.Empty, + DataType = "Int32", AccessLevel = TagAccessLevel.Read, TagConfig = string.Empty, + }; + + protected override async Task OnParametersSetAsync() + { + _drivers = await DriverSvc.ListAsync(GenerationId, CancellationToken.None); + _equipment = await EquipmentSvc.ListAsync(GenerationId, CancellationToken.None); + await ReloadAsync(); + } + + private async Task ReloadAsync() + { + _tags = await TagSvc.ListAsync(GenerationId, + string.IsNullOrWhiteSpace(_filterDriverId) ? null : _filterDriverId, + equipmentId: null, + CancellationToken.None); + } + + private void StartAdd() + { + _draft = NewBlankDraft(); + _editMode = false; + _modbusAddress = null; + _isModbus = false; + _error = null; + _showForm = true; + } + + private void StartEdit(Tag row) + { + _draft = new Tag + { + TagRowId = row.TagRowId, + GenerationId = row.GenerationId, + TagId = row.TagId, + DriverInstanceId = row.DriverInstanceId, + DeviceId = row.DeviceId, + EquipmentId = row.EquipmentId, + Name = row.Name, + FolderPath = row.FolderPath, + DataType = row.DataType, + AccessLevel = row.AccessLevel, + WriteIdempotent = row.WriteIdempotent, + PollGroupId = row.PollGroupId, + TagConfig = row.TagConfig, + }; + _editMode = true; + OnDriverChanged(); + // Try to extract addressString from existing JSON config so the Modbus editor pre-fills. + if (_isModbus) _modbusAddress = TryExtractAddressString(row.TagConfig); + _error = null; + _showForm = true; + } + + private void OnDriverChanged() + { + var driver = _drivers?.FirstOrDefault(d => d.DriverInstanceId == _draft.DriverInstanceId); + _isModbus = driver is not null + && string.Equals(driver.DriverType, "Modbus", StringComparison.OrdinalIgnoreCase); + } + + private void OnAddressChanged() + { + // Sync the address string into TagConfig as a JSON object the factory consumes. + if (string.IsNullOrWhiteSpace(_modbusAddress)) return; + _draft.TagConfig = JsonSerializer.Serialize(new { addressString = _modbusAddress }); + } + + private static string? TryExtractAddressString(string tagConfig) + { + try + { + using var doc = JsonDocument.Parse(tagConfig); + return doc.RootElement.TryGetProperty("addressString", out var v) ? v.GetString() : null; + } + catch { return null; } + } + + private void Cancel() + { + _showForm = false; + _editMode = false; + } + + private async Task SaveAsync() + { + _error = null; + try + { + if (string.IsNullOrWhiteSpace(_draft.Name) || string.IsNullOrWhiteSpace(_draft.DriverInstanceId)) + { + _error = "Name and DriverInstance are required."; + return; + } + if (_editMode) + await TagSvc.UpdateAsync(_draft, CancellationToken.None); + else + await TagSvc.CreateAsync(GenerationId, _draft, CancellationToken.None); + _showForm = false; + _editMode = false; + await ReloadAsync(); + } + catch (Exception ex) { _error = ex.Message; } + } + + private async Task DeleteAsync(Guid id) + { + await TagSvc.DeleteAsync(id, CancellationToken.None); + await ReloadAsync(); + } +} diff --git a/src/ZB.MOM.WW.OtOpcUa.Admin/Program.cs b/src/ZB.MOM.WW.OtOpcUa.Admin/Program.cs index 6277f22..209f963 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Admin/Program.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Admin/Program.cs @@ -41,6 +41,7 @@ builder.Services.AddDbContext(opt => builder.Services.AddScoped(); builder.Services.AddScoped(); builder.Services.AddScoped(); +builder.Services.AddScoped(); builder.Services.AddScoped(); builder.Services.AddScoped(); builder.Services.AddScoped(); diff --git a/src/ZB.MOM.WW.OtOpcUa.Admin/Services/TagService.cs b/src/ZB.MOM.WW.OtOpcUa.Admin/Services/TagService.cs new file mode 100644 index 0000000..6036967 --- /dev/null +++ b/src/ZB.MOM.WW.OtOpcUa.Admin/Services/TagService.cs @@ -0,0 +1,71 @@ +using Microsoft.EntityFrameworkCore; +using ZB.MOM.WW.OtOpcUa.Configuration; +using ZB.MOM.WW.OtOpcUa.Configuration.Entities; + +namespace ZB.MOM.WW.OtOpcUa.Admin.Services; + +/// +/// #155 — Tag CRUD scoped to a draft generation. Tags are the canonical signal definitions +/// (one row per OPC UA variable) the Server materialises into the address space at startup. +/// Mirrors the shape of ; writes are restricted to draft +/// generations only (published generations are immutable per the validation pipeline). +/// +public sealed class TagService(OtOpcUaConfigDbContext db) +{ + /// Lists all tags in a generation, ordered by name. Optional driver / equipment filter. + public Task> ListAsync(long generationId, + string? driverInstanceId = null, + string? equipmentId = null, + CancellationToken ct = default) + { + var query = db.Tags.AsNoTracking().Where(t => t.GenerationId == generationId); + if (!string.IsNullOrWhiteSpace(driverInstanceId)) + query = query.Where(t => t.DriverInstanceId == driverInstanceId); + if (!string.IsNullOrWhiteSpace(equipmentId)) + query = query.Where(t => t.EquipmentId == equipmentId); + return query.OrderBy(t => t.Name).ToListAsync(ct); + } + + /// + /// Creates a new tag row in the given draft. TagId is auto-derived as a GUID — the + /// human-friendly Name is the user-facing identifier. + /// + public async Task CreateAsync(long draftId, Tag input, CancellationToken ct) + { + input.GenerationId = draftId; + if (string.IsNullOrWhiteSpace(input.TagId)) + input.TagId = Guid.NewGuid().ToString("N"); + db.Tags.Add(input); + await db.SaveChangesAsync(ct); + return input; + } + + public async Task UpdateAsync(Tag updated, CancellationToken ct) + { + var existing = await db.Tags + .FirstOrDefaultAsync(t => t.TagRowId == updated.TagRowId, ct) + ?? throw new InvalidOperationException($"Tag row {updated.TagRowId} not found"); + + // Editable fields. TagId / GenerationId are immutable; the Validation pipeline rejects + // changes that would break referential integrity (sp_ValidateDraft per decision #110). + existing.Name = updated.Name; + existing.DriverInstanceId = updated.DriverInstanceId; + existing.DeviceId = updated.DeviceId; + existing.EquipmentId = updated.EquipmentId; + existing.FolderPath = updated.FolderPath; + existing.DataType = updated.DataType; + existing.AccessLevel = updated.AccessLevel; + existing.WriteIdempotent = updated.WriteIdempotent; + existing.PollGroupId = updated.PollGroupId; + existing.TagConfig = updated.TagConfig; + await db.SaveChangesAsync(ct); + } + + public async Task DeleteAsync(Guid tagRowId, CancellationToken ct) + { + var existing = await db.Tags.FirstOrDefaultAsync(t => t.TagRowId == tagRowId, ct); + if (existing is null) return; + db.Tags.Remove(existing); + await db.SaveChangesAsync(ct); + } +} diff --git a/src/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriverFactoryExtensions.cs b/src/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriverFactoryExtensions.cs index ae1fa41..4813c46 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriverFactoryExtensions.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriverFactoryExtensions.cs @@ -28,10 +28,12 @@ public static class ModbusDriverFactoryExtensions registry.Register(DriverTypeName, (id, json) => CreateInstance(id, json, loggerFactory)); } - internal static ModbusDriver CreateInstance(string driverInstanceId, string driverConfigJson) + /// Public for the Server-side bootstrapper + test consumers (Admin.Tests, etc.). + public static ModbusDriver CreateInstance(string driverInstanceId, string driverConfigJson) => CreateInstance(driverInstanceId, driverConfigJson, loggerFactory: null); - internal static ModbusDriver CreateInstance(string driverInstanceId, string driverConfigJson, ILoggerFactory? loggerFactory) + /// Logger-aware overload — used by 's closure when wired through DI. + public static ModbusDriver CreateInstance(string driverInstanceId, string driverConfigJson, ILoggerFactory? loggerFactory) { ArgumentException.ThrowIfNullOrWhiteSpace(driverInstanceId); ArgumentException.ThrowIfNullOrWhiteSpace(driverConfigJson); @@ -57,6 +59,7 @@ public static class ModbusDriverFactoryExtensions UseFC16ForSingleRegisterWrites = dto.UseFC16ForSingleRegisterWrites ?? false, DisableFC23 = dto.DisableFC23 ?? false, WriteOnChangeOnly = dto.WriteOnChangeOnly ?? false, + MaxReadGap = dto.MaxReadGap ?? 0, Family = dto.Family is null ? ModbusFamily.Generic : ParseEnum(dto.Family, "", driverInstanceId, "Family"), MelsecSubFamily = dto.MelsecSubFamily is null ? MelsecFamily.Q_L_iQR @@ -188,6 +191,7 @@ public static class ModbusDriverFactoryExtensions public bool? UseFC16ForSingleRegisterWrites { get; init; } public bool? DisableFC23 { get; init; } public bool? WriteOnChangeOnly { get; init; } + public ushort? MaxReadGap { get; init; } public string? Family { get; init; } public string? MelsecSubFamily { get; init; } public int? AutoProhibitReprobeMs { get; init; } diff --git a/tests/ZB.MOM.WW.OtOpcUa.Admin.Tests/TagServiceTests.cs b/tests/ZB.MOM.WW.OtOpcUa.Admin.Tests/TagServiceTests.cs new file mode 100644 index 0000000..c77ba80 --- /dev/null +++ b/tests/ZB.MOM.WW.OtOpcUa.Admin.Tests/TagServiceTests.cs @@ -0,0 +1,96 @@ +using Microsoft.EntityFrameworkCore; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Admin.Services; +using ZB.MOM.WW.OtOpcUa.Configuration; +using ZB.MOM.WW.OtOpcUa.Configuration.Entities; +using ZB.MOM.WW.OtOpcUa.Configuration.Enums; + +namespace ZB.MOM.WW.OtOpcUa.Admin.Tests; + +/// +/// #155 — TagService CRUD round-trip coverage. Mirrors the EquipmentService test shape; +/// uses EF Core InMemory so no SQL Server is required. +/// +[Trait("Category", "Unit")] +public sealed class TagServiceTests +{ + [Fact] + public async Task Create_And_List_Surfaces_The_Tag() + { + using var ctx = NewContext(); + var svc = new TagService(ctx); + + var created = await svc.CreateAsync(draftId: 1, NewTag("Temp"), TestContext.Current.CancellationToken); + created.TagId.ShouldNotBeNullOrEmpty(); + created.GenerationId.ShouldBe(1); + + var list = await svc.ListAsync(1, ct: TestContext.Current.CancellationToken); + list.Count.ShouldBe(1); + list[0].Name.ShouldBe("Temp"); + } + + [Fact] + public async Task List_Filters_By_DriverInstance() + { + using var ctx = NewContext(); + var svc = new TagService(ctx); + await svc.CreateAsync(1, NewTag("a", driver: "drv-1"), TestContext.Current.CancellationToken); + await svc.CreateAsync(1, NewTag("b", driver: "drv-2"), TestContext.Current.CancellationToken); + await svc.CreateAsync(1, NewTag("c", driver: "drv-1"), TestContext.Current.CancellationToken); + + var d1 = await svc.ListAsync(1, driverInstanceId: "drv-1", ct: TestContext.Current.CancellationToken); + d1.Count.ShouldBe(2); + d1.Select(t => t.Name).ShouldBe(new[] { "a", "c" }, ignoreOrder: true); + } + + [Fact] + public async Task Update_Persists_Editable_Fields() + { + using var ctx = NewContext(); + var svc = new TagService(ctx); + var t = await svc.CreateAsync(1, NewTag("Original"), TestContext.Current.CancellationToken); + + t.Name = "Renamed"; + t.DataType = "Float"; + t.AccessLevel = TagAccessLevel.ReadWrite; + t.TagConfig = "{\"addressString\":\"40001:F\"}"; + await svc.UpdateAsync(t, TestContext.Current.CancellationToken); + + var fresh = (await svc.ListAsync(1, ct: TestContext.Current.CancellationToken))[0]; + fresh.Name.ShouldBe("Renamed"); + fresh.DataType.ShouldBe("Float"); + fresh.AccessLevel.ShouldBe(TagAccessLevel.ReadWrite); + fresh.TagConfig.ShouldContain("40001:F"); + } + + [Fact] + public async Task Delete_Removes_The_Row() + { + using var ctx = NewContext(); + var svc = new TagService(ctx); + var t = await svc.CreateAsync(1, NewTag("Doomed"), TestContext.Current.CancellationToken); + + await svc.DeleteAsync(t.TagRowId, TestContext.Current.CancellationToken); + + (await svc.ListAsync(1, ct: TestContext.Current.CancellationToken)).ShouldBeEmpty(); + } + + private static Tag NewTag(string name, string driver = "drv-1") => new() + { + TagId = string.Empty, // CreateAsync auto-assigns + DriverInstanceId = driver, + Name = name, + DataType = "Int32", + AccessLevel = TagAccessLevel.Read, + TagConfig = "{}", + }; + + private static OtOpcUaConfigDbContext NewContext() + { + var opts = new DbContextOptionsBuilder() + .UseInMemoryDatabase(Guid.NewGuid().ToString()) + .Options; + return new OtOpcUaConfigDbContext(opts); + } +}