diff --git a/docs/components/InboundAPI.md b/docs/components/InboundAPI.md index a661c5e1..fdb4563b 100644 --- a/docs/components/InboundAPI.md +++ b/docs/components/InboundAPI.md @@ -223,7 +223,7 @@ The `Route.To().Call()` inherits the method-level timeout automatically. A scrip ### Startup compilation and hot-reload -At startup the Host loads all `ApiMethod` rows from the configuration database and calls `CompileAndRegister` on each. After a method is updated via the Management API or CLI, the Management Service calls `CompileAndRegister` again — the updated script takes effect on the next request, with no node restart. Methods created after startup compile lazily on first invocation. Scripts modified directly in the database do not take effect until the next node restart; always use the Management API, CLI, or Central UI. +At startup the Host loads all `ApiMethod` rows from the configuration database and calls `CompileAndRegister` on each. After a method is updated via the Management API or CLI, the Management Service calls `CompileAndRegister` again — the updated script takes effect on the next request, with no node restart. Methods created after startup compile lazily on first invocation. Each request resolves the method's stored row (through `ApiMethodCache`, `InboundApi:ApiMethodCacheTtl`, default 5 s) and revision-checks the cached handler against its script text, recompiling on a mismatch — so a script edited directly in the database takes effect on each node within that TTL, without a restart. Management API, CLI, Central UI and bundle-import changes also drop the method's cached row and handler on **both** central nodes at once: the node that made the change directly, the peer through `InboundApiMethodInvalidated` on the central cache-invalidation topic (#103, see [Host](./Host.md)). ## Configuration @@ -252,11 +252,11 @@ The inbound body-capture cap for audit is configured separately under `AuditLog: ### A method always returns 500 after a script update -The in-memory handler cache still holds the previous compiled delegate. If the update went through the Management API or CLI, `CompileAndRegister` should have been called automatically and the new script should be active on the next request. If the script was edited directly in the database, the cached delegate is stale until the next node restart. Check the `ScadaBridge.InboundAPI` log category for a `"script compilation failed"` or `"trust model violation"` warning to distinguish a compile error from a routing failure. +The in-memory handler cache still holds the previous compiled delegate. If the update went through the Management API or CLI, `CompileAndRegister` should have been called automatically and the new script should be active on the next request. If the script was edited directly in the database, the per-request revision check recompiles it once the node's cached `ApiMethod` row expires (`ApiMethodCacheTtl`, default 5 s) — no restart is needed. Check the `ScadaBridge.InboundAPI` log category for a `"script compilation failed"` or `"trust model violation"` warning to distinguish a compile error from a routing failure. ### 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`. 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. +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). `get` and `validate` report the answering central node; a `recompile` also drops the method's handler and known-bad record on the peer central node (#103), which compiles the current script on its next request. 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/plans/2026-11-09-central-cache-staleness-audit.md b/docs/plans/2026-11-09-central-cache-staleness-audit.md index c992794d..a167d526 100644 --- a/docs/plans/2026-11-09-central-cache-staleness-audit.md +++ b/docs/plans/2026-11-09-central-cache-staleness-audit.md @@ -22,7 +22,7 @@ node after a change is made through the other one. Same defect class as #83 | Cache | Holds | Local invalidation | Peer staleness bound | Verdict | |---|---|---|---|---| | `InboundScriptExecutor` compiled handlers (InboundAPI) | Compiled method delegates | `InvalidateMethod` on delete; change bus on import | **None** — every request revision-checks the cached handler against the resolved `ApiMethod.Script` and recompiles on mismatch | OK | -| `ApiMethodCache` (InboundAPI) | `ApiMethod` rows, incl. not-found | Change bus (bundle import); **management create/update/delete — added by #85** | `InboundApi:ApiMethodCacheTtl`, default **5 s** | OK (local gap fixed, see below) | +| `ApiMethodCache` (InboundAPI) | `ApiMethod` rows, incl. not-found | Change bus (bundle import); **management create/update/delete — added by #85** | Change through the other node: **none** (`InboundApiMethodInvalidated`, #103); direct SQL: `InboundApi:ApiMethodCacheTtl`, default **5 s** | OK (local gap fixed by #85, peer by #103) | | `ExternalDefinitionLookupCache` (ESG, central only) | External systems / methods / DB connections (found only) | 9 ManagementActor handlers (#65); bundle import (#83) | Edit or import through the other node: **none** (`ExternalDefinitionsInvalidated` on `central-cache-invalidation`, #100); direct SQL: `ExternalSystemGateway:DefinitionCacheTtl`, **10 s** on central | Fixed — #100 item 3 (see D3) | | `RoleMappingCache` (Security) — the cache at `ManagementActor` `InvalidateRoleMappingCacheAfterCommit` | LDAP group → role map + Deployer site scopes | Mapping / scope-rule commands, post-commit | **None** (`RoleMappingsInvalidated` on `central-cache-invalidation`, #100); TTL `Security:RoleMappingCacheSeconds` (**30 s**) only if the broadcast is lost | Fixed — #100 item 3 (see D1) | | `PendingUpdatesCache` (DeploymentManager, #91) | Fleet pending-updates snapshot | Deployment finished on this node | `DeploymentManager:PendingUpdatesCacheTtl`, default **60 s** | OK — display only | @@ -84,7 +84,9 @@ messages (`ICentralCacheInvalidation`: `SitePskInvalidated`, `RoleMappingsInvali The #96 topic was migrated onto it: `SitePskInvalidated` travels on the shared topic, and for one release is also published bare on `site-psk-invalidation` (still subscribed), so a mixed-version central pair keeps working. Each envelope carries the publisher's origin id and the subscriber -skips its own. Publishers: ManagementActor post-commit (role mappings, external definitions), -the Transport bundle importer (external definitions) and the PSK invalidator. Every cache keeps its TTL / revision check as the fallback for a +skips its own. Publishers: ManagementActor post-commit (role mappings, external definitions, +API methods), the Transport bundle importer (external definitions, API methods), the API-method +recompile (#103: a recompile now clears a stale known-bad record on the peer too) and the PSK +invalidator. Every cache keeps its TTL / revision check as the fallback for a lost broadcast or an out-of-band (SQL) edit. Design: Component-Host.md → *Central cache invalidation*; tests include a real two-node cluster (`CentralCacheInvalidationTwoNodeTests`). diff --git a/docs/requirements/Component-Host.md b/docs/requirements/Component-Host.md index 351a6668..9ab1f36f 100644 --- a/docs/requirements/Component-Host.md +++ b/docs/requirements/Component-Host.md @@ -352,5 +352,5 @@ built)* section for where the shipped code deviates from the original design). - **Inbound method warm-up is a hosted service** (`InboundMethodWarmupService`, registered after `AkkaHostedService`), not a pre-`RunAsync` loop — see Component-InboundAPI.md. - **`SitePskProvider` distinguishes a secret-store fault from a missing secret.** A store fault logs a Warning with the exception and throws `SecretStoreUnavailableException` (not cached); only a genuinely absent/empty/tombstoned secret produces the "seed `SB-GRPC-PSK-{site}`" message. - **`SitePskProvider`'s cache follows an in-place rotation (Gitea #96).** Central replaces the secrets library's `ISecretCacheInvalidator` with `SitePskRotationInvalidator` (`AddSitePskRotationInvalidation`), which drops the site's cached key when `SB-GRPC-PSK-{site}` is written and publishes `SitePskInvalidated` to the peer over the central cache-invalidation topic (below). A site's auth rejection and a `SitePsks` reload also drop the cache — see Component-Communication.md → *Authentication*. -- **Central cache invalidation (ScadaBridge#100 item 3).** One `DistributedPubSub` topic, `central-cache-invalidation`, carries typed `ICentralCacheInvalidation` messages (Commons `Messages/Communication/CentralCacheInvalidations.cs`) between the two central nodes, each wrapped in a `CentralCacheInvalidationEnvelope` stamped with the publishing process's origin id. `AddCentralCacheInvalidation` registers `CentralCacheInvalidationBus` as `ICentralCacheInvalidationPublisher`; `AkkaHostedService.RegisterCentralActors` gives it the mediator and starts `CentralCacheInvalidationSubscriber`, which skips its own node's envelopes and hands the peer's to every registered `ICentralCacheInvalidationHandler`: `SitePskRotationInvalidator` (`SitePskInvalidated`, #96), `RoleMappingCache` (`RoleMappingsInvalidated`, audit D1), and `ExternalDefinitionLookupCache` (`ExternalDefinitionsInvalidated`, audit D3 / #83). Publishers invalidate locally first, post-commit, then publish; delivery is best-effort, so each cache's TTL or revision check stays the fallback. Migration: the #96 topic `site-psk-invalidation` is still published to (bare `SitePskInvalidated`) and subscribed, so a briefly mixed-version central pair keeps exchanging PSK invalidations — remove it once every central node runs this build. +- **Central cache invalidation (ScadaBridge#100 item 3).** One `DistributedPubSub` topic, `central-cache-invalidation`, carries typed `ICentralCacheInvalidation` messages (Commons `Messages/Communication/CentralCacheInvalidations.cs`) between the two central nodes, each wrapped in a `CentralCacheInvalidationEnvelope` stamped with the publishing process's origin id. `AddCentralCacheInvalidation` registers `CentralCacheInvalidationBus` as `ICentralCacheInvalidationPublisher`; `AkkaHostedService.RegisterCentralActors` gives it the mediator and starts `CentralCacheInvalidationSubscriber`, which skips its own node's envelopes and hands the peer's to every registered `ICentralCacheInvalidationHandler`: `SitePskRotationInvalidator` (`SitePskInvalidated`, #96), `RoleMappingCache` (`RoleMappingsInvalidated`, audit D1), `ExternalDefinitionLookupCache` (`ExternalDefinitionsInvalidated`, audit D3 / #83) and the Inbound API's `InboundApiCacheInvalidationHandler` (`InboundApiMethodInvalidated` — compiled handler, known-bad record and cached row of one method, #103). Publishers invalidate locally first, post-commit, then publish; delivery is best-effort, so each cache's TTL or revision check stays the fallback. Migration: the #96 topic `site-psk-invalidation` is still published to (bare `SitePskInvalidated`) and subscribed, so a briefly mixed-version central pair keeps exchanging PSK invalidations — remove it once every central node runs this build. - Central registers `ExternalSystemGatewayOptions.DefinitionCacheTtl = 10 s` ahead of `AddExternalSystemGateway()` so an explicit config value wins; sites leave it off. diff --git a/docs/requirements/Component-InboundAPI.md b/docs/requirements/Component-InboundAPI.md index 2d226826..80b67485 100644 --- a/docs/requirements/Component-InboundAPI.md +++ b/docs/requirements/Component-InboundAPI.md @@ -181,7 +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. +- **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, but a change does not stay local: create / update / delete / recompile through the management surface (and an API method written by a bundle import) publishes `InboundApiMethodInvalidated` on the central cache-invalidation topic, and the peer central node drops that method's compiled handler, known-bad record and cached row (`InboundApiCacheInvalidationHandler`, #103) — so a recompile that clears a stale compile failure clears it on both nodes. The peer compiles lazily on its next request. > **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/src/ZB.MOM.WW.ScadaBridge.InboundAPI/InboundApiCacheInvalidationHandler.cs b/src/ZB.MOM.WW.ScadaBridge.InboundAPI/InboundApiCacheInvalidationHandler.cs new file mode 100644 index 00000000..f3c64c40 --- /dev/null +++ b/src/ZB.MOM.WW.ScadaBridge.InboundAPI/InboundApiCacheInvalidationHandler.cs @@ -0,0 +1,44 @@ +using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services; +using ZB.MOM.WW.ScadaBridge.Commons.Messages.Communication; + +namespace ZB.MOM.WW.ScadaBridge.InboundAPI; + +/// +/// Applies a peer central node's to this node +/// (ScadaBridge#103): drops the method's compiled handler, its known-bad compile record and its +/// cached ApiMethod row, so the next request on this node reads the stored row and compiles +/// it afresh. +/// +/// +/// Compile state (#75) is per central node. Without this, a recompile on one node cleared the +/// known-bad record only there: the peer kept refusing the method with +/// SCRIPT_COMPILE_FAILED while its record matched the unchanged script text, because the +/// per-request revision check only notices a changed script. The peer does not recompile eagerly; +/// its next request (or its own recompile) compiles the current text. +/// +public sealed class InboundApiCacheInvalidationHandler : ICentralCacheInvalidationHandler +{ + private readonly InboundScriptExecutor _executor; + private readonly ApiMethodCache _methodCache; + + /// Creates the handler. + /// This node's inbound script executor. + /// This node's API-method row cache. + public InboundApiCacheInvalidationHandler(InboundScriptExecutor executor, ApiMethodCache methodCache) + { + ArgumentNullException.ThrowIfNull(executor); + ArgumentNullException.ThrowIfNull(methodCache); + _executor = executor; + _methodCache = methodCache; + } + + /// + public void Apply(ICentralCacheInvalidation invalidation) + { + if (invalidation is InboundApiMethodInvalidated { MethodName: { Length: > 0 } name }) + { + _executor.InvalidateMethod(name); + _methodCache.Invalidate(name); + } + } +} diff --git a/src/ZB.MOM.WW.ScadaBridge.InboundAPI/ServiceCollectionExtensions.cs b/src/ZB.MOM.WW.ScadaBridge.InboundAPI/ServiceCollectionExtensions.cs index 2ccf6406..3b9d4087 100644 --- a/src/ZB.MOM.WW.ScadaBridge.InboundAPI/ServiceCollectionExtensions.cs +++ b/src/ZB.MOM.WW.ScadaBridge.InboundAPI/ServiceCollectionExtensions.cs @@ -1,5 +1,7 @@ using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.DependencyInjection.Extensions; using Microsoft.Extensions.Options; +using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services; namespace ZB.MOM.WW.ScadaBridge.InboundAPI; @@ -50,6 +52,10 @@ public static class ServiceCollectionExtensions // and the executor's per-request revision check alone keeps handlers fresh. services.AddHostedService(); + // ScadaBridge#103: a create / update / delete / recompile made through the other central + // node drops this node's handler, known-bad record and cached row for the method. + services.TryAddEnumerable(ServiceDescriptor.Singleton()); + return services; } } diff --git a/src/ZB.MOM.WW.ScadaBridge.ManagementService/ApiMethodCompileOperations.cs b/src/ZB.MOM.WW.ScadaBridge.ManagementService/ApiMethodCompileOperations.cs index 77ceab10..74dac910 100644 --- a/src/ZB.MOM.WW.ScadaBridge.ManagementService/ApiMethodCompileOperations.cs +++ b/src/ZB.MOM.WW.ScadaBridge.ManagementService/ApiMethodCompileOperations.cs @@ -2,6 +2,7 @@ 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.Messages.Communication; using ZB.MOM.WW.ScadaBridge.Commons.Types.InboundApi; using ZB.MOM.WW.ScadaBridge.InboundAPI; @@ -13,7 +14,8 @@ namespace ZB.MOM.WW.ScadaBridge.ManagementService; /// 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. +/// so every result names the node it came from. A recompile also drops the method's compile +/// state on the peer central node (ScadaBridge#103), which recompiles on its next request. /// public static class ApiMethodCompileOperations { @@ -36,7 +38,9 @@ public static class ApiMethodCompileOperations /// 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. + /// The node's cached row is dropped too, so the next request reads the stored row, and the + /// peer central node is told to drop its handler, known-bad record and cached row for the + /// method (ScadaBridge#103), so a recompile clears a stale compile failure on both nodes. /// Audited as Recompile. /// /// The scoped service provider. @@ -48,6 +52,7 @@ public static class ApiMethodCompileOperations var method = await LoadAsync(sp, apiMethodId); var compiled = RequireExecutor(sp).CompileAndRegister(method, out var errors); sp.GetService()?.Invalidate(method.Name); + sp.GetService()?.Publish(new InboundApiMethodInvalidated(method.Name)); var result = new ApiMethodCompileResult(method.Id, method.Name, compiled, errors, NodeName(sp)); await sp.GetRequiredService().LogAsync( diff --git a/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs b/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs index 9ffe4ef0..bac5f1c2 100644 --- a/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs @@ -3915,15 +3915,24 @@ public class ManagementActor : ReceiveActor /// ApiMethodCacheTtl: a just-deleted method stayed callable (the executor recompiled /// it from the cached row), and a cached not-found answered 404 for a just-created one. /// Queued post-commit, like the role-mapping invalidation, so a concurrent request cannot - /// re-cache the pre-change row. The peer central node still converges within the TTL - /// (default 5 s) — see docs/plans/2026-11-09-central-cache-staleness-audit.md. + /// re-cache the pre-change row. The peer central node is told with an + /// on the central cache-invalidation topic + /// (ScadaBridge#103), which drops its row, compiled handler and known-bad record for the + /// method; the TTL (default 5 s) is only the fallback — see + /// docs/plans/2026-11-09-central-cache-staleness-audit.md. /// private static Task InvalidateApiMethodCacheAfterCommit(IServiceProvider sp, string name) { var cache = sp.GetService(); - return cache is null + var peers = sp.GetService(); + return cache is null && peers is null ? Task.CompletedTask - : AfterConfigurationCommit(sp, () => { cache.Invalidate(name); return Task.CompletedTask; }); + : AfterConfigurationCommit(sp, () => + { + cache?.Invalidate(name); + peers?.Publish(new InboundApiMethodInvalidated(name)); + return Task.CompletedTask; + }); } /// diff --git a/src/ZB.MOM.WW.ScadaBridge.Transport/Import/BundleImporter.cs b/src/ZB.MOM.WW.ScadaBridge.Transport/Import/BundleImporter.cs index dd16f212..c932cc44 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Transport/Import/BundleImporter.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Transport/Import/BundleImporter.cs @@ -1399,6 +1399,12 @@ public sealed class BundleImporter : IBundleImporter => resolutionMap != null && ResolveOrDefault(resolutionMap, entityType, name).Action == ResolutionAction.Skip; + /// + /// How matches override names to template + /// members: ordinal, exactly as the flattener does (ScadaBridge#103). + /// + private static readonly StringComparer LockedOverrideNameComparer = StringComparer.Ordinal; + /// /// Best-effort scan for instance overrides that target a LOCKED template member. /// The flattener drops such overrides (an override on a locked attribute / alarm / @@ -1416,6 +1422,16 @@ public sealed class BundleImporter : IBundleImporter /// instance's template is resolved from the bundle DTO (the version about to be /// written) first, else the pre-existing target template. /// + /// + /// Member names match EXACTLY (, ordinal), as the + /// flattener does: FlatteningService.ApplyInstanceOverrides / + /// ApplyInstanceAlarmOverrides / ApplyInstanceNativeAlarmSourceOverrides look + /// the override name up in ordinal-keyed dictionaries (ScadaBridge#103 decision). An override + /// whose name differs from a locked member only by case therefore never reaches that member's + /// lock check — the flattener drops it as unresolved — so a "targets a LOCKED member" warning + /// for it would be false. The SQL indexes on member names being case-insensitive does not + /// change this: they stop two members differing only by case, not a case-mismatched override. + /// /// private static IReadOnlyList<(string Instance, string Message)> CollectLockedOverrideWarnings( BundleContentDto content, @@ -1439,15 +1455,15 @@ public sealed class BundleImporter : IBundleImporter HashSet lockedSources; if (bundleTemplates.TryGetValue(inst.TemplateName, out var dto)) { - lockedAttrs = new(dto.Attributes.Where(a => a.IsLocked).Select(a => a.Name), StringComparer.Ordinal); - lockedAlarms = new(dto.Alarms.Where(a => a.IsLocked).Select(a => a.Name), StringComparer.Ordinal); - lockedSources = new(dto.NativeAlarmSources.Where(s => s.IsLocked).Select(s => s.Name), StringComparer.Ordinal); + lockedAttrs = new(dto.Attributes.Where(a => a.IsLocked).Select(a => a.Name), LockedOverrideNameComparer); + lockedAlarms = new(dto.Alarms.Where(a => a.IsLocked).Select(a => a.Name), LockedOverrideNameComparer); + lockedSources = new(dto.NativeAlarmSources.Where(s => s.IsLocked).Select(s => s.Name), LockedOverrideNameComparer); } else if (targetByName.TryGetValue(inst.TemplateName, out var tgt)) { - lockedAttrs = new(tgt.Attributes.Where(a => a.IsLocked).Select(a => a.Name), StringComparer.Ordinal); - lockedAlarms = new(tgt.Alarms.Where(a => a.IsLocked).Select(a => a.Name), StringComparer.Ordinal); - lockedSources = new(tgt.NativeAlarmSources.Where(s => s.IsLocked).Select(s => s.Name), StringComparer.Ordinal); + lockedAttrs = new(tgt.Attributes.Where(a => a.IsLocked).Select(a => a.Name), LockedOverrideNameComparer); + lockedAlarms = new(tgt.Alarms.Where(a => a.IsLocked).Select(a => a.Name), LockedOverrideNameComparer); + lockedSources = new(tgt.NativeAlarmSources.Where(s => s.IsLocked).Select(s => s.Name), LockedOverrideNameComparer); } else { @@ -2528,7 +2544,7 @@ public sealed class BundleImporter : IBundleImporter private void PublishScriptArtifactChanges(IReadOnlyList resolutions) { - if (_scriptArtifactChangeBus is null) + if (_scriptArtifactChangeBus is null && _cacheInvalidationPublisher is null) { return; } @@ -2560,11 +2576,21 @@ public sealed class BundleImporter : IBundleImporter return; } - _scriptArtifactChangeBus!.Publish(new ScriptArtifactsChanged( + _scriptArtifactChangeBus?.Publish(new ScriptArtifactsChanged( ArtifactKind: kind, Names: names, Source: "BundleImport", OccurredAtUtc: _timeProvider.GetUtcNow())); + + // ScadaBridge#103: the node-local bus above only reaches this node's executor; the + // peer central node is told per imported API method too. + if (kind == ScriptArtifactKinds.ApiMethod && _cacheInvalidationPublisher is not null) + { + foreach (var name in names) + { + _cacheInvalidationPublisher.Publish(new InboundApiMethodInvalidated(name)); + } + } } } diff --git a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/CompositionRootTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/CompositionRootTests.cs index 9c8d7e32..6d810e31 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/CompositionRootTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/CompositionRootTests.cs @@ -436,6 +436,7 @@ public class CentralCompositionRootTests : IDisposable Assert.Contains(typeof(SitePskRotationInvalidator), handlers); Assert.Contains(typeof(ZB.MOM.WW.ScadaBridge.Security.RoleMappingCache), handlers); Assert.Contains(typeof(ZB.MOM.WW.ScadaBridge.ExternalSystemGateway.ExternalDefinitionLookupCache), handlers); + Assert.Contains(typeof(ZB.MOM.WW.ScadaBridge.InboundAPI.InboundApiCacheInvalidationHandler), handlers); Assert.Equal(handlers.Count, handlers.Distinct().Count()); } diff --git a/tests/ZB.MOM.WW.ScadaBridge.InboundAPI.Tests/InboundApiCacheInvalidationHandlerTests.cs b/tests/ZB.MOM.WW.ScadaBridge.InboundAPI.Tests/InboundApiCacheInvalidationHandlerTests.cs new file mode 100644 index 00000000..fb1c8f33 --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.InboundAPI.Tests/InboundApiCacheInvalidationHandlerTests.cs @@ -0,0 +1,46 @@ +using Microsoft.Extensions.Logging.Abstractions; +using NSubstitute; +using ZB.MOM.WW.ScadaBridge.Commons.Entities.InboundApi; +using ZB.MOM.WW.ScadaBridge.Commons.Messages.Communication; + +namespace ZB.MOM.WW.ScadaBridge.InboundAPI.Tests; + +/// +/// ScadaBridge#103: a peer central node's drops this +/// node's compiled handler, known-bad record and cached row for that method only. +/// +public class InboundApiCacheInvalidationHandlerTests +{ + private readonly InboundScriptExecutor _executor = + new(NullLogger.Instance, Substitute.For()); + private readonly ApiMethodCache _cache = new(TimeSpan.FromHours(1)); + + private async Task Refetches(string name) + { + var fetched = false; + await _cache.GetOrFetchAsync(name, _ => { fetched = true; return Task.FromResult(null); }, CancellationToken.None); + return fetched; + } + + /// The named method's known-bad record, handler and row are dropped; others are kept. + /// A task that represents the asynchronous operation. + [Fact] + public async Task Apply_DropsOnlyTheNamedMethod() + { + var bad = new ApiMethod("Bad", "return doesNotExist;"); + var good = new ApiMethod("Good", "return 1;"); + Assert.False(_executor.CompileAndRegister(bad)); + Assert.True(_executor.CompileAndRegister(good)); + await _cache.GetOrFetchAsync("Bad", _ => Task.FromResult(bad), CancellationToken.None); + await _cache.GetOrFetchAsync("Good", _ => Task.FromResult(good), CancellationToken.None); + + var handler = new InboundApiCacheInvalidationHandler(_executor, _cache); + handler.Apply(new InboundApiMethodInvalidated("Bad")); + handler.Apply(new RoleMappingsInvalidated()); // not ours: ignored + + Assert.False(_executor.GetCompileState("Bad", bad.Script).IsKnownBad); + Assert.True(await Refetches("Bad")); + Assert.True(_executor.GetCompileState("Good", good.Script).IsCompiled); + Assert.False(await Refetches("Good")); + } +} diff --git a/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ApiMethodCacheInvalidationTests.cs b/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ApiMethodCacheInvalidationTests.cs index f1f3845b..e15eab64 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ApiMethodCacheInvalidationTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ApiMethodCacheInvalidationTests.cs @@ -6,6 +6,7 @@ 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.Communication; using ZB.MOM.WW.ScadaBridge.Commons.Messages.Management; using ZB.MOM.WW.ScadaBridge.InboundAPI; @@ -24,6 +25,7 @@ public class ApiMethodCacheInvalidationTests : TestKit // A TTL far longer than the test, so only an explicit invalidation can clear an entry. private readonly ApiMethodCache _cache = new(TimeSpan.FromHours(1)); + private readonly PeerCacheInvalidationPublisher _publisher = new(); /// Initializes the harness with a long-TTL method cache. public ApiMethodCacheInvalidationTests() @@ -31,6 +33,7 @@ public class ApiMethodCacheInvalidationTests : TestKit _services.AddScoped(_ => _apiRepo); _services.AddScoped(_ => Substitute.For()); _services.AddSingleton(_cache); + _services.AddSingleton(_publisher); _apiRepo.SaveChangesAsync(Arg.Any()).Returns(1); } @@ -111,4 +114,31 @@ public class ApiMethodCacheInvalidationTests : TestKit Assert.False(await Refetches("Other")); } + + /// + /// ScadaBridge#103: create, update and delete tell the peer central node to drop the method's + /// handler, known-bad record and cached row, once each and after commit. + /// + [Fact] + public void CreateUpdateDelete_TellThePeer() + { + var existing = new ApiMethod("Peer", "return 1;") { Id = 11, TimeoutSeconds = 10 }; + _apiRepo.GetApiMethodByIdAsync(11, Arg.Any()).Returns(existing); + var actor = CreateActor(); + + foreach (var command in new object[] + { + new CreateApiMethodCommand("Peer", "return 1;", 30, null, null), + new UpdateApiMethodCommand(11, "return 2;", 10, null, null), + new DeleteApiMethodCommand(11), + }) + { + actor.Tell(Envelope(command)); + ExpectMsg(TimeSpan.FromSeconds(30)); + } + + Assert.Equal( + Enumerable.Repeat(new InboundApiMethodInvalidated("Peer"), 3), + _publisher.Published); + } } diff --git a/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ApiMethodCompileDiagnosticsTests.cs b/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ApiMethodCompileDiagnosticsTests.cs index 201dffd8..22f61dce 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ApiMethodCompileDiagnosticsTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ApiMethodCompileDiagnosticsTests.cs @@ -7,6 +7,7 @@ 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.Communication; using ZB.MOM.WW.ScadaBridge.Commons.Messages.Management; using ZB.MOM.WW.ScadaBridge.InboundAPI; @@ -28,6 +29,11 @@ public class ApiMethodCompileDiagnosticsTests : TestKit private readonly InboundScriptExecutor _executor = new(NullLogger.Instance, Substitute.For()); private readonly ApiMethod _method = new("Broken", BrokenScript) { Id = 5, TimeoutSeconds = 10 }; + // The other central node, reached only through the peer invalidation (ScadaBridge#103). + private readonly InboundScriptExecutor _peerExecutor = + new(NullLogger.Instance, Substitute.For()); + private readonly ApiMethodCache _peerMethodCache = new(TimeSpan.FromHours(1)); + private readonly PeerCacheInvalidationPublisher _publisher; /// Initializes the harness with a real executor and a stubbed repository. public ApiMethodCompileDiagnosticsTests() @@ -35,6 +41,9 @@ public class ApiMethodCompileDiagnosticsTests : TestKit _services.AddScoped(_ => _apiRepo); _services.AddScoped(_ => _audit); _services.AddSingleton(_executor); + _publisher = new PeerCacheInvalidationPublisher( + new InboundApiCacheInvalidationHandler(_peerExecutor, _peerMethodCache)); + _services.AddSingleton(_publisher); _apiRepo.GetApiMethodByIdAsync(5, Arg.Any()).Returns(_method); _apiRepo.SaveChangesAsync(Arg.Any()).Returns(1); } @@ -116,6 +125,41 @@ public class ApiMethodCompileDiagnosticsTests : TestKit Assert.Contains("doesNotExist", state.LastCompileError); } + /// + /// ScadaBridge#103: compile state is per node, so a recompile used to clear the known-bad + /// record only on the node that answered — the peer kept refusing the unchanged script with + /// SCRIPT_COMPILE_FAILED until it restarted. The recompile now drops the peer's record (and + /// its cached row), so the peer compiles the current text on its next request. + /// + /// A task that represents the asynchronous operation. + [Fact] + public async Task Recompile_ClearsTheKnownBadRecordOnThePeerNode() + { + Assert.False(_peerExecutor.CompileAndRegister(_method)); + Assert.True(_peerExecutor.GetCompileState("Broken", BrokenScript).IsKnownBad); // positive control + await _peerMethodCache.GetOrFetchAsync("Broken", _ => Task.FromResult(_method), CancellationToken.None); + + Send(new RecompileApiMethodCommand(5)); + + Assert.False(_peerExecutor.GetCompileState("Broken", BrokenScript).IsKnownBad); + var refetched = false; + await _peerMethodCache.GetOrFetchAsync("Broken", _ => { refetched = true; return Task.FromResult(_method); }, CancellationToken.None); + Assert.True(refetched); + Assert.Equal(new[] { new InboundApiMethodInvalidated("Broken") }, _publisher.Published); + } + + /// Negative control: validate changes nothing anywhere, so it tells the peer nothing. + [Fact] + public void Validate_DoesNotTouchThePeer() + { + Assert.False(_peerExecutor.CompileAndRegister(_method)); + + Send(new ValidateApiMethodCommand(5)); + + Assert.True(_peerExecutor.GetCompileState("Broken", BrokenScript).IsKnownBad); + Assert.Empty(_publisher.Published); + } + /// Validate and recompile are Designer operations, like UpdateApiMethod. /// The command name to send. [Theory] diff --git a/tests/ZB.MOM.WW.ScadaBridge.TemplateEngine.Tests/Flattening/FlatteningServiceTests.cs b/tests/ZB.MOM.WW.ScadaBridge.TemplateEngine.Tests/Flattening/FlatteningServiceTests.cs index 0a063d41..d0bb9d6d 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.TemplateEngine.Tests/Flattening/FlatteningServiceTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.TemplateEngine.Tests/Flattening/FlatteningServiceTests.cs @@ -154,6 +154,33 @@ public class FlatteningServiceTests Assert.Equal("original", attr.Value); // Lock honored } + /// + /// Pins the name rule the Transport import's locked-override warnings mirror (ScadaBridge#103): + /// an override addresses a member by its exact (ordinal) name, so one differing only by case + /// is not applied — and so never reaches the member's lock check. + /// + [Fact] + public void Flatten_InstanceOverride_MatchesTheMemberNameExactly() + { + var template = CreateTemplate(1, "Base"); + template.Attributes.Add(new TemplateAttribute("Threshold") { DataType = DataType.Double, Value = "50.0" }); + + var instance = CreateInstance(); + instance.AttributeOverrides.Add(new InstanceAttributeOverride("threshold") { OverrideValue = "75.0" }); + + var result = _sut.Flatten( + instance, + [template], + new Dictionary>(), + new Dictionary>(), + new Dictionary()); + + Assert.True(result.IsSuccess); + var attr = Assert.Single(result.Value.Attributes, a => a.CanonicalName == "Threshold"); + Assert.Equal("50.0", attr.Value); + Assert.DoesNotContain(result.Value.Attributes, a => a.CanonicalName == "threshold"); + } + /// Verifies that members of a composed module receive path-qualified names. [Fact] public void Flatten_ComposedModule_PathQualifiedNames() 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 642b0d0e..8ad6030d 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/BundleImporterApplyTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/BundleImporterApplyTests.cs @@ -1858,6 +1858,9 @@ public sealed class BundleImporterApplyTests : IDisposable Assert.Equal(new[] { "HelperFn" }, byKind[ScriptArtifactKinds.SharedScript].Names); Assert.Equal(new[] { "Pump" }, byKind[ScriptArtifactKinds.Template].Names); Assert.All(_artifactBus.Received, n => Assert.Equal("BundleImport", n.Source)); + // ScadaBridge#103: the peer central node is told about the imported API method too. + Assert.Equal(new[] { new InboundApiMethodInvalidated("DoThing") }, + _peerPublisher.Published.OfType()); } /// Verifies that applying a bundle publishes script-artifacts-changed notifications for Add resolutions. diff --git a/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/SiteInstanceImportTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/SiteInstanceImportTests.cs index 89b4fc49..8653bed4 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/SiteInstanceImportTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/SiteInstanceImportTests.cs @@ -1375,6 +1375,42 @@ public sealed class SiteInstanceImportTests : IDisposable && b.BlockerReason!.Contains("locked", StringComparison.OrdinalIgnoreCase)); } + /// + /// ScadaBridge#103: the lock warning matches member names exactly, like the flattener. An + /// override named "setpoint" never reaches the locked "SetPoint" (the flattener drops it as + /// unresolved), so it must not be reported as targeting a locked member. + /// + /// A task that represents the asynchronous operation. + [Fact] + public async Task Import_OverrideDifferingOnlyByCaseFromALockedAttribute_NoLockWarning() + { + await SeedLockedMemberClosureAsync(lockAttribute: true, attributeOverrideName: "setpoint"); + + var sessionId = await ExportAllSitesAndLoadAsync(); + await WipeSiteClosureAsync(); + await using (var scope = _provider.CreateAsyncScope()) + { + var ctx = scope.ServiceProvider.GetRequiredService(); + ctx.Templates.Add(new Template("Pump") { Description = "pump tpl" }); + await ctx.SaveChangesAsync(); + } + + var result = await ApplyAsync( + sessionId, + new List + { + new("Template", "Pump", ResolutionAction.Skip, null), + new("Site", "plant-1", ResolutionAction.Add, null), + new("Instance", "Pump-01", ResolutionAction.Add, null), + }, + new BundleNameMap( + Sites: new[] { new SiteMapping("plant-1", MappingAction.CreateNew, null) }, + Connections: Array.Empty())); + + Assert.DoesNotContain(result.Warnings, w => + w.Contains("locked", StringComparison.OrdinalIgnoreCase)); + } + /// Verifies that an override on an unlocked attribute produces no lock warning. /// A task that represents the asynchronous operation. [Fact] @@ -1417,7 +1453,8 @@ public sealed class SiteInstanceImportTests : IDisposable /// The attribute override is always present; the native-source override (and its /// connection) are seeded only when a native source is involved. /// - private async Task SeedLockedMemberClosureAsync(bool lockAttribute = false, bool lockNativeSource = false) + private async Task SeedLockedMemberClosureAsync( + bool lockAttribute = false, bool lockNativeSource = false, string attributeOverrideName = "SetPoint") { await using var scope = _provider.CreateAsyncScope(); var ctx = scope.ServiceProvider.GetRequiredService(); @@ -1462,7 +1499,7 @@ public sealed class SiteInstanceImportTests : IDisposable SiteId = site.Id, State = InstanceState.Enabled, }; - instance.AttributeOverrides.Add(new InstanceAttributeOverride("SetPoint") { OverrideValue = "42" }); + instance.AttributeOverrides.Add(new InstanceAttributeOverride(attributeOverrideName) { OverrideValue = "42" }); if (lockNativeSource) { instance.NativeAlarmSourceOverrides.Add(new InstanceNativeAlarmSourceOverride("NativeSrc")