diff --git a/CLAUDE.md b/CLAUDE.md index 69ac236..8b2f4e8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -80,9 +80,12 @@ default JWT secret. **Dependency injection** (`app/ioc.py`): one `modern_di.Container` built from `ALL_GROUPS = [Database, Repositories, UseCases]`, attached via `modern_di_litestar.ModernDIPlugin`. Route handlers receive use cases as -parameters; each `app/api/endpoints/*.py` module declares them with -`modern_di_litestar.FromDI(...)` (wired centrally in `build_app`'s -`dependencies=` dict) so Litestar resolves them per-request. Provider scopes: +parameters typed `NamedDependency[SomeUseCase]`; nothing wires them by hand. +`build_app` passes `autowired_groups=[ioc.UseCases]` to the plugin, which +registers one Litestar dependency per provider on that group, named after the +provider attribute (`create_chat_use_case`, ...). `Database` and `Repositories` +are deliberately **not** autowired — a route handler has no business resolving +a session, a transaction or a repository directly. Provider scopes: - `Database.database_engine` — app-scoped factory, `cache=` finalizer disposes the engine. - `Database.database_session` — request-scoped, finalizer closes the session. @@ -135,7 +138,11 @@ env vars (see `docker-compose.yml`). `api_bootstrapper_config` builds the `route_handlers=[auth_endpoints.ROUTER, chats_endpoints.ROUTER, messages_endpoints.ROUTER]`. Add a new resource by creating `app/api/endpoints/.py`, defining handlers + a `ROUTER`, and adding it - to that list plus `build_app`'s `dependencies=` dict for any new use case. + to that list. A new use case needs no wiring beyond its `ioc.UseCases` + provider: `autowired_groups` exposes it under the provider's own name. + Handlers that need the caller annotate the request `app.api.auth.AuthedRequest` + and pass `actor=request.user` explicitly; every use case `__call__` is + keyword-only. - Use cases live in `app/use_cases/`, one `@dataclasses.dataclass(kw_only=True, frozen=True, slots=True)` per operation with an async `__call__` decorated `@db_retry.postgres_retry`. Shared authorization logic that more than one diff --git a/app/api/app.py b/app/api/app.py index 878285f..b64dfd9 100644 --- a/app/api/app.py +++ b/app/api/app.py @@ -18,16 +18,6 @@ from app.api.endpoints import messages as messages_endpoints from app.exceptions import ConflictError, PermissionDeniedError, ValidationError from app.settings import settings -from app.use_cases.authenticate_user import AuthenticateUserUseCase -from app.use_cases.create_chat import CreateChatUseCase -from app.use_cases.create_message import CreateMessageUseCase -from app.use_cases.delete_message import DeleteMessageUseCase -from app.use_cases.edit_message import EditMessageUseCase -from app.use_cases.fetch_chat import FetchChatUseCase -from app.use_cases.fetch_chats import FetchChatsUseCase -from app.use_cases.fetch_messages import FetchMessagesUseCase -from app.use_cases.mark_read import MarkReadUseCase -from app.use_cases.register_user import RegisterUserUseCase def build_app() -> litestar.Litestar: @@ -45,19 +35,14 @@ def build_app() -> litestar.Litestar: ConflictError: exception_handlers.conflict_error_handler, }, route_handlers=[auth_endpoints.ROUTER, chats_endpoints.ROUTER, messages_endpoints.ROUTER], - plugins=[modern_di_litestar.ModernDIPlugin(di_container), JWTCookieAuthPlugin()], - dependencies={ - "register_user_use_case": modern_di_litestar.FromDI(RegisterUserUseCase), - "authenticate_user_use_case": modern_di_litestar.FromDI(AuthenticateUserUseCase), - "create_chat_use_case": modern_di_litestar.FromDI(CreateChatUseCase), - "fetch_chat_use_case": modern_di_litestar.FromDI(FetchChatUseCase), - "create_message_use_case": modern_di_litestar.FromDI(CreateMessageUseCase), - "fetch_messages_use_case": modern_di_litestar.FromDI(FetchMessagesUseCase), - "edit_message_use_case": modern_di_litestar.FromDI(EditMessageUseCase), - "delete_message_use_case": modern_di_litestar.FromDI(DeleteMessageUseCase), - "fetch_chats_use_case": modern_di_litestar.FromDI(FetchChatsUseCase), - "mark_read_use_case": modern_di_litestar.FromDI(MarkReadUseCase), - }, + # autowired_groups exposes one Litestar dependency per UseCases provider, named + # after the provider attribute - which is what every handler parameter is already + # called. Database and Repositories are deliberately left out: route handlers have + # no business resolving a session, a transaction or a repository directly. + plugins=[ + modern_di_litestar.ModernDIPlugin(di_container, autowired_groups=[ioc.UseCases]), + JWTCookieAuthPlugin(), + ], request_max_body_size=settings.request_max_body_size, ), opentelemetry_instrumentors=[ diff --git a/app/api/auth.py b/app/api/auth.py index 8292062..47a0a41 100644 --- a/app/api/auth.py +++ b/app/api/auth.py @@ -1,6 +1,7 @@ import datetime import typing +import litestar import modern_di_litestar from litestar.config.app import AppConfig from litestar.connection import ASGIConnection @@ -13,6 +14,11 @@ from app.settings import settings +# Every authenticated handler annotates its request with this; `request.user` is a UsersTable +# because retrieve_user_handler below is what populates it. +type AuthedRequest = litestar.Request[tables.UsersTable, Token, typing.Any] + + async def retrieve_user_handler(token: Token, connection: ASGIConnection) -> tables.UsersTable | None: # Auth middleware runs before request-scoped DI is available, so resolve the app-scoped # engine and open a short-lived session through the same factory the container uses. That diff --git a/app/api/endpoints/auth.py b/app/api/endpoints/auth.py index 0fa0a86..08541eb 100644 --- a/app/api/endpoints/auth.py +++ b/app/api/endpoints/auth.py @@ -6,8 +6,7 @@ from litestar.exceptions import NotAuthorizedException from litestar.response import Response -from app.api.auth import jwt_cookie_auth -from app.database import tables +from app.api.auth import AuthedRequest, jwt_cookie_auth from app.schemas import api as schemas from app.use_cases.authenticate_user import AuthenticateUserUseCase from app.use_cases.register_user import RegisterUserUseCase @@ -18,7 +17,7 @@ async def register( data: schemas.RegisterRequest, register_user_use_case: NamedDependency[RegisterUserUseCase], ) -> Response[schemas.User]: - user: typing.Final = await register_user_use_case(data) + user: typing.Final = await register_user_use_case(data=data) return jwt_cookie_auth.login( identifier=str(user.id), response_body=schemas.User.model_validate(user), @@ -31,7 +30,7 @@ async def login( data: schemas.LoginRequest, authenticate_user_use_case: NamedDependency[AuthenticateUserUseCase], ) -> Response[schemas.User]: - user: typing.Final = await authenticate_user_use_case(data.username, data.password) + user: typing.Final = await authenticate_user_use_case(username=data.username, password=data.password) if user is None: raise NotAuthorizedException(detail="Invalid username or password") return jwt_cookie_auth.login( @@ -49,7 +48,7 @@ async def logout() -> Response[None]: @litestar.get("/auth/me/") -async def me(request: litestar.Request[tables.UsersTable, typing.Any, typing.Any]) -> schemas.User: +async def me(request: AuthedRequest) -> schemas.User: return schemas.User.model_validate(request.user) diff --git a/app/api/endpoints/chats.py b/app/api/endpoints/chats.py index e1b9f25..e1a3fc1 100644 --- a/app/api/endpoints/chats.py +++ b/app/api/endpoints/chats.py @@ -6,7 +6,7 @@ from litestar.openapi.datastructures import ResponseSpec from litestar.params import FromPath -from app.database import tables +from app.api.auth import AuthedRequest from app.schemas import api as schemas from app.use_cases.create_chat import CreateChatUseCase from app.use_cases.fetch_chat import FetchChatUseCase @@ -24,10 +24,10 @@ ) async def create_chat( data: schemas.CreateChatRequest, - request: litestar.Request[tables.UsersTable, typing.Any, typing.Any], + request: AuthedRequest, create_chat_use_case: NamedDependency[CreateChatUseCase], ) -> litestar.Response[schemas.ChatDetail]: - chat, created = await create_chat_use_case(request.user, data) + chat, created = await create_chat_use_case(actor=request.user, data=data) return litestar.Response( content=schemas.ChatDetail.model_validate(chat), status_code=status_codes.HTTP_201_CREATED if created else status_codes.HTTP_200_OK, @@ -36,23 +36,19 @@ async def create_chat( @litestar.get("/chats/") async def list_chats( - request: litestar.Request[tables.UsersTable, typing.Any, typing.Any], + request: AuthedRequest, fetch_chats_use_case: NamedDependency[FetchChatsUseCase], ) -> schemas.Chats: - rows: typing.Final = await fetch_chats_use_case(request.user) - return schemas.Chats.from_models( - schemas.ChatListItem.from_row(row.chat, unread_count=row.unread_count, last_message=row.last_message) - for row in rows - ) + return schemas.Chats.from_models(await fetch_chats_use_case(actor=request.user)) @litestar.get("/chats/{chat_id:int}/") async def get_chat( chat_id: FromPath[int], - request: litestar.Request[tables.UsersTable, typing.Any, typing.Any], + request: AuthedRequest, fetch_chat_use_case: NamedDependency[FetchChatUseCase], ) -> schemas.ChatDetail: - chat: typing.Final = await fetch_chat_use_case(request.user, chat_id) + chat: typing.Final = await fetch_chat_use_case(actor=request.user, chat_id=chat_id) return schemas.ChatDetail.model_validate(chat) @@ -60,10 +56,10 @@ async def get_chat( async def mark_read( chat_id: FromPath[int], data: schemas.MarkReadRequest, - request: litestar.Request[tables.UsersTable, typing.Any, typing.Any], + request: AuthedRequest, mark_read_use_case: NamedDependency[MarkReadUseCase], ) -> schemas.ChatMember: - member: typing.Final = await mark_read_use_case(request.user, chat_id, data) + member: typing.Final = await mark_read_use_case(actor=request.user, chat_id=chat_id, data=data) return schemas.ChatMember.model_validate(member) diff --git a/app/api/endpoints/messages.py b/app/api/endpoints/messages.py index 5bfb443..79021fe 100644 --- a/app/api/endpoints/messages.py +++ b/app/api/endpoints/messages.py @@ -6,7 +6,7 @@ from litestar.openapi.datastructures import ResponseSpec from litestar.params import FromPath, FromQuery -from app.database import tables +from app.api.auth import AuthedRequest from app.schemas import api as schemas from app.use_cases.create_message import CreateMessageUseCase from app.use_cases.delete_message import DeleteMessageUseCase @@ -25,10 +25,10 @@ async def send_message( chat_id: FromPath[int], data: schemas.SendMessageRequest, - request: litestar.Request[tables.UsersTable, typing.Any, typing.Any], + request: AuthedRequest, create_message_use_case: NamedDependency[CreateMessageUseCase], ) -> litestar.Response[schemas.Message]: - message, created = await create_message_use_case(request.user, chat_id, data) + message, created = await create_message_use_case(actor=request.user, chat_id=chat_id, data=data) return litestar.Response( content=schemas.Message.model_validate(message), status_code=status_codes.HTTP_201_CREATED if created else status_codes.HTTP_200_OK, @@ -38,7 +38,7 @@ async def send_message( @litestar.get("/chats/{chat_id:int}/messages/") async def list_messages( # noqa: PLR0913 - each is a distinct Litestar-bound path/query/DI param chat_id: FromPath[int], - request: litestar.Request[tables.UsersTable, typing.Any, typing.Any], + request: AuthedRequest, fetch_messages_use_case: NamedDependency[FetchMessagesUseCase], *, before_id: FromQuery[int | None] = None, @@ -46,7 +46,7 @@ async def list_messages( # noqa: PLR0913 - each is a distinct Litestar-bound pa limit: FromQuery[int] = 50, ) -> schemas.Messages: messages: typing.Final = await fetch_messages_use_case( - request.user, chat_id, before_id=before_id, after_id=after_id, limit=limit + actor=request.user, chat_id=chat_id, before_id=before_id, after_id=after_id, limit=limit ) return schemas.Messages.from_models(messages) @@ -55,20 +55,20 @@ async def list_messages( # noqa: PLR0913 - each is a distinct Litestar-bound pa async def edit_message( message_id: FromPath[int], data: schemas.EditMessageRequest, - request: litestar.Request[tables.UsersTable, typing.Any, typing.Any], + request: AuthedRequest, edit_message_use_case: NamedDependency[EditMessageUseCase], ) -> schemas.Message: - message: typing.Final = await edit_message_use_case(request.user, message_id, data) + message: typing.Final = await edit_message_use_case(actor=request.user, message_id=message_id, data=data) return schemas.Message.model_validate(message) @litestar.delete("/messages/{message_id:int}/") async def delete_message( message_id: FromPath[int], - request: litestar.Request[tables.UsersTable, typing.Any, typing.Any], + request: AuthedRequest, delete_message_use_case: NamedDependency[DeleteMessageUseCase], ) -> None: - await delete_message_use_case(request.user, message_id) + await delete_message_use_case(actor=request.user, message_id=message_id) ROUTER: typing.Final = litestar.Router( diff --git a/app/database/tables.py b/app/database/tables.py index c5727bf..22983ca 100644 --- a/app/database/tables.py +++ b/app/database/tables.py @@ -57,6 +57,30 @@ class ChatsTable(BigIntAuditBase): "ChatMembersTable", lazy="noload", uselist=True, viewonly=True ) + # Per-viewer state that the chat listing needs alongside the chat's own columns. Both are + # mapped here rather than assembled in Python so that a listed chat is a plain ChatsTable + # whose attribute names already match the response schema - no per-row DTO, no aliases. + # + # unread_count is only populated when the query asks for it via with_expression(); every + # other query gets default_expr's literal 0, so the attribute is never None. + unread_count: orm.Mapped[int] = orm.query_expression(default_expr=sa.literal(0)) + # last_message_id deliberately carries no ForeignKey (a chats -> messages FK would close a + # cycle with messages.chat_id), so the join column has to be annotated foreign() by hand. + # The soft-delete guard lives in the join rather than at the call site: a chat must never + # preview a deleted message, whatever loads it. DeleteMessageUseCase still repoints + # last_message_id in the same commit as the delete - this only keeps the mapping correct on + # its own if some other write path ever fails to. + last_message: orm.Mapped[MessagesTable | None] = orm.relationship( + "MessagesTable", + primaryjoin=lambda: sa.and_( + orm.foreign(ChatsTable.last_message_id) == MessagesTable.id, + MessagesTable.deleted_at.is_(None), + ), + lazy="noload", + uselist=False, + viewonly=True, + ) + class ChatMembersTable(BigIntBase): __tablename__ = "chat_members" diff --git a/app/repositories/chats_repository.py b/app/repositories/chats_repository.py index e197c60..fbb3d74 100644 --- a/app/repositories/chats_repository.py +++ b/app/repositories/chats_repository.py @@ -27,7 +27,7 @@ async def fetch_direct_by_key(self, direct_key: str) -> tables.ChatsTable | None load=[orm.selectinload(tables.ChatsTable.members)], ) - async def list_for_user(self, user_id: int) -> Sequence[sa.Row[tuple[tables.ChatsTable, int]]]: + async def list_for_user(self, user_id: int) -> Sequence[tables.ChatsTable]: unread_count: typing.Final = ( sa.select(sa.func.count(tables.MessagesTable.id)) .where( @@ -41,10 +41,19 @@ async def list_for_user(self, user_id: int) -> Sequence[sa.Row[tuple[tables.Chat .label("unread_count") ) statement: typing.Final = ( - sa.select(tables.ChatsTable, unread_count) + sa.select(tables.ChatsTable) + .options( + orm.with_expression(tables.ChatsTable.unread_count, unread_count), + orm.selectinload(tables.ChatsTable.last_message), + ) .join(tables.ChatMembersTable, tables.ChatMembersTable.chat_id == tables.ChatsTable.id) .where(tables.ChatMembersTable.user_id == user_id) .order_by(sa.func.coalesce(tables.ChatsTable.last_message_id, 0).desc()) + # Sessions run expire_on_commit=False, so a ChatsTable already in the identity map + # keeps whatever unread_count/last_message it was loaded with; without this, a second + # listing in the same session would hand back the first one's values. Safe here only + # because this query is read-only - populate_existing overwrites in-memory state. + .execution_options(populate_existing=True) ) result: typing.Final = await self.repository.session.execute(statement) - return result.all() + return result.scalars().all() diff --git a/app/schemas/api.py b/app/schemas/api.py index bb1f702..28b8279 100644 --- a/app/schemas/api.py +++ b/app/schemas/api.py @@ -87,26 +87,11 @@ class Messages(Collection[Message]): class ChatListItem(Chat): + # ChatsTable maps unread_count and last_message itself (see app/database/tables.py), so a + # listed chat validates straight through from_attributes like any other ORM instance. last_message: Message | None = None unread_count: int = 0 - @classmethod - def from_row(cls, chat: tables.ChatsTable, *, unread_count: int, last_message: tables.MessagesTable | None) -> Self: - # `chat` alone (via Chat's from_attributes=True) has no unread_count/last_message - # attributes - those are computed by FetchChatsUseCase, not columns on ChatsTable - so - # this validates them together from a dict instead of Chat.model_validate(chat) plus an - # unvalidated model_copy(update=...) patch. - return cls.model_validate( - { - "id": chat.id, - "chat_type": chat.chat_type, - "title": chat.title, - "created_by_id": chat.created_by_id, - "unread_count": unread_count, - "last_message": last_message, - } - ) - class Chats(Collection[ChatListItem]): pass diff --git a/app/use_cases/authenticate_user.py b/app/use_cases/authenticate_user.py index 3cc26f0..7160920 100644 --- a/app/use_cases/authenticate_user.py +++ b/app/use_cases/authenticate_user.py @@ -13,7 +13,7 @@ class AuthenticateUserUseCase: users_repository: UsersRepository @postgres_retry - async def __call__(self, username: str, password: str) -> tables.UsersTable | None: + async def __call__(self, *, username: str, password: str) -> tables.UsersTable | None: user: typing.Final = await self.users_repository.get_one_or_none(username=username) if user is None: # Hash anyway: skipping the argon2 work on an unknown username makes the diff --git a/app/use_cases/create_chat.py b/app/use_cases/create_chat.py index 1628d5f..3b5633e 100644 --- a/app/use_cases/create_chat.py +++ b/app/use_cases/create_chat.py @@ -21,7 +21,7 @@ class CreateChatUseCase: chat_members_repository: ChatMembersRepository @postgres_retry - async def __call__(self, actor: tables.UsersTable, data: CreateChatRequest) -> tuple[tables.ChatsTable, bool]: + async def __call__(self, *, actor: tables.UsersTable, data: CreateChatRequest) -> tuple[tables.ChatsTable, bool]: member_ids: typing.Final = {actor.id, *data.member_ids} direct_key: str | None = None diff --git a/app/use_cases/create_message.py b/app/use_cases/create_message.py index 9fbb00b..53a21a0 100644 --- a/app/use_cases/create_message.py +++ b/app/use_cases/create_message.py @@ -21,7 +21,7 @@ class CreateMessageUseCase: @postgres_retry async def __call__( - self, actor: tables.UsersTable, chat_id: int, data: SendMessageRequest + self, *, actor: tables.UsersTable, chat_id: int, data: SendMessageRequest ) -> tuple[tables.MessagesTable, bool]: if not await self.chat_members_repository.is_member(chat_id, actor.id): msg = "Not a member of this chat" diff --git a/app/use_cases/delete_message.py b/app/use_cases/delete_message.py index c9dc75b..977a565 100644 --- a/app/use_cases/delete_message.py +++ b/app/use_cases/delete_message.py @@ -18,7 +18,7 @@ class DeleteMessageUseCase: chats_repository: ChatsRepository @postgres_retry - async def __call__(self, actor: tables.UsersTable, message_id: int) -> None: + async def __call__(self, *, actor: tables.UsersTable, message_id: int) -> None: async with self.transaction: message = await fetch_message_for_author( messages_repository=self.messages_repository, diff --git a/app/use_cases/edit_message.py b/app/use_cases/edit_message.py index 4949230..7721601 100644 --- a/app/use_cases/edit_message.py +++ b/app/use_cases/edit_message.py @@ -19,7 +19,7 @@ class EditMessageUseCase: @postgres_retry async def __call__( - self, actor: tables.UsersTable, message_id: int, data: EditMessageRequest + self, *, actor: tables.UsersTable, message_id: int, data: EditMessageRequest ) -> tables.MessagesTable: async with self.transaction: message = await fetch_message_for_author( diff --git a/app/use_cases/fetch_chat.py b/app/use_cases/fetch_chat.py index 84ebfbb..db5c170 100644 --- a/app/use_cases/fetch_chat.py +++ b/app/use_cases/fetch_chat.py @@ -14,7 +14,7 @@ class FetchChatUseCase: chat_members_repository: ChatMembersRepository @postgres_retry - async def __call__(self, actor: tables.UsersTable, chat_id: int) -> tables.ChatsTable: + async def __call__(self, *, actor: tables.UsersTable, chat_id: int) -> tables.ChatsTable: if not await self.chat_members_repository.is_member(chat_id, actor.id): msg = "Not a member of this chat" raise PermissionDeniedError(msg) diff --git a/app/use_cases/fetch_chats.py b/app/use_cases/fetch_chats.py index b2b191f..03f606c 100644 --- a/app/use_cases/fetch_chats.py +++ b/app/use_cases/fetch_chats.py @@ -1,48 +1,19 @@ import dataclasses -import typing +from collections.abc import Sequence from db_retry import postgres_retry from app.database import tables from app.repositories.chats_repository import ChatsRepository -from app.repositories.messages_repository import MessagesRepository - - -@dataclasses.dataclass(frozen=True, slots=True) -class ChatListRow: - chat: tables.ChatsTable - unread_count: int - last_message: tables.MessagesTable | None @dataclasses.dataclass(kw_only=True, frozen=True, slots=True) class FetchChatsUseCase: chats_repository: ChatsRepository - messages_repository: MessagesRepository @postgres_retry - async def __call__(self, actor: tables.UsersTable) -> list[ChatListRow]: - rows: typing.Final = await self.chats_repository.list_for_user(actor.id) - - # One bounded lookup for every chat's last message, not one query per row: collect the - # non-null last_message_id values and load them with a single WHERE id IN (...). - last_message_ids: typing.Final = {row[0].last_message_id for row in rows if row[0].last_message_id is not None} - last_messages: dict[int, tables.MessagesTable] = {} - if last_message_ids: - # deleted_at.is_(None) is a self-defending guard, not the source of truth: DeleteMessageUseCase - # repoints chats.last_message_id off a deleted message in the same commit as the soft delete, - # so this filter should never actually exclude anything - it just keeps this query correct on - # its own if another write path ever sets the column without doing that. - messages = await self.messages_repository.get_many( - tables.MessagesTable.id.in_(last_message_ids), tables.MessagesTable.deleted_at.is_(None) - ) - last_messages = {message.id: message for message in messages} - - return [ - ChatListRow( - chat=row[0], - unread_count=row.unread_count, - last_message=last_messages.get(row[0].last_message_id) if row[0].last_message_id is not None else None, - ) - for row in rows - ] + async def __call__(self, *, actor: tables.UsersTable) -> Sequence[tables.ChatsTable]: + # Each returned chat already carries its unread_count and last_message: the count comes + # from a correlated subquery on the same statement and the preview from one selectinload, + # so this is two round trips for the whole list regardless of how many chats it holds. + return await self.chats_repository.list_for_user(actor.id) diff --git a/app/use_cases/fetch_messages.py b/app/use_cases/fetch_messages.py index 0581901..e5dfedc 100644 --- a/app/use_cases/fetch_messages.py +++ b/app/use_cases/fetch_messages.py @@ -21,9 +21,9 @@ class FetchMessagesUseCase: @postgres_retry async def __call__( self, + *, actor: tables.UsersTable, chat_id: int, - *, before_id: int | None = None, after_id: int | None = None, limit: int = 50, diff --git a/app/use_cases/mark_read.py b/app/use_cases/mark_read.py index cb1f4ff..7188e23 100644 --- a/app/use_cases/mark_read.py +++ b/app/use_cases/mark_read.py @@ -17,7 +17,9 @@ class MarkReadUseCase: messages_repository: MessagesRepository @postgres_retry - async def __call__(self, actor: tables.UsersTable, chat_id: int, data: MarkReadRequest) -> tables.ChatMembersTable: + async def __call__( + self, *, actor: tables.UsersTable, chat_id: int, data: MarkReadRequest + ) -> tables.ChatMembersTable: member: typing.Final = await self.chat_members_repository.fetch_member(chat_id, actor.id) if member is None: msg = "Not a member of this chat" diff --git a/app/use_cases/register_user.py b/app/use_cases/register_user.py index e87584a..4684b47 100644 --- a/app/use_cases/register_user.py +++ b/app/use_cases/register_user.py @@ -15,7 +15,7 @@ class RegisterUserUseCase: users_repository: UsersRepository @postgres_retry - async def __call__(self, data: RegisterRequest) -> tables.UsersTable: + async def __call__(self, *, data: RegisterRequest) -> tables.UsersTable: async with self.transaction: user: typing.Final = await self.users_repository.create( tables.UsersTable( diff --git a/architecture/chats.md b/architecture/chats.md index de55d0b..55529da 100644 --- a/architecture/chats.md +++ b/architecture/chats.md @@ -60,7 +60,10 @@ existing message id from a nonexistent one via `404` vs `403` on ## Listing and unread counts `GET /api/chats/` → `FetchChatsUseCase` (`app/use_cases/fetch_chats.py`), backed -by `ChatsRepository.list_for_user`. Unread count is a correlated scalar +by `ChatsRepository.list_for_user`, which returns plain `ChatsTable` instances +carrying two extra attributes mapped for exactly this query — so the endpoint +validates them straight through `schemas.Chats.from_models(...)` with no +per-row DTO in between. Unread count is a correlated scalar subquery per row, not a Python loop: `count(messages WHERE chat_id = ? AND id > COALESCE(member.last_read_message_id, 0) AND user_id IS DISTINCT FROM member.user_id AND deleted_at IS NULL)`, joined against `chat_members` and @@ -70,13 +73,26 @@ DISTINCT FROM` rather than `!=` matters because system messages carry `user_id IS NULL`, and `NULL != me` evaluates to `NULL` in SQL, which would silently drop every system message from the count. -The use case then loads every listed chat's `last_message` in one bounded -`WHERE id IN (...)` query (`FetchChatsUseCase.__call__`), not one query per -row — the `deleted_at.is_(None)` filter on that query is a self-defending -guard, not the source of truth, since `DeleteMessageUseCase` already repoints +The count reaches the ORM instance through `ChatsTable.unread_count`, an +`orm.query_expression()` that `list_for_user` fills with `with_expression(...)`; +any other query that loads a chat gets its `default_expr` literal `0`, so the +attribute is never `None`. The preview comes from `ChatsTable.last_message`, a +`viewonly` many-to-one on `last_message_id` loaded by one `selectinload` — one +extra round trip for the whole page, not one per row. `last_message_id` carries +no `ForeignKey` (that would close a cycle with `messages.chat_id`), so the +relationship annotates the join column `orm.foreign()` by hand and folds +`deleted_at IS NULL` into its `primaryjoin`: a self-defending guard, not the +source of truth, since `DeleteMessageUseCase` already repoints `last_message_id` off a deleted message in the same commit as the delete (see `messages.md`). +`list_for_user` runs with `populate_existing=True`. Sessions are built +`expire_on_commit=False`, so a `ChatsTable` already in the identity map would +otherwise keep the `unread_count` and `last_message` it was first loaded with, +and a second listing in the same session would hand back the first one's +numbers. That is safe only because this query is read-only — `populate_existing` +overwrites in-memory state on the entities it returns. + ## Marking read `POST /api/chats/{id}/read/` → `MarkReadUseCase` (`app/use_cases/mark_read.py`). diff --git a/planning/changes/2026-08-21.02-di-wiring-and-chat-listing.md b/planning/changes/2026-08-21.02-di-wiring-and-chat-listing.md new file mode 100644 index 0000000..f610865 --- /dev/null +++ b/planning/changes/2026-08-21.02-di-wiring-and-chat-listing.md @@ -0,0 +1,101 @@ +--- +summary: Removed the hand-written DI dependency dict, the per-row chat-listing DTO and its schema translation, and made every use case `__call__` keyword-only — 108 tests green at 100% coverage with no API or schema change. +--- + +# Design: Autowired DI, mapped chat-listing state, keyword-only use cases + +## Summary + +Four wiring simplifications with no change to the HTTP contract. `ModernDIPlugin` +autowires `ioc.UseCases`, replacing the twelve-entry `dependencies=` dict in +`build_app`. `ChatsTable` maps `unread_count` and `last_message` itself, so the +chat listing no longer assembles a `ChatListRow` per chat and no longer +translates it back into a schema field-by-field. Every use case `__call__` takes +keyword-only arguments. Authenticated handlers annotate the request +`AuthedRequest` instead of restating `litestar.Request[UsersTable, Any, Any]`. + +## Motivation + +Each of the four is small on its own; together they remove the places where +adding a feature meant editing a list that exists only to repeat something the +code already knows. + +- Every new use case had to be named three times: as a provider in + `ioc.UseCases`, as an import in `app/api/app.py`, and as a `dependencies=` + entry — under the name it already had as a provider attribute. +- `GET /api/chats/` ran the chat's four fields through two hand-written + mappings: `FetchChatsUseCase` packed a `ChatsTable` into `ChatListRow`, and + `ChatListItem.from_row` unpacked it again into a dict for validation. Neither + mapping renamed anything — `ChatsTable.id` was already `ChatListItem.id`. +- `create_chat_use_case(request.user, data)` gave no clue at the call site which + argument was the actor, on operations whose authorization turns on it. + +## Design + +**Autowiring.** `ModernDIPlugin(di_container, autowired_groups=[ioc.UseCases])` +registers one Litestar dependency per provider on the group, named after the +provider attribute — already the handler parameter names. `Database` and +`Repositories` stay out: a route handler resolving a session, a transaction or a +repository directly is a boundary violation, and autowiring them would also let +a handler parameter that happens to be called `transaction` become a silent DI +injection instead of a query parameter. + +**Chat listing.** `ChatsTable` gains two attributes that only the listing query +populates: + +```python +unread_count: orm.Mapped[int] = orm.query_expression(default_expr=sa.literal(0)) +last_message: orm.Mapped[MessagesTable | None] = orm.relationship(...) +``` + +`list_for_user` fills the first with `with_expression(...)` and loads the second +with one `selectinload`, so it returns `Sequence[ChatsTable]` rather than +`Sequence[Row[tuple[ChatsTable, int]]]`. `FetchChatsUseCase` becomes a single +repository call — the hand-rolled `WHERE id IN (...)` batch for previews is now +the `selectinload`, same one extra round trip. `ChatListItem` keeps inheriting +`Chat` and loses `from_row`; the endpoint is one `Chats.from_models(...)`. +Response JSON is unchanged. + +Two mapping details carry their own reasons, both documented in +`architecture/chats.md`: `last_message_id` has no `ForeignKey` (it would close a +cycle with `messages.chat_id`), so the relationship annotates the join column +`orm.foreign()` by hand and folds the soft-delete guard into its `primaryjoin`; +and `list_for_user` runs `populate_existing=True` because sessions are +`expire_on_commit=False`, so an identity-mapped chat would otherwise keep the +`unread_count` it was first loaded with. + +**Keyword-only.** All ten `__call__`s, e.g. +`create_chat_use_case(actor=request.user, data=data)`. `db_retry.postgres_retry` +forwards `**kwargs`, so the decorator is unaffected. + +## Non-goals + +- Injecting the actor through `modern-di`'s `ContextProvider`. It is operation + input, not a collaborator, and binding it to the request container would force + every use-case test that drives one instance as several actors to build a + container per actor. +- Nesting the listing response as `{"chat": {...}, "unread_count": ...}`. It + would spare four lines of Python at the cost of `item.chat.title` in every + client. + +## Testing + +`just test` — 108 passed, 100% coverage. `just lint` — clean. +`tests/use_cases/test_unread_counts.py` was rewritten to the flat shape first +and failed on `AttributeError: 'ChatListRow' object has no attribute 'id'` +before the mapping landed. `tests/api/test_chat_listing_api.py` needed no +change, which is the evidence that the response contract held. +`alembic check` reports the same pre-existing `ck_chats_chattype` diff as +`main` and nothing else: the two new attributes emit no DDL. + +## Risk + +- **Stale listing values within one session.** `expire_on_commit=False` plus an + ORM-mapped `unread_count` is exactly the combination that returns a previous + load's numbers. Mitigated by `populate_existing=True` on `list_for_user`, and + covered by the tests that list twice around a mutation + (`test_deleting_the_newest_message_updates_preview_and_ordering`, + `test_marking_read_is_monotonic`). +- **`last_message` lazy-loading in async context.** `lazy="noload"` means a chat + loaded by any other query has `last_message` as `None` rather than emitting IO; + a caller that wants it must ask for the loader option. diff --git a/tests/use_cases/conftest.py b/tests/use_cases/conftest.py index ecb8648..7cdb048 100644 --- a/tests/use_cases/conftest.py +++ b/tests/use_cases/conftest.py @@ -60,7 +60,7 @@ async def direct_chat( create_chat_use_case: CreateChatUseCase, alice: tables.UsersTable, bob: tables.UsersTable ) -> tables.ChatsTable: chat, _ = await create_chat_use_case( - alice, schemas.CreateChatRequest(chat_type=tables.ChatType.DIRECT, member_ids=[bob.id]) + actor=alice, data=schemas.CreateChatRequest(chat_type=tables.ChatType.DIRECT, member_ids=[bob.id]) ) return chat @@ -70,7 +70,7 @@ async def alice_message( create_message_use_case: CreateMessageUseCase, direct_chat: tables.ChatsTable, alice: tables.UsersTable ) -> tables.MessagesTable: message, _ = await create_message_use_case( - alice, direct_chat.id, schemas.SendMessageRequest(idempotency_key=uuid.uuid4(), text="hello") + actor=alice, chat_id=direct_chat.id, data=schemas.SendMessageRequest(idempotency_key=uuid.uuid4(), text="hello") ) return message @@ -83,7 +83,7 @@ def send( async def _send(actor: tables.UsersTable, chat_id: int, text: str) -> tuple[tables.MessagesTable, bool]: return await create_message_use_case( - actor, chat_id, schemas.SendMessageRequest(idempotency_key=uuid.uuid4(), text=text) + actor=actor, chat_id=chat_id, data=schemas.SendMessageRequest(idempotency_key=uuid.uuid4(), text=text) ) return _send diff --git a/tests/use_cases/test_create_chat.py b/tests/use_cases/test_create_chat.py index c549ff5..27d9b76 100644 --- a/tests/use_cases/test_create_chat.py +++ b/tests/use_cases/test_create_chat.py @@ -62,7 +62,7 @@ async def test_direct_chat_is_created_with_both_members( create_chat_use_case: CreateChatUseCase, alice: tables.UsersTable, bob: tables.UsersTable ) -> None: chat, created = await create_chat_use_case( - alice, schemas.CreateChatRequest(chat_type=tables.ChatType.DIRECT, member_ids=[bob.id]) + actor=alice, data=schemas.CreateChatRequest(chat_type=tables.ChatType.DIRECT, member_ids=[bob.id]) ) assert created is True assert chat.chat_type is tables.ChatType.DIRECT @@ -74,10 +74,10 @@ async def test_direct_chat_is_idempotent_for_the_same_pair( create_chat_use_case: CreateChatUseCase, alice: tables.UsersTable, bob: tables.UsersTable ) -> None: first, first_created = await create_chat_use_case( - alice, schemas.CreateChatRequest(chat_type=tables.ChatType.DIRECT, member_ids=[bob.id]) + actor=alice, data=schemas.CreateChatRequest(chat_type=tables.ChatType.DIRECT, member_ids=[bob.id]) ) second, second_created = await create_chat_use_case( - bob, schemas.CreateChatRequest(chat_type=tables.ChatType.DIRECT, member_ids=[alice.id]) + actor=bob, data=schemas.CreateChatRequest(chat_type=tables.ChatType.DIRECT, member_ids=[alice.id]) ) assert first.id == second.id assert first_created is True @@ -92,7 +92,7 @@ async def test_direct_chat_creation_recovers_from_a_concurrent_duplicate_key( ) -> None: # A real winner: create the direct chat normally first, so a genuinely committed row exists. winner, winner_created = await create_chat_use_case( - alice, schemas.CreateChatRequest(chat_type=tables.ChatType.DIRECT, member_ids=[bob.id]) + actor=alice, data=schemas.CreateChatRequest(chat_type=tables.ChatType.DIRECT, member_ids=[bob.id]) ) assert winner_created is True # Captured now, not read off `winner` after the racer runs: the racer shares this session, @@ -111,7 +111,7 @@ async def test_direct_chat_creation_recovers_from_a_concurrent_duplicate_key( chat_members_repository=create_chat_use_case.chat_members_repository, ) loser, loser_created = await racer( - bob, schemas.CreateChatRequest(chat_type=tables.ChatType.DIRECT, member_ids=[alice.id]) + actor=bob, data=schemas.CreateChatRequest(chat_type=tables.ChatType.DIRECT, member_ids=[alice.id]) ) assert loser_created is False assert loser.id == winner_id @@ -128,7 +128,7 @@ async def test_direct_chat_recovery_raises_if_the_winners_row_is_unreadable( chat_members_repository=create_chat_use_case.chat_members_repository, ) with pytest.raises(RuntimeError, match="could not be found"): - await broken(alice, schemas.CreateChatRequest(chat_type=tables.ChatType.DIRECT, member_ids=[bob.id])) + await broken(actor=alice, data=schemas.CreateChatRequest(chat_type=tables.ChatType.DIRECT, member_ids=[bob.id])) async def test_group_chat_reraises_an_unexpected_duplicate_key( @@ -145,7 +145,7 @@ async def test_group_chat_reraises_an_unexpected_duplicate_key( chat_members_repository=create_chat_use_case.chat_members_repository, ) with pytest.raises(DuplicateKeyError): - await broken(alice, schemas.CreateChatRequest(chat_type=tables.ChatType.GROUP, member_ids=[bob.id])) + await broken(actor=alice, data=schemas.CreateChatRequest(chat_type=tables.ChatType.GROUP, member_ids=[bob.id])) async def test_direct_chat_rejects_more_than_two_members( @@ -156,8 +156,7 @@ async def test_direct_chat_rejects_more_than_two_members( ) -> None: with pytest.raises(ValidationError): await create_chat_use_case( - alice, - schemas.CreateChatRequest(chat_type=tables.ChatType.DIRECT, member_ids=[bob.id, carol.id]), + actor=alice, data=schemas.CreateChatRequest(chat_type=tables.ChatType.DIRECT, member_ids=[bob.id, carol.id]) ) @@ -168,8 +167,8 @@ async def test_group_chat_includes_the_creator( carol: tables.UsersTable, ) -> None: chat, created = await create_chat_use_case( - alice, - schemas.CreateChatRequest(chat_type=tables.ChatType.GROUP, member_ids=[bob.id, carol.id], title="Team"), + actor=alice, + data=schemas.CreateChatRequest(chat_type=tables.ChatType.GROUP, member_ids=[bob.id, carol.id], title="Team"), ) assert created is True assert chat.direct_key is None diff --git a/tests/use_cases/test_create_message.py b/tests/use_cases/test_create_message.py index 088b115..530473a 100644 --- a/tests/use_cases/test_create_message.py +++ b/tests/use_cases/test_create_message.py @@ -59,7 +59,7 @@ async def test_send_returns_created_true_on_first_call( create_message_use_case: CreateMessageUseCase, direct_chat: tables.ChatsTable, alice: tables.UsersTable ) -> None: message, created = await create_message_use_case( - alice, direct_chat.id, schemas.SendMessageRequest(idempotency_key=uuid.uuid4(), text="hi") + actor=alice, chat_id=direct_chat.id, data=schemas.SendMessageRequest(idempotency_key=uuid.uuid4(), text="hi") ) assert created is True assert message.text == "hi" @@ -70,10 +70,10 @@ async def test_repeated_idempotency_key_returns_the_same_message( ) -> None: key = uuid.uuid4() first, first_created = await create_message_use_case( - alice, direct_chat.id, schemas.SendMessageRequest(idempotency_key=key, text="hi") + actor=alice, chat_id=direct_chat.id, data=schemas.SendMessageRequest(idempotency_key=key, text="hi") ) second, second_created = await create_message_use_case( - alice, direct_chat.id, schemas.SendMessageRequest(idempotency_key=key, text="hi again") + actor=alice, chat_id=direct_chat.id, data=schemas.SendMessageRequest(idempotency_key=key, text="hi again") ) assert first_created is True assert second_created is False @@ -88,7 +88,7 @@ async def test_send_updates_chat_last_message_id( alice: tables.UsersTable, ) -> None: message, _ = await create_message_use_case( - alice, direct_chat.id, schemas.SendMessageRequest(idempotency_key=uuid.uuid4(), text="hi") + actor=alice, chat_id=direct_chat.id, data=schemas.SendMessageRequest(idempotency_key=uuid.uuid4(), text="hi") ) chat = await chats_repository.get_one(id=direct_chat.id) assert chat.last_message_id == message.id @@ -99,7 +99,9 @@ async def test_non_member_cannot_send( ) -> None: with pytest.raises(PermissionDeniedError): await create_message_use_case( - carol, direct_chat.id, schemas.SendMessageRequest(idempotency_key=uuid.uuid4(), text="hi") + actor=carol, + chat_id=direct_chat.id, + data=schemas.SendMessageRequest(idempotency_key=uuid.uuid4(), text="hi"), ) @@ -108,7 +110,7 @@ async def test_concurrent_duplicate_key_recovers_the_winners_message( ) -> None: key = uuid.uuid4() winner, winner_created = await create_message_use_case( - alice, direct_chat.id, schemas.SendMessageRequest(idempotency_key=key, text="hi") + actor=alice, chat_id=direct_chat.id, data=schemas.SendMessageRequest(idempotency_key=key, text="hi") ) assert winner_created is True winner_id = winner.id @@ -124,7 +126,7 @@ async def test_concurrent_duplicate_key_recovers_the_winners_message( ), ) loser, loser_created = await racer( - alice, direct_chat.id, schemas.SendMessageRequest(idempotency_key=key, text="hi again") + actor=alice, chat_id=direct_chat.id, data=schemas.SendMessageRequest(idempotency_key=key, text="hi again") ) assert loser_created is False assert loser.id == winner_id @@ -143,7 +145,11 @@ async def test_send_recovery_raises_if_the_winners_row_is_unreadable( ), ) with pytest.raises(RuntimeError, match="could not be found"): - await broken(alice, direct_chat.id, schemas.SendMessageRequest(idempotency_key=uuid.uuid4(), text="hi")) + await broken( + actor=alice, + chat_id=direct_chat.id, + data=schemas.SendMessageRequest(idempotency_key=uuid.uuid4(), text="hi"), + ) async def test_same_idempotency_key_in_two_different_chats_creates_two_messages( @@ -157,15 +163,15 @@ async def test_same_idempotency_key_in_two_different_chats_creates_two_messages( # "send to this chat", not a retry across the whole table, so reusing it in a different # chat is a second, independent send. other_chat, _ = await create_chat_use_case( - alice, schemas.CreateChatRequest(chat_type=tables.ChatType.DIRECT, member_ids=[carol.id]) + actor=alice, data=schemas.CreateChatRequest(chat_type=tables.ChatType.DIRECT, member_ids=[carol.id]) ) key = uuid.uuid4() first, first_created = await create_message_use_case( - alice, direct_chat.id, schemas.SendMessageRequest(idempotency_key=key, text="hi") + actor=alice, chat_id=direct_chat.id, data=schemas.SendMessageRequest(idempotency_key=key, text="hi") ) second, second_created = await create_message_use_case( - alice, other_chat.id, schemas.SendMessageRequest(idempotency_key=key, text="hi") + actor=alice, chat_id=other_chat.id, data=schemas.SendMessageRequest(idempotency_key=key, text="hi") ) assert first_created is True diff --git a/tests/use_cases/test_edit_message.py b/tests/use_cases/test_edit_message.py index b985972..a483634 100644 --- a/tests/use_cases/test_edit_message.py +++ b/tests/use_cases/test_edit_message.py @@ -16,7 +16,9 @@ async def test_author_can_edit( edit_message_use_case: EditMessageUseCase, alice_message: tables.MessagesTable, alice: tables.UsersTable ) -> None: - edited = await edit_message_use_case(alice, alice_message.id, schemas.EditMessageRequest(text="fixed")) + edited = await edit_message_use_case( + actor=alice, message_id=alice_message.id, data=schemas.EditMessageRequest(text="fixed") + ) assert edited.text == "fixed" assert edited.edited_at is not None @@ -26,7 +28,9 @@ async def test_other_member_cannot_edit( ) -> None: # bob is a member of the chat, not the author - membership alone must not authorize the edit. with pytest.raises(PermissionDeniedError): - await edit_message_use_case(bob, alice_message.id, schemas.EditMessageRequest(text="nope")) + await edit_message_use_case( + actor=bob, message_id=alice_message.id, data=schemas.EditMessageRequest(text="nope") + ) async def _remove_alice_from_chat( @@ -47,7 +51,9 @@ async def test_author_without_membership_cannot_edit( # the membership check does anything the authorship check doesn't already cover. await _remove_alice_from_chat(chat_members_repository, alice_message, alice) with pytest.raises(PermissionDeniedError): - await edit_message_use_case(alice, alice_message.id, schemas.EditMessageRequest(text="nope")) + await edit_message_use_case( + actor=alice, message_id=alice_message.id, data=schemas.EditMessageRequest(text="nope") + ) async def test_non_member_cannot_edit( @@ -56,7 +62,9 @@ async def test_non_member_cannot_edit( # carol isn't in direct_chat at all - the membership gate must refuse her before authorship # is even considered. with pytest.raises(PermissionDeniedError): - await edit_message_use_case(carol, alice_message.id, schemas.EditMessageRequest(text="nope")) + await edit_message_use_case( + actor=carol, message_id=alice_message.id, data=schemas.EditMessageRequest(text="nope") + ) async def test_editing_a_deleted_message_raises_conflict( @@ -67,9 +75,11 @@ async def test_editing_a_deleted_message_raises_conflict( ) -> None: # The author is authorized; the request conflicts with the message's current state, so this # is a 409-shaped ConflictError, not a 403-shaped PermissionDeniedError. - await delete_message_use_case(alice, alice_message.id) + await delete_message_use_case(actor=alice, message_id=alice_message.id) with pytest.raises(ConflictError): - await edit_message_use_case(alice, alice_message.id, schemas.EditMessageRequest(text="nope")) + await edit_message_use_case( + actor=alice, message_id=alice_message.id, data=schemas.EditMessageRequest(text="nope") + ) async def test_author_can_delete( @@ -78,7 +88,7 @@ async def test_author_can_delete( alice_message: tables.MessagesTable, alice: tables.UsersTable, ) -> None: - await delete_message_use_case(alice, alice_message.id) + await delete_message_use_case(actor=alice, message_id=alice_message.id) stored = await messages_repository.get_one(id=alice_message.id) assert stored.deleted_at is not None @@ -88,7 +98,7 @@ async def test_other_member_cannot_delete( ) -> None: # Same distinction as edit: bob is a member of the chat but not the author. with pytest.raises(PermissionDeniedError): - await delete_message_use_case(bob, alice_message.id) + await delete_message_use_case(actor=bob, message_id=alice_message.id) async def test_author_without_membership_cannot_delete( @@ -100,7 +110,7 @@ async def test_author_without_membership_cannot_delete( # Same distinction as edit. await _remove_alice_from_chat(chat_members_repository, alice_message, alice) with pytest.raises(PermissionDeniedError): - await delete_message_use_case(alice, alice_message.id) + await delete_message_use_case(actor=alice, message_id=alice_message.id) async def test_non_member_cannot_delete( @@ -108,7 +118,7 @@ async def test_non_member_cannot_delete( ) -> None: # Same distinction as edit: carol isn't in direct_chat at all. with pytest.raises(PermissionDeniedError): - await delete_message_use_case(carol, alice_message.id) + await delete_message_use_case(actor=carol, message_id=alice_message.id) async def test_deleting_an_already_deleted_message_is_idempotent( @@ -117,10 +127,10 @@ async def test_deleting_an_already_deleted_message_is_idempotent( alice_message: tables.MessagesTable, alice: tables.UsersTable, ) -> None: - await delete_message_use_case(alice, alice_message.id) + await delete_message_use_case(actor=alice, message_id=alice_message.id) first_deleted_at = (await messages_repository.get_one(id=alice_message.id)).deleted_at - await delete_message_use_case(alice, alice_message.id) + await delete_message_use_case(actor=alice, message_id=alice_message.id) stored = await messages_repository.get_one(id=alice_message.id) assert stored.deleted_at == first_deleted_at @@ -136,10 +146,12 @@ async def test_deleted_message_disappears_from_listing( # A second, undeleted message proves the listing filters *deleted* messages specifically - # an empty result here would prove nothing, since the chat would just be empty either way. other, _ = await create_message_use_case( - alice, alice_message.chat_id, schemas.SendMessageRequest(idempotency_key=uuid.uuid4(), text="still here") + actor=alice, + chat_id=alice_message.chat_id, + data=schemas.SendMessageRequest(idempotency_key=uuid.uuid4(), text="still here"), ) - await delete_message_use_case(alice, alice_message.id) - page = await fetch_messages_use_case(alice, alice_message.chat_id) + await delete_message_use_case(actor=alice, message_id=alice_message.id) + page = await fetch_messages_use_case(actor=alice, chat_id=alice_message.chat_id) assert [message.id for message in page] == [other.id] diff --git a/tests/use_cases/test_unread_counts.py b/tests/use_cases/test_unread_counts.py index 6c592f3..7051a30 100644 --- a/tests/use_cases/test_unread_counts.py +++ b/tests/use_cases/test_unread_counts.py @@ -25,8 +25,8 @@ async def test_unread_counts_messages_from_others( ) -> None: await send(bob, direct_chat.id, "one") await send(bob, direct_chat.id, "two") - rows = await fetch_chats_use_case(alice) - assert rows[0].unread_count == 2 + chats = await fetch_chats_use_case(actor=alice) + assert chats[0].unread_count == 2 async def test_own_messages_are_never_unread( @@ -36,8 +36,8 @@ async def test_own_messages_are_never_unread( send: SendFixture, ) -> None: await send(alice, direct_chat.id, "mine") - rows = await fetch_chats_use_case(alice) - assert rows[0].unread_count == 0 + chats = await fetch_chats_use_case(actor=alice) + assert chats[0].unread_count == 0 async def test_system_messages_count_as_unread( @@ -49,8 +49,8 @@ async def test_system_messages_count_as_unread( await messages_repository.create( tables.MessagesTable(chat_id=direct_chat.id, user_id=None, idempotency_key=uuid.uuid4(), text="Bob joined") ) - rows = await fetch_chats_use_case(alice) - assert rows[0].unread_count == 1 + chats = await fetch_chats_use_case(actor=alice) + assert chats[0].unread_count == 1 async def test_marking_read_clears_the_count( # noqa: PLR0913, PLR0917 - each is a fixture-injected dependency @@ -62,9 +62,11 @@ async def test_marking_read_clears_the_count( # noqa: PLR0913, PLR0917 - each i send: SendFixture, ) -> None: message, _ = await send(bob, direct_chat.id, "one") - await mark_read_use_case(alice, direct_chat.id, schemas.MarkReadRequest(last_read_message_id=message.id)) - rows = await fetch_chats_use_case(alice) - assert rows[0].unread_count == 0 + await mark_read_use_case( + actor=alice, chat_id=direct_chat.id, data=schemas.MarkReadRequest(last_read_message_id=message.id) + ) + chats = await fetch_chats_use_case(actor=alice) + assert chats[0].unread_count == 0 async def test_deleted_messages_are_not_unread( # noqa: PLR0913, PLR0917 - each is a fixture-injected dependency @@ -76,18 +78,18 @@ async def test_deleted_messages_are_not_unread( # noqa: PLR0913, PLR0917 - each send: SendFixture, ) -> None: message, _ = await send(bob, direct_chat.id, "one") - await delete_message_use_case(bob, message.id) - rows = await fetch_chats_use_case(alice) - assert rows[0].unread_count == 0 + await delete_message_use_case(actor=bob, message_id=message.id) + chats = await fetch_chats_use_case(actor=alice) + assert chats[0].unread_count == 0 async def test_chat_with_no_messages_has_no_last_message( fetch_chats_use_case: FetchChatsUseCase, direct_chat: tables.ChatsTable, alice: tables.UsersTable ) -> None: - rows = await fetch_chats_use_case(alice) - assert rows[0].chat.id == direct_chat.id - assert rows[0].last_message is None - assert rows[0].unread_count == 0 + chats = await fetch_chats_use_case(actor=alice) + assert chats[0].id == direct_chat.id + assert chats[0].last_message is None + assert chats[0].unread_count == 0 async def test_listing_orders_most_recently_active_chat_first( # noqa: PLR0913, PLR0917 - fixture-injected @@ -99,11 +101,11 @@ async def test_listing_orders_most_recently_active_chat_first( # noqa: PLR0913, send: SendFixture, ) -> None: other_chat, _ = await create_chat_use_case( - alice, schemas.CreateChatRequest(chat_type=tables.ChatType.DIRECT, member_ids=[carol.id]) + actor=alice, data=schemas.CreateChatRequest(chat_type=tables.ChatType.DIRECT, member_ids=[carol.id]) ) await send(alice, direct_chat.id, "first chat gets a message") - rows = await fetch_chats_use_case(alice) - assert [row.chat.id for row in rows] == [direct_chat.id, other_chat.id] + chats = await fetch_chats_use_case(actor=alice) + assert [chat.id for chat in chats] == [direct_chat.id, other_chat.id] async def test_unread_counts_differ_per_chat( # noqa: PLR0913, PLR0917 - each is a fixture-injected dependency @@ -115,19 +117,19 @@ async def test_unread_counts_differ_per_chat( # noqa: PLR0913, PLR0917 - each i carol: tables.UsersTable, send: SendFixture, ) -> None: - # A correlated subquery that returned the same count for every row would still pass a test + # A correlated subquery that returned the same count for every chat would still pass a test # that only checks one chat - two chats with two different counts is what proves it's - # actually correlated per-row rather than computed once and reused. + # actually correlated per-chat rather than computed once and reused. other_chat, _ = await create_chat_use_case( - alice, schemas.CreateChatRequest(chat_type=tables.ChatType.DIRECT, member_ids=[carol.id]) + actor=alice, data=schemas.CreateChatRequest(chat_type=tables.ChatType.DIRECT, member_ids=[carol.id]) ) await send(bob, direct_chat.id, "one") await send(bob, direct_chat.id, "two") await send(carol, other_chat.id, "hi") - rows = await fetch_chats_use_case(alice) + chats = await fetch_chats_use_case(actor=alice) - counts = {row.chat.id: row.unread_count for row in rows} + counts = {chat.id: chat.unread_count for chat in chats} assert counts == {direct_chat.id: 2, other_chat.id: 1} @@ -135,7 +137,9 @@ async def test_non_member_cannot_mark_read( mark_read_use_case: MarkReadUseCase, direct_chat: tables.ChatsTable, carol: tables.UsersTable ) -> None: with pytest.raises(PermissionDeniedError): - await mark_read_use_case(carol, direct_chat.id, schemas.MarkReadRequest(last_read_message_id=1)) + await mark_read_use_case( + actor=carol, chat_id=direct_chat.id, data=schemas.MarkReadRequest(last_read_message_id=1) + ) async def test_marking_read_with_a_message_from_another_chat_is_rejected( # noqa: PLR0913, PLR0917 - fixture-injected @@ -147,18 +151,22 @@ async def test_marking_read_with_a_message_from_another_chat_is_rejected( # noq send: SendFixture, ) -> None: other_chat, _ = await create_chat_use_case( - alice, schemas.CreateChatRequest(chat_type=tables.ChatType.DIRECT, member_ids=[carol.id]) + actor=alice, data=schemas.CreateChatRequest(chat_type=tables.ChatType.DIRECT, member_ids=[carol.id]) ) other_message, _ = await send(alice, other_chat.id, "elsewhere") with pytest.raises(ValidationError): - await mark_read_use_case(alice, direct_chat.id, schemas.MarkReadRequest(last_read_message_id=other_message.id)) + await mark_read_use_case( + actor=alice, chat_id=direct_chat.id, data=schemas.MarkReadRequest(last_read_message_id=other_message.id) + ) async def test_marking_read_rejects_an_unknown_message_id( mark_read_use_case: MarkReadUseCase, direct_chat: tables.ChatsTable, alice: tables.UsersTable ) -> None: with pytest.raises(ValidationError): - await mark_read_use_case(alice, direct_chat.id, schemas.MarkReadRequest(last_read_message_id=999999)) + await mark_read_use_case( + actor=alice, chat_id=direct_chat.id, data=schemas.MarkReadRequest(last_read_message_id=999999) + ) async def test_marking_read_is_monotonic( # noqa: PLR0913, PLR0917 - each is a fixture-injected dependency @@ -171,14 +179,18 @@ async def test_marking_read_is_monotonic( # noqa: PLR0913, PLR0917 - each is a ) -> None: first, _ = await send(bob, direct_chat.id, "one") second, _ = await send(bob, direct_chat.id, "two") - await mark_read_use_case(alice, direct_chat.id, schemas.MarkReadRequest(last_read_message_id=second.id)) + await mark_read_use_case( + actor=alice, chat_id=direct_chat.id, data=schemas.MarkReadRequest(last_read_message_id=second.id) + ) # An out-of-order/replayed request naming an earlier message must not move the marker back. - member = await mark_read_use_case(alice, direct_chat.id, schemas.MarkReadRequest(last_read_message_id=first.id)) + member = await mark_read_use_case( + actor=alice, chat_id=direct_chat.id, data=schemas.MarkReadRequest(last_read_message_id=first.id) + ) assert member.last_read_message_id == second.id - rows = await fetch_chats_use_case(alice) - assert rows[0].unread_count == 0 + chats = await fetch_chats_use_case(actor=alice) + assert chats[0].unread_count == 0 async def test_deleting_the_newest_message_updates_preview_and_ordering( # noqa: PLR0913, PLR0917 - fixture-injected @@ -191,23 +203,23 @@ async def test_deleting_the_newest_message_updates_preview_and_ordering( # noqa send: SendFixture, ) -> None: other_chat, _ = await create_chat_use_case( - alice, schemas.CreateChatRequest(chat_type=tables.ChatType.DIRECT, member_ids=[carol.id]) + actor=alice, data=schemas.CreateChatRequest(chat_type=tables.ChatType.DIRECT, member_ids=[carol.id]) ) await send(alice, direct_chat.id, "direct chat message") # other_chat's only message - deleting it must also cover the "deleting the only message" # case: last_message becomes null and the chat sorts last. newest, _ = await send(alice, other_chat.id, "other chat message") - before = await fetch_chats_use_case(alice) - assert [row.chat.id for row in before] == [other_chat.id, direct_chat.id] + before = await fetch_chats_use_case(actor=alice) + assert [chat.id for chat in before] == [other_chat.id, direct_chat.id] - await delete_message_use_case(alice, newest.id) + await delete_message_use_case(actor=alice, message_id=newest.id) - after = await fetch_chats_use_case(alice) - assert [row.chat.id for row in after] == [direct_chat.id, other_chat.id] - other_row = next(row for row in after if row.chat.id == other_chat.id) - assert other_row.last_message is None - assert other_row.chat.last_message_id is None + after = await fetch_chats_use_case(actor=alice) + assert [chat.id for chat in after] == [direct_chat.id, other_chat.id] + listed_other_chat = next(chat for chat in after if chat.id == other_chat.id) + assert listed_other_chat.last_message is None + assert listed_other_chat.last_message_id is None async def test_deleting_a_non_newest_message_leaves_preview_and_ordering_unchanged( @@ -220,9 +232,9 @@ async def test_deleting_a_non_newest_message_leaves_preview_and_ordering_unchang first, _ = await send(alice, direct_chat.id, "first") second, _ = await send(alice, direct_chat.id, "second") - await delete_message_use_case(alice, first.id) + await delete_message_use_case(actor=alice, message_id=first.id) - rows = await fetch_chats_use_case(alice) - assert rows[0].chat.last_message_id == second.id - assert rows[0].last_message is not None - assert rows[0].last_message.id == second.id + chats = await fetch_chats_use_case(actor=alice) + assert chats[0].last_message_id == second.id + assert chats[0].last_message is not None + assert chats[0].last_message.id == second.id