Cover the provider outbox paths with integration tests and fix the store robustness gaps they exposed#323
Merged
Conversation
…preserve custom outbox stores; cover the provider outbox paths with integration tests
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.
Summary
Tests-first: this PR first adds provider-level integration coverage for every outbox path that had none (only Npgsql was exercised end-to-end), then fixes what the new tests proved broken.
New coverage (24 tests in
Vulthil.IntegrationTests, running against real containers on the shared assembly host):mysql:8.4: domain-event capture onSaveChangesAsync→FOR UPDATE SKIP LOCKEDrelay fetch → set-based marks → dead-letter atMaxRetries→ retention sweep, plus dispatch order and the 256-charTypecap (rejects, never truncates). First real-MySQL execution of the dialect SQL and the net10ServerVersion.AutoDetectpath.EnsureCreatedwith default indexing, dead-letter through the base update path, retention. This settles a suspected defect: the inherited two-propertyORDER BY (OccurredOnUtc, Id)was believed to fail every poll with 400 on default indexing — it does not; the query runs fine on the emulator. No Cosmos code change; the tests pin the behavior, and the package docs now note the composite index to add if the Azure service (documented as requiring composite indexes for multi-property ORDER BY) ever rejects the relay query.Fixes (each observed failing before, passing after):
FROM "OutboxMessages"and default column names; a model with a naming convention or custom mapping broke the relay at runtime (42P01/ table-not-found reproduced). The fetch SQL is now composed from the EF model's mapped identifiers viaISqlGenerationHelper, and the composed query carries an explicit outerORDER BY (OccurredOnUtc, Id), making dispatch order guaranteed rather than incidental.RelationalOutboxStore.DeleteProcessedAsyncignoredbatchSize(one unboundedExecuteDelete; a first sweep over a big backlog was one giant locking delete). It now deletes the oldest ≤batchSizerows by key per call, matching theIOutboxRetentionStorecontract and the relational inbox store's existing pattern (which already complied).UpdateMessagesAsyncissued a query+save per failure plus a dead-letter re-query pass; it now fetches all affected rows once and saves once (only non-relational stores hit this path — covered by the Cosmos tests).Use*extensions eagerly overwrote a store the user selected withUseOutboxStorewhen called after it. Store selection now defers toOnConfiguredand applies only as a default, so chaining order is genuinely irrelevant — as the docs already promised. Adds one public member:DatabaseInfrastructureConfigurator<TDbContext>.OutboxStoreCustomized(Unshipped).CosmosOutboxStorereturned an internalNoOpDbTransaction; it now returnsnull, which the base batch unit already handles. The class is deleted.Docs: provider articles updated (identifier support, ordering guarantee, store-selection precedence), the Cosmos article's incorrect "transactional batches" description replaced with the actual relay behavior plus the indexing note, and the retention article now states the per-batch delete bound.
Verification
Vulthil.IntegrationTestsrun (33 tests: 24 new + 9 pre-existing): all pass in ≈15.5 s wall on net10 with warm images — the three containers (PostgreSQL, MySQL, Cosmos emulator) start lazily and are shared across classes. Registration tests also verified on net9.Vulthil.SharedKernel.Outbox.Tests,Vulthil.SharedKernel.Outbox.EntityFrameworkCore.Tests,Vulthil.Messaging.Outbox.Tests,Vulthil.SharedKernel.Infrastructure.Relational.Tests.Backport to v1.0: no — fix-forward to 1.1.0; no 1.0.x consumers use the affected providers, and the label can be applied retroactively if one appears.