diff --git a/docs/components/CLI.md b/docs/components/CLI.md index 38439778..4eaf6a3a 100644 --- a/docs/components/CLI.md +++ b/docs/components/CLI.md @@ -118,7 +118,7 @@ scadabridge instance deploy --id 7 | `flow-debug` | `run-method`, `run-script`, `run-value-change`, `replay` | Deployer; replay also accepts OperationalAudit roles | | `shared-script` | `list`, `get`, `create`, `update`, `delete` | `Designer` | | `db-connection` | `list`, `get`, `create`, `update`, `delete` | `Designer` | -| `api-method` | `list`, `get`, `create`, `update`, `delete` | `Designer` | +| `api-method` | `list`, `get`, `create`, `update`, `delete`, `validate`, `recompile` | `Designer` | | `bundle` | `export`, `preview`, `import` | Designer export; Administrator preview/import | | `audit` | `query`, `export`, `tree`, `backfill-source-node`, `verify-chain` | Audit roles; Administrator backfill; verify-chain is a local placeholder | | `audit-config` | `query` (config-change audit trail; was `audit-log` pre-M8) | `Admin` | diff --git a/docs/components/InboundAPI.md b/docs/components/InboundAPI.md index f1789e8a..a661c5e1 100644 --- a/docs/components/InboundAPI.md +++ b/docs/components/InboundAPI.md @@ -256,7 +256,7 @@ The in-memory handler cache still holds the previous compiled delegate. If the u ### A method is stuck in the known-bad-methods cache -If a previously broken script is fixed but `ExecuteAsync` still returns `"Script compilation failed for this method"`, the method name is in `_knownBadMethods`. `CompileAndRegister` clears the bad-method entry on a successful compile; calling it (via the Management API or CLI `api-method update`) after the fix is applied resets the cache and makes the corrected script active immediately. +If a previously broken script is fixed but `ExecuteAsync` still returns `"Script compilation failed for this method"`, the method name is in `_knownBadMethods`. A record only short-circuits while the stored script text is the one that failed, so a changed script always recompiles; a stale record therefore means the script text is unchanged and something outside it made the compile fail. `api-method get --id ` shows the node's `isKnownBad` / `lastCompileError` (the real diagnostics; the data plane keeps its generic message), `api-method validate --id ` compiles the stored script and returns the diagnostics without touching the cache, and `api-method recompile --id ` re-runs `CompileAndRegister` — clearing the entry on success — without re-sending the entity (#75). All three are per central node; the Central UI's API-method edit page exposes the same state and actions. ### Routed calls time out but the site is reachable diff --git a/docs/components/ManagementService.md b/docs/components/ManagementService.md index e6d76237..ebba4ba0 100644 --- a/docs/components/ManagementService.md +++ b/docs/components/ManagementService.md @@ -177,7 +177,7 @@ There is **no** out-of-process actor path. The actor was advertised via `Cluster | Notification lists / SMTP | `ListNotificationLists`, `GetNotificationList`, `CreateNotificationList`, `UpdateNotificationList`, `DeleteNotificationList`, `ListSmtpConfigs`, `UpdateSmtpConfig` | Designer (mutations) | | Shared scripts | `ListSharedScripts`, `GetSharedScript`, `CreateSharedScript`, `UpdateSharedScript`, `DeleteSharedScript` | Designer (mutations) | | Database connections | `ListDatabaseConnections`, `GetDatabaseConnection`, `CreateDatabaseConnectionDef`, `UpdateDatabaseConnectionDef`, `DeleteDatabaseConnectionDef` | Designer (mutations) | -| Inbound API methods | `ListApiMethods`, `GetApiMethod`, `CreateApiMethod`, `UpdateApiMethod`, `DeleteApiMethod` | Designer (mutations) | +| Inbound API methods | `ListApiMethods`, `GetApiMethod`, `CreateApiMethod`, `UpdateApiMethod`, `DeleteApiMethod`, `ValidateApiMethod`, `RecompileApiMethod` | Designer (mutations, validate, recompile) | | Security | `ListRoleMappings`, `CreateRoleMapping`, `UpdateRoleMapping`, `DeleteRoleMapping`, `ListApiKeys`, `CreateApiKey`, `UpdateApiKey`, `DeleteApiKey`, `SetApiKeyMethods`, `ListScopeRules`, `AddScopeRule`, `DeleteScopeRule` | Administrator | | Deployments | `MgmtDeployArtifacts`, `QueryDeployments`, `GetDeploymentDiff`, `GetPendingUpdates` | Deployer | | Health | `GetHealthSummary`, `GetSiteHealth` | Any authenticated user | diff --git a/docs/requirements/Component-CLI.md b/docs/requirements/Component-CLI.md index 0e68bb87..51157e10 100644 --- a/docs/requirements/Component-CLI.md +++ b/docs/requirements/Component-CLI.md @@ -409,6 +409,8 @@ scadabridge api-method get --id scadabridge api-method create --name --script [--timeout ] [--parameters ] [--return-def ] scadabridge api-method update --id --script [--timeout ] [--parameters ] [--return-def ] scadabridge api-method delete --id +scadabridge api-method validate --id +scadabridge api-method recompile --id ``` ### Bundle Commands (Transport #24) diff --git a/docs/requirements/Component-InboundAPI.md b/docs/requirements/Component-InboundAPI.md index 80fae17e..2d226826 100644 --- a/docs/requirements/Component-InboundAPI.md +++ b/docs/requirements/Component-InboundAPI.md @@ -181,6 +181,7 @@ API method scripts are pre-compiled in the **background after startup** by `Inbo - Updating a method via the CLI (`api-method update --id --code '...'`) or Management API triggers immediate recompilation (`CompileAndRegister`). The updated script takes effect on the next API call — no node restart is required. - Creating a new method after startup: if the method is created but not yet compiled, the first invocation triggers lazy (on-demand) compilation. +- **Operator diagnostics (#75).** The data plane answers a broken method with the generic `Script compilation failed for this method` (the Roslyn text can leak code to an API-key caller). Operators get the real diagnostics through the role-gated management surface instead: `GetApiMethod` carries the node's `IsCompiled` / `IsKnownBad` / `LastCompileError`, `ValidateApiMethod` compiles the stored script and returns the errors without registering anything, and `RecompileApiMethod` re-runs `CompileAndRegister` for one method without a full-replace update (CLI `api-method get|validate|recompile`, Central UI API-method edit page). Compile state is per central node. > **Compile failures on save are non-fatal but surfaced, and the DB row is authoritative.** The persisted `ApiMethod.Script` row is the single source of truth: on every request `ExecuteAsync` revision-checks the cached delegate against the freshly-fetched script and recompiles in place when it differs, so a saved-but-broken script does **not** leave a previously registered handler silently serving. The change still persists, so the Create/Update Management result carries a top-level **`compileWarning`** (null when the script compiled) listing the Roslyn/trust-model errors. On both **create** and **update**, once a non-compiling script is saved the method returns a compilation error (`HTTP 500`) **consistently on every central node** until a compiling version is saved — there is no node-divergent "old delegate still works here" state. This means a saved script that does not compile is **not silently a no-op** — the caller (CLI / Central UI) sees the warning, and the real diagnostics are also logged centrally. diff --git a/docs/requirements/Component-ManagementService.md b/docs/requirements/Component-ManagementService.md index 776e7568..2182eae1 100644 --- a/docs/requirements/Component-ManagementService.md +++ b/docs/requirements/Component-ManagementService.md @@ -220,7 +220,9 @@ The two-person authorization workflow for writes through the MxAccess Gateway. B ### Inbound API Methods -- **ListApiMethods** / **GetApiMethod**: Query inbound API method definitions. +- **ListApiMethods** / **GetApiMethod**: Query inbound API method definitions. `GetApiMethod` also returns the answering central node's compile state for the stored script — `IsCompiled`, `IsKnownBad`, `LastCompileError` (the real diagnostics behind the data plane's generic `Script compilation failed for this method`) and `CompileStateNode` (#75). +- **ValidateApiMethod** (Designer): Compile the stored script on the receiving node and return `ApiMethodCompileResult` (`Success`, `Errors`, `Node`). Nothing is registered; the known-bad cache is untouched (#75). +- **RecompileApiMethod** (Designer): Re-run `CompileAndRegister` on the stored script without re-sending the entity, clearing a stale known-bad record on success, and drop the node's cached row. Returns `ApiMethodCompileResult`; audited as `Recompile`. Node-local, like every inbound compile (#75). - **CreateApiMethod** / **UpdateApiMethod** / **DeleteApiMethod**: Manage inbound API methods. Create/Update compile the script **once** (`InboundScriptExecutor.CompileForPublish`): the diagnostics become the result's `compileWarning` and the handler registration is queued as a post-commit action reusing that compile — each compile pins a non-collectible assembly, and the old validate-then-register pair compiled twice per save (review 2026-10-21 05/M1). ### Health diff --git a/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/ApiMethodCommands.cs b/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/ApiMethodCommands.cs index 60302da0..546075e6 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/ApiMethodCommands.cs +++ b/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/ApiMethodCommands.cs @@ -23,6 +23,8 @@ public static class ApiMethodCommands command.Add(BuildCreate(urlOption, formatOption, usernameOption, passwordOption)); command.Add(BuildUpdate(urlOption, formatOption, usernameOption, passwordOption)); command.Add(BuildDelete(urlOption, formatOption, usernameOption, passwordOption)); + command.Add(BuildValidate(urlOption, formatOption, usernameOption, passwordOption)); + command.Add(BuildRecompile(urlOption, formatOption, usernameOption, passwordOption)); return command; } @@ -52,6 +54,40 @@ public static class ApiMethodCommands return cmd; } + private static Command BuildValidate(Option urlOption, Option formatOption, Option usernameOption, Option passwordOption) + { + var idOption = new Option("--id") { Description = "API method ID", Required = true }; + var cmd = new Command("validate") + { + Description = "Compile an API method's stored script on the central node and show the compile errors (nothing is registered)" + }; + cmd.Add(idOption); + cmd.SetAction(async (ParseResult result) => + { + var id = result.GetValue(idOption); + return await CommandHelpers.ExecuteCommandAsync( + result, urlOption, formatOption, usernameOption, passwordOption, new ValidateApiMethodCommand(id)); + }); + return cmd; + } + + private static Command BuildRecompile(Option urlOption, Option formatOption, Option usernameOption, Option passwordOption) + { + var idOption = new Option("--id") { Description = "API method ID", Required = true }; + var cmd = new Command("recompile") + { + Description = "Recompile and re-register an API method's stored script on the central node, clearing a stale compile failure" + }; + cmd.Add(idOption); + cmd.SetAction(async (ParseResult result) => + { + var id = result.GetValue(idOption); + return await CommandHelpers.ExecuteCommandAsync( + result, urlOption, formatOption, usernameOption, passwordOption, new RecompileApiMethodCommand(id)); + }); + return cmd; + } + private static Command BuildCreate(Option urlOption, Option formatOption, Option usernameOption, Option passwordOption) { var nameOption = new Option("--name") { Description = "Method name", Required = true }; diff --git a/src/ZB.MOM.WW.ScadaBridge.CLI/README.md b/src/ZB.MOM.WW.ScadaBridge.CLI/README.md index 032a696c..d412a9c3 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CLI/README.md +++ b/src/ZB.MOM.WW.ScadaBridge.CLI/README.md @@ -2342,7 +2342,13 @@ scadabridge --url api-method list #### `api-method get` -Get a single inbound API method by ID. +Get a single inbound API method by ID. Besides the method's fields the reply carries the +answering central node's compile state for the stored script: `isCompiled`, `isKnownBad` +(the script failed to compile and requests fast-fail with the generic +`Script compilation failed for this method`), `lastCompileError` (the real compiler / +trust-model diagnostics when `isKnownBad`) and `compileStateNode`. Each central node +compiles independently, so the state is per node; the fields are null on a host with no +inbound API executor. ```sh scadabridge --url api-method get --id @@ -2352,6 +2358,37 @@ scadabridge --url api-method get --id |--------|----------|-------------| | `--id` | yes | API method ID | +#### `api-method validate` + +Compile the method's **stored** script on the central node and return +`{ apiMethodId, name, success, errors, node }` — the same compile the data plane runs, with +the diagnostics the data plane deliberately hides from API-key callers. Nothing is +registered and the known-bad cache is untouched. Requires `Designer`. + +```sh +scadabridge --url api-method validate --id +``` + +| Option | Required | Description | +|--------|----------|-------------| +| `--id` | yes | API method ID | + +#### `api-method recompile` + +Re-run `CompileAndRegister` on the method's stored script on the central node, without +re-sending the entity: a success replaces the cached handler and clears a stale compile +failure (for example one caused by something outside the script that has since been +fixed); a failure records the new diagnostics. Returns the same shape as `validate` and is +audited as `Recompile`. Affects the answering node only. Requires `Designer`. + +```sh +scadabridge --url api-method recompile --id +``` + +| Option | Required | Description | +|--------|----------|-------------| +| `--id` | yes | API method ID | + #### `api-method create` Create a new inbound API method. diff --git a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Design/ApiMethodForm.razor b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Design/ApiMethodForm.razor index c63ee4c9..3ed7bf8d 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Design/ApiMethodForm.razor +++ b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Design/ApiMethodForm.razor @@ -9,6 +9,9 @@ @using ZB.MOM.WW.ScadaBridge.Commons.Entities.InboundApi @using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories @using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Security +@using ZB.MOM.WW.ScadaBridge.Commons.Types.InboundApi +@using ZB.MOM.WW.ScadaBridge.ManagementService +@using Microsoft.Extensions.DependencyInjection @using ScriptAnalysis = ZB.MOM.WW.ScadaBridge.CentralUI.ScriptAnalysis @attribute [Authorize(Policy = AuthorizationPolicies.RequireDesign)] @inject IInboundApiRepository InboundApiRepository @@ -123,6 +126,65 @@ + @if (_existing != null) + { + @* ScadaBridge#75: the data plane answers a broken method with a generic + "Script compilation failed for this method"; operators read the real + diagnostics here (same operations as CLI api-method get/validate/recompile). *@ +
+ Compilation (saved script) +
+ @if (_compileStatus is null) + { + Unavailable on this node + } + else if (_compileStatus.IsKnownBad) + { + Compile failed + } + else if (_compileStatus.IsCompiled) + { + Compiled + } + else + { + Not compiled yet + } + @if (_compileStatus?.Node is { } node) + { + on node @node + } + + +
+ @if (_compileStatus?.LastCompileError is { } lastError) + { + 
@lastError
+ } + @if (_compileResult != null) + { +
+ @_compileAction: @(_compileResult.Success ? "the saved script compiles." : "the saved script does not compile.") +
+ @if (_compileResult.Errors.Count > 0) + { +
    + @foreach (var error in _compileResult.Errors) + { +
  • @error
    • + } +
+ } + } +
+ Checks the script as saved, not unsaved edits. Recompile re-registers it on this + central node, clearing a stale compile failure. +
+
+ } + @if (_formError != null) {
@_formError
@@ -245,6 +307,10 @@ private HashSet _initialKeyIds = new(StringComparer.Ordinal); private bool _showTestRun; + private ApiMethodCompileStatus? _compileStatus; + private ApiMethodCompileResult? _compileResult; + private string? _compileAction; + private bool _compiling; private bool _running; private Dictionary _paramValues = new(); private ScriptAnalysis.SandboxRunResult? _runResult; @@ -277,6 +343,7 @@ var keysForMethod = await ApiKeyAdmin.GetKeysForMethodAsync(_existing.Name); _initialKeyIds = new HashSet(keysForMethod, StringComparer.Ordinal); _selectedKeyIds = new HashSet(_initialKeyIds, StringComparer.Ordinal); + _compileStatus = ApiMethodCompileOperations.GetStatus(MutationServices, _existing); } } catch (Exception ex) { _formError = ex.Message; } @@ -284,6 +351,35 @@ _loading = false; } + private Task ValidateSavedScriptAsync() => + RunCompileActionAsync("Validate", (sp, _) => ApiMethodCompileOperations.ValidateAsync(sp, _existing!.Id)); + + private Task RecompileSavedScriptAsync() => + RunCompileActionAsync("Recompile", (sp, user) => ApiMethodCompileOperations.RecompileAsync(sp, _existing!.Id, user)); + + /// + /// Runs a compile action under the same Designer gate as the management commands, in a + /// fresh DI scope (the recompile writes an audit row), then refreshes the status badge. + /// + private async Task RunCompileActionAsync( + string action, Func> operation) + { + if (_existing is null) return; + _formError = null; + _compiling = true; + try + { + var user = await MutationAuth.RequireRoleAsync(Roles.Designer); + await using var scope = MutationServices.GetRequiredService().CreateAsyncScope(); + _compileResult = await operation(scope.ServiceProvider, user); + _compileAction = action; + var saved = await scope.ServiceProvider.GetRequiredService().GetApiMethodByIdAsync(_existing.Id); + _compileStatus = saved is null ? null : ApiMethodCompileOperations.GetStatus(scope.ServiceProvider, saved); + } + catch (Exception ex) { _formError = ex.Message; } + finally { _compiling = false; } + } + private void ToggleKey(string keyId, bool isChecked) { if (isChecked) _selectedKeyIds.Add(keyId); diff --git a/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Management/InboundApiCommands.cs b/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Management/InboundApiCommands.cs index d893f551..1119f586 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Management/InboundApiCommands.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Management/InboundApiCommands.cs @@ -7,3 +7,18 @@ public record CreateApiMethodCommand(string Name, string Script, int TimeoutSeco /// Flow Debugger (design 2026-09-05 §5.2): null leaves the stored value unchanged so an older CLI never resets it. public record UpdateApiMethodCommand(int ApiMethodId, string Script, int TimeoutSeconds, string? ParameterDefinitions, string? ReturnDefinition, bool? TraceAllCalls = null); public record DeleteApiMethodCommand(int ApiMethodId); +/// +/// Compiles the method's stored script on the receiving central node and returns the +/// diagnostics, without registering the handler or touching the known-bad cache +/// (ScadaBridge#75). Operators see the real compile error here; the data plane keeps +/// returning its generic message. +/// +/// The API method to validate. +public record ValidateApiMethodCommand(int ApiMethodId); +/// +/// Re-runs CompileAndRegister for the method's stored script on the receiving central +/// node, replacing its cached handler or known-bad record, without re-sending the entity +/// (ScadaBridge#75). Returns the diagnostics. +/// +/// The API method to recompile. +public record RecompileApiMethodCommand(int ApiMethodId); diff --git a/src/ZB.MOM.WW.ScadaBridge.Commons/Types/InboundApi/ApiMethodCompileResult.cs b/src/ZB.MOM.WW.ScadaBridge.Commons/Types/InboundApi/ApiMethodCompileResult.cs new file mode 100644 index 00000000..33821eb4 --- /dev/null +++ b/src/ZB.MOM.WW.ScadaBridge.Commons/Types/InboundApi/ApiMethodCompileResult.cs @@ -0,0 +1,25 @@ +namespace ZB.MOM.WW.ScadaBridge.Commons.Types.InboundApi; + +/// +/// Result of ValidateApiMethodCommand / RecompileApiMethodCommand +/// (ScadaBridge#75): the outcome of compiling an inbound API method's stored script on one +/// central node, with the compiler / trust-model diagnostics an operator needs. +/// +/// The API method id. +/// The API method name. +/// True when the script compiled. +/// The compile / trust-model errors; empty on success. +/// The central node that compiled it (compile state is per node), when known. +public sealed record ApiMethodCompileResult( + int ApiMethodId, string Name, bool Success, IReadOnlyList Errors, string? Node); + +/// +/// One central node's runtime compile state for an inbound API method (ScadaBridge#75), +/// judged against the method's current stored script. +/// +/// A handler compiled from the current script is cached on this node. +/// The current script failed to compile on this node; requests fast-fail. +/// The diagnostics of that failure, when . +/// The central node the state was read from, when known. +public sealed record ApiMethodCompileStatus( + bool IsCompiled, bool IsKnownBad, string? LastCompileError, string? Node); diff --git a/src/ZB.MOM.WW.ScadaBridge.InboundAPI/ApiMethodCompileState.cs b/src/ZB.MOM.WW.ScadaBridge.InboundAPI/ApiMethodCompileState.cs new file mode 100644 index 00000000..52776250 --- /dev/null +++ b/src/ZB.MOM.WW.ScadaBridge.InboundAPI/ApiMethodCompileState.cs @@ -0,0 +1,10 @@ +namespace ZB.MOM.WW.ScadaBridge.InboundAPI; + +/// +/// This node's compile state for one inbound API method, as reported by +/// (ScadaBridge#75). +/// +/// A handler compiled from the current script text is cached. +/// The current script text failed to compile; requests fast-fail without recompiling. +/// The joined diagnostics of that failure, when . +public sealed record ApiMethodCompileState(bool IsCompiled, bool IsKnownBad, string? LastCompileError); diff --git a/src/ZB.MOM.WW.ScadaBridge.InboundAPI/InboundScriptExecutor.cs b/src/ZB.MOM.WW.ScadaBridge.InboundAPI/InboundScriptExecutor.cs index 60247b00..d5f500ee 100644 --- a/src/ZB.MOM.WW.ScadaBridge.InboundAPI/InboundScriptExecutor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.InboundAPI/InboundScriptExecutor.cs @@ -57,7 +57,12 @@ public class InboundScriptExecutor // dropped — the cache is just a fast-fail optimisation; the per-request DB // lookup remains the correctness path. private const int KnownBadMethodsCap = 1000; - private readonly ConcurrentDictionary _knownBadMethods = new(); + private readonly ConcurrentDictionary _knownBadMethods = new(); + + // A known-bad record: the exact script text that failed and the diagnostics it failed + // with, so an operator can read them through the management surface (ScadaBridge#75) + // instead of the central log. + private sealed record KnownBadMethod(string Script, IReadOnlyList Errors); /// Diagnostic helper — whether a compiled handler is cached for the method. internal bool HasHandler(string methodName) => _scriptHandlers.ContainsKey(methodName); @@ -69,6 +74,31 @@ public class InboundScriptExecutor /// internal int KnownBadMethodCount => _knownBadMethods.Count; + /// + /// This node's compile state for , judged against the + /// method's CURRENT script text (ScadaBridge#75). A handler or failure record left over + /// from different script text describes an older revision, so it is reported as not yet + /// compiled — the next request (or a recompile) compiles the current text. The state is + /// per central node: each node compiles and caches independently. + /// + /// The method name. + /// The method's current (persisted) script text. + /// The compile state of the method on this node. + public ApiMethodCompileState GetCompileState(string methodName, string? currentScript) + { + var script = currentScript ?? string.Empty; + if (_knownBadMethods.TryGetValue(methodName, out var bad) + && string.Equals(bad.Script, script, StringComparison.Ordinal)) + { + return new ApiMethodCompileState(IsCompiled: false, IsKnownBad: true, string.Join("; ", bad.Errors)); + } + + // A null Script is the RegisterHandler test seam, which serves regardless of text. + var compiled = _scriptHandlers.TryGetValue(methodName, out var cached) + && (cached.Script is null || string.Equals(cached.Script, script, StringComparison.Ordinal)); + return new ApiMethodCompileState(IsCompiled: compiled, IsKnownBad: false, LastCompileError: null); + } + /// /// Records → in the /// known-bad-methods cache so the CURRENT failing script text short-circuits the @@ -79,18 +109,19 @@ public class InboundScriptExecutor /// (paying the cheap recompile next time rather than leaking memory under a /// unique-name flood). Records are cleared on a successful (re)compile. /// - private void TryRecordBadMethod(string methodName, string script) + private void TryRecordBadMethod(string methodName, string script, IReadOnlyList errors) { + var record = new KnownBadMethod(script, errors); // Overwrite an existing record (newer bad script wins) without counting // against the cap; only a genuinely new method name is gated by the cap. if (_knownBadMethods.ContainsKey(methodName)) { - _knownBadMethods[methodName] = script; + _knownBadMethods[methodName] = record; return; } if (_knownBadMethods.Count >= KnownBadMethodsCap) return; - _knownBadMethods.TryAdd(methodName, script); + _knownBadMethods.TryAdd(methodName, record); } private readonly IServiceProvider _serviceProvider; @@ -227,7 +258,7 @@ public class InboundScriptExecutor // through the capped TryRecordBadMethod helper so the cache // cannot grow without bound under a flood of unique method names. errors = compileErrors; - publish = () => TryRecordBadMethod(name, script ?? string.Empty); + publish = () => TryRecordBadMethod(name, script ?? string.Empty, compileErrors); return false; } @@ -562,20 +593,20 @@ public class InboundScriptExecutor if (cached == null) { // Fast-fail only when the CURRENT script text is the one already known bad. - if (_knownBadMethods.TryGetValue(method.Name, out var badScript) - && string.Equals(badScript, method.Script, StringComparison.Ordinal)) + if (_knownBadMethods.TryGetValue(method.Name, out var knownBad) + && string.Equals(knownBad.Script, method.Script, StringComparison.Ordinal)) return new InboundScriptResult(false, null, "Script compilation failed for this method", "SCRIPT_COMPILE_FAILED"); // Lazy compile (handles methods created after startup, and stale/known-bad // recompiles). Compile outside the cache so a failed compile is not stored, // then add so concurrent first-callers share a single handler instance. - var (compiled, _) = CompileSingleFlight(method); + var (compiled, compileErrors) = CompileSingleFlight(method); if (compiled == null) { // Cache the failure so the next request short-circuits above. // Routed through TryRecordBadMethod so the // cache is bounded under a flood of unique method names. - TryRecordBadMethod(method.Name, method.Script ?? string.Empty); + TryRecordBadMethod(method.Name, method.Script ?? string.Empty, compileErrors); return new InboundScriptResult(false, null, "Script compilation failed for this method", "SCRIPT_COMPILE_FAILED"); } diff --git a/src/ZB.MOM.WW.ScadaBridge.ManagementService/ApiMethodCompileOperations.cs b/src/ZB.MOM.WW.ScadaBridge.ManagementService/ApiMethodCompileOperations.cs new file mode 100644 index 00000000..77ceab10 --- /dev/null +++ b/src/ZB.MOM.WW.ScadaBridge.ManagementService/ApiMethodCompileOperations.cs @@ -0,0 +1,85 @@ +using Microsoft.Extensions.DependencyInjection; +using ZB.MOM.WW.ScadaBridge.Commons.Entities.InboundApi; +using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories; +using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services; +using ZB.MOM.WW.ScadaBridge.Commons.Types.InboundApi; +using ZB.MOM.WW.ScadaBridge.InboundAPI; + +namespace ZB.MOM.WW.ScadaBridge.ManagementService; + +/// +/// Operator access to inbound API method compilation (ScadaBridge#75): the real compile +/// diagnostics, this node's compile state, and a recompile that does not need the entity +/// re-sent. Shared by the handlers (CLI / Management API) and +/// the Central UI, which call it under the same role gate, so every surface reports the same +/// thing. Compile state is per central node — each node compiles and caches independently — +/// so every result names the node it came from. +/// +public static class ApiMethodCompileOperations +{ + /// + /// Compiles the method's stored script and returns the diagnostics. Nothing is registered + /// and the known-bad cache is untouched, so a validate never changes what requests see. + /// + /// The scoped service provider. + /// The API method id. + /// The compile outcome and its diagnostics. + public static async Task ValidateAsync(IServiceProvider sp, int apiMethodId) + { + var method = await LoadAsync(sp, apiMethodId); + var valid = RequireExecutor(sp).ValidateCompilation(method, out var errors); + return new ApiMethodCompileResult(method.Id, method.Name, valid, errors, NodeName(sp)); + } + + /// + /// Re-runs CompileAndRegister on the method's stored script: a success replaces the + /// cached handler and clears the known-bad record, a failure records the new diagnostics + /// (a previously working handler is never replaced by a failed compile, but the stored row + /// stays authoritative, see ). + /// The node's cached row is dropped too, so the next request reads the stored row. + /// Audited as Recompile. + /// + /// The scoped service provider. + /// The API method id. + /// The acting user, for the audit row. + /// The compile outcome and its diagnostics. + public static async Task RecompileAsync(IServiceProvider sp, int apiMethodId, string user) + { + var method = await LoadAsync(sp, apiMethodId); + var compiled = RequireExecutor(sp).CompileAndRegister(method, out var errors); + sp.GetService()?.Invalidate(method.Name); + + var result = new ApiMethodCompileResult(method.Id, method.Name, compiled, errors, NodeName(sp)); + await sp.GetRequiredService().LogAsync( + user, "Recompile", "ApiMethod", method.Id.ToString(), method.Name, + new { method.Id, method.Name, Compiled = compiled, ErrorCount = errors.Count, result.Node }); + await sp.GetRequiredService().SaveChangesAsync(); + return result; + } + + /// + /// This node's compile state for , or null on a host that + /// runs no inbound API executor. + /// + /// The service provider. + /// The API method, with its current stored script. + /// The compile state, or null. + public static ApiMethodCompileStatus? GetStatus(IServiceProvider sp, ApiMethod method) + { + var executor = sp.GetService(); + if (executor is null) + return null; + var state = executor.GetCompileState(method.Name, method.Script); + return new ApiMethodCompileStatus(state.IsCompiled, state.IsKnownBad, state.LastCompileError, NodeName(sp)); + } + + private static async Task LoadAsync(IServiceProvider sp, int apiMethodId) => + await sp.GetRequiredService().GetApiMethodByIdAsync(apiMethodId) + ?? throw new ManagementCommandException($"ApiMethod with ID {apiMethodId} not found."); + + private static InboundScriptExecutor RequireExecutor(IServiceProvider sp) => + sp.GetService() + ?? throw new ManagementCommandException("The inbound API script executor is not available on this node."); + + private static string? NodeName(IServiceProvider sp) => sp.GetService()?.NodeName; +} diff --git a/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs b/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs index ded272f4..fdebd1ed 100644 --- a/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs @@ -328,6 +328,9 @@ public class ManagementActor : ReceiveActor or CreateSharedSchemaCommand or UpdateSharedSchemaCommand or DeleteSharedSchemaCommand or CreateDatabaseConnectionDefCommand or UpdateDatabaseConnectionDefCommand or DeleteDatabaseConnectionDefCommand or CreateApiMethodCommand or UpdateApiMethodCommand or DeleteApiMethodCommand + // ScadaBridge#75: operator compile diagnostics + targeted recompile, the same + // authority as UpdateApiMethod (which recompiles as a side effect). + or ValidateApiMethodCommand or RecompileApiMethodCommand or CreateTemplateFolderCommand or RenameTemplateFolderCommand or MoveTemplateFolderCommand or ReorderTemplateFolderCommand or DeleteTemplateFolderCommand or MoveTemplateToFolderCommand @@ -526,6 +529,8 @@ public class ManagementActor : ReceiveActor CreateApiMethodCommand cmd => await HandleCreateApiMethod(sp, cmd, user.Username), UpdateApiMethodCommand cmd => await HandleUpdateApiMethod(sp, cmd, user.Username), DeleteApiMethodCommand cmd => await HandleDeleteApiMethod(sp, cmd, user.Username), + ValidateApiMethodCommand cmd => await ApiMethodCompileOperations.ValidateAsync(sp, cmd.ApiMethodId), + RecompileApiMethodCommand cmd => await ApiMethodCompileOperations.RecompileAsync(sp, cmd.ApiMethodId, user.Username), // Security ListRoleMappingsCommand => await HandleListRoleMappings(sp), @@ -3802,7 +3807,26 @@ public class ManagementActor : ReceiveActor private static async Task HandleGetApiMethod(IServiceProvider sp, GetApiMethodCommand cmd) { var repo = sp.GetRequiredService(); - return await repo.GetApiMethodByIdAsync(cmd.ApiMethodId); + var method = await repo.GetApiMethodByIdAsync(cmd.ApiMethodId); + if (method is null) + return null; + // ScadaBridge#75: the entity fields stay top-level (additive), plus this node's + // compile state so an operator can see why a method fails without firing it. + var status = ApiMethodCompileOperations.GetStatus(sp, method); + return new + { + method.Id, + method.Name, + method.Script, + method.ParameterDefinitions, + method.ReturnDefinition, + method.TimeoutSeconds, + method.TraceAllCalls, + status?.IsCompiled, + status?.IsKnownBad, + status?.LastCompileError, + CompileStateNode = status?.Node, + }; } private static async Task HandleCreateApiMethod(IServiceProvider sp, CreateApiMethodCommand cmd, string user) diff --git a/tests/ZB.MOM.WW.ScadaBridge.InboundAPI.Tests/InboundScriptExecutorTests.cs b/tests/ZB.MOM.WW.ScadaBridge.InboundAPI.Tests/InboundScriptExecutorTests.cs index 1570bcd9..4d47c438 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.InboundAPI.Tests/InboundScriptExecutorTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.InboundAPI.Tests/InboundScriptExecutorTests.cs @@ -712,6 +712,31 @@ public class InboundScriptExecutorTests Assert.True(_executor.CompileAndRegister(good)); } + /// + /// ScadaBridge#75: a failure on the data-plane (lazy-compile) path is observable per method + /// with its real diagnostic while the caller only gets the generic message; the state is + /// judged against the current script text and clears on a successful compile. + /// + /// A task that represents the asynchronous operation. + [Fact] + public async Task CompileState_ReportsTheLazyCompileFailureWithItsDiagnostic() + { + var method = new ApiMethod("diag", "return doesNotExist;") { Id = 1, TimeoutSeconds = 10 }; + Assert.Equal(new ApiMethodCompileState(false, false, null), _executor.GetCompileState("diag", method.Script)); + + var result = await _executor.ExecuteAsync(method, new Dictionary(), _route, TimeSpan.FromSeconds(10)); + + Assert.Equal("Script compilation failed for this method", result.ErrorMessage); + var state = _executor.GetCompileState("diag", method.Script); + Assert.True(state.IsKnownBad); + Assert.Contains("doesNotExist", state.LastCompileError); + // A different script text is a different revision: the record no longer describes it. + Assert.False(_executor.GetCompileState("diag", "return 1;").IsKnownBad); + + Assert.True(_executor.CompileAndRegister(new ApiMethod("diag", "return 1;") { Id = 1, TimeoutSeconds = 10 })); + Assert.Equal(new ApiMethodCompileState(true, false, null), _executor.GetCompileState("diag", "return 1;")); + } + // --- _knownBadMethods must be bounded so a spam attack of // unique method names cannot grow the cache without bound. --- diff --git a/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ApiMethodCompileDiagnosticsTests.cs b/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ApiMethodCompileDiagnosticsTests.cs new file mode 100644 index 00000000..201dffd8 --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ApiMethodCompileDiagnosticsTests.cs @@ -0,0 +1,135 @@ +using System.Text.Json; +using Akka.Actor; +using Akka.TestKit.Xunit2; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging.Abstractions; +using NSubstitute; +using ZB.MOM.WW.ScadaBridge.Commons.Entities.InboundApi; +using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories; +using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services; +using ZB.MOM.WW.ScadaBridge.Commons.Messages.Management; +using ZB.MOM.WW.ScadaBridge.InboundAPI; + +namespace ZB.MOM.WW.ScadaBridge.ManagementService.Tests; + +/// +/// ScadaBridge#75: an inbound method whose script fails to compile answers callers with a +/// generic message and its real diagnostic used to reach only the central log. The +/// management surface now returns the diagnostics (ValidateApiMethod), shows the +/// node's compile state on GetApiMethod, and can clear a stale failure without a +/// full-replace update (RecompileApiMethod). +/// +public class ApiMethodCompileDiagnosticsTests : TestKit +{ + private const string BrokenScript = "return doesNotExist;"; + private readonly ServiceCollection _services = new ServiceCollection().AddTestConfigurationTransaction(); + private readonly IInboundApiRepository _apiRepo = Substitute.For(); + private readonly IAuditService _audit = Substitute.For(); + private readonly InboundScriptExecutor _executor = + new(NullLogger.Instance, Substitute.For()); + private readonly ApiMethod _method = new("Broken", BrokenScript) { Id = 5, TimeoutSeconds = 10 }; + + /// Initializes the harness with a real executor and a stubbed repository. + public ApiMethodCompileDiagnosticsTests() + { + _services.AddScoped(_ => _apiRepo); + _services.AddScoped(_ => _audit); + _services.AddSingleton(_executor); + _apiRepo.GetApiMethodByIdAsync(5, Arg.Any()).Returns(_method); + _apiRepo.SaveChangesAsync(Arg.Any()).Returns(1); + } + + private JsonElement Send(object command, string role = "Designer") + { + var actor = Sys.ActorOf(Props.Create(() => new ManagementActor( + _services.BuildServiceProvider(), NullLogger.Instance))); + actor.Tell(new ManagementEnvelope( + new AuthenticatedUser("designer", "Designer", new[] { role }, Array.Empty()), + command, Guid.NewGuid().ToString("N"))); + var response = ExpectMsg(TimeSpan.FromSeconds(30)); + return JsonDocument.Parse(response.JsonData).RootElement.Clone(); + } + + /// + /// After the data plane recorded a compile failure, GetApiMethod reports it with the real + /// compiler diagnostic, alongside the unchanged entity fields. + /// + [Fact] + public void Get_AfterACompileFailure_ReportsKnownBadWithTheDiagnostic() + { + Assert.False(_executor.CompileAndRegister(_method)); + + var result = Send(new GetApiMethodCommand(5), role: "Viewer"); + + Assert.Equal("Broken", result.GetProperty("name").GetString()); + Assert.Equal(BrokenScript, result.GetProperty("script").GetString()); + Assert.True(result.GetProperty("isKnownBad").GetBoolean()); + Assert.False(result.GetProperty("isCompiled").GetBoolean()); + Assert.Contains("doesNotExist", result.GetProperty("lastCompileError").GetString()); + } + + /// Validate returns the diagnostics and leaves the handler cache untouched. + [Fact] + public void Validate_ReturnsDiagnostics_WithoutRecordingAFailure() + { + var result = Send(new ValidateApiMethodCommand(5)); + + Assert.False(result.GetProperty("success").GetBoolean()); + Assert.Contains(result.GetProperty("errors").EnumerateArray(), + e => e.GetString()!.Contains("doesNotExist", StringComparison.Ordinal)); + var state = _executor.GetCompileState("Broken", BrokenScript); + Assert.False(state.IsKnownBad); + Assert.False(state.IsCompiled); + } + + /// + /// A failure recorded for unchanged script text is cleared by a recompile once the cause + /// outside the script is gone — here modelled by the stored row now compiling — without + /// re-sending the entity. The recompile is audited. + /// + [Fact] + public void Recompile_ClearsAStaleFailure_AndIsAudited() + { + Assert.False(_executor.CompileAndRegister(_method)); + _method.Script = "return 1;"; + + var result = Send(new RecompileApiMethodCommand(5)); + + Assert.True(result.GetProperty("success").GetBoolean()); + Assert.Empty(result.GetProperty("errors").EnumerateArray()); + var state = _executor.GetCompileState("Broken", "return 1;"); + Assert.True(state.IsCompiled); + Assert.False(state.IsKnownBad); + _audit.Received(1).LogAsync("designer", "Recompile", "ApiMethod", "5", "Broken", + Arg.Any(), Arg.Any()); + } + + /// A recompile of a still-broken script records the new diagnostics. + [Fact] + public void Recompile_StillBroken_RecordsTheDiagnostics() + { + var result = Send(new RecompileApiMethodCommand(5)); + + Assert.False(result.GetProperty("success").GetBoolean()); + var state = _executor.GetCompileState("Broken", BrokenScript); + Assert.True(state.IsKnownBad); + Assert.Contains("doesNotExist", state.LastCompileError); + } + + /// Validate and recompile are Designer operations, like UpdateApiMethod. + /// The command name to send. + [Theory] + [InlineData("Validate")] + [InlineData("Recompile")] + public void ValidateAndRecompile_RequireDesigner(string command) + { + var actor = Sys.ActorOf(Props.Create(() => new ManagementActor( + _services.BuildServiceProvider(), NullLogger.Instance))); + object message = command == "Validate" ? new ValidateApiMethodCommand(5) : new RecompileApiMethodCommand(5); + actor.Tell(new ManagementEnvelope( + new AuthenticatedUser("viewer", "Viewer", new[] { "Viewer" }, Array.Empty()), + message, Guid.NewGuid().ToString("N"))); + + ExpectMsg(TimeSpan.FromSeconds(10)); + } +} diff --git a/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/RequiredRoleMatrixTests.cs b/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/RequiredRoleMatrixTests.cs index 64e0461c..3c01ed27 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/RequiredRoleMatrixTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/RequiredRoleMatrixTests.cs @@ -116,6 +116,9 @@ public class RequiredRoleMatrixTests ["CreateApiMethod"] = [Roles.Designer], ["UpdateApiMethod"] = [Roles.Designer], ["DeleteApiMethod"] = [Roles.Designer], + // ScadaBridge#75: compile diagnostics + targeted recompile — UpdateApiMethod's authority. + ["ValidateApiMethod"] = [Roles.Designer], + ["RecompileApiMethod"] = [Roles.Designer], ["CreateTemplateFolder"] = [Roles.Designer], ["RenameTemplateFolder"] = [Roles.Designer], ["MoveTemplateFolder"] = [Roles.Designer],