Merge 12fb1ea into 8410a0e

app_enabler/enable.py

@@ -19,15 +19,29 @@ def _verify_settings(imported: ModuleType, application_config: Dict[str, Any]) -
     :param ModuleType imported:  Update settings module
     :param dict application_config: addon configuration
     """
-    test_passed = True
-    for app in application_config.get("installed-apps", []):
-        test_passed = test_passed and app in imported.INSTALLED_APPS
-    for key, value in application_config.get("settings", {}).items():
+
+    def _validate_setting(key: str, value: Any):
+        """
+        Validate the given value for a single setting.
+
+        It's aware of the possible structures of the application config setting (either a literal or a dict with the
+        precedence information).
+        """
+        passed = True
         if isinstance(value, list):
             for item in value:
-                test_passed = test_passed and item in getattr(imported, key)
+                if isinstance(item, dict):
+                    real_item = item["value"]
+                    passed = passed and (real_item in getattr(imported, key))
+                else:
+                    passed = passed and (item in getattr(imported, key))
         else:
-            test_passed = test_passed and getattr(imported, key) == value
+            passed = passed and getattr(imported, key) == value
+        return passed
+
+    test_passed = _validate_setting("INSTALLED_APPS", application_config.get("installed-apps", []))
+    for setting_name, setting_value in application_config.get("settings", {}).items():
+        test_passed = test_passed and _validate_setting(setting_name, setting_value)
     return test_passed
 
 

app_enabler/patcher.py

@@ -2,7 +2,7 @@
 import os  # noqa - used when eval'ing the management command
 import sys
 from types import CodeType
-from typing import Any, Dict
+from typing import Any, Dict, Iterable, List, Union
 
 import astor
 
@@ -58,7 +58,6 @@ class DisableExecute(ast.NodeTransformer):
 
     def visit_Expr(self, node: ast.AST) -> Any:  # noqa
         """Visit the ``Expr`` node and remove it if it matches ``'execute_from_command_line'``."""
-        # long chained checks, but we have to remove the entire call, thus we have to remove the Expr node
         if (
             isinstance(node.value, ast.Call)
             and isinstance(node.value.func, ast.Name)  # noqa
@@ -69,6 +68,59 @@ def visit_Expr(self, node: ast.AST) -> Any:  # noqa
             return node
 
 
+def _update_list_setting(original_setting: List, configuration: Iterable):
+    def _ast_dict_lookup(dict_object: ast.Dict, lookup_key: str) -> Any:
+        """Get the value of the lookup key in the ast Dict object."""
+        key_position = [_ast_get_constant_value(dict_key) for dict_key in dict_object.keys].index(lookup_key)
+        return _ast_get_constant_value(dict_object.values[key_position])
+
+    def _ast_get_constant_value(ast_obj: Union[ast.Constant, ast.Str, ast.Num]) -> Any:
+        """
+        Extract the value from an ast.Constant / ast.Str / ast.Num obj.
+
+        Required as in python 3.6 / 3.7 ast.Str / ast.Num are not subclasses of ast.Constant
+        """
+        try:
+            return ast_obj.value
+        except AttributeError:
+            return ast_obj.s
+
+    def _ast_get_object_from_value(val: Any) -> ast.Constant:
+        """Convert value to AST via :py:func:`ast.parse`."""
+        return ast.parse(repr(val)).body[0].value
+
+    for config_value in configuration:
+        # configuration items can be either strings (which are appended) or dictionaries which contains information
+        # about the position of the item
+        if isinstance(config_value, str):
+            if config_value not in [_ast_get_constant_value(item) for item in original_setting]:
+                original_setting.append(ast.Str(config_value))
+        elif isinstance(config_value, dict):
+            value = config_value.get("value", None)
+            position = config_value.get("position", None)
+            relative_item = config_value.get("next", None)
+            key = config_value.get("key", None)
+            if relative_item:
+                # if the item is already existing, we skip its insertion
+                position = None
+                if key:
+                    # if the match is against a key we must both flatted the original setting to a list of literals
+                    # extracting the key value and getting the key value for the setting we want to add
+                    flattened_data = [_ast_dict_lookup(item, key) for item in original_setting]
+                    check_value = value[key]
+                else:
+                    flattened_data = [_ast_get_constant_value(item) for item in original_setting]
+                    check_value = value
+                if check_value not in flattened_data:
+                    try:
+                        position = flattened_data.index(relative_item)
+                    except ValueError:
+                        # in case the relative item is not found we add the value on top
+                        position = 0
+            if position is not None:
+                original_setting.insert(position, _ast_get_object_from_value(value))
+
+
 def update_setting(project_setting: str, config: Dict[str, Any]):
     """
     Patch the settings module to include addon settings.
@@ -84,16 +136,16 @@ def update_setting(project_setting: str, config: Dict[str, Any]):
     addon_installed_apps = config.get("installed-apps", [])
     for node in parsed.body:
         if isinstance(node, ast.Assign) and node.targets[0].id == "INSTALLED_APPS":
