Replaced ad-hoc caching of get_models with lru_cache.

Invalidate properly the cache whenever all_models or app_configs change.
This fixes some isolation issues in the test suite.

8 files changed, 15 insertions, 27 deletions

diff --git a/django/apps/cache.py b/django/apps/cache.py
index 840b9fbf68..a67b3cf999 100644
--- a/django/apps/cache.py
+++ b/django/apps/cache.py
@@ -7,6 +7,7 @@ import warnings
 
 from django.conf import settings
 from django.core.exceptions import ImproperlyConfigured
+from django.utils import lru_cache
 from django.utils.module_loading import import_lock
 from django.utils._os import upath
 
@@ -49,9 +50,6 @@ class AppCache(object):
         # Pending lookups for lazy relations.
         self._pending_lookups = {}
 
-        # Cache for get_models.
-        self._get_models_cache = {}
-
     def populate_apps(self, installed_apps=None):
         """
         Populate app-related information.
@@ -83,6 +81,7 @@ class AppCache(object):
                 app_config = AppConfig.create(app_name)
                 self.app_configs[app_config.label] = app_config
 
+            self.get_models.cache_clear()
             self._apps_loaded = True
 
     def populate_models(self):
@@ -131,6 +130,7 @@ class AppCache(object):
 
                 del self._postponed
 
+                self.get_models.cache_clear()
                 self._models_loaded = True
 
     def app_cache_ready(self):
@@ -180,6 +180,8 @@ class AppCache(object):
             raise LookupError("App with label %r doesn't have a models module." % app_label)
         return app_config
 
+    # This method is performance-critical at least for Django's test suite.
+    @lru_cache.lru_cache(maxsize=None)
     def get_models(self, app_mod=None,
                    include_auto_created=False, include_deferred=False,
                    only_installed=True, include_swapped=False):
@@ -206,12 +208,7 @@ class AppCache(object):
         """
         if not self.master:
             only_installed = False
-        cache_key = (app_mod, include_auto_created, include_deferred, only_installed, include_swapped)
         model_list = None
-        try:
-            return self._get_models_cache[cache_key]
-        except KeyError:
-            pass
         self.populate_models()
         if app_mod:
             app_label = app_mod.__name__.split('.')[-2]
