From 98676ede2e5dd9ce61096c0e4e0f69c8b08b529e Mon Sep 17 00:00:00 2001 From: Martin Cech Date: Tue, 11 Oct 2022 15:07:47 -0700 Subject: [PATCH 01/15] refactor workflow API delete method use the manager deletable mixin --- lib/galaxy/managers/workflows.py | 7 ++++++- lib/galaxy/webapps/galaxy/api/workflows.py | 19 +++++-------------- 2 files changed, 11 insertions(+), 15 deletions(-) diff --git a/lib/galaxy/managers/workflows.py b/lib/galaxy/managers/workflows.py index 71e765457b4..a13e531e435 100644 --- a/lib/galaxy/managers/workflows.py +++ b/lib/galaxy/managers/workflows.py @@ -42,6 +42,7 @@ from galaxy import ( util, ) from galaxy.job_execution.actions.post import ActionBox +from galaxy.managers import deletable from galaxy.managers import sharable from galaxy.managers.base import decode_id from galaxy.managers.context import ProvidesUserContext @@ -107,7 +108,7 @@ INDEX_SEARCH_FILTERS = { } -class WorkflowsManager(sharable.SharableModelManager): +class WorkflowsManager(sharable.SharableModelManager, deletable.DeletableManagerMixin): """Handle CRUD type operations related to workflows. More interesting stuff regarding workflow execution, step sorting, etc... can be found in the galaxy.workflow module. @@ -485,6 +486,10 @@ class WorkflowsManager(sharable.SharableModelManager): if self.check_security(trans, inv, check_ownership=True, check_accessible=False) ] return invocations, total_matches + + def delete(self, stored_workflow, flush=True): + """Mark the given workflow deleted.""" + super().delete(stored_workflow, flush=flush) CreatedWorkflow = namedtuple("CreatedWorkflow", ["stored_workflow", "workflow", "missing_tools"]) diff --git a/lib/galaxy/webapps/galaxy/api/workflows.py b/lib/galaxy/webapps/galaxy/api/workflows.py index 00a823de8b9..816e1304b8c 100644 --- a/lib/galaxy/webapps/galaxy/api/workflows.py +++ b/lib/galaxy/webapps/galaxy/api/workflows.py @@ -461,23 +461,14 @@ class WorkflowsAPIController( def delete(self, trans: ProvidesUserContext, id, **kwd): """ DELETE /api/workflows/{encoded_workflow_id} - Deletes a specified workflow - Author: rpark - - copied from galaxy.web.controllers.workflows.py (delete) + Delete a specified workflow """ - stored_workflow = self.__get_stored_workflow(trans, id, **kwd) - + workflow_to_delete = self.__get_stored_workflow(trans, id, **kwd) # check to see if user has permissions to selected workflow - if stored_workflow.user != trans.user and not trans.user_is_admin: + if workflow_to_delete.user != trans.user and not trans.user_is_admin: raise exceptions.InsufficientPermissionsException() - - # Mark a workflow as deleted - stored_workflow.deleted = True - trans.sa_session.flush() - - # TODO: Unsure of response message to let api know that a workflow was successfully deleted - return f"Workflow '{stored_workflow.name}' successfully deleted" + self.workflow_manager.delete(workflow_to_delete) + return self.workflow_contents_manager.workflow_to_dict(trans, workflow_to_delete, style="instance") @expose_api def import_new_workflow_deprecated(self, trans: GalaxyWebTransaction, payload, **kwd): From 81ea2d302db4aba8857ba99a10e08c19ca8016b3 Mon Sep 17 00:00:00 2001 From: Martin Cech Date: Wed, 12 Oct 2022 13:38:36 -0700 Subject: [PATCH 02/15] implement undelete using the deprecated API style --- lib/galaxy/webapps/galaxy/api/workflows.py | 16 +++++++++++++++- lib/galaxy/webapps/galaxy/buildapp.py | 7 +++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/lib/galaxy/webapps/galaxy/api/workflows.py b/lib/galaxy/webapps/galaxy/api/workflows.py index 816e1304b8c..804a468403a 100644 --- a/lib/galaxy/webapps/galaxy/api/workflows.py +++ b/lib/galaxy/webapps/galaxy/api/workflows.py @@ -464,12 +464,26 @@ class WorkflowsAPIController( Delete a specified workflow """ workflow_to_delete = self.__get_stored_workflow(trans, id, **kwd) - # check to see if user has permissions to selected workflow if workflow_to_delete.user != trans.user and not trans.user_is_admin: raise exceptions.InsufficientPermissionsException() self.workflow_manager.delete(workflow_to_delete) return self.workflow_contents_manager.workflow_to_dict(trans, workflow_to_delete, style="instance") + @expose_api + def undelete(self, trans: ProvidesUserContext, id, **kwd): + """ + POST /api/workflows/{encoded_workflow_id}/undelete + Undelete the workflow with the given ``id`` + + :param id: the encoded id of the user to be undeleted + :type id: str + """ + workflow_to_undelete = self.__get_stored_workflow(trans, id, **kwd) + if workflow_to_undelete.user != trans.user and not trans.user_is_admin: + raise exceptions.InsufficientPermissionsException() + self.workflow_manager.undelete(workflow_to_undelete) + return self.workflow_contents_manager.workflow_to_dict(trans, workflow_to_undelete, style="instance") + @expose_api def import_new_workflow_deprecated(self, trans: GalaxyWebTransaction, payload, **kwd): """ diff --git a/lib/galaxy/webapps/galaxy/buildapp.py b/lib/galaxy/webapps/galaxy/buildapp.py index 912a155ac33..0d350eecfaa 100644 --- a/lib/galaxy/webapps/galaxy/buildapp.py +++ b/lib/galaxy/webapps/galaxy/buildapp.py @@ -576,6 +576,13 @@ def populate_api_routes(webapp, app): controller="workflows", conditions=dict(method=["POST"]), ) + webapp.mapper.connect( + "undelete_workflow", + "/api/workflows/{id}/undelete", + action="undelete", + controller="workflows", + conditions=dict(method=["POST"]), + ) webapp.mapper.resource_with_deleted("user", "users", path_prefix="/api") webapp.mapper.resource("visualization", "visualizations", path_prefix="/api") From a4c26bb538d1c9a3086529389fd7bfff889b0fc6 Mon Sep 17 00:00:00 2001 From: Martin Cech Date: Wed, 12 Oct 2022 15:03:17 -0700 Subject: [PATCH 03/15] add API tests for new undelete endpoint --- lib/galaxy_test/api/test_workflows.py | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/lib/galaxy_test/api/test_workflows.py b/lib/galaxy_test/api/test_workflows.py index 5e8f8da1600..ac4e1089784 100644 --- a/lib/galaxy_test/api/test_workflows.py +++ b/lib/galaxy_test/api/test_workflows.py @@ -20,7 +20,8 @@ import yaml from requests import ( delete, get, - put, + post, + put ) from galaxy.exceptions import error_codes @@ -380,6 +381,26 @@ class WorkflowsApiTestCase(BaseWorkflowsApiTestCase, ChangeDatatypeTestCase): delete_response = delete(workflow_url) self._assert_status_code_is(delete_response, 403) + def test_undelete(self): + workflow_id = self.workflow_populator.simple_workflow("test_undelete") + workflow_name = "test_undelete" + self._assert_user_has_workflow_with_name(workflow_name) + workflow_delete_url = self._api_url(f"workflows/{workflow_id}", use_key=True) + delete(workflow_delete_url) + workflow_undelete_url = self._api_url(f"workflows/{workflow_id}/undelete", use_key=True) + undelete_response = post(workflow_undelete_url) + self._assert_status_code_is(undelete_response, 200) + assert workflow_name in self._workflow_names() + + def test_other_cannot_undelete(self): + workflow_id = self.workflow_populator.simple_workflow("test_other_undelete") + workflow_delete_url = self._api_url(f"workflows/{workflow_id}", use_key=True) + delete(workflow_delete_url) + with self._different_user(): + workflow_undelete_url = self._api_url(f"workflows/{workflow_id}/undelete", use_key=True) + undelete_response = post(workflow_undelete_url) + self._assert_status_code_is(undelete_response, 403) + def test_index(self): index_response = self._get("workflows") self._assert_status_code_is(index_response, 200) From 06ee8ad8654996ee79d573e30e0b5749bddbf7cb Mon Sep 17 00:00:00 2001 From: Martin Cech Date: Fri, 14 Oct 2022 10:46:51 -0700 Subject: [PATCH 04/15] port workflow (un)delete endpoints to fastapi --- lib/galaxy/managers/workflows.py | 2 +- lib/galaxy/webapps/galaxy/api/workflows.py | 59 ++++++++++++---------- lib/galaxy/webapps/galaxy/buildapp.py | 7 --- 3 files changed, 32 insertions(+), 36 deletions(-) diff --git a/lib/galaxy/managers/workflows.py b/lib/galaxy/managers/workflows.py index a13e531e435..87951544774 100644 --- a/lib/galaxy/managers/workflows.py +++ b/lib/galaxy/managers/workflows.py @@ -486,7 +486,7 @@ class WorkflowsManager(sharable.SharableModelManager, deletable.DeletableManager if self.check_security(trans, inv, check_ownership=True, check_accessible=False) ] return invocations, total_matches - + def delete(self, stored_workflow, flush=True): """Mark the given workflow deleted.""" super().delete(stored_workflow, flush=flush) diff --git a/lib/galaxy/webapps/galaxy/api/workflows.py b/lib/galaxy/webapps/galaxy/api/workflows.py index 804a468403a..4f0863f5d1b 100644 --- a/lib/galaxy/webapps/galaxy/api/workflows.py +++ b/lib/galaxy/webapps/galaxy/api/workflows.py @@ -457,33 +457,6 @@ class WorkflowsAPIController( else: return format_return_as_json(ret_dict, pretty=True) - @expose_api - def delete(self, trans: ProvidesUserContext, id, **kwd): - """ - DELETE /api/workflows/{encoded_workflow_id} - Delete a specified workflow - """ - workflow_to_delete = self.__get_stored_workflow(trans, id, **kwd) - if workflow_to_delete.user != trans.user and not trans.user_is_admin: - raise exceptions.InsufficientPermissionsException() - self.workflow_manager.delete(workflow_to_delete) - return self.workflow_contents_manager.workflow_to_dict(trans, workflow_to_delete, style="instance") - - @expose_api - def undelete(self, trans: ProvidesUserContext, id, **kwd): - """ - POST /api/workflows/{encoded_workflow_id}/undelete - Undelete the workflow with the given ``id`` - - :param id: the encoded id of the user to be undeleted - :type id: str - """ - workflow_to_undelete = self.__get_stored_workflow(trans, id, **kwd) - if workflow_to_undelete.user != trans.user and not trans.user_is_admin: - raise exceptions.InsufficientPermissionsException() - self.workflow_manager.undelete(workflow_to_undelete) - return self.workflow_contents_manager.workflow_to_dict(trans, workflow_to_undelete, style="instance") - @expose_api def import_new_workflow_deprecated(self, trans: GalaxyWebTransaction, payload, **kwd): """ @@ -1419,7 +1392,7 @@ class FastAPIWorkflows: self, trans: ProvidesUserContext = DependsOnTrans, invocation_id: DecodedDatabaseIdField = InvocationIDPathParam, - payload: WriteInvocationStoreToPayload = Body(...), + payload: WriteStoreToPayload = Body(...), ) -> AsyncTaskResultSummary: rval = self.invocations_service.write_store( trans, @@ -1428,6 +1401,36 @@ class FastAPIWorkflows: ) return rval + @router.delete( + "/api/workflows/{workflow_id}", + summary="Add the deleted flag to a workflow.", + ) + def delete_workflow( + self, + trans: ProvidesUserContext = DependsOnTrans, + workflow_id: DecodedDatabaseIdField = StoredWorkflowIDPathParam + ): + workflow_to_delete = self.service._workflows_manager.get_stored_workflow(trans, workflow_id) + if workflow_to_delete.user != trans.user and not trans.user_is_admin: + raise exceptions.InsufficientPermissionsException() + self.service._workflows_manager.delete(workflow_to_delete) + return self.service._workflow_contents_manager.workflow_to_dict(trans, workflow_to_delete, style="instance") + + @router.post( + "/api/workflows/{workflow_id}/undelete", + summary="Remove the deleted flag from a workflow.", + ) + def undelete_workflow( + self, + trans: ProvidesUserContext = DependsOnTrans, + workflow_id: DecodedDatabaseIdField = StoredWorkflowIDPathParam + ): + workflow_to_undelete = self.service._workflows_manager.get_stored_workflow(trans, workflow_id) + if workflow_to_undelete.user != trans.user and not trans.user_is_admin: + raise exceptions.InsufficientPermissionsException() + self.service._workflows_manager.undelete(workflow_to_undelete) + return self.service._workflow_contents_manager.workflow_to_dict(trans, workflow_to_undelete, style="instance") + # TODO: remove this endpoint after 23.1 release @router.get( "/api/invocations/{invocation_id}/biocompute", diff --git a/lib/galaxy/webapps/galaxy/buildapp.py b/lib/galaxy/webapps/galaxy/buildapp.py index 0d350eecfaa..912a155ac33 100644 --- a/lib/galaxy/webapps/galaxy/buildapp.py +++ b/lib/galaxy/webapps/galaxy/buildapp.py @@ -576,13 +576,6 @@ def populate_api_routes(webapp, app): controller="workflows", conditions=dict(method=["POST"]), ) - webapp.mapper.connect( - "undelete_workflow", - "/api/workflows/{id}/undelete", - action="undelete", - controller="workflows", - conditions=dict(method=["POST"]), - ) webapp.mapper.resource_with_deleted("user", "users", path_prefix="/api") webapp.mapper.resource("visualization", "visualizations", path_prefix="/api") From fa741bc8434a07c18d51c82e9608fb1845b45297 Mon Sep 17 00:00:00 2001 From: Martin Cech Date: Fri, 14 Oct 2022 11:39:17 -0700 Subject: [PATCH 05/15] hello darkness --- lib/galaxy/managers/workflows.py | 6 ++++-- lib/galaxy/webapps/galaxy/api/workflows.py | 4 ++-- lib/galaxy_test/api/test_workflows.py | 2 +- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/lib/galaxy/managers/workflows.py b/lib/galaxy/managers/workflows.py index 87951544774..26200a2a6ba 100644 --- a/lib/galaxy/managers/workflows.py +++ b/lib/galaxy/managers/workflows.py @@ -42,8 +42,10 @@ from galaxy import ( util, ) from galaxy.job_execution.actions.post import ActionBox -from galaxy.managers import deletable -from galaxy.managers import sharable +from galaxy.managers import ( + deletable, + sharable, +) from galaxy.managers.base import decode_id from galaxy.managers.context import ProvidesUserContext from galaxy.managers.executables import artifact_class diff --git a/lib/galaxy/webapps/galaxy/api/workflows.py b/lib/galaxy/webapps/galaxy/api/workflows.py index 4f0863f5d1b..2fc2c3d12d0 100644 --- a/lib/galaxy/webapps/galaxy/api/workflows.py +++ b/lib/galaxy/webapps/galaxy/api/workflows.py @@ -1408,7 +1408,7 @@ class FastAPIWorkflows: def delete_workflow( self, trans: ProvidesUserContext = DependsOnTrans, - workflow_id: DecodedDatabaseIdField = StoredWorkflowIDPathParam + workflow_id: DecodedDatabaseIdField = StoredWorkflowIDPathParam, ): workflow_to_delete = self.service._workflows_manager.get_stored_workflow(trans, workflow_id) if workflow_to_delete.user != trans.user and not trans.user_is_admin: @@ -1423,7 +1423,7 @@ class FastAPIWorkflows: def undelete_workflow( self, trans: ProvidesUserContext = DependsOnTrans, - workflow_id: DecodedDatabaseIdField = StoredWorkflowIDPathParam + workflow_id: DecodedDatabaseIdField = StoredWorkflowIDPathParam, ): workflow_to_undelete = self.service._workflows_manager.get_stored_workflow(trans, workflow_id) if workflow_to_undelete.user != trans.user and not trans.user_is_admin: diff --git a/lib/galaxy_test/api/test_workflows.py b/lib/galaxy_test/api/test_workflows.py index ac4e1089784..d83f540e7bc 100644 --- a/lib/galaxy_test/api/test_workflows.py +++ b/lib/galaxy_test/api/test_workflows.py @@ -21,7 +21,7 @@ from requests import ( delete, get, post, - put + put, ) from galaxy.exceptions import error_codes From 531e240a7c6adc02bebe12a57e968764824a2f6c Mon Sep 17 00:00:00 2001 From: Martin Cech Date: Mon, 17 Oct 2022 11:00:34 -0700 Subject: [PATCH 06/15] do not proxy to managers through service fix wrong rebase --- lib/galaxy/webapps/galaxy/api/workflows.py | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/lib/galaxy/webapps/galaxy/api/workflows.py b/lib/galaxy/webapps/galaxy/api/workflows.py index 2fc2c3d12d0..37db94fff11 100644 --- a/lib/galaxy/webapps/galaxy/api/workflows.py +++ b/lib/galaxy/webapps/galaxy/api/workflows.py @@ -43,6 +43,8 @@ from galaxy.managers.workflows import ( MissingToolsException, RefactorRequest, WorkflowCreateOptions, + WorkflowContentsManager, + WorkflowsManager, WorkflowUpdateOptions, ) from galaxy.model.item_attrs import UsesAnnotations @@ -1240,6 +1242,8 @@ SkipStepCountsQueryParam: bool = Query( class FastAPIWorkflows: service: WorkflowsService = depends(WorkflowsService) invocations_service: InvocationsService = depends(InvocationsService) + workflows_manager: WorkflowsManager = depends(WorkflowsManager) + workflow_contents_manager: WorkflowContentsManager = depends(WorkflowContentsManager) @router.get( "/api/workflows", @@ -1392,7 +1396,7 @@ class FastAPIWorkflows: self, trans: ProvidesUserContext = DependsOnTrans, invocation_id: DecodedDatabaseIdField = InvocationIDPathParam, - payload: WriteStoreToPayload = Body(...), + payload: WriteInvocationStoreToPayload = Body(...), ) -> AsyncTaskResultSummary: rval = self.invocations_service.write_store( trans, @@ -1410,11 +1414,11 @@ class FastAPIWorkflows: trans: ProvidesUserContext = DependsOnTrans, workflow_id: DecodedDatabaseIdField = StoredWorkflowIDPathParam, ): - workflow_to_delete = self.service._workflows_manager.get_stored_workflow(trans, workflow_id) + workflow_to_delete = self.workflows_manager.get_stored_workflow(trans, workflow_id) if workflow_to_delete.user != trans.user and not trans.user_is_admin: raise exceptions.InsufficientPermissionsException() - self.service._workflows_manager.delete(workflow_to_delete) - return self.service._workflow_contents_manager.workflow_to_dict(trans, workflow_to_delete, style="instance") + self.workflows_manager.delete(workflow_to_delete) + return self.workflow_contents_manager.workflow_to_dict(trans, workflow_to_delete, style="instance") @router.post( "/api/workflows/{workflow_id}/undelete", @@ -1425,11 +1429,11 @@ class FastAPIWorkflows: trans: ProvidesUserContext = DependsOnTrans, workflow_id: DecodedDatabaseIdField = StoredWorkflowIDPathParam, ): - workflow_to_undelete = self.service._workflows_manager.get_stored_workflow(trans, workflow_id) + workflow_to_undelete = self.workflows_manager.get_stored_workflow(trans, workflow_id) if workflow_to_undelete.user != trans.user and not trans.user_is_admin: raise exceptions.InsufficientPermissionsException() - self.service._workflows_manager.undelete(workflow_to_undelete) - return self.service._workflow_contents_manager.workflow_to_dict(trans, workflow_to_undelete, style="instance") + self.workflows_manager.undelete(workflow_to_undelete) + return self.workflow_contents_manager.workflow_to_dict(trans, workflow_to_undelete, style="instance") # TODO: remove this endpoint after 23.1 release @router.get( From 675c147bd79a4294b9b4ad0599aa96afe8d8d16f Mon Sep 17 00:00:00 2001 From: Martin Cech Date: Mon, 17 Oct 2022 11:05:55 -0700 Subject: [PATCH 07/15] darkness --- lib/galaxy/webapps/galaxy/api/workflows.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy/webapps/galaxy/api/workflows.py b/lib/galaxy/webapps/galaxy/api/workflows.py index 37db94fff11..2501decc783 100644 --- a/lib/galaxy/webapps/galaxy/api/workflows.py +++ b/lib/galaxy/webapps/galaxy/api/workflows.py @@ -42,8 +42,8 @@ from galaxy.managers.jobs import ( from galaxy.managers.workflows import ( MissingToolsException, RefactorRequest, - WorkflowCreateOptions, WorkflowContentsManager, + WorkflowCreateOptions, WorkflowsManager, WorkflowUpdateOptions, ) From 0eed6c5374aed8a5ab115fe10169e1289e4b0e57 Mon Sep 17 00:00:00 2001 From: Martin Cech Date: Tue, 18 Oct 2022 09:58:18 -0700 Subject: [PATCH 08/15] remove unneeded super call --- lib/galaxy/managers/workflows.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/lib/galaxy/managers/workflows.py b/lib/galaxy/managers/workflows.py index 26200a2a6ba..3b9170463f8 100644 --- a/lib/galaxy/managers/workflows.py +++ b/lib/galaxy/managers/workflows.py @@ -489,10 +489,6 @@ class WorkflowsManager(sharable.SharableModelManager, deletable.DeletableManager ] return invocations, total_matches - def delete(self, stored_workflow, flush=True): - """Mark the given workflow deleted.""" - super().delete(stored_workflow, flush=flush) - CreatedWorkflow = namedtuple("CreatedWorkflow", ["stored_workflow", "workflow", "missing_tools"]) From 8f0b708698b970e59a023a7190f3b678cee80be2 Mon Sep 17 00:00:00 2001 From: Martin Cech Date: Tue, 18 Oct 2022 10:11:22 -0700 Subject: [PATCH 09/15] drop actual workflow contents from the response --- lib/galaxy/webapps/galaxy/api/workflows.py | 5 ++--- lib/galaxy_test/api/test_workflows.py | 4 ++-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/lib/galaxy/webapps/galaxy/api/workflows.py b/lib/galaxy/webapps/galaxy/api/workflows.py index 2501decc783..3d968f80832 100644 --- a/lib/galaxy/webapps/galaxy/api/workflows.py +++ b/lib/galaxy/webapps/galaxy/api/workflows.py @@ -1243,7 +1243,6 @@ class FastAPIWorkflows: service: WorkflowsService = depends(WorkflowsService) invocations_service: InvocationsService = depends(InvocationsService) workflows_manager: WorkflowsManager = depends(WorkflowsManager) - workflow_contents_manager: WorkflowContentsManager = depends(WorkflowContentsManager) @router.get( "/api/workflows", @@ -1418,7 +1417,7 @@ class FastAPIWorkflows: if workflow_to_delete.user != trans.user and not trans.user_is_admin: raise exceptions.InsufficientPermissionsException() self.workflows_manager.delete(workflow_to_delete) - return self.workflow_contents_manager.workflow_to_dict(trans, workflow_to_delete, style="instance") + return Response(status_code=status.HTTP_204_NO_CONTENT) @router.post( "/api/workflows/{workflow_id}/undelete", @@ -1433,7 +1432,7 @@ class FastAPIWorkflows: if workflow_to_undelete.user != trans.user and not trans.user_is_admin: raise exceptions.InsufficientPermissionsException() self.workflows_manager.undelete(workflow_to_undelete) - return self.workflow_contents_manager.workflow_to_dict(trans, workflow_to_undelete, style="instance") + return Response(status_code=status.HTTP_204_NO_CONTENT) # TODO: remove this endpoint after 23.1 release @router.get( diff --git a/lib/galaxy_test/api/test_workflows.py b/lib/galaxy_test/api/test_workflows.py index d83f540e7bc..e3d295e9625 100644 --- a/lib/galaxy_test/api/test_workflows.py +++ b/lib/galaxy_test/api/test_workflows.py @@ -370,7 +370,7 @@ class WorkflowsApiTestCase(BaseWorkflowsApiTestCase, ChangeDatatypeTestCase): self._assert_user_has_workflow_with_name(workflow_name) workflow_url = self._api_url(f"workflows/{workflow_id}", use_key=True) delete_response = delete(workflow_url) - self._assert_status_code_is(delete_response, 200) + self._assert_status_code_is(delete_response, 204) # Make sure workflow is no longer in index by default. assert workflow_name not in self._workflow_names() @@ -389,7 +389,7 @@ class WorkflowsApiTestCase(BaseWorkflowsApiTestCase, ChangeDatatypeTestCase): delete(workflow_delete_url) workflow_undelete_url = self._api_url(f"workflows/{workflow_id}/undelete", use_key=True) undelete_response = post(workflow_undelete_url) - self._assert_status_code_is(undelete_response, 200) + self._assert_status_code_is(undelete_response, 204) assert workflow_name in self._workflow_names() def test_other_cannot_undelete(self): From 543d6d712d8fabbbfbf4b62c8b0001cb62ccda8d Mon Sep 17 00:00:00 2001 From: Martin Cech Date: Tue, 18 Oct 2022 19:15:11 +0200 Subject: [PATCH 10/15] use check_ownership() in place of explicit perm call Co-authored-by: Marius van den Beek --- lib/galaxy/webapps/galaxy/api/workflows.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/lib/galaxy/webapps/galaxy/api/workflows.py b/lib/galaxy/webapps/galaxy/api/workflows.py index 3d968f80832..5cb830ce61d 100644 --- a/lib/galaxy/webapps/galaxy/api/workflows.py +++ b/lib/galaxy/webapps/galaxy/api/workflows.py @@ -1413,9 +1413,7 @@ class FastAPIWorkflows: trans: ProvidesUserContext = DependsOnTrans, workflow_id: DecodedDatabaseIdField = StoredWorkflowIDPathParam, ): - workflow_to_delete = self.workflows_manager.get_stored_workflow(trans, workflow_id) - if workflow_to_delete.user != trans.user and not trans.user_is_admin: - raise exceptions.InsufficientPermissionsException() + workflow_to_delete = self.workflows_manager.get_stored_workflow(trans, workflow_id, check_ownership=True) self.workflows_manager.delete(workflow_to_delete) return Response(status_code=status.HTTP_204_NO_CONTENT) @@ -1428,9 +1426,7 @@ class FastAPIWorkflows: trans: ProvidesUserContext = DependsOnTrans, workflow_id: DecodedDatabaseIdField = StoredWorkflowIDPathParam, ): - workflow_to_undelete = self.workflows_manager.get_stored_workflow(trans, workflow_id) - if workflow_to_undelete.user != trans.user and not trans.user_is_admin: - raise exceptions.InsufficientPermissionsException() + workflow_to_undelete = self.workflows_manager.get_stored_workflow(trans, workflow_id, check_ownership=True) self.workflows_manager.undelete(workflow_to_undelete) return Response(status_code=status.HTTP_204_NO_CONTENT) From 8b1c562958f3ab61737721a23cf23df81a9d182c Mon Sep 17 00:00:00 2001 From: Martin Cech Date: Tue, 18 Oct 2022 11:16:58 -0700 Subject: [PATCH 11/15] remove unused import --- lib/galaxy/webapps/galaxy/api/workflows.py | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/galaxy/webapps/galaxy/api/workflows.py b/lib/galaxy/webapps/galaxy/api/workflows.py index 5cb830ce61d..47ba804ef35 100644 --- a/lib/galaxy/webapps/galaxy/api/workflows.py +++ b/lib/galaxy/webapps/galaxy/api/workflows.py @@ -42,7 +42,6 @@ from galaxy.managers.jobs import ( from galaxy.managers.workflows import ( MissingToolsException, RefactorRequest, - WorkflowContentsManager, WorkflowCreateOptions, WorkflowsManager, WorkflowUpdateOptions, From 76ab2cc064f9b4783f3330b9c54d2e6d068f4eef Mon Sep 17 00:00:00 2001 From: Martin Cech Date: Tue, 18 Oct 2022 13:48:21 -0700 Subject: [PATCH 12/15] use proper manager method for security check --- lib/galaxy/webapps/galaxy/api/workflows.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/galaxy/webapps/galaxy/api/workflows.py b/lib/galaxy/webapps/galaxy/api/workflows.py index 47ba804ef35..e824f17b5a9 100644 --- a/lib/galaxy/webapps/galaxy/api/workflows.py +++ b/lib/galaxy/webapps/galaxy/api/workflows.py @@ -1412,7 +1412,8 @@ class FastAPIWorkflows: trans: ProvidesUserContext = DependsOnTrans, workflow_id: DecodedDatabaseIdField = StoredWorkflowIDPathParam, ): - workflow_to_delete = self.workflows_manager.get_stored_workflow(trans, workflow_id, check_ownership=True) + workflow_to_delete = self.workflows_manager.get_stored_workflow(trans, workflow_id) + self.workflows_manager.check_security(trans, workflow_to_delete) self.workflows_manager.delete(workflow_to_delete) return Response(status_code=status.HTTP_204_NO_CONTENT) @@ -1425,7 +1426,8 @@ class FastAPIWorkflows: trans: ProvidesUserContext = DependsOnTrans, workflow_id: DecodedDatabaseIdField = StoredWorkflowIDPathParam, ): - workflow_to_undelete = self.workflows_manager.get_stored_workflow(trans, workflow_id, check_ownership=True) + workflow_to_undelete = self.workflows_manager.get_stored_workflow(trans, workflow_id) + self.workflows_manager.check_security(trans, workflow_to_undelete) self.workflows_manager.undelete(workflow_to_undelete) return Response(status_code=status.HTTP_204_NO_CONTENT) From efb8019f60f2205d95597218e7e432253d7204d2 Mon Sep 17 00:00:00 2001 From: Martin Cech Date: Tue, 18 Oct 2022 14:40:09 -0700 Subject: [PATCH 13/15] update expected response code for workflow test --- lib/galaxy_test/api/test_workflows.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy_test/api/test_workflows.py b/lib/galaxy_test/api/test_workflows.py index e3d295e9625..888de72ce00 100644 --- a/lib/galaxy_test/api/test_workflows.py +++ b/lib/galaxy_test/api/test_workflows.py @@ -412,7 +412,7 @@ class WorkflowsApiTestCase(BaseWorkflowsApiTestCase, ChangeDatatypeTestCase): assert [w for w in workflow_index if w["id"] == workflow_id] workflow_url = self._api_url(f"workflows/{workflow_id}", use_key=True) delete_response = delete(workflow_url) - self._assert_status_code_is(delete_response, 200) + self._assert_status_code_is(delete_response, 204) workflow_index = self._get("workflows").json() assert not [w for w in workflow_index if w["id"] == workflow_id] workflow_index = self._get("workflows?show_deleted=true").json() From 26f7249b6cad79ca48531bfb734ab96780fd30d2 Mon Sep 17 00:00:00 2001 From: Martin Cech Date: Wed, 19 Oct 2022 10:27:33 -0700 Subject: [PATCH 14/15] move more logic to the workflow service --- lib/galaxy/webapps/galaxy/api/workflows.py | 9 ++------- lib/galaxy/webapps/galaxy/services/workflows.py | 10 ++++++++++ 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/lib/galaxy/webapps/galaxy/api/workflows.py b/lib/galaxy/webapps/galaxy/api/workflows.py index e824f17b5a9..1f8e1c48340 100644 --- a/lib/galaxy/webapps/galaxy/api/workflows.py +++ b/lib/galaxy/webapps/galaxy/api/workflows.py @@ -1241,7 +1241,6 @@ SkipStepCountsQueryParam: bool = Query( class FastAPIWorkflows: service: WorkflowsService = depends(WorkflowsService) invocations_service: InvocationsService = depends(InvocationsService) - workflows_manager: WorkflowsManager = depends(WorkflowsManager) @router.get( "/api/workflows", @@ -1412,9 +1411,7 @@ class FastAPIWorkflows: trans: ProvidesUserContext = DependsOnTrans, workflow_id: DecodedDatabaseIdField = StoredWorkflowIDPathParam, ): - workflow_to_delete = self.workflows_manager.get_stored_workflow(trans, workflow_id) - self.workflows_manager.check_security(trans, workflow_to_delete) - self.workflows_manager.delete(workflow_to_delete) + self.service.delete(trans, workflow_id) return Response(status_code=status.HTTP_204_NO_CONTENT) @router.post( @@ -1426,9 +1423,7 @@ class FastAPIWorkflows: trans: ProvidesUserContext = DependsOnTrans, workflow_id: DecodedDatabaseIdField = StoredWorkflowIDPathParam, ): - workflow_to_undelete = self.workflows_manager.get_stored_workflow(trans, workflow_id) - self.workflows_manager.check_security(trans, workflow_to_undelete) - self.workflows_manager.undelete(workflow_to_undelete) + self.service.undelete(trans, workflow_id) return Response(status_code=status.HTTP_204_NO_CONTENT) # TODO: remove this endpoint after 23.1 release diff --git a/lib/galaxy/webapps/galaxy/services/workflows.py b/lib/galaxy/webapps/galaxy/services/workflows.py index 8758110155e..c96f654de33 100644 --- a/lib/galaxy/webapps/galaxy/services/workflows.py +++ b/lib/galaxy/webapps/galaxy/services/workflows.py @@ -99,6 +99,16 @@ class WorkflowsService(ServiceBase): return workflows, total_matches return rval, total_matches + def delete(self, trans, workflow_id): + workflow_to_delete = self._workflows_manager.get_stored_workflow(trans, workflow_id) + self._workflows_manager.check_security(trans, workflow_to_delete) + self._workflows_manager.delete(workflow_to_delete) + + def undelete(self, trans, workflow_id): + workflow_to_undelete = self._workflows_manager.get_stored_workflow(trans, workflow_id) + self._workflows_manager.check_security(trans, workflow_to_undelete) + self._workflows_manager.undelete(workflow_to_undelete) + def __get_full_shed_url(self, url): for shed_url in self._tool_shed_registry.tool_sheds.values(): if url in shed_url: From 9c7c903b6c2dfc6422d61d512f339c5bb0bbefc4 Mon Sep 17 00:00:00 2001 From: Martin Cech Date: Wed, 19 Oct 2022 11:29:09 -0700 Subject: [PATCH 15/15] drop dead import --- lib/galaxy/webapps/galaxy/api/workflows.py | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/galaxy/webapps/galaxy/api/workflows.py b/lib/galaxy/webapps/galaxy/api/workflows.py index 1f8e1c48340..38f2486a4ee 100644 --- a/lib/galaxy/webapps/galaxy/api/workflows.py +++ b/lib/galaxy/webapps/galaxy/api/workflows.py @@ -43,7 +43,6 @@ from galaxy.managers.workflows import ( MissingToolsException, RefactorRequest, WorkflowCreateOptions, - WorkflowsManager, WorkflowUpdateOptions, ) from galaxy.model.item_attrs import UsesAnnotations