django-cms · czpython · Jun 26, 2018 · Jun 14, 2018 · Jun 14, 2018 · Jun 15, 2018
diff --git a/cms/app_registration.py b/cms/app_registration.py
@@ -1,27 +1,96 @@
+import inspect
 from importlib import import_module
 
 from django.apps import apps
 from django.utils.module_loading import module_has_submodule
+from django.core.exceptions import ImproperlyConfigured
 
 
 CMS_CONFIG_NAME = 'cms_apps'
 
 
-def autodiscover_cms_files():
-    """Find and import all cms_apps.py modules"""
+def autodiscover_cms_configs():
+    """
+    Find and import all cms_apps.py files. Add a cms_app attribute
+    to django's app config with an instance of the cms config.
+    """
     for app_config in apps.get_app_configs():
         try:
-            import_module('%s.%s' % (app_config.name, CMS_CONFIG_NAME))
-        except Exception:
+            cms_module = import_module(
+                '%s.%s' % (app_config.name, CMS_CONFIG_NAME))
+        except:
+            # If something in cms_apps.py raises an exception let that
+            # exception bubble up. Only catch the exception if
+            # cms_apps.py doesn't exist
             if module_has_submodule(app_config.module, CMS_CONFIG_NAME):
                 raise
+        else:
+            cms_app_classes = []
+            # Find all classes that inherit from CMSAppConfig
+            for name, obj in inspect.getmembers(cms_module):
+                is_cms_app_config = (
+                    inspect.isclass(obj) and
+                    issubclass(obj, CMSAppConfig) and
+                    # Ignore the import of CMSAppConfig itself
+                    obj != CMSAppConfig
+                )
+                if is_cms_app_config:
+                    cms_app_classes.append(obj)
 
+            if len(cms_app_classes) == 1:
+                # We are adding this attribute here rather than in
+                # django's app config definition because there are
+                # all kinds of limitations as to what can be imported
+                # in django's apps.py and this could cause issues
+                app_config.cms_app = cms_app_classes[0]()
+            else:
+                raise ImproperlyConfigured(
+                    "cms_apps.py files must define exactly one "
+                    "class which inherits from CMSAppConfig")
 
-class CMSAppConfig():
-    # Temporary stub
-    pass
 
+def get_cms_apps_with_features():
+    """
+    Returns app configs of apps with cms features
+    """
+    apps_with_features = []
+
+    for app_config in apps.get_app_configs():
+        # The cms_app attr is added by the autodiscover_cms_configs
+        # function if a cms_apps.py file with a suitable class is found.
+        is_cms_app = hasattr(app_config, 'cms_app')
+        if is_cms_app:
+            # The configure_app method is only present on the cms
+            # app class if the app provides a cms feature. For classes
+            # that only have config this method will not be present.
+            has_cms_extension = hasattr(
+                app_config.cms_app, 'configure_app')
+        else:
+            has_cms_extension = False
+        if is_cms_app and has_cms_extension:
+            apps_with_features.append(app_config)
+
+    return apps_with_features
 
-class CMSAppExtension():
-    # Temporary stub
+
+def configure_cms_apps(apps_with_features):
+    """
+    Check installed apps for apps that are configured to use cms addons
+    and run code to register them with their config
+    """
+    for app_with_feature in apps_with_features:
+        enabled_property = "{app_name}_enabled".format(
+            app_name=app_with_feature.name)
+        configure_app = app_with_feature.cms_app.configure_app
+
+        for app_config in apps.get_app_configs():
+            if not hasattr(app_config, 'cms_app'):
+                # Not a cms app, so ignore
+                continue
+            if getattr(app_config.cms_app, enabled_property, False):
+                # Feature enabled for this app so configure
+                configure_app(app_config)
+
+
+class CMSAppConfig():
     pass
diff --git a/cms/tests/test_app_registration.py b/cms/tests/test_app_registration.py
@@ -1,50 +1,50 @@
 import sys
+from mock import patch, Mock
 
 from django.test import override_settings
+from django.apps import apps, AppConfig
+from django.core.exceptions import ImproperlyConfigured
 
+from cms import app_registration
 from cms.test_utils.testcases import CMSTestCase
-from cms.app_registration import autodiscover_cms_files
 
 
 class AutodiscoverTestCase(CMSTestCase):
 
-    def setUp(self):
+    def _clear_autodiscover_imports(self):
+        """Helper method to clear imports"""
         sys.path_importer_cache.clear()
 
         sys.modules.pop('cms.tests.test_app_registry.app_without_cms_file', None)
         sys.modules.pop('cms.tests.test_app_registry', None)
