- 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.

diff --git a/CHANGES b/CHANGES
index 29fdb757e26a08d673bbb7871a1db287d7309f96..a781307b429deb88d8f07009c2985d65c3b85941 100644 (file)
--- 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 a20554dc0da93fbba3a3796e3dc908cffa9fe00f..93f3cf04c443ece19a8562924a6087ed42321cb3 100644 (file)
--- 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 fec9d45fc700c685c5ad68854dca70cb00d1811c..7a87485566364f7c7d885d71b2cdc0f6e5f1b494 100644 (file)
--- 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 838bbcde063561b05cd333a16ac8c33d5b3ab9d1..93f349ff29060f203a4ba1372553a9e5bb836487 100644 (file)
--- 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,