Rework AliasedClass __getattr__ to use top-level getattr()

Reworked the attribute mechanics used by :class:`.AliasedClass` to no
longer rely upon calling ``__getattribute__`` on the MRO of the wrapped
class, and to instead resolve the attribute normally on the wrapped class
using getattr(), and then unwrap/adapt that.  This allows a greater range
of attribute styles on the mapped class including special ``__getattr__()``
schemes; but it also makes the code simpler and more resilient in general.

Fixes: #4694
Co-authored-by: Mike Bayer <mike_mp@zzzcomputing.com>
Change-Id: I28901e2472d3c21e881fe5cafa3b1d3af704fad8
(cherry picked from commit 5ac10699e1111283ae848f9d3a6dcc4e09d8c1ef)

diff --git a/doc/build/changelog/unreleased_13/4694.rst b/doc/build/changelog/unreleased_13/4694.rst
new file mode 100644 (file)
index 0000000..6205d0e
--- /dev/null
+++ b/doc/build/changelog/unreleased_13/4694.rst
@@ -0,0 +1,10 @@
+.. change::
+    :tags: bug, orm
+    :tickets: 4694
+
+    Reworked the attribute mechanics used by :class:`.AliasedClass` to no
+    longer rely upon calling ``__getattribute__`` on the MRO of the wrapped
+    class, and to instead resolve the attribute normally on the wrapped class
+    using getattr(), and then unwrap/adapt that.  This allows a greater range
+    of attribute styles on the mapped class including special ``__getattr__()``
+    schemes; but it also makes the code simpler and more resilient in general.

diff --git a/lib/sqlalchemy/orm/attributes.py b/lib/sqlalchemy/orm/attributes.py
index 2606eee8b5273a0e4adf23d3490e4fd01f8325fb..9d4a0d0823d3d6fdb372a7ca1f6d45979b7ee4ea 100644 (file)
--- a/lib/sqlalchemy/orm/attributes.py
+++ b/lib/sqlalchemy/orm/attributes.py
@@ -350,10 +350,14 @@ def create_proxied_attribute(descriptor):
             )
 
         def __get__(self, instance, owner):
-            if instance is None:
+            retval = self.descriptor.__get__(instance, owner)
+            # detect if this is a plain Python @property, which just returns
+            # itself for class level access.  If so, then return us.
+            # Otherwise, return the object returned by the descriptor.
+            if retval is self.descriptor and instance is None:
                 return self
             else:
-                return self.descriptor.__get__(instance, owner)
+                return retval
 
         def __str__(self):
             return "%s.%s" % (self.class_.__name__, self.key)

diff --git a/lib/sqlalchemy/orm/util.py b/lib/sqlalchemy/orm/util.py
index 36da781a41a1400b4d6dbf2246a8ac6399b12626..30fc48cd3586f789ff88534e6e5787ea883e90f5 100644 (file)
--- a/lib/sqlalchemy/orm/util.py
+++ b/lib/sqlalchemy/orm/util.py
@@ -7,6 +7,7 @@
 
 
 import re
+import types
 
 from . import attributes  # noqa
 from .base import _class_to_mapper  # noqa
@@ -495,34 +496,28 @@ class AliasedClass(object):
         except KeyError:
             raise AttributeError()
         else:
-            for base in _aliased_insp._target.__mro__:
-                try:
-                    attr = object.__getattribute__(base, key)
-                except AttributeError:
-                    continue
-                else:
-                    break
-            else:
-                raise AttributeError(key)
-
-        if isinstance(attr, PropComparator):
-            ret = attr.adapt_to_entity(_aliased_insp)
-            setattr(self, key, ret)
-            return ret
-        elif hasattr(attr, "func_code"):
-            is_method = getattr(_aliased_insp._target, key, None)
-            if is_method and is_method.__self__ is not None:
-                return util.types.MethodType(attr.__func__, self, self)
-            else:
-                return None
-        elif hasattr(attr, "__get__"):
-            ret = attr.__get__(None, self)
-            if isinstance(ret, PropComparator):
-                return ret.adapt_to_entity(_aliased_insp)
-            else:
-                return ret
-        else:
-            return attr
+            target = _aliased_insp._target
+            # maintain all getattr mechanics
+            attr = getattr(target, key)
+
+        # attribute is a method, that will be invoked against a
+        # "self"; so just return a new method with the same function and
+        # new self
+        if hasattr(attr, "__call__") and hasattr(attr, "__self__"):
+            return types.MethodType(attr.__func__, self)
+
+        # attribute is a descriptor, that will be invoked against a
+        # "self"; so invoke the descriptor against this self
+        if hasattr(attr, "__get__"):
+            attr = attr.__get__(None, self)
+
+        # attributes within the QueryableAttribute system will want this
+        # to be invoked so the object can be adapted
+        if hasattr(attr, "adapt_to_entity"):
+            attr = attr.adapt_to_entity(_aliased_insp)
+            setattr(self, key, attr)
+
+        return attr
 
     def __repr__(self):
         return "<AliasedClass at 0x%x; %s>" % (

diff --git a/test/orm/test_utils.py b/test/orm/test_utils.py
index fecb9ad91ade0b23f865f391b88a6b117487e7f3..ebff409da4b845c88f9cc79e3c84a97a8613ea84 100644 (file)
--- a/test/orm/test_utils.py
+++ b/test/orm/test_utils.py
@@ -3,7 +3,6 @@ from sqlalchemy import inspect
 from sqlalchemy import Integer
 from sqlalchemy import MetaData
 from sqlalchemy import Table
-from sqlalchemy import util
 from sqlalchemy.ext.hybrid import hybrid_method
 from sqlalchemy.ext.hybrid import hybrid_property
 from sqlalchemy.orm import aliased
@@ -20,6 +19,7 @@ from sqlalchemy.testing import AssertsCompiledSQL
 from sqlalchemy.testing import eq_
 from sqlalchemy.testing import fixtures
 from sqlalchemy.testing import is_
+from sqlalchemy.util import compat
 from test.orm import _fixtures
 from .inheritance import _poly_fixtures
 
@@ -72,12 +72,7 @@ class AliasedClassTest(fixtures.TestBase, AssertsCompiledSQL):
 
         assert Point.zero
 
-        # TODO: I don't quite understand this
-        # still
-        if util.py2k:
-            assert not getattr(alias, "zero")
-        else:
-            assert getattr(alias, "zero")
+        assert getattr(alias, "zero")
 
     def test_classmethod(self):
         class Point(object):
@@ -248,6 +243,106 @@ class AliasedClassTest(fixtures.TestBase, AssertsCompiledSQL):
             "WHERE point_1.x > point.x",
         )
 
