Merge pull request #608 from YunoHost/fix_change_url

[fix] Fix the way change_url updates the domain/path

data/actionsmap/yunohost.yml

@@ -606,14 +606,14 @@ app:
                     full: --domain
                     help: New app domain on which the application will be moved
                     extra:
-                        ask: ask_main_domain
+                        ask: ask_new_domain
                         pattern: *pattern_domain
                         required: True
                 -p:
                     full: --path
                     help: New path at which the application will be moved
                     extra:
-                        ask: ask_path
+                        ask: ask_new_path
                         required: True
 
         ### app_setting()

locales/en.json

@@ -57,6 +57,8 @@
     "ask_list_to_remove": "List to remove",
     "ask_main_domain": "Main domain",
     "ask_new_admin_password": "New administration password",
+    "ask_new_domain": "New domain",
+    "ask_new_path": "New path",
     "ask_password": "Password",
     "ask_path": "Path",
     "backup_abstract_method": "This backup method hasn't yet been implemented",

src/yunohost/app.py

@@ -445,6 +445,7 @@ def app_change_url(operation_logger, auth, app, domain, path):
 
     """
     from yunohost.hook import hook_exec, hook_callback
+    from yunohost.domain import _normalize_domain_path, _get_conflicting_apps
 
     installed = _is_installed(app)
     if not installed:
@@ -457,18 +458,24 @@ def app_change_url(operation_logger, auth, app, domain, path):
     old_path = app_setting(app, "path")
 
     # Normalize path and domain format
-    domain = domain.strip().lower()
-
-    old_path = normalize_url_path(old_path)
-    path = normalize_url_path(path)
+    old_domain, old_path = _normalize_domain_path(old_domain, old_path)
+    domain, path = _normalize_domain_path(domain, path)
 
     if (domain, path) == (old_domain, old_path):
         raise YunohostError("app_change_url_identical_domains", domain=domain, path=path)
 
-    # WARNING / FIXME : checkurl will modify the settings
-    # (this is a non intuitive behavior that should be changed)
-    # (or checkurl renamed in reserve_url)
-    app_checkurl(auth, '%s%s' % (domain, path), app)
+    # Check the url is available
+    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 YunohostError('app_location_unavailable', apps="\n".join(apps))
 
     manifest = json.load(open(os.path.join(APPS_SETTING_PATH, app, "manifest.json")))
 
@@ -486,9 +493,9 @@ def app_change_url(operation_logger, auth, app, domain, path):
     env_dict["YNH_APP_INSTANCE_NUMBER"] = str(app_instance_nb)
 
     env_dict["YNH_APP_OLD_DOMAIN"] = old_domain
-    env_dict["YNH_APP_OLD_PATH"] = old_path.rstrip("/")
+    env_dict["YNH_APP_OLD_PATH"] = old_path
     env_dict["YNH_APP_NEW_DOMAIN"] = domain
-    env_dict["YNH_APP_NEW_PATH"] = path.rstrip("/")
+    env_dict["YNH_APP_NEW_PATH"] = path
 
     if domain != old_domain:
         operation_logger.related_to.append(('domain', old_domain))
@@ -1252,7 +1259,6 @@ def app_register_url(auth, app, domain, path):
 
     # We cannot change the url of an app already installed simply by changing
     # the settings...
-    # FIXME should look into change_url once it's merged
 
     installed = app in app_list(installed=True, raw=True).keys()
     if installed:
@@ -1290,7 +1296,7 @@ def app_checkurl(auth, url, app=None):
 
     logger.error("Packagers /!\\ : 'app checkurl' is deprecated ! Please use the helper 'ynh_webpath_register' instead !")
 
-    from yunohost.domain import domain_list
+    from yunohost.domain import domain_list, _normalize_domain_path
 
     if "https://" == url[:8]:
         url = url[8:]
@@ -1304,8 +1310,7 @@ def app_checkurl(auth, url, app=None):
     path = url[url.index('/'):]
     installed = False
 
-    if path[-1:] != '/':
-        path = path + '/'
+    domain, path = _normalize_domain_path(domain, path)
 
     apps_map = app_map(raw=True)
 
@@ -2530,13 +2535,6 @@ def random_password(length=8):
     return ''.join([random.SystemRandom().choice(char_set) for x in range(length)])
 
 
-def normalize_url_path(url_path):
-    if url_path.strip("/").strip():
-        return '/' + url_path.strip("/").strip() + '/'
-
-    return "/"
-
-
 def unstable_apps():
 
     raw_app_installed = app_list(installed=True, raw=True)

src/yunohost/domain.py

@@ -298,7 +298,7 @@ def _normalize_domain_path(domain, path):
         domain = domain[len("http://"):]
 
     # Remove trailing slashes
-    domain = domain.rstrip("/")
+    domain = domain.rstrip("/").lower()
     path = "/" + path.strip("/")
 
     return domain, path

src/yunohost/tests/test_changeurl.py

@@ -34,9 +34,9 @@ def install_changeurl_app(path):
 def check_changeurl_app(path):
     appmap = app_map(raw=True)
 
-    assert path + "/" in appmap[maindomain].keys()
+    assert path in appmap[maindomain].keys()
 
-    assert appmap[maindomain][path + "/"]["id"] == "change_url_app"
+    assert appmap[maindomain][path]["id"] == "change_url_app"
 
     r = requests.get("https://127.0.0.1%s/" % path, headers={"domain": maindomain}, verify=False)
     assert r.status_code == 200