Tech-debt batch 3: tenant teardown, backend-mismatch guard, checksum reconciler (#583 #589 #590)

CHANGELOG.md

@@ -10,11 +10,32 @@ All notable changes to this project are documented in this file. The format is b
 > Versions not listed below still have no entry; consult the commit log.
 >
 > 0.0.28 through 0.0.37 were released before the release workflow cut this
-> file (#572), so their changes are still listed under `[Unreleased]` and will
-> land in the next dated section (#584).
+> file (#572), so their changes are listed under `[0.0.38]` (#584).
 
 ## [Unreleased]
 
+### Added
+- `EventBus.publish_durable_or_inline(event, db)`: enqueue durably when a transport is
+  installed, otherwise run the durable handlers inline in `db`'s transaction (#583).
+
+### Fixed
+- Tenants: deleting a tenant now tears down its data. `TenantDeleted` (now defined in
+  `simple_module_core.tenant_events`, still importable from `tenants.contracts.events`) is
+  published durably, and file_storage (files, upload sessions, then backend objects after
+  commit), api_tokens, sharing, feature_flags and the framework's tenant-scoped settings
+  overrides (including branding) purge that tenant's rows; audit history is kept. On multi-tenant
+  installs, public files of a deleted tenant answer 404 at once (the existence check is cached
+  60 s per worker and evicted on every worker by `TENANT_DELETED_CHANNEL`). Without `background_tasks` the handlers run inline in
+  the deleting transaction. Rows orphaned by earlier deletions are not purged retroactively (#583).
+- File storage: a file whose `backend` is not the active backend is no longer read or deleted
+  through the wrong store: downloads, public reads and thumbnails answer 409 with a translated
+  message, and a delete soft-deletes the row but leaves the object alone (logged). Switching
+  backends still needs a manual migration; see `docs/modules/file_storage.md` (#589).
+- Background tasks: `enqueue_after_commit` is documented as at-most-once (a broker outage after
+  commit loses the task; use `publish_durable` for must-run work). file_storage gains an hourly
+  `file_storage.reconcile_checksums` beat task that re-enqueues checksums for files left without
+  one for 30 minutes, up to 500 per run (#590).
+
 ## [0.0.38] — 2026-10-23
 
 ### Fixed

docs/framework/multi-tenancy.md

@@ -345,6 +345,48 @@ filter. A platform admin's actions are attributed to their **active**
 organisation when one is active — the write itself is scoped to it — and to the
 platform only when nothing is bound. See [audit_log](/modules/audit_log#multi-tenancy).
 
+## Deleting tenants
+
+Tenant deletion publishes `TenantDeleted` through
+`bus.publish_durable_or_inline` in the same transaction that removes the
+tenant, memberships and invitations. With the `background_tasks` outbox
+transport installed, delivery is durable and eventual (retried until it
+succeeds). Without it, the durable handlers run inline in the deleting
+transaction, so their row purges commit or roll back with the tenant row;
+backend objects are then deleted after commit by an `on_commit` callback,
+which is best-effort: a failure there is logged with the object keys, and
+the hourly `file_storage.reap_objects` task removes the orphaned objects once
+`background_tasks` is enabled. Either way, public file downloads check the
+tenant directory (cached for 60 s per worker, forgotten on `TenantDeleted`)
+and return 404 when the tenant no longer exists.
+
+Every owner of tenant data must register
+`bus.subscribe_durable(TenantDeleted, handler)` in `register_event_handlers`,
+including when `app=None` in the worker. Import the event from
+`simple_module_core.tenant_events`; no dependency on the `tenants` plugin is
+needed. Delete only `event.tenant_id`'s rows, using `tenant_context` for
+`MultiTenantMixin` tables, and hard-delete soft-deleted rows too. Explicit
+scope tables need matching scope and tenant predicates. Handlers must tolerate
+repeated deliveries and must never commit `ctx.db`; the outbox owns that
+transaction. Backend object deletion belongs after commit and must also be
+idempotent. File storage writes a second durable cleanup event containing the
+backend keys in the same transaction that hard-deletes its rows. Only after
+that transaction commits does the second handler remove originals, thumbnail
+variants and unfinished multipart uploads; backend failures retry from the
+persisted manifest rather than losing the keys.
+
+The framework registers cleanup of tenant-scoped `Setting` rows in both host
+and worker, even if the settings UI plugin is disabled. This includes branding
+overrides; branding's uploaded files belong to file storage cleanup. System
+and user settings and other tenants' rows are preserved. Feature flags also
+purges its tenant-scoped persisted overrides; system overrides survive.
+
+Audit entries are deliberately retained for historical accountability. Their
+original `tenant_id` remains the historical tenant marker even after the live
+tenant row disappears; readers must not require a live tenant join to display
+these entries. Deletion does not reassign audit entries to another tenant or
+to the platform, and audit retention policy still applies independently.
+
 ## Testing
 
 The `simple_module_test` plugin ships `tenant_client` (needs the `users` and

docs/modules/background_tasks.md

@@ -54,6 +54,20 @@ async def place(self, data: OrderCreate) -> OrderOut:
 - In a request (`get_db`), the publish runs from the session's `on_commit` hook — after the commit, before the response leaves.
 - Any other session — a task body's sync session, a CLI script's own `async with session.begin()` — hangs it off SQLAlchemy's `after_commit` event instead, and drops it when the outermost transaction ends without committing. Publishing then blocks the committing thread for the length of one kombu publish.
 - The task runs as the tenant bound when you called `enqueue_after_commit`, not whatever is bound at commit time.
+
+**Delivery is at-most-once.** The publish intent lives only in memory. A broker
+failure after commit is logged as `background_tasks.after_commit.publish_failed`
+without retry; a process exit between commit and publish also loses the job.
+For must-run work, use the event bus's `publish_durable(event, db)` with a durable
+handler (see durable events below), which persists outbox intent in the same
+transaction. This helper itself provides no execution or redelivery guarantee.
+
+File storage recovers missing checksum jobs with the hourly
+`file_storage.reconcile_checksums` beat task: it re-enqueues up to 500 completed,
+non-deleted files whose checksum is null and whose creation was more than 30
+minutes ago, across tenants. Failed publishes remain eligible on subsequent
+runs. Run beat and a worker for this recovery; duplicate checksum jobs are safe.
+
 - A publish that fails after the commit is logged, not raised: the transaction is already durable.
 
 The admin retry buttons follow the same rule — the new `pending` row commits first, then the task is sent (GH #452). If the send itself then fails, the new row is flipped to `failed` so the retry buttons can pick it up again; a task with no row is neither.

docs/modules/file_storage.md

@@ -386,3 +386,33 @@ class MyModule(ModuleBase):
 ## Locales
 
 Top-level keys in `file_storage/locales/en.json`: `browse`, `table`, `actions`, `delete_dialog`, `toasts`, `errors`. The `errors` namespace is keyed by error *code* (`not_found`, `too_large`, `bad_type`, `backend_error`, `bad_image`) so the UI can render a deterministic message per `StorageError` subclass.
+
+### Changing storage providers or locations
+
+Storage uses one active backend configuration; it does not retain or resolve
+historical configurations. A stored file records the provider ID (`filesystem`
+or `s3`), not the filesystem root, bucket, endpoint, or credentials. When its
+provider ID differs from the active provider, downloads, public reads and
+thumbnails fail with HTTP 409 and a translated message that the file is stored
+on a backend that is no longer configured. Checksum jobs also refuse the read.
+Deleting such a file still soft-deletes its row, but skips the original object
+and thumbnail deletion and logs a warning. Retain the old store for manual
+cleanup; the object reaper only handles the active provider.
+
+Changes within the same provider (for example a different filesystem root or
+S3 bucket) cannot be detected by this check. Do not switch locations while
+existing rows still depend on the old location. Upload sessions do not record
+a backend ID: finish or abort all active multipart sessions before any backend
+or location change, including sessions belonging to other users and tenants.
+
+For a manual migration, pause uploads and background storage jobs, drain active
+multipart sessions, and back up the database and both stores. Copy originals
+and any thumbnail variants to the destination under exactly the same object
+keys, including tenant prefixes; verify sizes and checksums before switching.
+When changing provider type, update the affected `StoredFile.backend` values to
+the destination provider ID in a controlled database maintenance transaction
+only after verifying the copies. Configure the destination, restart application
+and worker processes, and verify authenticated/public downloads, ranged reads,
+thumbnails and deletion before resuming writes. Keep the source and backups
+until verification is complete, then remove source objects explicitly. There
+is no automatic migration command or historical-backend registry.

framework/core/simple_module_core/events.py

@@ -11,6 +11,7 @@
 from pyee.asyncio import AsyncIOEventEmitter
 
 from simple_module_core.durable_events import (
+    DurableContext,
     DurableDeliveryUnavailable,
     DurableHandler,
     DurableRegistry,
@@ -141,3 +142,22 @@ async def publish_durable(self, event: Event, db: Any) -> None:
         if not names:
             return
         await transport.enqueue(event, names, db)
+
+    async def publish_durable_or_inline(self, event: Event, db: Any) -> None:
+        """Enqueue with a transport, otherwise run durable handlers in *db*.
+
+        Inline handlers run sequentially in subscription order on the caller's
+        transaction with attempt=1. They must flush rather than commit; errors
+        propagate so the caller can roll back. Ordinary subscribers are not run.
+        External effects must be deferred to the caller's post-commit hooks.
+        """
+        if self.durable_transport is not None:
+            await self.publish_durable(event, db)
+            return
+        ctx = DurableContext(db=db, attempt=1)
+        key = self._event_key(type(event))
+        for name in self.durable.handler_names(type(event)):
+            subscription = self.durable.lookup(key, name)
+            if subscription is None:
+                continue
+            await subscription.handler(event, ctx)

framework/core/simple_module_core/tenant_events.py

@@ -0,0 +1,20 @@
+"""Tenant lifecycle contracts shared without a dependency on a tenant provider."""
+
+from dataclasses import dataclass
+
+from simple_module_core.events import Event
+
+TENANT_DELETED_CHANNEL = "tenants.deleted"
+
+
+@dataclass
+class TenantDeleted(Event):
+    """The tenant registry row is gone; owners must durably tear down its data.
+
+    Delivery is at least once. Retention stores may keep historical rows,
+    identified by their original tenant id, but must never expose live resources.
+    """
+
+    tenant_id: str
+    slug: str
+    name: str

framework/core/tests/test_events_inline.py

@@ -0,0 +1,38 @@
+"""Explicit inline fallback preserves transaction ownership and error propagation."""
+
+from unittest.mock import AsyncMock
+
+import pytest
+from simple_module_core.durable_events import DurableContext
+from simple_module_core.events import EventBus
+from simple_module_core.tenant_events import TenantDeleted
+
+
+@pytest.mark.parametrize("transport", [False, True])
+async def test_durable_or_inline_dispatch(transport):
+    bus = EventBus()
+    db = object()
+    event = TenantDeleted("id", "slug", "Name")
+    handlers = [AsyncMock(), AsyncMock()]
+    for index, handler in enumerate(handlers):
+        bus.subscribe_durable(TenantDeleted, handler, name=str(index))
+    ordinary = AsyncMock()
+    bus.subscribe(TenantDeleted, ordinary)
+    if transport:
+        bus.durable_transport = AsyncMock()
+    await bus.publish_durable_or_inline(event, db)
+    for handler in handlers:
+        if transport:
+            handler.assert_not_awaited()
+        else:
+            handler.assert_awaited_once_with(event, DurableContext(db=db, attempt=1))
+    ordinary.assert_not_awaited()
+    if transport:
+        bus.durable_transport.enqueue.assert_awaited_once_with(event, ["0", "1"], db)
+
+
+async def test_inline_failure_propagates():
+    bus = EventBus()
+    bus.subscribe_durable(TenantDeleted, AsyncMock(side_effect=ValueError("failed")), name="fail")
+    with pytest.raises(ValueError, match="failed"):
+        await bus.publish_durable_or_inline(TenantDeleted("id", "slug", "Name"), object())

framework/db/simple_module_db/tenant_cleanup.py

@@ -0,0 +1,22 @@
+"""Durable teardown of framework-owned tenant settings, including branding."""
+
+from simple_module_core.durable_events import DurableContext
+from simple_module_core.events import EventBus
+from simple_module_core.tenant_events import TenantDeleted
+from sqlmodel import col, delete
+
+from simple_module_db.stored_settings import SCOPE_TENANT, Setting
+
+
+def register_tenant_cleanup(bus: EventBus) -> None:
+    """Register in both the host and workers, even without the settings plugin."""
+    bus.subscribe_durable(TenantDeleted, delete_tenant_settings)
+
+
+async def delete_tenant_settings(event: TenantDeleted, ctx: DurableContext) -> None:
+    """Hard-delete only this tenant's explicit overrides; safe on replay."""
+    await ctx.db.execute(
+        delete(Setting).where(
+            col(Setting.scope) == SCOPE_TENANT, col(Setting.scope_id) == event.tenant_id
+        )
+    )

framework/db/tests/test_db_tenant_cleanup.py

@@ -0,0 +1,58 @@
+"""Framework tenant settings teardown works independently of the UI plugin."""
+
+from unittest.mock import AsyncMock, patch
+
+from simple_module_core.durable_events import DurableContext
+from simple_module_core.events import EventBus
+from simple_module_core.tenant_events import TenantDeleted
+from simple_module_db.stored_settings import SCOPE_SYSTEM, SCOPE_TENANT, SCOPE_USER, Setting
+from simple_module_db.tenant_cleanup import register_tenant_cleanup
+from sqlalchemy.ext.asyncio import AsyncSession, create_async_engine
+from sqlmodel import select
+
+_TENANT = "deleted-tenant"
+_OTHER = "retained-tenant"
+_KEY = "branding.logo_file_id"
+
+
+async def test_settings_cleanup_is_durable_scoped_and_idempotent():
+    bus = EventBus()
+    register_tenant_cleanup(bus)
+    names = bus.durable.handler_names(TenantDeleted)
+    assert len(names) == 1
+    from simple_module_core.durable_events import event_key
+
+    subscription = bus.durable.lookup(event_key(TenantDeleted), names[0])
+    assert subscription is not None
+    engine = create_async_engine("sqlite+aiosqlite:///:memory:")
+    try:
+        async with engine.begin() as conn:
+            await conn.run_sync(Setting.metadata.create_all, tables=[Setting.__table__])
+        async with AsyncSession(engine) as session:
+            session.add_all(
+                [
+                    Setting(scope=scope, scope_id=scope_id, key=_KEY, value="file-id")
+                    for scope, scope_id in (
+                        (SCOPE_TENANT, _TENANT),
+                        (SCOPE_TENANT, _OTHER),
+                        (SCOPE_USER, _TENANT),
+                        (SCOPE_SYSTEM, ""),
+                    )
+                ]
+            )
+            await session.commit()
+            event = TenantDeleted(tenant_id=_TENANT, slug=_TENANT, name=_TENANT)
+            with patch.object(session, "commit", new_callable=AsyncMock) as commit:
+                await subscription.handler(event, DurableContext(db=session, attempt=1))
+                await subscription.handler(event, DurableContext(db=session, attempt=2))
+                commit.assert_not_awaited()
+            remaining = (await session.execute(select(Setting))).scalars().all()
+            assert {(row.scope, row.scope_id) for row in remaining} == {
+                (SCOPE_TENANT, _OTHER),
+                (SCOPE_USER, _TENANT),
+                (SCOPE_SYSTEM, ""),
+            }
+            await session.rollback()
+            assert len((await session.execute(select(Setting))).scalars().all()) == 4
+    finally:
+        await engine.dispose()

framework/hosting/simple_module_hosting/app_builder.py

@@ -20,6 +20,7 @@
 from simple_module_core.resources import ResourceTypeRegistry
 from simple_module_core.services import DiagnosticsState, Services
 from simple_module_core.setup_steps import SetupRegistry
+from simple_module_db.tenant_cleanup import register_tenant_cleanup
 
 from simple_module_hosting._app_boot import (
     build_fastapi_app,
@@ -128,6 +129,7 @@ def create_app(settings: Settings | None = None) -> FastAPI:
     perm_registry = PermissionRegistry()
     ff_registry = FeatureFlagRegistry()
     event_bus = EventBus()
+    register_tenant_cleanup(event_bus)
     invalidation_bus = InvalidationBus()
     health_registry = HealthRegistry()
     public_route_registry = PublicRouteRegistry()

modules/api_tokens/api_tokens/module.py

@@ -17,6 +17,7 @@
 if TYPE_CHECKING:
     from fastapi import APIRouter, FastAPI
     from simple_module_core.audit_links import AuditLinkRegistry
+    from simple_module_core.events import EventBus
     from simple_module_core.permissions import PermissionRegistry
 
 
@@ -35,6 +36,13 @@ class ApiTokensModule(ModuleBase):
         optional_depends_on=[c.MODULE_USERS, c.MODULE_SETTINGS],
     )
 
+    def register_event_handlers(self, bus: EventBus, app: FastAPI | None = None) -> None:
+        from simple_module_core.tenant_events import TenantDeleted
+
+        from api_tokens.tenant_cleanup import purge_tenant_data
+
+        bus.subscribe_durable(TenantDeleted, purge_tenant_data)
+
     def register_settings(self, app: FastAPI) -> None:
         from api_tokens.services import ApiTokensServices
         from api_tokens.settings import ApiTokensSettings

modules/api_tokens/api_tokens/tenant_cleanup.py

@@ -0,0 +1,14 @@
+"""Durable teardown of tenant-owned api_tokens rows."""
+
+from simple_module_core.durable_events import DurableContext
+from simple_module_core.tenant_events import TenantDeleted
+from simple_module_db.tenancy import tenant_context
+from sqlmodel import delete
+
+from api_tokens.models import ApiToken
+
+
+async def purge_tenant_data(event: TenantDeleted, ctx: DurableContext) -> None:
+    """Hard-delete tenant rows atomically with outbox acknowledgement."""
+    with tenant_context(event.tenant_id):
+        await ctx.db.execute(delete(ApiToken).where(ApiToken.tenant_id == event.tenant_id))