test(e2e): resiliência + exactly-once ponta a ponta (fase 6) - #8
Merged
Merged
Conversation
Sobe os 3 serviços in-process contra Postgres + RabbitMQ reais e prova as duas garantias centrais do lab — o "killer detail" da spec (§7). Testes (tests/Integration/EndToEndResilienceTests): - F1: com o broker congelado (docker pause), OrderPlaced fica retido no outbox (não publicado); ao descongelar, o dispatcher publica o que segurava — nada se perde - Exactly-once: o checkout completo nos 3 serviços conclui uma única vez (Order Confirmed, cobrado 1x, estoque reservado 1x) Para viabilizar/robustecer: - EfOutboxProcessor: SQL do dispatcher agora é schema-qualified (via metadados do EF), removendo a dependência de search_path — corrige bug de migração e funciona em qualquer schema - RabbitMqEventPublisher/RabbitMqConsumerHost: AutomaticRecoveryEnabled + TopologyRecoveryEnabled explícitos (reconexão após queda do broker) - Composição testável: AddOrders/AddInventory/AddPayments extraídos (usados pelo host e pelo harness de teste); Programs simplificados - Testes de integração rodam sequencialmente (containers não saturam o Docker) Notas de engenharia (descobertas pelo caminho): Testcontainers perde o mapeamento de porta em Stop/Start; docker stop é graceful-close (não dispara auto-recovery). Por isso a indisponibilidade é simulada com pause/unpause (preserva porta, dados e conexões). Build limpo (0/0); unit 4/4; integration 14/14 (2 execuções seguidas estáveis).
📝 WalkthroughWalkthroughThis PR hardens a multi-service distributed system with automatic RabbitMQ recovery, refactors outbox SQL generation to use EF metadata, extracts service composition into reusable DI extensions, and adds comprehensive end-to-end resilience tests that verify exactly-once semantics and outbox recovery after broker downtime. ChangesDistributed Resilience and DI Composition
Sequence Diagram(s)sequenceDiagram
participant Test
participant OrdersHost
participant RabbitMQ
participant DB
Test->>DB: migrate database
Test->>OrdersHost: start host
Test->>RabbitMQ: pause container
Test->>OrdersHost: PlaceOrderHandler (place order)
OrdersHost->>DB: insert outbox row<br/>ProcessedAt = null
DB-->>OrdersHost: row persisted
Test->>DB: assert outbox<br/>ProcessedAt == null
DB-->>Test: row unprocessed
Test->>RabbitMQ: unpause container
Test->>DB: poll until ProcessedAt<br/>is not null
RabbitMqConsumerHost->>DB: read + lock outbox row
RabbitMqConsumerHost->>RabbitMQ: publish event
RabbitMqConsumerHost->>DB: mark ProcessedAt
DB-->>Test: resilience verified
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 (3)
src/Services/Inventory/Program.cs (1)
12-12: ⚡ Quick winPrefer the
IConfigurationoverload for clearer intent.The current method-group syntax (
.Bind) works but is less idiomatic. Use theConfigure<TOptions>(IConfiguration)overload for better clarity.♻️ Suggested refactor
-builder.Services.Configure<RabbitMqOptions>(builder.Configuration.GetSection("RabbitMq").Bind); +builder.Services.Configure<RabbitMqOptions>(builder.Configuration.GetSection("RabbitMq"));🤖 Prompt for 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. In `@src/Services/Inventory/Program.cs` at line 12, Replace the method-group Bind approach with the IConfiguration overload for clarity: update the call that currently reads builder.Services.Configure<RabbitMqOptions>(builder.Configuration.GetSection("RabbitMq").Bind) to use the IConfiguration overload instead, e.g. pass builder.Configuration.GetSection("RabbitMq") directly to builder.Services.Configure<RabbitMqOptions>(...) so the configuration section is used as an IConfiguration source for RabbitMqOptions.src/Services/Orders/Program.cs (1)
13-13: ⚡ Quick winPrefer the
IConfigurationoverload for clearer intent.The current method-group syntax (
.Bind) works but is less idiomatic. Use theConfigure<TOptions>(IConfiguration)overload for better clarity.♻️ Suggested refactor
-builder.Services.Configure<RabbitMqOptions>(builder.Configuration.GetSection("RabbitMq").Bind); +builder.Services.Configure<RabbitMqOptions>(builder.Configuration.GetSection("RabbitMq"));🤖 Prompt for 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. In `@src/Services/Orders/Program.cs` at line 13, The Configure call currently passes a method group binder: builder.Services.Configure<RabbitMqOptions>(builder.Configuration.GetSection("RabbitMq").Bind); — replace this with the IConfiguration overload so intent is clearer: call Configure<RabbitMqOptions>(builder.Configuration.GetSection("RabbitMq")) (i.e. pass the IConfiguration section directly rather than the .Bind method) to register RabbitMqOptions from the "RabbitMq" configuration section.tests/Integration/EndToEndResilienceTests.cs (1)
84-84: ⚡ Quick winConsider using
FirstAsyncor adding an explicit filter.
SingleAsyncwill throw if multiple outbox rows exist. While this should be safe with a fresh database container, usingFirstAsync(o => o.ProcessedAt == null, ct)or adding an explicit filter would make the test more robust and produce clearer error messages if test isolation is ever compromised.🔍 Proposed defensive change
- var row = await db.Outbox.SingleAsync(ct); + var row = await db.Outbox.FirstAsync(o => o.ProcessedAt == null, ct);🤖 Prompt for 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. In `@tests/Integration/EndToEndResilienceTests.cs` at line 84, The test currently uses db.Outbox.SingleAsync(ct) which will throw if multiple outbox rows exist; change it to select a specific unprocessed row instead (e.g., use FirstAsync with a predicate like o => o.ProcessedAt == null or add an explicit filter before awaiting) so that the call targets the intended row and yields clearer failures if test isolation is broken; update the call referencing db.Outbox.SingleAsync to use db.Outbox.FirstAsync(o => o.ProcessedAt == null, ct) or an equivalent filtered query.
🤖 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.
Nitpick comments:
In `@src/Services/Inventory/Program.cs`:
- Line 12: Replace the method-group Bind approach with the IConfiguration
overload for clarity: update the call that currently reads
builder.Services.Configure<RabbitMqOptions>(builder.Configuration.GetSection("RabbitMq").Bind)
to use the IConfiguration overload instead, e.g. pass
builder.Configuration.GetSection("RabbitMq") directly to
builder.Services.Configure<RabbitMqOptions>(...) so the configuration section is
used as an IConfiguration source for RabbitMqOptions.
In `@src/Services/Orders/Program.cs`:
- Line 13: The Configure call currently passes a method group binder:
builder.Services.Configure<RabbitMqOptions>(builder.Configuration.GetSection("RabbitMq").Bind);
— replace this with the IConfiguration overload so intent is clearer: call
Configure<RabbitMqOptions>(builder.Configuration.GetSection("RabbitMq")) (i.e.
pass the IConfiguration section directly rather than the .Bind method) to
register RabbitMqOptions from the "RabbitMq" configuration section.
In `@tests/Integration/EndToEndResilienceTests.cs`:
- Line 84: The test currently uses db.Outbox.SingleAsync(ct) which will throw if
multiple outbox rows exist; change it to select a specific unprocessed row
instead (e.g., use FirstAsync with a predicate like o => o.ProcessedAt == null
or add an explicit filter before awaiting) so that the call targets the intended
row and yields clearer failures if test isolation is broken; update the call
referencing db.Outbox.SingleAsync to use db.Outbox.FirstAsync(o => o.ProcessedAt
== null, ct) or an equivalent filtered query.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7cd05c04-7e89-494a-a717-85de7ba6e802
📒 Files selected for processing (12)
src/BuildingBlocks/Messaging/RabbitMqConsumerHost.cssrc/BuildingBlocks/Messaging/RabbitMqEventPublisher.cssrc/BuildingBlocks/Persistence/EfOutboxProcessor.cssrc/Services/Inventory/InventoryServiceCollectionExtensions.cssrc/Services/Inventory/Program.cssrc/Services/Orders/OrdersServiceCollectionExtensions.cssrc/Services/Orders/Program.cssrc/Services/Payments/PaymentsServiceCollectionExtensions.cssrc/Services/Payments/Program.cstests/Integration/AssemblyInfo.cstests/Integration/EndToEndResilienceTests.cstests/Integration/ExactlyOnceTests.cs
💤 Files with no reviewable changes (1)
- tests/Integration/ExactlyOnceTests.cs
thomasmoreira
added a commit
that referenced
this pull request
Jun 10, 2026
…#8) Sobe os 3 serviços in-process contra Postgres + RabbitMQ reais e prova as duas garantias centrais do lab — o "killer detail" da spec (§7). Testes (tests/Integration/EndToEndResilienceTests): - F1: com o broker congelado (docker pause), OrderPlaced fica retido no outbox (não publicado); ao descongelar, o dispatcher publica o que segurava — nada se perde - Exactly-once: o checkout completo nos 3 serviços conclui uma única vez (Order Confirmed, cobrado 1x, estoque reservado 1x) Para viabilizar/robustecer: - EfOutboxProcessor: SQL do dispatcher agora é schema-qualified (via metadados do EF), removendo a dependência de search_path — corrige bug de migração e funciona em qualquer schema - RabbitMqEventPublisher/RabbitMqConsumerHost: AutomaticRecoveryEnabled + TopologyRecoveryEnabled explícitos (reconexão após queda do broker) - Composição testável: AddOrders/AddInventory/AddPayments extraídos (usados pelo host e pelo harness de teste); Programs simplificados - Testes de integração rodam sequencialmente (containers não saturam o Docker) Notas de engenharia (descobertas pelo caminho): Testcontainers perde o mapeamento de porta em Stop/Start; docker stop é graceful-close (não dispara auto-recovery). Por isso a indisponibilidade é simulada com pause/unpause (preserva porta, dados e conexões). Build limpo (0/0); unit 4/4; integration 14/14 (2 execuções seguidas estáveis).
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.
Summary by CodeRabbit
New Features
Tests
Refactor