From cdd65beb6ccaee71e495bbe17cce6398b07f8c33 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 24 Jun 2026 18:27:42 -0400 Subject: [PATCH] =?UTF-8?q?feat(cli+templateengine+deploymanager):=20resol?= =?UTF-8?q?ve=20follow-ups=20#4/#5/#6/#8=20=E2=80=94=20CLI=20ergonomics=20?= =?UTF-8?q?+=20structured=20deploy=20validation=20error?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the four remaining items in the 2026-06-24 template-inheritance/CLI follow-up tracker. #4 — CLI `instance set-bindings` can now set DataSourceReferenceOverride. `--bindings` accepts an optional 3rd element per entry: [attributeName, dataConnectionId, dataSourceReferenceOverride]. A string sets the override; a JSON null or an omitted 3rd element leaves it unset (template default). TryParseBindings accepts 2- or 3-element entries and rejects a non-string/non-null 3rd element or 4+ elements with a clean error. Previously the CLI sent the override as null and silently wiped any existing one (only a raw POST /management could set it). #5 — `template update` is partial, not full-replace (fixed server-side so all clients benefit). UpdateTemplateAsync now uses leave-unchanged semantics: a null description keeps the stored value (pass "" to clear); a null parentTemplateId keeps the existing parent. Parent stays immutable — a non-null differing value is still rejected — but omitting --parent-id is now a no-op instead of failing every derived-template update. #6 — compact `template list`/`get` table output + `--detail`. Table output is now id/name/description/parent/derived + member counts (#attrs/#alarms/ #scripts/#comps/#nativeAlarms) via TemplateTableProjection, fed through a new optional tableProjector seam on CommandHelpers. `--detail` restores the full dump. JSON output is left untouched (always full) so machine consumers are unaffected — the projector only runs on the table path. #8 — structured deploy-time validation error. New ValidationResult.SummarizeErrors() (Commons) returns a grouped, capped summary: leading total count, one line per ValidationCategory, and a per-module rollup (canonical name up to its last dot) with counts + "... and N more module(s)" caps. DeploymentService uses it for the "Pre-deployment validation failed" message and logs the full per-entry list via LogWarning. Replaces the flat semicolon-joined dump that became a wall of text for instances with 50-194 unbound attributes. Tests: +8 Commons (SummarizeErrors), +8 CLI (4 binding 3-element / 4 table projection), +2 net TemplateEngine (partial-update). Affected suites green: Commons 587, CLI 341, TemplateEngine 447, DeploymentManager 101, ManagementService 230, CentralUI 866; full solution builds 0/0. Docs: Component-DeploymentManager.md "Validation Error Reporting"; CLI README (set-bindings 3-element form, template update leave-unchanged, list/get --detail); UpdateTemplateCommand doc; known-issues tracker #4/#5/#6/#8 resolved (all 8 items now closed). --- ...mplate-inheritance-ui-and-cli-followups.md | 55 +++++++- .../Component-DeploymentManager.md | 13 ++ .../Commands/CommandHelpers.cs | 24 +++- .../Commands/InstanceCommands.cs | 56 ++++++-- .../Commands/TemplateCommands.cs | 30 ++++- .../Commands/TemplateTableProjection.cs | 112 ++++++++++++++++ src/ZB.MOM.WW.ScadaBridge.CLI/README.md | 49 +++++-- .../Messages/Management/TemplateCommands.cs | 7 + .../Types/Flattening/ValidationResult.cs | 94 ++++++++++++++ .../DeploymentService.cs | 14 +- .../TemplateService.cs | 24 +++- .../InstanceArgumentParsingTests.cs | 52 ++++++++ .../TemplateTableProjectionTests.cs | 122 ++++++++++++++++++ .../Types/ValidationResultSummaryTests.cs | 111 ++++++++++++++++ .../TemplateServiceTests.cs | 36 +++++- 15 files changed, 745 insertions(+), 54 deletions(-) create mode 100644 src/ZB.MOM.WW.ScadaBridge.CLI/Commands/TemplateTableProjection.cs create mode 100644 tests/ZB.MOM.WW.ScadaBridge.CLI.Tests/TemplateTableProjectionTests.cs create mode 100644 tests/ZB.MOM.WW.ScadaBridge.Commons.Tests/Types/ValidationResultSummaryTests.cs diff --git a/docs/known-issues/2026-06-24-template-inheritance-ui-and-cli-followups.md b/docs/known-issues/2026-06-24-template-inheritance-ui-and-cli-followups.md index 4197fcfa..3305ddce 100644 --- a/docs/known-issues/2026-06-24-template-inheritance-ui-and-cli-followups.md +++ b/docs/known-issues/2026-06-24-template-inheritance-ui-and-cli-followups.md @@ -1,9 +1,9 @@ # Follow-up tracker — template-inheritance UI gaps + CLI/validation footguns (2026-06-24 session) -**Status:** PARTIALLY RESOLVED · **Found:** 2026-06-24 · **Context:** live ops session on `wonder-app-vd03` (CvdReactor / Z28061 / Z28061Sim) — renaming the template, adding the LeakTest module, and adding MoveInType to the MESReceiver children. -**Components:** Central UI (#9), Template Engine (#1), CLI (#19), Configuration Database (#17) +**Status:** RESOLVED · **Found:** 2026-06-24 · **Context:** live ops session on `wonder-app-vd03` (CvdReactor / Z28061 / Z28061Sim) — renaming the template, adding the LeakTest module, and adding MoveInType to the MESReceiver children. +**Components:** Central UI (#9), Template Engine (#1), CLI (#19), Configuration Database (#17), Deployment Manager (#2) -**Resolved:** #3 (collision detector) and #7 (sandbox compile surface) on branch `fix/followups-3-7`; #1 + #2 (inherited-member propagation & resync) on branch `fix/followups-1-2` (2026-06-24). Open: #4, #5, #6, #8. +**Resolved:** #3 (collision detector) and #7 (sandbox compile surface) on branch `fix/followups-3-7`; #1 + #2 (inherited-member propagation & resync) on branch `fix/followups-1-2`; #4 + #5 + #6 + #8 (CLI ergonomics + structured deploy validation error) on branch `fix/followups-4-5-6-8` (all 2026-06-24). All items resolved. Issues are listed worst-first. Severities are author estimates. None caused data loss; the runtime/flattened config and deployed instances are correct. @@ -56,7 +56,17 @@ Issues are listed worst-first. Severities are author estimates. None caused data --- ## 4. CLI `instance set-bindings` cannot set `DataSourceReferenceOverride` -**Severity:** Medium · **Components:** CLI (#19) +**Severity:** Medium · **Components:** CLI (#19) · **✅ RESOLVED 2026-06-24 (branch `fix/followups-4-5-6-8`)** + +**Fix:** `--bindings` now accepts an optional **third element** per entry — +`[attributeName, dataConnectionId, dataSourceReferenceOverride]` — so the CLI can set the +per-instance reference override that the wire contract (`ConnectionBinding`) already +carried. A string sets it; a JSON `null` or an omitted third element leaves it unset +(template default). `TryParseBindings` accepts 2- or 3-element entries and rejects a +non-string/non-null third element or 4+ elements with a clean validation error. The +`--bindings` help and CLI README now document the full-replace behaviour (omitting the +override on a re-bind clears any previously-set one). Covered by +`InstanceArgumentParsingTests` (three-element / explicit-null / wrong-type / four-element). **Symptom:** `instance set-bindings --bindings` only accepts `[attributeName, dataConnectionId]` pairs (`InstanceCommands.cs` → `ConnectionBinding(name, connId)` 2-arg). The override is sent as `null`, and because `SetConnectionBindingsAsync` upserts `DataSourceReferenceOverride = b.DataSourceReferenceOverride` (`InstanceService.cs:340`), using the CLI on an attribute that already has an override would **wipe** it. @@ -67,7 +77,19 @@ Issues are listed worst-first. Severities are author estimates. None caused data --- ## 5. CLI `template update` is full-replace, not partial -**Severity:** Low · **Components:** CLI (#19), Template Engine (#1) +**Severity:** Low · **Components:** CLI (#19), Template Engine (#1) · **✅ RESOLVED 2026-06-24 (branch `fix/followups-4-5-6-8`)** + +**Fix:** `TemplateService.UpdateTemplateAsync` now uses **leave-unchanged** semantics for +optional fields (fixed server-side, so every client benefits): a `null` description keeps +the stored value (pass `""` to explicitly clear it), and a `null` `parentTemplateId` keeps +the existing parent. The parent remains immutable — a non-null value that differs from the +current parent is still rejected — but omitting it (the CLI default) is now a no-op instead +of tripping the immutability guard, which previously made `template update` fail on any +derived template unless `--parent-id` was re-passed. CLI `--description`/`--parent-id` help, +the `UpdateTemplateCommand` doc, and the CLI README document the semantics. Tests: +`UpdateTemplate_OmittedParentAndDescription_LeavesUnchanged`, +`UpdateTemplate_EmptyDescription_ClearsIt` (the prior `UpdateTemplate_ClearParent_Fails` +was repurposed, since a null parent now means leave-unchanged rather than clear-and-fail). **Symptom:** omitting `--description` on `template update` overwrites the stored description to NULL (`TemplateService.cs:124-125` assigns Name+Description unconditionally). Renaming a template silently drops its description unless you re-pass it. @@ -76,7 +98,16 @@ Issues are listed worst-first. Severities are author estimates. None caused data --- ## 6. (Minor) CLI `template list`/`get` table output dumps every attribute -**Severity:** Low · **Components:** CLI (#19) +**Severity:** Low · **Components:** CLI (#19) · **✅ RESOLVED 2026-06-24 (branch `fix/followups-4-5-6-8`)** + +**Fix:** `template list`/`get` **table** output is now a compact projection — id / name / +description / parentTemplateId / isDerived plus member **counts** (`#attrs`, `#alarms`, +`#scripts`, `#comps`, `#nativeAlarms`) — via a new `TemplateTableProjection.ProjectSummary` +fed through an optional `tableProjector` seam on `CommandHelpers.ExecuteCommandAsync`/ +`HandleResponse`. A `--detail` flag restores the full table dump. **JSON output is +deliberately left untouched** (always the full payload) so machine consumers are unaffected +— the projector only runs on the table path. Covered by `TemplateTableProjectionTests` +(array/object projection, counts, non-JSON passthrough, size-shrink sanity check). **Symptom:** `--format table template list` emitted ~171 KB (the full attribute set per template inline), unusable in a terminal. `--format json` is fine. @@ -102,7 +133,17 @@ Issues are listed worst-first. Severities are author estimates. None caused data --- ## 8. Deploy-time unbound-binding validation returns one giant semicolon-joined error string -**Severity:** Low · **Components:** Template Engine (#1), Deployment Manager (#2) +**Severity:** Low · **Components:** Template Engine (#1), Deployment Manager (#2) · **✅ RESOLVED 2026-06-24 (branch `fix/followups-4-5-6-8`)** + +**Fix:** new `ValidationResult.SummarizeErrors()` (Commons) returns a grouped, capped +summary: a leading total count, one line per `ValidationCategory`, and within a category a +per-**module** rollup (canonical name up to its last dot) with counts and a `… and N more +module(s)` cap. `DeploymentService` now uses it for the `Pre-deployment validation failed:` +message and logs the full per-entry list via `LogWarning` so nothing is lost. Entity-less +findings (e.g. script-compile errors) fall back to a capped message list. Documented in +`Component-DeploymentManager.md` → "Validation Error Reporting". Covered by +`ValidationResultSummaryTests` (count header, module rollup, breadth cap, root grouping, +message fallback, mixed categories). **Symptom:** Deploying an instance whose data-sourced attributes aren't all bound fails with a single error that concatenates one clause per attribute: `Pre-deployment validation failed: Attribute 'LeftReactorSide.LeakTest.DeltaVac' has a data source reference but no connection binding; Attribute 'LeftReactorSide.LeakTest.ResultType' has …; …`. For 50–194 unbound attrs (e.g. Z28062's unbound LeakTest members) it's a wall of text that's hard to scan in a CLI/UI toast. diff --git a/docs/requirements/Component-DeploymentManager.md b/docs/requirements/Component-DeploymentManager.md index 50bf6f79..76abee0f 100644 --- a/docs/requirements/Component-DeploymentManager.md +++ b/docs/requirements/Component-DeploymentManager.md @@ -64,6 +64,19 @@ flowchart TD class step4 start ``` +### Validation Error Reporting + +When step 2 fails, the returned error is a **grouped, capped summary** rather than a flat +semicolon-joined dump (followup #8). `ValidationResult.SummarizeErrors()` (Commons) leads +with the total error count, then lists one line per `ValidationCategory`; within a +category, entity-scoped findings (notably the unbound connection-binding case, which can +produce 50–194 entries for a richly data-sourced instance) are rolled up by **module** — +the attribute's canonical name up to its last dot — with per-module counts, and the breadth +is capped with a `… and N more module(s)` suffix. The complete per-entry list remains on +`ValidationResult.Errors` and is written to the deploy log (`LogWarning`) so operators can +still see every clause when needed. This keeps the UI/CLI failure toast scannable while +preserving full detail for diagnosis. + ## Deployment Identity & Idempotency - Every deployment is assigned a unique **deployment ID** and includes the flattened configuration's **revision hash** (from the Template Engine). diff --git a/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/CommandHelpers.cs b/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/CommandHelpers.cs index d99f0f88..83492f53 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/CommandHelpers.cs +++ b/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/CommandHelpers.cs @@ -30,6 +30,13 @@ internal static class CommandHelpers /// () is preserved on the error path either way, /// closing CLI-017's regression. /// + /// + /// Optional transform applied to the success JSON body only when the resolved + /// format is table. Lets a command render a compact table projection (e.g. + /// template list dropping per-template attribute dumps, followup #6) while + /// leaving JSON output untouched for machine consumers. Ignored when + /// is supplied. + /// /// A task that resolves to the process exit code (0 = success, 1 = error, 2 = authorization failure). internal static async Task ExecuteCommandAsync( ParseResult result, @@ -39,7 +46,8 @@ internal static class CommandHelpers Option passwordOption, object command, TimeSpan? timeout = null, - Func? onSuccess = null) + Func? onSuccess = null, + Func? tableProjector = null) { var config = CliConfig.Load(); var format = ResolveFormat(result, formatOption, config); @@ -98,7 +106,7 @@ internal static class CommandHelpers return IsAuthorizationFailure(response) ? 2 : 1; } - return HandleResponse(response, format); + return HandleResponse(response, format, tableProjector); } /// @@ -158,8 +166,12 @@ internal static class CommandHelpers /// /// Response received from the management API. /// Output format (json or table). + /// + /// Optional transform applied to the JSON body before table rendering only — JSON + /// output is never altered. See . + /// /// The process exit code (0 = success, 1 = error, 2 = authorization failure). - internal static int HandleResponse(ManagementResponse response, string format) + internal static int HandleResponse(ManagementResponse response, string format, Func? tableProjector = null) { if (response.JsonData != null) { @@ -173,7 +185,11 @@ internal static class CommandHelpers if (string.Equals(format, "table", StringComparison.OrdinalIgnoreCase)) { - WriteAsTable(response.JsonData); + // A table projector compacts the body for terminal display (e.g. dropping + // per-template attribute dumps). JSON output stays full/untouched so + // machine consumers keep the complete payload. + var body = tableProjector != null ? tableProjector(response.JsonData) : response.JsonData; + WriteAsTable(body); } else { diff --git a/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/InstanceCommands.cs b/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/InstanceCommands.cs index f8df02d2..9a6fcadf 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/InstanceCommands.cs +++ b/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/InstanceCommands.cs @@ -54,7 +54,18 @@ public static class InstanceCommands private static Command BuildSetBindings(Option urlOption, Option formatOption, Option usernameOption, Option passwordOption) { var idOption = new Option("--id") { Description = "Instance ID", Required = true }; - var bindingsOption = new Option("--bindings") { Description = "JSON array of [attributeName, dataConnectionId] pairs", Required = true }; + var bindingsOption = new Option("--bindings") + { + Description = "JSON array of binding entries. Each entry is either " + + "[attributeName, dataConnectionId] or " + + "[attributeName, dataConnectionId, dataSourceReferenceOverride] " + + "(the 3rd element overrides the attribute's data-source reference; " + + "pass null or omit it to use the template default). " + + "NOTE: this REPLACES all bindings for the instance — include the " + + "override on every entry that needs one, or omitting it clears any " + + "previously-set override.", + Required = true + }; var cmd = new Command("set-bindings") { Description = "Set data connection bindings for an instance" }; cmd.Add(idOption); @@ -76,11 +87,16 @@ public static class InstanceCommands } /// - /// Parses the --bindings argument — a JSON array of - /// [attributeName, dataConnectionId] pairs — into a typed list. - /// Returns false with a descriptive instead of - /// throwing when the JSON is malformed, a pair has the wrong arity, or an element - /// has the wrong type. + /// Parses the --bindings argument — a JSON array of binding entries — into a + /// typed list. Each entry is either a two-element + /// [attributeName, dataConnectionId] pair or a three-element + /// [attributeName, dataConnectionId, dataSourceReferenceOverride] triple. The + /// optional third element carries the per-instance data-source reference override + /// (); a JSON + /// null (or an omitted third element) leaves it unset so the template default + /// applies. Returns false with a descriptive instead + /// of throwing when the JSON is malformed, an entry has the wrong arity, or an + /// element has the wrong type. /// /// The JSON string to parse. /// The parsed bindings list, or null if parsing fails. @@ -99,16 +115,19 @@ public static class InstanceCommands .Deserialize>>(json); if (pairs == null) { - error = "Bindings JSON must be a non-null array of [attributeName, dataConnectionId] pairs."; + error = "Bindings JSON must be a non-null array of " + + "[attributeName, dataConnectionId] or " + + "[attributeName, dataConnectionId, dataSourceReferenceOverride] entries."; return false; } var result = new List(pairs.Count); foreach (var pair in pairs) { - if (pair.Count != 2) + if (pair.Count is not (2 or 3)) { - error = "Each binding must be a [attributeName, dataConnectionId] pair of exactly two elements."; + error = "Each binding must be a [attributeName, dataConnectionId] pair, " + + "optionally with a third dataSourceReferenceOverride element."; return false; } if (pair[0].ValueKind != System.Text.Json.JsonValueKind.String) @@ -122,7 +141,24 @@ public static class InstanceCommands error = "The second element of each binding (dataConnectionId) must be an integer."; return false; } - result.Add(new ConnectionBinding(pair[0].GetString()!, connectionId)); + + string? referenceOverride = null; + if (pair.Count == 3) + { + var third = pair[2]; + if (third.ValueKind == System.Text.Json.JsonValueKind.String) + { + referenceOverride = third.GetString(); + } + else if (third.ValueKind != System.Text.Json.JsonValueKind.Null) + { + error = "The third element of each binding (dataSourceReferenceOverride) " + + "must be a string or null."; + return false; + } + } + + result.Add(new ConnectionBinding(pair[0].GetString()!, connectionId, referenceOverride)); } bindings = result; diff --git a/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/TemplateCommands.cs b/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/TemplateCommands.cs index c1819e39..9d501039 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/TemplateCommands.cs +++ b/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/TemplateCommands.cs @@ -54,11 +54,19 @@ public static class TemplateCommands private static Command BuildList(Option urlOption, Option formatOption, Option usernameOption, Option passwordOption) { - var cmd = new Command("list") { Description = "List all templates" }; + var detailOption = new Option("--detail") + { + Description = "Include full template definitions (all attributes/alarms/scripts) in table output. " + + "Without it, table output is a compact summary (counts only). JSON output is always full." + }; + var cmd = new Command("list") { Description = "List all templates (compact table summary; use --detail for the full dump)" }; + cmd.Add(detailOption); cmd.SetAction(async (ParseResult result) => { + var detail = result.GetValue(detailOption); return await CommandHelpers.ExecuteCommandAsync( - result, urlOption, formatOption, usernameOption, passwordOption, new ListTemplatesCommand()); + result, urlOption, formatOption, usernameOption, passwordOption, new ListTemplatesCommand(), + tableProjector: detail ? null : TemplateTableProjection.ProjectSummary); }); return cmd; } @@ -66,13 +74,21 @@ public static class TemplateCommands private static Command BuildGet(Option urlOption, Option formatOption, Option usernameOption, Option passwordOption) { var idOption = new Option("--id") { Description = "Template ID", Required = true }; - var cmd = new Command("get") { Description = "Get a template by ID" }; + var detailOption = new Option("--detail") + { + Description = "Include full template definitions (all attributes/alarms/scripts) in table output. " + + "Without it, table output is a compact summary (counts only). JSON output is always full." + }; + var cmd = new Command("get") { Description = "Get a template by ID (compact table summary; use --detail for the full dump)" }; cmd.Add(idOption); + cmd.Add(detailOption); cmd.SetAction(async (ParseResult result) => { var id = result.GetValue(idOption); + var detail = result.GetValue(detailOption); return await CommandHelpers.ExecuteCommandAsync( - result, urlOption, formatOption, usernameOption, passwordOption, new GetTemplateCommand(id)); + result, urlOption, formatOption, usernameOption, passwordOption, new GetTemplateCommand(id), + tableProjector: detail ? null : TemplateTableProjection.ProjectSummary); }); return cmd; } @@ -103,10 +119,10 @@ public static class TemplateCommands { var idOption = new Option("--id") { Description = "Template ID", Required = true }; var nameOption = new Option("--name") { Description = "Template name", Required = true }; - var descOption = new Option("--description") { Description = "Template description" }; - var parentOption = new Option("--parent-id") { Description = "Parent template ID" }; + var descOption = new Option("--description") { Description = "Template description. Omit to leave unchanged; pass an empty string (\"\") to clear it." }; + var parentOption = new Option("--parent-id") { Description = "Parent template ID. Immutable after creation; omit to leave unchanged." }; - var cmd = new Command("update") { Description = "Update a template" }; + var cmd = new Command("update") { Description = "Update a template (omitted optional fields are left unchanged)" }; cmd.Add(idOption); cmd.Add(nameOption); cmd.Add(descOption); diff --git a/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/TemplateTableProjection.cs b/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/TemplateTableProjection.cs new file mode 100644 index 00000000..5fe567ec --- /dev/null +++ b/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/TemplateTableProjection.cs @@ -0,0 +1,112 @@ +using System.Text.Json; +using System.Text.Json.Nodes; + +namespace ZB.MOM.WW.ScadaBridge.CLI.Commands; + +/// +/// Compact table projection for template list / template get (followup #6). +/// The management API returns full Template entities — every attribute, alarm, +/// script, and composition inline — which the generic table renderer dumps as one giant +/// cell per template (~171 KB for a real catalogue, unusable in a terminal). This +/// projector reduces each template to id / name / description / parent / derived plus +/// member counts, leaving JSON output untouched (callers pass this only on the +/// table path) and the full dump available via the command's --detail flag. +/// +internal static class TemplateTableProjection +{ + /// + /// Projects a templates JSON response (an array from list or a single object + /// from get) to its compact summary form. Returns the input unchanged when it + /// is not JSON or not the expected shape, so the generic renderer's own fallbacks + /// still apply. + /// + /// The raw success JSON body from the management API. + /// Compact JSON (same array/object shape) suitable for table rendering. + internal static string ProjectSummary(string json) + { + JsonDocument doc; + try + { + doc = JsonDocument.Parse(json); + } + catch (JsonException) + { + // Not JSON (e.g. a proxy error page) — let the renderer print it verbatim. + return json; + } + + using (doc) + { + var root = doc.RootElement; + if (root.ValueKind == JsonValueKind.Array) + { + var arr = new JsonArray(); + foreach (var item in root.EnumerateArray()) + arr.Add(ProjectElement(item)); + return arr.ToJsonString(); + } + if (root.ValueKind == JsonValueKind.Object) + { + return ProjectElement(root).ToJsonString(); + } + return json; + } + } + + /// Projects a single template object to its compact summary node. + private static JsonNode ProjectElement(JsonElement element) + { + if (element.ValueKind != JsonValueKind.Object) + return JsonValue.Create(element.ToString())!; + + // JsonObject preserves insertion order, fixing the column order for the table. + return new JsonObject + { + ["id"] = Int(element, "id"), + ["name"] = Str(element, "name"), + ["description"] = Str(element, "description"), + ["parentTemplateId"] = Int(element, "parentTemplateId"), + ["isDerived"] = Bool(element, "isDerived"), + ["#attrs"] = Count(element, "attributes"), + ["#alarms"] = Count(element, "alarms"), + ["#scripts"] = Count(element, "scripts"), + ["#comps"] = Count(element, "compositions"), + ["#nativeAlarms"] = Count(element, "nativeAlarmSources"), + }; + } + + private static bool TryGetPropertyCI(JsonElement obj, string name, out JsonElement value) + { + foreach (var prop in obj.EnumerateObject()) + { + if (string.Equals(prop.Name, name, StringComparison.OrdinalIgnoreCase)) + { + value = prop.Value; + return true; + } + } + value = default; + return false; + } + + private static JsonNode? Str(JsonElement obj, string name) + => TryGetPropertyCI(obj, name, out var v) && v.ValueKind == JsonValueKind.String + ? JsonValue.Create(v.GetString()) + : null; + + private static JsonNode? Int(JsonElement obj, string name) + => TryGetPropertyCI(obj, name, out var v) && v.ValueKind == JsonValueKind.Number && v.TryGetInt32(out var n) + ? JsonValue.Create(n) + : null; + + private static JsonNode? Bool(JsonElement obj, string name) + => TryGetPropertyCI(obj, name, out var v) && (v.ValueKind == JsonValueKind.True || v.ValueKind == JsonValueKind.False) + ? JsonValue.Create(v.GetBoolean()) + : null; + + private static JsonNode Count(JsonElement obj, string name) + => JsonValue.Create( + TryGetPropertyCI(obj, name, out var v) && v.ValueKind == JsonValueKind.Array + ? v.GetArrayLength() + : 0); +} diff --git a/src/ZB.MOM.WW.ScadaBridge.CLI/README.md b/src/ZB.MOM.WW.ScadaBridge.CLI/README.md index 1aa44e44..6f5dd2f8 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CLI/README.md +++ b/src/ZB.MOM.WW.ScadaBridge.CLI/README.md @@ -86,23 +86,35 @@ Exit codes: #### `template list` -List all templates with their full attribute, alarm, script, and composition definitions. +List all templates. **Table** output (`--format table`) shows a compact summary — id, +name, description, parent, and member **counts** (`#attrs`, `#alarms`, `#scripts`, +`#comps`, `#nativeAlarms`) — so it stays readable in a terminal. Add `--detail` to dump +the full attribute/alarm/script/composition definitions in the table. **JSON** output +(`--format json`) is always the full, unmodified payload regardless of `--detail`. ```sh -scadabridge --url template list +scadabridge --url template list # compact table +scadabridge --url --format table template list --detail # full table dump +scadabridge --url --format json template list # full JSON (always) ``` +| Option | Required | Description | +|--------|----------|-------------| +| `--detail` | no | Include full definitions in table output (no effect on JSON) | + #### `template get` -Get a single template by ID. +Get a single template by ID. Like `template list`, table output is a compact summary +unless `--detail` is supplied; JSON output is always full. ```sh -scadabridge --url template get --id +scadabridge --url template get --id [--detail] ``` | Option | Required | Description | |--------|----------|-------------| | `--id` | yes | Template ID | +| `--detail` | no | Include full definitions in table output (no effect on JSON) | #### `template create` @@ -120,9 +132,11 @@ scadabridge --url template create --name [--description ] #### `template update` -Update an existing template. An update **replaces** the whole entity — every required -field below must be supplied with the value it should have after the update, even if -it is unchanged. +Update an existing template. Optional fields use **leave-unchanged** semantics: omitting +`--description` or `--parent-id` keeps the stored value rather than wiping it. To +explicitly clear the description, pass an empty string (`--description ""`). The parent +template is immutable after creation — omit `--parent-id` (or pass the current value); +passing a different value is rejected. ```sh scadabridge --url template update --id --name [--description ] [--parent-id ] @@ -132,8 +146,8 @@ scadabridge --url template update --id --name [--descriptio |--------|----------|-------------| | `--id` | yes | Template ID | | `--name` | yes | Template name | -| `--description` | no | Updated description | -| `--parent-id` | no | Updated parent template ID | +| `--description` | no | Updated description. Omit to leave unchanged; pass `""` to clear. | +| `--parent-id` | no | Immutable; omit to leave unchanged. | #### `template delete` @@ -544,7 +558,22 @@ scadabridge --url instance set-bindings --id --bindings | Option | Required | Description | |--------|----------|-------------| | `--id` | yes | Instance ID | -| `--bindings` | yes | JSON array of `[attributeName, dataConnectionId]` pairs (e.g. `[["Speed",7],["Temperature",7]]`) | +| `--bindings` | yes | JSON array of binding entries, each either `[attributeName, dataConnectionId]` or `[attributeName, dataConnectionId, dataSourceReferenceOverride]` | + +The optional **third element** sets the per-instance data-source reference override +(the OPC UA node id / protocol address used in place of the template's +`DataSourceReference` for that attribute). Pass a string to set it, or `null` / omit +it to use the template default: + +```sh +# Bind Speed with an address override; bind Mode using the template default reference. +scadabridge --url instance set-bindings --id 42 \ + --bindings '[["Speed",7,"ns=2;s=Reactor.Speed"],["Mode",7]]' +``` + +> **Note:** this command **replaces** all bindings for the instance. Include the +> override on every entry that needs one — omitting the third element clears any +> previously-set override for that attribute. #### `instance set-overrides` diff --git a/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Management/TemplateCommands.cs b/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Management/TemplateCommands.cs index 35426680..7d3ffa28 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Management/TemplateCommands.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Management/TemplateCommands.cs @@ -3,6 +3,13 @@ namespace ZB.MOM.WW.ScadaBridge.Commons.Messages.Management; public record ListTemplatesCommand; public record GetTemplateCommand(int TemplateId); public record CreateTemplateCommand(string Name, string? Description, int? ParentTemplateId); + +/// +/// Updates a template. Optional fields use leave-unchanged semantics (followup #5): +/// a null keeps the stored description (pass an empty +/// string to clear it), and a null keeps the +/// existing parent (the parent is immutable; a non-null value that differs is rejected). +/// public record UpdateTemplateCommand(int TemplateId, string Name, string? Description, int? ParentTemplateId); public record DeleteTemplateCommand(int TemplateId); public record ValidateTemplateCommand(int TemplateId); diff --git a/src/ZB.MOM.WW.ScadaBridge.Commons/Types/Flattening/ValidationResult.cs b/src/ZB.MOM.WW.ScadaBridge.Commons/Types/Flattening/ValidationResult.cs index 10cb5c31..1d36503c 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Commons/Types/Flattening/ValidationResult.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Commons/Types/Flattening/ValidationResult.cs @@ -1,3 +1,5 @@ +using System.Text; + namespace ZB.MOM.WW.ScadaBridge.Commons.Types.Flattening; /// @@ -12,6 +14,98 @@ public sealed record ValidationResult /// Non-blocking validation warnings. public IReadOnlyList Warnings { get; init; } = []; + /// + /// Produces a compact, human-readable summary of the validation errors instead of a + /// flat semicolon-joined dump (followup #8). The old behaviour concatenated one clause + /// per error — for an instance with 50–194 unbound attributes that is a wall of text + /// unreadable in a CLI/UI toast. This groups errors by + /// and, within a category, rolls entries up by "module" (the entity's canonical name up + /// to its last dot) with counts, capping the breadth with "… and N more". The full, + /// per-entry list is still available on for a detail view or log. + /// + /// + /// Maximum number of modules (or messages, when entries are not entity-scoped) to list + /// per category before collapsing the remainder into a "… and N more" suffix. + /// + /// + /// A multi-line summary, or an empty string when there are no errors. The first line is + /// the total error count; subsequent lines are one per category. + /// + public string SummarizeErrors(int maxGroups = 6) + { + if (Errors.Count == 0) + return string.Empty; + + var sb = new StringBuilder(); + sb.Append(Errors.Count).Append(Errors.Count == 1 ? " error:" : " errors:"); + + // Group errors by category, preserving first-seen order for a stable rendering. + var byCategory = new List<(ValidationCategory Category, List Items)>(); + var categoryIndex = new Dictionary(); + foreach (var error in Errors) + { + if (!categoryIndex.TryGetValue(error.Category, out var i)) + { + i = byCategory.Count; + categoryIndex[error.Category] = i; + byCategory.Add((error.Category, [])); + } + byCategory[i].Items.Add(error); + } + + foreach (var (category, items) in byCategory) + { + sb.AppendLine(); + sb.Append(" • ").Append(category).Append(" (").Append(items.Count).Append("): "); + + // Prefer an entity/module rollup when every entry names an entity (the unbound- + // binding case). Otherwise fall back to a capped list of the raw messages. + if (items.TrueForAll(e => !string.IsNullOrEmpty(e.EntityName))) + { + var modules = new List<(string Module, int Count)>(); + var moduleIndex = new Dictionary(StringComparer.Ordinal); + foreach (var entry in items) + { + var module = ModuleOf(entry.EntityName!); + if (!moduleIndex.TryGetValue(module, out var mi)) + { + mi = modules.Count; + moduleIndex[module] = mi; + modules.Add((module, 0)); + } + modules[mi] = (module, modules[mi].Count + 1); + } + + var shown = modules.Take(maxGroups).Select(m => $"{m.Module} ({m.Count})"); + sb.Append(string.Join(", ", shown)); + if (modules.Count > maxGroups) + sb.Append($", … and {modules.Count - maxGroups} more module(s)"); + } + else + { + var shown = items.Take(maxGroups).Select(e => e.Message); + sb.Append(string.Join("; ", shown)); + if (items.Count > maxGroups) + sb.Append($"; … and {items.Count - maxGroups} more"); + } + } + + return sb.ToString(); + } + + /// + /// Returns the "module" portion of a canonical attribute name — everything up to the + /// last . (e.g. LeftReactorSide.LeakTest.DeltaVac → LeftReactorSide.LeakTest). + /// A name with no dot is reported under (root). + /// + /// The entity's canonical name. + /// The module prefix, or (root) for top-level members. + private static string ModuleOf(string canonicalName) + { + var lastDot = canonicalName.LastIndexOf('.'); + return lastDot > 0 ? canonicalName[..lastDot] : "(root)"; + } + /// Returns a result with no errors or warnings. /// A with empty error and warning lists. public static ValidationResult Success() => new(); diff --git a/src/ZB.MOM.WW.ScadaBridge.DeploymentManager/DeploymentService.cs b/src/ZB.MOM.WW.ScadaBridge.DeploymentManager/DeploymentService.cs index 32bf9731..20f669cb 100644 --- a/src/ZB.MOM.WW.ScadaBridge.DeploymentManager/DeploymentService.cs +++ b/src/ZB.MOM.WW.ScadaBridge.DeploymentManager/DeploymentService.cs @@ -179,8 +179,18 @@ public class DeploymentService if (!validationResult.IsValid) { - var errors = string.Join("; ", validationResult.Errors.Select(e => e.Message)); - return Result.Failure($"Pre-deployment validation failed: {errors}"); + // Followup #8: return a grouped/summarized error (leading count + per-module + // rollup, capped) instead of a flat semicolon-joined dump that becomes a wall + // of text for instances with dozens of unbound attributes. The full per-entry + // list still goes to the deploy log for operators who need every clause. + _logger.LogWarning( + "Pre-deployment validation failed for instance {InstanceId} ({ErrorCount} error(s)): {Detail}", + instanceId, + validationResult.Errors.Count, + string.Join("; ", validationResult.Errors.Select(e => e.Message))); + + return Result.Failure( + $"Pre-deployment validation failed: {validationResult.SummarizeErrors()}"); } // Serialize for transmission (also the payload stored in the deployed diff --git a/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/TemplateService.cs b/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/TemplateService.cs index 9a6ab7c2..52d7539b 100644 --- a/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/TemplateService.cs +++ b/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/TemplateService.cs @@ -90,12 +90,17 @@ public class TemplateService } /// - /// Updates a template's name and description. Parent template is immutable after creation. + /// Updates a template's name and (optionally) description. Optional fields use + /// leave-unchanged semantics so an omitted field is not silently wiped (followup #5): + /// a null leaves the stored description as-is — + /// pass an empty string to explicitly clear it. Parent template is immutable after + /// creation; a null leaves it unchanged, + /// and a non-null value that differs from the current parent is rejected. /// /// ID of the template to update. - /// New name for the template. - /// New description. - /// Must match the existing parent (cannot be changed). + /// New name for the template (required, non-empty). + /// New description, or null to leave unchanged. Empty string clears it. + /// null to leave unchanged; a non-null value must match the existing parent (cannot be changed). /// Username of the user updating the template. /// Cancellation token. /// Result containing the updated template or failure message. @@ -115,15 +120,20 @@ public class TemplateService return Result