From 2d158b5a6d20275bee9d76f0a8611e942b235f35 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Tue, 21 Sep 2021 20:21:05 +0200 Subject: [PATCH 01/21] Rework prompt() again --- locales/en.json | 2 +- src/yunohost/user.py | 2 +- src/yunohost/utils/config.py | 76 ++++++++++++++++-------------------- 3 files changed, 35 insertions(+), 45 deletions(-) diff --git a/locales/en.json b/locales/en.json index 4601bd92f..447f137a5 100644 --- a/locales/en.json +++ b/locales/en.json @@ -16,7 +16,7 @@ "app_argument_choice_invalid": "Use one of these choices '{choices}' for the argument '{name}' instead of '{value}'", "app_argument_invalid": "Pick a valid value for the argument '{name}': {error}", "app_argument_password_help_keep": "Press Enter to keep the current value", - "app_argument_password_help_optional": "Type one space to empty the password", + "app_argument_password_help_optional": "Enter a single space to empty the password", "app_argument_password_no_default": "Error while parsing password argument '{name}': password argument can't have a default value for security reason", "app_argument_required": "Argument '{name}' is required", "app_change_url_identical_domains": "The old and new domain/url_path are identical ('{domain}{path}'), nothing to do.", diff --git a/src/yunohost/user.py b/src/yunohost/user.py index b32e03dfa..22168d3e7 100644 --- a/src/yunohost/user.py +++ b/src/yunohost/user.py @@ -420,7 +420,7 @@ def user_update( # without a specified value, change_password will be set to the const 0. # In this case we prompt for the new password. if Moulinette.interface.type == "cli" and not change_password: - change_password = Moulinette.prompt(m18n.n("ask_password"), True, True) + change_password = Moulinette.prompt(m18n.n("ask_password"), is_password=True, confirm=True) # Ensure sufficiently complex password assert_password_is_strong_enough("user", change_password) diff --git a/src/yunohost/utils/config.py b/src/yunohost/utils/config.py index 99c898d15..7b3d44b57 100644 --- a/src/yunohost/utils/config.py +++ b/src/yunohost/utils/config.py @@ -99,6 +99,9 @@ class ConfigPanel: result[key]["value"] = question_class.humanize( option["current_value"], option ) + # FIXME: semantics, technically here this is not about a prompt... + if question_class.hide_user_input_in_prompt: + result[key]["value"] = "**************" # Prevent displaying password in `config get` if mode == "full": return self.config @@ -480,6 +483,8 @@ class Question(object): @staticmethod def normalize(value, option={}): + if isinstance(value, str): + value = value.strip() return value def _prompt(self, text): @@ -491,9 +496,11 @@ class Question(object): self.value = Moulinette.prompt( message=text, is_password=self.hide_user_input_in_prompt, - confirm=False, # We doesn't want to confirm this kind of password like in webadmin + confirm=False, prefill=prefill, is_multiline=(self.type == "text"), + autocomplete=self.choices, + help=_value_for_locale(self.help) ) def ask_if_needed(self): @@ -558,18 +565,8 @@ class Question(object): choices=", ".join(self.choices), ) - def _format_text_for_user_input_in_cli(self, column=False): - text_for_user_input_in_cli = _value_for_locale(self.ask) - - if self.choices: - text_for_user_input_in_cli += " [{0}]".format(" | ".join(self.choices)) - - if self.help or column: - text_for_user_input_in_cli += ":\033[m" - if self.help: - text_for_user_input_in_cli += "\n - " - text_for_user_input_in_cli += _value_for_locale(self.help) - return text_for_user_input_in_cli + def _format_text_for_user_input_in_cli(self): + return _value_for_locale(self.ask) def _post_parse_value(self): if not self.redact: @@ -659,6 +656,8 @@ class TagsQuestion(Question): def normalize(value, option={}): if isinstance(value, list): return ",".join(value) + if isinstance(value, str): + value = value.strip() return value def _prevalidate(self): @@ -692,12 +691,6 @@ class PasswordQuestion(Question): "app_argument_password_no_default", name=self.name ) - @staticmethod - def humanize(value, option={}): - if value: - return "********" # Avoid to display the password on screen - return "" - def _prevalidate(self): super()._prevalidate() @@ -712,29 +705,6 @@ class PasswordQuestion(Question): assert_password_is_strong_enough("user", self.value) - def _format_text_for_user_input_in_cli(self): - need_column = self.current_value or self.optional - text_for_user_input_in_cli = super()._format_text_for_user_input_in_cli( - need_column - ) - if self.current_value: - text_for_user_input_in_cli += "\n - " + m18n.n( - "app_argument_password_help_keep" - ) - if self.optional: - text_for_user_input_in_cli += "\n - " + m18n.n( - "app_argument_password_help_optional" - ) - - return text_for_user_input_in_cli - - def _prompt(self, text): - super()._prompt(text) - if self.current_value and self.value == "": - self.value = self.current_value - elif self.value == " ": - self.value = "" - class PathQuestion(Question): argument_type = "path" @@ -769,14 +739,30 @@ class BooleanQuestion(Question): "app_argument_choice_invalid", name=option.get("name", ""), value=value, + # FIXME : this doesn't match yes_answers / no_answers... choices="yes, no, y, n, 1, 0", ) @staticmethod def normalize(value, option={}): + if isinstance(value, str): + value = value.strip() + yes = option.get("yes", 1) no = option.get("no", 0) + # + # FIXME: Shouldn't we also check that value == yes ? + # Otherwise is yes = "foobar", normalize is not idempotent + # i.e normalize("true") will return "foobar" + # but normalize(normalize("true")) will raise an exception ? + # + # FIXME: it's also a bit confusing to understand if the + # packager-provided 'yes' value is meant for humans (in which case + # shouldn't it be used as a return value for humanize?) + # or as an internal value for better interfacing with scripts/computers + # + # Also shouldnt we be using normalize() in humanize() ? if str(value).lower() in BooleanQuestion.yes_answers: return yes @@ -881,14 +867,18 @@ class NumberQuestion(Question): if isinstance(value, int): return value + if isinstance(value, str): + value = value.strip() + if isinstance(value, str) and value.isdigit(): return int(value) if value in [None, ""]: return value + # FIXME: option.name may not exist if option={}... raise YunohostValidationError( - "app_argument_invalid", name=option.name, error=m18n.n("invalid_number") + "app_argument_invalid", name=option.get("name", ""), error=m18n.n("invalid_number") ) def _prevalidate(self): From d1c371ec380f216b34afda2f054a4d70c1d0cb4f Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Thu, 23 Sep 2021 00:12:24 +0200 Subject: [PATCH 02/21] cli questions: Restore displaying available choices, though limit to the first 20 available options --- locales/en.json | 1 + src/yunohost/utils/config.py | 17 ++++++++++++++++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/locales/en.json b/locales/en.json index 447f137a5..4572e6f86 100644 --- a/locales/en.json +++ b/locales/en.json @@ -541,6 +541,7 @@ "migrations_to_be_ran_manually": "Migration {id} has to be run manually. Please go to Tools → Migrations on the webadmin page, or run `yunohost tools migrations run`.", "not_enough_disk_space": "Not enough free space on '{path}'", "operation_interrupted": "The operation was manually interrupted?", + "other_available_options": "... and {n} other available options not shown", "packages_upgrade_failed": "Could not upgrade all the packages", "password_listed": "This password is among the most used passwords in the world. Please choose something more unique.", "password_too_simple_1": "The password needs to be at least 8 characters long", diff --git a/src/yunohost/utils/config.py b/src/yunohost/utils/config.py index 7b3d44b57..2b83507f0 100644 --- a/src/yunohost/utils/config.py +++ b/src/yunohost/utils/config.py @@ -566,7 +566,22 @@ class Question(object): ) def _format_text_for_user_input_in_cli(self): - return _value_for_locale(self.ask) + + text_for_user_input_in_cli = _value_for_locale(self.ask) + + if self.choices: + + # Prevent displaying a shitload of choices + # (e.g. 100+ available users when choosing an app admin...) + choices_to_display = self.choices[:20] + remaining_choices = len(self.choices[20:]) + + if remaining_choices > 0: + choices_to_display += [m18n.n("other_available_options", n=remaining_choices)] + + text_for_user_input_in_cli += " [{0}]".format(" | ".join(choices_to_display)) + + return text_for_user_input_in_cli def _post_parse_value(self): if not self.redact: From 92f9e15e2fd788bf4fcb0409603e834a70d9a14d Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Thu, 23 Sep 2021 00:13:18 +0200 Subject: [PATCH 03/21] Stale i18n strings --- locales/en.json | 2 -- 1 file changed, 2 deletions(-) diff --git a/locales/en.json b/locales/en.json index 4572e6f86..3354dc19c 100644 --- a/locales/en.json +++ b/locales/en.json @@ -15,8 +15,6 @@ "app_already_up_to_date": "{app} is already up-to-date", "app_argument_choice_invalid": "Use one of these choices '{choices}' for the argument '{name}' instead of '{value}'", "app_argument_invalid": "Pick a valid value for the argument '{name}': {error}", - "app_argument_password_help_keep": "Press Enter to keep the current value", - "app_argument_password_help_optional": "Enter a single space to empty the password", "app_argument_password_no_default": "Error while parsing password argument '{name}': password argument can't have a default value for security reason", "app_argument_required": "Argument '{name}' is required", "app_change_url_identical_domains": "The old and new domain/url_path are identical ('{domain}{path}'), nothing to do.", From 8ea160b9fe0a17df7f8bc69931db9e1690cf71da Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Thu, 23 Sep 2021 00:29:22 +0200 Subject: [PATCH 04/21] Fix tests --- src/yunohost/tests/test_questions.py | 18 ++++++++++++++++++ src/yunohost/utils/config.py | 27 ++++++++++++++------------- 2 files changed, 32 insertions(+), 13 deletions(-) diff --git a/src/yunohost/tests/test_questions.py b/src/yunohost/tests/test_questions.py index 9753b08e4..62bb61cc2 100644 --- a/src/yunohost/tests/test_questions.py +++ b/src/yunohost/tests/test_questions.py @@ -207,6 +207,8 @@ def test_question_string_input_test_ask(): confirm=False, prefill="", is_multiline=False, + autocomplete=[], + help=None, ) @@ -232,6 +234,8 @@ def test_question_string_input_test_ask_with_default(): confirm=False, prefill=default_text, is_multiline=False, + autocomplete=[], + help=None, ) @@ -526,6 +530,8 @@ def test_question_password_input_test_ask(): confirm=False, prefill="", is_multiline=False, + autocomplete=[], + help=None, ) @@ -787,6 +793,8 @@ def test_question_path_input_test_ask(): confirm=False, prefill="", is_multiline=False, + autocomplete=[], + help=None, ) @@ -813,6 +821,8 @@ def test_question_path_input_test_ask_with_default(): confirm=False, prefill=default_text, is_multiline=False, + autocomplete=[], + help=None, ) @@ -1162,6 +1172,8 @@ def test_question_boolean_input_test_ask(): confirm=False, prefill="no", is_multiline=False, + autocomplete=[], + help=None, ) @@ -1188,6 +1200,8 @@ def test_question_boolean_input_test_ask_with_default(): confirm=False, prefill="yes", is_multiline=False, + autocomplete=[], + help=None, ) @@ -1735,6 +1749,8 @@ def test_question_number_input_test_ask(): confirm=False, prefill="", is_multiline=False, + autocomplete=[], + help=None, ) @@ -1761,6 +1777,8 @@ def test_question_number_input_test_ask_with_default(): confirm=False, prefill=str(default_value), is_multiline=False, + autocomplete=[], + help=None, ) diff --git a/src/yunohost/utils/config.py b/src/yunohost/utils/config.py index 2b83507f0..d6ddb81f9 100644 --- a/src/yunohost/utils/config.py +++ b/src/yunohost/utils/config.py @@ -573,13 +573,16 @@ class Question(object): # Prevent displaying a shitload of choices # (e.g. 100+ available users when choosing an app admin...) - choices_to_display = self.choices[:20] - remaining_choices = len(self.choices[20:]) + choices = list(self.choices.values()) if isinstance(self.choices, dict) else self.choices + choices_to_display = choices[:20] + remaining_choices = len(choices[20:]) if remaining_choices > 0: choices_to_display += [m18n.n("other_available_options", n=remaining_choices)] - text_for_user_input_in_cli += " [{0}]".format(" | ".join(choices_to_display)) + choices_to_display = " | ".join(choices_to_display) + + text_for_user_input_in_cli += f" [{choices_to_display}]" return text_for_user_input_in_cli @@ -752,7 +755,7 @@ class BooleanQuestion(Question): raise YunohostValidationError( "app_argument_choice_invalid", - name=option.get("name", ""), + name=getattr(option, "name", None) or option.get("name"), value=value, # FIXME : this doesn't match yes_answers / no_answers... choices="yes, no, y, n, 1, 0", @@ -788,7 +791,7 @@ class BooleanQuestion(Question): return None raise YunohostValidationError( "app_argument_choice_invalid", - name=option.get("name", ""), + name=getattr(option, "name", None) or option.get("name"), value=value, choices="yes, no, y, n, 1, 0", ) @@ -808,10 +811,7 @@ class BooleanQuestion(Question): return text_for_user_input_in_cli def get(self, key, default=None): - try: - return getattr(self, key) - except AttributeError: - return default + return getattr(self, key, default) class DomainQuestion(Question): @@ -843,7 +843,7 @@ class UserQuestion(Question): from yunohost.domain import _get_maindomain super().__init__(question, user_answers) - self.choices = user_list()["users"] + self.choices = list(user_list()["users"].keys()) if not self.choices: raise YunohostValidationError( @@ -854,7 +854,7 @@ class UserQuestion(Question): if self.default is None: root_mail = "root@%s" % _get_maindomain() - for user in self.choices.keys(): + for user in self.choices: if root_mail in user_info(user).get("mail-aliases", []): self.default = user break @@ -891,9 +891,10 @@ class NumberQuestion(Question): if value in [None, ""]: return value - # FIXME: option.name may not exist if option={}... raise YunohostValidationError( - "app_argument_invalid", name=option.get("name", ""), error=m18n.n("invalid_number") + "app_argument_invalid", + name=getattr(option, "name", None) or option.get("name"), + error=m18n.n("invalid_number") ) def _prevalidate(self): From 78ad3b5da32b509cb91df1106c49e2eabb880d96 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Thu, 23 Sep 2021 02:39:39 +0200 Subject: [PATCH 05/21] Fix weird definition for boolean's humanize/normalize --- src/yunohost/utils/config.py | 59 +++++++++++++++++++----------------- 1 file changed, 31 insertions(+), 28 deletions(-) diff --git a/src/yunohost/utils/config.py b/src/yunohost/utils/config.py index d6ddb81f9..64b472c39 100644 --- a/src/yunohost/utils/config.py +++ b/src/yunohost/utils/config.py @@ -740,25 +740,21 @@ class BooleanQuestion(Question): yes = option.get("yes", 1) no = option.get("no", 0) - value = str(value).lower() - if value == str(yes).lower(): - return "yes" - if value == str(no).lower(): - return "no" - if value in BooleanQuestion.yes_answers: - return "yes" - if value in BooleanQuestion.no_answers: - return "no" - if value in ["none", ""]: + value = BooleanQuestion.normalize(value, option) + + if value == yes: + return "yes" + if value == no: + return "no" + if value is None: return "" raise YunohostValidationError( "app_argument_choice_invalid", name=getattr(option, "name", None) or option.get("name"), value=value, - # FIXME : this doesn't match yes_answers / no_answers... - choices="yes, no, y, n, 1, 0", + choices="yes/no", ) @staticmethod @@ -769,31 +765,38 @@ class BooleanQuestion(Question): yes = option.get("yes", 1) no = option.get("no", 0) - # - # FIXME: Shouldn't we also check that value == yes ? - # Otherwise is yes = "foobar", normalize is not idempotent - # i.e normalize("true") will return "foobar" - # but normalize(normalize("true")) will raise an exception ? - # - # FIXME: it's also a bit confusing to understand if the - # packager-provided 'yes' value is meant for humans (in which case - # shouldn't it be used as a return value for humanize?) - # or as an internal value for better interfacing with scripts/computers - # - # Also shouldnt we be using normalize() in humanize() ? - if str(value).lower() in BooleanQuestion.yes_answers: - return yes + strvalue = str(value).lower() - if str(value).lower() in BooleanQuestion.no_answers: + # + # N.B.: + # we check first if the value is equal to yes + # then if equal to no + # then if in yes_answers + # then if in no_answers + # + # This is to hopefully cover the weird edgecase + # where the value for yes/no could be reversed + # such as yes=false or yes=0 + # no=true no=1 + # + + if strvalue == str(yes).lower(): + return yes + if strvalue == str(no).lower(): + return no + if strvalue in BooleanQuestion.yes_answers: + return yes + if strvalue in BooleanQuestion.no_answers: return no if value in [None, ""]: return None + raise YunohostValidationError( "app_argument_choice_invalid", name=getattr(option, "name", None) or option.get("name"), value=value, - choices="yes, no, y, n, 1, 0", + choices="yes/no", ) def __init__(self, question, user_answers): From d023333fa42128d70a87eb49b0f34f55aa8d1540 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Thu, 23 Sep 2021 02:58:13 +0200 Subject: [PATCH 06/21] questions prompt: include normalize in the try/except to re-ask question if not properly formated --- src/yunohost/utils/config.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/yunohost/utils/config.py b/src/yunohost/utils/config.py index 64b472c39..38cc28894 100644 --- a/src/yunohost/utils/config.py +++ b/src/yunohost/utils/config.py @@ -520,12 +520,9 @@ class Question(object): ): self.value = class_default if self.default is None else self.default - # Normalization - # This is done to enforce a certain formating like for boolean - self.value = self.normalize(self.value, self) - - # Prevalidation try: + # Normalize and validate + self.value = self.normalize(self.value, self) self._prevalidate() except YunohostValidationError as e: # If in interactive cli, re-ask the current question From 430f53aa73fc68c0b286af11a2104bea20daf240 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Thu, 23 Sep 2021 03:39:43 +0200 Subject: [PATCH 07/21] Semantic, simplify code... --- src/yunohost/app.py | 83 +++++------------------------------- src/yunohost/utils/config.py | 25 ++++++----- 2 files changed, 26 insertions(+), 82 deletions(-) diff --git a/src/yunohost/app.py b/src/yunohost/app.py index 91f7a41ef..b0ae7f0d5 100644 --- a/src/yunohost/app.py +++ b/src/yunohost/app.py @@ -32,7 +32,6 @@ import time import re import subprocess import glob -import urllib.parse import tempfile from collections import OrderedDict @@ -55,7 +54,7 @@ from moulinette.utils.filesystem import ( from yunohost.utils import packages from yunohost.utils.config import ( ConfigPanel, - parse_args_in_yunohost_format, + ask_questions_and_parse_answers, ) from yunohost.utils.i18n import _value_for_locale from yunohost.utils.error import YunohostError, YunohostValidationError @@ -453,16 +452,10 @@ def app_change_url(operation_logger, app, domain, path): # Check the url is available _assert_no_conflicting_apps(domain, path, ignore_app=app) - manifest = _get_manifest_of_app(os.path.join(APPS_SETTING_PATH, app)) - - # Retrieve arguments list for change_url script - # TODO: Allow to specify arguments - args_odict = _parse_args_from_manifest(manifest, "change_url") - tmp_workdir_for_app = _make_tmp_workdir_for_app(app=app) # Prepare env. var. to pass to script - env_dict = _make_environment_for_app_script(app, args=args_odict) + env_dict = _make_environment_for_app_script(app) env_dict["YNH_APP_OLD_DOMAIN"] = old_domain env_dict["YNH_APP_OLD_PATH"] = old_path env_dict["YNH_APP_NEW_DOMAIN"] = domain @@ -614,12 +607,8 @@ def app_upgrade(app=[], url=None, file=None, force=False, no_safety_backup=False app_setting_path = os.path.join(APPS_SETTING_PATH, app_instance_name) - # Retrieve arguments list for upgrade script - # TODO: Allow to specify arguments - args_odict = _parse_args_from_manifest(manifest, "upgrade") - # Prepare env. var. to pass to script - env_dict = _make_environment_for_app_script(app_instance_name, args=args_odict) + env_dict = _make_environment_for_app_script(app_instance_name) env_dict["YNH_APP_UPGRADE_TYPE"] = upgrade_type env_dict["YNH_APP_MANIFEST_VERSION"] = str(app_new_version) env_dict["YNH_APP_CURRENT_VERSION"] = str(app_current_version) @@ -905,13 +894,11 @@ def app_install( app_instance_name = app_id # Retrieve arguments list for install script - args_dict = ( - {} if not args else dict(urllib.parse.parse_qsl(args, keep_blank_values=True)) - ) - args_odict = _parse_args_from_manifest(manifest, "install", args=args_dict) + questions = manifest.get("arguments", {}).get("install", {}) + args = ask_questions_and_parse_answers(questions, prefilled_answers=args) # Validate domain / path availability for webapps - _validate_and_normalize_webpath(args_odict, extracted_app_folder) + _validate_and_normalize_webpath(args, extracted_app_folder) # Attempt to patch legacy helpers ... _patch_legacy_helpers(extracted_app_folder) @@ -976,11 +963,11 @@ def app_install( ) # Prepare env. var. to pass to script - env_dict = _make_environment_for_app_script(app_instance_name, args=args_odict) + env_dict = _make_environment_for_app_script(app_instance_name, args=args) env_dict["YNH_APP_BASEDIR"] = extracted_app_folder env_dict_for_logging = env_dict.copy() - for arg_name, arg_value_and_type in args_odict.items(): + for arg_name, arg_value_and_type in args.items(): if arg_value_and_type[1] == "password": del env_dict_for_logging["YNH_APP_ARG_%s" % arg_name.upper()] @@ -1642,15 +1629,13 @@ def app_action_run(operation_logger, app, action, args=None): action_declaration = actions[action] # Retrieve arguments list for install script - args_dict = ( - dict(urllib.parse.parse_qsl(args, keep_blank_values=True)) if args else {} - ) - args_odict = _parse_args_for_action(actions[action], args=args_dict) + questions = actions[action].get("arguments", {}) + args = ask_questions_and_parse_answers(questions, prefilled_answers=args) tmp_workdir_for_app = _make_tmp_workdir_for_app(app=app) env_dict = _make_environment_for_app_script( - app, args=args_odict, args_prefix="ACTION_" + app, args=args, args_prefix="ACTION_" ) env_dict["YNH_ACTION"] = action env_dict["YNH_APP_BASEDIR"] = tmp_workdir_for_app @@ -2380,52 +2365,6 @@ def _check_manifest_requirements(manifest, app_instance_name): ) -def _parse_args_from_manifest(manifest, action, args={}): - """Parse arguments needed for an action from the manifest - - Retrieve specified arguments for the action from the manifest, and parse - given args according to that. If some required arguments are not provided, - its values will be asked if interaction is possible. - Parsed arguments will be returned as an OrderedDict - - Keyword arguments: - manifest -- The app manifest to use - action -- The action to retrieve arguments for - args -- A dictionnary of arguments to parse - - """ - if action not in manifest["arguments"]: - logger.debug("no arguments found for '%s' in manifest", action) - return OrderedDict() - - action_args = manifest["arguments"][action] - return parse_args_in_yunohost_format(args, action_args) - - -def _parse_args_for_action(action, args={}): - """Parse arguments needed for an action from the actions list - - Retrieve specified arguments for the action from the manifest, and parse - given args according to that. If some required arguments are not provided, - its values will be asked if interaction is possible. - Parsed arguments will be returned as an OrderedDict - - Keyword arguments: - action -- The action - args -- A dictionnary of arguments to parse - - """ - args_dict = OrderedDict() - - if "arguments" not in action: - logger.debug("no arguments found for '%s' in manifest", action) - return args_dict - - action_args = action["arguments"] - - return parse_args_in_yunohost_format(args, action_args) - - def _validate_and_normalize_webpath(args_dict, app_folder): # If there's only one "domain" and "path", validate that domain/path diff --git a/src/yunohost/utils/config.py b/src/yunohost/utils/config.py index 38cc28894..3431e9bdc 100644 --- a/src/yunohost/utils/config.py +++ b/src/yunohost/utils/config.py @@ -464,14 +464,16 @@ class Question(object): self.name = question["name"] self.type = question.get("type", "string") self.default = question.get("default", None) - self.current_value = question.get("current_value") self.optional = question.get("optional", False) self.choices = question.get("choices", []) self.pattern = question.get("pattern", self.pattern) self.ask = question.get("ask", {"en": self.name}) self.help = question.get("help") - self.value = user_answers.get(self.name) self.redact = question.get("redact", False) + # .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 = user_answers.get(self.name) # Empty value is parsed as empty string if self.default == "": @@ -1063,25 +1065,28 @@ ARGUMENTS_TYPE_PARSERS = { } -def parse_args_in_yunohost_format(user_answers, argument_questions): +def ask_questions_and_parse_answers(questions, prefilled_answers=""): + _setuser_answers, argument_questions): """Parse arguments store in either manifest.json or actions.json or from a config panel against the user answers when they are present. Keyword arguments: - user_answers -- a dictionnary of arguments from the user (generally - empty in CLI, filed from the admin interface) + prefilled_answers -- a dictionnary of arguments from the user (generally + empty in CLI, filed from the admin interface) argument_questions -- the arguments description store in yunohost format from actions.json/toml, manifest.json/toml or config_panel.json/toml """ - parsed_answers_dict = OrderedDict() - for question in argument_questions: + prefilled_answers = dict(urllib.parse.parse_qsl(prefilled_answers or "", keep_blank_values=True)) + out = OrderedDict() + + for question in questions: question_class = ARGUMENTS_TYPE_PARSERS[question.get("type", "string")] - question = question_class(question, user_answers) + question = question_class(question, prefilled_answers) answer = question.ask_if_needed() if answer is not None: - parsed_answers_dict[question.name] = answer + out[question.name] = answer - return parsed_answers_dict + return out From 9c3ff52d98e1e2c717e10ba470c8c99c5eefc53e Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Thu, 23 Sep 2021 04:35:22 +0200 Subject: [PATCH 08/21] file questions: we don't need to know the filename, we don't need to validate extension server-side --- src/yunohost/utils/config.py | 70 ++++++++---------------------------- 1 file changed, 15 insertions(+), 55 deletions(-) diff --git a/src/yunohost/utils/config.py b/src/yunohost/utils/config.py index 3431e9bdc..4c9457c94 100644 --- a/src/yunohost/utils/config.py +++ b/src/yunohost/utils/config.py @@ -961,83 +961,44 @@ class FileQuestion(Question): def __init__(self, question, user_answers): super().__init__(question, user_answers) - if question.get("accept"): - self.accept = question.get("accept") - else: - self.accept = "" - if Moulinette.interface.type == "api": - if user_answers.get(f"{self.name}[name]"): - self.value = { - "content": self.value, - "filename": user_answers.get(f"{self.name}[name]", self.name), - } + self.accept = question.get("accept", "") def _prevalidate(self): if self.value is None: self.value = self.current_value super()._prevalidate() - if ( - isinstance(self.value, str) - and self.value - and not os.path.exists(self.value) - ): - raise YunohostValidationError( - "app_argument_invalid", - name=self.name, - error=m18n.n("file_does_not_exist", path=self.value), - ) - if self.value in [None, ""] or not self.accept: - return - filename = self.value if isinstance(self.value, str) else self.value["filename"] - if "." not in filename or "." + filename.split(".")[ - -1 - ] not in self.accept.replace(" ", "").split(","): + if not self.value or not os.path.exists(str(self.value)): raise YunohostValidationError( "app_argument_invalid", name=self.name, - error=m18n.n( - "file_extension_not_accepted", file=filename, accept=self.accept - ), + error=m18n.n("file_does_not_exist", path=str(self.value)), ) def _post_parse_value(self): from base64 import b64decode - # Upload files from API - # A file arg contains a string with "FILENAME:BASE64_CONTENT" if not self.value: return self.value - if Moulinette.interface.type == "api" and isinstance(self.value, dict): + # FIXME : in the cli case, don't we want to also copy the file + # to a tmp work dir ? + + if Moulinette.interface.type == "api": upload_dir = tempfile.mkdtemp(prefix="tmp_configpanel_") + _, file_path = tempfile.mkstemp(prefix="foobar", dir=upload_dir) + FileQuestion.upload_dirs += [upload_dir] - filename = self.value["filename"] - logger.debug( - f"Save uploaded file {self.value['filename']} from API into {upload_dir}" - ) + logger.debug(f"Save uploaded file from API into {file_path}") - # Filename is given by user of the API. For security reason, we have replaced - # os.path.join to avoid the user to be able to rewrite a file in filesystem - # i.e. os.path.join("/foo", "/etc/passwd") == "/etc/passwd" - file_path = os.path.normpath(upload_dir + "/" + filename) - if not file_path.startswith(upload_dir + "/"): - raise YunohostError( - f"Filename '{filename}' received from the API got a relative parent path, which is forbidden", - raw_msg=True, - ) - i = 2 - while os.path.exists(file_path): - file_path = os.path.normpath(upload_dir + "/" + filename + (".%d" % i)) - i += 1 - - content = self.value["content"] + content = self.value write_to_file(file_path, b64decode(content), file_mode="wb") self.value = file_path + return self.value @@ -1066,19 +1027,18 @@ ARGUMENTS_TYPE_PARSERS = { def ask_questions_and_parse_answers(questions, prefilled_answers=""): - _setuser_answers, argument_questions): """Parse arguments store in either manifest.json or actions.json or from a config panel against the user answers when they are present. Keyword arguments: - prefilled_answers -- a dictionnary of arguments from the user (generally - empty in CLI, filed from the admin interface) - argument_questions -- the arguments description store in yunohost + questions -- the arguments description store in yunohost format from actions.json/toml, manifest.json/toml or config_panel.json/toml + prefilled_answers -- a url-formated string such as "domain=yolo.test&path=/foobar&admin=sam" """ prefilled_answers = dict(urllib.parse.parse_qsl(prefilled_answers or "", keep_blank_values=True)) + out = OrderedDict() for question in questions: From d5748b519ac8237b2364642ee41411d8dcab5914 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Thu, 23 Sep 2021 05:02:27 +0200 Subject: [PATCH 09/21] config panels: attempt to improve the semantic of 'convert()' --- src/yunohost/utils/config.py | 51 ++++++++++++++++++------------------ 1 file changed, 26 insertions(+), 25 deletions(-) diff --git a/src/yunohost/utils/config.py b/src/yunohost/utils/config.py index 4c9457c94..558498869 100644 --- a/src/yunohost/utils/config.py +++ b/src/yunohost/utils/config.py @@ -201,20 +201,20 @@ class ConfigPanel: # Transform toml format into internal format format_description = { - "toml": { + "root": { "properties": ["version", "i18n"], - "default": {"version": 1.0}, + "defaults": {"version": 1.0}, }, "panels": { "properties": ["name", "services", "actions", "help"], - "default": { + "defaults": { "services": [], "actions": {"apply": {"en": "Apply"}}, }, }, "sections": { "properties": ["name", "services", "optional", "help", "visible"], - "default": { + "defaults": { "name": "", "services": [], "optional": True, @@ -244,11 +244,11 @@ class ConfigPanel: "accept", "redact", ], - "default": {}, + "defaults": {}, }, } - def convert(toml_node, node_type): + 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, @@ -256,48 +256,49 @@ class ConfigPanel: - some properties have default values This function detects all children nodes and put them in a list """ - # Prefill the node default keys if needed - default = format_description[node_type]["default"] - node = {key: toml_node.get(key, value) for key, value in default.items()} - properties = format_description[node_type]["properties"] + defaults = format_description[level]["defaults"] + properties = format_description[level]["properties"] - # Define the filter_key part to use and the children type - i = list(format_description).index(node_type) - subnode_type = ( - list(format_description)[i + 1] if node_type != "options" else None + # 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 toml_node.items(): + for key, value in raw_infos.items(): # Key/value are a child node if ( isinstance(value, OrderedDict) and key not in properties - and subnode_type + and sublevel ): # We exclude all nodes not referenced by the filter_key if search_key and key != search_key: continue - subnode = convert(value, subnode_type) + subnode = _build_internal_config_panel(value, sublevel) subnode["id"] = key - if node_type == "toml": + if level == "root": subnode.setdefault("name", {"en": key.capitalize()}) - elif node_type == "sections": + elif level == "sections": subnode["name"] = key # legacy - subnode.setdefault("optional", toml_node.get("optional", True)) - node.setdefault(subnode_type, []).append(subnode) + subnode.setdefault("optional", raw_infos.get("optional", 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 toml") + logger.warning(f"Unknown key '{key}' found in config panel") # Todo search all i18n keys - node[key] = ( + out[key] = ( value if key not in ["ask", "help", "name"] else {"en": value} ) - return node + return out - self.config = convert(toml_config_panel, "toml") + self.config = _build_internal_config_panel(toml_config_panel, "root") try: self.config["panels"][0]["sections"][0]["options"][0] From f14d7805884f6ba27f400b997d0bdf9b6e6a50b9 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Thu, 23 Sep 2021 05:09:57 +0200 Subject: [PATCH 10/21] Get rid of unecessary 'user_answers' arg in constructor --- src/yunohost/utils/config.py | 35 ++++++++++++++++++----------------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/src/yunohost/utils/config.py b/src/yunohost/utils/config.py index 558498869..81b651be5 100644 --- a/src/yunohost/utils/config.py +++ b/src/yunohost/utils/config.py @@ -461,7 +461,7 @@ class Question(object): hide_user_input_in_prompt = False pattern: Optional[Dict] = None - def __init__(self, question, user_answers): + def __init__(self, question): self.name = question["name"] self.type = question.get("type", "string") self.default = question.get("default", None) @@ -474,7 +474,7 @@ class Question(object): # .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 = user_answers.get(self.name) + self.value = question.get("value") # Empty value is parsed as empty string if self.default == "": @@ -701,8 +701,8 @@ class PasswordQuestion(Question): default_value = "" forbidden_chars = "{}" - def __init__(self, question, user_answers): - super().__init__(question, user_answers) + def __init__(self, question): + super().__init__(question) self.redact = True if self.default is not None: raise YunohostValidationError( @@ -799,8 +799,8 @@ class BooleanQuestion(Question): choices="yes/no", ) - def __init__(self, question, user_answers): - super().__init__(question, user_answers) + def __init__(self, question): + super().__init__(question) self.yes = question.get("yes", 1) self.no = question.get("no", 0) if self.default is None: @@ -820,10 +820,10 @@ class BooleanQuestion(Question): class DomainQuestion(Question): argument_type = "domain" - def __init__(self, question, user_answers): + def __init__(self, question): from yunohost.domain import domain_list, _get_maindomain - super().__init__(question, user_answers) + super().__init__(question) if self.default is None: self.default = _get_maindomain() @@ -841,11 +841,11 @@ class DomainQuestion(Question): class UserQuestion(Question): argument_type = "user" - def __init__(self, question, user_answers): + def __init__(self, question): from yunohost.user import user_list, user_info from yunohost.domain import _get_maindomain - super().__init__(question, user_answers) + super().__init__(question) self.choices = list(user_list()["users"].keys()) if not self.choices: @@ -874,8 +874,8 @@ class NumberQuestion(Question): argument_type = "number" default_value = None - def __init__(self, question, user_answers): - super().__init__(question, user_answers) + 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) @@ -924,8 +924,8 @@ class DisplayTextQuestion(Question): argument_type = "display_text" readonly = True - def __init__(self, question, user_answers): - super().__init__(question, user_answers) + def __init__(self, question): + super().__init__(question) self.optional = True self.style = question.get( @@ -960,8 +960,8 @@ class FileQuestion(Question): if os.path.exists(upload_dir): shutil.rmtree(upload_dir) - def __init__(self, question, user_answers): - super().__init__(question, user_answers) + def __init__(self, question): + super().__init__(question) self.accept = question.get("accept", "") def _prevalidate(self): @@ -1044,7 +1044,8 @@ def ask_questions_and_parse_answers(questions, prefilled_answers=""): for question in questions: question_class = ARGUMENTS_TYPE_PARSERS[question.get("type", "string")] - question = question_class(question, prefilled_answers) + question["value"] = prefilled_answers.get(question["name"]) + question = question_class(question) answer = question.ask_if_needed() if answer is not None: From 4cc2c9787db31c72b176b13297804a73c32e5e9e Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Thu, 23 Sep 2021 05:20:19 +0200 Subject: [PATCH 11/21] Propagate changes on tests --- src/yunohost/tests/test_questions.py | 251 +++++++++++++-------------- src/yunohost/utils/config.py | 2 +- 2 files changed, 126 insertions(+), 127 deletions(-) diff --git a/src/yunohost/tests/test_questions.py b/src/yunohost/tests/test_questions.py index 62bb61cc2..a01ad71f0 100644 --- a/src/yunohost/tests/test_questions.py +++ b/src/yunohost/tests/test_questions.py @@ -2,7 +2,7 @@ import sys import pytest import os -from mock import patch, MagicMock +from mock import patch from io import StringIO from collections import OrderedDict @@ -10,9 +10,8 @@ from moulinette import Moulinette from yunohost import domain, user from yunohost.utils.config import ( - parse_args_in_yunohost_format, + ask_questions_and_parse_answers, PasswordQuestion, - Question, ) from yunohost.utils.error import YunohostError @@ -41,7 +40,7 @@ User answers: def test_question_empty(): - assert parse_args_in_yunohost_format({}, []) == {} + assert ask_questions_and_parse_answers([], {}) == {} def test_question_string(): @@ -53,7 +52,7 @@ def test_question_string(): ] answers = {"some_string": "some_value"} expected_result = OrderedDict({"some_string": ("some_value", "string")}) - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result def test_question_string_default_type(): @@ -64,7 +63,7 @@ def test_question_string_default_type(): ] answers = {"some_string": "some_value"} expected_result = OrderedDict({"some_string": ("some_value", "string")}) - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result def test_question_string_no_input(): @@ -76,7 +75,7 @@ def test_question_string_no_input(): answers = {} with pytest.raises(YunohostError), patch.object(os, "isatty", return_value=False): - parse_args_in_yunohost_format(answers, questions) + ask_questions_and_parse_answers(questions, answers) def test_question_string_input(): @@ -92,7 +91,7 @@ def test_question_string_input(): with patch.object(Moulinette, "prompt", return_value="some_value"), patch.object( os, "isatty", return_value=True ): - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result def test_question_string_input_no_ask(): @@ -107,7 +106,7 @@ def test_question_string_input_no_ask(): with patch.object(Moulinette, "prompt", return_value="some_value"), patch.object( os, "isatty", return_value=True ): - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result def test_question_string_no_input_optional(): @@ -120,7 +119,7 @@ def test_question_string_no_input_optional(): answers = {} expected_result = OrderedDict({"some_string": ("", "string")}) with patch.object(os, "isatty", return_value=False): - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result def test_question_string_optional_with_input(): @@ -137,7 +136,7 @@ def test_question_string_optional_with_input(): with patch.object(Moulinette, "prompt", return_value="some_value"), patch.object( os, "isatty", return_value=True ): - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result def test_question_string_optional_with_empty_input(): @@ -154,7 +153,7 @@ def test_question_string_optional_with_empty_input(): with patch.object(Moulinette, "prompt", return_value=""), patch.object( os, "isatty", return_value=True ): - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result def test_question_string_optional_with_input_without_ask(): @@ -170,7 +169,7 @@ def test_question_string_optional_with_input_without_ask(): with patch.object(Moulinette, "prompt", return_value="some_value"), patch.object( os, "isatty", return_value=True ): - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result def test_question_string_no_input_default(): @@ -184,7 +183,7 @@ def test_question_string_no_input_default(): answers = {} expected_result = OrderedDict({"some_string": ("some_value", "string")}) with patch.object(os, "isatty", return_value=False): - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result def test_question_string_input_test_ask(): @@ -200,7 +199,7 @@ def test_question_string_input_test_ask(): with patch.object( Moulinette, "prompt", return_value="some_value" ) as prompt, patch.object(os, "isatty", return_value=True): - parse_args_in_yunohost_format(answers, questions) + ask_questions_and_parse_answers(questions, answers) prompt.assert_called_with( message=ask_text, is_password=False, @@ -227,7 +226,7 @@ def test_question_string_input_test_ask_with_default(): with patch.object( Moulinette, "prompt", return_value="some_value" ) as prompt, patch.object(os, "isatty", return_value=True): - parse_args_in_yunohost_format(answers, questions) + ask_questions_and_parse_answers(questions, answers) prompt.assert_called_with( message=ask_text, is_password=False, @@ -255,7 +254,7 @@ def test_question_string_input_test_ask_with_example(): with patch.object( Moulinette, "prompt", return_value="some_value" ) as prompt, patch.object(os, "isatty", return_value=True): - parse_args_in_yunohost_format(answers, questions) + ask_questions_and_parse_answers(questions, answers) assert ask_text in prompt.call_args[1]["message"] assert example_text in prompt.call_args[1]["message"] @@ -276,7 +275,7 @@ def test_question_string_input_test_ask_with_help(): with patch.object( Moulinette, "prompt", return_value="some_value" ) as prompt, patch.object(os, "isatty", return_value=True): - parse_args_in_yunohost_format(answers, questions) + ask_questions_and_parse_answers(questions, answers) assert ask_text in prompt.call_args[1]["message"] assert help_text in prompt.call_args[1]["message"] @@ -285,7 +284,7 @@ def test_question_string_with_choice(): questions = [{"name": "some_string", "type": "string", "choices": ["fr", "en"]}] answers = {"some_string": "fr"} expected_result = OrderedDict({"some_string": ("fr", "string")}) - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result def test_question_string_with_choice_prompt(): @@ -295,7 +294,7 @@ def test_question_string_with_choice_prompt(): with patch.object(Moulinette, "prompt", return_value="fr"), patch.object( os, "isatty", return_value=True ): - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result def test_question_string_with_choice_bad(): @@ -303,7 +302,7 @@ def test_question_string_with_choice_bad(): answers = {"some_string": "bad"} with pytest.raises(YunohostError), patch.object(os, "isatty", return_value=False): - assert parse_args_in_yunohost_format(answers, questions) + assert ask_questions_and_parse_answers(questions, answers) def test_question_string_with_choice_ask(): @@ -321,7 +320,7 @@ def test_question_string_with_choice_ask(): with patch.object(Moulinette, "prompt", return_value="ru") as prompt, patch.object( os, "isatty", return_value=True ): - parse_args_in_yunohost_format(answers, questions) + ask_questions_and_parse_answers(questions, answers) assert ask_text in prompt.call_args[1]["message"] for choice in choices: @@ -340,7 +339,7 @@ def test_question_string_with_choice_default(): answers = {} expected_result = OrderedDict({"some_string": ("en", "string")}) with patch.object(os, "isatty", return_value=False): - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result def test_question_password(): @@ -352,7 +351,7 @@ def test_question_password(): ] answers = {"some_password": "some_value"} expected_result = OrderedDict({"some_password": ("some_value", "password")}) - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result def test_question_password_no_input(): @@ -365,7 +364,7 @@ def test_question_password_no_input(): answers = {} with pytest.raises(YunohostError), patch.object(os, "isatty", return_value=False): - parse_args_in_yunohost_format(answers, questions) + ask_questions_and_parse_answers(questions, answers) def test_question_password_input(): @@ -382,7 +381,7 @@ def test_question_password_input(): with patch.object(Moulinette, "prompt", return_value="some_value"), patch.object( os, "isatty", return_value=True ): - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result def test_question_password_input_no_ask(): @@ -398,7 +397,7 @@ def test_question_password_input_no_ask(): with patch.object(Moulinette, "prompt", return_value="some_value"), patch.object( os, "isatty", return_value=True ): - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result def test_question_password_no_input_optional(): @@ -413,14 +412,14 @@ def test_question_password_no_input_optional(): expected_result = OrderedDict({"some_password": ("", "password")}) with patch.object(os, "isatty", return_value=False): - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result questions = [ {"name": "some_password", "type": "password", "optional": True, "default": ""} ] with patch.object(os, "isatty", return_value=False): - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result def test_question_password_optional_with_input(): @@ -438,7 +437,7 @@ def test_question_password_optional_with_input(): with patch.object(Moulinette, "prompt", return_value="some_value"), patch.object( os, "isatty", return_value=True ): - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result def test_question_password_optional_with_empty_input(): @@ -456,7 +455,7 @@ def test_question_password_optional_with_empty_input(): with patch.object(Moulinette, "prompt", return_value=""), patch.object( os, "isatty", return_value=True ): - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result def test_question_password_optional_with_input_without_ask(): @@ -473,7 +472,7 @@ def test_question_password_optional_with_input_without_ask(): with patch.object(Moulinette, "prompt", return_value="some_value"), patch.object( os, "isatty", return_value=True ): - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result def test_question_password_no_input_default(): @@ -489,7 +488,7 @@ def test_question_password_no_input_default(): # no default for password! with pytest.raises(YunohostError), patch.object(os, "isatty", return_value=False): - parse_args_in_yunohost_format(answers, questions) + ask_questions_and_parse_answers(questions, answers) @pytest.mark.skip # this should raises @@ -506,7 +505,7 @@ def test_question_password_no_input_example(): # no example for password! with pytest.raises(YunohostError), patch.object(os, "isatty", return_value=False): - parse_args_in_yunohost_format(answers, questions) + ask_questions_and_parse_answers(questions, answers) def test_question_password_input_test_ask(): @@ -523,7 +522,7 @@ def test_question_password_input_test_ask(): with patch.object( Moulinette, "prompt", return_value="some_value" ) as prompt, patch.object(os, "isatty", return_value=True): - parse_args_in_yunohost_format(answers, questions) + ask_questions_and_parse_answers(questions, answers) prompt.assert_called_with( message=ask_text, is_password=True, @@ -552,7 +551,7 @@ def test_question_password_input_test_ask_with_example(): with patch.object( Moulinette, "prompt", return_value="some_value" ) as prompt, patch.object(os, "isatty", return_value=True): - parse_args_in_yunohost_format(answers, questions) + ask_questions_and_parse_answers(questions, answers) assert ask_text in prompt.call_args[1]["message"] assert example_text in prompt.call_args[1]["message"] @@ -574,7 +573,7 @@ def test_question_password_input_test_ask_with_help(): with patch.object( Moulinette, "prompt", return_value="some_value" ) as prompt, patch.object(os, "isatty", return_value=True): - parse_args_in_yunohost_format(answers, questions) + ask_questions_and_parse_answers(questions, answers) assert ask_text in prompt.call_args[1]["message"] assert help_text in prompt.call_args[1]["message"] @@ -593,7 +592,7 @@ def test_question_password_bad_chars(): with pytest.raises(YunohostError), patch.object( os, "isatty", return_value=False ): - parse_args_in_yunohost_format({"some_password": i * 8}, questions) + ask_questions_and_parse_answers(questions, {"some_password": i * 8}) def test_question_password_strong_enough(): @@ -608,10 +607,10 @@ def test_question_password_strong_enough(): with pytest.raises(YunohostError), patch.object(os, "isatty", return_value=False): # too short - parse_args_in_yunohost_format({"some_password": "a"}, questions) + ask_questions_and_parse_answers(questions, {"some_password": "a"}) with pytest.raises(YunohostError), patch.object(os, "isatty", return_value=False): - parse_args_in_yunohost_format({"some_password": "password"}, questions) + ask_questions_and_parse_answers(questions, {"some_password": "password"}) def test_question_password_optional_strong_enough(): @@ -626,10 +625,10 @@ def test_question_password_optional_strong_enough(): with pytest.raises(YunohostError), patch.object(os, "isatty", return_value=False): # too short - parse_args_in_yunohost_format({"some_password": "a"}, questions) + ask_questions_and_parse_answers(questions, {"some_password": "a"}) with pytest.raises(YunohostError), patch.object(os, "isatty", return_value=False): - parse_args_in_yunohost_format({"some_password": "password"}, questions) + ask_questions_and_parse_answers(questions, {"some_password": "password"}) def test_question_path(): @@ -641,7 +640,7 @@ def test_question_path(): ] answers = {"some_path": "some_value"} expected_result = OrderedDict({"some_path": ("some_value", "path")}) - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result def test_question_path_no_input(): @@ -654,7 +653,7 @@ def test_question_path_no_input(): answers = {} with pytest.raises(YunohostError), patch.object(os, "isatty", return_value=False): - parse_args_in_yunohost_format(answers, questions) + ask_questions_and_parse_answers(questions, answers) def test_question_path_input(): @@ -671,7 +670,7 @@ def test_question_path_input(): with patch.object(Moulinette, "prompt", return_value="some_value"), patch.object( os, "isatty", return_value=True ): - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result def test_question_path_input_no_ask(): @@ -687,7 +686,7 @@ def test_question_path_input_no_ask(): with patch.object(Moulinette, "prompt", return_value="some_value"), patch.object( os, "isatty", return_value=True ): - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result def test_question_path_no_input_optional(): @@ -701,7 +700,7 @@ def test_question_path_no_input_optional(): answers = {} expected_result = OrderedDict({"some_path": ("", "path")}) with patch.object(os, "isatty", return_value=False): - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result def test_question_path_optional_with_input(): @@ -719,7 +718,7 @@ def test_question_path_optional_with_input(): with patch.object(Moulinette, "prompt", return_value="some_value"), patch.object( os, "isatty", return_value=True ): - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result def test_question_path_optional_with_empty_input(): @@ -737,7 +736,7 @@ def test_question_path_optional_with_empty_input(): with patch.object(Moulinette, "prompt", return_value=""), patch.object( os, "isatty", return_value=True ): - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result def test_question_path_optional_with_input_without_ask(): @@ -754,7 +753,7 @@ def test_question_path_optional_with_input_without_ask(): with patch.object(Moulinette, "prompt", return_value="some_value"), patch.object( os, "isatty", return_value=True ): - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result def test_question_path_no_input_default(): @@ -769,7 +768,7 @@ def test_question_path_no_input_default(): answers = {} expected_result = OrderedDict({"some_path": ("some_value", "path")}) with patch.object(os, "isatty", return_value=False): - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result def test_question_path_input_test_ask(): @@ -786,7 +785,7 @@ def test_question_path_input_test_ask(): with patch.object( Moulinette, "prompt", return_value="some_value" ) as prompt, patch.object(os, "isatty", return_value=True): - parse_args_in_yunohost_format(answers, questions) + ask_questions_and_parse_answers(questions, answers) prompt.assert_called_with( message=ask_text, is_password=False, @@ -814,7 +813,7 @@ def test_question_path_input_test_ask_with_default(): with patch.object( Moulinette, "prompt", return_value="some_value" ) as prompt, patch.object(os, "isatty", return_value=True): - parse_args_in_yunohost_format(answers, questions) + ask_questions_and_parse_answers(questions, answers) prompt.assert_called_with( message=ask_text, is_password=False, @@ -843,7 +842,7 @@ def test_question_path_input_test_ask_with_example(): with patch.object( Moulinette, "prompt", return_value="some_value" ) as prompt, patch.object(os, "isatty", return_value=True): - parse_args_in_yunohost_format(answers, questions) + ask_questions_and_parse_answers(questions, answers) assert ask_text in prompt.call_args[1]["message"] assert example_text in prompt.call_args[1]["message"] @@ -865,7 +864,7 @@ def test_question_path_input_test_ask_with_help(): with patch.object( Moulinette, "prompt", return_value="some_value" ) as prompt, patch.object(os, "isatty", return_value=True): - parse_args_in_yunohost_format(answers, questions) + ask_questions_and_parse_answers(questions, answers) assert ask_text in prompt.call_args[1]["message"] assert help_text in prompt.call_args[1]["message"] @@ -879,7 +878,7 @@ def test_question_boolean(): ] answers = {"some_boolean": "y"} expected_result = OrderedDict({"some_boolean": (1, "boolean")}) - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result def test_question_boolean_all_yes(): @@ -891,46 +890,46 @@ def test_question_boolean_all_yes(): ] expected_result = OrderedDict({"some_boolean": (1, "boolean")}) assert ( - parse_args_in_yunohost_format({"some_boolean": "y"}, questions) + ask_questions_and_parse_answers(questions, {"some_boolean": "y"}) == expected_result ) assert ( - parse_args_in_yunohost_format({"some_boolean": "Y"}, questions) + ask_questions_and_parse_answers(questions, {"some_boolean": "Y"}) == expected_result ) assert ( - parse_args_in_yunohost_format({"some_boolean": "yes"}, questions) + ask_questions_and_parse_answers(questions, {"some_boolean": "yes"}) == expected_result ) assert ( - parse_args_in_yunohost_format({"some_boolean": "Yes"}, questions) + ask_questions_and_parse_answers(questions, {"some_boolean": "Yes"}) == expected_result ) assert ( - parse_args_in_yunohost_format({"some_boolean": "YES"}, questions) + ask_questions_and_parse_answers(questions, {"some_boolean": "YES"}) == expected_result ) assert ( - parse_args_in_yunohost_format({"some_boolean": "1"}, questions) + ask_questions_and_parse_answers(questions, {"some_boolean": "1"}) == expected_result ) assert ( - parse_args_in_yunohost_format({"some_boolean": 1}, questions) == expected_result + ask_questions_and_parse_answers(questions, {"some_boolean": 1}) == expected_result ) assert ( - parse_args_in_yunohost_format({"some_boolean": True}, questions) + ask_questions_and_parse_answers(questions, {"some_boolean": True}) == expected_result ) assert ( - parse_args_in_yunohost_format({"some_boolean": "True"}, questions) + ask_questions_and_parse_answers(questions, {"some_boolean": "True"}) == expected_result ) assert ( - parse_args_in_yunohost_format({"some_boolean": "TRUE"}, questions) + ask_questions_and_parse_answers(questions, {"some_boolean": "TRUE"}) == expected_result ) assert ( - parse_args_in_yunohost_format({"some_boolean": "true"}, questions) + ask_questions_and_parse_answers(questions, {"some_boolean": "true"}) == expected_result ) @@ -944,46 +943,46 @@ def test_question_boolean_all_no(): ] expected_result = OrderedDict({"some_boolean": (0, "boolean")}) assert ( - parse_args_in_yunohost_format({"some_boolean": "n"}, questions) + ask_questions_and_parse_answers(questions, {"some_boolean": "n"}) == expected_result ) assert ( - parse_args_in_yunohost_format({"some_boolean": "N"}, questions) + ask_questions_and_parse_answers(questions, {"some_boolean": "N"}) == expected_result ) assert ( - parse_args_in_yunohost_format({"some_boolean": "no"}, questions) + ask_questions_and_parse_answers(questions, {"some_boolean": "no"}) == expected_result ) assert ( - parse_args_in_yunohost_format({"some_boolean": "No"}, questions) + ask_questions_and_parse_answers(questions, {"some_boolean": "No"}) == expected_result ) assert ( - parse_args_in_yunohost_format({"some_boolean": "No"}, questions) + ask_questions_and_parse_answers(questions, {"some_boolean": "No"}) == expected_result ) assert ( - parse_args_in_yunohost_format({"some_boolean": "0"}, questions) + ask_questions_and_parse_answers(questions, {"some_boolean": "0"}) == expected_result ) assert ( - parse_args_in_yunohost_format({"some_boolean": 0}, questions) == expected_result + ask_questions_and_parse_answers(questions, {"some_boolean": 0}) == expected_result ) assert ( - parse_args_in_yunohost_format({"some_boolean": False}, questions) + ask_questions_and_parse_answers(questions, {"some_boolean": False}) == expected_result ) assert ( - parse_args_in_yunohost_format({"some_boolean": "False"}, questions) + ask_questions_and_parse_answers(questions, {"some_boolean": "False"}) == expected_result ) assert ( - parse_args_in_yunohost_format({"some_boolean": "FALSE"}, questions) + ask_questions_and_parse_answers(questions, {"some_boolean": "FALSE"}) == expected_result ) assert ( - parse_args_in_yunohost_format({"some_boolean": "false"}, questions) + ask_questions_and_parse_answers(questions, {"some_boolean": "false"}) == expected_result ) @@ -1000,7 +999,7 @@ def test_question_boolean_no_input(): expected_result = OrderedDict({"some_boolean": (0, "boolean")}) with patch.object(os, "isatty", return_value=False): - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result def test_question_boolean_bad_input(): @@ -1013,7 +1012,7 @@ def test_question_boolean_bad_input(): answers = {"some_boolean": "stuff"} with pytest.raises(YunohostError), patch.object(os, "isatty", return_value=False): - parse_args_in_yunohost_format(answers, questions) + ask_questions_and_parse_answers(questions, answers) def test_question_boolean_input(): @@ -1030,13 +1029,13 @@ def test_question_boolean_input(): with patch.object(Moulinette, "prompt", return_value="y"), patch.object( os, "isatty", return_value=True ): - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result expected_result = OrderedDict({"some_boolean": (0, "boolean")}) with patch.object(Moulinette, "prompt", return_value="n"), patch.object( os, "isatty", return_value=True ): - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result def test_question_boolean_input_no_ask(): @@ -1052,7 +1051,7 @@ def test_question_boolean_input_no_ask(): with patch.object(Moulinette, "prompt", return_value="y"), patch.object( os, "isatty", return_value=True ): - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result def test_question_boolean_no_input_optional(): @@ -1066,7 +1065,7 @@ def test_question_boolean_no_input_optional(): answers = {} expected_result = OrderedDict({"some_boolean": (0, "boolean")}) # default to false with patch.object(os, "isatty", return_value=False): - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result def test_question_boolean_optional_with_input(): @@ -1084,7 +1083,7 @@ def test_question_boolean_optional_with_input(): with patch.object(Moulinette, "prompt", return_value="y"), patch.object( os, "isatty", return_value=True ): - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result def test_question_boolean_optional_with_empty_input(): @@ -1102,7 +1101,7 @@ def test_question_boolean_optional_with_empty_input(): with patch.object(Moulinette, "prompt", return_value=""), patch.object( os, "isatty", return_value=True ): - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result def test_question_boolean_optional_with_input_without_ask(): @@ -1119,7 +1118,7 @@ def test_question_boolean_optional_with_input_without_ask(): with patch.object(Moulinette, "prompt", return_value="n"), patch.object( os, "isatty", return_value=True ): - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result def test_question_boolean_no_input_default(): @@ -1134,7 +1133,7 @@ def test_question_boolean_no_input_default(): answers = {} expected_result = OrderedDict({"some_boolean": (0, "boolean")}) with patch.object(os, "isatty", return_value=False): - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result def test_question_boolean_bad_default(): @@ -1148,7 +1147,7 @@ def test_question_boolean_bad_default(): ] answers = {} with pytest.raises(YunohostError): - parse_args_in_yunohost_format(answers, questions) + ask_questions_and_parse_answers(questions, answers) def test_question_boolean_input_test_ask(): @@ -1165,7 +1164,7 @@ def test_question_boolean_input_test_ask(): with patch.object(Moulinette, "prompt", return_value=0) as prompt, patch.object( os, "isatty", return_value=True ): - parse_args_in_yunohost_format(answers, questions) + ask_questions_and_parse_answers(questions, answers) prompt.assert_called_with( message=ask_text + " [yes | no]", is_password=False, @@ -1193,7 +1192,7 @@ def test_question_boolean_input_test_ask_with_default(): with patch.object(Moulinette, "prompt", return_value=1) as prompt, patch.object( os, "isatty", return_value=True ): - parse_args_in_yunohost_format(answers, questions) + ask_questions_and_parse_answers(questions, answers) prompt.assert_called_with( message=ask_text + " [yes | no]", is_password=False, @@ -1223,7 +1222,7 @@ def test_question_domain_empty(): ), patch.object( os, "isatty", return_value=False ): - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result def test_question_domain(): @@ -1242,7 +1241,7 @@ def test_question_domain(): with patch.object( domain, "_get_maindomain", return_value=main_domain ), patch.object(domain, "domain_list", return_value={"domains": domains}): - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result def test_question_domain_two_domains(): @@ -1262,7 +1261,7 @@ def test_question_domain_two_domains(): with patch.object( domain, "_get_maindomain", return_value=main_domain ), patch.object(domain, "domain_list", return_value={"domains": domains}): - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result answers = {"some_domain": main_domain} expected_result = OrderedDict({"some_domain": (main_domain, "domain")}) @@ -1270,7 +1269,7 @@ def test_question_domain_two_domains(): with patch.object( domain, "_get_maindomain", return_value=main_domain ), patch.object(domain, "domain_list", return_value={"domains": domains}): - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result def test_question_domain_two_domains_wrong_answer(): @@ -1292,7 +1291,7 @@ def test_question_domain_two_domains_wrong_answer(): with pytest.raises(YunohostError), patch.object( os, "isatty", return_value=False ): - parse_args_in_yunohost_format(answers, questions) + ask_questions_and_parse_answers(questions, answers) def test_question_domain_two_domains_default_no_ask(): @@ -1316,7 +1315,7 @@ def test_question_domain_two_domains_default_no_ask(): ), patch.object( os, "isatty", return_value=False ): - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result def test_question_domain_two_domains_default(): @@ -1335,7 +1334,7 @@ def test_question_domain_two_domains_default(): ), patch.object( os, "isatty", return_value=False ): - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result def test_question_domain_two_domains_default_input(): @@ -1355,11 +1354,11 @@ def test_question_domain_two_domains_default_input(): ): expected_result = OrderedDict({"some_domain": (main_domain, "domain")}) with patch.object(Moulinette, "prompt", return_value=main_domain): - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result expected_result = OrderedDict({"some_domain": (other_domain, "domain")}) with patch.object(Moulinette, "prompt", return_value=other_domain): - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result def test_question_user_empty(): @@ -1385,7 +1384,7 @@ def test_question_user_empty(): with pytest.raises(YunohostError), patch.object( os, "isatty", return_value=False ): - parse_args_in_yunohost_format(answers, questions) + ask_questions_and_parse_answers(questions, answers) def test_question_user(): @@ -1413,7 +1412,7 @@ def test_question_user(): with patch.object(user, "user_list", return_value={"users": users}), patch.object( user, "user_info", return_value={} ): - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result def test_question_user_two_users(): @@ -1448,7 +1447,7 @@ def test_question_user_two_users(): with patch.object(user, "user_list", return_value={"users": users}), patch.object( user, "user_info", return_value={} ): - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result answers = {"some_user": username} expected_result = OrderedDict({"some_user": (username, "user")}) @@ -1456,7 +1455,7 @@ def test_question_user_two_users(): with patch.object(user, "user_list", return_value={"users": users}), patch.object( user, "user_info", return_value={} ): - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result def test_question_user_two_users_wrong_answer(): @@ -1491,7 +1490,7 @@ def test_question_user_two_users_wrong_answer(): with pytest.raises(YunohostError), patch.object( os, "isatty", return_value=False ): - parse_args_in_yunohost_format(answers, questions) + ask_questions_and_parse_answers(questions, answers) def test_question_user_two_users_no_default(): @@ -1521,7 +1520,7 @@ def test_question_user_two_users_no_default(): with pytest.raises(YunohostError), patch.object( os, "isatty", return_value=False ): - parse_args_in_yunohost_format(answers, questions) + ask_questions_and_parse_answers(questions, answers) def test_question_user_two_users_default_input(): @@ -1554,13 +1553,13 @@ def test_question_user_two_users_default_input(): expected_result = OrderedDict({"some_user": (username, "user")}) with patch.object(Moulinette, "prompt", return_value=username): assert ( - parse_args_in_yunohost_format(answers, questions) == expected_result + ask_questions_and_parse_answers(questions, answers) == expected_result ) expected_result = OrderedDict({"some_user": (other_user, "user")}) with patch.object(Moulinette, "prompt", return_value=other_user): assert ( - parse_args_in_yunohost_format(answers, questions) == expected_result + ask_questions_and_parse_answers(questions, answers) == expected_result ) @@ -1573,7 +1572,7 @@ def test_question_number(): ] answers = {"some_number": 1337} expected_result = OrderedDict({"some_number": (1337, "number")}) - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result def test_question_number_no_input(): @@ -1586,7 +1585,7 @@ def test_question_number_no_input(): answers = {} with pytest.raises(YunohostError), patch.object(os, "isatty", return_value=False): - parse_args_in_yunohost_format(answers, questions) + ask_questions_and_parse_answers(questions, answers) def test_question_number_bad_input(): @@ -1599,11 +1598,11 @@ def test_question_number_bad_input(): answers = {"some_number": "stuff"} with pytest.raises(YunohostError), patch.object(os, "isatty", return_value=False): - parse_args_in_yunohost_format(answers, questions) + ask_questions_and_parse_answers(questions, answers) answers = {"some_number": 1.5} with pytest.raises(YunohostError), patch.object(os, "isatty", return_value=False): - parse_args_in_yunohost_format(answers, questions) + ask_questions_and_parse_answers(questions, answers) def test_question_number_input(): @@ -1620,18 +1619,18 @@ def test_question_number_input(): with patch.object(Moulinette, "prompt", return_value="1337"), patch.object( os, "isatty", return_value=True ): - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result with patch.object(Moulinette, "prompt", return_value=1337), patch.object( os, "isatty", return_value=True ): - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result expected_result = OrderedDict({"some_number": (0, "number")}) with patch.object(Moulinette, "prompt", return_value="0"), patch.object( os, "isatty", return_value=True ): - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result def test_question_number_input_no_ask(): @@ -1647,7 +1646,7 @@ def test_question_number_input_no_ask(): with patch.object(Moulinette, "prompt", return_value="1337"), patch.object( os, "isatty", return_value=True ): - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result def test_question_number_no_input_optional(): @@ -1661,7 +1660,7 @@ def test_question_number_no_input_optional(): answers = {} expected_result = OrderedDict({"some_number": (None, "number")}) # default to 0 with patch.object(os, "isatty", return_value=False): - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result def test_question_number_optional_with_input(): @@ -1679,7 +1678,7 @@ def test_question_number_optional_with_input(): with patch.object(Moulinette, "prompt", return_value="1337"), patch.object( os, "isatty", return_value=True ): - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result def test_question_number_optional_with_input_without_ask(): @@ -1696,7 +1695,7 @@ def test_question_number_optional_with_input_without_ask(): with patch.object(Moulinette, "prompt", return_value="0"), patch.object( os, "isatty", return_value=True ): - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result def test_question_number_no_input_default(): @@ -1711,7 +1710,7 @@ def test_question_number_no_input_default(): answers = {} expected_result = OrderedDict({"some_number": (1337, "number")}) with patch.object(os, "isatty", return_value=False): - assert parse_args_in_yunohost_format(answers, questions) == expected_result + assert ask_questions_and_parse_answers(questions, answers) == expected_result def test_question_number_bad_default(): @@ -1725,7 +1724,7 @@ def test_question_number_bad_default(): ] answers = {} with pytest.raises(YunohostError), patch.object(os, "isatty", return_value=False): - parse_args_in_yunohost_format(answers, questions) + ask_questions_and_parse_answers(questions, answers) def test_question_number_input_test_ask(): @@ -1742,7 +1741,7 @@ def test_question_number_input_test_ask(): with patch.object( Moulinette, "prompt", return_value="1111" ) as prompt, patch.object(os, "isatty", return_value=True): - parse_args_in_yunohost_format(answers, questions) + ask_questions_and_parse_answers(questions, answers) prompt.assert_called_with( message=ask_text, is_password=False, @@ -1770,7 +1769,7 @@ def test_question_number_input_test_ask_with_default(): with patch.object( Moulinette, "prompt", return_value="1111" ) as prompt, patch.object(os, "isatty", return_value=True): - parse_args_in_yunohost_format(answers, questions) + ask_questions_and_parse_answers(questions, answers) prompt.assert_called_with( message=ask_text, is_password=False, @@ -1799,7 +1798,7 @@ def test_question_number_input_test_ask_with_example(): with patch.object( Moulinette, "prompt", return_value="1111" ) as prompt, patch.object(os, "isatty", return_value=True): - parse_args_in_yunohost_format(answers, questions) + ask_questions_and_parse_answers(questions, answers) assert ask_text in prompt.call_args[1]["message"] assert example_value in prompt.call_args[1]["message"] @@ -1821,7 +1820,7 @@ def test_question_number_input_test_ask_with_help(): with patch.object( Moulinette, "prompt", return_value="1111" ) as prompt, patch.object(os, "isatty", return_value=True): - parse_args_in_yunohost_format(answers, questions) + ask_questions_and_parse_answers(questions, answers) assert ask_text in prompt.call_args[1]["message"] assert help_value in prompt.call_args[1]["message"] @@ -1833,5 +1832,5 @@ def test_question_display_text(): with patch.object(sys, "stdout", new_callable=StringIO) as stdout, patch.object( os, "isatty", return_value=True ): - parse_args_in_yunohost_format(answers, questions) + ask_questions_and_parse_answers(questions, answers) assert "foobar" in stdout.getvalue() diff --git a/src/yunohost/utils/config.py b/src/yunohost/utils/config.py index 81b651be5..e83d794ff 100644 --- a/src/yunohost/utils/config.py +++ b/src/yunohost/utils/config.py @@ -381,7 +381,7 @@ class ConfigPanel: # Check and ask unanswered questions self.new_values.update( - parse_args_in_yunohost_format(self.args, section["options"]) + ask_questions_and_parse_answers(section["options"], self.args) ) self.new_values = { key: value[0] From 26a49929611434361182585eba5a521cff557462 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Thu, 23 Sep 2021 05:33:20 +0200 Subject: [PATCH 12/21] Refactor _normalize_domain_path into DomainQuestion + PathQuestion normalizers --- src/yunohost/app.py | 37 ++++++++++++++---------------------- src/yunohost/utils/config.py | 16 ++++++++++++++++ 2 files changed, 30 insertions(+), 23 deletions(-) diff --git a/src/yunohost/app.py b/src/yunohost/app.py index b0ae7f0d5..396ce04e7 100644 --- a/src/yunohost/app.py +++ b/src/yunohost/app.py @@ -55,6 +55,8 @@ from yunohost.utils import packages from yunohost.utils.config import ( ConfigPanel, ask_questions_and_parse_answers, + DomainQuestion, + PathQuestion ) from yunohost.utils.i18n import _value_for_locale from yunohost.utils.error import YunohostError, YunohostValidationError @@ -441,8 +443,11 @@ def app_change_url(operation_logger, app, domain, path): old_path = app_setting(app, "path") # Normalize path and domain format - old_domain, old_path = _normalize_domain_path(old_domain, old_path) - domain, path = _normalize_domain_path(domain, path) + + domain = DomainQuestion.normalize(domain) + old_domain = DomainQuestion.normalize(old_domain) + path = PathQuestion.normalize(path) + old_path = PathQuestion.normalize(old_path) if (domain, path) == (old_domain, old_path): raise YunohostValidationError( @@ -1459,7 +1464,8 @@ def app_register_url(app, domain, path): permission_sync_to_user, ) - domain, path = _normalize_domain_path(domain, path) + domain = DomainQuestion.normalize(domain) + path = PathQuestion.normalize(path) # We cannot change the url of an app already installed simply by changing # the settings... @@ -2381,7 +2387,9 @@ def _validate_and_normalize_webpath(args_dict, app_folder): domain = domain_args[0][1] path = path_args[0][1] - domain, path = _normalize_domain_path(domain, path) + + domain = DomainQuestion.normalize(domain) + path = PathQuestion.normalize(path) # Check the url is available _assert_no_conflicting_apps(domain, path) @@ -2415,24 +2423,6 @@ def _validate_and_normalize_webpath(args_dict, app_folder): _assert_no_conflicting_apps(domain, "/", full_domain=True) -def _normalize_domain_path(domain, path): - - # We want url to be of the format : - # some.domain.tld/foo - - # Remove http/https prefix if it's there - if domain.startswith("https://"): - domain = domain[len("https://") :] - elif domain.startswith("http://"): - domain = domain[len("http://") :] - - # Remove trailing slashes - domain = domain.rstrip("/").lower() - path = "/" + path.strip("/") - - return domain, path - - def _get_conflicting_apps(domain, path, ignore_app=None): """ Return a list of all conflicting apps with a domain/path (it can be empty) @@ -2445,7 +2435,8 @@ def _get_conflicting_apps(domain, path, ignore_app=None): from yunohost.domain import _assert_domain_exists - domain, path = _normalize_domain_path(domain, path) + domain = DomainQuestion.normalize(domain) + path = PathQuestion.normalize(path) # Abort if domain is unknown _assert_domain_exists(domain) diff --git a/src/yunohost/utils/config.py b/src/yunohost/utils/config.py index e83d794ff..1a39c0da4 100644 --- a/src/yunohost/utils/config.py +++ b/src/yunohost/utils/config.py @@ -728,6 +728,10 @@ class PathQuestion(Question): argument_type = "path" default_value = "" + @staticmethod + def normalize(value, option={}): + return "/" + value.strip("/") + class BooleanQuestion(Question): argument_type = "boolean" @@ -837,6 +841,18 @@ class DomainQuestion(Question): error=m18n.n("domain_name_unknown", domain=self.value), ) + @staticmethod + def normalize(value, option={}): + if value.startswith("https://"): + value = value[len("https://"):] + elif value.startswith("http://"): + value = value[len("http://"):] + + # Remove trailing slashes + value = value.rstrip("/").lower() + + return value + class UserQuestion(Question): argument_type = "user" From d4c3a6975e06d6dfc8fc8d9eb24d503615bacf7c Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Thu, 23 Sep 2021 10:38:30 +0200 Subject: [PATCH 13/21] ask_questions_and_parse_answers may take directly a dict as input --- src/yunohost/utils/config.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/yunohost/utils/config.py b/src/yunohost/utils/config.py index 1a39c0da4..604e20f4f 100644 --- a/src/yunohost/utils/config.py +++ b/src/yunohost/utils/config.py @@ -25,7 +25,7 @@ import urllib.parse import tempfile import shutil from collections import OrderedDict -from typing import Optional, Dict, List +from typing import Optional, Dict, List, Union, Any, Mapping from moulinette.interfaces.cli import colorize from moulinette import Moulinette, m18n @@ -461,7 +461,7 @@ class Question(object): hide_user_input_in_prompt = False pattern: Optional[Dict] = None - def __init__(self, question): + def __init__(self, question: Dict[str, Any]): self.name = question["name"] self.type = question.get("type", "string") self.default = question.get("default", None) @@ -1043,7 +1043,7 @@ ARGUMENTS_TYPE_PARSERS = { } -def ask_questions_and_parse_answers(questions, prefilled_answers=""): +def ask_questions_and_parse_answers(questions: Dict, prefilled_answers: Union[str, Mapping[str, Any]] = {}) -> Mapping[str, Any]: """Parse arguments store in either manifest.json or actions.json or from a config panel against the user answers when they are present. @@ -1051,10 +1051,12 @@ def ask_questions_and_parse_answers(questions, prefilled_answers=""): questions -- the arguments description store in yunohost format from actions.json/toml, manifest.json/toml or config_panel.json/toml - prefilled_answers -- a url-formated string such as "domain=yolo.test&path=/foobar&admin=sam" + prefilled_answers -- a url "query-string" such as "domain=yolo.test&path=/foobar&admin=sam" + or a dict such as {"domain": "yolo.test", "path": "/foobar", "admin": "sam"} """ - prefilled_answers = dict(urllib.parse.parse_qsl(prefilled_answers or "", keep_blank_values=True)) + if isinstance(prefilled_answers, str): + prefilled_answers = dict(urllib.parse.parse_qs(prefilled_answers or "", keep_blank_values=True)) out = OrderedDict() From 68d849f7abcceb451f11e7a18c9d749b77105bcc Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Thu, 23 Sep 2021 10:45:37 +0200 Subject: [PATCH 14/21] Adapt tests for domain/path normalize --- src/yunohost/tests/test_appurl.py | 18 +----------------- src/yunohost/tests/test_questions.py | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 17 deletions(-) diff --git a/src/yunohost/tests/test_appurl.py b/src/yunohost/tests/test_appurl.py index f15ed391f..5954d239a 100644 --- a/src/yunohost/tests/test_appurl.py +++ b/src/yunohost/tests/test_appurl.py @@ -4,7 +4,7 @@ import os from .conftest import get_test_apps_dir from yunohost.utils.error import YunohostError -from yunohost.app import app_install, app_remove, _normalize_domain_path +from yunohost.app import app_install, app_remove from yunohost.domain import _get_maindomain, domain_url_available from yunohost.permission import _validate_and_sanitize_permission_url @@ -28,22 +28,6 @@ def teardown_function(function): pass -def test_normalize_domain_path(): - - assert _normalize_domain_path("https://yolo.swag/", "macnuggets") == ( - "yolo.swag", - "/macnuggets", - ) - assert _normalize_domain_path("http://yolo.swag", "/macnuggets/") == ( - "yolo.swag", - "/macnuggets", - ) - assert _normalize_domain_path("yolo.swag/", "macnuggets/") == ( - "yolo.swag", - "/macnuggets", - ) - - def test_urlavailable(): # Except the maindomain/macnuggets to be available diff --git a/src/yunohost/tests/test_questions.py b/src/yunohost/tests/test_questions.py index a01ad71f0..9680ccb46 100644 --- a/src/yunohost/tests/test_questions.py +++ b/src/yunohost/tests/test_questions.py @@ -12,6 +12,8 @@ from yunohost import domain, user from yunohost.utils.config import ( ask_questions_and_parse_answers, PasswordQuestion, + DomainQuestion, + PathQuestion ) from yunohost.utils.error import YunohostError @@ -1834,3 +1836,19 @@ def test_question_display_text(): ): ask_questions_and_parse_answers(questions, answers) assert "foobar" in stdout.getvalue() + + +def test_normalize_domain(): + + assert DomainQuestion("https://yolo.swag/") == "yolo.swag" + assert DomainQuestion("http://yolo.swag") == "yolo.swag" + assert DomainQuestion("yolo.swag/") == "yolo.swag" + + +def test_normalize_path(): + + assert PathQuestion("macnuggets") == "/macnuggets" + assert PathQuestion("mac/nuggets") == "/mac/nuggets" + assert PathQuestion("/macnuggets/") == "/macnuggets" + assert PathQuestion("macnuggets/") == "/macnuggets" + assert PathQuestion("////macnuggets///") == "/macnuggets" From 548042d5036d9458c3dd7fa65598a20a505de666 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Thu, 23 Sep 2021 13:07:46 +0200 Subject: [PATCH 15/21] Moar fixes .. + add test for boolean normalize/humanize --- src/yunohost/tests/test_questions.py | 120 ++++++++++++++++++++++----- src/yunohost/utils/config.py | 73 +++++++++------- 2 files changed, 142 insertions(+), 51 deletions(-) diff --git a/src/yunohost/tests/test_questions.py b/src/yunohost/tests/test_questions.py index 9680ccb46..cf4a8832d 100644 --- a/src/yunohost/tests/test_questions.py +++ b/src/yunohost/tests/test_questions.py @@ -13,9 +13,10 @@ from yunohost.utils.config import ( ask_questions_and_parse_answers, PasswordQuestion, DomainQuestion, - PathQuestion + PathQuestion, + BooleanQuestion ) -from yunohost.utils.error import YunohostError +from yunohost.utils.error import YunohostError, YunohostValidationError """ @@ -640,8 +641,8 @@ def test_question_path(): "type": "path", } ] - answers = {"some_path": "some_value"} - expected_result = OrderedDict({"some_path": ("some_value", "path")}) + answers = {"some_path": "/some_value"} + expected_result = OrderedDict({"some_path": ("/some_value", "path")}) assert ask_questions_and_parse_answers(questions, answers) == expected_result @@ -667,9 +668,9 @@ def test_question_path_input(): } ] answers = {} - expected_result = OrderedDict({"some_path": ("some_value", "path")}) + expected_result = OrderedDict({"some_path": ("/some_value", "path")}) - with patch.object(Moulinette, "prompt", return_value="some_value"), patch.object( + with patch.object(Moulinette, "prompt", return_value="/some_value"), patch.object( os, "isatty", return_value=True ): assert ask_questions_and_parse_answers(questions, answers) == expected_result @@ -683,9 +684,9 @@ def test_question_path_input_no_ask(): } ] answers = {} - expected_result = OrderedDict({"some_path": ("some_value", "path")}) + expected_result = OrderedDict({"some_path": ("/some_value", "path")}) - with patch.object(Moulinette, "prompt", return_value="some_value"), patch.object( + with patch.object(Moulinette, "prompt", return_value="/some_value"), patch.object( os, "isatty", return_value=True ): assert ask_questions_and_parse_answers(questions, answers) == expected_result @@ -715,9 +716,9 @@ def test_question_path_optional_with_input(): } ] answers = {} - expected_result = OrderedDict({"some_path": ("some_value", "path")}) + expected_result = OrderedDict({"some_path": ("/some_value", "path")}) - with patch.object(Moulinette, "prompt", return_value="some_value"), patch.object( + with patch.object(Moulinette, "prompt", return_value="/some_value"), patch.object( os, "isatty", return_value=True ): assert ask_questions_and_parse_answers(questions, answers) == expected_result @@ -750,9 +751,9 @@ def test_question_path_optional_with_input_without_ask(): } ] answers = {} - expected_result = OrderedDict({"some_path": ("some_value", "path")}) + expected_result = OrderedDict({"some_path": ("/some_value", "path")}) - with patch.object(Moulinette, "prompt", return_value="some_value"), patch.object( + with patch.object(Moulinette, "prompt", return_value="/some_value"), patch.object( os, "isatty", return_value=True ): assert ask_questions_and_parse_answers(questions, answers) == expected_result @@ -768,7 +769,7 @@ def test_question_path_no_input_default(): } ] answers = {} - expected_result = OrderedDict({"some_path": ("some_value", "path")}) + expected_result = OrderedDict({"some_path": ("/some_value", "path")}) with patch.object(os, "isatty", return_value=False): assert ask_questions_and_parse_answers(questions, answers) == expected_result @@ -801,7 +802,7 @@ def test_question_path_input_test_ask(): def test_question_path_input_test_ask_with_default(): ask_text = "some question" - default_text = "some example" + default_text = "someexample" questions = [ { "name": "some_path", @@ -1838,17 +1839,92 @@ def test_question_display_text(): assert "foobar" in stdout.getvalue() +def test_normalize_boolean_nominal(): + + assert BooleanQuestion.normalize("yes") == 1 + assert BooleanQuestion.normalize("Yes") == 1 + assert BooleanQuestion.normalize(" yes ") == 1 + assert BooleanQuestion.normalize("y") == 1 + assert BooleanQuestion.normalize("true") == 1 + assert BooleanQuestion.normalize("True") == 1 + assert BooleanQuestion.normalize("on") == 1 + assert BooleanQuestion.normalize("1") == 1 + assert BooleanQuestion.normalize(1) == 1 + + assert BooleanQuestion.normalize("no") == 0 + assert BooleanQuestion.normalize("No") == 0 + assert BooleanQuestion.normalize(" no ") == 0 + assert BooleanQuestion.normalize("n") == 0 + assert BooleanQuestion.normalize("false") == 0 + assert BooleanQuestion.normalize("False") == 0 + assert BooleanQuestion.normalize("off") == 0 + assert BooleanQuestion.normalize("0") == 0 + assert BooleanQuestion.normalize(0) == 0 + + assert BooleanQuestion.normalize("") is None + assert BooleanQuestion.normalize(" ") is None + assert BooleanQuestion.normalize(" none ") is None + assert BooleanQuestion.normalize("None") is None + assert BooleanQuestion.normalize("none") is None + assert BooleanQuestion.normalize(None) is None + + +def test_normalize_boolean_humanize(): + + assert BooleanQuestion.humanize("yes") == "yes" + assert BooleanQuestion.humanize("true") == "yes" + assert BooleanQuestion.humanize("on") == "yes" + + assert BooleanQuestion.humanize("no") == "no" + assert BooleanQuestion.humanize("false") == "no" + assert BooleanQuestion.humanize("off") == "no" + + +def test_normalize_boolean_invalid(): + + with pytest.raises(YunohostValidationError): + BooleanQuestion.normalize("yesno") + with pytest.raises(YunohostValidationError): + BooleanQuestion.normalize("foobar") + with pytest.raises(YunohostValidationError): + BooleanQuestion.normalize("enabled") + + +def test_normalize_boolean_special_yesno(): + + customyesno = {"yes": "enabled", "no": "disabled"} + + assert BooleanQuestion.normalize("yes", customyesno) == "enabled" + assert BooleanQuestion.normalize("true", customyesno) == "enabled" + assert BooleanQuestion.normalize("enabled", customyesno) == "enabled" + assert BooleanQuestion.humanize("yes", customyesno) == "yes" + assert BooleanQuestion.humanize("true", customyesno) == "yes" + assert BooleanQuestion.humanize("enabled", customyesno) == "yes" + + assert BooleanQuestion.normalize("no", customyesno) == "disabled" + assert BooleanQuestion.normalize("false", customyesno) == "disabled" + assert BooleanQuestion.normalize("disabled", customyesno) == "disabled" + assert BooleanQuestion.humanize("no", customyesno) == "no" + assert BooleanQuestion.humanize("false", customyesno) == "no" + assert BooleanQuestion.humanize("disabled", customyesno) == "no" + + def test_normalize_domain(): - assert DomainQuestion("https://yolo.swag/") == "yolo.swag" - assert DomainQuestion("http://yolo.swag") == "yolo.swag" - assert DomainQuestion("yolo.swag/") == "yolo.swag" + assert DomainQuestion.normalize("https://yolo.swag/") == "yolo.swag" + assert DomainQuestion.normalize("http://yolo.swag") == "yolo.swag" + assert DomainQuestion.normalize("yolo.swag/") == "yolo.swag" def test_normalize_path(): - assert PathQuestion("macnuggets") == "/macnuggets" - assert PathQuestion("mac/nuggets") == "/mac/nuggets" - assert PathQuestion("/macnuggets/") == "/macnuggets" - assert PathQuestion("macnuggets/") == "/macnuggets" - assert PathQuestion("////macnuggets///") == "/macnuggets" + assert PathQuestion.normalize("") == "/" + assert PathQuestion.normalize("") == "/" + assert PathQuestion.normalize("macnuggets") == "/macnuggets" + assert PathQuestion.normalize("/macnuggets") == "/macnuggets" + assert PathQuestion.normalize(" /macnuggets ") == "/macnuggets" + assert PathQuestion.normalize("/macnuggets") == "/macnuggets" + assert PathQuestion.normalize("mac/nuggets") == "/mac/nuggets" + assert PathQuestion.normalize("/macnuggets/") == "/macnuggets" + assert PathQuestion.normalize("macnuggets/") == "/macnuggets" + assert PathQuestion.normalize("////macnuggets///") == "/macnuggets" diff --git a/src/yunohost/utils/config.py b/src/yunohost/utils/config.py index 604e20f4f..179e9640f 100644 --- a/src/yunohost/utils/config.py +++ b/src/yunohost/utils/config.py @@ -730,7 +730,23 @@ class PathQuestion(Question): @staticmethod def normalize(value, option={}): - return "/" + value.strip("/") + + option = option.__dict__ if isinstance(option, Question) else option + + if not value.strip(): + if option.get("optional"): + return "" + # Hmpf here we could just have a "else" case + # but we also want PathQuestion.normalize("") to return "/" + # (i.e. if no option is provided, hence .get("optional") is None + elif option.get("optional") is False: + raise YunohostValidationError( + "app_argument_invalid", + name=option.get("name"), + error="Question is mandatory" + ) + + return "/" + value.strip().strip(" /") class BooleanQuestion(Question): @@ -742,6 +758,8 @@ class BooleanQuestion(Question): @staticmethod def humanize(value, option={}): + option = option.__dict__ if isinstance(option, Question) else option + yes = option.get("yes", 1) no = option.get("no", 0) @@ -756,50 +774,45 @@ class BooleanQuestion(Question): raise YunohostValidationError( "app_argument_choice_invalid", - name=getattr(option, "name", None) or option.get("name"), + name=option.get("name"), value=value, choices="yes/no", ) @staticmethod def normalize(value, option={}): + + option = option.__dict__ if isinstance(option, Question) else option + if isinstance(value, str): value = value.strip() - yes = option.get("yes", 1) - no = option.get("no", 0) + technical_yes = option.get("yes", 1) + technical_no = option.get("no", 0) + + no_answers = BooleanQuestion.no_answers + yes_answers = BooleanQuestion.yes_answers + + assert str(technical_yes).lower() not in no_answers, f"'yes' value can't be in {no_answers}" + assert 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()] strvalue = str(value).lower() - # - # N.B.: - # we check first if the value is equal to yes - # then if equal to no - # then if in yes_answers - # then if in no_answers - # - # This is to hopefully cover the weird edgecase - # where the value for yes/no could be reversed - # such as yes=false or yes=0 - # no=true no=1 - # + if strvalue in yes_answers: + return technical_yes + if strvalue in no_answers: + return technical_no - if strvalue == str(yes).lower(): - return yes - if strvalue == str(no).lower(): - return no - if strvalue in BooleanQuestion.yes_answers: - return yes - if strvalue in BooleanQuestion.no_answers: - return no - - if value in [None, ""]: + if strvalue in ["none", ""]: return None raise YunohostValidationError( "app_argument_choice_invalid", - name=getattr(option, "name", None) or option.get("name"), - value=value, + name=option.get("name"), + value=strvalue, choices="yes/no", ) @@ -898,6 +911,7 @@ class NumberQuestion(Question): @staticmethod def normalize(value, option={}): + if isinstance(value, int): return value @@ -910,9 +924,10 @@ class NumberQuestion(Question): if value in [None, ""]: return value + option = option.__dict__ if isinstance(option, Question) else option raise YunohostValidationError( "app_argument_invalid", - name=getattr(option, "name", None) or option.get("name"), + name=option.get("name"), error=m18n.n("invalid_number") ) From 6b8cb0c00508f8a94d2678b02277e08737e3b42b Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Thu, 23 Sep 2021 18:57:01 +0200 Subject: [PATCH 16/21] Hmpf refactor moar stuff but does this ever ends --- src/yunohost/app.py | 90 ++++++++++++++------------- src/yunohost/tests/test_app_config.py | 6 +- src/yunohost/utils/config.py | 37 ++++++----- 3 files changed, 74 insertions(+), 59 deletions(-) diff --git a/src/yunohost/app.py b/src/yunohost/app.py index 396ce04e7..01c0cb0cd 100644 --- a/src/yunohost/app.py +++ b/src/yunohost/app.py @@ -34,6 +34,7 @@ import subprocess import glob import tempfile from collections import OrderedDict +from typing import List from moulinette import Moulinette, m18n from moulinette.core import MoulinetteError @@ -55,6 +56,7 @@ from yunohost.utils import packages from yunohost.utils.config import ( ConfigPanel, ask_questions_and_parse_answers, + Question, DomainQuestion, PathQuestion ) @@ -899,11 +901,13 @@ def app_install( app_instance_name = app_id # Retrieve arguments list for install script - questions = manifest.get("arguments", {}).get("install", {}) - args = ask_questions_and_parse_answers(questions, prefilled_answers=args) + raw_questions = manifest.get("arguments", {}).get("install", {}) + questions = ask_questions_and_parse_answers(raw_questions, prefilled_answers=args) + args = {question.name: question.value for question in questions if question.value is not None} # Validate domain / path availability for webapps - _validate_and_normalize_webpath(args, extracted_app_folder) + path_requirement = _guess_webapp_path_requirement(questions, extracted_app_folder) + _validate_webpath_requirement(questions, path_requirement) # Attempt to patch legacy helpers ... _patch_legacy_helpers(extracted_app_folder) @@ -972,9 +976,10 @@ def app_install( env_dict["YNH_APP_BASEDIR"] = extracted_app_folder env_dict_for_logging = env_dict.copy() - for arg_name, arg_value_and_type in args.items(): - if arg_value_and_type[1] == "password": - del env_dict_for_logging["YNH_APP_ARG_%s" % arg_name.upper()] + for question in questions: + # Or should it be more generally question.redact ? + if question.type == "password": + del env_dict_for_logging["YNH_APP_ARG_%s" % question.name.upper()] operation_logger.extra.update({"env": env_dict_for_logging}) @@ -1635,8 +1640,9 @@ def app_action_run(operation_logger, app, action, args=None): action_declaration = actions[action] # Retrieve arguments list for install script - questions = actions[action].get("arguments", {}) - args = ask_questions_and_parse_answers(questions, prefilled_answers=args) + raw_questions = actions[action].get("arguments", {}) + questions = ask_questions_and_parse_answers(raw_questions, prefilled_answers=args) + args = {question.name: question.value for question in questions if question.value is not None} tmp_workdir_for_app = _make_tmp_workdir_for_app(app=app) @@ -2371,35 +2377,20 @@ def _check_manifest_requirements(manifest, app_instance_name): ) -def _validate_and_normalize_webpath(args_dict, app_folder): +def _guess_webapp_path_requirement(questions: List[Question], app_folder: str) -> str: # If there's only one "domain" and "path", validate that domain/path # is an available url and normalize the path. - domain_args = [ - (name, value[0]) for name, value in args_dict.items() if value[1] == "domain" - ] - path_args = [ - (name, value[0]) for name, value in args_dict.items() if value[1] == "path" - ] + domain_questions = [question for question in questions if question.type == "domain"] + path_questions = [question for question in questions if question.type == "path"] - if len(domain_args) == 1 and len(path_args) == 1: - - domain = domain_args[0][1] - path = path_args[0][1] - - domain = DomainQuestion.normalize(domain) - path = PathQuestion.normalize(path) - - # Check the url is available - _assert_no_conflicting_apps(domain, path) - - # (We save this normalized path so that the install script have a - # standard path format to deal with no matter what the user inputted) - args_dict[path_args[0][0]] = (path, "path") - - # This is likely to be a full-domain app... - elif len(domain_args) == 1 and len(path_args) == 0: + if len(domain_questions) == 0 and len(path_questions) == 0: + return None + if len(domain_questions) == 1 and len(path_questions) == 1: + return "domain_and_path" + if len(domain_questions) == 1 and len(path_questions) == 0: + # This is likely to be a full-domain app... # Confirm that this is a full-domain app This should cover most cases # ... though anyway the proper solution is to implement some mechanism @@ -2409,18 +2400,33 @@ def _validate_and_normalize_webpath(args_dict, app_folder): # Full-domain apps typically declare something like path_url="/" or path=/ # and use ynh_webpath_register or yunohost_app_checkurl inside the install script - install_script_content = open( - os.path.join(app_folder, "scripts/install") - ).read() + install_script_content = read_file(os.path.join(app_folder, "scripts/install")) if re.search( - r"\npath(_url)?=[\"']?/[\"']?\n", install_script_content + r"\npath(_url)?=[\"']?/[\"']?", install_script_content ) and re.search( - r"(ynh_webpath_register|yunohost app checkurl)", install_script_content + r"ynh_webpath_register", install_script_content ): + return "full_domain" - domain = domain_args[0][1] - _assert_no_conflicting_apps(domain, "/", full_domain=True) + return "?" + + +def _validate_webpath_requirement(questions: List[Question], path_requirement: str) -> None: + + domain_questions = [question for question in questions if question.type == "domain"] + path_questions = [question for question in questions if question.type == "path"] + + if path_requirement == "domain_and_path": + + domain = domain_questions[0].value + path = path_questions[0].value + _assert_no_conflicting_apps(domain, path, full_domain=True) + + elif path_requirement == "full_domain": + + domain = domain_questions[0].value + _assert_no_conflicting_apps(domain, "/", full_domain=True) def _get_conflicting_apps(domain, path, ignore_app=None): @@ -2499,10 +2505,8 @@ def _make_environment_for_app_script(app, args={}, args_prefix="APP_ARG_"): "YNH_APP_MANIFEST_VERSION": manifest.get("version", "?"), } - for arg_name, arg_value_and_type in args.items(): - env_dict["YNH_%s%s" % (args_prefix, arg_name.upper())] = str( - arg_value_and_type[0] - ) + for arg_name, arg_value in args.items(): + env_dict["YNH_%s%s" % (args_prefix, arg_name.upper())] = str(arg_value) return env_dict diff --git a/src/yunohost/tests/test_app_config.py b/src/yunohost/tests/test_app_config.py index d705076c4..248f035f5 100644 --- a/src/yunohost/tests/test_app_config.py +++ b/src/yunohost/tests/test_app_config.py @@ -2,9 +2,11 @@ import glob import os import shutil import pytest +from mock import patch from .conftest import get_test_apps_dir +from moulinette import Moulinette from moulinette.utils.filesystem import read_file from yunohost.domain import _get_maindomain @@ -146,7 +148,9 @@ def test_app_config_regular_setting(config_app): assert app_config_get(config_app, "main.components.boolean") == "1" assert app_setting(config_app, "boolean") == "1" - with pytest.raises(YunohostValidationError): + with pytest.raises(YunohostValidationError), \ + patch.object(os, "isatty", return_value=False), \ + patch.object(Moulinette, "prompt", return_value="pwet"): app_config_set(config_app, "main.components.boolean", "pwet") diff --git a/src/yunohost/utils/config.py b/src/yunohost/utils/config.py index 179e9640f..e5f078267 100644 --- a/src/yunohost/utils/config.py +++ b/src/yunohost/utils/config.py @@ -380,14 +380,13 @@ class ConfigPanel: display_header(f"\n# {name}") # Check and ask unanswered questions - self.new_values.update( - ask_questions_and_parse_answers(section["options"], self.args) - ) - self.new_values = { - key: value[0] - for key, value in self.new_values.items() - if not value[0] is None - } + questions = ask_questions_and_parse_answers(section["options"], self.args) + self.new_values.update({ + question.name: question.value + for question in questions + if question.value is not None + }) + self.errors = None def _get_default_values(self): @@ -538,9 +537,10 @@ class Question(object): raise break + self.value = self._post_parse_value() - return (self.value, self.argument_type) + return self.value def _prevalidate(self): if self.value in [None, ""] and not self.optional: @@ -1058,7 +1058,7 @@ ARGUMENTS_TYPE_PARSERS = { } -def ask_questions_and_parse_answers(questions: Dict, prefilled_answers: Union[str, Mapping[str, Any]] = {}) -> Mapping[str, Any]: +def ask_questions_and_parse_answers(questions: Dict, prefilled_answers: Union[str, Mapping[str, Any]] = {}) -> List[Question]: """Parse arguments store in either manifest.json or actions.json or from a config panel against the user answers when they are present. @@ -1071,17 +1071,24 @@ def ask_questions_and_parse_answers(questions: Dict, prefilled_answers: Union[st """ if isinstance(prefilled_answers, str): - prefilled_answers = dict(urllib.parse.parse_qs(prefilled_answers or "", keep_blank_values=True)) + # FIXME FIXME : this is not uniform with config_set() which uses parse.qs (no l) + # parse_qsl parse single values + # 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) + prefilled_answers = dict(urllib.parse.parse_qsl(prefilled_answers or "", keep_blank_values=True)) - out = OrderedDict() + if not prefilled_answers: + prefilled_answers = {} + + out = [] for question in questions: question_class = ARGUMENTS_TYPE_PARSERS[question.get("type", "string")] question["value"] = prefilled_answers.get(question["name"]) question = question_class(question) - answer = question.ask_if_needed() - if answer is not None: - out[question.name] = answer + question.ask_if_needed() + out.append(question) return out From 74102b607d54b94aeafdbaf999659ef50cb96eb4 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Thu, 23 Sep 2021 19:21:48 +0200 Subject: [PATCH 17/21] Simplify error management --- locales/en.json | 2 +- src/yunohost/utils/config.py | 29 ++++++----------------------- 2 files changed, 7 insertions(+), 24 deletions(-) diff --git a/locales/en.json b/locales/en.json index 3354dc19c..99432492c 100644 --- a/locales/en.json +++ b/locales/en.json @@ -13,7 +13,7 @@ "app_already_installed": "{app} is already installed", "app_already_installed_cant_change_url": "This app is already installed. The URL cannot be changed just by this function. Check in `app changeurl` if it's available.", "app_already_up_to_date": "{app} is already up-to-date", - "app_argument_choice_invalid": "Use one of these choices '{choices}' for the argument '{name}' instead of '{value}'", + "app_argument_choice_invalid": "Pick a valid value for argument '{name}': '{value}' is not among the available choices ({choices})", "app_argument_invalid": "Pick a valid value for the argument '{name}': {error}", "app_argument_password_no_default": "Error while parsing password argument '{name}': password argument can't have a default value for security reason", "app_argument_required": "Argument '{name}' is required", diff --git a/src/yunohost/utils/config.py b/src/yunohost/utils/config.py index e5f078267..012d2ed17 100644 --- a/src/yunohost/utils/config.py +++ b/src/yunohost/utils/config.py @@ -549,7 +549,12 @@ class Question(object): # we have an answer, do some post checks if self.value not in [None, ""]: if self.choices and self.value not in self.choices: - self._raise_invalid_answer() + raise YunohostValidationError( + "app_argument_choice_invalid", + name=self.name, + value=self.value, + choices=", ".join(self.choices), + ) if self.pattern and not re.match(self.pattern["regexp"], str(self.value)): raise YunohostValidationError( self.pattern["error"], @@ -557,14 +562,6 @@ class Question(object): value=self.value, ) - def _raise_invalid_answer(self): - raise YunohostValidationError( - "app_argument_choice_invalid", - name=self.name, - value=self.value, - choices=", ".join(self.choices), - ) - def _format_text_for_user_input_in_cli(self): text_for_user_input_in_cli = _value_for_locale(self.ask) @@ -847,13 +844,6 @@ class DomainQuestion(Question): self.choices = domain_list()["domains"] - def _raise_invalid_answer(self): - raise YunohostValidationError( - "app_argument_invalid", - name=self.name, - error=m18n.n("domain_name_unknown", domain=self.value), - ) - @staticmethod def normalize(value, option={}): if value.startswith("https://"): @@ -891,13 +881,6 @@ class UserQuestion(Question): self.default = user break - def _raise_invalid_answer(self): - raise YunohostValidationError( - "app_argument_invalid", - name=self.name, - error=m18n.n("user_unknown", user=self.value), - ) - class NumberQuestion(Question): argument_type = "number" From e001f26f874e112166481bea3eb5cd23b1c8345c Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Thu, 23 Sep 2021 19:22:54 +0200 Subject: [PATCH 18/21] Stale i18n string --- locales/en.json | 1 - 1 file changed, 1 deletion(-) diff --git a/locales/en.json b/locales/en.json index 99432492c..1d1bafd58 100644 --- a/locales/en.json +++ b/locales/en.json @@ -364,7 +364,6 @@ "extracting": "Extracting...", "field_invalid": "Invalid field '{}'", "file_does_not_exist": "The file {path} does not exist.", - "file_extension_not_accepted": "Refusing file '{path}' because its extension is not among the accepted extensions: {accept}", "firewall_reload_failed": "Could not reload the firewall", "firewall_reloaded": "Firewall reloaded", "firewall_rules_cmd_failed": "Some firewall rule commands have failed. More info in log.", From 8cc229e19b123dc31afaba4af25b11161b14db93 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Thu, 23 Sep 2021 20:04:27 +0200 Subject: [PATCH 19/21] Zblerg propagate changes on tests --- src/yunohost/app.py | 2 +- src/yunohost/tests/test_questions.py | 519 ++++++++++++++++----------- 2 files changed, 304 insertions(+), 217 deletions(-) diff --git a/src/yunohost/app.py b/src/yunohost/app.py index 01c0cb0cd..8d24e28c5 100644 --- a/src/yunohost/app.py +++ b/src/yunohost/app.py @@ -2386,7 +2386,7 @@ def _guess_webapp_path_requirement(questions: List[Question], app_folder: str) - path_questions = [question for question in questions if question.type == "path"] if len(domain_questions) == 0 and len(path_questions) == 0: - return None + return "" if len(domain_questions) == 1 and len(path_questions) == 1: return "domain_and_path" if len(domain_questions) == 1 and len(path_questions) == 0: diff --git a/src/yunohost/tests/test_questions.py b/src/yunohost/tests/test_questions.py index cf4a8832d..01b46097a 100644 --- a/src/yunohost/tests/test_questions.py +++ b/src/yunohost/tests/test_questions.py @@ -43,7 +43,7 @@ User answers: def test_question_empty(): - assert ask_questions_and_parse_answers([], {}) == {} + ask_questions_and_parse_answers([], {}) == [] def test_question_string(): @@ -54,8 +54,12 @@ def test_question_string(): } ] answers = {"some_string": "some_value"} - expected_result = OrderedDict({"some_string": ("some_value", "string")}) - assert ask_questions_and_parse_answers(questions, answers) == expected_result + + out = ask_questions_and_parse_answers(questions, answers)[0] + + assert out.name == "some_string" + assert out.type == "string" + assert out.value == "some_value" def test_question_string_default_type(): @@ -65,8 +69,13 @@ def test_question_string_default_type(): } ] answers = {"some_string": "some_value"} - expected_result = OrderedDict({"some_string": ("some_value", "string")}) - assert ask_questions_and_parse_answers(questions, answers) == expected_result + + out = ask_questions_and_parse_answers(questions, answers)[0] + + assert out.name == "some_string" + assert out.type == "string" + assert out.value == "some_value" + def test_question_string_no_input(): @@ -89,12 +98,15 @@ def test_question_string_input(): } ] answers = {} - expected_result = OrderedDict({"some_string": ("some_value", "string")}) with patch.object(Moulinette, "prompt", return_value="some_value"), patch.object( os, "isatty", return_value=True ): - assert ask_questions_and_parse_answers(questions, answers) == expected_result + out = ask_questions_and_parse_answers(questions, answers)[0] + + assert out.name == "some_string" + assert out.type == "string" + assert out.value == "some_value" def test_question_string_input_no_ask(): @@ -104,12 +116,15 @@ def test_question_string_input_no_ask(): } ] answers = {} - expected_result = OrderedDict({"some_string": ("some_value", "string")}) with patch.object(Moulinette, "prompt", return_value="some_value"), patch.object( os, "isatty", return_value=True ): - assert ask_questions_and_parse_answers(questions, answers) == expected_result + out = ask_questions_and_parse_answers(questions, answers)[0] + + assert out.name == "some_string" + assert out.type == "string" + assert out.value == "some_value" def test_question_string_no_input_optional(): @@ -120,9 +135,12 @@ def test_question_string_no_input_optional(): } ] answers = {} - expected_result = OrderedDict({"some_string": ("", "string")}) with patch.object(os, "isatty", return_value=False): - assert ask_questions_and_parse_answers(questions, answers) == expected_result + out = ask_questions_and_parse_answers(questions, answers)[0] + + assert out.name == "some_string" + assert out.type == "string" + assert out.value == "" def test_question_string_optional_with_input(): @@ -134,12 +152,15 @@ def test_question_string_optional_with_input(): } ] answers = {} - expected_result = OrderedDict({"some_string": ("some_value", "string")}) with patch.object(Moulinette, "prompt", return_value="some_value"), patch.object( os, "isatty", return_value=True ): - assert ask_questions_and_parse_answers(questions, answers) == expected_result + out = ask_questions_and_parse_answers(questions, answers)[0] + + assert out.name == "some_string" + assert out.type == "string" + assert out.value == "some_value" def test_question_string_optional_with_empty_input(): @@ -151,12 +172,15 @@ def test_question_string_optional_with_empty_input(): } ] answers = {} - expected_result = OrderedDict({"some_string": ("", "string")}) with patch.object(Moulinette, "prompt", return_value=""), patch.object( os, "isatty", return_value=True ): - assert ask_questions_and_parse_answers(questions, answers) == expected_result + out = ask_questions_and_parse_answers(questions, answers)[0] + + assert out.name == "some_string" + assert out.type == "string" + assert out.value == "" def test_question_string_optional_with_input_without_ask(): @@ -167,12 +191,15 @@ def test_question_string_optional_with_input_without_ask(): } ] answers = {} - expected_result = OrderedDict({"some_string": ("some_value", "string")}) with patch.object(Moulinette, "prompt", return_value="some_value"), patch.object( os, "isatty", return_value=True ): - assert ask_questions_and_parse_answers(questions, answers) == expected_result + out = ask_questions_and_parse_answers(questions, answers)[0] + + assert out.name == "some_string" + assert out.type == "string" + assert out.value == "some_value" def test_question_string_no_input_default(): @@ -184,9 +211,12 @@ def test_question_string_no_input_default(): } ] answers = {} - expected_result = OrderedDict({"some_string": ("some_value", "string")}) with patch.object(os, "isatty", return_value=False): - assert ask_questions_and_parse_answers(questions, answers) == expected_result + out = ask_questions_and_parse_answers(questions, answers)[0] + + assert out.name == "some_string" + assert out.type == "string" + assert out.value == "some_value" def test_question_string_input_test_ask(): @@ -286,18 +316,24 @@ def test_question_string_input_test_ask_with_help(): def test_question_string_with_choice(): questions = [{"name": "some_string", "type": "string", "choices": ["fr", "en"]}] answers = {"some_string": "fr"} - expected_result = OrderedDict({"some_string": ("fr", "string")}) - assert ask_questions_and_parse_answers(questions, answers) == expected_result + out = ask_questions_and_parse_answers(questions, answers)[0] + + assert out.name == "some_string" + assert out.type == "string" + assert out.value == "fr" def test_question_string_with_choice_prompt(): questions = [{"name": "some_string", "type": "string", "choices": ["fr", "en"]}] answers = {"some_string": "fr"} - expected_result = OrderedDict({"some_string": ("fr", "string")}) with patch.object(Moulinette, "prompt", return_value="fr"), patch.object( os, "isatty", return_value=True ): - assert ask_questions_and_parse_answers(questions, answers) == expected_result + out = ask_questions_and_parse_answers(questions, answers)[0] + + assert out.name == "some_string" + assert out.type == "string" + assert out.value == "fr" def test_question_string_with_choice_bad(): @@ -305,7 +341,7 @@ def test_question_string_with_choice_bad(): answers = {"some_string": "bad"} with pytest.raises(YunohostError), patch.object(os, "isatty", return_value=False): - assert ask_questions_and_parse_answers(questions, answers) + ask_questions_and_parse_answers(questions, answers) def test_question_string_with_choice_ask(): @@ -340,9 +376,12 @@ def test_question_string_with_choice_default(): } ] answers = {} - expected_result = OrderedDict({"some_string": ("en", "string")}) with patch.object(os, "isatty", return_value=False): - assert ask_questions_and_parse_answers(questions, answers) == expected_result + out = ask_questions_and_parse_answers(questions, answers)[0] + + assert out.name == "some_string" + assert out.type == "string" + assert out.value == "en" def test_question_password(): @@ -353,8 +392,11 @@ def test_question_password(): } ] answers = {"some_password": "some_value"} - expected_result = OrderedDict({"some_password": ("some_value", "password")}) - assert ask_questions_and_parse_answers(questions, answers) == expected_result + out = ask_questions_and_parse_answers(questions, answers)[0] + + assert out.name == "some_password" + assert out.type == "password" + assert out.value == "some_value" def test_question_password_no_input(): @@ -379,12 +421,15 @@ def test_question_password_input(): } ] answers = {} - expected_result = OrderedDict({"some_password": ("some_value", "password")}) with patch.object(Moulinette, "prompt", return_value="some_value"), patch.object( os, "isatty", return_value=True ): - assert ask_questions_and_parse_answers(questions, answers) == expected_result + out = ask_questions_and_parse_answers(questions, answers)[0] + + assert out.name == "some_password" + assert out.type == "password" + assert out.value == "some_value" def test_question_password_input_no_ask(): @@ -395,12 +440,15 @@ def test_question_password_input_no_ask(): } ] answers = {} - expected_result = OrderedDict({"some_password": ("some_value", "password")}) with patch.object(Moulinette, "prompt", return_value="some_value"), patch.object( os, "isatty", return_value=True ): - assert ask_questions_and_parse_answers(questions, answers) == expected_result + out = ask_questions_and_parse_answers(questions, answers)[0] + + assert out.name == "some_password" + assert out.type == "password" + assert out.value == "some_value" def test_question_password_no_input_optional(): @@ -412,17 +460,29 @@ def test_question_password_no_input_optional(): } ] answers = {} - expected_result = OrderedDict({"some_password": ("", "password")}) with patch.object(os, "isatty", return_value=False): - assert ask_questions_and_parse_answers(questions, answers) == expected_result + out = ask_questions_and_parse_answers(questions, answers)[0] + + assert out.name == "some_password" + assert out.type == "password" + assert out.value == "" questions = [ - {"name": "some_password", "type": "password", "optional": True, "default": ""} + { + "name": "some_password", + "type": "password", + "optional": True, + "default": "" + } ] with patch.object(os, "isatty", return_value=False): - assert ask_questions_and_parse_answers(questions, answers) == expected_result + out = ask_questions_and_parse_answers(questions, answers)[0] + + assert out.name == "some_password" + assert out.type == "password" + assert out.value == "" def test_question_password_optional_with_input(): @@ -435,12 +495,15 @@ def test_question_password_optional_with_input(): } ] answers = {} - expected_result = OrderedDict({"some_password": ("some_value", "password")}) with patch.object(Moulinette, "prompt", return_value="some_value"), patch.object( os, "isatty", return_value=True ): - assert ask_questions_and_parse_answers(questions, answers) == expected_result + out = ask_questions_and_parse_answers(questions, answers)[0] + + assert out.name == "some_password" + assert out.type == "password" + assert out.value == "some_value" def test_question_password_optional_with_empty_input(): @@ -453,12 +516,15 @@ def test_question_password_optional_with_empty_input(): } ] answers = {} - expected_result = OrderedDict({"some_password": ("", "password")}) with patch.object(Moulinette, "prompt", return_value=""), patch.object( os, "isatty", return_value=True ): - assert ask_questions_and_parse_answers(questions, answers) == expected_result + out = ask_questions_and_parse_answers(questions, answers)[0] + + assert out.name == "some_password" + assert out.type == "password" + assert out.value == "" def test_question_password_optional_with_input_without_ask(): @@ -470,12 +536,15 @@ def test_question_password_optional_with_input_without_ask(): } ] answers = {} - expected_result = OrderedDict({"some_password": ("some_value", "password")}) with patch.object(Moulinette, "prompt", return_value="some_value"), patch.object( os, "isatty", return_value=True ): - assert ask_questions_and_parse_answers(questions, answers) == expected_result + out = ask_questions_and_parse_answers(questions, answers)[0] + + assert out.name == "some_password" + assert out.type == "password" + assert out.value == "some_value" def test_question_password_no_input_default(): @@ -642,8 +711,11 @@ def test_question_path(): } ] answers = {"some_path": "/some_value"} - expected_result = OrderedDict({"some_path": ("/some_value", "path")}) - assert ask_questions_and_parse_answers(questions, answers) == expected_result + out = ask_questions_and_parse_answers(questions, answers)[0] + + assert out.name == "some_path" + assert out.type == "path" + assert out.value == "/some_value" def test_question_path_no_input(): @@ -668,12 +740,15 @@ def test_question_path_input(): } ] answers = {} - expected_result = OrderedDict({"some_path": ("/some_value", "path")}) with patch.object(Moulinette, "prompt", return_value="/some_value"), patch.object( os, "isatty", return_value=True ): - assert ask_questions_and_parse_answers(questions, answers) == expected_result + out = ask_questions_and_parse_answers(questions, answers)[0] + + assert out.name == "some_path" + assert out.type == "path" + assert out.value == "/some_value" def test_question_path_input_no_ask(): @@ -684,12 +759,15 @@ def test_question_path_input_no_ask(): } ] answers = {} - expected_result = OrderedDict({"some_path": ("/some_value", "path")}) with patch.object(Moulinette, "prompt", return_value="/some_value"), patch.object( os, "isatty", return_value=True ): - assert ask_questions_and_parse_answers(questions, answers) == expected_result + out = ask_questions_and_parse_answers(questions, answers)[0] + + assert out.name == "some_path" + assert out.type == "path" + assert out.value == "/some_value" def test_question_path_no_input_optional(): @@ -701,9 +779,12 @@ def test_question_path_no_input_optional(): } ] answers = {} - expected_result = OrderedDict({"some_path": ("", "path")}) with patch.object(os, "isatty", return_value=False): - assert ask_questions_and_parse_answers(questions, answers) == expected_result + out = ask_questions_and_parse_answers(questions, answers)[0] + + assert out.name == "some_path" + assert out.type == "path" + assert out.value == "" def test_question_path_optional_with_input(): @@ -716,12 +797,15 @@ def test_question_path_optional_with_input(): } ] answers = {} - expected_result = OrderedDict({"some_path": ("/some_value", "path")}) with patch.object(Moulinette, "prompt", return_value="/some_value"), patch.object( os, "isatty", return_value=True ): - assert ask_questions_and_parse_answers(questions, answers) == expected_result + out = ask_questions_and_parse_answers(questions, answers)[0] + + assert out.name == "some_path" + assert out.type == "path" + assert out.value == "/some_value" def test_question_path_optional_with_empty_input(): @@ -734,12 +818,15 @@ def test_question_path_optional_with_empty_input(): } ] answers = {} - expected_result = OrderedDict({"some_path": ("", "path")}) with patch.object(Moulinette, "prompt", return_value=""), patch.object( os, "isatty", return_value=True ): - assert ask_questions_and_parse_answers(questions, answers) == expected_result + out = ask_questions_and_parse_answers(questions, answers)[0] + + assert out.name == "some_path" + assert out.type == "path" + assert out.value == "" def test_question_path_optional_with_input_without_ask(): @@ -751,12 +838,15 @@ def test_question_path_optional_with_input_without_ask(): } ] answers = {} - expected_result = OrderedDict({"some_path": ("/some_value", "path")}) with patch.object(Moulinette, "prompt", return_value="/some_value"), patch.object( os, "isatty", return_value=True ): - assert ask_questions_and_parse_answers(questions, answers) == expected_result + out = ask_questions_and_parse_answers(questions, answers)[0] + + assert out.name == "some_path" + assert out.type == "path" + assert out.value == "/some_value" def test_question_path_no_input_default(): @@ -769,9 +859,12 @@ def test_question_path_no_input_default(): } ] answers = {} - expected_result = OrderedDict({"some_path": ("/some_value", "path")}) with patch.object(os, "isatty", return_value=False): - assert ask_questions_and_parse_answers(questions, answers) == expected_result + out = ask_questions_and_parse_answers(questions, answers)[0] + + assert out.name == "some_path" + assert out.type == "path" + assert out.value == "/some_value" def test_question_path_input_test_ask(): @@ -880,8 +973,11 @@ def test_question_boolean(): } ] answers = {"some_boolean": "y"} - expected_result = OrderedDict({"some_boolean": (1, "boolean")}) - assert ask_questions_and_parse_answers(questions, answers) == expected_result + out = ask_questions_and_parse_answers(questions, answers)[0] + + assert out.name == "some_boolean" + assert out.type == "boolean" + assert out.value == 1 def test_question_boolean_all_yes(): @@ -891,50 +987,12 @@ def test_question_boolean_all_yes(): "type": "boolean", } ] - expected_result = OrderedDict({"some_boolean": (1, "boolean")}) - assert ( - ask_questions_and_parse_answers(questions, {"some_boolean": "y"}) - == expected_result - ) - assert ( - ask_questions_and_parse_answers(questions, {"some_boolean": "Y"}) - == expected_result - ) - assert ( - ask_questions_and_parse_answers(questions, {"some_boolean": "yes"}) - == expected_result - ) - assert ( - ask_questions_and_parse_answers(questions, {"some_boolean": "Yes"}) - == expected_result - ) - assert ( - ask_questions_and_parse_answers(questions, {"some_boolean": "YES"}) - == expected_result - ) - assert ( - ask_questions_and_parse_answers(questions, {"some_boolean": "1"}) - == expected_result - ) - assert ( - ask_questions_and_parse_answers(questions, {"some_boolean": 1}) == expected_result - ) - assert ( - ask_questions_and_parse_answers(questions, {"some_boolean": True}) - == expected_result - ) - assert ( - ask_questions_and_parse_answers(questions, {"some_boolean": "True"}) - == expected_result - ) - assert ( - ask_questions_and_parse_answers(questions, {"some_boolean": "TRUE"}) - == expected_result - ) - assert ( - ask_questions_and_parse_answers(questions, {"some_boolean": "true"}) - == expected_result - ) + + for value in ["Y", "yes", "Yes", "YES", "1", 1, True, "True", "TRUE", "true"]: + out = ask_questions_and_parse_answers(questions, {"some_boolean": value})[0] + assert out.name == "some_boolean" + assert out.type == "boolean" + assert out.value == 1 def test_question_boolean_all_no(): @@ -944,50 +1002,12 @@ def test_question_boolean_all_no(): "type": "boolean", } ] - expected_result = OrderedDict({"some_boolean": (0, "boolean")}) - assert ( - ask_questions_and_parse_answers(questions, {"some_boolean": "n"}) - == expected_result - ) - assert ( - ask_questions_and_parse_answers(questions, {"some_boolean": "N"}) - == expected_result - ) - assert ( - ask_questions_and_parse_answers(questions, {"some_boolean": "no"}) - == expected_result - ) - assert ( - ask_questions_and_parse_answers(questions, {"some_boolean": "No"}) - == expected_result - ) - assert ( - ask_questions_and_parse_answers(questions, {"some_boolean": "No"}) - == expected_result - ) - assert ( - ask_questions_and_parse_answers(questions, {"some_boolean": "0"}) - == expected_result - ) - assert ( - ask_questions_and_parse_answers(questions, {"some_boolean": 0}) == expected_result - ) - assert ( - ask_questions_and_parse_answers(questions, {"some_boolean": False}) - == expected_result - ) - assert ( - ask_questions_and_parse_answers(questions, {"some_boolean": "False"}) - == expected_result - ) - assert ( - ask_questions_and_parse_answers(questions, {"some_boolean": "FALSE"}) - == expected_result - ) - assert ( - ask_questions_and_parse_answers(questions, {"some_boolean": "false"}) - == expected_result - ) + + for value in ["n", "N", "no", "No", "No", "0", 0, False, "False", "FALSE", "false"]: + out = ask_questions_and_parse_answers(questions, {"some_boolean": value})[0] + assert out.name == "some_boolean" + assert out.type == "boolean" + assert out.value == 0 # XXX apparently boolean are always False (0) by default, I'm not sure what to think about that @@ -1000,9 +1020,10 @@ def test_question_boolean_no_input(): ] answers = {} - expected_result = OrderedDict({"some_boolean": (0, "boolean")}) with patch.object(os, "isatty", return_value=False): - assert ask_questions_and_parse_answers(questions, answers) == expected_result + out = ask_questions_and_parse_answers(questions, answers)[0] + + assert out.value == 0 def test_question_boolean_bad_input(): @@ -1028,17 +1049,17 @@ def test_question_boolean_input(): ] answers = {} - expected_result = OrderedDict({"some_boolean": (1, "boolean")}) with patch.object(Moulinette, "prompt", return_value="y"), patch.object( os, "isatty", return_value=True ): - assert ask_questions_and_parse_answers(questions, answers) == expected_result + out = ask_questions_and_parse_answers(questions, answers)[0] + assert out.value == 1 - expected_result = OrderedDict({"some_boolean": (0, "boolean")}) with patch.object(Moulinette, "prompt", return_value="n"), patch.object( os, "isatty", return_value=True ): - assert ask_questions_and_parse_answers(questions, answers) == expected_result + out = ask_questions_and_parse_answers(questions, answers)[0] + assert out.value == 0 def test_question_boolean_input_no_ask(): @@ -1049,12 +1070,12 @@ def test_question_boolean_input_no_ask(): } ] answers = {} - expected_result = OrderedDict({"some_boolean": (1, "boolean")}) with patch.object(Moulinette, "prompt", return_value="y"), patch.object( os, "isatty", return_value=True ): - assert ask_questions_and_parse_answers(questions, answers) == expected_result + out = ask_questions_and_parse_answers(questions, answers)[0] + assert out.value == 1 def test_question_boolean_no_input_optional(): @@ -1066,9 +1087,9 @@ def test_question_boolean_no_input_optional(): } ] answers = {} - expected_result = OrderedDict({"some_boolean": (0, "boolean")}) # default to false with patch.object(os, "isatty", return_value=False): - assert ask_questions_and_parse_answers(questions, answers) == expected_result + out = ask_questions_and_parse_answers(questions, answers)[0] + assert out.value == 0 def test_question_boolean_optional_with_input(): @@ -1081,12 +1102,12 @@ def test_question_boolean_optional_with_input(): } ] answers = {} - expected_result = OrderedDict({"some_boolean": (1, "boolean")}) with patch.object(Moulinette, "prompt", return_value="y"), patch.object( os, "isatty", return_value=True ): - assert ask_questions_and_parse_answers(questions, answers) == expected_result + out = ask_questions_and_parse_answers(questions, answers)[0] + assert out.value == 1 def test_question_boolean_optional_with_empty_input(): @@ -1099,12 +1120,13 @@ def test_question_boolean_optional_with_empty_input(): } ] answers = {} - expected_result = OrderedDict({"some_boolean": (0, "boolean")}) # default to false with patch.object(Moulinette, "prompt", return_value=""), patch.object( os, "isatty", return_value=True ): - assert ask_questions_and_parse_answers(questions, answers) == expected_result + out = ask_questions_and_parse_answers(questions, answers)[0] + + assert out.value == 0 def test_question_boolean_optional_with_input_without_ask(): @@ -1116,12 +1138,13 @@ def test_question_boolean_optional_with_input_without_ask(): } ] answers = {} - expected_result = OrderedDict({"some_boolean": (0, "boolean")}) with patch.object(Moulinette, "prompt", return_value="n"), patch.object( os, "isatty", return_value=True ): - assert ask_questions_and_parse_answers(questions, answers) == expected_result + out = ask_questions_and_parse_answers(questions, answers)[0] + + assert out.value == 0 def test_question_boolean_no_input_default(): @@ -1134,9 +1157,11 @@ def test_question_boolean_no_input_default(): } ] answers = {} - expected_result = OrderedDict({"some_boolean": (0, "boolean")}) + with patch.object(os, "isatty", return_value=False): - assert ask_questions_and_parse_answers(questions, answers) == expected_result + out = ask_questions_and_parse_answers(questions, answers)[0] + + assert out.value == 0 def test_question_boolean_bad_default(): @@ -1215,7 +1240,6 @@ def test_question_domain_empty(): } ] main_domain = "my_main_domain.com" - expected_result = OrderedDict({"some_domain": (main_domain, "domain")}) answers = {} with patch.object( @@ -1225,7 +1249,11 @@ def test_question_domain_empty(): ), patch.object( os, "isatty", return_value=False ): - assert ask_questions_and_parse_answers(questions, answers) == expected_result + out = ask_questions_and_parse_answers(questions, answers)[0] + + assert out.name == "some_domain" + assert out.type == "domain" + assert out.value == main_domain def test_question_domain(): @@ -1239,12 +1267,15 @@ def test_question_domain(): ] answers = {"some_domain": main_domain} - expected_result = OrderedDict({"some_domain": (main_domain, "domain")}) with patch.object( domain, "_get_maindomain", return_value=main_domain ), patch.object(domain, "domain_list", return_value={"domains": domains}): - assert ask_questions_and_parse_answers(questions, answers) == expected_result + out = ask_questions_and_parse_answers(questions, answers)[0] + + assert out.name == "some_domain" + assert out.type == "domain" + assert out.value == main_domain def test_question_domain_two_domains(): @@ -1259,20 +1290,26 @@ def test_question_domain_two_domains(): } ] answers = {"some_domain": other_domain} - expected_result = OrderedDict({"some_domain": (other_domain, "domain")}) with patch.object( domain, "_get_maindomain", return_value=main_domain ), patch.object(domain, "domain_list", return_value={"domains": domains}): - assert ask_questions_and_parse_answers(questions, answers) == expected_result + out = ask_questions_and_parse_answers(questions, answers)[0] + + assert out.name == "some_domain" + assert out.type == "domain" + assert out.value == other_domain answers = {"some_domain": main_domain} - expected_result = OrderedDict({"some_domain": (main_domain, "domain")}) with patch.object( domain, "_get_maindomain", return_value=main_domain ), patch.object(domain, "domain_list", return_value={"domains": domains}): - assert ask_questions_and_parse_answers(questions, answers) == expected_result + out = ask_questions_and_parse_answers(questions, answers)[0] + + assert out.name == "some_domain" + assert out.type == "domain" + assert out.value == main_domain def test_question_domain_two_domains_wrong_answer(): @@ -1309,7 +1346,6 @@ def test_question_domain_two_domains_default_no_ask(): } ] answers = {} - expected_result = OrderedDict({"some_domain": (main_domain, "domain")}) with patch.object( domain, "_get_maindomain", return_value=main_domain @@ -1318,7 +1354,11 @@ def test_question_domain_two_domains_default_no_ask(): ), patch.object( os, "isatty", return_value=False ): - assert ask_questions_and_parse_answers(questions, answers) == expected_result + out = ask_questions_and_parse_answers(questions, answers)[0] + + assert out.name == "some_domain" + assert out.type == "domain" + assert out.value == main_domain def test_question_domain_two_domains_default(): @@ -1328,7 +1368,6 @@ def test_question_domain_two_domains_default(): questions = [{"name": "some_domain", "type": "domain", "ask": "choose a domain"}] answers = {} - expected_result = OrderedDict({"some_domain": (main_domain, "domain")}) with patch.object( domain, "_get_maindomain", return_value=main_domain @@ -1337,7 +1376,11 @@ def test_question_domain_two_domains_default(): ), patch.object( os, "isatty", return_value=False ): - assert ask_questions_and_parse_answers(questions, answers) == expected_result + out = ask_questions_and_parse_answers(questions, answers)[0] + + assert out.name == "some_domain" + assert out.type == "domain" + assert out.value == main_domain def test_question_domain_two_domains_default_input(): @@ -1355,13 +1398,19 @@ def test_question_domain_two_domains_default_input(): ), patch.object( os, "isatty", return_value=True ): - expected_result = OrderedDict({"some_domain": (main_domain, "domain")}) with patch.object(Moulinette, "prompt", return_value=main_domain): - assert ask_questions_and_parse_answers(questions, answers) == expected_result + out = ask_questions_and_parse_answers(questions, answers)[0] + + assert out.name == "some_domain" + assert out.type == "domain" + assert out.value == main_domain - expected_result = OrderedDict({"some_domain": (other_domain, "domain")}) with patch.object(Moulinette, "prompt", return_value=other_domain): - assert ask_questions_and_parse_answers(questions, answers) == expected_result + out = ask_questions_and_parse_answers(questions, answers)[0] + + assert out.name == "some_domain" + assert out.type == "domain" + assert out.value == other_domain def test_question_user_empty(): @@ -1410,12 +1459,14 @@ def test_question_user(): ] answers = {"some_user": username} - expected_result = OrderedDict({"some_user": (username, "user")}) - with patch.object(user, "user_list", return_value={"users": users}), patch.object( user, "user_info", return_value={} ): - assert ask_questions_and_parse_answers(questions, answers) == expected_result + out = ask_questions_and_parse_answers(questions, answers)[0] + + assert out.name == "some_user" + assert out.type == "user" + assert out.value == username def test_question_user_two_users(): @@ -1445,20 +1496,26 @@ def test_question_user_two_users(): } ] answers = {"some_user": other_user} - expected_result = OrderedDict({"some_user": (other_user, "user")}) with patch.object(user, "user_list", return_value={"users": users}), patch.object( user, "user_info", return_value={} ): - assert ask_questions_and_parse_answers(questions, answers) == expected_result + out = ask_questions_and_parse_answers(questions, answers)[0] + + assert out.name == "some_user" + assert out.type == "user" + assert out.value == other_user answers = {"some_user": username} - expected_result = OrderedDict({"some_user": (username, "user")}) with patch.object(user, "user_list", return_value={"users": users}), patch.object( user, "user_info", return_value={} ): - assert ask_questions_and_parse_answers(questions, answers) == expected_result + out = ask_questions_and_parse_answers(questions, answers)[0] + + assert out.name == "some_user" + assert out.type == "user" + assert out.value == username def test_question_user_two_users_wrong_answer(): @@ -1553,17 +1610,20 @@ def test_question_user_two_users_default_input(): os, "isatty", return_value=True ): with patch.object(user, "user_info", return_value={}): - expected_result = OrderedDict({"some_user": (username, "user")}) - with patch.object(Moulinette, "prompt", return_value=username): - assert ( - ask_questions_and_parse_answers(questions, answers) == expected_result - ) - expected_result = OrderedDict({"some_user": (other_user, "user")}) + with patch.object(Moulinette, "prompt", return_value=username): + out = ask_questions_and_parse_answers(questions, answers)[0] + + assert out.name == "some_user" + assert out.type == "user" + assert out.value == username + with patch.object(Moulinette, "prompt", return_value=other_user): - assert ( - ask_questions_and_parse_answers(questions, answers) == expected_result - ) + out = ask_questions_and_parse_answers(questions, answers)[0] + + assert out.name == "some_user" + assert out.type == "user" + assert out.value == other_user def test_question_number(): @@ -1574,8 +1634,11 @@ def test_question_number(): } ] answers = {"some_number": 1337} - expected_result = OrderedDict({"some_number": (1337, "number")}) - assert ask_questions_and_parse_answers(questions, answers) == expected_result + out = ask_questions_and_parse_answers(questions, answers)[0] + + assert out.name == "some_number" + assert out.type == "number" + assert out.value == 1337 def test_question_number_no_input(): @@ -1618,23 +1681,32 @@ def test_question_number_input(): ] answers = {} - expected_result = OrderedDict({"some_number": (1337, "number")}) with patch.object(Moulinette, "prompt", return_value="1337"), patch.object( os, "isatty", return_value=True ): - assert ask_questions_and_parse_answers(questions, answers) == expected_result + out = ask_questions_and_parse_answers(questions, answers)[0] + + assert out.name == "some_number" + assert out.type == "number" + assert out.value == 1337 with patch.object(Moulinette, "prompt", return_value=1337), patch.object( os, "isatty", return_value=True ): - assert ask_questions_and_parse_answers(questions, answers) == expected_result + out = ask_questions_and_parse_answers(questions, answers)[0] + + assert out.name == "some_number" + assert out.type == "number" + assert out.value == 1337 - expected_result = OrderedDict({"some_number": (0, "number")}) with patch.object(Moulinette, "prompt", return_value="0"), patch.object( os, "isatty", return_value=True ): - assert ask_questions_and_parse_answers(questions, answers) == expected_result + out = ask_questions_and_parse_answers(questions, answers)[0] + assert out.name == "some_number" + assert out.type == "number" + assert out.value == 0 def test_question_number_input_no_ask(): questions = [ @@ -1644,12 +1716,15 @@ def test_question_number_input_no_ask(): } ] answers = {} - expected_result = OrderedDict({"some_number": (1337, "number")}) with patch.object(Moulinette, "prompt", return_value="1337"), patch.object( os, "isatty", return_value=True ): - assert ask_questions_and_parse_answers(questions, answers) == expected_result + out = ask_questions_and_parse_answers(questions, answers)[0] + + assert out.name == "some_number" + assert out.type == "number" + assert out.value == 1337 def test_question_number_no_input_optional(): @@ -1661,9 +1736,12 @@ def test_question_number_no_input_optional(): } ] answers = {} - expected_result = OrderedDict({"some_number": (None, "number")}) # default to 0 with patch.object(os, "isatty", return_value=False): - assert ask_questions_and_parse_answers(questions, answers) == expected_result + out = ask_questions_and_parse_answers(questions, answers)[0] + + assert out.name == "some_number" + assert out.type == "number" + assert out.value == None def test_question_number_optional_with_input(): @@ -1676,12 +1754,15 @@ def test_question_number_optional_with_input(): } ] answers = {} - expected_result = OrderedDict({"some_number": (1337, "number")}) with patch.object(Moulinette, "prompt", return_value="1337"), patch.object( os, "isatty", return_value=True ): - assert ask_questions_and_parse_answers(questions, answers) == expected_result + out = ask_questions_and_parse_answers(questions, answers)[0] + + assert out.name == "some_number" + assert out.type == "number" + assert out.value == 1337 def test_question_number_optional_with_input_without_ask(): @@ -1693,12 +1774,15 @@ def test_question_number_optional_with_input_without_ask(): } ] answers = {} - expected_result = OrderedDict({"some_number": (0, "number")}) with patch.object(Moulinette, "prompt", return_value="0"), patch.object( os, "isatty", return_value=True ): - assert ask_questions_and_parse_answers(questions, answers) == expected_result + out = ask_questions_and_parse_answers(questions, answers)[0] + + assert out.name == "some_number" + assert out.type == "number" + assert out.value == 0 def test_question_number_no_input_default(): @@ -1711,9 +1795,12 @@ def test_question_number_no_input_default(): } ] answers = {} - expected_result = OrderedDict({"some_number": (1337, "number")}) with patch.object(os, "isatty", return_value=False): - assert ask_questions_and_parse_answers(questions, answers) == expected_result + out = ask_questions_and_parse_answers(questions, answers)[0] + + assert out.name == "some_number" + assert out.type == "number" + assert out.value == 1337 def test_question_number_bad_default(): @@ -1865,7 +1952,7 @@ def test_normalize_boolean_nominal(): assert BooleanQuestion.normalize(" ") is None assert BooleanQuestion.normalize(" none ") is None assert BooleanQuestion.normalize("None") is None - assert BooleanQuestion.normalize("none") is None + assert BooleanQuestion.normalize("noNe") is None assert BooleanQuestion.normalize(None) is None From 0693aa45de3aef0bf0ae8352c033d945b205de8f Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Thu, 23 Sep 2021 20:54:31 +0200 Subject: [PATCH 20/21] Add tests for FileQuestion --- src/yunohost/tests/test_questions.py | 94 +++++++++++++++++++++++++++- src/yunohost/utils/config.py | 47 +++++++------- 2 files changed, 117 insertions(+), 24 deletions(-) diff --git a/src/yunohost/tests/test_questions.py b/src/yunohost/tests/test_questions.py index 01b46097a..d141f53cc 100644 --- a/src/yunohost/tests/test_questions.py +++ b/src/yunohost/tests/test_questions.py @@ -14,7 +14,8 @@ from yunohost.utils.config import ( PasswordQuestion, DomainQuestion, PathQuestion, - BooleanQuestion + BooleanQuestion, + FileQuestion ) from yunohost.utils.error import YunohostError, YunohostValidationError @@ -62,6 +63,23 @@ def test_question_string(): assert out.value == "some_value" +def test_question_string_from_query_string(): + + questions = [ + { + "name": "some_string", + "type": "string", + } + ] + answers = "foo=bar&some_string=some_value&lorem=ipsum" + + out = ask_questions_and_parse_answers(questions, answers)[0] + + assert out.name == "some_string" + assert out.type == "string" + assert out.value == "some_value" + + def test_question_string_default_type(): questions = [ { @@ -1916,7 +1934,13 @@ def test_question_number_input_test_ask_with_help(): def test_question_display_text(): - questions = [{"name": "some_app", "type": "display_text", "ask": "foobar"}] + questions = [ + { + "name": "some_app", + "type": "display_text", + "ask": "foobar" + } + ] answers = {} with patch.object(sys, "stdout", new_callable=StringIO) as stdout, patch.object( @@ -1926,6 +1950,72 @@ def test_question_display_text(): assert "foobar" in stdout.getvalue() +def test_question_file_from_cli(): + + FileQuestion.clean_upload_dirs() + + filename = "/tmp/ynh_test_question_file" + os.system(f"rm -f {filename}") + os.system(f"echo helloworld > {filename}") + + questions = [ + { + "name": "some_file", + "type": "file", + } + ] + answers = {"some_file": filename} + + out = ask_questions_and_parse_answers(questions, answers)[0] + + assert out.name == "some_file" + assert out.type == "file" + + # The file is supposed to be copied somewhere else + assert out.value != filename + assert out.value.startswith("/tmp/") + assert os.path.exists(out.value) + assert "helloworld" in open(out.value).read().strip() + + FileQuestion.clean_upload_dirs() + + assert not os.path.exists(out.value) + + +def test_question_file_from_api(): + + FileQuestion.clean_upload_dirs() + + from base64 import b64encode + + b64content = b64encode("helloworld".encode()) + questions = [ + { + "name": "some_file", + "type": "file", + } + ] + answers = {"some_file": b64content} + + interface_type_bkp = Moulinette.interface.type + try: + Moulinette.interface.type = "api" + out = ask_questions_and_parse_answers(questions, answers)[0] + finally: + Moulinette.interface.type = interface_type_bkp + + assert out.name == "some_file" + assert out.type == "file" + + assert out.value.startswith("/tmp/") + assert os.path.exists(out.value) + assert "helloworld" in open(out.value).read().strip() + + FileQuestion.clean_upload_dirs() + + assert not os.path.exists(out.value) + + def test_normalize_boolean_nominal(): assert BooleanQuestion.normalize("yes") == 1 diff --git a/src/yunohost/utils/config.py b/src/yunohost/utils/config.py index 012d2ed17..78fb52252 100644 --- a/src/yunohost/utils/config.py +++ b/src/yunohost/utils/config.py @@ -31,6 +31,7 @@ from moulinette.interfaces.cli import colorize from moulinette import Moulinette, m18n from moulinette.utils.log import getActionLogger from moulinette.utils.filesystem import ( + read_file, write_to_file, read_toml, read_yaml, @@ -167,6 +168,9 @@ class ConfigPanel: raise finally: # Delete files uploaded from API + # FIXME : this is currently done in the context of config panels, + # but could also happen in the context of app install ... (or anywhere else + # where we may parse args etc...) FileQuestion.clean_upload_dirs() self._reload_services() @@ -969,10 +973,9 @@ class FileQuestion(Question): @classmethod def clean_upload_dirs(cls): # Delete files uploaded from API - if Moulinette.interface.type == "api": - for upload_dir in cls.upload_dirs: - if os.path.exists(upload_dir): - shutil.rmtree(upload_dir) + for upload_dir in cls.upload_dirs: + if os.path.exists(upload_dir): + shutil.rmtree(upload_dir) def __init__(self, question): super().__init__(question) @@ -984,12 +987,13 @@ class FileQuestion(Question): super()._prevalidate() - if not self.value or not os.path.exists(str(self.value)): - raise YunohostValidationError( - "app_argument_invalid", - name=self.name, - error=m18n.n("file_does_not_exist", path=str(self.value)), - ) + if Moulinette.interface.type != "api": + if not self.value or not os.path.exists(str(self.value)): + raise YunohostValidationError( + "app_argument_invalid", + name=self.name, + error=m18n.n("file_does_not_exist", path=str(self.value)), + ) def _post_parse_value(self): from base64 import b64decode @@ -997,22 +1001,21 @@ class FileQuestion(Question): if not self.value: return self.value - # FIXME : in the cli case, don't we want to also copy the file - # to a tmp work dir ? + upload_dir = tempfile.mkdtemp(prefix="ynh_filequestion_") + _, file_path = tempfile.mkstemp(dir=upload_dir) + + FileQuestion.upload_dirs += [upload_dir] + + logger.debug(f"Saving file {self.name} for file question into {file_path}") + if Moulinette.interface.type != "api": + content = read_file(str(self.value), file_mode="rb") if Moulinette.interface.type == "api": + content = b64decode(self.value) - upload_dir = tempfile.mkdtemp(prefix="tmp_configpanel_") - _, file_path = tempfile.mkstemp(prefix="foobar", dir=upload_dir) + write_to_file(file_path, content, file_mode="wb") - FileQuestion.upload_dirs += [upload_dir] - logger.debug(f"Save uploaded file from API into {file_path}") - - content = self.value - - write_to_file(file_path, b64decode(content), file_mode="wb") - - self.value = file_path + self.value = file_path return self.value From a5580caf4ec967228c2fcac6ba3fa82acd1a3c17 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Thu, 23 Sep 2021 21:46:35 +0200 Subject: [PATCH 21/21] Lint --- src/yunohost/tests/test_questions.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/yunohost/tests/test_questions.py b/src/yunohost/tests/test_questions.py index d141f53cc..99b5339ca 100644 --- a/src/yunohost/tests/test_questions.py +++ b/src/yunohost/tests/test_questions.py @@ -4,7 +4,6 @@ import os from mock import patch from io import StringIO -from collections import OrderedDict from moulinette import Moulinette @@ -1759,7 +1758,7 @@ def test_question_number_no_input_optional(): assert out.name == "some_number" assert out.type == "number" - assert out.value == None + assert out.value is None def test_question_number_optional_with_input():