From: Mike Bayer Date: Thu, 15 Feb 2018 22:42:48 +0000 (-0500) Subject: Test attributes for being non-mapped column properties more closely X-Git-Tag: rel_1_2_3~6^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=283a969d68688018abee04fde479a6fd8495b6f6;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git Test attributes for being non-mapped column properties more closely Fixed bug in concrete inheritance mapping where user-defined attributes such as hybrid properties that mirror the names of mapped attributes from sibling classes would be overwritten by the mapper as non-accessible at the instance level. Also ensured that user-bound descriptors are not implicitly invoked at the class level during the mapper configuration stage. Change-Id: I52b84a15c296b14efeaffb72941fc941d1d52c0d Fixes: #4188 --- diff --git a/doc/build/changelog/unreleased_12/4188.rst b/doc/build/changelog/unreleased_12/4188.rst new file mode 100644 index 0000000000..fd1085472d --- /dev/null +++ b/doc/build/changelog/unreleased_12/4188.rst @@ -0,0 +1,10 @@ +.. change:: + :tags: bug, orm + :tickets: 4188 + + Fixed bug in concrete inheritance mapping where user-defined + attributes such as hybrid properties that mirror the names + of mapped attributes from sibling classes would be overwritten by + the mapper as non-accessible at the instance level. Additionally + ensured that user-bound descriptors are not implicitly invoked at the class + level during the mapper configuration stage. diff --git a/lib/sqlalchemy/orm/instrumentation.py b/lib/sqlalchemy/orm/instrumentation.py index d5422b0d00..1b839cf5c3 100644 --- a/lib/sqlalchemy/orm/instrumentation.py +++ b/lib/sqlalchemy/orm/instrumentation.py @@ -168,6 +168,15 @@ class ClassManager(dict): if isinstance(val, interfaces.InspectionAttr): yield key, val + def _get_class_attr_mro(self, key, default=None): + """return an attribute on the class without tripping it.""" + + for supercls in self.class_.__mro__: + if key in supercls.__dict__: + return supercls.__dict__[key] + else: + return default + def _attr_has_impl(self, key): """Return True if the given attribute is fully initialized. diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py index cd9e00b8b0..a30a8c2433 100644 --- a/lib/sqlalchemy/orm/mapper.py +++ b/lib/sqlalchemy/orm/mapper.py @@ -1613,10 +1613,23 @@ class Mapper(InspectionAttr): if not self.concrete: self._configure_property(key, prop, init=False, setparent=False) elif key not in self._props: - self._configure_property( - key, - properties.ConcreteInheritedProperty(), - init=init, setparent=True) + # determine if the class implements this attribute; if not, + # or if it is implemented by the attribute that is handling the + # given superclass-mapped property, then we need to report that we + # can't use this at the instance level since we are a concrete + # mapper and we don't map this. don't trip user-defined + # descriptors that might have side effects when invoked. + implementing_attribute = self.class_manager._get_class_attr_mro( + key, prop) + if implementing_attribute is prop or (isinstance( + implementing_attribute, + attributes.InstrumentedAttribute) and + implementing_attribute._parententity is prop.parent + ): + self._configure_property( + key, + properties.ConcreteInheritedProperty(), + init=init, setparent=True) def _configure_property(self, key, prop, init=True, setparent=True): self._log("_configure_property(%s, %s)", key, prop.__class__.__name__) @@ -2413,9 +2426,8 @@ class Mapper(InspectionAttr): self.class_.__dict__[assigned_name]): return True else: - if getattr(self.class_, assigned_name, None) is not None \ - and self._is_userland_descriptor( - getattr(self.class_, assigned_name)): + attr = self.class_manager._get_class_attr_mro(assigned_name, None) + if attr is not None and self._is_userland_descriptor(attr): return True if self.include_properties is not None and \ diff --git a/test/orm/inheritance/test_concrete.py b/test/orm/inheritance/test_concrete.py index b103bad7be..edf2c4bdc3 100644 --- a/test/orm/inheritance/test_concrete.py +++ b/test/orm/inheritance/test_concrete.py @@ -10,6 +10,8 @@ from sqlalchemy.testing import fixtures from sqlalchemy.orm import attributes from sqlalchemy.testing import eq_ from sqlalchemy.testing.schema import Table, Column +from sqlalchemy.ext.hybrid import hybrid_property +from sqlalchemy.testing import mock class Employee(object): @@ -217,6 +219,73 @@ class ConcreteTest(fixtures.MappedTest): assert set([repr(x) for x in session.query(Hacker).all()]) \ == set(["Hacker Kurt 'Badass' knows how to hack"]) + def test_multi_level_no_base_w_hybrid(self): + pjoin = polymorphic_union( + {'manager': managers_table, 'engineer': engineers_table, + 'hacker': hackers_table}, + 'type', 'pjoin') + + test_calls = mock.Mock() + + class ManagerWHybrid(Employee): + + def __init__(self, name, manager_data): + self.name = name + self.manager_data = manager_data + + @hybrid_property + def engineer_info(self): + test_calls.engineer_info_instance() + return self.manager_data + + @engineer_info.expression + def engineer_info(cls): + test_calls.engineer_info_class() + return cls.manager_data + + employee_mapper = mapper(Employee, pjoin, + polymorphic_on=pjoin.c.type) + mapper( + ManagerWHybrid, managers_table, + inherits=employee_mapper, + concrete=True, + polymorphic_identity='manager') + mapper( + Engineer, + engineers_table, + inherits=employee_mapper, + concrete=True, + polymorphic_identity='engineer') + + session = create_session() + tom = ManagerWHybrid('Tom', 'mgrdata') + + # mapping did not impact the engineer_info + # hybrid in any way + eq_(test_calls.mock_calls, []) + + eq_( + tom.engineer_info, "mgrdata" + ) + eq_(test_calls.mock_calls, [mock.call.engineer_info_instance()]) + + session.add(tom) + session.flush() + + session.close() + + tom = session.query(ManagerWHybrid).filter( + ManagerWHybrid.engineer_info == 'mgrdata').one() + eq_( + test_calls.mock_calls, + [ + mock.call.engineer_info_instance(), + mock.call.engineer_info_class()] + ) + eq_( + tom.engineer_info, "mgrdata" + ) + def test_multi_level_with_base(self): pjoin = polymorphic_union({ 'employee': employees_table,