From 6bdaef26ec9cc9841b944e1d18b13185f7bbe2f5 Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Wed, 20 Apr 2011 17:58:37 +0000 Subject: [PATCH] Fixed #15866, #15850 -- Prevented get_model() and get_models() from returning not-installed models (by default). Thanks adsva for report. git-svn-id: http://code.djangoproject.com/svn/django/trunk@16053 bcc190cf-cafb-0310-a4f2-bffc1f526a37 --- django/contrib/contenttypes/models.py | 3 +- django/db/models/base.py | 6 ++-- django/db/models/fields/related.py | 3 +- django/db/models/loading.py | 33 +++++++++++++++---- tests/regressiontests/admin_scripts/tests.py | 6 +++- .../app_loading/not_installed/__init__.py | 0 .../app_loading/not_installed/models.py | 5 +++ tests/regressiontests/app_loading/tests.py | 23 ++++++++++++- 8 files changed, 67 insertions(+), 12 deletions(-) create mode 100644 tests/regressiontests/app_loading/not_installed/__init__.py create mode 100644 tests/regressiontests/app_loading/not_installed/models.py diff --git a/django/contrib/contenttypes/models.py b/django/contrib/contenttypes/models.py index af3abf249f..06e06bcc4e 100644 --- a/django/contrib/contenttypes/models.py +++ b/django/contrib/contenttypes/models.py @@ -90,7 +90,8 @@ class ContentType(models.Model): def model_class(self): "Returns the Python model class for this type of content." from django.db import models - return models.get_model(self.app_label, self.model) + return models.get_model(self.app_label, self.model, + only_installed=False) def get_object_for_this_type(self, **kwargs): """ diff --git a/django/db/models/base.py b/django/db/models/base.py index d352b9d5ae..9cf083416c 100644 --- a/django/db/models/base.py +++ b/django/db/models/base.py @@ -88,7 +88,8 @@ class ModelBase(type): new_class._base_manager = new_class._base_manager._copy_to_model(new_class) # Bail out early if we have already created this class. - m = get_model(new_class._meta.app_label, name, False) + m = get_model(new_class._meta.app_label, name, + seed_cache=False, only_installed=False) if m is not None: return m @@ -201,7 +202,8 @@ class ModelBase(type): # the first time this model tries to register with the framework. There # should only be one class for each model, so we always return the # registered version. - return get_model(new_class._meta.app_label, name, False) + return get_model(new_class._meta.app_label, name, + seed_cache=False, only_installed=False) def copy_managers(cls, base_managers): # This is in-place sorting of an Options attribute, but that's fine. diff --git a/django/db/models/fields/related.py b/django/db/models/fields/related.py index 1318706040..ffa5692646 100644 --- a/django/db/models/fields/related.py +++ b/django/db/models/fields/related.py @@ -67,7 +67,8 @@ def add_lazy_relation(cls, field, relation, operation): # string right away. If get_model returns None, it means that the related # model isn't loaded yet, so we need to pend the relation until the class # is prepared. - model = get_model(app_label, model_name, False) + model = get_model(app_label, model_name, + seed_cache=False, only_installed=False) if model: operation(field, model, cls) else: diff --git a/django/db/models/loading.py b/django/db/models/loading.py index 620cebc25c..6fab4a931b 100644 --- a/django/db/models/loading.py +++ b/django/db/models/loading.py @@ -25,7 +25,11 @@ class AppCache(object): # Keys of app_store are the model modules for each application. app_store = SortedDict(), + # Mapping of installed app_labels to model modules for that app. + 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(), # Mapping of app_labels to errors raised when trying to import the app. @@ -66,6 +70,13 @@ class AppCache(object): finally: self.write_lock.release() + def _label_for(self, app_mod): + """ + Return app_label for given models module. + + """ + return app_mod.__name__.split('.')[-2] + def load_app(self, app_name, can_postpone=False): """ Loads the app with the provided fully qualified name, and returns the @@ -99,6 +110,7 @@ class AppCache(object): self.nesting_level -= 1 if models not in self.app_store: self.app_store[models] = len(self.app_store) + self.app_labels[self._label_for(models)] = models return models def app_cache_ready(self): @@ -146,7 +158,8 @@ class AppCache(object): self._populate() return self.app_errors - def get_models(self, app_mod=None, include_auto_created=False, include_deferred=False): + def get_models(self, app_mod=None, + include_auto_created=False, include_deferred=False): """ Given a module containing models, returns a list of the models. Otherwise returns a list of all installed models. @@ -166,20 +179,26 @@ class AppCache(object): pass self._populate() if app_mod: - app_list = [self.app_models.get(app_mod.__name__.split('.')[-2], SortedDict())] + if app_mod in self.app_store: + app_list = [self.app_models.get(self._label_for(app_mod), + SortedDict())] + else: + app_list = [] else: - app_list = self.app_models.itervalues() + app_list = [self.app_models.get(app_label, SortedDict()) + for app_label in self.app_labels.iterkeys()] model_list = [] for app in app_list: 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)) + if ((not model._deferred or include_deferred) and + (not model._meta.auto_created or include_auto_created)) ) self._get_models_cache[cache_key] = model_list return model_list - def get_model(self, app_label, model_name, seed_cache=True): + def get_model(self, app_label, model_name, + seed_cache=True, only_installed=True): """ Returns the model matching the given app_label and case-insensitive model_name. @@ -188,6 +207,8 @@ class AppCache(object): """ if seed_cache: self._populate() + if only_installed and app_label not in self.app_labels: + return None return self.app_models.get(app_label, SortedDict()).get(model_name.lower()) def register_models(self, app_label, *models): diff --git a/tests/regressiontests/admin_scripts/tests.py b/tests/regressiontests/admin_scripts/tests.py index 32183fad3e..a8c5e882f1 100644 --- a/tests/regressiontests/admin_scripts/tests.py +++ b/tests/regressiontests/admin_scripts/tests.py @@ -1023,7 +1023,11 @@ class ManageValidate(AdminScriptTestCase): def test_app_with_import(self): "manage.py validate does not raise errors when an app imports a base class that itself has an abstract base" self.write_settings('settings.py', - apps=['admin_scripts.app_with_import', 'django.contrib.comments'], + apps=['admin_scripts.app_with_import', + 'django.contrib.comments', + 'django.contrib.auth', + 'django.contrib.contenttypes', + 'django.contrib.sites'], sdict={'DEBUG': True}) args = ['validate'] out, err = self.run_manage(args) diff --git a/tests/regressiontests/app_loading/not_installed/__init__.py b/tests/regressiontests/app_loading/not_installed/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/regressiontests/app_loading/not_installed/models.py b/tests/regressiontests/app_loading/not_installed/models.py new file mode 100644 index 0000000000..615feefddb --- /dev/null +++ b/tests/regressiontests/app_loading/not_installed/models.py @@ -0,0 +1,5 @@ +from django.db import models + + +class NotInstalledModel(models.Model): + pass diff --git a/tests/regressiontests/app_loading/tests.py b/tests/regressiontests/app_loading/tests.py index 78e6519e35..6ee06c50c9 100644 --- a/tests/regressiontests/app_loading/tests.py +++ b/tests/regressiontests/app_loading/tests.py @@ -4,7 +4,7 @@ import sys import time from django.conf import Settings -from django.db.models.loading import cache, load_app +from django.db.models.loading import cache, load_app, get_model, get_models from django.utils.unittest import TestCase @@ -81,3 +81,24 @@ class EggLoadingTest(TestCase): # Make sure the message is indicating the actual # problem in the broken app. self.assertTrue("modelz" in e.args[0]) + + +class GetModelsTest(TestCase): + def setUp(self): + import not_installed.models + self.not_installed_module = not_installed.models + + + def test_get_model_only_returns_installed_models(self): + self.assertEqual( + get_model("not_installed", "NotInstalledModel"), None) + + + def test_get_models_only_returns_installed_models(self): + self.assertFalse( + "NotInstalledModel" in + [m.__name__ for m in get_models()]) + + + def test_get_models_with_app_label_only_returns_installed_models(self): + self.assertEqual(get_models(self.not_installed_module), [])