From 4157042ffaffb767c3345c7f0938eafc215c6e5f Mon Sep 17 00:00:00 2001 From: John Chilton Date: Thu, 25 Jan 2024 10:03:06 -0500 Subject: [PATCH] Fix overlapping names in extra files for model stores. --- lib/galaxy/model/store/__init__.py | 13 ++++- test/unit/data/model/test_model_store.py | 60 ++++++++++++++++++++++-- 2 files changed, 68 insertions(+), 5 deletions(-) diff --git a/lib/galaxy/model/store/__init__.py b/lib/galaxy/model/store/__init__.py index 849266687d55..3863dab33383 100644 --- a/lib/galaxy/model/store/__init__.py +++ b/lib/galaxy/model/store/__init__.py @@ -1992,7 +1992,6 @@ def add(src, dest): as_dict["name"], as_dict["extension"], dataset_hid, conversion_key=conversion_key ) arcname = os.path.join(dir_name, target_filename) - src = file_name dest = os.path.join(export_directory, arcname) add(src, dest) @@ -2005,7 +2004,10 @@ def add(src, dest): file_list = [] if len(file_list): - arcname = os.path.join(dir_name, f"extra_files_path_{dataset_hid}") + extra_files_target_filename = get_export_dataset_extra_files_dir_name( + as_dict["name"], as_dict["extension"], dataset_hid, conversion_key=conversion_key + ) + arcname = os.path.join(dir_name, extra_files_target_filename) add(extra_files_path, os.path.join(export_directory, arcname)) as_dict["extra_files_path"] = arcname else: @@ -2982,6 +2984,13 @@ def get_export_dataset_filename(name: str, ext: str, hid: int, conversion_key: O return f"{base}_{hid}_conversion_{conversion_key}.{ext}" +def get_export_dataset_extra_files_dir_name(name: str, ext: str, hid: int, conversion_key: Optional[str]) -> str: + if not conversion_key: + return f"extra_files_path_{hid}" + else: + return f"extra_files_path_{hid}_conversion_{conversion_key}" + + def imported_store_for_metadata( directory: str, object_store: Optional[ObjectStore] = None ) -> BaseDirectoryImportModelStore: diff --git a/test/unit/data/model/test_model_store.py b/test/unit/data/model/test_model_store.py index 58e4b74d8558..b2a3b0739103 100644 --- a/test/unit/data/model/test_model_store.py +++ b/test/unit/data/model/test_model_store.py @@ -147,6 +147,49 @@ def test_import_export_history_with_implicit_conversion(): assert imported_history.active_datasets[2].hid == imported_history.active_datasets[1].hid +def test_import_export_history_with_implicit_conversion_and_extra_files(): + app = _mock_app() + + u, h, d1, d2, j = _setup_simple_cat_job(app) + + convert_ext = "fasta" + implicit_hda = model.HistoryDatasetAssociation(extension=convert_ext, create_dataset=True, flush=False, history=h) + implicit_hda.hid = d2.hid + # this adds and flushes the result... + d2.attach_implicitly_converted_dataset(app.model.context, implicit_hda, convert_ext) + app.object_store.update_from_file(implicit_hda.dataset, file_name=TEST_PATH_2_CONVERTED, create=True) + + d2.dataset.create_extra_files_path() + implicit_hda.dataset.create_extra_files_path() + + app.write_primary_file(d2, "cool primary file 1") + app.write_composite_file(d2, "cool composite file", "child_file") + + app.write_primary_file(implicit_hda, "cool primary file implicit") + app.write_composite_file(implicit_hda, "cool composite file implicit", "child_file_converted") + + assert len(h.active_datasets) == 3 + imported_history = _import_export_history(app, h, export_files="copy", include_hidden=True) + + assert len(imported_history.active_datasets) == 3 + recovered_hda_2 = imported_history.active_datasets[1] + assert recovered_hda_2.implicitly_converted_datasets + imported_conversion = recovered_hda_2.implicitly_converted_datasets[0] + assert imported_conversion.type == "fasta" + assert imported_conversion.dataset == imported_history.active_datasets[2] + + # implicit conversions have the same HID... ensure this property is recovered... + assert imported_history.active_datasets[2].hid == imported_history.active_datasets[1].hid + + _assert_extra_files_has_parent_directory_with_single_file_containing( + imported_history.active_datasets[1], "child_file", "cool composite file" + ) + + _assert_extra_files_has_parent_directory_with_single_file_containing( + imported_history.active_datasets[2], "child_file_converted", "cool composite file implicit" + ) + + def test_import_export_bag_archive(): """Test a simple job import/export using a BagIt archive.""" dest_parent = mkdtemp() @@ -721,15 +764,24 @@ def test_import_export_composite_datasets(): _perform_import_from_directory(temp_directory, app, u, import_history) assert len(import_history.datasets) == 1 import_dataset = import_history.datasets[0] - root_extra_files_path = import_dataset.extra_files_path + _assert_extra_files_has_parent_directory_with_single_file_containing( + import_dataset, "child_file", "cool composite file" + ) + + +def _assert_extra_files_has_parent_directory_with_single_file_containing( + dataset, expected_file_name, expected_contents +): + root_extra_files_path = dataset.extra_files_path assert len(os.listdir(root_extra_files_path)) == 1 assert os.listdir(root_extra_files_path)[0] == "parent_dir" composite_sub_dir = os.path.join(root_extra_files_path, "parent_dir") child_files = os.listdir(composite_sub_dir) assert len(child_files) == 1 + assert child_files[0] == expected_file_name with open(os.path.join(composite_sub_dir, child_files[0])) as f: contents = f.read() - assert contents == "cool composite file" + assert contents == expected_contents def test_edit_metadata_files(): @@ -1048,7 +1100,9 @@ def write_primary_file(self, dataset_instance, contents): primary = NamedTemporaryFile("w") primary.write(contents) primary.flush() - self.object_store.update_from_file(dataset_instance.dataset, file_name=primary.name, create=True, preserve_symlinks=True) + self.object_store.update_from_file( + dataset_instance.dataset, file_name=primary.name, create=True, preserve_symlinks=True + ) def write_composite_file(self, dataset_instance, contents, file_name): composite1 = NamedTemporaryFile("w")