From 3a1793c1c36ed435865262fb20c5665ead3caff3 Mon Sep 17 00:00:00 2001 From: Arash Date: Sat, 18 Nov 2023 04:31:34 +0100 Subject: [PATCH 01/13] pydantic models for the responses and payloads --- lib/galaxy/schema/groups.py | 91 +++++++++++++++++++++++++++++++++++++ 1 file changed, 91 insertions(+) create mode 100644 lib/galaxy/schema/groups.py diff --git a/lib/galaxy/schema/groups.py b/lib/galaxy/schema/groups.py new file mode 100644 index 000000000000..b8da8608f0d0 --- /dev/null +++ b/lib/galaxy/schema/groups.py @@ -0,0 +1,91 @@ +from typing import List + +from pydantic import ( + Field, + Required, +) + +from galaxy.schema.fields import EncodedDatabaseIdField +from galaxy.schema.schema import Model + + +class GroupIndexResponse(Model): + '''Response schema for a group.''' + model_class: str = Field( + Required, + title="model class", + description="model class", + ) + id: EncodedDatabaseIdField = Field( + Required, + title="group ID", + description="Encoded group ID", + ) + url: str = Field( + Required, + title="URL for the group", + description="URL for the group", + ) + name: str = Field( + Required, + title="name of the group", + description="name of the group", + ) + + +class GroupIndexListResponse(Model): + '''Response schema for listing groups.''' + __root__: List[GroupIndexResponse] + + +class GroupShowResponse(Model): + '''Response schema for showing a group.''' + model_class: str = Field( + Required, + title="model class", + description="model class", + ) + id: str = Field( + Required, + title="group ID", + description="Encoded group ID", + ) + url: str = Field( + Required, + title="URL for the group", + description="URL for the group", + ) + name: str = Field( + Required, + title="name of the group", + description="name of the group", + ) + roles_url: str = Field( + Required, + title="URL for the roles of the group", + description="URL for the roles of the group", + ) + users_url: str = Field( + Required, + title="URL for the users of the group", + description="URL for the users of the group", + ) + + +class GroupCreatePayload(Model): + '''Payload schema for creating a group.''' + name: str = Field( + Required, + title="name of the group", + description="name of the group", + ) + user_ids: List[EncodedDatabaseIdField] = Field( + [], + title="user IDs", + description="Encoded user IDs", + ) + role_ids: List[EncodedDatabaseIdField] = Field( + [], + title="role IDs", + description="Encoded role IDs", + ) From efbc55d0c5ab265bf83f8c62b019ab793e57a44a Mon Sep 17 00:00:00 2001 From: Arash Date: Sat, 18 Nov 2023 04:32:58 +0100 Subject: [PATCH 02/13] Add FastAPI routes for groups --- lib/galaxy/managers/groups.py | 23 +++--- lib/galaxy/webapps/galaxy/api/groups.py | 104 +++++++++++++++--------- lib/galaxy/webapps/galaxy/buildapp.py | 1 - lib/galaxy/webapps/galaxy/fast_app.py | 4 + 4 files changed, 78 insertions(+), 54 deletions(-) diff --git a/lib/galaxy/managers/groups.py b/lib/galaxy/managers/groups.py index 374428c979f1..f22a4d4f2fd0 100644 --- a/lib/galaxy/managers/groups.py +++ b/lib/galaxy/managers/groups.py @@ -1,8 +1,4 @@ -from typing import ( - Any, - Dict, - List, -) +from typing import List from sqlalchemy import ( false, @@ -29,6 +25,7 @@ DecodedDatabaseIdField, EncodedDatabaseIdField, ) +from galaxy.schema.groups import GroupCreatePayload from galaxy.structured_app import MinimalManagerApp from galaxy.web import url_for @@ -51,21 +48,21 @@ def index(self, trans: ProvidesAppContext): rval.append(item) return rval - def create(self, trans: ProvidesAppContext, payload: Dict[str, Any]): + def create(self, trans: ProvidesAppContext, payload: GroupCreatePayload): """ Creates a new group. """ sa_session = trans.sa_session - name = payload.get("name", None) + name = payload.name if name is None: raise ObjectAttributeMissingException("Missing required name") self._check_duplicated_group_name(sa_session, name) group = model.Group(name=name) sa_session.add(group) - encoded_user_ids = payload.get("user_ids", []) + encoded_user_ids = getattr(payload, "user_ids", []) users = self._get_users_by_encoded_ids(sa_session, encoded_user_ids) - encoded_role_ids = payload.get("role_ids", []) + encoded_role_ids = getattr(payload, "role_ids", []) roles = self._get_roles_by_encoded_ids(sa_session, encoded_role_ids) trans.app.security_agent.set_entity_group_associations(groups=[group], roles=roles, users=users) with transaction(sa_session): @@ -88,20 +85,20 @@ def show(self, trans: ProvidesAppContext, group_id: int): item["roles_url"] = url_for("group_roles", group_id=encoded_id) return item - def update(self, trans: ProvidesAppContext, group_id: int, payload: Dict[str, Any]): + def update(self, trans: ProvidesAppContext, group_id: int, payload: GroupCreatePayload): """ Modifies a group. """ sa_session = trans.sa_session group = self._get_group(sa_session, group_id) - name = payload.get("name", None) + name = payload.name if name: self._check_duplicated_group_name(sa_session, name) group.name = name sa_session.add(group) - encoded_user_ids = payload.get("user_ids", []) + encoded_user_ids = getattr(payload, "user_ids", []) users = self._get_users_by_encoded_ids(sa_session, encoded_user_ids) - encoded_role_ids = payload.get("role_ids", []) + encoded_role_ids = getattr(payload, "role_ids", []) roles = self._get_roles_by_encoded_ids(sa_session, encoded_role_ids) self._app.security_agent.set_entity_group_associations( groups=[group], roles=roles, users=users, delete_existing_assocs=False diff --git a/lib/galaxy/webapps/galaxy/api/groups.py b/lib/galaxy/webapps/galaxy/api/groups.py index 11fd9df9f77e..c1b63efbab9a 100644 --- a/lib/galaxy/webapps/galaxy/api/groups.py +++ b/lib/galaxy/webapps/galaxy/api/groups.py @@ -2,61 +2,85 @@ API operations on Group objects. """ import logging -from typing import ( - Any, - Dict, + +from fastapi import ( + Body, + Path, ) from galaxy.managers.context import ProvidesAppContext from galaxy.managers.groups import GroupsManager from galaxy.schema.fields import EncodedDatabaseIdField -from galaxy.web import ( - expose_api, - require_admin, +from galaxy.schema.groups import ( + GroupCreatePayload, + GroupIndexListResponse, + GroupShowResponse, ) from galaxy.webapps.galaxy.api import ( - BaseGalaxyAPIController, depends, + DependsOnTrans, + Router, ) log = logging.getLogger(__name__) +router = Router(tags=["groups"]) + -class GroupAPIController(BaseGalaxyAPIController): - manager = depends(GroupsManager) +@router.cbv +class FastAPIGroups: + manager: GroupsManager = depends(GroupsManager) - @expose_api - @require_admin - def index(self, trans: ProvidesAppContext, **kwd): - """ - GET /api/groups - Displays a collection (list) of groups. - """ + @router.get( + "/api/groups", + summary="Displays a collection (list) of groups.", + require_admin=True, + ) + def index( + self, + trans: ProvidesAppContext = DependsOnTrans, + ) -> GroupIndexListResponse: + """GET /api/groups - Displays a collection (list) of groups.""" return self.manager.index(trans) - @expose_api - @require_admin - def create(self, trans: ProvidesAppContext, payload: Dict[str, Any], **kwd): - """ - POST /api/groups - Creates a new group. - """ + @router.post( + "/api/groups", + summary="Creates a new group.", + require_admin=True, + ) + def create( + self, + trans: ProvidesAppContext = DependsOnTrans, + payload: GroupCreatePayload = Body(), + ) -> GroupIndexListResponse: + """POST /api/groups - Creates a new group.""" return self.manager.create(trans, payload) - @expose_api - @require_admin - def show(self, trans: ProvidesAppContext, id: EncodedDatabaseIdField, **kwd): - """ - GET /api/groups/{encoded_group_id} - Displays information about a group. - """ - return self.manager.show(trans, EncodedDatabaseIdField.decode(id)) - - @expose_api - @require_admin - def update(self, trans: ProvidesAppContext, id: EncodedDatabaseIdField, payload: Dict[str, Any], **kwd): - """ - PUT /api/groups/{encoded_group_id} - Modifies a group. - """ - self.manager.update(trans, EncodedDatabaseIdField.decode(id), payload) + @router.get( + "/api/groups/{encoded_group_id}", + summary="Displays information about a group.", + require_admin=True, + ) + def show( + self, + trans: ProvidesAppContext = DependsOnTrans, + encoded_group_id: EncodedDatabaseIdField = Path(), + ) -> GroupShowResponse: + """GET /api/groups/{encoded_group_id} - Displays information about a group.""" + group_id = EncodedDatabaseIdField.decode(encoded_group_id) + return self.manager.show(trans, group_id) + + @router.put( + "/api/groups/{encoded_group_id}", + summary="Modifies a group.", + require_admin=True, + ) + def update( + self, + trans: ProvidesAppContext = DependsOnTrans, + encoded_group_id: EncodedDatabaseIdField = Path(), + payload: GroupCreatePayload = Body(), + ): + """PUT /api/groups/{encoded_group_id} - Modifies a group.""" + group_id = EncodedDatabaseIdField.decode(encoded_group_id) + self.manager.update(trans, group_id, payload) diff --git a/lib/galaxy/webapps/galaxy/buildapp.py b/lib/galaxy/webapps/galaxy/buildapp.py index ed2b363e868f..86998f429af8 100644 --- a/lib/galaxy/webapps/galaxy/buildapp.py +++ b/lib/galaxy/webapps/galaxy/buildapp.py @@ -340,7 +340,6 @@ def populate_api_routes(webapp, app): controller="page_revisions", parent_resources=dict(member_name="page", collection_name="pages"), ) - webapp.mapper.resource("group", "groups", path_prefix="/api") webapp.mapper.connect("/api/cloud/authz/", action="index", controller="cloudauthz", conditions=dict(method=["GET"])) webapp.mapper.connect( diff --git a/lib/galaxy/webapps/galaxy/fast_app.py b/lib/galaxy/webapps/galaxy/fast_app.py index 997ec5b2b4c6..464f57270b09 100644 --- a/lib/galaxy/webapps/galaxy/fast_app.py +++ b/lib/galaxy/webapps/galaxy/fast_app.py @@ -45,6 +45,10 @@ "name": "group_roles", "description": "Operations with group roles.", }, + { + "name": "groups", + "description": "Operations with groups.", + }, { "name": "group_users", "description": "Operations with group users.", From 39b87a4e958e1685a25d4a0946a0ed667b7fc5af Mon Sep 17 00:00:00 2001 From: Arash Date: Sat, 18 Nov 2023 04:33:25 +0100 Subject: [PATCH 03/13] Add integration test for groups --- test/integration/test_groups.py | 122 ++++++++++++++++++++++++++++++++ 1 file changed, 122 insertions(+) create mode 100644 test/integration/test_groups.py diff --git a/test/integration/test_groups.py b/test/integration/test_groups.py new file mode 100644 index 000000000000..5464f71518d4 --- /dev/null +++ b/test/integration/test_groups.py @@ -0,0 +1,122 @@ +from typing import Optional + +from galaxy_test.base.populators import DatasetPopulator +from galaxy_test.driver import integration_util + + +class TestGroupsApi(integration_util.IntegrationTestCase): + dataset_populator: DatasetPopulator + + @classmethod + def handle_galaxy_config_kwds(cls, config): + super().handle_galaxy_config_kwds(config) + + def setUp(self): + super().setUp() + self.dataset_populator = DatasetPopulator(self.galaxy_interactor) + + def test_create_valid(self, group_name: Optional[str] = None): + payload = self._build_valid_group_payload(group_name) + response = self._post("groups", payload, admin=True, json=True) + self._assert_status_code_is(response, 200) + group = response.json()[0] # POST /api/groups returns a list + self._assert_valid_group(group) + return group + + def test_create_only_admin(self): + response = self._post("groups", json=True) + self._assert_status_code_is(response, 403) + + def test_create_invalid_params_raises_400(self): + payload = self._build_valid_group_payload() + payload["name"] = None + response = self._post("groups", payload, admin=True, json=True) + self._assert_status_code_is(response, 400) + + def test_create_duplicated_name_raises_409(self): + payload = self._build_valid_group_payload() + response = self._post("groups", payload, admin=True, json=True) + self._assert_status_code_is(response, 200) + + response = self._post("groups", payload, admin=True, json=True) + self._assert_status_code_is(response, 409) + + def test_index(self): + self.test_create_valid() + response = self._get("groups", admin=True) + self._assert_status_code_is(response, 200) + groups = response.json() + assert isinstance(groups, list) + assert len(groups) > 0 + for group in groups: + self._assert_valid_group(group) + + def test_index_only_admin(self): + response = self._get("groups") + self._assert_status_code_is(response, 403) + + def test_show(self): + group = self.test_create_valid() + group_id = group["id"] + response = self._get(f"groups/{group_id}", admin=True) + self._assert_status_code_is(response, 200) + response_group = response.json() + self._assert_valid_group(response_group) + self._assert_has_keys(response_group, "users_url", "roles_url") + + def test_show_only_admin(self): + group = self.test_create_valid() + group_id = group["id"] + response = self._get(f"groups/{group_id}") + self._assert_status_code_is(response, 403) + + def test_show_unknown_raises_400(self): + group_id = "invalid-group-id" + response = self._get(f"groups/{group_id}", admin=True) + self._assert_status_code_is(response, 400) + + def test_update(self): + group = self.test_create_valid(group_name="group-test") + + group_id = group["id"] + updated_name = "group-test-updated" + update_payload = { + "name": updated_name, + } + update_response = self._put(f"groups/{group_id}", data=update_payload, admin=True, json=True) + self._assert_status_code_is_ok(update_response) + + def test_update_only_admin(self): + group = self.test_create_valid() + group_id = group["id"] + response = self._put(f"groups/{group_id}") + self._assert_status_code_is(response, 403) + + def test_update_duplicating_name_raises_409(self): + group_a = self.test_create_valid() + group_b = self.test_create_valid() + + # Update group_b with the same name as group_a + group_b_id = group_b["id"] + updated_name = group_a["name"] + update_payload = { + "name": updated_name, + } + update_response = self._put(f"groups/{group_b_id}", data=update_payload, admin=True, json=True) + self._assert_status_code_is(update_response, 409) + + def _assert_valid_group(self, group, assert_id=None): + self._assert_has_keys(group, "id", "name", "model_class", "url") + if assert_id is not None: + assert group["id"] == assert_id + + def _build_valid_group_payload(self, name: Optional[str] = None): + name = name or self.dataset_populator.get_random_name() + user_id = self.dataset_populator.user_id() + role_id = self.dataset_populator.user_private_role_id() + payload = { + "name": name, + "user_ids": [user_id], + "role_ids": [role_id], + } + return payload From 0d2ed4ae749e039a7bf8f8123d5a755cbb28db50 Mon Sep 17 00:00:00 2001 From: Arash Date: Sat, 18 Nov 2023 12:23:19 +0100 Subject: [PATCH 04/13] Update group schema documentation --- lib/galaxy/schema/groups.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/lib/galaxy/schema/groups.py b/lib/galaxy/schema/groups.py index b8da8608f0d0..e204db6ca53c 100644 --- a/lib/galaxy/schema/groups.py +++ b/lib/galaxy/schema/groups.py @@ -10,7 +10,8 @@ class GroupIndexResponse(Model): - '''Response schema for a group.''' + """Response schema for a group.""" + model_class: str = Field( Required, title="model class", @@ -34,12 +35,14 @@ class GroupIndexResponse(Model): class GroupIndexListResponse(Model): - '''Response schema for listing groups.''' + """Response schema for listing groups.""" + __root__: List[GroupIndexResponse] class GroupShowResponse(Model): - '''Response schema for showing a group.''' + """Response schema for showing a group.""" + model_class: str = Field( Required, title="model class", @@ -73,7 +76,8 @@ class GroupShowResponse(Model): class GroupCreatePayload(Model): - '''Payload schema for creating a group.''' + """Payload schema for creating a group.""" + name: str = Field( Required, title="name of the group", From a80150be1a2b3312ab2ff942d0c6ee157720899f Mon Sep 17 00:00:00 2001 From: Arash Date: Sat, 18 Nov 2023 12:36:10 +0100 Subject: [PATCH 05/13] Add API endpoints for groups --- client/src/api/schema/schema.ts | 235 ++++++++++++++++++++++++++++++++ 1 file changed, 235 insertions(+) diff --git a/client/src/api/schema/schema.ts b/client/src/api/schema/schema.ts index d947c7b1f9c7..0d379ff4118c 100644 --- a/client/src/api/schema/schema.ts +++ b/client/src/api/schema/schema.ts @@ -381,6 +381,30 @@ export interface paths { /** Return raw sequence data */ get: operations["sequences_api_genomes__id__sequences_get"]; }; + "/api/groups": { + /** + * Displays a collection (list) of groups. + * @description GET /api/groups - Displays a collection (list) of groups. + */ + get: operations["index_api_groups_get"]; + /** + * Creates a new group. + * @description POST /api/groups - Creates a new group. + */ + post: operations["create_api_groups_post"]; + }; + "/api/groups/{encoded_group_id}": { + /** + * Displays information about a group. + * @description GET /api/groups/{encoded_group_id} - Displays information about a group. + */ + get: operations["show_api_groups__encoded_group_id__get"]; + /** + * Modifies a group. + * @description PUT /api/groups/{encoded_group_id} - Modifies a group. + */ + put: operations["update_api_groups__encoded_group_id__put"]; + }; "/api/groups/{group_id}/roles": { /** Displays a collection (list) of groups. */ get: operations["index_api_groups__group_id__roles_get"]; @@ -4626,6 +4650,61 @@ export interface components { /** Tags */ tags?: string[]; }; + /** + * GroupCreatePayload + * @description Payload schema for creating a group. + */ + GroupCreatePayload: { + /** + * name of the group + * @description name of the group + */ + name: string; + /** + * role IDs + * @description Encoded role IDs + * @default [] + */ + role_ids?: string[]; + /** + * user IDs + * @description Encoded user IDs + * @default [] + */ + user_ids?: string[]; + }; + /** + * GroupIndexListResponse + * @description Response schema for listing groups. + */ + GroupIndexListResponse: components["schemas"]["GroupIndexResponse"][]; + /** + * GroupIndexResponse + * @description Response schema for a group. + */ + GroupIndexResponse: { + /** + * group ID + * @description Encoded group ID + * @example 0123456789ABCDEF + */ + id: string; + /** + * model class + * @description model class + */ + model_class: string; + /** + * name of the group + * @description name of the group + */ + name: string; + /** + * URL for the group + * @description URL for the group + */ + url: string; + }; /** * GroupModel * @description User group model @@ -4687,6 +4766,42 @@ export interface components { */ url: string; }; + /** + * GroupShowResponse + * @description Response schema for showing a group. + */ + GroupShowResponse: { + /** + * group ID + * @description Encoded group ID + */ + id: string; + /** + * model class + * @description model class + */ + model_class: string; + /** + * name of the group + * @description name of the group + */ + name: string; + /** + * URL for the roles of the group + * @description URL for the roles of the group + */ + roles_url: string; + /** + * URL for the group + * @description URL for the group + */ + url: string; + /** + * URL for the users of the group + * @description URL for the users of the group + */ + users_url: string; + }; /** GroupUserListResponse */ GroupUserListResponse: components["schemas"]["GroupUserResponse"][]; /** GroupUserResponse */ @@ -11538,6 +11653,126 @@ export interface operations { }; }; }; + index_api_groups_get: { + /** + * Displays a collection (list) of groups. + * @description GET /api/groups - Displays a collection (list) of groups. + */ + parameters?: { + /** @description The user ID that will be used to effectively make this API call. Only admins and designated users can make API calls on behalf of other users. */ + header?: { + "run-as"?: string; + }; + }; + responses: { + /** @description Successful Response */ + 200: { + content: { + "application/json": components["schemas"]["GroupIndexListResponse"]; + }; + }; + /** @description Validation Error */ + 422: { + content: { + "application/json": components["schemas"]["HTTPValidationError"]; + }; + }; + }; + }; + create_api_groups_post: { + /** + * Creates a new group. + * @description POST /api/groups - Creates a new group. + */ + parameters?: { + /** @description The user ID that will be used to effectively make this API call. Only admins and designated users can make API calls on behalf of other users. */ + header?: { + "run-as"?: string; + }; + }; + requestBody: { + content: { + "application/json": components["schemas"]["GroupCreatePayload"]; + }; + }; + responses: { + /** @description Successful Response */ + 200: { + content: { + "application/json": components["schemas"]["GroupIndexListResponse"]; + }; + }; + /** @description Validation Error */ + 422: { + content: { + "application/json": components["schemas"]["HTTPValidationError"]; + }; + }; + }; + }; + show_api_groups__encoded_group_id__get: { + /** + * Displays information about a group. + * @description GET /api/groups/{encoded_group_id} - Displays information about a group. + */ + parameters: { + /** @description The user ID that will be used to effectively make this API call. Only admins and designated users can make API calls on behalf of other users. */ + header?: { + "run-as"?: string; + }; + path: { + encoded_group_id: string; + }; + }; + responses: { + /** @description Successful Response */ + 200: { + content: { + "application/json": components["schemas"]["GroupShowResponse"]; + }; + }; + /** @description Validation Error */ + 422: { + content: { + "application/json": components["schemas"]["HTTPValidationError"]; + }; + }; + }; + }; + update_api_groups__encoded_group_id__put: { + /** + * Modifies a group. + * @description PUT /api/groups/{encoded_group_id} - Modifies a group. + */ + parameters: { + /** @description The user ID that will be used to effectively make this API call. Only admins and designated users can make API calls on behalf of other users. */ + header?: { + "run-as"?: string; + }; + path: { + encoded_group_id: string; + }; + }; + requestBody: { + content: { + "application/json": components["schemas"]["GroupCreatePayload"]; + }; + }; + responses: { + /** @description Successful Response */ + 200: { + content: { + "application/json": Record; + }; + }; + /** @description Validation Error */ + 422: { + content: { + "application/json": components["schemas"]["HTTPValidationError"]; + }; + }; + }; + }; index_api_groups__group_id__roles_get: { /** Displays a collection (list) of groups. */ parameters: { From 2d391dc4e58d5c97ba0c42f76f6f9559ff3e4c55 Mon Sep 17 00:00:00 2001 From: Arash Date: Mon, 20 Nov 2023 15:11:03 +0100 Subject: [PATCH 06/13] Refactor group manager and API endpoints --- lib/galaxy/managers/groups.py | 8 +- lib/galaxy/schema/groups.py | 49 +++++----- lib/galaxy/webapps/galaxy/api/groups.py | 20 ++-- test/integration/test_groups.py | 122 ------------------------ 4 files changed, 35 insertions(+), 164 deletions(-) delete mode 100644 test/integration/test_groups.py diff --git a/lib/galaxy/managers/groups.py b/lib/galaxy/managers/groups.py index f22a4d4f2fd0..a6909e8d14aa 100644 --- a/lib/galaxy/managers/groups.py +++ b/lib/galaxy/managers/groups.py @@ -60,9 +60,9 @@ def create(self, trans: ProvidesAppContext, payload: GroupCreatePayload): group = model.Group(name=name) sa_session.add(group) - encoded_user_ids = getattr(payload, "user_ids", []) + encoded_user_ids = payload.user_ids users = self._get_users_by_encoded_ids(sa_session, encoded_user_ids) - encoded_role_ids = getattr(payload, "role_ids", []) + encoded_role_ids = payload.role_ids roles = self._get_roles_by_encoded_ids(sa_session, encoded_role_ids) trans.app.security_agent.set_entity_group_associations(groups=[group], roles=roles, users=users) with transaction(sa_session): @@ -96,9 +96,9 @@ def update(self, trans: ProvidesAppContext, group_id: int, payload: GroupCreateP self._check_duplicated_group_name(sa_session, name) group.name = name sa_session.add(group) - encoded_user_ids = getattr(payload, "user_ids", []) + encoded_user_ids = payload.user_ids users = self._get_users_by_encoded_ids(sa_session, encoded_user_ids) - encoded_role_ids = getattr(payload, "role_ids", []) + encoded_role_ids = payload.role_ids roles = self._get_roles_by_encoded_ids(sa_session, encoded_role_ids) self._app.security_agent.set_entity_group_associations( groups=[group], roles=roles, users=users, delete_existing_assocs=False diff --git a/lib/galaxy/schema/groups.py b/lib/galaxy/schema/groups.py index e204db6ca53c..50e3d1ebf0fe 100644 --- a/lib/galaxy/schema/groups.py +++ b/lib/galaxy/schema/groups.py @@ -1,34 +1,33 @@ from typing import List -from pydantic import ( - Field, - Required, -) +from pydantic import Field +from typing_extensions import Literal -from galaxy.schema.fields import EncodedDatabaseIdField +from galaxy.schema.fields import ( + DecodedDatabaseIdField, + ModelClassField, +) from galaxy.schema.schema import Model +GROUP_MODEL_CLASS = Literal["Group"] + class GroupIndexResponse(Model): """Response schema for a group.""" - model_class: str = Field( - Required, - title="model class", - description="model class", - ) - id: EncodedDatabaseIdField = Field( - Required, + model_class: GROUP_MODEL_CLASS = ModelClassField(GROUP_MODEL_CLASS) + id: DecodedDatabaseIdField = Field( + ..., title="group ID", description="Encoded group ID", ) url: str = Field( - Required, + ..., title="URL for the group", description="URL for the group", ) name: str = Field( - Required, + ..., title="name of the group", description="name of the group", ) @@ -43,33 +42,29 @@ class GroupIndexListResponse(Model): class GroupShowResponse(Model): """Response schema for showing a group.""" - model_class: str = Field( - Required, - title="model class", - description="model class", - ) + model_class: GROUP_MODEL_CLASS = ModelClassField(GROUP_MODEL_CLASS) id: str = Field( - Required, + ..., title="group ID", description="Encoded group ID", ) url: str = Field( - Required, + ..., title="URL for the group", description="URL for the group", ) name: str = Field( - Required, + ..., title="name of the group", description="name of the group", ) roles_url: str = Field( - Required, + ..., title="URL for the roles of the group", description="URL for the roles of the group", ) users_url: str = Field( - Required, + ..., title="URL for the users of the group", description="URL for the users of the group", ) @@ -79,16 +74,16 @@ class GroupCreatePayload(Model): """Payload schema for creating a group.""" name: str = Field( - Required, + ..., title="name of the group", description="name of the group", ) - user_ids: List[EncodedDatabaseIdField] = Field( + user_ids: List[DecodedDatabaseIdField] = Field( [], title="user IDs", description="Encoded user IDs", ) - role_ids: List[EncodedDatabaseIdField] = Field( + role_ids: List[DecodedDatabaseIdField] = Field( [], title="role IDs", description="Encoded role IDs", diff --git a/lib/galaxy/webapps/galaxy/api/groups.py b/lib/galaxy/webapps/galaxy/api/groups.py index c1b63efbab9a..ecd1fdbc0d76 100644 --- a/lib/galaxy/webapps/galaxy/api/groups.py +++ b/lib/galaxy/webapps/galaxy/api/groups.py @@ -10,7 +10,7 @@ from galaxy.managers.context import ProvidesAppContext from galaxy.managers.groups import GroupsManager -from galaxy.schema.fields import EncodedDatabaseIdField +from galaxy.schema.fields import DecodedDatabaseIdField from galaxy.schema.groups import ( GroupCreatePayload, GroupIndexListResponse, @@ -40,7 +40,7 @@ def index( self, trans: ProvidesAppContext = DependsOnTrans, ) -> GroupIndexListResponse: - """GET /api/groups - Displays a collection (list) of groups.""" + """ """ return self.manager.index(trans) @router.post( @@ -53,34 +53,32 @@ def create( trans: ProvidesAppContext = DependsOnTrans, payload: GroupCreatePayload = Body(), ) -> GroupIndexListResponse: - """POST /api/groups - Creates a new group.""" + """ """ return self.manager.create(trans, payload) @router.get( - "/api/groups/{encoded_group_id}", + "/api/groups/{group_id}", summary="Displays information about a group.", require_admin=True, ) def show( self, trans: ProvidesAppContext = DependsOnTrans, - encoded_group_id: EncodedDatabaseIdField = Path(), + group_id: DecodedDatabaseIdField = Path(), ) -> GroupShowResponse: - """GET /api/groups/{encoded_group_id} - Displays information about a group.""" - group_id = EncodedDatabaseIdField.decode(encoded_group_id) + """ """ return self.manager.show(trans, group_id) @router.put( - "/api/groups/{encoded_group_id}", + "/api/groups/{group_id}", summary="Modifies a group.", require_admin=True, ) def update( self, trans: ProvidesAppContext = DependsOnTrans, - encoded_group_id: EncodedDatabaseIdField = Path(), + group_id: DecodedDatabaseIdField = Path(), payload: GroupCreatePayload = Body(), ): - """PUT /api/groups/{encoded_group_id} - Modifies a group.""" - group_id = EncodedDatabaseIdField.decode(encoded_group_id) + """ """ self.manager.update(trans, group_id, payload) diff --git a/test/integration/test_groups.py b/test/integration/test_groups.py deleted file mode 100644 index 5464f71518d4..000000000000 --- a/test/integration/test_groups.py +++ /dev/null @@ -1,122 +0,0 @@ -from typing import Optional - -from galaxy_test.base.populators import DatasetPopulator -from galaxy_test.driver import integration_util - - -class TestGroupsApi(integration_util.IntegrationTestCase): - dataset_populator: DatasetPopulator - - @classmethod - def handle_galaxy_config_kwds(cls, config): - super().handle_galaxy_config_kwds(config) - - def setUp(self): - super().setUp() - self.dataset_populator = DatasetPopulator(self.galaxy_interactor) - - def test_create_valid(self, group_name: Optional[str] = None): - payload = self._build_valid_group_payload(group_name) - response = self._post("groups", payload, admin=True, json=True) - self._assert_status_code_is(response, 200) - group = response.json()[0] # POST /api/groups returns a list - self._assert_valid_group(group) - return group - - def test_create_only_admin(self): - response = self._post("groups", json=True) - self._assert_status_code_is(response, 403) - - def test_create_invalid_params_raises_400(self): - payload = self._build_valid_group_payload() - payload["name"] = None - response = self._post("groups", payload, admin=True, json=True) - self._assert_status_code_is(response, 400) - - def test_create_duplicated_name_raises_409(self): - payload = self._build_valid_group_payload() - response = self._post("groups", payload, admin=True, json=True) - self._assert_status_code_is(response, 200) - - response = self._post("groups", payload, admin=True, json=True) - self._assert_status_code_is(response, 409) - - def test_index(self): - self.test_create_valid() - response = self._get("groups", admin=True) - self._assert_status_code_is(response, 200) - groups = response.json() - assert isinstance(groups, list) - assert len(groups) > 0 - for group in groups: - self._assert_valid_group(group) - - def test_index_only_admin(self): - response = self._get("groups") - self._assert_status_code_is(response, 403) - - def test_show(self): - group = self.test_create_valid() - group_id = group["id"] - response = self._get(f"groups/{group_id}", admin=True) - self._assert_status_code_is(response, 200) - response_group = response.json() - self._assert_valid_group(response_group) - self._assert_has_keys(response_group, "users_url", "roles_url") - - def test_show_only_admin(self): - group = self.test_create_valid() - group_id = group["id"] - response = self._get(f"groups/{group_id}") - self._assert_status_code_is(response, 403) - - def test_show_unknown_raises_400(self): - group_id = "invalid-group-id" - response = self._get(f"groups/{group_id}", admin=True) - self._assert_status_code_is(response, 400) - - def test_update(self): - group = self.test_create_valid(group_name="group-test") - - group_id = group["id"] - updated_name = "group-test-updated" - update_payload = { - "name": updated_name, - } - update_response = self._put(f"groups/{group_id}", data=update_payload, admin=True, json=True) - self._assert_status_code_is_ok(update_response) - - def test_update_only_admin(self): - group = self.test_create_valid() - group_id = group["id"] - response = self._put(f"groups/{group_id}") - self._assert_status_code_is(response, 403) - - def test_update_duplicating_name_raises_409(self): - group_a = self.test_create_valid() - group_b = self.test_create_valid() - - # Update group_b with the same name as group_a - group_b_id = group_b["id"] - updated_name = group_a["name"] - update_payload = { - "name": updated_name, - } - update_response = self._put(f"groups/{group_b_id}", data=update_payload, admin=True, json=True) - self._assert_status_code_is(update_response, 409) - - def _assert_valid_group(self, group, assert_id=None): - self._assert_has_keys(group, "id", "name", "model_class", "url") - if assert_id is not None: - assert group["id"] == assert_id - - def _build_valid_group_payload(self, name: Optional[str] = None): - name = name or self.dataset_populator.get_random_name() - user_id = self.dataset_populator.user_id() - role_id = self.dataset_populator.user_private_role_id() - payload = { - "name": name, - "user_ids": [user_id], - "role_ids": [role_id], - } - return payload From 055816b6e00c5f0cb89ed699b4aefed93c09c282 Mon Sep 17 00:00:00 2001 From: Arash Date: Mon, 20 Nov 2023 15:54:39 +0100 Subject: [PATCH 07/13] Refactor API group endpoints --- client/src/api/schema/schema.ts | 70 ++++++++++++--------------------- 1 file changed, 25 insertions(+), 45 deletions(-) diff --git a/client/src/api/schema/schema.ts b/client/src/api/schema/schema.ts index 0d379ff4118c..311306edf922 100644 --- a/client/src/api/schema/schema.ts +++ b/client/src/api/schema/schema.ts @@ -382,28 +382,16 @@ export interface paths { get: operations["sequences_api_genomes__id__sequences_get"]; }; "/api/groups": { - /** - * Displays a collection (list) of groups. - * @description GET /api/groups - Displays a collection (list) of groups. - */ + /** Displays a collection (list) of groups. */ get: operations["index_api_groups_get"]; - /** - * Creates a new group. - * @description POST /api/groups - Creates a new group. - */ + /** Creates a new group. */ post: operations["create_api_groups_post"]; }; - "/api/groups/{encoded_group_id}": { - /** - * Displays information about a group. - * @description GET /api/groups/{encoded_group_id} - Displays information about a group. - */ - get: operations["show_api_groups__encoded_group_id__get"]; - /** - * Modifies a group. - * @description PUT /api/groups/{encoded_group_id} - Modifies a group. - */ - put: operations["update_api_groups__encoded_group_id__put"]; + "/api/groups/{group_id}": { + /** Displays information about a group. */ + get: operations["show_api_groups__group_id__get"]; + /** Modifies a group. */ + put: operations["update_api_groups__group_id__put"]; }; "/api/groups/{group_id}/roles": { /** Displays a collection (list) of groups. */ @@ -4690,10 +4678,12 @@ export interface components { */ id: string; /** - * model class - * @description model class + * Model class + * @description The name of the database model class. + * @default Group + * @enum {string} */ - model_class: string; + model_class: "Group"; /** * name of the group * @description name of the group @@ -4777,10 +4767,12 @@ export interface components { */ id: string; /** - * model class - * @description model class + * Model class + * @description The name of the database model class. + * @default Group + * @enum {string} */ - model_class: string; + model_class: "Group"; /** * name of the group * @description name of the group @@ -11654,10 +11646,7 @@ export interface operations { }; }; index_api_groups_get: { - /** - * Displays a collection (list) of groups. - * @description GET /api/groups - Displays a collection (list) of groups. - */ + /** Displays a collection (list) of groups. */ parameters?: { /** @description The user ID that will be used to effectively make this API call. Only admins and designated users can make API calls on behalf of other users. */ header?: { @@ -11680,10 +11669,7 @@ export interface operations { }; }; create_api_groups_post: { - /** - * Creates a new group. - * @description POST /api/groups - Creates a new group. - */ + /** Creates a new group. */ parameters?: { /** @description The user ID that will be used to effectively make this API call. Only admins and designated users can make API calls on behalf of other users. */ header?: { @@ -11710,18 +11696,15 @@ export interface operations { }; }; }; - show_api_groups__encoded_group_id__get: { - /** - * Displays information about a group. - * @description GET /api/groups/{encoded_group_id} - Displays information about a group. - */ + show_api_groups__group_id__get: { + /** Displays information about a group. */ parameters: { /** @description The user ID that will be used to effectively make this API call. Only admins and designated users can make API calls on behalf of other users. */ header?: { "run-as"?: string; }; path: { - encoded_group_id: string; + group_id: string; }; }; responses: { @@ -11739,18 +11722,15 @@ export interface operations { }; }; }; - update_api_groups__encoded_group_id__put: { - /** - * Modifies a group. - * @description PUT /api/groups/{encoded_group_id} - Modifies a group. - */ + update_api_groups__group_id__put: { + /** Modifies a group. */ parameters: { /** @description The user ID that will be used to effectively make this API call. Only admins and designated users can make API calls on behalf of other users. */ header?: { "run-as"?: string; }; path: { - encoded_group_id: string; + group_id: string; }; }; requestBody: { From 91b6becd50785953cda1ec1769fe6ec64a13d25f Mon Sep 17 00:00:00 2001 From: Arash Date: Mon, 20 Nov 2023 22:04:10 +0100 Subject: [PATCH 08/13] Refactor group manager methods and schema --- client/src/api/schema/schema.ts | 38 +++++-------------------- lib/galaxy/managers/groups.py | 38 +++++++++---------------- lib/galaxy/schema/groups.py | 10 ------- lib/galaxy/webapps/galaxy/api/groups.py | 12 +++----- 4 files changed, 24 insertions(+), 74 deletions(-) diff --git a/client/src/api/schema/schema.ts b/client/src/api/schema/schema.ts index 311306edf922..434e0e885e67 100644 --- a/client/src/api/schema/schema.ts +++ b/client/src/api/schema/schema.ts @@ -4650,13 +4650,11 @@ export interface components { name: string; /** * role IDs - * @description Encoded role IDs * @default [] */ role_ids?: string[]; /** * user IDs - * @description Encoded user IDs * @default [] */ user_ids?: string[]; @@ -4673,7 +4671,6 @@ export interface components { GroupIndexResponse: { /** * group ID - * @description Encoded group ID * @example 0123456789ABCDEF */ id: string; @@ -4684,15 +4681,9 @@ export interface components { * @enum {string} */ model_class: "Group"; - /** - * name of the group - * @description name of the group - */ + /** name of the group */ name: string; - /** - * URL for the group - * @description URL for the group - */ + /** URL for the group */ url: string; }; /** @@ -4761,10 +4752,7 @@ export interface components { * @description Response schema for showing a group. */ GroupShowResponse: { - /** - * group ID - * @description Encoded group ID - */ + /** group ID */ id: string; /** * Model class @@ -4773,25 +4761,13 @@ export interface components { * @enum {string} */ model_class: "Group"; - /** - * name of the group - * @description name of the group - */ + /** name of the group */ name: string; - /** - * URL for the roles of the group - * @description URL for the roles of the group - */ + /** URL for the roles of the group */ roles_url: string; - /** - * URL for the group - * @description URL for the group - */ + /** URL for the group */ url: string; - /** - * URL for the users of the group - * @description URL for the users of the group - */ + /** URL for the users of the group */ users_url: string; }; /** GroupUserListResponse */ diff --git a/lib/galaxy/managers/groups.py b/lib/galaxy/managers/groups.py index a6909e8d14aa..a8093aa18c61 100644 --- a/lib/galaxy/managers/groups.py +++ b/lib/galaxy/managers/groups.py @@ -12,7 +12,6 @@ ObjectAttributeMissingException, ObjectNotFound, ) -from galaxy.managers.base import decode_id from galaxy.managers.context import ProvidesAppContext from galaxy.managers.users import get_users_by_ids from galaxy.model import ( @@ -21,10 +20,7 @@ ) from galaxy.model.base import transaction from galaxy.model.scoped_session import galaxy_scoped_session -from galaxy.schema.fields import ( - DecodedDatabaseIdField, - EncodedDatabaseIdField, -) +from galaxy.schema.fields import DecodedDatabaseIdField from galaxy.schema.groups import GroupCreatePayload from galaxy.structured_app import MinimalManagerApp from galaxy.web import url_for @@ -60,10 +56,10 @@ def create(self, trans: ProvidesAppContext, payload: GroupCreatePayload): group = model.Group(name=name) sa_session.add(group) - encoded_user_ids = payload.user_ids - users = self._get_users_by_encoded_ids(sa_session, encoded_user_ids) - encoded_role_ids = payload.role_ids - roles = self._get_roles_by_encoded_ids(sa_session, encoded_role_ids) + user_ids = payload.user_ids + users = self._get_users_by_ids(sa_session, user_ids) + role_ids = payload.role_ids + roles = self._get_roles_by_ids(sa_session, role_ids) trans.app.security_agent.set_entity_group_associations(groups=[group], roles=roles, users=users) with transaction(sa_session): sa_session.commit() @@ -96,10 +92,10 @@ def update(self, trans: ProvidesAppContext, group_id: int, payload: GroupCreateP self._check_duplicated_group_name(sa_session, name) group.name = name sa_session.add(group) - encoded_user_ids = payload.user_ids - users = self._get_users_by_encoded_ids(sa_session, encoded_user_ids) - encoded_role_ids = payload.role_ids - roles = self._get_roles_by_encoded_ids(sa_session, encoded_role_ids) + user_ids = payload.user_ids + users = self._get_users_by_ids(sa_session, user_ids) + role_ids = payload.role_ids + roles = self._get_roles_by_ids(sa_session, role_ids) self._app.security_agent.set_entity_group_associations( groups=[group], roles=roles, users=users, delete_existing_assocs=False ) @@ -116,25 +112,17 @@ def _get_group(self, sa_session: galaxy_scoped_session, group_id: int) -> model. raise ObjectNotFound("Group with the provided id was not found.") return group - def _get_users_by_encoded_ids( - self, sa_session: galaxy_scoped_session, encoded_user_ids: List[EncodedDatabaseIdField] + def _get_users_by_ids( + self, sa_session: galaxy_scoped_session, user_ids: List[DecodedDatabaseIdField] ) -> List[model.User]: - user_ids = self._decode_ids(encoded_user_ids) return get_users_by_ids(sa_session, user_ids) - def _get_roles_by_encoded_ids( - self, sa_session: galaxy_scoped_session, encoded_role_ids: List[EncodedDatabaseIdField] + def _get_roles_by_ids( + self, sa_session: galaxy_scoped_session, role_ids: List[DecodedDatabaseIdField] ) -> List[model.Role]: - role_ids = self._decode_ids(encoded_role_ids) stmt = select(Role).where(Role.id.in_(role_ids)) return sa_session.scalars(stmt).all() - def _decode_id(self, encoded_id: EncodedDatabaseIdField) -> int: - return decode_id(self._app, encoded_id) - - def _decode_ids(self, encoded_ids: List[EncodedDatabaseIdField]) -> List[int]: - return [self._decode_id(encoded_id) for encoded_id in encoded_ids] - def get_group_by_name(session: Session, name: str): stmt = select(Group).filter(Group.name == name).limit(1) diff --git a/lib/galaxy/schema/groups.py b/lib/galaxy/schema/groups.py index 50e3d1ebf0fe..99433d3e90c9 100644 --- a/lib/galaxy/schema/groups.py +++ b/lib/galaxy/schema/groups.py @@ -19,17 +19,14 @@ class GroupIndexResponse(Model): id: DecodedDatabaseIdField = Field( ..., title="group ID", - description="Encoded group ID", ) url: str = Field( ..., title="URL for the group", - description="URL for the group", ) name: str = Field( ..., title="name of the group", - description="name of the group", ) @@ -46,27 +43,22 @@ class GroupShowResponse(Model): id: str = Field( ..., title="group ID", - description="Encoded group ID", ) url: str = Field( ..., title="URL for the group", - description="URL for the group", ) name: str = Field( ..., title="name of the group", - description="name of the group", ) roles_url: str = Field( ..., title="URL for the roles of the group", - description="URL for the roles of the group", ) users_url: str = Field( ..., title="URL for the users of the group", - description="URL for the users of the group", ) @@ -81,10 +73,8 @@ class GroupCreatePayload(Model): user_ids: List[DecodedDatabaseIdField] = Field( [], title="user IDs", - description="Encoded user IDs", ) role_ids: List[DecodedDatabaseIdField] = Field( [], title="role IDs", - description="Encoded role IDs", ) diff --git a/lib/galaxy/webapps/galaxy/api/groups.py b/lib/galaxy/webapps/galaxy/api/groups.py index ecd1fdbc0d76..6d05c5a658a7 100644 --- a/lib/galaxy/webapps/galaxy/api/groups.py +++ b/lib/galaxy/webapps/galaxy/api/groups.py @@ -40,7 +40,6 @@ def index( self, trans: ProvidesAppContext = DependsOnTrans, ) -> GroupIndexListResponse: - """ """ return self.manager.index(trans) @router.post( @@ -51,9 +50,8 @@ def index( def create( self, trans: ProvidesAppContext = DependsOnTrans, - payload: GroupCreatePayload = Body(), + payload: GroupCreatePayload = Body(...), ) -> GroupIndexListResponse: - """ """ return self.manager.create(trans, payload) @router.get( @@ -64,9 +62,8 @@ def create( def show( self, trans: ProvidesAppContext = DependsOnTrans, - group_id: DecodedDatabaseIdField = Path(), + group_id: DecodedDatabaseIdField = Path(...), ) -> GroupShowResponse: - """ """ return self.manager.show(trans, group_id) @router.put( @@ -77,8 +74,7 @@ def show( def update( self, trans: ProvidesAppContext = DependsOnTrans, - group_id: DecodedDatabaseIdField = Path(), - payload: GroupCreatePayload = Body(), + group_id: DecodedDatabaseIdField = Path(...), + payload: GroupCreatePayload = Body(...), ): - """ """ self.manager.update(trans, group_id, payload) From d03e1a787fb841c332f3b08bc7aff7d390819fa2 Mon Sep 17 00:00:00 2001 From: Arash Date: Tue, 21 Nov 2023 22:39:06 +0100 Subject: [PATCH 09/13] Refactor group API and role manager Change create response to a single group response instead of a list Add response for update Fixing url parameter --- client/src/api/schema/schema.ts | 88 ++++++++++--------------- lib/galaxy/managers/groups.py | 54 +++++++-------- lib/galaxy/managers/roles.py | 10 ++- lib/galaxy/schema/groups.py | 62 +++++++---------- lib/galaxy/webapps/galaxy/api/groups.py | 17 +++-- lib/galaxy_test/api/test_groups.py | 2 +- 6 files changed, 106 insertions(+), 127 deletions(-) diff --git a/client/src/api/schema/schema.ts b/client/src/api/schema/schema.ts index 434e0e885e67..0a6eb2a5e488 100644 --- a/client/src/api/schema/schema.ts +++ b/client/src/api/schema/schema.ts @@ -4643,10 +4643,7 @@ export interface components { * @description Payload schema for creating a group. */ GroupCreatePayload: { - /** - * name of the group - * @description name of the group - */ + /** name of the group */ name: string; /** * role IDs @@ -4660,32 +4657,10 @@ export interface components { user_ids?: string[]; }; /** - * GroupIndexListResponse + * GroupListResponse * @description Response schema for listing groups. */ - GroupIndexListResponse: components["schemas"]["GroupIndexResponse"][]; - /** - * GroupIndexResponse - * @description Response schema for a group. - */ - GroupIndexResponse: { - /** - * group ID - * @example 0123456789ABCDEF - */ - id: string; - /** - * Model class - * @description The name of the database model class. - * @default Group - * @enum {string} - */ - model_class: "Group"; - /** name of the group */ - name: string; - /** URL for the group */ - url: string; - }; + GroupListResponse: components["schemas"]["GroupResponse"][]; /** * GroupModel * @description User group model @@ -4725,6 +4700,32 @@ export interface components { */ model_class: "GroupQuotaAssociation"; }; + /** + * GroupResponse + * @description Response schema for a group. + */ + GroupResponse: { + /** + * group ID + * @example 0123456789ABCDEF + */ + id: string; + /** + * Model class + * @description The name of the database model class. + * @default Group + * @enum {string} + */ + model_class: "Group"; + /** name of the group */ + name: string; + /** URL for the roles of the group */ + roles_url?: string[]; + /** URL for the group */ + url: string; + /** URL for the users of the group */ + users_url?: string[]; + }; /** GroupRoleListResponse */ GroupRoleListResponse: components["schemas"]["GroupRoleResponse"][]; /** GroupRoleResponse */ @@ -4747,29 +4748,6 @@ export interface components { */ url: string; }; - /** - * GroupShowResponse - * @description Response schema for showing a group. - */ - GroupShowResponse: { - /** group ID */ - id: string; - /** - * Model class - * @description The name of the database model class. - * @default Group - * @enum {string} - */ - model_class: "Group"; - /** name of the group */ - name: string; - /** URL for the roles of the group */ - roles_url: string; - /** URL for the group */ - url: string; - /** URL for the users of the group */ - users_url: string; - }; /** GroupUserListResponse */ GroupUserListResponse: components["schemas"]["GroupUserResponse"][]; /** GroupUserResponse */ @@ -11633,7 +11611,7 @@ export interface operations { /** @description Successful Response */ 200: { content: { - "application/json": components["schemas"]["GroupIndexListResponse"]; + "application/json": components["schemas"]["GroupListResponse"]; }; }; /** @description Validation Error */ @@ -11661,7 +11639,7 @@ export interface operations { /** @description Successful Response */ 200: { content: { - "application/json": components["schemas"]["GroupIndexListResponse"]; + "application/json": components["schemas"]["GroupResponse"]; }; }; /** @description Validation Error */ @@ -11687,7 +11665,7 @@ export interface operations { /** @description Successful Response */ 200: { content: { - "application/json": components["schemas"]["GroupShowResponse"]; + "application/json": components["schemas"]["GroupResponse"]; }; }; /** @description Validation Error */ @@ -11718,7 +11696,7 @@ export interface operations { /** @description Successful Response */ 200: { content: { - "application/json": Record; + "application/json": components["schemas"]["GroupResponse"]; }; }; /** @description Validation Error */ diff --git a/lib/galaxy/managers/groups.py b/lib/galaxy/managers/groups.py index a8093aa18c61..e71de11eb508 100644 --- a/lib/galaxy/managers/groups.py +++ b/lib/galaxy/managers/groups.py @@ -13,17 +13,14 @@ ObjectNotFound, ) from galaxy.managers.context import ProvidesAppContext +from galaxy.managers.roles import get_roles_by_ids from galaxy.managers.users import get_users_by_ids -from galaxy.model import ( - Group, - Role, -) +from galaxy.model import Group from galaxy.model.base import transaction from galaxy.model.scoped_session import galaxy_scoped_session from galaxy.schema.fields import DecodedDatabaseIdField from galaxy.schema.groups import GroupCreatePayload from galaxy.structured_app import MinimalManagerApp -from galaxy.web import url_for class GroupsManager: @@ -40,7 +37,7 @@ def index(self, trans: ProvidesAppContext): for group in get_not_deleted_groups(trans.sa_session): item = group.to_dict(value_mapper={"id": DecodedDatabaseIdField.encode}) encoded_id = DecodedDatabaseIdField.encode(group.id) - item["url"] = url_for("group", id=encoded_id) + item["url"] = self._url_for(trans, "index", group_id=encoded_id) rval.append(item) return rval @@ -57,17 +54,17 @@ def create(self, trans: ProvidesAppContext, payload: GroupCreatePayload): group = model.Group(name=name) sa_session.add(group) user_ids = payload.user_ids - users = self._get_users_by_ids(sa_session, user_ids) + users = get_users_by_ids(sa_session, user_ids) role_ids = payload.role_ids - roles = self._get_roles_by_ids(sa_session, role_ids) + roles = get_roles_by_ids(sa_session, role_ids) trans.app.security_agent.set_entity_group_associations(groups=[group], roles=roles, users=users) with transaction(sa_session): sa_session.commit() encoded_id = DecodedDatabaseIdField.encode(group.id) item = group.to_dict(view="element", value_mapper={"id": DecodedDatabaseIdField.encode}) - item["url"] = url_for("group", id=encoded_id) - return [item] + item["url"] = self._url_for(trans, "create", group_id=encoded_id) + return item def show(self, trans: ProvidesAppContext, group_id: int): """ @@ -76,9 +73,17 @@ def show(self, trans: ProvidesAppContext, group_id: int): encoded_id = DecodedDatabaseIdField.encode(group_id) group = self._get_group(trans.sa_session, group_id) item = group.to_dict(view="element", value_mapper={"id": DecodedDatabaseIdField.encode}) - item["url"] = url_for("group", id=encoded_id) - item["users_url"] = url_for("group_users", group_id=encoded_id) - item["roles_url"] = url_for("group_roles", group_id=encoded_id) + item["url"] = self._url_for(trans, "show", group_id=encoded_id) + encoded_user_id = [DecodedDatabaseIdField.encode(gu.user.id) for gu in group.users] + encoded_role_id = [DecodedDatabaseIdField.encode(gr.role.id) for gr in group.roles] + item["users_url"] = [ + self._url_for(trans, "group_user", group_id=encoded_id, user_id=group_user) + for group_user in encoded_user_id + ] + item["roles_url"] = [ + self._url_for(trans, "group_role", group_id=encoded_id, role_id=group_role) + for group_role in encoded_role_id + ] return item def update(self, trans: ProvidesAppContext, group_id: int, payload: GroupCreatePayload): @@ -93,15 +98,23 @@ def update(self, trans: ProvidesAppContext, group_id: int, payload: GroupCreateP group.name = name sa_session.add(group) user_ids = payload.user_ids - users = self._get_users_by_ids(sa_session, user_ids) + users = get_users_by_ids(sa_session, user_ids) role_ids = payload.role_ids - roles = self._get_roles_by_ids(sa_session, role_ids) + roles = get_roles_by_ids(sa_session, role_ids) self._app.security_agent.set_entity_group_associations( groups=[group], roles=roles, users=users, delete_existing_assocs=False ) with transaction(sa_session): sa_session.commit() + encoded_id = DecodedDatabaseIdField.encode(group.id) + item = group.to_dict(view="element", value_mapper={"id": DecodedDatabaseIdField.encode}) + item["url"] = self._url_for(trans, "update", group_id=encoded_id) + return item + + def _url_for(self, trans, name, **kwargs): + return trans.url_builder(name, **kwargs) + def _check_duplicated_group_name(self, sa_session: galaxy_scoped_session, group_name: str) -> None: if get_group_by_name(sa_session, group_name): raise Conflict(f"A group with name '{group_name}' already exists") @@ -112,17 +125,6 @@ def _get_group(self, sa_session: galaxy_scoped_session, group_id: int) -> model. raise ObjectNotFound("Group with the provided id was not found.") return group - def _get_users_by_ids( - self, sa_session: galaxy_scoped_session, user_ids: List[DecodedDatabaseIdField] - ) -> List[model.User]: - return get_users_by_ids(sa_session, user_ids) - - def _get_roles_by_ids( - self, sa_session: galaxy_scoped_session, role_ids: List[DecodedDatabaseIdField] - ) -> List[model.Role]: - stmt = select(Role).where(Role.id.in_(role_ids)) - return sa_session.scalars(stmt).all() - def get_group_by_name(session: Session, name: str): stmt = select(Group).filter(Group.name == name).limit(1) diff --git a/lib/galaxy/managers/roles.py b/lib/galaxy/managers/roles.py index bc840937a942..fbe96ba2b2ad 100644 --- a/lib/galaxy/managers/roles.py +++ b/lib/galaxy/managers/roles.py @@ -8,7 +8,10 @@ false, select, ) -from sqlalchemy.orm import exc as sqlalchemy_exceptions +from sqlalchemy.orm import ( + exc as sqlalchemy_exceptions, + Session, +) import galaxy.exceptions from galaxy import model @@ -97,3 +100,8 @@ def create_role(self, trans: ProvidesUserContext, role_definition_model: RoleDef with transaction(trans.sa_session): trans.sa_session.commit() return role + + +def get_roles_by_ids(session: Session, role_ids): + stmt = select(Role).where(Role.id.in_(role_ids)) + return session.scalars(stmt).all() diff --git a/lib/galaxy/schema/groups.py b/lib/galaxy/schema/groups.py index 99433d3e90c9..4b0b26b3ed22 100644 --- a/lib/galaxy/schema/groups.py +++ b/lib/galaxy/schema/groups.py @@ -1,10 +1,17 @@ -from typing import List +from typing import ( + List, + Optional, +) -from pydantic import Field +from pydantic import ( + Field, + Required, +) from typing_extensions import Literal from galaxy.schema.fields import ( DecodedDatabaseIdField, + EncodedDatabaseIdField, ModelClassField, ) from galaxy.schema.schema import Model @@ -12,63 +19,44 @@ GROUP_MODEL_CLASS = Literal["Group"] -class GroupIndexResponse(Model): +class GroupResponse(Model): """Response schema for a group.""" model_class: GROUP_MODEL_CLASS = ModelClassField(GROUP_MODEL_CLASS) - id: DecodedDatabaseIdField = Field( - ..., + id: EncodedDatabaseIdField = Field( + Required, title="group ID", ) - url: str = Field( - ..., - title="URL for the group", - ) name: str = Field( - ..., + Required, title="name of the group", ) - - -class GroupIndexListResponse(Model): - """Response schema for listing groups.""" - - __root__: List[GroupIndexResponse] - - -class GroupShowResponse(Model): - """Response schema for showing a group.""" - - model_class: GROUP_MODEL_CLASS = ModelClassField(GROUP_MODEL_CLASS) - id: str = Field( - ..., - title="group ID", - ) url: str = Field( - ..., + Required, title="URL for the group", ) - name: str = Field( - ..., - title="name of the group", - ) - roles_url: str = Field( - ..., + roles_url: Optional[List[str]] = Field( + None, title="URL for the roles of the group", ) - users_url: str = Field( - ..., + users_url: Optional[List[str]] = Field( + None, title="URL for the users of the group", ) +class GroupListResponse(Model): + """Response schema for listing groups.""" + + __root__: List[GroupResponse] + + class GroupCreatePayload(Model): """Payload schema for creating a group.""" name: str = Field( - ..., + Required, title="name of the group", - description="name of the group", ) user_ids: List[DecodedDatabaseIdField] = Field( [], diff --git a/lib/galaxy/webapps/galaxy/api/groups.py b/lib/galaxy/webapps/galaxy/api/groups.py index 6d05c5a658a7..19d050669f53 100644 --- a/lib/galaxy/webapps/galaxy/api/groups.py +++ b/lib/galaxy/webapps/galaxy/api/groups.py @@ -13,8 +13,8 @@ from galaxy.schema.fields import DecodedDatabaseIdField from galaxy.schema.groups import ( GroupCreatePayload, - GroupIndexListResponse, - GroupShowResponse, + GroupListResponse, + GroupResponse, ) from galaxy.webapps.galaxy.api import ( depends, @@ -35,23 +35,25 @@ class FastAPIGroups: "/api/groups", summary="Displays a collection (list) of groups.", require_admin=True, + response_model_exclude_unset=True, ) def index( self, trans: ProvidesAppContext = DependsOnTrans, - ) -> GroupIndexListResponse: + ) -> GroupListResponse: return self.manager.index(trans) @router.post( "/api/groups", summary="Creates a new group.", require_admin=True, + response_model_exclude_unset=True, ) def create( self, trans: ProvidesAppContext = DependsOnTrans, payload: GroupCreatePayload = Body(...), - ) -> GroupIndexListResponse: + ) -> GroupResponse: return self.manager.create(trans, payload) @router.get( @@ -63,18 +65,19 @@ def show( self, trans: ProvidesAppContext = DependsOnTrans, group_id: DecodedDatabaseIdField = Path(...), - ) -> GroupShowResponse: + ) -> GroupResponse: return self.manager.show(trans, group_id) @router.put( "/api/groups/{group_id}", summary="Modifies a group.", require_admin=True, + response_model_exclude_unset=True, ) def update( self, trans: ProvidesAppContext = DependsOnTrans, group_id: DecodedDatabaseIdField = Path(...), payload: GroupCreatePayload = Body(...), - ): - self.manager.update(trans, group_id, payload) + ) -> GroupResponse: + return self.manager.update(trans, group_id, payload) diff --git a/lib/galaxy_test/api/test_groups.py b/lib/galaxy_test/api/test_groups.py index 6e561497a2ce..02bf3bae37ad 100644 --- a/lib/galaxy_test/api/test_groups.py +++ b/lib/galaxy_test/api/test_groups.py @@ -15,7 +15,7 @@ def test_create_valid(self, group_name: Optional[str] = None): payload = self._build_valid_group_payload(group_name) response = self._post("groups", payload, admin=True, json=True) self._assert_status_code_is(response, 200) - group = response.json()[0] # POST /api/groups returns a list + group = response.json() self._assert_valid_group(group) return group From 8982784f9fb0632dcb4b6c3079a6311e15e117e1 Mon Sep 17 00:00:00 2001 From: Arash Date: Tue, 21 Nov 2023 23:45:59 +0100 Subject: [PATCH 10/13] Remove unused import statement in groups.py --- lib/galaxy/managers/groups.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/galaxy/managers/groups.py b/lib/galaxy/managers/groups.py index e71de11eb508..f40c0d7857b3 100644 --- a/lib/galaxy/managers/groups.py +++ b/lib/galaxy/managers/groups.py @@ -1,5 +1,3 @@ -from typing import List - from sqlalchemy import ( false, select, From f372090dd2737f1abc05a48c09bc7db641a75e11 Mon Sep 17 00:00:00 2001 From: Arash Date: Wed, 22 Nov 2023 00:21:25 +0100 Subject: [PATCH 11/13] Edit create method to retun list --- lib/galaxy/managers/groups.py | 2 +- lib/galaxy/webapps/galaxy/api/groups.py | 2 +- lib/galaxy_test/api/test_groups.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/galaxy/managers/groups.py b/lib/galaxy/managers/groups.py index f40c0d7857b3..683d6ec4a8b4 100644 --- a/lib/galaxy/managers/groups.py +++ b/lib/galaxy/managers/groups.py @@ -62,7 +62,7 @@ def create(self, trans: ProvidesAppContext, payload: GroupCreatePayload): encoded_id = DecodedDatabaseIdField.encode(group.id) item = group.to_dict(view="element", value_mapper={"id": DecodedDatabaseIdField.encode}) item["url"] = self._url_for(trans, "create", group_id=encoded_id) - return item + return [item] def show(self, trans: ProvidesAppContext, group_id: int): """ diff --git a/lib/galaxy/webapps/galaxy/api/groups.py b/lib/galaxy/webapps/galaxy/api/groups.py index 19d050669f53..56c92a29e267 100644 --- a/lib/galaxy/webapps/galaxy/api/groups.py +++ b/lib/galaxy/webapps/galaxy/api/groups.py @@ -53,7 +53,7 @@ def create( self, trans: ProvidesAppContext = DependsOnTrans, payload: GroupCreatePayload = Body(...), - ) -> GroupResponse: + ) -> GroupListResponse: return self.manager.create(trans, payload) @router.get( diff --git a/lib/galaxy_test/api/test_groups.py b/lib/galaxy_test/api/test_groups.py index 02bf3bae37ad..6e561497a2ce 100644 --- a/lib/galaxy_test/api/test_groups.py +++ b/lib/galaxy_test/api/test_groups.py @@ -15,7 +15,7 @@ def test_create_valid(self, group_name: Optional[str] = None): payload = self._build_valid_group_payload(group_name) response = self._post("groups", payload, admin=True, json=True) self._assert_status_code_is(response, 200) - group = response.json() + group = response.json()[0] # POST /api/groups returns a list self._assert_valid_group(group) return group From 8e99cddc0efc4e205f7bf0bff8405a200e94faa0 Mon Sep 17 00:00:00 2001 From: Arash Date: Wed, 22 Nov 2023 04:27:05 +0100 Subject: [PATCH 12/13] Update GroupResponse to GroupListResponse in API schema --- client/src/api/schema/schema.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/src/api/schema/schema.ts b/client/src/api/schema/schema.ts index 0a6eb2a5e488..cdd8636a955f 100644 --- a/client/src/api/schema/schema.ts +++ b/client/src/api/schema/schema.ts @@ -11639,7 +11639,7 @@ export interface operations { /** @description Successful Response */ 200: { content: { - "application/json": components["schemas"]["GroupResponse"]; + "application/json": components["schemas"]["GroupListResponse"]; }; }; /** @description Validation Error */ From 361522dc54b4090bfdb079794adc86845740717f Mon Sep 17 00:00:00 2001 From: Arash Date: Wed, 22 Nov 2023 14:14:09 +0100 Subject: [PATCH 13/13] Fix group URLs in GroupsManager and schema Add name for group_users and group_roles endpoints --- client/src/api/schema/schema.ts | 16 ++++++++-------- lib/galaxy/managers/groups.py | 20 ++++++-------------- lib/galaxy/schema/groups.py | 4 ++-- lib/galaxy/webapps/galaxy/api/group_roles.py | 7 ++++++- lib/galaxy/webapps/galaxy/api/group_users.py | 7 ++++++- lib/galaxy/webapps/galaxy/api/groups.py | 1 + 6 files changed, 29 insertions(+), 26 deletions(-) diff --git a/client/src/api/schema/schema.ts b/client/src/api/schema/schema.ts index cdd8636a955f..15a0e8ddfb8b 100644 --- a/client/src/api/schema/schema.ts +++ b/client/src/api/schema/schema.ts @@ -389,13 +389,13 @@ export interface paths { }; "/api/groups/{group_id}": { /** Displays information about a group. */ - get: operations["show_api_groups__group_id__get"]; + get: operations["show_group_api_groups__group_id__get"]; /** Modifies a group. */ put: operations["update_api_groups__group_id__put"]; }; "/api/groups/{group_id}/roles": { /** Displays a collection (list) of groups. */ - get: operations["index_api_groups__group_id__roles_get"]; + get: operations["group_roles_api_groups__group_id__roles_get"]; }; "/api/groups/{group_id}/roles/{role_id}": { /** Displays information about a group role. */ @@ -430,7 +430,7 @@ export interface paths { * @description GET /api/groups/{encoded_group_id}/users * Displays a collection (list) of groups. */ - get: operations["index_api_groups__group_id__users_get"]; + get: operations["group_users_api_groups__group_id__users_get"]; }; "/api/groups/{group_id}/users/{user_id}": { /** @@ -4720,11 +4720,11 @@ export interface components { /** name of the group */ name: string; /** URL for the roles of the group */ - roles_url?: string[]; + roles_url?: string; /** URL for the group */ url: string; /** URL for the users of the group */ - users_url?: string[]; + users_url?: string; }; /** GroupRoleListResponse */ GroupRoleListResponse: components["schemas"]["GroupRoleResponse"][]; @@ -11650,7 +11650,7 @@ export interface operations { }; }; }; - show_api_groups__group_id__get: { + show_group_api_groups__group_id__get: { /** Displays information about a group. */ parameters: { /** @description The user ID that will be used to effectively make this API call. Only admins and designated users can make API calls on behalf of other users. */ @@ -11707,7 +11707,7 @@ export interface operations { }; }; }; - index_api_groups__group_id__roles_get: { + group_roles_api_groups__group_id__roles_get: { /** Displays a collection (list) of groups. */ parameters: { /** @description The user ID that will be used to effectively make this API call. Only admins and designated users can make API calls on behalf of other users. */ @@ -11919,7 +11919,7 @@ export interface operations { }; }; }; - index_api_groups__group_id__users_get: { + group_users_api_groups__group_id__users_get: { /** * Displays a collection (list) of groups. * @description GET /api/groups/{encoded_group_id}/users diff --git a/lib/galaxy/managers/groups.py b/lib/galaxy/managers/groups.py index 683d6ec4a8b4..a0950a4063b8 100644 --- a/lib/galaxy/managers/groups.py +++ b/lib/galaxy/managers/groups.py @@ -35,7 +35,7 @@ def index(self, trans: ProvidesAppContext): for group in get_not_deleted_groups(trans.sa_session): item = group.to_dict(value_mapper={"id": DecodedDatabaseIdField.encode}) encoded_id = DecodedDatabaseIdField.encode(group.id) - item["url"] = self._url_for(trans, "index", group_id=encoded_id) + item["url"] = self._url_for(trans, "show_group", group_id=encoded_id) rval.append(item) return rval @@ -61,7 +61,7 @@ def create(self, trans: ProvidesAppContext, payload: GroupCreatePayload): encoded_id = DecodedDatabaseIdField.encode(group.id) item = group.to_dict(view="element", value_mapper={"id": DecodedDatabaseIdField.encode}) - item["url"] = self._url_for(trans, "create", group_id=encoded_id) + item["url"] = self._url_for(trans, "show_group", group_id=encoded_id) return [item] def show(self, trans: ProvidesAppContext, group_id: int): @@ -71,17 +71,9 @@ def show(self, trans: ProvidesAppContext, group_id: int): encoded_id = DecodedDatabaseIdField.encode(group_id) group = self._get_group(trans.sa_session, group_id) item = group.to_dict(view="element", value_mapper={"id": DecodedDatabaseIdField.encode}) - item["url"] = self._url_for(trans, "show", group_id=encoded_id) - encoded_user_id = [DecodedDatabaseIdField.encode(gu.user.id) for gu in group.users] - encoded_role_id = [DecodedDatabaseIdField.encode(gr.role.id) for gr in group.roles] - item["users_url"] = [ - self._url_for(trans, "group_user", group_id=encoded_id, user_id=group_user) - for group_user in encoded_user_id - ] - item["roles_url"] = [ - self._url_for(trans, "group_role", group_id=encoded_id, role_id=group_role) - for group_role in encoded_role_id - ] + item["url"] = self._url_for(trans, "show_group", group_id=encoded_id) + item["users_url"] = self._url_for(trans, "group_users", group_id=encoded_id) + item["roles_url"] = self._url_for(trans, "group_roles", group_id=encoded_id) return item def update(self, trans: ProvidesAppContext, group_id: int, payload: GroupCreatePayload): @@ -107,7 +99,7 @@ def update(self, trans: ProvidesAppContext, group_id: int, payload: GroupCreateP encoded_id = DecodedDatabaseIdField.encode(group.id) item = group.to_dict(view="element", value_mapper={"id": DecodedDatabaseIdField.encode}) - item["url"] = self._url_for(trans, "update", group_id=encoded_id) + item["url"] = self._url_for(trans, "show_group", group_id=encoded_id) return item def _url_for(self, trans, name, **kwargs): diff --git a/lib/galaxy/schema/groups.py b/lib/galaxy/schema/groups.py index 4b0b26b3ed22..b6de76782b61 100644 --- a/lib/galaxy/schema/groups.py +++ b/lib/galaxy/schema/groups.py @@ -35,11 +35,11 @@ class GroupResponse(Model): Required, title="URL for the group", ) - roles_url: Optional[List[str]] = Field( + roles_url: Optional[str] = Field( None, title="URL for the roles of the group", ) - users_url: Optional[List[str]] = Field( + users_url: Optional[str] = Field( None, title="URL for the users of the group", ) diff --git a/lib/galaxy/webapps/galaxy/api/group_roles.py b/lib/galaxy/webapps/galaxy/api/group_roles.py index 21eca4638f02..7ebe74725798 100644 --- a/lib/galaxy/webapps/galaxy/api/group_roles.py +++ b/lib/galaxy/webapps/galaxy/api/group_roles.py @@ -39,7 +39,12 @@ def group_role_to_model(trans, group_id: int, role) -> GroupRoleResponse: class FastAPIGroupRoles: manager: GroupRolesManager = depends(GroupRolesManager) - @router.get("/api/groups/{group_id}/roles", require_admin=True, summary="Displays a collection (list) of groups.") + @router.get( + "/api/groups/{group_id}/roles", + require_admin=True, + summary="Displays a collection (list) of groups.", + name="group_roles", + ) def index( self, trans: ProvidesAppContext = DependsOnTrans, group_id: DecodedDatabaseIdField = GroupIDParam ) -> GroupRoleListResponse: diff --git a/lib/galaxy/webapps/galaxy/api/group_users.py b/lib/galaxy/webapps/galaxy/api/group_users.py index a3d9541e6e01..5fb77919771c 100644 --- a/lib/galaxy/webapps/galaxy/api/group_users.py +++ b/lib/galaxy/webapps/galaxy/api/group_users.py @@ -38,7 +38,12 @@ def group_user_to_model(trans, group_id, user) -> GroupUserResponse: class FastAPIGroupUsers: manager: GroupUsersManager = depends(GroupUsersManager) - @router.get("/api/groups/{group_id}/users", require_admin=True, summary="Displays a collection (list) of groups.") + @router.get( + "/api/groups/{group_id}/users", + require_admin=True, + summary="Displays a collection (list) of groups.", + name="group_users", + ) def index( self, trans: ProvidesAppContext = DependsOnTrans, group_id: DecodedDatabaseIdField = GroupIDParam ) -> GroupUserListResponse: diff --git a/lib/galaxy/webapps/galaxy/api/groups.py b/lib/galaxy/webapps/galaxy/api/groups.py index 56c92a29e267..2b4c7e1ca1d5 100644 --- a/lib/galaxy/webapps/galaxy/api/groups.py +++ b/lib/galaxy/webapps/galaxy/api/groups.py @@ -60,6 +60,7 @@ def create( "/api/groups/{group_id}", summary="Displays information about a group.", require_admin=True, + name="show_group", ) def show( self,