refactor(sharing): follow the module layout (moves only)

CHANGELOG.md

@@ -19,6 +19,7 @@ All notable changes to this project are documented in this file. The format is b
 - Scaffold new modules with `core/` and `web/` areas and mirrored nested Python tests; existing modules migrate individually. See `docs/module-layout.md`.
 - `mail` follows the module layout: `mail.core.{constants,settings,state,data_compatibility}` and `mail.delivery.{sender,runtime}`. `mail.settings` and `mail.constants` still import, with a `DeprecationWarning`, until 0.0.46.
 - `site_lock` follows the module layout: `site_lock.core.{constants,settings,state}` and `site_lock.web.{middleware,page,rate_limit}` (the unlock template moves with the page). `site_lock.settings` still imports, with a `DeprecationWarning`, until 0.0.46.
+- `sharing` follows the module layout: `sharing.core.{constants,settings,services,service,policy,audience,tenant_cleanup,user_cleanup}`, `sharing.web.{deps,present,endpoints}`, and the page components and types move to `ui/`. `sharing.policy`, `sharing.constants` and `sharing.endpoints.api` still import, with a `DeprecationWarning`, until 0.0.46; new code imports `sharing.core.policy`.
 - `dashboard` follows the module layout: `dashboard.core.constants`, `dashboard.web.{endpoints,stats,doctor}`, and the page components move from `pages/components/` to `ui/components/`. Nothing outside the module imported the old paths, so no aliases remain.
 - `feature_flags` follows the module layout: `feature_flags.core.{constants,service,invalidation,tenant_cleanup}`, `feature_flags.web.{deps,endpoints}`, and the page components and types move to `ui/`. `feature_flags.constants` still imports, with a `DeprecationWarning`, until 0.0.46.
 

docs/module-layout.md

@@ -120,6 +120,12 @@ loads non-package tests by basename, even when they live in different folders.
 Existing flat test names are grandfathered only while their module remains
 in `LEGACY_FLAT`; migration renames them with the module prefix.
 
+A helper shared by several test folders stays at `tests/` root as
+`_<module>_support.py`. The root `conftest.py` puts that directory on
+`sys.path`, so `from _<module>_support import ...` keeps working from nested
+folders; a module that has helpers needs that `conftest.py` even when it holds
+no fixtures.
+
 The root `pyproject.toml:137` lists `modules/<name>/tests` in `testpaths`, and
 `framework/cli/simple_module_cli/framework_repo.py` wires that root. Pytest
 recursively collects nested areas without listing each child directory.
@@ -157,7 +163,10 @@ The alias makes both paths the same module object, so mocks and
 `monkeypatch` strings keep working. Files containing both `deprecated_module(`
 and `sys.modules[__name__]` are auto-discovered by the expiry test and ignored
 by the root-layout test, so the module can leave `LEGACY_FLAT` while they
-remain. A shim that is a plain re-export instead (not an alias) must be added
+remain. When the old path was inside a package that moved (for example
+`orders.endpoints.api`), keep that package with a docstring-only `__init__.py`
+and alias each module in it; a package holding only aliases is ignored by the
+root-layout test too. A shim that is a plain re-export instead (not an alias) must be added
 by hand to the expiry test's `_SHIM_MODULES` tuple and `_EXPECTED` set and kept
 in the module's frozen legacy entries until removed. A completely migrated
 module leaves `LEGACY_FLAT` once its Python tests are nested.
@@ -209,7 +218,7 @@ when a later module PR moves files.
 | oidc | 16 | 24 | flat |
 | permissions | 12 | 14 | flat |
 | settings | 31 | 43 | flat |
-| sharing | 13 | 6 | flat |
+| sharing | 3 (+3 shims) | 0 | follows this layout |
 | site_lock | 2 (+1 shim) | 0 | follows this layout |
 | tenants | 19 | 23 | flat |
 | users | 31 | 97 | flat |

docs/modules/sharing.md

