From 2a76be1f9430a86f08b7a6062f2e4640929e8ca6 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 21 May 2026 05:27:17 -0400 Subject: [PATCH] feat(audit): multi-value filters across ManagementService, CLI and Central UI --- src/ScadaLink.CLI/Commands/AuditCommands.cs | 36 ++++-- .../Commands/AuditQueryHelpers.cs | 47 ++++++-- src/ScadaLink.CLI/README.md | 13 +- .../Audit/AuditExportEndpoints.cs | 90 +++++++++----- .../Components/Audit/AuditQueryModel.cs | 49 ++++---- .../Pages/Audit/AuditLogPage.razor.cs | 114 +++++++++++++----- .../AuditEndpoints.cs | 92 +++++++++----- .../Commands/AuditQueryCommandTests.cs | 77 +++++++++++- .../Components/Audit/AuditFilterBarTests.cs | 50 +++++++- .../Pages/AuditLogPageExportUrlTests.cs | 18 +++ .../AuditEndpointsTests.cs | 83 +++++++++++++ 11 files changed, 523 insertions(+), 146 deletions(-) diff --git a/src/ScadaLink.CLI/Commands/AuditCommands.cs b/src/ScadaLink.CLI/Commands/AuditCommands.cs index b411b91..57cd91d 100644 --- a/src/ScadaLink.CLI/Commands/AuditCommands.cs +++ b/src/ScadaLink.CLI/Commands/AuditCommands.cs @@ -26,16 +26,36 @@ public static class AuditCommands { var sinceOption = new Option("--since") { Description = "Start time: relative (1h, 24h, 7d) or ISO-8601" }; var untilOption = new Option("--until") { Description = "End time: relative (1h, 24h, 7d) or ISO-8601" }; - var channelOption = new Option("--channel") { Description = "Filter by channel (ApiOutbound, DbOutbound, Notification, ApiInbound)" }; + // --channel/--kind/--status/--site are multi-valued: System.CommandLine accepts + // both repeated tokens (--channel A --channel B) and, with + // AllowMultipleArgumentsPerToken, a single token carrying several values + // (--channel A B). AcceptOnlyFromAmong validates EACH supplied value. + var channelOption = new Option("--channel") + { + Description = "Filter by channel (ApiOutbound, DbOutbound, Notification, ApiInbound); repeatable", + AllowMultipleArgumentsPerToken = true, + }; channelOption.AcceptOnlyFromAmong("ApiOutbound", "DbOutbound", "Notification", "ApiInbound"); - var kindOption = new Option("--kind") { Description = "Filter by event kind (ApiCall, ApiCallCached, DbWrite, DbWriteCached, NotifySend, NotifyDeliver, InboundRequest, InboundAuthFailure, CachedSubmit, CachedResolve)" }; + var kindOption = new Option("--kind") + { + Description = "Filter by event kind (ApiCall, ApiCallCached, DbWrite, DbWriteCached, NotifySend, NotifyDeliver, InboundRequest, InboundAuthFailure, CachedSubmit, CachedResolve); repeatable", + AllowMultipleArgumentsPerToken = true, + }; kindOption.AcceptOnlyFromAmong( "ApiCall", "ApiCallCached", "DbWrite", "DbWriteCached", "NotifySend", "NotifyDeliver", "InboundRequest", "InboundAuthFailure", "CachedSubmit", "CachedResolve"); - var statusOption = new Option("--status") { Description = "Filter by status (Submitted, Forwarded, Attempted, Delivered, Failed, Parked, Discarded, Skipped)" }; + var statusOption = new Option("--status") + { + Description = "Filter by status (Submitted, Forwarded, Attempted, Delivered, Failed, Parked, Discarded, Skipped); repeatable", + AllowMultipleArgumentsPerToken = true, + }; statusOption.AcceptOnlyFromAmong( "Submitted", "Forwarded", "Attempted", "Delivered", "Failed", "Parked", "Discarded", "Skipped"); - var siteOption = new Option("--site") { Description = "Filter by source site ID" }; + var siteOption = new Option("--site") + { + Description = "Filter by source site ID; repeatable", + AllowMultipleArgumentsPerToken = true, + }; var targetOption = new Option("--target") { Description = "Filter by target (external system, DB connection, notification list)" }; var actorOption = new Option("--actor") { Description = "Filter by actor" }; var correlationIdOption = new Option("--correlation-id") { Description = "Filter by correlation ID" }; @@ -74,10 +94,10 @@ public static class AuditCommands { Since = result.GetValue(sinceOption), Until = result.GetValue(untilOption), - Channel = result.GetValue(channelOption), - Kind = result.GetValue(kindOption), - Status = result.GetValue(statusOption), - Site = result.GetValue(siteOption), + Channel = result.GetValue(channelOption) ?? Array.Empty(), + Kind = result.GetValue(kindOption) ?? Array.Empty(), + Status = result.GetValue(statusOption) ?? Array.Empty(), + Site = result.GetValue(siteOption) ?? Array.Empty(), Target = result.GetValue(targetOption), Actor = result.GetValue(actorOption), CorrelationId = result.GetValue(correlationIdOption), diff --git a/src/ScadaLink.CLI/Commands/AuditQueryHelpers.cs b/src/ScadaLink.CLI/Commands/AuditQueryHelpers.cs index f18f971..39918f5 100644 --- a/src/ScadaLink.CLI/Commands/AuditQueryHelpers.cs +++ b/src/ScadaLink.CLI/Commands/AuditQueryHelpers.cs @@ -9,15 +9,18 @@ namespace ScadaLink.CLI.Commands; /// Filter arguments for an audit query invocation. Mirrors the Bundle B /// GET /api/audit/query filter parameters; / /// are time-specs (relative like 1h/7d, or absolute ISO-8601). +/// /// +/// are multi-valued — each supplied value becomes a repeated query-string param so +/// the server's multi-value IN (…) filter sees the full set. /// public sealed class AuditQueryArgs { public string? Since { get; set; } public string? Until { get; set; } - public string? Channel { get; set; } - public string? Kind { get; set; } - public string? Status { get; set; } - public string? Site { get; set; } + public string[] Channel { get; set; } = Array.Empty(); + public string[] Kind { get; set; } = Array.Empty(); + public string[] Status { get; set; } = Array.Empty(); + public string[] Site { get; set; } = Array.Empty(); public string? Target { get; set; } public string? Actor { get; set; } public string? CorrelationId { get; set; } @@ -73,8 +76,11 @@ public static class AuditQueryHelpers /// /// Builds the ?... query string for GET /api/audit/query from the filter - /// args plus an optional keyset cursor. Unset filters are omitted. --errors-only - /// maps to status=Failed (the server takes a single status value). + /// args plus an optional keyset cursor. Unset filters are omitted. The multi-valued + /// --channel/--kind/--status/--site filters each emit ONE + /// repeated query-string key per value (e.g. channel=A&channel=B) so the + /// server's multi-value IN (…) filter receives the full set. --errors-only + /// maps to a single status=Failed and overrides any explicit --status. /// public static string BuildQueryString( AuditQueryArgs args, DateTimeOffset now, DateTimeOffset? afterOccurredAtUtc, string? afterEventId) @@ -87,20 +93,35 @@ public static class AuditQueryHelpers parts.Add($"{key}={Uri.EscapeDataString(value)}"); } + void AddEach(string key, IReadOnlyList values) + { + foreach (var value in values) + { + Add(key, value); + } + } + if (!string.IsNullOrWhiteSpace(args.Since)) Add("fromUtc", ResolveTimeSpec(args.Since!, now).ToString("o", CultureInfo.InvariantCulture)); if (!string.IsNullOrWhiteSpace(args.Until)) Add("toUtc", ResolveTimeSpec(args.Until!, now).ToString("o", CultureInfo.InvariantCulture)); - Add("channel", args.Channel); - Add("kind", args.Kind); + AddEach("channel", args.Channel); + AddEach("kind", args.Kind); - // --errors-only is a convenience shorthand for the single-value Failed status - // filter. The server's status filter accepts one value, so --errors-only and an - // explicit --status are mutually exclusive in effect; --errors-only wins. - Add("status", args.ErrorsOnly ? "Failed" : args.Status); + // --errors-only is a convenience shorthand for the Failed status filter. The + // server's status filter is multi-value, but --errors-only stays a single-status + // override: it pins status=Failed and supersedes any explicit --status values. + if (args.ErrorsOnly) + { + Add("status", "Failed"); + } + else + { + AddEach("status", args.Status); + } - Add("sourceSiteId", args.Site); + AddEach("sourceSiteId", args.Site); Add("target", args.Target); Add("actor", args.Actor); Add("correlationId", args.CorrelationId); diff --git a/src/ScadaLink.CLI/README.md b/src/ScadaLink.CLI/README.md index eed1ccf..d17e794 100644 --- a/src/ScadaLink.CLI/README.md +++ b/src/ScadaLink.CLI/README.md @@ -1078,10 +1078,10 @@ scadalink --url audit query [options] |--------|----------|---------|-------------| | `--since` | no | — | Start time: relative (`1h`, `24h`, `7d`) or ISO-8601 | | `--until` | no | — | End time: relative (`1h`, `24h`, `7d`) or ISO-8601 | -| `--channel` | no | — | Filter by channel (`ApiOutbound`, `DbOutbound`, `Notification`, `ApiInbound`) | -| `--kind` | no | — | Filter by event kind (`ApiCall`, `ApiCallCached`, `DbWrite`, `DbWriteCached`, `NotifySend`, `NotifyDeliver`, `InboundRequest`, `InboundAuthFailure`, `CachedSubmit`, `CachedResolve`) | -| `--status` | no | — | Filter by status (`Submitted`, `Forwarded`, `Attempted`, `Delivered`, `Failed`, `Parked`, `Discarded`, `Skipped`) | -| `--site` | no | — | Filter by source site ID | +| `--channel` | no | — | Filter by channel (`ApiOutbound`, `DbOutbound`, `Notification`, `ApiInbound`); repeatable — multiple values are OR-combined | +| `--kind` | no | — | Filter by event kind (`ApiCall`, `ApiCallCached`, `DbWrite`, `DbWriteCached`, `NotifySend`, `NotifyDeliver`, `InboundRequest`, `InboundAuthFailure`, `CachedSubmit`, `CachedResolve`); repeatable — multiple values are OR-combined | +| `--status` | no | — | Filter by status (`Submitted`, `Forwarded`, `Attempted`, `Delivered`, `Failed`, `Parked`, `Discarded`, `Skipped`); repeatable — multiple values are OR-combined | +| `--site` | no | — | Filter by source site ID; repeatable — multiple values are OR-combined | | `--target` | no | — | Filter by target (external system, DB connection, notification list) | | `--actor` | no | — | Filter by actor | | `--correlation-id` | no | — | Filter by correlation ID | @@ -1090,6 +1090,11 @@ scadalink --url audit query [options] | `--all` | no | `false` | Fetch every page, following the keyset cursor | | `--format` | no | `json` | Output format: `json` (JSONL, one event per line) or `table` | +The `--channel`/`--kind`/`--status`/`--site` filters accept multiple values — +either as repeated flags (`--channel ApiOutbound --channel DbOutbound`) or +space-separated after one flag (`--channel ApiOutbound DbOutbound`). Values +within one filter are OR-combined; the different filters are AND-combined. + With `--format table`, events render as an aligned text table with columns `OccurredAtUtc`, `Channel`, `Kind`, `Status`, `Target`, `Actor`, `DurationMs`, `HttpStatus`; long `Target`/`Actor` values are truncated with an ellipsis. With diff --git a/src/ScadaLink.CentralUI/Audit/AuditExportEndpoints.cs b/src/ScadaLink.CentralUI/Audit/AuditExportEndpoints.cs index 4047763..64b02aa 100644 --- a/src/ScadaLink.CentralUI/Audit/AuditExportEndpoints.cs +++ b/src/ScadaLink.CentralUI/Audit/AuditExportEndpoints.cs @@ -74,34 +74,21 @@ public static class AuditExportEndpoints } /// - /// Parses the query-string into an . - /// Unknown enum names / un-parseable Guids / dates are silently dropped - /// (same contract as AuditLogPage.ApplyQueryStringFilters). + /// Parses the query-string into an . The + /// channel/kind/status/site dimensions are + /// multi-value: a repeated query param yields a multi-element filter list, a + /// single param a one-element list. Unknown enum names / un-parseable Guids / + /// dates are silently dropped (same lax contract as + /// AuditLogPage.ApplyQueryStringFilters) — an unparseable value within + /// a repeated set is dropped, not the whole set. /// internal static AuditLogQueryFilter ParseFilter(IQueryCollection query) { - AuditChannel? channel = null; - if (query.TryGetValue("channel", out var channelValues) - && Enum.TryParse(channelValues.ToString(), ignoreCase: true, out var parsedChannel)) - { - channel = parsedChannel; - } + var channels = ParseEnumList(query, "channel"); + var kinds = ParseEnumList(query, "kind"); + var statuses = ParseEnumList(query, "status"); + var sites = ParseStringList(query, "site"); - AuditKind? kind = null; - if (query.TryGetValue("kind", out var kindValues) - && Enum.TryParse(kindValues.ToString(), ignoreCase: true, out var parsedKind)) - { - kind = parsedKind; - } - - AuditStatus? status = null; - if (query.TryGetValue("status", out var statusValues) - && Enum.TryParse(statusValues.ToString(), ignoreCase: true, out var parsedStatus)) - { - status = parsedStatus; - } - - string? site = TrimToNullable(query, "site"); string? target = TrimToNullable(query, "target"); string? actor = TrimToNullable(query, "actor"); @@ -116,10 +103,10 @@ public static class AuditExportEndpoints DateTime? toUtc = ParseUtcDate(query, "to"); return new AuditLogQueryFilter( - Channels: channel is { } c ? new[] { c } : null, - Kinds: kind is { } k ? new[] { k } : null, - Statuses: status is { } s ? new[] { s } : null, - SourceSiteIds: site is { } siteId ? new[] { siteId } : null, + Channels: channels, + Kinds: kinds, + Statuses: statuses, + SourceSiteIds: sites, Target: target, Actor: actor, CorrelationId: correlationId, @@ -127,6 +114,53 @@ public static class AuditExportEndpoints ToUtc: toUtc); } + /// + /// Reads EVERY value of a (possibly repeated) query param and parses each as + /// , dropping unparseable values silently. Returns + /// null when the param is absent or no value parsed. + /// + private static IReadOnlyList? ParseEnumList(IQueryCollection query, string key) + where TEnum : struct, Enum + { + if (!query.TryGetValue(key, out var values)) + { + return null; + } + + var parsed = new List(); + foreach (var raw in values) + { + if (Enum.TryParse(raw, ignoreCase: true, out var value)) + { + parsed.Add(value); + } + } + return parsed.Count > 0 ? parsed : null; + } + + /// + /// Reads EVERY value of a (possibly repeated) query param, trims each, and + /// drops blank entries. Returns null when the param is absent or every + /// value was blank. + /// + private static IReadOnlyList? ParseStringList(IQueryCollection query, string key) + { + if (!query.TryGetValue(key, out var values)) + { + return null; + } + + var parsed = new List(); + foreach (var raw in values) + { + if (!string.IsNullOrWhiteSpace(raw)) + { + parsed.Add(raw.Trim()); + } + } + return parsed.Count > 0 ? parsed : null; + } + /// /// Optional maxRows= query-string override. Falls back to /// on a missing / non-positive / unparseable diff --git a/src/ScadaLink.CentralUI/Components/Audit/AuditQueryModel.cs b/src/ScadaLink.CentralUI/Components/Audit/AuditQueryModel.cs index f902c90..08e8582 100644 --- a/src/ScadaLink.CentralUI/Components/Audit/AuditQueryModel.cs +++ b/src/ScadaLink.CentralUI/Components/Audit/AuditQueryModel.cs @@ -15,20 +15,20 @@ namespace ScadaLink.CentralUI.Components.Audit; /// /// /// -/// The repository filter contract () is single-value -/// per dimension today; the chip multi-selects therefore collapse to the FIRST -/// selected chip when the model is published via . That is a -/// deliberate Bundle B scope decision — the chip UI is preserved so a follow-up can -/// either repeat the query per chip or widen the filter contract without rewriting -/// the form. Instance and Script free-text are also UI-only today: the underlying -/// filter has no matching columns, so they are dropped during collapse. +/// The repository filter contract () is multi-value +/// per dimension: the chip multi-selects map straight through to the +/// Channels / Kinds / Statuses / SourceSiteIds filter +/// lists when the model is published via — an empty set means +/// "do not constrain". Instance and Script free-text remain UI-only: the underlying +/// filter has no matching columns, so they are dropped when the model is published. /// /// /// /// The Errors-only toggle is a convenience: when true AND no explicit Status chips -/// are selected, the collapsed filter pins (the -/// first of {Failed, Parked, Discarded}). When Status chips ARE selected the toggle -/// is a no-op — the explicit Status filter wins. +/// are selected, targets the full error-status set +/// {, , +/// }. When Status chips ARE selected the toggle +/// is a no-op — the explicit Status chips win. /// /// public sealed class AuditQueryModel @@ -104,20 +104,21 @@ public sealed class AuditQueryModel } /// - /// Collapses this UI model to the repository's single-value filter. - /// See class doc for the multi-select → single-value contract. + /// Publishes this UI model as the repository's multi-value filter: each chip + /// multi-select maps straight through to its filter list (an empty set yields + /// null — "do not constrain"). See class doc for the Errors-only rule. /// public AuditLogQueryFilter ToFilter(DateTime utcNow) { - var status = ResolveStatus(); + var statuses = ResolveStatuses(); var (fromUtc, toUtc) = ResolveTimeWindow(utcNow); return new AuditLogQueryFilter( - Channels: Channels.Count > 0 ? new[] { Channels.First() } : null, - Kinds: Kinds.Count > 0 ? new[] { Kinds.First() } : null, - Statuses: status is { } s ? new[] { s } : null, - SourceSiteIds: SiteIdentifiers.Count > 0 ? new[] { SiteIdentifiers.First() } : null, + Channels: Channels.Count > 0 ? Channels.ToArray() : null, + Kinds: Kinds.Count > 0 ? Kinds.ToArray() : null, + Statuses: statuses, + SourceSiteIds: SiteIdentifiers.Count > 0 ? SiteIdentifiers.ToArray() : null, Target: string.IsNullOrWhiteSpace(TargetSearch) ? null : TargetSearch.Trim(), Actor: string.IsNullOrWhiteSpace(ActorSearch) ? null : ActorSearch.Trim(), CorrelationId: null, @@ -125,20 +126,22 @@ public sealed class AuditQueryModel ToUtc: toUtc); } - private AuditStatus? ResolveStatus() + /// The non-success statuses targeted by the Errors-only toggle. + private static readonly AuditStatus[] ErrorStatuses = + { AuditStatus.Failed, AuditStatus.Parked, AuditStatus.Discarded }; + + private IReadOnlyList? ResolveStatuses() { if (Statuses.Count > 0) { // Explicit chips win — Errors-only is a no-op. - return Statuses.First(); + return Statuses.ToArray(); } if (ErrorsOnly) { - // Single-value filter contract: Failed is the lead non-success status. - // When the filter widens to multi-value the full {Failed, Parked, Discarded} - // set will flow through. - return AuditStatus.Failed; + // Multi-value filter: Errors-only targets the full non-success set. + return ErrorStatuses; } return null; diff --git a/src/ScadaLink.CentralUI/Components/Pages/Audit/AuditLogPage.razor.cs b/src/ScadaLink.CentralUI/Components/Pages/Audit/AuditLogPage.razor.cs index 9653150..66092e2 100644 --- a/src/ScadaLink.CentralUI/Components/Pages/Audit/AuditLogPage.razor.cs +++ b/src/ScadaLink.CentralUI/Components/Pages/Audit/AuditLogPage.razor.cs @@ -80,33 +80,18 @@ public partial class AuditLogPage } } - string? site = null; - if (query.TryGetValue("site", out var siteValues)) - { - var v = siteValues.ToString(); - if (!string.IsNullOrWhiteSpace(v)) - { - site = v.Trim(); - } - } + // site/channel/status accept repeated params for symmetry with the + // multi-value export URL — a single ?site=/?channel=/?status= drill-in + // still works (one-element list). Unknown enum names are silently dropped. + IReadOnlyList? sites = ParseStringList(query, "site"); - AuditChannel? channel = null; - if (query.TryGetValue("channel", out var channelValues) - && Enum.TryParse(channelValues.ToString(), ignoreCase: true, out var parsedChannel)) - { - channel = parsedChannel; - } + IReadOnlyList? channels = ParseEnumList(query, "channel"); // Bundle E (M7-T13): the Health-dashboard Audit error-rate tile drills in // with ?status=Failed (and operators may craft URLs with Parked/Discarded). // Unknown values are silently dropped — the page still renders without // the constraint. - AuditStatus? status = null; - if (query.TryGetValue("status", out var statusValues) - && Enum.TryParse(statusValues.ToString(), ignoreCase: true, out var parsedStatus)) - { - status = parsedStatus; - } + IReadOnlyList? statuses = ParseEnumList(query, "status"); // Instance is UI-only — the filter contract has no matching column, so we // pass it as a separate seam to the filter bar. @@ -123,20 +108,68 @@ public partial class AuditLogPage // auto-loads. Pure ?instance= deep links (UI-only) do not trigger auto-load // because the filter contract has no instance column — the user still needs // to refine + Apply for those. - if (correlationId is null && target is null && actor is null && site is null && channel is null && status is null) + if (correlationId is null && target is null && actor is null && sites is null && channels is null && statuses is null) { return; } _currentFilter = new AuditLogQueryFilter( - Channels: channel is { } ch ? new[] { ch } : null, - Statuses: status is { } st ? new[] { st } : null, - SourceSiteIds: site is { } siteId ? new[] { siteId } : null, + Channels: channels, + Statuses: statuses, + SourceSiteIds: sites, Target: target, Actor: actor, CorrelationId: correlationId); } + /// + /// Reads EVERY value of a (possibly repeated) query param and parses each as + /// , dropping unparseable values silently. Returns + /// null when the param is absent or no value parsed. + /// + private static IReadOnlyList? ParseEnumList( + Dictionary query, string key) + where TEnum : struct, Enum + { + if (!query.TryGetValue(key, out var values)) + { + return null; + } + + var parsed = new List(); + foreach (var raw in values) + { + if (Enum.TryParse(raw, ignoreCase: true, out var value)) + { + parsed.Add(value); + } + } + return parsed.Count > 0 ? parsed : null; + } + + /// + /// Reads EVERY value of a (possibly repeated) query param, trims each, and + /// drops blank entries. Returns null when absent or all-blank. + /// + private static IReadOnlyList? ParseStringList( + Dictionary query, string key) + { + if (!query.TryGetValue(key, out var values)) + { + return null; + } + + var parsed = new List(); + foreach (var raw in values) + { + if (!string.IsNullOrWhiteSpace(raw)) + { + parsed.Add(raw.Trim()); + } + } + return parsed.Count > 0 ? parsed : null; + } + private void HandleFilterChanged(AuditLogQueryFilter filter) { // Always reassign — the grid keys reloads on reference change, so even a @@ -181,24 +214,39 @@ public partial class AuditLogPage } var parts = new List>(9); - // Task 8: the filter dimensions are multi-value now; the export query-string - // is still single-value, so emit the first selected value of each list. - // Task 9 widens the query-string contract to carry the full set. + // Task 9: the filter dimensions are multi-value end-to-end. Emit ONE + // repeated query-string key per selected value (channel=A&channel=B); the + // export endpoint's ParseFilter reads the full repeated set. if (filter.Channels is { Count: > 0 } channels) { - parts.Add(new("channel", channels[0].ToString())); + foreach (var channel in channels) + { + parts.Add(new("channel", channel.ToString())); + } } if (filter.Kinds is { Count: > 0 } kinds) { - parts.Add(new("kind", kinds[0].ToString())); + foreach (var kind in kinds) + { + parts.Add(new("kind", kind.ToString())); + } } if (filter.Statuses is { Count: > 0 } statuses) { - parts.Add(new("status", statuses[0].ToString())); + foreach (var status in statuses) + { + parts.Add(new("status", status.ToString())); + } } - if (filter.SourceSiteIds is { Count: > 0 } sourceSiteIds && !string.IsNullOrWhiteSpace(sourceSiteIds[0])) + if (filter.SourceSiteIds is { Count: > 0 } sourceSiteIds) { - parts.Add(new("site", sourceSiteIds[0])); + foreach (var site in sourceSiteIds) + { + if (!string.IsNullOrWhiteSpace(site)) + { + parts.Add(new("site", site)); + } + } } if (!string.IsNullOrWhiteSpace(filter.Target)) { diff --git a/src/ScadaLink.ManagementService/AuditEndpoints.cs b/src/ScadaLink.ManagementService/AuditEndpoints.cs index 0cf58c3..ed83f1c 100644 --- a/src/ScadaLink.ManagementService/AuditEndpoints.cs +++ b/src/ScadaLink.ManagementService/AuditEndpoints.cs @@ -367,32 +367,20 @@ public static class AuditEndpoints // ───────────────────────────────────────────────────────────────────── /// - /// Parses the query-string into an . Unknown - /// enum names / un-parseable Guids / dates are silently dropped (no 400) — - /// the same lax contract the CentralUI export endpoint uses. + /// Parses the query-string into an . The + /// channel/kind/status/sourceSiteId dimensions are + /// multi-value: a repeated query param (channel=A&channel=B) yields + /// a multi-element filter list, while a single param yields a one-element + /// list. Unknown enum names / un-parseable Guids / dates are silently dropped + /// (no 400) — the same lax contract the CentralUI export endpoint uses; an + /// unparseable value within a repeated set is dropped, not the whole set. /// public static AuditLogQueryFilter ParseFilter(IQueryCollection query) { - AuditChannel? channel = null; - if (query.TryGetValue("channel", out var channelValues) - && Enum.TryParse(channelValues.ToString(), ignoreCase: true, out var parsedChannel)) - { - channel = parsedChannel; - } - - AuditKind? kind = null; - if (query.TryGetValue("kind", out var kindValues) - && Enum.TryParse(kindValues.ToString(), ignoreCase: true, out var parsedKind)) - { - kind = parsedKind; - } - - AuditStatus? status = null; - if (query.TryGetValue("status", out var statusValues) - && Enum.TryParse(statusValues.ToString(), ignoreCase: true, out var parsedStatus)) - { - status = parsedStatus; - } + var channels = ParseEnumList(query, "channel"); + var kinds = ParseEnumList(query, "kind"); + var statuses = ParseEnumList(query, "status"); + var sourceSiteIds = ParseStringList(query, "sourceSiteId"); Guid? correlationId = null; if (query.TryGetValue("correlationId", out var corrValues) @@ -401,13 +389,11 @@ public static class AuditEndpoints correlationId = parsedCorr; } - var sourceSiteId = TrimToNullable(query, "sourceSiteId"); - return new AuditLogQueryFilter( - Channels: channel is { } c ? new[] { c } : null, - Kinds: kind is { } k ? new[] { k } : null, - Statuses: status is { } s ? new[] { s } : null, - SourceSiteIds: sourceSiteId is { } site ? new[] { site } : null, + Channels: channels, + Kinds: kinds, + Statuses: statuses, + SourceSiteIds: sourceSiteIds, Target: TrimToNullable(query, "target"), Actor: TrimToNullable(query, "actor"), CorrelationId: correlationId, @@ -415,6 +401,54 @@ public static class AuditEndpoints ToUtc: ParseUtcDate(query, "toUtc")); } + /// + /// Reads EVERY value of a (possibly repeated) query param and parses each as + /// , dropping unparseable values silently. Returns + /// null when the param is absent or no value parsed — so the filter + /// dimension stays unconstrained. + /// + private static IReadOnlyList? ParseEnumList(IQueryCollection query, string key) + where TEnum : struct, Enum + { + if (!query.TryGetValue(key, out var values)) + { + return null; + } + + var parsed = new List(); + foreach (var raw in values) + { + if (Enum.TryParse(raw, ignoreCase: true, out var value)) + { + parsed.Add(value); + } + } + return parsed.Count > 0 ? parsed : null; + } + + /// + /// Reads EVERY value of a (possibly repeated) query param, trims each, and + /// drops blank entries. Returns null when the param is absent or every + /// value was blank. + /// + private static IReadOnlyList? ParseStringList(IQueryCollection query, string key) + { + if (!query.TryGetValue(key, out var values)) + { + return null; + } + + var parsed = new List(); + foreach (var raw in values) + { + if (!string.IsNullOrWhiteSpace(raw)) + { + parsed.Add(raw.Trim()); + } + } + return parsed.Count > 0 ? parsed : null; + } + /// /// Parses the keyset-paging query parameters into an /// . pageSize is clamped to diff --git a/tests/ScadaLink.CLI.Tests/Commands/AuditQueryCommandTests.cs b/tests/ScadaLink.CLI.Tests/Commands/AuditQueryCommandTests.cs index b3840c5..80f0ecd 100644 --- a/tests/ScadaLink.CLI.Tests/Commands/AuditQueryCommandTests.cs +++ b/tests/ScadaLink.CLI.Tests/Commands/AuditQueryCommandTests.cs @@ -58,10 +58,10 @@ public class AuditQueryCommandTests { Since = "1h", Until = "2026-05-20T12:00:00Z", - Channel = "ApiOutbound", - Kind = "ApiCallCached", - Status = "Delivered", - Site = "site-1", + Channel = new[] { "ApiOutbound" }, + Kind = new[] { "ApiCallCached" }, + Status = new[] { "Delivered" }, + Site = new[] { "site-1" }, Target = "weather-api", Actor = "multi-role", CorrelationId = "abc-123", @@ -96,6 +96,43 @@ public class AuditQueryCommandTests Assert.Equal("Failed", parsed["status"]); } + [Fact] + public void BuildQueryString_MultiValueChannel_EmitsOneKeyPerValue() + { + var now = DateTimeOffset.UtcNow; + var args = new AuditQueryArgs + { + Channel = new[] { "ApiOutbound", "DbOutbound" }, + Status = new[] { "Failed", "Parked" }, + Site = new[] { "site-1", "site-2" }, + }; + + var qs = AuditQueryHelpers.BuildQueryString(args, now, null, null); + var parsed = HttpUtility.ParseQueryString(qs.TrimStart('?')); + + Assert.Equal(new[] { "ApiOutbound", "DbOutbound" }, parsed.GetValues("channel")); + Assert.Equal(new[] { "Failed", "Parked" }, parsed.GetValues("status")); + Assert.Equal(new[] { "site-1", "site-2" }, parsed.GetValues("sourceSiteId")); + } + + [Fact] + public void BuildQueryString_ErrorsOnly_OverridesExplicitStatusValues() + { + // --errors-only stays a single-status override: it pins status=Failed and + // supersedes any explicit (multi-value) --status selection. + var now = DateTimeOffset.UtcNow; + var args = new AuditQueryArgs + { + ErrorsOnly = true, + Status = new[] { "Delivered", "Parked" }, + }; + + var qs = AuditQueryHelpers.BuildQueryString(args, now, null, null); + var parsed = HttpUtility.ParseQueryString(qs.TrimStart('?')); + + Assert.Equal(new[] { "Failed" }, parsed.GetValues("status")); + } + [Fact] public void BuildQueryString_Cursor_AppendsAfterParameters() { @@ -254,6 +291,38 @@ public class AuditQueryCommandTests Assert.Empty(parse.Errors); } + [Fact] + public void Query_MultipleChannelValues_SingleToken_AreAccepted() + { + // AllowMultipleArgumentsPerToken: --channel A B parses as two values. + var root = AuditCommandTestHarness.BuildRoot(); + var parse = root.Parse(new[] { "audit", "query", "--channel", "ApiOutbound", "DbOutbound" }); + Assert.Empty(parse.Errors); + } + + [Fact] + public void Query_MultipleChannelValues_RepeatedFlag_AreAccepted() + { + // --channel A --channel B parses as two values. + var root = AuditCommandTestHarness.BuildRoot(); + var parse = root.Parse(new[] + { + "audit", "query", "--channel", "ApiOutbound", "--channel", "Notification", + }); + Assert.Empty(parse.Errors); + } + + [Fact] + public void Query_MultiValueChannel_WithOneInvalidName_FailsFast() + { + // AcceptOnlyFromAmong validates EACH value of the multi-value option. + var root = AuditCommandTestHarness.BuildRoot(); + var (exit, _, err) = AuditCommandTestHarness.Invoke( + root, "audit", "query", "--channel", "ApiOutbound", "OutboundApi"); + Assert.NotEqual(0, exit); + Assert.NotEqual("", err); + } + [Fact] public void Query_ChannelWithInvalidName_FailsFast_NonZeroExit() { diff --git a/tests/ScadaLink.CentralUI.Tests/Components/Audit/AuditFilterBarTests.cs b/tests/ScadaLink.CentralUI.Tests/Components/Audit/AuditFilterBarTests.cs index 0c8c623..484c2bb 100644 --- a/tests/ScadaLink.CentralUI.Tests/Components/Audit/AuditFilterBarTests.cs +++ b/tests/ScadaLink.CentralUI.Tests/Components/Audit/AuditFilterBarTests.cs @@ -77,12 +77,30 @@ public class AuditFilterBarTests : BunitContext cut.Find("[data-test=\"filter-apply\"]").Click(); Assert.NotNull(captured); - // Task 8: the filter dimension is multi-value now; ToFilter still collapses - // the chip selection to a single-element list (Task 9 widens that). Assert.Equal(new[] { AuditChannel.ApiOutbound }, captured!.Channels); Assert.Equal("Plant-A-OPC", captured.Target); } + [Fact] + public void Apply_WithMultipleChannelChips_PassesAllSelectedChannels() + { + // Task 9: ToFilter no longer collapses the chip multi-select — every + // selected channel chip reaches the filter's Channels list. + AuditLogQueryFilter? captured = null; + var cut = Render(p => p + .Add(c => c.OnFilterChanged, EventCallback.Factory.Create(this, f => captured = f))); + + cut.Find("[data-test=\"chip-channel-ApiOutbound\"]").Click(); + cut.Find("[data-test=\"chip-channel-Notification\"]").Click(); + cut.Find("[data-test=\"filter-apply\"]").Click(); + + Assert.NotNull(captured); + Assert.NotNull(captured!.Channels); + Assert.Equal(2, captured.Channels!.Count); + Assert.Contains(AuditChannel.ApiOutbound, captured.Channels); + Assert.Contains(AuditChannel.Notification, captured.Channels); + } + [Fact] public void Channel_Narrows_Kind_Options_When_Selected() { @@ -119,8 +137,12 @@ public class AuditFilterBarTests : BunitContext cut.Find("[data-test=\"filter-apply\"]").Click(); Assert.NotNull(captured); - // Single-value collapse contract (Task 8): Failed leads the non-success set. - Assert.Equal(new[] { AuditStatus.Failed }, captured!.Statuses); + // Task 9: Errors-only targets the full non-success set {Failed, Parked, Discarded}. + Assert.NotNull(captured!.Statuses); + Assert.Equal(3, captured.Statuses!.Count); + Assert.Contains(AuditStatus.Failed, captured.Statuses); + Assert.Contains(AuditStatus.Parked, captured.Statuses); + Assert.Contains(AuditStatus.Discarded, captured.Statuses); // Now pin an explicit Status chip — Errors-only must yield (chip wins). cut.Find("[data-test=\"chip-status-Delivered\"]").Click(); @@ -129,6 +151,26 @@ public class AuditFilterBarTests : BunitContext Assert.Equal(new[] { AuditStatus.Delivered }, captured!.Statuses); } + [Fact] + public void Apply_WithMultipleStatusChips_PassesAllSelectedStatuses() + { + // Task 9: multiple explicit Status chips all reach the filter — and they + // win over the Errors-only default. + AuditLogQueryFilter? captured = null; + var cut = Render(p => p + .Add(c => c.OnFilterChanged, EventCallback.Factory.Create(this, f => captured = f))); + + cut.Find("[data-test=\"chip-status-Delivered\"]").Click(); + cut.Find("[data-test=\"chip-status-Failed\"]").Click(); + cut.Find("[data-test=\"filter-apply\"]").Click(); + + Assert.NotNull(captured); + Assert.NotNull(captured!.Statuses); + Assert.Equal(2, captured.Statuses!.Count); + Assert.Contains(AuditStatus.Delivered, captured.Statuses); + Assert.Contains(AuditStatus.Failed, captured.Statuses); + } + [Fact] public void TimeRange_LastHour_PopulatesFromUtc_ApproxOneHourAgo() { diff --git a/tests/ScadaLink.CentralUI.Tests/Pages/AuditLogPageExportUrlTests.cs b/tests/ScadaLink.CentralUI.Tests/Pages/AuditLogPageExportUrlTests.cs index 6fad432..60e3d56 100644 --- a/tests/ScadaLink.CentralUI.Tests/Pages/AuditLogPageExportUrlTests.cs +++ b/tests/ScadaLink.CentralUI.Tests/Pages/AuditLogPageExportUrlTests.cs @@ -74,4 +74,22 @@ public class AuditLogPageExportUrlTests Assert.Single(query); Assert.Equal("Notification", query["channel"]); } + + [Fact] + public void BuildExportUrl_MultiValueDimensions_EmitRepeatedParams() + { + // Task 9: each multi-value dimension emits one repeated query-string key + // per selected value so the export endpoint's ParseFilter sees them all. + var filter = new AuditLogQueryFilter( + Channels: new[] { AuditChannel.ApiOutbound, AuditChannel.DbOutbound }, + Statuses: new[] { AuditStatus.Failed, AuditStatus.Parked }, + SourceSiteIds: new[] { "plant-a", "plant-b" }); + + var url = AuditLogPage.BuildExportUrl(filter); + + var query = QueryHelpers.ParseQuery(new Uri("http://x" + url).Query); + Assert.Equal(new[] { "ApiOutbound", "DbOutbound" }, query["channel"].ToArray()); + Assert.Equal(new[] { "Failed", "Parked" }, query["status"].ToArray()); + Assert.Equal(new[] { "plant-a", "plant-b" }, query["site"].ToArray()); + } } diff --git a/tests/ScadaLink.ManagementService.Tests/AuditEndpointsTests.cs b/tests/ScadaLink.ManagementService.Tests/AuditEndpointsTests.cs index ee2f0a3..80fb691 100644 --- a/tests/ScadaLink.ManagementService.Tests/AuditEndpointsTests.cs +++ b/tests/ScadaLink.ManagementService.Tests/AuditEndpointsTests.cs @@ -367,6 +367,89 @@ public class AuditEndpointsTests Assert.Equal(AuditEndpoints.MaxPageSize, paging.PageSize); } + [Fact] + public void ParseFilter_RepeatedParams_ParseIntoMultiValueLists() + { + // Repeated query params (channel=A&channel=B …) must widen to multi-value + // filter lists — one element per supplied value. + var query = new Microsoft.AspNetCore.Http.QueryCollection( + new Dictionary + { + ["channel"] = new[] { "ApiOutbound", "DbOutbound" }, + ["kind"] = new[] { "ApiCall", "DbWrite" }, + ["status"] = new[] { "Failed", "Parked" }, + ["sourceSiteId"] = new[] { "plant-a", "plant-b" }, + }); + + var filter = AuditEndpoints.ParseFilter(query); + + Assert.Equal(new[] { AuditChannel.ApiOutbound, AuditChannel.DbOutbound }, filter.Channels); + Assert.Equal(new[] { AuditKind.ApiCall, AuditKind.DbWrite }, filter.Kinds); + Assert.Equal(new[] { AuditStatus.Failed, AuditStatus.Parked }, filter.Statuses); + Assert.Equal(new[] { "plant-a", "plant-b" }, filter.SourceSiteIds); + } + + [Fact] + public void ParseFilter_SingleParam_ParsesIntoOneElementList() + { + // The single-valued contract still holds — one param yields a + // one-element list, not a scalar. + var query = new Microsoft.AspNetCore.Http.QueryCollection( + new Dictionary + { + ["channel"] = "ApiOutbound", + ["status"] = "Delivered", + }); + + var filter = AuditEndpoints.ParseFilter(query); + + Assert.Equal(new[] { AuditChannel.ApiOutbound }, filter.Channels); + Assert.Equal(new[] { AuditStatus.Delivered }, filter.Statuses); + Assert.Null(filter.Kinds); + Assert.Null(filter.SourceSiteIds); + } + + [Fact] + public void ParseFilter_UnparseableValuesInRepeatedSet_AreDroppedSilently() + { + // Lax-parse contract: an unrecognised enum name is dropped, the rest of + // the repeated set survives — no 400, no whole-set drop. + var query = new Microsoft.AspNetCore.Http.QueryCollection( + new Dictionary + { + ["channel"] = new[] { "ApiOutbound", "Bogus", "Notification" }, + ["status"] = new[] { "Nonsense" }, + }); + + var filter = AuditEndpoints.ParseFilter(query); + + Assert.Equal(new[] { AuditChannel.ApiOutbound, AuditChannel.Notification }, filter.Channels); + // Every value unparseable → the dimension stays unconstrained (null). + Assert.Null(filter.Statuses); + } + + [Fact] + public async Task Query_RepeatedChannelParams_ReachRepositoryAsMultiValueFilter() + { + // End-to-end: a repeated channel= query param must surface at the + // repository as a two-element Channels list. + var (client, repo, host) = await BuildHostAsync(roles: new[] { "Audit" }); + using (host) + { + var response = await client.SendAsync(Get( + "/api/audit/query?channel=ApiOutbound&channel=DbOutbound")); + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + + await repo.Received().QueryAsync( + Arg.Is(f => + f.Channels != null && f.Channels.Count == 2 && + f.Channels.Contains(AuditChannel.ApiOutbound) && + f.Channels.Contains(AuditChannel.DbOutbound)), + Arg.Any(), + Arg.Any()); + } + } + [Fact] public void ParsePaging_HalfSuppliedCursor_IsDropped() {