From 0afe1ba02d5817f6a46a1ae567a90a5188239981 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Fri, 6 Aug 2010 13:19:59 -0400 Subject: [PATCH] - step one, remove "resolve_synonyms" and start using class attribute access to get at mapped properties by name --- lib/sqlalchemy/orm/dynamic.py | 2 +- lib/sqlalchemy/orm/interfaces.py | 6 ++- lib/sqlalchemy/orm/mapper.py | 30 ++++++------- lib/sqlalchemy/orm/properties.py | 16 +++---- lib/sqlalchemy/orm/query.py | 2 +- lib/sqlalchemy/orm/scoping.py | 3 +- lib/sqlalchemy/orm/strategies.py | 4 +- lib/sqlalchemy/orm/util.py | 11 +++-- test/orm/test_mapper.py | 20 +++++++-- test/orm/test_query.py | 73 ++++++++++++++++++++++++-------- 10 files changed, 106 insertions(+), 61 deletions(-) diff --git a/lib/sqlalchemy/orm/dynamic.py b/lib/sqlalchemy/orm/dynamic.py index d558380114..d2063dbc6f 100644 --- a/lib/sqlalchemy/orm/dynamic.py +++ b/lib/sqlalchemy/orm/dynamic.py @@ -192,7 +192,7 @@ class AppenderMixin(object): self.attr = attr mapper = object_mapper(instance) - prop = mapper.get_property(self.attr.key, resolve_synonyms=True) + prop = mapper.get_property(self.attr.key) self._criterion = prop.compare( operators.eq, instance, diff --git a/lib/sqlalchemy/orm/interfaces.py b/lib/sqlalchemy/orm/interfaces.py index 91c2ae403e..4053169769 100644 --- a/lib/sqlalchemy/orm/interfaces.py +++ b/lib/sqlalchemy/orm/interfaces.py @@ -838,8 +838,10 @@ class PropertyOption(MapperOption): path_element = entity.path_entity mapper = entity.mapper mappers.append(mapper) - prop = mapper.get_property(token, - resolve_synonyms=True, raiseerr=raiseerr) + if mapper.has_property(token): + prop = mapper.get_property(token) + else: + prop = None key = token elif isinstance(token, PropComparator): prop = token.property diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py index 9cb3581517..7c2f550eb7 100644 --- a/lib/sqlalchemy/orm/mapper.py +++ b/lib/sqlalchemy/orm/mapper.py @@ -905,26 +905,22 @@ class Mapper(object): def has_property(self, key): return key in self._props - def get_property(self, key, resolve_synonyms=False, raiseerr=True): - """return a MapperProperty associated with the given key.""" + def get_property(self, key, _compile_mappers=True): + """return a MapperProperty associated with the given key. + + Calls getattr() against the mapped class itself, so that class-level + proxies will be resolved to the underlying property, if any. + + """ - if not self.compiled: + if _compile_mappers and not self.compiled: self.compile() - return self._get_property(key, - resolve_synonyms=resolve_synonyms, - raiseerr=raiseerr) - - def _get_property(self, key, resolve_synonyms=False, raiseerr=True): - prop = self._props.get(key, None) - if resolve_synonyms: - while isinstance(prop, SynonymProperty): - prop = self._props.get(prop.name, None) - if prop is None and raiseerr: + try: + return getattr(self.class_, key).property + except AttributeError: raise sa_exc.InvalidRequestError( - "Mapper '%s' has no property '%s'" % - (self, key)) - return prop - + "Mapper '%s' has no property '%s'" % (self, key)) + @property def iterate_properties(self): """return an iterator of all MapperProperty objects.""" diff --git a/lib/sqlalchemy/orm/properties.py b/lib/sqlalchemy/orm/properties.py index cf5f31162f..53503862bb 100644 --- a/lib/sqlalchemy/orm/properties.py +++ b/lib/sqlalchemy/orm/properties.py @@ -284,7 +284,7 @@ class ConcreteInheritedProperty(MapperProperty): comparator_callable = None # TODO: put this process into a deferred callable? for m in self.parent.iterate_to_root(): - p = m._get_property(self.key) + p = m._props[self.key] if not isinstance(p, ConcreteInheritedProperty): comparator_callable = p.comparator_factory break @@ -337,8 +337,8 @@ class SynonymProperty(MapperProperty): def comparator_callable(prop, mapper): def comparator(): - prop = self.parent._get_property( - self.key, resolve_synonyms=True) + prop = getattr(self.parent.class_, + self.name).property if self.comparator_factory: return self.comparator_factory(prop, mapper) else: @@ -837,7 +837,7 @@ class RelationshipProperty(StrategizedProperty): yield (c, instance_mapper, instance_state) def _add_reverse_property(self, key): - other = self.mapper._get_property(key) + other = self.mapper.get_property(key, _compile_mappers=False) self._reverse_property.add(other) other._reverse_property.add(self) @@ -924,8 +924,7 @@ class RelationshipProperty(StrategizedProperty): if not self.parent.concrete: for inheriting in self.parent.iterate_to_root(): if inheriting is not self.parent \ - and inheriting._get_property(self.key, - raiseerr=False): + and inheriting.has_property(self.key): util.warn("Warning: relationship '%s' on mapper " "'%s' supercedes the same relationship " "on inherited mapper '%s'; this can " @@ -1216,7 +1215,7 @@ class RelationshipProperty(StrategizedProperty): def _assert_is_primary(self): if not self.is_primary() \ and not mapper.class_mapper(self.parent.class_, - compile=False)._get_property(self.key, raiseerr=False): + compile=False).has_property(self.key): raise sa_exc.ArgumentError("Attempting to assign a new " "relationship '%s' to a non-primary mapper on " "class '%s'. New relationships can only be added " @@ -1234,8 +1233,7 @@ class RelationshipProperty(StrategizedProperty): else: backref_key, kwargs = self.backref mapper = self.mapper.primary_mapper() - if mapper._get_property(backref_key, raiseerr=False) \ - is not None: + if mapper.has_property(backref_key): raise sa_exc.ArgumentError("Error creating backref " "'%s' on relationship '%s': property of that " "name exists on mapper '%s'" % (backref_key, diff --git a/lib/sqlalchemy/orm/query.py b/lib/sqlalchemy/orm/query.py index cc6d15a74b..8d468babe3 100644 --- a/lib/sqlalchemy/orm/query.py +++ b/lib/sqlalchemy/orm/query.py @@ -662,7 +662,7 @@ class Query(object): instance.__class__.__name__) ) else: - prop = mapper.get_property(property, resolve_synonyms=True) + prop = mapper.get_property(property) return self.filter(prop.compare( operators.eq, instance, value_is_parent=True)) diff --git a/lib/sqlalchemy/orm/scoping.py b/lib/sqlalchemy/orm/scoping.py index 40bbb3299d..1b9ad8e6f0 100644 --- a/lib/sqlalchemy/orm/scoping.py +++ b/lib/sqlalchemy/orm/scoping.py @@ -179,8 +179,7 @@ class _ScopedExt(MapperExtension): def __init__(self, **kwargs): for key, value in kwargs.iteritems(): if ext.validate: - if not mapper.get_property(key, resolve_synonyms=False, - raiseerr=False): + if not mapper.has_property(key): raise sa_exc.ArgumentError( "Invalid __init__ argument: '%s'" % key) setattr(self, key, value) diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py index 1c4571aed8..afa3ebf4ca 100644 --- a/lib/sqlalchemy/orm/strategies.py +++ b/lib/sqlalchemy/orm/strategies.py @@ -1215,7 +1215,7 @@ class LoadEagerFromAliasOption(PropertyOption): if isinstance(self.alias, basestring): mapper = mappers[-1] (root_mapper, propname) = paths[-1][-2:] - prop = mapper.get_property(propname, resolve_synonyms=True) + prop = mapper.get_property(propname) self.alias = prop.target.alias(self.alias) query._attributes[ ("user_defined_eager_row_processor", @@ -1224,7 +1224,7 @@ class LoadEagerFromAliasOption(PropertyOption): else: (root_mapper, propname) = paths[-1][-2:] mapper = mappers[-1] - prop = mapper.get_property(propname, resolve_synonyms=True) + prop = mapper.get_property(propname) adapter = query._polymorphic_adapters.get(prop.mapper, None) query._attributes[ ("user_defined_eager_row_processor", diff --git a/lib/sqlalchemy/orm/util.py b/lib/sqlalchemy/orm/util.py index ef5413724b..c9004990ab 100644 --- a/lib/sqlalchemy/orm/util.py +++ b/lib/sqlalchemy/orm/util.py @@ -347,10 +347,6 @@ class AliasedClass(object): return queryattr def __getattr__(self, key): - prop = self.__mapper._get_property(key, raiseerr=False) - if prop: - return self.__adapt_prop(prop) - for base in self.__target.__mro__: try: attr = object.__getattribute__(base, key) @@ -360,7 +356,10 @@ class AliasedClass(object): break else: raise AttributeError(key) - + + if isinstance(attr, attributes.QueryableAttribute): + return self.__adapt_prop(attr.property) + if hasattr(attr, 'func_code'): is_method = getattr(self.__target, key, None) if is_method and is_method.im_self is not None: @@ -494,7 +493,7 @@ def with_parent(instance, prop): """ if isinstance(prop, basestring): mapper = object_mapper(instance) - prop = mapper.get_property(prop, resolve_synonyms=True) + prop = mapper.get_property(prop) elif isinstance(prop, attributes.QueryableAttribute): prop = prop.property diff --git a/test/orm/test_mapper.py b/test/orm/test_mapper.py index e24906a1f5..e301b8d07e 100644 --- a/test/orm/test_mapper.py +++ b/test/orm/test_mapper.py @@ -1543,10 +1543,22 @@ class ComparatorFactoryTest(_fixtures.FixtureTest, AssertsCompiledSQL): class MyFactory(ColumnProperty.Comparator): __hash__ = None def __eq__(self, other): - return func.foobar(self.__clause_element__()) == func.foobar(other) - mapper(User, users, properties={'name':synonym('_name', map_column=True, comparator_factory=MyFactory)}) - self.assert_compile(User.name == 'ed', "foobar(users.name) = foobar(:foobar_1)", dialect=default.DefaultDialect()) - self.assert_compile(aliased(User).name == 'ed', "foobar(users_1.name) = foobar(:foobar_1)", dialect=default.DefaultDialect()) + return func.foobar(self.__clause_element__()) ==\ + func.foobar(other) + + mapper(User, users, properties={ + 'name':synonym('_name', map_column=True, + comparator_factory=MyFactory) + }) + self.assert_compile( + User.name == 'ed', + "foobar(users.name) = foobar(:foobar_1)", + dialect=default.DefaultDialect()) + + self.assert_compile( + aliased(User).name == 'ed', + "foobar(users_1.name) = foobar(:foobar_1)", + dialect=default.DefaultDialect()) @testing.resolve_artifact_names def test_relationship(self): diff --git a/test/orm/test_query.py b/test/orm/test_query.py index ebe72b5655..5fa316b76f 100644 --- a/test/orm/test_query.py +++ b/test/orm/test_query.py @@ -3448,53 +3448,92 @@ class SelectFromTest(QueryTest, AssertsCompiledSQL): 'orders':relationship(Order, backref='user'), # o2m, m2o }) mapper(Order, orders, properties={ - 'items':relationship(Item, secondary=order_items, order_by=items.c.id), #m2m + 'items':relationship(Item, secondary=order_items, + order_by=items.c.id), #m2m }) mapper(Item, items, properties={ - 'keywords':relationship(Keyword, secondary=item_keywords, order_by=keywords.c.id) #m2m + 'keywords':relationship(Keyword, secondary=item_keywords, + order_by=keywords.c.id) #m2m }) mapper(Keyword, keywords) - sel = users.select(users.c.id.in_([7, 8])) sess = create_session() - - eq_(sess.query(User).select_from(sel).join('orders', 'items', 'keywords').filter(Keyword.name.in_(['red', 'big', 'round'])).all(), [ + sel = users.select(users.c.id.in_([7, 8])) + + eq_(sess.query(User).select_from(sel).\ + join('orders', 'items', 'keywords').\ + filter(Keyword.name.in_(['red', 'big', 'round'])).\ + all(), + [ User(name=u'jack',id=7) ]) - eq_(sess.query(User).select_from(sel).join('orders', 'items', 'keywords', aliased=True).filter(Keyword.name.in_(['red', 'big', 'round'])).all(), [ + eq_(sess.query(User).select_from(sel).\ + join('orders', 'items', 'keywords', aliased=True).\ + filter(Keyword.name.in_(['red', 'big', 'round'])).\ + all(), + [ User(name=u'jack',id=7) ]) def go(): eq_( sess.query(User).select_from(sel). - options(joinedload_all('orders.items.keywords')). - join('orders', 'items', 'keywords', aliased=True). - filter(Keyword.name.in_(['red', 'big', 'round'])).all(), + options(joinedload_all('orders.items.keywords')). + join('orders', 'items', 'keywords', aliased=True). + filter(Keyword.name.in_(['red', 'big', 'round'])).\ + all(), [ User(name=u'jack',orders=[ Order(description=u'order 1',items=[ - Item(description=u'item 1',keywords=[Keyword(name=u'red'), Keyword(name=u'big'), Keyword(name=u'round')]), - Item(description=u'item 2',keywords=[Keyword(name=u'red',id=2), Keyword(name=u'small',id=5), Keyword(name=u'square')]), - Item(description=u'item 3',keywords=[Keyword(name=u'green',id=3), Keyword(name=u'big',id=4), Keyword(name=u'round',id=6)]) + Item(description=u'item 1', + keywords=[ + Keyword(name=u'red'), + Keyword(name=u'big'), + Keyword(name=u'round') + ]), + Item(description=u'item 2', + keywords=[ + Keyword(name=u'red',id=2), + Keyword(name=u'small',id=5), + Keyword(name=u'square') + ]), + Item(description=u'item 3', + keywords=[ + Keyword(name=u'green',id=3), + Keyword(name=u'big',id=4), + Keyword(name=u'round',id=6)]) ]), Order(description=u'order 3',items=[ - Item(description=u'item 3',keywords=[Keyword(name=u'green',id=3), Keyword(name=u'big',id=4), Keyword(name=u'round',id=6)]), + Item(description=u'item 3', + keywords=[ + Keyword(name=u'green',id=3), + Keyword(name=u'big',id=4), + Keyword(name=u'round',id=6) + ]), Item(description=u'item 4',keywords=[],id=4), Item(description=u'item 5',keywords=[],id=5) ]), - Order(description=u'order 5',items=[Item(description=u'item 5',keywords=[])])]) + Order(description=u'order 5', + items=[ + Item(description=u'item 5',keywords=[])]) + ]) ]) self.assert_sql_count(testing.db, go, 1) - + sess.expunge_all() sel2 = orders.select(orders.c.id.in_([1,2,3])) - eq_(sess.query(Order).select_from(sel2).join('items', 'keywords').filter(Keyword.name == 'red').order_by(Order.id).all(), [ + eq_(sess.query(Order).select_from(sel2).\ + join('items', 'keywords').\ + filter(Keyword.name == 'red').\ + order_by(Order.id).all(), [ Order(description=u'order 1',id=1), Order(description=u'order 2',id=2), ]) - eq_(sess.query(Order).select_from(sel2).join('items', 'keywords', aliased=True).filter(Keyword.name == 'red').order_by(Order.id).all(), [ + eq_(sess.query(Order).select_from(sel2).\ + join('items', 'keywords', aliased=True).\ + filter(Keyword.name == 'red').\ + order_by(Order.id).all(), [ Order(description=u'order 1',id=1), Order(description=u'order 2',id=2), ]) -- 2.47.3