From 4d0245c13e98a134b76234d18b4d8c490b5d4002 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 15 Jan 2025 11:20:34 +0100 Subject: [PATCH 01/44] drafts tests --- .../models_library/api_schemas_webserver/folders_v2.py | 1 + .../src/models_library/api_schemas_webserver/projects.py | 2 ++ services/web/server/tests/unit/with_dbs/03/test_trash.py | 8 +++++++- 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/packages/models-library/src/models_library/api_schemas_webserver/folders_v2.py b/packages/models-library/src/models_library/api_schemas_webserver/folders_v2.py index adf0766442e..5a23a70351f 100644 --- a/packages/models-library/src/models_library/api_schemas_webserver/folders_v2.py +++ b/packages/models-library/src/models_library/api_schemas_webserver/folders_v2.py @@ -19,6 +19,7 @@ class FolderGet(OutputSchema): created_at: datetime modified_at: datetime trashed_at: datetime | None + trashed_by: GroupID | None owner: GroupID workspace_id: WorkspaceID | None my_access_rights: AccessRights diff --git a/packages/models-library/src/models_library/api_schemas_webserver/projects.py b/packages/models-library/src/models_library/api_schemas_webserver/projects.py index 95b6b50805f..aee6b6b49bd 100644 --- a/packages/models-library/src/models_library/api_schemas_webserver/projects.py +++ b/packages/models-library/src/models_library/api_schemas_webserver/projects.py @@ -25,6 +25,7 @@ from ..basic_types import LongTruncatedStr, ShortTruncatedStr from ..emails import LowerCaseEmailStr from ..folders import FolderID +from ..groups import GroupID from ..projects import ClassifierID, DateTimeStr, NodesDict, ProjectID from ..projects_access import AccessRights, GroupIDStr from ..projects_state import ProjectState @@ -98,6 +99,7 @@ class ProjectGet(OutputSchema): folder_id: FolderID | None trashed_at: datetime | None + trashed_by: GroupID | None _empty_description = field_validator("description", mode="before")( none_to_empty_str_pre_validator diff --git a/services/web/server/tests/unit/with_dbs/03/test_trash.py b/services/web/server/tests/unit/with_dbs/03/test_trash.py index 08d9d4f864d..53ea9d26367 100644 --- a/services/web/server/tests/unit/with_dbs/03/test_trash.py +++ b/services/web/server/tests/unit/with_dbs/03/test_trash.py @@ -150,6 +150,7 @@ async def test_trash_projects( # noqa: PLR0915 assert got.trashed_at assert trashing_at < got.trashed_at assert got.trashed_at < arrow.utcnow().datetime + assert got.trashed_by == logged_user["primary_gid"] # LIST trashed resp = await client.get("/v0/projects", params={"filters": '{"trashed": true}'}) @@ -232,6 +233,8 @@ async def test_trash_single_folder(client: TestClient, logged_user: UserInfoDict assert got.trashed_at assert trashing_at < got.trashed_at assert got.trashed_at < arrow.utcnow().datetime + assert got.trashed_by == logged_user["primary_gid"] + assert got.owner == logged_user["primary_gid"] # LIST trashed resp = await client.get("/v0/folders", params={"filters": '{"trashed": true}'}) @@ -347,16 +350,19 @@ async def test_trash_folder_with_content( data, _ = await assert_status(resp, status.HTTP_200_OK) got = FolderGet.model_validate(data) assert got.trashed_at is not None + assert got.trashed_by == logged_user["primary_gid"] resp = await client.get(f"/v0/folders/{subfolder.folder_id}") data, _ = await assert_status(resp, status.HTTP_200_OK) got = FolderGet.model_validate(data) assert got.trashed_at is not None + assert got.trashed_by == logged_user["primary_gid"] resp = await client.get(f"/v0/projects/{project_uuid}") data, _ = await assert_status(resp, status.HTTP_200_OK) got = ProjectGet.model_validate(data) assert got.trashed_at is not None + assert got.trashed_by == logged_user["primary_gid"] # UNTRASH folder resp = await client.post(f"/v0/folders/{folder.folder_id}:untrash") @@ -471,7 +477,7 @@ async def test_trash_empty_workspace( ) assert page.data[0].trashed_at is not None assert before_trash < page.data[0].trashed_at - assert page.data[0].trashed_by == logged_user["id"] + assert page.data[0].trashed_by == logged_user["primary_gid"] # -------- From 41b94c49ca413bdd08790ab34471bf07de25e9cc Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 15 Jan 2025 11:56:50 +0100 Subject: [PATCH 02/44] back to uid --- .../api_schemas_webserver/folders_v2.py | 3 ++- .../models_library/api_schemas_webserver/projects.py | 4 ++-- .../projects/_crud_api_create.py | 12 +++++++++++- .../projects/_projects_db.py | 2 ++ .../projects/_trash_service.py | 12 ++++++++---- .../src/simcore_service_webserver/projects/models.py | 4 +++- .../projects/projects_service.py | 5 +++-- .../web/server/tests/unit/with_dbs/03/test_trash.py | 12 ++++++------ 8 files changed, 37 insertions(+), 17 deletions(-) diff --git a/packages/models-library/src/models_library/api_schemas_webserver/folders_v2.py b/packages/models-library/src/models_library/api_schemas_webserver/folders_v2.py index 5a23a70351f..4f4a3f4c7ef 100644 --- a/packages/models-library/src/models_library/api_schemas_webserver/folders_v2.py +++ b/packages/models-library/src/models_library/api_schemas_webserver/folders_v2.py @@ -7,6 +7,7 @@ from ..basic_types import IDStr from ..folders import FolderID from ..groups import GroupID +from ..users import UserID from ..utils.common_validators import null_or_none_str_to_none_validator from ..workspaces import WorkspaceID from ._base import InputSchema, OutputSchema @@ -19,7 +20,7 @@ class FolderGet(OutputSchema): created_at: datetime modified_at: datetime trashed_at: datetime | None - trashed_by: GroupID | None + trashed_by: UserID | None owner: GroupID workspace_id: WorkspaceID | None my_access_rights: AccessRights diff --git a/packages/models-library/src/models_library/api_schemas_webserver/projects.py b/packages/models-library/src/models_library/api_schemas_webserver/projects.py index aee6b6b49bd..e8ee8e23f3e 100644 --- a/packages/models-library/src/models_library/api_schemas_webserver/projects.py +++ b/packages/models-library/src/models_library/api_schemas_webserver/projects.py @@ -20,12 +20,12 @@ PlainSerializer, field_validator, ) +from simcore_service_webserver.models import UserID from ..api_schemas_long_running_tasks.tasks import TaskGet from ..basic_types import LongTruncatedStr, ShortTruncatedStr from ..emails import LowerCaseEmailStr from ..folders import FolderID -from ..groups import GroupID from ..projects import ClassifierID, DateTimeStr, NodesDict, ProjectID from ..projects_access import AccessRights, GroupIDStr from ..projects_state import ProjectState @@ -99,7 +99,7 @@ class ProjectGet(OutputSchema): folder_id: FolderID | None trashed_at: datetime | None - trashed_by: GroupID | None + trashed_by: UserID | None _empty_description = field_validator("description", mode="before")( none_to_empty_str_pre_validator diff --git a/services/web/server/src/simcore_service_webserver/projects/_crud_api_create.py b/services/web/server/src/simcore_service_webserver/projects/_crud_api_create.py index e083e16b28a..f0ef99a4c20 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_crud_api_create.py +++ b/services/web/server/src/simcore_service_webserver/projects/_crud_api_create.py @@ -35,6 +35,7 @@ copy_data_folders_from_project, get_project_total_size_simcore_s3, ) +from ..users._users_service import get_user_primary_group_id from ..users.api import get_user_fullname from ..workspaces.api import check_user_workspace_access, get_user_workspace from ..workspaces.errors import WorkspaceAccessForbiddenError @@ -440,7 +441,16 @@ async def create_project( # pylint: disable=too-many-arguments,too-many-branche } # Ensures is like ProjectGet - data = ProjectGet.from_domain_model(new_project).data(exclude_unset=True) + if trashed_by_uid := new_project.get("trashed_by"): + trashed_by_primary_gid = await get_user_primary_group_id( + request.app, trashed_by_uid + ) + else: + trashed_by_primary_gid = None + + data = ProjectGet.from_domain_model( + new_project, trashed_by_primary_gid=trashed_by_primary_gid + ).data(exclude_unset=True) raise web.HTTPCreated( text=json_dumps({"data": data}), diff --git a/services/web/server/src/simcore_service_webserver/projects/_projects_db.py b/services/web/server/src/simcore_service_webserver/projects/_projects_db.py index 92c73867f77..083543f8fdd 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_projects_db.py +++ b/services/web/server/src/simcore_service_webserver/projects/_projects_db.py @@ -35,6 +35,8 @@ projects.c.hidden, projects.c.workspace_id, projects.c.trashed, + projects.c.trashed_by, + projects.c.trashed_explicitly, ] diff --git a/services/web/server/src/simcore_service_webserver/projects/_trash_service.py b/services/web/server/src/simcore_service_webserver/projects/_trash_service.py index e535d387010..cc6be0de495 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_trash_service.py +++ b/services/web/server/src/simcore_service_webserver/projects/_trash_service.py @@ -15,7 +15,7 @@ from . import projects_service from ._access_rights_api import check_user_project_permission from .exceptions import ProjectRunningConflictError -from .models import ProjectPatchExtended +from .models import ProjectPatchInternalExtended _logger = logging.getLogger(__name__) @@ -94,8 +94,10 @@ async def _schedule(): user_id=user_id, product_name=product_name, project_uuid=project_id, - project_patch=ProjectPatchExtended( - trashed_at=arrow.utcnow().datetime, trashed_explicitly=explicit + project_patch=ProjectPatchInternalExtended( + trashed_at=arrow.utcnow().datetime, + trashed_explicitly=explicit, + trashed_by=user_id, ), ) @@ -113,5 +115,7 @@ async def untrash_project( user_id=user_id, product_name=product_name, project_uuid=project_id, - project_patch=ProjectPatchExtended(trashed_at=None, trashed_explicitly=False), + project_patch=ProjectPatchInternalExtended( + trashed_at=None, trashed_explicitly=False, trashed_by=None + ), ) diff --git a/services/web/server/src/simcore_service_webserver/projects/models.py b/services/web/server/src/simcore_service_webserver/projects/models.py index eab20e2f848..59a452bd48d 100644 --- a/services/web/server/src/simcore_service_webserver/projects/models.py +++ b/services/web/server/src/simcore_service_webserver/projects/models.py @@ -53,6 +53,7 @@ class ProjectDB(BaseModel): hidden: bool workspace_id: WorkspaceID | None trashed: datetime | None + trashed_by: UserID | None trashed_explicitly: bool = False model_config = ConfigDict(from_attributes=True, arbitrary_types_allowed=True) @@ -94,9 +95,10 @@ class UserProjectAccessRightsWithWorkspace(BaseModel): model_config = ConfigDict(from_attributes=True) -class ProjectPatchExtended(ProjectPatch): +class ProjectPatchInternalExtended(ProjectPatch): # ONLY used internally trashed_at: datetime | None + trashed_by: UserID | None trashed_explicitly: bool model_config = ConfigDict(populate_by_name=True, extra="forbid") diff --git a/services/web/server/src/simcore_service_webserver/projects/projects_service.py b/services/web/server/src/simcore_service_webserver/projects/projects_service.py index e34c784da73..73eb734170a 100644 --- a/services/web/server/src/simcore_service_webserver/projects/projects_service.py +++ b/services/web/server/src/simcore_service_webserver/projects/projects_service.py @@ -148,7 +148,8 @@ ProjectStartsTooManyDynamicNodesError, ProjectTooManyProjectOpenedError, ) -from .models import ProjectDict, ProjectPatchExtended +from .lock import get_project_locked_state, is_project_locked, lock_project +from .models import ProjectDict, ProjectPatchInternalExtended from .settings import ProjectsSettings, get_plugin_settings from .utils import extract_dns_without_default_port @@ -254,7 +255,7 @@ async def patch_project( *, user_id: UserID, project_uuid: ProjectID, - project_patch: ProjectPatch | ProjectPatchExtended, + project_patch: ProjectPatch | ProjectPatchInternalExtended, product_name: ProductName, ): patch_project_data = project_patch.to_domain_model() diff --git a/services/web/server/tests/unit/with_dbs/03/test_trash.py b/services/web/server/tests/unit/with_dbs/03/test_trash.py index 53ea9d26367..c2e37ef86ed 100644 --- a/services/web/server/tests/unit/with_dbs/03/test_trash.py +++ b/services/web/server/tests/unit/with_dbs/03/test_trash.py @@ -150,7 +150,7 @@ async def test_trash_projects( # noqa: PLR0915 assert got.trashed_at assert trashing_at < got.trashed_at assert got.trashed_at < arrow.utcnow().datetime - assert got.trashed_by == logged_user["primary_gid"] + assert got.trashed_by == logged_user["id"] # LIST trashed resp = await client.get("/v0/projects", params={"filters": '{"trashed": true}'}) @@ -233,7 +233,7 @@ async def test_trash_single_folder(client: TestClient, logged_user: UserInfoDict assert got.trashed_at assert trashing_at < got.trashed_at assert got.trashed_at < arrow.utcnow().datetime - assert got.trashed_by == logged_user["primary_gid"] + assert got.trashed_by == logged_user["id"] assert got.owner == logged_user["primary_gid"] # LIST trashed @@ -350,19 +350,19 @@ async def test_trash_folder_with_content( data, _ = await assert_status(resp, status.HTTP_200_OK) got = FolderGet.model_validate(data) assert got.trashed_at is not None - assert got.trashed_by == logged_user["primary_gid"] + assert got.trashed_by == logged_user["id"] resp = await client.get(f"/v0/folders/{subfolder.folder_id}") data, _ = await assert_status(resp, status.HTTP_200_OK) got = FolderGet.model_validate(data) assert got.trashed_at is not None - assert got.trashed_by == logged_user["primary_gid"] + assert got.trashed_by == logged_user["id"] resp = await client.get(f"/v0/projects/{project_uuid}") data, _ = await assert_status(resp, status.HTTP_200_OK) got = ProjectGet.model_validate(data) assert got.trashed_at is not None - assert got.trashed_by == logged_user["primary_gid"] + assert got.trashed_by == logged_user["id"] # UNTRASH folder resp = await client.post(f"/v0/folders/{folder.folder_id}:untrash") @@ -477,7 +477,7 @@ async def test_trash_empty_workspace( ) assert page.data[0].trashed_at is not None assert before_trash < page.data[0].trashed_at - assert page.data[0].trashed_by == logged_user["primary_gid"] + assert page.data[0].trashed_by == logged_user["id"] # -------- From 47d525535650b16e2c326c9b42ac088671b5c4b7 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 16 Jan 2025 21:35:58 +0100 Subject: [PATCH 03/44] rm import --- .../src/models_library/api_schemas_webserver/projects.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/models-library/src/models_library/api_schemas_webserver/projects.py b/packages/models-library/src/models_library/api_schemas_webserver/projects.py index e8ee8e23f3e..bf949bb8b9c 100644 --- a/packages/models-library/src/models_library/api_schemas_webserver/projects.py +++ b/packages/models-library/src/models_library/api_schemas_webserver/projects.py @@ -9,9 +9,6 @@ from typing import Annotated, Any, Literal, Self, TypeAlias from common_library.dict_tools import remap_keys -from models_library.folders import FolderID -from models_library.utils._original_fastapi_encoders import jsonable_encoder -from models_library.workspaces import WorkspaceID from pydantic import ( BeforeValidator, ConfigDict, @@ -20,7 +17,6 @@ PlainSerializer, field_validator, ) -from simcore_service_webserver.models import UserID from ..api_schemas_long_running_tasks.tasks import TaskGet from ..basic_types import LongTruncatedStr, ShortTruncatedStr @@ -30,6 +26,8 @@ from ..projects_access import AccessRights, GroupIDStr from ..projects_state import ProjectState from ..projects_ui import StudyUI +from ..users import UserID +from ..utils._original_fastapi_encoders import jsonable_encoder from ..utils.common_validators import ( empty_str_to_none_pre_validator, none_to_empty_str_pre_validator, @@ -129,7 +127,8 @@ class ProjectReplace(InputSchema): name: ShortTruncatedStr description: LongTruncatedStr thumbnail: Annotated[ - HttpUrl | None, BeforeValidator(empty_str_to_none_pre_validator) + HttpUrl | None, + BeforeValidator(empty_str_to_none_pre_validator), ] = Field(default=None) creation_date: DateTimeStr last_change_date: DateTimeStr From 10b5e79e532e783292611cffa10bdf0ed1c96e94 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 16 Jan 2025 21:44:29 +0100 Subject: [PATCH 04/44] fixes userid --- .../projects/_crud_api_create.py | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/projects/_crud_api_create.py b/services/web/server/src/simcore_service_webserver/projects/_crud_api_create.py index f0ef99a4c20..307780abdde 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_crud_api_create.py +++ b/services/web/server/src/simcore_service_webserver/projects/_crud_api_create.py @@ -18,7 +18,7 @@ from pydantic import TypeAdapter from servicelib.aiohttp.long_running_tasks.server import TaskProgress from servicelib.mimetype_constants import MIMETYPE_APPLICATION_JSON -from servicelib.redis import with_project_locked +from servicelib.rest_constants import RESPONSE_MODEL_POLICY from simcore_postgres_database.utils_projects_nodes import ( ProjectNode, ProjectNodeCreate, @@ -35,7 +35,6 @@ copy_data_folders_from_project, get_project_total_size_simcore_s3, ) -from ..users._users_service import get_user_primary_group_id from ..users.api import get_user_fullname from ..workspaces.api import check_user_workspace_access, get_user_workspace from ..workspaces.errors import WorkspaceAccessForbiddenError @@ -440,17 +439,7 @@ async def create_project( # pylint: disable=too-many-arguments,too-many-branche for gid, access in workspace.access_rights.items() } - # Ensures is like ProjectGet - if trashed_by_uid := new_project.get("trashed_by"): - trashed_by_primary_gid = await get_user_primary_group_id( - request.app, trashed_by_uid - ) - else: - trashed_by_primary_gid = None - - data = ProjectGet.from_domain_model( - new_project, trashed_by_primary_gid=trashed_by_primary_gid - ).data(exclude_unset=True) + data = ProjectGet.from_domain_model(new_project).data(**RESPONSE_MODEL_POLICY) raise web.HTTPCreated( text=json_dumps({"data": data}), From a900b67e4baea4ae77622c5e9965a3a9b0da29ae Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 16 Jan 2025 21:57:04 +0100 Subject: [PATCH 05/44] fixes folders --- .../src/models_library/folders.py | 37 ++----- .../folders/_folders_repository.py | 6 +- .../folders/_folders_service.py | 5 + .../tests/unit/with_dbs/03/test_trash.py | 102 +++++++++++++++++- 4 files changed, 120 insertions(+), 30 deletions(-) diff --git a/packages/models-library/src/models_library/folders.py b/packages/models-library/src/models_library/folders.py index 0a3821fc987..24b1ed37315 100644 --- a/packages/models-library/src/models_library/folders.py +++ b/packages/models-library/src/models_library/folders.py @@ -2,14 +2,7 @@ from enum import auto from typing import TypeAlias -from pydantic import ( - BaseModel, - ConfigDict, - Field, - PositiveInt, - ValidationInfo, - field_validator, -) +from pydantic import BaseModel, ConfigDict, PositiveInt, ValidationInfo, field_validator from .access_rights import AccessRights from .groups import GroupID @@ -52,25 +45,17 @@ class FolderDB(BaseModel): folder_id: FolderID name: str parent_folder_id: FolderID | None - created_by_gid: GroupID = Field( - ..., - description="GID of the group that owns this wallet", - ) - created: datetime = Field( - ..., - description="Timestamp on creation", - ) - modified: datetime = Field( - ..., - description="Timestamp of last modification", - ) - trashed: datetime | None = Field( - ..., - ) - - user_id: UserID | None - workspace_id: WorkspaceID | None + created_by_gid: GroupID + created: datetime + modified: datetime + + trashed: datetime | None + trashed_by: UserID | None + trashed_explicitly: bool + + user_id: UserID | None # owner? + workspace_id: WorkspaceID | None model_config = ConfigDict(from_attributes=True) diff --git a/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py b/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py index 243fba7e858..d098573063e 100644 --- a/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py +++ b/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py @@ -56,6 +56,8 @@ folders_v2.c.created, folders_v2.c.modified, folders_v2.c.trashed, + folders_v2.c.trashed_by, + folders_v2.c.trashed_explicitly, folders_v2.c.user_id, folders_v2.c.workspace_id, ) @@ -285,8 +287,8 @@ async def get( ) async with pass_or_acquire_connection(get_asyncpg_engine(app), connection) as conn: - result = await conn.stream(query) - row = await result.first() + result = await conn.execute(query) + row = result.first() if row is None: raise FolderAccessForbiddenError( reason=f"Folder {folder_id} does not exist.", diff --git a/services/web/server/src/simcore_service_webserver/folders/_folders_service.py b/services/web/server/src/simcore_service_webserver/folders/_folders_service.py index 2cd4f0103e9..396fcecf4e6 100644 --- a/services/web/server/src/simcore_service_webserver/folders/_folders_service.py +++ b/services/web/server/src/simcore_service_webserver/folders/_folders_service.py @@ -93,6 +93,7 @@ async def create_folder( created_at=folder_db.created, modified_at=folder_db.modified, trashed_at=folder_db.trashed, + trashed_by=folder_db.trashed_by, owner=folder_db.created_by_gid, workspace_id=workspace_id, my_access_rights=user_folder_access_rights, @@ -136,6 +137,7 @@ async def get_folder( created_at=folder_db.created, modified_at=folder_db.modified, trashed_at=folder_db.trashed, + trashed_by=folder_db.trashed_by, owner=folder_db.created_by_gid, workspace_id=folder_db.workspace_id, my_access_rights=user_folder_access_rights, @@ -186,6 +188,7 @@ async def list_folders( created_at=folder.created, modified_at=folder.modified, trashed_at=folder.trashed, + trashed_by=folder.trashed_by, owner=folder.created_by_gid, workspace_id=folder.workspace_id, my_access_rights=folder.my_access_rights, @@ -230,6 +233,7 @@ async def list_folders_full_depth( created_at=folder.created, modified_at=folder.modified, trashed_at=folder.trashed, + trashed_by=folder.trashed_by, owner=folder.created_by_gid, workspace_id=folder.workspace_id, my_access_rights=folder.my_access_rights, @@ -307,6 +311,7 @@ async def update_folder( created_at=folder_db.created, modified_at=folder_db.modified, trashed_at=folder_db.trashed, + trashed_by=folder_db.trashed_by, owner=folder_db.created_by_gid, workspace_id=folder_db.workspace_id, my_access_rights=user_folder_access_rights, diff --git a/services/web/server/tests/unit/with_dbs/03/test_trash.py b/services/web/server/tests/unit/with_dbs/03/test_trash.py index c2e37ef86ed..2a1b71d7b64 100644 --- a/services/web/server/tests/unit/with_dbs/03/test_trash.py +++ b/services/web/server/tests/unit/with_dbs/03/test_trash.py @@ -23,9 +23,10 @@ from pytest_simcore.helpers.assert_checks import assert_status from pytest_simcore.helpers.monkeypatch_envs import setenvs_from_dict from pytest_simcore.helpers.typing_env import EnvVarsDict -from pytest_simcore.helpers.webserver_login import UserInfoDict +from pytest_simcore.helpers.webserver_login import NewUser, UserInfoDict from servicelib.aiohttp import status from simcore_service_webserver.db.models import UserRole +from simcore_service_webserver.projects._groups_api import ProjectGroupGet from simcore_service_webserver.projects.models import ProjectDict from yarl import URL @@ -74,7 +75,7 @@ async def test_trash_projects( # noqa: PLR0915 ): assert client.app - # this test should have no errors stopping services + # this test should emulate NO errors stopping services mock_remove_dynamic_services = mocker.patch( "simcore_service_webserver.projects._trash_service.projects_service.remove_project_dynamic_services", autospec=True, @@ -182,6 +183,103 @@ async def test_trash_projects( # noqa: PLR0915 mock_remove_dynamic_services.assert_awaited() +@pytest.fixture +async def other_user( + client: TestClient, logged_user: UserInfoDict +) -> AsyncIterable[UserInfoDict]: + # new user different from logged_user + async with NewUser( + { + "name": f"other_user_than_{logged_user['name']}", + "role": "USER", + }, + client.app, + ) as user: + yield user + + +async def test_trash_projects_shared_among_users( + client: TestClient, + logged_user: UserInfoDict, + user_project: ProjectDict, + other_user: UserInfoDict, + mocked_catalog: None, + mocked_dynamic_services_interface: dict[str, MagicMock], +): + assert client.app + + project_uuid = UUID(user_project["uuid"]) + + # GET project + url = client.app.router["get_project"].url_for(project_id=f"{project_uuid}") + resp = await client.get(f"{url}") + data, _ = await assert_status(resp, status.HTTP_200_OK) + + project = ProjectGet.model_validate(data) + assert project.uuid == project_uuid + assert project.prj_owner == logged_user["email"] + + # SHARE PROJECT with other-user + url = client.app.router["create_project_group"].url_for( + project_id=f"{project_uuid}", group_id=f"{other_user['primary_gid']}" + ) + resp = await client.post( + f"{url}", + json={"read": True, "write": True, "delete": False}, + ) + data, _ = await assert_status(resp, status.HTTP_201_CREATED) + + project_group = ProjectGroupGet.model_validate(data) + assert project_group.gid == other_user["primary_gid"] + assert project_group.read is True + assert project_group.write is True + assert project_group.delete is False + + # TRASH project + trashing_at = arrow.utcnow().datetime + resp = await client.post( + f"/v0/projects/{project_uuid}:trash", params={"force": "true"} + ) + await assert_status(resp, status.HTTP_204_NO_CONTENT) + + # LIST trashed of logged_user + resp = await client.get("/v0/projects", params={"filters": '{"trashed": true}'}) + await assert_status(resp, status.HTTP_200_OK) + + page = Page[ProjectListItem].model_validate(await resp.json()) + assert page.meta.total == 1 + assert page.data[0].uuid == project_uuid + assert page.data[0].trashed_at + assert trashing_at < page.data[0].trashed_at + assert page.data[0].trashed_by == logged_user["id"] + + # Swith USER: LOGOUT + url = client.app.router["auth_logout"].url_for() + resp = await client.post(f"{url}") + await assert_status(resp, status.HTTP_200_OK) + + url = client.app.router["auth_login"].url_for() + resp = await client.post( + f"{url}", + json={ + "email": other_user["email"], + "password": other_user["raw_password"], + }, + ) + data, _ = await assert_status(resp, status.HTTP_200_OK) + + # LIST trashed of another_user + resp = await client.get("/v0/projects", params={"filters": '{"trashed": true}'}) + await assert_status(resp, status.HTTP_200_OK) + + page = Page[ProjectListItem].model_validate(await resp.json()) + assert page.meta.total == 1 + assert page.data[0].uuid == project_uuid + assert page.data[0].trashed_at + assert trashing_at < page.data[0].trashed_at + assert page.data[0].trashed_by == logged_user["id"] + + @pytest.mark.acceptance_test( "For https://github.com/ITISFoundation/osparc-simcore/pull/6642" ) From 5e2e63c3481f6512e12fd822a0566bab2fe722a1 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Fri, 17 Jan 2025 11:17:44 +0100 Subject: [PATCH 06/44] doc --- .../src/models_library/api_schemas_webserver/folders_v2.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/models-library/src/models_library/api_schemas_webserver/folders_v2.py b/packages/models-library/src/models_library/api_schemas_webserver/folders_v2.py index 4f4a3f4c7ef..ae26a2d4ae4 100644 --- a/packages/models-library/src/models_library/api_schemas_webserver/folders_v2.py +++ b/packages/models-library/src/models_library/api_schemas_webserver/folders_v2.py @@ -20,6 +20,8 @@ class FolderGet(OutputSchema): created_at: datetime modified_at: datetime trashed_at: datetime | None + # NOTE: by design a folder is constraint to trashed_by == owner + # but we add this here for completeness trashed_by: UserID | None owner: GroupID workspace_id: WorkspaceID | None From b3f671bbbd1116d7692a00ac2581c61687857bd2 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Fri, 17 Jan 2025 11:26:42 +0100 Subject: [PATCH 07/44] updates OAS --- .../api/v0/openapi.yaml | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml index 84e6f9ca1df..c208c094548 100644 --- a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml +++ b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml @@ -10011,6 +10011,13 @@ components: format: date-time - type: 'null' title: Trashedat + trashedBy: + anyOf: + - type: integer + exclusiveMinimum: true + minimum: 0 + - type: 'null' + title: Trashedby owner: type: integer exclusiveMinimum: true @@ -10032,6 +10039,7 @@ components: - createdAt - modifiedAt - trashedAt + - trashedBy - owner - workspaceId - myAccessRights @@ -12600,6 +12608,13 @@ components: format: date-time - type: 'null' title: Trashedat + trashedBy: + anyOf: + - type: integer + exclusiveMinimum: true + minimum: 0 + - type: 'null' + title: Trashedby type: object required: - uuid @@ -12616,6 +12631,7 @@ components: - workspaceId - folderId - trashedAt + - trashedBy title: ProjectGet ProjectGroupGet: properties: @@ -12848,6 +12864,13 @@ components: format: date-time - type: 'null' title: Trashedat + trashedBy: + anyOf: + - type: integer + exclusiveMinimum: true + minimum: 0 + - type: 'null' + title: Trashedby type: object required: - uuid @@ -12864,6 +12887,7 @@ components: - workspaceId - folderId - trashedAt + - trashedBy title: ProjectListItem ProjectLocked: properties: From 9384eefc9ee27fdbaf2e3e6a51d11dff3fd5a923 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Fri, 17 Jan 2025 11:26:52 +0100 Subject: [PATCH 08/44] =?UTF-8?q?services/webserver=20api=20version:=200.5?= =?UTF-8?q?0.0=20=E2=86=92=200.51.0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- services/web/server/VERSION | 2 +- services/web/server/setup.cfg | 6 +++--- .../src/simcore_service_webserver/api/v0/openapi.yaml | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/services/web/server/VERSION b/services/web/server/VERSION index 564edf82ddf..c5d4cee36a1 100644 --- a/services/web/server/VERSION +++ b/services/web/server/VERSION @@ -1 +1 @@ -0.50.0 +0.51.0 diff --git a/services/web/server/setup.cfg b/services/web/server/setup.cfg index 486fe83406d..65736bec36e 100644 --- a/services/web/server/setup.cfg +++ b/services/web/server/setup.cfg @@ -1,5 +1,5 @@ [bumpversion] -current_version = 0.50.0 +current_version = 0.51.0 commit = True message = services/webserver api version: {current_version} → {new_version} tag = False @@ -12,13 +12,13 @@ commit_args = --no-verify [tool:pytest] addopts = --strict-markers asyncio_mode = auto -markers = +markers = slow: marks tests as slow (deselect with '-m "not slow"') acceptance_test: "marks tests as 'acceptance tests' i.e. does the system do what the user expects? Typically those are workflows." testit: "marks test to run during development" heavy_load: "mark tests that require large amount of data" [mypy] -plugins = +plugins = pydantic.mypy sqlalchemy.ext.mypy.plugin diff --git a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml index c208c094548..84d455f2b54 100644 --- a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml +++ b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml @@ -2,7 +2,7 @@ openapi: 3.1.0 info: title: simcore-service-webserver description: Main service with an interface (http-API & websockets) to the web front-end - version: 0.50.0 + version: 0.51.0 servers: - url: '' description: webserver From f5357ad9678794c9d7ac11127f6875a25bb6c8ed Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Fri, 17 Jan 2025 11:49:55 +0100 Subject: [PATCH 09/44] updates workspaces to return groupid --- .../src/models_library/workspaces.py | 17 +-- .../workspaces/_workspaces_repository.py | 106 +++++++++--------- 2 files changed, 59 insertions(+), 64 deletions(-) diff --git a/packages/models-library/src/models_library/workspaces.py b/packages/models-library/src/models_library/workspaces.py index 01f66685fa1..ca31304a869 100644 --- a/packages/models-library/src/models_library/workspaces.py +++ b/packages/models-library/src/models_library/workspaces.py @@ -47,7 +47,7 @@ class Workspace(BaseModel): workspace_id: WorkspaceID name: str description: str | None - owner_primary_gid: PositiveInt = Field( + owner_primary_gid: GroupID = Field( ..., description="GID of the group that owns this wallet", ) @@ -62,6 +62,14 @@ class Workspace(BaseModel): ) trashed: datetime | None trashed_by: UserID | None + trashed_by_primary_gid: GroupID | None = None + + model_config = ConfigDict(from_attributes=True) + + +class UserWorkspaceWithAccessRights(Workspace): + my_access_rights: AccessRights + access_rights: dict[GroupID, AccessRights] model_config = ConfigDict(from_attributes=True) @@ -72,10 +80,3 @@ class WorkspaceUpdates(BaseModel): thumbnail: str | None = None trashed: datetime | None = None trashed_by: UserID | None = None - - -class UserWorkspaceWithAccessRights(Workspace): - my_access_rights: AccessRights - access_rights: dict[GroupID, AccessRights] - - model_config = ConfigDict(from_attributes=True) diff --git a/services/web/server/src/simcore_service_webserver/workspaces/_workspaces_repository.py b/services/web/server/src/simcore_service_webserver/workspaces/_workspaces_repository.py index 63907713bcc..5603c1cc712 100644 --- a/services/web/server/src/simcore_service_webserver/workspaces/_workspaces_repository.py +++ b/services/web/server/src/simcore_service_webserver/workspaces/_workspaces_repository.py @@ -19,6 +19,7 @@ WorkspaceUpdates, ) from pydantic import NonNegativeInt +from simcore_postgres_database.models.users import users from simcore_postgres_database.models.workspaces import workspaces from simcore_postgres_database.models.workspaces_access_rights import ( workspaces_access_rights, @@ -40,7 +41,7 @@ _logger = logging.getLogger(__name__) -_SELECTION_ARGS = ( +_WORKSPACE_SELECTION_COLS = ( workspaces.c.workspace_id, workspaces.c.name, workspaces.c.description, @@ -52,10 +53,12 @@ workspaces.c.trashed_by, ) -assert set(Workspace.model_fields) == {c.name for c in _SELECTION_ARGS} # nosec +assert set(Workspace.model_fields).issubset( + {c.name for c in _WORKSPACE_SELECTION_COLS} # nosec +), "part of the data-model is one-to-one to the domain model" assert set(WorkspaceUpdates.model_fields).issubset( # nosec c.name for c in workspaces.columns -) +), "part of the data-model is one-to-one to the domain model" async def create_workspace( @@ -80,32 +83,54 @@ async def create_workspace( modified=func.now(), product_name=product_name, ) - .returning(*_SELECTION_ARGS) + .returning(*_WORKSPACE_SELECTION_COLS) ) row = await result.first() return Workspace.model_validate(row) -_access_rights_subquery = ( - select( - workspaces_access_rights.c.workspace_id, - func.jsonb_object_agg( - workspaces_access_rights.c.gid, - func.jsonb_build_object( - "read", - workspaces_access_rights.c.read, - "write", - workspaces_access_rights.c.write, - "delete", - workspaces_access_rights.c.delete, - ), +def _create_base_query(caller_user_id: UserID, product_name: ProductName): + # any other access + access_rights_subquery = ( + select( + workspaces_access_rights.c.workspace_id, + func.jsonb_object_agg( + workspaces_access_rights.c.gid, + func.jsonb_build_object( + "read", + workspaces_access_rights.c.read, + "write", + workspaces_access_rights.c.write, + "delete", + workspaces_access_rights.c.delete, + ), + ) + .filter( + workspaces_access_rights.c.read # Filters out entries where "read" is False + ) + .label("access_rights"), + ).group_by(workspaces_access_rights.c.workspace_id) + ).subquery("access_rights_subquery") + + # caller's access rights + my_access_rights_subquery = create_my_workspace_access_rights_subquery( + user_id=caller_user_id + ) + + return ( + select( + *_WORKSPACE_SELECTION_COLS, + access_rights_subquery.c.access_rights, + my_access_rights_subquery.c.my_access_rights, + users.c.primary_gid.label("trashed_by_primary_gid"), ) - .filter( - workspaces_access_rights.c.read # Filters out entries where "read" is False + .select_from( + workspaces.join(access_rights_subquery) + .join(my_access_rights_subquery) + .outerjoin(users, workspaces.c.trashed_by == users.c.id) ) - .label("access_rights"), - ).group_by(workspaces_access_rights.c.workspace_id) -).subquery("access_rights_subquery") + .where(workspaces.c.product_name == product_name) + ) async def list_workspaces_for_user( @@ -120,21 +145,7 @@ async def list_workspaces_for_user( limit: NonNegativeInt, order_by: OrderBy, ) -> tuple[int, list[UserWorkspaceWithAccessRights]]: - my_access_rights_subquery = create_my_workspace_access_rights_subquery( - user_id=user_id - ) - - base_query = ( - select( - *_SELECTION_ARGS, - _access_rights_subquery.c.access_rights, - my_access_rights_subquery.c.my_access_rights, - ) - .select_from( - workspaces.join(_access_rights_subquery).join(my_access_rights_subquery) - ) - .where(workspaces.c.product_name == product_name) - ) + base_query = _create_base_query(caller_user_id=user_id, product_name=product_name) if filter_trashed is not None: base_query = base_query.where( @@ -178,24 +189,7 @@ async def get_workspace_for_user( workspace_id: WorkspaceID, product_name: ProductName, ) -> UserWorkspaceWithAccessRights: - my_access_rights_subquery = create_my_workspace_access_rights_subquery( - user_id=user_id - ) - - base_query = ( - select( - *_SELECTION_ARGS, - _access_rights_subquery.c.access_rights, - my_access_rights_subquery.c.my_access_rights, - ) - .select_from( - workspaces.join(_access_rights_subquery).join(my_access_rights_subquery) - ) - .where( - (workspaces.c.workspace_id == workspace_id) - & (workspaces.c.product_name == product_name) - ) - ) + base_query = _create_base_query(caller_user_id=user_id, product_name=product_name) async with pass_or_acquire_connection(get_asyncpg_engine(app), connection) as conn: result = await conn.stream(base_query) @@ -229,7 +223,7 @@ async def update_workspace( (workspaces.c.workspace_id == workspace_id) & (workspaces.c.product_name == product_name) ) - .returning(*_SELECTION_ARGS) + .returning(*_WORKSPACE_SELECTION_COLS) ) row = await result.first() if row is None: From 607c6a37f172f2b885ef4a2ed99ce23b7aba915e Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Fri, 17 Jan 2025 13:21:37 +0100 Subject: [PATCH 10/44] updates fakes in api-server --- .../api_schemas_webserver/workspaces.py | 11 ++++++----- services/api-server/tests/mocks/create_study_job.json | 1 + .../tests/mocks/for_test_api_routes_studies.json | 2 ++ .../mocks/get_job_pricing_unit_invalid_solver.json | 1 + .../tests/mocks/get_job_pricing_unit_success.json | 1 + .../api-server/tests/mocks/get_solver_outputs.json | 1 + services/api-server/tests/mocks/on_list_jobs.json | 2 ++ .../api-server/tests/mocks/run_study_workflow.json | 1 + .../tests/mocks/start_job_not_enough_credit.json | 1 + .../tests/mocks/start_job_with_payment.json | 1 + .../mocks/test_get_and_update_study_job_metadata.json | 1 + .../workspaces/_workspaces_repository.py | 7 ------- .../web/server/tests/unit/with_dbs/03/test_trash.py | 6 +++--- 13 files changed, 21 insertions(+), 15 deletions(-) diff --git a/packages/models-library/src/models_library/api_schemas_webserver/workspaces.py b/packages/models-library/src/models_library/api_schemas_webserver/workspaces.py index 17762e9efea..1305af4f345 100644 --- a/packages/models-library/src/models_library/api_schemas_webserver/workspaces.py +++ b/packages/models-library/src/models_library/api_schemas_webserver/workspaces.py @@ -1,12 +1,11 @@ from datetime import datetime -from typing import Self +from typing import Annotated, Self -from pydantic import ConfigDict +from pydantic import ConfigDict, Field from ..access_rights import AccessRights from ..basic_types import IDStr from ..groups import GroupID -from ..users import UserID from ..workspaces import UserWorkspaceWithAccessRights, WorkspaceID from ._base import InputSchema, OutputSchema @@ -19,7 +18,9 @@ class WorkspaceGet(OutputSchema): created_at: datetime modified_at: datetime trashed_at: datetime | None - trashed_by: UserID | None + trashed_by: Annotated[ + GroupID | None, Field(description="The primary gid of the user who trashed") + ] my_access_rights: AccessRights access_rights: dict[GroupID, AccessRights] @@ -33,7 +34,7 @@ def from_domain_model(cls, wks: UserWorkspaceWithAccessRights) -> Self: created_at=wks.created, modified_at=wks.modified, trashed_at=wks.trashed, - trashed_by=wks.trashed_by if wks.trashed else None, + trashed_by=wks.trashed_by_primary_gid if wks.trashed else None, my_access_rights=wks.my_access_rights, access_rights=wks.access_rights, ) diff --git a/services/api-server/tests/mocks/create_study_job.json b/services/api-server/tests/mocks/create_study_job.json index e2b522e66db..b0e234caede 100644 --- a/services/api-server/tests/mocks/create_study_job.json +++ b/services/api-server/tests/mocks/create_study_job.json @@ -85,6 +85,7 @@ "workspaceId": 23, "folderId": 4, "trashedAt": "2024-05-14T09:55:20.099Z", + "trashedBy": 2, "accessRights": { "3": { "read": true, diff --git a/services/api-server/tests/mocks/for_test_api_routes_studies.json b/services/api-server/tests/mocks/for_test_api_routes_studies.json index 9e5b0e679ae..71394134beb 100644 --- a/services/api-server/tests/mocks/for_test_api_routes_studies.json +++ b/services/api-server/tests/mocks/for_test_api_routes_studies.json @@ -92,6 +92,7 @@ "workspaceId": 278, "folderId": 123, "trashedAt": "2023-07-20T20:02:55.535Z", + "trashedBy": 2, "workbench": { "deea006c-a223-4103-b46e-7b677428de9f": { "key": "simcore/services/frontend/file-picker", @@ -314,6 +315,7 @@ "workspaceId": 278, "folderId": 123, "trashedAt": "2023-07-20T20:04:10.607Z", + "trashedBy": 2, "workbench": { "deea006c-a223-4103-b46e-7b677428de9f": { "key": "simcore/services/frontend/file-picker", diff --git a/services/api-server/tests/mocks/get_job_pricing_unit_invalid_solver.json b/services/api-server/tests/mocks/get_job_pricing_unit_invalid_solver.json index a99eca3ed8f..53baa5b4f02 100644 --- a/services/api-server/tests/mocks/get_job_pricing_unit_invalid_solver.json +++ b/services/api-server/tests/mocks/get_job_pricing_unit_invalid_solver.json @@ -39,6 +39,7 @@ "workspaceId": 3, "folderId": 31, "trashedAt": null, + "trashedBy": null, "workbench": { "4b03863d-107a-5c77-a3ca-c5ba1d7048c0": { "key": "simcore/services/comp/isolve", diff --git a/services/api-server/tests/mocks/get_job_pricing_unit_success.json b/services/api-server/tests/mocks/get_job_pricing_unit_success.json index 3c10f684e46..ba4cae5d53c 100644 --- a/services/api-server/tests/mocks/get_job_pricing_unit_success.json +++ b/services/api-server/tests/mocks/get_job_pricing_unit_success.json @@ -39,6 +39,7 @@ "workspaceId": 3, "folderId": 1, "trashedAt": null, + "trashedBy": null, "workbench": { "4b03863d-107a-5c77-a3ca-c5ba1d7048c0": { "key": "simcore/services/comp/isolve", diff --git a/services/api-server/tests/mocks/get_solver_outputs.json b/services/api-server/tests/mocks/get_solver_outputs.json index adda3db505e..4971676029f 100644 --- a/services/api-server/tests/mocks/get_solver_outputs.json +++ b/services/api-server/tests/mocks/get_solver_outputs.json @@ -39,6 +39,7 @@ "workspaceId": 2, "folderId": 2, "trashedAt": null, + "trashedBy": null, "workbench": { "df42d273-b6f0-509c-bfb5-4abbc5bb0581": { "key": "simcore/services/comp/itis/sleeper", diff --git a/services/api-server/tests/mocks/on_list_jobs.json b/services/api-server/tests/mocks/on_list_jobs.json index d954da588fa..0e94cec3ee2 100644 --- a/services/api-server/tests/mocks/on_list_jobs.json +++ b/services/api-server/tests/mocks/on_list_jobs.json @@ -104,6 +104,7 @@ "workspaceId": 7, "folderId": 1, "trashedAt": "2023-06-22T18:42:36.506Z", + "trashedBy": 2, "workbench": { "05c7ed3b-0be1-5077-8065-fb55f5e59ff3": { "key": "simcore/services/comp/itis/sleeper", @@ -177,6 +178,7 @@ "workspaceId": 4, "folderId": 8, "trashedAt": "2023-06-22T18:42:33.201Z", + "trashedBy": 2, "workbench": { "34805d7e-c2d0-561f-831f-c74a28fc9bd1": { "key": "simcore/services/comp/itis/sleeper", diff --git a/services/api-server/tests/mocks/run_study_workflow.json b/services/api-server/tests/mocks/run_study_workflow.json index 8078a8cc155..9bfcf708650 100644 --- a/services/api-server/tests/mocks/run_study_workflow.json +++ b/services/api-server/tests/mocks/run_study_workflow.json @@ -304,6 +304,7 @@ "workspaceId": 3, "folderId": 3, "trashedAt": null, + "trashedBy": null, "workbench": { "ab014072-a95f-5775-bb34-5582a13245a6": { "key": "simcore/services/frontend/iterator-consumer/probe/file", diff --git a/services/api-server/tests/mocks/start_job_not_enough_credit.json b/services/api-server/tests/mocks/start_job_not_enough_credit.json index 19f54e53ca6..4ebe858881c 100644 --- a/services/api-server/tests/mocks/start_job_not_enough_credit.json +++ b/services/api-server/tests/mocks/start_job_not_enough_credit.json @@ -39,6 +39,7 @@ "workspaceId": 3, "folderId": 2, "trashedAt": null, + "trashedBy": null, "workbench": { "3b0b20e0-c860-51d9-9f82-d6b4bc5c2f24": { "key": "simcore/services/comp/itis/sleeper", diff --git a/services/api-server/tests/mocks/start_job_with_payment.json b/services/api-server/tests/mocks/start_job_with_payment.json index ac3aed74ecb..e4cbc7cc21f 100644 --- a/services/api-server/tests/mocks/start_job_with_payment.json +++ b/services/api-server/tests/mocks/start_job_with_payment.json @@ -39,6 +39,7 @@ "workspaceId": 12, "folderId": 2, "trashedAt": null, + "trashedBy": null, "workbench": { "657b124c-0697-5166-b820-a2ea2704ae84": { "key": "simcore/services/comp/itis/sleeper", diff --git a/services/api-server/tests/mocks/test_get_and_update_study_job_metadata.json b/services/api-server/tests/mocks/test_get_and_update_study_job_metadata.json index a8f690f0827..2500f764bc1 100644 --- a/services/api-server/tests/mocks/test_get_and_update_study_job_metadata.json +++ b/services/api-server/tests/mocks/test_get_and_update_study_job_metadata.json @@ -211,6 +211,7 @@ "workspaceId": 3, "folderId": 12, "trashedAt": "2024-05-30T10:30:54.137359", + "trashedBy": 2, "workbench": { "45043872-d6d3-530b-bf40-67bfde79191c": { "key": "simcore/services/dynamic/jupyter-math", diff --git a/services/web/server/src/simcore_service_webserver/workspaces/_workspaces_repository.py b/services/web/server/src/simcore_service_webserver/workspaces/_workspaces_repository.py index 5603c1cc712..4a01091bcf1 100644 --- a/services/web/server/src/simcore_service_webserver/workspaces/_workspaces_repository.py +++ b/services/web/server/src/simcore_service_webserver/workspaces/_workspaces_repository.py @@ -53,13 +53,6 @@ workspaces.c.trashed_by, ) -assert set(Workspace.model_fields).issubset( - {c.name for c in _WORKSPACE_SELECTION_COLS} # nosec -), "part of the data-model is one-to-one to the domain model" -assert set(WorkspaceUpdates.model_fields).issubset( # nosec - c.name for c in workspaces.columns -), "part of the data-model is one-to-one to the domain model" - async def create_workspace( app: web.Application, diff --git a/services/web/server/tests/unit/with_dbs/03/test_trash.py b/services/web/server/tests/unit/with_dbs/03/test_trash.py index 2a1b71d7b64..c24074355c1 100644 --- a/services/web/server/tests/unit/with_dbs/03/test_trash.py +++ b/services/web/server/tests/unit/with_dbs/03/test_trash.py @@ -553,7 +553,7 @@ async def test_trash_empty_workspace( _exclude_attrs = {"trashed_by", "trashed_at", "modified_at"} # TRASH - before_trash = arrow.utcnow().datetime + trashing_at = arrow.utcnow().datetime resp = await client.post(f"/v0/workspaces/{workspace.workspace_id}:trash") await assert_status(resp, status.HTTP_204_NO_CONTENT) @@ -574,8 +574,8 @@ async def test_trash_empty_workspace( exclude=_exclude_attrs ) assert page.data[0].trashed_at is not None - assert before_trash < page.data[0].trashed_at - assert page.data[0].trashed_by == logged_user["id"] + assert trashing_at < page.data[0].trashed_at + assert page.data[0].trashed_by == logged_user["primary_gid"] # -------- From a51dedf4108258327ca29ca0d592af1ee55c0e52 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Fri, 17 Jan 2025 13:26:57 +0100 Subject: [PATCH 11/44] fixes tests --- .../test_api_schemas_webserver_projects.py | 8 +++--- .../simcore_webserver_projects_rest_api.py | 22 +++++++++------ .../tests/mocks/get_job_outputs.json | 1 + .../workspaces/_workspaces_repository.py | 27 ++++++++++++------- .../tests/unit/with_dbs/03/test_trash.py | 6 ++--- 5 files changed, 39 insertions(+), 25 deletions(-) diff --git a/packages/models-library/tests/test_api_schemas_webserver_projects.py b/packages/models-library/tests/test_api_schemas_webserver_projects.py index 295e9ee2304..acd7a5fa443 100644 --- a/packages/models-library/tests/test_api_schemas_webserver_projects.py +++ b/packages/models-library/tests/test_api_schemas_webserver_projects.py @@ -30,7 +30,7 @@ @pytest.mark.parametrize( "api_call", - (NEW_PROJECT, CREATE_FROM_SERVICE, CREATE_FROM_TEMPLATE), + [NEW_PROJECT, CREATE_FROM_SERVICE, CREATE_FROM_TEMPLATE], ids=lambda c: c.name, ) def test_create_project_schemas(api_call: HttpApiCallCapture): @@ -45,7 +45,7 @@ def test_create_project_schemas(api_call: HttpApiCallCapture): @pytest.mark.parametrize( "api_call", - (LIST_PROJECTS,), + [LIST_PROJECTS], ids=lambda c: c.name, ) def test_list_project_schemas(api_call: HttpApiCallCapture): @@ -59,7 +59,7 @@ def test_list_project_schemas(api_call: HttpApiCallCapture): @pytest.mark.parametrize( "api_call", - (GET_PROJECT, CREATE_FROM_TEMPLATE__TASK_RESULT), + [GET_PROJECT, CREATE_FROM_TEMPLATE__TASK_RESULT], ids=lambda c: c.name, ) def test_get_project_schemas(api_call: HttpApiCallCapture): @@ -74,7 +74,7 @@ def test_get_project_schemas(api_call: HttpApiCallCapture): @pytest.mark.parametrize( "api_call", - (REPLACE_PROJECT, REPLACE_PROJECT_ON_MODIFIED), + [REPLACE_PROJECT, REPLACE_PROJECT_ON_MODIFIED], ids=lambda c: c.name, ) def test_replace_project_schemas(api_call: HttpApiCallCapture): diff --git a/packages/pytest-simcore/src/pytest_simcore/simcore_webserver_projects_rest_api.py b/packages/pytest-simcore/src/pytest_simcore/simcore_webserver_projects_rest_api.py index fd6dd234720..88538b6b21b 100644 --- a/packages/pytest-simcore/src/pytest_simcore/simcore_webserver_projects_rest_api.py +++ b/packages/pytest-simcore/src/pytest_simcore/simcore_webserver_projects_rest_api.py @@ -76,7 +76,8 @@ def request_desc(self) -> str: "dev": None, "workspace_id": None, "folder_id": None, - "trashed_at": None, + "trashedAt": None, + "trashed_by": None, }, "error": None, }, @@ -102,7 +103,6 @@ def request_desc(self) -> str: "workbench": {}, "workspaceId": 123, "folderId": 2, - "trashedAt": "2021-12-06T10:13:18.100Z", "accessRights": {"2": {"read": True, "write": True, "delete": True}}, "dev": {}, "classifiers": [], @@ -115,7 +115,8 @@ def request_desc(self) -> str: }, "workspace_id": None, "folder_id": None, - "trashed_at": None, + "trashedAt": "2021-12-06T10:13:18.100Z", + "trashedBy": 3, } }, ) @@ -157,7 +158,8 @@ def request_desc(self) -> str: }, "workspace_id": None, "folder_id": None, - "trashed_at": None, + "trashedAt": None, + "trashed_by": None, } }, ) @@ -287,7 +289,8 @@ def request_desc(self) -> str: }, "workspace_id": None, "folder_id": None, - "trashed_at": None, + "trashedAt": None, + "trashed_by": None, } }, ) @@ -480,7 +483,8 @@ def request_desc(self) -> str: "dev": {}, "workspace_id": None, "folder_id": None, - "trashed_at": None, + "trashedAt": None, + "trashed_by": None, "classifiers": [], "ui": { "mode": "workbench", @@ -681,7 +685,8 @@ def request_desc(self) -> str: "dev": {}, "workspace_id": None, "folder_id": None, - "trashed_at": None, + "trashedAt": None, + "trashed_by": None, "quality": { "enabled": True, "tsr_target": { @@ -931,7 +936,8 @@ def request_desc(self) -> str: "dev": {}, "workspace_id": None, "folder_id": None, - "trashed_at": None, + "trashedAt": None, + "trashedBy": None, "quality": { "enabled": True, "tsr_target": { diff --git a/services/api-server/tests/mocks/get_job_outputs.json b/services/api-server/tests/mocks/get_job_outputs.json index a53e1742e95..d7649ebc2c0 100644 --- a/services/api-server/tests/mocks/get_job_outputs.json +++ b/services/api-server/tests/mocks/get_job_outputs.json @@ -147,6 +147,7 @@ "workspaceId": 5, "folderId": 2, "trashedAt": null, + "trashedBy": null, "workbench": { "dd875b4f-7663-529f-bd7f-3716b19e28af": { "key": "simcore/services/comp/itis/sleeper", diff --git a/services/web/server/src/simcore_service_webserver/workspaces/_workspaces_repository.py b/services/web/server/src/simcore_service_webserver/workspaces/_workspaces_repository.py index 4a01091bcf1..3a9690a3195 100644 --- a/services/web/server/src/simcore_service_webserver/workspaces/_workspaces_repository.py +++ b/services/web/server/src/simcore_service_webserver/workspaces/_workspaces_repository.py @@ -82,7 +82,7 @@ async def create_workspace( return Workspace.model_validate(row) -def _create_base_query(caller_user_id: UserID, product_name: ProductName): +def _create_base_select_query(caller_user_id: UserID, product_name: ProductName): # any other access access_rights_subquery = ( select( @@ -138,29 +138,34 @@ async def list_workspaces_for_user( limit: NonNegativeInt, order_by: OrderBy, ) -> tuple[int, list[UserWorkspaceWithAccessRights]]: - base_query = _create_base_query(caller_user_id=user_id, product_name=product_name) + base_select_query = _create_base_select_query( + caller_user_id=user_id, product_name=product_name + ) if filter_trashed is not None: - base_query = base_query.where( + base_select_query = base_select_query.where( workspaces.c.trashed.is_not(None) if filter_trashed else workspaces.c.trashed.is_(None) ) if filter_by_text is not None: - base_query = base_query.where( + base_select_query = base_select_query.where( (workspaces.c.name.ilike(f"%{filter_by_text}%")) | (workspaces.c.description.ilike(f"%{filter_by_text}%")) ) # Select total count from base_query - subquery = base_query.subquery() - count_query = select(func.count()).select_from(subquery) + count_query = select(func.count()).select_from(base_select_query.subquery()) # Ordering and pagination if order_by.direction == OrderDirection.ASC: - list_query = base_query.order_by(asc(getattr(workspaces.c, order_by.field))) + list_query = base_select_query.order_by( + asc(getattr(workspaces.c, order_by.field)) + ) else: - list_query = base_query.order_by(desc(getattr(workspaces.c, order_by.field))) + list_query = base_select_query.order_by( + desc(getattr(workspaces.c, order_by.field)) + ) list_query = list_query.offset(offset).limit(limit) async with pass_or_acquire_connection(get_asyncpg_engine(app), connection) as conn: @@ -182,10 +187,12 @@ async def get_workspace_for_user( workspace_id: WorkspaceID, product_name: ProductName, ) -> UserWorkspaceWithAccessRights: - base_query = _create_base_query(caller_user_id=user_id, product_name=product_name) + select_query = _create_base_select_query( + caller_user_id=user_id, product_name=product_name + ) async with pass_or_acquire_connection(get_asyncpg_engine(app), connection) as conn: - result = await conn.stream(base_query) + result = await conn.stream(select_query) row = await result.first() if row is None: raise WorkspaceAccessForbiddenError( diff --git a/services/web/server/tests/unit/with_dbs/03/test_trash.py b/services/web/server/tests/unit/with_dbs/03/test_trash.py index c24074355c1..ea142b11efd 100644 --- a/services/web/server/tests/unit/with_dbs/03/test_trash.py +++ b/services/web/server/tests/unit/with_dbs/03/test_trash.py @@ -513,12 +513,12 @@ async def workspace( # CREATE a workspace resp = await client.post("/v0/workspaces", json={"name": "My first workspace"}) data, _ = await assert_status(resp, status.HTTP_201_CREATED) - workspace = WorkspaceGet.model_validate(data) + wks = WorkspaceGet.model_validate(data) - yield workspace + yield wks # DELETE a workspace - resp = await client.delete(f"/v0/workspaces/{workspace.workspace_id}") + resp = await client.delete(f"/v0/workspaces/{wks.workspace_id}") data, _ = await assert_status(resp, status.HTTP_204_NO_CONTENT) From 9582a27808c231bdb44d628e26a77a1bf8fd22f8 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Fri, 17 Jan 2025 13:58:14 +0100 Subject: [PATCH 12/44] folders trashed-by --- .../api_schemas_webserver/folders_v2.py | 31 +++++++-- .../src/models_library/folders.py | 1 + .../folders/_folders_repository.py | 47 +++++++------ .../folders/_folders_service.py | 67 +++---------------- .../workspaces/_trash_services.py | 29 ++++---- .../tests/unit/with_dbs/03/test_trash.py | 6 +- 6 files changed, 74 insertions(+), 107 deletions(-) diff --git a/packages/models-library/src/models_library/api_schemas_webserver/folders_v2.py b/packages/models-library/src/models_library/api_schemas_webserver/folders_v2.py index ae26a2d4ae4..287ae64eb85 100644 --- a/packages/models-library/src/models_library/api_schemas_webserver/folders_v2.py +++ b/packages/models-library/src/models_library/api_schemas_webserver/folders_v2.py @@ -1,13 +1,12 @@ from datetime import datetime -from typing import NamedTuple +from typing import Annotated, NamedTuple, Self -from pydantic import ConfigDict, PositiveInt, field_validator +from pydantic import ConfigDict, Field, PositiveInt, field_validator from ..access_rights import AccessRights from ..basic_types import IDStr -from ..folders import FolderID +from ..folders import FolderDB, FolderID from ..groups import GroupID -from ..users import UserID from ..utils.common_validators import null_or_none_str_to_none_validator from ..workspaces import WorkspaceID from ._base import InputSchema, OutputSchema @@ -17,16 +16,34 @@ class FolderGet(OutputSchema): folder_id: FolderID parent_folder_id: FolderID | None = None name: str + created_at: datetime modified_at: datetime trashed_at: datetime | None - # NOTE: by design a folder is constraint to trashed_by == owner - # but we add this here for completeness - trashed_by: UserID | None + trashed_by: Annotated[ + GroupID | None, Field(description="The primary gid of the user who trashed") + ] owner: GroupID workspace_id: WorkspaceID | None my_access_rights: AccessRights + @classmethod + def from_domain_model( + cls, folder_db: FolderDB, user_folder_access_rights: AccessRights + ) -> Self: + return cls( + folder_id=folder_db.folder_id, + parent_folder_id=folder_db.parent_folder_id, + name=folder_db.name, + created_at=folder_db.created, + modified_at=folder_db.modified, + trashed_at=folder_db.trashed, + trashed_by=folder_db.trashed_by_primary_gid, + owner=folder_db.created_by_gid, + workspace_id=folder_db.workspace_id, + my_access_rights=user_folder_access_rights, + ) + class FolderGetPage(NamedTuple): items: list[FolderGet] diff --git a/packages/models-library/src/models_library/folders.py b/packages/models-library/src/models_library/folders.py index 24b1ed37315..af95bbc2766 100644 --- a/packages/models-library/src/models_library/folders.py +++ b/packages/models-library/src/models_library/folders.py @@ -52,6 +52,7 @@ class FolderDB(BaseModel): trashed: datetime | None trashed_by: UserID | None + trashed_by_primary_gid: GroupID | None trashed_explicitly: bool user_id: UserID | None # owner? diff --git a/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py b/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py index d098573063e..313101b954d 100644 --- a/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py +++ b/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py @@ -28,6 +28,7 @@ from simcore_postgres_database.models.folders_v2 import folders_v2 from simcore_postgres_database.models.projects import projects from simcore_postgres_database.models.projects_to_folders import projects_to_folders +from simcore_postgres_database.models.users import users from simcore_postgres_database.utils_repos import ( pass_or_acquire_connection, transaction_context, @@ -48,7 +49,7 @@ _unset: Final = UnSet() -_SELECTION_ARGS = ( +_FOLDERS_SELECTION_COLS = ( folders_v2.c.folder_id, folders_v2.c.name, folders_v2.c.parent_folder_id, @@ -91,7 +92,7 @@ async def create( created=func.now(), modified=func.now(), ) - .returning(*_SELECTION_ARGS) + .returning(*_FOLDERS_SELECTION_COLS) ) row = await result.first() return FolderDB.model_validate(row) @@ -109,7 +110,7 @@ def _create_private_workspace_query( ) return ( select( - *_SELECTION_ARGS, + *_FOLDERS_SELECTION_COLS, func.json_build_object( "read", sa.text("true"), @@ -146,7 +147,8 @@ def _create_shared_workspace_query( shared_workspace_query = ( select( - *_SELECTION_ARGS, workspace_access_rights_subquery.c.my_access_rights + *_FOLDERS_SELECTION_COLS, + workspace_access_rights_subquery.c.my_access_rights, ) .select_from( folders_v2.join( @@ -270,6 +272,20 @@ async def list_( # pylint: disable=too-many-arguments,too-many-branches return cast(int, total_count), folders +def _create_base_select_query(folder_id: FolderID, product_name: ProductName): + return ( + select( + *_FOLDERS_SELECTION_COLS, + users.c.primary_gid.label("trashed_by_primary_gid"), + ) + .select_from(folders_v2.outerjoin(users, folders_v2.c.trashed_by == users.c.id)) + .where( + (folders_v2.c.product_name == product_name) + & (folders_v2.c.folder_id == folder_id) + ) + ) + + async def get( app: web.Application, connection: AsyncConnection | None = None, @@ -277,14 +293,7 @@ async def get( folder_id: FolderID, product_name: ProductName, ) -> FolderDB: - query = ( - select(*_SELECTION_ARGS) - .select_from(folders_v2) - .where( - (folders_v2.c.product_name == product_name) - & (folders_v2.c.folder_id == folder_id) - ) - ) + query = _create_base_select_query(folder_id=folder_id, product_name=product_name) async with pass_or_acquire_connection(get_asyncpg_engine(app), connection) as conn: result = await conn.execute(query) @@ -309,16 +318,10 @@ async def get_for_user_or_workspace( user_id is not None and workspace_id is not None ), "Both user_id and workspace_id cannot be provided at the same time. Please provide only one." - query = ( - select(*_SELECTION_ARGS) - .select_from(folders_v2) - .where( - (folders_v2.c.product_name == product_name) - & (folders_v2.c.folder_id == folder_id) - ) - ) + query = _create_base_select_query(folder_id=folder_id, product_name=product_name) if user_id: + # ownership query = query.where(folders_v2.c.user_id == user_id) else: query = query.where(folders_v2.c.workspace_id == workspace_id) @@ -366,7 +369,7 @@ async def update( query = ( (folders_v2.update().values(modified=func.now(), **updated)) .where(folders_v2.c.product_name == product_name) - .returning(*_SELECTION_ARGS) + .returning(*_FOLDERS_SELECTION_COLS) ) if isinstance(folders_id_or_ids, set): @@ -583,4 +586,4 @@ async def get_folders_recursively( # Step 4: Execute the query to get all descendants final_query = select(folder_hierarchy_cte) result = await conn.stream(final_query) - return [FolderID(row[0]) async for row in result] + return cast(list[FolderID], [row.folder_id async for row in result]) diff --git a/services/web/server/src/simcore_service_webserver/folders/_folders_service.py b/services/web/server/src/simcore_service_webserver/folders/_folders_service.py index 396fcecf4e6..97e533fb591 100644 --- a/services/web/server/src/simcore_service_webserver/folders/_folders_service.py +++ b/services/web/server/src/simcore_service_webserver/folders/_folders_service.py @@ -86,18 +86,7 @@ async def create_folder( user_id=user_id if workspace_is_private else None, workspace_id=workspace_id, ) - return FolderGet( - folder_id=folder_db.folder_id, - parent_folder_id=folder_db.parent_folder_id, - name=folder_db.name, - created_at=folder_db.created, - modified_at=folder_db.modified, - trashed_at=folder_db.trashed, - trashed_by=folder_db.trashed_by, - owner=folder_db.created_by_gid, - workspace_id=workspace_id, - my_access_rights=user_folder_access_rights, - ) + return FolderGet.from_domain_model(folder_db, user_folder_access_rights) async def get_folder( @@ -130,18 +119,7 @@ async def get_folder( user_id=user_id if workspace_is_private else None, workspace_id=folder_db.workspace_id, ) - return FolderGet( - folder_id=folder_db.folder_id, - parent_folder_id=folder_db.parent_folder_id, - name=folder_db.name, - created_at=folder_db.created, - modified_at=folder_db.modified, - trashed_at=folder_db.trashed, - trashed_by=folder_db.trashed_by, - owner=folder_db.created_by_gid, - workspace_id=folder_db.workspace_id, - my_access_rights=user_folder_access_rights, - ) + return FolderGet.from_domain_model(folder_db, user_folder_access_rights) async def list_folders( @@ -181,17 +159,9 @@ async def list_folders( ) return FolderGetPage( items=[ - FolderGet( - folder_id=folder.folder_id, - parent_folder_id=folder.parent_folder_id, - name=folder.name, - created_at=folder.created, - modified_at=folder.modified, - trashed_at=folder.trashed, - trashed_by=folder.trashed_by, - owner=folder.created_by_gid, - workspace_id=folder.workspace_id, - my_access_rights=folder.my_access_rights, + FolderGet.from_domain_model( + folder, + folder.my_access_rights, ) for folder in folders ], @@ -226,17 +196,9 @@ async def list_folders_full_depth( ) return FolderGetPage( items=[ - FolderGet( - folder_id=folder.folder_id, - parent_folder_id=folder.parent_folder_id, - name=folder.name, - created_at=folder.created, - modified_at=folder.modified, - trashed_at=folder.trashed, - trashed_by=folder.trashed_by, - owner=folder.created_by_gid, - workspace_id=folder.workspace_id, - my_access_rights=folder.my_access_rights, + FolderGet.from_domain_model( + folder, + folder.my_access_rights, ) for folder in folders ], @@ -304,18 +266,7 @@ async def update_folder( parent_folder_id=parent_folder_id, product_name=product_name, ) - return FolderGet( - folder_id=folder_db.folder_id, - parent_folder_id=folder_db.parent_folder_id, - name=folder_db.name, - created_at=folder_db.created, - modified_at=folder_db.modified, - trashed_at=folder_db.trashed, - trashed_by=folder_db.trashed_by, - owner=folder_db.created_by_gid, - workspace_id=folder_db.workspace_id, - my_access_rights=user_folder_access_rights, - ) + return FolderGet.from_domain_model(folder_db, user_folder_access_rights) async def delete_folder( diff --git a/services/web/server/src/simcore_service_webserver/workspaces/_trash_services.py b/services/web/server/src/simcore_service_webserver/workspaces/_trash_services.py index 59f88f7ad90..611f3fb56e9 100644 --- a/services/web/server/src/simcore_service_webserver/workspaces/_trash_services.py +++ b/services/web/server/src/simcore_service_webserver/workspaces/_trash_services.py @@ -12,8 +12,7 @@ from ..db.plugin import get_asyncpg_engine from ..folders._trash_service import trash_folder, untrash_folder from ..projects._trash_service import trash_project, untrash_project -from ._workspaces_repository import update_workspace -from ._workspaces_service import check_user_workspace_access +from . import _workspaces_repository, _workspaces_service _logger = logging.getLogger(__name__) @@ -25,7 +24,7 @@ async def _check_exists_and_access( user_id: UserID, workspace_id: WorkspaceID, ): - await check_user_workspace_access( + await _workspaces_service.check_user_workspace_access( app=app, user_id=user_id, workspace_id=workspace_id, @@ -50,7 +49,7 @@ async def trash_workspace( async with transaction_context(get_asyncpg_engine(app)) as connection: # EXPLICIT trash - await update_workspace( + await _workspaces_repository.update_workspace( app, connection, product_name=product_name, @@ -59,10 +58,9 @@ async def trash_workspace( ) # IMPLICIT trash - child_folders: list[FolderID] = ( - [] + child_folders: list[FolderID] = [ # NOTE: follows up with https://github.com/ITISFoundation/osparc-simcore/issues/7034 - ) + ] for folder_id in child_folders: await trash_folder( @@ -73,10 +71,9 @@ async def trash_workspace( force_stop_first=force_stop_first, ) - child_projects: list[ProjectID] = ( - [] + child_projects: list[ProjectID] = [ # NOTE: follows up with https://github.com/ITISFoundation/osparc-simcore/issues/7034 - ) + ] for project_id in child_projects: await trash_project( @@ -102,7 +99,7 @@ async def untrash_workspace( async with transaction_context(get_asyncpg_engine(app)) as connection: # EXPLICIT UNtrash - await update_workspace( + await _workspaces_repository.update_workspace( app, connection, product_name=product_name, @@ -110,10 +107,9 @@ async def untrash_workspace( updates=WorkspaceUpdates(trashed=None, trashed_by=None), ) - child_folders: list[FolderID] = ( - [] + child_folders: list[FolderID] = [ # NOTE: follows up with https://github.com/ITISFoundation/osparc-simcore/issues/7034 - ) + ] for folder_id in child_folders: await untrash_folder( @@ -123,10 +119,9 @@ async def untrash_workspace( folder_id=folder_id, ) - child_projects: list[ProjectID] = ( - [] + child_projects: list[ProjectID] = [ # NOTE: follows up with https://github.com/ITISFoundation/osparc-simcore/issues/7034 - ) + ] for project_id in child_projects: await untrash_project( diff --git a/services/web/server/tests/unit/with_dbs/03/test_trash.py b/services/web/server/tests/unit/with_dbs/03/test_trash.py index ea142b11efd..2947f7778a2 100644 --- a/services/web/server/tests/unit/with_dbs/03/test_trash.py +++ b/services/web/server/tests/unit/with_dbs/03/test_trash.py @@ -448,19 +448,19 @@ async def test_trash_folder_with_content( data, _ = await assert_status(resp, status.HTTP_200_OK) got = FolderGet.model_validate(data) assert got.trashed_at is not None - assert got.trashed_by == logged_user["id"] + assert got.trashed_by == logged_user["primary_gid"] resp = await client.get(f"/v0/folders/{subfolder.folder_id}") data, _ = await assert_status(resp, status.HTTP_200_OK) got = FolderGet.model_validate(data) assert got.trashed_at is not None - assert got.trashed_by == logged_user["id"] + assert got.trashed_by == logged_user["primary_gid"] resp = await client.get(f"/v0/projects/{project_uuid}") data, _ = await assert_status(resp, status.HTTP_200_OK) got = ProjectGet.model_validate(data) assert got.trashed_at is not None - assert got.trashed_by == logged_user["id"] + assert got.trashed_by == logged_user["primary_gid"] # UNTRASH folder resp = await client.post(f"/v0/folders/{folder.folder_id}:untrash") From 908e4533b3ae57b79bc95f7c52a94e1114871dda Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Fri, 17 Jan 2025 14:39:16 +0100 Subject: [PATCH 13/44] domain models in folders --- .../api_schemas_webserver/folders_v2.py | 11 ++--- .../src/models_library/folders.py | 9 +++- .../folders/_folders_rest.py | 30 +++++++------ .../folders/_folders_service.py | 43 +++++++++---------- ...handlers__clone_in_workspace_and_folder.py | 2 +- .../tests/unit/with_dbs/03/test_trash.py | 4 +- 6 files changed, 51 insertions(+), 48 deletions(-) diff --git a/packages/models-library/src/models_library/api_schemas_webserver/folders_v2.py b/packages/models-library/src/models_library/api_schemas_webserver/folders_v2.py index 287ae64eb85..ed85519a00f 100644 --- a/packages/models-library/src/models_library/api_schemas_webserver/folders_v2.py +++ b/packages/models-library/src/models_library/api_schemas_webserver/folders_v2.py @@ -1,7 +1,7 @@ from datetime import datetime -from typing import Annotated, NamedTuple, Self +from typing import Annotated, Self -from pydantic import ConfigDict, Field, PositiveInt, field_validator +from pydantic import ConfigDict, Field, field_validator from ..access_rights import AccessRights from ..basic_types import IDStr @@ -31,7 +31,7 @@ class FolderGet(OutputSchema): def from_domain_model( cls, folder_db: FolderDB, user_folder_access_rights: AccessRights ) -> Self: - return cls( + return cls.model_construct( folder_id=folder_db.folder_id, parent_folder_id=folder_db.parent_folder_id, name=folder_db.name, @@ -45,11 +45,6 @@ def from_domain_model( ) -class FolderGetPage(NamedTuple): - items: list[FolderGet] - total: PositiveInt - - class FolderCreateBodyParams(InputSchema): name: IDStr parent_folder_id: FolderID | None = None diff --git a/packages/models-library/src/models_library/folders.py b/packages/models-library/src/models_library/folders.py index af95bbc2766..98405026d90 100644 --- a/packages/models-library/src/models_library/folders.py +++ b/packages/models-library/src/models_library/folders.py @@ -1,6 +1,6 @@ from datetime import datetime from enum import auto -from typing import TypeAlias +from typing import NamedTuple, TypeAlias from pydantic import BaseModel, ConfigDict, PositiveInt, ValidationInfo, field_validator @@ -52,7 +52,7 @@ class FolderDB(BaseModel): trashed: datetime | None trashed_by: UserID | None - trashed_by_primary_gid: GroupID | None + trashed_by_primary_gid: GroupID | None = None trashed_explicitly: bool user_id: UserID | None # owner? @@ -64,3 +64,8 @@ class UserFolderAccessRightsDB(FolderDB): my_access_rights: AccessRights model_config = ConfigDict(from_attributes=True) + + +class Folder(NamedTuple): + folder_db: FolderDB + my_access_rights: AccessRights diff --git a/services/web/server/src/simcore_service_webserver/folders/_folders_rest.py b/services/web/server/src/simcore_service_webserver/folders/_folders_rest.py index 80da8bd21e8..6f568e95a49 100644 --- a/services/web/server/src/simcore_service_webserver/folders/_folders_rest.py +++ b/services/web/server/src/simcore_service_webserver/folders/_folders_rest.py @@ -4,9 +4,9 @@ from models_library.api_schemas_webserver.folders_v2 import ( FolderCreateBodyParams, FolderGet, - FolderGetPage, FolderReplaceBodyParams, ) +from models_library.folders import Folder from models_library.rest_ordering import OrderBy from models_library.rest_pagination import ItemT, Page from models_library.rest_pagination_utils import paginate_data @@ -54,7 +54,7 @@ async def create_folder(request: web.Request): req_ctx = FoldersRequestContext.model_validate(request) body_params = await parse_request_body_as(FolderCreateBodyParams, request) - folder = await _folders_service.create_folder( + folder: Folder = await _folders_service.create_folder( request.app, user_id=req_ctx.user_id, name=body_params.name, @@ -63,7 +63,10 @@ async def create_folder(request: web.Request): workspace_id=body_params.workspace_id, ) - return envelope_json_response(folder, web.HTTPCreated) + return envelope_json_response( + FolderGet.from_domain_model(folder.folder_db, folder.my_access_rights), + web.HTTPCreated, + ) @routes.get(f"/{VTAG}/folders", name="list_folders") @@ -79,7 +82,7 @@ async def list_folders(request: web.Request): if not query_params.filters: query_params.filters = FolderFilters() - folders: FolderGetPage = await _folders_service.list_folders( + folders, total_count = await _folders_service.list_folders( app=request.app, user_id=req_ctx.user_id, product_name=req_ctx.product_name, @@ -93,9 +96,9 @@ async def list_folders(request: web.Request): page = Page[FolderGet].model_validate( paginate_data( - chunk=folders.items, + chunk=[FolderGet.from_domain_model(*f) for f in folders], request_url=request.url, - total=folders.total, + total=total_count, limit=query_params.limit, offset=query_params.offset, ) @@ -116,7 +119,7 @@ async def list_folders_full_search(request: web.Request): if not query_params.filters: query_params.filters = FolderFilters() - folders: FolderGetPage = await _folders_service.list_folders_full_depth( + folders, total_count = await _folders_service.list_folders_full_depth( app=request.app, user_id=req_ctx.user_id, product_name=req_ctx.product_name, @@ -129,9 +132,9 @@ async def list_folders_full_search(request: web.Request): page = Page[FolderGet].model_validate( paginate_data( - chunk=folders.items, + chunk=[FolderGet.from_domain_model(*f) for f in folders], request_url=request.url, - total=folders.total, + total=total_count, limit=query_params.limit, offset=query_params.offset, ) @@ -147,14 +150,15 @@ async def get_folder(request: web.Request): req_ctx = FoldersRequestContext.model_validate(request) path_params = parse_request_path_parameters_as(FoldersPathParams, request) - folder: FolderGet = await _folders_service.get_folder( + folder: Folder = await _folders_service.get_folder( app=request.app, folder_id=path_params.folder_id, user_id=req_ctx.user_id, product_name=req_ctx.product_name, ) - - return envelope_json_response(folder) + return envelope_json_response( + FolderGet.from_domain_model(folder.folder_db, folder.my_access_rights) + ) @routes.put( @@ -169,7 +173,7 @@ async def replace_folder(request: web.Request): path_params = parse_request_path_parameters_as(FoldersPathParams, request) body_params = await parse_request_body_as(FolderReplaceBodyParams, request) - folder = await _folders_service.update_folder( + folder: Folder = await _folders_service.update_folder( app=request.app, user_id=req_ctx.user_id, folder_id=path_params.folder_id, diff --git a/services/web/server/src/simcore_service_webserver/folders/_folders_service.py b/services/web/server/src/simcore_service_webserver/folders/_folders_service.py index 97e533fb591..a6f130482f5 100644 --- a/services/web/server/src/simcore_service_webserver/folders/_folders_service.py +++ b/services/web/server/src/simcore_service_webserver/folders/_folders_service.py @@ -4,8 +4,7 @@ from aiohttp import web from models_library.access_rights import AccessRights -from models_library.api_schemas_webserver.folders_v2 import FolderGet, FolderGetPage -from models_library.folders import FolderID, FolderQuery, FolderScope +from models_library.folders import Folder, FolderID, FolderQuery, FolderScope from models_library.products import ProductName from models_library.projects import ProjectID from models_library.rest_ordering import OrderBy @@ -36,7 +35,7 @@ async def create_folder( parent_folder_id: FolderID | None, product_name: ProductName, workspace_id: WorkspaceID | None, -) -> FolderGet: +) -> Folder: user = await get_user(app, user_id=user_id) workspace_is_private = True @@ -86,7 +85,7 @@ async def create_folder( user_id=user_id if workspace_is_private else None, workspace_id=workspace_id, ) - return FolderGet.from_domain_model(folder_db, user_folder_access_rights) + return Folder(folder_db=folder_db, my_access_rights=user_folder_access_rights) async def get_folder( @@ -94,7 +93,7 @@ async def get_folder( user_id: UserID, folder_id: FolderID, product_name: ProductName, -) -> FolderGet: +) -> Folder: folder_db = await folders_db.get( app, folder_id=folder_id, product_name=product_name ) @@ -119,7 +118,7 @@ async def get_folder( user_id=user_id if workspace_is_private else None, workspace_id=folder_db.workspace_id, ) - return FolderGet.from_domain_model(folder_db, user_folder_access_rights) + return Folder(folder_db=folder_db, my_access_rights=user_folder_access_rights) async def list_folders( @@ -132,7 +131,7 @@ async def list_folders( offset: NonNegativeInt, limit: int, order_by: OrderBy, -) -> FolderGetPage: +) -> tuple[list[Folder], int]: # NOTE: Folder access rights for listing are checked within the listing DB function. total_count, folders = await folders_db.list_( @@ -157,15 +156,15 @@ async def list_folders( limit=limit, order_by=order_by, ) - return FolderGetPage( - items=[ - FolderGet.from_domain_model( - folder, - folder.my_access_rights, + return ( + [ + Folder( + folder_db=folder, + my_access_rights=folder.my_access_rights, ) for folder in folders ], - total=total_count, + total_count, ) @@ -179,7 +178,7 @@ async def list_folders_full_depth( offset: NonNegativeInt, limit: int, order_by: OrderBy, -) -> FolderGetPage: +) -> tuple[list[Folder], int]: # NOTE: Folder access rights for listing are checked within the listing DB function. total_count, folders = await folders_db.list_( @@ -194,15 +193,15 @@ async def list_folders_full_depth( limit=limit, order_by=order_by, ) - return FolderGetPage( - items=[ - FolderGet.from_domain_model( - folder, - folder.my_access_rights, + return ( + [ + Folder( + folder_db=folder, + my_access_rights=folder.my_access_rights, ) for folder in folders ], - total=total_count, + total_count, ) @@ -214,7 +213,7 @@ async def update_folder( name: str, parent_folder_id: FolderID | None, product_name: ProductName, -) -> FolderGet: +) -> Folder: folder_db = await folders_db.get( app, folder_id=folder_id, product_name=product_name ) @@ -266,7 +265,7 @@ async def update_folder( parent_folder_id=parent_folder_id, product_name=product_name, ) - return FolderGet.from_domain_model(folder_db, user_folder_access_rights) + return Folder(folder_db, user_folder_access_rights) async def delete_folder( diff --git a/services/web/server/tests/unit/with_dbs/02/test_projects_crud_handlers__clone_in_workspace_and_folder.py b/services/web/server/tests/unit/with_dbs/02/test_projects_crud_handlers__clone_in_workspace_and_folder.py index 8d302b4a336..7f69fad580b 100644 --- a/services/web/server/tests/unit/with_dbs/02/test_projects_crud_handlers__clone_in_workspace_and_folder.py +++ b/services/web/server/tests/unit/with_dbs/02/test_projects_crud_handlers__clone_in_workspace_and_folder.py @@ -41,7 +41,7 @@ async def create_workspace_and_folder( product_name="osparc", ) - folder = await create_folder( + folder, _ = await create_folder( client.app, user_id=logged_user["id"], name="a", diff --git a/services/web/server/tests/unit/with_dbs/03/test_trash.py b/services/web/server/tests/unit/with_dbs/03/test_trash.py index 2947f7778a2..9b4d292c182 100644 --- a/services/web/server/tests/unit/with_dbs/03/test_trash.py +++ b/services/web/server/tests/unit/with_dbs/03/test_trash.py @@ -331,7 +331,7 @@ async def test_trash_single_folder(client: TestClient, logged_user: UserInfoDict assert got.trashed_at assert trashing_at < got.trashed_at assert got.trashed_at < arrow.utcnow().datetime - assert got.trashed_by == logged_user["id"] + assert got.trashed_by == logged_user["primary_gid"] assert got.owner == logged_user["primary_gid"] # LIST trashed @@ -460,7 +460,7 @@ async def test_trash_folder_with_content( data, _ = await assert_status(resp, status.HTTP_200_OK) got = ProjectGet.model_validate(data) assert got.trashed_at is not None - assert got.trashed_by == logged_user["primary_gid"] + assert got.trashed_by == logged_user["id"] # UNTRASH folder resp = await client.post(f"/v0/folders/{folder.folder_id}:untrash") From 4abded450fa47e30d78d3c5d4689de84919f25cb Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Fri, 17 Jan 2025 14:41:40 +0100 Subject: [PATCH 14/44] doc --- .../server/src/simcore_service_webserver/api/v0/openapi.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml index 84d455f2b54..97892230e19 100644 --- a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml +++ b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml @@ -10018,6 +10018,7 @@ components: minimum: 0 - type: 'null' title: Trashedby + description: The primary gid of the user who trashed owner: type: integer exclusiveMinimum: true @@ -15211,6 +15212,7 @@ components: minimum: 0 - type: 'null' title: Trashedby + description: The primary gid of the user who trashed myAccessRights: $ref: '#/components/schemas/AccessRights' accessRights: From 93dbfda87086da1b406b8c206135cbfa271c04da Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Fri, 17 Jan 2025 15:15:21 +0100 Subject: [PATCH 15/44] fixes tests --- services/api-server/tests/unit/conftest.py | 1 + .../src/simcore_service_webserver/folders/_folders_rest.py | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/services/api-server/tests/unit/conftest.py b/services/api-server/tests/unit/conftest.py index 2b994b2f612..0706a5b1d8f 100644 --- a/services/api-server/tests/unit/conftest.py +++ b/services/api-server/tests/unit/conftest.py @@ -521,6 +521,7 @@ def create_project_task(self, request: httpx.Request): "prjOwner": "owner@email.com", "dev": None, "trashed_at": None, + "trashed_by": None, **project_create, } ) diff --git a/services/web/server/src/simcore_service_webserver/folders/_folders_rest.py b/services/web/server/src/simcore_service_webserver/folders/_folders_rest.py index 6f568e95a49..e3460eeb8ae 100644 --- a/services/web/server/src/simcore_service_webserver/folders/_folders_rest.py +++ b/services/web/server/src/simcore_service_webserver/folders/_folders_rest.py @@ -181,7 +181,9 @@ async def replace_folder(request: web.Request): parent_folder_id=body_params.parent_folder_id, product_name=req_ctx.product_name, ) - return envelope_json_response(folder) + return envelope_json_response( + FolderGet.from_domain_model(folder.folder_db, folder.my_access_rights) + ) @routes.delete( From d9c57cd9eac6e4c76acd131887f5ea0f1da503b9 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Fri, 17 Jan 2025 18:04:59 +0100 Subject: [PATCH 16/44] projects schema, domain and data models --- .../api_schemas_webserver/projects.py | 18 ++++++++++++++---- .../src/models_library/projects.py | 7 ++++++- .../models/projects.py | 2 +- .../api/v0/openapi.yaml | 2 ++ 4 files changed, 23 insertions(+), 6 deletions(-) diff --git a/packages/models-library/src/models_library/api_schemas_webserver/projects.py b/packages/models-library/src/models_library/api_schemas_webserver/projects.py index bf949bb8b9c..f0269367fa0 100644 --- a/packages/models-library/src/models_library/api_schemas_webserver/projects.py +++ b/packages/models-library/src/models_library/api_schemas_webserver/projects.py @@ -26,7 +26,6 @@ from ..projects_access import AccessRights, GroupIDStr from ..projects_state import ProjectState from ..projects_ui import StudyUI -from ..users import UserID from ..utils._original_fastapi_encoders import jsonable_encoder from ..utils.common_validators import ( empty_str_to_none_pre_validator, @@ -35,6 +34,7 @@ ) from ..workspaces import WorkspaceID from ._base import EmptyModel, InputSchema, OutputSchema +from .groups import GroupID from .permalinks import ProjectPermalink @@ -97,7 +97,9 @@ class ProjectGet(OutputSchema): folder_id: FolderID | None trashed_at: datetime | None - trashed_by: UserID | None + trashed_by: Annotated[ + GroupID | None, Field(description="The primary gid of the user who trashed") + ] _empty_description = field_validator("description", mode="before")( none_to_empty_str_pre_validator @@ -109,8 +111,16 @@ class ProjectGet(OutputSchema): def from_domain_model(cls, project_data: dict[str, Any]) -> Self: return cls.model_validate( remap_keys( - project_data, - rename={"trashed": "trashed_at"}, + { + k: v + for k, v in project_data.items() + if k not in {"trashed_by", "trashedBy"} + }, + rename={ + "trashed": "trashed_at", + "trashed_by_primary_gid": "trashed_by", + "trashedByPrimaryGid": "trashedBy", + }, ) ) diff --git a/packages/models-library/src/models_library/projects.py b/packages/models-library/src/models_library/projects.py index 81f230768cc..50f38673e4b 100644 --- a/packages/models-library/src/models_library/projects.py +++ b/packages/models-library/src/models_library/projects.py @@ -15,6 +15,7 @@ from .basic_regex import DATE_RE, UUID_RE_BASE from .emails import LowerCaseEmailStr +from .groups import GroupID from .projects_access import AccessRights, GroupIDStr from .projects_nodes import Node from .projects_nodes_io import NodeIDStr @@ -106,7 +107,7 @@ class ProjectAtDB(BaseProjectModel): @field_validator("project_type", mode="before") @classmethod - def convert_sql_alchemy_enum(cls, v): + def _convert_sql_alchemy_enum(cls, v): if isinstance(v, Enum): return v.value return v @@ -185,8 +186,12 @@ class Project(BaseProjectModel): trashed: datetime | None = None trashed_by: Annotated[UserID | None, Field(alias="trashedBy")] = None + trashed_by_primary_gid: Annotated[ + GroupID | None, Field(alias="trashedByPrimaryGid") + ] = None trashed_explicitly: Annotated[bool, Field(alias="trashedExplicitly")] = False model_config = ConfigDict( + # NOTE: this is a security measure until we get rid of the ProjectDict variants extra="forbid", ) diff --git a/packages/postgres-database/src/simcore_postgres_database/models/projects.py b/packages/postgres-database/src/simcore_postgres_database/models/projects.py index e13f9bc4221..d72cfd9b74e 100644 --- a/packages/postgres-database/src/simcore_postgres_database/models/projects.py +++ b/packages/postgres-database/src/simcore_postgres_database/models/projects.py @@ -93,7 +93,7 @@ class ProjectType(enum.Enum): JSONB, nullable=False, server_default=sa.text("'{}'::jsonb"), - doc="Read/write/delete access rights of each group (gid) on this project", + doc="DEPRECATED: Read/write/delete access rights of each group (gid) on this project", ), sa.Column( "workbench", diff --git a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml index 97892230e19..07ed5efb592 100644 --- a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml +++ b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml @@ -12616,6 +12616,7 @@ components: minimum: 0 - type: 'null' title: Trashedby + description: The primary gid of the user who trashed type: object required: - uuid @@ -12872,6 +12873,7 @@ components: minimum: 0 - type: 'null' title: Trashedby + description: The primary gid of the user who trashed type: object required: - uuid From 92951c03d529aed3621083f6584f1091b7417074 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Fri, 17 Jan 2025 18:15:43 +0100 Subject: [PATCH 17/44] projects trashed by primary gid --- .../projects/_db_utils.py | 33 +++++-------------- .../projects/_projects_db.py | 24 +++++++++----- .../simcore_service_webserver/projects/db.py | 29 ++++++++-------- .../projects/models.py | 9 ++--- .../projects/projects_service.py | 7 ++-- .../tests/unit/with_dbs/03/test_trash.py | 8 ++--- 6 files changed, 50 insertions(+), 60 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/projects/_db_utils.py b/services/web/server/src/simcore_service_webserver/projects/_db_utils.py index da1f06d06df..7ced6295ee5 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_db_utils.py +++ b/services/web/server/src/simcore_service_webserver/projects/_db_utils.py @@ -19,12 +19,12 @@ from simcore_postgres_database.models.projects_to_products import projects_to_products from simcore_postgres_database.webserver_models import ProjectType, projects from sqlalchemy.dialects.postgresql import insert as pg_insert -from sqlalchemy.sql import select from sqlalchemy.sql.selectable import CompoundSelect, Select from ..db.models import GroupType, groups, projects_tags, user_to_groups, users from ..users.exceptions import UserNotFoundError from ..utils import format_datetime +from ._projects_db import BASE_PROJECT_SELECT_ARGS from .exceptions import ( NodeNotFoundError, ProjectInvalidRightsError, @@ -130,7 +130,7 @@ async def _list_user_groups( user_groups.append(everyone_group) else: result = await conn.execute( - select(groups) + sa.select(groups) .select_from(groups.join(user_to_groups)) .where(user_to_groups.c.uid == user_id) ) @@ -255,7 +255,7 @@ async def _get_project( exclude_foreign = exclude_foreign or [] access_rights_subquery = ( - select( + sa.select( project_to_groups.c.project_uuid, sa.func.jsonb_object_agg( project_to_groups.c.gid, @@ -275,29 +275,14 @@ async def _get_project( query = ( sa.select( - projects.c.id, - projects.c.type, - projects.c.uuid, - projects.c.name, - projects.c.description, - projects.c.thumbnail, - projects.c.prj_owner, # == user.id (who created) - projects.c.creation_date, - projects.c.last_change_date, - projects.c.workbench, - projects.c.ui, - projects.c.classifiers, - projects.c.dev, - projects.c.quality, - projects.c.published, - projects.c.hidden, - projects.c.trashed, - projects.c.trashed_by, # == user.id (who trashed) - projects.c.trashed_explicitly, - projects.c.workspace_id, + *BASE_PROJECT_SELECT_ARGS, access_rights_subquery.c.access_rights, ) - .select_from(projects.join(access_rights_subquery, isouter=True)) + .select_from( + projects.join(access_rights_subquery, isouter=True).outerjoin( + users, projects.c.trashed_by == users.c.id + ) + ) .where( (projects.c.uuid == f"{project_uuid}") & ( diff --git a/services/web/server/src/simcore_service_webserver/projects/_projects_db.py b/services/web/server/src/simcore_service_webserver/projects/_projects_db.py index 083543f8fdd..7a65a8feda2 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_projects_db.py +++ b/services/web/server/src/simcore_service_webserver/projects/_projects_db.py @@ -3,8 +3,9 @@ import sqlalchemy as sa from aiohttp import web from models_library.projects import ProjectID +from simcore_postgres_database.models.projects import projects +from simcore_postgres_database.models.users import users from simcore_postgres_database.utils_repos import transaction_context -from simcore_postgres_database.webserver_models import projects from sqlalchemy.ext.asyncio import AsyncConnection from ..db.plugin import get_asyncpg_engine @@ -14,17 +15,17 @@ _logger = logging.getLogger(__name__) -# NOTE: MD: I intentionally didn't include the workbench. There is a special interface -# for the workbench, and at some point, this column should be removed from the table. -# The same holds true for access_rights/ui/classifiers/quality, but we have decided to proceed step by step. -_SELECTION_PROJECT_DB_ARGS = [ # noqa: RUF012 +PROJECT_WITHOUT_WORKBENCH_COLS = [ # noqa: RUF012 + # NOTE: MD: I intentionally didn't include the workbench. There is a special interface + # for the workbench, and at some point, this column should be removed from the table. + # The same holds true for access_rights/ui/classifiers/quality, but we have decided to proceed step by step. projects.c.id, projects.c.type, projects.c.uuid, projects.c.name, projects.c.description, projects.c.thumbnail, - projects.c.prj_owner, + projects.c.prj_owner, # == user.id (who created) projects.c.creation_date, projects.c.last_change_date, projects.c.ui, @@ -35,10 +36,17 @@ projects.c.hidden, projects.c.workspace_id, projects.c.trashed, - projects.c.trashed_by, + projects.c.trashed_by, # == user.id (who trashed) projects.c.trashed_explicitly, ] +BASE_PROJECT_SELECT_ARGS = [ + *PROJECT_WITHOUT_WORKBENCH_COLS, + projects.c.workbench, + users.c.primary_gid.label("trashed_by_primary_gid"), + # NOTE: needs `.outerjoin(users, projects.c.trashed_by == users.c.id)` +] + async def patch_project( app: web.Application, @@ -53,7 +61,7 @@ async def patch_project( projects.update() .values(last_change_date=sa.func.now(), **new_partial_project_data) .where(projects.c.uuid == f"{project_uuid}") - .returning(*_SELECTION_PROJECT_DB_ARGS) + .returning(*PROJECT_WITHOUT_WORKBENCH_COLS) ) row = await result.first() if row is None: diff --git a/services/web/server/src/simcore_service_webserver/projects/db.py b/services/web/server/src/simcore_service_webserver/projects/db.py index 995f699466c..3a584e569e2 100644 --- a/services/web/server/src/simcore_service_webserver/projects/db.py +++ b/services/web/server/src/simcore_service_webserver/projects/db.py @@ -85,7 +85,7 @@ patch_workbench, update_workbench, ) -from ._projects_db import _SELECTION_PROJECT_DB_ARGS +from ._projects_db import BASE_PROJECT_SELECT_ARGS, PROJECT_WITHOUT_WORKBENCH_COLS from .exceptions import ( ProjectDeleteError, ProjectInvalidRightsError, @@ -437,11 +437,7 @@ async def list_projects( # pylint: disable=too-many-arguments,too-many-statemen private_workspace_query = ( sa.select( - *[ - col - for col in projects.columns - if col.name not in ["access_rights"] - ], + *BASE_PROJECT_SELECT_ARGS, self.access_rights_subquery.c.access_rights, projects_to_products.c.product_name, projects_to_folders.c.folder_id, @@ -462,6 +458,7 @@ async def list_projects( # pylint: disable=too-many-arguments,too-many-statemen isouter=True, ) .join(project_tags_subquery, isouter=True) + .outerjoin(users, projects.c.trashed_by == users.c.id) ) .where( ( @@ -494,11 +491,7 @@ async def list_projects( # pylint: disable=too-many-arguments,too-many-statemen shared_workspace_query = ( sa.select( - *[ - col - for col in projects.columns - if col.name not in ["access_rights"] - ], + *BASE_PROJECT_SELECT_ARGS, workspace_access_rights_subquery.c.access_rights, projects_to_products.c.product_name, projects_to_folders.c.folder_id, @@ -523,6 +516,7 @@ async def list_projects( # pylint: disable=too-many-arguments,too-many-statemen isouter=True, ) .join(project_tags_subquery, isouter=True) + .outerjoin(users, projects.c.trashed_by == users.c.id) ) .where( ( @@ -699,9 +693,13 @@ async def get_project( async def get_project_db(self, project_uuid: ProjectID) -> ProjectDB: async with self.engine.acquire() as conn: result = await conn.execute( - sa.select(*_SELECTION_PROJECT_DB_ARGS).where( - projects.c.uuid == f"{project_uuid}" + sa.select( + *BASE_PROJECT_SELECT_ARGS, + ) + .select_from( + projects.outerjoin(users, projects.c.trashed_by == users.c.id) ) + .where(projects.c.uuid == f"{project_uuid}") ) row = await result.fetchone() if row is None: @@ -713,7 +711,10 @@ async def get_user_specific_project_data_db( ) -> UserSpecificProjectDataDB: async with self.engine.acquire() as conn: result = await conn.execute( - sa.select(*_SELECTION_PROJECT_DB_ARGS, projects_to_folders.c.folder_id) + sa.select( + *PROJECT_WITHOUT_WORKBENCH_COLS, + projects_to_folders.c.folder_id, + ) .select_from( projects.join( projects_to_folders, diff --git a/services/web/server/src/simcore_service_webserver/projects/models.py b/services/web/server/src/simcore_service_webserver/projects/models.py index 59a452bd48d..9607e625df8 100644 --- a/services/web/server/src/simcore_service_webserver/projects/models.py +++ b/services/web/server/src/simcore_service_webserver/projects/models.py @@ -6,6 +6,7 @@ from common_library.dict_tools import remap_keys from models_library.api_schemas_webserver.projects import ProjectPatch from models_library.folders import FolderID +from models_library.groups import GroupID from models_library.projects import ClassifierID, ProjectID from models_library.projects_ui import StudyUI from models_library.users import UserID @@ -15,7 +16,7 @@ ) from models_library.workspaces import WorkspaceID from pydantic import BaseModel, ConfigDict, HttpUrl, field_validator -from simcore_postgres_database.models.projects import ProjectType, projects +from simcore_postgres_database.models.projects import ProjectType ProjectDict: TypeAlias = dict[str, Any] ProjectProxy: TypeAlias = RowProxy @@ -54,6 +55,7 @@ class ProjectDB(BaseModel): workspace_id: WorkspaceID | None trashed: datetime | None trashed_by: UserID | None + trashed_by_primary_gid: GroupID | None = None trashed_explicitly: bool = False model_config = ConfigDict(from_attributes=True, arbitrary_types_allowed=True) @@ -73,11 +75,6 @@ class UserSpecificProjectDataDB(ProjectDB): model_config = ConfigDict(from_attributes=True) -assert set(ProjectDB.model_fields.keys()).issubset( # nosec - {c.name for c in projects.columns if c.name not in ["access_rights"]} -) - - class UserProjectAccessRightsDB(BaseModel): uid: UserID read: bool diff --git a/services/web/server/src/simcore_service_webserver/projects/projects_service.py b/services/web/server/src/simcore_service_webserver/projects/projects_service.py index 73eb734170a..1a059e2fb85 100644 --- a/services/web/server/src/simcore_service_webserver/projects/projects_service.py +++ b/services/web/server/src/simcore_service_webserver/projects/projects_service.py @@ -124,14 +124,13 @@ from ..wallets import api as wallets_api from ..wallets.errors import WalletNotEnoughCreditsError from ..workspaces import _workspaces_repository as workspaces_db -from . import _crud_api_delete, _nodes_api, _projects_db, _projects_nodes_repository +from . import _crud_api_delete, _nodes_api, _projects_db, _wallets_api, _projects_nodes_repository from ._access_rights_api import ( check_user_project_permission, has_user_project_access_rights, ) from ._db_utils import PermissionStr from ._nodes_utils import set_reservation_same_as_limit, validate_new_service_resources -from ._wallets_api import connect_wallet_to_project, get_project_wallet from .db import APP_PROJECT_DBAPI, ProjectDBAPI from .exceptions import ( ClustersKeeperNotAvailableError, @@ -626,7 +625,7 @@ async def _() -> None: and app_settings.WEBSERVER_CREDIT_COMPUTATION_ENABLED ): # Deal with Wallet - project_wallet = await get_project_wallet( + project_wallet = await _wallets_api.get_project_wallet( request.app, project_id=project_uuid ) if project_wallet is None: @@ -641,7 +640,7 @@ async def _() -> None: project_wallet_id = TypeAdapter(WalletID).validate_python( user_default_wallet_preference.value ) - await connect_wallet_to_project( + await _wallets_api.connect_wallet_to_project( request.app, product_name=product_name, project_id=project_uuid, diff --git a/services/web/server/tests/unit/with_dbs/03/test_trash.py b/services/web/server/tests/unit/with_dbs/03/test_trash.py index 9b4d292c182..eb598bbdb1d 100644 --- a/services/web/server/tests/unit/with_dbs/03/test_trash.py +++ b/services/web/server/tests/unit/with_dbs/03/test_trash.py @@ -151,7 +151,7 @@ async def test_trash_projects( # noqa: PLR0915 assert got.trashed_at assert trashing_at < got.trashed_at assert got.trashed_at < arrow.utcnow().datetime - assert got.trashed_by == logged_user["id"] + assert got.trashed_by == logged_user["primary_gid"] # LIST trashed resp = await client.get("/v0/projects", params={"filters": '{"trashed": true}'}) @@ -251,7 +251,7 @@ async def test_trash_projects_shared_among_users( assert page.data[0].uuid == project_uuid assert page.data[0].trashed_at assert trashing_at < page.data[0].trashed_at - assert page.data[0].trashed_by == logged_user["id"] + assert page.data[0].trashed_by == logged_user["primary_gid"] # Swith USER: LOGOUT url = client.app.router["auth_logout"].url_for() @@ -277,7 +277,7 @@ async def test_trash_projects_shared_among_users( assert page.data[0].uuid == project_uuid assert page.data[0].trashed_at assert trashing_at < page.data[0].trashed_at - assert page.data[0].trashed_by == logged_user["id"] + assert page.data[0].trashed_by == logged_user["primary_gid"] @pytest.mark.acceptance_test( @@ -460,7 +460,7 @@ async def test_trash_folder_with_content( data, _ = await assert_status(resp, status.HTTP_200_OK) got = ProjectGet.model_validate(data) assert got.trashed_at is not None - assert got.trashed_by == logged_user["id"] + assert got.trashed_by == logged_user["primary_gid"] # UNTRASH folder resp = await client.post(f"/v0/folders/{folder.folder_id}:untrash") From 2f574a524a00a32c85ef7479ce45c733ece555a5 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Fri, 17 Jan 2025 23:18:09 +0100 Subject: [PATCH 18/44] fix tests --- .../server/src/simcore_service_webserver/projects/_db_utils.py | 1 + 1 file changed, 1 insertion(+) diff --git a/services/web/server/src/simcore_service_webserver/projects/_db_utils.py b/services/web/server/src/simcore_service_webserver/projects/_db_utils.py index 7ced6295ee5..bf28e9dfd91 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_db_utils.py +++ b/services/web/server/src/simcore_service_webserver/projects/_db_utils.py @@ -65,6 +65,7 @@ def convert_to_db_names(project_document_data: dict) -> dict: "tags", "prjOwner", "folderId", + "trashedByPrimaryGid", ] # No column for tags, prjOwner is a foreign key in db for key, value in project_document_data.items(): if key not in exclude_keys: From fa42231ac900691523ed3283380a37ceb0f5c82e Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Fri, 17 Jan 2025 23:24:04 +0100 Subject: [PATCH 19/44] fix tests --- .../server/src/simcore_service_webserver/projects/_db_utils.py | 1 + 1 file changed, 1 insertion(+) diff --git a/services/web/server/src/simcore_service_webserver/projects/_db_utils.py b/services/web/server/src/simcore_service_webserver/projects/_db_utils.py index bf28e9dfd91..b5d90f8f13b 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_db_utils.py +++ b/services/web/server/src/simcore_service_webserver/projects/_db_utils.py @@ -66,6 +66,7 @@ def convert_to_db_names(project_document_data: dict) -> dict: "prjOwner", "folderId", "trashedByPrimaryGid", + "trashed_by_primary_gid", ] # No column for tags, prjOwner is a foreign key in db for key, value in project_document_data.items(): if key not in exclude_keys: From afbbc44081b99f4cea069dea0dfe5018c569af91 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Sat, 18 Jan 2025 11:09:47 +0100 Subject: [PATCH 20/44] fix mypy --- .../src/models_library/api_schemas_webserver/projects.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/models-library/src/models_library/api_schemas_webserver/projects.py b/packages/models-library/src/models_library/api_schemas_webserver/projects.py index f0269367fa0..7632ba9cbab 100644 --- a/packages/models-library/src/models_library/api_schemas_webserver/projects.py +++ b/packages/models-library/src/models_library/api_schemas_webserver/projects.py @@ -22,6 +22,7 @@ from ..basic_types import LongTruncatedStr, ShortTruncatedStr from ..emails import LowerCaseEmailStr from ..folders import FolderID +from ..groups import GroupID from ..projects import ClassifierID, DateTimeStr, NodesDict, ProjectID from ..projects_access import AccessRights, GroupIDStr from ..projects_state import ProjectState @@ -34,7 +35,6 @@ ) from ..workspaces import WorkspaceID from ._base import EmptyModel, InputSchema, OutputSchema -from .groups import GroupID from .permalinks import ProjectPermalink From c069123f18d3a26cd58662331a6597907c32d1b2 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 22 Jan 2025 15:15:55 +0100 Subject: [PATCH 21/44] fix bad merge --- .../projects/_crud_api_create.py | 1 + .../projects/projects_service.py | 9 +++++++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/projects/_crud_api_create.py b/services/web/server/src/simcore_service_webserver/projects/_crud_api_create.py index 307780abdde..aed8607e44d 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_crud_api_create.py +++ b/services/web/server/src/simcore_service_webserver/projects/_crud_api_create.py @@ -18,6 +18,7 @@ from pydantic import TypeAdapter from servicelib.aiohttp.long_running_tasks.server import TaskProgress from servicelib.mimetype_constants import MIMETYPE_APPLICATION_JSON +from servicelib.redis import with_project_locked from servicelib.rest_constants import RESPONSE_MODEL_POLICY from simcore_postgres_database.utils_projects_nodes import ( ProjectNode, diff --git a/services/web/server/src/simcore_service_webserver/projects/projects_service.py b/services/web/server/src/simcore_service_webserver/projects/projects_service.py index 1a059e2fb85..241270bf578 100644 --- a/services/web/server/src/simcore_service_webserver/projects/projects_service.py +++ b/services/web/server/src/simcore_service_webserver/projects/projects_service.py @@ -124,7 +124,13 @@ from ..wallets import api as wallets_api from ..wallets.errors import WalletNotEnoughCreditsError from ..workspaces import _workspaces_repository as workspaces_db -from . import _crud_api_delete, _nodes_api, _projects_db, _wallets_api, _projects_nodes_repository +from . import ( + _crud_api_delete, + _nodes_api, + _projects_db, + _projects_nodes_repository, + _wallets_api, +) from ._access_rights_api import ( check_user_project_permission, has_user_project_access_rights, @@ -147,7 +153,6 @@ ProjectStartsTooManyDynamicNodesError, ProjectTooManyProjectOpenedError, ) -from .lock import get_project_locked_state, is_project_locked, lock_project from .models import ProjectDict, ProjectPatchInternalExtended from .settings import ProjectsSettings, get_plugin_settings from .utils import extract_dns_without_default_port From a77e64362aec6c6f2237fb05dc99c750b655112f Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 22 Jan 2025 15:47:06 +0100 Subject: [PATCH 22/44] split ProjectDB --- .../api_schemas_webserver/projects.py | 6 +-- .../projects/_db_utils.py | 6 ++- .../projects/_projects_db.py | 33 ++------------- .../simcore_service_webserver/projects/db.py | 40 +++++++++++++++---- .../projects/models.py | 13 ++++-- 5 files changed, 52 insertions(+), 46 deletions(-) diff --git a/packages/models-library/src/models_library/api_schemas_webserver/projects.py b/packages/models-library/src/models_library/api_schemas_webserver/projects.py index 7632ba9cbab..2aa9eaeaed7 100644 --- a/packages/models-library/src/models_library/api_schemas_webserver/projects.py +++ b/packages/models-library/src/models_library/api_schemas_webserver/projects.py @@ -111,11 +111,7 @@ class ProjectGet(OutputSchema): def from_domain_model(cls, project_data: dict[str, Any]) -> Self: return cls.model_validate( remap_keys( - { - k: v - for k, v in project_data.items() - if k not in {"trashed_by", "trashedBy"} - }, + project_data, rename={ "trashed": "trashed_at", "trashed_by_primary_gid": "trashed_by", diff --git a/services/web/server/src/simcore_service_webserver/projects/_db_utils.py b/services/web/server/src/simcore_service_webserver/projects/_db_utils.py index b5d90f8f13b..ecc88a5a59e 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_db_utils.py +++ b/services/web/server/src/simcore_service_webserver/projects/_db_utils.py @@ -24,7 +24,7 @@ from ..db.models import GroupType, groups, projects_tags, user_to_groups, users from ..users.exceptions import UserNotFoundError from ..utils import format_datetime -from ._projects_db import BASE_PROJECT_SELECT_ARGS +from ._projects_db import PROJECT_DB_COLS from .exceptions import ( NodeNotFoundError, ProjectInvalidRightsError, @@ -277,7 +277,9 @@ async def _get_project( query = ( sa.select( - *BASE_PROJECT_SELECT_ARGS, + *PROJECT_DB_COLS, + projects.c.workbench, + users.c.primary_gid.label("trashed_by_primary_gid"), access_rights_subquery.c.access_rights, ) .select_from( diff --git a/services/web/server/src/simcore_service_webserver/projects/_projects_db.py b/services/web/server/src/simcore_service_webserver/projects/_projects_db.py index 7a65a8feda2..c6eeffe15d4 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_projects_db.py +++ b/services/web/server/src/simcore_service_webserver/projects/_projects_db.py @@ -4,7 +4,6 @@ from aiohttp import web from models_library.projects import ProjectID from simcore_postgres_database.models.projects import projects -from simcore_postgres_database.models.users import users from simcore_postgres_database.utils_repos import transaction_context from sqlalchemy.ext.asyncio import AsyncConnection @@ -15,36 +14,12 @@ _logger = logging.getLogger(__name__) -PROJECT_WITHOUT_WORKBENCH_COLS = [ # noqa: RUF012 +PROJECT_DB_COLS = [ # noqa: RUF012 # NOTE: MD: I intentionally didn't include the workbench. There is a special interface # for the workbench, and at some point, this column should be removed from the table. # The same holds true for access_rights/ui/classifiers/quality, but we have decided to proceed step by step. - projects.c.id, - projects.c.type, - projects.c.uuid, - projects.c.name, - projects.c.description, - projects.c.thumbnail, - projects.c.prj_owner, # == user.id (who created) - projects.c.creation_date, - projects.c.last_change_date, - projects.c.ui, - projects.c.classifiers, - projects.c.dev, - projects.c.quality, - projects.c.published, - projects.c.hidden, - projects.c.workspace_id, - projects.c.trashed, - projects.c.trashed_by, # == user.id (who trashed) - projects.c.trashed_explicitly, -] - -BASE_PROJECT_SELECT_ARGS = [ - *PROJECT_WITHOUT_WORKBENCH_COLS, - projects.c.workbench, - users.c.primary_gid.label("trashed_by_primary_gid"), - # NOTE: needs `.outerjoin(users, projects.c.trashed_by == users.c.id)` + projects.columns[field_name] + for field_name in ProjectDB.model_fields ] @@ -61,7 +36,7 @@ async def patch_project( projects.update() .values(last_change_date=sa.func.now(), **new_partial_project_data) .where(projects.c.uuid == f"{project_uuid}") - .returning(*PROJECT_WITHOUT_WORKBENCH_COLS) + .returning(*PROJECT_DB_COLS) ) row = await result.first() if row is None: diff --git a/services/web/server/src/simcore_service_webserver/projects/db.py b/services/web/server/src/simcore_service_webserver/projects/db.py index 3a584e569e2..753d0dc8dff 100644 --- a/services/web/server/src/simcore_service_webserver/projects/db.py +++ b/services/web/server/src/simcore_service_webserver/projects/db.py @@ -85,7 +85,7 @@ patch_workbench, update_workbench, ) -from ._projects_db import BASE_PROJECT_SELECT_ARGS, PROJECT_WITHOUT_WORKBENCH_COLS +from ._projects_db import PROJECT_DB_COLS from .exceptions import ( ProjectDeleteError, ProjectInvalidRightsError, @@ -226,7 +226,9 @@ def _reraise_if_not_unique_uuid_error(err: UniqueViolation): version=node_info.get("version"), label=node_info.get("label"), ) - for node_id, node_info in selected_values["workbench"].items() + for node_id, node_info in selected_values[ + "workbench" + ].items() } nodes = [ @@ -240,7 +242,9 @@ def _reraise_if_not_unique_uuid_error(err: UniqueViolation): label=node_info.get("label"), ), ) - for node_id, node_info in selected_values["workbench"].items() + for node_id, node_info in selected_values[ + "workbench" + ].items() ] await project_nodes_repo.add(conn, nodes=nodes) return selected_values @@ -437,7 +441,10 @@ async def list_projects( # pylint: disable=too-many-arguments,too-many-statemen private_workspace_query = ( sa.select( - *BASE_PROJECT_SELECT_ARGS, + *PROJECT_DB_COLS, + projects.c.workbench, + users.c.primary_gid.label("trashed_by_primary_gid"), + # NOTE: needs `.outerjoin(users, projects.c.trashed_by == users.c.id)` self.access_rights_subquery.c.access_rights, projects_to_products.c.product_name, projects_to_folders.c.folder_id, @@ -491,7 +498,10 @@ async def list_projects( # pylint: disable=too-many-arguments,too-many-statemen shared_workspace_query = ( sa.select( - *BASE_PROJECT_SELECT_ARGS, + *PROJECT_DB_COLS, + projects.c.workbench, + users.c.primary_gid.label("trashed_by_primary_gid"), + # NOTE: needs `.outerjoin(users, projects.c.trashed_by == users.c.id)` workspace_access_rights_subquery.c.access_rights, projects_to_products.c.product_name, projects_to_folders.c.folder_id, @@ -694,7 +704,23 @@ async def get_project_db(self, project_uuid: ProjectID) -> ProjectDB: async with self.engine.acquire() as conn: result = await conn.execute( sa.select( - *BASE_PROJECT_SELECT_ARGS, + *PROJECT_DB_COLS, + projects.c.workbench, + ).where(projects.c.uuid == f"{project_uuid}") + ) + row = await result.fetchone() + if row is None: + raise ProjectNotFoundError(project_uuid=project_uuid) + return ProjectDB.model_validate(row) + + async def get_project_db2(self, project_uuid: ProjectID) -> ProjectDB: + async with self.engine.acquire() as conn: + result = await conn.execute( + sa.select( + *PROJECT_DB_COLS, + projects.c.workbench, + users.c.primary_gid.label("trashed_by_primary_gid"), + # NOTE: needs `.outerjoin(users, projects.c.trashed_by == users.c.id)` ) .select_from( projects.outerjoin(users, projects.c.trashed_by == users.c.id) @@ -712,7 +738,7 @@ async def get_user_specific_project_data_db( async with self.engine.acquire() as conn: result = await conn.execute( sa.select( - *PROJECT_WITHOUT_WORKBENCH_COLS, + *PROJECT_DB_COLS, projects_to_folders.c.folder_id, ) .select_from( diff --git a/services/web/server/src/simcore_service_webserver/projects/models.py b/services/web/server/src/simcore_service_webserver/projects/models.py index 9607e625df8..d9974b436b0 100644 --- a/services/web/server/src/simcore_service_webserver/projects/models.py +++ b/services/web/server/src/simcore_service_webserver/projects/models.py @@ -37,13 +37,14 @@ def to_project_type_db(cls, api_type: "ProjectTypeAPI") -> ProjectType | None: class ProjectDB(BaseModel): + # NOTE: model intented to read one-to-one columns of the `projects` table id: int type: ProjectType uuid: ProjectID name: str description: str thumbnail: HttpUrl | None - prj_owner: UserID + prj_owner: UserID # == user.id (who created) creation_date: datetime last_change_date: datetime ui: StudyUI | None @@ -53,11 +54,12 @@ class ProjectDB(BaseModel): published: bool hidden: bool workspace_id: WorkspaceID | None + trashed: datetime | None - trashed_by: UserID | None - trashed_by_primary_gid: GroupID | None = None + trashed_by: UserID | None # == user.id (who trashed) trashed_explicitly: bool = False + # config model_config = ConfigDict(from_attributes=True, arbitrary_types_allowed=True) # validators @@ -69,6 +71,11 @@ class ProjectDB(BaseModel): ) +class ProjectWithTrashExtra(ProjectDB): + # This field is not part of the tables + trashed_by_primary_gid: GroupID | None = None + + class UserSpecificProjectDataDB(ProjectDB): folder_id: FolderID | None From a0d8de7597d213e16a955c70308a0afef39245bf Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 22 Jan 2025 16:27:00 +0100 Subject: [PATCH 23/44] read functions returning ProjectDict --- .../simcore_postgres_database/utils_repos.py | 21 +++++++++++ .../projects/_crud_api_create.py | 2 +- .../projects/_crud_api_read.py | 6 +-- .../projects/_projects_access.py | 2 +- .../projects/_projects_db.py | 13 ++++--- .../simcore_service_webserver/projects/db.py | 37 +++---------------- .../projects/projects_service.py | 6 +-- .../studies_dispatcher/_studies_access.py | 4 +- 8 files changed, 45 insertions(+), 46 deletions(-) diff --git a/packages/postgres-database/src/simcore_postgres_database/utils_repos.py b/packages/postgres-database/src/simcore_postgres_database/utils_repos.py index efbdebc48f2..24f9b250476 100644 --- a/packages/postgres-database/src/simcore_postgres_database/utils_repos.py +++ b/packages/postgres-database/src/simcore_postgres_database/utils_repos.py @@ -1,7 +1,10 @@ import logging from collections.abc import AsyncIterator from contextlib import asynccontextmanager +from typing import TypeVar +import sqlalchemy as sa +from pydantic import BaseModel from sqlalchemy.ext.asyncio import AsyncConnection, AsyncEngine _logger = logging.getLogger(__name__) @@ -56,3 +59,21 @@ async def transaction_context( finally: assert not conn.closed # nosec assert not conn.in_transaction() # nosec + + +SQLModel = TypeVar( + # Towards using https://sqlmodel.tiangolo.com/#create-a-sqlmodel-model + "SQLModel", + bound="BaseModel", +) + + +def get_columns_from_db_model( + table: sa.Table, model_cls: type[SQLModel] +) -> list[sa.Column]: + """ + Usage example: + + query = sa.select( get_columns_from_db_model(project, ProjectDB) ) + """ + return [table.columns[field_name] for field_name in model_cls.model_fields] diff --git a/services/web/server/src/simcore_service_webserver/projects/_crud_api_create.py b/services/web/server/src/simcore_service_webserver/projects/_crud_api_create.py index aed8607e44d..97c92f666b0 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_crud_api_create.py +++ b/services/web/server/src/simcore_service_webserver/projects/_crud_api_create.py @@ -400,7 +400,7 @@ async def create_project( # pylint: disable=too-many-arguments,too-many-branche request.app, user_id, new_project["uuid"], product_name ) # get the latest state of the project (lastChangeDate for instance) - new_project, _ = await _projects_repository.get_project( + new_project, _ = await _projects_repository.get_project_dict_and_type( project_uuid=new_project["uuid"] ) # Appends state diff --git a/services/web/server/src/simcore_service_webserver/projects/_crud_api_read.py b/services/web/server/src/simcore_service_webserver/projects/_crud_api_read.py index 2c96a8d4cc0..ae7ef3f0e7e 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_crud_api_read.py +++ b/services/web/server/src/simcore_service_webserver/projects/_crud_api_read.py @@ -20,8 +20,8 @@ from ..catalog.client import get_services_for_user_in_product from ..folders import _folders_repository as folders_db from ..workspaces._workspaces_service import check_user_workspace_access -from ._permalink_api import update_or_pop_permalink_in_project from . import projects_service +from ._permalink_api import update_or_pop_permalink_in_project from .db import ProjectDBAPI from .models import ProjectDict, ProjectTypeAPI @@ -97,7 +97,7 @@ async def list_projects( # pylint: disable=too-many-arguments workspace_id=workspace_id, ) - db_projects, db_project_types, total_number_projects = await db.list_projects( + db_projects, db_project_types, total_number_projects = await db.list_projects_dicts( product_name=product_name, user_id=user_id, workspace_query=( @@ -166,7 +166,7 @@ async def list_projects_full_depth( request.app, user_id, product_name, only_key_versions=True ) - db_projects, db_project_types, total_number_projects = await db.list_projects( + db_projects, db_project_types, total_number_projects = await db.list_projects_dicts( product_name=product_name, user_id=user_id, workspace_query=WorkspaceQuery(workspace_scope=WorkspaceScope.ALL), diff --git a/services/web/server/src/simcore_service_webserver/projects/_projects_access.py b/services/web/server/src/simcore_service_webserver/projects/_projects_access.py index d16dceb9866..c0054dc5c3f 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_projects_access.py +++ b/services/web/server/src/simcore_service_webserver/projects/_projects_access.py @@ -30,7 +30,7 @@ async def can_update_node_inputs(context): permission="read", ) # get current version - current_project, _ = await db.get_project(project_uuid) + current_project, _ = await db.get_project_dict_and_type(project_uuid) diffs = jsondiff.diff(current_project, updated_project) diff --git a/services/web/server/src/simcore_service_webserver/projects/_projects_db.py b/services/web/server/src/simcore_service_webserver/projects/_projects_db.py index c6eeffe15d4..3bbed7462bd 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_projects_db.py +++ b/services/web/server/src/simcore_service_webserver/projects/_projects_db.py @@ -4,7 +4,10 @@ from aiohttp import web from models_library.projects import ProjectID from simcore_postgres_database.models.projects import projects -from simcore_postgres_database.utils_repos import transaction_context +from simcore_postgres_database.utils_repos import ( + get_columns_from_db_model, + transaction_context, +) from sqlalchemy.ext.asyncio import AsyncConnection from ..db.plugin import get_asyncpg_engine @@ -14,13 +17,13 @@ _logger = logging.getLogger(__name__) -PROJECT_DB_COLS = [ # noqa: RUF012 +PROJECT_DB_COLS = get_columns_from_db_model( # noqa: RUF012 # NOTE: MD: I intentionally didn't include the workbench. There is a special interface # for the workbench, and at some point, this column should be removed from the table. # The same holds true for access_rights/ui/classifiers/quality, but we have decided to proceed step by step. - projects.columns[field_name] - for field_name in ProjectDB.model_fields -] + projects, + ProjectDB, +) async def patch_project( diff --git a/services/web/server/src/simcore_service_webserver/projects/db.py b/services/web/server/src/simcore_service_webserver/projects/db.py index 753d0dc8dff..4fa5a872e79 100644 --- a/services/web/server/src/simcore_service_webserver/projects/db.py +++ b/services/web/server/src/simcore_service_webserver/projects/db.py @@ -349,7 +349,7 @@ async def upsert_project_linked_product( .on_conflict_do_nothing() ) - access_rights_subquery = ( + _access_rights_subquery = ( sa.select( project_to_groups.c.project_uuid, sa.func.jsonb_object_agg( @@ -370,7 +370,7 @@ async def upsert_project_linked_product( ).group_by(project_to_groups.c.project_uuid) ).subquery("access_rights_subquery") - async def list_projects( # pylint: disable=too-many-arguments,too-many-statements,too-many-branches + async def list_projects_dicts( # pylint: disable=too-many-arguments,too-many-statements,too-many-branches self, *, product_name: ProductName, @@ -393,7 +393,7 @@ async def list_projects( # pylint: disable=too-many-arguments,too-many-statemen limit: int | None = None, # order order_by: OrderBy = DEFAULT_ORDER_BY, - ) -> tuple[list[dict[str, Any]], list[ProjectType], int]: + ) -> tuple[list[ProjectDict], list[ProjectType], int]: if filter_tag_ids_list is None: filter_tag_ids_list = [] @@ -443,9 +443,7 @@ async def list_projects( # pylint: disable=too-many-arguments,too-many-statemen sa.select( *PROJECT_DB_COLS, projects.c.workbench, - users.c.primary_gid.label("trashed_by_primary_gid"), - # NOTE: needs `.outerjoin(users, projects.c.trashed_by == users.c.id)` - self.access_rights_subquery.c.access_rights, + self._access_rights_subquery.c.access_rights, projects_to_products.c.product_name, projects_to_folders.c.folder_id, sa.func.coalesce( @@ -454,7 +452,7 @@ async def list_projects( # pylint: disable=too-many-arguments,too-many-statemen ).label("tags"), ) .select_from( - projects.join(self.access_rights_subquery, isouter=True) + projects.join(self._access_rights_subquery, isouter=True) .join(projects_to_products) .join( projects_to_folders, @@ -465,7 +463,6 @@ async def list_projects( # pylint: disable=too-many-arguments,too-many-statemen isouter=True, ) .join(project_tags_subquery, isouter=True) - .outerjoin(users, projects.c.trashed_by == users.c.id) ) .where( ( @@ -500,8 +497,6 @@ async def list_projects( # pylint: disable=too-many-arguments,too-many-statemen sa.select( *PROJECT_DB_COLS, projects.c.workbench, - users.c.primary_gid.label("trashed_by_primary_gid"), - # NOTE: needs `.outerjoin(users, projects.c.trashed_by == users.c.id)` workspace_access_rights_subquery.c.access_rights, projects_to_products.c.product_name, projects_to_folders.c.folder_id, @@ -526,7 +521,6 @@ async def list_projects( # pylint: disable=too-many-arguments,too-many-statemen isouter=True, ) .join(project_tags_subquery, isouter=True) - .outerjoin(users, projects.c.trashed_by == users.c.id) ) .where( ( @@ -674,7 +668,7 @@ async def list_projects_uuids(self, user_id: int) -> list[str]: ) ] - async def get_project( + async def get_project_dict_and_type( self, project_uuid: str, *, @@ -713,25 +707,6 @@ async def get_project_db(self, project_uuid: ProjectID) -> ProjectDB: raise ProjectNotFoundError(project_uuid=project_uuid) return ProjectDB.model_validate(row) - async def get_project_db2(self, project_uuid: ProjectID) -> ProjectDB: - async with self.engine.acquire() as conn: - result = await conn.execute( - sa.select( - *PROJECT_DB_COLS, - projects.c.workbench, - users.c.primary_gid.label("trashed_by_primary_gid"), - # NOTE: needs `.outerjoin(users, projects.c.trashed_by == users.c.id)` - ) - .select_from( - projects.outerjoin(users, projects.c.trashed_by == users.c.id) - ) - .where(projects.c.uuid == f"{project_uuid}") - ) - row = await result.fetchone() - if row is None: - raise ProjectNotFoundError(project_uuid=project_uuid) - return ProjectDB.model_validate(row) - async def get_user_specific_project_data_db( self, project_uuid: ProjectID, private_workspace_user_id_or_none: UserID | None ) -> UserSpecificProjectDataDB: diff --git a/services/web/server/src/simcore_service_webserver/projects/projects_service.py b/services/web/server/src/simcore_service_webserver/projects/projects_service.py index 241270bf578..0e3b3a379cf 100644 --- a/services/web/server/src/simcore_service_webserver/projects/projects_service.py +++ b/services/web/server/src/simcore_service_webserver/projects/projects_service.py @@ -198,7 +198,7 @@ async def get_project_for_user( ) workspace_is_private = user_project_access.workspace_id is None - project, project_type = await db.get_project( + project, project_type = await db.get_project_dict_and_type( project_uuid, ) @@ -514,7 +514,7 @@ async def _check_project_node_has_all_required_inputs( permission="read", ) - project_dict, _ = await db.get_project(f"{project_uuid}") + project_dict, _ = await db.get_project_dict_and_type(f"{project_uuid}") nodes_map: dict[NodeID, Node] = { NodeID(k): Node(**v) for k, v in project_dict["workbench"].items() @@ -1035,7 +1035,7 @@ async def patch_project_node( # 2. If patching service key or version make sure it's valid if _node_patch_exclude_unset.get("key") or _node_patch_exclude_unset.get("version"): - _project, _ = await _projects_repository.get_project( + _project, _ = await _projects_repository.get_project_dict_and_type( project_uuid=f"{project_id}" ) _project_node_data = _project["workbench"][f"{node_id}"] diff --git a/services/web/server/src/simcore_service_webserver/studies_dispatcher/_studies_access.py b/services/web/server/src/simcore_service_webserver/studies_dispatcher/_studies_access.py index 812e03f961e..1dec4c84956 100644 --- a/services/web/server/src/simcore_service_webserver/studies_dispatcher/_studies_access.py +++ b/services/web/server/src/simcore_service_webserver/studies_dispatcher/_studies_access.py @@ -82,7 +82,7 @@ async def _get_published_template_project( only_public_projects = not is_user_authenticated try: - prj, _ = await db.get_project( + prj, _ = await db.get_project_dict_and_type( project_uuid=project_uuid, # NOTE: these are the conditions for a published study # 1. MUST be a template @@ -163,7 +163,7 @@ async def copy_study_to_account( ) # Avoids multiple copies of the same template on each account - await db.get_project(project_uuid) + await db.get_project_dict_and_type(project_uuid) except ProjectNotFoundError: # New project cloned from template From 007a1e6d02ad9f2bae2b21093c08d6fc525aa731 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 22 Jan 2025 17:59:54 +0100 Subject: [PATCH 24/44] list of project dict gets trashed_by_primary_gid --- .../api_schemas_webserver/projects.py | 7 ++++ .../projects/_crud_api_read.py | 34 +++++++++++++++---- .../projects/_crud_handlers.py | 10 +++--- .../projects/_projects_db.py | 32 +++++++++++++++++ 4 files changed, 71 insertions(+), 12 deletions(-) diff --git a/packages/models-library/src/models_library/api_schemas_webserver/projects.py b/packages/models-library/src/models_library/api_schemas_webserver/projects.py index 2aa9eaeaed7..33b74e2d842 100644 --- a/packages/models-library/src/models_library/api_schemas_webserver/projects.py +++ b/packages/models-library/src/models_library/api_schemas_webserver/projects.py @@ -5,6 +5,7 @@ """ +import copy from datetime import datetime from typing import Annotated, Any, Literal, Self, TypeAlias @@ -109,6 +110,12 @@ class ProjectGet(OutputSchema): @classmethod def from_domain_model(cls, project_data: dict[str, Any]) -> Self: + data = copy.copy(project_data) + # project_data["trashed_by"] is a UserID + # project_data["trashed_by_primary_gid"] is a GroupID + data.pop("trashed_by", None) + data.pop("trashedBy", None) + return cls.model_validate( remap_keys( project_data, diff --git a/services/web/server/src/simcore_service_webserver/projects/_crud_api_read.py b/services/web/server/src/simcore_service_webserver/projects/_crud_api_read.py index ae7ef3f0e7e..1bc76305cb8 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_crud_api_read.py +++ b/services/web/server/src/simcore_service_webserver/projects/_crud_api_read.py @@ -6,7 +6,6 @@ """ from aiohttp import web -from models_library.api_schemas_webserver.projects import ProjectListItem from models_library.folders import FolderID, FolderQuery, FolderScope from models_library.projects import ProjectID from models_library.rest_ordering import OrderBy @@ -16,6 +15,9 @@ from servicelib.utils import logged_gather from simcore_postgres_database.models.projects import ProjectType from simcore_postgres_database.webserver_models import ProjectType as ProjectTypeDB +from simcore_service_webserver.projects._projects_db import ( + get_trashed_by_primary_gid_from_project, +) from ..catalog.client import get_services_for_user_in_product from ..folders import _folders_repository as folders_db @@ -26,13 +28,13 @@ from .models import ProjectDict, ProjectTypeAPI -async def _append_item( +async def _update_project_dict( request: web.Request, *, user_id: UserID, project: ProjectDict, is_template: bool, -): +) -> ProjectDict: # state await projects_service.add_project_states_for_user( user_id=user_id, @@ -44,8 +46,22 @@ async def _append_item( # permalink await update_or_pop_permalink_in_project(request, project) - # validate - return ProjectListItem.from_domain_model(project).data(exclude_unset=True) + return project + + +async def _update_list_of_project_dict( + app: web.Application, list_of_project_dict: list[ProjectDict] +) -> list[ProjectDict]: + + # updating `trashed_by_primary_gid` + updates_values = await get_trashed_by_primary_gid_from_project( + app, projects_uuids=[ProjectID(p["uuid"]) for p in list_of_project_dict] + ) + + for project_dict, value in zip(list_of_project_dict, updates_values, strict=True): + project_dict.update(trashed_by_primary_gid=value) + + return list_of_project_dict async def list_projects( # pylint: disable=too-many-arguments @@ -127,9 +143,11 @@ async def list_projects( # pylint: disable=too-many-arguments order_by=order_by, ) + db_projects = await _update_list_of_project_dict(app, db_projects) + projects: list[ProjectDict] = await logged_gather( *( - _append_item( + _update_project_dict( request, user_id=user_id, project=prj, @@ -182,9 +200,11 @@ async def list_projects_full_depth( order_by=order_by, ) + db_projects = await _update_list_of_project_dict(request.app, db_projects) + projects: list[ProjectDict] = await logged_gather( *( - _append_item( + _update_project_dict( request, user_id=user_id, project=prj, diff --git a/services/web/server/src/simcore_service_webserver/projects/_crud_handlers.py b/services/web/server/src/simcore_service_webserver/projects/_crud_handlers.py index 57c0876fea6..0ab10a4327c 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_crud_handlers.py +++ b/services/web/server/src/simcore_service_webserver/projects/_crud_handlers.py @@ -14,6 +14,7 @@ ProjectCopyOverride, ProjectCreateNew, ProjectGet, + ProjectListItem, ProjectPatch, ) from models_library.generics import Envelope @@ -67,7 +68,6 @@ ProjectOwnerNotFoundInTheProjectAccessRightsError, WrongTagIdsInQueryError, ) -from .models import ProjectDict from .utils import get_project_unavailable_services, project_uses_available_services # When the user requests a project with a repo, the working copy might differ from @@ -212,9 +212,9 @@ async def list_projects(request: web.Request): order_by=OrderBy.model_construct(**query_params.order_by.model_dump()), ) - page = Page[ProjectDict].model_validate( + page = Page[ProjectListItem].model_validate( paginate_data( - chunk=projects, + chunk=[ProjectListItem.from_domain_model(prj) for prj in projects], request_url=request.url, total=total_number_of_projects, limit=query_params.limit, @@ -254,9 +254,9 @@ async def list_projects_full_search(request: web.Request): order_by=OrderBy.model_construct(**query_params.order_by.model_dump()), ) - page = Page[ProjectDict].model_validate( + page = Page[ProjectListItem].model_validate( paginate_data( - chunk=projects, + chunk=[ProjectListItem.from_domain_model(prj) for prj in projects], request_url=request.url, total=total_number_of_projects, limit=query_params.limit, diff --git a/services/web/server/src/simcore_service_webserver/projects/_projects_db.py b/services/web/server/src/simcore_service_webserver/projects/_projects_db.py index 3bbed7462bd..ebc83886a0d 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_projects_db.py +++ b/services/web/server/src/simcore_service_webserver/projects/_projects_db.py @@ -2,10 +2,13 @@ import sqlalchemy as sa from aiohttp import web +from models_library.groups import GroupID from models_library.projects import ProjectID from simcore_postgres_database.models.projects import projects +from simcore_postgres_database.models.users import users from simcore_postgres_database.utils_repos import ( get_columns_from_db_model, + pass_or_acquire_connection, transaction_context, ) from sqlalchemy.ext.asyncio import AsyncConnection @@ -45,3 +48,32 @@ async def patch_project( if row is None: raise ProjectNotFoundError(project_uuid=project_uuid) return ProjectDB.model_validate(row) + + +async def get_trashed_by_primary_gid_from_project( + app: web.Application, + connection: AsyncConnection | None = None, + *, + projects_uuids: list[ProjectID], +) -> list[GroupID | None]: + """ + Returns trashed_by as GroupID instead of UserID as is in the database + """ + async with pass_or_acquire_connection(get_asyncpg_engine(app), connection) as conn: + query = ( + sa.select( + users.c.primary_gid.label("trashed_by_primary_gid"), + ) + .select_from(projects.outerjoin(users, projects.c.trashed_by == users.c.id)) + .where(projects.c.uuid.in_(projects_uuids)) + ).order_by( + # Preserves the order of projects_uuids + sa.case( + *[ + (projects.c.uuid == uuid, index) + for index, uuid in enumerate(projects_uuids) + ] + ) + ) + result = await conn.stream(query) + return [row.trashed_by_primary_gid async for row in result] From 4d438e00127c36f821e96b42f915c015a90513e5 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 22 Jan 2025 18:07:47 +0100 Subject: [PATCH 25/44] list of project dict gets trashed_by_primary_gid --- .../projects/projects_service.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/services/web/server/src/simcore_service_webserver/projects/projects_service.py b/services/web/server/src/simcore_service_webserver/projects/projects_service.py index 0e3b3a379cf..9e69ffd19de 100644 --- a/services/web/server/src/simcore_service_webserver/projects/projects_service.py +++ b/services/web/server/src/simcore_service_webserver/projects/projects_service.py @@ -177,6 +177,7 @@ async def get_project_for_user( user_id: UserID, *, include_state: bool | None = False, + include_trashed_by_primary_gid: bool = False, check_permissions: str = "read", ) -> ProjectDict: """Returns a VALID project accessible to user @@ -215,6 +216,13 @@ async def get_project_for_user( user_id, project, project_type is ProjectType.TEMPLATE, app ) + if include_trashed_by_primary_gid and project.get("trashed_by") is not None: + _values = await _projects_db.get_trashed_by_primary_gid_from_project( + app, projects_uuids=[project["uuid"]] + ) + assert len(_values) == 1 + project.update(trashed_by_primary_gid=_values[0]) + if project["workspaceId"] is not None: workspace: UserWorkspaceWithAccessRights = ( await workspaces_db.get_workspace_for_user( From d5016e3a09c31d6ac6af0909aa9b8a477e7762bf Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 22 Jan 2025 18:45:45 +0100 Subject: [PATCH 26/44] doc --- .../src/simcore_postgres_database/utils_repos.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/postgres-database/src/simcore_postgres_database/utils_repos.py b/packages/postgres-database/src/simcore_postgres_database/utils_repos.py index 24f9b250476..5a54061d727 100644 --- a/packages/postgres-database/src/simcore_postgres_database/utils_repos.py +++ b/packages/postgres-database/src/simcore_postgres_database/utils_repos.py @@ -75,5 +75,13 @@ def get_columns_from_db_model( Usage example: query = sa.select( get_columns_from_db_model(project, ProjectDB) ) + + or + + query = ( + project.insert(). + # ... + .returning(*get_columns_from_db_model(project, ProjectDB)) + ) """ return [table.columns[field_name] for field_name in model_cls.model_fields] From 5905876391b0da7e55c01c3640b446873a92fea2 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 22 Jan 2025 18:46:29 +0100 Subject: [PATCH 27/44] handlers return item --- .../projects/_crud_api_read.py | 14 ++++++----- .../projects/_crud_handlers.py | 2 ++ .../projects/_projects_db.py | 23 +++++++++++++++++-- .../projects/projects_service.py | 9 ++++---- 4 files changed, 36 insertions(+), 12 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/projects/_crud_api_read.py b/services/web/server/src/simcore_service_webserver/projects/_crud_api_read.py index 1bc76305cb8..f97dea43633 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_crud_api_read.py +++ b/services/web/server/src/simcore_service_webserver/projects/_crud_api_read.py @@ -16,7 +16,7 @@ from simcore_postgres_database.models.projects import ProjectType from simcore_postgres_database.webserver_models import ProjectType as ProjectTypeDB from simcore_service_webserver.projects._projects_db import ( - get_trashed_by_primary_gid_from_project, + batch_get_trashed_by_primary_gid, ) from ..catalog.client import get_services_for_user_in_product @@ -49,16 +49,18 @@ async def _update_project_dict( return project -async def _update_list_of_project_dict( +async def _batch_update_list_of_project_dict( app: web.Application, list_of_project_dict: list[ProjectDict] ) -> list[ProjectDict]: # updating `trashed_by_primary_gid` - updates_values = await get_trashed_by_primary_gid_from_project( + trashed_by_primary_gid_values = await batch_get_trashed_by_primary_gid( app, projects_uuids=[ProjectID(p["uuid"]) for p in list_of_project_dict] ) - for project_dict, value in zip(list_of_project_dict, updates_values, strict=True): + for project_dict, value in zip( + list_of_project_dict, trashed_by_primary_gid_values, strict=True + ): project_dict.update(trashed_by_primary_gid=value) return list_of_project_dict @@ -143,7 +145,7 @@ async def list_projects( # pylint: disable=too-many-arguments order_by=order_by, ) - db_projects = await _update_list_of_project_dict(app, db_projects) + db_projects = await _batch_update_list_of_project_dict(app, db_projects) projects: list[ProjectDict] = await logged_gather( *( @@ -200,7 +202,7 @@ async def list_projects_full_depth( order_by=order_by, ) - db_projects = await _update_list_of_project_dict(request.app, db_projects) + db_projects = await _batch_update_list_of_project_dict(request.app, db_projects) projects: list[ProjectDict] = await logged_gather( *( diff --git a/services/web/server/src/simcore_service_webserver/projects/_crud_handlers.py b/services/web/server/src/simcore_service_webserver/projects/_crud_handlers.py index 0ab10a4327c..d48ceeaf736 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_crud_handlers.py +++ b/services/web/server/src/simcore_service_webserver/projects/_crud_handlers.py @@ -304,6 +304,7 @@ async def get_active_project(request: web.Request) -> web.Response: project_uuid=user_active_projects[0], user_id=req_ctx.user_id, include_state=True, + include_trashed_by_primary_gid=True, ) # updates project's permalink field @@ -344,6 +345,7 @@ async def get_project(request: web.Request): project_uuid=f"{path_params.project_id}", user_id=req_ctx.user_id, include_state=True, + include_trashed_by_primary_gid=True, ) if not await project_uses_available_services(project, user_available_services): unavilable_services = get_project_unavailable_services( diff --git a/services/web/server/src/simcore_service_webserver/projects/_projects_db.py b/services/web/server/src/simcore_service_webserver/projects/_projects_db.py index ebc83886a0d..956aa1f9eb3 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_projects_db.py +++ b/services/web/server/src/simcore_service_webserver/projects/_projects_db.py @@ -50,14 +50,33 @@ async def patch_project( return ProjectDB.model_validate(row) -async def get_trashed_by_primary_gid_from_project( +async def get_trashed_by_primary_gid( + app: web.Application, + connection: AsyncConnection | None = None, + *, + projects_uuid: ProjectID, +) -> GroupID | None: + async with pass_or_acquire_connection(get_asyncpg_engine(app), connection) as conn: + query = ( + sa.select( + users.c.primary_gid.label("trashed_by_primary_gid"), + ) + .select_from(projects.outerjoin(users, projects.c.trashed_by == users.c.id)) + .where(projects.c.uuid == projects_uuid) + ) + result = await conn.stream(query) + row = await result.first() + return row.trashed_by_primary_gid if row else None + + +async def batch_get_trashed_by_primary_gid( app: web.Application, connection: AsyncConnection | None = None, *, projects_uuids: list[ProjectID], ) -> list[GroupID | None]: """ - Returns trashed_by as GroupID instead of UserID as is in the database + Batch version of get_trashed_by_primary_gid preserving ORDER of projects_uuids """ async with pass_or_acquire_connection(get_asyncpg_engine(app), connection) as conn: query = ( diff --git a/services/web/server/src/simcore_service_webserver/projects/projects_service.py b/services/web/server/src/simcore_service_webserver/projects/projects_service.py index 9e69ffd19de..f889a7c5453 100644 --- a/services/web/server/src/simcore_service_webserver/projects/projects_service.py +++ b/services/web/server/src/simcore_service_webserver/projects/projects_service.py @@ -216,12 +216,13 @@ async def get_project_for_user( user_id, project, project_type is ProjectType.TEMPLATE, app ) + # adds `trashed_by_primary_gid` if include_trashed_by_primary_gid and project.get("trashed_by") is not None: - _values = await _projects_db.get_trashed_by_primary_gid_from_project( - app, projects_uuids=[project["uuid"]] + project.update( + trashed_by_primary_gid=await _projects_db.get_trashed_by_primary_gid( + app, projects_uuid=project["uuid"] + ) ) - assert len(_values) == 1 - project.update(trashed_by_primary_gid=_values[0]) if project["workspaceId"] is not None: workspace: UserWorkspaceWithAccessRights = ( From ced0e98078bd4171e9a89f0a7b75890aa7a11bc1 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 22 Jan 2025 19:27:17 +0100 Subject: [PATCH 28/44] by_alias since Page is strict --- .../tests/test_project_nodes.py | 1 + .../projects/_crud_api_create.py | 4 +- .../projects/_crud_handlers.py | 10 +++- .../projects/_projects_db.py | 52 ++++++++++--------- .../projects/projects_service.py | 8 ++- 5 files changed, 43 insertions(+), 32 deletions(-) diff --git a/packages/models-library/tests/test_project_nodes.py b/packages/models-library/tests/test_project_nodes.py index 09b5511e2bf..54a5d14bf24 100644 --- a/packages/models-library/tests/test_project_nodes.py +++ b/packages/models-library/tests/test_project_nodes.py @@ -25,6 +25,7 @@ def test_create_minimal_node(minimal_node_data_sample: dict[str, Any]): # a nice way to see how the simplest node looks like assert node.inputs == {} assert node.outputs == {} + assert node.state is not None assert node.state.current_status == RunningState.NOT_STARTED assert node.state.modified is True assert node.state.dependencies == set() diff --git a/services/web/server/src/simcore_service_webserver/projects/_crud_api_create.py b/services/web/server/src/simcore_service_webserver/projects/_crud_api_create.py index 97c92f666b0..d60ce232423 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_crud_api_create.py +++ b/services/web/server/src/simcore_service_webserver/projects/_crud_api_create.py @@ -440,7 +440,9 @@ async def create_project( # pylint: disable=too-many-arguments,too-many-branche for gid, access in workspace.access_rights.items() } - data = ProjectGet.from_domain_model(new_project).data(**RESPONSE_MODEL_POLICY) + data = ProjectGet.from_domain_model(new_project).model_dump( + **RESPONSE_MODEL_POLICY + ) raise web.HTTPCreated( text=json_dumps({"data": data}), diff --git a/services/web/server/src/simcore_service_webserver/projects/_crud_handlers.py b/services/web/server/src/simcore_service_webserver/projects/_crud_handlers.py index d48ceeaf736..fd92038194d 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_crud_handlers.py +++ b/services/web/server/src/simcore_service_webserver/projects/_crud_handlers.py @@ -214,7 +214,10 @@ async def list_projects(request: web.Request): page = Page[ProjectListItem].model_validate( paginate_data( - chunk=[ProjectListItem.from_domain_model(prj) for prj in projects], + chunk=[ + ProjectListItem.from_domain_model(prj).model_dump(by_alias=True) + for prj in projects + ], request_url=request.url, total=total_number_of_projects, limit=query_params.limit, @@ -256,7 +259,10 @@ async def list_projects_full_search(request: web.Request): page = Page[ProjectListItem].model_validate( paginate_data( - chunk=[ProjectListItem.from_domain_model(prj) for prj in projects], + chunk=[ + ProjectListItem.from_domain_model(prj).model_dump(by_alias=True) + for prj in projects + ], request_url=request.url, total=total_number_of_projects, limit=query_params.limit, diff --git a/services/web/server/src/simcore_service_webserver/projects/_projects_db.py b/services/web/server/src/simcore_service_webserver/projects/_projects_db.py index 956aa1f9eb3..9c1d1f839da 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_projects_db.py +++ b/services/web/server/src/simcore_service_webserver/projects/_projects_db.py @@ -56,14 +56,15 @@ async def get_trashed_by_primary_gid( *, projects_uuid: ProjectID, ) -> GroupID | None: - async with pass_or_acquire_connection(get_asyncpg_engine(app), connection) as conn: - query = ( - sa.select( - users.c.primary_gid.label("trashed_by_primary_gid"), - ) - .select_from(projects.outerjoin(users, projects.c.trashed_by == users.c.id)) - .where(projects.c.uuid == projects_uuid) + query = ( + sa.select( + users.c.primary_gid.label("trashed_by_primary_gid"), ) + .select_from(projects.outerjoin(users, projects.c.trashed_by == users.c.id)) + .where(projects.c.uuid == projects_uuid) + ) + + async with pass_or_acquire_connection(get_asyncpg_engine(app), connection) as conn: result = await conn.stream(query) row = await result.first() return row.trashed_by_primary_gid if row else None @@ -75,24 +76,27 @@ async def batch_get_trashed_by_primary_gid( *, projects_uuids: list[ProjectID], ) -> list[GroupID | None]: + """Batch version of get_trashed_by_primary_gid + + Returns: + values of trashed_by_primary_gid in the SAME ORDER as projects_uuids """ - Batch version of get_trashed_by_primary_gid preserving ORDER of projects_uuids - """ - async with pass_or_acquire_connection(get_asyncpg_engine(app), connection) as conn: - query = ( - sa.select( - users.c.primary_gid.label("trashed_by_primary_gid"), - ) - .select_from(projects.outerjoin(users, projects.c.trashed_by == users.c.id)) - .where(projects.c.uuid.in_(projects_uuids)) - ).order_by( - # Preserves the order of projects_uuids - sa.case( - *[ - (projects.c.uuid == uuid, index) - for index, uuid in enumerate(projects_uuids) - ] - ) + projects_uuids_str = [f"{uuid}" for uuid in projects_uuids] + query = ( + sa.select( + users.c.primary_gid.label("trashed_by_primary_gid"), ) + .select_from(projects.outerjoin(users, projects.c.trashed_by == users.c.id)) + .where(projects.c.uuid.in_(projects_uuids_str)) + ).order_by( + # Preserves the order of projects_uuids + sa.case( + *[ + (projects.c.uuid == uuid, index) + for index, uuid in enumerate(projects_uuids_str) + ] + ) + ) + async with pass_or_acquire_connection(get_asyncpg_engine(app), connection) as conn: result = await conn.stream(query) return [row.trashed_by_primary_gid async for row in result] diff --git a/services/web/server/src/simcore_service_webserver/projects/projects_service.py b/services/web/server/src/simcore_service_webserver/projects/projects_service.py index f889a7c5453..eb339955617 100644 --- a/services/web/server/src/simcore_service_webserver/projects/projects_service.py +++ b/services/web/server/src/simcore_service_webserver/projects/projects_service.py @@ -180,12 +180,10 @@ async def get_project_for_user( include_trashed_by_primary_gid: bool = False, check_permissions: str = "read", ) -> ProjectDict: - """Returns a VALID project accessible to user + """ + Raises: + ProjectNotFoundError: _description_ - :raises ProjectNotFoundError: if no match found - : - :return: schema-compliant project data - :rtype: Dict """ db = ProjectDBAPI.get_from_app_context(app) From b3cc85eb17483b2811472f52e9ce410bf8fb035b Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 22 Jan 2025 19:30:19 +0100 Subject: [PATCH 29/44] fixes test_proejcts_cancellations --- .../simcore_service_webserver/projects/_crud_handlers.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/projects/_crud_handlers.py b/services/web/server/src/simcore_service_webserver/projects/_crud_handlers.py index fd92038194d..fb66c4caea0 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_crud_handlers.py +++ b/services/web/server/src/simcore_service_webserver/projects/_crud_handlers.py @@ -215,7 +215,9 @@ async def list_projects(request: web.Request): page = Page[ProjectListItem].model_validate( paginate_data( chunk=[ - ProjectListItem.from_domain_model(prj).model_dump(by_alias=True) + ProjectListItem.from_domain_model(prj).model_dump( + by_alias=True, exclude_unset=True + ) for prj in projects ], request_url=request.url, @@ -260,7 +262,9 @@ async def list_projects_full_search(request: web.Request): page = Page[ProjectListItem].model_validate( paginate_data( chunk=[ - ProjectListItem.from_domain_model(prj).model_dump(by_alias=True) + ProjectListItem.from_domain_model(prj).model_dump( + by_alias=True, exclude_unset=True + ) for prj in projects ], request_url=request.url, From 4e069efb430b5b653538b7a0a546ed2bd49743e5 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 22 Jan 2025 19:54:43 +0100 Subject: [PATCH 30/44] fixes tests --- .../projects/_projects_db.py | 14 +++++++------- services/web/server/tests/conftest.py | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/projects/_projects_db.py b/services/web/server/src/simcore_service_webserver/projects/_projects_db.py index 9c1d1f839da..c61bc22a660 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_projects_db.py +++ b/services/web/server/src/simcore_service_webserver/projects/_projects_db.py @@ -81,7 +81,11 @@ async def batch_get_trashed_by_primary_gid( Returns: values of trashed_by_primary_gid in the SAME ORDER as projects_uuids """ + if not projects_uuids: + return [] + projects_uuids_str = [f"{uuid}" for uuid in projects_uuids] + query = ( sa.select( users.c.primary_gid.label("trashed_by_primary_gid"), @@ -89,13 +93,9 @@ async def batch_get_trashed_by_primary_gid( .select_from(projects.outerjoin(users, projects.c.trashed_by == users.c.id)) .where(projects.c.uuid.in_(projects_uuids_str)) ).order_by( - # Preserves the order of projects_uuids - sa.case( - *[ - (projects.c.uuid == uuid, index) - for index, uuid in enumerate(projects_uuids_str) - ] - ) + *[ + projects.c.uuid == uuid for uuid in projects_uuids_str + ] # Preserves the order of project_uuids ) async with pass_or_acquire_connection(get_asyncpg_engine(app), connection) as conn: result = await conn.stream(query) diff --git a/services/web/server/tests/conftest.py b/services/web/server/tests/conftest.py index ab862056e03..3322e0b8b31 100644 --- a/services/web/server/tests/conftest.py +++ b/services/web/server/tests/conftest.py @@ -439,7 +439,7 @@ async def _creator( "folderId", ] - for key in new_project: + for key in expected_data: if key not in modified_fields: assert expected_data[key] == new_project[key] From 294dc0bbc4c642b5879cf57e2505461257fa8290 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 22 Jan 2025 21:05:07 +0100 Subject: [PATCH 31/44] adapts folders --- .../api_schemas_webserver/folders_v2.py | 13 +- .../src/models_library/folders.py | 11 +- .../folders/_folders_repository.py | 91 +++++++++----- .../folders/_folders_rest.py | 26 ++-- .../folders/_folders_service.py | 112 +++++++++++++----- .../projects/_crud_api_create.py | 4 +- .../projects/_crud_api_read.py | 4 +- .../projects/_folders_api.py | 4 +- .../projects/_projects_db.py | 4 +- 9 files changed, 181 insertions(+), 88 deletions(-) diff --git a/packages/models-library/src/models_library/api_schemas_webserver/folders_v2.py b/packages/models-library/src/models_library/api_schemas_webserver/folders_v2.py index ed85519a00f..88333f0b0d9 100644 --- a/packages/models-library/src/models_library/api_schemas_webserver/folders_v2.py +++ b/packages/models-library/src/models_library/api_schemas_webserver/folders_v2.py @@ -29,16 +29,23 @@ class FolderGet(OutputSchema): @classmethod def from_domain_model( - cls, folder_db: FolderDB, user_folder_access_rights: AccessRights + cls, + folder_db: FolderDB, + trashed_by_primary_gid: GroupID | None, + user_folder_access_rights: AccessRights, ) -> Self: - return cls.model_construct( + if (folder_db.trashed_by is None) ^ (trashed_by_primary_gid is None): + msg = f"Incompatible inputs: {folder_db.trashed_by=} but not {trashed_by_primary_gid=}" + raise ValueError(msg) + + return cls( folder_id=folder_db.folder_id, parent_folder_id=folder_db.parent_folder_id, name=folder_db.name, created_at=folder_db.created, modified_at=folder_db.modified, trashed_at=folder_db.trashed, - trashed_by=folder_db.trashed_by_primary_gid, + trashed_by=trashed_by_primary_gid, owner=folder_db.created_by_gid, workspace_id=folder_db.workspace_id, my_access_rights=user_folder_access_rights, diff --git a/packages/models-library/src/models_library/folders.py b/packages/models-library/src/models_library/folders.py index 98405026d90..0333b463b34 100644 --- a/packages/models-library/src/models_library/folders.py +++ b/packages/models-library/src/models_library/folders.py @@ -36,11 +36,6 @@ def validate_folder_id(cls, value, info: ValidationInfo): return value -# -# DB -# - - class FolderDB(BaseModel): folder_id: FolderID name: str @@ -52,7 +47,6 @@ class FolderDB(BaseModel): trashed: datetime | None trashed_by: UserID | None - trashed_by_primary_gid: GroupID | None = None trashed_explicitly: bool user_id: UserID | None # owner? @@ -60,12 +54,13 @@ class FolderDB(BaseModel): model_config = ConfigDict(from_attributes=True) -class UserFolderAccessRightsDB(FolderDB): +class UserFolder(FolderDB): my_access_rights: AccessRights model_config = ConfigDict(from_attributes=True) -class Folder(NamedTuple): +class FolderTuple(NamedTuple): folder_db: FolderDB + trashed_by_primary_gid: GroupID | None my_access_rights: AccessRights diff --git a/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py b/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py index 313101b954d..bcf4367f015 100644 --- a/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py +++ b/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py @@ -16,7 +16,7 @@ FolderID, FolderQuery, FolderScope, - UserFolderAccessRightsDB, + UserFolder, ) from models_library.groups import GroupID from models_library.products import ProductName @@ -30,6 +30,7 @@ from simcore_postgres_database.models.projects_to_folders import projects_to_folders from simcore_postgres_database.models.users import users from simcore_postgres_database.utils_repos import ( + get_columns_from_db_model, pass_or_acquire_connection, transaction_context, ) @@ -49,19 +50,7 @@ _unset: Final = UnSet() -_FOLDERS_SELECTION_COLS = ( - folders_v2.c.folder_id, - folders_v2.c.name, - folders_v2.c.parent_folder_id, - folders_v2.c.created_by_gid, - folders_v2.c.created, - folders_v2.c.modified, - folders_v2.c.trashed, - folders_v2.c.trashed_by, - folders_v2.c.trashed_explicitly, - folders_v2.c.user_id, - folders_v2.c.workspace_id, -) +_FOLDER_DB_MODEL_COLS = get_columns_from_db_model(folders_v2, FolderDB) async def create( @@ -92,7 +81,7 @@ async def create( created=func.now(), modified=func.now(), ) - .returning(*_FOLDERS_SELECTION_COLS) + .returning(*_FOLDER_DB_MODEL_COLS) ) row = await result.first() return FolderDB.model_validate(row) @@ -110,7 +99,7 @@ def _create_private_workspace_query( ) return ( select( - *_FOLDERS_SELECTION_COLS, + *_FOLDER_DB_MODEL_COLS, func.json_build_object( "read", sa.text("true"), @@ -147,7 +136,7 @@ def _create_shared_workspace_query( shared_workspace_query = ( select( - *_FOLDERS_SELECTION_COLS, + *_FOLDER_DB_MODEL_COLS, workspace_access_rights_subquery.c.my_access_rights, ) .select_from( @@ -191,7 +180,7 @@ async def list_( # pylint: disable=too-many-arguments,too-many-branches limit: int, # order order_by: OrderBy, -) -> tuple[int, list[UserFolderAccessRightsDB]]: +) -> tuple[int, list[UserFolder]]: """ folder_query - Used to filter in which folder we want to list folders. trashed - If set to true, it returns folders **explicitly** trashed, if false then non-trashed folders. @@ -266,23 +255,16 @@ async def list_( # pylint: disable=too-many-arguments,too-many-branches total_count = await conn.scalar(count_query) result = await conn.stream(list_query) - folders: list[UserFolderAccessRightsDB] = [ - UserFolderAccessRightsDB.model_validate(row) async for row in result + folders: list[UserFolder] = [ + UserFolder.model_validate(row) async for row in result ] return cast(int, total_count), folders -def _create_base_select_query(folder_id: FolderID, product_name: ProductName): - return ( - select( - *_FOLDERS_SELECTION_COLS, - users.c.primary_gid.label("trashed_by_primary_gid"), - ) - .select_from(folders_v2.outerjoin(users, folders_v2.c.trashed_by == users.c.id)) - .where( - (folders_v2.c.product_name == product_name) - & (folders_v2.c.folder_id == folder_id) - ) +def _create_base_select_query(folder_id: FolderID, product_name: ProductName) -> Select: + return select(*_FOLDER_DB_MODEL_COLS,).where( + (folders_v2.c.product_name == product_name) + & (folders_v2.c.folder_id == folder_id) ) @@ -369,7 +351,7 @@ async def update( query = ( (folders_v2.update().values(modified=func.now(), **updated)) .where(folders_v2.c.product_name == product_name) - .returning(*_FOLDERS_SELECTION_COLS) + .returning(*_FOLDER_DB_MODEL_COLS) ) if isinstance(folders_id_or_ids, set): @@ -587,3 +569,48 @@ async def get_folders_recursively( final_query = select(folder_hierarchy_cte) result = await conn.stream(final_query) return cast(list[FolderID], [row.folder_id async for row in result]) + + +async def get_trashed_by_primary_gid( + app: web.Application, + connection: AsyncConnection | None = None, + *, + folder_id: FolderID, +) -> GroupID | None: + query = ( + sa.select( + users.c.primary_gid.label("trashed_by_primary_gid"), + ) + .select_from(projects.outerjoin(users, projects.c.trashed_by == users.c.id)) + .where(folders_v2.c.folder_id == folder_id) + ) + + async with pass_or_acquire_connection(get_asyncpg_engine(app), connection) as conn: + result = await conn.execute(query) + row = result.first() + return row.trashed_by_primary_gid if row else None + + +async def batch_get_trashed_by_primary_gid( + app: web.Application, + connection: AsyncConnection | None = None, + *, + folders_ids: list[FolderID], +) -> list[GroupID | None]: + + query = ( + sa.select( + users.c.primary_gid.label("trashed_by_primary_gid"), + ) + .select_from(folders_v2.outerjoin(users, folders_v2.c.trashed_by == users.c.id)) + .where(folders_v2.c.folder_id.in_(folders_ids)) + ).order_by( + *[ + folders_v2.c.folder_id == _id for _id in folders_ids + ] # Preserves the order of folders_ids + ) + + async with pass_or_acquire_connection(get_asyncpg_engine(app), connection) as conn: + result = await conn.execute(query) + rows = result.fetchall() + return [row.trashed_by_primary_gid for row in rows] diff --git a/services/web/server/src/simcore_service_webserver/folders/_folders_rest.py b/services/web/server/src/simcore_service_webserver/folders/_folders_rest.py index e3460eeb8ae..c0fe144e086 100644 --- a/services/web/server/src/simcore_service_webserver/folders/_folders_rest.py +++ b/services/web/server/src/simcore_service_webserver/folders/_folders_rest.py @@ -6,7 +6,7 @@ FolderGet, FolderReplaceBodyParams, ) -from models_library.folders import Folder +from models_library.folders import FolderTuple from models_library.rest_ordering import OrderBy from models_library.rest_pagination import ItemT, Page from models_library.rest_pagination_utils import paginate_data @@ -54,7 +54,7 @@ async def create_folder(request: web.Request): req_ctx = FoldersRequestContext.model_validate(request) body_params = await parse_request_body_as(FolderCreateBodyParams, request) - folder: Folder = await _folders_service.create_folder( + folder: FolderTuple = await _folders_service.create_folder( request.app, user_id=req_ctx.user_id, name=body_params.name, @@ -64,7 +64,11 @@ async def create_folder(request: web.Request): ) return envelope_json_response( - FolderGet.from_domain_model(folder.folder_db, folder.my_access_rights), + FolderGet.from_domain_model( + folder.folder_db, + trashed_by_primary_gid=None, + user_folder_access_rights=folder.my_access_rights, + ), web.HTTPCreated, ) @@ -150,14 +154,18 @@ async def get_folder(request: web.Request): req_ctx = FoldersRequestContext.model_validate(request) path_params = parse_request_path_parameters_as(FoldersPathParams, request) - folder: Folder = await _folders_service.get_folder( + folder: FolderTuple = await _folders_service.get_folder( app=request.app, folder_id=path_params.folder_id, user_id=req_ctx.user_id, product_name=req_ctx.product_name, ) return envelope_json_response( - FolderGet.from_domain_model(folder.folder_db, folder.my_access_rights) + FolderGet.from_domain_model( + folder.folder_db, + folder.trashed_by_primary_gid, + user_folder_access_rights=folder.my_access_rights, + ) ) @@ -173,7 +181,7 @@ async def replace_folder(request: web.Request): path_params = parse_request_path_parameters_as(FoldersPathParams, request) body_params = await parse_request_body_as(FolderReplaceBodyParams, request) - folder: Folder = await _folders_service.update_folder( + folder: FolderTuple = await _folders_service.update_folder( app=request.app, user_id=req_ctx.user_id, folder_id=path_params.folder_id, @@ -182,7 +190,11 @@ async def replace_folder(request: web.Request): product_name=req_ctx.product_name, ) return envelope_json_response( - FolderGet.from_domain_model(folder.folder_db, folder.my_access_rights) + FolderGet.from_domain_model( + folder.folder_db, + folder.trashed_by_primary_gid, + user_folder_access_rights=folder.my_access_rights, + ) ) diff --git a/services/web/server/src/simcore_service_webserver/folders/_folders_service.py b/services/web/server/src/simcore_service_webserver/folders/_folders_service.py index a6f130482f5..26e50075959 100644 --- a/services/web/server/src/simcore_service_webserver/folders/_folders_service.py +++ b/services/web/server/src/simcore_service_webserver/folders/_folders_service.py @@ -4,7 +4,7 @@ from aiohttp import web from models_library.access_rights import AccessRights -from models_library.folders import Folder, FolderID, FolderQuery, FolderScope +from models_library.folders import FolderID, FolderQuery, FolderScope, FolderTuple from models_library.products import ProductName from models_library.projects import ProjectID from models_library.rest_ordering import OrderBy @@ -22,7 +22,7 @@ WorkspaceAccessForbiddenError, WorkspaceFolderInconsistencyError, ) -from . import _folders_repository as folders_db +from . import _folders_repository from .errors import FolderValueNotPermittedError _logger = logging.getLogger(__name__) @@ -35,7 +35,7 @@ async def create_folder( parent_folder_id: FolderID | None, product_name: ProductName, workspace_id: WorkspaceID | None, -) -> Folder: +) -> FolderTuple: user = await get_user(app, user_id=user_id) workspace_is_private = True @@ -53,7 +53,7 @@ async def create_folder( # Check parent_folder_id lives in the workspace if parent_folder_id: - parent_folder_db = await folders_db.get( + parent_folder_db = await _folders_repository.get( app, folder_id=parent_folder_id, product_name=product_name ) if parent_folder_db.workspace_id != workspace_id: @@ -63,7 +63,7 @@ async def create_folder( if parent_folder_id: # Check user has access to the parent folder - parent_folder_db = await folders_db.get_for_user_or_workspace( + parent_folder_db = await _folders_repository.get_for_user_or_workspace( app, folder_id=parent_folder_id, product_name=product_name, @@ -76,7 +76,7 @@ async def create_folder( reason=f"Folder {parent_folder_id} does not exists in workspace {workspace_id}." ) - folder_db = await folders_db.create( + folder_db = await _folders_repository.create( app, product_name=product_name, created_by_gid=user["primary_gid"], @@ -85,7 +85,14 @@ async def create_folder( user_id=user_id if workspace_is_private else None, workspace_id=workspace_id, ) - return Folder(folder_db=folder_db, my_access_rights=user_folder_access_rights) + + assert folder_db.trashed_by is None # nosec + + return FolderTuple( + folder_db=folder_db, + trashed_by_primary_gid=None, # cannot be trashed upon creation + my_access_rights=user_folder_access_rights, + ) async def get_folder( @@ -93,8 +100,8 @@ async def get_folder( user_id: UserID, folder_id: FolderID, product_name: ProductName, -) -> Folder: - folder_db = await folders_db.get( +) -> FolderTuple: + folder_db = await _folders_repository.get( app, folder_id=folder_id, product_name=product_name ) @@ -111,14 +118,27 @@ async def get_folder( workspace_is_private = False user_folder_access_rights = user_workspace_access_rights.my_access_rights - folder_db = await folders_db.get_for_user_or_workspace( + folder_db = await _folders_repository.get_for_user_or_workspace( app, folder_id=folder_id, product_name=product_name, user_id=user_id if workspace_is_private else None, workspace_id=folder_db.workspace_id, ) - return Folder(folder_db=folder_db, my_access_rights=user_folder_access_rights) + + trashed_by_primary_gid = ( + await _folders_repository.get_trashed_by_primary_gid( + app, folder_id=folder_db.folder_id + ) + if folder_db.trashed_by + else None + ) + + return FolderTuple( + folder_db=folder_db, + trashed_by_primary_gid=trashed_by_primary_gid, + my_access_rights=user_folder_access_rights, + ) async def list_folders( @@ -131,10 +151,10 @@ async def list_folders( offset: NonNegativeInt, limit: int, order_by: OrderBy, -) -> tuple[list[Folder], int]: +) -> tuple[list[FolderTuple], int]: # NOTE: Folder access rights for listing are checked within the listing DB function. - total_count, folders = await folders_db.list_( + total_count, folders = await _folders_repository.list_( app, product_name=product_name, user_id=user_id, @@ -156,13 +176,23 @@ async def list_folders( limit=limit, order_by=order_by, ) + + _trashed_by_primary_gid_values = ( + await _folders_repository.batch_get_trashed_by_primary_gid( + app, folders_ids=[f.folder_id for f in folders] + ) + ) + return ( [ - Folder( + FolderTuple( folder_db=folder, + trashed_by_primary_gid=trashed_by_primary_gid, my_access_rights=folder.my_access_rights, ) - for folder in folders + for folder, trashed_by_primary_gid in zip( + folders, _trashed_by_primary_gid_values, strict=True + ) ], total_count, ) @@ -178,10 +208,10 @@ async def list_folders_full_depth( offset: NonNegativeInt, limit: int, order_by: OrderBy, -) -> tuple[list[Folder], int]: +) -> tuple[list[FolderTuple], int]: # NOTE: Folder access rights for listing are checked within the listing DB function. - total_count, folders = await folders_db.list_( + total_count, folders = await _folders_repository.list_( app, product_name=product_name, user_id=user_id, @@ -193,13 +223,22 @@ async def list_folders_full_depth( limit=limit, order_by=order_by, ) + _trashed_by_primary_gid_values = ( + await _folders_repository.batch_get_trashed_by_primary_gid( + app, folders_ids=[f.folder_id for f in folders] + ) + ) + return ( [ - Folder( + FolderTuple( folder_db=folder, + trashed_by_primary_gid=trashed_by_primary_gid, my_access_rights=folder.my_access_rights, ) - for folder in folders + for folder, trashed_by_primary_gid in zip( + folders, _trashed_by_primary_gid_values, strict=True + ) ], total_count, ) @@ -213,8 +252,8 @@ async def update_folder( name: str, parent_folder_id: FolderID | None, product_name: ProductName, -) -> Folder: - folder_db = await folders_db.get( +) -> FolderTuple: + folder_db = await _folders_repository.get( app, folder_id=folder_id, product_name=product_name ) @@ -232,7 +271,7 @@ async def update_folder( user_folder_access_rights = user_workspace_access_rights.my_access_rights # Check user has access to the folder - await folders_db.get_for_user_or_workspace( + await _folders_repository.get_for_user_or_workspace( app, folder_id=folder_id, product_name=product_name, @@ -242,7 +281,7 @@ async def update_folder( if folder_db.parent_folder_id != parent_folder_id and parent_folder_id is not None: # Check user has access to the parent folder - await folders_db.get_for_user_or_workspace( + await _folders_repository.get_for_user_or_workspace( app, folder_id=parent_folder_id, product_name=product_name, @@ -250,7 +289,7 @@ async def update_folder( workspace_id=folder_db.workspace_id, ) # Do not allow to move to a child folder id - _child_folders = await folders_db.get_folders_recursively( + _child_folders = await _folders_repository.get_folders_recursively( app, folder_id=folder_id, product_name=product_name ) if parent_folder_id in _child_folders: @@ -258,14 +297,27 @@ async def update_folder( reason="Parent folder id should not be one of children" ) - folder_db = await folders_db.update( + folder_db = await _folders_repository.update( app, folders_id_or_ids=folder_id, name=name, parent_folder_id=parent_folder_id, product_name=product_name, ) - return Folder(folder_db, user_folder_access_rights) + + trashed_by_primary_gid = ( + await _folders_repository.get_trashed_by_primary_gid( + app, folder_id=folder_db.folder_id + ) + if folder_db.trashed_by + else None + ) + + return FolderTuple( + folder_db=folder_db, + trashed_by_primary_gid=trashed_by_primary_gid, + my_access_rights=user_folder_access_rights, + ) async def delete_folder( @@ -274,7 +326,7 @@ async def delete_folder( folder_id: FolderID, product_name: ProductName, ) -> None: - folder_db = await folders_db.get( + folder_db = await _folders_repository.get( app, folder_id=folder_id, product_name=product_name ) @@ -290,7 +342,7 @@ async def delete_folder( workspace_is_private = False # Check user has access to the folder - await folders_db.get_for_user_or_workspace( + await _folders_repository.get_for_user_or_workspace( app, folder_id=folder_id, product_name=product_name, @@ -302,7 +354,7 @@ async def delete_folder( # 1.1 Delete all child projects that I am an owner project_id_list: list[ ProjectID - ] = await folders_db.get_projects_recursively_only_if_user_is_owner( + ] = await _folders_repository.get_projects_recursively_only_if_user_is_owner( app, folder_id=folder_id, private_workspace_user_id_or_none=user_id if workspace_is_private else None, @@ -324,6 +376,6 @@ async def delete_folder( ) # 1.2 Delete all child folders - await folders_db.delete_recursively( + await _folders_repository.delete_recursively( app, folder_id=folder_id, product_name=product_name ) diff --git a/services/web/server/src/simcore_service_webserver/projects/_crud_api_create.py b/services/web/server/src/simcore_service_webserver/projects/_crud_api_create.py index d60ce232423..feebc745fe6 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_crud_api_create.py +++ b/services/web/server/src/simcore_service_webserver/projects/_crud_api_create.py @@ -30,7 +30,7 @@ from ..catalog import client as catalog_client from ..director_v2 import api as director_v2_api from ..dynamic_scheduler import api as dynamic_scheduler_api -from ..folders import _folders_repository as folders_db +from ..folders import _folders_repository as _folders_repository from ..redis import get_redis_lock_manager_client_sdk from ..storage.api import ( copy_data_folders_from_project, @@ -293,7 +293,7 @@ async def create_project( # pylint: disable=too-many-arguments,too-many-branche ) if folder_id := predefined_project.get("folderId", None): # Check user has access to folder - await folders_db.get_for_user_or_workspace( + await _folders_repository.get_for_user_or_workspace( request.app, folder_id=folder_id, product_name=product_name, diff --git a/services/web/server/src/simcore_service_webserver/projects/_crud_api_read.py b/services/web/server/src/simcore_service_webserver/projects/_crud_api_read.py index f97dea43633..43ba76b6673 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_crud_api_read.py +++ b/services/web/server/src/simcore_service_webserver/projects/_crud_api_read.py @@ -20,7 +20,7 @@ ) from ..catalog.client import get_services_for_user_in_product -from ..folders import _folders_repository as folders_db +from ..folders import _folders_repository as _folders_repository from ..workspaces._workspaces_service import check_user_workspace_access from . import projects_service from ._permalink_api import update_or_pop_permalink_in_project @@ -107,7 +107,7 @@ async def list_projects( # pylint: disable=too-many-arguments if folder_id: # Check whether user has access to the folder - await folders_db.get_for_user_or_workspace( + await _folders_repository.get_for_user_or_workspace( app, folder_id=folder_id, product_name=product_name, diff --git a/services/web/server/src/simcore_service_webserver/projects/_folders_api.py b/services/web/server/src/simcore_service_webserver/projects/_folders_api.py index 865339a9165..7595f31d94d 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_folders_api.py +++ b/services/web/server/src/simcore_service_webserver/projects/_folders_api.py @@ -6,7 +6,7 @@ from models_library.projects import ProjectID from models_library.users import UserID -from ..folders import _folders_repository as folders_db +from ..folders import _folders_repository as _folders_repository from ..projects._access_rights_api import get_user_project_access_rights from . import _folders_db as project_to_folders_db from .db import APP_PROJECT_DBAPI, ProjectDBAPI @@ -45,7 +45,7 @@ async def move_project_into_folder( if folder_id: # Check user has access to folder - await folders_db.get_for_user_or_workspace( + await _folders_repository.get_for_user_or_workspace( app, folder_id=folder_id, product_name=product_name, diff --git a/services/web/server/src/simcore_service_webserver/projects/_projects_db.py b/services/web/server/src/simcore_service_webserver/projects/_projects_db.py index c61bc22a660..1a359f47476 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_projects_db.py +++ b/services/web/server/src/simcore_service_webserver/projects/_projects_db.py @@ -65,8 +65,8 @@ async def get_trashed_by_primary_gid( ) async with pass_or_acquire_connection(get_asyncpg_engine(app), connection) as conn: - result = await conn.stream(query) - row = await result.first() + result = await conn.execute(query) + row = result.first() return row.trashed_by_primary_gid if row else None From 667daa8a67ebebc759e747d2a533debab1cf668b Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 22 Jan 2025 21:11:52 +0100 Subject: [PATCH 32/44] testing folders --- ...rojects_crud_handlers__clone_in_workspace_and_folder.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/services/web/server/tests/unit/with_dbs/02/test_projects_crud_handlers__clone_in_workspace_and_folder.py b/services/web/server/tests/unit/with_dbs/02/test_projects_crud_handlers__clone_in_workspace_and_folder.py index 7f69fad580b..15542cff620 100644 --- a/services/web/server/tests/unit/with_dbs/02/test_projects_crud_handlers__clone_in_workspace_and_folder.py +++ b/services/web/server/tests/unit/with_dbs/02/test_projects_crud_handlers__clone_in_workspace_and_folder.py @@ -3,8 +3,9 @@ # pylint: disable=unused-argument # pylint: disable=unused-variable +from collections.abc import AsyncIterator from copy import deepcopy -from typing import Any, AsyncIterator +from typing import Any import pytest import sqlalchemy as sa @@ -41,7 +42,7 @@ async def create_workspace_and_folder( product_name="osparc", ) - folder, _ = await create_folder( + folder_db, *_ = await create_folder( client.app, user_id=logged_user["id"], name="a", @@ -50,7 +51,7 @@ async def create_workspace_and_folder( workspace_id=workspace.workspace_id, ) - yield (workspace.workspace_id, folder.folder_id) + yield (workspace.workspace_id, folder_db.folder_id) with postgres_db.connect() as con: con.execute(folders_v2.delete()) From f4c6886088f30b54b2b1b2d3dbb4e568d6df05d9 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 23 Jan 2025 16:51:47 +0100 Subject: [PATCH 33/44] minor --- .../src/simcore_postgres_database/utils_repos.py | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/postgres-database/src/simcore_postgres_database/utils_repos.py b/packages/postgres-database/src/simcore_postgres_database/utils_repos.py index 5a54061d727..c5da30eeb34 100644 --- a/packages/postgres-database/src/simcore_postgres_database/utils_repos.py +++ b/packages/postgres-database/src/simcore_postgres_database/utils_repos.py @@ -4,7 +4,6 @@ from typing import TypeVar import sqlalchemy as sa -from pydantic import BaseModel from sqlalchemy.ext.asyncio import AsyncConnection, AsyncEngine _logger = logging.getLogger(__name__) From 23fca694aa64570e76cd1ca52403a3ef662240bd Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 23 Jan 2025 17:14:08 +0100 Subject: [PATCH 34/44] fixes test and refactor query --- .../api_schemas_webserver/projects.py | 8 ++++---- .../folders/_folders_repository.py | 20 +++++++++---------- .../projects/_projects_db.py | 20 +++++++++---------- 3 files changed, 24 insertions(+), 24 deletions(-) diff --git a/packages/models-library/src/models_library/api_schemas_webserver/projects.py b/packages/models-library/src/models_library/api_schemas_webserver/projects.py index 33b74e2d842..27e5e722d4a 100644 --- a/packages/models-library/src/models_library/api_schemas_webserver/projects.py +++ b/packages/models-library/src/models_library/api_schemas_webserver/projects.py @@ -110,15 +110,15 @@ class ProjectGet(OutputSchema): @classmethod def from_domain_model(cls, project_data: dict[str, Any]) -> Self: - data = copy.copy(project_data) + trimmed_data = copy.copy(project_data) # project_data["trashed_by"] is a UserID # project_data["trashed_by_primary_gid"] is a GroupID - data.pop("trashed_by", None) - data.pop("trashedBy", None) + trimmed_data.pop("trashed_by", None) + trimmed_data.pop("trashedBy", None) return cls.model_validate( remap_keys( - project_data, + trimmed_data, rename={ "trashed": "trashed_at", "trashed_by_primary_gid": "trashed_by", diff --git a/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py b/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py index bcf4367f015..5aded011a95 100644 --- a/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py +++ b/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py @@ -571,18 +571,20 @@ async def get_folders_recursively( return cast(list[FolderID], [row.folder_id async for row in result]) +def _select_trashed_by_primary_gid_query(): + return sa.select(users.c.primary_gid.label("trashed_by_primary_gid")).select_from( + folders_v2.outerjoin(users, folders_v2.c.trashed_by == users.c.id), + ) + + async def get_trashed_by_primary_gid( app: web.Application, connection: AsyncConnection | None = None, *, folder_id: FolderID, ) -> GroupID | None: - query = ( - sa.select( - users.c.primary_gid.label("trashed_by_primary_gid"), - ) - .select_from(projects.outerjoin(users, projects.c.trashed_by == users.c.id)) - .where(folders_v2.c.folder_id == folder_id) + query = _select_trashed_by_primary_gid_query().where( + folders_v2.c.folder_id == folder_id ) async with pass_or_acquire_connection(get_asyncpg_engine(app), connection) as conn: @@ -599,11 +601,9 @@ async def batch_get_trashed_by_primary_gid( ) -> list[GroupID | None]: query = ( - sa.select( - users.c.primary_gid.label("trashed_by_primary_gid"), + _select_trashed_by_primary_gid_query().where( + folders_v2.c.folder_id.in_(folders_ids) ) - .select_from(folders_v2.outerjoin(users, folders_v2.c.trashed_by == users.c.id)) - .where(folders_v2.c.folder_id.in_(folders_ids)) ).order_by( *[ folders_v2.c.folder_id == _id for _id in folders_ids diff --git a/services/web/server/src/simcore_service_webserver/projects/_projects_db.py b/services/web/server/src/simcore_service_webserver/projects/_projects_db.py index 1a359f47476..15209f40a59 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_projects_db.py +++ b/services/web/server/src/simcore_service_webserver/projects/_projects_db.py @@ -50,18 +50,20 @@ async def patch_project( return ProjectDB.model_validate(row) +def _select_trashed_by_primary_gid_query(): + return sa.select( + users.c.primary_gid.label("trashed_by_primary_gid"), + ).select_from(projects.outerjoin(users, projects.c.trashed_by == users.c.id)) + + async def get_trashed_by_primary_gid( app: web.Application, connection: AsyncConnection | None = None, *, projects_uuid: ProjectID, ) -> GroupID | None: - query = ( - sa.select( - users.c.primary_gid.label("trashed_by_primary_gid"), - ) - .select_from(projects.outerjoin(users, projects.c.trashed_by == users.c.id)) - .where(projects.c.uuid == projects_uuid) + query = _select_trashed_by_primary_gid_query().where( + projects.c.uuid == projects_uuid ) async with pass_or_acquire_connection(get_asyncpg_engine(app), connection) as conn: @@ -87,11 +89,9 @@ async def batch_get_trashed_by_primary_gid( projects_uuids_str = [f"{uuid}" for uuid in projects_uuids] query = ( - sa.select( - users.c.primary_gid.label("trashed_by_primary_gid"), + _select_trashed_by_primary_gid_query().where( + projects.c.uuid.in_(projects_uuids_str) ) - .select_from(projects.outerjoin(users, projects.c.trashed_by == users.c.id)) - .where(projects.c.uuid.in_(projects_uuids_str)) ).order_by( *[ projects.c.uuid == uuid for uuid in projects_uuids_str From 1bf29423c35f05dc1e17b91ed85a1605de8b7435 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 23 Jan 2025 17:38:55 +0100 Subject: [PATCH 35/44] fixes order --- .../folders/_folders_repository.py | 16 ++++++++++------ .../projects/_projects_db.py | 12 +++++++++--- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py b/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py index 5aded011a95..2fd6cb85404 100644 --- a/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py +++ b/services/web/server/src/simcore_service_webserver/folders/_folders_repository.py @@ -599,18 +599,22 @@ async def batch_get_trashed_by_primary_gid( *, folders_ids: list[FolderID], ) -> list[GroupID | None]: + if not folders_ids: + return [] query = ( _select_trashed_by_primary_gid_query().where( folders_v2.c.folder_id.in_(folders_ids) ) ).order_by( - *[ - folders_v2.c.folder_id == _id for _id in folders_ids - ] # Preserves the order of folders_ids + # Preserves the order of folders_ids + # SEE https://docs.sqlalchemy.org/en/20/core/sqlelement.html#sqlalchemy.sql.expression.case + sa.case( + {folder_id: index for index, folder_id in enumerate(folders_ids)}, + value=folders_v2.c.folder_id, + ) ) async with pass_or_acquire_connection(get_asyncpg_engine(app), connection) as conn: - result = await conn.execute(query) - rows = result.fetchall() - return [row.trashed_by_primary_gid for row in rows] + result = await conn.stream(query) + return [row.trashed_by_primary_gid async for row in result] diff --git a/services/web/server/src/simcore_service_webserver/projects/_projects_db.py b/services/web/server/src/simcore_service_webserver/projects/_projects_db.py index 15209f40a59..85fe7e0a737 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_projects_db.py +++ b/services/web/server/src/simcore_service_webserver/projects/_projects_db.py @@ -93,9 +93,15 @@ async def batch_get_trashed_by_primary_gid( projects.c.uuid.in_(projects_uuids_str) ) ).order_by( - *[ - projects.c.uuid == uuid for uuid in projects_uuids_str - ] # Preserves the order of project_uuids + # Preserves the order of folders_ids + # SEE https://docs.sqlalchemy.org/en/20/core/sqlelement.html#sqlalchemy.sql.expression.case + sa.case( + { + project_uuid: index + for index, project_uuid in enumerate(projects_uuids_str) + }, + value=projects.c.uuid, + ) ) async with pass_or_acquire_connection(get_asyncpg_engine(app), connection) as conn: result = await conn.stream(query) From 347d56568cf9b636df5857f2390ce3ee33d7bbf9 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 23 Jan 2025 17:53:35 +0100 Subject: [PATCH 36/44] @GitHK review: annotations --- .../projects/_crud_handlers.py | 67 +++++++++---------- .../workspaces/_workspaces_repository.py | 6 +- 2 files changed, 37 insertions(+), 36 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/projects/_crud_handlers.py b/services/web/server/src/simcore_service_webserver/projects/_crud_handlers.py index fb66c4caea0..aa2163bdb04 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_crud_handlers.py +++ b/services/web/server/src/simcore_service_webserver/projects/_crud_handlers.py @@ -172,6 +172,27 @@ async def create_project(request: web.Request): ) +def _create_page_response(projects, request_url, total, limit, offset) -> web.Response: + page = Page[ProjectListItem].model_validate( + paginate_data( + chunk=[ + ProjectListItem.from_domain_model(prj).model_dump( + by_alias=True, exclude_unset=True + ) + for prj in projects + ], + request_url=request_url, + total=total, + limit=limit, + offset=offset, + ) + ) + return web.Response( + text=page.model_dump_json(**RESPONSE_MODEL_POLICY), + content_type=MIMETYPE_APPLICATION_JSON, + ) + + @routes.get(f"/{VTAG}/projects", name="list_projects") @login_required @permission_required("project.read") @@ -212,23 +233,12 @@ async def list_projects(request: web.Request): order_by=OrderBy.model_construct(**query_params.order_by.model_dump()), ) - page = Page[ProjectListItem].model_validate( - paginate_data( - chunk=[ - ProjectListItem.from_domain_model(prj).model_dump( - by_alias=True, exclude_unset=True - ) - for prj in projects - ], - request_url=request.url, - total=total_number_of_projects, - limit=query_params.limit, - offset=query_params.offset, - ) - ) - return web.Response( - text=page.model_dump_json(**RESPONSE_MODEL_POLICY), - content_type=MIMETYPE_APPLICATION_JSON, + return _create_page_response( + projects=projects, + request_url=request.url, + total=total_number_of_projects, + limit=query_params.limit, + offset=query_params.offset, ) @@ -259,23 +269,12 @@ async def list_projects_full_search(request: web.Request): order_by=OrderBy.model_construct(**query_params.order_by.model_dump()), ) - page = Page[ProjectListItem].model_validate( - paginate_data( - chunk=[ - ProjectListItem.from_domain_model(prj).model_dump( - by_alias=True, exclude_unset=True - ) - for prj in projects - ], - request_url=request.url, - total=total_number_of_projects, - limit=query_params.limit, - offset=query_params.offset, - ) - ) - return web.Response( - text=page.model_dump_json(**RESPONSE_MODEL_POLICY), - content_type=MIMETYPE_APPLICATION_JSON, + return _create_page_response( + projects=projects, + request_url=request.url, + total=total_number_of_projects, + limit=query_params.limit, + offset=query_params.offset, ) diff --git a/services/web/server/src/simcore_service_webserver/workspaces/_workspaces_repository.py b/services/web/server/src/simcore_service_webserver/workspaces/_workspaces_repository.py index 3a9690a3195..4fe2f81efdc 100644 --- a/services/web/server/src/simcore_service_webserver/workspaces/_workspaces_repository.py +++ b/services/web/server/src/simcore_service_webserver/workspaces/_workspaces_repository.py @@ -33,7 +33,7 @@ ) from sqlalchemy import asc, desc, func from sqlalchemy.ext.asyncio import AsyncConnection -from sqlalchemy.sql import select +from sqlalchemy.sql import Select, select from ..db.plugin import get_asyncpg_engine from .errors import WorkspaceAccessForbiddenError, WorkspaceNotFoundError @@ -82,7 +82,9 @@ async def create_workspace( return Workspace.model_validate(row) -def _create_base_select_query(caller_user_id: UserID, product_name: ProductName): +def _create_base_select_query( + caller_user_id: UserID, product_name: ProductName +) -> Select: # any other access access_rights_subquery = ( select( From 8f0a6264a122d9140b751308351011d1f81bea7e Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 23 Jan 2025 17:57:38 +0100 Subject: [PATCH 37/44] @bisgaard-itis review: camelcase or not --- .../pytest_simcore/simcore_webserver_projects_rest_api.py | 8 ++++---- .../projects/projects_service.py | 5 ++++- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/packages/pytest-simcore/src/pytest_simcore/simcore_webserver_projects_rest_api.py b/packages/pytest-simcore/src/pytest_simcore/simcore_webserver_projects_rest_api.py index 88538b6b21b..374d09cf6ce 100644 --- a/packages/pytest-simcore/src/pytest_simcore/simcore_webserver_projects_rest_api.py +++ b/packages/pytest-simcore/src/pytest_simcore/simcore_webserver_projects_rest_api.py @@ -77,7 +77,7 @@ def request_desc(self) -> str: "workspace_id": None, "folder_id": None, "trashedAt": None, - "trashed_by": None, + "trashedBy": None, }, "error": None, }, @@ -159,7 +159,7 @@ def request_desc(self) -> str: "workspace_id": None, "folder_id": None, "trashedAt": None, - "trashed_by": None, + "trashedBy": None, } }, ) @@ -290,7 +290,7 @@ def request_desc(self) -> str: "workspace_id": None, "folder_id": None, "trashedAt": None, - "trashed_by": None, + "trashedBy": None, } }, ) @@ -484,7 +484,7 @@ def request_desc(self) -> str: "workspace_id": None, "folder_id": None, "trashedAt": None, - "trashed_by": None, + "trashedBy": None, "classifiers": [], "ui": { "mode": "workbench", diff --git a/services/web/server/src/simcore_service_webserver/projects/projects_service.py b/services/web/server/src/simcore_service_webserver/projects/projects_service.py index eb339955617..91c018bdaef 100644 --- a/services/web/server/src/simcore_service_webserver/projects/projects_service.py +++ b/services/web/server/src/simcore_service_webserver/projects/projects_service.py @@ -215,7 +215,10 @@ async def get_project_for_user( ) # adds `trashed_by_primary_gid` - if include_trashed_by_primary_gid and project.get("trashed_by") is not None: + if ( + include_trashed_by_primary_gid + and project.get("trashed_by", project.get("trashedBy")) is not None + ): project.update( trashed_by_primary_gid=await _projects_db.get_trashed_by_primary_gid( app, projects_uuid=project["uuid"] From 906a8755c6e0ec003707b03fa7d6503dc24ce76b Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 23 Jan 2025 18:14:22 +0100 Subject: [PATCH 38/44] refactor to reduce complexity (sonarcloud) --- .../simcore_service_webserver/projects/db.py | 307 ++++++++++-------- 1 file changed, 169 insertions(+), 138 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/projects/db.py b/services/web/server/src/simcore_service_webserver/projects/db.py index 4fa5a872e79..3cf5b142f05 100644 --- a/services/web/server/src/simcore_service_webserver/projects/db.py +++ b/services/web/server/src/simcore_service_webserver/projects/db.py @@ -57,7 +57,7 @@ ProjectNodesRepo, ) from simcore_postgres_database.webserver_models import ProjectType, projects, users -from sqlalchemy import func, literal_column +from sqlalchemy import func, literal_column, sql from sqlalchemy.dialects.postgresql import BOOLEAN, INTEGER from sqlalchemy.dialects.postgresql import insert as pg_insert from sqlalchemy.sql import ColumnElement, CompoundSelect, Select, and_ @@ -370,6 +370,159 @@ async def upsert_project_linked_product( ).group_by(project_to_groups.c.project_uuid) ).subquery("access_rights_subquery") + def _create_private_workspace_query( + self, + *, + product_name: ProductName, + user_id: UserID, + workspace_query: WorkspaceQuery, + project_tags_subquery: sql.Subquery, + is_search_by_multi_columns: bool, + user_groups: list[RowProxy], + ) -> sql.Select | None: + private_workspace_query = None + if workspace_query.workspace_scope is not WorkspaceScope.SHARED: + assert workspace_query.workspace_scope in ( # nosec + WorkspaceScope.PRIVATE, + WorkspaceScope.ALL, + ) + + private_workspace_query = ( + sa.select( + *PROJECT_DB_COLS, + projects.c.workbench, + self._access_rights_subquery.c.access_rights, + projects_to_products.c.product_name, + projects_to_folders.c.folder_id, + sa.func.coalesce( + project_tags_subquery.c.tags, + sa.cast(sa.text("'{}'"), sa.ARRAY(sa.Integer)), + ).label("tags"), + ) + .select_from( + projects.join(self._access_rights_subquery, isouter=True) + .join(projects_to_products) + .join( + projects_to_folders, + ( + (projects_to_folders.c.project_uuid == projects.c.uuid) + & (projects_to_folders.c.user_id == user_id) + ), + isouter=True, + ) + .join(project_tags_subquery, isouter=True) + ) + .where( + ( + (projects.c.prj_owner == user_id) + | sa.text( + f"jsonb_exists_any(access_rights_subquery.access_rights, {assemble_array_groups(user_groups)})" + ) + ) + & (projects.c.workspace_id.is_(None)) # <-- Private workspace + & (projects_to_products.c.product_name == product_name) + ) + ) + if is_search_by_multi_columns: + private_workspace_query = private_workspace_query.join( + users, users.c.id == projects.c.prj_owner, isouter=True + ) + + return private_workspace_query + + def _create_shared_workspace_query( + self, + *, + product_name: ProductName, + workspace_query: WorkspaceQuery, + project_tags_subquery: sql.Subquery, + user_groups: list[RowProxy], + is_search_by_multi_columns: bool, + ) -> sql.Select | None: + + if workspace_query.workspace_scope is not WorkspaceScope.PRIVATE: + assert workspace_query.workspace_scope in ( + WorkspaceScope.SHARED, + WorkspaceScope.ALL, + ) + workspace_access_rights_subquery = ( + sa.select( + workspaces_access_rights.c.workspace_id, + sa.func.jsonb_object_agg( + workspaces_access_rights.c.gid, + sa.func.jsonb_build_object( + "read", + workspaces_access_rights.c.read, + "write", + workspaces_access_rights.c.write, + "delete", + workspaces_access_rights.c.delete, + ), + ) + .filter(workspaces_access_rights.c.read) + .label("access_rights"), + ).group_by(workspaces_access_rights.c.workspace_id) + ).subquery("workspace_access_rights_subquery") + + shared_workspace_query = ( + sa.select( + *PROJECT_DB_COLS, + projects.c.workbench, + workspace_access_rights_subquery.c.access_rights, + projects_to_products.c.product_name, + projects_to_folders.c.folder_id, + sa.func.coalesce( + project_tags_subquery.c.tags, + sa.cast(sa.text("'{}'"), sa.ARRAY(sa.Integer)), + ).label("tags"), + ) + .select_from( + projects.join( + workspace_access_rights_subquery, + projects.c.workspace_id + == workspace_access_rights_subquery.c.workspace_id, + ) + .join(projects_to_products) + .join( + projects_to_folders, + ( + (projects_to_folders.c.project_uuid == projects.c.uuid) + & (projects_to_folders.c.user_id.is_(None)) + ), + isouter=True, + ) + .join(project_tags_subquery, isouter=True) + ) + .where( + ( + sa.text( + f"jsonb_exists_any(workspace_access_rights_subquery.access_rights, {assemble_array_groups(user_groups)})" + ) + ) + & (projects_to_products.c.product_name == product_name) + ) + ) + if workspace_query.workspace_scope == WorkspaceScope.ALL: + shared_workspace_query = shared_workspace_query.where( + projects.c.workspace_id.is_not(None) # <-- All shared workspaces + ) + else: + assert workspace_query.workspace_scope == WorkspaceScope.SHARED + shared_workspace_query = shared_workspace_query.where( + projects.c.workspace_id + == workspace_query.workspace_id # <-- Specific shared workspace + ) + + if is_search_by_multi_columns: + # NOTE: fields searched with text include user's email + shared_workspace_query = shared_workspace_query.join( + users, users.c.id == projects.c.prj_owner, isouter=True + ) + + return shared_workspace_query + + return None + async def list_projects_dicts( # pylint: disable=too-many-arguments,too-many-statements,too-many-branches self, *, @@ -400,26 +553,6 @@ async def list_projects_dicts( # pylint: disable=too-many-arguments,too-many-st async with self.engine.acquire() as conn: user_groups: list[RowProxy] = await self._list_user_groups(conn, user_id) - - workspace_access_rights_subquery = ( - sa.select( - workspaces_access_rights.c.workspace_id, - sa.func.jsonb_object_agg( - workspaces_access_rights.c.gid, - sa.func.jsonb_build_object( - "read", - workspaces_access_rights.c.read, - "write", - workspaces_access_rights.c.write, - "delete", - workspaces_access_rights.c.delete, - ), - ) - .filter(workspaces_access_rights.c.read) - .label("access_rights"), - ).group_by(workspaces_access_rights.c.workspace_id) - ).subquery("workspace_access_rights_subquery") - project_tags_subquery = ( sa.select( projects_tags.c.project_id, @@ -433,127 +566,25 @@ async def list_projects_dicts( # pylint: disable=too-many-arguments,too-many-st # Private workspace query ### - if workspace_query.workspace_scope is not WorkspaceScope.SHARED: - assert workspace_query.workspace_scope in ( # nosec - WorkspaceScope.PRIVATE, - WorkspaceScope.ALL, - ) - - private_workspace_query = ( - sa.select( - *PROJECT_DB_COLS, - projects.c.workbench, - self._access_rights_subquery.c.access_rights, - projects_to_products.c.product_name, - projects_to_folders.c.folder_id, - sa.func.coalesce( - project_tags_subquery.c.tags, - sa.cast(sa.text("'{}'"), sa.ARRAY(sa.Integer)), - ).label("tags"), - ) - .select_from( - projects.join(self._access_rights_subquery, isouter=True) - .join(projects_to_products) - .join( - projects_to_folders, - ( - (projects_to_folders.c.project_uuid == projects.c.uuid) - & (projects_to_folders.c.user_id == user_id) - ), - isouter=True, - ) - .join(project_tags_subquery, isouter=True) - ) - .where( - ( - (projects.c.prj_owner == user_id) - | sa.text( - f"jsonb_exists_any(access_rights_subquery.access_rights, {assemble_array_groups(user_groups)})" - ) - ) - & (projects.c.workspace_id.is_(None)) # <-- Private workspace - & (projects_to_products.c.product_name == product_name) - ) - ) - if search_by_multi_columns is not None: - private_workspace_query = private_workspace_query.join( - users, users.c.id == projects.c.prj_owner, isouter=True - ) - else: - private_workspace_query = None + private_workspace_query = self._create_private_workspace_query( + product_name=product_name, + user_id=user_id, + workspace_query=workspace_query, + project_tags_subquery=project_tags_subquery, + is_search_by_multi_columns=search_by_multi_columns is not None, + user_groups=user_groups, + ) ### # Shared workspace query ### - - if workspace_query.workspace_scope is not WorkspaceScope.PRIVATE: - - assert workspace_query.workspace_scope in ( # nosec - WorkspaceScope.SHARED, - WorkspaceScope.ALL, - ) - - shared_workspace_query = ( - sa.select( - *PROJECT_DB_COLS, - projects.c.workbench, - workspace_access_rights_subquery.c.access_rights, - projects_to_products.c.product_name, - projects_to_folders.c.folder_id, - sa.func.coalesce( - project_tags_subquery.c.tags, - sa.cast(sa.text("'{}'"), sa.ARRAY(sa.Integer)), - ).label("tags"), - ) - .select_from( - projects.join( - workspace_access_rights_subquery, - projects.c.workspace_id - == workspace_access_rights_subquery.c.workspace_id, - ) - .join(projects_to_products) - .join( - projects_to_folders, - ( - (projects_to_folders.c.project_uuid == projects.c.uuid) - & (projects_to_folders.c.user_id.is_(None)) - ), - isouter=True, - ) - .join(project_tags_subquery, isouter=True) - ) - .where( - ( - sa.text( - f"jsonb_exists_any(workspace_access_rights_subquery.access_rights, {assemble_array_groups(user_groups)})" - ) - ) - & (projects_to_products.c.product_name == product_name) - ) - ) - if workspace_query.workspace_scope == WorkspaceScope.ALL: - shared_workspace_query = shared_workspace_query.where( - projects.c.workspace_id.is_not( - None - ) # <-- All shared workspaces - ) - else: - assert ( # nosec - workspace_query.workspace_scope == WorkspaceScope.SHARED - ) - shared_workspace_query = shared_workspace_query.where( - projects.c.workspace_id - == workspace_query.workspace_id # <-- Specific shared workspace - ) - - if search_by_multi_columns is not None: - # NOTE: fields searched with text include user's email - shared_workspace_query = shared_workspace_query.join( - users, users.c.id == projects.c.prj_owner, isouter=True - ) - - else: - shared_workspace_query = None + shared_workspace_query = self._create_shared_workspace_query( + product_name=product_name, + workspace_query=workspace_query, + project_tags_subquery=project_tags_subquery, + user_groups=user_groups, + is_search_by_multi_columns=search_by_multi_columns is not None, + ) ### # Attributes Filters From 09644ab553b64b98d8f8903991d05f6f0458be06 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 23 Jan 2025 18:32:04 +0100 Subject: [PATCH 39/44] fixes cancelation errors --- .../src/simcore_postgres_database/utils_repos.py | 3 ++- services/web/server/tests/conftest.py | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/postgres-database/src/simcore_postgres_database/utils_repos.py b/packages/postgres-database/src/simcore_postgres_database/utils_repos.py index c5da30eeb34..a00adb1455d 100644 --- a/packages/postgres-database/src/simcore_postgres_database/utils_repos.py +++ b/packages/postgres-database/src/simcore_postgres_database/utils_repos.py @@ -4,6 +4,7 @@ from typing import TypeVar import sqlalchemy as sa +from pydantic import BaseModel from sqlalchemy.ext.asyncio import AsyncConnection, AsyncEngine _logger = logging.getLogger(__name__) @@ -63,7 +64,7 @@ async def transaction_context( SQLModel = TypeVar( # Towards using https://sqlmodel.tiangolo.com/#create-a-sqlmodel-model "SQLModel", - bound="BaseModel", + bound=BaseModel, ) diff --git a/services/web/server/tests/conftest.py b/services/web/server/tests/conftest.py index 3322e0b8b31..46cdf6783e8 100644 --- a/services/web/server/tests/conftest.py +++ b/services/web/server/tests/conftest.py @@ -234,6 +234,7 @@ async def _setup( "workspaceId": None, "folderId": None, "trashedAt": None, + "trashedByPrimaryGid": None, } if from_study: from_study_wo_access_rights = deepcopy(from_study) From 5b8a385edf4713cf298279c1389a60a0676ea174 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 23 Jan 2025 19:21:06 +0100 Subject: [PATCH 40/44] cleanup --- .../simcore_service_webserver/projects/db.py | 58 +++++++++++-------- ...t_workspaces__folders_and_projects_crud.py | 7 ++- 2 files changed, 38 insertions(+), 27 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/projects/db.py b/services/web/server/src/simcore_service_webserver/projects/db.py index 3cf5b142f05..7bab4c1f4fc 100644 --- a/services/web/server/src/simcore_service_webserver/projects/db.py +++ b/services/web/server/src/simcore_service_webserver/projects/db.py @@ -349,27 +349,6 @@ async def upsert_project_linked_product( .on_conflict_do_nothing() ) - _access_rights_subquery = ( - sa.select( - project_to_groups.c.project_uuid, - sa.func.jsonb_object_agg( - project_to_groups.c.gid, - sa.func.jsonb_build_object( - "read", - project_to_groups.c.read, - "write", - project_to_groups.c.write, - "delete", - project_to_groups.c.delete, - ), - ) - .filter( - project_to_groups.c.read # Filters out entries where "read" is False - ) - .label("access_rights"), - ).group_by(project_to_groups.c.project_uuid) - ).subquery("access_rights_subquery") - def _create_private_workspace_query( self, *, @@ -387,11 +366,32 @@ def _create_private_workspace_query( WorkspaceScope.ALL, ) + access_rights_subquery = ( + sa.select( + project_to_groups.c.project_uuid, + sa.func.jsonb_object_agg( + project_to_groups.c.gid, + sa.func.jsonb_build_object( + "read", + project_to_groups.c.read, + "write", + project_to_groups.c.write, + "delete", + project_to_groups.c.delete, + ), + ) + .filter( + project_to_groups.c.read # Filters out entries where "read" is False + ) + .label("access_rights"), + ).group_by(project_to_groups.c.project_uuid) + ).subquery("access_rights_subquery") + private_workspace_query = ( sa.select( *PROJECT_DB_COLS, projects.c.workbench, - self._access_rights_subquery.c.access_rights, + access_rights_subquery.c.access_rights, projects_to_products.c.product_name, projects_to_folders.c.folder_id, sa.func.coalesce( @@ -400,7 +400,7 @@ def _create_private_workspace_query( ).label("tags"), ) .select_from( - projects.join(self._access_rights_subquery, isouter=True) + projects.join(access_rights_subquery, isouter=True) .join(projects_to_products) .join( projects_to_folders, @@ -423,6 +423,10 @@ def _create_private_workspace_query( & (projects_to_products.c.product_name == product_name) ) ) + assert ( # nosec + access_rights_subquery.description == "access_rights_subquery" + ) + if is_search_by_multi_columns: private_workspace_query = private_workspace_query.join( users, users.c.id == projects.c.prj_owner, isouter=True @@ -441,10 +445,11 @@ def _create_shared_workspace_query( ) -> sql.Select | None: if workspace_query.workspace_scope is not WorkspaceScope.PRIVATE: - assert workspace_query.workspace_scope in ( + assert workspace_query.workspace_scope in ( # nosec WorkspaceScope.SHARED, WorkspaceScope.ALL, ) + workspace_access_rights_subquery = ( sa.select( workspaces_access_rights.c.workspace_id, @@ -502,6 +507,11 @@ def _create_shared_workspace_query( & (projects_to_products.c.product_name == product_name) ) ) + assert ( # nosec + workspace_access_rights_subquery.description + == "workspace_access_rights_subquery" + ) + if workspace_query.workspace_scope == WorkspaceScope.ALL: shared_workspace_query = shared_workspace_query.where( projects.c.workspace_id.is_not(None) # <-- All shared workspaces diff --git a/services/web/server/tests/unit/with_dbs/04/workspaces/test_workspaces__folders_and_projects_crud.py b/services/web/server/tests/unit/with_dbs/04/workspaces/test_workspaces__folders_and_projects_crud.py index f7dd5a8e1aa..75702855cc1 100644 --- a/services/web/server/tests/unit/with_dbs/04/workspaces/test_workspaces__folders_and_projects_crud.py +++ b/services/web/server/tests/unit/with_dbs/04/workspaces/test_workspaces__folders_and_projects_crud.py @@ -393,6 +393,7 @@ async def test_workspaces_delete_folders( @pytest.mark.parametrize("user_role,expected", [(UserRole.USER, status.HTTP_200_OK)]) async def test_listing_folders_and_projects_in_workspace__multiple_workspaces_created( + request: pytest.FixtureRequest, client: TestClient, logged_user: UserInfoDict, user_project: ProjectDict, @@ -409,7 +410,7 @@ async def test_listing_folders_and_projects_in_workspace__multiple_workspaces_cr f"{url}", json={ "name": "My first workspace", - "description": "Custom description", + "description": f"workspace 1 at {request.node.name}", "thumbnail": None, }, ) @@ -441,8 +442,8 @@ async def test_listing_folders_and_projects_in_workspace__multiple_workspaces_cr resp = await client.post( f"{url}", json={ - "name": "My first workspace", - "description": "Custom description", + "name": "My second workspace", + "description": f"workspace 2 at {request.node.name}", "thumbnail": None, }, ) From fadfb3a9e638a9f631f1c36eab951c368a790ebd Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 23 Jan 2025 21:45:55 +0100 Subject: [PATCH 41/44] sonarcloud complexity and pylint --- .../simcore_service_webserver/projects/db.py | 132 +++++++++++------- services/web/server/tests/conftest.py | 9 +- 2 files changed, 88 insertions(+), 53 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/projects/db.py b/services/web/server/src/simcore_service_webserver/projects/db.py index 7bab4c1f4fc..66f285fcfa8 100644 --- a/services/web/server/src/simcore_service_webserver/projects/db.py +++ b/services/web/server/src/simcore_service_webserver/projects/db.py @@ -349,8 +349,8 @@ async def upsert_project_linked_product( .on_conflict_do_nothing() ) + @staticmethod def _create_private_workspace_query( - self, *, product_name: ProductName, user_id: UserID, @@ -434,8 +434,8 @@ def _create_private_workspace_query( return private_workspace_query + @staticmethod def _create_shared_workspace_query( - self, *, product_name: ProductName, workspace_query: WorkspaceQuery, @@ -533,6 +533,74 @@ def _create_shared_workspace_query( return None + @staticmethod + def _create_attributes_filters( + *, + filter_by_project_type: ProjectType | None, + filter_hidden: bool | None, + filter_published: bool | None, + filter_trashed: bool | None, + search_by_multi_columns: str | None, + search_by_project_name: str | None, + filter_tag_ids_list: list[int] | None, + folder_query: FolderQuery, + project_tags_subquery: sql.Subquery, + ) -> list[ColumnElement]: + attributes_filters: list[ColumnElement] = [] + + if filter_by_project_type is not None: + attributes_filters.append(projects.c.type == filter_by_project_type.value) + + if filter_hidden is not None: + attributes_filters.append(projects.c.hidden.is_(filter_hidden)) + + if filter_published is not None: + attributes_filters.append(projects.c.published.is_(filter_published)) + + if filter_trashed is not None: + attributes_filters.append( + # marked explicitly as trashed + ( + projects.c.trashed.is_not(None) + & projects.c.trashed_explicitly.is_(True) + ) + if filter_trashed + # not marked as trashed + else projects.c.trashed.is_(None) + ) + + if search_by_multi_columns is not None: + attributes_filters.append( + (projects.c.name.ilike(f"%{search_by_multi_columns}%")) + | (projects.c.description.ilike(f"%{search_by_multi_columns}%")) + | (projects.c.uuid.ilike(f"%{search_by_multi_columns}%")) + | (users.c.name.ilike(f"%{search_by_multi_columns}%")) + ) + + if search_by_project_name is not None: + attributes_filters.append( + projects.c.name.like(f"%{search_by_project_name}%") + ) + + if filter_tag_ids_list: + attributes_filters.append( + sa.func.coalesce( + project_tags_subquery.c.tags, + sa.cast(sa.text("'{}'"), sa.ARRAY(sa.Integer)), + ).op("@>")(filter_tag_ids_list) + ) + + if folder_query.folder_scope is not FolderScope.ALL: + if folder_query.folder_scope == FolderScope.SPECIFIC: + attributes_filters.append( + projects_to_folders.c.folder_id == folder_query.folder_id + ) + else: + assert folder_query.folder_scope == FolderScope.ROOT # nosec + attributes_filters.append(projects_to_folders.c.folder_id.is_(None)) + + return attributes_filters + async def list_projects_dicts( # pylint: disable=too-many-arguments,too-many-statements,too-many-branches self, *, @@ -600,55 +668,17 @@ async def list_projects_dicts( # pylint: disable=too-many-arguments,too-many-st # Attributes Filters ### - attributes_filters: list[ColumnElement] = [] - if filter_by_project_type is not None: - attributes_filters.append( - projects.c.type == filter_by_project_type.value - ) - - if filter_hidden is not None: - attributes_filters.append(projects.c.hidden.is_(filter_hidden)) - - if filter_published is not None: - attributes_filters.append(projects.c.published.is_(filter_published)) - - if filter_trashed is not None: - attributes_filters.append( - # marked explicitly as trashed - ( - projects.c.trashed.is_not(None) - & projects.c.trashed_explicitly.is_(True) - ) - if filter_trashed - # not marked as trashed - else projects.c.trashed.is_(None) - ) - if search_by_multi_columns is not None: - attributes_filters.append( - (projects.c.name.ilike(f"%{search_by_multi_columns}%")) - | (projects.c.description.ilike(f"%{search_by_multi_columns}%")) - | (projects.c.uuid.ilike(f"%{search_by_multi_columns}%")) - | (users.c.name.ilike(f"%{search_by_multi_columns}%")) - ) - if search_by_project_name is not None: - attributes_filters.append( - projects.c.name.like(f"%{search_by_project_name}%") - ) - if filter_tag_ids_list: - attributes_filters.append( - sa.func.coalesce( - project_tags_subquery.c.tags, - sa.cast(sa.text("'{}'"), sa.ARRAY(sa.Integer)), - ).op("@>")(filter_tag_ids_list) - ) - if folder_query.folder_scope is not FolderScope.ALL: - if folder_query.folder_scope == FolderScope.SPECIFIC: - attributes_filters.append( - projects_to_folders.c.folder_id == folder_query.folder_id - ) - else: - assert folder_query.folder_scope == FolderScope.ROOT # nosec - attributes_filters.append(projects_to_folders.c.folder_id.is_(None)) + attributes_filters = self._create_attributes_filters( + filter_by_project_type=filter_by_project_type, + filter_hidden=filter_hidden, + filter_published=filter_published, + filter_trashed=filter_trashed, + search_by_multi_columns=search_by_multi_columns, + search_by_project_name=search_by_project_name, + filter_tag_ids_list=filter_tag_ids_list, + folder_query=folder_query, + project_tags_subquery=project_tags_subquery, + ) ### # Combined diff --git a/services/web/server/tests/conftest.py b/services/web/server/tests/conftest.py index 46cdf6783e8..2ff0c5c2a0a 100644 --- a/services/web/server/tests/conftest.py +++ b/services/web/server/tests/conftest.py @@ -234,12 +234,17 @@ async def _setup( "workspaceId": None, "folderId": None, "trashedAt": None, - "trashedByPrimaryGid": None, + "trashedBy": None, } if from_study: + from_study_wo_access_rights = deepcopy(from_study) from_study_wo_access_rights.pop("accessRights") - expected_data = {**expected_data, **from_study_wo_access_rights} + expected_data = { + **expected_data, + "trashedByPrimaryGid": None, + **from_study_wo_access_rights, + } if not as_template: expected_data["name"] = f"{from_study['name']} (Copy)" From 7a92b4876a56789a7d058933a328033de47b31ae Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 23 Jan 2025 22:05:52 +0100 Subject: [PATCH 42/44] drop --- .../pytest-simcore/src/pytest_simcore/helpers/postgres_tools.py | 2 +- services/web/server/tests/unit/with_dbs/conftest.py | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/pytest-simcore/src/pytest_simcore/helpers/postgres_tools.py b/packages/pytest-simcore/src/pytest_simcore/helpers/postgres_tools.py index 5803adee9f8..5f1f6c6ffe4 100644 --- a/packages/pytest-simcore/src/pytest_simcore/helpers/postgres_tools.py +++ b/packages/pytest-simcore/src/pytest_simcore/helpers/postgres_tools.py @@ -61,7 +61,7 @@ def migrated_pg_tables_context( ) ) # SEE https://github.com/ITISFoundation/osparc-simcore/issues/1776 - metadata.drop_all(bind=postgres_engine) + metadata.drop_all(bind=conn) def is_postgres_responsive(url) -> bool: diff --git a/services/web/server/tests/unit/with_dbs/conftest.py b/services/web/server/tests/unit/with_dbs/conftest.py index f2556f81afe..1aceea40a80 100644 --- a/services/web/server/tests/unit/with_dbs/conftest.py +++ b/services/web/server/tests/unit/with_dbs/conftest.py @@ -536,6 +536,7 @@ def postgres_db( "WHERE state = 'idle in transaction';" ) ) + # SEE https://github.com/ITISFoundation/osparc-simcore/issues/1776 orm.metadata.drop_all(bind=conn) engine.dispose() From 0a7b242efbe5c0132511c9b51014139131301315 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Fri, 24 Jan 2025 17:44:53 +0100 Subject: [PATCH 43/44] fixes tests --- .../src/simcore_service_webserver/projects/_projects_db.py | 3 ++- .../src/simcore_service_webserver/projects/projects_service.py | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/projects/_projects_db.py b/services/web/server/src/simcore_service_webserver/projects/_projects_db.py index 85fe7e0a737..95af2e41284 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_projects_db.py +++ b/services/web/server/src/simcore_service_webserver/projects/_projects_db.py @@ -11,6 +11,7 @@ pass_or_acquire_connection, transaction_context, ) +from sqlalchemy import sql from sqlalchemy.ext.asyncio import AsyncConnection from ..db.plugin import get_asyncpg_engine @@ -50,7 +51,7 @@ async def patch_project( return ProjectDB.model_validate(row) -def _select_trashed_by_primary_gid_query(): +def _select_trashed_by_primary_gid_query() -> sql.Select: return sa.select( users.c.primary_gid.label("trashed_by_primary_gid"), ).select_from(projects.outerjoin(users, projects.c.trashed_by == users.c.id)) diff --git a/services/web/server/src/simcore_service_webserver/projects/projects_service.py b/services/web/server/src/simcore_service_webserver/projects/projects_service.py index 91c018bdaef..67c6154afa3 100644 --- a/services/web/server/src/simcore_service_webserver/projects/projects_service.py +++ b/services/web/server/src/simcore_service_webserver/projects/projects_service.py @@ -220,7 +220,7 @@ async def get_project_for_user( and project.get("trashed_by", project.get("trashedBy")) is not None ): project.update( - trashed_by_primary_gid=await _projects_db.get_trashed_by_primary_gid( + trashedByPrimaryGid=await _projects_db.get_trashed_by_primary_gid( app, projects_uuid=project["uuid"] ) ) From 13408625f04859b1a043a3972ebf73f6f0884183 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Mon, 27 Jan 2025 16:13:51 +0100 Subject: [PATCH 44/44] fixes bug introduced in workspaces --- .../workspaces/_workspaces_repository.py | 6 +++--- .../test_workspaces__folders_and_projects_crud.py | 15 ++++++++------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/workspaces/_workspaces_repository.py b/services/web/server/src/simcore_service_webserver/workspaces/_workspaces_repository.py index 4fe2f81efdc..e62efae0a10 100644 --- a/services/web/server/src/simcore_service_webserver/workspaces/_workspaces_repository.py +++ b/services/web/server/src/simcore_service_webserver/workspaces/_workspaces_repository.py @@ -191,11 +191,11 @@ async def get_workspace_for_user( ) -> UserWorkspaceWithAccessRights: select_query = _create_base_select_query( caller_user_id=user_id, product_name=product_name - ) + ).where(workspaces.c.workspace_id == workspace_id) async with pass_or_acquire_connection(get_asyncpg_engine(app), connection) as conn: - result = await conn.stream(select_query) - row = await result.first() + result = await conn.execute(select_query) + row = result.one_or_none() if row is None: raise WorkspaceAccessForbiddenError( reason=f"User {user_id} does not have access to the workspace {workspace_id}. Or workspace does not exist.", diff --git a/services/web/server/tests/unit/with_dbs/04/workspaces/test_workspaces__folders_and_projects_crud.py b/services/web/server/tests/unit/with_dbs/04/workspaces/test_workspaces__folders_and_projects_crud.py index 75702855cc1..c301ead5f90 100644 --- a/services/web/server/tests/unit/with_dbs/04/workspaces/test_workspaces__folders_and_projects_crud.py +++ b/services/web/server/tests/unit/with_dbs/04/workspaces/test_workspaces__folders_and_projects_crud.py @@ -416,7 +416,7 @@ async def test_listing_folders_and_projects_in_workspace__multiple_workspaces_cr ) added_workspace_1, _ = await assert_status(resp, status.HTTP_201_CREATED) - # Create project in workspace + # Create PROJECT in workspace project_data = deepcopy(fake_project) project_data["workspace_id"] = f"{added_workspace_1['workspaceId']}" await create_project( @@ -426,7 +426,7 @@ async def test_listing_folders_and_projects_in_workspace__multiple_workspaces_cr product_name="osparc", ) - # Create folder in workspace + # Create FOLDER in workspace url = client.app.router["create_folder"].url_for() resp = await client.post( f"{url}", @@ -437,7 +437,7 @@ async def test_listing_folders_and_projects_in_workspace__multiple_workspaces_cr ) first_folder, _ = await assert_status(resp, status.HTTP_201_CREATED) - # create a new workspace + # create a new WORKSPACE url = client.app.router["create_workspace"].url_for() resp = await client.post( f"{url}", @@ -448,8 +448,9 @@ async def test_listing_folders_and_projects_in_workspace__multiple_workspaces_cr }, ) added_workspace_2, _ = await assert_status(resp, status.HTTP_201_CREATED) + assert added_workspace_2["workspaceId"] != added_workspace_1["workspaceId"] - # Create project in workspace + # Create PROJECT in workspace project_data = deepcopy(fake_project) project_data["workspace_id"] = f"{added_workspace_2['workspaceId']}" await create_project( @@ -459,7 +460,7 @@ async def test_listing_folders_and_projects_in_workspace__multiple_workspaces_cr product_name="osparc", ) - # Create folder in workspace + # Create FOLDER in workspace url = client.app.router["create_folder"].url_for() resp = await client.post( f"{url}", @@ -470,7 +471,7 @@ async def test_listing_folders_and_projects_in_workspace__multiple_workspaces_cr ) first_folder, _ = await assert_status(resp, status.HTTP_201_CREATED) - # List projects in workspace 1 + # List PROJECTS in workspace 1 url = ( client.app.router["list_projects"] .url_for() @@ -480,7 +481,7 @@ async def test_listing_folders_and_projects_in_workspace__multiple_workspaces_cr data, _ = await assert_status(resp, status.HTTP_200_OK) assert len(data) == 1 - # List folders in workspace 1 + # List FOLDERS in workspace 1 url = ( client.app.router["list_folders"] .url_for()