A1: Pin SoulseekClient download lifecycle behavior
13 tests pin slskd HTTP API contract: endpoint format (`transfers/downloads/<username>` POST), payload shape (slskd web-interface array format), id extraction from dict / list / fallback responses, and the username-lookup fallback in cancel_download when no username hint is provided. Phase A of the download engine refactor — pinning current behavior of every source BEFORE moving any code so the engine extraction can't drift the per-source contract. Includes the plan doc at docs/download-engine-refactor-plan.md. Pure additive — no client code changes.
This commit is contained in:
Broque Thomas
parent
f9b763587d
commit
52ab9aeb5b
2 changed files with 460 additions and 0 deletions
202
docs/download-engine-refactor-plan.md
Normal file
202
docs/download-engine-refactor-plan.md
Normal file
|
|
@ -0,0 +1,202 @@
|
|||
# Download Engine Refactor Plan
|
||||
|
||||
## Goal
|
||||
|
||||
Mirror Cin's "metadata engine" architecture for the download dispatcher. Move shared logic OUT of the per-source clients (currently 1600+ LOC of duplicated thread workers, search retry ladders, rate-limiters, state machines) and INTO a central `DownloadEngine`. Clients become dumb: make raw API requests + manage their own auth state. Everything else is the engine.
|
||||
|
||||
This is the SAME architectural smell Cin flagged on the metadata layer, applied to downloads. If we keep adding sources (usenet planned + likely more), the only honest fix is to stop reinventing the wheel per client.
|
||||
|
||||
## Architecture target
|
||||
|
||||
```
|
||||
┌─────────────┐ ┌──────────────────┐
|
||||
│ feature │ ── search/download ──▶ ┌────────────────────┐ ─▶│ Soulseek (raw) │
|
||||
│ │ ◀── normalized ──────── │ DownloadEngine │ ─▶│ YouTube (raw) │
|
||||
└─────────────┘ │ │ ─▶│ Tidal (raw) │
|
||||
│ ◆ thread workers │ ─▶│ Qobuz (raw) │
|
||||
│ ◆ rate-limit pool │ ─▶│ HiFi (raw) │
|
||||
│ ◆ search retry │ ─▶│ Deezer (raw) │
|
||||
│ ◆ quality filter │ ─▶│ SoundCloud (raw) │
|
||||
│ ◆ state tracking │ ─▶│ Lidarr (album) │
|
||||
│ ◆ fallback chain │ └──────────────────┘
|
||||
│ ◆ cache │ clients only do:
|
||||
└────────────────────┘ - raw API request
|
||||
- auth/token state
|
||||
```
|
||||
|
||||
## What clients keep (legitimately per-source)
|
||||
|
||||
- Auth flow + token refresh (Tidal OAuth, Qobuz session, Deezer ARL, slskd API key, etc.)
|
||||
- Source-specific protocol (slskd events vs HTTP REST vs HLS demux vs Blowfish decrypt vs yt-dlp subprocess)
|
||||
- Source-specific search query shape (free text vs keyword filters vs MusicBrainz ID lookup)
|
||||
- Source-specific "download a thing" atomic operation (`_download_impl(target_id) → file_path`)
|
||||
|
||||
## What moves into the engine
|
||||
|
||||
| Today (per-client, duplicated) | Tomorrow (engine, single source of truth) |
|
||||
|---|---|
|
||||
| `self.active_downloads = {}` per client | `engine.active_downloads = {}` |
|
||||
| `self._download_lock = Lock()` per client | `engine.state_lock = Lock()` |
|
||||
| `self._download_semaphore = Semaphore(...)` per client | `engine.download_pool` (per-source semaphore from registry) |
|
||||
| `self._last_download_time / _download_delay` per client | `engine.rate_limiter.acquire(source)` |
|
||||
| `_download_thread_worker` × 7 (~70 LOC each) | `engine.dispatch_download(plugin, target_id)` |
|
||||
| Search retry ladder × 7 | `engine.search(query)` with shared retry policy |
|
||||
| Quality filter × 7 | `engine.filter_by_quality(results, prefs)` |
|
||||
| Result dedup × 7 | `engine.dedup(results)` |
|
||||
| Hybrid fallback (search only) | `engine.fallback_chain(operation)` (search AND download) |
|
||||
|
||||
## New plugin contract (much smaller)
|
||||
|
||||
```python
|
||||
class DownloadSourcePlugin(Protocol):
|
||||
# Identity
|
||||
name: str
|
||||
|
||||
# Lifecycle
|
||||
def is_configured(self) -> bool: ...
|
||||
async def check_connection(self) -> bool: ...
|
||||
def reload_settings(self) -> None: ...
|
||||
|
||||
# Search — DUMB. Just hit the API.
|
||||
async def search_raw(self, query: str) -> List[RawSearchResult]: ...
|
||||
|
||||
# Download — DUMB. Just download the bytes to a file.
|
||||
# The engine handles thread spawning, state tracking, rate limits.
|
||||
# Plugin returns the final file path on success or raises.
|
||||
async def download_raw(self, target_id: str, dest_dir: Path) -> Path: ...
|
||||
|
||||
# Cancel — best-effort. Engine handles state cleanup.
|
||||
async def cancel_raw(self, target_id: str) -> bool: ...
|
||||
```
|
||||
|
||||
Compare to today's plugin protocol (which my Phase 0 PR introduced) — that one was wrapped around fat clients. This one is dumber. Clients shrink dramatically (estimated 40-60% LOC reduction per file).
|
||||
|
||||
## Special cases
|
||||
|
||||
- **Soulseek** — slskd is event-driven, NOT thread-based. Engine's BackgroundDownloadWorker doesn't apply. Keep Soulseek's path special: `download_raw` returns immediately; engine subscribes to slskd events for state updates instead of running a thread.
|
||||
- **YouTube/SoundCloud** — yt-dlp is a subprocess. The "thread" is really `subprocess.run(['yt-dlp', ...]).wait()`. Engine handles thread; plugin's `download_raw` just runs subprocess and returns file path.
|
||||
- **Lidarr** — album-grabber, not track-grabber. Different contract. Either separate `AlbumOnlyPlugin` interface OR plugin declares `supports_track_search: bool = False`. Decide during migration.
|
||||
|
||||
## Phased commit plan
|
||||
|
||||
Each phase is one or more commits. Each commit independently revertable. Tests stay green between commits — never ship a half-broken state.
|
||||
|
||||
### Phase A — Behavior pinning tests (BEFORE any code moves)
|
||||
|
||||
**Goal:** Baseline tests for what each source's download path currently does. Catches regressions during extraction.
|
||||
|
||||
**Commit A1:** `tests/downloads/test_soulseek_download_path.py` — pin Soulseek's download lifecycle (search → download → completion → file path returned).
|
||||
**Commit A2:** Same for YouTube. Mock yt-dlp subprocess.
|
||||
**Commit A3:** Same for Tidal. Mock tidalapi.Session.
|
||||
**Commit A4:** Same for Qobuz. Mock Qobuz REST API.
|
||||
**Commit A5:** Same for HiFi. Mock hifi-api instance.
|
||||
**Commit A6:** Same for Deezer. Mock Deezer GW API + Blowfish stream.
|
||||
**Commit A7:** Same for SoundCloud. Mock yt-dlp scsearch.
|
||||
**Commit A8:** Same for Lidarr. Mock Lidarr REST API.
|
||||
|
||||
After Phase A: ~50 new tests pinning current behavior. We can refactor with confidence.
|
||||
|
||||
### Phase B — Engine skeleton + state lift
|
||||
|
||||
**Commit B1:** Create `core/download_engine/` package with `DownloadEngine` class. Engine starts EMPTY — just exposes `register_plugin(plugin)`, `active_downloads` dict, `state_lock`. Orchestrator gets a `self.engine` reference but doesn't use it yet.
|
||||
|
||||
**Commit B2:** Move `active_downloads` state out of every client into `engine.active_downloads`. Each client's `download()` now updates engine state via callback instead of `self.active_downloads[id] = ...`. Backward compat: each client's `self.active_downloads` becomes a property that delegates to `engine.active_downloads.filter(source=self.name)`.
|
||||
|
||||
**Commit B3:** Move `get_all_downloads` / `get_download_status` / `cancel_download` dispatch from orchestrator (which iterates plugins) into engine (which queries unified state). Orchestrator's methods become thin pass-throughs.
|
||||
|
||||
### Phase C — Background download worker lift
|
||||
|
||||
**Commit C1:** New `core/download_engine/worker.py` — `BackgroundDownloadWorker` class. Owns semaphore, rate-limit sleep, state-update lock pattern. Provides `dispatch(plugin, target_id, display_name) → download_id`.
|
||||
|
||||
**Commit C2:** Migrate YouTube to use BackgroundDownloadWorker. Strip `_download_thread_worker` from `youtube_client.py`. Add `download_raw(video_id, dest) → Path`. Tests stay green (Phase A pinned them).
|
||||
|
||||
**Commit C3:** Same for Tidal.
|
||||
**Commit C4:** Same for Qobuz.
|
||||
**Commit C5:** Same for HiFi.
|
||||
**Commit C6:** Same for Deezer.
|
||||
**Commit C7:** Same for SoundCloud.
|
||||
|
||||
After Phase C: ~490 LOC of duplicated thread management deleted. Each affected client shrinks.
|
||||
|
||||
### Phase D — Search retry + quality filter lift
|
||||
|
||||
**Commit D1:** New `core/download_engine/search.py` — `SearchOrchestrator`. Owns: query normalization, shortened-query retry ladder, quality filter, dedup. Calls `plugin.search_raw(query)` for the actual API hit.
|
||||
|
||||
**Commit D2:** Migrate Tidal's search. Strip `_generate_shortened_queries`, quality filter, dedup from client. Add `search_raw(query) → List[RawResult]`.
|
||||
**Commit D3:** Same for Qobuz.
|
||||
**Commit D4:** Same for HiFi.
|
||||
**Commit D5:** Same for YouTube.
|
||||
**Commit D6:** Same for Deezer.
|
||||
**Commit D7:** Same for SoundCloud.
|
||||
**Commit D8:** Same for Soulseek (keep slskd event-driven specifics, but the post-search filter/dedup moves out).
|
||||
**Commit D9:** Same for Lidarr.
|
||||
|
||||
### Phase E — Rate-limit pool
|
||||
|
||||
**Commit E1:** New `core/download_engine/rate_limit.py` — per-source rate limiter registry. Spotify limit, Qobuz 1/sec, etc. Each plugin declares its limits in its registry spec.
|
||||
**Commit E2:** Strip per-client rate-limit state. Replace with `await engine.rate_limit.acquire(self.name)` at the top of `search_raw` / `download_raw`.
|
||||
|
||||
### Phase F — Fallback chain into engine
|
||||
|
||||
**Commit F1:** Engine owns fallback: `engine.search_with_fallback(query, source_chain)` and `engine.download_with_fallback(target_id, source_chain)`. Search hybrid behavior preserved; download hybrid newly works (today it silently routes to one source).
|
||||
**Commit F2:** Orchestrator's `search` and `download` methods delegate to engine's fallback methods. Hybrid mode logic moves out of orchestrator.
|
||||
|
||||
### Phase G — Plugin contract narrows
|
||||
|
||||
**Commit G1:** Update `DownloadSourcePlugin` Protocol — narrow to the small surface (`search_raw`, `download_raw`, `cancel_raw`, `is_configured`, `check_connection`, `reload_settings`). Conformance tests updated.
|
||||
**Commit G2:** Remove dead methods from clients that the engine now owns (`_download_thread_worker`, `_filter_results_by_quality`, etc.). Clean up imports.
|
||||
|
||||
### Phase H — Cleanup + WHATS_NEW + version bump
|
||||
|
||||
**Commit H1:** Final cleanup pass — remove backward-compat shims that are no longer needed (legacy `self.active_downloads` properties etc., once nothing reaches in for them).
|
||||
**Commit H2:** WHATS_NEW entry, PR description.
|
||||
|
||||
## Total estimated scope
|
||||
|
||||
- ~25-30 commits
|
||||
- ~2000 LOC removed (duplicated thread workers, search retries, etc.)
|
||||
- ~1200 LOC added (engine + per-source slim adapters)
|
||||
- Net reduction: ~800 LOC
|
||||
- ~50 new tests (Phase A pinning) + ~20 engine-level tests
|
||||
- 1-2 days of focused work
|
||||
|
||||
## Risk profile
|
||||
|
||||
**Low risk:**
|
||||
- Phase A (only adds tests, never changes behavior)
|
||||
- Phase B1 (new file, doesn't touch existing code)
|
||||
- Phase H (cleanup of dead code)
|
||||
|
||||
**Medium risk:**
|
||||
- Phase B2-B3 (state lift — race conditions, lock contention)
|
||||
- Phase C (thread worker extraction — semaphore semantics, exception propagation)
|
||||
- Phase G (contract narrows — anything reaching in for removed methods breaks)
|
||||
|
||||
**High risk:**
|
||||
- Phase D (search retry — easy to subtly change retry ladder shape)
|
||||
- Phase E (rate-limit — wrong order can cause deadlocks or under-limit violations)
|
||||
- Phase F (fallback — easy to accidentally change hybrid mode behavior)
|
||||
|
||||
**Mitigation:** Phase A pinning tests catch behavior drift in every later phase. Each commit must pass full suite. Manual smoke test per source after Phase C and again at end.
|
||||
|
||||
## Coordination with Cin
|
||||
|
||||
- Cin's metadata engine PR will likely set the precedent for HOW abstractions look (Protocol vs ABC, sync vs async, state location). This plan defaults to Protocol + async (matches what we already have) but easy to mirror Cin's exact pattern when his PR lands.
|
||||
- If his pattern differs significantly, we may need to redo some commits. Best mitigation: don't dig too deep on contract shape (Phase G) until his PR is visible. Phases A-C don't depend on contract shape; they're safe to do regardless.
|
||||
- Send Cin a heads-up before starting — he may have feedback on the plan that saves a redesign later.
|
||||
|
||||
## Compatibility commitments
|
||||
|
||||
- Backward-compat `orchestrator.soulseek` / `orchestrator.youtube` / etc. attributes preserved through every phase.
|
||||
- Backward-compat `clear_all_searches`, `_make_request`, `signal_download_completion`, etc. (Soulseek-specific reaches) preserved through every phase.
|
||||
- Frontend status dashboard keys (`deezer_dl` alias) preserved.
|
||||
- Config format unchanged.
|
||||
- DB schema unchanged.
|
||||
- API endpoint surface unchanged.
|
||||
|
||||
## What's NOT in this PR
|
||||
|
||||
- Cin's metadata engine work (separate, his domain)
|
||||
- Media server client refactor (different subsystem, separate PR)
|
||||
- Match engine refactor (different subsystem, separate PR)
|
||||
- Adding new download sources (out of scope; the new contract makes them easier later)
|
||||
258
tests/downloads/test_soulseek_pinning.py
Normal file
258
tests/downloads/test_soulseek_pinning.py
Normal file
|
|
@ -0,0 +1,258 @@
|
|||
"""Phase A pinning tests for SoulseekClient's download lifecycle.
|
||||
|
||||
These tests pin the OBSERVABLE BEHAVIOR of `SoulseekClient.download` /
|
||||
`get_all_downloads` / `cancel_download` so the upcoming download
|
||||
engine refactor (which lifts shared state + thread workers + search
|
||||
retry into a central engine) can't drift the per-source contract.
|
||||
|
||||
The contract these tests pin is what the engine will call into via
|
||||
`plugin.download_raw(target_id)` / `plugin.cancel_raw(target_id)`
|
||||
after the refactor lands. If a future commit breaks any of these
|
||||
expectations, the diff fails fast — long before a real download
|
||||
attempt against a live slskd would have surfaced the bug.
|
||||
|
||||
NOTE: Soulseek is structurally different from the streaming sources.
|
||||
It has NO local thread worker — slskd manages downloads server-side
|
||||
and the client just polls for state. So Soulseek skips most of the
|
||||
engine refactor's thread-extraction work; what stays critical is
|
||||
the slskd HTTP API contract (endpoints, payload shape, id
|
||||
extraction). That's what these tests pin.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
from pathlib import Path
|
||||
from unittest.mock import AsyncMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from core.soulseek_client import SoulseekClient
|
||||
|
||||
|
||||
def _run_async(coro):
|
||||
loop = asyncio.new_event_loop()
|
||||
try:
|
||||
return loop.run_until_complete(coro)
|
||||
finally:
|
||||
loop.close()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Configuration / lifecycle
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_is_configured_returns_false_when_no_base_url():
|
||||
"""Pinning: an unconfigured client (no slskd URL set) reports
|
||||
is_configured() == False. The orchestrator's hybrid fallback +
|
||||
every consumer that gates on is_configured() depends on this."""
|
||||
client = SoulseekClient.__new__(SoulseekClient)
|
||||
client.base_url = None
|
||||
client.api_key = None
|
||||
assert client.is_configured() is False
|
||||
|
||||
|
||||
def test_is_configured_returns_true_when_base_url_set():
|
||||
"""Pinning: configured client (slskd URL present) reports True."""
|
||||
client = SoulseekClient.__new__(SoulseekClient)
|
||||
client.base_url = 'http://localhost:5030'
|
||||
client.api_key = 'test-key'
|
||||
assert client.is_configured() is True
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# download()
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def configured_client():
|
||||
"""A SoulseekClient with the slskd URL set but no real network. Tests
|
||||
individually patch `_make_request` to return whatever shape they
|
||||
want to exercise."""
|
||||
client = SoulseekClient.__new__(SoulseekClient)
|
||||
client.base_url = 'http://localhost:5030'
|
||||
client.api_key = 'test-key'
|
||||
client.download_path = Path('./test_downloads')
|
||||
return client
|
||||
|
||||
|
||||
def test_download_returns_none_when_not_configured():
|
||||
"""Pinning: an unconfigured client refuses downloads — returns
|
||||
None silently rather than raising. Used as the soft-fail signal
|
||||
by the orchestrator's per-source fallback chain."""
|
||||
client = SoulseekClient.__new__(SoulseekClient)
|
||||
client.base_url = None
|
||||
result = _run_async(client.download('user', 'song.flac', 1024))
|
||||
assert result is None
|
||||
|
||||
|
||||
def test_download_hits_transfers_downloads_username_endpoint(configured_client):
|
||||
"""Pinning: the primary download endpoint is
|
||||
`transfers/downloads/<username>` POST. This shape was chosen to
|
||||
match slskd's web-interface API exactly. Changing it breaks
|
||||
every download against current slskd builds."""
|
||||
captured = []
|
||||
|
||||
async def fake_request(method, endpoint, json=None, **kwargs):
|
||||
captured.append((method, endpoint, json))
|
||||
return {'id': 'dl-id-from-slskd'}
|
||||
|
||||
with patch.object(configured_client, '_make_request', side_effect=fake_request):
|
||||
result = _run_async(configured_client.download('user', 'song.flac', 1024))
|
||||
|
||||
assert result == 'dl-id-from-slskd'
|
||||
method, endpoint, payload = captured[0]
|
||||
assert method == 'POST'
|
||||
assert endpoint == 'transfers/downloads/user'
|
||||
# Payload is the slskd web-interface array format.
|
||||
assert isinstance(payload, list)
|
||||
assert payload[0]['filename'] == 'song.flac'
|
||||
assert payload[0]['size'] == 1024
|
||||
|
||||
|
||||
def test_download_extracts_id_from_dict_response(configured_client):
|
||||
"""Pinning: when slskd returns `{id: ...}`, that's the
|
||||
download_id the orchestrator uses to track the download."""
|
||||
with patch.object(configured_client, '_make_request',
|
||||
AsyncMock(return_value={'id': 'abc123'})):
|
||||
result = _run_async(configured_client.download('user', 'song.flac', 1024))
|
||||
assert result == 'abc123'
|
||||
|
||||
|
||||
def test_download_extracts_id_from_list_response(configured_client):
|
||||
"""Pinning: slskd sometimes returns a list of file objects.
|
||||
The first item's id is the download_id."""
|
||||
with patch.object(configured_client, '_make_request',
|
||||
AsyncMock(return_value=[{'id': 'list-id'}, {'id': 'second'}])):
|
||||
result = _run_async(configured_client.download('user', 'song.flac', 1024))
|
||||
assert result == 'list-id'
|
||||
|
||||
|
||||
def test_download_falls_back_to_filename_when_no_id_in_response(configured_client):
|
||||
"""Pinning: defensive — older slskd builds returned 201 Created
|
||||
with no id field. The client uses the filename as the download
|
||||
identifier in that case so downstream tracking still works."""
|
||||
with patch.object(configured_client, '_make_request',
|
||||
AsyncMock(return_value={'status': 'queued'})):
|
||||
result = _run_async(configured_client.download('user', 'song.flac', 1024))
|
||||
assert result == 'song.flac'
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# get_all_downloads()
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_get_all_downloads_returns_empty_when_not_configured():
|
||||
client = SoulseekClient.__new__(SoulseekClient)
|
||||
client.base_url = None
|
||||
result = _run_async(client.get_all_downloads())
|
||||
assert result == []
|
||||
|
||||
|
||||
def test_get_all_downloads_parses_nested_user_directory_files_response(configured_client):
|
||||
"""Pinning: slskd's `transfers/downloads` returns
|
||||
`[{username, directories: [{files: [...]}]}]`. The client
|
||||
flattens that into a list of DownloadStatus objects, one per
|
||||
file. Engine refactor's state aggregation depends on this shape."""
|
||||
fake_response = [
|
||||
{
|
||||
'username': 'peer1',
|
||||
'directories': [{
|
||||
'files': [
|
||||
{'id': 'f1', 'filename': 'a.flac', 'state': 'InProgress',
|
||||
'size': 100, 'bytesTransferred': 50, 'averageSpeed': 1024},
|
||||
{'id': 'f2', 'filename': 'b.flac', 'state': 'Completed, Succeeded',
|
||||
'size': 200, 'bytesTransferred': 200, 'averageSpeed': 2048},
|
||||
],
|
||||
}],
|
||||
},
|
||||
]
|
||||
|
||||
with patch.object(configured_client, '_make_request',
|
||||
AsyncMock(return_value=fake_response)):
|
||||
result = _run_async(configured_client.get_all_downloads())
|
||||
|
||||
assert len(result) == 2
|
||||
assert result[0].id == 'f1'
|
||||
assert result[0].username == 'peer1'
|
||||
assert result[0].state == 'InProgress'
|
||||
assert result[1].id == 'f2'
|
||||
# Pinning: 'Completed' state forces progress=100 regardless of source data.
|
||||
assert result[1].progress == 100.0
|
||||
|
||||
|
||||
def test_get_all_downloads_endpoint_is_transfers_downloads(configured_client):
|
||||
"""Pinning: the listing endpoint is `transfers/downloads` (no
|
||||
username). The 404'd `users/.../downloads` variant was tried
|
||||
once and removed — keep it gone."""
|
||||
captured = []
|
||||
|
||||
async def fake_request(method, endpoint, **kwargs):
|
||||
captured.append((method, endpoint))
|
||||
return []
|
||||
|
||||
with patch.object(configured_client, '_make_request', side_effect=fake_request):
|
||||
_run_async(configured_client.get_all_downloads())
|
||||
|
||||
assert captured == [('GET', 'transfers/downloads')]
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# cancel_download()
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_cancel_download_returns_false_when_not_configured():
|
||||
client = SoulseekClient.__new__(SoulseekClient)
|
||||
client.base_url = None
|
||||
result = _run_async(client.cancel_download('dl-id', 'user', remove=False))
|
||||
assert result is False
|
||||
|
||||
|
||||
def test_cancel_download_looks_up_username_when_not_provided(configured_client):
|
||||
"""Pinning: orchestrator may call cancel_download without a
|
||||
username hint. The client falls back to scanning all downloads
|
||||
to find which peer owns it. Engine refactor must preserve this
|
||||
so existing API endpoints that don't pass username keep working."""
|
||||
fake_listing = [
|
||||
{
|
||||
'username': 'peer-owner',
|
||||
'directories': [{
|
||||
'files': [{'id': 'target-dl', 'filename': 'x.flac',
|
||||
'state': 'InProgress', 'size': 0,
|
||||
'bytesTransferred': 0, 'averageSpeed': 0}],
|
||||
}],
|
||||
},
|
||||
]
|
||||
|
||||
captured_endpoints = []
|
||||
|
||||
async def fake_request(method, endpoint, **kwargs):
|
||||
captured_endpoints.append((method, endpoint))
|
||||
if method == 'GET' and endpoint == 'transfers/downloads':
|
||||
return fake_listing
|
||||
# The DELETE for cancel — return success
|
||||
return True
|
||||
|
||||
with patch.object(configured_client, '_make_request', side_effect=fake_request):
|
||||
_run_async(configured_client.cancel_download('target-dl', None, remove=False))
|
||||
|
||||
# The lookup hit get_all_downloads first, then the DELETE used the discovered username.
|
||||
assert ('GET', 'transfers/downloads') in captured_endpoints
|
||||
delete_calls = [(m, e) for m, e in captured_endpoints if m == 'DELETE']
|
||||
assert delete_calls, "Expected at least one DELETE after username lookup"
|
||||
# The cancel endpoint URL contains the discovered username.
|
||||
assert any('peer-owner' in e for _, e in delete_calls)
|
||||
|
||||
|
||||
def test_cancel_download_returns_false_when_username_lookup_fails(configured_client):
|
||||
"""Pinning: if the download_id isn't in the active list, return
|
||||
False rather than raising. Orchestrator treats False as "couldn't
|
||||
cancel" and continues; an exception would propagate to the user."""
|
||||
with patch.object(configured_client, '_make_request',
|
||||
AsyncMock(return_value=[])):
|
||||
result = _run_async(configured_client.cancel_download('missing-id', None))
|
||||
assert result is False
|
||||
Loading…
Reference in a new issue