Fix #796: Soulseek album bundle left completed files in slskd download folder
The album-bundle path COPIES slskd's completed files into private staging (then on to the library) but never removed slskd's originals, so they piled up in the download folder. (copy, not move, is correct for the torrent/usenet bundle paths — those clients keep seeding — so the shared copier can't just always delete.) Add an opt-in remove_source to copy_audio_files_atomically that deletes each source ONLY after it copies successfully (never on a failed stage), and set it for the Soulseek path only. Torrent/usenet keep their originals. Tests: keeps source by default / removes when requested / keeps on failed copy.
This commit is contained in:
parent
ef9af0cab5
commit
1590330171
4 changed files with 55 additions and 4 deletions
|
|
@ -706,13 +706,19 @@ def resolve_reported_save_path(
|
|||
|
||||
|
||||
def copy_audio_files_atomically(
|
||||
sources: Iterable[Path], staging_dir: Path,
|
||||
sources: Iterable[Path], staging_dir: Path, remove_source: bool = False,
|
||||
) -> list:
|
||||
"""Convenience wrapper: pick a non-colliding staging path for
|
||||
each source, copy via ``atomic_copy_to_staging``. Returns the
|
||||
list of final destination paths (as strings). Files that fail
|
||||
to copy are logged and skipped; the caller decides what to do
|
||||
with a partial result."""
|
||||
with a partial result.
|
||||
|
||||
``remove_source=True`` deletes each source AFTER it copies
|
||||
successfully — used by the Soulseek bundle path so slskd's
|
||||
completed downloads don't pile up in its download folder (#796).
|
||||
Kept False for torrent/usenet, whose clients must retain the
|
||||
originals (seeding / client-managed)."""
|
||||
staging_dir.mkdir(parents=True, exist_ok=True)
|
||||
out: list = []
|
||||
for src in sources:
|
||||
|
|
@ -720,6 +726,14 @@ def copy_audio_files_atomically(
|
|||
try:
|
||||
atomic_copy_to_staging(src, dest)
|
||||
out.append(str(dest))
|
||||
if remove_source:
|
||||
# Only after a verified copy — never lose data on a failed stage.
|
||||
try:
|
||||
Path(src).unlink()
|
||||
except FileNotFoundError:
|
||||
pass
|
||||
except Exception as e:
|
||||
logger.debug("[album_bundle] Could not remove staged source %s: %s", src, e)
|
||||
except Exception as e:
|
||||
logger.warning("[album_bundle] Failed to stage %s -> %s: %s", src, dest, e)
|
||||
return out
|
||||
|
|
|
|||
|
|
@ -1613,7 +1613,10 @@ class SoulseekClient(DownloadSourcePlugin):
|
|||
return result
|
||||
|
||||
_emit('staging', release=getattr(picked, 'album_title', folder_path) if picked else folder_path)
|
||||
copied = copy_audio_files_atomically(completed, Path(staging_dir))
|
||||
# remove_source=True: clean slskd's completed files once staged so they
|
||||
# don't pile up in the download folder (#796). Soulseek has no seeding,
|
||||
# unlike the torrent/usenet bundle paths which keep their originals.
|
||||
copied = copy_audio_files_atomically(completed, Path(staging_dir), remove_source=True)
|
||||
if not copied:
|
||||
result['error'] = 'No Soulseek album files copied to staging'
|
||||
return result
|
||||
|
|
|
|||
|
|
@ -360,6 +360,40 @@ def test_copy_audio_files_atomically_creates_staging_dir(tmp_path: Path) -> None
|
|||
assert staging.exists()
|
||||
|
||||
|
||||
def test_copy_audio_files_atomically_keeps_source_by_default(tmp_path: Path) -> None:
|
||||
"""Default (torrent/usenet): originals stay put (client keeps seeding)."""
|
||||
src = tmp_path / 'a.flac'
|
||||
src.write_bytes(b'a')
|
||||
staging = tmp_path / 'staging'
|
||||
out = copy_audio_files_atomically([src], staging)
|
||||
assert len(out) == 1
|
||||
assert src.exists() # source retained
|
||||
|
||||
|
||||
def test_copy_audio_files_atomically_removes_source_when_requested(tmp_path: Path) -> None:
|
||||
"""#796: Soulseek path removes slskd's completed files once staged so they
|
||||
don't pile up in the download folder."""
|
||||
src_a = tmp_path / 'a.flac'; src_a.write_bytes(b'a')
|
||||
src_c = tmp_path / 'c.flac'; src_c.write_bytes(b'c')
|
||||
staging = tmp_path / 'staging'
|
||||
out = copy_audio_files_atomically([src_a, src_c], staging, remove_source=True)
|
||||
assert len(out) == 2 # both staged
|
||||
assert not src_a.exists() and not src_c.exists() # sources removed
|
||||
assert sorted(Path(p).name for p in out) == ['a.flac', 'c.flac']
|
||||
|
||||
|
||||
def test_copy_audio_files_atomically_keeps_source_when_copy_fails(tmp_path: Path) -> None:
|
||||
"""Data safety: a source whose copy FAILS must never be deleted, even with
|
||||
remove_source=True."""
|
||||
src_ok = tmp_path / 'ok.flac'; src_ok.write_bytes(b'ok')
|
||||
src_missing = tmp_path / 'gone.flac' # never created -> copy fails
|
||||
staging = tmp_path / 'staging'
|
||||
out = copy_audio_files_atomically([src_ok, src_missing], staging, remove_source=True)
|
||||
assert len(out) == 1 # only ok staged
|
||||
assert not src_ok.exists() # staged source removed
|
||||
assert not src_missing.exists() # never existed (and not created)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Config-driven poll cadence
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -83,7 +83,7 @@ def test_healthy_folder_does_not_fall_back():
|
|||
# True. Patch the atomic copy to echo the completed paths through.
|
||||
from pathlib import Path
|
||||
completed = [Path("/staged/01.flac"), Path("/staged/02.flac")]
|
||||
with patch("core.soulseek_client.copy_audio_files_atomically", lambda files, dest: list(files)):
|
||||
with patch("core.soulseek_client.copy_audio_files_atomically", lambda files, dest, **kw: list(files)):
|
||||
stub = _Stub(completed)
|
||||
with patch("core.soulseek_client.run_async", lambda x: x):
|
||||
result = SoulseekClient.download_album_to_staging(
|
||||
|
|
|
|||
Loading…
Reference in a new issue