Stop committing in read-only repository methods

.gitignore

@@ -282,5 +282,4 @@ dmypy.json
 
 # Cython debug symbols
 cython_debug/
-uv.lock
 version.py

pyproject.toml

@@ -115,7 +115,7 @@ testpaths = [
 ]
 
 [tool.ruff]
-extend-exclude = ["docs", ".tox"]
+extend-exclude = ["docs", ".tox", "*.md"]
 target-version = "py39"
 
 [tool.ruff.lint]

sqlalchemy_bind_manager/_repository/abstract.py

@@ -88,10 +88,9 @@ async def delete_many(self, instances: Iterable[MODEL]) -> None:
 
     async def find(
         self,
-        search_params: Union[None, Mapping[str, Any]] = None,
+        search_params: Union[Mapping[str, Any], None] = None,
         order_by: Union[
-            None,
-            Iterable[Union[str, Tuple[str, Literal["asc", "desc"]]]],
+            Iterable[Union[str, Tuple[str, Literal["asc", "desc"]]]], None
         ] = None,
     ) -> List[MODEL]:
         """Find models using filters.
@@ -117,10 +116,9 @@ async def paginated_find(
         self,
         items_per_page: int,
         page: int = 1,
-        search_params: Union[None, Mapping[str, Any]] = None,
+        search_params: Union[Mapping[str, Any], None] = None,
         order_by: Union[
-            None,
-            Iterable[Union[str, Tuple[str, Literal["asc", "desc"]]]],
+            Iterable[Union[str, Tuple[str, Literal["asc", "desc"]]]], None
         ] = None,
     ) -> PaginatedResult[MODEL]:
         """Find models using filters and limit/offset pagination. Returned results
@@ -156,7 +154,7 @@ async def cursor_paginated_find(
         items_per_page: int,
         cursor_reference: Union[CursorReference, None] = None,
         is_before_cursor: bool = False,
-        search_params: Union[None, Mapping[str, Any]] = None,
+        search_params: Union[Mapping[str, Any], None] = None,
     ) -> CursorPaginatedResult[MODEL]:
         """Find models using filters and cursor based pagination. Returned results
         do include pagination metadata.
@@ -236,10 +234,9 @@ def delete_many(self, instances: Iterable[MODEL]) -> None:
 
     def find(
         self,
-        search_params: Union[None, Mapping[str, Any]] = None,
+        search_params: Union[Mapping[str, Any], None] = None,
         order_by: Union[
-            None,
-            Iterable[Union[str, Tuple[str, Literal["asc", "desc"]]]],
+            Iterable[Union[str, Tuple[str, Literal["asc", "desc"]]]], None
         ] = None,
     ) -> List[MODEL]:
         """Find models using filters.
@@ -265,10 +262,9 @@ def paginated_find(
         self,
         items_per_page: int,
         page: int = 1,
-        search_params: Union[None, Mapping[str, Any]] = None,
+        search_params: Union[Mapping[str, Any], None] = None,
         order_by: Union[
-            None,
-            Iterable[Union[str, Tuple[str, Literal["asc", "desc"]]]],
+            Iterable[Union[str, Tuple[str, Literal["asc", "desc"]]]], None
         ] = None,
     ) -> PaginatedResult[MODEL]:
         """Find models using filters and limit/offset pagination. Returned results
@@ -304,7 +300,7 @@ def cursor_paginated_find(
         items_per_page: int,
         cursor_reference: Union[CursorReference, None] = None,
         is_before_cursor: bool = False,
-        search_params: Union[None, Mapping[str, Any]] = None,
+        search_params: Union[Mapping[str, Any], None] = None,
     ) -> CursorPaginatedResult[MODEL]:
         """Find models using filters and cursor based pagination. Returned results
         do include pagination metadata.

sqlalchemy_bind_manager/_repository/async_.py

@@ -145,10 +145,9 @@ async def delete_many(self, instances: Iterable[MODEL]) -> None:
 
     async def find(
         self,
-        search_params: Union[None, Mapping[str, Any]] = None,
+        search_params: Union[Mapping[str, Any], None] = None,
         order_by: Union[
-            None,
-            Iterable[Union[str, Tuple[str, Literal["asc", "desc"]]]],
+            Iterable[Union[str, Tuple[str, Literal["asc", "desc"]]]], None
         ] = None,
     ) -> List[MODEL]:
         """Find models using filters.
@@ -170,18 +169,17 @@ async def find(
         """
         stmt = self._find_query(search_params, order_by)
 