-            installed_apps = [name.s for name in node.value.elts]
-            addon_apps = [ast.Constant(app) for app in addon_installed_apps if app not in installed_apps]
-            node.value.elts.extend(addon_apps)
+            _update_list_setting(node.value.elts, addon_installed_apps)
         elif isinstance(node, ast.Assign) and node.targets[0].id in addon_settings.keys():  # noqa
             config_param = addon_settings[node.targets[0].id]
             if isinstance(node.value, ast.List) and (
                 isinstance(config_param, list) or isinstance(config_param, tuple)
             ):
-                for config_value in config_param:
-                    node.value.elts.append(ast.Str(config_value))
+                _update_list_setting(node.value.elts, config_param)
+            elif type(node.value) in (ast.Constant, ast.Str, ast.Num):
+                # check required as in python 3.6 / 3.7 ast.Str / ast.Num are not subclasses of ast.Constant
+                node.value = ast.Constant(config_param)
             existing_setting.append(node.targets[0].id)
     for name, value in addon_settings.items():
         if name not in existing_setting:

changes/5.feature

@@ -0,0 +1 @@
+Improve merge strategy to support all the basic standard Django settings

docs/addon_configuration.rst

@@ -44,7 +44,7 @@ The following attributes are currently supported:
 * ``package-name`` [**required**]: package name as available on PyPi;
 * ``installed-apps`` [**required**]: list of django applications to be appended in the project ``INSTALLED_APPS``
   setting. Application must be already installed when the configuration is processed, thus they must declared as
-  package dependencies (or depedencies of direct dependencies, even if this is a bit risky);
+  package dependencies (or dependencies of direct dependencies, even if this is a bit risky);
 * ``urls`` [optional]: list of urlconfs to be added to the project ``ROOT_URLCONF``. List can be empty if no url
   configuration is needed or it can be omitted.
 
@@ -54,8 +54,40 @@ The following attributes are currently supported:
     (to add the urlconf to the root)
   * ``<include-dotted-path>`` must be a valid input for :py:func:`Django include() function <django:django.urls.include>`;
 * ``settings`` [optional]: A dictionary of custom settings that will be added to project settings verbatim;
-* ``message`` [optional]: A text message output after succesfull completion of the configuration;
+* ``message`` [optional]: A text message output after successful completion of the configuration;
 
+Attribute format
+----------------
+
+``installed-apps`` and ``settings`` values can have the following formats:
+
+- literal (``string``, ``int``, ``boolean``): value is applied as is
+- ``dict`` with the following structure:
+
+  - ``value: Any`` (required), the setting value
+  - ``position: int``, if set and the target setting is a list, ``value`` is inserted at position
+  - ``next: str``, name of an existing item before which the ``value`` is going to be inserted
+  - ``key: str``, in case ``value`` is a dictionary, the dictionary key to be used to match existing settings value for duplicates and to match the ``next`` value
+
+
+Merge strategy
+==============
+
+``settings`` items not existing in the target project settings are applied without further changes, so you can use whatever structure is needed.
+
+``settings`` which already exists in the project and ``installed-apps`` configuration are merged with the ones already existing according to this strategy:
+
+- setting does not exist -> custom setting is added verbatim
+- setting exists and its value is a literal -> target project setting is overridden
+- setting exists and its value is a list -> custom setting is merged:
+
+  - if the custom setting is a literal -> its value is appended to the setting list
+  - if it's a dictionary (see format above) ->
+
+    - if ``next`` is defined, a value matching the ``next`` value is searched in the project setting and the custom setting ``value`` is inserted before the ``next`` element or at the top of the list if the value is not found; in case ``value`` (and items in the project settings) are dictionaries (like for example ``AUTH_PASSWORD_VALIDATORS``), a ``key`` attribute must be provided as a lookup key;
+    - if ``position`` is defined, the custom setting value is inserted at that position;
+
+In any case, if a value is already present, is not duplicated and is simply ignored.
 
 Sample file
 ===========
