refactor: Use OutboxEventStore for IEventOutbox DI#369
Merged
Conversation
Remove direct registration and usage of provider-specific IEventOutbox implementations (EntityFrameworkOutbox, SqlServerEventOutbox, PostgreSqlEventOutbox, SQLiteEventOutbox) from the public API and DI setup. Instead, always register OutboxEventStore as the IEventOutbox implementation. Update extension method docs and tests to reflect this change. Delete provider-specific IEventOutbox classes and their unit tests. This enforces OutboxEventStore as the single entry point for event outbox operations, simplifying the API and reducing coupling to provider-specific types.
samtrion
added
state:ready for merge
Contributor
WalkthroughThe pull request removes database-specific Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 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. 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}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/NetEvolve.Pulse.Tests.Unit/SqlServer/SqlServerExtensionsTests.cs (1)
72-81:⚠️ Potential issue | 🟡 MinorThese assertions don't actually prove
AddSqlServerOutbox()wiresIEventOutbox.Both tests pre-register
IEventOutboxviaAddOutbox(), so they still pass even ifAddSqlServerOutbox(...)forgets to add the shared event store. To cover this PR's behavior, initialize with onlyAddSqlServerOutbox(...)in these two tests.Suggested test adjustment
- _ = services.AddPulse(config => config.AddOutbox().AddSqlServerOutbox("Server=.;Encrypt=true;")); + _ = services.AddPulse(config => config.AddSqlServerOutbox("Server=.;Encrypt=true;")); … - _ = services.AddPulse(config => config.AddOutbox().AddSqlServerOutbox(_ => "Server=.;Encrypt=true;")); + _ = services.AddPulse(config => config.AddSqlServerOutbox(_ => "Server=.;Encrypt=true;"));Also applies to: 153-164
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/NetEvolve.Pulse.Tests.Unit/SqlServer/SqlServerExtensionsTests.cs` around lines 72 - 81, The tests currently call services.AddPulse(config => config.AddOutbox().AddSqlServerOutbox(...)) which pre-registers IEventOutbox and masks whether AddSqlServerOutbox actually wires the shared event store; change both affected tests to initialize with only AddSqlServerOutbox(...) (remove AddOutbox()) so the assertion on the ServiceDescriptor for IEventOutbox truly verifies AddSqlServerOutbox's behavior (update both occurrences around the earlier block and the second block at lines ~153-164).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@tests/NetEvolve.Pulse.Tests.Unit/SqlServer/SqlServerExtensionsTests.cs`:
- Around line 72-81: The tests currently call services.AddPulse(config =>
config.AddOutbox().AddSqlServerOutbox(...)) which pre-registers IEventOutbox and
masks whether AddSqlServerOutbox actually wires the shared event store; change
both affected tests to initialize with only AddSqlServerOutbox(...) (remove
AddOutbox()) so the assertion on the ServiceDescriptor for IEventOutbox truly
verifies AddSqlServerOutbox's behavior (update both occurrences around the
earlier block and the second block at lines ~153-164).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c80c4bee-f3eb-487e-9300-59416655248b
📒 Files selected for processing (11)
src/NetEvolve.Pulse.EntityFramework/EntityFrameworkExtensions.cssrc/NetEvolve.Pulse.EntityFramework/Outbox/EntityFrameworkOutbox.cssrc/NetEvolve.Pulse.PostgreSql/Outbox/PostgreSqlEventOutbox.cssrc/NetEvolve.Pulse.SQLite/Outbox/SQLiteEventOutbox.cssrc/NetEvolve.Pulse.SqlServer/Outbox/SqlServerEventOutbox.cssrc/NetEvolve.Pulse.SqlServer/SqlServerExtensions.cstests/NetEvolve.Pulse.Tests.Unit/EntityFramework/EntityFrameworkEventOutboxTests.cstests/NetEvolve.Pulse.Tests.Unit/PostgreSql/PostgreSqlEventOutboxTests.cstests/NetEvolve.Pulse.Tests.Unit/SQLite/SQLiteEventOutboxTests.cstests/NetEvolve.Pulse.Tests.Unit/SqlServer/SqlServerEventOutboxTests.cstests/NetEvolve.Pulse.Tests.Unit/SqlServer/SqlServerExtensionsTests.cs
💤 Files with no reviewable changes (8)
- tests/NetEvolve.Pulse.Tests.Unit/EntityFramework/EntityFrameworkEventOutboxTests.cs
- src/NetEvolve.Pulse.PostgreSql/Outbox/PostgreSqlEventOutbox.cs
- tests/NetEvolve.Pulse.Tests.Unit/SQLite/SQLiteEventOutboxTests.cs
- src/NetEvolve.Pulse.SQLite/Outbox/SQLiteEventOutbox.cs
- src/NetEvolve.Pulse.EntityFramework/Outbox/EntityFrameworkOutbox.cs
- tests/NetEvolve.Pulse.Tests.Unit/PostgreSql/PostgreSqlEventOutboxTests.cs
- tests/NetEvolve.Pulse.Tests.Unit/SqlServer/SqlServerEventOutboxTests.cs
- src/NetEvolve.Pulse.SqlServer/Outbox/SqlServerEventOutbox.cs
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #369 +/- ##
==========================================
+ Coverage 83.61% 85.06% +1.45%
==========================================
Files 110 106 -4
Lines 4076 3837 -239
Branches 369 351 -18
==========================================
- Hits 3408 3264 -144
+ Misses 574 486 -88
+ Partials 94 87 -7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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
1 participant
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.
Remove direct registration and usage of provider-specific IEventOutbox implementations (EntityFrameworkOutbox, SqlServerEventOutbox, PostgreSqlEventOutbox, SQLiteEventOutbox) from the public API and DI setup. Instead, always register OutboxEventStore as the IEventOutbox implementation. Update extension method docs and tests to reflect this change. Delete provider-specific IEventOutbox classes and their unit tests. This enforces OutboxEventStore as the single entry point for event outbox operations, simplifying the API and reducing coupling to provider-specific types.
Summary by CodeRabbit
Refactor
Tests