diff --git a/TASKS.md b/TASKS.md index 2e94149..205611f 100644 --- a/TASKS.md +++ b/TASKS.md @@ -24,6 +24,20 @@ When a round closes, move its finished items to `tasks-done.md`. # old +## Round 28 — cancelable device sync (v0.3.0) + +- [x] "✕" button left of the sync progress group; expands to "cancel transfer" + on hover; confirm dialog (Cancel Transfer / Keep Copying) before cancelling. +- [x] `DeviceSyncWorker.cancel()`: stops at the next chunk boundary, removes + the in-flight partial file, emits `cancelled` with copied/total counts. +- [x] Cancelled syncs always leave a coherent device folder: the m3u is + rewritten to list only tracks actually present (fixes the pre-existing gap + where stale files deleted before a cancel could leave dangling m3u refs). +- [x] Recoverability verified: cancel-then-resync produces byte-identical + results to an uninterrupted sync (test + live run against the real Rabbit). + +Tests in `tests/test_round28.py`. Feature round → minor bump **0.3.0**. + ## Round 27 — device-sync guards (v0.2.2) - [x] Quit warning while a transfer is copying (closeEvent; covers X button, diff --git a/lintunes/__init__.py b/lintunes/__init__.py index e360eba..fab3560 100644 --- a/lintunes/__init__.py +++ b/lintunes/__init__.py @@ -1,3 +1,3 @@ """LinTunes — iTunes-style music library manager and player for Linux.""" -__version__ = "0.2.2" +__version__ = "0.3.0" diff --git a/lintunes/device_sync.py b/lintunes/device_sync.py index e2a700f..234b995 100644 --- a/lintunes/device_sync.py +++ b/lintunes/device_sync.py @@ -192,26 +192,45 @@ def build_m3u(entries: list) -> str: return "\n".join(lines) + "\n" +class _Cancelled(Exception): + """Internal: unwinds _run when cancel() was requested.""" + + class DeviceSyncWorker(QObject): """Runs a SyncPlan on a daemon thread, reporting through signals. Progress is emitted in KiB (a pyqtSignal(int) is a C int — byte counts overflow past 2 GiB). An unplug mid-copy surfaces as `failed`; the next sync self-heals because the partial file loses the name+size diff. + + `cancel()` (thread-safe, no-op once the run ended) stops the transfer at + the next chunk boundary: the in-flight partial file is removed and the + m3u is rewritten to list only tracks actually on the device, so a + cancelled sync always leaves a coherent (if partial) playlist that the + next sync completes. """ progress = pyqtSignal(int, int, str) # done_kib, total_kib, "12/240 Artist - Title" finished = pyqtSignal(dict) + cancelled = pyqtSignal(dict) failed = pyqtSignal(str) def __init__(self, plan: SyncPlan, parent=None): super().__init__(parent) self._plan = plan self._busy = False + self._cancel = threading.Event() def busy(self) -> bool: return self._busy + def cancel(self): + self._cancel.set() + + def _check_cancel(self): + if self._cancel.is_set(): + raise _Cancelled + def start(self): if self._busy: return @@ -226,11 +245,16 @@ class DeviceSyncWorker(QObject): def _run(self): plan = self._plan + device_name = plan.device.name if plan.device else "device" dest = None + removed = 0 + completed: set[str] = set() + vanished: set[str] = set() try: + self._check_cancel() plan.dest_dir.mkdir(parents=True, exist_ok=True) - removed = 0 for name in plan.stale: + self._check_cancel() try: (plan.dest_dir / name).unlink() removed += 1 @@ -238,8 +262,8 @@ class DeviceSyncWorker(QObject): pass # one stubborn file shouldn't kill the sync total_kib = max(plan.bytes_to_copy // 1024, 1) done = 0 - vanished: set[str] = set() for i, item in enumerate(plan.copies, start=1): + self._check_cancel() label = f"{i}/{len(plan.copies)} {Path(item.dest_name).stem}" # A source gone since planning means Syncthing moved/deleted # it under us (e.g. a metadata edit on the other machine @@ -264,26 +288,49 @@ class DeviceSyncWorker(QObject): fdst.write(chunk) done += len(chunk) self.progress.emit(done // 1024, total_kib, label) + self._check_cancel() dest = None + completed.add(item.dest_name) # Written last so an interrupted sync leaves the old m3u intact. # Vanished tracks are left out so the m3u only lists files that # are really there; the next sync picks up their new locations. entries = [e for e in plan.entries if e[0] not in vanished] (plan.dest_dir / plan.m3u_name).write_text( build_m3u(entries), encoding="utf-8") - device_name = plan.device.name if plan.device else "device" self.finished.emit({ "playlist": plan.playlist_name, "device": device_name, "copied": len(plan.copies) - len(vanished), "kept": plan.kept, "removed": removed, "skipped": plan.skipped, "vanished": len(vanished), }) + except _Cancelled: + if dest is not None: + try: + dest.unlink() # the mid-flight partial file + except OSError: + pass + # Leave a coherent playlist: an m3u of only what is actually on + # the device now (kept files + copies that fully landed). The + # next sync completes the rest. + not_copied = ({item.dest_name for item in plan.copies} + - completed) + entries = [e for e in plan.entries + if e[0] not in vanished and e[0] not in not_copied] + try: + (plan.dest_dir / plan.m3u_name).write_text( + build_m3u(entries), encoding="utf-8") + except OSError: + pass # device going away; the size diff self-heals anyway + self.cancelled.emit({ + "playlist": plan.playlist_name, "device": device_name, + "copied": len(completed), "total": len(plan.copies), + "removed": removed, + }) except OSError as e: if dest is not None: try: dest.unlink() # drop the half-copied file if the mount survives except OSError: pass - device_name = plan.device.name if plan.device else "device" self.failed.emit( f"Sync failed: {e}. Is the {device_name} still connected?") diff --git a/lintunes/gui/main_window.py b/lintunes/gui/main_window.py index 8c4d1ff..1814fba 100644 --- a/lintunes/gui/main_window.py +++ b/lintunes/gui/main_window.py @@ -4,7 +4,7 @@ from pathlib import Path from PyQt6.QtWidgets import ( QMainWindow, QSplitter, QStackedWidget, QWidget, QVBoxLayout, QLineEdit, QPlainTextEdit, QTextEdit, QAbstractSpinBox, QComboBox, QApplication, - QLabel, QMessageBox, QFileDialog, QProgressBar, + QLabel, QMessageBox, QFileDialog, QProgressBar, QPushButton, ) from PyQt6.QtCore import Qt, QEvent, QTimer from PyQt6.QtGui import QAction, QKeySequence @@ -37,6 +37,32 @@ def album_tracks(library, artist: str, album: str) -> list: and (t.album_artist or t.artist).casefold() == artist_cf] +class _CancelSyncButton(QPushButton): + """Compact "✕" beside the sync progress bar that expands to say what it + does ("cancel transfer") while hovered, then shrinks back.""" + + def __init__(self, parent=None): + super().__init__(parent) + self.setFlat(True) + self.setFocusPolicy(Qt.FocusPolicy.NoFocus) + self.setCursor(Qt.CursorShape.PointingHandCursor) + self.setToolTip("Cancel the transfer") + self.setStyleSheet("QPushButton { border: none; padding: 0 4px; }") + self._apply("✕") + + def _apply(self, text: str): + self.setText(text) + self.setFixedWidth(self.fontMetrics().horizontalAdvance(text) + 14) + + def enterEvent(self, event): + self._apply("cancel transfer") + super().enterEvent(event) + + def leaveEvent(self, event): + self._apply("✕") + super().leaveEvent(event) + + class MainWindow(QMainWindow): def __init__(self, manager, prefs, lastfm=None, parent=None): super().__init__(parent) @@ -106,6 +132,10 @@ class MainWindow(QMainWindow): # status bar's right and survive showMessage. No stretch (see above) # and hidden while idle so it costs no space. Text lives in its own # label — QProgressBar's overlay can't fit a sentence at this width. + self._sync_cancel = _CancelSyncButton() + self._sync_cancel.hide() + self._sync_cancel.clicked.connect(self._confirm_cancel_sync) + self.statusBar().addPermanentWidget(self._sync_cancel) self._sync_label = QLabel() self._sync_label.hide() self.statusBar().addPermanentWidget(self._sync_label) @@ -325,16 +355,49 @@ class MainWindow(QMainWindow): self._sync_worker = device_sync.DeviceSyncWorker(plan, self) self._sync_worker.progress.connect(self._on_sync_progress) self._sync_worker.finished.connect(self._on_sync_finished) + self._sync_worker.cancelled.connect(self._on_sync_cancelled) self._sync_worker.failed.connect(self._on_sync_failed) self._sync_device_name = device.name self._sync_label.setText(f"Copying to {device.name}") self._sync_progress.setRange(0, max(plan.bytes_to_copy // 1024, 1)) self._sync_progress.setValue(0) + self._sync_cancel.show() self._sync_label.show() self._sync_progress.show() self._sync_inhibitor.inhibit() self._sync_worker.start() + def _hide_sync_widgets(self): + self._sync_cancel.hide() + self._sync_label.hide() + self._sync_progress.hide() + + def _confirm_cancel_sync(self): + if self._sync_worker is None or not self._sync_worker.busy(): + return + box = QMessageBox(self) + box.setWindowTitle("Cancel transfer") + box.setText(f"Cancel the transfer to the {self._sync_device_name}?\n" + "Tracks already copied stay on the device; syncing again " + "later picks up where this left off.") + cancel_button = box.addButton( + "Cancel Transfer", QMessageBox.ButtonRole.DestructiveRole) + keep = box.addButton("Keep Copying", QMessageBox.ButtonRole.RejectRole) + box.setDefaultButton(keep) + box.exec() + # The transfer keeps running while the dialog is up — it may well + # have finished by now, in which case there is nothing to cancel. + if (box.clickedButton() is cancel_button + and self._sync_worker is not None): + self._sync_worker.cancel() + + def _on_sync_cancelled(self, summary): + self._sync_inhibitor.release() + self._hide_sync_widgets() + self.statusBar().showMessage( + f"Sync cancelled — {summary['copied']} of {summary['total']} " + "tracks copied; sync again to finish.", 8000) + def _on_sync_progress(self, done_kib, total_kib, label): self._sync_progress.setRange(0, total_kib) self._sync_progress.setValue(done_kib) @@ -345,8 +408,7 @@ class MainWindow(QMainWindow): def _on_sync_finished(self, summary): self._sync_inhibitor.release() - self._sync_label.hide() - self._sync_progress.hide() + self._hide_sync_widgets() msg = (f"Synced “{summary['playlist']}” to the {summary['device']}: " f"{summary['copied']} copied, {summary['kept']} up to date, " f"{summary['removed']} removed") @@ -359,8 +421,7 @@ class MainWindow(QMainWindow): def _on_sync_failed(self, message): self._sync_inhibitor.release() - self._sync_label.hide() - self._sync_progress.hide() + self._hide_sync_widgets() QMessageBox.warning(self, "Sync failed", message) # ---- view switching ---- diff --git a/tests/test_round28.py b/tests/test_round28.py new file mode 100644 index 0000000..e5a6f26 --- /dev/null +++ b/tests/test_round28.py @@ -0,0 +1,153 @@ +"""Round 28: cancelable device sync. + +`DeviceSyncWorker.cancel()` stops the transfer at the next chunk boundary. +The contract under test: the in-flight partial file is removed, tracks that +fully landed stay, and the m3u is rewritten to list only what is really on +the device — so a cancelled sync is always coherent and a later re-sync +completes it to exactly the same state an uninterrupted sync would have +produced. (The hover-expanding ✕ button and confirm dialog are plain Qt in +MainWindow — no MainWindow tests exist, GUI verified by hand.) + +Tests run the worker synchronously (`_run()`), so a `progress` handler runs +inline and can call `cancel()` at a chosen emit — each 100-byte file emits +exactly once, making "cancel during file N" deterministic. +""" + +from lintunes.device_sync import CHUNK, DeviceSyncWorker, plan_sync +from lintunes.models import Track + + +def _track(tid, name, artist, path): + return Track(track_id=tid, name=name, artist=artist, + location=str(path), total_time=180_000) + + +def _audio(tmp_path, filename, data=b"x" * 100): + path = tmp_path / "local" / filename + path.parent.mkdir(parents=True, exist_ok=True) + path.write_bytes(data) + return path + + +def _run(plan, cancel_on_emit=None): + """Run a plan synchronously; optionally cancel() on the Nth progress + emit. Returns {"finished"|"cancelled"|"error": payload}.""" + worker = DeviceSyncWorker(plan) + results = {} + emits = [0] + + def on_progress(done, total, label): + emits[0] += 1 + if cancel_on_emit is not None and emits[0] == cancel_on_emit: + worker.cancel() + + worker.progress.connect(on_progress) + worker.finished.connect(lambda s: results.update(finished=s)) + worker.cancelled.connect(lambda s: results.update(cancelled=s)) + worker.failed.connect(lambda m: results.update(error=m)) + worker._run() + return worker, results + + +class TestCancel: + def test_cancel_before_run_copies_nothing(self, qapp, tmp_path): + a = _audio(tmp_path, "a.mp3") + plan = plan_sync("Mix", [_track(1, "A", "X", a)], tmp_path / "Music") + worker = DeviceSyncWorker(plan) + results = {} + worker.finished.connect(lambda s: results.update(finished=s)) + worker.cancelled.connect(lambda s: results.update(cancelled=s)) + worker.cancel() + worker._run() + assert "cancelled" in results and "finished" not in results + assert results["cancelled"]["copied"] == 0 + assert not (plan.dest_dir / "X - A.mp3").exists() + + def test_cancel_during_second_file_keeps_first(self, qapp, tmp_path): + a = _audio(tmp_path, "a.mp3", b"a" * 100) + b = _audio(tmp_path, "b.mp3", b"b" * 100) + c = _audio(tmp_path, "c.mp3", b"c" * 100) + tracks = [_track(1, "A", "X", a), _track(2, "B", "X", b), + _track(3, "C", "X", c)] + plan = plan_sync("Mix", tracks, tmp_path / "Music") + + worker, results = _run(plan, cancel_on_emit=2) + + summary = results["cancelled"] + assert summary["copied"] == 1 and summary["total"] == 3 + assert (plan.dest_dir / "X - A.mp3").read_bytes() == b"a" * 100 + assert not (plan.dest_dir / "X - B.mp3").exists() # partial removed + assert not (plan.dest_dir / "X - C.mp3").exists() # never started + m3u = (plan.dest_dir / "Mix.m3u").read_text(encoding="utf-8") + assert "X - A.mp3" in m3u + assert "X - B.mp3" not in m3u and "X - C.mp3" not in m3u + + def test_mid_file_cancel_removes_partial(self, qapp, tmp_path): + big = _audio(tmp_path, "big.mp3", b"z" * (3 * CHUNK)) + plan = plan_sync("Mix", [_track(1, "Big", "X", big)], + tmp_path / "Music") + worker, results = _run(plan, cancel_on_emit=1) # after first chunk + assert results["cancelled"]["copied"] == 0 + assert not (plan.dest_dir / "X - Big.mp3").exists() + + def test_cancel_signal_is_exclusive(self, qapp, tmp_path): + a = _audio(tmp_path, "a.mp3") + plan = plan_sync("Mix", [_track(1, "A", "X", a)], tmp_path / "Music") + worker, results = _run(plan, cancel_on_emit=1) + assert "cancelled" in results + assert "finished" not in results and "error" not in results + + def test_m3u_stays_coherent_after_stale_deletion(self, qapp, tmp_path): + # Old m3u references a stale file; cancel lands after stale deletion + # but before the new copy finishes — the rewritten m3u must reference + # neither the deleted stale file nor the never-copied track. + kept = _audio(tmp_path, "kept.mp3", b"k" * 40) + new = _audio(tmp_path, "new.mp3", b"n" * 60) + dest = tmp_path / "Music" / "Mix" + dest.mkdir(parents=True) + (dest / "X - Kept.mp3").write_bytes(b"k" * 40) + (dest / "Gone - Old.mp3").write_bytes(b"g" * 30) + (dest / "Mix.m3u").write_text( + "#EXTM3U\n#EXTINF:1,Gone - Old\nGone - Old.mp3\n" + "#EXTINF:1,X - Kept\nX - Kept.mp3\n") + tracks = [_track(1, "Kept", "X", kept), _track(2, "New", "X", new)] + plan = plan_sync("Mix", tracks, tmp_path / "Music") + + worker, results = _run(plan, cancel_on_emit=1) # during "New" copy + + assert not (dest / "Gone - Old.mp3").exists() + m3u = (dest / "Mix.m3u").read_text(encoding="utf-8") + assert "X - Kept.mp3" in m3u + assert "Gone - Old.mp3" not in m3u and "X - New.mp3" not in m3u + + def test_cancelled_sync_recovers_to_identical_state(self, qapp, tmp_path): + # trav's scenario: full sync, playlist changes, re-sync cancelled + # midway, then a third sync — must equal an uninterrupted sync. + t1 = _track(1, "One", "X", _audio(tmp_path, "1.mp3", b"1" * 80)) + t2 = _track(2, "Two", "X", _audio(tmp_path, "2.mp3", b"2" * 80)) + t3 = _track(3, "Three", "X", _audio(tmp_path, "3.mp3", b"3" * 80)) + t4 = _track(4, "Four", "X", _audio(tmp_path, "4.mp3", b"4" * 80)) + music = tmp_path / "Music" + + _run(plan_sync("Mix", [t1, t2, t3], music)) # initial sync + final = [t1, t3, t4] # -Two, +Four + _run(plan_sync("Mix", final, music), cancel_on_emit=1) # cancelled + worker, results = _run(plan_sync("Mix", final, music)) # recovery + assert "finished" in results + + reference = tmp_path / "Reference" + _run(plan_sync("Mix", final, reference)) # clean baseline + + got = {p.name: p.read_bytes() + for p in (music / "Mix").iterdir()} + want = {p.name: p.read_bytes() + for p in (reference / "Mix").iterdir()} + assert got == want + + def test_cancel_after_run_is_noop(self, qapp, tmp_path): + a = _audio(tmp_path, "a.mp3") + plan = plan_sync("Mix", [_track(1, "A", "X", a)], tmp_path / "Music") + worker, results = _run(plan) + assert "finished" in results + worker.cancel() # after the fact: no crash, no state change + assert (plan.dest_dir / "X - A.mp3").exists()