This is an automated email from the ASF dual-hosted git repository. michaelsmolina pushed a commit to branch 5.0 in repository https://gitbox.apache.org/repos/asf/superset.git
commit d476490ea26ac1435b8628e9410b94edb038945c Author: Beto Dealmeida <[email protected]> AuthorDate: Fri Feb 28 09:59:03 2025 -0500 fix: prevent nested transactions (#32401) --- superset/utils/decorators.py | 7 ++++ tests/unit_tests/utils/test_decorators.py | 53 +++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+) diff --git a/superset/utils/decorators.py b/superset/utils/decorators.py index 844a8f063c..d5b73b0447 100644 --- a/superset/utils/decorators.py +++ b/superset/utils/decorators.py @@ -255,6 +255,11 @@ def transaction( # pylint: disable=redefined-outer-name def wrapped(*args: Any, **kwargs: Any) -> Any: from superset import db # pylint: disable=import-outside-toplevel + if getattr(g, "in_transaction", False): + # If already in a transaction, call the function directly + return func(*args, **kwargs) + + g.in_transaction = True try: result = func(*args, **kwargs) db.session.commit() # pylint: disable=consider-using-transaction @@ -266,6 +271,8 @@ def transaction( # pylint: disable=redefined-outer-name return on_error(ex) raise + finally: + g.in_transaction = False return wrapped diff --git a/tests/unit_tests/utils/test_decorators.py b/tests/unit_tests/utils/test_decorators.py index 0a622f4bce..df767e9387 100644 --- a/tests/unit_tests/utils/test_decorators.py +++ b/tests/unit_tests/utils/test_decorators.py @@ -24,6 +24,7 @@ from typing import Any, Optional from unittest.mock import call, Mock, patch import pytest +from pytest_mock import MockerFixture from superset import app from superset.utils import decorators @@ -294,3 +295,55 @@ def test_suppress_logging() -> None: decorated = decorators.suppress_logging("test-logger", logging.CRITICAL + 1)(func) decorated() assert len(handler.log_records) == 0 + + +def test_transacation_commit(mocker: MockerFixture) -> None: + """ + Test the `transaction` decorator when the function completes successfully. + """ + db = mocker.patch("superset.db") + + @decorators.transaction() + def func() -> int: + return 42 + + result = func() + assert result == 42 + db.session.commit.assert_called_once() + + +def test_transacation_rollback(mocker: MockerFixture) -> None: + """ + Test the `transaction` decorator when the function raises an exception. + """ + db = mocker.patch("superset.db") + + @decorators.transaction() + def func() -> None: + raise ValueError("error") + + with pytest.raises(ValueError, match="error"): + func() + db.session.commit.assert_not_called() + db.session.rollback.assert_called_once() + + +def test_transacation_nested(mocker: MockerFixture) -> None: + """ + Test the `transaction` decorator when the function is nested. + """ + db = mocker.patch("superset.db") + + @decorators.transaction() + def func() -> int: + return 42 + + @decorators.transaction() + def nested() -> int: + func() # should not commit + raise ValueError("error") + + with pytest.raises(ValueError, match="error"): + nested() + db.session.commit.assert_not_called() + db.session.rollback.assert_called_once()
