From 8c2743474386738d1a5dc83e0dc0562cba3e1b2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89loi=20Rivard?= Date: Sun, 25 Oct 2026 21:36:39 +0100 Subject: [PATCH 1/5] perf: copy only the returned page in the in-memory search --- doc/changelog.rst | 8 ++++++++ scim2_server/memory.py | 18 +++++++++++------- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/doc/changelog.rst b/doc/changelog.rst index d45ecf3..d80fe0e 100644 --- a/doc/changelog.rst +++ b/doc/changelog.rst @@ -1,6 +1,14 @@ Changelog ========= +[Unreleased] +------------ + +Changed +^^^^^^^ +- :class:`~scim2_server.memory.InMemoryStorage` searches faster. It copies only the resources of + the returned page. + [0.8.2] - 2026-10-23 -------------------- diff --git a/scim2_server/memory.py b/scim2_server/memory.py index cb85312..2983380 100644 --- a/scim2_server/memory.py +++ b/scim2_server/memory.py @@ -202,18 +202,22 @@ def search( ) -> SearchPage: with self.lock: candidates = [ - resource.model_copy(deep=True) + resource for resource in self.resources if any(self._is_of_type(resource, rt) for rt in resource_types) ] - scim_filter = search_request.filter - if scim_filter is not None and not scim_filter.models and candidates: - models = tuple(dict.fromkeys(type(r) for r in candidates)) - scim_filter = parametrize(ScimFilter, Union[models])(str(scim_filter)) # noqa: UP007 + scim_filter = search_request.filter + if scim_filter is not None and not scim_filter.models and candidates: + models = tuple(dict.fromkeys(type(r) for r in candidates)) + scim_filter = parametrize(ScimFilter, Union[models])(str(scim_filter)) # noqa: UP007 - found = [r for r in candidates if scim_filter is None or scim_filter.match(r)] - return _page(search_request, position, found) + found = [ + r for r in candidates if scim_filter is None or scim_filter.match(r) + ] + page = _page(search_request, position, found) + page.resources = [r.model_copy(deep=True) for r in page.resources] + return page def create( self, resource_type: ResourceType, resource: Resource[Any] From d82867de4263fd5ff0cd3c13a45e6e566c319a95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89loi=20Rivard?= Date: Sun, 25 Oct 2026 21:39:59 +0100 Subject: [PATCH 2/5] perf: store the in-memory resources by type and id --- doc/changelog.rst | 4 +-- scim2_server/memory.py | 64 ++++++++++++++++++++++++++---------------- tests/test_memory.py | 15 ++++++++++ 3 files changed, 57 insertions(+), 26 deletions(-) diff --git a/doc/changelog.rst b/doc/changelog.rst index d80fe0e..c9fdfe4 100644 --- a/doc/changelog.rst +++ b/doc/changelog.rst @@ -6,8 +6,8 @@ Changelog Changed ^^^^^^^ -- :class:`~scim2_server.memory.InMemoryStorage` searches faster. It copies only the resources of - the returned page. +- :class:`~scim2_server.memory.InMemoryStorage` reads and searches faster. A search copies only + the resources of the returned page. [0.8.2] - 2026-10-23 -------------------- diff --git a/scim2_server/memory.py b/scim2_server/memory.py index 2983380..cb01d21 100644 --- a/scim2_server/memory.py +++ b/scim2_server/memory.py @@ -155,11 +155,26 @@ class InMemoryStorage(ScimStorage): supports_cursors = True def __init__(self, clock: Callable[[], datetime.datetime] = utcnow) -> None: - self.resources: list[Resource[Any]] = [] + self._resources: dict[str | None, dict[str, Resource[Any]]] = {} self.clock = clock self.lock = RLock() self.versions = itertools.count(1) + @property + def resources(self) -> list[Resource[Any]]: + """The stored resources.""" + return [ + resource + for resources in self._resources.values() + for resource in resources.values() + ] + + @resources.setter + def resources(self, resources: list[Resource[Any]]) -> None: + self._resources = {} + for resource in resources: + self._store(resource) + def generate_id(self, resource_type: ResourceType, resource: Resource[Any]) -> str: """Return the identifier of a new resource. @@ -189,9 +204,7 @@ def get( response_parameters: ResponseParameters[Any] | None = None, ) -> Resource[Any]: with self.lock: - return self.resources[self._index(resource_type, resource_id)].model_copy( - deep=True - ) + return self._stored(resource_type, resource_id).model_copy(deep=True) def search( self, @@ -201,10 +214,11 @@ def search( position: Any = None, ) -> SearchPage: with self.lock: + names = dict.fromkeys(rt.name for rt in resource_types) candidates = [ resource - for resource in self.resources - if any(self._is_of_type(resource, rt) for rt in resource_types) + for name in names + for resource in self._resources.get(name, {}).values() ] scim_filter = search_request.filter @@ -233,7 +247,7 @@ def create( version=self.next_version(), ) self._check_uniqueness(resource) - self.resources.append(resource) + self._store(resource) return resource.model_copy(deep=True) def update( @@ -244,8 +258,7 @@ def update( expected_version: str | None = None, ) -> Resource[Any]: with self.lock: - index = self._index(resource_type, resource.id) - stored = self.resources[index] + stored = self._stored(resource_type, resource.id) self._check_version(stored, expected_version) updated = type(resource).model_validate(resource.model_dump()) @@ -258,7 +271,7 @@ def update( } ) self._check_uniqueness(updated) - self.resources[index] = updated + self._store(updated) return updated.model_copy(deep=True) def delete( @@ -269,26 +282,29 @@ def delete( expected_version: str | None = None, ) -> None: with self.lock: - index = self._index(resource_type, resource_id) - self._check_version(self.resources[index], expected_version) - del self.resources[index] + stored = self._stored(resource_type, resource_id) + self._check_version(stored, expected_version) + del self._resources[resource_type.name][resource_id] - def _is_of_type(self, resource: Resource[Any], resource_type: ResourceType) -> bool: - """Tell whether a resource belongs to a resource type. + def _store(self, resource: Resource[Any]) -> None: + """Keep a resource under its resource type and its identifier. Per RFC 7643 §3.1, meta.resourceType holds the name of the resource type, which may differ from its id. """ assert resource.meta is not None - return resource.meta.resource_type == resource_type.name - - def _index(self, resource_type: ResourceType, resource_id: str | None) -> int: - for index, resource in enumerate(self.resources): - if self._is_of_type(resource, resource_type) and resource.id == resource_id: - return index - raise NotFoundException( - detail=f"{resource_type.name} {resource_id!r} not found" - ) + resources = self._resources.setdefault(resource.meta.resource_type, {}) + resources[cast(str, resource.id)] = resource + + def _stored( + self, resource_type: ResourceType, resource_id: str | None + ) -> Resource[Any]: + try: + return self._resources[resource_type.name][cast(str, resource_id)] + except KeyError: + raise NotFoundException( + detail=f"{resource_type.name} {resource_id!r} not found" + ) from None @staticmethod def _check_version(resource: Resource[Any], expected_version: str | None) -> None: diff --git a/tests/test_memory.py b/tests/test_memory.py index 7e8d27d..9b01622 100644 --- a/tests/test_memory.py +++ b/tests/test_memory.py @@ -11,6 +11,7 @@ from scim2_models import Filter from scim2_models import InvalidFilterException from scim2_models import InvalidPathException +from scim2_models import NotFoundException from scim2_models import Resource from scim2_models import ResourceType from scim2_models import Schema @@ -207,6 +208,20 @@ def test_versions_differ_within_the_same_instant(user_type, scim_provider): assert updated.meta.version != user.meta.version +def test_the_resources_can_be_replaced(user_type, scim_provider): + """Resources assigned to a storage can be read, and the former ones are gone.""" + storage = InMemoryStorage() + User = scim_provider.model_for(user_type) + former = storage.create(user_type, User(user_name="bjensen")) + kept = InMemoryStorage().create(user_type, User(user_name="jsmith")) + + storage.resources = [kept] + + assert storage.get(user_type, kept.id).user_name == "jsmith" + with pytest.raises(NotFoundException): + storage.get(user_type, former.id) + + def test_operations_can_be_nested(user_type, scim_provider): """The lock of an operation can be taken again inside it.""" storage = InMemoryStorage() From 8a3ad1b1b3b9510f4857fbdff14a433faa0f0091 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89loi=20Rivard?= Date: Sun, 25 Oct 2026 21:45:14 +0100 Subject: [PATCH 3/5] test: check that the contract storages free the unique values --- doc/changelog.rst | 4 ++++ doc/how-to/check-a-storage.rst | 14 +++++++------- scim2_server/testing.py | 19 +++++++++++++++++++ 3 files changed, 30 insertions(+), 7 deletions(-) diff --git a/doc/changelog.rst b/doc/changelog.rst index c9fdfe4..b27f457 100644 --- a/doc/changelog.rst +++ b/doc/changelog.rst @@ -4,6 +4,10 @@ Changelog [Unreleased] ------------ +Added +^^^^^ +- The storage contract checks that an update or a deletion frees the unique values of a resource. + Changed ^^^^^^^ - :class:`~scim2_server.memory.InMemoryStorage` reads and searches faster. A search copies only diff --git a/doc/how-to/check-a-storage.rst b/doc/how-to/check-a-storage.rst index a18eac1..29d2280 100644 --- a/doc/how-to/check-a-storage.rst +++ b/doc/how-to/check-a-storage.rst @@ -78,9 +78,9 @@ Run the suite .. code-block:: console $ pytest - ........................................................................ [ 75%] - ........................ [100%] - 96 passed in 6.09s + ........................................................................ [ 66%] + .................................... [100%] + 108 passed in 6.09s Skip what the storage does not support -------------------------------------- @@ -101,7 +101,7 @@ override the ``provider`` fixture. In its :class:`~scim2_models.ServiceProviderC SKIPPED [46] scim2_server/testing.py:216: The service does not support filtering SKIPPED [19] scim2_server/testing.py:216: The service does not support sorting SKIPPED [10] scim2_server/testing.py:216: The service does not support cursor pagination - ======================= 31 passed, 75 skipped in 6.57s ========================= + ======================= 33 passed, 75 skipped in 6.57s ========================= For a storage that cannot search several resource types at once, set :attr:`~scim2_server.testing.ScimStorageContract.supports_root_search` to :data:`False`: @@ -114,7 +114,7 @@ For a storage that cannot search several resource types at once, set $ pytest -rs SKIPPED [2] scim2_server/testing.py:216: The service does not support searching at the root SKIPPED [10] scim2_server/testing.py:216: The service does not support cursor pagination - ======================= 94 passed, 12 skipped in 6.79s ========================= + ======================= 96 passed, 12 skipped in 6.79s ========================= A storage refuses a filter it cannot translate with :class:`~scim2_models.InvalidFilterException`, and a sort with :class:`~scim2_models.InvalidPathException`. The suite skips the filters and the @@ -123,8 +123,8 @@ sorts the storage refuses, and checks the others: .. code-block:: console $ pytest -rs - SKIPPED [11] scim2_server/testing.py:669: The storage refuses the search: Cannot filter on 'title' - SKIPPED [2] scim2_server/testing.py:669: The storage refuses the search: Cannot sort on 'title' + SKIPPED [11] scim2_server/testing.py:700: The storage refuses the search: Cannot filter on 'title' + SKIPPED [2] scim2_server/testing.py:700: The storage refuses the search: Cannot sort on 'title' The filters and the sorts search users with the attributes of the ``User`` resource type and of its enterprise extension. For a storage that does not store some of them, override diff --git a/scim2_server/testing.py b/scim2_server/testing.py index 46e17ce..66af00c 100644 --- a/scim2_server/testing.py +++ b/scim2_server/testing.py @@ -501,6 +501,25 @@ def test_update_with_a_taken_unique_value( storage.update(user_type, user) assert storage.get(user_type, user.id).user_name == "jsmith" + def test_update_frees_the_former_unique_values( + self, storage: Any, user_type: ResourceType, user_model: Any + ) -> None: + """A userName a resource gave up can be taken by another resource.""" + (user,) = self.create_users(storage, user_type, user_model, "bjensen") + user.user_name = "babs" + storage.update(user_type, user) + + self.create_users(storage, user_type, user_model, "bjensen") + + def test_delete_frees_the_unique_values( + self, storage: Any, user_type: ResourceType, user_model: Any + ) -> None: + """The userName of a deleted resource can be taken by a new resource.""" + (user,) = self.create_users(storage, user_type, user_model, "bjensen") + storage.delete(user_type, user.id) + + self.create_users(storage, user_type, user_model, "bjensen") + def test_search_a_resource_type( self, storage: Any, From b56f2c1f8a5c6e9e831688850642797b8aa4591f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89loi=20Rivard?= Date: Sun, 25 Oct 2026 21:45:25 +0100 Subject: [PATCH 4/5] perf: index the unique values in the in-memory storage --- doc/changelog.rst | 6 ++- scim2_server/memory.py | 102 ++++++++++++++++++++++++++++++----------- tests/test_memory.py | 25 ++++++++++ 3 files changed, 105 insertions(+), 28 deletions(-) diff --git a/doc/changelog.rst b/doc/changelog.rst index b27f457..a48bb3d 100644 --- a/doc/changelog.rst +++ b/doc/changelog.rst @@ -10,8 +10,10 @@ Added Changed ^^^^^^^ -- :class:`~scim2_server.memory.InMemoryStorage` reads and searches faster. A search copies only - the resources of the returned page. +- :class:`~scim2_server.memory.InMemoryStorage` reads, writes and searches faster. A search copies + only the resources of the returned page, and the unique values are indexed. +- :class:`~scim2_server.memory.InMemoryStorage` compares unique values under the policy in effect + when they were written, so it serves the resources of a single policy. [0.8.2] - 2026-10-23 -------------------- diff --git a/scim2_server/memory.py b/scim2_server/memory.py index cb01d21..8863962 100644 --- a/scim2_server/memory.py +++ b/scim2_server/memory.py @@ -142,12 +142,41 @@ def _decode(value: Any) -> Any: raise InvalidCursorException from None +Key = tuple[str | None, str] + + +def _key(resource: Resource[Any]) -> Key: + """Return the resource type and the identifier a resource is stored under. + + Per RFC 7643 §3.1, meta.resourceType holds the name of the resource + type, which may differ from its id. + """ + assert resource.meta is not None + return resource.meta.resource_type, cast(str, resource.id) + + +def _hashable(value: Any) -> Any: + """Return a form of a value that a dict can hold as a key. + + Lists and models cannot be hashed, so they are held by their + representation. + """ + try: + hash(value) + except TypeError: + return repr(value) + return value + + class InMemoryStorage(ScimStorage): """A storage keeping the resources in memory, for tests, debugging and demos. It is not optimized for performance: a search walks every resource. A lock serializes every access, so it can serve a threaded server. + It compares unique values under the policy in effect when they were + written, so it serves the resources of a single policy. + :param clock: Return the date of a write, for ``meta.created`` and ``meta.lastModified``. Pass a fixed clock to get predictable dates. """ @@ -156,6 +185,8 @@ class InMemoryStorage(ScimStorage): def __init__(self, clock: Callable[[], datetime.datetime] = utcnow) -> None: self._resources: dict[str | None, dict[str, Resource[Any]]] = {} + self._owners: dict[str, dict[Any, Key]] = {} + self._unique_values: dict[Key, dict[str, Any]] = {} self.clock = clock self.lock = RLock() self.versions = itertools.count(1) @@ -172,8 +203,10 @@ def resources(self) -> list[Resource[Any]]: @resources.setter def resources(self, resources: list[Resource[Any]]) -> None: self._resources = {} + self._owners = {} + self._unique_values = {} for resource in resources: - self._store(resource) + self._store(resource, self._values_of(resource)) def generate_id(self, resource_type: ResourceType, resource: Resource[Any]) -> str: """Return the identifier of a new resource. @@ -246,8 +279,9 @@ def create( last_modified=now, version=self.next_version(), ) - self._check_uniqueness(resource) - self._store(resource) + values = self._values_of(resource) + self._check_uniqueness(resource, values) + self._store(resource, values) return resource.model_copy(deep=True) def update( @@ -270,8 +304,9 @@ def update( "version": self.next_version(), } ) - self._check_uniqueness(updated) - self._store(updated) + values = self._values_of(updated) + self._check_uniqueness(updated, values) + self._store(updated, values) return updated.model_copy(deep=True) def delete( @@ -285,16 +320,21 @@ def delete( stored = self._stored(resource_type, resource_id) self._check_version(stored, expected_version) del self._resources[resource_type.name][resource_id] - - def _store(self, resource: Resource[Any]) -> None: - """Keep a resource under its resource type and its identifier. - - Per RFC 7643 §3.1, meta.resourceType holds the name of the resource - type, which may differ from its id. - """ - assert resource.meta is not None - resources = self._resources.setdefault(resource.meta.resource_type, {}) - resources[cast(str, resource.id)] = resource + self._release(_key(stored)) + + def _store(self, resource: Resource[Any], values: dict[str, Any]) -> None: + """Keep a resource under its resource type and its identifier, and own its unique values.""" + key = _key(resource) + self._resources.setdefault(key[0], {})[key[1]] = resource + self._release(key) + self._unique_values[key] = values + for urn, value in values.items(): + self._owners.setdefault(urn, {})[value] = key + + def _release(self, key: Key) -> None: + """Free the unique values a resource owned.""" + for urn, value in self._unique_values.pop(key, {}).items(): + del self._owners[urn][value] def _stored( self, resource_type: ResourceType, resource_id: str | None @@ -312,28 +352,38 @@ def _check_version(resource: Resource[Any], expected_version: str | None) -> Non if expected_version is not None and resource.meta.version != expected_version: raise PreconditionFailedException - def _check_uniqueness(self, resource: Resource[Any]) -> None: - """Refuse a resource sharing a unique value with another one of the same schema. + def _check_uniqueness( + self, resource: Resource[Any], values: dict[str, Any] + ) -> None: + """Refuse a resource sharing a unique value with another resource.""" + key = _key(resource) + for urn, value in values.items(): + if self._owners.get(urn, {}).get(value, key) != key: + raise UniquenessException() + + def _values_of(self, resource: Resource[Any]) -> dict[str, Any]: + """Return the unique values of a resource, by the URN of their attribute. Per RFC 7643 erratum 8279, the uniqueness applies to the resources using the schema that declares the attribute, whatever their resource - type. A missing value never clashes, as a SQL NULL does not. + type. The URN of an attribute holds its schema, so two resources clash + when they hold the same value under the same URN. A missing value + never clashes, as a SQL NULL does not. + + The values are compared under the policy of the write that stored + them, so a storage serves the resources of a single policy. """ unique_paths = parametrize(Path, type(resource)).iter_paths( include_subattributes=False, uniqueness=[Uniqueness.server, Uniqueness.global_], ) + values = {} for path in unique_paths: attribute = cast(AttributeBinding, path.resolve()) value = self._unique_value(resource, attribute) - if value is None: - continue - for existing_resource in self.resources: - if ( - existing_resource.id != resource.id - and self._unique_value(existing_resource, attribute) == value - ): - raise UniquenessException() + if value is not None: + values[attribute.urn] = _hashable(value) + return values @staticmethod def _unique_value(resource: Resource[Any], attribute: AttributeBinding) -> Any: diff --git a/tests/test_memory.py b/tests/test_memory.py index 9b01622..a033408 100644 --- a/tests/test_memory.py +++ b/tests/test_memory.py @@ -7,6 +7,7 @@ from scim2_models import URN from scim2_models import Attribute from scim2_models import CaseExact +from scim2_models import ComplexAttribute from scim2_models import Extension from scim2_models import Filter from scim2_models import InvalidFilterException @@ -220,6 +221,9 @@ def test_the_resources_can_be_replaced(user_type, scim_provider): assert storage.get(user_type, kept.id).user_name == "jsmith" with pytest.raises(NotFoundException): storage.get(user_type, former.id) + storage.create(user_type, User(user_name="bjensen")) + with pytest.raises(UniquenessException): + storage.create(user_type, User(user_name="jsmith")) def test_operations_can_be_nested(user_type, scim_provider): @@ -377,6 +381,27 @@ class Token(Resource): ) +def test_multi_valued_and_complex_unique_values_clash_when_equal(): + """Equal lists, or equal complex values, of a unique attribute clash.""" + + class Size(ComplexAttribute): + width: int | None = None + + class Badge(Resource): + __schema__ = URN("urn:example:2.0:Badge") + codes: Annotated[list[str] | None, Uniqueness.server] = None + size: Annotated[Size | None, Uniqueness.server] = None + + storage = InMemoryStorage() + badge_type = ResourceType.from_resource(Badge) + storage.create(badge_type, Badge(codes=["a", "b"], size=Size(width=1))) + storage.create(badge_type, Badge(codes=["a"], size=Size(width=2))) + with pytest.raises(UniquenessException): + storage.create(badge_type, Badge(codes=["a", "b"])) + with pytest.raises(UniquenessException): + storage.create(badge_type, Badge(size=Size(width=2))) + + @pytest.mark.parametrize("root", [False, True]) def test_a_filter_bound_to_no_model_is_resolved_against_the_stored_resources( user_type, scim_provider, root From b302fcdef0f962bc4667a56e54161863e5f82178 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89loi=20Rivard?= Date: Sun, 25 Oct 2026 21:54:47 +0100 Subject: [PATCH 5/5] perf: index the values searched with eq in the in-memory storage --- doc/changelog.rst | 7 +-- scim2_server/memory.py | 112 +++++++++++++++++++++++++++++++++-------- tests/test_memory.py | 66 +++++++++++++++++++++++- 3 files changed, 161 insertions(+), 24 deletions(-) diff --git a/doc/changelog.rst b/doc/changelog.rst index a48bb3d..891661c 100644 --- a/doc/changelog.rst +++ b/doc/changelog.rst @@ -11,9 +11,10 @@ Added Changed ^^^^^^^ - :class:`~scim2_server.memory.InMemoryStorage` reads, writes and searches faster. A search copies - only the resources of the returned page, and the unique values are indexed. -- :class:`~scim2_server.memory.InMemoryStorage` compares unique values under the policy in effect - when they were written, so it serves the resources of a single policy. + only the resources of the returned page. The unique values are indexed, and so are the values a + search compares with ``eq``, such as ``externalId eq "…"``. +- :class:`~scim2_server.memory.InMemoryStorage` indexes the values under the policy in effect when + they were written, so it serves the resources of a single policy. [0.8.2] - 2026-10-23 -------------------- diff --git a/scim2_server/memory.py b/scim2_server/memory.py index 8863962..f6c212f 100644 --- a/scim2_server/memory.py +++ b/scim2_server/memory.py @@ -22,6 +22,8 @@ from scim2_models import SearchRequest from scim2_models import Uniqueness from scim2_models import UniquenessException +from scim2_models.path import CompareOperator +from scim2_models.path import Comparison from scim2_server.storage import AsyncScimStorage from scim2_server.storage import ScimStorage @@ -155,6 +157,23 @@ def _key(resource: Resource[Any]) -> Key: return resource.meta.resource_type, cast(str, resource.id) +def _equality(scim_filter: ScimFilter[Any] | None) -> AttributeBinding | None: + """Return the attribute a filter compares to a string with eq, if it holds a single string.""" + if scim_filter is None or scim_filter.model is None: + return None + node = scim_filter.ast + if ( + not isinstance(node, Comparison) + or node.op != CompareOperator.eq + or not isinstance(node.value, str) + ): + return None + binding = scim_filter.resolve_comparison(node.attr_path, strict=False) + if binding is None or binding.is_multivalued or binding.target_type is not str: + return None + return binding + + def _hashable(value: Any) -> Any: """Return a form of a value that a dict can hold as a key. @@ -171,11 +190,13 @@ def _hashable(value: Any) -> Any: class InMemoryStorage(ScimStorage): """A storage keeping the resources in memory, for tests, debugging and demos. - It is not optimized for performance: a search walks every resource. A - lock serializes every access, so it can serve a threaded server. + It indexes the resources by identifier and the unique values. A search + walks every resource of the searched types, unless it compares a single + string attribute to a string with ``eq``. A lock serializes every access, + so it can serve a threaded server. - It compares unique values under the policy in effect when they were - written, so it serves the resources of a single policy. + It indexes the values under the policy in effect when they were written, + so it serves the resources of a single policy. :param clock: Return the date of a write, for ``meta.created`` and ``meta.lastModified``. Pass a fixed clock to get predictable dates. @@ -187,6 +208,10 @@ def __init__(self, clock: Callable[[], datetime.datetime] = utcnow) -> None: self._resources: dict[str | None, dict[str, Resource[Any]]] = {} self._owners: dict[str, dict[Any, Key]] = {} self._unique_values: dict[Key, dict[str, Any]] = {} + self._equalities: dict[ + str | None, dict[AttributeBinding, dict[Any, set[str]]] + ] = {} + self._equal_values: dict[Key, dict[AttributeBinding, Any]] = {} self.clock = clock self.lock = RLock() self.versions = itertools.count(1) @@ -205,6 +230,8 @@ def resources(self, resources: list[Resource[Any]]) -> None: self._resources = {} self._owners = {} self._unique_values = {} + self._equalities = {} + self._equal_values = {} for resource in resources: self._store(resource, self._values_of(resource)) @@ -247,14 +274,10 @@ def search( position: Any = None, ) -> SearchPage: with self.lock: - names = dict.fromkeys(rt.name for rt in resource_types) - candidates = [ - resource - for name in names - for resource in self._resources.get(name, {}).values() - ] - + names = list(dict.fromkeys(rt.name for rt in resource_types)) scim_filter = search_request.filter + candidates = self._candidates(names, scim_filter) + if scim_filter is not None and not scim_filter.models and candidates: models = tuple(dict.fromkeys(type(r) for r in candidates)) scim_filter = parametrize(ScimFilter, Union[models])(str(scim_filter)) # noqa: UP007 @@ -319,22 +342,71 @@ def delete( with self.lock: stored = self._stored(resource_type, resource_id) self._check_version(stored, expected_version) - del self._resources[resource_type.name][resource_id] - self._release(_key(stored)) + self._remove(_key(stored)) + + def _candidates( + self, names: list[str | None], scim_filter: ScimFilter[Any] | None + ) -> list[Resource[Any]]: + """Return the resources a search walks. + + A search on a single resource type, comparing a single string + attribute to a string with eq, walks only the resources holding an + equal value. The values are indexed on the first such search. + """ + binding = _equality(scim_filter) if len(names) == 1 else None + if binding is None: + return [ + resource + for name in names + for resource in self._resources.get(name, {}).values() + ] + + node = cast(Comparison, cast(ScimFilter[Any], scim_filter).ast) + try: + value = binding.comparable(node.value) + except ValueError: + return [] + resources = self._resources.get(names[0], {}) + ids = self._equality_index(names[0], binding).get(value, set()) + return [resources[resource_id] for resource_id in ids] + + def _equality_index( + self, name: str | None, binding: AttributeBinding + ) -> dict[Any, set[str]]: + """Return the identifiers of the resources of a type, by the value of an attribute.""" + indexes = self._equalities.setdefault(name, {}) + if binding not in indexes: + indexes[binding] = {} + for resource in self._resources.get(name, {}).values(): + self._index_equality(resource, binding) + return indexes[binding] + + def _index_equality( + self, resource: Resource[Any], binding: AttributeBinding + ) -> None: + key = _key(resource) + value = self._comparable_value(resource, binding) + self._equal_values.setdefault(key, {})[binding] = value + self._equalities[key[0]][binding].setdefault(value, set()).add(key[1]) def _store(self, resource: Resource[Any], values: dict[str, Any]) -> None: - """Keep a resource under its resource type and its identifier, and own its unique values.""" + """Keep a resource under its resource type and its identifier, and index its values.""" key = _key(resource) + self._remove(key) self._resources.setdefault(key[0], {})[key[1]] = resource - self._release(key) self._unique_values[key] = values for urn, value in values.items(): self._owners.setdefault(urn, {})[value] = key + for binding in self._equalities.get(key[0], {}): + self._index_equality(resource, binding) - def _release(self, key: Key) -> None: - """Free the unique values a resource owned.""" + def _remove(self, key: Key) -> None: + """Forget a resource and its indexed values.""" + self._resources.get(key[0], {}).pop(key[1], None) for urn, value in self._unique_values.pop(key, {}).items(): del self._owners[urn][value] + for binding, value in self._equal_values.pop(key, {}).items(): + self._equalities[key[0]][binding][value].discard(key[1]) def _stored( self, resource_type: ResourceType, resource_id: str | None @@ -380,14 +452,14 @@ def _values_of(self, resource: Resource[Any]) -> dict[str, Any]: values = {} for path in unique_paths: attribute = cast(AttributeBinding, path.resolve()) - value = self._unique_value(resource, attribute) + value = self._comparable_value(resource, attribute) if value is not None: values[attribute.urn] = _hashable(value) return values @staticmethod - def _unique_value(resource: Resource[Any], attribute: AttributeBinding) -> Any: - """Return the value a resource holds for a unique attribute, in the form it is compared in. + def _comparable_value(resource: Resource[Any], attribute: AttributeBinding) -> Any: + """Return the value a resource holds for an attribute, in the form it is compared in. A resource whose schemas do not declare the attribute holds no value. A string the policy cannot compare is equal to no other value, so it diff --git a/tests/test_memory.py b/tests/test_memory.py index a033408..bb97e9a 100644 --- a/tests/test_memory.py +++ b/tests/test_memory.py @@ -209,23 +209,87 @@ def test_versions_differ_within_the_same_instant(user_type, scim_provider): assert updated.meta.version != user.meta.version +def found_user_names(storage, user_type, User, scim_filter): + page = storage.search([user_type], SearchRequest[User](filter=scim_filter)) + return sorted(resource.user_name for resource in page.resources) + + def test_the_resources_can_be_replaced(user_type, scim_provider): - """Resources assigned to a storage can be read, and the former ones are gone.""" + """Resources assigned to a storage can be read and searched, and the former ones are gone.""" storage = InMemoryStorage() User = scim_provider.model_for(user_type) former = storage.create(user_type, User(user_name="bjensen")) kept = InMemoryStorage().create(user_type, User(user_name="jsmith")) + assert found_user_names(storage, user_type, User, 'userName eq "bjensen"') == [ + "bjensen" + ] storage.resources = [kept] assert storage.get(user_type, kept.id).user_name == "jsmith" with pytest.raises(NotFoundException): storage.get(user_type, former.id) + assert found_user_names(storage, user_type, User, 'userName eq "bjensen"') == [] + assert found_user_names(storage, user_type, User, 'userName eq "jsmith"') == [ + "jsmith" + ] storage.create(user_type, User(user_name="bjensen")) with pytest.raises(UniquenessException): storage.create(user_type, User(user_name="jsmith")) +def test_an_equality_search_follows_the_writes(user_type, scim_provider): + """A search for an equal value finds the resources as they are after each write.""" + storage = InMemoryStorage() + User = scim_provider.model_for(user_type) + user = storage.create(user_type, User(user_name="bjensen", external_id="a")) + assert found_user_names(storage, user_type, User, 'externalId eq "a"') == [ + "bjensen" + ] + + user.external_id = "b" + storage.update(user_type, user) + assert found_user_names(storage, user_type, User, 'externalId eq "a"') == [] + assert found_user_names(storage, user_type, User, 'externalId eq "b"') == [ + "bjensen" + ] + + storage.create(user_type, User(user_name="jsmith", external_id="b")) + assert found_user_names(storage, user_type, User, 'externalId eq "b"') == [ + "bjensen", + "jsmith", + ] + + storage.delete(user_type, user.id) + assert found_user_names(storage, user_type, User, 'externalId eq "b"') == ["jsmith"] + + +def test_an_equality_search_follows_the_case_exactness(user_type, scim_provider): + """A search for an equal value ignores the case unless the attribute is case-exact.""" + storage = InMemoryStorage() + User = scim_provider.model_for(user_type) + storage.create(user_type, User(user_name="bjensen", external_id="a")) + + assert found_user_names(storage, user_type, User, 'userName eq "BJensen"') == [ + "bjensen" + ] + assert found_user_names(storage, user_type, User, 'externalId eq "A"') == [] + + +def test_an_equality_search_for_a_value_the_policy_cannot_compare( + user_type, scim_provider +): + """A search for a value the policy refuses finds nothing.""" + storage = InMemoryStorage() + User = scim_provider.model_for(user_type) + storage.create(user_type, User(user_name="bjensen!")) + + with ScimPolicy(comparison_key=refuse_exclamation_marks): + assert ( + found_user_names(storage, user_type, User, 'userName eq "bjensen!"') == [] + ) + + def test_operations_can_be_nested(user_type, scim_provider): """The lock of an operation can be taken again inside it.""" storage = InMemoryStorage()