From: Mike Bayer Date: Thu, 23 Aug 2018 16:40:26 +0000 (-0400) Subject: Unwrap Proxy objects when scanning declared_attr X-Git-Tag: rel_1_3_0b1~95^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=626356842d77d4ec6427b3bfc04bdff93d24d246;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git Unwrap Proxy objects when scanning declared_attr Fixed bug where the declarative scan for attributes would receive the expression proxy delivered by a hybrid attribute at the class level, and not the hybrid attribute itself, when receiving the descriptor via the ``@declared_attr`` callable on a subclass of an already-mapped class. This would lead to an attribute that did not report itself as a hybrid when viewed within :attr:`.Mapper.all_orm_descriptors`. Fixes: #4326 Change-Id: I582d03f05c3768b3344f93e3791240e9e69b9d1e --- diff --git a/doc/build/changelog/unreleased_12/4326.rst b/doc/build/changelog/unreleased_12/4326.rst new file mode 100644 index 0000000000..12fef5b612 --- /dev/null +++ b/doc/build/changelog/unreleased_12/4326.rst @@ -0,0 +1,11 @@ +.. change:: + :tags: bug, orm, declarative + :tickets: 4326 + + Fixed bug where the declarative scan for attributes would receive the + expression proxy delivered by a hybrid attribute at the class level, and + not the hybrid attribute itself, when receiving the descriptor via the + ``@declared_attr`` callable on a subclass of an already-mapped class. This + would lead to an attribute that did not report itself as a hybrid when + viewed within :attr:`.Mapper.all_orm_descriptors`. + diff --git a/lib/sqlalchemy/ext/declarative/base.py b/lib/sqlalchemy/ext/declarative/base.py index 818b92c98a..9e15582d6f 100644 --- a/lib/sqlalchemy/ext/declarative/base.py +++ b/lib/sqlalchemy/ext/declarative/base.py @@ -11,7 +11,7 @@ from ...orm import mapper, class_mapper, synonym from ...orm.interfaces import MapperProperty from ...orm.properties import ColumnProperty, CompositeProperty from ...orm.attributes import QueryableAttribute -from ...orm.base import _is_mapped_class +from ...orm.base import _is_mapped_class, InspectionAttr from ... import util, exc from ...util import topological from ...sql import expression @@ -287,8 +287,18 @@ class _MapperConfig(object): util.warn_deprecated( "Use of sqlalchemy.util.classproperty on " "declarative classes is deprecated.") - dict_[name] = column_copies[obj] = \ - ret = getattr(cls, name) + # access attribute using normal class access + ret = getattr(cls, name) + + # correct for proxies created from hybrid_property + # or similar. note there is no known case that + # produces nested proxies, so we are only + # looking one level deep right now. + if isinstance(ret, InspectionAttr) and \ + ret._is_internal_proxy: + ret = ret.descriptor + + dict_[name] = column_copies[obj] = ret if isinstance(ret, (Column, MapperProperty)) and \ ret.doc is None: ret.doc = obj.__doc__ diff --git a/lib/sqlalchemy/orm/attributes.py b/lib/sqlalchemy/orm/attributes.py index 0bbe70655e..745032e447 100644 --- a/lib/sqlalchemy/orm/attributes.py +++ b/lib/sqlalchemy/orm/attributes.py @@ -271,6 +271,8 @@ def create_proxied_attribute(descriptor): self._adapt_to_entity = adapt_to_entity self.__doc__ = doc + _is_internal_proxy = True + @property def property(self): return self.comparator.property diff --git a/lib/sqlalchemy/orm/base.py b/lib/sqlalchemy/orm/base.py index 8c9d562695..e06e1fc78a 100644 --- a/lib/sqlalchemy/orm/base.py +++ b/lib/sqlalchemy/orm/base.py @@ -475,6 +475,13 @@ class InspectionAttr(object): """ + _is_internal_proxy = False + """True if this object is an internal proxy object. + + .. versionadded:: 1.2.12 + + """ + is_clause_element = False """True if this object is an instance of :class:`.ClauseElement`.""" diff --git a/lib/sqlalchemy/orm/descriptor_props.py b/lib/sqlalchemy/orm/descriptor_props.py index a8db3fe1c0..aaf53b6981 100644 --- a/lib/sqlalchemy/orm/descriptor_props.py +++ b/lib/sqlalchemy/orm/descriptor_props.py @@ -500,6 +500,7 @@ class ConcreteInheritedProperty(DescriptorProperty): (self.parent, self.key, self.parent)) class NoninheritedConcreteProp(object): + def __set__(s, obj, value): warn() diff --git a/test/ext/declarative/test_mixin.py b/test/ext/declarative/test_mixin.py index 07c790dc42..6f95bed60f 100644 --- a/test/ext/declarative/test_mixin.py +++ b/test/ext/declarative/test_mixin.py @@ -1325,6 +1325,52 @@ class DeclarativeMixinPropertyTest(DeclarativeTestBase): eq_(MyModel.type_.__doc__, """this is a document.""") eq_(MyModel.t2.__doc__, """this is another document.""") + def test_correct_for_proxies(self): + from sqlalchemy.ext.hybrid import hybrid_property + from sqlalchemy.ext import hybrid + from sqlalchemy import inspect + + class Mixin(object): + @hybrid_property + def hp1(cls): + return 42 + + @declared_attr + def hp2(cls): + @hybrid_property + def hp2(self): + return 42 + + return hp2 + + class Base(declarative_base(), Mixin): + __tablename__ = 'test' + id = Column(String, primary_key=True) + + class Derived(Base): + pass + + # in all cases we get a proxy when we use class-bound access + # for the hybrid + assert Base.hp1._is_internal_proxy + assert Base.hp2._is_internal_proxy + assert Derived.hp1._is_internal_proxy + assert Derived.hp2._is_internal_proxy + + # however when declarative sets it up, it checks for this proxy + # and adjusts + b1 = inspect(Base) + d1 = inspect(Derived) + is_( + b1.all_orm_descriptors['hp1'], + d1.all_orm_descriptors['hp1'], + ) + + is_( + b1.all_orm_descriptors['hp2'], + d1.all_orm_descriptors['hp2'], + ) + def test_column_in_mapper_args(self): class MyMixin(object):