-        async with self._get_session() as session:
+        async with self._get_session(commit=False) as session:
             result = await session.execute(stmt)
             return [x for x in result.scalars()]
 
     async def paginated_find(
         self,
         items_per_page: int,
         page: int = 1,
-        search_params: Union[None, Mapping[str, Any]] = None,
+        search_params: Union[Mapping[str, Any], None] = None,
         order_by: Union[
-            None,
-            Iterable[Union[str, Tuple[str, Literal["asc", "desc"]]]],
+            Iterable[Union[str, Tuple[str, Literal["asc", "desc"]]]], None
         ] = None,
     ) -> PaginatedResult[MODEL]:
         """Find models using filters and limit/offset pagination. Returned results
@@ -213,7 +211,7 @@ async def paginated_find(
         find_stmt = self._find_query(search_params, order_by)
         paginated_stmt = self._paginate_query_by_page(find_stmt, page, items_per_page)
 
-        async with self._get_session() as session:
+        async with self._get_session(commit=False) as session:
             total_items_count = (
                 await session.execute(self._count_query(find_stmt))
             ).scalar() or 0
@@ -233,7 +231,7 @@ async def cursor_paginated_find(
         items_per_page: int,
         cursor_reference: Union[CursorReference, None] = None,
         is_before_cursor: bool = False,
-        search_params: Union[None, Mapping[str, Any]] = None,
+        search_params: Union[Mapping[str, Any], None] = None,
     ) -> CursorPaginatedResult[MODEL]:
         """Find models using filters and cursor based pagination. Returned results
         do include pagination metadata.
@@ -268,7 +266,7 @@ async def cursor_paginated_find(
             items_per_page=items_per_page,
         )
 
-        async with self._get_session() as session:
+        async with self._get_session(commit=False) as session:
             total_items_count = (
                 await session.execute(self._count_query(find_stmt))
             ).scalar() or 0

sqlalchemy_bind_manager/_repository/base_repository.py

