From: Dmytro Starosud Date: Wed, 29 May 2019 13:47:13 +0000 (+0300) Subject: Rework AliasedClass __getattr__ to use top-level getattr() X-Git-Tag: rel_1_3_5~13 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=18c3cf29a983d7743747db9b64a14a7e196228e5;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git 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 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 index 0000000000..6205d0e24c --- /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 2606eee8b5..9d4a0d0823 100644 --- 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 36da781a41..30fc48cd35 100644 --- 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 "" % ( diff --git a/test/orm/test_utils.py b/test/orm/test_utils.py index fecb9ad91a..ebff409da4 100644 --- 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