V10.1.0/minimal api support#26
Merged
Merged
Conversation
📝 WalkthroughWalkthroughThis PR updates the Codebelt.Extensions.AspNetCore.Newtonsoft.Json package from version 10.0.4 to 10.1.0, introducing a new streamlined extension method Changes
Possibly related PRs
Poem
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/Codebelt.Extensions.AspNetCore.Newtonsoft.Json.Tests/Formatters/ServiceCollectionExtensionsTest.cs (1)
347-356: Strengthen idempotency test with behavioral assertion.You assert single
IConfigureOptionsregistration, but it would be safer to also assert the effective option value after both calls.♻️ Proposed test hardening
[Fact] public void AddNewtonsoftJsonFormatterOptions_ShouldOnlyRegisterOnce_WhenCalledMultipleTimes() { var services = new ServiceCollection(); services.AddNewtonsoftJsonFormatterOptions(o => o.SensitivityDetails = FaultSensitivityDetails.All); services.AddNewtonsoftJsonFormatterOptions(o => o.SensitivityDetails = FaultSensitivityDetails.Evidence); var count = services.Count(s => s.ServiceType == typeof(IConfigureOptions<NewtonsoftJsonFormatterOptions>)); + using var sp = services.BuildServiceProvider(); + var options = sp.GetRequiredService<IOptions<NewtonsoftJsonFormatterOptions>>(); Assert.Equal(1, count); + Assert.Equal(FaultSensitivityDetails.All, options.Value.SensitivityDetails); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/Codebelt.Extensions.AspNetCore.Newtonsoft.Json.Tests/Formatters/ServiceCollectionExtensionsTest.cs` around lines 347 - 356, The test currently only checks registration count; strengthen it by building the ServiceProvider from the ServiceCollection and resolving IOptions<NewtonsoftJsonFormatterOptions> to assert the effective value reflects the last call. Specifically, after the two AddNewtonsoftJsonFormatterOptions calls in AddNewtonsoftJsonFormatterOptions_ShouldOnlyRegisterOnce_WhenCalledMultipleTimes, call services.BuildServiceProvider(), resolve IOptions<NewtonsoftJsonFormatterOptions> (via GetRequiredService or GetService) and Assert that Value.SensitivityDetails equals FaultSensitivityDetails.Evidence to confirm the second registration wins.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@test/Codebelt.Extensions.AspNetCore.Newtonsoft.Json.Tests/Formatters/ServiceCollectionExtensionsTest.cs`:
- Around line 347-356: The test currently only checks registration count;
strengthen it by building the ServiceProvider from the ServiceCollection and
resolving IOptions<NewtonsoftJsonFormatterOptions> to assert the effective value
reflects the last call. Specifically, after the two
AddNewtonsoftJsonFormatterOptions calls in
AddNewtonsoftJsonFormatterOptions_ShouldOnlyRegisterOnce_WhenCalledMultipleTimes,
call services.BuildServiceProvider(), resolve
IOptions<NewtonsoftJsonFormatterOptions> (via GetRequiredService or GetService)
and Assert that Value.SensitivityDetails equals FaultSensitivityDetails.Evidence
to confirm the second registration wins.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
.docfx/Dockerfile.docfx.nuget/Codebelt.Extensions.AspNetCore.Mvc.Formatters.Newtonsoft.Json/PackageReleaseNotes.txt.nuget/Codebelt.Extensions.AspNetCore.Newtonsoft.Json/PackageReleaseNotes.txt.nuget/Codebelt.Extensions.Newtonsoft.Json.App/PackageReleaseNotes.txt.nuget/Codebelt.Extensions.Newtonsoft.Json/PackageReleaseNotes.txtCHANGELOG.mdsrc/Codebelt.Extensions.AspNetCore.Newtonsoft.Json/Formatters/ServiceCollectionExtensions.cssrc/Codebelt.Extensions.AspNetCore.Newtonsoft.Json/ServiceCollectionExtensions.cstest/Codebelt.Extensions.AspNetCore.Newtonsoft.Json.Tests/Formatters/ServiceCollectionExtensionsTest.cstest/Codebelt.Extensions.AspNetCore.Newtonsoft.Json.Tests/ServiceCollectionExtensionsTest.cs
There was a problem hiding this comment.
Pull request overview
This PR bumps the repo to version 10.1.0, adds a minimal-API-friendly DI extension for Newtonsoft.Json formatter integration, and makes formatter option registration idempotent to avoid duplicated IConfigureOptions<T> registrations.
Changes:
- Added
AddMinimalNewtonsoftJsonOptionsto simplify minimal API registration (delegates to the existing exception response formatter registration). - Updated
AddNewtonsoftJsonFormatterOptionsto useTryConfigureto prevent duplicate option registrations. - Updated changelog/release notes and bumped the DocFX build image version.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/Codebelt.Extensions.AspNetCore.Newtonsoft.Json.Tests/ServiceCollectionExtensionsTest.cs | New unit tests for AddMinimalNewtonsoftJsonOptions. |
| test/Codebelt.Extensions.AspNetCore.Newtonsoft.Json.Tests/Formatters/ServiceCollectionExtensionsTest.cs | Added unit tests for AddNewtonsoftJsonFormatterOptions idempotent registration and formatter registration. |
| src/Codebelt.Extensions.AspNetCore.Newtonsoft.Json/ServiceCollectionExtensions.cs | New minimal API-oriented service collection extension method. |
| src/Codebelt.Extensions.AspNetCore.Newtonsoft.Json/Formatters/ServiceCollectionExtensions.cs | Switched options registration to TryConfigure to avoid duplicates. |
| CHANGELOG.md | Added 10.1.0 entry describing the new method and TryConfigure change. |
| .nuget/Codebelt.Extensions.Newtonsoft.Json/PackageReleaseNotes.txt | Bumped release notes version to 10.1.0. |
| .nuget/Codebelt.Extensions.Newtonsoft.Json.App/PackageReleaseNotes.txt | Bumped release notes version to 10.1.0. |
| .nuget/Codebelt.Extensions.AspNetCore.Newtonsoft.Json/PackageReleaseNotes.txt | Bumped release notes version to 10.1.0 and documented new feature/improvement. |
| .nuget/Codebelt.Extensions.AspNetCore.Mvc.Formatters.Newtonsoft.Json/PackageReleaseNotes.txt | Bumped release notes version to 10.1.0. |
| .docfx/Dockerfile.docfx | Updated DocFX image tag from 2.78.4 to 2.78.5. |
Comments suppressed due to low confidence (1)
src/Codebelt.Extensions.AspNetCore.Newtonsoft.Json/Formatters/ServiceCollectionExtensions.cs:48
- Switching to
TryConfiguremakesAddNewtonsoftJsonFormatterOptionseffectively "first call wins"; subsequent calls with a differentsetupdelegate will be ignored. The current XML docs don't mention this behavioral constraint, which can surprise callers that expect later registrations to override earlier ones. Consider documenting this explicitly (e.g., in a<remarks>block) or using an alternative that merges/overrides intentionally (such asPostConfigure) if overriding is desired.
/// <summary>
/// Adds configuration of <see cref="NewtonsoftJsonFormatterOptions"/> for the application.
/// </summary>
/// <param name="services">The <see cref="IServiceCollection"/> to extend.</param>
/// <param name="setup">The <see cref="NewtonsoftJsonFormatterOptions"/> which may be configured.</param>
/// <returns>A reference to <paramref name="services" /> so that additional configuration calls can be chained.</returns>
/// <exception cref="ArgumentNullException">
/// <paramref name="services"/> cannot be null.
/// </exception>
/// <exception cref="ArgumentException">
/// <paramref name="setup"/> failed to configure an instance of <see cref="NewtonsoftJsonFormatterOptions"/> in a valid state.
/// </exception>
public static IServiceCollection AddNewtonsoftJsonFormatterOptions(this IServiceCollection services, Action<NewtonsoftJsonFormatterOptions> setup = null)
{
Validator.ThrowIfNull(services);
Validator.ThrowIfInvalidConfigurator(setup, out var options);
services.TryConfigure(setup ?? (o =>
{
o.Settings = options.Settings;
o.SensitivityDetails = options.SensitivityDetails;
o.SupportedMediaTypes = options.SupportedMediaTypes;
o.SynchronizeWithJsonConvert = options.SynchronizeWithJsonConvert;
}));
return services;
Comment on lines
+14
to
+27
| /// <summary> | ||
| /// Adds a <see cref="NewtonsoftJsonFormatterOptions"/> service to the specified <see cref="IServiceCollection"/>. | ||
| /// </summary> | ||
| /// <param name="services">The <see cref="IServiceCollection"/> to add services to.</param> | ||
| /// <param name="setup">The <see cref="NewtonsoftJsonFormatterOptions"/> which may be configured.</param> | ||
| /// <returns>An <see cref="IServiceCollection"/> that can be used to further configure other services.</returns> | ||
| /// <exception cref="ArgumentNullException"> | ||
| /// <paramref name="services"/> cannot be null. | ||
| /// </exception> | ||
| public static IServiceCollection AddMinimalNewtonsoftJsonOptions(this IServiceCollection services, Action<NewtonsoftJsonFormatterOptions> setup = null) | ||
| { | ||
| Validator.ThrowIfNull(services); | ||
| return services.AddNewtonsoftJsonExceptionResponseFormatter(setup); | ||
| } |
There was a problem hiding this comment.
The XML docs for AddMinimalNewtonsoftJsonOptions say it "Adds a NewtonsoftJsonFormatterOptions service", but the implementation delegates to AddNewtonsoftJsonExceptionResponseFormatter, which also registers an HttpExceptionDescriptorResponseFormatter<NewtonsoftJsonFormatterOptions>. Please update the summary/remarks (or rename the method) so the documentation matches the actual behavior (options + exception response formatter registration).
Member
Author
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
|
|
||
| # New Features | ||
| - ADDED ServiceCollectionExtensions class in the Codebelt.Extensions.AspNetCore.Newtonsoft.Json namespace with AddMinimalNewtonsoftJsonOptions method | ||
|
|
There was a problem hiding this comment.
There is a non-breaking space / stray whitespace-only line between the new "# New Features" entry and the "# Improvements" section (line 9). This can render oddly in release tooling and makes diffs noisier; replace it with a normal blank line (standard space/newline) for consistent formatting.
Suggested change
|
|
@gimlichael I've opened a new pull request, #27, to work on those changes. Once the pull request is ready, I'll request review from you. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces a minor release (10.1.0) focused on improving minimal API formatter integration and tightening option registration for ASP.NET Core formatter setup. It adds a new extension method for minimal configuration, ensures option registrations are not duplicated, and updates dependencies. The changes are documented and thoroughly tested.
New Features and Improvements
ServiceCollectionExtensionsclass in theCodebelt.Extensions.AspNetCore.Newtonsoft.Jsonnamespace, providing theAddMinimalNewtonsoftJsonOptionsmethod to simplify minimal API formatter setup. [1] [2] [3]AddNewtonsoftJsonFormatterOptionsmethod inServiceCollectionExtensions(Formatters) to useTryConfigure, preventing duplicate option registrations and improving robustness when called multiple times. [1] [2]Dependency and Version Updates
.docfx/Dockerfile.docfx.PackageReleaseNotes.txtfiles, documenting new features and improvements. [1] [2] [3] [4]Testing Enhancements
AddMinimalNewtonsoftJsonOptionsmethod and the improved formatter options registration, ensuring correct behavior, error handling, and prevention of duplicate registrations. [1] [2]Documentation
CHANGELOG.mdto reflect new features, changes, and fixes in version 10.1.0.Summary by CodeRabbit
New Features
AddMinimalNewtonsoftJsonOptionsextension method for streamlined Newtonsoft JSON formatter configuration in ASP.NET Core.Bug Fixes
IConfigureOptionsregistrations when formatter option methods are invoked repeatedly.Chores