From be47e4ca1494d3dbfd6eea5d23b797b2c9897fc6 Mon Sep 17 00:00:00 2001 From: Shane Harvey Date: Mon, 9 Aug 2021 10:02:36 -0700 Subject: [PATCH] PYTHON-2532 Remove modifiers option for find methods (#696) --- doc/api/pymongo/collection.rst | 4 +- doc/api/pymongo/cursor.rst | 4 +- doc/changelog.rst | 5 ++- doc/migrate-to-pymongo4.rst | 33 +++++++++++++++ pymongo/collection.py | 6 +-- pymongo/cursor.py | 21 ++++------ test/command_monitoring/legacy/find.json | 28 ++++++------- test/test_command_monitoring_legacy.py | 2 + test/test_cursor.py | 52 ------------------------ 9 files changed, 66 insertions(+), 89 deletions(-) diff --git a/doc/api/pymongo/collection.rst b/doc/api/pymongo/collection.rst index 1787f22c7..420c060e7 100644 --- a/doc/api/pymongo/collection.rst +++ b/doc/api/pymongo/collection.rst @@ -46,8 +46,8 @@ .. automethod:: aggregate .. automethod:: aggregate_raw_batches .. automethod:: watch - .. automethod:: find(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, 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, session=None, allow_disk_use=None) - .. automethod:: find_raw_batches(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, 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, session=None, allow_disk_use=None) + .. automethod:: find(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, batch_size=0, 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, session=None, allow_disk_use=None) + .. automethod:: find_raw_batches(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, batch_size=0, 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, session=None, allow_disk_use=None) .. automethod:: find_one(filter=None, *args, **kwargs) .. automethod:: find_one_and_delete .. automethod:: find_one_and_replace(filter, replacement, projection=None, sort=None, return_document=ReturnDocument.BEFORE, hint=None, session=None, **kwargs) diff --git a/doc/api/pymongo/cursor.rst b/doc/api/pymongo/cursor.rst index 68b52bcce..513f051ab 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, batch_size=0, 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, session=None, allow_disk_use=None) + .. 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, batch_size=0, 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, session=None, allow_disk_use=None) :members: .. describe:: c[index] @@ -24,4 +24,4 @@ .. automethod:: __getitem__ - .. autoclass:: pymongo.cursor.RawBatchCursor(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, 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, allow_disk_use=None) + .. autoclass:: pymongo.cursor.RawBatchCursor(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, batch_size=0, 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, allow_disk_use=None) diff --git a/doc/changelog.rst b/doc/changelog.rst index d97c8dbe6..fb937638e 100644 --- a/doc/changelog.rst +++ b/doc/changelog.rst @@ -76,9 +76,10 @@ Breaking Changes in 4.0 :attr:`pymongo.database.Database.outgoing_manipulators`, :attr:`pymongo.database.Database.incoming_copying_manipulators`, and :attr:`pymongo.database.Database.incoming_manipulators`. -- Removed the ``manipulate`` parameter from +- Removed the ``manipulate`` and ``modifiers`` parameters from :meth:`~pymongo.collection.Collection.find`, - :meth:`~pymongo.collection.Collection.find_one`, and + :meth:`~pymongo.collection.Collection.find_one`, + :meth:`~pymongo.collection.Collection.find_raw_batches`, and :meth:`~pymongo.cursor.Cursor`. - Removed :meth:`pymongo.message.delete`, :meth:`pymongo.message.get_more`, :meth:`pymongo.message.insert`, :meth:`pymongo.message.kill_cursors`, diff --git a/doc/migrate-to-pymongo4.rst b/doc/migrate-to-pymongo4.rst index 6b84c7fcb..eedd90b6b 100644 --- a/doc/migrate-to-pymongo4.rst +++ b/doc/migrate-to-pymongo4.rst @@ -448,6 +448,39 @@ can be changed to this:: .. _reIndex command: https://docs.mongodb.com/manual/reference/command/reIndex/ +The modifiers parameter is removed +.................................. + +Removed the ``modifiers`` parameter from +:meth:`~pymongo.collection.Collection.find`, +:meth:`~pymongo.collection.Collection.find_one`, +:meth:`~pymongo.collection.Collection.find_raw_batches`, and +:meth:`~pymongo.cursor.Cursor`. Pass the options directly to the method +instead. Code like this:: + + cursor = coll.find({}, modifiers={ + "$comment": "comment", + "$hint": {"_id": 1}, + "$min": {"_id": 0}, + "$max": {"_id": 6}, + "$maxTimeMS": 6000, + "$returnKey": False, + "$showDiskLoc": False, + }) + +can be changed to this:: + + cursor = coll.find( + {}, + comment="comment", + hint={"_id": 1}, + min={"_id": 0}, + max={"_id": 6}, + max_time_ms=6000, + return_key=False, + show_record_id=False, + ) + SONManipulator is removed ------------------------- diff --git a/pymongo/collection.py b/pymongo/collection.py index e4cb2487b..aba15cfeb 100644 --- a/pymongo/collection.py +++ b/pymongo/collection.py @@ -1313,9 +1313,6 @@ class Collection(common.BaseObject): interpret and trace the operation in the server logs and in profile data. 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. - `allow_disk_use` (optional): if True, MongoDB may use temporary disk files to store data exceeding the system memory limit while processing a blocking sort operation. The option has no effect if @@ -1339,6 +1336,9 @@ class Collection(common.BaseObject): connection will be closed and discarded without being returned to the connection pool. + .. versionchanged:: 4.0 + Removed the ``modifiers`` option. + .. versionchanged:: 3.11 Added the ``allow_disk_use`` option. Deprecated the ``oplog_replay`` option. Support for this option is diff --git a/pymongo/cursor.py b/pymongo/cursor.py index 2f6550d52..7a8c08c9e 100644 --- a/pymongo/cursor.py +++ b/pymongo/cursor.py @@ -136,10 +136,10 @@ class Cursor(object): 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, + batch_size=0, 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, session=None, + max=None, min=None, return_key=None, show_record_id=None, + snapshot=None, comment=None, session=None, allow_disk_use=None): """Create a new cursor. @@ -186,10 +186,6 @@ class Cursor(object): raise ValueError("not a valid value for cursor_type") 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, int): raise TypeError("batch_size must be an integer") if batch_size < 0: @@ -208,7 +204,6 @@ class Cursor(object): self.__skip = skip self.__limit = limit 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 = max_scan self.__explain = False @@ -318,7 +313,7 @@ class Cursor(object): "max_time_ms", "max_await_time_ms", "comment", "max", "min", "ordering", "explain", "hint", "batch_size", "max_scan", - "query_flags", "modifiers", "collation", "empty", + "query_flags", "collation", "empty", "show_record_id", "return_key", "allow_disk_use", "snapshot", "exhaust") data = dict((k, v) for k, v in self.__dict__.items() @@ -370,7 +365,7 @@ class Cursor(object): def __query_spec(self): """Get the spec to use for a query. """ - operators = self.__modifiers.copy() + operators = {} if self.__ordering: operators["$orderby"] = self.__ordering if self.__explain: @@ -387,12 +382,12 @@ class Cursor(object): operators["$max"] = self.__max if self.__min: operators["$min"] = self.__min - if self.__return_key: + if self.__return_key is not None: operators["$returnKey"] = self.__return_key - if self.__show_record_id: + if self.__show_record_id is not None: # This is upgraded to showRecordId for MongoDB 3.2+ "find" command. operators["$showDiskLoc"] = self.__show_record_id - if self.__snapshot: + if self.__snapshot is not None: operators["$snapshot"] = self.__snapshot if operators: diff --git a/test/command_monitoring/legacy/find.json b/test/command_monitoring/legacy/find.json index 55b185cc5..e2bb95306 100644 --- a/test/command_monitoring/legacy/find.json +++ b/test/command_monitoring/legacy/find.json @@ -89,21 +89,19 @@ "skip": { "$numberLong": "2" }, - "modifiers": { - "$comment": "test", - "$hint": { - "_id": 1 - }, - "$max": { - "_id": 6 - }, - "$maxTimeMS": 6000, - "$min": { - "_id": 0 - }, - "$returnKey": false, - "$showDiskLoc": false - } + "comment": "test", + "hint": { + "_id": 1 + }, + "max": { + "_id": 6 + }, + "maxTimeMS": 6000, + "min": { + "_id": 0 + }, + "returnKey": false, + "showRecordId": false } }, "expectations": [ diff --git a/test/test_command_monitoring_legacy.py b/test/test_command_monitoring_legacy.py index acebe3a23..2daa60679 100644 --- a/test/test_command_monitoring_legacy.py +++ b/test/test_command_monitoring_legacy.py @@ -109,6 +109,8 @@ def create_test(scenario_def, test): elif name == 'find': if 'sort' in args: args['sort'] = list(args['sort'].items()) + if 'hint' in args: + args['hint'] = list(args['hint'].items()) for arg in 'skip', 'limit': if arg in args: args[arg] = int(args[arg]) diff --git a/test/test_cursor.py b/test/test_cursor.py index 3ba455c18..f743c6dce 100644 --- a/test/test_cursor.py +++ b/test/test_cursor.py @@ -1317,58 +1317,6 @@ class TestCursor(IntegrationTest): next(cursor) self.assertRaises(InvalidOperation, cursor.comment, 'hello') - def test_modifiers(self): - 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}) - - # 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) - - if not client_context.version.at_least(3, 7, 3): - cur = c.find(snapshot=True, modifiers={"$snapshot": False}) - self.assertEqual(cur._Cursor__query_spec()["$snapshot"], True) - def test_alive(self): self.db.test.delete_many({}) self.db.test.insert_many([{} for _ in range(3)])