Playlists: one path-independent reconcile after every batch (closes wishlist gap)
Replaces the two organize-only triggers with a single reconcile_batch_playlists
called at both batch-completion points. It groups the batch's newly-resolved
tracks by their per-track playlist provenance:
- the batch's OWN organize playlist → full (re)build with prune, and
- a track that completed for a DIFFERENT playlist (e.g. a WISHLIST fulfilling a
track that belongs to an organize playlist) → ADDED to that folder, no prune.
So a late wishlist arrival now lands in its playlist folder immediately, instead
of only on the next sync/manual rebuild — the folder = the playlist's owned
members, kept true on every ownership change regardless of download path. Uses
the paths the batch already captured (no DB re-match, no waiting on the server
scan/sync). Non-fatal.
3 new reconcile tests (organize full-rebuild, wishlist add-without-prune, plain
batch no-op). 983 downloads/imports/playlist tests pass.
This commit is contained in:
parent
87621b7191
commit
997e76b6b4
4 changed files with 128 additions and 20 deletions
|
|
@ -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/<name>/
|
||||
# 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:
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
|
|
|
|||
|
|
@ -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/<name>/`` 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",
|
||||
]
|
||||
|
|
|
|||
|
|
@ -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."""
|
||||
|
|
|
|||
Loading…
Reference in a new issue