Feat/cqrs optimization#342
Conversation
- 新增 NotificationBenchmarks 的 Mediator concrete runtime 对照与对应通知合同实现 - 更新 benchmark README，明确 notification publish 已扩成三方对照 - 更新 cqrs-rewrite active tracking 与 trace，记录 RP-111 的基线、验证结果与下一恢复建议
- 新增 NotificationFanOutBenchmarks，量化固定四处理器 notification publish 对照 - 更新 benchmark README，补充 notification fan-out 场景说明 - 更新 cqrs-rewrite active tracking 与 trace，记录 RP-112 的基线、验证结果与下一恢复建议
- 新增 TaskWhenAllNotificationPublisher 内置并行通知发布器并保留默认顺序语义 - 补充通知发布策略回归测试与采用边界文档 - 更新 cqrs-rewrite 跟踪与执行追踪恢复点
- 新增默认顺序发布器与 TaskWhenAllNotificationPublisher 的 fixed 4 handler fan-out benchmark 对照 - 更新 benchmark README 与 cqrs-rewrite 恢复文档，记录 RP-114 的性能结论与下一步
- 新增 notification publisher 组合根注册扩展，提供 TaskWhenAll 与自定义策略入口 - 补充通知发布策略配置回归测试，并更新 CQRS 文档与恢复点记录
- 新增公开 SequentialNotificationPublisher，并让默认 runtime 回退复用该策略 - 增加顺序 notification publisher 组合根注册入口，并更新测试文档与恢复点
- 更新 notification publisher 的策略选择矩阵，明确顺序、并行与自定义 publisher 的适用边界 - 补充 CQRS 重写 tracking 与 trace，记录已撤回的无收益 request 热路径实验和当前恢复点
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 Walkthrough演练此 PR 为 CQRS 通知发布框架引入了并行策略选项、公开的注册 API 和全面的性能基准。 变更通知发布者策略框架
预估代码审查工作量🎯 3 (中等) | ⏱️ ~20 分钟 可能关联的 PR
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 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 |
Summary
Test Results
Details
Insights
Fail Rate
build-and-test: Run #1094
🎉 All tests passed!Slowest Tests
± Comparison with run #1093 at 005cdbc | 🍂 No flaky tests detected across all runs. | ⏱️ Measured over 41 runs. Github Test Reporter by CTRF 💚 |
✅
|
| Descriptor | Linter | Files | Fixed | Errors | Warnings | Elapsed time |
|---|---|---|---|---|---|---|
| dotnet-format | yes | 1 | no | 5.16s | ||
| ✅ REPOSITORY | gitleaks | yes | no | no | 8.0s | |
| ✅ REPOSITORY | trufflehog | yes | no | no | 6.3s |
Detailed Issues
⚠️ CSHARP / dotnet-format - 1 error
Welcome to .NET 9.0!
---------------------
SDK Version: 9.0.114
----------------
Installed an ASP.NET Core HTTPS development certificate.
To trust the certificate, run 'dotnet dev-certs https --trust'
Learn about HTTPS: https://aka.ms/dotnet-https
----------------
Write your first app: https://aka.ms/dotnet-hello-world
Find out what's new: https://aka.ms/dotnet-whats-new
Explore documentation: https://aka.ms/dotnet-docs
Report issues and find source on GitHub: https://github.com/dotnet/core
Use 'dotnet --help' to see available commands or visit: https://aka.ms/dotnet-cli
--------------------------------------------------------------------------------------
Unhandled exception: System.Exception: Restore operation failed.
at Microsoft.CodeAnalysis.Tools.CodeFormatter.OpenMSBuildWorkspaceAsync(String solutionOrProjectPath, WorkspaceType workspaceType, Boolean noRestore, Boolean requiresSemantics, String binaryLogPath, Boolean logWorkspaceWarnings, ILogger logger, CancellationToken cancellationToken)
at Microsoft.CodeAnalysis.Tools.CodeFormatter.FormatWorkspaceAsync(FormatOptions formatOptions, ILogger logger, CancellationToken cancellationToken, String binaryLogPath)
at Microsoft.CodeAnalysis.Tools.FormatCommandCommon.FormatAsync(FormatOptions formatOptions, ILogger`1 logger, CancellationToken cancellationToken)
at Microsoft.CodeAnalysis.Tools.Commands.RootFormatCommand.FormatCommandDefaultHandler.InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken)
at System.CommandLine.Invocation.InvocationPipeline.InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken)
See detailed reports in MegaLinter artifacts
Set VALIDATE_ALL_CODEBASE: true in mega-linter.yml to validate all sources, not only the diff
Show us your support by starring ⭐ the repository
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ai-plan/public/cqrs-rewrite/todos/cqrs-rewrite-migration-tracking.md`:
- Around line 156-159: The markdown currently shows an older fan-out measurement
in the line containing "固定 `4 handler` notification fan-out 对照当前约为 baseline
`8.302 ns / 0 B`、`Mediator` `4.314 ns / 0 B`、`MediatR` `230.304 ns / 1256
B`、`GFramework.Cqrs` `434.413 ns / 408 B` (referenced next to the `dotnet run
... --filter "*NotificationFanOutBenchmarks*"` command); update these numbers to
the latest fan-out benchmark results or, if you intend to keep the historical
values, append an explicit note like “历史基线（RP-112）” next to that inline code
snippet so readers aren’t confused with RP-117.
- Around line 10-13: 在文档中的“当前 PR 锚点：`PR `#341``”条目已过期，请将其更新为最新的 PR 编号（`PR
`#342``），确保文档中所有出现 `PR `#341`` 的位置（例如条目“当前 PR 锚点：`PR `#341``”）都替换为 `PR `#342`` 以同步恢复点
CQRS-REWRITE-RP-117 的入口指向最新评审上下文。
In `@GFramework.Cqrs.Benchmarks/Messaging/NotificationFanOutBenchmarks.cs`:
- Around line 229-234: MediatR explicit
INotificationHandler<BenchmarkNotification>.Handle implementations (found in
BenchmarkNotificationHandler1, BenchmarkNotificationHandler2,
BenchmarkNotificationHandler3, BenchmarkNotificationHandler4) currently return
Task.CompletedTask and bypass shared checks; change each explicit Handle to
invoke the existing HandleCore(notification, cancellationToken) (or await/return
its Task) so the MediatR branch performs the same null/cancellation logic as the
baseline/GFramework/Mediator path and maintains consistent fan-out benchmarking.
In `@GFramework.Cqrs/README.md`:
- Around line 131-136: The markdown table containing
SequentialNotificationPublisher, TaskWhenAllNotificationPublisher and
UseNotificationPublisher needs a blank line before and after it to satisfy
MD058; edit the README.md around the table and insert one empty line immediately
above the pipe-row starting with "| 策略" and one empty line immediately below the
final paragraph row so the table is separated from surrounding text.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9bb2e5c4-b6b5-47dd-9de0-9cc298e5e66f
📒 Files selected for processing (12)
GFramework.Cqrs.Benchmarks/Messaging/NotificationBenchmarks.csGFramework.Cqrs.Benchmarks/Messaging/NotificationFanOutBenchmarks.csGFramework.Cqrs.Benchmarks/README.mdGFramework.Cqrs.Tests/Cqrs/CqrsNotificationPublisherTests.csGFramework.Cqrs.Tests/Cqrs/NotificationPublisherRegistrationExtensionsTests.csGFramework.Cqrs/Extensions/NotificationPublisherRegistrationExtensions.csGFramework.Cqrs/Notification/SequentialNotificationPublisher.csGFramework.Cqrs/Notification/TaskWhenAllNotificationPublisher.csGFramework.Cqrs/README.mdai-plan/public/cqrs-rewrite/todos/cqrs-rewrite-migration-tracking.mdai-plan/public/cqrs-rewrite/traces/cqrs-rewrite-migration-trace.mddocs/zh-CN/core/cqrs.md
📜 Review details
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Analyze (C#)
- GitHub Check: Code Quality & Security
- GitHub Check: Build and Test
🧰 Additional context used
📓 Path-based instructions (10)
**/*.cs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.cs: Apply [Log] attribute for automatic logging field and logging helper method generation
Apply [Priority] attribute for automatic priority comparison implementation generation
Apply [GenerateEnumExtensions] attribute to generate enumeration extension capabilities
Apply [ContextAware] attribute to automatically implement IContextAware boilerplate logic
**/*.cs: All public, protected, and internal types and members MUST include XML documentation comments (///) in C#
XML documentation MUST use<summary>,<param>,<returns>,<exception>, and<remarks>where applicable, and explain intent, contract, and usage constraints instead of restating syntax
If a C# member participates in lifecycle, threading, registration, or disposal behavior, document that behavior explicitly
Core framework components (Architecture, Module, System, Context, Registry, Service Module, Lifecycle types) MUST include high-level explanations of responsibilities, lifecycle, interaction with other components, why abstraction exists, and when to use instead of alternatives
Generated logic and source generator pipelines MUST explain what is generated, why it is generated, semantic assumptions the generator relies on, and any diagnostics or fallback behavior
Do not rely on implicit imports. Declare every requiredusingexplicitly in C#
Write null-safe code that respects nullable annotations instead of suppressing warnings by default in C#
Use namespace patternGFramework.{Module}.{Feature}with PascalCase segments in C#
Follow standard C# naming: Types/methods/properties/events/constants use PascalCase, Interfaces useIprefix, Parameters and locals use camelCase, Private fields use_camelCase
Use Allman braces style for C#
Keepusingdirectives at the top of the file and sort them consistently in C#
Prefer one primary type per file unless surrounding project already uses different local pattern
Prefer explicit, readable code over clever shorthand in framework internals
M...
Files:
GFramework.Cqrs/Notification/TaskWhenAllNotificationPublisher.csGFramework.Cqrs/Extensions/NotificationPublisherRegistrationExtensions.csGFramework.Cqrs/Notification/SequentialNotificationPublisher.csGFramework.Cqrs.Benchmarks/Messaging/NotificationBenchmarks.csGFramework.Cqrs.Tests/Cqrs/NotificationPublisherRegistrationExtensionsTests.csGFramework.Cqrs.Tests/Cqrs/CqrsNotificationPublisherTests.csGFramework.Cqrs.Benchmarks/Messaging/NotificationFanOutBenchmarks.cs
**/*[!.]*
📄 CodeRabbit inference engine (AGENTS.md)
For files with shebang lines, keep shebang as first line and place license header immediately after it
Files:
GFramework.Cqrs/Notification/TaskWhenAllNotificationPublisher.csGFramework.Cqrs.Benchmarks/README.mdGFramework.Cqrs/Extensions/NotificationPublisherRegistrationExtensions.csGFramework.Cqrs/README.mdGFramework.Cqrs/Notification/SequentialNotificationPublisher.csai-plan/public/cqrs-rewrite/todos/cqrs-rewrite-migration-tracking.mdGFramework.Cqrs.Benchmarks/Messaging/NotificationBenchmarks.csdocs/zh-CN/core/cqrs.mdGFramework.Cqrs.Tests/Cqrs/NotificationPublisherRegistrationExtensionsTests.csGFramework.Cqrs.Tests/Cqrs/CqrsNotificationPublisherTests.csGFramework.Cqrs.Benchmarks/Messaging/NotificationFanOutBenchmarks.csai-plan/public/cqrs-rewrite/traces/cqrs-rewrite-migration-trace.md
**/*.{cs,ts,tsx,js,jsx,py,sh}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{cs,ts,tsx,js,jsx,py,sh}: All generated or modified code MUST include clear and meaningful comments where required by documentation rules
Comments MUST NOT be trivial, redundant, or misleading. Prefer explainingwhyandwhen, not justwhat. Code should remain understandable without requiring external context
Avoid obvious comments such as// increment i
Files:
GFramework.Cqrs/Notification/TaskWhenAllNotificationPublisher.csGFramework.Cqrs/Extensions/NotificationPublisherRegistrationExtensions.csGFramework.Cqrs/Notification/SequentialNotificationPublisher.csGFramework.Cqrs.Benchmarks/Messaging/NotificationBenchmarks.csGFramework.Cqrs.Tests/Cqrs/NotificationPublisherRegistrationExtensionsTests.csGFramework.Cqrs.Tests/Cqrs/CqrsNotificationPublisherTests.csGFramework.Cqrs.Benchmarks/Messaging/NotificationFanOutBenchmarks.cs
**/*.{cs,ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{cs,ts,tsx,js,jsx,py}: Add inline comments for non-trivial logic, concurrency/threading behavior, performance-sensitive paths, workarounds/compatibility constraints/edge cases, and registration order/lifecycle sequencing/generated code assumptions
Methods with non-trivial logic MUST document core idea, key decisions, and edge case handling
Separate logical blocks with blank lines when it improves readability
Unless there is clear and documented reason to keep file large, keep single source file under roughly 800-1000 lines
Validate external or user-controlled input before it reaches file system, serialization, reflection, code generation, or process boundaries
Do not build command strings, file paths, type names, or generated code from untrusted input without strict validation or allow-listing
Avoid logging secrets, tokens, credentials, or machine-specific sensitive data
Prefer least-privilege behavior for file, process, and environment access
Files:
GFramework.Cqrs/Notification/TaskWhenAllNotificationPublisher.csGFramework.Cqrs/Extensions/NotificationPublisherRegistrationExtensions.csGFramework.Cqrs/Notification/SequentialNotificationPublisher.csGFramework.Cqrs.Benchmarks/Messaging/NotificationBenchmarks.csGFramework.Cqrs.Tests/Cqrs/NotificationPublisherRegistrationExtensionsTests.csGFramework.Cqrs.Tests/Cqrs/CqrsNotificationPublisherTests.csGFramework.Cqrs.Benchmarks/Messaging/NotificationFanOutBenchmarks.cs
**/*.{csproj,cs}
📄 CodeRabbit inference engine (AGENTS.md)
Framework runtime, abstractions, and meta-package projects MUST NOT reference
*.SourceGenerators*projects or packages, and MUST NOT use source-generator attributes
Files:
GFramework.Cqrs/Notification/TaskWhenAllNotificationPublisher.csGFramework.Cqrs/Extensions/NotificationPublisherRegistrationExtensions.csGFramework.Cqrs/Notification/SequentialNotificationPublisher.csGFramework.Cqrs.Benchmarks/Messaging/NotificationBenchmarks.csGFramework.Cqrs.Tests/Cqrs/NotificationPublisherRegistrationExtensionsTests.csGFramework.Cqrs.Tests/Cqrs/CqrsNotificationPublisherTests.csGFramework.Cqrs.Benchmarks/Messaging/NotificationFanOutBenchmarks.cs
**/*.{cs,ts,tsx,js,jsx,py,sh,xml,csproj,props,targets}
📄 CodeRabbit inference engine (AGENTS.md)
Use 4 spaces for indentation. Do not use tabs
Files:
GFramework.Cqrs/Notification/TaskWhenAllNotificationPublisher.csGFramework.Cqrs/Extensions/NotificationPublisherRegistrationExtensions.csGFramework.Cqrs/Notification/SequentialNotificationPublisher.csGFramework.Cqrs.Benchmarks/Messaging/NotificationBenchmarks.csGFramework.Cqrs.Tests/Cqrs/NotificationPublisherRegistrationExtensionsTests.csGFramework.Cqrs.Tests/Cqrs/CqrsNotificationPublisherTests.csGFramework.Cqrs.Benchmarks/Messaging/NotificationFanOutBenchmarks.cs
**/*.{cs,ts,tsx,js,jsx,py,sh,xml}
📄 CodeRabbit inference engine (AGENTS.md)
Keep line length readable. Around 120 characters is preferred upper bound
Files:
GFramework.Cqrs/Notification/TaskWhenAllNotificationPublisher.csGFramework.Cqrs/Extensions/NotificationPublisherRegistrationExtensions.csGFramework.Cqrs/Notification/SequentialNotificationPublisher.csGFramework.Cqrs.Benchmarks/Messaging/NotificationBenchmarks.csGFramework.Cqrs.Tests/Cqrs/NotificationPublisherRegistrationExtensionsTests.csGFramework.Cqrs.Tests/Cqrs/CqrsNotificationPublisherTests.csGFramework.Cqrs.Benchmarks/Messaging/NotificationFanOutBenchmarks.cs
**/*.{md,mdx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{md,mdx}: Keep code samples, package names, and command examples aligned with current repository state in documentation
When public page references XML docs or API coverage, convert evidence into reader-facing guidance: explain which types/namespaces/entry points readers should inspect and why
For integration-oriented features such as AI-First config system, documentation MUST cover: project directory layout/file conventions, required project/package wiring, minimal working example, migration/compatibility notes
When examples are rewritten, preserve only parts that remain true. Delete or replace speculative examples instead of lightly editing into another inaccurate form
Files:
GFramework.Cqrs.Benchmarks/README.mdGFramework.Cqrs/README.mdai-plan/public/cqrs-rewrite/todos/cqrs-rewrite-migration-tracking.mddocs/zh-CN/core/cqrs.mdai-plan/public/cqrs-rewrite/traces/cqrs-rewrite-migration-trace.md
docs/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
Documentation should be organized with Chinese content in docs/zh-CN/ and structured to include getting started, module-specific capabilities (Core, Game, Godot, ECS), source generator usage, tutorials, best practices, and troubleshooting
Files:
docs/zh-CN/core/cqrs.md
**/Cqrs/**/*.cs
📄 CodeRabbit inference engine (CLAUDE.md)
Use CQRS (Command Query Responsibility Segregation) pattern with the Cqrs naming entry point instead of the historical Mediator alias
Files:
GFramework.Cqrs.Tests/Cqrs/NotificationPublisherRegistrationExtensionsTests.csGFramework.Cqrs.Tests/Cqrs/CqrsNotificationPublisherTests.cs
🧠 Learnings (1)
📚 Learning: 2026-04-06T12:45:43.921Z
Learnt from: GeWuYou
Repo: GeWuYou/GFramework PR: 190
File: GFramework.Game/Config/GameConfigBootstrap.cs:1-3
Timestamp: 2026-04-06T12:45:43.921Z
Learning: In the GeWuYou/GFramework repository, C# files may omit explicit `using System*` imports because the project-wide `GlobalUsings.cs` (referenced via manual global `using` directives) supplies common namespaces (e.g., `System`, `System.Threading`, `System.Threading.Tasks`). During code review, do not flag missing `using System...` directives in `.cs` files as long as `GlobalUsings.cs` is present/used to provide those namespaces.
Applied to files:
GFramework.Cqrs/Notification/TaskWhenAllNotificationPublisher.csGFramework.Cqrs/Extensions/NotificationPublisherRegistrationExtensions.csGFramework.Cqrs/Notification/SequentialNotificationPublisher.csGFramework.Cqrs.Benchmarks/Messaging/NotificationBenchmarks.csGFramework.Cqrs.Tests/Cqrs/NotificationPublisherRegistrationExtensionsTests.csGFramework.Cqrs.Tests/Cqrs/CqrsNotificationPublisherTests.csGFramework.Cqrs.Benchmarks/Messaging/NotificationFanOutBenchmarks.cs
🪛 LanguageTool
GFramework.Cqrs/README.md
[uncategorized] ~134-~134: 您的意思是“"不"齐”？
Context: ...序 | 不会在首个失败时停止其余处理器；会聚合最终异常或取消结果 | 更适合语义补齐，不是性能开关 | | `UseNotificationPublish...
(BU)
🪛 markdownlint-cli2 (0.22.1)
GFramework.Cqrs/README.md
[warning] 135-135: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
🔇 Additional comments (9)
GFramework.Cqrs/Notification/SequentialNotificationPublisher.cs (1)
8-35: 实现与文档语义一致，改动可接受。公开化后的类型与“顺序分发、失败即停”行为保持一致，注释也覆盖了关键契约。
GFramework.Cqrs/Notification/TaskWhenAllNotificationPublisher.cs (1)
27-58: 并行发布路径设计清晰，边界处理到位。0/1/多处理器分支与
Task.WhenAll聚合路径实现一致，可读性和可维护性都不错。GFramework.Cqrs.Benchmarks/Messaging/NotificationBenchmarks.cs (1)
73-93: 基准扩展与清理路径同步，改动稳健。新增 Mediator 对照路径后，初始化/释放和处理器契约都配套更新了，整体实现一致性很好。
Also applies to: 113-155
GFramework.Cqrs/Extensions/NotificationPublisherRegistrationExtensions.cs (1)
30-111: 注册入口分层明确，防重复注册逻辑合理。API 形态（实例/类型/内置快捷）和冲突保护都设计得很清楚。
docs/zh-CN/core/cqrs.md (1)
120-162: 文档与实现对齐良好，策略选择说明清晰。新增矩阵与示例能直接指导组合根配置，实用性很高。
GFramework.Cqrs.Tests/Cqrs/CqrsNotificationPublisherTests.cs (1)
94-127: 新增用例有效覆盖了并行发布器的核心语义差异。该测试能稳定验证“异常后仍触发后续处理器”的行为，价值很高。
GFramework.Cqrs.Tests/Cqrs/NotificationPublisherRegistrationExtensionsTests.cs (1)
25-106: 注册扩展的关键行为覆盖完整。语义测试（并行/顺序/实例复用/重复注册）都覆盖到了，回归保护充分。
GFramework.Cqrs.Benchmarks/README.md (1)
32-34: 这段文档补充清晰且对齐当前 benchmark 场景
NotificationFanOutBenchmarks的入口与对比维度描述完整，便于读者快速定位新增基准。ai-plan/public/cqrs-rewrite/traces/cqrs-rewrite-migration-trace.md (1)
5-180: 迁移追踪记录完整，恢复信息可执行性强阶段决策、验证命令和结论都落到了同一段里，作为恢复入口可读性很好。
|
| Filename | Overview |
|---|---|
| GFramework.Cqrs/Extensions/NotificationPublisherRegistrationExtensions.cs | New composition-root extension for registering notification publisher strategies; has a gap where registration is silently ignored when the runtime is created via the two-argument CqrsRuntimeFactory overload rather than through the module path. |
| GFramework.Cqrs/Notification/TaskWhenAllNotificationPublisher.cs | New parallel notification publisher using Task.WhenAll; correctly wraps synchronous throws via InvokeHandlerSafelyAsync for the N>1 path, but the single-handler fast path inconsistently allows synchronous exceptions to escape. |
| GFramework.Cqrs/Notification/SequentialNotificationPublisher.cs | New sequential publisher that extracts the pre-existing dispatcher behavior into an explicit, composable type; implementation is straightforward and well-documented. |
| GFramework.Cqrs.Tests/Cqrs/NotificationPublisherRegistrationExtensionsTests.cs | New test file covering instance-overload registration, duplicate-registration guard, and sequential/parallel semantics; generic type-registration overload UseNotificationPublisher lacks coverage. |
| GFramework.Cqrs.Tests/Cqrs/CqrsNotificationPublisherTests.cs | Adds a test verifying TaskWhenAllNotificationPublisher continues to invoke handlers after the first one throws; implementation is correct. |
| GFramework.Cqrs.Benchmarks/Messaging/NotificationFanOutBenchmarks.cs | New fan-out benchmark comparing GFramework sequential and TaskWhenAll publishers against MediatR and the Mediator source-generated library; handler parity with the baseline is maintained correctly. |
| GFramework.Cqrs.Benchmarks/Messaging/NotificationBenchmarks.cs | Adds the Mediator source-generated library as an additional benchmark comparison; renames existing fields for clarity; straightforward extension. |
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
GFramework.Cqrs/Extensions/NotificationPublisherRegistrationExtensions.cs:17-22
**Container registration silently ignored by `CqrsRuntimeFactory.CreateRuntime(container, logger)`**
`UseNotificationPublisher` / `UseTaskWhenAllNotificationPublisher` / `UseSequentialNotificationPublisher` register `INotificationPublisher` in the IoC container, but `CqrsRuntimeFactory.CreateRuntime(IIocContainer, ILogger)` — the two-argument overload that is the most common direct-use entry point — always creates a fresh `new SequentialNotificationPublisher()` without ever querying the container for a registered publisher. A developer who follows the natural pattern of "register in container → create runtime from the same container" will silently receive sequential publishing regardless of the strategy they registered, with no error or warning.
The XML `<remarks>` mentions that the module path will honor the registration, but it does not explicitly warn that `CqrsRuntimeFactory.CreateRuntime(container, logger)` ignores it. Either the factory's two-argument overload should be updated to check `container.HasRegistration(typeof(INotificationPublisher))` and use the resolved publisher, or the `<remarks>` should add a clear note that direct factory use must pass the publisher explicitly via the three-argument overload.
### Issue 2 of 2
GFramework.Cqrs/Notification/TaskWhenAllNotificationPublisher.cs:34-39
The fast path for a single handler calls `context.InvokeHandlerAsync` directly from a non-`async` method. If the underlying handler throws synchronously (before its first `await`), the exception escapes `PublishAsync` synchronously rather than being captured in a faulted `ValueTask`. For the N>1 case, `InvokeHandlerSafelyAsync`'s `async` state machine captures those same synchronous throws. Routing the single-handler case through `InvokeHandlerSafelyAsync` keeps exception-delivery semantics consistent for all callers.
```suggestion
return context.Handlers.Count switch
{
0 => ValueTask.CompletedTask,
1 => InvokeHandlerSafelyAsync(context, context.Handlers[0], cancellationToken),
_ => PublishCoreAsync(context, cancellationToken)
};
```
Reviews (2): Last reviewed commit: "fix(cqrs): 收口 PR342 审查遗留问题" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
- 修复 NotificationFanOutBenchmarks 中 MediatR handler 绕过 HandleCore 的对照偏差 - 更新 README 与中文文档中的 notification publisher 示例和表格格式 - 同步 cqrs-rewrite tracking 与 trace 到 PR #342 审查恢复点和最新验证结果
Summary
Test Results
Details
Insights
Fail Rate
build-and-test: Run #1095
🎉 All tests passed!Slowest Tests
± Comparison with run #1094 at 341205d | 🍂 No flaky tests detected across all runs. | ⏱️ Measured over 42 runs. Github Test Reporter by CTRF 💚 |
| { | ||
| /// <summary> | ||
| /// 将指定的 notification publisher 实例注册为当前容器唯一的发布策略。 | ||
| /// </summary> | ||
| /// <param name="container">目标依赖注入容器。</param> | ||
| /// <param name="notificationPublisher">要复用的 notification publisher 实例。</param> |
There was a problem hiding this comment.
CqrsRuntimeFactory.CreateRuntime(container, logger)
UseNotificationPublisher / UseTaskWhenAllNotificationPublisher / UseSequentialNotificationPublisher register INotificationPublisher in the IoC container, but CqrsRuntimeFactory.CreateRuntime(IIocContainer, ILogger) — the two-argument overload that is the most common direct-use entry point — always creates a fresh new SequentialNotificationPublisher() without ever querying the container for a registered publisher. A developer who follows the natural pattern of "register in container → create runtime from the same container" will silently receive sequential publishing regardless of the strategy they registered, with no error or warning.
The XML <remarks> mentions that the module path will honor the registration, but it does not explicitly warn that CqrsRuntimeFactory.CreateRuntime(container, logger) ignores it. Either the factory's two-argument overload should be updated to check container.HasRegistration(typeof(INotificationPublisher)) and use the resolved publisher, or the <remarks> should add a clear note that direct factory use must pass the publisher explicitly via the three-argument overload.
Prompt To Fix With AI
This is a comment left during a code review.
Path: GFramework.Cqrs/Extensions/NotificationPublisherRegistrationExtensions.cs
Line: 17-22
Comment:
**Container registration silently ignored by `CqrsRuntimeFactory.CreateRuntime(container, logger)`**
`UseNotificationPublisher` / `UseTaskWhenAllNotificationPublisher` / `UseSequentialNotificationPublisher` register `INotificationPublisher` in the IoC container, but `CqrsRuntimeFactory.CreateRuntime(IIocContainer, ILogger)` — the two-argument overload that is the most common direct-use entry point — always creates a fresh `new SequentialNotificationPublisher()` without ever querying the container for a registered publisher. A developer who follows the natural pattern of "register in container → create runtime from the same container" will silently receive sequential publishing regardless of the strategy they registered, with no error or warning.
The XML `<remarks>` mentions that the module path will honor the registration, but it does not explicitly warn that `CqrsRuntimeFactory.CreateRuntime(container, logger)` ignores it. Either the factory's two-argument overload should be updated to check `container.HasRegistration(typeof(INotificationPublisher))` and use the resolved publisher, or the `<remarks>` should add a clear note that direct factory use must pass the publisher explicitly via the three-argument overload.
How can I resolve this? If you propose a fix, please make it concise.
1 participant
Summary by CodeRabbit
发布说明
新功能
文档
测试