diff --git a/doc/changelog.rst b/doc/changelog.rst index 8a54c78f27..9e1c65e108 100644 --- a/doc/changelog.rst +++ b/doc/changelog.rst @@ -53,6 +53,13 @@ Version 3.11 adds support for MongoDB 4.4. Highlights include: :meth:`~pymongo.database.Database.command` to run the ``reIndex`` command instead. +Unavoidable breaking changes: + +- :class:`~gridfs.GridFSBucket` and :class:`~gridfs.GridFS` do not support + multi-document transactions. Running a GridFS operation in a transaction + now always raises the following error: + ``InvalidOperation: GridFS does not support multi-document transactions`` + .. _validate command: https://docs.mongodb.com/manual/reference/command/validate/ Issues Resolved diff --git a/gridfs/__init__.py b/gridfs/__init__.py index 6c56a605e6..f291e8c9ca 100644 --- a/gridfs/__init__.py +++ b/gridfs/__init__.py @@ -26,7 +26,8 @@ GridOut, GridOutCursor, DEFAULT_CHUNK_SIZE, - _clear_entity_type_registry) + _clear_entity_type_registry, + _disallow_transactions) from pymongo import (ASCENDING, DESCENDING) from pymongo.common import UNAUTHORIZED_CODES, validate_string @@ -50,6 +51,10 @@ def __init__(self, database, collection="fs", disable_md5=False): computed for uploaded files. Useful in environments where MD5 cannot be used for regulatory or other reasons. Defaults to False. + .. versionchanged:: 3.11 + Running a GridFS operation in a transaction now always raises an + error. GridFS does not support multi-document transactions. + .. versionchanged:: 3.1 Indexes are only ensured on the first write to the DB. @@ -68,7 +73,6 @@ def __init__(self, database, collection="fs", disable_md5=False): raise ConfigurationError('database must use ' 'acknowledged write_concern') - self.__database = database self.__collection = database[collection] self.__files = self.__collection.files self.__chunks = self.__collection.chunks @@ -88,8 +92,6 @@ def new_file(self, **kwargs): :Parameters: - `**kwargs` (optional): keyword arguments for file creation """ - # No need for __ensure_index_files_id() here; GridIn ensures - # the (files_id, n) index when needed. return GridIn( self.__collection, disable_md5=self.__disable_md5, **kwargs) @@ -192,6 +194,7 @@ def get_version(self, filename=None, version=-1, session=None, **kwargs): if filename is not None: query["filename"] = filename + _disallow_transactions(session) cursor = self.__files.find(query, session=session) if version < 0: skip = abs(version) - 1 @@ -249,6 +252,7 @@ def delete(self, file_id, session=None): .. versionchanged:: 3.1 ``delete`` no longer ensures indexes. """ + _disallow_transactions(session) self.__files.delete_one({"_id": file_id}, session=session) self.__chunks.delete_many({"files_id": file_id}, session=session) @@ -266,6 +270,7 @@ def list(self, session=None): .. versionchanged:: 3.1 ``list`` no longer ensures indexes. """ + _disallow_transactions(session) # With an index, distinct includes documents with no filename # as None. return [ @@ -299,6 +304,7 @@ def find_one(self, filter=None, session=None, *args, **kwargs): if filter is not None and not isinstance(filter, abc.Mapping): filter = {"_id": filter} + _disallow_transactions(session) for f in self.find(filter, *args, session=session, **kwargs): return f @@ -403,6 +409,7 @@ def exists(self, document_or_id=None, session=None, **kwargs): .. versionchanged:: 3.6 Added ``session`` parameter. """ + _disallow_transactions(session) if kwargs: f = self.__files.find_one(kwargs, ["_id"], session=session) else: @@ -439,6 +446,10 @@ def __init__(self, db, bucket_name="fs", computed for uploaded files. Useful in environments where MD5 cannot be used for regulatory or other reasons. Defaults to False. + .. versionchanged:: 3.11 + Running a GridFS operation in a transaction now always raises an + error. GridFSBucket does not support multi-document transactions. + .. versionadded:: 3.1 .. mongodoc:: gridfs @@ -452,7 +463,6 @@ def __init__(self, db, bucket_name="fs", if not wtc.acknowledged: raise ConfigurationError('write concern must be acknowledged') - self._db = db self._bucket_name = bucket_name self._collection = db[bucket_name] self._disable_md5 = disable_md5 @@ -746,6 +756,7 @@ def delete(self, file_id, session=None): .. versionchanged:: 3.6 Added ``session`` parameter. """ + _disallow_transactions(session) res = self._files.delete_one({"_id": file_id}, session=session) self._chunks.delete_many({"files_id": file_id}, session=session) if not res.deleted_count: @@ -839,9 +850,8 @@ def open_download_stream_by_name(self, filename, revision=-1, session=None): Added ``session`` parameter. """ validate_string("filename", filename) - query = {"filename": filename} - + _disallow_transactions(session) cursor = self._files.find(query, session=session) if revision < 0: skip = abs(revision) - 1 @@ -922,6 +932,7 @@ def rename(self, file_id, new_filename, session=None): .. versionchanged:: 3.6 Added ``session`` parameter. """ + _disallow_transactions(session) result = self._files.update_one({"_id": file_id}, {"$set": {"filename": new_filename}}, session=session) diff --git a/gridfs/grid_file.py b/gridfs/grid_file.py index 88b71ebb1f..23c629be68 100644 --- a/gridfs/grid_file.py +++ b/gridfs/grid_file.py @@ -31,6 +31,7 @@ from pymongo.errors import (ConfigurationError, CursorNotFound, DuplicateKeyError, + InvalidOperation, OperationFailure) from pymongo.read_preferences import ReadPreference @@ -105,6 +106,12 @@ def _clear_entity_type_registry(entity, **kwargs): return entity.with_options(codec_options=codecopts, **kwargs) +def _disallow_transactions(session): + if session and session.in_transaction: + raise InvalidOperation( + 'GridFS does not support multi-document transactions') + + class GridIn(object): """Class to write data to GridFS. """ @@ -168,6 +175,7 @@ def __init__( if not root_collection.write_concern.acknowledged: raise ConfigurationError('root_collection must use ' 'acknowledged write_concern') + _disallow_transactions(session) # Handle alternative naming if "content_type" in kwargs: @@ -207,6 +215,7 @@ def __create_index(self, collection, index_key, unique): def __ensure_indexes(self): if not object.__getattribute__(self, "_ensured_index"): + _disallow_transactions(self._session) self.__create_index(self._coll.files, _F_INDEX, False) self.__create_index(self._coll.chunks, _C_INDEX, True) object.__setattr__(self, "_ensured_index", True) @@ -456,6 +465,7 @@ def __init__(self, root_collection, file_id=None, file_document=None, if not isinstance(root_collection, Collection): raise TypeError("root_collection must be an " "instance of Collection") + _disallow_transactions(session) root_collection = _clear_entity_type_registry(root_collection) @@ -483,6 +493,7 @@ def __init__(self, root_collection, file_id=None, file_document=None, def _ensure_file(self): if not self._file: + _disallow_transactions(self._session) self._file = self.__files.find_one({"_id": self.__file_id}, session=self._session) if not self._file: @@ -718,6 +729,7 @@ def _create_cursor(self): filter = {"files_id": self._id} if self._next_chunk > 0: filter["n"] = {"$gte": self._next_chunk} + _disallow_transactions(self._session) self._cursor = self._chunks.find(filter, sort=[("n", 1)], session=self._session) @@ -810,6 +822,7 @@ def __init__(self, collection, filter=None, skip=0, limit=0, .. mongodoc:: cursors """ + _disallow_transactions(session) collection = _clear_entity_type_registry(collection) # Hold on to the base "fs" collection to create GridOut objects later. @@ -823,6 +836,7 @@ def __init__(self, collection, filter=None, skip=0, limit=0, def next(self): """Get next GridOut object from cursor. """ + _disallow_transactions(self.session) # Work around "super is not iterable" issue in Python 3.x next_file = super(GridOutCursor, self).next() return GridOut(self.__root_collection, file_document=next_file, diff --git a/test/test_transactions.py b/test/test_transactions.py index 85345e2748..a114db6e25 100644 --- a/test/test_transactions.py +++ b/test/test_transactions.py @@ -19,16 +19,21 @@ sys.path[0:0] = [""] +from bson.py3compat import StringIO + from pymongo import client_session, WriteConcern from pymongo.client_session import TransactionOptions from pymongo.errors import (CollectionInvalid, ConfigurationError, ConnectionFailure, + InvalidOperation, OperationFailure) from pymongo.operations import IndexModel, InsertOne from pymongo.read_concern import ReadConcern from pymongo.read_preferences import ReadPreference +from gridfs import GridFS, GridFSBucket + from test import unittest, client_context from test.utils import (rs_client, single_client, wait_until, OvertCommandListener, @@ -215,8 +220,7 @@ def test_unpin_for_non_transaction_operation(self): @client_context.require_transactions @client_context.require_version_min(4, 3, 4) def test_create_collection(self): - client = rs_client() - self.addCleanup(client.close) + client = client_context.client db = client.pymongo_test coll = db.test_create_collection self.addCleanup(coll.drop) @@ -241,6 +245,48 @@ def create_and_insert(session): db.create_collection(coll.name, session=s) self.assertEqual(ctx.exception.code, 48) # NamespaceExists + @client_context.require_transactions + def test_gridfs_does_not_support_transactions(self): + client = client_context.client + db = client.pymongo_test + gfs = GridFS(db) + bucket = GridFSBucket(db) + + def gridfs_find(*args, **kwargs): + return gfs.find(*args, **kwargs).next() + + def gridfs_open_upload_stream(*args, **kwargs): + bucket.open_upload_stream(*args, **kwargs).write(b'1') + + gridfs_ops = [ + (gfs.put, (b'123',)), + (gfs.get, (1,)), + (gfs.get_version, ('name',)), + (gfs.get_last_version, ('name',)), + (gfs.delete, (1, )), + (gfs.list, ()), + (gfs.find_one, ()), + (gridfs_find, ()), + (gfs.exists, ()), + (gridfs_open_upload_stream, ('name',)), + (bucket.upload_from_stream, ('name', b'data',)), + (bucket.download_to_stream, (1, StringIO(),)), + (bucket.download_to_stream_by_name, ('name', StringIO(),)), + (bucket.delete, (1,)), + (bucket.find, ()), + (bucket.open_download_stream, (1,)), + (bucket.open_download_stream_by_name, ('name',)), + (bucket.rename, (1, 'new-name',)), + ] + + with client.start_session() as s, s.start_transaction(): + for op, args in gridfs_ops: + with self.assertRaisesRegex( + InvalidOperation, + 'GridFS does not support multi-document transactions', + ): + op(*args, session=s) + class PatchSessionTimeout(object): """Patches the client_session's with_transaction timeout for testing."""