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

4 files changed, 74 insertions, 23 deletions

diff --git a/doc/build/changelog/changelog_11.rst b/doc/build/changelog/changelog_11.rst
index 05c4529bb..afa4389a8 100644
--- a/doc/build/changelog/changelog_11.rst
+++ b/doc/build/changelog/changelog_11.rst
@@ -22,6 +22,14 @@
     :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 026ebc317..867c6ee15 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 5f5ab1069..33feab0dc 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 429e0f308..93db643a6 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):