From c6dcb1810fd7d61b46f50f8211a61e4f76540da0 Mon Sep 17 00:00:00 2001 From: iklo Date: Fri, 18 Sep 2026 10:39:34 -0300 Subject: [PATCH 1/7] refactor: one EndpointHandler instead of two handler closures make_collection_handler and make_detail_handler were two 80-line closures with the same prologue and epilogue and an if/elif on request.method in the middle. Each branch repeated the same three-way choice: async override, async engine, sync engine. EndpointHandler keeps that choice in one place (_serve) and describes each HTTP method as data: which user method overrides it, which CRUD method serves it on each engine kind, whether it reads a body, whether GET caching applies. Collection and detail routes are the same class with a different verb table. body_reader.py and response_wrapper.py each held one small function used only here, so they move in as private helpers. The per-request lookups and the order of steps are unchanged: pre middlewares, auth, verb, dict-to-JSON, cache invalidation, background tasks, post middlewares. Route names stay `_collection` and `_detail`. Tests: 335 passed, 1 xfailed, 2 xpassed. --- docs/technical-reference/handlers.md | 6 +- lightapi/body_reader.py | 15 --- lightapi/endpoint_handler.py | 173 ++++++++++++++++++++++++ lightapi/handler_factory.py | 190 --------------------------- lightapi/lightapi.py | 38 +++--- lightapi/response_wrapper.py | 12 -- pyproject.toml | 2 - 7 files changed, 195 insertions(+), 241 deletions(-) delete mode 100644 lightapi/body_reader.py create mode 100644 lightapi/endpoint_handler.py delete mode 100644 lightapi/handler_factory.py delete mode 100644 lightapi/response_wrapper.py diff --git a/docs/technical-reference/handlers.md b/docs/technical-reference/handlers.md index 1587a84..4f2955b 100644 --- a/docs/technical-reference/handlers.md +++ b/docs/technical-reference/handlers.md @@ -12,10 +12,10 @@ When `app.register({"/items": ItemEndpoint})` is called, LightAPI registers two | Route | Methods | Handler | |-------|---------|---------| -| `/items` | `GET`, `POST` | `handler_factory.make_collection_handler(cls, ...)` | -| `/items/{id:int}` | `GET`, `PUT`, `PATCH`, `DELETE` | `handler_factory.make_detail_handler(cls, ...)` | +| `/items` | `GET`, `POST` | `EndpointHandler.for_collection(cls, ...).handle` | +| `/items/{id:int}` | `GET`, `PUT`, `PATCH`, `DELETE` | `EndpointHandler.for_detail(cls, ...).handle` | -Both handlers are closures that instantiate the endpoint class per request and dispatch to the appropriate CRUD method. +Both handlers instantiate the endpoint class per request and dispatch to the appropriate CRUD method. ## Request Dispatch Flow diff --git a/lightapi/body_reader.py b/lightapi/body_reader.py deleted file mode 100644 index e1fb07a..0000000 --- a/lightapi/body_reader.py +++ /dev/null @@ -1,15 +0,0 @@ -"""Body reader for parsing request bodies.""" - -import json -from typing import Any - -from starlette.requests import Request - - -async def read_body(request: Request) -> dict[str, Any]: - """Read and parse JSON body; return {} on failure.""" - try: - body = await request.body() - return json.loads(body) if body else {} - except Exception: - return {} diff --git a/lightapi/endpoint_handler.py b/lightapi/endpoint_handler.py new file mode 100644 index 0000000..3c31807 --- /dev/null +++ b/lightapi/endpoint_handler.py @@ -0,0 +1,173 @@ +"""Request handling for one RestEndpoint route.""" + +from __future__ import annotations + +import asyncio +import json +from dataclasses import dataclass +from typing import Any, Callable + +from starlette.background import BackgroundTasks +from starlette.requests import Request +from starlette.responses import JSONResponse, Response + +from lightapi.auth_checker import check_auth +from lightapi.cache_helper import ( + invalidate_cache_after_write, + maybe_cached, + maybe_cached_async, +) +from lightapi.constants import RESPONSE_KEY_DETAIL, HTTPStatus +from lightapi.middleware_runner import run_post_middlewares, run_pre_middlewares +from lightapi.rest import RestEndpoint +from lightapi.session import run_blocking + + +@dataclass(frozen=True) +class _Verb: + """How one HTTP method is served when the user did not override it.""" + + override: str + sync_name: str + async_name: str + args: Callable[[Request, dict[str, Any], Any], tuple[Any, ...]] + reads_body: bool = False + cached: bool = False + + +def _request_only(request: Request, data: dict[str, Any], pk: Any) -> tuple[Any, ...]: + return (request,) + + +def _data_only(request: Request, data: dict[str, Any], pk: Any) -> tuple[Any, ...]: + return (data,) + + +def _request_and_pk(request: Request, data: dict[str, Any], pk: Any) -> tuple[Any, ...]: + return (request, pk) + + +def _full_update(request: Request, data: dict[str, Any], pk: Any) -> tuple[Any, ...]: + return (data, pk, False) + + +def _partial_update(request: Request, data: dict[str, Any], pk: Any) -> tuple[Any, ...]: + return (data, pk, True) + + +_COLLECTION_VERBS = { + "GET": _Verb("get", "list", "_list_async", _request_only, cached=True), + "POST": _Verb("post", "create", "_create_async", _data_only, reads_body=True), +} + +_DETAIL_VERBS = { + "GET": _Verb("get", "retrieve", "_retrieve_async", _request_and_pk, cached=True), + "PUT": _Verb("put", "update", "_update_async", _full_update, reads_body=True), + "PATCH": _Verb( + "patch", "update", "_update_async", _partial_update, reads_body=True + ), + "DELETE": _Verb("delete", "destroy", "_destroy_async", _request_and_pk), +} + + +class EndpointHandler: + """Serves every request of one route (collection or detail) of an endpoint.""" + + def __init__( + self, + endpoint_cls: type[RestEndpoint], + verbs: dict[str, _Verb], + middlewares: list[type], + is_async: bool, + ) -> None: + self._endpoint_cls = endpoint_cls + self._verbs = verbs + self._middlewares = middlewares + self._is_async = is_async + + @classmethod + def for_collection( + cls, endpoint_cls: type[RestEndpoint], middlewares: list[type], is_async: bool + ) -> EndpointHandler: + return cls(endpoint_cls, _COLLECTION_VERBS, middlewares, is_async) + + @classmethod + def for_detail( + cls, endpoint_cls: type[RestEndpoint], middlewares: list[type], is_async: bool + ) -> EndpointHandler: + return cls(endpoint_cls, _DETAIL_VERBS, middlewares, is_async) + + @property + def methods(self) -> list[str]: + """HTTP methods of this route that the endpoint allows.""" + return [m for m in self._endpoint_cls._allowed_methods if m in self._verbs] + + async def handle(self, request: Request) -> Response: + endpoint = self._endpoint_cls() + endpoint._background = BackgroundTasks() + endpoint._current_request = request + + pre_result = await run_pre_middlewares(self._middlewares, request) + if pre_result is not None: + return pre_result + + auth_result = check_auth(self._endpoint_cls, request) + if auth_result is not None: + return auth_result + + verb = self._verbs.get(request.method) + if verb is None: + result = self._method_not_allowed() + else: + result = await self._serve(verb, endpoint, request) + + response = JSONResponse(result) if isinstance(result, dict) else result + await invalidate_cache_after_write(self._endpoint_cls, request) + + if endpoint._background.tasks: + response.background = endpoint._background + + return await run_post_middlewares(self._middlewares, request, response) + + async def _serve( + self, verb: _Verb, endpoint: RestEndpoint, request: Request + ) -> Any: + data = await _read_body(request) if verb.reads_body else {} + args = verb.args(request, data, request.path_params.get("id")) + + override = getattr(self._endpoint_cls, verb.override, None) + if override and asyncio.iscoroutinefunction(override): + return await override(endpoint, request) + + if self._is_async: + crud_async = getattr(endpoint, verb.async_name) + if verb.cached: + return await maybe_cached_async( + self._endpoint_cls, request, lambda: crud_async(*args) + ) + return await crud_async(*args) + + crud = getattr(endpoint, verb.sync_name) + engine = endpoint._get_engine() + if verb.cached: + return await run_blocking( + engine, maybe_cached, self._endpoint_cls, request, lambda: crud(*args) + ) + return await run_blocking(engine, crud, *args) + + def _method_not_allowed(self) -> Response: + allowed = ", ".join(sorted(self.methods)) + return JSONResponse( + {RESPONSE_KEY_DETAIL: f"Method Not Allowed. Allowed: {allowed}"}, + status_code=HTTPStatus.METHOD_NOT_ALLOWED, + headers={"Allow": allowed}, + ) + + +async def _read_body(request: Request) -> dict[str, Any]: + """Read and parse JSON body; return {} on failure.""" + try: + body = await request.body() + return json.loads(body) if body else {} + except Exception: + return {} diff --git a/lightapi/handler_factory.py b/lightapi/handler_factory.py deleted file mode 100644 index 11de341..0000000 --- a/lightapi/handler_factory.py +++ /dev/null @@ -1,190 +0,0 @@ -"""Handler factory for creating Starlette route handlers.""" - -import asyncio -from typing import Any - -from starlette.background import BackgroundTasks -from starlette.requests import Request -from starlette.responses import Response - -from lightapi.constants import RESPONSE_KEY_DETAIL, HTTPStatus -from lightapi.rest import RestEndpoint -from lightapi.session import run_blocking - - -def make_collection_handler( - cls: type[RestEndpoint], - middlewares: list[type], - is_async: bool, -) -> Any: - """Create collection endpoint handler (list/create).""" - - async def handler(request: Request) -> Response: - from lightapi.auth_checker import check_auth - from lightapi.body_reader import read_body - from lightapi.middleware_runner import run_post_middlewares, run_pre_middlewares - from lightapi.response_wrapper import wrap_dict_response - - endpoint = cls() - endpoint._background = BackgroundTasks() - endpoint._current_request = request - - pre_result = await run_pre_middlewares(middlewares, request) - if pre_result is not None: - return pre_result - - auth_result = check_auth(cls, request) - if auth_result is not None: - return auth_result - - if request.method == "GET": - get_override = getattr(cls, "get", None) - if get_override and asyncio.iscoroutinefunction(get_override): - result = await get_override(endpoint, request) - elif is_async: - from lightapi.cache_helper import maybe_cached_async - - result = await maybe_cached_async( - cls, request, lambda: endpoint._list_async(request) - ) - else: - from lightapi.cache_helper import maybe_cached - - result = await run_blocking( - endpoint._get_engine(), - maybe_cached, - cls, - request, - lambda: endpoint.list(request), - ) - elif request.method == "POST": - data = await read_body(request) - post_override = getattr(cls, "post", None) - if post_override and asyncio.iscoroutinefunction(post_override): - result = await post_override(endpoint, request) - elif is_async: - result = await endpoint._create_async(data) - else: - result = await run_blocking( - endpoint._get_engine(), endpoint.create, data - ) - else: - allowed = ", ".join(sorted(cls._allowed_methods & {"GET", "POST"})) - result = __import__( - "starlette.responses", fromlist=["JSONResponse"] - ).JSONResponse( - {RESPONSE_KEY_DETAIL: f"Method Not Allowed. Allowed: {allowed}"}, - status_code=HTTPStatus.METHOD_NOT_ALLOWED, - headers={"Allow": allowed}, - ) - - response = wrap_dict_response(result) - - from lightapi.cache_helper import invalidate_cache_after_write - - await invalidate_cache_after_write(cls, request) - - if endpoint._background.tasks: - response.background = endpoint._background - - return await run_post_middlewares(middlewares, request, response) - - handler.__name__ = f"{cls.__name__}_collection" - handler.__endpoint_cls__ = cls - return handler - - -def make_detail_handler( - cls: type[RestEndpoint], - middlewares: list[type], - is_async: bool, -) -> Any: - """Create detail endpoint handler (retrieve/update/delete).""" - - async def handler(request: Request) -> Response: - from lightapi.auth_checker import check_auth - from lightapi.body_reader import read_body - from lightapi.middleware_runner import run_post_middlewares, run_pre_middlewares - from lightapi.response_wrapper import wrap_dict_response - - pk: int = request.path_params["id"] - endpoint = cls() - endpoint._background = BackgroundTasks() - endpoint._current_request = request - - pre_result = await run_pre_middlewares(middlewares, request) - if pre_result is not None: - return pre_result - - auth_result = check_auth(cls, request) - if auth_result is not None: - return auth_result - - if request.method == "GET": - get_override = getattr(cls, "get", None) - if get_override and asyncio.iscoroutinefunction(get_override): - result = await get_override(endpoint, request) - elif is_async: - from lightapi.cache_helper import maybe_cached_async - - result = await maybe_cached_async( - cls, request, lambda: endpoint._retrieve_async(request, pk) - ) - else: - from lightapi.cache_helper import maybe_cached - - result = await run_blocking( - endpoint._get_engine(), - maybe_cached, - cls, - request, - lambda: endpoint.retrieve(request, pk), - ) - elif request.method in {"PUT", "PATCH"}: - data = await read_body(request) - partial = request.method == "PATCH" - put_override = getattr(cls, "put" if not partial else "patch", None) - if put_override and asyncio.iscoroutinefunction(put_override): - result = await put_override(endpoint, request) - elif is_async: - result = await endpoint._update_async(data, pk, partial=partial) - else: - result = await run_blocking( - endpoint._get_engine(), endpoint.update, data, pk, partial - ) - elif request.method == "DELETE": - delete_override = getattr(cls, "delete", None) - if delete_override and asyncio.iscoroutinefunction(delete_override): - result = await delete_override(endpoint, request) - elif is_async: - result = await endpoint._destroy_async(request, pk) - else: - result = await run_blocking( - endpoint._get_engine(), endpoint.destroy, request, pk - ) - else: - allowed = ", ".join( - sorted(cls._allowed_methods & {"GET", "PUT", "PATCH", "DELETE"}) - ) - result = __import__( - "starlette.responses", fromlist=["JSONResponse"] - ).JSONResponse( - {RESPONSE_KEY_DETAIL: f"Method Not Allowed. Allowed: {allowed}"}, - status_code=HTTPStatus.METHOD_NOT_ALLOWED, - headers={"Allow": allowed}, - ) - - response = wrap_dict_response(result) - - from lightapi.cache_helper import invalidate_cache_after_write - - await invalidate_cache_after_write(cls, request) - - if endpoint._background.tasks: - response.background = endpoint._background - - return await run_post_middlewares(middlewares, request, response) - - handler.__name__ = f"{cls.__name__}_detail" - handler.__endpoint_cls__ = cls - return handler diff --git a/lightapi/lightapi.py b/lightapi/lightapi.py index 3432b0e..d4bb028 100644 --- a/lightapi/lightapi.py +++ b/lightapi/lightapi.py @@ -18,6 +18,7 @@ from starlette.routing import Route from lightapi.authentication import BasicAuthentication, JWTAuthentication +from lightapi.endpoint_handler import EndpointHandler from lightapi.exceptions import ConfigurationError from lightapi.health import HEALTH_PATH, HealthCheckEndpoint from lightapi.rest import RestEndpoint @@ -255,28 +256,27 @@ class TestIsolationMeta: cls._schema_deferred = False cls._reflect_deferred = False - allowed = cls._allowed_methods - from lightapi.handler_factory import ( - make_collection_handler, - make_detail_handler, + is_async = self._mode == "async" + collection = EndpointHandler.for_collection( + cls, self._middlewares, is_async ) - - collection_route = Route( - path, - endpoint=make_collection_handler( - cls, self._middlewares, self._mode == "async" - ), - methods=[m for m in allowed if m in {"GET", "POST"}], + detail = EndpointHandler.for_detail(cls, self._middlewares, is_async) + self._routes.append( + Route( + path, + endpoint=collection.handle, + methods=collection.methods, + name=f"{cls.__name__}_collection", + ) ) - detail_route = Route( - path.rstrip("/") + "/{id:int}", - endpoint=make_detail_handler( - cls, self._middlewares, self._mode == "async" - ), - methods=[m for m in allowed if m in {"GET", "PUT", "PATCH", "DELETE"}], + self._routes.append( + Route( + path.rstrip("/") + "/{id:int}", + endpoint=detail.handle, + methods=detail.methods, + name=f"{cls.__name__}_detail", + ) ) - self._routes.append(collection_route) - self._routes.append(detail_route) self._endpoint_map[path] = cls # Auto-register /auth/login and /auth/token when JWT or Basic auth is used diff --git a/lightapi/response_wrapper.py b/lightapi/response_wrapper.py deleted file mode 100644 index 93b89d2..0000000 --- a/lightapi/response_wrapper.py +++ /dev/null @@ -1,12 +0,0 @@ -"""Response wrapper for handling dict responses.""" - -from typing import Any - -from starlette.responses import JSONResponse, Response - - -def wrap_dict_response(result: Any) -> Response: - """Wrap dict responses in JSONResponse.""" - if isinstance(result, dict): - return JSONResponse(result) - return result diff --git a/pyproject.toml b/pyproject.toml index a3a2d47..085a752 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -177,9 +177,7 @@ module = [ "lightapi._login", "lightapi.auth_checker", "lightapi.cache_helper", - "lightapi.handler_factory", "lightapi.middleware_runner", - "lightapi.response_wrapper", "lightapi.session_manager", "lightapi.table_mapping", ] From 19ba95b4c08a708036e0ec1243d3befbd456304a Mon Sep 17 00:00:00 2001 From: iklo Date: Fri, 18 Sep 2026 10:41:18 -0300 Subject: [PATCH 2/7] fix: serve sync verb overrides The handler only used a user's get/post/put/patch/delete when it was a coroutine function. A plain `def get(self, request)` was skipped without any warning and the built-in list() ran instead, although the tutorial, the endpoints guide and the REST reference all teach the sync form. Sync overrides now run through run_blocking on a sync engine. On an async engine they run inside AsyncSession.run_sync, so an override that calls self.list() or self.create() reaches the async driver. Only real functions count as overrides: an endpoint with a column named `post` must keep working, and a test pins that. tests/test_sync_verb_override.py: three of the four tests fail without this change. Tests: 339 passed, 1 xfailed, 2 xpassed. --- CHANGELOG.md | 5 ++ lightapi/endpoint_handler.py | 19 +++++- tests/test_sync_verb_override.py | 106 +++++++++++++++++++++++++++++++ 3 files changed, 128 insertions(+), 2 deletions(-) create mode 100644 tests/test_sync_verb_override.py diff --git a/CHANGELOG.md b/CHANGELOG.md index fa92499..5b654fc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,11 @@ Versions align with [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - **`Meta.cache` was ignored on async engines**: cached GETs and cache invalidation only ran on the sync path, so an app on an `AsyncEngine` never read or wrote the cache. Both engine kinds now use it. The Redis calls run in a worker thread. +- **Sync verb overrides were ignored**: a plain `def get(self, request)` (or `post`, + `put`, `patch`, `delete`) on an endpoint was skipped and the built-in CRUD ran + instead; only `async def` overrides were served. Both are served now, on sync and + async engines. A column that happens to be called `post` or `get` is still a + column, not an override. ## [Unreleased] — 0.1.24 diff --git a/lightapi/endpoint_handler.py b/lightapi/endpoint_handler.py index 3c31807..96a5984 100644 --- a/lightapi/endpoint_handler.py +++ b/lightapi/endpoint_handler.py @@ -3,6 +3,7 @@ from __future__ import annotations import asyncio +import inspect import json from dataclasses import dataclass from typing import Any, Callable @@ -135,9 +136,10 @@ async def _serve( data = await _read_body(request) if verb.reads_body else {} args = verb.args(request, data, request.path_params.get("id")) + # A column may share a verb's name (`post: str`), so only functions count. override = getattr(self._endpoint_cls, verb.override, None) - if override and asyncio.iscoroutinefunction(override): - return await override(endpoint, request) + if inspect.isfunction(override): + return await self._call_override(override, endpoint, request) if self._is_async: crud_async = getattr(endpoint, verb.async_name) @@ -155,6 +157,19 @@ async def _serve( ) return await run_blocking(engine, crud, *args) + async def _call_override( + self, override: Callable[..., Any], endpoint: RestEndpoint, request: Request + ) -> Any: + if asyncio.iscoroutinefunction(override): + return await override(endpoint, request) + if self._is_async: + # Gives a sync override the greenlet context that endpoint.list() and + # friends need to reach an async driver. + return await endpoint._in_async_session( + lambda _session: override(endpoint, request) + ) + return await run_blocking(endpoint._get_engine(), override, endpoint, request) + def _method_not_allowed(self) -> Response: allowed = ", ".join(sorted(self.methods)) return JSONResponse( diff --git a/tests/test_sync_verb_override.py b/tests/test_sync_verb_override.py new file mode 100644 index 0000000..4b6a382 --- /dev/null +++ b/tests/test_sync_verb_override.py @@ -0,0 +1,106 @@ +"""A plain `def get/post/...` override must be served, as the tutorial teaches.""" + +import pytest +import pytest_asyncio +from httpx import ASGITransport, AsyncClient +from sqlalchemy import create_engine +from sqlalchemy.ext.asyncio import create_async_engine +from sqlalchemy.pool import StaticPool +from starlette.requests import Request +from starlette.testclient import TestClient + +from lightapi import LightApi, RestEndpoint +from lightapi.fields import Field as LField + + +class GreetingEndpoint(RestEndpoint): + name: str = LField(min_length=1) + + def get(self, request: Request): + return {"message": "custom sync get"} + + +class DecoratedListEndpoint(RestEndpoint): + name: str = LField(min_length=1) + + def get(self, request: Request): + response = self.list(request) + response.headers["X-Served-By"] = "sync-override" + return response + + +class AsyncEngineSyncOverrideEndpoint(RestEndpoint): + name: str = LField(min_length=1) + + def get(self, request: Request): + response = self.list(request) + response.headers["X-Served-By"] = "sync-override" + return response + + +class BlogEntryEndpoint(RestEndpoint): + """`post` here is a column, not a verb override.""" + + post: str = LField(min_length=1) + + +@pytest.fixture +def sync_client(): + engine = create_engine( + "sqlite:///:memory:", + connect_args={"check_same_thread": False}, + poolclass=StaticPool, + ) + app = LightApi(engine=engine) + app.register( + { + "/greetings": GreetingEndpoint, + "/decorated": DecoratedListEndpoint, + "/blogentries": BlogEntryEndpoint, + } + ) + return TestClient(app.build_app()) + + +@pytest_asyncio.fixture +async def async_client(): + engine = create_async_engine("sqlite+aiosqlite:///:memory:") + app = LightApi(engine=engine, mode="async") + app.register({"/asyncsyncoverride": AsyncEngineSyncOverrideEndpoint}) + async with AsyncClient( + transport=ASGITransport(app=app.build_app()), base_url="http://test" + ) as client: + yield client + await engine.dispose() + + +def test_sync_get_override_returning_dict_is_served(sync_client): + response = sync_client.get("/greetings") + + assert response.status_code == 200 + assert response.json() == {"message": "custom sync get"} + + +def test_sync_get_override_can_call_the_builtin_list(sync_client): + response = sync_client.get("/decorated") + + assert response.status_code == 200 + assert response.headers["X-Served-By"] == "sync-override" + assert response.json() == {"results": []} + + +def test_column_named_like_a_verb_is_not_treated_as_an_override(sync_client): + created = sync_client.post("/blogentries", json={"post": "hello"}) + + assert created.status_code == 201 + assert created.json()["post"] == "hello" + + +async def test_sync_get_override_on_async_engine_can_call_the_builtin_list( + async_client, +): + response = await async_client.get("/asyncsyncoverride") + + assert response.status_code == 200 + assert response.headers["X-Served-By"] == "sync-override" + assert response.json() == {"results": []} From 9f62208e1f02f24fea51bbbd810f2c03422bd439 Mon Sep 17 00:00:00 2001 From: iklo Date: Fri, 18 Sep 2026 10:53:51 -0300 Subject: [PATCH 3/7] fix: build auth backends from the endpoint config, login included The framework built backends by comparing class names: `if backend.__name__ == "JWTAuthentication"` in register() and again in check_auth(). A subclass with another name fell into the `else` branch and was built with no settings. The login route was worse: it validated through a bare JWTAuthentication() with validate_credentials patched at runtime, so the subclass override the README tells users to write was never called and login answered 401. Backends now build themselves: BaseAuthentication.from_config, overridden by JWTAuthentication and BasicAuthentication. A subclass that defines its own __init__ is still built without arguments, as before. Authentication answers the questions that were asked about it from outside: permission_for(method), requires_login(method), build_backend(). EndpointGuard (was check_auth) uses them and receives the app's login_validator through its constructor. That removes the object.__setattr__ on the frozen Authentication dataclass that smuggled it in. LoginEndpoint (was login_handler plus a closure) gets the endpoint's real backend. What a successful login returns is the backend's call: login_response() gives token and user for JWT, user for Basic, which removes the has_jwt flag and four jwt_* parameters. validate_credentials may be sync or async. The login rate limits (1000/10000/100000) were written twice in lightapi.py; they are one constant now. tests/test_login_backend_subclass.py: three of six fail without the fix. The 1002-line test_login_auth.py passes unchanged. Tests: 345 passed, 1 xfailed, 2 xpassed. --- CHANGELOG.md | 9 ++ lightapi/_login.py | 127 +++++++--------- lightapi/auth_checker.py | 101 +++++-------- lightapi/authentication/base.py | 24 ++- lightapi/authentication/basic.py | 16 +- lightapi/authentication/jwt.py | 30 +++- lightapi/config.py | 34 ++++- lightapi/endpoint_handler.py | 36 +++-- lightapi/lightapi.py | 218 +++++++-------------------- tests/test_login_backend_subclass.py | 154 +++++++++++++++++++ 10 files changed, 437 insertions(+), 312 deletions(-) create mode 100644 tests/test_login_backend_subclass.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 5b654fc..9f4e595 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,15 @@ Versions align with [Semantic Versioning](https://semver.org/spec/v2.0.0.html). instead; only `async def` overrides were served. Both are served now, on sync and async engines. A column that happens to be called `post` or `get` is still a column, not an override. +- **`/auth/login` ignored a backend's `validate_credentials`**: the login route + always validated through a bare `JWTAuthentication()`, so overriding + `validate_credentials()` on a subclass (the README's "custom authentication") + never ran and login returned `401`. The route now uses the backend the endpoint + configured, sync or `async def`. An app-level `login_validator` still wins. +- **Auth settings were dropped for backend subclasses**: `jwt_expiration`, + `jwt_algorithm` and `jwt_extra_claims` only reached a backend whose class was named + exactly `JWTAuthentication`. Subclasses get them too, unless they define their own + `__init__`. ## [Unreleased] — 0.1.24 diff --git a/lightapi/_login.py b/lightapi/_login.py index afbedc2..375f731 100644 --- a/lightapi/_login.py +++ b/lightapi/_login.py @@ -3,22 +3,20 @@ from __future__ import annotations import base64 +import inspect import json import logging -from typing import Any, Optional +from typing import TYPE_CHECKING, Any from pydantic import BaseModel, ConfigDict, Field, ValidationError from starlette.requests import Request from starlette.responses import JSONResponse -from lightapi.constants import ( - RESPONSE_KEY_DETAIL, - RESPONSE_KEY_TOKEN, - RESPONSE_KEY_USER, - HTTPStatus, -) +from lightapi.constants import RESPONSE_KEY_DETAIL, HTTPStatus -# JWTAuthentication imported locally where needed to avoid circular import +if TYPE_CHECKING: + from lightapi.authentication.base import BaseAuthentication, LoginValidator + from lightapi.rate_limiter import RateLimiter logger = logging.getLogger(__name__) @@ -78,80 +76,65 @@ async def _read_body(request: Request) -> dict[str, Any]: return {} -async def login_handler( - request: Request, - *, - has_jwt: bool, - jwt_expiration: int | None = None, - jwt_extra_claims: list[str] | None = None, - jwt_algorithm: str | None = None, - rate_limiter: Optional[Any] = None, - auth_backend: Optional[Any] = None, -) -> JSONResponse: - """ - Handle POST /auth/login and POST /auth/token. +class LoginEndpoint: + """POST /auth/login and POST /auth/token. Returns 422 for body validation, 401 for malformed Basic, invalid credentials, - or any exception raised by the validator; 200 with token+user (JWT) or user (Basic). + or any exception raised while validating; 200 with whatever the backend puts + in a login response (token and user for JWT, user for Basic). """ - # Apply rate limiting if a rate limiter is provided - if rate_limiter is not None: - is_limited, window = rate_limiter.is_rate_limited(request, endpoint="auth") - if is_limited: - return rate_limiter.get_rate_limit_response(request, window) - if request.method != "POST": - return JSONResponse( - {RESPONSE_KEY_DETAIL: "method not allowed"}, - status_code=HTTPStatus.METHOD_NOT_ALLOWED, - headers={"Allow": "POST"}, - ) - - try: - creds = await _parse_credentials(request) - except ValidationError as exc: - return JSONResponse( - {RESPONSE_KEY_DETAIL: exc.errors()}, - status_code=HTTPStatus.UNPROCESSABLE_ENTITY, - ) - - if creds is None: - return JSONResponse( - {RESPONSE_KEY_DETAIL: "Invalid credentials"}, - status_code=HTTPStatus.UNAUTHORIZED, + def __init__( + self, + backend: BaseAuthentication, + login_validator: LoginValidator | None, + rate_limiter: RateLimiter, + ) -> None: + self._backend = backend + self._login_validator = login_validator + self._rate_limiter = rate_limiter + + async def handle(self, request: Request) -> JSONResponse: + is_limited, window = self._rate_limiter.is_rate_limited( + request, endpoint="auth" ) + if is_limited: + return self._rate_limiter.get_rate_limit_response(request, window) - username, password = creds + if request.method != "POST": + return JSONResponse( + {RESPONSE_KEY_DETAIL: "method not allowed"}, + status_code=HTTPStatus.METHOD_NOT_ALLOWED, + headers={"Allow": "POST"}, + ) - # Use auth_backend's validate_credentials if provided - payload = None - if auth_backend is not None: try: - payload = auth_backend.validate_credentials(username, password) - except Exception as e: - logger.warning("validate_credentials raised: %s", e) + creds = await _parse_credentials(request) + except ValidationError as exc: + return JSONResponse( + {RESPONSE_KEY_DETAIL: exc.errors()}, + status_code=HTTPStatus.UNPROCESSABLE_ENTITY, + ) + + user = await self._validated_user(*creds) if creds is not None else None + if user is None: return JSONResponse( {RESPONSE_KEY_DETAIL: "Invalid credentials"}, status_code=HTTPStatus.UNAUTHORIZED, ) + return JSONResponse(self._backend.login_response(user)) - if payload is None: - return JSONResponse( - {RESPONSE_KEY_DETAIL: "Invalid credentials"}, - status_code=HTTPStatus.UNAUTHORIZED, - ) - - if has_jwt: - from lightapi.authentication import JWTAuthentication - - jwt_auth = JWTAuthentication(algorithm=jwt_algorithm) - if jwt_extra_claims and isinstance(payload, dict): - token_payload = {k: payload[k] for k in jwt_extra_claims if k in payload} - if not token_payload: - token_payload = payload - else: - token_payload = payload - token = jwt_auth.generate_token(token_payload, expiration=jwt_expiration) - return JSONResponse({RESPONSE_KEY_TOKEN: token, RESPONSE_KEY_USER: payload}) - - return JSONResponse({RESPONSE_KEY_USER: payload}) + async def _validated_user( + self, username: str, password: str + ) -> dict[str, Any] | None: + """The app's login_validator wins; otherwise the backend's own override.""" + validate = self._login_validator or self._backend.validate_credentials + try: + user = validate(username, password) + # The README shows `async def validate_credentials` on a subclass. + if inspect.isawaitable(user): + user = await user + except Exception as exc: + logger.warning("validate_credentials raised: %s", exc) + return None + return user diff --git a/lightapi/auth_checker.py b/lightapi/auth_checker.py index f4a5a96..166ca72 100644 --- a/lightapi/auth_checker.py +++ b/lightapi/auth_checker.py @@ -1,4 +1,8 @@ -"""Authentication and authorization checker.""" +"""Authentication and permission check for one endpoint.""" + +from __future__ import annotations + +from typing import TYPE_CHECKING from starlette.requests import Request from starlette.responses import JSONResponse, Response @@ -6,69 +10,42 @@ from lightapi.authentication import AllowAny from lightapi.constants import HTTPStatus - -def check_auth(cls: type, request: Request) -> Response | None: - """Run authentication + permission checks; return 401/403 response or None.""" - - auth_cfg = cls._meta.get("authentication") - if auth_cfg is None: - return None - - backend = auth_cfg.backend - permission_cls = auth_cfg.permission - is_per_method = isinstance(permission_cls, dict) - - if is_per_method: - perm_cls = permission_cls.get(request.method) - if perm_cls is None: - perm_cls = AllowAny - elif permission_cls is not None: - perm_cls = permission_cls - else: - perm_cls = AllowAny - - # Explicitly setting AllowAny (non-dict) means the endpoint is fully public — - # skip the backend authentication check entirely. - # Note: permission_cls=None defaults to AllowAny but still requires auth - # if a backend is set; only an explicit AllowAny bypasses the backend. - if not is_per_method and permission_cls is AllowAny: - return None - - requires_auth = backend is not None and ( - not is_per_method or perm_cls is not AllowAny - ) - - if requires_auth: - login_validator = getattr(auth_cfg, "_login_validator", None) - - if backend.__name__ == "JWTAuthentication": - authenticator = backend( - expiration=getattr(auth_cfg, "jwt_expiration", None), - algorithm=getattr(auth_cfg, "jwt_algorithm", None), - rate_limiter=getattr(auth_cfg, "rate_limiter", None), - ) - elif backend.__name__ == "BasicAuthentication": - authenticator = backend( - rate_limiter=getattr(auth_cfg, "rate_limiter", None), - login_validator=login_validator, - ) - else: - authenticator = backend() - - authenticated = authenticator.authenticate(request) - - if not authenticated: - return JSONResponse( - {"detail": "Authentication required"}, - status_code=HTTPStatus.UNAUTHORIZED, - headers={"WWW-Authenticate": "Bearer"}, - ) - - if perm_cls is not AllowAny: - if not perm_cls().has_permission(request): +if TYPE_CHECKING: + from lightapi.authentication.base import LoginValidator + from lightapi.config import Authentication + + +class EndpointGuard: + """Decides whether a request may reach an endpoint.""" + + def __init__( + self, + authentication: Authentication | None, + login_validator: LoginValidator | None = None, + ) -> None: + self._authentication = authentication + self._login_validator = login_validator + + def check(self, request: Request) -> Response | None: + """The 401 or 403 response that stops the request, or None to let it in.""" + authentication = self._authentication + if authentication is None: + return None + + if authentication.requires_login(request.method): + backend = authentication.build_backend(self._login_validator) + if not backend.authenticate(request): + return JSONResponse( + {"detail": "Authentication required"}, + status_code=HTTPStatus.UNAUTHORIZED, + headers={"WWW-Authenticate": "Bearer"}, + ) + + permission = authentication.permission_for(request.method) + if permission is not AllowAny and not permission().has_permission(request): return JSONResponse( {"detail": "Insufficient permissions"}, status_code=HTTPStatus.FORBIDDEN, ) - return None + return None diff --git a/lightapi/authentication/base.py b/lightapi/authentication/base.py index 00b3e47..2d0cd73 100644 --- a/lightapi/authentication/base.py +++ b/lightapi/authentication/base.py @@ -2,13 +2,17 @@ from __future__ import annotations -from typing import TYPE_CHECKING, Any +from typing import TYPE_CHECKING, Any, Callable from starlette.requests import Request from starlette.responses import JSONResponse +from lightapi.constants import RESPONSE_KEY_USER + if TYPE_CHECKING: - pass + from lightapi.config import Authentication + +LoginValidator = Callable[[str, str], "dict[str, Any] | None"] class BaseAuthentication: @@ -18,6 +22,22 @@ class BaseAuthentication: By default, allows all requests. """ + @classmethod + def from_config( + cls, + authentication: Authentication, + login_validator: LoginValidator | None = None, + ) -> BaseAuthentication: + """Build the backend for an endpoint's ``Authentication`` settings. + + Backends that take settings override this; the default needs none. + """ + return cls() + + def login_response(self, user: dict[str, Any]) -> dict[str, Any]: + """Body of a successful ``/auth/login`` for a validated user.""" + return {RESPONSE_KEY_USER: user} + def authenticate(self, request: Request) -> bool: """Authenticate a request. diff --git a/lightapi/authentication/basic.py b/lightapi/authentication/basic.py index b9a6b87..f257f63 100644 --- a/lightapi/authentication/basic.py +++ b/lightapi/authentication/basic.py @@ -8,9 +8,10 @@ from starlette.responses import JSONResponse from lightapi._login import _parse_basic_header -from lightapi.authentication.base import BaseAuthentication +from lightapi.authentication.base import BaseAuthentication, LoginValidator if TYPE_CHECKING: + from lightapi.config import Authentication from lightapi.rate_limiter import RateLimiter @@ -29,6 +30,19 @@ def __init__( self.rate_limiter = rate_limiter self._login_validator = login_validator + @classmethod + def from_config( + cls, + authentication: Authentication, + login_validator: LoginValidator | None = None, + ) -> BasicAuthentication: + # A subclass with its own constructor keeps being built without arguments. + if cls.__init__ is not BasicAuthentication.__init__: + return cls() + return cls( + rate_limiter=authentication.rate_limiter, login_validator=login_validator + ) + def authenticate(self, request: Request) -> bool: if request.method == "OPTIONS": return True diff --git a/lightapi/authentication/jwt.py b/lightapi/authentication/jwt.py index 50f2292..18f380b 100644 --- a/lightapi/authentication/jwt.py +++ b/lightapi/authentication/jwt.py @@ -9,10 +9,12 @@ from starlette.requests import Request from starlette.responses import JSONResponse -from lightapi.authentication.base import BaseAuthentication +from lightapi.authentication.base import BaseAuthentication, LoginValidator from lightapi.config import config +from lightapi.constants import RESPONSE_KEY_TOKEN, RESPONSE_KEY_USER if TYPE_CHECKING: + from lightapi.config import Authentication from lightapi.rate_limiter import RateLimiter @@ -28,10 +30,36 @@ def __init__( algorithm: str | None = None, expiration: int | None = None, rate_limiter: "RateLimiter | None" = None, + extra_claims: tuple[str, ...] = (), ) -> None: self.algorithm = algorithm self.expiration = expiration self.rate_limiter = rate_limiter + self.extra_claims = extra_claims + + @classmethod + def from_config( + cls, + authentication: Authentication, + login_validator: LoginValidator | None = None, + ) -> JWTAuthentication: + # A subclass with its own constructor keeps being built without arguments. + if cls.__init__ is not JWTAuthentication.__init__: + return cls() + return cls( + algorithm=authentication.jwt_algorithm, + expiration=authentication.jwt_expiration, + rate_limiter=authentication.rate_limiter, + extra_claims=tuple(authentication.jwt_extra_claims or ()), + ) + + def login_response(self, user: dict[str, Any]) -> dict[str, Any]: + """Token plus user. The token carries ``extra_claims`` when the user has any.""" + claims = {key: user[key] for key in self.extra_claims if key in user} + return { + RESPONSE_KEY_TOKEN: self.generate_token(claims or user), + RESPONSE_KEY_USER: user, + } def authenticate(self, request: Request) -> bool: if request.method == "OPTIONS": diff --git a/lightapi/config.py b/lightapi/config.py index 4cab5a6..ea3ac1e 100644 --- a/lightapi/config.py +++ b/lightapi/config.py @@ -2,7 +2,7 @@ import os from dataclasses import dataclass, field -from typing import TYPE_CHECKING, Any +from typing import TYPE_CHECKING, Any, Callable from lightapi.constants import ( DEFAULT_CACHE_TTL, @@ -15,6 +15,7 @@ from lightapi.exceptions import ConfigurationError if TYPE_CHECKING: + from lightapi.authentication import BaseAuthentication from lightapi.rate_limiter import RateLimiter @@ -57,12 +58,41 @@ class Authentication: """Authentication configuration for a RestEndpoint.""" backend: type | None = None - permission: type | None = None + # One permission class for every method, or {"GET": ..., "DELETE": ...}. + permission: type | dict[str, type] | None = None jwt_expiration: int | None = None jwt_extra_claims: tuple[str, ...] = field(default_factory=tuple) jwt_algorithm: str | None = None rate_limiter: "RateLimiter | None" = field(default=None, repr=False) + def permission_for(self, method: str) -> type: + """Permission class for an HTTP method; AllowAny when none is set for it.""" + # lightapi.authentication imports this module, so this cannot be a top import. + from lightapi.authentication import AllowAny + + if isinstance(self.permission, dict): + return self.permission.get(method) or AllowAny + return self.permission or AllowAny + + def requires_login(self, method: str) -> bool: + """Whether the backend has to authenticate a request with this method. + + A backend with no permission set still authenticates. An AllowAny + permission, for every method or for this one, makes the request public. + """ + from lightapi.authentication import AllowAny + + if self.backend is None: + return False + if self.permission is None: + return True + return self.permission_for(method) is not AllowAny + + def build_backend( + self, login_validator: Callable[[str, str], Any] | None = None + ) -> BaseAuthentication: + return self.backend.from_config(self, login_validator) + @dataclass(frozen=True) class Filtering: diff --git a/lightapi/endpoint_handler.py b/lightapi/endpoint_handler.py index 96a5984..0ac52ca 100644 --- a/lightapi/endpoint_handler.py +++ b/lightapi/endpoint_handler.py @@ -6,13 +6,13 @@ import inspect import json from dataclasses import dataclass -from typing import Any, Callable +from typing import TYPE_CHECKING, Any, Callable from starlette.background import BackgroundTasks from starlette.requests import Request from starlette.responses import JSONResponse, Response -from lightapi.auth_checker import check_auth +from lightapi.auth_checker import EndpointGuard from lightapi.cache_helper import ( invalidate_cache_after_write, maybe_cached, @@ -23,6 +23,18 @@ from lightapi.rest import RestEndpoint from lightapi.session import run_blocking +if TYPE_CHECKING: + from lightapi.authentication.base import LoginValidator + + +@dataclass(frozen=True) +class AppContext: + """What every handler of one LightApi app shares.""" + + middlewares: list[type] + is_async: bool + login_validator: LoginValidator | None = None + @dataclass(frozen=True) class _Verb: @@ -78,25 +90,27 @@ def __init__( self, endpoint_cls: type[RestEndpoint], verbs: dict[str, _Verb], - middlewares: list[type], - is_async: bool, + app: AppContext, ) -> None: self._endpoint_cls = endpoint_cls self._verbs = verbs - self._middlewares = middlewares - self._is_async = is_async + self._middlewares = app.middlewares + self._is_async = app.is_async + self._guard = EndpointGuard( + endpoint_cls._meta.get("authentication"), app.login_validator + ) @classmethod def for_collection( - cls, endpoint_cls: type[RestEndpoint], middlewares: list[type], is_async: bool + cls, endpoint_cls: type[RestEndpoint], app: AppContext ) -> EndpointHandler: - return cls(endpoint_cls, _COLLECTION_VERBS, middlewares, is_async) + return cls(endpoint_cls, _COLLECTION_VERBS, app) @classmethod def for_detail( - cls, endpoint_cls: type[RestEndpoint], middlewares: list[type], is_async: bool + cls, endpoint_cls: type[RestEndpoint], app: AppContext ) -> EndpointHandler: - return cls(endpoint_cls, _DETAIL_VERBS, middlewares, is_async) + return cls(endpoint_cls, _DETAIL_VERBS, app) @property def methods(self) -> list[str]: @@ -112,7 +126,7 @@ async def handle(self, request: Request) -> Response: if pre_result is not None: return pre_result - auth_result = check_auth(self._endpoint_cls, request) + auth_result = self._guard.check(request) if auth_result is not None: return auth_result diff --git a/lightapi/lightapi.py b/lightapi/lightapi.py index d4bb028..706526d 100644 --- a/lightapi/lightapi.py +++ b/lightapi/lightapi.py @@ -7,29 +7,37 @@ import logging import os import warnings -from typing import TYPE_CHECKING, Any, Callable +from typing import Any, Callable import uvicorn from sqlalchemy import create_engine from starlette.applications import Starlette from starlette.middleware.cors import CORSMiddleware as StarletteCORSMiddleware -from starlette.requests import Request -from starlette.responses import Response from starlette.routing import Route -from lightapi.authentication import BasicAuthentication, JWTAuthentication -from lightapi.endpoint_handler import EndpointHandler +from lightapi._login import LoginEndpoint +from lightapi.authentication import ( + BaseAuthentication, + BasicAuthentication, + JWTAuthentication, +) +from lightapi.endpoint_handler import AppContext, EndpointHandler from lightapi.exceptions import ConfigurationError from lightapi.health import HEALTH_PATH, HealthCheckEndpoint +from lightapi.rate_limiter import RateLimiter from lightapi.rest import RestEndpoint from lightapi.session_manager import SessionManager from lightapi.yaml_loader import load_config -if TYPE_CHECKING: - from lightapi.rate_limiter import RateLimiter - logger = logging.getLogger(__name__) +# Limits of the login routes when the app sets none. +_LOGIN_RATE_LIMITS = { + "requests_per_minute": 1000, + "requests_per_hour": 10000, + "requests_per_day": 100000, +} + class LightApi: """Main application class for building REST APIs with LightAPI v2. @@ -93,15 +101,14 @@ def __init__( # Rate limiter setup self._rate_limiter_global: RateLimiter | None = None if rate_limiter is not None: - from lightapi.rate_limiter import RateLimiter as RL - - if isinstance(rate_limiter, RL): + if isinstance(rate_limiter, RateLimiter): self._rate_limiter_global = rate_limiter elif isinstance(rate_limiter, dict): - self._rate_limiter_global = RL( - requests_per_minute=rate_limiter.get("requests_per_minute", 1000), - requests_per_hour=rate_limiter.get("requests_per_hour", 10000), - requests_per_day=rate_limiter.get("requests_per_day", 100000), + self._rate_limiter_global = RateLimiter( + **{ + limit: rate_limiter.get(limit, default) + for limit, default in _LOGIN_RATE_LIMITS.items() + } ) self._routes: list[Route] = [] @@ -256,11 +263,11 @@ class TestIsolationMeta: cls._schema_deferred = False cls._reflect_deferred = False - is_async = self._mode == "async" - collection = EndpointHandler.for_collection( - cls, self._middlewares, is_async + app_context = AppContext( + self._middlewares, self._mode == "async", self._login_validator ) - detail = EndpointHandler.for_detail(cls, self._middlewares, is_async) + collection = EndpointHandler.for_collection(cls, app_context) + detail = EndpointHandler.for_detail(cls, app_context) self._routes.append( Route( path, @@ -279,151 +286,40 @@ class TestIsolationMeta: ) self._endpoint_map[path] = cls - # Auto-register /auth/login and /auth/token when JWT or Basic auth is used - - auth_backends: set[type] = set() - jwt_config_expiration: int | None = None - jwt_config_extra_claims: list[str] | None = None - jwt_config_algorithm: str | None = None - for cls in self._endpoint_map.values(): - auth_cfg = getattr(cls, "_meta", {}).get("authentication") - if auth_cfg and auth_cfg.backend: - auth_backends.add(auth_cfg.backend) - if ( - auth_cfg.backend is JWTAuthentication - and jwt_config_expiration is None - ): - jwt_config_expiration = getattr(auth_cfg, "jwt_expiration", None) - jwt_config_extra_claims = getattr( - auth_cfg, "jwt_extra_claims", None - ) - jwt_config_algorithm = getattr(auth_cfg, "jwt_algorithm", None) - - # Check if any endpoint has authentication configured - auth_backends_list = [] - for cls in self._endpoint_map.values(): - auth_cfg = getattr(cls, "_meta", {}).get("authentication") - if auth_cfg and auth_cfg.backend: - # Store login_validator in auth_cfg for use by _check_auth - # Use object.__setattr__ to bypass frozen dataclass restriction - object.__setattr__(auth_cfg, "_login_validator", self._login_validator) - - # Store backend instance with its config - backend = auth_cfg.backend - # Initialize backend with appropriate parameters based on type - if backend.__name__ == "JWTAuthentication": - backend_instance = backend( - expiration=getattr(auth_cfg, "jwt_expiration", None), - algorithm=getattr(auth_cfg, "jwt_algorithm", None), - rate_limiter=getattr(auth_cfg, "rate_limiter", None), - ) - elif backend.__name__ == "BasicAuthentication": - backend_instance = backend( - rate_limiter=getattr(auth_cfg, "rate_limiter", None), - login_validator=self._login_validator, - ) - else: - backend_instance = backend() - auth_backends_list.append((cls, auth_cfg, backend_instance)) - - # Auto-register /auth/login and /auth/token when JWT or Basic auth is used - has_auth_backends = any( - isinstance(auth[2], (JWTAuthentication, BasicAuthentication)) - for auth in auth_backends_list - ) - - if has_auth_backends: - # Use global rate limiter if set, otherwise create default - rate_limiter = self._rate_limiter_global - if rate_limiter is None: - from lightapi.rate_limiter import RateLimiter + self._register_login_routes() - rate_limiter = RateLimiter( - requests_per_minute=1000, - requests_per_hour=10000, - requests_per_day=100000, - ) - - has_jwt = any( - isinstance(auth[2], JWTAuthentication) for auth in auth_backends_list - ) - auth_path = self._auth_path.rstrip("/") + def _register_login_routes(self) -> None: + """(Re)build /auth/login and /auth/token when an endpoint uses JWT or Basic.""" + backend = self._login_backend() + if backend is None: + return - login_path = f"{auth_path}/login" - token_path = f"{auth_path}/token" + rate_limiter = self._rate_limiter_global or RateLimiter(**_LOGIN_RATE_LIMITS) + login = LoginEndpoint(backend, self._login_validator, rate_limiter) + auth_path = self._auth_path.rstrip("/") + login_paths = (f"{auth_path}/login", f"{auth_path}/token") - self._routes = [ - route - for route in self._routes - if not ( - isinstance(route, Route) and route.path in {login_path, token_path} - ) - ] - - login_endpoint = self._make_login_endpoint( - has_jwt=has_jwt, - jwt_expiration=jwt_config_expiration, - jwt_extra_claims=jwt_config_extra_claims, - jwt_algorithm=jwt_config_algorithm, - rate_limiter=rate_limiter, - ) - self._routes.insert( - 0, - Route( - login_path, - login_endpoint, - methods=["POST"], - ), - ) - self._routes.insert( - 1, - Route( - token_path, - login_endpoint, - methods=["POST"], - ), - ) - - def _make_login_endpoint( - self, - *, - has_jwt: bool, - jwt_expiration: int | None, - jwt_extra_claims: list[str] | None, - jwt_algorithm: str | None, - rate_limiter: "RateLimiter", - ) -> Any: - """Create the login/token handler with captured config.""" - from lightapi._login import login_handler - from lightapi.authentication import JWTAuthentication - - # Create auth backend that wraps login_validator for validate_credentials - auth_backend = JWTAuthentication() - - # Override validate_credentials to use login_validator if provided - original_validate = auth_backend.validate_credentials - - def wrapped_validate(username: str, password: str) -> dict[str, Any] | None: - # Try login_validator first if provided - if self._login_validator is not None: - return self._login_validator(username, password) - # Otherwise try the original method - return original_validate(username, password) - - auth_backend.validate_credentials = wrapped_validate - - async def handler(request: Request) -> Response: - return await login_handler( - request, - has_jwt=has_jwt, - jwt_expiration=jwt_expiration, - jwt_extra_claims=jwt_extra_claims, - jwt_algorithm=jwt_algorithm, - rate_limiter=rate_limiter, - auth_backend=auth_backend, - ) - - return handler + self._routes = [ + route + for route in self._routes + if not (isinstance(route, Route) and route.path in login_paths) + ] + for position, path in enumerate(login_paths): + self._routes.insert(position, Route(path, login.handle, methods=["POST"])) + + def _login_backend(self) -> BaseAuthentication | None: + """The backend that answers the login routes: a JWT one if any, else Basic.""" + configured = [ + authentication + for cls in self._endpoint_map.values() + if (authentication := cls._meta.get("authentication")) + and authentication.backend + ] + for kind in (JWTAuthentication, BasicAuthentication): + for authentication in configured: + if issubclass(authentication.backend, kind): + return authentication.build_backend(self._login_validator) + return None # ───────────────────────────────────────────────────────────────────────── # Run diff --git a/tests/test_login_backend_subclass.py b/tests/test_login_backend_subclass.py new file mode 100644 index 0000000..563c5bf --- /dev/null +++ b/tests/test_login_backend_subclass.py @@ -0,0 +1,154 @@ +"""/auth/login must use validate_credentials() of the backend the endpoint configured.""" + +import os + +import jwt +import pytest +from sqlalchemy import create_engine +from sqlalchemy.pool import StaticPool +from starlette.testclient import TestClient + +from lightapi import ( + Authentication, + IsAuthenticated, + JWTAuthentication, + LightApi, + RestEndpoint, +) +from lightapi.fields import Field as LField + +SECRET = "subclass-login-secret" +EXPIRATION_SECONDS = 120 + + +class SyncCredentialsBackend(JWTAuthentication): + def validate_credentials(self, username: str, password: str): + if password == "right": + return {"sub": username, "is_admin": False} + return None + + +class AsyncCredentialsBackend(JWTAuthentication): + async def validate_credentials(self, username: str, password: str): + if password == "right": + return {"sub": username, "is_admin": True} + return None + + +class OwnConstructorBackend(JWTAuthentication): + def __init__(self) -> None: + super().__init__(expiration=EXPIRATION_SECONDS) + + +class SyncBackendNote(RestEndpoint): + text: str = LField(min_length=1) + + class Meta: + authentication = Authentication( + backend=SyncCredentialsBackend, + permission=IsAuthenticated, + jwt_expiration=EXPIRATION_SECONDS, + ) + + +class AsyncBackendNote(RestEndpoint): + text: str = LField(min_length=1) + + class Meta: + authentication = Authentication( + backend=AsyncCredentialsBackend, permission=IsAuthenticated + ) + + +class OwnConstructorNote(RestEndpoint): + text: str = LField(min_length=1) + + class Meta: + authentication = Authentication( + backend=OwnConstructorBackend, permission=IsAuthenticated + ) + + +@pytest.fixture(autouse=True) +def jwt_secret(monkeypatch): + monkeypatch.setitem(os.environ, "LIGHTAPI_JWT_SECRET", SECRET) + + +def _client(route: str, endpoint: type, **app_kwargs) -> TestClient: + engine = create_engine( + "sqlite:///:memory:", + connect_args={"check_same_thread": False}, + poolclass=StaticPool, + ) + app = LightApi(engine=engine, **app_kwargs) + app.register({route: endpoint}) + return TestClient(app.build_app()) + + +def test_login_uses_the_subclass_validate_credentials(): + client = _client("/syncbackendnotes", SyncBackendNote) + + response = client.post("/auth/login", json={"username": "ana", "password": "right"}) + + assert response.status_code == 200 + claims = jwt.decode(response.json()["token"], SECRET, algorithms=["HS256"]) + assert claims["sub"] == "ana" + assert response.json()["user"] == {"sub": "ana", "is_admin": False} + + +def test_login_with_wrong_password_on_subclass_backend_returns_401(): + client = _client("/syncbackendnotes", SyncBackendNote) + + response = client.post("/auth/login", json={"username": "ana", "password": "wrong"}) + + assert response.status_code == 401 + + +def test_login_token_lifetime_comes_from_the_endpoint_config(): + client = _client("/syncbackendnotes", SyncBackendNote) + + response = client.post("/auth/login", json={"username": "ana", "password": "right"}) + + claims = jwt.decode(response.json()["token"], SECRET, algorithms=["HS256"]) + issued_for = claims["exp"] - _now() + assert EXPIRATION_SECONDS - 5 <= issued_for <= EXPIRATION_SECONDS + + +def test_login_awaits_an_async_validate_credentials(): + client = _client("/asyncbackendnotes", AsyncBackendNote) + + response = client.post("/auth/login", json={"username": "bia", "password": "right"}) + + assert response.status_code == 200 + assert response.json()["user"] == {"sub": "bia", "is_admin": True} + + +def test_app_login_validator_wins_over_the_subclass_override(): + def validator(username: str, password: str): + return {"sub": "from-login-validator"} + + client = _client("/syncbackendnotes", SyncBackendNote, login_validator=validator) + + response = client.post("/auth/login", json={"username": "ana", "password": "wrong"}) + + assert response.status_code == 200 + assert response.json()["user"] == {"sub": "from-login-validator"} + + +def test_backend_with_its_own_constructor_still_protects_the_endpoint(): + client = _client("/ownconstructornotes", OwnConstructorNote) + token = OwnConstructorBackend().generate_token({"sub": "carla"}) + + allowed = client.get( + "/ownconstructornotes", headers={"Authorization": f"Bearer {token}"} + ) + denied = client.get("/ownconstructornotes") + + assert allowed.status_code == 200 + assert denied.status_code == 401 + + +def _now() -> float: + import time + + return time.time() From b220571e6ffa91fc145a80217d64d02075183a6d Mon Sep 17 00:00:00 2001 From: iklo Date: Fri, 18 Sep 2026 10:56:33 -0300 Subject: [PATCH 4/7] refactor: paginators share one render() and config builds them _list picked the paginator with `if pagination_cfg.style == "cursor"` and then had to know that the two paginators return different tuples and take different wrap() arguments. No pagination was a third code path in the same method. Each paginator now has render(request, qs, session, serialize) -> dict, which is its own paginate() followed by its own wrap(). NoPagination is the third implementation. Pagination.build_paginator() picks the class from a table, and _list is five lines with no branch on style. paginate(), paginate_async() and wrap() keep their signatures. PageNumberPaginator() and CursorPaginator() still work with no arguments; page_size is an optional constructor argument now, which is also what docs/api-reference/pagination.md already shows. Tests: 345 passed, 1 xfailed, 2 xpassed. --- lightapi/config.py | 4 ++++ lightapi/pagination.py | 50 +++++++++++++++++++++++++++++++++++++++++- lightapi/rest.py | 33 +++++----------------------- 3 files changed, 59 insertions(+), 28 deletions(-) diff --git a/lightapi/config.py b/lightapi/config.py index ea3ac1e..26a279c 100644 --- a/lightapi/config.py +++ b/lightapi/config.py @@ -13,6 +13,7 @@ VALID_PAGINATION_STYLES, ) from lightapi.exceptions import ConfigurationError +from lightapi.pagination import PAGINATORS, Paginator if TYPE_CHECKING: from lightapi.authentication import BaseAuthentication @@ -120,6 +121,9 @@ def __post_init__(self): if self.page_size < 1: raise ConfigurationError("Pagination page_size must be a positive integer.") + def build_paginator(self) -> Paginator: + return PAGINATORS[self.style](self.page_size) + @dataclass(frozen=True) class Cache: diff --git a/lightapi/pagination.py b/lightapi/pagination.py index dc50445..54ca5bd 100644 --- a/lightapi/pagination.py +++ b/lightapi/pagination.py @@ -3,7 +3,7 @@ import base64 import json import math -from typing import TYPE_CHECKING, Any +from typing import TYPE_CHECKING, Any, Callable, Protocol from sqlalchemy import func, select from sqlalchemy.orm import Session @@ -11,6 +11,7 @@ from lightapi.constants import ( CURSOR_PARAM, + DEFAULT_PAGE_SIZE, PAGE_PARAM, RESPONSE_KEY_COUNT, RESPONSE_KEY_NEXT, @@ -31,9 +32,41 @@ def decode_cursor(cursor: str) -> int: return json.loads(base64.urlsafe_b64decode(cursor.encode()))["id"] +RowSerializer = Callable[[Any], dict[str, Any]] + + +class Paginator(Protocol): + """Turns a query into the body of a list response.""" + + def render( + self, request: Request, qs: Any, session: Session, serialize: RowSerializer + ) -> dict[str, Any]: ... + + +class NoPagination: + """Every row in a single response.""" + + def render( + self, request: Request, qs: Any, session: Session, serialize: RowSerializer + ) -> dict[str, Any]: + rows = session.execute(qs).scalars().all() + return {RESPONSE_KEY_RESULTS: [serialize(row) for row in rows]} + + class PageNumberPaginator: """Page-number based paginator that returns count/next/previous/results.""" + def __init__(self, page_size: int = DEFAULT_PAGE_SIZE) -> None: + self.page_size = page_size + + def render( + self, request: Request, qs: Any, session: Session, serialize: RowSerializer + ) -> dict[str, Any]: + rows, total = self.paginate(request, qs, session, self.page_size) + page = int(request.query_params.get(PAGE_PARAM, 1)) + results = [serialize(row) for row in rows] + return self.wrap(request, results, total, page, self.page_size) + def paginate( self, request: Request, @@ -91,6 +124,15 @@ def wrap( class CursorPaginator: """Keyset cursor-based paginator using base64(json({"id": last_id})).""" + def __init__(self, page_size: int = DEFAULT_PAGE_SIZE) -> None: + self.page_size = page_size + + def render( + self, request: Request, qs: Any, session: Session, serialize: RowSerializer + ) -> dict[str, Any]: + rows, next_cursor = self.paginate(request, qs, session, self.page_size) + return self.wrap([serialize(row) for row in rows], next_cursor, None) + def paginate( self, request: Request, @@ -147,3 +189,9 @@ def wrap( RESPONSE_KEY_PREVIOUS: prev_cursor, RESPONSE_KEY_RESULTS: results, } + + +PAGINATORS: dict[str, type[PageNumberPaginator] | type[CursorPaginator]] = { + "page_number": PageNumberPaginator, + "cursor": CursorPaginator, +} diff --git a/lightapi/rest.py b/lightapi/rest.py index 562aa6c..2769bbc 100644 --- a/lightapi/rest.py +++ b/lightapi/rest.py @@ -6,6 +6,7 @@ import datetime from collections.abc import Callable from decimal import Decimal +from functools import partial from typing import TYPE_CHECKING, Any, get_args, get_origin from uuid import UUID @@ -33,10 +34,10 @@ from lightapi.constants import ( AUTO_FIELDS, RESPONSE_KEY_DETAIL, - RESPONSE_KEY_RESULTS, HTTPStatus, ) from lightapi.exceptions import ConfigurationError +from lightapi.pagination import NoPagination from lightapi.schema import ( SchemaFactory, _apply_fields, @@ -343,33 +344,11 @@ def _serialize_row(self, row: Any, method: str) -> dict[str, Any]: # ── CRUD core: one implementation, always given an open sync Session ───── def _list(self, session: Session, request: Request, qs: Any) -> Response: - pagination_cfg = self._meta.get("pagination") + pagination = self._meta.get("pagination") + paginator = pagination.build_paginator() if pagination else NoPagination() qs = self._run_filter_backends(request, qs) - - if pagination_cfg: - from lightapi.pagination import CursorPaginator, PageNumberPaginator - - if pagination_cfg.style == "cursor": - pager = CursorPaginator() - rows, next_cursor = pager.paginate( - request, qs, session, pagination_cfg.page_size - ) - results = [self._serialize_row(r, "GET") for r in rows] - return JSONResponse(pager.wrap(results, next_cursor, None)) - else: - pager = PageNumberPaginator() - page = int(request.query_params.get("page", 1)) - rows, total = pager.paginate( - request, qs, session, pagination_cfg.page_size - ) - results = [self._serialize_row(r, "GET") for r in rows] - return JSONResponse( - pager.wrap(request, results, total, page, pagination_cfg.page_size) - ) - - instances = session.execute(qs).scalars().all() - results = [self._serialize_row(inst, "GET") for inst in instances] - return JSONResponse({RESPONSE_KEY_RESULTS: results}) + serialize = partial(self._serialize_row, method="GET") + return JSONResponse(paginator.render(request, qs, session, serialize)) def _retrieve(self, session: Session, pk: int) -> Response: cls = type(self) From 0cbcd72cde511039fc9514000529bf3da550aa3a Mon Sep 17 00:00:00 2001 From: iklo Date: Fri, 18 Sep 2026 11:00:20 -0300 Subject: [PATCH 5/7] refactor: table sources replace five class flags and five mapper classes The metaclass left five loose attributes on every endpoint class (_reflect_deferred, _schema_deferred, _all_columns, _reflect_partial_columns and, until recently, _test_isolation_table_name) and register() read them back with getattr(cls, "_flag", False) to decide how to map the table. The mapping itself went through TableMapper, ImperativeTableBuilder, ReflectedTableBuilder, TableNameResolver and FieldInfoStripper plus a module singleton, for two operations. The test-isolation naming rule was written twice, in register() and in TableNameResolver. The metaclass now stores one object, cls._table_source: DeclaredTable (columns from the annotated fields) or ReflectedTable (Meta.reflect). register() calls cls._table_source.map(cls, session_manager) and knows nothing else. ReflectedTable also builds the schemas, since they depend on the reflected columns. SessionManager answers what was being read from its private attributes: is_async, table_name_for(base_name) and create_tables_now(). The isolation naming rule lives only there. table_mapping.py: 239 lines to 133. register() loses 70 lines. Tests: 345 passed, 1 xfailed, 2 xpassed, including the 460-line test_reflection.py and test_async_reflection.py unchanged. --- lightapi/lightapi.py | 70 +------- lightapi/rest.py | 18 +- lightapi/session_manager.py | 15 ++ lightapi/table_mapping.py | 318 ++++++++++++------------------------ lightapi/yaml_loader.py | 2 +- 5 files changed, 130 insertions(+), 293 deletions(-) diff --git a/lightapi/lightapi.py b/lightapi/lightapi.py index 706526d..2d63bfd 100644 --- a/lightapi/lightapi.py +++ b/lightapi/lightapi.py @@ -177,48 +177,7 @@ def register(self, mapping: dict[str, type]) -> None: # Inject session manager into endpoint class cls._session_manager = self._session_manager - # Always map when test isolation is enabled or when not already mapped - if not getattr(cls, "_reflect_deferred", False): - from lightapi.session_manager import get_unique_table_name - from lightapi.table_mapping import map_imperatively - - # Use test isolation table name if available - meta_obj = getattr(cls, "Meta", None) - table_name = ( - getattr(meta_obj, "table", None) or f"{cls.__name__.lower()}s" - ) - - # Always re-map with test-specific metadata when test isolation is enabled - if self._session_manager._use_test_isolation: - # Generate unique table name for test isolation - table_name = get_unique_table_name(table_name) - logger.debug( - "Mapping %s with test isolation table name: %s", - cls.__name__, - table_name, - ) - - # Create a new meta object with the unique table name - class TestIsolationMeta: - table = table_name - reflect = getattr(meta_obj, "reflect", False) - - map_imperatively( - cls, - cls.__name__, - columns=getattr(cls, "_all_columns", []), - meta_obj=TestIsolationMeta, - session_manager=self._session_manager, - ) - else: - # Always map when not using test isolation - map_imperatively( - cls, - cls.__name__, - columns=getattr(cls, "_all_columns", []), - meta_obj=meta_obj, - session_manager=self._session_manager, - ) + cls._table_source.map(cls, self._session_manager) # Log registration for transparency logger.info(f"Registering endpoint {path} -> {cls.__name__}") @@ -236,33 +195,6 @@ class TestIsolationMeta: RuntimeWarning, stacklevel=2, ) - # Perform deferred reflection now that an engine is available - if getattr(cls, "_reflect_deferred", False): - from lightapi.table_mapping import map_reflected as _map_reflected - - meta_obj = getattr(cls, "Meta", None) or type("Meta", (), {}) - partial = cls._meta.get("reflect") == "partial" - extra_cols = getattr(cls, "_reflect_partial_columns", []) - _map_reflected( - cls, - cls.__name__, - meta_obj, - partial, - extra_cols, - session_manager=self._session_manager, - ) - if getattr(cls, "_schema_deferred", False): - from sqlalchemy import inspect as sa_inspect - - from lightapi.schema import SchemaFactory - - table = sa_inspect(cls).persist_selectable - cls.__schema_create__, cls.__schema_read__ = ( - SchemaFactory.build_from_reflected_table(cls, table) - ) - cls._schema_deferred = False - cls._reflect_deferred = False - app_context = AppContext( self._middlewares, self._mode == "async", self._login_validator ) diff --git a/lightapi/rest.py b/lightapi/rest.py index 2769bbc..080cc80 100644 --- a/lightapi/rest.py +++ b/lightapi/rest.py @@ -46,6 +46,7 @@ resolve_fields, ) from lightapi.session import get_async_session, get_sync_session +from lightapi.table_mapping import DeclaredTable, ReflectedTable, TableSource _AUTO_FIELDS = AUTO_FIELDS @@ -199,12 +200,11 @@ def _process(cls: type, name: str, namespace: dict[str, Any]) -> None: # ── Step 4: SchemaFactory ───────────────────────────────────────────── if reflect is True or reflect == "full" or reflect == "partial": + # Built by ReflectedTable.map() once the columns are known. cls.__schema_create__ = None # type: ignore[attr-defined] cls.__schema_read__ = None # type: ignore[attr-defined] - cls._schema_deferred = True # type: ignore[attr-defined] else: cls.__schema_create__, cls.__schema_read__ = SchemaFactory.build(cls) # type: ignore[attr-defined] - cls._schema_deferred = False # type: ignore[attr-defined] # ── Step 5: Parse Meta → _meta ──────────────────────────────────────── meta_obj = namespace.get("Meta") or getattr(cls, "Meta", None) @@ -246,16 +246,11 @@ def _process(cls: type, name: str, namespace: dict[str, Any]) -> None: allowed.add(http_method) cls._allowed_methods = allowed if allowed else set(_ALL_METHODS) # type: ignore[attr-defined] - # ── Step 7: Imperative SQLAlchemy mapping ───────────────────────────── + # ── Step 7: Table source, mapped by LightApi.register() ─────────────── if reflect is True or reflect == "full" or reflect == "partial": - # Defer reflection until LightApi.register() when an engine is available. - cls._reflect_deferred = True # type: ignore[attr-defined] - cls._reflect_partial_columns = columns if reflect == "partial" else [] # type: ignore[attr-defined] + cls._table_source = ReflectedTable(columns, partial=reflect == "partial") # type: ignore[attr-defined] else: - cls._reflect_deferred = False # type: ignore[attr-defined] - all_columns = auto_cols + columns - # Store columns for potential re-mapping during registration - cls._all_columns = all_columns # type: ignore[attr-defined] + cls._table_source = DeclaredTable(auto_cols + columns) # type: ignore[attr-defined] class RestEndpoint(metaclass=RestEndpointMeta): @@ -265,7 +260,8 @@ class RestEndpoint(metaclass=RestEndpointMeta): The metaclass auto-generates SQLAlchemy columns and Pydantic schemas. """ - _model_class: type + _model_class: type | None = None + _table_source: TableSource _meta: dict[str, Any] _allowed_methods: set[str] _fields_info: dict[str, Any] diff --git a/lightapi/session_manager.py b/lightapi/session_manager.py index 59f5c10..d422d03 100644 --- a/lightapi/session_manager.py +++ b/lightapi/session_manager.py @@ -96,3 +96,18 @@ def metadata(self) -> MetaData: def registry(self) -> registry: """Get the registry.""" return self._registry + + @property + def is_async(self) -> bool: + return self._is_async + + def table_name_for(self, base_name: str) -> str: + """The table name to use; test isolation gives every mapping a fresh one.""" + if self._use_test_isolation: + return get_unique_table_name(base_name) + return base_name + + def create_tables_now(self) -> None: + """Create missing tables on a sync engine; async engines wait for the loop.""" + if not self._is_async: + self._metadata.create_all(self._engine) diff --git a/lightapi/table_mapping.py b/lightapi/table_mapping.py index efc924b..4f74c8a 100644 --- a/lightapi/table_mapping.py +++ b/lightapi/table_mapping.py @@ -1,239 +1,133 @@ -from typing import Any +"""Where an endpoint's table comes from: its declared fields or an existing table.""" +from __future__ import annotations + +import asyncio +import concurrent.futures +from typing import TYPE_CHECKING, Any, Protocol + +from pydantic.fields import FieldInfo from sqlalchemy import Column, MetaData, Table -from sqlalchemy.orm import registry - - -class TableNameResolver: - @staticmethod - def resolve(meta_obj: Any, name: str, session_manager: Any | None = None) -> str: - base_name = getattr(meta_obj, "table", None) or f"{name.lower()}s" - if session_manager is not None and getattr( - session_manager, "_use_test_isolation", False - ): - from lightapi.session_manager import get_unique_table_name - - return get_unique_table_name(base_name) - return base_name - - -class FieldInfoStripper: - @staticmethod - def strip(cls: type, columns: list[Column]) -> None: - from pydantic.fields import FieldInfo - - for col in columns: - existing = cls.__dict__.get(col.name) - # Remove FieldInfo objects AND bare Ellipsis sentinels. Both block - # SQLAlchemy's instrumentation from replacing the class attribute - # with an InstrumentedAttribute, which silently drops the column - # from the mapper. - if isinstance(existing, FieldInfo) or existing is ...: - try: - delattr(cls, col.name) - except AttributeError: - pass - - -class ImperativeTableBuilder: - def build( - self, - cls: type, - name: str, - columns: list[Column], - meta_obj: Any, - session_manager: Any | None = None, - ) -> Table: - registry_obj, metadata = self._get_registry_and_metadata(session_manager) - table_name = TableNameResolver.resolve(meta_obj, name, session_manager) - - FieldInfoStripper.strip(cls, columns) - - copied_columns = [col.copy() for col in columns] +from sqlalchemy import inspect as sa_inspect + +from lightapi.exceptions import ConfigurationError +from lightapi.schema import SchemaFactory + +if TYPE_CHECKING: + from lightapi.session_manager import SessionManager + + +class TableSource(Protocol): + """Builds the endpoint's table and maps the endpoint class onto it.""" + + def map(self, endpoint_cls: type, session_manager: SessionManager) -> None: ... + + +class DeclaredTable: + """A table generated from the endpoint's annotated fields.""" + + def __init__(self, columns: list[Column]) -> None: + self._columns = columns + + def map(self, endpoint_cls: type, session_manager: SessionManager) -> None: + if endpoint_cls._model_class is not None: + return + _strip_field_infos(endpoint_cls, self._columns) table = Table( - table_name, - metadata, - *copied_columns, + _table_name(endpoint_cls, session_manager), + session_manager.metadata, + *[column.copy() for column in self._columns], extend_existing=True, keep_existing=False, ) - return table - - def _get_registry_and_metadata( - self, session_manager: Any | None = None - ) -> tuple[registry, MetaData]: - from lightapi.session_manager import _GLOBAL_METADATA, _GLOBAL_REGISTRY - - if session_manager is not None: - return session_manager.registry, session_manager.metadata - return _GLOBAL_REGISTRY, _GLOBAL_METADATA - - -class ReflectedTableBuilder: - def build( - self, - cls: type, - name: str, - meta_obj: Any, - partial: bool, - extra_columns: list[Column] | None = None, - session_manager: Any | None = None, - ) -> Table: - if session_manager is None: - raise RuntimeError( - "session_manager is required for reflection. " - "Ensure LightApi.register() was called with a properly configured app." - ) - - metadata = session_manager.metadata - engine = session_manager.engine - table_name = TableNameResolver.resolve(meta_obj, name, session_manager) + session_manager.create_tables_now() + session_manager.registry.map_imperatively(endpoint_cls, table) + endpoint_cls._model_class = endpoint_cls - is_async = hasattr(engine, "sync_engine") - if is_async: - table = self._reflect_async(engine, metadata, table_name) - else: - table = self._reflect_sync(engine, metadata, table_name) - if partial and extra_columns: - for col in extra_columns: - if col.name not in table.c: - table.append_column(col) +class ReflectedTable: + """A table that already exists in the database (``Meta.reflect``). - if partial: - FieldInfoStripper.strip(cls, list(table.c)) + With ``partial`` the endpoint's own fields are added to the reflected columns. + """ - return table + def __init__(self, partial_columns: list[Column], partial: bool) -> None: + self._partial_columns = partial_columns + self._partial = partial - def _reflect_sync(self, engine: Any, metadata: MetaData, table_name: str) -> Table: - from sqlalchemy import inspect as sa_inspect_engine - - from lightapi.exceptions import ConfigurationError - - inspector = sa_inspect_engine(engine) - table_names = inspector.get_table_names() - if table_name not in table_names: - raise ConfigurationError( - f"Table '{table_name}' does not exist in the database." - ) - return Table(table_name, metadata, autoload_with=engine) - - def _reflect_async(self, engine: Any, metadata: MetaData, table_name: str) -> Table: - import asyncio - import concurrent.futures - - def do_reflect(conn: Any) -> None: - metadata.reflect(bind=conn, only=[table_name]) - - async def async_reflect() -> None: - async with engine.connect() as conn: - await conn.run_sync(do_reflect) - - try: - asyncio.get_running_loop() - with concurrent.futures.ThreadPoolExecutor(max_workers=1) as pool: - pool.submit(asyncio.run, async_reflect()).result() - except RuntimeError: - asyncio.run(async_reflect()) - - if table_name not in metadata.tables: - from lightapi.exceptions import ConfigurationError - - raise ConfigurationError(f"Table '{table_name}' could not be reflected.") - return metadata.tables[table_name] - - -class TableMapper: - def __init__(self) -> None: - self._imperative_builder = ImperativeTableBuilder() - self._reflected_builder = ReflectedTableBuilder() - - def map_imperatively( - self, - cls: type, - name: str, - columns: list[Column], - meta_obj: Any, - session_manager: Any | None = None, - ) -> None: - if getattr(cls, "_model_class", None) is not None: + def map(self, endpoint_cls: type, session_manager: SessionManager) -> None: + if endpoint_cls._model_class is not None: return - table = self._imperative_builder.build( - cls, name, columns, meta_obj, session_manager + table = self._reflect( + _table_name(endpoint_cls, session_manager), session_manager ) - registry_obj, metadata = self._get_registry_and_metadata(session_manager) - - if session_manager is not None and hasattr(session_manager, "engine"): - engine = session_manager.engine - is_async = getattr(session_manager, "_is_async", False) - if is_async: - pass - elif hasattr(engine, "sync_engine"): - engine = engine.sync_engine - metadata.create_all(engine) - else: - metadata.create_all(engine) - - registry_obj.map_imperatively(cls, table) - cls._model_class = cls # type: ignore[attr-defined] - - def map_reflected( - self, - cls: type, - name: str, - meta_obj: Any, - partial: bool, - extra_columns: list[Column] | None = None, - session_manager: Any | None = None, - ) -> None: - table = self._reflected_builder.build( - cls, name, meta_obj, partial, extra_columns, session_manager + if self._partial: + for column in self._partial_columns: + if column.name not in table.c: + table.append_column(column) + _strip_field_infos(endpoint_cls, list(table.c)) + + session_manager.registry.map_imperatively(endpoint_cls, table) + endpoint_cls._model_class = endpoint_cls + # The schemas need the columns, which are only known after reflection. + mapped_table = sa_inspect(endpoint_cls).persist_selectable + endpoint_cls.__schema_create__, endpoint_cls.__schema_read__ = ( + SchemaFactory.build_from_reflected_table(endpoint_cls, mapped_table) ) - registry = session_manager.registry - try: - from sqlalchemy import inspect as sa_inspect + def _reflect(self, table_name: str, session_manager: SessionManager) -> Table: + if session_manager.is_async: + return _reflect_async( + session_manager.engine, session_manager.metadata, table_name + ) + return _reflect_sync( + session_manager.engine, session_manager.metadata, table_name + ) - sa_inspect(cls) - cls._model_class = cls # type: ignore[attr-defined] - return - except Exception: - pass - registry.map_imperatively(cls, table) - cls._model_class = cls # type: ignore[attr-defined] +def _table_name(endpoint_cls: type, session_manager: SessionManager) -> str: + base_name = endpoint_cls._meta.get("table") or f"{endpoint_cls.__name__.lower()}s" + return session_manager.table_name_for(base_name) - def _get_registry_and_metadata( - self, session_manager: Any | None = None - ) -> tuple[registry, MetaData]: - from lightapi.session_manager import _GLOBAL_METADATA, _GLOBAL_REGISTRY - if session_manager is not None: - return session_manager.registry, session_manager.metadata - return _GLOBAL_REGISTRY, _GLOBAL_METADATA +def _strip_field_infos(endpoint_cls: type, columns: list[Column]) -> None: + for column in columns: + existing = endpoint_cls.__dict__.get(column.name) + # Remove FieldInfo objects AND bare Ellipsis sentinels. Both block + # SQLAlchemy's instrumentation from replacing the class attribute + # with an InstrumentedAttribute, which silently drops the column + # from the mapper. + if isinstance(existing, FieldInfo) or existing is ...: + delattr(endpoint_cls, column.name) -_mapper = TableMapper() +def _reflect_sync(engine: Any, metadata: MetaData, table_name: str) -> Table: + if table_name not in sa_inspect(engine).get_table_names(): + raise ConfigurationError( + f"Table '{table_name}' does not exist in the database." + ) + return Table(table_name, metadata, autoload_with=engine) + +def _reflect_async(engine: Any, metadata: MetaData, table_name: str) -> Table: + def do_reflect(conn: Any) -> None: + metadata.reflect(bind=conn, only=[table_name]) -def map_imperatively( - cls: type, - name: str, - columns: list[Column], - meta_obj: Any, - session_manager: Any | None = None, -) -> None: - _mapper.map_imperatively(cls, name, columns, meta_obj, session_manager) + async def async_reflect() -> None: + async with engine.connect() as conn: + await conn.run_sync(do_reflect) + try: + asyncio.get_running_loop() + except RuntimeError: + asyncio.run(async_reflect()) + else: + # Already inside a loop (tests, notebooks): reflect on a loop of its own. + with concurrent.futures.ThreadPoolExecutor(max_workers=1) as pool: + pool.submit(asyncio.run, async_reflect()).result() -def map_reflected( - cls: type, - name: str, - meta_obj: Any, - partial: bool, - extra_columns: list[Column] | None = None, - session_manager: Any | None = None, -) -> None: - _mapper.map_reflected(cls, name, meta_obj, partial, extra_columns, session_manager) + if table_name not in metadata.tables: + raise ConfigurationError(f"Table '{table_name}' could not be reflected.") + return metadata.tables[table_name] diff --git a/lightapi/yaml_loader.py b/lightapi/yaml_loader.py index d1b670d..4041c53 100644 --- a/lightapi/yaml_loader.py +++ b/lightapi/yaml_loader.py @@ -628,7 +628,7 @@ def _build_endpoint_class(entry: EndpointConfig, defaults: DefaultsConfig) -> ty pydantic_kwargs["default"] = default class_attrs[field_name] = Field(**pydantic_kwargs) # A constraint-less, default-less field is annotation-only. Do NOT set a - # class attribute: an Ellipsis placeholder would survive FieldInfoStripper + # class attribute: an Ellipsis placeholder would survive _strip_field_infos # (which only removes FieldInfo) and stop SQLAlchemy from mapping the # column, so the value would silently never be inserted. From cd2cc9aa9fbdd7aa32b667aa1bd8c0ed79fa9bf4 Mon Sep 17 00:00:00 2001 From: iklo Date: Fri, 18 Sep 2026 11:01:54 -0300 Subject: [PATCH 6/7] refactor: take reflected column types from SQLAlchemy's python_type build_from_reflected_table mapped column types to Python types with a 70-line chain: isinstance checks, then __mro__ checks, then a second pass over lowercase type names, plus two try/except ImportError blocks for Uuid types that the dependency floor already guarantees. SQLAlchemy keeps that mapping itself as column.type.python_type. The function now asks for it and accepts the answer when it is one of the nine scalar types the chain knew (int, str, float, bool, Decimal, UUID, datetime, date, time). Anything else, or a type with no python_type, stays Any with the same warning, so JSON and binary columns validate as before. Checked that python_type agrees with the old chain for Integer, BigInteger, SmallInteger, String, Text, Numeric, Float, Boolean, DateTime, Date, Time, Uuid and the PostgreSQL UUID. schema.py: 302 lines to 241. The reflection suites pass unchanged. Tests: 345 passed, 1 xfailed, 2 xpassed. --- lightapi/schema.py | 133 ++++++++++++--------------------------------- 1 file changed, 35 insertions(+), 98 deletions(-) diff --git a/lightapi/schema.py b/lightapi/schema.py index 0347a76..6633961 100644 --- a/lightapi/schema.py +++ b/lightapi/schema.py @@ -2,7 +2,9 @@ import datetime import logging +from decimal import Decimal from typing import Any, Optional +from uuid import UUID from pydantic import ConfigDict, create_model from pydantic.fields import FieldInfo @@ -82,6 +84,38 @@ def _apply_fields(d: dict[str, Any], fields: list[str] | None) -> dict[str, Any] return {k: v for k, v in d.items() if k in fields} +# Python types a reflected column is validated as; any other type stays ``Any``. +_REFLECTED_PYTHON_TYPES = frozenset( + { + int, + str, + float, + bool, + Decimal, + UUID, + datetime.datetime, + datetime.date, + datetime.time, + } +) + + +def _reflected_annotation(column: Any) -> Any: + """Annotation for a reflected column, from SQLAlchemy's own type mapping.""" + try: + python_type = column.type.python_type + except NotImplementedError: + python_type = None + if python_type in _REFLECTED_PYTHON_TYPES: + return python_type + logger.warning( + "Unknown SQLAlchemy type %s for column %s; using Any", + type(column.type).__name__, + column.name, + ) + return Any + + class SchemaFactory: """Builds Pydantic validation models from a RestEndpoint class.""" @@ -169,109 +203,12 @@ def build_from_reflected_table(cls: type, table: Any) -> tuple[type, type]: Includes all columns in read schema. Uses Optional[T] when column.nullable is True. """ - from decimal import Decimal - from uuid import UUID - - from sqlalchemy import ( - Boolean, - Date, - DateTime, - Float, - Integer, - Numeric, - SmallInteger, - String, - Text, - Time, - ) - from sqlalchemy.types import BigInteger - - try: - from sqlalchemy import Uuid as SAUuid - except ImportError: - SAUuid = None # type: ignore[assignment,misc] - try: - from sqlalchemy.dialects.postgresql import UUID as PG_UUID - except ImportError: - PG_UUID = None # type: ignore[assignment,misc] - - def _col_type_to_annotation(col: Any) -> Any | None: - """Map SQLAlchemy column type to Pydantic annotation. None if unknown.""" - col_type = type(col.type) - if isinstance(col.type, (Integer, BigInteger, SmallInteger)): - return int - if col_type in (String, Text) or issubclass(col_type, String): - return str - if col_type == Numeric or ( - hasattr(Numeric, "__mro__") and Numeric in col_type.__mro__ - ): - return Decimal - if col_type == Float or ( - hasattr(Float, "__mro__") and Float in getattr(col_type, "__mro__", ()) - ): - return float - if col_type == Boolean: - return bool - if col_type == Time or ( - hasattr(Time, "__mro__") and Time in getattr(col_type, "__mro__", ()) - ): - return datetime.time - if col_type in (DateTime,) or ( - hasattr(DateTime, "__mro__") - and DateTime in getattr(col_type, "__mro__", ()) - ): - return datetime.datetime - if col_type == Date or ( - hasattr(Date, "__mro__") and Date in getattr(col_type, "__mro__", ()) - ): - return datetime.date - if SAUuid is not None and col_type == SAUuid: - return UUID - if PG_UUID is not None and col_type == PG_UUID: - return UUID - type_name = ( - getattr(col.type, "__visit_name__", "") or col_type.__name__ or "" - ).lower() - if type_name in ("integer", "big_integer", "small_integer", "int"): - return int - if type_name in ( - "string", - "varchar", - "char", - "text", - "unicode", - "unicode_text", - ): - return str - if type_name in ("numeric", "decimal"): - return Decimal - if type_name in ("float", "double", "real"): - return float - if type_name == "boolean": - return bool - if type_name in ("datetime", "timestamp"): - return datetime.datetime - if type_name == "date": - return datetime.date - if type_name == "time": - return datetime.time - if type_name == "uuid": - return UUID - logger.warning( - "Unknown SQLAlchemy type %s for column %s; using Any", - col_type.__name__, - col.name, - ) - return None - create_fields: dict[str, Any] = {} read_fields: dict[str, Any] = {} for col in table.c: name = col.key - annotation = _col_type_to_annotation(col) - if annotation is None: - annotation = Any + annotation = _reflected_annotation(col) use_optional = col.nullable ann = Optional[annotation] if use_optional else annotation # type: ignore[valid-type] From 67b8d78c68d1a3e1a80eb79eb614103f41dce0f1 Mon Sep 17 00:00:00 2001 From: iklo Date: Fri, 18 Sep 2026 13:01:31 -0300 Subject: [PATCH 7/7] fix: keep reflected float and uuid columns validating as before The previous commit replaced the 70-line type chain with column.type.python_type and said the two agreed. They did not. I had compared python_type with what I expected the chain to return, not with what it returned. In SQLAlchemy Float, Double and REAL are subclasses of Numeric, and the old chain tested Numeric first, so every reflected float column has always been annotated as Decimal. python_type says float. Clients see the difference: a Decimal is serialized as a JSON string ("0.5"), a float as a JSON number (0.5). The old chain also returned UUID for a Uuid column with as_uuid=False, where python_type says str. Running the old function from dev next to the new one over 43 column types (generic, PostgreSQL, MySQL, SQLite) showed 8 differences, all in those two families. _reflected_annotation now answers Decimal for any Numeric and UUID for any Uuid before asking python_type, and the same comparison shows 0 differences. Whether a reflected float should be a JSON number is a fair question, but it is a change to the wire format and belongs in the changelog with a decision behind it, not in a refactor. tests/test_reflected_annotations.py pins 25 column types. 8 of them fail without this fix. Found by tools/differential, which compares every response with the published 0.1.29. Tests: 370 passed, 1 xfailed, 2 xpassed. --- lightapi/schema.py | 9 +++ tests/test_reflected_annotations.py | 87 +++++++++++++++++++++++++++++ 2 files changed, 96 insertions(+) create mode 100644 tests/test_reflected_annotations.py diff --git a/lightapi/schema.py b/lightapi/schema.py index 6633961..8865bed 100644 --- a/lightapi/schema.py +++ b/lightapi/schema.py @@ -8,6 +8,7 @@ from pydantic import ConfigDict, create_model from pydantic.fields import FieldInfo +from sqlalchemy import Numeric, Uuid from lightapi.constants import AUTO_FIELDS from lightapi.exceptions import ConfigurationError, SerializationError @@ -102,6 +103,14 @@ def _apply_fields(d: dict[str, Any], fields: list[str] | None) -> dict[str, Any] def _reflected_annotation(column: Any) -> Any: """Annotation for a reflected column, from SQLAlchemy's own type mapping.""" + # Two answers of python_type differ from what released versions validated, + # and clients see the difference (a Decimal is a JSON string, a float a JSON + # number): Float, Double and REAL are Numeric subclasses and have always been + # Decimal here, and a Uuid column is a UUID whatever its as_uuid flag. + if isinstance(column.type, Numeric): + return Decimal + if isinstance(column.type, Uuid): + return UUID try: python_type = column.type.python_type except NotImplementedError: diff --git a/tests/test_reflected_annotations.py b/tests/test_reflected_annotations.py new file mode 100644 index 0000000..73e04c3 --- /dev/null +++ b/tests/test_reflected_annotations.py @@ -0,0 +1,87 @@ +"""Reflected columns must keep validating as the types released versions used. + +Clients see this: a column annotated as Decimal is a JSON string ("0.5"), one +annotated as float is a JSON number (0.5). +""" + +import datetime +from decimal import Decimal +from typing import Any +from uuid import UUID + +import pytest +from sqlalchemy import ( + JSON, + BigInteger, + Boolean, + Column, + Date, + DateTime, + Double, + Float, + Integer, + LargeBinary, + MetaData, + Numeric, + SmallInteger, + String, + Table, + Text, + Time, + Uuid, +) +from sqlalchemy.dialects import postgresql, sqlite + +from lightapi.schema import SchemaFactory + +EXPECTED = [ + (Integer(), int), + (BigInteger(), int), + (SmallInteger(), int), + (String(20), str), + (Text(), str), + (Boolean(), bool), + (DateTime(), datetime.datetime), + (Date(), datetime.date), + (Time(), datetime.time), + (Numeric(10, 2), Decimal), + # Float is a Numeric in SQLAlchemy, and the released mapping tested Numeric + # first, so every float column has always validated as Decimal. + (Numeric(10, 2, asdecimal=False), Decimal), + (Float(), Decimal), + (Double(), Decimal), + (postgresql.DOUBLE_PRECISION(), Decimal), + (postgresql.REAL(), Decimal), + (sqlite.REAL(), Decimal), + (Uuid(), UUID), + (Uuid(as_uuid=False), UUID), + (postgresql.UUID(), UUID), + (postgresql.UUID(as_uuid=False), UUID), + (LargeBinary(), Any), + (JSON(), Any), + (postgresql.JSONB(), Any), + (postgresql.ARRAY(Integer), Any), + (postgresql.INET(), Any), +] + + +class FakeEndpoint: + __name__ = "FakeEndpoint" + + +@pytest.mark.parametrize( + ("column_type", "annotation"), + EXPECTED, + ids=[repr(column_type) for column_type, _ in EXPECTED], +) +def test_reflected_column_annotation(column_type, annotation): + table = Table( + "reflected_annotation_probe", + MetaData(), + Column("id", Integer, primary_key=True), + Column("value", column_type, nullable=False), + ) + + schema_create, _ = SchemaFactory.build_from_reflected_table(FakeEndpoint, table) + + assert schema_create.model_fields["value"].annotation is annotation