From b02193bd7e84a8fe1ac74d16cf78a90bd3d59f7c Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Sun, 17 Apr 2011 21:03:02 -0400 Subject: [PATCH] - 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. --- 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(-) diff --git a/CHANGES b/CHANGES index 29fdb757e2..a781307b42 100644 --- a/CHANGES +++ b/CHANGES @@ -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 --git a/lib/sqlalchemy/__init__.py b/lib/sqlalchemy/__init__.py index a20554dc0d..93f3cf04c4 100644 --- a/lib/sqlalchemy/__init__.py +++ b/lib/sqlalchemy/__init__.py @@ -117,6 +117,6 @@ from sqlalchemy.engine import create_engine, engine_from_config __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 --git a/lib/sqlalchemy/orm/interfaces.py b/lib/sqlalchemy/orm/interfaces.py index fec9d45fc7..7a87485566 100644 --- a/lib/sqlalchemy/orm/interfaces.py +++ b/lib/sqlalchemy/orm/interfaces.py @@ -19,7 +19,7 @@ classes within should be considered mostly private. 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 @@ class PropertyOption(MapperOption): 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 @@ class PropertyOption(MapperOption): 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 @@ class PropertyOption(MapperOption): 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 @@ class PropertyOption(MapperOption): 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 @@ class PropertyOption(MapperOption): 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 @@ class InstrumentationManager(object): 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 --git a/test/orm/test_query.py b/test/orm/test_query.py index 838bbcde06..93f349ff29 100644 --- a/test/orm/test_query.py +++ b/test/orm/test_query.py @@ -1886,7 +1886,6 @@ class OptionsTest(QueryTest): sess = Session() q = sess.query(Order) - opt = self._option_fixture("addresses") self._assert_path_result(opt, q, [], []) @@ -2108,64 +2107,239 @@ class OptionsTest(QueryTest): 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], + (eagerload("keywords.foo"), ), + r"Can't find property named 'foo' on the mapped entity " + r"Mapper\|Keyword\|keywords in this Query." + ) + + def test_option_against_nonexistent_twolevel_all(self): + Item = self.classes.Item + self._assert_eager_not_found_exception( + [Item], + (eagerload_all("keywords.foo"), ), + r"Can't find property named 'foo' on the mapped entity " + r"Mapper\|Keyword\|keywords in this Query." + ) + + @testing.fails_if(lambda:True, + "PropertyOption doesn't yet check for relation/column on end result") + def test_option_against_non_relation_basestring(self): + Item = self.classes.Item + Keyword = self.classes.Keyword + self._assert_eager_not_found_exception( + [Keyword, Item], + (eagerload_all("keywords"), ), + r"Attribute 'keywords' of entity 'Mapper\|Keyword\|keywords' " + "does not refer to a mapped entity" + ) + + @testing.fails_if(lambda:True, + "PropertyOption doesn't yet check for relation/column on end result") + def test_option_against_multi_non_relation_basestring(self): + Item = self.classes.Item + Keyword = self.classes.Keyword + self._assert_eager_not_found_exception( + [Keyword, Item], + (eagerload_all("keywords"), ), + r"Attribute 'keywords' of entity 'Mapper\|Keyword\|keywords' " + "does not refer to a mapped entity" + ) + + def test_option_against_wrong_entity_type_basestring(self): + Item = self.classes.Item + self._assert_eager_not_found_exception( + [Item], + (eagerload_all("id", "keywords"), ), + r"Attribute 'id' of entity 'Mapper\|Item\|items' does not " + r"refer to a mapped entity" + ) + + def test_option_against_multi_non_relation_twolevel_basestring(self): + Item = self.classes.Item + Keyword = self.classes.Keyword + self._assert_eager_not_found_exception( + [Keyword, Item], + (eagerload_all("id", "keywords"), ), + r"Attribute 'id' of entity 'Mapper\|Keyword\|keywords' " + "does not refer to a mapped entity" + ) + + def test_option_against_multi_nonexistent_basestring(self): + Item = self.classes.Item + Keyword = self.classes.Keyword + self._assert_eager_not_found_exception( + [Keyword, Item], + (eagerload_all("description"), ), + r"Can't find property named 'description' on the mapped " + r"entity Mapper\|Keyword\|keywords in this Query." + ) + + def test_option_against_multi_no_entities_basestring(self): + Item = self.classes.Item + Keyword = self.classes.Keyword + self._assert_eager_not_found_exception( + [Keyword.id, Item.id], + (eagerload_all("keywords"), ), + r"Query has only expression-based entities - can't find property " + "named 'keywords'." + ) + + def test_option_against_wrong_multi_entity_type_attr_one(self): + Item = self.classes.Item + Keyword = self.classes.Keyword + self._assert_eager_not_found_exception( + [Keyword, Item], + (eagerload_all(Keyword.id, Item.keywords), ), + r"Attribute 'Keyword.id' of entity 'Mapper\|Keyword\|keywords' " + "does not refer to a mapped entity" + ) + + def test_option_against_wrong_multi_entity_type_attr_two(self): + Item = self.classes.Item + Keyword = self.classes.Keyword + self._assert_eager_not_found_exception( + [Keyword, Item], + (eagerload_all(Keyword.keywords, Item.keywords), ), + r"Attribute 'Keyword.keywords' of entity 'Mapper\|Keyword\|keywords' " + "does not refer to a mapped entity" + ) + + def test_option_against_wrong_multi_entity_type_attr_three(self): + Item = self.classes.Item + Keyword = self.classes.Keyword + self._assert_eager_not_found_exception( + [Keyword.id, Item.id], + (eagerload_all(Keyword.keywords, Item.keywords), ), + r"Query has only expression-based entities - " + "can't find property named 'keywords'." + ) + + def test_wrong_type_in_option(self): + Item = self.classes.Item + Keyword = self.classes.Keyword + self._assert_eager_not_found_exception( + [Item], + (eagerload_all(Keyword), ), + r"mapper option expects string key or list of attributes" + ) @classmethod def setup_mappers(cls): @@ -2174,8 +2348,9 @@ class OptionsNoPropTest(_fixtures.FixtureTest): cls.tables.item_keywords, cls.classes.Keyword, cls.classes.Item) - - mapper(Keyword, keywords) + mapper(Keyword, keywords, properties={ + "keywords":column_property(keywords.c.name + "some keyword") + }) mapper(Item, items, properties=dict(keywords=relationship(Keyword, secondary=item_keywords))) @@ -2188,6 +2363,13 @@ class OptionsNoPropTest(_fixtures.FixtureTest): key = ('loaderstrategy', (class_mapper(Item), 'keywords')) assert key in q._attributes + def _assert_eager_not_found_exception(self, entity_list, options, + message): + assert_raises_message(sa.exc.ArgumentError, + message, + create_session().query(*entity_list).options, + *options) + def _assert_eager_with_just_column_exception(self, column, eager_option, message): assert_raises_message(sa.exc.ArgumentError, message, -- 2.39.5