-        sys.modules.pop('cms.tests.test_app_registry.app_with_cms_file.models', None)
+        sys.modules.pop('cms.tests.test_app_registry.app_with_cms_extension.models', None)
         sys.modules.pop('cms.tests.test_app_registry.app_without_cms_file.models', None)
-        sys.modules.pop('cms.tests.test_app_registry.app_with_cms_file', None)
-        sys.modules.pop('cms.tests.test_app_registry.app_with_cms_file.cms_apps', None)
+        sys.modules.pop('cms.tests.test_app_registry.app_with_cms_extension', None)
+        sys.modules.pop('cms.tests.test_app_registry.app_with_cms_extension.cms_apps', None)
 
-    def tearDown(self):
-        sys.path_importer_cache.clear()
+    def setUp(self):
+        self._clear_autodiscover_imports()
 
-        sys.modules.pop('cms.tests.test_app_registry.app_without_cms_file', None)
-        sys.modules.pop('cms.tests.test_app_registry', None)
-        sys.modules.pop('cms.tests.test_app_registry.app_with_cms_file.models', None)
-        sys.modules.pop('cms.tests.test_app_registry.app_without_cms_file.models', None)
-        sys.modules.pop('cms.tests.test_app_registry.app_with_cms_file', None)
-        sys.modules.pop('cms.tests.test_app_registry.app_with_cms_file.cms_apps', None)
+    def tearDown(self):
+        self._clear_autodiscover_imports()
 
     @override_settings(INSTALLED_APPS=[
-        'cms.tests.test_app_registry.app_with_cms_file',
+        'cms.tests.test_app_registry.app_with_cms_extension',
         'cms.tests.test_app_registry.app_without_cms_file'
     ])
     def test_imports_cms_apps_files(self):
-        autodiscover_cms_files()
+        app_registration.autodiscover_cms_configs()
 
         loaded = set([
             str(module) for module in sys.modules
             if 'cms.tests.test_app_registry' in module])
         expected = set([
             'cms.tests.test_app_registry.app_without_cms_file',
             'cms.tests.test_app_registry',
-            'cms.tests.test_app_registry.app_with_cms_file.models',
+            'cms.tests.test_app_registry.app_with_cms_extension.models',
             'cms.tests.test_app_registry.app_without_cms_file.models',
-            'cms.tests.test_app_registry.app_with_cms_file',
-            'cms.tests.test_app_registry.app_with_cms_file.cms_apps'
+            'cms.tests.test_app_registry.app_with_cms_extension',
+            'cms.tests.test_app_registry.app_with_cms_extension.cms_apps'
         ])
         self.assertSetEqual(loaded, expected)
 
@@ -53,4 +53,186 @@ def test_imports_cms_apps_files(self):
     ])
     def test_raises_exception_raised_in_cms_file(self):
         with self.assertRaises(KeyError):
