From f3bb4a9676e099f5b8ddda6be1ad85957a251c3b Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 30 Oct 2024 11:07:45 +0100 Subject: [PATCH 01/26] update group user --- api/specs/web-server/_groups.py | 20 +++++++++---------- .../api_schemas_webserver/groups.py | 16 +++++++++++++++ .../groups/_handlers.py | 9 ++++----- 3 files changed, 30 insertions(+), 15 deletions(-) diff --git a/api/specs/web-server/_groups.py b/api/specs/web-server/_groups.py index 02c04b663fa..dade4d2f9e5 100644 --- a/api/specs/web-server/_groups.py +++ b/api/specs/web-server/_groups.py @@ -13,9 +13,13 @@ UsersGroup, ) from models_library.generics import Envelope -from models_library.users import GroupID, UserID +from models_library.users import GroupID from simcore_service_webserver._meta import API_VTAG -from simcore_service_webserver.groups._handlers import _ClassifiersQuery +from simcore_service_webserver.groups._handlers import ( + GroupUserPatch, + _ClassifiersQuery, + _GroupUserPathParams, +) from simcore_service_webserver.scicrunch.models import ResearchResource, ResourceHit router = APIRouter( @@ -91,8 +95,7 @@ async def add_group_user( response_model=Envelope[GroupUserGet], ) async def get_group_user( - gid: GroupID, - uid: UserID, + _p: Annotated[_GroupUserPathParams, Depends()], ): ... @@ -102,11 +105,9 @@ async def get_group_user( response_model=Envelope[GroupUserGet], ) async def update_group_user( - gid: GroupID, - uid: UserID, - _update: GroupUserGet, + _p: Annotated[_GroupUserPathParams, Depends()], + _b: GroupUserPatch, ): - # FIXME: update type ... @@ -115,8 +116,7 @@ async def update_group_user( status_code=status.HTTP_204_NO_CONTENT, ) async def delete_group_user( - gid: GroupID, - uid: UserID, + _p: Annotated[_GroupUserPathParams, Depends()], ): ... diff --git a/packages/models-library/src/models_library/api_schemas_webserver/groups.py b/packages/models-library/src/models_library/api_schemas_webserver/groups.py index e0b6d3fbb37..2f3835235aa 100644 --- a/packages/models-library/src/models_library/api_schemas_webserver/groups.py +++ b/packages/models-library/src/models_library/api_schemas_webserver/groups.py @@ -4,6 +4,7 @@ from pydantic import AnyUrl, BaseModel, Field, ValidationError, parse_obj_as, validator from ..emails import LowerCaseEmailStr +from ._base import InputSchema # # GROUPS MODELS defined in OPENAPI specs @@ -158,3 +159,18 @@ class Config: }, } } + + +class GroupUserPatch(InputSchema): + access_rights: GroupAccessRights | None = None + + class Config: + schema_extra: ClassVar[dict[str, Any]] = { + "example": { + "accessRights": { + "read": True, + "write": False, + "delete": False, + }, + } + } diff --git a/services/web/server/src/simcore_service_webserver/groups/_handlers.py b/services/web/server/src/simcore_service_webserver/groups/_handlers.py index b6284bed8bf..06d71928bce 100644 --- a/services/web/server/src/simcore_service_webserver/groups/_handlers.py +++ b/services/web/server/src/simcore_service_webserver/groups/_handlers.py @@ -7,6 +7,7 @@ from models_library.api_schemas_webserver.groups import ( AllUsersGroups, GroupUserGet, + GroupUserPatch, UsersGroup, ) from models_library.emails import LowerCaseEmailStr @@ -15,6 +16,7 @@ from pydantic import BaseModel, Extra, Field, parse_obj_as from servicelib.aiohttp import status from servicelib.aiohttp.requests_validation import ( + parse_request_body_as, parse_request_path_parameters_as, parse_request_query_parameters_as, ) @@ -249,18 +251,15 @@ async def get_group_user(request: web.Request): @permission_required("groups.*") @_handle_groups_exceptions async def update_group_user(request: web.Request): - """ - Modify specific user in group - """ req_ctx = _GroupsRequestContext.parse_obj(request) path_params = parse_request_path_parameters_as(_GroupUserPathParams, request) - new_values_for_user_in_group = await request.json() + update: GroupUserPatch = await parse_request_body_as(GroupUserPatch, request) user = await api.update_user_in_group( request.app, req_ctx.user_id, path_params.gid, path_params.uid, - new_values_for_user_in_group, + update.dict(exclude_unset=True), ) assert parse_obj_as(GroupUserGet, user) is not None # nosec return envelope_json_response(user) From 02ec18faa24693983130d187c9fb2b1618a4bdea Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 30 Oct 2024 11:11:47 +0100 Subject: [PATCH 02/26] rename GroupGet --- api/specs/web-server/_groups.py | 19 ++++++++++--------- .../api_schemas_webserver/groups.py | 14 +++++++------- .../groups/_handlers.py | 8 ++++---- .../tests/unit/isolated/test_groups_models.py | 6 +++--- 4 files changed, 24 insertions(+), 23 deletions(-) diff --git a/api/specs/web-server/_groups.py b/api/specs/web-server/_groups.py index dade4d2f9e5..667e61b494d 100644 --- a/api/specs/web-server/_groups.py +++ b/api/specs/web-server/_groups.py @@ -9,8 +9,8 @@ from fastapi import APIRouter, Depends, status from models_library.api_schemas_webserver.groups import ( AllUsersGroups, + GroupGet, GroupUserGet, - UsersGroup, ) from models_library.generics import Envelope from models_library.users import GroupID @@ -18,6 +18,7 @@ from simcore_service_webserver.groups._handlers import ( GroupUserPatch, _ClassifiersQuery, + _GroupPathParams, _GroupUserPathParams, ) from simcore_service_webserver.scicrunch.models import ResearchResource, ResourceHit @@ -40,7 +41,7 @@ async def list_groups(): @router.post( "/groups", - response_model=Envelope[UsersGroup], + response_model=Envelope[GroupGet], status_code=status.HTTP_201_CREATED, ) async def create_group(): @@ -49,17 +50,17 @@ async def create_group(): @router.get( "/groups/{gid}", - response_model=Envelope[UsersGroup], + response_model=Envelope[GroupGet], ) -async def get_group(gid: GroupID): +async def get_group(_p: Annotated[_GroupPathParams, Depends()]): ... @router.patch( "/groups/{gid}", - response_model=Envelope[UsersGroup], + response_model=Envelope[GroupGet], ) -async def update_group(gid: GroupID, _update: UsersGroup): +async def update_group(_p: Annotated[_GroupPathParams, Depends()], _update: GroupGet): ... @@ -67,7 +68,7 @@ async def update_group(gid: GroupID, _update: UsersGroup): "/groups/{gid}", status_code=status.HTTP_204_NO_CONTENT, ) -async def delete_group(gid: GroupID): +async def delete_group(_p: Annotated[_GroupPathParams, Depends()]): ... @@ -75,7 +76,7 @@ async def delete_group(gid: GroupID): "/groups/{gid}/users", response_model=Envelope[list[GroupUserGet]], ) -async def get_group_users(gid: GroupID): +async def get_group_users(_p: Annotated[_GroupPathParams, Depends()]): ... @@ -84,7 +85,7 @@ async def get_group_users(gid: GroupID): status_code=status.HTTP_204_NO_CONTENT, ) async def add_group_user( - gid: GroupID, + _p: Annotated[_GroupPathParams, Depends()], _new: GroupUserGet, ): ... diff --git a/packages/models-library/src/models_library/api_schemas_webserver/groups.py b/packages/models-library/src/models_library/api_schemas_webserver/groups.py index 2f3835235aa..fc51cdef69a 100644 --- a/packages/models-library/src/models_library/api_schemas_webserver/groups.py +++ b/packages/models-library/src/models_library/api_schemas_webserver/groups.py @@ -4,7 +4,7 @@ from pydantic import AnyUrl, BaseModel, Field, ValidationError, parse_obj_as, validator from ..emails import LowerCaseEmailStr -from ._base import InputSchema +from ._base import InputSchema, OutputSchema # # GROUPS MODELS defined in OPENAPI specs @@ -30,7 +30,7 @@ class Config: } -class UsersGroup(BaseModel): +class GroupGet(OutputSchema): gid: int = Field(..., description="the group ID") label: str = Field(..., description="the group name") description: str = Field(..., description="the group description") @@ -46,7 +46,7 @@ class UsersGroup(BaseModel): @validator("thumbnail", pre=True) @classmethod - def sanitize_legacy_data(cls, v): + def _sanitize_legacy_data(cls, v): if v: # Enforces null if thumbnail is not valid URL or empty with suppress(ValidationError): @@ -88,10 +88,10 @@ class Config: class AllUsersGroups(BaseModel): - me: UsersGroup | None = None - organizations: list[UsersGroup] | None = None - all: UsersGroup | None = None - product: UsersGroup | None = None + me: GroupGet | None = None + organizations: list[GroupGet] | None = None + all: GroupGet | None = None + product: GroupGet | None = None class Config: schema_extra: ClassVar[dict[str, Any]] = { diff --git a/services/web/server/src/simcore_service_webserver/groups/_handlers.py b/services/web/server/src/simcore_service_webserver/groups/_handlers.py index 06d71928bce..d3623791b1a 100644 --- a/services/web/server/src/simcore_service_webserver/groups/_handlers.py +++ b/services/web/server/src/simcore_service_webserver/groups/_handlers.py @@ -6,9 +6,9 @@ from aiohttp import web from models_library.api_schemas_webserver.groups import ( AllUsersGroups, + GroupGet, GroupUserGet, GroupUserPatch, - UsersGroup, ) from models_library.emails import LowerCaseEmailStr from models_library.users import GroupID, UserID @@ -127,7 +127,7 @@ async def get_group(request: web.Request): path_params = parse_request_path_parameters_as(_GroupPathParams, request) group = await api.get_user_group(request.app, req_ctx.user_id, path_params.gid) - assert parse_obj_as(UsersGroup, group) is not None # nosec + assert parse_obj_as(GroupGet, group) is not None # nosec return group @@ -141,7 +141,7 @@ async def create_group(request: web.Request): new_group = await request.json() created_group = await api.create_user_group(request.app, req_ctx.user_id, new_group) - assert parse_obj_as(UsersGroup, created_group) is not None # nosec + assert parse_obj_as(GroupGet, created_group) is not None # nosec raise web.HTTPCreated( text=json_dumps({"data": created_group}), content_type=MIMETYPE_APPLICATION_JSON ) @@ -159,7 +159,7 @@ async def update_group(request: web.Request): updated_group = await api.update_user_group( request.app, req_ctx.user_id, path_params.gid, new_group_values ) - assert parse_obj_as(UsersGroup, updated_group) is not None # nosec + assert parse_obj_as(GroupGet, updated_group) is not None # nosec return envelope_json_response(updated_group) diff --git a/services/web/server/tests/unit/isolated/test_groups_models.py b/services/web/server/tests/unit/isolated/test_groups_models.py index 10b49979b1c..3a5fd5e91ce 100644 --- a/services/web/server/tests/unit/isolated/test_groups_models.py +++ b/services/web/server/tests/unit/isolated/test_groups_models.py @@ -1,6 +1,6 @@ import models_library.groups import simcore_postgres_database.models.groups -from models_library.api_schemas_webserver.groups import UsersGroup +from models_library.api_schemas_webserver.groups import GroupGet from models_library.utils.enums import enum_to_dict @@ -14,7 +14,7 @@ def test_models_library_and_postgress_database_enums_are_equivalent(): def test_sanitize_legacy_data(): - users_group_1 = UsersGroup.parse_obj( + users_group_1 = GroupGet.parse_obj( { "gid": "27", "label": "A user", @@ -26,7 +26,7 @@ def test_sanitize_legacy_data(): assert users_group_1.thumbnail is None - users_group_2 = UsersGroup.parse_obj( + users_group_2 = GroupGet.parse_obj( { "gid": "27", "label": "A user", From b280ba9ff77547760d919f8dca80f0062b462a22 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 30 Oct 2024 11:17:08 +0100 Subject: [PATCH 03/26] GroupPatch --- api/specs/web-server/_groups.py | 6 +++++- .../src/models_library/api_schemas_webserver/groups.py | 6 ++++++ .../src/simcore_service_webserver/groups/_handlers.py | 4 +++- 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/api/specs/web-server/_groups.py b/api/specs/web-server/_groups.py index 667e61b494d..3ade7de7d1a 100644 --- a/api/specs/web-server/_groups.py +++ b/api/specs/web-server/_groups.py @@ -10,6 +10,7 @@ from models_library.api_schemas_webserver.groups import ( AllUsersGroups, GroupGet, + GroupPatch, GroupUserGet, ) from models_library.generics import Envelope @@ -60,7 +61,10 @@ async def get_group(_p: Annotated[_GroupPathParams, Depends()]): "/groups/{gid}", response_model=Envelope[GroupGet], ) -async def update_group(_p: Annotated[_GroupPathParams, Depends()], _update: GroupGet): +async def update_group( + _p: Annotated[_GroupPathParams, Depends()], + _b: GroupPatch, +): ... diff --git a/packages/models-library/src/models_library/api_schemas_webserver/groups.py b/packages/models-library/src/models_library/api_schemas_webserver/groups.py index fc51cdef69a..da4052f13eb 100644 --- a/packages/models-library/src/models_library/api_schemas_webserver/groups.py +++ b/packages/models-library/src/models_library/api_schemas_webserver/groups.py @@ -87,6 +87,12 @@ class Config: } +class GroupPatch(InputSchema): + label: str | None = None + description: str | None = None + thumbnail: AnyUrl | None = None + + class AllUsersGroups(BaseModel): me: GroupGet | None = None organizations: list[GroupGet] | None = None diff --git a/services/web/server/src/simcore_service_webserver/groups/_handlers.py b/services/web/server/src/simcore_service_webserver/groups/_handlers.py index d3623791b1a..41d98d02921 100644 --- a/services/web/server/src/simcore_service_webserver/groups/_handlers.py +++ b/services/web/server/src/simcore_service_webserver/groups/_handlers.py @@ -7,6 +7,7 @@ from models_library.api_schemas_webserver.groups import ( AllUsersGroups, GroupGet, + GroupPatch, GroupUserGet, GroupUserPatch, ) @@ -154,7 +155,8 @@ async def create_group(request: web.Request): async def update_group(request: web.Request): req_ctx = _GroupsRequestContext.parse_obj(request) path_params = parse_request_path_parameters_as(_GroupPathParams, request) - new_group_values = await request.json() + update: GroupPatch = await parse_request_body_as(GroupPatch, request) + new_group_values = update.dict(exclude_unset=True) updated_group = await api.update_user_group( request.app, req_ctx.user_id, path_params.gid, new_group_values From c6f8c8f3c7b95669bf302b2fbb81b5e96c5e1e79 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 30 Oct 2024 11:18:18 +0100 Subject: [PATCH 04/26] OAS --- .../api/v0/openapi.yaml | 135 +++++++++++------- 1 file changed, 81 insertions(+), 54 deletions(-) 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 0df5e076a76..dc44e2c61fc 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 @@ -473,7 +473,7 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/Envelope_UsersGroup_' + $ref: '#/components/schemas/Envelope_GroupGet_' /v0/groups/{gid}: get: tags: @@ -495,7 +495,7 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/Envelope_UsersGroup_' + $ref: '#/components/schemas/Envelope_GroupGet_' delete: tags: - groups @@ -531,7 +531,7 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/UsersGroup' + $ref: '#/components/schemas/GroupPatch' required: true responses: '200': @@ -539,7 +539,7 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/Envelope_UsersGroup_' + $ref: '#/components/schemas/Envelope_GroupGet_' /v0/groups/{gid}/users: get: tags: @@ -666,7 +666,7 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/GroupUserGet' + $ref: '#/components/schemas/GroupUserPatch' required: true responses: '200': @@ -4311,7 +4311,7 @@ paths: '403': description: ProjectInvalidRightsError '404': - description: UserDefaultWalletNotFoundError, ProjectNotFoundError + description: ProjectNotFoundError, UserDefaultWalletNotFoundError '409': description: ProjectTooManyProjectOpenedError '422': @@ -6027,16 +6027,16 @@ components: type: object properties: me: - $ref: '#/components/schemas/UsersGroup' + $ref: '#/components/schemas/GroupGet' organizations: title: Organizations type: array items: - $ref: '#/components/schemas/UsersGroup' + $ref: '#/components/schemas/GroupGet' all: - $ref: '#/components/schemas/UsersGroup' + $ref: '#/components/schemas/GroupGet' product: - $ref: '#/components/schemas/UsersGroup' + $ref: '#/components/schemas/GroupGet' example: me: gid: '27' @@ -7290,6 +7290,14 @@ components: $ref: '#/components/schemas/GetWalletAutoRecharge' error: title: Error + Envelope_GroupGet_: + title: Envelope[GroupGet] + type: object + properties: + data: + $ref: '#/components/schemas/GroupGet' + error: + title: Error Envelope_GroupUserGet_: title: Envelope[GroupUserGet] type: object @@ -7587,14 +7595,6 @@ components: $ref: '#/components/schemas/UserProfile' error: title: Error - Envelope_UsersGroup_: - title: Envelope[UsersGroup] - type: object - properties: - data: - $ref: '#/components/schemas/UsersGroup' - error: - title: Error Envelope_WalletGetWithAvailableCredits_: title: Envelope[WalletGetWithAvailableCredits] type: object @@ -8657,6 +8657,58 @@ components: title: Delete type: boolean description: defines acesss rights for the user + GroupGet: + title: GroupGet + required: + - gid + - label + - description + - accessRights + type: object + properties: + gid: + title: Gid + type: integer + description: the group ID + label: + title: Label + type: string + description: the group name + description: + title: Description + type: string + description: the group description + thumbnail: + title: Thumbnail + maxLength: 65536 + minLength: 1 + type: string + description: url to the group thumbnail + format: uri + accessRights: + $ref: '#/components/schemas/GroupAccessRights' + inclusionRules: + title: Inclusionrules + type: object + additionalProperties: + type: string + description: Maps user's column and regular expression + GroupPatch: + title: GroupPatch + type: object + properties: + label: + title: Label + type: string + description: + title: Description + type: string + thumbnail: + title: Thumbnail + maxLength: 65536 + minLength: 1 + type: string + format: uri GroupUserGet: title: GroupUserGet required: @@ -8701,6 +8753,17 @@ components: read: true write: false delete: false + GroupUserPatch: + title: GroupUserPatch + type: object + properties: + accessRights: + $ref: '#/components/schemas/GroupAccessRights' + example: + accessRights: + read: true + write: false + delete: false HardwareInfo: title: HardwareInfo required: @@ -12587,42 +12650,6 @@ components: - DELETED type: string description: An enumeration. - UsersGroup: - title: UsersGroup - required: - - gid - - label - - description - - accessRights - type: object - properties: - gid: - title: Gid - type: integer - description: the group ID - label: - title: Label - type: string - description: the group name - description: - title: Description - type: string - description: the group description - thumbnail: - title: Thumbnail - maxLength: 65536 - minLength: 1 - type: string - description: url to the group thumbnail - format: uri - accessRights: - $ref: '#/components/schemas/GroupAccessRights' - inclusionRules: - title: Inclusionrules - type: object - additionalProperties: - type: string - description: Maps user's column and regular expression Viewer: title: Viewer required: From 9abb98569d2af2c1ce27414ba09ef5f41a2facaa Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 30 Oct 2024 11:29:44 +0100 Subject: [PATCH 05/26] group create and rename patch by udpate --- api/specs/web-server/_groups.py | 11 ++++---- .../api_schemas_webserver/groups.py | 10 ++++++-- .../groups/_handlers.py | 25 +++++++++++-------- 3 files changed, 28 insertions(+), 18 deletions(-) diff --git a/api/specs/web-server/_groups.py b/api/specs/web-server/_groups.py index 3ade7de7d1a..12bbee0063b 100644 --- a/api/specs/web-server/_groups.py +++ b/api/specs/web-server/_groups.py @@ -9,15 +9,16 @@ from fastapi import APIRouter, Depends, status from models_library.api_schemas_webserver.groups import ( AllUsersGroups, + GroupCreate, GroupGet, - GroupPatch, + GroupUpdate, GroupUserGet, ) from models_library.generics import Envelope from models_library.users import GroupID from simcore_service_webserver._meta import API_VTAG from simcore_service_webserver.groups._handlers import ( - GroupUserPatch, + GroupUserUpdate, _ClassifiersQuery, _GroupPathParams, _GroupUserPathParams, @@ -45,7 +46,7 @@ async def list_groups(): response_model=Envelope[GroupGet], status_code=status.HTTP_201_CREATED, ) -async def create_group(): +async def create_group(_b: GroupCreate): ... @@ -63,7 +64,7 @@ async def get_group(_p: Annotated[_GroupPathParams, Depends()]): ) async def update_group( _p: Annotated[_GroupPathParams, Depends()], - _b: GroupPatch, + _b: GroupUpdate, ): ... @@ -111,7 +112,7 @@ async def get_group_user( ) async def update_group_user( _p: Annotated[_GroupUserPathParams, Depends()], - _b: GroupUserPatch, + _b: GroupUserUpdate, ): ... diff --git a/packages/models-library/src/models_library/api_schemas_webserver/groups.py b/packages/models-library/src/models_library/api_schemas_webserver/groups.py index da4052f13eb..10983d18da7 100644 --- a/packages/models-library/src/models_library/api_schemas_webserver/groups.py +++ b/packages/models-library/src/models_library/api_schemas_webserver/groups.py @@ -87,7 +87,13 @@ class Config: } -class GroupPatch(InputSchema): +class GroupCreate(InputSchema): + label: str + description: str + thumbnail: AnyUrl | None = None + + +class GroupUpdate(InputSchema): label: str | None = None description: str | None = None thumbnail: AnyUrl | None = None @@ -167,7 +173,7 @@ class Config: } -class GroupUserPatch(InputSchema): +class GroupUserUpdate(InputSchema): access_rights: GroupAccessRights | None = None class Config: diff --git a/services/web/server/src/simcore_service_webserver/groups/_handlers.py b/services/web/server/src/simcore_service_webserver/groups/_handlers.py index 41d98d02921..52d76649862 100644 --- a/services/web/server/src/simcore_service_webserver/groups/_handlers.py +++ b/services/web/server/src/simcore_service_webserver/groups/_handlers.py @@ -6,14 +6,14 @@ from aiohttp import web from models_library.api_schemas_webserver.groups import ( AllUsersGroups, + GroupCreate, GroupGet, - GroupPatch, + GroupUpdate, GroupUserGet, - GroupUserPatch, + GroupUserUpdate, ) from models_library.emails import LowerCaseEmailStr from models_library.users import GroupID, UserID -from models_library.utils.json_serialization import json_dumps from pydantic import BaseModel, Extra, Field, parse_obj_as from servicelib.aiohttp import status from servicelib.aiohttp.requests_validation import ( @@ -22,7 +22,6 @@ parse_request_query_parameters_as, ) from servicelib.aiohttp.typing_extension import Handler -from servicelib.mimetype_constants import MIMETYPE_APPLICATION_JSON from .._constants import RQ_PRODUCT_KEY, RQT_USERID_KEY from .._meta import API_VTAG @@ -139,13 +138,13 @@ async def get_group(request: web.Request): async def create_group(request: web.Request): """Creates organization groups""" req_ctx = _GroupsRequestContext.parse_obj(request) - new_group = await request.json() + create = await parse_request_body_as(GroupCreate, request) + new_group = create.dict(exclude_unset=True) created_group = await api.create_user_group(request.app, req_ctx.user_id, new_group) assert parse_obj_as(GroupGet, created_group) is not None # nosec - raise web.HTTPCreated( - text=json_dumps({"data": created_group}), content_type=MIMETYPE_APPLICATION_JSON - ) + + return envelope_json_response(created_group, status_cls=web.HTTPCreated) @routes.patch(f"/{API_VTAG}/groups/{{gid}}", name="update_group") @@ -153,9 +152,10 @@ async def create_group(request: web.Request): @permission_required("groups.*") @_handle_groups_exceptions async def update_group(request: web.Request): + """Updates organization groups""" req_ctx = _GroupsRequestContext.parse_obj(request) path_params = parse_request_path_parameters_as(_GroupPathParams, request) - update: GroupPatch = await parse_request_body_as(GroupPatch, request) + update: GroupUpdate = await parse_request_body_as(GroupUpdate, request) new_group_values = update.dict(exclude_unset=True) updated_group = await api.update_user_group( @@ -170,6 +170,7 @@ async def update_group(request: web.Request): @permission_required("groups.*") @_handle_groups_exceptions async def delete_group(request: web.Request): + """Deletes organization groups""" req_ctx = _GroupsRequestContext.parse_obj(request) path_params = parse_request_path_parameters_as(_GroupPathParams, request) @@ -182,6 +183,7 @@ async def delete_group(request: web.Request): @permission_required("groups.*") @_handle_groups_exceptions async def get_group_users(request: web.Request): + """Gets users in organization groups""" req_ctx = _GroupsRequestContext.parse_obj(request) path_params = parse_request_path_parameters_as(_GroupPathParams, request) @@ -237,7 +239,7 @@ class Config: @_handle_groups_exceptions async def get_group_user(request: web.Request): """ - Gets specific user in group + Gets specific user in an organization group """ req_ctx = _GroupsRequestContext.parse_obj(request) path_params = parse_request_path_parameters_as(_GroupUserPathParams, request) @@ -255,7 +257,8 @@ async def get_group_user(request: web.Request): async def update_group_user(request: web.Request): req_ctx = _GroupsRequestContext.parse_obj(request) path_params = parse_request_path_parameters_as(_GroupUserPathParams, request) - update: GroupUserPatch = await parse_request_body_as(GroupUserPatch, request) + update: GroupUserUpdate = await parse_request_body_as(GroupUserUpdate, request) + user = await api.update_user_in_group( request.app, req_ctx.user_id, From 53fcceae30204b070307fe2b12bd0420e9521014 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 30 Oct 2024 11:30:13 +0100 Subject: [PATCH 06/26] update OAS --- .../api/v0/openapi.yaml | 37 ++++++++++++++++--- 1 file changed, 31 insertions(+), 6 deletions(-) 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 dc44e2c61fc..e98491491b5 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 @@ -467,6 +467,12 @@ paths: - groups summary: Create Group operationId: create_group + requestBody: + content: + application/json: + schema: + $ref: '#/components/schemas/GroupCreate' + required: true responses: '201': description: Successful Response @@ -531,7 +537,7 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/GroupPatch' + $ref: '#/components/schemas/GroupUpdate' required: true responses: '200': @@ -666,7 +672,7 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/GroupUserPatch' + $ref: '#/components/schemas/GroupUserUpdate' required: true responses: '200': @@ -8657,6 +8663,25 @@ components: title: Delete type: boolean description: defines acesss rights for the user + GroupCreate: + title: GroupCreate + required: + - label + - description + type: object + properties: + label: + title: Label + type: string + description: + title: Description + type: string + thumbnail: + title: Thumbnail + maxLength: 65536 + minLength: 1 + type: string + format: uri GroupGet: title: GroupGet required: @@ -8693,8 +8718,8 @@ components: additionalProperties: type: string description: Maps user's column and regular expression - GroupPatch: - title: GroupPatch + GroupUpdate: + title: GroupUpdate type: object properties: label: @@ -8753,8 +8778,8 @@ components: read: true write: false delete: false - GroupUserPatch: - title: GroupUserPatch + GroupUserUpdate: + title: GroupUserUpdate type: object properties: accessRights: From 9728da1a51217a4695a1faec8ebe467c93f66809 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 30 Oct 2024 11:33:43 +0100 Subject: [PATCH 07/26] updates OAS --- api/specs/web-server/_groups.py | 20 ++++++++++++------- .../api/v0/openapi.yaml | 7 +++++++ 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/api/specs/web-server/_groups.py b/api/specs/web-server/_groups.py index 12bbee0063b..5cf350abfbf 100644 --- a/api/specs/web-server/_groups.py +++ b/api/specs/web-server/_groups.py @@ -55,7 +55,7 @@ async def create_group(_b: GroupCreate): response_model=Envelope[GroupGet], ) async def get_group(_p: Annotated[_GroupPathParams, Depends()]): - ... + """Creates organization groups""" @router.patch( @@ -66,7 +66,7 @@ async def update_group( _p: Annotated[_GroupPathParams, Depends()], _b: GroupUpdate, ): - ... + """Updates organization groups""" @router.delete( @@ -74,7 +74,7 @@ async def update_group( status_code=status.HTTP_204_NO_CONTENT, ) async def delete_group(_p: Annotated[_GroupPathParams, Depends()]): - ... + """Deletes organization groups""" @router.get( @@ -82,7 +82,7 @@ async def delete_group(_p: Annotated[_GroupPathParams, Depends()]): response_model=Envelope[list[GroupUserGet]], ) async def get_group_users(_p: Annotated[_GroupPathParams, Depends()]): - ... + """Gets users in organization groups""" @router.post( @@ -93,7 +93,9 @@ async def add_group_user( _p: Annotated[_GroupPathParams, Depends()], _new: GroupUserGet, ): - ... + """ + Adds a user to an organization group + """ @router.get( @@ -103,7 +105,9 @@ async def add_group_user( async def get_group_user( _p: Annotated[_GroupUserPathParams, Depends()], ): - ... + """ + Gets specific user in an organization group + """ @router.patch( @@ -114,7 +118,9 @@ async def update_group_user( _p: Annotated[_GroupUserPathParams, Depends()], _b: GroupUserUpdate, ): - ... + """ + Updates user (access) to an organization group + """ @router.delete( 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 e98491491b5..8683ea405f6 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 @@ -485,6 +485,7 @@ paths: tags: - groups summary: Get Group + description: Creates organization groups operationId: get_group parameters: - required: true @@ -506,6 +507,7 @@ paths: tags: - groups summary: Delete Group + description: Deletes organization groups operationId: delete_group parameters: - required: true @@ -523,6 +525,7 @@ paths: tags: - groups summary: Update Group + description: Updates organization groups operationId: update_group parameters: - required: true @@ -551,6 +554,7 @@ paths: tags: - groups summary: Get Group Users + description: Gets users in organization groups operationId: get_group_users parameters: - required: true @@ -572,6 +576,7 @@ paths: tags: - groups summary: Add Group User + description: Adds a user to an organization group operationId: add_group_user parameters: - required: true @@ -596,6 +601,7 @@ paths: tags: - groups summary: Get Group User + description: Gets specific user in an organization group operationId: get_group_user parameters: - required: true @@ -650,6 +656,7 @@ paths: tags: - groups summary: Update Group User + description: Updates user (access) to an organization group operationId: update_group_user parameters: - required: true From 5f88a2bd550e81bba06a7c94a4a7b6e4010a4821 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 30 Oct 2024 11:36:25 +0100 Subject: [PATCH 08/26] doc --- api/specs/web-server/_groups.py | 30 ++++++++++++++----- .../api/v0/openapi.yaml | 10 +++++-- 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/api/specs/web-server/_groups.py b/api/specs/web-server/_groups.py index 5cf350abfbf..46666c38ff0 100644 --- a/api/specs/web-server/_groups.py +++ b/api/specs/web-server/_groups.py @@ -38,7 +38,9 @@ response_model=Envelope[AllUsersGroups], ) async def list_groups(): - ... + """ + List all possible groups (organizations, primary, everyone and products) + """ @router.post( @@ -47,7 +49,9 @@ async def list_groups(): status_code=status.HTTP_201_CREATED, ) async def create_group(_b: GroupCreate): - ... + """ + Creates an organization group + """ @router.get( @@ -55,7 +59,9 @@ async def create_group(_b: GroupCreate): response_model=Envelope[GroupGet], ) async def get_group(_p: Annotated[_GroupPathParams, Depends()]): - """Creates organization groups""" + """ + Get an organization group + """ @router.patch( @@ -66,7 +72,9 @@ async def update_group( _p: Annotated[_GroupPathParams, Depends()], _b: GroupUpdate, ): - """Updates organization groups""" + """ + Updates organization groups + """ @router.delete( @@ -74,7 +82,9 @@ async def update_group( status_code=status.HTTP_204_NO_CONTENT, ) async def delete_group(_p: Annotated[_GroupPathParams, Depends()]): - """Deletes organization groups""" + """ + Deletes organization groups + """ @router.get( @@ -82,7 +92,9 @@ async def delete_group(_p: Annotated[_GroupPathParams, Depends()]): response_model=Envelope[list[GroupUserGet]], ) async def get_group_users(_p: Annotated[_GroupPathParams, Depends()]): - """Gets users in organization groups""" + """ + Gets users in organization groups + """ @router.post( @@ -119,7 +131,7 @@ async def update_group_user( _b: GroupUserUpdate, ): """ - Updates user (access) to an organization group + Updates user (access-rights) to an organization group """ @@ -130,7 +142,9 @@ async def update_group_user( async def delete_group_user( _p: Annotated[_GroupUserPathParams, Depends()], ): - ... + """ + Removes a user from an organization group + """ @router.get( 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 8683ea405f6..7418e48bbe6 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 @@ -454,6 +454,8 @@ paths: tags: - groups summary: List Groups + description: List all possible groups (organizations, primary, everyone and + products) operationId: list_groups responses: '200': @@ -466,6 +468,7 @@ paths: tags: - groups summary: Create Group + description: Creates an organization group operationId: create_group requestBody: content: @@ -485,7 +488,7 @@ paths: tags: - groups summary: Get Group - description: Creates organization groups + description: Get an organization group operationId: get_group parameters: - required: true @@ -631,6 +634,7 @@ paths: tags: - groups summary: Delete Group User + description: Removes a user from an organization group operationId: delete_group_user parameters: - required: true @@ -656,7 +660,7 @@ paths: tags: - groups summary: Update Group User - description: Updates user (access) to an organization group + description: Updates user (access-rights) to an organization group operationId: update_group_user parameters: - required: true @@ -4324,7 +4328,7 @@ paths: '403': description: ProjectInvalidRightsError '404': - description: ProjectNotFoundError, UserDefaultWalletNotFoundError + description: UserDefaultWalletNotFoundError, ProjectNotFoundError '409': description: ProjectTooManyProjectOpenedError '422': From 4d202f196c5cdbe39d66c459af5e7f37fff33b0e Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 30 Oct 2024 11:39:13 +0100 Subject: [PATCH 09/26] mygroups --- api/specs/web-server/_groups.py | 11 +- .../api_schemas_webserver/groups.py | 2 +- .../api/v0/openapi.yaml | 120 +++++++++--------- .../groups/_handlers.py | 34 +++-- .../users/schemas.py | 4 +- 5 files changed, 92 insertions(+), 79 deletions(-) diff --git a/api/specs/web-server/_groups.py b/api/specs/web-server/_groups.py index 46666c38ff0..67f307f6bf6 100644 --- a/api/specs/web-server/_groups.py +++ b/api/specs/web-server/_groups.py @@ -8,11 +8,11 @@ from fastapi import APIRouter, Depends, status from models_library.api_schemas_webserver.groups import ( - AllUsersGroups, GroupCreate, GroupGet, GroupUpdate, GroupUserGet, + MyGroupsGet, ) from models_library.generics import Envelope from models_library.users import GroupID @@ -35,11 +35,11 @@ @router.get( "/groups", - response_model=Envelope[AllUsersGroups], + response_model=Envelope[MyGroupsGet], ) async def list_groups(): """ - List all possible groups (organizations, primary, everyone and products) + List all groups (organizations, primary, everyone and products) I belong to """ @@ -147,6 +147,11 @@ async def delete_group_user( """ +# +# Classifiers +# + + @router.get( "/groups/{gid}/classifiers", response_model=Envelope[dict[str, Any]], diff --git a/packages/models-library/src/models_library/api_schemas_webserver/groups.py b/packages/models-library/src/models_library/api_schemas_webserver/groups.py index 10983d18da7..ee26d2d5998 100644 --- a/packages/models-library/src/models_library/api_schemas_webserver/groups.py +++ b/packages/models-library/src/models_library/api_schemas_webserver/groups.py @@ -99,7 +99,7 @@ class GroupUpdate(InputSchema): thumbnail: AnyUrl | None = None -class AllUsersGroups(BaseModel): +class MyGroupsGet(OutputSchema): me: GroupGet | None = None organizations: list[GroupGet] | None = None all: GroupGet | None = None 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 7418e48bbe6..6ff19b255a8 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 @@ -454,8 +454,8 @@ paths: tags: - groups summary: List Groups - description: List all possible groups (organizations, primary, everyone and - products) + description: List all groups (organizations, primary, everyone and products) + I belong to operationId: list_groups responses: '200': @@ -463,7 +463,7 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/Envelope_AllUsersGroups_' + $ref: '#/components/schemas/Envelope_MyGroupsGet_' post: tags: - groups @@ -6039,54 +6039,6 @@ components: queued: title: Queued type: boolean - AllUsersGroups: - title: AllUsersGroups - type: object - properties: - me: - $ref: '#/components/schemas/GroupGet' - organizations: - title: Organizations - type: array - items: - $ref: '#/components/schemas/GroupGet' - all: - $ref: '#/components/schemas/GroupGet' - product: - $ref: '#/components/schemas/GroupGet' - example: - me: - gid: '27' - label: A user - description: A very special user - accessRights: - read: true - write: true - delete: true - organizations: - - gid: '15' - label: ITIS Foundation - description: The Foundation for Research on Information Technologies in - Society - accessRights: - read: true - write: false - delete: false - - gid: '16' - label: Blue Fundation - description: Some foundation - accessRights: - read: true - write: false - delete: false - all: - gid: '0' - label: All - description: Open to all users - accessRights: - read: true - write: false - delete: false Annotation: title: Annotation required: @@ -7151,14 +7103,6 @@ components: type: object properties: {} additionalProperties: false - Envelope_AllUsersGroups_: - title: Envelope[AllUsersGroups] - type: object - properties: - data: - $ref: '#/components/schemas/AllUsersGroups' - error: - title: Error Envelope_AnyUrl_: title: Envelope[AnyUrl] type: object @@ -7363,6 +7307,14 @@ components: $ref: '#/components/schemas/LoginNextPage' error: title: Error + Envelope_MyGroupsGet_: + title: Envelope[MyGroupsGet] + type: object + properties: + data: + $ref: '#/components/schemas/MyGroupsGet' + error: + title: Error Envelope_NodeCreated_: title: Envelope[NodeCreated] type: object @@ -9092,6 +9044,54 @@ components: type: string format: color additionalProperties: false + MyGroupsGet: + title: MyGroupsGet + type: object + properties: + me: + $ref: '#/components/schemas/GroupGet' + organizations: + title: Organizations + type: array + items: + $ref: '#/components/schemas/GroupGet' + all: + $ref: '#/components/schemas/GroupGet' + product: + $ref: '#/components/schemas/GroupGet' + example: + me: + gid: '27' + label: A user + description: A very special user + accessRights: + read: true + write: true + delete: true + organizations: + - gid: '15' + label: ITIS Foundation + description: The Foundation for Research on Information Technologies in + Society + accessRights: + read: true + write: false + delete: false + - gid: '16' + label: Blue Fundation + description: Some foundation + accessRights: + read: true + write: false + delete: false + all: + gid: '0' + label: All + description: Open to all users + accessRights: + read: true + write: false + delete: false NoAuthentication: title: NoAuthentication type: object @@ -10255,7 +10255,7 @@ components: - ADMIN type: string groups: - $ref: '#/components/schemas/AllUsersGroups' + $ref: '#/components/schemas/MyGroupsGet' gravatar_id: title: Gravatar Id type: string diff --git a/services/web/server/src/simcore_service_webserver/groups/_handlers.py b/services/web/server/src/simcore_service_webserver/groups/_handlers.py index 52d76649862..ceadd0e4974 100644 --- a/services/web/server/src/simcore_service_webserver/groups/_handlers.py +++ b/services/web/server/src/simcore_service_webserver/groups/_handlers.py @@ -5,12 +5,12 @@ from aiohttp import web from models_library.api_schemas_webserver.groups import ( - AllUsersGroups, GroupCreate, GroupGet, GroupUpdate, GroupUserGet, GroupUserUpdate, + MyGroupsGet, ) from models_library.emails import LowerCaseEmailStr from models_library.users import GroupID, UserID @@ -79,11 +79,9 @@ async def wrapper(request: web.Request) -> web.StreamResponse: @permission_required("groups.read") @_handle_groups_exceptions async def list_groups(request: web.Request): - """Lists my groups - - List of the groups I belonged to """ - + List all groups (organizations, primary, everyone and products) I belong to + """ product: Product = get_current_product(request) req_ctx = _GroupsRequestContext.parse_obj(request) @@ -91,7 +89,7 @@ async def list_groups(request: web.Request): request.app, req_ctx.user_id ) - result = { + my_group = { "me": primary_group, "organizations": user_groups, "all": all_group, @@ -100,14 +98,19 @@ async def list_groups(request: web.Request): if product.group_id: with suppress(GroupNotFoundError): - result["product"] = await api.get_product_group_for_user( + my_group["product"] = await api.get_product_group_for_user( app=request.app, user_id=req_ctx.user_id, product_gid=product.group_id, ) - assert parse_obj_as(AllUsersGroups, result) is not None # nosec - return result + assert parse_obj_as(MyGroupsGet, my_group) is not None # nosec + return envelope_json_response(my_group) + + +# +# Organization groups +# class _GroupPathParams(BaseModel): @@ -122,13 +125,13 @@ class Config: @permission_required("groups.read") @_handle_groups_exceptions async def get_group(request: web.Request): - """Get one group details""" + """Get an organization group""" req_ctx = _GroupsRequestContext.parse_obj(request) path_params = parse_request_path_parameters_as(_GroupPathParams, request) group = await api.get_user_group(request.app, req_ctx.user_id, path_params.gid) assert parse_obj_as(GroupGet, group) is not None # nosec - return group + return envelope_json_response(group) @routes.post(f"/{API_VTAG}/groups", name="create_group") @@ -136,7 +139,7 @@ async def get_group(request: web.Request): @permission_required("groups.*") @_handle_groups_exceptions async def create_group(request: web.Request): - """Creates organization groups""" + """Creates an organization group""" req_ctx = _GroupsRequestContext.parse_obj(request) create = await parse_request_body_as(GroupCreate, request) new_group = create.dict(exclude_unset=True) @@ -178,6 +181,11 @@ async def delete_group(request: web.Request): return web.json_response(status=status.HTTP_204_NO_CONTENT) +# +# Users in organization groups +# + + @routes.get(f"/{API_VTAG}/groups/{{gid}}/users", name="get_group_users") @login_required @permission_required("groups.*") @@ -208,7 +216,7 @@ async def add_group_user(request: web.Request): assert "uid" in new_user_in_group or "email" in new_user_in_group # nosec - new_user_id = new_user_in_group["uid"] if "uid" in new_user_in_group else None + new_user_id = new_user_in_group.get("uid", None) new_user_email = ( parse_obj_as(LowerCaseEmailStr, new_user_in_group["email"]) if "email" in new_user_in_group diff --git a/services/web/server/src/simcore_service_webserver/users/schemas.py b/services/web/server/src/simcore_service_webserver/users/schemas.py index 53c9ee9b756..ef8973b3abf 100644 --- a/services/web/server/src/simcore_service_webserver/users/schemas.py +++ b/services/web/server/src/simcore_service_webserver/users/schemas.py @@ -3,7 +3,7 @@ from uuid import UUID from models_library.api_schemas_webserver._base import OutputSchema -from models_library.api_schemas_webserver.groups import AllUsersGroups +from models_library.api_schemas_webserver.groups import MyGroupsGet from models_library.api_schemas_webserver.users_preferences import AggregatedPreferences from models_library.emails import LowerCaseEmailStr from models_library.users import FirstNameStr, LastNameStr, UserID @@ -65,7 +65,7 @@ class ProfileGet(BaseModel): last_name: LastNameStr | None = None login: LowerCaseEmailStr role: Literal["ANONYMOUS", "GUEST", "USER", "TESTER", "PRODUCT_OWNER", "ADMIN"] - groups: AllUsersGroups | None = None + groups: MyGroupsGet | None = None gravatar_id: str | None = None expiration_date: date | None = Field( default=None, From d242e63ffa7adb31d0edf1659643d05efd1a620c Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 30 Oct 2024 11:57:04 +0100 Subject: [PATCH 10/26] GroupUserAdd body --- api/specs/web-server/_groups.py | 5 ++-- .../api_schemas_webserver/groups.py | 26 ++++++++++++++++++- .../api/v0/openapi.yaml | 6 ++--- .../groups/_handlers.py | 23 +++++----------- 4 files changed, 38 insertions(+), 22 deletions(-) diff --git a/api/specs/web-server/_groups.py b/api/specs/web-server/_groups.py index 67f307f6bf6..c29577b7f41 100644 --- a/api/specs/web-server/_groups.py +++ b/api/specs/web-server/_groups.py @@ -18,6 +18,7 @@ from models_library.users import GroupID from simcore_service_webserver._meta import API_VTAG from simcore_service_webserver.groups._handlers import ( + GroupUserAdd, GroupUserUpdate, _ClassifiersQuery, _GroupPathParams, @@ -91,7 +92,7 @@ async def delete_group(_p: Annotated[_GroupPathParams, Depends()]): "/groups/{gid}/users", response_model=Envelope[list[GroupUserGet]], ) -async def get_group_users(_p: Annotated[_GroupPathParams, Depends()]): +async def get_all_group_users(_p: Annotated[_GroupPathParams, Depends()]): """ Gets users in organization groups """ @@ -103,7 +104,7 @@ async def get_group_users(_p: Annotated[_GroupPathParams, Depends()]): ) async def add_group_user( _p: Annotated[_GroupPathParams, Depends()], - _new: GroupUserGet, + _b: GroupUserAdd, ): """ Adds a user to an organization group diff --git a/packages/models-library/src/models_library/api_schemas_webserver/groups.py b/packages/models-library/src/models_library/api_schemas_webserver/groups.py index ee26d2d5998..3852d67afb8 100644 --- a/packages/models-library/src/models_library/api_schemas_webserver/groups.py +++ b/packages/models-library/src/models_library/api_schemas_webserver/groups.py @@ -1,7 +1,16 @@ from contextlib import suppress from typing import Any, ClassVar -from pydantic import AnyUrl, BaseModel, Field, ValidationError, parse_obj_as, validator +from pydantic import ( + AnyUrl, + BaseModel, + Field, + ValidationError, + parse_obj_as, + root_validator, + validator, +) +from simcore_service_webserver.db.base_repository import UserID from ..emails import LowerCaseEmailStr from ._base import InputSchema, OutputSchema @@ -173,6 +182,21 @@ class Config: } +class GroupUserAdd(InputSchema): + uid: UserID | None = None + email: LowerCaseEmailStr | None = None + + @root_validator + @classmethod + def _check_uid_or_email(cls, values): + uid, email = values.get("uid"), values.get("email") + # Ensure exactly one of uid or email is set + if bool(uid is not None) ^ bool(email is not None): + msg = f"Either 'uid' or 'email' must be set, but not both. Got {uid=} and {email=}" + raise ValueError(msg) + return values + + class GroupUserUpdate(InputSchema): access_rights: GroupAccessRights | None = None 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 6ff19b255a8..7e93e7bea2d 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 @@ -556,9 +556,9 @@ paths: get: tags: - groups - summary: Get Group Users + summary: Get All Group Users description: Gets users in organization groups - operationId: get_group_users + operationId: get_all_group_users parameters: - required: true schema: @@ -4328,7 +4328,7 @@ paths: '403': description: ProjectInvalidRightsError '404': - description: UserDefaultWalletNotFoundError, ProjectNotFoundError + description: ProjectNotFoundError, UserDefaultWalletNotFoundError '409': description: ProjectTooManyProjectOpenedError '422': diff --git a/services/web/server/src/simcore_service_webserver/groups/_handlers.py b/services/web/server/src/simcore_service_webserver/groups/_handlers.py index ceadd0e4974..03a786192e6 100644 --- a/services/web/server/src/simcore_service_webserver/groups/_handlers.py +++ b/services/web/server/src/simcore_service_webserver/groups/_handlers.py @@ -8,11 +8,11 @@ GroupCreate, GroupGet, GroupUpdate, + GroupUserAdd, GroupUserGet, GroupUserUpdate, MyGroupsGet, ) -from models_library.emails import LowerCaseEmailStr from models_library.users import GroupID, UserID from pydantic import BaseModel, Extra, Field, parse_obj_as from servicelib.aiohttp import status @@ -182,15 +182,15 @@ async def delete_group(request: web.Request): # -# Users in organization groups +# Users in organization groups (i.e. members of an organization) # -@routes.get(f"/{API_VTAG}/groups/{{gid}}/users", name="get_group_users") +@routes.get(f"/{API_VTAG}/groups/{{gid}}/users", name="get_all_group_users") @login_required @permission_required("groups.*") @_handle_groups_exceptions -async def get_group_users(request: web.Request): +async def get_all_group_users(request: web.Request): """Gets users in organization groups""" req_ctx = _GroupsRequestContext.parse_obj(request) path_params = parse_request_path_parameters_as(_GroupPathParams, request) @@ -212,23 +212,14 @@ async def add_group_user(request: web.Request): """ req_ctx = _GroupsRequestContext.parse_obj(request) path_params = parse_request_path_parameters_as(_GroupPathParams, request) - new_user_in_group = await request.json() - - assert "uid" in new_user_in_group or "email" in new_user_in_group # nosec - - new_user_id = new_user_in_group.get("uid", None) - new_user_email = ( - parse_obj_as(LowerCaseEmailStr, new_user_in_group["email"]) - if "email" in new_user_in_group - else None - ) + added: GroupUserAdd = await parse_request_body_as(GroupUserAdd, request) await api.add_user_in_group( request.app, req_ctx.user_id, path_params.gid, - new_user_id=new_user_id, - new_user_email=new_user_email, + new_user_id=added.uid, + new_user_email=added.email, ) return web.json_response(status=status.HTTP_204_NO_CONTENT) From c63de06b0d5ffe57388283eb8f9001904cd2a73b Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 30 Oct 2024 11:57:22 +0100 Subject: [PATCH 11/26] GroupUserAdd body --- .../api/v0/openapi.yaml | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) 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 7e93e7bea2d..9a62467f43c 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 @@ -594,7 +594,7 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/GroupUserGet' + $ref: '#/components/schemas/GroupUserAdd' required: true responses: '204': @@ -4328,7 +4328,7 @@ paths: '403': description: ProjectInvalidRightsError '404': - description: ProjectNotFoundError, UserDefaultWalletNotFoundError + description: UserDefaultWalletNotFoundError, ProjectNotFoundError '409': description: ProjectTooManyProjectOpenedError '422': @@ -8697,6 +8697,19 @@ components: minLength: 1 type: string format: uri + GroupUserAdd: + title: GroupUserAdd + type: object + properties: + uid: + title: Uid + exclusiveMinimum: true + type: integer + minimum: 0 + email: + title: Email + type: string + format: email GroupUserGet: title: GroupUserGet required: From 76ca5d11de72c799ab1e18683db5dda3554f57be Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 30 Oct 2024 12:04:04 +0100 Subject: [PATCH 12/26] cleanup --- api/specs/web-server/_groups.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/api/specs/web-server/_groups.py b/api/specs/web-server/_groups.py index c29577b7f41..1f8d7e15f56 100644 --- a/api/specs/web-server/_groups.py +++ b/api/specs/web-server/_groups.py @@ -15,7 +15,6 @@ MyGroupsGet, ) from models_library.generics import Envelope -from models_library.users import GroupID from simcore_service_webserver._meta import API_VTAG from simcore_service_webserver.groups._handlers import ( GroupUserAdd, @@ -158,8 +157,8 @@ async def delete_group_user( response_model=Envelope[dict[str, Any]], ) async def get_group_classifiers( - gid: GroupID, - _query: Annotated[_ClassifiersQuery, Depends()], + _p: Annotated[_GroupPathParams, Depends()], + _q: Annotated[_ClassifiersQuery, Depends()], ): ... From e74ba5545985e4d3354d7818b5e429b625fdc5c6 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 30 Oct 2024 12:06:14 +0100 Subject: [PATCH 13/26] required mygroupget --- .../src/models_library/api_schemas_webserver/groups.py | 10 +++------- .../src/simcore_service_webserver/api/v0/openapi.yaml | 6 +++++- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/models-library/src/models_library/api_schemas_webserver/groups.py b/packages/models-library/src/models_library/api_schemas_webserver/groups.py index 3852d67afb8..fcafa7fbdad 100644 --- a/packages/models-library/src/models_library/api_schemas_webserver/groups.py +++ b/packages/models-library/src/models_library/api_schemas_webserver/groups.py @@ -15,10 +15,6 @@ from ..emails import LowerCaseEmailStr from ._base import InputSchema, OutputSchema -# -# GROUPS MODELS defined in OPENAPI specs -# - class GroupAccessRights(BaseModel): """ @@ -109,10 +105,10 @@ class GroupUpdate(InputSchema): class MyGroupsGet(OutputSchema): - me: GroupGet | None = None + me: GroupGet organizations: list[GroupGet] | None = None - all: GroupGet | None = None - product: GroupGet | None = None + all: GroupGet + product: GroupGet class Config: schema_extra: ClassVar[dict[str, Any]] = { 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 9a62467f43c..e78b8714571 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 @@ -4328,7 +4328,7 @@ paths: '403': description: ProjectInvalidRightsError '404': - description: UserDefaultWalletNotFoundError, ProjectNotFoundError + description: ProjectNotFoundError, UserDefaultWalletNotFoundError '409': description: ProjectTooManyProjectOpenedError '422': @@ -9059,6 +9059,10 @@ components: additionalProperties: false MyGroupsGet: title: MyGroupsGet + required: + - me + - all + - product type: object properties: me: From 514d720d91b5c4bd32ca522adcb89a0d3a6877cb Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 30 Oct 2024 12:44:41 +0100 Subject: [PATCH 14/26] setup tests --- .../api_schemas_webserver/groups.py | 2 +- .../test_api_schemas_webserver_groups.py | 27 +++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) create mode 100644 packages/models-library/tests/test_api_schemas_webserver_groups.py diff --git a/packages/models-library/src/models_library/api_schemas_webserver/groups.py b/packages/models-library/src/models_library/api_schemas_webserver/groups.py index fcafa7fbdad..f83bc538608 100644 --- a/packages/models-library/src/models_library/api_schemas_webserver/groups.py +++ b/packages/models-library/src/models_library/api_schemas_webserver/groups.py @@ -187,7 +187,7 @@ class GroupUserAdd(InputSchema): def _check_uid_or_email(cls, values): uid, email = values.get("uid"), values.get("email") # Ensure exactly one of uid or email is set - if bool(uid is not None) ^ bool(email is not None): + if not (bool(uid is not None) ^ bool(email is not None)): msg = f"Either 'uid' or 'email' must be set, but not both. Got {uid=} and {email=}" raise ValueError(msg) return values diff --git a/packages/models-library/tests/test_api_schemas_webserver_groups.py b/packages/models-library/tests/test_api_schemas_webserver_groups.py new file mode 100644 index 00000000000..02ec613e7b4 --- /dev/null +++ b/packages/models-library/tests/test_api_schemas_webserver_groups.py @@ -0,0 +1,27 @@ +from typing import Any + +import pytest +from models_library.api_schemas_webserver.groups import GroupUserAdd +from pydantic import ValidationError + +unset = object() + + +@pytest.mark.parametrize("uid", [1, None, unset]) +@pytest.mark.parametrize("email", ["user@foo.com", None, unset]) +def test_uid_or_email_are_set(uid: Any, email: Any): + kwargs = {} + if uid != unset: + kwargs["uid"] = uid + if email != unset: + kwargs["email"] = email + + none_are_set = kwargs.get("uid") is None and kwargs.get("email") is None + both_are_set = kwargs.get("uid") is not None and kwargs.get("email") is not None + + if none_are_set or both_are_set: + with pytest.raises(ValidationError, match="not both"): + GroupUserAdd(**kwargs) + else: + got = GroupUserAdd(**kwargs) + assert bool(got.email) ^ bool(got.uid) From fd15d7a8acfbd9250467da085e7896f80d70ce43 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 30 Oct 2024 13:15:16 +0100 Subject: [PATCH 15/26] generalized --- .../api_schemas_webserver/groups.py | 24 ++++++++------ .../models_library/utils/common_validators.py | 32 +++++++++++++++++++ 2 files changed, 46 insertions(+), 10 deletions(-) diff --git a/packages/models-library/src/models_library/api_schemas_webserver/groups.py b/packages/models-library/src/models_library/api_schemas_webserver/groups.py index f83bc538608..13bfe711dfb 100644 --- a/packages/models-library/src/models_library/api_schemas_webserver/groups.py +++ b/packages/models-library/src/models_library/api_schemas_webserver/groups.py @@ -10,9 +10,10 @@ root_validator, validator, ) -from simcore_service_webserver.db.base_repository import UserID from ..emails import LowerCaseEmailStr +from ..users import UserID +from ..utils.common_validators import create__only_one_is_set__root_validator from ._base import InputSchema, OutputSchema @@ -182,15 +183,18 @@ class GroupUserAdd(InputSchema): uid: UserID | None = None email: LowerCaseEmailStr | None = None - @root_validator - @classmethod - def _check_uid_or_email(cls, values): - uid, email = values.get("uid"), values.get("email") - # Ensure exactly one of uid or email is set - if not (bool(uid is not None) ^ bool(email is not None)): - msg = f"Either 'uid' or 'email' must be set, but not both. Got {uid=} and {email=}" - raise ValueError(msg) - return values + _check_uid_or_email = root_validator(allow_reuse=True)( + create__only_one_is_set__root_validator(["uid", "email"]) + ) + # @root_validator + # @classmethod + # def _check_uid_or_email(cls, values): + # uid, email = values.get("uid"), values.get("email") + # # Ensure exactly one of uid or email is set + # if not (bool(uid is not None) ^ bool(email is not None)): + # msg = f"Either 'uid' or 'email' must be set, but not both. Got {uid=} and {email=}" + # raise ValueError(msg) + # return values class GroupUserUpdate(InputSchema): diff --git a/packages/models-library/src/models_library/utils/common_validators.py b/packages/models-library/src/models_library/utils/common_validators.py index c60586dfa92..baf09445ffe 100644 --- a/packages/models-library/src/models_library/utils/common_validators.py +++ b/packages/models-library/src/models_library/utils/common_validators.py @@ -16,6 +16,8 @@ class MyModel(BaseModel): """ import enum +import functools +import operator from typing import Any @@ -69,3 +71,33 @@ def null_or_none_str_to_none_validator(value: Any): if isinstance(value, str) and value.lower() in ("null", "none"): return None return value + + +def create__only_one_is_set__root_validator(alternative_field_names: list[str]): + """Ensure exactly one and only one of the alternatives is set + + This is useful when you want to give the client alternative + ways to set the same thing e.g. set the user by email or id or username + and each of those has a different field + + NOTE: Alternatevely, the previous example can also be solved using a + single field as `user: Email | UserID | UserName` + + SEE test_uid_or_email_are_set.py for more details + """ + + def _validator(cls, values): + assert set(alternative_field_names).issubset(cls.__fields__) # nosec + + got = { + field_name: values.get(field_name) for field_name in alternative_field_names + } + + if not functools.reduce(operator.xor, (v is not None for v in got.values())): + msg = ( + f"Either { 'or'.join(got.keys()) } must be set, but not both. Got {got}" + ) + raise ValueError(msg) + return values + + return _validator From 050e75fc82f8f5849b3ee1586d297eefcfc06866 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 30 Oct 2024 13:20:14 +0100 Subject: [PATCH 16/26] generalized --- .../api_schemas_webserver/groups.py | 18 +++++++----------- .../models_library/utils/common_validators.py | 2 +- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/packages/models-library/src/models_library/api_schemas_webserver/groups.py b/packages/models-library/src/models_library/api_schemas_webserver/groups.py index 13bfe711dfb..965a597cc80 100644 --- a/packages/models-library/src/models_library/api_schemas_webserver/groups.py +++ b/packages/models-library/src/models_library/api_schemas_webserver/groups.py @@ -13,7 +13,7 @@ from ..emails import LowerCaseEmailStr from ..users import UserID -from ..utils.common_validators import create__only_one_is_set__root_validator +from ..utils.common_validators import create__check_only_one_is_set__root_validator from ._base import InputSchema, OutputSchema @@ -184,17 +184,13 @@ class GroupUserAdd(InputSchema): email: LowerCaseEmailStr | None = None _check_uid_or_email = root_validator(allow_reuse=True)( - create__only_one_is_set__root_validator(["uid", "email"]) + create__check_only_one_is_set__root_validator(["uid", "email"]) ) - # @root_validator - # @classmethod - # def _check_uid_or_email(cls, values): - # uid, email = values.get("uid"), values.get("email") - # # Ensure exactly one of uid or email is set - # if not (bool(uid is not None) ^ bool(email is not None)): - # msg = f"Either 'uid' or 'email' must be set, but not both. Got {uid=} and {email=}" - # raise ValueError(msg) - # return values + + class Config: + schema_extra: ClassVar[dict[str, Any]] = { + "examples": [{"uid": 42}, {"email": "foo@email.com"}] + } class GroupUserUpdate(InputSchema): diff --git a/packages/models-library/src/models_library/utils/common_validators.py b/packages/models-library/src/models_library/utils/common_validators.py index baf09445ffe..98d7f854125 100644 --- a/packages/models-library/src/models_library/utils/common_validators.py +++ b/packages/models-library/src/models_library/utils/common_validators.py @@ -73,7 +73,7 @@ def null_or_none_str_to_none_validator(value: Any): return value -def create__only_one_is_set__root_validator(alternative_field_names: list[str]): +def create__check_only_one_is_set__root_validator(alternative_field_names: list[str]): """Ensure exactly one and only one of the alternatives is set This is useful when you want to give the client alternative From 750c3b6b1ca89d3d474c654288bb6b79d86bbd2d Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 30 Oct 2024 13:26:37 +0100 Subject: [PATCH 17/26] doc --- .../src/models_library/utils/common_validators.py | 3 +++ .../tests/test_api_schemas_webserver_groups.py | 6 +++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/models-library/src/models_library/utils/common_validators.py b/packages/models-library/src/models_library/utils/common_validators.py index 98d7f854125..f1d754de5dc 100644 --- a/packages/models-library/src/models_library/utils/common_validators.py +++ b/packages/models-library/src/models_library/utils/common_validators.py @@ -76,6 +76,9 @@ def null_or_none_str_to_none_validator(value: Any): def create__check_only_one_is_set__root_validator(alternative_field_names: list[str]): """Ensure exactly one and only one of the alternatives is set + NOTE: a field is considered here `unset` when it is `not None`. When None + is used to indicate something else, please do not use this validator. + This is useful when you want to give the client alternative ways to set the same thing e.g. set the user by email or id or username and each of those has a different field diff --git a/packages/models-library/tests/test_api_schemas_webserver_groups.py b/packages/models-library/tests/test_api_schemas_webserver_groups.py index 02ec613e7b4..c254ef40cf4 100644 --- a/packages/models-library/tests/test_api_schemas_webserver_groups.py +++ b/packages/models-library/tests/test_api_schemas_webserver_groups.py @@ -16,10 +16,10 @@ def test_uid_or_email_are_set(uid: Any, email: Any): if email != unset: kwargs["email"] = email - none_are_set = kwargs.get("uid") is None and kwargs.get("email") is None - both_are_set = kwargs.get("uid") is not None and kwargs.get("email") is not None + none_are_defined = kwargs.get("uid") is None and kwargs.get("email") is None + both_are_defined = kwargs.get("uid") is not None and kwargs.get("email") is not None - if none_are_set or both_are_set: + if none_are_defined or both_are_defined: with pytest.raises(ValidationError, match="not both"): GroupUserAdd(**kwargs) else: From d70145523810e66d33106e068a20b667e962d5cb Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Fri, 1 Nov 2024 15:34:30 +0100 Subject: [PATCH 18/26] makes it optional --- .../src/models_library/api_schemas_webserver/groups.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/models-library/src/models_library/api_schemas_webserver/groups.py b/packages/models-library/src/models_library/api_schemas_webserver/groups.py index 965a597cc80..6d7cdf0d108 100644 --- a/packages/models-library/src/models_library/api_schemas_webserver/groups.py +++ b/packages/models-library/src/models_library/api_schemas_webserver/groups.py @@ -109,7 +109,8 @@ class MyGroupsGet(OutputSchema): me: GroupGet organizations: list[GroupGet] | None = None all: GroupGet - product: GroupGet + # FIXME: in GET /me is different as in GET /groups + product: GroupGet | None = None class Config: schema_extra: ClassVar[dict[str, Any]] = { From f441d1cb33d5b85f6d4cf3a722b815f060eae129 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Fri, 1 Nov 2024 15:38:51 +0100 Subject: [PATCH 19/26] updates fakers, ruff --- .../tests/test_archiving_utils.py | 9 ++++----- .../service-library/tests/test_async_utils.py | 20 ++++++++++--------- .../tests/unit/test_api_rest_containers.py | 5 ++--- services/storage/tests/unit/test_utils.py | 18 +++++++++++------ 4 files changed, 29 insertions(+), 23 deletions(-) diff --git a/packages/service-library/tests/test_archiving_utils.py b/packages/service-library/tests/test_archiving_utils.py index 3996be43ca5..3a94c9fd22c 100644 --- a/packages/service-library/tests/test_archiving_utils.py +++ b/packages/service-library/tests/test_archiving_utils.py @@ -7,7 +7,6 @@ import hashlib import itertools import os -import random import secrets import string import tempfile @@ -35,14 +34,14 @@ def _print_tree(path: Path, level=0): @pytest.fixture -def dir_with_random_content() -> Iterable[Path]: +def dir_with_random_content(faker: Faker) -> Iterable[Path]: def random_string(length: int) -> str: return "".join(secrets.choice(string.ascii_letters) for i in range(length)) def make_files_in_dir(dir_path: Path, file_count: int) -> None: for _ in range(file_count): (dir_path / f"{random_string(8)}.bin").write_bytes( - os.urandom(random.randint(1, 10)) + os.urandom(faker.random_int(1, 10)) ) def ensure_dir(path_to_ensure: Path) -> Path: @@ -53,13 +52,13 @@ def make_subdirectory_with_content(subdir_name: Path, max_file_count: int) -> No subdir_name = ensure_dir(subdir_name) make_files_in_dir( dir_path=subdir_name, - file_count=random.randint(1, max_file_count), + file_count=faker.random_int(1, max_file_count), ) def make_subdirectories_with_content( subdir_name: Path, max_subdirectories_count: int, max_file_count: int ) -> None: - subdirectories_count = random.randint(1, max_subdirectories_count) + subdirectories_count = faker.random_int(1, max_subdirectories_count) for _ in range(subdirectories_count): make_subdirectory_with_content( subdir_name=subdir_name / f"{random_string(4)}", diff --git a/packages/service-library/tests/test_async_utils.py b/packages/service-library/tests/test_async_utils.py index cf2d051fe95..902cbcc9b82 100644 --- a/packages/service-library/tests/test_async_utils.py +++ b/packages/service-library/tests/test_async_utils.py @@ -8,7 +8,7 @@ from collections import deque from dataclasses import dataclass from time import time -from typing import Any, Optional +from typing import Any import pytest from faker import Faker @@ -59,24 +59,25 @@ def _compensate_for_slow_systems(number: float) -> float: async def test_context_aware_dispatch( - sleep_duration: float, - ensure_run_in_sequence_context_is_empty: None, + sleep_duration: float, ensure_run_in_sequence_context_is_empty: None, faker: Faker ) -> None: @run_sequentially_in_context(target_args=["c1", "c2", "c3"]) async def orderly(c1: Any, c2: Any, c3: Any, control: Any) -> None: _ = (c1, c2, c3) await asyncio.sleep(sleep_duration) - context = dict(c1=c1, c2=c2, c3=c3) + context = {"c1": c1, "c2": c2, "c3": c3} await locked_stores[make_key_from_context(context)].push(control) def make_key_from_context(context: dict) -> str: return ".".join([f"{k}:{v}" for k, v in context.items()]) def make_context(): - return dict( - c1=random.randint(0, 10), c2=random.randint(0, 10), c3=random.randint(0, 10) - ) + return { + "c1": faker.random_int(0, 10), + "c2": faker.random_int(0, 10), + "c3": faker.random_int(0, 10), + } contexts = [make_context() for _ in range(10)] @@ -116,7 +117,8 @@ class DidFailException(Exception): @run_sequentially_in_context(target_args=["will_fail"]) async def sometimes_failing(will_fail: bool) -> bool: if will_fail: - raise DidFailException("I was instructed to fail") + msg = "I was instructed to fail" + raise DidFailException(msg) return True for x in range(100): @@ -201,7 +203,7 @@ class ObjectWithPropos: @run_sequentially_in_context(target_args=["object_with_props.attr1"]) async def test_attribute( - object_with_props: ObjectWithPropos, other_attr: Optional[int] = None + object_with_props: ObjectWithPropos, other_attr: int | None = None ) -> str: return object_with_props.attr1 diff --git a/services/dynamic-sidecar/tests/unit/test_api_rest_containers.py b/services/dynamic-sidecar/tests/unit/test_api_rest_containers.py index f16b883de15..83e7a803f10 100644 --- a/services/dynamic-sidecar/tests/unit/test_api_rest_containers.py +++ b/services/dynamic-sidecar/tests/unit/test_api_rest_containers.py @@ -5,7 +5,6 @@ import asyncio import json -import random from collections.abc import AsyncIterable from inspect import signature from pathlib import Path @@ -303,9 +302,9 @@ async def attachable_networks_and_ids(faker: Faker) -> AsyncIterable[dict[str, s @pytest.fixture -def mock_aiodocker_containers_get(mocker: MockerFixture) -> int: +def mock_aiodocker_containers_get(mocker: MockerFixture, faker: Faker) -> int: """raises a DockerError with a random HTTP status which is also returned""" - mock_status_code = random.randint(1, 999) # noqa: S311 + mock_status_code = faker.random_int(1, 999) async def mock_get(*args: str, **kwargs: Any) -> None: raise aiodocker.exceptions.DockerError( diff --git a/services/storage/tests/unit/test_utils.py b/services/storage/tests/unit/test_utils.py index 1b71a1c29d5..03dc7e2e630 100644 --- a/services/storage/tests/unit/test_utils.py +++ b/services/storage/tests/unit/test_utils.py @@ -6,9 +6,8 @@ import datetime -import random +from collections.abc import Callable from pathlib import Path -from typing import Callable from uuid import uuid4 import pytest @@ -17,6 +16,7 @@ from models_library.projects import ProjectID from models_library.projects_nodes_io import NodeID, SimcoreS3FileID from pydantic import ByteSize, HttpUrl, parse_obj_as +from pytest_simcore.helpers.faker_factories import DEFAULT_FAKER from simcore_service_storage.constants import S3_UNDEFINED_OR_EXTERNAL_MULTIPART_ID from simcore_service_storage.models import ETag, FileMetaData, S3BucketName, UploadID from simcore_service_storage.simcore_s3_dsm import SimcoreS3DataManager @@ -46,24 +46,30 @@ async def test_download_files(tmp_path: Path, httpbin_base_url: HttpUrl): [ (-1, None, None, None, False), (0, None, None, None, False), - (random.randint(1, 1000000), None, None, None, False), + (DEFAULT_FAKER.random_int(1, 1000000), None, None, None, False), (-1, "some_valid_entity_tag", None, None, False), (0, "some_valid_entity_tag", None, None, False), ( - random.randint(1, 1000000), + DEFAULT_FAKER.random_int(1, 1000000), "some_valid_entity_tag", "som_upload_id", None, False, ), ( - random.randint(1, 1000000), + DEFAULT_FAKER.random_int(1, 1000000), "some_valid_entity_tag", None, datetime.datetime.utcnow(), False, ), - (random.randint(1, 1000000), "some_valid_entity_tag", None, None, True), + ( + DEFAULT_FAKER.random_int(1, 1000000), + "some_valid_entity_tag", + None, + None, + True, + ), ], ) def test_file_entry_valid( From 7677692c045cd6f949116f1f1eec2698df1e70c0 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Fri, 1 Nov 2024 15:41:45 +0100 Subject: [PATCH 20/26] fixing tests --- .../src/models_library/api_schemas_webserver/groups.py | 4 +++- .../server/src/simcore_service_webserver/groups/_db.py | 8 ++++++-- .../src/simcore_service_webserver/groups/_handlers.py | 9 +++++---- .../server/src/simcore_service_webserver/groups/api.py | 4 ++-- .../web/server/tests/unit/with_dbs/01/test_groups.py | 10 +++++----- 5 files changed, 21 insertions(+), 14 deletions(-) diff --git a/packages/models-library/src/models_library/api_schemas_webserver/groups.py b/packages/models-library/src/models_library/api_schemas_webserver/groups.py index 6d7cdf0d108..73661fd9bff 100644 --- a/packages/models-library/src/models_library/api_schemas_webserver/groups.py +++ b/packages/models-library/src/models_library/api_schemas_webserver/groups.py @@ -195,7 +195,9 @@ class Config: class GroupUserUpdate(InputSchema): - access_rights: GroupAccessRights | None = None + # NOTE: since it is a single item, it is required. Cannot + # update for the moment partial attributes e.g. {read: False} + access_rights: GroupAccessRights class Config: schema_extra: ClassVar[dict[str, Any]] = { diff --git a/services/web/server/src/simcore_service_webserver/groups/_db.py b/services/web/server/src/simcore_service_webserver/groups/_db.py index 38bbb4e7d7c..d2c6f4d2db5 100644 --- a/services/web/server/src/simcore_service_webserver/groups/_db.py +++ b/services/web/server/src/simcore_service_webserver/groups/_db.py @@ -358,8 +358,12 @@ async def update_user_in_group( user_id: UserID, gid: GroupID, the_user_id_in_group: int, - new_values_for_user_in_group: dict, + access_rights: dict, ) -> dict[str, str]: + if not access_rights: + msg = f"Cannot update empty {access_rights}" + raise ValueError(msg) + # first check if the group exists group: RowProxy = await _get_user_group(conn, user_id, gid) check_group_permissions(group, user_id, gid, "write") @@ -368,7 +372,7 @@ async def update_user_in_group( conn, gid, the_user_id_in_group ) # modify the user access rights - new_db_values = {"access_rights": new_values_for_user_in_group["accessRights"]} + new_db_values = {"access_rights": access_rights} await conn.execute( # pylint: disable=no-value-for-parameter user_to_groups.update() diff --git a/services/web/server/src/simcore_service_webserver/groups/_handlers.py b/services/web/server/src/simcore_service_webserver/groups/_handlers.py index 03a786192e6..6ad47354ccd 100644 --- a/services/web/server/src/simcore_service_webserver/groups/_handlers.py +++ b/services/web/server/src/simcore_service_webserver/groups/_handlers.py @@ -98,6 +98,7 @@ async def list_groups(request: web.Request): if product.group_id: with suppress(GroupNotFoundError): + # Product is optional my_group["product"] = await api.get_product_group_for_user( app=request.app, user_id=req_ctx.user_id, @@ -260,10 +261,10 @@ async def update_group_user(request: web.Request): user = await api.update_user_in_group( request.app, - req_ctx.user_id, - path_params.gid, - path_params.uid, - update.dict(exclude_unset=True), + user_id=req_ctx.user_id, + gid=path_params.gid, + the_user_id_in_group=path_params.uid, + access_rights=update.access_rights.dict(), ) assert parse_obj_as(GroupUserGet, user) is not None # nosec return envelope_json_response(user) diff --git a/services/web/server/src/simcore_service_webserver/groups/api.py b/services/web/server/src/simcore_service_webserver/groups/api.py index e0a837f8399..168eb539bc2 100644 --- a/services/web/server/src/simcore_service_webserver/groups/api.py +++ b/services/web/server/src/simcore_service_webserver/groups/api.py @@ -173,7 +173,7 @@ async def update_user_in_group( user_id: UserID, gid: GroupID, the_user_id_in_group: int, - new_values_for_user_in_group: dict, + access_rights: dict, ) -> dict[str, str]: async with get_database_engine(app).acquire() as conn: return await _db.update_user_in_group( @@ -181,7 +181,7 @@ async def update_user_in_group( user_id=user_id, gid=gid, the_user_id_in_group=the_user_id_in_group, - new_values_for_user_in_group=new_values_for_user_in_group, + access_rights=access_rights, ) diff --git a/services/web/server/tests/unit/with_dbs/01/test_groups.py b/services/web/server/tests/unit/with_dbs/01/test_groups.py index f6e41225ff7..f616dee6110 100644 --- a/services/web/server/tests/unit/with_dbs/01/test_groups.py +++ b/services/web/server/tests/unit/with_dbs/01/test_groups.py @@ -4,7 +4,6 @@ # pylint: disable=unused-argument # pylint: disable=unused-variable -import random from collections.abc import AsyncIterator, Callable from contextlib import AsyncExitStack from copy import deepcopy @@ -160,7 +159,7 @@ async def test_list_groups( ) response = await client.patch( f"{url}", - json={"access_rights": {"read": True, "write": True, "delete": True}}, + json={"accessRights": {"read": True, "write": True, "delete": True}}, ) await assert_status(response, status.HTTP_403_FORBIDDEN) @@ -285,6 +284,7 @@ async def test_add_remove_users_from_group( logged_user: UserInfoDict, user_role: UserRole, expected: ExpectedResponse, + faker: Faker, ): assert client.app new_group = { @@ -295,7 +295,7 @@ async def test_add_remove_users_from_group( } # check that our group does not exist - url = client.app.router["get_group_users"].url_for(gid=new_group["gid"]) + url = client.app.router["get_all_group_users"].url_for(gid=new_group["gid"]) assert f"{url}" == f"/{API_VTAG}/groups/{new_group['gid']}/users" resp = await client.get(f"{url}") data, error = await assert_status(resp, expected.not_found) @@ -323,7 +323,7 @@ async def test_add_remove_users_from_group( assert assigned_group["accessRights"] == _DEFAULT_GROUP_OWNER_ACCESS_RIGHTS # check that our user is in the group of users - get_group_users_url = client.app.router["get_group_users"].url_for( + get_group_users_url = client.app.router["get_all_group_users"].url_for( gid=f"{assigned_group['gid']}" ) assert ( @@ -345,7 +345,7 @@ async def test_add_remove_users_from_group( assert ( f"{add_group_user_url}" == f"/{API_VTAG}/groups/{assigned_group['gid']}/users" ) - num_new_users = random.randint(1, 10) + num_new_users = faker.random_int(1, 10) created_users_list = [] async with AsyncExitStack() as users_stack: From 6cca53131a504f5a10c637e06f2cfd4dcffa0842 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Fri, 1 Nov 2024 19:15:18 +0100 Subject: [PATCH 21/26] doc --- .../src/models_library/api_schemas_webserver/groups.py | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/models-library/src/models_library/api_schemas_webserver/groups.py b/packages/models-library/src/models_library/api_schemas_webserver/groups.py index 73661fd9bff..969f42b8486 100644 --- a/packages/models-library/src/models_library/api_schemas_webserver/groups.py +++ b/packages/models-library/src/models_library/api_schemas_webserver/groups.py @@ -109,7 +109,6 @@ class MyGroupsGet(OutputSchema): me: GroupGet organizations: list[GroupGet] | None = None all: GroupGet - # FIXME: in GET /me is different as in GET /groups product: GroupGet | None = None class Config: From 804aaefd30268a62f468bf17dca13f093912cb6f Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Fri, 1 Nov 2024 19:18:14 +0100 Subject: [PATCH 22/26] doc --- .../src/models_library/api_schemas_webserver/groups.py | 4 ++++ .../src/simcore_service_webserver/api/v0/openapi.yaml | 6 ++++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/models-library/src/models_library/api_schemas_webserver/groups.py b/packages/models-library/src/models_library/api_schemas_webserver/groups.py index 969f42b8486..7c8948bdc63 100644 --- a/packages/models-library/src/models_library/api_schemas_webserver/groups.py +++ b/packages/models-library/src/models_library/api_schemas_webserver/groups.py @@ -180,6 +180,10 @@ class Config: class GroupUserAdd(InputSchema): + """ + To identify the user, you can either use `email` or user ID (`uid`) + """ + uid: UserID | None = None email: LowerCaseEmailStr | None = None 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 e78b8714571..2110ae8900b 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 @@ -4328,7 +4328,7 @@ paths: '403': description: ProjectInvalidRightsError '404': - description: ProjectNotFoundError, UserDefaultWalletNotFoundError + description: UserDefaultWalletNotFoundError, ProjectNotFoundError '409': description: ProjectTooManyProjectOpenedError '422': @@ -8710,6 +8710,7 @@ components: title: Email type: string format: email + description: To identify the user, you can either use `email` or user ID (`uid`) GroupUserGet: title: GroupUserGet required: @@ -8756,6 +8757,8 @@ components: delete: false GroupUserUpdate: title: GroupUserUpdate + required: + - accessRights type: object properties: accessRights: @@ -9062,7 +9065,6 @@ components: required: - me - all - - product type: object properties: me: From a546fcf55a6c45fd01be30321399eb70c2f52452 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Fri, 1 Nov 2024 19:20:29 +0100 Subject: [PATCH 23/26] doc --- .../src/models_library/api_schemas_webserver/groups.py | 2 +- .../server/src/simcore_service_webserver/api/v0/openapi.yaml | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/models-library/src/models_library/api_schemas_webserver/groups.py b/packages/models-library/src/models_library/api_schemas_webserver/groups.py index 7c8948bdc63..55107be55c5 100644 --- a/packages/models-library/src/models_library/api_schemas_webserver/groups.py +++ b/packages/models-library/src/models_library/api_schemas_webserver/groups.py @@ -181,7 +181,7 @@ class Config: class GroupUserAdd(InputSchema): """ - To identify the user, you can either use `email` or user ID (`uid`) + Identify the user with either `email` or `uid` — only one. """ uid: UserID | None = None 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 2110ae8900b..82a18aebb3b 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 @@ -4328,7 +4328,7 @@ paths: '403': description: ProjectInvalidRightsError '404': - description: UserDefaultWalletNotFoundError, ProjectNotFoundError + description: ProjectNotFoundError, UserDefaultWalletNotFoundError '409': description: ProjectTooManyProjectOpenedError '422': @@ -8710,7 +8710,7 @@ components: title: Email type: string format: email - description: To identify the user, you can either use `email` or user ID (`uid`) + description: "Identify the user with either `email` or `uid` \u2014 only one." GroupUserGet: title: GroupUserGet required: From 298161cf28d3ce90df00a7f09efdad0ec36c02e4 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Tue, 5 Nov 2024 14:55:37 +0100 Subject: [PATCH 24/26] email or uid --- .../server/src/simcore_service_webserver/groups/_handlers.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/services/web/server/src/simcore_service_webserver/groups/_handlers.py b/services/web/server/src/simcore_service_webserver/groups/_handlers.py index 6ad47354ccd..13d9acf71b5 100644 --- a/services/web/server/src/simcore_service_webserver/groups/_handlers.py +++ b/services/web/server/src/simcore_service_webserver/groups/_handlers.py @@ -57,7 +57,9 @@ async def wrapper(request: web.Request) -> web.StreamResponse: return await handler(request) except UserNotFoundError as exc: - raise web.HTTPNotFound(reason=f"User {exc.uid} not found") from exc + raise web.HTTPNotFound( + reason=f"User {exc.uid or exc.email} not found" + ) from exc except GroupNotFoundError as exc: raise web.HTTPNotFound(reason=f"Group {exc.gid} not found") from exc From 3612afdf6f147c02822fea977a218210239d7f58 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Tue, 5 Nov 2024 15:18:18 +0100 Subject: [PATCH 25/26] fixes exceptions in groups --- .../simcore_service_webserver/groups/_db.py | 24 ++++++++++++++----- .../groups/_handlers.py | 6 +++++ .../groups/exceptions.py | 16 ++++--------- 3 files changed, 28 insertions(+), 18 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/groups/_db.py b/services/web/server/src/simcore_service_webserver/groups/_db.py index d2c6f4d2db5..e3ad2f824c7 100644 --- a/services/web/server/src/simcore_service_webserver/groups/_db.py +++ b/services/web/server/src/simcore_service_webserver/groups/_db.py @@ -7,6 +7,7 @@ from models_library.groups import GroupAtDB from models_library.users import GroupID, UserID from pydantic import parse_obj_as +from simcore_postgres_database.errors import UniqueViolation from simcore_postgres_database.utils_products import get_or_create_product_group from sqlalchemy import and_, literal_column from sqlalchemy.dialects.postgresql import insert @@ -20,7 +21,11 @@ convert_groups_db_to_schema, convert_groups_schema_to_db, ) -from .exceptions import GroupNotFoundError, UserInGroupNotFoundError +from .exceptions import ( + GroupNotFoundError, + UserAlreadyInGroupError, + UserInGroupNotFoundError, +) _DEFAULT_PRODUCT_GROUP_ACCESS_RIGHTS = AccessRightsDict( read=False, @@ -305,6 +310,7 @@ async def add_new_user_in_group( # first check if the group exists group: RowProxy = await _get_user_group(conn, user_id, gid) check_group_permissions(group, user_id, gid, "write") + # now check the new user exists users_count = await conn.scalar( sa.select(sa.func.count()).where(users.c.id == new_user_id) @@ -317,12 +323,18 @@ async def add_new_user_in_group( user_access_rights = _DEFAULT_GROUP_READ_ACCESS_RIGHTS if access_rights: user_access_rights.update(access_rights) - await conn.execute( - # pylint: disable=no-value-for-parameter - user_to_groups.insert().values( - uid=new_user_id, gid=group.gid, access_rights=user_access_rights + + try: + await conn.execute( + # pylint: disable=no-value-for-parameter + user_to_groups.insert().values( + uid=new_user_id, gid=group.gid, access_rights=user_access_rights + ) ) - ) + except UniqueViolation as exc: + raise UserAlreadyInGroupError( + uid=new_user_id, gid=gid, user_id=user_id, access_rights=access_rights + ) from exc async def _get_user_in_group_permissions( diff --git a/services/web/server/src/simcore_service_webserver/groups/_handlers.py b/services/web/server/src/simcore_service_webserver/groups/_handlers.py index 13d9acf71b5..1fa398e84f2 100644 --- a/services/web/server/src/simcore_service_webserver/groups/_handlers.py +++ b/services/web/server/src/simcore_service_webserver/groups/_handlers.py @@ -38,6 +38,7 @@ from ._classifiers import GroupClassifierRepository, build_rrids_tree_view from .exceptions import ( GroupNotFoundError, + UserAlreadyInGroupError, UserInGroupNotFoundError, UserInsufficientRightsError, ) @@ -67,6 +68,11 @@ async def wrapper(request: web.Request) -> web.StreamResponse: except UserInGroupNotFoundError as exc: raise web.HTTPNotFound(reason=f"User not found in group {exc.gid}") from exc + except UserAlreadyInGroupError as exc: + raise web.HTTPConflict( + reason=f"User is already in group {exc.gid}" + ) from exc + except UserInsufficientRightsError as exc: raise web.HTTPForbidden from exc diff --git a/services/web/server/src/simcore_service_webserver/groups/exceptions.py b/services/web/server/src/simcore_service_webserver/groups/exceptions.py index 7556a8cae85..545c73a7727 100644 --- a/services/web/server/src/simcore_service_webserver/groups/exceptions.py +++ b/services/web/server/src/simcore_service_webserver/groups/exceptions.py @@ -4,19 +4,12 @@ class GroupsError(WebServerBaseError): - msg_template = "{msg}" - - def __init__(self, msg: str | None = None): - super().__init__(msg=msg or "Unexpected error occured in projects subpackage") + msg_template = "Groups plugin errored {msg}" class GroupNotFoundError(GroupsError): msg_template = "Group with id {gid} not found" - def __init__(self, gid, **extras): - super().__init__(**extras) - self.gid = gid - class UserInsufficientRightsError(GroupsError): ... @@ -25,7 +18,6 @@ class UserInsufficientRightsError(GroupsError): class UserInGroupNotFoundError(GroupsError): msg_template = "User id {uid} in Group {gid} not found" - def __init__(self, uid, gid, **extras): - super().__init__(**extras) - self.uid = uid - self.gid = gid + +class UserAlreadyInGroupError(GroupsError): + msg_template = "User `{uid}` is already in Group `{gid}`" From 4f4f7e98b93a515d9ffb9441abdc55e7d895f9f7 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Tue, 5 Nov 2024 17:09:19 +0100 Subject: [PATCH 26/26] fixes mypy issues --- .../src/simcore_service_webserver/groups/_db.py | 6 +++--- .../src/simcore_service_webserver/groups/_handlers.py | 11 ++++++----- .../src/simcore_service_webserver/groups/_utils.py | 5 +++-- .../src/simcore_service_webserver/groups/api.py | 4 ++-- .../simcore_service_webserver/groups/exceptions.py | 6 ++++-- 5 files changed, 18 insertions(+), 14 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/groups/_db.py b/services/web/server/src/simcore_service_webserver/groups/_db.py index e3ad2f824c7..d27d7a8a441 100644 --- a/services/web/server/src/simcore_service_webserver/groups/_db.py +++ b/services/web/server/src/simcore_service_webserver/groups/_db.py @@ -55,7 +55,7 @@ async def _get_user_group( ) group = await result.fetchone() if not group: - raise GroupNotFoundError(gid) + raise GroupNotFoundError(gid=gid) assert isinstance(group, RowProxy) # nosec return group @@ -317,7 +317,7 @@ async def add_new_user_in_group( ) if not users_count: assert new_user_id is not None # nosec - raise UserInGroupNotFoundError(new_user_id, gid) + raise UserInGroupNotFoundError(uid=new_user_id, gid=gid) # add the new user to the group now user_access_rights = _DEFAULT_GROUP_READ_ACCESS_RIGHTS @@ -348,7 +348,7 @@ async def _get_user_in_group_permissions( ) the_user: RowProxy | None = await result.fetchone() if not the_user: - raise UserInGroupNotFoundError(the_user_id_in_group, gid) + raise UserInGroupNotFoundError(uid=the_user_id_in_group, gid=gid) return the_user diff --git a/services/web/server/src/simcore_service_webserver/groups/_handlers.py b/services/web/server/src/simcore_service_webserver/groups/_handlers.py index 1fa398e84f2..32a9c4c1a40 100644 --- a/services/web/server/src/simcore_service_webserver/groups/_handlers.py +++ b/services/web/server/src/simcore_service_webserver/groups/_handlers.py @@ -63,15 +63,16 @@ async def wrapper(request: web.Request) -> web.StreamResponse: ) from exc except GroupNotFoundError as exc: - raise web.HTTPNotFound(reason=f"Group {exc.gid} not found") from exc + gid = getattr(exc, "gid", "") + raise web.HTTPNotFound(reason=f"Group {gid} not found") from exc except UserInGroupNotFoundError as exc: - raise web.HTTPNotFound(reason=f"User not found in group {exc.gid}") from exc + gid = getattr(exc, "gid", "") + raise web.HTTPNotFound(reason=f"User not found in group {gid}") from exc except UserAlreadyInGroupError as exc: - raise web.HTTPConflict( - reason=f"User is already in group {exc.gid}" - ) from exc + gid = getattr(exc, "gid", "") + raise web.HTTPConflict(reason=f"User is already in group {gid}") from exc except UserInsufficientRightsError as exc: raise web.HTTPForbidden from exc diff --git a/services/web/server/src/simcore_service_webserver/groups/_utils.py b/services/web/server/src/simcore_service_webserver/groups/_utils.py index 163a4923437..4f0f3ad759f 100644 --- a/services/web/server/src/simcore_service_webserver/groups/_utils.py +++ b/services/web/server/src/simcore_service_webserver/groups/_utils.py @@ -24,8 +24,9 @@ def check_group_permissions( group: RowProxy, user_id: int, gid: int, permission: str ) -> None: if not group.access_rights[permission]: - msg = f"User {user_id} has insufficient rights for {permission} access to group {gid}" - raise UserInsufficientRightsError(msg) + raise UserInsufficientRightsError( + user_id=user_id, gid=gid, permission=permission + ) def convert_groups_db_to_schema( diff --git a/services/web/server/src/simcore_service_webserver/groups/api.py b/services/web/server/src/simcore_service_webserver/groups/api.py index 168eb539bc2..f2cbeb00094 100644 --- a/services/web/server/src/simcore_service_webserver/groups/api.py +++ b/services/web/server/src/simcore_service_webserver/groups/api.py @@ -139,7 +139,7 @@ async def add_user_in_group( if not new_user_id and not new_user_email: msg = "Invalid method call, missing user id or user email" - raise GroupsError(msg) + raise GroupsError(msg=msg) async with get_database_engine(app).acquire() as conn: if new_user_email: @@ -148,7 +148,7 @@ async def add_user_in_group( if not new_user_id: msg = "Missing new user in arguments" - raise GroupsError(msg) + raise GroupsError(msg=msg) return await _db.add_new_user_in_group( conn, diff --git a/services/web/server/src/simcore_service_webserver/groups/exceptions.py b/services/web/server/src/simcore_service_webserver/groups/exceptions.py index 545c73a7727..f30bf56de30 100644 --- a/services/web/server/src/simcore_service_webserver/groups/exceptions.py +++ b/services/web/server/src/simcore_service_webserver/groups/exceptions.py @@ -4,7 +4,7 @@ class GroupsError(WebServerBaseError): - msg_template = "Groups plugin errored {msg}" + msg_template = "Groups plugin errored: {msg}" class GroupNotFoundError(GroupsError): @@ -12,7 +12,9 @@ class GroupNotFoundError(GroupsError): class UserInsufficientRightsError(GroupsError): - ... + msg = ( + "User {user_id} has insufficient rights for {permission} access to group {gid}" + ) class UserInGroupNotFoundError(GroupsError):