diff --git a/.github/workflows/test-python-atlas.yml b/.github/workflows/test-python-atlas.yml index bdb035aa..6eab2b7e 100644 --- a/.github/workflows/test-python-atlas.yml +++ b/.github/workflows/test-python-atlas.yml @@ -53,6 +53,6 @@ jobs: working-directory: . run: bash .github/workflows/start_local_atlas.sh mongodb/mongodb-atlas-local:7 - name: Run tests - run: python3 django_repo/tests/runtests.py --settings mongodb_settings -v 2 + run: python3 django_repo/tests/runtests_.py permissions: contents: read diff --git a/django_mongodb_backend/base.py b/django_mongodb_backend/base.py index 683afe60..7f337cf8 100644 --- a/django_mongodb_backend/base.py +++ b/django_mongodb_backend/base.py @@ -2,9 +2,7 @@ import os from django.core.exceptions import ImproperlyConfigured -from django.db import DEFAULT_DB_ALIAS from django.db.backends.base.base import BaseDatabaseWrapper -from django.db.backends.utils import debug_transaction from django.utils.asyncio import async_unsafe from django.utils.functional import cached_property from pymongo.collection import Collection @@ -34,17 +32,6 @@ def __exit__(self, exception_type, exception_value, exception_traceback): pass -def requires_transaction_support(func): - """Make a method a no-op if transactions aren't supported.""" - - def wrapper(self, *args, **kwargs): - if not self.features.supports_transactions: - return - func(self, *args, **kwargs) - - return wrapper - - class DatabaseWrapper(BaseDatabaseWrapper): data_types = { "AutoField": "int", @@ -155,15 +142,6 @@ def _isnull_operator(a, b): ops_class = DatabaseOperations validation_class = DatabaseValidation - def __init__(self, settings_dict, alias=DEFAULT_DB_ALIAS): - super().__init__(settings_dict, alias=alias) - self.session = None - - def check_settings(self): - super().check_settings() - if not self.settings_dict["AUTOCOMMIT"]: - raise ImproperlyConfigured("MongoDB does not support AUTOCOMMIT=False.") - def get_collection(self, name, **kwargs): collection = Collection(self.database, name, **kwargs) if self.queries_logged: @@ -213,48 +191,14 @@ def _driver_info(self): return DriverInfo("django-mongodb-backend", django_mongodb_backend_version) return None - @requires_transaction_support def _commit(self): - if self.session: - with debug_transaction(self, "session.commit_transaction()"): - self.session.commit_transaction() - self._end_session() + pass - @requires_transaction_support def _rollback(self): - if self.session: - with debug_transaction(self, "session.abort_transaction()"): - self.session.abort_transaction() - self._end_session() - - def _start_transaction(self): - # Private API, specific to this backend. - if self.session is None: - self.session = self.connection.start_session() - with debug_transaction(self, "session.start_transaction()"): - self.session.start_transaction() - - def _end_session(self): - # Private API, specific to this backend. - self.session.end_session() - self.session = None - - @requires_transaction_support - def _start_transaction_under_autocommit(self): - # Implementing this hook (intended only for SQLite), allows - # BaseDatabaseWrapper.set_autocommit() to use it to start a transaction - # rather than set_autocommit(), bypassing set_autocommit()'s call to - # debug_transaction(self, "BEGIN") which isn't semantic for a no-SQL - # backend. - self._start_transaction() + pass - @requires_transaction_support - def _set_autocommit(self, autocommit, force_begin_transaction_with_broken_autocommit=False): - # Besides @transaction.atomic() (which uses - # _start_transaction_under_autocommit(), disabling autocommit is - # another way to start a transaction. - if not autocommit: - self._start_transaction() + def set_autocommit(self, autocommit, force_begin_transaction_with_broken_autocommit=False): + self.autocommit = autocommit def _close(self): # Normally called by close(), this method is also called by some tests. @@ -268,10 +212,6 @@ def close(self): def close_pool(self): """Close the MongoClient.""" - # Clear commit hooks and session. - self.run_on_commit = [] - if self.session: - self._end_session() connection = self.connection if connection is None: return @@ -287,10 +227,6 @@ def close_pool(self): def cursor(self): return Cursor() - @requires_transaction_support - def validate_no_broken_transaction(self): - super().validate_no_broken_transaction() - def get_database_version(self): """Return a tuple of the database's version.""" return tuple(self.connection.server_info()["versionArray"]) diff --git a/django_mongodb_backend/compiler.py b/django_mongodb_backend/compiler.py index 7e124828..e74f6a85 100644 --- a/django_mongodb_backend/compiler.py +++ b/django_mongodb_backend/compiler.py @@ -678,10 +678,7 @@ def execute_sql(self, returning_fields=None): @wrap_database_errors def insert(self, docs, returning_fields=None): """Store a list of documents using field columns as element names.""" - self.connection.validate_no_broken_transaction() - inserted_ids = self.collection.insert_many( - docs, session=self.connection.session - ).inserted_ids + inserted_ids = self.collection.insert_many(docs).inserted_ids return [(x,) for x in inserted_ids] if returning_fields else [] @cached_property @@ -762,10 +759,7 @@ def execute_sql(self, result_type): @wrap_database_errors def update(self, criteria, pipeline): - self.connection.validate_no_broken_transaction() - return self.collection.update_many( - criteria, pipeline, session=self.connection.session - ).matched_count + return self.collection.update_many(criteria, pipeline).matched_count def check_query(self): super().check_query() diff --git a/django_mongodb_backend/features.py b/django_mongodb_backend/features.py index 3e9cc292..9f0245ec 100644 --- a/django_mongodb_backend/features.py +++ b/django_mongodb_backend/features.py @@ -36,6 +36,10 @@ class DatabaseFeatures(BaseDatabaseFeatures): supports_temporal_subtraction = True # MongoDB stores datetimes in UTC. supports_timezones = False + # While MongoDB supports transactions in some configurations (see + # DatabaseFeatures._supports_transactions), django.db.transaction.atomic() is a + # no-op on this backend. + supports_transactions = False supports_unspecified_pk = True uses_savepoints = False @@ -92,36 +96,6 @@ class DatabaseFeatures(BaseDatabaseFeatures): "expressions.tests.ExpressionOperatorTests.test_lefthand_bitwise_xor_right_null", "expressions.tests.ExpressionOperatorTests.test_lefthand_transformed_field_bitwise_or", } - _django_test_expected_failures_no_transactions = { - # "Save with update_fields did not affect any rows." instead of - # "An error occurred in the current transaction. You can't execute - # queries until the end of the 'atomic' block." - "basic.tests.SelectOnSaveTests.test_select_on_save_lying_update", - } - _django_test_expected_failures_transactions = { - # When update_or_create() fails with IntegrityError, the transaction - # is no longer usable. - "get_or_create.tests.UpdateOrCreateTests.test_manual_primary_key_test", - "get_or_create.tests.UpdateOrCreateTestsWithManualPKs.test_create_with_duplicate_primary_key", - # Tests that require savepoints - "admin_views.tests.AdminViewBasicTest.test_disallowed_to_field", - "admin_views.tests.AdminViewPermissionsTest.test_add_view", - "admin_views.tests.AdminViewPermissionsTest.test_change_view", - "admin_views.tests.AdminViewPermissionsTest.test_change_view_save_as_new", - "admin_views.tests.AdminViewPermissionsTest.test_delete_view", - "auth_tests.test_views.ChangelistTests.test_view_user_password_is_readonly", - "fixtures.tests.FixtureLoadingTests.test_loaddata_app_option", - "fixtures.tests.FixtureLoadingTests.test_unmatched_identifier_loading", - "fixtures_model_package.tests.FixtureTestCase.test_loaddata", - "get_or_create.tests.GetOrCreateTests.test_get_or_create_invalid_params", - "get_or_create.tests.UpdateOrCreateTests.test_integrity", - "many_to_many.tests.ManyToManyTests.test_add", - "many_to_one.tests.ManyToOneTests.test_fk_assignment_and_related_object_cache", - "model_fields.test_booleanfield.BooleanFieldTests.test_null_default", - "model_fields.test_floatfield.TestFloatField.test_float_validates_object", - "multiple_database.tests.QueryTestCase.test_generic_key_cross_database_protection", - "multiple_database.tests.QueryTestCase.test_m2m_cross_database_protection", - } @cached_property def django_test_expected_failures(self): @@ -129,10 +103,6 @@ def django_test_expected_failures(self): expected_failures.update(self._django_test_expected_failures) if not self.is_mongodb_6_3: expected_failures.update(self._django_test_expected_failures_bitwise) - if self.supports_transactions: - expected_failures.update(self._django_test_expected_failures_transactions) - else: - expected_failures.update(self._django_test_expected_failures_no_transactions) return expected_failures django_test_skips = { @@ -515,6 +485,17 @@ def django_test_expected_failures(self): "Connection health checks not implemented.": { "backends.base.test_base.ConnectionHealthChecksTests", }, + "transaction.atomic() is not supported.": { + "backends.base.test_base.DatabaseWrapperLoggingTests", + "basic.tests.SelectOnSaveTests.test_select_on_save_lying_update", + "migrations.test_executor.ExecutorTests.test_atomic_operation_in_non_atomic_migration", + "migrations.test_operations.OperationTests.test_run_python_atomic", + }, + "transaction.rollback() is not supported.": { + "transactions.tests.AtomicMiscTests.test_mark_for_rollback_on_error_in_autocommit", + "transactions.tests.AtomicMiscTests.test_mark_for_rollback_on_error_in_transaction", + "transactions.tests.NonAutocommitTests.test_orm_query_after_error_and_rollback", + }, "migrate --fake-initial is not supported.": { "migrations.test_commands.MigrateTests.test_migrate_fake_initial", "migrations.test_commands.MigrateTests.test_migrate_fake_split_initial", @@ -609,15 +590,11 @@ def supports_atlas_search(self): return True @cached_property - def supports_select_union(self): - # Stage not supported inside of a multi-document transaction: $unionWith - return not self.supports_transactions - - @cached_property - def supports_transactions(self): + def _supports_transactions(self): """ Transactions are enabled if MongoDB is configured as a replica set or a - sharded cluster. + sharded cluster. This is a MongoDB-specific feature flag distinct from Django's + supports_transactions (without the underscore prefix). """ self.connection.ensure_connection() client = self.connection.connection.admin diff --git a/django_mongodb_backend/query.py b/django_mongodb_backend/query.py index d59bc163..04977520 100644 --- a/django_mongodb_backend/query.py +++ b/django_mongodb_backend/query.py @@ -61,12 +61,9 @@ def __repr__(self): @wrap_database_errors def delete(self): """Execute a delete query.""" - self.compiler.connection.validate_no_broken_transaction() if self.compiler.subqueries: raise NotSupportedError("Cannot use QuerySet.delete() when a subquery is required.") - return self.compiler.collection.delete_many( - self.match_mql, session=self.compiler.connection.session - ).deleted_count + return self.compiler.collection.delete_many(self.match_mql).deleted_count @wrap_database_errors def get_cursor(self): @@ -74,10 +71,7 @@ def get_cursor(self): Return a pymongo CommandCursor that can be iterated on to give the results of the query. """ - self.compiler.connection.validate_no_broken_transaction() - return self.compiler.collection.aggregate( - self.get_pipeline(), session=self.compiler.connection.session - ) + return self.compiler.collection.aggregate(self.get_pipeline()) def get_pipeline(self): pipeline = [] diff --git a/django_mongodb_backend/queryset.py b/django_mongodb_backend/queryset.py index b02496fe..4a2d884d 100644 --- a/django_mongodb_backend/queryset.py +++ b/django_mongodb_backend/queryset.py @@ -35,7 +35,7 @@ def __init__(self, pipeline, using, model): def _execute_query(self): connection = connections[self.using] collection = connection.get_collection(self.model._meta.db_table) - self.cursor = collection.aggregate(self.pipeline, session=connection.session) + self.cursor = collection.aggregate(self.pipeline) def __str__(self): return str(self.pipeline) diff --git a/docs/source/ref/database.rst b/docs/source/ref/database.rst index 0a815941..365e1c73 100644 --- a/docs/source/ref/database.rst +++ b/docs/source/ref/database.rst @@ -41,47 +41,3 @@ effect. Rather, if you need to close the connection pool, use .. versionadded:: 5.2.0b0 Support for connection pooling and ``connection.close_pool()`` were added. - -.. _transactions: - -Transactions -============ - -.. versionadded:: 5.2.0b2 - -Support for :doc:`Django's transactions APIs ` -is enabled if MongoDB is configured as a :doc:`replica set` -or a :doc:`sharded cluster `. - -If transactions aren't supported, query execution uses Django and MongoDB's -default behavior of autocommit mode. Each query is immediately committed to the -database. Django's transaction management APIs, such as -:func:`~django.db.transaction.atomic`, function as no-ops. - -:ref:`Deactivating transaction management ` -by setting :setting:`AUTOCOMMIT ` to ``False`` in the -:setting:`DATABASES` setting isn't supported. - -.. _transactions-limitations: - -Limitations ------------ - -MongoDB's transaction limitations that are applicable to Django are: - -- :meth:`QuerySet.union() ` is not - supported inside a transaction. -- If a transaction raises an exception, the transaction is no longer usable. - For example, if the update stage of :meth:`QuerySet.update_or_create() - ` fails with - :class:`~django.db.IntegrityError` due to a unique constraint violation, the - create stage won't be able to proceed. - :class:`pymongo.errors.OperationFailure` is raised, wrapped by - :class:`django.db.DatabaseError`. -- Savepoints (i.e. nested :func:`~django.db.transaction.atomic` blocks) aren't - supported. The outermost :func:`~django.db.transaction.atomic` will start - a transaction while any subsequent :func:`~django.db.transaction.atomic` - blocks will have no effect. -- Migration operations aren't :ref:`wrapped in a transaction - ` because of MongoDB restrictions such as - adding indexes to existing collections while in a transaction. diff --git a/docs/source/releases/5.2.x.rst b/docs/source/releases/5.2.x.rst index 6bddff11..d7a86aca 100644 --- a/docs/source/releases/5.2.x.rst +++ b/docs/source/releases/5.2.x.rst @@ -13,7 +13,6 @@ New features - Added subquery support for :class:`~.fields.EmbeddedModelArrayField`. - Added the ``options`` parameter to :func:`~django_mongodb_backend.utils.parse_uri`. -- Added support for :ref:`database transactions `. - Added :class:`~.fields.PolymorphicEmbeddedModelField` and :class:`~.fields.PolymorphicEmbeddedModelArrayField` for storing a model instance or list of model instances that may be of more than one model class. diff --git a/docs/source/topics/known-issues.rst b/docs/source/topics/known-issues.rst index 60628f81..4b9edee7 100644 --- a/docs/source/topics/known-issues.rst +++ b/docs/source/topics/known-issues.rst @@ -80,7 +80,11 @@ Database functions Transaction management ====================== -See :ref:`transactions` for details. +Query execution uses Django and MongoDB's default behavior of autocommit mode. +Each query is immediately committed to the database. + +Django's :doc:`transaction management APIs ` +are not supported. Database introspection ====================== diff --git a/tests/backend_/test_base.py b/tests/backend_/test_base.py index bf8a143d..7695b6f4 100644 --- a/tests/backend_/test_base.py +++ b/tests/backend_/test_base.py @@ -1,9 +1,7 @@ -import copy - from django.core.exceptions import ImproperlyConfigured from django.db import connection from django.db.backends.signals import connection_created -from django.test import SimpleTestCase, TransactionTestCase +from django.test import SimpleTestCase, TestCase from django_mongodb_backend.base import DatabaseWrapper @@ -16,18 +14,8 @@ def test_database_name_empty(self): with self.assertRaisesMessage(ImproperlyConfigured, msg): DatabaseWrapper(settings).get_connection_params() - def test_autocommit_false(self): - new_connection = connection.copy() - new_connection.settings_dict = copy.deepcopy(connection.settings_dict) - new_connection.settings_dict["AUTOCOMMIT"] = False - msg = "MongoDB does not support AUTOCOMMIT=False." - with self.assertRaisesMessage(ImproperlyConfigured, msg): - new_connection.check_settings() - - -class DatabaseWrapperConnectionTests(TransactionTestCase): - available_apps = ["backend_"] +class DatabaseWrapperConnectionTests(TestCase): def test_set_autocommit(self): self.assertIs(connection.get_autocommit(), True) connection.set_autocommit(False) diff --git a/tests/backend_/test_features.py b/tests/backend_/test_features.py index 504d3a3f..05959fa7 100644 --- a/tests/backend_/test_features.py +++ b/tests/backend_/test_features.py @@ -7,10 +7,10 @@ class SupportsTransactionsTests(TestCase): def setUp(self): # Clear the cached property. - del connection.features.supports_transactions + connection.features.__dict__.pop("_supports_transactions", None) def tearDown(self): - del connection.features.supports_transactions + del connection.features._supports_transactions def test_replica_set(self): """A replica set supports transactions.""" @@ -21,7 +21,7 @@ def mocked_command(command): raise Exception("Unexpected command") with patch("pymongo.synchronous.database.Database.command", wraps=mocked_command): - self.assertIs(connection.features.supports_transactions, True) + self.assertIs(connection.features._supports_transactions, True) def test_sharded_cluster(self): """A sharded cluster supports transactions.""" @@ -32,7 +32,7 @@ def mocked_command(command): raise Exception("Unexpected command") with patch("pymongo.synchronous.database.Database.command", wraps=mocked_command): - self.assertIs(connection.features.supports_transactions, True) + self.assertIs(connection.features._supports_transactions, True) def test_no_support(self): """No support on a non-replica set, non-sharded cluster.""" @@ -43,4 +43,4 @@ def mocked_command(command): raise Exception("Unexpected command") with patch("pymongo.synchronous.database.Database.command", wraps=mocked_command): - self.assertIs(connection.features.supports_transactions, False) + self.assertIs(connection.features._supports_transactions, False) diff --git a/tests/queries_/test_objectid.py b/tests/queries_/test_objectid.py index 36d9561a..490d1b33 100644 --- a/tests/queries_/test_objectid.py +++ b/tests/queries_/test_objectid.py @@ -1,6 +1,6 @@ from bson import ObjectId from django.core.exceptions import ValidationError -from django.test import TestCase, skipUnlessDBFeature +from django.test import TestCase from .models import Order, OrderItem, Tag @@ -75,7 +75,6 @@ def test_filter_parent_by_children_values_obj(self): parent_qs = Tag.objects.filter(children__id__in=child_ids).distinct().order_by("name") self.assertSequenceEqual(parent_qs, [self.t1]) - @skipUnlessDBFeature("supports_select_union") def test_filter_group_id_union_with_str(self): """Combine queries using union with string values.""" qs_a = Tag.objects.filter(group_id=self.group_id_str_1) @@ -83,7 +82,6 @@ def test_filter_group_id_union_with_str(self): union_qs = qs_a.union(qs_b).order_by("name") self.assertSequenceEqual(union_qs, [self.t3, self.t4]) - @skipUnlessDBFeature("supports_select_union") def test_filter_group_id_union_with_obj(self): """Combine queries using union with ObjectId values.""" qs_a = Tag.objects.filter(group_id=self.group_id_obj_1) diff --git a/tests/raw_query_/test_raw_aggregate.py b/tests/raw_query_/test_raw_aggregate.py index bfea4593..f16ce3cb 100644 --- a/tests/raw_query_/test_raw_aggregate.py +++ b/tests/raw_query_/test_raw_aggregate.py @@ -182,8 +182,7 @@ def test_different_db_key_order(self): { field.name: getattr(author, field.name) for field in reversed(Author._meta.concrete_fields) - }, - session=connection.session, + } ) query = [] authors = Author.objects.all()