Skip to content

Commit

Permalink
Make CompressedFile a context manager
Browse files Browse the repository at this point in the history
to ensure that the archive is closed.
  • Loading branch information
nsoranzo committed Jan 17, 2025
1 parent 068cf2b commit 85ecf0f
Show file tree
Hide file tree
Showing 9 changed files with 51 additions and 17 deletions.
3 changes: 2 additions & 1 deletion lib/galaxy/datatypes/isa.py
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,8 @@ def groom_dataset_content(self, file_name: str) -> None:
# perform extraction
# For some ZIP files CompressedFile::extract() extract the file inside <output_folder>/<file_name> instead of outputing it inside <output_folder>. So we first create a temporary folder, extract inside it, and move content to final destination.
temp_folder = tempfile.mkdtemp()
CompressedFile(file_name).extract(temp_folder)
with CompressedFile(file_name) as cf:
cf.extract(temp_folder)
shutil.rmtree(output_path)
extracted_files = os.listdir(temp_folder)
logger.debug(" ".join(extracted_files))
Expand Down
3 changes: 2 additions & 1 deletion lib/galaxy/model/store/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -3075,7 +3075,8 @@ def source_to_import_store(
if ModelStoreFormat.is_compressed(model_store_format):
try:
temp_dir = mkdtemp()
target_dir = CompressedFile(target_path).extract(temp_dir)
with CompressedFile(target_path) as cf:
target_dir = cf.extract(temp_dir)
finally:
if delete:
os.remove(target_path)
Expand Down
4 changes: 2 additions & 2 deletions lib/galaxy/tools/data_fetch.py
Original file line number Diff line number Diff line change
Expand Up @@ -442,8 +442,8 @@ def _decompress_target(upload_config: "UploadConfig", target: Dict[str, Any]):
# fuzzy_root to False to literally interpret the target.
fuzzy_root = target.get("fuzzy_root", True)
temp_directory = os.path.abspath(tempfile.mkdtemp(prefix=elements_from_name, dir=upload_config.working_directory))
cf = CompressedFile(elements_from_path)
result = cf.extract(temp_directory)
with CompressedFile(elements_from_path) as cf:
result = cf.extract(temp_directory)
return result if fuzzy_root else temp_directory


Expand Down
30 changes: 27 additions & 3 deletions lib/galaxy/util/compression_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import tarfile
import tempfile
import zipfile
from types import TracebackType
from typing import (
Any,
cast,
Expand All @@ -18,10 +19,14 @@
Optional,
overload,
Tuple,
Type,
Union,
)

from typing_extensions import Literal
from typing_extensions import (
Literal,
Self,
)

from galaxy.util.path import (
safe_relpath,
Expand Down Expand Up @@ -167,12 +172,16 @@ def decompress_bytes_to_directory(content: bytes) -> str:
with tempfile.NamedTemporaryFile(delete=False) as fp:
fp.write(content)
fp.close()
return CompressedFile(fp.name).extract(temp_directory)
with CompressedFile(fp.name) as cf:
outdir = cf.extract(temp_directory)
return outdir


def decompress_path_to_directory(path: str) -> str:
temp_directory = tempfile.mkdtemp()
return CompressedFile(path).extract(temp_directory)
with CompressedFile(path) as cf:
outdir = cf.extract(temp_directory)
return outdir


class CompressedFile:
Expand Down Expand Up @@ -361,6 +370,21 @@ def zipfile_ok(self, path_to_archive: StrPath) -> bool:
return False
return True

def __enter__(self) -> Self:
return self

def __exit__(
self,
exc_type: Optional[Type[BaseException]],
exc_value: Optional[BaseException],
traceback: Optional[TracebackType],
) -> bool:
try:
self.archive.close()
return exc_type is None
except Exception:
return False


class FastZipFile(zipfile.ZipFile):
"""
Expand Down
3 changes: 2 additions & 1 deletion lib/tool_shed/test/functional/test_shed_repositories.py
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,8 @@ def test_repository_search(self):

def test_repo_tars(self):
for index, repo_path in enumerate(repo_tars("column_maker")):
path = CompressedFile(repo_path).extract(tempfile.mkdtemp())
with CompressedFile(repo_path) as cf:
path = cf.extract(tempfile.mkdtemp())
tool_xml_path = os.path.join(path, "column_maker.xml")
tool_source = get_tool_source(config_file=tool_xml_path)
tool_version = tool_source.parse_version()
Expand Down
6 changes: 4 additions & 2 deletions test/integration/test_workflow_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,13 @@ def test_export_import_invocation_with_input_as_output_sts(self):

def test_export_ro_crate_basic(self):
ro_crate_path = self._export_invocation_to_format(extension="rocrate.zip", to_uri=False)
assert CompressedFile(ro_crate_path).file_type == "zip"
with CompressedFile(ro_crate_path) as cf:
assert cf.file_type == "zip"

def test_export_ro_crate_to_uri(self):
ro_crate_path = self._export_invocation_to_format(extension="rocrate.zip", to_uri=True)
assert CompressedFile(ro_crate_path).file_type == "zip"
with CompressedFile(ro_crate_path) as cf:
assert cf.file_type == "zip"

def test_export_bco_basic(self):
bco_path = self._export_invocation_to_format(extension="bco.json", to_uri=False)
Expand Down
9 changes: 5 additions & 4 deletions test/unit/data/model/test_model_store.py
Original file line number Diff line number Diff line change
Expand Up @@ -646,9 +646,9 @@ def test_export_invocation_to_ro_crate_archive(tmp_path):
crate_zip = tmp_path / "crate.zip"
with store.ROCrateArchiveModelExportStore(crate_zip, app=app, export_files="symlink") as export_store:
export_store.export_workflow_invocation(workflow_invocation)
compressed_file = CompressedFile(crate_zip)
assert compressed_file.file_type == "zip"
compressed_file.extract(tmp_path)
with CompressedFile(crate_zip) as compressed_file:
assert compressed_file.file_type == "zip"
compressed_file.extract(tmp_path)
crate_directory = tmp_path / "crate"
validate_invocation_crate_directory(crate_directory)

Expand Down Expand Up @@ -1249,7 +1249,8 @@ class Options:

def import_archive(archive_path, app, user, import_options=None):
dest_parent = mkdtemp()
dest_dir = CompressedFile(archive_path).extract(dest_parent)
with CompressedFile(archive_path) as cf:
dest_dir = cf.extract(dest_parent)

import_options = import_options or store.ImportOptions()
model_store = store.get_import_model_store_for_directory(
Expand Down
7 changes: 5 additions & 2 deletions test/unit/util/test_compression_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ def test_compression_safety(self):
self.assert_safety("test-data/unsafe.zip", False)
self.assert_safety("test-data/4.bed.zip", True)
self.assert_safety("test-data/testdir.tar", True)
self.assert_safety("test-data/testdir1.tar.gz", True)
self.assert_safety("test-data/safetar_with_symlink.tar", True)
self.assert_safety("test-data/safe_relative_symlink.tar", True)

Expand All @@ -30,10 +31,12 @@ def assert_safety(self, path, expected_to_be_safe):
temp_dir = tempfile.mkdtemp()
try:
if expected_to_be_safe:
CompressedFile(path).extract(temp_dir)
with CompressedFile(path) as cf:
cf.extract(temp_dir)
else:
with self.assertRaisesRegex(Exception, "is blocked"):
CompressedFile(path).extract(temp_dir)
with CompressedFile(path) as cf:
cf.extract(temp_dir)
finally:
shutil.rmtree(temp_dir, ignore_errors=True)

Expand Down
3 changes: 2 additions & 1 deletion tools/data_source/upload.py
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,8 @@ def stage_file(name, composite_file_path, is_binary=False):
# a little more explicitly so we didn't need to dispatch on the datatype and so we
# could attach arbitrary extra composite data to an existing composite datatype if
# if need be? Perhaps that would be a mistake though.
CompressedFile(dp).extract(files_path)
with CompressedFile(dp) as cf:
cf.extract(files_path)
else:
tmpdir = output_adjacent_tmpdir(output_path)
tmp_prefix = "data_id_%s_convert_" % dataset.dataset_id
Expand Down

0 comments on commit 85ecf0f

Please sign in to comment.