Skip to content

Commit eef7fc0

Browse files
committed
tidy up JSON serialization
1 parent 72ebe57 commit eef7fc0

5 files changed

Lines changed: 28 additions & 9 deletions

File tree

.github/copilot-instructions.md

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,25 @@ Configured in `Directory.Build.props`: `IDE1006`, `IDE0079`, `IDE0042`, `CS0162`
6464
- **No magic strings in log messages**: When a log message references an enum value, class name, or other identifiable symbol, pass it via `nameof()` as a template argument rather than embedding it as a literal string in the message template.
6565
- **Avoid `nameof()` as label-only template parameters**: Do not use `nameof()` to inject property/type names as separate structured log parameters just to avoid a literal label — this clutters structured log output (Grafana, Loki, OpenTelemetry) with useless fields. Instead, use the property name as plain text in the message template and reserve `{Braces}` for actual values. E.g. `"{ClassName} ServiceFamily={ServiceFamily}"` with args `nameof(MyService), config.Value.ServiceFamily` — not `"{ClassName} {ServiceFamily}={ServiceFamilyValue}"` with args `nameof(MyService), nameof(MyConfig.ServiceFamily), config.Value.ServiceFamily`.
6666
- **`[LoggerMessage]` on hot paths**: Code inside tight loops, `Channel` readers, stream consumers, tick processors, and sink iterators must use `[LoggerMessage]`-attributed source-generated logging to eliminate allocations from `params object[]` boxing and interpolation. Declare `private static partial void` methods at the bottom of the partial class (or in a `{ClassName}.Logging.cs` file for larger services). Use `ILogger logger` as the first parameter (not `this ILogger` — that requires a top-level static class). Call sites use the static form: `LogXxx(logger, ...)` / `LogXxx(_logger, ...)`. For services with a primary constructor `ILogger<T> logger` parameter, pass `logger` directly; for traditional constructors, pass the `_logger` field. Leave dynamic-level calls (`logger.Log(level, ...)`) unconverted — `[LoggerMessage]` requires a compile-time constant level.
67+
- **Logging belongs in services, not controllers**: Domain-specific logging (`LogInformation` with request-specific fields) must live in the service method, not the controller. Controllers should not inject `ILogger` unless they perform work that cannot be delegated (e.g. SSE streaming loops with `LogTrace`).
68+
69+
### Controllers / Web API
70+
71+
- **Thin controllers**: Controllers must be pure pass-through — no business logic, no LINQ projections, no dictionary lookups, no logging. All domain logic and structured logging belongs in the service layer. A controller method should delegate to a single service call, map the result to an HTTP response type, and nothing else.
72+
- **No `ILogger` in pass-through controllers**: If every method in a controller simply delegates to a service, remove the `ILogger` injection entirely. The service layer owns observability.
73+
- **Expression-bodied methods**: Thin pass-through methods that are a single expression (or a single `await` + return) should use expression bodies (`=>`). For methods that branch on a nullable result (NotFound vs Ok), use a ternary with pattern matching.
74+
- **`<inheritdoc cref="..."/>` on controller methods**: When a controller method is a thin pass-through, use `/// <inheritdoc cref="ServiceType.Method(ParamTypes)"/>` referencing the service method's XML docs. Do not duplicate documentation between the controller and the service.
75+
- **Typed service methods over generic**: Controllers must not call generic base-class methods (e.g. `GetEntities<T>(tableName, ...)`, `Enqueue<T>(obj, ...)`) directly. Instead, add typed methods to the service interface that encapsulate domain knowledge (table names, queue keys) and include domain-specific logging. This keeps controllers ignorant of infrastructure details.
76+
- **Nullable returns for NotFound patterns**: Service methods consumed by controllers that may return HTTP 404 should use nullable return types (e.g. `PAFPlot<int>?`) rather than throwing exceptions. The controller uses pattern matching to map the result:
77+
78+
```csharp
79+
public Results<Ok<PAFPlot<int>>, NotFound> GetPlot(Symbol symbol, string name)
80+
=> pafSvc.TryGetPlot(symbol, name) is { } plot
81+
? TypedResults.Ok(plot)
82+
: TypedResults.NotFound();
83+
```
84+
85+
- **`<example>` tags on DTOs**: All public properties on Web API request/response DTOs should have `/// <example>value</example>` XML doc tags. Swashbuckle uses these to populate example values in the Swagger UI, improving API discoverability.
6786

6887
### Disposable Resources
6988