-            autodiscover_cms_files()
+            app_registration.autodiscover_cms_configs()
+
+    @override_settings(INSTALLED_APPS=[
+        'cms.tests.test_app_registry.app_with_cms_extension',
+        'cms.tests.test_app_registry.app_without_cms_file'
+    ])
+    def test_adds_cms_app_attribute_to_django_app_config(self):
+        app_registration.autodiscover_cms_configs()
+
+        app_list = [app for app in apps.get_app_configs()]
+        self.assertTrue(hasattr(app_list[0], 'cms_app'))
+        self.assertEqual(
+            app_list[0].cms_app.__class__.__name__, 'CMSSomeFeatureConfig')
+        self.assertFalse(hasattr(app_list[1], 'cms_app'))
+
+    @override_settings(INSTALLED_APPS=[
+        'cms.tests.test_app_registry.app_without_cms_app_class',
+    ])
+    def test_raises_exception_when_no_cms_app_class_found_in_cms_file(self):
+        with self.assertRaises(ImproperlyConfigured):
+            app_registration.autodiscover_cms_configs()
+
+    @override_settings(INSTALLED_APPS=[
+        'cms.tests.test_app_registry.app_with_two_cms_app_classes',
+    ])
+    def test_raises_exception_when_more_than_one_cms_app_class_found_in_cms_file(self):
+        with self.assertRaises(ImproperlyConfigured):
+            app_registration.autodiscover_cms_configs()
+
+
+class GetCmsAppsWithFeaturesTestCase(CMSTestCase):
+
+    @patch.object(apps, 'get_app_configs')
+    def test_returns_only_apps_with_features(self, mocked_apps):
+        # apps with cms_app attr and a configure method
+        app_with_cms_ext1 = Mock(cms_app=Mock(spec=['configure_app']))
+        app_with_cms_ext2 = Mock(cms_app=Mock(spec=['configure_app']))
+        # app with cms_app attr without a configure method
+        # this throws an AttributeError if configure_app is accessed
+        app_with_cms_config = Mock(cms_app=Mock(spec=[]))
+        # app without cms_app attr
+        # this throws an AttributeError if cms_app is accessed
+        non_cms_app = Mock(spec=[])
+        # mock what apps have been installed
+        mocked_apps.return_value = [
+            app_with_cms_ext1,
+            app_with_cms_config,
+            app_with_cms_ext2,
+            non_cms_app,
+        ]
+
+        apps_with_features = app_registration.get_cms_apps_with_features()
+
+        self.assertListEqual(
+            apps_with_features,
+            [app_with_cms_ext1, app_with_cms_ext2])
+
+
+class ConfigureCmsAppsTestCase(CMSTestCase):
+
+    @patch.object(apps, 'get_app_configs')
+    def test_runs_configure_app_method_for_app_with_enabled_config(
+        self, mocked_apps
+    ):
+        # Set up app with name djangocms_feature_x that has a cms feature
+        feature_app = Mock(spec=AppConfig)
+        feature_app.name = 'djangocms_feature_x'
+        feature_app.cms_app = Mock(spec=['configure_app'])
+        # Set up app that makes use of djangocms_feature_x
+        config_app_enabled = Mock(spec=AppConfig)
+        config_app_enabled.cms_app = Mock(
+            spec=['djangocms_feature_x_enabled'])
+        config_app_enabled.cms_app.djangocms_feature_x_enabled = True
+        # Pretend these mocked apps are in INSTALLED_APPS
+        mocked_apps.return_value = [
+            feature_app, config_app_enabled]
+
+        app_registration.configure_cms_apps([feature_app])
+
+        feature_app.cms_app.configure_app.assert_called_once_with(
+            config_app_enabled)
+
+    @patch.object(apps, 'get_app_configs')
+    def test_doesnt_run_configure_app_method_for_disabled_app(
+        self, mocked_apps
+    ):
+        # Set up app with name djangocms_feature_x that has a cms feature
+        feature_app = Mock(spec=AppConfig)
+        feature_app.name = 'djangocms_feature_x'
+        feature_app.cms_app = Mock(spec=['configure_app'])
+        # Set up two apps that do not make use of djangocms_feature_x.
+        # One does not define the enabled attr at all (most common
+        # use case) and one defines it as False
+        config_app_disabled1 = Mock(spec=AppConfig)
+        config_app_disabled1.cms_app = Mock(spec=[])
+        config_app_disabled2 = Mock(spec=AppConfig)
+        config_app_disabled2.cms_app = Mock(
+            spec=['djangocms_feature_x_enabled'])
+        config_app_disabled2.cms_app.djangocms_feature_x_enabled = False
+        # Pretend all these mocked apps are in INSTALLED_APPS
+        mocked_apps.return_value = [
+            feature_app, config_app_disabled1, config_app_disabled2]
+
+        app_registration.configure_cms_apps([feature_app])
+
+        self.assertFalse(feature_app.cms_app.configure_app.called)
+
+    @patch.object(apps, 'get_app_configs')
+    def test_doesnt_raise_exception_if_not_cms_app(
+        self, mocked_apps
+    ):
+        # Set up app with name djangocms_feature_x that has a cms feature
+        feature_app = Mock(spec=AppConfig)
+        feature_app.name = 'djangocms_feature_x'
+        feature_app.cms_app = Mock(spec=['configure_app'])
+        # Set up non cms app
+        non_cms_app = Mock(spec=AppConfig)
+        # Pretend these mocked apps are in INSTALLED_APPS
+        mocked_apps.return_value = [feature_app, non_cms_app]
+
+        try:
+            app_registration.configure_cms_apps([feature_app])
+        except AttributeError:
+            self.fail("Exception raised when cms app config not defined")
+
+    @patch.object(apps, 'get_app_configs')
+    def test_runs_configure_app_method_for_correct_apps_when_multiple_apps(
+        self, mocked_apps
+    ):
+        # Set up app with name djangocms_feature_x that has a cms feature
+        feature_app_x = Mock(spec=AppConfig)
+        feature_app_x.name = 'djangocms_feature_x'
+        feature_app_x.cms_app = Mock(spec=['configure_app'])
+        # Set up app with name djangocms_feature_y that has a cms feature
+        feature_app_y = Mock(spec=AppConfig)
+        feature_app_y.name = 'djangocms_feature_y'
+        feature_app_y.cms_app = Mock(spec=['configure_app'])
+        # Set up apps that makes use of djangocms_feature_x
        config_app_x = Mock(spec=AppConfig)
