From 8db889eaf7dce0cb715b075be32047c1b1b316da Mon Sep 17 00:00:00 2001 From: Erik Romijn Date: Sun, 3 Jul 2016 15:55:14 +0200 Subject: [PATCH] Fixed #18682 -- Expanded explanation in stale content type deletion. (#6869) --- django/contrib/contenttypes/management.py | 46 ++++++++++++++++++----- docs/releases/1.11.txt | 5 ++- tests/contenttypes_tests/tests.py | 22 +++++++++-- 3 files changed, 59 insertions(+), 14 deletions(-) diff --git a/django/contrib/contenttypes/management.py b/django/contrib/contenttypes/management.py index 0bdf8ed9fa..591f96705b 100644 --- a/django/contrib/contenttypes/management.py +++ b/django/contrib/contenttypes/management.py @@ -1,5 +1,6 @@ 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 @@ -141,21 +142,39 @@ def update_contenttypes(app_config, verbosity=2, interactive=True, using=DEFAULT 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: - content_type_display = '\n'.join( - ' %s | %s' % (ct.app_label, ct.model) - for ct in to_remove - ) - ok_to_delete = input("""The following content types are stale and need to be deleted: + 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 object%s of type %s.%s:' % ( + len(objs), + 's' if len(objs) != 1 else '', + obj_type._meta.app_label, + obj_type._meta.model_name) + ) + + 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 then also be deleted. +The content types, and the dependent objects that would be deleted, are: %s -Any objects related to these content types by a foreign key will also -be deleted. Are you sure you want to delete these content types? -If you're unsure, answer 'no'. +This list does not include data that might be in your database +outside of Django's models. - Type 'yes' to continue, or 'no' to cancel: """ % content_type_display) +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 @@ -167,3 +186,12 @@ If you're unsure, answer 'no'. else: if verbosity >= 2: print("Stale content types remain.") + + +class NoFastDeleteCollector(Collector): + def can_fast_delete(self, *args, **kwargs): + """ + We always want to load the objects into memory so that we can display + them to the user when asking confirmation. + """ + return False diff --git a/docs/releases/1.11.txt b/docs/releases/1.11.txt index 1d79b7ffca..2359eec910 100644 --- a/docs/releases/1.11.txt +++ b/docs/releases/1.11.txt @@ -73,7 +73,10 @@ Minor features :mod:`django.contrib.contenttypes` ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -* ... +* When stale content types are detected during a management command, there is + now an expansive list of objects that will be deleted. Previously, only + the content type objects themselves were listed, even if there were objects + with foreign keys towards the content types that would be deleted also. :mod:`django.contrib.gis` ~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/tests/contenttypes_tests/tests.py b/tests/contenttypes_tests/tests.py index 123e169fbc..5c8c675a4a 100644 --- a/tests/contenttypes_tests/tests.py +++ b/tests/contenttypes_tests/tests.py @@ -20,7 +20,7 @@ from django.test.utils import captured_stdout, isolate_apps from django.utils.encoding import force_str, force_text from .models import ( - Article, Author, ModelWithNullFKToSite, SchemeIncludedURL, + Article, Author, ModelWithNullFKToSite, Post, SchemeIncludedURL, Site as MockSite, ) @@ -383,13 +383,27 @@ class GenericRelationshipTests(SimpleTestCase): class UpdateContentTypesTests(TestCase): def setUp(self): self.before_count = ContentType.objects.count() - ContentType.objects.create(app_label='contenttypes_tests', model='Fake') + self.content_type = ContentType.objects.create(app_label='contenttypes_tests', model='Fake') self.app_config = apps.get_app_config('contenttypes_tests') - def test_interactive_true(self): + def test_interactive_true_with_dependent_objects(self): """ interactive mode of update_contenttypes() (the default) should delete - stale contenttypes. + stale contenttypes and warn of dependent objects + """ + Post.objects.create(title='post', content_type=self.content_type) + contenttypes_management.input = lambda x: force_str("yes") + with captured_stdout() as stdout: + contenttypes_management.update_contenttypes(self.app_config) + self.assertEqual(Post.objects.count(), 0) + self.assertIn("1 object of type contenttypes_tests.post:", stdout.getvalue()) + self.assertIn("Deleting stale content type", stdout.getvalue()) + self.assertEqual(ContentType.objects.count(), self.before_count) + + def test_interactive_true_without_dependent_objects(self): + """ + interactive mode of update_contenttypes() (the default) should delete + stale contenttypes and inform there are no dependent objects """ contenttypes_management.input = lambda x: force_str("yes") with captured_stdout() as stdout: