diff --git a/lib/galaxy/schema/schema.py b/lib/galaxy/schema/schema.py index 8b75e50f5a69..9cdf22507164 100644 --- a/lib/galaxy/schema/schema.py +++ b/lib/galaxy/schema/schema.py @@ -3229,30 +3229,65 @@ class DatasetAssociationRoles(Model): ) -class UpdateDatasetPermissionsPayload(Model): +class UpdateDatasetPermissionsPayloadBase(Model): action: Optional[DatasetPermissionAction] = Field( DatasetPermissionAction.set_permissions, title="Action", description="Indicates what action should be performed on the dataset.", ) - access_ids: Optional[RoleIdList] = Field( - [], - alias="access_ids[]", # Added for backward compatibility but it looks really ugly... + + +AccessIdsField = Annotated[ + Optional[RoleIdList], + Field( + default=None, title="Access IDs", description="A list of role encoded IDs defining roles that should have access permission on the dataset.", - ) - manage_ids: Optional[RoleIdList] = Field( - [], - alias="manage_ids[]", + ), +] + +ManageIdsField = Annotated[ + Optional[RoleIdList], + Field( + default=None, title="Manage IDs", description="A list of role encoded IDs defining roles that should have manage permission on the dataset.", - ) - modify_ids: Optional[RoleIdList] = Field( - [], - alias="modify_ids[]", + ), +] + +ModifyIdsField = Annotated[ + Optional[RoleIdList], + Field( + default=None, title="Modify IDs", description="A list of role encoded IDs defining roles that should have modify permission on the dataset.", - ) + ), +] + + +class UpdateDatasetPermissionsPayload(UpdateDatasetPermissionsPayloadBase): + access_ids: Annotated[Optional[RoleIdList], Field(default=None, alias="access_ids[]")] = None + manage_ids: Annotated[Optional[RoleIdList], Field(default=None, alias="manage_ids[]")] = None + modify_ids: Annotated[Optional[RoleIdList], Field(default=None, alias="modify_ids[]")] = None + + +class UpdateDatasetPermissionsPayloadAliasB(UpdateDatasetPermissionsPayloadBase): + access: AccessIdsField = None + manage: ManageIdsField = None + modify: ModifyIdsField = None + + +class UpdateDatasetPermissionsPayloadAliasC(UpdateDatasetPermissionsPayloadBase): + access_ids: AccessIdsField = None + manage_ids: ManageIdsField = None + modify_ids: ModifyIdsField = None + + +UpdateDatasetPermissionsPayloadAliases = Union[ + UpdateDatasetPermissionsPayload, + UpdateDatasetPermissionsPayloadAliasB, + UpdateDatasetPermissionsPayloadAliasC, +] @partial_model() diff --git a/lib/galaxy/webapps/galaxy/api/common.py b/lib/galaxy/webapps/galaxy/api/common.py index 4ba9cedbe53e..5ade80943bf8 100644 --- a/lib/galaxy/webapps/galaxy/api/common.py +++ b/lib/galaxy/webapps/galaxy/api/common.py @@ -2,13 +2,13 @@ from typing import ( Any, - Dict, List, Optional, Set, ) from fastapi import ( + Body, Path, Query, Request, @@ -21,7 +21,10 @@ ValueFilterQueryParams, ) from galaxy.schema.fields import DecodedDatabaseIdField -from galaxy.schema.schema import UpdateDatasetPermissionsPayload +from galaxy.schema.schema import ( + UpdateDatasetPermissionsPayload, + UpdateDatasetPermissionsPayloadAliases, +) from galaxy.util import listify HistoryIDPathParam = Annotated[ @@ -66,6 +69,20 @@ Path(..., title="Role ID", description="The ID of the role."), ] +UpdateDatasetPermissionsBody = Annotated[ + UpdateDatasetPermissionsPayloadAliases, + Body( + ..., + examples=[ + UpdateDatasetPermissionsPayload( + action="set_permissions", + access_ids=[], + manage_ids=[], + modify_ids=[], + ).model_dump() + ], + ), +] LibraryIdPathParam = Annotated[ DecodedDatabaseIdField, @@ -194,16 +211,23 @@ def get_filter_query_params( ) -def get_update_permission_payload(payload: Dict[str, Any]) -> UpdateDatasetPermissionsPayload: - """Converts the generic payload dictionary into a UpdateDatasetPermissionsPayload model with custom parsing. - This is an attempt on supporting multiple aliases for the permissions params.""" - # There are several allowed names for the same role list parameter, i.e.: `access`, `access_ids`, `access_ids[]` - # The `access_ids[]` name is not pydantic friendly, so this will be modelled as an alias but we can only set one alias +def normalize_permission_payload( + payload_aliases: UpdateDatasetPermissionsPayloadAliases, +) -> UpdateDatasetPermissionsPayload: + """Normalize the payload by choosing the first non-None value for each field. + + This is an attempt on supporting multiple aliases for the permissions params. + There are several allowed names for the same role list parameter, i.e.: `access`, `access_ids`, `access_ids[]` + """ # TODO: Maybe we should choose only one way/naming and deprecate the others? - payload["access_ids"] = payload.get("access_ids[]") or payload.get("access") - payload["manage_ids"] = payload.get("manage_ids[]") or payload.get("manage") - payload["modify_ids"] = payload.get("modify_ids[]") or payload.get("modify") - update_payload = UpdateDatasetPermissionsPayload(**payload) + payload = payload_aliases.model_dump() + normalized_payload = { + "action": payload.get("action"), + "access_ids": payload.get("access_ids") or payload.get("access_ids[]") or payload.get("access"), + "manage_ids": payload.get("manage_ids") or payload.get("manage_ids[]") or payload.get("manage"), + "modify_ids": payload.get("modify_ids") or payload.get("modify_ids[]") or payload.get("modify"), + } + update_payload = UpdateDatasetPermissionsPayload.model_construct(**normalized_payload) return update_payload diff --git a/lib/galaxy/webapps/galaxy/api/datasets.py b/lib/galaxy/webapps/galaxy/api/datasets.py index 53006b4ee8c1..fdc790d9fd96 100644 --- a/lib/galaxy/webapps/galaxy/api/datasets.py +++ b/lib/galaxy/webapps/galaxy/api/datasets.py @@ -9,9 +9,7 @@ StringIO, ) from typing import ( - Any, cast, - Dict, List, Optional, ) @@ -40,7 +38,6 @@ AsyncTaskResultSummary, DatasetAssociationRoles, DatasetSourceType, - UpdateDatasetPermissionsPayload, ) from galaxy.util.zipstream import ZipstreamWrapper from galaxy.webapps.base.api import GalaxyFileResponse @@ -52,10 +49,11 @@ from galaxy.webapps.galaxy.api.common import ( get_filter_query_params, get_query_parameters_from_request_excluding, - get_update_permission_payload, HistoryDatasetIDPathParam, HistoryIDPathParam, + normalize_permission_payload, query_serialization_params, + UpdateDatasetPermissionsBody, ) from galaxy.webapps.galaxy.services.datasets import ( ComputeDatasetHashPayload, @@ -235,15 +233,11 @@ def converted( def update_permissions( self, dataset_id: HistoryDatasetIDPathParam, + payload: UpdateDatasetPermissionsBody, trans=DependsOnTrans, - # Using a generic Dict here as an attempt on supporting multiple aliases for the permissions params. - payload: Dict[str, Any] = Body( - default=..., - examples=[UpdateDatasetPermissionsPayload()], - ), ) -> DatasetAssociationRoles: """Set permissions of the given history dataset to the given role ids.""" - update_payload = get_update_permission_payload(payload) + update_payload = normalize_permission_payload(payload) return self.service.update_permissions(trans, dataset_id, update_payload) @router.get( diff --git a/lib/galaxy/webapps/galaxy/api/history_contents.py b/lib/galaxy/webapps/galaxy/api/history_contents.py index 052ea3a2e465..39d7d64c8e7d 100644 --- a/lib/galaxy/webapps/galaxy/api/history_contents.py +++ b/lib/galaxy/webapps/galaxy/api/history_contents.py @@ -4,8 +4,6 @@ import logging from typing import ( - Any, - Dict, List, Optional, Union, @@ -51,7 +49,6 @@ MaterializeDatasetInstanceAPIRequest, MaterializeDatasetInstanceRequest, StoreExportPayload, - UpdateDatasetPermissionsPayload, UpdateHistoryContentsBatchPayload, UpdateHistoryContentsPayload, WriteStoreToPayload, @@ -64,12 +61,13 @@ ) from galaxy.webapps.galaxy.api.common import ( get_filter_query_params, - get_update_permission_payload, get_value_filter_query_params, HistoryHDCAIDPathParam, HistoryIDPathParam, HistoryItemIDPathParam, + normalize_permission_payload, query_serialization_params, + UpdateDatasetPermissionsBody, ) from galaxy.webapps.galaxy.services.history_contents import ( CreateHistoryContentFromStore, @@ -727,15 +725,11 @@ def update_permissions( self, history_id: HistoryIDPathParam, dataset_id: HistoryItemIDPathParam, + payload: UpdateDatasetPermissionsBody, trans: ProvidesHistoryContext = DependsOnTrans, - # Using a generic Dict here as an attempt on supporting multiple aliases for the permissions params. - payload: Dict[str, Any] = Body( - default=..., - examples=[UpdateDatasetPermissionsPayload().model_dump()], - ), ) -> DatasetAssociationRoles: """Set permissions of the given history dataset to the given role ids.""" - update_payload = get_update_permission_payload(payload) + update_payload = normalize_permission_payload(payload) return self.service.update_permissions(trans, dataset_id, update_payload) @router.put(