From 7b1cbace17348b71f49277d0edba7615b2da41d7 Mon Sep 17 00:00:00 2001 From: "A. Jesse Jiryu Davis" Date: Thu, 20 Jul 2017 13:56:13 -0400 Subject: [PATCH] PYTHON-1217 - Deprecate the "modifiers" find option --- doc/api/pymongo/cursor.rst | 2 +- doc/changelog.rst | 3 ++ doc/migrate-to-pymongo3.rst | 25 ++---------- pymongo/collection.py | 55 ++++++++++++++++++------- pymongo/cursor.py | 48 +++++++++++++++------- test/test_cursor.py | 62 ++++++++++++++++++++++------ test/test_monitoring.py | 80 ++++++++++++++++++++++++++----------- 7 files changed, 189 insertions(+), 86 deletions(-) diff --git a/doc/api/pymongo/cursor.rst b/doc/api/pymongo/cursor.rst index 42624d252..6c9ca4800 100644 --- a/doc/api/pymongo/cursor.rst +++ b/doc/api/pymongo/cursor.rst @@ -15,7 +15,7 @@ .. autoattribute:: EXHAUST :annotation: - .. autoclass:: pymongo.cursor.Cursor(collection, filter=None, projection=None, skip=0, limit=0, no_cursor_timeout=False, cursor_type=CursorType.NON_TAILABLE, sort=None, allow_partial_results=False, oplog_replay=False, modifiers=None, manipulate=True) + .. autoclass:: pymongo.cursor.Cursor(collection, filter=None, projection=None, skip=0, limit=0, no_cursor_timeout=False, cursor_type=CursorType.NON_TAILABLE, sort=None, allow_partial_results=False, oplog_replay=False, modifiers=None, batch_size=0, manipulate=True, collation=None, hint=None, max_scan=None, max_time_ms=None, max=None, min=None, return_key=False, show_record_id=False, snapshot=False, comment=None) :members: .. describe:: c[index] diff --git a/doc/changelog.rst b/doc/changelog.rst index e3428940c..40021d119 100644 --- a/doc/changelog.rst +++ b/doc/changelog.rst @@ -12,6 +12,9 @@ Highlights: Changes and Deprecations: +- :meth:`~pymongo.collection.Collection.find` has new options `return_key`, + `show_record_id`, `snapshot`, `hint`, `max_time_ms`, `max_scan`, `min`, `max`, + and `comment`. Deprecated the option `modifiers`. - Deprecated :meth:`~pymongo.collection.Collection.group`. The group command was deprecated in MongoDB 3.4 and is expected to be removed in MongoDB 3.6. Applications should use :meth:`~pymongo.collection.Collection.aggregate` diff --git a/doc/migrate-to-pymongo3.rst b/doc/migrate-to-pymongo3.rst index 6a02c4e8c..85f495ea9 100644 --- a/doc/migrate-to-pymongo3.rst +++ b/doc/migrate-to-pymongo3.rst @@ -113,27 +113,6 @@ can be changed to this with PyMongo 2.9 or later: >>> cursor = collection.find({"a": 1}, no_cursor_timeout=True) -"snapshot" and "max_scan" replaced by "modifiers" -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - -The `snapshot` and `max_scan` options have been removed. They can now be set, -along with other $ query modifiers, through the `modifiers` option. Code like -this:: - - >>> cursor = collection.find({"a": 1}, snapshot=True) - -can be changed to this with PyMongo 2.9 or later: - -.. doctest:: - - >>> cursor = collection.find({"a": 1}, modifiers={"$snapshot": True}) - -or with any version of PyMongo: - -.. doctest:: - - >>> cursor = collection.find({"$query": {"a": 1}, "$snapshot": True}) - "network_timeout" is removed ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -152,6 +131,10 @@ can be changed to this with PyMongo 2.9 or later: # Set a 5 second (5000 millisecond) server side query timeout. >>> cursor = collection.find({"a": 1}, modifiers={"$maxTimeMS": 5000}) +or with PyMongo 3.5 or later: + + >>> cursor = collection.find({"a": 1}, max_time_ms=5000) + or with any version of PyMongo: .. doctest:: diff --git a/pymongo/collection.py b/pymongo/collection.py index d2171afeb..3a98c7ec8 100644 --- a/pymongo/collection.py +++ b/pymongo/collection.py @@ -1086,19 +1086,13 @@ class Collection(common.BaseObject): - `**kwargs` (optional): any additional keyword arguments are the same as the arguments to :meth:`find`. - - `max_time_ms` (optional): a value for max_time_ms may be - specified as part of `**kwargs`, e.g. - >>> collection.find_one(max_time_ms=100) """ if (filter is not None and not isinstance(filter, collections.Mapping)): filter = {"_id": filter} - max_time_ms = kwargs.pop("max_time_ms", None) - cursor = self.find(filter, - *args, **kwargs).max_time_ms(max_time_ms) - + cursor = self.find(filter, *args, **kwargs) for result in cursor.limit(-1): return result return None @@ -1174,11 +1168,6 @@ class Collection(common.BaseObject): error. - `oplog_replay` (optional): If True, set the oplogReplay query flag. - - `modifiers` (optional): A dict specifying the MongoDB `query - modifiers`_ that should be used for this query. For example:: - - >>> db.test.find(modifiers={"$maxTimeMS": 500}) - - `batch_size` (optional): Limits the number of documents returned in a single batch. - `manipulate` (optional): **DEPRECATED** - If True (the default), @@ -1186,6 +1175,40 @@ class Collection(common.BaseObject): - `collation` (optional): An instance of :class:`~pymongo.collation.Collation`. This option is only supported on MongoDB 3.4 and above. + - `return_key` (optional): If True, return only the index keys in + each document. + - `show_record_id` (optional): If True, adds a field ``$recordId`` in + each document with the storage engine's internal record identifier. + - `snapshot` (optional): If True, prevents the cursor from returning + a document more than once because of an intervening write + operation. + - `hint` (optional): An index, in the same format as passed to + :meth:`~pymongo.collection.Collection.create_index` (e.g. + ``[('field', ASCENDING)]``). Pass this as an alternative to calling + :meth:`~pymongo.cursor.Cursor.hint` on the cursor to tell Mongo the + proper index to use for the query. + - `max_time_ms` (optional): Specifies a time limit for a query + operation. If the specified time is exceeded, the operation will be + aborted and :exc:`~pymongo.errors.ExecutionTimeout` is raised. Pass + this as an alternative to calling + :meth:`~pymongo.cursor.Cursor.max_time_ms` on the cursor. + - `max_scan` (optional): The maximum number of documents to scan. + Pass this as an alternative to calling + :meth:`~pymongo.cursor.Cursor.max_scan` on the cursor. + - `min` (optional): A list of field, limit pairs specifying the + inclusive lower bound for all keys of a specific index in order. + Pass this as an alternative to calling + :meth:`~pymongo.cursor.Cursor.min` on the cursor. + - `max` (optional): A list of field, limit pairs specifying the + exclusive upper bound for all keys of a specific index in order. + Pass this as an alternative to calling + :meth:`~pymongo.cursor.Cursor.max` on the cursor. + - `comment` (optional): A string or document. Pass this as an + alternative to calling :meth:`~pymongo.cursor.Cursor.comment` on the + cursor. + - `modifiers` (optional): **DEPRECATED** - A dict specifying + additional MongoDB query modifiers. Use the keyword arguments listed + above instead. .. note:: There are a number of caveats to using :attr:`~pymongo.cursor.CursorType.EXHAUST` as cursor_type: @@ -1203,6 +1226,11 @@ class Collection(common.BaseObject): connection will be closed and discarded without being returned to the connection pool. + .. versionchanged:: 3.5 + Added the options `return_key`, `show_record_id`, `snapshot`, + `hint`, `max_time_ms`, `max_scan`, `min`, `max`, and `comment`. + Deprecated the option `modifiers`. + .. versionchanged:: 3.4 Support the `collation` option. @@ -1231,10 +1259,9 @@ class Collection(common.BaseObject): The `tag_sets` and `secondary_acceptable_latency_ms` parameters. .. _PYTHON-500: https://jira.mongodb.org/browse/PYTHON-500 - .. _query modifiers: - http://docs.mongodb.org/manual/reference/operator/query-modifier/ .. mongodoc:: find + """ return Cursor(self, *args, **kwargs) diff --git a/pymongo/cursor.py b/pymongo/cursor.py index fa5133769..11137bf8a 100644 --- a/pymongo/cursor.py +++ b/pymongo/cursor.py @@ -16,6 +16,7 @@ import copy import datetime +import warnings from collections import deque @@ -106,7 +107,9 @@ class Cursor(object): cursor_type=CursorType.NON_TAILABLE, sort=None, allow_partial_results=False, oplog_replay=False, modifiers=None, batch_size=0, manipulate=True, - collation=None): + collation=None, hint=None, max_scan=None, max_time_ms=None, + max=None, min=None, return_key=False, show_record_id=False, + snapshot=False, comment=None): """Create a new cursor. Should not be called directly by application developers - see @@ -134,6 +137,8 @@ class Cursor(object): validate_boolean("allow_partial_results", allow_partial_results) validate_boolean("oplog_replay", oplog_replay) if modifiers is not None: + warnings.warn("the 'modifiers' parameter is deprecated", + DeprecationWarning, stacklevel=2) validate_is_mapping("modifiers", modifiers) if not isinstance(batch_size, integer_types): raise TypeError("batch_size must be an integer") @@ -153,16 +158,19 @@ class Cursor(object): self.__batch_size = batch_size self.__modifiers = modifiers and modifiers.copy() or {} self.__ordering = sort and helpers._index_document(sort) or None - self.__max_scan = None + self.__max_scan = max_scan self.__explain = False - self.__hint = None - self.__comment = None - self.__max_time_ms = None + self.__comment = comment + self.__max_time_ms = max_time_ms self.__max_await_time_ms = None - self.__max = None - self.__min = None + self.__max = max + self.__min = min self.__manipulate = manipulate self.__collation = validate_collation_or_none(collation) + self.__return_key = return_key + self.__show_record_id = show_record_id + self.__snapshot = snapshot + self.__set_hint(hint) # Exhaust cursor support if cursor_type == CursorType.EXHAUST: @@ -312,6 +320,13 @@ class Cursor(object): operators["$max"] = self.__max if self.__min: operators["$min"] = self.__min + if self.__return_key: + operators["$returnKey"] = self.__return_key + if self.__show_record_id: + # This is upgraded to showRecordId for MongoDB 3.2+ "find" command. + operators["$showDiskLoc"] = self.__show_record_id + if self.__snapshot: + operators["$snapshot"] = self.__snapshot if operators: # Make a shallow copy so we can cleanly rewind or clone. @@ -755,6 +770,16 @@ class Cursor(object): c.__limit = -abs(c.__limit) return next(c) + def __set_hint(self, index): + if index is None: + self.__hint = None + return + + if isinstance(index, string_type): + self.__hint = index + else: + self.__hint = helpers._index_document(index) + def hint(self, index): """Adds a 'hint', telling Mongo the proper index to use for the query. @@ -780,14 +805,7 @@ class Cursor(object): The :meth:`~hint` method accepts the name of the index. """ self.__check_okay_to_chain() - if index is None: - self.__hint = None - return self - - if isinstance(index, string_type): - self.__hint = index - else: - self.__hint = helpers._index_document(index) + self.__set_hint(index) return self def comment(self, comment): diff --git a/test/test_cursor.py b/test/test_cursor.py index b248618fe..ed49b3f4d 100644 --- a/test/test_cursor.py +++ b/test/test_cursor.py @@ -40,7 +40,7 @@ from test import (client_context, unittest, IntegrationTest) from test.utils import (rs_or_single_client, - WhiteListEventListener) + WhiteListEventListener, ignore_deprecations) if PY3: long = int @@ -1220,17 +1220,55 @@ class TestCursor(IntegrationTest): self.assertRaises(InvalidOperation, cursor.comment, 'hello') def test_modifiers(self): - cur = self.db.test.find() - self.assertTrue('$query' not in cur._Cursor__query_spec()) - cur = self.db.test.find().comment("testing").max_time_ms(500) - self.assertTrue('$query' in cur._Cursor__query_spec()) - self.assertEqual(cur._Cursor__query_spec()["$comment"], "testing") - self.assertEqual(cur._Cursor__query_spec()["$maxTimeMS"], 500) - cur = self.db.test.find( - modifiers={"$maxTimeMS": 500, "$comment": "testing"}) - self.assertTrue('$query' in cur._Cursor__query_spec()) - self.assertEqual(cur._Cursor__query_spec()["$comment"], "testing") - self.assertEqual(cur._Cursor__query_spec()["$maxTimeMS"], 500) + c = self.db.test + + # "modifiers" is deprecated. + with ignore_deprecations(): + cur = c.find() + self.assertTrue('$query' not in cur._Cursor__query_spec()) + cur = c.find().comment("testing").max_time_ms(500) + self.assertTrue('$query' in cur._Cursor__query_spec()) + self.assertEqual(cur._Cursor__query_spec()["$comment"], "testing") + self.assertEqual(cur._Cursor__query_spec()["$maxTimeMS"], 500) + cur = c.find( + modifiers={"$maxTimeMS": 500, "$comment": "testing"}) + self.assertTrue('$query' in cur._Cursor__query_spec()) + self.assertEqual(cur._Cursor__query_spec()["$comment"], "testing") + self.assertEqual(cur._Cursor__query_spec()["$maxTimeMS"], 500) + + # Keyword arg overwrites modifier. + # If we remove the "modifiers" arg, delete this test after checking + # that TestCommandMonitoring.test_find_options covers all cases. + cur = c.find(comment="hi", modifiers={"$comment": "bye"}) + self.assertEqual(cur._Cursor__query_spec()["$comment"], "hi") + + cur = c.find(max_scan=1, modifiers={"$maxScan": 2}) + self.assertEqual(cur._Cursor__query_spec()["$maxScan"], 1) + + cur = c.find(max_time_ms=1, modifiers={"$maxTimeMS": 2}) + self.assertEqual(cur._Cursor__query_spec()["$maxTimeMS"], 1) + + cur = c.find(min=1, modifiers={"$min": 2}) + self.assertEqual(cur._Cursor__query_spec()["$min"], 1) + + cur = c.find(max=1, modifiers={"$max": 2}) + self.assertEqual(cur._Cursor__query_spec()["$max"], 1) + + cur = c.find(return_key=True, modifiers={"$returnKey": False}) + self.assertEqual(cur._Cursor__query_spec()["$returnKey"], True) + + cur = c.find(hint=[("a", 1)], modifiers={"$hint": {"b": "1"}}) + self.assertEqual(cur._Cursor__query_spec()["$hint"], {"a": 1}) + + cur = c.find(snapshot=True, modifiers={"$snapshot": False}) + self.assertEqual(cur._Cursor__query_spec()["$snapshot"], True) + + # The arg is named show_record_id after the "find" command arg, the + # modifier is named $showDiskLoc for the OP_QUERY modifier. It's + # stored as $showDiskLoc then upgraded to showRecordId if we send a + # "find" command. + cur = c.find(show_record_id=True, modifiers={"$showDiskLoc": False}) + self.assertEqual(cur._Cursor__query_spec()["$showDiskLoc"], True) def test_alive(self): self.db.test.delete_many({}) diff --git a/test/test_monitoring.py b/test/test_monitoring.py index ad3803168..65465e47c 100644 --- a/test/test_monitoring.py +++ b/test/test_monitoring.py @@ -226,33 +226,20 @@ class TestCommandMonitoring(unittest.TestCase): self.assertEqual(self.client.address, succeeded.connection_id) self.assertEqual(res, succeeded.reply) - def test_find_options(self): - cmd = SON([('find', 'test'), - ('filter', {}), - ('comment', 'this is a test'), - ('sort', SON([('_id', 1)])), - ('projection', {'x': False}), - ('skip', 1), - ('batchSize', 2), - ('noCursorTimeout', True), - ('allowPartialResults', True)]) - self.client.pymongo_test.test.drop() - self.client.pymongo_test.test.insert_many([{'x': i} for i in range(5)]) - self.listener.results.clear() + def _test_find_options(self, query, expected_cmd): coll = self.client.pymongo_test.test + coll.drop() + coll.create_index('x') + coll.insert_many([{'x': i} for i in range(5)]) + # Test that we publish the unwrapped command. + self.listener.results.clear() if self.client.is_mongos and client_context.version.at_least(2, 4, 0): coll = coll.with_options( read_preference=ReadPreference.PRIMARY_PREFERRED) - cursor = coll.find( - filter={}, - projection={'x': False}, - skip=1, - no_cursor_timeout=True, - sort=[('_id', 1)], - allow_partial_results=True, - modifiers=SON([('$comment', 'this is a test')]), - batch_size=2) + + cursor = coll.find(**query) + next(cursor) try: results = self.listener.results @@ -261,7 +248,7 @@ class TestCommandMonitoring(unittest.TestCase): self.assertEqual(0, len(results['failed'])) self.assertTrue( isinstance(started, monitoring.CommandStartedEvent)) - self.assertEqual(cmd, started.command) + self.assertEqual(expected_cmd, started.command) self.assertEqual('find', started.command_name) self.assertEqual(self.client.address, started.connection_id) self.assertEqual('pymongo_test', started.database_name) @@ -276,6 +263,53 @@ class TestCommandMonitoring(unittest.TestCase): # Exhaust the cursor to avoid kill cursors. tuple(cursor) + def test_find_options(self): + query = dict(filter={}, + hint=[('x', 1)], + max_scan=10, + max_time_ms=10000, + max={'x': 10}, + min={'x': -10}, + return_key=True, + show_record_id=True, + projection={'x': False}, + skip=1, + no_cursor_timeout=True, + sort=[('_id', 1)], + allow_partial_results=True, + comment='this is a test', + batch_size=2) + + cmd = dict(find='test', + filter={}, + hint=SON([('x', 1)]), + comment='this is a test', + maxScan=10, + maxTimeMS=10000, + max={'x': 10}, + min={'x': -10}, + returnKey=True, + showRecordId=True, + sort=SON([('_id', 1)]), + projection={'x': False}, + skip=1, + batchSize=2, + noCursorTimeout=True, + allowPartialResults=True) + + self._test_find_options(query, cmd) + + def test_find_snapshot(self): + # Test "snapshot" parameter separately, can't combine with "sort". + query = dict(filter={}, + snapshot=True) + + cmd = dict(find='test', + filter={}, + snapshot=True) + + self._test_find_options(query, cmd) + @client_context.require_version_min(2, 6, 0) def test_command_and_get_more(self): self.client.pymongo_test.test.drop()