diff --git a/core/downloads/lifecycle.py b/core/downloads/lifecycle.py index a267f291..68054ccc 100644 --- a/core/downloads/lifecycle.py +++ b/core/downloads/lifecycle.py @@ -549,23 +549,22 @@ def on_download_completed(batch_id: str, task_id: str, success: bool, deps: Life except Exception as m3u_err: logger.error(f"[M3U] Error regenerating M3U on batch complete: {m3u_err}") - # PLAYLIST MATERIALIZE: if this was an organize-by-playlist batch, - # (re)build the playlist's folder of links/copies now that every - # track is imported. Built from the batch's own payload (owned + - # downloaded real paths) — non-fatal, derived view. (materialize.py) - if batch.get('playlist_folder_mode'): - try: - from core.playlists.materialize_service import materialize_playlist_from_batch - _mat = materialize_playlist_from_batch(batch, download_tasks, deps.config_manager) - if _mat: - logger.info( - f"[Playlist Folder] Rebuilt '{_mat.playlist_dir}': " - f"{_mat.linked} linked, {_mat.copied} copied, " - f"{_mat.unchanged} unchanged, {_mat.removed_stale} stale removed" - + (" (symlinks unsupported here → copied)" if _mat.fellback else "") - ) - except Exception as _mat_err: - logger.error(f"[Playlist Folder] Materialize failed (non-fatal): {_mat_err}") + # PLAYLIST MATERIALIZE: one path-independent reconcile — drop this + # batch's newly-resolved tracks into the right Playlists// + # folders. Covers an organize-by-playlist download AND a late + # wishlist arrival (via each track's playlist provenance). Built + # from the batch's own captured paths — non-fatal, derived view. + try: + from core.playlists.materialize_service import reconcile_batch_playlists + for _pl_name, _mat in reconcile_batch_playlists(batch, download_tasks, deps.config_manager): + logger.info( + f"[Playlist Folder] Rebuilt '{_mat.playlist_dir}': " + f"{_mat.linked} linked, {_mat.copied} copied, " + f"{_mat.unchanged} unchanged, {_mat.removed_stale} stale removed" + + (" (symlinks unsupported here → copied)" if _mat.fellback else "") + ) + except Exception as _mat_err: + logger.error(f"[Playlist Folder] Materialize failed (non-fatal): {_mat_err}") # REPAIR: Scan all album folders from this batch for track number issues if deps.repair_worker: diff --git a/core/downloads/master.py b/core/downloads/master.py index de136b85..b18b9b00 100644 --- a/core/downloads/master.py +++ b/core/downloads/master.py @@ -791,11 +791,10 @@ def run_full_missing_tracks_process(batch_id, playlist_id, tracks_json, deps: Ma # Gated + non-fatal; runs once after analysis, not in the per-track loop. if effective_playlist_folder_mode: try: - from core.playlists.materialize_service import materialize_playlist_from_batch + from core.playlists.materialize_service import reconcile_batch_playlists _batch = download_batches.get(batch_id) if _batch is not None: - _mat = materialize_playlist_from_batch(_batch, download_tasks, deps.config_manager) - if _mat: + for _pl_name, _mat in reconcile_batch_playlists(_batch, download_tasks, deps.config_manager): logger.info( f"[Playlist Folder] Rebuilt '{_mat.playlist_dir}' (all owned): " f"{_mat.linked} linked, {_mat.copied} copied, {_mat.removed_stale} stale removed" diff --git a/core/playlists/materialize_service.py b/core/playlists/materialize_service.py index 9be1e2b6..89759cbf 100644 --- a/core/playlists/materialize_service.py +++ b/core/playlists/materialize_service.py @@ -74,6 +74,58 @@ def materialize_playlist_from_batch(batch: dict, download_tasks: dict, config_ma return rebuild_playlist_folder(root, name, real_paths, mode) +def reconcile_batch_playlists(batch: dict, download_tasks: dict, config_manager): + """One post-batch step: drop the batch's newly-resolved tracks into the right + ``Playlists//`` folders. Path-independent — covers an organize-by-playlist + download AND a late wishlist arrival — using the per-track playlist provenance + each download already carries, plus the file paths the batch already captured + (so it needs no DB re-match and no waiting on the server scan/sync). + + - The batch's OWN organize playlist is fully rebuilt from the batch payload + (owned + downloaded), pruning tracks no longer present. + - A track that completed for a DIFFERENT playlist (e.g. a wishlist + fulfillment) is ADDED to that playlist's folder — no prune, since this + batch isn't that playlist's full membership (removals are handled by a + full rebuild on the next sync / the manual button). + + Returns a list of ``(playlist_name, RebuildSummary)``. Callers wrap non-fatally.""" + from core.library.path_resolver import resolve_library_file_path + + results = [] + root = docker_resolve_path(config_manager.get("playlists.materialize_path", "./Playlists")) + mode = normalize_mode(config_manager.get("playlists.materialize_mode", "symlink")) + batch_name = batch.get("playlist_name") if batch.get("playlist_folder_mode") else None + + # 1. The batch's own organize playlist → full materialize (prunes removed). + if batch_name: + summary = materialize_playlist_from_batch(batch, download_tasks, config_manager) + if summary: + results.append((batch_name, summary)) + + # 2. Tracks that completed for OTHER playlists (e.g. wishlist fulfilling a + # track that belongs to an organize playlist) → add-only into that folder. + extra: dict = {} + for tid in (batch.get("queue") or []): + task = (download_tasks or {}).get(tid) or {} + if task.get("status") != "completed" or not task.get("final_file_path"): + continue + pname = (task.get("track_info") or {}).get("_playlist_name") + if pname and pname != batch_name: + extra.setdefault(pname, []).append(task["final_file_path"]) + + for name, stored in extra.items(): + real, seen = [], set() + for p in stored: + r = resolve_library_file_path(str(p), config_manager=config_manager) + if r and r not in seen: + seen.add(r) + real.append(r) + if real: + results.append((name, rebuild_playlist_folder(root, name, real, mode, prune_stale=False))) + + return results + + def rebuild_organized_playlists_from_db(db, config_manager, *, profile_id: int = 1): """Rebuild every "organize by playlist" folder from CURRENT library ownership — for the manual "Rebuild" button. Unlike the post-download path (which uses the @@ -115,5 +167,6 @@ def rebuild_organized_playlists_from_db(db, config_manager, *, profile_id: int = __all__ = [ "collect_batch_real_paths", "materialize_playlist_from_batch", + "reconcile_batch_playlists", "rebuild_organized_playlists_from_db", ] diff --git a/tests/test_playlist_materialize_service.py b/tests/test_playlist_materialize_service.py index 36c8372d..4c810045 100644 --- a/tests/test_playlist_materialize_service.py +++ b/tests/test_playlist_materialize_service.py @@ -11,6 +11,7 @@ from core.playlists.materialize_service import ( collect_batch_real_paths, materialize_playlist_from_batch, rebuild_organized_playlists_from_db, + reconcile_batch_playlists, ) @@ -134,6 +135,62 @@ def test_materialize_skipped_when_not_organize(tmp_path: Path): assert not (tmp_path / "Playlists").exists() +def test_reconcile_organize_batch_full_rebuild(tmp_path: Path): + """An organize-by-playlist batch → its own folder is fully (re)built from the + batch payload.""" + owned, fresh = _mk(tmp_path, "Owned.mp3"), _mk(tmp_path, "Fresh.mp3") + batch = { + "playlist_folder_mode": True, + "playlist_name": "Mix", + "analysis_results": [{"found": True, "matched_file_path": owned[0]}], + "queue": ["t1"], + } + tasks = {"t1": {"status": "completed", "final_file_path": fresh[0], + "track_info": {"_playlist_name": "Mix"}}} # same playlist as batch + cfg = _Cfg(str(tmp_path / "Playlists")) + results = reconcile_batch_playlists(batch, tasks, cfg) + assert len(results) == 1 # only Mix (no double-add) + name, s = results[0] + assert name == "Mix" and s.linked == 2 + pdir = Path(s.playlist_dir) + assert (pdir / "Owned.mp3").exists() and (pdir / "Fresh.mp3").exists() + + +def test_reconcile_wishlist_adds_to_other_playlist_without_pruning(tmp_path: Path): + """The wishlist gap: a wishlist batch (not organize) downloads a track that + belongs to an organize playlist → it's ADDED to that playlist's folder, and + the folder's existing entries are NOT pruned.""" + # Pre-seed Smack That with an existing symlink (a track from a prior batch). + existing = _mk(tmp_path, "Existing.mp3")[0] + from core.playlists.materialize import rebuild_playlist_folder + rebuild_playlist_folder(str(tmp_path / "Playlists"), "Smack That", [existing], "symlink") + + # A WISHLIST batch (playlist_folder_mode False) completes a track tagged for + # the "Smack That" organize playlist. + late = _mk(tmp_path, "Late.mp3")[0] + batch = {"playlist_folder_mode": False, "playlist_name": "wishlist", + "analysis_results": [], "queue": ["w1"]} + tasks = {"w1": {"status": "completed", "final_file_path": late, + "track_info": {"_playlist_name": "Smack That"}}} + cfg = _Cfg(str(tmp_path / "Playlists")) + + results = reconcile_batch_playlists(batch, tasks, cfg) + assert len(results) == 1 and results[0][0] == "Smack That" + pdir = tmp_path / "Playlists" / "Smack That" + assert (pdir / "Late.mp3").exists() # the wishlist track was ADDED + assert (pdir / "Existing.mp3").exists() # and the prior entry was NOT pruned + + +def test_reconcile_noop_for_plain_batch(tmp_path: Path): + """A normal (non-organize, no provenance) batch → nothing happens.""" + f = _mk(tmp_path, "X.mp3")[0] + batch = {"playlist_folder_mode": False, "playlist_name": "album", + "analysis_results": [], "queue": ["a1"]} + tasks = {"a1": {"status": "completed", "final_file_path": f, "track_info": {}}} + assert reconcile_batch_playlists(batch, tasks, _Cfg(str(tmp_path / "Playlists"))) == [] + assert not (tmp_path / "Playlists").exists() + + def test_rebuild_from_db_only_organized_and_owned(tmp_path: Path): """The manual button: rebuild every organized playlist by re-matching with check_track_exists (name), linking only owned tracks."""