diff --git a/docs/module/crud.md b/docs/module/crud.md index 544ba94..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 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, 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/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/_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/__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/factory.py b/src/fastapi_toolsets/crud/factory.py index bc6b908..1d630de 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,31 +501,26 @@ 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( - cls.model, - search, - search_fields, - cls.searchable_fields, - search_column, - other_joins=[*order_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, ) @@ -696,6 +698,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) @@ -720,32 +727,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, @@ -1679,6 +1684,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 +1817,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 402a472..26adeca 100644 --- a/src/fastapi_toolsets/crud/search.py +++ b/src/fastapi_toolsets/crud/search.py @@ -1,10 +1,12 @@ """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 decimal import Decimal, localcontext from typing import TYPE_CHECKING, Any, Literal +from pydantic import TypeAdapter from sqlalchemy import ( Column, String, @@ -20,14 +22,18 @@ 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, + BigInteger, Boolean, Date, DateTime, Enum, + Float, Integer, Numeric, + SmallInteger, Time, Uuid, ) @@ -35,6 +41,7 @@ from .._orm import is_to_many, key_expr from ..exceptions import ( InvalidFacetFilterError, + InvalidFacetValueError, InvalidSearchColumnError, NoSearchableFieldsError, UnsupportedFacetTypeError, @@ -125,7 +132,6 @@ class SearchPlan: _INTEGER_CHARS = frozenset("0123456789-") _UUID_CHARS = frozenset("0123456789abcdef-") -_LIKE_SPECIAL = frozenset("%_\\") _Entry = tuple[tuple[Any, ...], "ColumnElement[bool]"] @@ -148,7 +154,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 @@ -162,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) @@ -173,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)] @@ -198,15 +206,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) @@ -239,8 +253,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 @@ -315,9 +327,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: @@ -584,6 +598,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.""" @@ -597,6 +614,88 @@ 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 _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): + 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 + 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) + + +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) + 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 + + def build_filter_by( filter_by: dict[str, Any], facet_fields: Sequence[FacetFieldType], @@ -608,7 +707,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 @@ -617,6 +716,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)) @@ -630,51 +730,21 @@ 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])] - 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/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 ea17f2a..71df855 100644 --- a/tests/test_crud_search.py +++ b/tests/test_crud_search.py @@ -8,7 +8,22 @@ import pytest from pydantic import BaseModel -from sqlalchemy import ForeignKey, ForeignKeyConstraint, Index, String, and_, select +from sqlalchemy import ( + BigInteger, + DateTime, + Float, + ForeignKey, + ForeignKeyConstraint, + Index, + Integer, + Numeric, + SmallInteger, + String, + Time, + and_, + select, +) +from sqlalchemy.dialects.postgresql import ARRAY from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker from sqlalchemy.orm import ( DeclarativeBase, @@ -20,9 +35,11 @@ ) from sqlalchemy.sql.elements import UnaryExpression +from fastapi_toolsets._orm import joined_tables from fastapi_toolsets.crud import ( CrudFactory, InvalidFacetFilterError, + InvalidFacetValueError, InvalidSearchColumnError, SearchConfig, UnsupportedFacetTypeError, @@ -30,7 +47,9 @@ ) from fastapi_toolsets.crud.factory import _CursorDirection, _encode_cursor from fastapi_toolsets.crud.search import ( + _EMPTY_PLAN, _coerce_bool, + _coercer, _facet_rows, build_filter_by, build_search_filters, @@ -55,6 +74,7 @@ ArticleCrud, ArticleRead, Color, + Event, EventCreate, EventCrud, EventDateCursorCrud, @@ -73,6 +93,7 @@ Post, PostCrud, PostWithTagsRead, + Product, ProductCreate, ProductCrud, ProductNumericCursorCrud, @@ -188,6 +209,7 @@ class _Reserved(_LocalBase): id: Mapped[int] = mapped_column(primary_key=True) search: Mapped[str] + order: Mapped[int] class _Team(_LocalBase): @@ -218,6 +240,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)) @@ -244,6 +273,10 @@ class _MatchRead(PydanticBase): id: int +class _CrateRead(PydanticBase): + id: int + + class _UserFilter(BaseModel): username: str | None = None is_active: bool | None = None @@ -313,6 +346,42 @@ 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): + 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 +566,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), @@ -800,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) @@ -820,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"] @@ -879,15 +943,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( @@ -921,6 +977,77 @@ 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"]} + + @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"], + } + + @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) @@ -1025,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]) @@ -1040,8 +1175,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=[ @@ -1094,6 +1229,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)] @@ -1128,6 +1280,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", @@ -1650,8 +1819,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"}), - ({"status": 1}, KeyError), - ({"status": [1, 3]}, KeyError), + ({"priority": "3"}, {"order-2"}), + ({"priority": ["1", "3"]}, {"order-1", "order-2", "order-3"}), ], ids=[ "int-enum-member", @@ -1662,18 +1831,144 @@ async def test_array_filters_check_containment_or_overlap(self, db_session): "str-enum-names", "integer-with-member", "integer-with-int", - "int-enum-plain-int", - "int-enum-plain-list", + "integer-with-str", + "integer-with-strs", ], ) 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 + 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( + ("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 + + @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"), + (_Crate.sizes, ["x"]), + (IntRole.id, "2147483648"), + ], + ids=[ + "int-enum-plain-int", + "int-enum-plain-list", + "int-enum-unknown-name", + "integer-not-a-number", + "integer-list-with-float", + "bool", + "uuid", + "date", + "array", + "integer-out-of-range", + ], + ) + 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 + + @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 {o.name for o in result.data} == 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 class TestParams: @@ -1858,6 +2153,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): 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,