From b1358d5e7b090664510b9122ba932eaaa72ba596 Mon Sep 17 00:00:00 2001 From: Ilya Skriblovsky Date: Fri, 14 Oct 2016 10:09:22 +0300 Subject: [PATCH 1/9] New signature for find() method and compatibility with old one --- tests/test_collection.py | 68 +++++++++++++++++++++++++++++++++++ txmongo/collection.py | 78 +++++++++++++++++++++++++++++++--------- 2 files changed, 129 insertions(+), 17 deletions(-) diff --git a/tests/test_collection.py b/tests/test_collection.py index 66864d0..f3ae96e 100644 --- a/tests/test_collection.py +++ b/tests/test_collection.py @@ -369,3 +369,71 @@ def test_Fail(self): else: self.fail() test_Fail.timeout = 10 + + +class TestFindSignatureCompat(unittest.TestCase): + def test_convert(self): + self.assertEqual( + Collection._find_args_compat({'x': 42}), + {"spec": {'x': 42}, "projection": None, "skip": 0, "limit": 0, "filter": None} + ) + self.assertEqual( + Collection._find_args_compat({'x': 42}, unknown_arg=123), + {"spec": {'x': 42}, "projection": None, "skip": 0, "limit": 0, "filter": None, + "unknown_arg": 123} + ) + self.assertEqual( + Collection._find_args_compat(spec={'x': 42}), + {"spec": {'x': 42}, "projection": None, "skip": 0, "limit": 0, "filter": None} + ) + self.assertEqual( + Collection._find_args_compat({'x': 42}, {'a': 1}), + {"spec": {'x': 42}, "projection": {'a': 1}, "skip": 0, "limit": 0, "filter": None} + ) + self.assertEqual( + Collection._find_args_compat({'x': 42}, projection={'a': 1}), + {"spec": {'x': 42}, "projection": {'a': 1}, "skip": 0, "limit": 0, "filter": None} + ) + self.assertEqual( + Collection._find_args_compat({'x': 42}, fields={'a': 1}), + {"spec": {'x': 42}, "projection": {'a': 1}, "skip": 0, "limit": 0, "filter": None, + "cursor": False} + ) + self.assertEqual( + Collection._find_args_compat({'x': 42}, 5), + {"spec": {'x': 42}, "projection": None, "skip": 5, "limit": 0, "filter": None, + "cursor": False} + ) + self.assertEqual( + Collection._find_args_compat({'x': 42}, {'a': 1}, 5), + {"spec": {'x': 42}, "projection": {'a': 1}, "skip": 5, "limit": 0, "filter": None} + ) + self.assertEqual( + Collection._find_args_compat({'x': 42}, {'a': 1}, 5, 6), + {"spec": {'x': 42}, "projection": {'a': 1}, "skip": 5, "limit": 6, "filter": None} + ) + self.assertEqual( + Collection._find_args_compat({'x': 42}, 5, 6, {'a': 1}), + {"spec": {'x': 42}, "projection": {'a': 1}, "skip": 5, "limit": 6, "filter": None, + "cursor": False} + ) + self.assertEqual( + Collection._find_args_compat({'x': 42}, {'a': 1}, 5, 6, qf.sort([('s', 1)])), + {"spec": {'x': 42}, "projection": {'a': 1}, "skip": 5, "limit": 6, + "filter": qf.sort([('s', 1)])} + ) + self.assertEqual( + Collection._find_args_compat({'x': 42}, 5, 6, {'a': 1}, qf.sort([('s', 1)])), + {"spec": {'x': 42}, "projection": {'a': 1}, "skip": 5, "limit": 6, + "filter": qf.sort([('s', 1)]), "cursor": False} + ) + self.assertEqual( + Collection._find_args_compat({'x': 42}, 5, 6, {'a': 1}, qf.sort([('s', 1)]), True), + {"spec": {'x': 42}, "projection": {'a': 1}, "skip": 5, "limit": 6, + "filter": qf.sort([('s', 1)]), "cursor": True} + ) + self.assertEqual( + Collection._find_args_compat({'x': 42}, filter=qf.sort([('s', 1)]), limit=6, projection={'a': 1}, skip=5), + {"spec": {'x': 42}, "projection": {'a': 1}, "skip": 5, "limit": 6, + "filter": qf.sort([('s', 1)])} + ) diff --git a/txmongo/collection.py b/txmongo/collection.py index 1ad8819..14156a6 100644 --- a/txmongo/collection.py +++ b/txmongo/collection.py @@ -6,6 +6,7 @@ import io import struct import collections +import warnings from bson import BSON, ObjectId from bson.code import Code from bson.son import SON @@ -219,8 +220,34 @@ def on_ok(result): .addErrback(on_3_0_fail)\ .addCallbacks(on_ok) + @staticmethod + def _find_args_compat(*args, **kwargs): + """ + signature of find() was changed from + (spec=None, skip=0, limit=0, fields=None, filter=None, cursor=False, **kwargs) + to + (spec=None, projection=None, skip=0, limit=0, filter=None, **kwargs) + + This function makes it compatible with both + """ + def old(spec=None, skip=0, limit=0, fields=None, filter=None, cursor=False, **kwargs): + warnings.warn("find(), find_with_cursor() and find_one() signatures have" + "changed. Please refer to documentation.", DeprecationWarning) + return new(spec, fields, skip, limit, filter, cursor=cursor, **kwargs) + + def new(spec=None, projection=None, skip=0, limit=0, filter=None, **kwargs): + args = {"spec": spec, "projection": projection, "skip": skip, "limit": limit, + "filter": filter} + args.update(kwargs) + return args + + if len(args) >= 2 and isinstance(args[1], int) or "fields" in kwargs: + return old(*args, **kwargs) + else: + return new(*args, **kwargs) + @timeout - def find(self, spec=None, skip=0, limit=0, fields=None, filter=None, cursor=False, **kwargs): + def find(self, *args, **kwargs): """Find documents in a collection. The `spec` argument is a MongoDB query document. To return all documents @@ -232,6 +259,13 @@ def find(self, spec=None, skip=0, limit=0, fields=None, filter=None, cursor=Fals Ordering, indexing hints and other query parameters can be set with `filter` argument. See :mod:`txmongo.filter` for details. + :param projection: + a list of field names that should be returned for each document + in the result set or a dict specifying field names to include or + exclude. If `projection` is a list ``_id`` fields will always be + returned. Use a dict form to exclude fields: + ``projection={"_id": False}``. + :param skip: the number of documents to omit from the start of the result set. @@ -239,13 +273,6 @@ def find(self, spec=None, skip=0, limit=0, fields=None, filter=None, cursor=Fals the maximum number of documents to return. All documents are returned when `limit` is zero. - :param fields: - a list of field names that should be returned for each document - in the result set or a dict specifying field names to include or - exclude. If `fields` is a list ``_id`` fields will always be - returned. Use a dict form to exclude fields: - ``fields={"_id": False}``. - :param as_class: *(keyword only)* if not ``None``, returned documents will be converted to type specified. For example, you can use ``as_class=collections.OrderedDict`` @@ -268,12 +295,20 @@ def query(): do_something(doc) docs, dfr = yield dfr """ + new_kwargs = self._find_args_compat(*args, **kwargs) + return self.__real_find(**new_kwargs) + + def __real_find(self, spec=None, projection=None, skip=0, limit=0, filter=None, **kwargs): + cursor = kwargs.pop("cursor", False) + rows = [] def on_ok(result, this_func): docs, dfr = result if cursor: + warnings.warn("find() with cursor=True is deprecated. Please use" + "find_with_cursor() instead.", DeprecationWarning) return docs, dfr if docs: @@ -282,8 +317,8 @@ def on_ok(result, this_func): else: return rows - return self.find_with_cursor(spec=spec, skip=skip, limit=limit, fields=fields, - filter=filter, **kwargs).addCallback(on_ok, on_ok) + return self.__real_find_with_cursor(spec, projection, skip, limit, filter, + **kwargs).addCallback(on_ok, on_ok) @staticmethod def __apply_find_filter(spec, c_filter): @@ -300,25 +335,29 @@ def __apply_find_filter(spec, c_filter): return spec @timeout - def find_with_cursor(self, spec=None, skip=0, limit=0, fields=None, filter=None, **kwargs): + def find_with_cursor(self, *args, **kwargs): """Find documents in a collection and return them in one batch at a time This methid is equivalent of :meth:`find()` with `cursor=True`. See :meth:`find()` for description of parameters and return value. """ + new_kwargs = self._find_args_compat(*args, **kwargs) + return self.__real_find_with_cursor(**new_kwargs) + + def __real_find_with_cursor(self, spec=None, projection=None, skip=0, limit=0, filter=None, **kwargs): if spec is None: spec = SON() if not isinstance(spec, dict): raise TypeError("TxMongo: spec must be an instance of dict.") - if not isinstance(fields, (dict, list)) and fields is not None: - raise TypeError("TxMongo: fields must be an instance of dict or list.") + if not isinstance(projection, (dict, list)) and projection is not None: + raise TypeError("TxMongo: projection must be an instance of dict or list.") if not isinstance(skip, int): raise TypeError("TxMongo: skip must be an instance of int.") if not isinstance(limit, int): raise TypeError("TxMongo: limit must be an instance of int.") - fields = self._normalize_fields_projection(fields) + projection = self._normalize_fields_projection(projection) spec = self.__apply_find_filter(spec, filter) @@ -332,7 +371,7 @@ def after_connection(protocol): query = Query(flags=flags, collection=str(self), n_to_skip=skip, n_to_return=limit, - query=spec, fields=fields) + query=spec, fields=projection) deferred_query = protocol.send_QUERY(query) deferred_query.addCallback(after_reply, protocol, after_reply) @@ -383,7 +422,7 @@ def after_reply(reply, protocol, this_func, fetched=0): return proto @timeout - def find_one(self, spec=None, fields=None, **kwargs): + def find_one(self, spec=None, **kwargs): """Get a single document from the collection. All arguments to :meth:`find()` are also valid for :meth:`find_one()`, @@ -396,8 +435,13 @@ def find_one(self, spec=None, fields=None, **kwargs): if isinstance(spec, ObjectId): spec = {"_id": spec} + if "fields" in kwargs: + warnings.warn("find_one() signature is changed. Please refer" + "to documentation", DeprecationWarning) + projection = kwargs.pop("projection", kwargs.pop("fields", None)) + kwargs.pop("limit", None) # do not conflict hard limit - return self.find(spec=spec, limit=1, fields=fields, **kwargs)\ + return self.__real_find(spec, projection, limit=1, **kwargs)\ .addCallback(lambda result: result[0] if result else None) @timeout From e0dc745fe61ceb8f6f5030a8fb816ede4e870b16 Mon Sep 17 00:00:00 2001 From: Ilya Skriblovsky Date: Sun, 16 Oct 2016 17:34:59 +0300 Subject: [PATCH 2/9] find() args renamed: spec->filter, filter->sort --- tests/test_collection.py | 66 ++++++++++++++++++++++++++-------------- txmongo/collection.py | 53 +++++++++++++++++--------------- 2 files changed, 73 insertions(+), 46 deletions(-) diff --git a/tests/test_collection.py b/tests/test_collection.py index f3ae96e..c86c458 100644 --- a/tests/test_collection.py +++ b/tests/test_collection.py @@ -373,67 +373,89 @@ def test_Fail(self): class TestFindSignatureCompat(unittest.TestCase): def test_convert(self): + self.assertEqual( + Collection._find_args_compat(spec={'x': 42}), + {"filter": {'x': 42}, "projection": None, "skip": 0, "limit": 0, "sort": None, + "cursor": False} + ) + self.assertEqual( + Collection._find_args_compat(filter={'x': 42}), + {"filter": {'x': 42}, "projection": None, "skip": 0, "limit": 0, "sort": None} + ) + self.assertEqual( + Collection._find_args_compat(filter=qf.sort(qf.ASCENDING('x'))), + {"filter": None, "projection": None, "skip": 0, "limit": 0, + "sort": qf.sort(qf.ASCENDING('x')), "cursor": False} + ) + self.assertEqual( + Collection._find_args_compat(sort=qf.sort(qf.ASCENDING('x'))), + {"filter": None, "projection": None, "skip": 0, "limit": 0, + "sort": qf.sort(qf.ASCENDING('x'))} + ) self.assertEqual( Collection._find_args_compat({'x': 42}), - {"spec": {'x': 42}, "projection": None, "skip": 0, "limit": 0, "filter": None} + {"filter": {'x': 42}, "projection": None, "skip": 0, "limit": 0, "sort": None} ) self.assertEqual( Collection._find_args_compat({'x': 42}, unknown_arg=123), - {"spec": {'x': 42}, "projection": None, "skip": 0, "limit": 0, "filter": None, + {"filter": {'x': 42}, "projection": None, "skip": 0, "limit": 0, "sort": None, "unknown_arg": 123} ) - self.assertEqual( - Collection._find_args_compat(spec={'x': 42}), - {"spec": {'x': 42}, "projection": None, "skip": 0, "limit": 0, "filter": None} - ) self.assertEqual( Collection._find_args_compat({'x': 42}, {'a': 1}), - {"spec": {'x': 42}, "projection": {'a': 1}, "skip": 0, "limit": 0, "filter": None} + {"filter": {'x': 42}, "projection": {'a': 1}, "skip": 0, "limit": 0, "sort": None} ) self.assertEqual( Collection._find_args_compat({'x': 42}, projection={'a': 1}), - {"spec": {'x': 42}, "projection": {'a': 1}, "skip": 0, "limit": 0, "filter": None} + {"filter": {'x': 42}, "projection": {'a': 1}, "skip": 0, "limit": 0, "sort": None} ) self.assertEqual( Collection._find_args_compat({'x': 42}, fields={'a': 1}), - {"spec": {'x': 42}, "projection": {'a': 1}, "skip": 0, "limit": 0, "filter": None, + {"filter": {'x': 42}, "projection": {'a': 1}, "skip": 0, "limit": 0, "sort": None, "cursor": False} ) self.assertEqual( Collection._find_args_compat({'x': 42}, 5), - {"spec": {'x': 42}, "projection": None, "skip": 5, "limit": 0, "filter": None, + {"filter": {'x': 42}, "projection": None, "skip": 5, "limit": 0, "sort": None, "cursor": False} ) self.assertEqual( Collection._find_args_compat({'x': 42}, {'a': 1}, 5), - {"spec": {'x': 42}, "projection": {'a': 1}, "skip": 5, "limit": 0, "filter": None} + {"filter": {'x': 42}, "projection": {'a': 1}, "skip": 5, "limit": 0, "sort": None} ) self.assertEqual( Collection._find_args_compat({'x': 42}, {'a': 1}, 5, 6), - {"spec": {'x': 42}, "projection": {'a': 1}, "skip": 5, "limit": 6, "filter": None} + {"filter": {'x': 42}, "projection": {'a': 1}, "skip": 5, "limit": 6, "sort": None} ) self.assertEqual( Collection._find_args_compat({'x': 42}, 5, 6, {'a': 1}), - {"spec": {'x': 42}, "projection": {'a': 1}, "skip": 5, "limit": 6, "filter": None, + {"filter": {'x': 42}, "projection": {'a': 1}, "skip": 5, "limit": 6, "sort": None, "cursor": False} ) self.assertEqual( Collection._find_args_compat({'x': 42}, {'a': 1}, 5, 6, qf.sort([('s', 1)])), - {"spec": {'x': 42}, "projection": {'a': 1}, "skip": 5, "limit": 6, - "filter": qf.sort([('s', 1)])} + {"filter": {'x': 42}, "projection": {'a': 1}, "skip": 5, "limit": 6, + "sort": qf.sort([('s', 1)])} ) self.assertEqual( Collection._find_args_compat({'x': 42}, 5, 6, {'a': 1}, qf.sort([('s', 1)])), - {"spec": {'x': 42}, "projection": {'a': 1}, "skip": 5, "limit": 6, - "filter": qf.sort([('s', 1)]), "cursor": False} + {"filter": {'x': 42}, "projection": {'a': 1}, "skip": 5, "limit": 6, + "sort": qf.sort([('s', 1)]), "cursor": False} ) self.assertEqual( Collection._find_args_compat({'x': 42}, 5, 6, {'a': 1}, qf.sort([('s', 1)]), True), - {"spec": {'x': 42}, "projection": {'a': 1}, "skip": 5, "limit": 6, - "filter": qf.sort([('s', 1)]), "cursor": True} + {"filter": {'x': 42}, "projection": {'a': 1}, "skip": 5, "limit": 6, + "sort": qf.sort([('s', 1)]), "cursor": True} + ) + self.assertEqual( + Collection._find_args_compat(spec={'x': 42}, filter=qf.sort([('s', 1)]), limit=6, + fields={'a': 1}, skip=5), + {"filter": {'x': 42}, "projection": {'a': 1}, "skip": 5, "limit": 6, + "sort": qf.sort([('s', 1)]), "cursor": False} ) self.assertEqual( - Collection._find_args_compat({'x': 42}, filter=qf.sort([('s', 1)]), limit=6, projection={'a': 1}, skip=5), - {"spec": {'x': 42}, "projection": {'a': 1}, "skip": 5, "limit": 6, - "filter": qf.sort([('s', 1)])} + Collection._find_args_compat(filter={'x': 42}, sort=qf.sort([('s', 1)]), limit=6, + projection={'a': 1}, skip=5), + {"filter": {'x': 42}, "projection": {'a': 1}, "skip": 5, "limit": 6, + "sort": qf.sort([('s', 1)])} ) diff --git a/txmongo/collection.py b/txmongo/collection.py index 14156a6..0ebc2df 100644 --- a/txmongo/collection.py +++ b/txmongo/collection.py @@ -22,6 +22,7 @@ validate_is_mapping, validate_boolean from pymongo.collection import ReturnDocument from pymongo.write_concern import WriteConcern +from txmongo.filter import _QueryFilter from txmongo.protocol import DELETE_SINGLE_REMOVE, UPDATE_UPSERT, UPDATE_MULTI, \ Query, Getmore, Insert, Update, Delete, KillCursors, INSERT_CONTINUE_ON_ERROR from txmongo.utils import check_deadline, timeout @@ -231,17 +232,25 @@ def _find_args_compat(*args, **kwargs): This function makes it compatible with both """ def old(spec=None, skip=0, limit=0, fields=None, filter=None, cursor=False, **kwargs): - warnings.warn("find(), find_with_cursor() and find_one() signatures have" + warnings.warn("find(), find_with_cursor() and find_one() signatures have " "changed. Please refer to documentation.", DeprecationWarning) return new(spec, fields, skip, limit, filter, cursor=cursor, **kwargs) - def new(spec=None, projection=None, skip=0, limit=0, filter=None, **kwargs): - args = {"spec": spec, "projection": projection, "skip": skip, "limit": limit, - "filter": filter} + def new(filter=None, projection=None, skip=0, limit=0, sort=None, **kwargs): + args = {"filter": filter, "projection": projection, "skip": skip, "limit": limit, + "sort": sort} args.update(kwargs) return args - if len(args) >= 2 and isinstance(args[1], int) or "fields" in kwargs: + old_if = ( + "fields" in kwargs, + "spec" in kwargs, + len(args) == 0 and isinstance(kwargs.get("filter"), _QueryFilter), + len(args) >= 1 and "filter" in kwargs, + len(args) >= 2 and isinstance(args[1], int), + ) + + if any(old_if): return old(*args, **kwargs) else: return new(*args, **kwargs) @@ -298,7 +307,7 @@ def query(): new_kwargs = self._find_args_compat(*args, **kwargs) return self.__real_find(**new_kwargs) - def __real_find(self, spec=None, projection=None, skip=0, limit=0, filter=None, **kwargs): + def __real_find(self, filter=None, projection=None, skip=0, limit=0, sort=None, **kwargs): cursor = kwargs.pop("cursor", False) rows = [] @@ -317,7 +326,7 @@ def on_ok(result, this_func): else: return rows - return self.__real_find_with_cursor(spec, projection, skip, limit, filter, + return self.__real_find_with_cursor(filter, projection, skip, limit, sort, **kwargs).addCallback(on_ok, on_ok) @staticmethod @@ -344,11 +353,11 @@ def find_with_cursor(self, *args, **kwargs): new_kwargs = self._find_args_compat(*args, **kwargs) return self.__real_find_with_cursor(**new_kwargs) - def __real_find_with_cursor(self, spec=None, projection=None, skip=0, limit=0, filter=None, **kwargs): - if spec is None: - spec = SON() + def __real_find_with_cursor(self, filter=None, projection=None, skip=0, limit=0, sort=None, **kwargs): + if filter is None: + filter = SON() - if not isinstance(spec, dict): + if not isinstance(filter, dict): raise TypeError("TxMongo: spec must be an instance of dict.") if not isinstance(projection, (dict, list)) and projection is not None: raise TypeError("TxMongo: projection must be an instance of dict or list.") @@ -359,7 +368,7 @@ def __real_find_with_cursor(self, spec=None, projection=None, skip=0, limit=0, f projection = self._normalize_fields_projection(projection) - spec = self.__apply_find_filter(spec, filter) + filter = self.__apply_find_filter(filter, sort) as_class = kwargs.get("as_class") proto = self._database.connection.getprotocol() @@ -371,7 +380,7 @@ def after_connection(protocol): query = Query(flags=flags, collection=str(self), n_to_skip=skip, n_to_return=limit, - query=spec, fields=projection) + query=filter, fields=projection) deferred_query = protocol.send_QUERY(query) deferred_query.addCallback(after_reply, protocol, after_reply) @@ -422,7 +431,7 @@ def after_reply(reply, protocol, this_func, fetched=0): return proto @timeout - def find_one(self, spec=None, **kwargs): + def find_one(self, *args, **kwargs): """Get a single document from the collection. All arguments to :meth:`find()` are also valid for :meth:`find_one()`, @@ -432,17 +441,13 @@ def find_one(self, spec=None, **kwargs): a :class:`Deferred` that called back with single document or ``None`` if no matching documents is found. """ - if isinstance(spec, ObjectId): - spec = {"_id": spec} - - if "fields" in kwargs: - warnings.warn("find_one() signature is changed. Please refer" - "to documentation", DeprecationWarning) - projection = kwargs.pop("projection", kwargs.pop("fields", None)) + new_kwargs = self._find_args_compat(*args, **kwargs) + if isinstance(new_kwargs["filter"], ObjectId): + new_kwargs["filter"] = {"_id": new_kwargs["filter"]} - kwargs.pop("limit", None) # do not conflict hard limit - return self.__real_find(spec, projection, limit=1, **kwargs)\ - .addCallback(lambda result: result[0] if result else None) + new_kwargs["limit"] = 1 + return self.__real_find(**new_kwargs)\ + .addCallback(lambda result: result[0] if result else None) @timeout def count(self, spec=None, **kwargs): From 514517ac73aab3902385935083e9e5ce4d43579b Mon Sep 17 00:00:00 2001 From: Ilya Skriblovsky Date: Sun, 16 Oct 2016 18:02:12 +0300 Subject: [PATCH 3/9] Docstrings updated --- txmongo/collection.py | 74 +++++++++++++++++++++++-------------------- 1 file changed, 39 insertions(+), 35 deletions(-) diff --git a/txmongo/collection.py b/txmongo/collection.py index 0ebc2df..5abeb4c 100644 --- a/txmongo/collection.py +++ b/txmongo/collection.py @@ -227,7 +227,7 @@ def _find_args_compat(*args, **kwargs): signature of find() was changed from (spec=None, skip=0, limit=0, fields=None, filter=None, cursor=False, **kwargs) to - (spec=None, projection=None, skip=0, limit=0, filter=None, **kwargs) + (filter=None, projection=None, skip=0, limit=0, sort=None, **kwargs) This function makes it compatible with both """ @@ -257,16 +257,19 @@ def new(filter=None, projection=None, skip=0, limit=0, sort=None, **kwargs): @timeout def find(self, *args, **kwargs): - """Find documents in a collection. + """find(filter=None, projection=None, skip=0, limit=0, sort=None, **kwargs) - The `spec` argument is a MongoDB query document. To return all documents - in a collection, omit this parameter or pass an empty document (``{}``). - You can pass ``{"key": "value"}`` to select documents having ``key`` - field equal to ``"value"`` or use any of `MongoDB's query selectors - `_. + Find documents in a collection. Ordering, indexing hints and other query parameters can be set with - `filter` argument. See :mod:`txmongo.filter` for details. + `sort` argument. See :mod:`txmongo.filter` for details. + + :param filter: + MongoDB query document. To return all documents in a collection, + omit this parameter or pass an empty document (``{}``). You can pass + ``{"key": "value"}`` to select documents having ``key`` field + equal to ``"value"`` or use any of `MongoDB's query selectors + `_. :param projection: a list of field names that should be returned for each document @@ -282,27 +285,12 @@ def find(self, *args, **kwargs): the maximum number of documents to return. All documents are returned when `limit` is zero. - :param as_class: *(keyword only)* - if not ``None``, returned documents will be converted to type - specified. For example, you can use ``as_class=collections.OrderedDict`` - or ``as_class=bson.SON`` when field ordering in documents is important. - - :returns: an instance of :class:`Deferred` that called back with one of: - - - if `cursor` is ``False`` (the default) --- all documents found - - if `cursor` is ``True`` --- tuple of ``(docs, dfr)``, where - ``docs`` is a partial result, returned by MongoDB in a first - batch and ``dfr`` is a :class:`Deferred` that fires with next - ``(docs, dfr)``. Last result will be ``([], None)``. Using this - mode you can iterate over the result set with code like that: - :: - @defer.inlineCallbacks - def query(): - docs, dfr = yield coll.find(query, cursor=True) - while docs: - for doc in docs: - do_something(doc) - docs, dfr = yield dfr + :param sort: + query filter. You can specify ordering, indexing hints and other query + parameters with this argument. See :mod:`txmongo.filter` for details. + + :returns: an instance of :class:`Deferred` that called back with a list with + all documents found. """ new_kwargs = self._find_args_compat(*args, **kwargs) return self.__real_find(**new_kwargs) @@ -345,10 +333,24 @@ def __apply_find_filter(spec, c_filter): @timeout def find_with_cursor(self, *args, **kwargs): - """Find documents in a collection and return them in one batch at a time - - This methid is equivalent of :meth:`find()` with `cursor=True`. - See :meth:`find()` for description of parameters and return value. + """find_with_cursor(filter=None, projection=None, skip=0, limit=0, sort=None, **kwargs) + + Find documents in a collection and return them in one batch at a time. + + Arguments are the same as for :meth:`find()`. + + :returns: an instance of :class:`Deferred` that fires with tuple of ``(docs, dfr)``, + where ``docs`` is a partial result, returned by MongoDB in a first batch and + ``dfr`` is a :class:`Deferred` that fires with next ``(docs, dfr)``. Last result + will be ``([], None)``. You can iterate over the result set with code like that: + :: + @defer.inlineCallbacks + def query(): + docs, dfr = yield coll.find(query, cursor=True) + while docs: + for doc in docs: + do_something(doc) + docs, dfr = yield dfr """ new_kwargs = self._find_args_compat(*args, **kwargs) return self.__real_find_with_cursor(**new_kwargs) @@ -358,7 +360,7 @@ def __real_find_with_cursor(self, filter=None, projection=None, skip=0, limit=0, filter = SON() if not isinstance(filter, dict): - raise TypeError("TxMongo: spec must be an instance of dict.") + raise TypeError("TxMongo: filter must be an instance of dict.") if not isinstance(projection, (dict, list)) and projection is not None: raise TypeError("TxMongo: projection must be an instance of dict or list.") if not isinstance(skip, int): @@ -432,7 +434,9 @@ def after_reply(reply, protocol, this_func, fetched=0): @timeout def find_one(self, *args, **kwargs): - """Get a single document from the collection. + """find_one(filter=None, projection=None, **kwargs) + + Get a single document from the collection. All arguments to :meth:`find()` are also valid for :meth:`find_one()`, although `limit` will be ignored. From a5b5bb68af438b22de79cdbf62f31761bbc656e6 Mon Sep 17 00:00:00 2001 From: Ilya Skriblovsky Date: Sun, 16 Oct 2016 18:02:50 +0300 Subject: [PATCH 4/9] spec->filter in count() signature (with backwards compatibility) --- txmongo/collection.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/txmongo/collection.py b/txmongo/collection.py index 5abeb4c..b5209be 100644 --- a/txmongo/collection.py +++ b/txmongo/collection.py @@ -454,10 +454,10 @@ def find_one(self, *args, **kwargs): .addCallback(lambda result: result[0] if result else None) @timeout - def count(self, spec=None, **kwargs): + def count(self, filter=None, **kwargs): """Get the number of documents in this collection. - :param spec: + :param filter: argument is a query document that selects which documents to count in the collection. @@ -467,8 +467,11 @@ def count(self, spec=None, **kwargs): :returns: a :class:`Deferred` that called back with a number of documents matching the criteria. """ + if "spec" in kwargs: + filter = kwargs["spec"] + return self._database.command("count", self._collection_name, - query=spec or SON(), **kwargs)\ + query=filter or SON(), **kwargs)\ .addCallback(lambda result: int(result['n'])) @timeout From ae09ae5d7b039e5d9fa4b11a74e528988a37ffea Mon Sep 17 00:00:00 2001 From: Ilya Skriblovsky Date: Sun, 16 Oct 2016 18:10:51 +0300 Subject: [PATCH 5/9] spec->filter for distinct() --- txmongo/collection.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/txmongo/collection.py b/txmongo/collection.py index b5209be..8091363 100644 --- a/txmongo/collection.py +++ b/txmongo/collection.py @@ -1045,10 +1045,11 @@ def rename(self, new_name, **kwargs): return self._database("admin").command("renameCollection", str(self), to=to, **kwargs) @timeout - def distinct(self, key, spec=None, **kwargs): + def distinct(self, key, filter=None, **kwargs): params = {"key": key} - if spec: - params["query"] = spec + filter = kwargs.pop("spec", filter) + if filter: + params["query"] = filter params.update(kwargs) return self._database.command("distinct", self._collection_name, **params)\ From 9e363e38bfd498f995f62a0e3f025985d89717e4 Mon Sep 17 00:00:00 2001 From: Ilya Skriblovsky Date: Sun, 16 Oct 2016 18:20:25 +0300 Subject: [PATCH 6/9] Update NEWS.rst --- docs/source/NEWS.rst | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/docs/source/NEWS.rst b/docs/source/NEWS.rst index 797b6f0..ae70f24 100644 --- a/docs/source/NEWS.rst +++ b/docs/source/NEWS.rst @@ -20,6 +20,20 @@ Features API Changes ^^^^^^^^^^^ +- ``find()``, ``find_one()``, ``find_with_cursor()``, ``count()`` and ``distinct()`` signatures + changed to more closely match PyMongo's counterparts. New signatures are: + + - ``find(filter=None, projection=None, skip=0, limit=0, sort=None, **kwargs)`` + - ``find_with_cursor(filter=None, projection=None, skip=0, limit=0, sort=None, **kwargs)`` + - ``find_one(filter=None, projection=None, **kwargs)`` + - ``count(filter=None, **kwargs)`` + - ``distinct(key, filter=None, **kwargs)`` + + Old signatures are now deprecated and will be supported in this and one subsequent releases. + After that only new signatures will be valid. +- ``cursor`` argument to ``find()`` is deprecated. Please use ``find_with_cursor()`` directly + if you need to iterate over results by batches. ``cursor`` will be supported in this and + one subsequent releases. - ``Database.command()`` now takes ``codec_options`` argument. - ``watchdog_interval`` and ``watchdog_timeout`` arguments of ``ConnectionPool`` renamed to ``ping_interval`` and ``ping_timeout`` correspondingly along with internal change of From ab4b80d54deb144daa91a41ec0df7720405f8329 Mon Sep 17 00:00:00 2001 From: Ilya Skriblovsky Date: Sun, 16 Oct 2016 23:57:57 +0300 Subject: [PATCH 7/9] Fixed using hint with count() --- tests/test_queries.py | 21 +++++++++++++++++++++ txmongo/collection.py | 12 ++++++++++++ 2 files changed, 33 insertions(+) diff --git a/tests/test_queries.py b/tests/test_queries.py index 29ccd27..8185d74 100644 --- a/tests/test_queries.py +++ b/tests/test_queries.py @@ -1038,3 +1038,24 @@ def test_count(self): self.assertEqual((yield self.coll.count({'x': {"$gt": 15}})), 2) self.assertEqual((yield self.db.non_existing.count()), 0) + + @defer.inlineCallbacks + def test_hint(self): + yield self.coll.create_index(qf.sort(qf.ASCENDING('x'))) + + cnt = yield self.coll.count(hint=qf.hint(qf.ASCENDING('x'))) + self.assertEqual(cnt, 3) + + yield self.assertFailure(self.coll.count(hint=qf.hint(qf.ASCENDING('y'))), + OperationFailure) + + self.assertRaises(TypeError, self.coll.count, hint={'x': 1}) + self.assertRaises(TypeError, self.coll.count, hint=[('x', 1)]) + + @defer.inlineCallbacks + def test_skip_limit(self): + cnt = yield self.coll.count(limit = 2) + self.assertEqual(cnt, 2) + + cnt = yield self.coll.count(skip = 1) + self.assertEqual(cnt, 2) diff --git a/txmongo/collection.py b/txmongo/collection.py index 8091363..8f36f62 100644 --- a/txmongo/collection.py +++ b/txmongo/collection.py @@ -464,12 +464,24 @@ def count(self, filter=None, **kwargs): :param hint: *(keyword only)* :class:`~txmongo.filter.hint` instance specifying index to use. + :param int limit: *(keyword only)* + The maximum number of documents to count. + + :param int skip: *(keyword only)* + The number of matching documents to skip before returning results. + :returns: a :class:`Deferred` that called back with a number of documents matching the criteria. """ if "spec" in kwargs: filter = kwargs["spec"] + if "hint" in kwargs: + hint = kwargs["hint"] + if not isinstance(hint, qf.hint): + raise TypeError("hint must be an instance of txmongo.filter.hint") + kwargs["hint"] = SON(kwargs["hint"]["hint"]) + return self._database.command("count", self._collection_name, query=filter or SON(), **kwargs)\ .addCallback(lambda result: int(result['n'])) From a2602d3769c411f32a01cec2863f8cfcefaff376 Mon Sep 17 00:00:00 2001 From: Ilya Skriblovsky Date: Sun, 16 Oct 2016 23:26:10 +0300 Subject: [PATCH 8/9] Added as_class deprecation in changelog --- docs/source/NEWS.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/source/NEWS.rst b/docs/source/NEWS.rst index ae70f24..47d7695 100644 --- a/docs/source/NEWS.rst +++ b/docs/source/NEWS.rst @@ -34,6 +34,9 @@ API Changes - ``cursor`` argument to ``find()`` is deprecated. Please use ``find_with_cursor()`` directly if you need to iterate over results by batches. ``cursor`` will be supported in this and one subsequent releases. +- ``as_class`` argument to ``find()``, ``find_with_cursor()`` and ``find_one()`` is deprecated. + Please use ``collection.with_options(codec_options=CodecOptions(document_class=...)).find()` + instead. It is lengthty, but it is more generic and this is how you do it with current PyMongo. - ``Database.command()`` now takes ``codec_options`` argument. - ``watchdog_interval`` and ``watchdog_timeout`` arguments of ``ConnectionPool`` renamed to ``ping_interval`` and ``ping_timeout`` correspondingly along with internal change of From 630f4cbf892a4c1cd6ef904cddbad31c6b87b46b Mon Sep 17 00:00:00 2001 From: Ilya Skriblovsky Date: Mon, 17 Oct 2016 10:28:54 +0300 Subject: [PATCH 9/9] Fixed TestCount.test_hint test with MongoDB < 3.2 --- tests/test_queries.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/tests/test_queries.py b/tests/test_queries.py index 8185d74..cc4dc19 100644 --- a/tests/test_queries.py +++ b/tests/test_queries.py @@ -1031,6 +1031,11 @@ def setUp(self): yield super(TestCount, self).setUp() yield self.coll.insert_many([{'x': 10}, {'x': 20}, {'x': 30}]) + @defer.inlineCallbacks + def tearDown(self): + yield self.db.system.profile.drop() + yield super(TestCount, self).tearDown() + @defer.inlineCallbacks def test_count(self): self.assertEqual((yield self.coll.count()), 3) @@ -1043,11 +1048,13 @@ def test_count(self): def test_hint(self): yield self.coll.create_index(qf.sort(qf.ASCENDING('x'))) + yield self.db.command("profile", 2) cnt = yield self.coll.count(hint=qf.hint(qf.ASCENDING('x'))) self.assertEqual(cnt, 3) + yield self.db.command("profile", 0) - yield self.assertFailure(self.coll.count(hint=qf.hint(qf.ASCENDING('y'))), - OperationFailure) + cmds = yield self.db.system.profile.count({"command.hint": {"x": 1}}) + self.assertEqual(cmds, 1) self.assertRaises(TypeError, self.coll.count, hint={'x': 1}) self.assertRaises(TypeError, self.coll.count, hint=[('x', 1)])