Repository: SwissDataScienceCenter/renku-python

Issue:
create project IRI based on project slug
Currently we URL encode the project name to create the project @id, like https://renku-ci-gr-738.dev.renku.ch/projects/jakub.chrobasik/test%25202021-09-14%252015-59-30. But for downstream applications, using the project slug would be better, as it can be directly resolved in the browser, e.g. https://renku-ci-gr-738.dev.renku.ch/projects/jakub.chrobasik/test-2021-09-14-15-59-30

We should switch the project id generation to calculate and use the project slug instead. renku.service already has some code to calculate the slug, we should move that to renku.core.

Discussion:

Patch:
diff --git a/renku/core/models/git.py b/renku/core/models/git.py
index 2a44884e..84859474 100644
--- a/renku/core/models/git.py
+++ b/renku/core/models/git.py
@@ -26,7 +26,7 @@ from urllib.parse import urlparse
 import attr
 
 from renku.core import errors
-from renku.core.utils.scm import is_ascii
+from renku.core.utils.scm import is_ascii, normalize_to_ascii
 
 _RE_PROTOCOL = r"(?P<protocol>(git\+)?(https?|git|ssh|rsync))\://"
 
@@ -95,6 +95,7 @@ class GitURL(object):
     port = attr.ib(default=None)
     owner = attr.ib(default=None)
     name = attr.ib(default=None, converter=filter_repo_name)
+    slug = attr.ib(default=None)
     _regex = attr.ib(default=None, eq=False, order=False)
 
     def __attrs_post_init__(self):
@@ -107,6 +108,8 @@ class GitURL(object):
         if not self.name and self.pathname:
             self.name = filter_repo_name(Path(self.pathname).name)
 
+        self.slug = normalize_to_ascii(self.name)
+
     @classmethod
     def parse(cls, href):
         """Derive URI components."""
diff --git a/renku/core/models/project.py b/renku/core/models/project.py
index 8d789d7f..3e4deb1b 100644
--- a/renku/core/models/project.py
+++ b/renku/core/models/project.py
@@ -29,6 +29,7 @@ from renku.core.models.calamus import DateTimeList, JsonLDSchema, Nested, String
 from renku.core.models.provenance.agent import Person, PersonSchema
 from renku.core.models.provenance.annotation import Annotation, AnnotationSchema
 from renku.core.utils.datetime8601 import fix_timezone, local_now, parse_date
+from renku.core.utils.scm import normalize_to_ascii
 
 
 class Project(persistent.Persistent):
@@ -80,7 +81,12 @@ class Project(persistent.Persistent):
 
     @classmethod
     def from_client(
-        cls, client, name: str = None, description: str = None, custom_metadata: Dict = None, creator: Person = None
+        cls,
+        client,
+        name: str = None,
+        description: str = None,
+        custom_metadata: Dict = None,
+        creator: Person = None,
     ) -> "Project":
         """Create an instance from a LocalClient."""
         namespace, name = cls.get_namespace_and_name(client=client, name=name, creator=creator)
@@ -121,9 +127,9 @@ class Project(persistent.Persistent):
         assert name, "Cannot generate Project id with no name"
 
         namespace = quote(namespace.strip("/"), safe="/")
-        name = quote(name, safe="")
+        slug = normalize_to_ascii(name)
 
-        return f"/projects/{namespace}/{name}"
+        return f"/projects/{namespace}/{slug}"
 
     def update_metadata(self, custom_metadata=None, **kwargs):
         """Updates metadata."""
diff --git a/renku/service/cache/models/project.py b/renku/service/cache/models/project.py
index 7335b1e3..05f58cb6 100644
--- a/renku/service/cache/models/project.py
+++ b/renku/service/cache/models/project.py
@@ -45,6 +45,7 @@ class Project(Model):
     git_url = TextField(index=True)
 
     name = TextField()
+    slug = TextField()
     fullname = TextField()
     description = TextField()
     email = TextField()
@@ -55,7 +56,7 @@ class Project(Model):
     @property
     def abs_path(self):
         """Full path of cached project."""
-        return CACHE_PROJECTS_PATH / self.user_id / self.project_id / self.owner / self.name
+        return CACHE_PROJECTS_PATH / self.user_id / self.project_id / self.owner / self.slug
 
     def read_lock(self):
         """Shared read lock on the project."""
