fix(management): audit-config query returned {} for every filter — project the repo ValueTuple into a named result record

Deferred-work register row 46. HandleQueryAuditLog returned ICentralUiRepository's
(IReadOnlyList<AuditLogEntry> Entries, int TotalCount) ValueTuple straight to
ManagementActor.SerializeResult. System.Text.Json writes properties only, and a
ValueTuple exposes Item1/Item2 as public FIELDS, so the whole payload serialized to
the literal "{}" — the CLI faithfully printed an empty object no matter how many
AuditLogEntries rows matched, with no error anywhere in the stack.

Fixed at the handler (the layer that leaks the tuple), not the CLI: new Commons record
AuditLogQueryResult(Entries, TotalCount), mirroring the SecuredWriteListResult shape
already used by the other paged management query.

Also guards the whole class of bug: SerializeResult now throws on any ITuple result,
so the next handler that returns a tuple fails loudly instead of silently emitting {}.

Tests (fail-before verified — the query test asserted exactly "{}"):
- QueryAuditLogCommand_ReturnsEntriesAndTotalCount
- SerializeResult_TupleResult_ThrowsInsteadOfEmittingEmptyObject
- SerializeResult_AuditLogQueryResult_EmitsEntriesAndTotalCount

Docs: Component-CLI.md + CLI README document the response shape and why it is a record.
This commit is contained in:
Joseph Doherty
2026-08-15 11:38:28 -04:00
parent c6769ee95e
commit 09108018f1
5 changed files with 167 additions and 3 deletions
+7
View File
@@ -241,6 +241,13 @@ warning; migrate scripts to `audit-config`.
scadabridge audit-config query [--user <username>] [--entity-type <type>] [--action <action>] [--from <date>] [--to <date>] [--page <n>] [--page-size <n>]
```
The response is an `AuditLogQueryResult` — `{ "entries": [ … ], "totalCount": <n> }`, where
`totalCount` is the unpaged match count. The result type is a named record specifically
because the handler must not return the repository's `(Entries, TotalCount)` ValueTuple:
`System.Text.Json` serializes properties only, so a tuple result renders as the literal
`{}` and every query looks empty regardless of how many rows matched. `ManagementActor.SerializeResult`
now rejects tuple results outright so the failure mode cannot recur silently.
The new centralized Audit Log component (#23) is exposed via the `scadabridge audit` group below.
### Centralized Audit Commands
+22
View File
@@ -1802,6 +1802,28 @@ scadabridge --url <url> audit-config query [options]
| `--page` | no | `1` | Page number |
| `--page-size` | no | `50` | Results per page |
Output shape:
```json
{
"entries": [
{
"id": 412,
"user": "multi-role",
"action": "Created",
"entityType": "Template",
"entityId": "7",
"entityName": "PumpTemplate",
"timestamp": "2026-08-15T12:00:00+00:00"
}
],
"totalCount": 137
}
```
`totalCount` is the unpaged number of rows matching the filters, so a client can page
without a second call.
---
### `shared-script` — Manage shared scripts
@@ -1,6 +1,25 @@
using ZB.MOM.WW.ScadaBridge.Commons.Entities.Audit;
namespace ZB.MOM.WW.ScadaBridge.Commons.Messages.Management;
public record QueryAuditLogCommand(
string? User = null, string? EntityType = null, string? Action = null,
DateTimeOffset? From = null, DateTimeOffset? To = null,
int Page = 1, int PageSize = 50);
/// <summary>
/// Result wrapper for <see cref="QueryAuditLogCommand"/>.
/// </summary>
/// <remarks>
/// This record exists specifically so the handler does not return the repository's
/// <c>(IReadOnlyList&lt;AuditLogEntry&gt; Entries, int TotalCount)</c> ValueTuple directly.
/// A ValueTuple exposes <c>Item1</c>/<c>Item2</c> as public <em>fields</em>, and
/// <see cref="System.Text.Json"/> serializes only properties unless
/// <c>JsonSerializerOptions.IncludeFields</c> is set — so a tuple result serialized to
/// the literal string <c>{}</c> and every <c>audit-config query</c> looked empty no
/// matter how many rows the query matched (deferred-work register row 46). Mirrors the
/// <c>SecuredWriteListResult</c> shape used by the other paged management query.
/// </remarks>
/// <param name="Entries">The page of configuration-change audit entries.</param>
/// <param name="TotalCount">Total number of rows matching the filters (unpaged).</param>
public record AuditLogQueryResult(IReadOnlyList<AuditLogEntry> Entries, int TotalCount);
@@ -90,10 +90,35 @@ public class ManagementActor : ReceiveActor
/// Serializes a command result to JSON using <see cref="System.Text.Json"/> — the same
/// serializer the HTTP endpoint uses — with cycle-safe settings.
/// </summary>
/// <remarks>
/// Rejects tuple results outright. <see cref="System.Text.Json"/> writes only
/// properties by default, and a <c>ValueTuple</c> exposes its members as public
/// <em>fields</em> — so a handler that returns a repository's
/// <c>(rows, totalCount)</c> tuple serializes to the literal <c>{}</c>, silently
/// handing every caller an empty result with no error anywhere in the stack. That is
/// exactly how <c>audit-config query</c> shipped broken (deferred-work register row
/// 46). Failing loudly here converts the whole class of bug into an immediate,
/// obvious error the first time the handler is exercised; the fix is always to
/// project the tuple into a named result record before returning it.
/// </remarks>
/// <param name="result">The command result object to serialize, or null.</param>
/// <returns>The JSON string representation of <paramref name="result"/>, or <c>"null"</c> when null.</returns>
public static string SerializeResult(object? result) =>
JsonSerializer.Serialize(result, ResultSerializerOptions);
/// <exception cref="InvalidOperationException">
/// Thrown when <paramref name="result"/> is a tuple, which cannot round-trip through
/// the JSON contract.
/// </exception>
public static string SerializeResult(object? result)
{
if (result is System.Runtime.CompilerServices.ITuple)
{
throw new InvalidOperationException(
$"Management command results must be named types, not tuples — "
+ $"System.Text.Json serializes '{result.GetType().Name}' to '{{}}' because tuple "
+ "members are fields, not properties. Project the tuple into a result record.");
}
return JsonSerializer.Serialize(result, ResultSerializerOptions);
}
private void HandleEnvelope(ManagementEnvelope envelope)
{
@@ -2578,12 +2603,24 @@ public class ManagementActor : ReceiveActor
// Audit Log handler
// ========================================================================
/// <summary>
/// Queries the configuration-change audit trail and projects the repository's
/// ValueTuple into <see cref="AuditLogQueryResult"/>.
/// </summary>
/// <remarks>
/// The projection is load-bearing, not cosmetic: returning the repository tuple
/// directly made <see cref="SerializeResult"/> emit the literal <c>{}</c> (ValueTuple
/// exposes <c>Item1</c>/<c>Item2</c> as fields, which System.Text.Json skips), so
/// <c>audit-config query</c> reported an empty result for every filter combination
/// regardless of how many rows matched — deferred-work register row 46.
/// </remarks>
private static async Task<object?> HandleQueryAuditLog(IServiceProvider sp, QueryAuditLogCommand cmd)
{
var repo = sp.GetRequiredService<ICentralUiRepository>();
return await repo.GetAuditLogEntriesAsync(
var (entries, totalCount) = await repo.GetAuditLogEntriesAsync(
cmd.User, cmd.EntityType, cmd.Action, cmd.From, cmd.To,
page: cmd.Page, pageSize: cmd.PageSize);
return new AuditLogQueryResult(entries, totalCount);
}
// ========================================================================
@@ -238,6 +238,85 @@ public class ManagementActorTests : TestKit, IDisposable
Assert.Equal(envelope.CorrelationId, response.CorrelationId);
}
// ========================================================================
// 2b. Configuration-change audit query -- result must not serialize to "{}"
// ========================================================================
/// <summary>
/// Deferred-work register row 46: <c>audit-config query</c> returned the literal
/// <c>{}</c> for every filter combination against the rig even though the
/// <c>AuditLogEntries</c> rows were present. The handler returned the repository's
/// <c>(Entries, TotalCount)</c> ValueTuple straight to <c>SerializeResult</c>, and
/// System.Text.Json writes properties only — a tuple's <c>Item1</c>/<c>Item2</c> are
/// fields, so the whole payload vanished with no error anywhere. Pins that the rows
/// and the total actually reach the wire.
/// </summary>
[Fact]
public void QueryAuditLogCommand_ReturnsEntriesAndTotalCount()
{
var uiRepo = Substitute.For<ICentralUiRepository>();
var entries = new List<Commons.Entities.Audit.AuditLogEntry>
{
new("alice", "Created", "Template", "7", "PumpTemplate")
{
Id = 1,
Timestamp = new DateTimeOffset(2026, 8, 15, 12, 0, 0, TimeSpan.Zero)
}
};
uiRepo.GetAuditLogEntriesAsync(
Arg.Any<string?>(), Arg.Any<string?>(), Arg.Any<string?>(),
Arg.Any<DateTimeOffset?>(), Arg.Any<DateTimeOffset?>(),
Arg.Any<string?>(), Arg.Any<string?>(), Arg.Any<Guid?>(),
Arg.Any<int>(), Arg.Any<int>(), Arg.Any<CancellationToken>())
.Returns(((IReadOnlyList<Commons.Entities.Audit.AuditLogEntry>)entries, 137));
_services.AddScoped(_ => uiRepo);
var actor = CreateActor();
var envelope = Envelope(new QueryAuditLogCommand(PageSize: 25), "Administrator");
actor.Tell(envelope);
var response = ExpectMsg<ManagementSuccess>(TimeSpan.FromSeconds(5));
Assert.NotEqual("{}", response.JsonData);
Assert.Contains("PumpTemplate", response.JsonData);
Assert.Contains("\"entries\"", response.JsonData);
Assert.Contains("\"totalCount\":137", response.JsonData);
}
/// <summary>
/// Guard for the whole class of bug behind row 46: any handler that returns a tuple
/// silently serializes to <c>{}</c>. <see cref="ManagementActor.SerializeResult"/>
/// now rejects tuples outright so the next occurrence is a loud failure rather than
/// an empty response.
/// </summary>
[Fact]
public void SerializeResult_TupleResult_ThrowsInsteadOfEmittingEmptyObject()
{
var ex = Assert.Throws<InvalidOperationException>(
() => ManagementActor.SerializeResult((new[] { "a" }, 1)));
Assert.Contains("tuple", ex.Message, StringComparison.OrdinalIgnoreCase);
}
/// <summary>
/// Pins the positive half of the guard: the named result record the handler now
/// returns serializes to a populated, camelCased object.
/// </summary>
[Fact]
public void SerializeResult_AuditLogQueryResult_EmitsEntriesAndTotalCount()
{
var json = ManagementActor.SerializeResult(new AuditLogQueryResult(
new List<Commons.Entities.Audit.AuditLogEntry>
{
new("bob", "Deleted", "Instance", "3", "Pump1")
},
42));
Assert.Contains("\"entries\"", json);
Assert.Contains("\"totalCount\":42", json);
Assert.Contains("Pump1", json);
}
// ========================================================================
// 3. Template list test -- verify data flows through
// ========================================================================