From c8985a8a7317042a641e870cb75b3005cc5d67b1 Mon Sep 17 00:00:00 2001 From: Russell Keith-Magee Date: Sat, 24 Nov 2012 13:43:20 +0800 Subject: [PATCH] Fixed #19806 -- Ensure that content types and permissions aren't created for swapped models. Thanks to rizumu for the report. --- django/contrib/contenttypes/management.py | 2 + django/core/management/validation.py | 28 ++++++----- django/db/models/loading.py | 33 ++++++++----- .../swappable_models/__init__.py | 0 .../swappable_models/models.py | 15 ++++++ .../regressiontests/swappable_models/tests.py | 46 +++++++++++++++++++ 6 files changed, 101 insertions(+), 23 deletions(-) create mode 100644 tests/regressiontests/swappable_models/__init__.py create mode 100644 tests/regressiontests/swappable_models/models.py create mode 100644 tests/regressiontests/swappable_models/tests.py diff --git a/django/contrib/contenttypes/management.py b/django/contrib/contenttypes/management.py index 85f76bac77..8329ab65d9 100644 --- a/django/contrib/contenttypes/management.py +++ b/django/contrib/contenttypes/management.py @@ -5,6 +5,7 @@ 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, db=DEFAULT_DB_ALIAS, **kwargs): """ Creates content types for models in the given app, removing any model @@ -77,6 +78,7 @@ If you're unsure, answer 'no'. if verbosity >= 2: print("Stale content types remain.") + def update_all_contenttypes(verbosity=2, **kwargs): for app in get_apps(): update_contenttypes(app, None, verbosity, **kwargs) diff --git a/django/core/management/validation.py b/django/core/management/validation.py index 32e7181dab..af15752f0f 100644 --- a/django/core/management/validation.py +++ b/django/core/management/validation.py @@ -35,7 +35,10 @@ def get_validation_errors(outfile, app=None): for (app_name, error) in get_app_errors().items(): e.add(app_name, error) - for cls in models.get_models(app): + inc = set(models.get_models(app, include_swapped=True)) + no_inc = set(models.get_models(app)) + + for cls in models.get_models(app, include_swapped=True): opts = cls._meta # Check swappable attribute. @@ -138,16 +141,17 @@ def get_validation_errors(outfile, app=None): # fields, m2m fields, m2m related objects or related objects if f.rel: if f.rel.to not in models.get_models(): - e.add(opts, "'%s' has a relation with model %s, which has either not been installed or is abstract." % (f.name, f.rel.to)) + # If the related model is swapped, provide a hint; + # otherwise, the model just hasn't been installed. + if not isinstance(f.rel.to, six.string_types) and f.rel.to._meta.swapped: + e.add(opts, "'%s' defines a relation with the model '%s.%s', which has been swapped out. Update the relation to point at settings.%s." % (f.name, f.rel.to._meta.app_label, f.rel.to._meta.object_name, f.rel.to._meta.swappable)) + else: + e.add(opts, "'%s' has a relation with model %s, which has either not been installed or is abstract." % (f.name, f.rel.to)) # it is a string and we could not find the model it refers to # so skip the next section if isinstance(f.rel.to, six.string_types): continue - # Make sure the model we're related hasn't been swapped out - if f.rel.to._meta.swapped: - e.add(opts, "'%s' defines a relation with the model '%s.%s', which has been swapped out. Update the relation to point at settings.%s." % (f.name, f.rel.to._meta.app_label, f.rel.to._meta.object_name, f.rel.to._meta.swappable)) - # Make sure the related field specified by a ForeignKey is unique if not f.rel.to._meta.get_field(f.rel.field_name).unique: e.add(opts, "Field '%s' under model '%s' must have a unique=True constraint." % (f.rel.field_name, f.rel.to.__name__)) @@ -184,16 +188,18 @@ def get_validation_errors(outfile, app=None): # existing fields, m2m fields, m2m related objects or related # objects if f.rel.to not in models.get_models(): - e.add(opts, "'%s' has an m2m relation with model %s, which has either not been installed or is abstract." % (f.name, f.rel.to)) + # If the related model is swapped, provide a hint; + # otherwise, the model just hasn't been installed. + if not isinstance(f.rel.to, six.string_types) and f.rel.to._meta.swapped: + e.add(opts, "'%s' defines a relation with the model '%s.%s', which has been swapped out. Update the relation to point at settings.%s." % (f.name, f.rel.to._meta.app_label, f.rel.to._meta.object_name, f.rel.to._meta.swappable)) + else: + e.add(opts, "'%s' has an m2m relation with model %s, which has either not been installed or is abstract." % (f.name, f.rel.to)) + # it is a string and we could not find the model it refers to # so skip the next section if isinstance(f.rel.to, six.string_types): continue - # Make sure the model we're related hasn't been swapped out - if f.rel.to._meta.swapped: - e.add(opts, "'%s' defines a relation with the model '%s.%s', which has been swapped out. Update the relation to point at settings.%s." % (f.name, f.rel.to._meta.app_label, f.rel.to._meta.object_name, f.rel.to._meta.swappable)) - # Check that the field is not set to unique. ManyToManyFields do not support unique. if f.unique: e.add(opts, "ManyToManyFields cannot be unique. Remove the unique argument on '%s'." % f.name) diff --git a/django/db/models/loading.py b/django/db/models/loading.py index 8a0e796f4b..a0510acc6d 100644 --- a/django/db/models/loading.py +++ b/django/db/models/loading.py @@ -24,24 +24,24 @@ class AppCache(object): # http://aspn.activestate.com/ASPN/Cookbook/Python/Recipe/66531. __shared_state = dict( # Keys of app_store are the model modules for each application. - app_store = SortedDict(), + app_store=SortedDict(), # Mapping of installed app_labels to model modules for that app. - app_labels = {}, + app_labels={}, # Mapping of app_labels to a dictionary of model names to model code. # May contain apps that are not installed. - app_models = SortedDict(), + app_models=SortedDict(), # Mapping of app_labels to errors raised when trying to import the app. - app_errors = {}, + app_errors={}, # -- Everything below here is only used when populating the cache -- - loaded = False, - handled = {}, - postponed = [], - nesting_level = 0, - _get_models_cache = {}, + loaded=False, + handled={}, + postponed=[], + nesting_level=0, + _get_models_cache={}, ) def __init__(self): @@ -167,7 +167,7 @@ class AppCache(object): def get_models(self, app_mod=None, include_auto_created=False, include_deferred=False, - only_installed=True): + only_installed=True, include_swapped=False): """ Given a module containing models, returns a list of the models. Otherwise returns a list of all installed models. @@ -179,8 +179,16 @@ class AppCache(object): By default, models created to satisfy deferred attribute queries are *not* included in the list of models. However, if you specify include_deferred, they will be. + + By default, models that aren't part of installed apps will *not* + be included in the list of models. However, if you specify + only_installed=False, they will be. + + By default, models that have been swapped out will *not* be + included in the list of models. However, if you specify + include_swapped, they will be. """ - cache_key = (app_mod, include_auto_created, include_deferred, only_installed) + cache_key = (app_mod, include_auto_created, include_deferred, only_installed, include_swapped) try: return self._get_models_cache[cache_key] except KeyError: @@ -203,7 +211,8 @@ class AppCache(object): model_list.extend( model for model in app.values() if ((not model._deferred or include_deferred) and - (not model._meta.auto_created or include_auto_created)) + (not model._meta.auto_created or include_auto_created) and + (not model._meta.swapped or include_swapped)) ) self._get_models_cache[cache_key] = model_list return model_list diff --git a/tests/regressiontests/swappable_models/__init__.py b/tests/regressiontests/swappable_models/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/regressiontests/swappable_models/models.py b/tests/regressiontests/swappable_models/models.py new file mode 100644 index 0000000000..92692d4396 --- /dev/null +++ b/tests/regressiontests/swappable_models/models.py @@ -0,0 +1,15 @@ +from django.db import models + + +class Article(models.Model): + title = models.CharField(max_length=100) + publication_date = models.DateField() + + class Meta: + swappable = 'TEST_ARTICLE_MODEL' + + +class AlternateArticle(models.Model): + title = models.CharField(max_length=100) + publication_date = models.DateField() + byline = models.CharField(max_length=100) diff --git a/tests/regressiontests/swappable_models/tests.py b/tests/regressiontests/swappable_models/tests.py new file mode 100644 index 0000000000..d9a01f9e26 --- /dev/null +++ b/tests/regressiontests/swappable_models/tests.py @@ -0,0 +1,46 @@ +from __future__ import absolute_import, unicode_literals + +from django.utils.six import StringIO + +from django.contrib.auth.models import Permission +from django.contrib.contenttypes.models import ContentType +from django.core import management +from django.db.models.loading import cache +from django.test import TestCase +from django.test.utils import override_settings + + +class SwappableModelTests(TestCase): + def setUp(self): + # This test modifies the installed apps, so we need to make sure + # we're not dealing with a cached app list. + cache._get_models_cache.clear() + + def tearDown(self): + # By fiddling with swappable models, we alter the installed models + # cache, so flush it to make sure there are no side effects. + cache._get_models_cache.clear() + + @override_settings(TEST_ARTICLE_MODEL='swappable_models.AlternateArticle') + def test_generated_data(self): + "Permissions and content types are not created for a swapped model" + + # Delete all permissions and content_types + Permission.objects.all().delete() + ContentType.objects.all().delete() + + # Re-run syncdb. This will re-build the permissions and content types. + new_io = StringIO() + management.call_command('syncdb', load_initial_data=False, interactive=False, stdout=new_io) + + # Check that content types and permissions exist for the swapped model, + # but not for the swappable model. + apps_models = [(p.content_type.app_label, p.content_type.model) + for p in Permission.objects.all()] + self.assertIn(('swappable_models', 'alternatearticle'), apps_models) + self.assertNotIn(('swappable_models', 'article'), apps_models) + + apps_models = [(ct.app_label, ct.model) + for ct in ContentType.objects.all()] + self.assertIn(('swappable_models', 'alternatearticle'), apps_models) + self.assertNotIn(('swappable_models', 'article'), apps_models)