Compare commits
9 Commits
13a33a6c78
...
39d737ebd6
| Author | SHA1 | Date | |
|---|---|---|---|
| 39d737ebd6 | |||
| 8dd74121c3 | |||
| 34588ae10c | |||
| a55502254e | |||
| 1e2e7d2e7c | |||
| b1f4251d75 | |||
| c583598888 | |||
| a2f6c1b9b2 | |||
| 3d0c1c6963 |
@@ -8,7 +8,7 @@
|
||||
| Last reviewed | 2026-05-16 |
|
||||
| Reviewer | claude-agent |
|
||||
| Commit reviewed | `9c60592` |
|
||||
| Open findings | 2 |
|
||||
| Open findings | 0 |
|
||||
|
||||
## Summary
|
||||
|
||||
@@ -248,7 +248,7 @@ Fixed by the commit whose message references `CentralUI-004`.
|
||||
|--|--|
|
||||
| Severity | Medium |
|
||||
| Category | Design-document adherence |
|
||||
| Status | Open |
|
||||
| Status | Resolved |
|
||||
| Location | `src/ScadaLink.CentralUI/Auth/AuthEndpoints.cs:47-81`; `src/ScadaLink.CentralUI/Components/Shared/SessionExpiry.razor:18-30` |
|
||||
|
||||
**Description**
|
||||
@@ -269,17 +269,48 @@ fixed 30-minute model. The code and the documented decision must agree.
|
||||
|
||||
**Resolution**
|
||||
|
||||
_Unresolved — requires a cross-module change plus a design decision, both out of
|
||||
scope for a CentralUI-only fix._ Verified 2026-05-16: the discrepancy is real.
|
||||
The sliding-expiration mechanism, however, is owned by the cookie
|
||||
authentication middleware configured in **`ScadaLink.Security`**
|
||||
(`ServiceCollectionExtensions.AddCookie` — currently sets neither
|
||||
`ExpireTimeSpan` nor `SlidingExpiration`); `AuthEndpoints` (CentralUI) only sets
|
||||
the absolute `ExpiresUtc`/`expires_at`. Implementing "15-minute sliding token"
|
||||
means editing `ScadaLink.Security`, which this module's review cannot touch, and
|
||||
the alternative — amending the documented decision to a fixed 30-minute model —
|
||||
is a design decision, not a code fix. Left Open and surfaced for a follow-up
|
||||
that spans CentralUI + Security, or a design-doc amendment.
|
||||
Resolved 2026-05-16 (commit `<pending>`) — cross-module fix (CentralUI +
|
||||
Security), explicitly authorized. Root cause confirmed against the source:
|
||||
`AddCookie` (`ScadaLink.Security/ServiceCollectionExtensions.cs`) set neither
|
||||
`ExpireTimeSpan` nor `SlidingExpiration`; `AuthEndpoints` stamped a fixed
|
||||
`expires_at = UtcNow + 30 min` claim and a 30-minute absolute cookie
|
||||
`ExpiresUtc`; `SessionExpiry.razor` scheduled one hard redirect at that fixed
|
||||
instant — a hard 30-minute cap, no sliding renewal, no 15-minute component.
|
||||
|
||||
**What was implemented — sliding session window.** ASP.NET Core cookie
|
||||
authentication exposes a single `ExpireTimeSpan` plus a `SlidingExpiration`
|
||||
flag; it cannot natively model *both* a 15-minute sliding token *and* a separate
|
||||
30-minute absolute idle cap. The faithful interpretation implemented: the cookie
|
||||
session window **is** the idle timeout. `AddSecurity` now post-configures the
|
||||
cookie options with `ExpireTimeSpan = TimeSpan.FromMinutes(SecurityOptions.IdleTimeoutMinutes)`
|
||||
(default 30, bound from `appsettings` via the existing options pattern, not
|
||||
hard-coded) and `SlidingExpiration = true`. The middleware therefore re-issues
|
||||
the cookie on activity once past the halfway mark of the window: an active user
|
||||
is continually renewed, an idle user is signed out after the 30-minute idle
|
||||
timeout — exactly the documented "sliding refresh, 30-minute idle timeout". The
|
||||
separate 15-minute `JwtExpiryMinutes` governs the lifetime of the *embedded JWT*
|
||||
itself (`JwtTokenService`) — a distinct layer from the cookie session window;
|
||||
it is not, and per the ASP.NET cookie model cannot be, a second independent
|
||||
sliding window inside the same cookie. `AuthEndpoints` no longer imposes a
|
||||
contradictory absolute cap: the `expires_at` claim and the manual cookie
|
||||
`ExpiresUtc` were removed, and a new `BuildSignInProperties()` helper sets only
|
||||
`IsPersistent = true` (no `ExpiresUtc`, `AllowRefresh` left unset) so the
|
||||
middleware owns expiry. `SessionExpiry.razor` no longer reads a fixed
|
||||
login-time deadline (the `expires_at` claim is gone) and no longer hard-redirects
|
||||
at a fixed instant: it now polls the authentication state on a recurring
|
||||
interval and redirects to `/login` only once the sliding cookie has actually
|
||||
lapsed server-side — so an active user is never logged out mid-session.
|
||||
|
||||
Regression tests fail against the pre-fix code and pass after. Security:
|
||||
`AddSecurity_AuthCookie_UsesSlidingExpiration`,
|
||||
`AddSecurity_AuthCookie_ExpireTimeSpanMatchesIdleTimeout` (pre-fix
|
||||
`ExpireTimeSpan` was the 14-day default — confirmed failing), and
|
||||
`AddSecurity_AuthCookie_ExpireTimeSpanIsConfigurable` (pins the options-pattern
|
||||
binding). CentralUI: `SessionExpiryPolicyTests.BuildSignInProperties_DoesNotSetFixedAbsoluteExpiry`,
|
||||
`..._IsPersistent`, `..._AllowsSlidingRefresh` pin that the login sign-in no
|
||||
longer imposes a fixed absolute cap. `dotnet build ScadaLink.slnx` clean;
|
||||
`tests/ScadaLink.Security.Tests` 57 passed, `tests/ScadaLink.CentralUI.Tests`
|
||||
254 passed.
|
||||
|
||||
### CentralUI-006 — Deployment status page polls every 10s despite the documented SignalR-push design
|
||||
|
||||
@@ -287,7 +318,7 @@ that spans CentralUI + Security, or a design-doc amendment.
|
||||
|--|--|
|
||||
| Severity | Medium |
|
||||
| Category | Design-document adherence |
|
||||
| Status | Open |
|
||||
| Status | Resolved |
|
||||
| Location | `src/ScadaLink.CentralUI/Components/Pages/Deployment/Deployments.razor:196-216` |
|
||||
|
||||
**Description**
|
||||
@@ -310,16 +341,59 @@ If polling is kept as a fallback, fetch only changed/in-progress records.
|
||||
|
||||
**Resolution**
|
||||
|
||||
_Unresolved — a genuine SignalR-push fix requires an event source in another
|
||||
module._ Verified 2026-05-16: `Deployments.razor` does poll every 10s, contrary
|
||||
to the design doc. But a real push implementation needs the **Deployment
|
||||
Manager** module (`ScadaLink.DeploymentManager` — `DeploymentService` /
|
||||
`ArtifactDeploymentService` write the `DeploymentRecord` rows) to raise a
|
||||
status-change event/observable that the page subscribes to; there is no such
|
||||
event today and no CentralUI-only seam to subscribe to. Building that event
|
||||
source is out of scope for a CentralUI-only review. Left Open and surfaced for a
|
||||
follow-up that adds a deployment-status broadcaster in DeploymentManager (or a
|
||||
design-doc amendment acknowledging the polling fallback).
|
||||
Resolved 2026-05-16 (commit `<pending>`) — cross-module fix (CentralUI +
|
||||
DeploymentManager), explicitly authorized. Root cause confirmed against the
|
||||
source: `Deployments.razor` ran a `Timer` (`OnInitializedAsync` → `StartTimer`,
|
||||
10s interval) that, every tick and for every open Blazor circuit, reloaded all
|
||||
deployment records (`GetAllDeploymentRecordsAsync`) and the full instance map
|
||||
(`GetAllInstancesAsync`) — contradicting Component-CentralUI "Real-Time Updates"
|
||||
("transitions push to the UI immediately via SignalR … no polling required").
|
||||
|
||||
**Process/DI topology confirmed.** `ScadaLink.Host/Program.cs` calls both
|
||||
`AddDeploymentManager()` (line 75) and `AddCentralUI()` (line 77) on the same
|
||||
`builder.Services` — DeploymentManager and the Central UI run **in the same
|
||||
central Host process**, so a DI singleton is genuinely shared between the
|
||||
DeploymentManager services and the Blazor circuit's scoped components. The
|
||||
shared-singleton seam is real; no out-of-process fallback was needed.
|
||||
|
||||
**What was implemented — push-based updates.** A new
|
||||
`IDeploymentStatusNotifier` (`ScadaLink.DeploymentManager/IDeploymentStatusNotifier.cs`)
|
||||
with a C# `event Action<DeploymentStatusChange>` and a small payload
|
||||
(`DeploymentStatusChange` = deployment id + instance id + new status). Its
|
||||
implementation `DeploymentStatusNotifier` invokes each subscriber in isolation
|
||||
and swallows/logs handler exceptions so a faulting circuit cannot break the
|
||||
deployment pipeline. It is registered as a **singleton** in `AddDeploymentManager`
|
||||
(`ServiceCollectionExtensions`). Every place `DeploymentService` writes a
|
||||
`DeploymentRecord` status now raises the notifier: the `Pending` create, the
|
||||
`InProgress` update, the site-response terminal update, the `Failed` cleanup
|
||||
write in the catch block, and the `DeploymentManager-006` reconciled-`Success`
|
||||
write — five call sites via a private `NotifyStatusChange` helper.
|
||||
`ArtifactDeploymentService` was inspected and writes only
|
||||
`SystemArtifactDeploymentRecord` rows, which `Deployments.razor` does not
|
||||
display, so it correctly raises nothing. `Deployments.razor` no longer has a
|
||||
`Timer`: `OnInitializedAsync` subscribes to `IDeploymentStatusNotifier.StatusChanged`,
|
||||
the handler reloads via `InvokeAsync(StateHasChanged)` (the notifier event is
|
||||
raised on the DeploymentManager service thread), and `Dispose` unsubscribes.
|
||||
Blazor Server pushes the re-render to the browser over its SignalR circuit
|
||||
automatically — satisfying the documented design. The existing "Pause/Resume
|
||||
updates" toggle now gates whether incoming push events are acted on, and
|
||||
"Refresh" still forces a manual reload. CLAUDE.md UI rules kept: Blazor Server +
|
||||
Bootstrap, custom components, no third-party frameworks.
|
||||
|
||||
Regression tests fail against the pre-fix code and pass after. DeploymentManager
|
||||
(`DeploymentStatusNotifierTests`): `DeployInstanceAsync_RaisesStatusChange_ForEveryRecordStatusWrite`
|
||||
(pre-fix: no notifier, fails to compile / silent), plus
|
||||
`NotifyStatusChanged_WithNoSubscribers_DoesNotThrow` and
|
||||
`NotifyStatusChanged_ThrowingSubscriber_DoesNotBreakOtherSubscribers`;
|
||||
`ServiceCollectionExtensionsTests.AddDeploymentManager_RegistersDeploymentStatusNotifier_AsSingleton`
|
||||
pins the shared-singleton seam. CentralUI (`DeploymentsPushUpdateTests`):
|
||||
`Deployments_DoesNotPoll_HasNoRefreshTimer` (pre-fix: the `_refreshTimer` field
|
||||
existed — confirmed failing), `Deployments_StatusChange_TriggersReload`, and
|
||||
`Deployments_Dispose_UnsubscribesFromNotifier`. `dotnet build ScadaLink.slnx`
|
||||
clean (0 warnings); `tests/ScadaLink.DeploymentManager.Tests` 76 passed,
|
||||
`tests/ScadaLink.CentralUI.Tests` 257 passed. (`TopologyPageTests`' DI fixture
|
||||
was also updated to register the new notifier, since it constructs the real
|
||||
`DeploymentService`.)
|
||||
|
||||
### CentralUI-007 — Monitoring nav links to Deployment-only pages are shown to all roles
|
||||
|
||||
|
||||
@@ -8,7 +8,7 @@
|
||||
| Last reviewed | 2026-05-16 |
|
||||
| Reviewer | claude-agent |
|
||||
| Commit reviewed | `9c60592` |
|
||||
| Open findings | 1 |
|
||||
| Open findings | 0 |
|
||||
|
||||
## Summary
|
||||
|
||||
@@ -357,7 +357,7 @@ carve-out makes these types compliant by design.
|
||||
|--|--|
|
||||
| Severity | Low |
|
||||
| Category | Akka.NET conventions |
|
||||
| Status | Open |
|
||||
| Status | Resolved |
|
||||
| Location | `src/ScadaLink.Commons/Messages/Management/InstanceCommands.cs:10` |
|
||||
|
||||
**Description**
|
||||
@@ -379,18 +379,26 @@ Replace the tuple with a small named record, e.g.
|
||||
|
||||
**Resolution**
|
||||
|
||||
_Open — deferred to a coordinated multi-module change._ The finding is confirmed valid:
|
||||
the `ValueTuple` is the only positional/tuple element in `Messages/` and is unfriendly to
|
||||
REQ-COM-5a additive evolution. However, `SetConnectionBindingsCommand.Bindings` is not a
|
||||
Commons-internal type — its `IReadOnlyList<(string,int)>` shape is part of a cross-module
|
||||
contract consumed by `ScadaLink.CLI` (`InstanceCommands.TryParseBindings` builds a
|
||||
`List<(string,int)>` and passes it to the constructor), `ScadaLink.ManagementService`
|
||||
(`ManagementActor` forwards `cmd.Bindings`), and `ScadaLink.TemplateEngine`
|
||||
(`InstanceService.SetConnectionBindingsAsync` takes an `IReadOnlyList<(string AttributeName,
|
||||
int DataConnectionId)>` parameter). Introducing a `ConnectionBinding` record therefore
|
||||
requires editing those three modules in lock-step, which is outside the scope of this
|
||||
Commons-only review pass (the constraint forbids touching other modules' source). Left
|
||||
Open and flagged for a follow-up cross-module change; the fix itself is unambiguous.
|
||||
Resolved 2026-05-16 (commit `<pending>`) — confirmed against the source: the `ValueTuple`
|
||||
was the only positional/tuple element in `Messages/` and `System.Text.Json` serializes it
|
||||
as `Item1`/`Item2`, unfriendly to REQ-COM-5a additive evolution. Introduced a named record
|
||||
`ConnectionBinding(string AttributeName, int DataConnectionId)` in
|
||||
`Messages/Management/InstanceCommands.cs` (alongside the command, matching the existing
|
||||
record-per-file convention) and changed `SetConnectionBindingsCommand.Bindings` to
|
||||
`IReadOnlyList<ConnectionBinding>`. All consumers were updated in lock-step: `ScadaLink.CLI`
|
||||
(`InstanceCommands.TryParseBindings` now builds a `List<ConnectionBinding>`),
|
||||
`ScadaLink.TemplateEngine` (`InstanceService.SetConnectionBindingsAsync` parameter),
|
||||
`ScadaLink.ManagementService` (`ManagementActor` forwards `cmd.Bindings` unchanged — the
|
||||
new element type flows through), and one consumer the finding did not list,
|
||||
`ScadaLink.CentralUI` (`InstanceConfigure.razor`'s `SaveBindings` built a `List<(string,int)>`).
|
||||
A repo-wide `src/` scan confirms no other references remain. Regression tests added in
|
||||
`ConnectionBindingSerializationTests` (`ConnectionBinding_SerializesWithNamedProperties`
|
||||
asserts named `AttributeName`/`DataConnectionId` JSON properties and the absence of
|
||||
`Item1`/`Item2`; `SetConnectionBindingsCommand_RoundTripsThroughJson` asserts a full
|
||||
JSON round-trip); existing parse/forward tests (`ParseBindings_ValidJson_ReturnsPairs`,
|
||||
`SetConnectionBindings_BulkAssignment_Success`) were updated to the new type. Affected
|
||||
suites are green (Commons 226, CLI 77, ManagementService 55, TemplateEngine 287) and
|
||||
`dotnet build ScadaLink.slnx` is clean.
|
||||
|
||||
### Commons-009 — `Component-Commons.md` is stale relative to the actual file set
|
||||
|
||||
|
||||
@@ -8,7 +8,7 @@
|
||||
| Last reviewed | 2026-05-16 |
|
||||
| Reviewer | claude-agent |
|
||||
| Commit reviewed | `9c60592` |
|
||||
| Open findings | 1 |
|
||||
| Open findings | 0 |
|
||||
|
||||
## Summary
|
||||
|
||||
@@ -608,7 +608,7 @@ the call hung the full 30 s and threw `AskTimeoutException`).
|
||||
|--|--|
|
||||
| Severity | Low |
|
||||
| Category | Security |
|
||||
| Status | Open |
|
||||
| Status | Resolved |
|
||||
| Location | `src/ScadaLink.DeploymentManager/ArtifactDeploymentService.cs:108-111` |
|
||||
|
||||
**Description**
|
||||
@@ -655,9 +655,21 @@ hardening item, not an active leak.
|
||||
|
||||
**Resolution**
|
||||
|
||||
_Unresolved — see Verification above. Left Open: requires cross-module
|
||||
cooperation (Communication, SiteRuntime, Commons) and a key-management design
|
||||
decision; out of scope for the DeploymentManager module._
|
||||
Resolved 2026-05-16 (commit `<pending>`). Re-verification confirmed the
|
||||
DeploymentManager code is clean: `ArtifactDeploymentService` maps
|
||||
`SmtpConfiguration.Credentials` into the artifact (which the design mandates —
|
||||
SMTP configuration is a deployable artifact) and never logs the credential.
|
||||
The finding's substantive ask — "at minimum this should be a conscious,
|
||||
documented decision" — is now satisfied: a **"Secret handling in artifacts"**
|
||||
subsection was added to `docs/requirements/Component-DeploymentManager.md`
|
||||
recording the accepted design decision and its controls — TLS-protected
|
||||
inter-cluster transport in transit, no credential values in logs, and an
|
||||
explicit statement that at-rest encryption of the credential field on site
|
||||
SQLite is not currently applied (accepted given the transport protection and
|
||||
trust boundary) with payload-field encryption noted as a possible future
|
||||
hardening item requiring a key-management scheme. No code change was warranted;
|
||||
the residual encryption item is a documented, deliberately-deferred hardening
|
||||
option rather than an open defect.
|
||||
|
||||
### DeploymentManager-014 — Dead `CreateCommand` helper in artifact tests
|
||||
|
||||
|
||||
@@ -8,7 +8,7 @@
|
||||
| Last reviewed | 2026-05-16 |
|
||||
| Reviewer | claude-agent |
|
||||
| Commit reviewed | `9c60592` |
|
||||
| Open findings | 1 |
|
||||
| Open findings | 0 |
|
||||
|
||||
## Summary
|
||||
|
||||
@@ -533,7 +533,7 @@ exception propagates; it was verified to fail before the `try/catch` was added.
|
||||
|--|--|
|
||||
| Severity | Low |
|
||||
| Category | Performance & resource management |
|
||||
| Status | Open |
|
||||
| Status | Resolved |
|
||||
| Location | `src/ScadaLink.ExternalSystemGateway/ExternalSystemClient.cs:360-374`, `src/ScadaLink.ExternalSystemGateway/DatabaseGateway.cs:169-176` |
|
||||
|
||||
**Description**
|
||||
@@ -554,26 +554,67 @@ rather than fetch-all-then-filter.
|
||||
|
||||
**Resolution**
|
||||
|
||||
2026-05-16 — **Root cause confirmed, but left Open: no correct fix is possible within
|
||||
this module's edit scope.** `ResolveSystemAndMethodAsync`
|
||||
(`ExternalSystemClient.cs:360`) does call `GetAllExternalSystemsAsync()` followed by
|
||||
`GetMethodsByExternalSystemIdAsync()` and filters in memory, and
|
||||
`ResolveConnectionAsync` (`DatabaseGateway.cs:169`) does `GetAllDatabaseConnectionsAsync()`
|
||||
then filters — fetch-all-then-filter on every hot-path call, as described.
|
||||
Resolved 2026-05-16 (commit `<pending>`). A cross-module change was explicitly
|
||||
authorized, so the **name-keyed repository lookup** recommendation was applied — the
|
||||
cleaner of the two options, and one that avoids the staleness hazard a
|
||||
deployment-invalidated cache would introduce.
|
||||
|
||||
Both recommended fixes require changes outside `src/ScadaLink.ExternalSystemGateway`:
|
||||
(a) a **name-keyed repository lookup** (e.g. `GetExternalSystemByNameAsync`) means adding
|
||||
methods to `IExternalSystemRepository` in `ScadaLink.Commons` and implementing them in
|
||||
`ScadaLink.ConfigurationDatabase` / `ScadaLink.SiteRuntime`; (b) an **in-memory cache
|
||||
invalidated on artifact deployment** requires subscribing to a deployment-applied event
|
||||
owned by `ScadaLink.SiteRuntime` / `ScadaLink.DeploymentManager`. A purely module-local
|
||||
cache with a time-based TTL was rejected as a fix: definitions only change on deployment
|
||||
and must reflect a deployment promptly, so a TTL would either be too short to help the
|
||||
hot path or long enough to serve stale definitions after a redeploy — trading a
|
||||
correctness hazard for a performance gain on a Low-severity issue. **Tracked follow-up:**
|
||||
add a name-keyed lookup to `IExternalSystemRepository` (Commons) and have the gateway use
|
||||
it, or add a deployment-invalidated definition cache wired from SiteRuntime. No source
|
||||
change was made in this module.
|
||||
Three name-keyed methods were added to `IExternalSystemRepository`
|
||||
(`ScadaLink.Commons`): `GetExternalSystemByNameAsync(name)`,
|
||||
`GetMethodByNameAsync(externalSystemId, methodName)` and
|
||||
`GetDatabaseConnectionByNameAsync(name)`. The connection lookup belongs on the same
|
||||
interface because database connection definitions are already part of
|
||||
`IExternalSystemRepository` (alongside `GetAllDatabaseConnectionsAsync` /
|
||||
`GetDatabaseConnectionByIdAsync`), so the existing repository organization was
|
||||
followed rather than introducing a new interface.
|
||||
|
||||
Both implementers of the interface were updated:
|
||||
|
||||
- `ScadaLink.ConfigurationDatabase.ExternalSystemRepository` — all three are genuine
|
||||
server-side keyed queries (`FirstOrDefaultAsync(x => x.Name == name)`, the
|
||||
method lookup additionally scoped by `ExternalSystemDefinitionId`), matching the
|
||||
existing `GetMethodByNameAsync` / `GetListByNameAsync` / `GetSharedScriptByNameAsync`
|
||||
convention in the other Central repositories.
|
||||
- `ScadaLink.SiteRuntime.SiteExternalSystemRepository` — `GetExternalSystemByNameAsync`
|
||||
and `GetDatabaseConnectionByNameAsync` are genuine single-row indexed SQLite queries
|
||||
(`WHERE name = @name`; both tables have `name` as the PRIMARY KEY).
|
||||
`GetMethodByNameAsync` resolves the named method from the parent system's
|
||||
`method_definitions` JSON column; this still requires resolving the parent system
|
||||
(one id→name scan), but the gateway's new call sequence performs **one** scan instead
|
||||
of the previous **two** (`GetAllExternalSystemsAsync` + `GetMethodsByExternalSystemIdAsync`,
|
||||
which itself scanned), so the site path is strictly better than before — noted as
|
||||
the one place a residual scan remains, bounded by the deployed system count.
|
||||
|
||||
`ExternalSystemClient.ResolveSystemAndMethodAsync` and
|
||||
`DatabaseGateway.ResolveConnectionAsync` were rewritten to call the keyed lookups; the
|
||||
`GetAllExternalSystemsAsync` / `GetMethodsByExternalSystemIdAsync` /
|
||||
`GetAllDatabaseConnectionsAsync` + in-memory `FirstOrDefault` filtering is gone from
|
||||
both hot paths.
|
||||
|
||||
Regression tests (TDD — written first, verified failing/not-compiling before the
|
||||
implementation, then confirmed green; one was additionally verified to fail when the
|
||||
keyed query is deliberately broken):
|
||||
|
||||
- ConfigurationDatabase (`RepositoryCoverageTests`):
|
||||
`GetExternalSystemByName_ReturnsMatchingRow`,
|
||||
`GetExternalSystemByName_MissingName_ReturnsNull`,
|
||||
`GetMethodByName_ReturnsMethodScopedToParentSystem`,
|
||||
`GetMethodByName_MissingMethod_ReturnsNull`,
|
||||
`GetDatabaseConnectionByName_ReturnsMatchingRow`,
|
||||
`GetDatabaseConnectionByName_MissingName_ReturnsNull`.
|
||||
- SiteRuntime (`SiteRepositoryTests`):
|
||||
`ExternalSystemRepository_GetByName_ReturnsMatchingDefinition`,
|
||||
`ExternalSystemRepository_GetByName_MissingName_ReturnsNull`,
|
||||
`ExternalSystemRepository_GetMethodByName_ResolvesScopedToSystem`,
|
||||
`DatabaseConnectionRepository_GetByName_ReturnsMatchingDefinition`.
|
||||
- ExternalSystemGateway: the existing `ExternalSystemClientTests` /
|
||||
`DatabaseGatewayTests` resolution stubs were migrated to the keyed methods (via
|
||||
`StubResolution` / `StubConnection` helpers), so the full gateway suite now exercises
|
||||
and protects the keyed-lookup resolution path.
|
||||
|
||||
`dotnet build ScadaLink.slnx` is clean; `ScadaLink.ExternalSystemGateway.Tests` (54),
|
||||
`ScadaLink.ConfigurationDatabase.Tests` (106), `ScadaLink.SiteRuntime.Tests` (196) and
|
||||
`ScadaLink.Commons.Tests` (226) all pass.
|
||||
|
||||
### ExternalSystemGateway-012 — Permanent-failure logging requirement is not met; `_logger` is injected but unused
|
||||
|
||||
|
||||
@@ -8,7 +8,7 @@
|
||||
| Last reviewed | 2026-05-16 |
|
||||
| Reviewer | claude-agent |
|
||||
| Commit reviewed | `9c60592` |
|
||||
| Open findings | 1 |
|
||||
| Open findings | 0 |
|
||||
|
||||
## Summary
|
||||
|
||||
@@ -98,9 +98,9 @@ response body; it failed before the fix and passes after. Full Host suite green
|
||||
|
||||
| | |
|
||||
|--|--|
|
||||
| Severity | Medium — re-triaged: stale design doc, Host code is correct |
|
||||
| Severity | Medium |
|
||||
| Category | Design-document adherence |
|
||||
| Status | Open |
|
||||
| Status | Resolved |
|
||||
| Location | `src/ScadaLink.Host/Actors/AkkaHostedService.cs:70-108`, `docs/requirements/Component-Host.md` REQ-HOST-6 |
|
||||
|
||||
**Description**
|
||||
@@ -126,21 +126,19 @@ add the plugin packages and HOCON. Either way, code and doc must agree.
|
||||
|
||||
**Resolution**
|
||||
|
||||
_Verified 2026-05-16, left Open — re-triaged._ The finding is accurate: a repo-wide
|
||||
search confirms there are **no** `PersistentActor` / `ReceivePersistentActor`
|
||||
subclasses anywhere in `src/`, no `akka.persistence` section in the HOCON built by
|
||||
`AkkaHostedService.StartAsync`, and `ScadaLink.Host.csproj` references no persistence
|
||||
plugin packages. The system deliberately uses component-owned SQLite storage
|
||||
services instead. The **Host code is therefore correct and internally consistent** —
|
||||
the only defect is that `docs/requirements/Component-Host.md` REQ-HOST-6 and its
|
||||
Dependencies list still mandate Akka.Persistence, a subsystem that does not (and is
|
||||
not intended to) exist. The sole fix is editing that design document, which lies
|
||||
outside this resolution task's permitted edit scope (`src/ScadaLink.Host`,
|
||||
`tests/ScadaLink.Host.Tests`, `code-reviews/Host/findings.md`). No code or test
|
||||
change can resolve a stale-doc finding. Left **Open** and surfaced for the design-doc
|
||||
owner: REQ-HOST-6 must drop the Akka.Persistence requirement (and the
|
||||
`Akka.Persistence.Hosting` Dependencies entry), stating that node-local persistence
|
||||
is provided by component-owned SQLite storage services.
|
||||
Resolved 2026-05-16 (commit `<pending>`). Confirmed accurate: a repo-wide search
|
||||
finds **no** `PersistentActor` / `ReceivePersistentActor` subclasses anywhere in
|
||||
`src/`, no `akka.persistence` section in the HOCON built by
|
||||
`AkkaHostedService.StartAsync`, and `ScadaLink.Host.csproj` references no
|
||||
persistence plugin packages. The Host code is correct and internally consistent;
|
||||
the defect was a stale design doc. Fix: updated `docs/requirements/Component-Host.md`
|
||||
— REQ-HOST-6 no longer lists Persistence as a configured subsystem and now carries
|
||||
an explicit Persistence note stating that durable state is owned by individual
|
||||
components and persisted through component-owned stores (SQLite at sites, MS SQL
|
||||
centrally), with no Akka journal/snapshot store and no `PersistentActor` subclasses
|
||||
by design. The Responsibilities line and the Dependencies entry
|
||||
(`Akka.Persistence.Hosting` removed) were corrected to match. Code and doc now
|
||||
agree; no code or test change required.
|
||||
|
||||
### Host-003 — Secrets committed in plaintext in `appsettings.Central.json`
|
||||
|
||||
|
||||
@@ -8,7 +8,7 @@
|
||||
| Last reviewed | 2026-05-16 |
|
||||
| Reviewer | claude-agent |
|
||||
| Commit reviewed | `9c60592` |
|
||||
| Open findings | 2 |
|
||||
| Open findings | 0 |
|
||||
|
||||
## Summary
|
||||
|
||||
@@ -294,9 +294,9 @@ and `FilterCapsMaxRequestBodySizeFeature` added.
|
||||
|
||||
| | |
|
||||
|--|--|
|
||||
| Severity | Medium — verified real drift; left Open pending a design decision (see Resolution) |
|
||||
| Severity | Medium |
|
||||
| Category | Design-document adherence |
|
||||
| Status | Open |
|
||||
| Status | Resolved |
|
||||
| Location | `src/ScadaLink.InboundAPI/InboundScriptExecutor.cs:188-203` |
|
||||
|
||||
**Description**
|
||||
@@ -317,27 +317,20 @@ API.
|
||||
|
||||
**Resolution**
|
||||
|
||||
_Unresolved — left Open; needs a design decision the resolving agent cannot make._
|
||||
Re-triage 2026-05-16: confirmed against the current source — the drift is **real**.
|
||||
`InboundScriptContext` (`InboundScriptExecutor.cs:188-203`) exposes only `Parameters`,
|
||||
`Route`, and `CancellationToken`; there is no `Database` member, so a method script
|
||||
following the documented `Database.Connection("name")` API fails to compile.
|
||||
|
||||
This finding cannot be closed by the InboundAPI module agent for two reasons:
|
||||
1. **Scope** — the alternative resolution (deleting the "Database Access" section)
|
||||
edits `docs/requirements/Component-InboundAPI.md`, which is outside the editable
|
||||
scope (`src/ScadaLink.InboundAPI`, `tests/`, this file only).
|
||||
2. **It is a genuine design decision.** Implementing `Database.Connection()` is not a
|
||||
mechanical fix: it hands inbound API scripts a *raw* MS SQL client. The ScadaLink
|
||||
script trust model (CLAUDE.md, Akka.NET conventions) forbids scripts from `System.IO`
|
||||
and raw network access, and `ForbiddenApiChecker` (added for InboundAPI-005) now
|
||||
statically blocks `System.Net`/`System.IO`. A raw `SqlConnection` is in clear
|
||||
tension with that trust model, and the set of connection names a script may open,
|
||||
read-only vs. read-write access, and connection lifetime/pooling all require a
|
||||
design call. **Surface to the design owner:** decide whether `Database.Connection()`
|
||||
is in scope — if yes, write a design note covering the trust-model carve-out and
|
||||
then implement a `Database` member backed by a connection-factory service; if no,
|
||||
delete the "Database Access" section from `Component-InboundAPI.md`.
|
||||
Resolved 2026-05-16 (commit `<pending>`). The drift was confirmed real:
|
||||
`InboundScriptContext` (`InboundScriptExecutor.cs:188-203`) exposes only
|
||||
`Parameters`, `Route`, and `CancellationToken` — there is no `Database` member,
|
||||
so a method script following the documented `Database.Connection("name")` API
|
||||
would fail to compile. Resolution direction: the design doc is stale, not the
|
||||
code. Implementing `Database.Connection()` would hand inbound API scripts a
|
||||
*raw* MS SQL client, in direct tension with the ScadaLink script trust model
|
||||
(scripts are forbidden `System.IO`, raw network, etc.; `ForbiddenApiChecker`
|
||||
statically enforces this). Rather than carve a hole in the trust model, the
|
||||
"Database Access" section was removed from `docs/requirements/Component-InboundAPI.md`
|
||||
and replaced with an explicit "No direct database access" note explaining that
|
||||
scripts interact only through the curated `Route`/`Parameters` surfaces, and
|
||||
that any future data access must go behind a dedicated scoped helper added as an
|
||||
explicit design change. Code and doc now agree; no code or test change required.
|
||||
|
||||
### InboundAPI-008 — Inbound API endpoint not restricted to the active central node
|
||||
|
||||
@@ -507,7 +500,7 @@ indistinguishable contract.
|
||||
|--|--|
|
||||
| Severity | Low |
|
||||
| Category | Code organization & conventions |
|
||||
| Status | Open |
|
||||
| Status | Resolved |
|
||||
| Location | `src/ScadaLink.InboundAPI/ParameterValidator.cs:128-133` |
|
||||
|
||||
**Description**
|
||||
@@ -528,18 +521,27 @@ components that work with method definitions.
|
||||
|
||||
**Resolution**
|
||||
|
||||
_Unresolved — left Open; the fix crosses this module's editable boundary._
|
||||
Re-triage 2026-05-16: confirmed against the source — `ParameterDefinition`
|
||||
(`ParameterValidator.cs:128-133`) is indeed an API-contract-shaped POCO declared in
|
||||
the component project. However the recommended fix is to **create the type in
|
||||
`ScadaLink.Commons`** (and update the validator plus any other consumers to reference
|
||||
it), which edits files outside this module's editable scope
|
||||
(`src/ScadaLink.InboundAPI`, `tests/`, this file only). It also touches a shared
|
||||
contract type: a Commons namespace placement and a likely-paired return-definition
|
||||
type are a cross-component code-organization decision. **Surface to the design/Commons
|
||||
owner:** add `ParameterDefinition` (and a return-definition counterpart) under a
|
||||
`ScadaLink.Commons` InboundApi types namespace, then repoint `ParameterValidator` and
|
||||
any other consumers at it.
|
||||
Resolved 2026-05-16 (commit `<pending>`): root cause confirmed against the source —
|
||||
`ParameterDefinition` was a persistence-ignorant, API-contract-shaped POCO (the
|
||||
deserialized form of the `ApiMethod.ParameterDefinitions` configuration-database
|
||||
column) declared inside the component project, contrary to CLAUDE.md's
|
||||
code-organization rule that such shared contract types live in `ScadaLink.Commons`.
|
||||
The type was moved to `src/ScadaLink.Commons/Types/InboundApi/ParameterDefinition.cs`
|
||||
(namespace `ScadaLink.Commons.Types.InboundApi`) — placed under `Types/` with an
|
||||
`InboundApi` domain subfolder, matching the existing `Types/Scripts/` precedent, since
|
||||
the column itself is the persisted form and this type is its deserialized contract
|
||||
shape (not an EF-mapped entity). It remains a pure POCO with no EF attributes and no
|
||||
behaviour. `ParameterValidator` now imports the moved type via a `using
|
||||
ScadaLink.Commons.Types.InboundApi;` directive; a tree-wide search confirmed
|
||||
`ParameterValidator.cs` was the type's only declaration and only direct consumer (all
|
||||
other `ParameterDefinition*` matches are the unrelated `ParameterDefinitions` string
|
||||
property). No return-definition type exists in the codebase — only a `ReturnDefinition`
|
||||
string column — so none was invented. No behavioural change, so no new runtime
|
||||
regression test: this is a compile-level type move, and the existing 52
|
||||
`ScadaLink.InboundAPI.Tests` (including the `ParameterValidator` suite) act as the
|
||||
regression guard. `dotnet test` for `ScadaLink.InboundAPI.Tests` (52 passed) and
|
||||
`ScadaLink.Commons.Tests` (226 passed) are green; `dotnet build ScadaLink.slnx`
|
||||
succeeds with 0 warnings / 0 errors.
|
||||
|
||||
### InboundAPI-013 — `ApiKeyValidationResult.NotFound` factory returns HTTP 400, contradicting its name
|
||||
|
||||
|
||||
+13
-23
@@ -41,26 +41,26 @@ module file and counted in **Total**.
|
||||
|----------|---------------|
|
||||
| Critical | 0 |
|
||||
| High | 0 |
|
||||
| Medium | 4 |
|
||||
| Low | 4 |
|
||||
| **Total** | **8** |
|
||||
| Medium | 0 |
|
||||
| Low | 0 |
|
||||
| **Total** | **0** |
|
||||
|
||||
## Module Status
|
||||
|
||||
| Module | Last reviewed | Commit | Open (C/H/M/L) | Open | Total |
|
||||
|--------|---------------|--------|----------------|------|-------|
|
||||
| [CLI](CLI/findings.md) | 2026-05-16 | `9c60592` | 0/0/0/0 | 0 | 13 |
|
||||
| [CentralUI](CentralUI/findings.md) | 2026-05-16 | `9c60592` | 0/0/2/0 | 2 | 19 |
|
||||
| [CentralUI](CentralUI/findings.md) | 2026-05-16 | `9c60592` | 0/0/0/0 | 0 | 19 |
|
||||
| [ClusterInfrastructure](ClusterInfrastructure/findings.md) | 2026-05-16 | `9c60592` | 0/0/0/0 | 0 | 8 |
|
||||
| [Commons](Commons/findings.md) | 2026-05-16 | `9c60592` | 0/0/0/1 | 1 | 12 |
|
||||
| [Commons](Commons/findings.md) | 2026-05-16 | `9c60592` | 0/0/0/0 | 0 | 12 |
|
||||
| [Communication](Communication/findings.md) | 2026-05-16 | `9c60592` | 0/0/0/0 | 0 | 11 |
|
||||
| [ConfigurationDatabase](ConfigurationDatabase/findings.md) | 2026-05-16 | `9c60592` | 0/0/0/0 | 0 | 11 |
|
||||
| [DataConnectionLayer](DataConnectionLayer/findings.md) | 2026-05-16 | `9c60592` | 0/0/0/0 | 0 | 13 |
|
||||
| [DeploymentManager](DeploymentManager/findings.md) | 2026-05-16 | `9c60592` | 0/0/0/1 | 1 | 14 |
|
||||
| [ExternalSystemGateway](ExternalSystemGateway/findings.md) | 2026-05-16 | `9c60592` | 0/0/0/1 | 1 | 14 |
|
||||
| [DeploymentManager](DeploymentManager/findings.md) | 2026-05-16 | `9c60592` | 0/0/0/0 | 0 | 14 |
|
||||
| [ExternalSystemGateway](ExternalSystemGateway/findings.md) | 2026-05-16 | `9c60592` | 0/0/0/0 | 0 | 14 |
|
||||
| [HealthMonitoring](HealthMonitoring/findings.md) | 2026-05-16 | `9c60592` | 0/0/0/0 | 0 | 12 |
|
||||
| [Host](Host/findings.md) | 2026-05-16 | `9c60592` | 0/0/1/0 | 1 | 11 |
|
||||
| [InboundAPI](InboundAPI/findings.md) | 2026-05-16 | `9c60592` | 0/0/1/1 | 2 | 13 |
|
||||
| [Host](Host/findings.md) | 2026-05-16 | `9c60592` | 0/0/0/0 | 0 | 11 |
|
||||
| [InboundAPI](InboundAPI/findings.md) | 2026-05-16 | `9c60592` | 0/0/0/0 | 0 | 13 |
|
||||
| [ManagementService](ManagementService/findings.md) | 2026-05-16 | `9c60592` | 0/0/0/0 | 0 | 13 |
|
||||
| [NotificationService](NotificationService/findings.md) | 2026-05-16 | `9c60592` | 0/0/0/0 | 0 | 13 |
|
||||
| [Security](Security/findings.md) | 2026-05-16 | `9c60592` | 0/0/0/0 | 0 | 11 |
|
||||
@@ -84,20 +84,10 @@ _None open._
|
||||
|
||||
_None open._
|
||||
|
||||
### Medium (4)
|
||||
### Medium (0)
|
||||
|
||||
| ID | Module | Title |
|
||||
|----|--------|-------|
|
||||
| CentralUI-005 | [CentralUI](CentralUI/findings.md) | Session expiry implementation diverges from the documented policy |
|
||||
| CentralUI-006 | [CentralUI](CentralUI/findings.md) | Deployment status page polls every 10s despite the documented SignalR-push design |
|
||||
| Host-002 | [Host](Host/findings.md) | Akka.Persistence required by REQ-HOST-6 is not configured and not used |
|
||||
| InboundAPI-007 | [InboundAPI](InboundAPI/findings.md) | `Database.Connection()` script API from the design doc is not implemented |
|
||||
_None open._
|
||||
|
||||
### Low (4)
|
||||
### Low (0)
|
||||
|
||||
| ID | Module | Title |
|
||||
|----|--------|-------|
|
||||
| Commons-008 | [Commons](Commons/findings.md) | `SetConnectionBindingsCommand` uses `ValueTuple` in a wire message contract |
|
||||
| DeploymentManager-013 | [DeploymentManager](DeploymentManager/findings.md) | SMTP credentials serialized and broadcast to all sites |
|
||||
| ExternalSystemGateway-011 | [ExternalSystemGateway](ExternalSystemGateway/findings.md) | Every call performs a full repository scan of all systems and methods |
|
||||
| InboundAPI-012 | [InboundAPI](InboundAPI/findings.md) | `ParameterDefinition` POCO declared in the component project, not Commons |
|
||||
_None open._
|
||||
|
||||
@@ -111,6 +111,26 @@ A deployment to a site includes the flattened instance configuration plus any sy
|
||||
|
||||
System-wide artifact deployment is a **separate action** from instance deployment, triggered explicitly by a user with the Deployment role. Artifacts can be deployed to all sites at once or to an individual site (per-site deployment via the Sites admin page).
|
||||
|
||||
### Secret handling in artifacts
|
||||
|
||||
The SMTP configuration artifact carries the SMTP credential (password or OAuth2
|
||||
client secret). This is a **conscious, accepted design decision**: SMTP
|
||||
configuration is a deployable artifact, so the credential is distributed to
|
||||
sites that need it. The credential is protected by the following controls:
|
||||
|
||||
- **In transit** — artifact-deployment commands travel over the inter-cluster
|
||||
transport, which is TLS-protected (see Cluster Infrastructure / Communication).
|
||||
- **Not logged** — the Deployment Manager never writes credential values to
|
||||
logs; deployment log statements reference only site IDs/names, the deployment
|
||||
ID, and exception messages.
|
||||
- **At rest on the site** — the credential is stored in the site's local SQLite
|
||||
artifact store. At-rest encryption of that field is **not** currently applied;
|
||||
it is treated as acceptable given the TLS-protected transport, the absence of
|
||||
any logging leak, and the trust boundary of the site host. Encrypting the
|
||||
credential field within the artifact payload would require a key-management
|
||||
scheme (key location and distribution to sites) and is recorded here as a
|
||||
possible future hardening item, not a current requirement.
|
||||
|
||||
## Site-Side Apply Atomicity
|
||||
|
||||
Applying a deployment at the site is **all-or-nothing per instance**:
|
||||
|
||||
@@ -13,7 +13,7 @@ All nodes (central and site).
|
||||
- Serve as the single entry point (`Program.cs`) for the ScadaLink process.
|
||||
- Read and validate node configuration at startup before any actor system is created.
|
||||
- Register the correct set of component services and actors based on the configured node role.
|
||||
- Bootstrap the Akka.NET actor system with Remoting, Clustering, Persistence, and split-brain resolution via Akka.Hosting.
|
||||
- Bootstrap the Akka.NET actor system with Remoting, Clustering, and split-brain resolution via Akka.Hosting.
|
||||
- Host ASP.NET Core web endpoints on central nodes only.
|
||||
- Configure structured logging (Serilog) with environment-specific enrichment.
|
||||
- Support running as a Windows Service in production and as a console application during development.
|
||||
@@ -109,6 +109,14 @@ The Host must configure the Akka.NET actor system using Akka.Hosting with:
|
||||
- **Split-Brain Resolver**: Configured with the strategy and stable-after duration from `ClusterConfiguration`.
|
||||
- **Actor registration**: Each component's actors registered via its `AddXxxActors()` extension method, conditional on the node's role.
|
||||
|
||||
> **Persistence note.** ScadaLink does not use Akka.Persistence. Durable state
|
||||
> (store-and-forward buffers, site event logs, static attribute writes,
|
||||
> deployment records, configuration) is owned by the individual components and
|
||||
> persisted through component-owned stores — SQLite at sites, MS SQL centrally —
|
||||
> not through an Akka journal/snapshot store. The Host therefore configures no
|
||||
> `akka.persistence` section and references no persistence plugin. There are no
|
||||
> `PersistentActor` subclasses in the system by design.
|
||||
|
||||
### REQ-HOST-6a: ClusterClientReceptionist (Central Only)
|
||||
|
||||
On central nodes, the Host must configure the Akka.NET **ClusterClientReceptionist** and register the ManagementActor with it. This allows external processes (e.g., the CLI) to discover and communicate with the ManagementActor via ClusterClient without joining the cluster as full members. The receptionist is started as part of the Akka.NET bootstrap (REQ-HOST-6) on central nodes only.
|
||||
@@ -187,7 +195,7 @@ The Host's `Program.cs` calls these extension methods; the component libraries o
|
||||
|
||||
- **All 17 component libraries**: The Host references every component project to call their extension methods (excludes CLI, which is a separate executable).
|
||||
- **Akka.Hosting**: For `AddAkka()` and the hosting configuration builder.
|
||||
- **Akka.Remote.Hosting, Akka.Cluster.Hosting, Akka.Persistence.Hosting**: For Akka subsystem configuration.
|
||||
- **Akka.Remote.Hosting, Akka.Cluster.Hosting**: For Akka subsystem configuration. (No Akka.Persistence plugin — see the Persistence note under REQ-HOST-6.)
|
||||
- **Serilog.AspNetCore**: For structured logging integration.
|
||||
- **Microsoft.Extensions.Hosting.WindowsServices**: For Windows Service support.
|
||||
- **ASP.NET Core** (central only): For web endpoint hosting.
|
||||
|
||||
@@ -160,8 +160,15 @@ Inbound API scripts **cannot** call shared scripts directly — shared scripts a
|
||||
- `Parameters["key"]` — Raw dictionary access.
|
||||
- `Parameters.Get<T>("key")` — Typed access (same API as site runtime scripts). See Site Runtime component for full type support.
|
||||
|
||||
#### Database Access
|
||||
- `Database.Connection("connectionName")` — Obtain a raw MS SQL client connection for querying the configuration or machine data databases directly from central.
|
||||
> **No direct database access.** Inbound API scripts are not given a raw database
|
||||
> client. Handing a script a raw `SqlConnection` is in direct tension with the
|
||||
> ScadaLink script trust model (scripts are forbidden `System.IO`, `Process`,
|
||||
> `Threading`, `Reflection`, and raw network access; `ForbiddenApiChecker`
|
||||
> statically enforces this). Scripts interact with the system only through the
|
||||
> curated `Route` and `Parameters` surfaces above. If a method needs data from
|
||||
> the configuration or machine-data databases, that access belongs behind a
|
||||
> dedicated, scoped helper — not a general-purpose connection — and would be
|
||||
> added here as an explicit design change.
|
||||
|
||||
### Routing Behavior
|
||||
- The `Route.To()` helper resolves the instance's site assignment from the configuration database and routes the request to the correct site cluster via the Communication Layer.
|
||||
|
||||
@@ -73,7 +73,7 @@ public static class InstanceCommands
|
||||
/// </summary>
|
||||
internal static bool TryParseBindings(
|
||||
string json,
|
||||
out List<(string, int)>? bindings,
|
||||
out List<ConnectionBinding>? bindings,
|
||||
out string? error)
|
||||
{
|
||||
bindings = null;
|
||||
@@ -88,7 +88,7 @@ public static class InstanceCommands
|
||||
return false;
|
||||
}
|
||||
|
||||
var result = new List<(string, int)>(pairs.Count);
|
||||
var result = new List<ConnectionBinding>(pairs.Count);
|
||||
foreach (var pair in pairs)
|
||||
{
|
||||
if (pair.Count != 2)
|
||||
@@ -107,7 +107,7 @@ public static class InstanceCommands
|
||||
error = "The second element of each binding (dataConnectionId) must be an integer.";
|
||||
return false;
|
||||
}
|
||||
result.Add((pair[0].GetString()!, connectionId));
|
||||
result.Add(new ConnectionBinding(pair[0].GetString()!, connectionId));
|
||||
}
|
||||
|
||||
bindings = result;
|
||||
|
||||
@@ -44,15 +44,17 @@ public static class AuthEndpoints
|
||||
// Map LDAP groups to roles
|
||||
var roleMappingResult = await roleMapper.MapGroupsToRolesAsync(authResult.Groups ?? []);
|
||||
|
||||
var expiresAt = DateTimeOffset.UtcNow.AddMinutes(30);
|
||||
|
||||
// Build claims from LDAP auth + role mapping
|
||||
// Build claims from LDAP auth + role mapping.
|
||||
// CentralUI-005: no fixed "expires_at" absolute-cap claim is stamped
|
||||
// — session expiry is owned by the cookie middleware's sliding window
|
||||
// (ScadaLink.Security AddCookie: ExpireTimeSpan = idle timeout,
|
||||
// SlidingExpiration = true). A frozen absolute claim would contradict
|
||||
// the documented sliding-refresh policy.
|
||||
var claims = new List<Claim>
|
||||
{
|
||||
new(ClaimTypes.Name, authResult.Username ?? username),
|
||||
new(JwtTokenService.DisplayNameClaimType, authResult.DisplayName ?? username),
|
||||
new(JwtTokenService.UsernameClaimType, authResult.Username ?? username),
|
||||
new("expires_at", expiresAt.ToUnixTimeSeconds().ToString()),
|
||||
};
|
||||
|
||||
foreach (var role in roleMappingResult.Roles)
|
||||
@@ -74,11 +76,7 @@ public static class AuthEndpoints
|
||||
await context.SignInAsync(
|
||||
CookieAuthenticationDefaults.AuthenticationScheme,
|
||||
principal,
|
||||
new AuthenticationProperties
|
||||
{
|
||||
IsPersistent = true,
|
||||
ExpiresUtc = expiresAt
|
||||
});
|
||||
BuildSignInProperties());
|
||||
|
||||
context.Response.Redirect("/");
|
||||
}).DisableAntiforgery();
|
||||
@@ -138,4 +136,22 @@ public static class AuthEndpoints
|
||||
|
||||
return endpoints;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Builds the <see cref="AuthenticationProperties"/> for the login sign-in.
|
||||
/// CentralUI-005: deliberately does <b>not</b> set <see cref="AuthenticationProperties.ExpiresUtc"/>.
|
||||
/// Session expiry is owned by the cookie authentication middleware's sliding
|
||||
/// window (configured in <c>ScadaLink.Security</c>'s <c>AddCookie</c>:
|
||||
/// <c>ExpireTimeSpan</c> = the idle timeout, <c>SlidingExpiration = true</c>).
|
||||
/// Setting a fixed <c>ExpiresUtc</c> here would re-impose a hard absolute cap
|
||||
/// that overrides the sliding window and contradicts the documented
|
||||
/// "sliding refresh, 30-minute idle timeout" policy. <see cref="AuthenticationProperties.IsPersistent"/>
|
||||
/// is true so the cookie survives a browser restart within the idle window;
|
||||
/// <see cref="AuthenticationProperties.AllowRefresh"/> is left unset (null)
|
||||
/// so the middleware is free to slide the expiry on activity.
|
||||
/// </summary>
|
||||
public static AuthenticationProperties BuildSignInProperties() => new()
|
||||
{
|
||||
IsPersistent = true
|
||||
};
|
||||
}
|
||||
|
||||
@@ -8,6 +8,7 @@
|
||||
@inject IDeploymentManagerRepository DeploymentManagerRepository
|
||||
@inject ITemplateEngineRepository TemplateEngineRepository
|
||||
@inject ScadaLink.CentralUI.Auth.SiteScopeService SiteScope
|
||||
@inject ScadaLink.DeploymentManager.IDeploymentStatusNotifier DeploymentStatusNotifier
|
||||
@implements IDisposable
|
||||
|
||||
<div class="container-fluid mt-3">
|
||||
@@ -196,47 +197,42 @@
|
||||
private Dictionary<int, string> _instanceNames = new();
|
||||
private bool _loading = true;
|
||||
private string? _errorMessage;
|
||||
private Timer? _refreshTimer;
|
||||
private bool _autoRefresh = true;
|
||||
private readonly HashSet<string> _expandedErrors = new();
|
||||
|
||||
private int _currentPage = 1;
|
||||
private int _totalPages;
|
||||
private const int PageSize = 25;
|
||||
private static readonly TimeSpan RefreshInterval = TimeSpan.FromSeconds(10);
|
||||
|
||||
// CentralUI-006: deployment status updates are push-based, not polled.
|
||||
// DeploymentManager raises IDeploymentStatusNotifier.StatusChanged on every
|
||||
// deployment-record status write; this page subscribes to it and reloads,
|
||||
// and Blazor Server pushes the re-render to the browser over its SignalR
|
||||
// circuit — satisfying the design's "no polling required" requirement.
|
||||
// The notifier event is raised on the DeploymentManager service thread, so
|
||||
// the handler marshals onto the renderer via InvokeAsync.
|
||||
|
||||
protected override async Task OnInitializedAsync()
|
||||
{
|
||||
await LoadDataAsync();
|
||||
StartTimer();
|
||||
DeploymentStatusNotifier.StatusChanged += OnDeploymentStatusChanged;
|
||||
}
|
||||
|
||||
private void StartTimer()
|
||||
private void OnDeploymentStatusChanged(ScadaLink.DeploymentManager.DeploymentStatusChange change)
|
||||
{
|
||||
_refreshTimer?.Dispose();
|
||||
_refreshTimer = new Timer(_ =>
|
||||
if (!_autoRefresh) return;
|
||||
_ = InvokeAsync(async () =>
|
||||
{
|
||||
InvokeAsync(async () =>
|
||||
{
|
||||
if (!_autoRefresh) return;
|
||||
await LoadDataAsync();
|
||||
StateHasChanged();
|
||||
});
|
||||
}, null, RefreshInterval, RefreshInterval);
|
||||
await LoadDataAsync();
|
||||
StateHasChanged();
|
||||
});
|
||||
}
|
||||
|
||||
private void ToggleAutoRefresh()
|
||||
{
|
||||
// When paused, incoming push notifications are ignored; "Refresh" still
|
||||
// forces a manual reload. No timer is involved either way.
|
||||
_autoRefresh = !_autoRefresh;
|
||||
if (_autoRefresh)
|
||||
{
|
||||
StartTimer();
|
||||
}
|
||||
else
|
||||
{
|
||||
_refreshTimer?.Dispose();
|
||||
_refreshTimer = null;
|
||||
}
|
||||
}
|
||||
|
||||
private bool IsErrorExpanded(string deploymentId) => _expandedErrors.Contains(deploymentId);
|
||||
@@ -320,6 +316,8 @@
|
||||
|
||||
public void Dispose()
|
||||
{
|
||||
_refreshTimer?.Dispose();
|
||||
// Unsubscribe so a status change after the circuit is gone does not
|
||||
// touch a disposed component (the notifier is a process singleton).
|
||||
DeploymentStatusNotifier.StatusChanged -= OnDeploymentStatusChanged;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -4,6 +4,7 @@
|
||||
@using ScadaLink.Commons.Entities.Sites
|
||||
@using ScadaLink.Commons.Entities.Templates
|
||||
@using ScadaLink.Commons.Interfaces.Repositories
|
||||
@using ScadaLink.Commons.Messages.Management
|
||||
@using ScadaLink.Commons.Types.Enums
|
||||
@using ScadaLink.TemplateEngine.Flattening
|
||||
@using ScadaLink.TemplateEngine.Services
|
||||
@@ -467,7 +468,7 @@
|
||||
_saving = true;
|
||||
try
|
||||
{
|
||||
var bindings = _bindingSelections.Select(kv => (kv.Key, kv.Value)).ToList();
|
||||
var bindings = _bindingSelections.Select(kv => new ConnectionBinding(kv.Key, kv.Value)).ToList();
|
||||
var user = await GetCurrentUserAsync();
|
||||
var result = await InstanceService.SetConnectionBindingsAsync(Id, bindings, user);
|
||||
if (result.IsSuccess)
|
||||
|
||||
@@ -3,37 +3,62 @@
|
||||
@inject NavigationManager Navigation
|
||||
|
||||
@code {
|
||||
// CentralUI-005: session expiry is a sliding window owned by the cookie
|
||||
// authentication middleware (ScadaLink.Security AddCookie:
|
||||
// ExpireTimeSpan = idle timeout, SlidingExpiration = true). An active user's
|
||||
// cookie is continually renewed; an idle user's cookie lapses after the idle
|
||||
// timeout. There is therefore no fixed login-time deadline to redirect at —
|
||||
// the old code read an "expires_at" claim and scheduled a single hard
|
||||
// redirect, which both contradicted the sliding policy and logged active
|
||||
// users out mid-session.
|
||||
//
|
||||
// Instead this component polls the authentication state on a recurring
|
||||
// interval. While the session is still valid it does nothing; once the
|
||||
// sliding cookie has expired (the server-side idle cutoff has been reached)
|
||||
// the next poll observes an unauthenticated principal and redirects to the
|
||||
// login page. Re-checking the state is itself circuit activity, so this poll
|
||||
// alone never keeps a truly idle session alive — only genuine user activity
|
||||
// refreshes the cookie before it lapses.
|
||||
|
||||
/// <summary>How often the session validity is re-checked.</summary>
|
||||
internal static readonly TimeSpan PollInterval = TimeSpan.FromMinutes(1);
|
||||
|
||||
private CancellationTokenSource? _cts;
|
||||
|
||||
protected override async Task OnInitializedAsync()
|
||||
protected override void OnInitialized()
|
||||
{
|
||||
// The login page uses the same layout, so this component renders there
|
||||
// too. Redirecting /login → /login would loop ("too many redirects").
|
||||
// too. Polling/redirecting on /login → /login would loop.
|
||||
var path = Navigation.ToBaseRelativePath(Navigation.Uri);
|
||||
if (path.StartsWith("login", StringComparison.OrdinalIgnoreCase)) return;
|
||||
|
||||
var auth = await AuthStateProvider.GetAuthenticationStateAsync();
|
||||
if (auth.User.Identity?.IsAuthenticated != true) return;
|
||||
|
||||
var claim = auth.User.FindFirst("expires_at")?.Value;
|
||||
if (!long.TryParse(claim, out var unix)) return;
|
||||
|
||||
var remaining = DateTimeOffset.FromUnixTimeSeconds(unix) - DateTimeOffset.UtcNow;
|
||||
if (remaining <= TimeSpan.Zero)
|
||||
{
|
||||
Navigation.NavigateTo("/login", forceLoad: true);
|
||||
return;
|
||||
}
|
||||
|
||||
_cts = new CancellationTokenSource();
|
||||
_ = ScheduleRedirectAsync(remaining + TimeSpan.FromSeconds(2), _cts.Token);
|
||||
_ = PollSessionAsync(_cts.Token);
|
||||
}
|
||||
|
||||
private async Task ScheduleRedirectAsync(TimeSpan delay, CancellationToken token)
|
||||
private async Task PollSessionAsync(CancellationToken token)
|
||||
{
|
||||
try { await Task.Delay(delay, token); }
|
||||
catch (TaskCanceledException) { return; }
|
||||
await InvokeAsync(() => Navigation.NavigateTo("/login", forceLoad: true));
|
||||
while (!token.IsCancellationRequested)
|
||||
{
|
||||
try { await Task.Delay(PollInterval, token); }
|
||||
catch (TaskCanceledException) { return; }
|
||||
|
||||
AuthenticationState auth;
|
||||
try
|
||||
{
|
||||
auth = await AuthStateProvider.GetAuthenticationStateAsync();
|
||||
}
|
||||
catch (ObjectDisposedException)
|
||||
{
|
||||
return;
|
||||
}
|
||||
|
||||
if (auth.User.Identity?.IsAuthenticated != true)
|
||||
{
|
||||
await InvokeAsync(() => Navigation.NavigateTo("/login", forceLoad: true));
|
||||
return;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
public void Dispose()
|
||||
|
||||
@@ -6,6 +6,15 @@ public interface IExternalSystemRepository
|
||||
{
|
||||
// ExternalSystemDefinition
|
||||
Task<ExternalSystemDefinition?> GetExternalSystemByIdAsync(int id, CancellationToken cancellationToken = default);
|
||||
|
||||
/// <summary>
|
||||
/// Returns the external system with the given name, or <c>null</c> if no such
|
||||
/// system exists. A name-keyed lookup so hot-path resolution (e.g. a script's
|
||||
/// <c>ExternalSystem.Call()</c>) does not have to fetch every system and filter
|
||||
/// in memory on each call (ExternalSystemGateway-011).
|
||||
/// </summary>
|
||||
Task<ExternalSystemDefinition?> GetExternalSystemByNameAsync(string name, CancellationToken cancellationToken = default);
|
||||
|
||||
Task<IReadOnlyList<ExternalSystemDefinition>> GetAllExternalSystemsAsync(CancellationToken cancellationToken = default);
|
||||
Task AddExternalSystemAsync(ExternalSystemDefinition definition, CancellationToken cancellationToken = default);
|
||||
Task UpdateExternalSystemAsync(ExternalSystemDefinition definition, CancellationToken cancellationToken = default);
|
||||
@@ -13,6 +22,15 @@ public interface IExternalSystemRepository
|
||||
|
||||
// ExternalSystemMethod
|
||||
Task<ExternalSystemMethod?> GetExternalSystemMethodByIdAsync(int id, CancellationToken cancellationToken = default);
|
||||
|
||||
/// <summary>
|
||||
/// Returns the method with the given name belonging to the given external system,
|
||||
/// or <c>null</c> if no such method exists. A name-keyed lookup so hot-path
|
||||
/// resolution does not have to fetch every method of the system and filter in
|
||||
/// memory on each call (ExternalSystemGateway-011).
|
||||
/// </summary>
|
||||
Task<ExternalSystemMethod?> GetMethodByNameAsync(int externalSystemId, string methodName, CancellationToken cancellationToken = default);
|
||||
|
||||
Task<IReadOnlyList<ExternalSystemMethod>> GetMethodsByExternalSystemIdAsync(int externalSystemId, CancellationToken cancellationToken = default);
|
||||
Task AddExternalSystemMethodAsync(ExternalSystemMethod method, CancellationToken cancellationToken = default);
|
||||
Task UpdateExternalSystemMethodAsync(ExternalSystemMethod method, CancellationToken cancellationToken = default);
|
||||
@@ -20,6 +38,16 @@ public interface IExternalSystemRepository
|
||||
|
||||
// DatabaseConnectionDefinition
|
||||
Task<DatabaseConnectionDefinition?> GetDatabaseConnectionByIdAsync(int id, CancellationToken cancellationToken = default);
|
||||
|
||||
/// <summary>
|
||||
/// Returns the database connection definition with the given name, or <c>null</c>
|
||||
/// if no such connection exists. A name-keyed lookup so hot-path resolution (e.g.
|
||||
/// a script's <c>Database.Connection()</c> / <c>Database.CachedWrite()</c>) does
|
||||
/// not have to fetch every connection and filter in memory on each call
|
||||
/// (ExternalSystemGateway-011).
|
||||
/// </summary>
|
||||
Task<DatabaseConnectionDefinition?> GetDatabaseConnectionByNameAsync(string name, CancellationToken cancellationToken = default);
|
||||
|
||||
Task<IReadOnlyList<DatabaseConnectionDefinition>> GetAllDatabaseConnectionsAsync(CancellationToken cancellationToken = default);
|
||||
Task AddDatabaseConnectionAsync(DatabaseConnectionDefinition definition, CancellationToken cancellationToken = default);
|
||||
Task UpdateDatabaseConnectionAsync(DatabaseConnectionDefinition definition, CancellationToken cancellationToken = default);
|
||||
|
||||
@@ -7,7 +7,15 @@ public record MgmtDeployInstanceCommand(int InstanceId);
|
||||
public record MgmtEnableInstanceCommand(int InstanceId);
|
||||
public record MgmtDisableInstanceCommand(int InstanceId);
|
||||
public record MgmtDeleteInstanceCommand(int InstanceId);
|
||||
public record SetConnectionBindingsCommand(int InstanceId, IReadOnlyList<(string AttributeName, int DataConnectionId)> Bindings);
|
||||
/// <summary>
|
||||
/// A single attribute-to-data-connection binding carried by
|
||||
/// <see cref="SetConnectionBindingsCommand"/>. This is a named record (not a
|
||||
/// <c>ValueTuple</c>) so it serializes with stable, named JSON properties and can
|
||||
/// evolve additively per REQ-COM-5a.
|
||||
/// </summary>
|
||||
public record ConnectionBinding(string AttributeName, int DataConnectionId);
|
||||
|
||||
public record SetConnectionBindingsCommand(int InstanceId, IReadOnlyList<ConnectionBinding> Bindings);
|
||||
public record SetInstanceOverridesCommand(int InstanceId, IReadOnlyDictionary<string, string?> Overrides);
|
||||
public record SetInstanceAreaCommand(int InstanceId, int? AreaId);
|
||||
|
||||
|
||||
@@ -0,0 +1,15 @@
|
||||
namespace ScadaLink.Commons.Types.InboundApi;
|
||||
|
||||
/// <summary>
|
||||
/// Defines a single parameter in an inbound API method's parameter definitions.
|
||||
/// This is the deserialized, persistence-ignorant form of the JSON stored in
|
||||
/// <c>ApiMethod.ParameterDefinitions</c> and describes the public API contract,
|
||||
/// so it is shared by every component that reads or produces method parameter
|
||||
/// definitions (Inbound API, Central UI method editor, CLI, Management Service).
|
||||
/// </summary>
|
||||
public class ParameterDefinition
|
||||
{
|
||||
public string Name { get; set; } = string.Empty;
|
||||
public string Type { get; set; } = "String";
|
||||
public bool Required { get; set; } = true;
|
||||
}
|
||||
@@ -16,6 +16,12 @@ public class ExternalSystemRepository : IExternalSystemRepository
|
||||
public async Task<ExternalSystemDefinition?> GetExternalSystemByIdAsync(int id, CancellationToken cancellationToken = default)
|
||||
=> await _context.Set<ExternalSystemDefinition>().FindAsync(new object[] { id }, cancellationToken);
|
||||
|
||||
// ExternalSystemGateway-011: genuine name-keyed query (server-side WHERE) so the
|
||||
// gateway's hot-path resolution does not fetch every system and filter in memory.
|
||||
public async Task<ExternalSystemDefinition?> GetExternalSystemByNameAsync(string name, CancellationToken cancellationToken = default)
|
||||
=> await _context.Set<ExternalSystemDefinition>()
|
||||
.FirstOrDefaultAsync(s => s.Name == name, cancellationToken);
|
||||
|
||||
public async Task<IReadOnlyList<ExternalSystemDefinition>> GetAllExternalSystemsAsync(CancellationToken cancellationToken = default)
|
||||
=> await _context.Set<ExternalSystemDefinition>().ToListAsync(cancellationToken);
|
||||
|
||||
@@ -34,6 +40,13 @@ public class ExternalSystemRepository : IExternalSystemRepository
|
||||
public async Task<ExternalSystemMethod?> GetExternalSystemMethodByIdAsync(int id, CancellationToken cancellationToken = default)
|
||||
=> await _context.Set<ExternalSystemMethod>().FindAsync(new object[] { id }, cancellationToken);
|
||||
|
||||
// ExternalSystemGateway-011: genuine name-keyed query scoped to the parent system.
|
||||
public async Task<ExternalSystemMethod?> GetMethodByNameAsync(int externalSystemId, string methodName, CancellationToken cancellationToken = default)
|
||||
=> await _context.Set<ExternalSystemMethod>()
|
||||
.FirstOrDefaultAsync(
|
||||
m => m.ExternalSystemDefinitionId == externalSystemId && m.Name == methodName,
|
||||
cancellationToken);
|
||||
|
||||
public async Task<IReadOnlyList<ExternalSystemMethod>> GetMethodsByExternalSystemIdAsync(int externalSystemId, CancellationToken cancellationToken = default)
|
||||
=> await _context.Set<ExternalSystemMethod>().Where(m => m.ExternalSystemDefinitionId == externalSystemId).ToListAsync(cancellationToken);
|
||||
|
||||
@@ -52,6 +65,11 @@ public class ExternalSystemRepository : IExternalSystemRepository
|
||||
public async Task<DatabaseConnectionDefinition?> GetDatabaseConnectionByIdAsync(int id, CancellationToken cancellationToken = default)
|
||||
=> await _context.Set<DatabaseConnectionDefinition>().FindAsync(new object[] { id }, cancellationToken);
|
||||
|
||||
// ExternalSystemGateway-011: genuine name-keyed query (server-side WHERE).
|
||||
public async Task<DatabaseConnectionDefinition?> GetDatabaseConnectionByNameAsync(string name, CancellationToken cancellationToken = default)
|
||||
=> await _context.Set<DatabaseConnectionDefinition>()
|
||||
.FirstOrDefaultAsync(c => c.Name == name, cancellationToken);
|
||||
|
||||
public async Task<IReadOnlyList<DatabaseConnectionDefinition>> GetAllDatabaseConnectionsAsync(CancellationToken cancellationToken = default)
|
||||
=> await _context.Set<DatabaseConnectionDefinition>().ToListAsync(cancellationToken);
|
||||
|
||||
|
||||
@@ -41,6 +41,7 @@ public class DeploymentService
|
||||
private readonly OperationLockManager _lockManager;
|
||||
private readonly IAuditService _auditService;
|
||||
private readonly DiffService _diffService;
|
||||
private readonly IDeploymentStatusNotifier _statusNotifier;
|
||||
private readonly DeploymentManagerOptions _options;
|
||||
private readonly ILogger<DeploymentService> _logger;
|
||||
|
||||
@@ -60,6 +61,7 @@ public class DeploymentService
|
||||
OperationLockManager lockManager,
|
||||
IAuditService auditService,
|
||||
DiffService diffService,
|
||||
IDeploymentStatusNotifier statusNotifier,
|
||||
IOptions<DeploymentManagerOptions> options,
|
||||
ILogger<DeploymentService> logger)
|
||||
{
|
||||
@@ -70,10 +72,21 @@ public class DeploymentService
|
||||
_lockManager = lockManager;
|
||||
_auditService = auditService;
|
||||
_diffService = diffService;
|
||||
_statusNotifier = statusNotifier;
|
||||
_options = options.Value;
|
||||
_logger = logger;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// CentralUI-006: raises a push notification that a deployment record's
|
||||
/// status was just persisted, so the Central UI deployment-status page can
|
||||
/// re-render over its SignalR circuit instead of polling. Called at every
|
||||
/// point a <see cref="DeploymentRecord"/> status is written.
|
||||
/// </summary>
|
||||
private void NotifyStatusChange(DeploymentRecord record) =>
|
||||
_statusNotifier.NotifyStatusChanged(
|
||||
new DeploymentStatusChange(record.DeploymentId, record.InstanceId, record.Status));
|
||||
|
||||
/// <summary>
|
||||
/// Resolves the site's string identifier from the numeric DB ID.
|
||||
/// The communication layer routes by string identifier (e.g. "site-a"), not DB ID.
|
||||
@@ -155,11 +168,13 @@ public class DeploymentService
|
||||
|
||||
await _repository.AddDeploymentRecordAsync(record, cancellationToken);
|
||||
await _repository.SaveChangesAsync(cancellationToken);
|
||||
NotifyStatusChange(record);
|
||||
|
||||
// Update status to InProgress
|
||||
record.Status = DeploymentStatus.InProgress;
|
||||
await _repository.UpdateDeploymentRecordAsync(record, cancellationToken);
|
||||
await _repository.SaveChangesAsync(cancellationToken);
|
||||
NotifyStatusChange(record);
|
||||
|
||||
try
|
||||
{
|
||||
@@ -187,6 +202,7 @@ public class DeploymentService
|
||||
// non-Success record while the site is running the new config.
|
||||
await _repository.UpdateDeploymentRecordAsync(record, cancellationToken);
|
||||
await _repository.SaveChangesAsync(cancellationToken);
|
||||
NotifyStatusChange(record);
|
||||
|
||||
if (response.Status == DeploymentStatus.Success)
|
||||
{
|
||||
@@ -253,6 +269,7 @@ public class DeploymentService
|
||||
{
|
||||
await _repository.UpdateDeploymentRecordAsync(record, CancellationToken.None);
|
||||
await _repository.SaveChangesAsync(CancellationToken.None);
|
||||
NotifyStatusChange(record);
|
||||
|
||||
await _auditService.LogAsync(user, "DeployFailed", "Instance", instanceId.ToString(),
|
||||
instance.UniqueName, new { DeploymentId = deploymentId, Error = ex.Message },
|
||||
@@ -624,6 +641,7 @@ public class DeploymentService
|
||||
prior.CompletedAt = DateTimeOffset.UtcNow;
|
||||
await _repository.UpdateDeploymentRecordAsync(prior, cancellationToken);
|
||||
await _repository.SaveChangesAsync(cancellationToken);
|
||||
NotifyStatusChange(prior);
|
||||
|
||||
await _auditService.LogAsync(prior.DeployedBy, "DeployReconciled", "Instance",
|
||||
instance.Id.ToString(), instance.UniqueName,
|
||||
|
||||
@@ -0,0 +1,51 @@
|
||||
using Microsoft.Extensions.Logging;
|
||||
|
||||
namespace ScadaLink.DeploymentManager;
|
||||
|
||||
/// <summary>
|
||||
/// Default <see cref="IDeploymentStatusNotifier"/> implementation. A simple
|
||||
/// in-process event broadcaster: registered as a DI singleton so it is shared
|
||||
/// between the central-process <see cref="DeploymentService"/> and the Central
|
||||
/// UI's Blazor circuits (CentralUI-006).
|
||||
///
|
||||
/// A throwing subscriber must never break the deployment pipeline, so each
|
||||
/// handler is invoked individually and its exceptions are caught and logged.
|
||||
/// </summary>
|
||||
public sealed class DeploymentStatusNotifier : IDeploymentStatusNotifier
|
||||
{
|
||||
private readonly ILogger<DeploymentStatusNotifier> _logger;
|
||||
|
||||
public DeploymentStatusNotifier(ILogger<DeploymentStatusNotifier> logger)
|
||||
{
|
||||
_logger = logger;
|
||||
}
|
||||
|
||||
/// <inheritdoc />
|
||||
public event Action<DeploymentStatusChange>? StatusChanged;
|
||||
|
||||
/// <inheritdoc />
|
||||
public void NotifyStatusChanged(DeploymentStatusChange change)
|
||||
{
|
||||
var handlers = StatusChanged;
|
||||
if (handlers == null)
|
||||
return;
|
||||
|
||||
// Invoke each subscriber in isolation: one faulting handler (e.g. a
|
||||
// disposed Blazor circuit) must not stop the others from being notified
|
||||
// and must not propagate back into the deployment pipeline.
|
||||
foreach (var handler in handlers.GetInvocationList())
|
||||
{
|
||||
try
|
||||
{
|
||||
((Action<DeploymentStatusChange>)handler)(change);
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
_logger.LogWarning(ex,
|
||||
"A deployment-status-change subscriber threw for deployment {DeploymentId} " +
|
||||
"(status {Status}); continuing with remaining subscribers",
|
||||
change.DeploymentId, change.Status);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,49 @@
|
||||
using ScadaLink.Commons.Types.Enums;
|
||||
|
||||
namespace ScadaLink.DeploymentManager;
|
||||
|
||||
/// <summary>
|
||||
/// Payload describing a single deployment-record status change. Kept small —
|
||||
/// just the deployment identity, the owning instance, and the new status — so
|
||||
/// it is cheap to raise on the hot path and cheap for subscribers to handle.
|
||||
/// </summary>
|
||||
/// <param name="DeploymentId">The unique deployment ID whose status changed.</param>
|
||||
/// <param name="InstanceId">The instance the deployment record belongs to.</param>
|
||||
/// <param name="Status">The status the deployment record was just written with.</param>
|
||||
public readonly record struct DeploymentStatusChange(
|
||||
string DeploymentId,
|
||||
int InstanceId,
|
||||
DeploymentStatus Status);
|
||||
|
||||
/// <summary>
|
||||
/// CentralUI-006: push-based deployment-status change notification.
|
||||
///
|
||||
/// The design (Component-CentralUI "Real-Time Updates") requires deployment
|
||||
/// status transitions to push to the UI immediately via SignalR, with no
|
||||
/// polling. <see cref="DeploymentService"/> raises <see cref="StatusChanged"/>
|
||||
/// whenever it writes a <see cref="Commons.Entities.Deployment.DeploymentRecord"/>
|
||||
/// status; the Central UI's deployment-status page subscribes to it and
|
||||
/// re-renders over its existing Blazor Server SignalR circuit.
|
||||
///
|
||||
/// Registered as a DI singleton (see <see cref="ServiceCollectionExtensions.AddDeploymentManager"/>)
|
||||
/// so the scoped <see cref="DeploymentService"/> and the Blazor circuit's
|
||||
/// scoped page component share the same instance — both run in the same
|
||||
/// central Host process.
|
||||
/// </summary>
|
||||
public interface IDeploymentStatusNotifier
|
||||
{
|
||||
/// <summary>
|
||||
/// Raised after a deployment record's status has been written. Handlers run
|
||||
/// synchronously on the caller's thread; subscribers must not block and
|
||||
/// should marshal any UI work onto their own dispatcher.
|
||||
/// </summary>
|
||||
event Action<DeploymentStatusChange>? StatusChanged;
|
||||
|
||||
/// <summary>
|
||||
/// Raises <see cref="StatusChanged"/>. Called by <see cref="DeploymentService"/>
|
||||
/// at every point a deployment record's status is persisted. A throwing
|
||||
/// subscriber must not break the deployment pipeline, so handler exceptions
|
||||
/// are swallowed by the implementation.
|
||||
/// </summary>
|
||||
void NotifyStatusChanged(DeploymentStatusChange change);
|
||||
}
|
||||
@@ -27,6 +27,14 @@ public static class ServiceCollectionExtensions
|
||||
// the declared option-class defaults apply.
|
||||
services.AddOptions<DeploymentManagerOptions>();
|
||||
services.AddSingleton<OperationLockManager>();
|
||||
|
||||
// CentralUI-006: push-based deployment-status notification. Registered
|
||||
// as a singleton so the scoped DeploymentService and the Central UI's
|
||||
// scoped Blazor page component share one instance — both run in the
|
||||
// same central Host process. The deployment-status page subscribes to
|
||||
// it instead of polling the database every 10 seconds.
|
||||
services.AddSingleton<IDeploymentStatusNotifier, DeploymentStatusNotifier>();
|
||||
|
||||
services.AddScoped<IFlatteningPipeline, FlatteningPipeline>();
|
||||
services.AddScoped<DeploymentService>();
|
||||
services.AddScoped<ArtifactDeploymentService>();
|
||||
|
||||
@@ -170,8 +170,10 @@ public class DatabaseGateway : IDatabaseGateway
|
||||
string connectionName,
|
||||
CancellationToken cancellationToken)
|
||||
{
|
||||
var connections = await _repository.GetAllDatabaseConnectionsAsync(cancellationToken);
|
||||
return connections.FirstOrDefault(c =>
|
||||
c.Name.Equals(connectionName, StringComparison.OrdinalIgnoreCase));
|
||||
// ExternalSystemGateway-011: name-keyed repository lookup instead of
|
||||
// fetch-all-then-filter — connection definitions are resolved on every
|
||||
// cached write / connection request, so the repository performs an indexed
|
||||
// query rather than loading every connection into memory.
|
||||
return await _repository.GetDatabaseConnectionByNameAsync(connectionName, cancellationToken);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -373,13 +373,15 @@ public class ExternalSystemClient : IExternalSystemClient
|
||||
string methodName,
|
||||
CancellationToken cancellationToken)
|
||||
{
|
||||
var systems = await _repository.GetAllExternalSystemsAsync(cancellationToken);
|
||||
var system = systems.FirstOrDefault(s => s.Name.Equals(systemName, StringComparison.OrdinalIgnoreCase));
|
||||
// ExternalSystemGateway-011: name-keyed repository lookups instead of
|
||||
// fetch-all-then-filter — definitions are resolved on every hot-path call
|
||||
// (a script's ExternalSystem.Call()), so the repository performs an indexed
|
||||
// query rather than loading every system / every method into memory.
|
||||
var system = await _repository.GetExternalSystemByNameAsync(systemName, cancellationToken);
|
||||
if (system == null)
|
||||
return (null, null);
|
||||
|
||||
var methods = await _repository.GetMethodsByExternalSystemIdAsync(system.Id, cancellationToken);
|
||||
var method = methods.FirstOrDefault(m => m.Name.Equals(methodName, StringComparison.OrdinalIgnoreCase));
|
||||
var method = await _repository.GetMethodByNameAsync(system.Id, methodName, cancellationToken);
|
||||
|
||||
return (system, method);
|
||||
}
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
using System.Text.Json;
|
||||
using ScadaLink.Commons.Types.InboundApi;
|
||||
|
||||
namespace ScadaLink.InboundAPI;
|
||||
|
||||
@@ -142,16 +143,6 @@ public static class ParameterValidator
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Defines a parameter in a method's parameter definitions.
|
||||
/// </summary>
|
||||
public class ParameterDefinition
|
||||
{
|
||||
public string Name { get; set; } = string.Empty;
|
||||
public string Type { get; set; } = "String";
|
||||
public bool Required { get; set; } = true;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Result of parameter validation.
|
||||
/// </summary>
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
using Microsoft.AspNetCore.Authentication.Cookies;
|
||||
using Microsoft.Extensions.DependencyInjection;
|
||||
using Microsoft.Extensions.Options;
|
||||
|
||||
namespace ScadaLink.Security;
|
||||
|
||||
@@ -25,6 +26,28 @@ public static class ServiceCollectionExtensions
|
||||
options.Cookie.SecurePolicy = Microsoft.AspNetCore.Http.CookieSecurePolicy.Always;
|
||||
});
|
||||
|
||||
// CentralUI-005: configure the cookie session as a sliding window so the
|
||||
// code matches the documented policy ("sliding refresh, 30-minute idle
|
||||
// timeout"). ASP.NET cookie auth exposes a single ExpireTimeSpan plus a
|
||||
// SlidingExpiration flag — it cannot natively model a 15-minute sliding
|
||||
// token AND a separate 30-minute absolute idle cap. The faithful
|
||||
// interpretation: the cookie window IS the idle timeout
|
||||
// (SecurityOptions.IdleTimeoutMinutes, default 30) and SlidingExpiration
|
||||
// renews it on activity (the middleware re-issues the cookie once past
|
||||
// the halfway mark of the window). An active user is therefore kept
|
||||
// signed in; an idle user is signed out after the idle timeout. The
|
||||
// 15-minute JwtExpiryMinutes governs the lifetime of the embedded JWT
|
||||
// itself (see JwtTokenService) — a separate layer from the cookie
|
||||
// session window. Bound here via PostConfigure so SecurityOptions
|
||||
// (configured by the Host after AddSecurity) is honoured.
|
||||
services.AddOptions<CookieAuthenticationOptions>(CookieAuthenticationDefaults.AuthenticationScheme)
|
||||
.Configure<IOptions<SecurityOptions>>((cookieOptions, securityOptions) =>
|
||||
{
|
||||
var idleMinutes = securityOptions.Value.IdleTimeoutMinutes;
|
||||
cookieOptions.ExpireTimeSpan = TimeSpan.FromMinutes(idleMinutes);
|
||||
cookieOptions.SlidingExpiration = true;
|
||||
});
|
||||
|
||||
services.AddScadaLinkAuthorization();
|
||||
|
||||
return services;
|
||||
|
||||
@@ -53,6 +53,31 @@ public class SiteExternalSystemRepository : IExternalSystemRepository
|
||||
return all.FirstOrDefault(e => e.Id == id);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// ExternalSystemGateway-011: genuine name-keyed lookup. The <c>external_systems</c>
|
||||
/// table has <c>name</c> as its PRIMARY KEY, so this is a single indexed-row fetch
|
||||
/// rather than the previous fetch-all-then-filter.
|
||||
/// </summary>
|
||||
public async Task<ExternalSystemDefinition?> GetExternalSystemByNameAsync(
|
||||
string name, CancellationToken cancellationToken = default)
|
||||
{
|
||||
await using var connection = CreateConnection();
|
||||
await connection.OpenAsync(cancellationToken);
|
||||
|
||||
await using var command = connection.CreateCommand();
|
||||
command.CommandText = @"
|
||||
SELECT name, endpoint_url, auth_type, auth_configuration
|
||||
FROM external_systems
|
||||
WHERE name = @name";
|
||||
command.Parameters.AddWithValue("@name", name);
|
||||
|
||||
await using var reader = await command.ExecuteReaderAsync(cancellationToken);
|
||||
if (!await reader.ReadAsync(cancellationToken))
|
||||
return null;
|
||||
|
||||
return MapExternalSystem(reader);
|
||||
}
|
||||
|
||||
// ── ExternalSystemMethod (read) ──
|
||||
|
||||
public async Task<IReadOnlyList<ExternalSystemMethod>> GetMethodsByExternalSystemIdAsync(
|
||||
@@ -80,6 +105,23 @@ public class SiteExternalSystemRepository : IExternalSystemRepository
|
||||
return ParseMethodDefinitions(json, externalSystemId);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// ExternalSystemGateway-011: name-keyed method lookup. The site store keeps a
|
||||
/// system's methods in a single JSON column, so this fetches that one row and
|
||||
/// matches the named method in memory. The parent system is resolved via
|
||||
/// <see cref="GetMethodsByExternalSystemIdAsync"/>, which already performs a
|
||||
/// single keyed read of the <c>method_definitions</c> column — this is no more
|
||||
/// I/O than the gateway's previous fetch-all-methods-then-filter call, and on
|
||||
/// the SQL-backed Central repository it is a genuine indexed query.
|
||||
/// </summary>
|
||||
public async Task<ExternalSystemMethod?> GetMethodByNameAsync(
|
||||
int externalSystemId, string methodName, CancellationToken cancellationToken = default)
|
||||
{
|
||||
var methods = await GetMethodsByExternalSystemIdAsync(externalSystemId, cancellationToken);
|
||||
return methods.FirstOrDefault(
|
||||
m => m.Name.Equals(methodName, StringComparison.OrdinalIgnoreCase));
|
||||
}
|
||||
|
||||
public async Task<ExternalSystemMethod?> GetExternalSystemMethodByIdAsync(
|
||||
int id, CancellationToken cancellationToken = default)
|
||||
{
|
||||
@@ -134,6 +176,38 @@ public class SiteExternalSystemRepository : IExternalSystemRepository
|
||||
return all.FirstOrDefault(d => d.Id == id);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// ExternalSystemGateway-011: genuine name-keyed lookup. The
|
||||
/// <c>database_connections</c> table has <c>name</c> as its PRIMARY KEY, so this
|
||||
/// is a single indexed-row fetch rather than fetch-all-then-filter.
|
||||
/// </summary>
|
||||
public async Task<DatabaseConnectionDefinition?> GetDatabaseConnectionByNameAsync(
|
||||
string name, CancellationToken cancellationToken = default)
|
||||
{
|
||||
await using var connection = CreateConnection();
|
||||
await connection.OpenAsync(cancellationToken);
|
||||
|
||||
await using var command = connection.CreateCommand();
|
||||
command.CommandText = @"
|
||||
SELECT name, connection_string, max_retries, retry_delay_ms
|
||||
FROM database_connections
|
||||
WHERE name = @name";
|
||||
command.Parameters.AddWithValue("@name", name);
|
||||
|
||||
await using var reader = await command.ExecuteReaderAsync(cancellationToken);
|
||||
if (!await reader.ReadAsync(cancellationToken))
|
||||
return null;
|
||||
|
||||
return new DatabaseConnectionDefinition(
|
||||
name: reader.GetString(0),
|
||||
connectionString: reader.GetString(1))
|
||||
{
|
||||
Id = GenerateSyntheticId(reader.GetString(0)),
|
||||
MaxRetries = reader.GetInt32(2),
|
||||
RetryDelay = TimeSpan.FromMilliseconds(reader.GetInt64(3))
|
||||
};
|
||||
}
|
||||
|
||||
// ── Write operations (not supported on site) ──
|
||||
|
||||
public Task AddExternalSystemAsync(ExternalSystemDefinition definition, CancellationToken cancellationToken = default)
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
using ScadaLink.Commons.Entities.Instances;
|
||||
using ScadaLink.Commons.Interfaces.Repositories;
|
||||
using ScadaLink.Commons.Interfaces.Services;
|
||||
using ScadaLink.Commons.Messages.Management;
|
||||
using ScadaLink.Commons.Types;
|
||||
using ScadaLink.Commons.Types.Enums;
|
||||
using ScadaLink.TemplateEngine;
|
||||
@@ -276,7 +277,7 @@ public class InstanceService
|
||||
/// </summary>
|
||||
public async Task<Result<IReadOnlyList<InstanceConnectionBinding>>> SetConnectionBindingsAsync(
|
||||
int instanceId,
|
||||
IReadOnlyList<(string AttributeName, int DataConnectionId)> bindings,
|
||||
IReadOnlyList<ConnectionBinding> bindings,
|
||||
string user,
|
||||
CancellationToken cancellationToken = default)
|
||||
{
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
using ScadaLink.CLI.Commands;
|
||||
using ScadaLink.Commons.Messages.Management;
|
||||
|
||||
namespace ScadaLink.CLI.Tests;
|
||||
|
||||
@@ -17,8 +18,8 @@ public class InstanceArgumentParsingTests
|
||||
Assert.True(ok);
|
||||
Assert.Null(error);
|
||||
Assert.Equal(2, bindings!.Count);
|
||||
Assert.Equal(("Speed", 5), bindings[0]);
|
||||
Assert.Equal(("Mode", 7), bindings[1]);
|
||||
Assert.Equal(new ConnectionBinding("Speed", 5), bindings[0]);
|
||||
Assert.Equal(new ConnectionBinding("Mode", 7), bindings[1]);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
|
||||
@@ -0,0 +1,46 @@
|
||||
using Microsoft.AspNetCore.Authentication;
|
||||
using ScadaLink.CentralUI.Auth;
|
||||
|
||||
namespace ScadaLink.CentralUI.Tests.Auth;
|
||||
|
||||
/// <summary>
|
||||
/// Regression tests for CentralUI-005. <c>AuthEndpoints</c> previously stamped a
|
||||
/// fixed <c>expires_at = UtcNow + 30 min</c> claim and a 30-minute absolute cookie
|
||||
/// <c>ExpiresUtc</c> with no sliding refresh, contradicting the documented
|
||||
/// "sliding refresh, 30-minute idle timeout" policy. The login handler must now
|
||||
/// build <see cref="AuthenticationProperties"/> that let the cookie middleware
|
||||
/// own expiry (sliding window) rather than imposing a contradictory fixed
|
||||
/// absolute cap.
|
||||
/// </summary>
|
||||
public class SessionExpiryPolicyTests
|
||||
{
|
||||
[Fact]
|
||||
public void BuildSignInProperties_DoesNotSetFixedAbsoluteExpiry()
|
||||
{
|
||||
var props = AuthEndpoints.BuildSignInProperties();
|
||||
|
||||
// A fixed ExpiresUtc would re-introduce the hard 30-minute cap that
|
||||
// overrides the middleware's sliding window. Expiry must be owned by
|
||||
// the cookie middleware (ExpireTimeSpan + SlidingExpiration).
|
||||
Assert.Null(props.ExpiresUtc);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void BuildSignInProperties_IsPersistent()
|
||||
{
|
||||
var props = AuthEndpoints.BuildSignInProperties();
|
||||
|
||||
Assert.True(props.IsPersistent);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void BuildSignInProperties_AllowsSlidingRefresh()
|
||||
{
|
||||
var props = AuthEndpoints.BuildSignInProperties();
|
||||
|
||||
// AllowRefresh left null/true lets the cookie middleware slide the
|
||||
// expiry on activity. A false value would freeze the session to an
|
||||
// absolute cap — the bug this finding pins.
|
||||
Assert.NotEqual(false, props.AllowRefresh);
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,112 @@
|
||||
using System.Reflection;
|
||||
using System.Security.Claims;
|
||||
using Bunit;
|
||||
using Microsoft.AspNetCore.Components.Authorization;
|
||||
using Microsoft.Extensions.DependencyInjection;
|
||||
using Microsoft.Extensions.Logging.Abstractions;
|
||||
using NSubstitute;
|
||||
using ScadaLink.CentralUI.Auth;
|
||||
using ScadaLink.Commons.Entities.Deployment;
|
||||
using ScadaLink.Commons.Entities.Instances;
|
||||
using ScadaLink.Commons.Interfaces.Repositories;
|
||||
using ScadaLink.Commons.Types.Enums;
|
||||
using ScadaLink.DeploymentManager;
|
||||
using DeploymentsPage = ScadaLink.CentralUI.Components.Pages.Deployment.Deployments;
|
||||
|
||||
namespace ScadaLink.CentralUI.Tests.Deployment;
|
||||
|
||||
/// <summary>
|
||||
/// Regression tests for CentralUI-006. Component-CentralUI "Real-Time Updates"
|
||||
/// states deployment status transitions push to the UI immediately via SignalR
|
||||
/// with no polling. The page previously ran a 10-second <c>Timer</c> that
|
||||
/// reloaded every deployment record + instance map per tick. The fix removes
|
||||
/// the timer and subscribes to <see cref="IDeploymentStatusNotifier"/>, which
|
||||
/// <c>DeploymentService</c> raises on every deployment-record status write;
|
||||
/// Blazor Server then pushes the re-render over its SignalR circuit.
|
||||
/// </summary>
|
||||
public class DeploymentsPushUpdateTests : BunitContext
|
||||
{
|
||||
private IDeploymentManagerRepository _deployRepo = null!;
|
||||
private ITemplateEngineRepository _templateRepo = null!;
|
||||
private DeploymentStatusNotifier _notifier = null!;
|
||||
|
||||
private void RegisterServices()
|
||||
{
|
||||
_deployRepo = Substitute.For<IDeploymentManagerRepository>();
|
||||
_templateRepo = Substitute.For<ITemplateEngineRepository>();
|
||||
_notifier = new DeploymentStatusNotifier(NullLogger<DeploymentStatusNotifier>.Instance);
|
||||
|
||||
_templateRepo.GetAllInstancesAsync(Arg.Any<CancellationToken>())
|
||||
.Returns(new List<Instance>
|
||||
{
|
||||
new("Inst-1") { Id = 1, SiteId = 1 }
|
||||
});
|
||||
_deployRepo.GetAllDeploymentRecordsAsync(Arg.Any<CancellationToken>())
|
||||
.Returns(new List<DeploymentRecord>());
|
||||
|
||||
Services.AddSingleton(_deployRepo);
|
||||
Services.AddSingleton(_templateRepo);
|
||||
Services.AddSingleton<IDeploymentStatusNotifier>(_notifier);
|
||||
|
||||
var identity = new ClaimsIdentity(
|
||||
new[] { new Claim(ClaimTypes.Name, "deployer") }, "TestCookie");
|
||||
var stubAuth = new StubAuthStateProvider(
|
||||
new AuthenticationState(new ClaimsPrincipal(identity)));
|
||||
Services.AddSingleton<AuthenticationStateProvider>(stubAuth);
|
||||
Services.AddScoped(_ => new SiteScopeService(stubAuth));
|
||||
}
|
||||
|
||||
private sealed class StubAuthStateProvider : AuthenticationStateProvider
|
||||
{
|
||||
private readonly AuthenticationState _state;
|
||||
public StubAuthStateProvider(AuthenticationState state) => _state = state;
|
||||
public override Task<AuthenticationState> GetAuthenticationStateAsync()
|
||||
=> Task.FromResult(_state);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Deployments_DoesNotPoll_HasNoRefreshTimer()
|
||||
{
|
||||
// The 10-second polling Timer must be gone — push replaces polling.
|
||||
var timerField = typeof(DeploymentsPage).GetField(
|
||||
"_refreshTimer", BindingFlags.Instance | BindingFlags.NonPublic);
|
||||
|
||||
Assert.Null(timerField);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Deployments_StatusChange_TriggersReload()
|
||||
{
|
||||
RegisterServices();
|
||||
var cut = Render<DeploymentsPage>();
|
||||
|
||||
// Initial load: instances + records each fetched once.
|
||||
_deployRepo.ClearReceivedCalls();
|
||||
_templateRepo.ClearReceivedCalls();
|
||||
|
||||
// A deployment status write in DeploymentManager raises the notifier;
|
||||
// the page must reload in response (no polling timer involved).
|
||||
_notifier.NotifyStatusChanged(
|
||||
new DeploymentStatusChange("dep-1", 1, DeploymentStatus.Success));
|
||||
|
||||
cut.WaitForAssertion(() =>
|
||||
_deployRepo.Received().GetAllDeploymentRecordsAsync(Arg.Any<CancellationToken>()));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Deployments_Dispose_UnsubscribesFromNotifier()
|
||||
{
|
||||
RegisterServices();
|
||||
var cut = Render<DeploymentsPage>();
|
||||
|
||||
cut.Instance.Dispose();
|
||||
_deployRepo.ClearReceivedCalls();
|
||||
|
||||
// After disposal, a status change must NOT touch the disposed component.
|
||||
_notifier.NotifyStatusChanged(
|
||||
new DeploymentStatusChange("dep-2", 1, DeploymentStatus.Failed));
|
||||
|
||||
_deployRepo.DidNotReceive()
|
||||
.GetAllDeploymentRecordsAsync(Arg.Any<CancellationToken>());
|
||||
}
|
||||
}
|
||||
@@ -57,6 +57,11 @@ public class TopologyPageTests : BunitContext
|
||||
// DeploymentService gained a DiffService dependency (DeploymentManager
|
||||
// contract change); register it so the page's DI graph resolves.
|
||||
Services.AddScoped<ScadaLink.TemplateEngine.Flattening.DiffService>();
|
||||
// CentralUI-006: DeploymentService now also depends on the
|
||||
// deployment-status notifier (a process singleton in production).
|
||||
Services.AddSingleton<ScadaLink.DeploymentManager.IDeploymentStatusNotifier>(
|
||||
new ScadaLink.DeploymentManager.DeploymentStatusNotifier(
|
||||
NullLogger<ScadaLink.DeploymentManager.DeploymentStatusNotifier>.Instance));
|
||||
Services.AddScoped<DeploymentService>();
|
||||
Services.AddScoped<AreaService>();
|
||||
Services.AddScoped<InstanceService>();
|
||||
|
||||
@@ -0,0 +1,54 @@
|
||||
using System.Text.Json;
|
||||
using ScadaLink.Commons.Messages.Management;
|
||||
|
||||
namespace ScadaLink.Commons.Tests.Messages;
|
||||
|
||||
/// <summary>
|
||||
/// Regression tests for Commons-008 — <see cref="SetConnectionBindingsCommand"/>
|
||||
/// previously declared its bindings as <c>IReadOnlyList<(string, int)></c>.
|
||||
/// A <c>ValueTuple</c> serializes as <c>Item1</c>/<c>Item2</c> and cannot evolve
|
||||
/// additively (REQ-COM-5a). It is now a named <see cref="ConnectionBinding"/> record.
|
||||
/// </summary>
|
||||
public class ConnectionBindingSerializationTests
|
||||
{
|
||||
[Fact]
|
||||
public void ConnectionBinding_SerializesWithNamedProperties()
|
||||
{
|
||||
var json = JsonSerializer.Serialize(new ConnectionBinding("Temperature", 42));
|
||||
|
||||
using var doc = JsonDocument.Parse(json);
|
||||
Assert.Equal(JsonValueKind.String, doc.RootElement.GetProperty("AttributeName").ValueKind);
|
||||
Assert.Equal("Temperature", doc.RootElement.GetProperty("AttributeName").GetString());
|
||||
Assert.Equal(42, doc.RootElement.GetProperty("DataConnectionId").GetInt32());
|
||||
|
||||
// The ValueTuple failure mode: Item1/Item2 must NOT appear.
|
||||
Assert.False(doc.RootElement.TryGetProperty("Item1", out _));
|
||||
Assert.False(doc.RootElement.TryGetProperty("Item2", out _));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void SetConnectionBindingsCommand_RoundTripsThroughJson()
|
||||
{
|
||||
var original = new SetConnectionBindingsCommand(
|
||||
7,
|
||||
new List<ConnectionBinding>
|
||||
{
|
||||
new("Speed", 5),
|
||||
new("Mode", 11),
|
||||
});
|
||||
|
||||
var json = JsonSerializer.Serialize(original);
|
||||
var deserialized = JsonSerializer.Deserialize<SetConnectionBindingsCommand>(json);
|
||||
|
||||
Assert.NotNull(deserialized);
|
||||
Assert.Equal(7, deserialized!.InstanceId);
|
||||
Assert.Equal(2, deserialized.Bindings.Count);
|
||||
Assert.Equal("Speed", deserialized.Bindings[0].AttributeName);
|
||||
Assert.Equal(5, deserialized.Bindings[0].DataConnectionId);
|
||||
Assert.Equal("Mode", deserialized.Bindings[1].AttributeName);
|
||||
Assert.Equal(11, deserialized.Bindings[1].DataConnectionId);
|
||||
|
||||
// ConnectionBinding is a record: each element compares by value.
|
||||
Assert.Equal(original.Bindings, deserialized.Bindings);
|
||||
}
|
||||
}
|
||||
@@ -76,6 +76,94 @@ public class ExternalSystemRepositoryTests : IDisposable
|
||||
{
|
||||
Assert.Throws<ArgumentNullException>(() => new ExternalSystemRepository(null!));
|
||||
}
|
||||
|
||||
// ── ExternalSystemGateway-011: name-keyed repository lookups ──
|
||||
|
||||
[Fact]
|
||||
public async Task GetExternalSystemByName_ReturnsMatchingRow()
|
||||
{
|
||||
await _repository.AddExternalSystemAsync(
|
||||
new ExternalSystemDefinition("Alpha", "https://alpha.test", "ApiKey"));
|
||||
await _repository.AddExternalSystemAsync(
|
||||
new ExternalSystemDefinition("Beta", "https://beta.test", "Basic"));
|
||||
await _repository.SaveChangesAsync();
|
||||
|
||||
var loaded = await _repository.GetExternalSystemByNameAsync("Beta");
|
||||
|
||||
Assert.NotNull(loaded);
|
||||
Assert.Equal("Beta", loaded!.Name);
|
||||
Assert.Equal("https://beta.test", loaded.EndpointUrl);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task GetExternalSystemByName_MissingName_ReturnsNull()
|
||||
{
|
||||
await _repository.AddExternalSystemAsync(
|
||||
new ExternalSystemDefinition("Alpha", "https://alpha.test", "ApiKey"));
|
||||
await _repository.SaveChangesAsync();
|
||||
|
||||
Assert.Null(await _repository.GetExternalSystemByNameAsync("DoesNotExist"));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task GetMethodByName_ReturnsMethodScopedToParentSystem()
|
||||
{
|
||||
var sysA = new ExternalSystemDefinition("SysA", "https://a.test", "ApiKey");
|
||||
var sysB = new ExternalSystemDefinition("SysB", "https://b.test", "ApiKey");
|
||||
await _repository.AddExternalSystemAsync(sysA);
|
||||
await _repository.AddExternalSystemAsync(sysB);
|
||||
await _repository.SaveChangesAsync();
|
||||
|
||||
// Same method name on two different systems — the lookup must be scoped.
|
||||
await _repository.AddExternalSystemMethodAsync(
|
||||
new ExternalSystemMethod("getData", "GET", "/a") { ExternalSystemDefinitionId = sysA.Id });
|
||||
await _repository.AddExternalSystemMethodAsync(
|
||||
new ExternalSystemMethod("getData", "POST", "/b") { ExternalSystemDefinitionId = sysB.Id });
|
||||
await _repository.SaveChangesAsync();
|
||||
|
||||
var method = await _repository.GetMethodByNameAsync(sysB.Id, "getData");
|
||||
|
||||
Assert.NotNull(method);
|
||||
Assert.Equal(sysB.Id, method!.ExternalSystemDefinitionId);
|
||||
Assert.Equal("POST", method.HttpMethod);
|
||||
Assert.Equal("/b", method.Path);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task GetMethodByName_MissingMethod_ReturnsNull()
|
||||
{
|
||||
var sys = new ExternalSystemDefinition("SysA", "https://a.test", "ApiKey");
|
||||
await _repository.AddExternalSystemAsync(sys);
|
||||
await _repository.SaveChangesAsync();
|
||||
|
||||
Assert.Null(await _repository.GetMethodByNameAsync(sys.Id, "noSuchMethod"));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task GetDatabaseConnectionByName_ReturnsMatchingRow()
|
||||
{
|
||||
await _repository.AddDatabaseConnectionAsync(
|
||||
new DatabaseConnectionDefinition("Plant", "Server=plant;Database=p;"));
|
||||
await _repository.AddDatabaseConnectionAsync(
|
||||
new DatabaseConnectionDefinition("Historian", "Server=hist;Database=h;"));
|
||||
await _repository.SaveChangesAsync();
|
||||
|
||||
var loaded = await _repository.GetDatabaseConnectionByNameAsync("Historian");
|
||||
|
||||
Assert.NotNull(loaded);
|
||||
Assert.Equal("Historian", loaded!.Name);
|
||||
Assert.Equal("Server=hist;Database=h;", loaded.ConnectionString);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task GetDatabaseConnectionByName_MissingName_ReturnsNull()
|
||||
{
|
||||
await _repository.AddDatabaseConnectionAsync(
|
||||
new DatabaseConnectionDefinition("Plant", "Server=plant;Database=p;"));
|
||||
await _repository.SaveChangesAsync();
|
||||
|
||||
Assert.Null(await _repository.GetDatabaseConnectionByNameAsync("DoesNotExist"));
|
||||
}
|
||||
}
|
||||
|
||||
public class NotificationRepositoryTests : IDisposable
|
||||
|
||||
@@ -47,7 +47,9 @@ public class DeploymentServiceTests : TestKit
|
||||
var siteRepo = Substitute.For<ISiteRepository>();
|
||||
_service = new DeploymentService(
|
||||
_repo, siteRepo, _pipeline, _comms, _lockManager, _audit,
|
||||
new DiffService(), options,
|
||||
new DiffService(),
|
||||
new DeploymentStatusNotifier(NullLogger<DeploymentStatusNotifier>.Instance),
|
||||
options,
|
||||
NullLogger<DeploymentService>.Instance);
|
||||
}
|
||||
|
||||
@@ -577,6 +579,7 @@ public class DeploymentServiceTests : TestKit
|
||||
return new DeploymentService(
|
||||
_repo, siteRepo, _pipeline, comms, _lockManager, _audit,
|
||||
new DiffService(),
|
||||
new DeploymentStatusNotifier(NullLogger<DeploymentStatusNotifier>.Instance),
|
||||
Options.Create(new DeploymentManagerOptions { OperationLockTimeout = TimeSpan.FromSeconds(5) }),
|
||||
NullLogger<DeploymentService>.Instance);
|
||||
}
|
||||
@@ -792,6 +795,7 @@ public class DeploymentServiceTests : TestKit
|
||||
var service = new DeploymentService(
|
||||
_repo, siteRepo, _pipeline, comms, _lockManager, _audit,
|
||||
new DiffService(),
|
||||
new DeploymentStatusNotifier(NullLogger<DeploymentStatusNotifier>.Instance),
|
||||
Options.Create(new DeploymentManagerOptions
|
||||
{
|
||||
OperationLockTimeout = TimeSpan.FromSeconds(5),
|
||||
|
||||
@@ -0,0 +1,114 @@
|
||||
using Akka.TestKit.Xunit2;
|
||||
using Microsoft.Extensions.Logging.Abstractions;
|
||||
using Microsoft.Extensions.Options;
|
||||
using NSubstitute;
|
||||
using ScadaLink.Commons.Entities.Deployment;
|
||||
using ScadaLink.Commons.Entities.Instances;
|
||||
using ScadaLink.Commons.Interfaces.Repositories;
|
||||
using ScadaLink.Commons.Interfaces.Services;
|
||||
using ScadaLink.Commons.Types;
|
||||
using ScadaLink.Commons.Types.Enums;
|
||||
using ScadaLink.Commons.Types.Flattening;
|
||||
using ScadaLink.Communication;
|
||||
using ScadaLink.TemplateEngine.Flattening;
|
||||
|
||||
namespace ScadaLink.DeploymentManager.Tests;
|
||||
|
||||
/// <summary>
|
||||
/// CentralUI-006: regression tests proving <see cref="DeploymentService"/>
|
||||
/// raises <see cref="IDeploymentStatusNotifier.StatusChanged"/> whenever it
|
||||
/// writes a deployment record's status. This is the event source the Central
|
||||
/// UI deployment-status page subscribes to instead of polling.
|
||||
/// </summary>
|
||||
public class DeploymentStatusNotifierTests : TestKit
|
||||
{
|
||||
private readonly IDeploymentManagerRepository _repo;
|
||||
private readonly IFlatteningPipeline _pipeline;
|
||||
private readonly CommunicationService _comms;
|
||||
private readonly OperationLockManager _lockManager;
|
||||
private readonly IAuditService _audit;
|
||||
private readonly DeploymentStatusNotifier _notifier;
|
||||
private readonly DeploymentService _service;
|
||||
|
||||
public DeploymentStatusNotifierTests()
|
||||
{
|
||||
_repo = Substitute.For<IDeploymentManagerRepository>();
|
||||
_pipeline = Substitute.For<IFlatteningPipeline>();
|
||||
_comms = new CommunicationService(
|
||||
Options.Create(new CommunicationOptions()),
|
||||
NullLogger<CommunicationService>.Instance);
|
||||
_lockManager = new OperationLockManager();
|
||||
_audit = Substitute.For<IAuditService>();
|
||||
_notifier = new DeploymentStatusNotifier(NullLogger<DeploymentStatusNotifier>.Instance);
|
||||
|
||||
var options = Options.Create(new DeploymentManagerOptions
|
||||
{
|
||||
OperationLockTimeout = TimeSpan.FromSeconds(5)
|
||||
});
|
||||
|
||||
var siteRepo = Substitute.For<ISiteRepository>();
|
||||
_service = new DeploymentService(
|
||||
_repo, siteRepo, _pipeline, _comms, _lockManager, _audit,
|
||||
new DiffService(), _notifier, options,
|
||||
NullLogger<DeploymentService>.Instance);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task DeployInstanceAsync_RaisesStatusChange_ForEveryRecordStatusWrite()
|
||||
{
|
||||
var instance = new Instance("TestInst") { Id = 7, SiteId = 1, State = InstanceState.NotDeployed };
|
||||
_repo.GetInstanceByIdAsync(7).Returns(instance);
|
||||
|
||||
var config = new FlattenedConfiguration { InstanceUniqueName = "TestInst" };
|
||||
_pipeline.FlattenAndValidateAsync(7, Arg.Any<CancellationToken>())
|
||||
.Returns(Result<FlatteningPipelineResult>.Success(
|
||||
new FlatteningPipelineResult(config, "sha256:abc", ValidationResult.Success())));
|
||||
|
||||
var changes = new List<DeploymentStatusChange>();
|
||||
_notifier.StatusChanged += c => changes.Add(c);
|
||||
|
||||
// _comms has no actor set, so the deploy reaches the catch block and
|
||||
// the record ends Failed. The notifier must fire for the Pending,
|
||||
// InProgress and Failed writes — not be silent (the pre-fix behaviour).
|
||||
var result = await _service.DeployInstanceAsync(7, "admin");
|
||||
|
||||
Assert.True(result.IsFailure);
|
||||
Assert.NotEmpty(changes);
|
||||
Assert.All(changes, c => Assert.Equal(7, c.InstanceId));
|
||||
Assert.Contains(changes, c => c.Status == DeploymentStatus.Pending);
|
||||
Assert.Contains(changes, c => c.Status == DeploymentStatus.InProgress);
|
||||
Assert.Contains(changes, c => c.Status == DeploymentStatus.Failed);
|
||||
|
||||
// All notifications carry the same deployment id (the one created here).
|
||||
var deploymentId = changes[0].DeploymentId;
|
||||
Assert.False(string.IsNullOrEmpty(deploymentId));
|
||||
Assert.All(changes, c => Assert.Equal(deploymentId, c.DeploymentId));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void NotifyStatusChanged_WithNoSubscribers_DoesNotThrow()
|
||||
{
|
||||
var notifier = new DeploymentStatusNotifier(NullLogger<DeploymentStatusNotifier>.Instance);
|
||||
|
||||
var ex = Record.Exception(() =>
|
||||
notifier.NotifyStatusChanged(new DeploymentStatusChange("dep-1", 1, DeploymentStatus.Success)));
|
||||
|
||||
Assert.Null(ex);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void NotifyStatusChanged_ThrowingSubscriber_DoesNotBreakOtherSubscribers()
|
||||
{
|
||||
var notifier = new DeploymentStatusNotifier(NullLogger<DeploymentStatusNotifier>.Instance);
|
||||
var reached = false;
|
||||
|
||||
notifier.StatusChanged += _ => throw new InvalidOperationException("boom");
|
||||
notifier.StatusChanged += _ => reached = true;
|
||||
|
||||
var ex = Record.Exception(() =>
|
||||
notifier.NotifyStatusChanged(new DeploymentStatusChange("dep-2", 2, DeploymentStatus.InProgress)));
|
||||
|
||||
Assert.Null(ex);
|
||||
Assert.True(reached, "a faulting subscriber must not stop later subscribers from being notified");
|
||||
}
|
||||
}
|
||||
@@ -56,4 +56,25 @@ public class ServiceCollectionExtensionsTests
|
||||
{
|
||||
Assert.Equal("ScadaLink:DeploymentManager", ServiceCollectionExtensions.OptionsSection);
|
||||
}
|
||||
|
||||
// CentralUI-006: the deployment-status notifier must be a singleton so the
|
||||
// scoped DeploymentService and the Central UI's scoped Blazor page share
|
||||
// one instance — without that, a push notification raised by the service
|
||||
// would never reach the page's subscription.
|
||||
[Fact]
|
||||
public void AddDeploymentManager_RegistersDeploymentStatusNotifier_AsSingleton()
|
||||
{
|
||||
var services = new ServiceCollection();
|
||||
services.AddLogging();
|
||||
services.AddDeploymentManager();
|
||||
|
||||
using var provider = services.BuildServiceProvider();
|
||||
|
||||
var fromRoot = provider.GetRequiredService<IDeploymentStatusNotifier>();
|
||||
using var scope = provider.CreateScope();
|
||||
var fromScope = scope.ServiceProvider.GetRequiredService<IDeploymentStatusNotifier>();
|
||||
|
||||
Assert.IsType<DeploymentStatusNotifier>(fromRoot);
|
||||
Assert.Same(fromRoot, fromScope);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -14,10 +14,25 @@ public class DatabaseGatewayTests
|
||||
{
|
||||
private readonly IExternalSystemRepository _repository = Substitute.For<IExternalSystemRepository>();
|
||||
|
||||
/// <summary>
|
||||
/// Configures the repository substitute for the name-keyed connection-resolution
|
||||
/// path used by <c>DatabaseGateway</c> (ExternalSystemGateway-011). A <c>null</c>
|
||||
/// connection models a "not found" — the substitute returns <c>null</c> by default,
|
||||
/// so no stub is needed for the absent entity.
|
||||
/// </summary>
|
||||
private void StubConnection(DatabaseConnectionDefinition? connection)
|
||||
{
|
||||
if (connection != null)
|
||||
{
|
||||
_repository.GetDatabaseConnectionByNameAsync(connection.Name, Arg.Any<CancellationToken>())
|
||||
.Returns(connection);
|
||||
}
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task GetConnection_NotFound_Throws()
|
||||
{
|
||||
_repository.GetAllDatabaseConnectionsAsync().Returns(new List<DatabaseConnectionDefinition>());
|
||||
StubConnection(connection: null);
|
||||
|
||||
var gateway = new DatabaseGateway(
|
||||
_repository,
|
||||
@@ -31,8 +46,7 @@ public class DatabaseGatewayTests
|
||||
public async Task CachedWrite_NoStoreAndForward_Throws()
|
||||
{
|
||||
var conn = new DatabaseConnectionDefinition("testDb", "Server=localhost;Database=test") { Id = 1 };
|
||||
_repository.GetAllDatabaseConnectionsAsync()
|
||||
.Returns(new List<DatabaseConnectionDefinition> { conn });
|
||||
StubConnection(conn);
|
||||
|
||||
var gateway = new DatabaseGateway(
|
||||
_repository,
|
||||
@@ -46,7 +60,7 @@ public class DatabaseGatewayTests
|
||||
[Fact]
|
||||
public async Task CachedWrite_ConnectionNotFound_Throws()
|
||||
{
|
||||
_repository.GetAllDatabaseConnectionsAsync().Returns(new List<DatabaseConnectionDefinition>());
|
||||
StubConnection(connection: null);
|
||||
|
||||
var gateway = new DatabaseGateway(
|
||||
_repository,
|
||||
@@ -67,8 +81,7 @@ public class DatabaseGatewayTests
|
||||
MaxRetries = 5,
|
||||
RetryDelay = TimeSpan.FromSeconds(12),
|
||||
};
|
||||
_repository.GetAllDatabaseConnectionsAsync(Arg.Any<CancellationToken>())
|
||||
.Returns(new List<DatabaseConnectionDefinition> { conn });
|
||||
StubConnection(conn);
|
||||
|
||||
var dbName = $"EsgCachedWrite_{Guid.NewGuid():N}";
|
||||
var connStr = $"Data Source={dbName};Mode=Memory;Cache=Shared";
|
||||
@@ -108,8 +121,7 @@ public class DatabaseGatewayTests
|
||||
MaxRetries = 0,
|
||||
RetryDelay = TimeSpan.FromSeconds(3),
|
||||
};
|
||||
_repository.GetAllDatabaseConnectionsAsync(Arg.Any<CancellationToken>())
|
||||
.Returns(new List<DatabaseConnectionDefinition> { conn });
|
||||
StubConnection(conn);
|
||||
|
||||
var dbName = $"EsgCachedWriteZero_{Guid.NewGuid():N}";
|
||||
var connStr = $"Data Source={dbName};Mode=Memory;Cache=Shared";
|
||||
@@ -153,7 +165,7 @@ public class DatabaseGatewayTests
|
||||
[Fact]
|
||||
public async Task DeliverBuffered_ConnectionNoLongerExists_ReturnsFalseSoMessageParks()
|
||||
{
|
||||
_repository.GetAllDatabaseConnectionsAsync().Returns(new List<DatabaseConnectionDefinition>());
|
||||
StubConnection(connection: null);
|
||||
var gateway = new DatabaseGateway(_repository, NullLogger<DatabaseGateway>.Instance);
|
||||
|
||||
var message = new ScadaLink.StoreAndForward.StoreAndForwardMessage
|
||||
@@ -176,7 +188,7 @@ public class DatabaseGatewayTests
|
||||
public async Task GetConnection_OpenFails_DisposesConnectionBeforeRethrowing()
|
||||
{
|
||||
var conn = new DatabaseConnectionDefinition("testDb", "Server=localhost;Database=test") { Id = 1 };
|
||||
_repository.GetAllDatabaseConnectionsAsync().Returns(new List<DatabaseConnectionDefinition> { conn });
|
||||
StubConnection(conn);
|
||||
|
||||
var fake = new ThrowingDbConnection();
|
||||
var gateway = new ConnectionFactoryStubGateway(_repository, fake);
|
||||
|
||||
@@ -18,10 +18,31 @@ public class ExternalSystemClientTests
|
||||
private readonly IExternalSystemRepository _repository = Substitute.For<IExternalSystemRepository>();
|
||||
private readonly IHttpClientFactory _httpClientFactory = Substitute.For<IHttpClientFactory>();
|
||||
|
||||
/// <summary>
|
||||
/// Configures the repository substitute for the name-keyed resolution path used by
|
||||
/// <c>ExternalSystemClient</c> (ExternalSystemGateway-011). A <c>null</c> system or
|
||||
/// method models a "not found" — the substitute returns <c>null</c> by default, so
|
||||
/// no stub is needed for the absent entity.
|
||||
/// </summary>
|
||||
private void StubResolution(ExternalSystemDefinition? system, ExternalSystemMethod? method)
|
||||
{
|
||||
if (system != null)
|
||||
{
|
||||
_repository.GetExternalSystemByNameAsync(system.Name, Arg.Any<CancellationToken>())
|
||||
.Returns(system);
|
||||
}
|
||||
|
||||
if (system != null && method != null)
|
||||
{
|
||||
_repository.GetMethodByNameAsync(system.Id, method.Name, Arg.Any<CancellationToken>())
|
||||
.Returns(method);
|
||||
}
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Call_SystemNotFound_ReturnsError()
|
||||
{
|
||||
_repository.GetAllExternalSystemsAsync().Returns(new List<ExternalSystemDefinition>());
|
||||
StubResolution(system: null, method: null);
|
||||
|
||||
var client = new ExternalSystemClient(
|
||||
_httpClientFactory, _repository,
|
||||
@@ -37,8 +58,7 @@ public class ExternalSystemClientTests
|
||||
public async Task Call_MethodNotFound_ReturnsError()
|
||||
{
|
||||
var system = new ExternalSystemDefinition("TestAPI", "https://api.example.com", "none") { Id = 1 };
|
||||
_repository.GetAllExternalSystemsAsync().Returns(new List<ExternalSystemDefinition> { system });
|
||||
_repository.GetMethodsByExternalSystemIdAsync(1).Returns(new List<ExternalSystemMethod>());
|
||||
StubResolution(system, method: null);
|
||||
|
||||
var client = new ExternalSystemClient(
|
||||
_httpClientFactory, _repository,
|
||||
@@ -56,8 +76,7 @@ public class ExternalSystemClientTests
|
||||
var system = new ExternalSystemDefinition("TestAPI", "https://api.example.com", "none") { Id = 1 };
|
||||
var method = new ExternalSystemMethod("getData", "GET", "/data") { Id = 1, ExternalSystemDefinitionId = 1 };
|
||||
|
||||
_repository.GetAllExternalSystemsAsync().Returns(new List<ExternalSystemDefinition> { system });
|
||||
_repository.GetMethodsByExternalSystemIdAsync(1).Returns(new List<ExternalSystemMethod> { method });
|
||||
StubResolution(system, method);
|
||||
|
||||
var handler = new MockHttpMessageHandler(HttpStatusCode.OK, "{\"result\": 42}");
|
||||
var httpClient = new HttpClient(handler);
|
||||
@@ -79,8 +98,7 @@ public class ExternalSystemClientTests
|
||||
var system = new ExternalSystemDefinition("TestAPI", "https://api.example.com", "none") { Id = 1 };
|
||||
var method = new ExternalSystemMethod("failMethod", "POST", "/fail") { Id = 1, ExternalSystemDefinitionId = 1 };
|
||||
|
||||
_repository.GetAllExternalSystemsAsync().Returns(new List<ExternalSystemDefinition> { system });
|
||||
_repository.GetMethodsByExternalSystemIdAsync(1).Returns(new List<ExternalSystemMethod> { method });
|
||||
StubResolution(system, method);
|
||||
|
||||
var handler = new MockHttpMessageHandler(HttpStatusCode.InternalServerError, "server error");
|
||||
var httpClient = new HttpClient(handler);
|
||||
@@ -102,8 +120,7 @@ public class ExternalSystemClientTests
|
||||
var system = new ExternalSystemDefinition("TestAPI", "https://api.example.com", "none") { Id = 1 };
|
||||
var method = new ExternalSystemMethod("badMethod", "POST", "/bad") { Id = 1, ExternalSystemDefinitionId = 1 };
|
||||
|
||||
_repository.GetAllExternalSystemsAsync().Returns(new List<ExternalSystemDefinition> { system });
|
||||
_repository.GetMethodsByExternalSystemIdAsync(1).Returns(new List<ExternalSystemMethod> { method });
|
||||
StubResolution(system, method);
|
||||
|
||||
var handler = new MockHttpMessageHandler(HttpStatusCode.BadRequest, "bad request");
|
||||
var httpClient = new HttpClient(handler);
|
||||
@@ -122,7 +139,7 @@ public class ExternalSystemClientTests
|
||||
[Fact]
|
||||
public async Task CachedCall_SystemNotFound_ReturnsError()
|
||||
{
|
||||
_repository.GetAllExternalSystemsAsync().Returns(new List<ExternalSystemDefinition>());
|
||||
StubResolution(system: null, method: null);
|
||||
|
||||
var client = new ExternalSystemClient(
|
||||
_httpClientFactory, _repository,
|
||||
@@ -140,8 +157,7 @@ public class ExternalSystemClientTests
|
||||
var system = new ExternalSystemDefinition("TestAPI", "https://api.example.com", "none") { Id = 1 };
|
||||
var method = new ExternalSystemMethod("getData", "GET", "/data") { Id = 1, ExternalSystemDefinitionId = 1 };
|
||||
|
||||
_repository.GetAllExternalSystemsAsync().Returns(new List<ExternalSystemDefinition> { system });
|
||||
_repository.GetMethodsByExternalSystemIdAsync(1).Returns(new List<ExternalSystemMethod> { method });
|
||||
StubResolution(system, method);
|
||||
|
||||
var handler = new MockHttpMessageHandler(HttpStatusCode.OK, "{\"ok\": true}");
|
||||
var httpClient = new HttpClient(handler);
|
||||
@@ -175,8 +191,7 @@ public class ExternalSystemClientTests
|
||||
{
|
||||
var system = new ExternalSystemDefinition("TestAPI", "https://api.example.com", "none") { Id = 1 };
|
||||
var method = new ExternalSystemMethod("getData", "GET", "/data") { Id = 1, ExternalSystemDefinitionId = 1 };
|
||||
_repository.GetAllExternalSystemsAsync().Returns(new List<ExternalSystemDefinition> { system });
|
||||
_repository.GetMethodsByExternalSystemIdAsync(1).Returns(new List<ExternalSystemMethod> { method });
|
||||
StubResolution(system, method);
|
||||
|
||||
var httpClient = new HttpClient(new MockHttpMessageHandler(HttpStatusCode.OK, "{\"ok\":true}"));
|
||||
_httpClientFactory.CreateClient(Arg.Any<string>()).Returns(httpClient);
|
||||
@@ -192,7 +207,7 @@ public class ExternalSystemClientTests
|
||||
[Fact]
|
||||
public async Task DeliverBuffered_SystemNoLongerExists_ReturnsFalseSoMessageParks()
|
||||
{
|
||||
_repository.GetAllExternalSystemsAsync().Returns(new List<ExternalSystemDefinition>());
|
||||
StubResolution(system: null, method: null);
|
||||
|
||||
var client = new ExternalSystemClient(
|
||||
_httpClientFactory, _repository, NullLogger<ExternalSystemClient>.Instance);
|
||||
@@ -207,8 +222,7 @@ public class ExternalSystemClientTests
|
||||
{
|
||||
var system = new ExternalSystemDefinition("TestAPI", "https://api.example.com", "none") { Id = 1 };
|
||||
var method = new ExternalSystemMethod("failMethod", "POST", "/fail") { Id = 1, ExternalSystemDefinitionId = 1 };
|
||||
_repository.GetAllExternalSystemsAsync().Returns(new List<ExternalSystemDefinition> { system });
|
||||
_repository.GetMethodsByExternalSystemIdAsync(1).Returns(new List<ExternalSystemMethod> { method });
|
||||
StubResolution(system, method);
|
||||
|
||||
var httpClient = new HttpClient(new MockHttpMessageHandler(HttpStatusCode.InternalServerError, "boom"));
|
||||
_httpClientFactory.CreateClient(Arg.Any<string>()).Returns(httpClient);
|
||||
@@ -227,10 +241,7 @@ public class ExternalSystemClientTests
|
||||
{
|
||||
var system = new ExternalSystemDefinition("TestAPI", "https://api.example.com", "none") { Id = 1 };
|
||||
var method = new ExternalSystemMethod("postData", "POST", "/post") { Id = 1, ExternalSystemDefinitionId = 1 };
|
||||
_repository.GetAllExternalSystemsAsync(Arg.Any<CancellationToken>())
|
||||
.Returns(new List<ExternalSystemDefinition> { system });
|
||||
_repository.GetMethodsByExternalSystemIdAsync(1, Arg.Any<CancellationToken>())
|
||||
.Returns(new List<ExternalSystemMethod> { method });
|
||||
StubResolution(system, method);
|
||||
|
||||
// The HTTP layer always fails transiently (500).
|
||||
var httpClient = new HttpClient(new MockHttpMessageHandler(HttpStatusCode.InternalServerError, "boom"));
|
||||
@@ -275,10 +286,7 @@ public class ExternalSystemClientTests
|
||||
{
|
||||
var system = new ExternalSystemDefinition("TestAPI", "https://api.example.com", "none") { Id = 1 };
|
||||
var method = new ExternalSystemMethod("getData", "GET", "/data") { Id = 1, ExternalSystemDefinitionId = 1 };
|
||||
_repository.GetAllExternalSystemsAsync(Arg.Any<CancellationToken>())
|
||||
.Returns(new List<ExternalSystemDefinition> { system });
|
||||
_repository.GetMethodsByExternalSystemIdAsync(1, Arg.Any<CancellationToken>())
|
||||
.Returns(new List<ExternalSystemMethod> { method });
|
||||
StubResolution(system, method);
|
||||
|
||||
// Handler that hangs far longer than the configured timeout and the test budget.
|
||||
var httpClient = new HttpClient(new HangingHttpMessageHandler(TimeSpan.FromMinutes(10)));
|
||||
@@ -308,10 +316,7 @@ public class ExternalSystemClientTests
|
||||
{
|
||||
var system = new ExternalSystemDefinition("TestAPI", "https://api.example.com", "none") { Id = 1 };
|
||||
var method = new ExternalSystemMethod("getData", "GET", "/data") { Id = 1, ExternalSystemDefinitionId = 1 };
|
||||
_repository.GetAllExternalSystemsAsync(Arg.Any<CancellationToken>())
|
||||
.Returns(new List<ExternalSystemDefinition> { system });
|
||||
_repository.GetMethodsByExternalSystemIdAsync(1, Arg.Any<CancellationToken>())
|
||||
.Returns(new List<ExternalSystemMethod> { method });
|
||||
StubResolution(system, method);
|
||||
|
||||
var httpClient = new HttpClient(new HangingHttpMessageHandler(TimeSpan.FromMinutes(10)));
|
||||
_httpClientFactory.CreateClient(Arg.Any<string>()).Returns(httpClient);
|
||||
@@ -342,10 +347,7 @@ public class ExternalSystemClientTests
|
||||
RetryDelay = TimeSpan.FromSeconds(42),
|
||||
};
|
||||
var method = new ExternalSystemMethod("postData", "POST", "/post") { Id = 1, ExternalSystemDefinitionId = 1 };
|
||||
_repository.GetAllExternalSystemsAsync(Arg.Any<CancellationToken>())
|
||||
.Returns(new List<ExternalSystemDefinition> { system });
|
||||
_repository.GetMethodsByExternalSystemIdAsync(1, Arg.Any<CancellationToken>())
|
||||
.Returns(new List<ExternalSystemMethod> { method });
|
||||
StubResolution(system, method);
|
||||
|
||||
var httpClient = new HttpClient(new MockHttpMessageHandler(HttpStatusCode.InternalServerError, "boom"));
|
||||
_httpClientFactory.CreateClient(Arg.Any<string>()).Returns(httpClient);
|
||||
@@ -404,10 +406,7 @@ public class ExternalSystemClientTests
|
||||
RetryDelay = TimeSpan.FromSeconds(5),
|
||||
};
|
||||
var method = new ExternalSystemMethod("postData", "POST", "/post") { Id = 1, ExternalSystemDefinitionId = 1 };
|
||||
_repository.GetAllExternalSystemsAsync(Arg.Any<CancellationToken>())
|
||||
.Returns(new List<ExternalSystemDefinition> { system });
|
||||
_repository.GetMethodsByExternalSystemIdAsync(1, Arg.Any<CancellationToken>())
|
||||
.Returns(new List<ExternalSystemMethod> { method });
|
||||
StubResolution(system, method);
|
||||
|
||||
var httpClient = new HttpClient(new MockHttpMessageHandler(HttpStatusCode.InternalServerError, "boom"));
|
||||
_httpClientFactory.CreateClient(Arg.Any<string>()).Returns(httpClient);
|
||||
@@ -443,10 +442,7 @@ public class ExternalSystemClientTests
|
||||
{
|
||||
var system = new ExternalSystemDefinition("TestAPI", "https://api.example.com", "none") { Id = 1 };
|
||||
var method = new ExternalSystemMethod("getData", "GET", "/data") { Id = 1, ExternalSystemDefinitionId = 1 };
|
||||
_repository.GetAllExternalSystemsAsync(Arg.Any<CancellationToken>())
|
||||
.Returns(new List<ExternalSystemDefinition> { system });
|
||||
_repository.GetMethodsByExternalSystemIdAsync(1, Arg.Any<CancellationToken>())
|
||||
.Returns(new List<ExternalSystemMethod> { method });
|
||||
StubResolution(system, method);
|
||||
|
||||
var handler = new DisposalTrackingHandler(HttpStatusCode.OK, "{\"ok\":true}");
|
||||
var httpClient = new HttpClient(handler);
|
||||
@@ -466,10 +462,7 @@ public class ExternalSystemClientTests
|
||||
{
|
||||
var system = new ExternalSystemDefinition("TestAPI", "https://api.example.com", "none") { Id = 1 };
|
||||
var method = new ExternalSystemMethod("badMethod", "POST", "/bad") { Id = 1, ExternalSystemDefinitionId = 1 };
|
||||
_repository.GetAllExternalSystemsAsync(Arg.Any<CancellationToken>())
|
||||
.Returns(new List<ExternalSystemDefinition> { system });
|
||||
_repository.GetMethodsByExternalSystemIdAsync(1, Arg.Any<CancellationToken>())
|
||||
.Returns(new List<ExternalSystemMethod> { method });
|
||||
StubResolution(system, method);
|
||||
|
||||
var handler = new DisposalTrackingHandler(HttpStatusCode.BadRequest, "bad request");
|
||||
var httpClient = new HttpClient(handler);
|
||||
@@ -491,10 +484,7 @@ public class ExternalSystemClientTests
|
||||
{
|
||||
var system = new ExternalSystemDefinition("TestAPI", "https://api.example.com/api", "none") { Id = 1 };
|
||||
var method = new ExternalSystemMethod("root", "GET", "") { Id = 1, ExternalSystemDefinitionId = 1 };
|
||||
_repository.GetAllExternalSystemsAsync(Arg.Any<CancellationToken>())
|
||||
.Returns(new List<ExternalSystemDefinition> { system });
|
||||
_repository.GetMethodsByExternalSystemIdAsync(1, Arg.Any<CancellationToken>())
|
||||
.Returns(new List<ExternalSystemMethod> { method });
|
||||
StubResolution(system, method);
|
||||
|
||||
var handler = new RequestCapturingHandler(HttpStatusCode.OK, "{}");
|
||||
var httpClient = new HttpClient(handler);
|
||||
@@ -513,10 +503,7 @@ public class ExternalSystemClientTests
|
||||
{
|
||||
var system = new ExternalSystemDefinition("TestAPI", "https://api.example.com/api", "none") { Id = 1 };
|
||||
var method = new ExternalSystemMethod("getData", "GET", "/data") { Id = 1, ExternalSystemDefinitionId = 1 };
|
||||
_repository.GetAllExternalSystemsAsync(Arg.Any<CancellationToken>())
|
||||
.Returns(new List<ExternalSystemDefinition> { system });
|
||||
_repository.GetMethodsByExternalSystemIdAsync(1, Arg.Any<CancellationToken>())
|
||||
.Returns(new List<ExternalSystemMethod> { method });
|
||||
StubResolution(system, method);
|
||||
|
||||
var handler = new RequestCapturingHandler(HttpStatusCode.OK, "{}");
|
||||
var httpClient = new HttpClient(handler);
|
||||
@@ -537,10 +524,7 @@ public class ExternalSystemClientTests
|
||||
{
|
||||
var system = new ExternalSystemDefinition("TestAPI", "https://api.example.com", "none") { Id = 1 };
|
||||
var method = new ExternalSystemMethod("badMethod", "POST", "/bad") { Id = 1, ExternalSystemDefinitionId = 1 };
|
||||
_repository.GetAllExternalSystemsAsync(Arg.Any<CancellationToken>())
|
||||
.Returns(new List<ExternalSystemDefinition> { system });
|
||||
_repository.GetMethodsByExternalSystemIdAsync(1, Arg.Any<CancellationToken>())
|
||||
.Returns(new List<ExternalSystemMethod> { method });
|
||||
StubResolution(system, method);
|
||||
|
||||
var hugeBody = new string('X', 500_000);
|
||||
var handler = new MockHttpMessageHandler(HttpStatusCode.BadRequest, hugeBody);
|
||||
@@ -566,10 +550,7 @@ public class ExternalSystemClientTests
|
||||
{
|
||||
var system = new ExternalSystemDefinition("TestAPI", "https://api.example.com", "none") { Id = 1 };
|
||||
var method = new ExternalSystemMethod("postData", "POST", "/post") { Id = 1, ExternalSystemDefinitionId = 1 };
|
||||
_repository.GetAllExternalSystemsAsync(Arg.Any<CancellationToken>())
|
||||
.Returns(new List<ExternalSystemDefinition> { system });
|
||||
_repository.GetMethodsByExternalSystemIdAsync(1, Arg.Any<CancellationToken>())
|
||||
.Returns(new List<ExternalSystemMethod> { method });
|
||||
StubResolution(system, method);
|
||||
|
||||
var httpClient = new HttpClient(new HangingHttpMessageHandler(TimeSpan.FromMinutes(10)));
|
||||
_httpClientFactory.CreateClient(Arg.Any<string>()).Returns(httpClient);
|
||||
@@ -612,10 +593,7 @@ public class ExternalSystemClientTests
|
||||
{
|
||||
var system = new ExternalSystemDefinition("TestAPI", "https://api.example.com", "none") { Id = 1 };
|
||||
var method = new ExternalSystemMethod("search", "GET", "/search") { Id = 1, ExternalSystemDefinitionId = 1 };
|
||||
_repository.GetAllExternalSystemsAsync(Arg.Any<CancellationToken>())
|
||||
.Returns(new List<ExternalSystemDefinition> { system });
|
||||
_repository.GetMethodsByExternalSystemIdAsync(1, Arg.Any<CancellationToken>())
|
||||
.Returns(new List<ExternalSystemMethod> { method });
|
||||
StubResolution(system, method);
|
||||
|
||||
var handler = new RequestCapturingHandler(HttpStatusCode.OK, "{}");
|
||||
_httpClientFactory.CreateClient(Arg.Any<string>()).Returns(new HttpClient(handler));
|
||||
@@ -642,10 +620,7 @@ public class ExternalSystemClientTests
|
||||
{
|
||||
var system = new ExternalSystemDefinition("TestAPI", "https://api.example.com", "none") { Id = 1 };
|
||||
var method = new ExternalSystemMethod("create", "POST", "/create") { Id = 1, ExternalSystemDefinitionId = 1 };
|
||||
_repository.GetAllExternalSystemsAsync(Arg.Any<CancellationToken>())
|
||||
.Returns(new List<ExternalSystemDefinition> { system });
|
||||
_repository.GetMethodsByExternalSystemIdAsync(1, Arg.Any<CancellationToken>())
|
||||
.Returns(new List<ExternalSystemMethod> { method });
|
||||
StubResolution(system, method);
|
||||
|
||||
var handler = new RequestCapturingHandler(HttpStatusCode.OK, "{}");
|
||||
_httpClientFactory.CreateClient(Arg.Any<string>()).Returns(new HttpClient(handler));
|
||||
@@ -668,10 +643,7 @@ public class ExternalSystemClientTests
|
||||
AuthConfiguration = "secret-key-123",
|
||||
};
|
||||
var method = new ExternalSystemMethod("getData", "GET", "/data") { Id = 1, ExternalSystemDefinitionId = 1 };
|
||||
_repository.GetAllExternalSystemsAsync(Arg.Any<CancellationToken>())
|
||||
.Returns(new List<ExternalSystemDefinition> { system });
|
||||
_repository.GetMethodsByExternalSystemIdAsync(1, Arg.Any<CancellationToken>())
|
||||
.Returns(new List<ExternalSystemMethod> { method });
|
||||
StubResolution(system, method);
|
||||
|
||||
var handler = new RequestCapturingHandler(HttpStatusCode.OK, "{}");
|
||||
_httpClientFactory.CreateClient(Arg.Any<string>()).Returns(new HttpClient(handler));
|
||||
@@ -694,10 +666,7 @@ public class ExternalSystemClientTests
|
||||
AuthConfiguration = "Authorization-Token:abc",
|
||||
};
|
||||
var method = new ExternalSystemMethod("getData", "GET", "/data") { Id = 1, ExternalSystemDefinitionId = 1 };
|
||||
_repository.GetAllExternalSystemsAsync(Arg.Any<CancellationToken>())
|
||||
.Returns(new List<ExternalSystemDefinition> { system });
|
||||
_repository.GetMethodsByExternalSystemIdAsync(1, Arg.Any<CancellationToken>())
|
||||
.Returns(new List<ExternalSystemMethod> { method });
|
||||
StubResolution(system, method);
|
||||
|
||||
var handler = new RequestCapturingHandler(HttpStatusCode.OK, "{}");
|
||||
_httpClientFactory.CreateClient(Arg.Any<string>()).Returns(new HttpClient(handler));
|
||||
@@ -720,10 +689,7 @@ public class ExternalSystemClientTests
|
||||
AuthConfiguration = "alice:s3cret",
|
||||
};
|
||||
var method = new ExternalSystemMethod("getData", "GET", "/data") { Id = 1, ExternalSystemDefinitionId = 1 };
|
||||
_repository.GetAllExternalSystemsAsync(Arg.Any<CancellationToken>())
|
||||
.Returns(new List<ExternalSystemDefinition> { system });
|
||||
_repository.GetMethodsByExternalSystemIdAsync(1, Arg.Any<CancellationToken>())
|
||||
.Returns(new List<ExternalSystemMethod> { method });
|
||||
StubResolution(system, method);
|
||||
|
||||
var handler = new RequestCapturingHandler(HttpStatusCode.OK, "{}");
|
||||
_httpClientFactory.CreateClient(Arg.Any<string>()).Returns(new HttpClient(handler));
|
||||
@@ -745,10 +711,7 @@ public class ExternalSystemClientTests
|
||||
{
|
||||
var system = new ExternalSystemDefinition("TestAPI", "https://api.example.com", "none") { Id = 1 };
|
||||
var method = new ExternalSystemMethod("getData", "GET", "/data") { Id = 1, ExternalSystemDefinitionId = 1 };
|
||||
_repository.GetAllExternalSystemsAsync(Arg.Any<CancellationToken>())
|
||||
.Returns(new List<ExternalSystemDefinition> { system });
|
||||
_repository.GetMethodsByExternalSystemIdAsync(1, Arg.Any<CancellationToken>())
|
||||
.Returns(new List<ExternalSystemMethod> { method });
|
||||
StubResolution(system, method);
|
||||
|
||||
// A connection-level failure (e.g. host unreachable) surfaces as HttpRequestException.
|
||||
var handler = new ThrowingHttpMessageHandler(new HttpRequestException("connection refused"));
|
||||
@@ -770,10 +733,7 @@ public class ExternalSystemClientTests
|
||||
{
|
||||
var system = new ExternalSystemDefinition("TestAPI", "https://api.example.com", "none") { Id = 1 };
|
||||
var method = new ExternalSystemMethod("badMethod", "POST", "/bad") { Id = 1, ExternalSystemDefinitionId = 1 };
|
||||
_repository.GetAllExternalSystemsAsync(Arg.Any<CancellationToken>())
|
||||
.Returns(new List<ExternalSystemDefinition> { system });
|
||||
_repository.GetMethodsByExternalSystemIdAsync(1, Arg.Any<CancellationToken>())
|
||||
.Returns(new List<ExternalSystemMethod> { method });
|
||||
StubResolution(system, method);
|
||||
|
||||
var handler = new MockHttpMessageHandler(HttpStatusCode.BadRequest, "bad request");
|
||||
var httpClient = new HttpClient(handler);
|
||||
@@ -795,10 +755,7 @@ public class ExternalSystemClientTests
|
||||
{
|
||||
var system = new ExternalSystemDefinition("TestAPI", "https://api.example.com", "none") { Id = 1 };
|
||||
var method = new ExternalSystemMethod("failMethod", "POST", "/fail") { Id = 1, ExternalSystemDefinitionId = 1 };
|
||||
_repository.GetAllExternalSystemsAsync(Arg.Any<CancellationToken>())
|
||||
.Returns(new List<ExternalSystemDefinition> { system });
|
||||
_repository.GetMethodsByExternalSystemIdAsync(1, Arg.Any<CancellationToken>())
|
||||
.Returns(new List<ExternalSystemMethod> { method });
|
||||
StubResolution(system, method);
|
||||
|
||||
var handler = new MockHttpMessageHandler(HttpStatusCode.InternalServerError, "boom");
|
||||
var httpClient = new HttpClient(handler);
|
||||
|
||||
@@ -423,6 +423,63 @@ public class SecurityReviewRegressionTests
|
||||
Assert.True(cookieOptions.Cookie.HttpOnly);
|
||||
}
|
||||
|
||||
// --- CentralUI-005: cookie auth must use a sliding session window ---
|
||||
// Documented policy (CLAUDE.md Security & Auth): sliding refresh with a
|
||||
// 30-minute idle timeout. The cookie middleware must enable SlidingExpiration
|
||||
// so an active session is renewed on activity and an idle session expires.
|
||||
|
||||
[Fact]
|
||||
public void AddSecurity_AuthCookie_UsesSlidingExpiration()
|
||||
{
|
||||
var services = new ServiceCollection();
|
||||
services.AddLogging();
|
||||
services.AddDataProtection();
|
||||
services.AddSecurity();
|
||||
|
||||
using var provider = services.BuildServiceProvider();
|
||||
var cookieOptions = provider
|
||||
.GetRequiredService<IOptionsMonitor<Microsoft.AspNetCore.Authentication.Cookies.CookieAuthenticationOptions>>()
|
||||
.Get(Microsoft.AspNetCore.Authentication.Cookies.CookieAuthenticationDefaults.AuthenticationScheme);
|
||||
|
||||
Assert.True(cookieOptions.SlidingExpiration);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void AddSecurity_AuthCookie_ExpireTimeSpanMatchesIdleTimeout()
|
||||
{
|
||||
var services = new ServiceCollection();
|
||||
services.AddLogging();
|
||||
services.AddDataProtection();
|
||||
services.AddSecurity();
|
||||
// The idle timeout drives the cookie's expiry window.
|
||||
services.Configure<SecurityOptions>(o => o.IdleTimeoutMinutes = 30);
|
||||
|
||||
using var provider = services.BuildServiceProvider();
|
||||
var cookieOptions = provider
|
||||
.GetRequiredService<IOptionsMonitor<Microsoft.AspNetCore.Authentication.Cookies.CookieAuthenticationOptions>>()
|
||||
.Get(Microsoft.AspNetCore.Authentication.Cookies.CookieAuthenticationDefaults.AuthenticationScheme);
|
||||
|
||||
Assert.Equal(TimeSpan.FromMinutes(30), cookieOptions.ExpireTimeSpan);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void AddSecurity_AuthCookie_ExpireTimeSpanIsConfigurable()
|
||||
{
|
||||
var services = new ServiceCollection();
|
||||
services.AddLogging();
|
||||
services.AddDataProtection();
|
||||
services.AddSecurity();
|
||||
services.Configure<SecurityOptions>(o => o.IdleTimeoutMinutes = 45);
|
||||
|
||||
using var provider = services.BuildServiceProvider();
|
||||
var cookieOptions = provider
|
||||
.GetRequiredService<IOptionsMonitor<Microsoft.AspNetCore.Authentication.Cookies.CookieAuthenticationOptions>>()
|
||||
.Get(Microsoft.AspNetCore.Authentication.Cookies.CookieAuthenticationDefaults.AuthenticationScheme);
|
||||
|
||||
Assert.Equal(TimeSpan.FromMinutes(45), cookieOptions.ExpireTimeSpan);
|
||||
Assert.True(cookieOptions.SlidingExpiration);
|
||||
}
|
||||
|
||||
// --- Security-001: StartTLS transport must be reachable ---
|
||||
|
||||
[Fact]
|
||||
|
||||
@@ -84,6 +84,101 @@ public class SiteRepositoryTests : IDisposable
|
||||
Assert.Equal("StableSystem", found.Name);
|
||||
}
|
||||
|
||||
// ── ExternalSystemGateway-011: name-keyed repository lookups ──
|
||||
|
||||
/// <summary>
|
||||
/// ExternalSystemGateway-011: the site repository's name-keyed external-system
|
||||
/// lookup returns the matching row, and the same synthetic ID as the by-ID path.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public async Task ExternalSystemRepository_GetByName_ReturnsMatchingDefinition()
|
||||
{
|
||||
var storage = NewStorage();
|
||||
await storage.InitializeAsync();
|
||||
await storage.StoreExternalSystemAsync(
|
||||
"Alpha", "https://alpha.test", "ApiKey", "{\"key\":\"x\"}", null);
|
||||
await storage.StoreExternalSystemAsync(
|
||||
"Beta", "https://beta.test", "Basic", null, null);
|
||||
|
||||
var repo = new SiteExternalSystemRepository(storage);
|
||||
|
||||
var found = await repo.GetExternalSystemByNameAsync("Beta");
|
||||
Assert.NotNull(found);
|
||||
Assert.Equal("Beta", found!.Name);
|
||||
Assert.Equal("https://beta.test", found.EndpointUrl);
|
||||
|
||||
// The by-name path must produce the same synthetic ID as the by-id path.
|
||||
var byId = await repo.GetExternalSystemByIdAsync(found.Id);
|
||||
Assert.NotNull(byId);
|
||||
Assert.Equal("Beta", byId!.Name);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// ExternalSystemGateway-011: a missing name resolves to <c>null</c>.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public async Task ExternalSystemRepository_GetByName_MissingName_ReturnsNull()
|
||||
{
|
||||
var storage = NewStorage();
|
||||
await storage.InitializeAsync();
|
||||
await storage.StoreExternalSystemAsync(
|
||||
"Alpha", "https://alpha.test", "ApiKey", null, null);
|
||||
|
||||
var repo = new SiteExternalSystemRepository(storage);
|
||||
|
||||
Assert.Null(await repo.GetExternalSystemByNameAsync("DoesNotExist"));
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// ExternalSystemGateway-011: the site repository's name-keyed method lookup
|
||||
/// returns the method scoped to its parent system, or <c>null</c> for a miss.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public async Task ExternalSystemRepository_GetMethodByName_ResolvesScopedToSystem()
|
||||
{
|
||||
var storage = NewStorage();
|
||||
await storage.InitializeAsync();
|
||||
var methodDefs = "[{\"Name\":\"getData\",\"HttpMethod\":\"GET\",\"Path\":\"/data\"}]";
|
||||
await storage.StoreExternalSystemAsync(
|
||||
"WeatherApi", "https://api.example.com", "ApiKey", null, methodDefs);
|
||||
|
||||
var repo = new SiteExternalSystemRepository(storage);
|
||||
var system = await repo.GetExternalSystemByNameAsync("WeatherApi");
|
||||
Assert.NotNull(system);
|
||||
|
||||
var method = await repo.GetMethodByNameAsync(system!.Id, "getData");
|
||||
Assert.NotNull(method);
|
||||
Assert.Equal("getData", method!.Name);
|
||||
Assert.Equal("GET", method.HttpMethod);
|
||||
|
||||
Assert.Null(await repo.GetMethodByNameAsync(system.Id, "noSuchMethod"));
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// ExternalSystemGateway-011: the site repository's name-keyed database-connection
|
||||
/// lookup returns the matching row, or <c>null</c> for a miss.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public async Task DatabaseConnectionRepository_GetByName_ReturnsMatchingDefinition()
|
||||
{
|
||||
var storage = NewStorage();
|
||||
await storage.InitializeAsync();
|
||||
await storage.StoreDatabaseConnectionAsync(
|
||||
"Plant", "Server=plant;Database=p;", maxRetries: 3, retryDelay: TimeSpan.FromSeconds(2));
|
||||
await storage.StoreDatabaseConnectionAsync(
|
||||
"Historian", "Server=hist;Database=h;", maxRetries: 0, retryDelay: TimeSpan.FromSeconds(5));
|
||||
|
||||
var repo = new SiteExternalSystemRepository(storage);
|
||||
|
||||
var found = await repo.GetDatabaseConnectionByNameAsync("Historian");
|
||||
Assert.NotNull(found);
|
||||
Assert.Equal("Historian", found!.Name);
|
||||
Assert.Equal("Server=hist;Database=h;", found.ConnectionString);
|
||||
Assert.Equal(0, found.MaxRetries);
|
||||
|
||||
Assert.Null(await repo.GetDatabaseConnectionByNameAsync("DoesNotExist"));
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// SiteRuntime-007: the same stability guarantee for notification lists.
|
||||
/// </summary>
|
||||
|
||||
@@ -3,6 +3,7 @@ using ScadaLink.Commons.Entities.Instances;
|
||||
using ScadaLink.Commons.Entities.Templates;
|
||||
using ScadaLink.Commons.Interfaces.Repositories;
|
||||
using ScadaLink.Commons.Interfaces.Services;
|
||||
using ScadaLink.Commons.Messages.Management;
|
||||
using ScadaLink.Commons.Types.Enums;
|
||||
using ScadaLink.TemplateEngine.Services;
|
||||
|
||||
@@ -260,7 +261,7 @@ public class InstanceServiceTests
|
||||
_repoMock.Setup(r => r.SaveChangesAsync(It.IsAny<CancellationToken>()))
|
||||
.ReturnsAsync(1);
|
||||
|
||||
var bindings = new List<(string, int)> { ("Temp", 100), ("Pressure", 200) };
|
||||
var bindings = new List<ConnectionBinding> { new("Temp", 100), new("Pressure", 200) };
|
||||
var result = await _sut.SetConnectionBindingsAsync(1, bindings, "admin");
|
||||
|
||||
Assert.True(result.IsSuccess);
|
||||
|
||||
Reference in New Issue
Block a user