Use system text json source generators in boilerplate SignalR (#11406) #11407
Conversation
WalkthroughSignalR message payloads were standardized to Dictionary<string, string?> across client and server handlers. Server configured SignalR to use System.Text.Json source-generated contexts. Exception handling now returns AppProblemDetails instead of ProblemDetails. Minor formatting changes were applied, with no other public API changes besides these. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Server Controller
participant SignalR Hub
participant Client (AppClientCoordinator)
participant Notification UI
User->>Server Controller: Trigger action
Server Controller->>SignalR Hub: SendAsync("SHOW_MESSAGE", title, data: Dictionary<string,string?>, requireAction)
Note right of SignalR Hub: Payload uses source-generated JSON contexts
SignalR Hub-->>Client (AppClientCoordinator): SHOW_MESSAGE(title, Dictionary, bool)
Client (AppClientCoordinator)->>Notification UI: Show(title, data["pageUrl"], data["action"])
Notification UI-->>Client (AppClientCoordinator): success: bool
alt success
Client (AppClientCoordinator)-->>SignalR Hub: return true
else failure
Client (AppClientCoordinator)->>Notification UI: Show(title) // fallback
Client (AppClientCoordinator)-->>SignalR Hub: return false
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull Request Overview
This PR implements System.Text.Json source generators in the boilerplate's SignalR configuration to improve serialization performance and enable trimming support. The changes replace anonymous objects with strongly typed alternatives and configure SignalR to use the project's JSON source generators.
Key changes:
- Configure SignalR to use System.Text.Json source generators through JsonProtocol
- Replace anonymous objects in SignalR calls with Dictionary<string, string?> for better serialization
- Update exception handling to use AppProblemDetails instead of ProblemDetails
- Add explicit mapping for User objects in SignalR messages
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| Program.Services.cs | Configures SignalR with JSON source generators using TypeInfoResolverChain |
| ServerExceptionHandler.cs | Updates return types from ProblemDetails to AppProblemDetails |
| RoleManagementController.cs | Replaces anonymous object with Dictionary in SignalR call |
| DiagnosticsController.cs | Replaces anonymous object with Dictionary in SignalR call |
| AttachmentController.cs | Adds explicit mapping for User object in SignalR message |
| AppClientCoordinator.cs | Updates SignalR event handler to expect Dictionary instead of object |
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Program.Services.cs
Show resolved
Hide resolved
...ilerplate/src/Server/Boilerplate.Server.Api/Controllers/Identity/RoleManagementController.cs
Show resolved
Hide resolved
...ilerplate/src/Server/Boilerplate.Server.Api/Controllers/Diagnostics/DiagnosticsController.cs
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Services/ServerExceptionHandler.cs (1)
146-157: Consistent concrete type for problem details.Constructing
new AppProblemDetails { ... }is consistent with the type change. Note: the earlierif (instance is null || traceIdentifier is null) problemDetails = null;becomes a no‑op as this assignment overwrites it; if the intent was “no body when missing context,” consider guarding this block.src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Program.Services.cs (1)
231-241: Simplify SignalR JSON resolver wiring; avoid temp options.No functional issue, but you can add resolvers directly without the temporary
jsonOptionsandforeach.Apply this diff:
- }).AddJsonProtocol(options => - { - JsonSerializerOptions jsonOptions = new JsonSerializerOptions(AppJsonContext.Default.Options); - jsonOptions.TypeInfoResolverChain.Add(IdentityJsonContext.Default); - jsonOptions.TypeInfoResolverChain.Add(ServerJsonContext.Default); - - foreach (var chain in jsonOptions.TypeInfoResolverChain) - { - options.PayloadSerializerOptions.TypeInfoResolverChain.Add(chain); - } - }); + }).AddJsonProtocol(options => + { + options.PayloadSerializerOptions.TypeInfoResolverChain.AddRange([ + AppJsonContext.Default, + IdentityJsonContext.Default, + ServerJsonContext.Default + ]); + });Optionally, also align naming policies for SignalR payloads with controllers:
options.PayloadSerializerOptions.PropertyNamingPolicy = JsonNamingPolicy.CamelCase; options.PayloadSerializerOptions.DictionaryKeyPolicy = JsonNamingPolicy.CamelCase;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (6)
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/AppClientCoordinator.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/AttachmentController.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/Diagnostics/DiagnosticsController.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/Identity/RoleManagementController.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Program.Services.cs(2 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Services/ServerExceptionHandler.cs(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build and test
🔇 Additional comments (7)
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/Diagnostics/DiagnosticsController.cs (1)
62-66: SHOW_MESSAGE payload standardized to dictionary — looks good.The move to
Dictionary<string,string?>matches the clientOn<string, Dictionary<string,string?>?, bool>signature and stays friendly to STJ source gen. No issues spotted.src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/AppClientCoordinator.cs (1)
180-211: Client handler generic types aligned with server — nice.
On<string, Dictionary<string,string?>?, bool>matches serverInvokeAsync<bool>and enables typed payloads. The fallback returningfalsewhendata != nullis a sensible signal.src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/Identity/RoleManagementController.cs (2)
229-231: SignalR payload refactor to dictionary — aligned.Using
{ "pageUrl": dto.PageUrl }keeps parity with the client’s typed handler forSHOW_MESSAGE. Looks good.
234-239: Push filter parameters clarified — LGTM.Named args improve readability. The role‑based
customSubscriptionFilteris clear; keep an eye on null‑forgiving operators if model shapes change.Consider adding a null‑safe predicate to avoid relying on
!if feasible in the push layer.src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Program.Services.cs (1)
181-183: CORS exposed headers: formatting only.No behavioral change detected.
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/AttachmentController.cs (1)
106-106: Good switch to DTO for SignalR payload — UserDto is source‑generated; confirm SignalR registrationIdentityJsonContext declares [JsonSerializable(typeof(UserDto))] (src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Dtos/IdentityJsonContext.cs). Ensure that this JsonSerializerContext is configured for SignalR's JSON protocol (i.e., the JsonContext/TypeInfoResolver used by AddSignalR().AddJsonProtocol or equivalent).
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Services/ServerExceptionHandler.cs (1)
173-178: Public API now returns AppProblemDetails — OK.Signature change is coherent. Repo search shows the class-level call sites; only SignalR/AppHub.Chatbot.cs captures the returned value (var problemDetails = serverExceptionHandler.Handle(exp)); other callers invoke via IExceptionHandler or ignore the return. Compile-time will catch any remaining mismatches.
...erplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Services/ServerExceptionHandler.cs
Show resolved
Hide resolved
1 participant
closes #11406
Summary by CodeRabbit
Bug Fixes
Refactor