diff --git a/litellm/proxy/list_api/list_framework.py b/litellm/proxy/list_api/list_framework.py index 78b6029d7bf..e4b72ddaa5e 100644 --- a/litellm/proxy/list_api/list_framework.py +++ b/litellm/proxy/list_api/list_framework.py @@ -4,7 +4,9 @@ A resource declares a `ListSpec`; `build_query_plan` turns query parameters into `QueryPlan` or an RFC 9457 problem without touching a database, and `handle_list` runs that plan through an injected `ListExecutor`. Keeping the planning pure is what lets a caller assert the plan as a value instead of asserting against a live Prisma -client, and it keeps this module free of any database dependency. +client, and it keeps this module free of any database dependency. An endpoint that +declares its query parameters hands them over typed, as a `ListQuery`, and +`plan_list_query` plans them the same way. A plan's `where` is a tuple of frozen `Predicate`s rather than a backend-shaped mapping, so the framework never has to know which query builder executes it and a @@ -368,8 +370,7 @@ def _parse_page_size(spec: ListSpec[TRow, TOut], params: Mapping[str, str]) -> i return min(value, spec.max_page_size) -def _parse_sort(spec: ListSpec[TRow, TOut], params: Mapping[str, str]) -> tuple[SortKey, ...] | ProblemDetail: - raw: Final = params.get(SORT_PARAM) +def _sort_keys(spec: ListSpec[TRow, TOut], raw: str | None) -> tuple[SortKey, ...] | ProblemDetail: if raw is None: return spec.default_sort segments: Final = tuple(segment.strip() for segment in raw.split(",")) @@ -464,8 +465,7 @@ def _parse_filters(spec: ListSpec[TRow, TOut], params: Mapping[str, str]) -> tup return tuple(item for item in parsed if not isinstance(item, ProblemDetail)) -def _search_predicate(spec: ListSpec[TRow, TOut], params: Mapping[str, str]) -> Predicate | None: - raw: Final = params.get(SEARCH_PARAM) +def _search_predicate(spec: ListSpec[TRow, TOut], raw: str | None) -> Predicate | None: if not raw: return None return AnyOf(clauses=tuple(Compare(field=field, op="contains", value=raw) for field in sorted(spec.searchable))) @@ -505,7 +505,7 @@ def build_query_plan( if isinstance(page_size, ProblemDetail): return page_size - sort: Final = _parse_sort(spec, params) + sort: Final = _sort_keys(spec, params.get(SORT_PARAM)) if isinstance(sort, ProblemDetail): return sort @@ -513,10 +513,22 @@ def build_query_plan( if isinstance(filters, ProblemDetail): return filters - search: Final = _search_predicate(spec, params) + return _assemble_plan(spec, scope_predicates, filters, params.get(SEARCH_PARAM), sort, page, page_size) + + +def _assemble_plan( + spec: ListSpec[TRow, TOut], + scope_predicates: tuple[Predicate, ...], + filters: tuple[Predicate, ...], + search: str | None, + sort: tuple[SortKey, ...], + page: int, + page_size: int, +) -> QueryPlan: + search_predicate: Final = _search_predicate(spec, search) return QueryPlan( # Scope first: conjuncts a caller filter sits behind and cannot replace. - where=scope_predicates + filters + ((search,) if search is not None else ()), + where=scope_predicates + filters + ((search_predicate,) if search_predicate is not None else ()), # Ordering by an all-null column without a unique final key lets Postgres return # the same row on two different pages. order=sort + (SortKey(field=spec.tiebreaker, descending=False),), @@ -525,6 +537,35 @@ def build_query_plan( ) +@dataclass(frozen=True, slots=True) +class ListQuery: + """List parameters an endpoint declared in its signature, so they arrive typed and already validated.""" + + page: int = 1 + page_size: int | None = None + sort: str | None = None + search: str | None = None + filters: tuple[Predicate, ...] = () + + +def plan_list_query( + spec: ListSpec[TRow, TOut], + query: ListQuery, + caller: UserAPIKeyAuth, +) -> QueryPlan | ProblemDetail: + """`build_query_plan` for an endpoint that declares its query parameters instead of reading them as text.""" + scope_predicates: Final = _scope_predicates(spec.scope(caller)) + if isinstance(scope_predicates, ProblemDetail): + return scope_predicates + + sort: Final = _sort_keys(spec, query.sort) + if isinstance(sort, ProblemDetail): + return sort + + page_size: Final = spec.default_page_size if query.page_size is None else min(query.page_size, spec.max_page_size) + return _assemble_plan(spec, scope_predicates, query.filters, query.search, sort, query.page, page_size) + + def _facet_allowed_params(spec: ListSpec[TRow, TOut]) -> tuple[str, ...]: """A facet's values are always ascending, so `sort` is not one of its parameters.""" return tuple(name for name in _allowed_params(spec) if name != SORT_PARAM) @@ -541,7 +582,7 @@ def _facet_where( filters: Final = _parse_filters(spec, params) if isinstance(filters, ProblemDetail): return filters - search: Final = _search_predicate(spec, params) + search: Final = _search_predicate(spec, params.get(SEARCH_PARAM)) return scope_predicates + filters + ((search,) if search is not None else ()) @@ -602,15 +643,9 @@ def _duplicate_params(request: Request) -> tuple[str, ...]: return tuple(sorted(frozenset(name for name in names if names.count(name) > 1))) -def plan_list_request(spec: ListSpec[TRow, TOut], request: Request, caller: UserAPIKeyAuth) -> QueryPlan: - """The request's query parameters as a plan. Parameters that cannot be planned raise their RFC 9457 problem.""" - plan: Final = build_query_plan(spec=spec, params=request.query_params, caller=caller) - if isinstance(plan, ProblemDetail): - raise ManagementProblem(plan) - - # Checked here rather than in build_query_plan because a Mapping[str, str] cannot - # represent a repeat: query_params.get() silently keeps the last one, so ?page=1&page=999 - # would page from 999 without the caller ever being told which value won. +def reject_repeated_params(request: Request) -> None: + """Neither a `Mapping[str, str]` nor a declared query model can represent a repeat: both silently keep + the last one, so ?page=1&page=999 would page from 999 without the caller ever being told which value won.""" duplicates: Final = _duplicate_params(request) if duplicates: raise ManagementProblem( @@ -622,6 +657,14 @@ def plan_list_request(spec: ListSpec[TRow, TOut], request: Request, caller: User f"use a comma-separated list for multiple sort keys or filter values.", ) ) + + +def plan_list_request(spec: ListSpec[TRow, TOut], request: Request, caller: UserAPIKeyAuth) -> QueryPlan: + """The request's query parameters as a plan. Parameters that cannot be planned raise their RFC 9457 problem.""" + plan: Final = build_query_plan(spec=spec, params=request.query_params, caller=caller) + if isinstance(plan, ProblemDetail): + raise ManagementProblem(plan) + reject_repeated_params(request) return plan diff --git a/litellm/proxy/management/teams/dependencies.py b/litellm/proxy/management/teams/dependencies.py index 6d5dc1d1961..966d109e9df 100644 --- a/litellm/proxy/management/teams/dependencies.py +++ b/litellm/proxy/management/teams/dependencies.py @@ -7,11 +7,11 @@ from fastapi import Depends, Query, Request from litellm.proxy._types import CommonProxyErrors, LiteLLM_TeamTable, UserAPIKeyAuth from litellm.proxy.auth.user_api_key_auth import user_api_key_auth from litellm.proxy.list_api.common import PROBLEM_TYPE_BASE, ManagementProblem -from litellm.proxy.list_api.list_framework import QueryPlan, plan_list_request +from litellm.proxy.list_api.list_framework import QueryPlan, plan_list_query, reject_repeated_params from litellm.proxy.management.teams.authz import TeamAccess from litellm.proxy.management.teams.exceptions import members_not_readable, team_not_found from litellm.proxy.management.teams.schemas import TeamMembersQuery -from litellm.proxy.management.teams.service import TEAM_MEMBERS_LIST_SPEC +from litellm.proxy.management.teams.service import TEAM_MEMBERS_LIST_SPEC, team_members_list_query from litellm.proxy.management.users.service import PrismaOrgRoles from litellm.repositories.team_repository import TeamRepository from litellm.types.proxy.management_endpoints.management_v1 import ProblemDetail @@ -73,5 +73,8 @@ def get_team_members_plan( request: Request, caller: Annotated[UserAPIKeyAuth, Depends(user_api_key_auth)], ) -> QueryPlan: - """`query` is declared so FastAPI documents and validates the parameters; the plan is read from the same request.""" - return plan_list_request(TEAM_MEMBERS_LIST_SPEC, request, caller) + plan: Final = plan_list_query(TEAM_MEMBERS_LIST_SPEC, team_members_list_query(query), caller) + if isinstance(plan, ProblemDetail): + raise ManagementProblem(plan) + reject_repeated_params(request) + return plan diff --git a/litellm/proxy/management/teams/service.py b/litellm/proxy/management/teams/service.py index f88667938fe..91260e516e3 100644 --- a/litellm/proxy/management/teams/service.py +++ b/litellm/proxy/management/teams/service.py @@ -5,9 +5,9 @@ from types import MappingProxyType from typing import Final from litellm.proxy._types import TeamMemberBudgetSource, UserAPIKeyAuth -from litellm.proxy.list_api.list_framework import FilterSpec, ListSpec, QueryPlan, Scope, ScopeAll +from litellm.proxy.list_api.list_framework import Compare, ListQuery, ListSpec, QueryPlan, Scope, ScopeAll, Within from litellm.proxy.management.teams.repository import RawQuery, TeamMemberRow, TeamMemberRows -from litellm.proxy.management.teams.schemas import TeamMemberListItem +from litellm.proxy.management.teams.schemas import TeamMemberListItem, TeamMembersQuery def member_budget_source(budget_id: str | None, team_default_budget_id: str | None) -> TeamMemberBudgetSource: @@ -46,7 +46,7 @@ TEAM_MEMBERS_LIST_SPEC: Final[ListSpec[TeamMemberRow, TeamMemberListItem]] = Lis ("user_alias", "user_email", "user_id", "role", "spend", "total_spend", "max_budget_in_team", "budget_reset_at") ), searchable=frozenset(("user_id", "user_email")), - filters=MappingProxyType({"role": FilterSpec(type=str, ops=frozenset(("eq", "in")))}), + filters=MappingProxyType({}), default_sort=(), default_page_size=50, max_page_size=100, @@ -56,6 +56,23 @@ TEAM_MEMBERS_LIST_SPEC: Final[ListSpec[TeamMemberRow, TeamMemberListItem]] = Lis ) +def team_members_list_query(query: TeamMembersQuery) -> ListQuery: + return ListQuery( + page=query.page, + page_size=query.page_size, + sort=query.sort, + search=query.q, + filters=( + *(() if query.role is None else (Compare(field="role", op="eq", value=query.role),)), + *( + () + if query.role_in is None + else (Within(field="role", values=tuple(role.strip() for role in query.role_in.split(","))),) + ), + ), + ) + + @dataclass(frozen=True, slots=True) class TeamMembersPage: members: tuple[TeamMemberListItem, ...] diff --git a/tests/unit/proxy/list_api/test_list_framework.py b/tests/unit/proxy/list_api/test_list_framework.py index ab7ef229e1a..6b222a8074e 100644 --- a/tests/unit/proxy/list_api/test_list_framework.py +++ b/tests/unit/proxy/list_api/test_list_framework.py @@ -17,6 +17,7 @@ from litellm.proxy.list_api.list_framework import ( Compare, FilterSpec, IsNull, + ListQuery, ListSpec, QueryPlan, ScopeAll, @@ -28,6 +29,7 @@ from litellm.proxy.list_api.list_framework import ( handle_facet, handle_list, order_by_sql, + plan_list_query, where_sql, ) from litellm.proxy.management_endpoints.management_v1.common import MANAGEMENT_V1_PREFIX @@ -140,7 +142,10 @@ def _conjuncts(plan: QueryPlan) -> tuple[object, ...]: def test_appends_the_tiebreaker_to_the_default_sort(): """Without a unique final key, ordering by an all-null column lets Postgres hand the same row back on two different pages.""" - assert _plan({}).order == (SortKey(field="created_at", descending=True), SortKey(field="budget_id", descending=False)) + assert _plan({}).order == ( + SortKey(field="created_at", descending=True), + SortKey(field="budget_id", descending=False), + ) def test_appends_the_tiebreaker_to_an_explicit_multi_key_sort(): @@ -240,13 +245,7 @@ def test_a_search_renders_as_a_parenthesised_or(): ) ) - assert sql == ( - '"created_by" = $1 AND (' - "\"budget_id\" ILIKE $2 ESCAPE '\\'" - " OR " - "\"created_by\" ILIKE $3 ESCAPE '\\'" - ")" - ) + assert sql == ('"created_by" = $1 AND ("budget_id" ILIKE $2 ESCAPE \'\\\' OR "created_by" ILIKE $3 ESCAPE \'\\\')') assert params == ("alice", "%prod%", "%prod%") @@ -274,11 +273,7 @@ def test_a_planned_filter_renders_end_to_end(): sql, params = where_sql(_plan({"filter[max_budget][is_null]": "true", "q": "prod"}).where) assert sql == ( - '"max_budget" IS NULL AND (' - "\"budget_id\" ILIKE $1 ESCAPE '\\'" - " OR " - "\"created_by\" ILIKE $2 ESCAPE '\\'" - ")" + '"max_budget" IS NULL AND ("budget_id" ILIKE $1 ESCAPE \'\\\' OR "created_by" ILIKE $2 ESCAPE \'\\\')' ) assert params == ("%prod%", "%prod%") @@ -627,15 +622,11 @@ def test_in_splits_on_commas_and_coerces_every_member(): def test_is_null_true_matches_rows_with_no_budget(): """Budgets renders a null max_budget as "Unlimited"; without is_null there is no way to ask for those rows.""" - assert _conjuncts(_plan({"filter[max_budget][is_null]": "true"})) == ( - IsNull(field="max_budget", negated=False), - ) + assert _conjuncts(_plan({"filter[max_budget][is_null]": "true"})) == (IsNull(field="max_budget", negated=False),) def test_is_null_false_matches_rows_that_have_one(): - assert _conjuncts(_plan({"filter[max_budget][is_null]": "false"})) == ( - IsNull(field="max_budget", negated=True), - ) + assert _conjuncts(_plan({"filter[max_budget][is_null]": "false"})) == (IsNull(field="max_budget", negated=True),) def test_is_null_rejects_a_non_boolean(): @@ -688,9 +679,7 @@ async def test_returns_the_page_mode_envelope(): total_count=42, ) - response = await handle_list( - spec=_spec(), executor=executor, request=_request("page=2&page_size=5"), caller=CALLER - ) + response = await handle_list(spec=_spec(), executor=executor, request=_request("page=2&page_size=5"), caller=CALLER) body = response.model_dump(by_alias=True) assert body["meta"] == {"total_count": 42, "page": 2, "page_size": 5, "total_pages": 9} @@ -714,9 +703,7 @@ async def test_serializes_rows_flat_without_a_json_api_resource_wrapper(): async def test_links_let_a_client_page_without_building_urls(): executor = RecordingExecutor(rows=(), total_count=42) - response = await handle_list( - spec=_spec(), executor=executor, request=_request("page=2&page_size=5"), caller=CALLER - ) + response = await handle_list(spec=_spec(), executor=executor, request=_request("page=2&page_size=5"), caller=CALLER) links = response.model_dump(by_alias=True)["links"] assert links["self"] == f"{BUDGETS_PATH}?page_size=5&page=2" @@ -730,9 +717,7 @@ async def test_links_let_a_client_page_without_building_urls(): async def test_the_last_page_has_no_next_link(): executor = RecordingExecutor(rows=(), total_count=10) - response = await handle_list( - spec=_spec(), executor=executor, request=_request("page=2&page_size=5"), caller=CALLER - ) + response = await handle_list(spec=_spec(), executor=executor, request=_request("page=2&page_size=5"), caller=CALLER) links = response.model_dump(by_alias=True)["links"] assert links["next"] is None @@ -997,3 +982,48 @@ async def test_a_facet_rejects_a_page_size_that_is_not_a_positive_integer(): assert problem.status == 400 assert "'page_size'" in problem.detail + + +# ------------------------------------------------------ declared (typed) parameters + + +@pytest.mark.parametrize( + ("typed", "as_text"), + [ + pytest.param(ListQuery(), {}, id="defaults"), + pytest.param(ListQuery(page=3, page_size=10), {"page": "3", "page_size": "10"}, id="paging"), + pytest.param(ListQuery(page_size=1000), {"page_size": "1000"}, id="page-size-over-the-cap"), + pytest.param(ListQuery(sort="-max_budget,budget_id"), {"sort": "-max_budget,budget_id"}, id="sort"), + pytest.param(ListQuery(search="acme"), {"q": "acme"}, id="search"), + pytest.param( + ListQuery( + filters=( + Within(field="created_by", values=("u1", "u2")), + Compare(field="max_budget", op="gte", value=5.0), + ) + ), + {"filter[created_by][in]": "u1,u2", "filter[max_budget][gte]": "5"}, + id="filters", + ), + ], +) +def test_declared_params_plan_exactly_as_the_same_params_sent_as_text(typed: ListQuery, as_text: Mapping[str, str]): + scoped = _spec(scope=lambda caller: ScopeWhere(where=(Compare(field="created_by", op="eq", value="caller-1"),))) + + assert plan_list_query(scoped, typed, CALLER) == _plan(as_text, scoped) + + +@pytest.mark.parametrize( + ("typed", "spec", "status"), + [ + pytest.param(ListQuery(sort="created_by"), _spec(), 400, id="unsortable-field"), + pytest.param(ListQuery(), _spec(scope=lambda caller: ScopeDenied(reason="no")), 403, id="scope-denied"), + ], +) +def test_declared_params_that_cannot_be_planned_are_a_problem( + typed: ListQuery, spec: ListSpec[BudgetRow, BudgetOut], status: int +): + problem = plan_list_query(spec, typed, CALLER) + + assert isinstance(problem, ProblemDetail) + assert problem.status == status diff --git a/tests/unit/proxy/management/teams/test_dependencies.py b/tests/unit/proxy/management/teams/test_dependencies.py index d24172144d5..1fa7eb117d7 100644 --- a/tests/unit/proxy/management/teams/test_dependencies.py +++ b/tests/unit/proxy/management/teams/test_dependencies.py @@ -10,11 +10,9 @@ from fastapi.testclient import TestClient from litellm.proxy._types import LiteLLM_TeamTable, LitellmUserRoles, Member, UserAPIKeyAuth from litellm.proxy.auth.user_api_key_auth import user_api_key_auth from litellm.proxy.list_api.common import ManagementProblem -from litellm.proxy.list_api.list_framework import QueryPlan, build_query_plan +from litellm.proxy.list_api.list_framework import QueryPlan from litellm.proxy.management.teams.authz import TeamAccess from litellm.proxy.management.teams.dependencies import get_readable_team, get_team_members_plan -from litellm.proxy.management.teams.service import TEAM_MEMBERS_LIST_SPEC -from litellm.types.proxy.management_endpoints.management_v1 import ProblemDetail TEAM: Final = LiteLLM_TeamTable( team_id="team-1", organization_id=None, members_with_roles=[Member(user_id="member", role="user")] @@ -98,9 +96,8 @@ def test_get_team_members_plan_is_not_reached_by_params_the_query_model_rejects( assert plan_client(caller("member")).get("/plan", params=params).is_client_error -def test_documented_query_params_are_exactly_the_ones_the_roster_can_be_planned_by() -> None: - refusal: Final = build_query_plan(spec=TEAM_MEMBERS_LIST_SPEC, params={"undeclared": "1"}, caller=caller("member")) - documented: Final = plan_client(caller("member")).app.openapi()["paths"]["/plan"]["get"]["parameters"] +def test_get_team_members_plan_refuses_a_param_sent_twice() -> None: + with pytest.raises(ManagementProblem) as refused: + plan_client(caller("member")).get("/plan?page=1&page=2") - assert isinstance(refusal, ProblemDetail) - assert sorted(param["name"] for param in documented) == refusal.allowed + assert refused.value.problem.status == 400 diff --git a/tests/unit/proxy/management/teams/test_service.py b/tests/unit/proxy/management/teams/test_service.py index 1e3ffc5b77e..a3760cc4678 100644 --- a/tests/unit/proxy/management/teams/test_service.py +++ b/tests/unit/proxy/management/teams/test_service.py @@ -8,13 +8,15 @@ from typing import Final import pytest from litellm.proxy._types import TeamMemberBudgetSource -from litellm.proxy.list_api.list_framework import QueryPlan, SortKey +from litellm.proxy.list_api.list_framework import Compare, ListQuery, QueryPlan, SortKey, Within from litellm.proxy.management.teams.repository import TeamMemberRow +from litellm.proxy.management.teams.schemas import TeamMembersQuery from litellm.proxy.management.teams.service import ( TEAM_MEMBERS_LIST_SPEC, TeamMembersPage, get_team_members_list, member_budget_source, + team_members_list_query, ) RESET_AT: Final = datetime(2026, 11, 1, tzinfo=timezone.utc) @@ -91,3 +93,28 @@ def test_list_item_carries_the_member_row_and_derives_its_budget_source() -> Non "rpm_limit": 10, "allowed_models": ("gpt-a", "gpt-b"), } + + +def test_team_members_list_query_carries_every_declared_param_with_roles_as_roster_filters() -> None: + declared: Final = TeamMembersQuery.model_validate( + { + "q": "ada", + "filter[role]": "admin", + "filter[role][in]": "admin, user", + "sort": "-spend", + "page": 2, + "page_size": 10, + } + ) + + assert team_members_list_query(declared) == ListQuery( + page=2, + page_size=10, + sort="-spend", + search="ada", + filters=(Compare(field="role", op="eq", value="admin"), Within(field="role", values=("admin", "user"))), + ) + + +def test_team_members_list_query_with_no_params_filters_nothing() -> None: + assert team_members_list_query(TeamMembersQuery()) == ListQuery()