From e953c78eeb81ee69dccd356145563fd6f9e4c7b6 Mon Sep 17 00:00:00 2001 From: Claude Paroz Date: Sat, 9 Nov 2013 09:41:03 +0100 Subject: [PATCH] Fixed #16969 -- Don't connect to named database when possible Thanks Andreas Pelme for the report and initial patch, and Aymeric Augustin, Shai Berger and Tim Graham for the reviews. --- django/db/backends/creation.py | 37 +++++++++++-------- .../db/backends/postgresql_psycopg2/base.py | 5 ++- .../contributing/writing-code/unit-tests.txt | 18 +++++---- 3 files changed, 36 insertions(+), 24 deletions(-) diff --git a/django/db/backends/creation.py b/django/db/backends/creation.py index 866031447d..7c14b6a09c 100644 --- a/django/db/backends/creation.py +++ b/django/db/backends/creation.py @@ -6,6 +6,7 @@ import warnings from django.conf import settings from django.db.utils import load_backend from django.utils.encoding import force_bytes +from django.utils.functional import cached_property from django.utils.six.moves import input from .utils import truncate_name @@ -29,6 +30,24 @@ class BaseDatabaseCreation(object): def __init__(self, connection): self.connection = connection + @cached_property + def _nodb_connection(self): + """ + 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. + """ + settings_dict = self.connection.settings_dict.copy() + settings_dict['NAME'] = None + backend = load_backend(settings_dict['ENGINE']) + nodb_connection = backend.DatabaseWrapper( + settings_dict, + alias='__no_db__', + allow_thread_sharing=False) + return nodb_connection + @classmethod def _digest(cls, *args): """ @@ -386,7 +405,7 @@ class BaseDatabaseCreation(object): qn = self.connection.ops.quote_name # Create the test database and connect to it. - cursor = self.connection.cursor() + cursor = self._nodb_connection.cursor() try: cursor.execute( "CREATE DATABASE %s %s" % (qn(test_database_name), suffix)) @@ -431,18 +450,7 @@ class BaseDatabaseCreation(object): print("Destroying test database for alias '%s'%s..." % ( self.connection.alias, test_db_repr)) - # Temporarily use a new connection and a copy of the settings dict. - # This prevents the production database from being exposed to potential - # child threads while (or after) the test database is destroyed. - # Refs #10868 and #17786. - settings_dict = self.connection.settings_dict.copy() - settings_dict['NAME'] = old_database_name - backend = load_backend(settings_dict['ENGINE']) - new_connection = backend.DatabaseWrapper( - settings_dict, - alias='__destroy_test_db__', - allow_thread_sharing=False) - new_connection.creation._destroy_test_db(test_database_name, verbosity) + self._destroy_test_db(test_database_name, verbosity) def _destroy_test_db(self, test_database_name, verbosity): """ @@ -452,12 +460,11 @@ class BaseDatabaseCreation(object): # 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. - cursor = self.connection.cursor() + cursor = self._nodb_connection.cursor() # Wait to avoid "database is being accessed by other users" errors. time.sleep(1) cursor.execute("DROP DATABASE %s" % self.connection.ops.quote_name(test_database_name)) - self.connection.close() def set_autocommit(self): """ diff --git a/django/db/backends/postgresql_psycopg2/base.py b/django/db/backends/postgresql_psycopg2/base.py index b22c653bd7..cb23c33a5c 100644 --- a/django/db/backends/postgresql_psycopg2/base.py +++ b/django/db/backends/postgresql_psycopg2/base.py @@ -101,13 +101,14 @@ class DatabaseWrapper(BaseDatabaseWrapper): def get_connection_params(self): settings_dict = self.settings_dict - if not settings_dict['NAME']: + # None may be used to connect to the default 'postgres' db + if settings_dict['NAME'] == '': from django.core.exceptions import ImproperlyConfigured raise ImproperlyConfigured( "settings.DATABASES is improperly configured. " "Please supply the NAME value.") conn_params = { - 'database': settings_dict['NAME'], + 'database': settings_dict['NAME'] or 'postgres', } conn_params.update(settings_dict['OPTIONS']) if 'autocommit' in conn_params: diff --git a/docs/internals/contributing/writing-code/unit-tests.txt b/docs/internals/contributing/writing-code/unit-tests.txt index 99969e36d6..e83d0bef17 100644 --- a/docs/internals/contributing/writing-code/unit-tests.txt +++ b/docs/internals/contributing/writing-code/unit-tests.txt @@ -76,17 +76,21 @@ If you're using a backend that isn't SQLite, you will need to provide other details for each database: * The :setting:`USER` option needs to specify an existing user account - for the database. + for the database. That user needs permission to execute ``CREATE DATABASE`` + so that the test database can be created. * The :setting:`PASSWORD` option needs to provide the password for the :setting:`USER` that has been specified. -* The :setting:`NAME` option must be the name of an existing database to - which the given user has permission to connect. The unit tests will not - touch this database; the test runner creates a new database whose name - is :setting:`NAME` prefixed with ``test_``, and this test database is - deleted when the tests are finished. This means your user account needs - permission to execute ``CREATE DATABASE``. +Test databases get their names by prepending ``test_`` to the value of the +:setting:`NAME` settings for the databases defined in :setting:`DATABASES`. +These test databases are deleted when the tests are finished. + +.. versionchanged:: 1.7 + + Before Django 1.7, the :setting:`NAME` setting was mandatory and had to + be the name of an existing database to which the given user had permission + to connect. You will also need to ensure that your database uses UTF-8 as the default character set. If your database server doesn't use UTF-8 as a default charset,