Browse Source

Refactored the migration signals to use app configs.

De-aliased pre/post_syncdb to pre/post_migrate to increase
backwards-compatibility.
Aymeric Augustin 11 years ago
parent
commit
00110904ac

+ 16 - 11
django/contrib/auth/management/__init__.py

@@ -60,18 +60,21 @@ def _check_permission_clashing(custom, builtin, ctype):
         pool.add(codename)
 
 
-def create_permissions(app, created_models, verbosity, db=DEFAULT_DB_ALIAS, **kwargs):
+def create_permissions(app_config, verbosity=22, interactive=True, db=DEFAULT_DB_ALIAS, **kwargs):
+    if not app_config.models_module:
+        return
+
     try:
-        apps.get_model('auth', 'Permission')
+        Permission = apps.get_model('auth', 'Permission')
     except LookupError:
         return
 
-    if not router.allow_migrate(db, auth_app.Permission):
+    if not router.allow_migrate(db, Permission):
         return
 
     from django.contrib.contenttypes.models import ContentType
 
-    app_models = apps.get_models(app)
+    app_models = apps.get_models(app_config.models_module)
 
     # This will hold the permissions we're looking for as
     # (content_type, (codename, name))
@@ -89,20 +92,20 @@ def create_permissions(app, created_models, verbosity, db=DEFAULT_DB_ALIAS, **kw
     # Find all the Permissions that have a content_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.using(db).filter(
+    all_perms = set(Permission.objects.using(db).filter(
         content_type__in=ctypes,
     ).values_list(
         "content_type", "codename"
     ))
 
     perms = [
-        auth_app.Permission(codename=codename, name=name, content_type=ctype)
+        Permission(codename=codename, name=name, content_type=ctype)
         for ctype, (codename, name) in searched_perms
         if (ctype.pk, codename) not in all_perms
     ]
     # Validate the permissions before bulk_creation to avoid cryptic
     # database error when the verbose_name is longer than 50 characters
-    permission_name_max_length = auth_app.Permission._meta.get_field('name').max_length
+    permission_name_max_length = Permission._meta.get_field('name').max_length
     verbose_name_max_length = permission_name_max_length - 11  # len('Can change ') prefix
     for perm in perms:
         if len(perm.name) > permission_name_max_length:
@@ -112,13 +115,13 @@ def create_permissions(app, created_models, verbosity, db=DEFAULT_DB_ALIAS, **kw
                     verbose_name_max_length,
                 )
             )
-    auth_app.Permission.objects.using(db).bulk_create(perms)
+    Permission.objects.using(db).bulk_create(perms)
     if verbosity >= 2:
         for perm in perms:
             print("Adding permission '%s'" % perm)
 
 
-def create_superuser(app, created_models, verbosity, db, **kwargs):
+def create_superuser(app_config, verbosity=22, interactive=True, db=DEFAULT_DB_ALIAS, **kwargs):
     try:
         apps.get_model('auth', 'Permission')
     except LookupError:
@@ -128,7 +131,7 @@ def create_superuser(app, created_models, verbosity, db, **kwargs):
 
     from django.core.management import call_command
 
-    if UserModel in created_models and kwargs.get('interactive', True):
+    if not UserModel.objects.exists() and interactive:
         msg = ("\nYou just installed Django's auth system, which means you "
             "don't have any superusers defined.\nWould you like to create one "
             "now? (yes/no): ")
@@ -203,7 +206,9 @@ def get_default_username(check_db=True):
             return ''
     return default_username
 
+
 signals.post_migrate.connect(create_permissions,
     dispatch_uid="django.contrib.auth.management.create_permissions")
 signals.post_migrate.connect(create_superuser,
-    sender=auth_app, dispatch_uid="django.contrib.auth.management.create_superuser")
+    sender=apps.get_app_config('auth'),
+    dispatch_uid="django.contrib.auth.management.create_superuser")

+ 12 - 6
django/contrib/auth/tests/test_management.py

