From f48f671223a20b161ca819cf7d6298e43b8ba5fe Mon Sep 17 00:00:00 2001 From: Jon Dufresne Date: Mon, 3 Feb 2020 19:07:00 -0800 Subject: [PATCH] Refs #31233 -- Changed DatabaseWrapper._nodb_connection to _nodb_cursor(). It is now a method instead of a property and returns a context manager that yields a cursor on entry and closes the cursor and connection upon exit. --- django/db/backends/base/base.py | 21 ++++++++++------ django/db/backends/base/creation.py | 12 +++------ django/db/backends/mysql/creation.py | 2 +- django/db/backends/postgresql/base.py | 17 ++++++++----- django/db/backends/postgresql/creation.py | 2 +- docs/releases/3.1.txt | 5 ++++ tests/backends/postgresql/tests.py | 30 ++++++++++++++++------- tests/postgres_tests/test_signals.py | 3 ++- 8 files changed, 58 insertions(+), 34 deletions(-) diff --git a/django/db/backends/base/base.py b/django/db/backends/base/base.py index 2a0141fdab..a1b5b1db64 100644 --- a/django/db/backends/base/base.py +++ b/django/db/backends/base/base.py @@ -606,16 +606,21 @@ class BaseDatabaseWrapper: if must_close: self.close() - @property - def _nodb_connection(self): + @contextmanager + def _nodb_cursor(self): """ - Return an alternative connection to be used when there is no need to - access the main database, specifically for test db creation/deletion. - This also prevents the production database from being exposed to - potential child threads while (or after) the test database is destroyed. - Refs #10868, #17786, #16969. + Return a cursor from an alternative connection to be used when there is + no need to access the main database, specifically for test db + creation/deletion. This also prevents the production database from + being exposed to potential child threads while (or after) the test + database is destroyed. Refs #10868, #17786, #16969. """ - return self.__class__({**self.settings_dict, 'NAME': None}, alias=NO_DB_ALIAS) + conn = self.__class__({**self.settings_dict, 'NAME': None}, alias=NO_DB_ALIAS) + try: + with conn.cursor() as cursor: + yield cursor + finally: + conn.close() def schema_editor(self, *args, **kwargs): """ diff --git a/django/db/backends/base/creation.py b/django/db/backends/base/creation.py index dd3407bac3..c50fc90171 100644 --- a/django/db/backends/base/creation.py +++ b/django/db/backends/base/creation.py @@ -20,12 +20,8 @@ class BaseDatabaseCreation: def __init__(self, connection): self.connection = connection - @property - def _nodb_connection(self): - """ - Used to be defined here, now moved to DatabaseWrapper. - """ - return self.connection._nodb_connection + def _nodb_cursor(self): + return self.connection._nodb_cursor() def log(self, msg): sys.stderr.write(msg + os.linesep) @@ -166,7 +162,7 @@ class BaseDatabaseCreation: 'suffix': self.sql_table_creation_suffix(), } # Create the test database and connect to it. - with self._nodb_connection.cursor() as cursor: + with self._nodb_cursor() as cursor: try: self._execute_create_test_db(cursor, test_db_params, keepdb) except Exception as e: @@ -272,7 +268,7 @@ class BaseDatabaseCreation: # ourselves. Connect to the previous database (not the test database) # to do so, because it's not allowed to delete a database while being # connected to it. - with self.connection._nodb_connection.cursor() as cursor: + with self._nodb_cursor() as cursor: cursor.execute("DROP DATABASE %s" % self.connection.ops.quote_name(test_database_name)) diff --git a/django/db/backends/mysql/creation.py b/django/db/backends/mysql/creation.py index c9330819e5..79d2959d5f 100644 --- a/django/db/backends/mysql/creation.py +++ b/django/db/backends/mysql/creation.py @@ -35,7 +35,7 @@ class DatabaseCreation(BaseDatabaseCreation): 'dbname': self.connection.ops.quote_name(target_database_name), 'suffix': self.sql_table_creation_suffix(), } - with self._nodb_connection.cursor() as cursor: + with self._nodb_cursor() as cursor: try: self._execute_create_test_db(cursor, test_db_params, keepdb) except Exception: diff --git a/django/db/backends/postgresql/base.py b/django/db/backends/postgresql/base.py index 12b969142e..0f45ca93e1 100644 --- a/django/db/backends/postgresql/base.py +++ b/django/db/backends/postgresql/base.py @@ -7,6 +7,7 @@ Requires psycopg 2: https://www.psycopg.org/ import asyncio import threading import warnings +from contextlib import contextmanager from django.conf import settings from django.core.exceptions import ImproperlyConfigured @@ -288,11 +289,11 @@ class DatabaseWrapper(BaseDatabaseWrapper): else: return True - @property - def _nodb_connection(self): - nodb_connection = super()._nodb_connection + @contextmanager + def _nodb_cursor(self): try: - nodb_connection.ensure_connection() + with super()._nodb_cursor() as cursor: + yield cursor except (Database.DatabaseError, WrappedDatabaseError): warnings.warn( "Normally Django will use a connection to the 'postgres' database " @@ -304,11 +305,15 @@ class DatabaseWrapper(BaseDatabaseWrapper): ) for connection in connections.all(): if connection.vendor == 'postgresql' and connection.settings_dict['NAME'] != 'postgres': - return self.__class__( + conn = self.__class__( {**self.settings_dict, 'NAME': connection.settings_dict['NAME']}, alias=self.alias, ) - return nodb_connection + try: + with conn.cursor() as cursor: + yield cursor + finally: + conn.close() @cached_property def pg_version(self): diff --git a/django/db/backends/postgresql/creation.py b/django/db/backends/postgresql/creation.py index 0cb26a4341..a609f33fd6 100644 --- a/django/db/backends/postgresql/creation.py +++ b/django/db/backends/postgresql/creation.py @@ -61,7 +61,7 @@ class DatabaseCreation(BaseDatabaseCreation): 'dbname': self._quote_name(target_database_name), 'suffix': self._get_database_create_suffix(template=source_database_name), } - with self._nodb_connection.cursor() as cursor: + with self._nodb_cursor() as cursor: try: self._execute_create_test_db(cursor, test_db_params, keepdb) except Exception: diff --git a/docs/releases/3.1.txt b/docs/releases/3.1.txt index 903e2510ba..58fed5080a 100644 --- a/docs/releases/3.1.txt +++ b/docs/releases/3.1.txt @@ -386,6 +386,11 @@ backends. on databases that support time zones. Previously, it was ``None`` on databases that support time zones. +* ``connection._nodb_connection`` property is changed to the + ``connection._nodb_cursor()`` method and now returns a context manager that + yields a cursor and automatically closes the cursor and connection upon + exiting the ``with`` statement. + Dropped support for MariaDB 10.1 -------------------------------- diff --git a/tests/backends/postgresql/tests.py b/tests/backends/postgresql/tests.py index 4e51c1c7b5..ab8ad4bb93 100644 --- a/tests/backends/postgresql/tests.py +++ b/tests/backends/postgresql/tests.py @@ -4,24 +4,32 @@ from unittest import mock from django.core.exceptions import ImproperlyConfigured from django.db import DatabaseError, connection, connections +from django.db.backends.base.base import BaseDatabaseWrapper from django.test import TestCase, override_settings @unittest.skipUnless(connection.vendor == 'postgresql', 'PostgreSQL tests') class Tests(TestCase): + databases = {'default', 'other'} - def test_nodb_connection(self): + def test_nodb_cursor(self): """ - The _nodb_connection property fallbacks to the default connection - database when access to the 'postgres' database is not granted. + The _nodb_cursor() fallbacks to the default connection database when + access to the 'postgres' database is not granted. """ + orig_connect = BaseDatabaseWrapper.connect + def mocked_connect(self): if self.settings_dict['NAME'] is None: raise DatabaseError() - return '' + return orig_connect(self) - nodb_conn = connection._nodb_connection - self.assertIsNone(nodb_conn.settings_dict['NAME']) + with connection._nodb_cursor() as cursor: + self.assertIs(cursor.closed, False) + self.assertIsNotNone(cursor.db.connection) + self.assertIsNone(cursor.db.settings_dict['NAME']) + self.assertIs(cursor.closed, True) + self.assertIsNone(cursor.db.connection) # Now assume the 'postgres' db isn't available msg = ( @@ -39,9 +47,13 @@ class Tests(TestCase): 'settings_dict', {**connection.settings_dict, 'NAME': 'postgres'}, ): - nodb_conn = connection._nodb_connection - self.assertIsNotNone(nodb_conn.settings_dict['NAME']) - self.assertEqual(nodb_conn.settings_dict['NAME'], connections['other'].settings_dict['NAME']) + with connection._nodb_cursor() as cursor: + self.assertIs(cursor.closed, False) + self.assertIsNotNone(cursor.db.connection) + self.assertIs(cursor.closed, True) + self.assertIsNone(cursor.db.connection) + self.assertIsNotNone(cursor.db.settings_dict['NAME']) + self.assertEqual(cursor.db.settings_dict['NAME'], connections['other'].settings_dict['NAME']) def test_database_name_too_long(self): from django.db.backends.postgresql.base import DatabaseWrapper diff --git a/tests/postgres_tests/test_signals.py b/tests/postgres_tests/test_signals.py index 3c6502b5e7..f1569c361c 100644 --- a/tests/postgres_tests/test_signals.py +++ b/tests/postgres_tests/test_signals.py @@ -38,4 +38,5 @@ class OIDTests(PostgreSQLTestCase): def test_register_type_handlers_no_db(self): """Registering type handlers for the nodb connection does nothing.""" - register_type_handlers(connection._nodb_connection) + with connection._nodb_cursor() as cursor: + register_type_handlers(cursor.db)