From 05e1be86c6cbb896991284087daafdcb758d1075 Mon Sep 17 00:00:00 2001 From: Laurent Peuch Date: Sun, 5 Aug 2018 20:29:10 +0200 Subject: [PATCH 1/4] [mod] remove unused variable --- src/yunohost/domain.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/yunohost/domain.py b/src/yunohost/domain.py index 913b7868e..bea496d44 100644 --- a/src/yunohost/domain.py +++ b/src/yunohost/domain.py @@ -242,20 +242,17 @@ def domain_url_available(auth, domain, path): apps_map = app_map(raw=True) # Loop through all apps to check if path is taken by one of them - available = True if domain in apps_map: # Loop through apps for p, a in apps_map[domain].items(): if path == p: - available = False - break + return False # We also don't want conflicts with other apps starting with # same name elif path.startswith(p) or p.startswith(path): - available = False - break + return False - return available + return True def _get_maindomain(): From 66f92449616e8c12b05b817caa484c2e7e33f31d Mon Sep 17 00:00:00 2001 From: Laurent Peuch Date: Mon, 6 Aug 2018 01:10:14 +0200 Subject: [PATCH 2/4] [enh] display which app conflict on domain/path not available --- locales/en.json | 2 +- src/yunohost/app.py | 16 ++++++++++------ src/yunohost/domain.py | 20 ++++++++++++++++---- 3 files changed, 27 insertions(+), 11 deletions(-) diff --git a/locales/en.json b/locales/en.json index edea4eed6..b4648577e 100644 --- a/locales/en.json +++ b/locales/en.json @@ -22,7 +22,7 @@ "app_location_already_used": "The app '{app}' is already installed on that location ({path})", "app_make_default_location_already_used": "Can't make the app '{app}' the default on the domain {domain} is already used by the other app '{other_app}'", "app_location_install_failed": "Unable to install the app in this location because it conflit with the app '{other_app}' already installed on '{other_path}'", - "app_location_unavailable": "This url is not available or conflicts with an already installed app", + "app_location_unavailable": "This url is not available or conflicts with the already installed app '{app_label:s}' ({app_id:s}) on '{domain:s}{path:s}'", "app_manifest_invalid": "Invalid app manifest: {error}", "app_no_upgrade": "No app to upgrade", "app_not_correctly_installed": "{app:s} seems to be incorrectly installed", diff --git a/src/yunohost/app.py b/src/yunohost/app.py index cc37051e7..ab5809d37 100644 --- a/src/yunohost/app.py +++ b/src/yunohost/app.py @@ -1147,7 +1147,7 @@ def app_register_url(auth, app, domain, path): # This line can't be moved on top of file, otherwise it creates an infinite # loop of import with tools.py... - from domain import domain_url_available, _normalize_domain_path + from domain import _get_conflicting_app, _normalize_domain_path domain, path = _normalize_domain_path(domain, path) @@ -1163,9 +1163,11 @@ def app_register_url(auth, app, domain, path): m18n.n('app_already_installed_cant_change_url')) # Check the url is available - if not domain_url_available(auth, domain, path): + path_and_app = _get_conflicting_app(auth, domain, path) + if not path_and_app: + path, app_id, app_label = path_and_app raise MoulinetteError(errno.EINVAL, - m18n.n('app_location_unavailable')) + m18n.n('app_location_unavailable', app_id=app_id, app_label=app_label, path=path, domain=domain)) app_setting(app, 'domain', value=domain) app_setting(app, 'path', value=path) @@ -2050,7 +2052,7 @@ def _parse_action_args_in_yunohost_format(args, action_args, auth=None): """Parse arguments store in either manifest.json or actions.json """ from yunohost.domain import (domain_list, _get_maindomain, - domain_url_available, _normalize_domain_path) + _get_conflicting_app, _normalize_domain_path) from yunohost.user import user_info, user_list args_dict = OrderedDict() @@ -2178,9 +2180,11 @@ def _parse_action_args_in_yunohost_format(args, action_args, auth=None): domain, path = _normalize_domain_path(domain, path) # Check the url is available - if not domain_url_available(auth, domain, path): + path_and_app = _get_conflicting_app(auth, domain, path) + if not path_and_app: + path, app_id, app_label = path_and_app raise MoulinetteError(errno.EINVAL, - m18n.n('app_location_unavailable')) + m18n.n('app_location_unavailable', app_id=app_id, app_label=app_label, path=path, domain=domain)) # (We save this normalized path so that the install script have a # standard path format to deal with no matter what the user inputted) diff --git a/src/yunohost/domain.py b/src/yunohost/domain.py index bea496d44..9a1f0f1cf 100644 --- a/src/yunohost/domain.py +++ b/src/yunohost/domain.py @@ -219,7 +219,7 @@ def domain_cert_renew(auth, domain_list, force=False, no_checks=False, email=Fal return yunohost.certificate.certificate_renew(auth, domain_list, force, no_checks, email, staging) -def domain_url_available(auth, domain, path): +def _get_conflicting_app(auth, domain, path): """ Check availability of a web path @@ -246,13 +246,25 @@ def domain_url_available(auth, domain, path): # Loop through apps for p, a in apps_map[domain].items(): if path == p: - return False + return (p, a["id"], a["label"]) # We also don't want conflicts with other apps starting with # same name elif path.startswith(p) or p.startswith(path): - return False + return (p, a["id"], a["label"]) - return True + return None + + +def domain_url_available(auth, domain, path): + """ + Check availability of a web path + + Keyword argument: + domain -- The domain for the web path (e.g. your.domain.tld) + path -- The path to check (e.g. /coffee) + """ + + return bool(_get_conflicting_app(auth, domain, path)) def _get_maindomain(): From 73f97037568bf22b73f3ff22e0b98d683963f9c8 Mon Sep 17 00:00:00 2001 From: Laurent Peuch Date: Mon, 6 Aug 2018 01:21:38 +0200 Subject: [PATCH 3/4] [enh] returns ALL conflictings applications --- locales/en.json | 2 +- src/yunohost/app.py | 38 ++++++++++++++++++++++++++------------ src/yunohost/domain.py | 13 +++++++------ 3 files changed, 34 insertions(+), 19 deletions(-) diff --git a/locales/en.json b/locales/en.json index b4648577e..400ff5f1b 100644 --- a/locales/en.json +++ b/locales/en.json @@ -22,7 +22,7 @@ "app_location_already_used": "The app '{app}' is already installed on that location ({path})", "app_make_default_location_already_used": "Can't make the app '{app}' the default on the domain {domain} is already used by the other app '{other_app}'", "app_location_install_failed": "Unable to install the app in this location because it conflit with the app '{other_app}' already installed on '{other_path}'", - "app_location_unavailable": "This url is not available or conflicts with the already installed app '{app_label:s}' ({app_id:s}) on '{domain:s}{path:s}'", + "app_location_unavailable": "This url is not available or conflicts with the already installed app(s):\n{apps:s}", "app_manifest_invalid": "Invalid app manifest: {error}", "app_no_upgrade": "No app to upgrade", "app_not_correctly_installed": "{app:s} seems to be incorrectly installed", diff --git a/src/yunohost/app.py b/src/yunohost/app.py index ab5809d37..8604c6c41 100644 --- a/src/yunohost/app.py +++ b/src/yunohost/app.py @@ -1147,7 +1147,7 @@ def app_register_url(auth, app, domain, path): # This line can't be moved on top of file, otherwise it creates an infinite # loop of import with tools.py... - from domain import _get_conflicting_app, _normalize_domain_path + from domain import _get_conflicting_apps, _normalize_domain_path domain, path = _normalize_domain_path(domain, path) @@ -1163,11 +1163,18 @@ def app_register_url(auth, app, domain, path): m18n.n('app_already_installed_cant_change_url')) # Check the url is available - path_and_app = _get_conflicting_app(auth, domain, path) - if not path_and_app: - path, app_id, app_label = path_and_app - raise MoulinetteError(errno.EINVAL, - m18n.n('app_location_unavailable', app_id=app_id, app_label=app_label, path=path, domain=domain)) + conflicts = _get_conflicting_apps(auth, domain, path) + if conflicts: + apps = [] + for path, app_id, app_label in conflicts: + apps.append(" * {domain:s}{path:s} → {app_label:s} ({app_id:s})".format( + domain=domain, + path=path, + app_id=app_id, + app_label=app_label, + )) + + raise MoulinetteError(errno.EINVAL, m18n.n('app_location_unavailable', apps="\n".join(apps))) app_setting(app, 'domain', value=domain) app_setting(app, 'path', value=path) @@ -2052,7 +2059,7 @@ def _parse_action_args_in_yunohost_format(args, action_args, auth=None): """Parse arguments store in either manifest.json or actions.json """ from yunohost.domain import (domain_list, _get_maindomain, - _get_conflicting_app, _normalize_domain_path) + _get_conflicting_apps, _normalize_domain_path) from yunohost.user import user_info, user_list args_dict = OrderedDict() @@ -2180,11 +2187,18 @@ def _parse_action_args_in_yunohost_format(args, action_args, auth=None): domain, path = _normalize_domain_path(domain, path) # Check the url is available - path_and_app = _get_conflicting_app(auth, domain, path) - if not path_and_app: - path, app_id, app_label = path_and_app - raise MoulinetteError(errno.EINVAL, - m18n.n('app_location_unavailable', app_id=app_id, app_label=app_label, path=path, domain=domain)) + conflicts = _get_conflicting_apps(auth, domain, path) + if conflicts: + apps = [] + for path, app_id, app_label in conflicts: + apps.append(" * {domain:s}{path:s} → {app_label:s} ({app_id:s})".format( + domain=domain, + path=path, + app_id=app_id, + app_label=app_label, + )) + + raise MoulinetteError(errno.EINVAL, m18n.n('app_location_unavailable', "\n".join(apps=apps))) # (We save this normalized path so that the install script have a # standard path format to deal with no matter what the user inputted) diff --git a/src/yunohost/domain.py b/src/yunohost/domain.py index 9a1f0f1cf..8220f9022 100644 --- a/src/yunohost/domain.py +++ b/src/yunohost/domain.py @@ -219,9 +219,9 @@ def domain_cert_renew(auth, domain_list, force=False, no_checks=False, email=Fal return yunohost.certificate.certificate_renew(auth, domain_list, force, no_checks, email, staging) -def _get_conflicting_app(auth, domain, path): +def _get_conflicting_apps(auth, domain, path): """ - Check availability of a web path + Return a list of all conflicting apps with a domain/path (it can be empty) Keyword argument: domain -- The domain for the web path (e.g. your.domain.tld) @@ -242,17 +242,18 @@ def _get_conflicting_app(auth, domain, path): apps_map = app_map(raw=True) # Loop through all apps to check if path is taken by one of them + conflicts = [] if domain in apps_map: # Loop through apps for p, a in apps_map[domain].items(): if path == p: - return (p, a["id"], a["label"]) + conflicts.append((p, a["id"], a["label"])) # We also don't want conflicts with other apps starting with # same name elif path.startswith(p) or p.startswith(path): - return (p, a["id"], a["label"]) + conflicts.append((p, a["id"], a["label"])) - return None + return conflicts def domain_url_available(auth, domain, path): @@ -264,7 +265,7 @@ def domain_url_available(auth, domain, path): path -- The path to check (e.g. /coffee) """ - return bool(_get_conflicting_app(auth, domain, path)) + return bool(_get_conflicting_apps(auth, domain, path)) def _get_maindomain(): From 9a73e34c9672aaae26b2deb912c8d1d50a2481ac Mon Sep 17 00:00:00 2001 From: Laurent Peuch Date: Mon, 6 Aug 2018 01:22:43 +0200 Subject: [PATCH 4/4] [fix] test was bad, we don't want any conflicts --- src/yunohost/domain.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/yunohost/domain.py b/src/yunohost/domain.py index 8220f9022..08d74185b 100644 --- a/src/yunohost/domain.py +++ b/src/yunohost/domain.py @@ -265,7 +265,7 @@ def domain_url_available(auth, domain, path): path -- The path to check (e.g. /coffee) """ - return bool(_get_conflicting_apps(auth, domain, path)) + return len(_get_conflicting_apps(auth, domain, path)) == 0 def _get_maindomain():