Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 37 additions & 6 deletions beets/importer/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -804,18 +804,26 @@ class ArchiveImportTask(SentinelImportTask):
"""An import task that represents the processing of an archive.

`toppath` must be a `zip`, `tar`, or `rar` archive. Archive tasks
serve two purposes:
serve three purposes:
Comment thread
snystrom marked this conversation as resolved.
Outdated
- First, it will unarchive the files to a temporary directory and
return it. The client should read tasks from the resulting
directory and send them through the pipeline.
- Second, it will clean up the temporary directory when it proceeds
through the pipeline. The client should send the archive task
after sending the rest of the music tasks to make this work.
- Third, when the import mode is ``move`` and every file in the
archive was successfully imported, it will remove the source
archive itself. Archives are preserved on partial imports and in
non-move modes.
"""

def __init__(self, toppath):
super().__init__(toppath, ())
self.extracted = False
# ``extract()`` reassigns ``self.toppath`` to the temp extraction
# directory; here we track the original archive location so
# ``cleanup()`` can remove it when the import mode demands.
self.archive_path = toppath

@classmethod
def is_archive(cls, path):
Expand Down Expand Up @@ -862,13 +870,36 @@ def handlers(cls) -> list[ArchiveHandler]:
return _handlers

def cleanup(self, copy=False, delete=False, move=False):
"""Removes the temporary directory the archive was extracted to."""
if self.extracted and self.toppath:
"""Remove the temporary extraction directory. In ``move`` mode,
if the extraction directory is empty after the pipeline has run
Comment thread
snejus marked this conversation as resolved.
Outdated
(i.e. every file in the archive was successfully imported)
also remove the source archive. Archives are preserved on
partial imports and in non-move modes.
"""
if not self.extracted:
return

all_files_imported = move and not any(
files for _, _, files in os.walk(util.syspath(self.toppath))
)

log.debug(
"Removing extracted directory: {}",
util.displayable_path(self.toppath),
)
shutil.rmtree(util.syspath(self.toppath))

if all_files_imported:
log.debug(
"Removing imported archive: {}",
util.displayable_path(self.archive_path),
)
util.remove(self.archive_path)
elif move:
log.debug(
"Removing extracted directory: {}",
util.displayable_path(self.toppath),
"Not removing partially imported archive: {}",
util.displayable_path(self.archive_path),
)
shutil.rmtree(util.syspath(self.toppath))

def extract(self):
"""Extracts the archive to a temporary directory and sets
Expand Down
5 changes: 5 additions & 0 deletions docs/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,11 @@ New features
- :doc:`plugins/lyrics`: Add ``keep_synced`` config option and ``--keep-synced``
CLI flag to skip re-fetching lyrics for tracks that already have synced
lyrics, even when ``force`` is enabled. :bug:`5249`
- :ref:`import-cmd`: When importing an archive (zip, tar, rar, or 7z) with
``move: yes``, the source archive is now removed after a successful import.
Archives are preserved if any file in the archive was not imported (e.g.
skipped as a duplicate, or the import was aborted), and in non-move import
modes.
Comment thread
snejus marked this conversation as resolved.
Outdated

Bug fixes
~~~~~~~~~
Expand Down
45 changes: 45 additions & 0 deletions test/test_importer.py
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,9 @@ def setUp(self):
super().setUp()
self.want_resume = False
self.config["incremental"] = False
self.config["copy"] = False
self.config["move"] = False
self.config["delete"] = False
self._old_home = None

def test_rm(self):
Expand All @@ -191,6 +194,48 @@ def test_rm(self):
archive_task.finalize(self)
assert not tmp_path.exists()

def test_archive_removed_on_move_complete(self):
zip_path = create_archive(self)
archive_task = importer.ArchiveImportTask(zip_path)
archive_task.extract()
for root, _, files in os.walk(syspath(archive_task.toppath)):
for f in files:
os.remove(os.path.join(root, f))
assert Path(os.fsdecode(zip_path)).exists()
archive_task.cleanup(move=True)
assert not Path(os.fsdecode(zip_path)).exists()

def test_archive_preserved_on_move_partial(self):
zip_path = create_archive(self)
archive_task = importer.ArchiveImportTask(zip_path)
archive_task.extract()
archive_task.cleanup(move=True)
assert Path(os.fsdecode(zip_path)).exists()

def test_archive_preserved_on_copy(self):
zip_path = create_archive(self)
archive_task = importer.ArchiveImportTask(zip_path)
archive_task.extract()
archive_task.cleanup(copy=True)
assert Path(os.fsdecode(zip_path)).exists()

def test_tempdir_removed_in_all_modes(self):
for cleanup_kwargs in (
{},
{"move": True},
{"copy": True},
{"copy": True, "delete": True},
):
zip_path = create_archive(self)
archive_task = importer.ArchiveImportTask(zip_path)
archive_task.extract()
tmp_path = Path(os.fsdecode(archive_task.toppath))
assert tmp_path.exists(), f"extract failed for {cleanup_kwargs}"
archive_task.cleanup(**cleanup_kwargs)
assert not tmp_path.exists(), (
f"tempdir {tmp_path} not removed for {cleanup_kwargs}"
)


class ImportZipTest(AsIsImporterMixin, ImportTestCase):
def test_import_zip(self):
Expand Down
Loading