From 7f043a9666eecdecc54fe779ffdd50a7d5bb0086 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Thu, 25 Oct 2012 20:28:03 -0400 Subject: [PATCH] - some naming changes on PropComparator, Comparator: 1. all Comparators now have "parent" which is always the parent mapper or AliasedClass instance 2. only RelationshipProperty.Comparator has "mapper" now, which is the target mapper 3. The names "parententity" and "parentmapper" are underscored also improved the message with the "neither comparator nor instruentedattribute...." to include the classname + attribute name --- lib/sqlalchemy/orm/attributes.py | 19 +++++++++++---- lib/sqlalchemy/orm/interfaces.py | 13 +++++----- lib/sqlalchemy/orm/properties.py | 37 ++++++++++++++++++++++------ lib/sqlalchemy/orm/query.py | 5 ++-- lib/sqlalchemy/orm/util.py | 2 +- test/orm/test_inspect.py | 42 ++++++++++++++++++++++++++++++-- test/orm/test_mapper.py | 26 ++++++++++++++------ 7 files changed, 113 insertions(+), 31 deletions(-) diff --git a/lib/sqlalchemy/orm/attributes.py b/lib/sqlalchemy/orm/attributes.py index 86da9a61d1..7effd8e58b 100644 --- a/lib/sqlalchemy/orm/attributes.py +++ b/lib/sqlalchemy/orm/attributes.py @@ -131,7 +131,7 @@ class QueryableAttribute(interfaces._MappedAttribute, self.key = key self.impl = impl self.comparator = comparator - self.parententity = parententity + self._parententity = parententity self._of_type = of_type manager = manager_of_class(class_) @@ -158,6 +158,10 @@ class QueryableAttribute(interfaces._MappedAttribute, # TODO: conditionally attach this method based on clause_element ? return self + @property + def parent(self): + return self._parententity + @property def expression(self): return self.comparator.__clause_element__() @@ -171,7 +175,7 @@ class QueryableAttribute(interfaces._MappedAttribute, self.key, self.impl, self.comparator.of_type(cls), - self.parententity, + self._parententity, of_type=cls) def label(self, name): @@ -191,9 +195,11 @@ class QueryableAttribute(interfaces._MappedAttribute, return getattr(self.comparator, key) except AttributeError: raise AttributeError( - 'Neither %r object nor %r object has an attribute %r' % ( + 'Neither %r object nor %r object associated with %s ' + 'has an attribute %r' % ( type(self).__name__, type(self.comparator).__name__, + self, key) ) @@ -281,7 +287,7 @@ def create_proxied_attribute(descriptor): return self.descriptor.__get__(instance, owner) def __str__(self): - return self.key + return "%s.%s" % (self.class_.__name__, self.key) def __getattr__(self, attribute): """Delegate __getattr__ to the original descriptor and/or @@ -294,12 +300,15 @@ def create_proxied_attribute(descriptor): return getattr(self.comparator, attribute) except AttributeError: raise AttributeError( - 'Neither %r object nor %r object has an attribute %r' % ( + 'Neither %r object nor %r object associated with %s ' + 'has an attribute %r' % ( type(descriptor).__name__, type(self.comparator).__name__, + self, attribute) ) + Proxy.__name__ = type(descriptor).__name__ + 'Proxy' util.monkeypatch_proxied_specials(Proxy, type(descriptor), diff --git a/lib/sqlalchemy/orm/interfaces.py b/lib/sqlalchemy/orm/interfaces.py index 272d4edd57..b30630434d 100644 --- a/lib/sqlalchemy/orm/interfaces.py +++ b/lib/sqlalchemy/orm/interfaces.py @@ -305,11 +305,12 @@ class PropComparator(operators.ColumnOperators): """ - def __init__(self, prop, mapper, adapter=None): + def __init__(self, prop, parentmapper, adapter=None): self.prop = self.property = prop - self.mapper = mapper + self._parentmapper = parentmapper self.adapter = adapter + def __clause_element__(self): raise NotImplementedError("%r" % self) @@ -319,7 +320,7 @@ class PropComparator(operators.ColumnOperators): """ - return self.__class__(self.prop, self.mapper, adapter) + return self.__class__(self.prop, self._parentmapper, adapter) @staticmethod def any_op(a, b, **kwargs): @@ -503,7 +504,7 @@ class PropertyOption(MapperOption): d['key'] = ret = [] for token in util.to_list(self.key): if isinstance(token, PropComparator): - ret.append((token.mapper.class_, token.key)) + ret.append((token._parentmapper.class_, token.key)) else: ret.append(token) return d @@ -628,7 +629,7 @@ class PropertyOption(MapperOption): # matching tokens to entities if current_path: if current_path[0:2] == \ - [token.parententity, prop.key]: + [token._parententity, prop.key]: current_path = current_path[2:] continue else: @@ -638,7 +639,7 @@ class PropertyOption(MapperOption): entity = self._find_entity_prop_comparator( query, prop.key, - token.parententity, + token._parententity, raiseerr) if not entity: return no_result diff --git a/lib/sqlalchemy/orm/properties.py b/lib/sqlalchemy/orm/properties.py index 048b4fad39..a0abb2743e 100644 --- a/lib/sqlalchemy/orm/properties.py +++ b/lib/sqlalchemy/orm/properties.py @@ -189,8 +189,8 @@ class ColumnProperty(StrategizedProperty): return self.adapter(self.prop.columns[0]) else: return self.prop.columns[0]._annotate({ - "parententity": self.mapper, - "parentmapper": self.mapper}) + "parententity": self._parentmapper, + "parentmapper": self._parentmapper}) def __getattr__(self, key): """proxy attribute access down to the mapped column. @@ -352,13 +352,13 @@ class RelationshipProperty(StrategizedProperty): _of_type = None - def __init__(self, prop, mapper, of_type=None, adapter=None): + def __init__(self, prop, parentmapper, of_type=None, adapter=None): """Construction of :class:`.RelationshipProperty.Comparator` is internal to the ORM's attribute mechanics. """ self.prop = prop - self.mapper = mapper + self._parentmapper = parentmapper self.adapter = adapter if of_type: self._of_type = of_type @@ -370,14 +370,37 @@ class RelationshipProperty(StrategizedProperty): """ - return self.__class__(self.property, self.mapper, + return self.__class__(self.property, self._parentmapper, getattr(self, '_of_type', None), adapter) @util.memoized_property - def parententity(self): + def mapper(self): + """The target :class:`.Mapper` referred to by this + :class:`.RelationshipProperty.Comparator. + + This is the "target" or "remote" side of the + :func:`.relationship`. + + """ + return self.property.mapper + + @util.memoized_property + def parent(self): + """The parent :class:`.Mapper` or :class:`.AliasedClass` + referred to by this + :class:`.RelationshipProperty.Comparator. + + This is the "parent" or "local" side of the + :func:`.relationship`. + + """ return self.property.parent + @util.memoized_property + def _parententity(self): + return self.parent + def _source_selectable(self): elem = self.property.parent._with_polymorphic_selectable if self.adapter: @@ -412,7 +435,7 @@ class RelationshipProperty(StrategizedProperty): """ return RelationshipProperty.Comparator( self.property, - self.mapper, + self._parentmapper, cls, adapter=self.adapter) def in_(self, other): diff --git a/lib/sqlalchemy/orm/query.py b/lib/sqlalchemy/orm/query.py index a6d20a9730..02fb7d4f77 100644 --- a/lib/sqlalchemy/orm/query.py +++ b/lib/sqlalchemy/orm/query.py @@ -1727,7 +1727,7 @@ class Query(object): # and Class is that of the current joinpoint elif from_joinpoint and \ isinstance(onclause, interfaces.PropComparator): - left_entity = onclause.parententity + left_entity = onclause._parententity info = inspect(self._joinpoint_zero()) left_mapper, left_selectable, left_is_aliased = \ @@ -1750,7 +1750,8 @@ class Query(object): else: right_entity = onclause.property.mapper - left_entity = onclause.parententity + left_entity = onclause._parententity + assert left_entity is onclause.parent prop = onclause.property if not isinstance(onclause, attributes.QueryableAttribute): diff --git a/lib/sqlalchemy/orm/util.py b/lib/sqlalchemy/orm/util.py index 2e38e0ce36..fb4197c584 100644 --- a/lib/sqlalchemy/orm/util.py +++ b/lib/sqlalchemy/orm/util.py @@ -464,7 +464,7 @@ class AliasedClass(object): # used to assign a name to the RowTuple object # returned by Query. self._sa_label_name = aliased_insp.name - self.__name__ = 'AliasedClass_' + str(self.__target) + self.__name__ = 'AliasedClass_%s' % self.__target.__name__ @util.memoized_property def _sa_path_registry(self): diff --git a/test/orm/test_inspect.py b/test/orm/test_inspect.py index f504ad16e1..cd9a30b1a7 100644 --- a/test/orm/test_inspect.py +++ b/test/orm/test_inspect.py @@ -175,23 +175,61 @@ class TestORMInspection(_fixtures.FixtureTest): set(['orders', 'addresses']) ) - def test_insp_prop(self): + def test_insp_relationship_prop(self): User = self.classes.User + Address = self.classes.Address prop = inspect(User.addresses) is_(prop, User.addresses) + is_(prop.parent, class_mapper(User)) + is_(prop._parentmapper, class_mapper(User)) + is_(prop.mapper, class_mapper(Address)) - def test_insp_aliased_prop(self): + def test_insp_aliased_relationship_prop(self): User = self.classes.User + Address = self.classes.Address ua = aliased(User) prop = inspect(ua.addresses) is_(prop, ua.addresses) + is_(prop.property.parent, class_mapper(User)) + is_(prop.property.mapper, class_mapper(Address)) + is_(prop.parent, ua) + is_(prop._parentmapper, class_mapper(User)) + is_(prop.mapper, class_mapper(Address)) + + is_(prop._parententity, ua) + + def test_insp_column_prop(self): + User = self.classes.User + prop = inspect(User.name) + is_(prop, User.name) + + is_(prop.parent, class_mapper(User)) + assert not hasattr(prop, "mapper") + + def test_insp_aliased_column_prop(self): + User = self.classes.User + ua = aliased(User) + prop = inspect(ua.name) + is_(prop, ua.name) + + is_(prop.property.parent, class_mapper(User)) + assert not hasattr(prop.property, "mapper") + is_(prop.parent, ua) + is_(prop._parentmapper, class_mapper(User)) + + assert not hasattr(prop, "mapper") + + is_(prop._parententity, ua) + def test_rel_accessors(self): User = self.classes.User Address = self.classes.Address prop = inspect(User.addresses) is_(prop.property.parent, class_mapper(User)) is_(prop.property.mapper, class_mapper(Address)) + is_(prop.parent, class_mapper(User)) + is_(prop.mapper, class_mapper(Address)) assert not hasattr(prop, 'columns') assert hasattr(prop, 'expression') diff --git a/test/orm/test_mapper.py b/test/orm/test_mapper.py index f41843455c..1d7e0228cd 100644 --- a/test/orm/test_mapper.py +++ b/test/orm/test_mapper.py @@ -1331,7 +1331,7 @@ class MapperTest(_fixtures.FixtureTest, AssertsCompiledSQL): assert_raises_message( AttributeError, "Neither 'extendedproperty' object nor 'UCComparator' " - "object has an attribute 'nonexistent'", + "object associated with User.uc_name has an attribute 'nonexistent'", getattr, User.uc_name, 'nonexistent') # test compile @@ -1350,8 +1350,8 @@ class MapperTest(_fixtures.FixtureTest, AssertsCompiledSQL): sess.expunge_all() q = sess.query(User) - u2 = q.filter(User.name=='some user name').one() - u3 = q.filter(User.uc_name=='SOME USER NAME').one() + u2 = q.filter(User.name == 'some user name').one() + u3 = q.filter(User.uc_name == 'SOME USER NAME').one() assert u2 is u3 @@ -1365,23 +1365,33 @@ class MapperTest(_fixtures.FixtureTest, AssertsCompiledSQL): __hash__ = None def __eq__(self, other): # lower case comparison - return func.lower(self.__clause_element__()) == func.lower(other) + return func.lower(self.__clause_element__() + ) == func.lower(other) def intersects(self, other): # non-standard comparator return self.__clause_element__().op('&=')(other) mapper(User, users, properties={ - 'name':sa.orm.column_property(users.c.name, comparator_factory=MyComparator) + 'name': sa.orm.column_property(users.c.name, + comparator_factory=MyComparator) }) assert_raises_message( AttributeError, - "Neither 'InstrumentedAttribute' object nor 'MyComparator' object has an attribute 'nonexistent'", + "Neither 'InstrumentedAttribute' object nor " + "'MyComparator' object associated with User.name has " + "an attribute 'nonexistent'", getattr, User.name, "nonexistent") - eq_(str((User.name == 'ed').compile(dialect=sa.engine.default.DefaultDialect())) , "lower(users.name) = lower(:lower_1)") - eq_(str((User.name.intersects('ed')).compile(dialect=sa.engine.default.DefaultDialect())), "users.name &= :name_1") + eq_( + str((User.name == 'ed').compile( + dialect=sa.engine.default.DefaultDialect())), + "lower(users.name) = lower(:lower_1)") + eq_( + str((User.name.intersects('ed')).compile( + dialect=sa.engine.default.DefaultDialect())), + "users.name &= :name_1") def test_reentrant_compile(self): -- 2.47.3