Finalizing task implementation and documentation - #831
Conversation
Co-authored-by: AndreaCuneo <5227688+AndreaCuneo@users.noreply.github.com>
Co-authored-by: AndreaCuneo <5227688+AndreaCuneo@users.noreply.github.com>
Co-authored-by: AndreaCuneo <5227688+AndreaCuneo@users.noreply.github.com>
Co-authored-by: AndreaCuneo <5227688+AndreaCuneo@users.noreply.github.com>
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
There was a problem hiding this comment.
🟡 Not ready to approve
The updated mapper introduces a compile break (missing UnconditionalSuppressMessage qualification/import) and uses Expression.Compile() without handling dynamic-code-disabled/AOT environments.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
Improve BusinessRuleViolationException → RFC7807 mapping performance by caching per-violation-type property accessors, and add tests/benchmarks/docs to validate compatibility and measure gains.
Changes:
- Add a
ConcurrentDictionary<Type, Accessor[]>cache and compiled getters for business-rule violation extension payload mapping. - Add compatibility + concurrency tests for business-rule violation payload shaping.
- Add BenchmarkDotNet benchmarks and update benchmark runner/docs to capture baseline vs cached performance.
File summaries
| File | Description |
|---|---|
| tests/Ark.Tools.AspNetCore.ProblemDetails.Tests/ProblemDetailsShapeTests.cs | Adds payload-compatibility and concurrency tests for business rule violation mapping. |
| src/aspnetcore/Ark.Tools.AspNetCore.ProblemDetails/ExceptionProblemDetailsMapper.cs | Introduces cached accessors and expression-compiled getters for business-rule violation payloads. |
| docs/performance/completed/003-business-rule-problem-details.md | Documents decisions and measured benchmark results for the accessor cache approach. |
| benchmarks/Ark.Tools.Benchmarks/Program.cs | Switches benchmark entrypoint to BenchmarkSwitcher for assembly-wide execution. |
| benchmarks/Ark.Tools.Benchmarks/packages.lock.json | Updates lockfile to include the new project reference closure. |
| benchmarks/Ark.Tools.Benchmarks/ExceptionProblemDetailsBenchmarks.cs | Adds reflection vs cached mapping benchmarks for representative violation shapes. |
| benchmarks/Ark.Tools.Benchmarks/Ark.Tools.Benchmarks.csproj | Adds project reference to Ark.Tools.AspNetCore.ProblemDetails to benchmark the mapper. |
Review details
- Files reviewed: 7/7 changed files
- Comments generated: 2
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
…callyAccessedMembers on violationType parameter Co-authored-by: AndreaCuneo <5227688+AndreaCuneo@users.noreply.github.com>
|
@copilot sync with master and validate build and tests runs |
…o-completed Co-authored-by: AndreaCuneo <5227688+AndreaCuneo@users.noreply.github.com>
Synced with master (merged |
Pull request created by AI Agent