[Sqlalchemy-commits] sqlalchemy: - Fixed regression introduced in 0.7b4 (!) whereby
Brought to you by:
zzzeek
From: <co...@sq...> - 2011-04-18 01:03:19
|
details: http://hg.sqlalchemy.org/sqlalchemy/sqlalchemy/rev/fa3feb4adc72 changeset: 7551:fa3feb4adc72 user: zzzeek date: Sun Apr 17 21:03:02 2011 -0400 description: - Fixed regression introduced in 0.7b4 (!) whereby query.options(someoption("nonexistent name")) would fail to raise an error. Also added additional error catching for cases where the option would try to build off a column-based element, further fixed up some of the error messages tailored in [ticket:2069] - added another huge crapload of tests to the existing crapload of tests we already had for options..._get_paths() and dependencies are covered 100% now - one case still doesn't do the "right" thing, using an option specific to relationships will silently pass if the endpoint is a column-based attribute, and vice versa. diffstat: CHANGES | 12 ++ lib/sqlalchemy/__init__.py | 2 +- lib/sqlalchemy/orm/interfaces.py | 53 ++++++-- test/orm/test_query.py | 222 +++++++++++++++++++++++++++++++++++--- 4 files changed, 251 insertions(+), 38 deletions(-) diffs (truncated from 444 to 300 lines): diff -r 3f4f05942eb7 -r fa3feb4adc72 CHANGES --- a/CHANGES Sun Apr 17 16:51:04 2011 -0400 +++ b/CHANGES Sun Apr 17 21:03:02 2011 -0400 @@ -3,6 +3,18 @@ ======= CHANGES ======= +0.7.0b5 +======= +- (note b5 may become 0.7.0) +- orm + - Fixed regression introduced in 0.7b4 (!) whereby + query.options(someoption("nonexistent name")) would + fail to raise an error. Also added additional + error catching for cases where the option would + try to build off a column-based element, further + fixed up some of the error messages tailored + in [ticket:2069] + 0.7.0b4 ======= - general diff -r 3f4f05942eb7 -r fa3feb4adc72 lib/sqlalchemy/__init__.py --- a/lib/sqlalchemy/__init__.py Sun Apr 17 16:51:04 2011 -0400 +++ b/lib/sqlalchemy/__init__.py Sun Apr 17 21:03:02 2011 -0400 @@ -117,6 +117,6 @@ __all__ = sorted(name for name, obj in locals().items() if not (name.startswith('_') or inspect.ismodule(obj))) -__version__ = '0.7b4' +__version__ = '0.7b5' del inspect, sys diff -r 3f4f05942eb7 -r fa3feb4adc72 lib/sqlalchemy/orm/interfaces.py --- a/lib/sqlalchemy/orm/interfaces.py Sun Apr 17 16:51:04 2011 -0400 +++ b/lib/sqlalchemy/orm/interfaces.py Sun Apr 17 21:03:02 2011 -0400 @@ -19,7 +19,7 @@ from itertools import chain from sqlalchemy import exc as sa_exc -from sqlalchemy import log, util, event +from sqlalchemy import util from sqlalchemy.sql import expression deque = util.importlater('collections').deque @@ -435,12 +435,20 @@ return ent else: if raiseerr: - raise sa_exc.ArgumentError( - "Can't find property '%s' on any entity " - "specified in this Query. Note the full path " - "from root (%s) to target entity must be specified." - % (token, ",".join(str(x) for x in query._mapper_entities)) - ) + if not list(query._mapper_entities): + raise sa_exc.ArgumentError( + "Query has only expression-based entities - " + "can't find property named '%s'." + % (token, ) + ) + else: + raise sa_exc.ArgumentError( + "Can't find property '%s' on any entity " + "specified in this Query. Note the full path " + "from root (%s) to target entity must be specified." + % (token, ",".join(str(x) for + x in query._mapper_entities)) + ) else: return None @@ -453,10 +461,9 @@ else: if raiseerr: raise sa_exc.ArgumentError( - "Can't find property named '%s' on the first mapped " - "entity in this Query. " - "Consider using an attribute object instead of a " - "string name to target a specific entity." % (token, ) + "Query has only expression-based entities - " + "can't find property named '%s'." + % (token, ) ) else: return None @@ -493,13 +500,22 @@ query, token, raiseerr) + if entity is None: + return [], [] path_element = entity.path_entity mapper = entity.mapper mappers.append(mapper) if mapper.has_property(token): prop = mapper.get_property(token) else: - prop = None + if raiseerr: + raise sa_exc.ArgumentError( + "Can't find property named '%s' on the " + "mapped entity %s in this Query. " % ( + token, mapper) + ) + else: + return [], [] elif isinstance(token, PropComparator): prop = token.property @@ -528,14 +544,19 @@ raise sa_exc.ArgumentError( "mapper option expects " "string key or list of attributes") - if prop is None: - return [], [] + assert prop is not None path = build_path(path_element, prop.key, path) l.append(path) if getattr(token, '_of_type', None): path_element = mapper = token._of_type else: path_element = mapper = getattr(prop, 'mapper', None) + if mapper is None and tokens: + raise sa_exc.ArgumentError( + "Attribute '%s' of entity '%s' does not " + "refer to a mapped entity" % + (token, entity) + ) if path_element: path_element = path_element @@ -547,8 +568,6 @@ return l, mappers - - class StrategizedOption(PropertyOption): """A MapperOption that affects which LoaderStrategy will be used for an operation by a StrategizedProperty. @@ -726,7 +745,7 @@ setattr(instance, '_default_state', state) def remove_state(self, class_, instance): - delattr(instance, '_default_state', state) + delattr(instance, '_default_state') def state_getter(self, class_): return lambda instance: getattr(instance, '_default_state') diff -r 3f4f05942eb7 -r fa3feb4adc72 test/orm/test_query.py --- a/test/orm/test_query.py Sun Apr 17 16:51:04 2011 -0400 +++ b/test/orm/test_query.py Sun Apr 17 21:03:02 2011 -0400 @@ -1886,7 +1886,6 @@ sess = Session() q = sess.query(Order) - opt = self._option_fixture("addresses") self._assert_path_result(opt, q, [], []) @@ -2108,64 +2107,239 @@ opt = self._option_fixture(Address.user, User.addresses) self._assert_path_result(opt, q, [], []) + def test_multi_entity_opt_on_second(self): + Item = self.classes.Item + Order = self.classes.Order + opt = self._option_fixture(Order.items) + sess = Session() + q = sess.query(Item, Order) + self._assert_path_result(opt, q, [(Order, "items")], [Order]) + + def test_multi_entity_opt_on_string(self): + Item = self.classes.Item + Order = self.classes.Order + opt = self._option_fixture("items") + sess = Session() + q = sess.query(Item, Order) + self._assert_path_result(opt, q, [], []) + + def test_multi_entity_no_mapped_entities(self): + Item = self.classes.Item + Order = self.classes.Order + opt = self._option_fixture("items") + sess = Session() + q = sess.query(Item.id, Order.id) + self._assert_path_result(opt, q, [], []) + + def test_path_exhausted(self): + User = self.classes.User + Item = self.classes.Item + Order = self.classes.Order + opt = self._option_fixture(User.orders) + sess = Session() + q = sess.query(Item)._with_current_path( + self._make_path([User, 'orders', Order, 'items']) + ) + self._assert_path_result(opt, q, [], []) + class OptionsNoPropTest(_fixtures.FixtureTest): """test the error messages emitted when using property - options in conjunection with column-only entities. - + options in conjunection with column-only entities, or + for not existing options + """ run_create_tables = False run_inserts = None run_deletes = None - def test_option_with_mapper_using_basestring(self): + def test_option_with_mapper_basestring(self): Item = self.classes.Item self._assert_option([Item], 'keywords') - def test_option_with_mapper_using_PropCompatator(self): + def test_option_with_mapper_PropCompatator(self): Item = self.classes.Item self._assert_option([Item], Item.keywords) - def test_option_with_mapper_then_column_using_basestring(self): + def test_option_with_mapper_then_column_basestring(self): Item = self.classes.Item self._assert_option([Item, Item.id], 'keywords') - def test_option_with_mapper_then_column_using_PropComparator(self): + def test_option_with_mapper_then_column_PropComparator(self): Item = self.classes.Item self._assert_option([Item, Item.id], Item.keywords) - def test_option_with_column_then_mapper_using_basestring(self): + def test_option_with_column_then_mapper_basestring(self): Item = self.classes.Item self._assert_option([Item.id, Item], 'keywords') - def test_option_with_column_then_mapper_using_PropComparator(self): + def test_option_with_column_then_mapper_PropComparator(self): Item = self.classes.Item self._assert_option([Item.id, Item], Item.keywords) - def test_option_with_column_using_basestring(self): + def test_option_with_column_basestring(self): Item = self.classes.Item message = \ - "Can't find property named 'keywords' on the first mapped "\ - "entity in this Query. Consider using an attribute object "\ - "instead of a string name to target a specific entity." + "Query has only expression-based entities - "\ + "can't find property named 'keywords'." self._assert_eager_with_just_column_exception(Item.id, 'keywords', message) - def test_option_with_column_using_PropComparator(self): + def test_option_with_column_PropComparator(self): Item = self.classes.Item - message = \ - "Can't find property 'keywords' on any entity specified "\ - "in this Query\." self._assert_eager_with_just_column_exception(Item.id, - Item.keywords, message) + Item.keywords, + "Query has only expression-based entities " + "- can't find property named 'keywords'." + ) + + def test_option_against_nonexistent_PropComparator(self): + Item = self.classes.Item + Keyword = self.classes.Keyword + self._assert_eager_not_found_exception( + [Keyword], + (eagerload(Item.keywords), ), + r"Can't find property 'keywords' on any entity specified " + r"in this Query. Note the full path from root " + r"\(Mapper\|Keyword\|keywords\) to target entity must be specified." + ) + + def test_option_against_nonexistent_basestring(self): + Item = self.classes.Item + self._assert_eager_not_found_exception( + [Item], + (eagerload("foo"), ), + r"Can't find property named 'foo' on the mapped " + r"entity Mapper\|Item\|items in this Query." + ) + + def test_option_against_nonexistent_twolevel_basestring(self): + Item = self.classes.Item + self._assert_eager_not_found_exception( + [Item], |