refactor: remove the clock parameter of the storages

The storages date every write with the current time in UTC. The clock parameter came from the in-memory storage of scim2-server, and is not part of the storage contract. A test now checks that a change of links only, such as a new member, gives the resource a new version.

Author: azmeuk

doc/how-to/map-a-data-model.rst

@@ -35,8 +35,8 @@ types. Use a UUID, with the :class:`~sqlalchemy.types.Uuid` type, or a sequence
 table shares.
 
 The primary key may be an integer, a string or a UUID, and the storage returns it as a string.
-The storage does not fill the key: give its column a default, such as ``default=uuid.uuid4``, a
-server default, or an autoincrement. The storage reads the key the database gives.
+Give its column a default, such as ``default=uuid.uuid4``, a server default, or an
+autoincrement, so that the database fills the key of each new resource.
 
 Map a column
 ------------

src/scim2_sqlalchemy/storage.py

@@ -72,11 +72,9 @@ class _StorageBase:
     def __init__(
         self,
         mappings: Mapping[str, ResourceMapping],
-        clock: Callable[[], datetime.datetime] | None,
         provider: ScimProvider | None,
     ) -> None:
         self.mappings = dict(mappings)
-        self.clock = clock or _utcnow
         self.provider = provider
         self._policy = provider.policy if provider else ScimPolicy()
         self._comparators: dict[str, _Comparator] = {}
@@ -198,7 +196,7 @@ def _new_record(self, resource_type: ResourceType, resource: Resource[Any]) -> A
         """
         mapping = self._mapping(resource_type)
         record = mapping.record()
-        now = self.clock()
+        now = _utcnow()
         setattr(record, mapping._created.key, now)
         setattr(record, mapping._last_modified.key, now)
         return record
@@ -210,8 +208,13 @@ def _write_record(
         record: Any,
         links: dict[_Collection, list[Any]],
     ) -> None:
+        """Write a resource in its record, and date the write.
+
+        The date changes the row even when only links change, such as the
+        members of a group, so that the version changes too.
+        """
         _from_scim(mapping, resource, record, links, creating=False)
-        setattr(record, mapping._last_modified.key, self.clock())
+        setattr(record, mapping._last_modified.key, _utcnow())
 
 
 class SqlAlchemyStorage(_StorageBase, ScimStorage):
@@ -230,8 +233,6 @@ class SqlAlchemyStorage(_StorageBase, ScimStorage):
     :param mappings: The mapping of each resource type, by resource type name.
     :param session: Return the current :class:`~sqlalchemy.orm.Session`, such
         as a :class:`~sqlalchemy.orm.scoped_session`.
-    :param clock: Return the date of a write, for ``meta.created`` and
-        ``meta.lastModified``. The current date in UTC by default.
     :param provider: The provider of the server. The storage then returns
         instances of its models, rather than of the models of the mappings,
         and gives each link a ``$ref`` relative to the SCIM root, from the
@@ -243,10 +244,9 @@ def __init__(
         self,
         mappings: Mapping[str, ResourceMapping],
         session: Callable[[], Session],
-        clock: Callable[[], datetime.datetime] | None = None,
         provider: ScimProvider | None = None,
     ) -> None:
-        super().__init__(mappings, clock, provider)
+        super().__init__(mappings, provider)
         self.session = session
 
     @contextmanager
@@ -418,8 +418,6 @@ class AsyncSqlAlchemyStorage(_StorageBase, AsyncScimStorage):
     :param session: Return the current
         :class:`~sqlalchemy.ext.asyncio.AsyncSession`, such as an
         :class:`~sqlalchemy.ext.asyncio.async_scoped_session`.
-    :param clock: Return the date of a write, for ``meta.created`` and
-        ``meta.lastModified``. The current date in UTC by default.
     :param provider: The provider of the server. The storage then returns
         instances of its models, rather than of the models of the mappings.
         Strings are compared with the
@@ -430,10 +428,9 @@ def __init__(
         self,
         mappings: Mapping[str, ResourceMapping],
         session: Callable[[], AsyncSession],
-        clock: Callable[[], datetime.datetime] | None = None,
         provider: ScimProvider | None = None,
     ) -> None:
-        super().__init__(mappings, clock, provider)
+        super().__init__(mappings, provider)
         self.session = session
 
     @asynccontextmanager

tests/test_storage.py

@@ -538,6 +538,18 @@ def test_a_uuid_identifier_matches_its_own_text(storage, user_type, scim_filter,
     assert total == (1 if found else 0)
 
 
+def test_a_change_of_links_only_changes_the_version(storage, user_type, group_type):
+    """Adding a member only writes the association table, yet the group gets a new version."""
+    bob = storage.create(user_type, User(user_name="bob"))
+    group = storage.create(group_type, Group(display_name="Admins"))
+    group.members = [{"value": bob.id}]
+
+    updated = storage.update(group_type, group)
+
+    assert updated.meta.version != group.meta.version
+    assert updated.meta.last_modified > group.meta.last_modified
+
+
 class ChangedMeanwhile:
     """Make the record look loaded at another version, as when another transaction changes it right before the write."""