diff --git a/core/imports/quarantine.py b/core/imports/quarantine.py index 43ed5191..3df79f85 100644 --- a/core/imports/quarantine.py +++ b/core/imports/quarantine.py @@ -14,6 +14,7 @@ from __future__ import annotations import json import os import shutil +import time from pathlib import Path from typing import Any, Dict, List, Optional, Tuple @@ -289,6 +290,56 @@ def _restore_filename(quarantined_filename: str, sidecar_original: Optional[str] return base +def get_quarantine_entry_stream_info( + quarantine_dir: str, entry_id: str +) -> Optional[Tuple[str, str]]: + """Resolve a quarantined entry to ``(file_path, original_extension)`` for + in-app playback. + + The on-disk file carries a ``.quarantined`` suffix, so its own extension is + useless for picking an audio MIME type. Recover the real extension from the + sidecar's ``original_filename`` when present, else from the quarantine + filename convention. Returns None when the entry's file can't be found. + """ + file_path, sidecar_path = _resolve_entry_paths(quarantine_dir, entry_id) + if not file_path or not os.path.isfile(file_path): + return None + + sidecar_original: Optional[str] = None + if sidecar_path: + try: + with open(sidecar_path, encoding="utf-8") as f: + sidecar_original = json.load(f).get("original_filename") + except Exception as exc: + logger.debug("stream-info: sidecar read failed for %s: %s", entry_id, exc) + + original_name = _restore_filename(os.path.basename(file_path), sidecar_original) + extension = os.path.splitext(original_name)[1].lower() + return file_path, extension + + +def _move_with_retry(src: str, dst: str, attempts: int = 4, delay: float = 0.4) -> bool: + """Move a file, retrying briefly on transient OS locks. + + On Windows a still-open read handle (e.g. the in-app quarantine preview + player that just streamed the file) makes shutil.move raise + PermissionError / WinError 32 "file in use". The handle is released a beat + after playback stops, so a few short retries clear the common case without + failing the whole Approve/Recover. Returns True on success. + """ + last_exc: Optional[BaseException] = None + for i in range(attempts): + try: + shutil.move(src, dst) + return True + except OSError as exc: + last_exc = exc + if i < attempts - 1: + time.sleep(delay) + logger.error("move failed after %d attempts: %s -> %s: %s", attempts, src, dst, last_exc) + return False + + def approve_quarantine_entry( quarantine_dir: str, entry_id: str, @@ -334,10 +385,9 @@ def approve_quarantine_entry( restored_path = os.path.join(restore_dir, original_name) restored_path = _ensure_unique_path(restored_path) - try: - shutil.move(file_path, restored_path) - except OSError as exc: - logger.error("approve: failed to restore %s -> %s: %s", file_path, restored_path, exc) + if not _move_with_retry(file_path, restored_path): + logger.error("approve: failed to restore %s -> %s (file may still be in use)", + file_path, restored_path) return None try: @@ -377,10 +427,8 @@ def recover_to_staging( os.makedirs(staging_dir, exist_ok=True) target = _ensure_unique_path(os.path.join(staging_dir, restored_name)) - try: - shutil.move(file_path, target) - except OSError as exc: - logger.error("recover: failed to move %s -> %s: %s", file_path, target, exc) + if not _move_with_retry(file_path, target): + logger.error("recover: failed to move %s -> %s (file may still be in use)", file_path, target) return None if sidecar_path and os.path.isfile(sidecar_path): diff --git a/tests/imports/test_quarantine_management.py b/tests/imports/test_quarantine_management.py index d2c527c0..3615ca6a 100644 --- a/tests/imports/test_quarantine_management.py +++ b/tests/imports/test_quarantine_management.py @@ -5,6 +5,7 @@ from core.imports.quarantine import ( approve_quarantine_entry, delete_quarantine_entry, entry_id_from_quarantined_filename, + get_quarantine_entry_stream_info, get_quarantined_source_keys, list_quarantine_entries, recover_to_staging, @@ -125,6 +126,41 @@ def test_list_handles_orphan_quarantined_file_without_sidecar(tmp_path): assert entries[0]["has_full_context"] is False +# ────────────────────────────────────────────────────────────────────── +# get_quarantine_entry_stream_info — in-app "Listen" support +# ────────────────────────────────────────────────────────────────────── + +def test_stream_info_resolves_path_and_extension_from_sidecar(tmp_path): + qfile, _ = _write_entry(tmp_path, "20260514_120000", "song.flac", with_context=True) + entry_id = entry_id_from_quarantined_filename(qfile.name) + + info = get_quarantine_entry_stream_info(str(tmp_path), entry_id) + + assert info is not None + file_path, ext = info + assert file_path == str(qfile) + assert ext == ".flac" # real audio ext, NOT ".quarantined" + + +def test_stream_info_recovers_extension_without_sidecar(tmp_path): + # Orphan .quarantined with no sidecar — extension comes from the filename + # convention so playback still gets a correct Content-Type. + qfile = tmp_path / "20260514_120000_orphan.mp3.quarantined" + qfile.write_bytes(b"X" * 100) + entry_id = entry_id_from_quarantined_filename(qfile.name) + + info = get_quarantine_entry_stream_info(str(tmp_path), entry_id) + + assert info is not None + file_path, ext = info + assert file_path == str(qfile) + assert ext == ".mp3" + + +def test_stream_info_returns_none_for_missing_entry(tmp_path): + assert get_quarantine_entry_stream_info(str(tmp_path), "does_not_exist") is None + + def test_list_skips_orphan_sidecars_without_file(tmp_path): sidecar = tmp_path / "20260514_120000_only.json" sidecar.write_text(json.dumps({"original_filename": "only.flac", "quarantine_reason": "x"})) @@ -398,3 +434,23 @@ def test_source_keys_dedup_repeated_sources(tmp_path): keys = get_quarantined_source_keys(str(tmp_path)) assert keys == {("peer", "dupe.flac")} + + +# ────────────────────────────────────────────────────────────────────── +# _move_with_retry — resilient move (Windows file-lock case) +# ────────────────────────────────────────────────────────────────────── + +def test_move_with_retry_succeeds(tmp_path): + from core.imports.quarantine import _move_with_retry + src = tmp_path / "a.flac"; src.write_bytes(b"x" * 10) + dst = tmp_path / "out" / "a.flac" + (tmp_path / "out").mkdir() + assert _move_with_retry(str(src), str(dst)) is True + assert dst.exists() and not src.exists() + + +def test_move_with_retry_returns_false_on_missing_source(tmp_path): + from core.imports.quarantine import _move_with_retry + # attempts=1 keeps the test fast (no retry sleeps) + assert _move_with_retry(str(tmp_path / "nope.flac"), str(tmp_path / "dst.flac"), + attempts=1, delay=0) is False diff --git a/web_server.py b/web_server.py index 51724911..2025220d 100644 --- a/web_server.py +++ b/web_server.py @@ -7131,6 +7131,25 @@ def approve_quarantine_item(entry_id): return jsonify({"success": False, "error": str(e)}), 500 +@app.route('/api/quarantine//stream', methods=['GET']) +def stream_quarantine_item(entry_id): + """Stream a quarantined audio file in-app (range-supported) so the user can + listen before deciding to approve, search again, or delete it. The file + lives in the quarantine dir with a `.quarantined` suffix, so the real audio + extension (and thus Content-Type) is recovered from the sidecar.""" + try: + from core.imports.quarantine import get_quarantine_entry_stream_info + info = get_quarantine_entry_stream_info(_get_quarantine_dir(), entry_id) + if info is None: + return jsonify({"error": "Quarantined file not found"}), 404 + file_path, extension = info + mimetype = _AUDIO_MIME_TYPES.get(extension, 'audio/mpeg') + return _serve_audio_file_with_range(file_path, mimetype_override=mimetype) + except Exception as e: + logger.error(f"[Quarantine] Error streaming {entry_id}: {e}") + return jsonify({"error": str(e)}), 500 + + @app.route('/api/quarantine//recover', methods=['POST']) def recover_quarantine_item(entry_id): """Fallback for legacy thin sidecars: move file into Staging so the user @@ -11877,17 +11896,22 @@ _AUDIO_MIME_TYPES = { } -def _serve_audio_file_with_range(file_path): +def _serve_audio_file_with_range(file_path, mimetype_override=None): """Serve an on-disk audio file with HTTP range support (HTML5 seeking). Shared by /stream/audio (current track) and /stream/library-audio (the crossfade pre-loader, which plays the NEXT track on a second