Merge pull request galaxyproject#18563 from davelopez/improve_dataset…

…s_permissions_api_typing

Improve datasets permissions API schema typing

client/src/api/schema/schema.ts

@@ -3980,6 +3980,11 @@ export interface components {
              */
             name: string;
         };
+        /**
+         * DatasetPermissionAction
+         * @enum {string}
+         */
+        DatasetPermissionAction: "set_permissions" | "make_private" | "remove_restrictions";
         /**
          * DatasetPermissions
          * @description Role-based permissions for accessing and managing a dataset.
@@ -12272,6 +12277,69 @@ export interface components {
             /** Creator */
             creator?: unknown;
         };
+        /** UpdateDatasetPermissionsPayload */
+        UpdateDatasetPermissionsPayload: {
+            /** Access Ids[] */
+            "access_ids[]"?: string[] | string | null;
+            /**
+             * Action
+             * @description Indicates what action should be performed on the dataset.
+             * @default set_permissions
+             */
+            action?: components["schemas"]["DatasetPermissionAction"] | null;
+            /** Manage Ids[] */
+            "manage_ids[]"?: string[] | string | null;
+            /** Modify Ids[] */
+            "modify_ids[]"?: string[] | string | null;
+        };
+        /** UpdateDatasetPermissionsPayloadAliasB */
+        UpdateDatasetPermissionsPayloadAliasB: {
+            /**
+             * Access IDs
+             * @description A list of role encoded IDs defining roles that should have access permission on the dataset.
+             */
+            access?: string[] | string | null;
+            /**
+             * Action
+             * @description Indicates what action should be performed on the dataset.
+             * @default set_permissions
+             */
+            action?: components["schemas"]["DatasetPermissionAction"] | null;
+            /**
+             * Manage IDs
+             * @description A list of role encoded IDs defining roles that should have manage permission on the dataset.
+             */
+            manage?: string[] | string | null;
+            /**
+             * Modify IDs
+             * @description A list of role encoded IDs defining roles that should have modify permission on the dataset.
+             */
+            modify?: string[] | string | null;
+        };
+        /** UpdateDatasetPermissionsPayloadAliasC */
+        UpdateDatasetPermissionsPayloadAliasC: {
+            /**
+             * Access IDs
+             * @description A list of role encoded IDs defining roles that should have access permission on the dataset.
+             */
+            access_ids?: string[] | string | null;
+            /**
+             * Action
+             * @description Indicates what action should be performed on the dataset.
+             * @default set_permissions
+             */
+            action?: components["schemas"]["DatasetPermissionAction"] | null;
+            /**
+             * Manage IDs
+             * @description A list of role encoded IDs defining roles that should have manage permission on the dataset.
+             */
+            manage_ids?: string[] | string | null;
+            /**
+             * Modify IDs
+             * @description A list of role encoded IDs defining roles that should have modify permission on the dataset.
+             */
+            modify_ids?: string[] | string | null;
+        };
         /**
          * UpdateHistoryContentsBatchPayload
          * @description Contains property values that will be updated for all the history `items` provided.
@@ -14653,7 +14721,10 @@ export interface operations {
         };
         requestBody: {
             content: {
-                "application/json": Record<string, never>;
+                "application/json":
+                    | components["schemas"]["UpdateDatasetPermissionsPayload"]
+                    | components["schemas"]["UpdateDatasetPermissionsPayloadAliasB"]
+                    | components["schemas"]["UpdateDatasetPermissionsPayloadAliasC"];
             };
         };
         responses: {
@@ -18165,7 +18236,10 @@ export interface operations {
         };
         requestBody: {
             content: {
-                "application/json": Record<string, never>;
+                "application/json":
+                    | components["schemas"]["UpdateDatasetPermissionsPayload"]
+                    | components["schemas"]["UpdateDatasetPermissionsPayloadAliasB"]
+                    | components["schemas"]["UpdateDatasetPermissionsPayloadAliasC"];
             };
         };
         responses: {

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()

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
 
 

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(

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(