From 3faf3c84be55f709d828ea7837db988c56b0239e Mon Sep 17 00:00:00 2001 From: Jonathan Prates Date: Tue, 27 May 2014 16:33:38 -0300 Subject: [PATCH 01/14] Avoid to open all documents from cursors in an if stmt Using a cursos in an if statement: cursor = Collection.objects if cursor: (...) Will open all documents, because there are not an __nonzero__ method. This change check only one document (if present) and returns True or False. --- mongoengine/queryset/base.py | 9 +++++++++ tests/queryset/queryset.py | 23 +++++++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/mongoengine/queryset/base.py b/mongoengine/queryset/base.py index c2ad027e..823bc164 100644 --- a/mongoengine/queryset/base.py +++ b/mongoengine/queryset/base.py @@ -154,6 +154,15 @@ class BaseQuerySet(object): def __iter__(self): raise NotImplementedError + def __nonzero__(self): + """ Avoid to open all records in an if stmt """ + + for value in self: + self.rewind() + return True + + return False + # Core functions def all(self): diff --git a/tests/queryset/queryset.py b/tests/queryset/queryset.py index 7ff2965d..f274e0ee 100644 --- a/tests/queryset/queryset.py +++ b/tests/queryset/queryset.py @@ -3814,6 +3814,29 @@ class QuerySetTest(unittest.TestCase): self.assertEqual(Example.objects(size=instance_size).count(), 1) self.assertEqual(Example.objects(size__in=[instance_size]).count(), 1) + def test_cursor_in_an_if_stmt(self): + + class Test(Document): + test_field = StringField() + + Test.drop_collection() + queryset = Test.objects + + if queryset: + raise AssertionError('Empty cursor returns True') + + test = Test() + test.test_field = 'test' + test.save() + + queryset = Test.objects + if not test: + raise AssertionError('There is data, but cursor returned False') + + queryset.next() + if queryset: + raise AssertionError('There is no data left in cursor') + if __name__ == '__main__': unittest.main() From 9ba657797ed64d50a876c95bc2a243bed3037e19 Mon Sep 17 00:00:00 2001 From: Jonathan Prates Date: Wed, 28 May 2014 08:33:22 -0300 Subject: [PATCH 02/14] Authors updated according guideline --- AUTHORS | 1 + 1 file changed, 1 insertion(+) diff --git a/AUTHORS b/AUTHORS index d6994d50..5d6a45fa 100644 --- a/AUTHORS +++ b/AUTHORS @@ -189,3 +189,4 @@ that much better: * Tom (https://github.com/tomprimozic) * j0hnsmith (https://github.com/j0hnsmith) * Damien Churchill (https://github.com/damoxc) + * Jonathan Simon Prates (https://github.com/jonathansp) \ No newline at end of file From 47f0de9836a7e340047c6639dee9ade29b9755cb Mon Sep 17 00:00:00 2001 From: Jonathan Prates Date: Wed, 28 May 2014 08:36:57 -0300 Subject: [PATCH 03/14] Py3 fix --- mongoengine/queryset/base.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/mongoengine/queryset/base.py b/mongoengine/queryset/base.py index 823bc164..022b7c1f 100644 --- a/mongoengine/queryset/base.py +++ b/mongoengine/queryset/base.py @@ -161,7 +161,12 @@ class BaseQuerySet(object): self.rewind() return True - return False + return False + + def __bool__(self): + """ Same behaviour in Py3 """ + + return self.__nonzero__(): # Core functions From edcdfeb0573f253227a196b3080344e849f48109 Mon Sep 17 00:00:00 2001 From: Jonathan Prates Date: Wed, 28 May 2014 09:03:12 -0300 Subject: [PATCH 04/14] Fix syntax error --- mongoengine/queryset/base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mongoengine/queryset/base.py b/mongoengine/queryset/base.py index 022b7c1f..0dfff7cc 100644 --- a/mongoengine/queryset/base.py +++ b/mongoengine/queryset/base.py @@ -166,7 +166,7 @@ class BaseQuerySet(object): def __bool__(self): """ Same behaviour in Py3 """ - return self.__nonzero__(): + return self.__nonzero__() # Core functions From dfdecef8e72fdd0379cdec4452f3175f0826b892 Mon Sep 17 00:00:00 2001 From: Jonathan Prates Date: Wed, 28 May 2014 09:40:22 -0300 Subject: [PATCH 05/14] Fix py2 and py3 --- mongoengine/queryset/base.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/mongoengine/queryset/base.py b/mongoengine/queryset/base.py index 0dfff7cc..09fb5bf6 100644 --- a/mongoengine/queryset/base.py +++ b/mongoengine/queryset/base.py @@ -154,7 +154,7 @@ class BaseQuerySet(object): def __iter__(self): raise NotImplementedError - def __nonzero__(self): + def _bool(self): """ Avoid to open all records in an if stmt """ for value in self: @@ -163,10 +163,15 @@ class BaseQuerySet(object): return False + def __nonzero__(self): + """ Same behaviour in Py2 """ + + return self._bool() + def __bool__(self): """ Same behaviour in Py3 """ - return self.__nonzero__() + return self._bool() # Core functions From ee0c7fd8bfa08ec885f379b2aa6dd72f48d8d7c5 Mon Sep 17 00:00:00 2001 From: Jonathan Prates Date: Wed, 28 May 2014 13:21:00 -0300 Subject: [PATCH 06/14] Change for loop to self.first() --- mongoengine/queryset/base.py | 6 +----- tests/queryset/queryset.py | 4 ++-- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/mongoengine/queryset/base.py b/mongoengine/queryset/base.py index 09fb5bf6..099831fe 100644 --- a/mongoengine/queryset/base.py +++ b/mongoengine/queryset/base.py @@ -157,11 +157,7 @@ class BaseQuerySet(object): def _bool(self): """ Avoid to open all records in an if stmt """ - for value in self: - self.rewind() - return True - - return False + return False if self.first() is None else True def __nonzero__(self): """ Same behaviour in Py2 """ diff --git a/tests/queryset/queryset.py b/tests/queryset/queryset.py index f274e0ee..f6adaf39 100644 --- a/tests/queryset/queryset.py +++ b/tests/queryset/queryset.py @@ -3834,8 +3834,8 @@ class QuerySetTest(unittest.TestCase): raise AssertionError('There is data, but cursor returned False') queryset.next() - if queryset: - raise AssertionError('There is no data left in cursor') + if not queryset: + raise AssertionError('There is data, cursor must return True') if __name__ == '__main__': From 30964f65e465f7e29960cd49caf29d5c9d4ac756 Mon Sep 17 00:00:00 2001 From: Jonathan Prates Date: Wed, 28 May 2014 17:06:15 -0300 Subject: [PATCH 07/14] Remove orderby in if stmt --- mongoengine/queryset/base.py | 8 +++++- tests/queryset/queryset.py | 53 ++++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 1 deletion(-) diff --git a/mongoengine/queryset/base.py b/mongoengine/queryset/base.py index 099831fe..fe1285c6 100644 --- a/mongoengine/queryset/base.py +++ b/mongoengine/queryset/base.py @@ -157,7 +157,13 @@ class BaseQuerySet(object): def _bool(self): """ Avoid to open all records in an if stmt """ - return False if self.first() is None else True + queryset = self.clone() + queryset._ordering = [] + try: + queryset[0] + return True + except IndexError: + return False def __nonzero__(self): """ Same behaviour in Py2 """ diff --git a/tests/queryset/queryset.py b/tests/queryset/queryset.py index f6adaf39..65f7255b 100644 --- a/tests/queryset/queryset.py +++ b/tests/queryset/queryset.py @@ -3837,6 +3837,59 @@ class QuerySetTest(unittest.TestCase): if not queryset: raise AssertionError('There is data, cursor must return True') + def test_bool_performance(self): + + class Person(Document): + name = StringField() + + Person.drop_collection() + for i in xrange(100): + Person(name="No: %s" % i).save() + + with query_counter() as q: + if Person.objects: + pass + + self.assertEqual(q, 1) + op = q.db.system.profile.find({"ns": + {"$ne": "%s.system.indexes" % q.db.name}})[0] + + self.assertEqual(op['nreturned'], 1) + + + def test_bool_with_ordering(self): + + class Person(Document): + name = StringField() + + Person.drop_collection() + Person(name="Test").save() + + qs = Person.objects.order_by('name') + + with query_counter() as q: + + if qs: + pass + + op = q.db.system.profile.find({"ns": + {"$ne": "%s.system.indexes" % q.db.name}})[0] + + self.assertFalse('$orderby' in op['query'], + 'BaseQuerySet cannot use orderby in if stmt') + + with query_counter() as p: + + for x in qs: + pass + + op = p.db.system.profile.find({"ns": + {"$ne": "%s.system.indexes" % q.db.name}})[0] + + + self.assertTrue('$orderby' in op['query'], + 'BaseQuerySet cannot remove orderby in for loop') + if __name__ == '__main__': unittest.main() From 39735594bd935f3003d85009c3dc14a8d3d71f23 Mon Sep 17 00:00:00 2001 From: Jonathan Prates Date: Wed, 28 May 2014 17:15:48 -0300 Subject: [PATCH 08/14] Removed blank line --- tests/queryset/queryset.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/queryset/queryset.py b/tests/queryset/queryset.py index 65f7255b..c5fea003 100644 --- a/tests/queryset/queryset.py +++ b/tests/queryset/queryset.py @@ -3886,7 +3886,6 @@ class QuerySetTest(unittest.TestCase): op = p.db.system.profile.find({"ns": {"$ne": "%s.system.indexes" % q.db.name}})[0] - self.assertTrue('$orderby' in op['query'], 'BaseQuerySet cannot remove orderby in for loop') From c87801f0a911bd02f4b4ae910751c8b04717fd02 Mon Sep 17 00:00:00 2001 From: Jonathan Prates Date: Wed, 28 May 2014 17:26:28 -0300 Subject: [PATCH 09/14] Using first() from cloned queryset --- mongoengine/queryset/base.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/mongoengine/queryset/base.py b/mongoengine/queryset/base.py index fe1285c6..85d2dc3f 100644 --- a/mongoengine/queryset/base.py +++ b/mongoengine/queryset/base.py @@ -159,11 +159,7 @@ class BaseQuerySet(object): queryset = self.clone() queryset._ordering = [] - try: - queryset[0] - return True - except IndexError: - return False + return False if queryset.first() is None else True def __nonzero__(self): """ Same behaviour in Py2 """ From c744104a18829f7f34d491a580914b89ecf3c620 Mon Sep 17 00:00:00 2001 From: Jonathan Prates Date: Thu, 29 May 2014 10:53:20 -0300 Subject: [PATCH 10/14] Added test with meta --- tests/queryset/queryset.py | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tests/queryset/queryset.py b/tests/queryset/queryset.py index c5fea003..bad0d1e5 100644 --- a/tests/queryset/queryset.py +++ b/tests/queryset/queryset.py @@ -3889,6 +3889,33 @@ class QuerySetTest(unittest.TestCase): self.assertTrue('$orderby' in op['query'], 'BaseQuerySet cannot remove orderby in for loop') + def test_bool_with_ordering_from_meta_dict(self): + + class Person(Document): + name = StringField() + meta = { + 'ordering': ['name'] + } + + Person.drop_collection() + + Person(name="B").save() + Person(name="C").save() + Person(name="A").save() + + with query_counter() as q: + + if Person.objects: + pass + + op = q.db.system.profile.find({"ns": + {"$ne": "%s.system.indexes" % q.db.name}})[0] + + self.assertTrue('$orderby' in op['query'], + 'BaseQuerySet cannot remove orderby from meta in boolen test') + + self.assertEqual(Person.objects.first().name, 'A') + if __name__ == '__main__': unittest.main() From 819ff2a90286030740e2d6d886c2f14e29d0ccc2 Mon Sep 17 00:00:00 2001 From: Jonathan Prates Date: Thu, 29 May 2014 14:36:30 -0300 Subject: [PATCH 11/14] Renamed to has_data() --- AUTHORS | 3 ++- mongoengine/queryset/base.py | 12 ++++++------ tests/queryset/queryset.py | 8 ++++++-- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/AUTHORS b/AUTHORS index 5d6a45fa..7b466d6b 100644 --- a/AUTHORS +++ b/AUTHORS @@ -189,4 +189,5 @@ that much better: * Tom (https://github.com/tomprimozic) * j0hnsmith (https://github.com/j0hnsmith) * Damien Churchill (https://github.com/damoxc) - * Jonathan Simon Prates (https://github.com/jonathansp) \ No newline at end of file + * Jonathan Simon Prates (https://github.com/jonathansp) + * Thiago Papageorgiou (https://github.com/tmpapageorgiou) \ No newline at end of file diff --git a/mongoengine/queryset/base.py b/mongoengine/queryset/base.py index 85d2dc3f..ef8cd2a7 100644 --- a/mongoengine/queryset/base.py +++ b/mongoengine/queryset/base.py @@ -154,22 +154,22 @@ class BaseQuerySet(object): def __iter__(self): raise NotImplementedError - def _bool(self): - """ Avoid to open all records in an if stmt """ + def has_data(self): + """ Retrieves whether cursor has any data. """ queryset = self.clone() queryset._ordering = [] return False if queryset.first() is None else True def __nonzero__(self): - """ Same behaviour in Py2 """ + """ Avoid to open all records in an if stmt in Py2. """ - return self._bool() + return self.has_data() def __bool__(self): - """ Same behaviour in Py3 """ + """ Avoid to open all records in an if stmt in Py3. """ - return self._bool() + return self.has_data() # Core functions diff --git a/tests/queryset/queryset.py b/tests/queryset/queryset.py index bad0d1e5..fe932367 100644 --- a/tests/queryset/queryset.py +++ b/tests/queryset/queryset.py @@ -3831,11 +3831,15 @@ class QuerySetTest(unittest.TestCase): queryset = Test.objects if not test: - raise AssertionError('There is data, but cursor returned False') + raise AssertionError('Cursor has data and returned False') queryset.next() if not queryset: - raise AssertionError('There is data, cursor must return True') + raise AssertionError('Cursor has data and it must returns False,' + ' even in the last item.') + + self.assertTrue(queryset.has_data(), 'Cursor has data and ' + 'returned False') def test_bool_performance(self): From 85187239b629772ba276aaee54eb678c64ad8207 Mon Sep 17 00:00:00 2001 From: Jonathan Prates Date: Thu, 29 May 2014 15:21:24 -0300 Subject: [PATCH 12/14] Fix tests msg --- tests/queryset/queryset.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/queryset/queryset.py b/tests/queryset/queryset.py index fe932367..f68468ff 100644 --- a/tests/queryset/queryset.py +++ b/tests/queryset/queryset.py @@ -3835,7 +3835,7 @@ class QuerySetTest(unittest.TestCase): queryset.next() if not queryset: - raise AssertionError('Cursor has data and it must returns False,' + raise AssertionError('Cursor has data and it must returns True,' ' even in the last item.') self.assertTrue(queryset.has_data(), 'Cursor has data and ' From 1eacc6fbff0bbe14a18d62032fe0b616194c8d26 Mon Sep 17 00:00:00 2001 From: Stefan Wojcik Date: Fri, 30 May 2014 15:08:03 -0700 Subject: [PATCH 13/14] clear ordering via empty order_by --- mongoengine/queryset/base.py | 7 ++-- tests/queryset/queryset.py | 70 ++++++++++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 3 deletions(-) diff --git a/mongoengine/queryset/base.py b/mongoengine/queryset/base.py index c2ad027e..4f451667 100644 --- a/mongoengine/queryset/base.py +++ b/mongoengine/queryset/base.py @@ -50,7 +50,7 @@ class BaseQuerySet(object): self._initial_query = {} self._where_clause = None self._loaded_fields = QueryFieldList() - self._ordering = [] + self._ordering = None self._snapshot = False self._timeout = True self._class_check = True @@ -1189,8 +1189,9 @@ class BaseQuerySet(object): if self._ordering: # Apply query ordering self._cursor_obj.sort(self._ordering) - elif self._document._meta['ordering']: - # Otherwise, apply the ordering from the document model + elif self._ordering is None and self._document._meta['ordering']: + # Otherwise, apply the ordering from the document model, unless + # it's been explicitly cleared via order_by with no arguments order = self._get_order_by(self._document._meta['ordering']) self._cursor_obj.sort(order) diff --git a/tests/queryset/queryset.py b/tests/queryset/queryset.py index 7ff2965d..d706eda8 100644 --- a/tests/queryset/queryset.py +++ b/tests/queryset/queryset.py @@ -1040,6 +1040,76 @@ class QuerySetTest(unittest.TestCase): expected = [blog_post_1, blog_post_2, blog_post_3] self.assertSequence(qs, expected) + def test_clear_ordering(self): + """ Make sure one can clear the query set ordering by applying a + consecutive order_by() + """ + + class Person(Document): + name = StringField() + + Person.drop_collection() + Person(name="A").save() + Person(name="B").save() + + qs = Person.objects.order_by('-name') + + # Make sure we can clear a previously specified ordering + with query_counter() as q: + lst = list(qs.order_by()) + + op = q.db.system.profile.find({"ns": + {"$ne": "%s.system.indexes" % q.db.name}})[0] + + self.assertTrue('$orderby' not in op['query']) + self.assertEqual(lst[0].name, 'A') + + # Make sure previously specified ordering is preserved during + # consecutive calls to the same query set + with query_counter() as q: + lst = list(qs) + + op = q.db.system.profile.find({"ns": + {"$ne": "%s.system.indexes" % q.db.name}})[0] + + self.assertTrue('$orderby' in op['query']) + self.assertEqual(lst[0].name, 'B') + + def test_clear_default_ordering(self): + + class Person(Document): + name = StringField() + meta = { + 'ordering': ['-name'] + } + + Person.drop_collection() + Person(name="A").save() + Person(name="B").save() + + qs = Person.objects + + # Make sure clearing default ordering works + with query_counter() as q: + lst = list(qs.order_by()) + + op = q.db.system.profile.find({"ns": + {"$ne": "%s.system.indexes" % q.db.name}})[0] + + self.assertTrue('$orderby' not in op['query']) + self.assertEqual(lst[0].name, 'A') + + # Make sure default ordering is preserved during consecutive calls + # to the same query set + with query_counter() as q: + lst = list(qs) + + op = q.db.system.profile.find({"ns": + {"$ne": "%s.system.indexes" % q.db.name}})[0] + + self.assertTrue('$orderby' in op['query']) + self.assertEqual(lst[0].name, 'B') + def test_find_embedded(self): """Ensure that an embedded document is properly returned from a query. """ From 7bb2fe128a4b8c8b08f4d800f15229e2814bfac8 Mon Sep 17 00:00:00 2001 From: Jonathan Prates Date: Thu, 12 Jun 2014 11:08:41 -0300 Subject: [PATCH 14/14] Added PR #657 --- mongoengine/queryset/base.py | 11 +++++------ tests/queryset/queryset.py | 9 ++++----- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/mongoengine/queryset/base.py b/mongoengine/queryset/base.py index 94a6e4b5..cb48f6ca 100644 --- a/mongoengine/queryset/base.py +++ b/mongoengine/queryset/base.py @@ -154,22 +154,21 @@ class BaseQuerySet(object): def __iter__(self): raise NotImplementedError - def has_data(self): + def _has_data(self): """ Retrieves whether cursor has any data. """ - queryset = self.clone() - queryset._ordering = [] + queryset = self.order_by() return False if queryset.first() is None else True def __nonzero__(self): """ Avoid to open all records in an if stmt in Py2. """ - return self.has_data() + return self._has_data() def __bool__(self): """ Avoid to open all records in an if stmt in Py3. """ - return self.has_data() + return self._has_data() # Core functions @@ -1410,7 +1409,7 @@ class BaseQuerySet(object): pass key_list.append((key, direction)) - if self._cursor_obj: + if self._cursor_obj and key_list: self._cursor_obj.sort(key_list) return key_list diff --git a/tests/queryset/queryset.py b/tests/queryset/queryset.py index 38499df4..a2438e21 100644 --- a/tests/queryset/queryset.py +++ b/tests/queryset/queryset.py @@ -3908,9 +3908,6 @@ class QuerySetTest(unittest.TestCase): raise AssertionError('Cursor has data and it must returns True,' ' even in the last item.') - self.assertTrue(queryset.has_data(), 'Cursor has data and ' - 'returned False') - def test_bool_performance(self): class Person(Document): @@ -3985,10 +3982,12 @@ class QuerySetTest(unittest.TestCase): op = q.db.system.profile.find({"ns": {"$ne": "%s.system.indexes" % q.db.name}})[0] - self.assertTrue('$orderby' in op['query'], - 'BaseQuerySet cannot remove orderby from meta in boolen test') + self.assertFalse('$orderby' in op['query'], + 'BaseQuerySet must remove orderby from meta in boolen test') self.assertEqual(Person.objects.first().name, 'A') + self.assertTrue(Person.objects._has_data(), + 'Cursor has data and returned False') if __name__ == '__main__':