From 6a2af01452966d10155b720f4f5e7b09c7e3e419 Mon Sep 17 00:00:00 2001 From: Tim Graham Date: Fri, 2 Oct 2015 10:46:29 -0400 Subject: [PATCH] Fixed #24865 -- Added remove_stale_contenttypes management command. Thanks Simon Charette for the review. --- django/contrib/contenttypes/apps.py | 4 +- .../{management.py => management/__init__.py} | 103 ++++-------------- .../management/commands/__init__.py | 0 .../commands/remove_stale_contenttypes.py | 86 +++++++++++++++ django/contrib/contenttypes/models.py | 5 +- docs/ref/django-admin.txt | 22 ++++ docs/releases/1.11.txt | 12 +- tests/contenttypes_tests/tests.py | 38 ++++--- 8 files changed, 164 insertions(+), 106 deletions(-) rename django/contrib/contenttypes/{management.py => management/__init__.py} (62%) create mode 100644 django/contrib/contenttypes/management/commands/__init__.py create mode 100644 django/contrib/contenttypes/management/commands/remove_stale_contenttypes.py diff --git a/django/contrib/contenttypes/apps.py b/django/contrib/contenttypes/apps.py index e5708adc99..5754f8c24c 100644 --- a/django/contrib/contenttypes/apps.py +++ b/django/contrib/contenttypes/apps.py @@ -5,7 +5,7 @@ from django.db.models.signals import post_migrate, pre_migrate from django.utils.translation import ugettext_lazy as _ from .management import ( - inject_rename_contenttypes_operations, update_contenttypes, + create_contenttypes, inject_rename_contenttypes_operations, ) @@ -15,5 +15,5 @@ class ContentTypesConfig(AppConfig): def ready(self): pre_migrate.connect(inject_rename_contenttypes_operations, sender=self) - post_migrate.connect(update_contenttypes) + post_migrate.connect(create_contenttypes) checks.register(check_generic_foreign_keys, checks.Tags.models) diff --git a/django/contrib/contenttypes/management.py b/django/contrib/contenttypes/management/__init__.py similarity index 62% rename from django/contrib/contenttypes/management.py rename to django/contrib/contenttypes/management/__init__.py index aa1c198fc4..d0d5b52f09 100644 --- a/django/contrib/contenttypes/management.py +++ b/django/contrib/contenttypes/management/__init__.py @@ -1,9 +1,7 @@ from django.apps import apps as global_apps from django.db import DEFAULT_DB_ALIAS, migrations, router, transaction -from django.db.models.deletion import Collector from django.db.utils import IntegrityError from django.utils import six -from django.utils.six.moves import input class RenameContentType(migrations.RunPython): @@ -30,8 +28,8 @@ class RenameContentType(migrations.RunPython): content_type.save(update_fields={'model'}) except IntegrityError: # Gracefully fallback if a stale content type causes a - # conflict as update_contenttypes will take care of asking the - # user what should be done next. + # conflict as remove_stale_contenttypes will take care of + # asking the user what should be done next. content_type.model = old_model else: # Clear the cache as the `get_by_natual_key()` call will cache @@ -87,10 +85,26 @@ def inject_rename_contenttypes_operations(plan=None, apps=global_apps, using=DEF migration.operations.insert(inserted + index, operation) -def update_contenttypes(app_config, verbosity=2, interactive=True, using=DEFAULT_DB_ALIAS, apps=global_apps, **kwargs): +def get_contenttypes_and_models(app_config, using, ContentType): + if not router.allow_migrate_model(using, ContentType): + return None, None + + ContentType.objects.clear_cache() + + content_types = { + ct.model: ct + for ct in ContentType.objects.using(using).filter(app_label=app_config.label) + } + app_models = { + model._meta.model_name: model + for model in app_config.get_models() + } + return content_types, app_models + + +def create_contenttypes(app_config, verbosity=2, interactive=True, using=DEFAULT_DB_ALIAS, apps=global_apps, **kwargs): """ - Creates content types for models in the given app, removing any model - entries that no longer have a matching model class. + Creates content types for models in the given app. """ if not app_config.models_module: return @@ -102,32 +116,11 @@ def update_contenttypes(app_config, verbosity=2, interactive=True, using=DEFAULT except LookupError: return - if not router.allow_migrate_model(using, ContentType): - return - - ContentType.objects.clear_cache() - # Always clear the global content types cache. - if apps is not global_apps: - global_apps.get_model('contenttypes', 'ContentType').objects.clear_cache() - - app_models = { - model._meta.model_name: model - for model in app_config.get_models()} + content_types, app_models = get_contenttypes_and_models(app_config, using, ContentType) if not app_models: return - # Get all the content types - content_types = { - ct.model: ct - for ct in ContentType.objects.using(using).filter(app_label=app_label) - } - to_remove = [ - ct - for (model_name, ct) in six.iteritems(content_types) - if model_name not in app_models - ] - cts = [ ContentType( app_label=app_label, @@ -140,55 +133,3 @@ def update_contenttypes(app_config, verbosity=2, interactive=True, using=DEFAULT if verbosity >= 2: for ct in cts: print("Adding content type '%s | %s'" % (ct.app_label, ct.model)) - - # Confirm that the content type is stale before deletion. - using = router.db_for_write(ContentType) - if to_remove: - if interactive: - ct_info = [] - for ct in to_remove: - ct_info.append(' - Content type for %s.%s' % (ct.app_label, ct.model)) - collector = NoFastDeleteCollector(using=using) - collector.collect([ct]) - - for obj_type, objs in collector.data.items(): - if objs == {ct}: - continue - ct_info.append(' - %s %s object(s)' % ( - len(objs), - obj_type._meta.label, - )) - - content_type_display = '\n'.join(ct_info) - print("""Some content types in your database are stale and can be deleted. -Any objects that depend on these content types will also be deleted. -The content types and dependent objects that would be deleted are: - -%s - -This list doesn't include any cascade deletions to data outside of Django's -models (uncommon). - -Are you sure you want to delete these content types? -If you're unsure, answer 'no'. - """ % content_type_display) - ok_to_delete = input("Type 'yes' to continue, or 'no' to cancel: ") - else: - ok_to_delete = False - - if ok_to_delete == 'yes': - for ct in to_remove: - if verbosity >= 2: - print("Deleting stale content type '%s | %s'" % (ct.app_label, ct.model)) - ct.delete() - else: - if verbosity >= 2: - print("Stale content types remain.") - - -class NoFastDeleteCollector(Collector): - def can_fast_delete(self, *args, **kwargs): - """ - Always load related objects to display them when showing confirmation. - """ - return False diff --git a/django/contrib/contenttypes/management/commands/__init__.py b/django/contrib/contenttypes/management/commands/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/django/contrib/contenttypes/management/commands/remove_stale_contenttypes.py b/django/contrib/contenttypes/management/commands/remove_stale_contenttypes.py new file mode 100644 index 0000000000..2a3b23b0d0 --- /dev/null +++ b/django/contrib/contenttypes/management/commands/remove_stale_contenttypes.py @@ -0,0 +1,86 @@ +from django.apps import apps +from django.contrib.contenttypes.models import ContentType +from django.core.management import BaseCommand +from django.db import DEFAULT_DB_ALIAS, router +from django.db.models.deletion import Collector +from django.utils import six +from django.utils.six.moves import input + +from ...management import get_contenttypes_and_models + + +class Command(BaseCommand): + + def add_arguments(self, parser): + parser.add_argument( + '--noinput', '--no-input', + action='store_false', dest='interactive', default=True, + help='Tells Django to NOT prompt the user for input of any kind.', + ) + parser.add_argument( + '--database', action='store', dest='database', default=DEFAULT_DB_ALIAS, + help='Nominates the database to use. Defaults to the "default" database.', + ) + + def handle(self, **options): + db = options['database'] + interactive = options['interactive'] + verbosity = options['verbosity'] + + for app_config in apps.get_app_configs(): + content_types, app_models = get_contenttypes_and_models(app_config, db, ContentType) + if not app_models: + continue + to_remove = [ + ct for (model_name, ct) in six.iteritems(content_types) + if model_name not in app_models + ] + # Confirm that the content type is stale before deletion. + using = router.db_for_write(ContentType) + if to_remove: + if interactive: + ct_info = [] + for ct in to_remove: + ct_info.append(' - Content type for %s.%s' % (ct.app_label, ct.model)) + collector = NoFastDeleteCollector(using=using) + collector.collect([ct]) + + for obj_type, objs in collector.data.items(): + if objs == {ct}: + continue + ct_info.append(' - %s %s object(s)' % ( + len(objs), + obj_type._meta.label, + )) + content_type_display = '\n'.join(ct_info) + self.stdout.write("""Some content types in your database are stale and can be deleted. +Any objects that depend on these content types will also be deleted. +The content types and dependent objects that would be deleted are: + +%s + +This list doesn't include any cascade deletions to data outside of Django's +models (uncommon). + +Are you sure you want to delete these content types? +If you're unsure, answer 'no'.\n""" % content_type_display) + ok_to_delete = input("Type 'yes' to continue, or 'no' to cancel: ") + else: + ok_to_delete = False + + if ok_to_delete == 'yes': + for ct in to_remove: + if verbosity >= 2: + self.stdout.write("Deleting stale content type '%s | %s'" % (ct.app_label, ct.model)) + ct.delete() + else: + if verbosity >= 2: + self.stdout.write("Stale content types remain.") + + +class NoFastDeleteCollector(Collector): + def can_fast_delete(self, *args, **kwargs): + """ + Always load related objects to display them when showing confirmation. + """ + return False diff --git a/django/contrib/contenttypes/models.py b/django/contrib/contenttypes/models.py index 21432f897a..4823e2251a 100644 --- a/django/contrib/contenttypes/models.py +++ b/django/contrib/contenttypes/models.py @@ -118,10 +118,7 @@ class ContentTypeManager(models.Manager): def clear_cache(self): """ - Clear out the content-type cache. This needs to happen during database - flushes to prevent caching of "stale" content type IDs (see - django.contrib.contenttypes.management.update_contenttypes for where - this gets called). + Clear out the content-type cache. """ self._cache.clear() diff --git a/docs/ref/django-admin.txt b/docs/ref/django-admin.txt index 3421478f33..74f105be74 100644 --- a/docs/ref/django-admin.txt +++ b/docs/ref/django-admin.txt @@ -1456,6 +1456,28 @@ it could be useful if you have a ``ForeignKey`` in allow creating an instance instead of entering the primary key of an existing instance. +``django.contrib.contenttypes`` +------------------------------- + +``remove_stale_contenttypes`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +.. django-admin:: remove_stale_contenttypes + +.. versionadded:: 1.11 + +This command is only available if Django's :doc:`contenttypes app +` (:mod:`django.contrib.contenttypes`) is installed. + +Deletes stale content types (from deleted models) in your database. Any objects +that depend on the deleted content types will also be deleted. A list of +deleted objects will be displayed before you confirm it's okay to proceed with +the deletion. + +.. django-admin-option:: --database DATABASE + +Specifies the database to use. Defaults to ``default``. + ``django.contrib.gis`` ---------------------- diff --git a/docs/releases/1.11.txt b/docs/releases/1.11.txt index bb8c851f72..46bc7f1529 100644 --- a/docs/releases/1.11.txt +++ b/docs/releases/1.11.txt @@ -104,9 +104,11 @@ Minor features :mod:`django.contrib.contenttypes` ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -* When stale content types are detected after the ``migrate`` command, there's - now a list of related objects such as ``auth.Permission``\s that will also be - deleted. Previously, only the content types were listed. +* When stale content types are detected in the + :djadmin:`remove_stale_contenttypes` command, there's now a list of related + objects such as ``auth.Permission``\s that will also be deleted. Previously, + only the content types were listed (and this prompt was after ``migrate`` + rather than in a separate command). :mod:`django.contrib.gis` ~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -477,6 +479,10 @@ Miscellaneous be backwards-incompatible if you have some :ref:`template tags that aren't thread safe `. +* The prompt for stale content type deletion no longer occurs after running the + ``migrate`` command. Use the new :djadmin:`remove_stale_contenttypes` command + instead. + .. _deprecated-features-1.11: Features deprecated in 1.11 diff --git a/tests/contenttypes_tests/tests.py b/tests/contenttypes_tests/tests.py index 6d57a23355..ef13d3f4bb 100644 --- a/tests/contenttypes_tests/tests.py +++ b/tests/contenttypes_tests/tests.py @@ -12,6 +12,7 @@ from django.contrib.contenttypes.fields import ( from django.contrib.contenttypes.models import ContentType from django.contrib.sites.models import Site from django.core import checks, management +from django.core.management import call_command from django.db import connections, migrations, models from django.test import ( SimpleTestCase, TestCase, TransactionTestCase, mock, override_settings, @@ -388,16 +389,19 @@ class UpdateContentTypesTests(TestCase): def test_interactive_true_with_dependent_objects(self): """ - interactive mode of update_contenttypes() (the default) should delete - stale contenttypes and warn of dependent objects. + interactive mode of remove_stale_contenttypes (the default) should + delete stale contenttypes and warn of dependent objects. """ post = Post.objects.create(title='post', content_type=self.content_type) # A related object is needed to show that a custom collector with # can_fast_delete=False is needed. ModelWithNullFKToSite.objects.create(post=post) - contenttypes_management.input = lambda x: force_str("yes") - with captured_stdout() as stdout: - contenttypes_management.update_contenttypes(self.app_config) + with mock.patch( + 'django.contrib.contenttypes.management.commands.remove_stale_contenttypes.input', + return_value='yes' + ): + with captured_stdout() as stdout: + call_command('remove_stale_contenttypes', verbosity=2, stdout=stdout) self.assertEqual(Post.objects.count(), 0) output = stdout.getvalue() self.assertIn('- Content type for contenttypes_tests.Fake', output) @@ -408,33 +412,35 @@ class UpdateContentTypesTests(TestCase): def test_interactive_true_without_dependent_objects(self): """ - interactive mode of update_contenttypes() (the default) should delete - stale contenttypes even if there aren't any dependent objects. + interactive mode of remove_stale_contenttypes (the default) should + delete stale contenttypes even if there aren't any dependent objects. """ - contenttypes_management.input = lambda x: force_str("yes") - with captured_stdout() as stdout: - contenttypes_management.update_contenttypes(self.app_config) + with mock.patch( + 'django.contrib.contenttypes.management.commands.remove_stale_contenttypes.input', + return_value='yes' + ): + with captured_stdout() as stdout: + call_command('remove_stale_contenttypes', verbosity=2) self.assertIn("Deleting stale content type", stdout.getvalue()) self.assertEqual(ContentType.objects.count(), self.before_count) def test_interactive_false(self): """ - non-interactive mode of update_contenttypes() shouldn't delete stale - content types. + non-interactive mode of remove_stale_contenttypes shouldn't delete + stale content types. """ with captured_stdout() as stdout: - contenttypes_management.update_contenttypes(self.app_config, interactive=False) + call_command('remove_stale_contenttypes', interactive=False, verbosity=2) self.assertIn("Stale content types remain.", stdout.getvalue()) self.assertEqual(ContentType.objects.count(), self.before_count + 1) def test_unavailable_content_type_model(self): """ - #24075 - A ContentType shouldn't be created or deleted if the model - isn't available. + A ContentType shouldn't be created if the model isn't available. """ apps = Apps() with self.assertNumQueries(0): - contenttypes_management.update_contenttypes(self.app_config, interactive=False, verbosity=0, apps=apps) + contenttypes_management.create_contenttypes(self.app_config, interactive=False, verbosity=0, apps=apps) self.assertEqual(ContentType.objects.count(), self.before_count + 1)