From 31526f3feb729480b3f8cc635eac770d0f372842 Mon Sep 17 00:00:00 2001 From: Noah Stapp Date: Mon, 24 Mar 2025 11:57:29 -0400 Subject: [PATCH 1/6] PYTHON-4947 - GridFS spec: Add performant 'delete revisions by filename' feature - delete_by_name --- gridfs/asynchronous/grid_file.py | 30 ++++ gridfs/synchronous/grid_file.py | 28 ++++ test/asynchronous/unified_format.py | 5 +- test/gridfs/deleteByName.json | 230 ++++++++++++++++++++++++++++ test/unified_format.py | 5 +- 5 files changed, 296 insertions(+), 2 deletions(-) create mode 100644 test/gridfs/deleteByName.json diff --git a/gridfs/asynchronous/grid_file.py b/gridfs/asynchronous/grid_file.py index 3f3179c45c..d0eab10d3f 100644 --- a/gridfs/asynchronous/grid_file.py +++ b/gridfs/asynchronous/grid_file.py @@ -834,6 +834,36 @@ async def delete(self, file_id: Any, session: Optional[AsyncClientSession] = Non if not res.deleted_count: raise NoFile("no file could be deleted because none matched %s" % file_id) + @_csot.apply + async def delete_by_name( + self, filename: str, session: Optional[AsyncClientSession] = None + ) -> None: + """Given a filename, delete this stored file's files collection document(s) + and associated chunks from a GridFS bucket. + + For example:: + + my_db = AsyncMongoClient().test + fs = AsyncGridFSBucket(my_db) + await fs.upload_from_stream("test_file", "data I want to store!") + await fs.delete_by_name("test_file") + + Raises :exc:`~gridfs.errors.NoFile` if no file with filename exists. + + :param filename: The name of the file to be deleted. + :param session: a + :class:`~pymongo.client_session.AsyncClientSession` + + .. versionadded:: 4.13 + """ + _disallow_transactions(session) + file_ids = self._files.find({"filename": filename}, {"_id": 1}, session=session) + file_ids = [file_id["_id"] async for file_id in file_ids] + res = await self._files.delete_many({"_id": {"$in": file_ids}}, session=session) + await self._chunks.delete_many({"files_id": {"$in": file_ids}}, session=session) + if not res.deleted_count: + raise NoFile("no file could be deleted because none matched %s" % filename) + def find(self, *args: Any, **kwargs: Any) -> AsyncGridOutCursor: """Find and return the files collection documents that match ``filter`` diff --git a/gridfs/synchronous/grid_file.py b/gridfs/synchronous/grid_file.py index 35386857d6..f8c7b2748e 100644 --- a/gridfs/synchronous/grid_file.py +++ b/gridfs/synchronous/grid_file.py @@ -830,6 +830,34 @@ def delete(self, file_id: Any, session: Optional[ClientSession] = None) -> None: if not res.deleted_count: raise NoFile("no file could be deleted because none matched %s" % file_id) + @_csot.apply + def delete_by_name(self, filename: str, session: Optional[ClientSession] = None) -> None: + """Given a filename, delete this stored file's files collection document(s) + and associated chunks from a GridFS bucket. + + For example:: + + my_db = MongoClient().test + fs = GridFSBucket(my_db) + fs.upload_from_stream("test_file", "data I want to store!") + fs.delete_by_name("test_file") + + Raises :exc:`~gridfs.errors.NoFile` if no file with filename exists. + + :param filename: The name of the file to be deleted. + :param session: a + :class:`~pymongo.client_session.ClientSession` + + .. versionadded:: 4.13 + """ + _disallow_transactions(session) + file_ids = self._files.find({"filename": filename}, {"_id": 1}, session=session) + file_ids = [file_id["_id"] for file_id in file_ids] + res = self._files.delete_many({"_id": {"$in": file_ids}}, session=session) + self._chunks.delete_many({"files_id": {"$in": file_ids}}, session=session) + if not res.deleted_count: + raise NoFile("no file could be deleted because none matched %s" % filename) + def find(self, *args: Any, **kwargs: Any) -> GridOutCursor: """Find and return the files collection documents that match ``filter`` diff --git a/test/asynchronous/unified_format.py b/test/asynchronous/unified_format.py index 886b31e4a6..3d63ced9d8 100644 --- a/test/asynchronous/unified_format.py +++ b/test/asynchronous/unified_format.py @@ -66,7 +66,7 @@ from bson import SON, json_util from bson.codec_options import DEFAULT_CODEC_OPTIONS from bson.objectid import ObjectId -from gridfs import AsyncGridFSBucket, GridOut +from gridfs import AsyncGridFSBucket, GridOut, NoFile from pymongo import ASCENDING, AsyncMongoClient, CursorType, _csot from pymongo.asynchronous.change_stream import AsyncChangeStream from pymongo.asynchronous.client_session import AsyncClientSession, TransactionOptions, _TxnState @@ -630,6 +630,9 @@ def process_error(self, exception, spec): self.assertNotIsInstance(error, NotPrimaryError) elif isinstance(error, (InvalidOperation, ConfigurationError, EncryptionError)): pass + # gridfs NoFile errors are considered client errors. + elif isinstance(error, NoFile): + pass else: self.assertNotIsInstance(error, PyMongoError) diff --git a/test/gridfs/deleteByName.json b/test/gridfs/deleteByName.json new file mode 100644 index 0000000000..884d0300ce --- /dev/null +++ b/test/gridfs/deleteByName.json @@ -0,0 +1,230 @@ +{ + "description": "gridfs-deleteByName", + "schemaVersion": "1.0", + "createEntities": [ + { + "client": { + "id": "client0" + } + }, + { + "database": { + "id": "database0", + "client": "client0", + "databaseName": "gridfs-tests" + } + }, + { + "bucket": { + "id": "bucket0", + "database": "database0" + } + }, + { + "collection": { + "id": "bucket0_files_collection", + "database": "database0", + "collectionName": "fs.files" + } + }, + { + "collection": { + "id": "bucket0_chunks_collection", + "database": "database0", + "collectionName": "fs.chunks" + } + } + ], + "initialData": [ + { + "collectionName": "fs.files", + "databaseName": "gridfs-tests", + "documents": [ + { + "_id": { + "$oid": "000000000000000000000001" + }, + "length": 0, + "chunkSize": 4, + "uploadDate": { + "$date": "1970-01-01T00:00:00.000Z" + }, + "filename": "filename", + "metadata": {} + }, + { + "_id": { + "$oid": "000000000000000000000002" + }, + "length": 0, + "chunkSize": 4, + "uploadDate": { + "$date": "1970-01-01T00:00:00.000Z" + }, + "filename": "filename", + "metadata": {} + }, + { + "_id": { + "$oid": "000000000000000000000003" + }, + "length": 2, + "chunkSize": 4, + "uploadDate": { + "$date": "1970-01-01T00:00:00.000Z" + }, + "filename": "filename", + "metadata": {} + }, + { + "_id": { + "$oid": "000000000000000000000004" + }, + "length": 8, + "chunkSize": 4, + "uploadDate": { + "$date": "1970-01-01T00:00:00.000Z" + }, + "filename": "otherfilename", + "metadata": {} + } + ] + }, + { + "collectionName": "fs.chunks", + "databaseName": "gridfs-tests", + "documents": [ + { + "_id": { + "$oid": "000000000000000000000001" + }, + "files_id": { + "$oid": "000000000000000000000002" + }, + "n": 0, + "data": { + "$binary": { + "base64": "", + "subType": "00" + } + } + }, + { + "_id": { + "$oid": "000000000000000000000002" + }, + "files_id": { + "$oid": "000000000000000000000003" + }, + "n": 0, + "data": { + "$binary": { + "base64": "", + "subType": "00" + } + } + }, + { + "_id": { + "$oid": "000000000000000000000003" + }, + "files_id": { + "$oid": "000000000000000000000003" + }, + "n": 0, + "data": { + "$binary": { + "base64": "", + "subType": "00" + } + } + }, + { + "_id": { + "$oid": "000000000000000000000004" + }, + "files_id": { + "$oid": "000000000000000000000004" + }, + "n": 0, + "data": { + "$binary": { + "base64": "", + "subType": "00" + } + } + } + ] + } + ], + "tests": [ + { + "description": "delete when multiple revisions of the file exist", + "operations": [ + { + "name": "deleteByName", + "object": "bucket0", + "arguments": { + "filename": "filename" + } + } + ], + "outcome": [ + { + "collectionName": "fs.files", + "databaseName": "gridfs-tests", + "documents": [ + { + "_id": { + "$oid": "000000000000000000000004" + }, + "length": 8, + "chunkSize": 4, + "uploadDate": { + "$date": "1970-01-01T00:00:00.000Z" + }, + "filename": "otherfilename", + "metadata": {} + } + ] + }, + { + "collectionName": "fs.chunks", + "databaseName": "gridfs-tests", + "documents": [ + { + "_id": { + "$oid": "000000000000000000000004" + }, + "files_id": { + "$oid": "000000000000000000000004" + }, + "n": 0, + "data": { + "$binary": { + "base64": "", + "subType": "00" + } + } + } + ] + } + ] + }, + { + "description": "delete when file name does not exist", + "operations": [ + { + "name": "deleteByName", + "object": "bucket0", + "arguments": { + "filename": "missing-file" + }, + "expectError": { + "isClientError": true + } + } + ] + } + ] +} diff --git a/test/unified_format.py b/test/unified_format.py index 471a067bee..2ff24c737b 100644 --- a/test/unified_format.py +++ b/test/unified_format.py @@ -65,7 +65,7 @@ from bson import SON, json_util from bson.codec_options import DEFAULT_CODEC_OPTIONS from bson.objectid import ObjectId -from gridfs import GridFSBucket, GridOut +from gridfs import GridFSBucket, GridOut, NoFile from pymongo import ASCENDING, CursorType, MongoClient, _csot from pymongo.encryption_options import _HAVE_PYMONGOCRYPT from pymongo.errors import ( @@ -629,6 +629,9 @@ def process_error(self, exception, spec): self.assertNotIsInstance(error, NotPrimaryError) elif isinstance(error, (InvalidOperation, ConfigurationError, EncryptionError)): pass + # gridfs NoFile errors are considered client errors. + elif isinstance(error, NoFile): + pass else: self.assertNotIsInstance(error, PyMongoError) From 3cfa69c05bfde799e2ab766d69dc58fb4b96734b Mon Sep 17 00:00:00 2001 From: Noah Stapp Date: Mon, 24 Mar 2025 12:07:21 -0400 Subject: [PATCH 2/6] Typing fixes --- gridfs/asynchronous/grid_file.py | 4 ++-- gridfs/synchronous/grid_file.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/gridfs/asynchronous/grid_file.py b/gridfs/asynchronous/grid_file.py index d0eab10d3f..6c9d5f16c7 100644 --- a/gridfs/asynchronous/grid_file.py +++ b/gridfs/asynchronous/grid_file.py @@ -857,8 +857,8 @@ async def delete_by_name( .. versionadded:: 4.13 """ _disallow_transactions(session) - file_ids = self._files.find({"filename": filename}, {"_id": 1}, session=session) - file_ids = [file_id["_id"] async for file_id in file_ids] + files = self._files.find({"filename": filename}, {"_id": 1}, session=session) + file_ids = [file_id["_id"] async for file_id in files] res = await self._files.delete_many({"_id": {"$in": file_ids}}, session=session) await self._chunks.delete_many({"files_id": {"$in": file_ids}}, session=session) if not res.deleted_count: diff --git a/gridfs/synchronous/grid_file.py b/gridfs/synchronous/grid_file.py index f8c7b2748e..8d784e1a34 100644 --- a/gridfs/synchronous/grid_file.py +++ b/gridfs/synchronous/grid_file.py @@ -851,8 +851,8 @@ def delete_by_name(self, filename: str, session: Optional[ClientSession] = None) .. versionadded:: 4.13 """ _disallow_transactions(session) - file_ids = self._files.find({"filename": filename}, {"_id": 1}, session=session) - file_ids = [file_id["_id"] for file_id in file_ids] + files = self._files.find({"filename": filename}, {"_id": 1}, session=session) + file_ids = [file_id["_id"] for file_id in files] res = self._files.delete_many({"_id": {"$in": file_ids}}, session=session) self._chunks.delete_many({"files_id": {"$in": file_ids}}, session=session) if not res.deleted_count: From e98c5afdbc3beaa40febab7a477cf43d1dd82907 Mon Sep 17 00:00:00 2001 From: Noah Stapp Date: Tue, 25 Mar 2025 09:41:41 -0400 Subject: [PATCH 3/6] Add prose test --- gridfs/asynchronous/grid_file.py | 9 ++++----- gridfs/synchronous/grid_file.py | 9 ++++----- test/asynchronous/test_gridfs_bucket.py | 11 +++++++++++ test/asynchronous/unified_format.py | 5 +---- test/test_gridfs_bucket.py | 11 +++++++++++ test/unified_format.py | 5 +---- 6 files changed, 32 insertions(+), 18 deletions(-) diff --git a/gridfs/asynchronous/grid_file.py b/gridfs/asynchronous/grid_file.py index 6c9d5f16c7..04b6d0b8b0 100644 --- a/gridfs/asynchronous/grid_file.py +++ b/gridfs/asynchronous/grid_file.py @@ -848,13 +848,12 @@ async def delete_by_name( await fs.upload_from_stream("test_file", "data I want to store!") await fs.delete_by_name("test_file") - Raises :exc:`~gridfs.errors.NoFile` if no file with filename exists. + Raises :exc:`~gridfs.errors.NoFile` if no file with the given filename exists. :param filename: The name of the file to be deleted. - :param session: a - :class:`~pymongo.client_session.AsyncClientSession` + :param session: a :class:`~pymongo.client_session.AsyncClientSession` - .. versionadded:: 4.13 + .. versionadded:: 4.12 """ _disallow_transactions(session) files = self._files.find({"filename": filename}, {"_id": 1}, session=session) @@ -862,7 +861,7 @@ async def delete_by_name( res = await self._files.delete_many({"_id": {"$in": file_ids}}, session=session) await self._chunks.delete_many({"files_id": {"$in": file_ids}}, session=session) if not res.deleted_count: - raise NoFile("no file could be deleted because none matched %s" % filename) + raise NoFile(f"no file could be deleted because none matched filename {filename!r}") def find(self, *args: Any, **kwargs: Any) -> AsyncGridOutCursor: """Find and return the files collection documents that match ``filter`` diff --git a/gridfs/synchronous/grid_file.py b/gridfs/synchronous/grid_file.py index 8d784e1a34..9f157e058e 100644 --- a/gridfs/synchronous/grid_file.py +++ b/gridfs/synchronous/grid_file.py @@ -842,13 +842,12 @@ def delete_by_name(self, filename: str, session: Optional[ClientSession] = None) fs.upload_from_stream("test_file", "data I want to store!") fs.delete_by_name("test_file") - Raises :exc:`~gridfs.errors.NoFile` if no file with filename exists. + Raises :exc:`~gridfs.errors.NoFile` if no file with the given filename exists. :param filename: The name of the file to be deleted. - :param session: a - :class:`~pymongo.client_session.ClientSession` + :param session: a :class:`~pymongo.client_session.ClientSession` - .. versionadded:: 4.13 + .. versionadded:: 4.12 """ _disallow_transactions(session) files = self._files.find({"filename": filename}, {"_id": 1}, session=session) @@ -856,7 +855,7 @@ def delete_by_name(self, filename: str, session: Optional[ClientSession] = None) res = self._files.delete_many({"_id": {"$in": file_ids}}, session=session) self._chunks.delete_many({"files_id": {"$in": file_ids}}, session=session) if not res.deleted_count: - raise NoFile("no file could be deleted because none matched %s" % filename) + raise NoFile(f"no file could be deleted because none matched filename {filename!r}") def find(self, *args: Any, **kwargs: Any) -> GridOutCursor: """Find and return the files collection documents that match ``filter`` diff --git a/test/asynchronous/test_gridfs_bucket.py b/test/asynchronous/test_gridfs_bucket.py index 29877ee9c4..03d49d5c3d 100644 --- a/test/asynchronous/test_gridfs_bucket.py +++ b/test/asynchronous/test_gridfs_bucket.py @@ -115,6 +115,17 @@ async def test_multi_chunk_delete(self): self.assertEqual(0, await self.db.fs.files.count_documents({})) self.assertEqual(0, await self.db.fs.chunks.count_documents({})) + async def test_delete_by_name(self): + self.assertEqual(0, await self.db.fs.files.count_documents({})) + self.assertEqual(0, await self.db.fs.chunks.count_documents({})) + gfs = gridfs.AsyncGridFSBucket(self.db) + await gfs.upload_from_stream("test_filename", b"hello", chunk_size_bytes=1) + self.assertEqual(1, await self.db.fs.files.count_documents({})) + self.assertEqual(5, await self.db.fs.chunks.count_documents({})) + await gfs.delete_by_name("test_filename") + self.assertEqual(0, await self.db.fs.files.count_documents({})) + self.assertEqual(0, await self.db.fs.chunks.count_documents({})) + async def test_empty_file(self): oid = await self.fs.upload_from_stream("test_filename", b"") self.assertEqual(b"", await (await self.fs.open_download_stream(oid)).read()) diff --git a/test/asynchronous/unified_format.py b/test/asynchronous/unified_format.py index 3d63ced9d8..5b784e7881 100644 --- a/test/asynchronous/unified_format.py +++ b/test/asynchronous/unified_format.py @@ -628,10 +628,7 @@ def process_error(self, exception, spec): # Connection errors are considered client errors. if isinstance(error, ConnectionFailure): self.assertNotIsInstance(error, NotPrimaryError) - elif isinstance(error, (InvalidOperation, ConfigurationError, EncryptionError)): - pass - # gridfs NoFile errors are considered client errors. - elif isinstance(error, NoFile): + elif isinstance(error, (InvalidOperation, ConfigurationError, EncryptionError, NoFile)): pass else: self.assertNotIsInstance(error, PyMongoError) diff --git a/test/test_gridfs_bucket.py b/test/test_gridfs_bucket.py index d68c9f6ba2..04063a213d 100644 --- a/test/test_gridfs_bucket.py +++ b/test/test_gridfs_bucket.py @@ -115,6 +115,17 @@ def test_multi_chunk_delete(self): self.assertEqual(0, self.db.fs.files.count_documents({})) self.assertEqual(0, self.db.fs.chunks.count_documents({})) + def test_delete_by_name(self): + self.assertEqual(0, self.db.fs.files.count_documents({})) + self.assertEqual(0, self.db.fs.chunks.count_documents({})) + gfs = gridfs.GridFSBucket(self.db) + gfs.upload_from_stream("test_filename", b"hello", chunk_size_bytes=1) + self.assertEqual(1, self.db.fs.files.count_documents({})) + self.assertEqual(5, self.db.fs.chunks.count_documents({})) + gfs.delete_by_name("test_filename") + self.assertEqual(0, self.db.fs.files.count_documents({})) + self.assertEqual(0, self.db.fs.chunks.count_documents({})) + def test_empty_file(self): oid = self.fs.upload_from_stream("test_filename", b"") self.assertEqual(b"", (self.fs.open_download_stream(oid)).read()) diff --git a/test/unified_format.py b/test/unified_format.py index 2ff24c737b..797bacab9e 100644 --- a/test/unified_format.py +++ b/test/unified_format.py @@ -627,10 +627,7 @@ def process_error(self, exception, spec): # Connection errors are considered client errors. if isinstance(error, ConnectionFailure): self.assertNotIsInstance(error, NotPrimaryError) - elif isinstance(error, (InvalidOperation, ConfigurationError, EncryptionError)): - pass - # gridfs NoFile errors are considered client errors. - elif isinstance(error, NoFile): + elif isinstance(error, (InvalidOperation, ConfigurationError, EncryptionError, NoFile)): pass else: self.assertNotIsInstance(error, PyMongoError) From 370ba42a6f7909f9d5be1a47d278bf10c5b3b230 Mon Sep 17 00:00:00 2001 From: Noah Stapp Date: Thu, 27 Mar 2025 09:58:07 -0400 Subject: [PATCH 4/6] Address review --- doc/changelog.rst | 2 ++ gridfs/asynchronous/grid_file.py | 2 +- gridfs/synchronous/grid_file.py | 2 +- test/asynchronous/test_session.py | 5 ++++- test/asynchronous/test_transactions.py | 3 ++- test/test_session.py | 5 ++++- test/test_transactions.py | 3 ++- 7 files changed, 16 insertions(+), 6 deletions(-) diff --git a/doc/changelog.rst b/doc/changelog.rst index b172da6b8e..ee763437cd 100644 --- a/doc/changelog.rst +++ b/doc/changelog.rst @@ -9,6 +9,8 @@ PyMongo 4.12 brings a number of changes including: - Support for configuring DEK cache lifetime via the ``key_expiration_ms`` argument to :class:`~pymongo.encryption_options.AutoEncryptionOpts`. - Support for $lookup in CSFLE and QE supported on MongoDB 8.1+. +- Added :meth:`gridfs.asynchronous.grid_file.AsyncGridFSBucket.delete_by_name` and :meth:`gridfs.grid_file.GridFSBucket.delete_by_name` + for more performant deletion of a file with multiple revisions. Issues Resolved ............... diff --git a/gridfs/asynchronous/grid_file.py b/gridfs/asynchronous/grid_file.py index 04b6d0b8b0..d634eb745a 100644 --- a/gridfs/asynchronous/grid_file.py +++ b/gridfs/asynchronous/grid_file.py @@ -857,7 +857,7 @@ async def delete_by_name( """ _disallow_transactions(session) files = self._files.find({"filename": filename}, {"_id": 1}, session=session) - file_ids = [file_id["_id"] async for file_id in files] + file_ids = [file["_id"] async for file in files] res = await self._files.delete_many({"_id": {"$in": file_ids}}, session=session) await self._chunks.delete_many({"files_id": {"$in": file_ids}}, session=session) if not res.deleted_count: diff --git a/gridfs/synchronous/grid_file.py b/gridfs/synchronous/grid_file.py index 9f157e058e..c5c3c62cde 100644 --- a/gridfs/synchronous/grid_file.py +++ b/gridfs/synchronous/grid_file.py @@ -851,7 +851,7 @@ def delete_by_name(self, filename: str, session: Optional[ClientSession] = None) """ _disallow_transactions(session) files = self._files.find({"filename": filename}, {"_id": 1}, session=session) - file_ids = [file_id["_id"] for file_id in files] + file_ids = [file["_id"] for file in files] res = self._files.delete_many({"_id": {"$in": file_ids}}, session=session) self._chunks.delete_many({"files_id": {"$in": file_ids}}, session=session) if not res.deleted_count: diff --git a/test/asynchronous/test_session.py b/test/asynchronous/test_session.py index 4431cbcb16..476564224d 100644 --- a/test/asynchronous/test_session.py +++ b/test/asynchronous/test_session.py @@ -45,7 +45,7 @@ from bson import DBRef from gridfs.asynchronous.grid_file import AsyncGridFS, AsyncGridFSBucket -from pymongo import ASCENDING, AsyncMongoClient, monitoring +from pymongo import ASCENDING, AsyncMongoClient, _csot, monitoring from pymongo.asynchronous.command_cursor import AsyncCommandCursor from pymongo.asynchronous.cursor import AsyncCursor from pymongo.asynchronous.helpers import anext @@ -458,6 +458,7 @@ async def test_cursor(self): f"{name} sent wrong lsid with {event.command_name}", ) + @_csot.apply async def test_gridfs(self): client = self.client fs = AsyncGridFS(client.pymongo_test) @@ -500,6 +501,7 @@ async def find_list(session=None): (fs.delete, [1], {}), ) + @_csot.apply async def test_gridfs_bucket(self): client = self.client bucket = AsyncGridFSBucket(client.pymongo_test) @@ -546,6 +548,7 @@ async def find(session=None): (bucket.delete, [2], {}), ) + @_csot.apply async def test_gridfsbucket_cursor(self): client = self.client bucket = AsyncGridFSBucket(client.pymongo_test) diff --git a/test/asynchronous/test_transactions.py b/test/asynchronous/test_transactions.py index 884110cd45..2b84d22e86 100644 --- a/test/asynchronous/test_transactions.py +++ b/test/asynchronous/test_transactions.py @@ -32,7 +32,7 @@ from bson import encode from bson.raw_bson import RawBSONDocument -from pymongo import WriteConcern +from pymongo import WriteConcern, _csot from pymongo.asynchronous import client_session from pymongo.asynchronous.client_session import TransactionOptions from pymongo.asynchronous.command_cursor import AsyncCommandCursor @@ -240,6 +240,7 @@ async def create_and_insert(session): self.assertEqual(ctx.exception.code, 48) # NamespaceExists @async_client_context.require_transactions + @_csot.apply async def test_gridfs_does_not_support_transactions(self): client = async_client_context.client db = client.pymongo_test diff --git a/test/test_session.py b/test/test_session.py index 905539a1f8..b248710157 100644 --- a/test/test_session.py +++ b/test/test_session.py @@ -45,7 +45,7 @@ from bson import DBRef from gridfs.synchronous.grid_file import GridFS, GridFSBucket -from pymongo import ASCENDING, MongoClient, monitoring +from pymongo import ASCENDING, MongoClient, _csot, monitoring from pymongo.common import _MAX_END_SESSIONS from pymongo.errors import ConfigurationError, InvalidOperation, OperationFailure from pymongo.operations import IndexModel, InsertOne, UpdateOne @@ -458,6 +458,7 @@ def test_cursor(self): f"{name} sent wrong lsid with {event.command_name}", ) + @_csot.apply def test_gridfs(self): client = self.client fs = GridFS(client.pymongo_test) @@ -500,6 +501,7 @@ def find_list(session=None): (fs.delete, [1], {}), ) + @_csot.apply def test_gridfs_bucket(self): client = self.client bucket = GridFSBucket(client.pymongo_test) @@ -546,6 +548,7 @@ def find(session=None): (bucket.delete, [2], {}), ) + @_csot.apply def test_gridfsbucket_cursor(self): client = self.client bucket = GridFSBucket(client.pymongo_test) diff --git a/test/test_transactions.py b/test/test_transactions.py index 80b3e3765e..83a66a65b6 100644 --- a/test/test_transactions.py +++ b/test/test_transactions.py @@ -32,7 +32,7 @@ from bson import encode from bson.raw_bson import RawBSONDocument -from pymongo import WriteConcern +from pymongo import WriteConcern, _csot from pymongo.errors import ( CollectionInvalid, ConfigurationError, @@ -232,6 +232,7 @@ def create_and_insert(session): self.assertEqual(ctx.exception.code, 48) # NamespaceExists @client_context.require_transactions + @_csot.apply def test_gridfs_does_not_support_transactions(self): client = client_context.client db = client.pymongo_test From fc1813b6b0aa8418e653b8cdc1a225b4a3c2d267 Mon Sep 17 00:00:00 2001 From: Noah Stapp Date: Thu, 27 Mar 2025 10:29:24 -0400 Subject: [PATCH 5/6] Undo csot.apply additions --- test/asynchronous/test_session.py | 3 --- test/asynchronous/test_transactions.py | 1 - test/test_session.py | 3 --- test/test_transactions.py | 1 - 4 files changed, 8 deletions(-) diff --git a/test/asynchronous/test_session.py b/test/asynchronous/test_session.py index 476564224d..3148dbd224 100644 --- a/test/asynchronous/test_session.py +++ b/test/asynchronous/test_session.py @@ -458,7 +458,6 @@ async def test_cursor(self): f"{name} sent wrong lsid with {event.command_name}", ) - @_csot.apply async def test_gridfs(self): client = self.client fs = AsyncGridFS(client.pymongo_test) @@ -501,7 +500,6 @@ async def find_list(session=None): (fs.delete, [1], {}), ) - @_csot.apply async def test_gridfs_bucket(self): client = self.client bucket = AsyncGridFSBucket(client.pymongo_test) @@ -548,7 +546,6 @@ async def find(session=None): (bucket.delete, [2], {}), ) - @_csot.apply async def test_gridfsbucket_cursor(self): client = self.client bucket = AsyncGridFSBucket(client.pymongo_test) diff --git a/test/asynchronous/test_transactions.py b/test/asynchronous/test_transactions.py index 2b84d22e86..20e01ec287 100644 --- a/test/asynchronous/test_transactions.py +++ b/test/asynchronous/test_transactions.py @@ -240,7 +240,6 @@ async def create_and_insert(session): self.assertEqual(ctx.exception.code, 48) # NamespaceExists @async_client_context.require_transactions - @_csot.apply async def test_gridfs_does_not_support_transactions(self): client = async_client_context.client db = client.pymongo_test diff --git a/test/test_session.py b/test/test_session.py index b248710157..aa76b20de6 100644 --- a/test/test_session.py +++ b/test/test_session.py @@ -458,7 +458,6 @@ def test_cursor(self): f"{name} sent wrong lsid with {event.command_name}", ) - @_csot.apply def test_gridfs(self): client = self.client fs = GridFS(client.pymongo_test) @@ -501,7 +500,6 @@ def find_list(session=None): (fs.delete, [1], {}), ) - @_csot.apply def test_gridfs_bucket(self): client = self.client bucket = GridFSBucket(client.pymongo_test) @@ -548,7 +546,6 @@ def find(session=None): (bucket.delete, [2], {}), ) - @_csot.apply def test_gridfsbucket_cursor(self): client = self.client bucket = GridFSBucket(client.pymongo_test) diff --git a/test/test_transactions.py b/test/test_transactions.py index 83a66a65b6..a7b0d74c88 100644 --- a/test/test_transactions.py +++ b/test/test_transactions.py @@ -232,7 +232,6 @@ def create_and_insert(session): self.assertEqual(ctx.exception.code, 48) # NamespaceExists @client_context.require_transactions - @_csot.apply def test_gridfs_does_not_support_transactions(self): client = client_context.client db = client.pymongo_test From 3f112d51b67a9acf02756586dabb1030d23948b5 Mon Sep 17 00:00:00 2001 From: Noah Stapp Date: Thu, 27 Mar 2025 15:40:00 -0400 Subject: [PATCH 6/6] Address review --- test/asynchronous/test_session.py | 2 +- test/asynchronous/test_transactions.py | 1 + test/test_session.py | 2 +- test/test_transactions.py | 1 + 4 files changed, 4 insertions(+), 2 deletions(-) diff --git a/test/asynchronous/test_session.py b/test/asynchronous/test_session.py index 3148dbd224..3c249718ce 100644 --- a/test/asynchronous/test_session.py +++ b/test/asynchronous/test_session.py @@ -543,7 +543,7 @@ async def find(session=None): (bucket.rename, [1, "f2"], {}), # Delete both files so _test_ops can run these operations twice. (bucket.delete, [1], {}), - (bucket.delete, [2], {}), + (bucket.delete_by_name, ["f"], {}), ) async def test_gridfsbucket_cursor(self): diff --git a/test/asynchronous/test_transactions.py b/test/asynchronous/test_transactions.py index 20e01ec287..ea4d1e3e6c 100644 --- a/test/asynchronous/test_transactions.py +++ b/test/asynchronous/test_transactions.py @@ -295,6 +295,7 @@ async def gridfs_open_upload_stream(*args, **kwargs): "new-name", ), ), + (bucket.delete_by_name, ("new-name",)), ] async with client.start_session() as s, await s.start_transaction(): diff --git a/test/test_session.py b/test/test_session.py index aa76b20de6..ec25a735e7 100644 --- a/test/test_session.py +++ b/test/test_session.py @@ -543,7 +543,7 @@ def find(session=None): (bucket.rename, [1, "f2"], {}), # Delete both files so _test_ops can run these operations twice. (bucket.delete, [1], {}), - (bucket.delete, [2], {}), + (bucket.delete_by_name, ["f"], {}), ) def test_gridfsbucket_cursor(self): diff --git a/test/test_transactions.py b/test/test_transactions.py index a7b0d74c88..c549b743be 100644 --- a/test/test_transactions.py +++ b/test/test_transactions.py @@ -287,6 +287,7 @@ def gridfs_open_upload_stream(*args, **kwargs): "new-name", ), ), + (bucket.delete_by_name, ("new-name",)), ] with client.start_session() as s, s.start_transaction():