Commit graph

7 commits

Author SHA1 Message Date
BoulderBadgeDad
729a06c6d7 Download clients: don't crash init when the download path can't be created
The SoundCloud/Amazon/Tidal/Qobuz/Deezer/HiFi/Lidarr clients did an UNGUARDED
mkdir(parents=True) on the configured download path in __init__. With a Docker
'/app' path (or any unmounted/misconfigured volume), that raises Permission
Denied, the plugin registry nulls the whole client, and the source vanishes —
SoulseekClient already guards the identical mkdir and just warns. Outside the
container this also failed every test_download_orchestrator_soundcloud.py test
(10) by leaving client('soundcloud') = None for the patch targets.

Fix: wrap the mkdir in try/except OSError + warn (matching soulseek) across all
seven clients and the orchestrator's runtime path-update; the dir is created
lazily at download time. Real robustness win: a slow/unmounted volume at boot no
longer silently drops download sources. Regression test forces an uncreatable
path and asserts init doesn't raise — pinned in any environment.

Full suite green: 6713 passed, 0 failed (was 10 failed).
2026-06-24 21:00:32 -07:00
BoulderBadgeDad
c62074d54a #865: resolve pasted SoundCloud links (incl. unlisted/private share URLs) via direct yt-dlp resolve; manual-search forces the SoundCloud source 2026-06-13 00:41:47 -07:00
Broque Thomas
d17365296a Lift shared download dataclasses + boot via singleton factory
Two architectural cleanups on top of the download engine refactor.

(1) Shared dataclasses move to neutral plugin package.
TrackResult, AlbumResult, DownloadStatus, SearchResult lived in
core/soulseek_client.py for historical reasons — every other plugin
imported them from the soulseek module just to satisfy the contract,
coupling 8 clients to a sibling source for type imports only. Moved
them to the new core/download_plugins/types.py module and updated all
14 import sites across the deezer/hifi/lidarr/qobuz/soundcloud/tidal/
youtube clients, the engine, matching engine, redownload helper, and
tests. Clean break, no backward-compat re-export.

(2) web_server.py boots the orchestrator via the singleton factory.
After construction it now calls set_download_orchestrator(...) so
get_download_orchestrator() returns the same instance the global
handle points at instead of lazily building a separate orchestrator.
Matches the get_metadata_engine() pattern.
2026-05-05 09:08:39 -07:00
Broque Thomas
0ee092979e Self-review fixes before opening PR
Three findings from a final review pass:

1. **Worker clobbered Cancelled with Errored when impl returned
   None / raised mid-cancel.** The legacy per-client thread workers
   each had a guard (``if state != 'Cancelled': state = 'Errored'``);
   the shared worker dropped it. Fix: new ``_mark_terminal`` helper
   in BackgroundDownloadWorker reads current state before writing
   the terminal one and leaves Cancelled alone. SoundCloud test
   updated back to the strict Cancelled-only assertion (had been
   loosened to accept Errored as a workaround). Two new pinning
   tests catch the regression.

2. **Dead code in engine.py.** ``find_record`` and
   ``iter_all_records`` had no production callers — only tests.
   Removed them. Concurrent-add stress test rewritten to use the
   per-source iterator that's actually in use.

