From b8e811e87c32c9b160bfb5b51a005228357d4aca Mon Sep 17 00:00:00 2001 From: d3vyce Date: Thu, 22 Oct 2026 13:22:57 -0400 Subject: [PATCH 1/9] fix: coerce facet filter values to the column type --- src/fastapi_toolsets/crud/search.py | 78 ++++++++++++------------- tests/test_crud_search.py | 91 +++++++++++++++++++++++------ 2 files changed, 112 insertions(+), 57 deletions(-) diff --git a/src/fastapi_toolsets/crud/search.py b/src/fastapi_toolsets/crud/search.py index 402a472..8836df5 100644 --- a/src/fastapi_toolsets/crud/search.py +++ b/src/fastapi_toolsets/crud/search.py @@ -1,10 +1,11 @@ """Search utilities for AsyncCrud.""" import functools -from collections.abc import Iterable, Sequence +from collections.abc import Callable, Iterable, Sequence from dataclasses import dataclass, replace from typing import TYPE_CHECKING, Any, Literal +from pydantic import TypeAdapter from sqlalchemy import ( Column, String, @@ -597,6 +598,41 @@ def _coerce_bool(value: Any) -> bool: raise ValueError(f"Cannot coerce {value!r} to bool") +def _coerce_enum(enum_class: Any, value: Any) -> Any: + """Look up an enum member by name ("PENDING", "RED") unless given one.""" + return value if isinstance(value, enum_class) else enum_class[value] + + +@functools.cache +def _validator(python_type: type) -> Callable[[Any], Any]: + """Pydantic validator for *python_type*, built once per type.""" + return TypeAdapter(python_type).validate_python + + +def _coercer(col_type: Any) -> Callable[[Any], Any]: + """Convert a filter value, often a query string, to the column's Python type.""" + if isinstance(col_type, Boolean): + return _coerce_bool + if isinstance(col_type, Enum) and col_type.enum_class is not None: + return functools.partial(_coerce_enum, col_type.enum_class) + if isinstance(col_type, String) or not isinstance(col_type, _EQUALITY_TYPES): + return lambda value: value + return _validator(col_type.python_type) + + +def _filter_condition(key: str, column: Any, value: Any) -> "ColumnElement[bool]": + """Equality, IN or array overlap condition on *column* for *value*.""" + col_type = column.property.columns[0].type + if not isinstance(col_type, (ARRAY, Boolean, *_EQUALITY_TYPES)): + raise UnsupportedFacetTypeError(key, type(col_type).__name__) + coerce = _coercer(col_type) + many = isinstance(value, list) + coerced = [coerce(v) for v in value] if many else coerce(value) + if isinstance(col_type, ARRAY): + return column.overlap(coerced) if many else any_(column) == coerced + return column.in_(coerced) if many else column == coerced + + def build_filter_by( filter_by: dict[str, Any], facet_fields: Sequence[FacetFieldType], @@ -630,45 +666,7 @@ def build_filter_by( if key not in index: raise InvalidFacetFilterError(key, valid_keys) - column, _ = index[key] - - col_type = column.property.columns[0].type - if isinstance(col_type, Boolean): - coerce = _coerce_bool - if isinstance(value, list): - filters[key] = column.in_([coerce(v) for v in value]) - else: - filters[key] = column == coerce(value) - elif isinstance(col_type, ARRAY): - if isinstance(value, list): - filters[key] = column.overlap(value) - else: - filters[key] = any_(column) == value - elif isinstance(col_type, Enum): - enum_class = col_type.enum_class - if enum_class is not None: - - def _coerce_enum(v: Any, enum_class: Any = enum_class) -> Any: - if isinstance(v, enum_class): - return v - return enum_class[v] # lookup by name: "PENDING", "RED" - - if isinstance(value, list): - filters[key] = column.in_([_coerce_enum(v) for v in value]) - else: - filters[key] = column == _coerce_enum(value) - else: # pragma: no cover - if isinstance(value, list): - filters[key] = column.in_(value) - else: - filters[key] = column == value - elif isinstance(col_type, _EQUALITY_TYPES): - if isinstance(value, list): - filters[key] = column.in_(value) - else: - filters[key] = column == value - else: - raise UnsupportedFacetTypeError(key, type(col_type).__name__) + filters[key] = _filter_condition(key, index[key][0], value) paths = {key: index[key][1] for key in filters} to_one = [key for key in filters if not is_to_many(paths[key])] diff --git a/tests/test_crud_search.py b/tests/test_crud_search.py index ea17f2a..6e87e19 100644 --- a/tests/test_crud_search.py +++ b/tests/test_crud_search.py @@ -55,6 +55,7 @@ ArticleCrud, ArticleRead, Color, + Event, EventCreate, EventCrud, EventDateCursorCrud, @@ -73,6 +74,7 @@ Post, PostCrud, PostWithTagsRead, + Product, ProductCreate, ProductCrud, ProductNumericCursorCrud, @@ -313,6 +315,28 @@ async def _orders(session: AsyncSession) -> None: ) +async def _products(session: AsyncSession, count: int) -> None: + """Products p0, p1, ... priced 1.99, 2.99, ...""" + for i in range(count): + await ProductCrud.create( + session, ProductCreate(name=f"p{i}", price=decimal.Decimal(f"{i + 1}.99")) + ) + + +async def _events(session: AsyncSession, count: int) -> None: + """Events e0, e1, ... an hour and a day apart from 2024-01-01.""" + base = datetime.datetime(2024, 1, 1) + for i in range(count): + await EventCrud.create( + session, + EventCreate( + name=f"e{i}", + occurred_at=base + datetime.timedelta(hours=i), + scheduled_date=datetime.date(2024, 1, i + 1), + ), + ) + + def _aggregates(statements: list[str]) -> list[str]: return [sql for sql in statements if "array_agg" in sql] @@ -497,24 +521,13 @@ async def test_order_by_and_load_options_apply(self, db_session): async def test_cursor_values_round_trip_for_each_column_type( self, db_session, kind ): - base = datetime.datetime(2024, 1, 1) - for i in range(5): - if kind == "integer": + if kind == "integer": + for i in range(5): await IntRoleCursorCrud.create(db_session, IntRoleCreate(name=f"r{i}")) - elif kind == "numeric": - await ProductCrud.create( - db_session, - ProductCreate(name=f"p{i}", price=decimal.Decimal(f"{i + 1}.99")), - ) - else: - await EventCrud.create( - db_session, - EventCreate( - name=f"e{i}", - occurred_at=base + datetime.timedelta(hours=i), - scheduled_date=datetime.date(2024, 1, i + 1), - ), - ) + elif kind == "numeric": + await _products(db_session, 5) + else: + await _events(db_session, 5) crud, schema = { "integer": (IntRoleCursorCrud, IntRoleRead), "datetime": (EventDateTimeCursorCrud, EventRead), @@ -1650,6 +1663,8 @@ async def test_array_filters_check_containment_or_overlap(self, db_session): ({"color": ["RED"]}, {"order-1", "order-3"}), ({"priority": OrderStatus.PENDING}, {"order-1", "order-3"}), ({"priority": 3}, {"order-2"}), + ({"priority": "3"}, {"order-2"}), + ({"priority": ["1", "3"]}, {"order-1", "order-2", "order-3"}), ({"status": 1}, KeyError), ({"status": [1, 3]}, KeyError), ], @@ -1662,6 +1677,8 @@ async def test_array_filters_check_containment_or_overlap(self, db_session): "str-enum-names", "integer-with-member", "integer-with-int", + "integer-with-str", + "integer-with-strs", "int-enum-plain-int", "int-enum-plain-list", ], @@ -1675,6 +1692,46 @@ async def test_enum_and_integer_filters(self, db_session, filter_by, expected): ) assert {o.name for o in result.data} == expected + @pytest.mark.anyio + @pytest.mark.parametrize( + ("seed", "crud", "schema", "filter_by", "expected"), + [ + ( + _products, + CrudFactory(Product, facet_fields=[Product.price]), + ProductRead, + {"price": ["1.99", "3.99"]}, + {"p0", "p2"}, + ), + ( + _events, + CrudFactory(Event, facet_fields=[Event.scheduled_date]), + EventRead, + {"scheduled_date": "2024-01-02"}, + {"e1"}, + ), + ( + _events, + CrudFactory(Event, facet_fields=[Event.occurred_at]), + EventRead, + {"occurred_at": ["2024-01-01T00:00:00"]}, + {"e0"}, + ), + ], + ids=["numeric", "date", "datetime"], + ) + async def test_string_values_coerced_to_column_type( + self, db_session, seed, crud, schema, filter_by, expected + ): + """Query parameters arrive as strings, whatever the column type.""" + await seed(db_session, 3) + + result = await crud.offset_paginate( + db_session, filter_by=filter_by, schema=schema + ) + + assert {row.name for row in result.data} == expected + class TestParams: """The FastAPI dependencies built by *_paginate_params().""" From d0e32da8ce4a49e4827e1070bcc2488417e2fad4 Mon Sep 17 00:00:00 2001 From: d3vyce Date: Thu, 22 Oct 2026 13:26:59 -0400 Subject: [PATCH 2/9] feat: answer invalid facet filter values with a 400 InvalidFacetValueError --- docs/module/crud.md | 2 +- docs/module/exceptions.md | 1 + docs/reference/exceptions.md | 3 ++ src/fastapi_toolsets/crud/__init__.py | 2 + src/fastapi_toolsets/crud/search.py | 7 ++- src/fastapi_toolsets/exceptions/__init__.py | 2 + src/fastapi_toolsets/exceptions/exceptions.py | 22 +++++++++ tests/test_crud_search.py | 47 +++++++++++++++---- tests/test_exceptions.py | 9 ++++ 9 files changed, 83 insertions(+), 12 deletions(-) diff --git a/docs/module/crud.md b/docs/module/crud.md index 544ba94..dd51332 100644 --- a/docs/module/crud.md +++ b/docs/module/crud.md @@ -529,7 +529,7 @@ The distinct values for each facet field are returned in the `filter_attributes` ``` !!! info "Key format uses `__` as a separator for relationship chains." - A direct column `User.status` produces `"status"`. A relationship tuple `(User.role, Role.name)` produces `"role__name"`. A deeper chain `(User.role, Role.permission, Permission.name)` produces `"role__permission__name"`. An unknown `filter_by` key raises [`InvalidFacetFilterError`](../reference/exceptions.md#fastapi_toolsets.exceptions.exceptions.InvalidFacetFilterError) (HTTP 422). + A direct column `User.status` produces `"status"`. A relationship tuple `(User.role, Role.name)` produces `"role__name"`. A deeper chain `(User.role, Role.permission, Permission.name)` produces `"role__permission__name"`. An unknown `filter_by` key raises [`InvalidFacetFilterError`](../reference/exceptions.md#fastapi_toolsets.exceptions.exceptions.InvalidFacetFilterError) (HTTP 400). Values are converted to the column type, so query strings like `"42"` or `"2026-01-02"` work on integer and date columns; a value that does not convert raises [`InvalidFacetValueError`](../reference/exceptions.md#fastapi_toolsets.exceptions.exceptions.InvalidFacetValueError) (HTTP 400). #### Skipping facet queries diff --git a/docs/module/exceptions.md b/docs/module/exceptions.md index 1545ff6..3e29298 100644 --- a/docs/module/exceptions.md +++ b/docs/module/exceptions.md @@ -39,6 +39,7 @@ It also patches `app.openapi()` to replace the default Pydantic 422 schema with | [`ConflictError`](../reference/exceptions.md#fastapi_toolsets.exceptions.exceptions.ConflictError) | 409 | Conflict | | [`NoSearchableFieldsError`](../reference/exceptions.md#fastapi_toolsets.exceptions.exceptions.NoSearchableFieldsError) | 400 | No Searchable Fields | | [`InvalidFacetFilterError`](../reference/exceptions.md#fastapi_toolsets.exceptions.exceptions.InvalidFacetFilterError) | 400 | Invalid Facet Filter | +| [`InvalidFacetValueError`](../reference/exceptions.md#fastapi_toolsets.exceptions.exceptions.InvalidFacetValueError) | 400 | Invalid Facet Value | | [`InvalidOrderFieldError`](../reference/exceptions.md#fastapi_toolsets.exceptions.exceptions.InvalidOrderFieldError) | 422 | Invalid Order Field | | [`PoolExhaustedError`](../reference/exceptions.md#fastapi_toolsets.exceptions.exceptions.PoolExhaustedError) | 503 | Service Unavailable | | [`LockTimeoutError`](../reference/exceptions.md#fastapi_toolsets.exceptions.exceptions.LockTimeoutError) | 503 | Service Unavailable | diff --git a/docs/reference/exceptions.md b/docs/reference/exceptions.md index b457358..7018cd5 100644 --- a/docs/reference/exceptions.md +++ b/docs/reference/exceptions.md @@ -14,6 +14,7 @@ from fastapi_toolsets.exceptions import ( NoSearchableFieldsError, InvalidSearchColumnError, InvalidFacetFilterError, + InvalidFacetValueError, InvalidOrderFieldError, PoolExhaustedError, LockTimeoutError, @@ -45,6 +46,8 @@ from fastapi_toolsets.exceptions import ( ## ::: fastapi_toolsets.exceptions.exceptions.InvalidFacetFilterError +## ::: fastapi_toolsets.exceptions.exceptions.InvalidFacetValueError + ## ::: fastapi_toolsets.exceptions.exceptions.InvalidOrderFieldError ## ::: fastapi_toolsets.exceptions.exceptions.PoolExhaustedError diff --git a/src/fastapi_toolsets/crud/__init__.py b/src/fastapi_toolsets/crud/__init__.py index 26b9ea3..2ea360d 100644 --- a/src/fastapi_toolsets/crud/__init__.py +++ b/src/fastapi_toolsets/crud/__init__.py @@ -2,6 +2,7 @@ from ..exceptions import ( InvalidFacetFilterError, + InvalidFacetValueError, InvalidSearchColumnError, NoSearchableFieldsError, UnsupportedFacetTypeError, @@ -23,6 +24,7 @@ "CrudFactory", "FacetFieldType", "InvalidFacetFilterError", + "InvalidFacetValueError", "InvalidSearchColumnError", "JoinType", "M2MFieldType", diff --git a/src/fastapi_toolsets/crud/search.py b/src/fastapi_toolsets/crud/search.py index 8836df5..51c1687 100644 --- a/src/fastapi_toolsets/crud/search.py +++ b/src/fastapi_toolsets/crud/search.py @@ -36,6 +36,7 @@ from .._orm import is_to_many, key_expr from ..exceptions import ( InvalidFacetFilterError, + InvalidFacetValueError, InvalidSearchColumnError, NoSearchableFieldsError, UnsupportedFacetTypeError, @@ -627,7 +628,10 @@ def _filter_condition(key: str, column: Any, value: Any) -> "ColumnElement[bool] raise UnsupportedFacetTypeError(key, type(col_type).__name__) coerce = _coercer(col_type) many = isinstance(value, list) - coerced = [coerce(v) for v in value] if many else coerce(value) + try: + coerced = [coerce(v) for v in value] if many else coerce(value) + except (ValueError, KeyError) as exc: + raise InvalidFacetValueError(key, value) from exc if isinstance(col_type, ARRAY): return column.overlap(coerced) if many else any_(column) == coerced return column.in_(coerced) if many else column == coerced @@ -653,6 +657,7 @@ def build_filter_by( Raises: InvalidFacetFilterError: If a key in filter_by is not a declared facet field + InvalidFacetValueError: If a value cannot be converted to the column type """ index: dict[str, tuple[InstrumentedAttribute[Any], tuple[Any, ...]]] = { key: (_field_column(field), _field_rels(field)) diff --git a/src/fastapi_toolsets/exceptions/__init__.py b/src/fastapi_toolsets/exceptions/__init__.py index fc13584..8f1de41 100644 --- a/src/fastapi_toolsets/exceptions/__init__.py +++ b/src/fastapi_toolsets/exceptions/__init__.py @@ -10,6 +10,7 @@ ForeignKeyViolationError, IntegrityViolationError, InvalidFacetFilterError, + InvalidFacetValueError, InvalidOrderFieldError, InvalidSearchColumnError, LockTimeoutError, @@ -35,6 +36,7 @@ "ForeignKeyViolationError", "IntegrityViolationError", "InvalidFacetFilterError", + "InvalidFacetValueError", "InvalidOrderFieldError", "InvalidSearchColumnError", "LockTimeoutError", diff --git a/src/fastapi_toolsets/exceptions/exceptions.py b/src/fastapi_toolsets/exceptions/exceptions.py index 29f8fe3..e8f45b8 100644 --- a/src/fastapi_toolsets/exceptions/exceptions.py +++ b/src/fastapi_toolsets/exceptions/exceptions.py @@ -146,6 +146,28 @@ def __init__(self, key: str, valid_keys: set[str]) -> None: ) +class InvalidFacetValueError(ApiException): + """Raised when a filter_by value cannot be converted to the facet column type.""" + + api_error = ApiError( + code=400, + msg="Invalid Facet Value", + desc="One or more filter_by values do not match the facet column type.", + err_code="FACET-VALUE-400", + ) + + def __init__(self, key: str, value: Any) -> None: + """Initialize the exception. + + Args: + key: The facet field key. + value: The value that could not be converted. + """ + self.key = key + self.value = value + super().__init__(desc=f"Invalid value {value!r} for facet field '{key}'.") + + class UnsupportedFacetTypeError(ApiException): """Raised when a facet field has a column type not supported by filter_by.""" diff --git a/tests/test_crud_search.py b/tests/test_crud_search.py index 6e87e19..3c82ada 100644 --- a/tests/test_crud_search.py +++ b/tests/test_crud_search.py @@ -23,6 +23,7 @@ from fastapi_toolsets.crud import ( CrudFactory, InvalidFacetFilterError, + InvalidFacetValueError, InvalidSearchColumnError, SearchConfig, UnsupportedFacetTypeError, @@ -96,7 +97,6 @@ create_user, created_tables, post_tags, - raises_if, ) # Models only compiled to SQL, never created in the database. @@ -1665,8 +1665,6 @@ async def test_array_filters_check_containment_or_overlap(self, db_session): ({"priority": 3}, {"order-2"}), ({"priority": "3"}, {"order-2"}), ({"priority": ["1", "3"]}, {"order-1", "order-2", "order-3"}), - ({"status": 1}, KeyError), - ({"status": [1, 3]}, KeyError), ], ids=[ "int-enum-member", @@ -1679,18 +1677,16 @@ async def test_array_filters_check_containment_or_overlap(self, db_session): "integer-with-int", "integer-with-str", "integer-with-strs", - "int-enum-plain-int", - "int-enum-plain-list", ], ) async def test_enum_and_integer_filters(self, db_session, filter_by, expected): await _orders(db_session) - with raises_if(KeyError, expected is KeyError): - result = await OrderFacetCrud.offset_paginate( - db_session, filter_by=filter_by, schema=OrderRead - ) - assert {o.name for o in result.data} == expected + result = await OrderFacetCrud.offset_paginate( + db_session, filter_by=filter_by, schema=OrderRead + ) + + assert {o.name for o in result.data} == expected @pytest.mark.anyio @pytest.mark.parametrize( @@ -1732,6 +1728,37 @@ async def test_string_values_coerced_to_column_type( assert {row.name for row in result.data} == expected + @pytest.mark.parametrize( + ("field", "value"), + [ + (Order.status, 1), + (Order.status, [1, 3]), + (Order.status, "LOST"), + (Order.priority, "high"), + (Order.priority, ["1", "1.5"]), + (User.is_active, "maybe"), + ((User.role, Role.id), ["not-a-uuid"]), + (Event.scheduled_date, "tomorrow"), + ], + ids=[ + "int-enum-plain-int", + "int-enum-plain-list", + "int-enum-unknown-name", + "integer-not-a-number", + "integer-list-with-float", + "bool", + "uuid", + "date", + ], + ) + def test_invalid_value_raises(self, field, value): + key = facet_keys([field])[0] + + with pytest.raises(InvalidFacetValueError) as exc: + build_filter_by({key: value}, [field]) + + assert exc.value.key == key and exc.value.value == value + class TestParams: """The FastAPI dependencies built by *_paginate_params().""" diff --git a/tests/test_exceptions.py b/tests/test_exceptions.py index 883f5fb..84a0eb5 100644 --- a/tests/test_exceptions.py +++ b/tests/test_exceptions.py @@ -18,6 +18,7 @@ ForbiddenError, ForeignKeyViolationError, InvalidFacetFilterError, + InvalidFacetValueError, InvalidOrderFieldError, InvalidSearchColumnError, LockTimeoutError, @@ -127,6 +128,14 @@ class _Count(BaseModel): "Valid keys: ['shape', 'size']", id="invalid-facet-filter", ), + pytest.param( + InvalidFacetValueError("priority", "high"), + 400, + "FACET-VALUE-400", + "Invalid Facet Value", + "Invalid value 'high' for facet field 'priority'", + id="invalid-facet-value", + ), pytest.param( UnsupportedFacetTypeError("blob", "LargeBinary"), 400, From 5379f7ac9a4ff092e38b439cd723fb4be95aaf04 Mon Sep 17 00:00:00 2001 From: d3vyce Date: Thu, 22 Oct 2026 13:32:19 -0400 Subject: [PATCH 3/9] fix: subquery to-many filter paths that would join the same table twice --- src/fastapi_toolsets/crud/search.py | 14 ++++-- tests/test_crud_search.py | 71 +++++++++++++++++++++++++---- 2 files changed, 73 insertions(+), 12 deletions(-) diff --git a/src/fastapi_toolsets/crud/search.py b/src/fastapi_toolsets/crud/search.py index 51c1687..24c4998 100644 --- a/src/fastapi_toolsets/crud/search.py +++ b/src/fastapi_toolsets/crud/search.py @@ -675,9 +675,17 @@ def build_filter_by( paths = {key: index[key][1] for key in filters} to_one = [key for key in filters if not is_to_many(paths[key])] - to_many = [rel for key in filters if key not in to_one for rel in paths[key]] - moves = _moves([(paths[k], filters[k]) for k in to_one], [*other_joins, *to_many]) - moved = {key for key, move in zip(to_one, moves) if move} + to_many = [key for key in filters if key not in to_one] + + def moving(keys: list[str], joins: Sequence[Any]) -> set[str]: + moves = _moves([(paths[k], filters[k]) for k in keys], joins) + return {key for key, move in zip(keys, moves) if move} + + # To-many paths stay joined, so their filters match the same related + # row, unless the join would repeat a table. + moved = moving(to_many, other_joins) + joined = [rel for key in to_many if key not in moved for rel in paths[key]] + moved |= moving(to_one, [*other_joins, *joined]) for key in moved: filters[key] = _semi_join_tree([(paths[key], filters[key])], "all")[0] joins = [rel for key in filters if key not in moved for rel in paths[key]] diff --git a/tests/test_crud_search.py b/tests/test_crud_search.py index 3c82ada..62d5238 100644 --- a/tests/test_crud_search.py +++ b/tests/test_crud_search.py @@ -315,6 +315,20 @@ async def _orders(session: AsyncSession) -> None: ) +async def _league(session: AsyncSession, *players: tuple[str, int]) -> None: + """Teams ab, bc, ca, *players* as (name, team), matches ab-bc, ca-ab, bc-ca.""" + session.add_all(_Team(id=i, name=name) for i, name in enumerate(["ab", "bc", "ca"])) + await session.flush() + session.add_all( + _Player(id=i, name=name, team_id=team) for i, (name, team) in enumerate(players) + ) + session.add_all( + _Match(id=i, home_id=h, away_id=a) + for i, (h, a) in enumerate([(0, 1), (2, 0), (1, 2)]) + ) + await session.commit() + + async def _products(session: AsyncSession, count: int) -> None: """Products p0, p1, ... priced 1.99, 2.99, ...""" for i in range(count): @@ -892,15 +906,7 @@ async def test_relationships_to_one_table_are_searched_filtered_and_faceted( created_tables(engine, _LocalBase.metadata), async_sessionmaker(engine, expire_on_commit=False)() as session, ): - session.add_all( - _Team(id=i, name=name) for i, name in enumerate(["ab", "bc", "ca"]) - ) - await session.flush() - session.add_all( - _Match(id=i, home_id=h, away_id=a) - for i, (h, a) in enumerate([(0, 1), (2, 0), (1, 2)]) - ) - await session.commit() + await _league(session) async def ids(**kwargs: Any) -> list[int]: page = await _paginate( @@ -934,6 +940,36 @@ async def ids(**kwargs: Any) -> list[int]: } assert sorted(ordered) == [0, 1] + @pytest.mark.anyio + @_methods + async def test_to_many_filter_with_order_on_the_same_table(self, engine, method): + players = (_Match.home, _Team.players, _Player.name) + crud = CrudFactory( + _Match, + searchable_fields=[players], + facet_fields=[players], + cursor_column=_Match.id, + ) + async with ( + created_tables(engine, _LocalBase.metadata), + async_sessionmaker(engine, expire_on_commit=False)() as session, + ): + await _league(session, ("ann", 0), ("bob", 1), ("bea", 2)) + + page = await _paginate( + crud, + method, + session, + search="b", + filter_by={"home__players__name": ["bob"]}, + order_by=_Team.name, + order_joins=[_Match.away], + schema=_MatchRead, + ) + + assert [m.id for m in page.data] == [2] + assert page.filter_attributes == {"home__players__name": ["bea", "bob"]} + def test_searchable_fields_are_detected_from_the_mapper(self): user_fields = get_searchable_fields(User) role_fields = get_searchable_fields(Role) @@ -1141,6 +1177,23 @@ def test_to_many_filters_stay_joined_when_other_filters_move(self): assert "IN (SELECT" not in str(filters["home__players__name"]) assert "IN (SELECT" in str(filters["away__name"]) + def test_to_many_filters_move_when_they_would_repeat_a_table(self): + facets = [(_Match.home, _Team.players, _Player.name), (_Match.away, _Team.name)] + + filters, joins = build_filter_by( + {"home__players__name": "a"}, facets, other_joins=[_Match.away] + ) + both, both_joins = build_filter_by( + {"home__players__name": "a", "away__name": "b"}, + facets, + other_joins=[_Match.away], + ) + + assert joins == [] + assert "IN (SELECT" in str(filters["home__players__name"]) + assert both_joins == [_Match.away] + assert "IN (SELECT" not in str(both["away__name"]) + _fan_out = pytest.mark.parametrize( "fan_out", From eae49c96e60260231ec09381ca4c8e542d871735 Mon Sep 17 00:00:00 2001 From: d3vyce Date: Thu, 22 Oct 2026 13:36:22 -0400 Subject: [PATCH 4/9] fix: coerce array facet filter values to the item type --- src/fastapi_toolsets/crud/search.py | 2 ++ tests/test_crud_search.py | 50 ++++++++++++++++++++++++++++- 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/src/fastapi_toolsets/crud/search.py b/src/fastapi_toolsets/crud/search.py index 24c4998..351b66b 100644 --- a/src/fastapi_toolsets/crud/search.py +++ b/src/fastapi_toolsets/crud/search.py @@ -616,6 +616,8 @@ def _coercer(col_type: Any) -> Callable[[Any], Any]: return _coerce_bool if isinstance(col_type, Enum) and col_type.enum_class is not None: return functools.partial(_coerce_enum, col_type.enum_class) + if isinstance(col_type, ARRAY): + return _coercer(col_type.item_type) if isinstance(col_type, String) or not isinstance(col_type, _EQUALITY_TYPES): return lambda value: value return _validator(col_type.python_type) diff --git a/tests/test_crud_search.py b/tests/test_crud_search.py index 62d5238..3c16339 100644 --- a/tests/test_crud_search.py +++ b/tests/test_crud_search.py @@ -8,7 +8,16 @@ import pytest from pydantic import BaseModel -from sqlalchemy import ForeignKey, ForeignKeyConstraint, Index, String, and_, select +from sqlalchemy import ( + ForeignKey, + ForeignKeyConstraint, + Index, + Integer, + String, + and_, + select, +) +from sqlalchemy.dialects.postgresql import ARRAY from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker from sqlalchemy.orm import ( DeclarativeBase, @@ -32,6 +41,7 @@ from fastapi_toolsets.crud.factory import _CursorDirection, _encode_cursor from fastapi_toolsets.crud.search import ( _coerce_bool, + _coercer, _facet_rows, build_filter_by, build_search_filters, @@ -220,6 +230,13 @@ class _Match(_LocalBase): away: Mapped[_Team] = relationship(foreign_keys=[away_id]) +class _Crate(_LocalBase): + __tablename__ = "crates" + + id: Mapped[int] = mapped_column(primary_key=True) + sizes: Mapped[list[int]] = mapped_column(ARRAY(Integer)) + + def _sql(model, filters) -> str: return str(select(*model.__mapper__.primary_key).where(*filters)) @@ -246,6 +263,10 @@ class _MatchRead(PydanticBase): id: int +class _CrateRead(PydanticBase): + id: int + + class _UserFilter(BaseModel): username: str | None = None is_active: bool | None = None @@ -1792,6 +1813,7 @@ async def test_string_values_coerced_to_column_type( (User.is_active, "maybe"), ((User.role, Role.id), ["not-a-uuid"]), (Event.scheduled_date, "tomorrow"), + (_Crate.sizes, ["x"]), ], ids=[ "int-enum-plain-int", @@ -1802,6 +1824,7 @@ async def test_string_values_coerced_to_column_type( "bool", "uuid", "date", + "array", ], ) def test_invalid_value_raises(self, field, value): @@ -1812,6 +1835,31 @@ def test_invalid_value_raises(self, field, value): assert exc.value.key == key and exc.value.value == value + @pytest.mark.anyio + @pytest.mark.parametrize( + ("value", "expected"), + [(["1", "4"], [0, 1]), ("3", [1])], + ids=["overlap", "contains"], + ) + async def test_array_values_coerced_to_item_type(self, engine, value, expected): + crud = CrudFactory(_Crate, facet_fields=[_Crate.sizes]) + async with ( + created_tables(engine, _LocalBase.metadata), + async_sessionmaker(engine, expire_on_commit=False)() as session, + ): + session.add_all([_Crate(id=0, sizes=[1, 2]), _Crate(id=1, sizes=[3, 4])]) + await session.commit() + + page = await crud.offset_paginate( + session, filter_by={"sizes": value}, schema=_CrateRead + ) + + assert sorted(c.id for c in page.data) == expected + + def test_array_items_follow_the_item_type_rules(self): + assert _coercer(ARRAY(Order.color.type))("RED") is Color.RED + assert _coercer(ARRAY(User.is_active.type))("true") is True + class TestParams: """The FastAPI dependencies built by *_paginate_params().""" From e5d9e98ac2d35a8630ef06a2480d9c46efa569a1 Mon Sep 17 00:00:00 2001 From: d3vyce Date: Thu, 22 Oct 2026 13:59:38 -0400 Subject: [PATCH 5/9] fix: match LIKE wildcards in search queries literally --- src/fastapi_toolsets/crud/search.py | 9 ++++----- tests/test_crud_search.py | 23 +++++++++++++++++++++-- 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/src/fastapi_toolsets/crud/search.py b/src/fastapi_toolsets/crud/search.py index 351b66b..d93f358 100644 --- a/src/fastapi_toolsets/crud/search.py +++ b/src/fastapi_toolsets/crud/search.py @@ -127,7 +127,6 @@ class SearchPlan: _INTEGER_CHARS = frozenset("0123456789-") _UUID_CHARS = frozenset("0123456789abcdef-") -_LIKE_SPECIAL = frozenset("%_\\") _Entry = tuple[tuple[Any, ...], "ColumnElement[bool]"] @@ -241,8 +240,6 @@ def _untangle( def _can_contain(column: Any, chars: set[str]) -> bool: """False when the text of an integer or UUID column can never contain *chars*.""" - if chars & _LIKE_SPECIAL: - return True col_type = column.type if isinstance(col_type, Integer): return chars <= _INTEGER_CHARS @@ -317,9 +314,11 @@ def _search_condition( if isinstance(column.type, String) and not isinstance(column.type, Enum) else column.cast(String) ) + # Backslash is the default LIKE escape, so the query matches literally. + escaped = query.replace("\\", "\\\\").replace("%", "\\%").replace("_", "\\_") if config.case_sensitive: - return column_as_string.like(f"%{query}%") - return column_as_string.ilike(f"%{query}%") + return column_as_string.like(f"%{escaped}%") + return column_as_string.ilike(f"%{escaped}%") def _is_plain_join(condition: Any, pairs: Sequence[tuple[Any, Any]]) -> bool: diff --git a/tests/test_crud_search.py b/tests/test_crud_search.py index 3c16339..617b7e8 100644 --- a/tests/test_crud_search.py +++ b/tests/test_crud_search.py @@ -991,6 +991,25 @@ async def test_to_many_filter_with_order_on_the_same_table(self, engine, method) assert [m.id for m in page.data] == [2] assert page.filter_attributes == {"home__players__name": ["bea", "bob"]} + @pytest.mark.anyio + async def test_like_wildcards_match_literally(self, db_session): + crud = CrudFactory(Role, searchable_fields=[Role.name]) + for name in ["50% off", "a_b", "c\\d", "500 off", "axb"]: + await create_role(db_session, name) + + async def names(search: str) -> list[str]: + page = await crud.offset_paginate( + db_session, search=search, schema=RoleRead + ) + return [r.name for r in page.data] + + assert {s: await names(s) for s in ["%", "_", "\\", "0%"]} == { + "%": ["50% off"], + "_": ["a_b"], + "\\": ["c\\d"], + "0%": ["50% off"], + } + def test_searchable_fields_are_detected_from_the_mapper(self): user_fields = get_searchable_fields(User) role_fields = get_searchable_fields(Role) @@ -1110,8 +1129,8 @@ def test_casts_only_non_string_columns(self): (IntRole.id, "a1", False), (User.id, "AB-12", True), (User.id, "xyz", False), - (User.id, "x%", True), - (User.id, "x_", True), + (User.id, "1%", False), + (User.id, "1_", False), (User.id, SearchConfig(query="AB", case_sensitive=True), False), ], ids=[ From 314cdcd01e4f46a64410fdd6327b322cd4f491ca Mon Sep 17 00:00:00 2001 From: d3vyce Date: Thu, 22 Oct 2026 14:02:20 -0400 Subject: [PATCH 6/9] fix: subquery relationship paths into tables the caller already joins --- src/fastapi_toolsets/_orm.py | 18 ++++++++++++- src/fastapi_toolsets/crud/factory.py | 20 +++++++++++--- src/fastapi_toolsets/crud/search.py | 23 ++++++++++------ tests/test_crud_search.py | 40 ++++++++++++++++++++++++++++ 4 files changed, 88 insertions(+), 13 deletions(-) diff --git a/src/fastapi_toolsets/_orm.py b/src/fastapi_toolsets/_orm.py index 65bf02a..8cfdd9a 100644 --- a/src/fastapi_toolsets/_orm.py +++ b/src/fastapi_toolsets/_orm.py @@ -3,8 +3,9 @@ from collections.abc import Iterable, Sequence from typing import Any +from sqlalchemy import Table, tuple_ from sqlalchemy import inspect as sa_inspect -from sqlalchemy import tuple_ +from sqlalchemy.orm import Mapper def key_expr(columns: Sequence[Any]) -> Any: @@ -12,6 +13,21 @@ def key_expr(columns: Sequence[Any]) -> Any: return columns[0] if len(columns) == 1 else tuple_(*columns) +def joined_tables(targets: Iterable[Any]) -> list[Any]: + """Tables that joining *targets*, mapped classes or tables, brings in. + + An aliased target is left out: it can never repeat a table. + """ + tables: list[Any] = [] + for target in targets: + info = sa_inspect(target, raiseerr=False) + if isinstance(info, Mapper): + tables.extend(info.tables) + elif isinstance(info, Table): + tables.append(info) + return tables + + def is_to_many(rels: Iterable[Any]) -> bool: """True if the relationship path *rels* crosses a collection.""" return any(rel.property.uselist for rel in rels) diff --git a/src/fastapi_toolsets/crud/factory.py b/src/fastapi_toolsets/crud/factory.py index bc6b908..1fa20f7 100644 --- a/src/fastapi_toolsets/crud/factory.py +++ b/src/fastapi_toolsets/crud/factory.py @@ -41,7 +41,13 @@ from sqlalchemy.sql.roles import WhereHavingRole from sqlalchemy.sql.util import ClauseAdapter -from .._orm import is_expired, is_to_many, key_expr, loaded_relationships +from .._orm import ( + is_expired, + is_to_many, + joined_tables, + key_expr, + loaded_relationships, +) from ..db import transaction from ..exceptions import InvalidOrderFieldError, NotFoundError from ..schemas import ( @@ -487,6 +493,7 @@ def _query_plan( filter_by: dict[str, Any] | BaseModel | None, facet_fields: Sequence[FacetFieldType] | None, order_joins: Sequence[Any] | None, + joins: JoinType | None, ) -> _QueryPlan: """Combine the caller's filters, the search and ``filter_by`` into a plan. @@ -494,9 +501,12 @@ def _query_plan( individually, so ``fb_filters`` is left out of ``facet_base``. """ filters = list(filters) if filters else [] - order_joins = order_joins or () + other_joins = [ + *(order_joins or ()), + *joined_tables(target for target, _ in joins or ()), + ] fb_filters, fb_joins = cls._prepare_filter_by( - filter_by, facet_fields, other_joins=order_joins + filter_by, facet_fields, other_joins=other_joins ) plan = ( build_search_plan( @@ -505,7 +515,7 @@ def _query_plan( search_fields, cls.searchable_fields, search_column, - other_joins=[*order_joins, *fb_joins], + other_joins=[*other_joins, *fb_joins], ) if search else None @@ -1679,6 +1689,7 @@ async def offset_paginate( filter_by=filter_by, facet_fields=facet_fields, order_joins=order_joins, + joins=joins, ) q = select(cls.model) q = _apply_joins(q, joins, outer_join) @@ -1811,6 +1822,7 @@ async def cursor_paginate( filter_by=filter_by, facet_fields=facet_fields, order_joins=order_joins, + joins=joins, ) # Rows sharing a cursor value are ordered, and resumed, by primary key. tiebreak = cls._tiebreak_attrs(cursor_column) diff --git a/src/fastapi_toolsets/crud/search.py b/src/fastapi_toolsets/crud/search.py index d93f358..f007de1 100644 --- a/src/fastapi_toolsets/crud/search.py +++ b/src/fastapi_toolsets/crud/search.py @@ -21,6 +21,7 @@ from sqlalchemy.orm.attributes import InstrumentedAttribute from sqlalchemy.sql import operators, visitors from sqlalchemy.sql.elements import BinaryExpression +from sqlalchemy.sql.selectable import FromClause from sqlalchemy.types import ( ARRAY, Boolean, @@ -149,7 +150,7 @@ def build_search_plan( default_fields: Default fields (from ClassVar) search_column: Optional key to narrow search to a single field. Must match one of the resolved search field keys. - other_joins: Relationships the query joins besides the search. + other_joins: Relationships, or tables, the query joins besides the search. Raises: NoSearchableFieldsError: If no searchable field has been configured @@ -199,15 +200,21 @@ def build_search_plan( def _joins_collide(paths: Sequence[Sequence[Any]], other_joins: Sequence[Any]) -> bool: - """True if joining *paths* after *other_joins* would repeat a table.""" + """True if joining *paths* after *other_joins* would repeat a table. + + *other_joins* holds relationships, and tables the query joins directly. + """ seen = {t for rels in paths if rels for t in rels[0].property.parent.tables} - for rel in unique_relationships( + for join in unique_relationships( [*other_joins, *(r for rels in paths for r in rels)] ): - prop = rel.property - tables = [*prop.mapper.tables] - if prop.secondary is not None: - tables.append(prop.secondary) + if isinstance(join, FromClause): + tables = [join] + else: + prop = join.property + tables = [*prop.mapper.tables] + if prop.secondary is not None: + tables.append(prop.secondary) if seen.intersection(tables): return True seen.update(tables) @@ -649,7 +656,7 @@ def build_filter_by( Args: filter_by: Mapping of column key to scalar value or list of values facet_fields: Declared facet fields to validate keys against - other_joins: Relationships the query joins besides the filters + other_joins: Relationships, or tables, the query joins besides the filters Returns: Tuple of ({facet_key: filter_condition}, joins_needed). One filter diff --git a/tests/test_crud_search.py b/tests/test_crud_search.py index 617b7e8..f9d65fb 100644 --- a/tests/test_crud_search.py +++ b/tests/test_crud_search.py @@ -29,6 +29,7 @@ ) from sqlalchemy.sql.elements import UnaryExpression +from fastapi_toolsets._orm import joined_tables from fastapi_toolsets.crud import ( CrudFactory, InvalidFacetFilterError, @@ -1010,6 +1011,28 @@ async def names(search: str) -> list[str]: "0%": ["50% off"], } + @pytest.mark.anyio + @_methods + async def test_caller_joins_on_a_searched_and_filtered_table( + self, db_session, method + ): + admin = await create_role(db_session, "admin") + await _users(db_session, "alice", role=admin) + await _users(db_session, "bob") + + page = await _paginate( + UserSearchCrud, + method, + db_session, + joins=[(Role, User.role_id == Role.id)], + search="adm", + filter_by={"role__name": ["admin"]}, + schema=UserRead, + ) + + assert [u.username for u in page.data] == ["alice"] + assert page.filter_attributes["role__name"] == ["admin"] + def test_searchable_fields_are_detected_from_the_mapper(self): user_fields = get_searchable_fields(User) role_fields = get_searchable_fields(Role) @@ -1183,6 +1206,23 @@ def test_joins_that_would_repeat_a_table_are_subqueried(self): assert after_filter.page_joins == [] assert self_ref.page_joins == [] and self_ref.agg_joins == [] + def test_tables_joined_by_the_caller_count_as_joined(self): + facets = [(_Match.home, _Team.name)] + + filters, joins = build_filter_by( + {"home__name": "a"}, facets, other_joins=joined_tables([_Team]) + ) + plan = build_search_plan( + _Match, "x", facets, other_joins=joined_tables([_Team.__table__]) + ) + aliased_team, _ = build_filter_by( + {"home__name": "a"}, facets, other_joins=joined_tables([aliased(_Team)]) + ) + + assert joins == [] and "IN (SELECT" in str(filters["home__name"]) + assert plan.page_joins == [] + assert "IN (SELECT" not in str(aliased_team["home__name"]) + def test_filter_by_joins_that_would_repeat_a_table_are_subqueried(self): facets = [(_Match.home, _Team.name), (_Match.away, _Team.name)] From b7e0c5de3220eab78410d7c99658931ced102bab Mon Sep 17 00:00:00 2001 From: d3vyce Date: Thu, 22 Oct 2026 14:03:15 -0400 Subject: [PATCH 7/9] fix: reserve the order params before checking facet keys --- src/fastapi_toolsets/crud/factory.py | 53 +++++++++++++++------------- tests/test_crud_search.py | 8 +++++ 2 files changed, 36 insertions(+), 25 deletions(-) diff --git a/src/fastapi_toolsets/crud/factory.py b/src/fastapi_toolsets/crud/factory.py index 1fa20f7..91ee8b6 100644 --- a/src/fastapi_toolsets/crud/factory.py +++ b/src/fastapi_toolsets/crud/factory.py @@ -706,6 +706,11 @@ def _build_paginate_params( ) reserved_names.update({"search", "search_column"}) + # Reserved before the facets are checked, though added after them. + resolved_order = cls._resolve_order_fields(order_fields) if order else None + if resolved_order: + reserved_names.update({"order_by", "order"}) + filter_keys: list[str] | None = None if filter: resolved_facets = cls._resolve_facet_fields(facet_fields) @@ -730,32 +735,30 @@ def _build_paginate_params( order_field_map: dict[str, OrderFieldType] | None = None order_valid_keys: list[str] | None = None - if order: - resolved_order = cls._resolve_order_fields(order_fields) - if resolved_order: - keys = facet_keys(resolved_order) - order_field_map = dict(zip(keys, resolved_order)) - order_valid_keys = sorted(order_field_map.keys()) - all_params.extend( - [ - inspect.Parameter( - "order_by", - inspect.Parameter.KEYWORD_ONLY, - annotation=str | None, - default=Query( - None, - description=f"Field to order by. Valid values: {order_valid_keys}", - json_schema_extra={"enum": order_valid_keys}, - ), - ), - inspect.Parameter( - "order", - inspect.Parameter.KEYWORD_ONLY, - annotation=Literal["asc", "desc"], - default=Query(default_order, description="Sort direction"), + if resolved_order: + keys = facet_keys(resolved_order) + order_field_map = dict(zip(keys, resolved_order)) + order_valid_keys = sorted(order_field_map.keys()) + all_params.extend( + [ + inspect.Parameter( + "order_by", + inspect.Parameter.KEYWORD_ONLY, + annotation=str | None, + default=Query( + None, + description=f"Field to order by. Valid values: {order_valid_keys}", + json_schema_extra={"enum": order_valid_keys}, ), - ] - ) + ), + inspect.Parameter( + "order", + inspect.Parameter.KEYWORD_ONLY, + annotation=Literal["asc", "desc"], + default=Query(default_order, description="Sort direction"), + ), + ] + ) fixed: dict[str, Any] = { **pagination_fixed, diff --git a/tests/test_crud_search.py b/tests/test_crud_search.py index f9d65fb..a1e973a 100644 --- a/tests/test_crud_search.py +++ b/tests/test_crud_search.py @@ -201,6 +201,7 @@ class _Reserved(_LocalBase): id: Mapped[int] = mapped_column(primary_key=True) search: Mapped[str] + order: Mapped[int] class _Team(_LocalBase): @@ -2102,6 +2103,13 @@ def test_no_fields_means_no_params(self): CrudFactory( _Reserved, facet_fields=[_Reserved.search] ).offset_paginate_params() + with pytest.raises(ValueError, match="conflicts with a reserved"): + CrudFactory( + _Reserved, facet_fields=[_Reserved.order], order_fields=[_Reserved.id] + ).offset_paginate_params() + unordered = CrudFactory(_Reserved, facet_fields=[_Reserved.order]) + params = inspect.signature(unordered.offset_paginate_params()).parameters + assert "order" in params @pytest.mark.anyio async def test_awaiting_the_dependency_collects_the_values(self): From 47b428371ec136ffcc0fda38c46a006b4aab9a50 Mon Sep 17 00:00:00 2001 From: d3vyce Date: Thu, 22 Oct 2026 14:06:41 -0400 Subject: [PATCH 8/9] fix: reject facet filter values that do not fit the column --- docs/module/crud.md | 2 +- src/fastapi_toolsets/crud/search.py | 49 +++++++++++++++++++++++++++++ tests/test_crud_search.py | 35 +++++++++++++++++++++ 3 files changed, 85 insertions(+), 1 deletion(-) diff --git a/docs/module/crud.md b/docs/module/crud.md index dd51332..1576411 100644 --- a/docs/module/crud.md +++ b/docs/module/crud.md @@ -529,7 +529,7 @@ The distinct values for each facet field are returned in the `filter_attributes` ``` !!! info "Key format uses `__` as a separator for relationship chains." - A direct column `User.status` produces `"status"`. A relationship tuple `(User.role, Role.name)` produces `"role__name"`. A deeper chain `(User.role, Role.permission, Permission.name)` produces `"role__permission__name"`. An unknown `filter_by` key raises [`InvalidFacetFilterError`](../reference/exceptions.md#fastapi_toolsets.exceptions.exceptions.InvalidFacetFilterError) (HTTP 400). Values are converted to the column type, so query strings like `"42"` or `"2026-01-02"` work on integer and date columns; a value that does not convert raises [`InvalidFacetValueError`](../reference/exceptions.md#fastapi_toolsets.exceptions.exceptions.InvalidFacetValueError) (HTTP 400). + A direct column `User.status` produces `"status"`. A relationship tuple `(User.role, Role.name)` produces `"role__name"`. A deeper chain `(User.role, Role.permission, Permission.name)` produces `"role__permission__name"`. An unknown `filter_by` key raises [`InvalidFacetFilterError`](../reference/exceptions.md#fastapi_toolsets.exceptions.exceptions.InvalidFacetFilterError) (HTTP 400). Values are converted to the column type, so query strings like `"42"` or `"2026-01-02"` work on integer and date columns; a value that does not convert, or does not fit the column (an integer out of range, a datetime with a UTC offset on a column without a timezone, or the reverse), raises [`InvalidFacetValueError`](../reference/exceptions.md#fastapi_toolsets.exceptions.exceptions.InvalidFacetValueError) (HTTP 400). #### Skipping facet queries diff --git a/src/fastapi_toolsets/crud/search.py b/src/fastapi_toolsets/crud/search.py index f007de1..0bfa38d 100644 --- a/src/fastapi_toolsets/crud/search.py +++ b/src/fastapi_toolsets/crud/search.py @@ -3,6 +3,7 @@ import functools from collections.abc import Callable, Iterable, Sequence from dataclasses import dataclass, replace +from decimal import Decimal, localcontext from typing import TYPE_CHECKING, Any, Literal from pydantic import TypeAdapter @@ -24,12 +25,15 @@ from sqlalchemy.sql.selectable import FromClause from sqlalchemy.types import ( ARRAY, + BigInteger, Boolean, Date, DateTime, Enum, + Float, Integer, Numeric, + SmallInteger, Time, Uuid, ) @@ -592,6 +596,9 @@ def _facet_rows( _EQUALITY_TYPES = (String, Integer, Numeric, Date, DateTime, Time, Enum, Uuid) """Column types that support equality / IN filtering in build_filter_by.""" +_INTEGER_BITS = ((BigInteger, 64), (SmallInteger, 16)) +"""Integer column widths besides the 32-bit default.""" + def _coerce_bool(value: Any) -> bool: """Coerce a string value to a Python bool for Boolean column filtering.""" @@ -616,6 +623,37 @@ def _validator(python_type: type) -> Callable[[Any], Any]: return TypeAdapter(python_type).validate_python +def _coerce_int(bits: int, value: Any) -> int: + """*value* as an int that fits a signed *bits*-bit column.""" + number = _validator(int)(value) + if not -(2 ** (bits - 1)) <= number < 2 ** (bits - 1): + raise ValueError(f"{number} does not fit a {bits}-bit integer") + return number + + +def _numeric_limit(precision: int, scale: int) -> Decimal: + """Smallest magnitude that overflows ``NUMERIC(precision, scale)``.""" + # Postgres rounds half away from zero to the scale, then checks the size. + with localcontext(prec=precision + 2): + return Decimal(10) ** (precision - scale) - Decimal(5).scaleb(-scale - 1) + + +def _coerce_decimal(limit: Decimal, value: Any) -> Decimal: + """*value* as a Decimal whose magnitude stays below *limit*.""" + number = _validator(Decimal)(value) + if not number.is_finite() or abs(number) >= limit: + raise ValueError(f"{number} does not fit the numeric column") + return number + + +def _coerce_moment(python_type: type, timezone: bool, value: Any) -> Any: + """*value* as a datetime or time, with an offset exactly when the column has one.""" + moment = _validator(python_type)(value) + if (moment.tzinfo is not None) != timezone: + raise ValueError(f"{value!r} does not match the column's timezone") + return moment + + def _coercer(col_type: Any) -> Callable[[Any], Any]: """Convert a filter value, often a query string, to the column's Python type.""" if isinstance(col_type, Boolean): @@ -626,6 +664,17 @@ def _coercer(col_type: Any) -> Callable[[Any], Any]: return _coercer(col_type.item_type) if isinstance(col_type, String) or not isinstance(col_type, _EQUALITY_TYPES): return lambda value: value + if isinstance(col_type, Integer): + bits = next((b for t, b in _INTEGER_BITS if isinstance(col_type, t)), 32) + return functools.partial(_coerce_int, bits) + # Float subclasses Numeric, but counts its precision in binary digits. + numeric = isinstance(col_type, Numeric) and not isinstance(col_type, Float) + if numeric and col_type.precision is not None: + limit = _numeric_limit(col_type.precision, col_type.scale or 0) + return functools.partial(_coerce_decimal, limit) + if isinstance(col_type, (DateTime, Time)): + timezone = bool(col_type.timezone) + return functools.partial(_coerce_moment, col_type.python_type, timezone) return _validator(col_type.python_type) diff --git a/tests/test_crud_search.py b/tests/test_crud_search.py index a1e973a..71b40a0 100644 --- a/tests/test_crud_search.py +++ b/tests/test_crud_search.py @@ -9,11 +9,17 @@ import pytest from pydantic import BaseModel from sqlalchemy import ( + BigInteger, + DateTime, + Float, ForeignKey, ForeignKeyConstraint, Index, Integer, + Numeric, + SmallInteger, String, + Time, and_, select, ) @@ -108,6 +114,7 @@ create_user, created_tables, post_tags, + raises_if, ) # Models only compiled to SQL, never created in the database. @@ -1874,6 +1881,7 @@ async def test_string_values_coerced_to_column_type( ((User.role, Role.id), ["not-a-uuid"]), (Event.scheduled_date, "tomorrow"), (_Crate.sizes, ["x"]), + (IntRole.id, "2147483648"), ], ids=[ "int-enum-plain-int", @@ -1885,6 +1893,7 @@ async def test_string_values_coerced_to_column_type( "uuid", "date", "array", + "integer-out-of-range", ], ) def test_invalid_value_raises(self, field, value): @@ -1916,6 +1925,32 @@ async def test_array_values_coerced_to_item_type(self, engine, value, expected): assert sorted(c.id for c in page.data) == expected + @pytest.mark.parametrize( + ("col_type", "value", "fits"), + [ + (SmallInteger(), "32767", True), + (SmallInteger(), "32768", False), + (Integer(), "-2147483648", True), + (Integer(), "-2147483649", False), + (BigInteger(), str(2**63 - 1), True), + (BigInteger(), str(2**63), False), + (Numeric(5, 2), "999.994", True), + (Numeric(5, 2), "-999.995", False), + (Numeric(5, 2), "Infinity", False), + (Numeric(), "1e400", True), + (Float(10), "1e30", True), + (DateTime(), "2024-01-01T00:00:00", True), + (DateTime(), "2024-01-01T00:00:00+01:00", False), + (DateTime(timezone=True), "2024-01-01T00:00:00Z", True), + (DateTime(timezone=True), "2024-01-01T00:00:00", False), + (Time(), "10:00:00Z", False), + (Time(timezone=True), "10:00:00", False), + ], + ) + def test_values_must_fit_the_column(self, col_type, value, fits): + with raises_if(ValueError, not fits): + _coercer(col_type)(value) + def test_array_items_follow_the_item_type_rules(self): assert _coercer(ARRAY(Order.color.type))("RED") is Color.RED assert _coercer(ARRAY(User.is_active.type))("true") is True From a7f47550afc83ae5a0ff0e96016d5026ce99be5d Mon Sep 17 00:00:00 2001 From: d3vyce Date: Thu, 22 Oct 2026 14:08:52 -0400 Subject: [PATCH 9/9] fix: validate search_column even when the search is empty --- src/fastapi_toolsets/crud/factory.py | 30 ++++++++++------------------ src/fastapi_toolsets/crud/search.py | 4 +++- tests/test_crud_search.py | 19 ++++++++++++++++-- 3 files changed, 31 insertions(+), 22 deletions(-) diff --git a/src/fastapi_toolsets/crud/factory.py b/src/fastapi_toolsets/crud/factory.py index 91ee8b6..1d630de 100644 --- a/src/fastapi_toolsets/crud/factory.py +++ b/src/fastapi_toolsets/crud/factory.py @@ -508,27 +508,19 @@ def _query_plan( fb_filters, fb_joins = cls._prepare_filter_by( filter_by, facet_fields, other_joins=other_joins ) - plan = ( - build_search_plan( - cls.model, - search, - search_fields, - cls.searchable_fields, - search_column, - other_joins=[*other_joins, *fb_joins], - ) - if search - else None + plan = build_search_plan( + cls.model, + search or "", + search_fields, + cls.searchable_fields, + search_column, + other_joins=[*other_joins, *fb_joins], ) return _QueryPlan( - filters=[ - *filters, - *(plan.page_filters if plan else []), - *fb_filters.values(), - ], - joins=[*fb_joins, *(plan.page_joins if plan else [])], - facet_base=[*filters, *(plan.agg_filters if plan else [])], - search_joins=plan.agg_joins if plan else [], + filters=[*filters, *plan.page_filters, *fb_filters.values()], + joins=[*fb_joins, *plan.page_joins], + facet_base=[*filters, *plan.agg_filters], + search_joins=plan.agg_joins, fb_filters=fb_filters, fb_joins=fb_joins, ) diff --git a/src/fastapi_toolsets/crud/search.py b/src/fastapi_toolsets/crud/search.py index 0bfa38d..26adeca 100644 --- a/src/fastapi_toolsets/crud/search.py +++ b/src/fastapi_toolsets/crud/search.py @@ -168,7 +168,7 @@ def build_search_plan( else search ) query = config.query.strip() if config.query else "" - if not query: + if not query and search_column is None: return _EMPTY_PLAN fields = config.fields or default_fields or get_searchable_fields(model) @@ -179,6 +179,8 @@ def build_search_plan( if search_column not in index: raise InvalidSearchColumnError(search_column, sorted(index)) fields = [index[search_column]] + if not query: + return _EMPTY_PLAN chars = set(query if config.case_sensitive else query.lower()) possible = [f for f in fields if _can_contain(_field_column(f), chars)] diff --git a/tests/test_crud_search.py b/tests/test_crud_search.py index 71b40a0..71df855 100644 --- a/tests/test_crud_search.py +++ b/tests/test_crud_search.py @@ -47,6 +47,7 @@ ) from fastapi_toolsets.crud.factory import _CursorDirection, _encode_cursor from fastapi_toolsets.crud.search import ( + _EMPTY_PLAN, _coerce_bool, _coercer, _facet_rows, @@ -857,7 +858,7 @@ async def test_a_missing_relationship_does_not_hide_the_row(self, db_session): @pytest.mark.anyio @_methods - async def test_search_column_narrows_and_is_validated(self, db_session, method): + async def test_search_column_narrows(self, db_session, method): admin = await RoleCrud.create(db_session, RoleCreate(name="admin")) await _users(db_session, "admin-user") await _users(db_session, "alice", role=admin) @@ -877,15 +878,21 @@ async def test_search_column_narrows_and_is_validated(self, db_session, method): assert [u.username for u in narrowed.data] == ["admin-user"] assert len(everywhere.data) == 2 assert narrowed.search_columns == ["id", "username", "role__name"] + + @pytest.mark.anyio + @_methods + @pytest.mark.parametrize("search", ["x", "", None]) + async def test_unknown_search_column_is_rejected(self, db_session, method, search): with pytest.raises(InvalidSearchColumnError) as exc: await _paginate( UserSearchCrud, method, db_session, - search="x", + search=search, search_column="email", schema=UserRead, ) + assert exc.value.column == "email" assert exc.value.valid_columns == ["id", "role__name", "username"] @@ -1145,6 +1152,14 @@ def test_to_one_fields_are_subqueried_with_the_to_many_ones( assert "nodes.id IN (SELECT nodes.parent_id" in sql assert sql.count("IN (SELECT") == (1 if kept_joined else 2) + def test_search_column_is_checked_without_a_query(self): + valid = build_search_plan(User, " ", [User.username], search_column="username") + unconfigured = build_search_plan(_Crate, "") + + with pytest.raises(InvalidSearchColumnError): + build_search_plan(User, "", [User.username], search_column="email") + assert valid == _EMPTY_PLAN and unconfigured == _EMPTY_PLAN + def test_casts_only_non_string_columns(self): plain, _ = build_search_filters(User, "john", search_fields=[User.username]) cast, _ = build_search_filters(User, "1a", search_fields=[User.id])