@@ -232,13 +232,15 @@ class PermissionTestCase(TestCase):
         Test that we show proper error message if we are trying to create
         duplicate permissions.
         """
+        auth_app_config = apps.get_app_config('auth')
+
         # check duplicated default permission
         models.Permission._meta.permissions = [
             ('change_permission', 'Can edit permission (duplicate)')]
         six.assertRaisesRegex(self, CommandError,
             "The permission codename 'change_permission' clashes with a "
             "builtin permission for model 'auth.Permission'.",
-            create_permissions, models, [], verbosity=0)
+            create_permissions, auth_app_config, verbosity=0)
 
         # check duplicated custom permissions
         models.Permission._meta.permissions = [
@@ -249,21 +251,23 @@ class PermissionTestCase(TestCase):
         six.assertRaisesRegex(self, CommandError,
             "The permission codename 'my_custom_permission' is duplicated for model "
             "'auth.Permission'.",
-            create_permissions, models, [], verbosity=0)
+            create_permissions, auth_app_config, verbosity=0)
 
         # should not raise anything
         models.Permission._meta.permissions = [
             ('my_custom_permission', 'Some permission'),
             ('other_one', 'Some other permission'),
         ]
-        create_permissions(models, [], verbosity=0)
+        create_permissions(auth_app_config, verbosity=0)
 
     def test_default_permissions(self):
+        auth_app_config = apps.get_app_config('auth')
+
         permission_content_type = ContentType.objects.get_by_natural_key('auth', 'permission')
         models.Permission._meta.permissions = [
             ('my_custom_permission', 'Some permission'),
         ]
-        create_permissions(models, [], verbosity=0)
+        create_permissions(auth_app_config, verbosity=0)
 
         # add/change/delete permission by default + custom permission
         self.assertEqual(models.Permission.objects.filter(
@@ -272,7 +276,7 @@ class PermissionTestCase(TestCase):
 
         models.Permission.objects.filter(content_type=permission_content_type).delete()
         models.Permission._meta.default_permissions = []
-        create_permissions(models, [], verbosity=0)
+        create_permissions(auth_app_config, verbosity=0)
 
         # custom permission only since default permissions is empty
         self.assertEqual(models.Permission.objects.filter(
@@ -280,10 +284,12 @@ class PermissionTestCase(TestCase):
         ).count(), 1)
 
     def test_verbose_name_length(self):
+        auth_app_config = apps.get_app_config('auth')
+
         permission_content_type = ContentType.objects.get_by_natural_key('auth', 'permission')
         models.Permission.objects.filter(content_type=permission_content_type).delete()
         models.Permission._meta.verbose_name = "some ridiculously long verbose name that is out of control"
 
         six.assertRaisesRegex(self, exceptions.ValidationError,
             "The verbose_name of permission is longer than 39 characters",
-            create_permissions, models, [], verbosity=0)
+            create_permissions, auth_app_config, verbosity=0)

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

@@ -1,5 +1,4 @@
 from django.apps import apps
-from django.contrib.contenttypes.models import ContentType
 from django.db import DEFAULT_DB_ALIAS, router
 from django.db.models import signals
 from django.utils.encoding import smart_text
@@ -7,13 +6,16 @@ from django.utils import six
 from django.utils.six.moves import input
 
 
-def update_contenttypes(app, created_models, verbosity=2, db=DEFAULT_DB_ALIAS, **kwargs):
+def update_contenttypes(app_config, verbosity=2, interactive=True, 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 app_config.models_module:
+        return
+
     try:
-        apps.get_model('contenttypes', 'ContentType')
+        ContentType = apps.get_model('contenttypes', 'ContentType')
     except LookupError:
         return
 
@@ -21,7 +23,7 @@ def update_contenttypes(app, created_models, verbosity=2, db=DEFAULT_DB_ALIAS, *
         return
 
     ContentType.objects.clear_cache()
-    app_models = apps.get_models(app)
+    app_models = apps.get_models(app_config.models_module)
     if not app_models:
         return
     # They all have the same app_label, get the first one.
@@ -85,11 +87,13 @@ If you're unsure, answer 'no'.
                 print("Stale content types remain.")
 
 
-def update_all_contenttypes(verbosity=2, **kwargs):
+def update_all_contenttypes(**kwargs):
     for app_config in apps.get_app_configs(only_with_models_module=True):
-        update_contenttypes(app_config.models_module, None, verbosity, **kwargs)
+        update_contenttypes(app_config, **kwargs)
+
 
 signals.post_migrate.connect(update_contenttypes)
 
+
 if __name__ == "__main__":
     update_all_contenttypes()

+ 16 - 10
django/contrib/sites/management.py

@@ -2,17 +2,22 @@
 Creates the default Site object.
 """
 
-from django.db.models import signals
-from django.db import connections
-from django.db import router
-from django.contrib.sites.models import Site
-from django.contrib.sites import models as site_app
+from django.apps import apps
 from django.core.management.color import no_style
+from django.db import DEFAULT_DB_ALIAS, connections, router
+from django.db.models import signals
+
 
