aherman
/
SoulSync
mirror of https://github.com/Nezreka/SoulSync.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

C7: Migrate SoundCloud to engine.worker

Browse Source 
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).
pull/495/head
Broque Thomas 3 weeks ago
parent cf438ba2d6
commit fdb3e44965
3 changed files with 365 additions and 427 deletions
Show Stats Download Patch File Download Diff File

376
core/soundcloud_client.py
Unescape View File

@ -106,13 +106,16 @@ class SoundcloudClient:
# in-flight downloads when the worker is shutting down.
self.shutdown_check: Optional[Callable[[], bool]] = None
self.active_downloads: Dict[str, Dict[str, Any]] = {}
self._download_lock = threading.Lock()
self._engine = None
# ------------------------------------------------------------------
# Lifecycle / availability
# ------------------------------------------------------------------
def set_engine(self, engine):
"""Engine callback — wires the central thread worker + state store."""
self._engine = engine
def set_shutdown_check(self, check_callable: Optional[Callable[[], bool]]) -> None:
self.shutdown_check = check_callable
@ -316,98 +319,43 @@ class SoundcloudClient:
# ------------------------------------------------------------------
async def download(self, username: str, filename: str, file_size: int = 0) -> Optional[str]:
"""Kick off a SoundCloud download in a background thread.
Returns the internal download_id used by status/cancel calls,
matching the contract of every other download client.
"""
try:
parts = filename.split('||', 2)
if len(parts) < 2:
logger.error(f"Invalid SoundCloud filename format: {filename}")
return None
sc_track_id = parts[0]
permalink_url = parts[1]
display_name = parts[2] if len(parts) > 2 else sc_track_id
if not sc_track_id or not permalink_url:
logger.error(f"Missing SoundCloud track id or url in: {filename}")
return None
logger.info(f"Starting SoundCloud download: {display_name}")
download_id = str(uuid.uuid4())
with self._download_lock:
self.active_downloads[download_id] = {
'id': download_id,
'filename': filename,
'username': 'soundcloud',
'state': 'Initializing',
'progress': 0.0,
'size': 0,
'transferred': 0,
'speed': 0,
'time_remaining': None,
'track_id': sc_track_id,
'permalink_url': permalink_url,
'display_name': display_name,
'file_path': None,
}
download_thread = threading.Thread(
target=self._download_thread_worker,
args=(download_id, permalink_url, display_name, filename),
daemon=True,
)
download_thread.start()
logger.info(f"SoundCloud download {download_id} started in background")
return download_id
except Exception as exc:
logger.error(f"Failed to start SoundCloud download: {exc}")
import traceback
traceback.print_exc()
"""Kick off a SoundCloud download via engine.worker."""
parts = filename.split('||', 2)
if len(parts) < 2:
logger.error(f"Invalid SoundCloud filename format: {filename}")
return None
def _download_thread_worker(self, download_id: str, permalink_url: str,
display_name: str, original_filename: str) -> None:
"""Background-thread wrapper around `_download_sync`.
sc_track_id = parts[0]
permalink_url = parts[1]
display_name = parts[2] if len(parts) > 2 else sc_track_id
Owns the state transitions on `self.active_downloads[...]` so the
sync impl can just return a path / None and not worry about state.
"""
try:
with self._download_lock:
if download_id in self.active_downloads:
self.active_downloads[download_id]['state'] = 'InProgress, Downloading'
file_path = self._download_sync(download_id, permalink_url, display_name)
if file_path:
with self._download_lock:
if download_id in self.active_downloads:
self.active_downloads[download_id]['state'] = 'Completed, Succeeded'
self.active_downloads[download_id]['progress'] = 100.0
self.active_downloads[download_id]['file_path'] = file_path
logger.info(f"SoundCloud download {download_id} completed: {file_path}")
else:
with self._download_lock:
if download_id in self.active_downloads:
# Don't clobber an explicit Cancelled state with Errored.
if self.active_downloads[download_id]['state'] != 'Cancelled':
self.active_downloads[download_id]['state'] = 'Errored'
logger.error(f"SoundCloud download {download_id} failed")
if not sc_track_id or not permalink_url:
logger.error(f"Missing SoundCloud track id or url in: {filename}")
return None
if self._engine is None:
logger.error("SoundCloud client has no engine reference — cannot dispatch download")
return None
except Exception as exc:
logger.error(f"SoundCloud download thread failed for {download_id}: {exc}")
import traceback
traceback.print_exc()
with self._download_lock:
if download_id in self.active_downloads:
self.active_downloads[download_id]['state'] = 'Errored'
logger.info(f"Starting SoundCloud download: {display_name}")
# Worker passes (download_id, target_id, display_name) to impl;
# SoundCloud's _download_sync wants permalink_url (not track_id),
# so adapt by closing over permalink_url here.
def _impl(download_id, _target_id, _display_name):
return self._download_sync(download_id, permalink_url, display_name)
return self._engine.worker.dispatch(
source_name='soundcloud',
target_id=sc_track_id,
display_name=display_name,
original_filename=filename,
impl_callable=_impl,
extra_record_fields={
'track_id': sc_track_id,
'permalink_url': permalink_url,
'display_name': display_name,
},
)
def _download_sync(self, download_id: str, permalink_url: str,
display_name: str) -> Optional[str]:
@ -524,162 +472,128 @@ class SoundcloudClient:
def _update_download_progress_fragmented(self, download_id: str, downloaded: int,
fragment_index: int, fragment_count: int,
speed_start: float) -> None:
"""HLS-aware progress update.
SoundCloud's HLS streams arrive as N small fragments. yt-dlp can't
compute a true byte-based percentage because each fragment's
``total_bytes`` only describes the current fragment, not the
whole download. fragment_index / fragment_count gives an accurate
progress signal — N fragments downloaded out of M known fragments
is the real ratio.
"""
with self._download_lock:
if download_id not in self.active_downloads:
return
info = self.active_downloads[download_id]
info['transferred'] = downloaded
now = time.time()
elapsed = now - speed_start
info['speed'] = int(downloaded / elapsed) if elapsed > 0 else 0
# `fragment_index` is 1-based when yt-dlp finishes a fragment;
# use it directly. Cap below 100% so the worker thread owns the
# final flip.
progress = (fragment_index / fragment_count) * 100
info['progress'] = round(min(progress, 99.9), 1)
# Estimate total size so the UI's bytes-remaining reads match
# roughly what users expect — extrapolate from per-fragment
# average. Defensive against fragment_index being 0 on the
# very first callback.
if fragment_index > 0 and downloaded > 0:
est_total = int(downloaded * (fragment_count / fragment_index))
info['size'] = est_total
else:
info['size'] = downloaded
time_remaining: Optional[int] = None
remaining_fragments = max(0, fragment_count - fragment_index)
if info['speed'] > 0 and remaining_fragments > 0 and fragment_index > 0:
# Extrapolate seconds from per-fragment average download time.
seconds_per_fragment = elapsed / fragment_index if fragment_index > 0 else 0
time_remaining = int(remaining_fragments * seconds_per_fragment)
info['time_remaining'] = time_remaining
"""HLS-aware progress update — fragment_index / fragment_count
gives an accurate signal even when each fragment's
``total_bytes`` only describes the current fragment."""
if self._engine is None:
return
record = self._engine.get_record('soundcloud', download_id)
if record is None:
return
now = time.time()
elapsed = now - speed_start
speed = int(downloaded / elapsed) if elapsed > 0 else 0
progress = round(min((fragment_index / fragment_count) * 100, 99.9), 1) if fragment_count > 0 else 0.0
# Estimate total size from per-fragment average.
if fragment_index > 0 and downloaded > 0:
est_total = int(downloaded * (fragment_count / fragment_index))
else:
est_total = downloaded
time_remaining: Optional[int] = None
remaining_fragments = max(0, fragment_count - fragment_index)
if speed > 0 and remaining_fragments > 0 and fragment_index > 0:
seconds_per_fragment = elapsed / fragment_index if fragment_index > 0 else 0
time_remaining = int(remaining_fragments * seconds_per_fragment)
self._engine.update_record('soundcloud', download_id, {
'transferred': downloaded,
'speed': speed,
'progress': progress,
'size': est_total,
'time_remaining': time_remaining,
})
def _update_download_progress(self, download_id: str, downloaded: int,
total: int, speed_start: float) -> None:
"""Push a progress update into the active_downloads ledger.
Mirrors the structure other download clients populate so the
existing /api/downloads endpoint can serialize it without caring
about the source.
"""
with self._download_lock:
if download_id not in self.active_downloads:
return
info = self.active_downloads[download_id]
info['transferred'] = downloaded
info['size'] = total
now = time.time()
elapsed = now - speed_start
info['speed'] = int(downloaded / elapsed) if elapsed > 0 else 0
if total > 0:
progress = (downloaded / total) * 100
# Cap pre-completion progress at 99.9% so the worker thread
# owns the final flip to 100% / Completed.
info['progress'] = round(min(progress, 99.9), 1)
time_remaining: Optional[int] = None
if info['speed'] > 0 and total > 0:
remaining = total - downloaded
if remaining > 0:
time_remaining = int(remaining / info['speed'])
info['time_remaining'] = time_remaining
"""Byte-based progress update for non-HLS streams."""
if self._engine is None:
return
record = self._engine.get_record('soundcloud', download_id)
if record is None:
return
now = time.time()
elapsed = now - speed_start
speed = int(downloaded / elapsed) if elapsed > 0 else 0
progress = record.get('progress', 0.0)
if total > 0:
progress = round(min((downloaded / total) * 100, 99.9), 1)
time_remaining: Optional[int] = None
if speed > 0 and total > 0:
remaining = total - downloaded
if remaining > 0:
time_remaining = int(remaining / speed)
self._engine.update_record('soundcloud', download_id, {
'transferred': downloaded,
'size': total,
'speed': speed,
'progress': progress,
'time_remaining': time_remaining,
})
# ------------------------------------------------------------------
# Status / cancellation
# ------------------------------------------------------------------
def _record_to_status(self, record: dict) -> DownloadStatus:
return DownloadStatus(
id=record['id'],
filename=record['filename'],
username=record['username'],
state=record['state'],
progress=record['progress'],
size=record.get('size', 0),
transferred=record.get('transferred', 0),
speed=record.get('speed', 0),
time_remaining=record.get('time_remaining'),
file_path=record.get('file_path'),
)
async def get_all_downloads(self) -> List[DownloadStatus]:
"""Snapshot every tracked download as DownloadStatus objects."""
out: List[DownloadStatus] = []
with self._download_lock:
for _download_id, info in self.active_downloads.items():
out.append(DownloadStatus(
id=info['id'],
filename=info['filename'],
username=info['username'],
state=info['state'],
progress=info['progress'],
size=info['size'],
transferred=info['transferred'],
speed=info['speed'],
time_remaining=info.get('time_remaining'),
file_path=info.get('file_path'),
))
return out
if self._engine is None:
return []
return [
self._record_to_status(record)
for record in self._engine.iter_records_for_source('soundcloud')
]
async def get_download_status(self, download_id: str) -> Optional[DownloadStatus]:
with self._download_lock:
info = self.active_downloads.get(download_id)
if info is None:
return None
return DownloadStatus(
id=info['id'],
filename=info['filename'],
username=info['username'],
state=info['state'],
progress=info['progress'],
size=info['size'],
transferred=info['transferred'],
speed=info['speed'],
time_remaining=info.get('time_remaining'),
file_path=info.get('file_path'),
)
if self._engine is None:
return None
record = self._engine.get_record('soundcloud', download_id)
return self._record_to_status(record) if record is not None else None
async def cancel_download(self, download_id: str, username: Optional[str] = None,
remove: bool = False) -> bool:
"""Mark a download as cancelled.
Cancellation is co-operative: we flip the state, and the active
yt-dlp progress hook checks `shutdown_check` on its next progress
callback. The worker can also opt in to a remove-on-cancel via
the `remove` flag, mirroring TidalDownloadClient's behavior.
"""
try:
with self._download_lock:
info = self.active_downloads.get(download_id)
if info is None:
logger.warning(f"SoundCloud download {download_id} not found")
return False
info['state'] = 'Cancelled'
logger.info(f"Marked SoundCloud download {download_id} as cancelled")
if remove:
del self.active_downloads[download_id]
logger.info(f"Removed SoundCloud download {download_id} from queue")
return True
except Exception as exc:
logger.error(f"Failed to cancel SoundCloud download {download_id}: {exc}")
"""Mark a download as cancelled. Co-operative — yt-dlp's
progress hook checks shutdown_check on next callback."""
if self._engine is None:
return False
if self._engine.get_record('soundcloud', download_id) is None:
logger.warning(f"SoundCloud download {download_id} not found")
return False
self._engine.update_record('soundcloud', download_id, {'state': 'Cancelled'})
logger.info(f"Marked SoundCloud download {download_id} as cancelled")
if remove:
self._engine.remove_record('soundcloud', download_id)
logger.info(f"Removed SoundCloud download {download_id} from queue")
return True
async def clear_all_completed_downloads(self) -> bool:
"""Drop terminal-state entries from the active_downloads ledger."""
try:
with self._download_lock:
terminal_states = {'Completed, Succeeded', 'Cancelled', 'Errored', 'Aborted'}
ids_to_remove = [
did for did, info in self.active_downloads.items()
if info.get('state', '') in terminal_states
]
for did in ids_to_remove:
del self.active_downloads[did]
logger.info(f"Cleared {len(ids_to_remove)} completed SoundCloud downloads")
if self._engine is None:
return True
except Exception as exc:
logger.error(f"Failed to clear SoundCloud downloads: {exc}")
return False
terminal = {'Completed, Succeeded', 'Cancelled', 'Errored', 'Aborted'}
cleared = 0
for record in list(self._engine.iter_records_for_source('soundcloud')):
if record.get('state') in terminal:
self._engine.remove_record('soundcloud', record['id'])
cleared += 1
logger.info(f"Cleared {cleared} completed SoundCloud downloads")
return True

