From e4bcd9bb13abc695c21ff3c1fc8b927034a61456 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Malthe=20J=C3=B8rgensen?= Date: Mon, 12 Dec 2016 10:44:37 +0100 Subject: [PATCH 1/5] [Dev] _import_class: Add support for BaseDocument --- mongoengine/common.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/mongoengine/common.py b/mongoengine/common.py index 3e63e98ee..bde7e78c4 100644 --- a/mongoengine/common.py +++ b/mongoengine/common.py @@ -34,7 +34,10 @@ class from the :data:`mongoengine.common._class_registry_cache`. queryset_classes = ('OperationError',) deref_classes = ('DeReference',) - if cls_name in doc_classes: + if cls_name == 'BaseDocument': + from mongoengine.base import document as module + import_classes = ['BaseDocument'] + elif cls_name in doc_classes: from mongoengine import document as module import_classes = doc_classes elif cls_name in field_classes: From 0a5ef2e6e2ee8910cb3cc2a7426e5f3ea67e8f0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Malthe=20J=C3=B8rgensen?= Date: Thu, 18 Feb 2016 16:36:30 +0100 Subject: [PATCH 2/5] Raise TypeError when __in-query used with Document Doing a query like `BlogPost.objects(authors__in=author).count()`, where `author` is a single Document, will now raise a TypeError explaining that you cannot use a single `Document` with the `in`-operator. i.e. `BlogPost.objects(authors__in=[author]).count()` Similarly using the `in`-operator with a non-iterable will also raise a TypeError explaining that you need to use an iterable with the `in`-operator. The same applies to the `nin`, `all`, and `near` operators. Note: The added code uses `_import_class('BaseDocument')` to break any circular dependency. --- mongoengine/queryset/transform.py | 17 ++++++++-- tests/queryset/queryset.py | 54 +++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 2 deletions(-) diff --git a/mongoengine/queryset/transform.py b/mongoengine/queryset/transform.py index af59917cf..2d2d10f65 100644 --- a/mongoengine/queryset/transform.py +++ b/mongoengine/queryset/transform.py @@ -101,8 +101,21 @@ def query(_doc_cls=None, **kwargs): value = value['_id'] elif op in ('in', 'nin', 'all', 'near') and not isinstance(value, dict): - # 'in', 'nin' and 'all' require a list of values - value = [field.prepare_query_value(op, v) for v in value] + # Raise an error if the in/nin/all/near param is not iterable. We need a + # special check for BaseDocument, because - although it's iterable - using + # it as such in the context of this method is most definitely a mistake. + BaseDocument = _import_class('BaseDocument') + if isinstance(value, BaseDocument): + raise TypeError('When using the `in`, `nin`, `all`, or ' \ + '`near`-operators you can\'t use a ' \ + '`Document`, you must wrap your object ' \ + 'in a list (object -> [object]).') + elif not hasattr(value, '__iter__'): + raise TypeError('The `in`, `nin`, `all`, or ' \ + '`near`-operators must be applied to an ' \ + 'iterable (e.g. a list).') + else: + value = [field.prepare_query_value(op, v) for v in value] # If we're querying a GenericReferenceField, we need to alter the # key depending on the value: diff --git a/tests/queryset/queryset.py b/tests/queryset/queryset.py index e4c71de76..f8078b4a7 100644 --- a/tests/queryset/queryset.py +++ b/tests/queryset/queryset.py @@ -4963,6 +4963,60 @@ class Data(Document): self.assertEqual(i, 249) self.assertEqual(j, 249) + def test_in_operator_on_non_iterable(self): + """Ensure that using the `__in` operator on a non-iterable raises an + error. + """ + class User(Document): + name = StringField() + + class BlogPost(Document): + content = StringField() + authors = ListField(ReferenceField(User)) + + User.drop_collection() + BlogPost.drop_collection() + + author = User(name='Test User') + author.save() + post = BlogPost(content='Had a good coffee today...', authors=[author]) + post.save() + + blog_posts = BlogPost.objects(authors__in=[author]) + self.assertEqual(list(blog_posts), [post]) + + with self.assertRaises(TypeError): + # Using the `__in`-operator with a non-iterable should raise a + # TypeError + BlogPost.objects(authors__in=author.id).count() + + def test_in_operator_on_document(self): + """Ensure that using the `__in` operator on a `Document` raises an + error. + """ + class User(Document): + name = StringField() + + class BlogPost(Document): + content = StringField() + authors = ListField(ReferenceField(User)) + + User.drop_collection() + BlogPost.drop_collection() + + author = User(name='Test User') + author.save() + post = BlogPost(content='Had a good coffee today...', authors=[author]) + post.save() + + blog_posts = BlogPost.objects(authors__in=[author]) + self.assertEqual(list(blog_posts), [post]) + + with self.assertRaises(TypeError): + # Using the `__in`-operator with a `Document` should raise a + # TypeError + BlogPost.objects(authors__in=author).count() + if __name__ == '__main__': unittest.main() From 4c00f0bda9781b17fd8019649c76cb3fdeba61bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Malthe=20J=C3=B8rgensen?= Date: Tue, 6 Dec 2016 14:19:32 +0100 Subject: [PATCH 3/5] queryset/transform.py: flake8 compliance --- mongoengine/queryset/transform.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/mongoengine/queryset/transform.py b/mongoengine/queryset/transform.py index 2d2d10f65..61d434904 100644 --- a/mongoengine/queryset/transform.py +++ b/mongoengine/queryset/transform.py @@ -106,14 +106,14 @@ def query(_doc_cls=None, **kwargs): # it as such in the context of this method is most definitely a mistake. BaseDocument = _import_class('BaseDocument') if isinstance(value, BaseDocument): - raise TypeError('When using the `in`, `nin`, `all`, or ' \ - '`near`-operators you can\'t use a ' \ - '`Document`, you must wrap your object ' \ - 'in a list (object -> [object]).') + raise TypeError("When using the `in`, `nin`, `all`, or " + "`near`-operators you can\'t use a " + "`Document`, you must wrap your object " + "in a list (object -> [object]).") elif not hasattr(value, '__iter__'): - raise TypeError('The `in`, `nin`, `all`, or ' \ - '`near`-operators must be applied to an ' \ - 'iterable (e.g. a list).') + raise TypeError("The `in`, `nin`, `all`, or " + "`near`-operators must be applied to an " + "iterable (e.g. a list).") else: value = [field.prepare_query_value(op, v) for v in value] From b5eb453689cf122bb2d15aa9e2e9d7bf6b1535c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Malthe=20J=C3=B8rgensen?= Date: Tue, 6 Dec 2016 14:40:14 +0100 Subject: [PATCH 4/5] tests/queryset/queryset.py: Python 2.6 compat --- tests/queryset/queryset.py | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/tests/queryset/queryset.py b/tests/queryset/queryset.py index f8078b4a7..28b831cd8 100644 --- a/tests/queryset/queryset.py +++ b/tests/queryset/queryset.py @@ -4985,10 +4985,8 @@ class BlogPost(Document): blog_posts = BlogPost.objects(authors__in=[author]) self.assertEqual(list(blog_posts), [post]) - with self.assertRaises(TypeError): - # Using the `__in`-operator with a non-iterable should raise a - # TypeError - BlogPost.objects(authors__in=author.id).count() + # Using the `__in`-operator with a non-iterable should raise a TypeError + self.assertRaises(TypeError, BlogPost.objects(authors__in=author.id).count) def test_in_operator_on_document(self): """Ensure that using the `__in` operator on a `Document` raises an @@ -5012,10 +5010,8 @@ class BlogPost(Document): blog_posts = BlogPost.objects(authors__in=[author]) self.assertEqual(list(blog_posts), [post]) - with self.assertRaises(TypeError): - # Using the `__in`-operator with a `Document` should raise a - # TypeError - BlogPost.objects(authors__in=author).count() + # Using the `__in`-operator with a `Document` should raise a TypeError + self.assertRaises(TypeError, BlogPost.objects(authors__in=author).count) if __name__ == '__main__': From de8d65d09b4bfe47feeebbc62811658f8d873f99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Malthe=20J=C3=B8rgensen?= Date: Mon, 12 Dec 2016 10:43:18 +0100 Subject: [PATCH 5/5] [Dev] flake8: Raise max_complexity to 47 `queryset/transform.py::query()` has complexity 47. --- setup.cfg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.cfg b/setup.cfg index 1887c4768..eabe32719 100644 --- a/setup.cfg +++ b/setup.cfg @@ -7,5 +7,5 @@ cover-package=mongoengine [flake8] ignore=E501,F401,F403,F405,I201 exclude=build,dist,docs,venv,venv3,.tox,.eggs,tests -max-complexity=45 +max-complexity=47 application-import-names=mongoengine,tests