From 521414ba7f28e8a40da2fb9e65c9d30b8b5c02ac Mon Sep 17 00:00:00 2001 From: Sidney Richards Date: Wed, 18 Dec 2024 10:43:53 +0100 Subject: [PATCH] [#37] Explicitly handle transactions in the runner --- .../commands/setup_configuration.py | 2 - django_setup_configuration/runner.py | 25 ++++- tests/test_runner.py | 2 + tests/test_test_utils.py | 2 + tests/test_transactions.py | 105 ++++++++++++++++++ 5 files changed, 131 insertions(+), 5 deletions(-) create mode 100644 tests/test_transactions.py diff --git a/django_setup_configuration/management/commands/setup_configuration.py b/django_setup_configuration/management/commands/setup_configuration.py index 118c23b..fb4f918 100644 --- a/django_setup_configuration/management/commands/setup_configuration.py +++ b/django_setup_configuration/management/commands/setup_configuration.py @@ -1,7 +1,6 @@ from pathlib import Path from django.core.management import BaseCommand, CommandError -from django.db import transaction from django_setup_configuration.exceptions import ValidateRequirementsFailure from django_setup_configuration.runner import SetupConfigurationRunner @@ -33,7 +32,6 @@ def add_arguments(self, parser): help="Path to YAML file containing the configurations", ) - @transaction.atomic def handle(self, **options): yaml_file = Path(options["yaml_file"]).resolve() if not yaml_file.exists(): diff --git a/django_setup_configuration/runner.py b/django_setup_configuration/runner.py index 9f6e7e1..9bb9ddf 100644 --- a/django_setup_configuration/runner.py +++ b/django_setup_configuration/runner.py @@ -6,6 +6,7 @@ from typing import Any, Generator from django.conf import settings +from django.db import transaction from django.utils.module_loading import import_string from pydantic import ValidationError @@ -157,7 +158,8 @@ def _execute_step( step_exc = None try: - step.execute(config_model) + with transaction.atomic(): + step.execute(config_model) except BaseException as exc: step_exc = exc finally: @@ -207,8 +209,25 @@ def execute_all_iter(self) -> Generator[StepExecutionResult, Any, None]: Generator[StepExecutionResult, Any, None]: The results of each step's execution. """ - for step in self.enabled_steps: - yield self._execute_step(step) + + # Not the most elegant approach to rollbacks, but it's preferable to the + # pitfalls of manual transaction management. We want all steps to run and only + # rollback at the end, hence intra-step exceptions are caught and persisted. + class Rollback(BaseException): + pass + + try: + with transaction.atomic(): + results = [] + for step in self.enabled_steps: + result = self._execute_step(step) + results.append(result) + yield result + + if any(result.run_exception for result in results): + raise Rollback # Trigger the rollback + except Rollback: + pass def execute_all(self) -> list[StepExecutionResult]: """ diff --git a/tests/test_runner.py b/tests/test_runner.py index 40eb93c..402215d 100644 --- a/tests/test_runner.py +++ b/tests/test_runner.py @@ -12,6 +12,8 @@ from django_setup_configuration.test_utils import execute_single_step from tests.conftest import TestStep, TestStepConfig +pytestmark = pytest.mark.django_db + def test_runner_raises_on_non_existent_step_module_path(test_step_yaml_path): with pytest.raises(ConfigurationException): diff --git a/tests/test_test_utils.py b/tests/test_test_utils.py index 8efb192..1f85978 100644 --- a/tests/test_test_utils.py +++ b/tests/test_test_utils.py @@ -4,6 +4,8 @@ from django_setup_configuration.test_utils import execute_single_step from tests.conftest import TestStep +pytestmark = pytest.mark.django_db + def test_exception_during_execute_step_is_immediately_raised( step_execute_mock, diff --git a/tests/test_transactions.py b/tests/test_transactions.py new file mode 100644 index 0000000..2658564 --- /dev/null +++ b/tests/test_transactions.py @@ -0,0 +1,105 @@ +from unittest import mock + +from django.contrib.auth.models import User + +import pytest + +from django_setup_configuration.configuration import BaseConfigurationStep +from django_setup_configuration.models import ConfigurationModel +from tests.conftest import TestStep + +pytestmark = pytest.mark.django_db + + +def side_effect_test_func(): + pass + + +class TransactionTestConfigurationModel(ConfigurationModel): + + username: str + + +class TransactionTestConfigurationStep( + BaseConfigurationStep[TransactionTestConfigurationModel] +): + + config_model = TransactionTestConfigurationModel + enable_setting = "transaction_test_configuration_enabled" + namespace = "transaction_test_configuration" + verbose_name = "Transaction Test Configuration" + + def execute(self, model) -> None: + User.objects.create_user( + username=model.username, + password="secret", + ) + + side_effect_test_func() + + +@pytest.fixture() +def valid_config_object(test_step_valid_config): + return { + "transaction_test_configuration_enabled": True, + "transaction_test_configuration": {"username": "alice"}, + } | test_step_valid_config + + +def test_runner_rolls_back_all_on_failing_step( + runner_factory, valid_config_object, step_execute_mock +): + runner = runner_factory( + steps=[TransactionTestConfigurationStep, TestStep], + object_source=valid_config_object, + ) + exc = Exception() + step_execute_mock.side_effect = exc + + user_configuration_step_result, test_step_result = runner.execute_all() + + # Initial run is rolled back, so no objects created + assert test_step_result.has_run + assert test_step_result.run_exception is exc + + assert user_configuration_step_result.has_run + assert user_configuration_step_result.run_exception is None + assert User.objects.count() == 0 + + # Subsequent run does not raise, so the objects are created + step_execute_mock.side_effect = None + + user_configuration_step_result, test_step_result = runner.execute_all() + + assert test_step_result.has_run + assert test_step_result.run_exception is None + + assert user_configuration_step_result.has_run + assert user_configuration_step_result.run_exception is None + assert User.objects.count() == 1 + + +def test_runner_rolls_back_on_executing_single_step( + runner_factory, valid_config_object +): + runner = runner_factory( + steps=[TransactionTestConfigurationStep, TestStep], + object_source=valid_config_object, + ) + with mock.patch("tests.test_transactions.side_effect_test_func") as m: + exc = Exception() + m.side_effect = exc + + user_configuration_step_result = runner._execute_step( + runner.configured_steps[0] + ) + + assert user_configuration_step_result.has_run + assert user_configuration_step_result.run_exception is exc + assert User.objects.count() == 0 + + user_configuration_step_result = runner._execute_step(runner.configured_steps[0]) + + assert user_configuration_step_result.has_run + assert user_configuration_step_result.run_exception is None + assert User.objects.count() == 1