Compare commits
6 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 25a05af05d | |||
| 0b4c1563aa | |||
| c07f524ca4 | |||
| 746ab90444 | |||
| d7b275fc9b | |||
| 404216b4ee |
@@ -8,7 +8,7 @@
|
|||||||
| Last reviewed | 2026-05-16 |
|
| Last reviewed | 2026-05-16 |
|
||||||
| Reviewer | claude-agent |
|
| Reviewer | claude-agent |
|
||||||
| Commit reviewed | `9c60592` |
|
| Commit reviewed | `9c60592` |
|
||||||
| Open findings | 6 |
|
| Open findings | 0 |
|
||||||
|
|
||||||
## Summary
|
## Summary
|
||||||
|
|
||||||
@@ -318,7 +318,7 @@ env vars (see CLI-006).
|
|||||||
|--|--|
|
|--|--|
|
||||||
| Severity | Low |
|
| Severity | Low |
|
||||||
| Category | Code organization & conventions |
|
| Category | Code organization & conventions |
|
||||||
| Status | Open |
|
| Status | Resolved |
|
||||||
| Location | `src/ScadaLink.CLI/Program.cs:10-11`, `src/ScadaLink.CLI/Commands/CommandHelpers.cs:60` |
|
| Location | `src/ScadaLink.CLI/Program.cs:10-11`, `src/ScadaLink.CLI/Commands/CommandHelpers.cs:60` |
|
||||||
|
|
||||||
**Description**
|
**Description**
|
||||||
@@ -334,7 +334,12 @@ Restrict the option to the accepted values, e.g. `formatOption.AcceptOnlyFromAmo
|
|||||||
|
|
||||||
**Resolution**
|
**Resolution**
|
||||||
|
|
||||||
_Unresolved._
|
Resolved 2026-05-16 (commit pending). Root cause confirmed — the `--format` option was
|
||||||
|
constructed inline in `Program.cs` with no value constraint. Extracted option
|
||||||
|
construction into a new `CliOptions.CreateFormatOption()` factory which applies
|
||||||
|
`AcceptOnlyFromAmong("json", "table")`; `Program.cs` now uses it. Invalid values are
|
||||||
|
rejected by `System.CommandLine` with a clear parse error. Regression tests in
|
||||||
|
`FormatOptionValidationTests`.
|
||||||
|
|
||||||
### CLI-009 — Exit-code documentation does not match `HandleResponse` behaviour
|
### CLI-009 — Exit-code documentation does not match `HandleResponse` behaviour
|
||||||
|
|
||||||
@@ -342,7 +347,7 @@ _Unresolved._
|
|||||||
|--|--|
|
|--|--|
|
||||||
| Severity | Low |
|
| Severity | Low |
|
||||||
| Category | Documentation & comments |
|
| Category | Documentation & comments |
|
||||||
| Status | Open |
|
| Status | Resolved |
|
||||||
| Location | `docs/requirements/Component-CLI.md:238-249`, `src/ScadaLink.CLI/Commands/CommandHelpers.cs:75` |
|
| Location | `docs/requirements/Component-CLI.md:238-249`, `src/ScadaLink.CLI/Commands/CommandHelpers.cs:75` |
|
||||||
|
|
||||||
**Description**
|
**Description**
|
||||||
@@ -365,7 +370,19 @@ with whichever is chosen.
|
|||||||
|
|
||||||
**Resolution**
|
**Resolution**
|
||||||
|
|
||||||
_Unresolved._
|
Resolved 2026-05-16 (commit pending). Took the recommendation's second option — keying
|
||||||
|
the authorization exit code off the response `code` field — since it makes the CLI honour
|
||||||
|
the documented "authorization failure → exit 2" contract regardless of which channel the
|
||||||
|
server uses, and it is the in-scope (code) fix. Replaced
|
||||||
|
`return response.StatusCode == 403 ? 2 : 1;` with a `HandleResponse` →
|
||||||
|
`IsAuthorizationFailure` helper that returns exit 2 for HTTP 403 **or** an error code of
|
||||||
|
`UNAUTHORIZED`/`FORBIDDEN` (case-insensitive), and exit 1 for everything else.
|
||||||
|
Authentication failure (HTTP 401 / bad credentials) deliberately remains exit 1.
|
||||||
|
Regression tests in `ExitCodeTests`. Note: the doc-side clarification of the full
|
||||||
|
client-side exit-code list (`NO_URL`, `NO_CREDENTIALS`, etc. → exit 1) was not made here —
|
||||||
|
`docs/requirements/Component-CLI.md` is outside this module's editable surface; the
|
||||||
|
remaining work is a non-blocking documentation-completeness edit owned by the docs
|
||||||
|
surface, and the CLI's exit-code behaviour itself is now correct and pinned by tests.
|
||||||
|
|
||||||
### CLI-010 — `debug stream` reports Ctrl+C during connect as a connection failure
|
### CLI-010 — `debug stream` reports Ctrl+C during connect as a connection failure
|
||||||
|
|
||||||
@@ -373,7 +390,7 @@ _Unresolved._
|
|||||||
|--|--|
|
|--|--|
|
||||||
| Severity | Low |
|
| Severity | Low |
|
||||||
| Category | Error handling & resilience |
|
| Category | Error handling & resilience |
|
||||||
| Status | Open |
|
| Status | Resolved |
|
||||||
| Location | `src/ScadaLink.CLI/Commands/DebugCommands.cs:181-189` |
|
| Location | `src/ScadaLink.CLI/Commands/DebugCommands.cs:181-189` |
|
||||||
|
|
||||||
**Description**
|
**Description**
|
||||||
@@ -394,7 +411,13 @@ lines 209-215 already treats cancellation as graceful.
|
|||||||
|
|
||||||
**Resolution**
|
**Resolution**
|
||||||
|
|
||||||
_Unresolved._
|
Resolved 2026-05-16 (commit pending). Root cause confirmed — the `StartAsync` `catch`
|
||||||
|
block reported every exception as `CONNECTION_FAILED`/exit 1. Extracted a pure
|
||||||
|
`DebugStreamHelpers.ClassifyConnectFailure(ex, cancellationRequested)` classifier: an
|
||||||
|
`OperationCanceledException` that coincides with a user-requested cancellation is treated
|
||||||
|
as a graceful shutdown (exit 0, no error printed); anything else is a genuine connection
|
||||||
|
failure (exit 1). The `StartAsync` catch block now uses it and disposes the connection on
|
||||||
|
both paths. Regression tests in `DebugStreamTests` (`ClassifyConnectFailure_*`).
|
||||||
|
|
||||||
### CLI-011 — `CancellationTokenSource` in `debug stream` is never disposed
|
### CLI-011 — `CancellationTokenSource` in `debug stream` is never disposed
|
||||||
|
|
||||||
@@ -402,7 +425,7 @@ _Unresolved._
|
|||||||
|--|--|
|
|--|--|
|
||||||
| Severity | Low |
|
| Severity | Low |
|
||||||
| Category | Performance & resource management |
|
| Category | Performance & resource management |
|
||||||
| Status | Open |
|
| Status | Resolved |
|
||||||
| Location | `src/ScadaLink.CLI/Commands/DebugCommands.cs:89` |
|
| Location | `src/ScadaLink.CLI/Commands/DebugCommands.cs:89` |
|
||||||
|
|
||||||
**Description**
|
**Description**
|
||||||
@@ -420,7 +443,12 @@ a `try/finally`).
|
|||||||
|
|
||||||
**Resolution**
|
**Resolution**
|
||||||
|
|
||||||
_Unresolved._
|
Resolved 2026-05-16 (commit pending). Root cause confirmed — `cts` was a plain local with
|
||||||
|
no disposal on any exit path. Changed to `using var cts = new CancellationTokenSource();`
|
||||||
|
in `StreamDebugAsync`, so it is disposed on every return path including the early
|
||||||
|
connect/subscribe-failure returns. No dedicated regression test — undisposed-`IDisposable`
|
||||||
|
is not observable from a unit test; the `using` declaration is verified by inspection and
|
||||||
|
covered indirectly by the `DebugStreamTests` exit-path tests.
|
||||||
|
|
||||||
### CLI-012 — `debug stream` exit code is unreliable after stream termination
|
### CLI-012 — `debug stream` exit code is unreliable after stream termination
|
||||||
|
|
||||||
@@ -428,7 +456,7 @@ _Unresolved._
|
|||||||
|--|--|
|
|--|--|
|
||||||
| Severity | Low |
|
| Severity | Low |
|
||||||
| Category | Concurrency & thread safety |
|
| Category | Concurrency & thread safety |
|
||||||
| Status | Open |
|
| Status | Resolved |
|
||||||
| Location | `src/ScadaLink.CLI/Commands/DebugCommands.cs:208-227` |
|
| Location | `src/ScadaLink.CLI/Commands/DebugCommands.cs:208-227` |
|
||||||
|
|
||||||
**Description**
|
**Description**
|
||||||
@@ -452,7 +480,15 @@ Consider awaiting `exitTcs.Task` without the cancellation token after a brief gr
|
|||||||
|
|
||||||
**Resolution**
|
**Resolution**
|
||||||
|
|
||||||
_Unresolved._
|
Resolved 2026-05-16 (commit pending). Root cause confirmed — the final
|
||||||
|
`exitTcs.Task.IsCompletedSuccessfully ? ... : 0` could miss an in-flight `TrySetResult`
|
||||||
|
that races with the cancelled `WaitAsync`. Extracted
|
||||||
|
`DebugStreamHelpers.ResolveStreamExitCodeAsync(exitTask)`: it returns an already-set
|
||||||
|
result immediately, otherwise waits up to a 250 ms grace period (`Task.WhenAny` against
|
||||||
|
`Task.Delay`) for a termination/closed result to land, and falls back to exit 0 only when
|
||||||
|
no result is ever produced (pure Ctrl+C). `StreamDebugAsync` now resolves its exit code
|
||||||
|
solely through this helper. Regression tests in `DebugStreamTests`
|
||||||
|
(`ResolveStreamExitCodeAsync_*`, including the termination-racing-with-cancellation case).
|
||||||
|
|
||||||
### CLI-013 — HTTP client, `debug stream`, and JSON-argument parsing are untested
|
### CLI-013 — HTTP client, `debug stream`, and JSON-argument parsing are untested
|
||||||
|
|
||||||
@@ -460,7 +496,7 @@ _Unresolved._
|
|||||||
|--|--|
|
|--|--|
|
||||||
| Severity | Low |
|
| Severity | Low |
|
||||||
| Category | Testing coverage |
|
| Category | Testing coverage |
|
||||||
| Status | Open |
|
| Status | Resolved |
|
||||||
| Location | `tests/ScadaLink.CLI.Tests/` (vs. `src/ScadaLink.CLI/ManagementHttpClient.cs`, `src/ScadaLink.CLI/Commands/DebugCommands.cs`, `src/ScadaLink.CLI/Commands/InstanceCommands.cs:55-58`) |
|
| Location | `tests/ScadaLink.CLI.Tests/` (vs. `src/ScadaLink.CLI/ManagementHttpClient.cs`, `src/ScadaLink.CLI/Commands/DebugCommands.cs`, `src/ScadaLink.CLI/Commands/InstanceCommands.cs:55-58`) |
|
||||||
|
|
||||||
**Description**
|
**Description**
|
||||||
@@ -487,4 +523,17 @@ resolves via `ManagementCommandRegistry`.
|
|||||||
|
|
||||||
**Resolution**
|
**Resolution**
|
||||||
|
|
||||||
_Unresolved._
|
Resolved 2026-05-16 (commit pending). Coverage gaps confirmed and closed:
|
||||||
|
- `ManagementHttpClientTests` — added an `internal` test-only `ManagementHttpClient`
|
||||||
|
constructor accepting a pre-built `HttpClient`, and a stub `HttpMessageHandler`; tests
|
||||||
|
cover the success, JSON error-body, non-JSON error-body fallback, connection-failure
|
||||||
|
(status 0), and timeout (504) paths.
|
||||||
|
- `CommandTreeTests` — builds all 14 command groups, recursively collects every leaf
|
||||||
|
command and asserts each has an action (no dead wiring), and verifies representative
|
||||||
|
command payload records round-trip through `ManagementCommandRegistry`.
|
||||||
|
- `DebugStreamTests` — covers the `debug stream` decision logic via the new
|
||||||
|
`DebugStreamHelpers` (see CLI-010, CLI-012).
|
||||||
|
- JSON-argument parsing (`set-bindings`/`set-overrides`) was already extracted into
|
||||||
|
`InstanceCommands.TryParseBindings`/`TryParseOverrides` and covered by
|
||||||
|
`InstanceArgumentParsingTests` under CLI-005.
|
||||||
|
The CLI test suite went from 42 to 77 passing tests.
|
||||||
|
|||||||
@@ -8,7 +8,7 @@
|
|||||||
| Last reviewed | 2026-05-16 |
|
| Last reviewed | 2026-05-16 |
|
||||||
| Reviewer | claude-agent |
|
| Reviewer | claude-agent |
|
||||||
| Commit reviewed | `9c60592` |
|
| Commit reviewed | `9c60592` |
|
||||||
| Open findings | 7 |
|
| Open findings | 2 |
|
||||||
|
|
||||||
## Summary
|
## Summary
|
||||||
|
|
||||||
@@ -664,7 +664,7 @@ cannot silently regress.
|
|||||||
|--|--|
|
|--|--|
|
||||||
| Severity | Low |
|
| Severity | Low |
|
||||||
| Category | Concurrency & thread safety |
|
| Category | Concurrency & thread safety |
|
||||||
| Status | Open |
|
| Status | Won't Fix |
|
||||||
| Location | `src/ScadaLink.CentralUI/ServiceCollectionExtensions.cs:24`; `src/ScadaLink.CentralUI/Components/Shared/DialogService.cs:18-69` |
|
| Location | `src/ScadaLink.CentralUI/ServiceCollectionExtensions.cs:24`; `src/ScadaLink.CentralUI/Components/Shared/DialogService.cs:18-69` |
|
||||||
|
|
||||||
**Description**
|
**Description**
|
||||||
@@ -684,7 +684,25 @@ call sites for off-thread state mutation.
|
|||||||
|
|
||||||
**Resolution**
|
**Resolution**
|
||||||
|
|
||||||
_Unresolved._
|
Won't Fix — **re-triaged 2026-05-16, the finding's premise is incorrect.** The
|
||||||
|
finding claims `ContinueWith(..., TaskScheduler.Default)` makes an awaiting
|
||||||
|
caller resume on a thread-pool thread. It does not. `TaskScheduler.Default` on
|
||||||
|
`ContinueWith` only governs where the trivial *projection lambda* runs (inside
|
||||||
|
`DialogService`); it has no effect on where the *caller* resumes. An `await`
|
||||||
|
always captures and resumes on the awaiter's own `SynchronizationContext` — for
|
||||||
|
a Blazor event-handler caller, that is the renderer's dispatcher — regardless of
|
||||||
|
where the awaited task completes. This was verified directly:
|
||||||
|
`DialogServiceThreadingTests.ConfirmAsync_AwaiterResumesOnItsCapturedSyncContext`
|
||||||
|
pins that the continuation posts back to the caller's captured context, and the
|
||||||
|
test **passes against both** the original `ContinueWith` form and the current
|
||||||
|
code, confirming there was never an off-render-thread resume to fix. The
|
||||||
|
`DialogService` was nonetheless cleaned up opportunistically — the explicit
|
||||||
|
`ContinueWith(..., TaskScheduler.Default)` projections were replaced with an
|
||||||
|
inline typed projection (`Project<TResult>`), removing a needless thread-pool
|
||||||
|
hop and making the flow easier to read — but that is a quality tidy-up, not a
|
||||||
|
bug fix. Characterization tests `DialogServiceThreadingTests` (4 tests) pin the
|
||||||
|
sync-context behaviour and the confirm/prompt/cancel resolution contract so the
|
||||||
|
service cannot silently regress.
|
||||||
|
|
||||||
### CentralUI-016 — Pagers render one button per page with no windowing
|
### CentralUI-016 — Pagers render one button per page with no windowing
|
||||||
|
|
||||||
@@ -692,7 +710,7 @@ _Unresolved._
|
|||||||
|--|--|
|
|--|--|
|
||||||
| Severity | Low |
|
| Severity | Low |
|
||||||
| Category | Performance & resource management |
|
| Category | Performance & resource management |
|
||||||
| Status | Open |
|
| Status | Resolved |
|
||||||
| Location | `src/ScadaLink.CentralUI/Components/Shared/DataTable.razor:62-68`; `src/ScadaLink.CentralUI/Components/Pages/Deployment/Deployments.razor:167-173` |
|
| Location | `src/ScadaLink.CentralUI/Components/Shared/DataTable.razor:62-68`; `src/ScadaLink.CentralUI/Components/Pages/Deployment/Deployments.razor:167-173` |
|
||||||
|
|
||||||
**Description**
|
**Description**
|
||||||
@@ -710,7 +728,19 @@ large lists to a "load more" / numeric jump input.
|
|||||||
|
|
||||||
**Resolution**
|
**Resolution**
|
||||||
|
|
||||||
_Unresolved._
|
Resolved 2026-05-16 (commit pending). Confirmed: both `DataTable` and
|
||||||
|
`Deployments` looped `for i = 1..totalPages` and emitted one numbered `<li>`
|
||||||
|
button per page — 200 buttons for a 5000-row dataset at page size 25. Added a
|
||||||
|
pure `PagerWindow.Build(currentPage, totalPages)` helper
|
||||||
|
(`Components/Shared/PagerWindow.cs`) that returns a bounded window — always the
|
||||||
|
first and last page plus a small range around the current page, with a `0`
|
||||||
|
sentinel marking an elided gap (rendered as a disabled `…`). Both
|
||||||
|
paginators now iterate `PagerWindow.Build(...)` instead of the full range;
|
||||||
|
small datasets (<= 9 pages) still render every page so nothing is hidden
|
||||||
|
needlessly. Regression tests: `DataTablePagerTests` (3 bUnit tests — proves the
|
||||||
|
windowed pager renders <= 12 numbered buttons for 200 pages where the pre-fix
|
||||||
|
code rendered 200, still renders all pages for a small dataset, and always
|
||||||
|
includes first/last) and `PagerWindowTests` (6 tests pinning the helper logic).
|
||||||
|
|
||||||
### CentralUI-017 — `/auth/logout` POST disables antiforgery, enabling logout CSRF
|
### CentralUI-017 — `/auth/logout` POST disables antiforgery, enabling logout CSRF
|
||||||
|
|
||||||
@@ -718,7 +748,7 @@ _Unresolved._
|
|||||||
|--|--|
|
|--|--|
|
||||||
| Severity | Low |
|
| Severity | Low |
|
||||||
| Category | Security |
|
| Category | Security |
|
||||||
| Status | Open |
|
| Status | Resolved |
|
||||||
| Location | `src/ScadaLink.CentralUI/Auth/AuthEndpoints.cs:127-138` |
|
| Location | `src/ScadaLink.CentralUI/Auth/AuthEndpoints.cs:127-138` |
|
||||||
|
|
||||||
**Description**
|
**Description**
|
||||||
@@ -737,7 +767,21 @@ can include the antiforgery token), and remove or protect the state-changing
|
|||||||
|
|
||||||
**Resolution**
|
**Resolution**
|
||||||
|
|
||||||
_Unresolved._
|
Resolved 2026-05-16 (commit pending). Confirmed: `POST /auth/logout` called
|
||||||
|
`.DisableAntiforgery()` and a plain `GET /logout` route also signed the user
|
||||||
|
out — either was triggerable cross-site (`<img src="/logout">` or an
|
||||||
|
auto-submitting form) to forcibly log a user out. The `.DisableAntiforgery()`
|
||||||
|
call was removed from `POST /auth/logout` so it now requires a valid
|
||||||
|
antiforgery token, and the `NavMenu` sign-out form was given an
|
||||||
|
`<AntiforgeryToken />` so the legitimate logout still works. The state-changing
|
||||||
|
`GET /logout` route was deleted outright (a state-changing GET is itself a CSRF
|
||||||
|
vector). `POST /auth/login` intentionally keeps `.DisableAntiforgery()` — it is
|
||||||
|
a pre-auth endpoint where there is no session/token yet. Regression tests
|
||||||
|
`AuthEndpointsCsrfTests` (3 tests, inspecting the mapped endpoints' metadata):
|
||||||
|
`PostAuthLogout_DoesNotDisableAntiforgery` and
|
||||||
|
`GetLogout_StateChangingRoute_IsRemoved` fail against the pre-fix code and pass
|
||||||
|
after; `PostAuthLogin_StillDisablesAntiforgery_PreAuthIsAcceptable` guards that
|
||||||
|
the pre-auth login exemption was not over-corrected.
|
||||||
|
|
||||||
### CentralUI-018 — Broad `catch {}` blocks swallow JS interop and storage errors silently
|
### CentralUI-018 — Broad `catch {}` blocks swallow JS interop and storage errors silently
|
||||||
|
|
||||||
@@ -745,7 +789,7 @@ _Unresolved._
|
|||||||
|--|--|
|
|--|--|
|
||||||
| Severity | Low |
|
| Severity | Low |
|
||||||
| Category | Error handling & resilience |
|
| Category | Error handling & resilience |
|
||||||
| Status | Open |
|
| Status | Resolved |
|
||||||
| Location | `src/ScadaLink.CentralUI/Components/Shared/MonacoEditor.razor:116-118,123,142,164,170,176,182,189`; `src/ScadaLink.CentralUI/Components/Shared/TreeView.razor:129,139`; `src/ScadaLink.CentralUI/Components/Pages/Admin/Sites.razor:316-319` |
|
| Location | `src/ScadaLink.CentralUI/Components/Shared/MonacoEditor.razor:116-118,123,142,164,170,176,182,189`; `src/ScadaLink.CentralUI/Components/Shared/TreeView.razor:129,139`; `src/ScadaLink.CentralUI/Components/Pages/Admin/Sites.razor:316-319` |
|
||||||
|
|
||||||
**Description**
|
**Description**
|
||||||
@@ -766,7 +810,30 @@ Catch the specific expected exception type (e.g. `JSDisconnectedException`,
|
|||||||
|
|
||||||
**Resolution**
|
**Resolution**
|
||||||
|
|
||||||
_Unresolved._
|
Resolved 2026-05-16 (commit pending). Confirmed all three locations.
|
||||||
|
(1) **TreeView** — the storage-restore `JsonSerializer.Deserialize<List<string>>`
|
||||||
|
was outside any try block, so a corrupt `treeviewStorage` payload threw an
|
||||||
|
uncaught `JsonException` out of `OnAfterRenderAsync`. The deserialize is now
|
||||||
|
wrapped in a `try/catch (JsonException)` that treats an unparseable payload as
|
||||||
|
"no prior state" (falling back to `InitiallyExpanded`); the `treeviewStorage.load`
|
||||||
|
interop call is guarded for `JSDisconnectedException`; and the context-menu
|
||||||
|
`FocusAsync` catch was narrowed from a bare `catch` to the specific expected
|
||||||
|
types (`JSException`/`JSDisconnectedException`/`InvalidOperationException`).
|
||||||
|
(2) **MonacoEditor** — every JS interop call had a bare `catch { }`. The
|
||||||
|
component now injects `ILogger<MonacoEditor>`; `createEditor` distinguishes the
|
||||||
|
expected prerender (`InvalidOperationException`) and disconnect
|
||||||
|
(`JSDisconnectedException`) cases — silent — from a genuine `JSException`, which
|
||||||
|
is logged via `LogError`. The other six interop calls route through a new
|
||||||
|
`SafeInvokeAsync` helper that swallows `JSDisconnectedException` but logs a real
|
||||||
|
`JSException` via `LogWarning`. (3) **Sites.CopyAsync** — the bare `catch` was
|
||||||
|
split into a silent `JSDisconnectedException` arm and a `JSException` arm that
|
||||||
|
logs via a newly injected `ILogger<Sites>` before showing the error toast.
|
||||||
|
Regression tests: `TreeViewStorageResilienceTests` (2 tests — a corrupt and a
|
||||||
|
wrong-shaped payload no longer throw and the tree still renders; both fail
|
||||||
|
against the pre-fix unguarded `Deserialize`) and `MonacoEditorLoggingTests`
|
||||||
|
(2 tests — a genuine `JSException` during init is logged, verified to fail
|
||||||
|
against the pre-fix bare `catch {}`; a prerender `InvalidOperationException` is
|
||||||
|
not logged).
|
||||||
|
|
||||||
### CentralUI-019 — Sparse unit-test coverage for a large module; critical paths untested
|
### CentralUI-019 — Sparse unit-test coverage for a large module; critical paths untested
|
||||||
|
|
||||||
@@ -774,7 +841,7 @@ _Unresolved._
|
|||||||
|--|--|
|
|--|--|
|
||||||
| Severity | Low |
|
| Severity | Low |
|
||||||
| Category | Testing coverage |
|
| Category | Testing coverage |
|
||||||
| Status | Open |
|
| Status | Resolved |
|
||||||
| Location | `tests/ScadaLink.CentralUI.Tests/` |
|
| Location | `tests/ScadaLink.CentralUI.Tests/` |
|
||||||
|
|
||||||
**Description**
|
**Description**
|
||||||
@@ -798,4 +865,25 @@ findings.
|
|||||||
|
|
||||||
**Resolution**
|
**Resolution**
|
||||||
|
|
||||||
_Unresolved._
|
Resolved 2026-05-16 (commit pending). The coverage gap has been closed across
|
||||||
|
the cumulative fixes for CentralUI-001 .. 018 — every critical path the finding
|
||||||
|
named now has tests. Sandbox-run / forbidden-API rejection:
|
||||||
|
`ScriptAnalysisServiceTests`, `ScriptAnalysisAsyncResolveTests`,
|
||||||
|
`TestRunWarningTests` (from CentralUI-001/013/014). Auth bridge:
|
||||||
|
`CookieAuthenticationStateProviderTests`, `SiteScopeServiceTests`,
|
||||||
|
`AuthEndpointsCsrfTests` (from CentralUI-002/004/017). Dialog resolution:
|
||||||
|
`DiffDialogTests` and the new `DialogServiceThreadingTests` (4 tests pinning
|
||||||
|
`ConfirmAsync`/`PromptAsync` sync-context and confirm/prompt/cancel resolution
|
||||||
|
semantics). DebugView lifecycle: `DebugViewDisposalTests` (from CentralUI-009).
|
||||||
|
Toast/timer disposal: `ToastNotificationTests` (from CentralUI-010).
|
||||||
|
This batch also added `BrowserTimeTests`, `MonitoringAuthorizationTests`,
|
||||||
|
`SitesPageTests`, `DataTablePagerTests` + `PagerWindowTests`,
|
||||||
|
`TreeViewStorageResilienceTests`, and `MonacoEditorLoggingTests`. The
|
||||||
|
`tests/ScadaLink.CentralUI.Tests` suite is green at 251 tests. Remaining
|
||||||
|
untested paths are low-risk render-only pages; the Critical/High/Medium paths
|
||||||
|
the finding prioritised are all now covered, so the finding is considered
|
||||||
|
resolved. (Note: `TopologyPageTests`'s DI setup was also updated this session —
|
||||||
|
it was failing on the baseline because `DeploymentService` had gained a
|
||||||
|
`DiffService` constructor dependency from a DeploymentManager contract change
|
||||||
|
that the test fixture had not been updated for; `DiffService` is now registered
|
||||||
|
in the fixture.)
|
||||||
|
|||||||
@@ -8,7 +8,7 @@
|
|||||||
| Last reviewed | 2026-05-16 |
|
| Last reviewed | 2026-05-16 |
|
||||||
| Reviewer | claude-agent |
|
| Reviewer | claude-agent |
|
||||||
| Commit reviewed | `9c60592` |
|
| Commit reviewed | `9c60592` |
|
||||||
| Open findings | 3 |
|
| Open findings | 0 |
|
||||||
|
|
||||||
## Summary
|
## Summary
|
||||||
|
|
||||||
@@ -302,7 +302,7 @@ attributes cannot. Covered by `ClusterOptionsValidatorTests` (8 cases) and
|
|||||||
|--|--|
|
|--|--|
|
||||||
| Severity | Low |
|
| Severity | Low |
|
||||||
| Category | Code organization & conventions |
|
| Category | Code organization & conventions |
|
||||||
| Status | Open |
|
| Status | Resolved |
|
||||||
| Location | `src/ScadaLink.ClusterInfrastructure/ClusterOptions.cs:3` |
|
| Location | `src/ScadaLink.ClusterInfrastructure/ClusterOptions.cs:3` |
|
||||||
|
|
||||||
**Description**
|
**Description**
|
||||||
@@ -323,7 +323,17 @@ Add a `public const string SectionName = "Cluster";` (or the agreed name) to
|
|||||||
|
|
||||||
**Resolution**
|
**Resolution**
|
||||||
|
|
||||||
_Unresolved._
|
Confirmed against the source: `ClusterOptions` previously exposed no section-name
|
||||||
|
constant, leaving binding sites to hard-code the magic string.
|
||||||
|
|
||||||
|
**Resolved** — fixing commit `commit pending`, date 2026-05-16. `ClusterOptions` now
|
||||||
|
exposes `public const string SectionName = "ScadaLink:Cluster";` as the single source
|
||||||
|
of truth for the `appsettings.json` section name, with an XML doc explaining its
|
||||||
|
purpose. The chosen value matches the `ScadaLink:`-prefixed section convention used by
|
||||||
|
peer option classes and referenced by `ClusterOptionsValidator` / the design doc.
|
||||||
|
Covered by `ClusterOptionsTests.SectionName_IsTheExpectedAppSettingsSection`, which
|
||||||
|
both pins the value and — by referencing the constant — guards against its removal
|
||||||
|
(its deletion would break compilation).
|
||||||
|
|
||||||
### ClusterInfrastructure-006 — No tests for any cluster behaviour; only the options POCO is covered
|
### ClusterInfrastructure-006 — No tests for any cluster behaviour; only the options POCO is covered
|
||||||
|
|
||||||
@@ -385,7 +395,7 @@ design); `ClusterOptionsValidator` is the layer that now rejects `keep-majority`
|
|||||||
|--|--|
|
|--|--|
|
||||||
| Severity | Low |
|
| Severity | Low |
|
||||||
| Category | Documentation & comments |
|
| Category | Documentation & comments |
|
||||||
| Status | Open |
|
| Status | Resolved |
|
||||||
| Location | `src/ScadaLink.ClusterInfrastructure/ClusterOptions.cs:3-11` |
|
| Location | `src/ScadaLink.ClusterInfrastructure/ClusterOptions.cs:3-11` |
|
||||||
|
|
||||||
**Description**
|
**Description**
|
||||||
@@ -407,7 +417,20 @@ the relevant design-doc sections as peer modules do.
|
|||||||
|
|
||||||
**Resolution**
|
**Resolution**
|
||||||
|
|
||||||
_Unresolved._
|
Confirmed against the source: `ClusterOptions` previously had no XML doc comments on
|
||||||
|
the class or any property.
|
||||||
|
|
||||||
|
**Resolved** — fixing commit `commit pending`, date 2026-05-16. `ClusterOptions` now
|
||||||
|
carries a class-level `<summary>` documenting the cluster configuration contract and
|
||||||
|
the deliberate ownership split (node identity / remoting / gRPC in
|
||||||
|
`Host.NodeOptions`, storage paths in the database options, cluster-formation settings
|
||||||
|
here), plus `<summary>` comments on all seven properties — each stating units,
|
||||||
|
defaults, and the design-doc constraints. Notably `MinNrOfMembers` is documented as
|
||||||
|
required to be `1` (a value of `2` blocks the cluster singleton after failover) and
|
||||||
|
`HeartbeatInterval` as required to be well below `FailureDetectionThreshold`, giving a
|
||||||
|
future editor the in-code warnings the recommendation asks for. This is a
|
||||||
|
documentation-only change, so no runtime regression test is meaningful; verified by
|
||||||
|
inspection of `ClusterOptions.cs:3-74`. Module test suite green (17 passed).
|
||||||
|
|
||||||
### ClusterInfrastructure-008 — "Phase 0 skeleton" status is undocumented at the module level
|
### ClusterInfrastructure-008 — "Phase 0 skeleton" status is undocumented at the module level
|
||||||
|
|
||||||
@@ -415,7 +438,7 @@ _Unresolved._
|
|||||||
|--|--|
|
|--|--|
|
||||||
| Severity | Low |
|
| Severity | Low |
|
||||||
| Category | Documentation & comments |
|
| Category | Documentation & comments |
|
||||||
| Status | Open |
|
| Status | Resolved |
|
||||||
| Location | `src/ScadaLink.ClusterInfrastructure/ServiceCollectionExtensions.cs:9`, `src/ScadaLink.ClusterInfrastructure/ServiceCollectionExtensions.cs:16` |
|
| Location | `src/ScadaLink.ClusterInfrastructure/ServiceCollectionExtensions.cs:9`, `src/ScadaLink.ClusterInfrastructure/ServiceCollectionExtensions.cs:16` |
|
||||||
|
|
||||||
**Description**
|
**Description**
|
||||||
@@ -439,4 +462,31 @@ about which components are skeletons versus implemented.
|
|||||||
|
|
||||||
**Resolution**
|
**Resolution**
|
||||||
|
|
||||||
_Unresolved._
|
Re-triaged in light of CI-001's resolution. The original defect was the mismatch
|
||||||
|
between two misleading inline `// Phase 0: skeleton only` /
|
||||||
|
`// Phase 0: placeholder` comments buried in private method bodies and a
|
||||||
|
complete-looking design doc with no caveat. That premise has been overtaken by the
|
||||||
|
CI-001/CI-002 work:
|
||||||
|
|
||||||
|
- The "Phase 0 skeleton" comments no longer exist anywhere in
|
||||||
|
`src/ScadaLink.ClusterInfrastructure` (verified by `grep`). `ServiceCollectionExtensions`
|
||||||
|
now does real work (registers `ClusterOptionsValidator`) and `AddClusterInfrastructureActors`
|
||||||
|
throws explicitly — both with accurate XML docs explaining the ownership split.
|
||||||
|
- The module is no longer an unimplemented skeleton. CI-001 established that the Akka
|
||||||
|
bootstrap legitimately lives in `ScadaLink.Host`, and this project's true scope —
|
||||||
|
the `ClusterOptions` configuration contract, its validator, and DI registration — is
|
||||||
|
fully implemented and tested.
|
||||||
|
- The design doc `Component-ClusterInfrastructure.md` now opens with an
|
||||||
|
"Implementation Note — Code Placement" section (added by CI-001) that explicitly
|
||||||
|
states the component is a *design responsibility* realised across
|
||||||
|
`ScadaLink.ClusterInfrastructure` (configuration model) and `ScadaLink.Host`
|
||||||
|
(bootstrap/runtime wiring), and the README component table (row 13) was updated to
|
||||||
|
match. A reader of the design doc no longer assumes a single fully-built project.
|
||||||
|
|
||||||
|
**Resolved** — fixing commit `commit pending`, date 2026-05-16. No further change
|
||||||
|
required: the misleading skeleton comments are gone, the module's real status is
|
||||||
|
documented at the design-doc level via the "Implementation Note — Code Placement"
|
||||||
|
section, and the component table reflects the true placement. This is a
|
||||||
|
documentation-only finding, so no runtime regression test is meaningful; verified by
|
||||||
|
inspection of `ServiceCollectionExtensions.cs` and
|
||||||
|
`docs/requirements/Component-ClusterInfrastructure.md:21-39`.
|
||||||
|
|||||||
@@ -8,7 +8,7 @@
|
|||||||
| Last reviewed | 2026-05-16 |
|
| Last reviewed | 2026-05-16 |
|
||||||
| Reviewer | claude-agent |
|
| Reviewer | claude-agent |
|
||||||
| Commit reviewed | `9c60592` |
|
| Commit reviewed | `9c60592` |
|
||||||
| Open findings | 8 |
|
| Open findings | 1 |
|
||||||
|
|
||||||
## Summary
|
## Summary
|
||||||
|
|
||||||
@@ -229,7 +229,7 @@ SiteRuntime all build clean against the change. Regression tests added in
|
|||||||
|--|--|
|
|--|--|
|
||||||
| Severity | Low |
|
| Severity | Low |
|
||||||
| Category | Error handling & resilience |
|
| Category | Error handling & resilience |
|
||||||
| Status | Open |
|
| Status | Resolved |
|
||||||
| Location | `src/ScadaLink.Commons/Serialization/OpcUaEndpointConfigSerializer.cs:25-51` |
|
| Location | `src/ScadaLink.Commons/Serialization/OpcUaEndpointConfigSerializer.cs:25-51` |
|
||||||
|
|
||||||
**Description**
|
**Description**
|
||||||
@@ -252,7 +252,19 @@ empty form. Update the XML doc to describe the failure branch.
|
|||||||
|
|
||||||
**Resolution**
|
**Resolution**
|
||||||
|
|
||||||
_Unresolved._
|
Resolved 2026-05-16 (commit pending) — confirmed the over-reporting and silent data-loss
|
||||||
|
branch. `Deserialize` now returns an `OpcUaConfigParseResult` (a `readonly record struct`)
|
||||||
|
carrying an explicit `OpcUaConfigParseStatus` (`Typed` / `Legacy` / `Malformed`); genuinely
|
||||||
|
unparseable input is classified `Malformed` with `IsLegacy == false` instead of being
|
||||||
|
mislabelled as a recoverable legacy row. The struct keeps a custom two-element
|
||||||
|
`Deconstruct(out Config, out bool isLegacy)` so the existing SiteRuntime caller
|
||||||
|
(`var (config, _) = ...`) is unaffected — verified by a green SiteRuntime build. The XML
|
||||||
|
doc now describes all three outcomes. Throwing was rejected because the sole consumer
|
||||||
|
(`DeploymentManagerActor.FlattenConnectionConfig`) does not wrap the call and is
|
||||||
|
out-of-scope to change. Regression tests added in `OpcUaEndpointConfigSerializerTests`
|
||||||
|
(`Deserialize_Malformed_ReportsMalformedNotLegacy`, `Deserialize_LegacyParsed_StatusIsLegacy`,
|
||||||
|
`Deserialize_ObjectWithoutEndpointUrl_ParsesAsLegacy`,
|
||||||
|
`Deserialize_TwoElementDeconstruction_StillWorks`).
|
||||||
|
|
||||||
### Commons-006 — `DynamicJsonElement.TryConvert` reports success for unconvertible target types
|
### Commons-006 — `DynamicJsonElement.TryConvert` reports success for unconvertible target types
|
||||||
|
|
||||||
@@ -260,7 +272,7 @@ _Unresolved._
|
|||||||
|--|--|
|
|--|--|
|
||||||
| Severity | Low |
|
| Severity | Low |
|
||||||
| Category | Correctness & logic bugs |
|
| Category | Correctness & logic bugs |
|
||||||
| Status | Open |
|
| Status | Resolved |
|
||||||
| Location | `src/ScadaLink.Commons/Types/DynamicJsonElement.cs:47-51`, `:66-76` |
|
| Location | `src/ScadaLink.Commons/Types/DynamicJsonElement.cs:47-51`, `:66-76` |
|
||||||
|
|
||||||
**Description**
|
**Description**
|
||||||
@@ -282,7 +294,16 @@ For the `object` target, return the element itself (or `Wrap(_element)`) rather
|
|||||||
|
|
||||||
**Resolution**
|
**Resolution**
|
||||||
|
|
||||||
_Unresolved._
|
Resolved 2026-05-16 (commit pending) — confirmed by a direct `TryConvert` regression test
|
||||||
|
that the original code returned `(result: null, true)` for an `object` conversion of a
|
||||||
|
present value. `TryConvert` now handles the `object` target explicitly: it returns
|
||||||
|
`Wrap(_element)` (the unwrapped scalar, or the wrapper for objects/arrays) and yields
|
||||||
|
`null` only for a genuine `JsonValueKind.Null`. Non-`object` targets keep the correct
|
||||||
|
behavior — a `null` from `ConvertTo` now reports `false` (the dead `|| typeof(object)`
|
||||||
|
clause is removed) so the binder surfaces a real binding error for unconvertible pairs.
|
||||||
|
Regression tests added in `DynamicJsonElementTests` (`TryConvert_ObjectTarget_OnPresentValue_ReturnsNonNull`,
|
||||||
|
`TryConvert_ObjectTarget_OnJsonNull_ReturnsNull`,
|
||||||
|
`TryConvert_NonObjectTarget_OnUnconvertibleValue_ReportsFailure`).
|
||||||
|
|
||||||
### Commons-007 — Several Commons types carry non-trivial logic, stretching REQ-COM-6
|
### Commons-007 — Several Commons types carry non-trivial logic, stretching REQ-COM-6
|
||||||
|
|
||||||
@@ -290,7 +311,7 @@ _Unresolved._
|
|||||||
|--|--|
|
|--|--|
|
||||||
| Severity | Low |
|
| Severity | Low |
|
||||||
| Category | Design-document adherence |
|
| Category | Design-document adherence |
|
||||||
| Status | Open |
|
| Status | Resolved |
|
||||||
| Location | `src/ScadaLink.Commons/Types/ScriptParameters.cs`, `src/ScadaLink.Commons/Serialization/OpcUaEndpointConfigSerializer.cs`, `src/ScadaLink.Commons/Validators/OpcUaEndpointConfigValidator.cs`, `src/ScadaLink.Commons/Types/StaleTagMonitor.cs`, `src/ScadaLink.Commons/Types/ScriptArgs.cs` |
|
| Location | `src/ScadaLink.Commons/Types/ScriptParameters.cs`, `src/ScadaLink.Commons/Serialization/OpcUaEndpointConfigSerializer.cs`, `src/ScadaLink.Commons/Validators/OpcUaEndpointConfigValidator.cs`, `src/ScadaLink.Commons/Types/StaleTagMonitor.cs`, `src/ScadaLink.Commons/Types/ScriptArgs.cs` |
|
||||||
|
|
||||||
**Description**
|
**Description**
|
||||||
@@ -317,7 +338,18 @@ them. Tighten the architectural test if the rule is meant to be enforced.
|
|||||||
|
|
||||||
**Resolution**
|
**Resolution**
|
||||||
|
|
||||||
_Unresolved._
|
Resolved 2026-05-16 (commit pending) — design-document decision. This is drift, not a
|
||||||
|
defect: the helpers are pure, dependency-light (no Akka/ASP.NET/EF, no I/O), and shared
|
||||||
|
across components, so the intentional choice is to keep them in Commons rather than
|
||||||
|
duplicate the logic. REQ-COM-6 in `docs/requirements/Component-Commons.md` was amended
|
||||||
|
with an explicit "pure-helper carve-out" — stateless, side-effect-free helpers that only
|
||||||
|
transform/format/parse/validate Commons' own data types are now permitted, with the
|
||||||
|
qualifying conditions (no I/O dependency, no shared mutable cross-call state, no
|
||||||
|
orchestration) and the current examples (`Result<T>`, `ScriptParameters`, `ScriptArgs`,
|
||||||
|
`ValueFormatter`, `DynamicJsonElement`, `StaleTagMonitor`, `OpcUaEndpointConfigSerializer`,
|
||||||
|
`OpcUaEndpointConfigValidator`) spelled out. No regression test — this is a documentation
|
||||||
|
policy decision. The `ArchitecturalConstraintTests` heuristic was left as-is since the
|
||||||
|
carve-out makes these types compliant by design.
|
||||||
|
|
||||||
### Commons-008 — `SetConnectionBindingsCommand` uses `ValueTuple` in a wire message contract
|
### Commons-008 — `SetConnectionBindingsCommand` uses `ValueTuple` in a wire message contract
|
||||||
|
|
||||||
@@ -347,7 +379,18 @@ Replace the tuple with a small named record, e.g.
|
|||||||
|
|
||||||
**Resolution**
|
**Resolution**
|
||||||
|
|
||||||
_Unresolved._
|
_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.
|
||||||
|
|
||||||
### Commons-009 — `Component-Commons.md` is stale relative to the actual file set
|
### Commons-009 — `Component-Commons.md` is stale relative to the actual file set
|
||||||
|
|
||||||
@@ -355,7 +398,7 @@ _Unresolved._
|
|||||||
|--|--|
|
|--|--|
|
||||||
| Severity | Low |
|
| Severity | Low |
|
||||||
| Category | Documentation & comments |
|
| Category | Documentation & comments |
|
||||||
| Status | Open |
|
| Status | Resolved |
|
||||||
| Location | `docs/requirements/Component-Commons.md:61-198` |
|
| Location | `docs/requirements/Component-Commons.md:61-198` |
|
||||||
|
|
||||||
**Description**
|
**Description**
|
||||||
@@ -385,7 +428,18 @@ folders.
|
|||||||
|
|
||||||
**Resolution**
|
**Resolution**
|
||||||
|
|
||||||
_Unresolved._
|
Resolved 2026-05-16 (commit pending) — `docs/requirements/Component-Commons.md` was
|
||||||
|
refreshed against the actual file set. REQ-COM-3 now lists `TemplateFolder`,
|
||||||
|
`InstanceAlarmOverride` and `DeployedConfigSnapshot` and drops the non-existent
|
||||||
|
`SiteDataConnectionAssignment` (no such entity in the code). REQ-COM-4 adds
|
||||||
|
`ISiteRepository` (eight repositories). REQ-COM-4a documents `IDatabaseGateway`,
|
||||||
|
`IExternalSystemClient`, `IInstanceLocator` and `INotificationDeliveryService` alongside
|
||||||
|
`IAuditService`, and corrects the `IAuditService` owner to the Configuration Database
|
||||||
|
component. The REQ-COM-5b folder tree was rewritten to include the previously-absent
|
||||||
|
`Types/DataConnections`, `Types/Flattening`, `Types/Scripts`, the helper types under
|
||||||
|
`Types/`, the `Messages/DataConnection|Instance|Integration|InboundApi|RemoteQuery|Management`
|
||||||
|
namespaces, and the `Serialization/` and `Validators/` folders. Documentation-only; no
|
||||||
|
regression test.
|
||||||
|
|
||||||
### Commons-010 — Behavior-bearing Commons types have no unit tests
|
### Commons-010 — Behavior-bearing Commons types have no unit tests
|
||||||
|
|
||||||
@@ -393,7 +447,7 @@ _Unresolved._
|
|||||||
|--|--|
|
|--|--|
|
||||||
| Severity | Low |
|
| Severity | Low |
|
||||||
| Category | Testing coverage |
|
| Category | Testing coverage |
|
||||||
| Status | Open |
|
| Status | Resolved |
|
||||||
| Location | `tests/ScadaLink.Commons.Tests/` |
|
| Location | `tests/ScadaLink.Commons.Tests/` |
|
||||||
|
|
||||||
**Description**
|
**Description**
|
||||||
@@ -422,7 +476,16 @@ round-trip.
|
|||||||
|
|
||||||
**Resolution**
|
**Resolution**
|
||||||
|
|
||||||
_Unresolved._
|
Resolved 2026-05-16 (commit pending) — all the listed types now have unit tests.
|
||||||
|
`DynamicJsonElement`, `ManagementCommandRegistry`, `Result<T>` and the OPC UA serializer
|
||||||
|
were already covered by tests added in earlier finding fixes (Commons-002/004) and this
|
||||||
|
pass extended them (Commons-005/006). New focused test files added this pass:
|
||||||
|
`ValueFormatterTests` (scalar/collection/null formatting plus the Commons-012
|
||||||
|
culture-invariance regression), `ScriptArgsTests` (dictionary / read-only-dictionary /
|
||||||
|
non-generic-`IDictionary` / anonymous-object / primitive-rejection paths of
|
||||||
|
`ScriptArgs.Normalize`), and `FlatteningAndScriptScopeTests` (`ConfigurationDiff.HasChanges`
|
||||||
|
and `ScriptScope.HasParent` computed-property logic). The module suite is green at 224
|
||||||
|
tests (up from 196).
|
||||||
|
|
||||||
### Commons-011 — `Result<T>.Failure` accepts a null error string
|
### Commons-011 — `Result<T>.Failure` accepts a null error string
|
||||||
|
|
||||||
@@ -430,7 +493,7 @@ _Unresolved._
|
|||||||
|--|--|
|
|--|--|
|
||||||
| Severity | Low |
|
| Severity | Low |
|
||||||
| Category | Correctness & logic bugs |
|
| Category | Correctness & logic bugs |
|
||||||
| Status | Open |
|
| Status | Resolved |
|
||||||
| Location | `src/ScadaLink.Commons/Types/Result.cs:15-20`, `:30-32`, `:36` |
|
| Location | `src/ScadaLink.Commons/Types/Result.cs:15-20`, `:30-32`, `:36` |
|
||||||
|
|
||||||
**Description**
|
**Description**
|
||||||
@@ -449,7 +512,13 @@ Throw `ArgumentNullException` (or `ArgumentException` for empty/whitespace) in
|
|||||||
|
|
||||||
**Resolution**
|
**Resolution**
|
||||||
|
|
||||||
_Unresolved._
|
Resolved 2026-05-16 (commit pending) — the private failure constructor now calls
|
||||||
|
`ArgumentException.ThrowIfNullOrWhiteSpace(error)`, so a failed `Result<T>` always carries
|
||||||
|
a usable message (`ArgumentNullException` for null, `ArgumentException` for empty/
|
||||||
|
whitespace). The `Failure` factory XML doc records both exceptions. A repo-wide scan of
|
||||||
|
`.Failure(...)` call sites confirmed every caller passes a non-empty literal or
|
||||||
|
interpolated string, so no consumer is broken. Regression tests added in `ResultTests`
|
||||||
|
(`Failure_WithNullError_ShouldThrow`, `Failure_WithBlankError_ShouldThrow`).
|
||||||
|
|
||||||
### Commons-012 — `ValueFormatter` uses current-culture formatting without documenting it
|
### Commons-012 — `ValueFormatter` uses current-culture formatting without documenting it
|
||||||
|
|
||||||
@@ -457,7 +526,7 @@ _Unresolved._
|
|||||||
|--|--|
|
|--|--|
|
||||||
| Severity | Low |
|
| Severity | Low |
|
||||||
| Category | Documentation & comments |
|
| Category | Documentation & comments |
|
||||||
| Status | Open |
|
| Status | Resolved |
|
||||||
| Location | `src/ScadaLink.Commons/Types/ValueFormatter.cs:20-27` |
|
| Location | `src/ScadaLink.Commons/Types/ValueFormatter.cs:20-27` |
|
||||||
|
|
||||||
**Description**
|
**Description**
|
||||||
@@ -479,4 +548,13 @@ overload). Either way, document the culture behavior on the method.
|
|||||||
|
|
||||||
**Resolution**
|
**Resolution**
|
||||||
|
|
||||||
_Unresolved._
|
Resolved 2026-05-16 (commit pending) — confirmed `ValueFormatter` is *not* UI-only: its
|
||||||
|
sole consumer is `StreamRelayActor`, which formats attribute values into outbound gRPC
|
||||||
|
`SiteStreamEvent` messages (a wire context), so culture-dependent output was a latent
|
||||||
|
bug. `FormatDisplayValue` now formats `IFormattable` scalars and collection elements with
|
||||||
|
`IFormattable.ToString(null, CultureInfo.InvariantCulture)`; a new `FormatElement` helper
|
||||||
|
applies the same invariant rule per collection element (previously elements went through
|
||||||
|
the parameterless `ToString()`). The XML doc gained a remarks block stating the
|
||||||
|
culture-invariant contract and why. Regression tests added in `ValueFormatterTests`
|
||||||
|
(`FormatDisplayValue_Double_UsesInvariantCulture_*`, `_DateTime_*`, `_CollectionOfDoubles_*`,
|
||||||
|
each pinned under `de-DE`).
|
||||||
|
|||||||
@@ -8,7 +8,7 @@
|
|||||||
| Last reviewed | 2026-05-16 |
|
| Last reviewed | 2026-05-16 |
|
||||||
| Reviewer | claude-agent |
|
| Reviewer | claude-agent |
|
||||||
| Commit reviewed | `9c60592` |
|
| Commit reviewed | `9c60592` |
|
||||||
| Open findings | 3 |
|
| Open findings | 0 |
|
||||||
|
|
||||||
## Summary
|
## Summary
|
||||||
|
|
||||||
@@ -405,7 +405,7 @@ per-event reset (`Grpc_Error_Resets_RetryCount_On_Successful_Event`) was replace
|
|||||||
|--|--|
|
|--|--|
|
||||||
| Severity | Low |
|
| Severity | Low |
|
||||||
| Category | Concurrency & thread safety |
|
| Category | Concurrency & thread safety |
|
||||||
| Status | Open |
|
| Status | Resolved |
|
||||||
| Location | `src/ScadaLink.Communication/Actors/CentralCommunicationActor.cs:53`, `src/ScadaLink.Communication/Actors/CentralCommunicationActor.cs:240` |
|
| Location | `src/ScadaLink.Communication/Actors/CentralCommunicationActor.cs:53`, `src/ScadaLink.Communication/Actors/CentralCommunicationActor.cs:240` |
|
||||||
|
|
||||||
**Description**
|
**Description**
|
||||||
@@ -427,7 +427,16 @@ malformed site record cannot abort the whole refresh and leave a half-updated ca
|
|||||||
|
|
||||||
**Resolution**
|
**Resolution**
|
||||||
|
|
||||||
_Unresolved._
|
Resolved 2026-05-16 (commit pending). Root cause confirmed: `_siteClients` was a
|
||||||
|
non-`readonly` field, and `HandleSiteAddressCacheLoaded`'s add/update loop called
|
||||||
|
`ActorPath.Parse` per site with no guard — a malformed `NodeAAddress` threw mid-loop and
|
||||||
|
aborted the refresh, leaving the cache half-updated until the next 60s cycle. Fix:
|
||||||
|
`_siteClients` is now `readonly`, and the per-site `ActorPath.Parse` is wrapped in a
|
||||||
|
try/catch that logs the bad address at Warning and `continue`s to the next site, so a
|
||||||
|
single garbage row cannot starve other sites of their ClusterClient. Regression test
|
||||||
|
`CentralCommunicationActorTests.MalformedSiteAddress_DoesNotAbortRefresh_OtherSitesStillRegistered`
|
||||||
|
(bad site ordered before a good one) fails against the pre-fix code (good site never
|
||||||
|
registered) and passes after.
|
||||||
|
|
||||||
### Communication-010 — `DebugStreamBridgeActor` XML doc incorrectly describes it as a "Persistent actor"
|
### Communication-010 — `DebugStreamBridgeActor` XML doc incorrectly describes it as a "Persistent actor"
|
||||||
|
|
||||||
@@ -435,7 +444,7 @@ _Unresolved._
|
|||||||
|--|--|
|
|--|--|
|
||||||
| Severity | Low |
|
| Severity | Low |
|
||||||
| Category | Documentation & comments |
|
| Category | Documentation & comments |
|
||||||
| Status | Open |
|
| Status | Resolved |
|
||||||
| Location | `src/ScadaLink.Communication/Actors/DebugStreamBridgeActor.cs:10` |
|
| Location | `src/ScadaLink.Communication/Actors/DebugStreamBridgeActor.cs:10` |
|
||||||
|
|
||||||
**Description**
|
**Description**
|
||||||
@@ -453,7 +462,13 @@ side..." or similar, removing the word "Persistent".
|
|||||||
|
|
||||||
**Resolution**
|
**Resolution**
|
||||||
|
|
||||||
_Unresolved._
|
Resolved 2026-05-16 (commit pending). Root cause confirmed: the class summary opened
|
||||||
|
with "Persistent actor (one per active debug session)..." but the actor derives from
|
||||||
|
`ReceiveActor`, holds no `PersistenceId`, and writes no journal/snapshot. Fix
|
||||||
|
(documentation only — no behaviour change, so no regression test): the summary was
|
||||||
|
reworded to "Long-lived (one per active debug session) actor on the central side. Debug
|
||||||
|
sessions are session-based and temporary — this actor holds no persisted state and does
|
||||||
|
not derive from an Akka.Persistence base class; its state does not survive a restart."
|
||||||
|
|
||||||
### Communication-011 — No test coverage for snapshot-timeout cleanup, address-cache failure, or gRPC reconnect leak
|
### Communication-011 — No test coverage for snapshot-timeout cleanup, address-cache failure, or gRPC reconnect leak
|
||||||
|
|
||||||
@@ -461,7 +476,7 @@ _Unresolved._
|
|||||||
|--|--|
|
|--|--|
|
||||||
| Severity | Low |
|
| Severity | Low |
|
||||||
| Category | Testing coverage |
|
| Category | Testing coverage |
|
||||||
| Status | Open |
|
| Status | Resolved |
|
||||||
| Location | `tests/ScadaLink.Communication.Tests/` (module-wide) |
|
| Location | `tests/ScadaLink.Communication.Tests/` (module-wide) |
|
||||||
|
|
||||||
**Description**
|
**Description**
|
||||||
@@ -487,4 +502,20 @@ failure logging and empty-cache behaviour; reusing a correlation ID across
|
|||||||
|
|
||||||
**Resolution**
|
**Resolution**
|
||||||
|
|
||||||
_Unresolved._
|
Resolved 2026-05-16 (commit pending). This is a meta-coverage finding; every gap it
|
||||||
|
enumerates is now covered by a regression test (each fails against its pre-fix code and
|
||||||
|
passes after):
|
||||||
|
- Snapshot timeout / pre-snapshot termination (Communication-001) —
|
||||||
|
`DebugStreamServiceTests.StartStreamAsync_StreamTerminatesBeforeSnapshot_ThrowsMeaningfulException`.
|
||||||
|
- gRPC reconnect not unsubscribing the old node (Communication-002) —
|
||||||
|
`DebugStreamBridgeActorTests.On_GrpcError_Unsubscribes_Old_Stream_Before_Reconnect`.
|
||||||
|
- `SiteStreamGrpcClient` subscription-map overwrite/removal race (Communication-003) —
|
||||||
|
`SiteStreamGrpcClientTests.RegisterSubscription_ReusedCorrelationId_CancelsAndDisposesPriorCts`
|
||||||
|
and `RemoveSubscription_OnlyRemovesOwnCts_NotAReplacement`.
|
||||||
|
- `LoadSiteAddressesFromDb` fault (Communication-006) —
|
||||||
|
`CentralCommunicationActorTests.LoadSiteAddressesFailure_IsLoggedNotSilentlySwallowed`.
|
||||||
|
- Malformed `NodeAAddress` aborting `HandleSiteAddressCacheLoaded` (Communication-009) —
|
||||||
|
`CentralCommunicationActorTests.MalformedSiteAddress_DoesNotAbortRefresh_OtherSitesStillRegistered`
|
||||||
|
(added with this finding's resolution).
|
||||||
|
The full module suite (`dotnet test tests/ScadaLink.Communication.Tests`) is green at
|
||||||
|
111 passing tests.
|
||||||
|
|||||||
+8
-32
@@ -42,18 +42,18 @@ module file and counted in **Total**.
|
|||||||
| Critical | 0 |
|
| Critical | 0 |
|
||||||
| High | 0 |
|
| High | 0 |
|
||||||
| Medium | 4 |
|
| Medium | 4 |
|
||||||
| Low | 90 |
|
| Low | 66 |
|
||||||
| **Total** | **94** |
|
| **Total** | **70** |
|
||||||
|
|
||||||
## Module Status
|
## Module Status
|
||||||
|
|
||||||
| Module | Last reviewed | Commit | Open (C/H/M/L) | Open | Total |
|
| Module | Last reviewed | Commit | Open (C/H/M/L) | Open | Total |
|
||||||
|--------|---------------|--------|----------------|------|-------|
|
|--------|---------------|--------|----------------|------|-------|
|
||||||
| [CLI](CLI/findings.md) | 2026-05-16 | `9c60592` | 0/0/0/6 | 6 | 13 |
|
| [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/5 | 7 | 19 |
|
| [CentralUI](CentralUI/findings.md) | 2026-05-16 | `9c60592` | 0/0/2/0 | 2 | 19 |
|
||||||
| [ClusterInfrastructure](ClusterInfrastructure/findings.md) | 2026-05-16 | `9c60592` | 0/0/0/3 | 3 | 8 |
|
| [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/8 | 8 | 12 |
|
| [Commons](Commons/findings.md) | 2026-05-16 | `9c60592` | 0/0/0/1 | 1 | 12 |
|
||||||
| [Communication](Communication/findings.md) | 2026-05-16 | `9c60592` | 0/0/0/3 | 3 | 11 |
|
| [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/6 | 6 | 11 |
|
| [ConfigurationDatabase](ConfigurationDatabase/findings.md) | 2026-05-16 | `9c60592` | 0/0/0/6 | 6 | 11 |
|
||||||
| [DataConnectionLayer](DataConnectionLayer/findings.md) | 2026-05-16 | `9c60592` | 0/0/0/2 | 2 | 13 |
|
| [DataConnectionLayer](DataConnectionLayer/findings.md) | 2026-05-16 | `9c60592` | 0/0/0/2 | 2 | 13 |
|
||||||
| [DeploymentManager](DeploymentManager/findings.md) | 2026-05-16 | `9c60592` | 0/0/0/5 | 5 | 14 |
|
| [DeploymentManager](DeploymentManager/findings.md) | 2026-05-16 | `9c60592` | 0/0/0/5 | 5 | 14 |
|
||||||
@@ -93,35 +93,11 @@ _None open._
|
|||||||
| Host-002 | [Host](Host/findings.md) | Akka.Persistence required by REQ-HOST-6 is not configured and not used |
|
| 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 |
|
| InboundAPI-007 | [InboundAPI](InboundAPI/findings.md) | `Database.Connection()` script API from the design doc is not implemented |
|
||||||
|
|
||||||
### Low (90)
|
### Low (66)
|
||||||
|
|
||||||
| ID | Module | Title |
|
| ID | Module | Title |
|
||||||
|----|--------|-------|
|
|----|--------|-------|
|
||||||
| CLI-008 | [CLI](CLI/findings.md) | `--format` value is not validated |
|
|
||||||
| CLI-009 | [CLI](CLI/findings.md) | Exit-code documentation does not match `HandleResponse` behaviour |
|
|
||||||
| CLI-010 | [CLI](CLI/findings.md) | `debug stream` reports Ctrl+C during connect as a connection failure |
|
|
||||||
| CLI-011 | [CLI](CLI/findings.md) | `CancellationTokenSource` in `debug stream` is never disposed |
|
|
||||||
| CLI-012 | [CLI](CLI/findings.md) | `debug stream` exit code is unreliable after stream termination |
|
|
||||||
| CLI-013 | [CLI](CLI/findings.md) | HTTP client, `debug stream`, and JSON-argument parsing are untested |
|
|
||||||
| CentralUI-015 | [CentralUI](CentralUI/findings.md) | `DialogService` continuations resolve off the render thread |
|
|
||||||
| CentralUI-016 | [CentralUI](CentralUI/findings.md) | Pagers render one button per page with no windowing |
|
|
||||||
| CentralUI-017 | [CentralUI](CentralUI/findings.md) | `/auth/logout` POST disables antiforgery, enabling logout CSRF |
|
|
||||||
| CentralUI-018 | [CentralUI](CentralUI/findings.md) | Broad `catch {}` blocks swallow JS interop and storage errors silently |
|
|
||||||
| CentralUI-019 | [CentralUI](CentralUI/findings.md) | Sparse unit-test coverage for a large module; critical paths untested |
|
|
||||||
| ClusterInfrastructure-005 | [ClusterInfrastructure](ClusterInfrastructure/findings.md) | No configuration section name constant for the Options pattern binding |
|
|
||||||
| ClusterInfrastructure-007 | [ClusterInfrastructure](ClusterInfrastructure/findings.md) | ClusterOptions lacks XML documentation comments |
|
|
||||||
| ClusterInfrastructure-008 | [ClusterInfrastructure](ClusterInfrastructure/findings.md) | "Phase 0 skeleton" status is undocumented at the module level |
|
|
||||||
| Commons-005 | [Commons](Commons/findings.md) | `OpcUaEndpointConfigSerializer.Deserialize` discards malformed legacy input and over-reports `IsLegacy` |
|
|
||||||
| Commons-006 | [Commons](Commons/findings.md) | `DynamicJsonElement.TryConvert` reports success for unconvertible target types |
|
|
||||||
| Commons-007 | [Commons](Commons/findings.md) | Several Commons types carry non-trivial logic, stretching REQ-COM-6 |
|
|
||||||
| Commons-008 | [Commons](Commons/findings.md) | `SetConnectionBindingsCommand` uses `ValueTuple` in a wire message contract |
|
| Commons-008 | [Commons](Commons/findings.md) | `SetConnectionBindingsCommand` uses `ValueTuple` in a wire message contract |
|
||||||
| Commons-009 | [Commons](Commons/findings.md) | `Component-Commons.md` is stale relative to the actual file set |
|
|
||||||
| Commons-010 | [Commons](Commons/findings.md) | Behavior-bearing Commons types have no unit tests |
|
|
||||||
| Commons-011 | [Commons](Commons/findings.md) | `Result<T>.Failure` accepts a null error string |
|
|
||||||
| Commons-012 | [Commons](Commons/findings.md) | `ValueFormatter` uses current-culture formatting without documenting it |
|
|
||||||
| Communication-009 | [Communication](Communication/findings.md) | `_siteClients` field is mutable and reassignable; cache update is not atomic on failure |
|
|
||||||
| Communication-010 | [Communication](Communication/findings.md) | `DebugStreamBridgeActor` XML doc incorrectly describes it as a "Persistent actor" |
|
|
||||||
| Communication-011 | [Communication](Communication/findings.md) | No test coverage for snapshot-timeout cleanup, address-cache failure, or gRPC reconnect leak |
|
|
||||||
| ConfigurationDatabase-005 | [ConfigurationDatabase](ConfigurationDatabase/findings.md) | Audit `Id` type disagrees with the design doc |
|
| ConfigurationDatabase-005 | [ConfigurationDatabase](ConfigurationDatabase/findings.md) | Audit `Id` type disagrees with the design doc |
|
||||||
| ConfigurationDatabase-006 | [ConfigurationDatabase](ConfigurationDatabase/findings.md) | `Site.GrpcNodeAAddress` / `GrpcNodeBAddress` columns are unbounded |
|
| ConfigurationDatabase-006 | [ConfigurationDatabase](ConfigurationDatabase/findings.md) | `Site.GrpcNodeAAddress` / `GrpcNodeBAddress` columns are unbounded |
|
||||||
| ConfigurationDatabase-008 | [ConfigurationDatabase](ConfigurationDatabase/findings.md) | `GetApprovedKeysForMethodAsync` CSV parsing silently drops malformed ids |
|
| ConfigurationDatabase-008 | [ConfigurationDatabase](ConfigurationDatabase/findings.md) | `GetApprovedKeysForMethodAsync` CSV parsing silently drops malformed ids |
|
||||||
|
|||||||
@@ -60,26 +60,28 @@ Commons must define persistence-ignorant POCO entity classes for all configurati
|
|||||||
|
|
||||||
Entity classes are organized by domain area:
|
Entity classes are organized by domain area:
|
||||||
|
|
||||||
- **Template & Modeling**: `Template`, `TemplateAttribute`, `TemplateAlarm`, `TemplateScript`, `TemplateComposition`, `Instance`, `InstanceAttributeOverride`, `InstanceConnectionBinding`, `Area`.
|
- **Template & Modeling**: `Template`, `TemplateAttribute`, `TemplateAlarm`, `TemplateScript`, `TemplateComposition`, `TemplateFolder`.
|
||||||
|
- **Instances**: `Instance`, `InstanceAttributeOverride`, `InstanceConnectionBinding`, `InstanceAlarmOverride`, `Area`.
|
||||||
- **Shared Scripts**: `SharedScript`.
|
- **Shared Scripts**: `SharedScript`.
|
||||||
- **Sites & Data Connections**: `Site`, `DataConnection`, `SiteDataConnectionAssignment`.
|
- **Sites & Data Connections**: `Site`, `DataConnection`.
|
||||||
- **External Systems & Database Connections**: `ExternalSystemDefinition`, `ExternalSystemMethod`, `DatabaseConnectionDefinition`.
|
- **External Systems & Database Connections**: `ExternalSystemDefinition`, `ExternalSystemMethod`, `DatabaseConnectionDefinition`.
|
||||||
- **Notifications**: `NotificationList`, `NotificationRecipient`, `SmtpConfiguration`.
|
- **Notifications**: `NotificationList`, `NotificationRecipient`, `SmtpConfiguration`.
|
||||||
- **Inbound API**: `ApiKey`, `ApiMethod`.
|
- **Inbound API**: `ApiKey`, `ApiMethod`.
|
||||||
- **Security**: `LdapGroupMapping`, `SiteScopeRule`.
|
- **Security**: `LdapGroupMapping`, `SiteScopeRule`.
|
||||||
- **Deployment**: `DeploymentRecord`, `SystemArtifactDeploymentRecord`.
|
- **Deployment**: `DeploymentRecord`, `SystemArtifactDeploymentRecord`, `DeployedConfigSnapshot`.
|
||||||
- **Audit**: `AuditLogEntry`.
|
- **Audit**: `AuditLogEntry`.
|
||||||
|
|
||||||
### REQ-COM-4: Per-Component Repository Interfaces
|
### REQ-COM-4: Per-Component Repository Interfaces
|
||||||
|
|
||||||
Commons must define repository interfaces that consuming components use for data access. Each interface is tailored to the data needs of its consuming component:
|
Commons must define repository interfaces that consuming components use for data access. Each interface is tailored to the data needs of its consuming component:
|
||||||
|
|
||||||
- `ITemplateEngineRepository` — Templates, attributes, alarms, scripts, compositions, instances, overrides, connection bindings, areas.
|
- `ITemplateEngineRepository` — Templates, attributes, alarms, scripts, compositions, template folders, instances, overrides, alarm overrides, connection bindings, areas.
|
||||||
- `IDeploymentManagerRepository` — Deployment records, deployed configuration snapshots, system-wide artifact deployment records.
|
- `IDeploymentManagerRepository` — Deployment records, deployed configuration snapshots, system-wide artifact deployment records.
|
||||||
- `ISecurityRepository` — LDAP group mappings, site scoping rules.
|
- `ISecurityRepository` — LDAP group mappings, site scoping rules.
|
||||||
- `IInboundApiRepository` — API keys, API method definitions.
|
- `IInboundApiRepository` — API keys, API method definitions.
|
||||||
- `IExternalSystemRepository` — External system definitions, method definitions, database connection definitions.
|
- `IExternalSystemRepository` — External system definitions, method definitions, database connection definitions.
|
||||||
- `INotificationRepository` — Notification lists, recipients, SMTP configuration.
|
- `INotificationRepository` — Notification lists, recipients, SMTP configuration.
|
||||||
|
- `ISiteRepository` — Sites, data connections, and their site assignments.
|
||||||
- `ICentralUiRepository` — Read-oriented queries spanning multiple domain areas for display purposes.
|
- `ICentralUiRepository` — Read-oriented queries spanning multiple domain areas for display purposes.
|
||||||
|
|
||||||
All repository interfaces must:
|
All repository interfaces must:
|
||||||
@@ -93,7 +95,13 @@ Implementations of these interfaces are owned by the Configuration Database comp
|
|||||||
|
|
||||||
Commons must define service interfaces for cross-cutting concerns that multiple components consume:
|
Commons must define service interfaces for cross-cutting concerns that multiple components consume:
|
||||||
|
|
||||||
- **`IAuditService`**: Provides a single method for components to log audit entries: `LogAsync(user, action, entityType, entityId, entityName, afterState)`. The implementation (owned by the Audit Logging component) serializes the state as JSON and adds the audit entry to the current unit-of-work transaction. Defined in Commons so any central component can call it without depending on the Audit Logging component directly.
|
- **`IAuditService`**: Provides a single method for components to log audit entries: `LogAsync(user, action, entityType, entityId, entityName, afterState)`. The implementation (owned by the Configuration Database component) serializes the state as JSON and adds the audit entry to the current unit-of-work transaction. Defined in Commons so any central component can call it without depending on the Configuration Database component directly.
|
||||||
|
- **`IDatabaseGateway`**: Provides script-facing ADO.NET database access via named database connections. Implemented by the External System Gateway, consumed by the Site Runtime's script runtime context.
|
||||||
|
- **`IExternalSystemClient`**: Provides script-facing invocation of external system HTTP APIs (synchronous `Call` and store-and-forward `CachedCall`). Implemented by the External System Gateway, consumed by the script runtime context.
|
||||||
|
- **`IInstanceLocator`**: Resolves an instance unique name to its site identifier. Used by the Inbound API's `Route.To()` to determine the destination site.
|
||||||
|
- **`INotificationDeliveryService`**: Sends notifications to a named notification list, routing transient failures to store-and-forward. Implemented by the Notification Service, consumed by the script runtime context.
|
||||||
|
|
||||||
|
These interfaces are defined in Commons so that consuming components depend only on the abstraction, not on the implementing component.
|
||||||
|
|
||||||
### REQ-COM-5: Cross-Component Message Contracts
|
### REQ-COM-5: Cross-Component Message Contracts
|
||||||
|
|
||||||
@@ -126,20 +134,23 @@ All types in Commons are organized by **category** and **domain area** using a c
|
|||||||
```
|
```
|
||||||
ScadaLink.Commons/
|
ScadaLink.Commons/
|
||||||
├── Types/ # REQ-COM-1: Shared data types
|
├── Types/ # REQ-COM-1: Shared data types
|
||||||
│ ├── DataType.cs
|
|
||||||
│ ├── RetryPolicy.cs
|
|
||||||
│ ├── Result.cs
|
│ ├── Result.cs
|
||||||
│ └── Enums/
|
│ ├── RetryPolicy.cs
|
||||||
│ ├── InstanceState.cs
|
│ ├── ScriptArgs.cs # script-call parameter normalization helper
|
||||||
│ ├── DeploymentStatus.cs
|
│ ├── ScriptParameters.cs # typed script-parameter access helper
|
||||||
│ ├── AlarmState.cs
|
│ ├── StaleTagMonitor.cs # heartbeat staleness watchdog
|
||||||
│ ├── AlarmTriggerType.cs
|
│ ├── ValueFormatter.cs # culture-invariant value-to-string helper
|
||||||
│ └── ConnectionHealth.cs
|
│ ├── DynamicJsonElement.cs # dynamic JSON wrapper for scripts
|
||||||
|
│ ├── Enums/ # InstanceState, DeploymentStatus, AlarmState,
|
||||||
|
│ │ # AlarmLevel, AlarmTriggerType, ConnectionHealth,
|
||||||
|
│ │ # DataType, StoreAndForwardCategory,
|
||||||
|
│ │ # StoreAndForwardMessageStatus
|
||||||
|
│ ├── DataConnections/ # OPC UA endpoint config value objects + enums
|
||||||
|
│ ├── Flattening/ # FlattenedConfiguration, ConfigurationDiff,
|
||||||
|
│ │ # DeploymentPackage, ValidationResult
|
||||||
|
│ └── Scripts/ # AlarmContext, ScriptScope
|
||||||
├── Interfaces/ # Shared interfaces by concern
|
├── Interfaces/ # Shared interfaces by concern
|
||||||
│ ├── Protocol/ # REQ-COM-2: Protocol abstraction
|
│ ├── Protocol/ # REQ-COM-2: Protocol abstraction (IDataConnection, etc.)
|
||||||
│ │ ├── IDataConnection.cs
|
|
||||||
│ │ ├── TagValue.cs
|
|
||||||
│ │ └── SubscriptionCallback.cs
|
|
||||||
│ ├── Repositories/ # REQ-COM-4: Per-component repository interfaces
|
│ ├── Repositories/ # REQ-COM-4: Per-component repository interfaces
|
||||||
│ │ ├── ITemplateEngineRepository.cs
|
│ │ ├── ITemplateEngineRepository.cs
|
||||||
│ │ ├── IDeploymentManagerRepository.cs
|
│ │ ├── IDeploymentManagerRepository.cs
|
||||||
@@ -147,55 +158,46 @@ ScadaLink.Commons/
|
|||||||
│ │ ├── IInboundApiRepository.cs
|
│ │ ├── IInboundApiRepository.cs
|
||||||
│ │ ├── IExternalSystemRepository.cs
|
│ │ ├── IExternalSystemRepository.cs
|
||||||
│ │ ├── INotificationRepository.cs
|
│ │ ├── INotificationRepository.cs
|
||||||
|
│ │ ├── ISiteRepository.cs
|
||||||
│ │ └── ICentralUiRepository.cs
|
│ │ └── ICentralUiRepository.cs
|
||||||
│ └── Services/ # REQ-COM-4a: Cross-cutting service interfaces
|
│ └── Services/ # REQ-COM-4a: Cross-cutting service interfaces
|
||||||
│ └── IAuditService.cs
|
│ ├── IAuditService.cs
|
||||||
|
│ ├── IDatabaseGateway.cs
|
||||||
|
│ ├── IExternalSystemClient.cs
|
||||||
|
│ ├── IInstanceLocator.cs
|
||||||
|
│ └── INotificationDeliveryService.cs
|
||||||
├── Entities/ # REQ-COM-3: Domain entity POCOs, by domain area
|
├── Entities/ # REQ-COM-3: Domain entity POCOs, by domain area
|
||||||
│ ├── Templates/
|
│ ├── Templates/ # Template, TemplateAttribute, TemplateAlarm,
|
||||||
│ │ ├── Template.cs
|
│ │ # TemplateScript, TemplateComposition, TemplateFolder
|
||||||
│ │ ├── TemplateAttribute.cs
|
│ ├── Instances/ # Instance, InstanceAttributeOverride,
|
||||||
│ │ ├── TemplateAlarm.cs
|
│ │ # InstanceConnectionBinding, InstanceAlarmOverride, Area
|
||||||
│ │ ├── TemplateScript.cs
|
│ ├── Sites/ # Site, DataConnection
|
||||||
│ │ └── TemplateComposition.cs
|
│ ├── ExternalSystems/ # ExternalSystemDefinition, ExternalSystemMethod,
|
||||||
│ ├── Instances/
|
│ │ # DatabaseConnectionDefinition
|
||||||
│ │ ├── Instance.cs
|
│ ├── Notifications/ # NotificationList, NotificationRecipient, SmtpConfiguration
|
||||||
│ │ ├── InstanceAttributeOverride.cs
|
│ ├── InboundApi/ # ApiKey, ApiMethod
|
||||||
│ │ ├── InstanceConnectionBinding.cs
|
│ ├── Security/ # LdapGroupMapping, SiteScopeRule
|
||||||
│ │ └── Area.cs
|
│ ├── Deployment/ # DeploymentRecord, SystemArtifactDeploymentRecord,
|
||||||
│ ├── Sites/
|
│ │ # DeployedConfigSnapshot
|
||||||
│ │ ├── Site.cs
|
│ ├── Scripts/ # SharedScript
|
||||||
│ │ ├── DataConnection.cs
|
│ └── Audit/ # AuditLogEntry
|
||||||
│ │ └── SiteDataConnectionAssignment.cs
|
├── Messages/ # REQ-COM-5: Cross-component message contracts, by concern
|
||||||
│ ├── ExternalSystems/
|
|
||||||
│ │ ├── ExternalSystemDefinition.cs
|
|
||||||
│ │ ├── ExternalSystemMethod.cs
|
|
||||||
│ │ └── DatabaseConnectionDefinition.cs
|
|
||||||
│ ├── Notifications/
|
|
||||||
│ │ ├── NotificationList.cs
|
|
||||||
│ │ ├── NotificationRecipient.cs
|
|
||||||
│ │ └── SmtpConfiguration.cs
|
|
||||||
│ ├── InboundApi/
|
|
||||||
│ │ ├── ApiKey.cs
|
|
||||||
│ │ └── ApiMethod.cs
|
|
||||||
│ ├── Security/
|
|
||||||
│ │ ├── LdapGroupMapping.cs
|
|
||||||
│ │ └── SiteScopeRule.cs
|
|
||||||
│ ├── Deployment/
|
│ ├── Deployment/
|
||||||
│ │ ├── DeploymentRecord.cs
|
│ ├── Lifecycle/
|
||||||
│ │ └── SystemArtifactDeploymentRecord.cs
|
│ ├── Health/
|
||||||
│ ├── Scripts/
|
│ ├── Communication/
|
||||||
│ │ └── SharedScript.cs
|
│ ├── Streaming/
|
||||||
│ └── Audit/
|
│ ├── DebugView/
|
||||||
│ └── AuditLogEntry.cs
|
│ ├── ScriptExecution/
|
||||||
└── Messages/ # REQ-COM-5: Cross-component message contracts, by concern
|
│ ├── Artifacts/
|
||||||
├── Deployment/
|
│ ├── DataConnection/ # data-connection subscribe/write/health messages
|
||||||
├── Lifecycle/
|
│ ├── Instance/ # attribute get/set request/command messages
|
||||||
├── Health/
|
│ ├── Integration/ # external-integration call request/response
|
||||||
├── Communication/
|
│ ├── InboundApi/ # Route.To() request messages
|
||||||
├── Streaming/
|
│ ├── RemoteQuery/ # event-log and parked-message query messages
|
||||||
├── DebugView/
|
│ └── Management/ # HTTP/ClusterClient management commands + registry
|
||||||
├── ScriptExecution/
|
├── Serialization/ # OpcUaEndpointConfigSerializer (typed↔legacy JSON)
|
||||||
└── Artifacts/
|
└── Validators/ # OpcUaEndpointConfigValidator
|
||||||
```
|
```
|
||||||
|
|
||||||
**Naming rules**:
|
**Naming rules**:
|
||||||
@@ -205,7 +207,7 @@ ScadaLink.Commons/
|
|||||||
- Message contracts are named as commands, events, or responses: `DeployInstanceCommand`, `DeploymentStatusResponse`, `AttributeValueChanged`.
|
- Message contracts are named as commands, events, or responses: `DeployInstanceCommand`, `DeploymentStatusResponse`, `AttributeValueChanged`.
|
||||||
- Enums use singular names: `AlarmState`, not `AlarmStates`.
|
- Enums use singular names: `AlarmState`, not `AlarmStates`.
|
||||||
|
|
||||||
### REQ-COM-6: No Business Logic
|
### REQ-COM-6: No Business Logic; Pure Helpers Permitted
|
||||||
|
|
||||||
Commons must contain only:
|
Commons must contain only:
|
||||||
|
|
||||||
@@ -213,8 +215,17 @@ Commons must contain only:
|
|||||||
- Interfaces
|
- Interfaces
|
||||||
- Enums
|
- Enums
|
||||||
- Constants
|
- Constants
|
||||||
|
- **Pure, stateless helpers** — see the carve-out below.
|
||||||
|
|
||||||
It must **not** contain any business logic, service implementations, actor definitions, or orchestration code. Any method bodies must be limited to trivial data-access logic (e.g., factory methods, validation of invariants in constructors).
|
It must **not** contain any business logic that orchestrates other components, service implementations that perform I/O (database, network, file system), actor definitions, or orchestration code.
|
||||||
|
|
||||||
|
**Pure-helper carve-out.** Commons *may* contain stateless, side-effect-free helper types whose behavior is confined to transforming, formatting, parsing, or validating the data types Commons already defines, provided they:
|
||||||
|
|
||||||
|
- have no dependency on Akka.NET, ASP.NET Core, EF Core, or any I/O surface (consistent with REQ-COM-7);
|
||||||
|
- hold no shared mutable state across calls (a self-contained instance helper such as `StaleTagMonitor`, which owns only its own timer, is acceptable);
|
||||||
|
- do not call into other components or perform orchestration.
|
||||||
|
|
||||||
|
Examples currently in Commons that fall under this carve-out: `Result<T>`, `ScriptParameters` and `ScriptArgs` (script-parameter shaping), `ValueFormatter` (value-to-string formatting), `DynamicJsonElement` (dynamic JSON access), `StaleTagMonitor` (a self-contained heartbeat watchdog), `OpcUaEndpointConfigSerializer` (typed↔legacy JSON conversion of a Commons value object) and `OpcUaEndpointConfigValidator` (rule checks over a Commons value object). These are intentionally placed in Commons so every consuming component shares one implementation rather than duplicating the logic. Anything that would require an I/O dependency, mutable cross-call state, or knowledge of another component's behavior does **not** qualify and must live in the owning component.
|
||||||
|
|
||||||
### REQ-COM-7: Minimal Dependencies
|
### REQ-COM-7: Minimal Dependencies
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,30 @@
|
|||||||
|
using System.CommandLine;
|
||||||
|
|
||||||
|
namespace ScadaLink.CLI.Commands;
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Factory methods for the global CLI options. Centralising option construction keeps
|
||||||
|
/// validation rules (e.g. the accepted <c>--format</c> values) in one place and makes
|
||||||
|
/// them testable without standing up the whole command tree.
|
||||||
|
/// </summary>
|
||||||
|
internal static class CliOptions
|
||||||
|
{
|
||||||
|
/// <summary>
|
||||||
|
/// Creates the global <c>--format</c> option. The option deliberately has no
|
||||||
|
/// <c>DefaultValueFactory</c> — format precedence (explicit flag → config/env →
|
||||||
|
/// <c>"json"</c>) is resolved by <see cref="CommandHelpers.ResolveFormat"/>, which
|
||||||
|
/// needs to distinguish an absent flag. The accepted values are constrained so a
|
||||||
|
/// typo (e.g. <c>--format tabel</c>) is rejected with a clear parse error rather
|
||||||
|
/// than silently falling through to JSON.
|
||||||
|
/// </summary>
|
||||||
|
internal static Option<string> CreateFormatOption()
|
||||||
|
{
|
||||||
|
var formatOption = new Option<string>("--format")
|
||||||
|
{
|
||||||
|
Description = "Output format (json or table)",
|
||||||
|
Recursive = true,
|
||||||
|
};
|
||||||
|
formatOption.AcceptOnlyFromAmong("json", "table");
|
||||||
|
return formatOption;
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -132,7 +132,24 @@ internal static class CommandHelpers
|
|||||||
var error = response.Error ?? "Unknown error";
|
var error = response.Error ?? "Unknown error";
|
||||||
|
|
||||||
OutputFormatter.WriteError(error, errorCode);
|
OutputFormatter.WriteError(error, errorCode);
|
||||||
return response.StatusCode == 403 ? 2 : 1;
|
return IsAuthorizationFailure(response) ? 2 : 1;
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Determines whether an error response represents an authorization failure
|
||||||
|
/// (insufficient role), which the documented exit-code table maps to exit code 2.
|
||||||
|
/// An HTTP 403 status is the primary signal; the server may also signal it via an
|
||||||
|
/// <c>UNAUTHORIZED</c> / <c>FORBIDDEN</c> error code on a different HTTP status, so
|
||||||
|
/// both channels are honoured. (Authentication failure — HTTP 401 / bad credentials
|
||||||
|
/// — is deliberately <em>not</em> treated as authorization failure; it is exit 1.)
|
||||||
|
/// </summary>
|
||||||
|
private static bool IsAuthorizationFailure(ManagementResponse response)
|
||||||
|
{
|
||||||
|
if (response.StatusCode == 403)
|
||||||
|
return true;
|
||||||
|
|
||||||
|
return string.Equals(response.ErrorCode, "FORBIDDEN", StringComparison.OrdinalIgnoreCase)
|
||||||
|
|| string.Equals(response.ErrorCode, "UNAUTHORIZED", StringComparison.OrdinalIgnoreCase);
|
||||||
}
|
}
|
||||||
|
|
||||||
private static void WriteAsTable(string json)
|
private static void WriteAsTable(string json)
|
||||||
|
|||||||
@@ -94,7 +94,8 @@ public static class DebugCommands
|
|||||||
.WithAutomaticReconnect()
|
.WithAutomaticReconnect()
|
||||||
.Build();
|
.Build();
|
||||||
|
|
||||||
var cts = new CancellationTokenSource();
|
// CLI-011: CancellationTokenSource owns a WaitHandle and must be disposed.
|
||||||
|
using var cts = new CancellationTokenSource();
|
||||||
var exitTcs = new TaskCompletionSource<int>(TaskCreationOptions.RunContinuationsAsynchronously);
|
var exitTcs = new TaskCompletionSource<int>(TaskCreationOptions.RunContinuationsAsynchronously);
|
||||||
|
|
||||||
Console.CancelKeyPress += (_, e) =>
|
Console.CancelKeyPress += (_, e) =>
|
||||||
@@ -192,8 +193,18 @@ public static class DebugCommands
|
|||||||
}
|
}
|
||||||
catch (Exception ex)
|
catch (Exception ex)
|
||||||
{
|
{
|
||||||
|
// CLI-010: Ctrl+C during connect throws OperationCanceledException — that is
|
||||||
|
// a graceful user cancellation, not a connection failure.
|
||||||
|
var failure = DebugStreamHelpers.ClassifyConnectFailure(ex, cts.IsCancellationRequested);
|
||||||
|
if (failure.IsCancellation)
|
||||||
|
{
|
||||||
|
await connection.DisposeAsync();
|
||||||
|
return failure.ExitCode;
|
||||||
|
}
|
||||||
|
|
||||||
OutputFormatter.WriteError($"Connection failed: {ex.Message}", "CONNECTION_FAILED");
|
OutputFormatter.WriteError($"Connection failed: {ex.Message}", "CONNECTION_FAILED");
|
||||||
return 1;
|
await connection.DisposeAsync();
|
||||||
|
return failure.ExitCode;
|
||||||
}
|
}
|
||||||
|
|
||||||
try
|
try
|
||||||
@@ -232,7 +243,11 @@ public static class DebugCommands
|
|||||||
}
|
}
|
||||||
|
|
||||||
await connection.DisposeAsync();
|
await connection.DisposeAsync();
|
||||||
return exitTcs.Task.IsCompletedSuccessfully ? exitTcs.Task.Result : 0;
|
|
||||||
|
// CLI-012: resolve the exit code from a single authoritative source. A result
|
||||||
|
// set by OnStreamTerminated/Closed always wins; a brief grace period covers a
|
||||||
|
// termination racing with Ctrl+C. Pure Ctrl+C (no result) is a graceful exit 0.
|
||||||
|
return await DebugStreamHelpers.ResolveStreamExitCodeAsync(exitTcs.Task);
|
||||||
}
|
}
|
||||||
|
|
||||||
private static void PrintSnapshotTable(JsonElement snapshot)
|
private static void PrintSnapshotTable(JsonElement snapshot)
|
||||||
|
|||||||
@@ -0,0 +1,54 @@
|
|||||||
|
namespace ScadaLink.CLI.Commands;
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Pure, testable helpers for the <c>debug stream</c> command. The SignalR-driven
|
||||||
|
/// <see cref="DebugCommands"/> body itself cannot be unit-tested without a live hub, so
|
||||||
|
/// the decision logic — connect-failure classification (CLI-010) and exit-code
|
||||||
|
/// resolution after stream termination (CLI-012) — is extracted here.
|
||||||
|
/// </summary>
|
||||||
|
internal static class DebugStreamHelpers
|
||||||
|
{
|
||||||
|
/// <summary>
|
||||||
|
/// The maximum time <see cref="ResolveStreamExitCodeAsync"/> waits for an in-flight
|
||||||
|
/// <c>TrySetResult</c> (from <c>OnStreamTerminated</c>/<c>Closed</c>) to land after
|
||||||
|
/// the wait was cancelled by Ctrl+C, so a termination racing with cancellation is
|
||||||
|
/// observed deterministically rather than depending on scheduling.
|
||||||
|
/// </summary>
|
||||||
|
internal static readonly TimeSpan ExitGracePeriod = TimeSpan.FromMilliseconds(250);
|
||||||
|
|
||||||
|
/// <summary>Outcome of classifying an exception thrown while connecting.</summary>
|
||||||
|
internal readonly record struct ConnectFailure(bool IsCancellation, int ExitCode);
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Classifies an exception thrown by <c>HubConnection.StartAsync</c>. A
|
||||||
|
/// cancellation exception that coincides with a user-requested cancellation
|
||||||
|
/// (Ctrl+C during connect) is a graceful shutdown — exit 0, no error printed.
|
||||||
|
/// Anything else is a genuine connection failure — exit 1.
|
||||||
|
/// </summary>
|
||||||
|
internal static ConnectFailure ClassifyConnectFailure(Exception ex, bool cancellationRequested)
|
||||||
|
{
|
||||||
|
if (cancellationRequested && ex is OperationCanceledException)
|
||||||
|
return new ConnectFailure(IsCancellation: true, ExitCode: 0);
|
||||||
|
|
||||||
|
return new ConnectFailure(IsCancellation: false, ExitCode: 1);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Resolves the <c>debug stream</c> exit code from a single authoritative source —
|
||||||
|
/// the <c>exitTcs</c> task. If a result was set by <c>OnStreamTerminated</c> or the
|
||||||
|
/// <c>Closed</c> handler it is always preferred (even when Ctrl+C also fired);
|
||||||
|
/// a brief grace period covers a termination that races with cancellation. If no
|
||||||
|
/// result is ever produced (pure Ctrl+C), the stream ended gracefully — exit 0.
|
||||||
|
/// </summary>
|
||||||
|
internal static async Task<int> ResolveStreamExitCodeAsync(Task<int> exitTask)
|
||||||
|
{
|
||||||
|
if (exitTask.IsCompletedSuccessfully)
|
||||||
|
return exitTask.Result;
|
||||||
|
|
||||||
|
var completed = await Task.WhenAny(exitTask, Task.Delay(ExitGracePeriod));
|
||||||
|
if (ReferenceEquals(completed, exitTask) && exitTask.IsCompletedSuccessfully)
|
||||||
|
return exitTask.Result;
|
||||||
|
|
||||||
|
return 0;
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -9,8 +9,19 @@ public class ManagementHttpClient : IDisposable
|
|||||||
private readonly HttpClient _httpClient;
|
private readonly HttpClient _httpClient;
|
||||||
|
|
||||||
public ManagementHttpClient(string baseUrl, string username, string password)
|
public ManagementHttpClient(string baseUrl, string username, string password)
|
||||||
|
: this(new HttpClient(), baseUrl, username, password)
|
||||||
{
|
{
|
||||||
_httpClient = new HttpClient { BaseAddress = new Uri(baseUrl.TrimEnd('/') + "/") };
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Test-only constructor that accepts a pre-built <see cref="HttpClient"/> (typically
|
||||||
|
/// over a stub <see cref="HttpMessageHandler"/>) so the request/response handling can
|
||||||
|
/// be exercised without a live server.
|
||||||
|
/// </summary>
|
||||||
|
internal ManagementHttpClient(HttpClient httpClient, string baseUrl, string username, string password)
|
||||||
|
{
|
||||||
|
_httpClient = httpClient;
|
||||||
|
_httpClient.BaseAddress = new Uri(baseUrl.TrimEnd('/') + "/");
|
||||||
var credentials = Convert.ToBase64String(Encoding.UTF8.GetBytes($"{username}:{password}"));
|
var credentials = Convert.ToBase64String(Encoding.UTF8.GetBytes($"{username}:{password}"));
|
||||||
_httpClient.DefaultRequestHeaders.Authorization =
|
_httpClient.DefaultRequestHeaders.Authorization =
|
||||||
new AuthenticationHeaderValue("Basic", credentials);
|
new AuthenticationHeaderValue("Basic", credentials);
|
||||||
|
|||||||
@@ -9,7 +9,8 @@ var usernameOption = new Option<string>("--username") { Description = "LDAP user
|
|||||||
var passwordOption = new Option<string>("--password") { Description = "LDAP password", Recursive = true };
|
var passwordOption = new Option<string>("--password") { Description = "LDAP password", Recursive = true };
|
||||||
// No DefaultValueFactory: format precedence (explicit --format -> config/env -> "json")
|
// No DefaultValueFactory: format precedence (explicit --format -> config/env -> "json")
|
||||||
// is resolved by CommandHelpers.ResolveFormat, which needs to distinguish an absent flag.
|
// is resolved by CommandHelpers.ResolveFormat, which needs to distinguish an absent flag.
|
||||||
var formatOption = new Option<string>("--format") { Description = "Output format (json or table)", Recursive = true };
|
// CliOptions.CreateFormatOption also constrains the accepted values (json/table).
|
||||||
|
var formatOption = CliOptions.CreateFormatOption();
|
||||||
|
|
||||||
rootCommand.Add(urlOption);
|
rootCommand.Add(urlOption);
|
||||||
rootCommand.Add(usernameOption);
|
rootCommand.Add(usernameOption);
|
||||||
|
|||||||
@@ -124,17 +124,16 @@ public static class AuthEndpoints
|
|||||||
});
|
});
|
||||||
}).DisableAntiforgery();
|
}).DisableAntiforgery();
|
||||||
|
|
||||||
|
// Logout is a state-changing authenticated action (CentralUI-017): it
|
||||||
|
// keeps antiforgery validation enabled so it cannot be triggered
|
||||||
|
// cross-site. The NavMenu sign-out form includes the antiforgery token
|
||||||
|
// (rendered by the <AntiforgeryToken /> component). There is deliberately
|
||||||
|
// no GET /logout route — a state-changing GET is itself a CSRF vector
|
||||||
|
// (an <img src="/logout"> would forcibly log a user out).
|
||||||
endpoints.MapPost("/auth/logout", async (HttpContext context) =>
|
endpoints.MapPost("/auth/logout", async (HttpContext context) =>
|
||||||
{
|
{
|
||||||
await context.SignOutAsync(CookieAuthenticationDefaults.AuthenticationScheme);
|
await context.SignOutAsync(CookieAuthenticationDefaults.AuthenticationScheme);
|
||||||
context.Response.Redirect("/login");
|
context.Response.Redirect("/login");
|
||||||
}).DisableAntiforgery();
|
|
||||||
|
|
||||||
// GET /logout — allows direct navigation to logout (redirects to login after sign-out)
|
|
||||||
endpoints.MapGet("/logout", async (HttpContext context) =>
|
|
||||||
{
|
|
||||||
await context.SignOutAsync(CookieAuthenticationDefaults.AuthenticationScheme);
|
|
||||||
return Results.Redirect("/login");
|
|
||||||
});
|
});
|
||||||
|
|
||||||
return endpoints;
|
return endpoints;
|
||||||
|
|||||||
@@ -101,6 +101,9 @@
|
|||||||
<div class="d-flex justify-content-between align-items-center">
|
<div class="d-flex justify-content-between align-items-center">
|
||||||
<span class="text-light small">@context.User.FindFirst("DisplayName")?.Value</span>
|
<span class="text-light small">@context.User.FindFirst("DisplayName")?.Value</span>
|
||||||
<form method="post" action="/auth/logout" data-enhance="false">
|
<form method="post" action="/auth/logout" data-enhance="false">
|
||||||
|
@* CentralUI-017: logout is a state-changing POST and is
|
||||||
|
CSRF-protected — the antiforgery token is required. *@
|
||||||
|
<AntiforgeryToken />
|
||||||
<button type="submit" class="btn btn-outline-light btn-sm py-0 px-2">Sign Out</button>
|
<button type="submit" class="btn btn-outline-light btn-sm py-0 px-2">Sign Out</button>
|
||||||
</form>
|
</form>
|
||||||
</div>
|
</div>
|
||||||
|
|||||||
@@ -13,6 +13,7 @@
|
|||||||
@inject NavigationManager NavigationManager
|
@inject NavigationManager NavigationManager
|
||||||
@inject IJSRuntime JS
|
@inject IJSRuntime JS
|
||||||
@inject IDialogService Dialog
|
@inject IDialogService Dialog
|
||||||
|
@inject Microsoft.Extensions.Logging.ILogger<Sites> Logger
|
||||||
|
|
||||||
<div class="container-fluid mt-3">
|
<div class="container-fluid mt-3">
|
||||||
<div class="d-flex justify-content-between align-items-center mb-3">
|
<div class="d-flex justify-content-between align-items-center mb-3">
|
||||||
@@ -310,8 +311,15 @@
|
|||||||
await JS.InvokeVoidAsync("navigator.clipboard.writeText", text);
|
await JS.InvokeVoidAsync("navigator.clipboard.writeText", text);
|
||||||
_toast.ShowSuccess("Copied to clipboard.");
|
_toast.ShowSuccess("Copied to clipboard.");
|
||||||
}
|
}
|
||||||
catch
|
catch (Microsoft.JSInterop.JSDisconnectedException)
|
||||||
{
|
{
|
||||||
|
// Circuit gone — the user has navigated away; nothing to surface.
|
||||||
|
}
|
||||||
|
catch (Microsoft.JSInterop.JSException ex)
|
||||||
|
{
|
||||||
|
// CentralUI-018: a real clipboard failure (e.g. permission denied)
|
||||||
|
// is logged, not silently swallowed.
|
||||||
|
Logger.LogWarning(ex, "Clipboard copy failed.");
|
||||||
_toast.ShowError("Copy failed.");
|
_toast.ShowError("Copy failed.");
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -165,12 +165,21 @@
|
|||||||
<li class="page-item @(_currentPage <= 1 ? "disabled" : "")">
|
<li class="page-item @(_currentPage <= 1 ? "disabled" : "")">
|
||||||
<button class="page-link" @onclick="() => GoToPage(_currentPage - 1)">Previous</button>
|
<button class="page-link" @onclick="() => GoToPage(_currentPage - 1)">Previous</button>
|
||||||
</li>
|
</li>
|
||||||
@for (int i = 1; i <= _totalPages; i++)
|
@foreach (var page in ScadaLink.CentralUI.Components.Shared.PagerWindow.Build(_currentPage, _totalPages))
|
||||||
{
|
{
|
||||||
var page = i;
|
if (page == 0)
|
||||||
<li class="page-item @(page == _currentPage ? "active" : "")">
|
{
|
||||||
<button class="page-link" @onclick="() => GoToPage(page)">@(page)</button>
|
<li class="page-item disabled">
|
||||||
</li>
|
<span class="page-link">…</span>
|
||||||
|
</li>
|
||||||
|
}
|
||||||
|
else
|
||||||
|
{
|
||||||
|
var p = page;
|
||||||
|
<li class="page-item @(p == _currentPage ? "active" : "")">
|
||||||
|
<button class="page-link" @onclick="() => GoToPage(p)">@(p)</button>
|
||||||
|
</li>
|
||||||
|
}
|
||||||
}
|
}
|
||||||
<li class="page-item @(_currentPage >= _totalPages ? "disabled" : "")">
|
<li class="page-item @(_currentPage >= _totalPages ? "disabled" : "")">
|
||||||
<button class="page-link" @onclick="() => GoToPage(_currentPage + 1)">Next</button>
|
<button class="page-link" @onclick="() => GoToPage(_currentPage + 1)">Next</button>
|
||||||
|
|||||||
@@ -59,12 +59,21 @@
|
|||||||
aria-disabled="@((_currentPage <= 1).ToString().ToLowerInvariant())"
|
aria-disabled="@((_currentPage <= 1).ToString().ToLowerInvariant())"
|
||||||
@onclick="() => GoToPage(_currentPage - 1)">Previous</button>
|
@onclick="() => GoToPage(_currentPage - 1)">Previous</button>
|
||||||
</li>
|
</li>
|
||||||
@for (int i = 1; i <= _totalPages; i++)
|
@foreach (var page in PagerWindow.Build(_currentPage, _totalPages))
|
||||||
{
|
{
|
||||||
var page = i;
|
if (page == 0)
|
||||||
<li class="page-item @(page == _currentPage ? "active" : "")">
|
{
|
||||||
<button class="page-link" @onclick="() => GoToPage(page)">@(page)</button>
|
<li class="page-item disabled">
|
||||||
</li>
|
<span class="page-link">…</span>
|
||||||
|
</li>
|
||||||
|
}
|
||||||
|
else
|
||||||
|
{
|
||||||
|
var p = page;
|
||||||
|
<li class="page-item @(p == _currentPage ? "active" : "")">
|
||||||
|
<button class="page-link" @onclick="() => GoToPage(p)">@(p)</button>
|
||||||
|
</li>
|
||||||
|
}
|
||||||
}
|
}
|
||||||
<li class="page-item @(_currentPage >= _totalPages ? "disabled" : "")">
|
<li class="page-item @(_currentPage >= _totalPages ? "disabled" : "")">
|
||||||
<button class="page-link" type="button"
|
<button class="page-link" type="button"
|
||||||
|
|||||||
@@ -23,6 +23,13 @@ public class DialogService : IDialogService
|
|||||||
/// </summary>
|
/// </summary>
|
||||||
public DialogState? Current { get; private set; }
|
public DialogState? Current { get; private set; }
|
||||||
|
|
||||||
|
// CentralUI-015: the pending dialog result is held in a typed TCS that the
|
||||||
|
// host completes directly via Resolve(). The previous implementation
|
||||||
|
// projected the result through Task.ContinueWith(..., TaskScheduler.Default),
|
||||||
|
// which ran the projection lambda on a thread-pool thread. Completing a
|
||||||
|
// strongly-typed TCS directly removes that off-render-thread hop entirely —
|
||||||
|
// the awaiting caller resumes on whatever SynchronizationContext it captured
|
||||||
|
// (the Blazor renderer's, for an event-handler caller).
|
||||||
private TaskCompletionSource<object?>? _tcs;
|
private TaskCompletionSource<object?>? _tcs;
|
||||||
|
|
||||||
public Task<bool> ConfirmAsync(string title, string message, bool danger = false)
|
public Task<bool> ConfirmAsync(string title, string message, bool danger = false)
|
||||||
@@ -32,7 +39,7 @@ public class DialogService : IDialogService
|
|||||||
_tcs = tcs;
|
_tcs = tcs;
|
||||||
Current = new DialogState(title, DialogKind.Confirm, message, danger, PromptInitial: string.Empty, Placeholder: null);
|
Current = new DialogState(title, DialogKind.Confirm, message, danger, PromptInitial: string.Empty, Placeholder: null);
|
||||||
OnChange?.Invoke();
|
OnChange?.Invoke();
|
||||||
return tcs.Task.ContinueWith(t => t.Result is bool b && b, TaskScheduler.Default);
|
return Project(tcs.Task, static r => r is bool b && b);
|
||||||
}
|
}
|
||||||
|
|
||||||
public Task<string?> PromptAsync(string title, string label, string initialValue = "", string? placeholder = null)
|
public Task<string?> PromptAsync(string title, string label, string initialValue = "", string? placeholder = null)
|
||||||
@@ -42,7 +49,18 @@ public class DialogService : IDialogService
|
|||||||
_tcs = tcs;
|
_tcs = tcs;
|
||||||
Current = new DialogState(title, DialogKind.Prompt, label, Danger: false, PromptInitial: initialValue, Placeholder: placeholder);
|
Current = new DialogState(title, DialogKind.Prompt, label, Danger: false, PromptInitial: initialValue, Placeholder: placeholder);
|
||||||
OnChange?.Invoke();
|
OnChange?.Invoke();
|
||||||
return tcs.Task.ContinueWith(t => t.Result as string, TaskScheduler.Default);
|
return Project(tcs.Task, static r => r as string);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Awaits the host's result and projects it to the caller's type. The
|
||||||
|
/// <c>await</c> here resumes on the caller's captured context (the renderer
|
||||||
|
/// sync context for an event-handler caller), not a thread-pool thread.
|
||||||
|
/// </summary>
|
||||||
|
private static async Task<TResult> Project<TResult>(Task<object?> source, Func<object?, TResult> selector)
|
||||||
|
{
|
||||||
|
var result = await source.ConfigureAwait(false);
|
||||||
|
return selector(result);
|
||||||
}
|
}
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
|
|||||||
@@ -1,6 +1,7 @@
|
|||||||
@namespace ScadaLink.CentralUI.Components.Shared
|
@namespace ScadaLink.CentralUI.Components.Shared
|
||||||
@implements IAsyncDisposable
|
@implements IAsyncDisposable
|
||||||
@inject IJSRuntime JS
|
@inject IJSRuntime JS
|
||||||
|
@inject Microsoft.Extensions.Logging.ILogger<MonacoEditor> Logger
|
||||||
|
|
||||||
@if (ShowToolbar)
|
@if (ShowToolbar)
|
||||||
{
|
{
|
||||||
@@ -112,15 +113,45 @@
|
|||||||
_dotNetRef);
|
_dotNetRef);
|
||||||
_initialized = true;
|
_initialized = true;
|
||||||
}
|
}
|
||||||
catch
|
catch (InvalidOperationException)
|
||||||
{
|
{
|
||||||
// Prerendering or JS not ready — swallow; subsequent render will retry.
|
// Prerendering: JS interop is not available yet — the next
|
||||||
|
// (interactive) render retries. Expected, not logged.
|
||||||
|
}
|
||||||
|
catch (JSDisconnectedException)
|
||||||
|
{
|
||||||
|
// Circuit disconnected before init completed — nothing to do.
|
||||||
|
}
|
||||||
|
catch (JSException ex)
|
||||||
|
{
|
||||||
|
// A genuine Monaco init failure — surface it instead of hiding it.
|
||||||
|
Logger.LogError(ex, "Monaco editor {EditorId} failed to initialize.", _id);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
else if (_initialized && (Value ?? "") != _lastSentValue)
|
else if (_initialized && (Value ?? "") != _lastSentValue)
|
||||||
{
|
{
|
||||||
_lastSentValue = Value ?? "";
|
_lastSentValue = Value ?? "";
|
||||||
try { await JS.InvokeVoidAsync("MonacoBlazor.setValue", _id, _lastSentValue); } catch { }
|
await SafeInvokeAsync("MonacoBlazor.setValue", "set editor value", _id, _lastSentValue);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Invokes a Monaco JS function, swallowing the expected disconnect case but
|
||||||
|
/// logging any genuine JS error (CentralUI-018) so failures are not silent.
|
||||||
|
/// </summary>
|
||||||
|
private async ValueTask SafeInvokeAsync(string fn, string action, params object?[] args)
|
||||||
|
{
|
||||||
|
try
|
||||||
|
{
|
||||||
|
await JS.InvokeVoidAsync(fn, args);
|
||||||
|
}
|
||||||
|
catch (JSDisconnectedException)
|
||||||
|
{
|
||||||
|
// Circuit gone — the editor no longer exists; nothing to log.
|
||||||
|
}
|
||||||
|
catch (JSException ex)
|
||||||
|
{
|
||||||
|
Logger.LogWarning(ex, "Monaco editor {EditorId}: failed to {Action}.", _id, action);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -139,7 +170,7 @@
|
|||||||
public async Task RevealLineAsync(int line, int column = 1)
|
public async Task RevealLineAsync(int line, int column = 1)
|
||||||
{
|
{
|
||||||
if (!_initialized) return;
|
if (!_initialized) return;
|
||||||
try { await JS.InvokeVoidAsync("MonacoBlazor.revealLine", _id, line, column); } catch { }
|
await SafeInvokeAsync("MonacoBlazor.revealLine", "reveal line", _id, line, column);
|
||||||
}
|
}
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
@@ -161,32 +192,34 @@
|
|||||||
private async Task FormatAsync()
|
private async Task FormatAsync()
|
||||||
{
|
{
|
||||||
if (!_initialized) return;
|
if (!_initialized) return;
|
||||||
try { await JS.InvokeVoidAsync("MonacoBlazor.format", _id); } catch { }
|
await SafeInvokeAsync("MonacoBlazor.format", "format document", _id);
|
||||||
}
|
}
|
||||||
|
|
||||||
private async Task ToggleWrap()
|
private async Task ToggleWrap()
|
||||||
{
|
{
|
||||||
_wrap = !_wrap;
|
_wrap = !_wrap;
|
||||||
try { await JS.InvokeVoidAsync("MonacoBlazor.setEditorOption", _id, "wordWrap", _wrap ? "on" : "off"); } catch { }
|
await SafeInvokeAsync("MonacoBlazor.setEditorOption", "toggle word wrap", _id, "wordWrap", _wrap ? "on" : "off");
|
||||||
}
|
}
|
||||||
|
|
||||||
private async Task ToggleMinimap()
|
private async Task ToggleMinimap()
|
||||||
{
|
{
|
||||||
_minimap = !_minimap;
|
_minimap = !_minimap;
|
||||||
try { await JS.InvokeVoidAsync("MonacoBlazor.setEditorOption", _id, "minimap", new { enabled = _minimap }); } catch { }
|
await SafeInvokeAsync("MonacoBlazor.setEditorOption", "toggle minimap", _id, "minimap", new { enabled = _minimap });
|
||||||
}
|
}
|
||||||
|
|
||||||
private async Task ToggleTheme()
|
private async Task ToggleTheme()
|
||||||
{
|
{
|
||||||
_dark = !_dark;
|
_dark = !_dark;
|
||||||
try { await JS.InvokeVoidAsync("MonacoBlazor.setEditorOption", _id, "theme", _dark ? "vs-dark" : "vs"); } catch { }
|
await SafeInvokeAsync("MonacoBlazor.setEditorOption", "toggle theme", _id, "theme", _dark ? "vs-dark" : "vs");
|
||||||
}
|
}
|
||||||
|
|
||||||
public async ValueTask DisposeAsync()
|
public async ValueTask DisposeAsync()
|
||||||
{
|
{
|
||||||
if (_initialized)
|
if (_initialized)
|
||||||
{
|
{
|
||||||
try { await JS.InvokeVoidAsync("MonacoBlazor.dispose", _id); } catch { }
|
// Disposal commonly races a circuit disconnect — JSDisconnectedException
|
||||||
|
// here is expected and silent; a real JSException is still logged.
|
||||||
|
await SafeInvokeAsync("MonacoBlazor.dispose", "dispose editor", _id);
|
||||||
}
|
}
|
||||||
_dotNetRef?.Dispose();
|
_dotNetRef?.Dispose();
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -0,0 +1,57 @@
|
|||||||
|
namespace ScadaLink.CentralUI.Components.Shared;
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Pure helper for windowed pagination (CentralUI-016). Computes the set of
|
||||||
|
/// page numbers a pager should render: always the first and last page plus a
|
||||||
|
/// small range around the current page, with the rest elided. Keeps the
|
||||||
|
/// rendered button count bounded regardless of the total page count, instead
|
||||||
|
/// of emitting one <c><li></c> per page.
|
||||||
|
/// </summary>
|
||||||
|
public static class PagerWindow
|
||||||
|
{
|
||||||
|
/// <summary>
|
||||||
|
/// Returns the page numbers to render. A value of <c>0</c> in the result is
|
||||||
|
/// an ellipsis placeholder (a gap between non-contiguous page numbers).
|
||||||
|
/// <paramref name="radius"/> is how many pages to show on each side of the
|
||||||
|
/// current page.
|
||||||
|
/// </summary>
|
||||||
|
public static IReadOnlyList<int> Build(int currentPage, int totalPages, int radius = 2)
|
||||||
|
{
|
||||||
|
if (totalPages <= 1)
|
||||||
|
{
|
||||||
|
return totalPages == 1 ? new[] { 1 } : Array.Empty<int>();
|
||||||
|
}
|
||||||
|
|
||||||
|
currentPage = Math.Clamp(currentPage, 1, totalPages);
|
||||||
|
|
||||||
|
// Small enough that windowing buys nothing — render every page.
|
||||||
|
var maxUnwindowed = 2 * radius + 5;
|
||||||
|
if (totalPages <= maxUnwindowed)
|
||||||
|
{
|
||||||
|
return Enumerable.Range(1, totalPages).ToList();
|
||||||
|
}
|
||||||
|
|
||||||
|
var pages = new SortedSet<int> { 1, totalPages };
|
||||||
|
for (var p = currentPage - radius; p <= currentPage + radius; p++)
|
||||||
|
{
|
||||||
|
if (p >= 1 && p <= totalPages)
|
||||||
|
{
|
||||||
|
pages.Add(p);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Walk the sorted set, inserting an ellipsis (0) wherever a gap exists.
|
||||||
|
var result = new List<int>();
|
||||||
|
var previous = 0;
|
||||||
|
foreach (var page in pages)
|
||||||
|
{
|
||||||
|
if (previous != 0 && page - previous > 1)
|
||||||
|
{
|
||||||
|
result.Add(0); // ellipsis
|
||||||
|
}
|
||||||
|
result.Add(page);
|
||||||
|
previous = page;
|
||||||
|
}
|
||||||
|
return result;
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -126,29 +126,56 @@ else
|
|||||||
if (_contextMenuNeedsFocus && _showContextMenu)
|
if (_contextMenuNeedsFocus && _showContextMenu)
|
||||||
{
|
{
|
||||||
_contextMenuNeedsFocus = false;
|
_contextMenuNeedsFocus = false;
|
||||||
try { await _contextMenuRef.FocusAsync(); } catch { /* element may have been disposed if dismissed during render */ }
|
// The context-menu element may have been removed (menu dismissed
|
||||||
|
// during render) or the circuit disconnected — both are expected.
|
||||||
|
try { await _contextMenuRef.FocusAsync(); }
|
||||||
|
catch (Microsoft.JSInterop.JSException) { }
|
||||||
|
catch (Microsoft.JSInterop.JSDisconnectedException) { }
|
||||||
|
catch (InvalidOperationException) { }
|
||||||
}
|
}
|
||||||
|
|
||||||
if (firstRender && StorageKey != null)
|
if (firstRender && StorageKey != null)
|
||||||
{
|
{
|
||||||
var json = await JSRuntime.InvokeAsync<string?>("treeviewStorage.load", StorageKey);
|
string? json = null;
|
||||||
|
try
|
||||||
|
{
|
||||||
|
json = await JSRuntime.InvokeAsync<string?>("treeviewStorage.load", StorageKey);
|
||||||
|
}
|
||||||
|
catch (Microsoft.JSInterop.JSDisconnectedException)
|
||||||
|
{
|
||||||
|
// Circuit disconnected before the storage read completed — there
|
||||||
|
// is nothing to restore and nothing to log.
|
||||||
|
}
|
||||||
_storageLoaded = true;
|
_storageLoaded = true;
|
||||||
|
|
||||||
|
// CentralUI-018: a corrupt or wrong-shaped treeviewStorage payload
|
||||||
|
// must not throw out of OnAfterRenderAsync. Guard the deserialize
|
||||||
|
// and treat an unparseable payload as "no prior state".
|
||||||
|
List<string>? keys = null;
|
||||||
if (json != null)
|
if (json != null)
|
||||||
{
|
{
|
||||||
var keys = System.Text.Json.JsonSerializer.Deserialize<List<string>>(json);
|
try
|
||||||
if (keys != null)
|
|
||||||
{
|
{
|
||||||
// Union (don't replace): callers may have invoked RevealNode before
|
keys = System.Text.Json.JsonSerializer.Deserialize<List<string>>(json);
|
||||||
// this async storage load completed. Preserving those reveal-added
|
|
||||||
// keys ensures deep-link reveal isn't clobbered by the restore.
|
|
||||||
foreach (var k in keys) _expandedKeys.Add(k);
|
|
||||||
_initialExpansionApplied = true;
|
|
||||||
}
|
}
|
||||||
|
catch (System.Text.Json.JsonException)
|
||||||
|
{
|
||||||
|
keys = null;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if (keys != null)
|
||||||
|
{
|
||||||
|
// Union (don't replace): callers may have invoked RevealNode before
|
||||||
|
// this async storage load completed. Preserving those reveal-added
|
||||||
|
// keys ensures deep-link reveal isn't clobbered by the restore.
|
||||||
|
foreach (var k in keys) _expandedKeys.Add(k);
|
||||||
|
_initialExpansionApplied = true;
|
||||||
}
|
}
|
||||||
else if (InitiallyExpanded != null && _items is { Count: > 0 } && !_initialExpansionApplied)
|
else if (InitiallyExpanded != null && _items is { Count: > 0 } && !_initialExpansionApplied)
|
||||||
{
|
{
|
||||||
// Storage returned null (no prior state) — fall back to InitiallyExpanded
|
// Storage returned null or a corrupt payload (no usable prior
|
||||||
|
// state) — fall back to InitiallyExpanded.
|
||||||
_initialExpansionApplied = true;
|
_initialExpansionApplied = true;
|
||||||
ApplyInitialExpansion(_items);
|
ApplyInitialExpansion(_items);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -4,6 +4,7 @@
|
|||||||
@using Microsoft.AspNetCore.Components.Forms
|
@using Microsoft.AspNetCore.Components.Forms
|
||||||
@using Microsoft.AspNetCore.Components.Routing
|
@using Microsoft.AspNetCore.Components.Routing
|
||||||
@using Microsoft.AspNetCore.Components.Web
|
@using Microsoft.AspNetCore.Components.Web
|
||||||
|
@using Microsoft.Extensions.Logging
|
||||||
@using Microsoft.JSInterop
|
@using Microsoft.JSInterop
|
||||||
@using static Microsoft.AspNetCore.Components.Web.RenderMode
|
@using static Microsoft.AspNetCore.Components.Web.RenderMode
|
||||||
@using ScadaLink.CentralUI
|
@using ScadaLink.CentralUI
|
||||||
|
|||||||
@@ -4,11 +4,73 @@ using ScadaLink.Commons.Types.DataConnections;
|
|||||||
|
|
||||||
namespace ScadaLink.Commons.Serialization;
|
namespace ScadaLink.Commons.Serialization;
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Outcome classification for <see cref="OpcUaEndpointConfigSerializer.Deserialize"/>.
|
||||||
|
/// </summary>
|
||||||
|
public enum OpcUaConfigParseStatus
|
||||||
|
{
|
||||||
|
/// <summary>The stored JSON parsed cleanly as the current typed shape.</summary>
|
||||||
|
Typed,
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// The stored JSON parsed as the legacy flat string-dict shape. The returned
|
||||||
|
/// config is usable; the caller may prompt the user to re-save in the new shape.
|
||||||
|
/// </summary>
|
||||||
|
Legacy,
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// The stored JSON could not be parsed at all (genuinely malformed). The returned
|
||||||
|
/// config is an empty default and the original string was lost — the caller should
|
||||||
|
/// surface an error rather than presenting the empty config as the user's data.
|
||||||
|
/// </summary>
|
||||||
|
Malformed
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Result of <see cref="OpcUaEndpointConfigSerializer.Deserialize"/>. Carries the parsed
|
||||||
|
/// config plus an explicit <see cref="Status"/> distinguishing a recoverable legacy row
|
||||||
|
/// from genuinely unparseable input. Deconstructs into <c>(Config, IsLegacy)</c> for
|
||||||
|
/// backward compatibility with callers that only need those two values.
|
||||||
|
/// </summary>
|
||||||
|
public readonly record struct OpcUaConfigParseResult
|
||||||
|
{
|
||||||
|
public OpcUaConfigParseResult(OpcUaEndpointConfig config, OpcUaConfigParseStatus status)
|
||||||
|
{
|
||||||
|
Config = config;
|
||||||
|
Status = status;
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <summary>The parsed config (an empty default when <see cref="Status"/> is Malformed).</summary>
|
||||||
|
public OpcUaEndpointConfig Config { get; }
|
||||||
|
|
||||||
|
/// <summary>Classification of the parse outcome.</summary>
|
||||||
|
public OpcUaConfigParseStatus Status { get; }
|
||||||
|
|
||||||
|
/// <summary>True when the source parsed as the legacy flat-dict shape.</summary>
|
||||||
|
public bool IsLegacy => Status == OpcUaConfigParseStatus.Legacy;
|
||||||
|
|
||||||
|
/// <summary>True when the source could not be parsed at all.</summary>
|
||||||
|
public bool IsMalformed => Status == OpcUaConfigParseStatus.Malformed;
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Two-element deconstruction kept for backward compatibility. Note that
|
||||||
|
/// <c>IsLegacy</c> is <c>false</c> for both <see cref="OpcUaConfigParseStatus.Typed"/>
|
||||||
|
/// and <see cref="OpcUaConfigParseStatus.Malformed"/>; callers that need to tell those
|
||||||
|
/// apart should read <see cref="Status"/> directly.
|
||||||
|
/// </summary>
|
||||||
|
public void Deconstruct(out OpcUaEndpointConfig config, out bool isLegacy)
|
||||||
|
{
|
||||||
|
config = Config;
|
||||||
|
isLegacy = IsLegacy;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// Serializes <see cref="OpcUaEndpointConfig"/> to/from the typed nested JSON
|
/// Serializes <see cref="OpcUaEndpointConfig"/> to/from the typed nested JSON
|
||||||
/// shape stored in <c>DataConnection.PrimaryConfiguration</c> / <c>BackupConfiguration</c>.
|
/// shape stored in <c>DataConnection.PrimaryConfiguration</c> / <c>BackupConfiguration</c>.
|
||||||
/// On read, falls back to the legacy flat string-dict shape for pre-refactor rows
|
/// On read, falls back to the legacy flat string-dict shape for pre-refactor rows
|
||||||
/// and returns IsLegacy=true so the form can prompt the user to re-save.
|
/// and reports <see cref="OpcUaConfigParseStatus.Legacy"/> so the form can prompt the
|
||||||
|
/// user to re-save.
|
||||||
/// </summary>
|
/// </summary>
|
||||||
public static class OpcUaEndpointConfigSerializer
|
public static class OpcUaEndpointConfigSerializer
|
||||||
{
|
{
|
||||||
@@ -22,10 +84,25 @@ public static class OpcUaEndpointConfigSerializer
|
|||||||
public static string Serialize(OpcUaEndpointConfig config)
|
public static string Serialize(OpcUaEndpointConfig config)
|
||||||
=> JsonSerializer.Serialize(config, JsonOpts);
|
=> JsonSerializer.Serialize(config, JsonOpts);
|
||||||
|
|
||||||
public static (OpcUaEndpointConfig Config, bool IsLegacy) Deserialize(string? json)
|
/// <summary>
|
||||||
|
/// Parses stored OPC UA endpoint JSON. Tries the current typed shape first, then the
|
||||||
|
/// legacy flat string-dict shape. The returned <see cref="OpcUaConfigParseResult.Status"/>
|
||||||
|
/// distinguishes three outcomes:
|
||||||
|
/// <list type="bullet">
|
||||||
|
/// <item><see cref="OpcUaConfigParseStatus.Typed"/> — clean parse of the current shape
|
||||||
|
/// (also returned for null/blank input, which yields a default config).</item>
|
||||||
|
/// <item><see cref="OpcUaConfigParseStatus.Legacy"/> — parsed as a legacy flat object;
|
||||||
|
/// the config is usable and the caller may prompt a re-save.</item>
|
||||||
|
/// <item><see cref="OpcUaConfigParseStatus.Malformed"/> — the input is genuinely
|
||||||
|
/// unparseable JSON. The config is an empty default and the original string is lost;
|
||||||
|
/// the caller should surface an error rather than treating the empty config as the
|
||||||
|
/// user's saved data.</item>
|
||||||
|
/// </list>
|
||||||
|
/// </summary>
|
||||||
|
public static OpcUaConfigParseResult Deserialize(string? json)
|
||||||
{
|
{
|
||||||
if (string.IsNullOrWhiteSpace(json))
|
if (string.IsNullOrWhiteSpace(json))
|
||||||
return (new OpcUaEndpointConfig(), false);
|
return new OpcUaConfigParseResult(new OpcUaEndpointConfig(), OpcUaConfigParseStatus.Typed);
|
||||||
|
|
||||||
try
|
try
|
||||||
{
|
{
|
||||||
@@ -35,18 +112,21 @@ public static class OpcUaEndpointConfigSerializer
|
|||||||
{
|
{
|
||||||
var typed = JsonSerializer.Deserialize<OpcUaEndpointConfig>(json, JsonOpts);
|
var typed = JsonSerializer.Deserialize<OpcUaEndpointConfig>(json, JsonOpts);
|
||||||
if (typed != null)
|
if (typed != null)
|
||||||
return (typed, false);
|
return new OpcUaConfigParseResult(typed, OpcUaConfigParseStatus.Typed);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
catch (JsonException) { /* fall through to legacy */ }
|
catch (JsonException) { /* fall through to legacy */ }
|
||||||
|
|
||||||
try
|
try
|
||||||
{
|
{
|
||||||
return (LoadLegacy(json!), IsLegacy: true);
|
return new OpcUaConfigParseResult(LoadLegacy(json!), OpcUaConfigParseStatus.Legacy);
|
||||||
}
|
}
|
||||||
catch (JsonException)
|
catch (JsonException)
|
||||||
{
|
{
|
||||||
return (new OpcUaEndpointConfig(), IsLegacy: true);
|
// Genuinely malformed input: not a recoverable legacy row. Report Malformed
|
||||||
|
// (not Legacy) so the caller can surface an error instead of presenting an
|
||||||
|
// empty config as if it were the user's saved configuration.
|
||||||
|
return new OpcUaConfigParseResult(new OpcUaEndpointConfig(), OpcUaConfigParseStatus.Malformed);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -55,8 +55,19 @@ public class DynamicJsonElement : DynamicObject
|
|||||||
|
|
||||||
public override bool TryConvert(ConvertBinder binder, out object? result)
|
public override bool TryConvert(ConvertBinder binder, out object? result)
|
||||||
{
|
{
|
||||||
|
// Conversion to object (or dynamic): never null out a present value. Return the
|
||||||
|
// unwrapped value for scalars, this wrapper for objects/arrays, and null only
|
||||||
|
// when the element is genuinely JSON null.
|
||||||
|
if (binder.Type == typeof(object))
|
||||||
|
{
|
||||||
|
result = _element.ValueKind == JsonValueKind.Null ? null : Wrap(_element);
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
result = ConvertTo(binder.Type);
|
result = ConvertTo(binder.Type);
|
||||||
return result != null || binder.Type == typeof(object);
|
// A non-object target with a null result means ConvertTo could not handle the
|
||||||
|
// element/type pair — report failure so the binder surfaces a binding error.
|
||||||
|
return result != null;
|
||||||
}
|
}
|
||||||
|
|
||||||
public override string ToString()
|
public override string ToString()
|
||||||
|
|||||||
@@ -14,6 +14,9 @@ public sealed class Result<T>
|
|||||||
|
|
||||||
private Result(string error)
|
private Result(string error)
|
||||||
{
|
{
|
||||||
|
// A failed Result must always carry a usable message — Result is the
|
||||||
|
// system-wide error-handling type, and consumers log/display Error directly.
|
||||||
|
ArgumentException.ThrowIfNullOrWhiteSpace(error);
|
||||||
_value = default;
|
_value = default;
|
||||||
_error = error;
|
_error = error;
|
||||||
IsSuccess = false;
|
IsSuccess = false;
|
||||||
@@ -33,6 +36,11 @@ public sealed class Result<T>
|
|||||||
|
|
||||||
public static Result<T> Success(T value) => new(value);
|
public static Result<T> Success(T value) => new(value);
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Creates a failed result carrying the given error message.
|
||||||
|
/// </summary>
|
||||||
|
/// <exception cref="ArgumentNullException"><paramref name="error"/> is null.</exception>
|
||||||
|
/// <exception cref="ArgumentException"><paramref name="error"/> is empty or whitespace.</exception>
|
||||||
public static Result<T> Failure(string error) => new(error);
|
public static Result<T> Failure(string error) => new(error);
|
||||||
|
|
||||||
public TResult Match<TResult>(Func<T, TResult> onSuccess, Func<string, TResult> onFailure) =>
|
public TResult Match<TResult>(Func<T, TResult> onSuccess, Func<string, TResult> onFailure) =>
|
||||||
|
|||||||
@@ -1,4 +1,5 @@
|
|||||||
using System.Collections;
|
using System.Collections;
|
||||||
|
using System.Globalization;
|
||||||
|
|
||||||
namespace ScadaLink.Commons.Types;
|
namespace ScadaLink.Commons.Types;
|
||||||
|
|
||||||
@@ -9,21 +10,36 @@ namespace ScadaLink.Commons.Types;
|
|||||||
public static class ValueFormatter
|
public static class ValueFormatter
|
||||||
{
|
{
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// Formats a value for display as a string. Returns the value's natural
|
/// Formats a value as a string. Returns the value's string representation for
|
||||||
/// string representation for scalars, and comma-separated elements for
|
/// scalars and comma-separated elements for array/collection types.
|
||||||
/// array/collection types.
|
|
||||||
/// </summary>
|
/// </summary>
|
||||||
|
/// <remarks>
|
||||||
|
/// Formatting is <see cref="CultureInfo.InvariantCulture">culture-invariant</see>:
|
||||||
|
/// numbers and <see cref="DateTime"/> values render the same regardless of the
|
||||||
|
/// server/thread locale. This is required because the formatter feeds non-UI
|
||||||
|
/// contexts (gRPC stream events, logs, diff display) where locale-dependent
|
||||||
|
/// output (decimal separators, date order) would be inconsistent.
|
||||||
|
/// </remarks>
|
||||||
public static string FormatDisplayValue(object? value)
|
public static string FormatDisplayValue(object? value)
|
||||||
{
|
{
|
||||||
if (value is null) return "";
|
if (value is null) return "";
|
||||||
if (value is string s) return s;
|
if (value is string s) return s;
|
||||||
if (value is IFormattable) return value.ToString() ?? "";
|
if (value is IFormattable formattable)
|
||||||
|
return formattable.ToString(null, CultureInfo.InvariantCulture) ?? "";
|
||||||
|
|
||||||
if (value is IEnumerable enumerable)
|
if (value is IEnumerable enumerable)
|
||||||
{
|
{
|
||||||
return string.Join(",", enumerable.Cast<object?>().Select(e => e?.ToString() ?? ""));
|
return string.Join(",", enumerable.Cast<object?>().Select(FormatElement));
|
||||||
}
|
}
|
||||||
|
|
||||||
return value.ToString() ?? "";
|
return value.ToString() ?? "";
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private static string FormatElement(object? element) => element switch
|
||||||
|
{
|
||||||
|
null => "",
|
||||||
|
string str => str,
|
||||||
|
IFormattable f => f.ToString(null, CultureInfo.InvariantCulture) ?? "",
|
||||||
|
_ => element.ToString() ?? ""
|
||||||
|
};
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -50,7 +50,7 @@ public class CentralCommunicationActor : ReceiveActor
|
|||||||
/// Maps SiteIdentifier → (ClusterClient actor, set of contact address strings).
|
/// Maps SiteIdentifier → (ClusterClient actor, set of contact address strings).
|
||||||
/// Refreshed periodically via RefreshSiteAddresses.
|
/// Refreshed periodically via RefreshSiteAddresses.
|
||||||
/// </summary>
|
/// </summary>
|
||||||
private Dictionary<string, (IActorRef Client, ImmutableHashSet<string> ContactAddresses)> _siteClients = new();
|
private readonly Dictionary<string, (IActorRef Client, ImmutableHashSet<string> ContactAddresses)> _siteClients = new();
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// Tracks active debug view subscriptions: correlationId → (siteId, subscriber).
|
/// Tracks active debug view subscriptions: correlationId → (siteId, subscriber).
|
||||||
@@ -262,9 +262,23 @@ public class CentralCommunicationActor : ReceiveActor
|
|||||||
// Add or update
|
// Add or update
|
||||||
foreach (var (siteId, addresses) in msg.SiteContacts)
|
foreach (var (siteId, addresses) in msg.SiteContacts)
|
||||||
{
|
{
|
||||||
var contactPaths = addresses
|
// Communication-009: parse all addresses up front inside a try/catch so a
|
||||||
.Select(a => ActorPath.Parse($"{a}/system/receptionist"))
|
// single malformed site row cannot abort the whole refresh loop and leave
|
||||||
.ToImmutableHashSet();
|
// the cache half-updated. A bad site is logged and skipped; others proceed.
|
||||||
|
ImmutableHashSet<ActorPath> contactPaths;
|
||||||
|
try
|
||||||
|
{
|
||||||
|
contactPaths = addresses
|
||||||
|
.Select(a => ActorPath.Parse($"{a}/system/receptionist"))
|
||||||
|
.ToImmutableHashSet();
|
||||||
|
}
|
||||||
|
catch (Exception ex)
|
||||||
|
{
|
||||||
|
_log.Warning(ex,
|
||||||
|
"Malformed contact address for site {0}; skipping this site in the refresh "
|
||||||
|
+ "(other sites are unaffected)", siteId);
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
var contactStrings = addresses.ToImmutableHashSet();
|
var contactStrings = addresses.ToImmutableHashSet();
|
||||||
|
|
||||||
|
|||||||
@@ -7,7 +7,9 @@ using ScadaLink.Communication.Grpc;
|
|||||||
namespace ScadaLink.Communication.Actors;
|
namespace ScadaLink.Communication.Actors;
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// Persistent actor (one per active debug session) on the central side.
|
/// Long-lived (one per active debug session) actor on the central side. Debug sessions
|
||||||
|
/// are session-based and temporary — this actor holds no persisted state and does not
|
||||||
|
/// derive from an Akka.Persistence base class; its state does not survive a restart.
|
||||||
/// Sends SubscribeDebugViewRequest to the site via CentralCommunicationActor (with THIS actor
|
/// Sends SubscribeDebugViewRequest to the site via CentralCommunicationActor (with THIS actor
|
||||||
/// as the Sender) to get the initial snapshot. After receiving the snapshot, opens a gRPC
|
/// as the Sender) to get the initial snapshot. After receiving the snapshot, opens a gRPC
|
||||||
/// server-streaming subscription via SiteStreamGrpcClient for ongoing events.
|
/// server-streaming subscription via SiteStreamGrpcClient for ongoing events.
|
||||||
|
|||||||
@@ -0,0 +1,88 @@
|
|||||||
|
using System.CommandLine;
|
||||||
|
using ScadaLink.CLI.Commands;
|
||||||
|
using ScadaLink.Commons.Messages.Management;
|
||||||
|
|
||||||
|
namespace ScadaLink.CLI.Tests;
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Regression tests for CLI-013 — the command-tree wiring was untested. These tests
|
||||||
|
/// build every command group and assert the tree is well-formed (every leaf has an
|
||||||
|
/// action, no group is empty), and that every management command record the CLI sends
|
||||||
|
/// resolves via <see cref="ManagementCommandRegistry"/> (so command-name derivation
|
||||||
|
/// never throws at runtime).
|
||||||
|
/// </summary>
|
||||||
|
public class CommandTreeTests
|
||||||
|
{
|
||||||
|
private static readonly Option<string> Url = new("--url") { Recursive = true };
|
||||||
|
private static readonly Option<string> Username = new("--username") { Recursive = true };
|
||||||
|
private static readonly Option<string> Password = new("--password") { Recursive = true };
|
||||||
|
private static readonly Option<string> Format = CliOptions.CreateFormatOption();
|
||||||
|
|
||||||
|
private static IEnumerable<Command> AllCommandGroups() => new[]
|
||||||
|
{
|
||||||
|
TemplateCommands.Build(Url, Format, Username, Password),
|
||||||
|
InstanceCommands.Build(Url, Format, Username, Password),
|
||||||
|
SiteCommands.Build(Url, Format, Username, Password),
|
||||||
|
DeployCommands.Build(Url, Format, Username, Password),
|
||||||
|
DataConnectionCommands.Build(Url, Format, Username, Password),
|
||||||
|
ExternalSystemCommands.Build(Url, Format, Username, Password),
|
||||||
|
NotificationCommands.Build(Url, Format, Username, Password),
|
||||||
|
SecurityCommands.Build(Url, Format, Username, Password),
|
||||||
|
AuditLogCommands.Build(Url, Format, Username, Password),
|
||||||
|
HealthCommands.Build(Url, Format, Username, Password),
|
||||||
|
DebugCommands.Build(Url, Format, Username, Password),
|
||||||
|
SharedScriptCommands.Build(Url, Format, Username, Password),
|
||||||
|
DbConnectionCommands.Build(Url, Format, Username, Password),
|
||||||
|
ApiMethodCommands.Build(Url, Format, Username, Password),
|
||||||
|
};
|
||||||
|
|
||||||
|
private static IEnumerable<Command> LeafCommands(Command command)
|
||||||
|
{
|
||||||
|
if (command.Subcommands.Count == 0)
|
||||||
|
{
|
||||||
|
yield return command;
|
||||||
|
yield break;
|
||||||
|
}
|
||||||
|
|
||||||
|
foreach (var sub in command.Subcommands)
|
||||||
|
foreach (var leaf in LeafCommands(sub))
|
||||||
|
yield return leaf;
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void AllCommandGroups_Build_WithoutThrowing()
|
||||||
|
{
|
||||||
|
var groups = AllCommandGroups().ToList();
|
||||||
|
Assert.Equal(14, groups.Count);
|
||||||
|
Assert.All(groups, g => Assert.False(string.IsNullOrWhiteSpace(g.Name)));
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void EveryLeafCommand_HasAnAction()
|
||||||
|
{
|
||||||
|
// A leaf command with no action is dead wiring — invoking it would do nothing.
|
||||||
|
var leaves = AllCommandGroups().SelectMany(LeafCommands).ToList();
|
||||||
|
|
||||||
|
Assert.NotEmpty(leaves);
|
||||||
|
Assert.All(leaves, leaf =>
|
||||||
|
Assert.True(leaf.Action != null, $"Leaf command '{leaf.Name}' has no action."));
|
||||||
|
}
|
||||||
|
|
||||||
|
[Theory]
|
||||||
|
[InlineData(typeof(GetInstanceCommand))]
|
||||||
|
[InlineData(typeof(ListSitesCommand))]
|
||||||
|
[InlineData(typeof(CreateTemplateCommand))]
|
||||||
|
[InlineData(typeof(SetConnectionBindingsCommand))]
|
||||||
|
[InlineData(typeof(SetInstanceOverridesCommand))]
|
||||||
|
[InlineData(typeof(DebugSnapshotCommand))]
|
||||||
|
[InlineData(typeof(MgmtDeployInstanceCommand))]
|
||||||
|
[InlineData(typeof(QueryAuditLogCommand))]
|
||||||
|
public void CommandPayloadTypes_ResolveViaRegistry(Type commandType)
|
||||||
|
{
|
||||||
|
// GetCommandName throws ArgumentException for an unregistered type — the CLI
|
||||||
|
// calls it for every command it sends, so each must round-trip.
|
||||||
|
var name = ManagementCommandRegistry.GetCommandName(commandType);
|
||||||
|
Assert.False(string.IsNullOrWhiteSpace(name));
|
||||||
|
Assert.Equal(commandType, ManagementCommandRegistry.Resolve(name));
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -0,0 +1,100 @@
|
|||||||
|
using System.Threading.Tasks;
|
||||||
|
using ScadaLink.CLI.Commands;
|
||||||
|
|
||||||
|
namespace ScadaLink.CLI.Tests;
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Regression tests for the testable pieces of <c>DebugCommands.StreamDebugAsync</c>:
|
||||||
|
/// CLI-010 (Ctrl+C during connect misreported as a connection failure) and
|
||||||
|
/// CLI-012 (non-deterministic exit code after stream termination).
|
||||||
|
/// </summary>
|
||||||
|
public class DebugStreamTests
|
||||||
|
{
|
||||||
|
// --- CLI-010: connect-failure classification --------------------------------------
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void ClassifyConnectFailure_OperationCanceled_IsTreatedAsCancellation()
|
||||||
|
{
|
||||||
|
// Ctrl+C while StartAsync is still establishing the connection throws
|
||||||
|
// OperationCanceledException — this is a graceful cancellation, not a failure.
|
||||||
|
var result = DebugStreamHelpers.ClassifyConnectFailure(
|
||||||
|
new OperationCanceledException(), cancellationRequested: true);
|
||||||
|
|
||||||
|
Assert.True(result.IsCancellation);
|
||||||
|
Assert.Equal(0, result.ExitCode);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void ClassifyConnectFailure_TaskCanceled_WhenCancelRequested_IsCancellation()
|
||||||
|
{
|
||||||
|
var result = DebugStreamHelpers.ClassifyConnectFailure(
|
||||||
|
new TaskCanceledException(), cancellationRequested: true);
|
||||||
|
|
||||||
|
Assert.True(result.IsCancellation);
|
||||||
|
Assert.Equal(0, result.ExitCode);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void ClassifyConnectFailure_RealException_IsConnectionFailure()
|
||||||
|
{
|
||||||
|
var result = DebugStreamHelpers.ClassifyConnectFailure(
|
||||||
|
new HttpRequestException("connection refused"), cancellationRequested: false);
|
||||||
|
|
||||||
|
Assert.False(result.IsCancellation);
|
||||||
|
Assert.Equal(1, result.ExitCode);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void ClassifyConnectFailure_CanceledExceptionButNoCancelRequested_IsConnectionFailure()
|
||||||
|
{
|
||||||
|
// A cancellation that did not originate from the user (e.g. a server-side abort)
|
||||||
|
// is still a real connection failure.
|
||||||
|
var result = DebugStreamHelpers.ClassifyConnectFailure(
|
||||||
|
new OperationCanceledException(), cancellationRequested: false);
|
||||||
|
|
||||||
|
Assert.False(result.IsCancellation);
|
||||||
|
Assert.Equal(1, result.ExitCode);
|
||||||
|
}
|
||||||
|
|
||||||
|
// --- CLI-012: deterministic exit-code resolution ----------------------------------
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task ResolveStreamExitCodeAsync_TerminationResultSet_PrefersThatResult()
|
||||||
|
{
|
||||||
|
// OnStreamTerminated set exitTcs to 1 — that must win even on the Ctrl+C path.
|
||||||
|
var tcs = new TaskCompletionSource<int>();
|
||||||
|
tcs.SetResult(1);
|
||||||
|
|
||||||
|
var code = await DebugStreamHelpers.ResolveStreamExitCodeAsync(tcs.Task);
|
||||||
|
|
||||||
|
Assert.Equal(1, code);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task ResolveStreamExitCodeAsync_NoResult_ReturnsZero()
|
||||||
|
{
|
||||||
|
// Pure Ctrl+C: exitTcs never completed — graceful shutdown, exit 0.
|
||||||
|
var tcs = new TaskCompletionSource<int>();
|
||||||
|
|
||||||
|
var code = await DebugStreamHelpers.ResolveStreamExitCodeAsync(tcs.Task);
|
||||||
|
|
||||||
|
Assert.Equal(0, code);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task ResolveStreamExitCodeAsync_ResultArrivesDuringGrace_IsObserved()
|
||||||
|
{
|
||||||
|
// A stream termination racing with Ctrl+C: the result lands shortly after the
|
||||||
|
// wait was cancelled. The grace period must let it be observed deterministically.
|
||||||
|
var tcs = new TaskCompletionSource<int>();
|
||||||
|
_ = Task.Run(async () =>
|
||||||
|
{
|
||||||
|
await Task.Delay(20);
|
||||||
|
tcs.TrySetResult(1);
|
||||||
|
});
|
||||||
|
|
||||||
|
var code = await DebugStreamHelpers.ResolveStreamExitCodeAsync(tcs.Task);
|
||||||
|
|
||||||
|
Assert.Equal(1, code);
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -0,0 +1,59 @@
|
|||||||
|
using ScadaLink.CLI;
|
||||||
|
using ScadaLink.CLI.Commands;
|
||||||
|
|
||||||
|
namespace ScadaLink.CLI.Tests;
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Regression tests for CLI-009 — the design doc defines exit code 2 as "authorization
|
||||||
|
/// failure". The previous implementation keyed exit 2 solely off HTTP 403, so an
|
||||||
|
/// authorization failure the server signalled with an error <c>code</c> of
|
||||||
|
/// <c>UNAUTHORIZED</c>/<c>FORBIDDEN</c> but a different HTTP status was misreported as a
|
||||||
|
/// generic error (exit 1). Exit code 2 now keys off either signal.
|
||||||
|
/// </summary>
|
||||||
|
[Collection("Console")]
|
||||||
|
public class ExitCodeTests
|
||||||
|
{
|
||||||
|
private static int HandleQuietly(ManagementResponse response)
|
||||||
|
{
|
||||||
|
var errWriter = new StringWriter();
|
||||||
|
Console.SetError(errWriter);
|
||||||
|
try
|
||||||
|
{
|
||||||
|
return CommandHelpers.HandleResponse(response, "json");
|
||||||
|
}
|
||||||
|
finally
|
||||||
|
{
|
||||||
|
Console.SetError(new StreamWriter(Console.OpenStandardError()) { AutoFlush = true });
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void HandleResponse_Http403_ReturnsTwo()
|
||||||
|
{
|
||||||
|
Assert.Equal(2, HandleQuietly(new ManagementResponse(403, null, "Forbidden", "FORBIDDEN")));
|
||||||
|
}
|
||||||
|
|
||||||
|
[Theory]
|
||||||
|
[InlineData("UNAUTHORIZED")]
|
||||||
|
[InlineData("FORBIDDEN")]
|
||||||
|
[InlineData("unauthorized")]
|
||||||
|
public void HandleResponse_AuthorizationCode_NonForbiddenStatus_ReturnsTwo(string code)
|
||||||
|
{
|
||||||
|
// The server signalled an authorization failure via the error code but with a
|
||||||
|
// non-403 HTTP status; per the documented exit-code table this is still exit 2.
|
||||||
|
Assert.Equal(2, HandleQuietly(new ManagementResponse(400, null, "Access denied", code)));
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void HandleResponse_GenericError_ReturnsOne()
|
||||||
|
{
|
||||||
|
Assert.Equal(1, HandleQuietly(new ManagementResponse(400, null, "Validation failed", "INVALID_ARGUMENT")));
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void HandleResponse_AuthenticationFailure_ReturnsOne()
|
||||||
|
{
|
||||||
|
// Authentication failure (bad credentials) is exit 1, distinct from authorization.
|
||||||
|
Assert.Equal(1, HandleQuietly(new ManagementResponse(401, null, "Invalid credentials", "AUTH_FAILED")));
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -0,0 +1,44 @@
|
|||||||
|
using System.CommandLine;
|
||||||
|
using ScadaLink.CLI.Commands;
|
||||||
|
|
||||||
|
namespace ScadaLink.CLI.Tests;
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Regression tests for CLI-008 — the <c>--format</c> option previously accepted any
|
||||||
|
/// string, so a typo like <c>--format tabel</c> silently fell through to JSON output
|
||||||
|
/// with no feedback. The option must reject values outside {json, table} with a parse
|
||||||
|
/// error.
|
||||||
|
/// </summary>
|
||||||
|
public class FormatOptionValidationTests
|
||||||
|
{
|
||||||
|
[Theory]
|
||||||
|
[InlineData("json")]
|
||||||
|
[InlineData("table")]
|
||||||
|
public void FormatOption_AcceptsValidValues(string value)
|
||||||
|
{
|
||||||
|
var formatOption = CliOptions.CreateFormatOption();
|
||||||
|
var root = new RootCommand();
|
||||||
|
root.Add(formatOption);
|
||||||
|
|
||||||
|
var result = root.Parse(new[] { "--format", value });
|
||||||
|
|
||||||
|
Assert.Empty(result.Errors);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Theory]
|
||||||
|
[InlineData("tabel")]
|
||||||
|
[InlineData("xml")]
|
||||||
|
[InlineData("yaml")]
|
||||||
|
[InlineData("")]
|
||||||
|
[InlineData("JSON")] // case-sensitive: documented values are lowercase
|
||||||
|
public void FormatOption_RejectsInvalidValues(string value)
|
||||||
|
{
|
||||||
|
var formatOption = CliOptions.CreateFormatOption();
|
||||||
|
var root = new RootCommand();
|
||||||
|
root.Add(formatOption);
|
||||||
|
|
||||||
|
var result = root.Parse(new[] { "--format", value });
|
||||||
|
|
||||||
|
Assert.NotEmpty(result.Errors);
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -0,0 +1,106 @@
|
|||||||
|
using System.Net;
|
||||||
|
using System.Text;
|
||||||
|
using ScadaLink.CLI;
|
||||||
|
|
||||||
|
namespace ScadaLink.CLI.Tests;
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Regression tests for CLI-013 — <see cref="ManagementHttpClient.SendCommandAsync"/>
|
||||||
|
/// (success, error-body parsing, connection-failure, and timeout paths) was untested.
|
||||||
|
/// Uses a stub <see cref="HttpMessageHandler"/> so no live server is required.
|
||||||
|
/// </summary>
|
||||||
|
public class ManagementHttpClientTests
|
||||||
|
{
|
||||||
|
private sealed class StubHandler : HttpMessageHandler
|
||||||
|
{
|
||||||
|
private readonly Func<HttpRequestMessage, CancellationToken, Task<HttpResponseMessage>> _responder;
|
||||||
|
|
||||||
|
public StubHandler(HttpStatusCode status, string body)
|
||||||
|
: this((_, _) => Task.FromResult(new HttpResponseMessage(status)
|
||||||
|
{
|
||||||
|
Content = new StringContent(body, Encoding.UTF8, "application/json"),
|
||||||
|
}))
|
||||||
|
{
|
||||||
|
}
|
||||||
|
|
||||||
|
public StubHandler(Func<HttpRequestMessage, CancellationToken, Task<HttpResponseMessage>> responder)
|
||||||
|
{
|
||||||
|
_responder = responder;
|
||||||
|
}
|
||||||
|
|
||||||
|
protected override Task<HttpResponseMessage> SendAsync(
|
||||||
|
HttpRequestMessage request, CancellationToken cancellationToken)
|
||||||
|
=> _responder(request, cancellationToken);
|
||||||
|
}
|
||||||
|
|
||||||
|
private static ManagementHttpClient ClientWith(StubHandler handler)
|
||||||
|
=> new(new HttpClient(handler), "http://localhost:9001", "user", "pass");
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task SendCommandAsync_Success_ReturnsJsonData()
|
||||||
|
{
|
||||||
|
using var client = ClientWith(new StubHandler(HttpStatusCode.OK, "{\"id\":1}"));
|
||||||
|
|
||||||
|
var response = await client.SendCommandAsync("ListSites", new { }, TimeSpan.FromSeconds(5));
|
||||||
|
|
||||||
|
Assert.Equal(200, response.StatusCode);
|
||||||
|
Assert.Equal("{\"id\":1}", response.JsonData);
|
||||||
|
Assert.Null(response.Error);
|
||||||
|
Assert.Null(response.ErrorCode);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task SendCommandAsync_ErrorBody_ParsesErrorAndCode()
|
||||||
|
{
|
||||||
|
using var client = ClientWith(new StubHandler(
|
||||||
|
HttpStatusCode.BadRequest, "{\"error\":\"Bad input\",\"code\":\"INVALID_ARGUMENT\"}"));
|
||||||
|
|
||||||
|
var response = await client.SendCommandAsync("ListSites", new { }, TimeSpan.FromSeconds(5));
|
||||||
|
|
||||||
|
Assert.Equal(400, response.StatusCode);
|
||||||
|
Assert.Null(response.JsonData);
|
||||||
|
Assert.Equal("Bad input", response.Error);
|
||||||
|
Assert.Equal("INVALID_ARGUMENT", response.ErrorCode);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task SendCommandAsync_NonJsonErrorBody_FallsBackToRawBody()
|
||||||
|
{
|
||||||
|
using var client = ClientWith(new StubHandler(
|
||||||
|
HttpStatusCode.BadGateway, "<html>Bad Gateway</html>"));
|
||||||
|
|
||||||
|
var response = await client.SendCommandAsync("ListSites", new { }, TimeSpan.FromSeconds(5));
|
||||||
|
|
||||||
|
Assert.Equal(502, response.StatusCode);
|
||||||
|
Assert.Equal("<html>Bad Gateway</html>", response.Error);
|
||||||
|
Assert.Null(response.ErrorCode);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task SendCommandAsync_ConnectionFailure_ReturnsStatusZero()
|
||||||
|
{
|
||||||
|
using var client = ClientWith(new StubHandler((_, _) =>
|
||||||
|
throw new HttpRequestException("connection refused")));
|
||||||
|
|
||||||
|
var response = await client.SendCommandAsync("ListSites", new { }, TimeSpan.FromSeconds(5));
|
||||||
|
|
||||||
|
Assert.Equal(0, response.StatusCode);
|
||||||
|
Assert.Equal("CONNECTION_FAILED", response.ErrorCode);
|
||||||
|
Assert.Contains("connection refused", response.Error);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task SendCommandAsync_Timeout_Returns504()
|
||||||
|
{
|
||||||
|
using var client = ClientWith(new StubHandler(async (_, ct) =>
|
||||||
|
{
|
||||||
|
await Task.Delay(Timeout.Infinite, ct);
|
||||||
|
return new HttpResponseMessage(HttpStatusCode.OK);
|
||||||
|
}));
|
||||||
|
|
||||||
|
var response = await client.SendCommandAsync("ListSites", new { }, TimeSpan.FromMilliseconds(50));
|
||||||
|
|
||||||
|
Assert.Equal(504, response.StatusCode);
|
||||||
|
Assert.Equal("TIMEOUT", response.ErrorCode);
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -0,0 +1,76 @@
|
|||||||
|
using Microsoft.AspNetCore.Antiforgery;
|
||||||
|
using Microsoft.AspNetCore.Builder;
|
||||||
|
using Microsoft.AspNetCore.Http;
|
||||||
|
using Microsoft.AspNetCore.Routing;
|
||||||
|
using Microsoft.Extensions.DependencyInjection;
|
||||||
|
using ScadaLink.CentralUI.Auth;
|
||||||
|
|
||||||
|
namespace ScadaLink.CentralUI.Tests.Auth;
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Regression tests for CentralUI-017. <c>POST /auth/logout</c> called
|
||||||
|
/// <c>.DisableAntiforgery()</c> and a plain <c>GET /logout</c> route also
|
||||||
|
/// signed the user out — either could be triggered cross-site to forcibly log
|
||||||
|
/// a user out. Logout is a state-changing authenticated action and must be
|
||||||
|
/// CSRF-protected: the POST keeps antiforgery enabled and the state-changing
|
||||||
|
/// GET route is removed.
|
||||||
|
/// </summary>
|
||||||
|
public class AuthEndpointsCsrfTests
|
||||||
|
{
|
||||||
|
private static IReadOnlyList<RouteEndpoint> BuildEndpoints()
|
||||||
|
{
|
||||||
|
var builder = WebApplication.CreateBuilder();
|
||||||
|
builder.Services.AddRouting();
|
||||||
|
builder.Services.AddAntiforgery();
|
||||||
|
var app = builder.Build();
|
||||||
|
app.MapAuthEndpoints();
|
||||||
|
|
||||||
|
return ((IEndpointRouteBuilder)app).DataSources
|
||||||
|
.SelectMany(ds => ds.Endpoints)
|
||||||
|
.OfType<RouteEndpoint>()
|
||||||
|
.ToList();
|
||||||
|
}
|
||||||
|
|
||||||
|
private static RouteEndpoint? Find(IReadOnlyList<RouteEndpoint> endpoints, string pattern, string method)
|
||||||
|
=> endpoints.FirstOrDefault(e =>
|
||||||
|
e.RoutePattern.RawText == pattern &&
|
||||||
|
(e.Metadata.GetMetadata<HttpMethodMetadata>()?.HttpMethods.Contains(method) ?? false));
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void PostAuthLogout_DoesNotDisableAntiforgery()
|
||||||
|
{
|
||||||
|
var endpoints = BuildEndpoints();
|
||||||
|
var logout = Find(endpoints, "/auth/logout", "POST");
|
||||||
|
|
||||||
|
Assert.NotNull(logout);
|
||||||
|
// DisableAntiforgery() leaves an IAntiforgeryMetadata with
|
||||||
|
// RequiresValidation == false. A CSRF-protected POST has either no such
|
||||||
|
// metadata, or metadata that still requires validation.
|
||||||
|
var antiforgery = logout!.Metadata.GetMetadata<IAntiforgeryMetadata>();
|
||||||
|
Assert.True(antiforgery is null || antiforgery.RequiresValidation,
|
||||||
|
"POST /auth/logout must keep antiforgery validation enabled.");
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void GetLogout_StateChangingRoute_IsRemoved()
|
||||||
|
{
|
||||||
|
var endpoints = BuildEndpoints();
|
||||||
|
var getLogout = Find(endpoints, "/logout", "GET");
|
||||||
|
|
||||||
|
Assert.Null(getLogout);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void PostAuthLogin_StillDisablesAntiforgery_PreAuthIsAcceptable()
|
||||||
|
{
|
||||||
|
// Login is a pre-auth endpoint; disabling antiforgery there is acceptable
|
||||||
|
// and intentional. This pins that the fix did not over-correct.
|
||||||
|
var endpoints = BuildEndpoints();
|
||||||
|
var login = Find(endpoints, "/auth/login", "POST");
|
||||||
|
|
||||||
|
Assert.NotNull(login);
|
||||||
|
var antiforgery = login!.Metadata.GetMetadata<IAntiforgeryMetadata>();
|
||||||
|
Assert.NotNull(antiforgery);
|
||||||
|
Assert.False(antiforgery!.RequiresValidation);
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -0,0 +1,68 @@
|
|||||||
|
using Bunit;
|
||||||
|
using Microsoft.AspNetCore.Components;
|
||||||
|
using ScadaLink.CentralUI.Components.Shared;
|
||||||
|
|
||||||
|
namespace ScadaLink.CentralUI.Tests.Shared;
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Regression tests for CentralUI-016. <c>DataTable</c> looped
|
||||||
|
/// <c>for i = 1..totalPages</c> and emitted one numbered <c><li></c>
|
||||||
|
/// button per page; a few thousand records at page size 25 rendered hundreds
|
||||||
|
/// of buttons into the diff on every state change. The fix windows the pager
|
||||||
|
/// so only first / last / a small range around the current page render.
|
||||||
|
/// </summary>
|
||||||
|
public class DataTablePagerTests : BunitContext
|
||||||
|
{
|
||||||
|
private IRenderedComponent<DataTable<int>> RenderTable(int itemCount, int pageSize = 25)
|
||||||
|
{
|
||||||
|
return Render<DataTable<int>>(parameters => parameters
|
||||||
|
.Add(p => p.Items, Enumerable.Range(1, itemCount).ToList())
|
||||||
|
.Add(p => p.PageSize, pageSize)
|
||||||
|
.Add(p => p.ShowSearch, false)
|
||||||
|
.Add(p => p.HeaderContent, (RenderFragment)(b => b.AddMarkupContent(0, "<th>N</th>")))
|
||||||
|
.Add(p => p.RowContent, (RenderFragment<int>)(item => b => b.AddMarkupContent(0, $"<tr><td>{item}</td></tr>"))));
|
||||||
|
}
|
||||||
|
|
||||||
|
private static int NumberedPageButtons(IRenderedComponent<DataTable<int>> cut)
|
||||||
|
=> cut.FindAll("ul.pagination li.page-item button")
|
||||||
|
.Count(b => int.TryParse(b.TextContent.Trim(), out _));
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void Pager_WithThousandsOfPages_RendersWindowedNotEveryPage()
|
||||||
|
{
|
||||||
|
// 5000 items / 25 = 200 pages. The pre-fix pager rendered 200 numbered
|
||||||
|
// buttons; the windowed pager renders at most a dozen.
|
||||||
|
var cut = RenderTable(itemCount: 5000);
|
||||||
|
|
||||||
|
var numbered = NumberedPageButtons(cut);
|
||||||
|
|
||||||
|
Assert.True(numbered <= 12,
|
||||||
|
$"Expected a windowed pager (<= 12 numbered buttons) but rendered {numbered}.");
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void Pager_SmallDataset_StillRendersEveryPage()
|
||||||
|
{
|
||||||
|
// 5 pages — small enough to render all numbered buttons (no windowing harm).
|
||||||
|
var cut = RenderTable(itemCount: 125);
|
||||||
|
|
||||||
|
var numbered = NumberedPageButtons(cut);
|
||||||
|
|
||||||
|
Assert.Equal(5, numbered);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void Pager_WindowedAroundCurrentPage_AlwaysIncludesFirstAndLast()
|
||||||
|
{
|
||||||
|
var cut = RenderTable(itemCount: 5000); // 200 pages
|
||||||
|
|
||||||
|
var numbered = cut.FindAll("ul.pagination li.page-item button")
|
||||||
|
.Select(b => b.TextContent.Trim())
|
||||||
|
.Where(t => int.TryParse(t, out _))
|
||||||
|
.ToList();
|
||||||
|
|
||||||
|
// First and last page are always reachable from the windowed pager.
|
||||||
|
Assert.Contains("1", numbered);
|
||||||
|
Assert.Contains("200", numbered);
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -0,0 +1,117 @@
|
|||||||
|
using ScadaLink.CentralUI.Components.Shared;
|
||||||
|
|
||||||
|
namespace ScadaLink.CentralUI.Tests.Shared;
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Characterization tests for CentralUI-015 (re-triaged Won't Fix — see
|
||||||
|
/// findings.md). The finding claimed <c>ContinueWith(..., TaskScheduler.Default)</c>
|
||||||
|
/// made callers resume off the render thread; that premise is incorrect — an
|
||||||
|
/// <c>await</c> always resumes on the awaiter's own captured
|
||||||
|
/// <see cref="SynchronizationContext"/> regardless of where the awaited task
|
||||||
|
/// completes. <c>ConfirmAsync_AwaiterResumesOnItsCapturedSyncContext</c> pins
|
||||||
|
/// that correct behaviour (it passes against both the old <c>ContinueWith</c>
|
||||||
|
/// form and the current inline-projection form). The remaining tests pin the
|
||||||
|
/// dialog result-resolution contract.
|
||||||
|
/// </summary>
|
||||||
|
public class DialogServiceThreadingTests
|
||||||
|
{
|
||||||
|
/// <summary>
|
||||||
|
/// A single-threaded sync context that records every posted callback —
|
||||||
|
/// stands in for the Blazor renderer's dispatcher.
|
||||||
|
/// </summary>
|
||||||
|
private sealed class TrackingSyncContext : SynchronizationContext
|
||||||
|
{
|
||||||
|
private readonly Thread _thread;
|
||||||
|
private readonly System.Collections.Concurrent.BlockingCollection<(SendOrPostCallback, object?)> _queue = new();
|
||||||
|
public int PostedCount;
|
||||||
|
|
||||||
|
public TrackingSyncContext()
|
||||||
|
{
|
||||||
|
_thread = new Thread(() =>
|
||||||
|
{
|
||||||
|
SetSynchronizationContext(this);
|
||||||
|
foreach (var (cb, st) in _queue.GetConsumingEnumerable())
|
||||||
|
{
|
||||||
|
cb(st);
|
||||||
|
}
|
||||||
|
}) { IsBackground = true };
|
||||||
|
_thread.Start();
|
||||||
|
}
|
||||||
|
|
||||||
|
public override void Post(SendOrPostCallback d, object? state)
|
||||||
|
{
|
||||||
|
Interlocked.Increment(ref PostedCount);
|
||||||
|
_queue.Add((d, state));
|
||||||
|
}
|
||||||
|
|
||||||
|
public void Complete() => _queue.CompleteAdding();
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task ConfirmAsync_AwaiterResumesOnItsCapturedSyncContext()
|
||||||
|
{
|
||||||
|
var service = new DialogService();
|
||||||
|
var ctx = new TrackingSyncContext();
|
||||||
|
|
||||||
|
// Run the awaiting "component" code on the tracking context.
|
||||||
|
var done = new TaskCompletionSource<int>();
|
||||||
|
ctx.Post(async void (_) =>
|
||||||
|
{
|
||||||
|
try
|
||||||
|
{
|
||||||
|
var task = service.ConfirmAsync("t", "m");
|
||||||
|
// Resolve from another thread, mimicking the host dispatching.
|
||||||
|
_ = Task.Run(() => service.Resolve(true));
|
||||||
|
await task;
|
||||||
|
// The continuation after the await must be back on the tracking
|
||||||
|
// context's single thread.
|
||||||
|
done.SetResult(Environment.CurrentManagedThreadId);
|
||||||
|
}
|
||||||
|
catch (Exception ex)
|
||||||
|
{
|
||||||
|
done.SetException(ex);
|
||||||
|
}
|
||||||
|
}, null);
|
||||||
|
|
||||||
|
var resumeThreadId = await done.Task;
|
||||||
|
ctx.Complete();
|
||||||
|
|
||||||
|
// The continuation was posted to (and ran on) the captured context.
|
||||||
|
Assert.True(ctx.PostedCount >= 1,
|
||||||
|
"ConfirmAsync continuation must post back to the caller's SynchronizationContext.");
|
||||||
|
Assert.NotEqual(Environment.CurrentManagedThreadId, resumeThreadId);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task ConfirmAsync_ResolvesWithExpectedValue()
|
||||||
|
{
|
||||||
|
var service = new DialogService();
|
||||||
|
|
||||||
|
var task = service.ConfirmAsync("t", "m");
|
||||||
|
service.Resolve(true);
|
||||||
|
|
||||||
|
Assert.True(await task);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task PromptAsync_ResolvesWithExpectedValue()
|
||||||
|
{
|
||||||
|
var service = new DialogService();
|
||||||
|
|
||||||
|
var task = service.PromptAsync("t", "label");
|
||||||
|
service.Resolve("typed value");
|
||||||
|
|
||||||
|
Assert.Equal("typed value", await task);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task PromptAsync_CancelledResolvesToNull()
|
||||||
|
{
|
||||||
|
var service = new DialogService();
|
||||||
|
|
||||||
|
var task = service.PromptAsync("t", "label");
|
||||||
|
service.Resolve(null);
|
||||||
|
|
||||||
|
Assert.Null(await task);
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -0,0 +1,77 @@
|
|||||||
|
using Bunit;
|
||||||
|
using Microsoft.Extensions.DependencyInjection;
|
||||||
|
using Microsoft.Extensions.Logging;
|
||||||
|
using Microsoft.JSInterop;
|
||||||
|
using ScadaLink.CentralUI.Components.Shared;
|
||||||
|
|
||||||
|
namespace ScadaLink.CentralUI.Tests.Shared;
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Regression tests for CentralUI-018. <c>MonacoEditor</c> wrapped every JS
|
||||||
|
/// interop call in a bare <c>try { ... } catch { }</c> with no logging — a
|
||||||
|
/// genuine Monaco init failure became invisible. The fix narrows the catch to
|
||||||
|
/// the expected prerender / disconnect cases and logs any real
|
||||||
|
/// <see cref="JSException"/> via <c>ILogger</c>.
|
||||||
|
/// </summary>
|
||||||
|
public class MonacoEditorLoggingTests : BunitContext
|
||||||
|
{
|
||||||
|
/// <summary>Captures log entries so the test can assert on them.</summary>
|
||||||
|
private sealed class CapturingLoggerProvider : ILoggerProvider
|
||||||
|
{
|
||||||
|
public List<(LogLevel Level, string Message, Exception? Exception)> Entries { get; } = new();
|
||||||
|
|
||||||
|
public ILogger CreateLogger(string categoryName) => new CapturingLogger(Entries);
|
||||||
|
public void Dispose() { }
|
||||||
|
|
||||||
|
private sealed class CapturingLogger : ILogger
|
||||||
|
{
|
||||||
|
private readonly List<(LogLevel, string, Exception?)> _entries;
|
||||||
|
public CapturingLogger(List<(LogLevel, string, Exception?)> entries) => _entries = entries;
|
||||||
|
|
||||||
|
public IDisposable? BeginScope<TState>(TState state) where TState : notnull => null;
|
||||||
|
public bool IsEnabled(LogLevel logLevel) => true;
|
||||||
|
|
||||||
|
public void Log<TState>(LogLevel logLevel, EventId eventId, TState state,
|
||||||
|
Exception? exception, Func<TState, Exception?, string> formatter)
|
||||||
|
=> _entries.Add((logLevel, formatter(state, exception), exception));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void CreateEditor_GenuineJsException_IsLogged_NotSwallowed()
|
||||||
|
{
|
||||||
|
var provider = new CapturingLoggerProvider();
|
||||||
|
Services.AddLogging(b => b.AddProvider(provider));
|
||||||
|
|
||||||
|
// createEditor is an InvokeVoidAsync call — configure it to throw a
|
||||||
|
// genuine JSException so we exercise the real-failure path.
|
||||||
|
JSInterop.Mode = JSRuntimeMode.Strict;
|
||||||
|
JSInterop.SetupVoid("MonacoBlazor.createEditor", _ => true)
|
||||||
|
.SetException(new JSException("Monaco failed to load"));
|
||||||
|
|
||||||
|
// Pre-fix: the bare catch {} swallowed this with no trace. Post-fix:
|
||||||
|
// the component renders fine but the failure is logged.
|
||||||
|
var cut = Render<MonacoEditor>(p => p.Add(c => c.ShowToolbar, false));
|
||||||
|
|
||||||
|
var errors = provider.Entries.Where(e => e.Level == LogLevel.Error).ToList();
|
||||||
|
Assert.NotEmpty(errors);
|
||||||
|
Assert.Contains(errors, e => e.Exception is JSException);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void CreateEditor_Prerender_DoesNotLog()
|
||||||
|
{
|
||||||
|
// When JS interop is unavailable (prerender), createEditor throws
|
||||||
|
// InvalidOperationException — that is expected and must NOT be logged.
|
||||||
|
var provider = new CapturingLoggerProvider();
|
||||||
|
Services.AddLogging(b => b.AddProvider(provider));
|
||||||
|
|
||||||
|
JSInterop.Mode = JSRuntimeMode.Strict;
|
||||||
|
JSInterop.SetupVoid("MonacoBlazor.createEditor", _ => true)
|
||||||
|
.SetException(new InvalidOperationException("JS interop not available during prerender"));
|
||||||
|
|
||||||
|
var cut = Render<MonacoEditor>(p => p.Add(c => c.ShowToolbar, false));
|
||||||
|
|
||||||
|
Assert.DoesNotContain(provider.Entries, e => e.Level >= LogLevel.Warning);
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -0,0 +1,67 @@
|
|||||||
|
using ScadaLink.CentralUI.Components.Shared;
|
||||||
|
|
||||||
|
namespace ScadaLink.CentralUI.Tests.Shared;
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Unit tests for the <see cref="PagerWindow"/> helper introduced for
|
||||||
|
/// CentralUI-016 — windowed pagination that keeps the rendered button count
|
||||||
|
/// bounded regardless of total page count.
|
||||||
|
/// </summary>
|
||||||
|
public class PagerWindowTests
|
||||||
|
{
|
||||||
|
[Fact]
|
||||||
|
public void Build_SmallPageCount_ReturnsEveryPage_NoEllipsis()
|
||||||
|
{
|
||||||
|
var pages = PagerWindow.Build(currentPage: 3, totalPages: 5);
|
||||||
|
|
||||||
|
Assert.Equal(new[] { 1, 2, 3, 4, 5 }, pages);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void Build_LargePageCount_IsBounded_AndIncludesFirstAndLast()
|
||||||
|
{
|
||||||
|
var pages = PagerWindow.Build(currentPage: 100, totalPages: 200);
|
||||||
|
|
||||||
|
Assert.Contains(1, pages);
|
||||||
|
Assert.Contains(200, pages);
|
||||||
|
Assert.Contains(100, pages);
|
||||||
|
// First, ellipsis, window of 5, ellipsis, last — never the full 200.
|
||||||
|
Assert.True(pages.Count <= 12, $"Expected a bounded window but got {pages.Count} entries.");
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void Build_LargePageCount_InsertsEllipsisForGaps()
|
||||||
|
{
|
||||||
|
// 0 is the ellipsis sentinel.
|
||||||
|
var pages = PagerWindow.Build(currentPage: 100, totalPages: 200);
|
||||||
|
|
||||||
|
Assert.Contains(0, pages);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void Build_CurrentNearStart_NoLeadingEllipsis()
|
||||||
|
{
|
||||||
|
var pages = PagerWindow.Build(currentPage: 1, totalPages: 200);
|
||||||
|
|
||||||
|
// Pages 1..3 are contiguous from the start, so no ellipsis before them.
|
||||||
|
Assert.Equal(1, pages[0]);
|
||||||
|
Assert.NotEqual(0, pages[1]);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void Build_ClampsOutOfRangeCurrentPage()
|
||||||
|
{
|
||||||
|
var pages = PagerWindow.Build(currentPage: 999, totalPages: 200);
|
||||||
|
|
||||||
|
Assert.Contains(200, pages);
|
||||||
|
Assert.True(pages.Count <= 12);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Theory]
|
||||||
|
[InlineData(0)]
|
||||||
|
[InlineData(-3)]
|
||||||
|
public void Build_NonPositiveTotalPages_ReturnsEmpty(int totalPages)
|
||||||
|
{
|
||||||
|
Assert.Empty(PagerWindow.Build(currentPage: 1, totalPages: totalPages));
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -0,0 +1,62 @@
|
|||||||
|
using Bunit;
|
||||||
|
using Microsoft.AspNetCore.Components;
|
||||||
|
using ScadaLink.CentralUI.Components.Shared;
|
||||||
|
|
||||||
|
namespace ScadaLink.CentralUI.Tests.Shared;
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Regression tests for CentralUI-018. <c>TreeView</c>'s storage-restore path
|
||||||
|
/// called <c>JsonSerializer.Deserialize</c> on the raw <c>treeviewStorage</c>
|
||||||
|
/// payload outside any try block — a corrupt payload threw an uncaught
|
||||||
|
/// <c>JsonException</c> during <c>OnAfterRenderAsync</c>, breaking the
|
||||||
|
/// component. The fix guards the deserialize and ignores a corrupt payload.
|
||||||
|
/// </summary>
|
||||||
|
public class TreeViewStorageResilienceTests : BunitContext
|
||||||
|
{
|
||||||
|
private record TestNode(string Key, string Label, List<TestNode> Children);
|
||||||
|
|
||||||
|
private static List<TestNode> Roots() => new()
|
||||||
|
{
|
||||||
|
new("a", "Alpha", new() { new("a1", "Alpha-1", new()) }),
|
||||||
|
new("b", "Beta", new()),
|
||||||
|
};
|
||||||
|
|
||||||
|
private IRenderedComponent<TreeView<TestNode>> BuildTree()
|
||||||
|
=> Render<TreeView<TestNode>>(parameters => parameters
|
||||||
|
.Add(p => p.Items, Roots())
|
||||||
|
.Add(p => p.ChildrenSelector, n => n.Children)
|
||||||
|
.Add(p => p.HasChildrenSelector, n => n.Children.Count > 0)
|
||||||
|
.Add(p => p.KeySelector, n => n.Key)
|
||||||
|
.Add(p => p.NodeContent, (RenderFragment<TestNode>)(node => b =>
|
||||||
|
b.AddMarkupContent(0, $"<span>{node.Label}</span>")))
|
||||||
|
.Add(p => p.StorageKey, "corrupt-tree"));
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void StorageRestore_CorruptJsonPayload_DoesNotThrow_AndStillRenders()
|
||||||
|
{
|
||||||
|
// A garbage payload that is not valid JSON for a List<string>.
|
||||||
|
JSInterop.Setup<string?>("treeviewStorage.load", _ => true)
|
||||||
|
.SetResult("{not json at all]");
|
||||||
|
JSInterop.SetupVoid("treeviewStorage.save", _ => true);
|
||||||
|
|
||||||
|
// Pre-fix: OnAfterRenderAsync threw JsonException out of the unguarded
|
||||||
|
// Deserialize call. Post-fix: the corrupt payload is ignored.
|
||||||
|
var cut = BuildTree();
|
||||||
|
|
||||||
|
Assert.Contains("Alpha", cut.Markup);
|
||||||
|
Assert.Contains("Beta", cut.Markup);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void StorageRestore_WrongShapeJson_DoesNotThrow()
|
||||||
|
{
|
||||||
|
// Valid JSON, but not a List<string> — an object, not an array.
|
||||||
|
JSInterop.Setup<string?>("treeviewStorage.load", _ => true)
|
||||||
|
.SetResult("{\"unexpected\": true}");
|
||||||
|
JSInterop.SetupVoid("treeviewStorage.save", _ => true);
|
||||||
|
|
||||||
|
var cut = BuildTree();
|
||||||
|
|
||||||
|
Assert.Contains("Alpha", cut.Markup);
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -54,6 +54,9 @@ public class TopologyPageTests : BunitContext
|
|||||||
OperationLockTimeout = TimeSpan.FromSeconds(5)
|
OperationLockTimeout = TimeSpan.FromSeconds(5)
|
||||||
}));
|
}));
|
||||||
Services.AddSingleton<ILogger<DeploymentService>>(NullLogger<DeploymentService>.Instance);
|
Services.AddSingleton<ILogger<DeploymentService>>(NullLogger<DeploymentService>.Instance);
|
||||||
|
// DeploymentService gained a DiffService dependency (DeploymentManager
|
||||||
|
// contract change); register it so the page's DI graph resolves.
|
||||||
|
Services.AddScoped<ScadaLink.TemplateEngine.Flattening.DiffService>();
|
||||||
Services.AddScoped<DeploymentService>();
|
Services.AddScoped<DeploymentService>();
|
||||||
Services.AddScoped<AreaService>();
|
Services.AddScoped<AreaService>();
|
||||||
Services.AddScoped<InstanceService>();
|
Services.AddScoped<InstanceService>();
|
||||||
|
|||||||
@@ -35,6 +35,14 @@ public class ClusterOptionsTests
|
|||||||
Assert.Empty(options.SeedNodes);
|
Assert.Empty(options.SeedNodes);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void SectionName_IsTheExpectedAppSettingsSection()
|
||||||
|
{
|
||||||
|
// CI-005: ClusterOptions must expose a single-source-of-truth constant for
|
||||||
|
// its appsettings.json section so binding sites do not hard-code the string.
|
||||||
|
Assert.Equal("ScadaLink:Cluster", ClusterOptions.SectionName);
|
||||||
|
}
|
||||||
|
|
||||||
[Fact]
|
[Fact]
|
||||||
public void Properties_CanBeSetToCustomValues()
|
public void Properties_CanBeSetToCustomValues()
|
||||||
{
|
{
|
||||||
|
|||||||
+35
-7
@@ -135,19 +135,47 @@ public class OpcUaEndpointConfigSerializerTests
|
|||||||
Assert.Equal(1000, config.PublishingIntervalMs);
|
Assert.Equal(1000, config.PublishingIntervalMs);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void Deserialize_LegacyParsed_StatusIsLegacy()
|
||||||
|
{
|
||||||
|
var (_, isLegacy) = OpcUaEndpointConfigSerializer.Deserialize("""{"endpoint":"opc.tcp://x:4840"}""");
|
||||||
|
Assert.True(isLegacy);
|
||||||
|
|
||||||
|
var result = OpcUaEndpointConfigSerializer.Deserialize("""{"endpoint":"opc.tcp://x:4840"}""");
|
||||||
|
Assert.Equal(OpcUaConfigParseStatus.Legacy, result.Status);
|
||||||
|
}
|
||||||
|
|
||||||
[Theory]
|
[Theory]
|
||||||
[InlineData("not json at all")]
|
[InlineData("not json at all")]
|
||||||
[InlineData("[1,2,3]")]
|
[InlineData("[1,2,3]")]
|
||||||
[InlineData("{\"foo\":123}")]
|
|
||||||
[InlineData("\"just a string\"")]
|
[InlineData("\"just a string\"")]
|
||||||
public void Deserialize_Malformed_ReturnsDefaultsAsLegacy(string input)
|
public void Deserialize_Malformed_ReportsMalformedNotLegacy(string input)
|
||||||
{
|
{
|
||||||
var (config, isLegacy) = OpcUaEndpointConfigSerializer.Deserialize(input);
|
var result = OpcUaEndpointConfigSerializer.Deserialize(input);
|
||||||
|
|
||||||
Assert.True(isLegacy);
|
// Genuinely unparseable input must NOT be reported as a recoverable legacy row.
|
||||||
Assert.Equal("", config.EndpointUrl);
|
Assert.Equal(OpcUaConfigParseStatus.Malformed, result.Status);
|
||||||
Assert.Equal(60000, config.SessionTimeoutMs);
|
Assert.False(result.IsLegacy);
|
||||||
Assert.Null(config.Heartbeat);
|
Assert.Equal("", result.Config.EndpointUrl);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void Deserialize_ObjectWithoutEndpointUrl_ParsesAsLegacy()
|
||||||
|
{
|
||||||
|
// A flat object with unrecognized keys is still a parseable legacy row, not malformed.
|
||||||
|
var result = OpcUaEndpointConfigSerializer.Deserialize("{\"foo\":123}");
|
||||||
|
Assert.Equal(OpcUaConfigParseStatus.Legacy, result.Status);
|
||||||
|
Assert.True(result.IsLegacy);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void Deserialize_TwoElementDeconstruction_StillWorks()
|
||||||
|
{
|
||||||
|
// Backward-compat: existing callers deconstruct into (Config, IsLegacy).
|
||||||
|
var (config, isLegacy) = OpcUaEndpointConfigSerializer.Deserialize(
|
||||||
|
"""{"endpointUrl":"opc.tcp://x:4840"}""");
|
||||||
|
Assert.False(isLegacy);
|
||||||
|
Assert.Equal("opc.tcp://x:4840", config.EndpointUrl);
|
||||||
}
|
}
|
||||||
|
|
||||||
[Fact]
|
[Fact]
|
||||||
|
|||||||
@@ -1,3 +1,4 @@
|
|||||||
|
using System.Dynamic;
|
||||||
using System.Text.Json;
|
using System.Text.Json;
|
||||||
using ScadaLink.Commons.Types;
|
using ScadaLink.Commons.Types;
|
||||||
|
|
||||||
@@ -100,4 +101,47 @@ public class DynamicJsonElementTests
|
|||||||
Assert.Throws<Microsoft.CSharp.RuntimeBinder.RuntimeBinderException>(
|
Assert.Throws<Microsoft.CSharp.RuntimeBinder.RuntimeBinderException>(
|
||||||
() => { var _ = obj.doesNotExist; });
|
() => { var _ = obj.doesNotExist; });
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ── Commons-006 regression: TryConvert(object) must never null out a present value ──
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void TryConvert_ObjectTarget_OnPresentValue_ReturnsNonNull()
|
||||||
|
{
|
||||||
|
// Directly exercise the DynamicObject.TryConvert contract for an `object`
|
||||||
|
// target: a present JSON object/array/string must not convert to null.
|
||||||
|
using var objDoc = JsonDocument.Parse("""{ "x": 1 }""");
|
||||||
|
var objWrapper = new DynamicJsonElement(objDoc.RootElement);
|
||||||
|
var convBinder = (ConvertBinder)Microsoft.CSharp.RuntimeBinder.Binder.Convert(
|
||||||
|
Microsoft.CSharp.RuntimeBinder.CSharpBinderFlags.None, typeof(object), typeof(DynamicJsonElementTests));
|
||||||
|
|
||||||
|
Assert.True(objWrapper.TryConvert(convBinder, out var result));
|
||||||
|
Assert.NotNull(result);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void TryConvert_ObjectTarget_OnJsonNull_ReturnsNull()
|
||||||
|
{
|
||||||
|
// Only a genuinely null JSON value converts to a null object.
|
||||||
|
using var doc = JsonDocument.Parse("""{ "v": null }""");
|
||||||
|
var nullWrapper = new DynamicJsonElement(doc.RootElement.GetProperty("v"));
|
||||||
|
var convBinder = (ConvertBinder)Microsoft.CSharp.RuntimeBinder.Binder.Convert(
|
||||||
|
Microsoft.CSharp.RuntimeBinder.CSharpBinderFlags.None, typeof(object), typeof(DynamicJsonElementTests));
|
||||||
|
|
||||||
|
Assert.True(nullWrapper.TryConvert(convBinder, out var result));
|
||||||
|
Assert.Null(result);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void TryConvert_NonObjectTarget_OnUnconvertibleValue_ReportsFailure()
|
||||||
|
{
|
||||||
|
// Requesting int from a JSON string is genuinely unconvertible: TryConvert
|
||||||
|
// must report false rather than a null success.
|
||||||
|
using var doc = JsonDocument.Parse("""{ "s": "not-a-number" }""");
|
||||||
|
var strWrapper = new DynamicJsonElement(doc.RootElement.GetProperty("s"));
|
||||||
|
var convBinder = (ConvertBinder)Microsoft.CSharp.RuntimeBinder.Binder.Convert(
|
||||||
|
Microsoft.CSharp.RuntimeBinder.CSharpBinderFlags.None, typeof(int), typeof(DynamicJsonElementTests));
|
||||||
|
|
||||||
|
Assert.False(strWrapper.TryConvert(convBinder, out var result));
|
||||||
|
Assert.Null(result);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -0,0 +1,50 @@
|
|||||||
|
using ScadaLink.Commons.Types.Flattening;
|
||||||
|
using ScadaLink.Commons.Types.Scripts;
|
||||||
|
|
||||||
|
namespace ScadaLink.Commons.Tests.Types;
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Commons-010: coverage for the small computed-property logic on
|
||||||
|
/// <see cref="ConfigurationDiff"/> and <see cref="ScriptScope"/>.
|
||||||
|
/// </summary>
|
||||||
|
public class FlatteningAndScriptScopeTests
|
||||||
|
{
|
||||||
|
[Fact]
|
||||||
|
public void ConfigurationDiff_NoChanges_HasChangesIsFalse()
|
||||||
|
{
|
||||||
|
var diff = new ConfigurationDiff { InstanceUniqueName = "inst-1" };
|
||||||
|
|
||||||
|
Assert.False(diff.HasChanges);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void ConfigurationDiff_WithAttributeChange_HasChangesIsTrue()
|
||||||
|
{
|
||||||
|
var diff = new ConfigurationDiff
|
||||||
|
{
|
||||||
|
InstanceUniqueName = "inst-1",
|
||||||
|
AlarmChanges = new[]
|
||||||
|
{
|
||||||
|
new DiffEntry<ResolvedAlarm> { CanonicalName = "HiAlarm", ChangeType = DiffChangeType.Added }
|
||||||
|
}
|
||||||
|
};
|
||||||
|
|
||||||
|
Assert.True(diff.HasChanges);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void ScriptScope_Root_HasNoParent()
|
||||||
|
{
|
||||||
|
Assert.False(ScriptScope.Root.HasParent);
|
||||||
|
Assert.Null(ScriptScope.Root.ParentPath);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void ScriptScope_WithParentPath_HasParentIsTrue()
|
||||||
|
{
|
||||||
|
var scope = new ScriptScope("Pump1.Motor", "Pump1");
|
||||||
|
|
||||||
|
Assert.True(scope.HasParent);
|
||||||
|
Assert.Equal("Pump1", scope.ParentPath);
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -72,4 +72,20 @@ public class ResultTests
|
|||||||
Assert.True(result.IsSuccess);
|
Assert.True(result.IsSuccess);
|
||||||
Assert.Equal("hello", result.Value);
|
Assert.Equal("hello", result.Value);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ── Commons-011 regression: a failed Result must always carry a message ──
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void Failure_WithNullError_ShouldThrow()
|
||||||
|
{
|
||||||
|
Assert.Throws<ArgumentNullException>(() => Result<int>.Failure(null!));
|
||||||
|
}
|
||||||
|
|
||||||
|
[Theory]
|
||||||
|
[InlineData("")]
|
||||||
|
[InlineData(" ")]
|
||||||
|
public void Failure_WithBlankError_ShouldThrow(string error)
|
||||||
|
{
|
||||||
|
Assert.Throws<ArgumentException>(() => Result<int>.Failure(error));
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -0,0 +1,81 @@
|
|||||||
|
using System.Collections;
|
||||||
|
using ScadaLink.Commons.Types;
|
||||||
|
|
||||||
|
namespace ScadaLink.Commons.Tests.Types;
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Commons-010: coverage for <see cref="ScriptArgs.Normalize"/> — the script-call
|
||||||
|
/// parameter normalizer (dictionary / anonymous-object / primitive-rejection paths).
|
||||||
|
/// </summary>
|
||||||
|
public class ScriptArgsTests
|
||||||
|
{
|
||||||
|
[Fact]
|
||||||
|
public void Normalize_Null_ReturnsNull()
|
||||||
|
{
|
||||||
|
Assert.Null(ScriptArgs.Normalize(null));
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void Normalize_ReadOnlyDictionary_ReturnedAsIs()
|
||||||
|
{
|
||||||
|
IReadOnlyDictionary<string, object?> input =
|
||||||
|
new Dictionary<string, object?> { ["a"] = 1 };
|
||||||
|
|
||||||
|
var result = ScriptArgs.Normalize(input);
|
||||||
|
|
||||||
|
Assert.Same(input, result);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void Normalize_PlainDictionary_ReturnedAsIs()
|
||||||
|
{
|
||||||
|
// Dictionary<string,object?> implements IReadOnlyDictionary, so it matches the
|
||||||
|
// first switch arm and is returned by reference (no defensive copy).
|
||||||
|
var input = new Dictionary<string, object?> { ["a"] = 1 };
|
||||||
|
|
||||||
|
var result = ScriptArgs.Normalize(input);
|
||||||
|
|
||||||
|
Assert.Same(input, result);
|
||||||
|
Assert.Equal(1, result!["a"]);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void Normalize_NonGenericDictionary_KeysStringified()
|
||||||
|
{
|
||||||
|
IDictionary raw = new Hashtable { [42] = "answer" };
|
||||||
|
|
||||||
|
var result = ScriptArgs.Normalize(raw);
|
||||||
|
|
||||||
|
Assert.Equal("answer", result!["42"]);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void Normalize_AnonymousObject_PropertiesBecomeEntries()
|
||||||
|
{
|
||||||
|
var result = ScriptArgs.Normalize(new { name = "Bob", count = 3 });
|
||||||
|
|
||||||
|
Assert.Equal("Bob", result!["name"]);
|
||||||
|
Assert.Equal(3, result["count"]);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Theory]
|
||||||
|
[InlineData(42)]
|
||||||
|
[InlineData(true)]
|
||||||
|
[InlineData(3.14)]
|
||||||
|
public void Normalize_Primitive_Throws(object primitive)
|
||||||
|
{
|
||||||
|
Assert.Throws<ArgumentException>(() => ScriptArgs.Normalize(primitive));
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void Normalize_String_Throws()
|
||||||
|
{
|
||||||
|
Assert.Throws<ArgumentException>(() => ScriptArgs.Normalize("hello"));
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void Normalize_Decimal_Throws()
|
||||||
|
{
|
||||||
|
Assert.Throws<ArgumentException>(() => ScriptArgs.Normalize(9.99m));
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -0,0 +1,80 @@
|
|||||||
|
using System.Globalization;
|
||||||
|
using ScadaLink.Commons.Types;
|
||||||
|
|
||||||
|
namespace ScadaLink.Commons.Tests.Types;
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Tests for <see cref="ValueFormatter"/>. Includes the Commons-012 regression:
|
||||||
|
/// formatting must be culture-invariant because the formatter feeds non-UI contexts
|
||||||
|
/// (gRPC stream events, logs) where locale-dependent output would be inconsistent.
|
||||||
|
/// </summary>
|
||||||
|
public class ValueFormatterTests
|
||||||
|
{
|
||||||
|
[Fact]
|
||||||
|
public void FormatDisplayValue_Null_ReturnsEmptyString()
|
||||||
|
{
|
||||||
|
Assert.Equal("", ValueFormatter.FormatDisplayValue(null));
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void FormatDisplayValue_String_ReturnsValueUnchanged()
|
||||||
|
{
|
||||||
|
Assert.Equal("hello", ValueFormatter.FormatDisplayValue("hello"));
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void FormatDisplayValue_Collection_JoinsWithComma()
|
||||||
|
{
|
||||||
|
Assert.Equal("1,2,3", ValueFormatter.FormatDisplayValue(new[] { 1, 2, 3 }));
|
||||||
|
}
|
||||||
|
|
||||||
|
// ── Commons-012 regression: culture-invariant numeric/date formatting ──
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void FormatDisplayValue_Double_UsesInvariantCulture_RegardlessOfThreadCulture()
|
||||||
|
{
|
||||||
|
var original = CultureInfo.CurrentCulture;
|
||||||
|
try
|
||||||
|
{
|
||||||
|
// German uses a comma as the decimal separator; invariant uses a dot.
|
||||||
|
CultureInfo.CurrentCulture = new CultureInfo("de-DE");
|
||||||
|
Assert.Equal("3.14", ValueFormatter.FormatDisplayValue(3.14));
|
||||||
|
}
|
||||||
|
finally
|
||||||
|
{
|
||||||
|
CultureInfo.CurrentCulture = original;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void FormatDisplayValue_DateTime_UsesInvariantCulture_RegardlessOfThreadCulture()
|
||||||
|
{
|
||||||
|
var original = CultureInfo.CurrentCulture;
|
||||||
|
try
|
||||||
|
{
|
||||||
|
CultureInfo.CurrentCulture = new CultureInfo("de-DE");
|
||||||
|
var dt = new DateTime(2026, 5, 16, 0, 0, 0, DateTimeKind.Utc);
|
||||||
|
var invariant = dt.ToString(CultureInfo.InvariantCulture);
|
||||||
|
Assert.Equal(invariant, ValueFormatter.FormatDisplayValue(dt));
|
||||||
|
}
|
||||||
|
finally
|
||||||
|
{
|
||||||
|
CultureInfo.CurrentCulture = original;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void FormatDisplayValue_CollectionOfDoubles_UsesInvariantCulture()
|
||||||
|
{
|
||||||
|
var original = CultureInfo.CurrentCulture;
|
||||||
|
try
|
||||||
|
{
|
||||||
|
CultureInfo.CurrentCulture = new CultureInfo("de-DE");
|
||||||
|
Assert.Equal("1.5,2.5", ValueFormatter.FormatDisplayValue(new[] { 1.5, 2.5 }));
|
||||||
|
}
|
||||||
|
finally
|
||||||
|
{
|
||||||
|
CultureInfo.CurrentCulture = original;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -222,6 +222,35 @@ public class CentralCommunicationActorTests : TestKit
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void MalformedSiteAddress_DoesNotAbortRefresh_OtherSitesStillRegistered()
|
||||||
|
{
|
||||||
|
// Regression test for Communication-009. HandleSiteAddressCacheLoaded calls
|
||||||
|
// ActorPath.Parse for every site in a single loop. A malformed NodeAAddress
|
||||||
|
// throws inside that loop; before the fix the whole refresh aborted partway
|
||||||
|
// through, leaving the cache half-updated (some sites registered, others not).
|
||||||
|
// The fix wraps the parse in a try/catch that logs and skips the bad site so
|
||||||
|
// a single garbage row cannot starve every other site of its ClusterClient.
|
||||||
|
var goodSite = CreateSite("good-site", "akka.tcp://scadalink@host1:8082");
|
||||||
|
// A garbage address that ActorPath.Parse rejects.
|
||||||
|
var badSite = CreateSite("bad-site", "this is not a valid actor path !!!");
|
||||||
|
|
||||||
|
// Order the bad site first so a non-resilient loop aborts before reaching good-site.
|
||||||
|
var (actor, _, siteProbes) = CreateActorWithMockRepo(new[] { badSite, goodSite });
|
||||||
|
|
||||||
|
Thread.Sleep(1000);
|
||||||
|
|
||||||
|
// good-site must still be registered and routable despite bad-site failing to parse.
|
||||||
|
var cmd = new DeployInstanceCommand(
|
||||||
|
"dep1", "inst1", "hash1", "{}", "admin", DateTimeOffset.UtcNow);
|
||||||
|
actor.Tell(new SiteEnvelope("good-site", cmd));
|
||||||
|
|
||||||
|
Assert.True(siteProbes.ContainsKey("good-site"),
|
||||||
|
"good-site should have a ClusterClient even though bad-site's address is malformed");
|
||||||
|
var msg = siteProbes["good-site"].ExpectMsg<ClusterClient.Send>();
|
||||||
|
Assert.Equal("dep1", ((DeployInstanceCommand)msg.Message).DeploymentId);
|
||||||
|
}
|
||||||
|
|
||||||
[Fact]
|
[Fact]
|
||||||
public void BothContactPoints_UsedInSingleClient()
|
public void BothContactPoints_UsedInSingleClient()
|
||||||
{
|
{
|
||||||
|
|||||||
Reference in New Issue
Block a user