@@ -150,10 +150,9 @@ def _filter_order_by(
 
     def _find_query(
         self,
-        search_params: Union[None, Mapping[str, Any]] = None,
+        search_params: Union[Mapping[str, Any], None] = None,
         order_by: Union[
-            None,
-            Iterable[Union[str, Tuple[str, Literal["asc", "desc"]]]],
+            Iterable[Union[str, Tuple[str, Literal["asc", "desc"]]]], None
         ] = None,
     ) -> Select:
         """Build a query with column filters and orders.

sqlalchemy_bind_manager/_repository/common.py

@@ -27,6 +27,13 @@
 MODEL = TypeVar("MODEL")
 PRIMARY_KEY = Union[str, int, tuple, dict, UUID]
 
+# Constrained rather than bound: a bound TypeVar would happily bind to the
+# union of the constraints, which lets mismatched operands (e.g. `str >= UUID`)
+# pass unnoticed. Constraining makes the checker solve for one concrete type,
+# so a cursor value and the reference it is compared against are provably the
+# same type.
+CURSOR_VALUE = TypeVar("CURSOR_VALUE", StrictStr, StrictInt, UUID)
+
 
 def get_model_pk_name(model_class: Type) -> str:
     """Retrieves the primary key column name from a SQLAlchemy model class.
@@ -81,9 +88,18 @@ class PaginatedResult(BaseModel, Generic[MODEL]):
     page_info: PageInfo
 
 
-class CursorReference(BaseModel):
+class CursorReference(BaseModel, Generic[CURSOR_VALUE]):
+    """A cursor position: an ordering column and a threshold value.
+
+    Generic in the value type so a cursor value read from a model and the
+    reference it is compared against are known to be the same type. The
+    parameter can be omitted (`CursorReference(column="id", value=123)`);
+    it is inferred, and existing annotations that name the class bare keep
+    working.
+    """
+
     column: str
-    value: Union[StrictStr, StrictInt, UUID]
+    value: CURSOR_VALUE
 
 
 class CursorPageInfo(BaseModel):

sqlalchemy_bind_manager/_repository/result_presenters.py

@@ -22,6 +22,7 @@
 from typing import List, Union
 
 from .common import (
+    CURSOR_VALUE,
     MODEL,
     CursorPageInfo,
     CursorPaginatedResult,
@@ -117,11 +118,13 @@ def _build_before_cursor_result(
         result_items: List[MODEL],
         total_items_count: int,
         items_per_page: int,
-        cursor_reference: CursorReference,
+        cursor_reference: CursorReference[CURSOR_VALUE],
     ) -> CursorPaginatedResult:
         index = -1
         reference_column = cursor_reference.column
-        last_found_cursor_value = getattr(result_items[index], reference_column)
+        last_found_cursor_value: CURSOR_VALUE = getattr(
+            result_items[index], reference_column
+        )
         if not isinstance(last_found_cursor_value, type(cursor_reference.value)):
             raise TypeError(
                 "Values from CursorReference and results must be of the same type"
@@ -164,11 +167,13 @@ def _build_after_cursor_result(
         result_items: List[MODEL],
         total_items_count: int,
         items_per_page: int,
-        cursor_reference: CursorReference,
+        cursor_reference: CursorReference[CURSOR_VALUE],
     ) -> CursorPaginatedResult:
         index = 0
         reference_column = cursor_reference.column
-        first_found_cursor_value = getattr(result_items[index], reference_column)
+        first_found_cursor_value: CURSOR_VALUE = getattr(
+            result_items[index], reference_column
+        )
         if not isinstance(first_found_cursor_value, type(cursor_reference.value)):
             raise TypeError(
                 "Values from CursorReference and results must be of the same type"

sqlalchemy_bind_manager/_repository/sync.py

@@ -142,10 +142,9 @@ def delete_many(self, instances: Iterable[MODEL]) -> None:
 
     def find(
         self,
-        search_params: Union[None, Mapping[str, Any]] = None,
+        search_params: Union[Mapping[str, Any], None] = None,
         order_by: Union[
-            None,
-            Iterable[Union[str, Tuple[str, Literal["asc", "desc"]]]],
+            Iterable[Union[str, Tuple[str, Literal["asc", "desc"]]]], None
         ] = None,
     ) -> List[MODEL]:
         """Find models using filters.
@@ -167,18 +166,17 @@ def find(
         """
         stmt = self._find_query(search_params, order_by)
 
-        with self._get_session() as session:
+        with self._get_session(commit=False) as session:
             result = session.execute(stmt)
             return [x for x in result.scalars()]
 
     def paginated_find(
         self,
         items_per_page: int,
         page: int = 1,
-        search_params: Union[None, Mapping[str, Any]] = None,
+        search_params: Union[Mapping[str, Any], None] = None,
         order_by: Union[
-            None,
-            Iterable[Union[str, Tuple[str, Literal["asc", "desc"]]]],
+            Iterable[Union[str, Tuple[str, Literal["asc", "desc"]]]], None
         ] = None,
     ) -> PaginatedResult[MODEL]:
         """Find models using filters and limit/offset pagination. Returned results
@@ -210,7 +208,7 @@ def paginated_find(
         find_stmt = self._find_query(search_params, order_by)
         paginated_stmt = self._paginate_query_by_page(find_stmt, page, items_per_page)
 
-        with self._get_session() as session:
+        with self._get_session(commit=False) as session:
             total_items_count = (
                 session.execute(self._count_query(find_stmt)).scalar() or 0
             )
@@ -228,7 +226,7 @@ def cursor_paginated_find(
         items_per_page: int,
         cursor_reference: Union[CursorReference, None] = None,
         is_before_cursor: bool = False,
-        search_params: Union[None, Mapping[str, Any]] = None,
+        search_params: Union[Mapping[str, Any], None] = None,
     ) -> CursorPaginatedResult[MODEL]:
         """Find models using filters and cursor based pagination. Returned results
         do include pagination metadata.
@@ -264,7 +262,7 @@ def cursor_paginated_find(
             items_per_page=items_per_page,
         )
 
-        with self._get_session() as session:
+        with self._get_session(commit=False) as session:
             total_items_count = (
                 session.execute(self._count_query(find_stmt)).scalar() or 0
             )

tests/repository/test_operation_isolation.py

@@ -122,3 +122,36 @@ async def test_commit_triggers_once_per_operation_using_internal_uow(
         await sync_async_wrapper(repo1.save(model1))
         await sync_async_wrapper(repo2.save(model2))
     assert mocked_uow_commit.call_count == 2
+
+
+async def test_read_operations_do_not_commit(
+    repository_class, model_class, sa_bind, sync_async_wrapper
+):
+    repo = repository_class(bind=sa_bind, model_class=model_class)
+    model = model_class(
+        name="Someone",
+    )
+    await sync_async_wrapper(repo.save(model))
+
+    session_handler_class = (
+        AsyncSessionHandler
+        if isinstance(sa_bind, SQLAlchemyAsyncBind)
+        else SessionHandler
+    )
+    session_handler_mock = (
+        AsyncMock if isinstance(sa_bind, SQLAlchemyAsyncBind) else MagicMock
+    )
+
+    with patch.object(
+        session_handler_class,
+        "commit",
+        new_callable=session_handler_mock,
+        return_value=None,
+    ) as mocked_commit:
+        await sync_async_wrapper(repo.get(model.model_id))
+        await sync_async_wrapper(repo.get_many([model.model_id]))
+        await sync_async_wrapper(repo.find())
+        await sync_async_wrapper(repo.paginated_find(10))
+        await sync_async_wrapper(repo.cursor_paginated_find(10))
+
+    assert mocked_commit.call_count == 0

tox.ini

@@ -12,7 +12,10 @@ env_list =
     format
 
 [testenv]
-runner = uv-venv-runner
+; uv-venv-lock-runner uses `uv sync`, so every environment resolves to the
+; versions pinned in uv.lock instead of re-resolving to whatever is latest.
+; Requires uv.lock to be committed.
+runner = uv-venv-lock-runner
 dependency_groups = dev
 commands =
     pytest