From f5c56db10ea6e27fae15c959b4ee9c09c16fef6f Mon Sep 17 00:00:00 2001 From: axolotle Date: Thu, 13 Apr 2023 20:11:03 +0200 Subject: [PATCH 01/50] form: use pydantic BaseModel in Options and add some validators --- src/utils/form.py | 470 +++++++++++++++++++++++++--------------------- 1 file changed, 261 insertions(+), 209 deletions(-) diff --git a/src/utils/form.py b/src/utils/form.py index f201f507b..8b47be430 100644 --- a/src/utils/form.py +++ b/src/utils/form.py @@ -17,6 +17,7 @@ # along with this program. If not, see . # import ast +import datetime import operator as op import os import re @@ -24,9 +25,18 @@ import shutil import tempfile import urllib.parse from enum import Enum -from typing import Any, Callable, Dict, List, Literal, Mapping, Optional, Union +from typing import Any, Callable, Dict, List, Literal, Mapping, Union from logging import getLogger +from pydantic import ( + BaseModel, + root_validator, + validator, +) +from pydantic.color import Color +from pydantic.networks import EmailStr, HttpUrl +from pydantic.types import FilePath + from moulinette import Moulinette, m18n from moulinette.interfaces.cli import colorize from moulinette.utils.filesystem import read_file, write_to_file @@ -36,7 +46,6 @@ from yunohost.utils.i18n import _value_for_locale logger = getLogger("yunohost.form") -Context = dict[str, Any] # ╭───────────────────────────────────────────────────────╮ # │ ┌─╴╷ ╷╭─┐╷ │ @@ -240,27 +249,58 @@ FORBIDDEN_READONLY_TYPES = { } -class BaseOption: - def __init__( - self, - question: Dict[str, Any], - ): - self.id = question["id"] - self.type = question.get("type", OptionType.string) - self.visible = question.get("visible", True) +Context = dict[str, Any] +Translation = Union[dict[str, str], str] +JSExpression = str +Values = dict[str, Any] - self.readonly = question.get("readonly", False) - if self.readonly and self.type in FORBIDDEN_READONLY_TYPES: - # FIXME i18n - raise YunohostError( - "config_forbidden_readonly_type", - type=self.type, - id=self.id, + +class Pattern(BaseModel): + regexp: str + error: Translation = "error_pattern" # FIXME add generic i18n key + + +class BaseOption(BaseModel): + type: OptionType + id: str + ask: Union[Translation, None] + readonly: bool = False + visible: Union[JSExpression, bool] = True + bind: Union[str, None] = None + + class Config: + arbitrary_types_allowed = True + use_enum_values = True + validate_assignment = True + + @staticmethod + def schema_extra(schema: dict[str, Any], model: Type["BaseOption"]) -> None: + # FIXME Do proper doctstring for Options + del schema["description"] + schema["additionalProperties"] = False + + @validator("ask", always=True) + def parse_or_set_default_ask( + cls, value: Union[Translation, None], values: Values + ) -> Translation: + if value is None: + return {"en": values["id"]} + if isinstance(value, str): + return {"en": value} + return value + + @validator("readonly", pre=True) + def can_be_readonly(cls, value: bool, values: Values) -> bool: + forbidden_types = ("password", "app", "domain", "user", "file") + if value is True and values["type"] in forbidden_types: + raise ValueError( + m18n.n( + "config_forbidden_readonly_type", + type=values["type"], + id=values["id"], + ) ) - - self.ask = question.get("ask", self.id) - if not isinstance(self.ask, dict): - self.ask = {"en": self.ask} + return value def is_visible(self, context: Context) -> bool: if isinstance(self.visible, bool): @@ -268,7 +308,7 @@ class BaseOption: return evaluate_simple_js_expression(self.visible, context=context) - def _get_prompt_message(self) -> str: + def _get_prompt_message(self, value: None) -> str: return _value_for_locale(self.ask) @@ -278,9 +318,7 @@ class BaseOption: class BaseReadonlyOption(BaseOption): - def __init__(self, question): - super().__init__(question) - self.readonly = True + readonly: Literal[True] = True class DisplayTextOption(BaseReadonlyOption): @@ -291,38 +329,35 @@ class MarkdownOption(BaseReadonlyOption): type: Literal[OptionType.markdown] = OptionType.markdown +class State(str, Enum): + success = "success" + info = "info" + warning = "warning" + danger = "danger" + + class AlertOption(BaseReadonlyOption): type: Literal[OptionType.alert] = OptionType.alert + style: State = State.info + icon: Union[str, None] = None - def __init__(self, question): - super().__init__(question) - self.style = question.get("style", "info") - - def _get_prompt_message(self) -> str: - text = _value_for_locale(self.ask) - - if self.style in ["success", "info", "warning", "danger"]: - color = { - "success": "green", - "info": "cyan", - "warning": "yellow", - "danger": "red", - } - prompt = m18n.g(self.style) if self.style != "danger" else m18n.n("danger") - return colorize(prompt, color[self.style]) + f" {text}" - else: - return text + def _get_prompt_message(self, value: None) -> str: + colors = { + State.success: "green", + State.info: "cyan", + State.warning: "yellow", + State.danger: "red", + } + message = m18n.g(self.style) if self.style != State.danger else m18n.n("danger") + return f"{colorize(message, colors[self.style])} {_value_for_locale(self.ask)}" class ButtonOption(BaseReadonlyOption): type: Literal[OptionType.button] = OptionType.button - enabled = True - - def __init__(self, question): - super().__init__(question) - self.help = question.get("help") - self.style = question.get("style", "success") - self.enabled = question.get("enabled", True) + help: Union[Translation, None] = None + style: State = State.success + icon: Union[str, None] = None + enabled: Union[JSExpression, bool] = True def is_enabled(self, context: Context) -> bool: if isinstance(self.enabled, bool): @@ -337,16 +372,21 @@ class ButtonOption(BaseReadonlyOption): class BaseInputOption(BaseOption): - hide_user_input_in_prompt = False - pattern: Optional[Dict] = None + help: Union[Translation, None] = None + example: Union[str, None] = None + placeholder: Union[str, None] = None + redact: bool = False + optional: bool = False # FIXME keep required as default? + default: Any = None - def __init__(self, question: Dict[str, Any]): - super().__init__(question) - self.default = question.get("default", None) - self.optional = question.get("optional", False) - self.pattern = question.get("pattern", self.pattern) - self.help = question.get("help") - self.redact = question.get("redact", False) + @validator("default", pre=True) + def check_empty_default(value: Any) -> Any: + if value == "": + return None + return value + + # FIXME remove + def old__init__(self, question: Dict[str, Any]): # .current_value is the currently stored value self.current_value = question.get("current_value") # .value is the "proposed" value which we got from the user @@ -354,10 +394,6 @@ class BaseInputOption(BaseOption): # Use to return several values in case answer is in mutipart self.values: Dict[str, Any] = {} - # Empty value is parsed as empty string - if self.default == "": - self.default = None - @staticmethod def humanize(value, option={}): return str(value) @@ -368,12 +404,12 @@ class BaseInputOption(BaseOption): value = value.strip() return value - def _get_prompt_message(self) -> str: - message = super()._get_prompt_message() + def _get_prompt_message(self, value: Any) -> str: + message = super()._get_prompt_message(value) if self.readonly: message = colorize(message, "purple") - return f"{message} {self.humanize(self.current_value)}" + return f"{message} {self.humanize(value, self)}" return message @@ -418,7 +454,8 @@ class BaseInputOption(BaseOption): class BaseStringOption(BaseInputOption): - default_value = "" + default: Union[str, None] + pattern: Union[Pattern, None] = None class StringOption(BaseStringOption): @@ -429,27 +466,23 @@ class TextOption(BaseStringOption): type: Literal[OptionType.text] = OptionType.text +FORBIDDEN_PASSWORD_CHARS = r"{}" + + class PasswordOption(BaseInputOption): type: Literal[OptionType.password] = OptionType.password - hide_user_input_in_prompt = True - default_value = "" - forbidden_chars = "{}" - - def __init__(self, question): - super().__init__(question) - self.redact = True - if self.default is not None: - raise YunohostValidationError( - "app_argument_password_no_default", name=self.id - ) + example: Literal[None] = None + default: Literal[None] = None + redact: Literal[True] = True + _forbidden_chars: str = FORBIDDEN_PASSWORD_CHARS def _value_pre_validator(self): super()._value_pre_validator() if self.value not in [None, ""]: - if any(char in self.value for char in self.forbidden_chars): + if any(char in self.value for char in self._forbidden_chars): raise YunohostValidationError( - "pattern_password_app", forbidden_chars=self.forbidden_chars + "pattern_password_app", forbidden_chars=self._forbidden_chars ) # If it's an optional argument the value should be empty or strong enough @@ -458,26 +491,25 @@ class PasswordOption(BaseInputOption): assert_password_is_strong_enough("user", self.value) -class ColorOption(BaseStringOption): +class ColorOption(BaseInputOption): type: Literal[OptionType.color] = OptionType.color - pattern = { - "regexp": r"^#[ABCDEFabcdef\d]{3,6}$", - "error": "config_validate_color", # i18n: config_validate_color - } + default: Union[str, None] + # pattern = { + # "regexp": r"^#[ABCDEFabcdef\d]{3,6}$", + # "error": "config_validate_color", # i18n: config_validate_color + # } # ─ NUMERIC ─────────────────────────────────────────────── class NumberOption(BaseInputOption): + # `number` and `range` are exactly the same, but `range` does render as a slider in web-admin type: Literal[OptionType.number, OptionType.range] = OptionType.number - default_value = None - - def __init__(self, question): - super().__init__(question) - self.min = question.get("min", None) - self.max = question.get("max", None) - self.step = question.get("step", None) + default: Union[int, None] + min: Union[int, None] = None + max: Union[int, None] = None + step: Union[int, None] = None @staticmethod def normalize(value, option={}): @@ -493,7 +525,7 @@ class NumberOption(BaseInputOption): if value in [None, ""]: return None - option = option.__dict__ if isinstance(option, BaseOption) else option + option = option.dict() if isinstance(option, BaseOption) else option raise YunohostValidationError( "app_argument_invalid", name=option.get("id"), @@ -525,20 +557,15 @@ class NumberOption(BaseInputOption): class BooleanOption(BaseInputOption): type: Literal[OptionType.boolean] = OptionType.boolean - default_value = 0 - yes_answers = ["1", "yes", "y", "true", "t", "on"] - no_answers = ["0", "no", "n", "false", "f", "off"] - - def __init__(self, question): - super().__init__(question) - self.yes = question.get("yes", 1) - self.no = question.get("no", 0) - if self.default is None: - self.default = self.no + yes: Any = 1 + no: Any = 0 + default: Union[bool, int, str, None] = 0 + _yes_answers: set[str] = {"1", "yes", "y", "true", "t", "on"} + _no_answers: set[str] = {"0", "no", "n", "false", "f", "off"} @staticmethod def humanize(value, option={}): - option = option.__dict__ if isinstance(option, BaseOption) else option + option = option.dict() if isinstance(option, BaseOption) else option yes = option.get("yes", 1) no = option.get("no", 0) @@ -561,7 +588,7 @@ class BooleanOption(BaseInputOption): @staticmethod def normalize(value, option={}): - option = option.__dict__ if isinstance(option, BaseOption) else option + option = option.dict() if isinstance(option, BaseOption) else option if isinstance(value, str): value = value.strip() @@ -569,8 +596,8 @@ class BooleanOption(BaseInputOption): technical_yes = option.get("yes", 1) technical_no = option.get("no", 0) - no_answers = BooleanOption.no_answers - yes_answers = BooleanOption.yes_answers + no_answers = BooleanOption._no_answers + yes_answers = BooleanOption._yes_answers assert ( str(technical_yes).lower() not in no_answers @@ -579,8 +606,8 @@ class BooleanOption(BaseInputOption): str(technical_no).lower() not in yes_answers ), f"'no' value can't be in {yes_answers}" - no_answers += [str(technical_no).lower()] - yes_answers += [str(technical_yes).lower()] + no_answers.add(str(technical_no).lower()) + yes_answers.add(str(technical_yes).lower()) strvalue = str(value).lower() @@ -602,8 +629,8 @@ class BooleanOption(BaseInputOption): def get(self, key, default=None): return getattr(self, key, default) - def _get_prompt_message(self): - message = super()._get_prompt_message() + def _get_prompt_message(self, value: Union[bool, None]) -> str: + message = super()._get_prompt_message(value) if not self.readonly: message += " [yes | no]" @@ -614,12 +641,13 @@ class BooleanOption(BaseInputOption): # ─ TIME ────────────────────────────────────────────────── -class DateOption(BaseStringOption): +class DateOption(BaseInputOption): type: Literal[OptionType.date] = OptionType.date - pattern = { - "regexp": r"^\d{4}-\d\d-\d\d$", - "error": "config_validate_date", # i18n: config_validate_date - } + default: Union[str, None] + # pattern = { + # "regexp": r"^\d{4}-\d\d-\d\d$", + # "error": "config_validate_date", # i18n: config_validate_date + # } def _value_pre_validator(self): from datetime import datetime @@ -633,32 +661,34 @@ class DateOption(BaseStringOption): raise YunohostValidationError("config_validate_date") -class TimeOption(BaseStringOption): +class TimeOption(BaseInputOption): type: Literal[OptionType.time] = OptionType.time - pattern = { - "regexp": r"^(?:\d|[01]\d|2[0-3]):[0-5]\d$", - "error": "config_validate_time", # i18n: config_validate_time - } + default: Union[str, int, None] + # pattern = { + # "regexp": r"^(?:\d|[01]\d|2[0-3]):[0-5]\d$", + # "error": "config_validate_time", # i18n: config_validate_time + # } # ─ LOCATIONS ───────────────────────────────────────────── -class EmailOption(BaseStringOption): +class EmailOption(BaseInputOption): type: Literal[OptionType.email] = OptionType.email - pattern = { - "regexp": r"^.+@.+", - "error": "config_validate_email", # i18n: config_validate_email - } + default: Union[EmailStr, None] + # pattern = { + # "regexp": r"^.+@.+", + # "error": "config_validate_email", # i18n: config_validate_email + # } class WebPathOption(BaseInputOption): type: Literal[OptionType.path] = OptionType.path - default_value = "" + default: Union[str, None] @staticmethod def normalize(value, option={}): - option = option.__dict__ if isinstance(option, BaseOption) else option + option = option.dict() if isinstance(option, BaseOption) else option if not isinstance(value, str): raise YunohostValidationError( @@ -685,10 +715,11 @@ class WebPathOption(BaseInputOption): class URLOption(BaseStringOption): type: Literal[OptionType.url] = OptionType.url - pattern = { - "regexp": r"^https?://.*$", - "error": "config_validate_url", # i18n: config_validate_url - } + default: Union[str, None] + # pattern = { + # "regexp": r"^https?://.*$", + # "error": "config_validate_url", # i18n: config_validate_url + # } # ─ FILE ────────────────────────────────────────────────── @@ -696,16 +727,16 @@ class URLOption(BaseStringOption): class FileOption(BaseInputOption): type: Literal[OptionType.file] = OptionType.file - upload_dirs: List[str] = [] - - def __init__(self, question): - super().__init__(question) - self.accept = question.get("accept", "") + # `FilePath` for CLI (path must exists and must be a file) + # `bytes` for API (a base64 encoded file actually) + accept: Union[str, None] = "" # currently only used by the web-admin + default: Union[str, None] + _upload_dirs: set[str] = set() @classmethod def clean_upload_dirs(cls): # Delete files uploaded from API - for upload_dir in cls.upload_dirs: + for upload_dir in cls._upload_dirs: if os.path.exists(upload_dir): shutil.rmtree(upload_dir) @@ -738,7 +769,7 @@ class FileOption(BaseInputOption): upload_dir = tempfile.mkdtemp(prefix="ynh_filequestion_") _, file_path = tempfile.mkstemp(dir=upload_dir) - FileOption.upload_dirs += [upload_dir] + FileOption._upload_dirs.add(upload_dir) logger.debug(f"Saving file {self.id} for file question into {file_path}") @@ -760,26 +791,30 @@ class FileOption(BaseInputOption): # ─ CHOICES ─────────────────────────────────────────────── -class BaseChoicesOption(BaseInputOption): - def __init__( - self, - question: Dict[str, Any], - ): - super().__init__(question) - # Don't restrict choices if there's none specified - self.choices = question.get("choices", None) +ChoosableOptions = Literal[ + OptionType.string, + OptionType.color, + OptionType.number, + OptionType.date, + OptionType.time, + OptionType.email, + OptionType.path, + OptionType.url, +] - def _get_prompt_message(self) -> str: - message = super()._get_prompt_message() + +class BaseChoicesOption(BaseInputOption): + # FIXME probably forbid choices to be None? + choices: Union[dict[str, Any], list[Any], None] + + def _get_prompt_message(self, value: Any) -> str: + message = super()._get_prompt_message(value) if self.readonly: - message = message - choice = self.current_value + if isinstance(self.choices, dict) and value is not None: + value = self.choices[value] - if isinstance(self.choices, dict) and choice is not None: - choice = self.choices[choice] - - return f"{colorize(message, 'purple')} {choice}" + return f"{colorize(message, 'purple')} {value}" if self.choices: # Prevent displaying a shitload of choices @@ -789,17 +824,15 @@ class BaseChoicesOption(BaseInputOption): if isinstance(self.choices, dict) else self.choices ) - choices_to_display = choices[:20] + splitted_choices = choices[:20] remaining_choices = len(choices[20:]) if remaining_choices > 0: - choices_to_display += [ + splitted_choices += [ m18n.n("other_available_options", n=remaining_choices) ] - choices_to_display = " | ".join( - str(choice) for choice in choices_to_display - ) + choices_to_display = " | ".join(str(choice) for choice in splitted_choices) return f"{message} [{choices_to_display}]" @@ -821,12 +854,15 @@ class BaseChoicesOption(BaseInputOption): class SelectOption(BaseChoicesOption): type: Literal[OptionType.select] = OptionType.select - default_value = "" + choices: Union[dict[str, Any], list[Any]] + default: Union[str, None] class TagsOption(BaseChoicesOption): type: Literal[OptionType.tags] = OptionType.tags - default_value = "" + choices: Union[list[str], None] = None + pattern: Union[Pattern, None] = None + default: Union[str, list[str], None] @staticmethod def humanize(value, option={}): @@ -879,20 +915,24 @@ class TagsOption(BaseChoicesOption): class DomainOption(BaseChoicesOption): type: Literal[OptionType.domain] = OptionType.domain + choices: Union[dict[str, str], None] - def __init__(self, question): - from yunohost.domain import domain_list, _get_maindomain + @root_validator() + def inject_domains_choices_and_default(cls, values: Values) -> Values: + # TODO remove calls to resources in validators (pydantic V2 should adress this) + from yunohost.domain import domain_list - super().__init__(question) - - if self.default is None: - self.default = _get_maindomain() - - self.choices = { - domain: domain + " ★" if domain == self.default else domain - for domain in domain_list()["domains"] + data = domain_list() + values["choices"] = { + domain: domain + " ★" if domain == data["main"] else domain + for domain in data["domains"] } + if values["default"] is None: + values["default"] = data["main"] + + return values + @staticmethod def normalize(value, option={}): if value.startswith("https://"): @@ -908,87 +948,99 @@ class DomainOption(BaseChoicesOption): class AppOption(BaseChoicesOption): type: Literal[OptionType.app] = OptionType.app + choices: Union[dict[str, str], None] + add_yunohost_portal_to_choices: bool = False + filter: Union[str, None] = None - def __init__(self, question): + @root_validator() + def inject_apps_choices(cls, values: Values) -> Values: from yunohost.app import app_list - super().__init__(question) - self.filter = question.get("filter", None) - self.add_yunohost_portal_to_choices = question.get("add_yunohost_portal_to_choices", False) - apps = app_list(full=True)["apps"] - if self.filter: + if values.get("filter", None): apps = [ app for app in apps - if evaluate_simple_js_expression(self.filter, context=app) + if evaluate_simple_js_expression(values["filter"], context=app) ] + values["choices"] = {"_none": "---"} - def _app_display(app): - domain_path_or_id = f" ({app.get('domain_path', app['id'])})" - return app["label"] + domain_path_or_id + if values.get("add_yunohost_portal_to_choices", False): + values["choices"]["_yunohost_portal_with_public_apps"] = "YunoHost's portal with public apps" - self.choices = {"_none": "---"} - if self.add_yunohost_portal_to_choices: - # FIXME: i18n - self.choices["_yunohost_portal_with_public_apps"] = "YunoHost's portal with public apps" - self.choices.update({app["id"]: _app_display(app) for app in apps}) + values["choices"].update( + { + app["id"]: f"{app['label']} ({app.get('domain_path', app['id'])})" + for app in apps + } + ) + + return values class UserOption(BaseChoicesOption): type: Literal[OptionType.user] = OptionType.user + choices: Union[dict[str, str], None] - def __init__(self, question): - from yunohost.user import user_list, user_info + @root_validator() + def inject_users_choices_and_default(cls, values: dict[str, Any]) -> dict[str, Any]: from yunohost.domain import _get_maindomain + from yunohost.user import user_info, user_list - super().__init__(question) - - self.choices = { + values["choices"] = { username: f"{infos['fullname']} ({infos['mail']})" for username, infos in user_list()["users"].items() } - if not self.choices: + # FIXME keep this to test if any user, do not raise error if no admin? + if not values["choices"]: raise YunohostValidationError( "app_argument_invalid", - name=self.id, + name=values["id"], error="You should create a YunoHost user first.", ) - if self.default is None: + if values["default"] is None: # FIXME: this code is obsolete with the new admins group # Should be replaced by something like "any first user we find in the admin group" root_mail = "root@%s" % _get_maindomain() - for user in self.choices.keys(): + for user in values["choices"].keys(): if root_mail in user_info(user).get("mail-aliases", []): - self.default = user + values["default"] = user break + return values + class GroupOption(BaseChoicesOption): type: Literal[OptionType.group] = OptionType.group + choices: Union[dict[str, str], None] - def __init__(self, question): + @root_validator() + def inject_groups_choices_and_default(cls, values: Values) -> Values: from yunohost.user import user_group_list - super().__init__(question) + groups = user_group_list(short=True, include_primary_groups=False)["groups"] - self.choices = list( - user_group_list(short=True, include_primary_groups=False)["groups"] - ) - - def _human_readable_group(g): + def _human_readable_group(groupname): # i18n: visitors # i18n: all_users # i18n: admins - return m18n.n(g) if g in ["visitors", "all_users", "admins"] else g + return ( + m18n.n(groupname) + if groupname in ["visitors", "all_users", "admins"] + else groupname + ) - self.choices = {g: _human_readable_group(g) for g in self.choices} + values["choices"] = { + groupname: _human_readable_group(groupname) for groupname in groups + } - if self.default is None: - self.default = "all_users" + if values["default"] is None: + values["default"] = "all_users" + + return values OPTIONS = { @@ -997,7 +1049,7 @@ OPTIONS = { OptionType.alert: AlertOption, OptionType.button: ButtonOption, OptionType.string: StringOption, - OptionType.text: StringOption, + OptionType.text: TextOption, OptionType.password: PasswordOption, OptionType.color: ColorOption, OptionType.number: NumberOption, From 89ae5e654de8104a0c2fe85b5582078c755c13d3 Mon Sep 17 00:00:00 2001 From: axolotle Date: Mon, 17 Apr 2023 15:06:53 +0200 Subject: [PATCH 02/50] form: update asking flow, separate form and options --- src/tests/test_questions.py | 32 ++-- src/utils/form.py | 297 +++++++++++++++++++++++++----------- 2 files changed, 225 insertions(+), 104 deletions(-) diff --git a/src/tests/test_questions.py b/src/tests/test_questions.py index a695e834d..9eceedcaf 100644 --- a/src/tests/test_questions.py +++ b/src/tests/test_questions.py @@ -11,11 +11,11 @@ from typing import Any, Literal, Sequence, TypedDict, Union from _pytest.mark.structures import ParameterSet - from moulinette import Moulinette from yunohost import app, domain, user from yunohost.utils.form import ( OPTIONS, + FORBIDDEN_PASSWORD_CHARS, ask_questions_and_parse_answers, BaseChoicesOption, BaseInputOption, @@ -378,8 +378,8 @@ def _fill_or_prompt_one_option(raw_option, intake): options = {id_: raw_option} answers = {id_: intake} if intake is not None else {} - option = ask_questions_and_parse_answers(options, answers)[0] - return (option, option.value if isinstance(option, BaseInputOption) else None) + options, form = ask_questions_and_parse_answers(options, answers) + return (options[0], form[id_] if isinstance(options[0], BaseInputOption) else None) def _test_value_is_expected_output(value, expected_output): @@ -551,7 +551,7 @@ class TestDisplayText(BaseTest): ask_questions_and_parse_answers({_id: raw_option}, answers) else: with patch.object(sys, "stdout", new_callable=StringIO) as stdout: - options = ask_questions_and_parse_answers( + options, form = ask_questions_and_parse_answers( {_id: raw_option}, answers ) assert stdout.getvalue() == f"{options[0].ask['en']}\n" @@ -604,7 +604,7 @@ class TestAlert(TestDisplayText): ) else: with patch.object(sys, "stdout", new_callable=StringIO) as stdout: - options = ask_questions_and_parse_answers( + options, form = ask_questions_and_parse_answers( {"display_text_id": raw_option}, answers ) ask = options[0].ask["en"] @@ -1925,9 +1925,7 @@ def test_options_query_string(): "&fake_id=fake_value" ) - def _assert_correct_values(options, raw_options): - form = {option.id: option.value for option in options} - + def _assert_correct_values(options, form, raw_options): for k, v in results.items(): if k == "file_id": assert os.path.exists(form["file_id"]) and os.path.isfile( @@ -1943,24 +1941,24 @@ def test_options_query_string(): with patch_interface("api"), patch_file_api(file_content1) as b64content: with patch_query_string(b64content.decode("utf-8")) as query_string: - options = ask_questions_and_parse_answers(raw_options, query_string) - _assert_correct_values(options, raw_options) + options, form = ask_questions_and_parse_answers(raw_options, query_string) + _assert_correct_values(options, form, raw_options) with patch_interface("cli"), patch_file_cli(file_content1) as filepath: with patch_query_string(filepath) as query_string: - options = ask_questions_and_parse_answers(raw_options, query_string) - _assert_correct_values(options, raw_options) + options, form = ask_questions_and_parse_answers(raw_options, query_string) + _assert_correct_values(options, form, raw_options) def test_question_string_default_type(): questions = {"some_string": {}} answers = {"some_string": "some_value"} - out = ask_questions_and_parse_answers(questions, answers)[0] - - assert out.id == "some_string" - assert out.type == "string" - assert out.value == "some_value" + options, form = ask_questions_and_parse_answers(questions, answers) + option = options[0] + assert option.id == "some_string" + assert option.type == "string" + assert form[option.id] == "some_value" def test_option_default_type_with_choices_is_select(): diff --git a/src/utils/form.py b/src/utils/form.py index 8b47be430..8a2a34f4b 100644 --- a/src/utils/form.py +++ b/src/utils/form.py @@ -25,17 +25,30 @@ import shutil import tempfile import urllib.parse from enum import Enum -from typing import Any, Callable, Dict, List, Literal, Mapping, Union from logging import getLogger +from typing import ( + Annotated, + Any, + Callable, + List, + Literal, + Mapping, + Type, + Union, + cast, +) from pydantic import ( BaseModel, + Extra, + ValidationError, + create_model, root_validator, validator, ) from pydantic.color import Color +from pydantic.fields import Field from pydantic.networks import EmailStr, HttpUrl -from pydantic.types import FilePath from moulinette import Moulinette, m18n from moulinette.interfaces.cli import colorize @@ -385,15 +398,6 @@ class BaseInputOption(BaseOption): return None return value - # FIXME remove - def old__init__(self, question: Dict[str, Any]): - # .current_value is the currently stored value - self.current_value = question.get("current_value") - # .value is the "proposed" value which we got from the user - self.value = question.get("value") - # Use to return several values in case answer is in mutipart - self.values: Dict[str, Any] = {} - @staticmethod def humanize(value, option={}): return str(value) @@ -650,8 +654,6 @@ class DateOption(BaseInputOption): # } def _value_pre_validator(self): - from datetime import datetime - super()._value_pre_validator() if self.value not in [None, ""]: @@ -1069,6 +1071,120 @@ OPTIONS = { OptionType.group: GroupOption, } +AnyOption = Union[ + DisplayTextOption, + MarkdownOption, + AlertOption, + ButtonOption, + StringOption, + TextOption, + PasswordOption, + ColorOption, + NumberOption, + BooleanOption, + DateOption, + TimeOption, + EmailOption, + WebPathOption, + URLOption, + FileOption, + SelectOption, + TagsOption, + DomainOption, + AppOption, + UserOption, + GroupOption, +] + + +# ╭───────────────────────────────────────────────────────╮ +# │ ┌─╴╭─╮┌─╮╭╮╮ │ +# │ ├─╴│ │├┬╯│││ │ +# │ ╵ ╰─╯╵ ╰╵╵╵ │ +# ╰───────────────────────────────────────────────────────╯ + + +class OptionsModel(BaseModel): + # Pydantic will match option types to their models class based on the "type" attribute + options: list[Annotated[AnyOption, Field(discriminator="type")]] + + @staticmethod + def options_dict_to_list(options: dict[str, Any], defaults: dict[str, Any] = {}): + return [ + option + | { + "id": id_, + "type": option.get("type", "string"), + } + for id_, option in options.items() + ] + + def __init__(self, **kwargs) -> None: + super().__init__(options=self.options_dict_to_list(kwargs)) + + +class FormModel(BaseModel): + """ + Base form on which dynamic forms are built upon Options. + """ + + class Config: + validate_assignment = True + extra = Extra.ignore + + def __getitem__(self, name: str): + # FIXME + # if a FormModel's required field is not instancied with a value, it is + # not available as an attr and therefor triggers an `AttributeError` + # Also since `BaseReadonlyOption`s do not end up in form, + # `form[AlertOption.id]` would also triggers an error + # For convinience in those 2 cases, we return `None` + if not hasattr(self, name): + # Return None to trigger a validation error instead for required fields + return None + + return getattr(self, name) + + def __setitem__(self, name: str, value: Any): + setattr(self, name, value) + + def get(self, attr: str, default: Any = None) -> Any: + try: + return getattr(self, attr) + except AttributeError: + return default + + +def build_form(options: list[AnyOption], name: str = "DynamicForm") -> Type[FormModel]: + """ + Returns a dynamic pydantic model class that can be used as a form. + Parsing/validation occurs at instanciation and assignements. + To avoid validation at instanciation, use `my_form.construct(**values)` + """ + options_as_fields: Any = {} + validators: dict[str, Any] = {} + + for option in options: + if not isinstance(option, BaseInputOption): + continue # filter out non input options + + options_as_fields[option.id] = option._as_dynamic_model_field() + + for step in ("pre", "post"): + validators[f"{option.id}_{step}_validator"] = validator( + option.id, allow_reuse=True, pre=step == "pre" + )(getattr(option, f"_value_{step}_validator")) + + return cast( + Type[FormModel], + create_model( + name, + __base__=FormModel, + __validators__=validators, + **options_as_fields, + ), + ) + def hydrate_option_type(raw_option: dict[str, Any]) -> dict[str, Any]: type_ = raw_option.get( @@ -1097,20 +1213,16 @@ Hooks = dict[str, Callable[[BaseInputOption], Any]] def prompt_or_validate_form( - raw_options: dict[str, Any], + options: list[AnyOption], + form: FormModel, prefilled_answers: dict[str, Any] = {}, context: Context = {}, hooks: Hooks = {}, -) -> list[BaseOption]: - options = [] +) -> FormModel: answers = {**prefilled_answers} + values = {} - for id_, raw_option in raw_options.items(): - raw_option["id"] = id_ - raw_option["value"] = answers.get(id_) - raw_option = hydrate_option_type(raw_option) - option = OPTIONS[raw_option["type"]](raw_option) - + for option in options: interactive = Moulinette.interface.type == "cli" and os.isatty(1) if isinstance(option, ButtonOption): @@ -1123,89 +1235,88 @@ def prompt_or_validate_form( help=_value_for_locale(option.help), ) - # FIXME not sure why we do not append Buttons to returned options - options.append(option) - if not option.is_visible(context): if isinstance(option, BaseInputOption): # FIXME There could be several use case if the question is not displayed: # - we doesn't want to give a specific value # - we want to keep the previous value # - we want the default value - option.value = context[option.id] = None + context[option.id] = form[option.id] = None continue - message = option._get_prompt_message() - - if option.readonly: - if interactive: - Moulinette.display(message) + # if we try to get a `BaseReadonlyOption` value, which doesn't exists in the form, + # we get `None` + value = form[option.id] + if isinstance(option, BaseReadonlyOption) or option.readonly: if isinstance(option, BaseInputOption): - option.value = context[option.id] = option.current_value + # FIXME normalized needed, form[option.id] should already be normalized + # only update the context with the value + context[option.id] = form[option.id] + + # FIXME here we could error out + if option.id in prefilled_answers: + logger.warning( + f"'{option.id}' is readonly, value '{prefilled_answers[option.id]}' is then ignored." + ) + + if interactive: + Moulinette.display(option._get_prompt_message(value)) continue - if isinstance(option, BaseInputOption): - for i in range(5): - if interactive and option.value is None: - prefill = "" - choices = ( - option.choices if isinstance(option, BaseChoicesOption) else [] - ) + for i in range(5): + if option.id in prefilled_answers: + value = prefilled_answers[option.id] + elif interactive: + value = option.humanize(value, option) + choices = ( + option.choices if isinstance(option, BaseChoicesOption) else [] + ) + value = Moulinette.prompt( + message=option._get_prompt_message(value), + is_password=isinstance(option, PasswordOption), + confirm=False, + prefill=value, + is_multiline=isinstance(option, TextOption), + autocomplete=choices, + help=_value_for_locale(option.help), + ) - if option.current_value is not None: - prefill = option.humanize(option.current_value, option) - elif option.default is not None: - prefill = option.humanize(option.default, option) + # Apply default value if none + if value is None or value == "" and option.default is not None: + value = option.default - option.value = Moulinette.prompt( - message=message, - is_password=isinstance(option, PasswordOption), - confirm=False, - prefill=prefill, - is_multiline=(option.type == "text"), - autocomplete=choices, - help=_value_for_locale(option.help), - ) + try: + # Normalize and validate + values[option.id] = form[option.id] = option.normalize(value, option) + except (ValidationError, YunohostValidationError) as e: + # If in interactive cli, re-ask the current question + if i < 4 and interactive: + logger.error(str(e)) + value = None + continue - # Apply default value - class_default = getattr(option, "default_value", None) - if option.value in [None, ""] and ( - option.default is not None or class_default is not None - ): - option.value = ( - class_default if option.default is None else option.default - ) + if isinstance(e, ValidationError): + error = "\n".join([err["msg"] for err in e.errors()]) + raise YunohostValidationError(error, raw_msg=True) - try: - # Normalize and validate - option.value = option.normalize(option.value, option) - option._value_pre_validator() - except YunohostValidationError as e: - # If in interactive cli, re-ask the current question - if i < 4 and interactive: - logger.error(str(e)) - option.value = None - continue + # Otherwise raise the ValidationError + raise e - # Otherwise raise the ValidationError - raise + break - break + # Search for post actions in hooks + post_hook = f"post_ask__{option.id}" + if post_hook in hooks: + values.update(hooks[post_hook](option)) + # FIXME reapply new values to form to validate it - option.value = option.values[option.id] = option._value_post_validator() + answers.update(values) + context.update(values) - # Search for post actions in hooks - post_hook = f"post_ask__{option.id}" - if post_hook in hooks: - option.values.update(hooks[post_hook](option)) - - answers.update(option.values) - context.update(option.values) - - return options + return form def ask_questions_and_parse_answers( @@ -1213,7 +1324,7 @@ def ask_questions_and_parse_answers( prefilled_answers: Union[str, Mapping[str, Any]] = {}, current_values: Mapping[str, Any] = {}, hooks: Hooks = {}, -) -> list[BaseOption]: +) -> tuple[list[AnyOption], FormModel]: """Parse arguments store in either manifest.json or actions.json or from a config panel against the user answers when they are present. @@ -1241,9 +1352,21 @@ def ask_questions_and_parse_answers( context = {**current_values, **answers} - return prompt_or_validate_form( - raw_options, prefilled_answers=answers, context=context, hooks=hooks + # Validate/parse the options attributes + try: + model = OptionsModel(**raw_options) + except ValidationError as e: + error = "\n".join([err["msg"] for err in e.errors()]) + # FIXME use YunohostError instead since it is not really a user mistake? + raise YunohostValidationError(error, raw_msg=True) + + # Build the form from those questions and instantiate it without + # parsing/validation (construct) since it may contains required questions. + form = build_form(model.options).construct() + form = prompt_or_validate_form( + model.options, form, prefilled_answers=answers, context=context, hooks=hooks ) + return (model.options, form) def hydrate_questions_with_choices(raw_questions: List) -> List: @@ -1251,7 +1374,7 @@ def hydrate_questions_with_choices(raw_questions: List) -> List: for raw_question in raw_questions: raw_question = hydrate_option_type(raw_question) - question = OPTIONS[raw_question["type"]](raw_question) + question = OPTIONS[raw_question["type"]](**raw_question) if isinstance(question, BaseChoicesOption) and question.choices: raw_question["choices"] = question.choices raw_question["default"] = question.default From 39437748111fc850003ccdb305f71876e49c2f6e Mon Sep 17 00:00:00 2001 From: axolotle Date: Mon, 17 Apr 2023 15:23:05 +0200 Subject: [PATCH 03/50] form: add dynamic annotation getters --- src/utils/form.py | 161 ++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 140 insertions(+), 21 deletions(-) diff --git a/src/utils/form.py b/src/utils/form.py index 8a2a34f4b..17b0a9432 100644 --- a/src/utils/form.py +++ b/src/utils/form.py @@ -27,6 +27,8 @@ import urllib.parse from enum import Enum from logging import getLogger from typing import ( + TYPE_CHECKING, + cast, Annotated, Any, Callable, @@ -35,7 +37,6 @@ from typing import ( Mapping, Type, Union, - cast, ) from pydantic import ( @@ -49,6 +50,7 @@ from pydantic import ( from pydantic.color import Color from pydantic.fields import Field from pydantic.networks import EmailStr, HttpUrl +from pydantic.types import constr from moulinette import Moulinette, m18n from moulinette.interfaces.cli import colorize @@ -57,6 +59,9 @@ from yunohost.log import OperationLogger from yunohost.utils.error import YunohostError, YunohostValidationError from yunohost.utils.i18n import _value_for_locale +if TYPE_CHECKING: + from pydantic.fields import FieldInfo + logger = getLogger("yunohost.form") @@ -391,6 +396,7 @@ class BaseInputOption(BaseOption): redact: bool = False optional: bool = False # FIXME keep required as default? default: Any = None + _annotation = Any @validator("default", pre=True) def check_empty_default(value: Any) -> Any: @@ -408,6 +414,57 @@ class BaseInputOption(BaseOption): value = value.strip() return value + @property + def _dynamic_annotation(self) -> Any: + """ + Returns the expected type of an Option's value. + This may be dynamic based on constraints. + """ + return self._annotation + + def _get_field_attrs(self) -> dict[str, Any]: + """ + Returns attributes to build a `pydantic.Field`. + This may contains non `Field` attrs that will end up in `Field.extra`. + Those extra can be used as constraints in custom validators and ends up + in the JSON Schema. + """ + # TODO + # - help + # - placeholder + attrs: dict[str, Any] = { + "redact": self.redact, # extra + "none_as_empty_str": self._none_as_empty_str, + } + + if self.readonly: + attrs["allow_mutation"] = False + + if self.example: + attrs["examples"] = [self.example] + + if self.default is not None: + attrs["default"] = self.default + else: + attrs["default"] = ... if not self.optional else None + + return attrs + + def _as_dynamic_model_field(self) -> tuple[Any, "FieldInfo"]: + """ + Return a tuple of a type and a Field instance to be injected in a + custom form declaration. + """ + attrs = self._get_field_attrs() + anno = ( + self._dynamic_annotation + if not self.optional + else Union[self._dynamic_annotation, None] + ) + field = Field(default=attrs.pop("default", None), **attrs) + + return (anno, field) + def _get_prompt_message(self, value: Any) -> str: message = super()._get_prompt_message(value) @@ -460,6 +517,22 @@ class BaseInputOption(BaseOption): class BaseStringOption(BaseInputOption): default: Union[str, None] pattern: Union[Pattern, None] = None + _annotation = str + + @property + def _dynamic_annotation(self) -> Type[str]: + if self.pattern: + return constr(regex=self.pattern.regexp) + + return self._annotation + + def _get_field_attrs(self) -> dict[str, Any]: + attrs = super()._get_field_attrs() + + if self.pattern: + attrs["regex_error"] = self.pattern.error # extra + + return attrs class StringOption(BaseStringOption): @@ -478,8 +551,16 @@ class PasswordOption(BaseInputOption): example: Literal[None] = None default: Literal[None] = None redact: Literal[True] = True + _annotation = str _forbidden_chars: str = FORBIDDEN_PASSWORD_CHARS + def _get_field_attrs(self) -> dict[str, Any]: + attrs = super()._get_field_attrs() + + attrs["forbidden_chars"] = self._forbidden_chars # extra + + return attrs + def _value_pre_validator(self): super()._value_pre_validator() @@ -498,10 +579,7 @@ class PasswordOption(BaseInputOption): class ColorOption(BaseInputOption): type: Literal[OptionType.color] = OptionType.color default: Union[str, None] - # pattern = { - # "regexp": r"^#[ABCDEFabcdef\d]{3,6}$", - # "error": "config_validate_color", # i18n: config_validate_color - # } + _annotation = Color # ─ NUMERIC ─────────────────────────────────────────────── @@ -514,6 +592,7 @@ class NumberOption(BaseInputOption): min: Union[int, None] = None max: Union[int, None] = None step: Union[int, None] = None + _annotation = int @staticmethod def normalize(value, option={}): @@ -536,6 +615,14 @@ class NumberOption(BaseInputOption): error=m18n.n("invalid_number"), ) + def _get_field_attrs(self) -> dict[str, Any]: + attrs = super()._get_field_attrs() + attrs["ge"] = self.min + attrs["le"] = self.max + attrs["step"] = self.step # extra + + return attrs + def _value_pre_validator(self): super()._value_pre_validator() if self.value in [None, ""]: @@ -564,6 +651,7 @@ class BooleanOption(BaseInputOption): yes: Any = 1 no: Any = 0 default: Union[bool, int, str, None] = 0 + _annotation = Union[bool, int, str] _yes_answers: set[str] = {"1", "yes", "y", "true", "t", "on"} _no_answers: set[str] = {"0", "no", "n", "false", "f", "off"} @@ -633,6 +721,14 @@ class BooleanOption(BaseInputOption): def get(self, key, default=None): return getattr(self, key, default) + def _get_field_attrs(self) -> dict[str, Any]: + attrs = super()._get_field_attrs() + attrs["parse"] = { # extra + True: self.yes, + False: self.no, + } + return attrs + def _get_prompt_message(self, value: Union[bool, None]) -> str: message = super()._get_prompt_message(value) @@ -648,10 +744,7 @@ class BooleanOption(BaseInputOption): class DateOption(BaseInputOption): type: Literal[OptionType.date] = OptionType.date default: Union[str, None] - # pattern = { - # "regexp": r"^\d{4}-\d\d-\d\d$", - # "error": "config_validate_date", # i18n: config_validate_date - # } + _annotation = datetime.date def _value_pre_validator(self): super()._value_pre_validator() @@ -666,10 +759,7 @@ class DateOption(BaseInputOption): class TimeOption(BaseInputOption): type: Literal[OptionType.time] = OptionType.time default: Union[str, int, None] - # pattern = { - # "regexp": r"^(?:\d|[01]\d|2[0-3]):[0-5]\d$", - # "error": "config_validate_time", # i18n: config_validate_time - # } + _annotation = datetime.time # ─ LOCATIONS ───────────────────────────────────────────── @@ -678,15 +768,13 @@ class TimeOption(BaseInputOption): class EmailOption(BaseInputOption): type: Literal[OptionType.email] = OptionType.email default: Union[EmailStr, None] - # pattern = { - # "regexp": r"^.+@.+", - # "error": "config_validate_email", # i18n: config_validate_email - # } + _annotation = EmailStr class WebPathOption(BaseInputOption): type: Literal[OptionType.path] = OptionType.path default: Union[str, None] + _annotation = str @staticmethod def normalize(value, option={}): @@ -718,10 +806,7 @@ class WebPathOption(BaseInputOption): class URLOption(BaseStringOption): type: Literal[OptionType.url] = OptionType.url default: Union[str, None] - # pattern = { - # "regexp": r"^https?://.*$", - # "error": "config_validate_url", # i18n: config_validate_url - # } + _annotation = HttpUrl # ─ FILE ────────────────────────────────────────────────── @@ -733,6 +818,7 @@ class FileOption(BaseInputOption): # `bytes` for API (a base64 encoded file actually) accept: Union[str, None] = "" # currently only used by the web-admin default: Union[str, None] + _annotation = str # TODO could be Path at some point _upload_dirs: set[str] = set() @classmethod @@ -809,6 +895,17 @@ class BaseChoicesOption(BaseInputOption): # FIXME probably forbid choices to be None? choices: Union[dict[str, Any], list[Any], None] + @property + def _dynamic_annotation(self) -> Union[object, Type[str]]: + if self.choices is not None: + choices = ( + self.choices if isinstance(self.choices, list) else self.choices.keys() + ) + # FIXME in case of dict, try to parse keys with `item_type` (at least number) + return Literal[tuple(choices)] + + return self._annotation + def _get_prompt_message(self, value: Any) -> str: message = super()._get_prompt_message(value) @@ -858,6 +955,7 @@ class SelectOption(BaseChoicesOption): type: Literal[OptionType.select] = OptionType.select choices: Union[dict[str, Any], list[Any]] default: Union[str, None] + _annotation = str class TagsOption(BaseChoicesOption): @@ -865,6 +963,7 @@ class TagsOption(BaseChoicesOption): choices: Union[list[str], None] = None pattern: Union[Pattern, None] = None default: Union[str, list[str], None] + _annotation = str @staticmethod def humanize(value, option={}): @@ -880,6 +979,26 @@ class TagsOption(BaseChoicesOption): value = value.strip() return value + @property + def _dynamic_annotation(self): + # TODO use Literal when serialization is seperated from validation + # if self.choices is not None: + # return Literal[tuple(self.choices)] + + # Repeat pattern stuff since we can't call the bare class `_dynamic_annotation` prop without instantiating it + if self.pattern: + return constr(regex=self.pattern.regexp) + + return self._annotation + + def _get_field_attrs(self) -> dict[str, Any]: + attrs = super()._get_field_attrs() + + if self.choices: + attrs["choices"] = self.choices # extra + + return attrs + def _value_pre_validator(self): values = self.value if isinstance(values, str): From ec5da99a79512b0864b549ce916d8cd805d33c4b Mon Sep 17 00:00:00 2001 From: axolotle Date: Mon, 17 Apr 2023 15:33:34 +0200 Subject: [PATCH 04/50] form: rework pre + post options validators --- src/utils/form.py | 277 ++++++++++++++++++++++++++-------------------- 1 file changed, 155 insertions(+), 122 deletions(-) diff --git a/src/utils/form.py b/src/utils/form.py index 17b0a9432..cbe64b499 100644 --- a/src/utils/form.py +++ b/src/utils/form.py @@ -60,7 +60,7 @@ from yunohost.utils.error import YunohostError, YunohostValidationError from yunohost.utils.i18n import _value_for_locale if TYPE_CHECKING: - from pydantic.fields import FieldInfo + from pydantic.fields import ModelField, FieldInfo logger = getLogger("yunohost.form") @@ -397,6 +397,7 @@ class BaseInputOption(BaseOption): optional: bool = False # FIXME keep required as default? default: Any = None _annotation = Any + _none_as_empty_str: bool = True @validator("default", pre=True) def check_empty_default(value: Any) -> Any: @@ -405,7 +406,9 @@ class BaseInputOption(BaseOption): return value @staticmethod - def humanize(value, option={}): + def humanize(value: Any, option={}) -> str: + if value is None: + return "" return str(value) @staticmethod @@ -474,31 +477,30 @@ class BaseInputOption(BaseOption): return message - def _value_pre_validator(self): - if self.value in [None, ""] and not self.optional: - raise YunohostValidationError("app_argument_required", name=self.id) + @classmethod + def _value_pre_validator(cls, value: Any, field: "ModelField") -> Any: + if value == "": + return None - # we have an answer, do some post checks - if self.value not in [None, ""]: - if self.pattern and not re.match(self.pattern["regexp"], str(self.value)): - raise YunohostValidationError( - self.pattern["error"], - name=self.id, - value=self.value, - ) + return value - def _value_post_validator(self): - if not self.redact: - return self.value + @classmethod + def _value_post_validator(cls, value: Any, field: "ModelField") -> Any: + extras = field.field_info.extra + + if value is None and extras["none_as_empty_str"]: + value = "" + + if not extras.get("redact"): + return value # Tell the operation_logger to redact all password-type / secret args # Also redact the % escaped version of the password that might appear in # the 'args' section of metadata (relevant for password with non-alphanumeric char) data_to_redact = [] - if self.value and isinstance(self.value, str): - data_to_redact.append(self.value) - if self.current_value and isinstance(self.current_value, str): - data_to_redact.append(self.current_value) + if value and isinstance(value, str): + data_to_redact.append(value) + data_to_redact += [ urllib.parse.quote(data) for data in data_to_redact @@ -508,7 +510,7 @@ class BaseInputOption(BaseOption): for operation_logger in OperationLogger._instances: operation_logger.data_to_redact.extend(data_to_redact) - return self.value + return value # ─ STRINGS ─────────────────────────────────────────────── @@ -561,19 +563,25 @@ class PasswordOption(BaseInputOption): return attrs - def _value_pre_validator(self): - super()._value_pre_validator() + @classmethod + def _value_pre_validator( + cls, value: Union[str, None], field: "ModelField" + ) -> Union[str, None]: + value = super()._value_pre_validator(value, field) - if self.value not in [None, ""]: - if any(char in self.value for char in self._forbidden_chars): + if value is not None and value != "": + forbidden_chars: str = field.field_info.extra["forbidden_chars"] + if any(char in value for char in forbidden_chars): raise YunohostValidationError( - "pattern_password_app", forbidden_chars=self._forbidden_chars + "pattern_password_app", forbidden_chars=forbidden_chars ) # If it's an optional argument the value should be empty or strong enough from yunohost.utils.password import assert_password_is_strong_enough - assert_password_is_strong_enough("user", self.value) + assert_password_is_strong_enough("user", value) + + return value class ColorOption(BaseInputOption): @@ -581,6 +589,29 @@ class ColorOption(BaseInputOption): default: Union[str, None] _annotation = Color + @staticmethod + def humanize(value: Union[Color, str, None], option={}) -> str: + if isinstance(value, Color): + value.as_named(fallback=True) + + return super(ColorOption, ColorOption).humanize(value, option) + + @staticmethod + def normalize(value: Union[Color, str, None], option={}) -> str: + if isinstance(value, Color): + return value.as_hex() + + return super(ColorOption, ColorOption).normalize(value, option) + + @classmethod + def _value_post_validator( + cls, value: Union[Color, None], field: "ModelField" + ) -> Union[str, None]: + if isinstance(value, Color): + return value.as_hex() + + return super()._value_post_validator(value, field) + # ─ NUMERIC ─────────────────────────────────────────────── @@ -623,24 +654,16 @@ class NumberOption(BaseInputOption): return attrs - def _value_pre_validator(self): - super()._value_pre_validator() - if self.value in [None, ""]: - return + @classmethod + def _value_pre_validator( + cls, value: Union[int, None], field: "ModelField" + ) -> Union[int, None]: + value = super()._value_pre_validator(value, field) - if self.min is not None and int(self.value) < self.min: - raise YunohostValidationError( - "app_argument_invalid", - name=self.id, - error=m18n.n("invalid_number_min", min=self.min), - ) + if value is None: + return None - if self.max is not None and int(self.value) > self.max: - raise YunohostValidationError( - "app_argument_invalid", - name=self.id, - error=m18n.n("invalid_number_max", max=self.max), - ) + return value # ─ BOOLEAN ─────────────────────────────────────────────── @@ -654,6 +677,7 @@ class BooleanOption(BaseInputOption): _annotation = Union[bool, int, str] _yes_answers: set[str] = {"1", "yes", "y", "true", "t", "on"} _no_answers: set[str] = {"0", "no", "n", "false", "f", "off"} + _none_as_empty_str = False @staticmethod def humanize(value, option={}): @@ -737,6 +761,15 @@ class BooleanOption(BaseInputOption): return message + @classmethod + def _value_post_validator( + cls, value: Union[bool, None], field: "ModelField" + ) -> Any: + if isinstance(value, bool): + return field.field_info.extra["parse"][value] + + return super()._value_post_validator(value, field) + # ─ TIME ────────────────────────────────────────────────── @@ -746,14 +779,14 @@ class DateOption(BaseInputOption): default: Union[str, None] _annotation = datetime.date - def _value_pre_validator(self): - super()._value_pre_validator() + @classmethod + def _value_post_validator( + cls, value: Union[datetime.date, None], field: "ModelField" + ) -> Union[str, None]: + if isinstance(value, datetime.date): + return value.isoformat() - if self.value not in [None, ""]: - try: - datetime.strptime(self.value, "%Y-%m-%d") - except ValueError: - raise YunohostValidationError("config_validate_date") + return super()._value_post_validator(value, field) class TimeOption(BaseInputOption): @@ -761,6 +794,16 @@ class TimeOption(BaseInputOption): default: Union[str, int, None] _annotation = datetime.time + @classmethod + def _value_post_validator( + cls, value: Union[datetime.date, None], field: "ModelField" + ) -> Union[str, None]: + if isinstance(value, datetime.time): + # FIXME could use `value.isoformat()` to get `%H:%M:%S` + return value.strftime("%H:%M") + + return super()._value_post_validator(value, field) + # ─ LOCATIONS ───────────────────────────────────────────── @@ -780,6 +823,9 @@ class WebPathOption(BaseInputOption): def normalize(value, option={}): option = option.dict() if isinstance(option, BaseOption) else option + if value is None: + value = "" + if not isinstance(value, str): raise YunohostValidationError( "app_argument_invalid", @@ -828,52 +874,40 @@ class FileOption(BaseInputOption): if os.path.exists(upload_dir): shutil.rmtree(upload_dir) - def _value_pre_validator(self): - if self.value is None: - self.value = self.current_value - - super()._value_pre_validator() - - # Validation should have already failed if required - if self.value in [None, ""]: - return self.value - - if Moulinette.interface.type != "api": - if not os.path.exists(str(self.value)) or not os.path.isfile( - str(self.value) - ): - raise YunohostValidationError( - "app_argument_invalid", - name=self.id, - error=m18n.n("file_does_not_exist", path=str(self.value)), - ) - - def _value_post_validator(self): + @classmethod + def _value_post_validator(cls, value: Any, field: "ModelField") -> Any: from base64 import b64decode - if not self.value: + if not value: return "" + def is_file_path(s): + return ( + isinstance(s, str) + and s.startswith("/") + and os.path.exists(s) + and os.path.isfile(s) + ) + + file_exists = is_file_path(value) + if Moulinette.interface.type != "api" and not file_exists: + # FIXME error + raise YunohostValidationError("File doesn't exists", raw_msg=True) + elif file_exists: + content = read_file(str(value), file_mode="rb") + else: + content = b64decode(value) + upload_dir = tempfile.mkdtemp(prefix="ynh_filequestion_") _, file_path = tempfile.mkstemp(dir=upload_dir) FileOption._upload_dirs.add(upload_dir) - logger.debug(f"Saving file {self.id} for file question into {file_path}") - - def is_file_path(s): - return isinstance(s, str) and s.startswith("/") and os.path.exists(s) - - if Moulinette.interface.type != "api" or is_file_path(self.value): - content = read_file(str(self.value), file_mode="rb") - else: - content = b64decode(self.value) + logger.debug(f"Saving file {field.name} for file question into {file_path}") write_to_file(file_path, content, file_mode="wb") - self.value = file_path - - return self.value + return file_path # ─ CHOICES ─────────────────────────────────────────────── @@ -895,6 +929,13 @@ class BaseChoicesOption(BaseInputOption): # FIXME probably forbid choices to be None? choices: Union[dict[str, Any], list[Any], None] + @validator("choices", pre=True) + def parse_comalist_choices(value: Any) -> Union[dict[str, Any], list[Any], None]: + if isinstance(value, str): + values = [value.strip() for value in value.split(",")] + return [value for value in values if value] + return value + @property def _dynamic_annotation(self) -> Union[object, Type[str]]: if self.choices is not None: @@ -937,19 +978,6 @@ class BaseChoicesOption(BaseInputOption): return message - def _value_pre_validator(self): - super()._value_pre_validator() - - # we have an answer, do some post checks - if self.value not in [None, ""]: - if self.choices and self.value not in self.choices: - raise YunohostValidationError( - "app_argument_choice_invalid", - name=self.id, - value=self.value, - choices=", ".join(str(choice) for choice in self.choices), - ) - class SelectOption(BaseChoicesOption): type: Literal[OptionType.select] = OptionType.select @@ -969,6 +997,8 @@ class TagsOption(BaseChoicesOption): def humanize(value, option={}): if isinstance(value, list): return ",".join(str(v) for v in value) + if not value: + return "" return value @staticmethod @@ -976,7 +1006,9 @@ class TagsOption(BaseChoicesOption): if isinstance(value, list): return ",".join(str(v) for v in value) if isinstance(value, str): - value = value.strip() + value = value.strip().strip(",") + if value is None or value == "": + return "" return value @property @@ -999,36 +1031,37 @@ class TagsOption(BaseChoicesOption): return attrs - def _value_pre_validator(self): - values = self.value - if isinstance(values, str): - values = values.split(",") - elif values is None: - values = [] + @classmethod + def _value_pre_validator( + cls, value: Union[list, str, None], field: "ModelField" + ) -> Union[str, None]: + if value is None or value == "": + return None - if not isinstance(values, list): - if self.choices: - raise YunohostValidationError( - "app_argument_choice_invalid", - name=self.id, - value=self.value, - choices=", ".join(str(choice) for choice in self.choices), - ) + if not isinstance(value, (list, str, type(None))): raise YunohostValidationError( "app_argument_invalid", - name=self.id, - error=f"'{str(self.value)}' is not a list", + name=field.name, + error=f"'{str(value)}' is not a list", ) - for value in values: - self.value = value - super()._value_pre_validator() - self.value = values + if isinstance(value, str): + value = [v.strip() for v in value.split(",")] + value = [v for v in value if v] - def _value_post_validator(self): - if isinstance(self.value, list): - self.value = ",".join(self.value) - return super()._value_post_validator() + if isinstance(value, list): + choices = field.field_info.extra.get("choices") + if choices: + if not all(v in choices for v in value): + raise YunohostValidationError( + "app_argument_choice_invalid", + name=field.name, + value=value, + choices=", ".join(str(choice) for choice in choices), + ) + + return ",".join(str(v) for v in value) + return value # ─ ENTITIES ────────────────────────────────────────────── From c428ba616a3f2d63a22257560345429f0af29729 Mon Sep 17 00:00:00 2001 From: axolotle Date: Mon, 17 Apr 2023 15:37:56 +0200 Subject: [PATCH 05/50] test:options: update tests results to pydantic parsing --- src/tests/test_questions.py | 374 +++++++++++++++++++----------------- 1 file changed, 197 insertions(+), 177 deletions(-) diff --git a/src/tests/test_questions.py b/src/tests/test_questions.py index 9eceedcaf..1f8667adf 100644 --- a/src/tests/test_questions.py +++ b/src/tests/test_questions.py @@ -20,7 +20,6 @@ from yunohost.utils.form import ( BaseChoicesOption, BaseInputOption, BaseReadonlyOption, - PasswordOption, DomainOption, WebPathOption, BooleanOption, @@ -436,6 +435,10 @@ class BaseTest: @classmethod def _test_basic_attrs(self): raw_option = self.get_raw_option(optional=True) + + if raw_option["type"] == "select": + raw_option["choices"] = ["one"] + id_ = raw_option["id"] option, value = _fill_or_prompt_one_option(raw_option, None) @@ -481,6 +484,7 @@ class BaseTest: base_raw_option = prefill_data["raw_option"] prefill = prefill_data["prefill"] + # FIXME could patch prompt with prefill if we switch to "do not apply default if value is None|''" with patch_prompt("") as prompt: raw_option = self.get_raw_option( raw_option=base_raw_option, @@ -583,9 +587,7 @@ class TestAlert(TestDisplayText): (None, None, {"ask": "Some text\na new line"}), (None, None, {"ask": {"en": "Some text\na new line", "fr": "Un peu de texte\nune nouvelle ligne"}}), *[(None, None, {"ask": "question", "style": style}) for style in ("success", "info", "warning", "danger")], - *xpass(scenarios=[ - (None, None, {"ask": "question", "style": "nimp"}), - ], reason="Should fail, wrong style"), + (None, FAIL, {"ask": "question", "style": "nimp"}), ] # fmt: on @@ -643,11 +645,15 @@ class TestString(BaseTest): scenarios = [ *nones(None, "", output=""), # basic typed values - *unchanged(False, True, 0, 1, -1, 1337, 13.37, [], ["one"], {}, raw_option={"optional": True}), # FIXME should output as str? + (False, "False"), + (True, "True"), + (0, "0"), + (1, "1"), + (-1, "-1"), + (1337, "1337"), + (13.37, "13.37"), + *all_fails([], ["one"], {}), *unchanged("none", "_none", "False", "True", "0", "1", "-1", "1337", "13.37", "[]", ",", "['one']", "one,two", r"{}", "value", raw_option={"optional": True}), - *xpass(scenarios=[ - ([], []), - ], reason="Should fail"), # test strip ("value", "value"), ("value\n", "value"), @@ -660,7 +666,7 @@ class TestString(BaseTest): (" ##value \n \tvalue\n ", "##value \n \tvalue"), ], reason=r"should fail or without `\n`?"), # readonly - ("overwrite", "expected value", {"readonly": True, "current_value": "expected value"}), + ("overwrite", "expected value", {"readonly": True, "default": "expected value"}), # FIXME do we want to fail instead? ] # fmt: on @@ -680,11 +686,15 @@ class TestText(BaseTest): scenarios = [ *nones(None, "", output=""), # basic typed values - *unchanged(False, True, 0, 1, -1, 1337, 13.37, [], ["one"], {}, raw_option={"optional": True}), # FIXME should fail or output as str? + (False, "False"), + (True, "True"), + (0, "0"), + (1, "1"), + (-1, "-1"), + (1337, "1337"), + (13.37, "13.37"), + *all_fails([], ["one"], {}), *unchanged("none", "_none", "False", "True", "0", "1", "-1", "1337", "13.37", "[]", ",", "['one']", "one,two", r"{}", "value", raw_option={"optional": True}), - *xpass(scenarios=[ - ([], []) - ], reason="Should fail"), ("value", "value"), ("value\n value", "value\n value"), # test no strip @@ -697,7 +707,7 @@ class TestText(BaseTest): (r" ##value \n \tvalue\n ", r"##value \n \tvalue\n"), ], reason="Should not be stripped"), # readonly - ("overwrite", "expected value", {"readonly": True, "current_value": "expected value"}), + ("overwrite", "expected value", {"readonly": True, "default": "expected value"}), ] # fmt: on @@ -715,7 +725,7 @@ class TestPassword(BaseTest): } # fmt: off scenarios = [ - *all_fails(False, True, 0, 1, -1, 1337, 13.37, raw_option={"optional": True}, error=TypeError), # FIXME those fails with TypeError + *all_fails(False, True, 0, 1, -1, 1337, 13.37, raw_option={"optional": True}), *all_fails([], ["one"], {}, raw_option={"optional": True}, error=AttributeError), # FIXME those fails with AttributeError *all_fails("none", "_none", "False", "True", "0", "1", "-1", "1337", "13.37", "[]", ",", "['one']", "one,two", r"{}", "value", "value\n", raw_option={"optional": True}), *nones(None, "", output=""), @@ -729,9 +739,9 @@ class TestPassword(BaseTest): ], reason="Should output exactly the same"), ("s3cr3t!!", "s3cr3t!!"), ("secret", FAIL), - *[("supersecret" + char, FAIL) for char in PasswordOption.forbidden_chars], # FIXME maybe add ` \n` to the list? + *[("supersecret" + char, FAIL) for char in FORBIDDEN_PASSWORD_CHARS], # FIXME maybe add ` \n` to the list? # readonly - ("s3cr3t!!", YunohostError, {"readonly": True, "current_value": "isforbidden"}), # readonly is forbidden + ("s3cr3t!!", FAIL, {"readonly": True, "current_value": "isforbidden"}), # readonly is forbidden ] # fmt: on @@ -744,35 +754,31 @@ class TestPassword(BaseTest): class TestColor(BaseTest): raw_option = {"type": "color", "id": "color_id"} prefill = { - "raw_option": {"default": "#ff0000"}, - "prefill": "#ff0000", - # "intake": "#ff00ff", + "raw_option": {"default": "red"}, + "prefill": "red", } # fmt: off scenarios = [ *all_fails(False, True, 0, 1, -1, 1337, 13.37, [], ["one"], {}, raw_option={"optional": True}), - *all_fails("none", "_none", "False", "True", "0", "1", "-1", "1337", "13.37", "[]", ",", "['one']", "one,two", r"{}", "value", "value\n", raw_option={"optional": True}), + *all_fails("none", "_none", "False", "True", "0", "1", "-1", "13.37", "[]", ",", "['one']", "one,two", r"{}", "value", "value\n", raw_option={"optional": True}), *nones(None, "", output=""), # custom valid - ("#000000", "#000000"), + (" #fe1 ", "#fe1"), + ("#000000", "#000"), ("#000", "#000"), - ("#fe100", "#fe100"), - (" #fe100 ", "#fe100"), - ("#ABCDEF", "#ABCDEF"), + ("#ABCDEF", "#abcdef"), + ('1337', "#1337"), # rgba=(17, 51, 51, 0.47) + ("000000", "#000"), + ("#feaf", "#fea"), # `#feaf` is `#fea` with alpha at `f|100%` -> equivalent to `#fea` + # named + ("red", "#f00"), + ("yellow", "#ff0"), # custom fail - *xpass(scenarios=[ - ("#feaf", "#feaf"), - ], reason="Should fail; not a legal color value"), - ("000000", FAIL), ("#12", FAIL), ("#gggggg", FAIL), ("#01010101af", FAIL), - *xfail(scenarios=[ - ("red", "#ff0000"), - ("yellow", "#ffff00"), - ], reason="Should work with pydantic"), # readonly - ("#ffff00", "#fe100", {"readonly": True, "current_value": "#fe100"}), + ("#ffff00", "#000", {"readonly": True, "default": "#000"}), ] # fmt: on @@ -796,10 +802,8 @@ class TestNumber(BaseTest): *nones(None, "", output=None), *unchanged(0, 1, -1, 1337), - *xpass(scenarios=[(False, False)], reason="should fail or output as `0`"), - *xpass(scenarios=[(True, True)], reason="should fail or output as `1`"), - *all_as("0", 0, output=0), - *all_as("1", 1, output=1), + *all_as(False, "0", 0, output=0), # FIXME should `False` fail instead? + *all_as(True, "1", 1, output=1), # FIXME should `True` fail instead? *all_as("1337", 1337, output=1337), *xfail(scenarios=[ ("-1", -1) @@ -814,7 +818,7 @@ class TestNumber(BaseTest): (-10, -10, {"default": 10}), (-10, -10, {"default": 10, "optional": True}), # readonly - (1337, 10000, {"readonly": True, "current_value": 10000}), + (1337, 10000, {"readonly": True, "default": "10000"}), ] # fmt: on # FIXME should `step` be some kind of "multiple of"? @@ -839,14 +843,20 @@ class TestBoolean(BaseTest): *all_fails("none", "None"), # FIXME should output as `0` (default) like other none values when required? *all_as(None, "", output=0, raw_option={"optional": True}), # FIXME should output as `None`? *all_as("none", "None", output=None, raw_option={"optional": True}), - # FIXME even if default is explicity `None|""`, it ends up with class_default `0` - *all_as(None, "", output=0, raw_option={"default": None}), # FIXME this should fail, default is `None` - *all_as(None, "", output=0, raw_option={"optional": True, "default": None}), # FIXME even if default is explicity None, it ends up with class_default - *all_as(None, "", output=0, raw_option={"default": ""}), # FIXME this should fail, default is `""` - *all_as(None, "", output=0, raw_option={"optional": True, "default": ""}), # FIXME even if default is explicity None, it ends up with class_default - # With "none" behavior is ok - *all_fails(None, "", raw_option={"default": "none"}), - *all_as(None, "", output=None, raw_option={"optional": True, "default": "none"}), + { + "raw_options": [ + {"default": None}, + {"default": ""}, + {"default": "none"}, + {"default": "None"} + ], + "scenarios": [ + # All none values fails if default is overriden + *all_fails(None, "", "none", "None"), + # All none values ends up as None if default is overriden + *all_as(None, "", "none", "None", output=None, raw_option={"optional": True}), + ] + }, # Unhandled types should fail *all_fails(1337, "1337", "string", [], "[]", ",", "one,two"), *all_fails(1337, "1337", "string", [], "[]", ",", "one,two", {"optional": True}), @@ -879,7 +889,7 @@ class TestBoolean(BaseTest): "scenarios": all_fails("", "y", "n", error=AssertionError), }, # readonly - (1, 0, {"readonly": True, "current_value": 0}), + (1, 0, {"readonly": True, "default": 0}), ] @@ -896,8 +906,12 @@ class TestDate(BaseTest): } # fmt: off scenarios = [ - *all_fails(False, True, 0, 1, -1, 1337, 13.37, [], ["one"], {}, raw_option={"optional": True}), - *all_fails("none", "_none", "False", "True", "0", "1", "-1", "1337", "13.37", "[]", ",", "['one']", "one,two", r"{}", "value", "value\n", raw_option={"optional": True}), + # Those passes since False|True are parsed as 0|1 then int|float are considered a timestamp in seconds which ends up as default Unix date + *all_as(False, True, 0, 1, 1337, 13.37, "0", "1", "1337", "13.37", output="1970-01-01"), + # Those are negative one second timestamp ending up as Unix date - 1 sec (so day change) + *all_as(-1, "-1", output="1969-12-31"), + *all_fails([], ["one"], {}, raw_option={"optional": True}), + *all_fails("none", "_none", "False", "True", "[]", ",", "['one']", "one,two", r"{}", "value", "value\n", raw_option={"optional": True}), *nones(None, "", output=""), # custom valid ("2070-12-31", "2070-12-31"), @@ -906,18 +920,16 @@ class TestDate(BaseTest): ("2025-06-15T13:45:30", "2025-06-15"), ("2025-06-15 13:45:30", "2025-06-15") ], reason="iso date repr should be valid and extra data striped"), - *xfail(scenarios=[ - (1749938400, "2025-06-15"), - (1749938400.0, "2025-06-15"), - ("1749938400", "2025-06-15"), - ("1749938400.0", "2025-06-15"), - ], reason="timestamp could be an accepted value"), + (1749938400, "2025-06-14"), + (1749938400.0, "2025-06-14"), + ("1749938400", "2025-06-14"), + ("1749938400.0", "2025-06-14"), # custom invalid ("29-12-2070", FAIL), ("12-01-10", FAIL), ("2022-02-29", FAIL), # readonly - ("2070-12-31", "2024-02-29", {"readonly": True, "current_value": "2024-02-29"}), + ("2070-12-31", "2024-02-29", {"readonly": True, "default": "2024-02-29"}), ] # fmt: on @@ -935,22 +947,26 @@ class TestTime(BaseTest): } # fmt: off scenarios = [ - *all_fails(False, True, 0, 1, -1, 1337, 13.37, [], ["one"], {}, raw_option={"optional": True}), - *all_fails("none", "_none", "False", "True", "0", "1", "-1", "1337", "13.37", "[]", ",", "['one']", "one,two", r"{}", "value", "value\n", raw_option={"optional": True}), + # Those passes since False|True are parsed as 0|1 then int|float are considered a timestamp in seconds but we don't take seconds into account so -> 00:00 + *all_as(False, True, 0, 1, 13.37, "0", "1", "13.37", output="00:00"), + # 1337 seconds == 22 minutes + *all_as(1337, "1337", output="00:22"), + # Negative timestamp fails + *all_fails(-1, "-1", error=OverflowError), # FIXME should handle that as a validation error + # *all_fails(False, True, 0, 1, -1, 1337, 13.37, [], ["one"], {}, raw_option={"optional": True}), + *all_fails("none", "_none", "False", "True", "[]", ",", "['one']", "one,two", r"{}", "value", "value\n", raw_option={"optional": True}), *nones(None, "", output=""), # custom valid *unchanged("00:00", "08:00", "12:19", "20:59", "23:59"), - ("3:00", "3:00"), # FIXME should fail or output as `"03:00"`? - *xfail(scenarios=[ - ("22:35:05", "22:35"), - ("22:35:03.514", "22:35"), - ], reason="time as iso format could be valid"), + ("3:00", "03:00"), + ("23:1", "23:01"), + ("22:35:05", "22:35"), + ("22:35:03.514", "22:35"), # custom invalid ("24:00", FAIL), - ("23:1", FAIL), ("23:005", FAIL), # readonly - ("00:00", "08:00", {"readonly": True, "current_value": "08:00"}), + ("00:00", "08:00", {"readonly": True, "default": "08:00"}), ] # fmt: on @@ -973,72 +989,75 @@ class TestEmail(BaseTest): *nones(None, "", output=""), ("\n Abc@example.tld ", "Abc@example.tld"), + *xfail(scenarios=[("admin@ynh.local", "admin@ynh.local")], reason="Should this pass?"), # readonly - ("Abc@example.tld", "admin@ynh.local", {"readonly": True, "current_value": "admin@ynh.local"}), + ("Abc@example.tld", "admin@ynh.org", {"readonly": True, "default": "admin@ynh.org"}), # Next examples are from https://github.com/JoshData/python-email-validator/blob/main/tests/test_syntax.py # valid email values - ("Abc@example.tld", "Abc@example.tld"), - ("Abc.123@test-example.com", "Abc.123@test-example.com"), - ("user+mailbox/department=shipping@example.tld", "user+mailbox/department=shipping@example.tld"), - ("伊昭傑@郵件.商務", "伊昭傑@郵件.商務"), - ("राम@मोहन.ईन्फो", "राम@मोहन.ईन्फो"), - ("юзер@екзампл.ком", "юзер@екзампл.ком"), - ("θσερ@εχαμπλε.ψομ", "θσερ@εχαμπλε.ψομ"), - ("葉士豪@臺網中心.tw", "葉士豪@臺網中心.tw"), - ("jeff@臺網中心.tw", "jeff@臺網中心.tw"), - ("葉士豪@臺網中心.台灣", "葉士豪@臺網中心.台灣"), - ("jeff葉@臺網中心.tw", "jeff葉@臺網中心.tw"), - ("ñoñó@example.tld", "ñoñó@example.tld"), - ("甲斐黒川日本@example.tld", "甲斐黒川日本@example.tld"), - ("чебурашкаящик-с-апельсинами.рф@example.tld", "чебурашкаящик-с-апельсинами.рф@example.tld"), - ("उदाहरण.परीक्ष@domain.with.idn.tld", "उदाहरण.परीक्ष@domain.with.idn.tld"), - ("ιωάννης@εεττ.gr", "ιωάννης@εεττ.gr"), + *unchanged( + "Abc@example.tld", + "Abc.123@test-example.com", + "user+mailbox/department=shipping@example.tld", + "伊昭傑@郵件.商務", + "राम@मोहन.ईन्फो", + "юзер@екзампл.ком", + "θσερ@εχαμπλε.ψομ", + "葉士豪@臺網中心.tw", + "jeff@臺網中心.tw", + "葉士豪@臺網中心.台灣", + "jeff葉@臺網中心.tw", + "ñoñó@example.tld", + "甲斐黒川日本@example.tld", + "чебурашкаящик-с-апельсинами.рф@example.tld", + "उदाहरण.परीक्ष@domain.with.idn.tld", + "ιωάννης@εεττ.gr", + ), # invalid email (Hiding because our current regex is very permissive) - # ("my@localhost", FAIL), - # ("my@.leadingdot.com", FAIL), - # ("my@．leadingfwdot.com", FAIL), - # ("my@twodots..com", FAIL), - # ("my@twofwdots．．.com", FAIL), - # ("my@trailingdot.com.", FAIL), - # ("my@trailingfwdot.com．", FAIL), - # ("me@-leadingdash", FAIL), - # ("me@－leadingdashfw", FAIL), - # ("me@trailingdash-", FAIL), - # ("me@trailingdashfw－", FAIL), - # ("my@baddash.-.com", FAIL), - # ("my@baddash.-a.com", FAIL), - # ("my@baddash.b-.com", FAIL), - # ("my@baddashfw.－.com", FAIL), - # ("my@baddashfw.－a.com", FAIL), - # ("my@baddashfw.b－.com", FAIL), - # ("my@example.com\n", FAIL), - # ("my@example\n.com", FAIL), - # ("me@x!", FAIL), - # ("me@x ", FAIL), - # (".leadingdot@domain.com", FAIL), - # ("twodots..here@domain.com", FAIL), - # ("trailingdot.@domain.email", FAIL), - # ("me@⒈wouldbeinvalid.com", FAIL), - ("@example.com", FAIL), - # ("\nmy@example.com", FAIL), - ("m\ny@example.com", FAIL), - ("my\n@example.com", FAIL), - # ("11111111112222222222333333333344444444445555555555666666666677777@example.com", FAIL), - # ("111111111122222222223333333333444444444455555555556666666666777777@example.com", FAIL), - # ("me@1111111111222222222233333333334444444444555555555.6666666666777777777788888888889999999999000000000.1111111111222222222233333333334444444444555555555.6666666666777777777788888888889999999999000000000.111111111122222222223333333333444444444455555555556.com", FAIL), - # ("me@1111111111222222222233333333334444444444555555555.6666666666777777777788888888889999999999000000000.1111111111222222222233333333334444444444555555555.6666666666777777777788888888889999999999000000000.1111111111222222222233333333334444444444555555555566.com", FAIL), - # ("me@中1111111111222222222233333333334444444444555555555.6666666666777777777788888888889999999999000000000.1111111111222222222233333333334444444444555555555.6666666666777777777788888888889999999999000000000.1111111111222222222233333333334444444444555555555566.com", FAIL), - # ("my.long.address@1111111111222222222233333333334444444444555555555.6666666666777777777788888888889999999999000000000.1111111111222222222233333333334444444444555555555.6666666666777777777788888888889999999999000000000.11111111112222222222333333333344444.info", FAIL), - # ("my.long.address@λ111111111222222222233333333334444444444555555555.6666666666777777777788888888889999999999000000000.1111111111222222222233333333334444444444555555555.6666666666777777777788888888889999999999000000000.11111111112222222222333333.info", FAIL), - # ("my.long.address@λ111111111222222222233333333334444444444555555555.6666666666777777777788888888889999999999000000000.1111111111222222222233333333334444444444555555555.6666666666777777777788888888889999999999000000000.1111111111222222222233333333334444.info", FAIL), - # ("my.λong.address@1111111111222222222233333333334444444444555555555.6666666666777777777788888888889999999999000000000.1111111111222222222233333333334444444444555555555.6666666666777777777788888888889999999999000000000.111111111122222222223333333333444.info", FAIL), - # ("my.λong.address@1111111111222222222233333333334444444444555555555.6666666666777777777788888888889999999999000000000.1111111111222222222233333333334444444444555555555.6666666666777777777788888888889999999999000000000.1111111111222222222233333333334444.info", FAIL), - # ("me@bad-tld-1", FAIL), - # ("me@bad.tld-2", FAIL), - # ("me@xn--0.tld", FAIL), - # ("me@yy--0.tld", FAIL), - # ("me@yy－－0.tld", FAIL), + *all_fails( + "my@localhost", + "my@.leadingdot.com", + "my@．leadingfwdot.com", + "my@twodots..com", + "my@twofwdots．．.com", + "my@trailingdot.com.", + "my@trailingfwdot.com．", + "me@-leadingdash", + "me@－leadingdashfw", + "me@trailingdash-", + "me@trailingdashfw－", + "my@baddash.-.com", + "my@baddash.-a.com", + "my@baddash.b-.com", + "my@baddashfw.－.com", + "my@baddashfw.－a.com", + "my@baddashfw.b－.com", + "my@example\n.com", + "me@x!", + "me@x ", + ".leadingdot@domain.com", + "twodots..here@domain.com", + "trailingdot.@domain.email", + "me@⒈wouldbeinvalid.com", + "@example.com", + "m\ny@example.com", + "my\n@example.com", + "11111111112222222222333333333344444444445555555555666666666677777@example.com", + "111111111122222222223333333333444444444455555555556666666666777777@example.com", + "me@1111111111222222222233333333334444444444555555555.6666666666777777777788888888889999999999000000000.1111111111222222222233333333334444444444555555555.6666666666777777777788888888889999999999000000000.111111111122222222223333333333444444444455555555556.com", + "me@1111111111222222222233333333334444444444555555555.6666666666777777777788888888889999999999000000000.1111111111222222222233333333334444444444555555555.6666666666777777777788888888889999999999000000000.1111111111222222222233333333334444444444555555555566.com", + "me@中1111111111222222222233333333334444444444555555555.6666666666777777777788888888889999999999000000000.1111111111222222222233333333334444444444555555555.6666666666777777777788888888889999999999000000000.1111111111222222222233333333334444444444555555555566.com", + "my.long.address@1111111111222222222233333333334444444444555555555.6666666666777777777788888888889999999999000000000.1111111111222222222233333333334444444444555555555.6666666666777777777788888888889999999999000000000.11111111112222222222333333333344444.info", + "my.long.address@λ111111111222222222233333333334444444444555555555.6666666666777777777788888888889999999999000000000.1111111111222222222233333333334444444444555555555.6666666666777777777788888888889999999999000000000.11111111112222222222333333.info", + "my.long.address@λ111111111222222222233333333334444444444555555555.6666666666777777777788888888889999999999000000000.1111111111222222222233333333334444444444555555555.6666666666777777777788888888889999999999000000000.1111111111222222222233333333334444.info", + "my.λong.address@1111111111222222222233333333334444444444555555555.6666666666777777777788888888889999999999000000000.1111111111222222222233333333334444444444555555555.6666666666777777777788888888889999999999000000000.111111111122222222223333333333444.info", + "my.λong.address@1111111111222222222233333333334444444444555555555.6666666666777777777788888888889999999999000000000.1111111111222222222233333333334444444444555555555.6666666666777777777788888888889999999999000000000.1111111111222222222233333333334444.info", + "me@bad-tld-1", + "me@bad.tld-2", + "me@xn--0.tld", + "me@yy--0.tld", + "me@yy－－0.tld", + ) ] # fmt: on @@ -1087,7 +1106,7 @@ class TestWebPath(BaseTest): ("https://example.com/folder", "/https://example.com/folder") ], reason="Should fail or scheme+domain removed"), # readonly - ("/overwrite", "/value", {"readonly": True, "current_value": "/value"}), + ("/overwrite", "/value", {"readonly": True, "default": "/value"}), # FIXME should path have forbidden_chars? ] # fmt: on @@ -1111,21 +1130,17 @@ class TestUrl(BaseTest): *nones(None, "", output=""), ("http://some.org/folder/file.txt", "http://some.org/folder/file.txt"), + (' https://www.example.com \n', 'https://www.example.com'), # readonly - ("https://overwrite.org", "https://example.org", {"readonly": True, "current_value": "https://example.org"}), + ("https://overwrite.org", "https://example.org", {"readonly": True, "default": "https://example.org"}), # rest is taken from https://github.com/pydantic/pydantic/blob/main/tests/test_networks.py # valid *unchanged( # Those are valid but not sure how they will output with pydantic 'http://example.org', - 'http://test', - 'http://localhost', 'https://example.org/whatever/next/', 'https://example.org', - 'http://localhost', - 'http://localhost/', - 'http://localhost:8000', - 'http://localhost:8000/', + 'https://foo_bar.example.com/', 'http://example.co.jp', 'http://www.example.com/a%C2%B1b', @@ -1149,29 +1164,31 @@ class TestUrl(BaseTest): 'http://twitter.com/@handle/', 'http://11.11.11.11.example.com/action', 'http://abc.11.11.11.11.example.com/action', - 'http://example#', - 'http://example/#', - 'http://example/#fragment', - 'http://example/?#', 'http://example.org/path#', 'http://example.org/path#fragment', 'http://example.org/path?query#', 'http://example.org/path?query#fragment', + 'https://foo_bar.example.com/', + 'https://exam_ple.com/', + 'HTTP://EXAMPLE.ORG', + 'https://example.org', + 'https://example.org?a=1&b=2', + 'https://example.org#a=3;b=3', + 'https://example.xn--p1ai', + 'https://example.xn--vermgensberatung-pwb', + 'https://example.xn--zfr164b', ), - # Pydantic default parsing add a final `/` - ('https://foo_bar.example.com/', 'https://foo_bar.example.com/'), - ('https://exam_ple.com/', 'https://exam_ple.com/'), *xfail(scenarios=[ - (' https://www.example.com \n', 'https://www.example.com/'), - ('HTTP://EXAMPLE.ORG', 'http://example.org/'), - ('https://example.org', 'https://example.org/'), - ('https://example.org?a=1&b=2', 'https://example.org/?a=1&b=2'), - ('https://example.org#a=3;b=3', 'https://example.org/#a=3;b=3'), - ('https://example.xn--p1ai', 'https://example.xn--p1ai/'), - ('https://example.xn--vermgensberatung-pwb', 'https://example.xn--vermgensberatung-pwb/'), - ('https://example.xn--zfr164b', 'https://example.xn--zfr164b/'), - ], reason="pydantic default behavior would append a final `/`"), - + ('http://test', 'http://test'), + ('http://localhost', 'http://localhost'), + ('http://localhost/', 'http://localhost/'), + ('http://localhost:8000', 'http://localhost:8000'), + ('http://localhost:8000/', 'http://localhost:8000/'), + ('http://example#', 'http://example#'), + ('http://example/#', 'http://example/#'), + ('http://example/#fragment', 'http://example/#fragment'), + ('http://example/?#', 'http://example/?#'), + ], reason="Should this be valid?"), # invalid *all_fails( 'ftp://example.com/', @@ -1182,15 +1199,13 @@ class TestUrl(BaseTest): "/", "+http://example.com/", "ht*tp://example.com/", + "http:///", + "http://??", + "https://example.org more", + "http://2001:db8::ff00:42:8329", + "http://[192.168.1.1]:8329", + "http://example.com:99999", ), - *xpass(scenarios=[ - ("http:///", "http:///"), - ("http://??", "http://??"), - ("https://example.org more", "https://example.org more"), - ("http://2001:db8::ff00:42:8329", "http://2001:db8::ff00:42:8329"), - ("http://[192.168.1.1]:8329", "http://[192.168.1.1]:8329"), - ("http://example.com:99999", "http://example.com:99999"), - ], reason="Should fail"), ] # fmt: on @@ -1361,7 +1376,6 @@ class TestSelect(BaseTest): # [-1, 0, 1] "raw_options": [ {"choices": [-1, 0, 1, 10]}, - {"choices": {-1: "verbose -one", 0: "verbose zero", 1: "verbose one", 10: "verbose ten"}}, ], "scenarios": [ *nones(None, "", output=""), @@ -1375,6 +1389,18 @@ class TestSelect(BaseTest): *all_fails("100", 100), ] }, + { + "raw_options": [ + {"choices": {-1: "verbose -one", 0: "verbose zero", 1: "verbose one", 10: "verbose ten"}}, + {"choices": {"-1": "verbose -one", "0": "verbose zero", "1": "verbose one", "10": "verbose ten"}}, + ], + "scenarios": [ + *nones(None, "", output=""), + *all_fails(-1, 0, 1, 10), # Should pass? converted to str? + *unchanged("-1", "0", "1", "10"), + *all_fails("100", 100), + ] + }, # [True, False, None] *unchanged(True, False, raw_option={"choices": [True, False, None]}), # FIXME we should probably forbid None in choices (None, FAIL, {"choices": [True, False, None]}), @@ -1402,7 +1428,7 @@ class TestSelect(BaseTest): ] }, # readonly - ("one", "two", {"readonly": True, "choices": ["one", "two"], "current_value": "two"}), + ("one", "two", {"readonly": True, "choices": ["one", "two"], "default": "two"}), ] # fmt: on @@ -1411,7 +1437,7 @@ class TestSelect(BaseTest): # │ TAGS │ # ╰───────────────────────────────────────────────────────╯ - +# [], ["one"], {} class TestTags(BaseTest): raw_option = {"type": "tags", "id": "tags_id"} prefill = { @@ -1420,12 +1446,7 @@ class TestTags(BaseTest): } # fmt: off scenarios = [ - *nones(None, [], "", output=""), - # FIXME `","` could be considered a none value which kinda already is since it fail when required - (",", FAIL), - *xpass(scenarios=[ - (",", ",", {"optional": True}) - ], reason="Should output as `''`? ie: None"), + *nones(None, [], "", ",", output=""), { "raw_options": [ {}, @@ -1450,7 +1471,7 @@ class TestTags(BaseTest): *all_fails(*([t] for t in [False, True, -1, 0, 1, 1337, 13.37, [], ["one"], {}]), raw_option={"choices": [False, True, -1, 0, 1, 1337, 13.37, [], ["one"], {}]}), *all_fails(*([str(t)] for t in [False, True, -1, 0, 1, 1337, 13.37, [], ["one"], {}]), raw_option={"choices": [False, True, -1, 0, 1, 1337, 13.37, [], ["one"], {}]}), # readonly - ("one", "one,two", {"readonly": True, "choices": ["one", "two"], "current_value": "one,two"}), + ("one", "one,two", {"readonly": True, "choices": ["one", "two"], "default": "one,two"}), ] # fmt: on @@ -1566,8 +1587,7 @@ class TestApp(BaseTest): ], "scenarios": [ # FIXME there are currently 3 different nones (`None`, `""` and `_none`), choose one? - *nones(None, output=None), # FIXME Should return chosen none? - *nones("", output=""), # FIXME Should return chosen none? + *nones(None, "", output=""), # FIXME Should return chosen none? *xpass(scenarios=[ ("_none", "_none"), ("_none", "_none", {"default": "_none"}), @@ -1590,7 +1610,7 @@ class TestApp(BaseTest): (installed_webapp["id"], installed_webapp["id"], {"filter": "is_webapp"}), (installed_webapp["id"], FAIL, {"filter": "is_webapp == false"}), (installed_webapp["id"], FAIL, {"filter": "id != 'my_webapp'"}), - (None, None, {"filter": "id == 'fake_app'", "optional": True}), + (None, "", {"filter": "id == 'fake_app'", "optional": True}), ] }, { @@ -1800,7 +1820,7 @@ class TestGroup(BaseTest): ("", "custom_group", {"default": "custom_group"}), ], reason="Should throw 'default must be in (None, 'all_users', 'visitors', 'admins')"), # readonly - ("admins", YunohostError, {"readonly": True}), # readonly is forbidden + ("admins", "all_users", {"readonly": True}), # readonly is forbidden (default is "all_users") ] }, ] @@ -1880,12 +1900,12 @@ def test_options_query_string(): "string_id": "string", "text_id": "text\ntext", "password_id": "sUpRSCRT", - "color_id": "#ffff00", + "color_id": "#ff0", "number_id": 10, "boolean_id": 1, "date_id": "2030-03-06", "time_id": "20:55", - "email_id": "coucou@ynh.local", + "email_id": "coucou@ynh.org", "path_id": "/ynh-dev", "url_id": "https://yunohost.org", "file_id": file_content1, @@ -1908,7 +1928,7 @@ def test_options_query_string(): "&boolean_id=y" "&date_id=2030-03-06" "&time_id=20:55" - "&email_id=coucou@ynh.local" + "&email_id=coucou@ynh.org" "&path_id=ynh-dev/" "&url_id=https://yunohost.org" f"&file_id={file_repr}" From 3ff6e6ed96c91e9ae94821f54f9939811edb930e Mon Sep 17 00:00:00 2001 From: axolotle Date: Mon, 17 Apr 2023 16:20:52 +0200 Subject: [PATCH 06/50] app: update app_install --- src/app.py | 40 ++++++++++++++++++---------------------- 1 file changed, 18 insertions(+), 22 deletions(-) diff --git a/src/app.py b/src/app.py index 754a920a0..1a2e80442 100644 --- a/src/app.py +++ b/src/app.py @@ -1098,13 +1098,9 @@ def app_install( app_setting_path = os.path.join(APPS_SETTING_PATH, app_instance_name) # Retrieve arguments list for install script - raw_questions = manifest["install"] - questions = ask_questions_and_parse_answers(raw_questions, prefilled_answers=args) - args = { - question.id: question.value - for question in questions - if not question.readonly and question.value is not None - } + raw_options = manifest["install"] + options, form = ask_questions_and_parse_answers(raw_options, prefilled_answers=args) + args = form.dict(exclude_none=True) # Validate domain / path availability for webapps # (ideally this should be handled by the resource system for manifest v >= 2 @@ -1141,15 +1137,15 @@ def app_install( "current_revision": manifest.get("remote", {}).get("revision", "?"), } - # If packaging_format v2+, save all install questions as settings + # If packaging_format v2+, save all install options as settings if packaging_format >= 2: - for question in questions: + for option in options: # Except user-provider passwords # ... which we need to reinject later in the env_dict - if question.type == "password": + if option.type == "password": continue - app_settings[question.id] = question.value + app_settings[option.id] = form[option.id] _set_app_settings(app_instance_name, app_settings) @@ -1202,23 +1198,23 @@ def app_install( app_instance_name, args=args, workdir=extracted_app_folder, action="install" ) - # If packaging_format v2+, save all install questions as settings + # If packaging_format v2+, save all install options as settings if packaging_format >= 2: - for question in questions: + for option in options: # Reinject user-provider passwords which are not in the app settings # (cf a few line before) - if question.type == "password": - env_dict[question.id] = question.value + if option.type == "password": + env_dict[option.id] = form[option.id] # We want to hav the env_dict in the log ... but not password values env_dict_for_logging = env_dict.copy() - for question in questions: - # Or should it be more generally question.redact ? - if question.type == "password": - if f"YNH_APP_ARG_{question.id.upper()}" in env_dict_for_logging: - del env_dict_for_logging[f"YNH_APP_ARG_{question.id.upper()}"] - if question.id in env_dict_for_logging: - del env_dict_for_logging[question.id] + for option in options: + # Or should it be more generally option.redact ? + if option.type == "password": + if f"YNH_APP_ARG_{option.id.upper()}" in env_dict_for_logging: + del env_dict_for_logging[f"YNH_APP_ARG_{option.id.upper()}"] + if option.id in env_dict_for_logging: + del env_dict_for_logging[option.id] operation_logger.extra.update({"env": env_dict_for_logging}) From 582b1ed311e5feb0823a2742a1f8b58c64a2165d Mon Sep 17 00:00:00 2001 From: axolotle Date: Mon, 17 Apr 2023 19:51:16 +0200 Subject: [PATCH 07/50] form: add translating method --- src/tests/test_questions.py | 10 ++++----- src/utils/form.py | 41 +++++++++++++++++++++++++------------ 2 files changed, 33 insertions(+), 18 deletions(-) diff --git a/src/tests/test_questions.py b/src/tests/test_questions.py index 1f8667adf..99ab9f156 100644 --- a/src/tests/test_questions.py +++ b/src/tests/test_questions.py @@ -447,7 +447,7 @@ class BaseTest: assert isinstance(option, OPTIONS[raw_option["type"]]) assert option.type == raw_option["type"] assert option.id == id_ - assert option.ask == {"en": id_} + assert option.ask == id_ assert option.readonly is (True if is_special_readonly_option else False) assert option.visible is True # assert option.bind is None @@ -493,7 +493,7 @@ class BaseTest: ) option, value = _fill_or_prompt_one_option(raw_option, None) - expected_message = option.ask["en"] + expected_message = option.ask choices = [] if isinstance(option, BaseChoicesOption): @@ -510,7 +510,7 @@ class BaseTest: prefill=prefill, is_multiline=option.type == "text", autocomplete=choices, - help=option.help["en"], + help=option.help, ) def test_scenarios(self, intake, expected_output, raw_option, data): @@ -558,7 +558,7 @@ class TestDisplayText(BaseTest): options, form = ask_questions_and_parse_answers( {_id: raw_option}, answers ) - assert stdout.getvalue() == f"{options[0].ask['en']}\n" + assert stdout.getvalue() == f"{options[0].ask}\n" # ╭───────────────────────────────────────────────────────╮ @@ -609,7 +609,7 @@ class TestAlert(TestDisplayText): options, form = ask_questions_and_parse_answers( {"display_text_id": raw_option}, answers ) - ask = options[0].ask["en"] + ask = options[0].ask if style in colors: color = colors[style] title = style.title() + (":" if style != "success" else "!") diff --git a/src/utils/form.py b/src/utils/form.py index cbe64b499..71954ee2b 100644 --- a/src/utils/form.py +++ b/src/utils/form.py @@ -297,15 +297,6 @@ class BaseOption(BaseModel): del schema["description"] schema["additionalProperties"] = False - @validator("ask", always=True) - def parse_or_set_default_ask( - cls, value: Union[Translation, None], values: Values - ) -> Translation: - if value is None: - return {"en": values["id"]} - if isinstance(value, str): - return {"en": value} - return value @validator("readonly", pre=True) def can_be_readonly(cls, value: bool, values: Values) -> bool: @@ -327,7 +318,9 @@ class BaseOption(BaseModel): return evaluate_simple_js_expression(self.visible, context=context) def _get_prompt_message(self, value: None) -> str: - return _value_for_locale(self.ask) + # force type to str + # `OptionsModel.translate_options()` should have been called before calling this method + return cast(str, self.ask) # ╭───────────────────────────────────────────────────────╮ @@ -367,7 +360,7 @@ class AlertOption(BaseReadonlyOption): State.danger: "red", } message = m18n.g(self.style) if self.style != State.danger else m18n.n("danger") - return f"{colorize(message, colors[self.style])} {_value_for_locale(self.ask)}" + return f"{colorize(message, colors[self.style])} {self.ask}" class ButtonOption(BaseReadonlyOption): @@ -624,6 +617,7 @@ class NumberOption(BaseInputOption): max: Union[int, None] = None step: Union[int, None] = None _annotation = int + _none_as_empty_str = False @staticmethod def normalize(value, option={}): @@ -1274,6 +1268,26 @@ class OptionsModel(BaseModel): def __init__(self, **kwargs) -> None: super().__init__(options=self.options_dict_to_list(kwargs)) + def translate_options(self, i18n_key: Union[str, None] = None): + """ + Mutate in place translatable attributes of options to their translations + """ + for option in self.options: + for key in ("ask", "help"): + if not hasattr(option, key): + continue + + value = getattr(option, key) + if value: + setattr(option, key, _value_for_locale(value)) + elif key == "ask" and m18n.key_exists(f"{i18n_key}_{option.id}"): + setattr(option, key, m18n.n(f"{i18n_key}_{option.id}")) + elif key == "help" and m18n.key_exists(f"{i18n_key}_{option.id}_help"): + setattr(option, key, m18n.n(f"{i18n_key}_{option.id}_help")) + elif key == "ask": + # FIXME warn? + option.ask = option.id + class FormModel(BaseModel): """ @@ -1384,7 +1398,7 @@ def prompt_or_validate_form( raise YunohostValidationError( "config_action_disabled", action=option.id, - help=_value_for_locale(option.help), + help=option.help, ) if not option.is_visible(context): @@ -1433,7 +1447,7 @@ def prompt_or_validate_form( prefill=value, is_multiline=isinstance(option, TextOption), autocomplete=choices, - help=_value_for_locale(option.help), + help=option.help, ) # Apply default value if none @@ -1512,6 +1526,7 @@ def ask_questions_and_parse_answers( # FIXME use YunohostError instead since it is not really a user mistake? raise YunohostValidationError(error, raw_msg=True) + model.translate_options() # Build the form from those questions and instantiate it without # parsing/validation (construct) since it may contains required questions. form = build_form(model.options).construct() From a574855a039b0ef39cd459690e0ac62c31ae7cda Mon Sep 17 00:00:00 2001 From: axolotle Date: Mon, 17 Apr 2023 20:01:31 +0200 Subject: [PATCH 08/50] form: fix forbidden readonly type --- src/tests/test_questions.py | 10 +++++----- src/utils/form.py | 3 +-- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/tests/test_questions.py b/src/tests/test_questions.py index 99ab9f156..959f2c8b7 100644 --- a/src/tests/test_questions.py +++ b/src/tests/test_questions.py @@ -741,7 +741,7 @@ class TestPassword(BaseTest): ("secret", FAIL), *[("supersecret" + char, FAIL) for char in FORBIDDEN_PASSWORD_CHARS], # FIXME maybe add ` \n` to the list? # readonly - ("s3cr3t!!", FAIL, {"readonly": True, "current_value": "isforbidden"}), # readonly is forbidden + ("s3cr3t!!", FAIL, {"readonly": True}), # readonly is forbidden ] # fmt: on @@ -1519,7 +1519,7 @@ class TestDomain(BaseTest): ("doesnt_exist.pouet", FAIL, {}), ("fake.com", FAIL, {"choices": ["fake.com"]}), # readonly - (domains1[0], YunohostError, {"readonly": True}), # readonly is forbidden + (domains1[0], FAIL, {"readonly": True}), # readonly is forbidden ] }, { @@ -1619,7 +1619,7 @@ class TestApp(BaseTest): (installed_non_webapp["id"], installed_non_webapp["id"]), (installed_non_webapp["id"], FAIL, {"filter": "is_webapp"}), # readonly - (installed_non_webapp["id"], YunohostError, {"readonly": True}), # readonly is forbidden + (installed_non_webapp["id"], FAIL, {"readonly": True}), # readonly is forbidden ] }, ] @@ -1736,7 +1736,7 @@ class TestUser(BaseTest): ("", regular_username, {"default": regular_username}) ], reason="Should throw 'no default allowed'"), # readonly - (admin_username, YunohostError, {"readonly": True}), # readonly is forbidden + (admin_username, FAIL, {"readonly": True}), # readonly is forbidden ] }, ] @@ -1820,7 +1820,7 @@ class TestGroup(BaseTest): ("", "custom_group", {"default": "custom_group"}), ], reason="Should throw 'default must be in (None, 'all_users', 'visitors', 'admins')"), # readonly - ("admins", "all_users", {"readonly": True}), # readonly is forbidden (default is "all_users") + ("admins", FAIL, {"readonly": True}), # readonly is forbidden ] }, ] diff --git a/src/utils/form.py b/src/utils/form.py index 71954ee2b..6246a91e5 100644 --- a/src/utils/form.py +++ b/src/utils/form.py @@ -300,8 +300,7 @@ class BaseOption(BaseModel): @validator("readonly", pre=True) def can_be_readonly(cls, value: bool, values: Values) -> bool: - forbidden_types = ("password", "app", "domain", "user", "file") - if value is True and values["type"] in forbidden_types: + if value is True and values["type"] in FORBIDDEN_READONLY_TYPES: raise ValueError( m18n.n( "config_forbidden_readonly_type", From 774b11cbbeffcc2480b146f8557143876d992509 Mon Sep 17 00:00:00 2001 From: axolotle Date: Mon, 17 Apr 2023 20:02:02 +0200 Subject: [PATCH 09/50] form: add legacy "name" attr --- src/utils/form.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/utils/form.py b/src/utils/form.py index 6246a91e5..7e009c5b5 100644 --- a/src/utils/form.py +++ b/src/utils/form.py @@ -285,6 +285,7 @@ class BaseOption(BaseModel): readonly: bool = False visible: Union[JSExpression, bool] = True bind: Union[str, None] = None + name: Union[str, None] = None # LEGACY (replaced by `id`) class Config: arbitrary_types_allowed = True @@ -297,6 +298,12 @@ class BaseOption(BaseModel): del schema["description"] schema["additionalProperties"] = False + # FIXME Legacy, is `name` still needed? + @validator("name", pre=True, always=True) + def apply_legacy_name(cls, value: Union[str, None], values: Values) -> str: + if value is None: + return values["id"] + return value @validator("readonly", pre=True) def can_be_readonly(cls, value: bool, values: Values) -> bool: From bec34b92b065f964b88be37eef7c79bafa4598fc Mon Sep 17 00:00:00 2001 From: axolotle Date: Mon, 17 Apr 2023 20:03:35 +0200 Subject: [PATCH 10/50] form: add reserved "id" validator --- src/utils/configpanel.py | 25 ------------------------- src/utils/form.py | 27 ++++++++++++++++++++++++++- 2 files changed, 26 insertions(+), 26 deletions(-) diff --git a/src/utils/configpanel.py b/src/utils/configpanel.py index 86dea2e7d..53cd4b9c8 100644 --- a/src/utils/configpanel.py +++ b/src/utils/configpanel.py @@ -455,31 +455,6 @@ class ConfigPanel: "config_unknown_filter_key", filter_key=self.filter_key ) - # List forbidden keywords from helpers and sections toml (to avoid conflict) - forbidden_keywords = [ - "old", - "app", - "changed", - "file_hash", - "binds", - "types", - "formats", - "getter", - "setter", - "short_setting", - "type", - "bind", - "nothing_changed", - "changes_validated", - "result", - "max_progression", - ] - forbidden_keywords += format_description["sections"] - - for _, _, option in self._iterate(): - if option["id"] in forbidden_keywords: - raise YunohostError("config_forbidden_keyword", keyword=option["id"]) - return self.config def _get_default_values(self): diff --git a/src/utils/form.py b/src/utils/form.py index 7e009c5b5..6c14bcdf0 100644 --- a/src/utils/form.py +++ b/src/utils/form.py @@ -265,7 +265,26 @@ FORBIDDEN_READONLY_TYPES = { OptionType.user, OptionType.group, } - +FORBIDDEN_KEYWORDS = { + "old", + "app", + "changed", + "file_hash", + "binds", + "types", + "formats", + "getter", + "setter", + "short_setting", + "type", + "bind", + "nothing_changed", + "changes_validated", + "result", + "max_progression", + "properties", + "defaults", +} Context = dict[str, Any] Translation = Union[dict[str, str], str] @@ -298,6 +317,12 @@ class BaseOption(BaseModel): del schema["description"] schema["additionalProperties"] = False + @validator("id", pre=True) + def check_id_is_not_forbidden(cls, value: str) -> str: + if value in FORBIDDEN_KEYWORDS: + raise ValueError(m18n.n("config_forbidden_keyword", keyword=value)) + return value + # FIXME Legacy, is `name` still needed? @validator("name", pre=True, always=True) def apply_legacy_name(cls, value: Union[str, None], values: Values) -> str: From 564a66de2fab6b29b6d4457ea5e5b9084fdfbb0c Mon Sep 17 00:00:00 2001 From: axolotle Date: Mon, 17 Apr 2023 20:08:52 +0200 Subject: [PATCH 11/50] configpanel: add config panel models --- src/utils/configpanel.py | 192 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 190 insertions(+), 2 deletions(-) diff --git a/src/utils/configpanel.py b/src/utils/configpanel.py index 53cd4b9c8..bf441798c 100644 --- a/src/utils/configpanel.py +++ b/src/utils/configpanel.py @@ -21,8 +21,10 @@ import os import re import urllib.parse from collections import OrderedDict -from typing import Union from logging import getLogger +from typing import TYPE_CHECKING, Any, Literal, Sequence, Type, Union + +from pydantic import BaseModel, Extra, validator from moulinette import Moulinette, m18n from moulinette.interfaces.cli import colorize @@ -30,20 +32,206 @@ from moulinette.utils.filesystem import mkdir, read_toml, read_yaml, write_to_ya from yunohost.utils.error import YunohostError, YunohostValidationError from yunohost.utils.form import ( OPTIONS, - BaseChoicesOption, BaseInputOption, BaseOption, + BaseReadonlyOption, FileOption, + OptionsModel, OptionType, + Translation, ask_questions_and_parse_answers, evaluate_simple_js_expression, ) from yunohost.utils.i18n import _value_for_locale +if TYPE_CHECKING: + from pydantic.fields import ModelField + logger = getLogger("yunohost.configpanel") + + +# ╭───────────────────────────────────────────────────────╮ +# │ ╭╮╮╭─╮┌─╮┌─╴╷ ╭─╴ │ +# │ ││││ ││ │├─╴│ ╰─╮ │ +# │ ╵╵╵╰─╯└─╯╰─╴╰─╴╶─╯ │ +# ╰───────────────────────────────────────────────────────╯ + CONFIG_PANEL_VERSION_SUPPORTED = 1.0 +class ContainerModel(BaseModel): + id: str + name: Union[Translation, None] = None + services: list[str] = [] + help: Union[Translation, None] = None + + def translate(self, i18n_key: Union[str, None] = None): + """ + Translate `ask` and `name` attributes of panels and section. + This is in place mutation. + """ + + for key in ("help", "name"): + value = getattr(self, key) + if value: + setattr(self, key, _value_for_locale(value)) + elif key == "help" and m18n.key_exists(f"{i18n_key}_{self.id}_help"): + setattr(self, key, m18n.n(f"{i18n_key}_{self.id}_help")) + + +class SectionModel(ContainerModel, OptionsModel): + visible: Union[bool, str] = True + optional: bool = True + + # Don't forget to pass arguments to super init + def __init__( + self, + id: str, + name: Union[Translation, None] = None, + services: list[str] = [], + help: Union[Translation, None] = None, + visible: Union[bool, str] = True, + **kwargs, + ) -> None: + options = self.options_dict_to_list(kwargs, defaults={"optional": True}) + + ContainerModel.__init__( + self, + id=id, + name=name, + services=services, + help=help, + visible=visible, + options=options, + ) + + @property + def is_action_section(self): + return any([option.type is OptionType.button for option in self.options]) + + def is_visible(self, context: dict[str, Any]): + if isinstance(self.visible, bool): + return self.visible + + return evaluate_simple_js_expression(self.visible, context=context) + + def translate(self, i18n_key: Union[str, None] = None): + """ + Call to `Container`'s `translate` for self translation + + Call to `OptionsContainer`'s `translate_options` for options translation + """ + super().translate(i18n_key) + self.translate_options(i18n_key) + + +class PanelModel(ContainerModel): + # FIXME what to do with `actions? + actions: dict[str, Translation] = {"apply": {"en": "Apply"}} + sections: list[SectionModel] + + class Config: + extra = Extra.allow + + # Don't forget to pass arguments to super init + def __init__( + self, + id: str, + name: Union[Translation, None] = None, + services: list[str] = [], + help: Union[Translation, None] = None, + **kwargs, + ) -> None: + sections = [data | {"id": name} for name, data in kwargs.items()] + super().__init__( + id=id, name=name, services=services, help=help, sections=sections + ) + + def translate(self, i18n_key: Union[str, None] = None): + """ + Recursivly mutate translatable attributes to their translation + """ + super().translate(i18n_key) + + for section in self.sections: + section.translate(i18n_key) + + +class ConfigPanelModel(BaseModel): + version: float = CONFIG_PANEL_VERSION_SUPPORTED + i18n: Union[str, None] = None + panels: list[PanelModel] + + class Config: + arbitrary_types_allowed = True + extra = Extra.allow + + # Don't forget to pass arguments to super init + def __init__( + self, + version: float, + i18n: Union[str, None] = None, + **kwargs, + ) -> None: + panels = [data | {"id": name} for name, data in kwargs.items()] + super().__init__(version=version, i18n=i18n, panels=panels) + + @property + def sections(self): + """Convinient prop to iter on all sections""" + for panel in self.panels: + for section in panel.sections: + yield section + + @property + def options(self): + """Convinient prop to iter on all options""" + for section in self.sections: + for option in section.options: + yield option + + + def iter_children( + self, + trigger: list[Literal["panel", "section", "option", "action"]] = ["option"], + ): + for panel in self.panels: + if "panel" in trigger: + yield (panel, None, None) + for section in panel.sections: + if "section" in trigger: + yield (panel, section, None) + if "action" in trigger: + for option in section.options: + if option.type is OptionType.button: + yield (panel, section, option) + if "option" in trigger: + for option in section.options: + yield (panel, section, option) + + def translate(self): + """ + Recursivly mutate translatable attributes to their translation + """ + for panel in self.panels: + panel.translate(self.i18n) + + @validator("version", always=True) + def check_version(cls, value, field: "ModelField"): + if value < CONFIG_PANEL_VERSION_SUPPORTED: + raise ValueError( + f"Config panels version '{value}' are no longer supported." + ) + + return value + + +# ╭───────────────────────────────────────────────────────╮ +# │ ╭─╴╭─╮╭╮╷┌─╴╶┬╴╭─╮ ╶┬╴╭╮╮┌─╮╷ │ +# │ │ │ ││││├─╴ │ │╶╮ │ │││├─╯│ │ +# │ ╰─╴╰─╯╵╰╯╵ ╶┴╴╰─╯ ╶┴╴╵╵╵╵ ╰─╴ │ +# ╰───────────────────────────────────────────────────────╯ + + class ConfigPanel: entity_type = "config" save_path_tpl: Union[str, None] = None From 02948ad49c909f3d8eaed9bf3fdde0aada5bf11a Mon Sep 17 00:00:00 2001 From: axolotle Date: Tue, 18 Apr 2023 01:26:41 +0200 Subject: [PATCH 12/50] config: rework config+settings getter methods --- src/app.py | 10 +- src/dns.py | 10 +- src/domain.py | 59 ++++--- src/settings.py | 26 ++-- src/tests/test_dns.py | 6 +- src/utils/configpanel.py | 327 +++++++++++++-------------------------- src/utils/form.py | 4 +- 7 files changed, 166 insertions(+), 276 deletions(-) diff --git a/src/app.py b/src/app.py index 1a2e80442..ffcd1ecc3 100644 --- a/src/app.py +++ b/src/app.py @@ -26,7 +26,8 @@ import re import subprocess import tempfile import copy -from typing import List, Tuple, Dict, Any, Iterator, Optional +from collections import OrderedDict +from typing import TYPE_CHECKING, List, Tuple, Dict, Any, Iterator, Optional from packaging import version from logging import getLogger from pathlib import Path @@ -71,6 +72,9 @@ from yunohost.app_catalog import ( # noqa APPS_CATALOG_LOGOS, ) +if TYPE_CHECKING: + from yunohost.utils.configpanel import ConfigPanelModel, RawSettings + logger = getLogger("yunohost.app") APPS_SETTING_PATH = "/etc/yunohost/apps/" @@ -1802,8 +1806,8 @@ class AppConfigPanel(ConfigPanel): env = {key: str(value) for key, value in self.new_values.items()} self._call_config_script(action, env=env) - def _get_raw_settings(self): - self.values = self._call_config_script("show") + def _get_raw_settings(self, config: "ConfigPanelModel") -> "RawSettings": + return self._call_config_script("show") def _apply(self): env = {key: str(value) for key, value in self.new_values.items()} diff --git a/src/dns.py b/src/dns.py index 9a081e228..07ff2fb21 100644 --- a/src/dns.py +++ b/src/dns.py @@ -528,7 +528,7 @@ def _get_registrar_config_section(domain): parent_domain=parent_domain, parent_domain_link=parent_domain_link, ), - "value": "parent_domain", + "default": "parent_domain", } ) return OrderedDict(registrar_infos) @@ -541,7 +541,7 @@ def _get_registrar_config_section(domain): "type": "alert", "style": "success", "ask": m18n.n("domain_dns_registrar_yunohost"), - "value": "yunohost", + "default": "yunohost", } ) return OrderedDict(registrar_infos) @@ -551,7 +551,7 @@ def _get_registrar_config_section(domain): "type": "alert", "style": "info", "ask": m18n.n("domain_dns_conf_special_use_tld"), - "value": None, + "default": None, } ) @@ -563,7 +563,7 @@ def _get_registrar_config_section(domain): "type": "alert", "style": "warning", "ask": m18n.n("domain_dns_registrar_not_supported"), - "value": None, + "default": None, } ) else: @@ -572,7 +572,7 @@ def _get_registrar_config_section(domain): "type": "alert", "style": "info", "ask": m18n.n("domain_dns_registrar_supported", registrar=registrar), - "value": registrar, + "default": registrar, } ) diff --git a/src/domain.py b/src/domain.py index 2a897c625..a796e0142 100644 --- a/src/domain.py +++ b/src/domain.py @@ -19,7 +19,7 @@ import os import time from pathlib import Path -from typing import List, Optional +from typing import TYPE_CHECKING, List, Optional from collections import OrderedDict from logging import getLogger @@ -47,6 +47,9 @@ from yunohost.utils.error import YunohostError, YunohostValidationError from yunohost.utils.dns import is_yunohost_dyndns_domain from yunohost.log import is_unit_operation +if TYPE_CHECKING: + from yunohost.utils.configpanel import RawConfig + logger = getLogger("yunohost.domain") DOMAIN_SETTINGS_DIR = "/etc/yunohost/domains" @@ -666,10 +669,14 @@ class DomainConfigPanel(ConfigPanel): return result - def _get_raw_config(self): - toml = super()._get_raw_config() + def _get_raw_config(self) -> "RawConfig": + # TODO add mechanism to share some settings with other domains on the same zone + raw_config = super()._get_raw_config() - toml["feature"]["xmpp"]["xmpp"]["default"] = ( + any_filter = all(self.filter_key) + panel_id, section_id, option_id = self.filter_key + + raw_config["feature"]["xmpp"]["xmpp"]["default"] = ( 1 if self.entity == _get_maindomain() else 0 ) @@ -680,55 +687,43 @@ class DomainConfigPanel(ConfigPanel): # 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": + if not any_filter or panel_id == "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 _get_raw_settings ... - self.registar_id = toml["dns"]["registrar"]["registrar"]["value"] - del toml["dns"]["registrar"]["registrar"]["value"] + raw_config["dns"]["registrar"] = _get_registrar_config_section(self.entity) # Cert stuff - if not filter_key or filter_key[0] == "cert": + if not any_filter or panel_id == "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"] + raw_config["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( + raw_config["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 _get_raw_settings ... - self.cert_status = status + for option_id, status_key in [ + ("cert_validity", "validity"), + ("cert_issuer", "CA_type"), + ("acme_eligible", "ACME_eligible"), + # FIXME not sure why "summary" was injected in settings values + # ("summary", "summary") + ]: + raw_config["cert"]["cert"][option_id]["default"] = status[status_key] - return toml + # Other specific strings used in config panels + # i18n: domain_config_cert_renew_help - def _get_raw_settings(self): - # TODO add mechanism to share some settings with other domains on the same zone - super()._get_raw_settings() - - # FIXME: Ugly hack to save the registar id/value and reinject it in _get_raw_settings ... - 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 _get_raw_settings ... - 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"] + return raw_config def _apply(self): if ( diff --git a/src/settings.py b/src/settings.py index e2f34bda9..f3340e8e9 100644 --- a/src/settings.py +++ b/src/settings.py @@ -19,6 +19,7 @@ import os import subprocess from logging import getLogger +from typing import TYPE_CHECKING from moulinette import m18n from yunohost.utils.error import YunohostError, YunohostValidationError @@ -29,6 +30,9 @@ from yunohost.firewall import firewall_reload from yunohost.log import is_unit_operation from yunohost.utils.legacy import translate_legacy_settings_to_configpanel_settings +if TYPE_CHECKING: + from yunohost.utils.configpanel import ConfigPanelModel, RawConfig, RawSettings + logger = getLogger("yunohost.settings") SETTINGS_PATH = "/etc/yunohost/settings.yml" @@ -180,8 +184,8 @@ class SettingsConfigPanel(ConfigPanel): logger.success(m18n.n("global_settings_reset_success")) operation_logger.success() - def _get_raw_config(self): - toml = super()._get_raw_config() + def _get_raw_config(self) -> "RawConfig": + raw_config = super()._get_raw_config() # Dynamic choice list for portal themes THEMEDIR = "/usr/share/ssowat/portal/assets/themes/" @@ -189,28 +193,30 @@ class SettingsConfigPanel(ConfigPanel): 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 + raw_config["misc"]["portal"]["portal_theme"]["choices"] = themes - return toml + return raw_config - def _get_raw_settings(self): - super()._get_raw_settings() + def _get_raw_settings(self, config: "ConfigPanelModel") -> "RawSettings": + raw_settings = super()._get_raw_settings(config) # 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"] = "" + raw_settings["root_password"] = "" + raw_settings["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( + raw_settings["passwordless_sudo"] = "!authenticate" in ldap.search( "ou=sudo", "cn=admins", ["sudoOption"] )[0].get("sudoOption", []) except Exception: - self.values["passwordless_sudo"] = False + raw_settings["passwordless_sudo"] = False + + return raw_settings def _apply(self): root_password = self.new_values.pop("root_password", None) diff --git a/src/tests/test_dns.py b/src/tests/test_dns.py index e896d9c9f..744e3e789 100644 --- a/src/tests/test_dns.py +++ b/src/tests/test_dns.py @@ -49,19 +49,19 @@ def test_registrar_list_integrity(): def test_magic_guess_registrar_weird_domain(): - assert _get_registrar_config_section("yolo.tld")["registrar"]["value"] is None + assert _get_registrar_config_section("yolo.tld")["registrar"]["default"] is None def test_magic_guess_registrar_ovh(): assert ( - _get_registrar_config_section("yolo.yunohost.org")["registrar"]["value"] + _get_registrar_config_section("yolo.yunohost.org")["registrar"]["default"] == "ovh" ) def test_magic_guess_registrar_yunodyndns(): assert ( - _get_registrar_config_section("yolo.nohost.me")["registrar"]["value"] + _get_registrar_config_section("yolo.nohost.me")["registrar"]["default"] == "yunohost" ) diff --git a/src/utils/configpanel.py b/src/utils/configpanel.py index bf441798c..e113d007b 100644 --- a/src/utils/configpanel.py +++ b/src/utils/configpanel.py @@ -31,15 +31,16 @@ from moulinette.interfaces.cli import colorize from moulinette.utils.filesystem import mkdir, read_toml, read_yaml, write_to_yaml from yunohost.utils.error import YunohostError, YunohostValidationError from yunohost.utils.form import ( - OPTIONS, BaseInputOption, BaseOption, BaseReadonlyOption, FileOption, + FormModel, OptionsModel, OptionType, Translation, ask_questions_and_parse_answers, + build_form, evaluate_simple_js_expression, ) from yunohost.utils.i18n import _value_for_locale @@ -93,7 +94,7 @@ class SectionModel(ContainerModel, OptionsModel): visible: Union[bool, str] = True, **kwargs, ) -> None: - options = self.options_dict_to_list(kwargs, defaults={"optional": True}) + options = self.options_dict_to_list(kwargs, optional=True) ContainerModel.__init__( self, @@ -231,12 +232,33 @@ class ConfigPanelModel(BaseModel): # │ ╰─╴╰─╯╵╰╯╵ ╶┴╴╰─╯ ╶┴╴╵╵╵╵ ╰─╴ │ # ╰───────────────────────────────────────────────────────╯ +if TYPE_CHECKING: + FilterKey = Sequence[Union[str, None]] + RawConfig = OrderedDict[str, Any] + RawSettings = dict[str, Any] + + +def parse_filter_key(key: Union[str, None] = None) -> "FilterKey": + if key and key.count(".") > 2: + raise YunohostError( + f"The filter key {key} has too many sub-levels, the max is 3.", + raw_msg=True, + ) + + if not key: + return (None, None, None) + keys = key.split(".") + return tuple(keys[i] if len(keys) > i else None for i in range(3)) + class ConfigPanel: entity_type = "config" save_path_tpl: Union[str, None] = None config_path_tpl = "/usr/share/yunohost/config_{entity_type}.toml" save_mode = "full" + filter_key: "FilterKey" = (None, None, None) + config: Union[ConfigPanelModel, None] = None + form: Union[FormModel, None] = None @classmethod def list(cls): @@ -265,9 +287,6 @@ class ConfigPanel: self.save_path = save_path if not save_path and self.save_path_tpl: self.save_path = self.save_path_tpl.format(entity=entity) - self.config = {} - self.values = {} - self.new_values = {} if ( self.save_path @@ -501,215 +520,103 @@ class ConfigPanel: logger.success(f"Action {action_id} successful") operation_logger.success() - def _get_raw_config(self): + def _get_raw_config(self) -> "RawConfig": + if not os.path.exists(self.config_path): + raise YunohostValidationError("config_no_panel") + return read_toml(self.config_path) - def _get_config_panel(self): - # Split filter_key - filter_key = self.filter_key.split(".") if self.filter_key != "" else [] - if len(filter_key) > 3: - raise YunohostError( - f"The filter key {filter_key} has too many sub-levels, the max is 3.", - raw_msg=True, + def _get_raw_settings(self, config: ConfigPanelModel) -> "RawSettings": + if not self.save_path or not os.path.exists(self.save_path): + raise YunohostValidationError("config_no_settings") + + return read_yaml(self.save_path) + + def _get_partial_raw_config(self) -> "RawConfig": + def filter_keys( + data: "RawConfig", + key: str, + model: Union[Type[ConfigPanelModel], Type[PanelModel], Type[SectionModel]], + ) -> "RawConfig": + # filter in keys defined in model, filter out panels/sections/options that aren't `key` + return OrderedDict( + {k: v for k, v in data.items() if k in model.__fields__ or k == key} ) - if not os.path.exists(self.config_path): - logger.debug(f"Config panel {self.config_path} doesn't exists") - return None + raw_config = self._get_raw_config() - toml_config_panel = self._get_raw_config() + panel_id, section_id, option_id = self.filter_key + if panel_id: + raw_config = filter_keys(raw_config, panel_id, ConfigPanelModel) - # Check TOML config panel is in a supported version - if float(toml_config_panel["version"]) < CONFIG_PANEL_VERSION_SUPPORTED: - logger.error( - f"Config panels version {toml_config_panel['version']} are not supported" - ) - return None + if section_id: + raw_config[panel_id] = filter_keys( + raw_config[panel_id], section_id, PanelModel + ) - # Transform toml format into internal format - format_description = { - "root": { - "properties": ["version", "i18n"], - "defaults": {"version": 1.0}, - }, - "panels": { - "properties": ["name", "services", "actions", "help"], - "defaults": { - "services": [], - "actions": {"apply": {"en": "Apply"}}, - }, - }, - "sections": { - "properties": ["name", "services", "optional", "help", "visible"], - "defaults": { - "name": "", - "services": [], - "optional": True, - "is_action_section": False, - }, - }, - "options": { - "properties": [ - "ask", - "type", - "bind", - "help", - "example", - "default", - "style", - "icon", - "placeholder", - "visible", - "optional", - "choices", - "yes", - "no", - "pattern", - "limit", - "min", - "max", - "step", - "accept", - "redact", - "filter", - "readonly", - "enabled", - "add_yunohost_portal_to_choices", - # "confirm", # TODO: to ask confirmation before running an action - ], - "defaults": {}, - }, - } - - def _build_internal_config_panel(raw_infos, level): - """Convert TOML in internal format ('full' mode used by webadmin) - Here are some properties of 1.0 config panel in toml: - - node properties and node children are mixed, - - text are in english only - - some properties have default values - This function detects all children nodes and put them in a list - """ - - defaults = format_description[level]["defaults"] - properties = format_description[level]["properties"] - - # Start building the ouput (merging the raw infos + defaults) - out = {key: raw_infos.get(key, value) for key, value in defaults.items()} - - # Now fill the sublevels (+ apply filter_key) - i = list(format_description).index(level) - sublevel = list(format_description)[i + 1] if level != "options" else None - search_key = filter_key[i] if len(filter_key) > i else False - - for key, value in raw_infos.items(): - # Key/value are a child node - if ( - isinstance(value, OrderedDict) - and key not in properties - and sublevel - ): - # We exclude all nodes not referenced by the filter_key - if search_key and key != search_key: - continue - subnode = _build_internal_config_panel(value, sublevel) - subnode["id"] = key - if level == "root": - subnode.setdefault("name", {"en": key.capitalize()}) - elif level == "sections": - subnode["name"] = key # legacy - subnode.setdefault("optional", raw_infos.get("optional", True)) - # If this section contains at least one button, it becomes an "action" section - if subnode.get("type") == OptionType.button: - out["is_action_section"] = True - out.setdefault(sublevel, []).append(subnode) - # Key/value are a property - else: - if key not in properties: - logger.warning(f"Unknown key '{key}' found in config panel") - # Todo search all i18n keys - out[key] = ( - value - if key not in ["ask", "help", "name"] or isinstance(value, dict) - else {"en": value} + if option_id: + raw_config[panel_id][section_id] = filter_keys( + raw_config[panel_id][section_id], option_id, SectionModel ) - return out - self.config = _build_internal_config_panel(toml_config_panel, "root") + return raw_config + + def _get_partial_raw_settings_and_mutate_config( + self, config: ConfigPanelModel + ) -> tuple[ConfigPanelModel, "RawSettings"]: + raw_settings = self._get_raw_settings(config) + values = {} + + for _, section, option in config.iter_children(): + value = data = raw_settings.get(option.id, getattr(option, "default", None)) + + if isinstance(data, dict): + # Settings data if gathered from bash "ynh_app_config_show" + # may be a custom getter that returns a dict with `value` or `current_value` + # and other attributes meant to override those of the option. + + if "value" in data: + value = data.pop("value") + + # Allow to use value instead of current_value in app config script. + # e.g. apps may write `echo 'value: "foobar"'` in the config file (which is more intuitive that `echo 'current_value: "foobar"'` + # For example hotspot used it... + # See https://github.com/YunoHost/yunohost/pull/1546 + # FIXME do we still need the `current_value`? + if "current_value" in data: + value = data.pop("current_value") + + # Mutate other possible option attributes + for k, v in data.items(): + setattr(option, k, v) + + if isinstance(option, BaseInputOption): # or option.bind == "null": + values[option.id] = value + + return (config, values) + + def _get_config_panel( + self, prevalidate: bool = False + ) -> tuple[ConfigPanelModel, FormModel]: + raw_config = self._get_partial_raw_config() + config = ConfigPanelModel(**raw_config) + config, raw_settings = self._get_partial_raw_settings_and_mutate_config(config) + config.translate() + Settings = build_form(config.options) + settings = ( + Settings(**raw_settings) + if prevalidate + else Settings.construct(**raw_settings) + ) try: - self.config["panels"][0]["sections"][0]["options"][0] + config.panels[0].sections[0].options[0] except (KeyError, IndexError): raise YunohostValidationError( "config_unknown_filter_key", filter_key=self.filter_key ) - return self.config - - def _get_default_values(self): - return { - option["id"]: option["default"] - for _, _, option in self._iterate() - if "default" in option - } - - def _get_raw_settings(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(): - if option["id"] not in self.values: - allowed_empty_types = { - OptionType.alert, - OptionType.display_text, - OptionType.markdown, - OptionType.file, - OptionType.button, - } - - if section["is_action_section"] and option.get("default") is not None: - self.values[option["id"]] = option["default"] - elif ( - option["type"] in allowed_empty_types - or option.get("bind") == "null" - ): - continue - else: - raise YunohostError( - f"Config panel question '{option['id']}' should be initialized with a value during install or upgrade.", - raw_msg=True, - ) - value = self.values[option["id"]] - - # Allow to use value instead of current_value in app config script. - # e.g. apps may write `echo 'value: "foobar"'` in the config file (which is more intuitive that `echo 'current_value: "foobar"'` - # For example hotspot used it... - # See https://github.com/YunoHost/yunohost/pull/1546 - if ( - isinstance(value, dict) - and "value" in value - and "current_value" not in value - ): - value["current_value"] = value["value"] - - # In general, the value is just a simple value. - # Sometimes it could be a dict used to overwrite the option itself - value = value if isinstance(value, dict) else {"current_value": value} - option.update(value) - - self.values[option["id"]] = value.get("current_value") - - return self.values + return (config, settings) def _ask(self, action=None): logger.debug("Ask unanswered question and prevalidate data") @@ -781,19 +688,6 @@ class ConfigPanel: } ) - @property - def future_values(self): - return {**self.values, **self.new_values} - - def __getattr__(self, name): - if "new_values" in self.__dict__ and name in self.new_values: - return self.new_values[name] - - if "values" in self.__dict__ and name in self.values: - return self.values[name] - - return self.__dict__[name] - 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()} @@ -836,14 +730,3 @@ class ConfigPanel: if hasattr(self, "entity"): service = service.replace("__APP__", self.entity) service_reload_or_restart(service) - - def _iterate(self, trigger=["option"]): - for panel in self.config.get("panels", []): - if "panel" in trigger: - yield (panel, None, panel) - for section in panel.get("sections", []): - if "section" in trigger: - yield (panel, section, section) - if "option" in trigger: - for option in section.get("options", []): - yield (panel, section, option) diff --git a/src/utils/form.py b/src/utils/form.py index 6c14bcdf0..9ca0393d0 100644 --- a/src/utils/form.py +++ b/src/utils/form.py @@ -1286,12 +1286,14 @@ class OptionsModel(BaseModel): options: list[Annotated[AnyOption, Field(discriminator="type")]] @staticmethod - def options_dict_to_list(options: dict[str, Any], defaults: dict[str, Any] = {}): + def options_dict_to_list(options: dict[str, Any], optional: bool = False): return [ option | { "id": id_, "type": option.get("type", "string"), + # ConfigPanel options needs to be set as optional by default + "optional": option.get("optional", optional) } for id_, option in options.items() ] From 80dbd6dac46cbe2848a063758f29f3b369417529 Mon Sep 17 00:00:00 2001 From: axolotle Date: Tue, 18 Apr 2023 15:25:40 +0200 Subject: [PATCH 13/50] form: rework entities validators to avoid multiple calls to them --- src/utils/form.py | 90 +++++++++++++++++++++++++++++------------------ 1 file changed, 55 insertions(+), 35 deletions(-) diff --git a/src/utils/form.py b/src/utils/form.py index 9ca0393d0..8ed83393e 100644 --- a/src/utils/form.py +++ b/src/utils/form.py @@ -44,7 +44,6 @@ from pydantic import ( Extra, ValidationError, create_model, - root_validator, validator, ) from pydantic.color import Color @@ -324,7 +323,7 @@ class BaseOption(BaseModel): return value # FIXME Legacy, is `name` still needed? - @validator("name", pre=True, always=True) + @validator("name") def apply_legacy_name(cls, value: Union[str, None], values: Values) -> str: if value is None: return values["id"] @@ -1096,21 +1095,30 @@ class DomainOption(BaseChoicesOption): type: Literal[OptionType.domain] = OptionType.domain choices: Union[dict[str, str], None] - @root_validator() - def inject_domains_choices_and_default(cls, values: Values) -> Values: + @validator("choices", pre=True, always=True) + def inject_domains_choices( + cls, value: Union[dict[str, str], None], values: Values + ) -> dict[str, str]: # TODO remove calls to resources in validators (pydantic V2 should adress this) from yunohost.domain import domain_list data = domain_list() - values["choices"] = { + return { domain: domain + " ★" if domain == data["main"] else domain for domain in data["domains"] } - if values["default"] is None: - values["default"] = data["main"] + @validator("default") + def inject_default( + cls, value: Union[str, None], values: Values + ) -> Union[str, None]: + # TODO remove calls to resources in validators (pydantic V2 should adress this) + from yunohost.domain import _get_maindomain - return values + if value is None: + return _get_maindomain() + + return value @staticmethod def normalize(value, option={}): @@ -1131,8 +1139,11 @@ class AppOption(BaseChoicesOption): add_yunohost_portal_to_choices: bool = False filter: Union[str, None] = None - @root_validator() - def inject_apps_choices(cls, values: Values) -> Values: + @validator("choices", pre=True, always=True) + def inject_apps_choices( + cls, value: Union[dict[str, str], None], values: Values + ) -> dict[str, str]: + # TODO remove calls to resources in validators (pydantic V2 should adress this) from yunohost.app import app_list apps = app_list(full=True)["apps"] @@ -1143,61 +1154,77 @@ class AppOption(BaseChoicesOption): for app in apps if evaluate_simple_js_expression(values["filter"], context=app) ] - values["choices"] = {"_none": "---"} + + value = {"_none": "---"} if values.get("add_yunohost_portal_to_choices", False): - values["choices"]["_yunohost_portal_with_public_apps"] = "YunoHost's portal with public apps" + value["_yunohost_portal_with_public_apps"] = "YunoHost's portal with public apps" - values["choices"].update( + value.update( { app["id"]: f"{app['label']} ({app.get('domain_path', app['id'])})" for app in apps } ) - return values + return value class UserOption(BaseChoicesOption): type: Literal[OptionType.user] = OptionType.user choices: Union[dict[str, str], None] - @root_validator() - def inject_users_choices_and_default(cls, values: dict[str, Any]) -> dict[str, Any]: - from yunohost.domain import _get_maindomain - from yunohost.user import user_info, user_list + @validator("choices", pre=True, always=True) + def inject_users_choices( + cls, value: Union[dict[str, str], None], values: Values + ) -> dict[str, str]: + # TODO remove calls to resources in validators (pydantic V2 should adress this) + from yunohost.user import user_list - values["choices"] = { + value = { username: f"{infos['fullname']} ({infos['mail']})" for username, infos in user_list()["users"].items() } # FIXME keep this to test if any user, do not raise error if no admin? - if not values["choices"]: + if not value: raise YunohostValidationError( "app_argument_invalid", name=values["id"], error="You should create a YunoHost user first.", ) - if values["default"] is None: + return value + + @validator("default") + def inject_default( + cls, value: Union[str, None], values: Values + ) -> Union[str, None]: + # TODO remove calls to resources in validators (pydantic V2 should adress this) + from yunohost.domain import _get_maindomain + from yunohost.user import user_info + + if value is None: # FIXME: this code is obsolete with the new admins group # Should be replaced by something like "any first user we find in the admin group" root_mail = "root@%s" % _get_maindomain() for user in values["choices"].keys(): if root_mail in user_info(user).get("mail-aliases", []): - values["default"] = user - break + return user - return values + return value class GroupOption(BaseChoicesOption): type: Literal[OptionType.group] = OptionType.group choices: Union[dict[str, str], None] + default: Union[Literal["visitors", "all_users", "admins"], None] = "all_users" - @root_validator() - def inject_groups_choices_and_default(cls, values: Values) -> Values: + @validator("choices", pre=True, always=True) + def inject_groups_choices( + cls, value: Union[dict[str, str], None], values: Values + ) -> dict[str, str]: + # TODO remove calls to resources in validators (pydantic V2 should adress this) from yunohost.user import user_group_list groups = user_group_list(short=True, include_primary_groups=False)["groups"] @@ -1212,14 +1239,7 @@ class GroupOption(BaseChoicesOption): else groupname ) - values["choices"] = { - groupname: _human_readable_group(groupname) for groupname in groups - } - - if values["default"] is None: - values["default"] = "all_users" - - return values + return {groupname: _human_readable_group(groupname) for groupname in groups} OPTIONS = { @@ -1293,7 +1313,7 @@ class OptionsModel(BaseModel): "id": id_, "type": option.get("type", "string"), # ConfigPanel options needs to be set as optional by default - "optional": option.get("optional", optional) + "optional": option.get("optional", optional), } for id_, option in options.items() ] From a92e22b6534e22e4ce95e50b4312f44953e10713 Mon Sep 17 00:00:00 2001 From: axolotle Date: Tue, 18 Apr 2023 16:11:14 +0200 Subject: [PATCH 14/50] config: rework get method --- src/domain.py | 19 ++----- src/settings.py | 21 ++++---- src/utils/configpanel.py | 108 ++++++++++++++++++--------------------- 3 files changed, 62 insertions(+), 86 deletions(-) diff --git a/src/domain.py b/src/domain.py index a796e0142..6d57a0f10 100644 --- a/src/domain.py +++ b/src/domain.py @@ -652,22 +652,9 @@ 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 + # i18n: domain_config_cert_renew_help + # i18n: domain_config_default_app_help + # i18n: domain_config_xmpp_help def _get_raw_config(self) -> "RawConfig": # TODO add mechanism to share some settings with other domains on the same zone diff --git a/src/settings.py b/src/settings.py index f3340e8e9..e66195802 100644 --- a/src/settings.py +++ b/src/settings.py @@ -19,7 +19,7 @@ import os import subprocess from logging import getLogger -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Any, Union from moulinette import m18n from yunohost.utils.error import YunohostError, YunohostValidationError @@ -31,7 +31,12 @@ from yunohost.log import is_unit_operation from yunohost.utils.legacy import translate_legacy_settings_to_configpanel_settings if TYPE_CHECKING: - from yunohost.utils.configpanel import ConfigPanelModel, RawConfig, RawSettings + from yunohost.utils.configpanel import ( + ConfigPanelGetMode, + ConfigPanelModel, + RawConfig, + RawSettings, + ) logger = getLogger("yunohost.settings") @@ -129,17 +134,11 @@ class SettingsConfigPanel(ConfigPanel): def __init__(self, config_path=None, save_path=None, creation=False): super().__init__("settings") - def get(self, key="", mode="classic"): + def get( + self, key: Union[str, None] = None, mode: "ConfigPanelGetMode" = "classic" + ) -> Any: result = super().get(key=key, mode=mode) - if mode == "full": - for panel, section, option in self._iterate(): - if m18n.key_exists(self.config["i18n"] + "_" + option["id"] + "_help"): - option["help"] = m18n.n( - self.config["i18n"] + "_" + option["id"] + "_help" - ) - return self.config - # Dirty hack to let settings_get() to work from a python script if isinstance(result, str) and result in ["True", "False"]: result = bool(result == "True") diff --git a/src/utils/configpanel.py b/src/utils/configpanel.py index e113d007b..e39d5c91c 100644 --- a/src/utils/configpanel.py +++ b/src/utils/configpanel.py @@ -31,6 +31,7 @@ from moulinette.interfaces.cli import colorize from moulinette.utils.filesystem import mkdir, read_toml, read_yaml, write_to_yaml from yunohost.utils.error import YunohostError, YunohostValidationError from yunohost.utils.form import ( + AnyOption, BaseInputOption, BaseOption, BaseReadonlyOption, @@ -190,6 +191,13 @@ class ConfigPanelModel(BaseModel): for option in section.options: yield option + def get_option(self, option_id) -> Union[AnyOption, None]: + for option in self.options: + if option.id == option_id: + return option + # FIXME raise error? + return None + def iter_children( self, @@ -236,6 +244,7 @@ if TYPE_CHECKING: FilterKey = Sequence[Union[str, None]] RawConfig = OrderedDict[str, Any] RawSettings = dict[str, Any] + ConfigPanelGetMode = Literal["classic", "full", "export"] def parse_filter_key(key: Union[str, None] = None) -> "FilterKey": @@ -310,78 +319,59 @@ class ConfigPanel: and re.match("^(validate|post_ask)__", func) } - def get(self, key="", mode="classic"): - self.filter_key = key or "" + def get( + self, key: Union[str, None] = None, mode: "ConfigPanelGetMode" = "classic" + ) -> Any: + self.filter_key = parse_filter_key(key) + self.config, self.form = self._get_config_panel(prevalidate=False) - # Read config panel toml - self._get_config_panel() - - if not self.config: - raise YunohostValidationError("config_no_panel") - - # Read or get values and hydrate the config - self._get_raw_settings() - self._hydrate() + panel_id, section_id, option_id = self.filter_key # In 'classic' mode, we display the current value if key refer to an option - if self.filter_key.count(".") == 2 and mode == "classic": - option = self.filter_key.split(".")[-1] - value = self.values.get(option, None) + if option_id and mode == "classic": + option = self.config.get_option(option_id) - option_type = None - for _, _, option_ in self._iterate(): - if option_["id"] == option: - option_type = OPTIONS[option_["type"]] - break + if option is None: + # FIXME i18n + raise YunohostValidationError( + f"Couldn't find any option with id {option_id}" + ) - return option_type.normalize(value) if option_type else value + if isinstance(option, BaseReadonlyOption): + return None + + return self.form[option_id] # Format result in 'classic' or 'export' mode + self.config.translate() logger.debug(f"Formating result in '{mode}' mode") - result = {} - for panel, section, option in self._iterate(): - if section["is_action_section"] and mode != "full": - continue + result = OrderedDict() + for panel in self.config.panels: + for section in panel.sections: + if section.is_action_section and mode != "full": + continue - key = f"{panel['id']}.{section['id']}.{option['id']}" - if mode == "export": - result[option["id"]] = option.get("current_value") - continue + for option in section.options: + if mode == "export": + if isinstance(option, BaseInputOption): + result[option.id] = self.form[option.id] + continue - ask = None - if "ask" in option: - ask = _value_for_locale(option["ask"]) - elif "i18n" in self.config: - ask = m18n.n(self.config["i18n"] + "_" + option["id"]) + if mode == "classic": + key = f"{panel.id}.{section.id}.{option.id}" + result[key] = {"ask": option.ask} - if mode == "full": - option["ask"] = ask - question_class = OPTIONS[option.get("type", OptionType.string)] - # FIXME : maybe other properties should be taken from the question, not just choices ?. - if issubclass(question_class, BaseChoicesOption): - option["choices"] = question_class(option).choices - if issubclass(question_class, BaseInputOption): - option["default"] = question_class(option).default - option["pattern"] = question_class(option).pattern - else: - result[key] = {"ask": ask} - if "current_value" in option: - question_class = OPTIONS[option.get("type", OptionType.string)] - if hasattr(question_class, "humanize"): - result[key]["value"] = question_class.humanize( - option["current_value"], option - ) - else: - result[key]["value"] = option["current_value"] - - # FIXME: semantics, technically here this is not about a prompt... - if getattr(question_class, "hide_user_input_in_prompt", None): - result[key][ - "value" - ] = "**************" # Prevent displaying password in `config get` + if isinstance(option, BaseInputOption): + result[key]["value"] = option.humanize( + self.form[option.id], option + ) + if option.type is OptionType.password: + result[key][ + "value" + ] = "**************" # Prevent displaying password in `config get` if mode == "full": - return self.config + return self.config.dict(exclude_none=True) else: return result From dbaea019fe1945fe8e126d51c00fcc019eeca1c2 Mon Sep 17 00:00:00 2001 From: axolotle Date: Tue, 18 Apr 2023 16:54:54 +0200 Subject: [PATCH 15/50] form+config: replace _parse_pre_answered method with generic function --- src/utils/configpanel.py | 19 ++++++------------- src/utils/form.py | 31 +++++++++++++++++++++++++++---- 2 files changed, 33 insertions(+), 17 deletions(-) diff --git a/src/utils/configpanel.py b/src/utils/configpanel.py index e39d5c91c..1f240a105 100644 --- a/src/utils/configpanel.py +++ b/src/utils/configpanel.py @@ -43,6 +43,7 @@ from yunohost.utils.form import ( ask_questions_and_parse_answers, build_form, evaluate_simple_js_expression, + parse_prefilled_values, ) from yunohost.utils.i18n import _value_for_locale @@ -397,7 +398,10 @@ class ConfigPanel: # Import and parse pre-answered options logger.debug("Import and parse pre-answered options") - self._parse_pre_answered(args, value, args_file) + if option_id and value is not None: + self.args = {option_id: value} + else: + self.args = parse_prefilled_values(args, value, args_file) # Read or get values and hydrate the config self._get_raw_settings() @@ -468,7 +472,7 @@ class ConfigPanel: # Import and parse pre-answered options logger.debug("Import and parse pre-answered options") - self._parse_pre_answered(args, None, args_file) + self.args = parse_prefilled_values(args, args_file) # Read or get values and hydrate the config self._get_raw_settings() @@ -678,17 +682,6 @@ class ConfigPanel: } ) - 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()} - - if args_file: - # Import YAML / JSON file but keep --args values - self.args = {**read_yaml(args_file), **self.args} - - if value is not None: - self.args = {self.filter_key.split(".")[-1]: value} - def _apply(self): logger.info("Saving the new configuration...") dir_path = os.path.dirname(os.path.realpath(self.save_path)) diff --git a/src/utils/form.py b/src/utils/form.py index 8ed83393e..7a97259b9 100644 --- a/src/utils/form.py +++ b/src/utils/form.py @@ -53,7 +53,7 @@ from pydantic.types import constr from moulinette import Moulinette, m18n from moulinette.interfaces.cli import colorize -from moulinette.utils.filesystem import read_file, write_to_file +from moulinette.utils.filesystem import read_file, read_yaml, write_to_file from yunohost.log import OperationLogger from yunohost.utils.error import YunohostError, YunohostValidationError from yunohost.utils.i18n import _value_for_locale @@ -1431,6 +1431,31 @@ def hydrate_option_type(raw_option: dict[str, Any]) -> dict[str, Any]: Hooks = dict[str, Callable[[BaseInputOption], Any]] +def parse_prefilled_values( + args: Union[str, None] = None, + args_file: Union[str, None] = None, + method: Literal["parse_qs", "parse_qsl"] = "parse_qs", +) -> dict[str, Any]: + """ + Retrieve form values from yaml file or query string. + """ + values: Values = {} + if args_file: + # Import YAML / JSON file + values |= read_yaml(args_file) + if args: + # FIXME See `ask_questions_and_parse_answers` + parsed = getattr(urllib.parse, method)(args, keep_blank_values=True) + if isinstance(parsed, dict): # parse_qs + # FIXME could do the following to get a list directly? + # k: None if not len(v) else (v if len(v) > 1 else v[0]) + values |= {k: ",".join(v) for k, v in parsed.items()} + else: + values |= dict(parsed) + + return values + + def prompt_or_validate_form( options: list[AnyOption], form: FormModel, @@ -1561,9 +1586,7 @@ def ask_questions_and_parse_answers( # whereas parse.qs return list of values (which is useful for tags, etc) # For now, let's not migrate this piece of code to parse_qs # Because Aleks believes some bits of the app CI rely on overriding values (e.g. foo=foo&...&foo=bar) - answers = dict( - urllib.parse.parse_qsl(prefilled_answers or "", keep_blank_values=True) - ) + answers = parse_prefilled_values(prefilled_answers, method="parse_qsl") elif isinstance(prefilled_answers, Mapping): answers = {**prefilled_answers} else: From f1038de56d0289d770317109d770bf5144129703 Mon Sep 17 00:00:00 2001 From: axolotle Date: Tue, 18 Apr 2023 17:51:21 +0200 Subject: [PATCH 16/50] form: fix entities validators order for filter and apply the right default --- src/tests/test_questions.py | 11 +---------- src/utils/form.py | 28 ++++++++++++++++++---------- 2 files changed, 19 insertions(+), 20 deletions(-) diff --git a/src/tests/test_questions.py b/src/tests/test_questions.py index 959f2c8b7..387d5c0f9 100644 --- a/src/tests/test_questions.py +++ b/src/tests/test_questions.py @@ -1816,9 +1816,7 @@ class TestGroup(BaseTest): "scenarios": [ ("custom_group", "custom_group"), *all_as("", None, output="visitors", raw_option={"default": "visitors"}), - *xpass(scenarios=[ - ("", "custom_group", {"default": "custom_group"}), - ], reason="Should throw 'default must be in (None, 'all_users', 'visitors', 'admins')"), + ("", FAIL, {"default": "custom_group"}), # Not allowed to set a default which is not a default group # readonly ("admins", FAIL, {"readonly": True}), # readonly is forbidden ] @@ -1837,13 +1835,6 @@ class TestGroup(BaseTest): "prefill": "admins", } ) - # FIXME This should fail, not allowed to set a default which is not a default group - super().test_options_prompted_with_ask_help( - prefill_data={ - "raw_option": {"default": "custom_group"}, - "prefill": "custom_group", - } - ) def test_scenarios(self, intake, expected_output, raw_option, data): with patch_groups(**data): diff --git a/src/utils/form.py b/src/utils/form.py index 7a97259b9..7098692a4 100644 --- a/src/utils/form.py +++ b/src/utils/form.py @@ -951,6 +951,7 @@ ChoosableOptions = Literal[ class BaseChoicesOption(BaseInputOption): # FIXME probably forbid choices to be None? + filter: Union[JSExpression, None] = None # filter before choices choices: Union[dict[str, Any], list[Any], None] @validator("choices", pre=True) @@ -1093,6 +1094,7 @@ class TagsOption(BaseChoicesOption): class DomainOption(BaseChoicesOption): type: Literal[OptionType.domain] = OptionType.domain + filter: Literal[None] = None choices: Union[dict[str, str], None] @validator("choices", pre=True, always=True) @@ -1108,17 +1110,14 @@ class DomainOption(BaseChoicesOption): for domain in data["domains"] } - @validator("default") + @validator("default", pre=True, always=True) def inject_default( cls, value: Union[str, None], values: Values ) -> Union[str, None]: # TODO remove calls to resources in validators (pydantic V2 should adress this) from yunohost.domain import _get_maindomain - if value is None: - return _get_maindomain() - - return value + return _get_maindomain() @staticmethod def normalize(value, option={}): @@ -1135,9 +1134,9 @@ class DomainOption(BaseChoicesOption): class AppOption(BaseChoicesOption): type: Literal[OptionType.app] = OptionType.app - choices: Union[dict[str, str], None] + filter: Union[JSExpression, None] = None add_yunohost_portal_to_choices: bool = False - filter: Union[str, None] = None + choices: Union[dict[str, str], None] @validator("choices", pre=True, always=True) def inject_apps_choices( @@ -1172,6 +1171,7 @@ class AppOption(BaseChoicesOption): class UserOption(BaseChoicesOption): type: Literal[OptionType.user] = OptionType.user + filter: Literal[None] = None choices: Union[dict[str, str], None] @validator("choices", pre=True, always=True) @@ -1196,19 +1196,19 @@ class UserOption(BaseChoicesOption): return value - @validator("default") + @validator("default", pre=True, always=True) def inject_default( cls, value: Union[str, None], values: Values ) -> Union[str, None]: # TODO remove calls to resources in validators (pydantic V2 should adress this) from yunohost.domain import _get_maindomain - from yunohost.user import user_info + from yunohost.user import user_list, user_info if value is None: # FIXME: this code is obsolete with the new admins group # Should be replaced by something like "any first user we find in the admin group" root_mail = "root@%s" % _get_maindomain() - for user in values["choices"].keys(): + for user in user_list()["users"].keys(): if root_mail in user_info(user).get("mail-aliases", []): return user @@ -1217,6 +1217,7 @@ class UserOption(BaseChoicesOption): class GroupOption(BaseChoicesOption): type: Literal[OptionType.group] = OptionType.group + filter: Literal[None] = None choices: Union[dict[str, str], None] default: Union[Literal["visitors", "all_users", "admins"], None] = "all_users" @@ -1241,6 +1242,13 @@ class GroupOption(BaseChoicesOption): return {groupname: _human_readable_group(groupname) for groupname in groups} + @validator("default", pre=True, always=True) + def inject_default(cls, value: Union[str, None], values: Values) -> str: + # FIXME do we really want to default to something all the time? + if value is None: + return "all_users" + return value + OPTIONS = { OptionType.display_text: DisplayTextOption, From 2c35dcbb2498f2227282a10a8d8eac6da06f82e6 Mon Sep 17 00:00:00 2001 From: axolotle Date: Tue, 18 Apr 2023 20:04:20 +0200 Subject: [PATCH 17/50] configpanel: update _reload_services --- src/utils/configpanel.py | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/utils/configpanel.py b/src/utils/configpanel.py index 1f240a105..22ea5b3b8 100644 --- a/src/utils/configpanel.py +++ b/src/utils/configpanel.py @@ -199,6 +199,17 @@ class ConfigPanelModel(BaseModel): # FIXME raise error? return None + @property + def services(self) -> list[str]: + services = set() + for panel in self.panels: + services |= set(panel.services) + for section in panel.sections: + services |= set(section.services) + + services_ = list(services) + services_.sort(key="nginx".__eq__) + return services_ def iter_children( self, @@ -701,12 +712,8 @@ class ConfigPanel: def _reload_services(self): from yunohost.service import service_reload_or_restart - services_to_reload = set() - for panel, section, obj in self._iterate(["panel", "section", "option"]): - services_to_reload |= set(obj.get("services", [])) + services_to_reload = self.config.services - services_to_reload = list(services_to_reload) - services_to_reload.sort(key="nginx".__eq__) if services_to_reload: logger.info("Reloading services...") for service in services_to_reload: From 7a60703ef58b17de4e1cde9e7861fa4e50298e6e Mon Sep 17 00:00:00 2001 From: axolotle Date: Tue, 18 Apr 2023 20:13:01 +0200 Subject: [PATCH 18/50] configpanel: update _ask --- src/utils/configpanel.py | 95 +++++++++++++++++++++------------------- 1 file changed, 51 insertions(+), 44 deletions(-) diff --git a/src/utils/configpanel.py b/src/utils/configpanel.py index 22ea5b3b8..22318e5e0 100644 --- a/src/utils/configpanel.py +++ b/src/utils/configpanel.py @@ -19,7 +19,6 @@ import glob import os import re -import urllib.parse from collections import OrderedDict from logging import getLogger from typing import TYPE_CHECKING, Any, Literal, Sequence, Type, Union @@ -36,19 +35,19 @@ from yunohost.utils.form import ( BaseOption, BaseReadonlyOption, FileOption, - FormModel, OptionsModel, OptionType, Translation, - ask_questions_and_parse_answers, build_form, evaluate_simple_js_expression, parse_prefilled_values, + prompt_or_validate_form, ) from yunohost.utils.i18n import _value_for_locale if TYPE_CHECKING: from pydantic.fields import ModelField + from yunohost.utils.form import FormModel, Hooks logger = getLogger("yunohost.configpanel") @@ -279,7 +278,8 @@ class ConfigPanel: save_mode = "full" filter_key: "FilterKey" = (None, None, None) config: Union[ConfigPanelModel, None] = None - form: Union[FormModel, None] = None + form: Union["FormModel", None] = None + hooks: "Hooks" = {} @classmethod def list(cls): @@ -602,7 +602,7 @@ class ConfigPanel: def _get_config_panel( self, prevalidate: bool = False - ) -> tuple[ConfigPanelModel, FormModel]: + ) -> tuple[ConfigPanelModel, "FormModel"]: raw_config = self._get_partial_raw_config() config = ConfigPanelModel(**raw_config) config, raw_settings = self._get_partial_raw_settings_and_mutate_config(config) @@ -623,58 +623,62 @@ class ConfigPanel: return (config, settings) - def _ask(self, action=None): + def ask( + self, + config: ConfigPanelModel, + settings: "FormModel", + prefilled_answers: dict[str, Any] = {}, + action_id: Union[str, None] = None, + hooks: "Hooks" = {}, + ) -> "FormModel": + # FIXME could be turned into a staticmethod logger.debug("Ask unanswered question and prevalidate data") - if "i18n" in self.config: - for panel, section, option in self._iterate(): - if "ask" not in option: - option["ask"] = m18n.n(self.config["i18n"] + "_" + option["id"]) - # auto add i18n help text if present in locales - if m18n.key_exists(self.config["i18n"] + "_" + option["id"] + "_help"): - option["help"] = m18n.n( - self.config["i18n"] + "_" + option["id"] + "_help" - ) + interactive = Moulinette.interface.type == "cli" and os.isatty(1) - def display_header(message): - """CLI panel/section header display""" - if Moulinette.interface.type == "cli" and self.filter_key.count(".") < 2: - Moulinette.display(colorize(message, "purple")) + if interactive: + config.translate() - for panel, section, obj in self._iterate(["panel", "section"]): - if ( - section - and section.get("visible") - and not evaluate_simple_js_expression( - section["visible"], context=self.future_values + for panel in config.panels: + if interactive: + Moulinette.display( + colorize(f"\n{'='*40}\n>>>> {panel.name}\n{'='*40}", "purple") ) - ): - continue - # Ugly hack to skip action section ... except when when explicitly running actions - if not action: - if section and section["is_action_section"]: + # A section or option may only evaluate its conditions (`visible` + # and `enabled`) with its panel's local context that is built + # prompt after prompt. + # That means that a condition can only reference options of its + # own panel and options that are previously defined. + context: dict[str, Any] = {} + + for section in panel.sections: + if ( + action_id is None and section.is_action_section + ) or not section.is_visible(context): + # FIXME useless? + Moulinette.display("Skipping section '{panel.id}.{section.id}'…") continue - if panel == obj: - name = _value_for_locale(panel["name"]) - display_header(f"\n{'='*40}\n>>>> {name}\n{'='*40}") - else: - name = _value_for_locale(section["name"]) - if name: - display_header(f"\n# {name}") - elif section: + if interactive and section.name: + Moulinette.display(colorize(f"\n# {section.name}", "purple")) + # filter action section options in case of multiple buttons - section["options"] = [ + options = [ option - for option in section["options"] - if option.get("type", OptionType.string) != OptionType.button - or option["id"] == action + for option in section.options + if option.type is not OptionType.button or option.id == action_id ] - if panel == obj: - continue + settings = prompt_or_validate_form( + options, + settings, + prefilled_answers=prefilled_answers, + context=context, + hooks=hooks, + ) +<<<<<<< HEAD # Check and ask unanswered questions prefilled_answers = self.args.copy() prefilled_answers.update(self.new_values) @@ -692,6 +696,9 @@ class ConfigPanel: if not question.readonly and question.value is not None } ) +======= + return settings +>>>>>>> be777b928 (configpanel: update _ask) def _apply(self): logger.info("Saving the new configuration...") From 5f9ea5831391908eeace58c3f496796c69351bd5 Mon Sep 17 00:00:00 2001 From: axolotle Date: Tue, 18 Apr 2023 20:15:25 +0200 Subject: [PATCH 19/50] configpanel: update _apply --- src/app.py | 14 +++++++-- src/domain.py | 66 +++++++++++++++++----------------------- src/settings.py | 42 +++++++++++++------------ src/utils/configpanel.py | 47 ++++++++++++---------------- 4 files changed, 81 insertions(+), 88 deletions(-) diff --git a/src/app.py b/src/app.py index ffcd1ecc3..08e222579 100644 --- a/src/app.py +++ b/src/app.py @@ -27,7 +27,7 @@ import subprocess import tempfile import copy from collections import OrderedDict -from typing import TYPE_CHECKING, List, Tuple, Dict, Any, Iterator, Optional +from typing import TYPE_CHECKING, List, Tuple, Dict, Any, Iterator, Optional, Union from packaging import version from logging import getLogger from pathlib import Path @@ -73,7 +73,10 @@ from yunohost.app_catalog import ( # noqa ) if TYPE_CHECKING: + from pydantic.typing import AbstractSetIntStr, MappingIntStrAny + from yunohost.utils.configpanel import ConfigPanelModel, RawSettings + from yunohost.utils.form import FormModel logger = getLogger("yunohost.app") @@ -1809,8 +1812,13 @@ class AppConfigPanel(ConfigPanel): def _get_raw_settings(self, config: "ConfigPanelModel") -> "RawSettings": return self._call_config_script("show") - def _apply(self): - env = {key: str(value) for key, value in self.new_values.items()} + def _apply( + self, + form: "FormModel", + previous_settings: dict[str, Any], + exclude: Union["AbstractSetIntStr", "MappingIntStrAny", None] = None, + ): + env = {key: str(value) for key, value in form.dict().items()} return_content = self._call_config_script("apply", env=env) # If the script returned validation error diff --git a/src/domain.py b/src/domain.py index 6d57a0f10..f0531e624 100644 --- a/src/domain.py +++ b/src/domain.py @@ -19,7 +19,7 @@ import os import time from pathlib import Path -from typing import TYPE_CHECKING, List, Optional +from typing import TYPE_CHECKING, Any, List, Optional, Union from collections import OrderedDict from logging import getLogger @@ -48,7 +48,10 @@ from yunohost.utils.dns import is_yunohost_dyndns_domain from yunohost.log import is_unit_operation if TYPE_CHECKING: + from pydantic.typing import AbstractSetIntStr, MappingIntStrAny + from yunohost.utils.configpanel import RawConfig + from yunohost.utils.form import FormModel logger = getLogger("yunohost.domain") @@ -669,7 +672,7 @@ class DomainConfigPanel(ConfigPanel): # Portal settings are only available on "topest" domains if _get_parent_domain_of(self.entity, topest=True) is not None: - del toml["feature"]["portal"] + del raw_config["feature"]["portal"] # Optimize wether or not to load the DNS section, # e.g. we don't want to trigger the whole _get_registary_config_section @@ -712,17 +715,23 @@ class DomainConfigPanel(ConfigPanel): return raw_config - def _apply(self): - if ( - "default_app" in self.future_values - and self.future_values["default_app"] != self.values["default_app"] - ): + def _apply( + self, + form: "FormModel", + previous_settings: dict[str, Any], + exclude: Union["AbstractSetIntStr", "MappingIntStrAny", None] = None, + ): + next_settings = { + k: v for k, v in form.dict().items() if previous_settings.get(k) != v + } + + if "default_app" in next_settings: from yunohost.app import app_ssowatconf, app_map if "/" in app_map(raw=True).get(self.entity, {}): raise YunohostValidationError( "app_make_default_location_already_used", - app=self.future_values["default_app"], + app=next_settings["default_app"], domain=self.entity, other_app=app_map(raw=True)[self.entity]["/"]["id"], ) @@ -735,8 +744,7 @@ class DomainConfigPanel(ConfigPanel): "portal_theme", ] if _get_parent_domain_of(self.entity, topest=True) is None and any( - option in self.future_values - and self.new_values[option] != self.values.get(option) + option in next_settings for option in portal_options ): from yunohost.portal import PORTAL_SETTINGS_DIR @@ -744,9 +752,8 @@ class DomainConfigPanel(ConfigPanel): # Portal options are also saved in a `domain.portal.yml` file # that can be read by the portal API. # FIXME remove those from the config panel saved values? - portal_values = { - option: self.future_values[option] for option in portal_options - } + + portal_values = form.dict(include=portal_options) portal_settings_path = Path(f"{PORTAL_SETTINGS_DIR}/{self.entity}.json") portal_settings = {"apps": {}} @@ -760,38 +767,21 @@ class DomainConfigPanel(ConfigPanel): str(portal_settings_path), portal_settings, sort_keys=True, indent=4 ) - super()._apply() + super()._apply(form, previous_settings) # Reload ssowat if default app changed - if ( - "default_app" in self.future_values - and self.future_values["default_app"] != self.values["default_app"] - ): + if "default_app" in next_settings: app_ssowatconf() - stuff_to_regen_conf = [] - if ( - "xmpp" in self.future_values - and self.future_values["xmpp"] != self.values["xmpp"] - ): - stuff_to_regen_conf.append("nginx") - stuff_to_regen_conf.append("metronome") + stuff_to_regen_conf = set() + if "xmpp" in next_settings: + stuff_to_regen_conf.update({"nginx", "metronome"}) - if ( - "mail_in" in self.future_values - and self.future_values["mail_in"] != self.values["mail_in"] - ) or ( - "mail_out" in self.future_values - and self.future_values["mail_out"] != self.values["mail_out"] - ): - if "nginx" not in stuff_to_regen_conf: - stuff_to_regen_conf.append("nginx") - stuff_to_regen_conf.append("postfix") - stuff_to_regen_conf.append("dovecot") - stuff_to_regen_conf.append("rspamd") + if "mail_in" in next_settings or "mail_out" in next_settings: + stuff_to_regen_conf.update({"nginx", "postfix", "dovecot", "rspamd"}) if stuff_to_regen_conf: - regen_conf(names=stuff_to_regen_conf) + regen_conf(names=list(stuff_to_regen_conf)) def domain_action_run(domain, action, args=None): diff --git a/src/settings.py b/src/settings.py index e66195802..6a05217dc 100644 --- a/src/settings.py +++ b/src/settings.py @@ -31,12 +31,15 @@ from yunohost.log import is_unit_operation from yunohost.utils.legacy import translate_legacy_settings_to_configpanel_settings if TYPE_CHECKING: + from pydantic.typing import AbstractSetIntStr, MappingIntStrAny + from yunohost.utils.configpanel import ( ConfigPanelGetMode, ConfigPanelModel, RawConfig, RawSettings, ) + from yunohost.utils.form import FormModel logger = getLogger("yunohost.settings") @@ -217,19 +220,15 @@ class SettingsConfigPanel(ConfigPanel): return raw_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) + def _apply( + self, + form: "FormModel", + previous_settings: dict[str, Any], + exclude: Union["AbstractSetIntStr", "MappingIntStrAny", None] = None, + ): + root_password = form.get("root_password", None) + root_password_confirm = form.get("root_password_confirm", None) + passwordless_sudo = form.get("passwordless_sudo", None) if root_password and root_password.strip(): if root_password != root_password_confirm: @@ -248,15 +247,20 @@ class SettingsConfigPanel(ConfigPanel): {"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 + # First save settings except virtual + default ones + super()._apply(form, previous_settings, exclude=self.virtual_settings) + next_settings = { + k: v + for k, v in form.dict(exclude=self.virtual_settings).items() + if previous_settings.get(k) != v } - for setting_name, value in settings.items(): + + for setting_name, value in next_settings.items(): try: + # FIXME not sure to understand why we need the previous value if + # updated_settings has already been filtered trigger_post_change_hook( - setting_name, self.values.get(setting_name), value + setting_name, previous_settings.get(setting_name), value ) except Exception as e: logger.error(f"Post-change hook for setting failed : {e}") diff --git a/src/utils/configpanel.py b/src/utils/configpanel.py index 22318e5e0..56b0584e3 100644 --- a/src/utils/configpanel.py +++ b/src/utils/configpanel.py @@ -47,6 +47,8 @@ from yunohost.utils.i18n import _value_for_locale if TYPE_CHECKING: from pydantic.fields import ModelField + from pydantic.typing import AbstractSetIntStr, MappingIntStrAny + from yunohost.utils.form import FormModel, Hooks logger = getLogger("yunohost.configpanel") @@ -678,43 +680,32 @@ class ConfigPanel: hooks=hooks, ) -<<<<<<< HEAD - # Check and ask unanswered questions - prefilled_answers = self.args.copy() - prefilled_answers.update(self.new_values) - - questions = ask_questions_and_parse_answers( - {question["id"]: question for question in section["options"]}, - prefilled_answers=prefilled_answers, - current_values=self.values, - hooks=self.hooks, - ) - self.new_values.update( - { - question.id: question.value - for question in questions - if not question.readonly and question.value is not None - } - ) -======= return settings ->>>>>>> be777b928 (configpanel: update _ask) - def _apply(self): + def _apply( + self, + form: "FormModel", + previous_settings: dict[str, Any], + exclude: Union["AbstractSetIntStr", "MappingIntStrAny", None] = None, + ) -> dict[str, Any]: + """ + Save settings in yaml file. + If `save_mode` is `"diff"` (which is the default), only values that are + different from their default value will be saved. + """ logger.info("Saving the new configuration...") + dir_path = os.path.dirname(os.path.realpath(self.save_path)) if not os.path.exists(dir_path): mkdir(dir_path, mode=0o700) - values_to_save = self.future_values - if self.save_mode == "diff": - defaults = self._get_default_values() - values_to_save = { - k: v for k, v in values_to_save.items() if defaults.get(k) != v - } + exclude_defaults = self.save_mode == "diff" + settings = form.dict(exclude_defaults=exclude_defaults, exclude=exclude) # type: ignore # Save the settings to the .yaml file - write_to_yaml(self.save_path, values_to_save) + write_to_yaml(self.save_path, settings) + + return settings def _reload_services(self): from yunohost.service import service_reload_or_restart From 6b3691ce534df73c98479e4eba3387024cca8000 Mon Sep 17 00:00:00 2001 From: axolotle Date: Tue, 18 Apr 2023 21:04:05 +0200 Subject: [PATCH 20/50] configpanel: update set --- src/settings.py | 2 +- src/utils/configpanel.py | 49 ++++++++++++++++++++++++---------------- 2 files changed, 31 insertions(+), 20 deletions(-) diff --git a/src/settings.py b/src/settings.py index 6a05217dc..0d0a5406b 100644 --- a/src/settings.py +++ b/src/settings.py @@ -132,7 +132,7 @@ class SettingsConfigPanel(ConfigPanel): entity_type = "global" save_path_tpl = SETTINGS_PATH save_mode = "diff" - virtual_settings = ["root_password", "root_password_confirm", "passwordless_sudo"] + virtual_settings = {"root_password", "root_password_confirm", "passwordless_sudo"} def __init__(self, config_path=None, save_path=None, creation=False): super().__init__("settings") diff --git a/src/utils/configpanel.py b/src/utils/configpanel.py index 56b0584e3..20a421925 100644 --- a/src/utils/configpanel.py +++ b/src/utils/configpanel.py @@ -50,6 +50,7 @@ if TYPE_CHECKING: from pydantic.typing import AbstractSetIntStr, MappingIntStrAny from yunohost.utils.form import FormModel, Hooks + from yunohost.log import OperationLogger logger = getLogger("yunohost.configpanel") @@ -390,15 +391,15 @@ class ConfigPanel: return result def set( - self, key=None, value=None, args=None, args_file=None, operation_logger=None + self, + key: Union[str, None] = None, + value: Any = None, + args: Union[str, None] = None, + args_file: Union[str, None] = None, + operation_logger: Union["OperationLogger", None] = None, ): - self.filter_key = key or "" - - # Read config panel toml - self._get_config_panel() - - if not self.config: - raise YunohostValidationError("config_no_panel") + self.filter_key = parse_filter_key(key) + panel_id, section_id, option_id = self.filter_key if (args is not None or args_file is not None) and value is not None: raise YunohostValidationError( @@ -406,27 +407,35 @@ class ConfigPanel: raw_msg=True, ) - if self.filter_key.count(".") != 2 and value is not None: + if not option_id 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") if option_id and value is not None: - self.args = {option_id: value} + prefilled_answers = {option_id: value} else: - self.args = parse_prefilled_values(args, value, args_file) + prefilled_answers = parse_prefilled_values(args, args_file) - # Read or get values and hydrate the config - self._get_raw_settings() - self._hydrate() - BaseOption.operation_logger = operation_logger - self._ask() + self.config, self.form = self._get_config_panel() + # FIXME find a better way to exclude previous settings + previous_settings = self.form.dict() + + # FIXME Not sure if this is need (redact call to operation logger does it on all the instances) + # BaseOption.operation_logger = operation_logger + + self.form = self._ask( + self.config, + self.form, + prefilled_answers=prefilled_answers, + hooks=self.hooks, + ) if operation_logger: operation_logger.start() try: - self._apply() + self._apply(self.form, previous_settings) except YunohostError: raise # Script got manually interrupted ... @@ -452,7 +461,9 @@ class ConfigPanel: self._reload_services() logger.success("Config updated as expected") - operation_logger.success() + + if operation_logger: + operation_logger.success() def list_actions(self): actions = {} @@ -625,7 +636,7 @@ class ConfigPanel: return (config, settings) - def ask( + def _ask( self, config: ConfigPanelModel, settings: "FormModel", From 15c827908f917ab57e61b303d4af5e36a1a9d5ce Mon Sep 17 00:00:00 2001 From: axolotle Date: Tue, 18 Apr 2023 21:05:25 +0200 Subject: [PATCH 21/50] configpanel: update run_action --- src/app.py | 11 ++++---- src/utils/configpanel.py | 59 +++++++++++++++++++++++++--------------- 2 files changed, 43 insertions(+), 27 deletions(-) diff --git a/src/app.py b/src/app.py index 08e222579..970a66fb2 100644 --- a/src/app.py +++ b/src/app.py @@ -46,10 +46,11 @@ from moulinette.utils.filesystem import ( chmod, ) -from yunohost.utils.configpanel import ConfigPanel, ask_questions_and_parse_answers +from yunohost.utils.configpanel import ConfigPanel from yunohost.utils.form import ( DomainOption, WebPathOption, + ask_questions_and_parse_answers, hydrate_questions_with_choices, ) from yunohost.utils.i18n import _value_for_locale @@ -1805,10 +1806,6 @@ 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 _run_action(self, action): - env = {key: str(value) for key, value in self.new_values.items()} - self._call_config_script(action, env=env) - def _get_raw_settings(self, config: "ConfigPanelModel") -> "RawSettings": return self._call_config_script("show") @@ -1832,6 +1829,10 @@ class AppConfigPanel(ConfigPanel): error=message, ) + def _run_action(self, form: "FormModel", action_id: str): + env = {key: str(value) for key, value in form.dict().items()} + self._call_config_script(action_id, env=env) + def _call_config_script(self, action, env=None): from yunohost.hook import hook_exec diff --git a/src/utils/configpanel.py b/src/utils/configpanel.py index 20a421925..eb244aa08 100644 --- a/src/utils/configpanel.py +++ b/src/utils/configpanel.py @@ -479,30 +479,41 @@ class ConfigPanel: return actions - def run_action(self, action=None, args=None, args_file=None, operation_logger=None): + def run_action( + self, + key: Union[str, None] = None, + args: Union[str, None] = None, + args_file: Union[str, None] = None, + operation_logger: Union["OperationLogger", None] = None, + ): # # FIXME : this stuff looks a lot like set() ... # + panel_id, section_id, action_id = parse_filter_key(key) + # since an action may require some options from its section, + # remove the action_id from the filter + self.filter_key = (panel_id, section_id, None) - self.filter_key = ".".join(action.split(".")[:2]) - action_id = action.split(".")[2] - - # Read config panel toml - self._get_config_panel() + self.config, self.form = self._get_config_panel() # FIXME: should also check that there's indeed a key called action - if not self.config: - raise YunohostValidationError(f"No action named {action}", raw_msg=True) + if not action_id or not self.config.get_option(action_id): + raise YunohostValidationError(f"No action named {action_id}", raw_msg=True) # Import and parse pre-answered options logger.debug("Import and parse pre-answered options") - self.args = parse_prefilled_values(args, args_file) + prefilled_answers = parse_prefilled_values(args, args_file) - # Read or get values and hydrate the config - self._get_raw_settings() - self._hydrate() - BaseOption.operation_logger = operation_logger - self._ask(action=action_id) + self.form = self._ask( + self.config, + self.form, + prefilled_answers=prefilled_answers, + action_id=action_id, + hooks=self.hooks, + ) + + # FIXME Not sure if this is need (redact call to operation logger does it on all the instances) + # BaseOption.operation_logger = operation_logger # FIXME: here, we could want to check constrains on # the action's visibility / requirements wrt to the answer to questions ... @@ -511,21 +522,21 @@ class ConfigPanel: operation_logger.start() try: - self._run_action(action_id) + self._run_action(self.form, action_id) 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_action_failed", action=action, error=error)) + logger.error(m18n.n("config_action_failed", action=key, 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_action_failed", action=action, error=error)) + logger.error(m18n.n("config_action_failed", action=key, error=error)) raise finally: # Delete files uploaded from API @@ -536,7 +547,9 @@ class ConfigPanel: # FIXME: i18n logger.success(f"Action {action_id} successful") - operation_logger.success() + + if operation_logger: + operation_logger.success() def _get_raw_config(self) -> "RawConfig": if not os.path.exists(self.config_path): @@ -648,12 +661,13 @@ class ConfigPanel: logger.debug("Ask unanswered question and prevalidate data") interactive = Moulinette.interface.type == "cli" and os.isatty(1) + verbose = action_id is None or len(list(config.options)) > 1 if interactive: config.translate() for panel in config.panels: - if interactive: + if interactive and verbose: Moulinette.display( colorize(f"\n{'='*40}\n>>>> {panel.name}\n{'='*40}", "purple") ) @@ -669,11 +683,9 @@ class ConfigPanel: if ( action_id is None and section.is_action_section ) or not section.is_visible(context): - # FIXME useless? - Moulinette.display("Skipping section '{panel.id}.{section.id}'…") continue - if interactive and section.name: + if interactive and verbose and section.name: Moulinette.display(colorize(f"\n# {section.name}", "purple")) # filter action section options in case of multiple buttons @@ -718,6 +730,9 @@ class ConfigPanel: return settings + def _run_action(self, form: "FormModel", action_id: str): + raise NotImplementedError() + def _reload_services(self): from yunohost.service import service_reload_or_restart From 25ccbd5f78e25cc7a75128ae7f0e2fcc8f8f74fe Mon Sep 17 00:00:00 2001 From: axolotle Date: Tue, 18 Apr 2023 22:00:58 +0200 Subject: [PATCH 22/50] configpanel: quickly update list_actions --- src/utils/configpanel.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/utils/configpanel.py b/src/utils/configpanel.py index eb244aa08..96bf910b0 100644 --- a/src/utils/configpanel.py +++ b/src/utils/configpanel.py @@ -470,12 +470,12 @@ class ConfigPanel: # FIXME : meh, loading the entire config panel is again going to cause # stupid issues for domain (e.g loading registrar stuff when willing to just list available actions ...) - self.filter_key = "" - self._get_config_panel() - for panel, section, option in self._iterate(): - if option["type"] == OptionType.button: - key = f"{panel['id']}.{section['id']}.{option['id']}" - actions[key] = _value_for_locale(option["ask"]) + self.config, self.form = self._get_config_panel() + + for panel, section, option in self.config.iter_children(): + if option.type == OptionType.button: + key = f"{panel.id}.{section.id}.{option.id}" + actions[key] = _value_for_locale(option.ask) return actions From f087a6b967d434c32517822bd1b8741c62547798 Mon Sep 17 00:00:00 2001 From: axolotle Date: Wed, 19 Apr 2023 14:34:43 +0200 Subject: [PATCH 23/50] config: normalize get option value --- src/utils/configpanel.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/configpanel.py b/src/utils/configpanel.py index 96bf910b0..4e1e0e8ce 100644 --- a/src/utils/configpanel.py +++ b/src/utils/configpanel.py @@ -355,7 +355,7 @@ class ConfigPanel: if isinstance(option, BaseReadonlyOption): return None - return self.form[option_id] + return option.normalize(self.form[option_id], option) # Format result in 'classic' or 'export' mode self.config.translate() From 73b795be1d192329936a052787ec67178dc81ddd Mon Sep 17 00:00:00 2001 From: axolotle Date: Wed, 19 Apr 2023 14:37:49 +0200 Subject: [PATCH 24/50] config: readd value has been initialized + change test removed value from settings since boolean has a native default value --- src/tests/test_app_config.py | 4 ++-- src/utils/configpanel.py | 11 +++++++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/tests/test_app_config.py b/src/tests/test_app_config.py index 4a74cbc0d..24abdc5dc 100644 --- a/src/tests/test_app_config.py +++ b/src/tests/test_app_config.py @@ -125,9 +125,9 @@ def test_app_config_get_nonexistentstuff(config_app): with pytest.raises(YunohostValidationError): app_config_get(config_app, "main.components.nonexistent") - app_setting(config_app, "boolean", delete=True) + app_setting(config_app, "number", delete=True) with pytest.raises(YunohostError): - app_config_get(config_app, "main.components.boolean") + app_config_get(config_app, "main.components.number") def test_app_config_regular_setting(config_app): diff --git a/src/utils/configpanel.py b/src/utils/configpanel.py index 4e1e0e8ce..eb9455a80 100644 --- a/src/utils/configpanel.py +++ b/src/utils/configpanel.py @@ -601,6 +601,17 @@ class ConfigPanel: for _, section, option in config.iter_children(): value = data = raw_settings.get(option.id, getattr(option, "default", None)) + if isinstance(option, BaseInputOption) and option.id not in raw_settings: + if option.default is not None: + value = option.default + elif option.type is OptionType.file or option.bind == "null": + continue + else: + raise YunohostError( + f"Config panel question '{option.id}' should be initialized with a value during install or upgrade.", + raw_msg=True, + ) + if isinstance(data, dict): # Settings data if gathered from bash "ynh_app_config_show" # may be a custom getter that returns a dict with `value` or `current_value` From 98d3b4ffc8d7bfed4eea3c147a4da68a2f80dd68 Mon Sep 17 00:00:00 2001 From: axolotle Date: Wed, 19 Apr 2023 15:05:16 +0200 Subject: [PATCH 25/50] form: rework context/values/hooks in prompt_or_validate_form --- src/utils/form.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/utils/form.py b/src/utils/form.py index 7098692a4..1496f445b 100644 --- a/src/utils/form.py +++ b/src/utils/form.py @@ -1471,9 +1471,6 @@ def prompt_or_validate_form( context: Context = {}, hooks: Hooks = {}, ) -> FormModel: - answers = {**prefilled_answers} - values = {} - for option in options: interactive = Moulinette.interface.type == "cli" and os.isatty(1) @@ -1505,7 +1502,7 @@ def prompt_or_validate_form( if isinstance(option, BaseInputOption): # FIXME normalized needed, form[option.id] should already be normalized # only update the context with the value - context[option.id] = form[option.id] + context[option.id] = option.normalize(form[option.id]) # FIXME here we could error out if option.id in prefilled_answers: @@ -1542,7 +1539,8 @@ def prompt_or_validate_form( try: # Normalize and validate - values[option.id] = form[option.id] = option.normalize(value, option) + form[option.id] = option.normalize(value, option) + context[option.id] = form[option.id] except (ValidationError, YunohostValidationError) as e: # If in interactive cli, re-ask the current question if i < 4 and interactive: @@ -1562,11 +1560,13 @@ def prompt_or_validate_form( # Search for post actions in hooks post_hook = f"post_ask__{option.id}" if post_hook in hooks: - values.update(hooks[post_hook](option)) - # FIXME reapply new values to form to validate it - - answers.update(values) - context.update(values) + # Hooks looks like they can return multiple values, validate those + values = hooks[post_hook](option) + for option_id, value in values.items(): + option = next(opt for opt in options if option.id == option_id) + if option and isinstance(option, BaseInputOption): + form[option.id] = option.normalize(value, option) + context[option.id] = form[option.id] return form From 98ec5448f2aba6e8b51c5720b5f80fe600cf51ab Mon Sep 17 00:00:00 2001 From: axolotle Date: Wed, 19 Apr 2023 15:07:34 +0200 Subject: [PATCH 26/50] form: cli retries as variable to be patched in tests --- src/tests/test_questions.py | 8 ++++++++ src/utils/form.py | 5 ++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/tests/test_questions.py b/src/tests/test_questions.py index 387d5c0f9..6aca55e1a 100644 --- a/src/tests/test_questions.py +++ b/src/tests/test_questions.py @@ -26,6 +26,7 @@ from yunohost.utils.form import ( FileOption, evaluate_simple_js_expression, ) +from yunohost.utils import form from yunohost.utils.error import YunohostError, YunohostValidationError @@ -94,6 +95,12 @@ def patch_with_tty(): yield +@pytest.fixture +def patch_cli_retries(): + with patch.object(form, "MAX_RETRIES", 0): + yield + + # ╭───────────────────────────────────────────────────────╮ # │ ╭─╴╭─╴┌─╴╭╮╷╭─┐┌─╮╶┬╴╭─╮╭─╴ │ # │ ╰─╮│ ├─╴│││├─┤├┬╯ │ │ │╰─╮ │ @@ -405,6 +412,7 @@ def _test_intake_may_fail(raw_option, intake, expected_output): _test_intake(raw_option, intake, expected_output) +@pytest.mark.usefixtures("patch_cli_retries") # To avoid chain error logging class BaseTest: raw_option: dict[str, Any] = {} prefill: dict[Literal["raw_option", "prefill", "intake"], Any] diff --git a/src/utils/form.py b/src/utils/form.py index 1496f445b..bf50d93a4 100644 --- a/src/utils/form.py +++ b/src/utils/form.py @@ -1464,6 +1464,9 @@ def parse_prefilled_values( return values +MAX_RETRIES = 4 + + def prompt_or_validate_form( options: list[AnyOption], form: FormModel, @@ -1543,7 +1546,7 @@ def prompt_or_validate_form( context[option.id] = form[option.id] except (ValidationError, YunohostValidationError) as e: # If in interactive cli, re-ask the current question - if i < 4 and interactive: + if i < MAX_RETRIES and interactive: logger.error(str(e)) value = None continue From b45515049dfee9af447a4244ba653b86cb1225fb Mon Sep 17 00:00:00 2001 From: axolotle Date: Wed, 19 Apr 2023 17:26:46 +0200 Subject: [PATCH 27/50] config: fix wrong diff logic on settings apply --- src/utils/configpanel.py | 31 ++++++++++++++++++++++++++++--- src/utils/form.py | 2 +- 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/src/utils/configpanel.py b/src/utils/configpanel.py index eb9455a80..b48046ca3 100644 --- a/src/utils/configpanel.py +++ b/src/utils/configpanel.py @@ -282,6 +282,7 @@ class ConfigPanel: filter_key: "FilterKey" = (None, None, None) config: Union[ConfigPanelModel, None] = None form: Union["FormModel", None] = None + raw_settings: "RawSettings" = {} hooks: "Hooks" = {} @classmethod @@ -596,6 +597,8 @@ class ConfigPanel: self, config: ConfigPanelModel ) -> tuple[ConfigPanelModel, "RawSettings"]: raw_settings = self._get_raw_settings(config) + # Save `raw_settings` for diff at `_apply` + self.raw_settings = raw_settings values = {} for _, section, option in config.iter_children(): @@ -734,12 +737,34 @@ class ConfigPanel: mkdir(dir_path, mode=0o700) exclude_defaults = self.save_mode == "diff" - settings = form.dict(exclude_defaults=exclude_defaults, exclude=exclude) # type: ignore + # get settings keys filtered by filter_key + partial_settings_keys = form.__fields__.keys() + # get filtered settings + partial_settings = form.dict(exclude_defaults=exclude_defaults, exclude=exclude) # type: ignore + # get previous settings that we will updated with new settings + current_settings = self.raw_settings.copy() + + if exclude: + current_settings = { + key: value + for key, value in current_settings.items() + if key not in exclude + } + + for key in partial_settings_keys: + if ( + exclude_defaults + and key not in partial_settings + and key in current_settings + ): + del current_settings[key] + elif key in partial_settings: + current_settings[key] = partial_settings[key] # Save the settings to the .yaml file - write_to_yaml(self.save_path, settings) + write_to_yaml(self.save_path, current_settings) - return settings + return current_settings def _run_action(self, form: "FormModel", action_id: str): raise NotImplementedError() diff --git a/src/utils/form.py b/src/utils/form.py index bf50d93a4..d8ff4b8c7 100644 --- a/src/utils/form.py +++ b/src/utils/form.py @@ -1493,7 +1493,7 @@ def prompt_or_validate_form( # - we doesn't want to give a specific value # - we want to keep the previous value # - we want the default value - context[option.id] = form[option.id] = None + context[option.id] = None continue From 54cc23c90cc87d853cfac9e355efc8891e9bda07 Mon Sep 17 00:00:00 2001 From: axolotle Date: Wed, 19 Apr 2023 17:46:55 +0200 Subject: [PATCH 28/50] config: update SettingsConfigPanel.reset --- src/settings.py | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/src/settings.py b/src/settings.py index 0d0a5406b..5f645e3dc 100644 --- a/src/settings.py +++ b/src/settings.py @@ -23,7 +23,7 @@ from typing import TYPE_CHECKING, Any, Union from moulinette import m18n from yunohost.utils.error import YunohostError, YunohostValidationError -from yunohost.utils.configpanel import ConfigPanel +from yunohost.utils.configpanel import ConfigPanel, parse_filter_key from yunohost.utils.form import BaseOption from yunohost.regenconf import regen_conf from yunohost.firewall import firewall_reload @@ -31,6 +31,8 @@ from yunohost.log import is_unit_operation from yunohost.utils.legacy import translate_legacy_settings_to_configpanel_settings if TYPE_CHECKING: + from yunohost.log import OperationLogger + from pydantic.typing import AbstractSetIntStr, MappingIntStrAny from yunohost.utils.configpanel import ( @@ -148,25 +150,26 @@ class SettingsConfigPanel(ConfigPanel): return result - def reset(self, key="", operation_logger=None): - self.filter_key = key + def reset(self, key: Union[str, None] = None, operation_logger: Union["OperationLogger", None] = None,): + self.filter_key = parse_filter_key(key) # Read config panel toml - self._get_config_panel() + self.config, self.form = self._get_config_panel(prevalidate=True) - if not self.config: - raise YunohostValidationError("config_no_panel") + # FIXME find a better way to exclude previous settings + previous_settings = self.form.dict() - # Replace all values with default values - self.values = self._get_default_values() + for option in self.config.options: + if not option.readonly and (option.optional or option.default not in {None, ""}): + self.form[option.id] = option.normalize(option.default, option) - BaseOption.operation_logger = operation_logger + # FIXME Not sure if this is need (redact call to operation logger does it on all the instances) + # BaseOption.operation_logger = operation_logger if operation_logger: operation_logger.start() - try: - self._apply() + self._apply(self.form, previous_settings) except YunohostError: raise # Script got manually interrupted ... @@ -184,7 +187,9 @@ class SettingsConfigPanel(ConfigPanel): raise logger.success(m18n.n("global_settings_reset_success")) - operation_logger.success() + + if operation_logger: + operation_logger.success() def _get_raw_config(self) -> "RawConfig": raw_config = super()._get_raw_config() From 37b4eb956da2afe63e1c8bc4480d4fa4654fc7ca Mon Sep 17 00:00:00 2001 From: axolotle Date: Wed, 19 Apr 2023 18:52:59 +0200 Subject: [PATCH 29/50] typing: add missing type + misc typing fixes --- src/app.py | 13 ++++++---- src/domain.py | 2 +- src/log.py | 2 +- src/settings.py | 17 ++++++++++---- src/utils/configpanel.py | 51 ++++++++++++++++++++-------------------- src/utils/form.py | 46 +++++++++++++++++++----------------- 6 files changed, 72 insertions(+), 59 deletions(-) diff --git a/src/app.py b/src/app.py index 970a66fb2..0514066c9 100644 --- a/src/app.py +++ b/src/app.py @@ -1814,26 +1814,29 @@ class AppConfigPanel(ConfigPanel): form: "FormModel", previous_settings: dict[str, Any], exclude: Union["AbstractSetIntStr", "MappingIntStrAny", None] = None, - ): + ) -> None: env = {key: str(value) for key, value in form.dict().items()} return_content = self._call_config_script("apply", env=env) # If the script returned validation error # raise a ValidationError exception using # the first key - if return_content: - for key, message in return_content.get("validation_errors").items(): + errors = return_content.get("validation_errors") + if errors: + for key, message in errors.items(): raise YunohostValidationError( "app_argument_invalid", name=key, error=message, ) - def _run_action(self, form: "FormModel", action_id: str): + def _run_action(self, form: "FormModel", action_id: str) -> None: env = {key: str(value) for key, value in form.dict().items()} self._call_config_script(action_id, env=env) - def _call_config_script(self, action, env=None): + def _call_config_script( + self, action: str, env: Union[dict[str, Any], None] = None + ) -> dict[str, Any]: from yunohost.hook import hook_exec if env is None: diff --git a/src/domain.py b/src/domain.py index f0531e624..892220a68 100644 --- a/src/domain.py +++ b/src/domain.py @@ -720,7 +720,7 @@ class DomainConfigPanel(ConfigPanel): form: "FormModel", previous_settings: dict[str, Any], exclude: Union["AbstractSetIntStr", "MappingIntStrAny", None] = None, - ): + ) -> None: next_settings = { k: v for k, v in form.dict().items() if previous_settings.get(k) != v } diff --git a/src/log.py b/src/log.py index 13683d8ef..5a72411d8 100644 --- a/src/log.py +++ b/src/log.py @@ -469,7 +469,7 @@ class OperationLogger: This class record logs and metadata like context or start time/end time. """ - _instances: List[object] = [] + _instances: List["OperationLogger"] = [] def __init__(self, operation, related_to=None, **kwargs): # TODO add a way to not save password on app installation diff --git a/src/settings.py b/src/settings.py index 5f645e3dc..f70f9df61 100644 --- a/src/settings.py +++ b/src/settings.py @@ -136,7 +136,7 @@ class SettingsConfigPanel(ConfigPanel): save_mode = "diff" virtual_settings = {"root_password", "root_password_confirm", "passwordless_sudo"} - def __init__(self, config_path=None, save_path=None, creation=False): + def __init__(self, config_path=None, save_path=None, creation=False) -> None: super().__init__("settings") def get( @@ -150,7 +150,11 @@ class SettingsConfigPanel(ConfigPanel): return result - def reset(self, key: Union[str, None] = None, operation_logger: Union["OperationLogger", None] = None,): + def reset( + self, + key: Union[str, None] = None, + operation_logger: Union["OperationLogger", None] = None, + ) -> None: self.filter_key = parse_filter_key(key) # Read config panel toml @@ -160,8 +164,11 @@ class SettingsConfigPanel(ConfigPanel): previous_settings = self.form.dict() for option in self.config.options: - if not option.readonly and (option.optional or option.default not in {None, ""}): - self.form[option.id] = option.normalize(option.default, option) + if not option.readonly and ( + option.optional or option.default not in {None, ""} + ): + # FIXME Mypy complains about option.default not being a valid type for normalize but this should be ok + self.form[option.id] = option.normalize(option.default, option) # type: ignore # FIXME Not sure if this is need (redact call to operation logger does it on all the instances) # BaseOption.operation_logger = operation_logger @@ -230,7 +237,7 @@ class SettingsConfigPanel(ConfigPanel): form: "FormModel", previous_settings: dict[str, Any], exclude: Union["AbstractSetIntStr", "MappingIntStrAny", None] = None, - ): + ) -> None: root_password = form.get("root_password", None) root_password_confirm = form.get("root_password_confirm", None) passwordless_sudo = form.get("passwordless_sudo", None) diff --git a/src/utils/configpanel.py b/src/utils/configpanel.py index b48046ca3..b23df6ddd 100644 --- a/src/utils/configpanel.py +++ b/src/utils/configpanel.py @@ -21,7 +21,7 @@ import os import re from collections import OrderedDict from logging import getLogger -from typing import TYPE_CHECKING, Any, Literal, Sequence, Type, Union +from typing import TYPE_CHECKING, Any, Iterator, Literal, Sequence, Type, Union from pydantic import BaseModel, Extra, validator @@ -32,7 +32,6 @@ from yunohost.utils.error import YunohostError, YunohostValidationError from yunohost.utils.form import ( AnyOption, BaseInputOption, - BaseOption, BaseReadonlyOption, FileOption, OptionsModel, @@ -70,7 +69,7 @@ class ContainerModel(BaseModel): services: list[str] = [] help: Union[Translation, None] = None - def translate(self, i18n_key: Union[str, None] = None): + def translate(self, i18n_key: Union[str, None] = None) -> None: """ Translate `ask` and `name` attributes of panels and section. This is in place mutation. @@ -111,16 +110,16 @@ class SectionModel(ContainerModel, OptionsModel): ) @property - def is_action_section(self): + def is_action_section(self) -> bool: return any([option.type is OptionType.button for option in self.options]) - def is_visible(self, context: dict[str, Any]): + def is_visible(self, context: dict[str, Any]) -> bool: if isinstance(self.visible, bool): return self.visible return evaluate_simple_js_expression(self.visible, context=context) - def translate(self, i18n_key: Union[str, None] = None): + def translate(self, i18n_key: Union[str, None] = None) -> None: """ Call to `Container`'s `translate` for self translation + Call to `OptionsContainer`'s `translate_options` for options translation @@ -151,7 +150,7 @@ class PanelModel(ContainerModel): id=id, name=name, services=services, help=help, sections=sections ) - def translate(self, i18n_key: Union[str, None] = None): + def translate(self, i18n_key: Union[str, None] = None) -> None: """ Recursivly mutate translatable attributes to their translation """ @@ -181,14 +180,14 @@ class ConfigPanelModel(BaseModel): super().__init__(version=version, i18n=i18n, panels=panels) @property - def sections(self): + def sections(self) -> Iterator[SectionModel]: """Convinient prop to iter on all sections""" for panel in self.panels: for section in panel.sections: yield section @property - def options(self): + def options(self) -> Iterator[AnyOption]: """Convinient prop to iter on all options""" for section in self.sections: for option in section.options: @@ -231,7 +230,7 @@ class ConfigPanelModel(BaseModel): for option in section.options: yield (panel, section, option) - def translate(self): + def translate(self) -> None: """ Recursivly mutate translatable attributes to their translation """ @@ -239,7 +238,7 @@ class ConfigPanelModel(BaseModel): panel.translate(self.i18n) @validator("version", always=True) - def check_version(cls, value, field: "ModelField"): + def check_version(cls, value: float, field: "ModelField") -> float: if value < CONFIG_PANEL_VERSION_SUPPORTED: raise ValueError( f"Config panels version '{value}' are no longer supported." @@ -302,7 +301,9 @@ class ConfigPanel: entities = [] return entities - def __init__(self, entity, config_path=None, save_path=None, creation=False): + def __init__( + self, entity, config_path=None, save_path=None, creation=False + ) -> None: self.entity = entity self.config_path = config_path if not config_path: @@ -350,7 +351,7 @@ class ConfigPanel: if option is None: # FIXME i18n raise YunohostValidationError( - f"Couldn't find any option with id {option_id}" + f"Couldn't find any option with id {option_id}", raw_msg=True ) if isinstance(option, BaseReadonlyOption): @@ -398,7 +399,7 @@ class ConfigPanel: args: Union[str, None] = None, args_file: Union[str, None] = None, operation_logger: Union["OperationLogger", None] = None, - ): + ) -> None: self.filter_key = parse_filter_key(key) panel_id, section_id, option_id = self.filter_key @@ -466,7 +467,7 @@ class ConfigPanel: if operation_logger: operation_logger.success() - def list_actions(self): + def list_actions(self) -> dict[str, str]: actions = {} # FIXME : meh, loading the entire config panel is again going to cause @@ -486,7 +487,7 @@ class ConfigPanel: args: Union[str, None] = None, args_file: Union[str, None] = None, operation_logger: Union["OperationLogger", None] = None, - ): + ) -> None: # # FIXME : this stuff looks a lot like set() ... # @@ -666,7 +667,7 @@ class ConfigPanel: def _ask( self, config: ConfigPanelModel, - settings: "FormModel", + form: "FormModel", prefilled_answers: dict[str, Any] = {}, action_id: Union[str, None] = None, hooks: "Hooks" = {}, @@ -709,22 +710,22 @@ class ConfigPanel: if option.type is not OptionType.button or option.id == action_id ] - settings = prompt_or_validate_form( + form = prompt_or_validate_form( options, - settings, + form, prefilled_answers=prefilled_answers, context=context, hooks=hooks, ) - return settings + return form def _apply( self, form: "FormModel", previous_settings: dict[str, Any], exclude: Union["AbstractSetIntStr", "MappingIntStrAny", None] = None, - ) -> dict[str, Any]: + ) -> None: """ Save settings in yaml file. If `save_mode` is `"diff"` (which is the default), only values that are @@ -764,15 +765,13 @@ class ConfigPanel: # Save the settings to the .yaml file write_to_yaml(self.save_path, current_settings) - return current_settings - - def _run_action(self, form: "FormModel", action_id: str): + def _run_action(self, form: "FormModel", action_id: str) -> None: raise NotImplementedError() - def _reload_services(self): + def _reload_services(self) -> None: from yunohost.service import service_reload_or_restart - services_to_reload = self.config.services + services_to_reload = self.config.services if self.config else [] if services_to_reload: logger.info("Reloading services...") diff --git a/src/utils/form.py b/src/utils/form.py index d8ff4b8c7..4d62b0a29 100644 --- a/src/utils/form.py +++ b/src/utils/form.py @@ -32,7 +32,7 @@ from typing import ( Annotated, Any, Callable, - List, + Iterable, Literal, Mapping, Type, @@ -207,7 +207,7 @@ def js_to_python(expr): return py_expr -def evaluate_simple_js_expression(expr, context={}): +def evaluate_simple_js_expression(expr: str, context: dict[str, Any] = {}) -> bool: if not expr.strip(): return False node = ast.parse(js_to_python(expr), mode="eval").body @@ -650,7 +650,7 @@ class NumberOption(BaseInputOption): _none_as_empty_str = False @staticmethod - def normalize(value, option={}): + def normalize(value, option={}) -> Union[int, None]: if isinstance(value, int): return value @@ -704,7 +704,7 @@ class BooleanOption(BaseInputOption): _none_as_empty_str = False @staticmethod - def humanize(value, option={}): + def humanize(value, option={}) -> str: option = option.dict() if isinstance(option, BaseOption) else option yes = option.get("yes", 1) @@ -727,7 +727,7 @@ class BooleanOption(BaseInputOption): ) @staticmethod - def normalize(value, option={}): + def normalize(value, option={}) -> Any: option = option.dict() if isinstance(option, BaseOption) else option if isinstance(value, str): @@ -844,7 +844,7 @@ class WebPathOption(BaseInputOption): _annotation = str @staticmethod - def normalize(value, option={}): + def normalize(value, option={}) -> str: option = option.dict() if isinstance(option, BaseOption) else option if value is None: @@ -892,14 +892,14 @@ class FileOption(BaseInputOption): _upload_dirs: set[str] = set() @classmethod - def clean_upload_dirs(cls): + def clean_upload_dirs(cls) -> None: # Delete files uploaded from API for upload_dir in cls._upload_dirs: if os.path.exists(upload_dir): shutil.rmtree(upload_dir) @classmethod - def _value_post_validator(cls, value: Any, field: "ModelField") -> Any: + def _value_post_validator(cls, value: Any, field: "ModelField") -> str: from base64 import b64decode if not value: @@ -967,7 +967,6 @@ class BaseChoicesOption(BaseInputOption): choices = ( self.choices if isinstance(self.choices, list) else self.choices.keys() ) - # FIXME in case of dict, try to parse keys with `item_type` (at least number) return Literal[tuple(choices)] return self._annotation @@ -1006,6 +1005,7 @@ class BaseChoicesOption(BaseInputOption): class SelectOption(BaseChoicesOption): type: Literal[OptionType.select] = OptionType.select + filter: Literal[None] = None choices: Union[dict[str, Any], list[Any]] default: Union[str, None] _annotation = str @@ -1013,13 +1013,14 @@ class SelectOption(BaseChoicesOption): class TagsOption(BaseChoicesOption): type: Literal[OptionType.tags] = OptionType.tags + filter: Literal[None] = None choices: Union[list[str], None] = None pattern: Union[Pattern, None] = None default: Union[str, list[str], None] _annotation = str @staticmethod - def humanize(value, option={}): + def humanize(value, option={}) -> str: if isinstance(value, list): return ",".join(str(v) for v in value) if not value: @@ -1027,7 +1028,7 @@ class TagsOption(BaseChoicesOption): return value @staticmethod - def normalize(value, option={}): + def normalize(value, option={}) -> str: if isinstance(value, list): return ",".join(str(v) for v in value) if isinstance(value, str): @@ -1037,7 +1038,7 @@ class TagsOption(BaseChoicesOption): return value @property - def _dynamic_annotation(self): + def _dynamic_annotation(self) -> Type[str]: # TODO use Literal when serialization is seperated from validation # if self.choices is not None: # return Literal[tuple(self.choices)] @@ -1120,7 +1121,7 @@ class DomainOption(BaseChoicesOption): return _get_maindomain() @staticmethod - def normalize(value, option={}): + def normalize(value, option={}) -> str: if value.startswith("https://"): value = value[len("https://") :] elif value.startswith("http://"): @@ -1314,7 +1315,9 @@ class OptionsModel(BaseModel): options: list[Annotated[AnyOption, Field(discriminator="type")]] @staticmethod - def options_dict_to_list(options: dict[str, Any], optional: bool = False): + def options_dict_to_list( + options: dict[str, Any], optional: bool = False + ) -> list[dict[str, Any]]: return [ option | { @@ -1329,7 +1332,7 @@ class OptionsModel(BaseModel): def __init__(self, **kwargs) -> None: super().__init__(options=self.options_dict_to_list(kwargs)) - def translate_options(self, i18n_key: Union[str, None] = None): + def translate_options(self, i18n_key: Union[str, None] = None) -> None: """ Mutate in place translatable attributes of options to their translations """ @@ -1359,7 +1362,7 @@ class FormModel(BaseModel): validate_assignment = True extra = Extra.ignore - def __getitem__(self, name: str): + def __getitem__(self, name: str) -> Any: # FIXME # if a FormModel's required field is not instancied with a value, it is # not available as an attr and therefor triggers an `AttributeError` @@ -1372,7 +1375,7 @@ class FormModel(BaseModel): return getattr(self, name) - def __setitem__(self, name: str, value: Any): + def __setitem__(self, name: str, value: Any) -> None: setattr(self, name, value) def get(self, attr: str, default: Any = None) -> Any: @@ -1382,7 +1385,9 @@ class FormModel(BaseModel): return default -def build_form(options: list[AnyOption], name: str = "DynamicForm") -> Type[FormModel]: +def build_form( + options: Iterable[AnyOption], name: str = "DynamicForm" +) -> Type[FormModel]: """ Returns a dynamic pydantic model class that can be used as a form. Parsing/validation occurs at instanciation and assignements. @@ -1468,7 +1473,7 @@ MAX_RETRIES = 4 def prompt_or_validate_form( - options: list[AnyOption], + options: Iterable[AnyOption], form: FormModel, prefilled_answers: dict[str, Any] = {}, context: Context = {}, @@ -1503,7 +1508,6 @@ def prompt_or_validate_form( if isinstance(option, BaseReadonlyOption) or option.readonly: if isinstance(option, BaseInputOption): - # FIXME normalized needed, form[option.id] should already be normalized # only update the context with the value context[option.id] = option.normalize(form[option.id]) @@ -1623,7 +1627,7 @@ def ask_questions_and_parse_answers( return (model.options, form) -def hydrate_questions_with_choices(raw_questions: List) -> List: +def hydrate_questions_with_choices(raw_questions: list[dict[str, Any]]) -> list[dict[str, Any]]: out = [] for raw_question in raw_questions: From 2a28e289adcd0224ecd4856314c2ae75b2135b7c Mon Sep 17 00:00:00 2001 From: axolotle Date: Wed, 19 Apr 2023 19:58:14 +0200 Subject: [PATCH 30/50] form: rework 'hydrate_questions...' with a new 'parse_raw_options' that parse and validate options --- src/app.py | 5 ++--- src/utils/form.py | 50 +++++++++++++++++++++++++++++------------------ 2 files changed, 33 insertions(+), 22 deletions(-) diff --git a/src/app.py b/src/app.py index 0514066c9..8ab683d81 100644 --- a/src/app.py +++ b/src/app.py @@ -51,7 +51,7 @@ from yunohost.utils.form import ( DomainOption, WebPathOption, ask_questions_and_parse_answers, - hydrate_questions_with_choices, + parse_raw_options, ) from yunohost.utils.i18n import _value_for_locale from yunohost.utils.error import YunohostError, YunohostValidationError @@ -963,8 +963,7 @@ def app_upgrade( def app_manifest(app, with_screenshot=False): manifest, extracted_app_folder = _extract_app(app) - raw_questions = manifest.get("install", {}).values() - manifest["install"] = hydrate_questions_with_choices(raw_questions) + manifest["install"] = parse_raw_options(manifest.get("install", {}), serialize=True) # Add a base64 image to be displayed in web-admin if with_screenshot and Moulinette.interface.type == "api": diff --git a/src/utils/form.py b/src/utils/form.py index 4d62b0a29..dce4b94c8 100644 --- a/src/utils/form.py +++ b/src/utils/form.py @@ -29,6 +29,7 @@ from logging import getLogger from typing import ( TYPE_CHECKING, cast, + overload, Annotated, Any, Callable, @@ -1609,6 +1610,33 @@ def ask_questions_and_parse_answers( context = {**current_values, **answers} + model_options = parse_raw_options(raw_options, serialize=False) + # Build the form from those questions and instantiate it without + # parsing/validation (construct) since it may contains required questions. + form = build_form(model_options).construct() + form = prompt_or_validate_form( + model_options, form, prefilled_answers=answers, context=context, hooks=hooks + ) + return (model_options, form) + + +@overload +def parse_raw_options( + raw_options: dict[str, Any], serialize: Literal[True] +) -> list[dict[str, Any]]: + ... + + +@overload +def parse_raw_options( + raw_options: dict[str, Any], serialize: Literal[False] = False +) -> list[AnyOption]: + ... + + +def parse_raw_options( + raw_options: dict[str, Any], serialize: bool = False +) -> Union[list[dict[str, Any]], list[AnyOption]]: # Validate/parse the options attributes try: model = OptionsModel(**raw_options) @@ -1618,24 +1646,8 @@ def ask_questions_and_parse_answers( raise YunohostValidationError(error, raw_msg=True) model.translate_options() - # Build the form from those questions and instantiate it without - # parsing/validation (construct) since it may contains required questions. - form = build_form(model.options).construct() - form = prompt_or_validate_form( - model.options, form, prefilled_answers=answers, context=context, hooks=hooks - ) - return (model.options, form) + if serialize: + return model.dict()["options"] -def hydrate_questions_with_choices(raw_questions: list[dict[str, Any]]) -> list[dict[str, Any]]: - out = [] - - for raw_question in raw_questions: - raw_question = hydrate_option_type(raw_question) - question = OPTIONS[raw_question["type"]](**raw_question) - if isinstance(question, BaseChoicesOption) and question.choices: - raw_question["choices"] = question.choices - raw_question["default"] = question.default - out.append(raw_question) - - return out + return model.options From 6bef4b1e0e9a856ec4dfb026d704addedfb13dc6 Mon Sep 17 00:00:00 2001 From: axolotle Date: Wed, 19 Apr 2023 20:00:02 +0200 Subject: [PATCH 31/50] app: remove call of 'domain_config_get' to avoid infinite recursion --- src/app.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/app.py b/src/app.py index 8ab683d81..b3e34221d 100644 --- a/src/app.py +++ b/src/app.py @@ -130,7 +130,6 @@ def app_info(app, full=False, upgradable=False): Get info for a specific app """ from yunohost.permission import user_permission_list - from yunohost.domain import domain_config_get _assert_is_installed(app) @@ -229,9 +228,7 @@ def app_info(app, full=False, upgradable=False): ret["is_webapp"] = "domain" in settings and "path" in settings if ret["is_webapp"]: - ret["is_default"] = ( - domain_config_get(settings["domain"], "feature.app.default_app") == app - ) + ret["is_default"] = settings.get("default_app", "_none") ret["supports_change_url"] = os.path.exists( os.path.join(setting_path, "scripts", "change_url") From b778aaf780ff5deb708bd2624547b4fc739a3102 Mon Sep 17 00:00:00 2001 From: axolotle Date: Wed, 19 Apr 2023 20:01:18 +0200 Subject: [PATCH 32/50] form: remove ChoosableOptions for now --- src/utils/form.py | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/src/utils/form.py b/src/utils/form.py index dce4b94c8..81bd10ba8 100644 --- a/src/utils/form.py +++ b/src/utils/form.py @@ -938,18 +938,6 @@ class FileOption(BaseInputOption): # ─ CHOICES ─────────────────────────────────────────────── -ChoosableOptions = Literal[ - OptionType.string, - OptionType.color, - OptionType.number, - OptionType.date, - OptionType.time, - OptionType.email, - OptionType.path, - OptionType.url, -] - - class BaseChoicesOption(BaseInputOption): # FIXME probably forbid choices to be None? filter: Union[JSExpression, None] = None # filter before choices From bd9bf29a88670efc495680137125f49a01c6ddbe Mon Sep 17 00:00:00 2001 From: axolotle Date: Thu, 20 Apr 2023 15:48:51 +0200 Subject: [PATCH 33/50] debian: add python3-pydantic + python3-email-validator dependencies --- debian/control | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/debian/control b/debian/control index 9f156ddea..70a780af7 100644 --- a/debian/control +++ b/debian/control @@ -16,7 +16,7 @@ Depends: ${python3:Depends}, ${misc:Depends} , python3-toml, python3-packaging, python3-publicsuffix2 , python3-ldap, python3-zeroconf (>= 0.47), python3-lexicon, , python3-cryptography, python3-jwt - , python-is-python3 + , python-is-python3, python3-pydantic, python3-email-validator , nginx, nginx-extras (>=1.22) , apt, apt-transport-https, apt-utils, dirmngr , openssh-server, iptables, fail2ban, bind9-dnsutils From fccb291d7885c92a9e26a34867a715e9b9ef6045 Mon Sep 17 00:00:00 2001 From: axolotle Date: Fri, 21 Apr 2023 22:05:49 +0200 Subject: [PATCH 34/50] form: readd `pattern` to `path` --- src/utils/form.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/utils/form.py b/src/utils/form.py index 81bd10ba8..76328f113 100644 --- a/src/utils/form.py +++ b/src/utils/form.py @@ -396,6 +396,7 @@ class AlertOption(BaseReadonlyOption): class ButtonOption(BaseReadonlyOption): type: Literal[OptionType.button] = OptionType.button + bind: Literal["null"] = "null" help: Union[Translation, None] = None style: State = State.success icon: Union[str, None] = None @@ -839,10 +840,8 @@ class EmailOption(BaseInputOption): _annotation = EmailStr -class WebPathOption(BaseInputOption): +class WebPathOption(BaseStringOption): type: Literal[OptionType.path] = OptionType.path - default: Union[str, None] - _annotation = str @staticmethod def normalize(value, option={}) -> str: @@ -876,7 +875,6 @@ class WebPathOption(BaseInputOption): class URLOption(BaseStringOption): type: Literal[OptionType.url] = OptionType.url - default: Union[str, None] _annotation = HttpUrl From 48f882ecd314ba5cb9849dc55372a8ebc074834b Mon Sep 17 00:00:00 2001 From: axolotle Date: Sat, 22 Apr 2023 18:47:05 +0200 Subject: [PATCH 35/50] form+configpanel: reflect Section `optional` value to all its Options --- src/utils/configpanel.py | 3 ++- src/utils/form.py | 3 +-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/utils/configpanel.py b/src/utils/configpanel.py index b23df6ddd..55dd07787 100644 --- a/src/utils/configpanel.py +++ b/src/utils/configpanel.py @@ -95,9 +95,10 @@ class SectionModel(ContainerModel, OptionsModel): services: list[str] = [], help: Union[Translation, None] = None, visible: Union[bool, str] = True, + optional: bool = True, **kwargs, ) -> None: - options = self.options_dict_to_list(kwargs, optional=True) + options = self.options_dict_to_list(kwargs, optional=optional) ContainerModel.__init__( self, diff --git a/src/utils/form.py b/src/utils/form.py index 76328f113..bc21c309a 100644 --- a/src/utils/form.py +++ b/src/utils/form.py @@ -1308,9 +1308,8 @@ class OptionsModel(BaseModel): return [ option | { - "id": id_, + "id": option.get("id", id_), "type": option.get("type", "string"), - # ConfigPanel options needs to be set as optional by default "optional": option.get("optional", optional), } for id_, option in options.items() From d370cb0b241695d9d1dd6cf6e0c2482ac3f66648 Mon Sep 17 00:00:00 2001 From: axolotle Date: Mon, 24 Apr 2023 15:09:44 +0200 Subject: [PATCH 36/50] configpanel: add `value` in options dict for config get --full --- src/utils/configpanel.py | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/utils/configpanel.py b/src/utils/configpanel.py index 55dd07787..776577d3e 100644 --- a/src/utils/configpanel.py +++ b/src/utils/configpanel.py @@ -363,7 +363,20 @@ class ConfigPanel: # Format result in 'classic' or 'export' mode self.config.translate() logger.debug(f"Formating result in '{mode}' mode") + + if mode == "full": + result = self.config.dict(exclude_none=True) + + for panel in result["panels"]: + for section in panel["sections"]: + for opt in section["options"]: + instance = self.config.get_option(opt["id"]) + if isinstance(instance, BaseInputOption): + opt["value"] = instance.normalize(self.form[opt["id"]], instance) + return result + result = OrderedDict() + for panel in self.config.panels: for section in panel.sections: if section.is_action_section and mode != "full": @@ -388,10 +401,7 @@ class ConfigPanel: "value" ] = "**************" # Prevent displaying password in `config get` - if mode == "full": - return self.config.dict(exclude_none=True) - else: - return result + return result def set( self, From 51d302bf180bf6f8fbc3cbf16494147f2a7f2b7d Mon Sep 17 00:00:00 2001 From: axolotle Date: Mon, 24 Apr 2023 15:10:27 +0200 Subject: [PATCH 37/50] configpanel: `is_action_section` as attr --- src/utils/configpanel.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/utils/configpanel.py b/src/utils/configpanel.py index 776577d3e..756981b63 100644 --- a/src/utils/configpanel.py +++ b/src/utils/configpanel.py @@ -86,6 +86,7 @@ class ContainerModel(BaseModel): class SectionModel(ContainerModel, OptionsModel): visible: Union[bool, str] = True optional: bool = True + is_action_section: bool = False # Don't forget to pass arguments to super init def __init__( @@ -99,7 +100,7 @@ class SectionModel(ContainerModel, OptionsModel): **kwargs, ) -> None: options = self.options_dict_to_list(kwargs, optional=optional) - + is_action_section = any([option["type"] == OptionType.button for option in options]) ContainerModel.__init__( self, id=id, @@ -108,12 +109,9 @@ class SectionModel(ContainerModel, OptionsModel): help=help, visible=visible, options=options, + is_action_section=is_action_section, ) - @property - def is_action_section(self) -> bool: - return any([option.type is OptionType.button for option in self.options]) - def is_visible(self, context: dict[str, Any]) -> bool: if isinstance(self.visible, bool): return self.visible From 2f4c88ec55b85fbe813470d0881896331a30551e Mon Sep 17 00:00:00 2001 From: axolotle Date: Mon, 24 Apr 2023 15:12:54 +0200 Subject: [PATCH 38/50] form: parse pydantic error in logging --- src/utils/form.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/utils/form.py b/src/utils/form.py index bc21c309a..36e816110 100644 --- a/src/utils/form.py +++ b/src/utils/form.py @@ -1537,7 +1537,11 @@ def prompt_or_validate_form( except (ValidationError, YunohostValidationError) as e: # If in interactive cli, re-ask the current question if i < MAX_RETRIES and interactive: - logger.error(str(e)) + logger.error( + "\n".join([err["msg"] for err in e.errors()]) + if isinstance(e, ValidationError) + else str(e) + ) value = None continue @@ -1627,8 +1631,7 @@ def parse_raw_options( model = OptionsModel(**raw_options) except ValidationError as e: error = "\n".join([err["msg"] for err in e.errors()]) - # FIXME use YunohostError instead since it is not really a user mistake? - raise YunohostValidationError(error, raw_msg=True) + raise YunohostError(error, raw_msg=True) model.translate_options() From 6953a8bf15202856c25c9114f3491dabe9b8f5ea Mon Sep 17 00:00:00 2001 From: axolotle Date: Mon, 24 Apr 2023 15:22:10 +0200 Subject: [PATCH 39/50] configpanel: quick fix option typing --- src/utils/configpanel.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/utils/configpanel.py b/src/utils/configpanel.py index 756981b63..a602266da 100644 --- a/src/utils/configpanel.py +++ b/src/utils/configpanel.py @@ -21,7 +21,7 @@ import os import re from collections import OrderedDict from logging import getLogger -from typing import TYPE_CHECKING, Any, Iterator, Literal, Sequence, Type, Union +from typing import TYPE_CHECKING, Any, Iterator, Literal, Sequence, Type, Union, cast from pydantic import BaseModel, Extra, validator @@ -100,7 +100,9 @@ class SectionModel(ContainerModel, OptionsModel): **kwargs, ) -> None: options = self.options_dict_to_list(kwargs, optional=optional) - is_action_section = any([option["type"] == OptionType.button for option in options]) + is_action_section = any( + [option["type"] == OptionType.button for option in options] + ) ContainerModel.__init__( self, id=id, @@ -370,7 +372,9 @@ class ConfigPanel: for opt in section["options"]: instance = self.config.get_option(opt["id"]) if isinstance(instance, BaseInputOption): - opt["value"] = instance.normalize(self.form[opt["id"]], instance) + opt["value"] = instance.normalize( + self.form[opt["id"]], instance + ) return result result = OrderedDict() @@ -381,6 +385,9 @@ class ConfigPanel: continue for option in section.options: + # FIXME not sure why option resolves as possibly `None` + option = cast(AnyOption, option) + if mode == "export": if isinstance(option, BaseInputOption): result[option.id] = self.form[option.id] From ef860ee6eebb8d64aaad2707dd06363755183eed Mon Sep 17 00:00:00 2001 From: axolotle Date: Fri, 28 Apr 2023 16:11:38 +0200 Subject: [PATCH 40/50] form: default type to "select" if choices in option --- src/utils/form.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/form.py b/src/utils/form.py index 36e816110..396419f3e 100644 --- a/src/utils/form.py +++ b/src/utils/form.py @@ -1309,7 +1309,7 @@ class OptionsModel(BaseModel): option | { "id": option.get("id", id_), - "type": option.get("type", "string"), + "type": option.get("type", "select" if "choices" in option else "string"), "optional": option.get("optional", optional), } for id_, option in options.items() From 3f417bb9b32a45be1e8dd48c8ee806cc8c0cdde9 Mon Sep 17 00:00:00 2001 From: axolotle Date: Fri, 28 Apr 2023 17:23:02 +0200 Subject: [PATCH 41/50] tests: update error instance in tests to YunohostError for packaging errors --- src/tests/test_questions.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/tests/test_questions.py b/src/tests/test_questions.py index 6aca55e1a..8f8e701e9 100644 --- a/src/tests/test_questions.py +++ b/src/tests/test_questions.py @@ -595,7 +595,7 @@ class TestAlert(TestDisplayText): (None, None, {"ask": "Some text\na new line"}), (None, None, {"ask": {"en": "Some text\na new line", "fr": "Un peu de texte\nune nouvelle ligne"}}), *[(None, None, {"ask": "question", "style": style}) for style in ("success", "info", "warning", "danger")], - (None, FAIL, {"ask": "question", "style": "nimp"}), + (None, YunohostError, {"ask": "question", "style": "nimp"}), ] # fmt: on @@ -737,7 +737,7 @@ class TestPassword(BaseTest): *all_fails([], ["one"], {}, raw_option={"optional": True}, error=AttributeError), # FIXME those fails with AttributeError *all_fails("none", "_none", "False", "True", "0", "1", "-1", "1337", "13.37", "[]", ",", "['one']", "one,two", r"{}", "value", "value\n", raw_option={"optional": True}), *nones(None, "", output=""), - ("s3cr3t!!", FAIL, {"default": "SUPAs3cr3t!!"}), # default is forbidden + ("s3cr3t!!", YunohostError, {"default": "SUPAs3cr3t!!"}), # default is forbidden *xpass(scenarios=[ ("s3cr3t!!", "s3cr3t!!", {"example": "SUPAs3cr3t!!"}), # example is forbidden ], reason="Should fail; example is forbidden"), @@ -749,7 +749,7 @@ class TestPassword(BaseTest): ("secret", FAIL), *[("supersecret" + char, FAIL) for char in FORBIDDEN_PASSWORD_CHARS], # FIXME maybe add ` \n` to the list? # readonly - ("s3cr3t!!", FAIL, {"readonly": True}), # readonly is forbidden + ("s3cr3t!!", YunohostError, {"readonly": True}), # readonly is forbidden ] # fmt: on @@ -1474,10 +1474,10 @@ class TestTags(BaseTest): # basic types (not in a list) should fail *all_fails(True, False, -1, 0, 1, 1337, 13.37, {}), # Mixed choices should fail - ([False, True, -1, 0, 1, 1337, 13.37, [], ["one"], {}], FAIL, {"choices": [False, True, -1, 0, 1, 1337, 13.37, [], ["one"], {}]}), - ("False,True,-1,0,1,1337,13.37,[],['one'],{}", FAIL, {"choices": [False, True, -1, 0, 1, 1337, 13.37, [], ["one"], {}]}), - *all_fails(*([t] for t in [False, True, -1, 0, 1, 1337, 13.37, [], ["one"], {}]), raw_option={"choices": [False, True, -1, 0, 1, 1337, 13.37, [], ["one"], {}]}), - *all_fails(*([str(t)] for t in [False, True, -1, 0, 1, 1337, 13.37, [], ["one"], {}]), raw_option={"choices": [False, True, -1, 0, 1, 1337, 13.37, [], ["one"], {}]}), + ([False, True, -1, 0, 1, 1337, 13.37, [], ["one"], {}], YunohostError, {"choices": [False, True, -1, 0, 1, 1337, 13.37, [], ["one"], {}]}), + ("False,True,-1,0,1,1337,13.37,[],['one'],{}", YunohostError, {"choices": [False, True, -1, 0, 1, 1337, 13.37, [], ["one"], {}]}), + *all_fails(*([t] for t in [False, True, -1, 0, 1, 1337, 13.37, [], ["one"], {}]), raw_option={"choices": [False, True, -1, 0, 1, 1337, 13.37, [], ["one"], {}]}, error=YunohostError), + *all_fails(*([str(t)] for t in [False, True, -1, 0, 1, 1337, 13.37, [], ["one"], {}]), raw_option={"choices": [False, True, -1, 0, 1, 1337, 13.37, [], ["one"], {}]}, error=YunohostError), # readonly ("one", "one,two", {"readonly": True, "choices": ["one", "two"], "default": "one,two"}), ] @@ -1527,7 +1527,7 @@ class TestDomain(BaseTest): ("doesnt_exist.pouet", FAIL, {}), ("fake.com", FAIL, {"choices": ["fake.com"]}), # readonly - (domains1[0], FAIL, {"readonly": True}), # readonly is forbidden + (domains1[0], YunohostError, {"readonly": True}), # readonly is forbidden ] }, { @@ -1627,7 +1627,7 @@ class TestApp(BaseTest): (installed_non_webapp["id"], installed_non_webapp["id"]), (installed_non_webapp["id"], FAIL, {"filter": "is_webapp"}), # readonly - (installed_non_webapp["id"], FAIL, {"readonly": True}), # readonly is forbidden + (installed_non_webapp["id"], YunohostError, {"readonly": True}), # readonly is forbidden ] }, ] @@ -1744,7 +1744,7 @@ class TestUser(BaseTest): ("", regular_username, {"default": regular_username}) ], reason="Should throw 'no default allowed'"), # readonly - (admin_username, FAIL, {"readonly": True}), # readonly is forbidden + (admin_username, YunohostError, {"readonly": True}), # readonly is forbidden ] }, ] @@ -1824,9 +1824,9 @@ class TestGroup(BaseTest): "scenarios": [ ("custom_group", "custom_group"), *all_as("", None, output="visitors", raw_option={"default": "visitors"}), - ("", FAIL, {"default": "custom_group"}), # Not allowed to set a default which is not a default group + ("", YunohostError, {"default": "custom_group"}), # Not allowed to set a default which is not a default group # readonly - ("admins", FAIL, {"readonly": True}), # readonly is forbidden + ("admins", YunohostError, {"readonly": True}), # readonly is forbidden ] }, ] From 3a5d353c4b60770b0ed5ddaaa1a9caa85e66895d Mon Sep 17 00:00:00 2001 From: axolotle Date: Fri, 28 Apr 2023 17:24:11 +0200 Subject: [PATCH 42/50] form: force option type to 'select' if there's 'choices' --- src/tests/test_questions.py | 4 ++-- src/utils/form.py | 26 ++++++++++++++++++-------- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/src/tests/test_questions.py b/src/tests/test_questions.py index 8f8e701e9..fbbf757c9 100644 --- a/src/tests/test_questions.py +++ b/src/tests/test_questions.py @@ -1989,10 +1989,10 @@ def test_option_default_type_with_choices_is_select(): } answers = {"some_choices": "a", "some_legacy": "a"} - options = ask_questions_and_parse_answers(questions, answers) + options, form = ask_questions_and_parse_answers(questions, answers) for option in options: assert option.type == "select" - assert option.value == "a" + assert form[option.id] == "a" @pytest.mark.skip # we should do something with this example diff --git a/src/utils/form.py b/src/utils/form.py index 396419f3e..c586467fc 100644 --- a/src/utils/form.py +++ b/src/utils/form.py @@ -1305,15 +1305,25 @@ class OptionsModel(BaseModel): def options_dict_to_list( options: dict[str, Any], optional: bool = False ) -> list[dict[str, Any]]: - return [ - option - | { - "id": option.get("id", id_), - "type": option.get("type", "select" if "choices" in option else "string"), - "optional": option.get("optional", optional), + options_list = [] + + for id_, data in options.items(): + option = data | { + "id": data.get("id", id_), + "type": data.get("type", OptionType.select if "choices" in data else OptionType.string), + "optional": data.get("optional", optional), } - for id_, option in options.items() - ] + + # LEGACY (`choices` in option `string` used to be valid) + if "choices" in option and option["type"] == OptionType.string: + logger.warning( + f"Packagers: option {id_} has 'choices' but has type 'string', use 'select' instead to remove this warning." + ) + option["type"] = OptionType.select + + options_list.append(option) + + return options_list def __init__(self, **kwargs) -> None: super().__init__(options=self.options_dict_to_list(kwargs)) From 3cae07970e913f04a249b3e930b9693a08aebaa9 Mon Sep 17 00:00:00 2001 From: axolotle Date: Sun, 30 Apr 2023 17:29:28 +0200 Subject: [PATCH 43/50] form: remove no longer used hydrate_option_type method --- src/utils/form.py | 21 ++++----------------- 1 file changed, 4 insertions(+), 17 deletions(-) diff --git a/src/utils/form.py b/src/utils/form.py index c586467fc..07be55312 100644 --- a/src/utils/form.py +++ b/src/utils/form.py @@ -1310,7 +1310,10 @@ class OptionsModel(BaseModel): for id_, data in options.items(): option = data | { "id": data.get("id", id_), - "type": data.get("type", OptionType.select if "choices" in data else OptionType.string), + "type": data.get( + "type", + OptionType.select if "choices" in data else OptionType.string, + ), "optional": data.get("optional", optional), } @@ -1414,22 +1417,6 @@ def build_form( ) -def hydrate_option_type(raw_option: dict[str, Any]) -> dict[str, Any]: - type_ = raw_option.get( - "type", OptionType.select if "choices" in raw_option else OptionType.string - ) - # LEGACY (`choices` in option `string` used to be valid) - if "choices" in raw_option and type_ == OptionType.string: - logger.warning( - f"Packagers: option {raw_option['id']} has 'choices' but has type 'string', use 'select' instead to remove this warning." - ) - type_ = OptionType.select - - raw_option["type"] = type_ - - return raw_option - - # ╭───────────────────────────────────────────────────────╮ # │ ╷ ╷╶┬╴╶┬╴╷ ╭─╴ │ # │ │ │ │ │ │ ╰─╮ │ From 66cb855c0ca0f484e62db4087bac8b802af0b0bc Mon Sep 17 00:00:00 2001 From: axolotle Date: Sun, 22 Oct 2023 17:47:43 +0200 Subject: [PATCH 44/50] domain: type fix --- src/domain.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/domain.py b/src/domain.py index 892220a68..7ac8a50cb 100644 --- a/src/domain.py +++ b/src/domain.py @@ -753,10 +753,10 @@ class DomainConfigPanel(ConfigPanel): # that can be read by the portal API. # FIXME remove those from the config panel saved values? - portal_values = form.dict(include=portal_options) + portal_values = form.dict(include=set(portal_options)) portal_settings_path = Path(f"{PORTAL_SETTINGS_DIR}/{self.entity}.json") - portal_settings = {"apps": {}} + portal_settings: dict[str, Any] = {"apps": {}} if portal_settings_path.exists(): portal_settings.update(read_json(str(portal_settings_path))) From e7b43c763c28af767fef4b7e0acd97df9fb9059a Mon Sep 17 00:00:00 2001 From: axolotle Date: Sun, 22 Oct 2023 17:49:08 +0200 Subject: [PATCH 45/50] configpanel: do not raise error if no settings file --- src/utils/configpanel.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/utils/configpanel.py b/src/utils/configpanel.py index a602266da..d79b8a80a 100644 --- a/src/utils/configpanel.py +++ b/src/utils/configpanel.py @@ -577,7 +577,8 @@ class ConfigPanel: def _get_raw_settings(self, config: ConfigPanelModel) -> "RawSettings": if not self.save_path or not os.path.exists(self.save_path): - raise YunohostValidationError("config_no_settings") + return {} + # raise YunohostValidationError("config_no_settings") return read_yaml(self.save_path) From 3a31984e3c2c193fc3ab5b2b27acbabd8e50d6db Mon Sep 17 00:00:00 2001 From: axolotle Date: Sun, 22 Oct 2023 17:51:04 +0200 Subject: [PATCH 46/50] configpanel: allow other ConfigPanels to have no settings defined --- src/app.py | 1 + src/utils/configpanel.py | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/app.py b/src/app.py index b3e34221d..21219b2ce 100644 --- a/src/app.py +++ b/src/app.py @@ -1801,6 +1801,7 @@ class AppConfigPanel(ConfigPanel): entity_type = "app" 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") + settings_must_be_defined: bool = True def _get_raw_settings(self, config: "ConfigPanelModel") -> "RawSettings": return self._call_config_script("show") diff --git a/src/utils/configpanel.py b/src/utils/configpanel.py index d79b8a80a..325f6579d 100644 --- a/src/utils/configpanel.py +++ b/src/utils/configpanel.py @@ -279,6 +279,7 @@ class ConfigPanel: save_path_tpl: Union[str, None] = None config_path_tpl = "/usr/share/yunohost/config_{entity_type}.toml" save_mode = "full" + settings_must_be_defined: bool = False filter_key: "FilterKey" = (None, None, None) config: Union[ConfigPanelModel, None] = None form: Union["FormModel", None] = None @@ -627,7 +628,7 @@ class ConfigPanel: value = option.default elif option.type is OptionType.file or option.bind == "null": continue - else: + elif self.settings_must_be_defined: raise YunohostError( f"Config panel question '{option.id}' should be initialized with a value during install or upgrade.", raw_msg=True, From 9134515604782cbde14a08745c2da393a5880958 Mon Sep 17 00:00:00 2001 From: axolotle Date: Sun, 22 Oct 2023 17:53:50 +0200 Subject: [PATCH 47/50] domain:config: make 'registrar' info a frozen input since an alert has no value --- src/dns.py | 88 +++++++++++++++++++++++------------------------------- 1 file changed, 38 insertions(+), 50 deletions(-) diff --git a/src/dns.py b/src/dns.py index 07ff2fb21..fc4b26a75 100644 --- a/src/dns.py +++ b/src/dns.py @@ -502,11 +502,26 @@ def _get_relative_name_for_dns_zone(domain, base_dns_zone): def _get_registrar_config_section(domain): from lexicon.providers.auto import _relevant_provider_for_domain - registrar_infos = { - "name": m18n.n( - "registrar_infos" - ), # This is meant to name the config panel section, for proper display in the webadmin - } + registrar_infos = OrderedDict( + { + "name": m18n.n( + "registrar_infos" + ), # This is meant to name the config panel section, for proper display in the webadmin + "registrar": OrderedDict( + { + "readonly": True, + "visible": False, + "default": None, + } + ), + "infos": OrderedDict( + { + "type": "alert", + "style": "info", + } + ), + } + ) dns_zone = _get_dns_zone_for_domain(domain) @@ -519,61 +534,34 @@ def _get_registrar_config_section(domain): else: parent_domain_link = parent_domain - registrar_infos["registrar"] = OrderedDict( - { - "type": "alert", - "style": "info", - "ask": m18n.n( - "domain_dns_registrar_managed_in_parent_domain", - parent_domain=parent_domain, - parent_domain_link=parent_domain_link, - ), - "default": "parent_domain", - } + registrar_infos["registrar"]["default"] = "parent_domain" + registrar_infos["infos"]["ask"] = m18n.n( + "domain_dns_registrar_managed_in_parent_domain", + parent_domain=parent_domain, + parent_domain_link=parent_domain_link, ) - return OrderedDict(registrar_infos) + return registrar_infos # TODO big project, integrate yunohost's dynette as a registrar-like provider # TODO big project, integrate other dyndns providers such as netlib.re, or cf the list of dyndns providers supported by cloudron... if is_yunohost_dyndns_domain(dns_zone): - registrar_infos["registrar"] = OrderedDict( - { - "type": "alert", - "style": "success", - "ask": m18n.n("domain_dns_registrar_yunohost"), - "default": "yunohost", - } - ) - return OrderedDict(registrar_infos) + registrar_infos["registrar"]["default"] = "yunohost" + registrar_infos["infos"]["style"] = "success" + registrar_infos["infos"]["ask"] = m18n.n("domain_dns_registrar_yunohost") + + return registrar_infos elif is_special_use_tld(dns_zone): - registrar_infos["registrar"] = OrderedDict( - { - "type": "alert", - "style": "info", - "ask": m18n.n("domain_dns_conf_special_use_tld"), - "default": None, - } - ) + registrar_infos["infos"]["ask"] = m18n.n("domain_dns_conf_special_use_tld") try: registrar = _relevant_provider_for_domain(dns_zone)[0] except ValueError: - registrar_infos["registrar"] = OrderedDict( - { - "type": "alert", - "style": "warning", - "ask": m18n.n("domain_dns_registrar_not_supported"), - "default": None, - } - ) + registrar_infos["registrar"]["default"] = None + registrar_infos["infos"]["ask"] = m18n.n("domain_dns_registrar_not_supported") else: - registrar_infos["registrar"] = OrderedDict( - { - "type": "alert", - "style": "info", - "ask": m18n.n("domain_dns_registrar_supported", registrar=registrar), - "default": registrar, - } + registrar_infos["registrar"]["default"] = registrar + registrar_infos["infos"]["ask"] = m18n.n( + "domain_dns_registrar_supported", registrar=registrar ) TESTED_REGISTRARS = ["ovh", "gandi"] @@ -601,7 +589,7 @@ def _get_registrar_config_section(domain): infos["optional"] = infos.get("optional", "False") registrar_infos.update(registrar_credentials) - return OrderedDict(registrar_infos) + return registrar_infos def _get_registar_settings(domain): From c4c79c61fe30491388a2cda4fe5627de81428263 Mon Sep 17 00:00:00 2001 From: axolotle Date: Wed, 25 Oct 2023 15:06:10 +0200 Subject: [PATCH 48/50] configpanel: forbid extra props on BaseOption + accordingly fix tests --- src/tests/test_questions.py | 4 ++++ src/utils/form.py | 11 ++++++++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/tests/test_questions.py b/src/tests/test_questions.py index fbbf757c9..afd73c861 100644 --- a/src/tests/test_questions.py +++ b/src/tests/test_questions.py @@ -16,6 +16,7 @@ from yunohost import app, domain, user from yunohost.utils.form import ( OPTIONS, FORBIDDEN_PASSWORD_CHARS, + READONLY_TYPES, ask_questions_and_parse_answers, BaseChoicesOption, BaseInputOption, @@ -444,6 +445,9 @@ class BaseTest: def _test_basic_attrs(self): raw_option = self.get_raw_option(optional=True) + if raw_option["type"] in READONLY_TYPES: + del raw_option["optional"] + if raw_option["type"] == "select": raw_option["choices"] = ["one"] diff --git a/src/utils/form.py b/src/utils/form.py index 07be55312..70afeb8f3 100644 --- a/src/utils/form.py +++ b/src/utils/form.py @@ -258,6 +258,12 @@ class OptionType(str, Enum): group = "group" +READONLY_TYPES = { + OptionType.display_text, + OptionType.markdown, + OptionType.alert, + OptionType.button, +} FORBIDDEN_READONLY_TYPES = { OptionType.password, OptionType.app, @@ -310,6 +316,7 @@ class BaseOption(BaseModel): arbitrary_types_allowed = True use_enum_values = True validate_assignment = True + extra = Extra.forbid @staticmethod def schema_extra(schema: dict[str, Any], model: Type["BaseOption"]) -> None: @@ -1314,9 +1321,11 @@ class OptionsModel(BaseModel): "type", OptionType.select if "choices" in data else OptionType.string, ), - "optional": data.get("optional", optional), } + if option["type"] not in READONLY_TYPES: + option["optional"] = option.get("optional", optional) + # LEGACY (`choices` in option `string` used to be valid) if "choices" in option and option["type"] == OptionType.string: logger.warning( From 3faa5742674074562f168183560a5daf5ebc0968 Mon Sep 17 00:00:00 2001 From: axolotle Date: Wed, 25 Oct 2023 15:07:31 +0200 Subject: [PATCH 49/50] configpanel: add proper schema definition --- doc/generate_json_schema.py | 4 ++++ src/utils/configpanel.py | 35 +++++++++++++++++++++++++++++++++++ src/utils/form.py | 12 ++++++++---- 3 files changed, 47 insertions(+), 4 deletions(-) create mode 100644 doc/generate_json_schema.py diff --git a/doc/generate_json_schema.py b/doc/generate_json_schema.py new file mode 100644 index 000000000..1abf88915 --- /dev/null +++ b/doc/generate_json_schema.py @@ -0,0 +1,4 @@ +from yunohost.utils.configpanel import ConfigPanelModel + + +print(ConfigPanelModel.schema_json(indent=2)) diff --git a/src/utils/configpanel.py b/src/utils/configpanel.py index 325f6579d..e3ceeff88 100644 --- a/src/utils/configpanel.py +++ b/src/utils/configpanel.py @@ -88,6 +88,14 @@ class SectionModel(ContainerModel, OptionsModel): optional: bool = True is_action_section: bool = False + class Config: + @staticmethod + def schema_extra(schema: dict[str, Any]) -> None: + del schema["properties"]["id"] + options = schema["properties"].pop("options") + del schema["required"] + schema["additionalProperties"] = options["items"] + # Don't forget to pass arguments to super init def __init__( self, @@ -137,6 +145,13 @@ class PanelModel(ContainerModel): class Config: extra = Extra.allow + @staticmethod + def schema_extra(schema: dict[str, Any]) -> None: + del schema["properties"]["id"] + del schema["properties"]["sections"] + del schema["required"] + schema["additionalProperties"] = {"$ref": "#/definitions/SectionModel"} + # Don't forget to pass arguments to super init def __init__( self, @@ -170,6 +185,26 @@ class ConfigPanelModel(BaseModel): arbitrary_types_allowed = True extra = Extra.allow + @staticmethod + def schema_extra(schema: dict[str, Any]) -> None: + """Update the schema to the expected input + In actual TOML definition, schema is like: + ```toml + [panel_1] + [panel_1.section_1] + [panel_1.section_1.option_1] + ``` + Which is equivalent to `{"panel_1": {"section_1": {"option_1": {}}}}` + so `section_id` (and `option_id`) are additional property of `panel_id`, + which is convinient to write but not ideal to iterate. + In ConfigPanelModel we gather additional properties of panels, sections + and options as lists so that structure looks like: + `{"panels`: [{"id": "panel_1", "sections": [{"id": "section_1", "options": [{"id": "option_1"}]}]}] + """ + del schema["properties"]["panels"] + del schema["required"] + schema["additionalProperties"] = {"$ref": "#/definitions/PanelModel"} + # Don't forget to pass arguments to super init def __init__( self, diff --git a/src/utils/form.py b/src/utils/form.py index 70afeb8f3..bd373badb 100644 --- a/src/utils/form.py +++ b/src/utils/form.py @@ -319,10 +319,14 @@ class BaseOption(BaseModel): extra = Extra.forbid @staticmethod - def schema_extra(schema: dict[str, Any], model: Type["BaseOption"]) -> None: - # FIXME Do proper doctstring for Options - del schema["description"] - schema["additionalProperties"] = False + def schema_extra(schema: dict[str, Any]) -> None: + del schema["properties"]["id"] + del schema["properties"]["name"] + schema["required"] = [ + required for required in schema.get("required", []) if required != "id" + ] + if not schema["required"]: + del schema["required"] @validator("id", pre=True) def check_id_is_not_forbidden(cls, value: str) -> str: From 9423168aaf836b376783692941e6e9bcf5b2f042 Mon Sep 17 00:00:00 2001 From: axolotle Date: Mon, 30 Oct 2023 15:17:01 +0100 Subject: [PATCH 50/50] configpanels: fix app `is_default` + dns alert style --- src/app.py | 6 ++++-- src/dns.py | 1 + 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/app.py b/src/app.py index 21219b2ce..52e1ebede 100644 --- a/src/app.py +++ b/src/app.py @@ -26,7 +26,6 @@ import re import subprocess import tempfile import copy -from collections import OrderedDict from typing import TYPE_CHECKING, List, Tuple, Dict, Any, Iterator, Optional, Union from packaging import version from logging import getLogger @@ -129,6 +128,7 @@ def app_info(app, full=False, upgradable=False): """ Get info for a specific app """ + from yunohost.domain import domain_config_get from yunohost.permission import user_permission_list _assert_is_installed(app) @@ -228,7 +228,9 @@ def app_info(app, full=False, upgradable=False): ret["is_webapp"] = "domain" in settings and "path" in settings if ret["is_webapp"]: - ret["is_default"] = settings.get("default_app", "_none") + ret["is_default"] = ( + domain_config_get(settings["domain"], "feature.app.default_app") == app + ) ret["supports_change_url"] = os.path.exists( os.path.join(setting_path, "scripts", "change_url") diff --git a/src/dns.py b/src/dns.py index fc4b26a75..126f81dbf 100644 --- a/src/dns.py +++ b/src/dns.py @@ -558,6 +558,7 @@ def _get_registrar_config_section(domain): except ValueError: registrar_infos["registrar"]["default"] = None registrar_infos["infos"]["ask"] = m18n.n("domain_dns_registrar_not_supported") + registrar_infos["infos"]["style"] = "warning" else: registrar_infos["registrar"]["default"] = registrar registrar_infos["infos"]["ask"] = m18n.n(