From b3ca7641fe5819c263076b55c036be22e9d04e92 Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Thu, 8 Aug 2024 11:34:10 +0200 Subject: [PATCH 1/3] added folder_get sharing part of fodler_list --- .../utils_folders.py | 163 +++++++++++++----- 1 file changed, 123 insertions(+), 40 deletions(-) diff --git a/packages/postgres-database/src/simcore_postgres_database/utils_folders.py b/packages/postgres-database/src/simcore_postgres_database/utils_folders.py index 8b829bbb484..1f344caecd0 100644 --- a/packages/postgres-database/src/simcore_postgres_database/utils_folders.py +++ b/packages/postgres-database/src/simcore_postgres_database/utils_folders.py @@ -5,7 +5,7 @@ from datetime import datetime from enum import Enum from functools import reduce -from typing import Any, ClassVar, TypeAlias +from typing import Any, ClassVar, Final, TypeAlias import sqlalchemy as sa from aiopg.sa.connection import SAConnection @@ -22,6 +22,7 @@ from pydantic.errors import PydanticErrorMixin from sqlalchemy.dialects import postgresql from sqlalchemy.sql.elements import ColumnElement +from sqlalchemy.sql.selectable import ScalarSelect from .models.folders import folders, folders_access_rights, folders_to_projects from .models.groups import GroupType, groups @@ -53,6 +54,8 @@ * CannotMoveFolderSharedViaNonPrimaryGroupError * BaseAddProjectError * ProjectAlreadyExistsInFolderError + * BaseFolderGet + * FolderNotFoundError """ @@ -122,6 +125,14 @@ class ProjectAlreadyExistsInFolderError(BaseAddProjectError): ) +class BaseFolderGet(FoldersError): + pass + + +class FolderNotFoundError(BaseFolderGet): + msg_template = "no entry found for folder_id={folder_id} and gid={gid}" + + ### ### UTILS ACCESS LAYER ### @@ -179,6 +190,7 @@ def _or_dicts_list(dicts: Iterable[_FolderPermissions]) -> _FolderPermissions: class _BasePermissions: + GET_FOLDER: ClassVar[_FolderPermissions] = _make_permissions(r=True) LIST_FOLDERS: ClassVar[_FolderPermissions] = _make_permissions(r=True) CREATE_FOLDER: ClassVar[_FolderPermissions] = _make_permissions(w=True) @@ -931,6 +943,51 @@ async def folder_remove_project( ) +def _get_subquery_my_access_rights( + access_via_gid: _GroupID, access_via_folder_id: _FolderID | None +) -> ScalarSelect: + return ( + sa.select( + sa.func.jsonb_build_object( + "read", + folders_access_rights.c.read, + "write", + folders_access_rights.c.write, + "delete", + folders_access_rights.c.delete, + ).label("my_access_rights"), + ) + .where( + folders_access_rights.c.folder_id == access_via_folder_id + if access_via_gid and access_via_folder_id + else folders_access_rights.c.folder_id == folders.c.id + ) + .where(folders_access_rights.c.gid == access_via_gid) + .correlate(folders) + .scalar_subquery() + ) + + +_SUBQUERY_ACCESS_RIGHTS: Final[ScalarSelect] = ( + sa.select( + sa.func.jsonb_object_agg( + folders_access_rights.c.gid, + sa.func.jsonb_build_object( + "read", + folders_access_rights.c.read, + "write", + folders_access_rights.c.write, + "delete", + folders_access_rights.c.delete, + ), + ).label("access_rights"), + ) + .where(folders_access_rights.c.folder_id == folders.c.id) + .correlate(folders) + .scalar_subquery() +) + + async def folder_list( connection: SAConnection, product_name: _ProductName, @@ -968,45 +1025,67 @@ async def folder_list( access_via_gid = resolved_access_rights.gid access_via_folder_id = resolved_access_rights.folder_id - subquery_my_access_rights = ( + query = ( sa.select( - sa.func.jsonb_build_object( - "read", - folders_access_rights.c.read, - "write", - folders_access_rights.c.write, - "delete", - folders_access_rights.c.delete, + folders, + folders_access_rights, + folders_access_rights.c.created.label("access_created"), + folders_access_rights.c.modified.label("access_modified"), + sa.literal_column(f"{access_via_gid}").label("access_via_gid"), + _get_subquery_my_access_rights( + access_via_gid, access_via_folder_id ).label("my_access_rights"), + _SUBQUERY_ACCESS_RIGHTS.label("access_rights"), + ) + .join( + folders_access_rights, folders.c.id == folders_access_rights.c.folder_id ) .where( - folders_access_rights.c.folder_id == access_via_folder_id - if access_via_gid and access_via_folder_id - else folders_access_rights.c.folder_id == folders.c.id + folders_access_rights.c.traversal_parent_id.is_(None) + if folder_id is None + else folders_access_rights.c.traversal_parent_id == folder_id ) - .where(folders_access_rights.c.gid == access_via_gid) - .correlate(folders) - .scalar_subquery() + .where( + folders_access_rights.c.gid.in_([access_via_gid]) + if folder_id is None + else True + ) + .where( + _get_and_calsue_with_only_true_entries( + _required_permissions, folders_access_rights + ) + if folder_id is None + else True + ) + .offset(offset) + .limit(limit) ) - subquery_access_rights = ( - sa.select( - sa.func.jsonb_object_agg( - folders_access_rights.c.gid, - sa.func.jsonb_build_object( - "read", - folders_access_rights.c.read, - "write", - folders_access_rights.c.write, - "delete", - folders_access_rights.c.delete, - ), - ).label("access_rights"), - ) - .where(folders_access_rights.c.folder_id == folders.c.id) - .correlate(folders) - .scalar_subquery() + async for entry in connection.execute(query): + results.append(FolderEntry.from_orm(entry)) # noqa: PERF401s + + return results + + +async def folder_get( + connection: SAConnection, + product_name: _ProductName, + folder_id: _FolderID, + gid: _GroupID, + *, + required_permissions=_requires(_BasePermissions.GET_FOLDER), # noqa: B008 +) -> FolderEntry: + async with connection.begin(): + resolved_access_rights: _ResolvedAccessRights = await _check_folder_and_access( + connection, + product_name, + folder_id=folder_id, + gid=gid, + permissions=required_permissions, + enforece_all_permissions=False, ) + access_via_gid = resolved_access_rights.gid + access_via_folder_id = resolved_access_rights.folder_id query = ( sa.select( @@ -1015,8 +1094,10 @@ async def folder_list( folders_access_rights.c.created.label("access_created"), folders_access_rights.c.modified.label("access_modified"), sa.literal_column(f"{access_via_gid}").label("access_via_gid"), - subquery_my_access_rights.label("my_access_rights"), - subquery_access_rights.label("access_rights"), + _get_subquery_my_access_rights( + access_via_gid, access_via_folder_id + ).label("my_access_rights"), + _SUBQUERY_ACCESS_RIGHTS.label("access_rights"), ) .join( folders_access_rights, folders.c.id == folders_access_rights.c.folder_id @@ -1033,16 +1114,18 @@ async def folder_list( ) .where( _get_and_calsue_with_only_true_entries( - _required_permissions, folders_access_rights + required_permissions, folders_access_rights ) if folder_id is None else True ) - .offset(offset) - .limit(limit) ) - async for entry in connection.execute(query): - results.append(FolderEntry.from_orm(entry)) # noqa: PERF401s + query_result: RowProxy | None = await ( + await connection.execute(query.params(start_folder_id=folder_id)) + ).fetchone() - return results + if query_result is None: + raise FolderNotFoundError(folder_id=folder_id, gid=gid) + + return FolderEntry.from_orm(query_result) From 12cc597d91593426bceff317ed6c66370614f23d Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Thu, 8 Aug 2024 11:45:43 +0200 Subject: [PATCH 2/3] added tests --- .../utils_folders.py | 12 +--- .../tests/test_utils_folders.py | 67 +++++++++++++++++++ 2 files changed, 69 insertions(+), 10 deletions(-) diff --git a/packages/postgres-database/src/simcore_postgres_database/utils_folders.py b/packages/postgres-database/src/simcore_postgres_database/utils_folders.py index 1f344caecd0..9f3753ac612 100644 --- a/packages/postgres-database/src/simcore_postgres_database/utils_folders.py +++ b/packages/postgres-database/src/simcore_postgres_database/utils_folders.py @@ -1102,16 +1102,8 @@ async def folder_get( .join( folders_access_rights, folders.c.id == folders_access_rights.c.folder_id ) - .where( - folders_access_rights.c.traversal_parent_id.is_(None) - if folder_id is None - else folders_access_rights.c.traversal_parent_id == folder_id - ) - .where( - folders_access_rights.c.gid.in_([access_via_gid]) - if folder_id is None - else True - ) + .where(folders_access_rights.c.folder_id == folder_id) + .where(folders_access_rights.c.gid.in_([access_via_gid])) .where( _get_and_calsue_with_only_true_entries( required_permissions, folders_access_rights diff --git a/packages/postgres-database/tests/test_utils_folders.py b/packages/postgres-database/tests/test_utils_folders.py index 7a4714b4b5a..6660dd74010 100644 --- a/packages/postgres-database/tests/test_utils_folders.py +++ b/packages/postgres-database/tests/test_utils_folders.py @@ -48,6 +48,7 @@ folder_add_project, folder_create, folder_delete, + folder_get, folder_list, folder_move, folder_remove_project, @@ -2015,3 +2016,69 @@ async def test_folder_list_shared_with_different_permissions( ), }, ) + + +async def test_folder_get( + connection: SAConnection, + default_product_name: _ProductName, + get_unique_gids: Callable[[int], tuple[_GroupID, ...]], + make_folders: Callable[[set[MkFolder]], Awaitable[dict[str, _FolderID]]], +): + ####### + # SETUP + ####### + ( + gid_owner, + gid_other_owner, + gid_not_shared, + ) = get_unique_gids(3) + + folder_ids = await make_folders( + { + MkFolder( + name="owner_folder", + gid=gid_owner, + shared_with={ + gid_other_owner: FolderAccessRole.OWNER, + }, + children={ + *{MkFolder(name=f"f{i}", gid=gid_owner) for i in range(1, 3)}, + MkFolder( + name="f10", + gid=gid_owner, + children={ + MkFolder(name=f"sub_f{i}", gid=gid_owner) + for i in range(1, 3) + }, + ), + }, + ) + } + ) + + folder_id_owner_folder = folder_ids["owner_folder"] + folder_id_f1 = folder_ids["f1"] + folder_id_f2 = folder_ids["f2"] + folder_id_sub_f1 = folder_ids["sub_f1"] + folder_id_sub_f2 = folder_ids["sub_f2"] + + ####### + # TESTS + ####### + + for folder_id_to_list in ( + None, + folder_id_owner_folder, + folder_id_f1, + folder_id_f2, + folder_id_sub_f1, + folder_id_sub_f2, + ): + folder_entries = await _list_folder_as( + connection, default_product_name, folder_id_to_list, gid_owner + ) + for entry in folder_entries: + queried_folder = await folder_get( + connection, default_product_name, entry.id, entry.access_via_gid + ) + assert entry == queried_folder From 65d5419f4389d5e7524dae989737a519cbe4ab03 Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Thu, 8 Aug 2024 11:51:37 +0200 Subject: [PATCH 3/3] extend tests --- .../utils_folders.py | 22 ++++++------------- .../tests/test_utils_folders.py | 19 ++++++++++++++++ 2 files changed, 26 insertions(+), 15 deletions(-) diff --git a/packages/postgres-database/src/simcore_postgres_database/utils_folders.py b/packages/postgres-database/src/simcore_postgres_database/utils_folders.py index 9f3753ac612..57a51094b35 100644 --- a/packages/postgres-database/src/simcore_postgres_database/utils_folders.py +++ b/packages/postgres-database/src/simcore_postgres_database/utils_folders.py @@ -54,8 +54,6 @@ * CannotMoveFolderSharedViaNonPrimaryGroupError * BaseAddProjectError * ProjectAlreadyExistsInFolderError - * BaseFolderGet - * FolderNotFoundError """ @@ -72,9 +70,7 @@ class FolderAccessError(FoldersError): class FolderNotFoundError(FolderAccessError): - msg_template = ( - "no entry found for folder_id={folder_id} and product_name={product_name}" - ) + msg_template = "no entry found for folder_id={folder_id}, gid={gid} and product_name={product_name}" class FolderNotSharedWithGidError(FolderAccessError): @@ -125,14 +121,6 @@ class ProjectAlreadyExistsInFolderError(BaseAddProjectError): ) -class BaseFolderGet(FoldersError): - pass - - -class FolderNotFoundError(BaseFolderGet): - msg_template = "no entry found for folder_id={folder_id} and gid={gid}" - - ### ### UTILS ACCESS LAYER ### @@ -445,7 +433,9 @@ async def _check_folder_and_access( .where(folders.c.product_name == product_name) ) if not folder_entry: - raise FolderNotFoundError(folder_id=folder_id, product_name=product_name) + raise FolderNotFoundError( + folder_id=folder_id, gid=gid, product_name=product_name + ) # check if folder was shared resolved_access_rights_without_permissions = await _get_resolved_access_rights( @@ -1118,6 +1108,8 @@ async def folder_get( ).fetchone() if query_result is None: - raise FolderNotFoundError(folder_id=folder_id, gid=gid) + raise FolderNotFoundError( + folder_id=folder_id, gid=gid, product_name=product_name + ) return FolderEntry.from_orm(query_result) diff --git a/packages/postgres-database/tests/test_utils_folders.py b/packages/postgres-database/tests/test_utils_folders.py index 6660dd74010..2fdbd6bce8d 100644 --- a/packages/postgres-database/tests/test_utils_folders.py +++ b/packages/postgres-database/tests/test_utils_folders.py @@ -2066,6 +2066,7 @@ async def test_folder_get( # TESTS ####### + # 1. query exsisting directories for folder_id_to_list in ( None, folder_id_owner_folder, @@ -2082,3 +2083,21 @@ async def test_folder_get( connection, default_product_name, entry.id, entry.access_via_gid ) assert entry == queried_folder + + # 2. query via gid_not_shared + with pytest.raises(FolderNotSharedWithGidError): + await folder_get( + connection, default_product_name, folder_id_owner_folder, gid_not_shared + ) + + # 3. query with missing folder_id + missing_folder_id = 12312313123 + for gid_to_test in ( + gid_owner, + gid_other_owner, + gid_not_shared, + ): + with pytest.raises(FolderNotFoundError): + await folder_get( + connection, default_product_name, missing_folder_id, gid_to_test + )