From ff1b73a576278fa094da8626f32df27e1226cf95 Mon Sep 17 00:00:00 2001 From: Alex Shnitman Date: Mon, 27 Jul 2026 21:19:01 +0300 Subject: [PATCH] refactor: make POST /retry take a singular id The endpoint accepted {ids: [x]} and then rejected anything but exactly one id, so the schema advertised a batch it never supported. Retry is genuinely singular: unlike the /delete, /start and /cancel batches, which act on local state and can't meaningfully fail for one id and not another, each retry re-extracts the URL and the caller removes that item's done record only once it is confirmed re-queued. A real batch form would need per-id results in the response for the caller to know which records to remove; that only becomes worth designing alongside moving done-record deletion server-side. /retry has not shipped yet, so there is no compatibility cost. --- app/main.py | 16 ++++++++++++---- app/tests/test_api.py | 10 +++++++++- ui/src/app/services/downloads.service.spec.ts | 2 +- ui/src/app/services/downloads.service.ts | 2 +- 4 files changed, 23 insertions(+), 7 deletions(-) diff --git a/app/main.py b/app/main.py index 5a381a5..2f7a628 100644 --- a/app/main.py +++ b/app/main.py @@ -895,11 +895,12 @@ async def cancel_add(request): @routes.post(config.URL_PREFIX + 'retry') async def retry(request): + # Singular by design, unlike the 'ids' batch endpoints: a retry re-extracts + # the URL, so it can fail per item, and the caller removes that item's done + # record only once it is confirmed re-queued. A batch form would have to + # report per-id results for the caller to know which ones to remove. post = await _read_json_request(request) - ids = _require_id_list(post) - if len(ids) != 1: - raise web.HTTPBadRequest(reason="'ids' must contain exactly one download id") - status = await dqueue.retry(ids[0]) + status = await dqueue.retry(_require_id(post)) return web.Response(text=serializer.encode(status), content_type='application/json') @@ -995,6 +996,13 @@ async def subscriptions_check(request): result = await submgr.check_now([str(i) for i in ids] if ids else None) return web.Response(text=serializer.encode(result)) +def _require_id(post: dict) -> str: + id = post.get('id') + if not isinstance(id, str) or not id: + raise web.HTTPBadRequest(reason="'id' must be a non-empty string") + return id + + def _require_id_list(post: dict) -> list: ids = post.get('ids') if not isinstance(ids, list) or not ids or not all(isinstance(i, str) for i in ids): diff --git a/app/tests/test_api.py b/app/tests/test_api.py index e1ea96a..629209f 100644 --- a/app/tests/test_api.py +++ b/app/tests/test_api.py @@ -72,12 +72,20 @@ async def test_add_ok(mock_dqueue): @pytest.mark.asyncio async def test_retry_passes_failed_download_id(mock_dqueue): - req = _json_request({"ids": ["https://example.com/watch?v=1"]}) + req = _json_request({"id": "https://example.com/watch?v=1"}) resp = await main.retry(req) assert resp.status == 200 mock_dqueue.retry.assert_awaited_once_with("https://example.com/watch?v=1") +@pytest.mark.asyncio +@pytest.mark.parametrize("body", [{}, {"id": ""}, {"id": ["a"]}, {"ids": ["a"]}]) +async def test_retry_rejects_missing_or_non_string_id(mock_dqueue, body): + with pytest.raises(web.HTTPBadRequest): + await main.retry(_json_request(body)) + mock_dqueue.retry.assert_not_awaited() + + @pytest.mark.asyncio async def test_add_passes_preset_and_overrides(mock_dqueue, monkeypatch): monkeypatch.setattr(main.config, "YTDL_OPTIONS_PRESETS", {"Preset A": {"writesubtitles": True}}) diff --git a/ui/src/app/services/downloads.service.spec.ts b/ui/src/app/services/downloads.service.spec.ts index 237bd91..f271a56 100644 --- a/ui/src/app/services/downloads.service.spec.ts +++ b/ui/src/app/services/downloads.service.spec.ts @@ -121,7 +121,7 @@ describe('DownloadsService', () => { service.retry('https://example.com/v').subscribe(); const req = httpMock.expectOne('retry'); expect(req.request.method).toBe('POST'); - expect(req.request.body).toEqual({ ids: ['https://example.com/v'] }); + expect(req.request.body).toEqual({ id: 'https://example.com/v' }); req.flush({ status: 'ok' }); }); diff --git a/ui/src/app/services/downloads.service.ts b/ui/src/app/services/downloads.service.ts index 5086fd8..b3c6ab7 100644 --- a/ui/src/app/services/downloads.service.ts +++ b/ui/src/app/services/downloads.service.ts @@ -170,7 +170,7 @@ export class DownloadsService { } public retry(id: string) { - return this.http.post('retry', { ids: [id] }).pipe( + return this.http.post('retry', { id: id }).pipe( catchError(this.handleHTTPError) ); }