Share tenancy-neutral identity-map state across ForTenant sessions (#4947)#4948
Merged
Merged
Conversation
…4947) A ForTenant() view of an identity/dirty-tracked session stopped seeing tenancy-neutral (global) documents that the parent session tracks. A global document has exactly one row per id for the whole database, so LoadAsync through the ForTenant view missed the identity map, queried the database, found nothing (the doc was Store()d but not yet committed) and returned null. Silent wrong answer, not an error. Introduced by 74b11a4 (#4807, the fix for #4801), which tenant-scoped the identity map and version tracker for ForTenant sessions. That was right for CONJOINED documents -- where the same id means a different document per tenant -- but it was applied per SESSION rather than per DOCUMENT TYPE, so it also isolated document types that are tenancy-neutral and must be shared. Broken since 9.13.0. The sharing is now decided per document type. A nested ForTenant session aliases the parent's identity-map entry (and version-tracker entry) for a type only when all of the following hold: - the storage is identity-mapped (so it tracks documents at all), - the type is NOT Conjoined -- #4801's isolation is preserved exactly, and - the nested session's Database is the same instance as the parent's, because under database-per-tenant the same id in another tenant's database is a different document even for a tenancy-neutral type. Aliasing happens in selectStorage, which runs on every StorageFor<T>() -- i.e. before any load or store can create a competing map entry -- so it is deterministic rather than order-dependent. Tests: Bug_4947_for_tenant_tenantless_documents -- the reporter's repro, LoadManyAsync, committed-document instance identity, storing through ForTenant, plus two guard rails asserting conjoined documents stay isolated. 5 of the 6 fail on unmodified master. The Bug_4801 suite still passes, and full DocumentDbTests is 1047/0. Fixes #4947. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
Fixes #4947. Correctness regression against the shipped 9.13/9.14/9.15 line — targeting a 9.15.1 patch.
The bug
A
ForTenant()view of an identity- or dirty-tracked session stopped seeing tenancy-neutral (global) documents the parent session tracks. A global document has exactly one row per id for the whole database, soLoadAsyncthrough theForTenantview missed the identity map, went to the database, found nothing (in @dervagabund's repro the document isStore()d but not yet committed) and returnednull.Silent wrong answer, not an error. Reported working on 9.12.0, broken on 9.15.0.
Root cause
Commit
74b11a461(PR #4807, the fix for #4801) tenant-scoped the identity map and version tracker forForTenantsessions:src/Marten/Internal/Sessions/NestedTenantSession.cs:29-33—ItemMap/Versionsare shared from the parent only whentenant.TenantId == parent.TenantId; any other tenant gets a fresh, emptyItemMap. Same inNestedTenantQuerySession.cs:20-24.That was right for conjoined documents — where the same id means a different document per tenant — but it was applied per session rather than per document type, so it also isolated document types that are tenancy-neutral and must be shared. Broken since 9.13.0.
The fix
Sharing is now decided per document type. A nested
ForTenantsession aliases the parent's identity-map entry (and version-tracker entry) for a type only when all of these hold:DirtyCheckedDocumentStoragevia the same seam),Conjoined— Identity-map cache returns wrong tenant's document when session reused across tenants via ForTenant #4801's isolation is preserved exactly, andDatabaseis the same instance as the parent's, because under database-per-tenant the same id in another tenant's database is a different document even for a tenancy-neutral type.Aliasing happens in
selectStorage, which runs on everyStorageFor<T>()— i.e. before any load or store can create a competing map entry — so it is deterministic rather than order-dependent.Tests
Bug_4947_for_tenant_tenantless_documents— the reporter's repro,LoadManyAsync, committed-document instance identity, storing throughForTenant, plus two guard rails asserting conjoined documents stay isolated.Bug_4801suite still passes — the isolation that fix introduced is intact.DocumentDbTests: 1047 passed, 0 failed.Honest caveats
SubClassDocumentStoragedoes not implement the new seam, so a subclassed global document resolved via the subclass type won't have the parent's map aliased at that moment (it delegates to the root storage, which aliases when accessed through the root type). Judged out of scope for a minimal patch; untested edge.EjectAllOfTypeon a nested session removes the entry only from that session'sItemMap; for a now-shared global type the underlying dictionary is the parent's, so the parent keeps seeing it. Pre-existing asymmetry in the eject paths, unchanged in behavior here.net9.0was exercised locally.🤖 Generated with Claude Code