From: Mike Bayer Date: Tue, 21 Mar 2017 21:11:18 +0000 (-0400) Subject: Raise on flag_modified() for non-present attribute X-Git-Tag: rel_1_2_0b1~139 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=711d29f8e4dc096f5083c075a1f64eb38e2d2e4a;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git Raise on flag_modified() for non-present attribute The :func:`.attributes.flag_modified` function now raises :class:`.InvalidRequestError` if the named attribute key is not present within the object, as this is assumed to be present in the flush process. To mark an object "dirty" for a flush without referring to any specific attribute, the :func:`.attributes.flag_dirty` function may be used. Change-Id: I6c64e4d253c239e38632f38c27bb16e68fe8dfbe Fixes: #3753 --- diff --git a/doc/build/changelog/changelog_12.rst b/doc/build/changelog/changelog_12.rst index cd36f41384..e5b63eb47c 100644 --- a/doc/build/changelog/changelog_12.rst +++ b/doc/build/changelog/changelog_12.rst @@ -13,6 +13,21 @@ .. changelog:: :version: 1.2.0b1 + .. change:: 3753 + :tags: bug, orm + :tickets: 3753 + + The :func:`.attributes.flag_modified` function now raises + :class:`.InvalidRequestError` if the named attribute key is not + present within the object, as this is assumed to be present + in the flush process. To mark an object "dirty" for a flush + without referring to any specific attribute, the + :func:`.attributes.flag_dirty` function may be used. + + .. seealso:: + + :ref:`change_3753` + .. change:: 3911_3912 :tags: bug, ext :tickets: 3911, 3912 diff --git a/doc/build/changelog/migration_12.rst b/doc/build/changelog/migration_12.rst index 7097c6aec9..c7f4ad869c 100644 --- a/doc/build/changelog/migration_12.rst +++ b/doc/build/changelog/migration_12.rst @@ -404,6 +404,38 @@ new collection. :ticket:`3913` +.. _change_3753: + +Use flag_dirty() to mark an object as "dirty" without any attribute changing +---------------------------------------------------------------------------- + +An exception is now raised if the :func:`.attributes.flag_modified` function +is used to mark an attribute as modified that isn't actually loaded:: + + a1 = A(data='adf') + s.add(a1) + + s.flush() + + # expire, similarly as though we said s.commit() + s.expire(a1, 'data') + + # will raise InvalidRequestError + attributes.flag_modified(a1, 'data') + +This because the flush process will most likely fail in any case if the +attribute remains un-present by the time flush occurs. To mark an object +as "modified" without referring to any attribute specifically, so that it +is considered within the flush process for the purpose of custom event handlers +such as :meth:`.SessionEvents.before_flush`, use the new +:func:`.attributes.flag_dirty` function:: + + from sqlalchemy.orm import attributes + + attributes.flag_dirty(a1) + +:ticket:`3753` + Key Behavioral Changes - Core ============================= diff --git a/doc/build/orm/session_api.rst b/doc/build/orm/session_api.rst index 3754ac80b8..795e830253 100644 --- a/doc/build/orm/session_api.rst +++ b/doc/build/orm/session_api.rst @@ -53,6 +53,8 @@ those described in :doc:`/orm/events`. .. autofunction:: flag_modified +.. autofunction:: flag_dirty + .. function:: instance_state Return the :class:`.InstanceState` for a given diff --git a/lib/sqlalchemy/orm/attributes.py b/lib/sqlalchemy/orm/attributes.py index 2b8b38d58b..a387e7d76f 100644 --- a/lib/sqlalchemy/orm/attributes.py +++ b/lib/sqlalchemy/orm/attributes.py @@ -1617,8 +1617,42 @@ def flag_modified(instance, key): This sets the 'modified' flag on the instance and establishes an unconditional change event for the given attribute. + The attribute must have a value present, else an + :class:`.InvalidRequestError` is raised. + + To mark an object "dirty" without referring to any specific attribute + so that it is considered within a flush, use the + :func:`.attributes.flag_dirty` call. + + .. seealso:: + + :func:`.attributes.flag_dirty` """ state, dict_ = instance_state(instance), instance_dict(instance) impl = state.manager[key].impl - state._modified_event(dict_, impl, NO_VALUE, force=True) + state._modified_event(dict_, impl, NO_VALUE, is_userland=True) + + +def flag_dirty(instance): + """Mark an instance as 'dirty' without any specific attribute mentioned. + + This is a special operation that will allow the object to travel through + the flush process for interception by events such as + :meth:`.SessionEvents.before_flush`. Note that no SQL will be emitted in + the flush process for an object that has no changes, even if marked dirty + via this method. However, a :meth:`.SessionEvents.before_flush` handler + will be able to see the object in the :attr:`.Session.dirty` collection and + may establish changes on it, which will then be included in the SQL + emitted. + + .. versionadded:: 1.2 + + .. seealso:: + + :func:`.attributes.flag_modified` + + """ + + state, dict_ = instance_state(instance), instance_dict(instance) + state._modified_event(dict_, None, NO_VALUE, is_userland=True) diff --git a/lib/sqlalchemy/orm/state.py b/lib/sqlalchemy/orm/state.py index 0fba24004d..1781a41e9a 100644 --- a/lib/sqlalchemy/orm/state.py +++ b/lib/sqlalchemy/orm/state.py @@ -15,6 +15,7 @@ defines a large part of the ORM's interactivity. import weakref from .. import util from .. import inspection +from .. import exc as sa_exc from . import exc as orm_exc, interfaces from .path_registry import PathRegistry from .base import PASSIVE_NO_RESULT, SQL_OK, NEVER_SET, ATTR_WAS_SET, \ @@ -634,19 +635,23 @@ class InstanceState(interfaces.InspectionAttr): return None def _modified_event( - self, dict_, attr, previous, collection=False, force=False): - if not attr.send_modified_events: - return - if attr.key not in self.committed_state or force: - if collection: - if previous is NEVER_SET: - if attr.key in dict_: - previous = dict_[attr.key] - - if previous not in (None, NO_VALUE, NEVER_SET): - previous = attr.copy(previous) - - self.committed_state[attr.key] = previous + self, dict_, attr, previous, collection=False, is_userland=False): + if attr: + if not attr.send_modified_events: + return + if is_userland and attr.key not in dict_: + raise sa_exc.InvalidRequestError( + "Can't flag attribute '%s' modified; it's not present in " + "the object state" % attr.key) + if attr.key not in self.committed_state or is_userland: + if collection: + if previous is NEVER_SET: + if attr.key in dict_: + previous = dict_[attr.key] + + if previous not in (None, NO_VALUE, NEVER_SET): + previous = attr.copy(previous) + self.committed_state[attr.key] = previous # assert self._strong_obj is None or self.modified @@ -664,7 +669,7 @@ class InstanceState(interfaces.InspectionAttr): if self.session_id: self._strong_obj = inst - if inst is None: + if inst is None and attr: raise orm_exc.ObjectDereferencedError( "Can't emit change event for attribute '%s' - " "parent object of type %s has been garbage " diff --git a/test/orm/test_attributes.py b/test/orm/test_attributes.py index d3a63c3868..6dcba1f3f5 100644 --- a/test/orm/test_attributes.py +++ b/test/orm/test_attributes.py @@ -4,7 +4,7 @@ from sqlalchemy.orm.collections import collection from sqlalchemy.orm.interfaces import AttributeExtension from sqlalchemy import exc as sa_exc from sqlalchemy.testing import eq_, ne_, assert_raises, \ - assert_raises_message + assert_raises_message, is_true, is_false from sqlalchemy.testing import fixtures from sqlalchemy.testing.util import gc_collect, all_partial_orderings from sqlalchemy.util import jython @@ -1963,6 +1963,43 @@ class HistoryTest(fixtures.TestBase): attributes.flag_modified(f, 'someattr') eq_(self._someattr_history(f), ([{'a': 'b'}], (), ())) + def test_flag_modified_but_no_value_raises(self): + Foo = self._fixture(uselist=False, useobject=False, + active_history=False) + f = Foo() + f.someattr = 'foo' + self._commit_someattr(f) + eq_(self._someattr_history(f), ((), ['foo'], ())) + + attributes.instance_state(f)._expire_attributes( + attributes.instance_dict(f), + ['someattr']) + + assert_raises_message( + sa_exc.InvalidRequestError, + "Can't flag attribute 'someattr' modified; it's " + "not present in the object state", + attributes.flag_modified, f, 'someattr' + ) + + def test_mark_dirty_no_attr(self): + Foo = self._fixture(uselist=False, useobject=False, + active_history=False) + f = Foo() + f.someattr = 'foo' + attributes.instance_state(f)._commit_all(f.__dict__) + eq_(self._someattr_history(f), ((), ['foo'], ())) + + attributes.instance_state(f)._expire_attributes( + attributes.instance_dict(f), + ['someattr']) + + is_false(attributes.instance_state(f).modified) + + attributes.flag_dirty(f) + + is_true(attributes.instance_state(f).modified) + def test_use_object_init(self): Foo, Bar = self._two_obj_fixture(uselist=False) f = Foo()