diff --git a/.dockerignore b/.dockerignore new file mode 100644 index 0000000..431184e --- /dev/null +++ b/.dockerignore @@ -0,0 +1,6 @@ +.git +.venv +.idea +.superpowers +__pycache__ +*.pyc diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml new file mode 100644 index 0000000..d6e136f --- /dev/null +++ b/.github/workflows/main.yml @@ -0,0 +1,59 @@ +name: CI + +on: + push: + branches: [main] + pull_request: + branches: [main] + +concurrency: + group: ${{ github.head_ref || github.run_id }} + cancel-in-progress: true + +jobs: + lint: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + - uses: astral-sh/setup-uv@v8.2.0 + - run: uv python install 3.14 + - run: uv python pin 3.14 + - run: | + uv sync --all-extras --all-groups --no-install-project + uv run ruff format . --check + uv run ruff check . --no-fix + uv run ty check + uv run python planning/index.py --check + + pytest: + runs-on: ubuntu-latest + services: + postgres: + image: postgres:17 + env: + POSTGRES_DB: postgres + POSTGRES_PASSWORD: password + POSTGRES_USER: postgres + ports: ["5432:5432"] + options: >- + --health-cmd pg_isready + --health-interval 10s + --health-timeout 5s + --health-retries 5 + steps: + - uses: actions/checkout@v6 + - uses: astral-sh/setup-uv@v8.2.0 + - run: uv python install 3.14 + - run: uv python pin 3.14 + - run: | + uv sync --all-extras --all-groups --no-install-project + uv run alembic upgrade head + uv run pytest . + env: + SERVICE_ENVIRONMENT: ci + PYTHONDONTWRITEBYTECODE: 1 + PYTHONUNBUFFERED: 1 + DB_DSN: postgresql+asyncpg://postgres:password@127.0.0.1/postgres + # Must match docker-compose.yml and stay >= 32 bytes: PyJWT warns below the + # HS256 minimum (RFC 7518 3.2), and filterwarnings = ["error"] makes that fatal. + JWT_SECRET: insecure-ci-secret-do-not-use-in-prod diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..69ac236 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,158 @@ +# CLAUDE.md + +This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. + +`chat-app` was bootstrapped from `litestar-sqlalchemy-template`. It is **not** +the template with new routes — three things below differ from what the +template teaches, and getting any of them wrong by pattern-matching on the +template breaks the transaction model or the DI wiring. + +## The three things most likely to be got wrong + +1. **`modern-di` 3.x uses `cache=`, not `cache_settings=`.** Providers that + need a finalizer or app-scope caching pass `cache=providers.CacheSettings(finalizer=...)` + (see `app/ioc.py::Database.database_engine`). The template predates this + API; do not copy `cache_settings=` from memory or from an older `modern-di` + example. +2. **Repositories run `auto_commit=False`.** Every `*_repository` provider in + `app/ioc.py` is constructed with `kwargs={"session": ..., "auto_commit": + False}` — the opposite of the template's `auto_commit=True`. A repository + here never commits on its own. +3. **Use cases own the transaction boundary, not repositories.** Every use + case that writes wraps its work in `async with self.transaction:` (a + `db_retry.Transaction`) and calls `await self.transaction.commit()` + explicitly once every write that must land together has been made — e.g. + `CreateMessageUseCase` commits the new message row and the + `chats.last_message_id` update together. This exists because a single + operation can span more than one repository write and they must succeed or + fail as a unit; giving that back to individually auto-committing + repositories would make that impossible. See `architecture/messages.md` + and `architecture/chats.md` for the two hazards this creates around + `Transaction.__aexit__`'s unconditional rollback-on-open-transaction + behavior (returning a loaded ORM object from inside an uncommitted `async + with self.transaction:` block detaches it). + +## Commands + +Recipes live in the `Justfile` — run `just --list` to see them; this section +only covers what isn't obvious from the recipe names. + +Almost everything runs through Docker Compose: the app and Postgres come up +together, and running tests/migrations outside Docker is **not** the +supported path (`just install` and `just lint` are the exceptions — they run +on the host). Inside the container, raw commands look like `uv run pytest +...`, `uv run alembic ...`. + +- `just test` cycles the DB (downgrade to `base`, upgrade to `head`) before + pytest and tears the stack down before and after. Pass pytest args through, + e.g. `just test tests/use_cases/test_create_chat.py -k race -x`. +- `just migration "message"` takes a **single positional argument** — not a + `-m` flag — quoted so a multi-word message survives as one token (the + recipe shell-quotes it with `quote()` before handing it to `alembic + revision --autogenerate -m`). It runs against an already-upgraded DB; the + recipe enforces that by upgrading first, so don't run autogen by hand. +- `just lint` runs `eof-fixer`, `ruff format`, `ruff check --fix`, then `ty + check` — this project uses `ty`, not mypy; suppress with `# ty: + ignore[]` (not `# type: ignore`). +- `just index` prints the planning change/decision listing; `just + check-planning` validates `planning/changes/` and `planning/decisions/` + frontmatter (CI-equivalent check, run before pushing a planning change). + +Python is 3.14, dependencies managed by `uv`. The API is exposed on `:8000`. + +## Architecture + +**Stack**: Litestar + SQLAlchemy 2 (async) + advanced-alchemy + Alembic + +Postgres 17 + Granian (ASGI server) + `modern-di` (IoC) + `lite-bootstrap` +(observability/CORS/Sentry/OTel wiring) + `db-retry` (transaction boundary + +retry decorator). + +**Request flow**: `app/api/__main__.py` → `granian` → `app.api.app:build_app` +(factory) → `LitestarBootstrapper` from `lite-bootstrap` wraps a +`litestar.Litestar` with OpenTelemetry (asyncpg + SQLAlchemy instrumentors, +with `AsyncPGInstrumentor(capture_parameters=False)` so argon2 password +hashes bound as INSERT parameters never reach the OTel collector), Sentry, +CORS, Swagger, etc., based on `Settings.api_bootstrapper_config`. +`build_app` also calls `settings.ensure_jwt_secret_is_configured()` first, +which raises at startup if a non-local environment is still running the +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: +- `Database.database_engine` — app-scoped factory, `cache=` finalizer disposes + the engine. +- `Database.database_session` — request-scoped, finalizer closes the session. +- `Database.transaction` — request-scoped `db_retry.Transaction`, the object + every write-side use case wraps its commit in. +- `Repositories.*` — request-scoped, `auto_commit=False` (see above). +- `UseCases.*` — request-scoped, one class per operation. + +**Persistence**: Models inherit `advanced_alchemy.base.BigIntAuditBase` / +`BigIntBase`. `app/database/tables.py` shares metadata with +`orm.DeclarativeBase.metadata` (`METADATA = orm_registry.metadata; +orm.DeclarativeBase.metadata = METADATA`) so Alembic autogen sees everything — +this line mutates a third-party base class at import time; see the comment +above it in the source for why. Repositories are +`SQLAlchemyAsyncRepositoryService[Model]` with a nested +`BaseRepository(SQLAlchemyAsyncRepository[Model])`, same shape as the +template, but every service here is constructed with `auto_commit=False`. + +**Test isolation** (`tests/conftest.py`): `db_session` opens a connection, +starts a transaction, then **overrides** `Database.database_engine` in the DI +container to return that connection; `create_session`'s +`join_transaction_mode="create_savepoint"` is what makes every session opened +against it — fixture or route handler — nest as a savepoint instead of +committing past the outer transaction. Teardown rolls the outer transaction +back. `app`/`client` fixtures build the real app and run it through +`httpx.ASGITransport` + `asgi_lifespan.LifespanManager`. +`modern_di_pytest.expose(ioc.Repositories, ioc.UseCases, +container_fixture="request_container")` (`tests/use_cases/conftest.py`) +exposes every repository/use case provider as a same-named pytest fixture — +the template predates this and hand-assembles dependencies instead. Full +detail, including the race-simulation pattern used to test the +concurrent-retry paths without a second real connection, is in +`architecture/testing.md`. + +**Migrations**: `migrations/env.py` reads the shared `METADATA` and rewrites +the DSN driver from `postgresql+asyncpg` → `postgresql` (Alembic uses sync +psycopg2). Always run autogen against an upgraded DB — `just migration` +enforces this. + +**Settings** (`app/settings.py`): `pydantic_settings.BaseSettings` reads from +env vars (see `docker-compose.yml`). `api_bootstrapper_config` builds the +`LitestarConfig` consumed by `lite-bootstrap`. `jwt_cookie_secure` defaults +`False` for local `http://` development and must be `True` behind HTTPS. + +## Conventions + +- Routes live in `app/api/endpoints/`, one module per resource (`auth.py`, + `chats.py`, `messages.py`), each exposing its own `ROUTER` (`litestar.Router`, + prefix `/api`). `app/api/app.py::build_app` registers them all via + `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. +- 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 + use case needs (e.g. the author-and-member gate for edit/delete) lives in a + plain module-level function, not a base class — see + `app/use_cases/message_authorization.py`. +- Pydantic schemas in `app/schemas/api.py` use `from_attributes=True` (via + `Base`) so they validate directly from ORM instances + (`schemas.X.model_validate(orm_instance)`). Collection responses go through + `Collection[T].from_models(...)` (e.g. `schemas.Messages`, `schemas.Chats`). +- Domain exceptions (`app/exceptions.py`: `PermissionDeniedError`, + `ValidationError`, `ConflictError`) are registered as handlers in + `build_app`'s `exception_handlers` dict alongside the `advanced_alchemy` + exceptions (`NotFoundError`, `DuplicateKeyError`, `ForeignKeyError`). Full + mapping table and the one deliberate exception (login's `401` via Litestar's + own `NotAuthorizedException`) are in `architecture/messages.md` and + `architecture/auth.md`. +- `ruff` is configured with `select = ["ALL"]` and a line length of 120 — + expect strict lint. Type-check with `ty`; use `# ty: ignore[]` for + suppressions. diff --git a/Dockerfile b/Dockerfile new file mode 100644 index 0000000..450acb3 --- /dev/null +++ b/Dockerfile @@ -0,0 +1,26 @@ +FROM python:3.14-slim + +RUN apt update \ + && apt install -y --no-install-recommends build-essential libpq-dev \ + && apt clean \ + && rm -rf /var/lib/apt/lists/* + +COPY --from=ghcr.io/astral-sh/uv:latest /uv /bin/uv +RUN useradd --no-create-home --gid root runner + +ENV UV_PROJECT_ENVIRONMENT=/code/.venv \ + UV_NO_MANAGED_PYTHON=1 \ + UV_NO_CACHE=true \ + UV_LINK_MODE=copy + +WORKDIR /code + +COPY pyproject.toml . + +RUN uv sync --all-extras --all-groups --no-install-project + +COPY . . + +RUN chown -R runner:root /code && chmod -R g=u /code + +USER runner diff --git a/Justfile b/Justfile new file mode 100644 index 0000000..b02f2b0 --- /dev/null +++ b/Justfile @@ -0,0 +1,41 @@ +default: install lint build test + +down: + docker compose down --remove-orphans + +sh: + docker compose run --service-ports api bash + +test *args: down && down + docker compose run api sh -c "sleep 1 && uv run alembic downgrade base && uv run alembic upgrade head && uv run pytest {{ args }}" + +run: + docker compose run --service-ports api sh -c "sleep 1 && uv run alembic upgrade head && uv run python -m app.api" + +migration message: && down + # `message` is a single named parameter, shell-quoted via quote() so a multi-word message + # survives intact - a variadic *args parameter only ever joins tokens with spaces when + # interpolated, losing the quoting boundaries the invoking shell already stripped, so a + # multi-word message would otherwise reach sh -c as several disconnected words. + docker compose run api sh -c "sleep 1 && uv run alembic upgrade head && uv run alembic revision --autogenerate -m {{ quote(message) }}" + +build: + docker compose build api + +install: + uv lock --upgrade + uv sync --all-extras --all-groups --no-install-project + +lint: + uv run eof-fixer . + uv run ruff format . + uv run ruff check . --fix + uv run ty check + +# Print the planning change index (flat, newest-first) to stdout. +index: + uv run python planning/index.py + +# Validate planning changes + decisions (frontmatter, lanes, spec links); CI runs this. +check-planning: + uv run python planning/index.py --check diff --git a/LICENSE b/LICENSE new file mode 100644 index 0000000..667cfff --- /dev/null +++ b/LICENSE @@ -0,0 +1,21 @@ +MIT License + +Copyright (c) 2026 Artur Shiriev + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. diff --git a/alembic.ini b/alembic.ini new file mode 100644 index 0000000..455a1fa --- /dev/null +++ b/alembic.ini @@ -0,0 +1,87 @@ +# A generic, single database configuration. + +[alembic] +# path to migration scripts +script_location = migrations + +# template used to generate migration files +file_template = %%(year)d-%%(month).2d-%%(day).2d_%%(slug)s + +# sys.path path, will be prepended to sys.path if present. +# defaults to the current working directory. +prepend_sys_path = . + +# timezone to use when rendering the date +# within the migration file as well as the filename. +# string value is passed to dateutil.tz.gettz() +# leave blank for localtime +# timezone = + +# max length of characters to apply to the +# "slug" field +# truncate_slug_length = 40 + +# set to 'true' to run the environment during +# the 'revision' command, regardless of autogenerate +# revision_environment = false + +# set to 'true' to allow .pyc and .pyo files without +# a source .py file to be detected as revisions in the +# versions/ directory +# sourceless = false + +# version location specification; this defaults +# to migrations/versions. When using multiple version +# directories, initial revisions must be specified with --version-path +# version_locations = %(here)s/bar %(here)s/bat migrations/versions + +# the output encoding used when revision files +# are written from script.py.mako +# output_encoding = utf-8 + + +[post_write_hooks] +# post_write_hooks defines scripts or Python functions that are run +# on newly generated revision scripts. See the documentation for further +# detail and examples + +# format using "black" - use the console_scripts runner, against the "black" entrypoint +# hooks=black +# black.type=console_scripts +# black.entrypoint=black +# black.options=-l 79 + +# Logging configuration +[loggers] +keys = root,sqlalchemy,alembic + +[handlers] +keys = console + +[formatters] +keys = generic + +[logger_root] +level = WARN +handlers = console +qualname = + +[logger_sqlalchemy] +level = WARN +handlers = +qualname = sqlalchemy.engine + +[logger_alembic] +level = INFO +handlers = +qualname = alembic + +[handler_console] +class = StreamHandler +args = (sys.stderr,) +level = NOTSET +formatter = generic + +[formatter_generic] +format = %(levelname)-5.5s [%(name)s] %(message)s +datefmt = %H:%M:%S diff --git a/app/__init__.py b/app/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/app/api/__init__.py b/app/api/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/app/api/__main__.py b/app/api/__main__.py new file mode 100644 index 0000000..0d33b8e --- /dev/null +++ b/app/api/__main__.py @@ -0,0 +1,17 @@ +import granian +from granian.constants import Interfaces, Loops +from granian.log import LogLevels + +from app.settings import settings + + +if __name__ == "__main__": + granian.Granian( + target="app.api.app:build_app", + factory=True, + address=settings.app_host, + port=settings.app_port, + interface=Interfaces.ASGI, + log_level=LogLevels(settings.log_level), + loop=Loops.uvloop, + ).serve() diff --git a/app/api/app.py b/app/api/app.py new file mode 100644 index 0000000..878285f --- /dev/null +++ b/app/api/app.py @@ -0,0 +1,70 @@ +import dataclasses +import typing + +import litestar +import modern_di +import modern_di_litestar +from advanced_alchemy.exceptions import DuplicateKeyError, ForeignKeyError, NotFoundError +from lite_bootstrap import LitestarBootstrapper +from litestar.config.app import AppConfig +from opentelemetry.instrumentation.asyncpg import AsyncPGInstrumentor +from opentelemetry.instrumentation.sqlalchemy import SQLAlchemyInstrumentor + +from app import ioc +from app.api import exception_handlers +from app.api.auth import JWTCookieAuthPlugin +from app.api.endpoints import auth as auth_endpoints +from app.api.endpoints import chats as chats_endpoints +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: + settings.ensure_jwt_secret_is_configured() + di_container: typing.Final = modern_di.Container(groups=ioc.ALL_GROUPS) + bootstrap_config: typing.Final = dataclasses.replace( + settings.api_bootstrapper_config, + application_config=AppConfig( + exception_handlers={ + NotFoundError: exception_handlers.not_found_error_handler, + PermissionDeniedError: exception_handlers.permission_denied_handler, + DuplicateKeyError: exception_handlers.duplicate_key_error_handler, + ForeignKeyError: exception_handlers.foreign_key_error_handler, + ValidationError: exception_handlers.validation_error_handler, + 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), + }, + request_max_body_size=settings.request_max_body_size, + ), + opentelemetry_instrumentors=[ + SQLAlchemyInstrumentor(), + # False: bound query parameters include argon2 password hashes (every registration + # INSERTs one) - capturing them would ship credential material to the OTel collector. + AsyncPGInstrumentor(capture_parameters=False), + ], + ) + return LitestarBootstrapper(bootstrap_config=bootstrap_config).bootstrap() diff --git a/app/api/auth.py b/app/api/auth.py new file mode 100644 index 0000000..8292062 --- /dev/null +++ b/app/api/auth.py @@ -0,0 +1,69 @@ +import datetime +import typing + +import modern_di_litestar +from litestar.config.app import AppConfig +from litestar.connection import ASGIConnection +from litestar.plugins import InitPlugin +from litestar.security.jwt import JWTCookieAuth, Token + +from app import ioc +from app.database import resources as database_resources +from app.database import tables +from app.settings import settings + + +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 + # factory sets join_transaction_mode="create_savepoint", which is what keeps the per-test + # rollback fixture intact when the engine provider is overridden with a live connection. + try: + user_id = int(token.sub) + except ValueError: + # Token.sub is only guaranteed to be a non-empty string; a malformed/forged subject + # must fail authentication (401 via the middleware), not crash the request (500). + return None + di_container: typing.Final = modern_di_litestar.fetch_di_container(connection.app) + engine: typing.Final = di_container.resolve_provider(ioc.Database.database_engine) + session: typing.Final = database_resources.create_session(engine) + try: + return await session.get(tables.UsersTable, user_id) + finally: + await database_resources.close_session(session) + + +jwt_cookie_auth: typing.Final = JWTCookieAuth[tables.UsersTable]( + retrieve_user_handler=retrieve_user_handler, + token_secret=settings.jwt_secret, + default_token_expiration=datetime.timedelta(seconds=settings.jwt_lifetime_seconds), + secure=settings.jwt_cookie_secure, + exclude=[ + # /auth/register and /auth/login opt out via exclude_from_auth=True on the handlers + # themselves (see app/api/endpoints/auth.py) - that is their one policy home, not here. + # Litestar joins these into a single alternation and matches with an unanchored findall + # (litestar/middleware/_utils.py), so each pattern is anchored to the path start to avoid + # accidentally un-authenticating a future route that merely contains "/docs" or "/health" + # as a substring (e.g. "/api/chats/{id}/health"). + "^/docs", + "^/health", + # Swagger's offline assets (settings.swagger_offline_docs=True) are served from here; + # without this the docs page loads but every asset request 401s for an anonymous visitor. + "^/static", + # A Prometheus scrape target must be reachable without a session cookie; the endpoint + # carries no user data, only process/request metrics. + "^/metrics", + ], +) + + +class JWTCookieAuthPlugin(InitPlugin): + # AppConfig has no on_app_init field (that hook is a Litestar.__init__-only parameter, + # unavailable through LitestarBootstrapper's AppConfig -> Litestar.from_config path), and + # jwt_cookie_auth itself is an unhashable dataclass so it cannot sit in `plugins` directly + # (PluginRegistry stores plugins in a frozenset). This plugin wrapper is hashable by identity + # and forwards to jwt_cookie_auth.on_app_init, which Litestar.__init__ calls for every + # InitPluginProtocol member of `plugins` after the bootstrapper has finished mutating + # application_config (so openapi_config is already populated by then). + def on_app_init(self, app_config: AppConfig) -> AppConfig: + return jwt_cookie_auth.on_app_init(app_config) diff --git a/app/api/endpoints/__init__.py b/app/api/endpoints/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/app/api/endpoints/auth.py b/app/api/endpoints/auth.py new file mode 100644 index 0000000..0fa0a86 --- /dev/null +++ b/app/api/endpoints/auth.py @@ -0,0 +1,59 @@ +import typing + +import litestar +from litestar import status_codes +from litestar.di import NamedDependency +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.schemas import api as schemas +from app.use_cases.authenticate_user import AuthenticateUserUseCase +from app.use_cases.register_user import RegisterUserUseCase + + +@litestar.post("/auth/register/", status_code=status_codes.HTTP_201_CREATED, exclude_from_auth=True) +async def register( + data: schemas.RegisterRequest, + register_user_use_case: NamedDependency[RegisterUserUseCase], +) -> Response[schemas.User]: + user: typing.Final = await register_user_use_case(data) + return jwt_cookie_auth.login( + identifier=str(user.id), + response_body=schemas.User.model_validate(user), + response_status_code=status_codes.HTTP_201_CREATED, + ) + + +@litestar.post("/auth/login/", status_code=status_codes.HTTP_200_OK, exclude_from_auth=True) +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) + if user is None: + raise NotAuthorizedException(detail="Invalid username or password") + return jwt_cookie_auth.login( + identifier=str(user.id), + response_body=schemas.User.model_validate(user), + response_status_code=status_codes.HTTP_200_OK, + ) + + +@litestar.post("/auth/logout/", status_code=status_codes.HTTP_204_NO_CONTENT) +async def logout() -> Response[None]: + response: typing.Final = Response(content=None, status_code=status_codes.HTTP_204_NO_CONTENT) + response.delete_cookie(jwt_cookie_auth.key) + return response + + +@litestar.get("/auth/me/") +async def me(request: litestar.Request[tables.UsersTable, typing.Any, typing.Any]) -> schemas.User: + return schemas.User.model_validate(request.user) + + +ROUTER: typing.Final = litestar.Router( + path="/api", + route_handlers=[register, login, logout, me], +) diff --git a/app/api/endpoints/chats.py b/app/api/endpoints/chats.py new file mode 100644 index 0000000..e1b9f25 --- /dev/null +++ b/app/api/endpoints/chats.py @@ -0,0 +1,70 @@ +import typing + +import litestar +from litestar import status_codes +from litestar.di import NamedDependency +from litestar.openapi.datastructures import ResponseSpec +from litestar.params import FromPath + +from app.database import tables +from app.schemas import api as schemas +from app.use_cases.create_chat import CreateChatUseCase +from app.use_cases.fetch_chat import FetchChatUseCase +from app.use_cases.fetch_chats import FetchChatsUseCase +from app.use_cases.mark_read import MarkReadUseCase + + +@litestar.post( + "/chats/", + responses={ + status_codes.HTTP_200_OK: ResponseSpec( + data_container=schemas.ChatDetail, description="An existing direct chat for this pair of members" + ), + }, +) +async def create_chat( + data: schemas.CreateChatRequest, + request: litestar.Request[tables.UsersTable, typing.Any, typing.Any], + create_chat_use_case: NamedDependency[CreateChatUseCase], +) -> litestar.Response[schemas.ChatDetail]: + chat, created = await create_chat_use_case(request.user, 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, + ) + + +@litestar.get("/chats/") +async def list_chats( + request: litestar.Request[tables.UsersTable, typing.Any, typing.Any], + 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 + ) + + +@litestar.get("/chats/{chat_id:int}/") +async def get_chat( + chat_id: FromPath[int], + request: litestar.Request[tables.UsersTable, typing.Any, typing.Any], + fetch_chat_use_case: NamedDependency[FetchChatUseCase], +) -> schemas.ChatDetail: + chat: typing.Final = await fetch_chat_use_case(request.user, chat_id) + return schemas.ChatDetail.model_validate(chat) + + +@litestar.post("/chats/{chat_id:int}/read/", status_code=status_codes.HTTP_200_OK) +async def mark_read( + chat_id: FromPath[int], + data: schemas.MarkReadRequest, + request: litestar.Request[tables.UsersTable, typing.Any, typing.Any], + mark_read_use_case: NamedDependency[MarkReadUseCase], +) -> schemas.ChatMember: + member: typing.Final = await mark_read_use_case(request.user, chat_id, data) + return schemas.ChatMember.model_validate(member) + + +ROUTER: typing.Final = litestar.Router(path="/api", route_handlers=[create_chat, list_chats, get_chat, mark_read]) diff --git a/app/api/endpoints/messages.py b/app/api/endpoints/messages.py new file mode 100644 index 0000000..5bfb443 --- /dev/null +++ b/app/api/endpoints/messages.py @@ -0,0 +1,76 @@ +import typing + +import litestar +from litestar import status_codes +from litestar.di import NamedDependency +from litestar.openapi.datastructures import ResponseSpec +from litestar.params import FromPath, FromQuery + +from app.database import tables +from app.schemas import api as schemas +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_messages import FetchMessagesUseCase + + +@litestar.post( + "/chats/{chat_id:int}/messages/", + responses={ + status_codes.HTTP_200_OK: ResponseSpec( + data_container=schemas.Message, description="A message already sent with this idempotency key" + ), + }, +) +async def send_message( + chat_id: FromPath[int], + data: schemas.SendMessageRequest, + request: litestar.Request[tables.UsersTable, typing.Any, typing.Any], + create_message_use_case: NamedDependency[CreateMessageUseCase], +) -> litestar.Response[schemas.Message]: + message, created = await create_message_use_case(request.user, chat_id, 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, + ) + + +@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], + fetch_messages_use_case: NamedDependency[FetchMessagesUseCase], + *, + before_id: FromQuery[int | None] = None, + after_id: FromQuery[int | None] = None, + 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 + ) + return schemas.Messages.from_models(messages) + + +@litestar.patch("/messages/{message_id:int}/") +async def edit_message( + message_id: FromPath[int], + data: schemas.EditMessageRequest, + request: litestar.Request[tables.UsersTable, typing.Any, typing.Any], + edit_message_use_case: NamedDependency[EditMessageUseCase], +) -> schemas.Message: + message: typing.Final = await edit_message_use_case(request.user, message_id, 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], + delete_message_use_case: NamedDependency[DeleteMessageUseCase], +) -> None: + await delete_message_use_case(request.user, message_id) + + +ROUTER: typing.Final = litestar.Router( + path="/api", route_handlers=[send_message, list_messages, edit_message, delete_message] +) diff --git a/app/api/exception_handlers.py b/app/api/exception_handlers.py new file mode 100644 index 0000000..84b4f76 --- /dev/null +++ b/app/api/exception_handlers.py @@ -0,0 +1,60 @@ +import typing + +import litestar +from litestar import status_codes + +from app.exceptions import ConflictError, PermissionDeniedError, ValidationError + + +if typing.TYPE_CHECKING: + from advanced_alchemy.exceptions import DuplicateKeyError, ForeignKeyError, NotFoundError + + +def not_found_error_handler(_: object, __: NotFoundError) -> litestar.Response[dict[str, typing.Any]]: + return litestar.Response( + media_type=litestar.MediaType.JSON, + content={"detail": "Not found"}, + status_code=status_codes.HTTP_404_NOT_FOUND, + ) + + +def duplicate_key_error_handler(_: object, __: DuplicateKeyError) -> litestar.Response[dict[str, typing.Any]]: + return litestar.Response( + media_type=litestar.MediaType.JSON, + content={"detail": "Conflict"}, + status_code=status_codes.HTTP_409_CONFLICT, + ) + + +def foreign_key_error_handler(_: object, __: ForeignKeyError) -> litestar.Response[dict[str, typing.Any]]: + return litestar.Response( + media_type=litestar.MediaType.JSON, + # Constant detail, not str(exc): the underlying integrity error can carry bound + # parameter values (e.g. ids from other tables) that shouldn't be echoed back verbatim. + content={"detail": "Invalid reference"}, + status_code=status_codes.HTTP_400_BAD_REQUEST, + ) + + +def permission_denied_handler(_: object, exc: PermissionDeniedError) -> litestar.Response[dict[str, typing.Any]]: + return litestar.Response( + media_type=litestar.MediaType.JSON, + content={"detail": str(exc) or "Permission denied"}, + status_code=status_codes.HTTP_403_FORBIDDEN, + ) + + +def validation_error_handler(_: object, exc: ValidationError) -> litestar.Response[dict[str, typing.Any]]: + return litestar.Response( + media_type=litestar.MediaType.JSON, + content={"detail": str(exc) or "Validation error"}, + status_code=status_codes.HTTP_400_BAD_REQUEST, + ) + + +def conflict_error_handler(_: object, exc: ConflictError) -> litestar.Response[dict[str, typing.Any]]: + return litestar.Response( + media_type=litestar.MediaType.JSON, + content={"detail": str(exc) or "Conflict"}, + status_code=status_codes.HTTP_409_CONFLICT, + ) diff --git a/app/database/__init__.py b/app/database/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/app/database/resources.py b/app/database/resources.py new file mode 100644 index 0000000..f88291d --- /dev/null +++ b/app/database/resources.py @@ -0,0 +1,40 @@ +import asyncio +import typing + +from sqlalchemy.ext import asyncio as sa + +from app.settings import settings + + +def create_database_engine() -> sa.AsyncEngine: + return sa.create_async_engine( + url=settings.db_dsn_parsed, + echo=settings.service_debug, + echo_pool=settings.service_debug, + pool_size=settings.db_pool_size, + pool_pre_ping=settings.db_pool_pre_ping, + max_overflow=settings.db_max_overflow, + ) + + +async def close_database_engine(engine: sa.AsyncEngine) -> None: + await engine.dispose() + + +def create_session(engine: sa.AsyncEngine | sa.AsyncConnection) -> sa.AsyncSession: + # join_transaction_mode is inert in production (the session binds to an engine); when tests bind + # the session to a connection already in a transaction, it makes the session own a savepoint so + # the outer transaction survives commits and the per-test rollback stays clean. The `db_session` + # test fixture overrides `Database.database_engine` with a live `AsyncConnection`, so DI-resolved + # sessions built through this same function get that connection, not just an `AsyncEngine`. + return sa.AsyncSession( + engine, + expire_on_commit=False, + autoflush=False, + join_transaction_mode="create_savepoint", + ) + + +async def close_session(session: sa.AsyncSession) -> None: + task: typing.Final = asyncio.create_task(session.close()) + await asyncio.shield(task) diff --git a/app/database/tables.py b/app/database/tables.py new file mode 100644 index 0000000..c5727bf --- /dev/null +++ b/app/database/tables.py @@ -0,0 +1,92 @@ +import datetime +import enum +import typing +import uuid + +import sqlalchemy as sa +from advanced_alchemy.base import BigIntAuditBase, BigIntBase, orm_registry +from advanced_alchemy.types import GUID, DateTimeUTC +from sqlalchemy import orm + + +METADATA: typing.Final = orm_registry.metadata +# Redirects SQLAlchemy's shared declarative base onto advanced-alchemy's registry metadata so +# that every model below - which inherits BigIntAuditBase/BigIntBase, themselves built on +# orm.DeclarativeBase - registers its table on METADATA. Alembic's env.py autogenerates against +# METADATA directly; without this reassignment, models would register on orm.DeclarativeBase's +# own separate metadata instead, and autogen would see no tables at all. +orm.DeclarativeBase.metadata = METADATA + + +class UsersTable(BigIntAuditBase): + __tablename__ = "users" + + username: orm.Mapped[str] = orm.mapped_column(sa.String(length=64), unique=True) + password_hash: orm.Mapped[str] = orm.mapped_column(sa.String) + display_name: orm.Mapped[str] = orm.mapped_column(sa.String(length=128)) + + +class ChatType(enum.StrEnum): + DIRECT = "direct" + GROUP = "group" + + +def build_direct_key(user_id_a: int, user_id_b: int) -> str: + """Order-independent identity for a direct chat between two users.""" + low, high = sorted((user_id_a, user_id_b)) + return f"{low}:{high}" + + +class ChatsTable(BigIntAuditBase): + __tablename__ = "chats" + + chat_type: orm.Mapped[ChatType] = orm.mapped_column( + sa.Enum( + ChatType, + native_enum=False, + create_constraint=True, + values_callable=lambda enum_cls: [member.value for member in enum_cls], + ) + ) + title: orm.Mapped[str | None] = orm.mapped_column(sa.String(length=128), nullable=True) + created_by_id: orm.Mapped[int] = orm.mapped_column(sa.ForeignKey("users.id")) + last_message_id: orm.Mapped[int | None] = orm.mapped_column(sa.BigInteger, nullable=True) + direct_key: orm.Mapped[str | None] = orm.mapped_column(sa.String(length=64), nullable=True, unique=True) + + members: orm.Mapped[list[ChatMembersTable]] = orm.relationship( + "ChatMembersTable", lazy="noload", uselist=True, viewonly=True + ) + + +class ChatMembersTable(BigIntBase): + __tablename__ = "chat_members" + __table_args__ = (sa.UniqueConstraint("chat_id", "user_id", name="uk_chat_members_chat_id_user_id"),) + + chat_id: orm.Mapped[int] = orm.mapped_column(sa.ForeignKey("chats.id")) + user_id: orm.Mapped[int] = orm.mapped_column(sa.ForeignKey("users.id"), index=True) + last_read_message_id: orm.Mapped[int | None] = orm.mapped_column(sa.BigInteger, nullable=True) + joined_at: orm.Mapped[datetime.datetime] = orm.mapped_column( + DateTimeUTC(timezone=True), default=lambda: datetime.datetime.now(tz=datetime.UTC) + ) + + +class MessagesTable(BigIntBase): + __tablename__ = "messages" + __table_args__ = ( + # No standalone index on chat_id: this composite index already serves every query + # that would use one. + sa.Index("ix_messages_chat_id_id", "chat_id", "id"), + # Idempotency is a property of "send this message to this chat" - two different chats + # are two different operations, so the key is unique per chat, not table-wide. + sa.UniqueConstraint("chat_id", "idempotency_key", name="uk_messages_chat_id_idempotency_key"), + ) + + chat_id: orm.Mapped[int] = orm.mapped_column(sa.ForeignKey("chats.id")) + user_id: orm.Mapped[int | None] = orm.mapped_column(sa.ForeignKey("users.id"), nullable=True, index=True) + idempotency_key: orm.Mapped[uuid.UUID] = orm.mapped_column(GUID) + text: orm.Mapped[str] = orm.mapped_column(sa.String) + created_at: orm.Mapped[datetime.datetime] = orm.mapped_column( + DateTimeUTC(timezone=True), default=lambda: datetime.datetime.now(tz=datetime.UTC) + ) + edited_at: orm.Mapped[datetime.datetime | None] = orm.mapped_column(DateTimeUTC(timezone=True), nullable=True) + deleted_at: orm.Mapped[datetime.datetime | None] = orm.mapped_column(DateTimeUTC(timezone=True), nullable=True) diff --git a/app/exceptions.py b/app/exceptions.py new file mode 100644 index 0000000..c22817c --- /dev/null +++ b/app/exceptions.py @@ -0,0 +1,14 @@ +class ChatAppError(Exception): + """Base class for domain errors raised by use cases.""" + + +class PermissionDeniedError(ChatAppError): + """Raised when an authenticated user may not perform the requested action.""" + + +class ValidationError(ChatAppError): + """Raised when a request is well-formed but violates a domain invariant.""" + + +class ConflictError(ChatAppError): + """Raised when an otherwise-authorized request conflicts with the resource's current state.""" diff --git a/app/ioc.py b/app/ioc.py new file mode 100644 index 0000000..d46b4c1 --- /dev/null +++ b/app/ioc.py @@ -0,0 +1,72 @@ +import typing + +from db_retry import Transaction +from modern_di import Group, Scope, providers + +from app.database import resources as database_resources +from app.repositories.chat_members_repository import ChatMembersRepository +from app.repositories.chats_repository import ChatsRepository +from app.repositories.messages_repository import MessagesRepository +from app.repositories.users_repository import UsersRepository +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 + + +class Database(Group): + database_engine = providers.Factory( + creator=database_resources.create_database_engine, + cache=providers.CacheSettings(finalizer=database_resources.close_database_engine), + ) + database_session = providers.Factory( + scope=Scope.REQUEST, + creator=database_resources.create_session, + cache=providers.CacheSettings(finalizer=database_resources.close_session), + ) + transaction = providers.Factory( + scope=Scope.REQUEST, + creator=Transaction, + kwargs={"session": database_session}, + ) + + +class Repositories(Group, scope=Scope.REQUEST): + users_repository = providers.Factory( + creator=UsersRepository, + kwargs={"session": Database.database_session, "auto_commit": False}, + ) + chats_repository = providers.Factory( + creator=ChatsRepository, + kwargs={"session": Database.database_session, "auto_commit": False}, + ) + chat_members_repository = providers.Factory( + creator=ChatMembersRepository, + kwargs={"session": Database.database_session, "auto_commit": False}, + ) + messages_repository = providers.Factory( + creator=MessagesRepository, + kwargs={"session": Database.database_session, "auto_commit": False}, + ) + + +class UseCases(Group, scope=Scope.REQUEST): + register_user_use_case = providers.Factory(creator=RegisterUserUseCase) + authenticate_user_use_case = providers.Factory(creator=AuthenticateUserUseCase) + create_chat_use_case = providers.Factory(creator=CreateChatUseCase) + fetch_chat_use_case = providers.Factory(creator=FetchChatUseCase) + create_message_use_case = providers.Factory(creator=CreateMessageUseCase) + fetch_messages_use_case = providers.Factory(creator=FetchMessagesUseCase) + edit_message_use_case = providers.Factory(creator=EditMessageUseCase) + delete_message_use_case = providers.Factory(creator=DeleteMessageUseCase) + fetch_chats_use_case = providers.Factory(creator=FetchChatsUseCase) + mark_read_use_case = providers.Factory(creator=MarkReadUseCase) + + +ALL_GROUPS: typing.Final[list[type[Group]]] = [Database, Repositories, UseCases] diff --git a/app/repositories/__init__.py b/app/repositories/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/app/repositories/chat_members_repository.py b/app/repositories/chat_members_repository.py new file mode 100644 index 0000000..833b26a --- /dev/null +++ b/app/repositories/chat_members_repository.py @@ -0,0 +1,42 @@ +import typing + +import sqlalchemy as sa +from advanced_alchemy.repository import SQLAlchemyAsyncRepository +from advanced_alchemy.service import SQLAlchemyAsyncRepositoryService + +from app.database import tables + + +class ChatMembersRepository(SQLAlchemyAsyncRepositoryService[tables.ChatMembersTable]): + class BaseRepository(SQLAlchemyAsyncRepository[tables.ChatMembersTable]): + model_type = tables.ChatMembersTable + + repository_type = BaseRepository + + async def is_member(self, chat_id: int, user_id: int) -> bool: + return await self.exists(chat_id=chat_id, user_id=user_id) + + async def fetch_member(self, chat_id: int, user_id: int) -> tables.ChatMembersTable | None: + return await self.get_one_or_none(chat_id=chat_id, user_id=user_id) + + async def mark_read(self, member_id: int, requested_message_id: int) -> tables.ChatMembersTable: + """Advance last_read_message_id to GREATEST(current, requested), atomically. + + The GREATEST() is computed by the UPDATE itself rather than in Python from a prior read: + a read-modify-write across two calls (fetch_member, then update) would let two concurrent + POST /read/ requests interleave and let the lower id win. This UPDATE's row lock + serializes concurrent writers, and each one recomputes GREATEST against whatever the + winner of that lock just committed. + """ + statement: typing.Final = ( + sa.update(tables.ChatMembersTable) + .where(tables.ChatMembersTable.id == member_id) + .values( + last_read_message_id=sa.func.greatest( + sa.func.coalesce(tables.ChatMembersTable.last_read_message_id, 0), requested_message_id + ) + ) + .returning(tables.ChatMembersTable) + ) + result: typing.Final = await self.repository.session.execute(statement) + return result.scalar_one() diff --git a/app/repositories/chats_repository.py b/app/repositories/chats_repository.py new file mode 100644 index 0000000..e197c60 --- /dev/null +++ b/app/repositories/chats_repository.py @@ -0,0 +1,50 @@ +import typing +from collections.abc import Sequence + +import sqlalchemy as sa +from advanced_alchemy.repository import SQLAlchemyAsyncRepository +from advanced_alchemy.service import SQLAlchemyAsyncRepositoryService +from sqlalchemy import orm + +from app.database import tables + + +class ChatsRepository(SQLAlchemyAsyncRepositoryService[tables.ChatsTable]): + class BaseRepository(SQLAlchemyAsyncRepository[tables.ChatsTable]): + model_type = tables.ChatsTable + + repository_type = BaseRepository + + async def fetch_with_members(self, chat_id: int) -> tables.ChatsTable: + return await self.get_one( + tables.ChatsTable.id == chat_id, + load=[orm.selectinload(tables.ChatsTable.members)], + ) + + async def fetch_direct_by_key(self, direct_key: str) -> tables.ChatsTable | None: + return await self.get_one_or_none( + tables.ChatsTable.direct_key == direct_key, + load=[orm.selectinload(tables.ChatsTable.members)], + ) + + async def list_for_user(self, user_id: int) -> Sequence[sa.Row[tuple[tables.ChatsTable, int]]]: + unread_count: typing.Final = ( + sa.select(sa.func.count(tables.MessagesTable.id)) + .where( + tables.MessagesTable.chat_id == tables.ChatMembersTable.chat_id, + tables.MessagesTable.deleted_at.is_(None), + tables.MessagesTable.user_id.is_distinct_from(tables.ChatMembersTable.user_id), + tables.MessagesTable.id > sa.func.coalesce(tables.ChatMembersTable.last_read_message_id, 0), + ) + .correlate(tables.ChatMembersTable) + .scalar_subquery() + .label("unread_count") + ) + statement: typing.Final = ( + sa.select(tables.ChatsTable, unread_count) + .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()) + ) + result: typing.Final = await self.repository.session.execute(statement) + return result.all() diff --git a/app/repositories/messages_repository.py b/app/repositories/messages_repository.py new file mode 100644 index 0000000..e5e82dd --- /dev/null +++ b/app/repositories/messages_repository.py @@ -0,0 +1,52 @@ +import typing +import uuid +from collections.abc import Sequence + +import sqlalchemy as sa +from advanced_alchemy.filters import LimitOffset +from advanced_alchemy.repository import SQLAlchemyAsyncRepository +from advanced_alchemy.service import SQLAlchemyAsyncRepositoryService + +from app.database import tables + + +class MessagesRepository(SQLAlchemyAsyncRepositoryService[tables.MessagesTable]): + class BaseRepository(SQLAlchemyAsyncRepository[tables.MessagesTable]): + model_type = tables.MessagesTable + + repository_type = BaseRepository + + async def fetch_by_idempotency_key(self, chat_id: int, idempotency_key: uuid.UUID) -> tables.MessagesTable | None: + return await self.get_one_or_none(chat_id=chat_id, idempotency_key=idempotency_key) + + async def fetch_latest_active(self, chat_id: int) -> tables.MessagesTable | None: + """Return the newest non-deleted message in a chat, or None if none remains. + + Used to repoint ChatsTable.last_message_id after a delete removes the current pointer. + """ + messages = await self.get_many( + tables.MessagesTable.chat_id == chat_id, + tables.MessagesTable.deleted_at.is_(None), + LimitOffset(limit=1, offset=0), + order_by=[("id", True)], + ) + return messages[0] if messages else None + + async def list_page( + self, + chat_id: int, + *, + before_id: int | None, + after_id: int | None, + limit: int, + ) -> Sequence[tables.MessagesTable]: + filters: typing.Final[list[sa.ColumnElement[bool]]] = [ + tables.MessagesTable.chat_id == chat_id, + tables.MessagesTable.deleted_at.is_(None), + ] + if after_id is not None: + filters.append(tables.MessagesTable.id > after_id) + return await self.get_many(*filters, LimitOffset(limit=limit, offset=0), order_by=[("id", False)]) + if before_id is not None: + filters.append(tables.MessagesTable.id < before_id) + return await self.get_many(*filters, LimitOffset(limit=limit, offset=0), order_by=[("id", True)]) diff --git a/app/repositories/users_repository.py b/app/repositories/users_repository.py new file mode 100644 index 0000000..b46b989 --- /dev/null +++ b/app/repositories/users_repository.py @@ -0,0 +1,11 @@ +from advanced_alchemy.repository import SQLAlchemyAsyncRepository +from advanced_alchemy.service import SQLAlchemyAsyncRepositoryService + +from app.database import tables + + +class UsersRepository(SQLAlchemyAsyncRepositoryService[tables.UsersTable]): + class BaseRepository(SQLAlchemyAsyncRepository[tables.UsersTable]): + model_type = tables.UsersTable + + repository_type = BaseRepository diff --git a/app/schemas/__init__.py b/app/schemas/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/app/schemas/api.py b/app/schemas/api.py new file mode 100644 index 0000000..bb1f702 --- /dev/null +++ b/app/schemas/api.py @@ -0,0 +1,112 @@ +import datetime +import uuid +from collections.abc import Iterable +from typing import Any, Self + +import pydantic +from pydantic import BaseModel, PositiveInt + +from app.database import tables + + +class Base(BaseModel): + model_config = pydantic.ConfigDict(from_attributes=True) + + +class Collection[T: Base](Base): + items: list[T] + + @classmethod + def from_models(cls, objects: Iterable[Any]) -> Self: + return cls.model_validate({"items": list(objects)}) + + +class RegisterRequest(Base): + username: str = pydantic.Field(min_length=3, max_length=64) + password: str = pydantic.Field(min_length=8, max_length=128) + display_name: str = pydantic.Field(min_length=1, max_length=128) + + +class LoginRequest(Base): + username: str + password: str + + +class User(Base): + id: PositiveInt + username: str + display_name: str + + +class CreateChatRequest(Base): + chat_type: tables.ChatType + member_ids: list[PositiveInt] = pydantic.Field(min_length=1) + title: str | None = pydantic.Field(default=None, max_length=128) + + +class ChatMember(Base): + user_id: PositiveInt + last_read_message_id: PositiveInt | None = None + + +class MarkReadRequest(Base): + last_read_message_id: PositiveInt + + +class Chat(Base): + id: PositiveInt + chat_type: tables.ChatType + title: str | None = None + created_by_id: PositiveInt + + +class ChatDetail(Chat): + members: list[ChatMember] + + +class SendMessageRequest(Base): + idempotency_key: uuid.UUID + text: str = pydantic.Field(min_length=1, max_length=4000) + + +class EditMessageRequest(Base): + text: str = pydantic.Field(min_length=1, max_length=4000) + + +class Message(Base): + id: PositiveInt + chat_id: PositiveInt + user_id: PositiveInt | None = None + text: str + created_at: datetime.datetime + edited_at: datetime.datetime | None = None + + +class Messages(Collection[Message]): + pass + + +class ChatListItem(Chat): + 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/security.py b/app/security.py new file mode 100644 index 0000000..3d5638c --- /dev/null +++ b/app/security.py @@ -0,0 +1,18 @@ +import typing + +import argon2 +from argon2.exceptions import Argon2Error + + +_HASHER: typing.Final = argon2.PasswordHasher() + + +def hash_password(password: str) -> str: + return _HASHER.hash(password) + + +def verify_password(password_hash: str, password: str) -> bool: + try: + return _HASHER.verify(password_hash, password) + except Argon2Error, argon2.exceptions.InvalidHashError: + return False diff --git a/app/settings.py b/app/settings.py new file mode 100644 index 0000000..3eb2d28 --- /dev/null +++ b/app/settings.py @@ -0,0 +1,81 @@ +import typing + +import pydantic_settings +from lite_bootstrap import LitestarConfig +from sqlalchemy.engine.url import URL, make_url + + +# >= 32 bytes: PyJWT warns (InsecureKeyLengthWarning) below that for HS256. +INSECURE_JWT_SECRET: typing.Final = "insecure-local-secret-do-not-use-in-prod" + + +class Settings(pydantic_settings.BaseSettings): + service_name: str = "chat-app" + service_version: str = "1.0.0" + service_environment: str = "local" + # Enabling this turns on SQLAlchemy's echo/echo_pool (app/database/resources.py), which logs + # every statement WITH ITS BOUND PARAMETERS - including argon2 password_hash values on every + # registration - and makes Litestar return stack traces in responses. Never set True outside + # a throwaway local session. + service_debug: bool = False + log_level: str = "info" + + db_dsn: str = "postgresql+asyncpg://postgres:password@db/postgres" + db_pool_size: int = 5 + db_max_overflow: int = 0 + db_pool_pre_ping: bool = True + + app_host: str = "0.0.0.0" # noqa: S104 + app_port: int = 8000 + + jwt_secret: str = INSECURE_JWT_SECRET + jwt_lifetime_seconds: int = 60 * 60 * 24 * 7 + # Litestar leaves this unset by default. Production MUST set this to True (it requires + # serving over HTTPS); left False here so local http:// development still gets the cookie. + jwt_cookie_secure: bool = False + + opentelemetry_endpoint: str = "" + sentry_dsn: str = "" + logging_buffer_capacity: int = 0 + swagger_offline_docs: bool = True + + cors_allowed_origins: list[str] = [] + cors_allowed_methods: list[str] = ["*"] + cors_allowed_headers: list[str] = ["*"] + cors_exposed_headers: list[str] = [] + + request_max_body_size: int = 1024 * 1024 + + def ensure_jwt_secret_is_configured(self) -> None: + # The whole auth boundary is a token signed with jwt_secret: with the shipped default, + # anyone can forge a token for any user.id. Only "local" may run with it. + if self.service_environment != "local" and self.jwt_secret == INSECURE_JWT_SECRET: + message = ( + f"jwt_secret is still the insecure default while service_environment=" + f"{self.service_environment!r}; set the JWT_SECRET environment variable." + ) + raise RuntimeError(message) + + @property + def db_dsn_parsed(self) -> URL: + return make_url(self.db_dsn) + + @property + def api_bootstrapper_config(self) -> LitestarConfig: + return LitestarConfig( + service_name=self.service_name, + service_version=self.service_version, + service_environment=self.service_environment, + service_debug=self.service_debug, + opentelemetry_endpoint=self.opentelemetry_endpoint, + sentry_dsn=self.sentry_dsn, + cors_allowed_origins=self.cors_allowed_origins, + cors_allowed_methods=self.cors_allowed_methods, + cors_allowed_headers=self.cors_allowed_headers, + cors_exposed_headers=self.cors_exposed_headers, + logging_buffer_capacity=self.logging_buffer_capacity, + swagger_offline_docs=self.swagger_offline_docs, + ) + + +settings = Settings() diff --git a/app/use_cases/__init__.py b/app/use_cases/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/app/use_cases/authenticate_user.py b/app/use_cases/authenticate_user.py new file mode 100644 index 0000000..3cc26f0 --- /dev/null +++ b/app/use_cases/authenticate_user.py @@ -0,0 +1,25 @@ +import dataclasses +import typing + +from db_retry import postgres_retry + +from app import security +from app.database import tables +from app.repositories.users_repository import UsersRepository + + +@dataclasses.dataclass(kw_only=True, frozen=True, slots=True) +class AuthenticateUserUseCase: + users_repository: UsersRepository + + @postgres_retry + 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 + # response measurably faster and turns login into a username oracle. + security.hash_password(password) + return None + if not security.verify_password(user.password_hash, password): + return None + return user diff --git a/app/use_cases/create_chat.py b/app/use_cases/create_chat.py new file mode 100644 index 0000000..1628d5f --- /dev/null +++ b/app/use_cases/create_chat.py @@ -0,0 +1,85 @@ +import dataclasses +import typing + +from advanced_alchemy.exceptions import DuplicateKeyError +from db_retry import Transaction, postgres_retry + +from app.database import tables +from app.exceptions import ValidationError +from app.repositories.chat_members_repository import ChatMembersRepository +from app.repositories.chats_repository import ChatsRepository +from app.schemas.api import CreateChatRequest + + +_DIRECT_CHAT_MEMBER_COUNT: typing.Final = 2 + + +@dataclasses.dataclass(kw_only=True, frozen=True, slots=True) +class CreateChatUseCase: + transaction: Transaction + chats_repository: ChatsRepository + chat_members_repository: ChatMembersRepository + + @postgres_retry + 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 + + if data.chat_type is tables.ChatType.DIRECT: + if len(member_ids) != _DIRECT_CHAT_MEMBER_COUNT: + msg = "A direct chat must have exactly two distinct members" + raise ValidationError(msg) + low, high = sorted(member_ids) + direct_key = tables.build_direct_key(low, high) + + # Kept outside the `async with` block below: Transaction.__aexit__ unconditionally + # rolls back and closes the session whenever it is left with an open, uncommitted + # transaction (session.in_transaction() True with no prior commit()), which expires + # and detaches every loaded attribute. A `return` from inside the block after this + # read - with no commit following it - would trigger exactly that on `existing`. + # (For direct chats specifically, this SELECT autobegins the session's transaction, + # so the `async with self.transaction:` block below actually *joins* that same + # transaction rather than starting a new one - see Transaction.__aenter__. That does + # not change the __aexit__ hazard above; it only means direct and group chats reach + # the block by different routes.) + existing = await self.chats_repository.fetch_direct_by_key(direct_key) + if existing is not None: + return existing, False + + chat: tables.ChatsTable | None = None + async with self.transaction: + try: + chat = await self.chats_repository.create( + tables.ChatsTable( + chat_type=data.chat_type, + title=data.title if data.chat_type is tables.ChatType.GROUP else None, + created_by_id=actor.id, + direct_key=direct_key, + ) + ) + except DuplicateKeyError: + # Two concurrent requests to open the same direct chat both passed the + # fetch_direct_by_key pre-check above and both tried to insert; the loser hits + # uq_chats_direct_key here. Group chats have no unique constraint on `chats` to + # violate, so an unexpected DuplicateKeyError there re-raises and maps to the + # standard 409 instead of being funnelled into this direct-chat recovery path. + if direct_key is None: + raise + # Roll back and re-read the winner's row outside this block (same __aexit__ + # hazard as the comment above): a `return` from in here without a commit + # first would detach whatever `fetch_direct_by_key` loaded, same as before. + await self.transaction.rollback() + else: + for user_id in sorted(member_ids): + await self.chat_members_repository.create(tables.ChatMembersTable(chat_id=chat.id, user_id=user_id)) + await self.transaction.commit() + + if chat is None: + existing = await self.chats_repository.fetch_direct_by_key(direct_key) # ty: ignore[invalid-argument-type] + if existing is None: + msg = "Direct chat creation raced but the resulting row could not be found" + raise RuntimeError(msg) + return existing, False + + # Kept outside the `async with` block for the same reason as the lookup above. + return await self.chats_repository.fetch_with_members(chat.id), True diff --git a/app/use_cases/create_message.py b/app/use_cases/create_message.py new file mode 100644 index 0000000..9fbb00b --- /dev/null +++ b/app/use_cases/create_message.py @@ -0,0 +1,67 @@ +import dataclasses +import typing + +from advanced_alchemy.exceptions import DuplicateKeyError +from db_retry import Transaction, postgres_retry + +from app.database import tables +from app.exceptions import PermissionDeniedError +from app.repositories.chat_members_repository import ChatMembersRepository +from app.repositories.chats_repository import ChatsRepository +from app.repositories.messages_repository import MessagesRepository +from app.schemas.api import SendMessageRequest + + +@dataclasses.dataclass(kw_only=True, frozen=True, slots=True) +class CreateMessageUseCase: + transaction: Transaction + chats_repository: ChatsRepository + chat_members_repository: ChatMembersRepository + messages_repository: MessagesRepository + + @postgres_retry + async def __call__( + 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" + raise PermissionDeniedError(msg) + + existing: typing.Final = await self.messages_repository.fetch_by_idempotency_key(chat_id, data.idempotency_key) + if existing is not None: + return existing, False + + message: tables.MessagesTable | None = None + async with self.transaction: + try: + message = await self.messages_repository.create( + tables.MessagesTable( + chat_id=chat_id, + user_id=actor.id, + idempotency_key=data.idempotency_key, + text=data.text, + ) + ) + except DuplicateKeyError: + # Two concurrent retries of the same (chat_id, key); the loser reads the winner's + # row. Roll back and re-read outside this block (Transaction.__aexit__ + # unconditionally rolls back and closes the session on an open, uncommitted + # transaction, which expires every loaded attribute - a `return` from in here + # would detach whatever fetch_by_idempotency_key loaded). + await self.transaction.rollback() + else: + await self.chats_repository.update( + tables.ChatsTable(id=chat_id, last_message_id=message.id), + item_id=chat_id, + attribute_names=["last_message_id"], + ) + await self.transaction.commit() + + if message is None: + duplicate = await self.messages_repository.fetch_by_idempotency_key(chat_id, data.idempotency_key) + if duplicate is None: + msg = "Message send raced but the resulting row could not be found" + raise RuntimeError(msg) + return duplicate, False + + return message, True diff --git a/app/use_cases/delete_message.py b/app/use_cases/delete_message.py new file mode 100644 index 0000000..c9dc75b --- /dev/null +++ b/app/use_cases/delete_message.py @@ -0,0 +1,51 @@ +import dataclasses +import datetime + +from db_retry import Transaction, postgres_retry + +from app.database import tables +from app.repositories.chat_members_repository import ChatMembersRepository +from app.repositories.chats_repository import ChatsRepository +from app.repositories.messages_repository import MessagesRepository +from app.use_cases.message_authorization import fetch_message_for_author + + +@dataclasses.dataclass(kw_only=True, frozen=True, slots=True) +class DeleteMessageUseCase: + transaction: Transaction + messages_repository: MessagesRepository + chat_members_repository: ChatMembersRepository + chats_repository: ChatsRepository + + @postgres_retry + 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, + chat_members_repository=self.chat_members_repository, + actor=actor, + message_id=message_id, + action="delete", + ) + if message.deleted_at is not None: + # DELETE is idempotent under HTTP semantics: a second delete of an + # already-deleted message is not an error, unlike PATCH via EditMessageUseCase. + return + message.deleted_at = datetime.datetime.now(tz=datetime.UTC) + await self.messages_repository.update(message, item_id=message_id) + + chat = await self.chats_repository.get_one(id=message.chat_id) + if chat.last_message_id == message_id: + # chats.last_message_id means "the newest non-deleted message in this chat" - the + # chat listing's preview and its ordering both read this column, so repointing it + # here (in the same commit as the soft delete) is what keeps a delete from being + # only half-effective: leaving it pointed at a deleted message would make the + # listing preview show deleted text and rank the chat by a message that no longer + # counts. + newest = await self.messages_repository.fetch_latest_active(message.chat_id) + await self.chats_repository.update( + tables.ChatsTable(id=message.chat_id, last_message_id=newest.id if newest is not None else None), + item_id=message.chat_id, + attribute_names=["last_message_id"], + ) + await self.transaction.commit() diff --git a/app/use_cases/edit_message.py b/app/use_cases/edit_message.py new file mode 100644 index 0000000..4949230 --- /dev/null +++ b/app/use_cases/edit_message.py @@ -0,0 +1,43 @@ +import dataclasses +import datetime + +from db_retry import Transaction, postgres_retry + +from app.database import tables +from app.exceptions import ConflictError +from app.repositories.chat_members_repository import ChatMembersRepository +from app.repositories.messages_repository import MessagesRepository +from app.schemas.api import EditMessageRequest +from app.use_cases.message_authorization import fetch_message_for_author + + +@dataclasses.dataclass(kw_only=True, frozen=True, slots=True) +class EditMessageUseCase: + transaction: Transaction + messages_repository: MessagesRepository + chat_members_repository: ChatMembersRepository + + @postgres_retry + async def __call__( + self, actor: tables.UsersTable, message_id: int, data: EditMessageRequest + ) -> tables.MessagesTable: + async with self.transaction: + message = await fetch_message_for_author( + messages_repository=self.messages_repository, + chat_members_repository=self.chat_members_repository, + actor=actor, + message_id=message_id, + action="edit", + ) + if message.deleted_at is not None: + msg = "This message has been deleted" + raise ConflictError(msg) + message.text = data.text + message.edited_at = datetime.datetime.now(tz=datetime.UTC) + updated = await self.messages_repository.update(message, item_id=message_id) + await self.transaction.commit() + # Returned from inside the block, right after commit(): __aexit__ then sees no open + # transaction (commit ended it) and only closes the session - it does not roll back, + # so `updated`'s already-loaded attributes (no relationships here to eager-load) stay + # usable for the caller. + return updated diff --git a/app/use_cases/fetch_chat.py b/app/use_cases/fetch_chat.py new file mode 100644 index 0000000..84ebfbb --- /dev/null +++ b/app/use_cases/fetch_chat.py @@ -0,0 +1,21 @@ +import dataclasses + +from db_retry import postgres_retry + +from app.database import tables +from app.exceptions import PermissionDeniedError +from app.repositories.chat_members_repository import ChatMembersRepository +from app.repositories.chats_repository import ChatsRepository + + +@dataclasses.dataclass(kw_only=True, frozen=True, slots=True) +class FetchChatUseCase: + chats_repository: ChatsRepository + chat_members_repository: ChatMembersRepository + + @postgres_retry + 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) + return await self.chats_repository.fetch_with_members(chat_id) diff --git a/app/use_cases/fetch_chats.py b/app/use_cases/fetch_chats.py new file mode 100644 index 0000000..b2b191f --- /dev/null +++ b/app/use_cases/fetch_chats.py @@ -0,0 +1,48 @@ +import dataclasses +import typing + +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 + ] diff --git a/app/use_cases/fetch_messages.py b/app/use_cases/fetch_messages.py new file mode 100644 index 0000000..0581901 --- /dev/null +++ b/app/use_cases/fetch_messages.py @@ -0,0 +1,42 @@ +import dataclasses +import typing +from collections.abc import Sequence + +from db_retry import postgres_retry + +from app.database import tables +from app.exceptions import PermissionDeniedError, ValidationError +from app.repositories.chat_members_repository import ChatMembersRepository +from app.repositories.messages_repository import MessagesRepository + + +MAX_PAGE_SIZE: typing.Final = 100 + + +@dataclasses.dataclass(kw_only=True, frozen=True, slots=True) +class FetchMessagesUseCase: + chat_members_repository: ChatMembersRepository + messages_repository: MessagesRepository + + @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, + ) -> Sequence[tables.MessagesTable]: + if not await self.chat_members_repository.is_member(chat_id, actor.id): + msg = "Not a member of this chat" + raise PermissionDeniedError(msg) + if before_id is not None and after_id is not None: + msg = "before_id and after_id are mutually exclusive" + raise ValidationError(msg) + if limit < 1: + msg = "limit must be at least 1" + raise ValidationError(msg) + return await self.messages_repository.list_page( + chat_id, before_id=before_id, after_id=after_id, limit=min(limit, MAX_PAGE_SIZE) + ) diff --git a/app/use_cases/mark_read.py b/app/use_cases/mark_read.py new file mode 100644 index 0000000..cb1f4ff --- /dev/null +++ b/app/use_cases/mark_read.py @@ -0,0 +1,45 @@ +import dataclasses +import typing + +from db_retry import Transaction, postgres_retry + +from app.database import tables +from app.exceptions import PermissionDeniedError, ValidationError +from app.repositories.chat_members_repository import ChatMembersRepository +from app.repositories.messages_repository import MessagesRepository +from app.schemas.api import MarkReadRequest + + +@dataclasses.dataclass(kw_only=True, frozen=True, slots=True) +class MarkReadUseCase: + transaction: Transaction + chat_members_repository: ChatMembersRepository + messages_repository: MessagesRepository + + @postgres_retry + 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" + raise PermissionDeniedError(msg) + + # The requested marker must name a real message in *this* chat - otherwise a client could + # set it to an arbitrary large id and permanently zero its own unread count. + target_message = await self.messages_repository.get_one_or_none(id=data.last_read_message_id, chat_id=chat_id) + if target_message is None: + msg = "last_read_message_id does not name a message in this chat" + raise ValidationError(msg) + + async with self.transaction: + # Monotonic: an out-of-order or replayed request naming an earlier message must not + # move the marker backwards and resurrect messages that were already marked read. + # The GREATEST(...) that enforces this lives in the UPDATE itself (see + # ChatMembersRepository.mark_read) rather than being computed here from `member`'s + # already-read value - that would be a read-modify-write race between concurrent + # POST /read/ calls. + updated = await self.chat_members_repository.mark_read(member.id, data.last_read_message_id) + await self.transaction.commit() + # Returned from inside the block, right after commit(): __aexit__ then sees no open + # transaction (commit ended it) and only closes the session - it does not roll back, + # so `updated`'s already-loaded attributes stay usable for the caller. + return updated diff --git a/app/use_cases/message_authorization.py b/app/use_cases/message_authorization.py new file mode 100644 index 0000000..5039d45 --- /dev/null +++ b/app/use_cases/message_authorization.py @@ -0,0 +1,34 @@ +import typing + +from app.database import tables +from app.exceptions import PermissionDeniedError +from app.repositories.chat_members_repository import ChatMembersRepository +from app.repositories.messages_repository import MessagesRepository + + +async def fetch_message_for_author( + *, + messages_repository: MessagesRepository, + chat_members_repository: ChatMembersRepository, + actor: tables.UsersTable, + message_id: int, + action: str, +) -> tables.MessagesTable: + """Look up a message and authorize `actor` to act on it as its author. + + Shared by EditMessageUseCase and DeleteMessageUseCase so the check ordering is defined in + exactly one place: existence (get_one raises NotFoundError -> 404), then chat membership + (-> 403), then authorship (-> 403). Membership is checked even though a non-author is + already refused by the authorship check below - mirroring FetchMessagesUseCase's and + FetchChatUseCase's membership-first posture keeps this consistent with the rest of the + codebase rather than leaving message mutation as the one actor-scoped use case that never + reconfirms the actor still belongs to the chat. + """ + message: typing.Final = await messages_repository.get_one(id=message_id) + if not await chat_members_repository.is_member(message.chat_id, actor.id): + msg = "Not a member of this chat" + raise PermissionDeniedError(msg) + if message.user_id != actor.id: + msg = f"Only the author may {action} this message" + raise PermissionDeniedError(msg) + return message diff --git a/app/use_cases/register_user.py b/app/use_cases/register_user.py new file mode 100644 index 0000000..e87584a --- /dev/null +++ b/app/use_cases/register_user.py @@ -0,0 +1,28 @@ +import dataclasses +import typing + +from db_retry import Transaction, postgres_retry + +from app import security +from app.database import tables +from app.repositories.users_repository import UsersRepository +from app.schemas.api import RegisterRequest + + +@dataclasses.dataclass(kw_only=True, frozen=True, slots=True) +class RegisterUserUseCase: + transaction: Transaction + users_repository: UsersRepository + + @postgres_retry + async def __call__(self, data: RegisterRequest) -> tables.UsersTable: + async with self.transaction: + user: typing.Final = await self.users_repository.create( + tables.UsersTable( + username=data.username, + password_hash=security.hash_password(data.password), + display_name=data.display_name, + ) + ) + await self.transaction.commit() + return user diff --git a/architecture/README.md b/architecture/README.md new file mode 100644 index 0000000..28b8c74 --- /dev/null +++ b/architecture/README.md @@ -0,0 +1,24 @@ +# Architecture + +The living truth about what `chat-app` does **now** — one file per capability, +updated by hand whenever a change ships. The *why* and *how it got here* live +in [`../planning/changes/`](../planning/changes/), and decisions deliberately +taken (including options rejected) in +[`../planning/decisions/`](../planning/decisions/); this directory is the +present. + +These files carry **no frontmatter** — they are prose, dated by git. + +## Capabilities + +- [auth.md](auth.md) — registration, login, the JWT cookie, `retrieve_user_handler`. +- [chats.md](chats.md) — direct/group chats, the direct-chat upsert, membership. +- [messages.md](messages.md) — idempotent send, cursor pagination, edit/delete authorization, unread counts. +- [testing.md](testing.md) — the per-test rollback fixture, DI-fixture exposure, the race-simulation pattern. +- [glossary.md](glossary.md) — the domain's ubiquitous language. + +## Promotion rule + +Shipping a change hand-edits the affected capability file(s) here to match the +new reality, in the same PR as the code. The change file stays in place under +[`../planning/changes/`](../planning/changes/) — no folder move. diff --git a/architecture/auth.md b/architecture/auth.md new file mode 100644 index 0000000..4c8bd0c --- /dev/null +++ b/architecture/auth.md @@ -0,0 +1,83 @@ +# Auth + +Litestar's `JWTCookieAuth[UsersTable]` (`app/api/auth.py`), configured with +`token_secret=settings.jwt_secret` and a 7-day default expiration +(`jwt_lifetime_seconds`). Cookie rather than bearer header: a browser +`EventSource` (planned for the realtime follow-on) cannot set an +`Authorization` header, so the cookie is the one auth variant every endpoint — +REST today, SSE later — can share identically. + +## Registration and login + +`POST /api/auth/register/` (`app/api/endpoints/auth.py::register`) runs +`RegisterUserUseCase`, which hashes the password with `argon2` (`app/security.py`) +inside its own transaction and returns `201` with the cookie set via +`jwt_cookie_auth.login`. A duplicate username raises `DuplicateKeyError` from +the unique constraint on `users.username`, mapped to `409` by the app-wide +handler — there is no auth-specific duplicate check. + +`POST /api/auth/login/` runs `AuthenticateUserUseCase`, which looks the user up +by username and verifies the password hash. On failure — unknown username or +wrong password — it raises Litestar's own `NotAuthorizedException` (`401`), +not `app.exceptions.PermissionDeniedError`: this is the one place the +`litestar.exceptions` vocabulary is used directly, because login failure is +not an authorization decision to gate downstream of an already-identified +actor, it *is* the identification step. On success it returns `200` (not +`201` — nothing was created) with a fresh cookie. + +`AuthenticateUserUseCase` hashes the submitted password even when the +username doesn't exist (`app/use_cases/authenticate_user.py`) specifically so +an unknown-username response isn't measurably faster than a +wrong-password response — skipping the argon2 work would turn login into a +username oracle. + +`POST /api/auth/logout/` deletes the cookie and returns `204`. It does not +revoke the JWT: a token copied before logout stays valid for the rest of its +lifetime, because no `revoked_token_handler` is configured on +`jwt_cookie_auth`. See `planning/deferred.md`. + +Both `register` and `login` opt out of the auth middleware with +`exclude_from_auth=True` on the handler, not through `jwt_cookie_auth`'s +`exclude` list — that list is reserved for path-shaped exclusions, each +anchored with `^` so a future route merely containing `/docs` as a path +segment isn't accidentally deauthenticated. + +The anonymous surface is therefore exactly four prefixes: `/docs` and +`/health`, plus `/static` (Swagger's offline assets, served from there +because `swagger_offline_docs` is on — without the exclusion the docs page +loads but every asset request 401s) and `/metrics` (a Prometheus scrape +target must be reachable without a session cookie; it carries process and +request metrics, no user data). + +## Request-time identity + +`retrieve_user_handler` (`app/api/auth.py`) runs inside Litestar's auth +middleware, which executes *before* request-scoped DI is available. It cannot +resolve a use case or repository, so it resolves the app-scoped +`Database.database_engine` provider directly off the DI container +(`modern_di_litestar.fetch_di_container(connection.app)`) and opens its own +short-lived session through the same `database_resources.create_session` +factory the container uses, then closes it in a `finally`. This means every +authenticated request opens **two** sessions — one here, one for the +request-scoped repositories — against a pool sized `db_pool_size=5`, +`db_max_overflow=0`. See `planning/deferred.md`. + +`Token.sub` is only guaranteed to be a non-empty string; `retrieve_user_handler` +converts it with `int(token.sub)` and returns `None` (→ `401` via the +middleware) on `ValueError` rather than letting a forged or malformed subject +crash the request. A validly signed token whose subject names a user that no +longer exists resolves to `None` from `session.get` the same way. + +`GET /api/auth/me/` returns the authenticated `request.user` — no separate use +case, since the middleware has already loaded it. + +## Configuration + +`jwt_cookie_secure` (`app/settings.py`) defaults `False` so local `http://` +development still receives the cookie; it must be `True` in any deployment +served over HTTPS. `Settings.ensure_jwt_secret_is_configured`, called at the +top of `build_app`, raises `RuntimeError` at startup if +`service_environment != "local"` and `jwt_secret` is still the shipped +`INSECURE_JWT_SECRET` — the whole auth boundary is a token signed with that +secret, so running any non-local environment on the default would let anyone +forge a token for any `user.id`. diff --git a/architecture/chats.md b/architecture/chats.md new file mode 100644 index 0000000..de55d0b --- /dev/null +++ b/architecture/chats.md @@ -0,0 +1,90 @@ +# Chats + +## Shape + +`ChatsTable` (`app/database/tables.py`): `chat_type`, optional `title` +(group chats only — `CreateChatUseCase` forces it to `None` for direct chats +even if the request supplied one), `created_by_id`, `last_message_id` +(nullable, repointed by message send/delete — see `messages.md`), and +`direct_key` (nullable, unique). `chat_type` is stored as `sa.Enum(ChatType, +native_enum=False, create_constraint=True, values_callable=...)` — a `VARCHAR` +plus a `CHECK` constraint storing the lowercase string values (`"direct"`, +`"group"`), not a native Postgres enum type. A native enum would need +`alembic-postgresql-enum` for autogenerate to emit correct `ALTER TYPE` +migrations, a dependency not worth buying to store two values. See +`planning/decisions/2026-08-21-sequence-ids-not-snowflakes.md` for the +adjacent id-strategy call. + +`ChatMembersTable` is `(chat_id, user_id)` under `uk_chat_members_chat_id_user_id`, +plus `last_read_message_id` and `joined_at`. + +## Creating a chat + +`POST /api/chats/` → `CreateChatUseCase` (`app/use_cases/create_chat.py`). +`member_ids` from the request is unioned with the actor's own id, so the +creator is always a member even if they omitted themselves. + +**Direct** (`chat_type = "direct"`) requires the union to resolve to exactly +two distinct users (`ValidationError` → `400` otherwise), builds +`direct_key = build_direct_key(low, high)`, and checks +`fetch_direct_by_key` first: if a direct chat for this pair already exists, +it's returned as-is with `created=False` (→ `200`). This pre-check does not +close the race — two concurrent requests can both miss it before either +commits. The `INSERT` itself is the real guard: it hits `uq_chats_direct_key`, +and the loser catches `DuplicateKeyError`, rolls back, and re-reads +`fetch_direct_by_key` to return the winner's row. **Group** chats have no +unique constraint on `chats` to collide on, so an unexpected +`DuplicateKeyError` from a group-chat insert is not funnelled into this +recovery path — it re-raises and maps to the standard `409`. + +The rollback-then-reread shape (here and in `CreateMessageUseCase`, see +`messages.md`) exists because `Transaction.__aexit__` unconditionally rolls +back and closes the session on an open, uncommitted transaction, which expires +every loaded attribute — returning the just-loaded row from *inside* the +`async with self.transaction:` block without a preceding `commit()` would hand +the caller a detached object. + +## Membership and 403-vs-404 + +Every chat- and message-scoped use case checks membership before doing +anything else (`chat_members_repository.is_member` / +`fetch_member`), and a non-member gets `PermissionDeniedError` → `403` — not +`404`. `FetchChatUseCase` (`app/use_cases/fetch_chat.py`) deliberately returns +`403` for a chat that exists but that the actor isn't in, rather than `404` +pretending it doesn't exist; other use cases follow the same posture for +consistency. One accepted consequence: a non-member can distinguish an +existing message id from a nonexistent one via `404` vs `403` on +`PATCH`/`DELETE /api/messages/{id}/` (see `messages.md` and +`planning/deferred.md`). + +## 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 +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 +ordered by `COALESCE(last_message_id, 0) DESC` so the most recently active +chat sorts first (a chat with no messages yet sorts last, not first). `IS +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 +`last_message_id` off a deleted message in the same commit as the delete (see +`messages.md`). + +## Marking read + +`POST /api/chats/{id}/read/` → `MarkReadUseCase` (`app/use_cases/mark_read.py`). +The requested `last_read_message_id` must name a real message in *this* chat +— `ValidationError` → `400` otherwise, since accepting an arbitrary id would +let a client zero its own unread count by naming a message from another chat +or one that doesn't exist. The marker only ever advances: `ChatMembersRepository.mark_read` +computes `GREATEST(COALESCE(current, 0), requested)` inside the `UPDATE` +itself rather than in Python from a prior read, so two concurrent `POST +/read/` calls can't race a read-modify-write and let the lower id win — the +row lock on the `UPDATE` serializes them. diff --git a/architecture/glossary.md b/architecture/glossary.md new file mode 100644 index 0000000..ce5ccd0 --- /dev/null +++ b/architecture/glossary.md @@ -0,0 +1,62 @@ +# Glossary + +The project's ubiquitous language — the domain terms that code, specs, and +capability pages share. Living prose, no frontmatter, dated by git. Each entry +is a term, what it *is* (not what it does), and the synonyms to avoid. + +**Chat**: +A row in `chats`: a `chat_type` (`direct` or `group`), an optional `title`, +the `id` of the user who created it, and a pointer (`last_message_id`) to its +newest non-deleted message. Owns a set of `Member` rows through `chat_members`. +_Avoid_: conversation, room, thread + +**Direct chat**: +A `Chat` with `chat_type = "direct"` between exactly two users, identified by +`direct_key` — the canonical `min(user_id):max(user_id)` string under a unique +constraint. Opening a direct chat with the same pair twice returns the same +row; the key is what makes that an upsert instead of a read-then-race. A +`group` chat has no `direct_key` and no member-count ceiling. +_Avoid_: DM, 1:1 + +**Member**: +A row in `chat_members`: the `(chat_id, user_id)` pair that grants access to a +`Chat`, plus that user's `last_read_message_id`. Membership is what +`is_member`/`fetch_member` check before any read or write on a chat is +authorized; it is necessary but, for editing or deleting a message, not +sufficient — see `Read marker`. +_Avoid_: participant, subscriber + +**Idempotency key**: +The client-supplied `idempotency_key` (a UUID) on a send-message request, +unique per `(chat_id, idempotency_key)` — scoped to one chat, not global, +because the key identifies a retry of "send this message to this chat," and +the same key reused in a different chat is a second, independent send. A +repeated key returns the first send's row with `200` instead of creating a +second one with `201`. +_Avoid_: dedupe key, request id + +**Unread**: +A message counted by `chats_repository.list_for_user`'s correlated subquery: +`id > member.last_read_message_id` (treating `NULL` as `0`), not authored by +the viewing member (`user_id IS DISTINCT FROM`, so system messages with +`user_id IS NULL` still count), and not soft-deleted. There is no per-message +receipt row — unread is a count computed at read time against one marker per +member, not a set of rows written per message per recipient. +_Avoid_: unseen, badge count + +**Cursor**: +A message `id` passed as `before_id` or `after_id` to page `GET +.../messages/`. `before_id` returns older messages, newest-first, excluding +the cursor row; `after_id` returns newer messages, oldest-first, excluding the +cursor row. The two are mutually exclusive on one request. Message ids are a +Postgres identity sequence, so "greater id" is a total order a cursor can walk +without an offset. +_Avoid_: page token, offset + +**Read marker**: +A member's `last_read_message_id` — the highest message id that member has +acknowledged reading in that chat. Advanced only forward: `mark_read` sets it +to `GREATEST(current, requested)` inside the UPDATE itself, so an out-of-order +or replayed request naming an earlier message can never move it backwards and +resurrect messages that were already read. +_Avoid_: read receipt, watermark diff --git a/architecture/messages.md b/architecture/messages.md new file mode 100644 index 0000000..7231d7e --- /dev/null +++ b/architecture/messages.md @@ -0,0 +1,107 @@ +# Messages + +## Shape + +`MessagesTable` (`app/database/tables.py`): `chat_id`, nullable `user_id` +(`NULL` for system messages, e.g. "Bob joined" — no sentinel user), an +`idempotency_key` (UUID), `text`, `created_at`, nullable `edited_at` / +`deleted_at`. `ix_messages_chat_id_id` is a composite index on `(chat_id, id)` +— there is no standalone index on `chat_id` alone, because every query that +would use one (membership-scoped listing, cursor pagination) is already served +by the composite. `uk_messages_chat_id_idempotency_key` is a unique constraint +on `(chat_id, idempotency_key)`, **not** a global unique constraint on the key +alone — see `Idempotency key` in `glossary.md`. + +## Sending: idempotent with a concurrent-retry fallback + +`POST /api/chats/{id}/messages/` → `CreateMessageUseCase` +(`app/use_cases/create_message.py`). After the membership check, it pre-reads +`fetch_by_idempotency_key(chat_id, key)`; a hit returns that row with +`created=False` (→ `200`) without writing anything. A miss proceeds to +`INSERT`, then updates `chats.last_message_id` to the new message's id in the +same commit, and returns `created=True` (→ `201`). + +The pre-check does not close the race between two concurrent sends of the +same key: both can miss it before either commits. The unique constraint is +the real guard — the loser's `INSERT` raises `DuplicateKeyError`, which is +caught, the transaction is rolled back (`await self.transaction.rollback()`, +not a `return` from inside the `async with` block — see the same +`__aexit__`-detaches-loaded-attributes hazard documented in `chats.md`), and +the loser re-reads `fetch_by_idempotency_key` outside the block to return the +winner's row with `created=False`. If that re-read still finds nothing, it's +treated as impossible (`RuntimeError`, `# pragma: no cover`) — the unique +constraint that just fired guarantees a matching row exists. + +Reusing the same key in a different chat is a second, independent send: +idempotency is scoped `(chat_id, key)` because the key identifies a retry of +"send to this chat," not a retry across the table. + +## Pagination + +`GET /api/chats/{id}/messages/` → `FetchMessagesUseCase` +(`app/use_cases/fetch_messages.py`) → `MessagesRepository.list_page`. +`before_id` and `after_id` are mutually exclusive (`ValidationError` → `400` +if both are set). `before_id` returns strictly older messages, newest-first, +excluding the cursor row itself; `after_id` returns strictly newer messages, +oldest-first, excluding the cursor row — the ascending form exists for +resync-on-reconnect in the realtime follow-on and is why the composite index +is shaped the way it is. `limit` is clamped to `MAX_PAGE_SIZE = 100` (silently +capped, not rejected) but rejected outright below `1` (`ValidationError` → +`400`). All variants filter `deleted_at IS NULL` — a soft-deleted message +disappears from every listing on its next fetch rather than appearing as a +tombstone. + +## Edit and delete: author **and** member + +`PATCH /api/messages/{id}/` and `DELETE /api/messages/{id}/` are gated by +`fetch_message_for_author` (`app/use_cases/message_authorization.py`), shared +by `EditMessageUseCase` and `DeleteMessageUseCase` so the check order is +defined in exactly one place: existence (`get_one` → `NotFoundError` → `404`), +then chat membership (→ `403`), then authorship — `message.user_id != +actor.id` (→ `403`). Authorship alone is not sufficient: an author who has +been removed from the chat (membership deleted) can no longer edit or delete +their own message, because the membership check runs first and unconditionally +— this is the one state where authorship and membership disagree, and the +only state that proves the membership check does something the authorship +check doesn't already cover on its own. + +One accepted consequence of checking membership before authorship: a +non-member gets `403` for both an existing message and (via the `404` from +`get_one`) a nonexistent one, so the two are distinguishable by status code. +This mirrors the same `FetchChatUseCase` 403-vs-404 posture in `chats.md`, and +is recorded, not treated as a bug, in `planning/deferred.md`. + +`EditMessageUseCase` additionally rejects editing an already-deleted message +with `ConflictError` → `409` (the actor is authorized; the request conflicts +with the message's current state). `DeleteMessageUseCase` treats a second +delete of an already-deleted message as a no-op returning `204` — DELETE is +idempotent under HTTP semantics where PATCH is not. + +Deleting a chat's newest message repoints `chats.last_message_id` atomically, +in the same commit as the soft delete: `DeleteMessageUseCase` checks whether +`chat.last_message_id == message_id`, and if so looks up +`fetch_latest_active` (the next-newest non-deleted message, or `None` if none +remains) and writes that back. Without this, the chat listing's preview and +its activity ordering (`chats.md`) would both keep reading a deleted message +until something else happened to send a new one. + +## Error vocabulary + +Registered in `build_app` (`app/api/app.py`) via `app/api/exception_handlers.py`, +mapping `app/exceptions.py`'s domain hierarchy plus a few `advanced_alchemy` +exceptions: + +| Exception | Status | Meaning | +|---|---|---| +| `advanced_alchemy.exceptions.NotFoundError` | 404 | the resource doesn't exist | +| `app.exceptions.PermissionDeniedError` | 403 | authenticated, but not authorized for this action | +| `app.exceptions.ValidationError` | 400 | well-formed request, violates a domain invariant | +| `app.exceptions.ConflictError` | 409 | authorized, but conflicts with the resource's current state | +| `advanced_alchemy.exceptions.DuplicateKeyError` | 409 | unique-constraint violation not otherwise recovered | +| `advanced_alchemy.exceptions.ForeignKeyError` | 400 | a referenced id doesn't exist | + +Litestar's own `NotAuthorizedException` (401) is used exactly once, for a +failed login (`app/api/endpoints/auth.py::login`) — see `auth.md`. It is the +one place `app.exceptions` is deliberately not used, because a bad +credential is an identification failure, not a downstream authorization +decision on an already-identified actor. diff --git a/architecture/testing.md b/architecture/testing.md new file mode 100644 index 0000000..cf84631 --- /dev/null +++ b/architecture/testing.md @@ -0,0 +1,90 @@ +# Testing + +`just test` cycles the DB (`alembic downgrade base && alembic upgrade head`) +and runs `pytest` in Compose against a migrated Postgres, gated at +`--cov-fail-under=100` with zero warnings. + +## Per-test rollback via a container override + +`db_session` (`tests/conftest.py`) opens its own `AsyncConnection`, begins a +transaction on it, then calls +`di_container.override(ioc.Database.database_engine, connection)` — every +provider downstream of `Database.database_engine` in the DI graph (sessions, +repositories, use cases, and `retrieve_user_handler`'s own ad-hoc session in +`app/api/auth.py`) now resolves against that one connection instead of the +real pooled engine. `database_resources.create_session` sets +`join_transaction_mode="create_savepoint"`, so every session opened against +that connection — whether by a fixture or by a route handler mid-request — +nests inside the outer transaction as a savepoint rather than committing past +it. Teardown does `if connection.in_transaction(): await +transaction.rollback()`, which discards every write the test made. + +That `if` guard is fail-silent: it exists to tolerate tests that already +closed their own transaction, but if a session were ever able to commit the +*outer* transaction rather than nesting a savepoint under it, teardown would +skip the rollback without raising and the next test would see leaked state. +See `planning/deferred.md`. + +`di_container` (`tests/conftest.py`) itself comes from the already-built +`app` fixture (`modern_di_litestar.fetch_di_container(app)`), so `db_session` +overrides the same container instance production request handling resolves +providers from — a request-scoped child container built during a test +(`tests/use_cases/conftest.py::request_container`) inherits the override. + +`tests/test_main.py::test_db_session_insert_is_visible_within_test` and +`test_db_session_rolls_back_between_tests` are a paired proof of this +mechanism: the first inserts a user and commits (on the fixture's own +session, inside the savepoint), the second asserts the table is empty. The +pair only proves rollback if pytest runs them in file order — running the +second alone (`-k test_db_session_rolls_back_between_tests`) passes +vacuously, since an empty table before any insert looks identical to a +successfully rolled-back one. See `planning/deferred.md`. + +## DI providers as pytest fixtures + +`tests/use_cases/conftest.py` calls `modern_di_pytest.expose(ioc.Repositories, +ioc.UseCases, container_fixture="request_container")` once, which generates +one pytest fixture per provider on both groups, named after the class +attribute (`create_chat_use_case`, `messages_repository`, …). Every +repository or use case added to `app/ioc.py` becomes an injectable test +fixture automatically — no test file hand-assembles a use case's dependency +graph. `request_container` itself is a child container built at +`modern_di.Scope.REQUEST`, depending on `db_session` (via an unused parameter +that forces the engine override to run first). + +Layered fixtures build on top: `alice`/`bob`/`carol` (users via +`UserFactory`, a `polyfactory.SQLAlchemyFactory`), `direct_chat` (a real +`CreateChatUseCase` call between alice and bob), `alice_message` (a real +`CreateMessageUseCase` call), and `send` — a callable that stamps a fresh +`idempotency_key` per invocation, so ordinary test bodies never collide with +each other on retries. + +## API-level tests + +`tests/conftest.py::client` runs the real `build_app()` output through +`httpx.ASGITransport` plus `asgi_lifespan.LifespanManager`, so these tests +exercise the actual route handlers, middleware, and DI wiring — not a stub. +`tests/api/*.py` drive it with plain `AsyncClient` calls and helper functions +(`register`, `login`, `create_direct_chat`, `send`, shared from +`tests/api/helpers.py` and imported with a leading-underscore alias per the +local call-site convention) rather than fixtures, since the cookie-carrying +`client` instance is itself the shared state across a test's sequence of +requests. + +## Simulating a DB race at the repository seam + +`tests/use_cases/test_create_chat.py::_RacingChatsRepository` and +`tests/use_cases/test_create_message.py::_RacingMessagesRepository` subclass +the real repository and override exactly two methods: the pre-check read +(`fetch_direct_by_key` / `fetch_by_idempotency_key`) returns `None` once, as +if the winner's row weren't visible yet, then delegates to the real +implementation; `create` always raises `DuplicateKeyError`, as if the insert +collided with a row a concurrent request just committed. A second use case +instance is built by hand with the racing repository swapped in but sharing +the *same* `transaction`/session as the real winner call, so the winner's +already-committed row is visible to the loser's recovery re-read — this is +what lets a single-process test prove the two-request race without an actual +second connection. `_AlwaysDuplicateChatsRepository` is the companion negative +case: `create` always raises, with no direct-chat recovery path available +(group chat), proving the exception still propagates instead of being +funnelled into recovery it doesn't apply to. diff --git a/docker-compose.yml b/docker-compose.yml new file mode 100644 index 0000000..e292f12 --- /dev/null +++ b/docker-compose.yml @@ -0,0 +1,30 @@ +services: + api: + build: + context: . + dockerfile: ./Dockerfile + restart: always + volumes: + - .:/code + - /code/.venv + ports: + - "8000:8000" + depends_on: + db: + condition: service_healthy + environment: + - SERVICE_ENVIRONMENT=ci + - DB_DSN=postgresql+asyncpg://postgres:password@db/postgres + - JWT_SECRET=insecure-ci-secret-do-not-use-in-prod + command: ["uv", "run", "python", "-m", "app.api"] + + db: + image: postgres:17 + restart: always + environment: + - POSTGRES_PASSWORD=password + healthcheck: + test: ["CMD-SHELL", "pg_isready -U postgres -d postgres"] + interval: 1s + timeout: 5s + retries: 15 diff --git a/migrations/README b/migrations/README new file mode 100644 index 0000000..2500aa1 --- /dev/null +++ b/migrations/README @@ -0,0 +1 @@ +Generic single-database configuration. diff --git a/migrations/env.py b/migrations/env.py new file mode 100644 index 0000000..c5fb8a6 --- /dev/null +++ b/migrations/env.py @@ -0,0 +1,44 @@ +from logging.config import fileConfig + +from alembic import context +from sqlalchemy import URL, create_engine + +from app.database.tables import METADATA +from app.settings import settings + + +def get_dsn() -> URL: + return settings.db_dsn_parsed.set(drivername="postgresql") + + +config = context.config + +if config.config_file_name is not None: + fileConfig(config.config_file_name) + +target_metadata = METADATA + + +def run_migrations_offline() -> None: + context.configure( + url=get_dsn(), + target_metadata=target_metadata, + literal_binds=True, + dialect_opts={"paramstyle": "named"}, + ) + with context.begin_transaction(): + context.run_migrations() + + +def run_migrations_online() -> None: + connectable = create_engine(get_dsn()) + with connectable.connect() as connection: + context.configure(connection=connection, target_metadata=target_metadata) + with context.begin_transaction(): + context.run_migrations() + + +if context.is_offline_mode(): + run_migrations_offline() +else: + run_migrations_online() diff --git a/migrations/script.py.mako b/migrations/script.py.mako new file mode 100644 index 0000000..a65d971 --- /dev/null +++ b/migrations/script.py.mako @@ -0,0 +1,25 @@ +"""${message} + +Revision ID: ${up_revision} +Revises: ${down_revision | comma,n} +Create Date: ${create_date} + +""" +import advanced_alchemy +from alembic import op +import sqlalchemy as sa +${imports if imports else ""} + +# revision identifiers, used by Alembic. +revision = ${repr(up_revision)} +down_revision = ${repr(down_revision)} +branch_labels = ${repr(branch_labels)} +depends_on = ${repr(depends_on)} + + +def upgrade() -> None: + ${upgrades if upgrades else "pass"} + + +def downgrade() -> None: + ${downgrades if downgrades else "pass"} diff --git a/migrations/versions/2026-08-21_chats_and_members.py b/migrations/versions/2026-08-21_chats_and_members.py new file mode 100644 index 0000000..a425e69 --- /dev/null +++ b/migrations/versions/2026-08-21_chats_and_members.py @@ -0,0 +1,62 @@ +"""chats_and_members. + +Revision ID: 88ba0ea3f7e6 +Revises: b8565e6bbe4b +Create Date: 2026-08-21 10:09:23.876506 + +""" + +import advanced_alchemy +import sqlalchemy as sa +from alembic import op + + +# revision identifiers, used by Alembic. +revision = "88ba0ea3f7e6" +down_revision = "b8565e6bbe4b" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + # ### commands auto generated by Alembic - please adjust! ### + op.create_table( + "chats", + sa.Column("id", sa.BigInteger().with_variant(sa.Integer(), "sqlite"), nullable=False), + sa.Column( + "chat_type", + sa.Enum("direct", "group", name="chattype", native_enum=False, create_constraint=True), + nullable=False, + ), + sa.Column("title", sa.String(length=128), nullable=True), + sa.Column("created_by_id", sa.BigInteger().with_variant(sa.Integer(), "sqlite"), nullable=False), + sa.Column("last_message_id", sa.BigInteger(), nullable=True), + sa.Column("direct_key", sa.String(length=64), nullable=True), + sa.Column("created_at", advanced_alchemy.types.datetime.DateTimeUTC(timezone=True), nullable=False), + sa.Column("updated_at", advanced_alchemy.types.datetime.DateTimeUTC(timezone=True), nullable=False), + sa.ForeignKeyConstraint(["created_by_id"], ["users.id"], name=op.f("fk_chats_created_by_id_users")), + sa.PrimaryKeyConstraint("id", name=op.f("pk_chats")), + sa.UniqueConstraint("direct_key", name=op.f("uq_chats_direct_key")), + ) + op.create_table( + "chat_members", + sa.Column("id", sa.BigInteger().with_variant(sa.Integer(), "sqlite"), nullable=False), + sa.Column("chat_id", sa.BigInteger().with_variant(sa.Integer(), "sqlite"), nullable=False), + sa.Column("user_id", sa.BigInteger().with_variant(sa.Integer(), "sqlite"), nullable=False), + sa.Column("last_read_message_id", sa.BigInteger(), nullable=True), + sa.Column("joined_at", advanced_alchemy.types.datetime.DateTimeUTC(timezone=True), nullable=False), + sa.ForeignKeyConstraint(["chat_id"], ["chats.id"], name=op.f("fk_chat_members_chat_id_chats")), + sa.ForeignKeyConstraint(["user_id"], ["users.id"], name=op.f("fk_chat_members_user_id_users")), + sa.PrimaryKeyConstraint("id", name=op.f("pk_chat_members")), + sa.UniqueConstraint("chat_id", "user_id", name="uk_chat_members_chat_id_user_id"), + ) + op.create_index(op.f("ix_chat_members_user_id"), "chat_members", ["user_id"], unique=False) + # ### end Alembic commands ### + + +def downgrade() -> None: + # ### commands auto generated by Alembic - please adjust! ### + op.drop_index(op.f("ix_chat_members_user_id"), table_name="chat_members") + op.drop_table("chat_members") + op.drop_table("chats") + # ### end Alembic commands ### diff --git a/migrations/versions/2026-08-21_init.py b/migrations/versions/2026-08-21_init.py new file mode 100644 index 0000000..1895d08 --- /dev/null +++ b/migrations/versions/2026-08-21_init.py @@ -0,0 +1,40 @@ +"""init. + +Revision ID: b8565e6bbe4b +Revises: +Create Date: 2026-08-21 09:09:58.314813 + +""" + +import advanced_alchemy +import sqlalchemy as sa +from alembic import op + + +# revision identifiers, used by Alembic. +revision = "b8565e6bbe4b" +down_revision = None +branch_labels = None +depends_on = None + + +def upgrade() -> None: + # ### commands auto generated by Alembic - please adjust! ### + op.create_table( + "users", + sa.Column("id", sa.BigInteger().with_variant(sa.Integer(), "sqlite"), nullable=False), + sa.Column("username", sa.String(length=64), nullable=False), + sa.Column("password_hash", sa.String(), nullable=False), + sa.Column("display_name", sa.String(length=128), nullable=False), + sa.Column("created_at", advanced_alchemy.types.datetime.DateTimeUTC(timezone=True), nullable=False), + sa.Column("updated_at", advanced_alchemy.types.datetime.DateTimeUTC(timezone=True), nullable=False), + sa.PrimaryKeyConstraint("id", name=op.f("pk_users")), + sa.UniqueConstraint("username", name=op.f("uq_users_username")), + ) + # ### end Alembic commands ### + + +def downgrade() -> None: + # ### commands auto generated by Alembic - please adjust! ### + op.drop_table("users") + # ### end Alembic commands ### diff --git a/migrations/versions/2026-08-21_messages.py b/migrations/versions/2026-08-21_messages.py new file mode 100644 index 0000000..1470192 --- /dev/null +++ b/migrations/versions/2026-08-21_messages.py @@ -0,0 +1,55 @@ +"""messages. + +Revision ID: 1be68642e392 +Revises: 88ba0ea3f7e6 +Create Date: 2026-08-21 11:09:52.596868 + +""" + +import advanced_alchemy +import sqlalchemy as sa +from alembic import op + + +# revision identifiers, used by Alembic. +revision = "1be68642e392" +down_revision = "88ba0ea3f7e6" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + # ### commands auto generated by Alembic - please adjust! ### + op.create_table( + "messages", + sa.Column("id", sa.BigInteger().with_variant(sa.Integer(), "sqlite"), nullable=False), + sa.Column("chat_id", sa.BigInteger().with_variant(sa.Integer(), "sqlite"), nullable=False), + sa.Column("user_id", sa.BigInteger().with_variant(sa.Integer(), "sqlite"), nullable=True), + sa.Column("idempotency_key", advanced_alchemy.types.guid.GUID(length=16), nullable=False), + sa.Column("text", sa.String(), nullable=False), + sa.Column("created_at", advanced_alchemy.types.datetime.DateTimeUTC(timezone=True), nullable=False), + sa.Column("edited_at", advanced_alchemy.types.datetime.DateTimeUTC(timezone=True), nullable=True), + sa.Column("deleted_at", advanced_alchemy.types.datetime.DateTimeUTC(timezone=True), nullable=True), + sa.ForeignKeyConstraint(["chat_id"], ["chats.id"], name=op.f("fk_messages_chat_id_chats")), + sa.ForeignKeyConstraint(["user_id"], ["users.id"], name=op.f("fk_messages_user_id_users")), + sa.PrimaryKeyConstraint("id", name=op.f("pk_messages")), + sa.UniqueConstraint("chat_id", "idempotency_key", name="uk_messages_chat_id_idempotency_key"), + ) + op.create_index("ix_messages_chat_id_id", "messages", ["chat_id", "id"], unique=False) + op.create_index(op.f("ix_messages_user_id"), "messages", ["user_id"], unique=False) + # ### end Alembic commands ### + # NOTE: autogenerate also proposed `op.drop_constraint(op.f('ck_chats_chattype'), 'chats', type_='check')` + # here. That's a known Alembic false positive for `sa.Enum(native_enum=False, create_constraint=True)` + # columns: Postgres reflects the CHECK constraint body back as `chat_type::text = ANY (ARRAY[...])`, + # which never textually matches what Alembic renders from the model, so every autogenerate run + # "detects" this same constraint as removed even though nothing about `chats.chat_type` changed. + # Dropping it here would be unrelated to this migration's purpose (adding `messages`) and would + # silently remove enum validation from `chats.chat_type`, so it is intentionally omitted. + + +def downgrade() -> None: + # ### commands auto generated by Alembic - please adjust! ### + op.drop_index(op.f("ix_messages_user_id"), table_name="messages") + op.drop_index("ix_messages_chat_id_id", table_name="messages") + op.drop_table("messages") + # ### end Alembic commands ### diff --git a/planning/.convention-version b/planning/.convention-version new file mode 100644 index 0000000..227cea2 --- /dev/null +++ b/planning/.convention-version @@ -0,0 +1 @@ +2.0.0 diff --git a/planning/_templates/change.md b/planning/_templates/change.md new file mode 100644 index 0000000..5aa7e81 --- /dev/null +++ b/planning/_templates/change.md @@ -0,0 +1,32 @@ +--- +summary: One line — shown in the generated index. Written at creation; finalize at ship to state the realized result. +--- + +# Change: One-line capitalized title + +**Lane:** lightweight — ≲30 LOC net, ≤2 files, no new file, no public-API +change, a single straightforward test. If it outgrows this, rewrite it from +the design template. + +## Goal + +One or two sentences: what changes and why. + +## Approach + +The shape of the change in brief — enough that a reviewer sees the design +without a full spec. Link the truth home (`architecture/.md`) if a +capability contract moves. + +## Files + +- `path/to/file.py` — what changes +- `tests/test_x.py` — test added / updated + +## Verification + +- [ ] Failing test first — command + expected error. +- [ ] Apply the change. +- [ ] Test passes — command. +- [ ] `just test` — full suite green. +- [ ] `just lint` — clean. diff --git a/planning/_templates/decision.md b/planning/_templates/decision.md new file mode 100644 index 0000000..45ccaf0 --- /dev/null +++ b/planning/_templates/decision.md @@ -0,0 +1,23 @@ +--- +status: accepted # accepted | superseded +summary: One line — shown in `just index`. +supersedes: null +superseded_by: null +--- + +# One-line capitalized title + +**Decision:** What was decided, in a sentence. + +## Context + +Why this came up; the options that were on the table. + +## Decision & rationale + +The call and why — including why the alternatives were rejected. Enough that a +future explorer doesn't re-litigate it. + +## Revisit trigger + +The concrete signal that should reopen this decision. diff --git a/planning/_templates/design.md b/planning/_templates/design.md new file mode 100644 index 0000000..17dbee1 --- /dev/null +++ b/planning/_templates/design.md @@ -0,0 +1,39 @@ +--- +summary: One line — shown in the generated index. Written at creation; finalize at ship to state the realized result. +--- + +# Design: One-line capitalized title + + + +## Summary + +One paragraph. What changes, at the level a reader needs to decide if this +spec is worth reading in full. + +## Motivation + +Why now. What is broken or missing. Concrete observations / numbers, not +abstract complaints. + +## Design + +What changes, in enough detail that a reader who has not seen the codebase +can follow. Sketches and interface fragments welcome; never the full +diff-to-be. Reference rejected alternatives in `decisions/` instead of +retelling them. + +## Non-goals + +What is deliberately out of scope and (when nontrivial) why. One line each. + +## Testing + +How we know it landed correctly. Be specific: the command and the expected +signal. + +## Risk + +What could go wrong, ranked by likelihood × impact. Mitigations. diff --git a/planning/changes/2026-08-21.01-chat-app-bootstrap.md b/planning/changes/2026-08-21.01-chat-app-bootstrap.md index 8fc17e7..e668d8c 100644 --- a/planning/changes/2026-08-21.01-chat-app-bootstrap.md +++ b/planning/changes/2026-08-21.01-chat-app-bootstrap.md @@ -1,5 +1,5 @@ --- -summary: Bootstrap the chat-app showcase repo — package skeleton, DI container, JWT cookie auth, and the core chat domain (chats, members, messages) over REST. +summary: Shipped the chat-app showcase repo — modern-di container, JWT cookie auth, and chats/members/messages over REST with idempotent send, cursor pagination, author-and-member-gated edit/delete, and per-member read markers with unread counts, at 100% coverage. --- # Design: Bootstrap chat-app skeleton and core chat domain diff --git a/planning/decisions/2026-08-21-anonymous-doc-and-metrics-paths.md b/planning/decisions/2026-08-21-anonymous-doc-and-metrics-paths.md new file mode 100644 index 0000000..b165fe4 --- /dev/null +++ b/planning/decisions/2026-08-21-anonymous-doc-and-metrics-paths.md @@ -0,0 +1,41 @@ +--- +status: accepted +summary: The auth exclude list carries four anchored prefixes — /docs, /health, /static and /metrics — and nothing else. +--- + +# The anonymous surface is four prefixes + +`jwt_cookie_auth`'s `exclude` list holds `^/docs`, `^/health`, `^/static` and +`^/metrics`. Every pattern is anchored, because Litestar joins them into one +alternation and matches it with an unanchored `findall`: an unanchored `/health` +would silently deauthenticate any future route containing that substring, such +as `/api/chats/{id}/health`. + +`/static` is Swagger's own offline asset directory, mounted because +`swagger_offline_docs` is on. Without the exclusion the docs page returns `200` +and then every asset request returns `401`, so the page loads and fails to +render for anonymous visitors. `/metrics` is a Prometheus scrape target, +registered because `prometheus_client` ships in `lite-bootstrap[litestar-all]`; +a scrape target behind a session cookie is a broken feature, and the endpoint +exposes process and request metrics, not user data. + +Route-level exemptions are expressed differently: `register` and `login` use +`exclude_from_auth=True` on the handler. Path-shaped exclusions go in the list; +route-shaped ones go on the route. Each policy has one home. + +## Rejected: leaving /metrics authenticated + +Defensible, and it is what shipped initially by omission. But it silently +disables a feature the bootstrapper registers, and the standard hardening for +metrics is a separate port or a network ACL, which is a deployment concern this +repository does not model. + +## Consequence + +Anything served under those four prefixes is public. A future route must not be +placed under them casually. + +## Revisit trigger + +Metrics carrying anything user-identifying, a deployment that exposes `/metrics` +to the internet, or Litestar changing where Swagger's offline assets are mounted. diff --git a/planning/decisions/2026-08-21-coverage-exclusions-structural-only.md b/planning/decisions/2026-08-21-coverage-exclusions-structural-only.md new file mode 100644 index 0000000..2a64b99 --- /dev/null +++ b/planning/decisions/2026-08-21-coverage-exclusions-structural-only.md @@ -0,0 +1,47 @@ +--- +status: accepted +summary: Coverage exclusions are reserved for code pytest structurally cannot execute; unreachable-in-production branches are tested through repository seams instead. +--- + +# Coverage exclusions are structural only + +The suite runs at `--cov-fail-under=100`. Two mechanisms can exempt code, and +each has a narrow warrant: + +- `[tool.coverage.run] omit` lists `migrations/*`, `app/api/__main__.py` and + `planning/index.py` — files pytest never imports at all. +- `# pragma: no cover` is not used anywhere in `app/`, `tests/` or + `migrations/`. + +"Awkward to reach" is not a warrant. Where a branch looked untestable, the +answer was a repository subclass that raises the condition the database would +raise. That is how both `DuplicateKeyError` recovery paths and both defensive +`is None` guards became executed code. + +`filterwarnings = ["error"]` enforces the companion property: the suite runs at +zero warnings, and a new warning fails a test rather than scrolling past. + +## Rejected: pragmas on unreachable-in-production guards + +Argued for twice during implementation, on the grounds that the guards cannot +fire while the unique constraint holds. Rejected because an excluded branch is +one nobody notices when it stops being unreachable, and because the coverage +number then asserts something untrue about what the tests exercise. The two +guards in question were reachable through a seam the tests already owned. + +## Rejected: tests written only to move the number + +Also seen and removed: an `assert __name__ != "__main__"` that could not fail, +and an `isinstance` check against a function whose body constructs that type. +The gate exists to make untested code visible; satisfying it with assertions +that cannot fail defeats it more thoroughly than a lower number would. + +## Consequence + +Adding genuinely unexecutable code requires an `omit` entry with a stated +reason, reviewed as a decision rather than applied inline. + +## Revisit trigger + +A dependency that emits an unfixable warning, or platform-specific code paths +that cannot run in CI. diff --git a/planning/decisions/2026-08-21-domain-error-vocabulary.md b/planning/decisions/2026-08-21-domain-error-vocabulary.md new file mode 100644 index 0000000..a2c6272 --- /dev/null +++ b/planning/decisions/2026-08-21-domain-error-vocabulary.md @@ -0,0 +1,41 @@ +--- +status: accepted +summary: Domain failures are split across PermissionDeniedError (403), ValidationError (400) and ConflictError (409) rather than expressed as authorization failures. +--- + +# Three domain exceptions, not one + +`app/exceptions.py` defines `ChatAppError` and three subclasses, each with a +handler registered in `build_app`: + +- `PermissionDeniedError` to `403`, for authorization only: the caller may not + perform this action on this resource. +- `ValidationError` to `400`, for request shape: "a direct chat must have + exactly two distinct members", "before_id and after_id are mutually + exclusive", "limit must be at least 1", "that message is not in this chat". +- `ConflictError` to `409`, for state conflict: editing a message that has been + deleted. + +`advanced-alchemy`'s `NotFoundError` maps to `404`, `DuplicateKeyError` to +`409`, and `ForeignKeyError` to `400` with a constant detail string. Litestar +handles `NotAuthorizedException` natively as `401`. + +## Rejected: PermissionDeniedError for everything + +The initial design raised `PermissionDeniedError` for malformed request bodies +and for state conflicts as well as for authorization. It is the shape a reader +copies, and it is wrong twice over: a body with three members in a direct chat +is not a permissions problem, and the author of a deleted message *is* +authorized. Returning either inside a "Permission denied" envelope tells the +client to go find credentials it already has. + +## Consequence + +Every new use case must pick a category deliberately. Handlers must not +stringify the underlying exception when the query carried credential material, +which is why `DuplicateKeyError` and `ForeignKeyError` return constant details. + +## Revisit trigger + +A fourth failure category that fits none of the three, or an `RFC 9457` +problem-details response format, which would restructure all of them. diff --git a/planning/decisions/2026-08-21-explicit-cookie-secure-flag.md b/planning/decisions/2026-08-21-explicit-cookie-secure-flag.md new file mode 100644 index 0000000..6a44231 --- /dev/null +++ b/planning/decisions/2026-08-21-explicit-cookie-secure-flag.md @@ -0,0 +1,44 @@ +--- +status: accepted +summary: The session cookie's Secure flag comes from an explicit jwt_cookie_secure setting, not from inspecting service_environment. +--- + +# Cookie security is an explicit setting + +`Settings.jwt_cookie_secure` defaults to `False` and is passed straight to +`JWTCookieAuth(secure=...)`. Production must set it. + +Litestar sets `httponly=True` and `samesite="lax"` for you, but leaves `secure` +as `None`, so without this setting a session JWT with a seven-day lifetime +travels over plain HTTP. + +A companion guard lives in `Settings.ensure_jwt_secret_is_configured`, called as +the first statement of `build_app`: booting with the default `jwt_secret` +outside `service_environment="local"` raises rather than starting a service +whose every user's token is forgeable. + +## Rejected: deriving it from the environment + +`secure = service_environment != "local"` needs no new setting and is right by +default. It was rejected because a reader of a reference application should see +where the decision is made. A security property inferred from an unrelated +string is a property nobody audits, and the inference is silently wrong the +first time someone introduces an environment name the expression did not +anticipate. + +## Rejected: defaulting to True + +Correct for production and unusable for local development over HTTP, which is +how this application is demonstrated. + +## Consequence + +A deployment that forgets `JWT_COOKIE_SECURE` transmits session cookies in +clear. The startup guard covers the forged-token case but deliberately does not +cover this one, because there is no way to distinguish "HTTP because local" from +"HTTP by mistake" at boot. + +## Revisit trigger + +Adding HSTS or terminating TLS in-process, either of which would make `True` a +safe default. diff --git a/planning/decisions/2026-08-21-idempotency-scoped-per-chat.md b/planning/decisions/2026-08-21-idempotency-scoped-per-chat.md new file mode 100644 index 0000000..d994ac2 --- /dev/null +++ b/planning/decisions/2026-08-21-idempotency-scoped-per-chat.md @@ -0,0 +1,40 @@ +--- +status: accepted +summary: Message idempotency is scoped to (chat_id, idempotency_key), not to the key alone. +--- + +# Idempotency is scoped per chat + +`messages` carries `UniqueConstraint("chat_id", "idempotency_key")`. The +pre-check lookup, the constraint, and the `DuplicateKeyError` recovery re-read +all filter on the same pair. + +Idempotency is a property of an operation, and the operation is "send this +message *to this chat*". Two different chats are two different operations; a key +reused across them is not a retry of anything. + +## Rejected: a global unique constraint on `idempotency_key` + +Shipped first, and it made a reachable state look unreachable. With a global +constraint and a chat-scoped lookup, a cross-chat key reuse under concurrency +raises `DuplicateKeyError` from the insert, the scoped re-read misses, and +control reaches a guard whose only justification was "the unique constraint +guarantees a match here". That guarantee no longer held. + +Keeping the constraint global while scoping only the lookup also meant a client +reusing a key across chats received the *other* chat's message and its intended +message was never written: a silent wrong-row return, which is worse than an +error. + +The three surfaces must agree. Aligning the constraint with the lookup was the +cheaper direction, and it is the one that matches the domain. + +## Consequence + +The same client-generated key may legitimately appear in two chats. Callers that +assume global uniqueness of `idempotency_key` are wrong. + +## Revisit trigger + +A cross-chat operation that must be idempotent as a unit, such as forwarding one +message into several chats in a single request. diff --git a/planning/decisions/2026-08-21-mutation-requires-membership.md b/planning/decisions/2026-08-21-mutation-requires-membership.md new file mode 100644 index 0000000..447451a --- /dev/null +++ b/planning/decisions/2026-08-21-mutation-requires-membership.md @@ -0,0 +1,40 @@ +--- +status: accepted +summary: Editing and deleting a message requires chat membership as well as authorship; the check order is existence, membership, authorship. +--- + +# Mutation requires membership, not just authorship + +`fetch_message_for_author` (`app/use_cases/message_authorization.py`) is the +single definition of the check order for both `EditMessageUseCase` and +`DeleteMessageUseCase`: load the message (`404` if absent), verify the actor is +a member of its chat (`403`), then verify the actor is the author (`403`). + +## Rejected: authorship alone + +Shipped first, and it left every authenticated user able to `PATCH`/`DELETE` an +arbitrary message id in a chat they had no visibility into, and to distinguish +"does not exist" from "exists, not mine" for it. Authorship happens to block the +ordinary case, which is why the first round of tests passed identically with and +without the membership check. + +It was also inconsistent with the rest of the codebase: every other actor-scoped +use case gates on membership first. A reference application that applies its own +authorization rule unevenly teaches the wrong habit. + +The check is proven by a test that constructs the one state where membership and +authorship disagree: the author's `chat_members` row is deleted, leaving her the +author of a message in a chat she is no longer in. + +## Consequence + +A non-member still learns whether a message id exists, because the message must +be loaded before its chat is known. That residual is accepted deliberately and +mirrors the decision that `FetchChatUseCase` returns `403` rather than +pretending the chat does not exist. See `planning/deferred.md`. + +## Revisit trigger + +A moderator or administrator role that must act on messages in chats it does not +belong to, or a requirement to close the existence oracle, which would mean +scoping the lookup through a `chat_members` join. diff --git a/planning/decisions/2026-08-21-read-marker-integrity.md b/planning/decisions/2026-08-21-read-marker-integrity.md new file mode 100644 index 0000000..8abb5ff --- /dev/null +++ b/planning/decisions/2026-08-21-read-marker-integrity.md @@ -0,0 +1,46 @@ +--- +status: accepted +summary: The read marker advances only to a message in its own chat, and advances atomically via GREATEST so it can never move backwards. +--- + +# Read-marker integrity + +`MarkReadUseCase` does three things in order: verify membership, verify that +`last_read_message_id` names a message in *that* chat, then advance the marker +with a single statement: + +```sql +UPDATE chat_members +SET last_read_message_id = GREATEST(COALESCE(last_read_message_id, 0), :requested) +``` + +Unread is then `count(messages WHERE chat_id = ? AND id > COALESCE(marker, 0) +AND user_id IS DISTINCT FROM me AND deleted_at IS NULL)`. + +`IS DISTINCT FROM` rather than `!=` is load-bearing: system messages carry +`user_id IS NULL`, and `NULL != 1` evaluates to NULL, which silently drops every +system message from the count. A regression test asserts this. + +## Rejected: monotonicity enforced in Python + +Read the member row, compute `max(current, requested)`, write it back. Two +concurrent `POST /read/` calls interleave and the lower id wins, which is +exactly the regression the monotonic rule exists to prevent. `GREATEST` in the +UPDATE makes it atomic without a lock. + +## Rejected: accepting any id + +Without the message-in-chat check a client can set its marker to an arbitrarily +large id and permanently zero its own unread counts. Persisting self-inflicted +data corruption is worse than refusing the request. + +## Consequence + +Marking read costs one extra lookup. Advancing to a lower id is a silent no-op +rather than an error, because a replayed or out-of-order request is not a client +mistake worth reporting. + +## Revisit trigger + +Per-device read markers, or a requirement to move a marker backwards +deliberately, such as "mark as unread". diff --git a/planning/decisions/2026-08-21-repoint-last-message-on-delete.md b/planning/decisions/2026-08-21-repoint-last-message-on-delete.md new file mode 100644 index 0000000..0839dad --- /dev/null +++ b/planning/decisions/2026-08-21-repoint-last-message-on-delete.md @@ -0,0 +1,37 @@ +--- +status: accepted +summary: Soft-deleting a chat's newest message repoints chats.last_message_id in the same transaction, rather than filtering the deleted row out of the listing preview. +--- + +# Repoint last_message_id on delete + +`DeleteMessageUseCase` soft-deletes the message and, if it was the chat's +`last_message_id`, repoints that column to the newest remaining message with +`deleted_at IS NULL`, or to `NULL` if none remains. Both writes commit together. + +The column therefore has one meaning: the newest non-deleted message in this +chat. The listing preview and the listing's ordering +(`coalesce(chats.last_message_id, 0) DESC`) both read it, and both stay correct. + +## Rejected: filtering the preview query instead + +Adding `deleted_at IS NULL` to the preview fetch is the smaller change and was +considered first. It leaves two half-broken behaviours instead of one correct +one: the preview goes blank while older messages still exist, and the chat +continues to sort by the deleted message's id, because ordering reads the same +column the preview stopped trusting. + +The filter is still present on the preview fetch, but as a self-defending +invariant guard rather than as the mechanism. + +## Consequence + +Deleting the newest message costs one extra query. `chats.last_message_id` +remains a plain `BigInteger` rather than a foreign key, because `chats` is +created before `messages` exists and a circular constraint pair would buy +nothing at this scale. + +## Revisit trigger + +A hard-delete path, a bulk delete, or any other writer of +`chats.last_message_id` that would need the same repointing logic. diff --git a/planning/decisions/2026-08-21-upsert-via-duplicate-key-recovery.md b/planning/decisions/2026-08-21-upsert-via-duplicate-key-recovery.md new file mode 100644 index 0000000..d610f79 --- /dev/null +++ b/planning/decisions/2026-08-21-upsert-via-duplicate-key-recovery.md @@ -0,0 +1,44 @@ +--- +status: accepted +summary: Direct-chat creation and message send recover from a unique-constraint violation and re-read, rather than trusting a pre-check. +--- + +# Upsert by recovering from DuplicateKeyError + +Both `CreateChatUseCase` and `CreateMessageUseCase` read first to see whether +the row already exists, then insert. The read is an optimisation. The +correctness guarantee is the `except DuplicateKeyError:` branch, which rolls +back and re-reads the row the winner committed. + +Both recovery paths are exercised by tests, through repository subclasses +(`_RacingChatsRepository`, `_RacingMessagesRepository`) whose `create` raises +`DuplicateKeyError` and whose lookup misses once before delegating to the real +implementation. That simulates a database condition at a seam the tests already +own, rather than mocking the unit under test. + +## Rejected: the pre-check alone + +The original design assumed a read inside the transaction made the insert safe. +It does not. At READ COMMITTED two concurrent "open a DM with Bob" requests both +miss the read, both insert, and the loser violates `uq_chats_direct_key`. That +surfaces as a `409` to a user who should simply have received the existing chat, +which contradicts the reason `direct_key` exists at all. + +`@postgres_retry` does not rescue it either: `db-retry` retries serialization +and connection failures, not integrity violations. + +## Rejected: `SELECT ... FOR UPDATE` + +There is no row to lock. The race is between two inserts of a row that does not +yet exist, so row-level locking has nothing to take. + +## Consequence + +The happy path costs one extra read. The contended path costs a rolled-back +insert plus a re-read, which is strictly better than returning an error for a +request that should have succeeded. + +## Revisit trigger + +A write path where the losing racer's rollback is too expensive to accept, or a +move to an `INSERT ... ON CONFLICT` form that still needs the same re-read. diff --git a/planning/deferred.md b/planning/deferred.md index 7863969..d6c55d0 100644 --- a/planning/deferred.md +++ b/planning/deferred.md @@ -40,3 +40,86 @@ This reports "has an open stream", not "is looking at this chat", and a client killed between heartbeats stays online until expiry. **Revisit trigger:** the demo needing per-chat presence or accurate last-seen. + +## Per-test rollback is fail-silent on an unexpected commit + +The `if connection.in_transaction():` guard in `tests/conftest.py`'s +`db_session` teardown skips the rollback without error whenever the outer +transaction is already closed. It exists to tolerate tests that legitimately +closed their own transaction, but it can't distinguish that from a session +somewhere having committed the outer transaction instead of nesting a +savepoint under it — that failure mode would leak state into the next test +with no diagnostic. + +**Revisit trigger:** a test suite flake that looks like cross-test state +leakage, or before adding any code path that opens a session without going +through `database_resources.create_session`. + +## Isolation test pair is order-dependent + +`tests/test_main.py::test_db_session_insert_is_visible_within_test` and +`test_db_session_rolls_back_between_tests` together prove the per-test +rollback fixture, but only when pytest runs them in file order: the first +inserts and commits, the second asserts the table is empty. Run the second +alone (e.g. `-k test_db_session_rolls_back_between_tests`) and it passes +vacuously — an empty table before any insert is indistinguishable from a +correctly rolled-back one. + +**Revisit trigger:** test order ever becomes non-deterministic (parallel +pytest execution, `pytest-randomly`), or before trusting `-k` output from just +this pair as proof the fixture works. + +## Logout does not revoke the JWT + +`POST /api/auth/logout/` deletes the cookie but the token itself stays valid +for the rest of its `jwt_lifetime_seconds` (7 days by default) if it was +copied out of the cookie beforehand — no `revoked_token_handler` is +configured on `jwt_cookie_auth`. + +**Revisit trigger:** any deployment where a leaked/copied token is a realistic +threat model, or before shipping a "log out of all devices" feature. + +## Every authenticated request opens two DB sessions + +Auth middleware runs before request-scoped DI is available, so +`retrieve_user_handler` (`app/api/auth.py`) opens its own short-lived session +for the user lookup, separate from the request-scoped session the resolved +use case's repositories use. That's two sessions per authenticated request +against `db_pool_size=5` / `db_max_overflow=0`. + +**Revisit trigger:** before deploying this anywhere with real concurrent +traffic — pool exhaustion under load is the first thing to check if requests +start timing out waiting for a connection. + +## Message id existence is distinguishable via 404-vs-403 + +A non-member issuing `PATCH`/`DELETE /api/messages/{id}/` gets `404` for an +id that doesn't exist and `403` for one that does but belongs to a chat +they're not in — the two status codes leak whether the id is real. Accepted +deliberately: it mirrors the spec's own decision that `FetchChatUseCase` +returns `403` for a chat the actor isn't a member of rather than pretending +the chat doesn't exist (see `architecture/chats.md`), and checking membership +before authorship on every message use case keeps that posture consistent +rather than making message mutation the one place that hides existence. + +**Revisit trigger:** a threat model where message-id enumeration by a +non-member is a real concern (e.g. ids that encode something sensitive). + +## `EditMessageRequest` duplicates `SendMessageRequest`'s text constraints + +Both `app/schemas/api.py::SendMessageRequest.text` and `EditMessageRequest.text` +independently declare `pydantic.Field(min_length=1, max_length=4000)`. A +change to one's bounds is silently not a change to the other's. + +**Revisit trigger:** the two are ever meant to diverge deliberately, or a bug +report about edit accepting/rejecting text that send doesn't (or vice versa). + +## No query-count instrumentation + +Nothing in the test suite counts queries per request, so an N+1 regression in +the chat listing (e.g. `FetchChatsUseCase`'s bounded `last_message` lookup +regressing back to one query per chat) would keep `just test` green as long +as the returned data is still correct. + +**Revisit trigger:** a reported latency regression on `GET /api/chats/`, or +before adding another listing endpoint that joins per-row data. diff --git a/planning/index.py b/planning/index.py new file mode 100644 index 0000000..60116da --- /dev/null +++ b/planning/index.py @@ -0,0 +1,183 @@ +# planning/ is not a Python package (this file is vendored into consumers' planning/) +"""Generate the planning index from frontmatter. + +Run via ``just index``. Globs ``planning/changes/*.md`` and +``planning/decisions/*.md``, reads their frontmatter, and prints a Markdown +listing to stdout — changes then decisions, newest-first. Never writes a file: +the listing is a query over the files, not a committed artifact. + +``date`` and ``slug`` are derived from the file name, not +frontmatter — the name is the single source of truth for both. +""" + +import pathlib +import re +import sys + + +ROOT = pathlib.Path(__file__).parent +VALID_DECISION_STATUS = {"accepted", "superseded"} +CHANGE_RE = re.compile(r"^(?P\d{4}-\d{2}-\d{2})\.\d{2}-(?P.+)$") +DECISION_RE = re.compile(r"^(?P\d{4}-\d{2}-\d{2})-(?P.+)$") +SPEC_REQUIRED = ("summary",) +DECISION_REQUIRED = ("status", "summary") + + +def parse_frontmatter(text: str) -> dict[str, str]: + """Parse a single-line-scalar YAML frontmatter block into a dict.""" + lines = text.splitlines() + if not lines or lines[0].strip() != "---": + return {} + fields: dict[str, str] = {} + for line in lines[1:]: + if line.strip() == "---": + break + if line[:1] in (" ", "\t"): + continue + key, sep, value = line.partition(": ") + if not sep: + continue + cleaned = value.strip().strip('"').strip("'") + fields[key.strip()] = "" if cleaned == "null" else cleaned + return fields + + +def _named(fields: dict[str, str], name: str, pattern: re.Pattern[str]) -> dict[str, str]: + """Inject ``date``/``slug`` derived from a file name into ``fields``.""" + match = pattern.match(name) + if match: + fields["date"] = match.group("date") + fields["slug"] = match.group("slug") + return fields + + +def load_changes(root: pathlib.Path) -> list[dict[str, str]]: + """Read each change file's summary; derive date/slug from the file name.""" + changes_dir = root / "changes" + changes: list[dict[str, str]] = [] + if not changes_dir.is_dir(): + return changes + for path in sorted(changes_dir.glob("*.md")): + if path.name == "README.md" or path.name.startswith(("_", ".")): + continue + fields = _named(parse_frontmatter(path.read_text(encoding="utf-8")), path.stem, CHANGE_RE) + fields["path"] = f"changes/{path.name}" + fields["name"] = path.stem + changes.append(fields) + return changes + + +def load_decisions(root: pathlib.Path) -> list[dict[str, str]]: + """Read each decision's frontmatter; derive date/slug from the file name.""" + decisions_dir = root / "decisions" + decisions: list[dict[str, str]] = [] + if not decisions_dir.is_dir(): + return decisions + for path in sorted(decisions_dir.glob("*.md")): + if path.name == "README.md" or path.name.startswith("_"): + continue + fields = _named(parse_frontmatter(path.read_text(encoding="utf-8")), path.stem, DECISION_RE) + fields["path"] = f"decisions/{path.name}" + fields["name"] = path.stem + decisions.append(fields) + return decisions + + +def format_row(row: dict[str, str]) -> str: + """Render one change or decision as a Markdown list item.""" + slug = row.get("slug", "?") + path = row.get("path", "") + date = row.get("date", "") + summary = row.get("summary") or "(no summary)" + line = f"- **[{slug}]({path})** ({date}) — {summary}" + if row.get("supersedes"): + line += f" _(supersedes {row['supersedes']})_" + if row.get("superseded_by"): + line += f" _(superseded by {row['superseded_by']})_" + return line + + +def render(changes: list[dict[str, str]], decisions: list[dict[str, str]]) -> str: + """Render the full Markdown listing: changes then decisions, newest-first.""" + out = ["# Planning index", "", "_Generated by `just index` — do not edit._", "", "## Changes", ""] + change_rows = sorted(changes, key=lambda b: b.get("name", ""), reverse=True) + out += [format_row(b) for b in change_rows] if change_rows else ["_None._"] + out += ["", "## Decisions", ""] + decision_rows = sorted(decisions, key=lambda d: d.get("name", ""), reverse=True) + out += [format_row(d) for d in decision_rows] if decision_rows else ["_None._"] + out.append("") + return "\n".join(out).rstrip() + "\n" + + +def _require(fields: dict[str, str], keys: tuple[str, ...], rel: str, violations: list[str]) -> None: + """Append a violation for each required key that is absent or empty.""" + violations.extend(f"{rel}: missing or empty frontmatter key '{key}'" for key in keys if not fields.get(key)) + + +def _check_change(path: pathlib.Path, violations: list[str]) -> None: + """Validate one change file (requires `summary`).""" + rel = f"changes/{path.name}" + if CHANGE_RE.match(path.stem) is None: + violations.append(f"{rel}: file name is not 'YYYY-MM-DD.NN-slug.md'") + fields = parse_frontmatter(path.read_text(encoding="utf-8")) + _require(fields, SPEC_REQUIRED, rel, violations) + + +def _check_decision(path: pathlib.Path, violations: list[str]) -> None: + """Validate one decision file (requires `status` + `summary`).""" + rel = f"decisions/{path.name}" + if DECISION_RE.match(path.stem) is None: + violations.append(f"{rel}: file name is not 'YYYY-MM-DD-slug.md'") + fields = parse_frontmatter(path.read_text(encoding="utf-8")) + _require(fields, DECISION_REQUIRED, rel, violations) + status = fields.get("status", "") + if status and status not in VALID_DECISION_STATUS: + violations.append(f"{rel}: invalid status '{status}' (allowed: {', '.join(sorted(VALID_DECISION_STATUS))})") + + +def check(root: pathlib.Path) -> list[str]: + """Validate every change file and decision; return the list of violation strings.""" + violations: list[str] = [] + changes_dir = root / "changes" + decisions_dir = root / "decisions" + if changes_dir.is_dir(): + for path in sorted(changes_dir.iterdir()): + if path.is_dir(): + violations.append( + f"changes/{path.name}: directory found — convention 2.0.0 uses flat change files " + f"(changes/YYYY-MM-DD.NN-slug.md; see CHANGELOG 2.0.0 for the migration)" + ) + continue + if path.name == "README.md" or path.name.startswith(("_", ".")): + continue + if path.suffix != ".md": + violations.append(f"changes/{path.name}: unexpected non-md file in changes/") + else: + _check_change(path, violations) + if decisions_dir.is_dir(): + for path in sorted(decisions_dir.glob("*.md")): + if path.name == "README.md" or path.name.startswith("_"): + continue + _check_decision(path, violations) + return violations + + +def main(argv: list[str] | None = None, root: pathlib.Path | None = None) -> int: + """Print the listing to stdout, or validate change files and decisions with --check.""" + argv = sys.argv[1:] if argv is None else argv + root = ROOT if root is None else root + if "--check" in argv: + violations = check(root) + if violations: + sys.stderr.write(f"planning: {len(violations)} violation(s)\n") + for violation in violations: + sys.stderr.write(f" - {violation}\n") + return 1 + sys.stdout.write("planning: OK\n") + return 0 + sys.stdout.write(render(load_changes(root), load_decisions(root))) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/pyproject.toml b/pyproject.toml new file mode 100644 index 0000000..1d6308d --- /dev/null +++ b/pyproject.toml @@ -0,0 +1,91 @@ +[project] +name = "chat-app" +version = "0" +description = "Reference chat application for the modern-python organisation" +readme = "readme.md" +requires-python = ">=3.14" +authors = [{ name = "Artur Shiriev", email = "me@shiriev.ru" }] +license = "MIT" +dependencies = [ + "litestar[jwt]", + "lite-bootstrap[litestar-all]", + "modern-di-litestar>=3,<4", + "advanced-alchemy", + "pydantic-settings", + "granian[uvloop]", + "argon2-cffi", + "db-retry", + # database + "alembic", + "psycopg2", + "sqlalchemy[asyncio]", + "asyncpg", + # tracing + "opentelemetry-instrumentation-asyncpg", + "opentelemetry-instrumentation-sqlalchemy", +] + +[dependency-groups] +dev = [ + "polyfactory", + "httpx", + "pytest", + "pytest-cov", + "pytest-asyncio", + "asgi_lifespan", + "modern-di-pytest>=3,<4", +] +lint = ["ruff", "ty", "eof-fixer"] + +[tool.ruff] +fix = true +unsafe-fixes = true +line-length = 120 +target-version = "py314" + +[tool.ruff.lint] +select = ["ALL"] +ignore = [ + "D1", + "FBT", + "INP", + "B008", + "ANN204", + "RUF001", + "D203", + "D213", + "COM812", + "ISC001", + "S105", + "TC001", + "TC002", + "TC003", + "CPY001", # allow missing copyright notice +] +isort.lines-after-imports = 2 +isort.no-lines-before = ["standard-library", "local-folder"] + +[tool.ruff.lint.extend-per-file-ignores] +"tests/*.py" = ["S101", "PLR2004"] +"migrations/*.py" = ["ERA001"] + +[tool.pytest.ini_options] +addopts = "--cov=. --cov-report term-missing --cov-fail-under=100" +asyncio_mode = "auto" +asyncio_default_fixture_loop_scope = "function" +filterwarnings = ["error"] + +[tool.coverage.report] +exclude_also = ["if typing.TYPE_CHECKING:"] + +[tool.coverage.run] +concurrency = ["thread", "greenlet"] +disable_warnings = ["couldnt-parse"] +omit = [ + "migrations/*", + "app/api/__main__.py", + # No __init__.py under planning/ (see the comment atop planning/index.py), so coverage's + # package walk never reaches this file on its own - omit it explicitly rather than relying + # on that as an accident of discovery. + "planning/index.py", +] diff --git a/readme.md b/readme.md new file mode 100644 index 0000000..a143f44 --- /dev/null +++ b/readme.md @@ -0,0 +1,71 @@ +# chat-app + +[![Coverage](https://img.shields.io/badge/coverage-100%25-brightgreen.svg)](https://github.com/modern-python/chat-app/actions/workflows/main.yml) +[![CI](https://github.com/modern-python/chat-app/actions/workflows/main.yml/badge.svg)](https://github.com/modern-python/chat-app/actions/workflows/main.yml) +[![License](https://img.shields.io/github/license/modern-python/chat-app.svg)](https://github.com/modern-python/chat-app/blob/main/LICENSE) +[![GitHub stars](https://img.shields.io/github/stars/modern-python/chat-app)](https://github.com/modern-python/chat-app/stargazers) +[![uv](https://img.shields.io/endpoint?url=https://raw.githubusercontent.com/astral-sh/uv/main/assets/badge/v0.json)](https://github.com/astral-sh/uv) +[![Ruff](https://img.shields.io/endpoint?url=https://raw.githubusercontent.com/astral-sh/ruff/main/assets/badge/v2.json)](https://github.com/astral-sh/ruff) +[![ty](https://img.shields.io/endpoint?url=https://raw.githubusercontent.com/astral-sh/ty/main/assets/badge/v0.json)](https://github.com/astral-sh/ty) + +### Description + +Reference chat application for the `modern-python` organisation: a +single-package Litestar service — JWT cookie auth, direct and group chats, +idempotent message send, cursor-paginated history, per-member read markers +and unread counts — built to show the org's libraries composed on a domain +more realistic than a two-table CRUD template. + +## Key Features + +- tests on `pytest` with automatic rollback after each test case, DI providers + exposed as fixtures via `modern-di-pytest` +- IOC (Inversion of Control) container built on + [modern-di](https://github.com/modern-python/modern-di/), one container for + app- and request-scoped providers +- Observability tools integration built on + [lite-bootstrap](https://github.com/modern-python/lite-bootstrap/) +- Linting and formatting using `ruff` and `ty` +- `Alembic` for DB migrations +- retried, use-case-owned transactions via + [db-retry](https://github.com/modern-python/db-retry/) + +### After `git clone` run + +```bash +just --list +``` + +to see every recipe. `just run` brings up the app and Postgres in Docker +Compose and serves the API on `:8000`. `just test` cycles the database and +runs the full test suite (also via Docker Compose) at 100% coverage. + +## Why this repo + +`litestar-sqlalchemy-template` shows each library in isolation on a two-table +domain. Nothing shows them composed under load-bearing decisions — a +transaction that must span two writes, a unique constraint that two concurrent +requests can both hit, a count that must not cost a row per event. This repo +answers that with a domain that actually needs it. See +`planning/changes/2026-08-21.01-chat-app-bootstrap.md` for the full design and +`architecture/` for the capabilities as shipped. + +| Pattern | Where to look | +|---|---| +| One DI container, app + request scopes | `app/ioc.py` | +| Use case owns the transaction boundary | `app/use_cases/create_message.py` | +| Idempotent write with a concurrent-retry fallback | `app/use_cases/create_message.py` | +| Direct-chat upsert that survives a race | `app/use_cases/create_chat.py` | +| Cursor pagination in both directions | `app/repositories/messages_repository.py` | +| Unread counts without receipt rows | `app/repositories/chats_repository.py` | +| Atomic monotonic read marker | `app/repositories/chat_members_repository.py` | +| Per-test rollback via a container override | `tests/conftest.py` | +| DI providers as pytest fixtures | `tests/use_cases/conftest.py` | +| Simulating a DB race at the repository seam | `tests/use_cases/test_create_chat.py` | + +## 📝 [License](LICENSE) + +## Part of `modern-python` + +Browse the full list of templates and libraries in +[`modern-python`](https://github.com/modern-python) — see the org profile for the categorized index. diff --git a/tests/__init__.py b/tests/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/api/__init__.py b/tests/api/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/api/helpers.py b/tests/api/helpers.py new file mode 100644 index 0000000..e4c3eef --- /dev/null +++ b/tests/api/helpers.py @@ -0,0 +1,35 @@ +import typing +import uuid + +from httpx import AsyncClient + + +async def register(client: AsyncClient, username: str) -> int: + response = await client.post( + "/api/auth/register/", + json={"username": username, "password": "hunter2hunter2", "display_name": username.title()}, + ) + user_id: typing.Final = response.json()["id"] + return user_id + + +async def login(client: AsyncClient, username: str) -> None: + await client.post("/api/auth/login/", json={"username": username, "password": "hunter2hunter2"}) + + +async def create_direct_chat(client: AsyncClient) -> tuple[int, int]: + """Register bob then alice (alice ends up holding the cookie/actor) and open a direct chat.""" + bob_id = await register(client, "bob") + await register(client, "alice") + chat_id: typing.Final = ( + await client.post("/api/chats/", json={"chat_type": "direct", "member_ids": [bob_id]}) + ).json()["id"] + return chat_id, bob_id + + +async def send(client: AsyncClient, chat_id: int, text: str, key: uuid.UUID | None = None) -> dict[str, typing.Any]: + response = await client.post( + f"/api/chats/{chat_id}/messages/", + json={"idempotency_key": str(key or uuid.uuid4()), "text": text}, + ) + return response.json() diff --git a/tests/api/test_auth_api.py b/tests/api/test_auth_api.py new file mode 100644 index 0000000..bc38f69 --- /dev/null +++ b/tests/api/test_auth_api.py @@ -0,0 +1,122 @@ +import pytest +import sqlalchemy as sa +from httpx import AsyncClient +from sqlalchemy.ext.asyncio import AsyncSession + +from app.api.auth import jwt_cookie_auth +from app.database import tables + + +REGISTRATION = {"username": "alice", "password": "hunter2hunter2", "display_name": "Alice"} + + +@pytest.mark.usefixtures("db_session") +async def test_register_returns_user_and_sets_cookie(client: AsyncClient) -> None: + response = await client.post("/api/auth/register/", json=REGISTRATION) + assert response.status_code == 201 + assert response.json()["username"] == "alice" + assert "password" not in response.text + assert "token" in response.cookies + + +@pytest.mark.usefixtures("db_session") +async def test_register_rejects_duplicate_username(client: AsyncClient) -> None: + await client.post("/api/auth/register/", json=REGISTRATION) + response = await client.post("/api/auth/register/", json=REGISTRATION) + assert response.status_code == 409 + + +@pytest.mark.usefixtures("db_session") +async def test_login_succeeds_with_correct_password(client: AsyncClient) -> None: + await client.post("/api/auth/register/", json=REGISTRATION) + response = await client.post( + "/api/auth/login/", + json={"username": "alice", "password": "hunter2hunter2"}, + ) + assert response.status_code == 200 + assert response.json()["display_name"] == "Alice" + + +@pytest.mark.usefixtures("db_session") +async def test_login_rejects_wrong_password(client: AsyncClient) -> None: + await client.post("/api/auth/register/", json=REGISTRATION) + response = await client.post( + "/api/auth/login/", + json={"username": "alice", "password": "wrong-password"}, + ) + assert response.status_code == 401 + + +@pytest.mark.usefixtures("db_session") +async def test_login_rejects_unknown_username(client: AsyncClient) -> None: + response = await client.post( + "/api/auth/login/", + json={"username": "nobody", "password": "hunter2hunter2"}, + ) + assert response.status_code == 401 + + +@pytest.mark.usefixtures("db_session") +async def test_me_requires_authentication(client: AsyncClient) -> None: + response = await client.get("/api/auth/me/") + assert response.status_code == 401 + + +@pytest.mark.usefixtures("db_session") +async def test_me_returns_the_logged_in_user(client: AsyncClient) -> None: + await client.post("/api/auth/register/", json=REGISTRATION) + response = await client.get("/api/auth/me/") + assert response.status_code == 200 + assert response.json()["username"] == "alice" + + +@pytest.mark.usefixtures("db_session") +async def test_logout_clears_the_cookie(client: AsyncClient) -> None: + await client.post("/api/auth/register/", json=REGISTRATION) + logout_response = await client.post("/api/auth/logout/") + assert logout_response.status_code == 204 + me_response = await client.get("/api/auth/me/") + assert me_response.status_code == 401 + + +async def test_password_is_stored_hashed(client: AsyncClient, db_session: AsyncSession) -> None: + await client.post("/api/auth/register/", json=REGISTRATION) + stored = await db_session.scalar(sa.select(tables.UsersTable.password_hash)) + assert stored is not None + assert stored.startswith("$argon2") + + +async def test_me_rejects_tampered_cookie(client: AsyncClient) -> None: + client.cookies.set(jwt_cookie_auth.key, "tampered.not-a-jwt.value") + response = await client.get("/api/auth/me/") + assert response.status_code == 401 + + +@pytest.mark.usefixtures("db_session") +async def test_me_rejects_token_with_non_numeric_subject(client: AsyncClient) -> None: + # Exercises retrieve_user_handler's int(token.sub) ValueError guard: Token only requires + # sub to be a non-empty string, so a forged/malformed subject must 401, not 500. + token = jwt_cookie_auth.create_token(identifier="not-a-number") + client.cookies.set(jwt_cookie_auth.key, token) + response = await client.get("/api/auth/me/") + assert response.status_code == 401 + + +async def test_static_swagger_assets_are_reachable_without_a_cookie(client: AsyncClient) -> None: + response = await client.get("/static/swagger-ui-bundle.js") + assert response.status_code == 200 + + +async def test_metrics_are_reachable_without_a_cookie(client: AsyncClient) -> None: + response = await client.get("/metrics") + assert response.status_code == 200 + + +@pytest.mark.usefixtures("db_session") +async def test_me_rejects_token_for_a_user_that_no_longer_exists(client: AsyncClient) -> None: + # A validly signed token whose subject has no matching row: session.get returns None and + # the middleware must turn that into 401, not treat it as an authenticated request. + token = jwt_cookie_auth.create_token(identifier="999999999") + client.cookies.set(jwt_cookie_auth.key, token) + response = await client.get("/api/auth/me/") + assert response.status_code == 401 diff --git a/tests/api/test_chat_listing_api.py b/tests/api/test_chat_listing_api.py new file mode 100644 index 0000000..da0af7a --- /dev/null +++ b/tests/api/test_chat_listing_api.py @@ -0,0 +1,115 @@ +import pytest +from httpx import AsyncClient + +from tests.api.helpers import create_direct_chat as _create_direct_chat +from tests.api.helpers import login as _login +from tests.api.helpers import register as _register +from tests.api.helpers import send as _send + + +@pytest.mark.usefixtures("db_session") +async def test_listing_returns_only_the_callers_chats(client: AsyncClient) -> None: + chat_id, _ = await _create_direct_chat(client) + await _register(client, "mallory") # mallory is not a member of any chat + + mallory_response = await client.get("/api/chats/") + await _login(client, "alice") + alice_response = await client.get("/api/chats/") + + assert mallory_response.json()["items"] == [] + assert [item["id"] for item in alice_response.json()["items"]] == [chat_id] + + +@pytest.mark.usefixtures("db_session") +async def test_listing_populates_unread_count_and_last_message(client: AsyncClient) -> None: + chat_id, _ = await _create_direct_chat(client) + await _login(client, "bob") + first = await _send(client, chat_id, "one") + await _send(client, chat_id, "two") + await _login(client, "alice") + + response = await client.get("/api/chats/") + + assert response.status_code == 200 + item = response.json()["items"][0] + assert item["unread_count"] == 2 + assert item["last_message"]["id"] != first["id"] + assert item["last_message"]["text"] == "two" + + +@pytest.mark.usefixtures("db_session") +async def test_chat_with_no_messages_has_null_last_message_and_zero_unread(client: AsyncClient) -> None: + await _create_direct_chat(client) + + response = await client.get("/api/chats/") + + item = response.json()["items"][0] + assert item["last_message"] is None + assert item["unread_count"] == 0 + + +@pytest.mark.usefixtures("db_session") +async def test_mark_read_by_non_member_is_forbidden(client: AsyncClient) -> None: + chat_id, _ = await _create_direct_chat(client) + await _register(client, "mallory") + + response = await client.post(f"/api/chats/{chat_id}/read/", json={"last_read_message_id": 1}) + + assert response.status_code == 403 + + +@pytest.mark.usefixtures("db_session") +async def test_mark_read_clears_unread_count(client: AsyncClient) -> None: + chat_id, _ = await _create_direct_chat(client) + await _login(client, "bob") + message = await _send(client, chat_id, "one") + await _login(client, "alice") + + read_response = await client.post(f"/api/chats/{chat_id}/read/", json={"last_read_message_id": message["id"]}) + listing = await client.get("/api/chats/") + + assert read_response.status_code == 200 + assert read_response.json()["last_read_message_id"] == message["id"] + assert listing.json()["items"][0]["unread_count"] == 0 + + +@pytest.mark.usefixtures("db_session") +async def test_marking_read_with_a_lower_id_leaves_the_marker_unchanged(client: AsyncClient) -> None: + chat_id, _ = await _create_direct_chat(client) + await _login(client, "bob") + first = await _send(client, chat_id, "one") + second = await _send(client, chat_id, "two") + await _login(client, "alice") + await client.post(f"/api/chats/{chat_id}/read/", json={"last_read_message_id": second["id"]}) + + response = await client.post(f"/api/chats/{chat_id}/read/", json={"last_read_message_id": first["id"]}) + + assert response.status_code == 200 + assert response.json()["last_read_message_id"] == second["id"] + + +@pytest.mark.usefixtures("db_session") +async def test_mark_read_rejects_a_message_id_from_a_different_chat(client: AsyncClient) -> None: + chat_id, bob_id = await _create_direct_chat(client) + other_message = await _send(client, chat_id, "in the direct chat") + carol_id = await _register(client, "carol") + await _login(client, "alice") + group_chat_id = ( + await client.post("/api/chats/", json={"chat_type": "group", "member_ids": [bob_id, carol_id], "title": "g"}) + ).json()["id"] + + response = await client.post( + f"/api/chats/{group_chat_id}/read/", json={"last_read_message_id": other_message["id"]} + ) + + assert response.status_code == 400 + + +async def test_list_chats_requires_authentication(client: AsyncClient) -> None: + response = await client.get("/api/chats/") + assert response.status_code == 401 + + +async def test_mark_read_requires_authentication(client: AsyncClient) -> None: + response = await client.post("/api/chats/1/read/", json={"last_read_message_id": 1}) + assert response.status_code == 401 diff --git a/tests/api/test_chats_api.py b/tests/api/test_chats_api.py new file mode 100644 index 0000000..d8a4377 --- /dev/null +++ b/tests/api/test_chats_api.py @@ -0,0 +1,77 @@ +import pytest +from httpx import AsyncClient + +from tests.api.helpers import register as _register + + +@pytest.mark.usefixtures("db_session") +async def test_create_group_chat_returns_all_members(client: AsyncClient) -> None: + bob_id = await _register(client, "bob") + await _register(client, "alice") + response = await client.post( + "/api/chats/", + json={"chat_type": "group", "member_ids": [bob_id], "title": "Team"}, + ) + assert response.status_code == 201 + assert response.json()["title"] == "Team" + assert response.json()["chat_type"] == "group" + assert len(response.json()["members"]) == 2 + + +@pytest.mark.usefixtures("db_session") +async def test_create_direct_chat_twice_returns_the_same_chat(client: AsyncClient) -> None: + bob_id = await _register(client, "bob") + await _register(client, "alice") + first = await client.post("/api/chats/", json={"chat_type": "direct", "member_ids": [bob_id]}) + second = await client.post("/api/chats/", json={"chat_type": "direct", "member_ids": [bob_id]}) + assert first.status_code == 201 + assert second.status_code == 200 + assert first.json()["id"] == second.json()["id"] + + +@pytest.mark.usefixtures("db_session") +async def test_create_chat_rejects_unknown_member_id(client: AsyncClient) -> None: + await _register(client, "alice") + response = await client.post( + "/api/chats/", + json={"chat_type": "group", "member_ids": [999999]}, + ) + assert response.status_code == 400 + + +@pytest.mark.usefixtures("db_session") +async def test_create_direct_chat_rejects_more_than_two_members(client: AsyncClient) -> None: + bob_id = await _register(client, "bob") + mallory_id = await _register(client, "mallory") + await _register(client, "alice") # registers last -> alice holds the cookie and is the actor + response = await client.post( + "/api/chats/", + json={"chat_type": "direct", "member_ids": [bob_id, mallory_id]}, + ) + assert response.status_code == 400 + + +@pytest.mark.usefixtures("db_session") +async def test_get_chat_returns_the_chat_for_a_member(client: AsyncClient) -> None: + bob_id = await _register(client, "bob") + await _register(client, "alice") + chat_id = (await client.post("/api/chats/", json={"chat_type": "direct", "member_ids": [bob_id]})).json()["id"] + response = await client.get(f"/api/chats/{chat_id}/") + assert response.status_code == 200 + assert response.json()["id"] == chat_id + + +@pytest.mark.usefixtures("db_session") +async def test_get_chat_rejects_non_member(client: AsyncClient) -> None: + bob_id = await _register(client, "bob") + await _register(client, "alice") + chat_id = (await client.post("/api/chats/", json={"chat_type": "direct", "member_ids": [bob_id]})).json()["id"] + await _register(client, "mallory") + response = await client.get(f"/api/chats/{chat_id}/") + assert response.status_code == 403 + + +@pytest.mark.usefixtures("db_session") +async def test_create_chat_requires_authentication(client: AsyncClient) -> None: + response = await client.post("/api/chats/", json={"chat_type": "group", "member_ids": [1]}) + assert response.status_code == 401 diff --git a/tests/api/test_message_mutations_api.py b/tests/api/test_message_mutations_api.py new file mode 100644 index 0000000..b906c70 --- /dev/null +++ b/tests/api/test_message_mutations_api.py @@ -0,0 +1,124 @@ +import pytest +from httpx import AsyncClient + +from tests.api.helpers import create_direct_chat as _create_direct_chat +from tests.api.helpers import login as _login +from tests.api.helpers import register as _register +from tests.api.helpers import send as _send + + +@pytest.mark.usefixtures("db_session") +async def test_author_can_edit_message(client: AsyncClient) -> None: + chat_id, _ = await _create_direct_chat(client) + message = await _send(client, chat_id, "hi") + + response = await client.patch(f"/api/messages/{message['id']}/", json={"text": "fixed"}) + + assert response.status_code == 200 + assert response.json()["text"] == "fixed" + assert response.json()["edited_at"] is not None + + +@pytest.mark.usefixtures("db_session") +async def test_non_author_member_cannot_edit_message(client: AsyncClient) -> None: + chat_id, _ = await _create_direct_chat(client) + message = await _send(client, chat_id, "hi") + await _login(client, "bob") # bob is a member of the chat but not the author + + response = await client.patch(f"/api/messages/{message['id']}/", json={"text": "nope"}) + + assert response.status_code == 403 + + +@pytest.mark.usefixtures("db_session") +async def test_non_member_cannot_edit_message(client: AsyncClient) -> None: + chat_id, _ = await _create_direct_chat(client) + message = await _send(client, chat_id, "hi") + await _register(client, "mallory") # mallory is not in the chat at all + + response = await client.patch(f"/api/messages/{message['id']}/", json={"text": "nope"}) + + assert response.status_code == 403 + + +@pytest.mark.usefixtures("db_session") +async def test_author_can_delete_message(client: AsyncClient) -> None: + chat_id, _ = await _create_direct_chat(client) + message = await _send(client, chat_id, "hi") + + response = await client.delete(f"/api/messages/{message['id']}/") + + assert response.status_code == 204 + + +@pytest.mark.usefixtures("db_session") +async def test_non_author_member_cannot_delete_message(client: AsyncClient) -> None: + chat_id, _ = await _create_direct_chat(client) + message = await _send(client, chat_id, "hi") + await _login(client, "bob") # bob is a member of the chat but not the author + + response = await client.delete(f"/api/messages/{message['id']}/") + + assert response.status_code == 403 + + +@pytest.mark.usefixtures("db_session") +async def test_non_member_cannot_delete_message(client: AsyncClient) -> None: + chat_id, _ = await _create_direct_chat(client) + message = await _send(client, chat_id, "hi") + await _register(client, "mallory") # mallory is not in the chat at all + + response = await client.delete(f"/api/messages/{message['id']}/") + + assert response.status_code == 403 + + +@pytest.mark.usefixtures("db_session") +async def test_editing_a_missing_message_returns_404(client: AsyncClient) -> None: + await _register(client, "alice") + + response = await client.patch("/api/messages/999999/", json={"text": "nope"}) + + assert response.status_code == 404 + + +@pytest.mark.usefixtures("db_session") +async def test_deleting_a_missing_message_returns_404(client: AsyncClient) -> None: + await _register(client, "alice") + + response = await client.delete("/api/messages/999999/") + + assert response.status_code == 404 + + +async def test_edit_message_requires_authentication(client: AsyncClient) -> None: + response = await client.patch("/api/messages/1/", json={"text": "nope"}) + assert response.status_code == 401 + + +async def test_delete_message_requires_authentication(client: AsyncClient) -> None: + response = await client.delete("/api/messages/1/") + assert response.status_code == 401 + + +@pytest.mark.usefixtures("db_session") +async def test_editing_a_deleted_message_returns_409(client: AsyncClient) -> None: + chat_id, _ = await _create_direct_chat(client) + message = await _send(client, chat_id, "hi") + await client.delete(f"/api/messages/{message['id']}/") + + response = await client.patch(f"/api/messages/{message['id']}/", json={"text": "nope"}) + + assert response.status_code == 409 + + +@pytest.mark.usefixtures("db_session") +async def test_deleting_an_already_deleted_message_returns_204(client: AsyncClient) -> None: + chat_id, _ = await _create_direct_chat(client) + message = await _send(client, chat_id, "hi") + first = await client.delete(f"/api/messages/{message['id']}/") + + second = await client.delete(f"/api/messages/{message['id']}/") + + assert first.status_code == 204 + assert second.status_code == 204 diff --git a/tests/api/test_messages_api.py b/tests/api/test_messages_api.py new file mode 100644 index 0000000..cbabf57 --- /dev/null +++ b/tests/api/test_messages_api.py @@ -0,0 +1,107 @@ +import uuid + +import pytest +from httpx import AsyncClient + +from app.use_cases.fetch_messages import MAX_PAGE_SIZE +from tests.api.helpers import create_direct_chat as _create_direct_chat +from tests.api.helpers import register as _register +from tests.api.helpers import send as _send + + +@pytest.mark.usefixtures("db_session") +async def test_send_message_returns_201(client: AsyncClient) -> None: + chat_id, _ = await _create_direct_chat(client) + response = await client.post( + f"/api/chats/{chat_id}/messages/", + json={"idempotency_key": str(uuid.uuid4()), "text": "hi"}, + ) + assert response.status_code == 201 + assert response.json()["text"] == "hi" + + +@pytest.mark.usefixtures("db_session") +async def test_resending_the_same_key_returns_200_with_the_same_id(client: AsyncClient) -> None: + chat_id, _ = await _create_direct_chat(client) + key = uuid.uuid4() + first = await client.post(f"/api/chats/{chat_id}/messages/", json={"idempotency_key": str(key), "text": "hi"}) + second = await client.post( + f"/api/chats/{chat_id}/messages/", json={"idempotency_key": str(key), "text": "hi again"} + ) + assert first.status_code == 201 + assert second.status_code == 200 + assert first.json()["id"] == second.json()["id"] + + +@pytest.mark.usefixtures("db_session") +async def test_before_id_returns_newest_first_and_excludes_the_cursor_row(client: AsyncClient) -> None: + chat_id, _ = await _create_direct_chat(client) + first = await _send(client, chat_id, "one") + second = await _send(client, chat_id, "two") + third = await _send(client, chat_id, "three") + + response = await client.get(f"/api/chats/{chat_id}/messages/", params={"before_id": third["id"]}) + + assert response.status_code == 200 + ids = [item["id"] for item in response.json()["items"]] + assert ids == [second["id"], first["id"]] + + +@pytest.mark.usefixtures("db_session") +async def test_after_id_returns_oldest_first_and_excludes_the_cursor_row(client: AsyncClient) -> None: + chat_id, _ = await _create_direct_chat(client) + first = await _send(client, chat_id, "one") + second = await _send(client, chat_id, "two") + third = await _send(client, chat_id, "three") + + response = await client.get(f"/api/chats/{chat_id}/messages/", params={"after_id": first["id"]}) + + assert response.status_code == 200 + ids = [item["id"] for item in response.json()["items"]] + assert ids == [second["id"], third["id"]] + + +@pytest.mark.usefixtures("db_session") +async def test_non_member_is_rejected_on_send_and_list(client: AsyncClient) -> None: + chat_id, _ = await _create_direct_chat(client) + await _register(client, "mallory") + + send_response = await client.post( + f"/api/chats/{chat_id}/messages/", json={"idempotency_key": str(uuid.uuid4()), "text": "hi"} + ) + list_response = await client.get(f"/api/chats/{chat_id}/messages/") + + assert send_response.status_code == 403 + assert list_response.status_code == 403 + + +@pytest.mark.usefixtures("db_session") +async def test_both_cursors_together_are_rejected(client: AsyncClient) -> None: + chat_id, _ = await _create_direct_chat(client) + await _send(client, chat_id, "one") + + response = await client.get(f"/api/chats/{chat_id}/messages/", params={"before_id": 1, "after_id": 1}) + + assert response.status_code == 400 + + +@pytest.mark.usefixtures("db_session") +async def test_limit_above_max_page_size_is_clamped(client: AsyncClient) -> None: + chat_id, _ = await _create_direct_chat(client) + for i in range(MAX_PAGE_SIZE + 5): + await _send(client, chat_id, f"message {i}") + + response = await client.get(f"/api/chats/{chat_id}/messages/", params={"limit": MAX_PAGE_SIZE + 50}) + + assert response.status_code == 200 + assert len(response.json()["items"]) == MAX_PAGE_SIZE + + +@pytest.mark.usefixtures("db_session") +async def test_negative_limit_is_rejected(client: AsyncClient) -> None: + chat_id, _ = await _create_direct_chat(client) + await _send(client, chat_id, "one") + + response = await client.get(f"/api/chats/{chat_id}/messages/", params={"limit": -1}) + + assert response.status_code == 400 diff --git a/tests/conftest.py b/tests/conftest.py new file mode 100644 index 0000000..11af99e --- /dev/null +++ b/tests/conftest.py @@ -0,0 +1,55 @@ +import typing + +import litestar +import modern_di +import modern_di_litestar +import pytest +from asgi_lifespan import LifespanManager +from httpx import ASGITransport, AsyncClient +from sqlalchemy.ext.asyncio import AsyncSession + +from app import ioc +from app.api.app import build_app +from app.database.resources import create_database_engine, create_session + + +@pytest.fixture +async def app() -> typing.AsyncIterator[litestar.Litestar]: + app_ = build_app() + async with LifespanManager(app_): # ty: ignore[invalid-argument-type] + yield app_ + + +@pytest.fixture +async def client(app: litestar.Litestar) -> typing.AsyncIterator[AsyncClient]: + async with AsyncClient( + transport=ASGITransport(app=app), # ty: ignore[invalid-argument-type] + base_url="http://test", + ) as client_: + yield client_ + + +@pytest.fixture +async def di_container(app: litestar.Litestar) -> typing.AsyncIterator[modern_di.Container]: + container = modern_di_litestar.fetch_di_container(app) + try: + yield container + finally: + await container.close_async() + + +@pytest.fixture +async def db_session(di_container: modern_di.Container) -> typing.AsyncIterator[AsyncSession]: + engine = create_database_engine() + connection = await engine.connect() + transaction = await connection.begin() + di_container.override(ioc.Database.database_engine, connection) + + try: + yield create_session(connection) + finally: + if connection.in_transaction(): + await transaction.rollback() + await connection.close() + await engine.dispose() + di_container.reset_override(ioc.Database.database_engine) diff --git a/tests/factories.py b/tests/factories.py new file mode 100644 index 0000000..bf210c9 --- /dev/null +++ b/tests/factories.py @@ -0,0 +1,10 @@ +from polyfactory.factories.sqlalchemy_factory import SQLAlchemyFactory + +from app.database import tables + + +class UserFactory(SQLAlchemyFactory[tables.UsersTable]): + __set_association_proxy__ = False + __set_relationships__ = False + __check_model__ = False + id = None diff --git a/tests/test_main.py b/tests/test_main.py new file mode 100644 index 0000000..efd97cb --- /dev/null +++ b/tests/test_main.py @@ -0,0 +1,87 @@ +import modern_di +import sqlalchemy as sa +from advanced_alchemy.exceptions import NotFoundError +from httpx import AsyncClient +from sqlalchemy.ext.asyncio import AsyncSession +from sqlalchemy.pool import QueuePool + +from app import ioc +from app.api import exception_handlers +from app.database.resources import close_database_engine, create_database_engine +from app.database.tables import UsersTable +from app.exceptions import PermissionDeniedError +from app.settings import settings +from tests.factories import UserFactory + + +async def test_health_check_returns_ok(client: AsyncClient) -> None: + response = await client.get("/health/") + assert response.status_code == 200 + + +async def test_openapi_schema_is_served(client: AsyncClient) -> None: + response = await client.get("/docs/openapi.json") + assert response.status_code == 200 + assert response.json()["info"]["title"] == "chat-app" + + +async def test_not_found_error_handler_returns_404() -> None: + response = exception_handlers.not_found_error_handler(object(), NotFoundError()) + assert response.status_code == 404 + assert response.content == {"detail": "Not found"} + + +async def test_permission_denied_handler_uses_exception_message() -> None: + response = exception_handlers.permission_denied_handler(object(), PermissionDeniedError("nope")) + assert response.status_code == 403 + assert response.content == {"detail": "nope"} + + +async def test_permission_denied_handler_defaults_message_when_empty() -> None: + response = exception_handlers.permission_denied_handler(object(), PermissionDeniedError()) + assert response.content == {"detail": "Permission denied"} + + +async def test_create_database_engine_reads_settings_and_can_be_disposed() -> None: + # Exercises `close_database_engine` too: nothing else calls it, since the `db_session` + # fixture always overrides `Database.database_engine` before it is ever resolved, so its + # cache finalizer never fires. + engine = create_database_engine() + try: + assert isinstance(engine.pool, QueuePool) + assert engine.pool.size() == settings.db_pool_size + assert engine.url.database == settings.db_dsn_parsed.database + finally: + await close_database_engine(engine) + + +async def test_db_session_insert_is_visible_within_test(db_session: AsyncSession) -> None: + user = UserFactory.build() + db_session.add(user) + await db_session.commit() + + result = await db_session.scalars(sa.select(UsersTable)) + assert len(result.all()) == 1 + + +async def test_db_session_rolls_back_between_tests(db_session: AsyncSession) -> None: + result = await db_session.scalars(sa.select(UsersTable)) + assert result.all() == [] + + +async def test_di_resolved_session_shares_the_overridden_connection( + di_container: modern_di.Container, + db_session: AsyncSession, +) -> None: + # Proves the load-bearing part of the `db_session` fixture: a request-scoped session + # resolved through the real DI provider (`create_session`/`close_session`, the path + # production route handlers use) sees writes made on the fixture's own session, because + # both share the connection that `db_session` overrode `Database.database_engine` with. + user = UserFactory.build() + db_session.add(user) + await db_session.flush() + + async with di_container.build_child_container(scope=modern_di.Scope.REQUEST) as request_container: + resolved_session = request_container.resolve_provider(ioc.Database.database_session) + result = await resolved_session.scalars(sa.select(UsersTable).where(UsersTable.username == user.username)) + assert result.one().id == user.id diff --git a/tests/test_schemas.py b/tests/test_schemas.py new file mode 100644 index 0000000..3d76f28 --- /dev/null +++ b/tests/test_schemas.py @@ -0,0 +1,10 @@ +from app.schemas.api import Collection, User +from tests.factories import UserFactory + + +def test_collection_builds_from_models() -> None: + # Every real caller passes SQLAlchemy ORM rows, which is why `Base` sets + # from_attributes=True; a dict here wouldn't exercise the attribute-access path. + user = UserFactory.build(id=1, username="alice", display_name="Alice") + collection = Collection[User].from_models([user]) + assert collection.items == [User(id=1, username="alice", display_name="Alice")] diff --git a/tests/test_security.py b/tests/test_security.py new file mode 100644 index 0000000..23fc7d1 --- /dev/null +++ b/tests/test_security.py @@ -0,0 +1,19 @@ +from app.security import hash_password, verify_password + + +def test_hash_is_not_the_plaintext() -> None: + hashed = hash_password("hunter2") + assert hashed != "hunter2" + assert hashed.startswith("$argon2") + + +def test_verify_accepts_correct_password() -> None: + assert verify_password(hash_password("hunter2"), "hunter2") is True + + +def test_verify_rejects_wrong_password() -> None: + assert verify_password(hash_password("hunter2"), "hunter3") is False + + +def test_verify_rejects_malformed_hash() -> None: + assert verify_password("not-a-hash", "hunter2") is False diff --git a/tests/test_settings.py b/tests/test_settings.py new file mode 100644 index 0000000..87e3209 --- /dev/null +++ b/tests/test_settings.py @@ -0,0 +1,32 @@ +import pytest + +from app.settings import INSECURE_JWT_SECRET, Settings + + +def test_db_dsn_parsed_exposes_driver() -> None: + settings = Settings(db_dsn="postgresql+asyncpg://user:pw@host/dbname") + assert settings.db_dsn_parsed.drivername == "postgresql+asyncpg" + assert settings.db_dsn_parsed.database == "dbname" + + +def test_api_bootstrapper_config_carries_service_identity() -> None: + settings = Settings(service_name="svc", service_version="9.9.9") + config = settings.api_bootstrapper_config + assert config.service_name == "svc" + assert config.service_version == "9.9.9" + + +def test_ensure_jwt_secret_is_configured_allows_the_default_secret_locally() -> None: + Settings(service_environment="local").ensure_jwt_secret_is_configured() + + +def test_ensure_jwt_secret_is_configured_allows_a_real_secret_outside_local() -> None: + Settings(service_environment="production", jwt_secret="a-real-secret").ensure_jwt_secret_is_configured() # noqa: S106 + + +def test_ensure_jwt_secret_is_configured_rejects_the_default_secret_outside_local() -> None: + # jwt_secret set explicitly (rather than left to the JWT_SECRET env var, which the test + # container sets) to isolate this test from the environment it happens to run in. + settings = Settings(service_environment="production", jwt_secret=INSECURE_JWT_SECRET) + with pytest.raises(RuntimeError, match="jwt_secret"): + settings.ensure_jwt_secret_is_configured() diff --git a/tests/use_cases/__init__.py b/tests/use_cases/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/use_cases/conftest.py b/tests/use_cases/conftest.py new file mode 100644 index 0000000..ecb8648 --- /dev/null +++ b/tests/use_cases/conftest.py @@ -0,0 +1,89 @@ +import typing +import uuid + +import modern_di +import pytest +from modern_di_pytest import expose +from sqlalchemy.ext.asyncio import AsyncSession + +from app import ioc, security +from app.database import tables +from app.schemas import api as schemas +from app.use_cases.create_chat import CreateChatUseCase +from app.use_cases.create_message import CreateMessageUseCase +from tests.factories import UserFactory + + +@pytest.fixture +async def request_container( + di_container: modern_di.Container, + db_session: AsyncSession, # noqa: ARG001 - forces db_session's engine override to run first +) -> typing.AsyncIterator[modern_di.Container]: + async with di_container.build_child_container(scope=modern_di.Scope.REQUEST) as container: + yield container + + +# One pytest fixture per provider on both groups, named after the class attribute. +# Every use case and repository added in later tasks becomes a fixture automatically, +# so no test file has to hand-assemble dependencies. +expose(ioc.Repositories, ioc.UseCases, container_fixture="request_container") + + +async def _make_user(session: AsyncSession, username: str) -> tables.UsersTable: + user: typing.Final = UserFactory.build( + username=username, + password_hash=security.hash_password("hunter2hunter2"), + display_name=username.title(), + ) + session.add(user) + await session.flush() + return user + + +@pytest.fixture +async def alice(db_session: AsyncSession) -> tables.UsersTable: + return await _make_user(db_session, "alice") + + +@pytest.fixture +async def bob(db_session: AsyncSession) -> tables.UsersTable: + return await _make_user(db_session, "bob") + + +@pytest.fixture +async def carol(db_session: AsyncSession) -> tables.UsersTable: + return await _make_user(db_session, "carol") + + +@pytest.fixture +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]) + ) + return chat + + +@pytest.fixture +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") + ) + return message + + +@pytest.fixture +def send( + create_message_use_case: CreateMessageUseCase, +) -> typing.Callable[[tables.UsersTable, int, str], typing.Awaitable[tuple[tables.MessagesTable, bool]]]: + """Send a message with a fresh idempotency key per call, so callers never collide on retries.""" + + 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) + ) + + return _send diff --git a/tests/use_cases/test_create_chat.py b/tests/use_cases/test_create_chat.py new file mode 100644 index 0000000..c549ff5 --- /dev/null +++ b/tests/use_cases/test_create_chat.py @@ -0,0 +1,176 @@ +import pytest +from advanced_alchemy.exceptions import DuplicateKeyError + +from app.database import tables +from app.exceptions import ValidationError +from app.repositories.chats_repository import ChatsRepository +from app.schemas import api as schemas +from app.use_cases.create_chat import CreateChatUseCase + + +class _RacingChatsRepository(ChatsRepository): + """Simulates losing a create-direct-chat race. + + The pre-check misses (as if the winner's row weren't committed/visible yet), the insert + then collides with the winner's now-committed row (DuplicateKeyError), and the recovery + re-read must find it. + """ + + _missed_precheck: bool = False + + async def fetch_direct_by_key(self, direct_key: str) -> tables.ChatsTable | None: + if not self._missed_precheck: + self._missed_precheck = True + return None + return await super().fetch_direct_by_key(direct_key) + + async def create(self, *_args: object, **_kwargs: object) -> tables.ChatsTable: + msg = "simulated race: another request already created this direct chat" + raise DuplicateKeyError(msg) + + +class _NeverFoundChatsRepository(ChatsRepository): + """Simulates a race whose recovery re-read can never find the winner's row. + + Both `fetch_direct_by_key` calls (the pre-check and the post-rollback recovery re-read) + return `None`, and `create()` always raises `DuplicateKeyError` - a state the unique + constraint on `direct_key` should make unreachable in production, exercised here only to + prove `CreateChatUseCase` raises `RuntimeError` rather than returning `None` silently. + """ + + async def fetch_direct_by_key(self, direct_key: str) -> tables.ChatsTable | None: # noqa: ARG002 + return None + + async def create(self, *_args: object, **_kwargs: object) -> tables.ChatsTable: + msg = "simulated race: another request already created this direct chat" + raise DuplicateKeyError(msg) + + +class _AlwaysDuplicateChatsRepository(ChatsRepository): + """Stub whose create() always raises DuplicateKeyError. + + Simulates an unexpected unique-constraint violation at the seam the real repository + would raise it from. + """ + + async def create(self, *_args: object, **_kwargs: object) -> tables.ChatsTable: + msg = "simulated duplicate key" + raise DuplicateKeyError(msg) + + +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]) + ) + assert created is True + assert chat.chat_type is tables.ChatType.DIRECT + assert chat.direct_key == tables.build_direct_key(alice.id, bob.id) + assert {member.user_id for member in chat.members} == {alice.id, bob.id} + + +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]) + ) + second, second_created = await create_chat_use_case( + bob, schemas.CreateChatRequest(chat_type=tables.ChatType.DIRECT, member_ids=[alice.id]) + ) + assert first.id == second.id + assert first_created is True + assert second_created is False + # `second` is returned from the early-return, no-write path (existing direct chat found). + # Its relationship must still be readable without triggering a lazy load on a closed/rolled-back session. + assert {member.user_id for member in second.members} == {alice.id, bob.id} + + +async def test_direct_chat_creation_recovers_from_a_concurrent_duplicate_key( + create_chat_use_case: CreateChatUseCase, alice: tables.UsersTable, bob: tables.UsersTable +) -> 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]) + ) + assert winner_created is True + # Captured now, not read off `winner` after the racer runs: the racer shares this session, + # and its own recovery `rollback()` expires every object already loaded on that session - + # including `winner` - exactly the hazard the surrounding comments describe, just now + # crossing between two calls that happen to share a session instead of within one call. + winner_id = winner.id + + # The losing side of the race, sharing `create_chat_use_case`'s own transaction/session so + # the winner row (committed above) is visible to the recovery re-read. + racer = CreateChatUseCase( + transaction=create_chat_use_case.transaction, + chats_repository=_RacingChatsRepository( + session=create_chat_use_case.chats_repository.repository.session, auto_commit=False + ), + 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]) + ) + assert loser_created is False + assert loser.id == winner_id + + +async def test_direct_chat_recovery_raises_if_the_winners_row_is_unreadable( + create_chat_use_case: CreateChatUseCase, alice: tables.UsersTable, bob: tables.UsersTable +) -> None: + broken = CreateChatUseCase( + transaction=create_chat_use_case.transaction, + chats_repository=_NeverFoundChatsRepository( + session=create_chat_use_case.chats_repository.repository.session, auto_commit=False + ), + 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])) + + +async def test_group_chat_reraises_an_unexpected_duplicate_key( + create_chat_use_case: CreateChatUseCase, alice: tables.UsersTable, bob: tables.UsersTable +) -> None: + # Group chats have no unique constraint to race on `chats`; a DuplicateKeyError there is + # unexpected and must propagate (mapping to the standard 409), not be funnelled into the + # direct-chat recovery path. + broken = CreateChatUseCase( + transaction=create_chat_use_case.transaction, + chats_repository=_AlwaysDuplicateChatsRepository( + session=create_chat_use_case.chats_repository.repository.session, auto_commit=False + ), + 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])) + + +async def test_direct_chat_rejects_more_than_two_members( + create_chat_use_case: CreateChatUseCase, + alice: tables.UsersTable, + bob: tables.UsersTable, + carol: tables.UsersTable, +) -> None: + with pytest.raises(ValidationError): + await create_chat_use_case( + alice, + schemas.CreateChatRequest(chat_type=tables.ChatType.DIRECT, member_ids=[bob.id, carol.id]), + ) + + +async def test_group_chat_includes_the_creator( + create_chat_use_case: CreateChatUseCase, + alice: tables.UsersTable, + bob: tables.UsersTable, + 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"), + ) + assert created is True + assert chat.direct_key is None + assert {member.user_id for member in chat.members} == {alice.id, bob.id, carol.id} diff --git a/tests/use_cases/test_create_message.py b/tests/use_cases/test_create_message.py new file mode 100644 index 0000000..088b115 --- /dev/null +++ b/tests/use_cases/test_create_message.py @@ -0,0 +1,175 @@ +import uuid + +import pytest +from advanced_alchemy.exceptions import DuplicateKeyError + +from app.database import tables +from app.exceptions import PermissionDeniedError +from app.repositories.chats_repository import ChatsRepository +from app.repositories.messages_repository import MessagesRepository +from app.schemas import api as schemas +from app.use_cases.create_chat import CreateChatUseCase +from app.use_cases.create_message import CreateMessageUseCase + + +class _RacingMessagesRepository(MessagesRepository): + """Simulate losing a concurrent-send-with-the-same-key race. + + The pre-check misses (as if the winner's row weren't committed/visible yet), the insert + then collides with the winner's now-committed row (DuplicateKeyError), and the recovery + re-read must find it. + """ + + _missed_precheck: bool = False + + async def fetch_by_idempotency_key(self, chat_id: int, idempotency_key: uuid.UUID) -> tables.MessagesTable | None: + if not self._missed_precheck: + self._missed_precheck = True + return None + return await super().fetch_by_idempotency_key(chat_id, idempotency_key) + + async def create(self, *_args: object, **_kwargs: object) -> tables.MessagesTable: + msg = "simulated race: another request already sent this message" + raise DuplicateKeyError(msg) + + +class _NeverFoundMessagesRepository(MessagesRepository): + """Simulates a race whose recovery re-read can never find the winner's row. + + Both `fetch_by_idempotency_key` calls (the pre-check and the post-rollback recovery + re-read) return `None`, and `create()` always raises `DuplicateKeyError` - a state the + unique constraint on `(chat_id, idempotency_key)` should make unreachable in production, + exercised here only to prove `CreateMessageUseCase` raises `RuntimeError` rather than + returning `None` silently. + """ + + async def fetch_by_idempotency_key( + self, + chat_id: int, # noqa: ARG002 + idempotency_key: uuid.UUID, # noqa: ARG002 + ) -> tables.MessagesTable | None: + return None + + async def create(self, *_args: object, **_kwargs: object) -> tables.MessagesTable: + msg = "simulated race: another request already sent this message" + raise DuplicateKeyError(msg) + + +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") + ) + assert created is True + assert message.text == "hi" + + +async def test_repeated_idempotency_key_returns_the_same_message( + create_message_use_case: CreateMessageUseCase, direct_chat: tables.ChatsTable, alice: tables.UsersTable +) -> None: + key = uuid.uuid4() + first, first_created = await create_message_use_case( + alice, direct_chat.id, 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") + ) + assert first_created is True + assert second_created is False + assert first.id == second.id + assert second.text == "hi" + + +async def test_send_updates_chat_last_message_id( + create_message_use_case: CreateMessageUseCase, + chats_repository: ChatsRepository, + direct_chat: tables.ChatsTable, + alice: tables.UsersTable, +) -> None: + message, _ = await create_message_use_case( + alice, direct_chat.id, 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 + + +async def test_non_member_cannot_send( + create_message_use_case: CreateMessageUseCase, direct_chat: tables.ChatsTable, carol: tables.UsersTable +) -> None: + with pytest.raises(PermissionDeniedError): + await create_message_use_case( + carol, direct_chat.id, schemas.SendMessageRequest(idempotency_key=uuid.uuid4(), text="hi") + ) + + +async def test_concurrent_duplicate_key_recovers_the_winners_message( + create_message_use_case: CreateMessageUseCase, direct_chat: tables.ChatsTable, alice: tables.UsersTable +) -> None: + key = uuid.uuid4() + winner, winner_created = await create_message_use_case( + alice, direct_chat.id, schemas.SendMessageRequest(idempotency_key=key, text="hi") + ) + assert winner_created is True + winner_id = winner.id + + # Shares the winner's session/transaction so the committed row is visible to the recovery + # re-read, same setup as CreateChatUseCase's equivalent race test. + racer = CreateMessageUseCase( + transaction=create_message_use_case.transaction, + chats_repository=create_message_use_case.chats_repository, + chat_members_repository=create_message_use_case.chat_members_repository, + messages_repository=_RacingMessagesRepository( + session=create_message_use_case.messages_repository.repository.session, auto_commit=False + ), + ) + loser, loser_created = await racer( + alice, direct_chat.id, schemas.SendMessageRequest(idempotency_key=key, text="hi again") + ) + assert loser_created is False + assert loser.id == winner_id + assert loser.text == "hi" + + +async def test_send_recovery_raises_if_the_winners_row_is_unreadable( + create_message_use_case: CreateMessageUseCase, direct_chat: tables.ChatsTable, alice: tables.UsersTable +) -> None: + broken = CreateMessageUseCase( + transaction=create_message_use_case.transaction, + chats_repository=create_message_use_case.chats_repository, + chat_members_repository=create_message_use_case.chat_members_repository, + messages_repository=_NeverFoundMessagesRepository( + session=create_message_use_case.messages_repository.repository.session, auto_commit=False + ), + ) + with pytest.raises(RuntimeError, match="could not be found"): + await broken(alice, direct_chat.id, schemas.SendMessageRequest(idempotency_key=uuid.uuid4(), text="hi")) + + +async def test_same_idempotency_key_in_two_different_chats_creates_two_messages( + create_message_use_case: CreateMessageUseCase, + create_chat_use_case: CreateChatUseCase, + direct_chat: tables.ChatsTable, + alice: tables.UsersTable, + carol: tables.UsersTable, +) -> None: + # Idempotency is scoped per (chat_id, idempotency_key): the key identifies a retry of + # "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]) + ) + key = uuid.uuid4() + + first, first_created = await create_message_use_case( + alice, direct_chat.id, 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") + ) + + assert first_created is True + assert second_created is True + assert first.id != second.id + assert first.chat_id == direct_chat.id + assert second.chat_id == other_chat.id diff --git a/tests/use_cases/test_edit_message.py b/tests/use_cases/test_edit_message.py new file mode 100644 index 0000000..b985972 --- /dev/null +++ b/tests/use_cases/test_edit_message.py @@ -0,0 +1,145 @@ +import uuid + +import pytest + +from app.database import tables +from app.exceptions import ConflictError, PermissionDeniedError +from app.repositories.chat_members_repository import ChatMembersRepository +from app.repositories.messages_repository import MessagesRepository +from app.schemas import api as schemas +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_messages import FetchMessagesUseCase + + +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")) + assert edited.text == "fixed" + assert edited.edited_at is not None + + +async def test_other_member_cannot_edit( + edit_message_use_case: EditMessageUseCase, alice_message: tables.MessagesTable, bob: tables.UsersTable +) -> 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")) + + +async def _remove_alice_from_chat( + chat_members_repository: ChatMembersRepository, alice_message: tables.MessagesTable, alice: tables.UsersTable +) -> None: + membership = await chat_members_repository.get_one(chat_id=alice_message.chat_id, user_id=alice.id) + await chat_members_repository.delete(item_id=membership.id) + + +async def test_author_without_membership_cannot_edit( + edit_message_use_case: EditMessageUseCase, + chat_members_repository: ChatMembersRepository, + alice_message: tables.MessagesTable, + alice: tables.UsersTable, +) -> None: + # alice is still the message's author but no longer a member of its chat (e.g. removed) - + # the one state where authorship and membership disagree, and the only state that can prove + # 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")) + + +async def test_non_member_cannot_edit( + edit_message_use_case: EditMessageUseCase, alice_message: tables.MessagesTable, carol: tables.UsersTable +) -> None: + # 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")) + + +async def test_editing_a_deleted_message_raises_conflict( + edit_message_use_case: EditMessageUseCase, + delete_message_use_case: DeleteMessageUseCase, + alice_message: tables.MessagesTable, + alice: tables.UsersTable, +) -> 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) + with pytest.raises(ConflictError): + await edit_message_use_case(alice, alice_message.id, schemas.EditMessageRequest(text="nope")) + + +async def test_author_can_delete( + delete_message_use_case: DeleteMessageUseCase, + messages_repository: MessagesRepository, + alice_message: tables.MessagesTable, + alice: tables.UsersTable, +) -> None: + await delete_message_use_case(alice, alice_message.id) + stored = await messages_repository.get_one(id=alice_message.id) + assert stored.deleted_at is not None + + +async def test_other_member_cannot_delete( + delete_message_use_case: DeleteMessageUseCase, alice_message: tables.MessagesTable, bob: tables.UsersTable +) -> 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) + + +async def test_author_without_membership_cannot_delete( + delete_message_use_case: DeleteMessageUseCase, + chat_members_repository: ChatMembersRepository, + alice_message: tables.MessagesTable, + alice: tables.UsersTable, +) -> None: + # 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) + + +async def test_non_member_cannot_delete( + delete_message_use_case: DeleteMessageUseCase, alice_message: tables.MessagesTable, carol: tables.UsersTable +) -> 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) + + +async def test_deleting_an_already_deleted_message_is_idempotent( + delete_message_use_case: DeleteMessageUseCase, + messages_repository: MessagesRepository, + alice_message: tables.MessagesTable, + alice: tables.UsersTable, +) -> None: + await delete_message_use_case(alice, 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) + + stored = await messages_repository.get_one(id=alice_message.id) + assert stored.deleted_at == first_deleted_at + + +async def test_deleted_message_disappears_from_listing( + delete_message_use_case: DeleteMessageUseCase, + fetch_messages_use_case: FetchMessagesUseCase, + create_message_use_case: CreateMessageUseCase, + alice_message: tables.MessagesTable, + alice: tables.UsersTable, +) -> None: + # 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") + ) + + await delete_message_use_case(alice, alice_message.id) + page = await fetch_messages_use_case(alice, 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 new file mode 100644 index 0000000..6c592f3 --- /dev/null +++ b/tests/use_cases/test_unread_counts.py @@ -0,0 +1,228 @@ +import typing +import uuid + +import pytest + +from app.database import tables +from app.exceptions import PermissionDeniedError, ValidationError +from app.repositories.messages_repository import MessagesRepository +from app.schemas import api as schemas +from app.use_cases.create_chat import CreateChatUseCase +from app.use_cases.delete_message import DeleteMessageUseCase +from app.use_cases.fetch_chats import FetchChatsUseCase +from app.use_cases.mark_read import MarkReadUseCase + + +SendFixture = typing.Callable[[tables.UsersTable, int, str], typing.Awaitable[tuple[tables.MessagesTable, bool]]] + + +async def test_unread_counts_messages_from_others( + fetch_chats_use_case: FetchChatsUseCase, + direct_chat: tables.ChatsTable, + alice: tables.UsersTable, + bob: tables.UsersTable, + send: SendFixture, +) -> 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 + + +async def test_own_messages_are_never_unread( + fetch_chats_use_case: FetchChatsUseCase, + direct_chat: tables.ChatsTable, + alice: tables.UsersTable, + send: SendFixture, +) -> None: + await send(alice, direct_chat.id, "mine") + rows = await fetch_chats_use_case(alice) + assert rows[0].unread_count == 0 + + +async def test_system_messages_count_as_unread( + fetch_chats_use_case: FetchChatsUseCase, + messages_repository: MessagesRepository, + direct_chat: tables.ChatsTable, + alice: tables.UsersTable, +) -> None: + 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 + + +async def test_marking_read_clears_the_count( # noqa: PLR0913, PLR0917 - each is a fixture-injected dependency + fetch_chats_use_case: FetchChatsUseCase, + mark_read_use_case: MarkReadUseCase, + direct_chat: tables.ChatsTable, + alice: tables.UsersTable, + bob: tables.UsersTable, + 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 + + +async def test_deleted_messages_are_not_unread( # noqa: PLR0913, PLR0917 - each is a fixture-injected dependency + fetch_chats_use_case: FetchChatsUseCase, + delete_message_use_case: DeleteMessageUseCase, + direct_chat: tables.ChatsTable, + alice: tables.UsersTable, + bob: tables.UsersTable, + 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 + + +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 + + +async def test_listing_orders_most_recently_active_chat_first( # noqa: PLR0913, PLR0917 - fixture-injected + fetch_chats_use_case: FetchChatsUseCase, + create_chat_use_case: CreateChatUseCase, + direct_chat: tables.ChatsTable, + alice: tables.UsersTable, + carol: tables.UsersTable, + send: SendFixture, +) -> None: + other_chat, _ = await create_chat_use_case( + alice, 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] + + +async def test_unread_counts_differ_per_chat( # noqa: PLR0913, PLR0917 - each is a fixture-injected dependency + fetch_chats_use_case: FetchChatsUseCase, + create_chat_use_case: CreateChatUseCase, + direct_chat: tables.ChatsTable, + alice: tables.UsersTable, + bob: tables.UsersTable, + carol: tables.UsersTable, + send: SendFixture, +) -> None: + # A correlated subquery that returned the same count for every row 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. + other_chat, _ = await create_chat_use_case( + alice, 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) + + counts = {row.chat.id: row.unread_count for row in rows} + assert counts == {direct_chat.id: 2, other_chat.id: 1} + + +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)) + + +async def test_marking_read_with_a_message_from_another_chat_is_rejected( # noqa: PLR0913, PLR0917 - fixture-injected + mark_read_use_case: MarkReadUseCase, + create_chat_use_case: CreateChatUseCase, + direct_chat: tables.ChatsTable, + alice: tables.UsersTable, + carol: tables.UsersTable, + send: SendFixture, +) -> None: + other_chat, _ = await create_chat_use_case( + alice, 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)) + + +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)) + + +async def test_marking_read_is_monotonic( # noqa: PLR0913, PLR0917 - each is a fixture-injected dependency + fetch_chats_use_case: FetchChatsUseCase, + mark_read_use_case: MarkReadUseCase, + direct_chat: tables.ChatsTable, + alice: tables.UsersTable, + bob: tables.UsersTable, + send: SendFixture, +) -> 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)) + + # 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)) + + assert member.last_read_message_id == second.id + rows = await fetch_chats_use_case(alice) + assert rows[0].unread_count == 0 + + +async def test_deleting_the_newest_message_updates_preview_and_ordering( # noqa: PLR0913, PLR0917 - fixture-injected + fetch_chats_use_case: FetchChatsUseCase, + delete_message_use_case: DeleteMessageUseCase, + create_chat_use_case: CreateChatUseCase, + direct_chat: tables.ChatsTable, + alice: tables.UsersTable, + carol: tables.UsersTable, + send: SendFixture, +) -> None: + other_chat, _ = await create_chat_use_case( + alice, 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] + + await delete_message_use_case(alice, 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 + + +async def test_deleting_a_non_newest_message_leaves_preview_and_ordering_unchanged( + fetch_chats_use_case: FetchChatsUseCase, + delete_message_use_case: DeleteMessageUseCase, + direct_chat: tables.ChatsTable, + alice: tables.UsersTable, + send: SendFixture, +) -> None: + first, _ = await send(alice, direct_chat.id, "first") + second, _ = await send(alice, direct_chat.id, "second") + + await delete_message_use_case(alice, 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