-
Notifications
You must be signed in to change notification settings - Fork 4
Expose CQRS JsonSerializerOptions configuration #816
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Expose CQRS JsonSerializerOptions configuration #816
Conversation
Summary
Skipped
🎉 No failed tests in this run. Github Test Reporter by CTRF 💚 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR refactors the CQRS JSON serialization configuration by extracting duplicated configuration logic into a reusable extension method ConfigureForCQRS(), making it available for use in other libraries like LeanPipe.
Key Changes:
- Introduced a new
CQRSJsonSerializerOptionsExtensionsclass with aConfigureForCQRS()extension method that centralizes the JSON serializer configuration - Refactored
ServiceCollectionCQRSExtensionsto use the new extension method instead of inline configuration - Updated
LeanCodeTestFactoryto use the new extension method, replacing the previous inline configuration
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/Infrastructure/LeanCode.Serialization/CQRSJsonSerializerOptionsExtensions.cs |
New extension method that configures JsonSerializerOptions with the three Lax converters and sets PropertyNamingPolicy to null |
src/CQRS/LeanCode.CQRS.AspNetCore/ServiceCollectionCQRSExtensions.cs |
Refactored to use the new ConfigureForCQRS extension method instead of inline converter configuration |
src/Testing/LeanCode.IntegrationTestHelpers/LeanCodeTestFactory.cs |
Updated to use the new ConfigureForCQRS extension method, simplifying the constructor initialization |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| public static void ConfigureForCQRS(this JsonSerializerOptions options) | ||
| { | ||
| options.Converters.Add(new JsonLaxDateOnlyConverter()); | ||
| options.Converters.Add(new JsonLaxTimeOnlyConverter()); | ||
| options.Converters.Add(new JsonLaxDateTimeOffsetConverter()); | ||
| options.PropertyNamingPolicy = null; | ||
| } |
Copilot
AI
Jan 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new ConfigureForCQRS extension method lacks test coverage. Consider adding a test class to verify that:
- All three converters (JsonLaxDateOnlyConverter, JsonLaxTimeOnlyConverter, JsonLaxDateTimeOffsetConverter) are correctly added to the options
- PropertyNamingPolicy is set to null as expected
This would follow the existing testing pattern seen in other extension method tests in the codebase (e.g., OutputCachingServiceCollectionExtensionsTests, ServiceCollectionExtensionsTests).
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## v10.0-preview #816 +/- ##
=================================================
- Coverage 84.60% 84.57% -0.03%
=================================================
Files 236 237 +1
Lines 4819 4810 -9
Branches 341 347 +6
=================================================
- Hits 4077 4068 -9
Misses 659 659
Partials 83 83 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
LeanPipe will benefit from this, and some other future libraries could as well.