@@ -20,7 +20,7 @@ own share model. Nothing depends on it: the framework only sees the
 ```python
 from simple_module_core.resources import ResourceType
 from simple_module_db.resource_policy import OwnedResourcePolicy, select_by_id
-from sharing.policy import share_exists_for
+from sharing.core.policy import share_exists_for
 
 NOTE_TYPE = "notes.note"
 NOTE_POLICY = OwnedResourcePolicy(
@@ -37,7 +37,7 @@ class NotesModule(ModuleBase):
         )
 ```
 
-A module that uses `sharing.policy` must list `Sharing` in `depends_on`.
+A module that uses `sharing.core.policy` must list `Sharing` in `depends_on`.
 
 By default, `ROLE_IMPLIES` supplies two roles: `view` grants view; `edit` grants
 view and edit. Set the keyword-only `ResourceType.share_roles` to a mapping of

host/tests/test_module_layout.py

@@ -111,12 +111,6 @@
         reload.py service.py services.py settings.py store.py tenant_scope.py tenant_view.py
         """.split()
     ),
-    "sharing": frozenset(
-        """
-        audience.py constants.py deps.py endpoints policy.py present.py service.py
-        services.py settings.py tenant_cleanup.py user_cleanup.py
-        """.split()
-    ),
     "tenants": frozenset(
         """
         _deletion.py _membership_ops.py _owner_guard.py components constants.py deps.py
@@ -144,6 +138,12 @@ def _is_layout_shim(path: Path) -> bool:
     return "deprecated_module(" in text and "sys.modules[__name__]" in text
 
 
+def _is_shim_package(path: Path) -> bool:
+    """An old package path kept only for its aliases: shims plus a docstring __init__."""
+    files = [f for f in path.glob("*.py") if f.name != "__init__.py"]
+    return bool(files) and all(_is_layout_shim(f) for f in files)
+
+
 @pytest.mark.parametrize("module", sorted(p.name for p in MODULES.iterdir() if p.is_dir()))
 def test_module_root_layout(module: str) -> None:
     package = MODULES / module / module
@@ -152,7 +152,7 @@ def test_module_root_layout(module: str) -> None:
         p.name
         for p in package.iterdir()
         if (p.suffix == ".py" and not _is_layout_shim(p))
-        or (p.is_dir() and p.name != "__pycache__")
+        or (p.is_dir() and p.name != "__pycache__" and not _is_shim_package(p))
     }
     allowed = ALLOWED_ROOT | DOMAIN_AREAS.get(module, frozenset())
     legacy = LEGACY_FLAT.get(module, frozenset())

modules/sharing/README.md

