Search through mapper superclass hierarchy for owner

Fixed regression caused by fix for issue :ticket:`4116` affecting versions
1.2.2 as well as 1.1.15, which had the effect of mis-calculation of the
"owning class" of an :class:`.AssociationProxy` as the ``NoneType`` class
in some declarative mixin/inheritance situations as well as if the
association proxy were accessed off of an un-mapped class.  The "figure out
the owner" logic has been replaced by an in-depth routine that searches
through the complete mapper hierarchy assigned to the class or subclass to
determine the correct (we hope) match; will not assign the owner if no
match is found.  An exception is now raised if the proxy is used
against an un-mapped instance.

Change-Id: I611b590df2babe077ce6c19bea89e84251d1a7f4
Fixes: #4185

diff --git a/doc/build/changelog/unreleased_11/4185.rst b/doc/build/changelog/unreleased_11/4185.rst
new file mode 100644 (file)
index 0000000..8be1f77
--- /dev/null
+++ b/doc/build/changelog/unreleased_11/4185.rst
@@ -0,0 +1,16 @@
+.. change::
+    :tags: bug, orm
+    :tickets: 4185
+    :versions: 1.2.3
+
+    Fixed regression caused by fix for issue :ticket:`4116` affecting versions
+    1.2.2 as well as 1.1.15, which had the effect of mis-calculation of the
+    "owning class" of an :class:`.AssociationProxy` as the ``NoneType`` class
+    in some declarative mixin/inheritance situations as well as if the
+    association proxy were accessed off of an un-mapped class.  The "figure out
+    the owner" logic has been replaced by an in-depth routine that searches
+    through the complete mapper hierarchy assigned to the class or subclass to
+    determine the correct (we hope) match; will not assign the owner if no
+    match is found.  An exception is now raised if the proxy is used
+    against an un-mapped instance.
+

diff --git a/lib/sqlalchemy/ext/associationproxy.py b/lib/sqlalchemy/ext/associationproxy.py
index 8b90f0925fd10a7562069ce57912ed4ea2dc7666..72200c5e21922b09fcb8236705d77c923e70a817 100644 (file)
--- a/lib/sqlalchemy/ext/associationproxy.py
+++ b/lib/sqlalchemy/ext/associationproxy.py
@@ -212,7 +212,12 @@ class AssociationProxy(interfaces.InspectionAttrInfo):
         return (self.local_attr, self.remote_attr)
 
     def _get_property(self):
