From 298b64a95d018389ec16e7936cb1fa9ac1bfe309 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89loi=20Rivard?= Date: Sat, 10 Oct 2026 15:12:27 +0200 Subject: [PATCH] feat: page the search results with cursors --- doc/changelog.rst | 5 +- doc/explanation.rst | 28 ++++- pyproject.toml | 6 +- src/scim2_sqlalchemy/keyset.py | 143 +++++++++++++++++++++++ src/scim2_sqlalchemy/query.py | 111 +++++++++++------- src/scim2_sqlalchemy/storage.py | 174 ++++++++++++++++++++++----- tests/models.py | 15 +-- tests/test_comparison.py | 4 +- tests/test_cursor.py | 200 ++++++++++++++++++++++++++++++++ tests/test_enum.py | 21 +++- tests/test_mapping.py | 6 +- tests/test_projection.py | 12 +- tests/test_root_search.py | 3 +- tests/test_search.py | 32 +++-- tests/test_storage.py | 11 +- uv.lock | 18 +-- 16 files changed, 647 insertions(+), 142 deletions(-) create mode 100644 src/scim2_sqlalchemy/keyset.py create mode 100644 tests/test_cursor.py diff --git a/doc/changelog.rst b/doc/changelog.rst index b5c2261..ee3c3bb 100644 --- a/doc/changelog.rst +++ b/doc/changelog.rst @@ -14,8 +14,9 @@ Added - :class:`~scim2_sqlalchemy.SqlAlchemyStorage` and :class:`~scim2_sqlalchemy.AsyncSqlAlchemyStorage`, which serve the mappings to :doc:`scim2-server `. They filter, sort and page the resources in the - database, on one resource type or at the root of the server. The asynchronous storage needs - the ``asyncio`` extra. + database, on one resource type or at the root of the server. They page with ``startIndex``, or + with the cursors of :rfc:`9865`, which give stable pages. The asynchronous storage needs the + ``asyncio`` extra. - :meth:`ResourceMapping.schemas `, which publishes the attributes a mapping stores. - Filters, sorts and uniqueness checks compare strings with the comparison key of the policy of diff --git a/doc/explanation.rst b/doc/explanation.rst index f4e8951..04c1019 100644 --- a/doc/explanation.rst +++ b/doc/explanation.rst @@ -103,6 +103,31 @@ the identifier of each resource of the page. It then loads the records of each t type without the sort attribute sorts its resources as having no value. Between two resources of different types that share a sort value, the order of the resource types decides. +Cursors give stable pages +------------------------- + +The storage pages with ``startIndex``, and with the cursors of :rfc:`9865`. With a cursor, the +position of a page holds the sort value and the identifier of a resource: the last one of the +page for the next page, and the first one for the previous page. The next page selects the +resources after them in the sort order. A resource created or deleted between two pages does not +move the other ones. :doc:`scim2_server:explanation/pagination` explains what these stable pages +guarantee, and :doc:`scim2_server:how-to/page-with-cursors` explains how to announce the +cursors. + +The sort value of a position is the value the database compares, after the comparison key of the +policy. The storage writes the condition on the sort value by hand, since a row comparison would +not place ``NULL`` where the sort does. At the root, the position also holds the resource type, +which orders the resources of different types that share a sort value. + +The storage reads one resource more than ``count``, to know whether a page follows. It only knows +it in the direction it reads: forward for a next page, backward for a previous page. In the other +direction, it assumes that the page the client comes from still exists. When all the resources of +that page were deleted meanwhile, the client gets an empty page without cursors, and starts again +from the first page. + +Each page shows the resources as they are when it is read. The storage does not give a snapshot +of the collection as it was on the first page. + Writes that cannot be stored are refused ---------------------------------------- @@ -173,9 +198,6 @@ send the same queries. Limits ------ -The storage pages with ``startIndex`` and ``count``. It does not support the cursors of -:rfc:`9865`. - A search or a read only loads the collections its response keeps, following the ``attributes`` and ``excludedAttributes`` parameters. With ``excludedAttributes=members``, the members of the groups are not loaded. The storage still loads every column of the records, which costs little diff --git a/pyproject.toml b/pyproject.toml index 299cf3e..6a22d52 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -15,8 +15,8 @@ authors = [ ] requires-python = ">=3.11" dependencies = [ - "scim2-models>=0.12.1", - "scim2-server>=0.7.1", + "scim2-models>=0.12.2", + "scim2-server>=0.8.0", "sqlalchemy>=2.0.41", ] @@ -41,7 +41,7 @@ dev = [ "pytest-cov>=7.1.0", "pytest-postgresql>=9.1.0", "scim2-client>=0.12.0", - "scim2-server[testing]>=0.7.1", + "scim2-server[testing]>=0.8.0", "scim2-tester>=0.5.2", "tox-uv>=1.36.0", "werkzeug>=3.1.8", diff --git a/src/scim2_sqlalchemy/keyset.py b/src/scim2_sqlalchemy/keyset.py new file mode 100644 index 0000000..5822e6c --- /dev/null +++ b/src/scim2_sqlalchemy/keyset.py @@ -0,0 +1,143 @@ +import datetime +import decimal +import uuid +from collections.abc import Callable +from collections.abc import Sequence +from dataclasses import dataclass +from typing import Any + +from scim2_models import InvalidCursorException +from sqlalchemy import ColumnElement +from sqlalchemy import and_ +from sqlalchemy import false +from sqlalchemy import nulls_first +from sqlalchemy import nulls_last +from sqlalchemy import or_ +from sqlalchemy import tuple_ + +_TAGGED: dict[str, tuple[type, Callable[[Any], str], Callable[[str], Any]]] = { + "datetime": ( + datetime.datetime, + datetime.datetime.isoformat, + datetime.datetime.fromisoformat, + ), + "date": (datetime.date, datetime.date.isoformat, datetime.date.fromisoformat), + "decimal": (decimal.Decimal, str, decimal.Decimal), + "uuid": (uuid.UUID, str, uuid.UUID), +} + + +def _encode(value: Any) -> Any: + """Return a sort value as a value JSON can hold, tagged with its type when JSON has none.""" + if value is None or isinstance(value, bool | int | float | str): + return value + for tag, (python_type, dump, _) in _TAGGED.items(): + if isinstance(value, python_type): + return {tag: dump(value)} + raise TypeError(f"Cannot page on a sort value of type {type(value).__name__}") + + +def _decode(value: Any) -> Any: + """Return the sort value a position holds.""" + if value is None or isinstance(value, bool | int | float | str): + return value + if not isinstance(value, dict) or len(value) != 1: + raise InvalidCursorException + ((tag, text),) = value.items() + if tag not in _TAGGED or not isinstance(text, str): + raise InvalidCursorException + try: + return _TAGGED[tag][2](text) + except (ValueError, decimal.InvalidOperation): + raise InvalidCursorException from None + + +@dataclass(frozen=True) +class _Anchor: + """The row a cursor page starts after, or ends before. + + ties are the values that order the rows sharing a sort value: the + identifier as text, preceded at the root by the position of the resource + type. + """ + + forward: bool + key: Any + ties: tuple[Any, ...] + + +def _anchor(position: Any, root: bool) -> _Anchor | None: + """Read the position a storage gave, or return None for the first page.""" + if position is None: + return None + names = {"d", "k", "t", "i"} if root else {"d", "k", "i"} + if not isinstance(position, dict) or set(position) != names: + raise InvalidCursorException + if position["d"] not in ("next", "previous") or not isinstance(position["i"], str): + raise InvalidCursorException + ties: tuple[Any, ...] = (position["i"],) + if root: + if type(position["t"]) is not int: + raise InvalidCursorException + ties = (position["t"], position["i"]) + return _Anchor(position["d"] == "next", _decode(position["k"]), ties) + + +def _position(direction: str, key: Any, ties: Sequence[Any]) -> Any: + """Return the position of the page before or after a row, as a value JSON can hold. + + The sort value is null when the search has no sort. + """ + position: dict[str, Any] = {"d": direction, "k": _encode(key)} + if len(ties) == 2: + position["t"] = ties[0] + position["i"] = ties[-1] + return position + + +def _key_order(key: Any, ascending: bool) -> Any: + """Per RFC 7644 §3.4.2.3, resources without a value come last when ascending and first when descending.""" + if ascending: + return nulls_last(key.asc()) + return nulls_first(key.desc()) + + +def _keyset_order( + key: Any, ties: Sequence[Any], descending: bool, forward: bool +) -> list[Any]: + """Return the ORDER BY terms of a search: the sort value, then the ties. + + A previous page is read backward, in the reverse order of the sort. + """ + terms = [] if key is None else [_key_order(key, descending != forward)] + return [*terms, *(tie.asc() if forward else tie.desc() for tie in ties)] + + +def _beyond( + key: Any, + ties: Sequence[Any], + anchor: _Anchor, + values: Sequence[Any], + descending: bool, +) -> ColumnElement[bool]: + """Return the condition keeping the rows past the anchor, in the reading direction. + + values are the ties of the anchor, in the types of the ties. The + condition on the sort value is written out, as a row comparison would not + place NULL where the sort does. + """ + tie_after: ColumnElement[bool] + if len(ties) == 1: + tie_after = ties[0] > values[0] if anchor.forward else ties[0] < values[0] + else: + row, bound = tuple_(*ties), tuple_(*values) + tie_after = row > bound if anchor.forward else row < bound + if key is None: + return tie_after + + ascending = descending != anchor.forward + if anchor.key is None: + key_after = key.is_not(None) if not ascending else false() + return or_(key_after, and_(key.is_(None), tie_after)) + key_after = or_(key > anchor.key, key.is_(None)) if ascending else key < anchor.key + return or_(key_after, and_(key == anchor.key, tie_after)) diff --git a/src/scim2_sqlalchemy/query.py b/src/scim2_sqlalchemy/query.py index 726078c..6b28ce7 100644 --- a/src/scim2_sqlalchemy/query.py +++ b/src/scim2_sqlalchemy/query.py @@ -3,6 +3,7 @@ from typing import Any from scim2_models import AttributeBinding +from scim2_models import InvalidCursorException from scim2_models import InvalidFilterException from scim2_models import InvalidPathException from scim2_models import ResponseParameters @@ -29,7 +30,6 @@ from sqlalchemy import literal from sqlalchemy import not_ from sqlalchemy import null -from sqlalchemy import nulls_first from sqlalchemy import nulls_last from sqlalchemy import or_ from sqlalchemy import select @@ -40,6 +40,9 @@ from .comparison import _SUBSTRINGS from .comparison import _Comparator from .conversion import _as_stored +from .keyset import _Anchor +from .keyset import _beyond +from .keyset import _keyset_order from .mapping import ResourceMapping from .mapping import _Collection from .mapping import _Column @@ -270,28 +273,6 @@ def _sort_key( return expression -def _sorted(expression: Any, search_request: SearchRequest[Any]) -> Any: - """Per RFC 7644 §3.4.2.3, resources without a value come last when ascending and first when descending.""" - if search_request.sort_order == SearchRequest.SortOrder.descending: - return nulls_first(expression.desc()) - return nulls_last(expression.asc()) - - -def _order_by( - mapping: ResourceMapping, - search_request: SearchRequest[Any], - comparator: _Comparator, -) -> list[Any]: - """Return the ORDER BY terms of a search, the record identifier last. - - Without the identifier, records sharing a sort value could change pages - from a request to the next. - """ - key = _sort_key(mapping, search_request, comparator) - terms = [] if key is None else [_sorted(key, search_request)] - return [*terms, mapping._id.expression] - - def _first_entry(collection: _Collection, expression: Any) -> Any: """Return the value of the primary entry of a collection, or else of its first one.""" relationship = collection.relationship_property @@ -321,39 +302,74 @@ def _where( return _FilterTranslator(mapping, scim_filter, comparator).visit(scim_filter.ast) +def _cursor_page(page: Select[Any], search_request: SearchRequest[Any]) -> Select[Any]: + """Limit a cursor page to count + 1 rows, the last one telling whether a page follows.""" + if search_request.count is None: + return page + return page.limit(search_request.count + 1) + + +def _index_page(page: Select[Any], search_request: SearchRequest[Any]) -> Select[Any]: + page = page.offset(search_request.start_index_0 or 0) + if search_request.count is None: + return page + return page.limit(search_request.count) + + def _search_statements( mapping: ResourceMapping, search_request: SearchRequest[Any], comparator: _Comparator, -) -> tuple[Select[tuple[int]], Select[Any]]: - """Return the statement counting the matching records, and the one selecting a page of them.""" + anchor: _Anchor | None = None, +) -> tuple[Select[tuple[int]], Select[tuple[Any, Any, str]]]: + """Return the statement counting the matching records, and the one selecting a page of them. + + The page lists each record with its sort value and its identifier as + text. The records are ordered by sort value, then by identifier: without + the identifier, records sharing a sort value could change pages from a + request to the next. A cursor page is read from the anchor, in the reverse + order for a previous page. + """ condition = _where(mapping, search_request, comparator) count = select(func.count()).select_from(mapping.record).where(condition) + key = _sort_key(mapping, search_request, comparator) + record_id = mapping._id.expression + descending = search_request.sort_order == SearchRequest.SortOrder.descending + forward = anchor is None or anchor.forward page = ( - select(mapping.record) + select( + mapping.record, + (null() if key is None else key).label("key"), + cast(record_id, String).label("id"), + ) .where(condition) - .order_by(*_order_by(mapping, search_request, comparator)) + .order_by(*_keyset_order(key, [record_id], descending, forward)) .options( *mapping._loader_options(mapping._returned_collections(search_request)) ) - .offset(search_request.start_index_0 or 0) ) - if search_request.count is not None: - page = page.limit(search_request.count) - return count, page + if search_request.cursor is None: + return count, _index_page(page, search_request) + if anchor is not None: + value = _as_key(mapping._id, anchor.ties[0]) + if value is None: + raise InvalidCursorException + page = page.where(_beyond(key, [record_id], anchor, [value], descending)) + return count, _cursor_page(page, search_request) def _root_statements( mappings: list[ResourceMapping], search_request: SearchRequest[Any], comparator: _Comparator, -) -> tuple[Select[tuple[int]], Select[tuple[int, str]]]: + anchor: _Anchor | None = None, +) -> tuple[Select[tuple[int]], Select[tuple[int, str, Any]]]: """Return the statement counting the matching records of several mappings, and the one selecting a page of them. - The page lists the position of the mapping and the identifier of each - record, as text, so that the identifiers of every table fit in one column. - A mapping whose model does not declare the sort attribute sorts its - records as having no value. + The page lists the position of the mapping, the identifier of each + record as text, so that the identifiers of every table fit in one column, + and its sort value. A mapping whose model does not declare the sort + attribute sorts its records as having no value. """ conditions = [_where(mapping, search_request, comparator) for mapping in mappings] keys = [_sort_key(mapping, search_request, comparator) for mapping in mappings] @@ -382,15 +398,20 @@ def _root_statements( columns.append(key.label("key")) branches.append(select(*columns).where(condition)) rows = union_all(*branches).subquery() - terms = [] if key_type is None else [_sorted(rows.c.key, search_request)] - page = ( - select(rows.c.position, rows.c.id) - .order_by(*terms, rows.c.position, rows.c.id) - .offset(search_request.start_index_0 or 0) - ) - if search_request.count is not None: - page = page.limit(search_request.count) - return count, page + row_key = None if key_type is None else rows.c.key + ties = [rows.c.position, rows.c.id] + descending = search_request.sort_order == SearchRequest.SortOrder.descending + forward = anchor is None or anchor.forward + page = select( + rows.c.position, rows.c.id, (null() if row_key is None else row_key) + ).order_by(*_keyset_order(row_key, ties, descending, forward)) + if search_request.cursor is None: + return count, _index_page(page, search_request) + if anchor is not None: + if anchor.ties[0] not in range(len(mappings)): + raise InvalidCursorException + page = page.where(_beyond(row_key, ties, anchor, anchor.ties, descending)) + return count, _cursor_page(page, search_request) def _records_statement( diff --git a/src/scim2_sqlalchemy/storage.py b/src/scim2_sqlalchemy/storage.py index a40093c..c90c472 100644 --- a/src/scim2_sqlalchemy/storage.py +++ b/src/scim2_sqlalchemy/storage.py @@ -23,6 +23,7 @@ from scim2_models import UniquenessException from scim2_server.storage import AsyncScimStorage from scim2_server.storage import ScimStorage +from scim2_server.storage import SearchPage from sqlalchemy import Select from sqlalchemy.exc import IntegrityError from sqlalchemy.ext.asyncio import AsyncSession @@ -36,6 +37,9 @@ from .conversion import _link_ids from .conversion import _to_scim from .conversion import _version +from .keyset import _Anchor +from .keyset import _anchor +from .keyset import _position from .mapping import ResourceMapping from .mapping import _Collection from .query import _links_statement @@ -53,11 +57,54 @@ def _utcnow() -> datetime.datetime: def _by_position(rows: Sequence[Any]) -> dict[int, list[str]]: """Group the identifiers of a page at the root by position of their resource type.""" ids: dict[int, list[str]] = {} - for position, record_id in rows: + for position, record_id, _ in rows: ids.setdefault(position, []).append(record_id) return ids +def _page_rows( + rows: Sequence[Any], search_request: SearchRequest[Any], anchor: _Anchor | None +) -> tuple[list[Any], bool]: + """Return the rows of a page in the sort order, and whether more rows follow in the reading direction. + + The rows of a cursor page were read count + 1 at a time, in the reverse + order for a previous page. + """ + if search_request.cursor is None: + return list(rows), False + count = search_request.count + more = count is not None and len(rows) > count + page = list(rows if count is None else rows[:count]) + if anchor is not None and not anchor.forward: + page.reverse() + return page, more + + +def _cursor_page( + total: int, + resources: list[Resource[Any]], + bounds: list[tuple[Any, tuple[Any, ...]]], + more: bool, + anchor: _Anchor | None, +) -> SearchPage: + """Return a cursor page with the positions of the pages around it. + + bounds holds the sort value and the ties of the first and the last rows + of the page. A page is known to follow in the reading direction only. In + the other direction, the page the client comes from is assumed to still + exist. + """ + page = SearchPage(total, resources) + if not bounds: + return page + forward = anchor is None or anchor.forward + if more or not forward: + page.next = _position("next", *bounds[-1]) + if anchor is not None and (more or forward): + page.previous = _position("previous", *bounds[0]) + return page + + def _nothing() -> None: pass @@ -105,10 +152,49 @@ def _root_page( self._to_scim( resource_types[position], records[position, record_id], search_request ) - for position, record_id in rows + for position, record_id, _ in rows if (position, record_id) in records ] + def _root_search_page( + self, + resource_types: list[ResourceType], + total: int, + rows: list[Any], + more: bool, + records: dict[tuple[int, str], Any], + search_request: SearchRequest[Any], + anchor: _Anchor | None, + ) -> SearchPage: + """Build the page of a search at the root, from its rows and the records they load.""" + resources = self._root_page(resource_types, rows, records, search_request) + if search_request.cursor is None: + return SearchPage(total, resources) + bounds = [ + (key, (type_position, record_id)) + for type_position, record_id, key in rows[:1] + rows[-1:] + ] + return _cursor_page(total, resources, bounds, more, anchor) + + def _page( + self, + resource_type: ResourceType, + total: int, + rows: Sequence[Any], + search_request: SearchRequest[Any], + anchor: _Anchor | None, + ) -> SearchPage: + """Build the page of a search on one resource type, from its records with their sort value and identifier.""" + page, more = _page_rows(rows, search_request, anchor) + resources = [ + self._to_scim(resource_type, record, search_request) + for record, _, _ in page + ] + if search_request.cursor is None: + return SearchPage(total, resources) + bounds = [(key, (record_id,)) for _, key, record_id in page[:1] + page[-1:]] + return _cursor_page(total, resources, bounds, more, anchor) + def _model(self, resource_type: ResourceType) -> type[Resource[Any]]: """Return the model of the resources of a resource type: the one of the provider, if any.""" model = self.provider.model_for(resource_type) if self.provider else None @@ -240,6 +326,8 @@ class SqlAlchemyStorage(_StorageBase, ScimStorage): :attr:`~scim2_models.ScimPolicy.comparison_key` of its policy. """ + supports_cursors = True + def __init__( self, mappings: Mapping[str, ResourceMapping], @@ -271,31 +359,41 @@ def get( return self._to_scim(resource_type, record, response_parameters) def search( - self, resource_types: list[ResourceType], search_request: SearchRequest[Any] - ) -> tuple[int, list[Resource[Any]]]: + self, + resource_types: list[ResourceType], + search_request: SearchRequest[Any], + *, + position: Any = None, + ) -> SearchPage: if not resource_types: - return 0, [] + return SearchPage(0, []) session = self.session() comparator = self._ready_comparator(session) mappings = [self._mapping(resource_type) for resource_type in resource_types] + anchor = _anchor(position, root=len(mappings) > 1) if len(mappings) == 1: - count, page = _search_statements(mappings[0], search_request, comparator) + count, statement = _search_statements( + mappings[0], search_request, comparator, anchor + ) total = session.scalar(count) or 0 - found = session.scalars(page).all() - return total, [ - self._to_scim(resource_types[0], record, search_request) - for record in found - ] + rows = session.execute(statement).all() + return self._page(resource_types[0], total, rows, search_request, anchor) - count, statement = _root_statements(mappings, search_request, comparator) + count, statement = _root_statements( + mappings, search_request, comparator, anchor + ) total = session.scalar(count) or 0 - rows = session.execute(statement).all() + rows, more = _page_rows( + session.execute(statement).all(), search_request, anchor + ) records = {} - for position, ids in _by_position(rows).items(): - loading = _records_statement(mappings[position], ids, search_request) + for type_position, ids in _by_position(rows).items(): + loading = _records_statement(mappings[type_position], ids, search_request) for record_id, record in session.execute(loading): - records[position, record_id] = record - return total, self._root_page(resource_types, rows, records, search_request) + records[type_position, record_id] = record + return self._root_search_page( + resource_types, total, rows, more, records, search_request, anchor + ) def create( self, resource_type: ResourceType, resource: Resource[Any] @@ -424,6 +522,8 @@ class AsyncSqlAlchemyStorage(_StorageBase, AsyncScimStorage): :attr:`~scim2_models.ScimPolicy.comparison_key` of its policy. """ + supports_cursors = True + def __init__( self, mappings: Mapping[str, ResourceMapping], @@ -450,31 +550,41 @@ async def get( return self._to_scim(resource_type, record, response_parameters) async def search( - self, resource_types: list[ResourceType], search_request: SearchRequest[Any] - ) -> tuple[int, list[Resource[Any]]]: + self, + resource_types: list[ResourceType], + search_request: SearchRequest[Any], + *, + position: Any = None, + ) -> SearchPage: if not resource_types: - return 0, [] + return SearchPage(0, []) session = self.session() comparator = await self._ready_comparator(session) mappings = [self._mapping(resource_type) for resource_type in resource_types] + anchor = _anchor(position, root=len(mappings) > 1) if len(mappings) == 1: - count, page = _search_statements(mappings[0], search_request, comparator) + count, statement = _search_statements( + mappings[0], search_request, comparator, anchor + ) total = await session.scalar(count) or 0 - found = (await session.scalars(page)).all() - return total, [ - self._to_scim(resource_types[0], record, search_request) - for record in found - ] + rows = (await session.execute(statement)).all() + return self._page(resource_types[0], total, rows, search_request, anchor) - count, statement = _root_statements(mappings, search_request, comparator) + count, statement = _root_statements( + mappings, search_request, comparator, anchor + ) total = await session.scalar(count) or 0 - rows = (await session.execute(statement)).all() + rows, more = _page_rows( + (await session.execute(statement)).all(), search_request, anchor + ) records = {} - for position, ids in _by_position(rows).items(): - loading = _records_statement(mappings[position], ids, search_request) + for type_position, ids in _by_position(rows).items(): + loading = _records_statement(mappings[type_position], ids, search_request) for record_id, record in await session.execute(loading): - records[position, record_id] = record - return total, self._root_page(resource_types, rows, records, search_request) + records[type_position, record_id] = record + return self._root_search_page( + resource_types, total, rows, more, records, search_request, anchor + ) async def create( self, resource_type: ResourceType, resource: Resource[Any] diff --git a/tests/models.py b/tests/models.py index 59ce44c..7850527 100644 --- a/tests/models.py +++ b/tests/models.py @@ -1,5 +1,4 @@ import datetime -import itertools import uuid from scim2_models import EnterpriseUser @@ -23,14 +22,6 @@ ENTERPRISE = "urn:ietf:params:scim:schemas:extension:enterprise:2.0:User" -_ids = itertools.count(1) - - -def increasing_id() -> uuid.UUID: - """Return a new identifier, greater than the previous ones, whose text holds letters.""" - return uuid.UUID(int=(0xA << 124) | next(_ids)) - - class Base(DeclarativeBase): pass @@ -46,11 +37,12 @@ class Base(DeclarativeBase): class UserRecord(Base): __tablename__ = "users" - id: Mapped[uuid.UUID] = mapped_column(primary_key=True, default=increasing_id) + id: Mapped[uuid.UUID] = mapped_column(primary_key=True, default=uuid.uuid4) external_id: Mapped[str | None] user_name: Mapped[str] = mapped_column(unique=True) display_name: Mapped[str | None] title: Mapped[str | None] + nick_name: Mapped[str | None] user_type: Mapped[str | None] = mapped_column(default="Employee") given_name: Mapped[str | None] family_name: Mapped[str | None] @@ -151,7 +143,8 @@ def users_mapping(changes=None): "displayName": UserRecord.display_name, "title": UserRecord.title, "userType": UserRecord.user_type, - "nickName": UserRecord.lower_name, + "nickName": UserRecord.nick_name, + "locale": UserRecord.lower_name, "name.givenName": UserRecord.given_name, "name.familyName": UserRecord.family_name, "active": UserRecord.active, diff --git a/tests/test_comparison.py b/tests/test_comparison.py index 3c379d1..eecd7b5 100644 --- a/tests/test_comparison.py +++ b/tests/test_comparison.py @@ -48,8 +48,8 @@ def strict_provider() -> ScimProvider: def user_names(storage, resource_type, **parameters): - _, resources = storage.search([resource_type], SearchRequest(**parameters)) - return [resource.user_name for resource in resources] + page = storage.search([resource_type], SearchRequest(**parameters)) + return [resource.user_name for resource in page.resources] def test_a_decomposed_value_matches_its_composed_form(storage, user_type): diff --git a/tests/test_cursor.py b/tests/test_cursor.py new file mode 100644 index 0000000..4bdddfd --- /dev/null +++ b/tests/test_cursor.py @@ -0,0 +1,200 @@ +import datetime +import decimal +import uuid + +import pytest +from scim2_models import Group +from scim2_models import InvalidCursorException +from scim2_models import SearchRequest +from scim2_models import User + +from scim2_sqlalchemy.keyset import _decode +from scim2_sqlalchemy.keyset import _encode + + +@pytest.fixture +def users(storage, user_type): + """Create five users, two of them without a title.""" + for user_name, title in [ + ("alice", "b"), + ("bob", None), + ("carol", "a"), + ("dave", "b"), + ("erin", None), + ]: + storage.create(user_type, User(user_name=user_name, title=title)) + + +def walk(storage, resource_types, search_request): + """Follow the next cursors from the first page, and return the pages.""" + pages = [storage.search(resource_types, search_request)] + while pages[-1].next is not None: + pages.append( + storage.search(resource_types, search_request, position=pages[-1].next) + ) + return pages + + +def walk_back(storage, resource_types, search_request, page): + """Follow the previous cursors from a page, and return the pages in the sort order.""" + pages = [page] + while pages[0].previous is not None: + pages.insert( + 0, + storage.search(resource_types, search_request, position=pages[0].previous), + ) + return pages + + +def user_names(pages): + return [resource.user_name for page in pages for resource in page.resources] + + +@pytest.mark.parametrize("sort_order", ["ascending", "descending"]) +def test_cursors_page_through_the_sort_and_back(users, storage, user_type, sort_order): + """The pages follow the sort, users without a title included, in both directions.""" + search_request = SearchRequest( + cursor="", count=2, sort_by="title", sort_order=sort_order + ) + expected = user_names( + [ + storage.search( + [user_type], SearchRequest(sort_by="title", sort_order=sort_order) + ) + ] + ) + + forward = walk(storage, [user_type], search_request) + backward = walk_back(storage, [user_type], search_request, forward[-1]) + + assert user_names(forward) == expected + assert user_names(backward) == expected + + +@pytest.mark.parametrize("sort_by", ["meta.created", "id"]) +def test_cursors_page_on_dates_and_identifiers(users, storage, user_type, sort_by): + """A sort value that JSON cannot hold as it is still resumes the paging.""" + search_request = SearchRequest(cursor="", count=2, sort_by=sort_by) + + pages = walk(storage, [user_type], search_request) + + assert sorted(user_names(pages)) == ["alice", "bob", "carol", "dave", "erin"] + + +def test_a_cursor_without_count_returns_every_resource(users, storage, user_type): + """Without count, the first page holds every resource, and no page follows.""" + page = storage.search([user_type], SearchRequest(cursor="")) + + assert len(page.resources) == 5 + assert page.next is None + assert page.previous is None + + +def test_a_page_reached_forward_assumes_a_previous_page(users, storage, user_type): + """A page after the first one gives a previous cursor, even when the resources before it are gone.""" + search_request = SearchRequest(cursor="", count=2) + first, second, _ = walk(storage, [user_type], search_request) + for resource in first.resources: + storage.delete(user_type, resource.id) + + previous = storage.search([user_type], search_request, position=second.previous) + + assert second.previous is not None + assert previous.resources == [] + assert previous.next is None + assert previous.previous is None + + +@pytest.mark.parametrize("sort_by", [None, "displayName"]) +def test_cursors_page_at_the_root(storage, user_type, group_type, sort_by): + """A search at the root pages the resources of every type as one collection, in both directions.""" + for name in ["d", "b", "e"]: + storage.create(user_type, User(user_name=name, display_name=name)) + for name in ["c", "a"]: + storage.create(group_type, Group(display_name=name)) + resource_types = [user_type, group_type] + search_request = SearchRequest(cursor="", count=2, sort_by=sort_by) + expected = [ + resource.display_name + for resource in storage.search( + resource_types, SearchRequest(sort_by=sort_by) + ).resources + ] + + forward = walk(storage, resource_types, search_request) + backward = walk_back(storage, resource_types, search_request, forward[-1]) + + names = [[r.display_name for r in page.resources] for page in forward] + assert [name for page in names for name in page] == expected + assert [[r.display_name for r in page.resources] for page in backward] == names + + +@pytest.mark.parametrize( + "position", + [ + "next", + {"d": "next", "i": "x"}, + {"d": "sideways", "k": None, "i": "x"}, + {"d": "next", "k": None, "i": 1}, + {"d": "next", "k": {"datetime": "never"}, "i": "x"}, + {"d": "next", "k": None, "i": "not an identifier"}, + ], +) +def test_a_malformed_position_is_an_invalid_cursor(storage, user_type, position): + """A position the storage did not give raises invalidCursor.""" + with pytest.raises(InvalidCursorException): + storage.search([user_type], SearchRequest(cursor="x"), position=position) + + +@pytest.mark.parametrize( + "position", + [ + {"d": "next", "k": None, "i": "x"}, + {"d": "next", "k": None, "t": True, "i": "x"}, + {"d": "next", "k": None, "t": 2, "i": "x"}, + ], +) +def test_a_malformed_root_position_is_an_invalid_cursor( + storage, user_type, group_type, position +): + """A position at the root needs the position of an existing resource type.""" + with pytest.raises(InvalidCursorException): + storage.search( + [user_type, group_type], SearchRequest(cursor="x"), position=position + ) + + +@pytest.mark.parametrize( + "value", + [ + None, + True, + 3, + 1.5, + "text", + datetime.datetime(2026, 10, 9, 12, tzinfo=datetime.UTC), + datetime.date(2026, 10, 9), + decimal.Decimal("1.50"), + uuid.UUID(int=1), + ], +) +def test_a_sort_value_survives_its_position(value): + """A sort value is read back from its position in its own type.""" + assert _decode(_encode(value)) == value + assert type(_decode(_encode(value))) is type(value) + + +def test_a_sort_value_of_an_unknown_type_cannot_be_paged(): + """A sort value JSON cannot hold is refused rather than written wrong.""" + with pytest.raises(TypeError): + _encode(b"bytes") + + +@pytest.mark.parametrize( + "value", + [["a"], {"a": "1", "b": "2"}, {"bytes": "x"}, {"date": 1}, {"decimal": "x"}], +) +def test_a_malformed_sort_value_is_an_invalid_cursor(value): + """A sort value that no position holds raises invalidCursor.""" + with pytest.raises(InvalidCursorException): + _decode(value) diff --git a/tests/test_enum.py b/tests/test_enum.py index 57c2ea3..7bee2c8 100644 --- a/tests/test_enum.py +++ b/tests/test_enum.py @@ -110,13 +110,14 @@ def storage(database_url, storage_factory, user_type): def user_names(storage, user_type, **parameters): - _, resources = storage.search([user_type], SearchRequest(**parameters)) - return [resource.user_name for resource in resources] + page = storage.search([user_type], SearchRequest(**parameters)) + return [resource.user_name for resource in page.resources] def test_an_enumerated_column_reads_back_the_string_it_stores(storage, user_type): """A resource holds the string the column stores: the name of the member, or its value with values_callable.""" - _, (user,) = storage.search([user_type], SearchRequest(filter='userName eq "a"')) + page = storage.search([user_type], SearchRequest(filter='userName eq "a"')) + (user,) = page.resources assert user.title == "Red" assert user.external_id == "Crimson" @@ -164,6 +165,20 @@ def test_an_enumerated_column_sorts_on_the_string_it_stores( ) +@pytest.mark.parametrize("sort_by", ["title", "externalId"]) +def test_cursors_page_on_an_enumerated_column(storage, user_type, sort_by): + """The cursors resume on the string the column stores, in the order of the strings.""" + expected = user_names(storage, user_type, sort_by=sort_by) + search_request = SearchRequest(cursor="", count=1, sort_by=sort_by) + pages = [storage.search([user_type], search_request)] + while pages[-1].next is not None: + pages.append( + storage.search([user_type], search_request, position=pages[-1].next) + ) + + assert [r.user_name for page in pages for r in page.resources] == expected + + @pytest.mark.parametrize( "user", [ diff --git a/tests/test_mapping.py b/tests/test_mapping.py index 8f79f2b..90e3c51 100644 --- a/tests/test_mapping.py +++ b/tests/test_mapping.py @@ -207,11 +207,11 @@ def test_the_schemas_hold_the_mapped_attributes_only(): def test_an_attribute_the_mapping_only_reads_is_published_read_only(): - """NickName comes from a hybrid property without setter.""" + """Locale comes from a hybrid property without setter.""" (user, _) = users_mapping().schemas() - (nick_name,) = [a for a in user.attributes if a.name == "nickName"] - assert nick_name.mutability == Mutability.read_only + (locale,) = [a for a in user.attributes if a.name == "locale"] + assert locale.mutability == Mutability.read_only def test_a_link_publishes_its_value_its_ref_and_a_read_only_display(): diff --git a/tests/test_projection.py b/tests/test_projection.py index a6e2c05..3ae27d7 100644 --- a/tests/test_projection.py +++ b/tests/test_projection.py @@ -46,9 +46,9 @@ def bob(storage, user_type, group_type): def search_bob(storage, user_type, statements, **parameters): """Return bob as a search returns him, and the statements the search ran.""" statements.clear() - _, resources = storage.search( + resources = storage.search( [user_type], SearchRequest(filter='userName eq "bob"', **parameters) - ) + ).resources return resources[0], [statement.lower() for statement in statements] @@ -109,9 +109,9 @@ def test_an_excluded_extension_does_not_load_its_links( def test_excluded_members_are_not_loaded(bob, storage, group_type, statements): """Excluding the members of a group saves loading them and their users.""" statements.clear() - _, groups = storage.search( + groups = storage.search( [group_type], SearchRequest(excluded_attributes=["members"]) - ) + ).resources assert groups[0].members is None assert not loads([statement.lower() for statement in statements], "users") @@ -121,10 +121,10 @@ def test_a_filter_on_an_excluded_collection_still_applies( bob, storage, user_type, statements ): """The filter runs in SQL, whether the response keeps the collection or not.""" - _, resources = storage.search( + resources = storage.search( [user_type], SearchRequest(filter='emails co "bob"', excluded_attributes=["emails"]), - ) + ).resources assert [user.user_name for user in resources] == ["bob"] diff --git a/tests/test_root_search.py b/tests/test_root_search.py index 35bd71b..504c6a6 100644 --- a/tests/test_root_search.py +++ b/tests/test_root_search.py @@ -26,7 +26,8 @@ def everyone(storage, user_type, group_type): def search(storage, resource_types, **parameters): - return storage.search(resource_types, SearchRequest(**parameters)) + page = storage.search(resource_types, SearchRequest(**parameters)) + return page.total, page.resources def names(resources): diff --git a/tests/test_search.py b/tests/test_search.py index aff87e0..f37e886 100644 --- a/tests/test_search.py +++ b/tests/test_search.py @@ -114,19 +114,16 @@ def memory(user_type): @pytest.fixture def sql(storage, user_type): - """Return a storage holding the users of USERS. - - Their identifiers increase, so that ties sort in creation order, as in memory. - """ + """Return a storage holding the users of USERS.""" for user in USERS: storage.create(user_type, UserModel.model_validate(user)) return storage def user_names(storage, user_type, **parameters): - total, resources = storage.search([user_type], SearchRequest(**parameters)) - names = [resource.user_name for resource in resources] - assert total == len(names) + page = storage.search([user_type], SearchRequest(**parameters)) + names = [resource.user_name for resource in page.resources] + assert page.total == len(names) return names @@ -142,15 +139,16 @@ def test_a_filter_keeps_the_resources_kept_in_memory( @pytest.mark.parametrize("sort_order", ["ascending", "descending"]) @pytest.mark.parametrize("sort_by", SORTS) -def test_a_sort_orders_the_resources_as_in_memory( - memory, sql, user_type, sort_by, sort_order +def test_a_sort_orders_the_resources_as_search_request_sort( + sql, user_type, sort_by, sort_order ): - """The database orders the resources as SearchRequest.sort does.""" - parameters = {"sort_by": sort_by, "sort_order": sort_order} + """The database orders the resources as SearchRequest.sort does, ties by identifier.""" + search_request = SearchRequest(sort_by=sort_by, sort_order=sort_order) + by_id = sql.search([user_type], SearchRequest()).resources - assert user_names(sql, user_type, **parameters) == user_names( - memory, user_type, **parameters - ) + assert user_names(sql, user_type, sort_by=sort_by, sort_order=sort_order) == [ + resource.user_name for resource in search_request.sort(by_id) + ] def test_a_page_is_cut_after_filtering_and_sorting(memory, sql, user_type): @@ -161,7 +159,7 @@ def test_a_page_is_cut_after_filtering_and_sorting(memory, sql, user_type): "start_index": 2, "count": 2, } - total, resources = sql.search([user_type], SearchRequest(**parameters)) + page = sql.search([user_type], SearchRequest(**parameters)) - assert total == 4 - assert [r.user_name for r in resources] == ["Bob", "carol"] + assert page.total == 4 + assert [r.user_name for r in page.resources] == ["Bob", "carol"] diff --git a/tests/test_storage.py b/tests/test_storage.py index 3110bf3..fea8abf 100644 --- a/tests/test_storage.py +++ b/tests/test_storage.py @@ -33,7 +33,8 @@ def search(storage, resource_type, **parameters): - return storage.search([resource_type], SearchRequest(**parameters)) + page = storage.search([resource_type], SearchRequest(**parameters)) + return page.total, page.resources def test_emails_are_read_back_in_their_order(storage, user_type): @@ -115,18 +116,18 @@ def test_a_value_whose_column_cannot_be_empty_cannot_be_removed(storage, user_ty def test_a_hybrid_property_without_setter_is_read_only(storage, user_type): - """NickName comes from userName, and is filtered on as any attribute.""" + """Locale comes from userName, and is filtered on as any attribute.""" user = storage.create(user_type, UserModel(user_name="BJensen")) - assert user.nick_name == "bjensen" - _, found = search(storage, user_type, filter='nickName eq "bjensen"') + assert user.locale == "bjensen" + _, found = search(storage, user_type, filter='locale eq "bjensen"') assert [r.user_name for r in found] == ["BJensen"] def test_a_read_only_attribute_cannot_be_changed(storage, user_type): """A value the mapping cannot write raises a 400, rather than being lost.""" with pytest.raises(MutabilityException): - storage.create(user_type, UserModel(user_name="bjensen", nick_name="babs")) + storage.create(user_type, UserModel(user_name="bjensen", locale="babs")) def test_a_read_resource_can_be_written_back(storage, user_type, group_type): diff --git a/uv.lock b/uv.lock index 445878e..098cf65 100644 --- a/uv.lock +++ b/uv.lock @@ -1277,27 +1277,27 @@ wheels = [ [[package]] name = "scim2-models" -version = "0.12.1" +version = "0.12.2" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "lark" }, { name = "pydantic", extra = ["email"] }, ] -sdist = { url = "https://files.pythonhosted.org/packages/4f/76/1998e59747e445740a9ba3ac516a652167a869f247dce6be931d8725053d/scim2_models-0.12.1.tar.gz", hash = "sha256:13e2dc9aad991a05b24c2ae902c239ee6ed041ea8cd7551ed8f8ac4ac5011e96", size = 108082, upload-time = "2026-10-07T12:46:54.705Z" } +sdist = { url = "https://files.pythonhosted.org/packages/5b/be/cba7330563df2f809db0e8cddd0ed0d64beddb42ff3af1eec6cacbf449c7/scim2_models-0.12.2.tar.gz", hash = "sha256:c8b1f7ae25af4874dba53edd2855ce126f7594d0dbe694a6e9bb32c369082234", size = 108565, upload-time = "2026-10-09T14:54:39.665Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/22/23/3e92c9ee55dba37009aa3934ffbe26da53de44038510268d293b683fd737/scim2_models-0.12.1-py3-none-any.whl", hash = "sha256:b127b5be4e915d73f3a0b8bfab48cd695516defe45bbb8c45683a9b5dc8da636", size = 130870, upload-time = "2026-10-07T12:46:53.323Z" }, + { url = "https://files.pythonhosted.org/packages/c2/dc/3c3e0ea81420937a8502747c848379172608b0f9c5f77da9b796b19c3258/scim2_models-0.12.2-py3-none-any.whl", hash = "sha256:721b627f9ae50d24956103ba9fc2e852a1badbada8d25c0939bc8656881af62a", size = 131343, upload-time = "2026-10-09T14:54:38.229Z" }, ] [[package]] name = "scim2-server" -version = "0.7.1" +version = "0.8.0" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "scim2-models" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/0f/19/79b9c8ee822c3a149645fb090006259a253d731bd7635bf41412443089f3/scim2_server-0.7.1.tar.gz", hash = "sha256:c7a3271d307677a8ab7d912ba3b6e57a54c431a842a345982b17bdadcdabeac1", size = 48686, upload-time = "2026-10-07T17:40:22.526Z" } +sdist = { url = "https://files.pythonhosted.org/packages/99/86/1f39b73322199d9cb6a4b5e8aaaaea3c68c3838b337c1b16590fbdfe9848/scim2_server-0.8.0.tar.gz", hash = "sha256:cdfcdbd6ac072f15322558df3253ca02b0af3722a5b7506467eeb6f251b6051a", size = 56042, upload-time = "2026-10-09T21:08:40.219Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/ca/cd/4c30aee55cb2c3b7bd502e876ae26cec33d5e5c537d93b963ca671d3bb09/scim2_server-0.7.1-py3-none-any.whl", hash = "sha256:f788d351b61a3c32c6c1a572f6d90123206616cab3945755f4278b2528c3fa48", size = 57219, upload-time = "2026-10-07T17:40:20.615Z" }, + { url = "https://files.pythonhosted.org/packages/2e/d9/0d32c734cb5d282be58a846a3d91f8ddaea06775cc79c0cfb2364aee3bdd/scim2_server-0.8.0-py3-none-any.whl", hash = "sha256:23e3bcfd6f13a43b3f14498ba3181d4d6132f0f2512e30479f5b8a1f8e848b3e", size = 64840, upload-time = "2026-10-09T21:08:38.749Z" }, ] [package.optional-dependencies] @@ -1346,8 +1346,8 @@ doc = [ [package.metadata] requires-dist = [ - { name = "scim2-models", specifier = ">=0.12.1" }, - { name = "scim2-server", specifier = ">=0.7.1" }, + { name = "scim2-models", specifier = ">=0.12.2" }, + { name = "scim2-server", specifier = ">=0.8.0" }, { name = "sqlalchemy", specifier = ">=2.0.41" }, { name = "sqlalchemy", extras = ["asyncio"], marker = "extra == 'asyncio'", specifier = ">=2.0.41" }, ] @@ -1363,7 +1363,7 @@ dev = [ { name = "pytest-cov", specifier = ">=7.1.0" }, { name = "pytest-postgresql", specifier = ">=9.1.0" }, { name = "scim2-client", specifier = ">=0.12.0" }, - { name = "scim2-server", extras = ["testing"], specifier = ">=0.7.1" }, + { name = "scim2-server", extras = ["testing"], specifier = ">=0.8.0" }, { name = "scim2-tester", specifier = ">=0.5.2" }, { name = "sqlalchemy", extras = ["asyncio"], specifier = ">=2.0.41" }, { name = "tox-uv", specifier = ">=1.36.0" },