From 0f26838119ecb76294f972b37f54796b6c790c0f Mon Sep 17 00:00:00 2001 From: Arash Date: Fri, 20 Sep 2024 16:12:24 +0200 Subject: [PATCH] Refactor library content routes and controllers --- lib/galaxy/schema/library_contents.py | 40 ++-- .../webapps/galaxy/api/library_contents.py | 17 +- .../galaxy/services/library_contents.py | 174 ++++++++++-------- 3 files changed, 135 insertions(+), 96 deletions(-) diff --git a/lib/galaxy/schema/library_contents.py b/lib/galaxy/schema/library_contents.py index d02c0350989b..d0f423c9dd9e 100644 --- a/lib/galaxy/schema/library_contents.py +++ b/lib/galaxy/schema/library_contents.py @@ -125,11 +125,11 @@ class LibraryContentsFileCreatePayload(LibraryContentsCreatePayload): class LibraryContentsFolderCreatePayload(LibraryContentsCreatePayload): name: Optional[str] = Field( "", - title="(only if create_type is 'folder') name of the folder to create", + title="name of the folder to create", ) description: Optional[str] = Field( "", - title="(only if create_type is 'folder') description of the folder to create", + title="description of the folder to create", ) @@ -211,15 +211,15 @@ class LibraryContentsShowDatasetResponse(LibraryContentsShowResponse): folder_id: EncodedLibraryFolderDatabaseIdField state: str file_name: str - created_from_basename: str + created_from_basename: Optional[str] uploaded_by: str message: Optional[str] date_uploaded: str file_size: int file_ext: str data_type: str - misc_info: str - misc_blurb: str + misc_info: Optional[str] + misc_blurb: Optional[str] peek: Optional[str] uuid: str metadata_dbkey: str @@ -237,28 +237,34 @@ class LibraryContentsCreateFolderListResponse(RootModel): root: List[LibraryContentsCreateFolderResponse] -class LibraryContentsCreateDatasetResponse(Model): - id: EncodedDatabaseIdField +class LibraryContentsCreateDatasetResponseBase(Model): + id: str # should change to EncodedDatabaseIdField latter hda_ldda: str model_class: str name: str deleted: bool visible: bool state: str - library_dataset_id: EncodedDatabaseIdField + library_dataset_id: str # should change to EncodedDatabaseIdField latter file_size: int file_name: str update_time: str file_ext: str data_type: str genome_build: str - misc_info: str - misc_blurb: str - created_from_basename: str + misc_info: Optional[str] + misc_blurb: Optional[str] + created_from_basename: Optional[str] uuid: str - parent_library_id: EncodedDatabaseIdField + parent_library_id: str # should change to EncodedDatabaseIdField latter metadata_dbkey: str - metadata_data_lines: int + + +class LibraryContentsCreateDatasetResponse(LibraryContentsCreateDatasetResponseBase): + metadata_data_lines: Optional[int] + + +class LibraryContentsCreateDatasetExtendedResponse(LibraryContentsCreateDatasetResponse): metadata_comment_lines: Union[str, int] metadata_columns: int metadata_column_types: List[str] @@ -267,8 +273,12 @@ class LibraryContentsCreateDatasetResponse(Model): class LibraryContentsCreateDatasetListResponse(RootModel): - root: List[LibraryContentsCreateDatasetResponse] + root: List[LibraryContentsCreateDatasetResponseBase] class LibraryContentsDeleteResponse(Model): - pass + id: EncodedDatabaseIdField + deleted: bool + +class LibraryContentsPurgedResponse(LibraryContentsDeleteResponse): + purged: bool \ No newline at end of file diff --git a/lib/galaxy/webapps/galaxy/api/library_contents.py b/lib/galaxy/webapps/galaxy/api/library_contents.py index 24d122e70775..3c5406faee43 100644 --- a/lib/galaxy/webapps/galaxy/api/library_contents.py +++ b/lib/galaxy/webapps/galaxy/api/library_contents.py @@ -3,7 +3,12 @@ """ import logging -from typing import Union +from typing import ( + Optional, + Union, +) + +from fastapi import Body from galaxy.managers.context import ( ProvidesHistoryContext, @@ -12,6 +17,7 @@ from galaxy.schema.fields import DecodedDatabaseIdField from galaxy.schema.library_contents import ( LibraryContentsCollectionCreatePayload, + LibraryContentsCreateDatasetExtendedResponse, LibraryContentsCreateDatasetListResponse, LibraryContentsCreateDatasetResponse, LibraryContentsCreateFolderListResponse, @@ -72,12 +78,15 @@ def show( def create( self, library_id: DecodedDatabaseIdField, - payload: Union[LibraryContentsFolderCreatePayload, LibraryContentsFileCreatePayload, LibraryContentsCollectionCreatePayload], + payload: Union[ + LibraryContentsFolderCreatePayload, LibraryContentsFileCreatePayload, LibraryContentsCollectionCreatePayload + ], trans: ProvidesHistoryContext = DependsOnTrans, ) -> Union[ LibraryContentsCreateFolderListResponse, LibraryContentsCreateDatasetResponse, LibraryContentsCreateDatasetListResponse, + LibraryContentsCreateDatasetExtendedResponse, ]: return self.service.create(trans, library_id, payload) @@ -103,7 +112,9 @@ def delete( self, library_id: DecodedDatabaseIdField, id: DecodedDatabaseIdField, - payload: LibraryContentsDeletePayload = LibraryContentsDeletePayload(), + payload: Optional[LibraryContentsDeletePayload] = Body(None), trans: ProvidesHistoryContext = DependsOnTrans, ): + if payload is None: + payload = LibraryContentsDeletePayload() return self.service.delete(trans, id, payload) diff --git a/lib/galaxy/webapps/galaxy/services/library_contents.py b/lib/galaxy/webapps/galaxy/services/library_contents.py index 0f9b1f893c4d..ccfc3559e1fc 100644 --- a/lib/galaxy/webapps/galaxy/services/library_contents.py +++ b/lib/galaxy/webapps/galaxy/services/library_contents.py @@ -24,7 +24,11 @@ validate_path_upload, validate_server_directory_upload, ) -from galaxy.managers.collections_util import dictify_dataset_collection_instance +from galaxy.managers.collections import DatasetCollectionManager +from galaxy.managers.collections_util import ( + api_payload_to_create_params, + dictify_dataset_collection_instance, +) from galaxy.managers.context import ( ProvidesHistoryContext, ProvidesUserContext, @@ -44,9 +48,12 @@ ) from galaxy.schema.library_contents import ( LibraryContentsCollectionCreatePayload, + LibraryContentsCreateDatasetExtendedResponse, LibraryContentsCreateDatasetListResponse, LibraryContentsCreateDatasetResponse, LibraryContentsCreateFolderListResponse, + LibraryContentsCreateFolderResponse, + LibraryContentsCreatePayload, LibraryContentsDeletePayload, LibraryContentsDeleteResponse, LibraryContentsFileCreatePayload, @@ -85,9 +92,15 @@ class LibraryContentsService(ServiceBase, LibraryActions, UsesLibraryMixinItems, Interface/service shared by controllers for interacting with the contents of a library contents. """ - def __init__(self, security: IdEncodingHelper, hda_manager: HDAManager): + def __init__( + self, + security: IdEncodingHelper, + hda_manager: HDAManager, + collection_manager: DatasetCollectionManager, + ): super().__init__(security) self.hda_manager = hda_manager + self.collection_manager = collection_manager def index( self, @@ -149,6 +162,7 @@ def create( LibraryContentsCreateFolderListResponse, LibraryContentsCreateDatasetResponse, LibraryContentsCreateDatasetListResponse, + LibraryContentsCreateDatasetExtendedResponse, ]: """Create a new library file or folder.""" if trans.user_is_bootstrap_admin: @@ -161,53 +175,28 @@ def create( # we'll need the id and any message to attach, then branch to that private function if payload.create_type == "file": if payload.from_hda_id: - return self._copy_hda_to_library_folder( + rval = self._copy_hda_to_library_folder( trans, self.hda_manager, payload.from_hda_id, payload.folder_id, payload.ldda_message ) + if "metadata_comment_lines" in rval: + return LibraryContentsCreateDatasetExtendedResponse(**rval) + else: + return LibraryContentsCreateDatasetResponse(**rval) elif payload.from_hdca_id: - return self._copy_hdca_to_library_folder( + rval = self._copy_hdca_to_library_folder( trans, self.hda_manager, payload.from_hdca_id, payload.folder_id, payload.ldda_message ) - else: - raise exceptions.RequestParameterInvalidException("Invalid create request") + return LibraryContentsCreateDatasetListResponse(root=rval) # Now create the desired content object, either file or folder. if payload.create_type == "file": - output = self._upload_library_dataset( - trans, payload.folder_id, cast(LibraryContentsFileCreatePayload, payload) - ) + return self._upload_library_dataset(trans, cast(LibraryContentsFileCreatePayload, payload), library_id) elif payload.create_type == "folder": - output = self._create_folder(trans, payload.folder_id, cast(LibraryContentsFolderCreatePayload, payload)) + return self._create_folder(trans, cast(LibraryContentsFolderCreatePayload, payload), library_id) elif payload.create_type == "collection": - # Not delegating to library_common, so need to check access to parent - # folder here. - payload = cast(LibraryContentsCollectionCreatePayload, payload) - self.check_user_can_add_to_library_item(trans, parent, check_accessible=True) - create_params = dict( - collection_type=payload.collection_type, - element_identifiers=payload.element_identifiers, - name=payload.name or None, - hide_source_items=payload.hide_source_items or False, - copy_elements=payload.copy_elements or False, - parent=parent, - ) - dataset_collection_instance = trans.app.dataset_collection_manager.create(**create_params) - dataset_collection = dictify_dataset_collection_instance( - dataset_collection_instance, security=trans.security, url_builder=trans.url_builder, parent=parent - ) - return [dataset_collection] - # return LibraryContentsCreateListResponse(root=dataset_collection) - rval = [] - for v in output.values(): - if payload.extended_metadata is not None: - # If there is extended metadata, store it, attach it to the dataset, and index it - self.create_extended_metadata(trans, payload.extended_metadata) - if isinstance(v, trans.app.model.LibraryDatasetDatasetAssociation): - v = v.library_dataset - url = self._url_for(trans, library_id, v.id, payload.create_type) - rval.append(dict(id=v.id, name=v.name, url=url)) - return rval - # return LibraryContentsCreateListResponse(root=rval) + return self._create_collection(trans, cast(LibraryContentsCollectionCreatePayload, payload), parent) + else: + raise exceptions.RequestParameterInvalidException("Invalid create_type specified.") def update( self, @@ -305,39 +294,6 @@ def _url_for( encoded_id = f"F{encoded_id}" return trans.url_builder("library_content", library_id=library_id, id=encoded_id) if trans.url_builder else None - def _upload_library_dataset( - self, - trans: ProvidesHistoryContext, - folder_id: LibraryFolderDatabaseIdField, - payload: LibraryContentsFileCreatePayload, - ) -> Dict: - replace_dataset: Optional[LibraryDataset] = None - is_admin = trans.user_is_admin - current_user_roles = trans.get_current_user_roles() - folder = trans.sa_session.get(LibraryFolder, folder_id) - if not folder: - raise exceptions.RequestParameterInvalidException("Invalid folder id specified.") - self._check_access(trans, is_admin, folder, current_user_roles) - self._check_add(trans, is_admin, folder, current_user_roles) - library = folder.parent_library - if payload.upload_option == "upload_paths": - validate_path_upload(trans) # Duplicate check made in _upload_dataset. - elif payload.roles: - # Check to see if the user selected roles to associate with the DATASET_ACCESS permission - # on the dataset that would cause accessibility issues. - vars = dict(DATASET_ACCESS_in=payload.roles) - permissions, in_roles, error, message = trans.app.security_agent.derive_roles_from_access( - trans, library.id, "api", library=True, **vars - ) - if error: - raise exceptions.RequestParameterInvalidException(message) - created_outputs_dict = self._upload_dataset( - trans, payload=payload, folder_id=folder.id, replace_dataset=replace_dataset - ) - if not created_outputs_dict: - raise exceptions.RequestParameterInvalidException("Upload failed") - return created_outputs_dict - def _traverse(self, trans: ProvidesUserContext, folder, current_user_roles): admin = trans.user_is_admin rval = [] @@ -362,12 +318,36 @@ def _traverse(self, trans: ProvidesUserContext, folder, current_user_roles): rval.append(ld) return rval + def _upload_library_dataset( + self, + trans: ProvidesHistoryContext, + payload: LibraryContentsFileCreatePayload, + library_id: DecodedDatabaseIdField, + ) -> LibraryContentsCreateFolderListResponse: + is_admin = trans.user_is_admin + current_user_roles = trans.get_current_user_roles() + folder = trans.sa_session.get(LibraryFolder, payload.folder_id) + if not folder: + raise exceptions.RequestParameterInvalidException("Invalid folder id specified.") + self._check_access(trans, is_admin, folder, current_user_roles) + self._check_add(trans, is_admin, folder, current_user_roles) + if payload.roles: + # Check to see if the user selected roles to associate with the DATASET_ACCESS permission + # on the dataset that would cause accessibility issues. + vars = dict(DATASET_ACCESS_in=payload.roles) + permissions, in_roles, error, message = trans.app.security_agent.derive_roles_from_access( + trans, folder.parent_library.id, "api", library=True, **vars + ) + if error: + raise exceptions.RequestParameterInvalidException(message) + created_outputs_dict = self._upload_dataset(trans, payload=payload, folder_id=folder.id) + return self._convert_output_to_rval(trans, payload, created_outputs_dict, library_id) + def _upload_dataset( self, trans: ProvidesHistoryContext, payload: LibraryContentsFileCreatePayload, folder_id: LibraryFolderDatabaseIdField, - replace_dataset: Optional[LibraryDataset], ) -> Dict[str, List]: # Set up the traditional tool state/params cntrller = "api" @@ -393,7 +373,7 @@ def _upload_dataset( try: # FIXME: instead of passing params here ( which have been processed by util.Params(), the original payload # should be passed so that complex objects that may have been included in the initial request remain. - library_bunch = upload_common.handle_library_params(trans, payload.model_dump(), folder_id, replace_dataset) + library_bunch = upload_common.handle_library_params(trans, payload.model_dump(), folder_id, None) except Exception: raise exceptions.InvalidFileFormatError("Invalid folder specified") # Proceed with (mostly) regular upload processing if we're still errorless @@ -421,6 +401,8 @@ def _upload_dataset( trans, tool_params, tool, json_file_path, data_list, folder=library_bunch.folder, job_params=job_params ) trans.app.job_manager.enqueue(job, tool=tool) + if not output: + raise exceptions.RequestParameterInvalidException("Upload failed") return output def _get_server_dir_uploaded_datasets( @@ -484,12 +466,12 @@ def _get_server_dir_files( def _create_folder( self, trans: ProvidesUserContext, - parent_id: LibraryFolderDatabaseIdField, payload: LibraryContentsFolderCreatePayload, - ) -> Dict[str, LibraryFolder]: + library_id: DecodedDatabaseIdField, + ) -> LibraryContentsCreateFolderListResponse: is_admin = trans.user_is_admin current_user_roles = trans.get_current_user_roles() - parent_folder = trans.sa_session.get(LibraryFolder, parent_id) + parent_folder = trans.sa_session.get(LibraryFolder, payload.folder_id) if not parent_folder: raise exceptions.RequestParameterInvalidException("Invalid folder id specified.") # Check the library which actually contains the user-supplied parent folder, not the user-supplied @@ -507,7 +489,43 @@ def _create_folder( trans.sa_session.commit() # New folders default to having the same permissions as their parent folder trans.app.security_agent.copy_library_permissions(trans, parent_folder, new_folder) - return dict(created=new_folder) + new_folder_dict = dict(created=new_folder) + return self._convert_output_to_rval(trans, payload, new_folder_dict, library_id) + + def _convert_output_to_rval( + self, + trans: ProvidesUserContext, + payload: LibraryContentsCreatePayload, + output: Dict, + library_id: DecodedDatabaseIdField, + ) -> LibraryContentsCreateFolderListResponse: + rval = [] + for v in output.values(): + if payload.extended_metadata is not None: + # If there is extended metadata, store it, attach it to the dataset, and index it + self.create_extended_metadata(trans, payload.extended_metadata) + if isinstance(v, trans.app.model.LibraryDatasetDatasetAssociation): + v = v.library_dataset + url = self._url_for(trans, library_id, v.id, payload.create_type) + rval.append(LibraryContentsCreateFolderResponse(id=v.id, name=v.name, url=url)) + return LibraryContentsCreateFolderListResponse(root=rval) + + def _create_collection( + self, + trans: ProvidesUserContext, + payload: LibraryContentsCollectionCreatePayload, + parent: LibraryFolder, + ) -> LibraryContentsCreateDatasetListResponse: + # Not delegating to library_common, so need to check access to parent folder here. + self.check_user_can_add_to_library_item(trans, parent, check_accessible=True) + create_params = api_payload_to_create_params(payload.model_dump()) + create_params["trans"] = trans + create_params["parent"] = parent + dataset_collection_instance = self.collection_manager.create(**create_params) + dataset_collection = dictify_dataset_collection_instance( + dataset_collection_instance, security=trans.security, url_builder=trans.url_builder, parent=parent + ) + return LibraryContentsCreateDatasetListResponse(root=[dataset_collection]) def _check_access( self,