Merge pull request #1526 from YunoHost/enh-apps-v2

Enh apps v2

conf/nginx/yunohost_admin.conf.inc

@@ -19,6 +19,10 @@ location /yunohost/admin/ {
         more_set_headers "Cache-Control: no-store, no-cache, must-revalidate";
     }
 
+    location /yunohost/admin/applogos/ {
+        alias /usr/share/yunohost/applogos/;
+    }
+
     more_set_headers "Content-Security-Policy: upgrade-insecure-requests; default-src 'self'; connect-src 'self' https://paste.yunohost.org wss://$host; style-src 'self' 'unsafe-inline'; script-src 'self' 'unsafe-eval'; object-src 'none'; img-src 'self' data:;";
     more_set_headers "Content-Security-Policy-Report-Only:";
 }

helpers/config

@@ -22,7 +22,7 @@ _ynh_app_config_get_one() {
         if [[ "$bind" == "settings" ]]; then
             ynh_die --message="File '${short_setting}' can't be stored in settings"
         fi
-        old[$short_setting]="$(ls "$(echo $bind | sed s@__FINALPATH__@$final_path@ | sed s/__APP__/$app/)" 2>/dev/null || echo YNH_NULL)"
+        old[$short_setting]="$(ls "$(echo $bind | sed s@__INSTALL_DIR__@$install_dir@ | sed s@__FINALPATH__@$final_path@ | sed s/__APP__/$app/)" 2>/dev/null || echo YNH_NULL)"
         file_hash[$short_setting]="true"
 
     # Get multiline text from settings or from a full file
@@ -32,7 +32,7 @@ _ynh_app_config_get_one() {
         elif [[ "$bind" == *":"* ]]; then
             ynh_die --message="For technical reasons, multiline text '${short_setting}' can't be stored automatically  in a variable file, you have to create custom getter/setter"
         else
-            old[$short_setting]="$(cat $(echo $bind | sed s@__FINALPATH__@$final_path@ | sed s/__APP__/$app/) 2>/dev/null || echo YNH_NULL)"
+            old[$short_setting]="$(cat $(echo $bind | sed s@__INSTALL_DIR__@$install_dir@ | sed s@__FINALPATH__@$final_path@ | sed s/__APP__/$app/) 2>/dev/null || echo YNH_NULL)"
         fi
 
     # Get value from a kind of key/value file
@@ -47,7 +47,7 @@ _ynh_app_config_get_one() {
             bind_after="$(echo "${bind_key_}" | cut -d'>' -f1)"
             bind_key_="$(echo "${bind_key_}" | cut -d'>' -f2)"
         fi
-        local bind_file="$(echo "$bind" | cut -d: -f2 | sed s@__FINALPATH__@$final_path@ | sed s/__APP__/$app/)"
+        local bind_file="$(echo "$bind" | cut -d: -f2 | sed s@__INSTALL_DIR__@$install_dir@ | sed s@__FINALPATH__@$final_path@ | sed s/__APP__/$app/)"
         old[$short_setting]="$(ynh_read_var_in_file --file="${bind_file}" --key="${bind_key_}" --after="${bind_after}")"
 
     fi
@@ -73,7 +73,7 @@ _ynh_app_config_apply_one() {
             if [[ "$bind" == "settings" ]]; then
                 ynh_die --message="File '${short_setting}' can't be stored in settings"
             fi
-            local bind_file="$(echo "$bind" | sed s@__FINALPATH__@$final_path@ | sed s/__APP__/$app/)"
+            local bind_file="$(echo "$bind" | sed s@__INSTALL_DIR__@$install_dir@ | sed s@__FINALPATH__@$final_path@ | sed s/__APP__/$app/)"
             if [[ "${!short_setting}" == "" ]]; then
                 ynh_backup_if_checksum_is_different --file="$bind_file"
                 ynh_secure_remove --file="$bind_file"
@@ -98,7 +98,7 @@ _ynh_app_config_apply_one() {
             if [[ "$bind" == *":"* ]]; then
                 ynh_die --message="For technical reasons, multiline text '${short_setting}' can't be stored automatically  in a variable file, you have to create custom getter/setter"
             fi
-            local bind_file="$(echo "$bind" | sed s@__FINALPATH__@$final_path@ | sed s/__APP__/$app/)"
+            local bind_file="$(echo "$bind" | sed s@__INSTALL_DIR__@$install_dir@ | sed s@__FINALPATH__@$final_path@ | sed s/__APP__/$app/)"
             ynh_backup_if_checksum_is_different --file="$bind_file"
             echo "${!short_setting}" >"$bind_file"
             ynh_store_file_checksum --file="$bind_file" --update_only
@@ -113,7 +113,7 @@ _ynh_app_config_apply_one() {
                 bind_after="$(echo "${bind_key_}" | cut -d'>' -f1)"
                 bind_key_="$(echo "${bind_key_}" | cut -d'>' -f2)"
             fi
-            local bind_file="$(echo "$bind" | cut -d: -f2 | sed s@__FINALPATH__@$final_path@ | sed s/__APP__/$app/)"
+            local bind_file="$(echo "$bind" | cut -d: -f2 | sed s@__INSTALL_DIR__@$install_dir@ | sed s@__FINALPATH__@$final_path@ | sed s/__APP__/$app/)"
 
             ynh_backup_if_checksum_is_different --file="$bind_file"
             ynh_write_var_in_file --file="${bind_file}" --key="${bind_key_}" --value="${!short_setting}" --after="${bind_after}"

helpers/php

@@ -474,9 +474,9 @@ YNH_COMPOSER_VERSION=${YNH_COMPOSER_VERSION:-$YNH_DEFAULT_COMPOSER_VERSION}
 
 # Execute a command with Composer
 #
-# usage: ynh_composer_exec [--phpversion=phpversion] [--workdir=$final_path] --commands="commands"
+# usage: ynh_composer_exec [--phpversion=phpversion] [--workdir=$install_dir] --commands="commands"
 # | arg: -v, --phpversion - PHP version to use with composer
-# | arg: -w, --workdir - The directory from where the command will be executed. Default $final_path.
+# | arg: -w, --workdir - The directory from where the command will be executed. Default $install_dir or $final_path
 # | arg: -c, --commands - Commands to execute.
 #
 # Requires YunoHost version 4.2 or higher.
@@ -489,7 +489,7 @@ ynh_composer_exec() {
     local commands
     # Manage arguments with getopts
     ynh_handle_getopts_args "$@"
-    workdir="${workdir:-$final_path}"
+    workdir="${workdir:-${install_dir:-$final_path}}"
     phpversion="${phpversion:-$YNH_PHP_VERSION}"
 
     COMPOSER_HOME="$workdir/.composer" COMPOSER_MEMORY_LIMIT=-1 \

helpers/utils

@@ -192,6 +192,9 @@ ynh_setup_source() {
     # Extract source into the app dir
     mkdir --parents "$dest_dir"
 
+    if [ -n "${install_dir:-}" ] && [ "$dest_dir" == "$install_dir" ]; then
+        _ynh_apply_default_permissions $dest_dir
+    fi
     if [ -n "${final_path:-}" ] && [ "$dest_dir" == "$final_path" ]; then
         _ynh_apply_default_permissions $dest_dir
     fi
@@ -330,7 +333,7 @@ ynh_local_curl() {
 # | arg: -d, --destination=    - Destination of the config file
 #
 # examples:
-#    ynh_add_config --template=".env" --destination="$final_path/.env" use the template file "../conf/.env"
+#    ynh_add_config --template=".env" --destination="$install_dir/.env" use the template file "../conf/.env"
 #    ynh_add_config --template="/etc/nginx/sites-available/default" --destination="etc/nginx/sites-available/mydomain.conf"
 #
 # The template can be by default the name of a file in the conf directory
@@ -444,8 +447,10 @@ ynh_replace_vars() {
         ynh_replace_string --match_string="__NAMETOCHANGE__" --replace_string="$app" --target_file="$file"
         ynh_replace_string --match_string="__USER__" --replace_string="$app" --target_file="$file"
     fi
+    # Legacy
     if test -n "${final_path:-}"; then
         ynh_replace_string --match_string="__FINALPATH__" --replace_string="$final_path" --target_file="$file"
+        ynh_replace_string --match_string="__INSTALL_DIR__" --replace_string="$final_path" --target_file="$file"
     fi
     if test -n "${YNH_PHP_VERSION:-}"; then
         ynh_replace_string --match_string="__PHPVERSION__" --replace_string="$YNH_PHP_VERSION" --target_file="$file"

locales/en.json

@@ -13,6 +13,7 @@
     "app_already_installed": "{app} is already installed",
     "app_already_installed_cant_change_url": "This app is already installed. The URL cannot be changed just by this function. Check in `app changeurl` if it's available.",
     "app_already_up_to_date": "{app} is already up-to-date",
+    "app_arch_not_supported": "This app can only be installed on architectures {', '.join(required)} but your server architecture is {current}",
     "app_argument_choice_invalid": "Pick a valid value for argument '{name}': '{value}' is not among the available choices ({choices})",
     "app_argument_invalid": "Pick a valid value for the argument '{name}': {error}",
     "app_argument_password_no_default": "Error while parsing password argument '{name}': password argument can't have a default value for security reasons",
@@ -26,6 +27,7 @@
     "app_full_domain_unavailable": "Sorry, this app must be installed on a domain of its own, but other apps are already installed on the domain '{domain}'. You could use a subdomain dedicated to this app instead.",
     "app_id_invalid": "Invalid app ID",
     "app_install_failed": "Unable to install {app}: {error}",
+    "app_resource_failed": "Provisioning, deprovisioning, or updating resources for {app} failed: {error}",
     "app_install_files_invalid": "These files cannot be installed",
     "app_install_script_failed": "An error occurred inside the app installation script",
     "app_label_deprecated": "This command is deprecated! Please use the new command 'yunohost user permission update' to manage the app label.",
@@ -39,6 +41,8 @@
     "app_manifest_install_ask_password": "Choose an administration password for this app",
     "app_manifest_install_ask_path": "Choose the URL path (after the domain) where this app should be installed",
     "app_not_correctly_installed": "{app} seems to be incorrectly installed",
+    "app_not_enough_disk": "This app requires {required} free space.",
+    "app_not_enough_ram": "This app requires {required} RAM to install/upgrade but only {current} is available right now.",
     "app_not_installed": "Could not find {app} in the list of installed apps: {all_apps}",
     "app_not_properly_removed": "{app} has not been properly removed",
     "app_not_upgraded": "The app '{failed_app}' failed to upgrade, and as a consequence the following apps' upgrades have been cancelled: {apps}",
@@ -61,6 +65,7 @@
     "app_upgrade_several_apps": "The following apps will be upgraded: {apps}",
     "app_upgrade_some_app_failed": "Some apps could not be upgraded",
     "app_upgraded": "{app} upgraded",
+    "app_yunohost_version_not_supported": "This app requires YunoHost >= {required} but current installed version is {current}",
     "apps_already_up_to_date": "All apps are already up-to-date",
     "apps_catalog_failed_to_download": "Unable to download the {apps_catalog} app catalog: {error}",
     "apps_catalog_init_success": "App catalog system initialized!",
@@ -161,6 +166,8 @@
     "confirm_app_install_danger": "DANGER! This app is known to be still experimental (if not explicitly not working)! You should probably NOT install it unless you know what you are doing. NO SUPPORT will be provided if this app doesn't work or breaks your system... If you are willing to take that risk anyway, type '{answers}'",
     "confirm_app_install_thirdparty": "DANGER! This app is not part of YunoHost's app catalog. Installing third-party apps may compromise the integrity and security of your system. You should probably NOT install it unless you know what you are doing. NO SUPPORT will be provided if this app doesn't work or breaks your system... If you are willing to take that risk anyway, type '{answers}'",
     "confirm_app_install_warning": "Warning: This app may work, but is not well-integrated into YunoHost. Some features such as single sign-on and backup/restore might not be available. Install anyway? [{answers}] ",
+    "confirm_app_insufficient_ram": "DANGER! This app requires {required} RAM to install/upgrade but only {current} is available right now. Even if this app could run, its installation/upgrade process requires a large amount of RAM so your server may freeze and fail miserably. If you are willing to take that risk anyway, type '{answers}'",
+    "confirm_notifications_read": "WARNING: You should check the app notifications above before continuing, there might be important stuff to know. [{answers}]",
     "custom_app_url_required": "You must provide a URL to upgrade your custom app {app}",
     "danger": "Danger:",
     "diagnosis_apps_allgood": "All installed apps respect basic packaging practices",

share/actionsmap.yml

@@ -778,6 +778,10 @@ app:
                     full: --with-categories
                     help: Also return a list of app categories
                     action: store_true
+                -a:
+                    full: --with-antifeatures
+                    help: Also return a list of antifeatures categories
+                    action: store_true
 
         ### app_search()
         search:
@@ -793,6 +797,10 @@ app:
             arguments:
                 app:
                     help: Name, local path or git URL of the app to fetch the manifest of
+                -s:
+                    full: --with-screenshot
+                    help: Also return a base64 screenshot if any (API only)
+                    action: store_true
 
         ### app_list()
         list:
@@ -963,6 +971,17 @@ app:
                     help: Undo redirection
                     action: store_true
 
+        ### app_dismiss_notification
+        dismiss-notification:
+            hide_in_help: True
+            action_help: Dismiss post_install or post_upgrade notification
+            api: PUT /apps/<app>/dismiss_notification/<name>
+            arguments:
+                app:
+                    help: App ID to dismiss notification for
+                name:
+                    help: Notification name, either post_install or post_upgrade
+
         ### app_ssowatconf()
         ssowatconf:
             action_help: Regenerate SSOwat configuration file

src/app.py

@@ -29,7 +29,7 @@ import subprocess
 import tempfile
 import copy
 from collections import OrderedDict
-from typing import List, Tuple, Dict, Any
+from typing import List, Tuple, Dict, Any, Iterator
 from packaging import version
 
 from moulinette import Moulinette, m18n
@@ -71,6 +71,7 @@ from yunohost.app_catalog import (  # noqa
     app_catalog,
     app_search,
     _load_apps_catalog,
+    APPS_CATALOG_LOGOS,
 )
 
 logger = getActionLogger("yunohost.app")
@@ -151,6 +152,13 @@ def app_info(app, full=False, upgradable=False):
     absolute_app_name, _ = _parse_app_instance_name(app)
     from_catalog = _load_apps_catalog()["apps"].get(absolute_app_name, {})
 
+    # Check if $app.png exists in the app logo folder, this is a trick to be able to easily customize the logo
+    # of an app just by creating $app.png (instead of the hash.png) in the corresponding folder
+    ret["logo"] = (
+        app
+        if os.path.exists(f"{APPS_CATALOG_LOGOS}/{app}.png")
+        else from_catalog.get("logo_hash")
+    )
     ret["upgradable"] = _app_upgradable({**ret, "from_catalog": from_catalog})
 
     if ret["upgradable"] == "yes":
@@ -164,6 +172,8 @@ def app_info(app, full=False, upgradable=False):
             ret["current_version"] = f" ({current_revision})"
             ret["new_version"] = f" ({new_revision})"
 
+    ret["settings"] = settings
+
     if not full:
         return ret
 
@@ -175,7 +185,6 @@ def app_info(app, full=False, upgradable=False):
     ret["manifest"]["install"] = _set_default_ask_questions(
         ret["manifest"].get("install", {})
     )
-    ret["settings"] = settings
 
     ret["from_catalog"] = from_catalog
 
@@ -185,6 +194,15 @@ def app_info(app, full=False, upgradable=False):
             ret["manifest"]["doc"][pagename][lang] = _hydrate_app_template(
                 content, settings
             )
+
+    # Filter dismissed notification
+    ret["manifest"]["notifications"] = {
+        k: v
+        for k, v in ret["manifest"]["notifications"].items()
+        if not _notification_is_dismissed(k, settings)
+    }
+
+    # Hydrate notifications (also filter uneeded post_upgrade notification based on version)
     for step, notifications in ret["manifest"]["notifications"].items():
         for name, content_per_lang in notifications.items():
             for lang, content in content_per_lang.items():
@@ -526,6 +544,8 @@ def app_upgrade(app=[], url=None, file=None, force=False, no_safety_backup=False
     if len(apps) > 1:
         logger.info(m18n.n("app_upgrade_several_apps", apps=", ".join(apps)))
 
+    notifications = {}
+
     for number, app_instance_name in enumerate(apps):
         logger.info(m18n.n("app_upgrade_app_name", app=app_instance_name))
 
@@ -587,7 +607,29 @@ def app_upgrade(app=[], url=None, file=None, force=False, no_safety_backup=False
                     upgrade_type = "UPGRADE_FULL"
 
         # Check requirements
-        _check_manifest_requirements(manifest, action="upgrade")
+        for name, passed, values, err in _check_manifest_requirements(
+            manifest, action="upgrade"
+        ):
+            if not passed:
+                if name == "ram":
+                    _ask_confirmation(
+                        "confirm_app_insufficient_ram", params=values, force=force
+                    )
+                else:
+                    raise YunohostValidationError(err, **values)
+
+        # Display pre-upgrade notifications and ask for simple confirm
+        if (
+            manifest["notifications"]["PRE_UPGRADE"]
+            and Moulinette.interface.type == "cli"
+        ):
+            settings = _get_app_settings(app_instance_name)
+            notifications = _filter_and_hydrate_notifications(
+                manifest["notifications"]["PRE_UPGRADE"],
+                current_version=app_current_version,
+                data=settings,
+            )
+            _display_notifications(notifications, force=force)
 
         if manifest["packaging_format"] >= 2:
             if no_safety_backup:
@@ -650,13 +692,12 @@ def app_upgrade(app=[], url=None, file=None, force=False, no_safety_backup=False
         if manifest["packaging_format"] >= 2:
             from yunohost.utils.resources import AppResourceManager
 
-            try:
+            AppResourceManager(
-                AppResourceManager(
+                app_instance_name, wanted=manifest, current=app_dict["manifest"]
-                    app_instance_name, wanted=manifest, current=app_dict["manifest"]
+            ).apply(
-                ).apply(rollback_if_failure=True)
+                rollback_and_raise_exception_if_failure=True,
-            except Exception:
+                operation_logger=operation_logger,
-                # FIXME : improve error handling ....
+            )
-                raise
 
         # Execute the app upgrade script
         upgrade_failed = True
@@ -771,6 +812,24 @@ def app_upgrade(app=[], url=None, file=None, force=False, no_safety_backup=False
             # So much win
             logger.success(m18n.n("app_upgraded", app=app_instance_name))
 
+            # Format post-upgrade notifications
+            if manifest["notifications"]["POST_UPGRADE"]:
+                # Get updated settings to hydrate notifications
+                settings = _get_app_settings(app_instance_name)
+                notifications = _filter_and_hydrate_notifications(
+                    manifest["notifications"]["POST_UPGRADE"],
+                    current_version=app_current_version,
+                    data=settings,
+                )
+                if Moulinette.interface.type == "cli":
+                    # ask for simple confirm
+                    _display_notifications(notifications, force=force)
+
+            # Reset the dismiss flag for post upgrade notification
+            app_setting(
+                app_instance_name, "_dismiss_notification_post_upgrade", delete=True
+            )
+
             hook_callback("post_app_upgrade", env=env_dict)
             operation_logger.success()
 
@@ -778,16 +837,48 @@ def app_upgrade(app=[], url=None, file=None, force=False, no_safety_backup=False
 
     logger.success(m18n.n("upgrade_complete"))
 
+    if Moulinette.interface.type == "api":
+        return {"notifications": {"POST_UPGRADE": notifications}}
 
-def app_manifest(app):
+
+def app_manifest(app, with_screenshot=False):
 
     manifest, extracted_app_folder = _extract_app(app)
 
-    shutil.rmtree(extracted_app_folder)
-
     raw_questions = manifest.get("install", {}).values()
     manifest["install"] = hydrate_questions_with_choices(raw_questions)
 
+    # Add a base64 image to be displayed in web-admin
+    if with_screenshot and Moulinette.interface.type == "api":
+        import base64
+
+        manifest["screenshot"] = None
+        screenshots_folder = os.path.join(extracted_app_folder, "doc", "screenshots")
+
+        if os.path.exists(screenshots_folder):
+            with os.scandir(screenshots_folder) as it:
+                for entry in it:
+                    ext = os.path.splitext(entry.name)[1].replace(".", "").lower()
+                    if entry.is_file() and ext in ("png", "jpg", "jpeg", "webp", "gif"):
+                        with open(entry.path, "rb") as img_file:
+                            data = base64.b64encode(img_file.read()).decode("utf-8")
+                            manifest["screenshot"] = f"data:image/{ext};charset=utf-8;base64,{data}"
+                        break
+
+    shutil.rmtree(extracted_app_folder)
+
+    manifest["requirements"] = {}
+    for name, passed, values, err in _check_manifest_requirements(
+        manifest, action="install"
+    ):
+        if Moulinette.interface.type == "api":
+            manifest["requirements"][name] = {
+                "pass": passed,
+                "values": values,
+            }
+        else:
+            manifest["requirements"][name] = "ok" if passed else m18n.n(err, **values)
+
     return manifest
 
 
@@ -807,19 +898,9 @@ def _confirm_app_install(app, force=False):
     # i18n: confirm_app_install_thirdparty
 
     if quality in ["danger", "thirdparty"]:
-        answer = Moulinette.prompt(
+        _ask_confirmation("confirm_app_install_" + quality, kind="hard")
-            m18n.n("confirm_app_install_" + quality, answers="Yes, I understand"),
-            color="red",
-        )
-        if answer != "Yes, I understand":
-            raise YunohostError("aborting")
-
     else:
-        answer = Moulinette.prompt(
+        _ask_confirmation("confirm_app_install_" + quality, kind="soft")
-            m18n.n("confirm_app_install_" + quality, answers="Y/N"), color="yellow"
-        )
-        if answer.upper() != "Y":
-            raise YunohostError("aborting")
 
 
 @is_unit_operation()
@@ -867,12 +948,11 @@ def app_install(
     manifest, extracted_app_folder = _extract_app(app)
 
     # Display pre_install notices in cli mode
-    if manifest["notifications"]["pre_install"] and Moulinette.interface.type == "cli":
+    if manifest["notifications"]["PRE_INSTALL"] and Moulinette.interface.type == "cli":
-        for notice in manifest["notifications"]["pre_install"].values():
+        notifications = _filter_and_hydrate_notifications(
-            # Should we render the markdown maybe? idk
+            manifest["notifications"]["PRE_INSTALL"]
-            print("==========")
+        )
-            print(_value_for_locale(notice))
+        _display_notifications(notifications, force=force)
-            print("==========")
 
     packaging_format = manifest["packaging_format"]
 
@@ -883,7 +963,17 @@ def app_install(
     app_id = manifest["id"]
 
     # Check requirements
-    _check_manifest_requirements(manifest, action="install")
+    for name, passed, values, err in _check_manifest_requirements(
+        manifest, action="install"
+    ):
+        if not passed:
+            if name == "ram":
+                _ask_confirmation(
+                    "confirm_app_insufficient_ram", params=values, force=force
+                )
+            else:
+                raise YunohostValidationError(err, **values)
+
     _assert_system_is_sane_for_app(manifest, "pre")
 
     # Check if app can be forked
@@ -961,16 +1051,18 @@ def app_install(
                 recursive=True,
             )
 
+    # Override manifest name by given label
+    # This info is also later picked-up by the 'permission' resource initialization
+    if label:
+        manifest["name"] = label
+
     if packaging_format >= 2:
         from yunohost.utils.resources import AppResourceManager
 
-        try:
+        AppResourceManager(app_instance_name, wanted=manifest, current={}).apply(
-            AppResourceManager(app_instance_name, wanted=manifest, current={}).apply(
+            rollback_and_raise_exception_if_failure=True,
-                rollback_if_failure=True
+            operation_logger=operation_logger,
-            )
+        )
-        except Exception:
-            # FIXME : improve error handling ....
-            raise
     else:
         # Initialize the main permission for the app
         # The permission is initialized with no url associated, and with tile disabled
@@ -980,7 +1072,7 @@ def app_install(
         permission_create(
             app_instance_name + ".main",
             allowed=["all_users"],
-            label=label if label else manifest["name"],
+            label=manifest["name"],
             show_tile=False,
             protected=False,
         )
@@ -1034,6 +1126,9 @@ def app_install(
                 "Packagers /!\\ This app manually modified some system configuration files! This should not happen! If you need to do so, you should implement a proper conf_regen hook. Those configuration were affected:\n    - "
                 + "\n     -".join(manually_modified_files_by_app)
             )
+            # Actually forbid this for app packaging >= 2
+            if packaging_format >= 2:
+                broke_the_system = True
 
         # If the install failed or broke the system, we remove it
         if install_failed or broke_the_system:
@@ -1083,13 +1178,9 @@ def app_install(
             if packaging_format >= 2:
                 from yunohost.utils.resources import AppResourceManager
 
-                try:
+                AppResourceManager(
-                    AppResourceManager(
+                    app_instance_name, wanted={}, current=manifest
-                        app_instance_name, wanted={}, current=manifest
+                ).apply(rollback_and_raise_exception_if_failure=False)
-                    ).apply(rollback_if_failure=False)
-                except Exception:
-                    # FIXME : improve error handling ....
-                    raise
             else:
                 # Remove all permission in LDAP
                 for permission_name in user_permission_list()["permissions"].keys():
@@ -1130,19 +1221,23 @@ def app_install(
 
     logger.success(m18n.n("installation_complete"))
 
+    # Get the generated settings to hydrate notifications
+    settings = _get_app_settings(app_instance_name)
+    notifications = _filter_and_hydrate_notifications(
+        manifest["notifications"]["POST_INSTALL"], data=settings
+    )
+
     # Display post_install notices in cli mode
-    if manifest["notifications"]["post_install"] and Moulinette.interface.type == "cli":
+    if notifications and Moulinette.interface.type == "cli":
-        # (Call app_info to get the version hydrated with settings)
+        _display_notifications(notifications, force=force)
-        infos = app_info(app_instance_name, full=True)
-        for notice in infos["manifest"]["notifications"]["post_install"].values():
-            # Should we render the markdown maybe? idk
-            print("==========")
-            print(_value_for_locale(notice))
-            print("==========")
 
     # Call postinstall hook
     hook_callback("post_app_install", env=env_dict)
 
+    # Return hydrated post install notif for API
+    if Moulinette.interface.type == "api":
+        return {"notifications": notifications}
+
 
 @is_unit_operation()
 def app_remove(operation_logger, app, purge=False):
@@ -1210,15 +1305,11 @@ def app_remove(operation_logger, app, purge=False):
 
     packaging_format = manifest["packaging_format"]
     if packaging_format >= 2:
-        try:
+        from yunohost.utils.resources import AppResourceManager
-            from yunohost.utils.resources import AppResourceManager
 
-            AppResourceManager(app, wanted={}, current=manifest).apply(
+        AppResourceManager(app, wanted={}, current=manifest).apply(
-                rollback_if_failure=False
+            rollback_and_raise_exception_if_failure=False, purge_data_dir=purge
-            )
+        )
-        except Exception:
-            # FIXME : improve error handling ....
-            raise
     else:
         # Remove all permission in LDAP
         for permission_name in user_permission_list(apps=[app])["permissions"].keys():
@@ -1616,8 +1707,15 @@ def app_config_get(app, key="", full=False, export=False):
     else:
         mode = "classic"
 
-    config_ = AppConfigPanel(app)
+    try:
-    return config_.get(key, mode)
+        config_ = AppConfigPanel(app)
+        return config_.get(key, mode)
+    except YunohostValidationError as e:
+        if Moulinette.interface.type == "api" and e.key == "config_no_panel":
+            # Be more permissive when no config panel found
+            return {}
+        else:
+            raise
 
 
 @is_unit_operation()
@@ -1690,6 +1788,7 @@ ynh_app_config_run $1
                 "app": app,
                 "app_instance_nb": str(app_instance_nb),
                 "final_path": settings.get("final_path", ""),
+                "install_dir": settings.get("install_dir", ""),
                 "YNH_APP_BASEDIR": os.path.join(APPS_SETTING_PATH, app),
             }
         )
@@ -1924,6 +2023,7 @@ def _get_manifest_of_app(path):
 def _parse_app_doc_and_notifications(path):
 
     doc = {}
+    notification_names = ["PRE_INSTALL", "POST_INSTALL", "PRE_UPGRADE", "POST_UPGRADE"]
 
     for filepath in glob.glob(os.path.join(path, "doc") + "/*.md"):
 
@@ -1934,7 +2034,12 @@ def _parse_app_doc_and_notifications(path):
         if not m:
             # FIXME: shall we display a warning ? idk
             continue
+
         pagename, lang = m.groups()
+
+        if pagename in notification_names:
+            continue
+
         lang = lang.strip("_") if lang else "en"
 
         if pagename not in doc:
@@ -1943,11 +2048,9 @@ def _parse_app_doc_and_notifications(path):
 
     notifications = {}
 
-    for step in ["pre_install", "post_install", "pre_upgrade", "post_upgrade"]:
+    for step in notification_names:
         notifications[step] = {}
-        for filepath in glob.glob(
+        for filepath in glob.glob(os.path.join(path, "doc", f"{step}*.md")):
-            os.path.join(path, "doc", "notifications", f"{step}*.md")
-        ):
             m = re.match(step + "(_[a-z]{2,3})?.md", filepath.split("/")[-1])
             if not m:
                 continue
@@ -1957,9 +2060,7 @@ def _parse_app_doc_and_notifications(path):
                 notifications[step][pagename] = {}
             notifications[step][pagename][lang] = read_file(filepath).strip()
 
-        for filepath in glob.glob(
+        for filepath in glob.glob(os.path.join(path, "doc", f"{step}.d") + "/*.md"):
-            os.path.join(path, "doc", "notifications", f"{step}.d") + "/*.md"
-        ):
             m = re.match(
                 r"([A-Za-z0-9\.\~]*)(_[a-z]{2,3})?.md", filepath.split("/")[-1]
             )
@@ -2007,12 +2108,12 @@ def _convert_v1_manifest_to_v2(manifest):
         .replace(">", "")
         .replace("=", "")
         .replace(" ", ""),
-        "architectures": "all",
+        "architectures": "?",
         "multi_instance": manifest.get("multi_instance", False),
         "ldap": "?",
         "sso": "?",
-        "disk": "50M",
+        "disk": "?",
-        "ram": {"build": "50M", "runtime": "10M"},
+        "ram": {"build": "?", "runtime": "?"},
     }
 
     maintainers = manifest.get("maintainer", {})
@@ -2257,6 +2358,10 @@ def _extract_app_from_folder(path: str) -> Tuple[Dict, str]:
     logger.debug(m18n.n("done"))
 
     manifest["remote"] = {"type": "file", "path": path}
+    manifest["quality"] = {"level": -1, "state": "thirdparty"}
+    manifest["antifeatures"] = []
+    manifest["potential_alternative_to"] = []
+
     return manifest, extracted_app_folder
 
 
@@ -2303,9 +2408,36 @@ def _extract_app_from_gitrepo(
         manifest["remote"]["revision"] = revision
         manifest["lastUpdate"] = app_info.get("lastUpdate")
 
+    manifest["quality"] = {
+        "level": app_info.get("level", -1),
+        "state": app_info.get("state", "thirdparty"),
+    }
+    manifest["antifeatures"] = app_info.get("antifeatures", [])
+    manifest["potential_alternative_to"] = app_info.get("potential_alternative_to", [])
+
     return manifest, extracted_app_folder
 
 
+def _list_upgradable_apps():
+    upgradable_apps = list(app_list(upgradable=True)["apps"])
+
+    # Retrieve next manifest pre_upgrade notifications
+    for app in upgradable_apps:
+        absolute_app_name, _ = _parse_app_instance_name(app["id"])
+        manifest, extracted_app_folder = _extract_app(absolute_app_name)
+        app["notifications"] = {}
+        if manifest["notifications"]["PRE_UPGRADE"]:
+            app["notifications"]["PRE_UPGRADE"] = _filter_and_hydrate_notifications(
+                manifest["notifications"]["PRE_UPGRADE"],
+                app["current_version"],
+                app["settings"],
+            )
+        del app["settings"]
+        shutil.rmtree(extracted_app_folder)
+
+    return upgradable_apps
+
+
 #
 # ############################### #
 #        Small utilities          #
@@ -2354,74 +2486,100 @@ def _get_all_installed_apps_id():
     return all_apps_ids_formatted
 
 
-def _check_manifest_requirements(manifest: Dict, action: str):
+def _check_manifest_requirements(
+    manifest: Dict, action: str = ""
+) -> Iterator[Tuple[str, bool, object, str]]:
     """Check if required packages are met from the manifest"""
 
+    app_id = manifest["id"]
+    logger.debug(m18n.n("app_requirements_checking", app=app_id))
+
+    # Packaging format
     if manifest["packaging_format"] not in [1, 2]:
         raise YunohostValidationError("app_packaging_format_not_supported")
 
-    app_id = manifest["id"]
+    # Yunohost version
-
+    required_yunohost_version = (
-    logger.debug(m18n.n("app_requirements_checking", app=app_id))
+        manifest["integration"].get("yunohost", "4.3").strip(">= ")
-
-    # Yunohost version requirement
-    yunohost_requirement = version.parse(
-        manifest["integration"]["yunohost"].strip(">= ") or "4.3"
     )
-    yunohost_installed_version = version.parse(
+    current_yunohost_version = get_ynh_package_version("yunohost")["version"]
-        get_ynh_package_version("yunohost")["version"]
+
+    yield (
+        "required_yunohost_version",
+        version.parse(required_yunohost_version)
+        <= version.parse(current_yunohost_version),
+        {"current": current_yunohost_version, "required": required_yunohost_version},
+        "app_yunohost_version_not_supported",  # i18n: app_yunohost_version_not_supported
     )
-    if yunohost_requirement > yunohost_installed_version:
-        # FIXME : i18n
-        raise YunohostValidationError(
-            f"This app requires Yunohost >= {yunohost_requirement} but current installed version is {yunohost_installed_version}"
-        )
 
     # Architectures
     arch_requirement = manifest["integration"]["architectures"]
-    if arch_requirement != "all":
+    arch = system_arch()
-        arch = system_arch()
+
-        if arch not in arch_requirement:
+    yield (
-            # FIXME: i18n
+        "arch",
-            raise YunohostValidationError(
+        arch_requirement in ["all", "?"] or arch in arch_requirement,
-                f"This app can only be installed on architectures {', '.join(arch_requirement)} but your server architecture is {arch}"
+        {"current": arch, "required": arch_requirement},
-            )
+        "app_arch_not_supported",  # i18n: app_arch_not_supported
+    )
 
     # Multi-instance
-    if action == "install" and manifest["integration"]["multi_instance"] is False:
+    if action == "install":
-        apps = _installed_apps()
+        multi_instance = manifest["integration"]["multi_instance"] is True
-        sibling_apps = [a for a in apps if a == app_id or a.startswith(f"{app_id}__")]
+        if not multi_instance:
-        if len(sibling_apps) > 0:
+            apps = _installed_apps()
-            raise YunohostValidationError("app_already_installed", app=app_id)
+            sibling_apps = [
+                a for a in apps if a == app_id or a.startswith(f"{app_id}__")
+            ]
+            multi_instance = len(sibling_apps) == 0
+
+        yield (
+            "install",
+            multi_instance,
+            {"app": app_id},
+            "app_already_installed",  # i18n: app_already_installed
+        )
 
     # Disk
     if action == "install":
-        disk_requirement = manifest["integration"]["disk"]
+        root_free_space = free_space_in_directory("/")
-
+        var_free_space = free_space_in_directory("/var")
-        if free_space_in_directory("/") <= human_to_binary(
+        if manifest["integration"]["disk"] == "?":
-            disk_requirement
+            has_enough_disk = True
-        ) or free_space_in_directory("/var") <= human_to_binary(disk_requirement):
+        else:
-            # FIXME : i18m
+            disk_req_bin = human_to_binary(manifest["integration"]["disk"])
-            raise YunohostValidationError(
+            has_enough_disk = (
-                f"This app requires {disk_requirement} free space."
+                root_free_space > disk_req_bin and var_free_space > disk_req_bin
             )
+        free_space = binary_to_human(min(root_free_space, var_free_space))
 
-    # Ram for build
+        yield (
-    ram_build_requirement = manifest["integration"]["ram"]["build"]
+            "disk",
-    # Is "include_swap" really useful ? We should probably decide wether to always include it or not instead
+            has_enough_disk,
-    ram_include_swap = manifest["integration"]["ram"].get("include_swap", False)
+            {"current": free_space, "required": manifest["integration"]["disk"]},
-
+            "app_not_enough_disk",  # i18n: app_not_enough_disk
-    ram, swap = ram_available()
-    if ram_include_swap:
-        ram += swap
-
-    if ram < human_to_binary(ram_build_requirement):
-        # FIXME : i18n
-        ram_human = binary_to_human(ram)
-        raise YunohostValidationError(
-            f"This app requires {ram_build_requirement} RAM to install/upgrade but only {ram_human} is available right now."
         )
 
+    # Ram
+    ram_requirement = manifest["integration"]["ram"]
+    ram, swap = ram_available()
+    # Is "include_swap" really useful ? We should probably decide wether to always include it or not instead
+    if ram_requirement.get("include_swap", False):
+        ram += swap
+    can_build = ram_requirement["build"] == "?" or ram > human_to_binary(
+        ram_requirement["build"]
+    )
+    can_run = ram_requirement["runtime"] == "?" or ram > human_to_binary(
+        ram_requirement["runtime"]
+    )
+
+    yield (
+        "ram",
+        can_build and can_run,
+        {"current": binary_to_human(ram), "required": ram_requirement["build"]},
+        "app_not_enough_ram",  # i18n: app_not_enough_ram
+    )
+
 
 def _guess_webapp_path_requirement(app_folder: str) -> str:
 
@@ -2740,3 +2898,111 @@ def _assert_system_is_sane_for_app(manifest, when):
             raise YunohostValidationError("dpkg_is_broken")
         elif when == "post":
             raise YunohostError("this_action_broke_dpkg")
+
+
+def app_dismiss_notification(app, name):
+
+    assert isinstance(name, str)
+    name = name.lower()
+    assert name in ["post_install", "post_upgrade"]
+    _assert_is_installed(app)
+
+    app_setting(app, f"_dismiss_notification_{name}", value="1")
+
+
+def _notification_is_dismissed(name, settings):
+    # Check for _dismiss_notiication_$name setting and also auto-dismiss
+    # notifications after one week (otherwise people using mostly CLI would
+    # never really dismiss the notification and it would be displayed forever)
+
+    if name == "POST_INSTALL":
+        return (
+            settings.get("_dismiss_notification_post_install")
+            or (int(time.time()) - settings.get("install_time", 0)) / (24 * 3600) > 7
+        )
+    elif name == "POST_UPGRADE":
+        # Check on update_time also implicitly prevent the post_upgrade notification
+        # from being displayed after install, because update_time is only set during upgrade
+        return (
+            settings.get("_dismiss_notification_post_upgrade")
+            or (int(time.time()) - settings.get("update_time", 0)) / (24 * 3600) > 7
+        )
+    else:
+        return False
+
+
+def _filter_and_hydrate_notifications(notifications, current_version=None, data={}):
+
+    def is_version_more_recent_than_current_version(name):
+        # Boring code to handle the fact that "0.1 < 9999~ynh1" is False
+
+        if "~" in name:
+            return version.parse(name) > version.parse(current_version)
+        else:
+            return version.parse(name) > version.parse(current_version.split("~")[0])
+
+    return {
+        # Should we render the markdown maybe? idk
+        name: _hydrate_app_template(_value_for_locale(content_per_lang), data)
+        for name, content_per_lang in notifications.items()
+        if current_version is None
+        or name == "main"
+        or is_version_more_recent_than_current_version(name)
+    }
+
+
+def _display_notifications(notifications, force=False):
+    if not notifications:
+        return
+
+    for name, content in notifications.items():
+        print("==========")
+        print(content)
+    print("==========")
+
+    _ask_confirmation("confirm_notifications_read", kind="simple", force=force)
+
+
+# FIXME: move this to Moulinette
+def _ask_confirmation(
+    question: str,
+    params: dict = {},
+    kind: str = "hard",
+    force: bool = False,
+):
+    """
+    Ask confirmation
+
+    Keyword argument:
+        question -- m18n key or string
+        params -- dict of values passed to the string formating
+        kind -- "hard": ask with "Yes, I understand", "soft": "Y/N", "simple": "press enter"
+        force -- Will not ask for confirmation
+
+    """
+    if force or Moulinette.interface.type == "api":
+        return
+
+    # If ran from the CLI in a non-interactive context,
+    # skip confirmation (except in hard mode)
+    if not os.isatty(1) and kind in ["simple", "soft"]:
+        return
+    if kind == "simple":
+        answer = Moulinette.prompt(
+            m18n.n(question, answers="Press enter to continue", **params),
+            color="yellow",
+        )
+        answer = True
+    elif kind == "soft":
+        answer = Moulinette.prompt(
+            m18n.n(question, answers="Y/N", **params), color="yellow"
+        )
+        answer = answer.upper() == "Y"
+    else:
+        answer = Moulinette.prompt(
+            m18n.n(question, answers="Yes, I understand", **params), color="red"
+        )
+        answer = answer == "Yes, I understand"
+
+    if not answer:
+        raise YunohostError("aborting")

src/app_catalog.py

@@ -18,6 +18,7 @@
 #
 import os
 import re
+import hashlib
 
 from moulinette import m18n
 from moulinette.utils.log import getActionLogger
@@ -36,17 +37,18 @@ from yunohost.utils.error import YunohostError
 logger = getActionLogger("yunohost.app_catalog")
 
 APPS_CATALOG_CACHE = "/var/cache/yunohost/repo"
+APPS_CATALOG_LOGOS = "/usr/share/yunohost/applogos"
 APPS_CATALOG_CONF = "/etc/yunohost/apps_catalog.yml"
 APPS_CATALOG_API_VERSION = 3
 APPS_CATALOG_DEFAULT_URL = "https://app.yunohost.org/default"
 
 
-def app_catalog(full=False, with_categories=False):
+def app_catalog(full=False, with_categories=False, with_antifeatures=False):
     """
     Return a dict of apps available to installation from Yunohost's app catalog
     """
 
-    from yunohost.app import _installed_apps, _set_default_ask_questions
+    from yunohost.app import _installed_apps
 
     # Get app list from catalog cache
     catalog = _load_apps_catalog()
@@ -65,28 +67,38 @@ def app_catalog(full=False, with_categories=False):
                 "description": infos["manifest"]["description"],
                 "level": infos["level"],
             }
-        else:
-            infos["manifest"]["install"] = _set_default_ask_questions(
-                infos["manifest"].get("install", {})
-            )
 
-    # Trim info for categories if not using --full
+    _catalog = {"apps": catalog["apps"]}
-    for category in catalog["categories"]:
-        category["title"] = _value_for_locale(category["title"])
-        category["description"] = _value_for_locale(category["description"])
-        for subtags in category.get("subtags", []):
-            subtags["title"] = _value_for_locale(subtags["title"])
 
-    if not full:
+    if with_categories:
-        catalog["categories"] = [
+        for category in catalog["categories"]:
-            {"id": c["id"], "description": c["description"]}
+            category["title"] = _value_for_locale(category["title"])
-            for c in catalog["categories"]
+            category["description"] = _value_for_locale(category["description"])
-        ]
+            for subtags in category.get("subtags", []):
+                subtags["title"] = _value_for_locale(subtags["title"])
 
-    if not with_categories:
+        if not full:
-        return {"apps": catalog["apps"]}
+            catalog["categories"] = [
-    else:
+                {"id": c["id"], "description": c["description"]}
-        return {"apps": catalog["apps"], "categories": catalog["categories"]}
+                for c in catalog["categories"]
+            ]
+
+        _catalog["categories"] = catalog["categories"]
+
+    if with_antifeatures:
+        for antifeature in catalog["antifeatures"]:
+            antifeature["title"] = _value_for_locale(antifeature["title"])
+            antifeature["description"] = _value_for_locale(antifeature["description"])
+
+        if not full:
+            catalog["antifeatures"] = [
+                {"id": a["id"], "description": a["description"]}
+                for a in catalog["antifeatures"]
+            ]
+
+        _catalog["antifeatures"] = catalog["antifeatures"]
+
+    return _catalog
 
 
 def app_search(string):
@@ -172,6 +184,9 @@ def _update_apps_catalog():
         logger.debug("Initialize folder for apps catalog cache")
         mkdir(APPS_CATALOG_CACHE, mode=0o750, parents=True, uid="root")
 
+    if not os.path.exists(APPS_CATALOG_LOGOS):
+        mkdir(APPS_CATALOG_LOGOS, mode=0o755, parents=True, uid="root")
+
     for apps_catalog in apps_catalog_list:
         if apps_catalog["url"] is None:
             continue
@@ -202,6 +217,37 @@ def _update_apps_catalog():
                 raw_msg=True,
             )
 
+        # Download missing app logos
+        logos_to_download = []
+        for app, infos in apps_catalog_content["apps"].items():
+            logo_hash = infos.get("logo_hash")
+            if not logo_hash or os.path.exists(f"{APPS_CATALOG_LOGOS}/{logo_hash}.png"):
+                continue
+            logos_to_download.append(logo_hash)
+
+        if len(logos_to_download) > 20:
+            logger.info(f"(Will fetch {len(logos_to_download)} logos, this may take a couple minutes)")
+
+        import requests
+        from multiprocessing.pool import ThreadPool
+
+        def fetch_logo(logo_hash):
+            try:
+                r = requests.get(f"{apps_catalog['url']}/v{APPS_CATALOG_API_VERSION}/logos/{logo_hash}.png", timeout=10)
+                assert r.status_code == 200, f"Got status code {r.status_code}, expected 200"
+                if hashlib.sha256(r.content).hexdigest() != logo_hash:
+                    raise Exception(f"Found inconsistent hash while downloading logo {logo_hash}")
+                open(f"{APPS_CATALOG_LOGOS}/{logo_hash}.png", "wb").write(r.content)
+                return True
+            except Exception as e:
+                logger.debug(f"Failed to download logo {logo_hash} : {e}")
+                return False
+
+        results = ThreadPool(8).imap_unordered(fetch_logo, logos_to_download)
+        for result in results:
+            # Is this even needed to iterate on the results ?
+            pass
+
     logger.success(m18n.n("apps_catalog_update_success"))
 
 
@@ -211,7 +257,7 @@ def _load_apps_catalog():
     corresponding to all known apps and categories
     """
 
-    merged_catalog = {"apps": {}, "categories": []}
+    merged_catalog = {"apps": {}, "categories": [], "antifeatures": []}
 
     for apps_catalog_id in [L["id"] for L in _read_apps_catalog_list()]:
 
@@ -261,7 +307,9 @@ def _load_apps_catalog():
             info["repository"] = apps_catalog_id
             merged_catalog["apps"][app] = info
 
-        # Annnnd categories
+        # Annnnd categories + antifeatures
-        merged_catalog["categories"] += apps_catalog_content["categories"]
+        # (we use .get here, only because the dev catalog doesnt include the categories/antifeatures keys)
+        merged_catalog["categories"] += apps_catalog_content.get("categories", [])
+        merged_catalog["antifeatures"] += apps_catalog_content.get("antifeatures", [])
 
     return merged_catalog

src/backup.py

@@ -1518,13 +1518,9 @@ class RestoreManager:
         if manifest["packaging_format"] >= 2:
             from yunohost.utils.resources import AppResourceManager
 
-            try:
+            AppResourceManager(
-                AppResourceManager(
+                app_instance_name, wanted=manifest, current={}
-                    app_instance_name, wanted=manifest, current={}
+            ).apply(rollback_and_raise_exception_if_failure=True, operation_logger=operation_logger)
-                ).apply(rollback_if_failure=True)
-            except Exception:
-                # FIXME : improve error handling ....
-                raise
 
         # Execute the app install script
         restore_failed = True

src/firewall.py

@@ -32,7 +32,7 @@ logger = getActionLogger("yunohost.firewall")
 
 
 def firewall_allow(
-    protocol, port, ipv4_only=False, ipv6_only=False, no_upnp=False, no_reload=False
+    protocol, port, ipv4_only=False, ipv6_only=False, no_upnp=False, no_reload=False, reload_only_if_change=False
 ):
     """
     Allow connections on a port
@@ -70,14 +70,18 @@ def firewall_allow(
             "ipv6",
         ]
 
+    changed = False
+
     for p in protocols:
         # Iterate over IP versions to add port
         for i in ipvs:
             if port not in firewall[i][p]:
                 firewall[i][p].append(port)
+                changed = True
             else:
                 ipv = "IPv%s" % i[3]
-                logger.warning(m18n.n("port_already_opened", port=port, ip_version=ipv))
+                if not reload_only_if_change:
+                    logger.warning(m18n.n("port_already_opened", port=port, ip_version=ipv))
         # Add port forwarding with UPnP
         if not no_upnp and port not in firewall["uPnP"][p]:
             firewall["uPnP"][p].append(port)
@@ -89,12 +93,12 @@ def firewall_allow(
 
     # Update and reload firewall
     _update_firewall_file(firewall)
-    if not no_reload:
+    if not no_reload or (reload_only_if_change and changed):
         return firewall_reload()
 
 
 def firewall_disallow(
-    protocol, port, ipv4_only=False, ipv6_only=False, upnp_only=False, no_reload=False
+    protocol, port, ipv4_only=False, ipv6_only=False, upnp_only=False, no_reload=False, reload_only_if_change=False
 ):
     """
     Disallow connections on a port
@@ -139,14 +143,18 @@ def firewall_disallow(
     elif upnp_only:
         ipvs = []
 
+    changed = False
+
     for p in protocols:
         # Iterate over IP versions to remove port
         for i in ipvs:
             if port in firewall[i][p]:
                 firewall[i][p].remove(port)
+                changed = True
             else:
                 ipv = "IPv%s" % i[3]
-                logger.warning(m18n.n("port_already_closed", port=port, ip_version=ipv))
+                if not reload_only_if_change:
+                    logger.warning(m18n.n("port_already_closed", port=port, ip_version=ipv))
         # Remove port forwarding with UPnP
         if upnp and port in firewall["uPnP"][p]:
             firewall["uPnP"][p].remove(port)
@@ -156,7 +164,7 @@ def firewall_disallow(
 
     # Update and reload firewall
     _update_firewall_file(firewall)
-    if not no_reload:
+    if not no_reload or (reload_only_if_change and changed):
         return firewall_reload()
 
 

src/permission.py

@@ -479,6 +479,7 @@ def permission_url(
     url=None,
     add_url=None,
     remove_url=None,
+    set_url=None,
     auth_header=None,
     clear_urls=False,
     sync_perm=True,
@@ -491,6 +492,7 @@ def permission_url(
         url         -- (optional) URL for which access will be allowed/forbidden.
         add_url     -- (optional) List of additional url to add for which access will be allowed/forbidden
         remove_url  -- (optional) List of additional url to remove for which access will be allowed/forbidden
+        set_url     -- (optional) List of additional url to set/replace for which access will be allowed/forbidden
         auth_header -- (optional) Define for the URL of this permission, if SSOwat pass the authentication header to the application
         clear_urls  -- (optional) Clean all urls (url and additional_urls)
     """
@@ -556,6 +558,9 @@ def permission_url(
 
         new_additional_urls = [u for u in new_additional_urls if u not in remove_url]
 
+    if set_url:
+        new_additional_urls = set_url
+
     if auth_header is None:
         auth_header = existing_permission["auth_header"]
 

src/tests/test_app_resources.py

@@ -11,6 +11,7 @@ from yunohost.utils.resources import (
     AppResourceClassesByType,
 )
 from yunohost.permission import user_permission_list, permission_delete
+from yunohost.firewall import firewall_list
 
 dummyfile = "/tmp/dummyappresource-testapp"
 
@@ -75,7 +76,7 @@ def test_provision_dummy():
 
     assert not os.path.exists(dummyfile)
     AppResourceManager("testapp", current=current, wanted=wanted).apply(
-        rollback_if_failure=False
+        rollback_and_raise_exception_if_failure=False
     )
     assert open(dummyfile).read().strip() == "foo"
 
@@ -89,7 +90,7 @@ def test_deprovision_dummy():
 
     assert open(dummyfile).read().strip() == "foo"
     AppResourceManager("testapp", current=current, wanted=wanted).apply(
-        rollback_if_failure=False
+        rollback_and_raise_exception_if_failure=False
     )
     assert not os.path.exists(dummyfile)
 
@@ -101,7 +102,7 @@ def test_provision_dummy_nondefaultvalue():
 
     assert not os.path.exists(dummyfile)
     AppResourceManager("testapp", current=current, wanted=wanted).apply(
-        rollback_if_failure=False
+        rollback_and_raise_exception_if_failure=False
     )
     assert open(dummyfile).read().strip() == "bar"
 
@@ -115,26 +116,11 @@ def test_update_dummy():
 
     assert open(dummyfile).read().strip() == "foo"
     AppResourceManager("testapp", current=current, wanted=wanted).apply(
-        rollback_if_failure=False
+        rollback_and_raise_exception_if_failure=False
     )
     assert open(dummyfile).read().strip() == "bar"
 
 
-def test_update_dummy_fail():
-
-    current = {"resources": {"dummy": {}}}
-    wanted = {"resources": {"dummy": {"content": "forbiddenvalue"}}}
-
-    open(dummyfile, "w").write("foo")
-
-    assert open(dummyfile).read().strip() == "foo"
-    with pytest.raises(Exception):
-        AppResourceManager("testapp", current=current, wanted=wanted).apply(
-            rollback_if_failure=False
-        )
-    assert open(dummyfile).read().strip() == "forbiddenvalue"
-
-
 def test_update_dummy_failwithrollback():
 
     current = {"resources": {"dummy": {}}}
@@ -145,7 +131,7 @@ def test_update_dummy_failwithrollback():
     assert open(dummyfile).read().strip() == "foo"
     with pytest.raises(Exception):
         AppResourceManager("testapp", current=current, wanted=wanted).apply(
-            rollback_if_failure=True
+            rollback_and_raise_exception_if_failure=True
         )
     assert open(dummyfile).read().strip() == "foo"
 
@@ -276,6 +262,26 @@ def test_resource_ports_several():
     assert not app_setting("testapp", "port_foobar")
 
 
+def test_resource_ports_firewall():
+
+    r = AppResourceClassesByType["ports"]
+    conf = {"main": {"default": 12345}}
+
+    r(conf, "testapp").provision_or_update()
+
+    assert 12345 not in firewall_list()["opened_ports"]
+
+    conf = {"main": {"default": 12345, "exposed": "TCP"}}
+
+    r(conf, "testapp").provision_or_update()
+
+    assert 12345 in firewall_list()["opened_ports"]
+
+    r(conf, "testapp").deprovision()
+
+    assert 12345 not in firewall_list()["opened_ports"]
+
+
 def test_resource_database():
 
     r = AppResourceClassesByType["database"]
@@ -397,9 +403,7 @@ def test_resource_permissions():
 
     res = user_permission_list(full=True)["permissions"]
 
-    # FIXME FIXME FIXME : this is the current behavior but
+    assert res["testapp.admin"]["url"] == "/adminpanel"
-    # it is NOT okay. c.f. comment in the code
-    assert res["testapp.admin"]["url"] == "/admin"  # should be '/adminpanel'
 
     r(conf, "testapp").deprovision()
 

src/tests/test_apps.py

@@ -223,10 +223,10 @@ def test_legacy_app_manifest_preinstall():
     assert "install" in m
     assert m["doc"] == {}
     assert m["notifications"] == {
-        "pre_install": {},
+        "PRE_INSTALL": {},
-        "pre_upgrade": {},
+        "PRE_UPGRADE": {},
-        "post_install": {},
+        "POST_INSTALL": {},
-        "post_upgrade": {},
+        "POST_UPGRADE": {},
     }
 
 
@@ -249,11 +249,11 @@ def test_manifestv2_app_manifest_preinstall():
     assert "notifications" in m
     assert (
         "This is a dummy disclaimer to display prior to the install"
-        in m["notifications"]["pre_install"]["main"]["en"]
+        in m["notifications"]["PRE_INSTALL"]["main"]["en"]
     )
     assert (
         "Ceci est un faux disclaimer à présenter avant l'installation"
-        in m["notifications"]["pre_install"]["main"]["fr"]
+        in m["notifications"]["PRE_INSTALL"]["main"]["fr"]
     )
 
 
@@ -295,15 +295,15 @@ def test_manifestv2_app_info_postinstall():
     assert "notifications" in m
     assert (
         "The app install dir is /var/www/manifestv2_app"
-        in m["notifications"]["post_install"]["main"]["en"]
+        in m["notifications"]["POST_INSTALL"]["main"]["en"]
     )
     assert (
         "The app id is manifestv2_app"
-        in m["notifications"]["post_install"]["main"]["en"]
+        in m["notifications"]["POST_INSTALL"]["main"]["en"]
     )
     assert (
         f"The app url is {main_domain}/manifestv2"
-        in m["notifications"]["post_install"]["main"]["en"]
+        in m["notifications"]["POST_INSTALL"]["main"]["en"]
     )
 
 
@@ -341,7 +341,7 @@ def test_manifestv2_app_info_preupgrade(monkeypatch):
     # should parse the files in the original app repo, possibly with proper i18n etc
     assert (
         "This is a dummy disclaimer to display prior to any upgrade"
-        in i["from_catalog"]["manifest"]["notifications"]["pre_upgrade"]["main"]["en"]
+        in i["from_catalog"]["manifest"]["notifications"]["PRE_UPGRADE"]["main"]["en"]
     )
 
 

src/tools.py

@@ -29,7 +29,11 @@ from moulinette.utils.log import getActionLogger
 from moulinette.utils.process import call_async_output
 from moulinette.utils.filesystem import read_yaml, write_to_yaml, cp, mkdir, rm, chown
 
-from yunohost.app import app_upgrade, app_list
+from yunohost.app import (
+    app_upgrade,
+    app_list,
+    _list_upgradable_apps,
+)
 from yunohost.app_catalog import (
     _initialize_apps_catalog_system,
     _update_apps_catalog,
@@ -363,7 +367,7 @@ def tools_update(target=None):
         except YunohostError as e:
             logger.error(str(e))
 
-        upgradable_apps = list(app_list(upgradable=True)["apps"])
+        upgradable_apps = _list_upgradable_apps()
 
     if len(upgradable_apps) == 0 and len(upgradable_system_packages) == 0:
         logger.info(m18n.n("already_up_to_date"))

src/utils/resources.py

@@ -22,6 +22,7 @@ import shutil
 import random
 from typing import Dict, Any, List
 
+from moulinette import m18n
 from moulinette.utils.process import check_output
 from moulinette.utils.log import getActionLogger
 from moulinette.utils.filesystem import mkdir, chown, chmod, write_to_file
@@ -29,16 +30,13 @@ from moulinette.utils.filesystem import (
     rm,
 )
 
-from yunohost.utils.error import YunohostError
+from yunohost.utils.error import YunohostError, YunohostValidationError
 
 logger = getActionLogger("yunohost.app_resources")
 
 
 class AppResourceManager:
 
-    # FIXME : add some sort of documentation mechanism
-    # to create a have a detailed description of each resource behavior
-
     def __init__(self, app: str, current: Dict, wanted: Dict):
 
         self.app = app
@@ -50,7 +48,7 @@ class AppResourceManager:
         if "resources" not in self.wanted:
             self.wanted["resources"] = {}
 
-    def apply(self, rollback_if_failure, **context):
+    def apply(self, rollback_and_raise_exception_if_failure, operation_logger=None, **context):
 
         todos = list(self.compute_todos())
         completed = []
@@ -69,12 +67,13 @@ class AppResourceManager:
                 elif todo == "update":
                     logger.info(f"Updating {name} ...")
                     new.provision_or_update(context=context)
-            # FIXME FIXME FIXME : this exception doesnt catch Ctrl+C ?!?!
+            except (KeyboardInterrupt, Exception) as e:
-            except Exception as e:
                 exception = e
-                # FIXME: better error handling ? display stacktrace ?
+                if isinstance(e, KeyboardInterrupt):
-                logger.warning(f"Failed to {todo} for {name} : {e}")
+                    logger.error(m18n.n("operation_interrupted"))
-                if rollback_if_failure:
+                else:
+                    logger.warning(f"Failed to {todo} {name} : {e}")
+                if rollback_and_raise_exception_if_failure:
                     rollback = True
                     completed.append((todo, name, old, new))
                     break
@@ -97,12 +96,24 @@ class AppResourceManager:
                     elif todo == "update":
                         logger.info(f"Reverting {name} ...")
                         old.provision_or_update(context=context)
-                except Exception as e:
+                except (KeyboardInterrupt, Exception) as e:
-                    # FIXME: better error handling ? display stacktrace ?
+                    if isinstance(e, KeyboardInterrupt):
-                    logger.error(f"Failed to rollback {name} : {e}")
+                        logger.error(m18n.n("operation_interrupted"))
+                    else:
+                        logger.error(f"Failed to rollback {name} : {e}")
 
         if exception:
-            raise exception
+            if rollback_and_raise_exception_if_failure:
+                logger.error(m18n.n("app_resource_failed", app=self.app, error=exception))
+                if operation_logger:
+                    failure_message_with_debug_instructions = operation_logger.error(str(exception))
+                    raise YunohostError(
+                        failure_message_with_debug_instructions, raw_msg=True
+                    )
+                else:
+                    raise YunohostError(str(exception), raw_msg=True)
+            else:
+                logger.error(exception)
 
     def compute_todos(self):
 
@@ -248,7 +259,7 @@ class PermissionsResource(AppResource):
 
     ##### Provision/Update:
     - Delete any permissions that may exist and be related to this app yet is not declared anymore
-    - Loop over the declared permissions and create them if needed or update them with the new values (FIXME : update ain't implemented yet >_>)
+    - Loop over the declared permissions and create them if needed or update them with the new values
 
     ##### Deprovision:
     - Delete all permission related to this app
@@ -302,7 +313,7 @@ class PermissionsResource(AppResource):
 
         from yunohost.permission import (
             permission_create,
-            # permission_url,
+            permission_url,
             permission_delete,
             user_permission_list,
             user_permission_update,
@@ -320,7 +331,8 @@ class PermissionsResource(AppResource):
                 permission_delete(perm, force=True, sync_perm=False)
 
         for perm, infos in self.permissions.items():
-            if f"{self.app}.{perm}" not in existing_perms:
+            perm_id = f"{self.app}.{perm}"
+            if perm_id not in existing_perms:
                 # Use the 'allowed' key from the manifest,
                 # or use the 'init_{perm}_permission' from the install questions
                 # which is temporarily saved as a setting as an ugly hack to pass the info to this piece of code...
@@ -330,7 +342,7 @@ class PermissionsResource(AppResource):
                     or []
                 )
                 permission_create(
-                    f"{self.app}.{perm}",
+                    perm_id,
                     allowed=init_allowed,
                     # This is why the ugly hack with self.manager exists >_>
                     label=self.manager.wanted["name"] if perm == "main" else perm,
@@ -341,17 +353,19 @@ class PermissionsResource(AppResource):
                 )
                 self.delete_setting(f"init_{perm}_permission")
 
-                user_permission_update(
+            user_permission_update(
-                    f"{self.app}.{perm}",
+                perm_id,
-                    show_tile=infos["show_tile"],
+                show_tile=infos["show_tile"],
-                    protected=infos["protected"],
+                protected=infos["protected"],
-                    sync_perm=False,
+                sync_perm=False,
-                )
+            )
-            else:
+            permission_url(
-                pass
+                perm_id,
-                # FIXME : current implementation of permission_url is hell for
+                url=infos["url"],
-                # easy declarativeness of additional_urls >_> ...
+                set_url=infos["additional_urls"],
-                # permission_url(f"{self.app}.{perm}", url=infos["url"], auth_header=infos["auth_header"], sync_perm=False)
+                auth_header=infos["auth_header"],
+                sync_perm=False,
+            )
 
         permission_sync_to_user()
 
@@ -523,6 +537,8 @@ class InstalldirAppResource(AppResource):
         if not current_install_dir and os.path.isdir(self.dir):
             rm(self.dir, recursive=True)
 
+        # isdir will be True if the path is a symlink pointing to a dir
+        # This should cover cases where people moved the data dir to another place via a symlink (ie we dont enter the if)
         if not os.path.isdir(self.dir):
             # Handle case where install location changed, in which case we shall move the existing install dir
             # FIXME: confirm that's what we wanna do
@@ -551,8 +567,10 @@ class InstalldirAppResource(AppResource):
 
         perm_octal = 0o100 * owner_perm_octal + 0o010 * group_perm_octal
 
-        chmod(self.dir, perm_octal)
+        # NB: we use realpath here to cover cases where self.dir could actually be a symlink
-        chown(self.dir, owner, group)
+        # in which case we want to apply the perm to the pointed dir, not to the symlink
+        chmod(os.path.realpath(self.dir), perm_octal)
+        chown(os.path.realpath(self.dir), owner, group)
         # FIXME: shall we apply permissions recursively ?
 
         self.set_setting("install_dir", self.dir)
@@ -592,9 +610,8 @@ class DatadirAppResource(AppResource):
     - save the value of `dir` as `data_dir` in the app's settings, which can be then used by the app scripts (`$data_dir`) and conf templates (`__DATA_DIR__`)
 
     ##### Deprovision:
-    - recursively deletes the directory if it exists
+    - (only if the purge option is chosen by the user) recursively deletes the directory if it exists
-    - FIXME: this should only be done if the PURGE option is set
+    - also delete the corresponding setting
-    - FIXME: this should also delete the corresponding setting
 
     ##### Legacy management:
     - In the past, the setting may have been called `datadir`. The code will automatically rename it as `data_dir`.
@@ -628,11 +645,15 @@ class DatadirAppResource(AppResource):
 
         current_data_dir = self.get_setting("data_dir") or self.get_setting("datadir")
 
+        # isdir will be True if the path is a symlink pointing to a dir
+        # This should cover cases where people moved the data dir to another place via a symlink (ie we dont enter the if)
         if not os.path.isdir(self.dir):
             # Handle case where install location changed, in which case we shall move the existing install dir
             # FIXME: same as install_dir, is this what we want ?
-            # FIXME: What if people manually mved the data dir and changed the setting value and dont want the folder to be moved ? x_x
             if current_data_dir and os.path.isdir(current_data_dir):
+                logger.warning(
+                    f"Moving {current_data_dir} to {self.dir} ... (this may take a while)"
+                )
                 shutil.move(current_data_dir, self.dir)
             else:
                 mkdir(self.dir)
@@ -651,8 +672,10 @@ class DatadirAppResource(AppResource):
         )
         perm_octal = 0o100 * owner_perm_octal + 0o010 * group_perm_octal
 
-        chmod(self.dir, perm_octal)
+        # NB: we use realpath here to cover cases where self.dir could actually be a symlink
-        chown(self.dir, owner, group)
+        # in which case we want to apply the perm to the pointed dir, not to the symlink
+        chmod(os.path.realpath(self.dir), perm_octal)
+        chown(os.path.realpath(self.dir), owner, group)
 
         self.set_setting("data_dir", self.dir)
         self.delete_setting("datadir")  # Legacy
@@ -663,11 +686,10 @@ class DatadirAppResource(AppResource):
         assert self.owner.strip()
         assert self.group.strip()
 
-        # FIXME: This should rm the datadir only if purge is enabled
+        if context.get("purge_data_dir", False) and os.path.isdir(self.dir):
-        pass
+            rm(self.dir, recursive=True)
-        # if os.path.isdir(self.dir):
+
-        #    rm(self.dir, recursive=True)
+        self.delete_setting("data_dir")
-        # FIXME : in fact we should delete settings to be consistent
 
 
 class AptDependenciesAppResource(AppResource):
@@ -756,16 +778,16 @@ class PortsResource(AppResource):
 
     ##### Properties (for every port name):
     - `default`: The prefered value for the port. If this port is already being used by another process right now, or is booked in another app's setting, the code will increment the value until it finds a free port and store that value as the setting. If no value is specified, a random value between 10000 and 60000 is used.
-    - `exposed`: (default: `false`) Wether this port should be opened on the firewall and be publicly reachable. This should be kept to `false` for the majority of apps than only need a port for internal reverse-proxying! Possible values: `false`, `true`(=`Both`), `Both`, `TCP`, `UDP`. This will result in the port being opened on the firewall, and the diagnosis checking that a program answers on that port.  (FIXME: this is not implemented yet)
+    - `exposed`: (default: `false`) Wether this port should be opened on the firewall and be publicly reachable. This should be kept to `false` for the majority of apps than only need a port for internal reverse-proxying! Possible values: `false`, `true`(=`Both`), `Both`, `TCP`, `UDP`. This will result in the port being opened on the firewall, and the diagnosis checking that a program answers on that port.
-    - `fixed`: (default: `false`) Tells that the app absolutely needs the specific value provided in `default`, typically because it's needed for a specific protocol (FIXME: this is not implemented yet)
+    - `fixed`: (default: `false`) Tells that the app absolutely needs the specific value provided in `default`, typically because it's needed for a specific protocol
 
     ##### Provision/Update (for every port name):
     - If not already booked, look for a free port, starting with the `default` value (or a random value between 10000 and 60000 if no `default` set)
-    - (FIXME) If `exposed` is not `false`, open the port in the firewall accordingly - otherwise make sure it's closed.
+    - If `exposed` is not `false`, open the port in the firewall accordingly - otherwise make sure it's closed.
     - The value of the port is stored in the `$port` setting for the `main` port, or `$port_NAME` for other `NAME`s
 
     ##### Deprovision:
-    - (FIXME) Close the ports on the firewall
+    - Close the ports on the firewall if relevant
     - Deletes all the port settings
 
     ##### Legacy management:
@@ -784,8 +806,8 @@ class PortsResource(AppResource):
 
     default_port_properties = {
         "default": None,
-        "exposed": False,  # or True(="Both"), "TCP", "UDP"   # FIXME : implement logic for exposed port (allow/disallow in firewall ?)
+        "exposed": False,  # or True(="Both"), "TCP", "UDP"
-        "fixed": False,  # FIXME: implement logic. Corresponding to wether or not the port is "fixed" or any random port is ok
+        "fixed": False,
     }
 
     ports: Dict[str, Dict[str, Any]]
@@ -817,6 +839,8 @@ class PortsResource(AppResource):
 
     def provision_or_update(self, context: Dict = {}):
 
+        from yunohost.firewall import firewall_allow, firewall_disallow
+
         for name, infos in self.ports.items():
 
             setting_name = f"port_{name}" if name != "main" else "port"
@@ -832,16 +856,31 @@ class PortsResource(AppResource):
 
             if not port_value:
                 port_value = infos["default"]
-                while self._port_is_used(port_value):
+
-                    port_value += 1
+                if infos["fixed"]:
+                    if self._port_is_used(port_value):
+                        raise YunohostValidationError(f"Port {port_value} is already used by another process or app.")
+                else:
+                    while self._port_is_used(port_value):
+                        port_value += 1
 
             self.set_setting(setting_name, port_value)
 
+            if infos["exposed"]:
+                firewall_allow(infos["exposed"], port_value, reload_only_if_change=True)
+            else:
+                firewall_disallow(infos["exposed"], port_value, reload_only_if_change=True)
+
     def deprovision(self, context: Dict = {}):
 
+        from yunohost.firewall import firewall_disallow
+
         for name, infos in self.ports.items():
             setting_name = f"port_{name}" if name != "main" else "port"
+            value = self.get_setting(setting_name)
             self.delete_setting(setting_name)
+            if value and str(value).strip():
+                firewall_disallow(infos["exposed"], int(value), reload_only_if_change=True)
 
 
 class DatabaseAppResource(AppResource):
@@ -881,9 +920,10 @@ class DatabaseAppResource(AppResource):
 
     type = "database"
     priority = 90
+    dbtype: str = ""
 
     default_properties: Dict[str, Any] = {
-        "type": None,  # FIXME: eeeeeeeh is this really a good idea considering 'type' is supposed to be the resource type x_x
+        "dbtype": None,
     }
 
     def __init__(self, properties: Dict[str, Any], *args, **kwargs):
@@ -893,16 +933,22 @@ class DatabaseAppResource(AppResource):
             "postgresql",
         ]:
             raise YunohostError(
-                "Specifying the type of db ('mysql' or 'postgresql') is mandatory for db resources"
+                "Specifying the type of db ('mysql' or 'postgresql') is mandatory for db resources",
+                raw_msg=True
             )
 
+        # Hack so that people can write type = "mysql/postgresql" in toml but it's loaded as dbtype
+        # to avoid conflicting with the generic self.type of the resource object ...
+        # dunno if that's really a good idea :|
+        properties["dbtype"] = properties.pop("type")
+
         super().__init__(properties, *args, **kwargs)
 
     def db_exists(self, db_name):
 
-        if self.type == "mysql":
+        if self.dbtype == "mysql":
             return os.system(f"mysqlshow '{db_name}' >/dev/null 2>/dev/null") == 0
-        elif self.type == "postgresql":
+        elif self.dbtype == "postgresql":
             return (
                 os.system(
                     f"sudo --login --user=postgres psql -c '' '{db_name}' >/dev/null 2>/dev/null"
@@ -926,7 +972,7 @@ class DatabaseAppResource(AppResource):
         else:
             # Legacy setting migration
             legacypasswordsetting = (
-                "psqlpwd" if self.type == "postgresql" else "mysqlpwd"
+                "psqlpwd" if self.dbtype == "postgresql" else "mysqlpwd"
             )
             if self.get_setting(legacypasswordsetting):
                 db_pwd = self.get_setting(legacypasswordsetting)
@@ -941,12 +987,12 @@ class DatabaseAppResource(AppResource):
 
         if not self.db_exists(db_name):
 
-            if self.type == "mysql":
+            if self.dbtype == "mysql":
                 self._run_script(
                     "provision",
                     f"ynh_mysql_create_db '{db_name}' '{db_user}' '{db_pwd}'",
                 )
-            elif self.type == "postgresql":
+            elif self.dbtype == "postgresql":
                 self._run_script(
                     "provision",
                     f"ynh_psql_create_user '{db_user}' '{db_pwd}'; ynh_psql_create_db '{db_name}' '{db_user}'",
@@ -957,11 +1003,11 @@ class DatabaseAppResource(AppResource):
         db_name = self.app.replace("-", "_").replace(".", "_")
         db_user = db_name
 
-        if self.type == "mysql":
+        if self.dbtype == "mysql":
             self._run_script(
                 "deprovision", f"ynh_mysql_remove_db '{db_name}' '{db_user}'"
             )
-        elif self.type == "postgresql":
+        elif self.dbtype == "postgresql":
             self._run_script(
                 "deprovision", f"ynh_psql_remove_db '{db_name}' '{db_user}'"
             )

tests/test_helpers.d/ynhtest_setup_source.sh

@@ -18,51 +18,51 @@ _make_dummy_src() {
 }
 
 ynhtest_setup_source_nominal() {
-    final_path="$(mktemp -d -p $VAR_WWW)"
+    install_dir="$(mktemp -d -p $VAR_WWW)"
     _make_dummy_src > ../conf/dummy.src
     
-    ynh_setup_source --dest_dir="$final_path" --source_id="dummy"
+    ynh_setup_source --dest_dir="$install_dir" --source_id="dummy"
 
-    test -e "$final_path"
+    test -e "$install_dir"
-    test -e "$final_path/index.html"
+    test -e "$install_dir/index.html"
 }
 
 ynhtest_setup_source_nominal_upgrade() {
-    final_path="$(mktemp -d -p $VAR_WWW)"
+    install_dir="$(mktemp -d -p $VAR_WWW)"
     _make_dummy_src > ../conf/dummy.src
 
-    ynh_setup_source --dest_dir="$final_path" --source_id="dummy"
+    ynh_setup_source --dest_dir="$install_dir" --source_id="dummy"
 
-    test "$(cat $final_path/index.html)" == "Lorem Ipsum"
+    test "$(cat $install_dir/index.html)" == "Lorem Ipsum"
     
     # Except index.html to get overwritten during next ynh_setup_source
-    echo "IEditedYou!" > $final_path/index.html
+    echo "IEditedYou!" > $install_dir/index.html
-    test "$(cat $final_path/index.html)" == "IEditedYou!"
+    test "$(cat $install_dir/index.html)" == "IEditedYou!"
 
-    ynh_setup_source --dest_dir="$final_path" --source_id="dummy"
+    ynh_setup_source --dest_dir="$install_dir" --source_id="dummy"
 
-    test "$(cat $final_path/index.html)" == "Lorem Ipsum"
+    test "$(cat $install_dir/index.html)" == "Lorem Ipsum"
 }
 
 
 ynhtest_setup_source_with_keep() {
-    final_path="$(mktemp -d -p $VAR_WWW)"
+    install_dir="$(mktemp -d -p $VAR_WWW)"
     _make_dummy_src > ../conf/dummy.src
 
-    echo "IEditedYou!" > $final_path/index.html
+    echo "IEditedYou!" > $install_dir/index.html
-    echo "IEditedYou!" > $final_path/test.txt
+    echo "IEditedYou!" > $install_dir/test.txt
 
-    ynh_setup_source --dest_dir="$final_path" --source_id="dummy" --keep="index.html test.txt"
+    ynh_setup_source --dest_dir="$install_dir" --source_id="dummy" --keep="index.html test.txt"
 
-    test -e "$final_path"
+    test -e "$install_dir"
-    test -e "$final_path/index.html"
+    test -e "$install_dir/index.html"
-    test -e "$final_path/test.txt"
+    test -e "$install_dir/test.txt"
-    test "$(cat $final_path/index.html)" == "IEditedYou!"
+    test "$(cat $install_dir/index.html)" == "IEditedYou!"
-    test "$(cat $final_path/test.txt)" == "IEditedYou!"
+    test "$(cat $install_dir/test.txt)" == "IEditedYou!"
 }
 
 ynhtest_setup_source_with_patch() {
-    final_path="$(mktemp -d -p $VAR_WWW)"
+    install_dir="$(mktemp -d -p $VAR_WWW)"
     _make_dummy_src > ../conf/dummy.src
 
     mkdir -p ../sources/patches
@@ -74,7 +74,7 @@ ynhtest_setup_source_with_patch() {
 +Lorem Ipsum dolor sit amet
 EOF
 
-    ynh_setup_source --dest_dir="$final_path" --source_id="dummy"
+    ynh_setup_source --dest_dir="$install_dir" --source_id="dummy"
 
-    test "$(cat $final_path/index.html)" == "Lorem Ipsum dolor sit amet"
+    test "$(cat $install_dir/index.html)" == "Lorem Ipsum dolor sit amet"
 }