From c06f30f875dfbb563d764596449d7b1d5d66e387 Mon Sep 17 00:00:00 2001 From: "Jeremy D. Miller" Date: Wed, 15 Jul 2026 07:27:46 -0500 Subject: [PATCH] =?UTF-8?q?#4956=20=E2=80=94=20lock=20down=20ForTenant()?= =?UTF-8?q?=20conjoined-document=20tenant=20isolation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #4956 reports a "regression": a ForTenant() view of an identity session that Store()s a conjoined (MultiTenanted) document under one tenant, then reads it back through the BASE session (a different tenant), no longer sees it. This is the intended behavior of #4801/#4947 — a conjoined id is a different document per tenant, so a session bound to tenant X must not resolve tenant Y's document. Pre-9.13 the shared tenant-blind identity map leaked one tenant's uncommitted write into another's view; #4801 tenant-scoped the map and #4947 restored sharing only for tenancy-neutral documents. No code change — this adds a regression suite that pins the ForTenant() matrix so the isolation and legitimate sharing cases can't silently drift again. Covers: base-session cross-tenant read returns null (pending + committed/DB parity), same-tenant view sees its own pending write, multiple ForTenant stores all commit through one base SaveChanges with no cross-tenant bleed, and same-tenant ForTenant sharing in both directions. Co-Authored-By: Claude Opus 4.8 (1M context) --- ...Bug_4956_for_tenant_conjoined_isolation.cs | 136 ++++++++++++++++++ 1 file changed, 136 insertions(+) create mode 100644 src/DocumentDbTests/Bugs/Bug_4956_for_tenant_conjoined_isolation.cs diff --git a/src/DocumentDbTests/Bugs/Bug_4956_for_tenant_conjoined_isolation.cs b/src/DocumentDbTests/Bugs/Bug_4956_for_tenant_conjoined_isolation.cs new file mode 100644 index 0000000000..d6bbc6c1b1 --- /dev/null +++ b/src/DocumentDbTests/Bugs/Bug_4956_for_tenant_conjoined_isolation.cs @@ -0,0 +1,136 @@ +using System; +using System.Threading.Tasks; +using Marten; +using Marten.Testing.Harness; +using Shouldly; +using Xunit; + +namespace DocumentDbTests.Bugs; + +// #4956 — a ForTenant() view of an identity session that Store()s a CONJOINED (MultiTenanted) +// document under one tenant, then reads it back through the BASE session (a *different* +// tenant). Reported as a regression from 9.12.0: the base session no longer "sees" the doc. +// +// This is the correct behavior for conjoined tenancy and the intended effect of #4801/#4947: +// a conjoined id means a *different* document per tenant, so a session bound to tenant X must +// not resolve a document stored for tenant Y. The pre-9.13 behavior leaked one tenant's +// uncommitted write into another tenant's view through a tenant-blind identity map -- an +// in-memory answer a real database query would never give. These tests lock the intended +// ForTenant() matrix so the isolation and the legitimate sharing cases can't silently drift. +public class Bug_4956_for_tenant_conjoined_isolation: BugIntegrationContext +{ + public class TestEntity + { + public Guid Id { get; set; } + public string Label { get; set; } = null!; + public string TenantId { get; set; } = "none"; + } + + private void ConjoinedStore() => StoreOptions(opts => opts.Schema.For().MultiTenanted()); + + // The reporter's exact scenario. The base session is the default tenant; the write went to + // tenant "t1". A cross-tenant read of a conjoined document must NOT resolve it. + [Fact] + public async Task base_session_cannot_see_other_tenants_pending_conjoined_write() + { + ConjoinedStore(); + + await using var session = theStore.IdentitySession(); + + var item1 = new TestEntity { Id = Guid.NewGuid(), Label = "foo1", TenantId = "t1" }; + session.ForTenant("t1").Store(item1); + + (await session.LoadAsync(item1.Id)).ShouldBeNull(); + } + + // The correct pattern: read a conjoined document back through the SAME tenant view it was + // stored under. ForTenant() caches one nested session per tenant, so the pending write is + // in that view's own identity map. + [Fact] + public async Task same_tenant_view_sees_its_own_pending_conjoined_write() + { + ConjoinedStore(); + + await using var session = theStore.IdentitySession(); + + var item1 = new TestEntity { Id = Guid.NewGuid(), Label = "foo1", TenantId = "t1" }; + session.ForTenant("t1").Store(item1); + + var res = await session.ForTenant("t1").LoadAsync(item1.Id); + + res.ShouldNotBeNull(); + res.ShouldBeSameAs(item1); + } + + // Prove the in-memory isolation matches what the database actually holds: after committing + // through the base session, a FRESH default-tenant session still cannot load the t1 doc, + // while a fresh ForTenant("t1") view can. i.e. #4956 is not an identity-map artifact. + [Fact] + public async Task committed_conjoined_write_is_only_visible_to_its_own_tenant() + { + ConjoinedStore(); + + var id = Guid.NewGuid(); + + await using (var session = theStore.IdentitySession()) + { + session.ForTenant("t1").Store(new TestEntity { Id = id, Label = "foo1", TenantId = "t1" }); + await session.SaveChangesAsync(); + } + + await using var fresh = theStore.IdentitySession(); + + (await fresh.LoadAsync(id)).ShouldBeNull(); + + var viaTenant = await fresh.ForTenant("t1").LoadAsync(id); + viaTenant.ShouldNotBeNull(); + viaTenant.Label.ShouldBe("foo1"); + } + + // Storing several tenants' conjoined docs through ForTenant() on one session is a single + // unit of work (the nested sessions share the parent work tracker): base SaveChanges() + // commits them all, each to its own tenant. + [Fact] + public async Task multiple_for_tenant_stores_all_commit_through_base_save_changes() + { + ConjoinedStore(); + + var id1 = Guid.NewGuid(); + var id2 = Guid.NewGuid(); + + await using (var session = theStore.IdentitySession()) + { + session.ForTenant("t1").Store(new TestEntity { Id = id1, Label = "foo1", TenantId = "t1" }); + session.ForTenant("t2").Store(new TestEntity { Id = id2, Label = "foo2", TenantId = "t2" }); + await session.SaveChangesAsync(); + } + + await using var fresh = theStore.IdentitySession(); + + (await fresh.ForTenant("t1").LoadAsync(id1)).Label.ShouldBe("foo1"); + (await fresh.ForTenant("t2").LoadAsync(id2)).Label.ShouldBe("foo2"); + + // and no cross-tenant bleed + (await fresh.ForTenant("t2").LoadAsync(id1)).ShouldBeNull(); + (await fresh.ForTenant("t1").LoadAsync(id2)).ShouldBeNull(); + } + + // The legitimate sharing case: when the ForTenant tenant MATCHES the base session's own + // tenant, the identity map is shared, so a pending write through either path is visible to + // the other. This is the scenario the reporter almost certainly wanted. + [Fact] + public async Task for_tenant_matching_base_tenant_shares_pending_writes_both_directions() + { + ConjoinedStore(); + + await using var session = theStore.IdentitySession("t1"); + + var viaTenant = new TestEntity { Id = Guid.NewGuid(), Label = "via-tenant", TenantId = "t1" }; + session.ForTenant("t1").Store(viaTenant); + (await session.LoadAsync(viaTenant.Id)).ShouldBeSameAs(viaTenant); + + var viaBase = new TestEntity { Id = Guid.NewGuid(), Label = "via-base", TenantId = "t1" }; + session.Store(viaBase); + (await session.ForTenant("t1").LoadAsync(viaBase.Id)).ShouldBeSameAs(viaBase); + } +}