From c7c0bafcf795b18260c79998ff0cc4abb4eab250 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Mon, 15 Apr 2024 19:30:31 +0200 Subject: [PATCH] Use or copy StoredWorkflow when copying step Fixes https://sentry.galaxyproject.org/share/issue/509cadd762e749e1a782bb4806161ed3/: ``` AttributeError: 'NoneType' object has no attribute 'latest_workflow' File "starlette/applications.py", line 123, in __call__ await self.middleware_stack(scope, receive, send) File "starlette/middleware/errors.py", line 186, in __call__ raise exc File "starlette/middleware/errors.py", line 164, in __call__ await self.app(scope, receive, _send) File "starlette_context/middleware/raw_middleware.py", line 92, in __call__ await self.app(scope, receive, send_wrapper) File "starlette/middleware/base.py", line 189, in __call__ with collapse_excgroups(): File "contextlib.py", line 155, in __exit__ self.gen.throw(typ, value, traceback) File "starlette/_utils.py", line 93, in collapse_excgroups raise exc File "starlette/middleware/base.py", line 191, in __call__ response = await self.dispatch_func(request, call_next) File "galaxy/webapps/galaxy/fast_app.py", line 108, in add_x_frame_options response = await call_next(request) File "starlette/middleware/base.py", line 165, in call_next raise app_exc File "starlette/middleware/base.py", line 151, in coro await self.app(scope, receive_or_disconnect, send_no_error) File "starlette/middleware/exceptions.py", line 62, in __call__ await wrap_app_handling_exceptions(self.app, conn)(scope, receive, send) File "starlette/_exception_handler.py", line 64, in wrapped_app raise exc File "starlette/_exception_handler.py", line 53, in wrapped_app await app(scope, receive, sender) File "starlette/routing.py", line 758, in __call__ await self.middleware_stack(scope, receive, send) File "starlette/routing.py", line 778, in app await route.handle(scope, receive, send) File "starlette/routing.py", line 299, in handle await self.app(scope, receive, send) File "starlette/routing.py", line 79, in app await wrap_app_handling_exceptions(app, request)(scope, receive, send) File "starlette/_exception_handler.py", line 64, in wrapped_app raise exc File "starlette/_exception_handler.py", line 53, in wrapped_app await app(scope, receive, sender) File "starlette/routing.py", line 74, in app response = await func(request) File "fastapi/routing.py", line 278, in app raw_response = await run_endpoint_function( File "fastapi/routing.py", line 193, in run_endpoint_function return await run_in_threadpool(dependant.call, **values) File "starlette/concurrency.py", line 42, in run_in_threadpool return await anyio.to_thread.run_sync(func, *args) File "anyio/to_thread.py", line 56, in run_sync return await get_async_backend().run_sync_in_worker_thread( File "anyio/_backends/_asyncio.py", line 2144, in run_sync_in_worker_thread return await future File "anyio/_backends/_asyncio.py", line 851, in run result = context.run(func, *args) File "galaxy/webapps/galaxy/api/workflows.py", line 987, in refactor return self.service.refactor(trans, workflow_id, payload, instance or False) File "galaxy/webapps/galaxy/services/workflows.py", line 218, in refactor return self._workflow_contents_manager.refactor(trans, stored_workflow, payload) File "galaxy/managers/workflows.py", line 1963, in refactor refactored_workflow, action_executions = self.do_refactor(trans, stored_workflow, refactor_request) File "galaxy/managers/workflows.py", line 1947, in do_refactor action_executions = refactor_executor.refactor(refactor_request) File "galaxy/workflow/refactor/execute.py", line 77, in refactor refactor_method(action, execution) File "galaxy/workflow/refactor/execute.py", line 374, in _apply_upgrade_subworkflow content_id = trans.security.encode_id(stored_workflow.latest_workflow.id) ``` You can reproduce this by creating a subworkflow, renaming the parent workflow and then attempting to update the subworkflow step. --- lib/galaxy/model/__init__.py | 18 ++++++++++++++---- test/unit/data/test_galaxy_mapping.py | 3 +++ 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/lib/galaxy/model/__init__.py b/lib/galaxy/model/__init__.py index 7e582782e911..7c239fd720cd 100644 --- a/lib/galaxy/model/__init__.py +++ b/lib/galaxy/model/__init__.py @@ -8002,10 +8002,20 @@ def copy_to(self, copied_step, step_mapping, user=None): copied_step.annotations = annotations if subworkflow := self.subworkflow: - copied_subworkflow = subworkflow.copy() - copied_step.subworkflow = copied_subworkflow - for subworkflow_step, copied_subworkflow_step in zip(subworkflow.steps, copied_subworkflow.steps): - subworkflow_step_mapping[subworkflow_step.id] = copied_subworkflow_step + stored_subworkflow = subworkflow.stored_workflow + if stored_subworkflow and stored_subworkflow.user == user: + # This should be fine and reduces the number of stored subworkflows + copied_step.subworkflow = subworkflow + else: + # Can this even happen, building a workflow with a subworkflow you don't own ? + copied_subworkflow = subworkflow.copy() + stored_workflow = StoredWorkflow( + user, name=copied_subworkflow.name, workflow=copied_subworkflow, hidden=True + ) + copied_subworkflow.stored_workflow = stored_workflow + copied_step.subworkflow = copied_subworkflow + for subworkflow_step, copied_subworkflow_step in zip(subworkflow.steps, copied_subworkflow.steps): + subworkflow_step_mapping[subworkflow_step.id] = copied_subworkflow_step for old_conn, new_conn in zip(self.input_connections, copied_step.input_connections): new_conn.input_step_input = copied_step.get_or_add_input(old_conn.input_name) diff --git a/test/unit/data/test_galaxy_mapping.py b/test/unit/data/test_galaxy_mapping.py index 7e6c35d757f1..aa4ff3a86b3a 100644 --- a/test/unit/data/test_galaxy_mapping.py +++ b/test/unit/data/test_galaxy_mapping.py @@ -881,6 +881,9 @@ def test_workflows(self): assert loaded_invocation assert loaded_invocation.history.id == history_id + # recover user after expunge + user = loaded_invocation.history.user + step_1, step_2 = loaded_invocation.workflow.steps assert not step_1.subworkflow