v0.3.0: cancelable device sync
A "✕" button sits left of the sync progress group and expands to "cancel transfer" on hover; clicking asks Cancel Transfer / Keep Copying (the copy keeps running under the dialog). Cancel stops the worker at the next chunk boundary, removes the in-flight partial file, and rewrites the m3u to list only tracks actually on the device — so a cancelled sync always leaves a coherent partial playlist. That last bit also closes a pre-existing gap: stale files are deleted before copying, so an interruption could previously leave the old m3u pointing at deleted files. Recoverability proven both ways: a test asserts cancel-then-resync ends byte-identical to an uninterrupted sync, and a live run against the real Rabbit (cancel mid-file-2 of 3) recovered with kept=1/copies=2. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -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,
|
||||
|
||||
@@ -1,3 +1,3 @@
|
||||
"""LinTunes — iTunes-style music library manager and player for Linux."""
|
||||
|
||||
__version__ = "0.2.2"
|
||||
__version__ = "0.3.0"
|
||||
|
||||
+51
-4
@@ -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?")
|
||||
|
||||
@@ -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 ----
|
||||
|
||||
@@ -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()
|
||||
Reference in New Issue
Block a user