App registration: Various minor changes after code review

django-cms · czpython · Jun 26, 2018 · Jun 14, 2018 · Jun 14, 2018 · Jun 15, 2018
commit d908a66a3a28961b33e7e3b8890a312b8921228c
diff --git a/CHANGELOG.txt b/CHANGELOG.txt
@@ -1,8 +1,3 @@
-=== 4.0.0 (unreleased) ===
-
-* Added app registration system.
-
-
 === 3.6.0 (unreleased) ===
 
 * Removed the ``cms moderator`` command.

diff --git a/cms/app_base.py b/cms/app_base.py
@@ -113,4 +113,4 @@ def configure_app(self, app):
 
         :param app: django app that has defined the feature as enabled
         """
-        raise NotImplementedError()
+        raise NotImplementedError
diff --git a/cms/app_registration.py b/cms/app_registration.py
@@ -1,5 +1,6 @@
 # -*- coding: utf-8 -*-
 import inspect
+from functools import lru_cache
 from importlib import import_module
 
 from django.apps import apps
@@ -52,18 +53,15 @@ def autodiscover_cms_configs():
                     "class which inherits from CMSAppConfig")
 
 
+@lru_cache(maxsize=None)
 def get_cms_apps():
     """
     Returns django app configs of apps with a cms config
     """
-    cms_apps = []
-
-    for app_config in apps.get_app_configs():
-        # The cms_app attr is added by the autodiscover_cms_configs
-        # function if a cms_config.py file with a suitable class is found.
-        if hasattr(app_config, 'cms_app'):
-            cms_apps.append(app_config)
-
+    # NOTE: The cms_app attr is added by the autodiscover_cms_configs
+    # function if a cms_config.py file with a suitable class is found.
+    cms_apps = [app_config for app_config in apps.get_app_configs()
+                if hasattr(app_config, 'cms_app')]
     return cms_apps
 
 

diff --git a/cms/test_utils/project/app_with_bad_cms_file/cms_config.py b/cms/test_utils/project/app_with_bad_cms_file/cms_config.py
@@ -1 +1,3 @@
-raise KeyError
+# Use this app to test what happens when exceptions are raised in this
+# file. RuntimeError is just an example exception
+raise RuntimeError
diff --git a/cms/tests/test_app_registration.py b/cms/tests/test_app_registration.py
@@ -32,10 +32,10 @@ def test_adds_cms_app_attribute_to_django_app_config(self):
         'cms.test_utils.project.app_with_bad_cms_file',
     ])
     def test_raises_exception_raised_in_cms_file(self):
-        # The cms file intentionally raises a KeyError. We need
+        # The cms file intentionally raises a RuntimeError. We need
         # to make sure the exception definitely bubbles up and doesn't
         # get caught.
-        with self.assertRaises(KeyError):
+        with self.assertRaises(RuntimeError):
             app_registration.autodiscover_cms_configs()
 
     @override_settings(INSTALLED_APPS=[
@@ -57,6 +57,13 @@ def test_raises_exception_when_more_than_one_cms_app_class_found_in_cms_file(sel
 
 class GetCmsAppsTestCase(CMSTestCase):
 
+    def setUp(self):
+        # The result of get_cms_apps is cached. Clear this cache
+        # because installed apps change between tests and therefore
+        # unlike in a live environment, results of this function
+        # can change between tests
+        app_registration.get_cms_apps.cache_clear()
+
     @patch.object(apps, 'get_app_configs')
     def test_returns_only_cms_apps(self, mocked_apps):
         # apps with cms_app attr and a configure method
@@ -216,6 +223,13 @@ def test_runs_configure_app_method_for_correct_apps_when_multiple_apps(
 
 class SetupCmsAppsTestCase(CMSTestCase):
 
+    def setUp(self):
+        # The result of get_cms_apps is cached. Clear this cache
+        # because installed apps change between tests and therefore
+        # unlike in a live environment, results of this function
+        # can change between tests
+        app_registration.get_cms_apps.cache_clear()
+
     @patch.object(setup, 'setup_cms_apps')
     def test_setup_cms_apps_function_run_on_startup(self, mocked_setup):
         cms_module = import_module('cms')

diff --git a/cms/utils/setup.py b/cms/utils/setup.py
@@ -3,8 +3,10 @@
 
 from cms.utils.compat.dj import is_installed as app_is_installed
 from cms.app_registration import (
-    autodiscover_cms_configs, get_cms_apps,
-    configure_cms_apps)
+    autodiscover_cms_configs,
+    configure_cms_apps,
+    get_cms_apps,
+)
 
 
 def validate_dependencies():