travandClaude Opus 5
6fad0247ee
Right-click a track (or a multi-selection) for "Remove from Library" or "Remove from Library and Delete File". The second moves the file to the desktop trash rather than unlinking it, so it stays recoverable by Ctrl+Z in-app and by "Restore" from the file manager afterwards. The trash is per-filesystem: music lives on a mounted volume, so the file belongs in <topdir>/.Trash-<uid> with a topdir-relative, percent-encoded Path. Using ~/.local/share/Trash would be a cross-device copy recording an original path "Restore" can't reach. lintunes/trash.py implements the freedesktop spec directly rather than adding a dependency that would need a manual reinstall on the other machine. Playlist cleanup is synchronous with the removal: playlist edits address tracks by row index into track_ids while the view skips ids missing from the library, so a dangling id would desync the two and make a later "Remove from Playlist" hit the wrong track. A file that can't be trashed keeps its track — better a song to delete again than a library entry orphaned from a file still on disk. Player gains drop_tracks() so deleting the playing track stops cleanly instead of erroring out when the queue walk later reaches a dead id. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
387 lines
15 KiB
Python
387 lines
15 KiB
Python
"""Round 33: deleting songs from the library.
|
|
|
|
Right-clicking a track (or a multi-selection) now offers "Remove from Library"
|
|
and "Remove from Library and Delete File". The second moves the file to the
|
|
desktop trash rather than unlinking it, so it stays recoverable both by Ctrl+Z
|
|
in-app and by "Restore" from the file manager afterwards.
|
|
|
|
Three things this covers that are easy to get wrong:
|
|
|
|
1. **The trash is per-filesystem.** Music usually sits on a mounted volume, not
|
|
the home filesystem, so the file belongs in ``<topdir>/.Trash-<uid>``. Putting
|
|
it in ``~/.local/share/Trash`` would be a cross-device copy and would record
|
|
an original path the desktop can't restore to.
|
|
2. **Playlist cleanup has to be synchronous.** Playlist edits address tracks by
|
|
row index into ``track_ids`` while the view skips ids missing from the
|
|
library, so a dangling id would desync the two and make a later "Remove from
|
|
Playlist" hit the wrong track.
|
|
3. **A file that can't be trashed must keep its track.** Better a song you have
|
|
to delete again than a library entry orphaned from a file still on disk.
|
|
"""
|
|
|
|
import os
|
|
from pathlib import Path
|
|
from unittest.mock import MagicMock
|
|
|
|
import pytest
|
|
|
|
from lintunes import trash
|
|
from lintunes.library_manager import LibraryManager
|
|
from lintunes.models import Library, Playlist, PlaylistType, Track
|
|
|
|
|
|
# --------------------------------------------------------------------------
|
|
# 1. lintunes/trash.py — the freedesktop Trash spec
|
|
# --------------------------------------------------------------------------
|
|
|
|
@pytest.fixture
|
|
def home_trash(tmp_path, monkeypatch):
|
|
"""Point XDG_DATA_HOME at tmp_path so the 'home' trash is under test."""
|
|
monkeypatch.setenv("XDG_DATA_HOME", str(tmp_path / "xdg"))
|
|
return tmp_path / "xdg" / "Trash"
|
|
|
|
|
|
def _info_for(trashed: Path) -> dict:
|
|
text = (trashed.parent.parent / "info" / f"{trashed.name}.trashinfo").read_text()
|
|
assert text.splitlines()[0] == "[Trash Info]"
|
|
return dict(line.split("=", 1) for line in text.splitlines()[1:] if "=" in line)
|
|
|
|
|
|
class TestTrashModule:
|
|
def test_file_lands_in_trash_with_info(self, tmp_path, home_trash):
|
|
song = tmp_path / "song.mp3"
|
|
song.write_bytes(b"audio")
|
|
|
|
dest = trash.send_to_trash(song)
|
|
|
|
assert not song.exists()
|
|
assert dest == home_trash / "files" / "song.mp3"
|
|
assert dest.read_bytes() == b"audio"
|
|
info = _info_for(dest)
|
|
# Home trash records an absolute path.
|
|
assert info["Path"] == str(song.resolve())
|
|
assert "T" in info["DeletionDate"]
|
|
|
|
def test_path_is_percent_encoded(self, tmp_path, home_trash):
|
|
song = tmp_path / "a song & more.mp3"
|
|
song.write_bytes(b"x")
|
|
info = _info_for(trash.send_to_trash(song))
|
|
assert "%20" in info["Path"] and " " not in info["Path"]
|
|
assert trash.original_path(
|
|
home_trash / "files" / "a song & more.mp3") == song.resolve()
|
|
|
|
def test_volume_trash_used_for_other_filesystem(self, tmp_path, monkeypatch):
|
|
"""A file on its own mount goes to <topdir>/.Trash-<uid> with a
|
|
topdir-relative Path, not to the home trash."""
|
|
volume = tmp_path / "volume"
|
|
(volume / "music").mkdir(parents=True)
|
|
song = volume / "music" / "song.mp3"
|
|
song.write_bytes(b"audio")
|
|
|
|
monkeypatch.setattr(os.path, "ismount", lambda p: Path(p) == volume)
|
|
real_stat = os.stat
|
|
monkeypatch.setattr(
|
|
os, "stat",
|
|
lambda p, *a, **k: (MagicMock(st_dev=99) if Path(p) == song
|
|
else real_stat(p, *a, **k)))
|
|
|
|
dest = trash.send_to_trash(song)
|
|
|
|
assert dest == volume / f".Trash-{os.getuid()}" / "files" / "song.mp3"
|
|
assert _info_for(dest)["Path"] == "music/song.mp3" # relative to topdir
|
|
|
|
def test_name_collision_gets_a_suffix(self, tmp_path, home_trash):
|
|
first = tmp_path / "song.mp3"
|
|
first.write_bytes(b"one")
|
|
trash.send_to_trash(first)
|
|
|
|
(tmp_path / "other").mkdir()
|
|
second = tmp_path / "other" / "song.mp3"
|
|
second.write_bytes(b"two")
|
|
dest = trash.send_to_trash(second)
|
|
|
|
assert dest.name == "song 1.mp3"
|
|
assert dest.read_bytes() == b"two"
|
|
assert (home_trash / "files" / "song.mp3").read_bytes() == b"one"
|
|
|
|
def test_restore_round_trip(self, tmp_path, home_trash):
|
|
song = tmp_path / "song.mp3"
|
|
song.write_bytes(b"audio")
|
|
dest = trash.send_to_trash(song)
|
|
|
|
trash.restore_from_trash(dest, song)
|
|
|
|
assert song.read_bytes() == b"audio"
|
|
assert not dest.exists()
|
|
# The .trashinfo must go too, or the name stays claimed forever.
|
|
assert not (home_trash / "info" / "song.mp3.trashinfo").exists()
|
|
|
|
def test_failed_move_leaves_no_orphan_info(self, tmp_path, home_trash,
|
|
monkeypatch):
|
|
song = tmp_path / "song.mp3"
|
|
song.write_bytes(b"audio")
|
|
|
|
def boom(*a, **k):
|
|
raise OSError("disk on fire")
|
|
monkeypatch.setattr(os, "rename", boom)
|
|
|
|
with pytest.raises(trash.TrashError):
|
|
trash.send_to_trash(song)
|
|
|
|
assert song.exists() # caller can still treat it as un-deleted
|
|
assert list((home_trash / "info").iterdir()) == []
|
|
|
|
def test_missing_file_raises(self, tmp_path, home_trash):
|
|
with pytest.raises(trash.TrashError):
|
|
trash.send_to_trash(tmp_path / "nope.mp3")
|
|
|
|
|
|
# --------------------------------------------------------------------------
|
|
# 2. LibraryManager.delete_tracks
|
|
# --------------------------------------------------------------------------
|
|
|
|
def _library(tmp_path, count=3):
|
|
"""A library whose tracks are real files under an organized music tree."""
|
|
music = tmp_path / "music"
|
|
library = Library(music_folder=str(music))
|
|
for tid in range(1, count + 1):
|
|
path = music / "Music" / f"Artist{tid}" / "Album" / f"T{tid}.mp3"
|
|
path.parent.mkdir(parents=True, exist_ok=True)
|
|
path.write_bytes(b"audio" + str(tid).encode())
|
|
library.tracks[tid] = Track(track_id=tid, name=f"T{tid}",
|
|
artist=f"Artist{tid}", location=str(path))
|
|
return library
|
|
|
|
|
|
def _manager(tmp_path, library):
|
|
return LibraryManager(library, tmp_path / "data")
|
|
|
|
|
|
def _playlist(library, pid, track_ids, name="Mix"):
|
|
library.playlists[pid] = Playlist(
|
|
name=name, persistent_id=pid, playlist_type=PlaylistType.REGULAR,
|
|
track_ids=list(track_ids))
|
|
return library.playlists[pid]
|
|
|
|
|
|
class TestDeleteTracks:
|
|
def test_removes_from_library_and_playlists(self, qapp, tmp_path, home_trash):
|
|
library = _library(tmp_path)
|
|
playlist = _playlist(library, "AAAA1111", [1, 2, 3])
|
|
manager = _manager(tmp_path, library)
|
|
|
|
removed, failures = manager.delete_tracks([2], delete_files=True)
|
|
|
|
assert removed == [2] and failures == []
|
|
assert 2 not in manager.library.tracks
|
|
# No dangling id: stored rows must match what the view will show.
|
|
assert playlist.track_ids == [1, 3]
|
|
|
|
def test_file_goes_to_the_trash(self, qapp, tmp_path, home_trash):
|
|
library = _library(tmp_path)
|
|
path = Path(library.tracks[1].location)
|
|
manager = _manager(tmp_path, library)
|
|
|
|
manager.delete_tracks([1], delete_files=True)
|
|
|
|
assert not path.exists()
|
|
assert (home_trash / "files" / "T1.mp3").read_bytes() == b"audio1"
|
|
|
|
def test_keep_file_variant_leaves_it_alone(self, qapp, tmp_path, home_trash):
|
|
library = _library(tmp_path)
|
|
path = Path(library.tracks[1].location)
|
|
manager = _manager(tmp_path, library)
|
|
|
|
manager.delete_tracks([1], delete_files=False)
|
|
|
|
assert 1 not in manager.library.tracks
|
|
assert path.exists()
|
|
assert not (home_trash / "files").exists()
|
|
|
|
def test_emptied_dirs_pruned_but_not_the_root(self, qapp, tmp_path, home_trash):
|
|
library = _library(tmp_path)
|
|
album = Path(library.tracks[1].location).parent
|
|
manager = _manager(tmp_path, library)
|
|
|
|
manager.delete_tracks([1], delete_files=True)
|
|
|
|
assert not album.exists() and not album.parent.exists()
|
|
assert manager.organize_root().exists()
|
|
|
|
def test_trash_failure_keeps_the_track(self, qapp, tmp_path, home_trash,
|
|
monkeypatch):
|
|
library = _library(tmp_path)
|
|
manager = _manager(tmp_path, library)
|
|
monkeypatch.setattr(trash, "send_to_trash",
|
|
MagicMock(side_effect=trash.TrashError("nope")))
|
|
|
|
removed, failures = manager.delete_tracks([1], delete_files=True)
|
|
|
|
assert removed == []
|
|
assert 1 in manager.library.tracks # never orphan the entry
|
|
assert failures and failures[0][0] == "T1"
|
|
|
|
def test_partial_failure_still_deletes_the_rest(self, qapp, tmp_path,
|
|
home_trash, monkeypatch):
|
|
library = _library(tmp_path)
|
|
manager = _manager(tmp_path, library)
|
|
real = trash.send_to_trash
|
|
|
|
def flaky(path):
|
|
if Path(path).name == "T1.mp3":
|
|
raise trash.TrashError("nope")
|
|
return real(path)
|
|
monkeypatch.setattr(trash, "send_to_trash", flaky)
|
|
|
|
removed, failures = manager.delete_tracks([1, 2], delete_files=True)
|
|
|
|
assert removed == [2] and len(failures) == 1
|
|
assert 1 in manager.library.tracks and 2 not in manager.library.tracks
|
|
|
|
def test_unknown_ids_are_ignored(self, qapp, tmp_path, home_trash):
|
|
manager = _manager(tmp_path, _library(tmp_path))
|
|
assert manager.delete_tracks([999], delete_files=True) == ([], [])
|
|
|
|
def test_emits_tracks_removed(self, qapp, tmp_path, home_trash):
|
|
manager = _manager(tmp_path, _library(tmp_path))
|
|
seen = []
|
|
manager.tracks_removed.connect(seen.append)
|
|
|
|
manager.delete_tracks([1, 3], delete_files=False)
|
|
|
|
assert seen == [[1, 3]]
|
|
|
|
|
|
class TestDeleteUndo:
|
|
def test_undo_restores_track_file_and_playlist_position(
|
|
self, qapp, tmp_path, home_trash):
|
|
library = _library(tmp_path)
|
|
playlist = _playlist(library, "AAAA1111", [1, 2, 3])
|
|
path = Path(library.tracks[2].location)
|
|
manager = _manager(tmp_path, library)
|
|
|
|
manager.delete_tracks([2], delete_files=True)
|
|
manager.undo_stack.undo()
|
|
|
|
assert 2 in manager.library.tracks
|
|
assert path.read_bytes() == b"audio2"
|
|
# Back in the middle, not appended to the end.
|
|
assert playlist.track_ids == [1, 2, 3]
|
|
|
|
def test_redo_deletes_again(self, qapp, tmp_path, home_trash):
|
|
library = _library(tmp_path)
|
|
playlist = _playlist(library, "AAAA1111", [1, 2, 3])
|
|
path = Path(library.tracks[2].location)
|
|
manager = _manager(tmp_path, library)
|
|
|
|
manager.delete_tracks([2], delete_files=True)
|
|
manager.undo_stack.undo()
|
|
manager.undo_stack.redo()
|
|
|
|
assert 2 not in manager.library.tracks
|
|
assert not path.exists()
|
|
assert playlist.track_ids == [1, 3]
|
|
|
|
def test_undo_label_reflects_the_variant(self, qapp, tmp_path, home_trash):
|
|
manager = _manager(tmp_path, _library(tmp_path))
|
|
manager.delete_tracks([1], delete_files=True)
|
|
assert manager.undo_stack.undo_label() == "Delete from Library"
|
|
|
|
manager.delete_tracks([2], delete_files=False)
|
|
assert manager.undo_stack.undo_label() == "Remove from Library"
|
|
|
|
def test_undo_restores_entry_even_if_trash_was_emptied(
|
|
self, qapp, tmp_path, home_trash):
|
|
library = _library(tmp_path)
|
|
manager = _manager(tmp_path, library)
|
|
manager.delete_tracks([1], delete_files=True)
|
|
(home_trash / "files" / "T1.mp3").unlink() # user emptied the trash
|
|
|
|
manager.undo_stack.undo()
|
|
|
|
# The file is unrecoverable, but losing the library entry too would be
|
|
# a second, silent loss.
|
|
assert 1 in manager.library.tracks
|
|
|
|
def test_deleted_track_is_gone_from_saved_library(self, qapp, tmp_path,
|
|
home_trash):
|
|
library = _library(tmp_path)
|
|
manager = _manager(tmp_path, library)
|
|
manager.delete_tracks([1], delete_files=True)
|
|
manager.flush()
|
|
|
|
import json
|
|
saved = json.loads((tmp_path / "data" / "library.json").read_text())
|
|
assert "1" not in saved and "2" in saved
|
|
|
|
|
|
# --------------------------------------------------------------------------
|
|
# 3. Player.drop_tracks
|
|
# --------------------------------------------------------------------------
|
|
|
|
def _player(qapp, track_ids):
|
|
from lintunes.player import Player
|
|
library = Library()
|
|
for tid in track_ids:
|
|
library.tracks[tid] = Track(track_id=tid, name=f"T{tid}")
|
|
manager = MagicMock()
|
|
manager.library = library
|
|
player = Player(manager)
|
|
player._queue = list(track_ids)
|
|
return player
|
|
|
|
|
|
class TestPlayerDropTracks:
|
|
def test_stops_when_the_playing_track_is_deleted(self, qapp):
|
|
player = _player(qapp, [1, 2, 3])
|
|
player._index = 1
|
|
player._current_track = player._manager.library.tracks[2]
|
|
player._sink = MagicMock()
|
|
|
|
player.drop_tracks({2})
|
|
|
|
assert player.current_track is None
|
|
assert player._queue == [1, 3]
|
|
|
|
def test_current_track_keeps_its_place(self, qapp):
|
|
player = _player(qapp, [1, 2, 3])
|
|
player._index = 2
|
|
player._current_track = player._manager.library.tracks[3]
|
|
|
|
player.drop_tracks({1})
|
|
|
|
# Track 3 moved from index 2 to index 1; a stale index would make
|
|
# "next" replay the wrong song.
|
|
assert player._queue == [2, 3]
|
|
assert player._index == 1
|
|
assert player.current_track.track_id == 3
|
|
|
|
def test_empty_set_is_a_no_op(self, qapp):
|
|
player = _player(qapp, [1, 2, 3])
|
|
player._index = 1
|
|
player.drop_tracks(set())
|
|
assert player._queue == [1, 2, 3] and player._index == 1
|
|
|
|
|
|
# --------------------------------------------------------------------------
|
|
# 4. The context-menu signals
|
|
# --------------------------------------------------------------------------
|
|
|
|
class TestContextMenuSignals:
|
|
@pytest.mark.parametrize("playlist_mode", [False, True])
|
|
def test_both_actions_exist_in_both_modes(self, qapp, playlist_mode):
|
|
"""Unlike "Remove from Playlist", library removal is offered in the
|
|
library view too."""
|
|
from lintunes.gui.track_table import TrackTableView
|
|
table = TrackTableView(playlist_mode=playlist_mode)
|
|
assert hasattr(table, "remove_from_library_requested")
|
|
assert hasattr(table, "delete_from_library_requested")
|
|
|
|
def test_signals_carry_track_ids(self, qapp):
|
|
from lintunes.gui.track_table import TrackTableView
|
|
table = TrackTableView()
|
|
seen = []
|
|
table.delete_from_library_requested.connect(seen.append)
|
|
table.delete_from_library_requested.emit([7, 9])
|
|
assert seen == [[7, 9]]
|