+        config_app_x.cms_app = Mock(
+            spec=['djangocms_feature_x_enabled'])
+        config_app_x.cms_app.djangocms_feature_x_enabled = True
+        # Set up app that makes use of djangocms_feature_y
+        config_app_y = Mock(spec=AppConfig)
+        config_app_y.cms_app = Mock(
+            spec=['djangocms_feature_y_enabled'])
+        conf
F438
ig_app_y.cms_app.djangocms_feature_y_enabled = True
+        # Set up app that makes use of feature x & y
+        config_app_xy = Mock(spec=AppConfig)
+        config_app_xy.cms_app = Mock(
+            spec=['djangocms_feature_x_enabled',
+                  'djangocms_feature_y_enabled']
+        )
+        config_app_xy.cms_app.djangocms_feature_x_enabled = True
+        config_app_xy.cms_app.djangocms_feature_y_enabled = True
+        # Set up non cms app
+        non_cms_app = Mock(spec=AppConfig)
+        # Pretend these mocked apps are in INSTALLED_APPS
+        mocked_apps.return_value = [
+            feature_app_x, non_cms_app, config_app_xy, config_app_y,
+            config_app_x, feature_app_y]
+
+        app_registration.configure_cms_apps(
+            [feature_app_x, feature_app_y])
+
+        # Assert we configured the 2 apps we expected with feature x
+        self.assertEqual(
+            feature_app_x.cms_app.configure_app.call_count, 2)
+        self.assertEqual(
+            feature_app_x.cms_app.configure_app.call_args_list[0][0][0],
+            config_app_xy)
+        self.assertEqual(
+            feature_app_x.cms_app.configure_app.call_args_list[1][0][0],
+            config_app_x)
+        # Assert we configured the 2 apps we expected with feature y
+        self.assertEqual(
+            feature_app_y.cms_app.configure_app.call_count, 2)
+        self.assertEqual(
+            feature_app_y.cms_app.configure_app.call_args_list[0][0][0],
+            config_app_xy)
+        self.assertEqual(
+            feature_app_y.cms_app.configure_app.call_args_list[1][0][0],
+            config_app_y)
diff --git a/...pp_registry/app_with_cms_file/__init__.py → ...gistry/app_with_cms_extension/__init__.py b/...pp_registry/app_with_cms_file/__init__.py → ...gistry/app_with_cms_extension/__init__.py
diff --git a/...t_app_registry/app_with_cms_file/admin.py → ..._registry/app_with_cms_extension/admin.py b/...t_app_registry/app_with_cms_file/admin.py → ..._registry/app_with_cms_extension/admin.py
diff --git a/cms/tests/test_app_registry/app_with_cms_extension/cms_apps.py b/cms/tests/test_app_registry/app_with_cms_extension/cms_apps.py
@@ -0,0 +1,26 @@
+from cms.app_registration import CMSAppConfig
+
+
+def some_function(*args, **kwargs):
+    # this should not be called by autodiscover
+    raise Exception('some_function called')
+
+
+class SomeClass():
+    # this should not be picked up by autodiscover
+
+    def __init__(self, *args, **kwargs):
+        raise Exception('SomeClass instantiated')
+
+
+class CMSSomeFeatureConfig(CMSAppConfig):
+
+    def configure_app(self, app):
+        pass
+
+
+class SomeOtherClass():
+    # this should not be picked up by autodiscover
+
+    def __init__(self, *args, **kwargs):
+        raise Exception('SomeOtherClass instantiated')
diff --git a/..._app_registry/app_with_cms_file/models.py → ...registry/app_with_cms_extension/models.py b/..._app_registry/app_with_cms_file/models.py → ...registry/app_with_cms_extension/models.py
diff --git a/...t_app_registry/app_with_cms_file/views.py → ..._registry/app_with_cms_extension/views.py b/...t_app_registry/app_with_cms_file/views.py → ..._registry/app_with_cms_extension/views.py
diff --git a/cms/tests/test_app_registry/app_with_two_cms_app_classes/__init__.py b/cms/tests/test_app_registry/app_with_two_cms_app_classes/__init__.py
diff --git a/cms/tests/test_app_registry/app_with_two_cms_app_classes/admin.py b/cms/tests/test_app_registry/app_with_two_cms_app_classes/admin.py