Check for duplicate calls to register_attribute_impl

Fixed bug whereby the event listeners used for backrefs could
be inadvertently applied multiple times, when using a deep class
inheritance hierarchy in conjunction with mutiple mapper configuration
steps.

Change-Id: I712beaf4674e2323bf5b282922658020a6d00b53
Fixes: #3710

diff --git a/doc/build/changelog/changelog_10.rst b/doc/build/changelog/changelog_10.rst
index a44b4d62bb2016d3d9cdf383c252eb46399fc826..c51040dd5569e56a211c774365f4208a8aabcba8 100644 (file)
--- a/doc/build/changelog/changelog_10.rst
+++ b/doc/build/changelog/changelog_10.rst
@@ -18,6 +18,15 @@
 .. changelog::
     :version: 1.0.13
 
+    .. change::
+        :tags: bug, orm
+        :tickets: 3710
+
+        Fixed bug whereby the event listeners used for backrefs could
+        be inadvertently applied multiple times, when using a deep class
+        inheritance hierarchy in conjunction with mutiple mapper configuration
+        steps.
+
     .. change::
         :tags: bug, orm
         :tickets: 3706

diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py
index 3c03a681dfb69cc866baa6d577487b6c33aa7864..37cecb07927d25bba177694b374200e766e61071 100644 (file)
--- a/lib/sqlalchemy/orm/strategies.py
+++ b/lib/sqlalchemy/orm/strategies.py
@@ -71,8 +71,20 @@ def _register_attribute(
             )
         )
 
+    # a single MapperProperty is shared down a class inheritance
+    # hierarchy, so we set up attribute instrumentation and backref event
+    # for each mapper down the hierarchy.
+
+    # typically, "mapper" is the same as prop.parent, due to the way
+    # the configure_mappers() process runs, however this is not strongly
+    # enforced, and in the case of a second configure_mappers() run the
+    # mapper here might not be prop.parent; also, a subclass mapper may
+    # be called here before a superclass mapper.  That is, can't depend
+    # on mappers not already being set up so we have to check each one.
+
     for m in mapper.self_and_descendants:
-        if prop is m._props.get(prop.key):
+        if prop is m._props.get(prop.key) and \
+                not m.class_manager._attr_has_impl(prop.key):
 
             desc = attributes.register_attribute_impl(
                 m.class_,
@@ -83,8 +95,8 @@ def _register_attribute(
                 useobject=useobject,
                 extension=attribute_ext,
                 trackparent=useobject and (
-                    prop.single_parent
-                    or prop.direction is interfaces.ONETOMANY),
+                    prop.single_parent or
+                    prop.direction is interfaces.ONETOMANY),
                 typecallable=typecallable,
                 callable_=callable_,
                 active_history=active_history,

diff --git a/test/orm/test_mapper.py b/test/orm/test_mapper.py
index 69a03968199732d4cc317d22bcdad0835ae4d9d5..e357a7e256470bf30cd62463ee8deac41dfc0012 100644 (file)
--- a/test/orm/test_mapper.py
+++ b/test/orm/test_mapper.py
@@ -373,6 +373,47 @@ class MapperTest(_fixtures.FixtureTest, AssertsCompiledSQL):
         })
         assert getattr(Foo().__class__, 'name').impl is not None
 
+    def test_class_hier_only_instrument_once_multiple_configure(self):
+        users, addresses = (self.tables.users, self.tables.addresses)
+
+        class A(object):
+            pass
+
+        class ASub(A):
+            pass
+
+        class ASubSub(ASub):
+            pass
+
+        class B(object):
+            pass
+
+        from sqlalchemy.testing import mock
+        from sqlalchemy.orm.attributes import register_attribute_impl
+
+        with mock.patch(
+            "sqlalchemy.orm.attributes.register_attribute_impl",
+            side_effect=register_attribute_impl
+        ) as some_mock:
+
+            mapper(A, users, properties={
+                'bs': relationship(B)
+            })
+            mapper(B, addresses)
+
+            configure_mappers()
+
+            mapper(ASub, inherits=A)
+            mapper(ASubSub, inherits=ASub)
+
+            configure_mappers()
+
+        b_calls = [
+            c for c in some_mock.mock_calls if c[1][1] == 'bs'
+        ]
+        eq_(len(b_calls), 3)
+
+
     def test_check_descriptor_as_method(self):
         User, users = self.classes.User, self.tables.users