From c60347c5e74e64e8e31eff2bbe70c36885a6a651 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Fri, 10 Jul 2026 02:12:26 -0400 Subject: [PATCH] =?UTF-8?q?fix(transport):=20blocker=20heuristic=20no=20lo?= =?UTF-8?q?nger=20hard-blocks=20valid=20imports=20=E2=80=94=20local=20decl?= =?UTF-8?q?arations=20excluded,=20denylist=20extended,=20template-script?= =?UTF-8?q?=20findings=20downgraded=20to=20warnings?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #05-T19. Import name-resolution findings are now split by origin: template-script (and attribute-default-expression) references become non-blocking advisory warnings (ConflictKind.Warning + ImportResult.Warnings) since the design-time deploy gate re-validates authoritatively; ApiMethod-script references stay hard errors (SemanticValidationException / ConflictKind.Blocker) as they have no downstream gate. Names declared locally in a body (Roslyn-parsed local functions / methods) are excluded so a script's own helper isn't flagged; KnownNonReferenceNames extended with common BCL/LINQ/SQL surface. DetectBlockersAsync and RunSemanticValidationAsync Pass 1 apply the identical split so preview and apply agree. Rollback/hard-fail tests retargeted to ApiMethods (the still-hard-failing vehicle). CentralUI preview renders the new Warning badge/advisory. Claude-Session: https://claude.ai/code/session_01MtdgwpEeCUn6cUA5f1LMPj --- docs/requirements/Component-Transport.md | 9 +- .../Pages/Design/TransportImport.razor | 16 ++ .../Types/Transport/ImportPreview.cs | 8 +- .../Types/Transport/ImportResult.cs | 13 +- .../Import/BundleImporter.cs | 168 ++++++++++++++---- .../Import/BundleImporterApplyTests.cs | 41 +++-- .../Import/BundleImporterPreviewTests.cs | 80 ++++++--- .../BundleImporterRollbackFailureTests.cs | 21 +-- .../SemanticValidatorImportTests.cs | 154 ++++++++++++++-- .../ValidationFailureTests.cs | 37 ++-- 10 files changed, 426 insertions(+), 121 deletions(-) diff --git a/docs/requirements/Component-Transport.md b/docs/requirements/Component-Transport.md index 50ac3e52..6f9080a5 100644 --- a/docs/requirements/Component-Transport.md +++ b/docs/requirements/Component-Transport.md @@ -25,7 +25,7 @@ As of M8 (T18), Transport is no longer limited to central-only configuration: it - Encrypt content with AES-256-GCM + PBKDF2-SHA256 (600 000 iterations) when a passphrase is supplied; leave content plaintext with a UI warning and an `UnencryptedBundleExport` audit event when none is given. - Validate `manifest.json` on upload: format version gating, SHA-256 content hash verification. - Manage in-memory `BundleSession` objects: 30-minute TTL, 3-strike passphrase lockout per session. -- Compute a per-artifact diff between bundle contents and the target environment, classifying each artifact as Identical, Modified, New, or a Blocker. +- Compute a per-artifact diff between bundle contents and the target environment, classifying each artifact as Identical, Modified, New, a Blocker (import-stopping), or a Warning (advisory, non-blocking). - Apply user-supplied conflict resolutions (Add, Overwrite, Skip, Rename) in a single EF transaction, running two-tier semantic validation before committing: a minimal name-resolution scan over the merged target (fails fast on unresolved SharedScript / ExternalSystem identifiers), then the full `SemanticValidator` from `ZB.MOM.WW.ScadaBridge.TemplateEngine` over each imported template's per-template `FlattenedConfiguration`. - Emit `BundleExported`, `BundleImported`, `BundleImportFailed`, `UnencryptedBundleExport`, `BundleImportUnlockFailed`, `BundleImportAlarmScriptUnresolved`, `BundleImportCompositionUnresolved`, and `BundleImportBaseTemplateUnresolved` audit events via `IAuditService`. - Thread a `BundleImportId` correlation GUID through every per-entity `AuditLogEntry` written during `ApplyAsync` via a scoped `IAuditCorrelationContext`. @@ -227,7 +227,9 @@ Bulk "Apply to all" at the top (Overwrite / Skip / Rename), overridable per row. Bundle references that cannot be satisfied in either the bundle or the target DB (e.g., a template references a shared script that is neither in the bundle nor pre-existing) appear as **blocker rows** — Apply is disabled until they are resolved (typically by skipping the dependent artifact or re-exporting with dependencies). Unmapped/unmatched sites and connections that the operator did not set to Create-new also surface as blocker rows here, mirroring the Step-3 Map decisions. -**Blocker-scan heuristic boundaries.** The scanner walks `TemplateScript.Code`, `TemplateAttribute.Value`, and `ApiMethod.Script` looking for top-level `Identifier(` or `Identifier.` tokens. To keep the heuristic usable on real script bodies it (a) skips identifiers preceded by `.` (member access — `obj.Method()` does not flag `Method`); (b) does NOT scan `TemplateAttribute.DataSourceReference` (an OPC UA address path, never script source); and (c) filters out a small `KnownNonReferenceNames` denylist of .NET stdlib types (`Convert`, `DateTimeOffset`, `ToString`, `Dispose`, `UtcNow`, …), ScadaBridge runtime API roots (`Notify`, `Database`, `ExternalSystem`, `Scripts`, `Instance`, `Parameters`, `Attributes`, `Route`, …), and common SQL keywords that appear inside string literals (`COUNT`, `SELECT`, `FROM`, …). Both the diff-step `DetectBlockersAsync` and the Apply-time `RunSemanticValidationAsync` Pass 1 share this filter, so the diff preview and the Apply gate agree. +**Blocker-scan heuristic boundaries.** The scanner walks `TemplateScript.Code`, `TemplateAttribute.Value`, and `ApiMethod.Script` looking for top-level `Identifier(` or `Identifier.` tokens. To keep the heuristic usable on real script bodies it (a) skips identifiers preceded by `.` (member access — `obj.Method()` does not flag `Method`); (b) does NOT scan `TemplateAttribute.DataSourceReference` (an OPC UA address path, never script source); (c) excludes names **declared locally in the body** — every `LocalFunctionStatement` / `MethodDeclaration` identifier is collected via a Roslyn script-kind parse, so a script's own helper (`decimal ComputeRate(...) => …; return ComputeRate(x);`) is not mistaken for a missing SharedScript; and (d) filters out a `KnownNonReferenceNames` denylist of .NET stdlib types (`Convert`, `DateTimeOffset`, `Regex`, `StringBuilder`, `Parse`, LINQ operators, …), ScadaBridge runtime API roots (`Notify`, `Database`, `ExternalSystem`, `Scripts`, `Instance`, `Parameters`, `Attributes`, `Route`, …), and common SQL keywords that appear inside string literals (`COUNT`, `SELECT`, `FROM`, `HAVING`, `VALUES`, …). Both the diff-step `DetectBlockersAsync` and the Apply-time `RunSemanticValidationAsync` Pass 1 share this filter, so the diff preview and the Apply gate agree. + +**Severity split — template-script findings are advisory, ApiMethod findings block.** A residual unresolved identifier (one the heuristic could not filter) is classified by its **origin**. Findings from **template scripts** and **attribute default expressions** are non-blocking **warnings**: they surface as `ConflictKind.Warning` rows in the preview and ride on `ImportResult.Warnings` after a successful apply — the import proceeds because the authoritative design-time **deploy gate re-validates** the script with a real compile (`ScriptCompiler.TryCompile`), so a heuristic false positive here must never stop a valid import. Findings from **ApiMethod scripts** remain **hard errors** (`ConflictKind.Blocker` in preview; `SemanticValidationException` at apply) because inbound API methods have no downstream design-time gate — the import review is their only pre-runtime check. The split is applied identically in `DetectBlockersAsync` and `RunSemanticValidationAsync` Pass 1 so preview and apply agree. **Step 5 — Confirm.** Final summary plus a "N instances will become stale" warning enumerating affected instances (a real count as of M8 — see "Stale-Instance Signaling"). User types the source environment name to confirm (typo-resistant gate at the prod boundary). @@ -307,7 +309,8 @@ As of M8 the import result enumerates affected instances for real (closing the p | Unlock | Wrong passphrase | Step 2 error; 3rd wrong attempt invalidates session, audit `BundleImportUnlockFailed` | | Map | Source site/connection neither auto-matched nor mapped/created | Listed as a blocker row in the Map step (UI) / Diff step; cannot Apply until mapped or set to Create-new. CLI aborts before apply unless `--create-missing-sites` / `--create-missing-connections` is given | | Map | `CreateNew` selected but the bundle does not carry that site/connection's config | Blocker — Create-new requires the full config to be present in the bundle payload | -| Preview | Bundle references shared script not in bundle and not in target DB | Listed as a blocker row in the Diff step; cannot Apply until resolved | +| Preview | ApiMethod script references shared script / external system not in bundle and not in target DB | Listed as a **blocker** row in the Diff step; cannot Apply until resolved (no downstream deploy gate for ApiMethods) | +| Preview | Template script references shared script / external system not in bundle and not in target DB | Listed as an advisory **warning** row; import proceeds — the design-time deploy gate re-validates the script authoritatively | | Apply | Semantic validation fails (call target type mismatch, etc.) | Modal: "Validation failed — N errors", per-error list, no DB writes | | Apply | DB transaction fails | Rollback; full transactional guarantee — nothing partial lands; `BundleImportFailed` audit row written outside the rolled-back transaction | | Session | TTL expired | Diff step onward, refresh prompts re-upload | diff --git a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Design/TransportImport.razor b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Design/TransportImport.razor index 8e875109..2921c128 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Design/TransportImport.razor +++ b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Design/TransportImport.razor @@ -275,6 +275,14 @@ } + @if (item.Kind == ConflictKind.Warning && !string.IsNullOrEmpty(item.BlockerReason)) + { + + +
@item.BlockerReason
+ + + } } @@ -421,6 +429,7 @@ ConflictKind.Modified => ("bg-warning text-dark", "Modified"), ConflictKind.New => ("bg-success", "New"), ConflictKind.Blocker => ("bg-danger", "Blocker"), + ConflictKind.Warning => ("bg-warning text-dark", "Warning"), _ => ("bg-light text-dark", item.Kind.ToString()), }; @label @@ -444,6 +453,13 @@ — return; } + // Warning items are advisory "Reference" rows, not importable entities — + // no resolution to choose (the import proceeds regardless). + if (item.Kind == ConflictKind.Warning) + { + — + return; + } var key = (item.EntityType, item.Name); diff --git a/src/ZB.MOM.WW.ScadaBridge.Commons/Types/Transport/ImportPreview.cs b/src/ZB.MOM.WW.ScadaBridge.Commons/Types/Transport/ImportPreview.cs index eeeb33fa..f17b34e6 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Commons/Types/Transport/ImportPreview.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Commons/Types/Transport/ImportPreview.cs @@ -1,6 +1,12 @@ namespace ZB.MOM.WW.ScadaBridge.Commons.Types.Transport; -public enum ConflictKind { Identical, Modified, New, Blocker } +// Warning is a NON-blocking advisory finding (appended after Blocker so the +// enum ordinal of the existing members is unchanged). Import may proceed with +// Warning items present; only Blocker items stop an apply. Template-script +// name-resolution findings surface as Warning because the design-time deploy +// gate (ScriptCompiler.TryCompile) re-validates them authoritatively; ApiMethod +// findings — which have no downstream gate — stay Blocker. +public enum ConflictKind { Identical, Modified, New, Blocker, Warning } public sealed record ImportPreviewItem( string EntityType, diff --git a/src/ZB.MOM.WW.ScadaBridge.Commons/Types/Transport/ImportResult.cs b/src/ZB.MOM.WW.ScadaBridge.Commons/Types/Transport/ImportResult.cs index 90c5366f..e2efa98d 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Commons/Types/Transport/ImportResult.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Commons/Types/Transport/ImportResult.cs @@ -11,4 +11,15 @@ public sealed record ImportResult( // Number of legacy inbound API keys found in the bundle that were ignored // (keys are not transported; re-create them on this environment). // Defaults to 0 so existing positional construction sites stay source-compatible. - int ApiKeysIgnored = 0); + int ApiKeysIgnored = 0, + // Advisory (non-blocking) import findings surfaced during apply — currently + // template-script name-resolution references that didn't resolve to a bundled + // or target SharedScript / ExternalSystem. These are warnings, NOT errors: + // the deploy-time gate re-validates them authoritatively. ApiMethod-script + // findings are hard errors instead and surface via SemanticValidationException. + // Defaulted null → normalized to empty so existing positional callers compile. + IReadOnlyList? Warnings = null) +{ + /// Advisory, non-blocking import findings. Never null. + public IReadOnlyList Warnings { get; init; } = Warnings ?? Array.Empty(); +} diff --git a/src/ZB.MOM.WW.ScadaBridge.Transport/Import/BundleImporter.cs b/src/ZB.MOM.WW.ScadaBridge.Transport/Import/BundleImporter.cs index 4b864a0c..0ba96f57 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Transport/Import/BundleImporter.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Transport/Import/BundleImporter.cs @@ -1,6 +1,9 @@ using System.IO.Compression; using System.Security.Cryptography; using System.Text.Json; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; @@ -779,10 +782,22 @@ public sealed class BundleImporter : IBundleImporter // Name is a valid identifier, if Name appears in NEITHER set, surface // it as a Blocker. This catches the documented use-case // (HelperFn() / ErpSystem.Call()) without combinatorial blowup. - var referencedFromBundle = new HashSet(StringComparer.Ordinal); + // #05-T19 severity split: track candidate references by ORIGIN. Template- + // script (and attribute-default-expression) references are advisory + // warnings — the design-time deploy gate re-validates them; ApiMethod + // references are hard blockers (no downstream gate). Local-function / + // method declarations in a body are excluded so a script's own helper + // isn't mistaken for a missing SharedScript. + var referencedFromTemplates = new HashSet(StringComparer.Ordinal); + var referencedFromApiMethods = new HashSet(StringComparer.Ordinal); + var locallyDeclared = new HashSet(StringComparer.Ordinal); foreach (var t in content.Templates) { - foreach (var s in t.Scripts) CollectCallIdentifiers(s.Code, referencedFromBundle); + foreach (var s in t.Scripts) + { + CollectCallIdentifiers(s.Code, referencedFromTemplates); + CollectLocalDeclarations(s.Code, locallyDeclared); + } foreach (var a in t.Attributes) { // Attribute.Value carries the design-time default expression, which @@ -791,37 +806,44 @@ public sealed class BundleImporter : IBundleImporter // it's never script source and must NOT be scanned, or the dot // delimiter trips the heuristic into flagging the address segments // as missing SharedScript/ExternalSystem references. - CollectCallIdentifiers(a.Value, referencedFromBundle); + CollectCallIdentifiers(a.Value, referencedFromTemplates); + CollectLocalDeclarations(a.Value, locallyDeclared); } } foreach (var m in content.ApiMethods) { - CollectCallIdentifiers(m.Script, referencedFromBundle); + CollectCallIdentifiers(m.Script, referencedFromApiMethods); + CollectLocalDeclarations(m.Script, locallyDeclared); } - // For each candidate, only report it as a blocker if it looks like a - // resource reference (PascalCase, length > 1), isn't a well-known - // language / runtime / SQL token, AND isn't present anywhere we can - // satisfy it. The denylist is the noise filter that keeps the - // heuristic usable on real script bodies — without it, every member - // access (`obj.ToString()`) and stdlib type (`DateTimeOffset`) gets - // flagged. - foreach (var candidate in referencedFromBundle.OrderBy(n => n, StringComparer.Ordinal)) + // For each candidate, only report it if it looks like a resource + // reference (PascalCase, length > 1), isn't a well-known language / + // runtime / SQL token, isn't the body's own local declaration, AND + // isn't present anywhere we can satisfy it. A candidate that appears in + // an ApiMethod script is a hard Blocker; a template-only candidate is a + // non-blocking Warning. + var allCandidates = new HashSet(referencedFromTemplates, StringComparer.Ordinal); + allCandidates.UnionWith(referencedFromApiMethods); + foreach (var candidate in allCandidates.OrderBy(n => n, StringComparer.Ordinal)) { if (!LooksLikeResourceName(candidate)) continue; if (KnownNonReferenceNames.Contains(candidate)) continue; + if (locallyDeclared.Contains(candidate)) continue; var isShared = sharedScriptNames.Contains(candidate); var isExternal = externalSystemNames.Contains(candidate); if (isShared || isExternal) continue; + var fromApiMethod = referencedFromApiMethods.Contains(candidate); blockers.Add(new ImportPreviewItem( EntityType: "Reference", Name: candidate, ExistingVersion: null, IncomingVersion: null, - Kind: ConflictKind.Blocker, + Kind: fromApiMethod ? ConflictKind.Blocker : ConflictKind.Warning, FieldDiffJson: null, - BlockerReason: $"References SharedScript or ExternalSystem '{candidate}' not present in bundle or target.")); + BlockerReason: fromApiMethod + ? $"References SharedScript or ExternalSystem '{candidate}' not present in bundle or target." + : $"Script references SharedScript or ExternalSystem '{candidate}' not present in bundle or target — advisory; the deploy-time gate re-validates.")); } return blockers; @@ -850,6 +872,40 @@ public sealed class BundleImporter : IBundleImporter } } + /// + /// Collects the names of every local function / method DECLARED inside a + /// script body, so the reference heuristic doesn't mistake a script's own + /// helper (decimal ComputeRate(...) => ...; return ComputeRate(x);) + /// for a missing SharedScript / ExternalSystem. Uses a Roslyn script-kind + /// parse (partial trees are fine — a body with syntax errors still yields + /// the declarations Roslyn could recover). Purely additive noise-suppression: + /// on any parse failure the sink is simply left unchanged. + /// + private static void CollectLocalDeclarations(string? body, HashSet sink) + { + if (string.IsNullOrEmpty(body)) return; + try + { + var root = CSharpSyntaxTree + .ParseText(body, new CSharpParseOptions(kind: SourceCodeKind.Script)) + .GetRoot(); + foreach (var fn in root.DescendantNodes().OfType()) + { + sink.Add(fn.Identifier.ValueText); + } + foreach (var m in root.DescendantNodes().OfType()) + { + sink.Add(m.Identifier.ValueText); + } + } + catch + { + // Best-effort: a parse failure just means no declarations are + // excluded for this body — the finding (at worst) surfaces as an + // advisory warning, never a hard error for template scripts. + } + } + /// /// Names that look like PascalCase references but are never user-defined /// SharedScripts or ExternalSystems. Filters the false-positive noise the @@ -877,7 +933,20 @@ public sealed class BundleImporter : IBundleImporter // SQL keywords commonly seen inside string literals "COUNT", "FROM", "GROUP", "INSERT", "JOIN", "ORDER", "SELECT", - "UPDATE", "WHERE", + "UPDATE", "WHERE", "HAVING", "VALUES", "DELETE", "DISTINCT", "LIMIT", + + // #05-T19 — extended stdlib / BCL surface commonly reached from scripts. + // The list will still drift as scripts use more of the BCL — that is + // precisely why template-script findings are downgraded to warnings + // (the deploy-time gate re-validates authoritatively). + "Regex", "Match", "Matches", "IsMatch", "Replace", "Split", + "StringBuilder", "Append", "AppendLine", "Parse", "TryParse", + "Format", "Join", "Abs", "Round", "Min", "Max", "Floor", "Ceiling", + "Pow", "Sqrt", "Json", "Serialize", "Deserialize", "Where", "Select", + "First", "FirstOrDefault", "Any", "All", "Count", "Sum", "Average", + "OrderBy", "OrderByDescending", "GroupBy", "Distinct", "Add", "Remove", + "Clear", "Contains", "ContainsKey", "TryGetValue", "StartsWith", + "EndsWith", "Substring", "Trim", "ToUpper", "ToLower", }; private static bool LooksLikeResourceName(string name) @@ -982,7 +1051,8 @@ public sealed class BundleImporter : IBundleImporter // Skip-resolved DTOs are excluded from the in-bundle name set so // a Skip on a dependency surfaces as a missing-reference error // rather than silently passing. - var validationErrors = await RunSemanticValidationAsync(content, resolutionMap, ct).ConfigureAwait(false); + var (validationErrors, validationWarnings) = + await RunSemanticValidationAsync(content, resolutionMap, ct).ConfigureAwait(false); // Validate every site / connection / template reference the // site-instance payload depends on BEFORE any row is staged. Running // this in the validation phase (not as an apply-pass guard) preserves @@ -999,6 +1069,17 @@ public sealed class BundleImporter : IBundleImporter throw new SemanticValidationException(validationErrors); } + // Advisory template-script reference findings never block the import; + // log them and surface them on the ImportResult so the operator sees + // the same advisory the preview showed. The deploy-time gate is the + // authoritative re-validation. + if (validationWarnings.Count > 0) + { + _logger?.LogWarning( + "Bundle import {BundleImportId}: {Count} advisory template-script reference warning(s): {Warnings}", + bundleImportId, validationWarnings.Count, string.Join("; ", validationWarnings)); + } + // ---- Site/instance-scoped apply: sites + connections FIRST ---- // Sites are the FK target for both data connections (SiteId) and // instances (SiteId); data connections are the FK target for instance @@ -1152,7 +1233,8 @@ public sealed class BundleImporter : IBundleImporter Renamed: summary.Renamed, StaleInstanceIds: staleInstanceIds, AuditEventCorrelation: bundleImportId.ToString(), - ApiKeysIgnored: apiKeysIgnored); + ApiKeysIgnored: apiKeysIgnored, + Warnings: validationWarnings); } catch (Exception ex) { @@ -4086,12 +4168,13 @@ public sealed class BundleImporter : IBundleImporter /// operator isn't trying to import) would block the import. /// /// - private async Task> RunSemanticValidationAsync( + private async Task<(IReadOnlyList Errors, IReadOnlyList Warnings)> RunSemanticValidationAsync( BundleContentDto content, Dictionary<(string, string), ImportResolution> resolutionMap, CancellationToken ct) { var errors = new List(); + var warnings = new List(); // ---- Pass 1: minimal name-resolution scan ---- @@ -4135,17 +4218,25 @@ public sealed class BundleImporter : IBundleImporter } // Collect every identifier-shaped call target from the bundle's - // templates + api methods. We only check the bundle's bodies here - // (matching PreviewAsync's blocker scan); pre-existing target rows are - // assumed already validated when they were originally written. - var referenced = new HashSet(StringComparer.Ordinal); + // templates + api methods, keyed by ORIGIN (#05-T19 severity split). + // We only check the bundle's bodies here (matching PreviewAsync's blocker + // scan); pre-existing target rows are assumed already validated when they + // were originally written. Local-function / method declarations in a body + // are collected too so a script's own helper isn't flagged as missing. + var referencedFromTemplates = new HashSet(StringComparer.Ordinal); + var referencedFromApiMethods = new HashSet(StringComparer.Ordinal); + var locallyDeclared = new HashSet(StringComparer.Ordinal); foreach (var t in content.Templates) { // Skip-resolved templates aren't being written, so their script // references don't need to resolve. var resolution = ResolveOrDefault(resolutionMap, "Template", t.Name); if (resolution.Action == ResolutionAction.Skip) continue; - foreach (var s in t.Scripts) CollectCallIdentifiers(s.Code, referenced); + foreach (var s in t.Scripts) + { + CollectCallIdentifiers(s.Code, referencedFromTemplates); + CollectLocalDeclarations(s.Code, locallyDeclared); + } foreach (var a in t.Attributes) { // Value can hold script-callable design-time expressions; @@ -4153,29 +4244,46 @@ public sealed class BundleImporter : IBundleImporter // "ns=3;s=Tank.Level") and must NOT be scanned, or the dot // delimiter will flag tag-path segments as missing references. // Symmetric with DetectBlockersAsync. - CollectCallIdentifiers(a.Value, referenced); + CollectCallIdentifiers(a.Value, referencedFromTemplates); + CollectLocalDeclarations(a.Value, locallyDeclared); } } foreach (var m in content.ApiMethods) { var resolution = ResolveOrDefault(resolutionMap, "ApiMethod", m.Name); if (resolution.Action == ResolutionAction.Skip) continue; - CollectCallIdentifiers(m.Script, referenced); + CollectCallIdentifiers(m.Script, referencedFromApiMethods); + CollectLocalDeclarations(m.Script, locallyDeclared); } - foreach (var candidate in referenced.OrderBy(n => n, StringComparer.Ordinal)) + // ApiMethod references are hard errors (no downstream deploy gate); + // template-only references are advisory warnings (the design-time deploy + // gate re-validates with a real compile). + var allCandidates = new HashSet(referencedFromTemplates, StringComparer.Ordinal); + allCandidates.UnionWith(referencedFromApiMethods); + foreach (var candidate in allCandidates.OrderBy(n => n, StringComparer.Ordinal)) { if (!LooksLikeResourceName(candidate)) continue; if (KnownNonReferenceNames.Contains(candidate)) continue; + if (locallyDeclared.Contains(candidate)) continue; if (sharedScriptNames.Contains(candidate) || externalSystemNames.Contains(candidate)) continue; - errors.Add( - $"Script references SharedScript or ExternalSystem '{candidate}' not present in bundle or target."); + if (referencedFromApiMethods.Contains(candidate)) + { + errors.Add( + $"Script references SharedScript or ExternalSystem '{candidate}' not present in bundle or target."); + } + else + { + warnings.Add( + $"Template script references SharedScript or ExternalSystem '{candidate}' not present in bundle or target — advisory; re-validated at deploy time."); + } } // Fail fast — running the full validator over templates that already // failed name resolution would produce duplicate / lower-quality errors // (the missing identifier shows up there as "callee not found" too). - if (errors.Count > 0) return errors; + // Warnings ride along regardless — they never gate the import. + if (errors.Count > 0) return (errors, warnings); // ---- Pass 2: full SemanticValidator over imported templates ---- @@ -4230,7 +4338,7 @@ public sealed class BundleImporter : IBundleImporter } } - return errors; + return (errors, warnings); } /// diff --git a/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/BundleImporterApplyTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/BundleImporterApplyTests.cs index 8f16e58d..2be247be 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/BundleImporterApplyTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/BundleImporterApplyTests.cs @@ -129,6 +129,7 @@ public sealed class BundleImporterApplyTests : IDisposable var templateIds = await ctx.Templates.Select(t => t.Id).ToListAsync(); var sharedScriptIds = await ctx.SharedScripts.Select(s => s.Id).ToListAsync(); var externalSystemIds = await ctx.ExternalSystemDefinitions.Select(e => e.Id).ToListAsync(); + var apiMethodIds = await ctx.ApiMethods.Select(m => m.Id).ToListAsync(); var selection = new ExportSelection( TemplateIds: templateIds, SharedScriptIds: sharedScriptIds, @@ -136,7 +137,7 @@ public sealed class BundleImporterApplyTests : IDisposable DatabaseConnectionIds: Array.Empty(), NotificationListIds: Array.Empty(), SmtpConfigurationIds: Array.Empty(), - ApiMethodIds: Array.Empty(), + ApiMethodIds: apiMethodIds, IncludeDependencies: false); bundleStream = await exporter.ExportAsync(selection, user: "alice", sourceEnvironment: "dev", passphrase: null, cancellationToken: CancellationToken.None); @@ -159,6 +160,7 @@ public sealed class BundleImporterApplyTests : IDisposable ctx.Templates.RemoveRange(ctx.Templates); ctx.SharedScripts.RemoveRange(ctx.SharedScripts); ctx.TemplateFolders.RemoveRange(ctx.TemplateFolders); + ctx.ApiMethods.RemoveRange(ctx.ApiMethods); await ctx.SaveChangesAsync(); } @@ -320,15 +322,15 @@ public sealed class BundleImporterApplyTests : IDisposable [Fact] public async Task ApplyAsync_rolls_back_all_changes_when_semantic_validation_fails() { - // Arrange: seed a template whose script body calls MissingHelper(). + // Arrange: seed an ApiMethod whose script body calls MissingHelper(). // No SharedScript by that name exists in source or (after wipe) in the - // target, so semantic validation must reject the apply. + // target. ApiMethod name-resolution findings are HARD errors (unlike + // template-script findings, which are advisory as of #05-T19), so this + // must reject the apply and roll everything back. await using (var scope = _provider.CreateAsyncScope()) { var ctx = scope.ServiceProvider.GetRequiredService(); - var t = new Template("BrokenPump") { Description = "broken" }; - t.Scripts.Add(new TemplateScript("init", "var x = MissingHelper();")); - ctx.Templates.Add(t); + ctx.ApiMethods.Add(new ApiMethod("BrokenIngest", "var x = MissingHelper();")); await ctx.SaveChangesAsync(); } var sessionId = await ExportAndLoadAsync(); @@ -340,16 +342,16 @@ public sealed class BundleImporterApplyTests : IDisposable var importer = scope.ServiceProvider.GetRequiredService(); await Assert.ThrowsAsync(() => importer.ApplyAsync(sessionId, - new List { new("Template", "BrokenPump", ResolutionAction.Add, null) }, + new List { new("ApiMethod", "BrokenIngest", ResolutionAction.Add, null) }, user: "bob")); } - // Assert — target still wiped (template not committed), AND a + // Assert — target still wiped (ApiMethod not committed), AND a // BundleImportFailed row exists. await using (var scope = _provider.CreateAsyncScope()) { var ctx = scope.ServiceProvider.GetRequiredService(); - Assert.Equal(0, await ctx.Templates.CountAsync()); + Assert.Equal(0, await ctx.ApiMethods.CountAsync()); Assert.True(await ctx.AuditLogEntries.AnyAsync(a => a.Action == "BundleImportFailed")); } @@ -488,9 +490,7 @@ public sealed class BundleImporterApplyTests : IDisposable await using (var scope = _provider.CreateAsyncScope()) { var ctx = scope.ServiceProvider.GetRequiredService(); - var t = new Template("BrokenPump") { Description = "broken" }; - t.Scripts.Add(new TemplateScript("init", "var x = MissingHelper();")); - ctx.Templates.Add(t); + ctx.ApiMethods.Add(new ApiMethod("BrokenIngest", "var x = MissingHelper();")); await ctx.SaveChangesAsync(); } var sessionId = await ExportAndLoadAsync(); @@ -502,7 +502,7 @@ public sealed class BundleImporterApplyTests : IDisposable var importer = scope.ServiceProvider.GetRequiredService(); await Assert.ThrowsAsync(() => importer.ApplyAsync(sessionId, - new List { new("Template", "BrokenPump", ResolutionAction.Add, null) }, + new List { new("ApiMethod", "BrokenIngest", ResolutionAction.Add, null) }, user: "bob")); } @@ -1400,14 +1400,13 @@ public sealed class BundleImporterApplyTests : IDisposable [Fact] public async Task ApplyAsync_failed_apply_publishes_nothing() { - // A template whose script calls an unresolved helper fails semantic - // validation BEFORE commit, so nothing is published. + // An ApiMethod whose script calls an unresolved helper fails semantic + // validation BEFORE commit (ApiMethod findings are hard errors), so + // nothing is published. await using (var scope = _provider.CreateAsyncScope()) { var ctx = scope.ServiceProvider.GetRequiredService(); - var t = new Template("Bad"); - t.Scripts.Add(new TemplateScript("init", "var x = UnknownHelper();")); - ctx.Templates.Add(t); + ctx.ApiMethods.Add(new ApiMethod("Bad", "var x = UnknownHelper();")); await ctx.SaveChangesAsync(); } @@ -1417,13 +1416,13 @@ public sealed class BundleImporterApplyTests : IDisposable var exporter = scope.ServiceProvider.GetRequiredService(); var ctx = scope.ServiceProvider.GetRequiredService(); var selection = new ExportSelection( - TemplateIds: await ctx.Templates.Select(t => t.Id).ToListAsync(), + TemplateIds: Array.Empty(), SharedScriptIds: Array.Empty(), ExternalSystemIds: Array.Empty(), DatabaseConnectionIds: Array.Empty(), NotificationListIds: Array.Empty(), SmtpConfigurationIds: Array.Empty(), - ApiMethodIds: Array.Empty(), + ApiMethodIds: await ctx.ApiMethods.Select(m => m.Id).ToListAsync(), IncludeDependencies: false); var stream = await exporter.ExportAsync(selection, user: "alice", sourceEnvironment: "dev", passphrase: null, cancellationToken: CancellationToken.None); @@ -1439,7 +1438,7 @@ public sealed class BundleImporterApplyTests : IDisposable var importer = scope.ServiceProvider.GetRequiredService(); await Assert.ThrowsAsync(() => importer.ApplyAsync(sessionId, - new List { new("Template", "Bad", ResolutionAction.Add, null) }, + new List { new("ApiMethod", "Bad", ResolutionAction.Add, null) }, user: "bob")); } diff --git a/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/BundleImporterPreviewTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/BundleImporterPreviewTests.cs index 546e1d47..23a795cf 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/BundleImporterPreviewTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/BundleImporterPreviewTests.cs @@ -2,6 +2,7 @@ using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using ZB.MOM.WW.ScadaBridge.Commons.Entities.ExternalSystems; +using ZB.MOM.WW.ScadaBridge.Commons.Entities.InboundApi; using ZB.MOM.WW.ScadaBridge.Commons.Entities.Instances; using ZB.MOM.WW.ScadaBridge.Commons.Entities.Scripts; using ZB.MOM.WW.ScadaBridge.Commons.Entities.Sites; @@ -330,7 +331,7 @@ public sealed class BundleImporterPreviewTests : IDisposable } [Fact] - public async Task PreviewAsync_emits_Blocker_when_required_dependency_missing() + public async Task PreviewAsync_emits_Warning_when_template_script_dependency_missing() { // Arrange: seed a template whose script body calls MissingHelper(), and // an unrelated HelperFn() shared script that *is* defined but isn't the @@ -338,10 +339,9 @@ public sealed class BundleImporterPreviewTests : IDisposable // selection that only pulls the template — the bundle won't carry // MissingHelper (it doesn't exist anywhere) so the preview must flag it. // - // To get MissingHelper into the bundle script body without the export - // resolver pulling it in (it can't — it doesn't exist), we just seed - // the template with a script that mentions it; the resolver scan only - // matters for entity discovery, the body text is preserved verbatim. + // Under #05-T19 a TEMPLATE-script name-resolution finding is ADVISORY + // (the design-time deploy gate re-validates it), so the preview surfaces + // it as ConflictKind.Warning — NOT a Blocker. await using (var scope = _provider.CreateAsyncScope()) { var ctx = scope.ServiceProvider.GetRequiredService(); @@ -357,18 +357,6 @@ public sealed class BundleImporterPreviewTests : IDisposable var bundleStream = await ExportTemplatesAsync(); var bytes = await StreamToBytes(bundleStream); - // Wipe the SharedScripts table so MissingHelper has no chance of being - // resolved in the target either. (HelperFn is intentionally seeded so - // we can verify the blocker check is specific — it should NOT flag - // HelperFn since it's in the target.) - await using (var scope = _provider.CreateAsyncScope()) - { - var ctx = scope.ServiceProvider.GetRequiredService(); - // Keep HelperFn + ErpSystem so they're in the target's resolved set. - // Just confirm via assertion that MissingHelper is the blocker name. - await ctx.SaveChangesAsync(); - } - // Act ImportPreview preview; await using (var scope = _provider.CreateAsyncScope()) @@ -378,16 +366,64 @@ public sealed class BundleImporterPreviewTests : IDisposable preview = await importer.PreviewAsync(session.SessionId); } - // Assert: there's at least one Blocker, and the MissingHelper one is in there. - Assert.Contains(preview.Items, i => i.Kind == ConflictKind.Blocker); + // Assert: MissingHelper surfaces as a Warning (advisory), not a Blocker. + Assert.Contains(preview.Items, i => + i.Kind == ConflictKind.Warning + && i.Name == "MissingHelper" + && i.BlockerReason is not null + && i.BlockerReason.Contains("MissingHelper", StringComparison.Ordinal)); + Assert.DoesNotContain(preview.Items, i => + i.Name == "MissingHelper" && i.Kind == ConflictKind.Blocker); + // Conversely, HelperFn must NOT be flagged at all — it's seeded in the target. + Assert.DoesNotContain(preview.Items, i => i.Name == "HelperFn"); + } + + [Fact] + public async Task PreviewAsync_emits_Blocker_when_ApiMethod_dependency_missing() + { + // #05-T19 severity split: an ApiMethod script referencing a missing + // dependency has no downstream deploy gate, so the preview must surface + // it as a hard Blocker (parity with the apply-time SemanticValidationException). + await using (var scope = _provider.CreateAsyncScope()) + { + var ctx = scope.ServiceProvider.GetRequiredService(); + ctx.ApiMethods.Add(new ApiMethod("Ingest", "var x = MissingHelper();")); + await ctx.SaveChangesAsync(); + } + + byte[] bytes; + await using (var scope = _provider.CreateAsyncScope()) + { + var exporter = scope.ServiceProvider.GetRequiredService(); + var ctx = scope.ServiceProvider.GetRequiredService(); + var apiIds = await ctx.ApiMethods.Select(m => m.Id).ToListAsync(); + var selection = new ExportSelection( + TemplateIds: Array.Empty(), + SharedScriptIds: Array.Empty(), + ExternalSystemIds: Array.Empty(), + DatabaseConnectionIds: Array.Empty(), + NotificationListIds: Array.Empty(), + SmtpConfigurationIds: Array.Empty(), + ApiMethodIds: apiIds, + IncludeDependencies: false); + var stream = await exporter.ExportAsync(selection, user: "alice", + sourceEnvironment: "dev", passphrase: null, cancellationToken: CancellationToken.None); + bytes = await StreamToBytes(stream); + } + + ImportPreview preview; + await using (var scope = _provider.CreateAsyncScope()) + { + var importer = scope.ServiceProvider.GetRequiredService(); + var session = await importer.LoadAsync(new MemoryStream(bytes), passphrase: null); + preview = await importer.PreviewAsync(session.SessionId); + } + Assert.Contains(preview.Items, i => i.Kind == ConflictKind.Blocker && i.Name == "MissingHelper" && i.BlockerReason is not null && i.BlockerReason.Contains("MissingHelper", StringComparison.Ordinal)); - // Conversely, HelperFn must NOT be a blocker — it's seeded in the target. - Assert.DoesNotContain(preview.Items, i => - i.Kind == ConflictKind.Blocker && i.Name == "HelperFn"); } [Fact] diff --git a/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/BundleImporterRollbackFailureTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/BundleImporterRollbackFailureTests.cs index 6b611f7e..8980a471 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/BundleImporterRollbackFailureTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/BundleImporterRollbackFailureTests.cs @@ -7,6 +7,7 @@ using Microsoft.EntityFrameworkCore.Storage.ValueConversion; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using ZB.MOM.WW.ScadaBridge.Commons.Entities.Deployment; +using ZB.MOM.WW.ScadaBridge.Commons.Entities.InboundApi; using ZB.MOM.WW.ScadaBridge.Commons.Entities.Scripts; using ZB.MOM.WW.ScadaBridge.Commons.Entities.Templates; using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories; @@ -108,17 +109,15 @@ public sealed class BundleImporterRollbackFailureTests : IDisposable [Fact] public async Task ApplyAsync_writes_BundleImportFailed_even_when_RollbackAsync_throws() { - // Arrange: seed a template whose script body references MissingHelper() - // so semantic validation will reject the apply (same broken-bundle shape - // as BundleImporterApplyTests.ApplyAsync_rolls_back_all_changes_…). Then - // arm the interceptor to throw on rollback so the catch path has to - // survive a rollback failure. + // Arrange: seed an ApiMethod whose script body references MissingHelper() + // so semantic validation will reject the apply (ApiMethod findings are + // hard errors; same broken-bundle shape as BundleImporterApplyTests. + // ApplyAsync_rolls_back_all_changes_…). Then arm the interceptor to throw + // on rollback so the catch path has to survive a rollback failure. await using (var scope = _provider.CreateAsyncScope()) { var ctx = scope.ServiceProvider.GetRequiredService(); - var t = new Template("BrokenPump") { Description = "broken" }; - t.Scripts.Add(new TemplateScript("init", "var x = MissingHelper();")); - ctx.Templates.Add(t); + ctx.ApiMethods.Add(new ApiMethod("BrokenIngest", "var x = MissingHelper();")); await ctx.SaveChangesAsync(); } @@ -136,7 +135,7 @@ public sealed class BundleImporterRollbackFailureTests : IDisposable var importer = scope.ServiceProvider.GetRequiredService(); thrown = await Assert.ThrowsAsync(() => importer.ApplyAsync(sessionId, - new List { new("Template", "BrokenPump", ResolutionAction.Add, null) }, + new List { new("ApiMethod", "BrokenIngest", ResolutionAction.Add, null) }, user: "bob")); } Assert.NotNull(thrown); @@ -177,6 +176,7 @@ public sealed class BundleImporterRollbackFailureTests : IDisposable var exporter = scope.ServiceProvider.GetRequiredService(); var ctx = scope.ServiceProvider.GetRequiredService(); var templateIds = await ctx.Templates.Select(t => t.Id).ToListAsync(); + var apiMethodIds = await ctx.ApiMethods.Select(m => m.Id).ToListAsync(); var selection = new ExportSelection( TemplateIds: templateIds, SharedScriptIds: Array.Empty(), @@ -184,7 +184,7 @@ public sealed class BundleImporterRollbackFailureTests : IDisposable DatabaseConnectionIds: Array.Empty(), NotificationListIds: Array.Empty(), SmtpConfigurationIds: Array.Empty(), - ApiMethodIds: Array.Empty(), + ApiMethodIds: apiMethodIds, IncludeDependencies: false); bundleStream = await exporter.ExportAsync(selection, user: "alice", sourceEnvironment: "dev", passphrase: null, cancellationToken: CancellationToken.None); @@ -207,6 +207,7 @@ public sealed class BundleImporterRollbackFailureTests : IDisposable ctx.Templates.RemoveRange(ctx.Templates); ctx.SharedScripts.RemoveRange(ctx.SharedScripts); ctx.TemplateFolders.RemoveRange(ctx.TemplateFolders); + ctx.ApiMethods.RemoveRange(ctx.ApiMethods); await ctx.SaveChangesAsync(); } diff --git a/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/SemanticValidatorImportTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/SemanticValidatorImportTests.cs index 7c0fe4c0..b4a5c05c 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/SemanticValidatorImportTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/SemanticValidatorImportTests.cs @@ -77,6 +77,10 @@ public sealed class SemanticValidatorImportTests : IDisposable var exporter = scope.ServiceProvider.GetRequiredService(); var ctx = scope.ServiceProvider.GetRequiredService(); var ids = await ctx.Templates.Select(t => t.Id).ToListAsync(); + // #05-T19: also carry any seeded ApiMethods so the severity-split + // tests (template-script = warning, ApiMethod = hard error) exercise + // both source kinds through the same export→load→apply path. + var apiIds = await ctx.ApiMethods.Select(m => m.Id).ToListAsync(); var selection = new ExportSelection( TemplateIds: ids, SharedScriptIds: Array.Empty(), @@ -84,7 +88,7 @@ public sealed class SemanticValidatorImportTests : IDisposable DatabaseConnectionIds: Array.Empty(), NotificationListIds: Array.Empty(), SmtpConfigurationIds: Array.Empty(), - ApiMethodIds: Array.Empty(), + ApiMethodIds: apiIds, IncludeDependencies: false); var stream = await exporter.ExportAsync(selection, @@ -103,6 +107,7 @@ public sealed class SemanticValidatorImportTests : IDisposable ctx.TemplateScripts.RemoveRange(ctx.TemplateScripts); ctx.TemplateAttributes.RemoveRange(ctx.TemplateAttributes); ctx.Templates.RemoveRange(ctx.Templates); + ctx.ApiMethods.RemoveRange(ctx.ApiMethods); await ctx.SaveChangesAsync(); } @@ -118,15 +123,14 @@ public sealed class SemanticValidatorImportTests : IDisposable } [Fact] - public async Task SemanticValidator_catches_invalid_call_target_at_import() + public async Task TemplateScript_with_unresolved_reference_warns_but_does_not_block_import() { - // Arrange — template whose script body calls UnknownHelper(): a + // #05-T19 — a template whose script body calls UnknownHelper(): a // PascalCase identifier that doesn't resolve to any SharedScript or - // ExternalSystem in the bundle or the target. This is the operator- - // facing "invalid call target" surface — the full SemanticValidator's - // CallScript/CallShared signature checks live downstream of name - // resolution (you can't check arg count against a function that - // doesn't exist). Pass 1 catches it first and fails fast. + // ExternalSystem in the bundle or the target. Under the severity split, + // template-script name-resolution findings are ADVISORY (the design-time + // deploy gate re-validates authoritatively), so the import SUCCEEDS with + // the finding surfaced on ImportResult.Warnings — it does NOT hard-block. await using (var scope = _provider.CreateAsyncScope()) { var ctx = scope.ServiceProvider.GetRequiredService(); @@ -140,8 +144,129 @@ public sealed class SemanticValidatorImportTests : IDisposable var sessionId = await ExportWipeAndLoadAsync(); - // Act — apply must throw SemanticValidationException carrying the bad - // call target by name. + // Act — apply succeeds (no throw); the finding rides on Warnings. + ImportResult result; + await using (var scope = _provider.CreateAsyncScope()) + { + var importer = scope.ServiceProvider.GetRequiredService(); + result = await importer.ApplyAsync(sessionId, + new List + { + new("Template", "ScriptCallsUnknown", ResolutionAction.Add, null), + }, + user: "bob"); + } + + // Assert — the template landed AND the advisory warning names the target. + Assert.Equal(1, result.Added); + Assert.Contains(result.Warnings, + w => w.Contains("UnknownHelper", StringComparison.Ordinal)); + await using (var scope = _provider.CreateAsyncScope()) + { + var ctx = scope.ServiceProvider.GetRequiredService(); + Assert.True(await ctx.Templates.AnyAsync(t => t.Name == "ScriptCallsUnknown")); + } + } + + [Fact] + public async Task Apply_TemplateScriptWithLocalPascalCaseMethod_DoesNotHardBlock() + { + // #05-T19 — a template script that DECLARES and calls its own local + // PascalCase function. The identifier scan would otherwise flag + // ComputeRate as a missing SharedScript reference; the local-declaration + // exclusion (Roslyn-parsed local functions/methods) removes it, so the + // import produces neither an error nor a warning. + await using (var scope = _provider.CreateAsyncScope()) + { + var ctx = scope.ServiceProvider.GetRequiredService(); + var t = new Template("SelfContainedCalc"); + t.Attributes.Add(new TemplateAttribute("A") { DataType = DataType.Double, Value = "1" }); + t.Scripts.Add(new Commons.Entities.Templates.TemplateScript( + "init", + "decimal ComputeRate(decimal x) => x * 2; return ComputeRate(2m);")); + ctx.Templates.Add(t); + await ctx.SaveChangesAsync(); + } + + var sessionId = await ExportWipeAndLoadAsync(); + + ImportResult result; + await using (var scope = _provider.CreateAsyncScope()) + { + var importer = scope.ServiceProvider.GetRequiredService(); + result = await importer.ApplyAsync(sessionId, + new List + { + new("Template", "SelfContainedCalc", ResolutionAction.Add, null), + }, + user: "bob"); + } + + Assert.Equal(1, result.Added); + // The locally-declared function must NOT surface as a warning. + Assert.DoesNotContain(result.Warnings, + w => w.Contains("ComputeRate", StringComparison.Ordinal)); + } + + [Fact] + public async Task Apply_TemplateScriptWithRegexAndStringBuilder_DoesNotHardBlock() + { + // #05-T19 — a template script using stdlib helpers that the extended + // denylist now recognizes (Regex, StringBuilder, …). None resolve to a + // user SharedScript/ExternalSystem, and none should be flagged. + await using (var scope = _provider.CreateAsyncScope()) + { + var ctx = scope.ServiceProvider.GetRequiredService(); + var t = new Template("UsesStdlib"); + t.Scripts.Add(new Commons.Entities.Templates.TemplateScript( + "init", + """ + var ok = Regex.IsMatch("abc", "a.c"); + var sb = new StringBuilder(); + sb.Append("x"); + return sb.ToString(); + """)); + ctx.Templates.Add(t); + await ctx.SaveChangesAsync(); + } + + var sessionId = await ExportWipeAndLoadAsync(); + + ImportResult result; + await using (var scope = _provider.CreateAsyncScope()) + { + var importer = scope.ServiceProvider.GetRequiredService(); + result = await importer.ApplyAsync(sessionId, + new List + { + new("Template", "UsesStdlib", ResolutionAction.Add, null), + }, + user: "bob"); + } + + Assert.Equal(1, result.Added); + Assert.DoesNotContain(result.Warnings, + w => w.Contains("Regex", StringComparison.Ordinal) + || w.Contains("StringBuilder", StringComparison.Ordinal)); + } + + [Fact] + public async Task Apply_ApiMethodWithUnresolvedReference_StillHardBlocks() + { + // #05-T19 — an ApiMethod script referencing a genuinely-missing + // ExternalSystem. ApiMethod findings have no downstream deploy gate, so + // they remain HARD errors: the apply throws SemanticValidationException. + await using (var scope = _provider.CreateAsyncScope()) + { + var ctx = scope.ServiceProvider.GetRequiredService(); + ctx.ApiMethods.Add(new Commons.Entities.InboundApi.ApiMethod( + "Ingest", + "var x = ErpMissing(); return x;")); + await ctx.SaveChangesAsync(); + } + + var sessionId = await ExportWipeAndLoadAsync(); + SemanticValidationException ex = default!; await using (var scope = _provider.CreateAsyncScope()) { @@ -150,21 +275,20 @@ public sealed class SemanticValidatorImportTests : IDisposable importer.ApplyAsync(sessionId, new List { - new("Template", "ScriptCallsUnknown", ResolutionAction.Add, null), + new("ApiMethod", "Ingest", ResolutionAction.Add, null), }, user: "bob")); } - // Assert — error message names the bad target. Assert.NotEmpty(ex.Errors); Assert.Contains(ex.Errors, - err => err.Contains("UnknownHelper", StringComparison.Ordinal)); + err => err.Contains("ErpMissing", StringComparison.Ordinal)); - // Rollback — no template row landed. + // Rollback — no ApiMethod row landed. await using (var scope = _provider.CreateAsyncScope()) { var ctx = scope.ServiceProvider.GetRequiredService(); - Assert.False(await ctx.Templates.AnyAsync(t => t.Name == "ScriptCallsUnknown")); + Assert.False(await ctx.ApiMethods.AnyAsync(m => m.Name == "Ingest")); } } diff --git a/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/ValidationFailureTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/ValidationFailureTests.cs index 3422a57a..484b04db 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/ValidationFailureTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/ValidationFailureTests.cs @@ -2,6 +2,7 @@ using Microsoft.EntityFrameworkCore; using Microsoft.EntityFrameworkCore.Diagnostics; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; +using ZB.MOM.WW.ScadaBridge.Commons.Entities.InboundApi; using ZB.MOM.WW.ScadaBridge.Commons.Entities.Scripts; using ZB.MOM.WW.ScadaBridge.Commons.Entities.Templates; using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories; @@ -57,37 +58,38 @@ public sealed class ValidationFailureTests : IDisposable public async Task Semantic_validation_failure_rolls_back_all_writes() { // Arrange: - // - Seed a template whose script body references MissingHelper(), + // - Seed an ApiMethod whose script body references MissingHelper(), // a SharedScript that is NOT in the bundle (we don't seed one) and - // also NOT in the target after the wipe. - // - Export the template only (no helper alongside). + // also NOT in the target after the wipe. ApiMethod name-resolution + // findings are HARD errors (no downstream deploy gate re-validates + // them — unlike template-script findings, which are advisory as of + // #05-T19), so this is the vehicle that still forces a rollback. + // - Export the ApiMethod only (no helper alongside). // - Wipe the target so the apply runs through Add-paths. // - Snapshot row counts so the rollback assertion is unambiguous. await using (var scope = _provider.CreateAsyncScope()) { var ctx = scope.ServiceProvider.GetRequiredService(); - var t = new Template("BrokenPump") { Description = "calls a missing helper" }; - t.Scripts.Add(new TemplateScript("init", "var x = MissingHelper();")); - ctx.Templates.Add(t); + ctx.ApiMethods.Add(new ApiMethod("BrokenIngest", "var x = MissingHelper();")); await ctx.SaveChangesAsync(); } - // Export only the broken template. IncludeDependencies=false guarantees + // Export only the broken ApiMethod. IncludeDependencies=false guarantees // MissingHelper is NOT pulled in even if one happened to exist. byte[] bundleBytes; await using (var scope = _provider.CreateAsyncScope()) { var exporter = scope.ServiceProvider.GetRequiredService(); var ctx = scope.ServiceProvider.GetRequiredService(); - var ids = await ctx.Templates.Select(t => t.Id).ToListAsync(); + var ids = await ctx.ApiMethods.Select(m => m.Id).ToListAsync(); var selection = new ExportSelection( - TemplateIds: ids, + TemplateIds: Array.Empty(), SharedScriptIds: Array.Empty(), ExternalSystemIds: Array.Empty(), DatabaseConnectionIds: Array.Empty(), NotificationListIds: Array.Empty(), SmtpConfigurationIds: Array.Empty(), - ApiMethodIds: Array.Empty(), + ApiMethodIds: ids, IncludeDependencies: false); var stream = await exporter.ExportAsync(selection, @@ -103,21 +105,20 @@ public sealed class ValidationFailureTests : IDisposable await using (var scope = _provider.CreateAsyncScope()) { var ctx = scope.ServiceProvider.GetRequiredService(); - ctx.TemplateScripts.RemoveRange(ctx.TemplateScripts); - ctx.Templates.RemoveRange(ctx.Templates); + ctx.ApiMethods.RemoveRange(ctx.ApiMethods); ctx.SharedScripts.RemoveRange(ctx.SharedScripts); await ctx.SaveChangesAsync(); } - int templatesBefore; + int apiMethodsBefore; int sharedScriptsBefore; await using (var scope = _provider.CreateAsyncScope()) { var ctx = scope.ServiceProvider.GetRequiredService(); - templatesBefore = await ctx.Templates.CountAsync(); + apiMethodsBefore = await ctx.ApiMethods.CountAsync(); sharedScriptsBefore = await ctx.SharedScripts.CountAsync(); } - Assert.Equal(0, templatesBefore); + Assert.Equal(0, apiMethodsBefore); Assert.Equal(0, sharedScriptsBefore); // Load the bundle into a session. @@ -139,7 +140,7 @@ public sealed class ValidationFailureTests : IDisposable importer.ApplyAsync(sessionId, new List { - new("Template", "BrokenPump", ResolutionAction.Add, null), + new("ApiMethod", "BrokenIngest", ResolutionAction.Add, null), }, user: "bob")); } @@ -150,11 +151,11 @@ public sealed class ValidationFailureTests : IDisposable Assert.Contains(ex.Errors, err => err.Contains("MissingHelper", StringComparison.Ordinal)); - // 2. No new Template or SharedScript row was committed. + // 2. No new ApiMethod or SharedScript row was committed. await using (var scope = _provider.CreateAsyncScope()) { var ctx = scope.ServiceProvider.GetRequiredService(); - Assert.Equal(templatesBefore, await ctx.Templates.CountAsync()); + Assert.Equal(apiMethodsBefore, await ctx.ApiMethods.CountAsync()); Assert.Equal(sharedScriptsBefore, await ctx.SharedScripts.CountAsync()); // 3. A BundleImportFailed audit row exists. The BundleImporter