From 62ed04ffc4dd23bc929e69bf8dbdb63e68283274 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Mon, 4 May 2020 10:36:20 -0400 Subject: [PATCH 1/2] Fix collection mapping in extended metadata mode. --- lib/galaxy/jobs/__init__.py | 2 +- lib/galaxy/model/__init__.py | 11 +++++++++-- lib/galaxy/model/store/__init__.py | 3 ++- 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/lib/galaxy/jobs/__init__.py b/lib/galaxy/jobs/__init__.py index 7c2567eb0ff..cde61b8df24 100644 --- a/lib/galaxy/jobs/__init__.py +++ b/lib/galaxy/jobs/__init__.py @@ -2050,7 +2050,7 @@ class JobWrapper(HasResourceParameters): safe_makedirs(os.path.join(self.working_directory, 'metadata')) self.app.datatypes_registry.to_xml_file(path=datatypes_config) - inp_data, out_data, out_collections = job.io_dicts() + inp_data, out_data, out_collections = job.io_dicts(exclude_implicit_outputs=True) job_metadata = os.path.join(self.tool_working_directory, self.tool.provided_metadata_file) object_store_conf = self.object_store.to_dict() command = self.external_output_metadata.setup_external_metadata(out_data, diff --git a/lib/galaxy/model/__init__.py b/lib/galaxy/model/__init__.py index d51b38cc251..dcaa6869993 100644 --- a/lib/galaxy/model/__init__.py +++ b/lib/galaxy/model/__init__.py @@ -740,13 +740,20 @@ class Job(JobLike, UsesCreateAndUpdateTime, Dictifiable, RepresentById): states.DELETED_NEW, ] - def io_dicts(self): + def io_dicts(self, exclude_implicit_outputs=False): inp_data = dict([(da.name, da.dataset) for da in self.input_datasets]) out_data = dict([(da.name, da.dataset) for da in self.output_datasets]) inp_data.update([(da.name, da.dataset) for da in self.input_library_datasets]) out_data.update([(da.name, da.dataset) for da in self.output_library_datasets]) - out_collections = dict([(obj.name, obj.dataset_collection_instance) for obj in self.output_dataset_collection_instances]) + if not exclude_implicit_outputs: + out_collections = dict([(obj.name, obj.dataset_collection_instance) for obj in self.output_dataset_collection_instances]) + else: + out_collections = {} + for obj in self.output_dataset_collection_instances: + if obj.name not in out_data: + out_collections[obj.name] = obj.dataset_collection_instance + # else this is a mapped over output out_collections.update([(obj.name, obj.dataset_collection) for obj in self.output_dataset_collections]) return inp_data, out_data, out_collections diff --git a/lib/galaxy/model/store/__init__.py b/lib/galaxy/model/store/__init__.py index ba8fa4d1f52..27d656f7c2f 100644 --- a/lib/galaxy/model/store/__init__.py +++ b/lib/galaxy/model/store/__init__.py @@ -696,7 +696,8 @@ class ModelImportStore(object): for order_index, job in enumerate(icj_attrs["jobs"]): icja = model.ImplicitCollectionJobsJobAssociation() icja.implicit_collection_jobs = icj - icja.job = object_import_tracker.jobs_by_key[job] + if job in object_import_tracker.jobs_by_key: + icja.job = object_import_tracker.jobs_by_key[job] icja.order_index = order_index icj.jobs.append(icja) self._session_add(icja) From 6c5c73dabee108c45c9510d480f5de79c396fb17 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Fri, 15 May 2020 09:46:58 -0400 Subject: [PATCH 2/2] Test case for collection mapping with extended metadata. --- lib/galaxy_test/api/test_tools.py | 147 ++++++++++-------- .../test_extended_metadata_mapping.py | 32 ++++ 2 files changed, 111 insertions(+), 68 deletions(-) create mode 100644 test/integration/test_extended_metadata_mapping.py diff --git a/lib/galaxy_test/api/test_tools.py b/lib/galaxy_test/api/test_tools.py index 784cb04a28d..64cb9adf45d 100644 --- a/lib/galaxy_test/api/test_tools.py +++ b/lib/galaxy_test/api/test_tools.py @@ -44,7 +44,60 @@ MINIMAL_TOOL_NO_ID = { } -class ToolsTestCase(ApiTestCase): +class TestsTools(object): + + def _build_pair(self, history_id, contents): + create_response = self.dataset_collection_populator.create_pair_in_history(history_id, contents=contents, direct_upload=True) + hdca_id = create_response.json()["outputs"][0]["id"] + return hdca_id + + def _run_and_check_simple_collection_mapping(self, history_id, inputs): + create = self._run_cat(history_id, inputs=inputs, assert_ok=True) + outputs = create['outputs'] + jobs = create['jobs'] + implicit_collections = create['implicit_collections'] + self.assertEqual(len(jobs), 2) + self.assertEqual(len(outputs), 2) + self.assertEqual(len(implicit_collections), 1) + output1 = outputs[0] + output2 = outputs[1] + output1_content = self.dataset_populator.get_history_dataset_content(history_id, dataset=output1) + output2_content = self.dataset_populator.get_history_dataset_content(history_id, dataset=output2) + self.assertEqual(output1_content.strip(), "123") + self.assertEqual(output2_content.strip(), "456") + + def _run_cat(self, history_id, inputs, assert_ok=False, **kwargs): + return self._run('cat', history_id, inputs, assert_ok=assert_ok, **kwargs) + + def _run(self, tool_id=None, history_id=None, inputs=None, tool_uuid=None, assert_ok=False, tool_version=None, use_cached_job=False, wait_for_job=False): + if inputs is None: + inputs = {} + if tool_id is None: + assert tool_uuid is not None + payload = self.dataset_populator.run_tool_payload( + tool_id=tool_id, + inputs=inputs, + history_id=history_id, + ) + if tool_uuid: + payload['tool_uuid'] = tool_uuid + if tool_version is not None: + payload["tool_version"] = tool_version + if use_cached_job: + payload['use_cached_job'] = True + create_response = self._post("tools", data=payload) + if wait_for_job: + self.dataset_populator.wait_for_job(job_id=create_response.json()['jobs'][0]['id']) + if assert_ok: + self._assert_status_code_is(create_response, 200) + create = create_response.json() + self._assert_has_keys(create, 'outputs') + return create + else: + return create_response + + +class ToolsTestCase(ApiTestCase, TestsTools): def setUp(self): super(ToolsTestCase, self).setUp() @@ -353,7 +406,7 @@ class ToolsTestCase(ApiTestCase): def test_unzip_collection(self): with self.dataset_populator.test_history() as history_id: - hdca_id = self.__build_pair(history_id, ["123", "456"]) + hdca_id = self._build_pair(history_id, ["123", "456"]) inputs = { "input": {"src": "hdca", "id": hdca_id}, } @@ -826,7 +879,7 @@ class ToolsTestCase(ApiTestCase): @skip_without_tool("collection_paired_test") @uses_test_history(require_new=True) def test_collection_parameter(self, history_id): - hdca_id = self.__build_pair(history_id, ["123\n", "456\n"]) + hdca_id = self._build_pair(history_id, ["123\n", "456\n"]) inputs = { "f1": {"src": "hdca", "id": hdca_id}, } @@ -1228,10 +1281,10 @@ class ToolsTestCase(ApiTestCase): [dataset_to_param(new_dataset3), dataset_to_param(new_dataset4)] ) - @skip_without_tool("cat1") + @skip_without_tool("cat") @uses_test_history(require_new=False) def test_map_over_collection(self, history_id): - hdca_id = self.__build_pair(history_id, ["123", "456"]) + hdca_id = self._build_pair(history_id, ["123", "456"]) inputs = { "input1": {'batch': True, 'values': [{'src': 'hdca', 'id': hdca_id}]}, } @@ -1259,7 +1312,7 @@ class ToolsTestCase(ApiTestCase): @uses_test_history(require_new=False) def test_map_over_with_output_format_actions(self, history_id): for use_action in ["do", "dont"]: - hdca_id = self.__build_pair(history_id, ["123", "456"]) + hdca_id = self._build_pair(history_id, ["123", "456"]) inputs = { "input_cond|dispatch": use_action, "input_cond|input": {'batch': True, 'values': [{'src': 'hdca', 'id': hdca_id}]}, @@ -1373,25 +1426,10 @@ class ToolsTestCase(ApiTestCase): hdca = self.dataset_collection_populator.create_list_in_history(history_id, contents=contents).json() return hdca["id"] - def _run_and_check_simple_collection_mapping(self, history_id, inputs): - create = self._run_cat1(history_id, inputs=inputs, assert_ok=True) - outputs = create['outputs'] - jobs = create['jobs'] - implicit_collections = create['implicit_collections'] - self.assertEqual(len(jobs), 2) - self.assertEqual(len(outputs), 2) - self.assertEqual(len(implicit_collections), 1) - output1 = outputs[0] - output2 = outputs[1] - output1_content = self.dataset_populator.get_history_dataset_content(history_id, dataset=output1) - output2_content = self.dataset_populator.get_history_dataset_content(history_id, dataset=output2) - self.assertEqual(output1_content.strip(), "123") - self.assertEqual(output2_content.strip(), "456") - @skip_without_tool("identifier_single") @uses_test_history(require_new=False) def test_identifier_in_map(self, history_id): - hdca_id = self.__build_pair(history_id, ["123", "456"]) + hdca_id = self._build_pair(history_id, ["123", "456"]) inputs = { "input1": {'batch': True, 'values': [{'src': 'hdca', 'id': hdca_id}]}, } @@ -1441,7 +1479,7 @@ class ToolsTestCase(ApiTestCase): @skip_without_tool("identifier_multiple") @uses_test_history(require_new=False) def test_identifier_in_multiple_reduce(self, history_id): - hdca_id = self.__build_pair(history_id, ["123", "456"]) + hdca_id = self._build_pair(history_id, ["123", "456"]) inputs = { "input1": {'src': 'hdca', 'id': hdca_id}, } @@ -1461,7 +1499,7 @@ class ToolsTestCase(ApiTestCase): @skip_without_tool("identifier_in_conditional") @uses_test_history(require_new=False) def test_identifier_map_over_multiple_input_in_conditional(self, history_id): - hdca_id = self.__build_pair(history_id, ["123", "456"]) + hdca_id = self._build_pair(history_id, ["123", "456"]) inputs = { "outer_cond|input1": {'src': 'hdca', 'id': hdca_id}, } @@ -1481,7 +1519,7 @@ class ToolsTestCase(ApiTestCase): @skip_without_tool("identifier_in_conditional") @uses_test_history(require_new=False) def test_identifier_map_over_input_in_conditional(self, history_id): - hdca_id = self.__build_pair(history_id, ["123", "456"]) + hdca_id = self._build_pair(history_id, ["123", "456"]) inputs = { "outer_cond|input1": {'batch': True, 'values': [{'src': 'hdca', 'id': hdca_id}]}, "outer_cond|multi_input": False, @@ -1506,7 +1544,7 @@ class ToolsTestCase(ApiTestCase): @skip_without_tool("identifier_multiple_in_conditional") @uses_test_history(require_new=False) def test_identifier_multiple_reduce_in_conditional(self, history_id): - hdca_id = self.__build_pair(history_id, ["123", "456"]) + hdca_id = self._build_pair(history_id, ["123", "456"]) inputs = { "outer_cond|inner_cond|input1": {'src': 'hdca', 'id': hdca_id}, } @@ -1526,7 +1564,7 @@ class ToolsTestCase(ApiTestCase): @skip_without_tool("identifier_multiple_in_repeat") @uses_test_history(require_new=False) def test_identifier_multiple_reduce_in_repeat(self, history_id): - hdca_id = self.__build_pair(history_id, ["123", "456"]) + hdca_id = self._build_pair(history_id, ["123", "456"]) inputs = { "the_repeat_0|the_data|input1": {'src': 'hdca', 'id': hdca_id}, } @@ -1546,7 +1584,7 @@ class ToolsTestCase(ApiTestCase): @skip_without_tool("identifier_single_in_repeat") @uses_test_history(require_new=False) def test_identifier_single_in_repeat(self, history_id): - hdca_id = self.__build_pair(history_id, ["123", "456"]) + hdca_id = self._build_pair(history_id, ["123", "456"]) inputs = { "the_repeat_0|the_data|input1": {'batch': True, 'values': [{'src': 'hdca', 'id': hdca_id}]} } @@ -1739,8 +1777,8 @@ class ToolsTestCase(ApiTestCase): @skip_without_tool("cat1") @uses_test_history(require_new=False) def test_map_over_two_collections(self, history_id): - hdca1_id = self.__build_pair(history_id, ["123\n", "456\n"]) - hdca2_id = self.__build_pair(history_id, ["789\n", "0ab\n"]) + hdca1_id = self._build_pair(history_id, ["123\n", "456\n"]) + hdca2_id = self._build_pair(history_id, ["789\n", "0ab\n"]) inputs = { "input1": {'batch': True, 'values': [{'src': 'hdca', 'id': hdca1_id}]}, "queries_0|input2": {'batch': True, 'values': [{'src': 'hdca', 'id': hdca2_id}]}, @@ -1767,8 +1805,8 @@ class ToolsTestCase(ApiTestCase): @skip_without_tool("cat1") @uses_test_history(require_new=False) def test_map_over_two_collections_unlinked(self, history_id): - hdca1_id = self.__build_pair(history_id, ["123\n", "456\n"]) - hdca2_id = self.__build_pair(history_id, ["789\n", "0ab\n"]) + hdca1_id = self._build_pair(history_id, ["123\n", "456\n"]) + hdca2_id = self._build_pair(history_id, ["789\n", "0ab\n"]) inputs = { "input1": {'batch': True, 'linked': False, 'values': [{'src': 'hdca', 'id': hdca1_id}]}, "queries_0|input2": {'batch': True, 'linked': False, 'values': [{'src': 'hdca', 'id': hdca2_id}]}, @@ -1819,7 +1857,7 @@ class ToolsTestCase(ApiTestCase): @skip_without_tool("cat1") @uses_test_history(require_new=False) def test_map_over_collected_and_individual_datasets(self, history_id): - hdca1_id = self.__build_pair(history_id, ["123\n", "456\n"]) + hdca1_id = self._build_pair(history_id, ["123\n", "456\n"]) new_dataset1 = self.dataset_populator.new_dataset(history_id, content='789') new_dataset2 = self.dataset_populator.new_dataset(history_id, content='0ab') @@ -1902,7 +1940,7 @@ class ToolsTestCase(ApiTestCase): @skip_without_tool("cat1") def test_cannot_map_over_incompatible_collections(self): with self.dataset_populator.test_history() as history_id: - hdca1_id = self.__build_pair(history_id, ["123\n", "456\n"]) + hdca1_id = self._build_pair(history_id, ["123\n", "456\n"]) hdca2_id = self.dataset_collection_populator.create_list_in_history(history_id).json()["id"] inputs = { "input1": { @@ -1960,7 +1998,7 @@ class ToolsTestCase(ApiTestCase): @skip_without_tool("multi_data_param") def test_reduce_collections_legacy(self): with self.dataset_populator.test_history() as history_id: - hdca1_id = self.__build_pair(history_id, ["123\n", "456\n"]) + hdca1_id = self._build_pair(history_id, ["123\n", "456\n"]) hdca2_id = self.dataset_collection_populator.create_list_in_history(history_id).json()["id"] inputs = { "f1": "__collection_reduce__|%s" % hdca1_id, @@ -1971,7 +2009,7 @@ class ToolsTestCase(ApiTestCase): @skip_without_tool("multi_data_param") def test_reduce_collections(self): with self.dataset_populator.test_history() as history_id: - hdca1_id = self.__build_pair(history_id, ["123\n", "456\n"]) + hdca1_id = self._build_pair(history_id, ["123\n", "456\n"]) hdca2_id = self.dataset_collection_populator.create_list_in_history(history_id).json()["id"] inputs = { "f1": {'src': 'hdca', 'id': hdca1_id}, @@ -1982,7 +2020,7 @@ class ToolsTestCase(ApiTestCase): @skip_without_tool("multi_data_param") def test_implicit_reduce_with_mapping(self): with self.dataset_populator.test_history() as history_id: - hdca1_id = self.__build_pair(history_id, ["123\n", "456\n"]) + hdca1_id = self._build_pair(history_id, ["123\n", "456\n"]) hdca2_id = self.dataset_collection_populator.create_list_of_list_in_history(history_id).json()["id"] inputs = { "f1": {'src': 'hdca', 'id': hdca1_id}, @@ -2040,7 +2078,7 @@ class ToolsTestCase(ApiTestCase): @skip_without_tool("multi_data_repeat") def test_reduce_collections_in_repeat(self): with self.dataset_populator.test_history() as history_id: - hdca1_id = self.__build_pair(history_id, ["123\n", "456\n"]) + hdca1_id = self._build_pair(history_id, ["123\n", "456\n"]) inputs = { "outer_repeat_0|f1": {'src': 'hdca', 'id': hdca1_id}, } @@ -2056,7 +2094,7 @@ class ToolsTestCase(ApiTestCase): @skip_without_tool("multi_data_repeat") def test_reduce_collections_in_repeat_legacy(self): with self.dataset_populator.test_history() as history_id: - hdca1_id = self.__build_pair(history_id, ["123\n", "456\n"]) + hdca1_id = self._build_pair(history_id, ["123\n", "456\n"]) inputs = { "outer_repeat_0|f1": "__collection_reduce__|%s" % hdca1_id, } @@ -2072,7 +2110,7 @@ class ToolsTestCase(ApiTestCase): @skip_without_tool("multi_data_param") def test_reduce_multiple_lists_on_multi_data(self): with self.dataset_populator.test_history() as history_id: - hdca1_id = self.__build_pair(history_id, ["123\n", "456\n"]) + hdca1_id = self._build_pair(history_id, ["123\n", "456\n"]) hdca2_id = self.dataset_collection_populator.create_list_in_history(history_id).json()["id"] inputs = { "f1": [{'src': 'hdca', 'id': hdca1_id}, {'src': 'hdca', 'id': hdca2_id}], @@ -2174,33 +2212,6 @@ class ToolsTestCase(ApiTestCase): def _run_cat1(self, history_id, inputs, assert_ok=False, **kwargs): return self._run('cat1', history_id, inputs, assert_ok=assert_ok, **kwargs) - def _run(self, tool_id=None, history_id=None, inputs=None, tool_uuid=None, assert_ok=False, tool_version=None, use_cached_job=False, wait_for_job=False): - if inputs is None: - inputs = {} - if tool_id is None: - assert tool_uuid is not None - payload = self.dataset_populator.run_tool_payload( - tool_id=tool_id, - inputs=inputs, - history_id=history_id, - ) - if tool_uuid: - payload['tool_uuid'] = tool_uuid - if tool_version is not None: - payload["tool_version"] = tool_version - if use_cached_job: - payload['use_cached_job'] = True - create_response = self._post("tools", data=payload) - if wait_for_job: - self.dataset_populator.wait_for_job(job_id=create_response.json()['jobs'][0]['id']) - if assert_ok: - self._assert_status_code_is(create_response, 200) - create = create_response.json() - self._assert_has_keys(create, 'outputs') - return create - else: - return create_response - def __tool_ids(self): index = self._get("tools") tools_index = index.json() @@ -2345,7 +2356,7 @@ class ToolsTestCase(ApiTestCase): hdca_list_id = response.json()["outputs"][0]["id"] return hdca_list_id - def __build_pair(self, history_id, contents): + def _build_pair(self, history_id, contents): create_response = self.dataset_collection_populator.create_pair_in_history(history_id, contents=contents, direct_upload=True) hdca_id = create_response.json()["outputs"][0]["id"] return hdca_id diff --git a/test/integration/test_extended_metadata_mapping.py b/test/integration/test_extended_metadata_mapping.py new file mode 100644 index 00000000000..6e163c56166 --- /dev/null +++ b/test/integration/test_extended_metadata_mapping.py @@ -0,0 +1,32 @@ +from galaxy_test.api.test_tools import TestsTools +from galaxy_test.base.populators import ( + DatasetCollectionPopulator, + DatasetPopulator, + uses_test_history, +) +from galaxy_test.driver import integration_util + + +class ExtendedMetadataMappingIntegrationTestCase(integration_util.IntegrationTestCase, TestsTools): + """Test mapping over tools with extended metadata enabled.""" + + framework_tool_and_types = True + + @classmethod + def handle_galaxy_config_kwds(cls, config): + config["metadata_strategy"] = "extended" + config["object_store_store_by"] = "uuid" + config["retry_metadata_internally"] = False + + def setUp(self): + super(ExtendedMetadataMappingIntegrationTestCase, self).setUp() + self.dataset_populator = DatasetPopulator(self.galaxy_interactor) + self.dataset_collection_populator = DatasetCollectionPopulator(self.galaxy_interactor) + + @uses_test_history() + def test_map_over_collection(self, history_id): + hdca_id = self._build_pair(history_id, ["123", "456"]) + inputs = { + "input1": {'batch': True, 'values': [{'src': 'hdca', 'id': hdca_id}]}, + } + self._run_and_check_simple_collection_mapping(history_id, inputs)