From e9a84ba220edc7df3a834079c3f0336c526889c3 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 15 Jun 2026 13:36:40 -0400 Subject: [PATCH] feat(deploy): surface connection-level changes in the deployment diff (#10) ComputeConnectionsDiff existed with tests but was never called and ConfigurationDiff had no slot for it, so standalone connection endpoint/protocol/failover drift never appeared in the deployment diff (only per-attribute binding drift did). Add a ConnectionChanges slot, wire ComputeConnectionsDiff into ComputeDiff, and render the connection section in the deployment diff UI. --- .../Pages/Deployment/Topology.razor | 125 ++++++++++++++++++ .../Types/Flattening/ConfigurationDiff.cs | 11 +- .../Flattening/DiffService.cs | 19 ++- .../TopologyPageTests.cs | 88 ++++++++++++ .../Flattening/DiffServiceTests.cs | 101 ++++++++++++++ 5 files changed, 336 insertions(+), 8 deletions(-) diff --git a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Deployment/Topology.razor b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Deployment/Topology.razor index 63be901a..6a0fb025 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Deployment/Topology.razor +++ b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Deployment/Topology.razor @@ -864,12 +864,137 @@ ? "The deployed revision hash differs from the current template-derived hash. Redeploy to apply changes." : "No differences between deployed and current configuration."); builder.CloseElement(); + + // DeploymentManager-018: render the structured diff sections so + // the operator sees WHAT changed, not just that the hash moved. + // Each section uses the same compact change-table idiom; the + // connection section surfaces standalone endpoint/protocol/ + // failover drift that no per-attribute row would show (#10). + var d = diffResult.Diff; + if (d != null) + { + RenderChangeSection(builder, 100_000, "Attributes", d.AttributeChanges, + a => a.Value ?? "—"); + + RenderChangeSection(builder, 200_000, "Alarms", d.AlarmChanges, + a => $"P{a.PriorityLevel} · {a.TriggerType}"); + + RenderChangeSection(builder, 300_000, "Scripts", d.ScriptChanges, + s => s.TriggerType ?? "—"); + + RenderChangeSection(builder, 400_000, "Connections", d.ConnectionChanges, + c => FormatConnection(c)); + } } }; await _diffDialog.ShowAsync($"Deployment Diff — {inst.UniqueName}", body); } + // Compact summary of a connection's deployment-relevant fields for the diff + // table's Before/After cells: protocol, primary endpoint config, and the + // failover retry count. Mirrors the fields ConnectionsEqual compares. + private static string FormatConnection( + ZB.MOM.WW.ScadaBridge.Commons.Types.Flattening.ConnectionConfig c) + { + var endpoint = string.IsNullOrWhiteSpace(c.ConfigurationJson) ? "—" : c.ConfigurationJson; + return $"{c.Protocol} · {endpoint} · failover ×{c.FailoverRetryCount}"; + } + + // Renders one change section (a heading plus a Bootstrap change-table) for a + // set of diff entries, matching the deployment-diff idiom used elsewhere in + // the UI: table-sm/table-striped, a colored change badge, and Before/After + // text columns. Nothing is rendered when the section has no entries, so the + // four sections (attributes, alarms, scripts, connections) all read the same + // and only appear when they actually changed. seqBase values are spaced + // 100k apart so each section's per-row sequence numbers (13 per row) stay in + // a disjoint, ascending range no matter how many entries a section has. + private static void RenderChangeSection( + Microsoft.AspNetCore.Components.Rendering.RenderTreeBuilder builder, + int seqBase, + string heading, + IReadOnlyList> entries, + Func summarize) + { + if (entries.Count == 0) + return; + + builder.OpenElement(seqBase, "div"); + builder.AddAttribute(seqBase + 1, "class", "mt-3"); + + builder.OpenElement(seqBase + 2, "div"); + builder.AddAttribute(seqBase + 3, "class", "fw-semibold small mb-1"); + builder.AddContent(seqBase + 4, $"{heading} ({entries.Count})"); + builder.CloseElement(); + + builder.OpenElement(seqBase + 5, "table"); + builder.AddAttribute(seqBase + 6, "class", "table table-sm table-striped align-middle mb-0"); + + // Header row. + builder.OpenElement(seqBase + 7, "thead"); + builder.OpenElement(seqBase + 8, "tr"); + AppendHeaderCell(builder, seqBase + 9, "Name"); + AppendHeaderCell(builder, seqBase + 12, "Change"); + AppendHeaderCell(builder, seqBase + 15, "Before"); + AppendHeaderCell(builder, seqBase + 18, "After"); + builder.CloseElement(); // tr + builder.CloseElement(); // thead + + builder.OpenElement(seqBase + 21, "tbody"); + var rowSeq = seqBase + 22; + foreach (var entry in entries) + { + builder.OpenElement(rowSeq, "tr"); + + builder.OpenElement(rowSeq + 1, "td"); + builder.AddContent(rowSeq + 2, entry.CanonicalName); + builder.CloseElement(); + + builder.OpenElement(rowSeq + 3, "td"); + builder.OpenElement(rowSeq + 4, "span"); + builder.AddAttribute(rowSeq + 5, "class", ChangeBadgeClass(entry.ChangeType)); + builder.AddContent(rowSeq + 6, entry.ChangeType.ToString()); + builder.CloseElement(); + builder.CloseElement(); + + builder.OpenElement(rowSeq + 7, "td"); + builder.AddAttribute(rowSeq + 8, "class", "small text-muted"); + builder.AddContent(rowSeq + 9, + entry.OldValue is null ? "—" : summarize(entry.OldValue)); + builder.CloseElement(); + + builder.OpenElement(rowSeq + 10, "td"); + builder.AddAttribute(rowSeq + 11, "class", "small"); + builder.AddContent(rowSeq + 12, + entry.NewValue is null ? "—" : summarize(entry.NewValue)); + builder.CloseElement(); + + builder.CloseElement(); // tr + rowSeq += 13; + } + builder.CloseElement(); // tbody + + builder.CloseElement(); // table + builder.CloseElement(); // div.mt-3 + } + + private static void AppendHeaderCell( + Microsoft.AspNetCore.Components.Rendering.RenderTreeBuilder builder, int seq, string text) + { + builder.OpenElement(seq, "th"); + builder.AddAttribute(seq + 1, "scope", "col"); + builder.AddContent(seq + 2, text); + builder.CloseElement(); + } + + private static string ChangeBadgeClass( + ZB.MOM.WW.ScadaBridge.Commons.Types.Flattening.DiffChangeType changeType) => changeType switch + { + ZB.MOM.WW.ScadaBridge.Commons.Types.Flattening.DiffChangeType.Added => "badge bg-success", + ZB.MOM.WW.ScadaBridge.Commons.Types.Flattening.DiffChangeType.Removed => "badge bg-danger", + _ => "badge bg-warning text-dark", + }; + // ---- Dropdown option helpers ---- private IEnumerable<(int Id, string Label)> EnumerateSiteOptions() { diff --git a/src/ZB.MOM.WW.ScadaBridge.Commons/Types/Flattening/ConfigurationDiff.cs b/src/ZB.MOM.WW.ScadaBridge.Commons/Types/Flattening/ConfigurationDiff.cs index 888f3da6..f1bc6c7b 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Commons/Types/Flattening/ConfigurationDiff.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Commons/Types/Flattening/ConfigurationDiff.cs @@ -12,8 +12,8 @@ public sealed record ConfigurationDiff public string? OldRevisionHash { get; init; } /// Revision hash of the new configuration being compared. public string? NewRevisionHash { get; init; } - /// True when any attribute, alarm, or script changes are present. - public bool HasChanges => AttributeChanges.Count > 0 || AlarmChanges.Count > 0 || ScriptChanges.Count > 0; + /// True when any attribute, alarm, script, or connection changes are present. + public bool HasChanges => AttributeChanges.Count > 0 || AlarmChanges.Count > 0 || ScriptChanges.Count > 0 || ConnectionChanges.Count > 0; /// Diff entries for resolved attributes. public IReadOnlyList> AttributeChanges { get; init; } = []; @@ -21,6 +21,13 @@ public sealed record ConfigurationDiff public IReadOnlyList> AlarmChanges { get; init; } = []; /// Diff entries for resolved scripts. public IReadOnlyList> ScriptChanges { get; init; } = []; + + /// + /// Diff entries for connection configurations, keyed by connection name. + /// Surfaces standalone endpoint/protocol/failover drift that does not show + /// up as a per-attribute binding change (TemplateEngine-018). + /// + public IReadOnlyList> ConnectionChanges { get; init; } = []; } /// diff --git a/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/Flattening/DiffService.cs b/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/Flattening/DiffService.cs index 71acf3f3..d6bc6a9f 100644 --- a/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/Flattening/DiffService.cs +++ b/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/Flattening/DiffService.cs @@ -42,6 +42,13 @@ public class DiffService s => s.CanonicalName, ScriptsEqual); + // TemplateEngine-018: surface standalone connection endpoint/protocol/ + // failover drift. Per-attribute binding changes already show up under + // AttributeChanges, but a connection's own ConfigurationJson / + // BackupConfigurationJson / Protocol / FailoverRetryCount edits do not — + // those only appear here. + var connectionChanges = ComputeConnectionsDiff(oldConfig, newConfig); + return new ConfigurationDiff { InstanceUniqueName = newConfig.InstanceUniqueName, @@ -49,7 +56,8 @@ public class DiffService NewRevisionHash = newRevisionHash, AttributeChanges = attributeChanges, AlarmChanges = alarmChanges, - ScriptChanges = scriptChanges + ScriptChanges = scriptChanges, + ConnectionChanges = connectionChanges }; } @@ -159,11 +167,10 @@ public class DiffService /// TemplateEngine-018: produces a per-connection diff between two flattened /// configurations, emitting Added / Removed / Changed entries keyed by the /// connection name. Mirrors the existing - /// shape used for attributes / alarms / scripts but is exposed as a separate - /// method because in - /// ZB.MOM.WW.ScadaBridge.Commons does not yet carry a ConnectionChanges - /// slot — the public diff record will be extended in a paired Commons change - /// (this file is the only one in this fix's scope). A null + /// shape used for attributes / alarms / scripts. Called by + /// to populate + /// , and exposed publicly so + /// callers can compute connection drift in isolation. A null /// Connections dictionary on either side is treated as the empty map. /// /// The previously deployed configuration, or null diff --git a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/TopologyPageTests.cs b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/TopologyPageTests.cs index 0fb3ea89..f4176b4f 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/TopologyPageTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/TopologyPageTests.cs @@ -1,4 +1,5 @@ using System.Security.Claims; +using System.Text.Json; using ZB.MOM.WW.ScadaBridge.Security; using Bunit; using Microsoft.AspNetCore.Components.Authorization; @@ -12,7 +13,10 @@ using ZB.MOM.WW.ScadaBridge.Commons.Entities.Sites; using ZB.MOM.WW.ScadaBridge.Commons.Entities.Templates; using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories; using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services; +using ZB.MOM.WW.ScadaBridge.Commons.Types; using ZB.MOM.WW.ScadaBridge.Commons.Types.Enums; +using ZB.MOM.WW.ScadaBridge.Commons.Types.Flattening; +using ZB.MOM.WW.ScadaBridge.Commons.Entities.Deployment; using ZB.MOM.WW.ScadaBridge.Communication; using ZB.MOM.WW.ScadaBridge.DeploymentManager; using ZB.MOM.WW.ScadaBridge.CentralUI.Components.Shared; @@ -292,6 +296,90 @@ public class TopologyPageTests : BunitContext Assert.Throws(() => instanceLabel.DoubleClick()); } + [Fact] + public void Diff_ConnectionEndpointChange_RendersConnectionSection() + { + // TemplateEngine-018 / DeploymentManager-018: a standalone connection + // endpoint edit (no per-attribute binding change) must surface in the + // deployment-diff modal. Before ConnectionChanges was wired through + // ComputeDiff + the UI, this redeploy showed only the stale-hash badge + // with no indication that the connection endpoint had moved. + // The DiffDialog body-scroll lock + focus call out to JS interop on + // open; loose mode no-ops the handlers we don't explicitly set up. + JSInterop.Mode = JSRuntimeMode.Loose; + + var areasBySite = new Dictionary> + { + [1] = new List { new("Line-1") { Id = 10, SiteId = 1 } } + }; + SeedRepos( + sites: new[] { new Site("Plant-A", "plant-a") { Id = 1 } }, + instances: new[] + { + new Instance("Pump-001") { Id = 100, SiteId = 1, AreaId = 10, State = InstanceState.Enabled } + }, + areasBySite: areasBySite); + + // Deployed snapshot: connection "plc1" points at host-a. + var deployedConfig = new FlattenedConfiguration + { + InstanceUniqueName = "Pump-001", + Connections = new Dictionary + { + ["plc1"] = new ConnectionConfig + { + Protocol = "OpcUa", + ConfigurationJson = "{\"endpoint\":\"opc.tcp://host-a:4840\"}", + FailoverRetryCount = 3, + } + } + }; + _deployRepo.GetDeployedSnapshotByInstanceIdAsync(100, Arg.Any()) + .Returns(Task.FromResult( + new DeployedConfigSnapshot("dep-1", "hash-old", + JsonSerializer.Serialize(deployedConfig)))); + + // Current template-derived config: same connection now points at host-b. + var currentConfig = new FlattenedConfiguration + { + InstanceUniqueName = "Pump-001", + Connections = new Dictionary + { + ["plc1"] = new ConnectionConfig + { + Protocol = "OpcUa", + ConfigurationJson = "{\"endpoint\":\"opc.tcp://host-b:4840\"}", + FailoverRetryCount = 3, + } + } + }; + _pipeline.FlattenAndValidateAsync(100, Arg.Any()) + .Returns(Task.FromResult(Result.Success( + new FlatteningPipelineResult(currentConfig, "hash-new", ValidationResult.Success())))); + + var cut = Render(); + FindToggleForLabel(cut, "Plant-A")!.Click(); + FindToggleForLabel(cut, "Line-1")!.Click(); + + // The per-node action menu only renders after a context-menu (right + // click) on the instance row, so open it first, then click "Diff". + var instanceRow = cut.FindAll(".tv-row") + .First(row => row.QuerySelector(".tv-label")?.TextContent == "Pump-001"); + instanceRow.ContextMenu(); + + var diffButton = cut.FindAll("button.dropdown-item") + .First(b => b.TextContent.Trim() == "Diff"); + diffButton.Click(); + + var markup = cut.Markup; + Assert.Contains("Connections", markup); + Assert.Contains("plc1", markup); + Assert.Contains("host-a", markup); + Assert.Contains("host-b", markup); + // The change is a modification, so the row carries the "Changed" badge. + Assert.Contains("Changed", markup); + } + [Fact] public void LegacyInstancesRoute_IsDeclaredOnTopologyPage() { diff --git a/tests/ZB.MOM.WW.ScadaBridge.TemplateEngine.Tests/Flattening/DiffServiceTests.cs b/tests/ZB.MOM.WW.ScadaBridge.TemplateEngine.Tests/Flattening/DiffServiceTests.cs index 56b56331..e405a109 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.TemplateEngine.Tests/Flattening/DiffServiceTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.TemplateEngine.Tests/Flattening/DiffServiceTests.cs @@ -369,4 +369,105 @@ public class DiffServiceTests Assert.Empty(diff); } + + // ── TemplateEngine-018: ComputeDiff wires ComputeConnectionsDiff into the + // public ConfigurationDiff.ConnectionChanges slot so standalone connection + // protocol/endpoint/failover drift surfaces in the deployment diff (#10). ── + + [Fact] + public void ComputeDiff_ConnectionProtocolAndEndpointAndFailoverChange_PopulatesConnectionChanges() + { + // Protocol, endpoint config JSON, and failover retry count all differ + // on the same connection. Before this wiring, ComputeDiff dropped the + // entire connection dimension so this redeploy showed "no changes". + var oldConfig = new FlattenedConfiguration + { + InstanceUniqueName = "Instance1", + Connections = new Dictionary + { + ["plc1"] = new ConnectionConfig + { + Protocol = "OpcUa", + ConfigurationJson = "{\"endpoint\":\"opc.tcp://host-a:4840\"}", + FailoverRetryCount = 3, + } + } + }; + var newConfig = new FlattenedConfiguration + { + InstanceUniqueName = "Instance1", + Connections = new Dictionary + { + ["plc1"] = new ConnectionConfig + { + Protocol = "Modbus", + ConfigurationJson = "{\"endpoint\":\"opc.tcp://host-b:4840\"}", + FailoverRetryCount = 5, + } + } + }; + + var diff = _sut.ComputeDiff(oldConfig, newConfig); + + Assert.True(diff.HasChanges); + Assert.Single(diff.ConnectionChanges); + var entry = diff.ConnectionChanges[0]; + Assert.Equal("plc1", entry.CanonicalName); + Assert.Equal(DiffChangeType.Changed, entry.ChangeType); + Assert.Equal("OpcUa", entry.OldValue!.Protocol); + Assert.Equal("Modbus", entry.NewValue!.Protocol); + Assert.Contains("host-a", entry.OldValue!.ConfigurationJson); + Assert.Contains("host-b", entry.NewValue!.ConfigurationJson); + Assert.Equal(3, entry.OldValue!.FailoverRetryCount); + Assert.Equal(5, entry.NewValue!.FailoverRetryCount); + } + + [Fact] + public void ComputeDiff_OnlyConnectionDiffers_HasChangesIsTrue() + { + // Attributes, alarms, and scripts are identical; only a connection's + // endpoint changed. HasChanges must be true so the diff view does not + // claim "no differences" while a connection endpoint silently moved. + var attributes = new List + { + new ResolvedAttribute { CanonicalName = "Temp", Value = "25", DataType = "Double" } + }; + var oldConfig = new FlattenedConfiguration + { + InstanceUniqueName = "Instance1", + Attributes = attributes, + Connections = new Dictionary + { + ["plc1"] = new ConnectionConfig + { + Protocol = "OpcUa", + ConfigurationJson = "{\"endpoint\":\"opc.tcp://host-a:4840\"}", + FailoverRetryCount = 3, + } + } + }; + var newConfig = new FlattenedConfiguration + { + InstanceUniqueName = "Instance1", + Attributes = attributes, + Connections = new Dictionary + { + ["plc1"] = new ConnectionConfig + { + Protocol = "OpcUa", + ConfigurationJson = "{\"endpoint\":\"opc.tcp://host-b:4840\"}", + FailoverRetryCount = 3, + } + } + }; + + var diff = _sut.ComputeDiff(oldConfig, newConfig); + + Assert.True(diff.HasChanges); + Assert.Empty(diff.AttributeChanges); + Assert.Empty(diff.AlarmChanges); + Assert.Empty(diff.ScriptChanges); + Assert.Single(diff.ConnectionChanges); + Assert.Equal(DiffChangeType.Changed, diff.ConnectionChanges[0].ChangeType); + } }