+def create_default_site(app_config, verbosity=22, interactive=True, db=DEFAULT_DB_ALIAS, **kwargs):
+    try:
+        Site = apps.get_model('sites', 'Site')
+    except LookupError:
+        return
 
-def create_default_site(app, created_models, verbosity, db, **kwargs):
-    # Only create the default sites in databases where Django created the table
-    if Site in created_models and router.allow_migrate(db, Site):
+    if not router.allow_migrate(db, Site):
+        return
+
+    if not Site.objects.exists():
         # The default settings set SITE_ID = 1, and some tests in Django's test
         # suite rely on this value. However, if database sequences are reused
         # (e.g. in the test suite after flush/syncdb), it isn't guaranteed that
@@ -32,6 +37,7 @@ def create_default_site(app, created_models, verbosity, db, **kwargs):
             for command in sequence_sql:
                 cursor.execute(command)
 
-    Site.objects.clear_cache()
+        Site.objects.clear_cache()
+
 
-signals.post_migrate.connect(create_default_site, sender=site_app)
+signals.post_migrate.connect(create_default_site, sender=apps.get_app_config('sites'))

+ 14 - 0
django/core/management/sql.py

@@ -211,6 +211,13 @@ def emit_pre_migrate_signal(create_models, verbosity, interactive, db):
         if verbosity >= 2:
             print("Running pre-migrate handlers for application %s" % app_config.label)
         models.signals.pre_migrate.send(
+            sender=app_config,
+            app_config=app_config,
+            verbosity=verbosity,
+            interactive=interactive,
+            db=db)
+        # For backwards-compatibility -- remove in Django 1.9.
+        models.signals.pre_syncdb.send(
             sender=app_config.models_module,
             app=app_config.models_module,
             create_models=create_models,
@@ -225,6 +232,13 @@ def emit_post_migrate_signal(created_models, verbosity, interactive, db):
         if verbosity >= 2:
             print("Running post-migrate handlers for application %s" % app_config.label)
         models.signals.post_migrate.send(
+            sender=app_config,
+            app_config=app_config,
+            verbosity=verbosity,
+            interactive=interactive,
+            db=db)
+        # For backwards-compatibility -- remove in Django 1.9.
+        models.signals.post_syncdb.send(
             sender=app_config.models_module,
             app=app_config.models_module,
             created_models=created_models,

+ 5 - 4
django/db/models/signals.py

@@ -62,7 +62,8 @@ post_delete = ModelSignal(providing_args=["instance", "using"], use_caching=True
 
 m2m_changed = ModelSignal(providing_args=["action", "instance", "reverse", "model", "pk_set", "using"], use_caching=True)
 
-pre_migrate = Signal(providing_args=["app", "create_models", "verbosity", "interactive", "db"])
-pre_syncdb = pre_migrate
-post_migrate = Signal(providing_args=["class", "app", "created_models", "verbosity", "interactive", "db"])
-post_syncdb = post_migrate
+pre_migrate = Signal(providing_args=["app_config", "verbosity", "interactive", "db"])
+post_migrate = Signal(providing_args=["app_config", "verbosity", "interactive", "db"])
+
+pre_syncdb = Signal(providing_args=["app", "create_models", "verbosity", "interactive", "db"])
+post_syncdb = Signal(providing_args=["class", "app", "created_models", "verbosity", "interactive", "db"])

+ 1 - 4
docs/internals/deprecation.txt

@@ -165,10 +165,7 @@ these changes.
 * The ``syncdb`` command will be removed.
 
 * ``django.db.models.signals.pre_syncdb`` and
-  ``django.db.models.signals.post_syncdb`` will be removed, and
-  ``django.db.models.signals.pre_migrate`` and
-  ``django.db.models.signals.post_migrate`` will lose their
-  ``create_models`` and ``created_models`` arguments.
+  ``django.db.models.signals.post_syncdb`` will be removed.
 
 * ``allow_syncdb`` on database routers will no longer automatically become
   ``allow_migrate``.

+ 98 - 19
docs/ref/signals.txt

@@ -381,12 +381,10 @@ handlers are registered anywhere else they may not be loaded by
 Arguments sent with this signal:
 
 ``sender``
-    The ``models`` module of the app about to be migrated/synced.
-    For example, if :djadmin:`migrate` is about to install
-    an app called ``"foo.bar.myapp"``, ``sender`` will be the
-    ``foo.bar.myapp.models`` module.
+    An :class:`~django.apps.AppConfig` instance for the application about to
+    be migrated/synced.
 
-``app``
+``app_config``
     Same as ``sender``.
 
 ``verbosity``
@@ -415,14 +413,48 @@ pre_syncdb
 
 .. deprecated:: 1.7
 
-    This signal has been renamed to :data:`~django.db.models.signals.pre_migrate`.
+    This signal has been replaced by :data:`~django.db.models.signals.pre_migrate`.
+
+Sent by the :djadmin:`syncdb` command before it starts to install an
+application.
+
+Any handlers that listen to this signal need to be written in a particular
+place: a ``management`` module in one of your :setting:`INSTALLED_APPS`. If
+handlers are registered anywhere else they may not be loaded by
+:djadmin:`syncdb`.
+
+Arguments sent with this signal:
+
+``sender``
+    The ``models`` module that was just installed. That is, if
+    :djadmin:`syncdb` just installed an app called ``"foo.bar.myapp"``,
+    ``sender`` will be the ``foo.bar.myapp.models`` module.
 
-Alias of :data:`django.db.models.signals.pre_migrate`. As long as this alias
-is present, for backwards-compatibility this signal has an extra argument it sends:
+``app``
+    Same as ``sender``.
 
 ``create_models``
-    A list of the model classes from any app which :djadmin:`migrate` is
-    going to create, **only if the app has no migrations**.
+    A list of the model classes from any app which :djadmin:`syncdb` plans to
+    create.
+
+
+``verbosity``
+    Indicates how much information manage.py is printing on screen. See
+    the :djadminopt:`--verbosity` flag for details.
+
+    Functions which listen for :data:`pre_syncdb` should adjust what they
+    output to the screen based on the value of this argument.
+
+``interactive``
+    If ``interactive`` is ``True``, it's safe to prompt the user to input
+    things on the command line. If ``interactive`` is ``False``, functions
+    which listen for this signal should not try to prompt for anything.
+
+    For example, the :mod:`django.contrib.auth` app only prompts to create a
+    superuser when ``interactive`` is ``True``.
+
+``db``
+    The alias of database on which a command will operate.
 
 post_migrate
 ------------
@@ -444,11 +476,10 @@ idempotent changes (e.g. no database alterations) as this may cause the
 Arguments sent with this signal:
 
 ``sender``
-    The ``models`` module that was just installed. That is, if
-    :djadmin:`migrate` just installed an app called ``"foo.bar.myapp"``,
-    ``sender`` will be the ``foo.bar.myapp.models`` module.
+    An :class:`~django.apps.AppConfig` instance for the application that was
+    just installed.
 
-``app``
+``app_config``
     Same as ``sender``.
 
 ``verbosity``
@@ -489,14 +520,62 @@ post_syncdb
 
 .. deprecated:: 1.7
 
-    This signal has been renamed to :data:`~django.db.models.signals.post_migrate`.
+    This signal has been replaced by :data:`~django.db.models.signals.post_migrate`.
 
-Alias of :data:`django.db.models.signals.post_migrate`. As long as this alias
-is present, for backwards-compatibility this signal has an extra argument it sends:
+Sent by the :djadmin:`syncdb` command after it installs an application, and the
+:djadmin:`flush` command.
+
+Any handlers that listen to this signal need to be written in a particular
+place: a ``management`` module in one of your :setting:`INSTALLED_APPS`. If
+handlers are registered anywhere else they may not be loaded by
+:djadmin:`syncdb`. It is important that handlers of this signal perform
+idempotent changes (e.g. no database alterations) as this may cause the
+:djadmin:`flush` management command to fail if it also ran during the
+:djadmin:`syncdb` command.
+
+Arguments sent with this signal:
+
+``sender``
+    The ``models`` module that was just installed. That is, if
+    :djadmin:`syncdb` just installed an app called ``"foo.bar.myapp"``,
+    ``sender`` will be the ``foo.bar.myapp.models`` module.
+
+``app``
+    Same as ``sender``.
 
 ``created_models``
-    A list of the model classes from any app which :djadmin:`migrate` has
-    created, **only if the app has no migrations**.
+    A list of the model classes from any app which :djadmin:`syncdb` has
+    created so far.
+
+``verbosity``
+    Indicates how much information manage.py is printing on screen. See
+    the :djadminopt:`--verbosity` flag for details.
+
+    Functions which listen for :data:`post_syncdb` should adjust what they
+    output to the screen based on the value of this argument.
+
+``interactive``
+    If ``interactive`` is ``True``, it's safe to prompt the user to input
+    things on the command line. If ``interactive`` is ``False``, functions
+    which listen for this signal should not try to prompt for anything.
+
+    For example, the :mod:`django.contrib.auth` app only prompts to create a
+    superuser when ``interactive`` is ``True``.
+
+``db``
+    The database alias used for synchronization. Defaults to the ``default``
+    database.
+
+For example, ``yourapp/management/__init__.py`` could be written like::
+
+    from django.db.models.signals import post_syncdb
+    import yourapp.models
+
+    def my_callback(sender, **kwargs):
+        # Your specific logic here
+        pass
+
+    post_syncdb.connect(my_callback, sender=yourapp.models)
 
 Request/response signals
 ========================

+ 5 - 4
docs/releases/1.7.txt

@@ -47,11 +47,12 @@ but a few of the key features are:
 * A new ``makemigrations`` command provides an easy way to autodetect changes
   to your models and make migrations for them.
 
-* :data:`~django.db.models.signals.pre_syncdb` and
-  :data:`~django.db.models.signals.post_syncdb` have been renamed to
+  :data:`~django.db.models.signals.pre_syncdb` and
+  :data:`~django.db.models.signals.post_syncdb` have been replaced by
   :data:`~django.db.models.signals.pre_migrate` and
-  :data:`~django.db.models.signals.post_migrate` respectively. The
-  ``create_models``/``created_models`` argument has also been deprecated.
+  :data:`~django.db.models.signals.post_migrate` respectively. These new
+  signals have slightly different arguments. Check the documentation for
+  details.
 
 * The ``allow_syncdb`` method on database routers is now called ``allow_migrate``,
   but still performs the same function. Routers with ``allow_syncdb`` methods

+ 2 - 2
tests/migrate_signals/models.py

@@ -1,2 +1,2 @@
-# Remove this module when pre/post_migrate are refactored to use something
-# other than a models module for their "sender" argument.
+# This module has to exist, otherwise pre/post_migrate aren't sent for the
+# migrate_signals application.

+ 8 - 10
tests/migrate_signals/tests.py

@@ -1,12 +1,12 @@
+from django.apps import apps
 from django.db.models import signals
 from django.test import TestCase
 from django.core import management
 from django.utils import six
 
-from . import models
 
-
-PRE_MIGRATE_ARGS = ['app', 'create_models', 'verbosity', 'interactive', 'db']
+APP_CONFIG = apps.get_app_config('migrate_signals')
+PRE_MIGRATE_ARGS = ['app_config', 'verbosity', 'interactive', 'db']
 MIGRATE_DATABASE = 'default'
 MIGRATE_VERBOSITY = 1
 MIGRATE_INTERACTIVE = False
@@ -39,7 +39,7 @@ class OneTimeReceiver(object):
             self.call_counter = self.call_counter + 1
             self.call_args = kwargs
             # we need to test only one call of migrate
-            signals.pre_migrate.disconnect(pre_migrate_receiver, sender=models)
+            signals.pre_migrate.disconnect(pre_migrate_receiver, sender=APP_CONFIG)
 
 
 # We connect receiver here and not in unit test code because we need to
@@ -51,21 +51,19 @@ class OneTimeReceiver(object):
 #   3. Test runner calls migrate for create default database.
 #   4. Test runner execute our unit test code.
 pre_migrate_receiver = OneTimeReceiver()
-signals.pre_migrate.connect(pre_migrate_receiver, sender=models)
+signals.pre_migrate.connect(pre_migrate_receiver, sender=APP_CONFIG)
 
 
 class MigrateSignalTests(TestCase):
 
-    available_apps = [
-        'migrate_signals',
-    ]
+    available_apps = ['migrate_signals']
 
     def test_pre_migrate_call_time(self):
         self.assertEqual(pre_migrate_receiver.call_counter, 1)
 
     def test_pre_migrate_args(self):
         r = PreMigrateReceiver()
-        signals.pre_migrate.connect(r, sender=models)
+        signals.pre_migrate.connect(r, sender=APP_CONFIG)
         management.call_command('migrate', database=MIGRATE_DATABASE,
             verbosity=MIGRATE_VERBOSITY, interactive=MIGRATE_INTERACTIVE,
             load_initial_data=False, stdout=six.StringIO())
@@ -73,7 +71,7 @@ class MigrateSignalTests(TestCase):
         args = r.call_args
         self.assertEqual(r.call_counter, 1)
         self.assertEqual(set(args), set(PRE_MIGRATE_ARGS))
-        self.assertEqual(args['app'], models)
+        self.assertEqual(args['app_config'], APP_CONFIG)
         self.assertEqual(args['verbosity'], MIGRATE_VERBOSITY)
         self.assertEqual(args['interactive'], MIGRATE_INTERACTIVE)
         self.assertEqual(args['db'], 'default')