configpanel: reorder ConfigPanel methods

src/app.py

@@ -1878,13 +1878,13 @@ class AppConfigPanel(ConfigPanel):
     save_path_tpl = os.path.join(APPS_SETTING_PATH, "{entity}/settings.yml")
     config_path_tpl = os.path.join(APPS_SETTING_PATH, "{entity}/config_panel.toml")
 
-    def _load_current_values(self):
-        self.values = self._call_config_script("show")
-
     def _run_action(self, action):
         env = {key: str(value) for key, value in self.new_values.items()}
         self._call_config_script(action, env=env)
 
+    def _load_current_values(self):
+        self.values = self._call_config_script("show")
+
     def _apply(self):
         env = {key: str(value) for key, value in self.new_values.items()}
         return_content = self._call_config_script("apply", env=env)

src/domain.py

@@ -538,6 +538,83 @@ class DomainConfigPanel(ConfigPanel):
     save_path_tpl = f"{DOMAIN_SETTINGS_DIR}/{{entity}}.yml"
     save_mode = "diff"
 
+    def get(self, key="", mode="classic"):
+        result = super().get(key=key, mode=mode)
+
+        if mode == "full":
+            for panel, section, option in self._iterate():
+                # This injects:
+                # i18n: domain_config_cert_renew_help
+                # i18n: domain_config_default_app_help
+                # i18n: domain_config_xmpp_help
+                if m18n.key_exists(self.config["i18n"] + "_" + option["id"] + "_help"):
+                    option["help"] = m18n.n(
+                        self.config["i18n"] + "_" + option["id"] + "_help"
+                    )
+            return self.config
+
+        return result
+
+    def _get_toml(self):
+        toml = super()._get_toml()
+
+        toml["feature"]["xmpp"]["xmpp"]["default"] = (
+            1 if self.entity == _get_maindomain() else 0
+        )
+
+        # Optimize wether or not to load the DNS section,
+        # e.g. we don't want to trigger the whole _get_registary_config_section
+        # when just getting the current value from the feature section
+        filter_key = self.filter_key.split(".") if self.filter_key != "" else []
+        if not filter_key or filter_key[0] == "dns":
+            from yunohost.dns import _get_registrar_config_section
+
+            toml["dns"]["registrar"] = _get_registrar_config_section(self.entity)
+
+            # FIXME: Ugly hack to save the registar id/value and reinject it in _load_current_values ...
+            self.registar_id = toml["dns"]["registrar"]["registrar"]["value"]
+            del toml["dns"]["registrar"]["registrar"]["value"]
+
+        # Cert stuff
+        if not filter_key or filter_key[0] == "cert":
+            from yunohost.certificate import certificate_status
+
+            status = certificate_status([self.entity], full=True)["certificates"][
+                self.entity
+            ]
+
+            toml["cert"]["cert"]["cert_summary"]["style"] = status["style"]
+
+            # i18n: domain_config_cert_summary_expired
+            # i18n: domain_config_cert_summary_selfsigned
+            # i18n: domain_config_cert_summary_abouttoexpire
+            # i18n: domain_config_cert_summary_ok
+            # i18n: domain_config_cert_summary_letsencrypt
+            toml["cert"]["cert"]["cert_summary"]["ask"] = m18n.n(
+                f"domain_config_cert_summary_{status['summary']}"
+            )
+
+            # FIXME: Ugly hack to save the cert status and reinject it in _load_current_values ...
+            self.cert_status = status
+
+        return toml
+
+    def _load_current_values(self):
+        # TODO add mechanism to share some settings with other domains on the same zone
+        super()._load_current_values()
+
+        # FIXME: Ugly hack to save the registar id/value and reinject it in _load_current_values ...
+        filter_key = self.filter_key.split(".") if self.filter_key != "" else []
+        if not filter_key or filter_key[0] == "dns":
+            self.values["registrar"] = self.registar_id
+
+        # FIXME: Ugly hack to save the cert status and reinject it in _load_current_values ...
+        if not filter_key or filter_key[0] == "cert":
+            self.values["cert_validity"] = self.cert_status["validity"]
+            self.values["cert_issuer"] = self.cert_status["CA_type"]
+            self.values["acme_eligible"] = self.cert_status["ACME_eligible"]
+            self.values["summary"] = self.cert_status["summary"]
+
     def _apply(self):
         if (
             "default_app" in self.future_values
@@ -586,83 +663,6 @@ class DomainConfigPanel(ConfigPanel):
         if stuff_to_regen_conf:
             regen_conf(names=stuff_to_regen_conf)
 
-    def _get_toml(self):
-        toml = super()._get_toml()
-
-        toml["feature"]["xmpp"]["xmpp"]["default"] = (
-            1 if self.entity == _get_maindomain() else 0
-        )
-
-        # Optimize wether or not to load the DNS section,
-        # e.g. we don't want to trigger the whole _get_registary_config_section
-        # when just getting the current value from the feature section
-        filter_key = self.filter_key.split(".") if self.filter_key != "" else []
-        if not filter_key or filter_key[0] == "dns":
-            from yunohost.dns import _get_registrar_config_section
-
-            toml["dns"]["registrar"] = _get_registrar_config_section(self.entity)
-
-            # FIXME: Ugly hack to save the registar id/value and reinject it in _load_current_values ...
-            self.registar_id = toml["dns"]["registrar"]["registrar"]["value"]
-            del toml["dns"]["registrar"]["registrar"]["value"]
-
-        # Cert stuff
-        if not filter_key or filter_key[0] == "cert":
-            from yunohost.certificate import certificate_status
-
-            status = certificate_status([self.entity], full=True)["certificates"][
-                self.entity
-            ]
-
-            toml["cert"]["cert"]["cert_summary"]["style"] = status["style"]
-
-            # i18n: domain_config_cert_summary_expired
-            # i18n: domain_config_cert_summary_selfsigned
-            # i18n: domain_config_cert_summary_abouttoexpire
-            # i18n: domain_config_cert_summary_ok
-            # i18n: domain_config_cert_summary_letsencrypt
-            toml["cert"]["cert"]["cert_summary"]["ask"] = m18n.n(
-                f"domain_config_cert_summary_{status['summary']}"
-            )
-
-            # FIXME: Ugly hack to save the cert status and reinject it in _load_current_values ...
-            self.cert_status = status
-
-        return toml
-
-    def get(self, key="", mode="classic"):
-        result = super().get(key=key, mode=mode)
-
-        if mode == "full":
-            for panel, section, option in self._iterate():
-                # This injects:
-                # i18n: domain_config_cert_renew_help
-                # i18n: domain_config_default_app_help
-                # i18n: domain_config_xmpp_help
-                if m18n.key_exists(self.config["i18n"] + "_" + option["id"] + "_help"):
-                    option["help"] = m18n.n(
-                        self.config["i18n"] + "_" + option["id"] + "_help"
-                    )
-            return self.config
-
-        return result
-
-    def _load_current_values(self):
-        # TODO add mechanism to share some settings with other domains on the same zone
-        super()._load_current_values()
-
-        # FIXME: Ugly hack to save the registar id/value and reinject it in _load_current_values ...
-        filter_key = self.filter_key.split(".") if self.filter_key != "" else []
-        if not filter_key or filter_key[0] == "dns":
-            self.values["registrar"] = self.registar_id
-
-        # FIXME: Ugly hack to save the cert status and reinject it in _load_current_values ...
-        if not filter_key or filter_key[0] == "cert":
-            self.values["cert_validity"] = self.cert_status["validity"]
-            self.values["cert_issuer"] = self.cert_status["CA_type"]
-            self.values["acme_eligible"] = self.cert_status["ACME_eligible"]
-            self.values["summary"] = self.cert_status["summary"]
-
 
 def domain_action_run(domain, action, args=None):
     import urllib.parse

src/settings.py

@@ -125,83 +125,6 @@ class SettingsConfigPanel(ConfigPanel):
     def __init__(self, config_path=None, save_path=None, creation=False):
         super().__init__("settings")
 
-    def _apply(self):
-        root_password = self.new_values.pop("root_password", None)
-        root_password_confirm = self.new_values.pop("root_password_confirm", None)
-        passwordless_sudo = self.new_values.pop("passwordless_sudo", None)
-
-        self.values = {
-            k: v for k, v in self.values.items() if k not in self.virtual_settings
-        }
-        self.new_values = {
-            k: v for k, v in self.new_values.items() if k not in self.virtual_settings
-        }
-
-        assert all(v not in self.future_values for v in self.virtual_settings)
-
-        if root_password and root_password.strip():
-            if root_password != root_password_confirm:
-                raise YunohostValidationError("password_confirmation_not_the_same")
-
-            from yunohost.tools import tools_rootpw
-
-            tools_rootpw(root_password, check_strength=True)
-
-        if passwordless_sudo is not None:
-            from yunohost.utils.ldap import _get_ldap_interface
-
-            ldap = _get_ldap_interface()
-            ldap.update(
-                "cn=admins,ou=sudo",
-                {"sudoOption": ["!authenticate"] if passwordless_sudo else []},
-            )
-
-        super()._apply()
-
-        settings = {
-            k: v for k, v in self.future_values.items() if self.values.get(k) != v
-        }
-        for setting_name, value in settings.items():
-            try:
-                trigger_post_change_hook(
-                    setting_name, self.values.get(setting_name), value
-                )
-            except Exception as e:
-                logger.error(f"Post-change hook for setting failed : {e}")
-                raise
-
-    def _get_toml(self):
-        toml = super()._get_toml()
-
-        # Dynamic choice list for portal themes
-        THEMEDIR = "/usr/share/ssowat/portal/assets/themes/"
-        try:
-            themes = [d for d in os.listdir(THEMEDIR) if os.path.isdir(THEMEDIR + d)]
-        except Exception:
-            themes = ["unsplash", "vapor", "light", "default", "clouds"]
-        toml["misc"]["portal"]["portal_theme"]["choices"] = themes
-
-        return toml
-
-    def _load_current_values(self):
-        super()._load_current_values()
-
-        # Specific logic for those settings who are "virtual" settings
-        # and only meant to have a custom setter mapped to tools_rootpw
-        self.values["root_password"] = ""
-        self.values["root_password_confirm"] = ""
-
-        # Specific logic for virtual setting "passwordless_sudo"
-        try:
-            from yunohost.utils.ldap import _get_ldap_interface
-
-            ldap = _get_ldap_interface()
-            self.values["passwordless_sudo"] = "!authenticate" in ldap.search(
-                "ou=sudo", "cn=admins", ["sudoOption"]
-            )[0].get("sudoOption", [])
-        except Exception:
-            self.values["passwordless_sudo"] = False
-
     def get(self, key="", mode="classic"):
         result = super().get(key=key, mode=mode)
 
@@ -257,6 +180,83 @@ class SettingsConfigPanel(ConfigPanel):
         logger.success(m18n.n("global_settings_reset_success"))
         operation_logger.success()
 
+    def _get_toml(self):
+        toml = super()._get_toml()
+
+        # Dynamic choice list for portal themes
+        THEMEDIR = "/usr/share/ssowat/portal/assets/themes/"
+        try:
+            themes = [d for d in os.listdir(THEMEDIR) if os.path.isdir(THEMEDIR + d)]
+        except Exception:
+            themes = ["unsplash", "vapor", "light", "default", "clouds"]
+        toml["misc"]["portal"]["portal_theme"]["choices"] = themes
+
+        return toml
+
+    def _load_current_values(self):
+        super()._load_current_values()
+
+        # Specific logic for those settings who are "virtual" settings
+        # and only meant to have a custom setter mapped to tools_rootpw
+        self.values["root_password"] = ""
+        self.values["root_password_confirm"] = ""
+
+        # Specific logic for virtual setting "passwordless_sudo"
+        try:
+            from yunohost.utils.ldap import _get_ldap_interface
+
+            ldap = _get_ldap_interface()
+            self.values["passwordless_sudo"] = "!authenticate" in ldap.search(
+                "ou=sudo", "cn=admins", ["sudoOption"]
+            )[0].get("sudoOption", [])
+        except Exception:
+            self.values["passwordless_sudo"] = False
+
+    def _apply(self):
+        root_password = self.new_values.pop("root_password", None)
+        root_password_confirm = self.new_values.pop("root_password_confirm", None)
+        passwordless_sudo = self.new_values.pop("passwordless_sudo", None)
+
+        self.values = {
+            k: v for k, v in self.values.items() if k not in self.virtual_settings
+        }
+        self.new_values = {
+            k: v for k, v in self.new_values.items() if k not in self.virtual_settings
+        }
+
+        assert all(v not in self.future_values for v in self.virtual_settings)
+
+        if root_password and root_password.strip():
+            if root_password != root_password_confirm:
+                raise YunohostValidationError("password_confirmation_not_the_same")
+
+            from yunohost.tools import tools_rootpw
+
+            tools_rootpw(root_password, check_strength=True)
+
+        if passwordless_sudo is not None:
+            from yunohost.utils.ldap import _get_ldap_interface
+
+            ldap = _get_ldap_interface()
+            ldap.update(
+                "cn=admins,ou=sudo",
+                {"sudoOption": ["!authenticate"] if passwordless_sudo else []},
+            )
+
+        super()._apply()
+
+        settings = {
+            k: v for k, v in self.future_values.items() if self.values.get(k) != v
+        }
+        for setting_name, value in settings.items():
+            try:
+                trigger_post_change_hook(
+                    setting_name, self.values.get(setting_name), value
+                )
+            except Exception as e:
+                logger.error(f"Post-change hook for setting failed : {e}")
+                raise
+
 
 # Meant to be a dict of setting_name -> function to call
 post_change_hooks = {}

src/utils/configpanel.py

@@ -175,6 +175,68 @@ class ConfigPanel:
         else:
             return result
 
+    def set(
+        self, key=None, value=None, args=None, args_file=None, operation_logger=None
+    ):
+        self.filter_key = key or ""
+
+        # Read config panel toml
+        self._get_config_panel()
+
+        if not self.config:
+            raise YunohostValidationError("config_no_panel")
+
+        if (args is not None or args_file is not None) and value is not None:
+            raise YunohostValidationError(
+                "You should either provide a value, or a serie of args/args_file, but not both at the same time",
+                raw_msg=True,
+            )
+
+        if self.filter_key.count(".") != 2 and value is not None:
+            raise YunohostValidationError("config_cant_set_value_on_section")
+
+        # Import and parse pre-answered options
+        logger.debug("Import and parse pre-answered options")
+        self._parse_pre_answered(args, value, args_file)
+
+        # Read or get values and hydrate the config
+        self._load_current_values()
+        self._hydrate()
+        BaseOption.operation_logger = operation_logger
+        self._ask()
+
+        if operation_logger:
+            operation_logger.start()
+
+        try:
+            self._apply()
+        except YunohostError:
+            raise
+        # Script got manually interrupted ...
+        # N.B. : KeyboardInterrupt does not inherit from Exception
+        except (KeyboardInterrupt, EOFError):
+            error = m18n.n("operation_interrupted")
+            logger.error(m18n.n("config_apply_failed", error=error))
+            raise
+        # Something wrong happened in Yunohost's code (most probably hook_exec)
+        except Exception:
+            import traceback
+
+            error = m18n.n("unexpected_error", error="\n" + traceback.format_exc())
+            logger.error(m18n.n("config_apply_failed", error=error))
+            raise
+        finally:
+            # Delete files uploaded from API
+            # FIXME : this is currently done in the context of config panels,
+            # but could also happen in the context of app install ... (or anywhere else
+            # where we may parse args etc...)
+            FileOption.clean_upload_dirs()
+
+        self._reload_services()
+
+        logger.success("Config updated as expected")
+        operation_logger.success()
+
     def list_actions(self):
         actions = {}
 
@@ -248,68 +310,6 @@ class ConfigPanel:
         logger.success(f"Action {action_id} successful")
         operation_logger.success()
 
-    def set(
-        self, key=None, value=None, args=None, args_file=None, operation_logger=None
-    ):
-        self.filter_key = key or ""
-
-        # Read config panel toml
-        self._get_config_panel()
-
-        if not self.config:
-            raise YunohostValidationError("config_no_panel")
-
-        if (args is not None or args_file is not None) and value is not None:
-            raise YunohostValidationError(
-                "You should either provide a value, or a serie of args/args_file, but not both at the same time",
-                raw_msg=True,
-            )
-
-        if self.filter_key.count(".") != 2 and value is not None:
-            raise YunohostValidationError("config_cant_set_value_on_section")
-
-        # Import and parse pre-answered options
-        logger.debug("Import and parse pre-answered options")
-        self._parse_pre_answered(args, value, args_file)
-
-        # Read or get values and hydrate the config
-        self._load_current_values()
-        self._hydrate()
-        BaseOption.operation_logger = operation_logger
-        self._ask()
-
-        if operation_logger:
-            operation_logger.start()
-
-        try:
-            self._apply()
-        except YunohostError:
-            raise
-        # Script got manually interrupted ...
-        # N.B. : KeyboardInterrupt does not inherit from Exception
-        except (KeyboardInterrupt, EOFError):
-            error = m18n.n("operation_interrupted")
-            logger.error(m18n.n("config_apply_failed", error=error))
-            raise
-        # Something wrong happened in Yunohost's code (most probably hook_exec)
-        except Exception:
-            import traceback
-
-            error = m18n.n("unexpected_error", error="\n" + traceback.format_exc())
-            logger.error(m18n.n("config_apply_failed", error=error))
-            raise
-        finally:
-            # Delete files uploaded from API
-            # FIXME : this is currently done in the context of config panels,
-            # but could also happen in the context of app install ... (or anywhere else
-            # where we may parse args etc...)
-            FileOption.clean_upload_dirs()
-
-        self._reload_services()
-
-        logger.success("Config updated as expected")
-        operation_logger.success()
-
     def _get_toml(self):
         return read_toml(self.config_path)
 
@@ -488,6 +488,26 @@ class ConfigPanel:
 
         return self.config
 
+    def _get_default_values(self):
+        return {
+            option["id"]: option["default"]
+            for _, _, option in self._iterate()
+            if "default" in option
+        }
+
+    def _load_current_values(self):
+        """
+        Retrieve entries in YAML file
+        And set default values if needed
+        """
+
+        # Inject defaults if needed (using the magic .update() ;))
+        self.values = self._get_default_values()
+
+        # Retrieve entries in the YAML
+        if os.path.exists(self.save_path) and os.path.isfile(self.save_path):
+            self.values.update(read_yaml(self.save_path) or {})
+
     def _hydrate(self):
         # Hydrating config panel with current value
         for _, section, option in self._iterate():
@@ -604,13 +624,6 @@ class ConfigPanel:
                 }
             )
 
-    def _get_default_values(self):
-        return {
-            option["id"]: option["default"]
-            for _, _, option in self._iterate()
-            if "default" in option
-        }
-
     @property
     def future_values(self):
         return {**self.values, **self.new_values}
@@ -624,19 +637,6 @@ class ConfigPanel:
 
         return self.__dict__[name]
 
-    def _load_current_values(self):
-        """
-        Retrieve entries in YAML file
-        And set default values if needed
-        """
-
-        # Inject defaults if needed (using the magic .update() ;))
-        self.values = self._get_default_values()
-
-        # Retrieve entries in the YAML
-        if os.path.exists(self.save_path) and os.path.isfile(self.save_path):
-            self.values.update(read_yaml(self.save_path) or {})
-
     def _parse_pre_answered(self, args, value, args_file):
         args = urllib.parse.parse_qs(args or "", keep_blank_values=True)
         self.args = {key: ",".join(value_) for key, value_ in args.items()}