+    def test_meta_getattr_one(self):
+        class MetaPoint(type):
+            def __getattr__(cls, key):
+                if key == "x_syn":
+                    return cls.x
+                raise AttributeError(key)
+
+        class Point(compat.with_metaclass(MetaPoint)):
+            pass
+
+        self._fixture(Point)
+        alias = aliased(Point)
+
+        eq_(str(Point.x_syn), "Point.x")
+        eq_(str(alias.x_syn), "AliasedClass_Point.x")
+
+        # from __clause_element__() perspective, Point.x_syn
+        # and Point.x return the same thing, so that's good
+        eq_(str(Point.x.__clause_element__()), "point.x")
+        eq_(str(Point.x_syn.__clause_element__()), "point.x")
+
+        # same for the alias
+        eq_(str(alias.x + 1), "point_1.x + :x_1")
+        eq_(str(alias.x_syn + 1), "point_1.x + :x_1")
+
+        is_(Point.x_syn.__clause_element__(), Point.x.__clause_element__())
+
+        eq_(str(alias.x_syn == alias.x), "point_1.x = point_1.x")
+
+        a2 = aliased(Point)
+        eq_(str(a2.x_syn == alias.x), "point_1.x = point_2.x")
+
+        sess = Session()
+
+        self.assert_compile(
+            sess.query(alias).filter(alias.x_syn > Point.x),
+            "SELECT point_1.id AS point_1_id, point_1.x AS point_1_x, "
+            "point_1.y AS point_1_y FROM point AS point_1, point "
+            "WHERE point_1.x > point.x",
+        )
+
+    def test_meta_getattr_two(self):
+        class MetaPoint(type):
+            def __getattr__(cls, key):
+                if key == "double_x":
+                    return cls._impl_double_x
+                raise AttributeError(key)
+
+        class Point(compat.with_metaclass(MetaPoint)):
+            @hybrid_property
+            def _impl_double_x(self):
+                return self.x * 2
+
+        self._fixture(Point)
+        alias = aliased(Point)
+
+        eq_(str(Point.double_x), "Point._impl_double_x")
+        eq_(str(alias.double_x), "AliasedClass_Point._impl_double_x")
+        eq_(str(Point.double_x.__clause_element__()), "point.x * :x_1")
+        eq_(str(alias.double_x.__clause_element__()), "point_1.x * :x_1")
+
+        sess = Session()
+
+        self.assert_compile(
+            sess.query(alias).filter(alias.double_x > Point.x),
+            "SELECT point_1.id AS point_1_id, point_1.x AS point_1_x, "
+            "point_1.y AS point_1_y FROM point AS point_1, point "
+            "WHERE point_1.x * :x_1 > point.x",
+        )
+
+    def test_meta_getattr_three(self):
+        class MetaPoint(type):
+            def __getattr__(cls, key):
+                @hybrid_property
+                def double_x(me):
+                    return me.x * 2
+
+                if key == "double_x":
+                    return double_x.__get__(None, cls)
+                raise AttributeError(key)
+
+        class Point(compat.with_metaclass(MetaPoint)):
+            pass
+
+        self._fixture(Point)
+
+        alias = aliased(Point)
+
+        eq_(str(Point.double_x.__clause_element__()), "point.x * :x_1")
+        eq_(str(alias.double_x.__clause_element__()), "point_1.x * :x_1")
+
+        sess = Session()
+
+        self.assert_compile(
+            sess.query(alias).filter(alias.double_x > Point.x),
+            "SELECT point_1.id AS point_1_id, point_1.x AS point_1_x, "
+            "point_1.y AS point_1_y FROM point AS point_1, point "
+            "WHERE point_1.x * :x_1 > point.x",
+        )
+
     def test_parententity_vs_parentmapper(self):
         class Point(object):
             pass