fix(db): scope secondary-table loaders, aliased selectables and attached objects; subqueryload works on scoped models

CHANGELOG.md

@@ -67,6 +67,33 @@ All notable changes to this project are documented in this file. The format is b
 - ORM restores now emit `restored` audit entries with translated action labels
   and ORM primary-key updates include the old and new key in their diffs
   (behaviour change: both used to leave no entry) (#811).
+- Tenant and soft-delete scoping in `simple_module_db` now also applies to:
+  relationships whose `secondary` is the table of a mapped `MultiTenantMixin` /
+  `SoftDeleteMixin` model (a bare association `Table` is not filtered), in
+  `joinedload()`, `selectinload()`, `subqueryload()` and joins through the
+  relationship (lazy loads already were; writes through it skip the
+  soft-delete condition, like every write); Core statements on
+  `Model.__table__` run before any ORM use has configured the mappers; and
+  sessions on a derived engine (`engine.execution_options(...)`), bound per
+  model (`binds={...}`) or routing through an overridden `get_bind()`, which
+  now keep the engine's strict mode, fallback tenant and audit sink.
+  `simple_module_db` now requires SQLAlchemy 2.0.2 or later.
+- **Behaviour change:** these shapes now raise `TenantIsolationError`:
+  - flushing an UPDATE or DELETE of an object the session did not load or
+    insert (`make_transient_to_detached()`, `merge(load=False)`, a detached
+    object passed to `session.add()`) whose row belongs to another tenant —
+    its `tenant_id` is read from the table first (only for such objects; a
+    found owner is read once, a missing row is read again on the next flush);
+  - `aliased(Model, <subquery or CTE>)` whose selectable reads a tenant-scoped
+    or soft-deletable table through Core, other than the entity's own table
+    (or the entity's own table with `GROUP BY`, `HAVING`, a window function
+    or any function other than a row-wise one such as `lower`), unless inside `all_tenants()` (with
+    `include_deleted=True`); build the selectable from ORM entities instead.
+    The same holds for `aliased(Model, <join>)` whose join holds a scoped
+    table other than the entity's own, and for `aliased(Model, text(…).columns(…))`
+    (bare, subquery or CTE) on a tenant-scoped or soft-deletable `Model`.
+- `subqueryload()` on a relationship between tenant-scoped or soft-deletable
+  models no longer fails with a duplicate `anon_1` alias (#816).
 - Tenant scoping in `simple_module_db` now applies to more statement shapes:
   ORM bulk UPDATE by primary key (`session.execute(update(Model), rows)`),
   `update().ordered_values()` (assigning `tenant_id` raises like `.values()`),

docs/database/mixins.md

@@ -89,6 +89,18 @@ Adds:
 
 When `SM_MULTI_TENANT=true` and a request has an active tenant, the `do_orm_execute` listener attaches `with_loader_criteria(cls, cls.tenant_id == tenant_id)` to every SELECT touching a `MultiTenantMixin` table.
 
+A tenant-scoped (or soft-deletable) model used as a relationship's link table,
+`relationship(..., secondary=Link.__table__)`, is filtered in every loader —
+lazy, `joinedload`, `selectinload`, `subqueryload` — and in joins through the
+relationship (writes through it skip the soft-delete condition). Only a link
+table mapped by a mixin class is filtered: a bare association `Table` with a
+`tenant_id` column is not, like every unmapped table.
+`aliased(Model, <subquery>)` over a selectable that reads a scoped table through Core is
+refused unless that table is the entity's own (and the selectable does not
+group, aggregate or use a window function); build such selectables from ORM
+entities. Details in
+[multi-tenancy](../framework/multi-tenancy.md#what-the-filter-can-see).
+
 ### Automatic stamping
 
 SQLite and Postgres `INSERT ... ON CONFLICT DO UPDATE` arms obey the same
@@ -98,7 +110,7 @@ predicate). Conflicts with another tenant's row leave it unchanged. An explicit
 `all_tenants()` / `execution_options(all_tenants=True)` bypass permits deliberate
 cross-tenant upserts (#749).
 
-INSERTs populate `tenant_id` from the `current_tenant_id` contextvar. Creating a row for a different tenant than the active one — or changing `tenant_id` on an existing row — raises `TenantIsolationError`.
+INSERTs populate `tenant_id` from the `current_tenant_id` contextvar. Creating a row for a different tenant than the active one — or changing `tenant_id` on an existing row — raises `TenantIsolationError`. Flushing an UPDATE or DELETE of an object the session did not load or insert (`make_transient_to_detached()`, `merge(load=False)`, a detached object passed to `session.add()`) first reads its row's `tenant_id`, once per identity; a row of another tenant raises `TenantIsolationError`.
 
 `insert(Model).from_select(...)` and Core table targets follow the same rules:
 when the target column list omits `tenant_id`, the guard adds it and a constant

docs/framework/multi-tenancy.md

@@ -19,7 +19,7 @@ tenant a request acts for.
 | ORM `update()` / `delete()` (also bulk UPDATE by primary key) | filtered to the tenant; assigning `tenant_id` (`.values()` or `.ordered_values()`) raises | `MissingTenantError` | unfiltered |
 | `session.add` + flush | `tenant_id` filled in; a different explicit value raises | `MissingTenantError`, even with an explicit `tenant_id` | `tenant_id` filled in with the install's fallback tenant unless set |
 | ORM `insert(Model)` (bulk / `.values()`) | `tenant_id` filled in; a different explicit value raises | `MissingTenantError`, even when every row sets `tenant_id` | `tenant_id` filled in with the install's fallback tenant unless set |
-| Flushing a change to, or a delete of, a loaded object | only if it belongs to the bound tenant | `MissingTenantError` | allowed |
+| Flushing a change to, or a delete of, a persistent object | only if its row belongs to the bound tenant (an object made persistent in memory has its owner read from the table first) | `MissingTenantError` | allowed |
 | Changing `tenant_id` | raises | raises | raises (only an `all_tenants()` block may move a row) |
 | Legacy `Session.bulk_save_objects` / `bulk_insert_mappings` / `bulk_update_mappings` | `TenantIsolationError` | `TenantIsolationError` | allowed, unscoped |
 
@@ -33,7 +33,20 @@ Tenant criteria — and soft-delete criteria, on reads — are attached for
 **every** tenant-scoped / soft-deletable model, not only the entities a
 statement names, so all of these are scoped (#332):
 
-- `select(Model)`, relationship loads, ORM `update()`/`delete()`/`insert()`;
+- `select(Model)`, ORM `update()`/`delete()`/`insert()`;
+- relationship loads — lazy, `joinedload`, `selectinload`, `subqueryload` —
+  and joins or `any()` through a relationship. A relationship whose
+  `secondary` is the table of a `MultiTenantMixin` / `SoftDeleteMixin` model
+  (`secondary=Link.__table__`) filters the link rows too: its join condition
+  carries `(:all_tenants OR link.tenant_id = :tenant)` and
+  `(:include_deleted OR link.is_deleted IS false)`, bound on each execution
+  from the bound tenant, `all_tenants()` and the statement's
+  `all_tenants` / `include_deleted` options. With no tenant under strict mode
+  it matches no link row. An `update()` / `delete()` through the relationship
+  (`delete(Parent).where(Parent.links.any())`) keeps the tenant condition and
+  drops the soft-delete one, like every write. A bare association `Table`
+  that no mapped mixin class owns is not filtered, `tenant_id` column or not,
+  like every other unmapped table: map it with the mixins to have it scoped;
 - a table as a **join target**: `select(Project).join(Doc)`;
 - subqueries: `select(Doc.id).where(...).exists()`, a bare Core
   `exists().where(Doc.x == ...)`, `in_(select(Doc.x))`, scalar subqueries;
@@ -73,6 +86,33 @@ statement names, so all of these are scoped (#332):
   loader criteria sit in `WHERE`, and a table with no primary key, whose
   padding cannot be told apart from a real row.
 
+`aliased(Model, <subquery or CTE>)` is rendered from the selectable it was
+built over, which the filter does not rewrite. Built from ORM entities
+(`aliased(Proj, select(Proj).where(Proj.id.in_(select(Doc.proj_id))).subquery())`)
+it is scoped by the loader criteria. One that reads only the entity's own
+tables (`aliased(Doc, select(Doc.__table__).where(…).subquery())`; for joined
+inheritance, the tables of its mapper) is scoped on the rows it returns, unless
+it may combine rows first: `GROUP BY`, `HAVING`, a window function, or any
+function other than a row-wise one such as `lower`, `coalesce` or `abs`. So is `aliased(Doc, Doc.__table__.join(tag))`,
+a `Join` of the entity's own table with tables that are not scoped. Any other
+Core read of a scoped table inside it, and SQL text under an entity of a
+scoped table, is refused (see below).
+
+The engine's policy (strict mode, the fallback tenant, the audit sink) is
+found through a derived engine (`engine.execution_options(...)`) and through a
+session bound per model (`binds={...}`, the strictest bind wins). A model's
+table is known to the filter as soon as the class is mapped, so a Core
+statement on `Model.__table__` is scoped even before any ORM use configures
+the mappers.
+
+The filter runs on `Session` executions. A statement run on a plain
+`Connection` (`connection.execute(select(Model))`, or Core on `Model.__table__`)
+is not scoped: no loader criteria, no Core predicates. Only the `secondary`
+conditions above apply to it, because they sit in the relationship's join and
+are read on every engine from the bound tenant, the statement's options and the
+engine's strict mode (off when no policy is bound). Run statements through a
+session.
+
 With no tenant under strict mode, a statement that *names* a tenant model, or
 reads a tenant table through Core anywhere in it, raises; an indirect ORM
 reference (a join, a subquery) matches nothing. The cost is one walk of the
@@ -82,8 +122,22 @@ and rewritten. Both are skipped when no tenant-scoped or soft-deletable model
 is installed.
 
 Refused outside `all_tenants()` (`TenantIsolationError`), because the rows
-they write cannot be checked:
-
+they read or write cannot be checked:
+
+- `aliased(Model, <subquery or CTE>)` whose selectable reads a tenant-scoped
+  or soft-deletable table through Core — `select(Doc.__table__)` joined to
+  another table, a nested `select(doc.c.x)`, an `exists()` over one — other
+  than the entity's own table in its own `FROM`. Build the selectable from
+  ORM entities, or run it under `all_tenants()` (with
+  `include_deleted=True` where soft-delete applies). A relationship loader's
+  own statement (`subqueryload` wraps the parent query) is not refused: it
+  was scoped when the parent query ran;
+- `aliased(Model, <join>)` (`Proj.__table__.join(Doc.__table__)`) whose join
+  reads a tenant-scoped or soft-deletable table through Core, other than the
+  entity's own table joined to unscoped tables (a second occurrence of the
+  entity's own table, an alias of it included, counts as another table);
+- `aliased(Model, text(…).columns(…))`, bare or as a subquery or CTE, when
+  `Model` is tenant-scoped or soft-deletable: the filter cannot read SQL text;
 - an `INSERT` into a tenant table embedded in another statement as a CTE —
   run it as a statement of its own;
 - SQLite `insert(…).prefix_with("OR REPLACE")` on a tenant table while a
@@ -113,6 +167,16 @@ tenant. The flush refuses to write or delete it, but reading it is not
 prevented: give each tenant its own session (or `session.expunge_all()`
 between tenants) in jobs and CLI loops. Per-request sessions are unaffected.
 
+The unit of work updates and deletes by primary key, so the flush trusts an
+object's `tenant_id` only when a session loaded or inserted it. An object
+attached as already persistent — `make_transient_to_detached()`,
+`merge(load=False)`, a transient copy given another row's primary key, any
+detached object passed to `session.add()` — has its owner read from the table
+before the flush writes it, and a row of another tenant raises
+`TenantIsolationError`. A found owner is read once per identity; a missing row
+is read again on the next flush. On a non-strict install with no bound tenant
+the flush guard does not check owners, as before.
+
 ## Acting outside a request
 
 ```python

framework/db/pyproject.toml

@@ -29,7 +29,7 @@ dependencies = [
     # URL to +psycopg2 because Alembic's online migrations are sync-only.
     "psycopg2-binary>=2.9,<3",
     "simple_module_core==0.0.42",
-    "sqlalchemy[asyncio]>=2.0,<3",
+    "sqlalchemy[asyncio]>=2.0.2,<3",
     "sqlmodel>=0.0.22,<0.1",
 ]
 

framework/db/simple_module_db/_core_walk.py

@@ -21,7 +21,14 @@
 from typing import Any
 
 from sqlalchemy.sql.elements import ClauseElement, ColumnClause, ColumnElement
-from sqlalchemy.sql.selectable import Alias, Join, Select, TableClause
+from sqlalchemy.sql.selectable import (
+    Alias,
+    CompoundSelect,
+    Join,
+    Select,
+    TableClause,
+    TextualSelect,
+)
 from sqlalchemy.sql.traversals import _get_children
 
 from simple_module_db._core_joins import _base_name, _is_orm, is_scoped_leaf
@@ -86,14 +93,18 @@ def core_where_froms(stmt: Select) -> list[Any]:
     return found
 
 
-def references_plain(stmt: Any, names: set[str]) -> bool:
+def references_plain(stmt: Any, names: set[str], opaque: frozenset[int] = frozenset()) -> bool:
     """Whether ``stmt`` reads a scoped table outside the ORM's annotations.
 
     ORM-annotated elements are not descended into: their tables are the
-    loader criteria's business, and every ORM column leads to a plain table.
+    loader criteria's business, and every ORM column leads to a plain table —
+    except the selectable of an ``aliased(Model, <subquery or join>)`` entity,
+    which is whatever the caller built and is walked like any FROM, unless its
+    id is in ``opaque`` (SQL text under a scoped entity counts as a read).
     The answer depends only on *which* elements are reachable, so an element
-    met twice is expanded once. ``seen`` keeps the objects, not just their
-    ids, so a temporary child list cannot free one and recycle its id.
+    met twice — ORM-annotated and opaque ones included — is looked at once.
+    ``seen`` keeps the objects, not just their ids, so a temporary child list
+    cannot free one and recycle its id.
     """
     stack = [stmt]
     seen: dict[int, Any] = {}
@@ -103,9 +114,16 @@ def references_plain(stmt: Any, names: set[str]) -> bool:
             continue
         if el is not stmt and _nested_dml_on(el, names):
             return True  # its target is checked even when ORM-annotated
+        seen[id(el)] = el
         if _is_orm(el):
+            selectable = aliased_selectable(el)
+            if selectable is not None and id(selectable) not in opaque:
+                if reads_text_as_scoped(el._annotations["parententity"], selectable, names):
+                    return True  # SQL text: nothing in it can be seen, let alone scoped
+                stack.append(selectable)
+            continue
+        if id(el) in opaque:
             continue
-        seen[id(el)] = el
         target = el if isinstance(el, (TableClause, Alias)) else _table_of(el)
         if isinstance(target, (TableClause, Alias)) and is_scoped_leaf(target, names):
             return True
@@ -114,6 +132,36 @@ def references_plain(stmt: Any, names: set[str]) -> bool:
     return False
 
 
+def aliased_selectable(el: Any) -> Any | None:
+    """The selectable an ``aliased(Model, selectable)`` entity of ``el`` reads, if any.
+
+    A subquery / CTE of a SELECT (or of SQL text), a bare ``text().columns()``
+    or a ``Join`` of tables: whatever the caller built. ``aliased(Model)`` (an
+    alias of the mapped table) and other entities give ``None``: their tables
+    are covered by the loader criteria.
+    """
+    insp = el._annotations.get("parententity")
+    if insp is None or not insp.is_aliased_class:
+        return None
+    selectable = insp.selectable
+    if isinstance(selectable, (Join, TextualSelect)):
+        return selectable
+    inner = getattr(selectable, "element", None)
+    return selectable if isinstance(inner, (Select, CompoundSelect, TextualSelect)) else None
+
+
+def is_textual(selectable: Any) -> bool:
+    """``text(...).columns(...)``, bare or as a subquery / CTE."""
+    return isinstance(selectable, TextualSelect) or isinstance(
+        getattr(selectable, "element", None), TextualSelect
+    )
+
+
+def reads_text_as_scoped(insp: Any, selectable: Any, names: set[str]) -> bool:
+    """An entity of a scoped table whose rows come from SQL text, which no filter can read."""
+    return is_textual(selectable) and _base_name(insp.mapper.local_table) in names
+
+
 _DML = ("insert", "update", "delete")
 
 

framework/db/simple_module_db/_engine_policies.py

@@ -0,0 +1,80 @@
+"""Per-engine tenancy policy, resolved through derived engines and session binds.
+
+Keyed by engine rather than held in a module global so two ``DatabaseState``s
+in one process (tests, a CLI next to an app) cannot switch each other's
+strict mode off. ``engine.execution_options(...)`` returns an ``OptionEngine``
+that proxies the engine it came from, and a session may be bound per model or
+table (``binds={...}``) instead of to one engine: both resolve to the policy
+of the base engine.
+"""
+
+from __future__ import annotations
+
+from typing import Any
+from weakref import WeakKeyDictionary
+
+from sqlalchemy.engine import Connection, Engine
+from sqlalchemy.exc import UnboundExecutionError
+from sqlalchemy.orm import Session
+from sqlalchemy.orm.exc import UnmappedClassError
+
+from simple_module_db import model_registry as registry
+
+_POLICY_KEY = "sm_engine_policy"
+_engine_policy: WeakKeyDictionary[Engine, Any] = WeakKeyDictionary()
+_NO_POLICY = object()  # cached for a session none of whose binds has a policy
+_bound = 0  # bumped by ``set_policy``: a memoized answer older than it is stale
+
+
+def set_policy(engine: Engine, policy: Any) -> None:
+    """Attach ``policy`` to ``engine`` and invalidate every session's memoized answer."""
+    global _bound
+    _engine_policy[engine] = policy
+    _bound += 1
+
+
+def policy_of(bind: Any) -> Any:
+    """The policy of ``bind`` (an engine, a derived engine or a connection), or ``None``."""
+    if isinstance(bind, Connection):
+        bind = bind.engine
+    while isinstance(bind, Engine):
+        found = _engine_policy.get(bind)
+        if found is not None:
+            return found
+        bind = getattr(bind, "_proxied", None)  # ``OptionEngine`` -> its base engine
+    return None
+
+
+def session_policy(session: Session) -> Any:
+    """The policy for ``session``: the strictest of the binds ``get_bind`` resolves.
+
+    Asked through the public ``Session.get_bind`` — once without a mapper and
+    once per tenant-scoped model — so per-model binds and a subclass that
+    overrides ``get_bind`` keep the strict policy of the engine they route to.
+    A class that is no longer mapped (or has no bind) is skipped, never raised.
+    The answer (a policy, or none) is memoized on ``session.info`` for the
+    session's ``bind``, the number of tenant models and the policy bindings
+    made; a bind added with ``bind_mapper()`` after the first statement is not
+    seen.
+    """
+    count = (len(registry.tenant_classes), _bound)
+    cached = session.info.get(_POLICY_KEY)
+    if cached is not None and cached[0] is session.bind and cached[1] == count:
+        return None if cached[2] is _NO_POLICY else cached[2]
+    found: dict[int, Any] = {}
+    for mapper in (None, *registry.tenant_classes):
+        try:
+            bind = session.get_bind(mapper=mapper)
+        except (UnboundExecutionError, UnmappedClassError):
+            continue
+        policy = policy_of(bind)
+        if policy is not None:
+            found.setdefault(id(policy), policy)
+    policies = list(found.values())
+    strict = [p for p in policies if getattr(p, "tenant_strict", False)]
+    policy = (strict or policies or [None])[0]
+    session.info[_POLICY_KEY] = (session.bind, count, _NO_POLICY if policy is None else policy)
+    return policy
+
+
+__all__ = ["policy_of", "session_policy", "set_policy"]