danvics/soulsync
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 1

Amazon download client: fix engine API calls in status methods

Browse source 
`get_all_downloads` was calling `engine.get_all_records()` — a method that
doesn't exist on DownloadEngine. Same story for `cancel_record` and
`clear_completed`. The engine exposes `iter_records_for_source`, `get_record`,
`update_record`, and `remove_record` — matching what every other streaming
client (Deezer, HiFi, Qobuz, SoundCloud, Tidal, YouTube) already uses.

With `get_all_downloads` silently returning `[]` on every call (the missing
method raised, `except Exception: return []` swallowed it), the download monitor
never saw Amazon records as complete — tasks stayed stuck at 0% even after the
file had fully downloaded.

Changes:
- `get_all_downloads` → `iter_records_for_source('amazon')`
- `get_download_status` → `get_record('amazon', id)`, no try/except
- `cancel_download` → `get_record` check + `update_record` (Cancelled) +
  optional `remove_record` — same pattern as deezer/hifi/etc
- `clear_all_completed_downloads` → iterate + `remove_record` for terminal
  states; returns True on no-engine (nothing to clear = success)
- `_record_to_status` drops the `download_id` argument; reads `rec['id']`
  instead (worker stores `'id'` in every record — `iter_records_for_source`
  returns the full record dict)

Tests updated to match: `iter_records_for_source` mock replaces
`get_all_records`, cancel test verifies `update_record`+`remove_record`,
clear test verifies only terminal-state records are removed, graceful-error
test replaced with no-records boundary test (exception propagation is handled
at the engine aggregator layer, not per-plugin).
This commit is contained in:
Broque Thomas 2026-05-16 10:49:11 -07:00
parent ebda0b8613
commit b4403ed393
2 changed files with 78 additions and 57 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

56
core/amazon_download_client.py
View file

@ -375,20 +375,16 @@ class AmazonDownloadClient(DownloadSourcePlugin):
async def get_all_downloads(self) -> List[DownloadStatus]:
if self._engine is None:
return []
try:
records = self._engine.get_all_records("amazon")
return [self._record_to_status(dl_id, rec) for dl_id, rec in records.items()]
except Exception:
return []
return [
self._record_to_status(record)
for record in self._engine.iter_records_for_source('amazon')
]
async def get_download_status(self, download_id: str) -> Optional[DownloadStatus]:
if self._engine is None:
return None
try:
rec = self._engine.get_record("amazon", download_id)
return self._record_to_status(download_id, rec) if rec is not None else None
except Exception:
return None
record = self._engine.get_record('amazon', download_id)
return self._record_to_status(record) if record is not None else None
async def cancel_download(
self,
@ -398,19 +394,21 @@ class AmazonDownloadClient(DownloadSourcePlugin):
) -> bool:
if self._engine is None:
return False
try:
return self._engine.cancel_record("amazon", download_id, remove=remove)
except Exception:
if self._engine.get_record('amazon', download_id) is None:
return False
self._engine.update_record('amazon', download_id, {'state': 'Cancelled'})
if remove:
self._engine.remove_record('amazon', download_id)
return True
async def clear_all_completed_downloads(self) -> bool:
if self._engine is None:
return False
try:
self._engine.clear_completed("amazon")
return True
except Exception:
return False
terminal = {'Completed, Succeeded', 'Cancelled', 'Errored', 'Aborted'}
for record in list(self._engine.iter_records_for_source('amazon')):
if record.get('state') in terminal:
self._engine.remove_record('amazon', record['id'])
return True
# ------------------------------------------------------------------
# Private helpers
@ -428,16 +426,16 @@ class AmazonDownloadClient(DownloadSourcePlugin):
return path.with_name(f"{stem}_{uuid.uuid4().hex[:8]}{suffix}")
@staticmethod
def _record_to_status(download_id: str, rec: Dict[str, Any]) -> DownloadStatus:
def _record_to_status(rec: Dict[str, Any]) -> DownloadStatus:
return DownloadStatus(
id=download_id,
filename=str(rec.get("filename", "")),
username="amazon",
state=str(rec.get("state", "queued")),
progress=float(rec.get("progress", 0.0)),
size=int(rec.get("size", 0)),
transferred=int(rec.get("transferred", 0)),
speed=int(rec.get("speed", 0)),
time_remaining=rec.get("time_remaining"),
file_path=rec.get("file_path"),
id=str(rec.get('id', '')),
filename=str(rec.get('filename', '')),
username='amazon',
state=str(rec.get('state', 'queued')),
progress=float(rec.get('progress', 0.0)),
size=int(rec.get('size', 0)),
transferred=int(rec.get('transferred', 0)),
speed=int(rec.get('speed', 0)),
time_remaining=rec.get('time_remaining'),
file_path=rec.get('file_path'),
)

79
tests/tools/test_amazon_download_client.py
View file