@@ -24,7 +24,7 @@ Declare the resource type in the owning module and hand the policy a
 `share_exists` factory:
 
 ```python
-from sharing.policy import share_exists_for
+from sharing.core.policy import share_exists_for
 
 NOTE_POLICY = OwnedResourcePolicy(
     Note.owner_id,

modules/sharing/sharing/constants.py

@@ -1,65 +1,11 @@
-"""Stable identifiers for the sharing module."""
+"""Moved to :mod:`sharing.core.constants`; this alias goes in 0.0.46 (module layout)."""
 
 from __future__ import annotations
 
-from typing import Final
+import importlib
+import sys
 
-from simple_module_core.resources import ACTION_EDIT, ACTION_VIEW
+from simple_module_core.deprecation import deprecated_module
 
-MODULE_NAME: Final = "Sharing"
-MODULE_PACKAGE: Final = "sharing"
-LOCALE_NAMESPACE: Final = "sharing"
-
-PERM_GROUP: Final = "Sharing"
-PERM_SHARE: Final = "sharing.share"
-"""Use the grant/revoke API at all; the resource's own policy still decides."""
-PERM_VIEW: Final = "sharing.view"
-"""See every share in the tenant (the admin page)."""
-PERM_MANAGE: Final = "sharing.manage"
-"""Revoke any share in the tenant."""
-USER_ROLE: Final = "user"
-
-PAGE_ADMIN: Final = "Sharing/Admin"
-
-API_PREFIX: Final = "/api/sharing"
-VIEW_PREFIX: Final = "/admin/sharing"
-# Trailing slash: the index route is registered at "/" under the prefix.
-ADMIN_URL: Final = f"{VIEW_PREFIX}/"
-AUDIT_URL_TEMPLATE: Final = f"{VIEW_PREFIX}/?share={{id}}"
-
-MENU_LABEL: Final = "Sharing"
-MENU_LABEL_KEY: Final = "sharing.nav.title"
-MENU_ICON: Final = "share"
-MENU_ORDER: Final = 60
-MENU_GROUP: Final = "Access"
-MENU_GROUP_KEY: Final = "ui.nav_groups.access"
-AUDIT_LABEL: Final = "Share"
-AUDIT_LABEL_KEY: Final = "sharing.audit.share"
-
-# Prefix is load-bearing: all modules share one schema.
-TABLE_SHARE: Final = "sharing_resource_share"
-UQ_SHARE: Final = "uq_sharing_resource_share_subject"
-IX_SHARE_RESOURCE: Final = "ix_sharing_resource_share_resource"
-
-RESOURCE_TYPE_MAX: Final = 100
-RESOURCE_ID_MAX: Final = 100
-SUBJECT_TYPE_MAX: Final = 16
-SUBJECT_ID_MAX: Final = 255
-ROLE_MAX: Final = 32
-
-SUBJECT_USER: Final = "user"
-"""The only subject kind so far; the column leaves room for groups or tenants."""
-
-# A role is the action it grants plus everything it implies.
-ROLE_IMPLIES: Final[dict[str, tuple[str, ...]]] = {
-    ACTION_VIEW: (ACTION_VIEW,),
-    ACTION_EDIT: (ACTION_VIEW, ACTION_EDIT),
-}
-
-ERR_UNKNOWN_TYPE: Final = "Unknown resource type"
-ERR_NOT_FOUND: Final = "Not found"
-ERR_FORBIDDEN: Final = "You may not share this resource"
-ERR_UNKNOWN_GRANTEE: Final = "Unknown user"
-ERR_BAD_ROLE: Final = "This role cannot be granted on this resource type"
-ERR_EXPIRED: Final = "expires_at must be in the future"
-ERR_SELF: Final = "You cannot share a resource with yourself"
+deprecated_module("sharing.constants", remove_in="0.0.46", use="sharing.core.constants")
+sys.modules[__name__] = importlib.import_module("sharing.core.constants")

modules/sharing/sharing/contracts/schemas.py

@@ -8,7 +8,7 @@
 from simple_module_core.resources import ACTION_VIEW
 from sqlmodel import Field, SQLModel
 
-from sharing.constants import RESOURCE_ID_MAX, RESOURCE_TYPE_MAX, ROLE_MAX, SUBJECT_ID_MAX
+from sharing.core.constants import RESOURCE_ID_MAX, RESOURCE_TYPE_MAX, ROLE_MAX, SUBJECT_ID_MAX
 
 
 class ShareGrant(SQLModel):

modules/sharing/sharing/core/__init__.py

@@ -0,0 +1,5 @@
+"""What every sharing feature builds on.
+
+Constants, settings, the services container, the share service, policies,
+audiences and cleanup hooks.
+"""

modules/sharing/sharing/core/constants.py

@@ -0,0 +1,65 @@
+"""Stable identifiers for the sharing module."""
+
+from __future__ import annotations
+
+from typing import Final
+
+from simple_module_core.resources import ACTION_EDIT, ACTION_VIEW
+
+MODULE_NAME: Final = "Sharing"
+MODULE_PACKAGE: Final = "sharing"
+LOCALE_NAMESPACE: Final = "sharing"
+
+PERM_GROUP: Final = "Sharing"
+PERM_SHARE: Final = "sharing.share"
+"""Use the grant/revoke API at all; the resource's own policy still decides."""
+PERM_VIEW: Final = "sharing.view"
+"""See every share in the tenant (the admin page)."""
+PERM_MANAGE: Final = "sharing.manage"
+"""Revoke any share in the tenant."""
+USER_ROLE: Final = "user"
+
+PAGE_ADMIN: Final = "Sharing/Admin"
+
+API_PREFIX: Final = "/api/sharing"
+VIEW_PREFIX: Final = "/admin/sharing"
+# Trailing slash: the index route is registered at "/" under the prefix.
+ADMIN_URL: Final = f"{VIEW_PREFIX}/"
+AUDIT_URL_TEMPLATE: Final = f"{VIEW_PREFIX}/?share={{id}}"
+
+MENU_LABEL: Final = "Sharing"
+MENU_LABEL_KEY: Final = "sharing.nav.title"
+MENU_ICON: Final = "share"
+MENU_ORDER: Final = 60
+MENU_GROUP: Final = "Access"
+MENU_GROUP_KEY: Final = "ui.nav_groups.access"
+AUDIT_LABEL: Final = "Share"
+AUDIT_LABEL_KEY: Final = "sharing.audit.share"
+
+# Prefix is load-bearing: all modules share one schema.
+TABLE_SHARE: Final = "sharing_resource_share"
+UQ_SHARE: Final = "uq_sharing_resource_share_subject"
+IX_SHARE_RESOURCE: Final = "ix_sharing_resource_share_resource"
+
+RESOURCE_TYPE_MAX: Final = 100
+RESOURCE_ID_MAX: Final = 100
+SUBJECT_TYPE_MAX: Final = 16
+SUBJECT_ID_MAX: Final = 255
+ROLE_MAX: Final = 32
+
+SUBJECT_USER: Final = "user"
+"""The only subject kind so far; the column leaves room for groups or tenants."""
+
+# A role is the action it grants plus everything it implies.
+ROLE_IMPLIES: Final[dict[str, tuple[str, ...]]] = {
+    ACTION_VIEW: (ACTION_VIEW,),
+    ACTION_EDIT: (ACTION_VIEW, ACTION_EDIT),
+}
+
+ERR_UNKNOWN_TYPE: Final = "Unknown resource type"
+ERR_NOT_FOUND: Final = "Not found"
+ERR_FORBIDDEN: Final = "You may not share this resource"
+ERR_UNKNOWN_GRANTEE: Final = "Unknown user"
+ERR_BAD_ROLE: Final = "This role cannot be granted on this resource type"
+ERR_EXPIRED: Final = "expires_at must be in the future"
+ERR_SELF: Final = "You cannot share a resource with yourself"

modules/sharing/sharing/core/policy.py

@@ -0,0 +1,119 @@
+"""Plug shares into a module's ``OwnedResourcePolicy`` (GH #526).
+
+::
+
+    from sharing.core.policy import share_exists_for
+
+    NOTE_POLICY = OwnedResourcePolicy(
+        Note.owner_id,
+        bypass_perm=PERM_NOTES_MANAGE,
+        share_exists=share_exists_for(NOTE_TYPE, Note.id),
+    )
+"""
+
+from __future__ import annotations
+
+import uuid
+from collections.abc import Callable, Mapping
+from datetime import UTC, datetime
+from typing import Any
+
+from simple_module_core.resources import ResourcePrincipal
+from sqlalchemy import String, cast, func, inspect, or_
+from sqlmodel import exists, select
+
+from sharing.core.constants import ROLE_IMPLIES, SUBJECT_USER
+from sharing.models import ResourceShare
+
+
+def resolve_roles(roles: Mapping[str, tuple[str, ...]] | None) -> Mapping[str, tuple[str, ...]]:
+    """*roles* as declared on a resource type; ``None`` means the default view/edit roles."""
+    return ROLE_IMPLIES if roles is None else roles
+
+
+def roles_granting(
+    action: str, *, roles: Mapping[str, tuple[str, ...]] | None = None
+) -> tuple[str, ...]:
+    """Every share role that implies *action*."""
+    return tuple(role for role, implied in resolve_roles(roles).items() if action in implied)
+
+
+def _is_uuid(column: Any) -> bool:
+    try:
+        return column.type.python_type is uuid.UUID
+    except (AttributeError, NotImplementedError):
+        return False
+
+
+def id_text(column: Any) -> Any:
+    """*column* rendered as the canonical text :func:`canonical_id` stores.
+
+    UUIDs become 32 lowercase hex digits on every dialect (Postgres casts with
+    hyphens, SQLite stores without), everything else its plain text.
+    """
+    text = cast(column, String)
+    if _is_uuid(column):
+        return func.replace(func.lower(text), "-", "")
+    return text
+
+
+def canonical_id(value: Any) -> str:
+    """The stored ``resource_id`` for a key value (matches :func:`id_text`)."""
+    if isinstance(value, uuid.UUID):
+        return value.hex
+    return str(value)
+
+
+def row_id(obj: Any) -> str:
+    """The canonical id of a loaded single-key row."""
+    identity = inspect(obj).identity
+    if not identity or len(identity) != 1:
+        raise ValueError("shared resources need a single-column primary key")
+    return canonical_id(identity[0])
+
+
+def active_share_clause(now: datetime | None = None) -> Any:
+    """Shares that have not expired, as of *now* (a bound value, not ``NOW()``)."""
+    moment = now or datetime.now(UTC)
+    return or_(ResourceShare.expires_at.is_(None), ResourceShare.expires_at > moment)
+
+
+def share_exists_for(
+    resource_type: str, id_col: Any, *, roles: Mapping[str, tuple[str, ...]] | None = None
+) -> Callable[[ResourcePrincipal, str], Any | None]:
+    """A ``share_exists`` factory for ``OwnedResourcePolicy``.
+
+    Correlated on *id_col*; tenant scoping comes from the share table's
+    ``MultiTenantMixin`` filter, and expired shares never match. *roles* is the
+    type's ``ResourceType.share_roles`` (``None``: the default view/edit roles);
+    pass the same mapping to both so SQL access and grant validation agree.
+    """
+    key = id_text(id_col)
+
+    def share_exists(principal: ResourcePrincipal, action: str) -> Any | None:
+        granting_roles = roles_granting(action, roles=roles)
+        if not granting_roles or principal.user_id is None:
+            return None
+        return exists(
+            select(ResourceShare.id).where(
+                ResourceShare.resource_type == resource_type,
+                ResourceShare.resource_id == key,
+                ResourceShare.subject_type == SUBJECT_USER,
+                ResourceShare.subject_id == principal.user_id,
+                ResourceShare.role.in_(granting_roles),
+                active_share_clause(),
+            )
+        )
+
+    return share_exists
+
+
+__all__ = [
+    "active_share_clause",
+    "canonical_id",
+    "id_text",
+    "resolve_roles",
+    "roles_granting",
+    "row_id",
+    "share_exists_for",
+]

modules/sharing/sharing/service.py → modules/sharing/sharing/core/service.py

@@ -25,8 +25,9 @@
 from sqlalchemy.ext.asyncio import AsyncSession
 from sqlmodel import select
 
-from sharing.audience import Audience, OpenAudience
-from sharing.constants import (
+from sharing.contracts.schemas import ShareGrant
+from sharing.core.audience import Audience, OpenAudience
+from sharing.core.constants import (
     ERR_BAD_ROLE,
     ERR_EXPIRED,
     ERR_FORBIDDEN,
@@ -37,9 +38,8 @@
     PERM_MANAGE,
     SUBJECT_USER,
 )
-from sharing.contracts.schemas import ShareGrant
+from sharing.core.policy import resolve_roles, row_id
 from sharing.models import ResourceShare
-from sharing.policy import resolve_roles, row_id
 
 
 class SharingError(Exception):