fix: tasks should require timer when there are no handler support.
This commit is contained in:
arabcoders
parent
7ac7039592
commit
b438947b0d
5 changed files with 384 additions and 11 deletions
|
|
@ -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))
|
||||
|
||||
|
|
|
|||
183
app/features/tasks/tests/test_tasks_router.py
Normal file
183
app/features/tasks/tests/test_tasks_router.py
Normal file
|
|
@ -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
|
||||
|
|
@ -367,7 +367,9 @@
|
|||
<UIcon name="i-lucide-rss" class="size-4 text-toned" />
|
||||
<p class="text-sm font-semibold text-default">Enable Handler</p>
|
||||
</div>
|
||||
<p class="text-xs text-toned">Handlers run regardless of task timer.</p>
|
||||
<p class="text-xs text-toned">
|
||||
Tasks without a supported handler must use a timer.
|
||||
</p>
|
||||
</div>
|
||||
<USwitch v-model="form.handler_enabled" :disabled="addInProgress" />
|
||||
</div>
|
||||
|
|
@ -443,6 +445,10 @@
|
|||
scheduled downloads to yt-dlp. Disable <code>Enable Handler</code> to disable RSS
|
||||
monitoring.
|
||||
</li>
|
||||
<li>
|
||||
<strong>Timer Requirement:</strong> if no supported handler matches the URL, you must
|
||||
set a CRON timer before saving.
|
||||
</li>
|
||||
</ul>
|
||||
</template>
|
||||
</UAlert>
|
||||
|
|
@ -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<void> => {
|
|||
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<void> => {
|
|||
} 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<void> => {
|
|||
form.url = await convert_url(form.url);
|
||||
};
|
||||
|
||||
const requireTimerForTask = async (
|
||||
item: Pick<Task, 'url' | 'preset' | 'timer' | 'handler_enabled'>,
|
||||
): Promise<void> => {
|
||||
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;
|
||||
|
|
|
|||
|
|
@ -661,8 +661,8 @@
|
|||
<code>Actions > Archive All</code> to mark existing items as already downloaded.
|
||||
</li>
|
||||
<li>
|
||||
<strong>Custom Handlers:</strong> Leave timer empty for custom handler definitions. The
|
||||
handler runs hourly and doesn't require timer.
|
||||
<strong>Timer Requirement:</strong> Tasks must have either a valid timer or a supported
|
||||
handler. If the URL does not match a handler, set a CRON timer.
|
||||
</li>
|
||||
<li>
|
||||
<strong>Generate metadata:</strong> will attempt first to save metadata based on the
|
||||
|
|
|
|||
|
|
@ -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', () => {
|
||||
|
|
|
|||
Loading…
Reference in a new issue