From af8ba89ac9c226b05660bd865b75b11ea41b2e9a Mon Sep 17 00:00:00 2001 From: Flora Thiebaut Date: Fri, 29 Nov 2024 13:31:34 +0000 Subject: [PATCH 1/7] feat: allow the project's slug to be updated --- components/renku_data_services/project/db.py | 42 +++++++++++++++++-- .../data_api/test_projects.py | 42 +++++++++++++++++++ 2 files changed, 81 insertions(+), 3 deletions(-) diff --git a/components/renku_data_services/project/db.py b/components/renku_data_services/project/db.py index e9550c4b6..28b0c6da4 100644 --- a/components/renku_data_services/project/db.py +++ b/components/renku_data_services/project/db.py @@ -5,7 +5,7 @@ import functools from collections.abc import AsyncGenerator, Awaitable, Callable from datetime import UTC, datetime -from typing import Concatenate, ParamSpec, TypeVar +from typing import Concatenate, ParamSpec, TypeVar, cast from sqlalchemy import Select, delete, func, select, update from sqlalchemy.ext.asyncio import AsyncSession @@ -147,8 +147,44 @@ async def get_project_by_namespace_slug( stmt = stmt.where(schemas.ProjectORM.slug.has(ns_schemas.EntitySlugORM.slug == slug)) if with_documentation: stmt = stmt.options(undefer(schemas.ProjectORM.documentation)) - result = await session.execute(stmt) - project_orm = result.scalars().first() + result = await session.scalars(stmt) + project_orm = result.first() + + if project_orm is None: + old_project_stmt_old_ns_current_slug = ( + select(schemas.ProjectORM) + .where(ns_schemas.NamespaceOldORM.slug == namespace.lower()) + .where(ns_schemas.NamespaceOldORM.latest_slug_id == ns_schemas.NamespaceORM.id) + .where(ns_schemas.EntitySlugORM.namespace_id == ns_schemas.NamespaceORM.id) + .where(schemas.ProjectORM.id == ns_schemas.EntitySlugORM.project_id) + .where(schemas.ProjectORM.slug.has(ns_schemas.EntitySlugORM.slug == slug)) + ) + old_project_stmt_current_ns_old_slug = ( + select(schemas.ProjectORM) + .where(ns_schemas.NamespaceORM.slug == namespace.lower()) + .where(ns_schemas.EntitySlugORM.namespace_id == ns_schemas.NamespaceORM.id) + .where(schemas.ProjectORM.id == ns_schemas.EntitySlugORM.project_id) + .where(ns_schemas.EntitySlugOldORM.slug == slug) + .where(ns_schemas.EntitySlugOldORM.latest_slug_id == ns_schemas.EntitySlugORM.id) + ) + old_project_stmt_old_ns_old_slug = ( + select(schemas.ProjectORM) + .where(ns_schemas.NamespaceOldORM.slug == namespace.lower()) + .where(ns_schemas.NamespaceOldORM.latest_slug_id == ns_schemas.NamespaceORM.id) + .where(ns_schemas.EntitySlugORM.namespace_id == ns_schemas.NamespaceORM.id) + .where(schemas.ProjectORM.id == ns_schemas.EntitySlugORM.project_id) + .where(ns_schemas.EntitySlugOldORM.slug == slug) + .where(ns_schemas.EntitySlugOldORM.latest_slug_id == ns_schemas.EntitySlugORM.id) + ) + old_project_stmt = old_project_stmt_old_ns_current_slug.union( + old_project_stmt_current_ns_old_slug, old_project_stmt_old_ns_old_slug + ) + result_old = cast(schemas.ProjectORM | None, await session.scalar(old_project_stmt)) + if result_old is not None: + stmt = select(schemas.ProjectORM) + if with_documentation: + stmt = stmt.options(undefer(schemas.ProjectORM.documentation)) + project_orm = await session.scalar(stmt) not_found_msg = ( f"Project with identifier '{namespace}/{slug}' does not exist or you do not have access to it." diff --git a/test/bases/renku_data_services/data_api/test_projects.py b/test/bases/renku_data_services/data_api/test_projects.py index 73452fefb..174cc406d 100644 --- a/test/bases/renku_data_services/data_api/test_projects.py +++ b/test/bases/renku_data_services/data_api/test_projects.py @@ -1426,3 +1426,45 @@ async def test_project_unlink_from_template_project( project = await get_project(project_id) assert "template_id" not in project or project["template_id"] is None + + +@pytest.mark.asyncio +async def test_get_project_after_group_moved( + create_project, + create_group, + sanic_client, + user_headers, +) -> None: + group = await create_group("test-group") + group_slug = group["slug"] + project = await create_project("My project", namespace=group_slug) + project_id = project["id"] + + new_group_slug = "test-group-updated" + patch = {"slug": new_group_slug} + _, response = await sanic_client.patch(f"/api/data/groups/{group_slug}", headers=user_headers, json=patch) + assert response.status_code == 200, response.text + + # Check that the project's namespace has been updated + _, response = await sanic_client.get(f"/api/data/projects/{project_id}", headers=user_headers) + assert response.status_code == 200, response.text + assert response.json is not None + assert response.json.get("id") == project_id + assert response.json.get("namespace") == new_group_slug + assert response.json.get("slug") == "my-project" + + # Check that we can get the project with the new namespace + _, response = await sanic_client.get( + f"/api/data/namespaces/{new_group_slug}/projects/{project['slug']}", headers=user_headers + ) + assert response.status_code == 200, response.text + assert response.json is not None + assert response.json.get("id") == project_id + + # Check that we can get the project with the old namespace + _, response = await sanic_client.get( + f"/api/data/namespaces/{group_slug}/projects/{project['slug']}", headers=user_headers + ) + assert response.status_code == 200, response.text + assert response.json is not None + assert response.json.get("id") == project_id From 9facc288feab907edcd635f4f5680d39b83d0a02 Mon Sep 17 00:00:00 2001 From: Flora Thiebaut Date: Fri, 29 Nov 2024 13:44:07 +0000 Subject: [PATCH 2/7] fix --- components/renku_data_services/project/db.py | 15 ++++++++------- .../renku_data_services/data_api/test_projects.py | 5 +++++ 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/components/renku_data_services/project/db.py b/components/renku_data_services/project/db.py index 28b0c6da4..3071c67ed 100644 --- a/components/renku_data_services/project/db.py +++ b/components/renku_data_services/project/db.py @@ -152,7 +152,7 @@ async def get_project_by_namespace_slug( if project_orm is None: old_project_stmt_old_ns_current_slug = ( - select(schemas.ProjectORM) + select(schemas.ProjectORM.id) .where(ns_schemas.NamespaceOldORM.slug == namespace.lower()) .where(ns_schemas.NamespaceOldORM.latest_slug_id == ns_schemas.NamespaceORM.id) .where(ns_schemas.EntitySlugORM.namespace_id == ns_schemas.NamespaceORM.id) @@ -160,7 +160,7 @@ async def get_project_by_namespace_slug( .where(schemas.ProjectORM.slug.has(ns_schemas.EntitySlugORM.slug == slug)) ) old_project_stmt_current_ns_old_slug = ( - select(schemas.ProjectORM) + select(schemas.ProjectORM.id) .where(ns_schemas.NamespaceORM.slug == namespace.lower()) .where(ns_schemas.EntitySlugORM.namespace_id == ns_schemas.NamespaceORM.id) .where(schemas.ProjectORM.id == ns_schemas.EntitySlugORM.project_id) @@ -168,7 +168,7 @@ async def get_project_by_namespace_slug( .where(ns_schemas.EntitySlugOldORM.latest_slug_id == ns_schemas.EntitySlugORM.id) ) old_project_stmt_old_ns_old_slug = ( - select(schemas.ProjectORM) + select(schemas.ProjectORM.id) .where(ns_schemas.NamespaceOldORM.slug == namespace.lower()) .where(ns_schemas.NamespaceOldORM.latest_slug_id == ns_schemas.NamespaceORM.id) .where(ns_schemas.EntitySlugORM.namespace_id == ns_schemas.NamespaceORM.id) @@ -179,12 +179,13 @@ async def get_project_by_namespace_slug( old_project_stmt = old_project_stmt_old_ns_current_slug.union( old_project_stmt_current_ns_old_slug, old_project_stmt_old_ns_old_slug ) - result_old = cast(schemas.ProjectORM | None, await session.scalar(old_project_stmt)) - if result_old is not None: - stmt = select(schemas.ProjectORM) + result_old = await session.scalars(old_project_stmt) + result_old_id = cast(ULID | None, result_old.first()) + if result_old_id is not None: + stmt = select(schemas.ProjectORM).where(schemas.ProjectORM.id == result_old_id) if with_documentation: stmt = stmt.options(undefer(schemas.ProjectORM.documentation)) - project_orm = await session.scalar(stmt) + project_orm = (await session.scalars(stmt)).first() not_found_msg = ( f"Project with identifier '{namespace}/{slug}' does not exist or you do not have access to it." diff --git a/test/bases/renku_data_services/data_api/test_projects.py b/test/bases/renku_data_services/data_api/test_projects.py index 174cc406d..d60a70743 100644 --- a/test/bases/renku_data_services/data_api/test_projects.py +++ b/test/bases/renku_data_services/data_api/test_projects.py @@ -1435,10 +1435,15 @@ async def test_get_project_after_group_moved( sanic_client, user_headers, ) -> None: + await create_project( + "Project 1", + ) + await create_project("Project 2") group = await create_group("test-group") group_slug = group["slug"] project = await create_project("My project", namespace=group_slug) project_id = project["id"] + await create_project("Project 3") new_group_slug = "test-group-updated" patch = {"slug": new_group_slug} From 5812c1ebf3fcae58631a40e9cace8ec281965826 Mon Sep 17 00:00:00 2001 From: Flora Thiebaut Date: Fri, 29 Nov 2024 13:59:42 +0000 Subject: [PATCH 3/7] wip: allow slug update --- .../renku_data_services/project/core.py | 1 + components/renku_data_services/project/db.py | 19 ++++++++++++++++++- .../renku_data_services/project/models.py | 1 + 3 files changed, 20 insertions(+), 1 deletion(-) diff --git a/components/renku_data_services/project/core.py b/components/renku_data_services/project/core.py index dc63bd0e0..4b24752ef 100644 --- a/components/renku_data_services/project/core.py +++ b/components/renku_data_services/project/core.py @@ -17,6 +17,7 @@ def validate_project_patch(patch: apispec.ProjectPatch) -> models.ProjectPatch: return models.ProjectPatch( name=patch.name, namespace=patch.namespace, + slug=None, # patch.slug, visibility=Visibility(patch.visibility.value) if patch.visibility is not None else None, repositories=patch.repositories, description=patch.description, diff --git a/components/renku_data_services/project/db.py b/components/renku_data_services/project/db.py index 3071c67ed..ded209646 100644 --- a/components/renku_data_services/project/db.py +++ b/components/renku_data_services/project/db.py @@ -303,13 +303,16 @@ async def update_project( if patch.namespace is not None and patch.namespace != old_project.namespace.slug: # NOTE: changing the namespace requires the user to be owner which means they should have DELETE permission required_scope = Scope.DELETE + if patch.slug is not None and patch.slug != old_project.slug: + # NOTE: changing the slug requires the user to be owner which means they should have DELETE permission + required_scope = Scope.DELETE authorized = await self.authz.has_permission(user, ResourceType.project, project_id, required_scope) if not authorized: raise errors.MissingResourceError( message=f"Project with id '{project_id}' does not exist or you do not have access to it." ) - current_etag = project.dump().etag + current_etag = old_project.etag if etag is not None and current_etag != etag: raise errors.ConflictError(message=f"Current ETag is {current_etag}, not {etag}.") @@ -332,6 +335,20 @@ async def update_project( message=f"The project cannot be moved because you do not have sufficient permissions with the namespace {patch.namespace}" # noqa: E501 ) project.slug.namespace_id = ns.id + if patch.slug is not None and patch.slug != old_project.slug: + namespace_id = project.slug.namespace_id + existing_entity = await session.scalar( + select(ns_schemas.EntitySlugORM) + .where(ns_schemas.EntitySlugORM.slug == patch.slug) + .where(ns_schemas.EntitySlugORM.namespace_id == namespace_id) + ) + if existing_entity is not None: + raise errors.ConflictError( + message=f"An entity with the slug '{project.slug.namespace.slug}/{patch.slug}' already exists." + ) + # project + session.add(ns_schemas.EntitySlugOldORM(slug=old_project.slug, latest_slug=project.slug)) + project.slug.slug = patch.slug if patch.visibility is not None: visibility_orm = ( project_apispec.Visibility(patch.visibility) diff --git a/components/renku_data_services/project/models.py b/components/renku_data_services/project/models.py index c847ab639..2e7b93713 100644 --- a/components/renku_data_services/project/models.py +++ b/components/renku_data_services/project/models.py @@ -59,6 +59,7 @@ class ProjectPatch: name: str | None namespace: str | None + slug: str | None visibility: Visibility | None repositories: list[Repository] | None description: str | None From d3d00db22ff561151f86ababacc7b65d9fd972ab Mon Sep 17 00:00:00 2001 From: Flora Thiebaut Date: Fri, 29 Nov 2024 14:33:41 +0000 Subject: [PATCH 4/7] working slug patch? --- .../renku_data_services/namespace/orm.py | 3 +- .../renku_data_services/project/api.spec.yaml | 2 + .../renku_data_services/project/apispec.py | 10 ++- .../renku_data_services/project/core.py | 2 +- components/renku_data_services/project/db.py | 2 +- .../data_api/test_projects.py | 67 +++++++++++++++++-- 6 files changed, 77 insertions(+), 9 deletions(-) diff --git a/components/renku_data_services/namespace/orm.py b/components/renku_data_services/namespace/orm.py index 625f30774..aabdc4958 100644 --- a/components/renku_data_services/namespace/orm.py +++ b/components/renku_data_services/namespace/orm.py @@ -276,7 +276,6 @@ class EntitySlugOldORM(BaseORM): latest_slug_id: Mapped[int] = mapped_column( ForeignKey(EntitySlugORM.id, ondelete="CASCADE"), nullable=False, - init=False, index=True, ) - latest_slug: Mapped[EntitySlugORM] = relationship(lazy="joined", repr=False, viewonly=True) + latest_slug: Mapped[EntitySlugORM] = relationship(lazy="joined", init=False, repr=False, viewonly=True) diff --git a/components/renku_data_services/project/api.spec.yaml b/components/renku_data_services/project/api.spec.yaml index 31b4b8082..3a7577b33 100644 --- a/components/renku_data_services/project/api.spec.yaml +++ b/components/renku_data_services/project/api.spec.yaml @@ -487,6 +487,8 @@ components: $ref: "#/components/schemas/ProjectName" namespace: $ref: "#/components/schemas/Slug" + slug: + $ref: "#/components/schemas/Slug" repositories: $ref: "#/components/schemas/RepositoriesList" visibility: diff --git a/components/renku_data_services/project/apispec.py b/components/renku_data_services/project/apispec.py index a519c4f6e..3e34c550b 100644 --- a/components/renku_data_services/project/apispec.py +++ b/components/renku_data_services/project/apispec.py @@ -1,6 +1,6 @@ # generated by datamodel-codegen: # filename: api.spec.yaml -# timestamp: 2024-11-25T14:22:47+00:00 +# timestamp: 2024-11-29T14:11:33+00:00 from __future__ import annotations @@ -345,6 +345,14 @@ class ProjectPatch(BaseAPISpec): min_length=1, pattern="^(?!.*\\.git$|.*\\.atom$|.*[\\-._][\\-._].*)[a-z0-9][a-z0-9\\-_.]*$", ) + slug: Optional[str] = Field( + None, + description="A command-line/url friendly name for a namespace", + example="a-slug-example", + max_length=99, + min_length=1, + pattern="^(?!.*\\.git$|.*\\.atom$|.*[\\-._][\\-._].*)[a-z0-9][a-z0-9\\-_.]*$", + ) repositories: Optional[List[str]] = Field( None, description="A list of repositories", diff --git a/components/renku_data_services/project/core.py b/components/renku_data_services/project/core.py index 4b24752ef..496070167 100644 --- a/components/renku_data_services/project/core.py +++ b/components/renku_data_services/project/core.py @@ -17,7 +17,7 @@ def validate_project_patch(patch: apispec.ProjectPatch) -> models.ProjectPatch: return models.ProjectPatch( name=patch.name, namespace=patch.namespace, - slug=None, # patch.slug, + slug=patch.slug, visibility=Visibility(patch.visibility.value) if patch.visibility is not None else None, repositories=patch.repositories, description=patch.description, diff --git a/components/renku_data_services/project/db.py b/components/renku_data_services/project/db.py index ded209646..06afcc0e8 100644 --- a/components/renku_data_services/project/db.py +++ b/components/renku_data_services/project/db.py @@ -347,7 +347,7 @@ async def update_project( message=f"An entity with the slug '{project.slug.namespace.slug}/{patch.slug}' already exists." ) # project - session.add(ns_schemas.EntitySlugOldORM(slug=old_project.slug, latest_slug=project.slug)) + session.add(ns_schemas.EntitySlugOldORM(slug=old_project.slug, latest_slug_id=project.slug.id)) project.slug.slug = patch.slug if patch.visibility is not None: visibility_orm = ( diff --git a/test/bases/renku_data_services/data_api/test_projects.py b/test/bases/renku_data_services/data_api/test_projects.py index d60a70743..bf4f25804 100644 --- a/test/bases/renku_data_services/data_api/test_projects.py +++ b/test/bases/renku_data_services/data_api/test_projects.py @@ -603,6 +603,58 @@ async def test_patch_description_as_editor_and_keep_namespace_and_visibility( assert response.json.get("description") == "Updated description" +@pytest.mark.asyncio +async def test_patch_project_slug( + sanic_client, + create_project, + get_project, + user_headers, +) -> None: + await create_project("Project 1") + await create_project("Project 2") + project = await create_project("My project", documentation="Hello, World!") + project_id = project["id"] + namespace = project["namespace"] + old_slug = project["slug"] + await create_project("Project 3") + + # Patch a project + headers = merge_headers(user_headers, {"If-Match": project["etag"]}) + new_slug = "some-updated-slug" + patch = {"slug": new_slug} + _, response = await sanic_client.patch(f"/api/data/projects/{project_id}", headers=headers, json=patch) + + assert response.status_code == 200, response.text + + # Check that the project's slug has been updated + project = await get_project(project_id=project_id) + assert project["id"] == project_id + assert project["name"] == "My project" + assert project["namespace"] == namespace + assert project["slug"] == new_slug + + # Check that we can get the project with the new slug + _, response = await sanic_client.get(f"/api/data/namespaces/{namespace}/projects/{new_slug}", headers=user_headers) + assert response.status_code == 200, response.text + assert response.json is not None + assert response.json.get("id") == project_id + + # Check that we can get the project with the old slug + _, response = await sanic_client.get(f"/api/data/namespaces/{namespace}/projects/{old_slug}", headers=user_headers) + assert response.status_code == 200, response.text + assert response.json is not None + assert response.json.get("id") == project_id + _, response = await sanic_client.get( + f"/api/data/namespaces/{namespace}/projects/{old_slug}", + params={"with_documentation": True}, + headers=user_headers, + ) + assert response.status_code == 200, response.text + assert response.json is not None + assert response.json.get("id") == project_id + assert response.json.get("documentation") == "Hello, World!" + + @pytest.mark.asyncio async def test_get_all_projects_for_specific_user( create_project, sanic_client, user_headers, admin_headers, unauthorized_headers @@ -1435,13 +1487,11 @@ async def test_get_project_after_group_moved( sanic_client, user_headers, ) -> None: - await create_project( - "Project 1", - ) + await create_project("Project 1") await create_project("Project 2") group = await create_group("test-group") group_slug = group["slug"] - project = await create_project("My project", namespace=group_slug) + project = await create_project("My project", namespace=group_slug, documentation="Hello, World!") project_id = project["id"] await create_project("Project 3") @@ -1473,3 +1523,12 @@ async def test_get_project_after_group_moved( assert response.status_code == 200, response.text assert response.json is not None assert response.json.get("id") == project_id + _, response = await sanic_client.get( + f"/api/data/namespaces/{group_slug}/projects/{project['slug']}", + params={"with_documentation": True}, + headers=user_headers, + ) + assert response.status_code == 200, response.text + assert response.json is not None + assert response.json.get("id") == project_id + assert response.json.get("documentation") == "Hello, World!" From 42540d1a723abd54b0efbf2559a051d45af174dd Mon Sep 17 00:00:00 2001 From: Flora Thiebaut Date: Tue, 3 Dec 2024 10:35:07 +0000 Subject: [PATCH 5/7] fix test --- test/bases/renku_data_services/data_api/test_projects.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/bases/renku_data_services/data_api/test_projects.py b/test/bases/renku_data_services/data_api/test_projects.py index bf4f25804..14319425f 100644 --- a/test/bases/renku_data_services/data_api/test_projects.py +++ b/test/bases/renku_data_services/data_api/test_projects.py @@ -512,7 +512,7 @@ async def test_patch_visibility_to_public_shows_project( @pytest.mark.asyncio -@pytest.mark.parametrize("field", ["id", "slug", "created_by", "creation_date"]) +@pytest.mark.parametrize("field", ["id", "created_by", "creation_date"]) async def test_cannot_patch_reserved_fields(create_project, get_project, sanic_client, user_headers, field) -> None: project = await create_project("Project 1") original_value = project[field] From 86898fd7b56a1bbdc29e69a0825d4ccd5fd14a63 Mon Sep 17 00:00:00 2001 From: Flora Thiebaut Date: Tue, 3 Dec 2024 10:44:07 +0000 Subject: [PATCH 6/7] wip: allow slug update on data connectors --- .../renku_data_services/data_connectors/db.py | 61 +++++++++++++++++-- components/renku_data_services/project/db.py | 1 - 2 files changed, 57 insertions(+), 5 deletions(-) diff --git a/components/renku_data_services/data_connectors/db.py b/components/renku_data_services/data_connectors/db.py index 6bda29641..cf77f865f 100644 --- a/components/renku_data_services/data_connectors/db.py +++ b/components/renku_data_services/data_connectors/db.py @@ -1,7 +1,7 @@ """Adapters for data connectors database classes.""" from collections.abc import AsyncIterator, Callable -from typing import TypeVar +from typing import TypeVar, cast from cryptography.hazmat.primitives.asymmetric import rsa from sqlalchemy import Select, delete, func, select @@ -94,6 +94,42 @@ async def get_data_connector_by_slug( stmt = stmt.where(ns_schemas.EntitySlugORM.slug == slug.lower()) result = await session.scalars(stmt) data_connector = result.one_or_none() + + if data_connector is None: + old_data_connector_stmt_old_ns_current_slug = ( + select(schemas.DataConnectorORM.id) + .where(ns_schemas.NamespaceOldORM.slug == namespace.lower()) + .where(ns_schemas.NamespaceOldORM.latest_slug_id == ns_schemas.NamespaceORM.id) + .where(ns_schemas.EntitySlugORM.namespace_id == ns_schemas.NamespaceORM.id) + .where(schemas.DataConnectorORM.id == ns_schemas.EntitySlugORM.data_connector_id) + .where(schemas.DataConnectorORM.slug.has(ns_schemas.EntitySlugORM.slug == slug.lower())) + ) + old_data_connector_stmt_current_ns_old_slug = ( + select(schemas.DataConnectorORM.id) + .where(ns_schemas.NamespaceORM.slug == namespace.lower()) + .where(ns_schemas.EntitySlugORM.namespace_id == ns_schemas.NamespaceORM.id) + .where(schemas.DataConnectorORM.id == ns_schemas.EntitySlugORM.data_connector_id) + .where(ns_schemas.EntitySlugOldORM.slug == slug.lower()) + .where(ns_schemas.EntitySlugOldORM.latest_slug_id == ns_schemas.EntitySlugORM.id) + ) + old_data_connector_stmt_old_ns_old_slug = ( + select(schemas.DataConnectorORM.id) + .where(ns_schemas.NamespaceOldORM.slug == namespace.lower()) + .where(ns_schemas.NamespaceOldORM.latest_slug_id == ns_schemas.NamespaceORM.id) + .where(ns_schemas.EntitySlugORM.namespace_id == ns_schemas.NamespaceORM.id) + .where(schemas.DataConnectorORM.id == ns_schemas.EntitySlugORM.data_connector_id) + .where(ns_schemas.EntitySlugOldORM.slug == slug.lower()) + .where(ns_schemas.EntitySlugOldORM.latest_slug_id == ns_schemas.EntitySlugORM.id) + ) + old_data_connector_stmt = old_data_connector_stmt_old_ns_current_slug.union( + old_data_connector_stmt_current_ns_old_slug, old_data_connector_stmt_old_ns_old_slug + ) + result_old = await session.scalars(old_data_connector_stmt) + result_old_id = cast(ULID | None, result_old.first()) + if result_old_id is not None: + stmt = select(schemas.DataConnectorORM).where(schemas.DataConnectorORM.id == result_old_id) + data_connector = (await session.scalars(stmt)).first() + if data_connector is None: raise errors.MissingResourceError(message=not_found_msg) @@ -211,17 +247,19 @@ async def update_data_connector( if patch.namespace is not None and patch.namespace != old_data_connector.namespace.slug: # NOTE: changing the namespace requires the user to be owner which means they should have DELETE permission # noqa E501 required_scope = Scope.DELETE + if patch.slug is not None and patch.slug != old_data_connector.slug: + # NOTE: changing the slug requires the user to be owner which means they should have DELETE permission + required_scope = Scope.DELETE authorized = await self.authz.has_permission( user, ResourceType.data_connector, data_connector_id, required_scope ) if not authorized: raise errors.MissingResourceError(message=not_found_msg) - current_etag = data_connector.dump().etag + current_etag = old_data_connector.etag if current_etag != etag: raise errors.ConflictError(message=f"Current ETag is {current_etag}, not {etag}.") - # TODO: handle slug update if patch.name is not None: data_connector.name = patch.name if patch.visibility is not None: @@ -231,7 +269,7 @@ async def update_data_connector( else apispec.Visibility(patch.visibility.value) ) data_connector.visibility = visibility_orm - if patch.namespace is not None: + if patch.namespace is not None and patch.namespace != old_data_connector.namespace.slug: ns = await session.scalar( select(ns_schemas.NamespaceORM).where(ns_schemas.NamespaceORM.slug == patch.namespace.lower()) ) @@ -248,6 +286,21 @@ async def update_data_connector( message=f"The data connector cannot be moved because you do not have sufficient permissions with the namespace {patch.namespace}." # noqa: E501 ) data_connector.slug.namespace_id = ns.id + if patch.slug is not None and patch.slug != old_data_connector.slug: + namespace_id = data_connector.slug.namespace_id + existing_entity = await session.scalar( + select(ns_schemas.EntitySlugORM) + .where(ns_schemas.EntitySlugORM.slug == patch.slug) + .where(ns_schemas.EntitySlugORM.namespace_id == namespace_id) + ) + if existing_entity is not None: + raise errors.ConflictError( + message=f"An entity with the slug '{data_connector.slug.namespace.slug}/{patch.slug}' already exists." # noqa E501 + ) + session.add( + ns_schemas.EntitySlugOldORM(slug=old_data_connector.slug, latest_slug_id=data_connector.slug.id) + ) + data_connector.slug.slug = patch.slug if patch.description is not None: data_connector.description = patch.description if patch.description else None if patch.keywords is not None: diff --git a/components/renku_data_services/project/db.py b/components/renku_data_services/project/db.py index 06afcc0e8..cb069bb3f 100644 --- a/components/renku_data_services/project/db.py +++ b/components/renku_data_services/project/db.py @@ -346,7 +346,6 @@ async def update_project( raise errors.ConflictError( message=f"An entity with the slug '{project.slug.namespace.slug}/{patch.slug}' already exists." ) - # project session.add(ns_schemas.EntitySlugOldORM(slug=old_project.slug, latest_slug_id=project.slug.id)) project.slug.slug = patch.slug if patch.visibility is not None: From 54755fb21472898cb4ab6a0ae4f9f56ea765001b Mon Sep 17 00:00:00 2001 From: Flora Thiebaut Date: Tue, 3 Dec 2024 11:02:09 +0000 Subject: [PATCH 7/7] add test --- .../data_api/test_data_connectors.py | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/test/bases/renku_data_services/data_api/test_data_connectors.py b/test/bases/renku_data_services/data_api/test_data_connectors.py index 457fa472d..4545a4a9b 100644 --- a/test/bases/renku_data_services/data_api/test_data_connectors.py +++ b/test/bases/renku_data_services/data_api/test_data_connectors.py @@ -639,6 +639,56 @@ async def test_patch_data_connector_as_editor( assert response.json.get("description") == "A new description" +@pytest.mark.asyncio +async def test_patch_data_connector_slug( + sanic_client: SanicASGITestClient, + create_data_connector, + user_headers, +) -> None: + await create_data_connector("Data connector 1") + await create_data_connector("Data connector 2") + data_connector = await create_data_connector("My data connector") + data_connector_id = data_connector["id"] + namespace = data_connector["namespace"] + old_slug = data_connector["slug"] + await create_data_connector("Data connector 3") + + # Patch a data connector + headers = merge_headers(user_headers, {"If-Match": data_connector["etag"]}) + new_slug = "some-updated-slug" + patch = {"slug": new_slug} + _, response = await sanic_client.patch( + f"/api/data/data_connectors/{data_connector_id}", headers=headers, json=patch + ) + + assert response.status_code == 200, response.text + + # Check that the data connector's slug has been updated + _, response = await sanic_client.get(f"/api/data/data_connectors/{data_connector_id}", headers=user_headers) + assert response.status_code == 200, response.text + data_connector = response.json + assert data_connector["id"] == data_connector_id + assert data_connector["name"] == "My data connector" + assert data_connector["namespace"] == namespace + assert data_connector["slug"] == new_slug + + # Check that we can get the data connector with the new slug + _, response = await sanic_client.get( + f"/api/data/namespaces/{namespace}/data_connectors/{new_slug}", headers=user_headers + ) + assert response.status_code == 200, response.text + assert response.json is not None + assert response.json.get("id") == data_connector_id + + # Check that we can get the data connector with the old slug + _, response = await sanic_client.get( + f"/api/data/namespaces/{namespace}/data_connectors/{old_slug}", headers=user_headers + ) + assert response.status_code == 200, response.text + assert response.json is not None + assert response.json.get("id") == data_connector_id + + @pytest.mark.asyncio async def test_delete_data_connector(sanic_client: SanicASGITestClient, create_data_connector, user_headers) -> None: await create_data_connector("Data connector 1")