Fix PublishingRelay misrouting default-tenant events under conjoined tenancy#2676
Closed
svenclaesson wants to merge 1 commit intoJasperFx:mainfrom
Closed
Conversation
…tenancy Under conjoined tenancy, PublishingRelay's default-tenant fallthrough let WolverineSubscriptionRunner's bus.TenantId (= operations.Database.Identifier) leak onto the outbound envelope. The handler chain then ran under the wrong tenant scope (or as a silent no-op) while Marten still advanced the subscription seq because publishing succeeded. Gate the override on Events.TenancyStyle == Conjoined so only the broken case is touched. Non-conjoined stores keep the existing fallthrough exactly, preserving setups that legitimately use Database.Identifier as the message tenant (e.g. per-tenant ancillary stores). Adds two regression tests in MartenSubscriptionTests: - carry_default_tenant_id_through_under_conjoined_tenancy reproduces the bug on main and passes after the fix. - non_conjoined_store_preserves_legacy_default_tenant_fallthrough guards the per-tenant-ancillary pattern against accidental over-application. Closes JasperFx#2675
Author
|
Broader MartenTests rerun completed: 434/439 pass on net9.0 with this fix applied. The 5 failures are pre-existing infrastructure flakes unrelated to
Verified by inspection that none reference |
Member
|
@svenclaesson I'm going to change this one just a wee bit to try to remove the runtime logic and the downcasting tomorrow, but the test will go in as is. |
2 tasks
jeremydmiller
added a commit
that referenced
this pull request
May 7, 2026
#2693 supersedes #2676. The original tests + bug analysis are @svenclaesson's work; this trailer attaches the attribution that the amended trailer on the prior commit couldn't carry because force-push is disabled by the branch protection rule. Co-Authored-By: Sven Claesson <sven.claesson@devshift.se>
jeremydmiller
added a commit
that referenced
this pull request
May 7, 2026
Under conjoined tenancy `PublishingRelay`'s default-tenant fallthrough let `WolverineSubscriptionRunner`'s `bus.TenantId` (= `operations.Database.Identifier`) leak onto the outbound envelope. Default-tenant events were dispatched under the database identifier (e.g. `"Main"`) instead of `*DEFAULT*`, the handler chain ran under the wrong tenant scope (or silently no-op'd), but Marten still advanced the subscription sequence because publishing succeeded. Closes GH-2675. Supersedes #2676 with a simpler implementation: bind the per-event publish path once at construction time so the hot loop in `ProcessEventsAsync` needs neither a `TenancyStyle` comparison nor a downcast through `IDocumentSession.DocumentStore`. The constructor now takes the `TenancyStyle` from the surrounding `ConfigureMarten` lambda and selects between two static method-group delegates: - `RelayWithEventTenant` (conjoined): always propagates `e.TenantId` verbatim — including `StorageConstants.DefaultTenantId` for default-tenant events. - `RelayWithLegacyFallthrough` (everything else): preserves the existing fallthrough so setups that legitimately use the database identifier as the message tenant (e.g. per-tenant ancillary stores with `TenancyStyle.Single`) keep working. Compatibility table — only the conjoined-default-tenant cell changes: | Setup | Pre-fix `Envelope.TenantId` | Post-fix `Envelope.TenantId` | |--------------------------------------------------|------------------------------|--------------------------------| | No tenancy (default Marten store) | `Database.Identifier` | `Database.Identifier` (unchanged) | | Conjoined, default-tenant event | `Database.Identifier` (bug) | `*DEFAULT*` (correct) | | Conjoined, named-tenant event | `"one"` | `"one"` (unchanged) | | `MultiTenantedDatabases(...)` per-tenant DB | `"tenant1"` | `"tenant1"` (unchanged) | | Per-tenant ancillary store, `TenancyStyle.Single`| custom identifier | custom identifier (unchanged) | Tests in `MartenSubscriptionTests` (carried over from #2676): 1. `carry_default_tenant_id_through_under_conjoined_tenancy` — publishes a default-tenant event through a conjoined store; asserts `Envelope.TenantId == StorageConstants.DefaultTenantId`. Reproduces the bug on main. 2. `non_conjoined_store_preserves_legacy_default_tenant_fallthrough` — companion test on a non-conjoined store; asserts the relay still falls through to the bus context's tenant id, guarding the per-tenant-ancillary-store pattern against accidental over-application. Both pass; full `MartenSubscriptionTests` suite is 12/12 green on net9.0. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Member
|
@svenclaesson I did this just a little bit differently, but you're listed as the co-author of the new PR |
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.
Closes #2675
Summary
Under conjoined tenancy,
PublishingRelay's default-tenant fallthrough letWolverineSubscriptionRunner'sbus.TenantId(set tooperations.Database.Identifier) leak onto the outbound envelope. Default-tenant events were dispatched under the database identifier (e.g."Main") instead of*DEFAULT*, the handler chain ran under the wrong tenant scope (or silently no-op'd), but Marten still advanced the subscription sequence because publishing succeeded.This change gates the override on
Events.TenancyStyle == Conjoined, so only the broken case is touched. Non-conjoined stores keep the existing fallthrough exactly, preserving setups that legitimately useDatabase.Identifieras the message tenant (e.g. per-tenant ancillary stores withTenancyStyle.Single).Compatibility
Envelope.TenantIdEnvelope.TenantIdDatabase.IdentifierDatabase.Identifier(unchanged)Database.Identifier(the bug)*DEFAULT*(correct)"one""one"(unchanged)MultiTenantedDatabases(...AddSingleTenantDatabase("tenant1"...))"tenant1""tenant1"(unchanged)Database.Identifier(TenancyStyle.Single)The only behavioural delta is the targeted bug case. Every other row keeps its pre-fix behaviour because the override only activates in conjoined mode.
Tests
Two regression tests added in
MartenSubscriptionTests:carry_default_tenant_id_through_under_conjoined_tenancy— sets up a conjoined store, publishes a default-tenant event, assertsEnvelope.TenantId == StorageConstants.DefaultTenantId. Reproduces the bug onmain(asserts"*DEFAULT*", gets"Main"); passes after the fix.non_conjoined_store_preserves_legacy_default_tenant_fallthrough— companion test using a plain non-conjoined Marten store; asserts the relay still falls through to the bus context's tenant id, guarding the per-tenant-ancillary-store pattern against accidental over-application. Passes both before and after.MartenSubscriptionTestsruns green on net8/net9/net10 (12/12).Test plan
mainMartenSubscriptionTestssuite green on net8.0, net9.0, net10.0MartenTestssuite (rerun in progress; previously: 433/439 with 6 unrelated infrastructure flakes — postgres connection pool exhaustion + agent-distribution timeouts; none touchPublishingRelay)See #2675 for the full root-cause trace through
WolverineSubscriptionRunner→PublishingRelay→MessageBus, and a side-by-side with the Polecat sibling that already handles tenancy correctly.