diff --git a/renku/service/cache/serializers/project.py b/renku/service/cache/serializers/project.py
index 031ca0d8..5ffbef54 100644
--- a/renku/service/cache/serializers/project.py
+++ b/renku/service/cache/serializers/project.py
@@ -36,6 +36,7 @@ class ProjectSchema(CreationSchema, MandatoryUserSchema):
     git_url = fields.String()
 
     name = fields.String(required=True)
+    slug = fields.String(required=True)
     description = fields.String(missing=None)
     fullname = fields.String(required=True)
     email = fields.String(required=True)
diff --git a/renku/service/controllers/templates_create_project.py b/renku/service/controllers/templates_create_project.py
index 701d92e6..6c64e37b 100644
--- a/renku/service/controllers/templates_create_project.py
+++ b/renku/service/controllers/templates_create_project.py
@@ -90,7 +90,8 @@ class TemplatesCreateProjectCtrl(ServiceCtrl, RenkuOperationMixin):
         new_project_data = {
             "clone_depth": self.ctx["depth"],
             "git_url": self.ctx["new_project_url"],
-            "name": self.ctx["project_name_stripped"],
+            "name": self.ctx["project_name"],
+            "slug": self.ctx["project_name_stripped"],
             "description": self.ctx["project_description"],
             "fullname": self.ctx["fullname"],
             "email": self.ctx["email"],
@@ -167,7 +168,8 @@ class TemplatesCreateProjectCtrl(ServiceCtrl, RenkuOperationMixin):
         return {
             "url": self.ctx["new_project_url"],
             "namespace": self.ctx["project_namespace"],
-            "name": self.ctx["project_name_stripped"],
+            "name": self.ctx["project_name"],
+            "slug": self.ctx["project_name_stripped"],
             "project_id": new_project.project_id,
         }
 
diff --git a/renku/service/serializers/cache.py b/renku/service/serializers/cache.py
index 0f17d3dd..5c87ac1f 100644
--- a/renku/service/serializers/cache.py
+++ b/renku/service/serializers/cache.py
@@ -25,6 +25,7 @@ from werkzeug.utils import secure_filename
 
 from renku.core.errors import ConfigurationError
 from renku.core.models.git import GitURL
+from renku.core.utils.scm import normalize_to_ascii
 from renku.service.config import PROJECT_CLONE_DEPTH_DEFAULT
 from renku.service.serializers.common import (
     ArchiveSchema,
@@ -103,6 +104,7 @@ class ProjectCloneContext(RepositoryCloneRequest):
 
     # user data
     name = fields.String()
+    slug = fields.String()
     fullname = fields.String()
     email = fields.String()
     owner = fields.String()
@@ -133,6 +135,7 @@ class ProjectCloneContext(RepositoryCloneRequest):
         if git_url.name is None:
             raise ValidationError("Invalid `git_url`")
         data["name"] = git_url.name
+        data["slug"] = normalize_to_ascii(data["name"])
 
         return data
 
diff --git a/renku/service/serializers/templates.py b/renku/service/serializers/templates.py
index fb18df02..45067117 100644
--- a/renku/service/serializers/templates.py
+++ b/renku/service/serializers/templates.py
@@ -136,6 +136,7 @@ class ProjectTemplateResponse(Schema):
     url = fields.String(required=True)
     namespace = fields.String(required=True)
     name = fields.String(required=True)
+    slug = fields.String(required=True)
     project_id = fields.String(required=False, default=None)
 
 
diff --git a/renku/service/utils/__init__.py b/renku/service/utils/__init__.py
index 476331bd..96c0d3f0 100644
--- a/renku/service/utils/__init__.py
+++ b/renku/service/utils/__init__.py
@@ -28,7 +28,7 @@ def make_project_path(user, project):
     valid_project = project and "owner" in project and "name" in project and "project_id" in project
 
     if valid_user and valid_project:
-        return CACHE_PROJECTS_PATH / user["user_id"] / project["project_id"] / project["owner"] / project["name"]
+        return CACHE_PROJECTS_PATH / user["user_id"] / project["project_id"] / project["owner"] / project["slug"]
 
 
 def make_file_path(user, cached_file):


Test patch:
diff --git a/tests/cli/test_init.py b/tests/cli/test_init.py
index 4ebd932e..ab6f8140 100644
--- a/tests/cli/test_init.py
+++ b/tests/cli/test_init.py
@@ -454,16 +454,18 @@ def test_default_init_parameters(isolated_runner, mocker, project_init, template
 def test_init_with_description(isolated_runner, template):
     """Test project initialization with description."""
     result = isolated_runner.invoke(
-        cli, ["init", "--description", "my project description", "new-project", "--template-id", template["id"]]
+        cli, ["init", "--description", "my project description", "new project", "--template-id", template["id"]]
     )
 
     assert 0 == result.exit_code, format_result_exception(result)
 
-    database = Database.from_path(Path("new-project") / ".renku" / "metadata")
+    database = Database.from_path(Path("new project") / ".renku" / "metadata")
     project = database.get("project")
 
+    assert "new project" == project.name
+    assert project.id.endswith("new-project")  # make sure id uses slug version of name without space
     assert "my project description" in project.template_metadata
     assert "my project description" == project.description
 
-    readme_content = (Path("new-project") / "README.md").read_text()
+    readme_content = (Path("new project") / "README.md").read_text()
     assert "my project description" in readme_content
diff --git a/tests/service/cache/test_cache.py b/tests/service/cache/test_cache.py
index 64379523..49dd849c 100644
--- a/tests/service/cache/test_cache.py
+++ b/tests/service/cache/test_cache.py
@@ -253,6 +253,7 @@ def test_service_cache_make_project(svc_client_cache):
     user = cache.ensure_user({"user_id": uuid.uuid4().hex})
     project_data = {
         "name": "renku-project-template",
+        "slug": "renku-project-template",
         "depth": 1,
         "git_url": "https://github.com/SwissDataScienceCenter/renku-project-template",
         "email": "contact@renkulab.io",
diff --git a/tests/service/controllers/test_templates_create_project.py b/tests/service/controllers/test_templates_create_project.py
index e629330f..e7417180 100644
--- a/tests/service/controllers/test_templates_create_project.py
+++ b/tests/service/controllers/test_templates_create_project.py
@@ -36,11 +36,11 @@ def test_template_create_project_ctrl(ctrl_init, svc_client_templates_creation,
 
     # Check response.
     assert {"result"} == response.json.keys()
-    assert {"project_id", "url", "namespace", "name"} == response.json["result"].keys()
+    assert {"project_id", "url", "namespace", "name", "slug"} == response.json["result"].keys()
 
     # Check ctrl_mock.
     assert ctrl_mock.call_count == 1
-    assert response.json["result"]["name"] == ctrl_mock.call_args[0][0].name
+    assert response.json["result"]["slug"] == ctrl_mock.call_args[0][0].name
 
     # Ctrl state.
     expected_context = {
@@ -55,6 +55,7 @@ def test_template_create_project_ctrl(ctrl_init, svc_client_templates_creation,
         "parameters",
         "project_name",
         "name",
+        "slug",
         "project_description",
         "new_project_url",
         "fullname",
@@ -144,8 +145,8 @@ def test_project_name_handler(project_name, expected_name, ctrl_init, svc_client
 
     # Check response.
     assert {"result"} == response.json.keys()
-    assert {"project_id", "url", "namespace", "name"} == response.json["result"].keys()
-    assert expected_name == response.json["result"]["name"]
+    assert {"project_id", "url", "namespace", "name", "slug"} == response.json["result"].keys()
+    assert expected_name == response.json["result"]["slug"]
 
 
 @pytest.mark.parametrize("project_name", ["здрасти", "---- --üäü ----", "-.-", "...", "----", "~.---", "`~~"])
diff --git a/tests/service/fixtures/service_integration.py b/tests/service/fixtures/service_integration.py
index 033e24b8..225a3aea 100644
--- a/tests/service/fixtures/service_integration.py
+++ b/tests/service/fixtures/service_integration.py
@@ -62,6 +62,7 @@ def integration_repo_path(headers, project_id, url_components):
         "project_id": project_id,
         "owner": url_components.owner,
         "name": url_components.name,
+        "slug": url_components.slug,
     }
 
     project_path = make_project_path(user, project)
@@ -219,6 +220,7 @@ def local_remote_repository(svc_client, tmp_path, mock_redis, identity_headers,
         data["owner"] = "dummy"
 
         data["name"] = "project"
+        data["slug"] = "project"
 
         return data
 
@@ -265,7 +267,7 @@ def local_remote_repository(svc_client, tmp_path, mock_redis, identity_headers,
             response = svc_client.post("/cache.project_clone", data=json.dumps(payload), headers=identity_headers)
 
             assert response
-            assert {"result"} == set(response.json.keys())
+            assert {"result"} == set(response.json.keys()), response.json
 
             project_id = response.json["result"]["project_id"]
             assert isinstance(uuid.UUID(project_id), uuid.UUID)
diff --git a/tests/service/fixtures/service_projects.py b/tests/service/fixtures/service_projects.py
index c88efcc7..c6e1c5cc 100644
--- a/tests/service/fixtures/service_projects.py
+++ b/tests/service/fixtures/service_projects.py
@@ -25,13 +25,17 @@ from urllib.parse import urlparse
 import git
 import pytest
 
+from renku.core.utils.scm import normalize_to_ascii
+
 
 @pytest.fixture
 def project_metadata(project):
     """Create project with metadata."""
+    name = Path(project).name
     metadata = {
         "project_id": uuid.uuid4().hex,
-        "name": Path(project).name,
+        "name": name,
+        "slug": normalize_to_ascii(name),
         "fullname": "full project name",
         "email": "my@email.com",
         "owner": "me",
diff --git a/tests/service/jobs/test_datasets.py b/tests/service/jobs/test_datasets.py
index 4992c2bb..f75d05fe 100644
--- a/tests/service/jobs/test_datasets.py
+++ b/tests/service/jobs/test_datasets.py
@@ -67,7 +67,13 @@ def test_dataset_url_import_job(url, svc_client_with_repo):
     assert {"job_id", "created_at"} == set(response.json["result"].keys())
 
     dest = make_project_path(
-        user_data, {"owner": url_components.owner, "name": url_components.name, "project_id": project_id}
+        user_data,
+        {
+            "owner": url_components.owner,
+            "name": url_components.name,
+            "slug": url_components.slug,
+            "project_id": project_id,
+        },
     )
 
     old_commit = Repo(dest).head.commit
@@ -109,7 +115,13 @@ def test_dataset_import_job(doi, svc_client_with_repo):
     assert {"job_id", "created_at"} == set(response.json["result"].keys())
 
     dest = make_project_path(
-        user, {"owner": url_components.owner, "name": url_components.name, "project_id": project_id}
+        user,
+        {
+            "owner": url_components.owner,
+            "name": url_components.name,
+            "slug": url_components.slug,
+            "project_id": project_id,
+        },
     )
 
     old_commit = Repo(dest).head.commit
@@ -158,7 +170,13 @@ def test_dataset_import_junk_job(doi, expected_err, svc_client_with_repo):
     assert {"job_id", "created_at"} == set(response.json["result"].keys())
 
     dest = make_project_path(
-        user, {"owner": url_components.owner, "name": url_components.name, "project_id": project_id}
+        user,
+        {
+            "owner": url_components.owner,
+            "name": url_components.name,
+            "slug": url_components.slug,
+            "project_id": project_id,
+        },
     )
 
     old_commit = Repo(dest).head.commit
@@ -201,7 +219,13 @@ def test_dataset_import_twice_job(doi, svc_client_with_repo):
     assert {"job_id", "created_at"} == set(response.json["result"].keys())
 
     dest = make_project_path(
-        user, {"owner": url_components.owner, "name": url_components.name, "project_id": project_id}
+        user,
+        {
+            "owner": url_components.owner,
+            "name": url_components.name,
+            "slug": url_components.slug,
+            "project_id": project_id,
+        },
     )
 
     old_commit = Repo(dest).head.commit
@@ -248,7 +272,13 @@ def test_dataset_add_remote_file(url, svc_client_with_repo):
     assert {"files", "name", "project_id", "remote_branch"} == set(response.json["result"].keys())
 
     dest = make_project_path(
-        user, {"owner": url_components.owner, "name": url_components.name, "project_id": project_id}
+        user,
+        {
+            "owner": url_components.owner,
+            "name": url_components.name,
+            "slug": url_components.slug,
+            "project_id": project_id,
+        },
     )
     old_commit = Repo(dest).head.commit
     job_id = response.json["result"]["files"][0]["job_id"]
@@ -363,7 +393,13 @@ def test_dataset_project_lock(doi, svc_client_with_repo):
     assert {"job_id", "created_at"} == set(response.json["result"].keys())
 
     dest = make_project_path(
-        user, {"owner": url_components.owner, "name": url_components.name, "project_id": project_id}
+        user,
+        {
+            "owner": url_components.owner,
+            "name": url_components.name,
+            "slug": url_components.slug,
+            "project_id": project_id,
+        },
     )
 
     old_commit = Repo(dest).head.commit
diff --git a/tests/service/jobs/test_jobs.py b/tests/service/jobs/test_jobs.py
index 53f76121..74e9e971 100644
--- a/tests/service/jobs/test_jobs.py
+++ b/tests/service/jobs/test_jobs.py
@@ -136,6 +136,7 @@ def test_cleanup_project_old_keys(svc_client_with_user, service_job):
     project = {
         "project_id": uuid.uuid4().hex,
         "name": "my-project",
+        "slug": "my-project",
         "fullname": "full project name",
         "email": "my@email.com",
         "owner": "me",
@@ -172,6 +173,7 @@ def test_job_constructor_lock(svc_client_with_user, service_job):
     project = {
         "project_id": uuid.uuid4().hex,
         "name": "my-project",
+        "slug": "my-project",
         "fullname": "full project name",
         "email": "my@email.com",
         "owner": "me",
diff --git a/tests/service/views/test_dataset_views.py b/tests/service/views/test_dataset_views.py
index 9f14b5e8..8eb42118 100644
--- a/tests/service/views/test_dataset_views.py
+++ b/tests/service/views/test_dataset_views.py
@@ -27,6 +27,7 @@ from pathlib import Path
 import pytest
 from werkzeug.utils import secure_filename
 
+from renku.core.utils.scm import normalize_to_ascii
 from renku.service.config import (
     GIT_ACCESS_DENIED_ERROR_CODE,
     INVALID_HEADERS_ERROR_CODE,
@@ -1126,9 +1127,12 @@ def test_cached_import_dataset_job(doi, svc_client_cache, project):
     user_id = encode_b64(secure_filename("9ab2fc80-3a5c-426d-ae78-56de01d214df"))
     user = cache.ensure_user({"user_id": user_id})
 
+    name = Path(project).name
+
     project_meta = {
         "project_id": uuid.uuid4().hex,
-        "name": Path(project).name,
+        "name": name,
+        "slug": normalize_to_ascii(name),
         "fullname": "full project name",
         "email": "my@email.com",
         "owner": "me",
diff --git a/tests/service/views/test_jobs_views.py b/tests/service/views/test_jobs_views.py
index b6623cd6..0dd82f17 100644
--- a/tests/service/views/test_jobs_views.py
+++ b/tests/service/views/test_jobs_views.py
@@ -69,7 +69,9 @@ def test_jobs_view_expected_job(svc_client_cache):
         "extras": {"progress": 42},
     }
 
-    project = Project(project_id="123", user_id=user.user_id, owner="renkumeister", name="testproject")
+    project = Project(
+        project_id="123", user_id=user.user_id, owner="renkumeister", name="testproject", slug="testproject"
+    )
     project.abs_path.mkdir(parents=True, exist_ok=True)
     project.save()
 
@@ -105,7 +107,9 @@ def test_jobs_view_check_exclusion(svc_client_cache):
             "renku_op": "dataset_import",
         }
 
-        project = Project(project_id="123", user_id=user.user_id, owner="renkumeister", name="testproject")
+        project = Project(
+            project_id="123", user_id=user.user_id, owner="renkumeister", name="testproject", slug="testproject"
+        )
         project.abs_path.mkdir(parents=True, exist_ok=True)
         project.save()
 
@@ -169,7 +173,9 @@ def test_job_details_by_user(svc_client_with_user):
         for _ in range(10)
     ]
 
-    project = Project(project_id="123", user_id=user.user_id, owner="renkumeister", name="testproject")
+    project = Project(
+        project_id="123", user_id=user.user_id, owner="renkumeister", name="testproject", slug="testproject"
+    )
     project.abs_path.mkdir(parents=True, exist_ok=True)
     project.save()
 
diff --git a/tests/service/views/test_templates_views.py b/tests/service/views/test_templates_views.py
index 95df3535..d110b997 100644
--- a/tests/service/views/test_templates_views.py
+++ b/tests/service/views/test_templates_views.py
@@ -129,7 +129,7 @@ def test_create_project_from_template(svc_client_templates_creation):
     assert response
     assert {"result"} == set(response.json.keys())
     stripped_name = normalize_to_ascii(payload["project_name"])
-    assert stripped_name == response.json["result"]["name"]
+    assert stripped_name == response.json["result"]["slug"]
     expected_url = "{0}/{1}/{2}".format(payload["project_repository"], payload["project_namespace"], stripped_name)
     assert expected_url == response.json["result"]["url"]