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.
This commit is contained in:
Alex Shnitman
2026-07-27 21:19:01 +03:00
parent 1a09dbd686
commit ff1b73a576
4 changed files with 23 additions and 7 deletions
+12 -4
View File
@@ -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):
+9 -1
View File
@@ -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}})
@@ -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' });
});
+1 -1
View File
@@ -170,7 +170,7 @@ export class DownloadsService {
}
public retry(id: string) {
return this.http.post<Status>('retry', { ids: [id] }).pipe(
return this.http.post<Status>('retry', { id: id }).pipe(
catchError(this.handleHTTPError)
);
}