-        return (orm.class_mapper(self.owning_class).
+        owning_class = self.owning_class
+        if owning_class is None:
+            raise exc.InvalidRequestError(
+                "This association proxy has no mapped owning class; "
+                "can't locate a mapped property")
+        return (orm.class_mapper(owning_class).
                 get_property(self.target_collection))
 
     @util.memoized_property
@@ -244,18 +249,33 @@ class AssociationProxy(interfaces.InspectionAttrInfo):
     def _target_is_object(self):
         return getattr(self.target_class, self.value_attr).impl.uses_objects
 
+    def _calc_owner(self, obj, class_):
+        if obj is not None and class_ is None:
+            target_cls = type(obj)
+        elif class_ is not None:
+            target_cls = class_
+        else:
+            return
+
+        # we might be getting invoked for a subclass
+        # that is not mapped yet, in some declarative situations.
+        # save until we are mapped
+        try:
+            insp = inspect(target_cls)
+        except exc.NoInspectionAvailable:
+            # can't find a mapper, don't set owner. if we are a not-yet-mapped
+            # subclass, we can also scan through __mro__ to find a mapped
+            # class, but instead just wait for us to be called again against a
+            # mapped class normally.
+            return
+
+        # note we can get our real .key here too
+        owner = insp.mapper.class_manager._locate_owning_manager(self)
+        self.owning_class = owner.class_
+
     def __get__(self, obj, class_):
         if self.owning_class is None:
-            try:
-                insp = inspect(class_)
-            except exc.NoInspectionAvailable:
-                pass
-            else:
-                if hasattr(insp, 'mapper'):
-                    self.owning_class = insp.mapper.class_
-
-            if self.owning_class is None:
-                self.owning_class = type(obj)
+            self._calc_owner(obj, class_)
 
         if obj is None:
             return self
@@ -278,7 +298,7 @@ class AssociationProxy(interfaces.InspectionAttrInfo):
 
     def __set__(self, obj, values):
         if self.owning_class is None:
-            self.owning_class = type(obj)
+            self._calc_owner(obj, None)
 
         if self.scalar:
             creator = self.creator and self.creator or self.target_class
@@ -295,7 +315,8 @@ class AssociationProxy(interfaces.InspectionAttrInfo):
 
     def __delete__(self, obj):
         if self.owning_class is None:
-            self.owning_class = type(obj)
+            self._calc_owner(obj, None)
+
         delattr(obj, self.key)
 
     def _initialize_scalar_accessors(self):
@@ -497,6 +518,7 @@ class AssociationProxy(interfaces.InspectionAttrInfo):
     def __repr__(self):
         return "AssociationProxy(%r, %r)" % (self.target_collection, self.value_attr)
 
+
 class _lazy_collection(object):
     def __init__(self, obj, target):
         self.ref = weakref.ref(obj)

diff --git a/lib/sqlalchemy/orm/instrumentation.py b/lib/sqlalchemy/orm/instrumentation.py
index 5a96c57cf7c56cf565eb9d2da17586fa8ff7bb47..d5422b0d002bd904e7350b9e81736a3ffa8f03c3 100644 (file)
--- a/lib/sqlalchemy/orm/instrumentation.py
+++ b/lib/sqlalchemy/orm/instrumentation.py
@@ -115,6 +115,41 @@ class ClassManager(dict):
         # raises unless self.mapper has been assigned
         raise exc.UnmappedClassError(self.class_)
 
+    def _locate_owning_manager(self, attribute):
+        """Scan through all instrumented classes in our hierarchy
+        searching for the given object as an attribute, and return
+        the bottommost owner.
+
+        E.g.::
+
+            foo = foobar()
+
+            class Parent:
+                attr = foo
+
+            class Child(Parent):
+                pass
+
+        Child.manager._locate_owning_manager(foo) would
+        give us Parent.
+
+        Needed by association proxy to correctly figure out the
+        owning class when the attribute is accessed.
+
+        """
+
+        stack = [None]
+        for supercls in self.class_.__mro__:
+            mgr = manager_of_class(supercls)
+            if not mgr:
+                continue
+            for key in set(supercls.__dict__):
+                val = supercls.__dict__[key]
+                if val is attribute:
+                    stack.append(mgr)
+                    continue
+        return stack[-1]
+
     def _all_sqla_attributes(self, exclude=None):
         """return an iterator of all classbound attributes that are
         implement :class:`.InspectionAttr`.

diff --git a/test/ext/test_associationproxy.py b/test/ext/test_associationproxy.py
index 0052bb1884114a8ae937f7e7d1d5276e582da409..111ba91a8f1b706749ac6596115934c90b7d4b5c 100644 (file)
--- a/test/ext/test_associationproxy.py
+++ b/test/ext/test_associationproxy.py
@@ -15,6 +15,7 @@ from sqlalchemy.testing.schema import Table, Column
 from sqlalchemy.testing.mock import Mock, call
 from sqlalchemy.testing.assertions import expect_warnings
 from sqlalchemy.ext.declarative import declarative_base
+from sqlalchemy.ext.declarative import declared_attr
 
 
 class DictCollection(dict):
@@ -1891,6 +1892,9 @@ class DictOfTupleUpdateTest(fixtures.TestBase):
 
 
 class AttributeAccessTest(fixtures.TestBase):
+    def teardown(self):
+        clear_mappers()
+
     def test_resolve_aliased_class(self):
         Base = declarative_base()
 
@@ -1914,6 +1918,160 @@ class AttributeAccessTest(fixtures.TestBase):
 
         is_(spec.owning_class, B)
 
+    def test_resolved_w_subclass(self):
+        # test for issue #4185, as well as several below
+
+        Base = declarative_base()
+
+        class Mixin(object):
+            @declared_attr
+            def children(cls):
+                return association_proxy('_children', 'value')
+
+        # 1. build parent, Mixin.children gets invoked, we add
+        # Parent.children
+        class Parent(Mixin, Base):
+            __tablename__ = 'parent'
+            id = Column(Integer, primary_key=True)
+
+            _children = relationship("Child")
+
+        class Child(Base):
+            __tablename__ = 'child'
+            parent_id = Column(
+                Integer, ForeignKey(Parent.id), primary_key=True)
+
+        # 2. declarative builds up SubParent, scans through all attributes
+        # over all classes.  Hits Mixin, hits "children", accesses "children"
+        # in terms of the class, e.g. SubParent.children.  SubParent isn't
+        # mapped yet.  association proxy then sets up "owning_class"
+        # as NoneType.
+        class SubParent(Parent):
+            __tablename__ = 'subparent'
+            id = Column(Integer, ForeignKey(Parent.id), primary_key=True)
+
+        configure_mappers()
+
+        # 3. which would break here.
+        p1 = Parent()
+        eq_(p1.children, [])
+
+    def test_resolved_to_correct_class_one(self):
+        Base = declarative_base()
+
+        class Parent(Base):
+            __tablename__ = 'parent'
+            id = Column(Integer, primary_key=True)
+            _children = relationship("Child")
+            children = association_proxy('_children', 'value')
+
+        class Child(Base):
+            __tablename__ = 'child'
+            parent_id = Column(
+                Integer, ForeignKey(Parent.id), primary_key=True)
+
+        class SubParent(Parent):
+            __tablename__ = 'subparent'
+            id = Column(Integer, ForeignKey(Parent.id), primary_key=True)
+
+        is_(SubParent.children.owning_class, Parent)
+
+    def test_resolved_to_correct_class_two(self):
+        Base = declarative_base()
+
+        class Parent(Base):
+            __tablename__ = 'parent'
+            id = Column(Integer, primary_key=True)
+            _children = relationship("Child")
+
+        class Child(Base):
+            __tablename__ = 'child'
+            parent_id = Column(
+                Integer, ForeignKey(Parent.id), primary_key=True)
+
+        class SubParent(Parent):
+            __tablename__ = 'subparent'
+            id = Column(Integer, ForeignKey(Parent.id), primary_key=True)
+            children = association_proxy('_children', 'value')
+
+        is_(SubParent.children.owning_class, SubParent)
+
+    def test_resolved_to_correct_class_three(self):
+        Base = declarative_base()
+
+        class Parent(Base):
+            __tablename__ = 'parent'
+            id = Column(Integer, primary_key=True)
+            _children = relationship("Child")
+
+        class Child(Base):
+            __tablename__ = 'child'
+            parent_id = Column(
+                Integer, ForeignKey(Parent.id), primary_key=True)
+
+        class SubParent(Parent):
+            __tablename__ = 'subparent'
+            id = Column(Integer, ForeignKey(Parent.id), primary_key=True)
+            children = association_proxy('_children', 'value')
+
+        class SubSubParent(SubParent):
+            __tablename__ = 'subsubparent'
+            id = Column(Integer, ForeignKey(SubParent.id), primary_key=True)
+
+        is_(SubSubParent.children.owning_class, SubParent)
+
+    def test_resolved_to_correct_class_four(self):
+        Base = declarative_base()
+
+        class Parent(Base):
+            __tablename__ = 'parent'
+            id = Column(Integer, primary_key=True)
+            _children = relationship("Child")
+            children = association_proxy(
+                '_children', 'value', creator=lambda value: Child(value=value))
+
+        class Child(Base):
+            __tablename__ = 'child'
+            parent_id = Column(
+                Integer, ForeignKey(Parent.id), primary_key=True)
+            value = Column(String)
+
+        class SubParent(Parent):
+            __tablename__ = 'subparent'
+            id = Column(Integer, ForeignKey(Parent.id), primary_key=True)
+
+        sp = SubParent()
+        sp.children = 'c'
+        is_(SubParent.children.owning_class, Parent)
+
+    def test_never_assign_nonetype(self):
+        foo = association_proxy('x', 'y')
+        foo._calc_owner(None, None)
+        is_(foo.owning_class, None)
+
+        class Bat(object):
+            foo = association_proxy('x', 'y')
+
+        Bat.foo
+        is_(Bat.foo.owning_class, None)
+
+        b1 = Bat()
+        assert_raises_message(
+            exc.InvalidRequestError,
+            "This association proxy has no mapped owning class; "
+            "can't locate a mapped property",
+            getattr, b1, "foo"
+        )
+        is_(Bat.foo.owning_class, None)
+
+        # after all that, we can map it
+        mapper(
+            Bat,
+            Table('bat', MetaData(), Column('x', Integer, primary_key=True)))
+
+        # answer is correct
+        is_(Bat.foo.owning_class, Bat)
+
 
 class InfoTest(fixtures.TestBase):
     def test_constructor(self):