From f9795725012f686c5b72b11bb00af505ea2962f8 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Wed, 18 Oct 2023 11:30:58 +0200 Subject: [PATCH] Remove more flushes in database operation tools We sort of rely on the dataset not being flushed yet in order to bypass the security check for tagging post job actions, so this fixes: ``` ItemOwnershipException: User does not own item. File "galaxy/workflow/run.py", line 233, in invoke incomplete_or_none = self._invoke_step(workflow_invocation_step) File "galaxy/workflow/run.py", line 309, in _invoke_step incomplete_or_none = invocation_step.workflow_step.module.execute( File "galaxy/workflow/modules.py", line 2274, in execute workflow_invocation_uuid=invocation.uuid.hex, File "galaxy/tools/execute.py", line 169, in execute execute_single_job(execution_slice, completed_jobs[i], skip=skip) File "galaxy/tools/execute.py", line 116, in execute_single_job job, result = tool.handle_single_execution( File "galaxy/tools/__init__.py", line 1957, in handle_single_execution execution_cache=execution_cache, File "galaxy/tools/__init__.py", line 1937, in handle_single_execution execution_slice, File "galaxy/tools/__init__.py", line 2034, in execute args[key] = model.User.expand_user_properties(trans.user, param.value) File "galaxy/tools/actions/model_operations.py", line 102, in execute job_callback(job) File "galaxy/workflow/modules.py", line 2284, in complete = True File "galaxy/workflow/modules.py", line 2351, in _handle_post_job_actions self.trans.sa_session.add(pjaa) File "galaxy/job_execution/actions/post.py", line 575, in execute ActionBox.actions[pja.action_type].execute( File "galaxy/job_execution/actions/post.py", line 473, in execute cls._execute(tag_handler, job.user, dataset_assoc.dataset, tags) File "galaxy/job_execution/actions/post.py", line 481, in _execute tag_handler.add_tags_from_list(user, output, tags, flush=False) File "galaxy/model/tags.py", line 74, in add_tags_from_list return self.set_tags_from_list(user, item, new_tags_set, flush=flush) File "galaxy/model/tags.py", line 92, in set_tags_from_list self.delete_item_tags(user, item) File "galaxy/model/tags.py", line 165, in delete_item_tags self._ensure_user_owns_item(user, item) File "galaxy/model/tags.py", line 193, in _ensure_user_owns_item raise ItemOwnershipException("User does not own item.") ``` (once again). From https://sentry.galaxyproject.org/share/issue/5b44d1ec20094a2ebe8331b1ef6e58a8/ --- lib/galaxy/model/__init__.py | 2 +- lib/galaxy/tools/__init__.py | 26 +++++++++++++++++--------- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/lib/galaxy/model/__init__.py b/lib/galaxy/model/__init__.py index 6652ff72814d..716a17eac4ae 100644 --- a/lib/galaxy/model/__init__.py +++ b/lib/galaxy/model/__init__.py @@ -5721,7 +5721,7 @@ def to_history_dataset_association(self, target_history, parent_id=None, add_to_ sa_session.commit() return hda - def copy(self, parent_id=None, target_folder=None): + def copy(self, parent_id=None, target_folder=None, flush=True): sa_session = object_session(self) ldda = LibraryDatasetDatasetAssociation( name=self.name, diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index 877ddfc722f4..866f1cc57f4f 100644 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -3230,7 +3230,9 @@ def produce_outputs(self, trans, out_data, output_collections, incoming, history assert collection.collection_type == "paired" forward_o, reverse_o = collection.dataset_instances - forward, reverse = forward_o.copy(copy_tags=forward_o.tags), reverse_o.copy(copy_tags=reverse_o.tags) + forward, reverse = forward_o.copy(copy_tags=forward_o.tags, flush=False), reverse_o.copy( + copy_tags=reverse_o.tags, flush=False + ) self._add_datasets_to_history(history, [forward, reverse]) out_data["forward"] = forward @@ -3246,7 +3248,9 @@ def produce_outputs(self, trans, out_data, output_collections, incoming, history forward_o = incoming["input_forward"] reverse_o = incoming["input_reverse"] - forward, reverse = forward_o.copy(copy_tags=forward_o.tags), reverse_o.copy(copy_tags=reverse_o.tags) + forward, reverse = forward_o.copy(copy_tags=forward_o.tags, flush=False), reverse_o.copy( + copy_tags=reverse_o.tags, flush=False + ) new_elements = {} new_elements["forward"] = forward new_elements["reverse"] = reverse @@ -3277,7 +3281,9 @@ def produce_outputs(self, trans, out_data, output_collections, incoming, history identifier = getattr(incoming_repeat["input"], "element_identifier", incoming_repeat["input"].name) elif id_select == "manual": identifier = incoming_repeat["id_cond"]["identifier"] - new_elements[identifier] = incoming_repeat["input"].copy(copy_tags=incoming_repeat["input"].tags) + new_elements[identifier] = incoming_repeat["input"].copy( + copy_tags=incoming_repeat["input"].tags, flush=False + ) self._add_datasets_to_history(history, new_elements.values()) output_collections.create_collection( @@ -3311,7 +3317,9 @@ def produce_outputs(self, trans, out_data, output_collections, incoming, history else: raise Exception("Invalid tool parameters.") extracted = extracted_element.element_object - extracted_o = extracted.copy(copy_tags=extracted.tags, new_name=extracted_element.element_identifier) + extracted_o = extracted.copy( + copy_tags=extracted.tags, new_name=extracted_element.element_identifier, flush=False + ) self._add_datasets_to_history(history, [extracted_o], datasets_visible=True) out_data["output"] = extracted_o @@ -3394,7 +3402,7 @@ def produce_outputs(self, trans, out_data, output_collections, incoming, history if getattr(value, "history_content_type", None) == "dataset": copied_value = value.copy(copy_tags=value.tags, flush=False) else: - copied_value = value.copy() + copied_value = value.copy(flush=False) new_elements[key] = copied_value self._add_datasets_to_history(history, new_elements.values()) @@ -3414,7 +3422,7 @@ def _get_new_elements(self, history, elements_to_copy): if getattr(dce.element_object, "history_content_type", None) == "dataset": copied_value = dce.element_object.copy(copy_tags=dce.element_object.tags, flush=False) else: - copied_value = dce.element_object.copy() + copied_value = dce.element_object.copy(flush=False) new_elements[element_identifier] = copied_value return new_elements @@ -3582,7 +3590,7 @@ def add_copied_value_to_new_elements(new_label, dce_object): if getattr(dce_object, "history_content_type", None) == "dataset": copied_value = dce_object.copy(copy_tags=dce_object.tags, flush=False) else: - copied_value = dce_object.copy() + copied_value = dce_object.copy(flush=False) new_elements[new_label] = copied_value new_labels_path = new_labels_dataset_assoc.file_name @@ -3688,7 +3696,7 @@ def add_copied_value_to_new_elements(new_tags_dict, dce): ) else: # We have a collection, and we copy the elements so that we don't manipulate the original tags - copied_value = dce.element_object.copy(element_destination=history) + copied_value = dce.element_object.copy(element_destination=history, flush=False) for new_element, old_element in zip(copied_value.dataset_elements, dce.element_object.dataset_elements): # TODO: This should be eliminated, but collections created by the collection builder # don't set `visible` to `False` if you don't hide the original elements. @@ -3748,7 +3756,7 @@ def produce_outputs(self, trans, out_data, output_collections, incoming, history if getattr(dce_object, "history_content_type", None) == "dataset": copied_value = dce_object.copy(copy_tags=dce_object.tags, flush=False) else: - copied_value = dce_object.copy() + copied_value = dce_object.copy(flush=False) if passes_filter: filtered_elements[element_identifier] = copied_value