fix: facet keys always use full relation chain (#190)

This commit is contained in:
d3vyce
2026-03-27 18:56:58 +01:00
committed by GitHub
parent 9dad59e25d
commit 5215b921ae
4 changed files with 35 additions and 31 deletions

View File

@@ -474,7 +474,7 @@ The distinct values are returned in the `filter_attributes` field of [`Paginated
"filter_attributes": { "filter_attributes": {
"status": ["active", "inactive"], "status": ["active", "inactive"],
"country": ["DE", "FR", "US"], "country": ["DE", "FR", "US"],
"name": ["admin", "editor", "viewer"] "role__name": ["admin", "editor", "viewer"]
} }
} }
``` ```
@@ -482,7 +482,7 @@ The distinct values are returned in the `filter_attributes` field of [`Paginated
Use `filter_by` to pass the client's chosen filter values directly — no need to build SQLAlchemy conditions by hand. Any unknown key raises [`InvalidFacetFilterError`](../reference/exceptions.md#fastapi_toolsets.exceptions.exceptions.InvalidFacetFilterError). Use `filter_by` to pass the client's chosen filter values directly — no need to build SQLAlchemy conditions by hand. Any unknown key raises [`InvalidFacetFilterError`](../reference/exceptions.md#fastapi_toolsets.exceptions.exceptions.InvalidFacetFilterError).
!!! info "The keys in `filter_by` are the same keys the client received in `filter_attributes`." !!! info "The keys in `filter_by` are the same keys the client received in `filter_attributes`."
Keys are normally the terminal `column.key` (e.g. `"name"` for `Role.name`). When two facet fields share the same column key (e.g. `(Build.project, Project.name)` and `(Build.os, Os.name)`), the relationship name is prepended automatically: `"project__name"` and `"os__name"`. Keys use `__` as a separator for the full relationship chain. 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"`.
`filter_by` and `filters` can be combined — both are applied with AND logic. `filter_by` and `filters` can be combined — both are applied with AND logic.
@@ -517,7 +517,7 @@ Both single-value and multi-value query parameters work:
``` ```
GET /users?status=active → filter_by={"status": ["active"]} GET /users?status=active → filter_by={"status": ["active"]}
GET /users?status=active&country=FR → filter_by={"status": ["active"], "country": ["FR"]} GET /users?status=active&country=FR → filter_by={"status": ["active"], "country": ["FR"]}
GET /users?role=admin&role=editor → filter_by={"role": ["admin", "editor"]} (IN clause) GET /users?role__name=admin&role__name=editor → filter_by={"role__name": ["admin", "editor"]} (IN clause)
``` ```
## Sorting ## Sorting

View File

@@ -2,7 +2,6 @@
import asyncio import asyncio
import functools import functools
from collections import Counter
from collections.abc import Sequence from collections.abc import Sequence
from dataclasses import dataclass, replace from dataclasses import dataclass, replace
from typing import TYPE_CHECKING, Any, Literal from typing import TYPE_CHECKING, Any, Literal
@@ -151,7 +150,7 @@ def build_search_filters(
def facet_keys(facet_fields: Sequence[FacetFieldType]) -> list[str]: def facet_keys(facet_fields: Sequence[FacetFieldType]) -> list[str]:
"""Return a key for each facet field, disambiguating duplicate column keys. """Return a key for each facet field.
Args: Args:
facet_fields: Sequence of facet fields — either direct columns or facet_fields: Sequence of facet fields — either direct columns or
@@ -160,22 +159,12 @@ def facet_keys(facet_fields: Sequence[FacetFieldType]) -> list[str]:
Returns: Returns:
A list of string keys, one per facet field, in the same order. A list of string keys, one per facet field, in the same order.
""" """
raw: list[tuple[str, str | None]] = [] keys: list[str] = []
for field in facet_fields: for field in facet_fields:
if isinstance(field, tuple): if isinstance(field, tuple):
rel = field[-2] keys.append("__".join(el.key for el in field))
column = field[-1]
raw.append((column.key, rel.key))
else: else:
raw.append((field.key, None)) keys.append(field.key)
counts = Counter(col_key for col_key, _ in raw)
keys: list[str] = []
for col_key, rel_key in raw:
if counts[col_key] > 1 and rel_key is not None:
keys.append(f"{rel_key}__{col_key}")
else:
keys.append(col_key)
return keys return keys

View File

@@ -646,7 +646,7 @@ class TestFacetsRelationship:
result = await UserRelFacetCrud.offset_paginate(db_session, schema=UserRead) result = await UserRelFacetCrud.offset_paginate(db_session, schema=UserRead)
assert result.filter_attributes is not None assert result.filter_attributes is not None
assert set(result.filter_attributes["name"]) == {"admin", "editor"} assert set(result.filter_attributes["role__name"]) == {"admin", "editor"}
@pytest.mark.anyio @pytest.mark.anyio
async def test_relationship_facet_none_excluded(self, db_session: AsyncSession): async def test_relationship_facet_none_excluded(self, db_session: AsyncSession):
@@ -661,7 +661,7 @@ class TestFacetsRelationship:
result = await UserRelFacetCrud.offset_paginate(db_session, schema=UserRead) result = await UserRelFacetCrud.offset_paginate(db_session, schema=UserRead)
assert result.filter_attributes is not None assert result.filter_attributes is not None
assert result.filter_attributes["name"] == [] assert result.filter_attributes["role__name"] == []
@pytest.mark.anyio @pytest.mark.anyio
async def test_relationship_facet_deduplicates_join_with_search( async def test_relationship_facet_deduplicates_join_with_search(
@@ -689,7 +689,7 @@ class TestFacetsRelationship:
) )
assert result.filter_attributes is not None assert result.filter_attributes is not None
assert result.filter_attributes["name"] == ["admin"] assert result.filter_attributes["role__name"] == ["admin"]
class TestFilterBy: class TestFilterBy:
@@ -755,7 +755,7 @@ class TestFilterBy:
) )
result = await UserRelFacetCrud.offset_paginate( result = await UserRelFacetCrud.offset_paginate(
db_session, filter_by={"name": "admin"}, schema=UserRead db_session, filter_by={"role__name": "admin"}, schema=UserRead
) )
assert isinstance(result.pagination, OffsetPagination) assert isinstance(result.pagination, OffsetPagination)
@@ -824,7 +824,7 @@ class TestFilterBy:
result = await UserRoleFacetCrud.offset_paginate( result = await UserRoleFacetCrud.offset_paginate(
db_session, db_session,
filter_by={"name": "admin", "id": str(admin.id)}, filter_by={"role__name": "admin", "role__id": str(admin.id)},
schema=UserRead, schema=UserRead,
) )
@@ -916,15 +916,15 @@ class TestFilterParamsSchema:
param_names = set(inspect.signature(dep).parameters) param_names = set(inspect.signature(dep).parameters)
assert param_names == {"username", "email"} assert param_names == {"username", "email"}
def test_relationship_facet_uses_column_key(self): def test_relationship_facet_uses_full_chain_key(self):
"""Relationship tuple uses the terminal column's key.""" """Relationship tuple uses the full chain joined by __ as the key."""
import inspect import inspect
UserRoleCrud = CrudFactory(User, facet_fields=[(User.role, Role.name)]) UserRoleCrud = CrudFactory(User, facet_fields=[(User.role, Role.name)])
dep = UserRoleCrud.filter_params() dep = UserRoleCrud.filter_params()
param_names = set(inspect.signature(dep).parameters) param_names = set(inspect.signature(dep).parameters)
assert param_names == {"name"} assert param_names == {"role__name"}
def test_raises_when_no_facet_fields(self): def test_raises_when_no_facet_fields(self):
"""ValueError raised when no facet_fields are configured or provided.""" """ValueError raised when no facet_fields are configured or provided."""
@@ -978,6 +978,22 @@ class TestFilterParamsSchema:
keys = facet_keys([(rel_a, col_a), (rel_b, col_b)]) keys = facet_keys([(rel_a, col_a), (rel_b, col_b)])
assert keys == ["project__name", "os__name"] assert keys == ["project__name", "os__name"]
def test_deep_chain_joins_all_segments(self):
"""Three-element tuple produces all relation segments joined by __."""
from unittest.mock import MagicMock
from fastapi_toolsets.crud.search import facet_keys
rel_a = MagicMock()
rel_a.key = "role"
rel_b = MagicMock()
rel_b.key = "permission"
col = MagicMock()
col.key = "name"
keys = facet_keys([(rel_a, rel_b, col)])
assert keys == ["role__permission__name"]
def test_unique_column_keys_kept_plain(self): def test_unique_column_keys_kept_plain(self):
"""Fields with unique column keys are not prefixed.""" """Fields with unique column keys are not prefixed."""
from fastapi_toolsets.crud.search import facet_keys from fastapi_toolsets.crud.search import facet_keys

View File

@@ -182,8 +182,7 @@ class TestOffsetPagination:
body = resp.json() body = resp.json()
fa = body["filter_attributes"] fa = body["filter_attributes"]
assert set(fa["status"]) == {"draft", "published"} assert set(fa["status"]) == {"draft", "published"}
# "name" is unique across all facet fields — no prefix needed assert set(fa["category__name"]) == {"backend", "python"}
assert set(fa["name"]) == {"backend", "python"}
@pytest.mark.anyio @pytest.mark.anyio
async def test_filter_attributes_scoped_to_filter( async def test_filter_attributes_scoped_to_filter(