@@ -77,7 +109,24 @@ Sample file
         ],
         "settings": {
             "META_SITE_PROTOCOL": "https",
-            "META_USE_SITES": true
+            "META_USE_SITES": true,
+            "MIDDLEWARE": [
+                "django.middleware.gzip.GZipMiddleware",
+                {"value": "django.middleware.http.ConditionalGetMiddleware", "position": 2},
+                {
+                    "value": "django.middleware.locale.LocaleMiddleware",
+                    "next": "django.middleware.common.CommonMiddleware",
+                },
+            ],
+            "AUTH_PASSWORD_VALIDATORS": [
+                {
+                    "value": {
+                        "NAME": "django.contrib.auth.password_validation.NumericPasswordValidator",
+                    },
+                    "next": "django.contrib.auth.password_validation.UserAttributeSimilarityValidator",
+                    "key": "NAME",
+                },
+            ],
         },
         "urls": [
             ["", "djangocms_blog.taggit_urls"]

docs/limitations.rst

@@ -14,8 +14,7 @@ settings.py
 * Only single file ``settings.py`` are currently supported.
   In case you are using splitted settings, the only way to use ``django-app-enabler`` is to have at least an empty
   ``INSTALLED_APPS`` list in the settings file declared in ``DJANGO_SETTINGS_MODULE``.
-* Only ``INSTALLED_APP`` setting is supported as a standard Django setting to be patched. Others like ``MIDDLEWARE``,
-  ``TEMPLATES`` etc are not supported yet. Any custom setting is supported, though.
+* Only the most common setting attributes are supported (with the notable exception of ``TEMPLATES``). Any custom setting is supported.
 * While extra requirements will be installed when including them in the package argument (as in ``djangocms-blog[search]``),
   they will not be added to ``INSTALLED_APPS`` and they must be added manually after command execution.
 

docs/todo.rst

@@ -4,15 +4,13 @@
 Planned features
 ################
 
-* More standard django settings than INSTALLED_APPS `issue-5`_
 * Support extra-requirements `issue-6`_
 * Support Django settings split in multiple files `issue-7`_
 * Support Django urlconf split in multiple files `issue-8`_
 
 
 
 
-.. _issue-5: https://github.com/nephila/django-app-enabler/issues/5
 .. _issue-6: https://github.com/nephila/django-app-enabler/issues/6
 .. _issue-7: https://github.com/nephila/django-app-enabler/issues/7
 .. _issue-8: https://github.com/nephila/django-app-enabler/issues/8

tests/conftest.py

@@ -60,18 +60,45 @@ def addon_config() -> Dict[str, Any]:
         "installed-apps": [
             "filer",
             "easy_thumbnails",
-            "aldryn_apphooks_config",
+            {"value": "aldryn_apphooks_config", "next": "cms"},
             "parler",
             "taggit",
-            "taggit_autosuggest",
+            {
+                "value": "taggit_autosuggest",
+                "next": "taggit",
+            },
             "meta",
             "djangocms_blog",
             "sortedm2m",
         ],
         "settings": {
             "META_SITE_PROTOCOL": "https",
             "META_USE_SITES": True,
-            "MIDDLEWARE": ["django.middleware.gzip.GZipMiddleware"],
+            "LANGUAGE_CODE": "it-IT",
+            "MIDDLEWARE": [
+                "django.middleware.gzip.GZipMiddleware",
+                {"value": "django.middleware.http.ConditionalGetMiddleware", "position": 2},
+                {
+                    "value": "django.middleware.locale.LocaleMiddleware",
+                    "next": "django.middleware.common.CommonMiddleware",
+                },
+            ],
+            "AUTH_PASSWORD_VALIDATORS": [
+                {
+                    "value": {
+                        "NAME": "django.contrib.auth.password_validation.NumericPasswordValidator",
+                    },
+                    "next": "django.contrib.auth.password_validation.UserAttributeSimilarityValidator",
+                    "key": "NAME",
+                },
+                {
+                    "value": {
+                        "NAME": "django.contrib.auth.password_validation.CommonPasswordValidator",
+                    },
+                    "next": "django.contrib.auth.password_validation.UserAttributeSimilarityValidator",
+                    "key": "NAME",
+                },
+            ],
         },
         "urls": [["", "djangocms_blog.taggit_urls"]],
         "message": "Please check documentation to complete the setup",

tests/sample/test_project/settings.py

@@ -94,9 +94,6 @@
     {
         "NAME": "django.contrib.auth.password_validation.CommonPasswordValidator",
     },
-    {
-        "NAME": "django.contrib.auth.password_validation.NumericPasswordValidator",
-    },
 ]
 
 

tests/test_patcher.py

@@ -50,6 +50,16 @@ def test_update_setting(pytester, project_dir, addon_config):
     sys.path.insert(0, str(settings_file.parent))
     imported = import_module("settings")
     assert _verify_settings(imported, addon_config)
+    assert imported.MIDDLEWARE.index("django.middleware.common.CommonMiddleware") > imported.MIDDLEWARE.index(
+        "django.middleware.locale.LocaleMiddleware"
+    )
+    assert imported.MIDDLEWARE.index("django.middleware.http.ConditionalGetMiddleware") == 2
+    assert (
+        imported.AUTH_PASSWORD_VALIDATORS[0]["NAME"]
+        == "django.contrib.auth.password_validation.NumericPasswordValidator"
+    )
+    assert imported.INSTALLED_APPS.index("taggit") == imported.INSTALLED_APPS.index("taggit_autosuggest") + 1
+    assert imported.INSTALLED_APPS.index("aldryn_apphooks_config") == 0
 
 
 def test_update_urlconf(pytester, django_setup, project_dir, addon_config):