From f480a56737ef13d8b6cdb3c63031d1cea3a3230e Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Fri, 10 Jul 2026 08:52:12 -0400 Subject: [PATCH] feat(instance): native-alarm-source-override CSV bulk import (deferred #12) + doc fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the operator parity gap the deferred-work register tracked as #12: native alarm source overrides could only be set one-at-a-time, while attribute overrides had a CSV bulk path. Adds an all-or-nothing CSV import for native sources. - Commons: extract the RFC-4180 line splitter into a shared CsvLineSplitter (refactor OverrideCsvParser onto it — no behavior change, pinned by its tests); new NativeAlarmSourceOverrideCsvParser (header SourceName,Connection, SourceReference,Filter; blank = inherited). - Commons: NativeAlarmSourceOverrideEntry + bulk SetInstanceNativeAlarmSource- OverridesCommand (auto-registered via the reflection command registry). - ManagementService: Deployer-gated handler — flattens once, validates every source resolves + is unlocked + no duplicates up front, then upserts the whole batch under a single SaveChanges (true all-or-nothing txn). Added to the frozen authorization matrix; dispatch-coverage guard passes. - CLI: `instance native-alarm-source import --instance-id --file` (parity with `instance import-overrides`) + README. - Tests: native parser (Commons), CLI parse/entry mapping, 3 bulk-handler tests (happy path single-commit, locked-source reject, unresolved-source reject). Also corrects a stale XML-doc line in ScriptRuntimeContext (WaitForAttribute quality-gated mode is shipped, not "planned") and updates the deferred-work register: marks #7/#13/#15/#16/#20 verified-resolved, #12 as CLI/API-shipped with only the Central UI upload affordance still pending. Claude-Session: https://claude.ai/code/session_01MtdgwpEeCUn6cUA5f1LMPj --- .../2026-07-08-deferred-work-register.md | 21 ++- .../Commands/InstanceCommands.cs | 47 ++++++ src/ZB.MOM.WW.ScadaBridge.CLI/README.md | 25 +++ .../Messages/Management/InstanceCommands.cs | 23 +++ .../Types/CsvLineSplitter.cs | 113 +++++++++++++ .../NativeAlarmSourceOverrideCsvParser.cs | 125 +++++++++++++++ .../Types/OverrideCsvParser.cs | 99 +----------- .../ManagementActor.cs | 74 +++++++++ .../Scripts/ScriptRuntimeContext.cs | 4 +- .../ImportNativeAlarmSourceOverridesTests.cs | 68 ++++++++ ...NativeAlarmSourceOverrideCsvParserTests.cs | 148 ++++++++++++++++++ .../ManagementActorTests.cs | 106 +++++++++++++ .../RequiredRoleMatrixTests.cs | 1 + 13 files changed, 748 insertions(+), 106 deletions(-) create mode 100644 src/ZB.MOM.WW.ScadaBridge.Commons/Types/CsvLineSplitter.cs create mode 100644 src/ZB.MOM.WW.ScadaBridge.Commons/Types/NativeAlarmSourceOverrideCsvParser.cs create mode 100644 tests/ZB.MOM.WW.ScadaBridge.CLI.Tests/ImportNativeAlarmSourceOverridesTests.cs create mode 100644 tests/ZB.MOM.WW.ScadaBridge.Commons.Tests/NativeAlarmSourceOverrideCsvParserTests.cs diff --git a/docs/plans/2026-07-08-deferred-work-register.md b/docs/plans/2026-07-08-deferred-work-register.md index 9e71443a..35b5426c 100644 --- a/docs/plans/2026-07-08-deferred-work-register.md +++ b/docs/plans/2026-07-08-deferred-work-register.md @@ -18,23 +18,30 @@ them and are removed from this table when that plan's task lands. ## Deferred (with rationale + revisit trigger) | # | Item | Where noted | Rationale for deferral | Revisit trigger | |---|------|-------------|------------------------|-----------------| -| 7 | SecuredWrite audit rows leave SourceNode NULL | CLAUDE.md Security | Logged follow-up; low-volume channel | First per-node forensic query against secured writes | | 8 | Hash-chain tamper evidence (T1); CLI verify-chain is a no-op stub | audit-log roadmap :12 | v1.x by locked decision; append-only DB roles are the control | Compliance requirement for cryptographic tamper evidence | | 9 | Parquet audit archival (T2); endpoint returns 501 | AuditEndpoints.cs:204 | v1.x; 501 + CLI messaging are honest | AuditLog partition volume nears retention ceiling | | 10 | Aggregated live alarm stream for Alarm Summary | m7 design :252 | Snapshot fan-out acceptable at current instance counts | Alarm Summary latency complaints or >~50 instances/site | | 11 | Central-persisted OPC UA cert-trust audit | m7 follow-ups | Broadcast-to-both-nodes covers HA | Governance/audit requirement for trust decisions | -| 12 | Native-alarm-source-override CSV import | m7 follow-ups | Parity gap only; attribute CSV shipped | First bulk native-alarm rollout request | -| 13 | WaitForAttribute quality-gated mode | ScriptRuntimeContext.cs:405 | Planned enhancement per spec §4.2 | First script needing Good-only waits | -| 14 | WaitForAttribute in Test-Run sandbox | waitfor-deferred :222 | Test-Run parity gap; production unaffected | Author feedback on Test-Run fidelity | -| 15 | BrowseNext final-page signal not surfaced | BrowseCommands.cs:34 | One wasted round-trip | UX complaint on browse paging | -| 16 | StubOpcUaClient throws on browse | m7 design :245 | Limits offline UI coverage only | Next browse/search UI regression | +| 12 | Native-alarm-source-override CSV import — **Central UI `InstanceConfigure` upload affordance only** (CLI + Management API + parser shipped 2026-07-10, see Resolved) | m7 follow-ups | CLI/API path closes the operator parity gap; the Blazor upload button is polish | First request to bulk-import native sources from the UI rather than the CLI | +| 14 | WaitForAttribute in Test-Run sandbox | waitfor-deferred :222 | Test-Run parity gap; production unaffected (sandbox throws a clearly-labelled ScriptSandboxException; deploy to a site to exercise) | Author feedback on Test-Run fidelity | | 17 | Unified notifications+site-calls outbox page | stillpending :118 | Explicit M9 decision to keep two pages | Operator confusion reports | | 18 | Folder drag-drop | same, [PERM] | Permanently closed; menu reorder shipped | — (closed) | | 19 | Bundle signing / cluster-to-cluster pull / differential bundles | transport-design :402 | v1 manifest hash + AES-GCM held sufficient | Non-repudiation requirement across orgs | -| 20 | Deployment EXPIRED-row purge | DeploymentManagerRepository.cs:310 | Read path already compensates | EXPIRED rows visible in ops queries | | 21 | SiteAuditBacklogReporter threshold consolidation | SiteAuditBacklogReporter.cs:28 | Config-shape cleanup only | Next SqliteAuditWriterOptions change | | 22 | KPI history hourly rollups | Component-KpiHistory.md | YAGNI; 90-day retention bounds table | KpiSample query latency on dashboards | +## Resolved (verified against the code 2026-07-10) +Rows removed from the Deferred table above once confirmed shipped. Kept here for traceability. + +| # | Item | Resolution | +|---|------|-----------| +| 7 | SecuredWrite audit rows leave SourceNode NULL | Resolved (PLAN-07): `ManagementActor.EmitSecuredWriteAuditAsync` routes through `ICentralAuditWriter`, which stamps `SourceNode` (`central-a`/`central-b`) from `INodeIdentityProvider`. | +| 12 (CLI/API) | Native-alarm-source-override CSV import | Shipped 2026-07-10: shared `CsvLineSplitter`, `NativeAlarmSourceOverrideCsvParser`, bulk all-or-nothing `SetInstanceNativeAlarmSourceOverridesCommand` + ManagementActor handler (Deployer-gated), CLI `instance native-alarm-source import --file`, parser/CLI/handler tests. **UI upload affordance still pending — see row 12 above.** | +| 13 | WaitForAttribute quality-gated ("Good"-only) mode | Already implemented (Commons `WaitForAttribute.RequireGoodQuality`, enforced in `InstanceActor`, threaded through `ScriptRuntimeContext`, tested in `InstanceActorWaitForAttributeTests`). Stale "planned enhancement" doc line corrected 2026-07-10. | +| 15 | BrowseNext final-page signal not surfaced | Already surfaced (M7 browse work): `RealOpcUaClient` sets `Truncated=false`/`ContinuationToken=null` on the last page; `BrowseNodeResult` carries both; `TreeRow.razor` renders "Load more" only when a continuation token remains — no wasted BrowseNext. | +| 16 | StubOpcUaClient throws on browse | Already resolved: `StubOpcUaClient` supports browse + address-space search, covered by `StubOpcUaClientBrowseTests`/`StubOpcUaClientSearchTests`. | +| 20 | Deployment EXPIRED-row purge | Already resolved (PLAN-04): `PendingDeploymentPurgeActor` central singleton (spawned in `AkkaHostedService`) ticks `IDeploymentManagerRepository.PurgeExpiredPendingDeploymentsAsync` every `CommunicationOptions.PendingDeploymentPurgeInterval` (default 1h), options-validated, tested. | + ## New deferrals from review 08 (this plan) | Item | Rationale | Revisit trigger | |------|-----------|-----------------| diff --git a/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/InstanceCommands.cs b/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/InstanceCommands.cs index 84de0585..5e59db00 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/InstanceCommands.cs +++ b/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/InstanceCommands.cs @@ -477,6 +477,53 @@ public static class InstanceCommands }); group.Add(clearCmd); + // import (CSV bulk apply — parity with `instance import-overrides`) + var importIdOption = new Option("--instance-id") { Description = "Instance ID", Required = true }; + var importFileOption = new Option("--file") + { + Description = "Path to the override CSV file (columns: SourceName,Connection,SourceReference,Filter; blank field = inherited)", + Required = true + }; + var importCmd = new Command("import") + { + Description = "Apply native alarm source overrides from a CSV file (all-or-nothing)" + }; + importCmd.Add(importIdOption); + importCmd.Add(importFileOption); + importCmd.SetAction(async (ParseResult result) => + { + var id = result.GetValue(importIdOption); + var filePath = result.GetValue(importFileOption)!; + + string csvText; + try + { + csvText = File.ReadAllText(filePath); + } + catch (Exception ex) + { + OutputFormatter.WriteError($"Cannot read file '{filePath}': {ex.Message}", "FILE_READ_ERROR"); + return 1; + } + + var parseResult = NativeAlarmSourceOverrideCsvParser.Parse(csvText); + if (parseResult.Errors.Count > 0) + { + foreach (var error in parseResult.Errors) + OutputFormatter.WriteError(error, "INVALID_CSV"); + return 1; + } + + var overrides = parseResult.Rows + .Select(r => new NativeAlarmSourceOverrideEntry( + r.SourceName, r.Connection, r.SourceReference, r.Filter)) + .ToList(); + return await CommandHelpers.ExecuteCommandAsync( + result, urlOption, formatOption, usernameOption, passwordOption, + new SetInstanceNativeAlarmSourceOverridesCommand(id, overrides)); + }); + group.Add(importCmd); + return group; } diff --git a/src/ZB.MOM.WW.ScadaBridge.CLI/README.md b/src/ZB.MOM.WW.ScadaBridge.CLI/README.md index 6090a9f4..513036cd 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CLI/README.md +++ b/src/ZB.MOM.WW.ScadaBridge.CLI/README.md @@ -503,6 +503,31 @@ Clear an instance's native alarm source override, reverting to the inherited bin scadabridge --url instance native-alarm-source clear --instance-id --source ``` +#### `instance native-alarm-source import` + +Bulk-apply native alarm source overrides for a single instance from a CSV file +(all-or-nothing — the native-source parity of `instance import-overrides`). The CSV +header is required: `SourceName,Connection,SourceReference,Filter`; a blank override +field keeps the inherited value. Every named source must resolve for the instance and +be unlocked, or the whole batch is rejected before any write. + +```sh +scadabridge --url instance native-alarm-source import --instance-id --file +``` + +| Option | Required | Description | +|--------|----------|-------------| +| `--instance-id` | yes | Instance ID | +| `--file` | yes | Path to the override CSV (`SourceName,Connection,SourceReference,Filter`; blank field = inherited) | + +Example CSV: + +```csv +SourceName,Connection,SourceReference,Filter +Pressure,OpcB,ns=2;s=Pump.Alarm,Active +Module.Temp,,, +``` + #### `instance deploy` Deploy an instance to its site. Acquires the per-instance operation lock. diff --git a/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Management/InstanceCommands.cs b/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Management/InstanceCommands.cs index 229da1cd..815b0ee6 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Management/InstanceCommands.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Management/InstanceCommands.cs @@ -62,3 +62,26 @@ public record DeleteInstanceNativeAlarmSourceOverrideCommand( string SourceCanonicalName); public record ListInstanceNativeAlarmSourceOverridesCommand(int InstanceId); + +/// +/// One entry in a bulk native-alarm-source-override apply: the source binding's +/// canonical name plus the three optional retarget fields (null keeps the inherited +/// value). Mirrors 's fields +/// for a single source. +/// +public record NativeAlarmSourceOverrideEntry( + string SourceCanonicalName, + string? ConnectionNameOverride, + string? SourceReferenceOverride, + string? ConditionFilterOverride); + +/// +/// Bulk, all-or-nothing apply of per-instance native-alarm-source overrides — the +/// native-source parity analogue of . Backs +/// the CLI instance native-alarm-source import --file CSV path. Every entry's +/// source must resolve for the instance and be unlocked; if any entry is invalid the +/// whole batch is rejected before any write. +/// +public record SetInstanceNativeAlarmSourceOverridesCommand( + int InstanceId, + IReadOnlyList Overrides); diff --git a/src/ZB.MOM.WW.ScadaBridge.Commons/Types/CsvLineSplitter.cs b/src/ZB.MOM.WW.ScadaBridge.Commons/Types/CsvLineSplitter.cs new file mode 100644 index 00000000..fab9f0ba --- /dev/null +++ b/src/ZB.MOM.WW.ScadaBridge.Commons/Types/CsvLineSplitter.cs @@ -0,0 +1,113 @@ +namespace ZB.MOM.WW.ScadaBridge.Commons.Types; + +/// +/// Shared, dependency-free RFC-4180-ish splitter for a single physical CSV line. +/// Extracted so the instance-override CSV parsers ( +/// and ) apply identical quoting +/// rules without duplicating the subtle field-state machine. +/// +/// +/// Quoting rules: +/// +/// A field is quoted only if its first non-whitespace char is a +/// " (leading whitespace before the opening quote is allowed and ignored). +/// Inside a quoted field, commas are literal and "" is an escaped single +/// quote; the closing " must be the last non-whitespace char of the field +/// (trailing whitespace after the close is allowed and ignored). +/// A " appearing anywhere else in an unquoted field (i.e. after +/// non-whitespace content) is a literal character and is preserved. +/// Unquoted fields are whitespace-trimmed; quoted field values are kept +/// verbatim. +/// +/// +public static class CsvLineSplitter +{ + /// + /// Splits a single physical CSV line into its fields, applying the quoting rules + /// described on . Returns true with the split + /// on success; returns false when a quoted field + /// is opened but never closed before end-of-line (the caller emits a per-line + /// "unterminated" error). + /// + /// The physical line to split (no trailing newline). + /// The split fields on success; an empty list on failure. + /// true when the line split cleanly; false on an unterminated quoted field. + public static bool TrySplit(string line, out List fields) + { + fields = new List(); + var field = new System.Text.StringBuilder(); + var inQuotes = false; // currently between an opening and closing quote + var quoted = false; // this field opened with a quote → keep value verbatim + var sawContent = false; // any non-whitespace char seen in the current field yet + + for (var i = 0; i < line.Length; i++) + { + var c = line[i]; + + if (inQuotes) + { + if (c == '"') + { + // Doubled quote inside a quoted field → a single literal quote. + if (i + 1 < line.Length && line[i + 1] == '"') + { + field.Append('"'); + i++; + } + else + { + inQuotes = false; // closing quote; only trailing whitespace may follow + } + } + else + { + field.Append(c); + } + + continue; + } + + switch (c) + { + case ',': + fields.Add(Finalize(field, quoted)); + field.Clear(); + inQuotes = false; + quoted = false; + sawContent = false; + break; + case '"' when !sawContent: + // Opening quote: first non-whitespace char of the field. Any + // leading whitespace seen so far is part of the (ignored) prefix. + field.Clear(); + inQuotes = true; + quoted = true; + sawContent = true; + break; + default: + // After a quoted field has closed, only whitespace may appear + // before the next delimiter — it is ignored, not appended. + if (quoted) + break; + + // A '"' here (sawContent already true) falls through as a literal. + if (!char.IsWhiteSpace(c)) + sawContent = true; + field.Append(c); + break; + } + } + + if (inQuotes) + return false; // opened a quoted field that was never closed + + fields.Add(Finalize(field, quoted)); + return true; + } + + private static string Finalize(System.Text.StringBuilder field, bool quoted) + { + var text = field.ToString(); + return quoted ? text : text.Trim(); // only unquoted whitespace is trimmed + } +} diff --git a/src/ZB.MOM.WW.ScadaBridge.Commons/Types/NativeAlarmSourceOverrideCsvParser.cs b/src/ZB.MOM.WW.ScadaBridge.Commons/Types/NativeAlarmSourceOverrideCsvParser.cs new file mode 100644 index 00000000..6fbc11e0 --- /dev/null +++ b/src/ZB.MOM.WW.ScadaBridge.Commons/Types/NativeAlarmSourceOverrideCsvParser.cs @@ -0,0 +1,125 @@ +namespace ZB.MOM.WW.ScadaBridge.Commons.Types; + +/// +/// One parsed per-instance native-alarm-source-override CSV row: the source +/// binding's canonical name plus the three optional retarget fields. A blank +/// (null) // +/// means "keep the inherited value" — the same blank-is-inherited semantics as the +/// CLI instance native-alarm-source set command. is +/// the 1-based source line (the header is line 1) so downstream errors can point +/// back at the operator's file. +/// +public sealed record NativeAlarmSourceOverrideCsvRow( + string SourceName, string? Connection, string? SourceReference, string? Filter, int LineNumber); + +/// +/// Outcome of parsing a native-alarm-source-override CSV: the successfully-parsed +/// plus per-line . Parsing never throws — +/// malformed rows are reported and excluded, valid rows still flow through. +/// Downstream callers validate source names against the flattened instance; this +/// parser is purely syntactic. +/// +public sealed record NativeAlarmSourceOverrideCsvParseResult( + IReadOnlyList Rows, IReadOnlyList Errors); + +/// +/// Pure, dependency-free, quote-aware parser turning per-instance +/// native-alarm-source-override CSV text into structured rows plus per-line errors. +/// Callers supply the text (no file I/O). The header row is required and +/// case-insensitive (SourceName,Connection,SourceReference,Filter). Fields +/// follow the shared RFC-4180 quoting rules — the same +/// splitter uses. The parity analogue of +/// for native alarm source retargets. +/// +public static class NativeAlarmSourceOverrideCsvParser +{ + private const string HeaderError = + "Missing or invalid header row. Expected 'SourceName,Connection,SourceReference,Filter'."; + + private const int ExpectedColumns = 4; + + /// + /// Parses native-alarm-source-override CSV . Returns + /// parsed rows and any per-line errors; never throws. On a missing/unrecognized + /// header returns zero rows and a single header error. + /// + /// The raw CSV text to parse. + /// The parsed rows and any per-line errors. + public static NativeAlarmSourceOverrideCsvParseResult Parse(string csvText) + { + var rows = new List(); + var errors = new List(); + + // Split into physical lines; \r\n and \r are normalized to \n boundaries. + var lines = (csvText ?? string.Empty).Replace("\r\n", "\n").Replace('\r', '\n').Split('\n'); + + var headerSeen = false; + + for (var i = 0; i < lines.Length; i++) + { + var lineNumber = i + 1; + var rawLine = lines[i]; + + // Skip fully-blank lines (whitespace-only included) without error. + if (string.IsNullOrWhiteSpace(rawLine)) + continue; + + if (!CsvLineSplitter.TrySplit(rawLine, out var fields)) + { + errors.Add($"Line {lineNumber}: Unterminated quoted field."); + continue; + } + + if (!headerSeen) + { + if (!TryMatchHeader(fields)) + { + errors.Add(HeaderError); + return new NativeAlarmSourceOverrideCsvParseResult(rows, errors); + } + + headerSeen = true; + continue; + } + + if (fields.Count != ExpectedColumns) + { + errors.Add( + $"Line {lineNumber}: expected {ExpectedColumns} columns but found {fields.Count}."); + continue; + } + + var sourceName = fields[0]; + if (string.IsNullOrWhiteSpace(sourceName)) + { + errors.Add($"Line {lineNumber}: SourceName must not be blank."); + continue; + } + + rows.Add(new NativeAlarmSourceOverrideCsvRow( + sourceName, + NullIfEmpty(fields[1]), + NullIfEmpty(fields[2]), + NullIfEmpty(fields[3]), + lineNumber)); + } + + if (!headerSeen) + errors.Add(HeaderError); + + return new NativeAlarmSourceOverrideCsvParseResult(rows, errors); + } + + /// Matches the required 4-column header (case-insensitive). + private static bool TryMatchHeader(IReadOnlyList fields) => + fields.Count == ExpectedColumns && + HeaderEquals(fields[0], "SourceName") && + HeaderEquals(fields[1], "Connection") && + HeaderEquals(fields[2], "SourceReference") && + HeaderEquals(fields[3], "Filter"); + + private static bool HeaderEquals(string field, string expected) => + string.Equals(field, expected, StringComparison.OrdinalIgnoreCase); + + private static string? NullIfEmpty(string field) => field.Length == 0 ? null : field; +} diff --git a/src/ZB.MOM.WW.ScadaBridge.Commons/Types/OverrideCsvParser.cs b/src/ZB.MOM.WW.ScadaBridge.Commons/Types/OverrideCsvParser.cs index c76cdc78..c97bd183 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Commons/Types/OverrideCsvParser.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Commons/Types/OverrideCsvParser.cs @@ -60,7 +60,7 @@ public static class OverrideCsvParser if (string.IsNullOrWhiteSpace(rawLine)) continue; - if (!SplitFields(rawLine, out var fields)) + if (!CsvLineSplitter.TrySplit(rawLine, out var fields)) { errors.Add($"Line {lineNumber}: Unterminated quoted field."); continue; @@ -134,101 +134,4 @@ public static class OverrideCsvParser string.Equals(field, expected, StringComparison.OrdinalIgnoreCase); private static string? NullIfEmpty(string field) => field.Length == 0 ? null : field; - - /// - /// RFC-4180-ish field splitter for a single physical line. Quoting rules: - /// - /// A field is quoted only if its first non-whitespace char is a - /// " (leading whitespace before the opening quote is allowed and - /// ignored). Inside a quoted field, commas are literal and "" is an - /// escaped single quote; the closing " must be the last non-whitespace - /// char of the field (trailing whitespace after the close is allowed and - /// ignored). - /// A " appearing anywhere else in an unquoted field (i.e. after - /// non-whitespace content) is a literal character and is preserved. - /// Unquoted fields are whitespace-trimmed; quoted field values are kept - /// verbatim. - /// - /// Returns true with the split on success; - /// returns false when a quoted field is opened but never closed before - /// end-of-line (the caller emits a per-line "unterminated" error). - /// - private static bool SplitFields(string line, out List fields) - { - fields = new List(); - var field = new System.Text.StringBuilder(); - var inQuotes = false; // currently between an opening and closing quote - var quoted = false; // this field opened with a quote → keep value verbatim - var sawContent = false; // any non-whitespace char seen in the current field yet - - for (var i = 0; i < line.Length; i++) - { - var c = line[i]; - - if (inQuotes) - { - if (c == '"') - { - // Doubled quote inside a quoted field → a single literal quote. - if (i + 1 < line.Length && line[i + 1] == '"') - { - field.Append('"'); - i++; - } - else - { - inQuotes = false; // closing quote; only trailing whitespace may follow - } - } - else - { - field.Append(c); - } - - continue; - } - - switch (c) - { - case ',': - fields.Add(Finalize(field, quoted)); - field.Clear(); - inQuotes = false; - quoted = false; - sawContent = false; - break; - case '"' when !sawContent: - // Opening quote: first non-whitespace char of the field. Any - // leading whitespace seen so far is part of the (ignored) prefix. - field.Clear(); - inQuotes = true; - quoted = true; - sawContent = true; - break; - default: - // After a quoted field has closed, only whitespace may appear - // before the next delimiter — it is ignored, not appended. - if (quoted) - break; - - // A '"' here (sawContent already true) falls through as a literal. - if (!char.IsWhiteSpace(c)) - sawContent = true; - field.Append(c); - break; - } - } - - if (inQuotes) - return false; // opened a quoted field that was never closed - - fields.Add(Finalize(field, quoted)); - return true; - } - - private static string Finalize(System.Text.StringBuilder field, bool quoted) - { - var text = field.ToString(); - return quoted ? text : text.Trim(); // only unquoted whitespace is trimmed - } } diff --git a/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs b/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs index 6faca1d9..9f057d10 100644 --- a/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs @@ -270,6 +270,7 @@ public class ManagementActor : ReceiveActor or SetConnectionBindingsCommand or SetInstanceOverridesCommand or SetInstanceAreaCommand or SetInstanceAlarmOverrideCommand or DeleteInstanceAlarmOverrideCommand or SetInstanceNativeAlarmSourceOverrideCommand or DeleteInstanceNativeAlarmSourceOverrideCommand + or SetInstanceNativeAlarmSourceOverridesCommand or GetDeploymentDiffCommand or MgmtDeployArtifactsCommand or QueryDeploymentsCommand @@ -347,6 +348,7 @@ public class ManagementActor : ReceiveActor DeleteInstanceAlarmOverrideCommand cmd => await HandleDeleteInstanceAlarmOverride(sp, cmd, user), ListInstanceAlarmOverridesCommand cmd => await HandleListInstanceAlarmOverrides(sp, cmd, user), SetInstanceNativeAlarmSourceOverrideCommand cmd => await HandleSetInstanceNativeAlarmSourceOverride(sp, cmd, user), + SetInstanceNativeAlarmSourceOverridesCommand cmd => await HandleSetInstanceNativeAlarmSourceOverrides(sp, cmd, user), DeleteInstanceNativeAlarmSourceOverrideCommand cmd => await HandleDeleteInstanceNativeAlarmSourceOverride(sp, cmd, user), ListInstanceNativeAlarmSourceOverridesCommand cmd => await HandleListInstanceNativeAlarmSourceOverrides(sp, cmd, user), @@ -998,6 +1000,78 @@ public class ManagementActor : ReceiveActor return existing; } + private static async Task HandleSetInstanceNativeAlarmSourceOverrides( + IServiceProvider sp, SetInstanceNativeAlarmSourceOverridesCommand cmd, AuthenticatedUser user) + { + await EnforceSiteScopeForInstance(sp, user, cmd.InstanceId); + var repo = sp.GetRequiredService(); + + // All-or-nothing (parity with HandleSetInstanceOverrides). Flatten the + // instance ONCE (script compilation skipped — a lock/resolve check does not + // need it) and validate EVERY requested source up front: a source that does + // not resolve would create a dead override the flattener silently drops, and a + // template-locked source cannot be overridden. Only once the whole batch is + // known valid do we upsert — a mid-batch reject never leaves partial state. + var pipeline = sp.GetRequiredService(); + var flattenResult = await pipeline.FlattenAndValidateAsync( + cmd.InstanceId, default, validateScripts: false); + if (flattenResult.IsFailure) + throw new ManagementCommandException( + $"Cannot set native alarm source overrides: the instance could not be flattened ({flattenResult.Error})."); + + var resolvedByName = flattenResult.Value.Configuration.NativeAlarmSources + .ToDictionary(s => s.CanonicalName, StringComparer.Ordinal); + + // Reject a batch that names the same source twice — the intended override + // would be ambiguous, and the last-writer-wins upsert would silently discard + // the earlier row. + var seen = new HashSet(StringComparer.Ordinal); + foreach (var entry in cmd.Overrides) + { + if (!resolvedByName.TryGetValue(entry.SourceCanonicalName, out var resolvedSource)) + throw new ManagementCommandException( + $"Native alarm source '{entry.SourceCanonicalName}' does not resolve for this instance " + + "and cannot be overridden. No overrides were applied."); + if (resolvedSource.IsLocked) + throw new ManagementCommandException( + $"Native alarm source '{entry.SourceCanonicalName}' is locked at the template level and " + + "cannot be overridden. No overrides were applied."); + if (!seen.Add(entry.SourceCanonicalName)) + throw new ManagementCommandException( + $"Native alarm source '{entry.SourceCanonicalName}' appears more than once in the batch. " + + "No overrides were applied."); + } + + var results = new List(); + foreach (var entry in cmd.Overrides) + { + var existing = await repo.GetNativeAlarmSourceOverrideAsync(cmd.InstanceId, entry.SourceCanonicalName); + if (existing == null) + { + var ovr = new InstanceNativeAlarmSourceOverride(entry.SourceCanonicalName) + { + InstanceId = cmd.InstanceId, + ConnectionNameOverride = entry.ConnectionNameOverride, + SourceReferenceOverride = entry.SourceReferenceOverride, + ConditionFilterOverride = entry.ConditionFilterOverride + }; + await repo.AddInstanceNativeAlarmSourceOverrideAsync(ovr); + results.Add(ovr); + } + else + { + existing.ConnectionNameOverride = entry.ConnectionNameOverride; + existing.SourceReferenceOverride = entry.SourceReferenceOverride; + existing.ConditionFilterOverride = entry.ConditionFilterOverride; + await repo.UpdateInstanceNativeAlarmSourceOverrideAsync(existing); + results.Add(existing); + } + } + + await repo.SaveChangesAsync(); + return results; + } + private static async Task HandleDeleteInstanceNativeAlarmSourceOverride( IServiceProvider sp, DeleteInstanceNativeAlarmSourceOverrideCommand cmd, AuthenticatedUser user) { diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptRuntimeContext.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptRuntimeContext.cs index 554559d2..082ac684 100644 --- a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptRuntimeContext.cs +++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptRuntimeContext.cs @@ -403,7 +403,9 @@ public class ScriptRuntimeContext /// /// Quality-agnostic by default (spec §4.2): a value arriving at Bad /// quality still satisfies the wait — the match tests the value, not the quality. - /// A quality-gated ("Good"-only) mode is a planned enhancement, deferred per spec §4.2. + /// Opt into a quality-gated ("Good"-only) wait via the requireGoodQuality + /// argument (spec §4.2); the InstanceActor then holds the wait until the value + /// satisfies the test at Good quality (or times out). /// /// /// diff --git a/tests/ZB.MOM.WW.ScadaBridge.CLI.Tests/ImportNativeAlarmSourceOverridesTests.cs b/tests/ZB.MOM.WW.ScadaBridge.CLI.Tests/ImportNativeAlarmSourceOverridesTests.cs new file mode 100644 index 00000000..4213cc25 --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.CLI.Tests/ImportNativeAlarmSourceOverridesTests.cs @@ -0,0 +1,68 @@ +using ZB.MOM.WW.ScadaBridge.Commons.Messages.Management; +using ZB.MOM.WW.ScadaBridge.Commons.Types; + +namespace ZB.MOM.WW.ScadaBridge.CLI.Tests; + +/// +/// Verifies the CSV parsing + entry-mapping behaviour that backs +/// instance native-alarm-source import: parse errors block the apply step; +/// valid CSV produces the list the +/// consumes. +/// +public class ImportNativeAlarmSourceOverridesTests +{ + // ── error path ─────────────────────────────────────────────────────────── + + [Fact] + public void CsvWithMissingHeader_ProducesErrors() + { + var result = NativeAlarmSourceOverrideCsvParser.Parse("Pressure,ConnA,ref,Active"); + + Assert.NotEmpty(result.Errors); + Assert.Empty(result.Rows); + } + + [Fact] + public void CsvWithBlankSourceName_ProducesError() + { + const string csv = "SourceName,Connection,SourceReference,Filter\n,ConnA,ref,Active"; + var result = NativeAlarmSourceOverrideCsvParser.Parse(csv); + + Assert.NotEmpty(result.Errors); + Assert.Empty(result.Rows); + } + + // ── happy path ─────────────────────────────────────────────────────────── + + [Fact] + public void ValidCsv_ProducesCorrectEntryList() + { + const string csv = """ + SourceName,Connection,SourceReference,Filter + Pressure,ConnA,ns=2;s=Pump,Active + Temp,,, + """; + + var parseResult = NativeAlarmSourceOverrideCsvParser.Parse(csv); + + Assert.Empty(parseResult.Errors); + Assert.Equal(2, parseResult.Rows.Count); + + // Build the same entry list that the CLI import action produces. + var entries = parseResult.Rows + .Select(r => new NativeAlarmSourceOverrideEntry( + r.SourceName, r.Connection, r.SourceReference, r.Filter)) + .ToList(); + + Assert.Equal("Pressure", entries[0].SourceCanonicalName); + Assert.Equal("ConnA", entries[0].ConnectionNameOverride); + Assert.Equal("ns=2;s=Pump", entries[0].SourceReferenceOverride); + Assert.Equal("Active", entries[0].ConditionFilterOverride); + + // All-blank override fields → inherited (null) on every field. + Assert.Equal("Temp", entries[1].SourceCanonicalName); + Assert.Null(entries[1].ConnectionNameOverride); + Assert.Null(entries[1].SourceReferenceOverride); + Assert.Null(entries[1].ConditionFilterOverride); + } +} diff --git a/tests/ZB.MOM.WW.ScadaBridge.Commons.Tests/NativeAlarmSourceOverrideCsvParserTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Commons.Tests/NativeAlarmSourceOverrideCsvParserTests.cs new file mode 100644 index 00000000..e7d1e0f5 --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.Commons.Tests/NativeAlarmSourceOverrideCsvParserTests.cs @@ -0,0 +1,148 @@ +using ZB.MOM.WW.ScadaBridge.Commons.Types; + +namespace ZB.MOM.WW.ScadaBridge.Commons.Tests; + +public class NativeAlarmSourceOverrideCsvParserTests +{ + [Fact] + public void Parse_SimpleFile_ReturnsRowsNoErrors() + { + const string csv = + "SourceName,Connection,SourceReference,Filter\n" + + "Pressure,ConnA,ns=2;s=Pump.Alarm,Active\n" + + "Module.Temp,ConnB,ns=2;s=Temp,\n"; + + var result = NativeAlarmSourceOverrideCsvParser.Parse(csv); + + Assert.Empty(result.Errors); + Assert.Equal(2, result.Rows.Count); + + Assert.Equal("Pressure", result.Rows[0].SourceName); + Assert.Equal("ConnA", result.Rows[0].Connection); + Assert.Equal("ns=2;s=Pump.Alarm", result.Rows[0].SourceReference); + Assert.Equal("Active", result.Rows[0].Filter); + Assert.Equal(2, result.Rows[0].LineNumber); + + Assert.Equal("Module.Temp", result.Rows[1].SourceName); + Assert.Equal("ConnB", result.Rows[1].Connection); + Assert.Equal("ns=2;s=Temp", result.Rows[1].SourceReference); + Assert.Null(result.Rows[1].Filter); + Assert.Equal(3, result.Rows[1].LineNumber); + } + + [Fact] + public void Parse_BlankOverrideFields_BecomeNullInherited() + { + const string csv = + "SourceName,Connection,SourceReference,Filter\n" + + "Pressure,,,\n"; + + var result = NativeAlarmSourceOverrideCsvParser.Parse(csv); + + Assert.Empty(result.Errors); + var row = Assert.Single(result.Rows); + Assert.Equal("Pressure", row.SourceName); + Assert.Null(row.Connection); + Assert.Null(row.SourceReference); + Assert.Null(row.Filter); + } + + [Fact] + public void Parse_HeaderCaseInsensitive_Accepted() + { + const string csv = + "sourcename,CONNECTION,SourceReference,filter\n" + + "Pressure,ConnA,ref,\n"; + + var result = NativeAlarmSourceOverrideCsvParser.Parse(csv); + + Assert.Empty(result.Errors); + Assert.Single(result.Rows); + } + + [Fact] + public void Parse_QuotedFieldWithComma_PreservesEmbeddedComma() + { + const string csv = + "SourceName,Connection,SourceReference,Filter\n" + + "Pressure,ConnA,\"ns=2;s=a,b,c\",Active\n"; + + var result = NativeAlarmSourceOverrideCsvParser.Parse(csv); + + Assert.Empty(result.Errors); + var row = Assert.Single(result.Rows); + Assert.Equal("ns=2;s=a,b,c", row.SourceReference); + } + + [Fact] + public void Parse_MissingHeader_ReturnsHeaderError() + { + const string csv = "Pressure,ConnA,ref,Active\n"; + + var result = NativeAlarmSourceOverrideCsvParser.Parse(csv); + + Assert.Empty(result.Rows); + var error = Assert.Single(result.Errors); + Assert.Contains("SourceName,Connection,SourceReference,Filter", error); + } + + [Fact] + public void Parse_WrongColumnCount_ReportsPerLineErrorAndSkipsRow() + { + const string csv = + "SourceName,Connection,SourceReference,Filter\n" + + "Pressure,ConnA\n" + + "Temp,ConnB,ref,Active\n"; + + var result = NativeAlarmSourceOverrideCsvParser.Parse(csv); + + var error = Assert.Single(result.Errors); + Assert.Contains("Line 2", error); + Assert.Contains("expected 4 columns", error); + // Valid rows still flow through. + var row = Assert.Single(result.Rows); + Assert.Equal("Temp", row.SourceName); + } + + [Fact] + public void Parse_BlankSourceName_ReportsError() + { + const string csv = + "SourceName,Connection,SourceReference,Filter\n" + + ",ConnA,ref,Active\n"; + + var result = NativeAlarmSourceOverrideCsvParser.Parse(csv); + + Assert.Empty(result.Rows); + var error = Assert.Single(result.Errors); + Assert.Contains("SourceName must not be blank", error); + } + + [Fact] + public void Parse_UnterminatedQuote_ReportsError() + { + const string csv = + "SourceName,Connection,SourceReference,Filter\n" + + "Pressure,ConnA,\"ns=2;s=open,Active\n"; + + var result = NativeAlarmSourceOverrideCsvParser.Parse(csv); + + Assert.Empty(result.Rows); + Assert.Contains(result.Errors, e => e.Contains("Unterminated quoted field")); + } + + [Fact] + public void Parse_BlankLinesSkippedWithoutError() + { + const string csv = + "SourceName,Connection,SourceReference,Filter\n" + + "\n" + + "Pressure,ConnA,ref,Active\n" + + " \n"; + + var result = NativeAlarmSourceOverrideCsvParser.Parse(csv); + + Assert.Empty(result.Errors); + Assert.Single(result.Rows); + } +} diff --git a/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ManagementActorTests.cs b/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ManagementActorTests.cs index ea1b4d0a..afde74b3 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ManagementActorTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ManagementActorTests.cs @@ -2874,6 +2874,112 @@ public class ManagementActorTests : TestKit, IDisposable Assert.Contains("Deployer", response.Message); } + [Fact] + public void SetInstanceNativeAlarmSourceOverrides_BulkHappyPath_AddsAllAndSavesOnce() + { + // #12: bulk CSV apply. Both sources resolve and are unlocked → each is added, + // and the whole batch commits with a SINGLE SaveChanges (all-or-nothing txn). + var pipeline = Substitute.For(); + var flattened = new FlattenedConfiguration + { + NativeAlarmSources = new List + { + new() { CanonicalName = "Pressure", IsLocked = false, Source = "Template" }, + new() { CanonicalName = "Temp", IsLocked = false, Source = "Template" } + } + }; + pipeline.FlattenAndValidateAsync(1, Arg.Any(), Arg.Any()) + .Returns(Result.Success( + new FlatteningPipelineResult(flattened, "hash", ValidationResult.Success()))); + _services.AddScoped(_ => pipeline); + + var actor = CreateActor(); + var envelope = Envelope( + new SetInstanceNativeAlarmSourceOverridesCommand(1, new List + { + new("Pressure", "Opc2", "ns=2;s=P", null), + new("Temp", null, "ns=2;s=T", "Active") + }), + "Deployer"); + + actor.Tell(envelope); + + ExpectMsg(TimeSpan.FromSeconds(5)); + _templateRepo.ReceivedWithAnyArgs(2).AddInstanceNativeAlarmSourceOverrideAsync(default!, default); + // All-or-nothing: exactly one commit for the whole batch. + _templateRepo.ReceivedWithAnyArgs(1).SaveChangesAsync(default); + } + + [Fact] + public void SetInstanceNativeAlarmSourceOverrides_OneLockedSource_RejectsWholeBatch() + { + // #12: all-or-nothing. If ANY entry targets a locked source, the whole batch + // is rejected before any write — no Add, no SaveChanges. + var pipeline = Substitute.For(); + var flattened = new FlattenedConfiguration + { + NativeAlarmSources = new List + { + new() { CanonicalName = "Pressure", IsLocked = false, Source = "Template" }, + new() { CanonicalName = "Locked", IsLocked = true, Source = "Template" } + } + }; + pipeline.FlattenAndValidateAsync(1, Arg.Any(), Arg.Any()) + .Returns(Result.Success( + new FlatteningPipelineResult(flattened, "hash", ValidationResult.Success()))); + _services.AddScoped(_ => pipeline); + + var actor = CreateActor(); + var envelope = Envelope( + new SetInstanceNativeAlarmSourceOverridesCommand(1, new List + { + new("Pressure", "Opc2", null, null), + new("Locked", "Opc2", null, null) + }), + "Deployer"); + + actor.Tell(envelope); + + var response = ExpectMsg(TimeSpan.FromSeconds(5)); + Assert.Contains("locked", response.Error, StringComparison.OrdinalIgnoreCase); + _templateRepo.DidNotReceiveWithAnyArgs().AddInstanceNativeAlarmSourceOverrideAsync(default!, default); + _templateRepo.DidNotReceiveWithAnyArgs().SaveChangesAsync(default); + } + + [Fact] + public void SetInstanceNativeAlarmSourceOverrides_UnresolvedSource_RejectsWholeBatch() + { + // #12: a source that does not resolve for the instance is rejected up front. + var pipeline = Substitute.For(); + var flattened = new FlattenedConfiguration + { + NativeAlarmSources = new List + { + new() { CanonicalName = "Pressure", IsLocked = false, Source = "Template" } + } + }; + pipeline.FlattenAndValidateAsync(1, Arg.Any(), Arg.Any()) + .Returns(Result.Success( + new FlatteningPipelineResult(flattened, "hash", ValidationResult.Success()))); + _services.AddScoped(_ => pipeline); + + var actor = CreateActor(); + var envelope = Envelope( + new SetInstanceNativeAlarmSourceOverridesCommand(1, new List + { + new("Pressure", "Opc2", null, null), + new("Ghost", "Opc2", null, null) + }), + "Deployer"); + + actor.Tell(envelope); + + var response = ExpectMsg(TimeSpan.FromSeconds(5)); + Assert.Contains("does not resolve", response.Error, StringComparison.OrdinalIgnoreCase); + _templateRepo.DidNotReceiveWithAnyArgs().AddInstanceNativeAlarmSourceOverrideAsync(default!, default); + _templateRepo.DidNotReceiveWithAnyArgs().SaveChangesAsync(default); + } + // ======================================================================== // MoveDataConnectionCommand (M9 / T24a) — guarded cross-site move // ======================================================================== diff --git a/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/RequiredRoleMatrixTests.cs b/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/RequiredRoleMatrixTests.cs index ba2305c4..efc0d6bd 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/RequiredRoleMatrixTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/RequiredRoleMatrixTests.cs @@ -124,6 +124,7 @@ public class RequiredRoleMatrixTests ["SetInstanceAlarmOverride"] = [Roles.Deployer], ["DeleteInstanceAlarmOverride"] = [Roles.Deployer], ["SetInstanceNativeAlarmSourceOverride"] = [Roles.Deployer], + ["SetInstanceNativeAlarmSourceOverrides"] = [Roles.Deployer], ["DeleteInstanceNativeAlarmSourceOverride"] = [Roles.Deployer], ["GetDeploymentDiff"] = [Roles.Deployer], ["MgmtDeployArtifacts"] = [Roles.Deployer],