diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 8a10562..6ab74d0 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -81,7 +81,14 @@ jobs: - name: Run tests run: | - pytest tests/ -v --cov=fastapi_viewsets --cov-report=term-missing --cov-fail-under=70 + pytest tests/ -v --cov=fastapi_viewsets --cov-report=xml --cov-report=term-missing --cov-fail-under=70 + + - name: Upload coverage to codecov + uses: codecov/codecov-action@v4 + with: + file: ./coverage.xml + flags: extras + fail_ci_if_error: false package: name: Build and test installed wheel diff --git a/README.md b/README.md index e43fbb4..e8735f3 100644 --- a/README.md +++ b/README.md @@ -40,18 +40,21 @@ Django REST Framework-style ViewSets for FastAPI — auto-generate CRUD endpoint pip install fastapi-viewsets ``` -SQLAlchemy 2.0 or newer is installed automatically. For a local SQLite app, -start with the [sync quickstart](#quickstart-sqlalchemy-sync); no separate database driver is needed. - -Optional extras (see `pyproject.toml`): +The base install is ORM-agnostic: it pulls in only FastAPI, Pydantic and +python-dotenv. Pick your ORM via an extra (see `pyproject.toml`): ```bash -pip install "fastapi-viewsets[sqlalchemy]" +pip install "fastapi-viewsets[sqlalchemy]" # SQLAlchemy 2.x, incl. asyncio support (greenlet) pip install "fastapi-viewsets[tortoise]" pip install "fastapi-viewsets[peewee]" pip install "fastapi-viewsets[test]" # pytest, httpx, coverage, etc. ``` +For a local SQLite app, start with the +[sync quickstart](#quickstart-sqlalchemy-sync); no separate database driver is needed. +To execute the quickstart's `python main.py` (or serve any app) you also +need an ASGI server, e.g. `pip install uvicorn`. + For async SQLAlchemy you still need a driver such as `aiosqlite`, `asyncpg`, or `aiomysql` alongside your database URL. ## Database connection examples @@ -443,10 +446,23 @@ Notes: `model_config = ConfigDict(from_attributes=True)` instead of the v1 `class Config: orm_mode = True`. - `PATCH` uses `model_dump(exclude_unset=True)` internally, so unset - fields are no longer overwritten with defaults. + fields are no longer overwritten with defaults. Since v1.5.2 the PATCH + body is validated against an auto-generated all-optional variant of + your `response_model`, so partial bodies pass validation even when the + schema has required fields, and an explicit JSON `null` clears a + nullable column. +- `PUT` replaces exactly the fields present in the payload; model + columns that are not part of the `response_model` are left untouched. +- `POST` never sends an explicit `NULL` primary key to the database — + `id: Optional[int] = None` in the schema is safe on every adapter. +- Integrity violations (e.g. duplicate unique values) return + `409 Conflict` with a sanitized message; raw SQL and driver internals + are never exposed in the response body. - If the async driver (`aiosqlite` / `asyncpg` / `aiomysql`) is not installed, sync usage still works — only `get_async_session()` raises - a helpful `RuntimeError`. + a helpful `RuntimeError`. The `sqlalchemy` extra installs + `SQLAlchemy[asyncio]`, which already includes `greenlet` for async + sessions. ## Overriding `list` and `create_element` (custom LIST and POST) @@ -454,8 +470,11 @@ Every CRUD handler is a regular method, so subclassing the viewset is the canonical way to add filtering, ordering, validation, conflict handling, and so on. The example below subclasses `AsyncBaseViewset` and overrides both `list` (case-insensitive search + simple ordering) -and `create_element` (input normalization + map `IntegrityError` to -409). +and `create_element` (input normalization + custom conflict message). + +> Since v1.5.2 the adapters themselves map `IntegrityError` to +> `409 Conflict`; override `create_element` only when you need a custom +> error payload or extra normalization. ```python from typing import List, Optional @@ -621,7 +640,7 @@ app.include_router(router) ## Pagination, filtering, ordering -**Pagination** — `BaseViewset.list` maps `limit` and `offset` to query parameters on the LIST route. +**Pagination** — `BaseViewset.list` maps `limit` and `offset` to query parameters on the LIST route. Defaults are `limit=10`, `offset=0`; negative values are rejected with `422`, and `limit` is capped at `10000`. ```python from fastapi_viewsets import BaseViewset @@ -682,6 +701,34 @@ class ItemsWithStats(BaseViewset): ## What is new +### v1.5.2 + +Bugfix release for the CRUD write paths (see +[RELEASE_1.5.2.md](https://github.com/svalench/fastapi_viewsets/blob/master/RELEASE_1.5.2.md)): + +- **Multi-viewset apps fixed**: `register()` no longer leaks one viewset's + body schema into other viewsets — every `POST`/`PUT`/`PATCH` endpoint + now validates against its own `response_model`. +- **PATCH is truly partial**: the PATCH body is validated against an + auto-generated all-optional variant of the response schema, and an + explicit JSON `null` clears a nullable column. +- **PUT no longer nulls columns** that are absent from the Pydantic + schema. +- **Integrity violations return `409 Conflict`** with a sanitized message + instead of `400` with raw SQL/driver internals. +- **Create never passes an explicit `NULL` primary key** — fixes Tortoise + `POST` and PostgreSQL inserts with `id: Optional[int] = None` schemas. +- **Pagination validated**: negative `limit`/`offset` rejected with `422`. +- **Filters advertised in OpenAPI**: whitelisted `ListConfig.filters` + fields and their `__op` variants show up in Swagger UI. +- **Leaner dependencies**: the base install no longer requires SQLAlchemy + or uvicorn; the `sqlalchemy` extra installs `SQLAlchemy[asyncio]` + (includes `greenlet`). + +Earlier releases: v1.5.0 (server-side `search`, declarative ordering and +filters — [RELEASE_1.5.0.md](https://github.com/svalench/fastapi_viewsets/blob/master/RELEASE_1.5.0.md)), +v1.4.0 ([RELEASE_1.4.0.md](https://github.com/svalench/fastapi_viewsets/blob/master/RELEASE_1.4.0.md)). + ### v1.3.0 - **Declarative eager loading** via `RelatedConfig` inside Pydantic schemas. diff --git a/RELEASE_1.5.2.md b/RELEASE_1.5.2.md new file mode 100644 index 0000000..e374ed2 --- /dev/null +++ b/RELEASE_1.5.2.md @@ -0,0 +1,59 @@ +# Release v1.5.2 + +Bugfix release for the CRUD write paths, route registration, and +packaging. All fixes are covered by regression tests +(`tests/test_write_path_regressions.py`). + +## 🐞 Fixes + +### Route registration + +- **Body-schema leak between viewsets (critical).** `register()` used to + mutate `__annotations__` of the shared class-level CRUD functions, so + in any process with two or more viewsets every `POST`/`PUT`/`PATCH` + endpoint validated against the *last registered* viewset's schema. + Handlers are now cloned per instance before the body annotation is + patched. +- **PATCH is truly partial.** The PATCH body is validated against an + auto-generated all-optional variant of the `response_model` + (`Patch`). Previously a partial PATCH body failed validation + with `422` whenever the schema had required fields. +- **PATCH honors explicit `null`.** Explicitly sent JSON `null` now + clears a nullable column instead of being silently dropped. +- **Pagination validated.** Negative `limit`/`offset` are rejected with + `422`; `limit` is capped at 10000. Defaults (`limit=10`, `offset=0`) + are documented. +- **Filters in OpenAPI.** Whitelisted `ListConfig.filters` fields and + their `__op` variants (`__gte`, `__in`, …) are advertised in the + generated OpenAPI schema and Swagger UI. + +### ORM adapters (SQLAlchemy sync/async, Tortoise, Peewee) + +- **PUT no longer nulls columns absent from the schema.** Only fields + explicitly present in the payload are written; the primary key is + never overwritten. Previously `PUT` set every non-schema column to + `NULL` (data loss). +- **Create strips an explicit `NULL` primary key.** Schemas with + `id: Optional[int] = None` no longer break `POST` on Tortoise + (`id is non nullable field, but null was passed`) and no longer send + `NULL` for the PK on backends like PostgreSQL. +- **Integrity violations return `409 Conflict`** with a sanitized + message (`"Integrity error: a database constraint was violated"`). + Raw SQL, parameters and driver internals are no longer exposed in + response bodies; generic adapter errors likewise use static messages. + +### Packaging + +- Base install is now truly ORM-agnostic: `SQLAlchemy` and `uvicorn` + were removed from core dependencies. The package imports cleanly + without SQLAlchemy installed. +- `fastapi-viewsets[sqlalchemy]` now installs `SQLAlchemy[asyncio]`, + which includes `greenlet` — async sessions work out of the box. + +## ⚠️ Behavior changes to be aware of + +- Integrity violations: `400` → `409` (message no longer contains raw SQL). +- PATCH with explicit `null` writes `NULL` (previously ignored). +- PUT no longer clears fields/columns missing from the payload. +- `pip install fastapi-viewsets` no longer pulls SQLAlchemy/uvicorn — + use `fastapi-viewsets[sqlalchemy]` and install `uvicorn` to run apps. diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 4f6cff2..09a698f 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -1,5 +1,15 @@ # Release Notes +## Version 1.5.2 + +Bugfix release for the CRUD write paths: multi-viewset body-schema leak, +true partial PATCH (incl. explicit `null`), PUT no longer nulls columns +outside the schema, `409 Conflict` without SQL leaks on integrity +violations, validated pagination, filters in OpenAPI, and leaner core +dependencies. + +Details: [RELEASE_1.5.2.md](RELEASE_1.5.2.md). + ## Version 1.5.1 Maintenance release fixing dependency metadata, source-distribution contents, diff --git a/docs/getting-started.md b/docs/getting-started.md index 0cb3d0c..8a5c73e 100644 --- a/docs/getting-started.md +++ b/docs/getting-started.md @@ -7,8 +7,8 @@ | Python | >= 3.9 | | FastAPI | >= 0.110 | | Pydantic | >= 2.5, < 3 | -| SQLAlchemy | >= 2.0.0 | | python-dotenv | >= 0.19 | +| SQLAlchemy | >= 2.0.0 (only for the `sqlalchemy` extra / default `ORM_TYPE`) | ## Install from PyPI @@ -18,8 +18,9 @@ pip install fastapi-viewsets ## Optional extras -SQLAlchemy is a core dependency (installed automatically). The extras below -add ORM-specific packages when you need Tortoise or Peewee: +The base install is ORM-agnostic (FastAPI + Pydantic + python-dotenv only). +Add the extra for your ORM — and an ASGI server such as `uvicorn` to run +your app: === "SQLAlchemy" diff --git a/docs/pagination-filtering.md b/docs/pagination-filtering.md index 67f4a94..d1199ca 100644 --- a/docs/pagination-filtering.md +++ b/docs/pagination-filtering.md @@ -10,8 +10,8 @@ GET /items?limit=10&offset=20 | Parameter | Type | Default | Description | | --- | --- | --- | --- | -| `limit` | `Optional[int]` | `10` | Maximum number of items to return | -| `offset` | `Optional[int]` | `0` | Number of items to skip | +| `limit` | `int` | `10` | Maximum number of items to return (must be `0..10000`, negative values are rejected with `422`) | +| `offset` | `int` | `0` | Number of items to skip (negative values are rejected with `422`) | No additional configuration needed — pagination is built into the default `list()` handler. diff --git a/fastapi_viewsets/__init__.py b/fastapi_viewsets/__init__.py index db643f7..ee46bd1 100644 --- a/fastapi_viewsets/__init__.py +++ b/fastapi_viewsets/__init__.py @@ -9,9 +9,8 @@ from typing import Any, Callable, Dict, List, Optional, Type, TypeVar, Union -from fastapi import APIRouter, Body, Depends, Request +from fastapi import APIRouter, Body, Depends, Query, Request from pydantic import BaseModel -from sqlalchemy.orm import Session from fastapi_viewsets._compat import model_to_dict from fastapi_viewsets._register import _RegisterMixin @@ -82,7 +81,7 @@ def __init__( self.endpoint: Optional[str] = endpoint self.response_model: Optional[Type[ResponseModelType]] = response_model self.model: Optional[Type[ModelType]] = model - self.db_session: Optional[Callable[[], Session]] = db_session + self.db_session: Optional[Callable[[], Any]] = db_session self.orm_adapter: Optional[BaseORMAdapter] = orm_adapter or get_orm_adapter() # ------------------------------------------------------------------ @@ -91,8 +90,8 @@ def __init__( def list( self, - limit: Optional[int] = 10, - offset: Optional[int] = 0, + limit: int = Query(10, ge=0, le=10000), + offset: int = Query(0, ge=0), search: Optional[str] = None, ordering: Optional[str] = None, request: Request = None, @@ -117,6 +116,13 @@ def list( parse_ordering_param, ) + # FastAPI resolves ``Query`` defaults for HTTP calls; direct + # programmatic calls receive the unresolved ``Query`` object. + if not isinstance(limit, int): + limit = 10 + if not isinstance(offset, int): + offset = 0 + config = get_list_config(self.response_model) return get_list_queryset( self.model, diff --git a/fastapi_viewsets/_register.py b/fastapi_viewsets/_register.py index 6b9836d..56a80fd 100644 --- a/fastapi_viewsets/_register.py +++ b/fastapi_viewsets/_register.py @@ -8,11 +8,13 @@ from __future__ import annotations import functools +import types from collections.abc import Iterable -from typing import List, Optional +from typing import Any, Dict, List, Optional, Type from fastapi import Depends from fastapi.security import OAuth2PasswordBearer +from pydantic import BaseModel, create_model from fastapi_viewsets.constants import MAP_METHODS @@ -21,6 +23,71 @@ # the ``/{id}`` route. _METHOD_ORDER = ("LIST", "POST", "GET", "PUT", "PATCH", "DELETE") +# Cache of generated partial (all-optional) PATCH body models, keyed by +# the source schema, so repeated ``register()`` calls reuse one model. +_PARTIAL_MODEL_CACHE: Dict[type, type] = {} + + +def _build_partial_model(model: Type[BaseModel]) -> Type[BaseModel]: + """Build an all-optional variant of a Pydantic schema for PATCH bodies. + + PATCH semantics require that clients may send any subset of fields, + so required fields in the response schema must become optional in + the request body. Unset fields are excluded later via + ``model_dump(exclude_unset=True)``; explicitly sent ``null`` values + are preserved so nullable columns can be cleared. + """ + cached = _PARTIAL_MODEL_CACHE.get(model) + if cached is not None: + return cached + + if not hasattr(model, "model_fields"): # pragma: no cover - pydantic v1 fallback + return model + + fields: Dict[str, Any] = {} + for name, field_info in model.model_fields.items(): + fields[name] = (Optional[field_info.annotation], None) + + partial = create_model(f"{model.__name__}Patch", **fields) + _PARTIAL_MODEL_CACHE[model] = partial + return partial + + +def _clone_bound_method(bound: Any, annotation_overrides: Dict[str, Any]) -> Any: + """Return a per-instance copy of a bound method with patched annotations. + + ``register()`` is called once per viewset instance, but CRUD handlers + are functions defined once on the class. Mutating + ``handler.__annotations__`` directly would leak one viewset's body + schema into every other viewset in the process, so the function is + cloned first and the clone's annotations are patched instead. + """ + func = getattr(bound, "__func__", None) + if func is None: + # Not a plain bound method (already a partial/closure): patch a + # fresh annotation dict on the object itself, best-effort. + try: + merged = {**getattr(bound, "__annotations__", {}), **annotation_overrides} + bound.__annotations__ = merged + except (AttributeError, TypeError): + pass + return bound + + new_func = types.FunctionType( + func.__code__, + func.__globals__, + func.__name__, + func.__defaults__, + func.__closure__, + ) + new_func.__dict__.update(func.__dict__) + new_func.__kwdefaults__ = func.__kwdefaults__ + new_func.__annotations__ = {**func.__annotations__, **annotation_overrides} + new_func.__doc__ = func.__doc__ + new_func.__module__ = func.__module__ + new_func.__qualname__ = func.__qualname__ + return types.MethodType(new_func, bound.__self__) + class _RegisterMixin: """Mixin providing :meth:`register` for CRUD endpoint wiring. @@ -77,15 +144,18 @@ def register( handler = getattr(self, spec["method"]) # Hint FastAPI/Pydantic about the body schema if the handler - # carries an ``item`` parameter. Annotation patching is - # best-effort: built-in/bound methods may reject it. + # carries an ``item`` parameter. The handler is cloned first + # so patching never mutates the shared class-level function + # (which would leak this viewset's schema into other + # viewsets). PATCH gets an all-optional partial model so + # clients may send any subset of fields. if self.response_model is not None and "item" in getattr( handler, "__annotations__", {} ): - try: - handler.__annotations__["item"] = self.response_model - except (AttributeError, TypeError): - pass + body_model = self.response_model + if spec["http_method"] == "PATCH": + body_model = _build_partial_model(self.response_model) + handler = _clone_bound_method(handler, {"item": body_model}) original_doc = handler.__doc__ @@ -104,6 +174,7 @@ def register( endpoint = (self.endpoint or "") + spec.get("path", "") response_model = self._build_response_model(spec) route_name = self._build_route_name(method) + openapi_extra = self._build_openapi_extra(spec) self.add_api_route( endpoint, @@ -112,10 +183,51 @@ def register( tags=self.tags, methods=[spec["http_method"]], name=route_name, + openapi_extra=openapi_extra, ) # --- helpers --------------------------------------------------- + def _build_openapi_extra(self, spec) -> Optional[Dict[str, Any]]: + """Document dynamic LIST filter parameters in OpenAPI. + + Filters are parsed from ``request.query_params`` at runtime, so + FastAPI cannot see them in the handler signature. Advertise the + whitelisted ``ListConfig.filters`` fields (and their ``__op`` + variants) explicitly via ``openapi_extra``. + """ + if not spec.get("is_list") or self.response_model is None: + return None + + from fastapi_viewsets.filtering import FILTER_OPS, get_list_config + + config = get_list_config(self.response_model) + if not config.filters: + return None + + parameters: List[Dict[str, Any]] = [] + for field_name in config.filters: + parameters.append( + { + "name": field_name, + "in": "query", + "required": False, + "schema": {"type": "string"}, + "description": f"Exact-match filter on `{field_name}`.", + } + ) + for op in FILTER_OPS: + parameters.append( + { + "name": f"{field_name}__{op}", + "in": "query", + "required": False, + "schema": {"type": "string"}, + "description": f"Comparison filter `{op}` on `{field_name}`.", + } + ) + return {"parameters": parameters} + def _build_response_model(self, spec): """Compute the FastAPI ``response_model`` for a given method spec.""" if spec.get("http_method") == "DELETE": diff --git a/fastapi_viewsets/async_base.py b/fastapi_viewsets/async_base.py index 1e2bfa7..8c4fc98 100644 --- a/fastapi_viewsets/async_base.py +++ b/fastapi_viewsets/async_base.py @@ -4,9 +4,8 @@ from typing import Any, Callable, Dict, List, Optional, Type, TypeVar, Union -from fastapi import APIRouter, Body, Depends, Request +from fastapi import APIRouter, Body, Depends, Query, Request from pydantic import BaseModel -from sqlalchemy.ext.asyncio import AsyncSession from fastapi_viewsets._compat import model_to_dict from fastapi_viewsets._register import _RegisterMixin @@ -51,7 +50,7 @@ def __init__( allowed_methods: Optional[List[str]] = None, endpoint: Optional[str] = None, model: Optional[Type[ModelType]] = None, - db_session: Optional[Callable[[], AsyncSession]] = None, + db_session: Optional[Callable[[], Any]] = None, response_model: Optional[Type[ResponseModelType]] = None, orm_adapter: Optional[BaseORMAdapter] = None, **kwargs, @@ -73,7 +72,7 @@ def __init__( self.endpoint: Optional[str] = endpoint self.response_model: Optional[Type[ResponseModelType]] = response_model self.model: Optional[Type[ModelType]] = model - self.db_session: Optional[Callable[[], AsyncSession]] = db_session + self.db_session: Optional[Callable[[], Any]] = db_session self.orm_adapter: Optional[BaseORMAdapter] = orm_adapter or get_orm_adapter() # ------------------------------------------------------------------ @@ -82,8 +81,8 @@ def __init__( async def list( self, - limit: Optional[int] = 10, - offset: Optional[int] = 0, + limit: int = Query(10, ge=0, le=10000), + offset: int = Query(0, ge=0), search: Optional[str] = None, ordering: Optional[str] = None, request: Request = None, @@ -108,6 +107,13 @@ async def list( parse_ordering_param, ) + # FastAPI resolves ``Query`` defaults for HTTP calls; direct + # programmatic calls receive the unresolved ``Query`` object. + if not isinstance(limit, int): + limit = 10 + if not isinstance(offset, int): + offset = 0 + config = get_list_config(self.response_model) return await get_list_queryset( self.model, diff --git a/fastapi_viewsets/orm/peewee_adapter.py b/fastapi_viewsets/orm/peewee_adapter.py index 7975b78..7898198 100644 --- a/fastapi_viewsets/orm/peewee_adapter.py +++ b/fastapi_viewsets/orm/peewee_adapter.py @@ -216,6 +216,10 @@ def create_element( ) -> ModelType: """Create new element in database (synchronous).""" try: + # Never pass a NULL primary key explicitly. + pk_name = getattr(model._meta.primary_key, "name", "id") + data = {k: v for k, v in dict(data).items() if not (v is None and k == pk_name)} + # Validate required fields required_fields = set() for field_name, field in model._meta.fields.items(): @@ -234,13 +238,13 @@ def create_element( return instance except PeeweeIntegrityError as e: raise HTTPException( - status_code=status.HTTP_400_BAD_REQUEST, - detail=f"Integrity error: {str(e)}" + status_code=status.HTTP_409_CONFLICT, + detail="Integrity error: a database constraint was violated" ) except Exception as e: raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, - detail=f"Error creating element: {str(e)}" + detail="Error creating element" ) async def create_element_async( @@ -269,13 +273,10 @@ def update_element( detail=f"Element with id {id} not found" ) - if partial: - # PATCH: update only provided fields, skip None values - data = {key: value for key, value in dict(data).items() if value is not None} - else: - # PUT: replace all fields with provided values, use None for missing fields - all_fields = set(model._meta.fields.keys()) - {'id'} - data = {col: data.get(col, None) for col in all_fields} + # Only write fields explicitly present in the payload; never + # touch the primary key. Explicit ``null`` values are honored. + pk_name = getattr(model._meta.primary_key, "name", "id") + data = {key: value for key, value in dict(data).items() if key != pk_name} if not data: return instance @@ -316,7 +317,7 @@ def delete_element( except Exception as e: raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, - detail=f"Error deleting element: {str(e)}" + detail="Error deleting element" ) async def delete_element_async( diff --git a/fastapi_viewsets/orm/sqlalchemy_adapter.py b/fastapi_viewsets/orm/sqlalchemy_adapter.py index 8c3fd30..f06d073 100644 --- a/fastapi_viewsets/orm/sqlalchemy_adapter.py +++ b/fastapi_viewsets/orm/sqlalchemy_adapter.py @@ -316,6 +316,11 @@ def create_element( """Create new element in database (synchronous).""" db = db_session() try: + # Never pass a NULL primary key explicitly — some backends + # (e.g. PostgreSQL) reject it instead of autoincrementing. + pk_cols = {col.name for col in model.__table__.primary_key.columns} + data = {k: v for k, v in dict(data).items() if not (v is None and k in pk_cols)} + # Validate required fields required_fields = {col.name for col in model.__table__.columns if not col.nullable and col.name != 'id' and not col.primary_key} @@ -338,20 +343,20 @@ def create_element( except IntegrityError as e: db.rollback() raise HTTPException( - status_code=status.HTTP_400_BAD_REQUEST, - detail=f"Integrity error: {str(e)}" + status_code=status.HTTP_409_CONFLICT, + detail="Integrity error: a database constraint was violated" ) except SQLAlchemyError as e: db.rollback() raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, - detail=f"Database error: {str(e)}" + detail="Database error" ) except Exception as e: db.rollback() raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, - detail=f"Error creating element: {str(e)}" + detail="Error creating element" ) finally: db.close() @@ -365,6 +370,11 @@ async def create_element_async( """Create new element in database (asynchronous).""" db = db_session() try: + # Never pass a NULL primary key explicitly — some backends + # (e.g. PostgreSQL) reject it instead of autoincrementing. + pk_cols = {col.name for col in model.__table__.primary_key.columns} + data = {k: v for k, v in dict(data).items() if not (v is None and k in pk_cols)} + # Validate required fields required_fields = {col.name for col in model.__table__.columns if not col.nullable and col.name != 'id' and not col.primary_key} @@ -384,20 +394,20 @@ async def create_element_async( except IntegrityError as e: await db.rollback() raise HTTPException( - status_code=status.HTTP_400_BAD_REQUEST, - detail=f"Integrity error: {str(e)}" + status_code=status.HTTP_409_CONFLICT, + detail="Integrity error: a database constraint was violated" ) except SQLAlchemyError as e: await db.rollback() raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, - detail=f"Database error: {str(e)}" + detail="Database error" ) except Exception as e: await db.rollback() raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, - detail=f"Error creating element: {str(e)}" + detail="Error creating element" ) finally: await db.close() @@ -420,11 +430,14 @@ def update_element( detail=f"Element with id {id} not found" ) - if partial: - data = {key: value for key, value in dict(data).items() if value is not None} - else: - all_columns = {col.name for col in model.__table__.columns if not col.primary_key} - data = {col: data.get(col, None) for col in all_columns} + # Only write fields explicitly present in the payload and + # never touch the primary key. PATCH payloads arrive with + # ``exclude_unset`` applied upstream, so explicit ``null`` + # values are honored (they clear the column); PUT replaces + # exactly the fields the schema carries and leaves columns + # absent from the schema untouched. + pk_cols = {col.name for col in model.__table__.primary_key.columns} + data = {key: value for key, value in dict(data).items() if key not in pk_cols} if not data: db.refresh(result) @@ -437,14 +450,14 @@ def update_element( except IntegrityError as e: db.rollback() raise HTTPException( - status_code=status.HTTP_400_BAD_REQUEST, - detail=f"Integrity error: {str(e)}" + status_code=status.HTTP_409_CONFLICT, + detail="Integrity error: a database constraint was violated" ) except SQLAlchemyError as e: db.rollback() raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, - detail=f"Database error: {str(e)}" + detail="Database error" ) except HTTPException: db.rollback() @@ -453,7 +466,7 @@ def update_element( db.rollback() raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, - detail=f"Error updating element: {str(e)}" + detail="Error updating element" ) finally: db.close() @@ -476,11 +489,14 @@ async def update_element_async( detail=f"Element with id {id} not found" ) - if partial: - data = {key: value for key, value in dict(data).items() if value is not None} - else: - all_columns = {col.name for col in model.__table__.columns if not col.primary_key} - data = {col: data.get(col, None) for col in all_columns} + # Only write fields explicitly present in the payload and + # never touch the primary key. PATCH payloads arrive with + # ``exclude_unset`` applied upstream, so explicit ``null`` + # values are honored (they clear the column); PUT replaces + # exactly the fields the schema carries and leaves columns + # absent from the schema untouched. + pk_cols = {col.name for col in model.__table__.primary_key.columns} + data = {key: value for key, value in dict(data).items() if key not in pk_cols} if not data: await db.refresh(result) @@ -495,14 +511,14 @@ async def update_element_async( except IntegrityError as e: await db.rollback() raise HTTPException( - status_code=status.HTTP_400_BAD_REQUEST, - detail=f"Integrity error: {str(e)}" + status_code=status.HTTP_409_CONFLICT, + detail="Integrity error: a database constraint was violated" ) except SQLAlchemyError as e: await db.rollback() raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, - detail=f"Database error: {str(e)}" + detail="Database error" ) except HTTPException: await db.rollback() @@ -511,7 +527,7 @@ async def update_element_async( await db.rollback() raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, - detail=f"Error updating element: {str(e)}" + detail="Error updating element" ) finally: await db.close() @@ -541,13 +557,13 @@ def delete_element( db.rollback() raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, - detail=f"Database error: {str(e)}" + detail="Database error" ) except Exception as e: db.rollback() raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, - detail=f"Error deleting element: {str(e)}" + detail="Error deleting element" ) finally: db.close() @@ -577,13 +593,13 @@ async def delete_element_async( await db.rollback() raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, - detail=f"Database error: {str(e)}" + detail="Database error" ) except Exception as e: await db.rollback() raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, - detail=f"Error deleting element: {str(e)}" + detail="Error deleting element" ) finally: await db.close() diff --git a/fastapi_viewsets/orm/tortoise_adapter.py b/fastapi_viewsets/orm/tortoise_adapter.py index d0400de..20f4cb4 100644 --- a/fastapi_viewsets/orm/tortoise_adapter.py +++ b/fastapi_viewsets/orm/tortoise_adapter.py @@ -218,6 +218,11 @@ async def create_element_async( await self._ensure_initialized() try: + # Never pass a NULL primary key explicitly — Tortoise + # rejects ``id=None`` instead of autoincrementing. + pk_attr = getattr(model._meta, "pk_attr", "id") + data = {k: v for k, v in dict(data).items() if not (v is None and k == pk_attr)} + # Validate required fields required_fields = set() for field_name, field in model._meta.fields_map.items(): @@ -236,13 +241,13 @@ async def create_element_async( return instance except TortoiseIntegrityError as e: raise HTTPException( - status_code=status.HTTP_400_BAD_REQUEST, - detail=f"Integrity error: {str(e)}" + status_code=status.HTTP_409_CONFLICT, + detail="Integrity error: a database constraint was violated" ) except Exception as e: raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, - detail=f"Error creating element: {str(e)}" + detail="Error creating element" ) def create_element( @@ -273,13 +278,10 @@ async def update_element_async( detail=f"Element with id {id} not found" ) - if partial: - # PATCH: update only provided fields, skip None values - data = {key: value for key, value in dict(data).items() if value is not None} - else: - # PUT: replace all fields with provided values, use None for missing fields - all_fields = set(model._meta.fields_map.keys()) - {'id'} - data = {col: data.get(col, None) for col in all_fields} + # Only write fields explicitly present in the payload; never + # touch the primary key. Explicit ``null`` values are honored. + pk_attr = getattr(model._meta, "pk_attr", "id") + data = {key: value for key, value in dict(data).items() if key != pk_attr} if not data: return instance @@ -322,7 +324,7 @@ async def delete_element_async( except Exception as e: raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, - detail=f"Error deleting element: {str(e)}" + detail="Error deleting element" ) def delete_element( diff --git a/pyproject.toml b/pyproject.toml index 2862c51..5e05e40 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "fastapi_viewsets" -version = "1.5.1" +version = "1.5.2" description = "DRF-style viewsets for FastAPI with SQLAlchemy/Tortoise/Peewee adapters and Pydantic v2 support." readme = "README.md" license = { text = "MIT" } @@ -27,14 +27,12 @@ classifiers = [ ] dependencies = [ "fastapi>=0.110.0", - "uvicorn>=0.17.6", - "SQLAlchemy>=2.0.0", "pydantic>=2.5,<3", "python-dotenv>=0.19.0", ] [project.optional-dependencies] -sqlalchemy = ["SQLAlchemy>=2.0.0"] +sqlalchemy = ["SQLAlchemy[asyncio]>=2.0.0"] tortoise = ["tortoise-orm>=0.20.0,<1.0", "asyncpg>=0.28.0"] peewee = ["peewee>=3.17.0,<4"] test = [ @@ -44,6 +42,8 @@ test = [ "httpx>=0.24.0", "faker>=18.0.0", "aiosqlite>=0.19.0", + "uvicorn>=0.17.6", + "SQLAlchemy[asyncio]>=2.0.0", ] lint = ["ruff>=0.5", "black>=24", "mypy>=1.8"] docs = ["mkdocs>=1.6", "mkdocs-material>=9.5", "pymdown-extensions>=10.7"] diff --git a/tests/test_async_utils.py b/tests/test_async_utils.py index b45175c..ed432d9 100644 --- a/tests/test_async_utils.py +++ b/tests/test_async_utils.py @@ -186,7 +186,7 @@ async def test_create_element_integrity_error(self, test_model, async_db_session with pytest.raises(HTTPException) as exc_info: await create_element(test_model, session_factory, sample_user_data) - assert exc_info.value.status_code == status.HTTP_400_BAD_REQUEST + assert exc_info.value.status_code == status.HTTP_409_CONFLICT assert "integrity" in exc_info.value.detail.lower() @pytest.mark.asyncio @@ -248,23 +248,23 @@ async def test_update_element_patch_partial(self, test_model, async_db_session_f assert result.is_active == sample_user_data["is_active"] # unchanged @pytest.mark.asyncio - async def test_update_element_patch_skip_none(self, test_model, async_db_session_factory, sample_user_data): - """Test PATCH update skips None values (async).""" + async def test_update_element_patch_explicit_null(self, test_model, async_db_session_factory, sample_user_data): + """PATCH writes explicitly provided null values (async).""" session_factory = async_db_session_factory - + # Create element created = await create_element(test_model, session_factory, sample_user_data) - - # Update with PATCH including None + + # Update with PATCH including an explicit None update_data = { "username": "patcheduser", "age": None } result = await update_element(test_model, session_factory, created.id, update_data, partial=True) - + assert result.username == "patcheduser" - # age should remain unchanged (not set to None in PATCH) - assert result.age == sample_user_data.get("age") + # explicit null clears the column + assert result.age is None @pytest.mark.asyncio async def test_update_nonexistent_element(self, test_model, async_db_session_factory): @@ -292,7 +292,7 @@ async def test_update_element_integrity_error(self, test_model, async_db_session with pytest.raises(HTTPException) as exc_info: await update_element(test_model, session_factory, user2.id, update_data, partial=True) - assert exc_info.value.status_code == status.HTTP_400_BAD_REQUEST + assert exc_info.value.status_code == status.HTTP_409_CONFLICT assert "integrity" in exc_info.value.detail.lower() diff --git a/tests/test_edge_cases.py b/tests/test_edge_cases.py index 73806c4..58ced68 100644 --- a/tests/test_edge_cases.py +++ b/tests/test_edge_cases.py @@ -119,22 +119,20 @@ def test_update_element_with_empty_dict(self, test_model, db_session_factory, sa assert result.email == sample_user_data["email"] def test_update_element_put_with_missing_fields(self, test_model, db_session_factory, sample_user_data): - """Test PUT update with missing required fields (should raise error).""" + """PUT updates only the provided fields and never nulls the rest.""" # Create element created = create_element(test_model, db_session_factory, sample_user_data) - - # Update with PUT but missing required fields (email is required) + + # Update with PUT but missing fields — they must stay intact update_data = { "username": "updated" - # Missing email, is_active, age + # email, is_active, age not provided } - - # PUT with missing required fields should raise IntegrityError - with pytest.raises(HTTPException) as exc_info: - update_element(test_model, db_session_factory, created.id, update_data, partial=False) - - assert exc_info.value.status_code == status.HTTP_400_BAD_REQUEST - assert "integrity" in exc_info.value.detail.lower() or "NOT NULL" in exc_info.value.detail + + result = update_element(test_model, db_session_factory, created.id, update_data, partial=False) + + assert result.username == "updated" + assert result.email == sample_user_data["email"] # preserved, not nulled def test_delete_element_with_zero_id(self, test_model, db_session_factory): """Test deleting element with ID=0.""" @@ -266,7 +264,7 @@ def test_create_duplicate_unique_field(self, test_model, db_session_factory, sam with pytest.raises(HTTPException) as exc_info: create_element(test_model, db_session_factory, sample_user_data) - assert exc_info.value.status_code == status.HTTP_400_BAD_REQUEST + assert exc_info.value.status_code == status.HTTP_409_CONFLICT assert "integrity" in exc_info.value.detail.lower() def test_update_to_duplicate_unique_field(self, test_model, db_session_factory, sample_users_data): @@ -281,6 +279,6 @@ def test_update_to_duplicate_unique_field(self, test_model, db_session_factory, with pytest.raises(HTTPException) as exc_info: update_element(test_model, db_session_factory, user2.id, update_data, partial=True) - assert exc_info.value.status_code == status.HTTP_400_BAD_REQUEST + assert exc_info.value.status_code == status.HTTP_409_CONFLICT assert "integrity" in exc_info.value.detail.lower() diff --git a/tests/test_error_handling.py b/tests/test_error_handling.py index f6ce74c..5f3d7c6 100644 --- a/tests/test_error_handling.py +++ b/tests/test_error_handling.py @@ -42,7 +42,7 @@ class TestModel(Base): with pytest.raises(HTTPException) as exc_info: adapter.create_element(TestModel, adapter.get_session, {"name": "test"}) - assert exc_info.value.status_code == 400 + assert exc_info.value.status_code == 409 assert "Integrity error" in str(exc_info.value.detail) # Cleanup diff --git a/tests/test_exception_handling.py b/tests/test_exception_handling.py index 0a123f2..3630bdc 100644 --- a/tests/test_exception_handling.py +++ b/tests/test_exception_handling.py @@ -118,7 +118,7 @@ class TestModel(Base): partial=True ) - assert exc_info.value.status_code == 400 + assert exc_info.value.status_code == 409 assert "Integrity error" in str(exc_info.value.detail) # Cleanup @@ -304,7 +304,7 @@ async def test_tortoise_adapter_create_element_integrity_error(self): {"name": "test1"} # Duplicate ) - assert exc_info.value.status_code == 400 + assert exc_info.value.status_code == 409 assert "Integrity error" in str(exc_info.value.detail) # Cleanup @@ -416,7 +416,7 @@ class TestModel(Base): {"name": "test1"} # Duplicate ) - assert exc_info.value.status_code == 400 + assert exc_info.value.status_code == 409 assert "Integrity error" in str(exc_info.value.detail) # Cleanup diff --git a/tests/test_integration.py b/tests/test_integration.py index 4d7efb3..92222fa 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -308,7 +308,7 @@ def test_400_on_duplicate_unique_field(self, test_model, test_schema, db_session # Try to create duplicate response2 = client.post('/users', json=user_data) - assert response2.status_code == 400 + assert response2.status_code == 409 assert "integrity" in response2.json()['detail'].lower() diff --git a/tests/test_utils.py b/tests/test_utils.py index 66fe6de..893c71d 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -169,7 +169,7 @@ def test_create_element_integrity_error(self, test_model, db_session_factory, sa with pytest.raises(HTTPException) as exc_info: create_element(test_model, db_session_factory, sample_user_data) - assert exc_info.value.status_code == status.HTTP_400_BAD_REQUEST + assert exc_info.value.status_code == status.HTTP_409_CONFLICT assert "integrity" in exc_info.value.detail.lower() def test_create_element_with_none_values(self, test_model, db_session_factory): @@ -221,21 +221,21 @@ def test_update_element_patch_partial(self, test_model, db_session_factory, samp assert result.email == sample_user_data["email"] # unchanged assert result.is_active == sample_user_data["is_active"] # unchanged - def test_update_element_patch_skip_none(self, test_model, db_session_factory, sample_user_data): - """Test PATCH update skips None values.""" + def test_update_element_patch_explicit_null(self, test_model, db_session_factory, sample_user_data): + """PATCH writes explicitly provided null values (JSON null clears the field).""" # Create element created = create_element(test_model, db_session_factory, sample_user_data) - - # Update with PATCH including None + + # Update with PATCH including an explicit None update_data = { "username": "patcheduser", "age": None } result = update_element(test_model, db_session_factory, created.id, update_data, partial=True) - + assert result.username == "patcheduser" - # age should remain unchanged (not set to None in PATCH) - assert result.age == sample_user_data.get("age") + # explicit null clears the column + assert result.age is None def test_update_nonexistent_element(self, test_model, db_session_factory): """Test updating non-existent element raises 404.""" @@ -258,7 +258,7 @@ def test_update_element_integrity_error(self, test_model, db_session_factory, sa with pytest.raises(HTTPException) as exc_info: update_element(test_model, db_session_factory, user2.id, update_data, partial=True) - assert exc_info.value.status_code == status.HTTP_400_BAD_REQUEST + assert exc_info.value.status_code == status.HTTP_409_CONFLICT assert "integrity" in exc_info.value.detail.lower() diff --git a/tests/test_write_path_regressions.py b/tests/test_write_path_regressions.py new file mode 100644 index 0000000..35ee07d --- /dev/null +++ b/tests/test_write_path_regressions.py @@ -0,0 +1,447 @@ +"""Regression tests for CRUD write-path bugs fixed in 1.5.2. + +Covers: +* B1 — body-schema annotations must not leak between viewsets. +* B2 — PATCH accepts partial bodies even when the schema has required fields. +* B4 — PATCH honors explicit JSON ``null`` (clears nullable columns). +* B3 — PUT never nulls columns absent from the Pydantic schema. +* B5 — ``id: Optional[int] = None`` in the schema must not break create. +* B6 — integrity violations return 409 without raw SQL internals. +* M2 — negative ``limit``/``offset`` are rejected with 422. +* M5 — whitelisted filters appear in the OpenAPI schema. +""" + +from typing import Optional + +import pytest +from fastapi import FastAPI +from fastapi.testclient import TestClient +from pydantic import BaseModel, ConfigDict, Field + + +@pytest.fixture() +def sqlalchemy_stack(tmp_path): + from fastapi_viewsets.orm.sqlalchemy_adapter import SQLAlchemyAdapter + + adapter = SQLAlchemyAdapter(database_url=f"sqlite:///{tmp_path}/reg.db") + yield adapter + adapter.engine.dispose() + + +def _make_model(adapter, tablename, extra_column=False): + from sqlalchemy import Column, Integer, String + + Base = adapter.get_base() + + class RegModel(Base): + __tablename__ = tablename + id = Column(Integer, primary_key=True) + name = Column(String(255), nullable=False, unique=True) + price = Column(Integer, nullable=True) + if extra_column: + secret = Column(String(255), nullable=True) + + Base.metadata.create_all(adapter.engine) + return RegModel + + +class _NameSchema(BaseModel): + model_config = ConfigDict(from_attributes=True) + id: Optional[int] = None + name: str = Field(min_length=1, max_length=255) + price: Optional[int] = None + + +class _TitleSchema(BaseModel): + model_config = ConfigDict(from_attributes=True) + id: Optional[int] = None + title: str = Field(min_length=1, max_length=255) + + +def _build_two_viewset_app(adapter): + """Two viewsets with different schemas in one process (B1 repro).""" + from sqlalchemy import Column, Integer, String + + from fastapi_viewsets import BaseViewset + + Base = adapter.get_base() + + class Alpha(Base): + __tablename__ = "reg_alpha" + id = Column(Integer, primary_key=True) + name = Column(String(255), nullable=False) + price = Column(Integer, nullable=True) + + class Beta(Base): + __tablename__ = "reg_beta" + id = Column(Integer, primary_key=True) + title = Column(String(255), nullable=False) + price = Column(Integer, nullable=True) + + Base.metadata.create_all(adapter.engine) + + app = FastAPI() + va = BaseViewset( + endpoint="/alpha", + model=Alpha, + response_model=_NameSchema, + db_session=adapter.get_session, + orm_adapter=adapter, + ) + va.register(methods=["POST", "PATCH"]) + vb = BaseViewset( + endpoint="/beta", + model=Beta, + response_model=_TitleSchema, + db_session=adapter.get_session, + orm_adapter=adapter, + ) + vb.register(methods=["POST", "PATCH"]) + app.include_router(va) + app.include_router(vb) + return app + + +def test_b1_schemas_do_not_leak_between_viewsets(sqlalchemy_stack): + """Each viewset validates against its own response_model.""" + app = _build_two_viewset_app(sqlalchemy_stack) + client = TestClient(app) + + r = client.post("/alpha", json={"name": "hello", "price": 1}) + assert r.status_code == 200, r.text + assert r.json()["name"] == "hello" + + r = client.post("/beta", json={"title": "world"}) + assert r.status_code == 200, r.text + assert r.json()["title"] == "world" + + # wrong schema is rejected on both endpoints + assert client.post("/alpha", json={"title": "x"}).status_code == 422 + assert client.post("/beta", json={"name": "x"}).status_code == 422 + + +def test_b2_patch_accepts_partial_body_with_required_fields(sqlalchemy_stack): + """PATCH must not require fields the client did not intend to change.""" + from fastapi_viewsets import BaseViewset + + model = _make_model(sqlalchemy_stack, "reg_patch_partial") + app = FastAPI() + vs = BaseViewset( + endpoint="/items", + model=model, + response_model=_NameSchema, + db_session=sqlalchemy_stack.get_session, + orm_adapter=sqlalchemy_stack, + ) + vs.register(methods=["POST", "PATCH"]) + app.include_router(vs) + client = TestClient(app) + + created = client.post("/items", json={"name": "patchme", "price": 100}).json() + r = client.patch(f"/items/{created['id']}", json={"price": 777}) + assert r.status_code == 200, r.text + assert r.json()["price"] == 777 + assert r.json()["name"] == "patchme" + + +def test_b4_patch_explicit_null_clears_field(sqlalchemy_stack): + """Explicit JSON null clears a nullable column (not silently ignored).""" + from fastapi_viewsets import BaseViewset + + model = _make_model(sqlalchemy_stack, "reg_patch_null") + app = FastAPI() + vs = BaseViewset( + endpoint="/items", + model=model, + response_model=_NameSchema, + db_session=sqlalchemy_stack.get_session, + orm_adapter=sqlalchemy_stack, + ) + vs.register(methods=["POST", "PATCH"]) + app.include_router(vs) + client = TestClient(app) + + created = client.post("/items", json={"name": "nullme", "price": 50}).json() + r = client.patch(f"/items/{created['id']}", json={"price": None}) + assert r.status_code == 200, r.text + assert r.json()["price"] is None + + +def test_b3_put_preserves_columns_absent_from_schema(sqlalchemy_stack): + """PUT must not null out model columns missing from the schema.""" + from sqlalchemy import text + + from fastapi_viewsets import BaseViewset + + model = _make_model(sqlalchemy_stack, "reg_put_secret", extra_column=True) + app = FastAPI() + vs = BaseViewset( + endpoint="/items", + model=model, + response_model=_NameSchema, + db_session=sqlalchemy_stack.get_session, + orm_adapter=sqlalchemy_stack, + ) + vs.register(methods=["POST", "PUT"]) + app.include_router(vs) + client = TestClient(app) + + created = client.post("/items", json={"name": "keepsecret", "price": 1}).json() + db = sqlalchemy_stack.get_session() + db.execute( + text("UPDATE reg_put_secret SET secret='classified' WHERE id=:i"), {"i": created["id"]} + ) + db.commit() + db.close() + + r = client.put(f"/items/{created['id']}", json={"name": "renamed", "price": 2}) + assert r.status_code == 200, r.text + + db = sqlalchemy_stack.get_session() + secret = db.execute( + text("SELECT secret FROM reg_put_secret WHERE id=:i"), {"i": created["id"]} + ).scalar() + db.close() + assert secret == "classified" + + +def test_b5_create_ignores_none_primary_key(sqlalchemy_stack): + """Schemas with ``id: Optional[int] = None`` must not break create.""" + from fastapi_viewsets import BaseViewset + + model = _make_model(sqlalchemy_stack, "reg_pk_none") + app = FastAPI() + vs = BaseViewset( + endpoint="/items", + model=model, + response_model=_NameSchema, + db_session=sqlalchemy_stack.get_session, + orm_adapter=sqlalchemy_stack, + ) + vs.register(methods=["POST"]) + app.include_router(vs) + client = TestClient(app) + + r = client.post("/items", json={"id": None, "name": "auto", "price": 3}) + assert r.status_code == 200, r.text + assert r.json()["id"] is not None + + +def test_b6_integrity_error_is_409_without_sql_leak(sqlalchemy_stack): + """Duplicate unique value -> 409, no SQL/driver internals in detail.""" + from fastapi_viewsets import BaseViewset + + model = _make_model(sqlalchemy_stack, "reg_conflict") + app = FastAPI() + vs = BaseViewset( + endpoint="/items", + model=model, + response_model=_NameSchema, + db_session=sqlalchemy_stack.get_session, + orm_adapter=sqlalchemy_stack, + ) + vs.register(methods=["POST"]) + app.include_router(vs) + client = TestClient(app) + + assert client.post("/items", json={"name": "dup"}).status_code == 200 + r = client.post("/items", json={"name": "dup"}) + assert r.status_code == 409, r.text + detail = r.json()["detail"] + assert "sqlite3" not in detail + assert "IntegrityError" not in detail + assert "INSERT" not in detail.upper() + + +def test_m2_negative_pagination_rejected(sqlalchemy_stack): + """limit/offset below zero must be rejected with 422.""" + from fastapi_viewsets import BaseViewset + + model = _make_model(sqlalchemy_stack, "reg_pagination") + app = FastAPI() + vs = BaseViewset( + endpoint="/items", + model=model, + response_model=_NameSchema, + db_session=sqlalchemy_stack.get_session, + orm_adapter=sqlalchemy_stack, + ) + vs.register(methods=["LIST"]) + app.include_router(vs) + client = TestClient(app) + + assert client.get("/items?limit=-5").status_code == 422 + assert client.get("/items?offset=-10").status_code == 422 + + +def test_m5_filters_documented_in_openapi(sqlalchemy_stack): + """Whitelisted ListConfig filters are advertised in OpenAPI.""" + from fastapi_viewsets import BaseViewset + + class FilterSchema(_NameSchema): + class ListConfig: + filters = ["price"] + + model = _make_model(sqlalchemy_stack, "reg_openapi_filters") + app = FastAPI() + vs = BaseViewset( + endpoint="/items", + model=model, + response_model=FilterSchema, + db_session=sqlalchemy_stack.get_session, + orm_adapter=sqlalchemy_stack, + ) + vs.register(methods=["LIST"]) + app.include_router(vs) + client = TestClient(app) + + spec = client.get("/openapi.json").json() + params = {p["name"] for p in spec["paths"]["/items"]["get"].get("parameters", [])} + assert "price" in params + assert "price__gte" in params + assert "price__in" in params + + +@pytest.mark.asyncio +async def test_b5_tortoise_create_with_none_pk(tmp_path): + """Tortoise adapter create strips an explicit None primary key.""" + pytest.importorskip("tortoise") + from tortoise import Tortoise + + from fastapi_viewsets.orm.tortoise_adapter import TortoiseAdapter + + # File-backed DB: the adapter and the schema generator each open + # their own connection, and ``:memory:`` databases are per-connection. + db_url = f"sqlite://{tmp_path}/tortoise_reg.db" + + adapter = TortoiseAdapter( + database_url=db_url, + models=["tests.test_write_path_regressions"], + app_label="regressions", + ) + await Tortoise.init( + db_url=db_url, + modules={"regressions": ["tests.test_write_path_regressions"]}, + ) + await Tortoise.generate_schemas(safe=True) + try: + obj = await adapter.create_element_async( + RegTortoiseModel, adapter.get_async_session, {"id": None, "name": "t1"} + ) + assert obj.id is not None + assert obj.name == "t1" + finally: + await Tortoise.close_connections() + + +try: + from tortoise import fields + from tortoise.models import Model as _TortoiseModel + + class RegTortoiseModel(_TortoiseModel): + id = fields.IntField(pk=True) + name = fields.CharField(max_length=255, unique=True) + + class Meta: + app = "regressions" + +except ImportError: # pragma: no cover - tortoise optional + RegTortoiseModel = None + + +# --- Coverage for the non-bound-method fallback in _register.py -------- + + +class _CustomPostHandler: + """A POST handler that is not a plain bound method (callable object). + + ``register()`` must fall back to best-effort annotation patching on + the handler object itself instead of cloning the function. + """ + + __annotations__ = {"item": "ItemSchema"} + + def __call__(self, item=None, token=None): + return {"status": True, "text": "created"} + + +class _UnpatchablePostHandler(_CustomPostHandler): + """A callable whose ``__annotations__`` cannot be reassigned. + + Registration must still succeed (patching is best-effort). + """ + + @property + def __annotations__(self): # noqa: N805 - instance-level property + return {"item": "ItemSchema"} + + +def test_register_patches_non_bound_method_handler(): + """Callable-object handlers get the body schema merged into their + own annotations without breaking registration.""" + from fastapi_viewsets import BaseViewset + + vs = BaseViewset(endpoint="/custom", model=None, response_model=_NameSchema) + handler = _CustomPostHandler() + vs.create_element = handler + + vs.register(methods=["POST"]) + + assert handler.__annotations__["item"] is _NameSchema + assert any(getattr(r, "path", None) == "/custom" for r in vs.routes) + + +def test_register_tolerates_unpatchable_non_bound_method_handler(): + """Best-effort patching: even a handler that rejects annotation + assignment must leave register() working.""" + from fastapi_viewsets import BaseViewset + + vs = BaseViewset(endpoint="/other", model=None, response_model=_NameSchema) + handler = _UnpatchablePostHandler() + vs.create_element = handler + + vs.register(methods=["POST"]) # must not raise + + # the untouched property still serves the original annotations + assert handler.__annotations__ == {"item": "ItemSchema"} + assert any(getattr(r, "path", None) == "/other" for r in vs.routes) + + +# --- Coverage for the Query-default fallback in list() ----------------- + + +def test_m6_programmatic_list_non_int_pagination(test_model, test_schema, db_session_factory): + """Direct programmatic ``list()`` calls receive the unresolved + ``Query`` defaults; the handler must fall back to sane values (sync).""" + from fastapi_viewsets import BaseViewset + + vs = BaseViewset( + endpoint="/test", + model=test_model, + response_model=test_schema, + db_session=db_session_factory, + tags=["Test"], + ) + + # no kwargs: limit/offset arrive as Query(...) objects, not ints + result = vs.list() + assert isinstance(result, list) + + +@pytest.mark.asyncio +async def test_m6_programmatic_async_list_non_int_pagination( + test_model, test_schema, async_db_session_factory +): + """Same fallback for ``AsyncBaseViewset.list``.""" + from fastapi_viewsets import AsyncBaseViewset + + vs = AsyncBaseViewset( + endpoint="/test", + model=test_model, + response_model=test_schema, + db_session=async_db_session_factory, + tags=["Test"], + ) + + result = await vs.list() + assert isinstance(result, list)