@@ -235,7 +232,6 @@ class AppCache(object):
                     (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
 
     def get_model(self, app_label, model_name, only_installed=True):
@@ -272,7 +268,7 @@ class AppCache(object):
             if os.path.splitext(fname1)[0] == os.path.splitext(fname2)[0]:
                 return
         models[model_name] = model
-        self._get_models_cache.clear()
+        self.get_models.cache_clear()
 
     def has_app(self, app_name):
         """
@@ -368,6 +364,7 @@ class AppCache(object):
         app_config = AppConfig.create(app_name)
         app_config.import_models(self.all_models[app_config.label])
         self.app_configs[app_config.label] = app_config
+        self.get_models.cache_clear()
         return app_config.models_module
 
     def get_app(self, app_label):
diff --git a/tests/app_loading/tests.py b/tests/app_loading/tests.py
index a9dc70f4b1..291877108f 100644
--- a/tests/app_loading/tests.py
+++ b/tests/app_loading/tests.py
@@ -21,7 +21,7 @@ class EggLoadingTest(TestCase):
 
     def tearDown(self):
         app_cache.all_models['app_loading'] = self._old_models
-        app_cache._get_models_cache = {}
+        app_cache.get_models.cache_clear()
 
         sys.path = self.old_path
 
diff --git a/tests/managers_regress/tests.py b/tests/managers_regress/tests.py
index 2b8e183e82..bf8e5b17d4 100644
--- a/tests/managers_regress/tests.py
+++ b/tests/managers_regress/tests.py
@@ -128,7 +128,7 @@ class ManagersRegressionTests(TestCase):
         finally:
             app_cache.app_configs['managers_regress'].models = _old_models
             app_cache.all_models['managers_regress'] = _old_models
-            app_cache._get_models_cache = {}
+            app_cache.get_models.cache_clear()
 
     @override_settings(TEST_SWAPPABLE_MODEL='managers_regress.Parent')
     def test_custom_swappable_manager(self):
@@ -156,7 +156,7 @@ class ManagersRegressionTests(TestCase):
         finally:
             app_cache.app_configs['managers_regress'].models = _old_models
             app_cache.all_models['managers_regress'] = _old_models
-            app_cache._get_models_cache = {}
+            app_cache.get_models.cache_clear()
 
     @override_settings(TEST_SWAPPABLE_MODEL='managers_regress.Parent')
     def test_explicit_swappable_manager(self):
@@ -184,7 +184,7 @@ class ManagersRegressionTests(TestCase):
         finally:
             app_cache.app_configs['managers_regress'].models = _old_models
             app_cache.all_models['managers_regress'] = _old_models
-            app_cache._get_models_cache = {}
+            app_cache.get_models.cache_clear()
 
     def test_regress_3871(self):
         related = RelatedModel.objects.create()
diff --git a/tests/migrations/test_commands.py b/tests/migrations/test_commands.py
index 9f740f72a2..9fa9be6cbf 100644
--- a/tests/migrations/test_commands.py
+++ b/tests/migrations/test_commands.py
@@ -136,7 +136,7 @@ class MakeMigrationsTests(MigrationTestBase):
     def tearDown(self):
         app_cache.app_configs['migrations'].models = self._old_models
         app_cache.all_models['migrations'] = self._old_models
-        app_cache._get_models_cache = {}
+        app_cache.get_models.cache_clear()
 
         os.chdir(self.test_dir)
         try:
diff --git a/tests/proxy_models/tests.py b/tests/proxy_models/tests.py
index 346924e320..4a3d6e5e5a 100644
--- a/tests/proxy_models/tests.py
+++ b/tests/proxy_models/tests.py
@@ -175,7 +175,7 @@ class ProxyModelTests(TestCase):
         finally:
             app_cache.app_configs['proxy_models'].models = _old_models
             app_cache.all_models['proxy_models'] = _old_models
-            app_cache._get_models_cache = {}
+            app_cache.get_models.cache_clear()
 
     def test_myperson_manager(self):
         Person.objects.create(name="fred")
diff --git a/tests/runtests.py b/tests/runtests.py
index c75d8a3ee9..ab6f778a81 100755
--- a/tests/runtests.py
+++ b/tests/runtests.py
@@ -170,6 +170,7 @@ def setup(verbosity, test_labels):
             app_config = AppConfig.create(module_label)
             app_config.import_models(app_cache.all_models[app_config.label])
             app_cache.app_configs[app_config.label] = app_config
+            app_cache.get_models.cache_clear()
 
     return state
 
diff --git a/tests/swappable_models/tests.py b/tests/swappable_models/tests.py
index cdebbef374..c703a99fcb 100644
--- a/tests/swappable_models/tests.py
+++ b/tests/swappable_models/tests.py
@@ -19,16 +19,6 @@ class SwappableModelTests(TestCase):
         'django.contrib.contenttypes',
     ]
 
-    def setUp(self):
-        # This test modifies the installed apps, so we need to make sure
-        # we're not dealing with a cached app list.
-        app_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.
-        app_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"
diff --git a/tests/tablespaces/tests.py b/tests/tablespaces/tests.py
index d761a93ab9..fdee877a9f 100644
--- a/tests/tablespaces/tests.py
+++ b/tests/tablespaces/tests.py
@@ -37,7 +37,7 @@ class TablespacesTests(TestCase):
 
         app_cache.app_configs['tablespaces'].models = self._old_models
         app_cache.all_models['tablespaces'] = self._old_models
-        app_cache._get_models_cache = {}
+        app_cache.get_models.cache_clear()
 
     def assertNumContains(self, haystack, needle, count):
         real_count = haystack.count(needle)