From 2b1c8eabb10c932f6e83d08147c75bb05f96a161 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Fri, 25 Oct 2013 13:13:24 -0400 Subject: [PATCH] - :func:`.attributes.get_history()` when used with a scalar column-mapped attribute will now honor the "passive" flag passed to it; as this defaults to ``PASSIVE_OFF``, the function will by default query the database if the value is not present. This is a behavioral change vs. 0.8. [ticket:2787] - Added new method :meth:`.AttributeState.load_history`, works like :attr:`.AttributeState.history` but also fires loader callables. --- doc/build/changelog/changelog_09.rst | 26 +++++++++++++++ doc/build/changelog/migration_09.rst | 47 ++++++++++++++++++++++++++++ lib/sqlalchemy/orm/attributes.py | 17 +++++++--- lib/sqlalchemy/orm/state.py | 32 ++++++++++++++++++- test/orm/test_attributes.py | 33 ++++++++++++++++--- test/orm/test_inspect.py | 44 +++++++++++++++++++++++++- 6 files changed, 188 insertions(+), 11 deletions(-) diff --git a/doc/build/changelog/changelog_09.rst b/doc/build/changelog/changelog_09.rst index 38ca115e49..8a7a8bfc0b 100644 --- a/doc/build/changelog/changelog_09.rst +++ b/doc/build/changelog/changelog_09.rst @@ -12,6 +12,32 @@ .. changelog:: :version: 0.9.0 + .. change:: + :tags: bug, orm + :tickets: 2787 + + :func:`.attributes.get_history()` when used with a scalar column-mapped + attribute will now honor the "passive" flag + passed to it; as this defaults to ``PASSIVE_OFF``, the function will + by default query the database if the value is not present. + This is a behavioral change vs. 0.8. + + .. seealso:: + + :ref:`change_2787` + + .. change:: + :tags: feature, orm + :tickets: 2787 + + Added new method :meth:`.AttributeState.load_history`, works like + :attr:`.AttributeState.history` but also fires loader callables. + + .. seealso:: + + :ref:`change_2787` + + .. change:: :tags: feature, sql :tickets: 2850 diff --git a/doc/build/changelog/migration_09.rst b/doc/build/changelog/migration_09.rst index b7fa41ac76..3aaf9670ba 100644 --- a/doc/build/changelog/migration_09.rst +++ b/doc/build/changelog/migration_09.rst @@ -486,6 +486,53 @@ now consistent:: :ticket:`2804` +.. _change_2787: + +attributes.get_history() will query from the DB by default if value not present +------------------------------------------------------------------------------- + +A bugfix regarding :func:`.attributes.get_history` allows a column-based attribute +to query out to the database for an unloaded value, assuming the ``passive`` +flag is left at its default of ``PASSIVE_OFF``. Previously, this flag would +not be honored. Additionally, a new method :meth:`.AttributeState.load_history` +is added to complement the :attr:`.AttributeState.history` attribute, which +will emit loader callables for an unloaded attribute. + +This is a small change demonstrated as follows:: + + from sqlalchemy import Column, Integer, String, create_engine, inspect + from sqlalchemy.orm import Session, attributes + from sqlalchemy.ext.declarative import declarative_base + + Base = declarative_base() + + class A(Base): + __tablename__ = 'a' + id = Column(Integer, primary_key=True) + data = Column(String) + + e = create_engine("sqlite://", echo=True) + Base.metadata.create_all(e) + + sess = Session(e) + + a1 = A(data='a1') + sess.add(a1) + sess.commit() # a1 is now expired + + # history doesn't emit loader callables + assert inspect(a1).attrs.data.history == (None, None, None) + + # in 0.8, this would fail to load the unloaded state. + assert attributes.get_history(a1, 'data') == ((), ['a1',], ()) + + # load_history() is now equiavlent to get_history() with + # passive=PASSIVE_OFF ^ INIT_OK + assert inspect(a1).attrs.data.load_history() == ((), ['a1',], ()) + +:ticket:`2787` + + New Features ============ diff --git a/lib/sqlalchemy/orm/attributes.py b/lib/sqlalchemy/orm/attributes.py index da9d62d137..e789734591 100644 --- a/lib/sqlalchemy/orm/attributes.py +++ b/lib/sqlalchemy/orm/attributes.py @@ -552,7 +552,6 @@ class AttributeImpl(object): def get(self, state, dict_, passive=PASSIVE_OFF): """Retrieve a value from the given object. - If a callable is assembled on this object's attribute, and passive is False, the callable will be executed and the resulting value will be set as the new value for this attribute. @@ -652,8 +651,16 @@ class ScalarAttributeImpl(AttributeImpl): del dict_[self.key] def get_history(self, state, dict_, passive=PASSIVE_OFF): - return History.from_scalar_attribute( - self, state, dict_.get(self.key, NO_VALUE)) + if self.key in dict_: + return History.from_scalar_attribute(self, state, dict_[self.key]) + else: + if passive & INIT_OK: + passive ^= INIT_OK + current = self.get(state, dict_, passive=passive) + if current is PASSIVE_NO_RESULT: + return HISTORY_BLANK + else: + return History.from_scalar_attribute(self, state, current) def set(self, state, dict_, value, initiator, passive=PASSIVE_OFF, check_old=None, pop=False): @@ -1226,7 +1233,7 @@ class History(History): original = state.committed_state.get(attribute.key, _NO_HISTORY) if original is _NO_HISTORY: - if current is NO_VALUE: + if current is NEVER_SET: return cls((), (), ()) else: return cls((), [current], ()) @@ -1243,7 +1250,7 @@ class History(History): deleted = () else: deleted = [original] - if current is NO_VALUE: + if current is NEVER_SET: return cls((), (), deleted) else: return cls([current], (), deleted) diff --git a/lib/sqlalchemy/orm/state.py b/lib/sqlalchemy/orm/state.py index 8d73c9426c..957e29700e 100644 --- a/lib/sqlalchemy/orm/state.py +++ b/lib/sqlalchemy/orm/state.py @@ -16,7 +16,7 @@ from .. import util 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, \ - NO_VALUE, PASSIVE_NO_INITIALIZE + NO_VALUE, PASSIVE_NO_INITIALIZE, INIT_OK, PASSIVE_OFF from . import base class InstanceState(interfaces._InspectionAttr): @@ -555,10 +555,40 @@ class AttributeState(object): """Return the current pre-flush change history for this attribute, via the :class:`.History` interface. + This method will **not** emit loader callables if the value of the + attribute is unloaded. + + .. seealso:: + + :meth:`.AttributeState.load_history` - retrieve history + using loader callables if the value is not locally present. + + :func:`.attributes.get_history` - underlying function + """ return self.state.get_history(self.key, PASSIVE_NO_INITIALIZE) + def load_history(self): + """Return the current pre-flush change history for + this attribute, via the :class:`.History` interface. + + This method **will** emit loader callables if the value of the + attribute is unloaded. + + .. seealso:: + + :attr:`.AttributeState.history` + + :func:`.attributes.get_history` - underlying function + + .. versionadded:: 0.9.0 + + """ + return self.state.get_history(self.key, + PASSIVE_OFF ^ INIT_OK) + + class PendingCollection(object): """A writable placeholder for an unloaded collection. diff --git a/test/orm/test_attributes.py b/test/orm/test_attributes.py index de44e4be39..c282bc44cd 100644 --- a/test/orm/test_attributes.py +++ b/test/orm/test_attributes.py @@ -1339,7 +1339,7 @@ class PendingBackrefTest(fixtures.ORMTest): ] ) - def test_lazy_history(self): + def test_lazy_history_collection(self): Post, Blog, lazy_posts = self._fixture() p1, p2, p3 = Post("post 1"), Post("post 2"), Post("post 3") @@ -1511,6 +1511,12 @@ class HistoryTest(fixtures.TestBase): return Foo, Bar def _someattr_history(self, f, **kw): + passive = kw.pop('passive', None) + if passive is True: + kw['passive'] = attributes.PASSIVE_NO_INITIALIZE + elif passive is False: + kw['passive'] = attributes.PASSIVE_OFF + return attributes.get_state_history( attributes.instance_state(f), 'someattr', **kw) @@ -1685,19 +1691,19 @@ class HistoryTest(fixtures.TestBase): Foo = self._fixture(uselist=True, useobject=True, active_history=True) f = Foo() - eq_(self._someattr_history(f, passive=True), ((), (), ())) + eq_(self._someattr_history(f, passive=True), (None, None, None)) def test_scalar_obj_never_set(self): Foo = self._fixture(uselist=False, useobject=True, active_history=True) f = Foo() - eq_(self._someattr_history(f, passive=True), ((), (), ())) + eq_(self._someattr_history(f, passive=True), (None, None, None)) def test_scalar_never_set(self): Foo = self._fixture(uselist=False, useobject=False, active_history=True) f = Foo() - eq_(self._someattr_history(f, passive=True), ((), (), ())) + eq_(self._someattr_history(f, passive=True), (None, None, None)) def test_scalar_active_set(self): Foo = self._fixture(uselist=False, useobject=False, @@ -1793,6 +1799,24 @@ class HistoryTest(fixtures.TestBase): eq_(self._someattr_history(f), (['two'], (), ())) + def test_scalar_passive_flag(self): + Foo = self._fixture(uselist=False, useobject=False, + active_history=True) + f = Foo() + f.someattr = 'one' + eq_(self._someattr_history(f), (['one'], (), ())) + + self._commit_someattr(f) + + state = attributes.instance_state(f) + state._expire_attribute_pre_commit(state.dict, 'someattr') + + def scalar_loader(state, toload): + state.dict['someattr'] = 'one' + state.manager.deferred_scalar_loader = scalar_loader + + eq_(self._someattr_history(f), ((), ['one'], ())) + def test_scalar_inplace_mutation_set(self): Foo = self._fixture(uselist=False, useobject=False, @@ -1848,6 +1872,7 @@ class HistoryTest(fixtures.TestBase): f.someattr = ['a'] eq_(self._someattr_history(f), ([['a']], (), ())) + def test_use_object_init(self): Foo, Bar = self._two_obj_fixture(uselist=False) f = Foo() diff --git a/test/orm/test_inspect.py b/test/orm/test_inspect.py index 61c1fd93eb..ed8ab32e9d 100644 --- a/test/orm/test_inspect.py +++ b/test/orm/test_inspect.py @@ -365,7 +365,7 @@ class TestORMInspection(_fixtures.FixtureTest): [] ) - def test_instance_state_attr_hist(self): + def test_instance_state_collection_attr_hist(self): User = self.classes.User u1 = User(name='ed') insp = inspect(u1) @@ -379,6 +379,48 @@ class TestORMInspection(_fixtures.FixtureTest): hist.unchanged, [] ) + def test_instance_state_scalar_attr_hist(self): + User = self.classes.User + u1 = User(name='ed') + sess = Session() + sess.add(u1) + sess.commit() + assert 'name' not in u1.__dict__ + insp = inspect(u1) + hist = insp.attrs.name.history + eq_( + hist.unchanged, None + ) + assert 'name' not in u1.__dict__ + + def test_instance_state_collection_attr_load_hist(self): + User = self.classes.User + u1 = User(name='ed') + insp = inspect(u1) + hist = insp.attrs.addresses.load_history() + eq_( + hist.unchanged, () + ) + u1.addresses + hist = insp.attrs.addresses.load_history() + eq_( + hist.unchanged, [] + ) + + def test_instance_state_scalar_attr_hist_load(self): + User = self.classes.User + u1 = User(name='ed') + sess = Session() + sess.add(u1) + sess.commit() + assert 'name' not in u1.__dict__ + insp = inspect(u1) + hist = insp.attrs.name.load_history() + eq_( + hist.unchanged, ['ed'] + ) + assert 'name' in u1.__dict__ + def test_instance_state_ident_transient(self): User = self.classes.User u1 = User(name='ed') -- 2.47.3