diff --git a/app/features/tasks/router.py b/app/features/tasks/router.py index 97fbd0f8..9cd23f19 100644 --- a/app/features/tasks/router.py +++ b/app/features/tasks/router.py @@ -151,6 +151,22 @@ def _serialize(model: Any) -> dict: return _model(model).model_dump() +async def _require_timer_or_handler(task: Task, handler: TaskHandle) -> None: + if task.timer: + return + + if task.handler_enabled is False: + msg = "Task requires a timer when the handler is disabled." + raise ValueError(msg) + + result = await handler.inspect(url=task.url, preset=task.preset, static_only=True) + if isinstance(result, TaskResult) and result.metadata.get("matched") is True: + return + + msg = "Task requires a timer when no supported handler matches the URL." + raise ValueError(msg) + + @route("GET", "api/tasks/", "tasks_list") async def tasks_list(request: Request, repo: TasksRepository, encoder: Encoder) -> Response: page, per_page = normalize_pagination(request) @@ -166,7 +182,13 @@ async def tasks_list(request: Request, repo: TasksRepository, encoder: Encoder) @route("POST", "api/tasks/", "tasks_add") -async def tasks_add(request: Request, repo: TasksRepository, encoder: Encoder, notify: EventBus) -> Response: +async def tasks_add( + request: Request, + repo: TasksRepository, + encoder: Encoder, + notify: EventBus, + handler: TaskHandle, +) -> Response: data = await request.json() if isinstance(data, dict): @@ -205,6 +227,7 @@ async def tasks_add(request: Request, repo: TasksRepository, encoder: Encoder, n status=web.HTTPConflict.status_code, ) + pending_tasks: list[dict[str, Any]] = [] created_tasks: list[dict[str, Any]] = [] base_settings = first_task.model_dump(exclude={"id", "name", "url", "created_at", "updated_at"}) @@ -243,11 +266,23 @@ async def tasks_add(request: Request, repo: TasksRepository, encoder: Encoder, n try: validated = Task.model_validate(task_dict) - task_data = validated.model_dump() except ValidationError as exc: - LOG.warning(f"Skipping task {idx}: validation failed - {exc}") - continue + return web.json_response( + data={"error": "Failed to validate task.", "detail": format_validation_errors(exc)}, + status=web.HTTPBadRequest.status_code, + ) + try: + await _require_timer_or_handler(validated, handler) + except ValueError as exc: + return web.json_response( + data={"error": str(exc)}, + status=web.HTTPBadRequest.status_code, + ) + + pending_tasks.append(validated.model_dump()) + + for idx, task_data in enumerate(pending_tasks): try: created = await repo.create(task_data) saved = _serialize(created) @@ -304,7 +339,13 @@ async def tasks_delete(request: Request, repo: TasksRepository, encoder: Encoder @route("PATCH", r"api/tasks/{id:\d+}", "tasks_patch") -async def tasks_patch(request: Request, repo: TasksRepository, encoder: Encoder, notify: EventBus) -> Response: +async def tasks_patch( + request: Request, + repo: TasksRepository, + encoder: Encoder, + notify: EventBus, + handler: TaskHandle, +) -> Response: if not (identifier := request.match_info.get("id")): return web.json_response({"error": "ID required"}, status=web.HTTPBadRequest.status_code) @@ -334,6 +375,16 @@ async def tasks_patch(request: Request, repo: TasksRepository, encoder: Encoder, status=web.HTTPConflict.status_code, ) + merged = _model(model).model_copy(update=validated.model_dump(exclude_unset=True)) + + try: + await _require_timer_or_handler(merged, handler) + except ValueError as exc: + return web.json_response( + data={"error": str(exc)}, + status=web.HTTPBadRequest.status_code, + ) + updated = _serialize(await repo.update(model.id, validated.model_dump(exclude_unset=True))) notify.emit(Events.CONFIG_UPDATE, data=ConfigEvent(feature=CEFeature.TASKS, action=CEAction.UPDATE, data=updated)) return web.json_response( @@ -344,7 +395,13 @@ async def tasks_patch(request: Request, repo: TasksRepository, encoder: Encoder, @route("PUT", r"api/tasks/{id:\d+}", "tasks_update") -async def tasks_update(request: Request, repo: TasksRepository, encoder: Encoder, notify: EventBus) -> Response: +async def tasks_update( + request: Request, + repo: TasksRepository, + encoder: Encoder, + notify: EventBus, + handler: TaskHandle, +) -> Response: if not (identifier := request.match_info.get("id")): return web.json_response({"error": "ID required"}, status=web.HTTPBadRequest.status_code) @@ -374,6 +431,14 @@ async def tasks_update(request: Request, repo: TasksRepository, encoder: Encoder status=web.HTTPConflict.status_code, ) + try: + await _require_timer_or_handler(validated, handler) + except ValueError as exc: + return web.json_response( + data={"error": str(exc)}, + status=web.HTTPBadRequest.status_code, + ) + updated = _serialize(await repo.update(model.id, validated.model_dump(exclude_unset=True))) notify.emit(Events.CONFIG_UPDATE, data=ConfigEvent(feature=CEFeature.TASKS, action=CEAction.UPDATE, data=updated)) diff --git a/app/features/tasks/tests/test_tasks_router.py b/app/features/tasks/tests/test_tasks_router.py new file mode 100644 index 00000000..d3064427 --- /dev/null +++ b/app/features/tasks/tests/test_tasks_router.py @@ -0,0 +1,183 @@ +from __future__ import annotations + +from types import SimpleNamespace + +import pytest +import pytest_asyncio +from aiohttp import web +from aiohttp.test_utils import make_mocked_request + +from app.features.tasks import router +from app.features.tasks.definitions.results import TaskFailure, TaskResult +from app.features.tasks.repository import TasksRepository +from app.library.encoder import Encoder +from app.library.sqlite_store import SqliteStore +from app.tests.helpers import make_in_memory_db_path + + +@pytest_asyncio.fixture +async def repo(): + TasksRepository._reset_singleton() + SqliteStore._reset_singleton() + + store = SqliteStore(db_path=make_in_memory_db_path("tasks-router")) + await store.get_connection() + + repository = TasksRepository.get_instance() + + yield repository + + await store.close() + TasksRepository._reset_singleton() + SqliteStore._reset_singleton() + + +@pytest.fixture(autouse=True) +def patch_get_info(monkeypatch: pytest.MonkeyPatch) -> None: + async def _fake_get_info(_url: str, _preset: str) -> tuple[None, None]: + return (None, None) + + monkeypatch.setattr(router, "_get_info", _fake_get_info) + + +def _json_request(method: str, path: str, payload: object, *, match_info: dict[str, str] | None = None) -> web.Request: + request = make_mocked_request(method, path, match_info=match_info or {}, app=SimpleNamespace()) + + async def _json() -> object: + return payload + + request.json = _json # type: ignore[attr-defined] + return request + + +class _Notify: + def emit(self, *_args, **_kwargs) -> None: + return None + + +class _Handler: + def __init__(self, matched: bool | dict[str, bool]) -> None: + self._matched = matched + + async def inspect(self, *, url: str, preset: str | None = None, static_only: bool = False, **_kwargs): + del preset, static_only + + if isinstance(self._matched, dict): + matched = self._matched.get(url, False) + else: + matched = self._matched + + if matched: + return TaskResult(metadata={"matched": True, "handler": "TestHandler"}) + + return TaskFailure(message="No handler", metadata={"matched": False, "handler": None}) + + +@pytest.mark.asyncio +async def test_add_requires_timer_without_handler(repo) -> None: + request = _json_request( + "POST", + "/api/tasks/", + {"name": "No Timer", "url": "https://example.com/channel"}, + ) + + response = await router.tasks_add(request, repo, Encoder(), _Notify(), _Handler(matched=False)) + + assert response.status == web.HTTPBadRequest.status_code + assert b"requires a timer" in response.body + assert await repo.list() == [] + + +@pytest.mark.asyncio +async def test_add_all_or_nothing(repo) -> None: + request = _json_request( + "POST", + "/api/tasks/", + [ + {"name": "First", "url": "https://example.com/first"}, + {"url": "https://example.com/second"}, + ], + ) + + response = await router.tasks_add( + request, + repo, + Encoder(), + _Notify(), + _Handler({"https://example.com/first": True, "https://example.com/second": False}), + ) + + assert response.status == web.HTTPBadRequest.status_code + assert b"requires a timer" in response.body + assert await repo.list() == [] + + +@pytest.mark.asyncio +async def test_add_allows_handler_only(repo) -> None: + request = _json_request( + "POST", + "/api/tasks/", + {"name": "Handler Only", "url": "https://example.com/feed"}, + ) + + response = await router.tasks_add(request, repo, Encoder(), _Notify(), _Handler(matched=True)) + + assert response.status == web.HTTPOk.status_code + items = await repo.list() + assert len(items) == 1 + assert items[0].name == "Handler Only" + + +@pytest.mark.asyncio +async def test_update_requires_timer_without_handler(repo) -> None: + item = await repo.create({"name": "Needs Timer", "url": "https://example.com/a", "timer": "0 0 * * *"}) + + request = _json_request( + "PUT", + f"/api/tasks/{item.id}", + {"name": item.name, "url": item.url, "timer": "", "preset": "", "folder": "", "template": "", "cli": ""}, + match_info={"id": str(item.id)}, + ) + + response = await router.tasks_update(request, repo, Encoder(), _Notify(), _Handler(matched=False)) + + assert response.status == web.HTTPBadRequest.status_code + refreshed = await repo.get(item.id) + assert refreshed is not None + assert refreshed.timer == "0 0 * * *" + + +@pytest.mark.asyncio +async def test_patch_requires_timer_without_handler(repo) -> None: + item = await repo.create({"name": "Patch Timer", "url": "https://example.com/b", "timer": "0 0 * * *"}) + + request = _json_request( + "PATCH", + f"/api/tasks/{item.id}", + {"timer": ""}, + match_info={"id": str(item.id)}, + ) + + response = await router.tasks_patch(request, repo, Encoder(), _Notify(), _Handler(matched=False)) + + assert response.status == web.HTTPBadRequest.status_code + refreshed = await repo.get(item.id) + assert refreshed is not None + assert refreshed.timer == "0 0 * * *" + + +@pytest.mark.asyncio +async def test_patch_requires_timer_when_handler_disabled(repo) -> None: + item = await repo.create({"name": "Disabled Handler", "url": "https://example.com/c", "timer": "0 0 * * *"}) + + request = _json_request( + "PATCH", + f"/api/tasks/{item.id}", + {"timer": "", "handler_enabled": False}, + match_info={"id": str(item.id)}, + ) + + response = await router.tasks_patch(request, repo, Encoder(), _Notify(), _Handler(matched=True)) + + assert response.status == web.HTTPBadRequest.status_code + assert b"handler is disabled" in response.body diff --git a/ui/app/components/TaskForm.vue b/ui/app/components/TaskForm.vue index f753cea4..5f0c9c4f 100644 --- a/ui/app/components/TaskForm.vue +++ b/ui/app/components/TaskForm.vue @@ -367,7 +367,9 @@

Enable Handler

-

Handlers run regardless of task timer.

+

+ Tasks without a supported handler must use a timer. +

@@ -443,6 +445,10 @@ scheduled downloads to yt-dlp. Disable Enable Handler to disable RSS monitoring. +
  • + Timer Requirement: if no supported handler matches the URL, you must + set a CRON timer before saving. +
  • @@ -523,6 +529,7 @@ const emitter = defineEmits<{ const toast = useNotification(); const config = useYtpConfig(); const dialog = useDialog(); +const tasksComposable = useTasks(); const { findPreset, getPresetDefault, selectItems } = usePresetOptions(); const showImport = useStorage('showTaskImport', false); @@ -786,6 +793,13 @@ const checkInfo = async (): Promise => { form.cli = form.cli.trim(); } + try { + await requireTimerForTask(form); + } catch (error: any) { + toast.error(error.message); + return; + } + if (urls.length === 1) { emitter('submit', { reference: toRaw(props.reference), @@ -811,12 +825,32 @@ const checkInfo = async (): Promise => { } as Task; } - return { url } as Task; + return { + url, + preset: form.preset, + timer: form.timer, + handler_enabled: form.handler_enabled, + } as Task; + }); + + try { + await Promise.all(tasks.map((item) => requireTimerForTask(item))); + } catch (error: any) { + toast.error(error.message); + return; + } + + const submitTasks: Task[] = tasks.map((item, idx) => { + if (idx === 0) { + return item; + } + + return { url: item.url } as Task; }); emitter('submit', { reference: toRaw(props.reference), - task: tasks, + task: submitTasks, archive_all: archiveAllAfterAdd.value, }); }; @@ -943,6 +977,34 @@ const convertCurrentUrl = async (): Promise => { form.url = await convert_url(form.url); }; +const requireTimerForTask = async ( + item: Pick, +): Promise => { + if (item.timer?.trim()) { + return; + } + + if (item.handler_enabled === false) { + throw new Error('This task needs a CRON timer because the handler is disabled.'); + } + + const result = await tasksComposable.inspectTaskHandler({ + url: item.url, + preset: item.preset, + static_only: true, + }); + + if (!result) { + throw new Error('Failed to verify handler support. Set a CRON timer or try again.'); + } + + if (result?.matched) { + return; + } + + throw new Error('This task needs a CRON timer because no supported handler matches the URL.'); +}; + const getDefault = (type: 'cookies' | 'cli' | 'template' | 'folder', ret: string = '') => { if (false !== hasFormatInConfig.value || !form.preset) { return ret; diff --git a/ui/app/pages/tasks.vue b/ui/app/pages/tasks.vue index 713a76e4..432da30a 100644 --- a/ui/app/pages/tasks.vue +++ b/ui/app/pages/tasks.vue @@ -661,8 +661,8 @@ Actions > Archive All to mark existing items as already downloaded.
  • - Custom Handlers: Leave timer empty for custom handler definitions. The - handler runs hourly and doesn't require timer. + Timer Requirement: Tasks must have either a valid timer or a supported + handler. If the URL does not match a handler, set a CRON timer.
  • Generate metadata: will attempt first to save metadata based on the diff --git a/ui/tests/composables/useTasks.test.ts b/ui/tests/composables/useTasks.test.ts index 8940cbb4..d515095c 100644 --- a/ui/tests/composables/useTasks.test.ts +++ b/ui/tests/composables/useTasks.test.ts @@ -258,6 +258,29 @@ describe('useTasks', () => { requestSpy.mockRestore() }) + it('store_timer_required_error', async () => { + const requestSpy = spyOn(utils, 'request') + requestSpy.mockResolvedValueOnce( + createMockResponse({ + ok: false, + status: 400, + jsonData: { error: 'Task requires a timer when no supported handler matches the URL.' }, + }), + ) + + const tasks = useTasks() + const newTask = { ...mockTask } + delete (newTask as any).id + + const result = await tasks.createTask(newTask) + + expect(result).toBeNull() + expect(tasks.lastError.value).toBe( + 'Task requires a timer when no supported handler matches the URL.', + ) + requestSpy.mockRestore() + }) + }) describe('updateTask', () => { @@ -280,6 +303,26 @@ describe('useTasks', () => { requestSpy.mockRestore() }) + it('store_update_timer_error', async () => { + const requestSpy = spyOn(utils, 'request') + requestSpy.mockResolvedValueOnce( + createMockResponse({ + ok: false, + status: 400, + jsonData: { error: 'Task requires a timer when the handler is disabled.' }, + }), + ) + + const tasks = useTasks() + const updated = { ...mockTask, timer: '', handler_enabled: false } + + const result = await tasks.updateTask(1, updated) + + expect(result).toBeNull() + expect(tasks.lastError.value).toBe('Task requires a timer when the handler is disabled.') + requestSpy.mockRestore() + }) + it('strip_update_id', async () => { const requestSpy = spyOn(utils, 'request') requestSpy.mockResolvedValueOnce( @@ -318,6 +361,26 @@ describe('useTasks', () => { requestSpy.mockRestore() }) + it('store_patch_timer_error', async () => { + const requestSpy = spyOn(utils, 'request') + requestSpy.mockResolvedValueOnce( + createMockResponse({ + ok: false, + status: 400, + jsonData: { error: 'Task requires a timer when no supported handler matches the URL.' }, + }), + ) + + const tasks = useTasks() + const result = await tasks.patchTask(1, { timer: '' }) + + expect(result).toBeNull() + expect(tasks.lastError.value).toBe( + 'Task requires a timer when no supported handler matches the URL.', + ) + requestSpy.mockRestore() + }) + }) describe('deleteTask', () => {