@ -325,6 +325,7 @@ class TestUniquePath:
class TestRecordToStatus:
def test_fields_mapped(self):
rec = {
"id": "dl-001",
"filename": "B1||Artist - Title",
"state": "downloading",
"progress": 0.5,
@ -334,7 +335,7 @@ class TestRecordToStatus:
"time_remaining": 5,
"file_path": "/tmp/track.flac",
}
status = AmazonDownloadClient._record_to_status("dl-001", rec)
status = AmazonDownloadClient._record_to_status(rec)
assert status.id == "dl-001"
assert status.filename == "B1||Artist - Title"
@ -348,7 +349,8 @@ class TestRecordToStatus:
assert status.file_path == "/tmp/track.flac"
def test_defaults_for_missing_fields(self):
status = AmazonDownloadClient._record_to_status("dl-002", {})
status = AmazonDownloadClient._record_to_status({})
assert status.id == ""
assert status.state == "queued"
assert status.progress == 0.0
assert status.size == 0
@ -716,22 +718,22 @@ class TestStatusInterface:
def test_get_all_downloads_converts_records(self, tmp_path):
client = _make_client(tmp_path)
engine = MagicMock()
engine.get_all_records.return_value = {
"dl-001": {
"filename": "B1||A - T",
"state": "complete",
"progress": 1.0,
"size": 5_000_000,
"transferred": 5_000_000,
"speed": 0,
}
}
engine.iter_records_for_source.return_value = iter([{
"id": "dl-001",
"filename": "B1||A - T",
"state": "complete",
"progress": 1.0,
"size": 5_000_000,
"transferred": 5_000_000,
"speed": 0,
}])
client._engine = engine
statuses = run(client.get_all_downloads())
assert len(statuses) == 1
assert statuses[0].id == "dl-001"
assert statuses[0].state == "complete"
engine.iter_records_for_source.assert_called_once_with('amazon')
def test_get_download_status_returns_none_without_engine(self, tmp_path):
client = _make_client(tmp_path)
@ -741,7 +743,8 @@ class TestStatusInterface:
client = _make_client(tmp_path)
engine = MagicMock()
engine.get_record.return_value = {
"original_filename": "B1||A - T",
"id": "dl-001",
"filename": "B1||A - T",
"state": "downloading",
"progress": 0.7,
"size": 10_000_000,
@ -752,9 +755,10 @@ class TestStatusInterface:
status = run(client.get_download_status("dl-001"))
assert status is not None
assert status.id == "dl-001"
assert status.state == "downloading"
assert status.progress == 0.7
engine.get_record.assert_called_once_with("amazon", "dl-001")
engine.get_record.assert_called_once_with('amazon', 'dl-001')
def test_get_download_status_miss(self, tmp_path):
client = _make_client(tmp_path)
@ -771,36 +775,55 @@ class TestStatusInterface:
def test_cancel_delegates_to_engine(self, tmp_path):
client = _make_client(tmp_path)
engine = MagicMock()
engine.cancel_record.return_value = True
engine.get_record.return_value = {'id': 'dl-001', 'state': 'downloading'}
client._engine = engine
result = run(client.cancel_download("dl-001", remove=True))
assert result is True
engine.cancel_record.assert_called_once_with("amazon", "dl-001", remove=True)
engine.update_record.assert_called_once_with('amazon', 'dl-001', {'state': 'Cancelled'})
engine.remove_record.assert_called_once_with('amazon', 'dl-001')
def test_clear_completed_returns_false_without_engine(self, tmp_path):
client = _make_client(tmp_path)
assert run(client.clear_all_completed_downloads()) is False
def test_clear_completed_delegates_to_engine(self, tmp_path):
def test_cancel_returns_false_when_record_not_found(self, tmp_path):
client = _make_client(tmp_path)
engine = MagicMock()
engine.get_record.return_value = None
client._engine = engine
result = run(client.cancel_download("nonexistent"))
assert result is False
engine.update_record.assert_not_called()
def test_clear_completed_returns_true_without_engine(self, tmp_path):
client = _make_client(tmp_path)
assert run(client.clear_all_completed_downloads()) is True
def test_clear_completed_removes_terminal_records(self, tmp_path):
client = _make_client(tmp_path)
engine = MagicMock()
engine.iter_records_for_source.return_value = iter([
{'id': 'dl-001', 'state': 'Completed, Succeeded'},
{'id': 'dl-002', 'state': 'InProgress, Downloading'},
{'id': 'dl-003', 'state': 'Errored'},
])
client._engine = engine
result = run(client.clear_all_completed_downloads())
assert result is True
engine.clear_completed.assert_called_once_with("amazon")
# Only terminal records removed
removed_ids = [c[0][1] for c in engine.remove_record.call_args_list]
assert 'dl-001' in removed_ids
assert 'dl-003' in removed_ids
assert 'dl-002' not in removed_ids
def test_status_methods_return_gracefully_on_engine_error(self, tmp_path):
def test_status_methods_no_records(self, tmp_path):
"""Engine with no Amazon records returns empty/None gracefully."""
client = _make_client(tmp_path)
engine = MagicMock()
engine.get_all_records.side_effect = Exception("engine crash")
engine.get_record.side_effect = Exception("engine crash")
engine.cancel_record.side_effect = Exception("engine crash")
engine.clear_completed.side_effect = Exception("engine crash")
engine.iter_records_for_source.return_value = iter([])
engine.get_record.return_value = None
client._engine = engine
assert run(client.get_all_downloads()) == []
assert run(client.get_download_status("dl-001")) is None
assert run(client.cancel_download("dl-001")) is False
assert run(client.clear_all_completed_downloads()) is False
assert run(client.clear_all_completed_downloads()) is True