From: Mike Bayer Date: Tue, 11 Oct 2022 21:01:43 +0000 (-0400) Subject: implement autobegin=False option X-Git-Tag: rel_2_0_0b1~4^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4580239b35642045c847c6faac8dd4fe304bb845;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git implement autobegin=False option Added new parameter :paramref:`_orm.Session.autobegin`, which when set to ``False`` will prevent the :class:`_orm.Session` from beginning a transaction implicitly. The :meth:`_orm.Session.begin` method must be called explicitly first in order to proceed with operations, otherwise an error is raised whenever any operation would otherwise have begun automatically. This option can be used to create a "safe" :class:`_orm.Session` that won't implicitly start new transactions. As part of this change, also added a new status variable :class:`_orm.SessionTransaction.origin` which may be useful for event handling code to be aware of the origin of a particular :class:`_orm.SessionTransaction`. Fixes: #6928 Change-Id: I246f895c4a475bff352216e5bc74b6a25e6a4ae7 --- diff --git a/doc/build/changelog/unreleased_20/6928.rst b/doc/build/changelog/unreleased_20/6928.rst new file mode 100644 index 0000000000..b2d7e954bc --- /dev/null +++ b/doc/build/changelog/unreleased_20/6928.rst @@ -0,0 +1,18 @@ +.. change:: + :tags: feature, orm + :tickets: 6928 + + Added new parameter :paramref:`_orm.Session.autobegin`, which when set to + ``False`` will prevent the :class:`_orm.Session` from beginning a + transaction implicitly. The :meth:`_orm.Session.begin` method must be + called explicitly first in order to proceed with operations, otherwise an + error is raised whenever any operation would otherwise have begun + automatically. This option can be used to create a "safe" + :class:`_orm.Session` that won't implicitly start new transactions. + + As part of this change, also added a new status variable + :class:`_orm.SessionTransaction.origin` which may be useful for event + handling code to be aware of the origin of a particular + :class:`_orm.SessionTransaction`. + + diff --git a/doc/build/orm/session_api.rst b/doc/build/orm/session_api.rst index 24dd7c900c..e26aca8d74 100644 --- a/doc/build/orm/session_api.rst +++ b/doc/build/orm/session_api.rst @@ -13,7 +13,6 @@ Session and sessionmaker() .. autoclass:: ORMExecuteState :members: - .. autoclass:: Session :members: :inherited-members: @@ -21,6 +20,9 @@ Session and sessionmaker() .. autoclass:: SessionTransaction :members: +.. autoclass:: SessionTransactionOrigin + :members: + Session Utilities ----------------- diff --git a/doc/build/orm/session_basics.rst b/doc/build/orm/session_basics.rst index 5046ef0b54..12413336a0 100644 --- a/doc/build/orm/session_basics.rst +++ b/doc/build/orm/session_basics.rst @@ -562,12 +562,6 @@ document at :doc:`queryguide/dml` for documentation. Auto Begin ~~~~~~~~~~ -.. versionadded:: 1.4 - - This section describes a behavior that is new in SQLAlchemy 1.4 and does - not apply to previous versions. Further details on the "autobegin" - change are at :ref:`change_5074`. - The :class:`_orm.Session` object features a behavior known as **autobegin**. This indicates that the :class:`_orm.Session` will internally consider itself to be in a "transactional" state as soon as any work is performed with the @@ -595,9 +589,33 @@ method is called, the :class:`_orm.Session` is placed into the "transactional" state unconditionally. :meth:`_orm.Session.begin` may be used as a context manager as described at :ref:`session_begin_commit_rollback_block`. -.. versionchanged:: 1.4.12 - autobegin now correctly occurs if object - attributes are modified; previously this was not occurring. +.. _session_autobegin_disable: + +Disabling Autobegin to Prevent Implicit Transactions +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +The "autobegin" behavior may be disabled using the +:paramref:`_orm.Session.autobegin` parameter set to ``False``. By using this +parameter, a :class:`_orm.Session` will require that the +:meth:`_orm.Session.begin` method is called explicitly. Upon construction, as +well as after any of the :meth:`_orm.Session.rollback`, +:meth:`_orm.Session.commit`, or :meth:`_orm.Session.close` methods are called, +the :class:`_orm.Session` won't implicitly begin any new transactions and will +raise an error if an attempt to use the :class:`_orm.Session` is made without +first calling :meth:`_orm.Session.begin`:: + + with Session(engine, autobegin=False) as session: + session.begin() # <-- required, else InvalidRequestError raised on next call + + session.add(User(name="u1")) + session.commit() + + session.begin() # <-- required, else InvalidRequestError raised on next call + + u1 = session.scalar(select(User).filter_by(name="u1")) +.. versionadded:: 2.0 Added :paramref:`_orm.Session.autobegin`, allowing + "autobegin" behavior to be disabled .. _session_committing: diff --git a/doc/build/orm/session_transaction.rst b/doc/build/orm/session_transaction.rst index 7ae6936328..be45906709 100644 --- a/doc/build/orm/session_transaction.rst +++ b/doc/build/orm/session_transaction.rst @@ -251,12 +251,15 @@ Commit as you go Both :class:`_orm.Session` and :class:`_engine.Connection` feature :meth:`_engine.Connection.commit` and :meth:`_engine.Connection.rollback` methods. Using SQLAlchemy 2.0-style operation, these methods affect the -**outermost** transaction in all cases. +**outermost** transaction in all cases. For the :class:`_orm.Session`, it is +assumed that :paramref:`_orm.Session.autobegin` is left at its default +value of ``True``. -Engine:: - engine = create_engine("postgresql+psycopg2://user:pass@host/dbname", future=True) +:class:`_engine.Engine`:: + + engine = create_engine("postgresql+psycopg2://user:pass@host/dbname") with engine.connect() as conn: conn.execute( @@ -269,9 +272,9 @@ Engine:: ) conn.commit() -Session:: +:class:`_orm.Session`:: - Session = sessionmaker(engine, future=True) + Session = sessionmaker(engine) with Session() as session: session.add_all( diff --git a/lib/sqlalchemy/orm/__init__.py b/lib/sqlalchemy/orm/__init__.py index c6b61f3b47..5e21615155 100644 --- a/lib/sqlalchemy/orm/__init__.py +++ b/lib/sqlalchemy/orm/__init__.py @@ -127,6 +127,7 @@ from .session import ORMExecuteState as ORMExecuteState from .session import Session as Session from .session import sessionmaker as sessionmaker from .session import SessionTransaction as SessionTransaction +from .session import SessionTransactionOrigin as SessionTransactionOrigin from .state import AttributeState as AttributeState from .state import InstanceState as InstanceState from .strategy_options import contains_eager as contains_eager diff --git a/lib/sqlalchemy/orm/scoping.py b/lib/sqlalchemy/orm/scoping.py index e3ba86c5a2..1971caedc0 100644 --- a/lib/sqlalchemy/orm/scoping.py +++ b/lib/sqlalchemy/orm/scoping.py @@ -382,9 +382,7 @@ class scoped_session(Generic[_S]): return self._proxied.add_all(instances) - def begin( - self, nested: bool = False, _subtrans: bool = False - ) -> SessionTransaction: + def begin(self, nested: bool = False) -> SessionTransaction: r"""Begin a transaction, or nested transaction, on this :class:`.Session`, if one is not already begun. @@ -425,7 +423,7 @@ class scoped_session(Generic[_S]): """ # noqa: E501 - return self._proxied.begin(nested=nested, _subtrans=_subtrans) + return self._proxied.begin(nested=nested) def begin_nested(self) -> SessionTransaction: r"""Begin a "nested" transaction on this Session, e.g. SAVEPOINT. @@ -1772,6 +1770,11 @@ class scoped_session(Generic[_S]): .. versionadded:: 1.4.24 + .. seealso:: + + :ref:`orm_queryguide_select_orm_entities` - contrasts the behavior + of :meth:`_orm.Session.execute` to :meth:`_orm.Session.scalars` + """ # noqa: E501 diff --git a/lib/sqlalchemy/orm/session.py b/lib/sqlalchemy/orm/session.py index c759f65413..22e47585d7 100644 --- a/lib/sqlalchemy/orm/session.py +++ b/lib/sqlalchemy/orm/session.py @@ -10,6 +10,7 @@ from __future__ import annotations import contextlib +from enum import Enum import itertools import sys import typing @@ -734,6 +735,30 @@ class ORMExecuteState(util.MemoizedSlots): ] +class SessionTransactionOrigin(Enum): + """indicates the origin of a :class:`.SessionTransaction`. + + This enumeration is present on the + :attr:`.SessionTransaction.origin` attribute of any + :class:`.SessionTransaction` object. + + .. versionadded:: 2.0 + + """ + + AUTOBEGIN = 0 + """transaction were started by autobegin""" + + BEGIN = 1 + """transaction were started by calling :meth:`_orm.Session.begin`""" + + BEGIN_NESTED = 2 + """tranaction were started by :meth:`_orm.Session.begin_nested`""" + + SUBTRANSACTION = 3 + """transaction is an internal "subtransaction" """ + + class SessionTransaction(_StateChange, TransactionalContext): """A :class:`.Session`-level transaction. @@ -790,29 +815,60 @@ class SessionTransaction(_StateChange, TransactionalContext): InstanceState[Any], Tuple[Any, Any] ] + origin: SessionTransactionOrigin + """Origin of this :class:`_orm.SessionTransaction`. + + Refers to a :class:`.SessionTransactionOrigin` instance which is an + enumeration indicating the source event that led to constructing + this :class:`_orm.SessionTransaction`. + + .. versionadded:: 2.0 + + """ + + nested: bool = False + """Indicates if this is a nested, or SAVEPOINT, transaction. + + When :attr:`.SessionTransaction.nested` is True, it is expected + that :attr:`.SessionTransaction.parent` will be present as well, + linking to the enclosing :class:`.SessionTransaction`. + + .. seealso:: + + :attr:`.SessionTransaction.origin` + + """ + def __init__( self, session: Session, + origin: SessionTransactionOrigin, parent: Optional[SessionTransaction] = None, - nested: bool = False, - autobegin: bool = False, ): TransactionalContext._trans_ctx_check(session) self.session = session self._connections = {} self._parent = parent - self.nested = nested + self.nested = nested = origin is SessionTransactionOrigin.BEGIN_NESTED + self.origin = origin + if nested: + if not parent: + raise sa_exc.InvalidRequestError( + "Can't start a SAVEPOINT transaction when no existing " + "transaction is in progress" + ) + self._previous_nested_transaction = session._nested_transaction + elif origin is SessionTransactionOrigin.SUBTRANSACTION: + assert parent is not None + else: + assert parent is None + self._state = SessionTransactionState.ACTIVE - if not parent and nested: - raise sa_exc.InvalidRequestError( - "Can't start a SAVEPOINT transaction when no existing " - "transaction is in progress" - ) - self._take_snapshot(autobegin=autobegin) + self._take_snapshot() # make sure transaction is assigned before we call the # dispatch @@ -866,14 +922,6 @@ class SessionTransaction(_StateChange, TransactionalContext): """ return self._parent - nested: bool = False - """Indicates if this is a nested, or SAVEPOINT, transaction. - - When :attr:`.SessionTransaction.nested` is True, it is expected - that :attr:`.SessionTransaction.parent` will be True as well. - - """ - @property def is_active(self) -> bool: return ( @@ -901,7 +949,13 @@ class SessionTransaction(_StateChange, TransactionalContext): (SessionTransactionState.ACTIVE,), _StateChangeStates.NO_CHANGE ) def _begin(self, nested: bool = False) -> SessionTransaction: - return SessionTransaction(self.session, self, nested=nested) + return SessionTransaction( + self.session, + SessionTransactionOrigin.BEGIN_NESTED + if nested + else SessionTransactionOrigin.SUBTRANSACTION, + self, + ) def _iterate_self_and_parents( self, upto: Optional[SessionTransaction] = None @@ -923,7 +977,7 @@ class SessionTransaction(_StateChange, TransactionalContext): return result - def _take_snapshot(self, autobegin: bool = False) -> None: + def _take_snapshot(self) -> None: if not self._is_transaction_boundary: parent = self._parent assert parent is not None @@ -933,7 +987,11 @@ class SessionTransaction(_StateChange, TransactionalContext): self._key_switches = parent._key_switches return - if not autobegin and not self.session._flushing: + is_begin = self.origin in ( + SessionTransactionOrigin.BEGIN, + SessionTransactionOrigin.AUTOBEGIN, + ) + if not is_begin and not self.session._flushing: self.session.flush() self._new = weakref.WeakKeyDictionary() @@ -1307,6 +1365,7 @@ class Session(_SessionClassMethods, EventTarget): autoflush: bool = True, future: Literal[True] = True, expire_on_commit: bool = True, + autobegin: bool = True, twophase: bool = False, binds: Optional[Dict[_SessionBindKey, _SessionBind]] = None, enable_baked_queries: bool = True, @@ -1330,6 +1389,20 @@ class Session(_SessionClassMethods, EventTarget): :ref:`session_flushing` - additional background on autoflush + :param autobegin: Automatically start transactions (i.e. equivalent to + invoking :meth:`_orm.Session.begin`) when database access is + requested by an operation. Defaults to ``True``. Set to + ``False`` to prevent a :class:`_orm.Session` from implicitly + beginning transactions after construction, as well as after any of + the :meth:`_orm.Session.rollback`, :meth:`_orm.Session.commit`, + or :meth:`_orm.Session.close` methods are called. + + .. versionadded:: 2.0 + + .. seealso:: + + :ref:`session_autobegin_disable` + :param bind: An optional :class:`_engine.Engine` or :class:`_engine.Connection` to which this ``Session`` should be bound. When specified, all SQL @@ -1455,6 +1528,7 @@ class Session(_SessionClassMethods, EventTarget): self._transaction = None self._nested_transaction = None self.hash_key = _new_sessionid() + self.autobegin = autobegin self.autoflush = autoflush self.expire_on_commit = expire_on_commit self.enable_baked_queries = enable_baked_queries @@ -1542,18 +1616,26 @@ class Session(_SessionClassMethods, EventTarget): """ return {} - def _autobegin_t(self) -> SessionTransaction: + def _autobegin_t(self, begin: bool = False) -> SessionTransaction: if self._transaction is None: - trans = SessionTransaction(self, autobegin=True) + if not begin and not self.autobegin: + raise sa_exc.InvalidRequestError( + "Autobegin is disabled on this Session; please call " + "session.begin() to start a new transaction" + ) + trans = SessionTransaction( + self, + SessionTransactionOrigin.BEGIN + if begin + else SessionTransactionOrigin.AUTOBEGIN, + ) assert self._transaction is trans return trans return self._transaction - def begin( - self, nested: bool = False, _subtrans: bool = False - ) -> SessionTransaction: + def begin(self, nested: bool = False) -> SessionTransaction: """Begin a transaction, or nested transaction, on this :class:`.Session`, if one is not already begun. @@ -1590,31 +1672,21 @@ class Session(_SessionClassMethods, EventTarget): trans = self._transaction if trans is None: - trans = self._autobegin_t() + trans = self._autobegin_t(begin=True) - if not nested and not _subtrans: + if not nested: return trans - if trans is not None: - if _subtrans or nested: - trans = trans._begin(nested=nested) - assert self._transaction is trans - if nested: - self._nested_transaction = trans - else: - raise sa_exc.InvalidRequestError( - "A transaction is already begun on this Session." - ) - else: - # outermost transaction. must be a not nested and not - # a subtransaction + assert trans is not None - assert not nested and not _subtrans - trans = SessionTransaction(self) + if nested: + trans = trans._begin(nested=nested) assert self._transaction is trans - - if TYPE_CHECKING: - assert self._transaction is not None + self._nested_transaction = trans + else: + raise sa_exc.InvalidRequestError( + "A transaction is already begun on this Session." + ) return trans # needed for __enter__/__exit__ hook @@ -3957,7 +4029,7 @@ class Session(_SessionClassMethods, EventTarget): if not flush_context.has_work: return - flush_context.transaction = transaction = self.begin(_subtrans=True) + flush_context.transaction = transaction = self._autobegin_t()._begin() try: self._warn_on_events = True try: @@ -4246,7 +4318,7 @@ class Session(_SessionClassMethods, EventTarget): mapper = _class_to_mapper(mapper) self._flushing = True - transaction = self.begin(_subtrans=True) + transaction = self._autobegin_t()._begin() try: if isupdate: bulk_persistence._bulk_update( diff --git a/test/orm/test_session.py b/test/orm/test_session.py index 49f769f813..520ff48dc1 100644 --- a/test/orm/test_session.py +++ b/test/orm/test_session.py @@ -220,6 +220,73 @@ class TransScopingTest(_fixtures.FixtureTest): assert not s.in_transaction() eq_(s.connection().scalar(select(User.name)), "u1") + @testing.combinations( + "select1", "lazyload", "unitofwork", argnames="trigger" + ) + @testing.combinations("commit", "close", "rollback", None, argnames="op") + def test_no_autobegin(self, op, trigger): + User, users = self.classes.User, self.tables.users + Address, addresses = self.classes.Address, self.tables.addresses + + self.mapper_registry.map_imperatively( + User, users, properties={"addresses": relationship(Address)} + ) + self.mapper_registry.map_imperatively(Address, addresses) + + with Session(testing.db) as sess: + + sess.add(User(name="u1")) + sess.commit() + + s = Session(testing.db, autobegin=False) + + orm_trigger = trigger == "lazyload" or trigger == "unitofwork" + with expect_raises_message( + sa.exc.InvalidRequestError, + r"Autobegin is disabled on this Session; please call " + r"session.begin\(\) to start a new transaction", + ): + if op or orm_trigger: + s.begin() + + is_true(s.in_transaction()) + + if orm_trigger: + u1 = s.scalar(select(User).filter_by(name="u1")) + else: + eq_(s.scalar(select(1)), 1) + + if op: + getattr(s, op)() + elif orm_trigger: + s.rollback() + + is_false(s.in_transaction()) + + if trigger == "select1": + s.execute(select(1)) + elif trigger == "lazyload": + if op == "close": + s.add(u1) + else: + u1.addresses + elif trigger == "unitofwork": + s.add(u1) + + s.begin() + if trigger == "select1": + s.execute(select(1)) + elif trigger == "lazyload": + if op == "close": + s.add(u1) + u1.addresses + + is_true(s.in_transaction()) + + if op: + getattr(s, op)() + is_false(s.in_transaction()) + def test_autobegin_begin_method(self): s = Session(testing.db) @@ -783,7 +850,7 @@ class SessionStateTest(_fixtures.FixtureTest): assert not sess.in_transaction() sess.begin() assert sess.is_active - sess.begin(_subtrans=True) + sess._autobegin_t()._begin() sess.rollback() assert sess.is_active diff --git a/test/orm/test_transaction.py b/test/orm/test_transaction.py index ec4fb2d682..c40cbfd579 100644 --- a/test/orm/test_transaction.py +++ b/test/orm/test_transaction.py @@ -2108,7 +2108,7 @@ class TransactionFlagsTest(fixtures.TestBase): eq_(s1.in_transaction(), True) is_(s1.get_transaction(), trans) - subtrans = s1.begin(_subtrans=True) + subtrans = s1._autobegin_t()._begin() is_(s1.get_transaction(), trans) eq_(s1.in_transaction(), True)