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.
This commit is contained in:
Jon Dufresne 2020-02-03 19:07:00 -08:00 committed by Mariusz Felisiak
parent 2d55cb5c4a
commit f48f671223
8 changed files with 58 additions and 34 deletions

View File

@ -606,16 +606,21 @@ class BaseDatabaseWrapper:
if must_close: if must_close:
self.close() self.close()
@property @contextmanager
def _nodb_connection(self): def _nodb_cursor(self):
""" """
Return an alternative connection to be used when there is no need to Return a cursor from an alternative connection to be used when there is
access the main database, specifically for test db creation/deletion. no need to access the main database, specifically for test db
This also prevents the production database from being exposed to creation/deletion. This also prevents the production database from
potential child threads while (or after) the test database is destroyed. being exposed to potential child threads while (or after) the test
Refs #10868, #17786, #16969. 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): def schema_editor(self, *args, **kwargs):
""" """

View File

@ -20,12 +20,8 @@ class BaseDatabaseCreation:
def __init__(self, connection): def __init__(self, connection):
self.connection = connection self.connection = connection
@property def _nodb_cursor(self):
def _nodb_connection(self): return self.connection._nodb_cursor()
"""
Used to be defined here, now moved to DatabaseWrapper.
"""
return self.connection._nodb_connection
def log(self, msg): def log(self, msg):
sys.stderr.write(msg + os.linesep) sys.stderr.write(msg + os.linesep)
@ -166,7 +162,7 @@ class BaseDatabaseCreation:
'suffix': self.sql_table_creation_suffix(), 'suffix': self.sql_table_creation_suffix(),
} }
# Create the test database and connect to it. # Create the test database and connect to it.
with self._nodb_connection.cursor() as cursor: with self._nodb_cursor() as cursor:
try: try:
self._execute_create_test_db(cursor, test_db_params, keepdb) self._execute_create_test_db(cursor, test_db_params, keepdb)
except Exception as e: except Exception as e:
@ -272,7 +268,7 @@ class BaseDatabaseCreation:
# ourselves. Connect to the previous database (not the test database) # ourselves. Connect to the previous database (not the test database)
# to do so, because it's not allowed to delete a database while being # to do so, because it's not allowed to delete a database while being
# connected to it. # connected to it.
with self.connection._nodb_connection.cursor() as cursor: with self._nodb_cursor() as cursor:
cursor.execute("DROP DATABASE %s" cursor.execute("DROP DATABASE %s"
% self.connection.ops.quote_name(test_database_name)) % self.connection.ops.quote_name(test_database_name))

View File

@ -35,7 +35,7 @@ class DatabaseCreation(BaseDatabaseCreation):
'dbname': self.connection.ops.quote_name(target_database_name), 'dbname': self.connection.ops.quote_name(target_database_name),
'suffix': self.sql_table_creation_suffix(), 'suffix': self.sql_table_creation_suffix(),
} }
with self._nodb_connection.cursor() as cursor: with self._nodb_cursor() as cursor:
try: try:
self._execute_create_test_db(cursor, test_db_params, keepdb) self._execute_create_test_db(cursor, test_db_params, keepdb)
except Exception: except Exception:

View File

