From 0242c56fd86137bc8f67404492b5c03987bc3d1a Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Tue, 17 Dec 2013 17:47:19 +0100 Subject: [PATCH] Deborgified the app cache. Improved Andrew's hack to create temporary app caches to handle migrations. Now the main app cache has a "master" flag set to True (which is a non-default keyword argument, thus unlikely to be used by mistake). Other app cache instances have "master" set to False. The only sanctioned way to access the app cache is by importing django.core.apps.app_cache. If you were instanciating an app cache and relying on the Borg pattern, you'll have to refactor your code. --- django/core/apps/cache.py | 74 ++++++++++------------------ django/db/backends/sqlite3/schema.py | 4 +- django/db/migrations/recorder.py | 4 +- django/db/migrations/state.py | 4 +- tests/app_cache/models.py | 4 +- tests/app_cache/tests.py | 11 ++++- tests/app_loading/tests.py | 19 +++---- tests/contenttypes_tests/tests.py | 4 +- tests/migrations/models.py | 6 +-- tests/migrations/test_state.py | 20 ++++---- tests/schema/models.py | 4 +- tests/validation/test_unique.py | 4 +- 12 files changed, 71 insertions(+), 87 deletions(-) diff --git a/django/core/apps/cache.py b/django/core/apps/cache.py index 9b847cc309..d04d6c59a4 100644 --- a/django/core/apps/cache.py +++ b/django/core/apps/cache.py @@ -23,47 +23,41 @@ class UnavailableApp(Exception): pass -def _initialize(): +class AppCache(object): """ - Returns a dictionary to be used as the initial value of the - [shared] state of the app cache. + A cache that stores installed applications and their models. Used to + provide reverse-relations and for app introspection. """ - return dict( + + def __init__(self, master=False): + # Only one master of the app-cache may exist at a given time, and it + # shall be the app_cache variable defined at the end of this module. + if master and hasattr(sys.modules[__name__], 'app_cache'): + raise RuntimeError("You may create only one master app cache.") + + # When master is set to False, the app cache isn't populated from + # INSTALLED_APPS and ignores the only_installed arguments to + # get_model[s]. + self.master = master + # Mapping of labels to AppConfig instances for installed apps. - app_configs=OrderedDict(), + self.app_configs=OrderedDict() # Pending lookups for lazy relations - pending_lookups={}, + self.pending_lookups={} # Set of app names. Allows restricting the set of installed apps. # Used by TransactionTestCase.available_apps for performance reasons. - available_apps=None, + self.available_apps=None # -- Everything below here is only used when populating the cache -- - loads_installed=True, - loaded=False, - handled=set(), - postponed=[], - nesting_level=0, - _get_models_cache={}, - ) + self.loaded=False + self.handled=set() + self.postponed=[] + self.nesting_level=0 + self._get_models_cache={} -class BaseAppCache(object): - """ - A cache that stores installed applications and their models. Used to - provide reverse-relations and for app introspection (e.g. admin). - - This provides the base (non-Borg) AppCache class - the AppCache - subclass adds borg-like behaviour for the few cases where it's needed. - """ - - def __init__(self): - self.__dict__ = _initialize() - # This stops populate loading from INSTALLED_APPS and ignores the - # only_installed arguments to get_model[s] - self.loads_installed = False - def populate(self): """ Fill in all the cache information. This method is threadsafe, in the @@ -72,7 +66,7 @@ class BaseAppCache(object): """ if self.loaded: return - if not self.loads_installed: + if not self.master: self.loaded = True return # Note that we want to use the import lock here - the app loading is @@ -217,7 +211,7 @@ class BaseAppCache(object): included in the list of models. However, if you specify include_swapped, they will be. """ - if not self.loads_installed: + if not self.master: only_installed = False cache_key = (app_mod, include_auto_created, include_deferred, only_installed, include_swapped) model_list = None @@ -271,7 +265,7 @@ class BaseAppCache(object): Raises UnavailableApp when set_available_apps() in in effect and doesn't include app_label. """ - if not self.loads_installed: + if not self.master: only_installed = False if seed_cache: self.populate() @@ -399,18 +393,4 @@ class BaseAppCache(object): self.register_model(app_label, model) -class AppCache(BaseAppCache): - """ - A cache that stores installed applications and their models. Used to - provide reverse-relations and for app introspection (e.g. admin). - - Borg version of the BaseAppCache class. - """ - - __shared_state = _initialize() - - def __init__(self): - self.__dict__ = self.__shared_state - - -app_cache = AppCache() +app_cache = AppCache(master=True) diff --git a/django/db/backends/sqlite3/schema.py b/django/db/backends/sqlite3/schema.py index 8314533bba..e2b9603819 100644 --- a/django/db/backends/sqlite3/schema.py +++ b/django/db/backends/sqlite3/schema.py @@ -1,4 +1,4 @@ -from django.core.apps.cache import BaseAppCache +from django.core.apps.cache import AppCache from django.db.backends.schema import BaseDatabaseSchemaEditor from django.db.models.fields.related import ManyToManyField @@ -39,7 +39,7 @@ class DatabaseSchemaEditor(BaseDatabaseSchemaEditor): del body[field.name] del mapping[field.column] # Work inside a new AppCache - app_cache = BaseAppCache() + app_cache = AppCache() # Construct a new model for the new state meta_contents = { 'app_label': model._meta.app_label, diff --git a/django/db/migrations/recorder.py b/django/db/migrations/recorder.py index 1c714ca272..c2cfd2e9af 100644 --- a/django/db/migrations/recorder.py +++ b/django/db/migrations/recorder.py @@ -1,4 +1,4 @@ -from django.core.apps.cache import BaseAppCache +from django.core.apps.cache import AppCache from django.db import models from django.utils.timezone import now @@ -22,7 +22,7 @@ class MigrationRecorder(object): applied = models.DateTimeField(default=now) class Meta: - app_cache = BaseAppCache() + app_cache = AppCache() app_label = "migrations" db_table = "django_migrations" diff --git a/django/db/migrations/state.py b/django/db/migrations/state.py index 2102773b71..c43728d58e 100644 --- a/django/db/migrations/state.py +++ b/django/db/migrations/state.py @@ -1,4 +1,4 @@ -from django.core.apps.cache import BaseAppCache +from django.core.apps.cache import AppCache from django.db import models from django.db.models.options import DEFAULT_NAMES, normalize_unique_together from django.utils import six @@ -32,7 +32,7 @@ class ProjectState(object): def render(self): "Turns the project state into actual models in a new AppCache" if self.app_cache is None: - self.app_cache = BaseAppCache() + self.app_cache = AppCache() # We keep trying to render the models in a loop, ignoring invalid # base errors, until the size of the unrendered models doesn't # decrease by at least one, meaning there's a base dependency loop/ diff --git a/tests/app_cache/models.py b/tests/app_cache/models.py index 99f9f57b67..10e9e6f1de 100644 --- a/tests/app_cache/models.py +++ b/tests/app_cache/models.py @@ -1,9 +1,9 @@ -from django.core.apps.cache import BaseAppCache +from django.core.apps.cache import AppCache from django.db import models # We're testing app cache presence on load, so this is handy. -new_app_cache = BaseAppCache() +new_app_cache = AppCache() class TotallyNormal(models.Model): diff --git a/tests/app_cache/tests.py b/tests/app_cache/tests.py index 29d5315de0..5a80155fec 100644 --- a/tests/app_cache/tests.py +++ b/tests/app_cache/tests.py @@ -1,7 +1,7 @@ from __future__ import absolute_import from django.core.apps import app_cache -from django.core.apps.cache import BaseAppCache +from django.core.apps.cache import AppCache from django.db import models from django.test import TestCase @@ -30,7 +30,7 @@ class AppCacheTests(TestCase): old_models = app_cache.get_models(app_cache.get_app_config("app_cache").models_module) # Construct a new model in a new app cache body = {} - new_app_cache = BaseAppCache() + new_app_cache = AppCache() meta_contents = { 'app_label': "app_cache", 'app_cache': new_app_cache, @@ -45,3 +45,10 @@ class AppCacheTests(TestCase): app_cache.get_models(app_cache.get_app_config("app_cache").models_module), ) self.assertEqual(new_app_cache.get_model("app_cache", "SouthPonies"), temp_model) + + def test_singleton_master(self): + """ + Ensures that only one master app cache can exist. + """ + with self.assertRaises(RuntimeError): + AppCache(master=True) diff --git a/tests/app_loading/tests.py b/tests/app_loading/tests.py index 3cc0ffb368..b32f510a24 100644 --- a/tests/app_loading/tests.py +++ b/tests/app_loading/tests.py @@ -75,17 +75,14 @@ class EggLoadingTest(TestCase): Test that repeated app loading doesn't succeed in case there is an error. Refs #17667. """ - # AppCache is a Borg, so we can instantiate one and change its - # loaded to False to force the following code to actually try to - # populate the cache. - a = AppCache() - a.loaded = False - try: - with override_settings(INSTALLED_APPS=('notexists',)): - self.assertRaises(ImportError, app_cache.get_model, 'notexists', 'nomodel', seed_cache=True) - self.assertRaises(ImportError, app_cache.get_model, 'notexists', 'nomodel', seed_cache=True) - finally: - a.loaded = True + app_cache = AppCache() + # Pretend we're the master app cache to test populate(). + app_cache.master = True + with override_settings(INSTALLED_APPS=('notexists',)): + with self.assertRaises(ImportError): + app_cache.get_model('notexists', 'nomodel', seed_cache=True) + with self.assertRaises(ImportError): + app_cache.get_model('notexists', 'nomodel', seed_cache=True) class GetModelsTest(TestCase): diff --git a/tests/contenttypes_tests/tests.py b/tests/contenttypes_tests/tests.py index ea61e5e893..9cc254e665 100644 --- a/tests/contenttypes_tests/tests.py +++ b/tests/contenttypes_tests/tests.py @@ -1,7 +1,7 @@ from __future__ import unicode_literals from django.contrib.contenttypes.models import ContentType -from django.core.apps.cache import BaseAppCache +from django.core.apps.cache import AppCache from django.db import models from django.test import TestCase @@ -61,7 +61,7 @@ class ContentTypesViewsTests(TestCase): class Meta: verbose_name = 'a model created on the fly' app_label = 'my_great_app' - app_cache = BaseAppCache() + app_cache = AppCache() ct = ContentType.objects.get_for_model(ModelCreatedOnTheFly) self.assertEqual(ct.app_label, 'my_great_app') diff --git a/tests/migrations/models.py b/tests/migrations/models.py index 03390b3a06..cfc9604551 100644 --- a/tests/migrations/models.py +++ b/tests/migrations/models.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- from __future__ import unicode_literals -from django.core.apps.cache import BaseAppCache +from django.core.apps.cache import AppCache from django.db import models from django.utils.encoding import python_2_unicode_compatible @@ -12,7 +12,7 @@ class UnicodeModel(models.Model): class Meta: # Disable auto loading of this model as we load it on our own - app_cache = BaseAppCache() + app_cache = AppCache() verbose_name = 'úñí©óðé µóðéø' verbose_name_plural = 'úñí©óðé µóðéøß' @@ -32,4 +32,4 @@ class UnserializableModel(models.Model): class Meta: # Disable auto loading of this model as we load it on our own - app_cache = BaseAppCache() + app_cache = AppCache() diff --git a/tests/migrations/test_state.py b/tests/migrations/test_state.py index 206bf78b35..7a06ecd474 100644 --- a/tests/migrations/test_state.py +++ b/tests/migrations/test_state.py @@ -1,4 +1,4 @@ -from django.core.apps.cache import BaseAppCache +from django.core.apps.cache import AppCache from django.db import models from django.db.migrations.state import ProjectState, ModelState, InvalidBasesError from django.test import TestCase @@ -14,7 +14,7 @@ class StateTests(TestCase): Tests making a ProjectState from an AppCache """ - new_app_cache = BaseAppCache() + new_app_cache = AppCache() class Author(models.Model): name = models.CharField(max_length=255) @@ -100,15 +100,15 @@ class StateTests(TestCase): class Meta: app_label = "migrations" - app_cache = BaseAppCache() + app_cache = AppCache() class Novel(Book): class Meta: app_label = "migrations" - app_cache = BaseAppCache() + app_cache = AppCache() # First, test rendering individually - app_cache = BaseAppCache() + app_cache = AppCache() # We shouldn't be able to render yet ms = ModelState.from_model(Novel) @@ -123,19 +123,19 @@ class StateTests(TestCase): class Foo(models.Model): class Meta: app_label = "migrations" - app_cache = BaseAppCache() + app_cache = AppCache() class Bar(models.Model): class Meta: app_label = "migrations" - app_cache = BaseAppCache() + app_cache = AppCache() class FooBar(Foo, Bar): class Meta: app_label = "migrations" - app_cache = BaseAppCache() + app_cache = AppCache() - app_cache = BaseAppCache() + app_cache = AppCache() # We shouldn't be able to render yet ms = ModelState.from_model(FooBar) @@ -152,7 +152,7 @@ class StateTests(TestCase): Tests that the ProjectState render method correctly renders models to account for inter-model base dependencies. """ - new_app_cache = BaseAppCache() + new_app_cache = AppCache() class A(models.Model): class Meta: diff --git a/tests/schema/models.py b/tests/schema/models.py index e1369bff05..8b24b88c05 100644 --- a/tests/schema/models.py +++ b/tests/schema/models.py @@ -1,11 +1,11 @@ -from django.core.apps.cache import BaseAppCache +from django.core.apps.cache import AppCache from django.db import models # Because we want to test creation and deletion of these as separate things, # these models are all inserted into a separate AppCache so the main test # runner doesn't migrate them. -new_app_cache = BaseAppCache() +new_app_cache = AppCache() class Author(models.Model): diff --git a/tests/validation/test_unique.py b/tests/validation/test_unique.py index 83863b19c6..f05256a6f1 100644 --- a/tests/validation/test_unique.py +++ b/tests/validation/test_unique.py @@ -3,7 +3,7 @@ from __future__ import unicode_literals import datetime import unittest -from django.core.apps.cache import BaseAppCache +from django.core.apps.cache import AppCache from django.core.exceptions import ValidationError from django.db import models from django.test import TestCase @@ -58,7 +58,7 @@ class GetUniqueCheckTests(unittest.TestCase): Meta = type(str('Meta'), (), { 'unique_together': unique_together, - 'app_cache': BaseAppCache() + 'app_cache': AppCache() }) checks, _ = M()._get_unique_checks()