From: Mike Bayer Date: Sun, 6 Nov 2016 17:46:28 +0000 (-0500) Subject: Move setup functionality into _register_attribute X-Git-Tag: rel_1_1_4~16 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c4a8afa4c6bf5e76c24e8ed0b5c11acc0c8904e3;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git Move setup functionality into _register_attribute Options like uselist and backref can be determined from within _register_attribute based on parent_property given; move this logic inside so that individual strategies have less responsibility. Also don't require that _register_attribute consider the "strategy" itself at all; it would be better if we could no longer require that Joined/Subquery/etc loaders call upon the "lazy" strategy in order to initialize attribute instrumentation and this could be done more generically. Fixes long-standing bug where the "noload" relationship loading strategy would cause backrefs and/or back_populates options to be ignored. There is concern that some application that uses "noload" might be surprised at a back-populating attribute appearing suddenly, which may have side effects. However, "noload" itself must be extremely seldom used since as a strategy, it already disables loading, population of attributes is the only behavior that is even supported, so that this issue has existed for at least through 0.7 four years ago without ever being reported indicates extremely low use of this option. Change-Id: Icffb9c83ac5782b76ce882ed1df4361a1efbfba3 Fixes: #3845 --- diff --git a/doc/build/changelog/changelog_11.rst b/doc/build/changelog/changelog_11.rst index 05c4529bb7..afa4389a88 100644 --- a/doc/build/changelog/changelog_11.rst +++ b/doc/build/changelog/changelog_11.rst @@ -21,6 +21,14 @@ .. changelog:: :version: 1.1.4 + .. change:: + :tags: bug, orm + :tickets: 3845 + + Fixed long-standing bug where the "noload" relationship loading + strategy would cause backrefs and/or back_populates options to be + ignored. + .. change:: :tags: bug, mysql :tickets: 3841 diff --git a/lib/sqlalchemy/orm/dynamic.py b/lib/sqlalchemy/orm/dynamic.py index 026ebc317e..867c6ee15f 100644 --- a/lib/sqlalchemy/orm/dynamic.py +++ b/lib/sqlalchemy/orm/dynamic.py @@ -32,15 +32,13 @@ class DynaLoader(strategies.AbstractRelationshipLoader): "many-to-one/one-to-one relationships and/or " "uselist=False." % self.parent_property) strategies._register_attribute( - self, + self.parent_property, mapper, useobject=True, - uselist=True, impl_class=DynamicAttributeImpl, target_mapper=self.parent_property.mapper, order_by=self.parent_property.order_by, query_class=self.parent_property.query_class, - backref=self.parent_property.back_populates, ) diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py index 5f5ab10690..33feab0dc3 100644 --- a/lib/sqlalchemy/orm/strategies.py +++ b/lib/sqlalchemy/orm/strategies.py @@ -28,10 +28,9 @@ import itertools def _register_attribute( - strategy, mapper, useobject, + prop, mapper, useobject, compare_function=None, typecallable=None, - uselist=False, callable_=None, proxy_property=None, active_history=False, @@ -39,12 +38,12 @@ def _register_attribute( **kw ): - prop = strategy.parent_property - attribute_ext = list(util.to_list(prop.extension, default=[])) listen_hooks = [] + uselist = useobject and prop.uselist + if useobject and prop.single_parent: listen_hooks.append(single_parent_validator) @@ -61,15 +60,16 @@ def _register_attribute( # need to assemble backref listeners # after the singleparentvalidator, mapper validator - backref = kw.pop('backref', None) - if backref: - listen_hooks.append( - lambda desc, prop: attributes.backref_listeners( - desc, - backref, - uselist + if useobject: + backref = prop.back_populates + if backref: + listen_hooks.append( + lambda desc, prop: attributes.backref_listeners( + desc, + backref, + uselist + ) ) - ) # a single MapperProperty is shared down a class inheritance # hierarchy, so we set up attribute instrumentation and backref event @@ -173,7 +173,7 @@ class ColumnLoader(LoaderStrategy): mapper.version_id_col in set(self.columns) _register_attribute( - self, mapper, useobject=False, + self.parent_property, mapper, useobject=False, compare_function=coltype.compare_values, active_history=active_history ) @@ -228,7 +228,7 @@ class DeferredColumnLoader(LoaderStrategy): self.is_class_level = True _register_attribute( - self, mapper, useobject=False, + self.parent_property, mapper, useobject=False, compare_function=self.columns[0].type.compare_values, callable_=self._load_for_state, expire_missing=False @@ -350,9 +350,8 @@ class NoLoader(AbstractRelationshipLoader): self.is_class_level = True _register_attribute( - self, mapper, + self.parent_property, mapper, useobject=True, - uselist=self.parent_property.uselist, typecallable=self.parent_property.collection_class, ) @@ -434,12 +433,10 @@ class LazyLoader(AbstractRelationshipLoader, util.MemoizedSlots): # in that case. otherwise we don't need the # "old" value during backref operations. _register_attribute( - self, + self.parent_property, mapper, useobject=True, callable_=self._load_for_state, - uselist=self.parent_property.uselist, - backref=self.parent_property.back_populates, typecallable=self.parent_property.collection_class, active_history=active_history ) diff --git a/test/orm/test_relationships.py b/test/orm/test_relationships.py index 429e0f3083..93db643a6a 100644 --- a/test/orm/test_relationships.py +++ b/test/orm/test_relationships.py @@ -11,7 +11,7 @@ from sqlalchemy.orm import mapper, relationship, relation, \ Session, composite, column_property, foreign,\ remote, synonym, joinedload, subqueryload from sqlalchemy.orm.interfaces import ONETOMANY, MANYTOONE -from sqlalchemy.testing import eq_, startswith_, AssertsCompiledSQL, is_ +from sqlalchemy.testing import eq_, startswith_, AssertsCompiledSQL, is_, in_ from sqlalchemy.testing import fixtures from test.orm import _fixtures from sqlalchemy import exc @@ -1621,6 +1621,54 @@ class ManualBackrefTest(_fixtures.FixtureTest): configure_mappers) + +class NoLoadBackPopulates(_fixtures.FixtureTest): + + """test the noload stratgegy which unlike others doesn't use + lazyloader to set up instrumentation""" + + def test_o2m(self): + users, Address, addresses, User = (self.tables.users, + self.classes.Address, + self.tables.addresses, + self.classes.User) + + mapper(User, users, properties={ + 'addresses': relationship( + Address, back_populates='user', lazy="noload") + }) + + mapper(Address, addresses, properties={ + 'user': relationship(User) + }) + + u1 = User() + a1 = Address() + u1.addresses.append(a1) + is_(a1.user, u1) + + def test_m2o(self): + users, Address, addresses, User = (self.tables.users, + self.classes.Address, + self.tables.addresses, + self.classes.User) + + mapper(User, users, properties={ + 'addresses': relationship( + Address) + }) + + mapper(Address, addresses, properties={ + 'user': relationship( + User, back_populates='addresses', lazy="noload") + }) + + u1 = User() + a1 = Address() + a1.user = u1 + in_(a1, u1.addresses) + + class JoinConditionErrorTest(fixtures.TestBase): def test_clauseelement_pj(self):