@ -7,6 +7,7 @@ Requires psycopg 2: https://www.psycopg.org/
import asyncio import asyncio
import threading import threading
import warnings import warnings
from contextlib import contextmanager
from django.conf import settings from django.conf import settings
from django.core.exceptions import ImproperlyConfigured from django.core.exceptions import ImproperlyConfigured
@ -288,11 +289,11 @@ class DatabaseWrapper(BaseDatabaseWrapper):
else: else:
return True return True
@property @contextmanager
def _nodb_connection(self): def _nodb_cursor(self):
nodb_connection = super()._nodb_connection
try: try:
nodb_connection.ensure_connection() with super()._nodb_cursor() as cursor:
yield cursor
except (Database.DatabaseError, WrappedDatabaseError): except (Database.DatabaseError, WrappedDatabaseError):
warnings.warn( warnings.warn(
"Normally Django will use a connection to the 'postgres' database " "Normally Django will use a connection to the 'postgres' database "
@ -304,11 +305,15 @@ class DatabaseWrapper(BaseDatabaseWrapper):
) )
for connection in connections.all(): for connection in connections.all():
if connection.vendor == 'postgresql' and connection.settings_dict['NAME'] != 'postgres': if connection.vendor == 'postgresql' and connection.settings_dict['NAME'] != 'postgres':
return self.__class__( conn = self.__class__(
{**self.settings_dict, 'NAME': connection.settings_dict['NAME']}, {**self.settings_dict, 'NAME': connection.settings_dict['NAME']},
alias=self.alias, alias=self.alias,
) )
return nodb_connection try:
with conn.cursor() as cursor:
yield cursor
finally:
conn.close()
@cached_property @cached_property
def pg_version(self): def pg_version(self):

View File

@ -61,7 +61,7 @@ class DatabaseCreation(BaseDatabaseCreation):
'dbname': self._quote_name(target_database_name), 'dbname': self._quote_name(target_database_name),
'suffix': self._get_database_create_suffix(template=source_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: try:
self._execute_create_test_db(cursor, test_db_params, keepdb) self._execute_create_test_db(cursor, test_db_params, keepdb)
except Exception: except Exception:

View File

@ -386,6 +386,11 @@ backends.
on databases that support time zones. Previously, it was ``None`` on on databases that support time zones. Previously, it was ``None`` on
databases that support time zones. 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 Dropped support for MariaDB 10.1
-------------------------------- --------------------------------

View File

@ -4,24 +4,32 @@ from unittest import mock
from django.core.exceptions import ImproperlyConfigured from django.core.exceptions import ImproperlyConfigured
from django.db import DatabaseError, connection, connections from django.db import DatabaseError, connection, connections
from django.db.backends.base.base import BaseDatabaseWrapper
from django.test import TestCase, override_settings from django.test import TestCase, override_settings
@unittest.skipUnless(connection.vendor == 'postgresql', 'PostgreSQL tests') @unittest.skipUnless(connection.vendor == 'postgresql', 'PostgreSQL tests')
class Tests(TestCase): 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 The _nodb_cursor() fallbacks to the default connection database when
database when access to the 'postgres' database is not granted. access to the 'postgres' database is not granted.
""" """
orig_connect = BaseDatabaseWrapper.connect
def mocked_connect(self): def mocked_connect(self):
if self.settings_dict['NAME'] is None: if self.settings_dict['NAME'] is None:
raise DatabaseError() raise DatabaseError()
return '' return orig_connect(self)
nodb_conn = connection._nodb_connection with connection._nodb_cursor() as cursor:
self.assertIsNone(nodb_conn.settings_dict['NAME']) 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 # Now assume the 'postgres' db isn't available
msg = ( msg = (
@ -39,9 +47,13 @@ class Tests(TestCase):
'settings_dict', 'settings_dict',
{**connection.settings_dict, 'NAME': 'postgres'}, {**connection.settings_dict, 'NAME': 'postgres'},
): ):
nodb_conn = connection._nodb_connection with connection._nodb_cursor() as cursor:
self.assertIsNotNone(nodb_conn.settings_dict['NAME']) self.assertIs(cursor.closed, False)
self.assertEqual(nodb_conn.settings_dict['NAME'], connections['other'].settings_dict['NAME']) 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): def test_database_name_too_long(self):
from django.db.backends.postgresql.base import DatabaseWrapper from django.db.backends.postgresql.base import DatabaseWrapper

View File

@ -38,4 +38,5 @@ class OIDTests(PostgreSQLTestCase):
def test_register_type_handlers_no_db(self): def test_register_type_handlers_no_db(self):
"""Registering type handlers for the nodb connection does nothing.""" """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)