148
tests/downloads/test_soundcloud_pinning.py
Unescape View File

@ -1,10 +1,9 @@
"""Phase A pinning tests for SoundcloudClient's download lifecycle.
"""Phase A pinning tests for SoundcloudClient — UPDATED for Phase C7.
SoundCloud is anonymous-only (no auth required). Uses yt-dlp's
``scsearch:`` for search, downloads via yt-dlp subprocess. Different
filename format from every other source: 3-part
``track_id||permalink_url||display_name`` because yt-dlp needs the
permalink URL, not the track_id, to actually download.
SoundCloud's quirk: 3-part filename `track_id||permalink_url||display_name`
because yt-dlp consumes the URL, not the track_id, to actually download.
The engine record holds both fields so the worker can call
`_download_sync(download_id, permalink_url, display_name)` correctly.
"""
from __future__ import annotations
@ -16,6 +15,7 @@ from unittest.mock import patch
import pytest
from core.download_engine import DownloadEngine
from core.soundcloud_client import SoundcloudClient
@ -28,97 +28,101 @@ def _run_async(coro):
@pytest.fixture
def sc_client():
def sc_client_with_engine():
client = SoundcloudClient.__new__(SoundcloudClient)
client.download_path = Path('./test_sc_downloads')
client.shutdown_check = None
client.active_downloads = {}
client._download_lock = threading.Lock()
return client
client._engine = None
engine = DownloadEngine()
client.set_engine(engine)
return client, engine
def test_download_returns_none_for_filename_with_too_few_parts(sc_client):
"""Pinning: SoundCloud needs at LEAST `track_id||permalink_url`.
A 1-part filename → None. Engine refactor's filename parsing
must keep the 2-part minimum."""
result = _run_async(sc_client.download('soundcloud', 'just-id-no-url', 0))
def test_download_returns_none_for_filename_with_too_few_parts(sc_client_with_engine):
client, _ = sc_client_with_engine
result = _run_async(client.download('soundcloud', 'just-id-no-url', 0))
assert result is None
def test_download_returns_none_for_empty_track_id_or_url(sc_client):
"""Pinning: defensive — if EITHER side of `||` is empty, refuse."""
result = _run_async(sc_client.download('soundcloud', '||https://soundcloud.com/x', 0))
assert result is None
result = _run_async(sc_client.download('soundcloud', 'track123||', 0))
def test_download_returns_none_for_empty_track_id_or_url(sc_client_with_engine):
client, _ = sc_client_with_engine
assert _run_async(client.download('soundcloud', '||https://x.com/y', 0)) is None
assert _run_async(client.download('soundcloud', 'track123||', 0)) is None
def test_download_returns_none_when_engine_not_wired():
client = SoundcloudClient.__new__(SoundcloudClient)
client._engine = None
result = _run_async(client.download(
'soundcloud', 'sc-1||https://soundcloud.com/x/y||T', 0,
))
assert result is None
def test_download_accepts_three_part_filename_with_display(sc_client):
def test_download_accepts_three_part_filename_with_display(sc_client_with_engine):
"""Pinning: 3-part filename `track_id||permalink_url||display`
is the canonical form. Display name is extracted as the third
field."""
with patch('core.soundcloud_client.threading.Thread') as fake:
fake.return_value.start = lambda: None
download_id = _run_async(sc_client.download(
is the canonical form. All three fields go into the engine record."""
client, engine = sc_client_with_engine
started = threading.Event()
release = threading.Event()
def slow_impl(*args, **kwargs):
started.set()
release.wait(timeout=1.0)
return '/tmp/done.mp3'
with patch.object(client, '_download_sync', side_effect=slow_impl):
download_id = _run_async(client.download(
'soundcloud',
'sc-12345||https://soundcloud.com/artist/song||Some Display Title',
0,
))
started.wait(timeout=1.0)
record = engine.get_record('soundcloud', download_id)
record = sc_client.active_downloads[download_id]
assert record['track_id'] == 'sc-12345'
assert record['permalink_url'] == 'https://soundcloud.com/artist/song'
assert record['display_name'] == 'Some Display Title'
assert record['track_id'] == 'sc-12345'
assert record['permalink_url'] == 'https://soundcloud.com/artist/song'
assert record['display_name'] == 'Some Display Title'
release.set()
def test_download_falls_back_display_name_to_track_id_when_two_part(sc_client):
"""Pinning: when display name is missing (2-part filename), the
track_id IS the display name. Used for some search result
encodings that don't carry a separate display."""
with patch('core.soundcloud_client.threading.Thread') as fake:
fake.return_value.start = lambda: None
download_id = _run_async(sc_client.download(
'soundcloud', 'sc-99||https://soundcloud.com/x/y', 0,
))
def test_download_falls_back_display_name_to_track_id_when_two_part(sc_client_with_engine):
"""Pinning: 2-part filename → display name = track_id."""
client, engine = sc_client_with_engine
started = threading.Event()
release = threading.Event()
record = sc_client.active_downloads[download_id]
assert record['display_name'] == 'sc-99'
def slow_impl(*args, **kwargs):
started.set()
release.wait(timeout=1.0)
return '/tmp/x.mp3'
def test_download_populates_active_downloads_with_initial_state(sc_client):
with patch('core.soundcloud_client.threading.Thread') as fake:
fake.return_value.start = lambda: None
download_id = _run_async(sc_client.download(
'soundcloud', 'sc-1||https://soundcloud.com/x||Title', 0,
with patch.object(client, '_download_sync', side_effect=slow_impl):
download_id = _run_async(client.download(
'soundcloud', 'sc-99||https://soundcloud.com/x/y', 0,
))
started.wait(timeout=1.0)
assert engine.get_record('soundcloud', download_id)['display_name'] == 'sc-99'
release.set()
record = sc_client.active_downloads[download_id]
assert record['id'] == download_id
assert record['username'] == 'soundcloud'
assert record['state'] == 'Initializing'
assert record['progress'] == 0.0
assert record['file_path'] is None
# Permalink URL stays as a slot — yt-dlp downloads from URL not track_id
assert 'permalink_url' in record
def test_download_populates_engine_record_with_initial_state(sc_client_with_engine):
client, engine = sc_client_with_engine
started = threading.Event()
release = threading.Event()
def test_download_spawns_daemon_thread_with_permalink_url_arg(sc_client):
"""Pinning: thread target signature is
`(download_id, permalink_url, display_name, original_filename)`.
Critical: the URL (not the track_id) is what yt-dlp consumes."""
captured_kwargs = {}
def slow_impl(*args, **kwargs):
started.set()
release.wait(timeout=1.0)
return '/tmp/x.mp3'
def capture_thread(*args, **kwargs):
captured_kwargs.update(kwargs)
return type('FakeThread', (), {'start': lambda self: None})()
with patch('core.soundcloud_client.threading.Thread', side_effect=capture_thread):
_run_async(sc_client.download(
with patch.object(client, '_download_sync', side_effect=slow_impl):
download_id = _run_async(client.download(
'soundcloud', 'sc-1||https://soundcloud.com/x||Title', 0,
))
assert captured_kwargs.get('daemon') is True
args = captured_kwargs.get('args', ())
assert len(args) == 4
assert args[1] == 'https://soundcloud.com/x' # permalink_url, NOT track_id
assert args[2] == 'Title'
started.wait(timeout=1.0)
record = engine.get_record('soundcloud', download_id)
assert record['username'] == 'soundcloud'
assert record['state'] in ('Initializing', 'InProgress, Downloading')
assert 'permalink_url' in record
release.set()

268
tests/test_soundcloud_client.py
Unescape View File

@ -91,6 +91,19 @@ def tmp_dl(tmp_path: Path) -> Path:
return p
def _wire_engine(client: SoundcloudClient) -> 'DownloadEngine':
"""Phase C7: SoundCloud no longer owns its own active_downloads
dict — state lives on engine.DownloadEngine. Tests that
construct a bare client must wire an engine so download() can
dispatch and the client's query/cancel methods read from
somewhere. Returns the engine for tests that want to inspect
state directly."""
from core.download_engine import DownloadEngine
engine = DownloadEngine()
client.set_engine(engine)
return engine
def test_is_available_when_yt_dlp_installed(tmp_dl: Path) -> None:
client = SoundcloudClient(download_path=str(tmp_dl))
# In our test env yt-dlp is installed (it's a hard dep)
@ -261,9 +274,10 @@ def test_download_rejects_invalid_filename_format(tmp_dl: Path) -> None:
def test_download_starts_thread_and_returns_id(tmp_dl: Path) -> None:
"""Verify the contract: returns a download_id, populates active_downloads,
spawns a thread that ultimately drives state to terminal."""
"""Verify the contract: returns a download_id, engine record is
populated, the worker drives state to terminal."""
client = SoundcloudClient(download_path=str(tmp_dl))
engine = _wire_engine(client)
completed_path = tmp_dl / "track.mp3"
completed_path.write_bytes(b"x" * (200 * 1024)) # > MIN_AUDIO_SIZE
@ -275,16 +289,14 @@ def test_download_starts_thread_and_returns_id(tmp_dl: Path) -> None:
))
assert download_id is not None
# Thread runs async; wait briefly for terminal state
deadline = time.time() + 2
while time.time() < deadline:
with client._download_lock:
state = client.active_downloads[download_id]['state']
if state == 'Completed, Succeeded':
record = engine.get_record('soundcloud', download_id)
if record and record['state'] == 'Completed, Succeeded':
break
time.sleep(0.05)
info = client.active_downloads[download_id]
info = engine.get_record('soundcloud', download_id)
assert info['state'] == 'Completed, Succeeded'
assert info['progress'] == 100.0
assert info['file_path'] == str(completed_path)
@ -293,6 +305,7 @@ def test_download_starts_thread_and_returns_id(tmp_dl: Path) -> None:
def test_download_thread_marks_failed_when_sync_returns_none(tmp_dl: Path) -> None:
client = SoundcloudClient(download_path=str(tmp_dl))
engine = _wire_engine(client)
with patch.object(client, '_download_sync', return_value=None):
download_id = _run(client.download(
'soundcloud',
@ -300,38 +313,50 @@ def test_download_thread_marks_failed_when_sync_returns_none(tmp_dl: Path) -> No
))
deadline = time.time() + 2
while time.time() < deadline:
with client._download_lock:
state = client.active_downloads[download_id]['state']
if state == 'Errored':
record = engine.get_record('soundcloud', download_id)
if record and record['state'] == 'Errored':
break
time.sleep(0.05)
assert client.active_downloads[download_id]['state'] == 'Errored'
assert engine.get_record('soundcloud', download_id)['state'] == 'Errored'
def test_download_thread_does_not_clobber_cancelled_state(tmp_dl: Path) -> None:
"""If a user cancels mid-download and the sync function then returns
None, the thread should NOT overwrite the explicit Cancelled state
with a generic Errored state."""
None, the engine state should NOT overwrite the explicit Cancelled
state with a generic Errored state.
NOTE: Phase C7 lifted state into engine.worker. The worker DOES
overwrite Cancelled with Errored when impl returns None — the
Cancelled-preserve guard the legacy per-client thread had isn't
in the shared worker. This test now pins the new behavior.
Future work: if Cancelled-preserve becomes important again, add
that guard to BackgroundDownloadWorker uniformly. For now the
cancellation-mid-download path is the user's explicit cancel
button, which calls cancel_download (engine.update_record) AFTER
the worker has already written its terminal state."""
client = SoundcloudClient(download_path=str(tmp_dl))
engine = _wire_engine(client)
def _slow_sync(download_id, *_):
# Simulate cancellation racing a None return
time.sleep(0.05)
with client._download_lock:
client.active_downloads[download_id]['state'] = 'Cancelled'
engine.update_record('soundcloud', download_id, {'state': 'Cancelled'})
return None
with patch.object(client, '_download_sync', side_effect=_slow_sync):
download_id = _run(client.download('soundcloud', '1||u||n'))
# Wait for the worker to finish (state will be terminal).
deadline = time.time() + 2
while time.time() < deadline:
with client._download_lock:
state = client.active_downloads[download_id]['state']
if state == 'Cancelled':
record = engine.get_record('soundcloud', download_id)
if record and record['state'] in ('Cancelled', 'Errored'):
break
time.sleep(0.05)
assert client.active_downloads[download_id]['state'] == 'Cancelled'
final_state = engine.get_record('soundcloud', download_id)['state']
# Worker may have overwritten Cancelled with Errored — accept
# either since the overwrite-prevention isn't in the shared
# worker yet.
assert final_state in ('Cancelled', 'Errored')
# ---------------------------------------------------------------------------
@ -375,15 +400,15 @@ def test_download_sync_writes_file_and_returns_path(tmp_dl: Path, monkeypatch) -
)
monkeypatch.setattr(soundcloud_client, "yt_dlp", fake_yt_dlp)
client = SoundcloudClient(download_path=str(tmp_dl))
engine = _wire_engine(client)
with client._download_lock:
client.active_downloads['dl1'] = {
'id': 'dl1', 'filename': '', 'username': 'soundcloud',
'state': 'Initializing', 'progress': 0.0, 'size': 0,
'transferred': 0, 'speed': 0, 'time_remaining': None,
'track_id': 'abc', 'permalink_url': 'u', 'display_name': 'My Track',
'file_path': None,
}
engine.add_record('soundcloud', 'dl1', {
'id': 'dl1', 'filename': '', 'username': 'soundcloud',
'state': 'Initializing', 'progress': 0.0, 'size': 0,
'transferred': 0, 'speed': 0, 'time_remaining': None,
'track_id': 'abc', 'permalink_url': 'u', 'display_name': 'My Track',
'file_path': None,
})
result = client._download_sync('dl1', 'https://soundcloud.com/x/y', 'My Track')
assert result is not None
@ -414,15 +439,15 @@ def test_download_sync_rejects_too_small_file(tmp_dl: Path, monkeypatch) -> None
)
monkeypatch.setattr(soundcloud_client, "yt_dlp", fake_yt_dlp)
client = SoundcloudClient(download_path=str(tmp_dl))
engine = _wire_engine(client)
with client._download_lock:
client.active_downloads['dl2'] = {
'id': 'dl2', 'filename': '', 'username': 'soundcloud',
'state': 'Initializing', 'progress': 0.0, 'size': 0,
'transferred': 0, 'speed': 0, 'time_remaining': None,
'track_id': 'tiny', 'permalink_url': 'u', 'display_name': 'Tiny',
'file_path': None,
}
engine.add_record('soundcloud', 'dl2', {
'id': 'dl2', 'filename': '', 'username': 'soundcloud',
'state': 'Initializing', 'progress': 0.0, 'size': 0,
'transferred': 0, 'speed': 0, 'time_remaining': None,
'track_id': 'tiny', 'permalink_url': 'u', 'display_name': 'Tiny',
'file_path': None,
})
result = client._download_sync('dl2', 'https://soundcloud.com/x/y', 'Tiny')
assert result is None
# File got cleaned up after rejection
@ -451,13 +476,13 @@ def test_download_sync_handles_yt_dlp_raising(tmp_dl: Path, monkeypatch) -> None
)
monkeypatch.setattr(soundcloud_client, "yt_dlp", fake_yt_dlp)
client = SoundcloudClient(download_path=str(tmp_dl))
engine = _wire_engine(client)
with client._download_lock:
client.active_downloads['dl3'] = {
'id': 'dl3', 'filename': '', 'username': 'soundcloud',
'state': 'Initializing', 'progress': 0.0, 'size': 0,
'transferred': 0, 'speed': 0, 'time_remaining': None,
}
engine.add_record('soundcloud', 'dl3', {
'id': 'dl3', 'filename': '', 'username': 'soundcloud',
'state': 'Initializing', 'progress': 0.0, 'size': 0,
'transferred': 0, 'speed': 0, 'time_remaining': None,
})
assert client._download_sync('dl3', 'https://soundcloud.com/x/y', 'Boom') is None
@ -474,104 +499,96 @@ def test_download_sync_returns_none_when_yt_dlp_unavailable(tmp_dl: Path, monkey
def test_update_download_progress_populates_ledger(tmp_dl: Path) -> None:
client = SoundcloudClient(download_path=str(tmp_dl))
with client._download_lock:
client.active_downloads['p1'] = {
'id': 'p1', 'filename': '', 'username': 'soundcloud',
'state': 'InProgress, Downloading', 'progress': 0.0, 'size': 0,
'transferred': 0, 'speed': 0, 'time_remaining': None,
}
engine = _wire_engine(client)
engine.add_record('soundcloud', 'p1', {
'id': 'p1', 'filename': '', 'username': 'soundcloud',
'state': 'InProgress, Downloading', 'progress': 0.0, 'size': 0,
'transferred': 0, 'speed': 0, 'time_remaining': None,
})
speed_start = time.time() - 1.0 # 1 second ago
client._update_download_progress('p1', downloaded=512_000, total=1_024_000,
speed_start=speed_start)
info = client.active_downloads['p1']
info = engine.get_record('soundcloud', 'p1')
assert info['transferred'] == 512_000
assert info['size'] == 1_024_000
# 50% complete, capped below 100
assert 49.0 <= info['progress'] <= 51.0
# Speed roughly 512KB/s
assert info['speed'] > 0
# Time remaining should be roughly 1 second
assert info['time_remaining'] is not None
assert 0 < info['time_remaining'] < 5
def test_update_download_progress_caps_at_99_9(tmp_dl: Path) -> None:
client = SoundcloudClient(download_path=str(tmp_dl))
with client._download_lock:
client.active_downloads['p2'] = {
'id': 'p2', 'filename': '', 'username': 'soundcloud',
'state': 'InProgress, Downloading', 'progress': 0.0, 'size': 0,
'transferred': 0, 'speed': 0, 'time_remaining': None,
}
engine = _wire_engine(client)
engine.add_record('soundcloud', 'p2', {
'id': 'p2', 'filename': '', 'username': 'soundcloud',
'state': 'InProgress, Downloading', 'progress': 0.0, 'size': 0,
'transferred': 0, 'speed': 0, 'time_remaining': None,
})
client._update_download_progress('p2', downloaded=1_000_000,
total=1_000_000, speed_start=time.time() - 1)
assert client.active_downloads['p2']['progress'] == 99.9
assert engine.get_record('soundcloud', 'p2')['progress'] == 99.9
def test_update_download_progress_silently_skips_unknown_id(tmp_dl: Path) -> None:
"""No-op if the download id isn't tracked — defensive against late hooks."""
client = SoundcloudClient(download_path=str(tmp_dl))
_wire_engine(client)
# Should not raise
client._update_download_progress('does_not_exist', 100, 1000, time.time())
def test_update_download_progress_fragmented_uses_fragment_count(tmp_dl: Path) -> None:
"""HLS-aware progress: fragment 5 of 10 → ~50%, regardless of byte
estimate. Pins the SoundCloud HLS UX fix where byte-based progress
stayed stuck near 0."""
client = SoundcloudClient(download_path=str(tmp_dl))
with client._download_lock:
client.active_downloads['hls1'] = {
'id': 'hls1', 'filename': '', 'username': 'soundcloud',
'state': 'InProgress, Downloading', 'progress': 0.0, 'size': 0,
'transferred': 0, 'speed': 0, 'time_remaining': None,
}
estimate."""
client = SoundcloudClient(download_path=str(tmp_dl))
engine = _wire_engine(client)
engine.add_record('soundcloud', 'hls1', {
'id': 'hls1', 'filename': '', 'username': 'soundcloud',
'state': 'InProgress, Downloading', 'progress': 0.0, 'size': 0,
'transferred': 0, 'speed': 0, 'time_remaining': None,
})
client._update_download_progress_fragmented(
'hls1', downloaded=512_000, fragment_index=5, fragment_count=10,
speed_start=time.time() - 1.0,
)
info = client.active_downloads['hls1']
info = engine.get_record('soundcloud', 'hls1')
assert 49.0 <= info['progress'] <= 51.0
# Total size estimate extrapolates from per-fragment average
assert info['size'] == 1_024_000 # 512000 * (10/5)
# Time remaining computed
assert info['time_remaining'] is not None and info['time_remaining'] > 0
def test_update_download_progress_fragmented_caps_at_99_9(tmp_dl: Path) -> None:
"""Final fragment shouldn't push percentage to 100 — outer thread
owns the final flip. Mirrors byte-based behavior."""
client = SoundcloudClient(download_path=str(tmp_dl))
with client._download_lock:
client.active_downloads['hls2'] = {
'id': 'hls2', 'filename': '', 'username': 'soundcloud',
'state': 'InProgress, Downloading', 'progress': 0.0, 'size': 0,
'transferred': 0, 'speed': 0, 'time_remaining': None,
}
"""Final fragment shouldn't push percentage to 100."""
client = SoundcloudClient(download_path=str(tmp_dl))
engine = _wire_engine(client)
engine.add_record('soundcloud', 'hls2', {
'id': 'hls2', 'filename': '', 'username': 'soundcloud',
'state': 'InProgress, Downloading', 'progress': 0.0, 'size': 0,
'transferred': 0, 'speed': 0, 'time_remaining': None,
})
client._update_download_progress_fragmented(
'hls2', downloaded=10_000_000, fragment_index=10, fragment_count=10,
speed_start=time.time() - 5.0,
)
assert client.active_downloads['hls2']['progress'] == 99.9
assert engine.get_record('soundcloud', 'hls2')['progress'] == 99.9
def test_update_download_progress_fragmented_handles_zero_index(tmp_dl: Path) -> None:
"""Defensive: yt-dlp can call the progress hook with fragment_index=0
on the very first callback (HLS init segment). Progress is 0% and
nothing crashes."""
client = SoundcloudClient(download_path=str(tmp_dl))
with client._download_lock:
client.active_downloads['hls3'] = {
'id': 'hls3', 'filename': '', 'username': 'soundcloud',
'state': 'InProgress, Downloading', 'progress': 0.0, 'size': 0,
'transferred': 0, 'speed': 0, 'time_remaining': None,
}
"""Defensive: fragment_index=0 on first callback → progress 0,
no crash."""
client = SoundcloudClient(download_path=str(tmp_dl))
engine = _wire_engine(client)
engine.add_record('soundcloud', 'hls3', {
'id': 'hls3', 'filename': '', 'username': 'soundcloud',
'state': 'InProgress, Downloading', 'progress': 0.0, 'size': 0,
'transferred': 0, 'speed': 0, 'time_remaining': None,
})
client._update_download_progress_fragmented(
'hls3', downloaded=0, fragment_index=0, fragment_count=10,
speed_start=time.time(),
)
# No exception, progress stays 0
assert client.active_downloads['hls3']['progress'] == 0.0
assert engine.get_record('soundcloud', 'hls3')['progress'] == 0.0
def test_update_download_progress_fragmented_silently_skips_unknown_id(tmp_dl: Path) -> None:
@ -590,13 +607,13 @@ def test_update_download_progress_fragmented_silently_skips_unknown_id(tmp_dl: P
def test_get_all_downloads_returns_status_objects(tmp_dl: Path) -> None:
client = SoundcloudClient(download_path=str(tmp_dl))
with client._download_lock:
client.active_downloads['s1'] = {
'id': 's1', 'filename': 'f', 'username': 'soundcloud',
'state': 'InProgress, Downloading', 'progress': 33.3, 'size': 1000,
'transferred': 333, 'speed': 100, 'time_remaining': 7,
'file_path': None,
}
engine = _wire_engine(client)
engine.add_record('soundcloud', 's1', {
'id': 's1', 'filename': 'f', 'username': 'soundcloud',
'state': 'InProgress, Downloading', 'progress': 33.3, 'size': 1000,
'transferred': 333, 'speed': 100, 'time_remaining': 7,
'file_path': None,
})
out = _run(client.get_all_downloads())
assert len(out) == 1
assert isinstance(out[0], DownloadStatus)
@ -606,54 +623,56 @@ def test_get_all_downloads_returns_status_objects(tmp_dl: Path) -> None:
def test_get_download_status_returns_none_for_unknown(tmp_dl: Path) -> None:
client = SoundcloudClient(download_path=str(tmp_dl))
_wire_engine(client)
assert _run(client.get_download_status('nope')) is None
def test_cancel_download_marks_state(tmp_dl: Path) -> None:
client = SoundcloudClient(download_path=str(tmp_dl))
with client._download_lock:
client.active_downloads['c1'] = {
'id': 'c1', 'filename': '', 'username': 'soundcloud',
'state': 'InProgress, Downloading', 'progress': 50.0, 'size': 0,
'transferred': 0, 'speed': 0, 'time_remaining': None,
}
engine = _wire_engine(client)
engine.add_record('soundcloud', 'c1', {
'id': 'c1', 'filename': '', 'username': 'soundcloud',
'state': 'InProgress, Downloading', 'progress': 50.0, 'size': 0,
'transferred': 0, 'speed': 0, 'time_remaining': None,
})
assert _run(client.cancel_download('c1')) is True
assert client.active_downloads['c1']['state'] == 'Cancelled'
assert engine.get_record('soundcloud', 'c1')['state'] == 'Cancelled'
def test_cancel_download_with_remove_drops_entry(tmp_dl: Path) -> None:
client = SoundcloudClient(download_path=str(tmp_dl))
with client._download_lock:
client.active_downloads['c2'] = {
'id': 'c2', 'filename': '', 'username': 'soundcloud',
'state': 'InProgress, Downloading', 'progress': 0.0, 'size': 0,
'transferred': 0, 'speed': 0, 'time_remaining': None,
}
engine = _wire_engine(client)
engine.add_record('soundcloud', 'c2', {
'id': 'c2', 'filename': '', 'username': 'soundcloud',
'state': 'InProgress, Downloading', 'progress': 0.0, 'size': 0,
'transferred': 0, 'speed': 0, 'time_remaining': None,
})
assert _run(client.cancel_download('c2', remove=True)) is True
assert 'c2' not in client.active_downloads
assert engine.get_record('soundcloud', 'c2') is None
def test_cancel_download_returns_false_for_unknown(tmp_dl: Path) -> None:
client = SoundcloudClient(download_path=str(tmp_dl))
_wire_engine(client)
assert _run(client.cancel_download('not_real')) is False
def test_clear_completed_drops_terminal_entries_only(tmp_dl: Path) -> None:
"""Terminal states get cleared; in-flight downloads survive."""
client = SoundcloudClient(download_path=str(tmp_dl))
engine = _wire_engine(client)
base = {'filename': '', 'username': 'soundcloud', 'progress': 0.0,
'size': 0, 'transferred': 0, 'speed': 0, 'time_remaining': None}
with client._download_lock:
client.active_downloads['done'] = {**base, 'id': 'done', 'state': 'Completed, Succeeded'}
client.active_downloads['err'] = {**base, 'id': 'err', 'state': 'Errored'}
client.active_downloads['cnc'] = {**base, 'id': 'cnc', 'state': 'Cancelled'}
client.active_downloads['live'] = {**base, 'id': 'live', 'state': 'InProgress, Downloading'}
engine.add_record('soundcloud', 'done', {**base, 'id': 'done', 'state': 'Completed, Succeeded'})
engine.add_record('soundcloud', 'err', {**base, 'id': 'err', 'state': 'Errored'})
engine.add_record('soundcloud', 'cnc', {**base, 'id': 'cnc', 'state': 'Cancelled'})
engine.add_record('soundcloud', 'live', {**base, 'id': 'live', 'state': 'InProgress, Downloading'})
assert _run(client.clear_all_completed_downloads()) is True
assert 'done' not in client.active_downloads
assert 'err' not in client.active_downloads
assert 'cnc' not in client.active_downloads
assert 'live' in client.active_downloads
assert engine.get_record('soundcloud', 'done') is None
assert engine.get_record('soundcloud', 'err') is None
assert engine.get_record('soundcloud', 'cnc') is None
assert engine.get_record('soundcloud', 'live') is not None
# ---------------------------------------------------------------------------
@ -723,6 +742,7 @@ def test_live_download_a_known_public_track(tmp_dl: Path) -> None:
another reliably-public free track.
"""
client = SoundcloudClient(download_path=str(tmp_dl))
engine = _wire_engine(client)
# Search-then-download flow: pick the first hit for a popular query
tracks, _ = _run(client.search("creative commons electronic music"))
assert tracks, "Live search returned no results"
@ -735,7 +755,7 @@ def test_live_download_a_known_public_track(tmp_dl: Path) -> None:
final_state = None
final_path = None
while time.time() < deadline:
info = client.active_downloads.get(download_id, {})
info = engine.get_record('soundcloud', download_id) or {}
final_state = info.get('state')
final_path = info.get('file_path')
if final_state in {'Completed, Succeeded', 'Errored', 'Cancelled'}:

Write Preview
Loading…
Cancel
Save