strip out UoW and fake UoW, add some tips

Author: hjwp

src/allocation/service_layer/services.py

@@ -16,16 +16,23 @@ def is_valid_sku(sku, batches):
 
 
 def add_batch(
-    ref: str, sku: str, qty: int, eta: Optional[date],
-    uow: unit_of_work.AbstractUnitOfWork,
+    ref: str,
+    sku: str,
+    qty: int,
+    eta: Optional[date],
+    uow  #: unit_of_work.AbstractUnitOfWork
+    # this argument could be start_uow: AbstractUnitOfWorkStarter instead?
 ):
+    # and this could be with start_uow() as uow:
     with uow:
         uow.batches.add(model.Batch(ref, sku, qty, eta))
         uow.commit()
 
 
 def allocate(
-    orderid: str, sku: str, qty: int,
+    orderid: str,
+    sku: str,
+    qty: int,
     uow: unit_of_work.AbstractUnitOfWork,
 ) -> str:
     line = OrderLine(orderid, sku, qty)

src/allocation/service_layer/unit_of_work.py

@@ -1,6 +1,7 @@
 # pylint: disable=attribute-defined-outside-init
 from __future__ import annotations
 import abc
+from typing import ContextManager
 from sqlalchemy import create_engine
 from sqlalchemy.orm import sessionmaker
 from sqlalchemy.orm.session import Session
@@ -10,13 +11,9 @@
 
 
 class AbstractUnitOfWork(abc.ABC):
-    batches: repository.AbstractRepository
-
-    def __enter__(self) -> AbstractUnitOfWork:
-        return self
-
-    def __exit__(self, *args):
-        self.rollback()
+    # should this class contain __enter__ and __exit__?
+    # or should the context manager and the UoW be separate?
+    # up to you!
 
     @abc.abstractmethod
     def commit(self):
@@ -34,21 +31,14 @@ def rollback(self):
 )
 
 
-class SqlAlchemyUnitOfWork(AbstractUnitOfWork):
-    def __init__(self, session_factory=DEFAULT_SESSION_FACTORY):
-        self.session_factory = session_factory
-
-    def __enter__(self):
-        self.session = self.session_factory()  # type: Session
-        self.batches = repository.SqlAlchemyRepository(self.session)
-        return super().__enter__()
+class SqlAlchemyUnitOfWork:
+    ...
 
-    def __exit__(self, *args):
-        super().__exit__(*args)
-        self.session.close()
 
-    def commit(self):
-        self.session.commit()
-
-    def rollback(self):
-        self.session.rollback()
+# One alternative would be to define a `start_uow` function,
+# or a UnitOfWorkStarter or UnitOfWorkManager that does the
+# job of context manager, leaving the UoW as a separate class
+# that's returned by the context manager's __enter__.
+#
+# A type like this could work?
+# AbstractUnitOfWorkStarter = ContextManager[AbstractUnitOfWork]

tests/unit/test_services.py

@@ -18,31 +18,27 @@ def list(self):
 
 
 class FakeUnitOfWork(unit_of_work.AbstractUnitOfWork):
-    def __init__(self):
-        self.batches = FakeRepository([])
-        self.committed = False
-
-    def commit(self):
-        self.committed = True
-
-    def rollback(self):
-        pass
+    ...
 
 
 def test_add_batch():
     uow = FakeUnitOfWork()
-    services.add_batch("b1", "CRUNCHY-ARMCHAIR", 100, None, uow)
+    # fake_uow_starter = FakeUoWContextManager(uow) ?
+    # fake_uow_starter = contextlib.nullcontext(uow) ?
+    # services.add_batch("b1", "CRUNCHY-ARMCHAIR", 100, None, fake_uow_starter)
     assert uow.batches.get("b1") is not None
     assert uow.committed
 
 
+@pytest.mark.skip("unskip and fix when ready")
 def test_allocate_returns_allocation():
     uow = FakeUnitOfWork()
     services.add_batch("batch1", "COMPLICATED-LAMP", 100, None, uow)
     result = services.allocate("o1", "COMPLICATED-LAMP", 10, uow)
     assert result == "batch1"
 
 
+@pytest.mark.skip("unskip and fix when ready")
 def test_allocate_errors_for_invalid_sku():
     uow = FakeUnitOfWork()
     services.add_batch("b1", "AREALSKU", 100, None, uow)
@@ -51,6 +47,7 @@ def test_allocate_errors_for_invalid_sku():
         services.allocate("o1", "NONEXISTENTSKU", 10, uow)
 
 
+@pytest.mark.skip("unskip and fix when ready")
 def test_allocate_commits():
     uow = FakeUnitOfWork()
     services.add_batch("b1", "OMINOUS-MIRROR", 100, None, uow)