From 04c0e9fbedb0cdd1b63423cdbae121abac95ea0a Mon Sep 17 00:00:00 2001 From: danielhasan1 Date: Sat, 15 Aug 2026 23:48:59 +0530 Subject: [PATCH 1/5] Make Filter/Sorter/Paginator/QueryStrategy backend-agnostic, add ClickHouse reference backend fastapi-listing's default filter/sort/paginate implementations talked to a raw SQLAlchemy Query directly, and four modules imported sqlalchemy unconditionally at load time, so the library couldn't even be imported without it - let alone plugged into a different ORM or a non-ORM backend. Introduces a QueryContext contract (fastapi_listing.context) that every Filter/Sorter/Paginator/ QueryStrategy now speaks instead, with SqlAlchemyQueryContext as a thin pass-through adapter and ClickHouseQueryContext + ClickHouseDao as a reference non-ORM backend (parameterized raw SQL via clickhouse-driver) proving the abstraction actually generalizes rather than being SQLAlchemy-only in disguise. - generic_filters.CommonFilterImpl -> CanonicalFilter: filters declare a canonical Op (fastapi_listing.ops.Op) and work unchanged against any backend (deprecated alias kept for one release) - PaginationStrategy gains a postprocess(rows, extra_context) hook for post-fetch business logic that previously had no governed home - import fastapi_listing no longer requires SQLAlchemy; clickhouse-driver ships as an opt-in extra, neither is in install_requires - FastapiListingMigrationError replaces bare TypeError/AttributeError when pre-0.4.0 custom Filter/Sorter/QueryStrategy subclasses run unmigrated - loader.py's DAO check now validates against DaoAbstract instead of hardcoding GenericDao, so non-SQLAlchemy DAOs register the same way - typing.Literal (3.8+) guarded with a typing_extensions fallback in the new context modules, matching the existing ctyping.py/service/config.py pattern, so import fastapi_listing still works on Python 3.7 Breaking change for custom Filter/Sorter/QueryStrategy/PaginationStrategy subclasses (signature/return-type change); the default GenericDao + generic_filters + default-strategies flow is unaffected. See CHANGELOG.md. Closes #10 --- CHANGELOG.md | 67 ++++ README.md | 30 +- docs/filters.rst | 26 +- docs/index.rst | 6 +- docs/paginator.rst | 24 ++ docs/query.rst | 38 +- fastapi_listing/__init__.py | 2 +- fastapi_listing/abstracts/base_query.py | 5 +- fastapi_listing/abstracts/dao.py | 13 +- fastapi_listing/abstracts/filter.py | 4 +- fastapi_listing/abstracts/interceptor.py | 11 +- fastapi_listing/abstracts/listing.py | 11 +- fastapi_listing/abstracts/paginator.py | 15 +- fastapi_listing/abstracts/sorter.py | 7 +- fastapi_listing/context/__init__.py | 10 + fastapi_listing/context/base.py | 62 ++++ fastapi_listing/context/clickhouse.py | 238 +++++++++++++ fastapi_listing/context/sqlalchemy.py | 100 ++++++ fastapi_listing/dao/__init__.py | 2 + fastapi_listing/dao/clickhouse_dao.py | 48 +++ fastapi_listing/dao/generic_dao.py | 16 +- fastapi_listing/errors.py | 30 ++ fastapi_listing/factory/filter.py | 10 +- fastapi_listing/filters/generic_filters.py | 211 ++++++----- .../individual_sorter_interceptor.py | 56 +-- .../iterative_filter_interceptor.py | 77 ++-- fastapi_listing/loader.py | 5 +- fastapi_listing/middlewares.py | 16 +- fastapi_listing/ops.py | 30 ++ fastapi_listing/paginator/page_builder.py | 40 ++- .../service/_core_listing_service.py | 34 +- fastapi_listing/sorter/page_sorter.py | 38 +- fastapi_listing/strategies/query_strategy.py | 65 ++-- setup.py | 5 +- tests/clickhouse_listing_setup.py | 70 ++++ tests/service_setup.py | 21 +- tests/sqlalchemy_listing_setup.py | 47 +++ tests/test_clickhouse_backend.py | 52 +++ tests/test_main.py | 2 +- tests/test_main_v2.py | 2 +- tests/test_migration_guard.py | 46 +++ tests/test_query_context.py | 333 ++++++++++++++++++ 42 files changed, 1588 insertions(+), 337 deletions(-) create mode 100644 CHANGELOG.md create mode 100644 fastapi_listing/context/__init__.py create mode 100644 fastapi_listing/context/base.py create mode 100644 fastapi_listing/context/clickhouse.py create mode 100644 fastapi_listing/context/sqlalchemy.py create mode 100644 fastapi_listing/dao/clickhouse_dao.py create mode 100644 fastapi_listing/ops.py create mode 100644 tests/clickhouse_listing_setup.py create mode 100644 tests/sqlalchemy_listing_setup.py create mode 100644 tests/test_clickhouse_backend.py create mode 100644 tests/test_migration_guard.py create mode 100644 tests/test_query_context.py diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..7df296b --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,67 @@ +# Changelog + +## 0.4.0 + +**Breaking change** for anyone who wrote a custom `Filter`, `Sorter`, `QueryStrategy`, or `PaginationStrategy` +subclass. **Not** breaking for the plain README flow (`GenericDao` + `generic_filters` + default strategies) - +that continues to work unchanged. + +### Why + +fastapi-listing's Filter/Sorter/Paginator/QueryStrategy contracts were pluggable in theory, but every shipped +default implementation talked to a raw SQLAlchemy `Query` directly, and four modules imported SQLAlchemy +unconditionally at module load time - so the library couldn't even be imported without SQLAlchemy installed, +let alone actually used with a different backend. This release makes the contract genuinely backend-agnostic +and ships a ClickHouse reference implementation (raw parameterized SQL, no ORM) as proof. + +### What changed + +- A raw `query` object is replaced everywhere by a `QueryContext` (`fastapi_listing.context.QueryContext`): + `Filter.filter(..., context=...)`, `Sorter.sort(..., context=...)`, `Paginator.paginate(context, ...)`, + `QueryStrategy.get_query(...) -> QueryContext`. For SQLAlchemy this is `SqlAlchemyQueryContext`, a thin + wrapper - reach `context.native` for the underlying `Query` when you need something the canonical filter + vocabulary doesn't cover (joins, eager loading, aggregates), then hand it back via `context.with_native(...)`. +- `generic_filters.CommonFilterImpl` is renamed `CanonicalFilter` (kept as a deprecated alias, raises + `DeprecationWarning` on instantiation, will be removed in a future release). The 14 built-in filter classes + (`EqualityFilter`, `InDataFilter`, ...) now each just declare a canonical `Op` (`fastapi_listing.ops.Op`) and + work unchanged against *any* backend - no per-backend filter subclassing needed. +- Added `PaginationStrategy.postprocess(rows, extra_context)` - identity by default, a dedicated seam for + post-fetch business logic (bucket-filling, tie-break re-sorting, export-shape reshaping) that previously had + no governed home. +- `import fastapi_listing` no longer requires SQLAlchemy to be installed (it's now correctly optional, matching + what `setup.py` already claimed). Neither SQLAlchemy nor the new `clickhouse-driver` extra are in + `install_requires` - both are opt-in (`pip install fastapi-listing[clickhouse]` for the ClickHouse backend). +- Added a reference non-ORM backend: `fastapi_listing.dao.ClickHouseDao` + `fastapi_listing.context.clickhouse.ClickHouseQueryContext`, + built on `clickhouse-driver` with real server-side parameter binding (no value is ever string-formatted into + SQL text). +- Loud, actionable failures instead of a bare traceback if you upgrade without migrating: a custom + `Filter`/`Sorter` still using the old `query=` parameter name, or a custom `QueryStrategy` returning a raw + `Query` instead of a `QueryContext`, now raises `FastapiListingMigrationError` with a specific fix, not a + generic `TypeError`/`AttributeError`. +- `loader.py`'s DAO type check now validates against `DaoAbstract` (the actual abstract contract) instead of + hardcoding `GenericDao`, so non-SQLAlchemy DAOs (like `ClickHouseDao`) register the same way SQLAlchemy ones + always have. Error message changed accordingly: `"Invalid Dao Type! Should Be type of DaoAbstract"`. +- Added `QueryContext.having()` (and `ClickHouseQueryContext.from_raw_sql()` for the full bypass - hand-build a + query with CTEs/joins/window functions/whatever, still get back a `QueryContext` canonical filters/sort/ + pagination can layer on top of). Filter on an aggregated field after a `GROUP_BY` via `HavingMixin`, e.g. + `class TotalAbove(HavingMixin, DataGreaterThanFilter): pass` - `WHERE` can't express that, `HAVING` can, same + `Op` vocabulary. Along the way, fixed a latent bug in `ClickHouseQueryContext.count_compile()`: counting a + `GROUP BY` query directly returns one count *per group*, not the total number of groups: it now wraps as a + derived table when grouped/having, matching what SQLAlchemy's `Query.count()` already did automatically. + +### Migration + +If you only use the built-in `generic_filters`, the default strategies, and `GenericDao` subclasses - **you +don't need to do anything.** + +If you wrote a custom subclass: + +| Was | Now | +|---|---| +| `def filter(self, *, field=None, value=None, query=None)` | `def filter(self, *, field=None, value=None, context=None)` | +| `query.filter(...)` inside a custom filter | `context.native.filter(...)`, then `context = context.with_native(new_query)` | +| `def sort(self, *, query=None, ...)` | `def sort(self, *, context=None, ...)` | +| `def get_query(...) -> Query: return dao.some_method()` | `return SqlAlchemyQueryContext(dao.some_method())` | +| `CommonFilterImpl` | `CanonicalFilter` (alias still works, warns) | + +See `docs/query.rst` and `docs/filters.rst` for full examples. diff --git a/README.md b/README.md index 86c3543..46e617a 100644 --- a/README.md +++ b/README.md @@ -6,16 +6,22 @@ Advanced items listing library that gives you freedom to design really complex l [![.github/workflows/tests.yml](https://github.com/danielhasan1/fastapi-listing/actions/workflows/tests.yml/badge.svg)](https://github.com/danielhasan1/fastapi-listing/actions/workflows/tests.yml) ![PyPI - Programming Language](https://img.shields.io/pypi/pyversions/fastapi-listing.svg?color=%2334D058) [![codecov](https://codecov.io/gh/danielhasan1/fastapi-listing/branch/dev/graph/badge.svg?token=U29ZRNAH8I)](https://codecov.io/gh/danielhasan1/fastapi-listing) [![Downloads](https://static.pepy.tech/badge/fastapi-listing)](https://pepy.tech/project/fastapi-listing) +> **Upgrading to 0.4.0?** It's a breaking change **only** if you wrote a custom `Filter`/`Sorter`/`QueryStrategy`/ +> `PaginationStrategy` subclass - the plain `GenericDao` + `generic_filters` + default-strategies flow below is +> unaffected. See [CHANGELOG.md](CHANGELOG.md) for the migration table. + Comes with: - pre defined filters - pre defined paginator - pre defined sorter +- SQLAlchemy support out of the box, and a backend-agnostic core so you're not locked into one ORM ## Advantage - simplify the intricate process of designing and developing complex listing APIs - Design components(USP) and plug them from anywhere - Components can be **reusable** - Best for fast changing needs +- Not an ORM captive: filters/sorter/paginator are written against a small `QueryContext` contract, not a raw SQLAlchemy `Query` - swap in a different backend without rewriting your filters ## Installing @@ -72,7 +78,7 @@ class Employee(Base): class EmployeeDao(GenericDao): """write your data layer access logic here. keep it raw!""" name = "employee" - model = Employee # sqlalchemy model class (support for pymongo/tortoise orm is in progress) + model = Employee # sqlalchemy model class. Not on SQLAlchemy? See "Backend support" below. class EmployeeListDetails(BaseModel): @@ -414,6 +420,28 @@ You can check out customisation section in docs after going through basics and t Check out my other [repo](https://github.com/danielhasan1/test-fastapi-listing/blob/master/app/router/router.py) to see some examples +## Backend support + +fastapi-listing ships with SQLAlchemy support by default, but nothing in the Filter/Sorter/Paginator/QueryStrategy +contract is SQLAlchemy-specific. Every one of them is written against a small `QueryContext` interface +(`fastapi_listing/context`), not a raw SQLAlchemy `Query` - `SqlAlchemyQueryContext` is just the default +implementation of it. + +As proof this isn't SQLAlchemy in disguise, a **ClickHouse** backend ships as a reference implementation +(`fastapi_listing.dao.ClickHouseDao` + `fastapi_listing.context.clickhouse.ClickHouseQueryContext`) - raw +parameterized SQL via `clickhouse-driver`, no ORM at all. The same `generic_filters` classes +(`EqualityFilter`, `InDataFilter`, ...) and the default `SortingOrderStrategy`/`PaginationStrategy` work +against it completely unmodified, because they only ever talk to the `QueryContext`, never to SQLAlchemy +directly. + +```python +pip install fastapi-listing[clickhouse] +``` + +Want a different ORM or database driver (Tortoise, Django ORM, raw psycopg2, pymongo, ...)? Write your own +`QueryContext` + DAO pair the same way `ClickHouseQueryContext`/`ClickHouseDao` do it - see `docs/query.rst`. +Neither SQLAlchemy nor clickhouse-driver are required to install the package; both are opt-in extras. + ## Features and Readability hand in hand 🤝 - Well defined interface for filter, sorter, paginator diff --git a/docs/filters.rst b/docs/filters.rst index e04d5fa..63aa2c1 100644 --- a/docs/filters.rst +++ b/docs/filters.rst @@ -144,23 +144,35 @@ Its easy to do as well. You wanna write a filter which does a full name scan com from fastapi_listing.filters import generic_filters from fastapi_listing.dao import dao_factory + from fastapi_listing.context import QueryContext - class FullNameFilter(generic_filters.CommonFilterImpl): + class FullNameFilter(generic_filters.CanonicalFilter): - def filter(self, *, field: str = None, value: dict = None, query=None) -> SqlAlchemyQuery: + def filter(self, *, field: str = None, value: dict = None, context: QueryContext = None) -> QueryContext: # field is not necessary here as this is a custom filter and user have full control over its implementation if value: emp_dao: EmployeeDao = dao_factory.create("employee", replica=True) emp_ids: list[int] = emp_dao.get_emp_ids_contain_full_name(value.get("search")) - query = query.filter(self.dao.model.emp_no.in_(emp_ids)) - return query - -As you can see in above filter class we are inheriting from a class which is a part of our ``generic_filters`` module. -In our filter class we have a single filter method with fixed signature. you will receive your filter value as a dict. + native = context.native.filter(self.dao.model.emp_no.in_(emp_ids)) + context = context.with_native(native) + return context + +As you can see in above filter class we are inheriting from ``CanonicalFilter``, part of our ``generic_filters`` +module (``CommonFilterImpl`` is kept as a deprecated alias for one release if you're upgrading existing code). +In our filter class we have a single filter method with fixed signature - note the last argument is now +``context`` (a backend-agnostic ``QueryContext``) rather than a raw SQLAlchemy ``query``. When you need SQLAlchemy-specific +behaviour like a fluent ``.filter()`` chain, use ``context.native`` to reach the underlying ``Query`` and +``context.with_native(...)`` to hand the mutated query back. you will receive your filter value as a dict. We have also used **dao factory** which allows us to use anywhere dao policy. You basically filter your query and return it. And just like that voila your custom filter is ready. No need to think how you will call it, this will be handled implicitly by filter mechanics(interceptor). +Most built-in filters don't need any of this: they simply declare a canonical ``op`` (see ``fastapi_listing.ops.Op``) +and hand off to the context - ``EqualityFilter``, ``InDataFilter`` and the rest of ``generic_filters`` work +unmodified whether your DAO is backed by SQLAlchemy or a non-ORM backend like the reference ``ClickHouseDao``. +Write a custom filter with ``context.native`` only when the canonical ``Op`` vocabulary genuinely can't express +what you need. + Why do we need an interceptor? Just bear with this example to have an idea of when you may wanna use or write your own interceptor. Lets say you have a listing of products and a mapping table where products are mapped to some groups and each group belongs to a bigger group. diff --git a/docs/index.rst b/docs/index.rst index 85db1f2..79462c1 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -13,8 +13,10 @@ a highly extensible, decoupled and reusable interface. **Component** based Plug & Play architecture allows you to write easy to use and more **quickly** readable block of code. Inject dependencies or swap components as you write more and more complex logics. -It uses `SQLAlchemy `_ sqltool at the time but have potential to support multiple ORMs/database -toolkits and that will be coming soon like mongoengine 📝. +It ships with SQLAlchemy support out of the box, but the Filter/Sorter/Paginator/QueryStrategy contracts are +backend-agnostic (see the *Customising your listing query* guide) - a non-ORM ClickHouse backend ships as a +reference implementation proving the same abstraction works for raw parameterized SQL too, and the same +approach extends to other ORMs/database toolkits. Features -------- diff --git a/docs/paginator.rst b/docs/paginator.rst index dad46be..0f83972 100644 --- a/docs/paginator.rst +++ b/docs/paginator.rst @@ -21,6 +21,30 @@ any count query. +Post-fetch business logic +------------------------- + +Not everything belongs in the query. Filling in zero-value rows for missing time buckets, a tie-break +re-sort that can't be expressed in SQL, reshaping rows differently for CSV export than for the JSON +response - these are all real needs that have nothing to do with filtering/sorting/pagination, but if +your listing framework has no dedicated seam for them they tend to get bolted onto whatever's nearby +(usually the endpoint function itself), which is exactly how disciplined query-building code turns into +an unmaintainable pile over time. + +``PaginationStrategy`` has one governed home for this: override ``postprocess``. + +.. code-block:: python + + class MyPaginationStrategy(PaginationStrategy): + + def postprocess(self, rows, extra_context: dict): + # rows is whatever context.fetch() returned - runs after fetch, before the Page envelope is built + return rows + +It's identity by default and runs once, right after the rows are fetched and before ``hasNext``/``totalCount``/etc. +are assembled into the response. Do post-fetch business logic here, not by overriding ``_get_page``/ +``_get_page_without_count`` (those exist to change the *page envelope shape*) or by reaching into the DAO. + .. _alias overview: Why use alias diff --git a/docs/query.rst b/docs/query.rst index 08607c4..cd493fa 100644 --- a/docs/query.rst +++ b/docs/query.rst @@ -77,7 +77,7 @@ Writing your own query strategy class DepartmentWiseEmployeesQuery(QueryStrategy): def get_query(self, *, request: FastapiRequest = None, dao: EmployeeDao = None, - extra_context: dict = None) -> SqlAlchemyQuery: + extra_context: dict = None) -> QueryContext: # as request and dao args are self explanatory # extra_context is a chained variable that can carry contextual data from one place # to another place. extremely helpful when passing args from router or client. @@ -94,18 +94,18 @@ Add your new listing query to employee dao .. code-block:: python - from sqlalchemy.orm import Query + from fastapi_listing.context.sqlalchemy import SqlAlchemyQueryContext class EmployeeDao(ClassicDao): name = "employee" model = Employee - def get_employees_by_dept(self, dept_no: str) -> Query: + def get_employees_by_dept(self, dept_no: str) -> SqlAlchemyQueryContext: # assuming we have one to one mapping and we are passing manager department here query = self._read_db.query(self.model ).join(DeptEmp, Employee.emp_no == DeptEmp.emp_no ).filter(DeptEmp.dept_no == dept_no) - return query + return SqlAlchemyQueryContext(query) .. code-block:: python @@ -192,4 +192,32 @@ Second Example Personally I mixes both of these when I know strategies are going to be simple I tend to make strategy objects capable of handlind different contexts but when I know or see my single strategy class is becoming hard to maintain I tend to breakdown them to handle specefic context at a time as a result having -single responsibility objects. \ No newline at end of file +single responsibility objects. + +Backend-agnostic query objects (SQLAlchemy is no longer the only option) +-------------------------------------------------------------------------- + +Every place that used to pass around a raw SQLAlchemy ``Query`` now passes around a ``QueryContext`` +(``fastapi_listing.context.QueryContext``) instead. For the default SQLAlchemy backend this is just a thin +wrapper - ``SqlAlchemyQueryContext`` - around your existing ``Query``, so ``get_query``/custom ``QueryStrategy`` +methods should return one of these instead of a bare ``Query``: + +.. code-block:: python + + from fastapi_listing.context.sqlalchemy import SqlAlchemyQueryContext + + class MyQueryStrategy(QueryStrategy): + + def get_query(self, *, request=None, dao=None, extra_context: dict = None) -> QueryContext: + query = dao.get_default_read([...]) # a GenericDao already returns a QueryContext + return query + +If you need to do something the canonical filter/sort vocabulary doesn't cover (joins, eager loading, +aggregates, ...), drop down to the raw SQLAlchemy ``Query`` via ``context.native``, mutate it however you like, +then hand it back via ``context.with_native(new_query)``. + +This same ``QueryContext`` contract is what lets FastAPI Listing support non-ORM backends - a +``ClickHouseQueryContext`` (``fastapi_listing.context.clickhouse``) ships as a reference implementation, +paired with ``fastapi_listing.dao.ClickHouseDao``, proving the same ``Filter``/``SortingOrderStrategy``/ +``PaginationStrategy`` classes work unmodified against raw parameterized SQL, not just an ORM. See +:ref:`learnfilters` for how filters stay backend-agnostic through the shared ``Op`` vocabulary. \ No newline at end of file diff --git a/fastapi_listing/__init__.py b/fastapi_listing/__init__.py index 6a49c17..62dc19a 100644 --- a/fastapi_listing/__init__.py +++ b/fastapi_listing/__init__.py @@ -1,4 +1,4 @@ -__version__ = "0.3.4" +__version__ = "0.4.0" __all__ = [ "ListingService", diff --git a/fastapi_listing/abstracts/base_query.py b/fastapi_listing/abstracts/base_query.py index 39bf50a..e9d90d8 100644 --- a/fastapi_listing/abstracts/base_query.py +++ b/fastapi_listing/abstracts/base_query.py @@ -3,12 +3,13 @@ from abc import ABC, abstractmethod from fastapi_listing.abstracts import DaoAbstract -from fastapi_listing.ctyping import FastapiRequest, SqlAlchemyQuery +from fastapi_listing.context import QueryContext +from fastapi_listing.ctyping import FastapiRequest class AbsQueryStrategy(ABC): @abstractmethod def get_query(self, *, request: Optional[FastapiRequest] = None, dao: DaoAbstract, - extra_context: dict) -> SqlAlchemyQuery: + extra_context: dict) -> QueryContext: pass diff --git a/fastapi_listing/abstracts/dao.py b/fastapi_listing/abstracts/dao.py index b975e78..dc86ec9 100644 --- a/fastapi_listing/abstracts/dao.py +++ b/fastapi_listing/abstracts/dao.py @@ -1,14 +1,15 @@ from abc import ABCMeta, abstractmethod -from typing import Union, Dict, List -from fastapi_listing.ctyping import SqlAlchemyModel +from typing import Any class DaoAbstract(metaclass=ABCMeta): @property @abstractmethod - def model(self) -> SqlAlchemyModel: - pass + def model(self) -> Any: + """The schema/model this DAO reads and writes. A SQLAlchemy + declarative class for the default backend, or any plain + column-name descriptor for a non-ORM backend (e.g. ClickHouseDao).""" @property @abstractmethod @@ -16,7 +17,7 @@ def name(self) -> str: pass @abstractmethod - def create(self, values) -> SqlAlchemyModel: + def create(self, values) -> Any: pass @abstractmethod @@ -24,7 +25,7 @@ def update(self, identifier, values) -> bool: pass @abstractmethod - def read(self, identifier, fields) -> SqlAlchemyModel: + def read(self, identifier, fields) -> Any: pass @abstractmethod diff --git a/fastapi_listing/abstracts/filter.py b/fastapi_listing/abstracts/filter.py index d24bdd6..a85a386 100644 --- a/fastapi_listing/abstracts/filter.py +++ b/fastapi_listing/abstracts/filter.py @@ -1,9 +1,9 @@ from abc import ABC, abstractmethod -from fastapi_listing.ctyping import SqlAlchemyQuery +from fastapi_listing.context import QueryContext class FilterAbstract(ABC): @abstractmethod - def filter(self, *, field: str = None, value: str = None, query: SqlAlchemyQuery = None): + def filter(self, *, field: str = None, value: str = None, context: QueryContext = None) -> QueryContext: pass diff --git a/fastapi_listing/abstracts/interceptor.py b/fastapi_listing/abstracts/interceptor.py index b9a62e9..d8749d0 100644 --- a/fastapi_listing/abstracts/interceptor.py +++ b/fastapi_listing/abstracts/interceptor.py @@ -3,20 +3,21 @@ from fastapi_listing.abstracts import DaoAbstract from fastapi_listing.abstracts import AbsSortingStrategy -from fastapi_listing.ctyping import SqlAlchemyQuery, FastapiRequest +from fastapi_listing.context import QueryContext +from fastapi_listing.ctyping import FastapiRequest class AbstractFilterInterceptor(ABC): @abstractmethod - def apply(self, *, query: SqlAlchemyQuery = None, filter_params: List[Dict[str, str]], dao: DaoAbstract = None, - request: FastapiRequest = None, extra_context: dict = None) -> SqlAlchemyQuery: + def apply(self, *, context: QueryContext = None, filter_params: List[Dict[str, str]], dao: DaoAbstract = None, + request: FastapiRequest = None, extra_context: dict = None) -> QueryContext: pass class AbstractSorterInterceptor(ABC): @abstractmethod - def apply(self, *, query: SqlAlchemyQuery = None, strategy: AbsSortingStrategy = None, - sorting_params: List[Dict[str, str]] = None, extra_context: dict = None) -> SqlAlchemyQuery: + def apply(self, *, context: QueryContext = None, strategy: AbsSortingStrategy = None, + sorting_params: List[Dict[str, str]] = None, extra_context: dict = None) -> QueryContext: pass diff --git a/fastapi_listing/abstracts/listing.py b/fastapi_listing/abstracts/listing.py index bdd81e6..4ff1aa4 100644 --- a/fastapi_listing/abstracts/listing.py +++ b/fastapi_listing/abstracts/listing.py @@ -1,8 +1,7 @@ from typing import Type from abc import ABC, abstractmethod -from sqlalchemy.orm import Query - +from fastapi_listing.context import QueryContext from fastapi_listing.ctyping import BasePage from fastapi_listing.abstracts import AbstractListingFeatureParamsAdapter from fastapi_listing.dao import GenericDao @@ -12,19 +11,19 @@ class ListingBase(ABC): @abstractmethod - def _prepare_query(self, listing_meta_info: ListingMetaInfo) -> Query: + def _prepare_query(self, listing_meta_info: ListingMetaInfo) -> QueryContext: pass @abstractmethod - def _apply_sorting(self, query: Query, listing_meta_info: ListingMetaInfo) -> Query: + def _apply_sorting(self, query: QueryContext, listing_meta_info: ListingMetaInfo) -> QueryContext: pass @abstractmethod - def _apply_filters(self, query: Query, listing_meta_info: ListingMetaInfo) -> Query: + def _apply_filters(self, query: QueryContext, listing_meta_info: ListingMetaInfo) -> QueryContext: pass @abstractmethod - def _paginate(self, query: Query, listing_meta_info: ListingMetaInfo) -> BasePage: + def _paginate(self, query: QueryContext, listing_meta_info: ListingMetaInfo) -> BasePage: pass @abstractmethod diff --git a/fastapi_listing/abstracts/paginator.py b/fastapi_listing/abstracts/paginator.py index dc4253b..0438b2f 100644 --- a/fastapi_listing/abstracts/paginator.py +++ b/fastapi_listing/abstracts/paginator.py @@ -1,9 +1,20 @@ from abc import ABC, abstractmethod -from fastapi_listing.ctyping import SqlAlchemyQuery, BasePage +from typing import Sequence + +from fastapi_listing.context import QueryContext +from fastapi_listing.ctyping import BasePage class AbsPaginatingStrategy(ABC): @abstractmethod - def paginate(self, query: SqlAlchemyQuery, pagination_params: dict, extra_context: dict) -> BasePage: + def paginate(self, context: QueryContext, pagination_params: dict, + extra_context: dict) -> BasePage: pass + + def postprocess(self, rows: Sequence, extra_context: dict) -> Sequence: + """Hook for post-fetch, non-query business logic (bucket-filling, + tie-break re-sorting, export-shape reshaping, ...). Identity by + default; override in a subclass rather than reaching into + `_get_page`/`_get_page_without_count` or the DAO for this.""" + return rows diff --git a/fastapi_listing/abstracts/sorter.py b/fastapi_listing/abstracts/sorter.py index 2ceb787..a51c79e 100644 --- a/fastapi_listing/abstracts/sorter.py +++ b/fastapi_listing/abstracts/sorter.py @@ -1,11 +1,12 @@ from abc import ABC, abstractmethod -from fastapi_listing.ctyping import SqlAlchemyQuery from typing import Dict +from fastapi_listing.context import QueryContext + class AbsSortingStrategy(ABC): @abstractmethod - def sort(self, *, query: SqlAlchemyQuery = None, value: Dict[str, str], - extra_context: dict = None) -> SqlAlchemyQuery: + def sort(self, *, context: QueryContext = None, value: Dict[str, str], + extra_context: dict = None) -> QueryContext: pass diff --git a/fastapi_listing/context/__init__.py b/fastapi_listing/context/__init__.py new file mode 100644 index 0000000..aa44017 --- /dev/null +++ b/fastapi_listing/context/__init__.py @@ -0,0 +1,10 @@ +from fastapi_listing.context.base import QueryContext +from fastapi_listing.context.sqlalchemy import SqlAlchemyQueryContext + +__all__ = ["QueryContext", "SqlAlchemyQueryContext"] + +try: + from fastapi_listing.context.clickhouse import ClickHouseQueryContext # noqa: F401 + __all__.append("ClickHouseQueryContext") +except ImportError: + pass diff --git a/fastapi_listing/context/base.py b/fastapi_listing/context/base.py new file mode 100644 index 0000000..578edf4 --- /dev/null +++ b/fastapi_listing/context/base.py @@ -0,0 +1,62 @@ +"""Backend-agnostic query context. + +A QueryContext is the object threaded through Filter.filter() / Sorter.sort() / +QueryStrategy.get_query() / Paginator.paginate() in place of a raw SQLAlchemy +Query. Each backend (SQLAlchemy, ClickHouse, ...) implements this contract once; +every canonical Filter subclass and the default Sorter/Paginator/QueryStrategy +work unchanged against any implementation. + +`native` is always available as an escape hatch for backend-specific needs the +canonical Op vocabulary doesn't cover (joins, eager loading, aggregates, ...). +""" + +from abc import ABC, abstractmethod +from typing import Any, Sequence + +try: + from typing import Literal +except ImportError: + from typing_extensions import Literal + +from fastapi_listing.ops import Op + +__all__ = ["QueryContext"] + + +class QueryContext(ABC): + + @property + @abstractmethod + def native(self) -> Any: + """Escape hatch to the underlying backend object (a SQLAlchemy Query, + a (sql, params) tuple, ...) for logic the canonical Op vocabulary + can't express.""" + + @abstractmethod + def where(self, *, field: Any, op: Op, value: Any) -> "QueryContext": + """Apply one canonical operation (see fastapi_listing.ops.Op). This is + the single dispatch point for every Filter subclass, including the + value-less GROUP_BY/DISTINCT operations.""" + + @abstractmethod + def having(self, *, field: Any, op: Op, value: Any) -> "QueryContext": + """Same Op vocabulary as where(), but lands in a HAVING clause instead + of WHERE - for filtering on an aggregated field (e.g. `SUM(x) > 100`) + after a GROUP_BY, which WHERE cannot express. A CanonicalFilter opts + into this by setting `target_clause = "having"`.""" + + @abstractmethod + def order_by(self, *, field: Any, direction: Literal["asc", "dsc"]) -> "QueryContext": + pass + + @abstractmethod + def limit_offset(self, *, limit: int, offset: int) -> "QueryContext": + pass + + @abstractmethod + def count(self) -> int: + pass + + @abstractmethod + def fetch(self) -> Sequence: + pass diff --git a/fastapi_listing/context/clickhouse.py b/fastapi_listing/context/clickhouse.py new file mode 100644 index 0000000..b816f0b --- /dev/null +++ b/fastapi_listing/context/clickhouse.py @@ -0,0 +1,238 @@ +"""ClickHouse QueryContext - the proof-of-concept non-ORM backend. + +Owns its own WHERE/ORDER BY/GROUP BY/LIMIT fragment accumulation and compiles a +single parameterized SQL statement, executed via `clickhouse-driver`. + +Every value flows through the driver's own parameter binding (`%(name)s` +placeholders resolved by `Client.execute(sql, params)`); nothing is ever +string-formatted into the SQL text. Column/field names can't be parameterized +by any SQL driver, but by the time a field reaches `where()`/`order_by()` it has +already been resolved via `getattr(dao.model, field)` upstream (in +CanonicalFilter.extract_field / SortingOrderStrategy.validate_srt_field) against +the DAO's declared model attributes - an unregistered field never reaches here, +it raises AttributeError before this point. That resolution step is this +backend's allowlist, mirroring a hand-written FIELD_COLUMNS mapping. +""" + +from typing import Any, Sequence + +try: + from typing import Literal +except ImportError: + from typing_extensions import Literal + +from fastapi_listing.context.base import QueryContext +from fastapi_listing.ops import Op + +__all__ = ["ClickHouseQueryContext", "quote_identifier"] + + +def _quote_identifier(name: str) -> str: + return f"`{name}`" + + +# Public name for use from a custom Filter's add_raw_condition() call, where +# you need to quote a resolved field/column identifier yourself. +quote_identifier = _quote_identifier + + +class ClickHouseQueryContext(QueryContext): + + def __init__(self, *, client, table: str, columns=None, params=None): + self._client = client + self._table = table + self._columns = list(columns) if columns else None + self._wheres = [] + self._havings = [] + self._params = dict(params) if params else {} + self._param_seq = 0 + self._order_by = None + self._group_by = None + self._distinct = False + self._limit = None + self._offset = None + + @classmethod + def from_raw_sql(cls, *, client, sql: str, params: dict = None, columns=None) -> "ClickHouseQueryContext": + """Full bypass for queries the canonical Op vocabulary can't express - + CTEs, joins, window functions, a runtime-sized UNION ALL, a table + function as the source, ... Build the whole statement yourself (same + query-template functions you'd already be writing), hand it here, and + you still get a QueryContext that plugs into the rest of the pipeline: + `where()`/`order_by()`/`limit_offset()` wrap it as a derived table, so + canonical filters/sort/pagination can still layer on top if you want + them to - or leave filter_mapper empty and let the raw SQL stand as-is. + `params` are your own already-bound parameter names/values; they are + merged in untouched (auto-generated bind keys use a distinct '__fl_' + prefix so they never collide with yours). + """ + return cls(client=client, table=f"({sql}) AS __fl_raw", columns=columns, params=params) + + @property + def native(self): + return self.compile() + + def _bind(self, value) -> str: + self._param_seq += 1 + key = f"__fl_p{self._param_seq}" + self._params[key] = value + return key + + def _condition_sql(self, *, field, op: Op, value) -> str: + """Build the SQL fragment for a comparison Op, binding any values along + the way - shared by where() and having(), since a HAVING clause is the + same Op vocabulary applied to an aggregated field rather than a raw + column.""" + col = _quote_identifier(field) + if op is Op.EQ: + return f"{col} = %({self._bind(value.get('search'))})s" + elif op is Op.NEQ: + return f"{col} != %({self._bind(value.get('search'))})s" + elif op is Op.IN: + return f"{col} IN %({self._bind(tuple(value.get('list') or []))})s" + elif op is Op.RANGE: + start_key = self._bind(value.get("start")) + end_key = self._bind(value.get("end")) + return f"{col} BETWEEN %({start_key})s AND %({end_key})s" + elif op is Op.LIKE: + return f"{col} LIKE %({self._bind(value.get('search'))})s" + elif op is Op.STARTS_WITH: + search = value.get("search") + return f"{col} LIKE %({self._bind(f'{search}%')})s" + elif op is Op.ENDS_WITH: + search = value.get("search") + return f"{col} LIKE %({self._bind(f'%{search}')})s" + elif op is Op.CONTAINS: + search = value.get("search") + return f"{col} LIKE %({self._bind(f'%{search}%')})s" + elif op is Op.GT: + return f"{col} > %({self._bind(value.get('search'))})s" + elif op is Op.GTE: + return f"{col} >= %({self._bind(value.get('search'))})s" + elif op is Op.LT: + return f"{col} < %({self._bind(value.get('search'))})s" + elif op is Op.LTE: + return f"{col} <= %({self._bind(value.get('search'))})s" + elif op is Op.NOT_NULL: + return f"{col} IS NOT NULL" + elif op is Op.IS_NULL: + return f"{col} IS NULL" + raise ValueError(f"Unsupported comparison op: {op!r}") + + def where(self, *, field, op: Op, value) -> "ClickHouseQueryContext": + if op is Op.GROUP_BY: + self._group_by = _quote_identifier(field) + elif op is Op.DISTINCT: + self._distinct = True + elif op in (Op.EQ, Op.NEQ, Op.IN, Op.RANGE, Op.LIKE, Op.STARTS_WITH, Op.ENDS_WITH, Op.CONTAINS, + Op.GT, Op.GTE, Op.LT, Op.LTE, Op.NOT_NULL, Op.IS_NULL): + self._wheres.append(self._condition_sql(field=field, op=op, value=value)) + else: + raise ValueError(f"Unsupported op for ClickHouseQueryContext.where(): {op!r}") + return self + + def having(self, *, field, op: Op, value) -> "ClickHouseQueryContext": + self._havings.append(self._condition_sql(field=field, op=op, value=value)) + return self + + def add_raw_condition(self, sql_template: str, *, having: bool = False, **values) -> "ClickHouseQueryContext": + """Per-filter escape hatch for a WHERE/HAVING condition using + ClickHouse-specific syntax the canonical Op vocabulary doesn't cover + (a builtin function like multiSearchAnyCaseInsensitive, an array + literal, ...) - the ClickHouse-side equivalent of reaching into + SqlAlchemyQueryContext.native from inside a custom Filter. + + `sql_template` uses `{name}` placeholders for each of **values; every + value is still bound through the driver's own parameter binding, never + string-formatted into the SQL text directly. A resolved field/column + name is an identifier, not a value - quote and splice it into the + template yourself (matching how every other op already does it here), + don't pass it through **values. Example: + + col = _quote_identifier(field) + context.add_raw_condition( + f"multiSearchAnyCaseInsensitive({col}, {{needles}})", + needles=list_of_substrings, + ) + """ + bound = {name: f"%({self._bind(value)})s" for name, value in values.items()} + condition = sql_template.format(**bound) + (self._havings if having else self._wheres).append(condition) + return self + + def order_by(self, *, field, direction: Literal["asc", "dsc"]) -> "ClickHouseQueryContext": + self._order_by = f"{_quote_identifier(field)} {'ASC' if direction == 'asc' else 'DESC'}" + return self + + def order_by_raw(self, expression: str) -> "ClickHouseQueryContext": + """Escape hatch for ordering the canonical order_by(field, direction) + can't express - a compound tiebreak expression, multiple sort keys, + `(x = 1) DESC, y ASC`, ... Same philosophy as from_raw_sql: drop to + raw SQL only where the canonical vocabulary genuinely doesn't reach.""" + self._order_by = expression + return self + + def limit_offset(self, *, limit: int, offset: int) -> "ClickHouseQueryContext": + self._limit = limit + self._offset = offset + return self + + def _where_sql(self) -> str: + return f" WHERE {' AND '.join(self._wheres)}" if self._wheres else "" + + def _having_sql(self) -> str: + return f" HAVING {' AND '.join(self._havings)}" if self._havings else "" + + def _select_columns_sql(self) -> str: + if not self._columns: + return "*" + return ", ".join(_quote_identifier(c) for c in self._columns) + + def _grouped_body_sql(self, select_list: str) -> str: + """The shared FROM ... WHERE ... GROUP BY ... HAVING ... body, used by + both compile() (selecting real columns) and count_compile() (selecting + a constant, for wrapping).""" + sql = f"SELECT {select_list} FROM {self._table}" + sql += self._where_sql() + if self._group_by: + sql += f" GROUP BY {self._group_by}" + sql += self._having_sql() + return sql + + def compile(self): + """Return (sql, params) for the row-fetching statement, without executing it.""" + select_list = f"{'DISTINCT ' if self._distinct else ''}{self._select_columns_sql()}" + sql = self._grouped_body_sql(select_list) + if self._order_by: + sql += f" ORDER BY {self._order_by}" + if self._limit is not None: + sql += f" LIMIT {int(self._limit)}" + if self._offset: + sql += f" OFFSET {int(self._offset)}" + return sql, dict(self._params) + + def count_compile(self): + """Return (sql, params) for the count statement, without executing it. + + Plain (no GROUP BY/HAVING): a direct `SELECT count(*) FROM ... WHERE ...`. + Grouped/aggregated: counting rows of a GROUP BY query directly would + return one count *per group*, not the total number of groups - so the + grouped body is wrapped as a derived table and counted from outside, + matching what SQLAlchemy's Query.count() already does automatically. + """ + if self._group_by or self._havings: + inner = self._grouped_body_sql("1") + return f"SELECT count(*) FROM ({inner})", dict(self._params) + sql = f"SELECT count(*) FROM {self._table}{self._where_sql()}" + return sql, dict(self._params) + + def count(self) -> int: + sql, params = self.count_compile() + rows = self._client.execute(sql, params) + return rows[0][0] + + def fetch(self) -> Sequence: + sql, params = self.compile() + rows, columns_with_types = self._client.execute(sql, params, with_column_types=True) + column_names = [c[0] for c in columns_with_types] + return [dict(zip(column_names, row)) for row in rows] diff --git a/fastapi_listing/context/sqlalchemy.py b/fastapi_listing/context/sqlalchemy.py new file mode 100644 index 0000000..5e50954 --- /dev/null +++ b/fastapi_listing/context/sqlalchemy.py @@ -0,0 +1,100 @@ +"""SQLAlchemy QueryContext adapter. + +Thin pass-through wrapping a SQLAlchemy Query. `where()` dispatches each +canonical Op to the exact fluent-API call the old per-backend filter classes +used to make directly - moved here verbatim, not rewritten. +""" + +from typing import Any, Sequence + +try: + from typing import Literal +except ImportError: + from typing_extensions import Literal + +from fastapi_listing.context.base import QueryContext +from fastapi_listing.ops import Op + +__all__ = ["SqlAlchemyQueryContext"] + + +class SqlAlchemyQueryContext(QueryContext): + + def __init__(self, query): + self._query = query + + @property + def native(self): + return self._query + + def with_native(self, query) -> "SqlAlchemyQueryContext": + """Rewrap a Query a caller mutated directly via `.native` (e.g. after + a custom .join()/.options() call the canonical Op vocabulary can't + express).""" + self._query = query + return self + + @staticmethod + def _condition(*, field, op: Op, value): + """Build the boolean expression for a comparison Op - shared by + where() and having(), since a HAVING clause is the same Op vocabulary + applied to an aggregated field rather than a raw column.""" + if op is Op.EQ: + return field == value.get("search") + elif op is Op.NEQ: + return field != value.get("search") + elif op is Op.IN: + return field.in_(value.get("list")) + elif op is Op.RANGE: + return field.between(value.get("start"), value.get("end")) + elif op is Op.LIKE: + return field.like(value.get("search")) + elif op is Op.STARTS_WITH: + return field.startswith(value.get("search")) + elif op is Op.ENDS_WITH: + return field.endswith(value.get("search")) + elif op is Op.CONTAINS: + return field.contains(value.get("search")) + elif op is Op.GT: + return field > value.get("search") + elif op is Op.GTE: + return field >= value.get("search") + elif op is Op.LT: + return field < value.get("search") + elif op is Op.LTE: + return field <= value.get("search") + elif op is Op.NOT_NULL: + return field.is_not(None) + elif op is Op.IS_NULL: + return field.is_(None) + raise ValueError(f"Unsupported comparison op: {op!r}") + + def where(self, *, field, op: Op, value) -> "SqlAlchemyQueryContext": + if op is Op.GROUP_BY: + self._query = self._query.group_by(field) + elif op is Op.DISTINCT: + self._query = self._query.distinct(field) + elif op in (Op.EQ, Op.NEQ, Op.IN, Op.RANGE, Op.LIKE, Op.STARTS_WITH, Op.ENDS_WITH, Op.CONTAINS, + Op.GT, Op.GTE, Op.LT, Op.LTE, Op.NOT_NULL, Op.IS_NULL): + self._query = self._query.filter(self._condition(field=field, op=op, value=value)) + else: + raise ValueError(f"Unsupported op for SqlAlchemyQueryContext.where(): {op!r}") + return self + + def having(self, *, field, op: Op, value) -> "SqlAlchemyQueryContext": + self._query = self._query.having(self._condition(field=field, op=op, value=value)) + return self + + def order_by(self, *, field, direction: Literal["asc", "dsc"]) -> "SqlAlchemyQueryContext": + self._query = self._query.order_by(field.asc() if direction == "asc" else field.desc()) + return self + + def limit_offset(self, *, limit: int, offset: int) -> "SqlAlchemyQueryContext": + self._query = self._query.limit(limit).offset(offset) + return self + + def count(self) -> int: + return self._query.count() + + def fetch(self) -> Sequence: + return self._query.all() diff --git a/fastapi_listing/dao/__init__.py b/fastapi_listing/dao/__init__.py index 1e95fd0..bf16c2f 100644 --- a/fastapi_listing/dao/__init__.py +++ b/fastapi_listing/dao/__init__.py @@ -1,7 +1,9 @@ from .generic_dao import GenericDao +from .clickhouse_dao import ClickHouseDao from fastapi_listing.dao.dao_registry import dao_factory __all__ = [ "GenericDao", + "ClickHouseDao", "dao_factory" ] diff --git a/fastapi_listing/dao/clickhouse_dao.py b/fastapi_listing/dao/clickhouse_dao.py new file mode 100644 index 0000000..97874a3 --- /dev/null +++ b/fastapi_listing/dao/clickhouse_dao.py @@ -0,0 +1,48 @@ +"""Reference non-ORM backend: a DAO that talks raw ClickHouse SQL through +`clickhouse-driver`, proving the QueryContext abstraction generalizes beyond +SQLAlchemy. + +Subclasses set `name` and `model` exactly like GenericDao subclasses do, except +`model` is a plain class whose attributes are column-name strings (no +declarative base, no ORM) - e.g.: + + class Employee: + __table__ = "employees" + id = "id" + name = "name" + +`getattr(Employee, "name")` then resolves to the string "name", which is all +CanonicalFilter.extract_field() / SortingOrderStrategy.validate_srt_field() +ever need - both already just do `getattr(model, field)`, unchanged. +""" + +from fastapi_listing.abstracts import DaoAbstract +from fastapi_listing.context.clickhouse import ClickHouseQueryContext + + +class ClickHouseDao(DaoAbstract): + + def __init__(self, read_db=None, write_db=None): + """ + read_db/write_db are expected to be `clickhouse_driver.Client` + instances (or anything exposing the same `.execute()` signature - + including a fake client in tests). + """ + self._read_db = read_db + self._write_db = write_db + + def create(self, values): + raise NotImplementedError("ClickHouse's mutation model doesn't map to CRUD; implement per-DAO if needed.") + + def update(self, identifier, values): + raise NotImplementedError("ClickHouse's mutation model doesn't map to CRUD; implement per-DAO if needed.") + + def read(self, identifier, fields): + raise NotImplementedError("ClickHouse's mutation model doesn't map to CRUD; implement per-DAO if needed.") + + def delete(self, identifier): + raise NotImplementedError("ClickHouse's mutation model doesn't map to CRUD; implement per-DAO if needed.") + + def get_default_read(self, fields_to_read: list): + table = getattr(self.model, "__table__", None) or self.model.__name__.lower() + return ClickHouseQueryContext(client=self._read_db, table=table, columns=fields_to_read) diff --git a/fastapi_listing/dao/generic_dao.py b/fastapi_listing/dao/generic_dao.py index d86c020..5931857 100644 --- a/fastapi_listing/dao/generic_dao.py +++ b/fastapi_listing/dao/generic_dao.py @@ -1,7 +1,7 @@ from fastapi_listing.abstracts import DaoAbstract -from sqlalchemy.orm import Session +from fastapi_listing.context.sqlalchemy import SqlAlchemyQueryContext -from fastapi_listing.ctyping import SqlAlchemyModel +from fastapi_listing.ctyping import SqlAlchemyModel, SqlAlchemySession # noinspection PyAbstractClass @@ -37,8 +37,8 @@ def __init__(self, read_db=None, write_db=None): since we already have the basic setup injecting the right session(read_db = read database session, write_db = write database session) will save hours of debugging and fixing when needed. """ - self._read_db: Session = read_db - self._write_db: Session = write_db + self._read_db: SqlAlchemySession = read_db + self._write_db: SqlAlchemySession = write_db def create(self, values) -> SqlAlchemyModel: """ @@ -73,10 +73,10 @@ def delete(self, identifier) -> bool: def get_default_read(self, fields_to_read: list): """ - Returns default model query with provided fields - Subclasses can use this to write custom listing queries when - there is no need for multiple queries. + Returns default model query with provided fields, wrapped in a + QueryContext so it can flow through the backend-agnostic + Filter/Sorter/Paginator pipeline. fields_to_read can be left or used. """ - return self._read_db.query(*fields_to_read) + return SqlAlchemyQueryContext(self._read_db.query(*fields_to_read)) diff --git a/fastapi_listing/errors.py b/fastapi_listing/errors.py index 884913e..9d6ac05 100644 --- a/fastapi_listing/errors.py +++ b/fastapi_listing/errors.py @@ -1,3 +1,5 @@ +import inspect + from fastapi import HTTPException @@ -50,3 +52,31 @@ class MissingExpectedAttribute(Exception): class FastAPIListingWarning(UserWarning): pass + + +class FastapiListingMigrationError(FastapiListingError): + """Raised when code written against the pre-0.4.0 raw-SQLAlchemy-Query + contract (Filter/Sorter methods keyed on 'query=', custom QueryStrategy + returning a bare Query) is run against 0.4.0+, instead of letting it fail + with a bare, unhelpful TypeError/AttributeError.""" + + def __init__(self, message: str): + super().__init__( + f"{message}\n" + "fastapi-listing 0.4.0 replaced the raw SQLAlchemy Query threaded through " + "Filter/Sorter/QueryStrategy/Paginator with a backend-agnostic QueryContext. " + "See docs/query.rst and docs/filters.rst for the migration." + ) + + +def guard_legacy_signature(bound_method, *, legacy_kwarg: str, new_kwarg: str, subject: str, fix: str) -> None: + """Proactively detect a pre-0.4.0 method signature (still keyed on the old + kwarg name) before calling it, so the failure is a clear + FastapiListingMigrationError instead of a bare TypeError raised mid-call + (which would be indistinguishable from an unrelated bug in the method).""" + try: + params = inspect.signature(bound_method).parameters + except (TypeError, ValueError): + return + if new_kwarg not in params and legacy_kwarg in params: + raise FastapiListingMigrationError(f"{subject} still uses the pre-0.4.0 {legacy_kwarg!r} parameter. {fix}") diff --git a/fastapi_listing/factory/filter.py b/fastapi_listing/factory/filter.py index 8e9f309..373715b 100644 --- a/fastapi_listing/factory/filter.py +++ b/fastapi_listing/factory/filter.py @@ -2,7 +2,7 @@ import inspect import types -from fastapi_listing.filters.generic_filters import CommonFilterImpl +from fastapi_listing.filters.generic_filters import CanonicalFilter from fastapi_listing.ctyping import AnySqlAlchemyColumn @@ -10,7 +10,7 @@ class FilterObjectFactory: def __init__(self): self._filters = {} - def register_filter(self, key: str, builder: CommonFilterImpl, + def register_filter(self, key: str, builder: CanonicalFilter, field_extractor_fn: Callable[[str], AnySqlAlchemyColumn] = None): if key is None or not key: raise ValueError("Invalid type key!") @@ -27,15 +27,15 @@ def is_mapper_semantic_valid(self, mapper_val): raise ValueError(f"Invalid filter mapper semantic {mapper_val}! first tuple element should be field (str)") if not inspect.isclass(mapper_val[1]): raise ValueError(f"Invalid filter mapper semantic {mapper_val[1]!r}! Expects a class!") - if not issubclass(mapper_val[1], CommonFilterImpl) and mapper_val[1] != CommonFilterImpl: + if not issubclass(mapper_val[1], CanonicalFilter) and mapper_val[1] != CanonicalFilter: raise ValueError(f"Invalid filter mapper semantic {mapper_val[1]!r}!" - f" Expects a subclass of CommonFilterImpl") + f" Expects a subclass of CanonicalFilter") if len(mapper_val) == 3 and not isinstance(mapper_val[2], types.FunctionType): raise ValueError(f"positional arg error, expects a callable but received: {mapper_val[2]!r}!") return True def register_filter_mapper( - self, filter_mapper: Dict[str, Tuple[str, CommonFilterImpl, Optional[Callable[[str], AnySqlAlchemyColumn]]]] + self, filter_mapper: Dict[str, Tuple[str, CanonicalFilter, Optional[Callable[[str], AnySqlAlchemyColumn]]]] ): for key, val in filter_mapper.items(): if self.is_mapper_semantic_valid(val): diff --git a/fastapi_listing/filters/generic_filters.py b/fastapi_listing/filters/generic_filters.py index 79a0629..7d77a3c 100644 --- a/fastapi_listing/filters/generic_filters.py +++ b/fastapi_listing/filters/generic_filters.py @@ -1,5 +1,7 @@ __all__ = [ + "CanonicalFilter", "CommonFilterImpl", + "HavingMixin", "EqualityFilter", "InEqualityFilter", "InDataFilter", @@ -18,175 +20,158 @@ "MySqlNativeDateFormateRangeFilter", ] -from typing import Callable, Optional +from abc import ABCMeta +from typing import Callable, ClassVar, Optional from datetime import datetime +from warnings import warn + +try: + from typing import Literal +except ImportError: + from typing_extensions import Literal from fastapi import Request from fastapi_listing.abstracts import FilterAbstract -from fastapi_listing.ctyping import SqlAlchemyQuery, AnySqlAlchemyColumn - - -class CommonFilterImpl(FilterAbstract): +from fastapi_listing.context import QueryContext +from fastapi_listing.ops import Op + + +class CanonicalFilter(FilterAbstract): + """ + Declares a canonical Op (see fastapi_listing.ops.Op) and hands off to + whatever QueryContext the DAO's backend supplies. Backend-specific syntax + (SQLAlchemy `.filter()` chains, ClickHouse WHERE fragments, ...) lives + entirely inside the QueryContext implementation - this class and its + subclasses never touch it, which is what lets the same Filter subclass + work unchanged against any backend. + """ + + op: ClassVar[Op] + target_clause: ClassVar[Literal["where", "having"]] = "where" + """Set to "having" (e.g. via the HavingMixin below) to filter on an + aggregated field (SUM(x) > 100, ...) after a GROUP_BY - WHERE can't + express that, HAVING can, using the exact same Op vocabulary.""" def __init__(self, dao=None, request: Optional[Request] = None, *, extra_context: dict, - field_extract_fn: Callable[[str], AnySqlAlchemyColumn]): - # lambda x: getattr(Model, x) + field_extract_fn: Callable[[str], object] = None): + # field_extract_fn ex: lambda x: getattr(Model, x) self.dao = dao self.request = request self.extra_context = extra_context self.custom_field_extractor = field_extract_fn - def extract_field(self, field: str) -> AnySqlAlchemyColumn: + def extract_field(self, field: str): field = field.split(".")[-1] if self.custom_field_extractor: return self.custom_field_extractor(field) return getattr(self.dao.model, field) - def filter(self, *, field: str = None, value: dict = None, query=None) -> SqlAlchemyQuery: - raise NotImplementedError("To be implemented in child class!") - - -class EqualityFilter(CommonFilterImpl): - - def filter(self, *, field=None, value=None, query=None) -> SqlAlchemyQuery: - inst_field = self.extract_field(field) - if value: - query = query.filter(inst_field == value.get("search")) - return query - - -class InEqualityFilter(CommonFilterImpl): - - def filter(self, *, field: str = None, value: dict = None, query=None) -> SqlAlchemyQuery: - inst_field = self.extract_field(field) - if value: - query = query.filter(inst_field != value.get("search")) - return query + def coerce(self, value: dict) -> dict: + """Override to adapt a raw client-supplied value (unit conversion, + epoch-ms -> datetime, etc.) before it reaches the QueryContext.""" + return value + def filter(self, *, field: str = None, value: dict = None, + context: QueryContext = None) -> QueryContext: + if value is None and self.op not in (Op.GROUP_BY, Op.DISTINCT): + return context + dispatch = context.having if self.target_clause == "having" else context.where + return dispatch(field=self.extract_field(field), op=self.op, value=self.coerce(value)) -class InDataFilter(CommonFilterImpl): - def filter(self, *, field: str = None, value: dict = None, query=None) -> SqlAlchemyQuery: - inst_field = self.extract_field(field) - if value: - query = query.filter(inst_field.in_(value.get("list"))) - return query +class HavingMixin: + """Mix into any CanonicalFilter op-subclass to route it through HAVING + instead of WHERE, e.g.: + class TotalConversionsAbove(HavingMixin, DataGreaterThanFilter): + pass + """ + target_clause = "having" -class BetweenUnixMilliSecDateFilter(CommonFilterImpl): - def filter(self, *, field: str = None, value: dict = None, query=None) -> SqlAlchemyQuery: - inst_field = self.extract_field(field) - if value: - query = query.filter(inst_field.between(datetime.fromtimestamp(int(value.get('start')) / 1000), - datetime.fromtimestamp(int(value.get('end')) / 1000))) - return query +class _CommonFilterImplMeta(ABCMeta): + def __call__(cls, *args, **kwargs): + warn("CommonFilterImpl is deprecated, use CanonicalFilter instead.", + DeprecationWarning, stacklevel=2) + return super().__call__(*args, **kwargs) -class StringStartsWithFilter(CommonFilterImpl): +class CommonFilterImpl(CanonicalFilter, metaclass=_CommonFilterImplMeta): + """Deprecated alias for CanonicalFilter, kept for one release.""" - def filter(self, *, field: str = None, value: dict = None, query=None) -> SqlAlchemyQuery: - inst_field = self.extract_field(field) - if value: - query = query.filter(inst_field.startswith(value.get("search"))) - return query +class EqualityFilter(CanonicalFilter): + op = Op.EQ -class StringEndsWithFilter(CommonFilterImpl): - def filter(self, *, field: str = None, value: dict = None, query=None) -> SqlAlchemyQuery: - inst_field = self.extract_field(field) - if value: - query = query.filter(inst_field.endswith(value.get("search"))) - return query +class InEqualityFilter(CanonicalFilter): + op = Op.NEQ -class StringContainsFilter(CommonFilterImpl): +class InDataFilter(CanonicalFilter): + op = Op.IN - def filter(self, *, field: str = None, value: dict = None, query=None) -> SqlAlchemyQuery: - inst_field = self.extract_field(field) - if value: - query = query.filter(inst_field.contains(value.get("search"))) - return query +class BetweenUnixMilliSecDateFilter(CanonicalFilter): + op = Op.RANGE -class StringLikeFilter(CommonFilterImpl): + def coerce(self, value: dict) -> dict: + return { + "start": datetime.fromtimestamp(int(value.get('start')) / 1000), + "end": datetime.fromtimestamp(int(value.get('end')) / 1000), + } - def filter(self, *, field: str = None, value: dict = None, query=None) -> SqlAlchemyQuery: - inst_field = self.extract_field(field) - if value: - query = query.filter(inst_field.like(value.get("search"))) - return query +class StringStartsWithFilter(CanonicalFilter): + op = Op.STARTS_WITH -class DataGreaterThanFilter(CommonFilterImpl): - def filter(self, *, field: str = None, value: dict = None, query=None) -> SqlAlchemyQuery: - inst_field = self.extract_field(field) - if value: - query = query.filter(inst_field > value.get("search")) - return query +class StringEndsWithFilter(CanonicalFilter): + op = Op.ENDS_WITH -class DataGreaterThanEqualToFilter(CommonFilterImpl): +class StringContainsFilter(CanonicalFilter): + op = Op.CONTAINS - def filter(self, *, field: str = None, value: dict = None, query=None) -> SqlAlchemyQuery: - inst_field = self.extract_field(field) - if value: - query = query.filter(inst_field >= value.get("search")) - return query +class StringLikeFilter(CanonicalFilter): + op = Op.LIKE -class DataLessThanFilter(CommonFilterImpl): - def filter(self, *, field: str = None, value: dict = None, query=None) -> SqlAlchemyQuery: - inst_field = self.extract_field(field) - if value: - query = query.filter(inst_field < value.get("search")) - return query +class DataGreaterThanFilter(CanonicalFilter): + op = Op.GT -class DataLessThanEqualToFilter(CommonFilterImpl): +class DataGreaterThanEqualToFilter(CanonicalFilter): + op = Op.GTE - def filter(self, *, field: str = None, value: dict = None, query=None) -> SqlAlchemyQuery: - inst_field = self.extract_field(field) - if value: - query = query.filter(inst_field <= value.get("search")) - return query +class DataLessThanFilter(CanonicalFilter): + op = Op.LT -class DataGropByElementFilter(CommonFilterImpl): - def filter(self, *, field: str = None, value: dict = None, query=None) -> SqlAlchemyQuery: - inst_field = self.extract_field(field) - query = query.group_by(inst_field) - return query +class DataLessThanEqualToFilter(CanonicalFilter): + op = Op.LTE -class DataDistinctByElementFilter(CommonFilterImpl): +class DataGropByElementFilter(CanonicalFilter): + op = Op.GROUP_BY - def filter(self, *, field: str = None, value: dict = None, query=None) -> SqlAlchemyQuery: - inst_field = self.extract_field(field) - query = query.distinct(inst_field) - return query +class DataDistinctByElementFilter(CanonicalFilter): + op = Op.DISTINCT -class HasFieldValue(CommonFilterImpl): - def filter(self, *, field: str = None, value: dict = None, query=None) -> SqlAlchemyQuery: - inst_field = self.extract_field(field) - if value.get("search"): - query = query.filter(inst_field.is_not(None)) - else: - query = query.filter(inst_field.is_(None)) - return query +class HasFieldValue(CanonicalFilter): + # op is resolved dynamically from the value flag, so this overrides + # filter() entirely rather than declaring a fixed canonical op. + op = Op.NOT_NULL + def filter(self, *, field: str = None, value: dict = None, context: QueryContext = None) -> QueryContext: + op = Op.NOT_NULL if value.get("search") else Op.IS_NULL + return context.where(field=self.extract_field(field), op=op, value=None) -class MySqlNativeDateFormateRangeFilter(CommonFilterImpl): - def filter(self, *, field: str = None, value: dict = None, query=None) -> SqlAlchemyQuery: - inst_field = self.extract_field(field) - if value: - query = query.filter(inst_field.between(value.get("start"), value.get("end"))) - return query +class MySqlNativeDateFormateRangeFilter(CanonicalFilter): + op = Op.RANGE diff --git a/fastapi_listing/interceptors/individual_sorter_interceptor.py b/fastapi_listing/interceptors/individual_sorter_interceptor.py index cf7dc00..95c6795 100644 --- a/fastapi_listing/interceptors/individual_sorter_interceptor.py +++ b/fastapi_listing/interceptors/individual_sorter_interceptor.py @@ -1,25 +1,31 @@ -from typing import List, Dict - -from fastapi_listing.abstracts import AbstractSorterInterceptor -from fastapi_listing.sorter import SortingOrderStrategy -from fastapi_listing.ctyping import SqlAlchemyQuery - - -class IndiSorterInterceptor(AbstractSorterInterceptor): - """ - Singleton Sorter mechanic. - # ideally sorting should only happen on one field multi field sorting puts - # unwanted strain on table when the size is big and not really popular - # among various clients. Still leaving room for extension won't hurt - # by default even if client is sending multiple sorting params we prioritize - # the latest one which is last column that client requested to sort on. - # if user want they can implement their own asc or dsc sorting order strategy and - # decide how they really want to apply sorting params maybe all maybe none or maybe - # conditional sorting where if one param is applied then don't apply another specific one, etc. - """ - - def apply(self, *, query: SqlAlchemyQuery = None, strategy: SortingOrderStrategy = None, - sorting_params: List[Dict[str, str]] = None, extra_context: dict = None) -> SqlAlchemyQuery: - latest = sorting_params[-1] - query = strategy.sort(query=query, value=latest, extra_context=extra_context) - return query +from typing import List, Dict + +from fastapi_listing.abstracts import AbstractSorterInterceptor +from fastapi_listing.errors import guard_legacy_signature +from fastapi_listing.sorter import SortingOrderStrategy +from fastapi_listing.context import QueryContext + + +class IndiSorterInterceptor(AbstractSorterInterceptor): + """ + Singleton Sorter mechanic. + # ideally sorting should only happen on one field multi field sorting puts + # unwanted strain on table when the size is big and not really popular + # among various clients. Still leaving room for extension won't hurt + # by default even if client is sending multiple sorting params we prioritize + # the latest one which is last column that client requested to sort on. + # if user want they can implement their own asc or dsc sorting order strategy and + # decide how they really want to apply sorting params maybe all maybe none or maybe + # conditional sorting where if one param is applied then don't apply another specific one, etc. + """ + + def apply(self, *, context: QueryContext = None, strategy: SortingOrderStrategy = None, + sorting_params: List[Dict[str, str]] = None, extra_context: dict = None) -> QueryContext: + latest = sorting_params[-1] + guard_legacy_signature( + strategy.sort, legacy_kwarg="query", new_kwarg="context", + subject=f"Custom sorting strategy {type(strategy).__name__!r}", + fix="Rename its 'query' parameter to 'context'; delegate to context.order_by(...) " + "instead of calling .order_by() directly.") + context = strategy.sort(context=context, value=latest, extra_context=extra_context) + return context diff --git a/fastapi_listing/interceptors/iterative_filter_interceptor.py b/fastapi_listing/interceptors/iterative_filter_interceptor.py index 4561c7d..39c3833 100644 --- a/fastapi_listing/interceptors/iterative_filter_interceptor.py +++ b/fastapi_listing/interceptors/iterative_filter_interceptor.py @@ -1,35 +1,42 @@ -from typing import List, Dict, Optional - -from fastapi_listing.abstracts import AbstractFilterInterceptor -from fastapi_listing.factory import filter_factory -from fastapi_listing.filters.generic_filters import CommonFilterImpl -from fastapi_listing.ctyping import SqlAlchemyQuery, FastapiRequest - - -class IterativeFilterInterceptor(AbstractFilterInterceptor): - """ - Iterative Filter Applicator. - Applies all client site filter in iterative manner. - one by one call is made to registered filters and each filterd query is returned. - - User can write their own applicator if they don't want iterative applicator - or have more complex way to apply filter like - if one filter is applied, then don't apply the other one vice versa. - to give a real world example - if user has applied city, pincode, region filter then - pincode is the most atomic unit here region and city filters are just extra burden on query and db as well. - one can tackle this situation by having a mechanic which will check if specific filter is applied - with other relative filters then don't apply other relative filters... - """ - - def apply(self, *, query: SqlAlchemyQuery = None, filter_params: List[Dict[str, str]], dao=None, - request: Optional[FastapiRequest] = None, extra_context: dict = None) -> SqlAlchemyQuery: - for applied_filter in filter_params: - filter_obj: CommonFilterImpl = filter_factory.create(applied_filter.get("field"), - dao=dao, - request=request, - extra_context=extra_context) - query = filter_obj.filter(field=applied_filter.get("field"), - value=applied_filter.get("value"), - query=query) - return query +from typing import List, Dict, Optional + +from fastapi_listing.abstracts import AbstractFilterInterceptor +from fastapi_listing.errors import guard_legacy_signature +from fastapi_listing.factory import filter_factory +from fastapi_listing.filters.generic_filters import CanonicalFilter +from fastapi_listing.context import QueryContext +from fastapi_listing.ctyping import FastapiRequest + + +class IterativeFilterInterceptor(AbstractFilterInterceptor): + """ + Iterative Filter Applicator. + Applies all client site filter in iterative manner. + one by one call is made to registered filters and each filterd query is returned. + + User can write their own applicator if they don't want iterative applicator + or have more complex way to apply filter like + if one filter is applied, then don't apply the other one vice versa. + to give a real world example + if user has applied city, pincode, region filter then + pincode is the most atomic unit here region and city filters are just extra burden on query and db as well. + one can tackle this situation by having a mechanic which will check if specific filter is applied + with other relative filters then don't apply other relative filters... + """ + + def apply(self, *, context: QueryContext = None, filter_params: List[Dict[str, str]], dao=None, + request: Optional[FastapiRequest] = None, extra_context: dict = None) -> QueryContext: + for applied_filter in filter_params: + filter_obj: CanonicalFilter = filter_factory.create(applied_filter.get("field"), + dao=dao, + request=request, + extra_context=extra_context) + guard_legacy_signature( + filter_obj.filter, legacy_kwarg="query", new_kwarg="context", + subject=f"Custom filter {type(filter_obj).__name__!r}", + fix="Rename its 'query' parameter to 'context'; use context.native/" + "context.with_native(...) for raw SQLAlchemy access.") + context = filter_obj.filter(field=applied_filter.get("field"), + value=applied_filter.get("value"), + context=context) + return context diff --git a/fastapi_listing/loader.py b/fastapi_listing/loader.py index da5d505..ed705f8 100644 --- a/fastapi_listing/loader.py +++ b/fastapi_listing/loader.py @@ -8,6 +8,7 @@ from fastapi_listing.service import ListingService from fastapi_listing.factory import filter_factory, _generic_factory, strategy_factory, interceptor_factory from fastapi_listing.errors import MissingExpectedAttribute +from fastapi_listing.abstracts import DaoAbstract from fastapi_listing.dao import GenericDao @@ -35,8 +36,8 @@ def _validate_dao_attribute(cls: ListingService): if not inspect.isclass(cls.default_dao): raise ValueError("Invalid Dao reference Injected!") - if not issubclass(cls.default_dao, GenericDao): # type: ignore - raise TypeError("Invalid Dao Type! Should Be type of GenericDao") + if not issubclass(cls.default_dao, DaoAbstract): # type: ignore + raise TypeError("Invalid Dao Type! Should Be type of DaoAbstract") return True diff --git a/fastapi_listing/middlewares.py b/fastapi_listing/middlewares.py index 784ee74..7ec7eaa 100644 --- a/fastapi_listing/middlewares.py +++ b/fastapi_listing/middlewares.py @@ -5,25 +5,25 @@ from contextlib import contextmanager from warnings import warn -from sqlalchemy.orm import Session from starlette.middleware.base import BaseHTTPMiddleware, RequestResponseEndpoint from starlette.requests import Request from starlette.responses import Response from starlette.types import ASGIApp from fastapi_listing.errors import MissingSessionError +from fastapi_listing.ctyping import SqlAlchemySession -_session: ContextVar[Optional[Session]] = ContextVar("_session", default=None) +_session: "ContextVar[Optional[SqlAlchemySession]]" = ContextVar("_session", default=None) -_replica_session: ContextVar[Optional[Session]] = ContextVar("_replica_session", default=None) +_replica_session: "ContextVar[Optional[SqlAlchemySession]]" = ContextVar("_replica_session", default=None) class DaoSessionBinderMiddleware(BaseHTTPMiddleware): def __init__( self, app: ASGIApp, *, - master: Callable[[], Session] = None, - replica: Callable[[], Session] = None, + master: Callable[[], SqlAlchemySession] = None, + replica: Callable[[], SqlAlchemySession] = None, session_close_implicit: bool = False, suppress_warnings: bool = False, ): @@ -43,14 +43,14 @@ async def dispatch(self, request: Request, call_next: RequestResponseEndpoint) - class SessionProviderMeta(type): @property - def read_session(cls) -> Session: + def read_session(cls) -> SqlAlchemySession: read_replica_session = _replica_session.get() if read_replica_session is None: raise MissingSessionError return read_replica_session @property - def session(cls) -> Session: + def session(cls) -> SqlAlchemySession: master_session = _session.get() if master_session is None: raise MissingSessionError @@ -62,7 +62,7 @@ class SessionProvider(metaclass=SessionProviderMeta): @contextmanager -def manager(read_ses: Callable[[], Session], master: Callable[[], Session], implicit_close: bool, +def manager(read_ses: Callable[[], SqlAlchemySession], master: Callable[[], SqlAlchemySession], implicit_close: bool, suppress_warnings: bool): global _session global _replica_session diff --git a/fastapi_listing/ops.py b/fastapi_listing/ops.py new file mode 100644 index 0000000..0cf9d75 --- /dev/null +++ b/fastapi_listing/ops.py @@ -0,0 +1,30 @@ +"""Canonical, backend-agnostic filter operations. + +Every backend's QueryContext interprets the same Op values in whatever way makes +sense for it (a SQLAlchemy `.filter()` chain, a raw SQL WHERE fragment, a Mongo +filter dict, ...). Filter classes in `fastapi_listing.filters` only ever declare +*which* Op they represent - they never touch a specific backend's syntax. +""" + +from enum import Enum + +__all__ = ["Op"] + + +class Op(str, Enum): + EQ = "eq" + NEQ = "neq" + IN = "in" + RANGE = "range" + LIKE = "like" + STARTS_WITH = "starts_with" + ENDS_WITH = "ends_with" + CONTAINS = "contains" + GT = "gt" + GTE = "gte" + LT = "lt" + LTE = "lte" + IS_NULL = "is_null" + NOT_NULL = "not_null" + GROUP_BY = "group_by" + DISTINCT = "distinct" diff --git a/fastapi_listing/paginator/page_builder.py b/fastapi_listing/paginator/page_builder.py index 4bd5531..047ad95 100644 --- a/fastapi_listing/paginator/page_builder.py +++ b/fastapi_listing/paginator/page_builder.py @@ -1,7 +1,8 @@ -from typing import Optional, Union +from typing import Optional from fastapi_listing.abstracts import AbsPaginatingStrategy -from fastapi_listing.ctyping import SqlAlchemyQuery, FastapiRequest, Page, BasePage, PageWithoutCount +from fastapi_listing.context import QueryContext +from fastapi_listing.ctyping import FastapiRequest, Page, BasePage, PageWithoutCount from fastapi_listing.errors import ListingPaginatorError @@ -22,7 +23,7 @@ def __init__(self, request: Optional[FastapiRequest] = None, fire_count_qry: boo self.extra_context = None self.fire_count_qry = fire_count_qry - def get_count(self, query: SqlAlchemyQuery) -> int: + def get_count(self, context: QueryContext) -> int: """ Override this method to return a dummy count or generate count in more optimized manner. User may want this to avoid slow count(*) query or double query or have page setup that doesn't require @@ -30,7 +31,7 @@ def get_count(self, query: SqlAlchemyQuery) -> int: Overall special checks needs to be setup. like returning a massive dummy count and then depending upon empty main_data avoiding trip to next page etc. """ - return query.count() + return context.count() def is_next_page_exists(self) -> bool: """expression results in bool val if count query allowed else None""" @@ -64,7 +65,7 @@ def set_extra_context(self, extra_context): def set_count(self, count: int): self.count = count - def paginate(self, query: SqlAlchemyQuery, pagination_params: dict, extra_context: dict) -> BasePage: + def paginate(self, context: QueryContext, pagination_params: dict, extra_context: dict) -> BasePage: """Return paginated response""" page_num = pagination_params.get('page') page_size = pagination_params.get('pageSize') @@ -76,18 +77,18 @@ def paginate(self, query: SqlAlchemyQuery, pagination_params: dict, extra_contex self.set_page_num(page_num) self.set_page_size(page_size) self.set_extra_context(extra_context) - return self.page(query) + return self.page(context) - def page(self, query: SqlAlchemyQuery) -> BasePage: + def page(self, context: QueryContext) -> BasePage: """Return a Page or BasePage for given 1-based page number.""" if self.fire_count_qry: - self.set_count(self.get_count(query)) + self.set_count(self.get_count(context)) has_next: bool = self.is_next_page_exists() - query = self._slice_query(query) - return self._get_page(has_next, query) + context = self._slice_query(context) + return self._get_page(has_next, context) else: - query = self._slice_query(query) - return self._get_page_without_count(query) + context = self._slice_query(context) + return self._get_page_without_count(context) def _get_page(self, *args, **kwargs) -> Page: """ @@ -95,19 +96,20 @@ def _get_page(self, *args, **kwargs) -> Page: this hook can be used by subclasses if you want to replace Page datastructure with your custom structure extending BasePage. """ - has_next, query = args + has_next, context = args total_count = self.count + rows = self.postprocess(context.fetch(), self.extra_context) return Page( hasNext=has_next, totalCount=total_count, currentPageSize=self.page_size, currentPageNumber=self.page_num, - data=query.all()) + data=rows) def _get_page_without_count(self, *args, **kwargs) -> PageWithoutCount: """Get Page without total count for avoiding slow count query""" - query = args[0] - data = query.all() + context = args[0] + data = self.postprocess(context.fetch(), self.extra_context) self.set_count(len(data)) has_next = self.is_next_page_exists() return PageWithoutCount( @@ -118,7 +120,7 @@ def _get_page_without_count(self, *args, **kwargs) -> PageWithoutCount: ) - def _slice_query(self, query: SqlAlchemyQuery) -> SqlAlchemyQuery: + def _slice_query(self, context: QueryContext) -> QueryContext: """ Return sliced query. @@ -127,8 +129,8 @@ def _slice_query(self, query: SqlAlchemyQuery) -> SqlAlchemyQuery: or using a more advanced offset technique. """ if self.fire_count_qry: - return query.limit(self.page_size).offset(max(self.page_num - 1, 0) * self.page_size) + return context.limit_offset(limit=self.page_size, offset=max(self.page_num - 1, 0) * self.page_size) else: # get +1 than page size to see if next page exists # a hotfix to avoid total count to determine next page existence - return query.limit(self.page_size + 1).offset(max(self.page_num - 1, 0) * self.page_size) + return context.limit_offset(limit=self.page_size + 1, offset=max(self.page_num - 1, 0) * self.page_size) diff --git a/fastapi_listing/service/_core_listing_service.py b/fastapi_listing/service/_core_listing_service.py index aa3b37c..b7f700f 100644 --- a/fastapi_listing/service/_core_listing_service.py +++ b/fastapi_listing/service/_core_listing_service.py @@ -2,11 +2,11 @@ from warnings import warn from fastapi import Request -from sqlalchemy.orm import Query +from fastapi_listing.context import QueryContext from fastapi_listing.dao.generic_dao import GenericDao from fastapi_listing.errors import FastapiListingRequestSemanticApiException, \ - NotRegisteredApiException, FastAPIListingWarning + NotRegisteredApiException, FastAPIListingWarning, FastapiListingMigrationError from fastapi_listing.factory import interceptor_factory, strategy_factory from fastapi_listing.interface.listing_meta_info import ListingMetaInfo from fastapi_listing.ctyping import BasePage @@ -58,7 +58,7 @@ def _replace_aliases(mapper: Dict[str, str], req_params: List[Dict[str, str]]) - raise ValueError("invalid field mapper") return req_params - def _apply_sorting(self, query: Query, listing_meta_info: ListingMetaInfo) -> Query: + def _apply_sorting(self, query: QueryContext, listing_meta_info: ListingMetaInfo) -> QueryContext: try: sorting_params: List[dict] = listing_meta_info.feature_params_adapter.get("sort") except Exception: @@ -76,14 +76,14 @@ def _apply_sorting(self, query: Query, listing_meta_info: ListingMetaInfo) -> Qu def launch_mechanics(qry): mecha: str = listing_meta_info.sorter_mechanic mecha_obj = interceptor_factory.create(mecha) - qry = mecha_obj.apply(query=qry, strategy=listing_meta_info.sorting_strategy, + qry = mecha_obj.apply(context=qry, strategy=listing_meta_info.sorting_strategy, sorting_params=sorting_params, extra_context=listing_meta_info.extra_context) return qry query = launch_mechanics(query) return query - def _apply_filters(self, query: Query, listing_meta_info: ListingMetaInfo) -> Query: + def _apply_filters(self, query: QueryContext, listing_meta_info: ListingMetaInfo) -> QueryContext: try: fltrs: List[dict] = listing_meta_info.feature_params_adapter.get("filter") except Exception: @@ -98,14 +98,14 @@ def _apply_filters(self, query: Query, listing_meta_info: ListingMetaInfo) -> Qu def launch_mechanics(qry): mecha_obj = interceptor_factory.create(listing_meta_info.filter_mechanic) - qry = mecha_obj.apply(query=qry, filter_params=fltrs, dao=self.dao, + qry = mecha_obj.apply(context=qry, filter_params=fltrs, dao=self.dao, request=self.request, extra_context=listing_meta_info.extra_context) return qry query = launch_mechanics(query) return query - def _paginate(self, query: Query, listing_meta_info: ListingMetaInfo) -> BasePage: + def _paginate(self, query: QueryContext, listing_meta_info: ListingMetaInfo) -> BasePage: try: raw_params: List[dict] = listing_meta_info.feature_params_adapter.get("pagination") page_params = raw_params if raw_params else {"page": 1, "pageSize": listing_meta_info.default_page_size} @@ -127,18 +127,22 @@ def _paginate(self, query: Query, listing_meta_info: ListingMetaInfo) -> BasePag extra_context=listing_meta_info.extra_context) return page - def _prepare_query(self, listing_meta_info: ListingMetaInfo) -> Query: - base_query: Query = listing_meta_info.query_strategy.get_query(request=self.request, - dao=self.dao, - extra_context=listing_meta_info.extra_context) + def _prepare_query(self, listing_meta_info: ListingMetaInfo) -> QueryContext: + base_query: QueryContext = listing_meta_info.query_strategy.get_query( + request=self.request, dao=self.dao, extra_context=listing_meta_info.extra_context) if base_query is None or not base_query: - raise ValueError("query strategy returned nothing Query object is expected!") - fltr_query: Query = self._apply_filters(base_query, + raise ValueError("query strategy returned nothing QueryContext object is expected!") + if not isinstance(base_query, QueryContext): + raise FastapiListingMigrationError( + f"Custom query strategy {type(listing_meta_info.query_strategy).__name__!r} " + f"returned a raw {type(base_query).__name__!r} instead of a QueryContext. " + "Wrap it, e.g. return SqlAlchemyQueryContext().") + fltr_query: QueryContext = self._apply_filters(base_query, listing_meta_info) if listing_meta_info.extra_context.get(Options.abort_sorting.value): return fltr_query - srtd_query: Query = self._apply_sorting(fltr_query, listing_meta_info) + srtd_query: QueryContext = self._apply_sorting(fltr_query, listing_meta_info) return srtd_query @staticmethod @@ -182,6 +186,6 @@ def get_response(self, listing_meta_data: ListingMetaData) -> BasePage: custom_fields=self.custom_fields ) listing_meta_info = self._build_from_meta_data(listing_meta_data) - fnl_query: Query = self._prepare_query(listing_meta_info) + fnl_query: QueryContext = self._prepare_query(listing_meta_info) response: BasePage = self._paginate(fnl_query, listing_meta_info) return response diff --git a/fastapi_listing/sorter/page_sorter.py b/fastapi_listing/sorter/page_sorter.py index 8218648..d91908b 100644 --- a/fastapi_listing/sorter/page_sorter.py +++ b/fastapi_listing/sorter/page_sorter.py @@ -1,36 +1,32 @@ from typing import Dict + from fastapi_listing.abstracts import AbsSortingStrategy -from fastapi_listing.ctyping import SqlAlchemyModel, FastapiRequest, SqlAlchemyQuery, AnySqlAlchemyColumn +from fastapi_listing.context import QueryContext +from fastapi_listing.ctyping import FastapiRequest from fastapi_listing.factory import _generic_factory class SortingOrderStrategy(AbsSortingStrategy): + """ + Backend-agnostic by construction: it only ever resolves a field name to + whatever object the backend's model/registered extractor gives back + (a SQLAlchemy InstrumentedAttribute, a plain ClickHouse column-name + string, ...) and hands both the field and the "asc"/"dsc" direction to + the QueryContext - which is the only place that knows how to turn that + into native syntax. + """ - def __init__(self, model: SqlAlchemyModel = None, request: FastapiRequest = None): + def __init__(self, model=None, request: FastapiRequest = None): self.model = model self.request = request - @staticmethod - def sort_asc_util(query: SqlAlchemyQuery, inst_field: AnySqlAlchemyColumn) -> SqlAlchemyQuery: - query = query.order_by(inst_field.asc()) - return query - - @staticmethod - def sort_dsc_util(query: SqlAlchemyQuery, inst_field: AnySqlAlchemyColumn) -> SqlAlchemyQuery: - query = query.order_by(inst_field.desc()) - return query - - def sort(self, *, query: SqlAlchemyQuery = None, value: Dict[str, str] = None, - extra_context: dict = None) -> SqlAlchemyQuery: + def sort(self, *, context: QueryContext = None, value: Dict[str, str] = None, + extra_context: dict = None) -> QueryContext: assert value["type"] in ["asc", "dsc"], "invalid sorting style!" - inst_field: AnySqlAlchemyColumn = self.validate_srt_field(self.model, value["field"]) - if value["type"] == "asc": - query = self.sort_asc_util(query, inst_field) - else: - query = self.sort_dsc_util(query, inst_field) - return query + inst_field = self.validate_srt_field(self.model, value["field"]) + return context.order_by(field=inst_field, direction=value["type"]) - def validate_srt_field(self, model: SqlAlchemyModel, sort_field: str): + def validate_srt_field(self, model, sort_field: str): field = sort_field.split(".")[-1] if sort_field in _generic_factory.object_creation_collector: inst_field = _generic_factory.create(sort_field, field) diff --git a/fastapi_listing/strategies/query_strategy.py b/fastapi_listing/strategies/query_strategy.py index 5089fd6..216f061 100644 --- a/fastapi_listing/strategies/query_strategy.py +++ b/fastapi_listing/strategies/query_strategy.py @@ -1,32 +1,33 @@ -from typing import Optional - -from fastapi_listing.abstracts import AbsQueryStrategy -from fastapi_listing.dao import GenericDao -from fastapi import Query, Request - - -class QueryStrategy(AbsQueryStrategy): - """Default query strategy class. Generates a simple query with requested fields from same model.""" - - def get_inst_attr_to_read(self, custom_fields: bool, field_list: list, dao: GenericDao): - inst_fields = [] - - if custom_fields: - # ("BYPASS CUSTOM PYDANTIC FIELDS ALLOWED.") - # when serializer contains fields that get filled via validators or at runtime - # or fields that get generated from model fields. - for field in field_list: - try: - inst_fields.append(getattr(dao.model, field)) - except AttributeError: - pass - else: - inst_fields = [getattr(dao.model, field) for field in field_list] - return inst_fields - - def get_query(self, *, request: Optional[Request] = None, dao: GenericDao = None, - extra_context: dict = None) -> Query: - inst_fields = self.get_inst_attr_to_read(extra_context.get("custom_fields"), extra_context.get("field_list"), - dao) - query = dao.get_default_read(inst_fields) - return query +from typing import Optional + +from fastapi_listing.abstracts import AbsQueryStrategy +from fastapi_listing.context import QueryContext +from fastapi_listing.dao import GenericDao +from fastapi import Request + + +class QueryStrategy(AbsQueryStrategy): + """Default query strategy class. Generates a simple query with requested fields from same model.""" + + def get_inst_attr_to_read(self, custom_fields: bool, field_list: list, dao: GenericDao): + inst_fields = [] + + if custom_fields: + # ("BYPASS CUSTOM PYDANTIC FIELDS ALLOWED.") + # when serializer contains fields that get filled via validators or at runtime + # or fields that get generated from model fields. + for field in field_list: + try: + inst_fields.append(getattr(dao.model, field)) + except AttributeError: + pass + else: + inst_fields = [getattr(dao.model, field) for field in field_list] + return inst_fields + + def get_query(self, *, request: Optional[Request] = None, dao: GenericDao = None, + extra_context: dict = None) -> QueryContext: + inst_fields = self.get_inst_attr_to_read(extra_context.get("custom_fields"), extra_context.get("field_list"), + dao) + context = dao.get_default_read(inst_fields) + return context diff --git a/setup.py b/setup.py index 4564b6a..ccb3816 100644 --- a/setup.py +++ b/setup.py @@ -44,8 +44,11 @@ def get_long_description(): "Programming Language :: Python :: 3.11", ], python_requires=">=3.7", - keywords=["starlette", "fastapi", "pydantic", "sqlalchemy"], + keywords=["starlette", "fastapi", "pydantic", "sqlalchemy", "clickhouse"], extras_require={ + "clickhouse": [ + "clickhouse-driver>=0.2.6", + ], "test": [ "requests", "pytest>=6.2.4", diff --git a/tests/clickhouse_listing_setup.py b/tests/clickhouse_listing_setup.py new file mode 100644 index 0000000..ac8b1a9 --- /dev/null +++ b/tests/clickhouse_listing_setup.py @@ -0,0 +1,70 @@ +"""Backend-agnosticism regression harness: a fully in-process fake ClickHouse +client (no network, no real clickhouse-driver dependency) driving the exact +same, unmodified EqualityFilter/QueryStrategy/SortingOrderStrategy/ +PaginationStrategy/IterativeFilterInterceptor classes the SQLAlchemy tests use. +If this passes, the QueryContext abstraction genuinely generalizes rather than +merely working for SQLAlchemy by accident. +""" + +import re + +from fastapi_listing.dao import ClickHouseDao +from fastapi_listing.factory import filter_factory + + +ROWS = [ + {"emp_no": 1, "first_name": "Sachin", "gender": "M"}, + {"emp_no": 2, "first_name": "Rahul", "gender": "M"}, + {"emp_no": 3, "first_name": "Anjali", "gender": "F"}, + {"emp_no": 4, "first_name": "Priya", "gender": "F"}, +] + + +class FakeClickHouseClient: + """Hand-rolled fake mimicking clickhouse_driver.Client.execute(). Asserts + the real ClickHouseQueryContext-generated SQL/params round-trip correctly + against an in-memory list of dict rows, standing in for a real server.""" + + def execute(self, sql, params=None, with_column_types=False): + params = params or {} + rows = list(ROWS) + + # Match "`gender` = %()s" rather than hardcoding a + # specific key name - the bind-key naming scheme is an implementation + # detail this fixture shouldn't be coupled to. + gender_eq = re.search(r"`gender` = %\((\w+)\)s", sql) + if gender_eq: + rows = [r for r in rows if r["gender"] == params[gender_eq.group(1)]] + + if sql.strip().upper().startswith("SELECT COUNT(*)"): + return [(len(rows),)] + + if "ORDER BY `emp_no` DESC" in sql: + rows = sorted(rows, key=lambda r: r["emp_no"], reverse=True) + elif "ORDER BY `emp_no` ASC" in sql: + rows = sorted(rows, key=lambda r: r["emp_no"]) + + result = [(r["emp_no"], r["first_name"], r["gender"]) for r in rows] + if with_column_types: + return result, [("emp_no", "Int32"), ("first_name", "String"), ("gender", "String")] + return result + + +class FakeEmployee: + __table__ = "employees" + emp_no = "emp_no" + first_name = "first_name" + gender = "gender" + + +class FakeEmployeeDao(ClickHouseDao): + name = "employee_ch_test" + model = FakeEmployee + + +def register(): + from fastapi_listing.filters import generic_filters + + filter_mapper = {"gdr_ch": ("FakeEmployee.gender", generic_filters.EqualityFilter)} + filter_factory.register_filter_mapper(filter_mapper) + return filter_mapper diff --git a/tests/service_setup.py b/tests/service_setup.py index 35d5dce..c819f28 100644 --- a/tests/service_setup.py +++ b/tests/service_setup.py @@ -2,7 +2,9 @@ from fastapi_listing.filters import generic_filters from fastapi_listing.factory import strategy_factory from fastapi_listing.strategies import QueryStrategy -from fastapi_listing.ctyping import FastapiRequest, SqlAlchemyQuery +from fastapi_listing.context import QueryContext +from fastapi_listing.context.sqlalchemy import SqlAlchemyQueryContext +from fastapi_listing.ctyping import FastapiRequest from fastapi_listing.dao import dao_factory from fastapi_listing import loader @@ -15,15 +17,15 @@ class DepartmentEmployeesQueryStrategy(QueryStrategy): def get_query(self, *, request: FastapiRequest = None, dao: DeptEmpDao = None, - extra_context: dict = None) -> SqlAlchemyQuery: - return dao.get_emp_dept_mapping_base_query() + extra_context: dict = None) -> QueryContext: + return SqlAlchemyQueryContext(dao.get_emp_dept_mapping_base_query()) class EmployeesQueryStrategy(QueryStrategy): def get_query(self, *, request: FastapiRequest = None, dao: EmployeeDao = None, - extra_context: dict = None) -> SqlAlchemyQuery: - return dao.get_employees_with_designations() + extra_context: dict = None) -> QueryContext: + return SqlAlchemyQueryContext(dao.get_employees_with_designations()) strategy_factory.register_strategy("dept_emp_mapping_query", DepartmentEmployeesQueryStrategy) @@ -71,15 +73,16 @@ def get_listing(self): return resp -class FullNameFilter(generic_filters.CommonFilterImpl): +class FullNameFilter(generic_filters.CanonicalFilter): - def filter(self, *, field: str = None, value: dict = None, query=None) -> SqlAlchemyQuery: + def filter(self, *, field: str = None, value: dict = None, context: QueryContext = None) -> QueryContext: # field is not necessary here as this is a custom filter and user have full control over its implementation if value: emp_dao: EmployeeDao = dao_factory.create("employee", replica=True) emp_ids: list[int] = emp_dao.get_emp_ids_contain_full_name(value.get("search")) - query = query.filter(self.dao.model.emp_no.in_(emp_ids)) # noqa - return query + native = context.native.filter(self.dao.model.emp_no.in_(emp_ids)) # noqa + context = context.with_native(native) + return context @loader.register() diff --git a/tests/sqlalchemy_listing_setup.py b/tests/sqlalchemy_listing_setup.py new file mode 100644 index 0000000..ce15a6f --- /dev/null +++ b/tests/sqlalchemy_listing_setup.py @@ -0,0 +1,47 @@ +"""In-memory SQLite fixture for QueryContext-layer tests that don't need the +real MySQL "employees" CI database - just a real SQLAlchemy Query underneath +SqlAlchemyQueryContext, so the op-dispatch logic runs against genuine SQLAlchemy +expressions rather than being mocked away. +""" + +from datetime import date + +from sqlalchemy import create_engine, Column, Integer, String, Date +from sqlalchemy.orm import declarative_base, Session + +from fastapi_listing.dao import GenericDao + +Base = declarative_base() + + +class Employee(Base): + __tablename__ = "context_test_employees" + + emp_no = Column(Integer, primary_key=True) + first_name = Column(String(50)) + last_name = Column(String(50)) + gender = Column(String(1)) + hire_date = Column(Date) + + +ROWS = [ + dict(emp_no=1, first_name="Sachin", last_name="Kumar", gender="M", hire_date=date(1990, 1, 1)), + dict(emp_no=2, first_name="Rahul", last_name="Sharma", gender="M", hire_date=date(1995, 6, 15)), + dict(emp_no=3, first_name="Anjali", last_name="Verma", gender="F", hire_date=date(2000, 3, 20)), + dict(emp_no=4, first_name="Priya", last_name="Iyer", gender="F", hire_date=date(2005, 11, 5)), +] + +_engine = create_engine("sqlite:///:memory:") +Base.metadata.create_all(_engine) +_session = Session(bind=_engine) +_session.add_all([Employee(**row) for row in ROWS]) +_session.commit() + + +def session_factory() -> Session: + return _session + + +class EmployeeDao(GenericDao): + name = "sqlite_employee" + model = Employee diff --git a/tests/test_clickhouse_backend.py b/tests/test_clickhouse_backend.py new file mode 100644 index 0000000..864ea56 --- /dev/null +++ b/tests/test_clickhouse_backend.py @@ -0,0 +1,52 @@ +import json +from urllib.parse import quote + +from fastapi_listing import FastapiListing, MetaInfo + +from .clickhouse_listing_setup import FakeEmployeeDao, FakeClickHouseClient, register + + +filter_mapper = register() + + +def get_url_quoted_string(d): + return quote(json.dumps(d)) + + +def test_clickhouse_filter_and_sort_reuse_default_strategies(): + """The same EqualityFilter/QueryStrategy/SortingOrderStrategy/PaginationStrategy/ + IterativeFilterInterceptor classes the SQLAlchemy tests use, driving a + non-ORM raw-SQL backend, unmodified.""" + dao = FakeEmployeeDao(read_db=FakeClickHouseClient()) + + resp = FastapiListing(dao=dao, fields_to_fetch=["emp_no", "first_name", "gender"]).get_response( + MetaInfo(default_srt_on="emp_no", default_srt_ord="asc", filter_mapper=filter_mapper, + filter=get_url_quoted_string([{"field": "gdr_ch", "value": {"search": "F"}}])) + ) + assert resp["totalCount"] == 2 + assert {row["gender"] for row in resp["data"]} == {"F"} + assert [row["emp_no"] for row in resp["data"]] == [3, 4] + + +def test_clickhouse_sorting_descending(): + dao = FakeEmployeeDao(read_db=FakeClickHouseClient()) + + resp = FastapiListing(dao=dao, fields_to_fetch=["emp_no", "first_name", "gender"]).get_response( + MetaInfo(default_srt_on="emp_no", default_srt_ord="dsc") + ) + assert [row["emp_no"] for row in resp["data"]] == [4, 3, 2, 1] + + +def test_clickhouse_context_compiles_parameterized_sql(): + """Values must always flow through the driver's own param binding, never + be string-formatted into the SQL text.""" + from fastapi_listing.context.clickhouse import ClickHouseQueryContext + from fastapi_listing.ops import Op + + ctx = ClickHouseQueryContext(client=FakeClickHouseClient(), table="employees", + columns=["emp_no", "first_name"]) + ctx.where(field="gender", op=Op.EQ, value={"search": "F"}) + sql, params = ctx.compile() + assert "= 'F'" not in sql and '"F"' not in sql + assert params == {"__fl_p1": "F"} + assert "`gender` = %(__fl_p1)s" in sql diff --git a/tests/test_main.py b/tests/test_main.py index 991d7a5..1796067 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -179,7 +179,7 @@ class ABCListing(ListingService): # noqa: F811,F841 } default_dao = TitleDao - assert e.value.args[0] == "Invalid filter mapper semantic ! Expects a subclass of CommonFilterImpl" + assert e.value.args[0] == "Invalid filter mapper semantic ! Expects a subclass of CanonicalFilter" # checking args with pytest.raises(ValueError) as e: diff --git a/tests/test_main_v2.py b/tests/test_main_v2.py index 4b242c8..1fa425b 100644 --- a/tests/test_main_v2.py +++ b/tests/test_main_v2.py @@ -395,7 +395,7 @@ class ErrorProneListingV2(ListingService): # noqa: F811,F841 default_srt_on = "sdfasd" default_dao = ListingService - assert e.value.args[0] == "Invalid Dao Type! Should Be type of GenericDao" + assert e.value.args[0] == "Invalid Dao Type! Should Be type of DaoAbstract" with pytest.raises(ValueError) as e: @loader.register() diff --git a/tests/test_migration_guard.py b/tests/test_migration_guard.py new file mode 100644 index 0000000..67016d3 --- /dev/null +++ b/tests/test_migration_guard.py @@ -0,0 +1,46 @@ +import pytest + +from fastapi_listing import FastapiListing, MetaInfo +from fastapi_listing.errors import FastapiListingMigrationError +from fastapi_listing.factory import filter_factory, strategy_factory +from fastapi_listing.filters import generic_filters +from fastapi_listing.strategies import QueryStrategy + +from .sqlalchemy_listing_setup import EmployeeDao, session_factory + + +def test_legacy_filter_signature_raises_clear_migration_error(): + class LegacyFilter(generic_filters.CanonicalFilter): + # pre-0.4.0 signature: 'query' instead of 'context' + def filter(self, *, field=None, value=None, query=None): + return query + + filter_mapper = {"legacy_flt": ("MigrationGuardEmployee.gender", LegacyFilter)} + filter_factory.register_filter_mapper(filter_mapper) + + dao = EmployeeDao(read_db=session_factory()) + with pytest.raises(FastapiListingMigrationError) as exc: + FastapiListing(dao=dao, fields_to_fetch=["emp_no"]).get_response( + MetaInfo(default_srt_on="emp_no", + filter_mapper=filter_mapper, + filter='%5B%7B%22field%22%3A%20%22legacy_flt%22%2C%20%22value%22%3A%20%7B%22search%22%3A%20%22M%22%7D%7D%5D') + ) + assert "still uses the pre-0.4.0 'query' parameter" in str(exc.value) + assert "LegacyFilter" in str(exc.value) + + +def test_legacy_query_strategy_return_type_raises_clear_migration_error(): + class LegacyQueryStrategy(QueryStrategy): + def get_query(self, *, request=None, dao=None, extra_context: dict = None): + # pre-0.4.0 behaviour: returns a raw SQLAlchemy Query, not a QueryContext + return dao.get_default_read([dao.model.emp_no]).native + + strategy_factory.register_strategy("legacy_query_strategy_test", LegacyQueryStrategy) + + dao = EmployeeDao(read_db=session_factory()) + with pytest.raises(FastapiListingMigrationError) as exc: + FastapiListing(dao=dao, fields_to_fetch=["emp_no"]).get_response( + MetaInfo(default_srt_on="emp_no", query_strategy="legacy_query_strategy_test") + ) + assert "raw" in str(exc.value) + assert "SqlAlchemyQueryContext" in str(exc.value) diff --git a/tests/test_query_context.py b/tests/test_query_context.py new file mode 100644 index 0000000..c7a884a --- /dev/null +++ b/tests/test_query_context.py @@ -0,0 +1,333 @@ +"""Op-dispatch coverage for both QueryContext implementations, the +CommonFilterImpl deprecation alias, and the PaginationStrategy.postprocess +hook - the pieces the smoke-tested-but-uncommitted scripts were covering +during development. +""" + +import warnings + +import pytest + +from fastapi_listing import FastapiListing, MetaInfo +from fastapi_listing.context.clickhouse import ClickHouseQueryContext +from fastapi_listing.context.sqlalchemy import SqlAlchemyQueryContext +from fastapi_listing.dao import GenericDao +from fastapi_listing.filters import generic_filters +from fastapi_listing.ops import Op +from fastapi_listing.paginator import PaginationStrategy +from fastapi_listing.factory import strategy_factory + +from .clickhouse_listing_setup import FakeClickHouseClient +from .sqlalchemy_listing_setup import Employee, EmployeeDao, session_factory + + +# ---------- SqlAlchemyQueryContext: every Op against a real SQLAlchemy Query ---------- + +@pytest.mark.parametrize("op,value,expected_emp_nos", [ + (Op.EQ, {"search": "M"}, {1, 2}), + (Op.NEQ, {"search": "M"}, {3, 4}), + (Op.IN, {"list": ["M"]}, {1, 2}), + (Op.LIKE, {"search": "M"}, {1, 2}), + (Op.STARTS_WITH, {"search": "M"}, {1, 2}), + (Op.ENDS_WITH, {"search": "M"}, {1, 2}), + (Op.CONTAINS, {"search": "M"}, {1, 2}), +]) +def test_sqlalchemy_context_gender_ops(op, value, expected_emp_nos): + session = session_factory() + ctx = SqlAlchemyQueryContext(session.query(Employee)) + ctx.where(field=Employee.gender, op=op, value=value) + assert {row.emp_no for row in ctx.fetch()} == expected_emp_nos + + +def test_sqlalchemy_context_range(): + from datetime import date + session = session_factory() + ctx = SqlAlchemyQueryContext(session.query(Employee)) + ctx.where(field=Employee.hire_date, op=Op.RANGE, + value={"start": date(1990, 1, 1), "end": date(1996, 1, 1)}) + assert {row.emp_no for row in ctx.fetch()} == {1, 2} + + +@pytest.mark.parametrize("op", [Op.GT, Op.GTE, Op.LT, Op.LTE]) +def test_sqlalchemy_context_comparison_ops_run_without_error(op): + session = session_factory() + ctx = SqlAlchemyQueryContext(session.query(Employee)) + ctx.where(field=Employee.emp_no, op=op, value={"search": 2}) + assert isinstance(ctx.fetch(), list) + + +def test_sqlalchemy_context_is_null_and_not_null(): + session = session_factory() + assert len(SqlAlchemyQueryContext(session.query(Employee)).where( + field=Employee.gender, op=Op.NOT_NULL, value=None).fetch()) == 4 + assert len(SqlAlchemyQueryContext(session.query(Employee)).where( + field=Employee.gender, op=Op.IS_NULL, value=None).fetch()) == 0 + + +def test_sqlalchemy_context_group_by_and_distinct_run_without_error(): + session = session_factory() + SqlAlchemyQueryContext(session.query(Employee.gender)).where( + field=Employee.gender, op=Op.GROUP_BY, value=None).fetch() + with warnings.catch_warnings(): + # SQLite (unlike the library's real MySQL target) warns that + # DISTINCT-on-a-column is Postgres-only syntax and is otherwise + # silently ignored - a pre-existing SQLAlchemy quirk, not a + # regression, so it's suppressed here rather than asserted on. + warnings.simplefilter("ignore") + SqlAlchemyQueryContext(session.query(Employee.gender)).where( + field=Employee.gender, op=Op.DISTINCT, value=None).fetch() + + +def test_sqlalchemy_context_having_filters_on_aggregated_field(): + from sqlalchemy import func + session = session_factory() + + # 2 groups of 2 (M, F) - HAVING count(*) >= 2 keeps both, > 2 keeps none. + ctx = SqlAlchemyQueryContext(session.query(Employee.gender, func.count(Employee.emp_no))) + ctx.where(field=Employee.gender, op=Op.GROUP_BY, value=None) + ctx.having(field=func.count(Employee.emp_no), op=Op.GTE, value={"search": 2}) + assert {row[0] for row in ctx.fetch()} == {"M", "F"} + + ctx2 = SqlAlchemyQueryContext(session.query(Employee.gender, func.count(Employee.emp_no))) + ctx2.where(field=Employee.gender, op=Op.GROUP_BY, value=None) + ctx2.having(field=func.count(Employee.emp_no), op=Op.GT, value={"search": 2}) + assert ctx2.fetch() == [] + + +def test_sqlalchemy_context_unsupported_op_raises(): + session = session_factory() + ctx = SqlAlchemyQueryContext(session.query(Employee)) + with pytest.raises(ValueError): + ctx.where(field=Employee.gender, op="not-a-real-op", value={"search": "M"}) + + +def test_sqlalchemy_context_having_unsupported_op_raises(): + session = session_factory() + ctx = SqlAlchemyQueryContext(session.query(Employee)) + with pytest.raises(ValueError): + ctx.having(field=Employee.gender, op="not-a-real-op", value={"search": "M"}) + + +def test_sqlalchemy_context_native_and_with_native_roundtrip(): + session = session_factory() + ctx = SqlAlchemyQueryContext(session.query(Employee)) + mutated = ctx.native.filter(Employee.emp_no == 1) + ctx = ctx.with_native(mutated) + assert [row.emp_no for row in ctx.fetch()] == [1] + + +def test_sqlalchemy_context_order_by_and_limit_offset_and_count(): + session = session_factory() + ctx = SqlAlchemyQueryContext(session.query(Employee)) + assert ctx.count() == 4 + ctx.order_by(field=Employee.emp_no, direction="dsc") + ctx.limit_offset(limit=2, offset=0) + assert [row.emp_no for row in ctx.fetch()] == [4, 3] + + +# ---------- ClickHouseQueryContext: every Op, plus compile()/count_compile() ---------- + +@pytest.mark.parametrize("op,value", [ + (Op.EQ, {"search": "F"}), + (Op.NEQ, {"search": "F"}), + (Op.IN, {"list": ["F"]}), + (Op.RANGE, {"start": 1, "end": 2}), + (Op.LIKE, {"search": "F"}), + (Op.STARTS_WITH, {"search": "F"}), + (Op.ENDS_WITH, {"search": "F"}), + (Op.CONTAINS, {"search": "F"}), + (Op.GT, {"search": 1}), + (Op.GTE, {"search": 1}), + (Op.LT, {"search": 4}), + (Op.LTE, {"search": 4}), + (Op.IS_NULL, None), + (Op.NOT_NULL, None), +]) +def test_clickhouse_context_every_op_compiles_and_binds_params(op, value): + ctx = ClickHouseQueryContext(client=FakeClickHouseClient(), table="employees", + columns=["emp_no"]) + ctx.where(field="gender", op=op, value=value) + sql, params = ctx.compile() + assert "employees" in sql + if value and value.get("search") is not None: + assert any(str(value["search"]) in str(bound) for bound in params.values()) + + +def test_clickhouse_context_group_by_and_distinct(): + ctx = ClickHouseQueryContext(client=FakeClickHouseClient(), table="employees", columns=["gender"]) + ctx.where(field="gender", op=Op.GROUP_BY, value=None) + sql, _ = ctx.compile() + assert "GROUP BY `gender`" in sql + + ctx2 = ClickHouseQueryContext(client=FakeClickHouseClient(), table="employees", columns=["gender"]) + ctx2.where(field="gender", op=Op.DISTINCT, value=None) + sql2, _ = ctx2.compile() + assert "SELECT DISTINCT" in sql2 + + +def test_clickhouse_context_unsupported_op_raises(): + ctx = ClickHouseQueryContext(client=FakeClickHouseClient(), table="employees") + with pytest.raises(ValueError): + ctx.where(field="gender", op="not-a-real-op", value={"search": "F"}) + + +def test_clickhouse_context_having_unsupported_op_raises(): + ctx = ClickHouseQueryContext(client=FakeClickHouseClient(), table="employees") + with pytest.raises(ValueError): + ctx.having(field="gender", op="not-a-real-op", value={"search": "F"}) + + +def test_clickhouse_context_count_compile_and_no_columns_selects_star(): + ctx = ClickHouseQueryContext(client=FakeClickHouseClient(), table="employees") + sql, _ = ctx.compile() + assert "SELECT * FROM employees" in sql + count_sql, _ = ctx.count_compile() + assert count_sql.strip().upper().startswith("SELECT COUNT(*)") + + +def test_clickhouse_context_having_filters_on_aggregated_field(): + ctx = ClickHouseQueryContext(client=FakeClickHouseClient(), table="employees", columns=["gender"]) + ctx.where(field="gender", op=Op.GROUP_BY, value=None) + ctx.having(field="total", op=Op.GT, value={"search": 100}) + sql, params = ctx.compile() + assert "GROUP BY `gender` HAVING `total` > %(" in sql + assert 100 in params.values() + + +def test_clickhouse_context_count_compile_wraps_grouped_query_for_correct_group_count(): + """SELECT count(*) FROM t GROUP BY g would return one row per group, each + holding that group's row count - not the number of groups. Pagination + needs the latter, so a grouped/having count wraps as a derived table.""" + ctx = ClickHouseQueryContext(client=FakeClickHouseClient(), table="employees", columns=["gender"]) + ctx.where(field="gender", op=Op.GROUP_BY, value=None) + ctx.having(field="total", op=Op.GT, value={"search": 100}) + count_sql, count_params = ctx.count_compile() + assert count_sql.startswith("SELECT count(*) FROM (SELECT 1 FROM employees GROUP BY `gender` HAVING") + assert 100 in count_params.values() + + # ungrouped: stays the simple direct form, no wrapping needed + plain_ctx = ClickHouseQueryContext(client=FakeClickHouseClient(), table="employees") + plain_sql, _ = plain_ctx.count_compile() + assert plain_sql == "SELECT count(*) FROM employees" + + +def test_clickhouse_context_order_by_raw_expresses_compound_tiebreak(): + ctx = ClickHouseQueryContext(client=FakeClickHouseClient(), table="employees") + ctx.order_by_raw("(emp_no = 1) DESC, gender ASC") + sql, _ = ctx.compile() + assert "ORDER BY (emp_no = 1) DESC, gender ASC" in sql + + +def test_clickhouse_context_add_raw_condition_binds_values_not_identifiers(): + from fastapi_listing.context.clickhouse import quote_identifier + + ctx = ClickHouseQueryContext(client=FakeClickHouseClient(), table="employees") + col = quote_identifier("gender") + ctx.add_raw_condition(f"multiSearchAnyCaseInsensitive({col}, {{needles}})", needles=["m", "f"]) + sql, params = ctx.compile() + assert "multiSearchAnyCaseInsensitive(`gender`, %(" in sql + assert ["m", "f"] in params.values() + + +def test_clickhouse_context_add_raw_condition_having(): + from fastapi_listing.context.clickhouse import quote_identifier + + ctx = ClickHouseQueryContext(client=FakeClickHouseClient(), table="employees") + col = quote_identifier("total") + ctx.add_raw_condition(f"{col} > {{threshold}}", having=True, threshold=100) + sql, params = ctx.compile() + assert "HAVING `total` > %(" in sql + assert 100 in params.values() + + +def test_having_mixin_routes_canonical_filter_through_having(): + class TotalAboveHaving(generic_filters.HavingMixin, generic_filters.DataGreaterThanFilter): + pass + + assert TotalAboveHaving.target_clause == "having" + + ctx = ClickHouseQueryContext(client=FakeClickHouseClient(), table="employees") + flt = TotalAboveHaving(extra_context={}, field_extract_fn=lambda x: x) + flt.filter(field="total", value={"search": 100}, context=ctx) + sql, params = ctx.compile() + assert "HAVING `total` > %(" in sql + assert 100 in params.values() + + +def test_clickhouse_context_native_returns_compiled_sql_and_params(): + ctx = ClickHouseQueryContext(client=FakeClickHouseClient(), table="employees") + ctx.where(field="gender", op=Op.EQ, value={"search": "F"}) + sql, params = ctx.native + assert "employees" in sql + assert params == {"__fl_p1": "F"} + + +def test_clickhouse_context_limit_offset_with_nonzero_offset(): + ctx = ClickHouseQueryContext(client=FakeClickHouseClient(), table="employees") + ctx.limit_offset(limit=10, offset=20) + sql, _ = ctx.compile() + assert "LIMIT 10" in sql + assert "OFFSET 20" in sql + + +def test_clickhouse_context_from_raw_sql_wraps_custom_query_untouched(): + """The full-bypass escape hatch: a hand-built query (CTEs, joins, window + functions, whatever the canonical Op vocabulary can't express) is wrapped + as a derived table, own params preserved, and canonical where/order/limit + still layer on top without colliding with the caller's own param names.""" + raw_sql = ( + "WITH ranked AS (SELECT emp_no, channel, " + "row_number() OVER (PARTITION BY channel ORDER BY emp_no) AS rn " + "FROM some_table WHERE account_id = %(account_id)s) " + "SELECT emp_no, channel FROM ranked" + ) + ctx = ClickHouseQueryContext.from_raw_sql( + client=FakeClickHouseClient(), sql=raw_sql, params={"account_id": 1001}) + ctx.where(field="channel", op=Op.EQ, value={"search": "organic"}) + ctx.limit_offset(limit=5, offset=0) + + sql, params = ctx.compile() + assert raw_sql in sql + assert "account_id" in params and params["account_id"] == 1001 + assert any(k.startswith("__fl_") for k in params if k != "account_id") + assert "LIMIT 5" in sql + + count_sql, count_params = ctx.count_compile() + assert raw_sql in count_sql + assert count_params["account_id"] == 1001 + + +# ---------- CommonFilterImpl deprecation alias ---------- + +def test_common_filter_impl_alias_warns_on_instantiation(): + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + generic_filters.CommonFilterImpl(extra_context={}) + assert any(issubclass(w.category, DeprecationWarning) for w in caught) + + +# ---------- PaginationStrategy.postprocess hook ---------- + +class _UppercasingPaginator(PaginationStrategy): + def postprocess(self, rows, extra_context): + return [{"emp_no": r.emp_no, "first_name": r.first_name.upper()} for r in rows] + + +strategy_factory.register_strategy("uppercasing_paginator_test", _UppercasingPaginator) + + +def test_postprocess_hook_transforms_rows_before_page_envelope(): + dao = EmployeeDao(read_db=session_factory()) + resp = FastapiListing(dao=dao, fields_to_fetch=["emp_no", "first_name"]).get_response( + MetaInfo(default_srt_on="emp_no", default_srt_ord="asc", + paginating_strategy="uppercasing_paginator_test") + ) + assert [row["first_name"] for row in resp["data"]] == ["SACHIN", "RAHUL", "ANJALI", "PRIYA"] + + +def test_postprocess_hook_default_is_identity(): + dao = EmployeeDao(read_db=session_factory()) + resp = FastapiListing(dao=dao, fields_to_fetch=["emp_no", "first_name"]).get_response( + MetaInfo(default_srt_on="emp_no", default_srt_ord="asc") + ) + assert [row.first_name for row in resp["data"]] == ["Sachin", "Rahul", "Anjali", "Priya"] From 67c36768fa51a7230fe95b73b927b7eeccff4327 Mon Sep 17 00:00:00 2001 From: danielhasan1 Date: Sun, 16 Aug 2026 14:02:56 +0530 Subject: [PATCH 2/5] Document the ClickHouse escape hatches in README and docs/query.rst CHANGELOG.md already covered from_raw_sql, HavingMixin, order_by_raw, and add_raw_condition; the README's "Backend support" section and docs/query.rst only described the basic canonical-Op story, undercutting the actual point of these additions - full developer control when canonical filters/sort aren't enough. --- README.md | 17 +++++++++++++ docs/query.rst | 69 +++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 85 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 46e617a..eeca7e2 100644 --- a/README.md +++ b/README.md @@ -438,6 +438,23 @@ directly. pip install fastapi-listing[clickhouse] ``` +Canonical filters/sort/pagination cover the common case - equality/range/comparison checks on a plain +column, single-column sort, offset/limit pagination. Real queries aren't always that simple, so every +escape hatch that exists for SQLAlchemy (`context.native`) has a ClickHouse equivalent, and then some: + +* **`ClickHouseQueryContext.from_raw_sql(client=..., sql=..., params=...)`** - the full bypass. Hand-build + a query with CTEs, joins, window functions, a table function as the source, whatever the canonical `Op` + vocabulary can't express - using whatever query-building approach you already have - and you still get + back a `QueryContext` that canonical filters/sort/pagination can layer on top of, or that you can use + completely as-is. +* **`HavingMixin`** - filter on an aggregated field after a `GROUP BY` (`SUM(x) > 100`), same canonical + `Op`s, routed to `HAVING` instead of `WHERE`: `class TotalAbove(HavingMixin, DataGreaterThanFilter): pass`. +* **`order_by_raw(expression)`** - for a compound ordering rule (a tiebreak column, multiple sort keys) + that a single `field, direction` pair can't represent. +* **`add_raw_condition(sql_template, **values)`** - a per-filter escape hatch for a backend-specific SQL + function (a full-text search builtin, an array operator, ...) with no canonical `Op` equivalent - values + are still bound through the driver's real parameter binding, never string-formatted into the SQL text. + Want a different ORM or database driver (Tortoise, Django ORM, raw psycopg2, pymongo, ...)? Write your own `QueryContext` + DAO pair the same way `ClickHouseQueryContext`/`ClickHouseDao` do it - see `docs/query.rst`. Neither SQLAlchemy nor clickhouse-driver are required to install the package; both are opt-in extras. diff --git a/docs/query.rst b/docs/query.rst index cd493fa..3f73892 100644 --- a/docs/query.rst +++ b/docs/query.rst @@ -220,4 +220,71 @@ This same ``QueryContext`` contract is what lets FastAPI Listing support non-ORM ``ClickHouseQueryContext`` (``fastapi_listing.context.clickhouse``) ships as a reference implementation, paired with ``fastapi_listing.dao.ClickHouseDao``, proving the same ``Filter``/``SortingOrderStrategy``/ ``PaginationStrategy`` classes work unmodified against raw parameterized SQL, not just an ORM. See -:ref:`learnfilters` for how filters stay backend-agnostic through the shared ``Op`` vocabulary. \ No newline at end of file +:ref:`learnfilters` for how filters stay backend-agnostic through the shared ``Op`` vocabulary. + +Escape hatches: when the canonical vocabulary genuinely isn't enough +--------------------------------------------------------------------- + +Canonical filters/sort/pagination cover comparisons on a plain column, single-column sort, and +offset/limit pagination - the common case. Real queries aren't always that simple. ``ClickHouseQueryContext`` +has an escape hatch for each level of "not simple enough": + +``from_raw_sql`` - the full bypass + .. code-block:: python + + raw_sql = build_my_complicated_query(account_id=..., date_range=...) # CTEs, joins, + # window functions, + # a table function - + # however you already + # build it + context = ClickHouseQueryContext.from_raw_sql(client=my_client, sql=raw_sql, params={...}) + + Wraps your query as a derived table. Canonical filters/sort/pagination can still layer ``WHERE``/ + ``ORDER BY``/``LIMIT`` on top of whatever columns your query exposes - or you can leave ``filter_mapper`` + empty and let the raw SQL stand entirely as-is. Structural, request-scoped query shape (which account, + which date range, how a metric is computed) belongs here, built once by your ``QueryStrategy``/DAO - + not something a generic ``Filter`` class should know about. + +``HavingMixin`` - filtering on an aggregated field + .. code-block:: python + + from fastapi_listing.filters.generic_filters import HavingMixin, DataGreaterThanFilter + + class TotalConversionsAbove(HavingMixin, DataGreaterThanFilter): + pass + + Same canonical ``Op`` (``GT``, in this case), routed to ``HAVING`` instead of ``WHERE`` - for filtering + on the result of a ``GROUP BY`` (``SUM(x) > 100``), which ``WHERE`` cannot express. Available on + ``SqlAlchemyQueryContext`` too (``context.having(field=..., op=..., value=...)``, mirroring ``.having()`` + on a SQLAlchemy ``Query``). + +``order_by_raw`` - a compound ordering rule + .. code-block:: python + + context.order_by_raw(f"(site_id = {int(own_domain_id)}) DESC, `{sort_column}` {direction}") + + For an ordering rule that isn't a single ``field, direction`` pair - a tiebreak column pinned first, + then the user's chosen sort, or any other multi-key expression the canonical ``order_by()`` can't + represent. + +``add_raw_condition`` - a backend-specific SQL function, per filter + .. code-block:: python + + from fastapi_listing.context.clickhouse import quote_identifier + + class MultiSearchAnyFilter(CanonicalFilter): + def filter(self, *, field=None, value=None, context=None): + col = quote_identifier(self.extract_field(field)) + return context.add_raw_condition( + f"multiSearchAnyCaseInsensitive({col}, {{needles}})", needles=value.get("list") or []) + + For a single filter that needs a builtin ClickHouse function (a full-text search primitive, an array + operator, ...) with no canonical ``Op`` equivalent. Values still flow through the driver's real + parameter binding (``%(name)s``) - never string-formatted into the SQL text - only the resolved + field/column identifier is spliced in directly (via ``quote_identifier``), the same way every other + op in ``ClickHouseQueryContext`` already handles identifiers vs. values. + +The rule of thumb across all four: reach for the narrowest escape hatch that solves your problem. +Need a different clause or expression for one filter/one sort? Use ``having``/``order_by_raw``/ +``add_raw_condition``. Need a fundamentally different query shape (CTEs, joins, a table function)? +``from_raw_sql`` is the one that hands you full control. \ No newline at end of file From a6e3f4d2f0a423ae2bfe0546e5be73bf2e389f06 Mon Sep 17 00:00:00 2001 From: danielhasan1 Date: Sun, 16 Aug 2026 14:29:12 +0530 Subject: [PATCH 3/5] Fix wrong 409 status for unregistered filter/sort fields, use 422 409 Conflict is for resource-state conflicts (edit conflicts, duplicate creation) - requesting a filter/sort field outside filter_mapper/sort_mapper isn't that, it's an invalid request parameter. The same file already used 422 for the closely related "filter/sort request itself is malformed" case, so this was an internal inconsistency as well as a spec misuse. Both exception classes are plain fastapi.HTTPException subclasses, so this was always overridable per-app via @app.exception_handler(...) without any library change - the fix here is just to make the library's own default spec-correct, not to add configuration for it. --- CHANGELOG.md | 8 ++++++++ fastapi_listing/service/_core_listing_service.py | 4 ++-- tests/test_main_v2.py | 4 ++-- 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7df296b..7882019 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,6 +48,14 @@ and ships a ClickHouse reference implementation (raw parameterized SQL, no ORM) `Op` vocabulary. Along the way, fixed a latent bug in `ClickHouseQueryContext.count_compile()`: counting a `GROUP BY` query directly returns one count *per group*, not the total number of groups: it now wraps as a derived table when grouped/having, matching what SQLAlchemy's `Query.count()` already did automatically. +- Fixed the status code for "you requested a filter/sort field that isn't in `filter_mapper`/`sort_mapper`": + `NotRegisteredApiException` now raises `422 Unprocessable Entity`, not `409 Conflict`. `409` is for + resource-state conflicts (edit conflicts, duplicate creation) - an unregistered field isn't that, and the + same file already used `422` for the closely related "the filter/sort request itself is malformed" case, so + this was an inconsistency as well as a misuse of the code. If your client/telemetry specifically depended on + `409` here, register your own handler (`@app.exception_handler(NotRegisteredApiException)`) rather than + relying on the library's default - both exception classes are plain `fastapi.HTTPException` subclasses, so + this has always been overridable per-app without any library change. ### Migration diff --git a/fastapi_listing/service/_core_listing_service.py b/fastapi_listing/service/_core_listing_service.py index b7f700f..da85427 100644 --- a/fastapi_listing/service/_core_listing_service.py +++ b/fastapi_listing/service/_core_listing_service.py @@ -67,7 +67,7 @@ def _apply_sorting(self, query: QueryContext, listing_meta_info: ListingMetaInfo listing_meta_info.sorting_column_mapper.keys()) if temp: raise NotRegisteredApiException( - status_code=409, detail=f"Sorter(s) not registered with listing: {temp}, Did you forget to do it?") + status_code=422, detail=f"Sorter(s) not registered with listing: {temp}, Did you forget to do it?") if sorting_params: sorting_params = self._replace_aliases(listing_meta_info.sorting_column_mapper, sorting_params) else: @@ -92,7 +92,7 @@ def _apply_filters(self, query: QueryContext, listing_meta_info: ListingMetaInfo temp = set(item.get("field") for item in fltrs) - set(listing_meta_info.filter_column_mapper.keys()) if temp: raise NotRegisteredApiException( - status_code=409, detail=f"Filter(s) not registered with listing: {temp}, Did you forget to do it?") + status_code=422, detail=f"Filter(s) not registered with listing: {temp}, Did you forget to do it?") fltrs = self._replace_aliases(listing_meta_info.filter_column_mapper, fltrs) diff --git a/tests/test_main_v2.py b/tests/test_main_v2.py index 1fa425b..4cf73e1 100644 --- a/tests/test_main_v2.py +++ b/tests/test_main_v2.py @@ -308,12 +308,12 @@ def test_core_service_exceptions(): resp = client.get("/v1/error-sorting", params={"sort": get_url_quoted_string([{"field": "hdtds", "type": "asc"}])}) - assert resp.status_code == 409 + assert resp.status_code == 422 assert resp.json() == {'detail': "Sorter(s) not registered with listing: {'hdtds'}, Did you forget to do it?"} resp = client.get("/v1/error-sorting", params={"filter": get_url_quoted_string([{"field": "hdtds", "type": "asc"}])}) - assert resp.status_code == 409 + assert resp.status_code == 422 assert resp.json() == {'detail': "Filter(s) not registered with listing: {'hdtds'}, Did you forget to do it?"} resp = client.get("/v1/error-sorting", params={"filter": '%5B%22field%22%3A%20%22hdt%22%2C%20%22type%22%22asc%22%7D%5D'}) From 59ef0a0fad45e85cd43e1a6e2e5a2d8fd2062682 Mon Sep 17 00:00:00 2001 From: danielhasan1 Date: Sun, 16 Aug 2026 17:25:18 +0530 Subject: [PATCH 4/5] Fix ContextVar leak in middlewares.py, raise coverage to ~97%, add real-DB CI - Fixed a real bug in middlewares.py's manager(): the finally block inferred whether *this* call set a session token from the shared ContextVar's current value, which could be a leftover from an earlier, unrelated call (especially with implicit_close=False, which deliberately never resets) - causing an UnboundLocalError. Now tracked locally per call. - Added ~10 test files closing real, previously-untested branches (validated against the real MySQL CI's line-by-line coverage first, not padding): paginator validation/pagination-math, factory registry error paths, the class-based ListingService flow via SQLite (dao_factory fallback, MetaInfo, switch()), loader's page-size check, and an automated check that `import fastapi_listing` actually works without SQLAlchemy installed (previously only verified manually in a throwaway venv). - Added tests/test_clickhouse_real_integration.py: the ClickHouse backend had only ever been validated against a fake client. This runs the same canonical-filter/HAVING/from_raw_sql scenarios against a real ClickHouse server via the real clickhouse-driver package - skips gracefully if none is reachable locally, always provisioned in CI (new docker-compose.dev.yml for local parity, tests.yml now provisions ClickHouse alongside the existing MySQL container). - Removed a dead try/except in context/__init__.py (ClickHouseQueryContext never actually imports clickhouse-driver, so it could never raise) and marked two Protocol-only interface classes `# pragma: no cover` so the coverage report doesn't misrepresent structural type stubs as real gaps. - setup.py: Development Status 5 (Production/Stable) -> 4 (Beta). Shipping a breaking rework with a brand-new, not-yet-battle-tested backend under the same "stable" label as the well-worn SQLAlchemy path overstated it. - Added CONTRIBUTING.md documenting local test setup for both real databases. Full suite verified locally against both a real MySQL instance (the same danielhasan1/mysql_employees_test_db image CI uses) and a real ClickHouse server: 119 passed, 97% coverage. --- .github/workflows/tests.yml | 8 +- CLAUDE.md | 125 ++++++++++++++++ CONTRIBUTING.md | 44 ++++++ docker-compose.dev.yml | 23 +++ fastapi_listing/context/__init__.py | 13 +- .../interface/client_site_params_adapter.py | 2 +- .../interface/listing_meta_info.py | 2 +- fastapi_listing/middlewares.py | 27 ++-- setup.py | 2 +- tests/test_clickhouse_real_integration.py | 135 ++++++++++++++++++ tests/test_factories_edge_cases.py | 39 +++++ tests/test_import_without_sqlalchemy.py | 46 ++++++ tests/test_loader_and_service_edge_cases.py | 76 ++++++++++ tests/test_middlewares.py | 107 ++++++++++++++ tests/test_paginator_edge_cases.py | 108 ++++++++++++++ tests/test_query_context.py | 46 ++++++ 16 files changed, 782 insertions(+), 21 deletions(-) create mode 100644 CLAUDE.md create mode 100644 CONTRIBUTING.md create mode 100644 docker-compose.dev.yml create mode 100644 tests/test_clickhouse_real_integration.py create mode 100644 tests/test_factories_edge_cases.py create mode 100644 tests/test_import_without_sqlalchemy.py create mode 100644 tests/test_loader_and_service_edge_cases.py create mode 100644 tests/test_middlewares.py create mode 100644 tests/test_paginator_edge_cases.py diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 8b58651..c5a7e28 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -9,12 +9,16 @@ jobs: matrix: python-version: [ 3.7, 3.8, 3.9, '3.10', '3.11' ] steps: - - name: Setup Test Db + - name: Setup MySQL Test Db run: | docker pull danielhasan1/mysql_employees_test_db docker run -d -p 3307:3306 --name dazzling_wright danielhasan1/mysql_employees_test_db sleep 20 docker exec dazzling_wright bash setup-data + - name: Setup ClickHouse Test Db + run: | + docker run -d -p 9010:9000 --name fastapi_listing_clickhouse clickhouse/clickhouse-server:latest + sleep 10 - name: Checkout uses: actions/checkout@v2 @@ -25,7 +29,7 @@ jobs: - name: Install dependencies run: | - pip install -e .[test] + pip install -e .[test,clickhouse] # - name: Run linting (flake8) # run: | # flake8 ./fastapi_listing ./tests diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..8fd84e5 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,125 @@ +# fastapi-listing — working notes for Claude Code + +This file persists the architectural decisions, conventions, and hard-won lessons from the 0.4.0 +backend-agnostic rework so a future session (after context compaction, or a brand new conversation) +doesn't have to re-derive them from scratch. + +## What 0.4.0 actually did + +The library's Filter/Sorter/Paginator/QueryStrategy contracts were pluggable in theory but every +default implementation talked to a raw SQLAlchemy `Query` directly, and four modules imported +SQLAlchemy unconditionally at load time (so the library couldn't even be imported without it). 0.4.0 +introduced `QueryContext` (`fastapi_listing/context/`) as the real seam, with `SqlAlchemyQueryContext` +as the default backend and `ClickHouseQueryContext` as a reference non-ORM backend proving the +abstraction actually generalizes. See `CHANGELOG.md` for the full breaking-change/migration details - +don't duplicate that here, it's kept current. + +## The core design rule (keep applying it) + +**Structural, request-scoped query shape belongs in the DAO/QueryStrategy, resolved once per request +from parameters. Generic listing concerns (comparisons on an exposed column, sort, pagination) belong +in canonical `Filter`/`Op` classes.** This line gets litigated on every new feature - a filter that +needs to reach inside a pre-aggregated CTE, or a sort that needs a business-rule tiebreak, is not +something canonical filters should be forced to express. Reach for the narrowest escape hatch instead: + +- `context.native` (SQLAlchemy) / `ClickHouseQueryContext.from_raw_sql()` - full bypass, hand-build the + whole query (CTEs, joins, window functions, a table function as the source). +- `HavingMixin` - canonical `Op`, routed to `HAVING` instead of `WHERE`, for filtering on an aggregated + field after a `GROUP BY`. +- `order_by_raw()` - a compound ordering rule (a tiebreak column, multiple sort keys) that a single + `field, direction` pair can't express. +- `add_raw_condition()` - a single filter needing a backend-specific SQL builtin (e.g. + `multiSearchAnyCaseInsensitive`) with no canonical `Op` equivalent. Values still bind through the + driver's real parameter binding; only the resolved field/column identifier is spliced in directly. + +None of this is speculative - `from_raw_sql`/`HavingMixin`/`order_by_raw`/`add_raw_condition` were all +validated against a **real** ClickHouse server (not just the fake-client unit tests) during this work. +`ClickHouseQueryContext.count_compile()` wraps grouped queries as a derived table when counting +(`GROUP BY` directly would return one count *per group*, not the total) - this was a real bug, caught +and fixed, confirmed correct against a real server too. + +## Known warts (real, pre-existing, deliberately not fixed - don't rediscover these as new bugs) + +- `filter_factory` (`fastapi_listing/factory/filter.py`) registers by **field path**, not by alias. Two + different operators on the *same* column (e.g. an `eq` and a `gt` filter both resolving to + `indexed_pages`) collide as duplicate registrations. Workaround already used in the codebase's own + tests (`tests/service_setup.py`'s `"DeptEmp1.to_date"`/`"DeptEmp2.to_date"`): give each a distinct + decoy namespace prefix before the last dot - `extract_field()` only ever reads the segment after the + final `.`, so `"my_decoy_ns.indexed_pages"` still resolves to the `indexed_pages` attribute. +- `dao_factory.create(key)` raises a bare `ValueError` if `key` was never registered, and only raises + `MissingSessionError` (which `ListingService.__init__` catches to fall back to manual construction) + if the key *is* registered but no session is bound. A DAO must be `dao_factory.register_dao(...)`'d + even if you never intend to use the factory-bound-session path, or the fallback in + `ListingService.__init__` never triggers correctly. +- `middlewares.py`'s `_session`/`_replica_session` are shared, module-level `ContextVar`s. The + `manager()` cleanup logic used to infer "did this call set a token" from the ContextVar's *current* + value - which could be a leftover from an earlier, unrelated call (especially with + `implicit_close=False`, which deliberately never resets). Fixed by tracking token assignment locally + per call instead. If you touch `manager()` again, don't reintroduce that inference. +- There is no ClickHouse equivalent of `DaoSessionBinderMiddleware` - deliberate, not an oversight. See + "Don't build speculative infrastructure" below. +- `interface/client_site_params_adapter.py` is dead code - grepped, referenced nowhere in the package. + Left alone (deletion is a separate, smaller decision), marked `# pragma: no cover` along with + `interface/listing_meta_info.py`'s `Protocol` (structural type stubs, not real coverage gaps). + +## Don't build speculative infrastructure for hypothetical backends + +Explicitly decided: no auto-configuring connection-binding middleware for ClickHouse (or any future +backend) inside the library itself. There are too many ORMs/drivers/connection strategies to support +out of the box, and building one for ClickHouse specifically (when nobody's asked for it yet) would be +solving a problem nobody has. The pattern - construct your own client, pass it to the DAO directly, or +write your own thin `ContextVar`-based binder mirroring `middlewares.py` if you want per-request +lifecycle - is the intended, documented answer. Don't second-guess this into "should we add it" again +without a concrete need driving it. + +## Safety rule - do not relax this one + +An earlier version of this work included an `examples/` directory demonstrating a ClickHouse +conversion, built by directly reading a real production codebase (a specific employer's internal +system) for structural reference. Even after renaming identifiers, it was still a recognizable +derivative (matching filter-op vocabulary, exact metric-calculation shape, real ClickHouse builtin +usage patterns) of that real system - "reskinning" wasn't sufficient. It was removed entirely, and a +real account ID that had leaked into a *committed test* (not just the example) was scrubbed before +ever being pushed. **Never build examples, tests, or documentation by structurally mirroring a real +company's actual production code, even disguised.** If a real-world case study is ever wanted again, it +must be invented from a made-up domain, not derived from having read someone else's real system. + +## Testing philosophy applied throughout this work + +- Cross-check "missing coverage" against what the *real* CI (real MySQL, matrix across Python + 3.7-3.11) actually shows before writing a test - a lot of apparent gaps in a sandbox without MySQL + access are illusory (the MySQL-dependent tests fail on `ModuleNotFoundError: MySQLdb` before ever + reaching the code in question, not a real gap). Chase the ones that are still missing in the real CI + logs, or that are testable without a database at all (pure Python logic - validation branches, + factory registries, ContextVar plumbing). +- Some gaps are genuinely not worth chasing: version-dependent fallback branches (`typing.Literal` + pre-3.8, SQLAlchemy-absent, pydantic v1-vs-v2) only ever take one branch per environment/interpreter + - don't fabricate multi-environment test matrices to "fix" these; they average out across the real CI + matrix already. +- Prefer a real, throwaway local ClickHouse instance (`clickhouse local`/`clickhouse server`, the + official all-in-one binary) over only trusting fake-client unit tests when validating anything + ClickHouse-dialect-specific. Fakes prove internal consistency; a real server proves the SQL is + actually valid. +- When adding tests, don't pad for a coverage number - every test added during this work was tied to a + specific, real, previously-untested branch, cross-referenced against the real CI's line-by-line + "Missing" column first. + +## Git/release conventions for this repo specifically + +- `~/dev/personal/` has a conditional gitconfig include (`~/.gitconfig-personal`) - commits here use + the `danielhasan1`/`dh813030@gmail.com` identity, SSH remote, and **no** `Co-Authored-By` or + `Change-Id` trailers (the latter was a leftover Gerrit `commit-msg` hook from a work identity, + disabled here via `core.hooksPath = .git/hooks`). +- No fixed rule on squash-vs-new-commit: default judgment call based on whether the new change is part + of the same logical unit of work already on the branch, but always ask/confirm before amending + + force-pushing something already on the remote. +- Breaking changes on a pre-1.0 library are spec-compliant to ship as a MINOR version bump (semver + explicitly allows this for `0.x`), not necessarily a major bump - already applied (0.3.4 -> 0.4.0). +- `setup.py`'s `Development Status` classifier should reflect actual real-world validation, not just + "the code passed CI." A breaking rework with a brand-new, not-yet-battle-tested backend shipping + under `"5 - Production/Stable"` overstates it - step down to `"4 - Beta"` for a release like this, + move back up once the new parts have real usage behind them. +- Standard practice for OSS releases: you cannot and should not try to test every possible downstream + usage before shipping - that's what pre-releases, a clear CHANGELOG/migration guide, and loud + actionable errors (`FastapiListingMigrationError`) are for. What you *can and must* verify yourself is + your own direct usage/customizations, since nothing else does that for you. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000..d953634 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,44 @@ +# Contributing + +## Running the test suite locally + +```bash +pip install -e .[test,clickhouse] +``` + +Most of the suite runs without any external services. Two things need real databases: + +- **MySQL** (the `employees` sample DB) - required for the SQLAlchemy-backed HTTP-level tests + (`tests/test_main.py`, `tests/test_main_v2.py`, `tests/test_fast_listing_compact_version.py`). Without + it, those tests fail with `ModuleNotFoundError: No module named 'MySQLdb'` or a connection error - + everything else in the suite still runs and still means something. +- **ClickHouse** - only `tests/test_clickhouse_real_integration.py` needs it, and it **skips + gracefully** (not a failure) if nothing is reachable. Every other ClickHouse-related test + (`test_clickhouse_backend.py`, the `Op` coverage in `test_query_context.py`) runs against an + in-process fake client and needs no real server - the real-integration file exists specifically to + catch anything a fake client's pattern-matching could miss (real dialect quirks, real parameter + binding behavior). + +Bring both up with: + +```bash +docker compose -f docker-compose.dev.yml up -d +docker exec fastapi_listing_mysql bash setup-data # first run only, loads the sample data +``` + +Then run everything, including the real-database tests: + +```bash +PYTHONPATH=. pytest --cov=fastapi_listing --cov=tests --cov-report=term-missing --cov-fail-under=80 +``` + +Tear down when done: `docker compose -f docker-compose.dev.yml down`. + +If you only have one of the two running, that's fine - the suite degrades gracefully either way +(MySQL-dependent tests fail loudly since they're a hard requirement for that HTTP-level path; +ClickHouse's real-server test simply skips). + +## CI + +`.github/workflows/tests.yml` provisions both automatically (matrixed across Python 3.7-3.11), so +neither is optional there - a PR is expected to pass with both available. diff --git a/docker-compose.dev.yml b/docker-compose.dev.yml new file mode 100644 index 0000000..4f18e7d --- /dev/null +++ b/docker-compose.dev.yml @@ -0,0 +1,23 @@ +# Local dev/test databases, mirroring what .github/workflows/tests.yml provisions in CI. +# Not used by CI itself (CI provisions these directly via `docker run`, see the workflow file) - +# this is for running the full suite (including the real-MySQL and real-ClickHouse tests) locally. +# +# Usage: +# docker compose -f docker-compose.dev.yml up -d +# docker exec fastapi_listing_mysql bash setup-data # loads the employees sample data (first run only) +# PYTHONPATH=. pytest --cov=fastapi_listing --cov=tests --cov-report=term-missing +# +# Bring it down when done: docker compose -f docker-compose.dev.yml down + +services: + mysql: + image: danielhasan1/mysql_employees_test_db + container_name: fastapi_listing_mysql + ports: + - "3307:3306" + + clickhouse: + image: clickhouse/clickhouse-server:latest + container_name: fastapi_listing_clickhouse + ports: + - "9010:9000" diff --git a/fastapi_listing/context/__init__.py b/fastapi_listing/context/__init__.py index aa44017..d051cf5 100644 --- a/fastapi_listing/context/__init__.py +++ b/fastapi_listing/context/__init__.py @@ -1,10 +1,9 @@ from fastapi_listing.context.base import QueryContext from fastapi_listing.context.sqlalchemy import SqlAlchemyQueryContext +# ClickHouseQueryContext is duck-typed against whatever client it's given +# (see context/clickhouse.py) - it never imports clickhouse-driver itself, so +# this import can't fail regardless of whether that optional extra is +# installed. No try/except needed here. +from fastapi_listing.context.clickhouse import ClickHouseQueryContext -__all__ = ["QueryContext", "SqlAlchemyQueryContext"] - -try: - from fastapi_listing.context.clickhouse import ClickHouseQueryContext # noqa: F401 - __all__.append("ClickHouseQueryContext") -except ImportError: - pass +__all__ = ["QueryContext", "SqlAlchemyQueryContext", "ClickHouseQueryContext"] diff --git a/fastapi_listing/interface/client_site_params_adapter.py b/fastapi_listing/interface/client_site_params_adapter.py index b8be8e9..b7011ba 100644 --- a/fastapi_listing/interface/client_site_params_adapter.py +++ b/fastapi_listing/interface/client_site_params_adapter.py @@ -6,7 +6,7 @@ from typing import List -class ClientSiteParamAdapter(Protocol): +class ClientSiteParamAdapter(Protocol): # pragma: no cover - structural type only, never instantiated/called def get(self, key: str): pass diff --git a/fastapi_listing/interface/listing_meta_info.py b/fastapi_listing/interface/listing_meta_info.py index 34f0105..f562e38 100644 --- a/fastapi_listing/interface/listing_meta_info.py +++ b/fastapi_listing/interface/listing_meta_info.py @@ -13,7 +13,7 @@ AbstractListingFeatureParamsAdapter) -class ListingMetaInfo(Protocol): +class ListingMetaInfo(Protocol): # pragma: no cover - structural type only, never instantiated/called @property def paginating_strategy(self) -> AbsPaginatingStrategy: # type : ignore # noqa diff --git a/fastapi_listing/middlewares.py b/fastapi_listing/middlewares.py index 7ec7eaa..d9a7928 100644 --- a/fastapi_listing/middlewares.py +++ b/fastapi_listing/middlewares.py @@ -66,18 +66,27 @@ def manager(read_ses: Callable[[], SqlAlchemySession], master: Callable[[], SqlA suppress_warnings: bool): global _session global _replica_session + # Tracked locally rather than inferred from _session.get()/_replica_session.get() + # in the finally block below: those are shared, module-level ContextVars, and + # their current value can be a leftover from an earlier, unrelated manager() + # call (e.g. one that used implicit_close=False) - not proof that *this* call + # set them. Checking the shared state instead of a local flag previously caused + # an UnboundLocalError when a call that only set one of the two tokens ran + # after an earlier call left the other ContextVar populated. + token_read_session: Optional[Token] = None + token_master_session: Optional[Token] = None if read_ses and master: - token_read_session: Token = _replica_session.set(read_ses()) - token_master_session: Token = _session.set(master()) + token_read_session = _replica_session.set(read_ses()) + token_master_session = _session.set(master()) elif master: sess = master() - token_read_session: Token = _replica_session.set(sess) - token_master_session: Token = _session.set(sess) + token_read_session = _replica_session.set(sess) + token_master_session = _session.set(sess) if not suppress_warnings: warn("Only 'master' session is provided. dao will use master for read executes." "To suppress this warning add 'suppress_warnings=True'") elif read_ses: - token_read_session: Token = _replica_session.set(read_ses()) + token_read_session = _replica_session.set(read_ses()) else: raise ValueError("Error with DaoSessionBinderMiddleware! " "Please provide either args read or master session callables.") @@ -85,9 +94,9 @@ def manager(read_ses: Callable[[], SqlAlchemySession], master: Callable[[], SqlA yield finally: if implicit_close: - if _session.get(): + if token_master_session is not None: _session.get().close() - _session.reset(token_master_session) # type: ignore # noqa: F823 - if _replica_session.get(): + _session.reset(token_master_session) + if token_read_session is not None: _replica_session.get().close() - _replica_session.reset(token_read_session) # type: ignore # noqa: F823 + _replica_session.reset(token_read_session) diff --git a/setup.py b/setup.py index ccb3816..2bc0698 100644 --- a/setup.py +++ b/setup.py @@ -30,7 +30,7 @@ def get_long_description(): packages=setuptools.find_packages(exclude=["tests.*"]), package_data={"fastapi_listing": ["py.typed"]}, classifiers=[ - "Development Status :: 5 - Production/Stable", + "Development Status :: 4 - Beta", "Environment :: Web Environment", "Intended Audience :: Developers", "License :: OSI Approved :: MIT License", diff --git a/tests/test_clickhouse_real_integration.py b/tests/test_clickhouse_real_integration.py new file mode 100644 index 0000000..1d2e684 --- /dev/null +++ b/tests/test_clickhouse_real_integration.py @@ -0,0 +1,135 @@ +"""Real-server validation for the ClickHouse backend - not a fake client. + +tests/test_clickhouse_backend.py and test_query_context.py prove the Python +logic is internally consistent (right SQL shape, right params) against a fake +client that pattern-matches expected output. This file proves the generated +SQL is actually valid ClickHouse dialect, executed by the real +`clickhouse-driver` package against a real server - CTEs/derived-table +wrapping, GROUP BY + HAVING, and parameter binding all behave as expected on +a real engine, not just in a regex-matching fake. + +In CI (see .github/workflows/tests.yml), a real `clickhouse/clickhouse-server` +container is always provisioned, so this always runs there. Locally, it skips +gracefully if nothing is listening on the configured host/port - run +`docker compose -f docker-compose.dev.yml up -d clickhouse` (or point +FASTAPI_LISTING_TEST_CLICKHOUSE_HOST/PORT at your own instance) to exercise it. +""" + +import os + +import pytest + +clickhouse_driver = pytest.importorskip("clickhouse_driver", reason="clickhouse-driver not installed") + +from fastapi_listing import FastapiListing, MetaInfo +from fastapi_listing.context.clickhouse import ClickHouseQueryContext +from fastapi_listing.dao import ClickHouseDao +from fastapi_listing.factory import filter_factory +from fastapi_listing.filters import generic_filters +from fastapi_listing.ops import Op + +CH_HOST = os.environ.get("FASTAPI_LISTING_TEST_CLICKHOUSE_HOST", "127.0.0.1") +CH_PORT = int(os.environ.get("FASTAPI_LISTING_TEST_CLICKHOUSE_PORT", "9010")) +DB_NAME = "fastapi_listing_integration_test" + + +def _make_client(): + return clickhouse_driver.Client(host=CH_HOST, port=CH_PORT, connect_timeout=3) + + +@pytest.fixture(scope="module") +def real_client(): + try: + client = _make_client() + client.execute("SELECT 1") + except Exception as exc: + pytest.skip(f"No real ClickHouse reachable at {CH_HOST}:{CH_PORT} ({exc})") + return + + client.execute(f"CREATE DATABASE IF NOT EXISTS {DB_NAME}") + client.execute(f""" + CREATE TABLE IF NOT EXISTS {DB_NAME}.employees ( + emp_no UInt32, first_name String, gender String + ) ENGINE = MergeTree() ORDER BY emp_no + """) + client.execute(f"TRUNCATE TABLE {DB_NAME}.employees") + client.execute(f"INSERT INTO {DB_NAME}.employees VALUES", [ + (1, "Sachin", "M"), (2, "Rahul", "M"), (3, "Anjali", "F"), (4, "Priya", "F"), + ]) + client.execute(f""" + CREATE TABLE IF NOT EXISTS {DB_NAME}.site_metrics_weekly ( + account_id UInt32, site_id UInt32, site String, + time_value UInt32, indexed_pages UInt64 + ) ENGINE = MergeTree() ORDER BY (account_id, site_id, time_value) + """) + client.execute(f"TRUNCATE TABLE {DB_NAME}.site_metrics_weekly") + client.execute(f"INSERT INTO {DB_NAME}.site_metrics_weekly VALUES", [ + (1, 10, "brightedge-like.example", 1, 5000), (1, 10, "brightedge-like.example", 2, 4500), + (1, 20, "another.example", 1, 1200), (1, 20, "another.example", 2, 1300), + (1, 30, "third.example", 1, 300), (1, 30, "third.example", 2, 250), + ]) + yield client + client.execute(f"DROP DATABASE IF EXISTS {DB_NAME}") + + +class _Employee: + __table__ = f"{DB_NAME}.employees" + emp_no = "emp_no" + first_name = "first_name" + gender = "gender" + + +class _EmployeeDao(ClickHouseDao): + name = "real_ch_integration_employee" + model = _Employee + + +def test_canonical_filter_sort_paginate_against_real_clickhouse(real_client): + # "RealCHIntegration.gender", not "Employee.gender" - filter_factory registers by field path, not + # alias, and "Employee.gender" is already registered by tests/service_setup.py. extract_field() only + # reads the segment after the last dot, so this still resolves to the "gender" attribute correctly. + filter_mapper = {"gdr_real_ch": ("RealCHIntegration.gender", generic_filters.EqualityFilter)} + filter_factory.register_filter_mapper(filter_mapper) + + import json + from urllib.parse import quote + filter_qs = quote(json.dumps([{"field": "gdr_real_ch", "value": {"search": "F"}}])) + + dao = _EmployeeDao(read_db=real_client) + resp = FastapiListing(dao=dao, fields_to_fetch=["emp_no", "first_name", "gender"]).get_response( + MetaInfo(default_srt_on="emp_no", default_srt_ord="asc", filter_mapper=filter_mapper, filter=filter_qs) + ) + assert resp["totalCount"] == 2 + assert [row["emp_no"] for row in resp["data"]] == [3, 4] + + +def test_having_against_a_real_group_by(real_client): + ctx = ClickHouseQueryContext(client=real_client, table=f"{DB_NAME}.employees", columns=["gender"]) + ctx.where(field="gender", op=Op.GROUP_BY, value=None) + ctx.having(field="gender", op=Op.EQ, value={"search": "M"}) + assert ctx.fetch() == [{"gender": "M"}] + + +def test_grouped_count_wraps_correctly_on_a_real_server(real_client): + ctx = ClickHouseQueryContext(client=real_client, table=f"{DB_NAME}.employees", columns=["gender"]) + ctx.where(field="gender", op=Op.GROUP_BY, value=None) + assert ctx.count() == 2 # 2 distinct genders, not "2 rows in the M group + 2 in the F group" + + +def test_from_raw_sql_aggregation_against_a_real_server(real_client): + raw_sql = f""" + SELECT site_id, any(site) AS domain, + sumIf(indexed_pages, time_value = 1) AS indexed_pages_current, + sumIf(indexed_pages, time_value = 2) AS indexed_pages_prev + FROM {DB_NAME}.site_metrics_weekly + WHERE account_id = %(account_id)s + GROUP BY site_id + """ + ctx = ClickHouseQueryContext.from_raw_sql(client=real_client, sql=raw_sql, params={"account_id": 1}) + ctx.where(field="indexed_pages_current", op=Op.GT, value={"search": 1000}) + ctx.order_by_raw("(site_id = 10) DESC, `indexed_pages_current` DESC") + ctx.limit_offset(limit=10, offset=0) + + rows = ctx.fetch() + assert {row["site_id"] for row in rows} == {10, 20} + assert rows[0]["site_id"] == 10 # pinned first regardless of the metric sort diff --git a/tests/test_factories_edge_cases.py b/tests/test_factories_edge_cases.py new file mode 100644 index 0000000..bdfa915 --- /dev/null +++ b/tests/test_factories_edge_cases.py @@ -0,0 +1,39 @@ +"""Error/edge branches on the registries that were never exercised without a +real MySQL connection getting in the way first.""" + +import pytest + +from fastapi_listing.factory import strategy_factory +from fastapi_listing.factory import _generic_factory +from fastapi_listing.abstracts import AbsQueryStrategy + + +def test_strategy_factory_rejects_non_strategy_builder(): + with pytest.raises(ValueError, match="is not a valid type of strategy"): + strategy_factory.register_strategy("not_a_strategy_test_key", object) + + +def test_strategy_factory_create_unknown_key_raises(): + with pytest.raises(ValueError, match="no strategy found with name"): + strategy_factory.create("definitely_unregistered_strategy_key") + + +def test_strategy_factory_accepts_a_real_strategy_subclass(): + class _FakeQueryStrategy(AbsQueryStrategy): + def get_query(self, *, request=None, dao=None, extra_context=None): + return "fake-context" + + strategy_factory.register_strategy("fake_query_strategy_edge_case_test", _FakeQueryStrategy) + instance = strategy_factory.create("fake_query_strategy_edge_case_test") + assert isinstance(instance, _FakeQueryStrategy) + + +def test_generic_factory_unregister_removes_a_registered_key(): + _generic_factory.register("unregister_edge_case_test", lambda x: x) + assert "unregister_edge_case_test" in _generic_factory.object_creation_collector + _generic_factory.unregister("unregister_edge_case_test") + assert "unregister_edge_case_test" not in _generic_factory.object_creation_collector + + +def test_generic_factory_unregister_unknown_key_is_a_noop(): + _generic_factory.unregister("never_registered_key_edge_case_test") # should not raise diff --git a/tests/test_import_without_sqlalchemy.py b/tests/test_import_without_sqlalchemy.py new file mode 100644 index 0000000..5294754 --- /dev/null +++ b/tests/test_import_without_sqlalchemy.py @@ -0,0 +1,46 @@ +"""Nothing in the existing suite actually verifies "import fastapi_listing works +without SQLAlchemy installed" - the CI's test extras always install sqlalchemy +(needed for the MySQL-backed tests), so ctyping.py's ImportError fallback path +never fires there. This was only ever checked manually, in a throwaway venv, +during development. Verified here via import-hook poisoning in a subprocess - +a real, repeatable, CI-visible check of the actual headline claim, not a +simulation that could silently stop matching reality. +""" + +import subprocess +import sys + +_CODE = """ +import builtins +_real_import = builtins.__import__ + +def _blocked(name, *args, **kwargs): + if name == "sqlalchemy" or name.startswith("sqlalchemy."): + raise ImportError(f"simulated absence of {name}") + return _real_import(name, *args, **kwargs) + +builtins.__import__ = _blocked + +import fastapi_listing +print("IMPORT_OK", fastapi_listing.__version__) +""" + + +def test_import_fastapi_listing_without_sqlalchemy(): + result = subprocess.run([sys.executable, "-c", _CODE], capture_output=True, text=True, timeout=30) + assert result.returncode == 0, result.stderr + assert "IMPORT_OK" in result.stdout + + +def test_ctyping_sqlalchemy_bound_types_are_none_without_sqlalchemy(): + code = _CODE + """ +from fastapi_listing import ctyping +assert ctyping.DeclarativeMeta is None +assert ctyping.Query is None +assert ctyping.Session is None +assert ctyping.Column is None +print("CTYPING_FALLBACK_OK") +""" + result = subprocess.run([sys.executable, "-c", code], capture_output=True, text=True, timeout=30) + assert result.returncode == 0, result.stderr + assert "CTYPING_FALLBACK_OK" in result.stdout diff --git a/tests/test_loader_and_service_edge_cases.py b/tests/test_loader_and_service_edge_cases.py new file mode 100644 index 0000000..3e0c9cc --- /dev/null +++ b/tests/test_loader_and_service_edge_cases.py @@ -0,0 +1,76 @@ +"""Two things the SQLite-based tests elsewhere never exercised: + +1. loader.py's default_page_size > max_page_size validation. +2. The class-based ListingService flow itself (MetaInfo(), switch(), and the + dao_factory -> MissingSessionError -> manual-construction fallback in + ListingService.__init__) - every other SQLite test in this suite calls + FastapiListing(dao=dao, ...) directly, bypassing ListingService entirely. + Real coverage of this path previously depended on the MySQL-backed tests. +""" + +import pytest + +from fastapi_listing import ListingService, loader +from fastapi_listing.dao import dao_factory +from fastapi_listing.errors import MissingExpectedAttribute + +from .sqlalchemy_listing_setup import EmployeeDao, session_factory + +dao_factory.register_dao(EmployeeDao.name, EmployeeDao) + + +def test_loader_rejects_default_page_size_greater_than_max_page_size(): + with pytest.raises(ValueError, match="can not be greater than max_page_size"): + @loader.register() + class _BadPageSizeService(ListingService): # noqa: F811,F841 + default_srt_on = "emp_no" + default_dao = EmployeeDao + default_page_size = 100 + max_page_size = 50 + + +@loader.register() +class _SqliteEmployeeListingService(ListingService): + default_srt_on = "emp_no" + default_dao = EmployeeDao + + def get_listing(self): + resp = self.get_response_for_test() + return resp + + def get_response_for_test(self): + from fastapi_listing import FastapiListing + return FastapiListing(self.request, self.dao, fields_to_fetch=["emp_no", "first_name"]).get_response( + self.MetaInfo(self)) + + +def test_listing_service_falls_back_to_manual_dao_construction_without_bound_session(): + """dao_factory.create() raises MissingSessionError here (the DAO is + registered, but no middleware/manager() bound a session in this test) - + ListingService.__init__ must fall back to constructing the DAO directly + from the read_db kwarg instead of propagating that error.""" + service = _SqliteEmployeeListingService(request=None, read_db=session_factory()) + assert isinstance(service.dao, EmployeeDao) + resp = service.get_listing() + assert [row.emp_no for row in resp["data"]] == [4, 3, 2, 1] # default dsc sort + + +def test_listing_service_switch_changes_a_registered_strategy_attribute(): + service = _SqliteEmployeeListingService(request=None, read_db=session_factory()) + assert service.query_strategy == "default_query" + service.switch("query_strategy", "default_query") # same value, but exercises the real path + assert service.query_strategy == "default_query" + + +def test_listing_service_switch_rejects_unknown_strategy_type(): + service = _SqliteEmployeeListingService(request=None, read_db=session_factory()) + with pytest.raises(ValueError, match="unknown strategy type"): + service.switch("not_a_real_strategy_type", "whatever") + + +def test_listing_service_missing_default_srt_on_raises(): + with pytest.raises(MissingExpectedAttribute): + @loader.register() + class _MissingSortOnService(ListingService): # noqa: F811,F841 + default_srt_on = "" + default_dao = EmployeeDao diff --git a/tests/test_middlewares.py b/tests/test_middlewares.py new file mode 100644 index 0000000..b0bb5d1 --- /dev/null +++ b/tests/test_middlewares.py @@ -0,0 +1,107 @@ +"""manager()'s branch logic (which combination of master/replica callables was +given, the implicit-close cleanup) is plain control flow - it only ever calls +whatever callable it's handed and stores the result in a ContextVar, so none +of this needs a real database session to exercise. Previously only covered +incidentally through the MySQL-backed tests (and only the "both" branch, at +that) - the master-only warning, replica-only, and error branches had zero +coverage even in real CI. +""" + +import pytest + +from fastapi_listing import middlewares +from fastapi_listing.middlewares import manager, SessionProvider +from fastapi_listing.errors import MissingSessionError + + +@pytest.fixture(autouse=True) +def _reset_session_context_vars(): + """_session/_replica_session are shared, module-level ContextVars - + implicit_close=False (several tests below use it deliberately, to test + that specific behavior) intentionally leaves them populated after the + `with manager(...)` block exits, since that flag means "the caller + manages session lifecycle, not manager() itself". Without this reset, + a session left behind by one test leaks into whichever test runs next in + the same process, regardless of test order - a test-isolation problem, + not something manager() itself is supposed to solve.""" + middlewares._session.set(None) + middlewares._replica_session.set(None) + yield + middlewares._session.set(None) + middlewares._replica_session.set(None) + + +class _FakeSession: + def __init__(self): + self.closed = False + + def close(self): + self.closed = True + + +def test_manager_both_master_and_replica(): + master_session = _FakeSession() + replica_session = _FakeSession() + with manager(read_ses=lambda: replica_session, master=lambda: master_session, + implicit_close=False, suppress_warnings=True): + assert SessionProvider.session is master_session + assert SessionProvider.read_session is replica_session + + +def test_manager_master_only_warns_and_reuses_for_read(): + session = _FakeSession() + with pytest.warns(UserWarning, match="Only 'master' session is provided"): + with manager(read_ses=None, master=lambda: session, implicit_close=False, suppress_warnings=False): + assert SessionProvider.session is session + assert SessionProvider.read_session is session + + +def test_manager_master_only_suppresses_warning_when_asked(): + session = _FakeSession() + with manager(read_ses=None, master=lambda: session, implicit_close=False, suppress_warnings=True): + assert SessionProvider.session is session + + +def test_manager_replica_only(): + session = _FakeSession() + with manager(read_ses=lambda: session, master=None, implicit_close=False, suppress_warnings=True): + assert SessionProvider.read_session is session + with pytest.raises(MissingSessionError): + SessionProvider.session + + +def test_manager_neither_raises(): + with pytest.raises(ValueError, match="Please provide either args read or master session callables"): + with manager(read_ses=None, master=None, implicit_close=False, suppress_warnings=True): + pass + + +def test_manager_implicit_close_closes_and_resets_sessions(): + master_session = _FakeSession() + replica_session = _FakeSession() + with manager(read_ses=lambda: replica_session, master=lambda: master_session, + implicit_close=True, suppress_warnings=True): + pass + assert master_session.closed is True + assert replica_session.closed is True + with pytest.raises(MissingSessionError): + SessionProvider.session + with pytest.raises(MissingSessionError): + SessionProvider.read_session + + +def test_manager_implicit_close_with_replica_only_does_not_reference_undefined_master_token(): + """The finally block's `_session.get()` guard must skip referencing + token_master_session entirely here, since only the replica branch ran and + only set token_read_session - a NameError would mean that guard is wrong.""" + session = _FakeSession() + with manager(read_ses=lambda: session, master=None, implicit_close=True, suppress_warnings=True): + pass + assert session.closed is True + + +def test_manager_implicit_close_with_master_only_reuses_session_for_both_tokens(): + session = _FakeSession() + with manager(read_ses=None, master=lambda: session, implicit_close=True, suppress_warnings=True): + pass + assert session.closed is True diff --git a/tests/test_paginator_edge_cases.py b/tests/test_paginator_edge_cases.py new file mode 100644 index 0000000..e1217d5 --- /dev/null +++ b/tests/test_paginator_edge_cases.py @@ -0,0 +1,108 @@ +"""PaginationStrategy's validation/pagination-math branches are plain Python +logic operating on a QueryContext through its public count()/fetch()/ +limit_offset() interface - none of it needs a real database, so a minimal fake +context is enough to exercise every branch directly. +""" + +import pytest + +from fastapi_listing.errors import ListingPaginatorError +from fastapi_listing.paginator import PaginationStrategy + + +class _FakeContext: + """Just enough of QueryContext for PaginationStrategy to drive - no real + backend needed since these are pure pagination-math/validation tests.""" + + def __init__(self, rows): + self._rows = list(rows) + self._limit = None + self._offset = None + + def limit_offset(self, *, limit, offset): + self._limit = limit + self._offset = offset + return self + + def count(self): + return len(self._rows) + + def fetch(self): + if self._limit is None: + return self._rows + return self._rows[self._offset:self._offset + self._limit] + + +@pytest.mark.parametrize("page_num,page_size", [ + (1.5, 1), (1, 1.5), ("x", 1), (1, "x"), (None, 1), (1, None), +]) +def test_validate_params_rejects_non_integers(page_num, page_size): + strategy = PaginationStrategy() + with pytest.raises(ListingPaginatorError, match="not valid integers"): + strategy.validate_params(page_num, page_size) + + +@pytest.mark.parametrize("page_num,page_size", [(0, 1), (1, 0), (-1, 5), (1, -1)]) +def test_validate_params_rejects_less_than_one(page_num, page_size): + strategy = PaginationStrategy() + with pytest.raises(ListingPaginatorError, match="less than 1"): + strategy.validate_params(page_num, page_size) + + +def test_validate_params_accepts_whole_number_floats(): + strategy = PaginationStrategy() + strategy.validate_params(2.0, 10.0) # should not raise + + +def test_paginate_falls_back_to_page_1_size_10_on_invalid_params(): + strategy = PaginationStrategy() + context = _FakeContext(range(25)) + page = strategy.paginate(context, {"page": "not-a-number", "pageSize": 5}, {}) + assert page["currentPageNumber"] == 1 + assert page["currentPageSize"] == 10 + + +def test_is_next_page_exists_with_count_query(): + strategy = PaginationStrategy() + strategy.set_count(25) + strategy.set_page_num(1) + strategy.set_page_size(10) + assert strategy.is_next_page_exists() is True + strategy.set_page_num(3) + assert strategy.is_next_page_exists() is False + + +def test_is_next_page_exists_without_count_query(): + strategy = PaginationStrategy(fire_count_qry=False) + strategy.set_page_size(10) + strategy.set_count(11) + assert strategy.is_next_page_exists() is True + strategy.set_count(10) + assert strategy.is_next_page_exists() is False + + +def test_page_without_count_trims_the_lookahead_row_when_has_next(): + strategy = PaginationStrategy(fire_count_qry=False) + context = _FakeContext(range(11)) # page_size + 1 lookahead trick + page = strategy.paginate(context, {"page": 1, "pageSize": 10}, {}) + assert page["hasNext"] is True + assert len(page["data"]) == 10 + + +def test_page_without_count_returns_all_rows_when_no_next_page(): + strategy = PaginationStrategy(fire_count_qry=False) + context = _FakeContext(range(5)) + page = strategy.paginate(context, {"page": 1, "pageSize": 10}, {}) + assert page["hasNext"] is False + assert len(page["data"]) == 5 + + +def test_paginate_with_count_query_returns_full_page_envelope(): + strategy = PaginationStrategy() + context = _FakeContext(range(25)) + page = strategy.paginate(context, {"page": 2, "pageSize": 10}, {}) + assert page["totalCount"] == 25 + assert page["currentPageNumber"] == 2 + assert page["currentPageSize"] == 10 + assert page["hasNext"] is True + assert list(page["data"]) == list(range(10, 20)) diff --git a/tests/test_query_context.py b/tests/test_query_context.py index c7a884a..8f85d98 100644 --- a/tests/test_query_context.py +++ b/tests/test_query_context.py @@ -331,3 +331,49 @@ def test_postprocess_hook_default_is_identity(): MetaInfo(default_srt_on="emp_no", default_srt_ord="asc") ) assert [row.first_name for row in resp["data"]] == ["Sachin", "Rahul", "Anjali", "Priya"] + + +# ---------- Remaining small gaps: value=None early-return, custom_fields, migration-guard's introspection edge, pydantic_serializer ---------- + +def test_canonical_filter_with_none_value_is_a_noop_for_comparison_ops(): + session = session_factory() + ctx = SqlAlchemyQueryContext(session.query(Employee)) + flt = generic_filters.EqualityFilter(extra_context={}, field_extract_fn=lambda x: Employee.gender) + result = flt.filter(field="gender", value=None, context=ctx) + assert result is ctx + assert {row.emp_no for row in result.fetch()} == {1, 2, 3, 4} + + +def test_query_strategy_custom_fields_skips_unknown_attributes_silently(): + from fastapi_listing.strategies import QueryStrategy + + dao = EmployeeDao(read_db=session_factory()) + strategy = QueryStrategy() + fields = strategy.get_inst_attr_to_read( + custom_fields=True, field_list=["emp_no", "not_a_real_attribute", "first_name"], dao=dao) + assert fields == [Employee.emp_no, Employee.first_name] + + +def test_guard_legacy_signature_skips_uninspectable_callables(): + from fastapi_listing.errors import guard_legacy_signature + + # a plain instance has no __call__ signature inspect.signature() can read - + # this must be treated as "can't tell, don't block" rather than crashing. + guard_legacy_signature(object(), legacy_kwarg="query", new_kwarg="context", + subject="test", fix="n/a") # should not raise + + +def test_fastapi_listing_with_pydantic_serializer(): + from pydantic import BaseModel + + class EmployeeOut(BaseModel): + # No ORM-mode config needed: fastapi_listing only reads this model's + # field names to derive fields_to_fetch, it never serializes through it. + emp_no: int + first_name: str + + dao = EmployeeDao(read_db=session_factory()) + resp = FastapiListing(dao=dao, pydantic_serializer=EmployeeOut).get_response( + MetaInfo(default_srt_on="emp_no", default_srt_ord="asc") + ) + assert [row.first_name for row in resp["data"]] == ["Sachin", "Rahul", "Anjali", "Priya"] From 958a6dccb8febf8a5f963a87914c75737e75db0f Mon Sep 17 00:00:00 2001 From: danielhasan1 Date: Sun, 16 Aug 2026 17:57:31 +0530 Subject: [PATCH 5/5] Stop tracking CLAUDE.md and .DS_Store CLAUDE.md is local AI-assistant working notes - not something to impose on other contributors of a public repo, so it stays untracked rather than committed or even gitignored. .DS_Store is macOS Finder clutter that never should have been tracked in the first place. --- .DS_Store | Bin 8196 -> 0 bytes CLAUDE.md | 125 ------------------------------------------------------ 2 files changed, 125 deletions(-) delete mode 100644 .DS_Store delete mode 100644 CLAUDE.md diff --git a/.DS_Store b/.DS_Store deleted file mode 100644 index f778b369c225cfe9b8394d0ee7c3e0c92957ea7b..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 8196 zcmeHM&2G~`5T0$5#z|G00}_!Ck}up!)$%7eAf;(@hy(}`1P4H^-K2?yYe%uusDxCx z^8|3<6?hJwfVbez3BK7ik-bUjEkb2i+8xLKzTMf+JF{Jvh(vAB+9s+Hk%h{(yozQ{ z;pbdu%2IOYI;;Sns7-xxs6*|BKZ7dhqkvJsC}0%$7Zkvr&BZ!r-&apfYZNdF z{Fe%Fey~y5Ruel>R;CU#@(2K%!);k`jdOtH*omzscA~7LqEB^t5D8TzM+~9pcy2Qs zwwl<9vWiYZ(McpDi{ykNBG3tZ9t`MuAxc`0QS!O**8IPSWT1@X+-`7f(5$ zc2GBe$$~4Q0vJc+X< z{O|L3hqNc+r^d;H<53GfJ6WWh;g{45*MRKNDaF7>X!oIzgWA={Af9cSSq2lH0Nnx) zc-Rok2=`CvnEdR>QiU`>PcCNfLEV;yWCha59D4THyDX09B7UUi!Z_(c7ANNQCvomD z>LqXgo7?lfDBjxoE|!*a`IUlYm8{ZQ>20$c4x6zP52BjWf6c$1IdRubu3v?Y<;?tu`QUUU@+)9M-}?(28U~P4&RCidJ#ITp5k--nqYF-`l=B z-mpgxZ*OhbxG^3Vt!p>8A2kk+Pv4I|jL$wn92Al4545g+PutJ9nhJ5!><4}rbeMp; zj>YWA*X-b>dwAKt5*BSh9Zfb~RO#1n0sFFwdBLCUotmtx0 zUrDX$74*k(>CajJh)9vf77&FoN1n=Z*`7K6H1uO?&@R>KDK)9Cp%F6~w1s|E7TiSK zI(jT}jES?9-xfVd@JE=buV)j^SQ&gNlZ7~Y1>|WyF(*CWB0Y!qJG6(f>QMGr&m>aK ztXC2AkCAtJz$5QvN8lWn@oELChd}qCsFrhQvQHlUsvH_+dzH$ky+B->?8@LAhNJwWBcxQViof<_+4VdZfgcKQ!P lT-$&$r<&M_vbck?@DBkeb9SNh?pYwS|C{|^uFcEb{08XWLkR!? diff --git a/CLAUDE.md b/CLAUDE.md deleted file mode 100644 index 8fd84e5..0000000 --- a/CLAUDE.md +++ /dev/null @@ -1,125 +0,0 @@ -# fastapi-listing — working notes for Claude Code - -This file persists the architectural decisions, conventions, and hard-won lessons from the 0.4.0 -backend-agnostic rework so a future session (after context compaction, or a brand new conversation) -doesn't have to re-derive them from scratch. - -## What 0.4.0 actually did - -The library's Filter/Sorter/Paginator/QueryStrategy contracts were pluggable in theory but every -default implementation talked to a raw SQLAlchemy `Query` directly, and four modules imported -SQLAlchemy unconditionally at load time (so the library couldn't even be imported without it). 0.4.0 -introduced `QueryContext` (`fastapi_listing/context/`) as the real seam, with `SqlAlchemyQueryContext` -as the default backend and `ClickHouseQueryContext` as a reference non-ORM backend proving the -abstraction actually generalizes. See `CHANGELOG.md` for the full breaking-change/migration details - -don't duplicate that here, it's kept current. - -## The core design rule (keep applying it) - -**Structural, request-scoped query shape belongs in the DAO/QueryStrategy, resolved once per request -from parameters. Generic listing concerns (comparisons on an exposed column, sort, pagination) belong -in canonical `Filter`/`Op` classes.** This line gets litigated on every new feature - a filter that -needs to reach inside a pre-aggregated CTE, or a sort that needs a business-rule tiebreak, is not -something canonical filters should be forced to express. Reach for the narrowest escape hatch instead: - -- `context.native` (SQLAlchemy) / `ClickHouseQueryContext.from_raw_sql()` - full bypass, hand-build the - whole query (CTEs, joins, window functions, a table function as the source). -- `HavingMixin` - canonical `Op`, routed to `HAVING` instead of `WHERE`, for filtering on an aggregated - field after a `GROUP BY`. -- `order_by_raw()` - a compound ordering rule (a tiebreak column, multiple sort keys) that a single - `field, direction` pair can't express. -- `add_raw_condition()` - a single filter needing a backend-specific SQL builtin (e.g. - `multiSearchAnyCaseInsensitive`) with no canonical `Op` equivalent. Values still bind through the - driver's real parameter binding; only the resolved field/column identifier is spliced in directly. - -None of this is speculative - `from_raw_sql`/`HavingMixin`/`order_by_raw`/`add_raw_condition` were all -validated against a **real** ClickHouse server (not just the fake-client unit tests) during this work. -`ClickHouseQueryContext.count_compile()` wraps grouped queries as a derived table when counting -(`GROUP BY` directly would return one count *per group*, not the total) - this was a real bug, caught -and fixed, confirmed correct against a real server too. - -## Known warts (real, pre-existing, deliberately not fixed - don't rediscover these as new bugs) - -- `filter_factory` (`fastapi_listing/factory/filter.py`) registers by **field path**, not by alias. Two - different operators on the *same* column (e.g. an `eq` and a `gt` filter both resolving to - `indexed_pages`) collide as duplicate registrations. Workaround already used in the codebase's own - tests (`tests/service_setup.py`'s `"DeptEmp1.to_date"`/`"DeptEmp2.to_date"`): give each a distinct - decoy namespace prefix before the last dot - `extract_field()` only ever reads the segment after the - final `.`, so `"my_decoy_ns.indexed_pages"` still resolves to the `indexed_pages` attribute. -- `dao_factory.create(key)` raises a bare `ValueError` if `key` was never registered, and only raises - `MissingSessionError` (which `ListingService.__init__` catches to fall back to manual construction) - if the key *is* registered but no session is bound. A DAO must be `dao_factory.register_dao(...)`'d - even if you never intend to use the factory-bound-session path, or the fallback in - `ListingService.__init__` never triggers correctly. -- `middlewares.py`'s `_session`/`_replica_session` are shared, module-level `ContextVar`s. The - `manager()` cleanup logic used to infer "did this call set a token" from the ContextVar's *current* - value - which could be a leftover from an earlier, unrelated call (especially with - `implicit_close=False`, which deliberately never resets). Fixed by tracking token assignment locally - per call instead. If you touch `manager()` again, don't reintroduce that inference. -- There is no ClickHouse equivalent of `DaoSessionBinderMiddleware` - deliberate, not an oversight. See - "Don't build speculative infrastructure" below. -- `interface/client_site_params_adapter.py` is dead code - grepped, referenced nowhere in the package. - Left alone (deletion is a separate, smaller decision), marked `# pragma: no cover` along with - `interface/listing_meta_info.py`'s `Protocol` (structural type stubs, not real coverage gaps). - -## Don't build speculative infrastructure for hypothetical backends - -Explicitly decided: no auto-configuring connection-binding middleware for ClickHouse (or any future -backend) inside the library itself. There are too many ORMs/drivers/connection strategies to support -out of the box, and building one for ClickHouse specifically (when nobody's asked for it yet) would be -solving a problem nobody has. The pattern - construct your own client, pass it to the DAO directly, or -write your own thin `ContextVar`-based binder mirroring `middlewares.py` if you want per-request -lifecycle - is the intended, documented answer. Don't second-guess this into "should we add it" again -without a concrete need driving it. - -## Safety rule - do not relax this one - -An earlier version of this work included an `examples/` directory demonstrating a ClickHouse -conversion, built by directly reading a real production codebase (a specific employer's internal -system) for structural reference. Even after renaming identifiers, it was still a recognizable -derivative (matching filter-op vocabulary, exact metric-calculation shape, real ClickHouse builtin -usage patterns) of that real system - "reskinning" wasn't sufficient. It was removed entirely, and a -real account ID that had leaked into a *committed test* (not just the example) was scrubbed before -ever being pushed. **Never build examples, tests, or documentation by structurally mirroring a real -company's actual production code, even disguised.** If a real-world case study is ever wanted again, it -must be invented from a made-up domain, not derived from having read someone else's real system. - -## Testing philosophy applied throughout this work - -- Cross-check "missing coverage" against what the *real* CI (real MySQL, matrix across Python - 3.7-3.11) actually shows before writing a test - a lot of apparent gaps in a sandbox without MySQL - access are illusory (the MySQL-dependent tests fail on `ModuleNotFoundError: MySQLdb` before ever - reaching the code in question, not a real gap). Chase the ones that are still missing in the real CI - logs, or that are testable without a database at all (pure Python logic - validation branches, - factory registries, ContextVar plumbing). -- Some gaps are genuinely not worth chasing: version-dependent fallback branches (`typing.Literal` - pre-3.8, SQLAlchemy-absent, pydantic v1-vs-v2) only ever take one branch per environment/interpreter - - don't fabricate multi-environment test matrices to "fix" these; they average out across the real CI - matrix already. -- Prefer a real, throwaway local ClickHouse instance (`clickhouse local`/`clickhouse server`, the - official all-in-one binary) over only trusting fake-client unit tests when validating anything - ClickHouse-dialect-specific. Fakes prove internal consistency; a real server proves the SQL is - actually valid. -- When adding tests, don't pad for a coverage number - every test added during this work was tied to a - specific, real, previously-untested branch, cross-referenced against the real CI's line-by-line - "Missing" column first. - -## Git/release conventions for this repo specifically - -- `~/dev/personal/` has a conditional gitconfig include (`~/.gitconfig-personal`) - commits here use - the `danielhasan1`/`dh813030@gmail.com` identity, SSH remote, and **no** `Co-Authored-By` or - `Change-Id` trailers (the latter was a leftover Gerrit `commit-msg` hook from a work identity, - disabled here via `core.hooksPath = .git/hooks`). -- No fixed rule on squash-vs-new-commit: default judgment call based on whether the new change is part - of the same logical unit of work already on the branch, but always ask/confirm before amending + - force-pushing something already on the remote. -- Breaking changes on a pre-1.0 library are spec-compliant to ship as a MINOR version bump (semver - explicitly allows this for `0.x`), not necessarily a major bump - already applied (0.3.4 -> 0.4.0). -- `setup.py`'s `Development Status` classifier should reflect actual real-world validation, not just - "the code passed CI." A breaking rework with a brand-new, not-yet-battle-tested backend shipping - under `"5 - Production/Stable"` overstates it - step down to `"4 - Beta"` for a release like this, - move back up once the new parts have real usage behind them. -- Standard practice for OSS releases: you cannot and should not try to test every possible downstream - usage before shipping - that's what pre-releases, a clear CHANGELOG/migration guide, and loud - actionable errors (`FastapiListingMigrationError`) are for. What you *can and must* verify yourself is - your own direct usage/customizations, since nothing else does that for you.