Prune slskd dedup orphans after import
slskd appends "_<19-digit unix-nanosecond timestamp>" to a downloaded filename when the destination already contains a same-named file (concurrent downloads of the same track, partial-file retries after a connection drop, cancelled-then-redownloaded files, the same track surfacing in multiple synced playlists). The file-finder code already recognized the suffix when matching a download to its source — but after the canonical file moved into the library, the leftover "_<timestamp>" siblings sat orphaned in the downloads folder forever. Reported on Discord by Shdjfgatdif. cleanup_slskd_dedup_siblings() runs at the end of each successful import (3 safe_move_file sites in pipeline.py) and prunes any remaining siblings that strip down to the canonical stem with the same extension. Conservative match (>= 18 trailing digits) keeps legitimate filenames like "Track 5" and "Album 1995" untouched. Per- file unlink failures are swallowed so a single locked file doesn't block the rest. 17 regression tests cover the suffix-strip primitive, orphan removal, no-op cases, mismatched extensions, subdirectories, and partial-failure recovery.
This commit is contained in:
Broque Thomas
parent
94b08bbb49
commit
46d8e15674
4 changed files with 281 additions and 0 deletions
|
|
@ -4,16 +4,104 @@ from __future__ import annotations
|
|||
|
||||
import logging
|
||||
import os
|
||||
import re
|
||||
import shutil
|
||||
import subprocess
|
||||
import time
|
||||
from pathlib import Path
|
||||
from typing import Iterable, List
|
||||
|
||||
from config.settings import config_manager
|
||||
|
||||
logger = logging.getLogger("imports.file_ops")
|
||||
|
||||
|
||||
# slskd appends "_<19-digit unix-nanosecond timestamp>" to a downloaded
|
||||
# filename when the destination already contains a file with the same
|
||||
# name (concurrent downloads of the same track, partial-file retries
|
||||
# after a connection drop, cancelled-then-redownloaded files, the same
|
||||
# track surfacing in multiple synced playlists, etc.). The original
|
||||
# canonical file usually gets imported and moved into the library while
|
||||
# the timestamp-suffixed siblings sit orphaned in the downloads folder
|
||||
# forever. Match the suffix conservatively (≥ 18 digits) so genuine
|
||||
# user filenames containing trailing numbers don't get hit.
|
||||
_SLSKD_DEDUP_SUFFIX_RE = re.compile(r"_\d{18,}$")
|
||||
|
||||
|
||||
def _strip_slskd_dedup_suffix(stem: str) -> str:
|
||||
"""Return the canonical stem with any slskd dedup suffix removed."""
|
||||
return _SLSKD_DEDUP_SUFFIX_RE.sub("", stem)
|
||||
|
||||
|
||||
def cleanup_slskd_dedup_siblings(source_path) -> List[str]:
|
||||
"""Remove orphan ``<basename>_<timestamp>.<ext>`` siblings of a just-
|
||||
imported file from the source directory.
|
||||
|
||||
Call this AFTER a successful import (the canonical file has already
|
||||
moved away) using the path the canonical file came from. Looks at
|
||||
siblings in the same directory whose stem, with the slskd dedup
|
||||
suffix stripped, equals the imported file's canonical stem and the
|
||||
same extension. Deletes them.
|
||||
|
||||
Returns the list of deleted paths so the caller can log a summary.
|
||||
Failures (permissions, racing reader, etc.) are swallowed
|
||||
individually so a single locked file doesn't block the rest of the
|
||||
cleanup.
|
||||
"""
|
||||
source = Path(source_path)
|
||||
parent = source.parent
|
||||
if not parent.is_dir():
|
||||
return []
|
||||
|
||||
canonical_name = source.name
|
||||
canonical_stem, canonical_ext = os.path.splitext(canonical_name)
|
||||
# If the imported file ITSELF already had a dedup suffix, the
|
||||
# "canonical" name is the stripped form — every other sibling that
|
||||
# also strips down to it is redundant.
|
||||
canonical_stem = _strip_slskd_dedup_suffix(canonical_stem)
|
||||
|
||||
deleted: List[str] = []
|
||||
try:
|
||||
children: Iterable[Path] = list(parent.iterdir())
|
||||
except OSError as e:
|
||||
logger.debug(f"[Dedup Cleanup] could not list {parent}: {e}")
|
||||
return []
|
||||
|
||||
for sibling in children:
|
||||
if not sibling.is_file():
|
||||
continue
|
||||
# Skip the imported file itself if it's still on disk (it
|
||||
# shouldn't be — caller invokes us after the move — but the
|
||||
# check is cheap and keeps the function safe to call from
|
||||
# other contexts later).
|
||||
if sibling.name == canonical_name:
|
||||
continue
|
||||
sib_stem, sib_ext = os.path.splitext(sibling.name)
|
||||
if sib_ext.lower() != canonical_ext.lower():
|
||||
continue
|
||||
sib_canonical_stem = _strip_slskd_dedup_suffix(sib_stem)
|
||||
if sib_canonical_stem != canonical_stem:
|
||||
continue
|
||||
# Defensive: don't delete a file that doesn't actually carry
|
||||
# the slskd dedup suffix — that would imply it's a legitimate
|
||||
# different file the user intentionally placed there.
|
||||
if sib_stem == sib_canonical_stem:
|
||||
continue
|
||||
try:
|
||||
sibling.unlink()
|
||||
deleted.append(str(sibling))
|
||||
except OSError as e:
|
||||
logger.debug(f"[Dedup Cleanup] could not remove {sibling}: {e}")
|
||||
|
||||
if deleted:
|
||||
logger.info(
|
||||
"[Dedup Cleanup] removed %d slskd dedup orphan(s) for %r",
|
||||
len(deleted),
|
||||
canonical_name,
|
||||
)
|
||||
return deleted
|
||||
|
||||
|
||||
def safe_move_file(src, dst):
|
||||
"""Move a file safely across filesystems."""
|
||||
src = Path(src)
|
||||
|
|
|
|||
|
|
@ -12,6 +12,7 @@ from typing import Any
|
|||
from config.settings import config_manager
|
||||
from core.imports.file_ops import (
|
||||
cleanup_empty_directories,
|
||||
cleanup_slskd_dedup_siblings,
|
||||
create_lossy_copy,
|
||||
downsample_hires_flac,
|
||||
get_audio_quality_string,
|
||||
|
|
@ -238,6 +239,7 @@ def post_process_matched_download(context_key, context, file_path, runtime, meta
|
|||
|
||||
safe_move_file(file_path, destination)
|
||||
logger.info(f"Moved simple download to: {destination}")
|
||||
cleanup_slskd_dedup_siblings(file_path)
|
||||
|
||||
with matched_context_lock:
|
||||
if context_key in matched_downloads_context:
|
||||
|
|
@ -402,6 +404,7 @@ def post_process_matched_download(context_key, context, file_path, runtime, meta
|
|||
logger.info(f"Moving '{os.path.basename(file_path)}' to '{final_path}'")
|
||||
safe_move_file(file_path, final_path)
|
||||
context['_final_processed_path'] = final_path
|
||||
cleanup_slskd_dedup_siblings(file_path)
|
||||
|
||||
if config_manager.get('post_processing.replaygain_enabled', False):
|
||||
try:
|
||||
|
|
@ -665,6 +668,7 @@ def post_process_matched_download(context_key, context, file_path, runtime, meta
|
|||
raise FileNotFoundError(f"Source file vanished before move and destination does not exist: {file_path}")
|
||||
|
||||
safe_move_file(file_path, final_path)
|
||||
cleanup_slskd_dedup_siblings(file_path)
|
||||
|
||||
if is_enhance_download and _enhance_source_info.get('original_file_path'):
|
||||
original_enhance_path = _enhance_source_info['original_file_path']
|
||||
|
|
|
|||
188
tests/imports/test_dedup_orphan_cleanup.py
Normal file
188
tests/imports/test_dedup_orphan_cleanup.py
Normal file
|
|
@ -0,0 +1,188 @@
|
|||
"""Regression tests for slskd dedup-suffix orphan cleanup.
|
||||
|
||||
Discord-reported (Shdjfgatdif): the downloads folder fills up with
|
||||
files like ``Song_639067852665564677.flac`` over time. slskd appends
|
||||
``_<19-digit unix-nanosecond timestamp>`` to a filename when the
|
||||
destination already contains a same-named file (concurrent downloads
|
||||
of the same track, partial-file retries after a connection drop,
|
||||
cancelled-then-redownloaded files, the same track surfacing in
|
||||
multiple synced playlists, etc.).
|
||||
|
||||
The file-finder code already RECOGNIZES the suffix when matching a
|
||||
download to its source. But after the canonical file is moved into
|
||||
the library, the leftover ``_<timestamp>`` siblings sat orphaned in
|
||||
the downloads folder forever. ``cleanup_slskd_dedup_siblings`` runs
|
||||
at the end of each successful import and prunes them.
|
||||
"""
|
||||
|
||||
import os
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
from core.imports.file_ops import (
|
||||
_strip_slskd_dedup_suffix,
|
||||
cleanup_slskd_dedup_siblings,
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Suffix-strip primitive
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"stem,expected",
|
||||
[
|
||||
("Song", "Song"),
|
||||
("Song_639067852665564677", "Song"), # 18 digits → match
|
||||
("Song_6390678526655646777", "Song"), # 19 digits → match
|
||||
("Song_63906785266556467770", "Song"), # 20 digits → match
|
||||
("Song_12345", "Song_12345"), # short → leave alone
|
||||
("Track 5", "Track 5"), # legitimate trailing digits → leave alone
|
||||
("Album 1995", "Album 1995"), # year suffix → leave alone
|
||||
("Mix_2024_639067852665564677", "Mix_2024"), # only the slskd suffix is stripped
|
||||
],
|
||||
)
|
||||
def test_strip_slskd_dedup_suffix(stem, expected) -> None:
|
||||
assert _strip_slskd_dedup_suffix(stem) == expected
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Orphan cleanup
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_removes_orphan_siblings_after_canonical_imported(tmp_path: Path) -> None:
|
||||
"""The reported scenario: canonical ``Song.flac`` was just imported
|
||||
(moved out of the downloads folder), and ``Song_<timestamp>.flac``
|
||||
siblings should be deleted."""
|
||||
canonical = tmp_path / "Song.flac"
|
||||
# Canonical file is GONE — caller invokes us after the move
|
||||
sibling_a = tmp_path / "Song_639067852665564677.flac"
|
||||
sibling_b = tmp_path / "Song_639067852665564999.flac"
|
||||
sibling_a.write_bytes(b"orphan a")
|
||||
sibling_b.write_bytes(b"orphan b")
|
||||
|
||||
deleted = cleanup_slskd_dedup_siblings(canonical)
|
||||
|
||||
assert len(deleted) == 2
|
||||
assert not sibling_a.exists()
|
||||
assert not sibling_b.exists()
|
||||
|
||||
|
||||
def test_does_not_touch_files_with_different_canonical_stem(tmp_path: Path) -> None:
|
||||
"""A sibling that strips down to a DIFFERENT canonical stem belongs
|
||||
to a different track and must not be deleted."""
|
||||
canonical = tmp_path / "Song.flac"
|
||||
other_track = tmp_path / "OtherSong_639067852665564677.flac"
|
||||
other_track.write_bytes(b"different track")
|
||||
|
||||
deleted = cleanup_slskd_dedup_siblings(canonical)
|
||||
assert deleted == []
|
||||
assert other_track.exists()
|
||||
|
||||
|
||||
def test_does_not_touch_files_with_different_extension(tmp_path: Path) -> None:
|
||||
"""Same canonical stem but different extension is a different
|
||||
file (e.g. an .mp3 next to a .flac). Don't cross-delete."""
|
||||
canonical = tmp_path / "Song.flac"
|
||||
different_ext = tmp_path / "Song_639067852665564677.mp3"
|
||||
different_ext.write_bytes(b"different format")
|
||||
|
||||
deleted = cleanup_slskd_dedup_siblings(canonical)
|
||||
assert deleted == []
|
||||
assert different_ext.exists()
|
||||
|
||||
|
||||
def test_does_not_touch_files_without_dedup_suffix(tmp_path: Path) -> None:
|
||||
"""A neighbouring file that happens to share the canonical stem
|
||||
but doesn't have a slskd dedup suffix is a legitimate user file —
|
||||
leave it alone, even though stripping it would match."""
|
||||
canonical = tmp_path / "Song.flac"
|
||||
legit = tmp_path / "Song.flac" # If it existed, the canonical wouldn't be here
|
||||
# Actually use a different shape — a file that strips to the same
|
||||
# canonical stem but has no suffix at all
|
||||
legit = tmp_path / "Song.flac" # Same as canonical name
|
||||
# We're running cleanup AFTER the move so the canonical itself is gone.
|
||||
# But guard against any case where it's still on disk for some reason.
|
||||
legit.write_bytes(b"still here")
|
||||
|
||||
deleted = cleanup_slskd_dedup_siblings(canonical)
|
||||
assert deleted == []
|
||||
assert legit.exists()
|
||||
|
||||
|
||||
def test_handles_canonical_file_that_itself_had_suffix(tmp_path: Path) -> None:
|
||||
"""If the imported file ITSELF had a slskd dedup suffix (because
|
||||
slskd renamed our preferred copy when an earlier download landed
|
||||
first), the cleanup must still find sibling orphans by stripping
|
||||
suffixes on both sides for comparison."""
|
||||
canonical = tmp_path / "Song_639000000000000000.flac" # The one we imported
|
||||
other = tmp_path / "Song_639067852665564677.flac"
|
||||
other.write_bytes(b"other orphan")
|
||||
|
||||
deleted = cleanup_slskd_dedup_siblings(canonical)
|
||||
assert len(deleted) == 1
|
||||
assert not other.exists()
|
||||
|
||||
|
||||
def test_returns_empty_list_when_directory_does_not_exist(tmp_path: Path) -> None:
|
||||
"""Defensive: caller passes a path whose parent dir was already
|
||||
cleaned up (e.g. another worker pruned the empty folder). Must
|
||||
not raise."""
|
||||
missing = tmp_path / "no" / "such" / "dir" / "Song.flac"
|
||||
deleted = cleanup_slskd_dedup_siblings(missing)
|
||||
assert deleted == []
|
||||
|
||||
|
||||
def test_returns_empty_list_when_no_orphans_exist(tmp_path: Path) -> None:
|
||||
"""The common case after most imports: nothing to clean up. Must
|
||||
return [] without errors."""
|
||||
canonical = tmp_path / "Song.flac"
|
||||
# Pre-existing unrelated files in the same directory
|
||||
(tmp_path / "TotallyDifferent.flac").write_bytes(b"unrelated")
|
||||
(tmp_path / "AnotherTrack.mp3").write_bytes(b"unrelated")
|
||||
|
||||
deleted = cleanup_slskd_dedup_siblings(canonical)
|
||||
assert deleted == []
|
||||
|
||||
|
||||
def test_skips_subdirectories(tmp_path: Path) -> None:
|
||||
"""A subdirectory whose name happens to match the dedup pattern
|
||||
must not be deleted — only files."""
|
||||
canonical = tmp_path / "Song.flac"
|
||||
subdir = tmp_path / "Song_639067852665564677.flac"
|
||||
subdir.mkdir() # Subdirectory matching the orphan filename pattern
|
||||
|
||||
deleted = cleanup_slskd_dedup_siblings(canonical)
|
||||
assert deleted == []
|
||||
assert subdir.exists()
|
||||
assert subdir.is_dir()
|
||||
|
||||
|
||||
def test_continues_after_individual_unlink_failure(tmp_path: Path, monkeypatch) -> None:
|
||||
"""A locked file must not block cleanup of the rest. Replace
|
||||
Path.unlink with a function that fails on a specific path and
|
||||
succeeds otherwise."""
|
||||
canonical = tmp_path / "Song.flac"
|
||||
locked = tmp_path / "Song_639067852665564677.flac"
|
||||
cleanable = tmp_path / "Song_639067852665564999.flac"
|
||||
locked.write_bytes(b"locked")
|
||||
cleanable.write_bytes(b"cleanable")
|
||||
|
||||
real_unlink = Path.unlink
|
||||
|
||||
def fake_unlink(self):
|
||||
if self.name == "Song_639067852665564677.flac":
|
||||
raise PermissionError("locked")
|
||||
return real_unlink(self)
|
||||
|
||||
monkeypatch.setattr(Path, "unlink", fake_unlink)
|
||||
|
||||
deleted = cleanup_slskd_dedup_siblings(canonical)
|
||||
|
||||
assert len(deleted) == 1
|
||||
assert "Song_639067852665564999.flac" in deleted[0]
|
||||
# The locked one stays
|
||||
assert locked.exists()
|
||||
|
|
@ -3452,6 +3452,7 @@ const WHATS_NEW = {
|
|||
{ title: 'Stats Endpoints Lifted to core/stats', desc: 'internal — moved /api/stats/* and /api/listening-stats/* logic out of web_server.py into core/stats/queries.py with full test coverage. no behavior change. step toward breaking up the web_server.py monolith.' },
|
||||
{ title: 'Search Endpoints Lifted to core/search', desc: 'internal — moved /api/search and /api/enhanced-search/* logic into core/search/ (cache, sources, library_check, stream, basic, orchestrator). 612 fewer lines in web_server.py, 94 new tests. no behavior change.' },
|
||||
{ title: 'Automation Endpoints Lifted to core/automation', desc: 'internal — moved /api/automations/* CRUD + run + history routes, progress tracking helpers, and signal collection into core/automation/ (api, progress, signals). 383 fewer lines in web_server.py, 72 new tests. action handler registration stays put — those closures are tangled with feature implementations.' },
|
||||
{ title: 'Clean Up slskd Dedup Orphans After Import', desc: 'slskd appends "_<timestamp>" to a download when the destination file already exists (e.g. retried partials, the same track in multiple playlists). the canonical file imported fine but the timestamp-suffixed siblings sat in the downloads folder forever. now they get pruned right after each successful import.', page: 'downloads' },
|
||||
],
|
||||
'2.4.0': [
|
||||
// --- April 26, 2026 — Search & Artists unification + reorganize queue ---
|
||||
|
|
|
|||
Loading…
Reference in a new issue