Browse Source

Fixed #16039 -- Made post_syncdb handlers multi-db aware.

Also reverted 8fb7a9002669fb7ba7bec7df90b465b92e1ed3c2. Refs #17055.
Aymeric Augustin 12 years ago
parent
commit
a026e480da

+ 13 - 7
django/contrib/auth/management/__init__.py

@@ -10,6 +10,7 @@ import unicodedata
 from django.contrib.auth import models as auth_app, get_user_model
 from django.core import exceptions
 from django.core.management.base import CommandError
+from django.db import DEFAULT_DB_ALIAS, router
 from django.db.models import get_models, signals
 from django.utils import six
 from django.utils.six.moves import input
@@ -57,7 +58,10 @@ def _check_permission_clashing(custom, builtin, ctype):
                 (codename, ctype.app_label, ctype.model_class().__name__))
         pool.add(codename)
 
-def create_permissions(app, created_models, verbosity, **kwargs):
+def create_permissions(app, created_models, verbosity, db=DEFAULT_DB_ALIAS, **kwargs):
+    if not router.allow_syncdb(db, auth_app.Permission):
+        return
+
     from django.contrib.contenttypes.models import ContentType
 
     app_models = get_models(app)
@@ -68,7 +72,9 @@ def create_permissions(app, created_models, verbosity, **kwargs):
     # The codenames and ctypes that should exist.
     ctypes = set()
     for klass in app_models:
-        ctype = ContentType.objects.get_for_model(klass)
+        # Force looking up the content types in the current database
+        # before creating foreign keys to them.
+        ctype = ContentType.objects.db_manager(db).get_for_model(klass)
         ctypes.add(ctype)
         for perm in _get_all_permissions(klass._meta, ctype):
             searched_perms.append((ctype, perm))
@@ -76,21 +82,21 @@ def create_permissions(app, created_models, verbosity, **kwargs):
     # Find all the Permissions that have a context_type for a model we're
     # looking for.  We don't need to check for codenames since we already have
     # a list of the ones we're going to create.
