diff --git a/core/downloads/master.py b/core/downloads/master.py index e85d1c01..56c99d90 100644 --- a/core/downloads/master.py +++ b/core/downloads/master.py @@ -611,25 +611,30 @@ def run_full_missing_tracks_process(batch_id, playlist_id, tracks_json, deps: Ma # Blocklist (Phase 2a): drop banned artists/albums/tracks before queueing, # so a blocked item can't slip in via playlist sync / album download / # discography. Same ID-cascade brain as the wishlist guard (Phase 1) — - # the only other auto-acquisition path. (Manual single-track downloads - # are Phase 2b, intentionally not gated here.) - try: - _bl_before = len(missing_tracks) - _bl_kept = [] - for res in missing_tracks: - reason = db.blocklist_reason_for_track( - batch_profile_id, res.get('track', {}), source=batch_source) - if reason: - logger.info("[Blocklist] Skipping %s '%s' from download queue (%s blocked)", - reason[0], res.get('track', {}).get('name', '?'), reason[0]) - else: - _bl_kept.append(res) - if len(_bl_kept) != _bl_before: - logger.info("[Blocklist] Filtered out %d blocklisted track(s) from download queue", - _bl_before - len(_bl_kept)) - missing_tracks = _bl_kept - except Exception as _bl_err: - logger.debug("blocklist queue filter skipped: %s", _bl_err) + # the only other auto-acquisition path. Skipped when the user confirmed + # "download anyway" at the modal (Phase 2b override). + _ignore_blocklist = False + with tasks_lock: + if batch_id in download_batches: + _ignore_blocklist = download_batches[batch_id].get('ignore_blocklist', False) + if not _ignore_blocklist: + try: + _bl_before = len(missing_tracks) + _bl_kept = [] + for res in missing_tracks: + reason = db.blocklist_reason_for_track( + batch_profile_id, res.get('track', {}), source=batch_source) + if reason: + logger.info("[Blocklist] Skipping %s '%s' from download queue (%s blocked)", + reason[0], res.get('track', {}).get('name', '?'), reason[0]) + else: + _bl_kept.append(res) + if len(_bl_kept) != _bl_before: + logger.info("[Blocklist] Filtered out %d blocklisted track(s) from download queue", + _bl_before - len(_bl_kept)) + missing_tracks = _bl_kept + except Exception as _bl_err: + logger.debug("blocklist queue filter skipped: %s", _bl_err) with tasks_lock: if batch_id in download_batches: diff --git a/tests/blocklist/test_blocklist_api.py b/tests/blocklist/test_blocklist_api.py index b9ad12e9..75320210 100644 --- a/tests/blocklist/test_blocklist_api.py +++ b/tests/blocklist/test_blocklist_api.py @@ -59,3 +59,56 @@ def test_add_requires_type_and_name(client): def test_invalid_entity_type_rejected(client): assert client.get("/api/blocklist?entity_type=bogus").status_code == 400 assert client.get("/api/blocklist/search?type=bogus&q=x").status_code == 400 + + +# ── Phase 2b: download-time guards (manual + modal up-front) ───────────────── + +@pytest.fixture() +def banned_artist(client): + """Block 'Banned Guy' for the test profile; clean up after.""" + db = web_server.get_database() + eid = db.add_blocklist_entry(1, "artist", "Banned Guy", spotify_id="bg-sp") + yield + db.remove_blocklist_entry(1, eid) + + +def test_manual_download_blocked_by_artist_name(client, banned_artist): + r = client.post("/api/download", json={ + "result_type": "track", "username": "peer", "filename": "x.flac", + "artist": "Banned Guy", "title": "Song"}) + assert r.status_code == 409 + body = r.get_json() + assert body["blocked"] is True and body["blocked_entity_type"] == "artist" + + +def test_manual_download_unrelated_artist_not_blocked(client, banned_artist): + # Allowed artist → guard passes (the download itself may fail offline; we + # only assert it wasn't blocked by the blocklist). + r = client.post("/api/download", json={ + "result_type": "track", "username": "peer", "filename": "y.flac", + "artist": "Allowed Artist", "title": "Song"}) + assert not (r.get_json() or {}).get("blocked") + + +def test_manual_download_override_passes_guard(client, banned_artist): + r = client.post("/api/download", json={ + "result_type": "track", "username": "peer", "filename": "x.flac", + "artist": "Banned Guy", "title": "Song", "ignore_blocklist": True}) + assert not (r.get_json() or {}).get("blocked") # override skips the guard + + +def test_modal_blocked_album_returns_409_before_starting(client): + db = web_server.get_database() + eid = db.add_blocklist_entry(1, "album", "Banned Album", spotify_id="ba-sp") + try: + r = client.post("/api/playlists/artist_album_test/start-missing-process", json={ + "tracks": [{"id": "t1", "name": "Track"}], + "is_album_download": True, + "album_context": {"id": "ba-sp", "name": "Banned Album"}, + "artist_context": {"id": "ar1", "name": "Someone"}, + }) + assert r.status_code == 409 + body = r.get_json() + assert body["blocked"] is True and body["blocked_entity_type"] == "album" + finally: + db.remove_blocklist_entry(1, eid) diff --git a/web_server.py b/web_server.py index c609aeea..f54fb24d 100644 --- a/web_server.py +++ b/web_server.py @@ -6152,6 +6152,25 @@ def start_download(): if not username or not filename: return jsonify({"error": "Missing username or filename."}), 400 + # Blocklist guard (Phase 2b): a manual download is source-file-centric + # (no metadata IDs), so this matches the blocked ARTIST by name. The + # frontend shows "blocked — download anyway?" and re-POSTs with + # ignore_blocklist=true on confirm. + if not data.get('ignore_blocklist'): + try: + _dl_artist = data.get('artist') + if _dl_artist and _dl_artist != 'Unknown': + _reason = get_database().blocklist_reason_for_track( + get_current_profile_id(), + {'name': data.get('title'), 'artists': [{'name': _dl_artist}]}) + if _reason: + return jsonify({ + "success": False, "blocked": True, + "blocked_entity_type": _reason[0], "blocked_name": _reason[1], + }), 409 + except Exception as _bl_err: + logger.debug("manual download blocklist check skipped: %s", _bl_err) + download_id = run_async(download_orchestrator.download(username, filename, file_size)) logger.info(f"Download ID returned: {download_id}") @@ -19147,10 +19166,37 @@ def start_missing_tracks_process(playlist_id): # album-download modal. Stored on the batch and propagated per-track by # the master worker so AcoustID never quarantines this request's files. skip_acoustid = bool(data.get('skip_acoustid', False)) + # Blocklist override (Phase 2b): set by the modal's "download anyway" confirm. + ignore_blocklist = bool(data.get('ignore_blocklist', False)) if not tracks: return jsonify({"success": False, "error": "No tracks provided"}), 400 + # Blocklist up-front check (Phase 2b): if the WHOLE album or artist being + # downloaded is blocklisted, stop here with a clear, actionable response + # instead of silently dropping every track in the per-track filter (2a) and + # leaving an empty batch. Scattered single-track bans still fall through to + # the per-track filter. The modal re-POSTs with ignore_blocklist=true after + # the user confirms "download anyway". + if not ignore_blocklist and (album_context or artist_context): + try: + _bsrc = _downloads_history.detect_sync_source(playlist_id) + _synthetic = { + 'album': {'id': (album_context or {}).get('id'), + 'name': (album_context or {}).get('name')}, + 'artists': [{'id': (artist_context or {}).get('id'), + 'name': (artist_context or {}).get('name')}], + } + _reason = get_database().blocklist_reason_for_track( + get_current_profile_id(), _synthetic, source=_bsrc) + if _reason: + return jsonify({ + "success": False, "blocked": True, + "blocked_entity_type": _reason[0], "blocked_name": _reason[1], + }), 409 + except Exception as _bl_err: + logger.debug("blocklist up-front check skipped: %s", _bl_err) + # Log album context if provided if is_album_download and album_context and artist_context: logger.info(f"[Artist Album] Received album context: '{album_context.get('name')}' by '{artist_context.get('name')}' ({album_context.get('album_type', 'album')})") @@ -19208,6 +19254,9 @@ def start_missing_tracks_process(playlist_id): 'analysis_results': [], 'force_download_all': force_download_all, # Pass the force flag to the batch 'ignore_manual_matches': ignore_manual_matches, + # Blocklist override (Phase 2b) — the user confirmed "download anyway" + # at the modal, so the per-track filter (2a) skips this batch. + 'ignore_blocklist': ignore_blocklist, 'playlist_folder_mode': playlist_folder_mode, # Organize downloads by playlist folder # Album context for artist album downloads (explicit folder structure) 'is_album_download': is_album_download, diff --git a/webui/static/downloads.js b/webui/static/downloads.js index 8a44b782..5d5ad688 100644 --- a/webui/static/downloads.js +++ b/webui/static/downloads.js @@ -1,6 +1,15 @@ // WING IT — Download without metadata discovery // ================================================================================== +// Blocklist (Phase 2b): when a download is refused because the artist/album/track +// is on the blocklist, the backend returns {blocked:true,...}. Ask the user +// whether to override; callers re-POST with ignore_blocklist:true on confirm. +function confirmBlockedDownload(data) { + const what = data.blocked_entity_type || 'item'; + const name = data.blocked_name || 'this item'; + return confirm(`"${name}" is on your blocklist (${what} blocked).\n\nDownload anyway?`); +} + function _toggleWingItDropdown(btn, urlHash) { // Remove any existing dropdown const existing = document.querySelector('.wing-it-dropdown.visible'); @@ -2524,13 +2533,27 @@ async function startMissingTracksProcess(playlistId) { } } - const response = await fetch(`/api/playlists/${playlistId}/start-missing-process`, { + let response = await fetch(`/api/playlists/${playlistId}/start-missing-process`, { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify(requestBody) }); - const data = await response.json(); + let data = await response.json(); + // Blocklist (Phase 2b): whole album/artist is blocked → confirm override. + if (data.blocked) { + if (!confirmBlockedDownload(data)) { + showToast(`Skipped — ${data.blocked_name} is blocklisted`, 'info'); + return; + } + requestBody.ignore_blocklist = true; + response = await fetch(`/api/playlists/${playlistId}/start-missing-process`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify(requestBody) + }); + data = await response.json(); + } if (!data.success) { // Special handling for rate limit if (response.status === 429) { diff --git a/webui/static/search.js b/webui/static/search.js index 750bde56..54110af1 100644 --- a/webui/static/search.js +++ b/webui/static/search.js @@ -1059,15 +1059,35 @@ function initializeSearchModeToggle() { try { const downloadData = type === 'album' ? { result_type: 'album', tracks: result.tracks || [] } - : { result_type: 'track', username: result.username, filename: result.filename, size: result.size }; + : { result_type: 'track', username: result.username, filename: result.filename, + size: result.size, + // Carry artist/title so the blocklist guard can match (Phase 2b). + artist: result.artist || result.artist_name || '', + title: result.title || result.name || '' }; - const response = await fetch('/api/download', { + let response = await fetch('/api/download', { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify(downloadData) }); + let data = await response.json(); - const data = await response.json(); + // Blocklist (Phase 2b): blocked artist → confirm override + retry. + if (data.blocked) { + if (typeof confirmBlockedDownload === 'function' && confirmBlockedDownload(data)) { + downloadData.ignore_blocklist = true; + response = await fetch('/api/download', { + method: 'POST', headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify(downloadData) + }); + data = await response.json(); + } else { + showToast(`Skipped — ${data.blocked_name} is blocklisted`, 'info'); + this.disabled = false; + this.innerHTML = '💾 Download'; + return; + } + } if (data.error) { showToast(`Download error: ${data.error}`, 'error');