Fixed #24865 -- Added remove_stale_contenttypes management command.

Thanks Simon Charette for the review.
This commit is contained in:
Tim Graham 2015-10-02 10:46:29 -04:00
parent 4c94336510
commit 6a2af01452
8 changed files with 164 additions and 106 deletions

View File

@ -5,7 +5,7 @@ from django.db.models.signals import post_migrate, pre_migrate
from django.utils.translation import ugettext_lazy as _ from django.utils.translation import ugettext_lazy as _
from .management import ( 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): def ready(self):
pre_migrate.connect(inject_rename_contenttypes_operations, sender=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) checks.register(check_generic_foreign_keys, checks.Tags.models)

View File

@ -1,9 +1,7 @@
from django.apps import apps as global_apps from django.apps import apps as global_apps
from django.db import DEFAULT_DB_ALIAS, migrations, router, transaction 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.db.utils import IntegrityError
from django.utils import six from django.utils import six
from django.utils.six.moves import input
class RenameContentType(migrations.RunPython): class RenameContentType(migrations.RunPython):
@ -30,8 +28,8 @@ class RenameContentType(migrations.RunPython):
content_type.save(update_fields={'model'}) content_type.save(update_fields={'model'})
except IntegrityError: except IntegrityError:
# Gracefully fallback if a stale content type causes a # Gracefully fallback if a stale content type causes a
# conflict as update_contenttypes will take care of asking the # conflict as remove_stale_contenttypes will take care of
# user what should be done next. # asking the user what should be done next.
content_type.model = old_model content_type.model = old_model
else: else:
# Clear the cache as the `get_by_natual_key()` call will cache # 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) 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 Creates content types for models in the given app.
entries that no longer have a matching model class.
""" """
if not app_config.models_module: if not app_config.models_module:
return return
@ -102,32 +116,11 @@ def update_contenttypes(app_config, verbosity=2, interactive=True, using=DEFAULT
except LookupError: except LookupError:
return return
if not router.allow_migrate_model(using, ContentType): content_types, app_models = get_contenttypes_and_models(app_config, 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()}
if not app_models: if not app_models:
return 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 = [ cts = [
ContentType( ContentType(
app_label=app_label, app_label=app_label,
@ -140,55 +133,3 @@ def update_contenttypes(app_config, verbosity=2, interactive=True, using=DEFAULT
if verbosity >= 2: if verbosity >= 2:
for ct in cts: for ct in cts:
print("Adding content type '%s | %s'" % (ct.app_label, ct.model)) 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

View File

@ -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

View File

@ -118,10 +118,7 @@ class ContentTypeManager(models.Manager):
def clear_cache(self): def clear_cache(self):
""" """
Clear out the content-type cache. This needs to happen during database Clear out the content-type cache.
flushes to prevent caching of "stale" content type IDs (see
django.contrib.contenttypes.management.update_contenttypes for where
this gets called).
""" """
self._cache.clear() self._cache.clear()

View File

@ -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 allow creating an instance instead of entering the primary key of an existing
instance. 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
</ref/contrib/contenttypes>` (: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`` ``django.contrib.gis``
---------------------- ----------------------

View File

@ -104,9 +104,11 @@ Minor features
:mod:`django.contrib.contenttypes` :mod:`django.contrib.contenttypes`
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
* When stale content types are detected after the ``migrate`` command, there's * When stale content types are detected in the
now a list of related objects such as ``auth.Permission``\s that will also be :djadmin:`remove_stale_contenttypes` command, there's now a list of related
deleted. Previously, only the content types were listed. 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` :mod:`django.contrib.gis`
~~~~~~~~~~~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~~
@ -477,6 +479,10 @@ Miscellaneous
be backwards-incompatible if you have some :ref:`template tags that aren't be backwards-incompatible if you have some :ref:`template tags that aren't
thread safe <template_tag_thread_safety>`. thread safe <template_tag_thread_safety>`.
* 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: .. _deprecated-features-1.11:
Features deprecated in 1.11 Features deprecated in 1.11

View File

@ -12,6 +12,7 @@ from django.contrib.contenttypes.fields import (
from django.contrib.contenttypes.models import ContentType from django.contrib.contenttypes.models import ContentType
from django.contrib.sites.models import Site from django.contrib.sites.models import Site
from django.core import checks, management from django.core import checks, management
from django.core.management import call_command
from django.db import connections, migrations, models from django.db import connections, migrations, models
from django.test import ( from django.test import (
SimpleTestCase, TestCase, TransactionTestCase, mock, override_settings, SimpleTestCase, TestCase, TransactionTestCase, mock, override_settings,
@ -388,16 +389,19 @@ class UpdateContentTypesTests(TestCase):
def test_interactive_true_with_dependent_objects(self): def test_interactive_true_with_dependent_objects(self):
""" """
interactive mode of update_contenttypes() (the default) should delete interactive mode of remove_stale_contenttypes (the default) should
stale contenttypes and warn of dependent objects. delete stale contenttypes and warn of dependent objects.
""" """
post = Post.objects.create(title='post', content_type=self.content_type) post = Post.objects.create(title='post', content_type=self.content_type)
# A related object is needed to show that a custom collector with # A related object is needed to show that a custom collector with
# can_fast_delete=False is needed. # can_fast_delete=False is needed.
ModelWithNullFKToSite.objects.create(post=post) ModelWithNullFKToSite.objects.create(post=post)
contenttypes_management.input = lambda x: force_str("yes") with mock.patch(
with captured_stdout() as stdout: 'django.contrib.contenttypes.management.commands.remove_stale_contenttypes.input',
contenttypes_management.update_contenttypes(self.app_config) return_value='yes'
):
with captured_stdout() as stdout:
call_command('remove_stale_contenttypes', verbosity=2, stdout=stdout)
self.assertEqual(Post.objects.count(), 0) self.assertEqual(Post.objects.count(), 0)
output = stdout.getvalue() output = stdout.getvalue()
self.assertIn('- Content type for contenttypes_tests.Fake', output) self.assertIn('- Content type for contenttypes_tests.Fake', output)
@ -408,33 +412,35 @@ class UpdateContentTypesTests(TestCase):
def test_interactive_true_without_dependent_objects(self): def test_interactive_true_without_dependent_objects(self):
""" """
interactive mode of update_contenttypes() (the default) should delete interactive mode of remove_stale_contenttypes (the default) should
stale contenttypes even if there aren't any dependent objects. delete stale contenttypes even if there aren't any dependent objects.
""" """
contenttypes_management.input = lambda x: force_str("yes") with mock.patch(
with captured_stdout() as stdout: 'django.contrib.contenttypes.management.commands.remove_stale_contenttypes.input',
contenttypes_management.update_contenttypes(self.app_config) return_value='yes'
):
with captured_stdout() as stdout:
call_command('remove_stale_contenttypes', verbosity=2)
self.assertIn("Deleting stale content type", stdout.getvalue()) self.assertIn("Deleting stale content type", stdout.getvalue())
self.assertEqual(ContentType.objects.count(), self.before_count) self.assertEqual(ContentType.objects.count(), self.before_count)
def test_interactive_false(self): def test_interactive_false(self):
""" """
non-interactive mode of update_contenttypes() shouldn't delete stale non-interactive mode of remove_stale_contenttypes shouldn't delete
content types. stale content types.
""" """
with captured_stdout() as stdout: 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.assertIn("Stale content types remain.", stdout.getvalue())
self.assertEqual(ContentType.objects.count(), self.before_count + 1) self.assertEqual(ContentType.objects.count(), self.before_count + 1)
def test_unavailable_content_type_model(self): def test_unavailable_content_type_model(self):
""" """
#24075 - A ContentType shouldn't be created or deleted if the model A ContentType shouldn't be created if the model isn't available.
isn't available.
""" """
apps = Apps() apps = Apps()
with self.assertNumQueries(0): 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) self.assertEqual(ContentType.objects.count(), self.before_count + 1)