Fix transaction issues (#36)

Internal refactoring of when the side-effects are run so if handles the transaction.on_commit gracefully.

.coveragerc

@@ -0,0 +1,6 @@
+[run]
+branch = True
+
+[report]
+ignore_errors = True
+include = side_effects/*

.github/workflows/tox.yml

@@ -60,14 +60,10 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        python: ["3.8", "3.9", "3.10", "3.11", "3.12"]
+        python: ["3.9", "3.10", "3.11", "3.12"]
         # build LTS version, next version, HEAD
         django: ["32", "42", "50", "main"]
         exclude:
-          - python: "3.8"
-            django: "50"
-          - python: "3.8"
-            django: "main"
           - python: "3.9"
             django: "50"
           - python: "3.9"

.gitignore

@@ -1,6 +1,7 @@
 .coverage
 .tox
 .venv
+.vscode
 *.egg-info
 *.lock
 *.pyc

.pre-commit-config.yaml

@@ -1,20 +1,15 @@
 repos:
-  # python code formatting - will amend files
-  - repo: https://github.com/ambv/black
-    rev: 23.10.1
-    hooks:
-      - id: black
-
   - repo: https://github.com/charliermarsh/ruff-pre-commit
     # Ruff version.
-    rev: "v0.1.5"
+    rev: "v0.2.1"
     hooks:
       - id: ruff
         args: [--fix, --exit-non-zero-on-fix]
+      - id: ruff-format
 
   # python static type checking
   - repo: https://github.com/pre-commit/mirrors-mypy
-    rev: v1.7.0
+    rev: v1.8.0
     hooks:
       - id: mypy
         args:

.ruff.toml

@@ -1,5 +1,5 @@
 line-length = 88
-ignore = [
+lint.ignore = [
     "D100",  # Missing docstring in public module
     "D101",  # Missing docstring in public class
     "D102",  # Missing docstring in public method
@@ -22,7 +22,7 @@ ignore = [
     "D417",
     "D417",  # Missing argument description in the docstring
 ]
-select = [
+lint.select = [
     "A",  # flake8 builtins
     "C9", # mcabe
     "D",  # pydocstyle
@@ -34,13 +34,13 @@ select = [
     "W",  # pycodestype (warnings)
 ]
 
-[isort]
+[lint.isort]
 combine-as-imports = true
 
-[mccabe]
+[lint.mccabe]
 max-complexity = 8
 
-[per-file-ignores]
+[lint.per-file-ignores]
 "*tests/*" = [
     "D205",  # 1 blank line required between summary line and description
     "D400",  # First line should end with a period

pyproject.toml

@@ -17,7 +17,6 @@ classifiers = [
     "Framework :: Django :: 5.0",
     "Operating System :: OS Independent",
     "Programming Language :: Python :: 3 :: Only",
-    "Programming Language :: Python :: 3.8",
     "Programming Language :: Python :: 3.9",
     "Programming Language :: Python :: 3.10",
     "Programming Language :: Python :: 3.11",
@@ -26,11 +25,10 @@ classifiers = [
 packages = [{ include = "side_effects" }]
 
 [tool.poetry.dependencies]
-python = "^3.8"
+python = "^3.9"
 django = "^3.2 || ^4.0 || ^5.0 "
 
 [tool.poetry.dev-dependencies]
-black = "*"
 coverage = "*"
 mypy = "*"
 pre-commit = "*"

side_effects/decorators.py

@@ -6,7 +6,6 @@
 from django.http import HttpResponse
 
 from . import registry
-from .settings import TEST_MODE
 
 
 def http_response_check(response: HttpResponse) -> bool:
@@ -63,14 +62,7 @@ def inner_func(*args: Any, **kwargs: Any) -> Any:
             # if the exit test fails we go no further
             if not run_on_exit(return_value):
                 return return_value
-            # when in TEST_MODE do not defer the side-effects, as they
-            # are never fired anyway.
-            registry_func = (
-                registry.run_side_effects
-                if TEST_MODE
-                else registry.run_side_effects_on_commit
-            )
-            registry_func(label, *args, return_value=return_value, **kwargs)
+            registry.run_side_effects(label, *args, return_value=return_value, **kwargs)
             return return_value
 
         return inner_func

side_effects/registry.py

@@ -31,11 +31,6 @@ def docstring(func: Callable) -> list[str] | None:
         return None
 
 
-class SideEffectsTestFailure(Exception):
-    def __init__(self, label: str):
-        super().__init__(f"Side-effects for '{label}' aborted; TEST_MODE_FAIL=True")
-
-
 class SignatureMismatch(Exception):
     def __init__(self, func: Callable):
         super().__init__(
@@ -116,52 +111,12 @@ def disable(self) -> None:
     def enable(self) -> None:
         self._suppress = False
 
-    def _run_side_effects(
-        self, label: str, *args: Any, return_value: Any | None = None, **kwargs: Any
-    ) -> None:
-        if settings.TEST_MODE_FAIL:
-            raise SideEffectsTestFailure(label)
-        for func in self[label]:
-            _run_func(func, *args, return_value=return_value, **kwargs)
-
     def run_side_effects(
         self, label: str, *args: Any, return_value: Any | None = None, **kwargs: Any
     ) -> None:
-        """
-        Run registered side-effects functions, or suppress as appropriate.
-
-        If TEST_MODE is on, or the _suppress attr is True, then the side-effects
-        are not run, but the `suppressed_side_effect` signal is sent - this is
-        primarily used by the disable_side_effects context manager to register
-        which side-effects events were suppressed (for testing purposes).
-
-        NB even if the side-effects themselves are not run, this method will try
-        to bind all of the receiver functions - this is to ensure that incompatible
-        functions fail hard and early.
-
-        """
-        # TODO: this is all becoming over-complex - need to simplify this
-        self.try_bind_all(label, *args, return_value=return_value, **kwargs)
-        if self.is_suppressed:
-            self.suppressed_side_effect.send(Registry, label=label)
-        else:
-            self._run_side_effects(label, *args, return_value=return_value, **kwargs)
-
-    def try_bind_all(
-        self, label: str, *args: Any, return_value: Any | None = None, **kwargs: Any
-    ) -> None:
-        """
-        Test all receivers for signature compatibility.
-
-        Raise SignatureMismatch if any function does not match.
-
-        """
+        """Run all registered side-effects functions."""
         for func in self[label]:
-            if not (
-                try_bind(func, *args, return_value=return_value, **kwargs)
-                or try_bind(func, *args, **kwargs)
-            ):
-                raise SignatureMismatch(func)
+            _run_func(func, *args, return_value=return_value, **kwargs)
 
 
 class disable_side_effects:
@@ -177,8 +132,7 @@ class disable_side_effects:
     """
 
     def __init__(self) -> None:
-        self.events = []  # type: List[str]
-        pass
+        self.events: list[str] = []
 
     def __enter__(self) -> list[str]:
         _registry.suppressed_side_effect.connect(self.on_event, dispatch_uid="suppress")
@@ -204,22 +158,20 @@ def run_side_effects(
     label: str, *args: Any, return_value: Any | None = None, **kwargs: Any
 ) -> None:
     """Run all of the side-effect functions registered for a label."""
-    _registry.run_side_effects(label, *args, return_value=return_value, **kwargs)
-
-
-def run_side_effects_on_commit(
-    label: str, *args: Any, return_value: Any | None = None, **kwargs: Any
-) -> None:
-    """Run all of the side-effects after current transaction on_commit."""
-    transaction.on_commit(
-        partial(
-            _registry.run_side_effects,
-            label,
-            *args,
-            return_value=return_value,
-            **kwargs,
+    # if the registry is suppressed we are inside  disable_side_effects,
+    # so we send the signal and return early.
+    if _registry.is_suppressed:
+        _registry.suppressed_side_effect.send(Registry, label=label)
+    else:
+        transaction.on_commit(
+            partial(
+                _registry.run_side_effects,
+                label,
+                *args,
+                return_value=return_value,
+                **kwargs,
+            )
         )
-    )
 
 
 def _run_func(
@@ -239,7 +191,7 @@ def _run_func(
         raise
     except Exception:  # noqa: B902
         logger.exception("Error running side_effect function '%s'", fname(func))
-        if settings.ABORT_ON_ERROR or settings.TEST_MODE_FAIL:
+        if settings.ABORT_ON_ERROR:
             raise
 
 

side_effects/settings.py

@@ -17,9 +17,3 @@ def get_setting(setting_name: str, default_value: Any) -> Any:
 # In test mode no side-effects are run
 # Default = False
 TEST_MODE: bool = get_setting("SIDE_EFFECTS_TEST_MODE", False)
-
-# In FAIL test mode any call to run_side_effects will raise an Exception
-# This is used to uncover any tests that are running side-effects when
-# they shouldn't be.
-# Default = False
-TEST_MODE_FAIL: bool = get_setting("SIDE_EFFECTS_TEST_MODE_FAIL", False)

side_effects/signals.py

@@ -0,0 +1,7 @@
+from django.dispatch import Signal
+
+# if using the disable_side_effects context manager or decorator,
+# then this signal is used to communicate details of events that
+# would have fired, but have been suppressed.
+# RemovedInDjango40Warning: providing_args=["label"]
+suppressed_side_effect = Signal()

tests/test_checks.py

@@ -1,17 +1,19 @@
+from typing import Any
+
 from django.test import TestCase
 
 from side_effects import checks, registry
 
 
 class SystemCheckTests(TestCase):
-    def test_multiple_functions(self):
-        def foo():
+    def test_multiple_functions(self) -> None:
+        def foo() -> None:
             pass
 
-        def bar():
+        def bar() -> None:
             pass
 
-        def baz(arg1):
+        def baz(arg1: Any) -> None:
             pass
 
         registry._registry.clear()