Merge pull request #21 from febus982/unit_of_work_rewrite

Unit of work rewrite

Author: febus982

README.md

@@ -1,6 +1,6 @@
 # SQLAlchemy bind manager
 [![Stable Version](https://img.shields.io/pypi/v/sqlalchemy-bind-manager?color=blue)](https://pypi.org/project/sqlalchemy-bind-manager/)
-[![stability-alpha](https://img.shields.io/badge/stability-alpha-f4d03f.svg)](https://github.com/mkenney/software-guides/blob/master/STABILITY-BADGES.md#alpha)
+[![stability-beta](https://img.shields.io/badge/stability-beta-33bbff.svg)](https://github.com/mkenney/software-guides/blob/master/STABILITY-BADGES.md#beta)
 
 [![Python 3.8](https://github.com/febus982/sqlalchemy-bind-manager/actions/workflows/python-3.8.yml/badge.svg?event=push)](https://github.com/febus982/sqlalchemy-bind-manager/actions/workflows/python-3.8.yml)
 [![Python 3.9](https://github.com/febus982/sqlalchemy-bind-manager/actions/workflows/python-3.9.yml/badge.svg?event=push)](https://github.com/febus982/sqlalchemy-bind-manager/actions/workflows/python-3.9.yml)
@@ -30,7 +30,7 @@ pip install sqlalchemy-bind-manager
 
 [//]: # (https://github.com/mkenney/software-guides/blob/master/STABILITY-BADGES.md)
 * [![stability-beta](https://img.shields.io/badge/stability-beta-33bbff.svg)](https://github.com/mkenney/software-guides/blob/master/STABILITY-BADGES.md#beta) **SQLAlchemy manager:** Implementation is mostly finalised, needs testing in production.
-* [![stability-wip](https://img.shields.io/badge/stability-wip-lightgrey.svg)](https://github.com/mkenney/software-guides/blob/master/STABILITY-BADGES.md#work-in-progress) **Repository / Unit of work:** Major work is stil necessary to finalise the interface, to hide the session management implementation details from the application.
+* [![stability-beta](https://img.shields.io/badge/stability-beta-33bbff.svg)](https://github.com/mkenney/software-guides/blob/master/STABILITY-BADGES.md#beta) **Repository / Unit of work:** Implementation is mostly finalised, needs testing in production.
 
 
 ## SQLAlchemy manager 
@@ -75,12 +75,15 @@ Example:
 ```python
 bind = sa_manager.get_bind()
 
+
 class DeclarativeModel(bind.model_declarative_base):
     pass
-    
+
+
 class ImperativeModel:
     id: int
 
+
 imperative_table = Table(
     "imperative",
     bind.registry_mapper.metadata,
@@ -94,7 +97,7 @@ bind.registry_mapper.map_imperatively(ImperativeModel, imperative_table)
 sa_manager.get_mapper().map_imperatively(ImperativeModel, imperative_table)
 
 # Persist an object
-o = ImperativeModel() # also o = DeclarativeModel()
+o = ImperativeModel()  # also o = DeclarativeModel()
 o.name = "John"
 with sa_manager.get_bind().session_class()() as session:
     session.add(o)
@@ -209,47 +212,52 @@ A `Repository` represents a generic interface to persist data object to a storag
 using SQLAlchemy. It makes sense that the lifecycle of a `Session` follows the one of the Repository
 (If we create a Repository, we're going to do a DB operation, otherwise we don't need a `Session`)
 
-This ensures we the `Session` we use is isolated, and the same for all the operations we do with the
-repository.
+This ensures the `Session` we use is isolated, and the same for all the operations we do with the
+same repository.
 
-The consequence of this choice is we can't use SQLAlchemy lazy loading, so we need to make sure
-relationship are loaded eagerly. You can do this by:
+The session is automatically closed and reopen with each Repository operation, this make sure these
+operation are independent from each other.
 
-* Setup your model/table relationships to always use always eager loading
-* Implement ad-hoc methods to deal with relationships as necessary
+These choices cause some consequences:
+* The operations that modify the database will reload the models from the DB, causing an additional
+  SELECT query to be issued.
+* We can't use SQLAlchemy lazy loading, so we'll need to make sure relationship are always loaded eagerly,
+  using either:
+  * Setup your model/table relationships to always use always eager loading
+  * Implement ad-hoc methods to deal with relationships as necessary
 
 Also `AsyncSession` has [the same limitation on lazy loading](https://docs.sqlalchemy.org/en/20/orm/extensions/asyncio.html#asyncio-orm-avoid-lazyloads)
 so it makes sense that the two repository implementations behave consistently.
 
-### Use the Unit Of Work to share a session among multiple repositories [alpha]
+### Use the Unit Of Work to share a session among multiple repositories
 
-It is possible we need to run several operations in a single database transaction. While a single
+It is possible we need to run several operations in a single database get_session. While a single
 repository provide by itself an isolated session for single operations, we have to use a different
 approach for multiple operations.
 
-We can use the `SQLAlchemyUnitOfWork` or the `SQLAlchemyUnitOfWork` class to provide a shared session to
+We can use the `UnitOfWork` or the `AsyncUnitOfWork` class to provide a shared session to
 be used for repository operations, **assumed the same bind is used for all the repositories**.
 (Two phase transactions are not currently supported).
 
-All repositories operation methods accept the `session` parameter for this purpose. This makes the
-operations to bypass the internal repository-managed session.
-
 ```python
+class MyRepo(SQLAlchemyRepository):
+    _model = MyModel
+class MyOtherRepo(SQLAlchemyRepository):
+    _model = MyOtherModel
+
 bind = sa_manager.get_bind()
-repo1 = MyRepo(bind)
-repo2 = MyOtherRepo(bind)
-uow = SQLAlchemyUnitOfWork(bind)
+uow = UnitOfWork(bind, (MyRepo, MyOtherRepo))
 
-with uow.get_session() as _session:
-    repo1.save(some_model, session=_session)
-    repo2.save(some_model, session=_session)
+with uow.transaction():
+    uow.MyRepo.save(some_model)
+    uow.MyOtherRepo.save(some_other_model)
 
 # Optionally disable the commit/rollback handling
-with uow.get_session(commit=False) as _session:
-    model1 = repo1.get(1, session=_session)
-    model2 = repo1.get(1, session=_session)
+with uow.transaction(read_only=True):
+    model1 = uow.MyRepo.get(1)
+    model2 = uow.MyOtherRepo.get(2)
 ```
 
 Both the UnitOfWork classes create an internal `scoped_session` or `async_scoped_session`, behaving
 in the same way at the repositories do. This provides the freedom to tune the session lifecycle based
-on our application requirements (e.g. one session per http request, per domain, etc.)
+on our application requirements (e.g. one unit of work per http request, per domain, etc.)

mypy.ini

@@ -9,7 +9,7 @@ readme = "README.md"
 packages = [{include = "sqlalchemy_bind_manager"}]
 keywords = ["sqlalchemy", "config", "manager"]
 classifiers = [
-    "Development Status :: 3 - Alpha",
+    "Development Status :: 4 - Beta",
     "Framework :: AsyncIO",
     "Framework :: Pydantic",
     "Intended Audience :: Developers",
@@ -63,3 +63,6 @@ addopts = "-n auto --cov-report=term-missing"
 testpaths = [
     "tests",
 ]
+
+[tool.mypy]
+files = "sqlalchemy_bind_manager"

pyproject.toml

@@ -9,6 +9,6 @@
     SortDirection,
 )
 from ._unit_of_work import (
-    SQLAlchemyUnitOfWork,
-    SQLAlchemyAsyncUnitOfWork,
+    UnitOfWork,
+    AsyncUnitOfWork,
 )

sqlalchemy_bind_manager/__init__.py

@@ -91,6 +91,7 @@ def __init_bind(
 
         session_options: dict = config.session_options or {}
         session_options.setdefault("expire_on_commit", False)
+        session_options.setdefault("autobegin", False)
 
         if isinstance(config, SQLAlchemyAsyncConfig):
             self.__binds[name] = self.__build_async_bind(

sqlalchemy_bind_manager/_bind_manager.py

@@ -3,102 +3,100 @@
 from typing import Union, Generic, Tuple, Iterable, List, AsyncIterator, Any, Mapping
 
 from sqlalchemy import select
-from sqlalchemy.ext.asyncio import async_scoped_session
+from sqlalchemy.ext.asyncio import AsyncSession
 
 from .._bind_manager import SQLAlchemyAsyncBind
-from .._unit_of_work import SQLAlchemyAsyncUnitOfWork
-from ..exceptions import ModelNotFound
+from .._transaction_handler import AsyncSessionHandler
+from ..exceptions import ModelNotFound, InvalidConfig
 from .common import MODEL, PRIMARY_KEY, SortDirection, BaseRepository
 
 
 class SQLAlchemyAsyncRepository(Generic[MODEL], BaseRepository[MODEL], ABC):
-    _UOW: SQLAlchemyAsyncUnitOfWork
+    _session_handler: AsyncSessionHandler
+    _external_session: Union[AsyncSession, None]
 
-    def __init__(self, bind: SQLAlchemyAsyncBind) -> None:
+    def __init__(
+        self,
+        bind: Union[SQLAlchemyAsyncBind, None] = None,
+        session: Union[AsyncSession, None] = None,
+    ) -> None:
         """
         :param bind: A configured instance of SQLAlchemyAsyncBind
         :type bind: SQLAlchemyAsyncBind
+        :param session: An externally managed session
+        :type session: AsyncSession
         """
         super().__init__()
-        self._UOW = SQLAlchemyAsyncUnitOfWork(bind)
+        if not (bool(bind) ^ bool(session)):
+            raise InvalidConfig("Either `bind` or `session` have to be used, not both")
+        self._external_session = session
+        if bind:
+            self._session_handler = AsyncSessionHandler(bind)
 
     @asynccontextmanager
-    async def _get_session(
-        self, session: Union[async_scoped_session, None] = None, commit: bool = True
-    ) -> AsyncIterator[async_scoped_session]:
-        if not session:
-            async with self._UOW.get_session(commit) as _session:
+    async def _get_session(self, commit: bool = True) -> AsyncIterator[AsyncSession]:
+        if not self._external_session:
+            async with self._session_handler.get_session(not commit) as _session:
                 yield _session
         else:
-            yield session
+            yield self._external_session
 
-    async def save(
-        self, instance: MODEL, session: Union[async_scoped_session, None] = None
-    ) -> MODEL:
+    async def save(self, instance: MODEL) -> MODEL:
         """Persist a model.
 
         :param instance: A mapped object instance to be persisted
-        :param session: Optional session with externally-managed lifecycle
         :return: The model instance after being persisted (e.g. with primary key populated)
         """
-        async with self._get_session(session) as session:  # type: ignore
+        async with self._get_session() as session:
             session.add(instance)
         return instance
 
     async def save_many(
         self,
         instances: Iterable[MODEL],
-        session: Union[async_scoped_session, None] = None,
     ) -> Iterable[MODEL]:
-        """Persist many models in a single database transaction.
+        """Persist many models in a single database get_session.
 
         :param instances: A list of mapped objects to be persisted
         :type instances: Iterable
-        :param session: Optional session with externally-managed lifecycle
         :return: The model instances after being persisted (e.g. with primary keys populated)
         """
-        async with self._get_session(session) as session:  # type: ignore
+        async with self._get_session() as session:
             session.add_all(instances)
         return instances
 
-    async def get(
-        self, identifier: PRIMARY_KEY, session: Union[async_scoped_session, None] = None
-    ) -> MODEL:
+    async def get(self, identifier: PRIMARY_KEY) -> MODEL:
         """Get a model by primary key.
 
         :param identifier: The primary key
         :return: A model instance
-        :param session: Optional session with externally-managed lifecycle
         :raises ModelNotFound: No model has been found using the primary key
         """
         # TODO: implement get_many()
-        async with self._get_session(session, commit=False) as session:
-            model = await session.get(self._model, identifier)  # type: ignore
+        async with self._get_session(commit=False) as session:
+            model = await session.get(self._model, identifier)
         if model is None:
             raise ModelNotFound("No rows found for provided primary key.")
         return model
 
     async def delete(
         self,
         entity: Union[MODEL, PRIMARY_KEY],
-        session: Union[async_scoped_session, None] = None,
     ) -> None:
         """Deletes a model.
 
         :param entity: The model instance or the primary key
         :type entity: Union[MODEL, PRIMARY_KEY]
-        :param session: Optional session with externally-managed lifecycle
         """
         # TODO: delete without loading the model
         obj = entity if self._is_mapped_object(entity) else await self.get(entity)  # type: ignore
-        async with self._get_session(session) as session:  # type: ignore
+        async with self._get_session() as session:
             await session.delete(obj)
 
     async def find(
         self,
         search_params: Union[None, Mapping[str, Any]] = None,
         order_by: Union[None, Iterable[Union[str, Tuple[str, SortDirection]]]] = None,
-        session: Union[async_scoped_session, None] = None,
     ) -> List[MODEL]:
         """Find models using filters
 
@@ -107,17 +105,16 @@ async def find(
 
         :param order_by:
         :param search_params: A dictionary containing equality filters
-        :param session: Optional session with externally-managed lifecycle
         :return: A collection of models
         :rtype: List
         """
-        stmt = select(self._model)  # type: ignore
+        stmt = select(self._model)
         if search_params:
             stmt = self._filter_select(stmt, search_params)
 
         if order_by is not None:
             stmt = self._filter_order_by(stmt, order_by)
 
-        async with self._get_session(session) as session:  # type: ignore
+        async with self._get_session() as session:
             result = await session.execute(stmt)
             return [x for x in result.scalars()]

sqlalchemy_bind_manager/_repository/async_.py

@@ -49,7 +49,7 @@ def _validate_mapped_property(self, property_name: str) -> None:
         :raises UnmappedProperty: When the property is not mapped.
         """
         m: Mapper = class_mapper(self._model)
-        if property_name not in m.column_attrs:  # type: ignore
+        if property_name not in m.column_attrs:
             raise UnmappedProperty(
                 f"Property `{property_name}` is not mapped in the ORM for model `{self._model}`"
             )