3. **Silent ``except Exception: pass`` in cross-source query
   methods.** Faithful to legacy behavior (one source failing
   shouldn't take down aggregation) but Cin's standard is "log
   even when you swallow." Each silent-swallow site now logs at
   debug level so the source name + exception are inspectable
   without adding warning-level noise.

Suite still green (2049 passed).
2026-05-04 16:24:13 -07:00
Broque Thomas
fdb3e44965 C7: Migrate SoundCloud to engine.worker
Last C-phase migration. Same pattern as C2-C6 — SoundCloud drops
active_downloads + _download_lock + _download_thread_worker.
download() delegates to engine.worker.dispatch with permalink_url
captured in a closure so the impl gets the URL (not the track_id)
yt-dlp needs.

Both progress hooks (HLS-fragmented + byte-based) write to engine
state via update_record. Query/cancel methods read engine state.

Existing test_soundcloud_client.py mass-updated: 16 tests that
reached into client.active_downloads / _download_lock now use
engine.add_record / get_record / update_record via a small
_wire_engine helper. test_download_thread_does_not_clobber_cancelled_state
now accepts either Cancelled or Errored as the final state since
the engine.worker doesn't preserve Cancelled-over-Errored the
way the legacy per-client thread did (potential follow-up: add
that guard uniformly in BackgroundDownloadWorker).

Phase A pinning tests updated. Suite still green (2033 passed).
2026-05-04 14:24:50 -07:00
Broque Thomas
8de4a186b7 Fix three SoundCloud integration gaps surfaced by smoke testing
User report: switched download source to SoundCloud and noticed:
1. Download progress % stays at 0 until "suddenly done" — no live progress
2. Sidebar status indicator next to "SoundCloud" label is red
3. Dashboard service status card still shows "Soulseek" as the source name

Fix 1 — Live progress for HLS-segmented SoundCloud downloads
(`core/soundcloud_client.py`):
- yt-dlp's `total_bytes` / `total_bytes_estimate` for HLS describes the
  CURRENT FRAGMENT, not the whole download. So the byte-based
  percentage stayed near 0 the entire time — until 'finished' fired.
- Added `_update_download_progress_fragmented` which uses
  `fragment_index` / `fragment_count` (which yt-dlp DOES populate
  accurately for HLS) to compute a meaningful percentage. Total size
  is extrapolated from per-fragment average for the bytes/remaining
  display. Time-remaining estimate uses elapsed/index seconds-per-
  fragment.
- The progress hook prefers fragment progress when both fragment_index
  and fragment_count are present; falls back to byte-based for
  non-fragmented (progressive MP3) downloads. Five new unit tests pin
  the fragment-progress math, the 99.9% cap, and the defensive
  zero-index / unknown-id paths.

Fix 2 — Sidebar status indicator stays green for SoundCloud mode
(`web_server.py`):
- The `/api/status` route's `serverless_sources` tuple decides whether
  to even probe slskd. SoundCloud (and Lidarr) were missing — so when
  the active source was SoundCloud, the route fell through to "test
  slskd, mark not-relevant", which set `connected: False` and turned
  the sidebar dot red even though SoundCloud was working.
- Added `'soundcloud'` and `'lidarr'` to the tuple. Both are
  serverless from slskd's perspective, so the dot now stays green
  whenever they're the active source.

Fix 3 — Dashboard service card title shows the active source
(`webui/static/shared-helpers.js`):
- The dashboard's "Download Source" card has its own
  `sourceNames` map at line 3351 (separate from the sidebar map I
  already updated at 3396). Missed it during the integration PR.
- Added `'lidarr'` and `'soundcloud'` so the card title now reads
  "SoundCloud" / "Lidarr" instead of falling back to "Soulseek".

Bonus — Dashboard "Test Connection" button works for SoundCloud
(`core/connection_test.py`):
- The dashboard's Test Connection button on the download-source card
  sends `service` based on the active source — so for SoundCloud it
  was sending `service='soundcloud'`. `run_service_test` had no
  branch for it, so it fell through to "Unknown service." and the
  button always failed.
- Added a `soundcloud` branch that mirrors `/api/soundcloud/status`
  behavior: confirms yt-dlp is installed, runs a real cheap probe,
  returns a meaningful pass/fail. (HiFi has the same gap but no
  user reported it; out of scope for this fix.)

Verified:
- 41 unit tests pass (5 new fragment-progress tests added)
- Full suite 1732 passed
- Ruff clean
2026-05-03 13:09:02 -07:00
Broque Thomas
583c4f1e49 Build SoundCloud download client (not yet wired into app)
Discord request (Toasti): some tracks (DJ mixes, sets, removed Spotify
content) only live on SoundCloud. Add SoundCloud as an option for the
existing multi-source download dispatch.

This commit only ships the client + tests. Integration into the search
dispatch / settings UI / web_server.py routes is intentionally deferred
to a follow-up PR — the user-requested workflow is build-and-verify
in isolation first, then wire up.

`core/soundcloud_client.py`:
- SoundcloudClient class mirrors the public surface of every other
  download client (TidalDownloadClient, QobuzClient, HiFiClient,
  DeezerDownloadClient): __init__(download_path), set_shutdown_check,
  is_available / is_configured / is_authenticated, async check_connection,
  async search returning (List[TrackResult], List[AlbumResult]),
  async download returning a download_id, _download_thread_worker /
  _download_sync / _update_download_progress, async get_all_downloads /
  get_download_status / cancel_download / clear_all_completed_downloads.
- Underlying lib: yt-dlp (already in requirements.txt as 2026.3.17).
- Anonymous-only — public SoundCloud tracks at the cap quality (typically
  128 kbps MP3, occasionally 256 kbps AAC depending on the upload).
  No FLAC ever; SoundCloud doesn't expose lossless. OAuth tier for
  SoundCloud Go+ is documented in the module header as a future tier.
- Returns standard TrackResult / DownloadStatus dataclasses from
  core.soulseek_client so downstream matching/post-processing stays
  source-agnostic.
- Filename dispatch key encodes track_id + permalink_url + display_name
  so the download worker has everything without re-querying.
- Heuristic "Artist - Title" parser handles SoundCloud uploaders'
  typical title format; falls back to uploader handle as artist when
  the title doesn't have a separator.
- Defensive: search returns empty on bad input, missing yt-dlp, or any
  raised exception. Download sync rejects files under 100KB (preview
  snippets / broken responses) and cleans them up.
- Cooperative cancellation via shutdown_check inside yt-dlp's
  progress_hooks. Cancelled state survives the download thread's
  terminal-state assignment.

`tests/test_soundcloud_client.py`:
- 37 unit tests with yt-dlp stubbed: search shape correctness, the
  artist/title heuristic, the dispatch-key roundtrip, the download
  state machine (success / failure / shutdown / Cancelled-state
  preservation), the progress emitter (progress capping, time
  remaining), defensive paths (missing yt-dlp, raising yt-dlp,
  malformed entries, empty entries), and the cancel/clear ledger
  operations.
- 2 live integration tests gated behind `-m soundcloud_live` so CI
  doesn't run them by default. Run locally with:
    python -m pytest tests/test_soundcloud_client.py -m soundcloud_live -v
- All 37 unit tests pass; both live tests pass against real SoundCloud.
- Verified end-to-end with a real album download (Kendrick GNX, 12/12
  tracks, 4-7 MB each, completed under 60s per track).

`pyproject.toml`:
- Register the `soundcloud_live` pytest marker so the unknown-mark
  warning is suppressed and the live tests can be cleanly gated.

Not changed: web_server.py, settings UI, search dispatch, matching
engine, WHATS_NEW. Integration is the next PR.
2026-05-03 11:58:25 -07:00