-    all_perms = set(auth_app.Permission.objects.filter(
+    all_perms = set(auth_app.Permission.objects.using(db).filter(
         content_type__in=ctypes,
     ).values_list(
         "content_type", "codename"
     ))
 
-    objs = [
+    perms = [
         auth_app.Permission(codename=codename, name=name, content_type=ctype)
         for ctype, (codename, name) in searched_perms
         if (ctype.pk, codename) not in all_perms
     ]
-    auth_app.Permission.objects.bulk_create(objs)
+    auth_app.Permission.objects.using(db).bulk_create(perms)
     if verbosity >= 2:
-        for obj in objs:
-            print("Adding permission '%s'" % obj)
+        for perm in perms:
+            print("Adding permission '%s'" % perm)
 
 
 def create_superuser(app, created_models, verbosity, db, **kwargs):

+ 10 - 4
django/contrib/contenttypes/management.py

@@ -1,14 +1,18 @@
 from django.contrib.contenttypes.models import ContentType
+from django.db import DEFAULT_DB_ALIAS, router
 from django.db.models import get_apps, get_models, signals
 from django.utils.encoding import smart_text
 from django.utils import six
 from django.utils.six.moves import input
 
-def update_contenttypes(app, created_models, verbosity=2, **kwargs):
+def update_contenttypes(app, created_models, verbosity=2, db=DEFAULT_DB_ALIAS, **kwargs):
     """
     Creates content types for models in the given app, removing any model
     entries that no longer have a matching model class.
     """
+    if not router.allow_syncdb(db, ContentType):
+        return
+
     ContentType.objects.clear_cache()
     app_models = get_models(app)
     if not app_models:
@@ -19,10 +23,11 @@ def update_contenttypes(app, created_models, verbosity=2, **kwargs):
         (model._meta.object_name.lower(), model)
         for model in app_models
     )
+
     # Get all the content types
     content_types = dict(
         (ct.model, ct)
-        for ct in ContentType.objects.filter(app_label=app_label)
+        for ct in ContentType.objects.using(db).filter(app_label=app_label)
     )
     to_remove = [
         ct
@@ -30,7 +35,7 @@ def update_contenttypes(app, created_models, verbosity=2, **kwargs):
         if model_name not in app_models
     ]
 
-    cts = ContentType.objects.bulk_create([
+    cts = [
         ContentType(
             name=smart_text(model._meta.verbose_name_raw),
             app_label=app_label,
@@ -38,7 +43,8 @@ def update_contenttypes(app, created_models, verbosity=2, **kwargs):
         )
         for (model_name, model) in six.iteritems(app_models)
         if model_name not in content_types
-    ])
+    ]
+    ContentType.objects.using(db).bulk_create(cts)
     if verbosity >= 2:
         for ct in cts:
             print("Adding content type '%s | %s'" % (ct.app_label, ct.model))

+ 14 - 0
docs/releases/1.5.txt

@@ -551,6 +551,20 @@ with the :meth:`~django.forms.Form.is_valid()` method and not with the
 presence or absence of the :attr:`~django.forms.Form.cleaned_data` attribute
 on the form.
 
+Behavior of :djadmin:`syncdb` with multiple databases
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+:djadmin:`syncdb` now queries the database routers to determine if content
+types (when :mod:`~django.contrib.contenttypes` is enabled) and permissions
+(when :mod:`~django.contrib.auth` is enabled) should be created in the target
+database. Previously, it created them in the default database, even when
+another database was specified with the :djadminopt:`--database` option.
+
+If you use :djadmin:`syncdb` on multiple databases, you should ensure that
+your routers allow synchronizing content types and permissions to only one of
+them. See the docs on the :ref:`behavior of contrib apps with multiple
+databases <contrib_app_multiple_databases>` for more information.
+
 Miscellaneous
 ~~~~~~~~~~~~~
 

+ 46 - 0
docs/topics/db/multi-db.txt

@@ -630,3 +630,49 @@ However, if you're using SQLite or MySQL with MyISAM tables, there is
 no enforced referential integrity; as a result, you may be able to
 'fake' cross database foreign keys. However, this configuration is not
 officially supported by Django.
+
+.. _contrib_app_multiple_databases:
+
+Behavior of contrib apps
+------------------------
+
+Several contrib apps include models, and some apps depend on others. Since
+cross-database relationships are impossible, this creates some restrictions on
+how you can split these models across databases:
+
+- each one of ``contenttypes.ContentType``, ``sessions.Session`` and
+  ``sites.Site`` can be stored in any database, given a suitable router.
+- ``auth`` models — ``User``, ``Group`` and ``Permission`` — are linked
+  together and linked to ``ContentType``, so they must be stored in the same
+  database as ``ContentType``.
+- ``admin`` and ``comments`` depend on ``auth``, so their models must be in
+  the same database as ``auth``.
+- ``flatpages`` and ``redirects`` depend on ``sites``, so their models must be
+  in the same database as ``sites``.
+
+In addition, some objects are automatically created just after
+:djadmin:`syncdb` creates a table to hold them in a database:
+
+- a default ``Site``,
+- a ``ContentType`` for each model (including those not stored in that
+  database),
+- three ``Permission`` for each model (including those not stored in that
+  database).
+
+.. versionchanged:: 1.5
+    Previously, ``ContentType`` and ``Permission`` instances were created only
+    in the default database.
+
+For common setups with multiple databases, it isn't useful to have these
+objects in more than one database. Common setups include master / slave and
+connecting to external databases. Therefore, it's recommended:
+
+- either to run :djadmin:`syncdb` only for the default database;
+- or to write :ref:`database router<topics-db-multi-db-routing>` that allows
+  synchronizing these three models only to one database.
+
+.. warning::
+
+    If you're synchronizing content types to more that one database, be aware
+    that their primary keys may not match across databases. This may result in
+    data corruption or data loss.

+ 33 - 22
tests/regressiontests/multiple_database/tests.py

@@ -16,16 +16,6 @@ from django.utils.six import StringIO
 from .models import Book, Person, Pet, Review, UserProfile
 
 
-def copy_content_types_from_default_to_other():
-    # On post_syncdb, content types are created in the 'default' database.
-    # However, tests of generic foreign keys require them in 'other' too.
-    # The problem is masked on backends that defer constraints checks: at the
-    # end of each test, there's a rollback, and constraints are never checked.
-    # It only appears on MySQL + InnoDB.
-    for ct in ContentType.objects.using('default').all():
-        ct.save(using='other')
-
-
 class QueryTestCase(TestCase):
     multi_db = True
 
@@ -705,8 +695,6 @@ class QueryTestCase(TestCase):
 
     def test_generic_key_separation(self):
         "Generic fields are constrained to a single database"
-        copy_content_types_from_default_to_other()
-
         # Create a book and author on the default database
         pro = Book.objects.create(title="Pro Django",
                                   published=datetime.date(2008, 12, 16))
@@ -734,8 +722,6 @@ class QueryTestCase(TestCase):
 
     def test_generic_key_reverse_operations(self):
         "Generic reverse manipulations are all constrained to a single DB"
-        copy_content_types_from_default_to_other()
-
         dive = Book.objects.using('other').create(title="Dive into Python",
                                                   published=datetime.date(2009, 5, 4))
 
@@ -780,8 +766,6 @@ class QueryTestCase(TestCase):
 
     def test_generic_key_cross_database_protection(self):
         "Operations that involve sharing generic key objects across databases raise an error"
-        copy_content_types_from_default_to_other()
-
         # Create a book and author on the default database
         pro = Book.objects.create(title="Pro Django",
                                   published=datetime.date(2008, 12, 16))
@@ -833,8 +817,6 @@ class QueryTestCase(TestCase):
 
     def test_generic_key_deletion(self):
         "Cascaded deletions of Generic Key relations issue queries on the right database"
-        copy_content_types_from_default_to_other()
-
         dive = Book.objects.using('other').create(title="Dive into Python",
                                                   published=datetime.date(2009, 5, 4))
         review = Review.objects.using('other').create(source="Python Weekly", content_object=dive)
@@ -1402,8 +1384,6 @@ class RouterTestCase(TestCase):
 
     def test_generic_key_cross_database_protection(self):
         "Generic Key operations can span databases if they share a source"
-        copy_content_types_from_default_to_other()
-
         # Create a book and author on the default database
         pro = Book.objects.using('default'
                 ).create(title="Pro Django", published=datetime.date(2008, 12, 16))
@@ -1515,8 +1495,6 @@ class RouterTestCase(TestCase):
 
     def test_generic_key_managers(self):
         "Generic key relations are represented by managers, and can be controlled like managers"
-        copy_content_types_from_default_to_other()
-
         pro = Book.objects.using('other').create(title="Pro Django",
                                                  published=datetime.date(2008, 12, 16))
 
@@ -1922,3 +1900,36 @@ class RouterModelArgumentTestCase(TestCase):
         pet = Pet.objects.create(owner=person, name='Wart')
         # test related FK collection
         person.delete()
+
+
+class SyncOnlyDefaultDatabaseRouter(object):
+    def allow_syncdb(self, db, model):
+        return db == DEFAULT_DB_ALIAS
+
+
+class SyncDBTestCase(TestCase):
+    multi_db = True
+
+    def test_syncdb_to_other_database(self):
+        """Regression test for #16039: syncdb with --database option."""
+        count = ContentType.objects.count()
+        self.assertGreater(count, 0)
+
+        ContentType.objects.using('other').delete()
+        management.call_command('syncdb', verbosity=0, interactive=False,
+            load_initial_data=False, database='other')
+
+        self.assertEqual(ContentType.objects.using("other").count(), count)
+
+    def test_syncdb_to_other_database_with_router(self):
+        """Regression test for #16039: syncdb with --database option."""
+        ContentType.objects.using('other').delete()
+        try:
+            old_routers = router.routers
+            router.routers = [SyncOnlyDefaultDatabaseRouter()]
+            management.call_command('syncdb', verbosity=0, interactive=False,
+                load_initial_data=False, database='other')
+        finally:
+            router.routers = old_routers
+
+        self.assertEqual(ContentType.objects.using("other").count(), 0)