src/CasCap.Api.SignalCli.Tests/SignalCliJsonRpcClientServiceTests.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ public void JsonRpcNotification_WithDataMessage_DeserializesCorrectly()
6262
}
6363
""";
6464

65-
var notification = System.Text.Json.JsonSerializer.Deserialize<SignalCliJsonRpcNotification>(json);
65+
var notification = json.FromJson<SignalCliJsonRpcNotification>();
6666

6767
Assert.NotNull(notification);
6868
Assert.Equal("2.0", notification.JsonRpc);
@@ -80,7 +80,7 @@ public void JsonRpcNotification_WithNullParams_DoesNotThrow()
8080
{
8181
const string json = """{"jsonrpc":"2.0","method":"receive"}""";
8282

83-
var notification = System.Text.Json.JsonSerializer.Deserialize<SignalCliJsonRpcNotification>(json);
83+
var notification = json.FromJson<SignalCliJsonRpcNotification>();
8484

8585
Assert.NotNull(notification);
8686
Assert.Null(notification.Params);

src/CasCap.Api.Wiz/Services/WizClientService.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ public async Task<IReadOnlyDictionary<string, WizBulb>> DiscoverBulbsAsync(Cance
5555
{
5656
var result = await udpClient.ReceiveAsync(cts.Token);
5757
var json = Encoding.UTF8.GetString(result.Buffer);
58-
var response = JsonSerializer.Deserialize<WizResponse<WizPilotState>>(json, s_jsonOptions);
58+
var response = json.FromJson<WizResponse<WizPilotState>>(s_jsonOptions);
5959
var ip = result.RemoteEndPoint.Address.ToString();
6060

6161
if (response?.Result is not null)
@@ -147,7 +147,7 @@ public async Task<bool> SetPilotAsync(string bulbIp, WizSetPilotRequest request,
147147
{
148148
var result = await udpClient.ReceiveAsync(cts.Token);
149149
var json = Encoding.UTF8.GetString(result.Buffer);
150-
return JsonSerializer.Deserialize<WizResponse<T>>(json, s_jsonOptions);
150+
return json.FromJson<WizResponse<T>>(s_jsonOptions);
151151
}
152152
catch (OperationCanceledException)
153153
{

src/CasCap.App.Console/ConsoleApp.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -842,7 +842,7 @@ internal static async IAsyncEnumerable<AgentResponseUpdate> AgentRunStreamingMid
842842
null => "null",
843843
string s when IsValidJson(s) => s,
844844
_ when result.ToString() is { } raw && IsValidJson(raw) => raw,
845-
_ => JsonSerializer.Serialize(result, s_jsonOptions),
845+
_ => result.ToJson(s_jsonOptions),
846846
};
847847

848848
// Detect base64-encoded image bytes in the result and render as a CanvasImage renderable
@@ -923,7 +923,7 @@ private static void LogOutgoingMessages(string middlewareName, IEnumerable<ChatM
923923
}).ToList(),
924924
}).ToList();
925925

926-
var json = JsonSerializer.Serialize(payload, s_jsonOptions);
926+
var json = payload.ToJson(s_jsonOptions);
927927
LogMiddleware(new Panel(new JsonText(json))
928928
.Header($"[blue]{Markup.Escape(middlewareName)}[/] [grey]({messageList.Count} messages)[/]")
929929
.Border(BoxBorder.Rounded)
@@ -957,7 +957,7 @@ private static void LogOutgoingMessages(string middlewareName, IEnumerable<ChatM
957957

958958
if (filtered.Count > 0)
959959
{
960-
var optionsJson = JsonSerializer.Serialize(filtered, s_jsonOptions);
960+
var optionsJson = filtered.ToJson(s_jsonOptions);
961961
LogMiddleware(new Panel(new JsonText(optionsJson))
962962
.Header($"[blue]{Markup.Escape(middlewareName)}[/] [grey](ChatOptions)[/]")
963963
.Border(BoxBorder.Rounded)
@@ -1038,7 +1038,7 @@ private static string TruncateBytesProperty(string json)
10381038
}
10391039
filtered[prop.Name] = prop.Value.Clone();
10401040
}
1041-
return JsonSerializer.Serialize(filtered, s_jsonOptions);
1041+
return filtered.ToJson(s_jsonOptions);
10421042
}
10431043
catch
10441044
{

src/CasCap.SmartHaus/Services/CommunicationsBgService.Messaging.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ private async Task ProcessDataMessageAsync(IReceivedNotification notification, C
8181
_logger.LogWarning(
8282
"{ClassName} received content-only message with no text or attachments from {Sender}, extension data keys: [{ExtensionKeys}], raw: {RawData}",
8383
nameof(CommunicationsBgService), notification.Sender, extensionKeys,
84-
dm.ExtensionData is not null ? JsonSerializer.Serialize(dm.ExtensionData) : "(empty)");
84+
dm.ExtensionData is not null ? dm.ExtensionData.ToJson() : "(empty)");
8585
return;
8686
}
8787

0 commit comments

Comments
 (0)